mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-23 16:36:23 +00:00
fix(desktop): scope submit drift guard to genuine session switches (#69578)
The submit "session context drift" guard (regression7acaff5ef/ #54527, partially fixed by8c288760dandda52ffea1) 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 (#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 from8c288760dare 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. (cherry picked from commitb390e3a22e) Co-authored-by: Kennedy Umege <kenmege@yahoo.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
87088d1821
commit
1bdd478efa
4 changed files with 280 additions and 16 deletions
125
apps/desktop/src/app/session/hooks/session-context-drift.test.ts
Normal file
125
apps/desktop/src/app/session/hooks/session-context-drift.test.ts
Normal file
|
|
@ -0,0 +1,125 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { NEW_CHAT_ROUTE, sessionRoute, SETTINGS_ROUTE } from '../../routes'
|
||||
|
||||
import { routeTargetFromToken, sessionContextDrift } from './session-context-drift'
|
||||
|
||||
const SESS_A = 'sess-a'
|
||||
const SESS_B = 'sess-b'
|
||||
|
||||
// Build a route token the way desktop-controller does: pathname:search:hash.
|
||||
const routeToken = (pathname: string, search = '', hash = '') => `${pathname}:${search}:${hash}`
|
||||
|
||||
describe('routeTargetFromToken', () => {
|
||||
it('maps a session route to its session id, a non-chat route to null, and the new-chat route to __new__', () => {
|
||||
expect(routeTargetFromToken(routeToken(sessionRoute(SESS_A)))).toBe(SESS_A)
|
||||
expect(routeTargetFromToken(routeToken(SETTINGS_ROUTE))).toBeNull()
|
||||
expect(routeTargetFromToken(routeToken(NEW_CHAT_ROUTE))).toBe('__new__')
|
||||
})
|
||||
|
||||
it('ignores search and hash — only the pathname selects the chat', () => {
|
||||
expect(routeTargetFromToken(routeToken(sessionRoute(SESS_A), '?panel=preview', '#reply'))).toBe(SESS_A)
|
||||
})
|
||||
|
||||
it('treats a colon-free token as a bare pathname', () => {
|
||||
expect(routeTargetFromToken(sessionRoute(SESS_A))).toBe(SESS_A)
|
||||
})
|
||||
})
|
||||
|
||||
describe('sessionContextDrift', () => {
|
||||
it('does not drift on search/hash-only route churn', () => {
|
||||
const reason = sessionContextDrift({
|
||||
startRouteToken: routeToken(sessionRoute(SESS_A)),
|
||||
nowRouteToken: routeToken(sessionRoute(SESS_A), '?panel=preview', '#reply'),
|
||||
startSelectedStoredId: SESS_A,
|
||||
nowSelectedStoredId: SESS_A,
|
||||
submitTargetStoredId: SESS_A
|
||||
})
|
||||
|
||||
expect(reason).toBeNull()
|
||||
})
|
||||
|
||||
it('does not drift on a selection null-reset (gateway/profile switch, reconnect)', () => {
|
||||
const reason = sessionContextDrift({
|
||||
startRouteToken: routeToken(sessionRoute(SESS_A)),
|
||||
nowRouteToken: routeToken(sessionRoute(SESS_A)),
|
||||
startSelectedStoredId: SESS_A,
|
||||
nowSelectedStoredId: null,
|
||||
submitTargetStoredId: SESS_A
|
||||
})
|
||||
|
||||
expect(reason).toBeNull()
|
||||
})
|
||||
|
||||
it('drifts when selection moves to a different non-null stored session', () => {
|
||||
const reason = sessionContextDrift({
|
||||
startRouteToken: routeToken(sessionRoute(SESS_A)),
|
||||
nowRouteToken: routeToken(sessionRoute(SESS_A)),
|
||||
startSelectedStoredId: SESS_A,
|
||||
nowSelectedStoredId: SESS_B,
|
||||
submitTargetStoredId: SESS_A
|
||||
})
|
||||
|
||||
expect(reason).toBe('selection:sess-a->sess-b')
|
||||
})
|
||||
|
||||
it('drifts when the routed session id changes to another session', () => {
|
||||
const reason = sessionContextDrift({
|
||||
startRouteToken: routeToken(sessionRoute(SESS_A)),
|
||||
nowRouteToken: routeToken(sessionRoute(SESS_B)),
|
||||
startSelectedStoredId: SESS_A,
|
||||
nowSelectedStoredId: SESS_A,
|
||||
submitTargetStoredId: SESS_A
|
||||
})
|
||||
|
||||
expect(reason).toBe('route:sess-a->sess-b')
|
||||
})
|
||||
|
||||
it('drifts when the route moves to the new-chat route mid-submit', () => {
|
||||
const reason = sessionContextDrift({
|
||||
startRouteToken: routeToken(sessionRoute(SESS_A)),
|
||||
nowRouteToken: routeToken(NEW_CHAT_ROUTE),
|
||||
startSelectedStoredId: SESS_A,
|
||||
nowSelectedStoredId: SESS_A,
|
||||
submitTargetStoredId: SESS_A
|
||||
})
|
||||
|
||||
expect(reason).toBe('route:sess-a->__new__')
|
||||
})
|
||||
|
||||
it('does not drift when the route moves to a non-chat route (null target)', () => {
|
||||
const reason = sessionContextDrift({
|
||||
startRouteToken: routeToken(sessionRoute(SESS_A)),
|
||||
nowRouteToken: routeToken(SETTINGS_ROUTE),
|
||||
startSelectedStoredId: SESS_A,
|
||||
nowSelectedStoredId: SESS_A,
|
||||
submitTargetStoredId: SESS_A
|
||||
})
|
||||
|
||||
expect(reason).toBeNull()
|
||||
})
|
||||
|
||||
it('does not drift when route and selection re-home onto the submit target (the create pipeline re-home)', () => {
|
||||
const reason = sessionContextDrift({
|
||||
startRouteToken: routeToken(NEW_CHAT_ROUTE),
|
||||
nowRouteToken: routeToken(sessionRoute(SESS_A)),
|
||||
startSelectedStoredId: null,
|
||||
nowSelectedStoredId: SESS_A,
|
||||
submitTargetStoredId: SESS_A
|
||||
})
|
||||
|
||||
expect(reason).toBeNull()
|
||||
})
|
||||
|
||||
it('drifts when a new-chat draft with no target yet is switched to an existing chat', () => {
|
||||
const reason = sessionContextDrift({
|
||||
startRouteToken: routeToken(NEW_CHAT_ROUTE),
|
||||
nowRouteToken: routeToken(sessionRoute(SESS_B)),
|
||||
startSelectedStoredId: null,
|
||||
nowSelectedStoredId: SESS_B,
|
||||
submitTargetStoredId: null
|
||||
})
|
||||
|
||||
expect(reason).toBe('route:__new__->sess-b')
|
||||
})
|
||||
})
|
||||
84
apps/desktop/src/app/session/hooks/session-context-drift.ts
Normal file
84
apps/desktop/src/app/session/hooks/session-context-drift.ts
Normal file
|
|
@ -0,0 +1,84 @@
|
|||
import { isNewChatRoute, routeSessionId } from '../../routes'
|
||||
|
||||
/**
|
||||
* The chat a route token points at: the stored/routed session id, `'__new__'`
|
||||
* for the new-chat route, or null for a route that isn't a chat (settings and
|
||||
* the other overlay routes). Used to compare two route tokens by their *chat*
|
||||
* rather than their raw string.
|
||||
*/
|
||||
export type RouteTarget = string | null
|
||||
|
||||
/**
|
||||
* Reduce a route token to the chat it targets. The token is
|
||||
* `${pathname}:${search}:${hash}` (desktop-controller's routeToken), and only
|
||||
* the pathname selects the chat — search/hash carry overlay/panel state, so a
|
||||
* change there must not read as a session switch. We take the substring before
|
||||
* the first ':' as the pathname; that is safe because `location.pathname` never
|
||||
* contains a raw ':' (sessionRoute encodeURIComponent's the id, so a ':' in an
|
||||
* id arrives as %3A, and the app's other routes are literal colon-free paths).
|
||||
*/
|
||||
export function routeTargetFromToken(token: string): RouteTarget {
|
||||
const separator = token.indexOf(':')
|
||||
const pathname = separator === -1 ? token : token.slice(0, separator)
|
||||
|
||||
return routeSessionId(pathname) ?? (isNewChatRoute(pathname) ? '__new__' : null)
|
||||
}
|
||||
|
||||
interface SessionContextDriftArgs {
|
||||
startRouteToken: string
|
||||
nowRouteToken: string
|
||||
startSelectedStoredId: string | null
|
||||
nowSelectedStoredId: string | null
|
||||
/**
|
||||
* The stored session this submit is bound to, when known. Drift ignores a
|
||||
* move *to* this id: the submit pipeline itself re-homes selection and route
|
||||
* onto its target (a fresh create, a resume), and that self-inflicted move is
|
||||
* not a user switch. Omit it (pre-create new-chat draft) to treat any move to
|
||||
* a real chat as drift.
|
||||
*/
|
||||
submitTargetStoredId?: string | null
|
||||
}
|
||||
|
||||
/**
|
||||
* Decide whether the session context genuinely changed under an in-flight
|
||||
* submit — the user (or a real navigation) moved to a DIFFERENT chat — as
|
||||
* opposed to the programmatic churn a busy gateway produces constantly:
|
||||
* - selection null-resets on a gateway/profile switch or reconnect
|
||||
* (gateway-switch's `setSelectedStoredSessionId(null)`),
|
||||
* - search/hash-only route changes from overlays and side panels,
|
||||
* - background gateway events retargeting the active runtime id (#47709 class,
|
||||
* which is why the active ref is not a prong here at all).
|
||||
* Returns null when nothing genuinely drifted, or a short reason string
|
||||
* (`route:<from>-><to>` / `selection:<from>-><to>`) for the abort log.
|
||||
*/
|
||||
export function sessionContextDrift({
|
||||
startRouteToken,
|
||||
nowRouteToken,
|
||||
startSelectedStoredId,
|
||||
nowSelectedStoredId,
|
||||
submitTargetStoredId
|
||||
}: SessionContextDriftArgs): string | null {
|
||||
const targetStart = routeTargetFromToken(startRouteToken)
|
||||
const targetNow = routeTargetFromToken(nowRouteToken)
|
||||
|
||||
// Route prong: the routed chat moved to a different, real chat. A null target
|
||||
// (navigated to settings / a non-chat overlay route) or a search/hash-only
|
||||
// change (same target) is not drift, and neither is landing on the submit's
|
||||
// own target.
|
||||
if (targetNow !== targetStart && targetNow !== null && targetNow !== submitTargetStoredId) {
|
||||
return `route:${targetStart}->${targetNow}`
|
||||
}
|
||||
|
||||
// Selection prong: selection moved to a different, real stored session. A
|
||||
// null-reset (nowSelectedStoredId === null) or a move onto the submit's own
|
||||
// target is not drift.
|
||||
if (
|
||||
nowSelectedStoredId !== null &&
|
||||
nowSelectedStoredId !== startSelectedStoredId &&
|
||||
nowSelectedStoredId !== submitTargetStoredId
|
||||
) {
|
||||
return `selection:${startSelectedStoredId}->${nowSelectedStoredId}`
|
||||
}
|
||||
|
||||
return null
|
||||
}
|
||||
|
|
@ -17,6 +17,7 @@ import { requestDesktopOnboarding } from '@/store/onboarding'
|
|||
import { setAwaitingResponse, setBusy, setMessages } from '@/store/session'
|
||||
|
||||
import type { ClientSessionState } from '../../../types'
|
||||
import { sessionContextDrift } from '../session-context-drift'
|
||||
|
||||
import {
|
||||
_submitInFlight,
|
||||
|
|
@ -175,11 +176,28 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) {
|
|||
|
||||
let startingRouteToken = getRouteToken()
|
||||
|
||||
const sessionContextDrifted = (): boolean =>
|
||||
targetStartedInCurrentView &&
|
||||
(selectedStoredSessionIdRef.current !== startingStoredSessionId || getRouteToken() !== startingRouteToken)
|
||||
// Reason string (or null) for why the session context genuinely drifted
|
||||
// under this in-flight submit. sessionContextDrift ignores the churn a
|
||||
// busy gateway produces (selection null-resets on a gateway/profile
|
||||
// switch, search/hash-only route changes, background active-ref
|
||||
// retargets) so a second-session send doesn't silently abort — it fires
|
||||
// only on a real move to a DIFFERENT chat. Reads the live refs/route each
|
||||
// call and measures against the (mutable) baseline, which is re-pinned to
|
||||
// the created chat after createBackendSessionForSend. submitTargetStoredId
|
||||
// is the stored session this submit targets, so a move ONTO it (the
|
||||
// pipeline's own re-home) is never counted as drift.
|
||||
const sessionDriftReason = (): string | null =>
|
||||
targetStartedInCurrentView
|
||||
? sessionContextDrift({
|
||||
startRouteToken: startingRouteToken,
|
||||
nowRouteToken: getRouteToken(),
|
||||
startSelectedStoredId: startingStoredSessionId,
|
||||
nowSelectedStoredId: selectedStoredSessionIdRef.current,
|
||||
submitTargetStoredId: startingStoredSessionId
|
||||
})
|
||||
: null
|
||||
|
||||
const targetIsCurrentView = (): boolean => targetStartedInCurrentView && !sessionContextDrifted()
|
||||
const targetIsCurrentView = (): boolean => targetStartedInCurrentView && !sessionDriftReason()
|
||||
|
||||
// One submit in flight per session — drop any concurrent re-fire so a
|
||||
// stalled turn can't stack the same prompt into multiple real turns. The
|
||||
|
|
@ -317,7 +335,11 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) {
|
|||
return abortForSessionSwitch(null)
|
||||
}
|
||||
|
||||
if (sessionContextDrifted()) {
|
||||
const routedResumeDrift = sessionDriftReason()
|
||||
|
||||
if (routedResumeDrift) {
|
||||
console.warn('[submit-drift-abort]', routedResumeDrift, { phase: 'post-routed-resume' })
|
||||
|
||||
return abortForSessionSwitch(null)
|
||||
}
|
||||
|
||||
|
|
@ -351,7 +373,11 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) {
|
|||
source: 'desktop'
|
||||
})
|
||||
|
||||
if (sessionContextDrifted()) {
|
||||
const resumeDrift = sessionDriftReason()
|
||||
|
||||
if (resumeDrift) {
|
||||
console.warn('[submit-drift-abort]', resumeDrift, { phase: 'post-resume' })
|
||||
|
||||
return abortForSessionSwitch(sessionId)
|
||||
}
|
||||
|
||||
|
|
@ -371,7 +397,11 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) {
|
|||
return abortForSessionSwitch(null)
|
||||
}
|
||||
|
||||
if (sessionContextDrifted()) {
|
||||
const resumeSettleDrift = sessionDriftReason()
|
||||
|
||||
if (resumeSettleDrift) {
|
||||
console.warn('[submit-drift-abort]', resumeSettleDrift, { phase: 'post-resume-settle' })
|
||||
|
||||
return abortForSessionSwitch(sessionId)
|
||||
}
|
||||
|
||||
|
|
@ -400,7 +430,11 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) {
|
|||
// createBackendSessionForSend returns null when the user switched
|
||||
// sessions mid-create (it closes the orphaned session itself) —
|
||||
// abort silently. Anything else is a real failure worth a toast.
|
||||
if (sessionContextDrifted()) {
|
||||
const createNullDrift = sessionDriftReason()
|
||||
|
||||
if (createNullDrift) {
|
||||
console.warn('[submit-drift-abort]', createNullDrift, { phase: 'post-create-null' })
|
||||
|
||||
return abortForSessionSwitch(null)
|
||||
}
|
||||
|
||||
|
|
@ -439,7 +473,11 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) {
|
|||
updateComposerAttachments: usingComposerAttachments
|
||||
})
|
||||
|
||||
if (sessionContextDrifted()) {
|
||||
const attachmentsDrift = sessionDriftReason()
|
||||
|
||||
if (attachmentsDrift) {
|
||||
console.warn('[submit-drift-abort]', attachmentsDrift, { phase: 'post-attachments' })
|
||||
|
||||
return abortForSessionSwitch(sessionId)
|
||||
}
|
||||
|
||||
|
|
@ -473,7 +511,11 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) {
|
|||
source: 'desktop'
|
||||
})
|
||||
|
||||
if (sessionContextDrifted()) {
|
||||
const resumeRetryDrift = sessionDriftReason()
|
||||
|
||||
if (resumeRetryDrift) {
|
||||
console.warn('[submit-drift-abort]', resumeRetryDrift, { phase: 'post-resume-retry' })
|
||||
|
||||
return abortForSessionSwitch(sessionId)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -64,6 +64,7 @@ import type { SessionCreateResponse, SessionResumeResponse, UsageStats } from '@
|
|||
|
||||
import { NEW_CHAT_ROUTE, sessionRoute, SETTINGS_ROUTE } from '../../../routes'
|
||||
import type { ClientSessionState, SidebarNavItem } from '../../../types'
|
||||
import { sessionContextDrift } from '../session-context-drift'
|
||||
|
||||
import {
|
||||
appendLiveSessionProjection,
|
||||
|
|
@ -339,7 +340,6 @@ export function useSessionActions({
|
|||
|
||||
const createBackendSessionForSend = useCallback(
|
||||
async (preview: string | null = null): Promise<string | null> => {
|
||||
const startingActiveSessionId = activeSessionIdRef.current
|
||||
const startingStoredSessionId = selectedStoredSessionIdRef.current
|
||||
const startingRouteToken = getRouteToken()
|
||||
|
||||
|
|
@ -362,11 +362,24 @@ export function useSessionActions({
|
|||
const created = await requestGateway<SessionCreateResponse>('session.create', params)
|
||||
const stored = created.stored_session_id ?? null
|
||||
|
||||
if (
|
||||
activeSessionIdRef.current !== startingActiveSessionId ||
|
||||
selectedStoredSessionIdRef.current !== startingStoredSessionId ||
|
||||
getRouteToken() !== startingRouteToken
|
||||
) {
|
||||
// Only a genuine move to a DIFFERENT chat mid-create should orphan the
|
||||
// session we just minted. The active runtime ref is deliberately not a
|
||||
// prong: background gateway events retarget it while other sessions
|
||||
// stream (#47709 class), and the seconds-long session.create round-trip
|
||||
// (server-side agent + MCP init) makes that churn near-certain — every
|
||||
// genuine user switch retargets selection AND route synchronously
|
||||
// anyway. submitTargetStoredId is the just-created stored session, so
|
||||
// our own upcoming re-home onto it never reads as drift.
|
||||
const drift = sessionContextDrift({
|
||||
startRouteToken: startingRouteToken,
|
||||
nowRouteToken: getRouteToken(),
|
||||
startSelectedStoredId: startingStoredSessionId,
|
||||
nowSelectedStoredId: selectedStoredSessionIdRef.current,
|
||||
submitTargetStoredId: stored
|
||||
})
|
||||
|
||||
if (drift) {
|
||||
console.warn('[submit-drift-abort]', drift, { phase: 'mid-create' })
|
||||
await requestGateway('session.close', { session_id: created.session_id }).catch(() => undefined)
|
||||
|
||||
return null
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue