Skip to content

fix(web): await the platform-session probe before the onboarding hosting/welcome fork#33201

Merged
vex-assistant-bot[bot] merged 1 commit into
mainfrom
fix-auth-middleware-platform-session-race
Jun 3, 2026
Merged

fix(web): await the platform-session probe before the onboarding hosting/welcome fork#33201
vex-assistant-bot[bot] merged 1 commit into
mainfrom
fix-auth-middleware-platform-session-race

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Prompt / plan

While investigating "login doesn't work in the local Electron app," I traced the local-mode auth bootstrap end-to-end and found a concrete, code-provable race in the auth middleware's onboarding routing. This PR fixes that race. (No live macOS repro — this is a code-level root-cause fix verified by a unit test.)

What was broken

In local mode with no assistants yet, the auth middleware forks the user to either the onboarding hosting picker or the new-user welcome flow based on hasPlatformSession:

const { hasPlatformSession } = useAuthStore.getState();
throw redirect(hasPlatformSession ? routes.onboarding.hosting : routes.onboarding.welcome);

But hasPlatformSession is set by a fire-and-forget platform-session probe (probePlatformSession) — the local gateway auth paths in initSession return control before the probe settles. When the middleware runs inside that window, hasPlatformSession reads an ambiguous false, so a returning platform user is sent to the new-user welcome flow instead of the hosting picker.

This is the same "ambiguous false" race that platformSessionResolved was introduced to fix in the prechat onboarding funnel (#33123) — but that gate was never applied to auth-middleware.ts. The store distinguishes "no session" from "not resolved yet" via platformSessionResolved; the middleware ignored it.

The fix

Wait for platformSessionResolved before reading hasPlatformSession, mirroring the existing waitForAuthReady() gate already used for isLoading. The probe always settles (probePlatformSession flips the flag in a finally), so the wait is bounded. In the no-platform-assistant local path initSession sets platformSessionResolved: true synchronously, so there is no added latency there.

Test plan

  • New apps/web/src/lib/auth/auth-middleware.test.ts (3 cases):
    • Race: with the probe in flight (platformSessionResolved: false), the middleware does not decide yet; once the probe settles with a live session it redirects to hosting (this fails against the old code, which would have redirected to welcome immediately).
    • Resolved + no platform session → welcome.
    • Resolved + platform session → hosting.
  • bun test src/lib/auth/auth-middleware.test.ts → 3 pass.
  • bun run lint and bunx tsc --noEmit (with regenerated API client, via the pre-commit hook) → clean.

Root cause analysis

  1. How did the code get into this state? platformSessionResolved was added in fix(web): gate onboarding Google step on a live platform session, not a cached id #33123 to fix the exact ambiguous-false race in the prechat funnel. The fix was applied at the funnel call site but not to auth-middleware.ts, which makes the same hosting-vs-welcome decision off the same flag. The middleware predates the resolved-flag concept and was never revisited.
  2. What decisions led to it? The local gateway auth paths intentionally return before the platform session is known (fire-and-forget probe) to avoid blocking startup. Consumers must therefore treat a not-yet-resolved session as "unknown," not "absent" — a contract the funnel learned but the middleware didn't.
  3. Warning signs missed? Two readers of hasPlatformSession making routing decisions, only one of them gated on platformSessionResolved. The asymmetry was the tell.
  4. Preventing recurrence? Any code that branches on hasPlatformSession for a navigation/routing decision must first ensure platformSessionResolved is true (or treat unresolved as unknown). Both readers now do.
  5. AGENTS.md update? Not warranted — this is a module-scoped contract, better captured by the doc comment on probePlatformSession (which already documents the resolved-flag semantics) and this fix, than by a project-wide rule that would risk going stale.

Notes

  • apps/web is a bundler package (moduleResolution: "Bundler"), so the test omits .js import extensions per repo convention.
  • No wire/protocol or IPC surface changes — this is renderer-internal routing logic, safe across macOS/platform version skew.

Link to Devin session: https://app.devin.ai/sessions/15bca57bd4c64a3085cfb80e1f26355a
Requested by: @ashleeradka

…ing/welcome fork

The local-mode auth middleware chose between the onboarding hosting and welcome screens off a bare `hasPlatformSession`. That flag is set by a fire-and-forget platform-session probe (the local gateway auth paths return before the session is known), so when the middleware runs during the probe window it reads an ambiguous `false` and sends a returning platform user -- one with zero local assistants -- to the new-user welcome flow instead of the hosting picker.

Wait for `platformSessionResolved` before forking, mirroring the `waitForAuthReady` gate already used for `isLoading` and the prechat funnel fix (#33123) that introduced the resolved flag for exactly this race.
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@vex-assistant-bot vex-assistant-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE

Value: Returning platform users in local mode reliably land on the hosting picker instead of the new-user welcome flow — closes the same ambiguous-false race as #33123 at the auth-middleware seam that was missed by that PR.

What this does: Adds a waitForPlatformSessionResolved() gate before the hosting-vs-welcome fork in auth-middleware.ts, mirroring the existing waitForAuthReady() pattern in the same file. Reads hasPlatformSession only after the probe has settled.

Analysis
  • Race shape matches #33123 exactly — local gateway initSession returns before the fire-and-forget probePlatformSession settles, so any consumer reading hasPlatformSession in that window sees an ambiguous false. #33123 introduced platformSessionResolved as the tri-state truth signal and gated the prechat funnel on it; this PR closes the missed sibling reader at auth-middleware.ts:48.
  • BoundednessprobePlatformSession flips platformSessionResolved=true in a finally (verified in #33123 auth-store.ts), so the wait can't hang. In the no-platform-assistant local path initSession flips it synchronously, so no added latency there either.
  • Subscribe-then-sync-check pattern is the canonical Zustand idiom (matches waitForAuthReady 4 lines above). Subscribe first, then read state synchronously to handle the already-resolved case. Zustand doesn't fire subscribers on initial subscribe, so no double-resolve risk.
  • Test coverage targets the exact race — case 1 asserts the middleware does NOT decide while platformSessionResolved=false, then verifies it routes to hosting once the probe settles with a live session. Would fail against pre-fix code (which would have immediately thrown welcome redirect).
  • Anti-pattern grep on diff: zero production as casts, zero ! in production code (settled! is test-only, guarded by adjacent expect(settled).not.toBeNull()), zero @ts-ignore, zero eslint-disable. Three test-scaffolding casts (as unknown as Parameters<...>, as AuthUser fixture) are conventional.
  • Cross-platform impact — renderer-internal routing logic, no wire/IPC surface change, safe across macOS/iOS/web.

Vellum Constitution — Trust-seeking: the asymmetry between two hasPlatformSession readers (one gated on resolved, one not) was the kind of silent contract drift that erodes trust — this PR makes the contract uniform.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant