Merge pull request #73875 from NousResearch/bb/review-69049

fix(desktop): keep queued drains out of the foreground session on session switch
This commit is contained in:
brooklyn! 2026-07-29 01:42:01 -05:00 committed by GitHub
commit 9bf1f7376f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 176 additions and 0 deletions

View File

@ -1625,6 +1625,7 @@ describe('usePromptActions submit / queue drain semantics', () => {
let handle: HarnessHandle | null = null
render(
<Harness
getRuntimeIdForStoredSession={storedId => (storedId === 'stored-session-a' ? 'rt-session-a' : null)}
onReady={h => (handle = h)}
onUpdateState={(sessionId, storedSessionId, state) => updates.push({ sessionId, state, storedSessionId })}
refreshSessions={async () => undefined}
@ -1656,6 +1657,140 @@ describe('usePromptActions submit / queue drain semantics', () => {
expect($busy.get()).toBe(false)
})
it('a fromQueue drain carrying a stale runtime id re-homes via session.resume instead of landing in the foreground session', async () => {
// The session-switch window this guards: the composer's queue key has
// already flipped to session B (route-driven) while the foreground runtime
// id prop still reads session A (resume-driven, one settle behind). Without
// the central-binding check, prompt.submit fires with session_id=A and B's
// queued prompt — plus its whole answer turn — lands inside A. With no
// binding recorded for B yet, the stale id must be dropped and the drain
// re-homed through the stored-session resume path.
const updates: { sessionId: string; state: Record<string, unknown>; storedSessionId: null | string | undefined }[] =
[]
const requestGateway = vi.fn(
async (method: string, _params?: Record<string, unknown>) =>
(method === 'session.resume' ? { session_id: 'rt-session-b' } : {}) as never
)
let handle: HarnessHandle | null = null
render(
<Harness
onReady={h => (handle = h)}
onUpdateState={(sessionId, storedSessionId, state) => updates.push({ sessionId, state, storedSessionId })}
refreshSessions={async () => undefined}
requestGateway={requestGateway}
/>
)
const accepted = await handle!.submitText('queued for B mid-switch', {
fromQueue: true,
sessionId: 'rt-session-a',
storedSessionId: 'stored-session-b'
})
expect(accepted).toBe(true)
expect(requestGateway).toHaveBeenCalledWith('session.resume', {
session_id: 'stored-session-b',
source: 'desktop'
})
expect(requestGateway).toHaveBeenCalledWith(
'prompt.submit',
{
queued: true,
session_id: 'rt-session-b',
text: 'queued for B mid-switch'
},
1_800_000
)
// The invariant: the stale foreground runtime never receives the prompt.
expect(
requestGateway.mock.calls.every(
([method, params]) =>
method !== 'prompt.submit' || (params as { session_id?: string }).session_id !== 'rt-session-a'
)
).toBe(true)
expect(
updates.some(update => update.sessionId === 'rt-session-b' && update.storedSessionId === 'stored-session-b')
).toBe(true)
})
it('a fromQueue drain rebinds to the centrally recorded runtime when its explicit id is stale', async () => {
// Same window, but B's runtime binding is already known centrally — the
// drain should adopt the authoritative binding directly (no resume
// round-trip) rather than trusting the leftover foreground id.
const requestGateway = vi.fn(async (_method: string, _params?: Record<string, unknown>) => ({}) as never)
let handle: HarnessHandle | null = null
render(
<Harness
getRuntimeIdForStoredSession={storedId => (storedId === 'stored-session-b' ? 'rt-session-b-live' : null)}
onReady={h => (handle = h)}
refreshSessions={async () => undefined}
requestGateway={requestGateway}
/>
)
const accepted = await handle!.submitText('queued for B, B already re-bound', {
fromQueue: true,
sessionId: 'rt-session-a',
storedSessionId: 'stored-session-b'
})
expect(accepted).toBe(true)
expect(requestGateway).toHaveBeenCalledWith(
'prompt.submit',
{
queued: true,
session_id: 'rt-session-b-live',
text: 'queued for B, B already re-bound'
},
1_800_000
)
expect(requestGateway).not.toHaveBeenCalledWith('session.resume', expect.anything())
expect(
requestGateway.mock.calls.every(
([method, params]) =>
method !== 'prompt.submit' || (params as { session_id?: string }).session_id !== 'rt-session-a'
)
).toBe(true)
})
it('a NON-queue explicit target keeps its runtime id even with no central binding recorded', async () => {
// The scoping invariant for the check above. A slash skill dispatch into a
// fresh ⌘T tab passes the same shape a stale drain does — sessionId and
// storedSessionId differ, and the tab has no central binding yet — but its
// two ids were resolved in the same tick, so the explicit target IS
// authoritative. Validating this caller against the (empty) binding would
// null the target and silently drop the kickoff into nowhere.
const requestGateway = vi.fn(async () => ({}) as never)
let handle: HarnessHandle | null = null
render(
<Harness
getRuntimeIdForStoredSession={() => null}
onReady={h => (handle = h)}
refreshSessions={async () => undefined}
requestGateway={requestGateway}
/>
)
const accepted = await handle!.submitText('kickoff for the tab', {
sessionId: 'rt-tab',
storedSessionId: 'stored-tab'
})
expect(accepted).toBe(true)
expect(requestGateway).toHaveBeenCalledWith(
'prompt.submit',
{
session_id: 'rt-tab',
text: 'kickoff for the tab'
},
1_800_000
)
})
it('a fromQueue drain with null runtime id does NOT land in the foreground session (cross-session leak guard)', async () => {
// The cross-session leak: a background drain fires with sessionId=null
// (the stored session's runtime was reaped by the gateway). Without the
@ -2619,6 +2754,13 @@ describe('usePromptActions sleep/wake session recovery', () => {
let handle: HarnessHandle | null = null
render(
<Harness
// The central binding is stale in lockstep with the caller here: the
// sleep/wake reaper only clears the GATEWAY's in-memory session, so
// client-side state still swears by the old runtime id. That is what
// routes this case to the reactive 404→resume→retry path instead of
// the proactive binding check (covered by the cross-session drain
// tests above).
getRuntimeIdForStoredSession={storedId => (storedId === STORED_SESSION_ID ? 'rt-background-stale' : null)}
onReady={h => (handle = h)}
refreshSessions={async () => undefined}
requestGateway={requestGateway}

View File

@ -198,6 +198,40 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) {
let sessionId: null | string = options?.sessionId ?? (isBackgroundQueueDrain ? null : activeSessionIdRef.current)
// A QUEUED runtime id is authoritative ONLY while it still belongs to its
// stored session. On a session switch the composer's queue key flips with
// the route while the foreground runtime id lags a resume behind, so a
// drain can fire with storedSessionId=B but sessionId=A-runtime — and the
// prompt.submit below would land B's queued prompt (and its whole answer
// turn) inside A. Verify the pair against the central binding and drop a
// stale queued id: the targetStoredSessionId resume path below then
// rebinds the right runtime, exactly as a background drain with an
// unknown binding does.
//
// Scoped to fromQueue on purpose. Only a drain pairs identifiers from two
// different clocks; every other explicit-target caller resolves both ids
// in the same tick and is authoritative by construction. A slash skill
// dispatch into a fresh ⌘T tab (slash.ts) passes exactly this shape —
// sessionId=tab-runtime, storedSessionId=tab-stored, no central binding
// recorded yet — so an unscoped check would null the target and silently
// drop the kickoff.
//
// The identity pair (storedSessionId === sessionId) is the fresh-chat
// fallback — an unpersisted conversation's queue key IS its runtime id,
// so it has no central binding to check against and is left untouched.
if (
options?.fromQueue &&
options.sessionId &&
options.storedSessionId &&
options.storedSessionId !== options.sessionId
) {
const boundRuntimeId = getRuntimeIdForStoredSession(options.storedSessionId)
if (boundRuntimeId !== options.sessionId) {
sessionId = boundRuntimeId
}
}
// Pin the foreground session context for the whole async submit pipeline.
// Without this, a fast session switch during session.resume / file.attach
// can redirect the user's text into a different chat (#54527). Mutable —