Skip to content

Commit 653e2cd

Browse files
committed
fix(dashboard-agent): stop a run of failed watch checks nesting forever
Each failed check stored the whole previous `lastResult` under `previous`, so consecutive failures wrapped one another without bound. The row escapes: an unverified expiry copies `lastResult` into the wake facts, which the alert and the webhook body serialise. A failure record is now unwrapped before it is stored, so `previous` is always the last observation the check really made.
1 parent 5ea7077 commit 653e2cd

2 files changed

Lines changed: 73 additions & 1 deletion

File tree

internal-packages/dashboard-agent/src/watch-lifecycle.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,27 @@ export type CheckOutcome =
5757
// non-2xx is a failed check and the tick keeps watching to the row's own deadline.
5858
export const REVOKED_CODES = new Set(["access_revoked", "cancelled", "not_found"]);
5959

60+
/**
61+
* The last result the check actually produced. A failure record is unwrapped, so a run of
62+
* failures replaces one another instead of nesting — the row's `lastResult` reaches the
63+
* wake facts, the alert and the webhook body.
64+
*/
65+
export function lastObservedResult(lastResult: unknown): Record<string, unknown> | undefined {
66+
let current = lastResult;
67+
while (isCheckFailure(current)) current = current.previous;
68+
return current !== null && typeof current === "object" && !Array.isArray(current)
69+
? (current as Record<string, unknown>)
70+
: undefined;
71+
}
72+
73+
function isCheckFailure(value: unknown): value is { previous?: unknown } {
74+
return (
75+
typeof value === "object" &&
76+
value !== null &&
77+
(value as { checkFailed?: unknown }).checkFailed === true
78+
);
79+
}
80+
6081
// One watch's tick, shared by the per-watch task and the batch. The order of the
6182
// branches below is the algorithm.
6283
export async function runWatchLifecycle(
@@ -121,7 +142,11 @@ export async function runWatchLifecycle(
121142
// The generation is spent and the result isn't trusted, so keep watching.
122143
await deps.store.recordWatchCheck({
123144
id: claimed.id,
124-
lastResult: { checkFailed: true, detail: check.detail, previous: claimed.lastResult },
145+
lastResult: {
146+
checkFailed: true,
147+
detail: check.detail,
148+
previous: lastObservedResult(claimed.lastResult),
149+
},
125150
});
126151
if (check.handOff) return { outcome: "handed_off", tickCount: args.tick };
127152
await deps.onPending(claimed, args.tick);

internal-packages/dashboard-agent/src/watch-tick.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,53 @@ describe("runWatchTick", () => {
566566
expect(row.lastResult).toMatchObject({ checkFailed: true, previous: { pending: 12 } });
567567
});
568568

569+
it("a run of failed checks does not nest: `previous` stays the last real observation", async () => {
570+
const { store, row } = fakeStore(
571+
watchRow({
572+
tickCount: 2,
573+
lastCheckedAt: new Date("2026-01-01T12:50:00.000Z"),
574+
lastResult: { pending: 12 },
575+
})
576+
);
577+
const { fetch } = fakeFetch(() => ({ status: 503, body: { error: "clickhouse is down" } }));
578+
const { deliver } = fakeDeliver();
579+
const { reschedule } = fakeReschedule();
580+
const d = deps({ store, fetch, deliver, reschedule });
581+
582+
for (const tick of [3, 4, 5, 6]) await runWatchTick(payloadFor(tick), d);
583+
584+
// One level, not four: the row is serialised into the wake, the alert and the webhook.
585+
expect((row.lastResult as { previous?: unknown }).previous).toEqual({ pending: 12 });
586+
});
587+
588+
it("the facts an unverified expiry carries are bounded by the same unwrap", async () => {
589+
const { store, row } = fakeStore(
590+
watchRow({
591+
tickCount: 28,
592+
lastCheckedAt: new Date("2026-01-01T12:50:00.000Z"),
593+
lastResult: { pending: 41 },
594+
})
595+
);
596+
const { fetch } = fakeFetch(() => ({ status: 500, body: { error: "metrics unavailable" } }));
597+
const { appends, deliver } = fakeDeliver();
598+
const { reschedule } = fakeReschedule();
599+
600+
for (const tick of [29, 30, 31]) {
601+
await runWatchTick(payloadFor(tick), deps({ store, fetch, deliver, reschedule }));
602+
}
603+
await runWatchTick(
604+
payloadFor(32),
605+
deps({ store, fetch, deliver, reschedule, now: new Date("2026-01-01T13:00:01.000Z") })
606+
);
607+
608+
expect(row.status).toBe("expired");
609+
const facts = (appends[0]!.action as { facts: Record<string, unknown> }).facts;
610+
expect(facts.reason).toBe("unverified_at_expiry");
611+
const observation = facts.lastObservation as { checkFailed?: boolean; previous?: unknown };
612+
expect(observation.checkFailed).toBe(true);
613+
expect(observation.previous).toEqual({ pending: 41 });
614+
});
615+
569616
it("access_revoked: exits without resolving, delivering, or rescheduling", async () => {
570617
const { store, calls, row } = fakeStore(watchRow());
571618
const { fetch } = fakeFetch(() => ({

0 commit comments

Comments
 (0)