You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
AI code review — automated review for reference; please use your judgment.
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.
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.
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.
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)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 asInvalid server routeand 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:
v2withTypeError: undefined is not an object (evaluating 'state.store')Verification:
bun test --conditions=solid --preload ./happydom.ts ./src/session/session-resolution.test.tsbun typecheck