Stop advertising fork for acp-cursor and acp-grok - #2150
Merged
Conversation
SawyerHood
marked this pull request as ready for review
August 21, 2026 05:24
plugins/provider-acp declared fork: "tip" for every built-in ACP agent, so
the registry, POST /threads/fork, and the app all offered fork for Cursor
and Grok. Neither cursor-agent nor `grok agent stdio` advertises ACP
sessionCapabilities.fork at initialize, so the bridge refused session/fork
only after the server had already created and started the fork thread,
which then landed in status error ("does not advertise session/fork
support"). opencode, omp, and hermes-agent do advertise fork and keep "tip".
Declare fork: "none" for acp-cursor and acp-grok so the server rejects the
fork up front (HTTP 400, no thread) and the app hides the action. Pin the
per-provider fork ladder in the first-party provider plugin golden test.
Fixes #1833
Co-Authored-By: Claude <noreply@anthropic.com>
SawyerHood
force-pushed
the
bb/fix-1833-acp-cursor-fork
branch
from
August 21, 2026 14:46
ece6899 to
2d84751
Compare
This was referenced Aug 21, 2026
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>
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
plugins/provider-acp/server.tsdeclaredfork: "tip"for every built-in ACP agent throughACP_BASE_CAPABILITIES. The server projects that tosupportsFork: true(thePOST /api/v1/threads/forkgate and the app's "Fork into new thread" action). But neithercursor-agent acp(2026.08.11) norgrok agent stdioadvertisessessionCapabilities.forkin its ACPinitializereply, 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 instatus: errorwithsystem/error { code: thread_command_failed }. Issue: #1833. Report: https://get-bb.github.io/reports/issues/1833.htmlThe issue suggested deriving fork per agent at initialize time. That is not a drop-in: the agent's
initializehappens 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 assupportsManualCompaction), so it has to match the agent.What changed
plugins/provider-acp/server.ts:acp-cursorandacp-groknow declarefork: "none". The base stays"tip"; opencode, omp (oh-my-pi), and hermes-agent all advertisesessionCapabilities.forkin 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 assertsregistration.serverCapabilities.fork,registry.supportsFork(), and the composedProviderInfo.capabilities.supportsForkagainst it.Effects:
POST /threads/forkon a Cursor or Grok thread returns HTTP 400Provider acp-cursor does not support thread forkswithout creating a thread; the app hides the fork action for those providers; the declared ceiling that ridesbridgeLaunch.capabilities.forkto the daemon also becomes"none", so the runtime rejectsthread/forkbefore spawning the agent.No wire shape changes (the existing
forkfield just carries a different value), so noHOST_DAEMON_PROTOCOL_VERSIONbump. No CLI or config surface changes. Customacp-*agents still fall back to the tier-wide"tip"; giving them a per-agentforkconfig field is a separate change.How you verified
Probed the agents directly (same
initializerequest the bridge sends):New test fails on origin/main with the test file applied and the plugin unchanged:
Passes on this branch (
Tests 2 passed (2)).From the committed tree (
git status --porcelainempty):The one server failure is
test/internal/internal-skill-trees.test.ts(mode: 420vs436), the known local umask 0002 failure unrelated to this change; it passes here underumask 022and in CI.Manual repro on this branch against a dev instance of this worktree:
Fixes #1833
Rebase
Rebased onto
origin/main(cf00cfe06) after the grammar-v3 bridge stack (#2148bb.providers.register+ singleProviderInfo, #2179 WS1b-acp) rewrote both touched files. The fix maps 1:1 onto the new code:plugins/provider-acp/server.ts: the declarations now carryexperimental_strings/experimental_serviceTiersand register throughbb.providers.register, butACP_BASE_CAPABILITIES.fork: "tip"and the per-agentcapabilitiesspreads are unchanged, so thefork: "none"overrides foracp-cursorandacp-grokand the base comment apply as before. Still no wire-shape change, so noHOST_DAEMON_PROTOCOL_VERSIONbump.apps/server/test/services/plugins/first-party-provider-plugins.test.ts: main droppedsupportsWorkflowsfrom the golden pin (conflict resolved by adding only theforkladder), and [stacked on #2153 ← #2136 ← #2124] WS2a: bb.providers.register + single ProviderInfo #2148 added a new test, "pins the client-read ProviderInfo fields of the four core providers", that pinnedacp-cursoratsupportsFork: true. That pin now readssupportsFork: falsewith a#1833comment;supportsSessionRewind: falseis unchanged. The fork-ladder assertions in the first test (registration.serverCapabilities.fork,registry.supportsFork(), composedProviderInfo.capabilities.supportsFork) carried over verbatim;serverCapabilities.forkstill exists on main (plugin-provider-registration.tsprojectssupportsFork: capabilities.fork !== "none").Re-verified on the new base. With
plugins/provider-acp/server.tsreverted toorigin/mainand the test file in place, the tests fail (so main does not already cover the bug):With the fix restored:
Tests 3 passed (3). From the committed tree (git status --porcelainempty):The one server failure is again
test/internal/internal-skill-trees.test.ts(mode: 420vs436), 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/mainis an ancestor,mergeable: MERGEABLE).Commands run:
Fail-before / pass-after:
plugins/provider-acp/server.tsreverted toorigin/mainand the PR's test file in place:Tests 1 failed | 1 passed (2)withAssertionError: acp-cursor: expected 'tip' to be 'none' // Object.is equality.Tests 2 passed (2).Typecheck and tests:
turbo typecheckfor@bb/serverandbb-plugin-provider-acp:Tasks: 6 successful, 6 total.turbo test:bb-plugin-provider-acp14 files passed;@bb/server1822 passed, 1 failed. The single failure istest/internal/internal-skill-trees.test.ts(mode: 420vs436), the known umask-0002 environment failure on this workstation; it also fails on cleanmainhere 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 stillTrue).acp-cursorthread (thr_4gh8bikbv6), waited toidle, thenpnpm 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:
bridgeLaunch.capabilities.forkto the runtime adapter'seffectiveFork()ceiling, sothread/forkis refused before the agent is spawned. No wire shape change, so no protocol bump is needed.acp-*agents configured throughcustomAcpAgentsstill inherit the tier-wideACP_TIER_CAPABILITIES.supportsFork: true(apps/server/src/services/providers/acp-provider-tier.ts), so a custom agent withoutsession/forkstill hits the errored-thread path. A per-agentforkfield 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).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 oncf00cfe06).origin/mainhad moved one more commit (fcada5a3b, #2120 provider-literal ratchet) by the time of this check; a localgit merge --no-commit origin/mainwas clean (the ratchet scans core only and excludesplugins/provider-*), and GitHub reportsmergeable: 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 gatesPOST /threads/forkonproviderRegistry.supportsFork(), which for a registered plugin provider readsregistration.info.capabilities.supportsFork=fork !== "none"(plugin-provider-registration.ts:211).ProviderInfo.capabilities.supportsFork(useForkThreadFromMessage.ts:54,ThreadDetailView.tsx:977,RootComposeView.tsx:538).provider-bridge-launch.ts:48-50still forwardsregistration.serverCapabilities.forkto the daemon, andpackages/agent-runtime/src/bridge-protocol-adapter.tseffectiveFork()still takes the declaration as a ceiling over the handshake, sothread/forkis refused before cursor-agent is spawned.plugins/provider-acp/src/bridge/bridge.ts) still advertisesfork: "tip"unconditionally in its handshake (line ~2372) and only checksagentCapabilities.sessionCapabilities.forkper 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:
Fail-before / pass-after (only non-test source file is
plugins/provider-acp/server.ts):server.tsfromorigin/mainand the PR's test file:Tests 2 failed | 1 passed (3). Failing assertions:AssertionError: acp-cursor: expected 'tip' to be 'none' // Object.is equalityand, in the [stacked on #2153 ← #2136 ← #2124] WS2a: bb.providers.register + single ProviderInfo #2148 pin test,AssertionError: expected { id: 'acp-cursor', …(8) } to strictly equal { id: 'acp-cursor', …(8) }(supportsFork true vs false).Tests 3 passed (3).Typecheck and tests (from the committed tree,
git status --porcelainempty):turbo typecheck:Tasks: 6 successful, 6 total.turbo test:bb-plugin-provider-acpTest Files 15 passed (15);@bb/serverTests 1 failed | 1896 passed (1897). The single failure istest/internal/internal-skill-trees.test.ts(mode: 420vs436), 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.acp-cursorthread (thr_2c8kkaj7rz) with cursor-agent 2026.08.11, waited toidle, thenpnpm 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 directPOST /api/v1/threads/forkalso 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 throughcustomAcpAgentsstill fall back toACP_TIER_CAPABILITIES.supportsFork: true(apps/server/src/services/providers/acp-provider-tier.ts:43) and can still hit the errored-thread path; a per-agentforkfield on that config is a follow-up. No wire shape change (theforkfield already ridesbridgeLaunch.capabilities; only its value for two providers changes), so noHOST_DAEMON_PROTOCOL_VERSIONbump is needed.