fix(delivery-queue): increment retryCount on deadline-deferred entries

stephenschoettler · steipete · commit b07312c55b5d · 2026-03-22T18:39:24.000-07:00
Codex P1: entries deferred by the recovery time budget kept retryCount=0
forever, so they could loop across restarts without ever reaching MAX_RETRIES.

After breaking on deadline, call failDelivery() for all remaining entries
so retryCount is incremented. Entries stay in queue until MAX_RETRIES is
reached and they are pruned normally.

Also updates the maxRecoveryMs test to assert retryCount=1 on deferred entries.
diff --git a/src/infra/outbound/delivery-queue.ts b/src/infra/outbound/delivery-queue.ts
@@ -341,10 +341,19 @@ export async function recoverPendingDeliveries(opts: {
   let skippedMaxRetries = 0;
   let deferredBackoff = 0;
 
-  for (const entry of pending) {
+  for (let i = 0; i < pending.length; i++) {
+    const entry = pending[i];
     const now = Date.now();
     if (now >= deadline) {
       opts.log.warn(`Recovery time budget exceeded — remaining entries deferred to next startup`);
+      // Increment retryCount for all remaining entries so that entries which
+      // are consistently deferred by the time budget eventually reach
+      // MAX_RETRIES and are pruned rather than looping forever.
+      await Promise.allSettled(
+        pending
+          .slice(i)
+          .map((e) => failDelivery(e.id, "recovery time budget exceeded", opts.stateDir)),
+      );
       break;
     }
     if (entry.retryCount >= MAX_RETRIES) {
diff --git a/src/infra/outbound/outbound.test.ts b/src/infra/outbound/outbound.test.ts
@@ -504,10 +504,14 @@ describe("delivery-queue", () => {
       expect(result.skippedMaxRetries).toBe(0);
       expect(result.deferredBackoff).toBe(0);
 
-      // All entries should still be in the queue.
+      // All entries should still be in the queue (retryCount < MAX_RETRIES).
       const remaining = await loadPendingDeliveries(tmpDir);
       expect(remaining).toHaveLength(3);
 
+      // retryCount should be incremented on all deferred entries so they
+      // eventually reach MAX_RETRIES and are pruned rather than looping forever.
+      expect(remaining.every((e) => e.retryCount === 1)).toBe(true);
+
       // Should have logged a warning about deferred entries.
       expect(log.warn).toHaveBeenCalledWith(expect.stringContaining("deferred to next startup"));
     });
