Skip to content

fix(app): wait for session route id - #44046

Open
opencode-agent[bot] wants to merge 1 commit into
v2from
opencode/session-route-id
Open

fix(app): wait for session route id#44046
opencode-agent[bot] wants to merge 1 commit into
v2from
opencode/session-route-id

Conversation

@opencode-agent

Copy link
Copy Markdown
Contributor

Desktop route transitions can temporarily clear the session ID before the previous route owner is disposed. During that window, session resolution has neither a target ID nor status.

The status guard previously compared two missing IDs, then dereferenced state.store. That error entered the session fallback, whose reactive server-key prop had also cleared, masking the original failure as Invalid server route and escalating to the renderer fatal screen.

This change models the route ID as optional at the resolution boundary and returns until an ID and matching status exist. It also avoids starting session sync without a target ID.

Regression test:

  • fails on v2 with TypeError: undefined is not an object (evaluating 'state.store')
  • passes with this change and verifies no sync starts without a route ID

Verification:

  • bun test --conditions=solid --preload ./happydom.ts ./src/session/session-resolution.test.ts
  • bun typecheck

@Hona
Hona enabled auto-merge (squash) August 22, 2026 03:18
@Hona
Hona disabled auto-merge August 22, 2026 03:18
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

  1. packages/app/src/session/session-resolution.test.ts:6 — The new test only covers the static undefined case (no route id ever present). The riskier path this fix touches is the transition defined → undefined → defined during navigation, where status still holds a settled Resolution for the previous id and the memo must not surface it. Why it matters: a regression there shows the wrong session's data mid-navigation. Suggestion: add a test driving sessionID through a signal across that transition and asserting undefined between hops and a refetch after.

  2. packages/app/src/session/session-resolution.ts:70 — if (!state || state.id !== id || …) is semantically identical to the old state?.id !== id || …; presumably kept for TypeScript narrowing of state.store. Why it matters: a future reader may "simplify" it back and hit the same type hole or miss its purpose. Suggestion: one short comment (// narrows state for TS) to pin intent.

  3. packages/app/src/session/session-resolution.ts:31 — The header contract comment still implies an always-present id ("Resolve failures rethrow on read…"), and the signature silently widened to string | undefined. Why it matters: callers need to know undefined means "route id not ready yet — render nothing," not "session missing." Suggestion: extend the comment to document the waiting semantics introduced here.

  4. packages/app/src/session/session-resolution.ts:38 (nit) — On defined→undefined the effect early-returns but leaves the last settled value in status; correctness currently relies solely on the memo's !id guard. Consider also clearing status on cleanup so the two sources can't drift apart in future edits.

— automated review (ox-alpha, round2)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant