Skip to content

Commit b556fd2

Browse files
authored
fix(worker): carry the real cause when the pre-PR repair agent fails to launch (#309)
1 parent 4dab584 commit b556fd2

2 files changed

Lines changed: 123 additions & 4 deletions

File tree

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

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ vi.mock("../lib/logger.js", () => ({
2525
logger: {
2626
info: vi.fn(),
2727
warn: vi.fn(),
28+
error: vi.fn(),
2829
},
2930
}));
3031

@@ -386,11 +387,88 @@ describe("runPrePrChecksWithFixes", () => {
386387
expect(result.fixCycles).toBe(1);
387388
expect(result.agentFailure).toMatchObject({
388389
category: "provider",
389-
diagnostic: { failureKind: "cli_exit", exitCode: 7 },
390+
diagnostic: {
391+
failureKind: "cli_exit",
392+
exitCode: 7,
393+
detail: "The CLI exited with code 7.",
394+
},
390395
});
391396
expect(checkRuns).toBe(1);
392397
});
393398

399+
it("names the cause when the repair process cannot be launched at all", async () => {
400+
// Production runs died here with a failure that named only the boundary:
401+
// no exit code, no captured bytes, and the thrown error destroyed at the
402+
// catch. Every distinct launch cause has to reach the run record instead.
403+
const launchWith = (thrown: unknown) => {
404+
mockRunCommand.mockImplementation((cmd, args) => {
405+
if (isWrapperLaunch(cmd)) throw thrown;
406+
if (cmd === "cat" && args[0] === WORKSPACE_MANIFEST_PATH) {
407+
return commandResult(0, JSON.stringify(manifest));
408+
}
409+
if (cmd === "git" && args[0] === "-C" && args[2] === "rev-parse") {
410+
return commandResult(0, "web-head");
411+
}
412+
if (isConfiguredCheck(cmd)) return commandResult(1, "", "still failing");
413+
return commandResult(0, "");
414+
});
415+
return runPrePrChecksWithFixes(
416+
"sbx-test-123",
417+
{ repositories: [config.repositories[0]!] },
418+
"codex",
419+
"gpt-5",
420+
);
421+
};
422+
423+
const reset = await launchWith(new Error("sandbox connection reset"));
424+
expect(reset.agentFailure).toMatchObject({
425+
diagnostic: {
426+
failureKind: "setup_failed",
427+
detail: expect.stringContaining("sandbox connection reset"),
428+
},
429+
});
430+
431+
const refused = await launchWith(
432+
Object.assign(new Error("connect ECONNREFUSED 10.0.0.1:443"), {
433+
code: "ECONNREFUSED",
434+
}),
435+
);
436+
expect(refused.agentFailure?.diagnostic.detail).toContain(
437+
"ECONNREFUSED: connect ECONNREFUSED 10.0.0.1:443",
438+
);
439+
});
440+
441+
it("bounds a very long launch failure cause instead of embedding it whole", async () => {
442+
const thrownMessage = "sandbox refused the launch. ".repeat(200);
443+
mockRunCommand.mockImplementation((cmd, args) => {
444+
if (isWrapperLaunch(cmd)) throw new Error(thrownMessage);
445+
if (cmd === "cat" && args[0] === WORKSPACE_MANIFEST_PATH) {
446+
return commandResult(0, JSON.stringify(manifest));
447+
}
448+
if (cmd === "git" && args[0] === "-C" && args[2] === "rev-parse") {
449+
return commandResult(0, "web-head");
450+
}
451+
if (isConfiguredCheck(cmd)) return commandResult(1, "", "still failing");
452+
return commandResult(0, "");
453+
});
454+
455+
const result = await runPrePrChecksWithFixes(
456+
"sbx-test-123",
457+
{ repositories: [config.repositories[0]!] },
458+
"codex",
459+
"gpt-5",
460+
);
461+
462+
const detail = result.agentFailure?.diagnostic.detail ?? "";
463+
expect(detail).toContain("The Pre-PR repair process could not be launched:");
464+
expect(detail).not.toContain(thrownMessage);
465+
// Sentence plus the bound the repair cause is clamped to, and nothing more,
466+
// so a runaway error text cannot become the run status.
467+
expect(detail.length).toBeLessThanOrEqual(
468+
"The Pre-PR repair process could not be launched: ".length + 200,
469+
);
470+
});
471+
394472
it("keeps valid repair usage when malformed protocol output becomes an execution failure", async () => {
395473
let checkRuns = 0;
396474
const malformedWithUsage = [
@@ -666,6 +744,18 @@ function isConfiguredCheck(cmd: unknown): boolean {
666744
);
667745
}
668746

747+
function isWrapperLaunch(cmd: unknown): boolean {
748+
const objectCommand = cmd as { cmd?: unknown; args?: unknown };
749+
return (
750+
typeof cmd === "object" &&
751+
cmd !== null &&
752+
objectCommand.cmd === "bash" &&
753+
Array.isArray(objectCommand.args) &&
754+
typeof objectCommand.args[0] === "string" &&
755+
objectCommand.args[0].endsWith("-wrapper.sh")
756+
);
757+
}
758+
669759
function isHeadInspection(cmd: unknown): boolean {
670760
const objectCommand = cmd as { cmd?: unknown; args?: unknown };
671761
return (

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

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ import {
2626

2727
export const MAX_PRE_PR_FIX_CYCLES = 3;
2828

29+
/** Longest launch cause carried into the Pre-PR repair failure detail. Same
30+
* bound the workspace gate puts on a carried inspection reason (AIW-223): long
31+
* enough for a connection or spawn verdict, short enough that the composed
32+
* sentence stays a failure detail rather than a payload. */
33+
const PRE_PR_LAUNCH_CAUSE_MAX_LENGTH = 200;
34+
2935
export interface PrePrCheckFailure {
3036
provider: WorkspaceRepo["provider"];
3137
repoPath: string;
@@ -396,15 +402,38 @@ async function runFixAgent(
396402
) {
397403
throw error;
398404
}
399-
const { protocolFailure } = await import("../sandbox/agents/protocol.js");
405+
const { protocolFailure, redactDiagnosticText } = await import(
406+
"../sandbox/agents/protocol.js"
407+
);
408+
const { logger } = await import("../lib/logger.js");
409+
// The thrown error used to be discarded here, so a launch that never
410+
// produced a process left no exit code, no bytes and no cause: the only
411+
// reachable text named the boundary. Carry the reason, redacted and bounded
412+
// exactly like a diagnostic tail so a runaway error text cannot become the
413+
// run status. The kind is `setup_failed`, not `provider_error`: nothing was
414+
// sent to a provider, this is the same "the phase never started" family as
415+
// the chmod failure above, and calling it a provider error is what made
416+
// every occurrence read as spent provider credits.
417+
const code = (error as { code?: unknown }).code;
418+
const label =
419+
typeof code === "string" && code
420+
? code
421+
: error instanceof Error
422+
? error.name
423+
: "";
424+
const message = error instanceof Error ? error.message : String(error);
425+
const cause = redactDiagnosticText(
426+
label && label !== "Error" ? `${label}: ${message}` : message,
427+
).slice(0, PRE_PR_LAUNCH_CAUSE_MAX_LENGTH);
428+
logger.error({ phase, cause }, "pre_pr_repair_launch_failed");
400429
const failure = protocolFailure({
401430
spec: adapter.cliSpec,
402431
phase,
403432
artifacts: { stdout: "", stderr: "", structuredOutput: null, exitCode: null },
404-
failureKind: "provider_error",
433+
failureKind: "setup_failed",
405434
category: "provider",
406435
message: "The current agent phase could not be completed.",
407-
detail: "The Pre-PR repair process could not be launched.",
436+
detail: `The Pre-PR repair process could not be launched: ${cause}`,
408437
});
409438
if (failure.ok) throw new Error("unreachable");
410439
return { usage: null, failure };

0 commit comments

Comments
 (0)