fix(overwatch): Fix when a coding agent hangs for a long time

overwatch/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@bpinternal/overwatch",
-  "version": "0.3.6",
+  "version": "0.3.7",
   "description": "Headless library for building control loops: scheduled bots that scan a repo for a class of anomaly, fix a few instances with a coding agent, and open a PR — no server, UI, or persistent process.",
   "keywords": [
     "control-loop",

overwatch/src/agents.test.ts

@@ -0,0 +1,86 @@
+import { describe, expect, test } from "vitest";
+import { Claude, Codex, type AgentContext } from "./agents";
+
+type ExecCall = { command: string; env?: Record<string, string>; timeoutSec?: number };
+
+/**
+ * AgentContext stand-in that records every exec. `logOutput` stands in for what the
+ * agent left in the redirect file, so a test can tell the log apart from whatever the
+ * exec itself reported.
+ */
+function fakeCtx(over: { exitCode?: number; logOutput?: string; logExitCode?: number } = {}): {
+  ctx: AgentContext;
+  calls: ExecCall[];
+  files: Map<string, string>;
+} {
+  const calls: ExecCall[] = [];
+  const files = new Map<string, string>();
+  const ctx: AgentContext = {
+    exec: async (command, options) => {
+      calls.push({ command, env: options?.env, timeoutSec: options?.timeoutSec });
+      if (command.startsWith("cat ")) {
+        return { exitCode: over.logExitCode ?? 0, output: over.logOutput ?? "" };
+      }
+      return { exitCode: over.exitCode ?? 0, output: "exec-side output" };
+    },
+    writeFile: async (path, content) => {
+      files.set(path, content);
+    },
+  };
+  return { ctx, calls, files };
+}
+
+const agentCall = (calls: ExecCall[]): ExecCall => calls.find((c) => !c.command.startsWith("cat "))!;
+
+describe.each([
+  {
+    name: "Claude",
+    make: () => new Claude({ apiKey: "k", timeoutSec: 120 }),
+    cli: "claude -p",
+    envKey: "ANTHROPIC_API_KEY",
+  },
+  {
+    name: "Codex",
+    make: () => new Codex({ apiKey: "k", timeoutSec: 120 }),
+    cli: "codex exec",
+    envKey: "OPENAI_API_KEY",
+  },
+])("$name.executeAgent", ({ make, cli, envKey }) => {
+  test("detaches all three streams from the exec's pipes", async () => {
+    const { ctx, calls } = fakeCtx();
+    await make().executeAgent("fix it", ctx);
+    const command = agentCall(calls).command;
+    expect(command).toContain(cli);
+    expect(command).toContain("</dev/null");
+    expect(command).toContain(">.control-loop/agent.log 2>&1");
+  });
+
+  test("stages the prompt rather than passing it as a shell argument", async () => {
+    const { ctx, files } = fakeCtx();
+    await make().executeAgent("fix it; rm -rf /", ctx);
+    expect(files.get(".control-loop/prompt.txt")).toBe("fix it; rm -rf /");
+  });
+
+  test("passes the api key and timeout to the agent command", async () => {
+    const { ctx, calls } = fakeCtx();
+    await make().executeAgent("fix it", ctx);
+    const call = agentCall(calls);
+    expect(call.env?.[envKey]).toBe("k");
+    expect(call.timeoutSec).toBe(120);
+  });
+
+  test("reports the redirected log when the agent fails", async () => {
+    const { ctx } = fakeCtx({ exitCode: 2, logOutput: "agent stack trace" });
+    await expect(make().executeAgent("fix it", ctx)).rejects.toThrow(/exited 2: agent stack trace/);
+  });
+
+  test("falls back to the exec's own output when the log is missing", async () => {
+    const { ctx } = fakeCtx({ exitCode: 2, logExitCode: 1 });
+    await expect(make().executeAgent("fix it", ctx)).rejects.toThrow(/exited 2: exec-side output/);
+  });
+
+  test("resolves without reading the log as an error when the agent succeeds", async () => {
+    const { ctx } = fakeCtx({ exitCode: 0, logOutput: "all good" });
+    await expect(make().executeAgent("fix it", ctx)).resolves.toBeUndefined();
+  });
+});

overwatch/src/agents.ts

@@ -26,6 +26,7 @@ export abstract class Agent {
 }
 
 const PROMPT_FILE = ".control-loop/prompt.txt";
+const LOG_FILE = ".control-loop/agent.log";
 
 /**
  * Writes the prompt to a scratch file and returns a shell fragment that expands to its
@@ -37,6 +38,33 @@ async function stagePrompt(instructions: string, ctx: AgentContext): Promise<str
   return `"$(cat ${PROMPT_FILE})"`;
 }
 
+/**
+ * Runs an agent CLI with all three standard streams detached from the exec's own pipes,
+ * then reads the output back from the file.
+ *
+ * The sandbox reads a command's output until EOF, and EOF arrives only once every process
+ * holding the write end has exited — not just the one that was launched. Agents routinely
+ * leave descendants behind (a package-manager daemon, a server started to check something),
+ * and those inherit the pipe and hold the exec open long after the agent has finished its
+ * work and exited. Redirecting to a file leaves the shell as the pipe's only owner, so the
+ * command ends when the agent does.
+ *
+ * `</dev/null` is part of the same story from the other end: it stops a CLI that decides to
+ * consult stdin (an approval prompt, a login it thinks is expired) from blocking on input
+ * that is never coming.
+ */
+async function runAgentCli(
+  command: string,
+  ctx: AgentContext,
+  options: { env?: Record<string, string>; timeoutSec?: number },
+): Promise<{ exitCode: number; output: string }> {
+  const run = await ctx.exec(`${command} </dev/null >${LOG_FILE} 2>&1`, options);
+  const log = await ctx.exec(`cat ${LOG_FILE}`);
+  // A missing log means the redirect itself never happened; the exec's own output is
+  // then the only account of what went wrong.
+  return { exitCode: run.exitCode, output: log.exitCode === 0 ? log.output : run.output };
+}
+
 export type ClaudeProps = {
   apiKey: string;
   /** e.g. "claude-sonnet-5". Defaults to the CLI's default model. */
@@ -62,7 +90,7 @@ export class Claude extends Agent {
   override async executeAgent(instructions: string, ctx: AgentContext): Promise<void> {
     const prompt = await stagePrompt(instructions, ctx);
     const model = this.props.model ? ` --model ${this.props.model}` : "";
-    const result = await ctx.exec(`claude -p ${prompt}${model} --dangerously-skip-permissions`, {
+    const result = await runAgentCli(`claude -p ${prompt}${model} --dangerously-skip-permissions`, ctx, {
       env: { ANTHROPIC_API_KEY: this.props.apiKey },
       timeoutSec: this.props.timeoutSec ?? 900,
     });
@@ -113,10 +141,10 @@ export class Codex extends Agent {
     const model = this.props.model ? ` --model ${this.props.model}` : "";
     // Codex's own sandbox (--sandbox workspace-write) relies on Landlock/seccomp, which
     // fails inside containers — and the Daytona sandbox already isolates everything, so
-    // bypassing it is what Codex documents for CI/container use. </dev/null stops
-    // `codex exec` from waiting on extra stdin input.
-    const result = await ctx.exec(
-      `codex exec ${prompt}${model} --dangerously-bypass-approvals-and-sandbox --skip-git-repo-check </dev/null`,
+    // bypassing it is what Codex documents for CI/container use.
+    const result = await runAgentCli(
+      `codex exec ${prompt}${model} --dangerously-bypass-approvals-and-sandbox --skip-git-repo-check`,
+      ctx,
       {
         env: { OPENAI_API_KEY: this.props.apiKey },
         timeoutSec: this.props.timeoutSec ?? 900,

overwatch/src/control-loop.ts

@@ -17,6 +17,7 @@ import {
   memoryNote,
   readMemory,
   runConfiguredCommand,
+  withDeadline,
 } from "./sandbox";
 import type { Sensor } from "./sensors";
 import type {
@@ -450,11 +451,11 @@ async function createSandbox(config: ControlLoopConfig): Promise<Sandbox> {
 function makeAgentContext(sandbox: Sandbox): AgentContext {
   return {
     exec: async (command, options) => {
-      const response = await sandbox.process.executeCommand(
+      const timeoutSec = options?.timeoutSec ?? 300;
+      const response = await withDeadline(
+        sandbox.process.executeCommand(command, REPO_PATH, options?.env, timeoutSec),
+        timeoutSec,
         command,
-        REPO_PATH,
-        options?.env,
-        options?.timeoutSec ?? 300,
       );
       return { exitCode: response.exitCode, output: response.result };
     },

overwatch/src/sandbox.test.ts

@@ -1,11 +1,12 @@
-import { describe, expect, test } from "vitest";
+import { afterEach, describe, expect, test, vi } from "vitest";
 import {
   appendMemory,
   commitAll,
   memoryNote,
   memoryPath,
   readMemory,
   runConfiguredCommand,
+  withDeadline,
 } from "./sandbox";
 import type { RunLog } from "./log";
 import type { AgentContext } from "./agents";
@@ -37,6 +38,53 @@ function fakeCtx(execImpl?: (command: string) => { exitCode: number; output: str
 const silentLog = () =>
   ({ step: async <T>(_label: string, fn: () => Promise<T>) => fn() }) as unknown as RunLog;
 
+describe("withDeadline", () => {
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
+  test("passes through a result that arrives in time", async () => {
+    await expect(withDeadline(Promise.resolve("ok"), 900, "ls")).resolves.toBe("ok");
+  });
+
+  test("passes through a rejection instead of waiting for the deadline", async () => {
+    await expect(withDeadline(Promise.reject(new Error("boom")), 900, "ls")).rejects.toThrow("boom");
+  });
+
+  test("rejects with the command once the grace window past the timeout elapses", async () => {
+    vi.useFakeTimers();
+    const hung = withDeadline(new Promise(() => {}), 900, "codex exec");
+    const asserted = expect(hung).rejects.toThrow(/no response within 960s: codex exec/);
+    await vi.advanceTimersByTimeAsync(960_000);
+    await asserted;
+  });
+
+  test("holds on past the command's own timeout, leaving room for a server-side error", async () => {
+    vi.useFakeTimers();
+    let settled = false;
+    const hung = withDeadline(new Promise(() => {}), 900, "codex exec").catch(() => {
+      settled = true;
+    });
+    await vi.advanceTimersByTimeAsync(900_000);
+    expect(settled).toBe(false);
+    await vi.advanceTimersByTimeAsync(60_000);
+    await hung;
+    expect(settled).toBe(true);
+  });
+
+  test("treats a non-positive timeout as no timeout", async () => {
+    vi.useFakeTimers();
+    let settled = false;
+    const forever = withDeadline(new Promise(() => {}), 0, "sleep").then(() => {
+      settled = true;
+    });
+    await vi.advanceTimersByTimeAsync(3_600_000);
+    expect(settled).toBe(false);
+    expect(vi.getTimerCount()).toBe(0);
+    void forever;
+  });
+});
+
 describe("memoryPath", () => {
   test("namespaces the memory file by loop label", () => {
     expect(memoryPath("my-loop")).toBe(".github/control-loop/memory-my-loop.md");

overwatch/src/sandbox.ts

@@ -30,6 +30,39 @@ function shellQuote(value: string): string {
   return `'${value.replace(/'/g, `'\\''`)}'`;
 }
 
+/**
+ * Extra seconds allowed past a command's own timeout before the client gives up on it.
+ * Wide enough that a server-side timeout, which produces a real error mentioning the
+ * command's output, always wins the race over this comparatively blind one.
+ */
+const DEADLINE_GRACE_SEC = 60;
+
+/**
+ * Arms a client-side deadline on a sandbox command.
+ *
+ * The sandbox SDK sends `timeout` as a field in the request body and never arms one
+ * locally — no axios timeout, no abort signal — so the promise settles only when the
+ * server sends a response. When it doesn't (a connection dropped mid-command, a process
+ * that never releases the exec) the run hangs indefinitely with no way back. Racing the
+ * call against a timer turns that into an ordinary failure the loop can report.
+ *
+ * A non-positive `timeoutSec` means "no timeout" and is passed through untouched.
+ */
+export function withDeadline<T>(work: Promise<T>, timeoutSec: number, command: string): Promise<T> {
+  if (timeoutSec <= 0) return work;
+  const limit = timeoutSec + DEADLINE_GRACE_SEC;
+  let timer: ReturnType<typeof setTimeout> | undefined;
+  const deadline = new Promise<never>((_resolve, reject) => {
+    timer = setTimeout(() => {
+      reject(new Error(`command produced no response within ${limit}s: ${command}`));
+    }, limit * 1000);
+  });
+  // The losing promise keeps running; unref'ing the timer keeps a pending deadline from
+  // holding the process open once the run itself is done.
+  timer?.unref?.();
+  return Promise.race([work, deadline]).finally(() => clearTimeout(timer));
+}
+
 /**
  * Runs one of the configured lifecycle hooks (`config.hooks`) from the repo root,
  * as a logged step. A missing hook is a no-op; a non-zero exit aborts the run.