mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(ai-sessions): correct payload stripping and turn-end reporting
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
9c4c5e1f30
commit
522b4bc67e
@@ -701,6 +701,14 @@ export class AIChatManager {
|
||||
* session runtime. */
|
||||
mirroringRemoteRun = $state(false)
|
||||
|
||||
/** Whether the turn that just finished is one a follow-up should be sent
|
||||
* after: it committed, or the user deliberately stopped it. False through a
|
||||
* provider error, an empty-response rollback, or a programmatic cancel — the
|
||||
* states this manager deliberately keeps its own queued message through. The
|
||||
* session runtime reports it to the other tabs, whose queues follow the same
|
||||
* rule. */
|
||||
lastTurnAcceptsFollowUp = false
|
||||
|
||||
// Workspace items the CURRENT chat modified via AI tool calls, as
|
||||
// `${UserDraftItemKind}:${storagePath}` keys (see modifiedItemsMask.ts).
|
||||
// undefined = untracked: only the global side-panel chat (never initialised),
|
||||
@@ -781,7 +789,10 @@ export class AIChatManager {
|
||||
// turn-end save.
|
||||
#maskPersistQueue: Promise<void> = Promise.resolve()
|
||||
#persistModifiedItems(): Promise<void> {
|
||||
// Runs outside any turn, so the run guard never sees it.
|
||||
// Runs outside any turn, so the run guard never sees it. Dropped rather
|
||||
// than deferred: the re-read when the run ends reseeds the mask from the
|
||||
// driving tab's record, so a write held back here would be overwritten by
|
||||
// it anyway — and writing now would put a mismatched pair in the record.
|
||||
if (this.mirroringRemoteRun) return this.#maskPersistQueue
|
||||
this.#maskPersistQueue = this.#maskPersistQueue.then(() =>
|
||||
this.historyManager
|
||||
@@ -2925,6 +2936,9 @@ export class AIChatManager {
|
||||
// send exits before install. Kept in a mutable local so every exit path
|
||||
// releases the right key.
|
||||
let reservationKey = options.resendReservationKey
|
||||
// Cleared up front so an exit before the verdict below (a refused mode, a
|
||||
// pre-flight throw) reports this turn rather than the previous one's.
|
||||
this.lastTurnAcceptsFollowUp = false
|
||||
const requestedMode = options.mode ?? this.mode
|
||||
if (!isAIModeVisible(requestedMode)) {
|
||||
this.#releaseOutgoingReservation(reservationKey)
|
||||
@@ -3812,7 +3826,8 @@ export class AIChatManager {
|
||||
// empty-response rollback, or a programmatic cancel (panel teardown,
|
||||
// save-and-clear) leaves it in place as a card so it isn't fired into a
|
||||
// failed or torn-down turn.
|
||||
if (turnCommittedCleanly || this.wasCancelledByUser()) {
|
||||
this.lastTurnAcceptsFollowUp = turnCommittedCleanly || this.wasCancelledByUser()
|
||||
if (this.lastTurnAcceptsFollowUp) {
|
||||
await this.flushQueuedMessage()
|
||||
}
|
||||
// A background job may have finished mid-turn: its note missed this turn's
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { withoutHeavyPayloads } from './sessionMirrorPayload'
|
||||
import type { DisplayMessage } from '$lib/components/copilot/chat/shared'
|
||||
|
||||
// Frames go out several times a second for the whole turn, so anything left in
|
||||
// them is re-cloned and re-broadcast on every tick. The caps allow megabytes per
|
||||
// image and per pasted file, and a structural typecheck cannot catch a field
|
||||
// name that no longer exists — the message union is cast on the way out.
|
||||
describe('withoutHeavyPayloads', () => {
|
||||
it('strips the bytes and keeps what the transcript renders', () => {
|
||||
const messages = [
|
||||
{
|
||||
role: 'user',
|
||||
content: 'look at this',
|
||||
images: [
|
||||
{ dataUrl: 'data:image/png;base64,AAAA', mediaType: 'image/png', name: 'shot.png' }
|
||||
],
|
||||
files: [{ name: 'notes.md', content: 'a very long pasted file', id: 'f1' }]
|
||||
},
|
||||
{ role: 'tool', content: 'took a screenshot', imageUrl: 'data:image/png;base64,BBBB' }
|
||||
] as unknown as DisplayMessage[]
|
||||
|
||||
const stripped = withoutHeavyPayloads(messages)
|
||||
|
||||
expect(JSON.stringify(stripped)).not.toContain('data:image')
|
||||
expect(JSON.stringify(stripped)).not.toContain('a very long pasted file')
|
||||
// The file chip is labelled from the name, so that has to survive.
|
||||
expect((stripped[0] as any).files).toEqual([{ name: 'notes.md', content: '', id: 'f1' }])
|
||||
// Images are dropped whole: the bubble renders one <img> per entry with no
|
||||
// per-image guard, so an emptied url would show a broken image instead.
|
||||
expect((stripped[0] as any).images).toBeUndefined()
|
||||
expect((stripped[0] as any).content).toBe('look at this')
|
||||
})
|
||||
|
||||
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])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,31 @@
|
||||
import type { DisplayMessage } from '$lib/components/copilot/chat/shared'
|
||||
|
||||
/**
|
||||
* Drop the attachment bytes from the transcript a mirror frame carries.
|
||||
*
|
||||
* A frame goes out several times a second for the whole turn, and the message
|
||||
* that holds an image or a pasted file sits in its tail unchanged throughout —
|
||||
* so shipping the payloads means re-cloning and re-broadcasting the same
|
||||
* megabytes on every tick (the caps allow roughly 16MB of base64 image plus 5MB
|
||||
* of file content on a single message). Watchers render the transcript without
|
||||
* them and receive the real thing when the turn ends and they re-read the
|
||||
* record the driving tab saved.
|
||||
*/
|
||||
export function withoutHeavyPayloads(messages: DisplayMessage[]): DisplayMessage[] {
|
||||
return messages.map((message) => {
|
||||
const images = 'images' in message ? message.images : undefined
|
||||
const files = 'files' in message ? message.files : undefined
|
||||
const imageUrl = 'imageUrl' in message ? message.imageUrl : undefined
|
||||
if (!images?.length && !files?.length && !imageUrl) return message
|
||||
const stripped: Record<string, unknown> = { ...message }
|
||||
// Dropped rather than blanked: the bubble renders one <img> per entry with
|
||||
// no per-image guard, so an emptied dataUrl would show a broken image where
|
||||
// the real one is about to appear.
|
||||
if (images?.length) delete stripped.images
|
||||
// Kept, minus the bytes: the chip is labelled from the name.
|
||||
if (files?.length) stripped.files = files.map((f) => ({ ...f, content: '' }))
|
||||
// Guarded at the render site, so an empty string renders nothing.
|
||||
if (imageUrl) stripped.imageUrl = ''
|
||||
return stripped as DisplayMessage
|
||||
})
|
||||
}
|
||||
@@ -93,6 +93,7 @@ import { getNonStreamingMetadataCompletion } from '$lib/components/copilot/lib'
|
||||
import { sendUserToast } from '$lib/toast'
|
||||
import { pendingUserAction, type DisplayMessage } from '$lib/components/copilot/chat/shared'
|
||||
import type { ChatCompletionMessageParam } from 'openai/resources/index.mjs'
|
||||
import { withoutHeavyPayloads } from './sessionMirrorPayload'
|
||||
import {
|
||||
broadcastMirror,
|
||||
broadcastTurnEnd,
|
||||
@@ -937,19 +938,27 @@ async function initRuntime(runtime: SessionRuntime, session: Session) {
|
||||
// turns interleave into one chat id.
|
||||
manager.runGuard = async (body) => {
|
||||
const outcome = await withSessionRunLock(session.id, async () => {
|
||||
// The whole turn protocol rides on ownership, not just the frames: a
|
||||
// turn-end announced without it would reach tabs that were never
|
||||
// mirroring and end a run of their own that is still going.
|
||||
if (!RUN_OWNERSHIP_AVAILABLE) return body()
|
||||
// The first frame doubles as the "a run started here" signal: it is
|
||||
// posted immediately and carries the chat id the watchers need.
|
||||
startMirroring(session.id)
|
||||
let committed = false
|
||||
try {
|
||||
const result = await body()
|
||||
committed = result !== false
|
||||
return result
|
||||
return await body()
|
||||
} finally {
|
||||
stopMirroring(session.id)
|
||||
// The chat id is re-read here, not reused from above: the turn may
|
||||
// have rotated it, and the listeners key their IndexedDB re-read on it.
|
||||
broadcastTurnEnd(session.id, manager.historyManager.getCurrentChatId(), committed)
|
||||
broadcastTurnEnd(
|
||||
session.id,
|
||||
manager.historyManager.getCurrentChatId(),
|
||||
// Not "did the send return truthy": that means the input was
|
||||
// consumed, and stays true through provider errors and rollbacks.
|
||||
// The manager reports the rule it applies to its own queue.
|
||||
manager.lastTurnAcceptsFollowUp
|
||||
)
|
||||
}
|
||||
})
|
||||
if (outcome === 'busy') {
|
||||
@@ -1008,27 +1017,6 @@ async function initRuntime(runtime: SessionRuntime, session: Session) {
|
||||
// that reactive tracking of the array root would miss.
|
||||
const mirrorTimers = new Map<string, ReturnType<typeof setInterval>>()
|
||||
|
||||
// Attachments a frame does not carry. They are bounded only by the upload caps
|
||||
// (megabytes of base64 per image, and again per pasted file), they sit in the
|
||||
// tail unchanged for the whole turn, and a frame goes out several times a
|
||||
// second — so shipping them would re-clone and re-post the same bytes on every
|
||||
// tick. Watchers render a placeholder until the turn ends, where the IndexedDB
|
||||
// re-read hands them the real thing.
|
||||
function withoutHeavyPayloads(messages: DisplayMessage[]): DisplayMessage[] {
|
||||
return messages.map((message) => {
|
||||
const images = 'images' in message ? message.images : undefined
|
||||
const files = 'files' in message ? message.files : undefined
|
||||
const imageUrl = 'imageUrl' in message ? message.imageUrl : undefined
|
||||
if (!images?.length && !files?.length && !imageUrl) return message
|
||||
return {
|
||||
...message,
|
||||
...(images?.length ? { images: images.map((i) => ({ ...i, url: '' })) } : {}),
|
||||
...(files?.length ? { files: files.map((f) => ({ ...f, content: '' })) } : {}),
|
||||
...(imageUrl ? { imageUrl: '' } : {})
|
||||
} as DisplayMessage
|
||||
})
|
||||
}
|
||||
|
||||
// The driver's transcript length at its last frame, so a shrink is detectable.
|
||||
const lastSentTotals = new Map<string, number>()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user