fix(server): ACP agents can read files under on-request approval

[juliusmarminge]

Under on-request or approval-required approval, T3 refused every file read from ACP agents. acpOperationDisposition returned ask for every operation, reads included, whenever the approval policy wasn't never. The client-side fs/read_text_file guard can't prompt, so an ask read passed only if an earlier approval grant covered that path. Anything else was denied. T3 advertises fs.readTextFile: true, and Grok then routes every read through the client without asking permission first. So under on-request, Grok couldn't read any file, including one it had just been approved to write.

Decision

Reads follow the sandbox and never ask. Approval policy governs writes and commands, not reads. Codex and Claude on-request work the same way: they ask before writes and commands, not reads.

• read, search and think (the kinds the never path already treated as reads) are allowed under read-only, workspace-write, full-access, external and unset sandboxes, whatever the approval policy. An unrecognised sandbox type still denies.
• Client fs/read_text_file: served without a grant. The guard now only allows or denies.
• session/request_permission for a read-kind tool call: auto-allowed, even though the provider chose to ask. Allowing it is the consistent answer, since the same read through the client fs path would be served without a prompt anyway.
• Containment is unchanged: reads were never path-confined on the never path (a workspace-write read outside the workspace was already allowed), so nothing becomes readable that never didn't already allow.
• Unchanged: writes, deletes, moves, execute, fetch and unknown kinds keep their ask/deny behavior.
• Removed: read grants (allowsRead, the file-read branch of recordApproval). Reads never reach ask now, so nothing consults them.

Grok source (xai-org/grok-build @ f0e3be1)

• crates/codegen/xai-grok-shell/src/agent/mvp_agent/agent_ops.rs:4160,4193-4196: when the client advertises both fs.readTextFile and fs.writeTextFile, Grok swaps its local filesystem for AcpSessionFs.
• crates/codegen/xai-grok-workspace/src/file_system/acp_fs.rs:65-74: AcpSessionFs::read_file is a plain fs/read_text_file request. Nothing asks permission before it.
• crates/codegen/xai-grok-workspace/src/permission/manager/mod.rs:1160-1166: Grok's own permission manager pre-approves AccessKind::Read and Grep as SAFE_COMMAND. A read never becomes a session/request_permission.
• crates/codegen/xai-grok-tools/src/implementations/opencode/write/mod.rs:115-119: the write tool reads the target first. On any error it treats the file as new (Err(_) => (false, None)), which is why Grok still wrote the file in the old recording.
• crates/codegen/xai-grok-tools/src/implementations/opencode/read/mod.rs:213-221: the read tool turns a failed client read into FileReadError("Failed to read file: …") for the model. Under the old policy, every Grok read under on-request ended there.

Evidence

tool_call_read_only_on_request/grok_transcript.ndjson was re-recorded live (Grok 1.0.41, record-grok-acp-replay-fixture.ts) with the fix. The write still asks once (kind: "edit" → allow-once → fs/write_text_file). The difference is in the reads:

	before (#13562 recording)	after
read before the write	denied: "The active T3 runtime policy requires approval for fs/read_text_file…"	reaches disk: "Could not read text file" (the file doesn't exist yet)
read-back after the write	denied, same message	served: {"content":"codex app-server approval fixture."}

The Grok variant now uses assertToolCallReadOnlyOnRequestGrokOutput. It runs the shared on-request assertion (still exactly one runtime request, the write), then requires that an fs/read_text_file response served the probe content and that no read was refused by the runtime policy. Against the old policy and the old recording, it fails with "T3 must serve Grok's client-mediated read of the approved file without asking".

Tests changed

• AcpClientPolicy.test.ts:
  • "asks in approval-required mode for reads, writes, and terminals" becomes "asks for writes and terminals but allows reads…". The read assertion flipped from ask to allow, which is the decision itself.
  • New cases cover the ones the old tests missed: reads allowed under on-request with read-only and workspace-write while writes and terminals still ask; read-kind permission requests auto-allowed while edit, delete, move, execute, fetch and other still ask; reads denied under an unknown sandbox.
  • The grant test "grants reads only to approved locations…" loses its allowsRead checks, since read grants no longer exist. It keeps the check that an approved read grants neither writes nor terminals.
• AcpAdapterV2.test.ts: "rejects unapproved client-mediated reads in approval-required mode" became "serves client-mediated reads without approval…". It asserts the file content comes back through the adapter's real read handler.
• GrokAdapterV2.test.ts: the approval-required read → ask assertion is now read → allow, plus edit → ask, so the test still proves approval-required asks.
• docs/user/providers-acp.md: "approval-required threads keep asking" now says the asking applies to edits and commands, not reads.

Verification

• vp test run src/orchestration-v2/testkit/OrchestratorReplayFixtures.integration.test.ts -t "grok|acpRegistry": 19 passed.
• -t "tool_call_read_only_on_request": all 4 variants (Codex, Claude, Grok, registry) pass.
• vp test run on AcpClientPolicy.test.ts, GrokAdapterV2.test.ts, XAiAcpExtension.test.ts and AcpAdapterV2.test.ts: 195 passed.
• Negative check: base policy plus the old recording against the new assertion fails as shown above. The new policy plus the old recording fails to replay, because T3 now answers the read the transcript expected to be denied.
• vp exec tsc --noEmit -p . in apps/server: 0 error TS / warning TS. vp run knip:check: clean. vp fmt / vp lint on touched files: no new findings (the unused NodePath import in AcpAdapterV2.ts and the other warnings are pre-existing).
• Not run: repo-wide suites.

Overlap: #13579 also edits tool_call_read_only_on_request/output.ts, in a different hunk (it removes the content === undefined skip in the shared assertion). This PR only appends a Grok-specific function after it, so the two should merge cleanly.

Model: Claude Opus 5.5 (Claude Code)

🤖 Generated with Claude Code

---

[github-actions]

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	—	4.9 KiB	—	6.8 KiB	✅
Codex	Thread snapshot wire	—	3.7 KiB	—	4.9 KiB	✅
Codex	Live turn WebSocket wire	—	1.2 KiB	—	2.0 KiB	✅
Codex	Live turn WebSocket decoded	—	20.4 KiB	—	29.3 KiB	✅
Codex	Live turn messages	—	2	—	8	✅
Claude	Total thread wire	—	4.9 KiB	—	6.8 KiB	✅
Claude	Thread snapshot wire	—	3.7 KiB	—	4.9 KiB	✅
Claude	Live turn WebSocket wire	—	1.2 KiB	—	2.0 KiB	✅
Claude	Live turn WebSocket decoded	—	20.8 KiB	—	29.3 KiB	✅
Claude	Live turn messages	—	2	—	8	✅

Baseline: unavailable · PR result: c9b9578 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

• Codex decoded thread snapshot: 106.1 KiB
• Claude decoded thread snapshot: 106.4 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

[macroscopeapp]

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR changes production ACP authorization semantics so file reads and searches bypass approval under on-request/approval-required sessions, while writes and commands remain gated. The change is well-scoped and tested but alters a sensitive access-control boundary and existing product behavior.

^{You can add or adjust custom eligibility rules. Learn more.}