From 8e384947aaa53717c724aaf0b0ec5a10c977f600 Mon Sep 17 00:00:00 2001
From: Devin Foley <devin@devinfoley.com>
Date: Thu, 26 Mar 2026 23:48:58 -0700
Subject: [PATCH] Address Greptile review: consolidate query, add idle tests,
 remove dead code

- Consolidate duplicate running-runs query in reapOrphanedRuns by
  reusing activeRuns for idle timeout pass (skip already-reaped runs)
- Add three integration tests: idle warning at 11 min, idle kill at
  16 min, and no-warning with recent output
- Remove unreachable idle_timeout entry from statusBadge (idle-killed
  runs have status "failed", not "idle_timeout")

Co-Authored-By: Paperclip <noreply@paperclip.ing>
---
 .../heartbeat-process-recovery.test.ts        | 52 +++++++++++++++++++
 server/src/services/heartbeat.ts              | 15 ++----
 ui/src/lib/status-colors.ts                   |  1 -
 3 files changed, 56 insertions(+), 12 deletions(-)

diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts
index 75857cde..b5412f5d 100644
--- a/server/src/__tests__/heartbeat-process-recovery.test.ts
+++ b/server/src/__tests__/heartbeat-process-recovery.test.ts
@@ -240,6 +240,58 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
     expect(issue?.checkoutRunId).toBe(runId);
   });
 
+  it("sets idle_warning when a run has no output for over 10 minutes", async () => {
+    const elevenMinutesAgo = new Date(Date.now() - 11 * 60 * 1000);
+    const { runId } = await seedRunFixture({
+      includeIssue: false,
+      startedAt: elevenMinutesAgo,
+      lastOutputAt: elevenMinutesAgo,
+    });
+    const heartbeat = heartbeatService(db);
+
+    const result = await heartbeat.reapOrphanedRuns();
+    expect(result.idleWarned).toBe(1);
+    expect(result.idleKilled).toBe(0);
+
+    const run = await heartbeat.getRun(runId);
+    expect(run?.status).toBe("running");
+    expect(run?.errorCode).toBe("idle_warning");
+  });
+
+  it("kills a run that has been idle for over 15 minutes", async () => {
+    const sixteenMinutesAgo = new Date(Date.now() - 16 * 60 * 1000);
+    const { runId } = await seedRunFixture({
+      processPid: 999_999_999,
+      startedAt: sixteenMinutesAgo,
+      lastOutputAt: sixteenMinutesAgo,
+    });
+    const heartbeat = heartbeatService(db);
+
+    const result = await heartbeat.reapOrphanedRuns();
+    expect(result.idleKilled).toBe(1);
+
+    const run = await heartbeat.getRun(runId);
+    expect(run?.status).toBe("failed");
+    expect(run?.errorCode).toBe("idle_timeout");
+  });
+
+  it("does not idle-warn a run with recent output", async () => {
+    const { runId } = await seedRunFixture({
+      includeIssue: false,
+      startedAt: new Date(Date.now() - 20 * 60 * 1000),
+      lastOutputAt: new Date(),
+    });
+    const heartbeat = heartbeatService(db);
+
+    const result = await heartbeat.reapOrphanedRuns();
+    expect(result.idleWarned).toBe(0);
+    expect(result.idleKilled).toBe(0);
+
+    const run = await heartbeat.getRun(runId);
+    expect(run?.status).toBe("running");
+    expect(run?.errorCode).toBeNull();
+  });
+
   it("clears the detached warning when the run reports activity again", async () => {
     const { runId } = await seedRunFixture({
       includeIssue: false,
diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts
index 57660339..996a724b 100644
--- a/server/src/services/heartbeat.ts
+++ b/server/src/services/heartbeat.ts
@@ -1837,20 +1837,13 @@ export function heartbeatService(db: Db) {
       logger.warn({ reapedCount: reaped.length, runIds: reaped }, "reaped orphaned heartbeat runs");
     }
 
-    // ── Idle-timeout pass: check all running runs for stalled output ──
+    // ── Idle-timeout pass: reuse activeRuns query result (no duplicate DB call) ──
     const idleWarned: string[] = [];
     const idleKilled: string[] = [];
 
-    const allRunningRuns = await db
-      .select({
-        run: heartbeatRuns,
-        adapterType: agents.adapterType,
-      })
-      .from(heartbeatRuns)
-      .innerJoin(agents, eq(heartbeatRuns.agentId, agents.id))
-      .where(eq(heartbeatRuns.status, "running"));
-
-    for (const { run, adapterType } of allRunningRuns) {
+    for (const { run, adapterType } of activeRuns) {
+      // Idle check applies to all running runs, including tracked ones
+      if (reaped.includes(run.id)) continue;
       const tracksLocalChild = isTrackedLocalChildProcessAdapter(adapterType);
       if (!tracksLocalChild) continue;
 
diff --git a/ui/src/lib/status-colors.ts b/ui/src/lib/status-colors.ts
index 4a2ac984..beee561a 100644
--- a/ui/src/lib/status-colors.ts
+++ b/ui/src/lib/status-colors.ts
@@ -55,7 +55,6 @@ export const statusBadge: Record<string, string> = {
   // Run statuses
   failed: "bg-red-100 text-red-700 dark:bg-red-900/50 dark:text-red-300",
   timed_out: "bg-orange-100 text-orange-700 dark:bg-orange-900/50 dark:text-orange-300",
-  idle_timeout: "bg-orange-100 text-orange-700 dark:bg-orange-900/50 dark:text-orange-300",
   succeeded: "bg-green-100 text-green-700 dark:bg-green-900/50 dark:text-green-300",
   error: "bg-red-100 text-red-700 dark:bg-red-900/50 dark:text-red-300",
   terminated: "bg-red-100 text-red-700 dark:bg-red-900/50 dark:text-red-300",