mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(ai-sessions): pass prompts to sends instead of staging the shared field
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019N8x51GqABUEFvg52V6aYo
This commit is contained in:
co-authored by
Claude Fable 5
parent
ad8fdb24bf
commit
3358fe6376
@@ -445,6 +445,10 @@ export class AIChatManager {
|
||||
#pipelineHelpersWaiters = new Set<() => void>()
|
||||
readonly isOpen = $derived(chatState.size > 0)
|
||||
savedSize = $state<number>(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<string>('')
|
||||
pendingPrompt = $state<string>('')
|
||||
// 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 = (
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -512,8 +512,7 @@
|
||||
}
|
||||
aiChatManager.changeMode(AIMode.APP)
|
||||
if (!aiChatManager.open) aiChatManager.toggleOpen()
|
||||
aiChatManager.instructions = prompt
|
||||
aiChatManager.sendRequest()
|
||||
aiChatManager.sendRequest({ instructions: prompt })
|
||||
}, 500)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user