Present ACP file-write permissions as file-change approvals by SawyerHood · Pull Request #1803 · get-bb/bb

SawyerHood · 2026-08-18T18:28:15Z

What was wrong

When an ACP agent (observed with OpenCode) asked permission to write a file, bb rendered the request as a command approval whose "command" was a bare directory path, and the timeline showed the item as commandExecution. The ACP bridge only forwarded the tool call's title/kind into the interaction payload and dropped toolCall.locations[].path, so buildAcpPermissionInteractionPayload had no file information and fell back to the command shape for every non-shell request. Report: https://get-bb.github.io/reports/issues/1719.html

What changed

All changes are in plugins/provider-acp; no server/daemon wire shape changed (no HOST_DAEMON_PROTOCOL_VERSION bump).

src/tool-call-operation.ts (new): one classifier, classifyAcpToolCall, that yields command / file_change (with the named paths) / generic. Both the timeline translator and the permission mapping use it, so an approval row and its timeline item can never disagree. move and location-free edit stay generic in both. resolveAcpFileChangeWriteScope normalizes paths, drops blank ones, and returns the one named path that contains all the others (linear scan) or null — so .. segments, cross-directory sets, and lookalike prefixes never produce a false write root.
src/bridge/bridge.ts: handlePermissionRequest forwards toolCall.locations[].path into the normalized permission tool call, and passes the in-flight edit tool call with the same id (from translator.resolveState().toolCallEventsByCallId) as the positive write signal for OpenCode's external_directory request, whose own kind is the generic other.
src/interactions.ts: AcpPermissionToolCall gains locations; buildAcpPermissionInteractionPayload returns a file_change subject (itemId = tool-call id, writeScope from the resolver, sessionGrant: null) when the shared classifier says file change; a request with a shell command, or an other-kind tool without a write signal, keeps the command fallback.
src/event-translation.ts: translateAcpToolCallItem uses the shared classifier (same behavior as before for existing shapes).
src/bridge/fake-acp-agent.mjs: fixture support for the external_directory permission shape.

Deviation from the report: the report proposed classifying edit/delete/move, or other+locations, as file changes. SlopCop's review showed that rule diverges from the timeline classifier and treats read-only other tools as writes, so it was replaced by the shared classifier above (findings addressed on the inline threads).

Tests

src/tool-call-operation.test.ts (new): classifier and write-scope cases — move, location-free edit, generic other+locations, .. / cross-directory / lookalike-prefix paths, blank paths.
src/interactions.test.ts: OpenCode edit-kind write → file_change; OpenCode external_directory (other kind, bare directory title) with the in-flight edit → file_change with writeScope: "/tmp/qa-1719"; edit without locations → writeScope: null; a shell-command request stays command. The three file-change cases failed before the change (subject.kind === "command").
src/bridge/bridge.test.ts: end-to-end bridge test for the OpenCode external_directory shape — asserts the file_change subject and that Allow settles the ACP request.

How I verified

pnpm exec turbo run lint typecheck test --filter=bb-plugin-provider-acp --force → 9 files / 161 tests pass, lint/typecheck/prettier clean. CI green.

Fixes #1719

AGENT GENERATED: by Claude Opus 5

bb-slop-cop · 2026-08-18T19:14:49Z

🚨 SLOP COP 🚨 · review

I am SlopCop, and I am reviewing this change now. I will check security, code quality, performance, architecture, duplication, and the user flow.

bb-slop-cop · 2026-08-18T19:34:23Z

+  if (!locations || locations.length === 0) {
+    return null;
+  }
+  const root = locations.find((candidate) =>


🚨 slopcop/review — This raw prefix test can show a false write boundary. For example, /repo/../secret passes under /repo. A cross-directory move also falls back to the first path and hides the second path. The nested find and every calls also make this work O(n²) for an unbounded list. Please normalize the paths and use a linear common-parent check. Return null when the code cannot prove a common scope.

Done: resolveAcpFileChangeWriteScope normalizes each path with path.normalize, picks the shortest path in one pass, and checks entry.startsWith(candidate + sep) in one more pass (O(n)); it returns null when any path is outside the candidate. Tests cover /repo/../secret, cross-directory pairs, and the /tmp/qa-17190 lookalike prefix.

bb-slop-cop · 2026-08-18T19:34:24Z

+    return true;
+  }
+  return (
+    (toolCall.kind === undefined || toolCall.kind === "other") &&


🚨 slopcop/review — This rule treats every command-free other tool with locations as a file change. ACP uses other as a generic kind, and locations can refer to read-only files. The prompt can therefore describe a read as a write. Please use one operation classifier for approval and event conversion. You can then add a narrow OpenCode rule for the external-directory case.

Done: other with locations is now generic. The positive write signal for opencode's external_directory case is the in-flight edit tool call with the same id, which the bridge reads from translator state and passes as startedToolCall. A read-only other/read tool stays a command-fallback approval. Tests: keeps a generic other-kind permission with locations as a command subject, plus a bridge test that runs the opencode shape end to end.

bb-slop-cop · 2026-08-18T19:34:24Z

+        other.startsWith(candidate.endsWith("/") ? candidate : `${candidate}/`),
+    ),
+  );
+  return toOptionalString(root ?? locations[0]) ?? null;


🚨 slopcop/review — A location array can contain path: "". The provider-bridge toOptionalString helper does not trim or reject empty text. This line can therefore return writeScope: "". The canonical schema requires at least one character, so the runtime rejects the approval before the user can answer it. Please remove blank paths at the bridge boundary.

Done: blank paths are dropped in extractAcpToolCallPaths and again in resolveAcpFileChangeWriteScope, so writeScope is never "". Tests: ignores blank paths and never yields an empty scope and uses a null write scope when a blank location path is the only one.

bb-slop-cop · 2026-08-18T19:34:34Z

+  if (toolCall.command !== undefined) {
+    return false;
+  }
+  if (


🚨 slopcop/review — This classifier includes move and location-free edits. The event converter only emits a file-change item for edit or delete with a path. One item can therefore appear as a file change during approval and as a generic tool later. The server can leave the approval row open. Please share one classifier between the permission and event paths.

Done: added src/tool-call-operation.ts with one classifyAcpToolCall that both event-translation.ts and interactions.ts use. Approval and timeline now agree: move and a location-free edit stay generic (command fallback), so no row flips type. Tests: keeps a move-kind permission as a command subject, keeps an edit-kind permission without any path as a command subject.

bb-slop-cop

🚨 SLOP COP 🚨 · review

Plain English summary: This PR changes ACP file-write prompts. A user sees a file-change card instead of a shell-command card. The normal OpenCode case now shows the correct prompt and directory.

I found four defects in less common ACP inputs:

High: The approval classifier and the timeline classifier use different rules. A move, a location-free edit, or a generic tool can leave a file-change approval row and create a generic tool row.
High: ACP uses other as a generic tool kind. A read-only other tool with locations now looks like a file write.
High: The scope code compares raw path text. Paths with .. or paths in different directories can show a false Write root value.
Medium: A location with an empty path creates writeScope: "". The canonical schema rejects that request before the user can answer it.

Architecture and duplicate logic: The permission path and the event path each classify ACP tools. Their rules already differ. One shared classifier should produce the operation kind and safe path data for both paths.

Performance: The scope search uses nested find and every calls. Its cost is O(n²), and the ACP schema has no location limit. A linear common-parent check will remove this risk.

Validation:

All 148 ACP tests passed.
The ACP package type check passed.
The exact diff passed git diff --check.
A real OpenCode run reached the changed route.
The browser showed Do you want to make these changes? and Write root: /tmp/qa-1803.
I stopped before approval of the file write. I removed the temporary directory and stopped the source server.

The happy path works, but the wrong consent text and the duplicate timeline rows affect user trust. I would fix these cases before I merge this SHA. I posted this as a comment-only review, as required.

The ACP bridge dropped toolCall.locations and always built a command subject from rawInput.command ?? title ?? kind. For a write, opencode sends no command and a title that is a file path or a bare parent directory, so the user saw "Do you want to run this command? $ /tmp/dir". Forward locations[].path into the permission payload and classify edit/delete/move tools, or unclassified tools that name locations, as file_change subjects when no shell command is present. Fixes #1719 Co-Authored-By: Claude <noreply@anthropic.com>

Address SlopCop review on #1803: - Add tool-call-operation.ts, one classifier for command / file_change / generic that both the timeline translator and the permission mapping use, so an approval row and its timeline item never disagree. - Treat kind "other" with locations as generic; the write signal for opencode's external_directory permission is the in-flight edit tool call with the same id, which the bridge now looks up in translator state. - Normalize paths and compute the write scope linearly; return null when no named path contains the others. - Drop blank location paths before they reach the write scope. - Add classifier, write-scope, mapping, and bridge tests. Co-Authored-By: Claude <noreply@anthropic.com>

Main commit 1952900 presented ACP file-write permissions as file-change approvals. interactions.ts, the new tool-call-operation.ts, the fake agent, and the bridge payload enrichment merged cleanly during the rebase, but the bridge still read the old translator's resolveState().toolCallEventsByCallId (gone with event-translation.ts) and the delta translator kept its own pre-#1803 copy of the command/file-change/generic classification. - The delta translator's classifyAcpToolCall now delegates to the shared classifier in tool-call-operation.ts (one copy; approval rows and timeline items cannot disagree) and its private extractAcpCommand / extractAcpToolCallPath helpers are deleted. - The translator exposes getMergedToolCall(threadId, toolCallId) over its merge cache; the bridge passes that in-flight tool call as the startedToolCall write signal for OpenCode's external_directory requests. bb-plugin-provider-acp typecheck+test: 9 files / 161 tests, matching the original commit's count (includes the ported interactions, classifier, and end-to-end bridge permission tests). Co-Authored-By: Claude Fable 5 <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>

…on (#2179) > **Stacked on #2178 (WS1b-claude).** Chain: contract → WS1a → WS2a → codex #2164 → claude #2178 → **this 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 item** — `plugins/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.title`** — `plugins/provider-acp/src/tool-classification.ts`: `execute` → `command`, diff / `edit` / `delete` with a path → `fileChange` (the shared operation classifier the permission mapping uses is untouched, #1803), `read` → `fileRead` when the path is known (`locations`, a `rawInput` path field, or a single code-ticked token in the title — how grok titles its reads), `search` → `search` when `rawInput` has a pattern/query/regex (content) or glob (path), `fetch` → `webFetch` when `rawInput` or the title carries the URL, `think` → `reasoning`. 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 (`forwardDynamicToolCall` → `noteInjectedToolCall`) 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 subject** — `interactions.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 #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 --------- Co-authored-by: Claude <noreply@anthropic.com>

bb-slop-cop Bot reviewed Aug 18, 2026

View reviewed changes

SawyerHood and others added 2 commits August 18, 2026 19:46

SawyerHood force-pushed the bb/draft-pr-for-1719-acp-file-write-approvals-prese-thr_5y2xgvhuma branch from a539500 to 5f1af7e Compare August 18, 2026 19:55

SawyerHood merged commit 1952900 into main Aug 18, 2026
10 checks passed

SawyerHood deleted the bb/draft-pr-for-1719-acp-file-write-approvals-prese-thr_5y2xgvhuma branch August 18, 2026 21:55

SawyerHood mentioned this pull request Aug 21, 2026

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

Merged

Navigation Menu

Repository navigation

Present ACP file-write permissions as file-change approvals - #1803

Present ACP file-write permissions as file-change approvals#1803
SawyerHood merged 2 commits into
mainfrom
bb/draft-pr-for-1719-acp-file-write-approvals-prese-thr_5y2xgvhuma

SawyerHood commented Aug 18, 2026 •

edited

Loading

Uh oh!

bb-slop-cop Bot commented Aug 18, 2026

Uh oh!

bb-slop-cop Bot Aug 18, 2026

Uh oh!

SawyerHood Aug 18, 2026

Uh oh!

bb-slop-cop Bot Aug 18, 2026

Uh oh!

SawyerHood Aug 18, 2026

Uh oh!

bb-slop-cop Bot Aug 18, 2026

Uh oh!

SawyerHood Aug 18, 2026

Uh oh!

bb-slop-cop Bot Aug 18, 2026

Uh oh!

SawyerHood Aug 18, 2026

Uh oh!

bb-slop-cop Bot left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

SawyerHood commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What was wrong

What changed

Tests

How I verified

Uh oh!

bb-slop-cop Bot commented Aug 18, 2026

Uh oh!

bb-slop-cop Bot Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

SawyerHood Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

bb-slop-cop Bot Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

SawyerHood Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

bb-slop-cop Bot Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

SawyerHood Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

bb-slop-cop Bot Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

SawyerHood Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

bb-slop-cop Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

SawyerHood commented Aug 18, 2026 •

edited

Loading