fix(watchdog): bank the idle-gate condition receipt only on positive delivery

dev50-qa NOT-CLEAR at f610eec7d, reproduced on an isolated candidate daemon: the
first attempt was sent/FAILED yet stored the fingerprint, the pane was then made
reachable without changing the row, and no retry ever came — the seat stayed
silent until an unrelated transition changed the fingerprint.

My repair was worse in kind than the bug it fixed. The original over-woke, which
is noise. This suppressed indefinitely on one transport failure, which is a
dropped obligation.

The cause is in my own justifying comment: I wrote the receipt during evaluate(),
"mirroring how the engine stamps last_fire_at on the send path regardless of
delivery outcome". That mirrored the timing and inverted the consequence.
`last_fire_at` gates a window that EXPIRES, so a failed delivery self-heals; the
condition gate holds until the gated set materially changes, so it never does.
Copying a mechanism's write timing without asking what BOUNDS the state it writes.

The policy now PROPOSES the fingerprint on its send evaluation; the engine banks
it only when delivery returned "ok". Failed delivery leaves the receipt untouched
and the next scan retries. That also means the layer claim from the previous
commit does not survive — suppression cannot be decided in the policy alone,
because only the engine knows whether the wake arrived. The engine change stays
policy-agnostic: a policy that proposes no receipt is untouched.

Pins moved onto the ENGINE path, because the defect lives in the join and a
policy-only pin asserts a decision rather than an outcome. Six pins are RED
without the banking, including QA's exact case: failed delivery banks nothing,
retries, then banks and suppresses once delivery succeeds.

One thing measured rather than assumed, after I got it wrong twice: following a
send, EVERY evaluation returns active_wake_not_due until the window elapses — the
throttle branch preserves `actionable`, unlike the generic skip branch which
clears it. The pins now advance a controllable clock past that window so a skip
can only be the condition gate, instead of passing on the throttle and proving
nothing.
This commit is contained in:
v-openrig-build committed 2026-09-01 12:19:54 +00:00
1 parent f610eec7d2
commit 0bbb9d9e22
5 files changed
+180 -72

No files matched your search

@@ -129,11 +129,20 @@ export function makeIdleGateQitemPolicy(deps: IdleGateQitemDeps): Policy {
};
}
// Both signals joined → ONE bounded wake (single-target, keepalive
// shape). Recorded at decision time, mirroring how the engine stamps
// last_fire_at on the send path regardless of delivery outcome.
db.prepare("UPDATE watchdog_jobs SET last_fired_condition = ? WHERE job_id = ?")
.run(fingerprint, job.jobId);
// Both signals joined → ONE bounded wake (single-target, keepalive shape).
//
// The fingerprint is PROPOSED here and persisted by the engine only if
// delivery succeeds. It is deliberately NOT written at this point.
//
// The first cut of this repair did write it here, justified as mirroring
// how the engine stamps `last_fire_at` on the send path regardless of
// delivery outcome. That mirrored the timing and inverted the consequence:
// `last_fire_at` gates a window that EXPIRES, so a failed delivery
// self-heals, while this gate holds until the gated set materially
// changes. dev50-qa reproduced the result — a send that failed transport
// still recorded the receipt, and the seat was never woken again until an
// unrelated transition released it. Suppression must rest on evidence the
// wake ARRIVED, never on the decision to attempt one.
const primary = gated[0]!;
const message =
job.message ??
@@ -143,6 +152,7 @@ export function makeIdleGateQitemPolicy(deps: IdleGateQitemDeps): Policy {
action: "send",
target: { session: seat },
message,
conditionReceipt: fingerprint,
notes: {
// Audit: which qitem + which activity signal + the join decision.
qitemId: primary.qitemId,
+18 -1
View File
@@ -37,7 +37,24 @@ export interface PolicyJob {
}
export type PolicyEvaluation =
| { action: "send"; target: { session: string }; message: string; notes?: Record<string, unknown> }
| {
action: "send";
target: { session: string };
message: string;
notes?: Record<string, unknown>;
/**
* OPR.0.5.8.1 S2 — an opaque receipt for the CONDITION this send is about,
* persisted by the engine ONLY when delivery actually succeeded.
*
* A policy that suppresses on "already told them" must record that against
* evidence the telling happened. The first cut of this wrote the receipt
* inside evaluate(), before delivery was attempted, so one transient
* transport failure suppressed the wake until the watched condition
* changed — silence instead of noise, which is the worse failure. Policies
* that do not set this are unaffected.
*/
conditionReceipt?: string;
}
| { action: "skip"; reason: string; notes?: Record<string, unknown> }
| { action: "terminal"; reason: string; notes?: Record<string, unknown> };
@@ -543,6 +543,20 @@ export class WatchdogJobsRepository {
.run(stamp, jobId);
}
/**
* OPR.0.5.8.1 S2 — record the CONDITION a job last successfully woke about.
*
* Called by the engine only after a delivery that returned `ok`. A policy that
* suppresses on "already told them" needs its receipt tied to evidence the
* telling happened; recording it on the ATTEMPT turns one transport failure
* into indefinite silence. One overwritten value per job, never appended to.
*/
recordConditionReceipt(jobId: string, receipt: string): void {
this.db
.prepare(`UPDATE watchdog_jobs SET last_fired_condition = ? WHERE job_id = ?`)
.run(receipt, jobId);
}
markTerminal(jobId: string, reason: string): void {
this.db
.prepare(
@@ -475,6 +475,14 @@ export class WatchdogPolicyEngine {
if (!isContextUsageThreshold) {
this.jobsRepo.recordEvaluation(job.jobId, evaluatedAt, true);
}
// OPR.0.5.8.1 S2 — a policy's condition receipt is banked ONLY on positive
// delivery. `status` is "ok" | "failed"; anything that is not a definite ok
// leaves the receipt untouched so the next scan retries. A policy that does
// not propose a receipt is untouched by this, so no other policy's
// behaviour changes.
if (outcome.conditionReceipt !== undefined && delivery.status === "ok") {
this.jobsRepo.recordConditionReceipt(job.jobId, outcome.conditionReceipt);
}
this.jobsRepo.setActionable(job.jobId, true, evaluatedAt, job.lastActionableAt);
this.eventBus.emit({
type: "watchdog.evaluation_fired",
@@ -102,17 +102,38 @@ describe("idle-gate-qitem policy (OPR.0.4.3.16)", () => {
// and a brief seat-active flicker cleared `actionable` and produced a re-fire
// 60.1s into a 120s window. The window was never a cooldown.
function seedJobRow(jobId = "job-1"): void {
db.prepare(
`INSERT INTO watchdog_jobs (job_id, policy, spec_yaml, target_session, interval_seconds,
active_wake_interval_seconds, state, registered_by_session, registered_at)
VALUES (?, 'idle-gate-qitem', 'policy: idle-gate-qitem', ?, 30, 300, 'active', 'ops@kernel', '2026-07-03T07:00:00Z')`,
).run(jobId, SEAT);
// Engine-path S2 pins. These must run through the ENGINE, not the policy
// alone: the condition receipt is proposed by the policy and banked by the
// engine only on a delivery that returned ok. Testing the policy in isolation
// would assert a decision while the defect QA found lives in the join.
function engineFor(deliveryStatus: () => "ok" | "failed") {
let clock = NOW;
const advance = (seconds: number) => { clock = new Date(clock.getTime() + seconds * 1000); };
const jobsRepo = new WatchdogJobsRepository(db);
const historyLog = new WatchdogHistoryLog(db);
const attempts: Array<{ targetSession: string; message: string }> = [];
const deliver: DeliveryFn = async (req) => {
attempts.push(req);
return { status: deliveryStatus() };
};
const engine = new WatchdogPolicyEngine({
jobsRepo, historyLog, eventBus, deliver, now: () => clock,
additionalPolicies: [makeIdleGateQitemPolicy({ db, agentActivityStore: store })],
});
const registered = jobsRepo.register({
policy: "idle-gate-qitem",
specYaml: `policy: idle-gate-qitem\ntarget:\n session: ${SEAT}\ninterval_seconds: 30\n`,
targetSession: SEAT,
intervalSeconds: 30,
activeWakeIntervalSeconds: 300,
registeredBySession: "ops@kernel",
});
const evaluate = () => engine.evaluate(jobsRepo.getByIdOrThrow(registered.jobId));
const receipt = () =>
(db.prepare("SELECT last_fired_condition AS c FROM watchdog_jobs WHERE job_id = ?")
.get(registered.jobId) as { c: string | null } | undefined)?.c ?? null;
return { attempts, evaluate, receipt, advance };
}
const firedCondition = (jobId = "job-1") =>
(db.prepare("SELECT last_fired_condition AS c FROM watchdog_jobs WHERE job_id = ?").get(jobId) as
| { c: string | null }
| undefined)?.c ?? null;
function appendTransition(qitemId: string, note: string, id: number): void {
db.prepare(
`INSERT INTO queue_transitions (transition_id, qitem_id, ts, state, actor_session, transition_note)
@@ -120,92 +141,130 @@ describe("idle-gate-qitem policy (OPR.0.4.3.16)", () => {
).run(id, qitemId, note);
}
it("S2 R-1 — an UNCHANGED gated set does not re-wake when the window expires", async () => {
seedJobRow();
it("S2 R-1 — an UNCHANGED gated set does not re-wake, however many windows pass", async () => {
seedGateQitem("q-gate-1");
seedActivity("Stop", FRESH);
const policy = makeIdleGateQitemPolicy({ db, agentActivityStore: store });
expect((await policy.evaluate(makeJob())).action).toBe("send");
// Second evaluation with the window fully elapsed and nothing changed.
const out = await policy.evaluate(makeJob({ lastFireAt: "2026-07-03T07:00:00.000Z" }));
expect(out.action).toBe("skip");
expect((out as { reason: string }).reason).toBe("gate_condition_unchanged");
const { attempts, evaluate, advance } = engineFor(() => "ok");
await evaluate();
advance(301);
await evaluate();
advance(301);
await evaluate();
expect(attempts).toHaveLength(1); // three windows, one wake
});
it("S2 R-2 — a seat_active FLICKER does not reset the fired-for memory", async () => {
// The engine clears `actionable` on every skip, which is what made a brief
// busy moment re-open immediate firing. The condition memory lives in the
// policy and keys to the CONDITION, so an activity skip cannot launder it.
seedJobRow();
it("S2 R-2 — a seat_active FLICKER cannot manufacture a second wake", async () => {
// The engine clears `actionable` on every skip, which is what let a brief
// busy moment bypass the active-wake window entirely. The condition receipt
// is not touched by an activity skip, so the flicker buys nothing.
seedGateQitem("q-gate-1");
seedActivity("Stop", FRESH);
const policy = makeIdleGateQitemPolicy({ db, agentActivityStore: store });
expect((await policy.evaluate(makeJob())).action).toBe("send");
const afterFire = firedCondition();
expect(afterFire).not.toBeNull();
seedActivity("UserPromptSubmit", FRESH); // seat goes active
const busy = await policy.evaluate(makeJob());
expect((busy as { reason: string }).reason).toBe("seat_active");
expect(firedCondition()).toBe(afterFire); // memory untouched by the flicker
const { attempts, evaluate, advance } = engineFor(() => "ok");
await evaluate();
expect(attempts).toHaveLength(1);
seedActivity("UserPromptSubmit", FRESH); // seat busy
await evaluate();
seedActivity("Stop", FRESH); // idle again
const out = await policy.evaluate(makeJob({ lastFireAt: null, actionable: false } as never));
expect(out.action).toBe("skip");
expect((out as { reason: string }).reason).toBe("gate_condition_unchanged");
advance(301); // past the window: only the condition gate can hold
await evaluate();
expect(attempts).toHaveLength(1);
});
it("S2 preserve — a NEW gate qitem arriving at the seat wakes", async () => {
seedJobRow();
it("S2 F1 — a FAILED delivery banks no receipt, so the next scan RETRIES", async () => {
// dev50-qa NOT-CLEAR at f610eec7d. The first cut wrote the receipt inside
// evaluate(), before delivery was attempted, so one transport failure
// suppressed the wake until the gated set changed — silence, which is worse
// than the noise being fixed. Suppression must rest on evidence the wake
// ARRIVED.
seedGateQitem("q-gate-1");
seedActivity("Stop", FRESH);
const policy = makeIdleGateQitemPolicy({ db, agentActivityStore: store });
expect((await policy.evaluate(makeJob())).action).toBe("send");
let status: "ok" | "failed" = "failed";
const { attempts, evaluate, receipt, advance } = engineFor(() => status);
seedGateQitem("q-gate-2");
expect((await policy.evaluate(makeJob())).action).toBe("send");
await evaluate();
expect(attempts).toHaveLength(1);
expect(receipt()).toBeNull(); // nothing banked for a failed send
// The engine still stamps last_fire_at on a failed send, so its active-wake
// window remains a FLOOR beneath the condition gate — exactly as ruled. The
// retry is therefore bounded by that window, not blocked until the row
// changes. Bounded and self-healing is the whole difference from the defect.
await evaluate();
expect(attempts).toHaveLength(1); // inside the window: floor holds
advance(301);
await evaluate();
expect(attempts).toHaveLength(2); // window elapsed -> retried, row unchanged
status = "ok";
advance(301);
await evaluate();
expect(attempts).toHaveLength(3);
expect(receipt()).not.toBeNull(); // banked only now
advance(301);
await evaluate();
expect(attempts).toHaveLength(3); // condition gate holds past the window
});
it("S2 preserve — a MATERIAL transition on the row re-wakes promptly", async () => {
seedJobRow();
it("S2 preserve — a MATERIAL transition re-wakes promptly", async () => {
seedGateQitem("q-gate-1");
seedActivity("Stop", FRESH);
const policy = makeIdleGateQitemPolicy({ db, agentActivityStore: store });
expect((await policy.evaluate(makeJob())).action).toBe("send");
expect((await policy.evaluate(makeJob())).action).toBe("skip");
const { attempts, evaluate, advance } = engineFor(() => "ok");
await evaluate();
await evaluate();
expect(attempts).toHaveLength(1);
appendTransition("q-gate-1", "reviewer asked a question", 9001);
expect((await policy.evaluate(makeJob())).action).toBe("send");
advance(301);
await evaluate();
expect(attempts).toHaveLength(2);
});
it("S2 preserve — a NEW gate qitem arriving wakes", async () => {
seedGateQitem("q-gate-1");
seedActivity("Stop", FRESH);
const { attempts, evaluate, advance } = engineFor(() => "ok");
await evaluate();
seedGateQitem("q-gate-2");
// MEASURED, not assumed — I got this wrong twice before instrumenting the
// engine. After a send, EVERY evaluation returns active_wake_not_due until
// the window elapses: the throttle branch preserves `actionable`, unlike the
// generic skip branch which clears it. So the window is a real floor here,
// and genuinely new work waits it out on a seat that stays idle.
await evaluate();
expect(attempts).toHaveLength(1); // floor holds inside the window
advance(301);
await evaluate();
expect(attempts).toHaveLength(2); // window elapsed, condition changed -> wakes
});
it("S2 — wake-machinery markers are NEVER material (else every wake justifies the next)", async () => {
seedJobRow();
seedGateQitem("q-gate-1");
seedActivity("Stop", FRESH);
const policy = makeIdleGateQitemPolicy({ db, agentActivityStore: store });
expect((await policy.evaluate(makeJob())).action).toBe("send");
const { attempts, evaluate, advance } = engineFor(() => "ok");
await evaluate();
appendTransition("q-gate-1", "wake-attempt: 1/3 outcome=delivered", 9002);
appendTransition("q-gate-1", "escalation-rung: orchestrator -> orch@rig", 9003);
const out = await policy.evaluate(makeJob());
expect(out.action).toBe("skip");
expect((out as { reason: string }).reason).toBe("gate_condition_unchanged");
// Past the window, so a skip here can only be the CONDITION gate — otherwise
// this pin would pass on the throttle and prove nothing.
advance(301);
await evaluate();
expect(attempts).toHaveLength(1);
});
it("S2 — a suppressed wake stays derivable: the job records the condition it fired for", async () => {
// Suppressing the WAKE, never the record. The row itself is untouched and
// still appears in held/escalations; the job carries the exact condition.
seedJobRow();
it("S2 — a suppressed wake stays derivable, and the row is never touched", async () => {
// Suppressing the WAKE, never the record.
seedGateQitem("q-gate-1");
seedActivity("Stop", FRESH);
const policy = makeIdleGateQitemPolicy({ db, agentActivityStore: store });
expect(firedCondition()).toBeNull();
await policy.evaluate(makeJob());
const recorded = firedCondition();
expect(recorded).toMatch(/^[0-9a-f]{32}$/);
await policy.evaluate(makeJob());
expect(firedCondition()).toBe(recorded); // one overwritten value, not a ledger
const { evaluate, receipt, advance } = engineFor(() => "ok");
expect(receipt()).toBeNull();
await evaluate();
const banked = receipt();
expect(banked).toMatch(/^[0-9a-f]{32}$/);
advance(301);
await evaluate();
expect(receipt()).toBe(banked); // one overwritten value, not a ledger
expect(
(db.prepare("SELECT COUNT(*) AS n FROM queue_items WHERE qitem_id = 'q-gate-1'").get() as { n: number }).n,
).toBe(1);