Skip to content

Commit 6ec291e

Browse files
authored
fix(worker): stamp completedAt/durationSec when a cancelled run settles (#281)
1 parent dd610e9 commit 6ec291e

2 files changed

Lines changed: 71 additions & 2 deletions

File tree

apps/worker/src/lib/telemetry/run-telemetry.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -747,6 +747,36 @@ describe("live clarification park status", () => {
747747
expect(await row("wrun_missing")).toBeUndefined();
748748
});
749749

750+
// RUN-CANCEL-BOOKKEEPING-001: "blocked" is terminal, so the cancel settle
751+
// must finalize the same completion bookkeeping a normally-finished run gets,
752+
// not leave the row terminal-yet-never-completed.
753+
it("markRunBlockedOnCancel finalizes completedAt/durationSec like a completed run", async () => {
754+
await db.insert(workflowRuns).values({
755+
runId: "wrun_cancel_bookkeeping",
756+
subjectKey: "ticket:jira:PROJ-1",
757+
workflowId: "wf_agent",
758+
workflowName: "Agent",
759+
ticketKey: "PROJ-1",
760+
status: "awaiting",
761+
startedAt: new Date("2026-06-15T10:00:05Z"),
762+
});
763+
await markRunBlockedOnCancel(db, "wrun_cancel_bookkeeping");
764+
const r = await row("wrun_cancel_bookkeeping");
765+
expect(r.status).toBe("blocked");
766+
expect(r.completedAt).not.toBeNull();
767+
expect(r.durationSec).not.toBeNull();
768+
expect(r.durationSec!).toBeGreaterThan(0); // now() - startedAt
769+
});
770+
771+
it("markRunBlockedOnCancel stamps completedAt but no duration when no start was recorded", async () => {
772+
await seed("wrun_cancel_no_start", "awaiting");
773+
await markRunBlockedOnCancel(db, "wrun_cancel_no_start");
774+
const r = await row("wrun_cancel_no_start");
775+
expect(r.status).toBe("blocked");
776+
expect(r.completedAt).not.toBeNull(); // the settle time itself
777+
expect(r.durationSec).toBeNull(); // no start to measure from, no fabricated zero
778+
});
779+
750780
// A workflow step can be re-executed after a worker restart, so every writer
751781
// has to survive being called twice with the same argument.
752782
it("repeating any of the three writes changes nothing", async () => {
@@ -814,6 +844,25 @@ describe("markRunBlockedByOperator", () => {
814844
expect(await row("wrun_op_missing")).toBeUndefined();
815845
});
816846

847+
// RUN-CANCEL-BOOKKEEPING-001: the operator settle is terminal too, so it must
848+
// finalize the same completion bookkeeping recordRunUsage writes.
849+
it("finalizes completedAt/durationSec like a completed run", async () => {
850+
await db.insert(workflowRuns).values({
851+
runId: "wrun_op_bookkeeping",
852+
subjectKey: "sched:demo:hourly",
853+
workflowId: "wf_agent",
854+
workflowName: "Agent",
855+
status: "running",
856+
startedAt: new Date("2026-06-15T10:00:05Z"),
857+
});
858+
await markRunBlockedByOperator(db, "wrun_op_bookkeeping", "cancelled by operator kate");
859+
const r = await row("wrun_op_bookkeeping");
860+
expect(r.status).toBe("blocked");
861+
expect(r.completedAt).not.toBeNull();
862+
expect(r.durationSec).not.toBeNull();
863+
expect(r.durationSec!).toBeGreaterThan(0); // now() - startedAt
864+
});
865+
817866
// A workflow step can be re-executed after a worker restart, so the write has
818867
// to survive being called twice: the second call sees 'blocked' and no-ops.
819868
it("is idempotent under re-execution", async () => {

apps/worker/src/lib/telemetry/run-telemetry.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -539,11 +539,23 @@ export async function markRunResumed(db: Db, runId: string): Promise<void> {
539539
* open). The run never resumes to clear it itself and the cron never downgrades
540540
* a frozen status, so without this the cancelled row keeps showing awaiting
541541
* input. Guarded on exactly "awaiting" so it only ever touches a parked run.
542+
*
543+
* "blocked" is terminal, so the settle also finalizes the lifecycle the way
544+
* recordRunUsage does on completion: completedAt keeps a precise value if one
545+
* was already recorded, else stamps now(); durationSec is filled from a known
546+
* start. Without this a cancelled run reads terminal-yet-never-completed, with
547+
* no duration, forever (the cron never touches a frozen status and keepIfNull
548+
* only keeps what the settle wrote).
542549
*/
543550
export async function markRunBlockedOnCancel(db: Db, runId: string): Promise<void> {
544551
await db
545552
.update(workflowRuns)
546-
.set({ status: "blocked", updatedAt: sql`now()` })
553+
.set({
554+
status: "blocked",
555+
completedAt: sql`coalesce(${workflowRuns.completedAt}, now())`,
556+
durationSec: durationFromStart(),
557+
updatedAt: sql`now()`,
558+
})
547559
.where(and(eq(workflowRuns.runId, runId), eq(workflowRuns.status, "awaiting")));
548560
}
549561

@@ -570,7 +582,15 @@ export async function markRunBlockedByOperator(
570582
): Promise<void> {
571583
await db
572584
.update(workflowRuns)
573-
.set({ status: "blocked", statusReason: reason, updatedAt: sql`now()` })
585+
.set({
586+
status: "blocked",
587+
statusReason: reason,
588+
// Terminal settle, so finalize completedAt/durationSec exactly like
589+
// recordRunUsage does on completion (see markRunBlockedOnCancel).
590+
completedAt: sql`coalesce(${workflowRuns.completedAt}, now())`,
591+
durationSec: durationFromStart(),
592+
updatedAt: sql`now()`,
593+
})
574594
.where(
575595
and(
576596
eq(workflowRuns.runId, runId),

0 commit comments

Comments
 (0)