From 3358fe6376d6513ee8ab430904bbb35d35e69fb9 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Mon, 31 Aug 2026 22:18:51 +0200 Subject: [PATCH] fix(ai-sessions): pass prompts to sends instead of staging the shared field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two sends can race the run guard, and the loser's refusal ran while the winner was mid-turn: restoreRefusedSend resolved the prompt from this.instructions — which the winner may already own — handed that text back, and blanked the field the winner still reads for its bubble and its outgoing message. The unconditional clear turned that race into an empty turn on a transcript a retry had already rewound. Every sender now passes its prompt in the options (retry/edit, AI Fix, Ask AI, background-job auto-resume, the raw-app hand-off page), so the field is written only by sendRequestImpl and a refusal touches nothing it does not own. Auto-resume drops its conditional unwind — with nothing staged there is nothing to leak on a refusal. Also trims the retry test's comment down to its assertions and adds a case pinning that a refusal parks its own prompt and leaves a concurrent winner's field untouched. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019N8x51GqABUEFvg52V6aYo --- .../copilot/chat/AIChatManager.svelte.ts | 36 ++++++++----------- .../copilot/chat/AIChatManager.test.ts | 26 ++++++++++++-- .../apps_raw/edit/[...path]/+page.svelte | 3 +- 3 files changed, 40 insertions(+), 25 deletions(-) diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts index 422cc6de43..07a00c4b79 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts @@ -445,6 +445,10 @@ export class AIChatManager { #pipelineHelpersWaiters = new Set<() => void>() readonly isOpen = $derived(chatState.size > 0) savedSize = $state(0) + // The prompt of the send in flight: copied from `options.instructions` by + // sendRequestImpl, cleared once consumed. Senders pass prompts in the options, + // never by staging this field — sends race the run guard, and a staged copy is + // what a refused loser would hand back or unwind from under the winner's turn. instructions = $state('') pendingPrompt = $state('') // Message queued while a turn is streaming. There is only ever one queued @@ -1127,12 +1131,7 @@ export class AIChatManager { const count = this.pendingJobNotes.length const note = count === 1 ? 'A background job just finished.' : `${count} background jobs just finished.` - this.instructions = note - const accepted = await this.sendRequest({ synthetic: true }) - // A refusal returns before sendRequestImpl's clear, and the guard above - // reads a leftover note as "the user is mid-compose" and never resumes. - // Conditional because a send that won the lock meanwhile owns this field. - if (accepted === false && this.instructions === note) this.instructions = '' + await this.sendRequest({ synthetic: true, instructions: note }) } catch (e) { console.error('Auto-resume after background job failed', e) } finally { @@ -1980,14 +1979,14 @@ export class AIChatManager { // a Retry that is refused charges them against the conversation's budget // for the lifetime of the manager. this.#releaseOutgoingReservation(options.resendReservationKey) - // A synthetic send carries none of the user's text; its caller owns the - // instructions it set and unwinds them itself. + // A synthetic send carries none of the user's text, so there is nothing to + // hand back. if (options.synthetic || options.queued) return - // The same fallback sendRequestImpl applies: the programmatic senders (an - // editor's AI Fix, Ask AI) leave the prompt on `this.instructions` and pass - // no `instructions` option, so reading the option alone would hand back an - // empty prompt and lose what they were about to send. - const instructions = options.instructions ?? this.instructions + // Only what the options carry: `this.instructions` may already hold the + // prompt of a concurrent send that won the run guard, which is also why + // nothing clears it here — blanking the field would empty the turn that + // send is still building from it. + const instructions = options.instructions ?? '' const restored = this.aiChatInput?.restoreInstructions( instructions, options.pastes ?? [], @@ -2005,10 +2004,6 @@ export class AIChatManager { options.files ) } - // Handed off, so this manager no longer holds it. Only sendRequestImpl clears - // the field and a refusal never reaches it; left set, it reads to the - // auto-resume guard and the sidebar's draft cue as text the user is writing. - this.instructions = '' } /** Send the queued message, if there is one, as its own turn. The queue only @@ -2453,8 +2448,8 @@ export class AIChatManager { if (this.scriptEditorOptions) { this.contextManager.setAskAiContext(options) } - this.instructions = prompt this.sendRequest({ + instructions: prompt, removeDiff: options.withDiff, addBackCode: options.withCode === false }) @@ -4076,7 +4071,6 @@ export class AIChatManager { // them); a bare Retry passes nothing and falls back to the original // contextElements. `undefined` for modes that don't attach context leaves the // live-selection behavior. An empty array is a deliberate "no context". - this.instructions = newContent ?? userMessage.content // Everything that rewinds the conversation, deferred to the point the turn is // ours. Doubles as the answer to "did the resend start": it runs exactly when // the send was not refused. @@ -4097,6 +4091,7 @@ export class AIChatManager { } await this.sendRequest({ rewind, + instructions: newContent ?? userMessage.content, pastes: pastes ?? userMessage.pastes, contextOverride: editedContext ?? userMessage.contextElements, contextOverrideOrigin: 'replay', @@ -4115,9 +4110,8 @@ export class AIChatManager { this.toggleOpen() } this.changeMode(AIMode.SCRIPT) - this.instructions = 'Fix the error' this.contextManager?.setFixContext() - this.sendRequest() + this.sendRequest({ instructions: 'Fix the error' }) } addSelectedLinesToContext = ( diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts index 11b36dbb14..6afcf9a4b0 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts @@ -265,12 +265,34 @@ describe('AIChatManager cross-tab run guard', () => { expect(mocks.runChatLoop).not.toHaveBeenCalled() expect(manager.displayMessages).toHaveLength(2) expect(manager.messages).toHaveLength(2) - // Handed back rather than lost — and off the manager, which reports a - // non-empty `instructions` to the sidebar as text the user is writing. expect(manager.queuedMessage).toBe('first') expect(manager.instructions).toBe('') }) + // Two sends can race the guard, and the loser's refusal runs while the winner + // is mid-turn. The refusal must hand back its own prompt (from its options) + // and leave `instructions` — which the winner is still reading — untouched. + it('does not touch a concurrent winner staged prompt when refusing a retry', async () => { + const manager = new AIChatManager() + manager.isSessionChat = true + manager.displayMessages = [ + { role: 'user', content: 'first', index: 0 }, + { role: 'assistant', content: 'answer' } + ] as any + manager.messages = [ + { role: 'user', content: 'first' }, + { role: 'assistant', content: 'answer' } + ] as any + manager.runGuard = async () => 'busy' + manager.instructions = 'the prompt of the send that won the lock' + + const started = await manager.restartGeneration(0) + + expect(started).toBe(false) + expect(manager.queuedMessage).toBe('first') + expect(manager.instructions).toBe('the prompt of the send that won the lock') + }) + // A turn flushes its queued message by re-entering sendRequest, and the lock // behind the guard is not reentrant: applying it to that nested send would // refuse the queued message as though a rival tab held the session. diff --git a/frontend/src/routes/(root)/(logged)/apps_raw/edit/[...path]/+page.svelte b/frontend/src/routes/(root)/(logged)/apps_raw/edit/[...path]/+page.svelte index 78483c327a..f36f876637 100644 --- a/frontend/src/routes/(root)/(logged)/apps_raw/edit/[...path]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/apps_raw/edit/[...path]/+page.svelte @@ -512,8 +512,7 @@ } aiChatManager.changeMode(AIMode.APP) if (!aiChatManager.open) aiChatManager.toggleOpen() - aiChatManager.instructions = prompt - aiChatManager.sendRequest() + aiChatManager.sendRequest({ instructions: prompt }) }, 500) } }