mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 08:02:38 +00:00
fix(ai-sessions): keep unbounded tool output out of mirror frames
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
2fa8b5e8ec
commit
cd3d24776e
@@ -1093,9 +1093,16 @@ export class AIChatManager {
|
||||
this.#autoResuming = true
|
||||
try {
|
||||
const count = this.pendingJobNotes.length
|
||||
this.instructions =
|
||||
const note =
|
||||
count === 1 ? 'A background job just finished.' : `${count} background jobs just finished.`
|
||||
await this.sendRequest({ synthetic: true })
|
||||
this.instructions = note
|
||||
const accepted = await this.sendRequest({ synthetic: true })
|
||||
// Only sendRequestImpl clears these, and a refusal returns before it —
|
||||
// leaving them set, which this method's own guard above reads as "the
|
||||
// user is mid-compose" and never resumes again. Cleared only while they
|
||||
// are still the note put there: a send that won the lock meanwhile owns
|
||||
// the field, and blanking it would take the user's message mid-turn.
|
||||
if (accepted === false && this.instructions === note) this.instructions = ''
|
||||
} catch (e) {
|
||||
console.error('Auto-resume after background job failed', e)
|
||||
} finally {
|
||||
@@ -1949,25 +1956,19 @@ 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, but the auto-resume set
|
||||
// `this.instructions` before calling it and only sendRequestImpl clears
|
||||
// them. Its own arming check bails while they are non-empty, so leaving
|
||||
// them here disarms every later auto-resume for this session.
|
||||
if (options.synthetic) {
|
||||
this.instructions = ''
|
||||
return
|
||||
}
|
||||
if (options.queued) return
|
||||
// A synthetic send carries none of the user's text; its caller owns the
|
||||
// instructions it set and unwinds them itself.
|
||||
if (options.synthetic || options.queued) return
|
||||
const restored = this.aiChatInput?.restoreInstructions(
|
||||
options.instructions ?? '',
|
||||
options.pastes ?? [],
|
||||
options.images ?? [],
|
||||
options.files ?? []
|
||||
)
|
||||
// No composer mounted (a programmatic send): park it on the queue so it is
|
||||
// still the user's to send rather than silently gone. The queue holds plain
|
||||
// text, so the pastes are expanded into it — parked as bare tokens they
|
||||
// would point at blobs nothing holds any more.
|
||||
// No composer mounted, or one that declined the restore: park it on the
|
||||
// queue so it is still the user's to send rather than silently gone. The
|
||||
// queue holds plain text, so the pastes are expanded into it — parked as
|
||||
// bare tokens they would point at blobs nothing holds any more.
|
||||
if (restored !== true && options.instructions) {
|
||||
this.restoreToInput(
|
||||
expanded(chatDraft(options.instructions, options.pastes ?? [])),
|
||||
|
||||
@@ -422,20 +422,22 @@ describe('AIChatManager.sendOrQueue', () => {
|
||||
|
||||
// The auto-resume writes its prompt into `this.instructions` before sending,
|
||||
// and only sendRequestImpl clears them. A refusal that returns before that
|
||||
// leaves them set, and the arming check bails while they are non-empty — so
|
||||
// one refused resume disarms every later one for the session.
|
||||
it('leaves nothing behind when a synthetic auto-resume is refused', async () => {
|
||||
// `instructions` belongs to whichever turn is running, not to the send being
|
||||
// refused. A synthetic auto-resume racing a user's send must not unwind it
|
||||
// from under them — its own caller clears it, and only while it still holds
|
||||
// the note it put there.
|
||||
it('does not blank instructions when a synthetic send is refused', async () => {
|
||||
const manager = new AIChatManager()
|
||||
manager.isSessionChat = true
|
||||
manager.sessionId = 'session-synthetic-refusal'
|
||||
onDriverLost(() => {})
|
||||
noteDriverAlive('session-synthetic-refusal', false)
|
||||
manager.instructions = 'A background job just finished.'
|
||||
manager.instructions = 'the message the user is sending'
|
||||
|
||||
await manager.sendRequest({ synthetic: true })
|
||||
|
||||
expect(mocks.runChatLoop).not.toHaveBeenCalled()
|
||||
expect(manager.instructions).toBe('')
|
||||
expect(manager.instructions).toBe('the message the user is sending')
|
||||
})
|
||||
|
||||
// These senders have no draft of their own, so a refusal that keeps nothing
|
||||
|
||||
@@ -36,6 +36,33 @@ describe('withoutHeavyPayloads', () => {
|
||||
expect((stripped[0] as any).content).toBe('look at this')
|
||||
})
|
||||
|
||||
// A tool's output is the one payload with no ceiling at all — a query result
|
||||
// or job logs ride the running turn's tail, which is re-cloned and re-sent
|
||||
// several times a second until the turn ends.
|
||||
it('drops a tool result and its logs, keeping the card that frames them', () => {
|
||||
const messages = [
|
||||
{
|
||||
role: 'tool',
|
||||
tool_call_id: 'tc-1',
|
||||
content: 'Ran the query',
|
||||
toolName: 'run_script',
|
||||
parameters: { path: 'f/demo/q' },
|
||||
result: { rows: Array.from({ length: 5000 }, (_, i) => ({ i, blob: 'x'.repeat(200) })) },
|
||||
logs: 'y'.repeat(100_000)
|
||||
}
|
||||
] as unknown as DisplayMessage[]
|
||||
|
||||
const stripped = withoutHeavyPayloads(messages)
|
||||
|
||||
expect(JSON.stringify(stripped).length).toBeLessThan(500)
|
||||
expect((stripped[0] as any).result).toBeUndefined()
|
||||
expect((stripped[0] as any).logs).toBeUndefined()
|
||||
// The card is still rendered from these while the output is in flight.
|
||||
expect((stripped[0] as any).toolName).toBe('run_script')
|
||||
expect((stripped[0] as any).parameters).toEqual({ path: 'f/demo/q' })
|
||||
expect((stripped[0] as any).content).toBe('Ran the query')
|
||||
})
|
||||
|
||||
it('passes through a message with nothing heavy in it', () => {
|
||||
const messages = [{ role: 'assistant', content: 'plain reply' }] as unknown as DisplayMessage[]
|
||||
expect(withoutHeavyPayloads(messages)[0]).toBe(messages[0])
|
||||
|
||||
@@ -24,6 +24,13 @@ const BLANKED_ITEM_FIELDS = [
|
||||
* about to appear. */
|
||||
const DROPPED_LIST_FIELDS = ['images'] as const
|
||||
|
||||
/** Dropped whole, and unbounded in a way the others are not: a tool's result or
|
||||
* logs can be a whole query result or job output, and a frame re-clones and
|
||||
* re-broadcasts the running turn's tail several times a second. A watching tab
|
||||
* shows the tool card without its output until the turn-end re-read supplies
|
||||
* it, which is the same trade the images above make. */
|
||||
const DROPPED_FIELDS = ['result', 'logs'] as const
|
||||
|
||||
/** Emptied outright; guarded at the render site, so it renders nothing. */
|
||||
const BLANKED_FIELDS = ['imageUrl'] as const
|
||||
|
||||
@@ -31,6 +38,7 @@ function isHeavy(message: DisplayMessage): boolean {
|
||||
const m = message as Record<string, any>
|
||||
return (
|
||||
DROPPED_LIST_FIELDS.some((f) => m[f]?.length) ||
|
||||
DROPPED_FIELDS.some((f) => m[f] !== undefined) ||
|
||||
BLANKED_ITEM_FIELDS.some(([f]) => m[f]?.length) ||
|
||||
BLANKED_FIELDS.some((f) => m[f])
|
||||
)
|
||||
@@ -42,6 +50,7 @@ export function withoutHeavyPayloads(messages: DisplayMessage[]): DisplayMessage
|
||||
if (!isHeavy(message)) return message
|
||||
const stripped: Record<string, any> = { ...message }
|
||||
for (const f of DROPPED_LIST_FIELDS) delete stripped[f]
|
||||
for (const f of DROPPED_FIELDS) delete stripped[f]
|
||||
for (const [f, item] of BLANKED_ITEM_FIELDS) {
|
||||
if (stripped[f]?.length) stripped[f] = stripped[f].map((v: any) => ({ ...v, [item]: '' }))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user