fix(first-run-tour): do not spend the tutorial flag on a deferred url-intent boot (#15355)

## What

Two fixes from chasing the reported ~1-in-10 flake in
`gettingStartedTour.spec.ts › arriving on a template link › tours the
template the link loaded`
(`coach-spotlight` never appears).

**1. The flake is a harness defect, not an app race.**

`comfyPage.setupSettings()` seeds `initialSettings` through the devtools
`/devtools/set_settings` route. The fixture wrapped that call in
`try { … } catch (e) { console.error(e) }`, so when the route is
unavailable the
seed is silently skipped and every test runs against whatever the shared
per-worker user (`playwright-test-<parallelIndex>`) last persisted
server-side.

For this test that means `Comfy.TutorialCompleted` stays `true` from an
earlier
test, `resolveStartupOutcome()` returns `restored` instead of
`url-intent`,
`handleUrlWorkflow` correctly declines, and the failure surfaces as
`coach-spotlight` not found — several layers away from the cause.
Whether a
given worker's user file happens to hold `true` is what makes it look
like a
low-rate race. Adding `waitForNodes()` does not help because the nodes
were
never the problem.

Instrumented proof (temporary build, since `console.info`/`log` are
dropped from
cloud builds by the `pure` list in `vite.config.mts`):

```
[TOURDBG] initializeWorkflow   {"search":"","tabState":null,"tutorialCompleted":true}
[TOURDBG] resolveStartupOutcome{"search":"?template=image_z_image_turbo","tutorialCompleted":true,…}
[TOURDBG] handleStartupOutcome {"outcome":"restored","tutorialCompleted":true}
[TOURDBG] handleUrlWorkflow    {"outcome":"restored","templateId":"image_z_image_turbo","candidate":false}
```

Every decision input the tour reads (`onboardingTourEnabled`,
`isDesktopWidth`,
`isSubscriptionEnabled`, `isNewUser`) was settled and correct. The seed
was the
only thing missing.

The fixture now lets the failure through, so a broken backend reports
`Failed to setup settings: 405: Method Not Allowed` on the first test
instead of
flaking somewhere else later.

**2. A real user-facing bug the same investigation exposed.**

`handleStartupOutcome` marked `Comfy.TutorialCompleted` unconditionally
for a
`url-intent` startup, *before* `handleUrlWorkflow` decided whether the
tour would
run. That flag is write-once and server-side. So a new cloud user
opening a
template or share link while the decision defers — window below the `md`
breakpoint, `onboarding_tour_enabled` off, `subscription_required` not
in remote
config yet — has their one first-run tour spent without ever seeing it,
on that
boot and every later boot, on every device.

The same write was also too early even for an eligible boot:
`handleUrlWorkflow`
can still decline when the link loaded nothing (the case
`arriving on a link that loads nothing` covers), spending the flag on a
tour
that never ran.

So the write now belongs to whatever earns it. `handleStartupOutcome`
marks a
`url-intent` boot only for a `complete` decision; `handleUrlWorkflow`
marks it
once `beginTour()` reports a tour actually started. A deferred boot, a
dead
link, or a start the engine refused all leave the account eligible — the
next
boot has no URL to honour, so it offers Getting Started, which is the
onboarding
the failed boot owed. Candidacy is still read once per boot:
`isNewUser()` is
decided by `initializeIfNewUser()` and cached, not re-read per call,
which is
what makes this ordering safe.

## Measurements

| harness | before | after |
| --- | --- | --- |
| devtools route down (as reported) | 4/10, then 0/6 as the leaked
`true` accumulates | fails immediately with the real reason |
| devtools route healthy | 50/50 | 20/20, and 18/18 for the whole spec
file |

`--repeat-each=50 --workers=14` on a healthy backend does not reproduce
the
flake before or after, which is the control for "this was never a timing
race".
No timeouts were raised.

The `405` came from `custom_nodes/ComfyUI_devtools/__init__.py` aborting
on
`from comfy_api.v0_0_2 import IO` against an older ComfyUI checkout —
the node
import runs before the route registrations, so all of its endpoints go
missing.
Worth pinning devtools to the backend it is tested against separately;
this PR
only makes the frontend notice.

## Test plan

- `pnpm vitest run
src/renderer/extensions/firstRunTour/gettingStarted/firstRunEntry.test.ts`
— 37 passed; 5 of them fail without the source change (red/green
verified).
- `pnpm vitest run
src/components/graph/GraphCanvas.firstRunEntry.test.ts` — passed.
- `pnpm typecheck`, `pnpm typecheck:browser`, `pnpm lint`, `pnpm
format:check` — clean.
- `browser_tests/tests/gettingStartedTour.spec.ts` full file,
`--repeat-each=3` — 18/18.
- `nodeSearchBox.spec.ts` run with and without the fixture change to
confirm its 23 local failures are pre-existing environment breakage, not
this diff.

## Note for reviewers

`browser_tests/tests/gettingStartedTour.spec.ts` is also edited on
`glary/layout-ref-retain-release` (#15092), which adds a test to this
same
describe. This PR deliberately leaves that spec untouched — the fixes
are in app
code and in the shared fixture — so there is no conflict.

Author: christian-byrne

browser_tests/fixtures/ComfyPage.ts

@@ -645,11 +645,7 @@ export const comfyPageFixture = base.extend<{
         ...(isVueNodes && { 'Comfy.VueNodes.Enabled': true }),
         ...initialSettings
       }
-      try {
-        await comfyPage.setupSettings(startupSettings)
-      } catch (e) {
-        console.error(e)
-      }
+      await comfyPage.setupSettings(startupSettings)
       if (testInfo.tags.includes('@cloud')) {
         const context = page.context()
         await context.route('**/api/auth/session', (route) =>

src/renderer/extensions/firstRunTour/gettingStarted/firstRunEntry.test.ts

@@ -3,6 +3,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
 import type * as VueUseCoreModule from '@vueuse/core'
 
 import type { StartupOutcome } from '@/platform/workflow/persistence/base/draftTypes'
+import type { SharedWorkflowUrlLoadStatus } from '@/platform/workflow/sharing/composables/useSharedWorkflowUrlLoader'
 
 type VueUseCore = typeof VueUseCoreModule
 
@@ -77,6 +78,8 @@ async function freshEntry() {
   return useFirstRunEntry()
 }
 
+type FirstRunEntry = Awaited<ReturnType<typeof freshEntry>>
+
 describe('useFirstRunEntry', () => {
   beforeEach(() => {
     mocks.isCloud = true
@@ -88,8 +91,23 @@ describe('useFirstRunEntry', () => {
     mocks.setSetting.mockImplementation((key: string, value: unknown) => {
       mocks.settings[key] = value
     })
+    // beginTour reports whether a tour actually started; default to the
+    // ordinary case so only tests about a refused start have to say so.
+    mocks.beginTour.mockResolvedValue(true)
   })
 
+  const permanentDisqualifiers = [
+    ['not on cloud', () => void (mocks.isCloud = false)],
+    ['a returning user', () => void (mocks.isNewUser = false)]
+  ] as const
+
+  const transientDisqualifiers = [
+    ['below the md breakpoint', () => void (mocks.isDesktopWidth = false)],
+    ['subscription disabled', () => void (mocks.subscriptionEnabled = false)],
+    ['new-user state undetermined', () => void (mocks.isNewUser = null)],
+    ['the tour flag off', () => void (mocks.tourFlag = false)]
+  ] as const
+
   describe('what a fresh user sees', () => {
     it('shows Getting Started to a candidate', async () => {
       const entry = await freshEntry()
@@ -100,18 +118,6 @@ describe('useFirstRunEntry', () => {
       expect(mocks.execute).not.toHaveBeenCalled()
     })
 
-    const permanentDisqualifiers: [string, () => void][] = [
-      ['not on cloud', () => void (mocks.isCloud = false)],
-      ['a returning user', () => void (mocks.isNewUser = false)]
-    ]
-
-    const transientDisqualifiers: [string, () => void][] = [
-      ['below the md breakpoint', () => void (mocks.isDesktopWidth = false)],
-      ['subscription disabled', () => void (mocks.subscriptionEnabled = false)],
-      ['new-user state undetermined', () => void (mocks.isNewUser = null)],
-      ['the tour flag off', () => void (mocks.tourFlag = false)]
-    ]
-
     it.for([...permanentDisqualifiers, ...transientDisqualifiers])(
       'opens the template browser instead, with %s',
       async ([, disqualify]) => {
@@ -174,7 +180,7 @@ describe('useFirstRunEntry', () => {
     })
   })
 
-  it('marks a url-intent startup completed without taking over the screen', async () => {
+  it('marks a url-intent startup completed once its tour starts, without taking over the screen', async () => {
     const entry = await freshEntry()
 
     await entry.handleStartupOutcome('url-intent')
@@ -184,12 +190,86 @@ describe('useFirstRunEntry', () => {
       'A share or template link is the user’s choice; onboarding must not cover it'
     ).toBe(false)
     expect(mocks.execute).not.toHaveBeenCalled()
+
+    await entry.handleUrlWorkflow('url-intent', 'image_z_image_turbo')
+
     expect(
       mocks.setSetting,
       'Without this the template browser reopens on every launch, as it did before this flow existed'
     ).toHaveBeenCalledWith('Comfy.TutorialCompleted', true)
   })
 
+  /** Both shapes of URL a first-run boot can arrive on. */
+  const urlArrivals = [
+    ['a template link', 'image_z_image_turbo', undefined],
+    ['a share link', undefined, 'loaded']
+  ] as const satisfies readonly [
+    string,
+    string | undefined,
+    SharedWorkflowUrlLoadStatus | undefined
+  ][]
+
+  const deferredUrlBoots = transientDisqualifiers.flatMap(([why, disqualify]) =>
+    urlArrivals.map(
+      ([arrival, templateId, sharedStatus]) =>
+        [
+          `${arrival} with ${why}`,
+          disqualify,
+          templateId,
+          sharedStatus
+        ] as const
+    )
+  )
+
+  it.for(deferredUrlBoots)(
+    'runs a url-intent boot to the end for %s, offering no tour and keeping eligibility',
+    async ([, disqualify, templateId, sharedStatus]) => {
+      disqualify()
+      const entry = await freshEntry()
+
+      await entry.handleStartupOutcome('url-intent')
+      await entry.handleUrlWorkflow('url-intent', templateId, sharedStatus)
+
+      expect(
+        mocks.beginTour,
+        'an ineligible boot has no tour to give'
+      ).not.toHaveBeenCalled()
+      expect(
+        entry.gettingStartedVisible.value,
+        'the link is the user’s choice; onboarding must not cover it'
+      ).toBe(false)
+      expect(
+        mocks.setSetting,
+        'no tour ran, so the write-once flag that pays for one must stay unspent for the boot that can lift this'
+      ).not.toHaveBeenCalled()
+    }
+  )
+
+  it('tours a template link on the boot after one that only deferred', async () => {
+    mocks.isDesktopWidth = false
+    const phone = await freshEntry()
+    await phone.handleStartupOutcome('url-intent')
+    await phone.handleUrlWorkflow('url-intent', 'image_z_image_turbo')
+
+    expect(mocks.beginTour).not.toHaveBeenCalled()
+
+    mocks.isDesktopWidth = true
+    // What `checkIsNewUser()` reads on the next launch.
+    mocks.isNewUser = !mocks.settings['Comfy.TutorialCompleted']
+    const laptop = await freshEntry()
+    await laptop.handleStartupOutcome('url-intent')
+    await laptop.handleUrlWorkflow('url-intent', 'image_z_image_turbo')
+
+    expect(
+      mocks.beginTour,
+      'opening a template link on a phone and returning on a laptop is ordinary behaviour'
+    ).toHaveBeenCalledWith('image_z_image_turbo')
+    expect(
+      mocks.settings['Comfy.TutorialCompleted'],
+      'the flag is spent on the boot that finally delivered the tour, not before'
+    ).toBe(true)
+  })
+
   it('never onboards over restored work, even for an apparent new user', async () => {
     const entry = await freshEntry()
 
@@ -205,18 +285,34 @@ describe('useFirstRunEntry', () => {
     ).not.toHaveBeenCalled()
   })
 
-  it.for(['fresh', 'url-intent'] as const)(
+  /**
+   * A `url-intent` boot only settles once `handleUrlWorkflow` has seen what the
+   * link loaded, so the invariant is asserted over the pair of handlers that
+   * GraphCanvas awaits in turn, not over the first one alone.
+   */
+  const startups: [StartupOutcome, (entry: FirstRunEntry) => Promise<void>][] =
+    [
+      ['fresh', async () => {}],
+      [
+        'url-intent',
+        (entry) => entry.handleUrlWorkflow('url-intent', 'image_z_image_turbo')
+      ]
+    ]
+
+  it.for(startups)(
     'settles the first-run decision on a %s startup rather than leaving it pending',
-    async (outcome: StartupOutcome) => {
+    async ([outcome, finishBoot]) => {
       const entry = await freshEntry()
 
       await entry.handleStartupOutcome(outcome)
+      await finishBoot(entry)
 
       expect(
         entry.gettingStartedVisible.value ||
+          mocks.beginTour.mock.calls.length > 0 ||
           mocks.setSetting.mock.calls.length > 0 ||
           mocks.execute.mock.calls.length > 0,
-        'a startup that opened a blank canvas must offer onboarding, record that it is done, or open the template browser; anything else strands the user with an empty screen'
+        'a startup that opened a blank canvas must offer onboarding, tour what the link loaded, record that it is done, or open the template browser; anything else strands the user with an empty screen'
       ).toBe(true)
     }
   )
@@ -259,7 +355,7 @@ describe('useFirstRunEntry', () => {
       ).toHaveBeenCalledWith(undefined)
     })
 
-    it('leaves the completion flag to the startup handler that already settled it', async () => {
+    it('leaves the completion flag alone when the engine refused to start', async () => {
       const entry = await freshEntry()
       await entry.handleStartupOutcome('url-intent')
       mocks.setSetting.mockClear()
@@ -269,7 +365,23 @@ describe('useFirstRunEntry', () => {
 
       expect(
         mocks.setSetting,
-        'writing it again here would mark onboarding done for a user whose tour never started, and postpone() exists to offer that user the tour again'
+        'writing it here would mark onboarding done for a user whose tour never started, and postpone() exists to offer that user the tour again'
+      ).not.toHaveBeenCalled()
+    })
+
+    it('keeps the account eligible when the link loaded nothing to tour', async () => {
+      const entry = await freshEntry()
+
+      await entry.handleStartupOutcome('url-intent')
+      await entry.handleUrlWorkflow('url-intent', undefined, 'failed')
+
+      expect(
+        mocks.beginTour,
+        'there is no workflow on the canvas to tour'
+      ).not.toHaveBeenCalled()
+      expect(
+        mocks.setSetting,
+        'a dead link must not spend the one tour the account gets; the next boot has no URL to honour and offers Getting Started instead'
       ).not.toHaveBeenCalled()
     })
 

src/renderer/extensions/firstRunTour/gettingStarted/firstRunEntry.ts

@@ -52,23 +52,19 @@ export const useFirstRunEntry = createSharedComposable(() => {
     return decideFirstRun() === 'getting-started'
   }
 
-  /**
-   * Candidacy is read once, here: a later breakpoint, flag or subscription
-   * change must not unmount the screen out from under the user.
-   * Only a boot that opened a blank canvas can be onboarded over. `isNewUser()`
-   * cannot carry that alone — it reads `Comfy.TutorialCompleted`, which is
-   * exactly what a user predating the setting is missing.
-   */
+  // `url-intent` defers to handleUrlWorkflow: we don't know yet whether
+  // anything arrived to tour, and TutorialCompleted is write-once.
   async function handleStartupOutcome(outcome: StartupOutcome) {
     if (outcome === 'restored') return
     if (settingStore.get('Comfy.TutorialCompleted')) return
 
+    const decision = decideFirstRun()
+
     if (outcome === 'url-intent') {
-      await markTutorialCompleted()
+      if (decision === 'complete') await markTutorialCompleted()
       return
     }
 
-    const decision = decideFirstRun()
     if (decision === 'getting-started') {
       gettingStartedVisible.value = true
       return
@@ -82,6 +78,11 @@ export const useFirstRunEntry = createSharedComposable(() => {
    * A share or template link loads its workflow instead of the Getting Started
    * screen, so the tour is offered over whatever arrived. The engine declines
    * to repeat a tour the user has already seen.
+   *
+   * A tour that actually started is what `Comfy.TutorialCompleted` pays for, so
+   * only that writes it. A link that loaded nothing, or a start the engine
+   * refused, leaves the account eligible: the next boot has no URL to honour and
+   * offers Getting Started, which is the onboarding this one failed to deliver.
    */
   async function handleUrlWorkflow(
     outcome: StartupOutcome | undefined,
@@ -92,9 +93,10 @@ export const useFirstRunEntry = createSharedComposable(() => {
     const shareLoaded =
       sharedStatus === 'loaded' || sharedStatus === 'loaded-without-assets'
     if (templateId === undefined && !shareLoaded) return
-    await useFirstRunTourController().beginTour(
+    const started = await useFirstRunTourController().beginTour(
       shareLoaded ? undefined : templateId
     )
+    if (started) await markTutorialCompleted()
   }
 
   // Applied locally before the request, so a failed write is next launch's problem.