fix(worker): launch the pre-PR repair agent detached so it survives invocation boundaries (#311)

Author: outof-place

apps/worker/src/pre-pr-checks/runner.test.ts

@@ -396,6 +396,114 @@ describe("runPrePrChecksWithFixes", () => {
     expect(checkRuns).toBe(1);
   });
 
+  it("launches the repair wrapper detached and reads its sentinel instead of holding the command open", async () => {
+    // A blocking launch keeps one sandbox ndjson stream open for the whole
+    // repair agent. Production runs whose pre-PR checks crossed a function
+    // invocation boundary lost that stream mid-flight, and the SDK's parse
+    // error was reported as a launch that never produced a process: empty
+    // output, empty logs, no exit code, for an agent that was in fact running.
+    // The launch has to return before the agent finishes, and completion has to
+    // come from the wrapper's sentinel file.
+    vi.useFakeTimers();
+    try {
+      let checkRuns = 0;
+      let sentinelReads = 0;
+      let sentinelReadsWhenLaunchReturned = -1;
+      mockRunCommand.mockImplementation((cmd, args) => {
+        const artifact = phaseArtifactCommand(cmd, args, "codex");
+        if (artifact) return artifact;
+        if (isWrapperLaunch(cmd)) {
+          sentinelReadsWhenLaunchReturned = sentinelReads;
+          // A detached command has not exited when runCommand resolves.
+          return detachedCommand();
+        }
+        if (isSentinelRead(cmd, args)) {
+          sentinelReads++;
+          return commandResult(sentinelReads >= 2 ? 0 : 1);
+        }
+        if (cmd === "cat" && args[0] === WORKSPACE_MANIFEST_PATH) {
+          return commandResult(0, JSON.stringify(manifest));
+        }
+        if (cmd === "git" && args[0] === "-C" && args[2] === "rev-parse") {
+          return commandResult(0, "web-head");
+        }
+        if (isConfiguredCheck(cmd)) {
+          checkRuns++;
+          return checkRuns === 1
+            ? commandResult(1, "", "Type error")
+            : commandResult(0, "ok");
+        }
+        return commandResult(0, "");
+      });
+
+      const pending = runPrePrChecksWithFixes(
+        "sbx-test-123",
+        { repositories: [config.repositories[0]!] },
+        "codex",
+        "gpt-5",
+      );
+      const result = await drainPollTicks(pending);
+
+      expect(result.passed).toBe(true);
+      expect(result.fixCycles).toBe(1);
+      expect(result.agentFailure).toBeUndefined();
+      expect(mockRunCommand).toHaveBeenCalledWith(
+        expect.objectContaining({
+          cmd: "bash",
+          args: ["/tmp/pre-pr-fix-1-wrapper.sh"],
+          cwd: "/vercel/sandbox",
+          detached: true,
+        }),
+      );
+      expect(mockRunCommand).toHaveBeenCalledWith("test", [
+        "-f",
+        "/tmp/pre-pr-fix-1-done",
+      ]);
+      // The launch resolved before any sentinel existed, and the phase only
+      // ended once a later poll found one: completion is driven by the poll,
+      // not by the launch call.
+      expect(sentinelReadsWhenLaunchReturned).toBe(0);
+      expect(sentinelReads).toBe(2);
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
+  it("propagates a deadline abort raised while the repair phase is polled", async () => {
+    // The deadline that used to abort the blocking launch now expires during
+    // the poll, and it must stay a real TimeoutError rather than become a
+    // "could not be launched" failure attributed to the provider.
+    let sentinelReads = 0;
+    mockRunCommand.mockImplementation((cmd, args) => {
+      const artifact = phaseArtifactCommand(cmd, args, "codex");
+      if (artifact) return artifact;
+      if (isWrapperLaunch(cmd)) return detachedCommand();
+      if (isSentinelRead(cmd, args)) {
+        sentinelReads++;
+        return commandResult(1);
+      }
+      if (cmd === "cat" && args[0] === WORKSPACE_MANIFEST_PATH) {
+        return commandResult(0, JSON.stringify(manifest));
+      }
+      if (isHeadInspection(cmd)) return commandResult(0, "web-head");
+      if (isConfiguredCheck(cmd)) return commandResult(1, "", "still failing");
+      return commandResult(0, "");
+    });
+
+    await expect(
+      runPrePrChecksWithFixes(
+        "sbx-test-123",
+        { repositories: [config.repositories[0]!] },
+        "codex",
+        "gpt-5",
+        3,
+        50,
+      ),
+    ).rejects.toMatchObject({ name: "TimeoutError" });
+
+    expect(sentinelReads).toBeGreaterThan(0);
+  });
+
   it("names the cause when the repair process cannot be launched at all", async () => {
     // Production runs died here with a failure that named only the boundary:
     // no exit code, no captured bytes, and the thrown error destroyed at the
@@ -744,6 +852,40 @@ function isConfiguredCheck(cmd: unknown): boolean {
   );
 }
 
+/** What a detached `runCommand` resolves to: the process is still running, so
+ *  there is no exit code yet. */
+function detachedCommand() {
+  return {
+    exitCode: null,
+    stdout: vi.fn().mockResolvedValue(""),
+    stderr: vi.fn().mockResolvedValue(""),
+  };
+}
+
+function isSentinelRead(cmd: unknown, args: unknown): boolean {
+  return cmd === "test" && Array.isArray(args) && args[0] === "-f";
+}
+
+/** Run the fake clock forward until the polled run settles, so a poll tick
+ *  costs the suite nothing. */
+async function drainPollTicks<T>(pending: Promise<T>): Promise<T> {
+  let settled = false;
+  const watched = pending.then(
+    (value) => {
+      settled = true;
+      return value;
+    },
+    (error) => {
+      settled = true;
+      throw error;
+    },
+  );
+  for (let tick = 0; tick < 20 && !settled; tick++) {
+    await vi.advanceTimersByTimeAsync(5_000);
+  }
+  return watched;
+}
+
 function isWrapperLaunch(cmd: unknown): boolean {
   const objectCommand = cmd as { cmd?: unknown; args?: unknown };
   return (

apps/worker/src/pre-pr-checks/runner.ts

@@ -1,4 +1,7 @@
-import type { Sandbox as SandboxType } from "@vercel/sandbox";
+import type {
+  Command as SandboxCommand,
+  Sandbox as SandboxType,
+} from "@vercel/sandbox";
 import { getSandboxCredentials } from "../sandbox/credentials.js";
 import {
   parseWorkspaceManifest,
@@ -118,6 +121,7 @@ export async function runPrePrChecksWithFixes(
   while (!result.passed && fixCycles < maxFixCycles) {
     fixCycles++;
     const fixer = await runFixAgent(
+      sandboxId,
       sandbox,
       result,
       agentKind,
@@ -261,7 +265,57 @@ async function hasRepositoryChanged(
   return !repo.preAgentSha || repo.preAgentSha !== headSha;
 }
 
+/** How often the detached Pre-PR repair wrapper's sentinel file is read. The
+ *  phase polls inside the caller's step rather than across workflow ticks, so
+ *  the tick is a plain sleep and can be far shorter than the 30s ceiling in
+ *  workflows/blocks/poll-phase.ts. */
+const PRE_PR_REPAIR_POLL_INTERVAL_MS = 5_000;
+
+/**
+ * Wait for the detached repair wrapper to touch its sentinel file.
+ *
+ * The only bound is the caller's deadline `signal`, which is what bounded the
+ * blocking runCommand this replaced: an expired deadline rejects with the
+ * signal's own reason, so a real AbortError/TimeoutError still propagates out
+ * of the repair phase instead of being reported as a failed launch. Returns
+ * "stopped" when the sandbox is gone, the same verdict checkPhaseDone gives
+ * every other polled agent phase.
+ */
+async function waitForRepairPhase(
+  sandboxId: string,
+  sentinelFile: string,
+  signal?: AbortSignal,
+): Promise<"done" | "stopped"> {
+  const { checkPhaseDone } = await import("../sandbox/poll-agent.js");
+  for (;;) {
+    signal?.throwIfAborted();
+    const status = await checkPhaseDone(sandboxId, sentinelFile);
+    if (status === true) return "done";
+    if (status === "stopped") return "stopped";
+    await sleepUntilNextPoll(PRE_PR_REPAIR_POLL_INTERVAL_MS, signal);
+  }
+}
+
+function sleepUntilNextPoll(ms: number, signal?: AbortSignal): Promise<void> {
+  return new Promise<void>((resolve, reject) => {
+    if (signal?.aborted) {
+      reject(signal.reason);
+      return;
+    }
+    const onAbort = () => {
+      clearTimeout(timer);
+      reject(signal?.reason);
+    };
+    const timer = setTimeout(() => {
+      signal?.removeEventListener("abort", onAbort);
+      resolve();
+    }, ms);
+    signal?.addEventListener("abort", onAbort, { once: true });
+  });
+}
+
 async function runFixAgent(
+  sandboxId: string,
   sandbox: SandboxSession,
   failedRun: PrePrCheckRunResult,
   agentKind: "claude" | "codex",
@@ -387,12 +441,21 @@ async function runFixAgent(
     if (failure.ok) throw new Error("unreachable");
     return { usage: null, failure };
   }
-  let launch: SandboxCommandResult;
+  let launch: SandboxCommand;
   try {
+    // Detached, like every other agent phase (see writeAndStartPhase in
+    // workflows/agent.ts): the sandbox SDK keeps one ndjson stream open for the
+    // whole of a blocking runCommand, and a repair agent outlives the function
+    // invocation that started it. When that invocation ends mid-stream the SDK
+    // raises a parse/stream error rather than an abort, which reached the
+    // generic catch below and reported a launch failure with no exit code and
+    // no bytes for an agent that was in fact running. A detached launch returns
+    // at once and completion is read from the wrapper's sentinel file instead.
     launch = await sandbox.runCommand({
       cmd: "bash",
       args: [paths.wrapper],
       cwd: "/vercel/sandbox",
+      detached: true,
       ...(signal ? { signal } : {}),
     });
   } catch (error) {
@@ -438,7 +501,9 @@ async function runFixAgent(
     if (failure.ok) throw new Error("unreachable");
     return { usage: null, failure };
   }
-  if (launch.exitCode !== 0) {
+  // A detached launch has no exit code yet while the wrapper runs; only a
+  // wrapper that already exited non-zero is a launch that failed.
+  if (launch.exitCode !== null && launch.exitCode !== 0) {
     const { commandProtocolFailure } = await import("../sandbox/agents/protocol.js");
     return {
       usage: null,
@@ -452,6 +517,21 @@ async function runFixAgent(
       }),
     };
   }
+  const phaseStatus = await waitForRepairPhase(sandboxId, paths.sentinel, signal);
+  if (phaseStatus === "stopped") {
+    const { protocolFailure } = await import("../sandbox/agents/protocol.js");
+    const failure = protocolFailure({
+      spec: adapter.cliSpec,
+      phase,
+      artifacts: { stdout: "", stderr: "", structuredOutput: null, exitCode: null },
+      failureKind: "provider_error",
+      category: "provider",
+      message: "The current agent phase could not be completed.",
+      detail: "The sandbox stopped before the Pre-PR repair process finished.",
+    });
+    if (failure.ok) throw new Error("unreachable");
+    return { usage: null, failure };
+  }
   const artifacts = await collectPhaseFromSandbox(sandbox, paths);
   const usage = adapter.extractUsage(artifacts.stdout, artifacts.structuredOutput);
   const protocol = adapter.validateFreeformProtocol(artifacts, phase);