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

apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts

@@ -2138,7 +2138,7 @@ describe("AcpAdapterV2", () => {
     }).pipe(Effect.provide(testLayer), Effect.scoped),
   );
 
-  it.effect("rejects unapproved client-mediated reads in approval-required mode", () =>
+  it.effect("serves client-mediated reads without approval in approval-required mode", () =>
     Effect.gen(function* () {
       const childProcessSpawner = yield* ChildProcessSpawner.ChildProcessSpawner;
       const fileSystem = yield* FileSystem.FileSystem;
@@ -2198,11 +2198,11 @@ describe("AcpAdapterV2", () => {
         return yield* Effect.die("ACP runtime must register the fs read handler");
       }
 
-      const deniedRead = yield* readTextFile(
+      const read = yield* readTextFile(
         { sessionId: "mock-session-1", path: readablePath },
         { requestId: "test-unapproved-read", method: "fs/read_text_file" },
-      ).pipe(Effect.exit);
-      assert.isTrue(Exit.isFailure(deniedRead));
+      );
+      assert.equal(read.content, "existing");
     }).pipe(Effect.provide(testLayer), Effect.scoped),
   );
 

apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts

@@ -5185,17 +5185,11 @@ export function makeAcpAdapterV2(options: AcpAdapterV2Options): ProviderAdapterV
 
         const guardClientFsRead = (path: string) =>
           clientPolicyContext.pipe(
-            Effect.flatMap(({ policy, turnKey }) => {
-              const disposition = acpClientReadDisposition(policy, path);
-              if (
-                disposition === "allow" ||
-                (disposition === "ask" &&
-                  clientPolicyGrants.allowsRead({ path, cwd: policy.cwd, turnKey }))
-              ) {
-                return Effect.void;
-              }
-              return denyClientRequest(`fs/read_text_file for '${path}'`, disposition);
-            }),
+            Effect.flatMap(({ policy }) =>
+              acpClientReadDisposition(policy) === "allow"
+                ? Effect.void
+                : denyClientRequest(`fs/read_text_file for '${path}'`, "deny"),
+            ),
           );
 
         const guardClientTerminalCreate = clientPolicyContext.pipe(

apps/server/src/orchestration-v2/Adapters/GrokAdapterV2.test.ts

@@ -241,9 +241,16 @@ describe("ACP permission policy", () => {
     assert.equal(
       acpPermissionDisposition(
         runtimePolicy({ runtimeMode: "approval-required" }),
-        permissionRequest("read"),
+        permissionRequest("edit"),
       ),
       "ask",
     );
+    assert.equal(
+      acpPermissionDisposition(
+        runtimePolicy({ runtimeMode: "approval-required" }),
+        permissionRequest("read"),
+      ),
+      "allow",
+    );
   });
 });

apps/server/src/orchestration-v2/testkit/fixtures/index.ts

@@ -85,7 +85,10 @@ import {
 import { assertToolCallReadOnlyClaudeOutput } from "./tool_call_read_only/claude_output.ts";
 import { assertToolCallReadOnlyCursorOutput } from "./tool_call_read_only/cursor_output.ts";
 import { toolCallReadOnlyInput } from "./tool_call_read_only/input.ts";
-import { assertToolCallReadOnlyOnRequestOutput } from "./tool_call_read_only_on_request/output.ts";
+import {
+  assertToolCallReadOnlyOnRequestGrokOutput,
+  assertToolCallReadOnlyOnRequestOutput,
+} from "./tool_call_read_only_on_request/output.ts";
 import { toolCallReadOnlyOnRequestInput } from "./tool_call_read_only_on_request/input.ts";
 import { assertToolCallRestrictedGranularClaudeOutput } from "./tool_call_restricted_granular/claude_output.ts";
 import { assertToolCallRestrictedGranularOutput } from "./tool_call_restricted_granular/codex_output.ts";
@@ -454,7 +457,7 @@ export const ORCHESTRATOR_REPLAY_FIXTURES: ReadonlyArray<OrchestratorReplayFixtu
         ),
         modelSelection: GROK_MODEL_SELECTION,
         runtimePolicyOverride: READ_ONLY_ON_REQUEST_POLICY,
-        assertOutput: assertToolCallReadOnlyOnRequestOutput,
+        assertOutput: assertToolCallReadOnlyOnRequestGrokOutput,
       },
       {
         driver: ProviderDriverKind.make("acpRegistry"),

apps/server/src/orchestration-v2/testkit/fixtures/tool_call_read_only_on_request/output.ts

@@ -65,6 +65,38 @@ export function assertToolCallReadOnlyOnRequestOutput(
   }
 }
 
+// Grok never asks permission to read and routes every read through the
+// client's fs/read_text_file. Reads follow the sandbox, not the approval
+// policy, so T3 must serve the read-back of the approved write without a
+// second request (the shared assertion pins the write as the only one).
+export function assertToolCallReadOnlyOnRequestGrokOutput(
+  result: OrchestratorV2ScenarioResult,
+  transcript: ProviderReplayTranscript,
+) {
+  assertToolCallReadOnlyOnRequestOutput(result, transcript);
+  const readResponses = transcript.entries.flatMap((entry) => {
+    if (entry.type !== "expect_outbound") return [];
+    const frame = entry.frame as {
+      method?: unknown;
+      result?: { content?: unknown };
+      error?: { message?: unknown };
+    };
+    return frame.method === "fs/read_text_file" ? [frame] : [];
+  });
+  assert.isTrue(
+    readResponses.some(
+      (frame) =>
+        typeof frame.result?.content === "string" && frame.result.content.includes(PROBE_CONTENT),
+    ),
+    "T3 must serve Grok's client-mediated read of the approved file without asking",
+  );
+  // Grok probes the path before creating it, so a not-found error is expected;
+  // a refusal by the runtime policy is not.
+  for (const frame of readResponses) {
+    assert.notInclude(String(frame.error?.message ?? ""), "runtime policy");
+  }
+}
+
 function writtenContent(item: OrchestrationV2TurnItem): string | undefined {
   switch (item.type) {
     case "command_execution":

apps/server/src/provider/acp/AcpClientPolicy.test.ts

@@ -91,6 +91,21 @@ describe("acpPermissionDisposition", () => {
     assert.equal(acpPermissionDisposition(policy, permissionRequest("execute")), "deny");
   });
 
+  it("auto-allows read-kind permission requests under on-request approval", () => {
+    for (const runtimePolicy of [
+      { ...policy, approvalPolicy: "on-request" },
+      { ...policy, approvalPolicy: "on-request", sandboxPolicy: { type: "readOnly" } },
+      { runtimeMode: "approval-required", cwd },
+    ] satisfies ReadonlyArray<AcpRuntimePolicy>) {
+      for (const kind of ["read", "search", "think"] as const) {
+        assert.equal(acpPermissionDisposition(runtimePolicy, permissionRequest(kind)), "allow");
+      }
+      for (const kind of ["edit", "delete", "move", "execute", "fetch", "other"] as const) {
+        assert.equal(acpPermissionDisposition(runtimePolicy, permissionRequest(kind)), "ask");
+      }
+    }
+  });
+
   it.effect("denies mutations through workspace symlinks that escape the writable roots", () =>
     Effect.gen(function* () {
       const fileSystem = yield* FileSystem.FileSystem;
@@ -262,17 +277,17 @@ describe("acpMcpToolApprovalElicitationDisposition", () => {
 describe("client-mediated dispositions", () => {
   const cwd = NodePath.resolve(process.cwd(), "acp-client-policy-workspace");
 
-  it("asks in approval-required mode for reads, writes, and terminals", () => {
+  it("asks for writes and terminals but allows reads in approval-required mode", () => {
     const policy: AcpRuntimePolicy = { runtimeMode: "approval-required", cwd };
-    assert.equal(acpClientReadDisposition(policy, NodePath.join(cwd, "file.ts")), "ask");
+    assert.equal(acpClientReadDisposition(policy), "allow");
     assert.equal(acpClientWriteDisposition(policy, NodePath.join(cwd, "file.ts")), "ask");
     assert.equal(acpClientExecuteDisposition(policy), "ask");
   });
 
   it("allows in auto and full-access modes without an explicit sandbox", () => {
     for (const runtimeMode of ["auto", "auto-accept-edits", "full-access"] as const) {
       const policy: AcpRuntimePolicy = { runtimeMode, cwd };
-      assert.equal(acpClientReadDisposition(policy, NodePath.join(cwd, "file.ts")), "allow");
+      assert.equal(acpClientReadDisposition(policy), "allow");
       assert.equal(acpClientWriteDisposition(policy, NodePath.join(cwd, "file.ts")), "allow");
       assert.equal(acpClientExecuteDisposition(policy), "allow");
     }
@@ -285,7 +300,7 @@ describe("client-mediated dispositions", () => {
       approvalPolicy: "never",
       sandboxPolicy: { type: "readOnly" },
     };
-    assert.equal(acpClientReadDisposition(policy, NodePath.join(cwd, "file.ts")), "allow");
+    assert.equal(acpClientReadDisposition(policy), "allow");
     assert.equal(acpClientWriteDisposition(policy, NodePath.join(cwd, "file.ts")), "deny");
     assert.equal(acpClientExecuteDisposition(policy), "deny");
   });
@@ -297,11 +312,40 @@ describe("client-mediated dispositions", () => {
       approvalPolicy: "never",
       sandboxPolicy: { type: "workspaceWrite", writableRoots: [], networkAccess: false },
     };
-    assert.equal(acpClientReadDisposition(policy, "/tmp/outside-workspace/file.ts"), "allow");
+    assert.equal(acpClientReadDisposition(policy), "allow");
     assert.equal(acpClientWriteDisposition(policy, NodePath.join(cwd, "src/file.ts")), "allow");
     assert.equal(acpClientWriteDisposition(policy, "/tmp/outside-workspace/file.ts"), "deny");
     assert.equal(acpClientExecuteDisposition(policy), "deny");
   });
+
+  it("allows reads without asking under on-request approval while writes and terminals ask", () => {
+    for (const sandboxPolicy of [
+      { type: "readOnly" },
+      { type: "workspaceWrite", writableRoots: [], networkAccess: false },
+    ]) {
+      const policy: AcpRuntimePolicy = {
+        runtimeMode: "full-access",
+        cwd,
+        approvalPolicy: "on-request",
+        sandboxPolicy,
+      };
+      assert.equal(acpClientReadDisposition(policy), "allow");
+      assert.equal(acpClientWriteDisposition(policy, NodePath.join(cwd, "file.ts")), "ask");
+      assert.equal(acpClientExecuteDisposition(policy), "ask");
+    }
+  });
+
+  it("denies reads under a sandbox type it does not recognize", () => {
+    for (const approvalPolicy of ["never", "on-request"]) {
+      const policy: AcpRuntimePolicy = {
+        runtimeMode: "full-access",
+        cwd,
+        approvalPolicy,
+        sandboxPolicy: { type: "futureSandbox" },
+      };
+      assert.equal(acpClientReadDisposition(policy), "deny");
+    }
+  });
 });
 
 describe("makeAcpClientPolicyGrants", () => {
@@ -355,7 +399,7 @@ describe("makeAcpClientPolicyGrants", () => {
     );
   });
 
-  it("grants reads only to approved locations and terminals only from commands", () => {
+  it("grants terminals only from commands, never from an approved read", () => {
     const grants = makeAcpClientPolicyGrants();
     grants.recordApproval({
       kind: "file-read",
@@ -364,11 +408,6 @@ describe("makeAcpClientPolicyGrants", () => {
       scope: "turn",
       turnKey: "turn-1",
     });
-    assert.isTrue(grants.allowsRead({ path: filePath, cwd, turnKey: "turn-1" }));
-    assert.isFalse(
-      grants.allowsRead({ path: NodePath.join(cwd, "src", "other.ts"), cwd, turnKey: "turn-1" }),
-    );
-    assert.isFalse(grants.allowsRead({ path: filePath, cwd, turnKey: "turn-2" }));
     assert.isFalse(grants.allowsExecute("turn-1"));
     assert.isFalse(grants.allowsWrite({ path: filePath, cwd, turnKey: "turn-1" }));
     grants.recordApproval({