From 8c288760d0b50107e608ed42df81f48c7ccedde5 Mon Sep 17 00:00:00 2001 From: emozilla Date: Mon, 13 Jul 2026 01:55:39 -0400 Subject: [PATCH] fix(desktop): stop the submit drift guard from aborting every new chat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #54527 context pin (7acaff5ef) snapshots the selected stored session and route token at submit entry and aborts when either changes mid-flight. But a NEW chat's create pipeline legitimately moves both: on success, createBackendSessionForSend re-homes selection and navigates to the chat it just minted. Judged against the pre-create draft baseline that read as a user switch, so every first send of a new chat aborted before prompt.submit — message dropped, no DB row persisted (row creation is lazy, server-side in prompt.submit), and the window stranded on a route whose REST reads 404 "Session not found" forever. Fix: after a successful create, verify no one re-homed during create's post-commit await via the active-session ref (a non-null return guarantees create set it; every switch path retargets it synchronously), then re-pin the drift baseline to the created chat. A mid-create switch still aborts through create's own null return, or through the active-ref check for the post-commit window. Re-pinning also restores the correct stored-id association for the optimistic-message state updates, which the pinned pre-create null had degraded. Tests: red-first regression for the new-chat send, an abort case for a switch landing in create's post-commit window, and the sleep/wake new-chat stub made faithful to the real create (it sets the active ref before returning — the inert stub is what let this ship green). --- .../hooks/use-prompt-actions/index.test.tsx | 115 +++++++++++++++++- .../hooks/use-prompt-actions/submit.ts | 36 ++++-- 2 files changed, 143 insertions(+), 8 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 5f028876a8e3..66a216d14ae7 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 @@ -1294,7 +1294,18 @@ describe('usePromptActions sleep/wake session recovery', () => { }) it('still creates a new session for a genuine new-chat draft (no stored session selected)', async () => { - const createBackendSessionForSend = vi.fn(async () => RUNTIME_SESSION_ID) + const activeSessionIdRef: MutableRefObject = { current: null } + + // Mirror the real createBackendSessionForSend: a successful create + // re-homes the active runtime ref to the session it minted BEFORE + // returning. An inert stub here is what let the new-chat drift-abort + // regression ship green. + const createBackendSessionForSend = vi.fn(async () => { + activeSessionIdRef.current = RUNTIME_SESSION_ID + + return RUNTIME_SESSION_ID + }) + const calls: string[] = [] const requestGateway = vi.fn(async (method: string) => { @@ -1307,6 +1318,7 @@ describe('usePromptActions sleep/wake session recovery', () => { render( (handle = h)} refreshSessions={async () => undefined} @@ -1440,6 +1452,107 @@ describe('usePromptActions submit session-context isolation (#54527)', () => { session_id: STORED_SESSION_A }) }) + + it('submits the first prompt of a new chat — the create pipeline re-homing selection/route is not user drift', async () => { + // Regression for the #54527 guard breaking every NEW chat: on a fresh draft + // (no stored session, no runtime session) createBackendSessionForSend + // legitimately sets selectedStoredSessionIdRef + navigates to the new + // session's route. Comparing against the pre-create (null) baseline made + // the guard read that self-inflicted move as a user switch and abort, so + // prompt.submit never fired: the message vanished, no DB row was ever + // persisted, and the desktop stranded on a route whose REST reads 404 + // ("Session not found"). + const calls: { method: string; params?: Record }[] = [] + const selectedStoredSessionIdRef: MutableRefObject = { current: null } + const activeSessionIdRef: MutableRefObject = { current: null } + let routeToken = '/' + + const requestGateway = vi.fn(async (method: string, params?: Record) => { + calls.push({ method, params }) + + return {} as never + }) + + // Mirror the real createBackendSessionForSend: on success it re-homes the + // refs AND the route to the session it just created. + const createBackendSessionForSend = vi.fn(async () => { + activeSessionIdRef.current = 'rt-new-chat' + selectedStoredSessionIdRef.current = 'stored-new-chat' + routeToken = '/stored-new-chat' + + return 'rt-new-chat' + }) + + let handle: HarnessHandle | null = null + render( + routeToken} + onReady={h => (handle = h)} + refreshSessions={async () => undefined} + requestGateway={requestGateway} + selectedStoredSessionIdRef={selectedStoredSessionIdRef} + storedSessionId={null} + /> + ) + await waitFor(() => expect(handle).not.toBeNull()) + + expect(await handle!.submitText('first message of a brand-new chat')).toBe(true) + expect(createBackendSessionForSend).toHaveBeenCalledTimes(1) + expect(calls.find(c => c.method === 'prompt.submit')?.params).toMatchObject({ + session_id: 'rt-new-chat' + }) + }) + + it('aborts when the user switches sessions during the tail of a successful create', async () => { + // createBackendSessionForSend awaits once more (armed-YOLO apply) AFTER + // committing the refs and returning a real id, so a switch in that window + // escapes its internal null-return drift check. The active ref is the + // tell: every switch path retargets it synchronously, so it no longer + // equals the id create returned. The submit must abort, not adopt the + // switched-to context as its re-pinned baseline. + const calls: { method: string; params?: Record }[] = [] + const selectedStoredSessionIdRef: MutableRefObject = { current: null } + const activeSessionIdRef: MutableRefObject = { current: null } + let routeToken = '/' + + const requestGateway = vi.fn(async (method: string, params?: Record) => { + calls.push({ method, params }) + + return {} as never + }) + + const createBackendSessionForSend = vi.fn(async () => { + // The user switched to Session B during the post-commit await: the + // switch path re-homed all three context markers before create returned. + activeSessionIdRef.current = RUNTIME_SESSION_B + selectedStoredSessionIdRef.current = STORED_SESSION_B + routeToken = `/${STORED_SESSION_B}` + + return 'rt-new-chat' + }) + + let handle: HarnessHandle | null = null + render( + routeToken} + onReady={h => (handle = h)} + refreshSessions={async () => undefined} + requestGateway={requestGateway} + selectedStoredSessionIdRef={selectedStoredSessionIdRef} + storedSessionId={null} + /> + ) + await waitFor(() => expect(handle).not.toBeNull()) + + expect(await handle!.submitText('message that must not land in session B')).toBe(false) + expect(calls.some(c => c.method === 'prompt.submit')).toBe(false) + }) }) describe('usePromptActions eager attachment upload (drop-time)', () => { 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..b1eaa4966bf2 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 @@ -117,10 +117,12 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { // Pin the 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). + // redirect the user's text into a different chat (#54527). Mutable — + // not const — because a new-chat submit legitimately re-homes to the + // session it creates (see the re-pin after createBackendSessionForSend). const startingActiveSessionId = activeSessionIdRef.current - const startingStoredSessionId = selectedStoredSessionIdRef.current - const startingRouteToken = getRouteToken() + let startingStoredSessionId = selectedStoredSessionIdRef.current + let startingRouteToken = getRouteToken() const sessionContextDrifted = (): boolean => selectedStoredSessionIdRef.current !== startingStoredSessionId || @@ -281,11 +283,14 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { return false } - if (sessionContextDrifted()) { - return abortForSessionSwitch(sessionId) - } - if (!sessionId) { + // 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()) { + return abortForSessionSwitch(null) + } + dropOptimistic(null) releaseBusy() notify({ kind: 'error', title: copy.sessionUnavailable, message: copy.createSessionFailed }) @@ -293,6 +298,23 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { return false } + // A successful create re-homes selection + route to the chat it just + // minted, so the pre-create baseline can't tell our own re-home from + // a user switch (judging it drift aborted EVERY first send of a new + // chat: no prompt.submit, no DB row, a stranded route that 404s + // "Session not found"). The drift signal for this window is the + // active ref instead: every switch path re-nulls or retargets it + // synchronously, so it only still equals the id create returned when + // nobody re-homed since. + if (activeSessionIdRef.current !== sessionId) { + return abortForSessionSwitch(sessionId) + } + + // Re-pin the baseline to the created chat for the rest of the + // pipeline; the closures (seedOptimistic et al) see the new value. + startingStoredSessionId = selectedStoredSessionIdRef.current + startingRouteToken = getRouteToken() + seedOptimistic(sessionId) }