fix(desktop): hold create guard until route catches new session - #66610
fix(desktop): hold create guard until route catches new session#66610stantheman0128 wants to merge 5 commits into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the create/route race; current main still has the delayed guard release at apps/desktop/src/app/session/hooks/use-session-actions/index.ts:344-348, and its stale-route self-heal can resume the old route at apps/desktop/src/app/session/hooks/use-route-resume.ts:137-150.
Problems
- The new suppression at
apps/desktop/src/app/session/hooks/use-route-resume.ts:150remains active after the proposed 3s timeout if navigation never moves from A to B: selection and active refs remain B, sostuckOnRoutedSessionstays false. That makes the claimed timeout recovery ineffective;ChatViewkeeps the route/selection mismatch in its loading state (apps/desktop/src/app/chat/index.tsx:281-315).
Suggested changes
- Tie this suppression to an active pending-create state, or reconcile route/selection when the timeout releases the guard. Add a regression test covering a route that never catches up and asserting recovery, not only guard release.
Automated hermes-sweeper review.
| const stuckOnRoutedSession = | ||
| routedSessionId !== selectedStoredSessionIdRef.current && | ||
| !freshDraftReady && | ||
| !selectionMovedAheadOfRoute | ||
|
|
There was a problem hiding this comment.
The 3s release does not restore recovery if navigation never reaches B: the route remains A while selected/active refs remain B, so this condition stays true and stuckOnRoutedSession never resumes A. Scope this suppression to pending creation or reconcile state when the timeout fires; add a test for that composed timeout path.
|
Thanks for flagging the timeout recovery hole. You were right: clearing Pushed
Evidence: |
270f9ec to
09423ec
Compare
…Research#66057) Stop setTimeout(0) from clearing creatingSessionRef before HashRouter lands on the created stored id, and ignore stale-route self-heal when selection already moved to the new runtime. Prevents resumeSession(A) jump-back after the first send on a new chat. Credit: Stan Shih (stantheman0128); AI Cursor/Grok Co-authored-by: Cursor <cursoragent@cursor.com>
Release creatingSessionRef when navigate throws, when the route leaves the pre-create location for a non-pending id, or after a 3s safety timeout so a stuck router cannot block resumes forever. Same clear on forkBranch catch. Co-authored-by: Cursor <cursoragent@cursor.com>
Scope selectionMovedAheadOfRoute to the pending-create hold and retry navigate when the 3s safety timeout fires so ChatView can leave a stuck A-route/B-selection loading state instead of suppressing resume forever. Co-authored-by: Cursor <cursoragent@cursor.com>
…trip An editing step on Windows decoded these files as cp950 and re-encoded them as ASCII, which turned every em dash, arrow and curly quote into "??" and swallowed the byte that followed it. The result was 34 comment lines showing up as unrelated changes in the diff. Restored each line by matching its mangled form against the merge-base blob, plus four comments added by this branch that had no upstream original. Only lines with a unique upstream match were rewritten, so nullish coalescing is untouched. No code changes. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…sses SessionActionsOptions gained routedSessionId and getRoutedStoredSessionId in this branch, but StoredIdRotationHarness only passed the getter and GuardHarness only passed the reactive id, so tsc failed on both call sites. Each harness already holds the route it is simulating; feed that to the missing option rather than a stub. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
09423ec to
7053884
Compare
|
Closing as author to bring our open PRs on hermes-agent back within a healthy throttle (we had 10 open with only sweeper keep_open and no concrete maintainer change requests for days). Keeping three Windows-focused PRs open for now:
Happy to reopen this one if a maintainer wants it prioritized. Thanks for the patience. |
Summary
Closes #66057
Credit: Stan Shih (@stantheman0128). Developed with AI assistance (Cursor / Grok).
After New Session, the first send could flash the new chat then jump back to the previous one.
createBackendSessionForSendupdated selection refs and navigated, then clearedcreatingSessionRefwithsetTimeout(0).use-route-resumecould then treat the still-old route as stuck andresumeSessionthe previous chat.Changes
creatingSessionReftrue until the route and selection agree on the created (or forked) stored session iduse-route-resume: if selection/runtime already moved ahead of a stale route, do not resume the old routeRelated but different: open #66376 covers stale resume at New Chat draft start, not this first-send create-route race.
Verification
`
npx vitest run --project ui src/app/session/hooks/use-route-resume.test.tsx src/app/session/hooks/use-session-actions.test.tsx
Test Files 2 passed | Tests 36 passed
`
`
python scripts/check-windows-footguns.py --diff HEAD~2
clean
`
Made with Cursor