Skip to content

[stacked on #2178] WS1b-acp: ACP bridge to grammar v3 with presentation - #2179

Merged
SawyerHood merged 6 commits into
bb/ws1b-claude-claude-code-bridge-to-v3-stack-on-21-thr_c7uz7y2huafrom
bb/ws1b-acp-acp-bridge-to-v3-stack-on-2178-thr_9g9skmtvfh
Aug 21, 2026
Merged

[stacked on #2178] WS1b-acp: ACP bridge to grammar v3 with presentation#2179
SawyerHood merged 6 commits into
bb/ws1b-claude-claude-code-bridge-to-v3-stack-on-21-thr_c7uz7y2huafrom
bb/ws1b-acp-acp-bridge-to-v3-stack-on-2178-thr_9g9skmtvfh

Conversation

@SawyerHood

@SawyerHood SawyerHood commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #2178 (WS1b-claude). Chain: contract → WS1a → WS2a → codex #2164 → claude #2178this PR (6 commits above claude head 61a0a9656). Do not merge before the chain beneath it lands.

What was wrong

The ACP bridge threw away the agent's native tool-call kind (read, edit, execute, search, fetch, think, other) and wrote the human title into the tool slot of a generic tool item (provider-acp/src/delta-translation.ts, the old classifyAcpToolCall). In the production database that is ~1,100 ACP rows whose "tool name" is Read `/path/to/file` , plus Web search:, Updating plan, MCP: tool, and tool. Generic tool permissions were faked as command approvals (interactions.ts) because that was the one subject with free text. No ACP item carried a presentation.

What changed

One commit per layer; the last deletes.

  1. Presentation on every itemplugins/provider-acp/src/presentation.ts: a label pair and host glyph per native kind, commands with the agent's Markdown code ticks stripped from the headline, file changes by verb (write/edit/delete) with their file names, plan snapshots, compactions. Every item.open/item.close carries it.
  2. Native kind → core kinds; title → presentation.titleplugins/provider-acp/src/tool-classification.ts: executecommand, diff / edit / delete with a path → fileChange (the shared operation classifier the permission mapping uses is untouched, Present ACP file-write permissions as file-change approvals #1803), readfileRead when the path is known (locations, a rawInput path field, or a single code-ticked token in the title — how grok titles its reads), searchsearch when rawInput has a pattern/query/regex (content) or glob (path), fetchwebFetch when rawInput or the title carries the URL, thinkreasoning. Everything else (other, move, no kind, or a kind whose shape the agent left unfilled — Cursor's read/fetch arrive with an empty rawInput and no locations) is a generic tool whose tool slot is the kind and whose presentation is the kind's label with the agent's title as headline. A row is never a fileRead without a path. ACP plan updates become settled planSteps snapshots (channel-keyed, collapsed by default, read by the todo banner) instead of turn.plan.
  3. bb-injected tools (Q31) — the translator learns the session's dynamicTools at construction and binds each call the MCP proxy forwards (forwardDynamicToolCallnoteInjectedToolCall) to the agent's announcement. ACP gives no id linking the two (Cursor announces every MCP call as "MCP: tool", kind other), so the binding is positional: the unbound other candidate whose title names the tool, else one that mentions MCP, else the oldest; a proxied call with no candidate open waits for the next announcement. A bound call's open or close reads { server: "bb", tool, presentation } from the DynamicTool definition (generic under bb's glyph when the definition predates the field); the close re-states it, so late binding still settles as the bb tool.
  4. tool_use approval subjectinteractions.ts raises { kind: "tool_use", itemId, tool, presentation } for every permission that is neither a command nor a file change, with the same classification the timeline row gets. apps/server/.../pending-interaction-timeline.ts stops throwing on the subject: a tool-use approval has no timeline item of its own (the agent's own tool_call row is the record; the banner renders the presentation) until WS5's interaction-lifecycle event. The runtime conformance harness validates the subject instead of throwing; the "no producer yet" comments on the client renderers are updated.
  5. Delete the ACP v2 translation path — ACP was the last turn.plan speaker, so the grammar member leaves thread-delta.ts and the assembler under the grammar range (the way [stacked on #2121 ← #2148 ← #2153 ← #2136 ← #2124] WS1b-codex: Codex bridge to grammar v3 with presentation #2164 dropped thread.goal/thread/openWork); PROVIDER_BRIDGE_PROTOCOL_VERSION stays 2, the G3 snapshot loses the entry. The persisted turn/plan/updated type stays as read-only history the view already ignores.

Wire: HOST_DAEMON_PROTOCOL_VERSION stays at 150. Nothing this PR sends between server and daemon changed shape: tool_use has been in the pending-interaction schema since the contract PR, and DynamicTool.presentation rode the wire since #2164.

Interim UX to know about: until the presentation-driven projection lands, the legacy tool row shows the tool slot as its name, so a generic ACP row reads other/read/fetch where it used to read the agent's title (the title is in the persisted presentation.title, which that projection will read). A denied tool_use approval leaves no denied row in the timeline (the old fake command did); the agent's own tool_call_update (usually failed) is the record.

How you verified

Typecheck (turbo, --concurrency 4): bb-plugin-provider-acp, bb-plugin-provider-codex, bb-plugin-provider-claude-code, @bb/agent-runtime, @bb/server, @bb/provider-bridge-protocol, @get-bb/plugin-sdk. npm version guard: PASS (0.4.11). eslint + prettier on every changed file.

Tests: bb-plugin-provider-acp 196/196 (new: presentation coverage, native-kind mapping incl. title-path/URL recovery and the unfilled-shape fallbacks, bb-injected binding in Cursor order / proxy-first order / by-name / never-a-command, tool_use subjects incl. bound bb tool and in-flight-call headline); @bb/provider-bridge-protocol 217/217; @bb/agent-runtime 439/439; @bb/thread-view 385/385; @bb/domain, @bb/core-ui, @get-bb/plugin-sdk; @bb/server pending-interactions / internal-interactive-requests / public-thread-interactions (65/65, new: tool_use approval registers, appends no item event, resolves with deny).

Conformance (acp-cursor): scripted protocol suite against the fake agent: pass. Recorded conformance: all 11 committed cells replayed through this bridge (10 replayable + process-scoped model-list), every rule passes.

Parity A2 (pnpm parity --old ~/.bb/parity-old-main --new . --provider <id>; old = origin/main f6fb434):

  • acp-cursor 10 passed, 0 failed, 1 skipped, zero unlisted, zero stale. Allowlist entries (17, all #2179):
    • events /*/item/presentation × approval-allow, approval-deny, steer, stop-interrupt, subagent, turn-tools, user-question, web-search — presentation is new; items otherwise byte-identical.
    • events /*/item/tool × subagent, turn-tools, user-question, web-search — "Task: Subagent task"other, "Read File"read, "Edit File"edit, "MCP: tool"other, "Web Fetch"fetch; the title now rides presentation.title.
    • rows /0/children/1/toolName × subagent, turn-tools, user-question, web-search and rows /0/children/3/toolName × turn-tools — the legacy row projects the tool slot.
  • codex 16 passed, 0 failed, 1 skipped — zero new diffs, zero stale.
  • claude-code 13 passed, 0 failed, 1 skipped — zero new diffs, zero stale.
  • acp-cursor bridge→runtime.current lanes re-recorded (pnpm rerecord --provider acp-cursor --plan-with ~/.bb/parity-old-main); parity self-suite 43/43; row-counts.json pins unchanged.

Corpus A4: BB_PROVIDER_CORPUS_SNAPSHOT=compare → 307/307 threads, 93,262 rows, diffThreads: [], allowedDiffs: 0; timeline-perf gate 21/21 (load 4.4 on 16 cores). A4 cannot prove ACP: the extracted corpus is codex (166) + claude-code (141) only; the 29 ACP production threads (acp-grok 25, acp-cursor 2, acp-pi-acp 2) were not extracted. What A4 proves here is that this PR does not touch codex or claude rows.

G11: acp-cursor provider/unhandled 0 → 0 in every cell (pins unchanged); the one pre-existing grammar drop in turn-tools is 1 → 1. The production ACP unhandled rows (155, all grok: tool_call_update 83, agent_message_chunk 37, agent_thought_chunk 35) are outside-turn/visibility residue the kind mapping does not touch; no ACP corpus exists to re-measure them.

What the recordings cannot prove (needs live QA)

  • The native-kind → core-kind class is not in any recording. Cursor's read/fetch calls carry no path/URL, so in acp-cursor every such row stays a generic tool (the /*/item/tool class above). fileRead, webFetch, search, reasoning from ACP are covered by unit tests only. The "every read/edit/execute row changes kind" class the spec predicted did not materialize on this agent; grok (which titles reads with the path) is the agent that will exercise it.
  • Q31 binding is not replayable. The recorded user-question cell had 1 item/tool/call; a replay has 0 because the MCP proxy socket is not part of a recording, so the replayed row is other, not ask_user_question. The positional binding is unit-tested in both orders; live QA with Cursor + ask-user-question is needed.
  • tool_use is not in any recording: both approval cells are execute (command subject). Unit tests + the server lifecycle test cover it; no generic-permission cell exists for acp.
  • No plan-mode, compaction, or auth-failure cell for acp (not capturable at recording time); fork is recorded as the agent's refusal (fork: "tip", 0 events) and stays that way.
  • The positional Q31 binding is a heuristic: two parallel MCP calls announced in a different order than they execute would swap labels (never the flow).

AGENT GENERATED: by Claude Opus 5

SawyerHood and others added 5 commits August 21, 2026 10:54
Grammar v3 moves tool-name knowledge out of core thread-view and into the
bridge: the ACP bridge now states how each of its items reads (label while
pending, label once settled, a host glyph, and the agent's own title as the
headline) on every item.open and item.close, and the assembler persists that
snapshot with the item.

plugins/provider-acp/src/presentation.ts is the one place ACP's native
vocabulary maps to row presentation: a label pair and glyph per native kind
(read/edit/delete/move/search/execute/think/fetch/other), commands with the
agent's Markdown code ticks stripped from the headline, file changes by verb
(write/edit/delete) with their file names, and compactions. The tool item
classification is unchanged in this commit; the next one maps the native kind
enum onto the core kinds.

Co-Authored-By: Claude <noreply@anthropic.com>
…ot the tool

The core defect of the ACP bridge: it threw the agent's native kind enum
away and wrote the human title into the `tool` slot, so the production
timeline carries ~1,100 ACP rows whose "tool name" is "Read `/path`",
"Web search:", "Updating plan", "MCP: tool" or "tool". Now the kind maps
straight onto the core kinds (plugins/provider-acp/src/tool-classification.ts):

- `execute` → `command` and a diff or an `edit`/`delete` with a path →
  `fileChange`, as before (the shared operation classifier the permission
  mapping uses is untouched, #1803).
- `read` → `fileRead` when the agent names the path (`locations`, a
  `rawInput` path field, or a single code-ticked token in the title, which
  is how grok titles its reads); `search` → `search` when `rawInput` carries
  a pattern/query/regex (content) or glob (path); `fetch` → `webFetch` when
  `rawInput` or the title carries the URL; `think` → `reasoning` with the
  thought as its content.
- Everything else — `other`, `move`, an agent that sent no kind, or a kind
  whose shape the agent left unfilled (Cursor's `read`/`fetch` arrive with an
  empty rawInput and no locations) — is a generic `tool` whose `tool` slot is
  the kind and whose presentation is the kind's label with the agent's title
  as the headline. A row is never a `fileRead` without a path.
- An ACP `plan` update carries the whole entry list, so each is a settled
  `planSteps` snapshot (channel-keyed, latest supersedes), collapsed by
  default: the todo banner reads it, and `turn/plan/updated` — which the
  timeline excluded as noise — is no longer produced by this bridge.

Progress text still streams for every item that is not a command or a file
change, exactly the set that used to be `tool` items.

Co-Authored-By: Claude <noreply@anthropic.com>
…presentation

Q31 for the ACP bridge. ACP agents announce a call to a bb-injected tool as
a generic `other` tool_call with no tool name (Cursor: "MCP: tool", grok:
"tool"); the bb tool's identity is known only to the bridge's MCP proxy,
which forwards the call with its name, and nothing on the wire links the two.
The translator now learns the session's injected tools at construction
(`configureInjectedTools`) and binds each proxied call
(`noteInjectedToolCall`) to the agent's announcement positionally: the
unbound candidate whose title names the tool, else the one that mentions MCP,
else the oldest unbound `other` call; a proxied call with no candidate open
waits for the next announcement. A title that names an injected tool binds
at the announcement itself. The bound call's open or close reads
`{ server: "bb", tool: <name> }` with the presentation the server resolved
onto the DynamicTool definition (generic under bb's glyph when the definition
predates the field); a close re-states it, so a call announced before the
proxy saw it still settles as the bb tool. Commands, file changes, and the
native kinds are never candidates.

Co-Authored-By: Claude <noreply@anthropic.com>
An ACP permission for anything that is neither a command nor a file change
(an MCP tool, a read outside the project, a kind with no core shape) used to
be faked as a `command` approval — the one subject with free text — with
title → kind → fixed text as the "command". The v3 contract added the
`tool_use` subject for exactly this, and the ACP bridge now raises it: the
subject's `tool` and `presentation` come from the same classification its
timeline row gets (a bound bb tool reads as that tool with its definition's
presentation), so the banner and the row read alike. Real commands keep the
`command` subject and file changes keep `file_change`; a request with no
tool call at all still yields a grantable `tool_use` subject.

The server's interaction timeline stops refusing the subject: a tool-use
approval has no timeline item of its own — the provider's own tool call
(the ACP agent's tool_call with the same id) is the timeline record and the
banner renders the presentation — until WS5's interaction-lifecycle event.
The runtime conformance harness validates the subject instead of throwing,
and the "no producer yet" comments on the client renderers are updated.

No daemon wire change: the subject has been part of the pending-interaction
schema since the contract PR.

Co-Authored-By: Claude <noreply@anthropic.com>
The ACP bridge was the last speaker of `turn.plan`, the turn-level plan
delta that assembled to `turn/plan/updated` — an event the timeline excludes
as noise and the todo banner ignores. Codex moved its `update_plan` to the
`planSteps` item in WS1b-codex; the previous commit moved ACP `plan` updates
there too; pi never spoke it. So the grammar member goes, the way WS1b-codex
dropped `thread.goal` and `thread/openWork`: removed from the thread/delta
union and the assembler under the grammar range (every bridge is in-repo and
the assembler speaks v3 only), PROVIDER_BRIDGE_PROTOCOL_VERSION stays 2, the
G3 snapshot loses the entry and its header names the member. The persisted
`turn/plan/updated` event type stays as read-only history that the view
already knows how to ignore.

Co-Authored-By: Claude <noreply@anthropic.com>
@SawyerHood
SawyerHood force-pushed the bb/ws1b-acp-acp-bridge-to-v3-stack-on-2178-thr_9g9skmtvfh branch from 1280ad3 to 176cd63 Compare August 21, 2026 10:55
…bridge lanes

`pnpm parity --old ~/.bb/parity-old-main --new . --provider acp-cursor`
passes 10/10 replayable cells with 17 entries naming #2179, each one class
this PR intends:

- events `/*/item/presentation` (8 cells): every item carries its
  presentation; items are otherwise byte-identical.
- events `/*/item/tool` (subagent, turn-tools, user-question, web-search):
  the tool slot names the native kind instead of the agent's title, which
  now rides presentation.title. Cursor's read/fetch calls carry no path or
  URL, so in these recordings the rows stay generic tools; the
  fileRead/webFetch/search/reasoning mappings are covered by unit tests, not
  by a recording.
- rows `/0/children/N/toolName` (the same four cells): the legacy tool row
  projects the tool slot.

Codex (16/16) and claude-code (13/13) stay at zero diffs and zero stale
entries. G11: acp-cursor unhandled 0 → 0 in every cell; the pins in
row-counts.json are unchanged. The acp-cursor `bridge→runtime.current`
lanes are re-recorded from this bridge (`pnpm rerecord --provider
acp-cursor --plan-with ~/.bb/parity-old-main`); the recorded-conformance
and parity self-suites read them.

Co-Authored-By: Claude <noreply@anthropic.com>
@SawyerHood
SawyerHood marked this pull request as ready for review August 21, 2026 11:07
@SawyerHood

Copy link
Copy Markdown
Collaborator Author

Coordinator review — APPROVE (SlopCop disabled; this review is the gate). WS1b is complete: all three bridges are on grammar v3.

The core defect is fixed — verified: the line that wrote the human title into the tool slot (tool: toOptionalString(event.title) ?? event.kind ?? "tool") is gone, replaced by a real native-kind → core-kind mapping (readfileRead, searchsearch, fetchwebFetch, thinkreasoning) with the title in presentation.title. The defensive rule is the right call and is enforced: a kind whose required shape fields the agent didn't fill stays a generic tool presenting as that kind — never a fileRead without a path or a webFetch without a URL. turn.plan has no remaining speaker (verified across all bridges), so its deletion from the grammar is safe. HOST_DAEMON stays 150 — correct, no wire change.

Oracle: acp parity 10/10 with 17 allowlist entries, zero unlisted/stale; codex 16/16 and claude 13/13 with zero new diffs; A4 307/307 diffThreads: [] (and correctly stated that A4 cannot prove ACP — the corpus is codex+claude); G11 0→0 on every cell.

The honest part, and why it's acceptable: the headline mapping did not materialize on the Cursor recordings because Cursor's read/fetch calls carry no path/URL — so those rows stayed generic tool and the core-kind paths are unit-tested only. That is a limit of the recordings (flagged in the pre-staged spec), not a defect in this PR; the defensive rule is why it's safe to land. It gets proven by live QA with grok, the agent that fills those fields — I'm scheduling that with the codex/claude live-QA cells. Same for the Q31 positional binding (MCP proxy socket isn't recorded) and tool_use (neither approval cell exercises it).

Good catch on my instruction: the merge-base recipe I gave would have replayed the whole chain after the stack rebase; using your real old base was correct.

Known interim UX (documented): generic ACP rows show the kind slot (other/read) until presentation-driven projection lands in WS3; a denied tool_use leaves no denied row until WS5. Both are the designed next steps.

Lands after #2178. Sawyer merges; I do not.

AGENT GENERATED: by Claude Opus 4.8

@SawyerHood
SawyerHood merged commit 810cf2e into main Aug 21, 2026
13 checks passed
@SawyerHood
SawyerHood deleted the bb/ws1b-acp-acp-bridge-to-v3-stack-on-2178-thr_9g9skmtvfh branch August 21, 2026 14:20
SawyerHood added a commit that referenced this pull request Aug 21, 2026
The provider-plugin v3 stack (#2124#2179) landed on main and deleted 61
provider-id references from core: the reserved-id tables, the three Claude
model tables, the placeholder provider infos, the claude/codex settings
knobs on the shared contract, and the per-provider runtime branches the
registry and bridges now own. Regenerated baseline: 148 references across
40 core files (from 209 / 55). The ratchet holds it there.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SawyerHood added a commit that referenced this pull request Aug 21, 2026
## What was wrong

`plugins/provider-acp/server.ts` declared `fork: "tip"` for every
built-in ACP agent through `ACP_BASE_CAPABILITIES`. The server projects
that to `supportsFork: true` (the `POST /api/v1/threads/fork` gate and
the app's "Fork into new thread" action). But neither `cursor-agent acp`
(2026.08.11) nor `grok agent stdio` advertises
`sessionCapabilities.fork` in its ACP `initialize` reply, so the
bridge's per-session guard (`bridge.ts`: "does not advertise
session/fork support") rejected the fork only after the server had
already created and started the fork thread. The thread landed in
`status: error` with `system/error { code: thread_command_failed }`.
Issue: #1833. Report: https://get-bb.github.io/reports/issues/1833.html

The issue suggested deriving fork per agent at initialize time. That is
not a drop-in: the agent's `initialize` happens per session inside the
bridge after the server has created the thread, and there is no
daemon-to-server capability channel. The declaration is the server-side
source of truth (same as `supportsManualCompaction`), so it has to match
the agent.

## What changed

- `plugins/provider-acp/server.ts`: `acp-cursor` and `acp-grok` now
declare `fork: "none"`. The base stays `"tip"`; opencode, omp
(oh-my-pi), and hermes-agent all advertise `sessionCapabilities.fork` in
their ACP adapters (checked upstream sources:
`packages/opencode/src/acp/service.ts`,
`packages/coding-agent/src/modes/acp/acp-agent.ts`,
`acp_adapter/server.py`). Added a comment on the base capability
explaining why the declaration must match the agent.
-
`apps/server/test/services/plugins/first-party-provider-plugins.test.ts`:
the golden pin now records the fork ladder per first-party provider and
asserts `registration.serverCapabilities.fork`,
`registry.supportsFork()`, and the composed
`ProviderInfo.capabilities.supportsFork` against it.

Effects: `POST /threads/fork` on a Cursor or Grok thread returns HTTP
400 `Provider acp-cursor does not support thread forks` without creating
a thread; the app hides the fork action for those providers; the
declared ceiling that rides `bridgeLaunch.capabilities.fork` to the
daemon also becomes `"none"`, so the runtime rejects `thread/fork`
before spawning the agent.

No wire shape changes (the existing `fork` field just carries a
different value), so no `HOST_DAEMON_PROTOCOL_VERSION` bump. No CLI or
config surface changes. Custom `acp-*` agents still fall back to the
tier-wide `"tip"`; giving them a per-agent `fork` config field is a
separate change.

## How you verified

Probed the agents directly (same `initialize` request the bridge sends):

```
$ node probe.mjs cursor-agent acp     -> sessionCapabilities: { list: {} }                 fork advertised: false
$ node probe.mjs grok agent stdio     -> sessionCapabilities: { list, resume, close }      fork advertised: false
```

New test fails on origin/main with the test file applied and the plugin
unchanged:

```
FAIL test/services/plugins/first-party-provider-plugins.test.ts > first-party provider plugins > are the sole source of the built-in providers
AssertionError: acp-cursor: expected 'tip' to be 'none' // Object.is equality
```

Passes on this branch (`Tests 2 passed (2)`).

From the committed tree (`git status --porcelain` empty):

```
pnpm exec turbo run typecheck --filter=@bb/server --filter=bb-plugin-provider-acp
 Tasks:    6 successful, 6 total
pnpm exec turbo run test --filter=@bb/server --filter=bb-plugin-provider-acp
 bb-plugin-provider-acp: Tests 181 passed (181)
 @bb/server: Test Files 1 failed | 194 passed (195); Tests 1 failed | 1822 passed (1823)
```

The one server failure is `test/internal/internal-skill-trees.test.ts`
(`mode: 420` vs `436`), the known local umask 0002 failure unrelated to
this change; it passes here under `umask 022` and in CI.

Manual repro on this branch against a dev instance of this worktree:

```
$ curl -s $BB_SERVER_URL/api/v1/system/providers | ... supportsFork
codex True / claude-code True / pi True / acp-cursor False / acp-grok False
$ pnpm bb:dev thread spawn --provider acp-cursor ... --prompt "Reply only with ok."   -> thr_7u6gb8ppn2, reached idle
$ pnpm bb:dev thread fork thr_7u6gb8ppn2 --workspace reuse --prompt "Reply only with ok." --json
Error: Failed to fork thread thr_7u6gb8ppn2: HTTP 400: Provider acp-cursor does not support thread forks
$ GET /threads?projectId=...   -> only thr_7u6gb8ppn2 (idle); no fork thread created
```

Fixes #1833

> AGENT GENERATED: by Claude Opus 5


## Rebase

Rebased onto `origin/main` (`cf00cfe06`) after the grammar-v3 bridge
stack (#2148 `bb.providers.register` + single `ProviderInfo`, #2179
WS1b-acp) rewrote both touched files. The fix maps 1:1 onto the new
code:

- `plugins/provider-acp/server.ts`: the declarations now carry
`experimental_strings` / `experimental_serviceTiers` and register
through `bb.providers.register`, but `ACP_BASE_CAPABILITIES.fork: "tip"`
and the per-agent `capabilities` spreads are unchanged, so the `fork:
"none"` overrides for `acp-cursor` and `acp-grok` and the base comment
apply as before. Still no wire-shape change, so no
`HOST_DAEMON_PROTOCOL_VERSION` bump.
-
`apps/server/test/services/plugins/first-party-provider-plugins.test.ts`:
main dropped `supportsWorkflows` from the golden pin (conflict resolved
by adding only the `fork` ladder), and #2148 added a new test, "pins the
client-read ProviderInfo fields of the four core providers", that pinned
`acp-cursor` at `supportsFork: true`. That pin now reads `supportsFork:
false` with a `#1833` comment; `supportsSessionRewind: false` is
unchanged. The fork-ladder assertions in the first test
(`registration.serverCapabilities.fork`, `registry.supportsFork()`,
composed `ProviderInfo.capabilities.supportsFork`) carried over
verbatim; `serverCapabilities.fork` still exists on main
(`plugin-provider-registration.ts` projects `supportsFork:
capabilities.fork !== "none"`).

Re-verified on the new base. With `plugins/provider-acp/server.ts`
reverted to `origin/main` and the test file in place, the tests fail (so
main does not already cover the bug):

```
FAIL ... > are the sole source of the built-in providers
AssertionError: acp-cursor: expected 'tip' to be 'none' // Object.is equality
FAIL ... > pins the client-read ProviderInfo fields of the four core providers
AssertionError: expected { id: 'acp-cursor', …(8) } to strictly equal { id: 'acp-cursor', …(8) }
Tests 2 failed | 1 passed (3)
```

With the fix restored: `Tests 3 passed (3)`. From the committed tree
(`git status --porcelain` empty):

```
pnpm exec turbo run typecheck --filter=@bb/server --filter=bb-plugin-provider-acp
 Tasks:    6 successful, 6 total
pnpm exec turbo run test --filter=@bb/server --filter=bb-plugin-provider-acp
 bb-plugin-provider-acp: Test Files 15 passed (15); Tests 196 passed (196)
 @bb/server: Test Files 1 failed | 197 passed | 1 skipped (199); Tests 1 failed | 1896 passed (1897)
```

The one server failure is again
`test/internal/internal-skill-trees.test.ts` (`mode: 420` vs `436`), the
known local umask 0002 failure; it passes in CI.

## Independent verification

Verified by a second agent on a separate worktree (branch
`verify-1833-r1` = `ece689920`, `origin/main` is an ancestor,
`mergeable: MERGEABLE`).

Commands run:

```
git fetch origin main && git fetch origin bb/fix-1833-acp-cursor-fork && git checkout -b verify-1833-r1 FETCH_HEAD
pnpm install --frozen-lockfile --prefer-offline && pnpm exec turbo run build
node probe.mjs cursor-agent acp   -> cursor-agent sessionCapabilities: {"list":{}} fork advertised: false
node probe.mjs grok agent stdio   -> grok sessionCapabilities: {"list":{},"resume":{},"close":{}} fork advertised: false
git checkout origin/main -- plugins/provider-acp/server.ts
cd apps/server && pnpm exec vitest run test/services/plugins/first-party-provider-plugins.test.ts
git checkout HEAD -- plugins/provider-acp/server.ts
cd apps/server && pnpm exec vitest run test/services/plugins/first-party-provider-plugins.test.ts
pnpm exec turbo run typecheck --filter=@bb/server --filter=bb-plugin-provider-acp
pnpm exec turbo run test --filter=@bb/server --filter=bb-plugin-provider-acp --force
```

Fail-before / pass-after:

- With `plugins/provider-acp/server.ts` reverted to `origin/main` and
the PR's test file in place: `Tests 1 failed | 1 passed (2)` with
`AssertionError: acp-cursor: expected 'tip' to be 'none' // Object.is
equality`.
- With the PR's source restored: `Tests 2 passed (2)`.

Typecheck and tests:

- `turbo typecheck` for `@bb/server` and `bb-plugin-provider-acp`:
`Tasks: 6 successful, 6 total`.
- `turbo test`: `bb-plugin-provider-acp` 14 files passed; `@bb/server`
1822 passed, 1 failed. The single failure is
`test/internal/internal-skill-trees.test.ts` (`mode: 420` vs `436`), the
known umask-0002 environment failure on this workstation; it also fails
on clean `main` here and passes in CI.

Repro on the fixed branch (own dev instance, server :20051, data dir
under `~/.bb-dev`, deleted afterwards):

- `GET /api/v1/system/providers` -> `acp-cursor False`, `acp-grok False`
(codex/claude-code/pi still `True`).
- Spawned an `acp-cursor` thread (`thr_4gh8bikbv6`), waited to `idle`,
then `pnpm bb:dev thread fork thr_4gh8bikbv6 --workspace reuse --prompt
"Reply only with ok." --json` -> `HTTP 400: Provider acp-cursor does not
support thread forks`. Thread list for the project still holds only the
source thread (`idle`); no errored fork thread was created. The original
symptom no longer reproduces.

CI: 11 checks pass, 2 skipped (Node Compatibility Smoke, iOS simulator
flows), none failing.

Review notes:

- Root-cause fix at the right layer: the server-side declaration is the
fork gate and the app affordance, and its value also rides
`bridgeLaunch.capabilities.fork` to the runtime adapter's
`effectiveFork()` ceiling, so `thread/fork` is refused before the agent
is spawned. No wire shape change, so no protocol bump is needed.
- Residual risk: custom `acp-*` agents configured through
`customAcpAgents` still inherit the tier-wide
`ACP_TIER_CAPABILITIES.supportsFork: true`
(`apps/server/src/services/providers/acp-provider-tier.ts`), so a custom
agent without `session/fork` still hits the errored-thread path. A
per-agent `fork` field on that config is a sensible follow-up, not a
blocker for this PR. opencode/omp/hermes fork support was checked from
upstream sources only (not installed here).

> AGENT GENERATED: by Claude Opus 5



## Independent verification (post-rebase)

Re-verified by a second agent after the rebase onto the grammar-v3
bridge rewrite (branch `verify-2150-rb` = `2d84751d9`, one commit on
`cf00cfe06`). `origin/main` had moved one more commit (`fcada5a3b`,
#2120 provider-literal ratchet) by the time of this check; a local `git
merge --no-commit origin/main` was clean (the ratchet scans core only
and excludes `plugins/provider-*`), and GitHub reports `mergeable:
MERGEABLE`.

Does the fix still target the right code path after the v3 rewrite? Yes:

- `apps/server/src/services/threads/thread-fork.ts:39`
(`requireForkCapableProvider`) still gates `POST /threads/fork` on
`providerRegistry.supportsFork()`, which for a registered plugin
provider reads `registration.info.capabilities.supportsFork` = `fork !==
"none"` (`plugin-provider-registration.ts:211`).
- The app still reads `ProviderInfo.capabilities.supportsFork`
(`useForkThreadFromMessage.ts:54`, `ThreadDetailView.tsx:977`,
`RootComposeView.tsx:538`).
- `provider-bridge-launch.ts:48-50` still forwards
`registration.serverCapabilities.fork` to the daemon, and
`packages/agent-runtime/src/bridge-protocol-adapter.ts`
`effectiveFork()` still takes the declaration as a ceiling over the
handshake, so `thread/fork` is refused before cursor-agent is spawned.
- The v3 bridge (`plugins/provider-acp/src/bridge/bridge.ts`) still
advertises `fork: "tip"` unconditionally in its handshake (line ~2372)
and only checks `agentCapabilities.sessionCapabilities.fork` per session
at agent initialize (lines 1718-1721, "does not advertise session/fork
support"). Nothing in the rewrite made the static declaration redundant;
it is still the only up-front gate.

Commands run:

```
git fetch origin main && git fetch origin bb/fix-1833-acp-cursor-fork && git checkout -b verify-2150-rb FETCH_HEAD
pnpm install --frozen-lockfile --prefer-offline && pnpm exec turbo run build
git checkout origin/main -- plugins/provider-acp/server.ts
cd apps/server && pnpm exec vitest run test/services/plugins/first-party-provider-plugins.test.ts
git checkout HEAD -- plugins/provider-acp/server.ts
cd apps/server && pnpm exec vitest run test/services/plugins/first-party-provider-plugins.test.ts
pnpm exec turbo run typecheck --filter=@bb/server --filter=bb-plugin-provider-acp
pnpm exec turbo run test --filter=@bb/server --filter=bb-plugin-provider-acp --force
```

Fail-before / pass-after (only non-test source file is
`plugins/provider-acp/server.ts`):

- With `server.ts` from `origin/main` and the PR's test file: `Tests 2
failed | 1 passed (3)`. Failing assertions: `AssertionError: acp-cursor:
expected 'tip' to be 'none' // Object.is equality` and, in the #2148 pin
test, `AssertionError: expected { id: 'acp-cursor', …(8) } to strictly
equal { id: 'acp-cursor', …(8) }` (supportsFork true vs false).
- With the PR's source restored: `Tests 3 passed (3)`.

Typecheck and tests (from the committed tree, `git status --porcelain`
empty):

- `turbo typecheck`: `Tasks: 6 successful, 6 total`.
- `turbo test`: `bb-plugin-provider-acp` Test Files 15 passed (15);
`@bb/server` Tests 1 failed | 1896 passed (1897). The single failure is
`test/internal/internal-skill-trees.test.ts` (`mode: 420` vs `436`), the
known umask-0002 local failure; it passes in CI.

Repro on the fixed branch (own dev instance, server :26547, data dir
under `~/.bb-dev`, deleted afterwards):

- `GET /api/v1/system/providers` -> `codex True`, `claude-code True`,
`pi True`, `acp-cursor False`, `acp-grok False`.
- Spawned an `acp-cursor` thread (`thr_2c8kkaj7rz`) with cursor-agent
2026.08.11, waited to `idle`, then `pnpm bb:dev thread fork
thr_2c8kkaj7rz --workspace reuse --prompt "Reply only with ok." --json`
-> `HTTP 400: Provider acp-cursor does not support thread forks`. A
direct `POST /api/v1/threads/fork` also returned 400. The project's
thread list still holds only the source thread (`idle`); no errored fork
thread was created. The original symptom no longer reproduces.

CI on `2d84751d9`: all checks pass (server, packages, integration,
app-1/2/3, both Package Smoke runners, Checks, version check); iOS
simulator flows and Node Compatibility Smoke skipped; none pending or
failing.

Residual risks (unchanged from the pre-rebase review): custom `acp-*`
agents configured through `customAcpAgents` still fall back to
`ACP_TIER_CAPABILITIES.supportsFork: true`
(`apps/server/src/services/providers/acp-provider-tier.ts:43`) and can
still hit the errored-thread path; a per-agent `fork` field on that
config is a follow-up. No wire shape change (the `fork` field already
rides `bridgeLaunch.capabilities`; only its value for two providers
changes), so no `HOST_DAEMON_PROTOCOL_VERSION` bump is needed.

> AGENT GENERATED: by Claude Opus 5

Co-authored-by: Claude <noreply@anthropic.com>
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>
SawyerHood added a commit that referenced this pull request Aug 21, 2026
…2169)

## What was wrong

Every thread lifecycle transition broadcast `status-changed` as a bare
dirty flag (metadata carried at most `projectId`). The app's registry
rule for it (`dirtyActiveThreadListQueries`, flush `immediate`)
invalidated the single `sidebarNavigation` query plus every cached
thread list for the project. The sidebar query is always active
(`AppLayout` observes it with `staleTime: Infinity`), so each push
re-downloaded the whole `GET /api/v1/sidebar-bootstrap` document: about
1 KB per unarchived thread, 134 KB on the seeded database, twice per
turn (turn start, turn end), for every thread that runs a turn. Nothing
in the push let the client patch the one row that changed. Issue: #1302.
Report: https://get-bb.github.io/reports/issues/1302.html

## What changed

Server-to-app realtime contract (no daemon change; the host daemon does
not consume thread change notifications, so
`HOST_DAEMON_PROTOCOL_VERSION` is unchanged):

- `packages/domain/src/change-kinds.ts`: `threadChangeMetadataSchema`
gains optional `statusChange: { status, runtime, activity,
latestAttentionAt, updatedAt }` (the list-row fields a lifecycle
transition rewrites). The lenient inbound twin parses it with
`.catch(undefined)` so a client that does not know a future status or
runtime value drops just that field and falls back to a refetch.
`threadRuntimeStateSchema` and `threadActivityStateSchema` are now
exported from `thread.ts`.
- `apps/server/src/services/threads/thread-runtime-display.ts`:
`buildThreadStatusChangeMetadata(deps, thread)` builds the metadata in
one place. Runtime is resolved from host connectivity the same way list
rows do. Activity (background task counts plus the plan-mode and goal
counts) is built by the new `buildThreadActivityStateByThreadId`, which
`toThreadListEntryResponses` now also uses, so a pushed row and a
fetched row cannot disagree. The builder therefore takes the
prompt-banner deps (`db`, `hub`, `providerRegistry`); every caller
already had them through `AppDeps`/`WorkSessionDeps`
(`failThreadProvisioning` and `applyTurnCompletedEvent` widen their
`Pick`).
- `packages/db/src/data/threads.ts`: `applyThreadLifecycleEvent(db,
args)` no longer takes a notifier or notifies. The db package cannot
resolve the runtime (host connectivity lives in the hub), so the server
wrapper `applyLoggedThreadLifecycleEvent` (`lifecycle-outcome.ts`) now
owns the `status-changed` push and attaches the metadata. This covers
turn start (`run.started` from the daemon's `turn/started`), turn end
(`turn/completed`), provisioning, reconciliation and failure paths.
- `thread-send.ts`, `queued-messages.ts`, `parent-system-messages.ts`:
the three post-commit producers that activate a thread now carry the
activated row out of the transaction (`activeThread: Thread | null`
replaces `threadBecameActive: boolean`) and attach the metadata.
`queued-messages.ts` also drops a redundant `status-changed` notify that
fired inside the transaction, before commit; the post-commit notify on
the next line already sent the same kind.
- `packages/domain/src/plugin-sdk-version.ts` +
`packages/plugin-sdk/package.json`: no longer changed by this PR. The
new `statusChange` field does change the SDK's bundled types and
`dist/provider-bridge.js`, so the npm version guard
(`check-npm-version-guard.mjs`) needs an unpublished version. `main` has
since moved the SDK to `0.4.13`, which npm has not published (npm latest
is `0.4.12`), so this PR adopts `main`'s version and the guard passes
without a further bump.
- In-transaction writers whose hub is a `NotificationBuffer` (stop
requested, command failure, thread-start success, finalize, host-wide
interruption, environment cleanup, host reconnect fan-out) still send
the bare kind. The client falls back to today's refetch for those; they
are not on the per-turn hot path.
- `apps/app`: `realtime-cache-effects.ts` merges `statusChange` into the
dirty context. A `status-changed` message is last-writer-wins for it: a
later message that carries no row snapshot replaces (drops) an earlier
one merged while the document was hidden, so the resume flush refetches
instead of patching the row to the earlier, now-stale status.
`realtime-cache-registry.ts` replaces `dirtyActiveThreadListQueries` in
the `status-changed` rule with `patchThreadListStatusState`: with
metadata it writes the five fields into every cached thread list row and
the sidebar bootstrap (`updateCachedThreadListStatusState` in
`query-cache.ts`, same shape as the existing pending-interaction patch),
invalidates only list/sidebar queries that have a fetch in flight (that
fetch read the database before the transition and would overwrite the
patch when it lands), and still dirties the search prefix. Without
metadata it behaves exactly as before. Thread detail invalidation is
unchanged (about 600 B when the thread is open).

Revision after review (two findings, both fixed here):

1. The first draft's patch left `ThreadListEntry.activity` stale. The
plan-mode and goal counts are server-computed, gated on `status ===
"active"`, and were only synced by the list refetch the patch removed,
so a finished plan turn kept its sidebar indicator lit. The push now
carries the post-transition activity and the app patches it with the
rest of the row.
2. The hidden-document merge kept an earlier `statusChange` when a later
bare `status-changed` arrived (stop, command failure, interruption), so
on resume the row was patched to `active` and never refetched.
`statusChange` is now last-writer-wins per `status-changed` message.

Deviation from the report's proposal: the report suggested `status` +
`runtime` only. `latestAttentionAt` and `updatedAt` are included because
the lifecycle writer rewrites them and the sidebar sorts inactive rows
by `latestAttentionAt`; `activity` for the reason above. The report's
parts 2 (trim the bootstrap wire shape) and 3 (per-project sidebar keys)
are not in this PR; with no refetch per turn, the payload size only
matters on initial load and on membership changes.

Known, pre-existing: the sidebar learns that a plan turn is active only
from a list row fetched after the provider's `turn/input/accepted`
lands. The turn-start push (and on `main`, the turn-start refetch) is
built at send time, before that event exists, and `events-appended` does
not refetch lists, so the plan-mode glyph at turn start was already a
race on `main`. This PR keeps that behavior and fixes the indicator
turning off at turn end. Pushing an activity patch on the accepted/goal
events is a separate follow-up.

## How you verified

Tests added:

- `apps/app/src/hooks/realtime-cache-effects.test.ts`: "patches cached
thread list status from notification metadata instead of refetching the
sidebar bootstrap". Fails on `origin/main` app sources with
`AssertionError: expected "vi.fn()" to be called 1 times, but got 2
times` (the sidebar query fn was refetched); passes after. The pushed
`statusChange` in this test carries `activity.activePlanModeCount: 1`
and the row assertion covers it. Also: "refetches thread lists for a
status change that carries no row metadata" (guards the fallback),
"restarts a sidebar fetch already in flight so its stale snapshot cannot
overwrite the patched status", and (hidden document) "refetches when a
bare status-changed follows one that carried the row". The last fails on
the first draft's merge with `AssertionError: expected { activity: {
…(5) }, …(5) } to be { activity: { …(5) }, …(5) } // Object.is equality`
(the idle row had been replaced by the patched active row); passes
after.
- `apps/server/test/services/threads/lifecycle-outcome.test.ts` (new):
`applyLoggedThreadLifecycleEvent` broadcasts `status-changed` with
`projectId` and the full `statusChange` (runtime `active` with a
registered daemon, `waiting-for-host` without), nothing when the event
is not applied, and "carries the status-gated plan and goal activity of
the post-transition row": with an open accepted `/plan` turn and an
active goal on record, `run.started` pushes `activePlanModeCount: 1,
activeGoalCount: 1` and `run.succeeded` pushes `status: idle` with
`activePlanModeCount: 0, activeGoalCount: 1`. On the first draft's
server builder the broadcast is rejected by the strict schema
(`ZodError: Invalid input: expected object, received undefined` for
`activity`); on `main` the first assertion fails because the db notify
carried only `projectId`.
- `packages/domain/test/change-kinds.test.ts`: maximal fixture extended
(the parity guard requires it) plus "drops a status change a stale
client cannot parse but keeps the message".
- `packages/db/test/data/thread-lifecycle.test.ts`: the notify assertion
moved to the server test; call sites updated for the new signature (also
`tests/integration/fake/recovery/idle-error-reconciliation.test.ts`,
which polls the API and does not depend on the push; ran it, passes).

Commands (on the committed tree, rebased on current `origin/main`, `git
status --porcelain` empty):

- `pnpm exec turbo run typecheck` (whole repo): `Tasks: 72 successful,
72 total`.
- `pnpm exec turbo run test --filter=@bb/domain --filter=@bb/db
--filter=@bb/server --filter=@bb/app --continue`: `Tasks: 9 successful,
10 total`. domain 136 passed; db 405 passed; app 3160 passed (3
skipped); server 1825 passed, 2 failed:
`test/internal/internal-skill-trees.test.ts` (`mode: 420` vs `436`, a
local umask 0002 artifact unrelated to this change, passes with `umask
022`) and `test/services/plugins/plugin-update.test.ts` "waits one full
interval" (`Test timed out in 5000ms` under the full parallel run;
passes alone, 27/27).

Manual check on my dev instance (scratch project, one codex thread, a
dedicated headless Chromium profile with a `window.fetch` logger
installed after load, then `POST /api/v1/threads/:id/send` with a `/plan
Reply only with ok.` command mention from the driving script, sampling
the sidebar row's indicator labels every 100 ms):

- Before (report, same experiment with plain `tell`): 19 requests, 2 ×
`GET /api/v1/sidebar-bootstrap` at 134,865 B and 134,861 B (96% of
bytes), plus child/fork list refetches at turn start and end.
- After: 10 requests, **0** `sidebar-bootstrap` calls, no thread list
refetches; the turn traffic is the thread detail (588/584 B), timeline
deltas, outline, prompt history, PR state and read receipt. The sidebar
row showed `Thread working` 123 ms after the send and cleared it at turn
end (2.3 s), from the pushed patch alone. At 1.5 s `GET
/threads?projectId=` reported `status: active, activePlanModeCount: 1`;
after the turn the row carried no stale plan indicator. Log saved at
`/tmp/bb-fix-batch/issues/1302/revise-plan-turn-api-log.json`.

Fixes #1302

> AGENT GENERATED: by Claude Opus 5


## Independent verification

Verified round 2 at head `1e1e56ff7` (rebased on `origin/main`
`c942421a4`; `git merge-base --is-ancestor origin/main HEAD` true,
GitHub reports MERGEABLE) in a fresh worktree.

Commands:

- `git fetch origin main && git fetch origin
bb/fix-1302-sidebar-bootstrap && git checkout -b verify-1302-r2
FETCH_HEAD`; `pnpm install --frozen-lockfile --prefer-offline`; `pnpm
exec turbo run build`.
- Fail-before: `git checkout origin/main -- <13 non-test source files>`
then `pnpm exec vitest run src/hooks/realtime-cache-effects.test.ts`
(apps/app): 1 failed / 56 passed, `AssertionError: expected "vi.fn()" to
be called 1 times, but got 2 times` ("patches cached thread list status
from notification metadata instead of refetching the sidebar
bootstrap"). `pnpm exec vitest run
test/services/threads/lifecycle-outcome.test.ts` (apps/server): 3 failed
/ 1 passed; the broadcast metadata was `{ projectId }` only (`-
"statusChange": { … }` in the assertion diff), `expected undefined to be
'waiting-for-host'`, `expected undefined to deeply equal {
activeGoalCount: 1, activePlanModeCount: 1, … }`.
- Revision check: with the first draft's `realtime-cache-effects.ts`
(`fdfaec771`) checked out, `-t "bare status-changed follows"` fails with
`AssertionError: expected { activity: { …(5) }, …(5) } to be { activity:
{ …(5) }, …(5) } // Object.is equality`.
- Pass-after (`git checkout HEAD -- …`, tree clean): app file 57/57,
server file 4/4.
- `pnpm exec turbo run typecheck --filter=@bb/domain --filter=@bb/db
--filter=@bb/server --filter=@bb/app --filter=@bb/integration-tests
--filter=@bb/mobile --filter=@bb/sdk --filter=@bb/desktop`: `Tasks: 12
successful, 12 total`.
- `pnpm exec turbo run test --filter=@bb/domain --filter=@bb/db
--filter=@bb/server --filter=@bb/app --continue`: domain 136, db 405,
app 3160 (3 skipped) passed; server 1826 passed / 1 failed =
`test/internal/internal-skill-trees.test.ts` (`mode: 420` vs `436`, the
known local umask 0002 artifact; passes in CI).
- CI on the PR: all checks pass (Checks, Package Smoke x2, Tests
app-1/2/3, integration, packages, server).

Repro on the fixed branch (own dev instance :18681/:26681/:34681,
scratch project, one codex thread, headless Chromium with a
`window.fetch` logger installed after load, `pnpm bb:dev thread tell
<id> "Reply only with ok."` from the shell, 100 ms DOM poll of the
sidebar row): three sends, each 9-10 API requests and **0** `GET
/api/v1/sidebar-bootstrap` (report on main: 19 requests, 2 bootstrap
downloads = 96% of bytes). The sidebar row showed `Thread working` about
100 ms after the send, `Unread thread succeeded` at turn end and cleared
after the read receipt, all from the pushed patch. No longer reproduces.

Review notes: server-to-app contract only; the host daemon does not
consume thread `changed` messages, so no `HOST_DAEMON_PROTOCOL_VERSION`
bump is needed; every inbound consumer (app, mobile, desktop, sdk) uses
the lenient schema; thread lists are ordered by pin/createdAt and not
filtered on status, so patching cannot change membership; all
`applyThreadLifecycleEvent` callers updated. Residual (documented in the
body): in-transaction producers (stop, command failure, thread-start
success, interruption, env cleanup, host reconnect) still push the bare
kind and refetch the whole bootstrap; the plan-mode glyph at turn start
stays a pre-existing race; the 138 KB payload shape and single sidebar
key (report parts 2 and 3) are untouched, so a reviewer may prefer to
keep #1302 open for the payload trim. Nit: `thread-runtime-display.ts`
L268-270 is not prettier-formatted (CI does not enforce it).

> AGENT GENERATED: by Claude Opus 5


## Rebase

Rebased onto `origin/main` `75d6fc4d4` (was 32 commits behind at
`c942421a4`) and squashed the two commits into one (`766f1928f`); the
commit message keeps the original subject and body and folds in the
revision-round notes (activity on the push, last-writer-wins merge).
`git rebase` applied cleanly with no conflicts: none of the 32 commits
on main touched the 23 files in this diff. The commits on main in the
neighbouring areas (`apps/server/src/services/threads`,
`packages/domain/src`, `apps/app/src/hooks`) are the provider v3
contract stack (#2124, #2136, #2148, #2164, #2179), the late tool-call
completion fix (#2176) and the acp fork capability change (#2150); they
do not touch the lifecycle writer, the thread change-kind schema, or the
realtime cache registry, so the fix maps onto the new base unchanged.
`origin/main` still has no `statusChange` in `change-kinds.ts` or
`realtime-cache-registry.ts`.

Re-proved on the new base (committed tree, `git status --porcelain`
empty):

- Fail-before: with the 13 non-test source files checked out from
`origin/main`, `apps/app` `realtime-cache-effects.test.ts`: 1 failed /
56 passed, `AssertionError: expected "vi.fn()" to be called 1 times, but
got 2 times`; `apps/server` `lifecycle-outcome.test.ts`: 3 failed / 1
passed (`expected undefined to be 'waiting-for-host'`, `expected
undefined to deeply equal { Object (activeBackgroundAgentCount, ...)
}`). Pass-after: 57/57 and 4/4.
- `pnpm exec turbo run typecheck --filter=@bb/domain --filter=@bb/db
--filter=@bb/server --filter=@bb/app --filter=@bb/integration-tests
--filter=@bb/mobile --filter=@bb/sdk --filter=@bb/desktop
--filter=@bb/host-daemon --filter=@bb/cli`: `Tasks: 14 successful, 14
total`.
- `pnpm exec turbo run test --filter=@bb/domain --filter=@bb/db
--filter=@bb/server --filter=@bb/app --continue`: domain 27/27 files, db
28/28 files; server 1899 passed / 2 failed; app 3190 passed / 3 failed
(3 skipped). The machine was under a load average of 40-70 from parallel
agents: every app failure and the `plugin-update.test.ts` server failure
were `Test timed out` in files unrelated to this change (different files
on each of two runs), and each passes when rerun alone (135/135, 27/27).
The one remaining server failure is
`test/internal/internal-skill-trees.test.ts` (`mode: 420` vs `436`), the
known local umask 0002 artifact that passes in CI.

> AGENT GENERATED: by Claude Opus 5




## Independent verification (guards)

Verified head `17746125f` (rebased onto `origin/main` `27d1017fe`; `git
merge-base --is-ancestor origin/main HEAD` true; GitHub reports
`MERGEABLE` / `CLEAN`) in a fresh worktree. Scope: confirm the
post-verification change is only the CI-guard fix, re-prove
fail-before/pass-after on the new head, confirm CI.

- Interdiff: `git diff 75d6fc4 766f192` (previously verified patch)
vs `git diff origin/main 1774612` differ by exactly two hunks:
`packages/domain/src/plugin-sdk-version.ts` `PLUGIN_SDK_VERSION =
"0.4.11"` → `"0.4.12"` and `packages/plugin-sdk/package.json`
`"version": "0.4.11"` → `"0.4.12"`. No other line of the PR changed.
`origin/main` and `npm view @get-bb/plugin-sdk version` are both
`0.4.11`; `@get-bb/plugin-sdk@0.4.12` is 404 on npm, so the patch bump
targets the next unpublished version. The commit keeps the original
subject, body, and `Co-Authored-By` trailer.
- CI on `17746125f`: all checks pass (Checks, Package Smoke x2, Tests
app-1/2/3, integration, packages, server, Version Lockstep x2; Node
Compatibility Smoke and iOS simulator flows skipped by design). The
`Check plugin SDK npm version guard` step logs `npm version guard: PASS
— @get-bb/plugin-sdk@0.4.12 is not on npm yet. The publish job will ship
this version.`
- Fail-before on the new head (`git checkout origin/main -- <15 non-test
source files>`): `packages/domain` `change-kinds.test.ts` 2 failed / 6
passed (`ZodError` on the maximal strict `thread` fixture, `expected [
'backgroundActivityChanged', …(4) ] to deeply equal [ …(3) ]`);
`apps/server` `lifecycle-outcome.test.ts` 3 failed / 1 passed (assertion
diff shows the broadcast `metadata` is `{ projectId }` only, `-
"statusChange": { activity, latestAttentionAt, runtime, status,
updatedAt }`); `apps/app` `realtime-cache-effects.test.ts` +
`cache-owner-registry.test.ts` 2 failed / 59 passed (`AssertionError:
expected "vi.fn()" to be called 1 times, but got 2 times` at
`realtime-cache-effects.test.ts:1905`).
- Pass-after (`git checkout HEAD -- …`, `git status --porcelain` empty):
domain 8/8, server 4/4, app 61/61. `pnpm exec turbo run typecheck
--filter=@bb/domain --filter=@get-bb/plugin-sdk`: `Tasks: 5 successful,
5 total`. `pnpm exec turbo run build`: `Tasks: 18 successful, 18 total`.
- Repro on the fixed branch: not re-run this round; the fix code is
byte-identical to the head whose browser repro (0 `GET
/api/v1/sidebar-bootstrap` per send) is recorded above.

Residual risks unchanged from the sections above. This PR no longer
carries an SDK version change; `main`'s unpublished `0.4.13` covers it.


## Rebase (2026-08-21)

Rebased onto `main` at `d41d1abee`. Only
`packages/domain/src/plugin-sdk-version.ts` and
`packages/plugin-sdk/package.json` conflicted, because `main` moved the
SDK from `0.4.12` to `0.4.13`. Both were resolved to `main`'s values, so
the version files have dropped out of this PR's diff entirely (25
changed files -> 23). No other line of the fix changed.

Re-verified on the new base: `node
packages/plugin-sdk/scripts/check-npm-version-guard.mjs` -> `PASS -
@get-bb/plugin-sdk@0.4.13 is not on npm yet`. `pnpm exec turbo run
typecheck --filter=@bb/app --filter=@bb/server --filter=@bb/db
--filter=@bb/domain --filter=@bb/integration-tests`: `Tasks: 9
successful, 9 total`. `pnpm exec turbo run test` for
app/server/db/domain: domain 27/27 files, db 28/28, server pass;
`@bb/app` reported one failure in `PromptBoxInternal.test.tsx >
selection reveal`, which passes on its own re-run and touches no file in
this PR (the app changes are confined to `src/hooks/cache-owners/`).
Treated as load-dependent flake; CI is the arbiter.

> AGENT GENERATED: by Claude Opus 5

Co-authored-by: Claude <noreply@anthropic.com>
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.

1 participant