Skip to content

Commit ed5b669

Browse files
committed
fix(showcase/harness): persist partial rollup when a probe run is aborted mid-flight
An aborted D6 run (orchestrator process killed during a pool-churn burst) left its `probe_runs` row orphaned in `running` state with a null summary. The boot-time `sweepStaleRuns` then stamped it `state:failed` with `{total:0,passed:0,failed:0}` and `duration_ms:null`, discarding the partial per-service results the run had actually computed — a 578s run with dozens of green features surfaced as `failed / total:0`. Two minimal, success-path-consistent changes: - probe-invoker: incrementally persist the running partial tally onto the probe_runs row via a new `runWriter.update()` as each fan-out target completes, so an orphaned row already carries real partial progress. Best-effort — never tanks the tick; `finish()` remains the authoritative final write. - run-history: `sweepStaleRuns` now PRESERVES an existing partial summary (and derives a real duration from the persisted started_at) instead of clobbering it to zeros. Rows that died before any target completed still fall back to an explicit empty rollup. Purely the result-aggregation-on-abort path; pool sizing and launch/recycle logic are untouched (churn root cause handled separately).
1 parent 4d2da53 commit ed5b669

5 files changed

Lines changed: 236 additions & 5 deletions

File tree

showcase/harness/src/http/probes.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ function makeFakeWriter(): ProbeRunWriter & {
9797
const recentMap = new Map<string, ProbeRunRecord[]>();
9898
return {
9999
start: async () => ({ id: "row1" }),
100+
update: async () => {},
100101
finish: async () => {},
101102
recent: async (probeId, _limit) => recentMap.get(probeId) ?? [],
102103
setRecent: (probeId, runs) => recentMap.set(probeId, runs),
@@ -896,6 +897,7 @@ describe("GET /api/probes/:id — R2-A.9 graceful degradation", () => {
896897
const sched = makeFakeScheduler();
897898
const writer: ProbeRunWriter = {
898899
start: async () => ({ id: "x" }),
900+
update: async () => {},
899901
finish: async () => {},
900902
recent: async () => {
901903
throw new Error("PB transient outage");

showcase/harness/src/probes/loader/probe-invoker.test.ts

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2428,6 +2428,10 @@ describe("buildProbeInvoker", () => {
24282428
function fakeRunWriter(): {
24292429
writer: ProbeRunWriter;
24302430
starts: Array<{ probeId: string; startedAt: number; triggered: boolean }>;
2431+
updates: Array<{
2432+
id: string;
2433+
summary: { total: number; passed: number; failed: number } | null;
2434+
}>;
24312435
finishes: Array<{
24322436
id: string;
24332437
finishedAt: number;
@@ -2440,6 +2444,10 @@ describe("buildProbeInvoker", () => {
24402444
startedAt: number;
24412445
triggered: boolean;
24422446
}> = [];
2447+
const updates: Array<{
2448+
id: string;
2449+
summary: { total: number; passed: number; failed: number } | null;
2450+
}> = [];
24432451
const finishes: Array<{
24442452
id: string;
24452453
finishedAt: number;
@@ -2452,6 +2460,19 @@ describe("buildProbeInvoker", () => {
24522460
starts.push(opts);
24532461
return { id: `run-${nextId++}` };
24542462
},
2463+
async update(opts) {
2464+
updates.push({
2465+
id: opts.id,
2466+
summary:
2467+
opts.summary === null
2468+
? null
2469+
: {
2470+
total: opts.summary.total,
2471+
passed: opts.summary.passed,
2472+
failed: opts.summary.failed,
2473+
},
2474+
});
2475+
},
24552476
async finish(opts) {
24562477
finishes.push({
24572478
id: opts.id,
@@ -2471,7 +2492,7 @@ describe("buildProbeInvoker", () => {
24712492
return [];
24722493
},
24732494
};
2474-
return { writer, starts, finishes };
2495+
return { writer, starts, updates, finishes };
24752496
}
24762497

24772498
it("registers a ProbeRunTracker on the scheduler entry while the handler runs and clears it after", async () => {
@@ -2734,6 +2755,65 @@ describe("buildProbeInvoker", () => {
27342755
});
27352756
});
27362757

2758+
// ---------------------------------------------------------------------
2759+
// Partial-rollup-on-abort: incrementally persist the partial summary to
2760+
// the probe_runs row as each target completes. Without this, a run that
2761+
// dies mid-flight (orchestrator process killed during a pool-churn
2762+
// burst) leaves a bare `running` row carrying NO partial counts — the
2763+
// boot-time sweep then has nothing to preserve and the dashboard shows
2764+
// `failed / total:0` even though dozens of features actually passed.
2765+
// ---------------------------------------------------------------------
2766+
it("incrementally persists partial summary via runWriter.update as targets complete", async () => {
2767+
const inputSchema = z.object({ key: z.string() }).passthrough();
2768+
const driver: ProbeDriver = {
2769+
kind: "smoke",
2770+
inputSchema,
2771+
async run(ctx, input) {
2772+
const key = (input as { key: string }).key;
2773+
return {
2774+
key,
2775+
state: key.endsWith("bad") ? "red" : "green",
2776+
signal: {},
2777+
observedAt: ctx.now().toISOString(),
2778+
};
2779+
},
2780+
};
2781+
const cfg: ProbeConfig = {
2782+
kind: "smoke",
2783+
id: "smoke",
2784+
schedule: "*/15 * * * *",
2785+
// Serialize so the update sequence is deterministic.
2786+
max_concurrency: 1,
2787+
targets: [{ key: "smoke:a" }, { key: "smoke:b" }, { key: "smoke:bad" }],
2788+
};
2789+
const { writer } = mkWriter();
2790+
const sched = fakeScheduler();
2791+
const rw = fakeRunWriter();
2792+
await buildProbeInvoker(cfg, {
2793+
driver,
2794+
schedulerId: cfg.id,
2795+
discoveryRegistry: createDiscoveryRegistry(),
2796+
writer,
2797+
scheduler: sched.scheduler,
2798+
runWriter: rw.writer,
2799+
...BASE_DEPS,
2800+
})();
2801+
// One update per completed target, each carrying the running partial
2802+
// tally so an orphaned row reflects real progress.
2803+
expect(rw.updates.map((u) => u.summary)).toEqual([
2804+
{ total: 3, passed: 1, failed: 0 },
2805+
{ total: 3, passed: 2, failed: 0 },
2806+
{ total: 3, passed: 2, failed: 1 },
2807+
]);
2808+
// All updates target the same row created by start().
2809+
expect(rw.updates.every((u) => u.id === "run-1")).toBe(true);
2810+
// The final summary still lands via finish().
2811+
expect(rw.finishes[0]).toMatchObject({
2812+
state: "completed",
2813+
summary: { total: 3, passed: 2, failed: 1 },
2814+
});
2815+
});
2816+
27372817
// ---------------------------------------------------------------------
27382818
// R3-A.3: orphan-risk warn log when runWriter.start fails
27392819
// ---------------------------------------------------------------------
@@ -2769,6 +2849,7 @@ describe("buildProbeInvoker", () => {
27692849
const sched = fakeScheduler();
27702850
const failingWriter: ProbeRunWriter = {
27712851
start: vi.fn().mockRejectedValue(new Error("network blip")),
2852+
update: vi.fn().mockResolvedValue(undefined),
27722853
finish: vi.fn().mockResolvedValue(undefined),
27732854
recent: vi.fn().mockResolvedValue([]),
27742855
};
@@ -2829,6 +2910,7 @@ describe("buildProbeInvoker", () => {
28292910
const sched = fakeScheduler();
28302911
const failingWriter: ProbeRunWriter = {
28312912
start: vi.fn().mockRejectedValue(new Error("PB down")),
2913+
update: vi.fn().mockResolvedValue(undefined),
28322914
finish: vi.fn().mockResolvedValue(undefined),
28332915
recent: vi.fn().mockResolvedValue([]),
28342916
};
@@ -2872,6 +2954,7 @@ describe("buildProbeInvoker", () => {
28722954
const sched = fakeScheduler();
28732955
const failingFinish: ProbeRunWriter = {
28742956
start: vi.fn().mockResolvedValue({ id: "run-x" }),
2957+
update: vi.fn().mockResolvedValue(undefined),
28752958
finish: vi.fn().mockRejectedValue(new Error("PB down")),
28762959
recent: vi.fn().mockResolvedValue([]),
28772960
};

showcase/harness/src/probes/loader/probe-invoker.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,27 @@ export function buildProbeInvoker(
634634
state: result.state,
635635
durationMs: Date.now() - targetStart,
636636
});
637+
// Partial-rollup persistence: stamp the running partial tally onto
638+
// the probe_runs row as each target completes so an orphaned row
639+
// (process killed mid-run during a pool-churn burst) reflects real
640+
// progress instead of a null summary. The boot-time sweep then
641+
// preserves this partial rollup rather than zeroing it. Best-effort:
642+
// a runWriter hiccup must never tank the probe tick — finish() in
643+
// the finally block is still the authoritative final write.
644+
if (runWriter && runRowId !== null) {
645+
try {
646+
await runWriter.update({
647+
id: runRowId,
648+
summary: { total: inputs.length, passed, failed },
649+
});
650+
} catch (err) {
651+
logger.error("probe.run-writer-update-failed", {
652+
probeId: cfg.id,
653+
runId: runRowId,
654+
err: err instanceof Error ? err.message : String(err),
655+
});
656+
}
657+
}
637658
try {
638659
await writer.write(result);
639660
} catch (err) {

showcase/harness/src/probes/run-history.test.ts

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@ import { describe, it, expect, beforeEach } from "vitest";
22
import {
33
PROBE_RUNS_COLLECTION,
44
createProbeRunWriter,
5-
type ProbeRunRecord,
5+
sweepStaleRuns,
66
} from "./run-history.js";
7+
import type { ProbeRunRecord } from "./run-history.js";
78
import type { PbClient, ListOpts, ListResult } from "../storage/pb-client.js";
89

910
/**
@@ -339,4 +340,77 @@ describe("run-history", () => {
339340
expect(fake.updateCalls).toHaveLength(0);
340341
});
341342
});
343+
344+
// ---------------------------------------------------------------------
345+
// Partial-rollup-on-abort: when a probe run is aborted mid-flight (the
346+
// orchestrator process dies during a pool-churn burst, leaving a
347+
// `running` row orphaned), the boot-time sweep must PRESERVE whatever
348+
// partial per-service rollup the row already carries rather than
349+
// clobbering it with `{ total: 0, passed: 0, failed: 0 }`. Pre-fix the
350+
// sweep discarded the real partial counts a 578s D6 run had computed
351+
// (41 assertions passed, dozens of features green) — the dashboard /
352+
// probe_runs then showed `failed / total:0` instead of reality.
353+
// ---------------------------------------------------------------------
354+
describe("sweepStaleRuns() — partial-rollup preservation", () => {
355+
it("preserves an existing partial summary instead of zeroing it", async () => {
356+
const fake = fakePb();
357+
const stale = new Date(Date.now() - 20 * 60 * 1000).toISOString();
358+
// An orphaned `running` row that DID accumulate partial per-service
359+
// results before the process died.
360+
const partialSummary = {
361+
total: 38,
362+
passed: 31,
363+
failed: 2,
364+
services: [
365+
{ slug: "d6:starter-lg-react", state: "completed", result: "green" },
366+
{ slug: "d6:starter-lg-py", state: "failed", error: "boom" },
367+
],
368+
};
369+
await fake.pb.create(PROBE_RUNS_COLLECTION, {
370+
probe_id: "d6-all-pills-e2e",
371+
started_at: stale,
372+
finished_at: null,
373+
duration_ms: null,
374+
triggered: false,
375+
state: "running",
376+
summary: partialSummary,
377+
});
378+
379+
const swept = await sweepStaleRuns(fake.pb);
380+
expect(swept).toBe(1);
381+
382+
const updated = fake.updateCalls.at(-1)!;
383+
// The run is terminal + failed (it was aborted), but the partial
384+
// rollup MUST survive — not be overwritten with zeros.
385+
expect(updated.record.state).toBe("failed");
386+
expect(updated.record.summary).toEqual(partialSummary);
387+
});
388+
389+
it("still zeroes the summary for a row that never accumulated partial results", async () => {
390+
const fake = fakePb();
391+
const stale = new Date(Date.now() - 20 * 60 * 1000).toISOString();
392+
// Orphaned row that died before any target completed — summary is
393+
// still null. The sweep should fall back to an explicit empty rollup.
394+
await fake.pb.create(PROBE_RUNS_COLLECTION, {
395+
probe_id: "smoke",
396+
started_at: stale,
397+
finished_at: null,
398+
duration_ms: null,
399+
triggered: false,
400+
state: "running",
401+
summary: null,
402+
});
403+
404+
const swept = await sweepStaleRuns(fake.pb);
405+
expect(swept).toBe(1);
406+
407+
const updated = fake.updateCalls.at(-1)!;
408+
expect(updated.record.state).toBe("failed");
409+
expect(updated.record.summary).toEqual({
410+
total: 0,
411+
passed: 0,
412+
failed: 0,
413+
});
414+
});
415+
});
342416
});

showcase/harness/src/probes/run-history.ts

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,18 @@ export interface ProbeRunWriter {
6363
startedAt: number;
6464
triggered: boolean;
6565
}): Promise<{ id: string }>;
66+
/**
67+
* Persist a partial rollup onto a still-`running` row WITHOUT marking it
68+
* terminal. Called incrementally as each fan-out target completes so an
69+
* orphaned row (orchestrator process killed mid-run — e.g. a pool-churn
70+
* burst) already carries the partial per-service results the run had
71+
* computed. Without this, a `running` row that never reaches `finish()`
72+
* has a null summary, and the boot-time `sweepStaleRuns` has nothing to
73+
* preserve — the dashboard then shows `failed / total:0` even though
74+
* dozens of features actually passed. State stays `running`; only
75+
* `summary` is updated. Best-effort, like `finish()`.
76+
*/
77+
update(opts: { id: string; summary: ProbeRunSummary }): Promise<void>;
6678
/**
6779
* Mark a row finished. `duration_ms` is computed from
6880
* `finishedAt - row.started_at` (read off the persisted row, not from a
@@ -126,6 +138,27 @@ export function createProbeRunWriter(pb: PbClient): ProbeRunWriter {
126138
return { id: created.id };
127139
},
128140

141+
async update(opts) {
142+
// Refresh only the partial summary on a still-running row. We leave
143+
// `state`, `finished_at`, and `duration_ms` untouched so this can be
144+
// called repeatedly mid-run without prematurely marking the row
145+
// terminal. The row must exist (start() created it); a missing row
146+
// means the caller is updating an id it never created — surface it
147+
// rather than silently writing junk, mirroring finish()'s guard.
148+
const existing = await pb.getOne<ProbeRunRow>(
149+
PROBE_RUNS_COLLECTION,
150+
opts.id,
151+
);
152+
if (!existing) {
153+
// eslint-disable-next-line no-console
154+
console.warn("run-history.update: row missing", { runId: opts.id });
155+
return;
156+
}
157+
await pb.update<ProbeRunRow>(PROBE_RUNS_COLLECTION, opts.id, {
158+
summary: opts.summary,
159+
});
160+
},
161+
129162
async finish(opts) {
130163
// Read the persisted started_at so duration is always defined off
131164
// the row — see the JSDoc on `finish()` for why we don't trust a
@@ -202,11 +235,29 @@ export async function sweepStaleRuns(
202235
let swept = 0;
203236
for (const row of stale.items) {
204237
try {
238+
// Partial-rollup preservation: an orphaned `running` row may already
239+
// carry the partial per-service rollup the invoker persisted
240+
// incrementally (`runWriter.update`) before the process died. Marking
241+
// the run `failed` is correct (it WAS aborted), but clobbering the
242+
// summary to `{0,0,0}` discards real work — a 578s D6 run with dozens
243+
// of green features would surface as `failed / total:0`. Keep the
244+
// existing summary when present; fall back to an explicit empty
245+
// rollup only for rows that died before any target completed.
246+
const summary = row.summary ?? { total: 0, passed: 0, failed: 0 };
247+
// Derive a real duration from the persisted started_at when possible
248+
// so a preserved partial run still reports how long it actually ran.
249+
const finishedAt = Date.now();
250+
const startedAtMs = row.started_at
251+
? Date.parse(row.started_at)
252+
: Number.NaN;
253+
const durationMs = Number.isFinite(startedAtMs)
254+
? finishedAt - startedAtMs
255+
: null;
205256
await pb.update<ProbeRunRow>(PROBE_RUNS_COLLECTION, row.id, {
206257
state: "failed",
207-
finished_at: new Date().toISOString(),
208-
duration_ms: null,
209-
summary: { total: 0, passed: 0, failed: 0 },
258+
finished_at: new Date(finishedAt).toISOString(),
259+
duration_ms: durationMs,
260+
summary,
210261
});
211262
swept++;
212263
} catch {

0 commit comments

Comments
 (0)