fix: address review feedback — stale timeout warning, active count, default timeout

kyletabor · claude · kyletabor · commit bd6c34610c68 · 2026-03-17T06:20:57.000-07:00
- Gate timeout diag.warn on completedCurrentGeneration; stale post-reset
  timeouts downgrade to diag.debug to avoid misleading on-call noise
- Capture activeTaskIds.size before completeTask() removal so the
  timeout warning reports the pre-removal active count
- Increase DEFAULT_TASK_TIMEOUT_MS from 5 to 15 minutes — the lane
  timeout is a last-resort safety net above the agent-level timeout
  (default 600s / 10 min), so it must be higher to avoid killing
  legitimate long-running tasks

Co-Authored-By: Claude Opus 4.6 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/src/process/command-queue.test.ts b/src/process/command-queue.test.ts
@@ -429,8 +429,8 @@ describe("command queue", () => {
         { taskTimeoutMs: 0 },
       );
 
-      // Advance well past the default 5-minute timeout.
-      await vi.advanceTimersByTimeAsync(10 * 60 * 1000);
+      // Advance well past the default 15-minute timeout.
+      await vi.advanceTimersByTimeAsync(20 * 60 * 1000);
 
       // Task should still be active — not timed out.
       expect(getActiveTaskCount()).toBe(1);
@@ -501,6 +501,14 @@ describe("command queue", () => {
       const err = await stuckResult;
       expect(err).toBeInstanceOf(CommandLaneTaskTimeoutError);
 
+      // Stale timeout should use debug, not warn.
+      expect(diagnosticMocks.diag.debug).toHaveBeenCalledWith(
+        expect.stringContaining("stale, post-reset"),
+      );
+      expect(diagnosticMocks.diag.warn).not.toHaveBeenCalledWith(
+        expect.stringContaining("lane task timed out:"),
+      );
+
       // Verify lane still works after all of this.
       const fresh = enqueueCommandInLane(lane, async () => "still works");
       await vi.advanceTimersByTimeAsync(10);
diff --git a/src/process/command-queue.ts b/src/process/command-queue.ts
@@ -41,8 +41,14 @@ export class CommandLaneTaskTimeoutError extends Error {
 // low-risk parallelism (e.g. cron jobs) without interleaving stdin / logs for
 // the main auto-reply workflow.
 
-/** Default task execution timeout (5 minutes). */
-const DEFAULT_TASK_TIMEOUT_MS = 5 * 60 * 1000;
+/**
+ * Default task execution timeout (15 minutes).  This is a last-resort
+ * safety net above the agent-level timeout (default 600s / 10 min in
+ * `src/agents/timeout.ts`).  Set high enough to avoid killing legitimate
+ * long-running tasks while still recovering stuck lanes within a
+ * reasonable window.
+ */
+const DEFAULT_TASK_TIMEOUT_MS = 15 * 60 * 1000;
 
 type QueueEntry = {
   task: () => Promise<unknown>;
@@ -166,12 +172,19 @@ function drainLane(lane: string) {
             if (timeoutHandle !== undefined) {
               clearTimeout(timeoutHandle);
             }
+            const activeBeforeComplete = state.activeTaskIds.size;
             const completedCurrentGeneration = completeTask(state, taskId, taskGeneration);
             const isProbeLane = lane.startsWith("auth-probe:") || lane.startsWith("session:probe-");
             if (err instanceof CommandLaneTaskTimeoutError) {
-              diag.warn(
-                `lane task timed out: lane=${lane} timeoutMs=${effectiveTimeout} durationMs=${Date.now() - startTime} active=${state.activeTaskIds.size} queued=${state.queue.length}`,
-              );
+              if (completedCurrentGeneration) {
+                diag.warn(
+                  `lane task timed out: lane=${lane} timeoutMs=${effectiveTimeout} durationMs=${Date.now() - startTime} active=${activeBeforeComplete} queued=${state.queue.length}`,
+                );
+              } else {
+                diag.debug(
+                  `lane task timed out (stale, post-reset): lane=${lane} timeoutMs=${effectiveTimeout} durationMs=${Date.now() - startTime}`,
+                );
+              }
             } else if (!isProbeLane) {
               diag.error(
                 `lane task error: lane=${lane} durationMs=${Date.now() - startTime} error="${String(err)}"`,
