From fac221c1bb765f21117652b72378e1fdbd02528f Mon Sep 17 00:00:00 2001 From: hugocasa Date: Mon, 31 Aug 2026 19:25:45 +0200 Subject: [PATCH] fix(ai-sessions): refuse a held run before mutating, and keep an in-flight draft Co-Authored-By: Claude Opus 5 --- .../copilot/chat/AIChatManager.svelte.ts | 9 +++++++ .../sessions/sessionRunOwner.svelte.ts | 5 ++++ .../sessions/sessionState.svelte.ts | 25 +++++++++++++------ .../components/sessions/sessionState.test.ts | 21 +++++++++++++--- 4 files changed, 50 insertions(+), 10 deletions(-) diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts index 06511f105f..01d782ec8c 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts @@ -3990,6 +3990,15 @@ export class AIChatManager { throw new Error('No user message found at the specified index') } + // Refused here rather than at the send below, which is reached only after the + // truncation has already rewound this tab's transcript: the driver's turn-end + // re-read would put it back, but until then the watcher sits on a conversation + // that lost its tail for a resend that never ran. + if (this.runHeldElsewhere) { + sendUserToast('This session is running in another tab. Retry once it finishes.', true) + return + } + // Resolve the API restart point BEFORE reserving bytes or truncating: a // stale index must fail while nothing has been mutated, or the transcript // would be left truncated with the reservation leaked. A negative index diff --git a/frontend/src/lib/components/sessions/sessionRunOwner.svelte.ts b/frontend/src/lib/components/sessions/sessionRunOwner.svelte.ts index b138fffadb..a5f9c438d4 100644 --- a/frontend/src/lib/components/sessions/sessionRunOwner.svelte.ts +++ b/frontend/src/lib/components/sessions/sessionRunOwner.svelte.ts @@ -167,6 +167,11 @@ export async function withSessionRunLock( // tab holds it, which is exactly the "refuse, don't stack up turns" // behavior we want. if (!lock) return 'busy' as const + // Re-checked here because the grant is a turn of the event loop away + // from the check above, and the driver's turn-end is both what frees + // the lock and what lands the status making this tab a watcher — so + // the one grant we must refuse is the one that arrives this way. + if (runHeldElsewhere(sessionId)) return 'busy' as const entered = true return await drive(sessionId, body) } diff --git a/frontend/src/lib/components/sessions/sessionState.svelte.ts b/frontend/src/lib/components/sessions/sessionState.svelte.ts index 6a92bb1b26..8067a6f4c4 100644 --- a/frontend/src/lib/components/sessions/sessionState.svelte.ts +++ b/frontend/src/lib/components/sessions/sessionState.svelte.ts @@ -517,11 +517,20 @@ async function applyRemoteSessionPut(id: string): Promise { const token = ++remoteReadSeq remoteReads.set(id, token) const db = await sessionsDb.whenReady() - if (!db) return + // Only the newest token is cleared on the way out: an older read that lost the + // race must leave the winner's token standing. + const releaseToken = () => { + if (remoteReads.get(id) === token) remoteReads.delete(id) + } + if (!db) { + releaseToken() + return + } let row: Session | undefined try { row = await db.get('sessions', id) } catch (e) { + releaseToken() console.error('Failed to read a session another tab wrote', e) return } @@ -557,16 +566,18 @@ async function applyRemoteSessionPut(id: string): Promise { * the other tab, a seen-watermark bump included. */ export function adoptRemoteRow(held: Session, row: Session): void { const stamped = new Map((held.previewTabs ?? []).map((t) => [t.id, t])) - // Keys the row no longer carries are removed, not left standing. This file - // clears a field by deleting it — see `applyLifecyclePatch`, and the delete of - // `archived` an unarchive performs — and `putSessionRow` stores a snapshot, so - // a dropped key is how "this is no longer set" arrives. Assigning over the - // held object alone keeps the stale value, and this tab's next write to the - // record puts it back into the store, undoing what the other tab did. + // A draft inside its debounce window is newer than anything the store can + // hold, and the pending flush writes this same object, so taking the row's + // older text here would end up persisted over what the user is still typing. + const pendingDraft = draftPromptFlushHandles.has(held.id) ? held.draftPrompt : undefined + // This file clears a field by deleting it and `putSessionRow` stores a + // snapshot, so a key the row lacks is how "no longer set" arrives. Assigning + // alone keeps the stale value, which this tab's next write puts back. for (const k of Object.keys(held)) { if (!(k in row)) delete (held as Record)[k] } Object.assign(held, row) + if (pendingDraft !== undefined) held.draftPrompt = pendingDraft if (row.previewTabs) { held.previewTabs = row.previewTabs.map((t) => { const live = stamped.get(t.id) diff --git a/frontend/src/lib/components/sessions/sessionState.test.ts b/frontend/src/lib/components/sessions/sessionState.test.ts index 1df5222785..b84901a88f 100644 --- a/frontend/src/lib/components/sessions/sessionState.test.ts +++ b/frontend/src/lib/components/sessions/sessionState.test.ts @@ -529,9 +529,7 @@ describe('adoptRemoteRow', () => { archived: true, archivedByWorkspace: true } as Session - // The other tab unarchived: this file clears a field by deleting it, so the - // row simply lacks the key. Assigning over the held object would keep the - // stale flag, and this tab's next write would put it back in the store. + // The other tab unarchived, so the row simply lacks the key. const row = { id: 's1', name: 's1', createdAt: 1 } as Session adoptRemoteRow(held, row) @@ -565,6 +563,23 @@ describe('adoptRemoteRow', () => { expect(held.previewTabs?.[0].friendlyLabel).toBe('My script') expect(held.previewTabs?.[0].friendlyPath).toBe('f/a/b') }) + + it('keeps a draft still inside its debounce window', () => { + // The pending flush writes this same object, so adopting the row's older + // text would persist it over what the user is still typing. + vi.useFakeTimers() + const held = session({ id: 'draft-race', name: 'draft-race', transient: true }) + sessionState.sessions.push(held) + try { + setSessionDraftPrompt('draft-race', 'half-typed') + adoptRemoteRow(held, { id: 'draft-race', name: 'draft-race', createdAt: 0 } as Session) + expect(held.draftPrompt).toBe('half-typed') + } finally { + vi.clearAllTimers() + vi.useRealTimers() + sessionState.sessions = sessionState.sessions.filter((s) => s.id !== 'draft-race') + } + }) }) describe('findEmptyLandingSession — where an unresolvable session link lands', () => {