mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-06 00:02:13 +00:00
fix: land a run whose updates endpoint fails, and anchor the run form's drawers
This commit is contained in:
@@ -1021,10 +1021,12 @@ export class AIChatManager {
|
||||
}
|
||||
const update = await reader.poll()
|
||||
if (gen !== this.#jobPollGeneration) return
|
||||
this.applyToolStatus(job.toolCallId, {
|
||||
logs: update.logs || undefined,
|
||||
resultStream: update.resultStream || undefined
|
||||
})
|
||||
if (update) {
|
||||
this.applyToolStatus(job.toolCallId, {
|
||||
logs: update.logs || undefined,
|
||||
resultStream: update.resultStream || undefined
|
||||
})
|
||||
}
|
||||
|
||||
const fetched = await JobService.getJob({
|
||||
workspace: job.workspace,
|
||||
|
||||
@@ -36,6 +36,7 @@ const mocks = vi.hoisted(() => ({
|
||||
runChatLoop: vi.fn(),
|
||||
listResource: vi.fn(),
|
||||
getJob: vi.fn(),
|
||||
getJobUpdates: vi.fn(),
|
||||
whoami: vi.fn(),
|
||||
workspace: 'test_workspace' as string | undefined,
|
||||
// The workspace being browsed, which a session chat's own workspace need not be.
|
||||
@@ -60,7 +61,8 @@ vi.mock('$lib/gen', () => ({
|
||||
whoami: mocks.whoami
|
||||
},
|
||||
JobService: {
|
||||
getJob: mocks.getJob
|
||||
getJob: mocks.getJob,
|
||||
getJobUpdates: mocks.getJobUpdates
|
||||
}
|
||||
}))
|
||||
|
||||
@@ -172,6 +174,10 @@ beforeEach(() => {
|
||||
mocks.getOpenaiClient.mockReturnValue({})
|
||||
mocks.getAnthropicClient.mockReturnValue({})
|
||||
mocks.listResource.mockResolvedValue([])
|
||||
// Re-seeded here rather than in the factory: clearAllMocks keeps implementations, so a
|
||||
// test that makes the updates endpoint fail would otherwise leave it failing for the rest
|
||||
// of the file. Neutral by default — completion is getJob's answer.
|
||||
mocks.getJobUpdates.mockResolvedValue({ completed: false, running: true })
|
||||
mocks.workspace = 'test_workspace'
|
||||
mocks.runChatLoop.mockResolvedValue({
|
||||
addedMessages: [],
|
||||
@@ -3986,6 +3992,20 @@ describe('AIChatManager background job completion', () => {
|
||||
expect((manager.displayMessages[0] as any).isLoading).toBe(false)
|
||||
})
|
||||
|
||||
// Streaming rides on a second endpoint; landing the job must not. A poll that always
|
||||
// fails would otherwise spend the failure budget and drain a job that finished, leaving
|
||||
// the card on "unreachable".
|
||||
it('completes a job whose updates endpoint keeps failing', async () => {
|
||||
const manager = new AIChatManager()
|
||||
manager.registerJob(datatableJob)
|
||||
mocks.getJobUpdates.mockRejectedValue(new Error('updates unavailable'))
|
||||
mocks.getJob.mockResolvedValue(completed({ result: [{ n: 1 }] }))
|
||||
|
||||
await completeDetachedJob(manager)
|
||||
|
||||
expect(manager.backgroundJobs[0]?.status).toBe('success')
|
||||
})
|
||||
|
||||
it('reconstructs the datatable result contract from the persisted resultFormat', async () => {
|
||||
const manager = new AIChatManager()
|
||||
manager.registerJob(datatableJob)
|
||||
|
||||
@@ -1434,19 +1434,50 @@ describe('pollJobCompletion detach', () => {
|
||||
const { JobService } = await import('$lib/gen')
|
||||
const getJob = vi.mocked(JobService.getJob)
|
||||
getJob.mockReset()
|
||||
const completed = { type: 'CompletedJob', success: true, result: 42 }
|
||||
const completed = { type: 'CompletedJob', success: true, result: 42, logs: 'ran' }
|
||||
getJob.mockResolvedValue(completed as any)
|
||||
// The updates endpoint is what says the job landed; the whole job is then
|
||||
// fetched once, with its logs.
|
||||
// Still landing on a tick the updates endpoint calls unfinished: the job can
|
||||
// complete between the two calls, and `getJob` is what says so.
|
||||
const getJobUpdates = vi.mocked(JobService.getJobUpdates)
|
||||
getJobUpdates.mockReset()
|
||||
getJobUpdates.mockResolvedValue({ completed: true } as any)
|
||||
getJobUpdates.mockResolvedValue({ completed: false, running: true } as any)
|
||||
const cbs = makeCallbacks()
|
||||
|
||||
const promise = pollJobCompletion('job1', 'w', 'tool1', cbs as any, { detachAfterMs: 15000 })
|
||||
await vi.advanceTimersByTimeAsync(1000)
|
||||
|
||||
expect(await promise).toBe(completed)
|
||||
const landed = await promise
|
||||
expect(landed).toBe(completed)
|
||||
// Fetched again with its logs rather than settled on the logless tick fetch,
|
||||
// which would reach the model as "No logs available".
|
||||
expect((landed as any).logs).toBe('ran')
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
// Streaming rides on a second endpoint; landing the job must not. A failing updates
|
||||
// endpoint costs live logs, never the run.
|
||||
it('returns the completed job with its logs when the updates endpoint fails', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
const { pollJobCompletion } = await import('./shared')
|
||||
const { JobService } = await import('$lib/gen')
|
||||
const getJob = vi.mocked(JobService.getJob)
|
||||
getJob.mockReset()
|
||||
const completed = { type: 'CompletedJob', success: true, result: 42, logs: 'ran' }
|
||||
getJob.mockResolvedValue(completed as any)
|
||||
const getJobUpdates = vi.mocked(JobService.getJobUpdates)
|
||||
getJobUpdates.mockReset()
|
||||
getJobUpdates.mockRejectedValue(new Error('updates unavailable'))
|
||||
const cbs = makeCallbacks()
|
||||
|
||||
const promise = pollJobCompletion('job1', 'w', 'tool1', cbs as any, { detachAfterMs: 15000 })
|
||||
await vi.advanceTimersByTimeAsync(1000)
|
||||
|
||||
const landed = await promise
|
||||
expect(landed).toBe(completed)
|
||||
expect((landed as any).logs).toBe('ran')
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
|
||||
@@ -1633,7 +1633,11 @@ export type BackgroundJobFormatter = (job: CompletedJob) => {
|
||||
/** Reads a running job's output incrementally through `getJobUpdates`, the only endpoint
|
||||
* carrying `new_result_stream`: `getJob` returns logs but never the partial result. Both the
|
||||
* inline wait and the background poller drive one, so a detached run keeps streaming, and
|
||||
* each keeps its own offsets so one starting over refetches from zero. */
|
||||
* each keeps its own offsets so one starting over refetches from zero.
|
||||
*
|
||||
* Best-effort by construction: a poll that fails answers `undefined` rather than throwing, so
|
||||
* a run always lands on `getJob` alone. Nothing is mutated before the response arrives, so the
|
||||
* next poll resumes from the same offsets. */
|
||||
export function createJobUpdateReader(jobId: string, workspace: string) {
|
||||
let logs = ''
|
||||
let resultStream = ''
|
||||
@@ -1641,14 +1645,19 @@ export function createJobUpdateReader(jobId: string, workspace: string) {
|
||||
let streamOffset = 0
|
||||
let started = false
|
||||
return {
|
||||
async poll(): Promise<{ completed: boolean; logs: string; resultStream: string }> {
|
||||
const update = await JobService.getJobUpdates({
|
||||
workspace,
|
||||
id: jobId,
|
||||
running: started,
|
||||
logOffset,
|
||||
streamOffset
|
||||
})
|
||||
async poll(): Promise<{ completed: boolean; logs: string; resultStream: string } | undefined> {
|
||||
let update: Awaited<ReturnType<typeof JobService.getJobUpdates>>
|
||||
try {
|
||||
update = await JobService.getJobUpdates({
|
||||
workspace,
|
||||
id: jobId,
|
||||
running: started,
|
||||
logOffset,
|
||||
streamOffset
|
||||
})
|
||||
} catch {
|
||||
return undefined
|
||||
}
|
||||
started ||= update.running ?? false
|
||||
// Both kept as a tail: the offsets come from the server, so dropping the head
|
||||
// costs nothing here, and neither is the record of the run — the logs are on the
|
||||
@@ -1694,36 +1703,43 @@ export async function pollJobCompletion(
|
||||
// The tray's snapshot is trimmed of logs (it is persisted), so the card is the
|
||||
// only place a running job's output can land. Cards that hide their logs while
|
||||
// loading are unaffected; the run card follows them line by line.
|
||||
toolCallbacks.setToolStatus(toolId, {
|
||||
logs: formatLogs(update.logs),
|
||||
resultStream: update.resultStream || undefined
|
||||
})
|
||||
|
||||
if (update.completed) {
|
||||
// Fetched whole rather than assembled from the ticks: the reader stops at
|
||||
// whatever the last one saw, and the tail written between then and the job
|
||||
// landing is only on the job itself.
|
||||
const completed = await JobService.getJob({
|
||||
workspace: workspace,
|
||||
id: jobId,
|
||||
noLogs: false,
|
||||
noCode: true
|
||||
if (update) {
|
||||
toolCallbacks.setToolStatus(toolId, {
|
||||
logs: formatLogs(update.logs),
|
||||
resultStream: update.resultStream || undefined
|
||||
})
|
||||
if (completed.type === 'CompletedJob') {
|
||||
job = completed
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
// Keeps the tray's status + Job snapshot fresh during the inline wait. Its logs
|
||||
// are skipped because the reader above already has them; the badge needs the real
|
||||
// Job to tell running from suspended or scheduled, which the updates do not say.
|
||||
// Ask for the logs when the run may be over — the tail written between the last
|
||||
// poll and the end is only on the job itself — or when there is no reader output
|
||||
// to have collected them.
|
||||
const wantLogs = !update || update.completed
|
||||
const fetchedJob = await JobService.getJob({
|
||||
workspace: workspace,
|
||||
id: jobId,
|
||||
noLogs: true,
|
||||
noLogs: !wantLogs,
|
||||
noCode: true
|
||||
})
|
||||
if (fetchedJob.type === 'CompletedJob') {
|
||||
// The updates can still call a landed job unfinished, so a completion seen on
|
||||
// a logless fetch is fetched again rather than settled without them: the model
|
||||
// reads these logs, and their absence is indistinguishable from a silent run.
|
||||
job = wantLogs
|
||||
? fetchedJob
|
||||
: ((await JobService.getJob({
|
||||
workspace: workspace,
|
||||
id: jobId,
|
||||
noLogs: false,
|
||||
noCode: true
|
||||
})) as CompletedJob)
|
||||
break
|
||||
}
|
||||
// With no reader, this is the only place the card's logs can come from.
|
||||
if (!update) {
|
||||
toolCallbacks.setToolStatus(toolId, { logs: formatLogs(fetchedJob.logs) })
|
||||
}
|
||||
// The badge needs the real Job to tell running from suspended or scheduled, which
|
||||
// the updates do not say.
|
||||
toolCallbacks.onJobStatus?.(jobId, {
|
||||
status: deriveChatJobStatus(fetchedJob),
|
||||
job: trimJob(fetchedJob)
|
||||
|
||||
@@ -329,8 +329,16 @@
|
||||
{/if}
|
||||
</div>
|
||||
{:else if slot.kind === 'runform' && mounted}
|
||||
<div class="absolute inset-0 flex flex-col min-h-0 bg-surface {visibility}" aria-hidden={!active}>
|
||||
{#if runtime}
|
||||
<div
|
||||
bind:this={overlayHostEl}
|
||||
class="absolute inset-0 flex flex-col min-h-0 bg-surface {visibility}"
|
||||
aria-hidden={!active}
|
||||
>
|
||||
<!-- Waits for the host element itself: Drawer portals when it mounts, and the portal
|
||||
action reads its target once, so a form mounted in the same pass as this div would
|
||||
resolve no host and open against the viewport. The branches above are async
|
||||
(a dynamic import, a loaded artifact), which is what spares them this. -->
|
||||
{#if runtime && overlayHostEl}
|
||||
<RunFormPreviewSlot manager={runtime.manager} toolCallId={slot.toolCallId} />
|
||||
{/if}
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user