fix(server): show diffs for ACP edits that send oldText/newText

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

@@ -82,6 +82,7 @@ import {
   acpPostSettleWakeEvidence,
   acpPostSettleWakeShouldBuffer,
   acpProjectedCommandExitCode,
+  acpToolCallDiffPatch,
   acpTurnStartShouldPreserveContinuation,
   makeAcpAdapterV2,
   type AcpAdapterV2ExtensionContext,
@@ -122,6 +123,57 @@ describe("acpProjectedCommandExitCode", () => {
   });
 });
 
+describe("acpToolCallDiffPatch", () => {
+  it("builds a patch per file from ACP v1 oldText/newText, with /dev/null for a new file", () => {
+    assert.equal(
+      acpToolCallDiffPatch([
+        { type: "diff", path: "/repo/new.txt", oldText: null, newText: "hello\n" },
+        { type: "diff", path: "/repo/a.ts", oldText: "a\nb\nc\n", newText: "a\nB\nc\n" },
+        { type: "diff", path: "/repo/same.ts", oldText: "x\n", newText: "x\n" },
+      ]),
+      [
+        "--- /dev/null",
+        "+++ /repo/new.txt",
+        "@@ -0,0 +1,1 @@",
+        "+hello",
+        "",
+        "--- /repo/a.ts",
+        "+++ /repo/a.ts",
+        "@@ -1,3 +1,3 @@",
+        " a",
+        "-b",
+        "+B",
+        " c",
+        "",
+      ].join("\n"),
+    );
+  });
+
+  it("keeps the ACP v2 patch text as sent", () => {
+    const text = "diff --git a/repo/a.ts b/repo/a.ts\n";
+    assert.equal(
+      acpToolCallDiffPatch([
+        {
+          type: "diff",
+          changes: [{ operation: "modify", path: "/repo/a.ts" }],
+          patch: { format: "git_patch", text },
+        },
+      ]),
+      text,
+    );
+  });
+
+  it("drops the patch for a rewrite too large to diff cheaply", () => {
+    const lines = (prefix: string) =>
+      Array.from({ length: 2_000 }, (_, index) => `${prefix} ${index}`).join("\n");
+    assert.isUndefined(
+      acpToolCallDiffPatch([
+        { type: "diff", path: "/repo/big.ts", oldText: lines("old"), newText: lines("new") },
+      ]),
+    );
+  });
+});
+
 describe("ACP continuation ownership", () => {
   it("preserves a continuation offered during non-buffered carryover handling", () => {
     assert.isFalse(

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

@@ -33,6 +33,7 @@ import {
 } from "@t3tools/contracts";
 import { modelSelectionsEqual } from "@t3tools/shared/model";
 import { type SelfInvocation, selfInvocationArgs } from "@t3tools/shared/nodeRuntime";
+import { FILE_HEADERS_ONLY, formatPatch, structuredPatch } from "diff";
 import * as Cause from "effect/Cause";
 import * as Crypto from "effect/Crypto";
 import * as DateTime from "effect/DateTime";
@@ -872,15 +873,45 @@ function structuredFileChanges(toolCall: AcpToolCallState) {
   });
 }
 
-function structuredDiffPatch(toolCall: AcpToolCallState): string | undefined {
-  const content = toolCall.data.content;
+// Past this edit distance an edit keeps no patch text, so projecting a large
+// rewrite cannot stall the event loop in the diff search. A created or emptied
+// file has one empty side and needs no search, so it is never capped.
+const ACP_V1_DIFF_MAX_EDITS = 1_000;
+
+/**
+ * Patch text for a tool call's diff content. ACP v2 diffs carry it as
+ * `patch.text`. ACP v1 diffs carry `oldText`/`newText` instead (`oldText`
+ * null or absent for a new file), and agents that negotiate v1 still send
+ * that shape, so the patch is built from the two sides.
+ */
+export function acpToolCallDiffPatch(content: unknown): string | undefined {
   if (!Array.isArray(content)) return undefined;
-  for (const entry of content) {
+  const diffs = content.flatMap((entry) => {
     const diff = unknownRecord(entry);
-    const patch = unknownRecord(diff?.patch);
-    if (diff?.type === "diff" && typeof patch?.text === "string") return patch.text;
+    return diff?.type === "diff" ? [diff] : [];
+  });
+  for (const diff of diffs) {
+    const patch = unknownRecord(diff.patch);
+    if (typeof patch?.text === "string") return patch.text;
   }
-  return undefined;
+  const v1Patches = diffs.flatMap((diff) => {
+    if (typeof diff.path !== "string" || typeof diff.newText !== "string") return [];
+    const oldText = typeof diff.oldText === "string" ? diff.oldText : undefined;
+    const patch = structuredPatch(
+      oldText === undefined ? "/dev/null" : diff.path,
+      diff.path,
+      oldText ?? "",
+      diff.newText,
+      undefined,
+      undefined,
+      {
+        context: 3,
+        maxEditLength: oldText && diff.newText ? ACP_V1_DIFF_MAX_EDITS : Number.POSITIVE_INFINITY,
+      },
+    );
+    return patch === undefined || patch.hunks.length === 0 ? [] : [patch];
+  });
+  return v1Patches.length === 0 ? undefined : formatPatch(v1Patches, FILE_HEADERS_ONLY);
 }
 
 function pathFromToolCall(toolCall: AcpToolCallState): string | undefined {
@@ -3070,7 +3101,8 @@ export function makeAcpAdapterV2(options: AcpAdapterV2Options): ProviderAdapterV
           const rawOutput = toolCall.data.rawOutput ?? toolCall.data.content;
           const changes = structuredFileChanges(toolCall);
           const path = changes[0]?.path ?? pathFromToolCall(toolCall);
-          const diffText = structuredDiffPatch(toolCall) ?? textFromUnknown(rawOutput);
+          const diffText =
+            acpToolCallDiffPatch(toolCall.data.content) ?? textFromUnknown(rawOutput);
           const rawInputRecord = unknownRecord(rawInput);
           const inputVariant =
             typeof rawInputRecord?.variant === "string"

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

@@ -56,11 +56,11 @@ export function assertToolCallReadOnlyOnRequestOutput(
     writes.some((item) => item.status === "completed"),
     "the approved write must complete",
   );
-  // A file_change projected from an ACP v1 diff ({ oldText, newText }) carries
-  // no content today; the adapter reads only the v2 patch form.
+  // Grok's write reports an ACP v1 diff ({ path, oldText, newText }); the
+  // file_change must still carry it, like the v2 patch form.
   for (const item of writes) {
     const content = writtenContent(item);
-    if (content === undefined) continue;
+    assert.isDefined(content, `the approved ${item.type} must carry what it wrote`);
     assert.include(content, PROBE_CONTENT, "the approved write must carry the requested content");
   }
 }