fix(update_job_status): status-gedreven lifecycle-timestamps (#51)
Een job kon CLAIMED -> done/failed/skipped gaan zonder ooit `running` te rapporteren, waardoor started_at NULL bleef terwijl finished_at wel gezet werd. Dat brak de invariant claimed_at <= started_at <= finished_at en elke duur-analyse. Nieuwe pure helper resolveJobTimestamps zet de lifecycle-timestamps set-once op basis van de status: started_at wordt gebackfild bij een terminale overgang, claimed_at defensief gevuld als die ontbreekt. De running-tak is nu set-once i.p.v. bij elke call overschrijven. Co-authored-by: Madhura68 <ID+Madhura68@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
51fc65e715
commit
fba2d67796
2 changed files with 109 additions and 4 deletions
74
__tests__/update-job-status-timestamps.test.ts
Normal file
74
__tests__/update-job-status-timestamps.test.ts
Normal file
|
|
@ -0,0 +1,74 @@
|
||||||
|
// Unit-tests voor resolveJobTimestamps — de status-gedreven timestamp-helper
|
||||||
|
// van update_job_status. Pure functie, geen mocks (zoals update-job-status-gate).
|
||||||
|
|
||||||
|
import { describe, it, expect } from 'vitest'
|
||||||
|
import { resolveJobTimestamps } from '../src/tools/update-job-status.js'
|
||||||
|
|
||||||
|
const NOW = new Date('2026-05-14T12:00:00.000Z')
|
||||||
|
const EARLIER = new Date('2026-05-14T11:00:00.000Z')
|
||||||
|
|
||||||
|
describe('resolveJobTimestamps', () => {
|
||||||
|
describe('running', () => {
|
||||||
|
it('sets started_at when not yet set, no finished_at', () => {
|
||||||
|
const r = resolveJobTimestamps('running', { claimed_at: EARLIER, started_at: null }, NOW)
|
||||||
|
expect(r.started_at).toBe(NOW)
|
||||||
|
expect(r.finished_at).toBeUndefined()
|
||||||
|
expect(r.claimed_at).toBeUndefined()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('is set-once: does not re-stamp started_at when already set', () => {
|
||||||
|
const r = resolveJobTimestamps('running', { claimed_at: EARLIER, started_at: EARLIER }, NOW)
|
||||||
|
expect(r.started_at).toBeUndefined()
|
||||||
|
expect(r.finished_at).toBeUndefined()
|
||||||
|
expect(r.claimed_at).toBeUndefined()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('terminal transitions (done/failed/skipped)', () => {
|
||||||
|
it.each(['done', 'failed', 'skipped'] as const)(
|
||||||
|
'backfills started_at and sets finished_at for %s when started_at is null',
|
||||||
|
(status) => {
|
||||||
|
const r = resolveJobTimestamps(status, { claimed_at: EARLIER, started_at: null }, NOW)
|
||||||
|
expect(r.started_at).toBe(NOW)
|
||||||
|
expect(r.finished_at).toBe(NOW)
|
||||||
|
expect(r.claimed_at).toBeUndefined()
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
it('only sets finished_at when started_at is already set', () => {
|
||||||
|
const r = resolveJobTimestamps('done', { claimed_at: EARLIER, started_at: EARLIER }, NOW)
|
||||||
|
expect(r.started_at).toBeUndefined()
|
||||||
|
expect(r.finished_at).toBe(NOW)
|
||||||
|
expect(r.claimed_at).toBeUndefined()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('claimed_at backfill', () => {
|
||||||
|
it.each(['running', 'done', 'failed', 'skipped'] as const)(
|
||||||
|
'backfills claimed_at for %s when it is null',
|
||||||
|
(status) => {
|
||||||
|
const r = resolveJobTimestamps(status, { claimed_at: null, started_at: null }, NOW)
|
||||||
|
expect(r.claimed_at).toBe(NOW)
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
it('never returns claimed_at when it is already set', () => {
|
||||||
|
const r = resolveJobTimestamps('done', { claimed_at: EARLIER, started_at: EARLIER }, NOW)
|
||||||
|
expect(r.claimed_at).toBeUndefined()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
it('returns only finished_at when all timestamps are already set and status is terminal', () => {
|
||||||
|
const r = resolveJobTimestamps('failed', { claimed_at: EARLIER, started_at: EARLIER }, NOW)
|
||||||
|
expect(r).toEqual({ finished_at: NOW })
|
||||||
|
})
|
||||||
|
|
||||||
|
it('defaults now to a fresh Date when omitted', () => {
|
||||||
|
const before = Date.now()
|
||||||
|
const r = resolveJobTimestamps('running', { claimed_at: EARLIER, started_at: null })
|
||||||
|
const after = Date.now()
|
||||||
|
expect(r.started_at).toBeInstanceOf(Date)
|
||||||
|
expect(r.started_at!.getTime()).toBeGreaterThanOrEqual(before)
|
||||||
|
expect(r.started_at!.getTime()).toBeLessThanOrEqual(after)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
@ -390,6 +390,32 @@ export function resolveNextAction(
|
||||||
return queueCount > 0 ? 'wait_for_job_again' : 'queue_empty'
|
return queueCount > 0 ? 'wait_for_job_again' : 'queue_empty'
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export type JobTimestampUpdate = {
|
||||||
|
claimed_at?: Date
|
||||||
|
started_at?: Date
|
||||||
|
finished_at?: Date
|
||||||
|
}
|
||||||
|
|
||||||
|
// Bepaalt welke lifecycle-timestamps update_job_status schrijft bij een
|
||||||
|
// status-overgang. Set-once (backfill alleen als nu null) houdt de invariant
|
||||||
|
// claimed_at ≤ started_at ≤ finished_at: een job die CLAIMED → done gaat
|
||||||
|
// zonder `running`-rapport krijgt alsnog een started_at, en claimed_at
|
||||||
|
// (normaal door wait_for_job bij claim gezet) wordt nooit overschreven.
|
||||||
|
export function resolveJobTimestamps(
|
||||||
|
status: 'running' | 'done' | 'failed' | 'skipped',
|
||||||
|
current: { claimed_at: Date | null; started_at: Date | null },
|
||||||
|
now: Date = new Date(),
|
||||||
|
): JobTimestampUpdate {
|
||||||
|
const isTerminal = status === 'done' || status === 'failed' || status === 'skipped'
|
||||||
|
const update: JobTimestampUpdate = {}
|
||||||
|
if (current.claimed_at == null) update.claimed_at = now
|
||||||
|
if (current.started_at == null && (status === 'running' || isTerminal)) {
|
||||||
|
update.started_at = now
|
||||||
|
}
|
||||||
|
if (isTerminal) update.finished_at = now
|
||||||
|
return update
|
||||||
|
}
|
||||||
|
|
||||||
export async function maybeCreateAutoPr(opts: {
|
export async function maybeCreateAutoPr(opts: {
|
||||||
jobId: string
|
jobId: string
|
||||||
productId: string
|
productId: string
|
||||||
|
|
@ -569,6 +595,8 @@ export function registerUpdateJobStatusTool(server: McpServer) {
|
||||||
'Report progress on a claimed ClaudeJob. Allowed transitions from CLAIMED/RUNNING: ' +
|
'Report progress on a claimed ClaudeJob. Allowed transitions from CLAIMED/RUNNING: ' +
|
||||||
'running (start), done (finished), failed (error), skipped (no-op exit). ' +
|
'running (start), done (finished), failed (error), skipped (no-op exit). ' +
|
||||||
'The Bearer token must match the token that claimed the job. ' +
|
'The Bearer token must match the token that claimed the job. ' +
|
||||||
|
'Stamps started_at on running and finished_at on done/failed/skipped, and backfills ' +
|
||||||
|
'claimed_at/started_at when missing so claimed_at ≤ started_at ≤ finished_at always holds. ' +
|
||||||
'Before marking done: call verify_task_against_plan first — done is rejected when ' +
|
'Before marking done: call verify_task_against_plan first — done is rejected when ' +
|
||||||
'verify_result is null, EMPTY (unless task.verify_only is true), or when the verify level ' +
|
'verify_result is null, EMPTY (unless task.verify_only is true), or when the verify level ' +
|
||||||
'doesn’t meet task.verify_required: ALIGNED-only is strict; ALIGNED_OR_PARTIAL accepts ' +
|
'doesn’t meet task.verify_required: ALIGNED-only is strict; ALIGNED_OR_PARTIAL accepts ' +
|
||||||
|
|
@ -608,6 +636,8 @@ export function registerUpdateJobStatusTool(server: McpServer) {
|
||||||
select: {
|
select: {
|
||||||
id: true,
|
id: true,
|
||||||
status: true,
|
status: true,
|
||||||
|
claimed_at: true,
|
||||||
|
started_at: true,
|
||||||
claimed_by_token_id: true,
|
claimed_by_token_id: true,
|
||||||
user_id: true,
|
user_id: true,
|
||||||
product_id: true,
|
product_id: true,
|
||||||
|
|
@ -751,10 +781,11 @@ export function registerUpdateJobStatusTool(server: McpServer) {
|
||||||
where: { id: job_id },
|
where: { id: job_id },
|
||||||
data: {
|
data: {
|
||||||
status: dbStatus,
|
status: dbStatus,
|
||||||
...(actualStatus === 'running' ? { started_at: now } : {}),
|
...resolveJobTimestamps(
|
||||||
...(actualStatus === 'done' || actualStatus === 'failed' || actualStatus === 'skipped'
|
actualStatus,
|
||||||
? { finished_at: now }
|
{ claimed_at: job.claimed_at, started_at: job.started_at },
|
||||||
: {}),
|
now,
|
||||||
|
),
|
||||||
...(branchToWrite !== undefined ? { branch: branchToWrite } : {}),
|
...(branchToWrite !== undefined ? { branch: branchToWrite } : {}),
|
||||||
...(pushedAt !== undefined ? { pushed_at: pushedAt } : {}),
|
...(pushedAt !== undefined ? { pushed_at: pushedAt } : {}),
|
||||||
...(summary !== undefined ? { summary } : {}),
|
...(summary !== undefined ? { summary } : {}),
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue