From 665408c6d015d5e8b9fe2758541d28cb3debaa8b Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 24 Jul 2026 16:40:47 -0500 Subject: [PATCH] fix(codex): classify mid-turn harness crashes structurally as retriable infra (#10210) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source control plane people use to manage AI agents and their work > - Heartbeat execution relies on adapters distinguishing agent failures from failures in the harness running beneath the agent > - Codex MCP transport crashes can kill the CLI after the JSONL protocol has started but before it emits a protocol-terminal event > - Those interrupted streams were left unclassified, so the control plane terminalized the heartbeat as `heartbeat_failed` / `agent_failure` with no continuation > - Agent-level failure is already expressible through the JSONL protocol via an `error` event, `turn.failed`, or `turn.completed`, so an interrupted nonzero exit can be classified structurally without inspecting unstable error strings > - This pull request reports that shape as `codex_harness_crash` in the `transient_upstream` family and routes it through Paperclip's existing bounded retry and recovery-continuation paths > - The benefit is that transient Codex harness failures recover safely without misclassifying quoted agent output or depending on transport-specific wording ## Linked Issues or Issue Description - **What happened:** Codex MCP transport failures, including rmcp worker death, could terminate the CLI mid-turn after protocol output began but before any terminal JSONL event. The run then became an unclassified terminal heartbeat failure with `continuationCount: 0`; this occurred in 3 of 44 L3 Codex-lane trials during the associated benchmark investigation. - **Expected behavior:** a nonzero Codex exit after the protocol starts but before an `error`, `turn.failed`, or `turn.completed` event should be treated as a harness/infrastructure crash and enter the existing bounded retry policy. - **Why structural classification:** transport error strings vary, and stdout may quote agent output that merely discusses network failures. The protocol boundary identifies whether the agent itself produced a terminal result without regex matching. - **Recovery behavior:** `codex_harness_crash` maps to `errorFamily: transient_upstream`, using the existing `same_session` → `safer_invocation` → `fresh_session` ladder plus the recovery-continuation transient-infrastructure path. - Supersedes the regex-based approach in #10150, which is closed. ## What Changed - Added protocol-state tracking that identifies a nonzero exit after protocol start and before any protocol-terminal event as `codex_harness_crash`. - Propagated the structural classification as `transient_upstream` through the Codex adapter. - Added parse unit coverage, including a faithful crash-shaped stream, without matching stderr transport strings. - Added adapter execution coverage using a fake Codex process that emits a protocol prefix and then dies with the observed rmcp stderr line. - Added heartbeat bounded-retry coverage, including the `errorCode`-only fallback, and recovery-continuation classification coverage. ## Verification - `parse.test.ts` — 16 passed. - `codex-local-execute.test.ts` — 16 passed. - `heartbeat-retry-scheduling.test.ts` — 30 passed. - `service.pause-durability.test.ts` — 6 passed. - Server and Codex adapter TypeScript checks passed. - The branch commit is unchanged from the tested and pushed `88f5464d40` handoff. ## Risks - Low risk: the classification requires a nonzero exit after protocol start and before any protocol-terminal event, so normal agent-declared failures and completed turns keep their existing behavior. - The change intentionally broadens recovery for structurally interrupted Codex runs; bounded retry limits still prevent indefinite continuation loops. - No schema, migration, public API, UI, lockfile, or workflow changes. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex coding agent. The exact runtime model ID and context-window size were not exposed by the execution environment; capabilities used for the implementation included repository analysis, reasoning, code editing, and terminal-based test execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [ ] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details — the pre-existing, already-pushed branch name was explicitly prescribed for this replacement PR - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- .../codex-local/src/server/execute.ts | 16 +++- .../adapters/codex-local/src/server/index.ts | 2 +- .../codex-local/src/server/parse.test.ts | 73 +++++++++++++++++++ .../adapters/codex-local/src/server/parse.ts | 27 +++++++ .../src/__tests__/codex-local-execute.test.ts | 62 ++++++++++++++++ .../heartbeat-retry-scheduling.test.ts | 60 +++++++++++++++ server/src/services/heartbeat.ts | 6 +- .../recovery/service.pause-durability.test.ts | 6 ++ server/src/services/recovery/service.ts | 1 + 9 files changed, 250 insertions(+), 3 deletions(-) diff --git a/packages/adapters/codex-local/src/server/execute.ts b/packages/adapters/codex-local/src/server/execute.ts index b2723ca0bd..7284c4dc92 100644 --- a/packages/adapters/codex-local/src/server/execute.ts +++ b/packages/adapters/codex-local/src/server/execute.ts @@ -53,6 +53,7 @@ import { parseCodexJsonl, classifyCodexAuthRefreshFailure, extractCodexRetryNotBefore, + isCodexHarnessCrash, isCodexProviderQuotaError, isCodexTransientUpstreamError, isCodexUnknownSessionError, @@ -1283,7 +1284,18 @@ export async function execute(ctx: AdapterExecutionContext): Promise { }, usageBasis: "per_run", errorMessage: "resume failed", + sawProtocolEvent: true, + sawProtocolTerminalEvent: true, }); }); @@ -67,10 +70,80 @@ describe("parseCodexJsonl", () => { }, usageBasis: "per_run", errorMessage: null, + sawProtocolEvent: true, + sawProtocolTerminalEvent: true, }); }); }); +describe("isCodexHarnessCrash", () => { + const crashedMidTurnStream = [ + JSON.stringify({ type: "thread.started", thread_id: "thread_123" }), + JSON.stringify({ + type: "item.completed", + item: { type: "agent_message", text: "Checking out the issue now." }, + }), + JSON.stringify({ type: "item.started", item: { type: "command_execution" } }), + ].join("\n"); + + it("classifies a nonzero exit with no protocol-terminal event as a harness crash", () => { + const parsed = parseCodexJsonl(crashedMidTurnStream); + expect(parsed.sawProtocolEvent).toBe(true); + expect(parsed.sawProtocolTerminalEvent).toBe(false); + expect(isCodexHarnessCrash({ exitCode: 1, ...parsed })).toBe(true); + }); + + it("does not classify runs whose turn reached a protocol-terminal event", () => { + const failedInProtocol = parseCodexJsonl( + [ + JSON.stringify({ type: "thread.started", thread_id: "thread_123" }), + JSON.stringify({ type: "turn.failed", error: { message: "the model rejected the request" } }), + ].join("\n"), + ); + expect(isCodexHarnessCrash({ exitCode: 1, ...failedInProtocol })).toBe(false); + + const completedThenFailedExit = parseCodexJsonl( + [ + JSON.stringify({ type: "thread.started", thread_id: "thread_123" }), + JSON.stringify({ + type: "turn.completed", + usage: { input_tokens: 10, cached_input_tokens: 2, output_tokens: 4 }, + }), + ].join("\n"), + ); + expect(isCodexHarnessCrash({ exitCode: 1, ...completedThenFailedExit })).toBe(false); + }); + + it("does not classify successful exits or streams that never spoke the protocol", () => { + expect(isCodexHarnessCrash({ exitCode: 0, ...parseCodexJsonl(crashedMidTurnStream) })).toBe(false); + expect(isCodexHarnessCrash({ exitCode: null, ...parseCodexJsonl(crashedMidTurnStream) })).toBe(false); + + const neverStarted = parseCodexJsonl("error: unexpected argument '--bogus-flag'\n"); + expect(neverStarted.sawProtocolEvent).toBe(false); + expect(isCodexHarnessCrash({ exitCode: 2, ...neverStarted })).toBe(false); + }); + + it("stays structural: agent output discussing network errors does not affect classification", () => { + const parsed = parseCodexJsonl( + [ + JSON.stringify({ type: "thread.started", thread_id: "thread_123" }), + JSON.stringify({ + type: "item.completed", + item: { type: "agent_message", text: "The deploy failed with connection reset by peer; investigating." }, + }), + JSON.stringify({ type: "turn.failed", error: { message: "agent gave up" } }), + ].join("\n"), + ); + expect(isCodexHarnessCrash({ exitCode: 1, ...parsed })).toBe(false); + expect( + isCodexTransientUpstreamError({ + stdout: "connection reset by peer while running the deploy", + errorMessage: "agent gave up", + }), + ).toBe(false); + }); +}); + describe("classifyCodexAuthRefreshFailure", () => { it("classifies explicit refresh-token failure messages", () => { expect(classifyCodexAuthRefreshFailure({ errorMessage: "provider error: refresh_token_reused" })).toBe( diff --git a/packages/adapters/codex-local/src/server/parse.ts b/packages/adapters/codex-local/src/server/parse.ts index a69931757f..7698977f40 100644 --- a/packages/adapters/codex-local/src/server/parse.ts +++ b/packages/adapters/codex-local/src/server/parse.ts @@ -31,6 +31,8 @@ export function parseCodexJsonl(stdout: string) { let sessionId: string | null = null; let finalMessage: string | null = null; let errorMessage: string | null = null; + let sawProtocolEvent = false; + let sawProtocolTerminalEvent = false; const usage = { inputTokens: 0, cachedInputTokens: 0, @@ -45,6 +47,10 @@ export function parseCodexJsonl(stdout: string) { if (!event) continue; const type = asString(event.type, ""); + if (type) sawProtocolEvent = true; + if (type === "error" || type === "turn.completed" || type === "turn.failed") { + sawProtocolTerminalEvent = true; + } if (type === "thread.started") { sessionId = asString(event.thread_id, sessionId ?? "") || sessionId; continue; @@ -86,9 +92,30 @@ export function parseCodexJsonl(stdout: string) { usage, usageBasis: "per_run" as const, errorMessage, + sawProtocolEvent, + sawProtocolTerminalEvent, }; } +/** + * Structural crash detection: the codex CLI can only report an agent-level + * failure through the JSONL protocol (an `error` event, `turn.failed`, or a + * finished `turn.completed` followed by a nonzero exit). A nonzero exit after + * the protocol stream started but before any terminal event means the process + * died out from under the agent (MCP transport crash, worker panic, killed + * tool server) — retriable infrastructure, not agent behavior. This + * deliberately does not match error text: transport failure strings vary, and + * stdout/stderr can quote agent output that merely discusses network errors. + */ +export function isCodexHarnessCrash(input: { + exitCode: number | null; + sawProtocolEvent: boolean; + sawProtocolTerminalEvent: boolean; +}): boolean { + if ((input.exitCode ?? 0) === 0) return false; + return input.sawProtocolEvent && !input.sawProtocolTerminalEvent; +} + export function isCodexUnknownSessionError(stdout: string, stderr: string): boolean { const haystack = `${stdout}\n${stderr}` .split(/\r?\n/) diff --git a/server/src/__tests__/codex-local-execute.test.ts b/server/src/__tests__/codex-local-execute.test.ts index 96b7cee8ae..a907119970 100644 --- a/server/src/__tests__/codex-local-execute.test.ts +++ b/server/src/__tests__/codex-local-execute.test.ts @@ -698,6 +698,68 @@ describe("codex execute", () => { } }); + it("classifies mid-turn harness crashes as retryable transient upstream errors", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-execute-harness-crash-")); + const workspace = path.join(root, "workspace"); + const commandPath = path.join(root, "codex"); + await fs.mkdir(workspace, { recursive: true }); + // Faithful to the observed MCP transport crash: the protocol stream starts, + // then the process dies with only a harness tracing line on stderr — no + // protocol-terminal event (error / turn.failed / turn.completed). + const script = `#!/usr/bin/env node +console.log(JSON.stringify({ type: "thread.started", thread_id: "thread-crash-1" })); +console.log(JSON.stringify({ type: "item.completed", item: { type: "agent_message", text: "Starting the task." } })); +console.error("2026-07-23T22:58:56.007042Z ERROR rmcp::transport::worker: worker quit with fatal: Transport channel closed, when UnexpectedContentType(Some(\\"text/plain\\"))"); +process.exit(1); +`; + await fs.writeFile(commandPath, script, "utf8"); + await fs.chmod(commandPath, 0o755); + + const previousHome = process.env.HOME; + process.env.HOME = root; + await seedSharedCodexAuth(root); + + try { + const result = await execute({ + runId: "run-harness-crash", + agent: { + id: "agent-1", + companyId: "company-1", + name: "Codex Coder", + adapterType: "codex_local", + adapterConfig: { engine: "cli" }, + }, + runtime: { + sessionId: null, + sessionParams: null, + sessionDisplayId: null, + taskKey: null, + }, + config: { + engine: "cli", + command: commandPath, + cwd: workspace, + promptTemplate: "Follow the paperclip heartbeat.", + }, + context: {}, + authToken: "run-jwt-token", + onLog: async () => {}, + }); + + expect(result.exitCode).toBe(1); + expect(result.errorCode).toBe("codex_harness_crash"); + expect(result.errorFamily).toBe("transient_upstream"); + expect(result.errorMessage).toContain("Transport channel closed"); + expect(result.sessionId).toBe("thread-crash-1"); + expect(result.clearSession).toBe(false); + expect((result.resultJson as Record).errorFamily).toBe("transient_upstream"); + } finally { + if (previousHome === undefined) delete process.env.HOME; + else process.env.HOME = previousHome; + await fs.rm(root, { recursive: true, force: true }); + } + }); + it("persists retry-not-before metadata for codex provider quota failures", async () => { const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-execute-usage-limit-")); const workspace = path.join(root, "workspace"); diff --git a/server/src/__tests__/heartbeat-retry-scheduling.test.ts b/server/src/__tests__/heartbeat-retry-scheduling.test.ts index a4f43a1609..9d8a1cbe0e 100644 --- a/server/src/__tests__/heartbeat-retry-scheduling.test.ts +++ b/server/src/__tests__/heartbeat-retry-scheduling.test.ts @@ -2133,6 +2133,66 @@ describeEmbeddedPostgres("heartbeat bounded retry scheduling", () => { } }); + it("schedules a recovery continuation for codex harness crashes", async () => { + const companyId = randomUUID(); + const agentId = randomUUID(); + const runId = randomUUID(); + const now = new Date("2026-07-24T12:00:00.000Z"); + + await seedRetryFixture({ + runId, + companyId, + agentId, + now, + errorCode: "codex_harness_crash", + errorFamily: "transient_upstream", + }); + + const scheduled = await heartbeat.scheduleBoundedRetry(runId, { + now, + random: () => 0.5, + }); + + expect(scheduled.outcome).toBe("scheduled"); + if (scheduled.outcome !== "scheduled") return; + + expect(scheduled.run.scheduledRetryAttempt).toBe(1); + expect(scheduled.run.scheduledRetryReason).toBe("transient_failure"); + const contextSnapshot = scheduled.run.contextSnapshot as Record; + expect(contextSnapshot.codexTransientFallbackMode).toBe("same_session"); + expect(contextSnapshot.retryOfRunId).toBe(runId); + + await cleanupRetryFixture(); + }); + + it("schedules a harness-crash recovery from the error code alone when the result json lost the error family", async () => { + const companyId = randomUUID(); + const agentId = randomUUID(); + const runId = randomUUID(); + const now = new Date("2026-07-24T13:00:00.000Z"); + + await seedRetryFixture({ + runId, + companyId, + agentId, + now, + errorCode: "codex_harness_crash", + errorFamily: null, + }); + + const scheduled = await heartbeat.scheduleBoundedRetry(runId, { + now, + random: () => 0.5, + }); + + expect(scheduled.outcome).toBe("scheduled"); + if (scheduled.outcome !== "scheduled") return; + expect(scheduled.run.scheduledRetryReason).toBe("transient_failure"); + expect((scheduled.run.contextSnapshot as Record).codexTransientFallbackMode).toBe("same_session"); + + await cleanupRetryFixture(); + }); + it("honors codex retry-not-before timestamps when they exceed the default bounded backoff", async () => { const companyId = randomUUID(); const agentId = randomUUID(); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 4dd31b4730..5a878a32c6 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -436,7 +436,11 @@ function readHeartbeatRunErrorFamily( if (run.errorCode === "provider_quota") { return "provider_quota"; } - if (run.errorCode === "codex_transient_upstream" || run.errorCode === "claude_transient_upstream") { + if ( + run.errorCode === "codex_transient_upstream" || + run.errorCode === "claude_transient_upstream" || + run.errorCode === "codex_harness_crash" + ) { return "transient_upstream"; } return null; diff --git a/server/src/services/recovery/service.pause-durability.test.ts b/server/src/services/recovery/service.pause-durability.test.ts index 5e63043f8e..cd873a16a1 100644 --- a/server/src/services/recovery/service.pause-durability.test.ts +++ b/server/src/services/recovery/service.pause-durability.test.ts @@ -26,6 +26,12 @@ describe("pause durability: continuation retry classification", () => { expect(c.maxAttempts).toBeGreaterThan(0); }); + it("codex harness crashes retry as transient infra", () => { + const c = classifyContinuationFailure(run("codex_harness_crash")); + expect(c.kind).toBe("transient_infra"); + expect(c.maxAttempts).toBeGreaterThan(0); + }); + it("generic cancelled (non-pause cancellation) is NOT non-retryable", () => { // non-pause cancellations (the internal invokability cancel and budget pause) keep errorCode "cancelled" -> default branch expect(classifyContinuationFailure(run("cancelled")).kind).toBe("default"); diff --git a/server/src/services/recovery/service.ts b/server/src/services/recovery/service.ts index b370f7b711..ed37976de5 100644 --- a/server/src/services/recovery/service.ts +++ b/server/src/services/recovery/service.ts @@ -274,6 +274,7 @@ function isTerminalIssueRun(latestRun: LatestIssueRun) { const TRANSIENT_INFRA_CONTINUATION_ERROR_CODES = new Set([ "adapter_failed", "codex_transient_upstream", + "codex_harness_crash", "claude_transient_upstream", "provider_quota", "timeout",