mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
refactor(ai-sessions): bound mirror frames to the running turn
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
e318a99255
commit
c7d1975828
@@ -1896,6 +1896,28 @@ export class AIChatManager {
|
||||
void this.sendRequest({ instructions: text })
|
||||
}
|
||||
|
||||
/** Hand a refused send's text back to the user. The composer takes the text
|
||||
* and clears itself before the send is attempted, so a refusal that keeps no
|
||||
* copy loses what they typed. An auto-sent queued message is excluded: the
|
||||
* caller reports the failure instead, which puts that one back on the queue.
|
||||
*/
|
||||
private restoreRefusedSend(
|
||||
options: NonNullable<Parameters<typeof this.sendRequestImpl>[0]>
|
||||
): void {
|
||||
if (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.
|
||||
if (restored !== true && options.instructions) {
|
||||
this.restoreToInput(options.instructions, options.images, options.files)
|
||||
}
|
||||
}
|
||||
|
||||
/** Send the queued message, if there is one, as its own turn. Also the path a
|
||||
* tab takes when the turn it was watching ends in another tab: the queue was
|
||||
* filled here while that run held the session, and it is owed the same
|
||||
@@ -2845,6 +2867,15 @@ export class AIChatManager {
|
||||
sendUserToast('This action needs the AI chat. Start an AI session to continue.', true)
|
||||
return
|
||||
}
|
||||
// Still holding a mirrored transcript against the pre-turn history: the
|
||||
// other tab's run has ended but this one has not finished reading what it
|
||||
// left behind. Sending now would put that stale history to the model and
|
||||
// persist it over the driver's completed turn.
|
||||
if (this.mirroringRemoteRun) {
|
||||
this.restoreRefusedSend(options)
|
||||
sendUserToast('Catching up on the turn that just finished. Try again in a moment.', true)
|
||||
return false
|
||||
}
|
||||
this.#sendsInFlight++
|
||||
try {
|
||||
// A guarded turn re-enters this method for the sends it spawns itself
|
||||
@@ -2864,23 +2895,7 @@ export class AIChatManager {
|
||||
}
|
||||
})
|
||||
if (outcome !== 'busy') return outcome
|
||||
// Refused: the body never ran. The composer already took the text on
|
||||
// submit, so hand it back rather than let the refusal eat it — an
|
||||
// auto-sent queued message is excluded, since reporting the failure
|
||||
// below is what puts that one back on the queue.
|
||||
if (!options.queued) {
|
||||
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.
|
||||
if (restored !== true && options.instructions) {
|
||||
this.restoreToInput(options.instructions, options.images, options.files)
|
||||
}
|
||||
}
|
||||
this.restoreRefusedSend(options)
|
||||
return false
|
||||
} finally {
|
||||
this.#sendsInFlight--
|
||||
|
||||
@@ -304,6 +304,24 @@ describe('AIChatManager cross-tab run guard', () => {
|
||||
expect(saveChat).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// The other tab's run has ended but this one is still reading what it left
|
||||
// behind, so its transcript is mirrored while `messages` is the pre-turn
|
||||
// history. A send in that window would put the stale history to the model and
|
||||
// persist it over the completed turn.
|
||||
it('refuses to send while still catching up on a finished remote turn', async () => {
|
||||
const manager = new AIChatManager()
|
||||
manager.isSessionChat = true
|
||||
manager.mirroringRemoteRun = true
|
||||
const restoreInstructions = vi.fn(() => true)
|
||||
manager.setAiChatInput({ restoreInstructions } as any)
|
||||
|
||||
const accepted = await manager.sendRequest({ instructions: 'too soon' })
|
||||
|
||||
expect(mocks.runChatLoop).not.toHaveBeenCalled()
|
||||
expect(accepted).toBe(false)
|
||||
expect(restoreInstructions).toHaveBeenCalledWith('too soon', [], [], [])
|
||||
})
|
||||
|
||||
// The composer hands its text over and clears itself before the send is even
|
||||
// attempted, so a refusal that keeps no copy loses what the user typed.
|
||||
it('hands a refused message back to the composer', async () => {
|
||||
|
||||
@@ -601,18 +601,23 @@ export default class HistoryManager {
|
||||
|
||||
/** Re-read one chat from the store into the in-memory mirror, for a record
|
||||
* another tab wrote after this manager last read it. `init()` is the wrong
|
||||
* tool: it re-reads the user's entire history to pick up a single chat. */
|
||||
async reloadChat(id: string): Promise<boolean> {
|
||||
* tool: it re-reads the user's entire history to pick up a single chat.
|
||||
*
|
||||
* 'missing' is a fact about the conversation (it holds nothing yet);
|
||||
* 'unavailable' is a fact about this browser. Callers act on the first and
|
||||
* must not act on the second — treating a closed database as an empty chat
|
||||
* would throw away a transcript that is merely unreadable right now. */
|
||||
async reloadChat(id: string): Promise<'loaded' | 'missing' | 'unavailable'> {
|
||||
const db = await this.dbh.whenReady()
|
||||
if (!db) return false
|
||||
if (!db) return 'unavailable'
|
||||
try {
|
||||
const chat = await db.get('chats', id)
|
||||
if (!chat) return false
|
||||
if (!chat) return 'missing'
|
||||
this.savedChats = { ...this.savedChats, [id]: chat }
|
||||
return true
|
||||
return 'loaded'
|
||||
} catch (err) {
|
||||
console.error('Could not reload chat', err)
|
||||
return false
|
||||
return 'unavailable'
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { withRestoredPayloads, withoutHeavyPayloads } from './sessionMirrorPayload'
|
||||
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
|
||||
@@ -41,29 +41,3 @@ describe('withoutHeavyPayloads', () => {
|
||||
expect(withoutHeavyPayloads(messages)[0]).toBe(messages[0])
|
||||
})
|
||||
})
|
||||
|
||||
// A frame carries the last several messages, but only the newest are new to the
|
||||
// watcher; the rest it already holds complete from IndexedDB.
|
||||
describe('withRestoredPayloads', () => {
|
||||
it('keeps attachments the watcher already had', () => {
|
||||
const local = {
|
||||
role: 'user',
|
||||
content: 'look at this',
|
||||
images: [{ dataUrl: 'data:image/png;base64,AAAA', mediaType: 'image/png' }],
|
||||
files: [{ name: 'notes.md', content: 'the real content', id: 'f1' }],
|
||||
pastes: [{ id: 1, lines: 400, content: 'the real paste' }]
|
||||
} as unknown as DisplayMessage
|
||||
|
||||
const [merged] = withRestoredPayloads(withoutHeavyPayloads([local]), () => local)
|
||||
|
||||
expect(merged).toEqual(local)
|
||||
})
|
||||
|
||||
it('leaves a message the watcher does not have yet stripped', () => {
|
||||
const incoming = [
|
||||
{ role: 'user', content: 'brand new', files: [{ name: 'a.md', content: '', id: 'f9' }] }
|
||||
] as unknown as DisplayMessage[]
|
||||
|
||||
expect(withRestoredPayloads(incoming, () => undefined)).toEqual(incoming)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -49,46 +49,3 @@ export function withoutHeavyPayloads(messages: DisplayMessage[]): DisplayMessage
|
||||
return stripped as DisplayMessage
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Put back the payloads a watcher already holds.
|
||||
*
|
||||
* A frame carries the last several messages, but only the newest one or two are
|
||||
* ever new to the watcher — the rest it loaded from IndexedDB with attachments
|
||||
* intact. Overwriting those with the stripped copies makes a screenshot from an
|
||||
* earlier turn vanish the moment another tab starts a turn, and stay gone until
|
||||
* the turn ends. Positional, which is what the frame's own indexing already
|
||||
* assumes: a transcript only grows within a turn, and a rewrite (compaction)
|
||||
* forces a full frame instead of a tail.
|
||||
*/
|
||||
export function withRestoredPayloads(
|
||||
incoming: DisplayMessage[],
|
||||
localAt: (offset: number) => DisplayMessage | undefined
|
||||
): DisplayMessage[] {
|
||||
return incoming.map((message, i) => {
|
||||
const local = localAt(i) as Record<string, any> | undefined
|
||||
if (!local || local.role !== (message as any).role) return message
|
||||
const merged: Record<string, any> = { ...message }
|
||||
let changed = false
|
||||
for (const f of DROPPED_LIST_FIELDS) {
|
||||
if (merged[f] === undefined && local[f]?.length) {
|
||||
merged[f] = local[f]
|
||||
changed = true
|
||||
}
|
||||
}
|
||||
for (const [f, item] of BLANKED_ITEM_FIELDS) {
|
||||
if (!merged[f]?.length || !local[f]?.length) continue
|
||||
merged[f] = merged[f].map((v: any, j: number) =>
|
||||
v[item] === '' && local[f][j]?.[item] ? { ...v, [item]: local[f][j][item] } : v
|
||||
)
|
||||
changed = true
|
||||
}
|
||||
for (const f of BLANKED_FIELDS) {
|
||||
if (merged[f] === '' && local[f]) {
|
||||
merged[f] = local[f]
|
||||
changed = true
|
||||
}
|
||||
}
|
||||
return (changed ? merged : message) as DisplayMessage
|
||||
})
|
||||
}
|
||||
|
||||
@@ -93,7 +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 { withRestoredPayloads, withoutHeavyPayloads } from './sessionMirrorPayload'
|
||||
import { withoutHeavyPayloads } from './sessionMirrorPayload'
|
||||
import {
|
||||
broadcastMirror,
|
||||
broadcastTurnEnd,
|
||||
@@ -1017,6 +1017,13 @@ async function initRuntime(runtime: SessionRuntime, session: Session) {
|
||||
// that reactive tracking of the array root would miss.
|
||||
const mirrorTimers = new Map<string, ReturnType<typeof setInterval>>()
|
||||
|
||||
// Where the running turn's own messages start, per session. A frame never
|
||||
// reaches below it, which is what makes overwriting safe: everything at or after
|
||||
// it either is new this turn or is a stripped copy the watcher got from an
|
||||
// earlier frame of this same turn, so a frame can never replace a message the
|
||||
// watcher holds complete from the store. Reset to 0 by a rewrite (compaction),
|
||||
// after which no index is stable and whole transcripts travel instead.
|
||||
const turnStarts = new Map<string, number>()
|
||||
// The driver's transcript length at its last frame, so a shrink is detectable.
|
||||
const lastSentTotals = new Map<string, number>()
|
||||
|
||||
@@ -1031,10 +1038,14 @@ function mirrorSnapshotOf(sessionId: string, full: boolean): MirrorSnapshot | un
|
||||
// lengths still line up. Send the whole thing instead.
|
||||
const rewritten = total < (lastSentTotals.get(sessionId) ?? 0)
|
||||
lastSentTotals.set(sessionId, total)
|
||||
if (rewritten) turnStarts.set(sessionId, 0)
|
||||
// Slice before cloning: `$state.snapshot` walks whatever it is handed, so
|
||||
// snapshotting the whole transcript to send ten messages would traverse the
|
||||
// entire conversation on every tick.
|
||||
const baseIndex = mirrorBaseIndex(total, full || rewritten)
|
||||
const baseIndex = Math.max(
|
||||
turnStarts.get(sessionId) ?? 0,
|
||||
mirrorBaseIndex(total, full || rewritten)
|
||||
)
|
||||
return {
|
||||
sessionId,
|
||||
chatId: m.historyManager.getCurrentChatId(),
|
||||
@@ -1060,6 +1071,9 @@ function postMirror(sessionId: string, { full = false } = {}): void {
|
||||
function startMirroring(sessionId: string): void {
|
||||
if (!RUN_OWNERSHIP_AVAILABLE) return
|
||||
stopMirroring(sessionId)
|
||||
// Captured before the turn pushes its first message, so it is the index of
|
||||
// the oldest message this turn owns.
|
||||
turnStarts.set(sessionId, runtimes.get(sessionId)?.manager.displayMessages.length ?? 0)
|
||||
postMirror(sessionId)
|
||||
mirrorTimers.set(
|
||||
sessionId,
|
||||
@@ -1073,6 +1087,7 @@ function stopMirroring(sessionId: string): void {
|
||||
clearInterval(timer)
|
||||
mirrorTimers.delete(sessionId)
|
||||
lastSentTotals.delete(sessionId)
|
||||
turnStarts.delete(sessionId)
|
||||
// One last frame: the closing tokens of a turn usually land between ticks,
|
||||
// and this is what the passive tabs render until their re-read completes.
|
||||
postMirror(sessionId)
|
||||
@@ -1102,12 +1117,12 @@ function applyMirror(msg: MirrorMsg): void {
|
||||
m.historyManager.setCurrentChatId(msg.chatId)
|
||||
setSessionChatId(msg.sessionId, msg.chatId)
|
||||
}
|
||||
// The frame's tail arrives stripped of attachment bytes; anything this tab
|
||||
// already holds complete stays complete, so an earlier turn's screenshot does
|
||||
// not blink out for the length of someone else's turn.
|
||||
const tail = withRestoredPayloads(msg.tail, (i) => m.displayMessages[msg.baseIndex + i])
|
||||
// Safe to overwrite wholesale: a frame reaches no further back than the
|
||||
// running turn's first message, so everything it replaces is either new or a
|
||||
// stripped copy of its own from an earlier frame — never a message this tab
|
||||
// holds complete from the store.
|
||||
m.displayMessages =
|
||||
msg.baseIndex === 0 ? tail : [...m.displayMessages.slice(0, msg.baseIndex), ...tail]
|
||||
msg.baseIndex === 0 ? msg.tail : [...m.displayMessages.slice(0, msg.baseIndex), ...msg.tail]
|
||||
// The frame carries the rendered transcript but not the API-format history,
|
||||
// so this manager is now holding a mismatched pair. Flag it: the save paths
|
||||
// that run outside a turn would otherwise write that pair over the record the
|
||||
@@ -1136,34 +1151,43 @@ async function applyTurnEnd(sessionId: string, chatId: string, committed: boolea
|
||||
const runtime = runtimes.get(sessionId)
|
||||
if (!runtime) return
|
||||
const m = runtime.manager
|
||||
// Cleared before the re-read, not after: loadPastChat refuses to run while
|
||||
// the manager looks busy, and `loading` here is the mirrored driver's.
|
||||
// `loading` has to go first: loadPastChat refuses to run while the manager
|
||||
// looks busy, and this one is the mirrored driver's, not a turn of our own.
|
||||
m.loading = false
|
||||
// The re-read below restores a matched transcript/history pair.
|
||||
m.mirroringRemoteRun = false
|
||||
m.currentReply = ''
|
||||
m.currentReasoning = ''
|
||||
m.currentReasoningActive = false
|
||||
m.loadingLabel = undefined
|
||||
m.compacting = false
|
||||
const id = chatId || m.historyManager.getCurrentChatId()
|
||||
if (id && (await m.historyManager.reloadChat(id))) {
|
||||
// `refresh`: this is the same conversation caught up from the store, so a
|
||||
// message queued here while the other tab held the session is still meant
|
||||
// for it — a plain load would drop it on the floor instead of sending it.
|
||||
await m.loadPastChat(id, { refresh: true })
|
||||
} else if (id && id !== m.historyManager.getCurrentChatId()) {
|
||||
// The driver rotated to a chat with no record yet: it ran "/clear", or its
|
||||
// turn rolled back to nothing. Either way this tab's transcript and model
|
||||
// history belong to the conversation just left, and keeping them would
|
||||
// send that history under the new id on the next turn.
|
||||
m.adoptEmptyChat(id)
|
||||
setSessionChatId(sessionId, id)
|
||||
try {
|
||||
if (!id) return
|
||||
const found = await m.historyManager.reloadChat(id)
|
||||
if (found === 'loaded') {
|
||||
// `refresh`: the same conversation caught up from the store, so a message
|
||||
// queued here while the other tab held the session is still meant for it.
|
||||
// A plain load would drop it instead of sending it below.
|
||||
await m.loadPastChat(id, { refresh: true })
|
||||
} else if (found === 'missing') {
|
||||
// The driver rotated to a chat that holds nothing: it ran "/clear", or its
|
||||
// turn rolled back to empty. This tab's transcript and model history
|
||||
// belong to the conversation just left, and keeping them would send that
|
||||
// history to the model under the new id on the next turn.
|
||||
m.adoptEmptyChat(id)
|
||||
setSessionChatId(sessionId, id)
|
||||
}
|
||||
// 'unavailable' leaves everything as it is: the store is unreadable right
|
||||
// now, which says nothing about the conversation.
|
||||
} finally {
|
||||
// Cleared only once the catch-up is done. Until then this manager still
|
||||
// pairs a mirrored transcript with the pre-turn history, and the send and
|
||||
// save paths gate on this flag to stay off that pair.
|
||||
m.mirroringRemoteRun = false
|
||||
}
|
||||
// Anything typed here while the other tab held the session was queued rather
|
||||
// than sent. Send it only after a turn that landed, which is the rule a turn
|
||||
// follows locally: firing it into a failed turn, or into the gap left by a
|
||||
// tab that vanished, is how a follow-up ends up answering nothing.
|
||||
// follows locally: firing it into a failed turn, or into the gap left by a tab
|
||||
// that vanished, is how a follow-up ends up answering nothing.
|
||||
if (committed) await m.flushQueuedMessage()
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user