Skip to content

Commit c27d65b

Browse files
christian-byrneclaude
authored andcommitted
fix(onboarding): re-check the canvas after the intro preview
Review feedback on #14673. The entry guard runs before `beginTour` awaits the renderer write and the 500 ms intro preview, so a switch into linear mode inside that window still opened the tour: the holds watcher fires while `activeTour` is still null, so it has nothing to end, and `engine.startTour` does not consult holds on the way in. Re-check before starting; the existing `!started` path already hands the renderer switch back. Also spy on `settingStore.set` rather than only reading the resulting value, so a refusal that flipped `Comfy.VueNodes.Enabled` and put it back can no longer pass as one that never touched it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent ca15761 commit c27d65b

2 files changed

Lines changed: 39 additions & 9 deletions

File tree

src/renderer/extensions/firstRunTour/tour/useFirstRunTourController.test.ts

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ const mocks = vi.hoisted(() => ({
2525
activeWorkflow: { value: null as unknown },
2626
linearMode: { value: false },
2727
vueNodesEnabled: true,
28+
setSetting: vi.fn(),
2829
steps: [] as CoachStep[],
2930
runState: { value: 'idle' } as Ref<string>,
3031
releaseFirstRunTargets: vi.fn(),
@@ -94,10 +95,9 @@ vi.mock('@/renderer/core/canvas/canvasStore', async () => {
9495
vi.mock('@/platform/settings/settingStore', () => ({
9596
useSettingStore: () => ({
9697
get: () => mocks.vueNodesEnabled,
97-
set: (_key: string, value: boolean) => {
98-
mocks.vueNodesEnabled = value
99-
return Promise.resolve()
100-
}
98+
// A spy, not a plain writer: a value that was flipped and put back reads
99+
// the same as one that was never touched.
100+
set: mocks.setSetting
101101
})
102102
}))
103103

@@ -238,6 +238,10 @@ describe('useFirstRunTourController', () => {
238238
mocks.activeWorkflow.value = null
239239
mocks.linearMode.value = false
240240
mocks.vueNodesEnabled = true
241+
mocks.setSetting.mockImplementation((_key: string, value: boolean) => {
242+
mocks.vueNodesEnabled = value
243+
return Promise.resolve()
244+
})
241245
mocks.steps = []
242246
mocks.engine.activeTour = null
243247
mocks.engine.lastEnding = null
@@ -339,9 +343,9 @@ describe('useFirstRunTourController', () => {
339343
'the cards would sit over a hidden canvas until their targets timed out'
340344
).not.toHaveBeenCalled()
341345
expect(
342-
mocks.vueNodesEnabled,
343-
'a tour that never opened must not leave the renderer switched behind it'
344-
).toBe(false)
346+
mocks.setSetting,
347+
'a tour that never opened must not touch the renderer setting at all'
348+
).not.toHaveBeenCalledWith('Comfy.VueNodes.Enabled', true)
345349
})
346350

347351
it('refuses to open on a viewport below the desktop layout', async () => {
@@ -358,7 +362,30 @@ describe('useFirstRunTourController', () => {
358362
'the spotlights are placed against a desktop layout, so below md they point nowhere'
359363
).toBe(false)
360364
expect(mocks.engine.startTour).not.toHaveBeenCalled()
361-
expect(mocks.vueNodesEnabled).toBe(false)
365+
expect(mocks.setSetting).not.toHaveBeenCalledWith(
366+
'Comfy.VueNodes.Enabled',
367+
true
368+
)
369+
})
370+
371+
it('refuses to open when the canvas goes away during the intro preview', async () => {
372+
mocks.vueNodesEnabled = false
373+
mocks.steps = [runStep()]
374+
const controller = await freshController()
375+
376+
const starting = controller.beginTour('image_z_image_turbo')
377+
mocks.linearMode.value = true
378+
await vi.advanceTimersByTimeAsync(INTRO_PREVIEW_MS)
379+
380+
expect(
381+
await starting,
382+
'the holds watcher cannot catch this — there is no active tour to end yet'
383+
).toBe(false)
384+
expect(mocks.engine.startTour).not.toHaveBeenCalled()
385+
expect(
386+
mocks.vueNodesEnabled,
387+
'the renderer switch thrown for a tour that never opened is handed back'
388+
).toBe(false)
362389
})
363390

364391
it('leaves the workflow undimmed before taking the screen over', async () => {

src/renderer/extensions/firstRunTour/tour/useFirstRunTourController.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,10 @@ function useFirstRunTourControllerInternal() {
171171
tourContextHolds
172172
)
173173
await delay(INTRO_PREVIEW_MS)
174-
const started = await engine.startTour('firstRun')
174+
// The preview is long enough for the canvas to go away underneath it, and
175+
// the holds watcher cannot catch that: there is no active tour to end yet.
176+
const started =
177+
canvasContextHolds.value && (await engine.startTour('firstRun'))
175178
if (!started) {
176179
releaseFirstRunTargets()
177180
tourWorkflow.value = null

0 commit comments

Comments
 (0)