From 8879b9e9b2ca6eabf03bb3c1b51221bcd6dde790 Mon Sep 17 00:00:00 2001 From: theone139344 Date: Sat, 25 Jul 2026 20:18:57 +0800 Subject: [PATCH 1/2] fix(desktop): keep queued drains out of the foreground session on session switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A queue drain pairs two identifiers from different clocks: the queue key (flips with the route) and the explicit runtime id (lags a resume behind). Mid-switch the composer can fire a drain with storedSessionId=B but sessionId=A-runtime, and prompt.submit then lands B's queued prompt — and its whole answer turn — inside session A. Make the central runtime binding authoritative for queued sends: when the explicit runtime id no longer matches the binding recorded for the target stored session, adopt the binding (or drop to the stored-id resume path when none exists yet). The identity pair (storedSessionId === sessionId) is the fresh-chat fallback and stays untouched. Tests: re-home-via-resume and rebind-to-central-runtime; existing background-drain and sleep/wake cases declare central bindings explicitly. Null-fallback guard already on main; this PR is the remaining half. --- .../hooks/use-prompt-actions/index.test.tsx | 104 ++++++++++++++++++ .../hooks/use-prompt-actions/submit.ts | 24 ++++ 2 files changed, 128 insertions(+) diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx index fb03f78ffd7f0..2151e5de55646 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx @@ -1625,6 +1625,7 @@ describe('usePromptActions submit / queue drain semantics', () => { let handle: HarnessHandle | null = null render( (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,102 @@ 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; storedSessionId: null | string | undefined }[] = + [] + + const requestGateway = vi.fn( + async (method: string) => (method === 'session.resume' ? { session_id: 'rt-session-b' } : {}) as never + ) + + let handle: HarnessHandle | null = null + render( + (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', + { + 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 () => ({}) as never) + + let handle: HarnessHandle | null = null + render( + (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', + { + 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 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 +2716,13 @@ describe('usePromptActions sleep/wake session recovery', () => { let handle: HarnessHandle | null = null render( (storedId === STORED_SESSION_ID ? 'rt-background-stale' : null)} onReady={h => (handle = h)} refreshSessions={async () => undefined} requestGateway={requestGateway} diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts index 23142f4f0b7b9..4818f26da7db3 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts @@ -198,6 +198,30 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { let sessionId: null | string = options?.sessionId ?? (isBackgroundQueueDrain ? null : activeSessionIdRef.current) + // An explicit 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 explicit id: the + // targetStoredSessionId resume path below then rebinds the right + // runtime, exactly as a background drain with an unknown binding does. + // 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?.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 — From 8830f22c9d33e6acf693c5c4c4147269f250df03 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 29 Jul 2026 00:12:12 -0500 Subject: [PATCH 2/2] fix(desktop): scope the queued-drain binding check to fromQueue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The binding check landed unscoped, so it fired for every caller passing a sessionId/storedSessionId pair — not just queue drains. A slash skill dispatch into a fresh ⌘T tab passes exactly that shape (sessionId=tab runtime, storedSessionId=tab stored) with no central binding recorded yet, so the check nulled the target and the kickoff dropped instead of landing in the tab. 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. Gate on fromQueue and add the scoping invariant as a test. Also refresh the two drain tests for `queued: true`, which prompt.submit started sending for queued drains in ab68c5efe after this work branched. Co-authored-by: theone139344 --- .../hooks/use-prompt-actions/index.test.tsx | 42 ++++++++++++++++++- .../hooks/use-prompt-actions/submit.ts | 32 +++++++++----- 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx index 2151e5de55646..4a807df6de72d 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx @@ -1669,7 +1669,8 @@ describe('usePromptActions submit / queue drain semantics', () => { [] const requestGateway = vi.fn( - async (method: string) => (method === 'session.resume' ? { session_id: 'rt-session-b' } : {}) as never + async (method: string, _params?: Record) => + (method === 'session.resume' ? { session_id: 'rt-session-b' } : {}) as never ) let handle: HarnessHandle | null = null @@ -1696,6 +1697,7 @@ describe('usePromptActions submit / queue drain semantics', () => { expect(requestGateway).toHaveBeenCalledWith( 'prompt.submit', { + queued: true, session_id: 'rt-session-b', text: 'queued for B mid-switch' }, @@ -1717,7 +1719,7 @@ describe('usePromptActions submit / queue drain semantics', () => { // 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 () => ({}) as never) + const requestGateway = vi.fn(async (_method: string, _params?: Record) => ({}) as never) let handle: HarnessHandle | null = null render( @@ -1739,6 +1741,7 @@ describe('usePromptActions submit / queue drain semantics', () => { expect(requestGateway).toHaveBeenCalledWith( 'prompt.submit', { + queued: true, session_id: 'rt-session-b-live', text: 'queued for B, B already re-bound' }, @@ -1753,6 +1756,41 @@ describe('usePromptActions submit / queue drain semantics', () => { ).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( + 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 diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts index 4818f26da7db3..21a2456dbe092 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts @@ -198,21 +198,31 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { let sessionId: null | string = options?.sessionId ?? (isBackgroundQueueDrain ? null : activeSessionIdRef.current) - // An explicit 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 explicit id: the - // targetStoredSessionId resume path below then rebinds the right - // runtime, exactly as a background drain with an unknown binding does. + // 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?.sessionId && - options?.storedSessionId && + options?.fromQueue && + options.sessionId && + options.storedSessionId && options.storedSessionId !== options.sessionId ) { const boundRuntimeId = getRuntimeIdForStoredSession(options.storedSessionId)