fix(desktop): scope submit drift guard to genuine session switches - #64327
fix(desktop): scope submit drift guard to genuine session switches#64327Kenmege wants to merge 2 commits into
Conversation
The submit "session context drift" guard (regression 7acaff5 / NousResearch#54527, partially fixed by 8c28876 and da52ffe) aborted a prompt submission whenever the selected stored id OR the route token changed mid-submit. Both signals churn programmatically on a busy gateway, so on machines with background streaming sessions, per-minute cron sessions, the Telegram surface, or gateway-profile switches, essentially every send from a second chat aborted silently: the optimistic message was dropped, the draft was left in the composer, no error was shown, and prompt.submit never fired. The false-positive churn sources were: - selection null-resets — gateway-switch's setSelectedStoredSessionId(null) on a gateway/profile switch or reconnect read as a switch away; - search/hash-only route-token changes — overlays and side panels park state in location.search/hash, so the pathname (the only part that selects a chat) was unchanged yet the raw token differed; - background-event active-ref retargets — createBackendSessionForSend's 3-prong check also watched activeSessionIdRef, which gateway events retarget while other sessions stream (NousResearch#47709 class), during a seconds-long session.create round-trip. New shared helper session-context-drift.ts reduces a route token to the chat it targets (pathname only; the new-chat route is '__new__', non-chat routes null) and reports drift only when selection or the routed chat moves to a DIFFERENT, non-null chat that is not the submit's own target. Selection null-resets, search/hash-only churn, and moves onto the submit target are no longer drift; genuine user switches (click another chat, click New Session mid-submit) still abort. Site A (submit.ts) routes all five guard points through the helper and logs '[submit-drift-abort]' with a per-site phase; the post-create active-ref check and baseline re-pin from 8c28876 are kept intact. Site B (createBackendSessionForSend) drops the active-ref prong entirely — every real switch retargets selection and route synchronously — and logs before closing the orphaned session. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Thanks for narrowing the drift predicate without removing the real misrouting protection. The current main still has the raw guard at Problems
Suggested changes
Current main has moved since the PR base and GitHub reports the branch as merge-conflicted, but the target code remains local and salvageable. This is an automated hermes-sweeper review. |
Reproduced on macOS Desktop v0.18.xConfirming this bug reproduces consistently on Hermes Desktop on macOS (current main, build with commit Symptoms
Both symptoms disappear after applying the changes from this PR (verified locally — the drift guard no longer aborts on programmatic ref/route changes, while genuine session switches are still correctly blocked). This fix is critical for daily use — without it the desktop app is practically unusable (the first message is lost every time on a fresh session). Really looking forward to seeing this merged and released. Thanks for working on this! |
…ift-scope-upstream # Conflicts: # apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts
|
Updated this branch to current |
|
looks mergeable Review setup: I reviewed a run-owned patch replay against current GitHub Security evidence:
Signed: GPT-5.6-sol-xhigh in Codex |
…ready
run_after_agent_ready() silently returned when _turn_cancel_requested or
running=False was set during lazy agent startup, leaving the Desktop with
a {"status":"streaming"} reply that never produced a message.start or
error event. The _wait_agent error branch 6 lines above already emits;
this mirrors it so the client can surface feedback instead of hanging.
This is the server-side half of #63078 — the client-side drift guard is
addressed by #64327, but even with that fix the server-side cancel race
still silently dropped the turn.
Adds two regression tests that capture _emit (the existing sibling test
mocked it to a no-op, so it could not catch the silent drop).
Closes #63078
…e submit pipeline (#64327) From PR #64327 (@Kenmege), whose target-aware drift predicate (compare route tokens by their routed chat target; ignore selection null-resets, search/hash churn, and background active-ref retargets; never count a move onto the submit's own target) already reached main via the #69578 salvage (1bdd478, Co-authored-by Kennedy Umege) and gained the #70986 composerScope prong. What main covered only at the unit level (session-context-drift.test.ts) is here pinned at the pipeline level, both directions: - a send from a second chat rides out simultaneous programmatic churn (selection null-reset from a gateway/profile reconnect, active-ref retarget from a background event, search/hash-only route change from an overlay) mid-session.resume and still reaches prompt.submit; - a genuine user switch (selection AND route moving to another real chat) mid-submit still aborts before prompt.submit. Part of the #63078 fix branch.
…ready
run_after_agent_ready() silently returned when _turn_cancel_requested or
running=False was set during lazy agent startup, leaving the Desktop with
a {"status":"streaming"} reply that never produced a message.start or
error event. The _wait_agent error branch 6 lines above already emits;
this mirrors it so the client can surface feedback instead of hanging.
This is the server-side half of NousResearch#63078 — the client-side drift guard is
addressed by NousResearch#64327, but even with that fix the server-side cancel race
still silently dropped the turn.
Adds two regression tests that capture _emit (the existing sibling test
mocked it to a no-op, so it could not catch the silent drop).
Closes NousResearch#63078
…e submit pipeline (NousResearch#64327) From PR NousResearch#64327 (@Kenmege), whose target-aware drift predicate (compare route tokens by their routed chat target; ignore selection null-resets, search/hash churn, and background active-ref retargets; never count a move onto the submit's own target) already reached main via the NousResearch#69578 salvage (b21b39f, Co-authored-by Kennedy Umege) and gained the NousResearch#70986 composerScope prong. What main covered only at the unit level (session-context-drift.test.ts) is here pinned at the pipeline level, both directions: - a send from a second chat rides out simultaneous programmatic churn (selection null-reset from a gateway/profile reconnect, active-ref retarget from a background event, search/hash-only route change from an overlay) mid-session.resume and still reaches prompt.submit; - a genuine user switch (selection AND route moving to another real chat) mid-submit still aborts before prompt.submit. Part of the NousResearch#63078 fix branch.
Summary
The submit "session context drift" guard (added in 7acaff5 / #54527) aborts a prompt submission whenever
$selectedStoredSessionIdor the raw route token (pathname:search:hash) changes mid-submit. Both signals churn programmatically on a busy gateway, so on machines with background streaming sessions, per-minute cron sessions, a messaging surface, or gateway-profile switches, essentially every send from a second chat aborts silently — optimistic message dropped, draft left in the composer, no error,prompt.submitnever fired. This affects existing sessions, not just the new-chat first-send covered by #63624 and the open first-send PRs.Fixes the client-side half of #63078 (see my comment for the live evidence). The server-side
_wait_agent30 s ceiling described there still deserves its own fix.False-positive churn sources removed
store/gateway-switch.tscallssetSelectedStoredSessionId(null)on a gateway/profile switch or reconnect; the guard read that as "user switched away".search/hash-only route changes — overlays and panels park state inlocation.search/hash; only the pathname selects a chat, but the raw token comparison fired anyway.createBackendSessionForSendstill comparedactiveSessionIdRef+ selection + raw token across the seconds-longsession.createround-trip (server-side agent/MCP init), during which background gateway events retarget the active ref ([Bug]: Desktop shows newer session output in an older session #47709 class). The just-created session wassession.closed and the send returned null.Change
session-context-drift.ts:routeTargetFromToken()reduces a route token to the chat it targets (routed session id,'__new__'for the new-chat route,nullfor non-chat routes);sessionContextDrift()returns a drift reason (or null): route prong fires only when the routed chat moves to a different, real chat; selection prong only when selection moves to a different, non-null stored id; both are target-aware, so the pipeline's own re-home onto the submit's target (fresh create, resume) never reads as drift.use-prompt-actions/submit.ts: all five drift checks use the helper; the 8c28876 post-create baseline re-pin and active-ref check are preserved unchanged.use-session-actions/index.ts(createBackendSessionForSend): mid-create check uses the helper; theactiveSessionIdRefprong is dropped (every genuine user switch retargets selection and route synchronously; the active ref churns from background events).console.warn('[submit-drift-abort]', reason, { phase }), so the next report of this class is diagnosable from the console.Genuine switches still abort: clicking another chat mid-submit (routed id + selection both move) and clicking "New session" while a send from an existing chat is in flight both keep the old behavior.
Testing
session-context-drift.test.ts: search/hash-only churn, selection null-reset, moves onto the submit target → no drift; moves to a different chat / the new-chat route → drift; non-chat routes → no drift.apps/desktop: full vitest suite 199 files / 1699 tests green (no existing test needed changes),npm run typecheckclean.🤖 Generated with Claude Code