Remove the URL QR toggle from the connect settings card - #2201
Merged
SawyerHood merged 1 commit intoAug 21, 2026
Conversation
Mobile pairing now owns the phone QR flow. The second QR for the public URL was confusing next to it. Co-Authored-By: Claude <noreply@anthropic.com>
SawyerHood
deleted the
bb/remove-confusing-bb-connect-content-thr_wz892ssvsm
branch
August 21, 2026 15:47
SawyerHood
added a commit
that referenced
this pull request
Aug 21, 2026
## 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 #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 --force` → `Tasks: 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 #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 Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
The connected card in the bb connect settings section showed a "Show QR for phone" toggle for the public URL. The card also shows the mobile-app pairing QR. Two QR codes with different purposes on one card confused users.
What changed
plugins/connect/app.tsx: removed the URL QR toggle, its hint text, and theqrOpenstate.QrCodeImagestays for the mobile pairing section.plugins/connect/app.test.tsx: the "rest of the paired card is unaffected" assertion now checks theRe-pairbutton.How you verified
pnpm exec turbo run typecheck test --filter=bb-plugin-connectpassed (90 tests).