mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-07 16:03:21 +00:00
fix: never restore a tool card that nothing left can resolve
This commit is contained in:
@@ -1024,7 +1024,8 @@ export class AIChatManager {
|
||||
this.updateJob(job.jobId, { status: 'failure', reported: true, job: trimJob(gone) })
|
||||
this.applyToolStatus(job.toolCallId, {
|
||||
content: 'Background job could not be retrieved (it may have been removed)',
|
||||
error: `Job ${job.jobId} was unreachable`
|
||||
error: `Job ${job.jobId} was unreachable`,
|
||||
isLoading: false
|
||||
})
|
||||
anyTerminal = true
|
||||
} else {
|
||||
@@ -1066,8 +1067,14 @@ export class AIChatManager {
|
||||
status === 'canceled' || !job.resultFormat
|
||||
? undefined
|
||||
: formatChatJobCompletion(completed, job.resultFormat)
|
||||
// Fill the tool card that launched it (we run outside a turn here).
|
||||
this.applyToolStatus(job.toolCallId, formatted?.card ?? completedJobToolStatus(completed))
|
||||
// Fill the tool card that launched it (we run outside a turn here). isLoading is
|
||||
// normally already false — processToolCall clears it when the launching tool
|
||||
// returns — but a card restored from a mid-turn checkpoint never saw that return,
|
||||
// so only this patch can stop it spinning.
|
||||
this.applyToolStatus(job.toolCallId, {
|
||||
...(formatted?.card ?? completedJobToolStatus(completed)),
|
||||
isLoading: false
|
||||
})
|
||||
// A user-canceled job needs no model note or auto-resume: the user stopped it
|
||||
// deliberately, so announcing it (as "FAILED", since a canceled job isn't a
|
||||
// success) or burning a turn on it would be noise.
|
||||
@@ -4273,6 +4280,15 @@ export class AIChatManager {
|
||||
if (this.isJobNonTerminal(j.status)) j.detached = true
|
||||
}
|
||||
if (this.backgroundJobs.length > 0) this.backgroundJobs = [...this.backgroundJobs]
|
||||
// Reloading resolves no card on its own. Settle every one the poller above
|
||||
// will not reach, whoever wrote it — a record from a build that stored cards
|
||||
// without their jobs would otherwise restore one that spins forever.
|
||||
const pollable = this.#pollableToolCalls()
|
||||
this.displayMessages = this.settledToolDisplay(
|
||||
this.displayMessages,
|
||||
'Interrupted',
|
||||
(message) => !pollable.has(message.tool_call_id)
|
||||
)
|
||||
this.#ensureJobPoller()
|
||||
// Message-attached files live in the transcript, not in the store's
|
||||
// persistence — rebuild their rows so the loaded chat's references are
|
||||
@@ -4674,11 +4690,7 @@ export class AIChatManager {
|
||||
* in the same record — registering one does not write it. So the jobs come back
|
||||
* with the transcript that depends on them, and both go into the same saveChat. */
|
||||
#interruptedSnapshot = (): { display: DisplayMessage[]; jobs: ChatJob[] } => {
|
||||
const polled = new Set(
|
||||
this.backgroundJobs
|
||||
.filter((j) => j.detached || this.isJobNonTerminal(j.status))
|
||||
.map((j) => j.toolCallId)
|
||||
)
|
||||
const polled = this.#pollableToolCalls()
|
||||
return {
|
||||
display: this.settledToolDisplay(
|
||||
this.displayMessages,
|
||||
@@ -4688,6 +4700,15 @@ export class AIChatManager {
|
||||
jobs: $state.snapshot(this.backgroundJobs) as ChatJob[]
|
||||
}
|
||||
}
|
||||
|
||||
/** Tool calls a restored transcript can still resolve. loadPastChat re-attaches the
|
||||
* poller to every non-terminal job and nothing else runs after a reload, so this is
|
||||
* the whole set — asked identically when storing a card and when restoring one, or
|
||||
* the two drift and a card is kept by one and stranded by the other. */
|
||||
#pollableToolCalls = (): Set<string> =>
|
||||
new Set(
|
||||
this.backgroundJobs.filter((j) => this.isJobNonTerminal(j.status)).map((j) => j.toolCallId)
|
||||
)
|
||||
}
|
||||
|
||||
export const aiChatManager = new AIChatManager()
|
||||
|
||||
@@ -2840,6 +2840,33 @@ describe('AIChatManager queued messages', () => {
|
||||
expect(manager.modifiedItems?.size).toBe(0)
|
||||
})
|
||||
|
||||
// Reloading resolves no card on its own. Only the poller can, and only for the jobs
|
||||
// that came back with the transcript — so a stored card without one must arrive
|
||||
// settled, whichever build wrote it.
|
||||
it('settles a restored loading card that no job came back to resolve', async () => {
|
||||
const manager = createManager(createInputMock())
|
||||
mocks.getJob.mockResolvedValue({ type: 'QueuedJob', id: 'job-1' })
|
||||
vi.spyOn(manager.historyManager, 'loadPastChat').mockReturnValue({
|
||||
id: 'reloaded',
|
||||
title: 'Reloaded',
|
||||
displayMessages: [
|
||||
{ role: 'tool', tool_call_id: 'orphan', content: 'Running...', isLoading: true },
|
||||
{ role: 'tool', tool_call_id: 'polled', content: 'Running...', isLoading: true }
|
||||
],
|
||||
actualMessages: [],
|
||||
lastModified: 0
|
||||
} as unknown as ReturnType<typeof manager.historyManager.loadPastChat>)
|
||||
vi.spyOn(manager.historyManager, 'getBackgroundJobs').mockReturnValue([
|
||||
{ jobId: 'job-1', toolCallId: 'polled', status: 'running' }
|
||||
] as any)
|
||||
|
||||
await manager.loadPastChat('reloaded')
|
||||
|
||||
const card = (id: string) => manager.displayMessages.find((m) => m.tool_call_id === id) as any
|
||||
expect(card('orphan')).toMatchObject({ isLoading: false, error: 'Interrupted' })
|
||||
expect(card('polled').isLoading).toBe(true)
|
||||
})
|
||||
|
||||
it('seeds a session chat mask from its stored modified-items', async () => {
|
||||
const manager = createManager(createInputMock())
|
||||
manager.isSessionChat = true
|
||||
@@ -3919,6 +3946,22 @@ describe('AIChatManager background job completion', () => {
|
||||
resultFormat: { kind: 'datatable' as const, datatableName: 'main' }
|
||||
}
|
||||
|
||||
// Live, processToolCall clears isLoading when the launching tool returns. A card
|
||||
// restored from a mid-turn checkpoint never sees that return, so completing its job
|
||||
// is the only thing left that can stop it spinning.
|
||||
it('stops a restored card spinning when the poller completes its job', async () => {
|
||||
const manager = new AIChatManager()
|
||||
manager.registerJob(datatableJob)
|
||||
manager.displayMessages = [
|
||||
{ role: 'tool', tool_call_id: 'tc-1', content: 'Running...', isLoading: true } as any
|
||||
]
|
||||
mocks.getJob.mockResolvedValue(completed({ result: [{ n: 1 }] }))
|
||||
|
||||
await completeDetachedJob(manager)
|
||||
|
||||
expect((manager.displayMessages[0] as any).isLoading).toBe(false)
|
||||
})
|
||||
|
||||
it('reconstructs the datatable result contract from the persisted resultFormat', async () => {
|
||||
const manager = new AIChatManager()
|
||||
manager.registerJob(datatableJob)
|
||||
@@ -3932,7 +3975,8 @@ describe('AIChatManager background job completion', () => {
|
||||
// the SQL contract (row count + shaped rows) rather than generic job output.
|
||||
expect(applyToolStatus).toHaveBeenCalledWith('tc-1', {
|
||||
content: 'Query returned 2 row(s)',
|
||||
result: JSON.stringify([{ n: 1 }, { n: 2 }], null, 2)
|
||||
result: JSON.stringify([{ n: 1 }, { n: 2 }], null, 2),
|
||||
isLoading: false
|
||||
})
|
||||
expect(manager.pendingJobNotes).toHaveLength(1)
|
||||
expect(manager.pendingJobNotes[0]).toContain('"rowCount": 2')
|
||||
@@ -3986,7 +4030,8 @@ describe('AIChatManager background job completion', () => {
|
||||
expect(manager.backgroundJobs[0]?.status).toBe('canceled')
|
||||
expect(applyToolStatus).toHaveBeenCalledWith('tc-1', {
|
||||
content: 'Background job canceled',
|
||||
logs: expect.anything()
|
||||
logs: expect.anything(),
|
||||
isLoading: false
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
Reference in New Issue
Block a user