Skip to content

fix(desktop): cancel stale session resume on new chat - #66376

Open
Astyyym wants to merge 2 commits into
NousResearch:mainfrom
Astyyym:fix/desktop-new-chat-stale-resume
Open

fix(desktop): cancel stale session resume on new chat#66376
Astyyym wants to merge 2 commits into
NousResearch:mainfrom
Astyyym:fix/desktop-new-chat-stale-resume

Conversation

@Astyyym

@Astyyym Astyyym commented Jul 17, 2026

Copy link
Copy Markdown

Summary

  • invalidate an in-flight session resume as soon as a new-chat draft starts
  • prevent stale profile-lookup or gateway-swap continuations from reselecting the old session
  • add a regression test for resume A -> New Chat -> stale resume completion

Testing

  • git diff --check
  • TypeScript check reaches the modified test without errors; remaining module-resolution errors are caused by running Windows Node against a WSL worktree
  • Vitest cannot start in this environment because Vite's watcher treats a linked workspace module as a directory; please rely on Linux CI for the full suite

@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 17, 2026
@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 18, 2026

@teknium1 teknium1 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.

Thanks for targeting the stale-resume generation race. The invalidation is the right mechanism, but it needs one more guard to cover the gateway-swap path described in the PR.

Problems

  • At apps/desktop/src/app/session/hooks/use-session-actions/index.ts:224, the PR invalidates a resume when New Chat starts. However, resumeSession checks the token only before await ensureGatewayProfile(sessionProfile) (PR-head lines 507-511). A New Chat during that await can still reach the warm-cache write at lines 538-544 or the cold-path selection at lines 578-598 and revive the old session.

Suggested changes

  • Re-check isCurrentResume() immediately after await ensureGatewayProfile(sessionProfile) and return before takeWarmCache() or any session-state writes.
  • Add a deferred gateway-profile test alongside the profile-lookup case, with New Chat occurring while the gateway swap is pending.

Automated hermes-sweeper review.

// user starts a new chat. Invalidate that request before clearing the
// view; otherwise its stale continuation can select and paint the old
// session over this fresh draft.
resumeRequestRef.current += 1

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.

This invalidates the lookup phase, but resumeSession does not re-check the token after await ensureGatewayProfile(sessionProfile). A New Chat during that gateway-swap await can still enter the warm/cold paths and reselect the old session; add an isCurrentResume() guard immediately after that await and cover it with a deferred-swap test.

@Astyyym Astyyym Jul 19, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You're right: the prior invalidation covered only the profile lookup. I added isCurrentResume() immediately after await ensureGatewayProfile(sessionProfile), before either the warm or cold resume path. The new deferred-swap regression covers: resume A -> wait until its profile swap is pending -> New Chat -> resolve the swap -> assert A cannot issue session.resume or restore active, selected, or message state. The targeted use-session-actions suite now passes: 20/20 tests. Pushed in 93fea22.

@teknium1 teknium1 added the area/sessions Session lifecycle, resume, persistence, history label Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants