From 743698e700638cddc4d3fc4c0ad89e6c132fab40 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 28 Oct 2026 15:11:22 -0500 Subject: [PATCH] fix(runner): preserve OpenCode bash process exit codes (#18076) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native Runner stores structured tool events so people can inspect actual work. > - OpenCode puts a completed bash command's process exit in `state.metadata.exit`. > - The canonical event mapper only read `state.exit`, so a real nonzero exit became unknown. > - This pull request preserves the documented builtin bash exit and rejects conflicting evidence. > - The benefit is an accurate process exit without changing tool completion or native run status. ## Linked Issues or Issue Description **What happened?** A completed OpenCode bash part with `state.metadata.exit: 37` produced `exitCode: null`. The process result was present, but the canonical tool event lost it. The pinned upstream [shell result](https://github.com/anomalyco/opencode/blob/v1.18.34/packages/opencode/src/tool/shell.ts#L544-L552) and [completed part](https://github.com/anomalyco/opencode/blob/v1.18.34/packages/opencode/src/session/processor.ts#L160-L170) establish the field path. **Expected behavior** The canonical event retains the numeric process exit from a completed builtin bash part. A completed tool call can have a nonzero command exit. Missing or invalid evidence must not become a successful command. **Steps to reproduce** 1. Pass a `type: "tool"`, `tool: "bash"` part with completed state and numeric `metadata.exit` to `canonicalProviderEventsFromOpenCodePart`. 2. Inspect `tool.execution.completed.payload.exitCode`. 3. Before this fix, the value is null. After this fix, it matches the process exit. **Paperclip version or commit** Reproduced on master `f4f02880cae4f5fa54d47d39565ca99b6b72446d`. Duplicate searches found no matching fix. Related PR #15715 concerns OpenCode V2 transport and version support; it does not change this mapper. ## What Changed - Read numeric metadata exits only from completed builtin bash parts. - Keep legacy numeric top-level exits. Return unknown when supplied legacy evidence conflicts. - Cover zero and nonzero exits, invalid values, foreign tools, incomplete calls and conflicting receipts. - Document the run-log field and the distinction between tool completion and command success. ## Verification - The regression suite first failed five assertions on the unchanged mapper. All 53 provider-event tests now pass. - A focused no-emit TypeScript check covers the changed mapper, its tests and imported protocol validators. - Tests use synthetic structured parts and real protocol validation. No local database, provider, SDK build or live scenario was run. - All 48 jobs in the normal PR workflow passed, including the required `ci / verify` and `ci / e2e` gates. CI also executed all 53 provider-event tests. - Greptile reviewed this exact head at 5/5 with no actionable findings. The separate hosted quality-gate review also completed successfully. ## Risks - Conflicting legacy and metadata exits now remain unknown for completed builtin bash calls. This avoids selecting one inconsistent value as proof. - Other tools retain their existing legacy behavior. Output prose never supplies an exit code. - The change adds no schema field and changes no provider version, budget, accounting, fixture or native run outcome. ## Model Used OpenAI `gpt-6.1-sol` through Codex, with reasoning and terminal tools. The session does not expose the context-window size. ## 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) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [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 --- doc/run-log-events.md | 8 ++++ .../src/provider-events.test.ts | 47 +++++++++++++++++++ .../paperclip-runner/src/provider-events.ts | 15 +++++- 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/doc/run-log-events.md b/doc/run-log-events.md index b449def19f6..5672c6a693c 100644 --- a/doc/run-log-events.md +++ b/doc/run-log-events.md @@ -92,6 +92,14 @@ They also become native Observability spans only with an operator OTLP endpoint. ## Native PRP Run-Log Events +Completed OpenCode builtin `bash` parts preserve the numeric process exit from +`state.metadata.exit` in `tool.execution.completed.payload.exitCode`. The tool +status stays `completed` even when the command exits with a nonzero code. This +status describes completion of the tool call, not success of the command or the +native run. Metadata adds an exit only when it is an integer. Conflicting supplied +legacy exits stay unknown. Existing numeric `state.exit` values remain supported; +metadata on other tools cannot add process-exit proof. + `auth.credentials.updated` is a runner-produced PRP event for a change to a supported credential store, including account identity changes and later token refreshes on the new account. The bundled transport awaits a host commit callback diff --git a/packages/paperclip-runner/src/provider-events.test.ts b/packages/paperclip-runner/src/provider-events.test.ts index a35c21179fd..ea9469238d2 100644 --- a/packages/paperclip-runner/src/provider-events.test.ts +++ b/packages/paperclip-runner/src/provider-events.test.ts @@ -484,6 +484,53 @@ describe("provider-neutral events", () => { ).toEqual([]); }); + it.each([0, 37, -1])("preserves OpenCode bash metadata exit %i without changing tool completion", (exit) => { + const event = canonicalProviderEventsFromOpenCodePart({ + id: "bash-part", type: "tool", tool: "bash", callID: "bash-call", + state: { status: "completed", input: { command: "node verify.mjs" }, + output: "Work validation finished", metadata: { exit, truncated: false }, time: { start: 1, end: 2 } }, + })[0]!; + expect(event).toMatchObject({ eventType: "tool.execution.completed", itemId: "bash-part", + payload: { executionId: "bash-part", name: "bash", status: "completed", exitCode: exit } }); + expect(validatePrpEvent(envelope(event)).ok).toBe(true); + }); + + it.each([undefined, null, "37", 1.5, Number.NaN, Number.POSITIVE_INFINITY, Number.MAX_SAFE_INTEGER + 1])("does not infer a bash exit from invalid metadata %s", (exit) => { + const event = canonicalProviderEventsFromOpenCodePart({ id: "bash-invalid", type: "tool", tool: "bash", + state: { status: "completed", output: "exitCode=37", metadata: { exit } } })[0]!; + expect(event.payload.exitCode).toBeNull(); + }); + + it.each([ + { type: "tool", tool: "read", state: { status: "completed" } }, + { type: "tool", tool: "mcp_bash", state: { status: "completed" } }, + { type: "tool", tool: "unknown", callID: "functions.bash:0", state: { status: "completed" } }, + { type: "bash", state: { status: "completed" } }, + { type: "tool", tool: "bash", state: { status: "running" } }, + { type: "tool", tool: "bash", state: { status: "error", error: "Tool execution aborted" } }, + ])("rejects metadata exits outside a completed builtin bash part %#", (part) => { + const event = canonicalProviderEventsFromOpenCodePart({ id: "not-bash-complete", ...part, + state: { ...part.state, metadata: { exit: 37 } } })[0]!; + expect(event.payload.exitCode).toBeNull(); + }); + + it.each([0, 37, "37", 1.5])("rejects conflicting legacy and metadata exits %s", (exit) => { + const event = canonicalProviderEventsFromOpenCodePart({ id: "bash-conflict", type: "tool", tool: "bash", + state: { status: "completed", exit, metadata: { exit: 1 } } })[0]!; + expect(event.payload.exitCode).toBeNull(); + }); + + it.each([ + { tool: "bash", exit: 37, metadata: undefined }, + { tool: "bash", exit: 37, metadata: { exit: 37 } }, + { tool: "bash", exit: 37, metadata: { exit: "37" } }, + { tool: "read", exit: 37, metadata: { exit: 1 } }, + ])("preserves an existing legacy numeric exit %#", ({ tool, exit, metadata }) => { + const event = canonicalProviderEventsFromOpenCodePart({ id: "legacy-exit", type: "tool", tool, + state: { status: "completed", exit, metadata } })[0]!; + expect(event.payload.exitCode).toBe(exit); + }); + it("maps OpenCode pending dynamic tools to a valid builtin execution start", () => { const event = canonicalProviderEventsFromOpenCodePart({ id: "prt_02c35c313001X42cqyvQrpXOFv", diff --git a/packages/paperclip-runner/src/provider-events.ts b/packages/paperclip-runner/src/provider-events.ts index a20ac855af1..dac4af58b80 100644 --- a/packages/paperclip-runner/src/provider-events.ts +++ b/packages/paperclip-runner/src/provider-events.ts @@ -1002,6 +1002,19 @@ export function canonicalProviderEventsFromCodex( return []; } +function openCodeToolExitCode( + part: Record, + state: Record, +): number | null { + const legacy = integer(state.exit); + if (part.type !== "tool" || part.tool !== "bash" || state.status !== "completed") { + return legacy; + } + const exit = integer(record(state.metadata).exit); + if (exit === null) return legacy; + return state.exit != null && legacy !== exit ? null : exit; +} + /** Maps documented OpenCode structured message parts. Unrecognized parts are deliberately unsupported. */ export function canonicalProviderEventsFromOpenCodePart( partValue: unknown, @@ -1051,7 +1064,7 @@ export function canonicalProviderEventsFromOpenCodePart( readOnly: /read|search|grep|find|list|glob/i.test(tool), status, durationMs: integer(state.time), - exitCode: integer(state.exit), + exitCode: openCodeToolExitCode(part, state), progress: status === "running" ? text(state.title).slice(0, 4000) || null : null, ...boundedOutput(