Skip to content

Read ACP command exit codes from rawOutput instead of the status - #2131

Merged
SawyerHood merged 1 commit into
mainfrom
bb/fix-1529-shell-session-deleted-cwd
Aug 21, 2026
Merged

Read ACP command exit codes from rawOutput instead of the status#2131
SawyerHood merged 1 commit into
mainfrom
bb/fix-1529-shell-session-deleted-cwd

Conversation

@SawyerHood

@SawyerHood SawyerHood commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

What was wrong

Issue #1529 (report: https://get-bb.github.io/reports/issues/1529.html). The wedge itself lives in Cursor's CLI: its Shell tool re-spawns every command with the cwd persisted from the last run, Node's spawn fails with ENOENT once that directory is deleted (git worktree remove), and Cursor's ACP adapter reports the call as status: "completed" with no content and no rawOutput. bb cannot fix that, but bb made it invisible. plugins/provider-acp/src/delta-translation.ts derived the exit code from the ACP status alone (completed -> 0, failed -> 1). ACP has no exit-code field, and Cursor also reports a command that really exited non-zero as status: "completed" with the real code in rawOutput.exitCode. So the bb timeline and bb thread log showed "exit code 0" both for commands that never ran and for commands that exited 1.

What changed

  • plugins/provider-acp/src/wire.ts: acpToolCallRawOutputExitCodeSchema parses { exitCode: integer } out of the agent-defined rawOutput payload at the boundary.
  • packages/provider-bridge-protocol/recordings/acp-cursor/*/bridge→runtime.current.ndjson: re-recorded acp-cursor lanes (six cells); only exitCode on item closes changed. Details under Rebase below.
  • plugins/provider-acp/src/delta-translation.ts: toolCallClose now gets its exit code from extractAcpExitCode. A reported integer rawOutput.exitCode wins. Without one, a failed call still maps to 1 (non-zero is all we know) and a completed call carries no exitCode instead of a fabricated 0. exitCode is already optional on the commandExecution item, the delta close, the thread-view exec lifecycle, the CLI formatter, and the app's TerminalOutputBlock, so consumers render "unknown" by omitting the exit-code line.

Behavior change for every ACP provider: a completed command without a reported exit code no longer shows exit code 0; a completed command with rawOutput.exitCode: 1 now renders as an error with exit 1. Translation only, nothing crosses the server/daemon wire, so no HOST_DAEMON_PROTOCOL_VERSION bump. I did not take the report's optional step of mapping a result-less completed call to failed: that guesses, and omitting the exit code is the honest signal.

The wedge remains upstream (Cursor CLI, cursor-agent 2026.08.11). With this change the agent's own "returned no exit status" text is no longer contradicted by a bb row claiming success.

How you verified

New tests in plugins/provider-acp/src/delta-translation.test.ts (command exit codes), using the exact tool_call/tool_call_update shapes recorded from cursor-agent acp. Against the origin/main sources they fail:

× uses rawOutput.exitCode when the agent reports a non-zero exit as completed
  AssertionError: expected +0 to be 1
× omits the exit code when a completed call carries no result at all
  AssertionError: expected +0 to be undefined
× prefers a reported exit code over the failed-status fallback
  AssertionError: expected 1 to be 127
× ignores non-integer exit codes in rawOutput
  AssertionError: expected +0 to be undefined

With the fix: pnpm exec turbo run typecheck test --filter=bb-plugin-provider-acp passes (14 files, 186 tests). The existing translates execute tool calls into command executions expectation dropped its exitCode: 0 (no exit code was reported in that fixture).

Manual check on my dev instance with a real acp-cursor thread and the report's prompt (scratch repo + worktree, git worktree remove --force, then echo hi; git status, then pwd with a working-directory override, then false). bb thread log --format verbose after the fix:

── Ran echo hi; git status
  $ echo hi; git status
                                   <- no fabricated "exit code 0"
── Ran pwd
  {"exitCode":0,"stdout":".../repo/base\n","stderr":""}
── Ran false (error)
  {"exitCode":1,"stdout":"","stderr":""}
  exit 1                           <- was "exit code 0" before

The persisted items: the two wedged echo hi; git status calls are status: "completed" with no exitCode; false is exitCode: 1.

Rebase

Rebased onto current main (cf00cfe06) after #2136 (WS1a generic assembler) and #2179 (WS1b-acp: ACP bridge to grammar v3 with presentation) rewrote plugins/provider-acp/src/delta-translation.ts. What moved:

  • extractAcpToolCallOutputText, buildAcpFileChanges, and classifyAcpToolCall now live in plugins/provider-acp/src/tool-classification.ts and return { item, presentation }; the PR's conflict hunks that carried copies of those helpers were dropped, not re-added.
  • toolCallClose still derived the exit code from the ACP status on main (terminal ? { exitCode: status === "failed" ? 1 : 0 } : {}), so the bug was still present. The fix maps one-to-one: extractAcpExitCode(args.event, args.status) replaces the terminal flag, and the close spreads exitCode only when one is known. The new classified.item / classified.presentation fields and the injected-tool binding cleanup from [stacked on #2178] WS1b-acp: ACP bridge to grammar v3 with presentation #2179 are kept as-is.
  • wire.ts (acpToolCallRawOutputExitCodeSchema) applied without conflict.
  • Test fixture translates execute tool calls into command executions keeps main's new presentation block and drops exitCode: 0, as before.

Re-verified on the new base. With git checkout origin/main -- delta-translation.ts wire.ts, vitest run src/delta-translation.test.ts: 5 failed / 32 passed (expected +0 to be 1, expected +0 to be undefined, expected 1 to be 127, expected +0 to be undefined, plus the fixture diff). With the fix restored, from the committed tree: pnpm exec turbo run typecheck test --filter=bb-plugin-provider-acp --forceTasks: 7 successful, 7 total; Test Files 15 passed (15), Tests 201 passed (201) (includes the recorded ACP conformance cells).

Fixes #1529

AGENT GENERATED: by Claude Opus 5

Rebase onto 75d6fc4d4 and acp-cursor re-record

Rebased onto current main (75d6fc4d4, five commits ahead of the previous base cf00cfe06: #2202, #2201, #2147, #2150, #2120). None touch plugins/provider-acp/src/{delta-translation,wire}.ts; the rebase applied clean and the fix is unchanged.

The verifier found that the fix changes what the ACP bridge emits for committed acp-cursor recordings, so pnpm exec turbo run test --filter=@bb/provider-parity --force failed on acp-cursor/{approval-deny,steer,web-search} (3 failed / 40 passed on the rebased tree before this change; CI had only been green through a turbo cache hit). Ran pnpm --filter @bb/provider-parity rerecord --provider acp-cursor (planning with this checkout's assembler; no --plan-with needed) and committed the resulting bridge→runtime.current.ndjson lanes. Checked every changed line at the delta level (parsed old vs new, removed exitCode, deep-equal): the ONLY change is exitCode on item.close deltas. Line counts are unchanged, row-count pins are unchanged, parity-allowlist.json is untouched.

  • approval-deny: touch ~/bb-recording-outside.txt close, exitCode: 0 -> absent (Cursor reported no rawOutput).
  • steer: the fourth sleep 2 close, exitCode: 0 -> absent (the steer interrupted it before a completed update; the other four keep their reported 0).
  • web-search: the node -e ... close, exitCode: 0 -> 1 (Cursor reported rawOutput.exitCode: 1 under status: "completed"; this is the Persistent shell session wedges silently when its stored cwd is deleted (git worktree remove); all subsequent spawns return no exit status #1529 shape).
  • subagent, turn-tools, user-question, web-search: tool/fileChange closes drop the exitCode: 0 the old code attached to every terminal close. The assembler only reads exitCode on commands, which is why those cells passed before; the re-recorded lanes now match the wire.
  • fork re-recorded identically except for the recording machine's checkout path inside the "does not advertise session/fork support" error string; left as committed.

Re-verified from the committed tree (git status --porcelain empty). Fail-before: with git checkout origin/main -- delta-translation.ts wire.ts, vitest run src/delta-translation.test.ts -> 5 failed / 32 passed (expected +0 to be 1, expected +0 to be undefined, expected 1 to be 127, expected +0 to be undefined, plus the fixture diff). Pass-after: 37 passed. pnpm exec turbo run typecheck test --filter=bb-plugin-provider-acp --filter=@bb/provider-bridge-protocol --filter=@bb/provider-parity --force -> Tasks: 11 successful, 11 total (Cached: 0 cached); bb-plugin-provider-acp 15 files / 201 tests, @bb/provider-bridge-protocol 15 files / 217 tests, @bb/provider-parity 1 file / 43 tests (all 43 cells reproduce their recordings).

Independent verification

Checked out bb/fix-1529-shell-session-deleted-cwd (e042d5a, one commit on top of 2ff8598) in a separate worktree; git merge-tree --write-tree origin/main HEAD merges clean against current main (703213a). PR diff is limited to plugins/provider-acp/src/{delta-translation.ts,wire.ts,delta-translation.test.ts}; no wire change between server and daemon, so no HOST_DAEMON_PROTOCOL_VERSION bump needed. Consumers of exitCode (delta-assembler close fields, thread-view/exec-lifecycle.ts, format-timeline-text.ts, TerminalOutputBlock) all treat it as optional/nullable; a missing code falls back to the item status, a non-zero code maps to error.

Fail-before / pass-after (pnpm exec vitest run src/delta-translation.test.ts in plugins/provider-acp):

  • With git checkout origin/main -- delta-translation.ts wire.ts: 5 failed / 20 passed. expected +0 to be 1, expected +0 to be undefined, expected 1 to be 127, expected +0 to be undefined, plus translates execute tool calls into command executions (fixture no longer expects the fabricated exitCode: 0).
  • With the PR sources restored: 25 passed.

pnpm exec turbo run typecheck test --filter=bb-plugin-provider-acp --force: 7 tasks successful, 14 test files / 186 tests passed. gh pr checks 2131: all checks pass (Checks, Package Smoke x2, Tests app-1/2/3, integration, packages, server, version check).

Repro on the fixed branch (own dev instance, scratch repo + worktree, real acp-cursor thread thr_eenfcb5cv5 with the report's prompt plus a final false): bb thread log --format verbose shows Ran echo hi; git status with no output and no exit-code line, and Ran false (error) ... exit 1. Persisted items: seq 42 {"command":"echo hi; git status","status":"completed"} (no exitCode), seq 54 {"command":"false","status":"completed","exitCode":1}. On main the same log printed exit code 0 for both. The upstream wedge itself still reproduces (Cursor returns "The shell command returned no exit status" for step 3 and heals after the working-directory override), which is outside bb's control.

Residual risks: completed ACP commands from agents that do not report rawOutput.exitCode (or use a different key) no longer show exit code 0; a Cursor call that never ran is still rendered as a completed row with no output rather than as failed. Merging closes #1529 although the wedge needs an upstream Cursor CLI fix; the body above says so.

AGENT GENERATED: by Claude Opus 5

Independent verification (post-rebase)

Checked out bb/fix-1529-shell-session-deleted-cwd at 06c27aa (one commit on top of 75d6fc4) in a fresh worktree. Since the rebase main gained only 85eec4d (#2210, iOS TestFlight CI; no overlap) and GitHub reports the PR MERGEABLE. The fix still targets the live path after today's grammar-v3 bridge stack: createAcpDeltaTranslator is wired once in plugins/provider-acp/src/bridge/bridge.ts, and toolCallClose is the only place the ACP bridge emits a command exitCode; no legacy event-translation.ts remains. The delta/assembler contract is unchanged (exitCode already optional in thread-delta.ts and applied only to commandExecution in delta-assembler.ts), so no HOST_DAEMON_PROTOCOL_VERSION bump is needed. thread-view projects a missing code to null (no exit line) and a non-zero one to an error row.

Recordings: parsed every changed line of the six re-recorded acp-cursor/*/bridge→runtime.current.ndjson lanes against 75d6fc4 with exitCode stripped; all 9 changed deltas are deep-equal otherwise, line counts unchanged, other acp-cursor cells byte-identical. The web-search node -e close moves 0 -> 1 because Cursor reported rawOutput.exitCode: 1 under status: "completed" (the #1529 shape in a real recording).

Fail-before / pass-after (pnpm exec vitest run src/delta-translation.test.ts in plugins/provider-acp): with git checkout origin/main -- plugins/provider-acp/src/delta-translation.ts plugins/provider-acp/src/wire.ts -> 5 failed / 32 passed: expected +0 to be 1, expected +0 to be undefined, expected 1 to be 127, expected +0 to be undefined, plus the translates execute tool calls into command executions fixture. Sources restored -> 37 passed (37).

From the committed tree (git status --porcelain empty), after pnpm install --frozen-lockfile --prefer-offline and pnpm exec turbo run build (18/18): pnpm exec turbo run typecheck test --filter=bb-plugin-provider-acp --filter=@bb/provider-bridge-protocol --filter=@bb/provider-parity --force -> Tasks: 11 successful, 11 total, Cached: 0 cached; bb-plugin-provider-acp 15 files / 201 tests, @bb/provider-bridge-protocol 15 files / 217 tests, @bb/provider-parity 1 file / 43 tests. gh pr checks 2131: all required checks pass on 06c27aa.

Repro on the fixed branch (own dev instance, scratch repo + git worktree add, real acp-cursor thread thr_5mn7xbb4j2 with the report's verbatim prompt): the upstream wedge still reproduces (Cursor returns "The shell command returned no exit status" for steps 3 and 4 and heals after the working-directory override), and bb now persists those two items as {"command":"echo hi; git status","status":"completed"} / {"command":"echo alive","status":"completed"} with no exitCode and no output, where main wrote exitCode: 0. A second thread (thr_sxg224nzrj) in which Cursor reported rawOutput.exitCode: 127 and 126 under status: "completed" rendered as exit 127 / exit 126 (error) rows in bb thread log.

Residual risks: unchanged from the previous section. The wedge itself is a Cursor CLI bug; merging closes #1529 on the bb side only (misreported exit codes), which the body states.

AGENT GENERATED: by Claude Opus 5

@SawyerHood
SawyerHood marked this pull request as ready for review August 21, 2026 03:32
@SawyerHood
SawyerHood force-pushed the bb/fix-1529-shell-session-deleted-cwd branch from e042d5a to c331136 Compare August 21, 2026 14:45
@SawyerHood
SawyerHood marked this pull request as draft August 21, 2026 14:54
The ACP translator turned a tool call's terminal status into an exit
code: completed became 0, failed became 1. ACP has no exit-code field,
and Cursor reports both a command that exited 1 and a command that
never spawned (its persistent shell's cwd was deleted) as
`status: "completed"`, so bb labelled both as exit code 0 and hid the
failure from the timeline and the CLI.

Read the agent's reported `rawOutput.exitCode` when it is an integer.
Without one, a failed call still maps to 1 and a completed call carries
no exit code rather than a fabricated 0.

Re-record the acp-cursor `bridge→runtime.current.ndjson` lanes
(`pnpm --filter @bb/provider-parity rerecord --provider acp-cursor`):
the only delta-level change is `exitCode` on item closes. Command
closes without a reported code drop the fabricated 0 (approval-deny,
steer, web-search); the web-search `node -e` close now carries the
reported 1; tool and fileChange closes no longer carry an exitCode the
assembler ignored (subagent, turn-tools, user-question, web-search).
Row-count pins are unchanged.

Refs #1529

Co-Authored-By: Claude <noreply@anthropic.com>
@SawyerHood
SawyerHood force-pushed the bb/fix-1529-shell-session-deleted-cwd branch from c331136 to 06c27aa Compare August 21, 2026 16:41
@SawyerHood
SawyerHood marked this pull request as ready for review August 21, 2026 16:54
@SawyerHood
SawyerHood merged commit 26fbf43 into main Aug 21, 2026
13 checks passed
@SawyerHood
SawyerHood deleted the bb/fix-1529-shell-session-deleted-cwd branch August 21, 2026 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Persistent shell session wedges silently when its stored cwd is deleted (git worktree remove); all subsequent spawns return no exit status

1 participant