fix(supervisor): only hold the verdict when the read itself failed

Author: nicktrn

.server-changes/backpressure-hold-last-verdict.md

@@ -3,4 +3,4 @@ area: supervisor
 type: fix
 ---
 
-Self-hosted Kubernetes deployments no longer resume pulling work the moment the safety check can't be read. It now holds its last decision for a grace period instead of releasing after a few seconds.
+Self-hosted Kubernetes deployments no longer resume pulling work as soon as the safety check becomes unreadable. It now holds its last decision for a grace period while the check recovers.

apps/supervisor/src/backpressure/backpressureMetrics.ts

@@ -8,7 +8,7 @@ export class BackpressureMetrics {
   readonly dryRun: Gauge<string>;
   /** Dequeue attempts the gate skipped - or would have, in dry-run (labelled). */
   readonly skipsTotal: Counter<string>;
-  /** Verdict reads that failed; the previous verdict is held until it ages out. */
+  /** Verdict source reads that failed (threw). */
   readonly readFailuresTotal: Counter<string>;
 
   constructor(opts: { register: Registry; prefix?: string }) {
@@ -35,7 +35,7 @@ export class BackpressureMetrics {
 
     this.readFailuresTotal = new Counter({
       name: `${prefix}_read_failures_total`,
-      help: "Verdict source reads that failed; the previous verdict is held until it ages out",
+      help: "Verdict source reads that threw",
       registers: [opts.register],
     });
   }

apps/supervisor/src/backpressure/backpressureMonitor.test.ts

@@ -120,6 +120,29 @@ describe("BackpressureMonitor", () => {
     monitor.stop();
   });
 
+  it("releases immediately on an explicit null even when a grace window is configured", async () => {
+    let engaged: boolean | null = true;
+    const source: BackpressureSignalSource = {
+      read: async () => (engaged === null ? null : { engaged, ts: Date.now() }),
+    };
+    const monitor = new BackpressureMonitor({
+      enabled: true,
+      source,
+      refreshIntervalMs: 1000,
+      maxVerdictAgeMs: 15_000,
+    });
+
+    monitor.start();
+    await vi.advanceTimersByTimeAsync(0);
+    expect(monitor.shouldSkipDequeue()).toBe(true);
+
+    engaged = null;
+    await vi.advanceTimersByTimeAsync(1000);
+    expect(monitor.shouldSkipDequeue()).toBe(false); // null is an answer, not a failure
+
+    monitor.stop();
+  });
+
   it("fails open when the source reports unknown (null)", async () => {
     const { source } = countingSource(null);
     const monitor = new BackpressureMonitor({ enabled: true, source, refreshIntervalMs: 1000 });

apps/supervisor/src/backpressure/backpressureMonitor.ts

@@ -12,9 +12,10 @@ export type BackpressureVerdict = {
 };
 
 /**
- * Source of the current backpressure verdict. `read()` returns `null` when the
- * verdict is unknown (missing/unreadable) - the monitor treats unknown as
- * "not engaged" (fail-open).
+ * Source of the current backpressure verdict. `read()` returns `null` when the source
+ * answered but there is no verdict - the monitor treats that as "not engaged"
+ * (fail-open). A thrown error is different: the read itself failed, so the monitor
+ * keeps the previous verdict until it ages past `maxVerdictAgeMs`.
  */
 export interface BackpressureSignalSource {
   read(): Promise<BackpressureVerdict | null>;
@@ -163,18 +164,20 @@ export class BackpressureMonitor {
       readError = error;
     }
 
-    if (next) {
-      this.verdict = next;
+    if (readError === undefined) {
+      this.verdict = next; // an explicit null means "no pressure", so honour it
       this.readFailing = false;
     } else {
-      if (this.opts.maxVerdictAgeMs === undefined) {
+      const held = this.opts.maxVerdictAgeMs !== undefined;
+      if (!held) {
         this.verdict = null; // unbounded hold could pin the brake forever
       }
       this.opts.metrics?.readFailuresTotal.inc();
       if (!this.readFailing) {
         this.readFailing = true; // log once per outage, not once per tick
-        this.opts.logger?.error("backpressure read failed, holding last verdict", {
-          reason: readError ? String(readError) : "no verdict",
+        this.opts.logger?.error("backpressure read failed", {
+          reason: String(readError),
+          heldPreviousVerdict: held,
           engaged: this.computeEngaged(),
         });
       }