From 616a3a4e290ff96138f5ef9749c8eb69ebe41144 Mon Sep 17 00:00:00 2001 From: yingliangzhang Date: Wed, 8 Jul 2026 22:51:38 +0800 Subject: [PATCH] fix(desktop): bind queued sends to source session + guard session.info state patches Race 5 (composer queue cross-session migration): When a prompt is queued in session A and the user switches to session B before the queue drains, the drain used the currently-active session id (B) instead of the source session (A). Now each QueuedPromptEntry stamps sourceRuntimeId and sourceStoredId at enqueue time, and runDrain threads them through onSubmit to submitPromptText so the prompt always lands in the original session. removeQueuedPrompt also uses the entry's source key. Fixes #56390 (complementary to PR #56444 which takes a similar approach). Race 3 (desktop event layer fallback): session.info events with empty session_id fell back to activeSessionIdRef.current for updateSessionState calls, potentially patching the active session's model/cwd/branch with a background session's state during a switch. Now the per-session state-patch path requires explicitSid - unscoped events still render (deltas, completions) but can't mutate per-session state attributes. --- .../chat/composer/hooks/use-composer-queue.ts | 34 ++++++++-- apps/desktop/src/app/chat/composer/index.tsx | 3 +- apps/desktop/src/app/chat/composer/types.ts | 2 +- .../hooks/use-message-stream/gateway-event.ts | 8 ++- .../hooks/use-prompt-actions/submit.ts | 60 ++++++++++++----- .../session/hooks/use-prompt-actions/utils.ts | 11 ++++ apps/desktop/src/lib/gateway-events.test.ts | 14 ++++ apps/desktop/src/store/composer-queue.test.ts | 66 +++++++++++++++++++ apps/desktop/src/store/composer-queue.ts | 22 ++++++- 9 files changed, 192 insertions(+), 28 deletions(-) diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-queue.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-queue.ts index c40d56a4826b..9bb15a97810f 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-queue.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-queue.ts @@ -34,6 +34,8 @@ interface UseComposerQueueArgs { queueEditRef: RefObject queueSessionKey: ChatBarProps['queueSessionKey'] sessionId: string | null | undefined + /** Stored session id for the active session — used to stamp queue entries. */ + storedSessionId: string | null | undefined } /** @@ -57,7 +59,8 @@ export function useComposerQueue({ onSubmit, queueEditRef, queueSessionKey, - sessionId + sessionId, + storedSessionId }: UseComposerQueueArgs) { const { t } = useI18n() @@ -168,7 +171,12 @@ export function useComposerQueue({ return false } - if (!enqueueQueuedPrompt(activeQueueSessionKey, { text, attachments })) { + if (!enqueueQueuedPrompt(activeQueueSessionKey, { + text, + attachments, + sourceRuntimeId: sessionId ?? undefined, + sourceStoredId: storedSessionId ?? undefined + })) { return false } @@ -177,10 +185,13 @@ export function useComposerQueue({ triggerHaptic('selection') return true - }, [activeQueueSessionKey, attachments, clearDraft, draftRef]) + }, [activeQueueSessionKey, attachments, clearDraft, draftRef, sessionId, storedSessionId]) // All queue drain paths share one lock + send-then-remove sequence. // `pickEntry` lets each caller choose head, by-id, or skip-edited. + // Each entry carries its source session ids so the drain always targets + // the session that owned the entry at enqueue time — never the + // currently-active session if the user switched in between (Race 5). const runDrain = useCallback( async (pickEntry: (entries: QueuedPromptEntry[]) => QueuedPromptEntry | undefined): Promise => { if (drainingQueueRef.current || !activeQueueSessionKey) { @@ -193,11 +204,24 @@ export function useComposerQueue({ return false } + // Determine which session key owns this entry. If the entry has a + // sourceRuntimeId that differs from the current activeQueueSessionKey, + // the user switched sessions — use the entry's source key for removal + // and pass the source ids to onSubmit so it targets the right session. + const entrySessionKey = entry.sourceRuntimeId && entry.sourceRuntimeId !== activeQueueSessionKey + ? entry.sourceRuntimeId + : activeQueueSessionKey + drainingQueueRef.current = true try { const accepted = await Promise.resolve( - onSubmit(entry.text, { attachments: entry.attachments, fromQueue: true }) + onSubmit(entry.text, { + attachments: entry.attachments, + fromQueue: true, + targetRuntimeId: entry.sourceRuntimeId, + targetStoredId: entry.sourceStoredId + }) ) if (accepted === false) { @@ -205,7 +229,7 @@ export function useComposerQueue({ } drainFailuresRef.current.delete(entry.id) - removeQueuedPrompt(activeQueueSessionKey, entry.id) + removeQueuedPrompt(entrySessionKey, entry.id) resetBrowseState(sessionId) return true diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index 1f5df46eb2a4..d40b5848c90f 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -183,7 +183,8 @@ export function ChatBar({ onSubmit, queueEditRef, queueSessionKey, - sessionId + sessionId, + storedSessionId: queueSessionKey }) const statusStackVisible = queuedPrompts.length > 0 || statusPresent diff --git a/apps/desktop/src/app/chat/composer/types.ts b/apps/desktop/src/app/chat/composer/types.ts index 59c7c17274c2..754a27d8e69d 100644 --- a/apps/desktop/src/app/chat/composer/types.ts +++ b/apps/desktop/src/app/chat/composer/types.ts @@ -54,7 +54,7 @@ export interface ChatBarProps { onSteer?: (text: string) => Promise | boolean onSubmit: ( value: string, - options?: { attachments?: ComposerAttachment[]; fromQueue?: boolean } + options?: { attachments?: ComposerAttachment[]; fromQueue?: boolean; targetRuntimeId?: string; targetStoredId?: string } ) => Promise | boolean onTranscribeAudio?: (audio: Blob) => Promise } diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts index 664e4b9d53ea..eeb9cb8dd07e 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts @@ -168,7 +168,13 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) { } } - if (sessionId && hasStatePatch) { + // Only apply per-session state patches when we have a real session_id. + // Without explicitSid, sessionId falls back to activeSessionIdRef.current + // — patching state with that fallback could misattribute a background + // session's model/cwd/branch onto the active session during a switch. + // The `apply` guard above already prevents global setters; this closes + // the per-session state-patch path (Race 3). + if (explicitSid && sessionId && hasStatePatch) { updateSessionState(sessionId, state => ({ ...state, ...statePatch, 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 5dd01fc6d6d8..950a70412817 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 @@ -128,7 +128,14 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { // 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. - const submitLockKey = startingStoredSessionId || startingActiveSessionId || '__pending_new__' + // When draining a queued prompt, key the lock on the TARGET session + // (the one that owns the queue entry), not the currently-active one. + const submitLockKey = + options?.targetStoredId || + options?.targetRuntimeId || + startingStoredSessionId || + startingActiveSessionId || + '__pending_new__' if (_submitInFlight.has(submitLockKey)) { return false @@ -226,7 +233,10 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { setAwaitingResponse(true) clearNotifications() - let sessionId: null | string = activeSessionId + // When draining a queued prompt, target the source session instead of the + // currently-active one. This prevents a queued prompt from firing into the + // wrong session after a session switch (Race 5). + let sessionId: null | string = options?.targetRuntimeId || activeSessionId if (sessionId) { seedOptimistic(sessionId) @@ -331,22 +341,36 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { // backend loop (#55578 symptom d) rejects the submit even though // the stored session is fine — resume + retry instead of erroring // out and losing the session binding. - const resumed = await requestGateway<{ session_id: string }>('session.resume', { - session_id: startingStoredSessionId, - source: 'desktop' - }) - - if (sessionContextDrifted()) { - return abortForSessionSwitch(sessionId) - } - - const recoveredId = resumed?.session_id - - if (recoveredId) { - activeSessionIdRef.current = recoveredId - await withSessionBusyRetry(() => - requestGateway('prompt.submit', { session_id: recoveredId, text }, PROMPT_SUBMIT_REQUEST_TIMEOUT_MS) - ) + // Use the queued prompt's source stored id when available so the + // resume targets the original session, not the active one. + const storedIdForResume = options?.targetStoredId || startingStoredSessionId + + if (storedIdForResume) { + const resumed = await requestGateway<{ session_id: string }>('session.resume', { + session_id: storedIdForResume, + source: 'desktop' + }) + + if (sessionContextDrifted()) { + return abortForSessionSwitch(sessionId) + } + + const recoveredId = resumed?.session_id + + if (recoveredId) { + // Update the ref only if we're targeting the active session. + // For background-session drains, don't clobber the active ref. + if (!options?.targetRuntimeId || options.targetRuntimeId === activeSessionIdRef.current) { + activeSessionIdRef.current = recoveredId + } + + sessionId = recoveredId + await withSessionBusyRetry(() => + requestGateway('prompt.submit', { session_id: recoveredId, text }, PROMPT_SUBMIT_REQUEST_TIMEOUT_MS) + ) + } else { + submitErr = firstErr + } } else { submitErr = firstErr } diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/utils.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/utils.ts index de501a52bbc7..f3f9bad916e7 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/utils.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/utils.ts @@ -226,4 +226,15 @@ export function visibleUserIndexAtOrdinal(messages: readonly ChatMessage[], targ export interface SubmitTextOptions { attachments?: ComposerAttachment[] fromQueue?: boolean + /** + * When draining a queued prompt, the source session's runtime id. If the + * user switched sessions between enqueue and drain, this ensures the prompt + * submits to the original session instead of the currently-active one. + */ + targetRuntimeId?: string + /** + * The stored session id paired with targetRuntimeId, used for session.resume + * retry when the runtime id has expired. + */ + targetStoredId?: string } diff --git a/apps/desktop/src/lib/gateway-events.test.ts b/apps/desktop/src/lib/gateway-events.test.ts index d51a943611f0..fb328a71e24f 100644 --- a/apps/desktop/src/lib/gateway-events.test.ts +++ b/apps/desktop/src/lib/gateway-events.test.ts @@ -24,4 +24,18 @@ describe('gateway event routing', () => { expect(gatewayEventRequiresSessionId('session.info')).toBe(false) expect(gatewayEventRequiresSessionId(undefined)).toBe(false) }) + + it('documents the Race 3 fix: session.info state patches require explicitSid', () => { + // gatewayEventRequiresSessionId('session.info') is still false — unscoped + // session.info events are NOT dropped (they carry the active turn's state). + // The Race 3 fix is in gateway-event.ts: the per-session state-patch + // (updateSessionState) is guarded by `explicitSid` so an unscoped + // session.info can't misattribute a background session's model/cwd/branch + // onto the active session during a switch. + // + // This test documents the invariant: the function-level gate stays open + // for session.info, but the state-patch path in the event handler checks + // explicitSid separately. + expect(gatewayEventRequiresSessionId('session.info')).toBe(false) + }) }) diff --git a/apps/desktop/src/store/composer-queue.test.ts b/apps/desktop/src/store/composer-queue.test.ts index 8012e2870f06..ffe8e896eb5e 100644 --- a/apps/desktop/src/store/composer-queue.test.ts +++ b/apps/desktop/src/store/composer-queue.test.ts @@ -168,3 +168,69 @@ describe('shouldAutoDrain', () => { expect(shouldAutoDrain({ isBusy: false, queueLength: 0 })).toBe(false) }) }) + +describe('Race 5: source session binding', () => { + beforeEach(() => { + window.localStorage.removeItem(QUEUE_STORAGE_KEY) + $queuedPromptsBySession.set({}) + }) + + it('stamps sourceRuntimeId and sourceStoredId onto queued entries', () => { + const entry = enqueueQueuedPrompt(SESSION_KEY, { + attachments: [], + text: 'race 5 test', + sourceRuntimeId: 'rt-session-a', + sourceStoredId: 'stored-session-a' + }) + + expect(entry).not.toBeNull() + expect(entry?.sourceRuntimeId).toBe('rt-session-a') + expect(entry?.sourceStoredId).toBe('stored-session-a') + + const queue = getQueuedPrompts(SESSION_KEY) + expect(queue[0]?.sourceRuntimeId).toBe('rt-session-a') + expect(queue[0]?.sourceStoredId).toBe('stored-session-a') + }) + + it('allows entries without source session ids (backward compatible)', () => { + const entry = enqueueQueuedPrompt(SESSION_KEY, { + attachments: [], + text: 'legacy entry' + }) + + expect(entry).not.toBeNull() + expect(entry?.sourceRuntimeId).toBeUndefined() + expect(entry?.sourceStoredId).toBeUndefined() + }) + + it('persists source session ids into local storage', () => { + enqueueQueuedPrompt(SESSION_KEY, { + attachments: [], + text: 'persist source ids', + sourceRuntimeId: 'rt-persist', + sourceStoredId: 'stored-persist' + }) + + const raw = window.localStorage.getItem(QUEUE_STORAGE_KEY) + expect(raw).toBeTruthy() + + const parsed = JSON.parse(String(raw)) as Record + expect(parsed[SESSION_KEY]?.[0]?.sourceRuntimeId).toBe('rt-persist') + expect(parsed[SESSION_KEY]?.[0]?.sourceStoredId).toBe('stored-persist') + }) + + it('survives migration with source ids intact', () => { + enqueueQueuedPrompt('rt-old', { + attachments: [], + text: 'migrate me', + sourceRuntimeId: 'rt-original', + sourceStoredId: 'stored-original' + }) + + migrateQueuedPrompts('rt-old', 'rt-new') + + const queue = getQueuedPrompts('rt-new') + expect(queue[0]?.sourceRuntimeId).toBe('rt-original') + expect(queue[0]?.sourceStoredId).toBe('stored-original') + }) +}) diff --git a/apps/desktop/src/store/composer-queue.ts b/apps/desktop/src/store/composer-queue.ts index 922e990fdce7..62d860bda54e 100644 --- a/apps/desktop/src/store/composer-queue.ts +++ b/apps/desktop/src/store/composer-queue.ts @@ -7,6 +7,17 @@ export interface QueuedPromptEntry { text: string attachments: ComposerAttachment[] queuedAt: number + /** + * The runtime session id that was active when this entry was enqueued. + * Used to bind the drain submit to the source session so a queued prompt + * never fires into a different session after a session switch (Race 5). + */ + sourceRuntimeId?: string + /** + * The stored session id that was active when this entry was enqueued. + * Paired with sourceRuntimeId for session.resume retry paths. + */ + sourceStoredId?: string } type QueueState = Record @@ -80,7 +91,12 @@ export const getQueuedPrompts = (key: string | null | undefined): QueuedPromptEn export const enqueueQueuedPrompt = ( key: string | null | undefined, - payload: { text: string; attachments: ComposerAttachment[] } + payload: { + text: string + attachments: ComposerAttachment[] + sourceRuntimeId?: string + sourceStoredId?: string + } ): null | QueuedPromptEntry => { const sid = sidOf(key) @@ -92,7 +108,9 @@ export const enqueueQueuedPrompt = ( id: nextId(), text: payload.text, attachments: cloneAttachments(payload.attachments), - queuedAt: Date.now() + queuedAt: Date.now(), + sourceRuntimeId: payload.sourceRuntimeId, + sourceStoredId: payload.sourceStoredId } writeSession(sid, [...queueFor(sid), entry])