fix(dashboard-agent): a failed batch check keeps the chain alive

Author: kathiekiwi

apps/webapp/app/services/dashboardAgentWatchToken.server.ts

@@ -97,7 +97,8 @@ const WATCH_BATCH_TOKEN_CLIENT = "dashboard-agent-watch-batch";
 
 /**
  * How long a chain's token lives. A chain has no deadline to pin it to, but it must still
- * expire; an expired one is self-healing via the re-arm backstop.
+ * expire. A chain whose token no longer verifies keeps ticking — a failed check is never a
+ * verdict — and gets a fresh token when the re-arm backstop starts the next epoch.
  */
 export const WATCH_BATCH_TOKEN_TTL_MS = 30 * 24 * 60 * 60 * 1000;
 

internal-packages/dashboard-agent/GUIDEBOOK.md

@@ -371,7 +371,9 @@ falls within one cadence is always due, so the final evaluation is never missed.
 The group's chain stops only when nothing is active **and** nothing is owed.
 
 A check that came back `unavailable` in a batch is recorded as an attempt rather
-than as a check, so the stall streak and the last-checked time survive it.
+than as a check, so the stall streak and the last-checked time survive it. If the
+batch check itself can't be read, nothing was looked at, so nothing is recorded —
+and the chain still reschedules, so the group keeps its cadence.
 
 A sweep (`dashboardAgentWatchSweep.server.ts`) is the backstop:
 

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

@@ -59,7 +59,8 @@ export type WatchBatchTickDeps = {
 };
 
 export type WatchBatchTickResult = {
-  outcome: "ticked" | "stale";
+  // `unavailable`: the check phase itself couldn't run, so the group learned nothing.
+  outcome: "ticked" | "stale" | "unavailable";
   results: Array<{ watchId: string; outcome?: WatchTickOutcome; error?: string }>;
   rescheduled: boolean;
 };
@@ -92,7 +93,22 @@ export async function runWatchBatchTick(
   payload: WatchBatchTickPayload,
   deps: WatchBatchTickDeps
 ): Promise<WatchBatchTickResult> {
-  const response = await deps.checkBatch(payload);
+  let response: WatchBatchCheckResponse;
+  try {
+    response = await deps.checkBatch(payload);
+  } catch (error) {
+    // A check failure is never a verdict: the group learned nothing, so nothing is
+    // recorded — no watch was looked at — and the chain ticks on to try again.
+    logger.error("dashboard-agent watch batch check is unavailable; ticking on", {
+      environmentId: payload.environmentId,
+      cadenceMinutes: payload.cadenceMinutes,
+      epoch: payload.epoch,
+      tick: payload.tick,
+      error: (error as Error).message,
+    });
+    await scheduleNextBatchTick(payload, deps);
+    return { outcome: "unavailable", results: [], rescheduled: true };
+  }
 
   if (response.stale) {
     logger.info("dashboard-agent watch batch is stale; exiting", {
@@ -114,16 +130,7 @@ export async function runWatchBatchTick(
   // Before the rethrow below, so the chain survives a watch that keeps failing.
   let rescheduled = false;
   if (response.continues) {
-    const next = payload.tick + 1;
-    await deps.reschedule(
-      { ...payload, tick: next },
-      {
-        delay: `${payload.cadenceMinutes}m`,
-        // Keyed on the epoch too, so a re-armed chain can't collide with its
-        // predecessor's keys.
-        idempotencyKey: `watch-batch:${payload.environmentId}:${payload.cadenceMinutes}:${payload.epoch}:tick:${next}`,
-      }
-    );
+    await scheduleNextBatchTick(payload, deps);
     rescheduled = true;
   }
 
@@ -141,6 +148,24 @@ export async function runWatchBatchTick(
   return { outcome: "ticked", results, rescheduled };
 }
 
+// The successor's generation comes from this run's own, and rides in the idempotency
+// key, so a resumed or duplicated tick schedules the same successor once.
+async function scheduleNextBatchTick(
+  payload: WatchBatchTickPayload,
+  deps: WatchBatchTickDeps
+): Promise<void> {
+  const next = payload.tick + 1;
+  await deps.reschedule(
+    { ...payload, tick: next },
+    {
+      delay: `${payload.cadenceMinutes}m`,
+      // Keyed on the epoch too, so a re-armed chain can't collide with its
+      // predecessor's keys.
+      idempotencyKey: `watch-batch:${payload.environmentId}:${payload.cadenceMinutes}:${payload.epoch}:tick:${next}`,
+    }
+  );
+}
+
 /** One watch of a batch: each resolves in its own try, so a failure isolates. */
 async function resolveBatchEntry(
   payload: WatchBatchTickPayload,

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

@@ -1344,28 +1344,42 @@ describe("runWatchBatchTick", () => {
     expect(appends.map((append) => append.chatId)).toEqual(["chat_2"]);
   });
 
-  it("throws when the batch check itself can't be read, before anything is scheduled", async () => {
-    const rows = group(2);
+  it("a batch check that can't be read keeps the chain alive, and records nothing", async () => {
+    const rows = group(2, { tickCount: 6, lastCheckedAt: NOW, lastResult: { runs: 1 } });
     const { store, calls } = fakeStore(rows[0]!, rows[1]!);
-    const { deliver } = fakeDeliver();
-    const triggers: unknown[] = [];
+    const { appends, deliver } = fakeDeliver();
+    const triggers: Array<{ payload: WatchBatchTickPayload; options: Record<string, unknown> }> =
+      [];
 
-    await expect(
-      runWatchBatchTick(
-        batchPayload(7),
-        batchDeps({
-          store,
-          response: async () => {
-            throw new Error("the batch check returned 500");
-          },
-          deliver,
-          reschedule: async () => void triggers.push(1),
-        })
-      )
-    ).rejects.toThrow("the batch check returned 500");
+    const result = await runWatchBatchTick(
+      batchPayload(7),
+      batchDeps({
+        store,
+        response: async () => {
+          throw new Error("the batch check returned 401");
+        },
+        deliver,
+        reschedule: async (payload, options) => void triggers.push({ payload, options }),
+      })
+    );
+
+    expect(result).toEqual({ outcome: "unavailable", results: [], rescheduled: true });
 
+    // Nothing was looked at, so the streak and the last-checked time stand.
     expect(calls.checks).toHaveLength(0);
-    expect(triggers).toHaveLength(0);
+    expect(calls.claims).toHaveLength(0);
+    expect(appends).toHaveLength(0);
+    expect(rows.map((row) => row.status)).toEqual(["active", "active"]);
+    expect(rows.map((row) => row.tickCount)).toEqual([6, 6]);
+    expect(rows.map((row) => row.lastCheckedAt)).toEqual([NOW, NOW]);
+
+    expect(triggers).toHaveLength(1);
+    expect(triggers[0]?.payload).toEqual(batchPayload(8));
+    // The same key the healthy path uses, so a retried tick can't fork the chain.
+    expect(triggers[0]?.options).toEqual({
+      delay: "5m",
+      idempotencyKey: "watch-batch:env_1:5:3:tick:8",
+    });
   });
 });