diff --git a/packages/cli/src/acp/permission.ts b/packages/cli/src/acp/permission.ts index cad3185dd1ff..4c33430ad3ed 100644 --- a/packages/cli/src/acp/permission.ts +++ b/packages/cli/src/acp/permission.ts @@ -1,10 +1,10 @@ -import type { PermissionOption } from "@agentclientprotocol/sdk" +import type { PermissionOption, SessionUpdate } from "@agentclientprotocol/sdk" import type { OpenCodeClient, OpenCodeEvent } from "@opencode/client/effect" import { FileDiff } from "@opencode/schema/file-diff" import type { Permission } from "@opencode/schema/permission" import type { Session } from "@opencode/schema/session" import { Patch } from "@opencode/util/patch" -import { applyPatch } from "diff" +import { applyPatch, parsePatch, reversePatch, structuredPatch, type StructuredPatch } from "diff" import { Effect, Option, Schema } from "effect" import { ACPChild } from "./child" import { ACPClient } from "./client" @@ -17,6 +17,7 @@ import { pendingToolCall, stringValue, toLocations, + type DiffSource, type ToolInput, } from "./tool" @@ -60,6 +61,7 @@ export const ask = Effect.fnUntraced(function* (input: Input) { sessionId: input.clientSessionID, toolCall: { ...toolCall, + name: input.tool?.name, rawInput: input.tool ? toolCall.rawInput : undefined, locations: permissionLocations(toolName, toolInput, input.event.data, input.cwd), ...(previews.length > 0 ? { content: previews } : {}), @@ -78,6 +80,45 @@ export function respond(input: Input, decision: Permission.Reply) { ) } +export const withCompletedDiffs = Effect.fnUntraced(function* ( + update: SessionUpdate, + source: DiffSource | undefined, + cwd: string, +) { + if (!source || update.sessionUpdate !== "tool_call_update") return update + const hunks = canonicalName(source.toolName) === "patch" ? patchHunks(source.input) : [] + const diffs = yield* Effect.forEach( + Option.getOrElse(decodeFiles(source.metadata?.files), () => []), + (file) => + Effect.gen(function* () { + const path = absolutePath(file.file, cwd) + const newText = file.status === "deleted" ? "" : yield* Effect.tryPromise(() => Bun.file(path).text()) + if (file.status === "added") return [diff(path, null, newText)] + const recorded = parsePatch(file.patch)[0] + if (!recorded) return [] + const oldText = yield* Effect.try(() => applyPatch(newText, reversePatch(recorded))) + if (oldText !== false) return [diff(path, oldText, newText)] + // Core trims indentation from patch-tool diffs, so rebuild those from its hunks and keep them if the ranges agree. + const hunk = hunks.find( + (item) => item.type === "update" && absolutePath(item.movePath ?? item.path, cwd) === path, + ) + if (hunk?.type !== "update" || hunk.chunks.some((chunk) => !chunk.oldLines.length || !chunk.newLines.length)) + return [] + const chunks = hunk.chunks.map((chunk) => ({ ...chunk, oldLines: chunk.newLines, newLines: chunk.oldLines })) + const rebuilt = (yield* Effect.try(() => Patch.derive(hunk.path, chunks, newText))).content + return ranges(structuredPatch(path, path, rebuilt, newText)) === ranges(recorded) + ? [diff(path, rebuilt, newText)] + : [] + }).pipe(Effect.orElseSucceed((): Preview[] => [])), + { concurrency: "unbounded" }, + ).pipe(Effect.map((items) => items.flat())) + return diffs.length === 0 ? update : { ...update, content: [...(update.content ?? []), ...diffs] } +}) + +function ranges(patch: StructuredPatch) { + return patch.hunks.map((hunk) => `${hunk.oldStart},${hunk.oldLines},${hunk.newStart},${hunk.newLines}`).join(" ") +} + // Core trims the patch tool's diffs for display, which breaks `applyPatch`, so its previews come from its own hunks. const permissionPreviews = Effect.fnUntraced(function* ( toolName: string, diff --git a/packages/cli/src/acp/replay.ts b/packages/cli/src/acp/replay.ts index c92e91b820d3..e5c6825145c1 100644 --- a/packages/cli/src/acp/replay.ts +++ b/packages/cli/src/acp/replay.ts @@ -7,6 +7,7 @@ import { ACPClient } from "./client" import { ACPCompaction } from "./compaction" import type { ACPConnection } from "./connection" import { partsToContentChunks } from "./content" +import { ACPPermission } from "./permission" import type { Attached } from "./sessions" import { ACPTranslate } from "./translate" import { completedToolUpdate, errorToolUpdate, pendingToolCall, runningToolUpdate } from "./tool" @@ -29,7 +30,10 @@ export function history( Stream.runForEach((message) => Effect.forEach( updates(message, attached.cwd, capabilities), - (update) => connection.sessionUpdate({ sessionId: attached.id, update }), + (update) => + ACPPermission.withCompletedDiffs(update, completedSource(message, update), attached.cwd).pipe( + Effect.flatMap((enriched) => connection.sessionUpdate({ sessionId: attached.id, update: enriched })), + ), { discard: true }, ), ), @@ -124,4 +128,11 @@ export function updates(message: SessionMessage.Info, cwd: string, capabilities: }) } +function completedSource(message: SessionMessage.Info, update: SessionUpdate) { + if (message.type !== "assistant" || update.sessionUpdate !== "tool_call_update") return undefined + const part = message.content.find((item) => item.type === "tool" && item.id === update.toolCallId) + if (part?.type !== "tool" || part.state.status !== "completed") return undefined + return { toolName: part.name, input: part.state.input, metadata: part.state.metadata } +} + export * as ACPReplay from "./replay" diff --git a/packages/cli/src/acp/tool.ts b/packages/cli/src/acp/tool.ts index 2bdfb0918ebb..d535055061f3 100644 --- a/packages/cli/src/acp/tool.ts +++ b/packages/cli/src/acp/tool.ts @@ -7,6 +7,12 @@ import { Result } from "effect" export type ToolInput = Record +export type DiffSource = { + readonly toolName: string + readonly input: ToolInput + readonly metadata?: Readonly> +} + function toToolKind(toolName: string): ToolKind { switch (canonicalName(toolName)) { case "shell": @@ -66,6 +72,7 @@ export function pendingToolCall(input: { }): ToolCall { return { toolCallId: input.toolCallId, + name: input.toolName, title: toolTitle(input.toolName, input.state.input, input.state.title), kind: toToolKind(input.toolName), status: "pending", @@ -106,18 +113,11 @@ export function completedToolUpdate(input: { read === undefined ? normalized.filter((part) => !images.includes(part)) : [{ type: "content" as const, content: { type: "text" as const, text: read } }] - const oldText = stringValue(input.input.oldString) - const newText = stringValue(input.input.newString) - const path = filePath(input.input) - const diff: ToolCallContent[] = - oldText === undefined || newText === undefined || path === undefined - ? [] - : [{ type: "diff", path: absolutePath(path, input.cwd), oldText, newText }] return { toolCallId: input.toolCallId, status: "completed", locations: toLocations(input.toolName, input.input, input.cwd), - content: [...primary, ...diff, ...images], + content: [...primary, ...images], rawOutput: { ...(input.metadata === undefined ? {} : { metadata: input.metadata }), }, diff --git a/packages/cli/src/acp/translate.ts b/packages/cli/src/acp/translate.ts index 08cff7b172f6..c6960b4cc296 100644 --- a/packages/cli/src/acp/translate.ts +++ b/packages/cli/src/acp/translate.ts @@ -7,7 +7,14 @@ import { TokenUsage } from "@opencode/schema/token-usage" import { ACPChild } from "./child" import { ACPCompaction } from "./compaction" import { ACPError } from "./error" -import { completedToolUpdate, errorToolUpdate, pendingToolCall, runningToolUpdate, type ToolInput } from "./tool" +import { + completedToolUpdate, + errorToolUpdate, + pendingToolCall, + runningToolUpdate, + type DiffSource, + type ToolInput, +} from "./tool" const RetryMeta = "opencode/retry" @@ -56,8 +63,8 @@ type FormEvent = Extract type CreatedEvent = Extract export type Output = - | { readonly _tag: "SessionUpdate"; readonly update: SessionUpdate } - | { readonly _tag: "ChildUpdate"; readonly update: ACPChild.Update } + | { readonly _tag: "SessionUpdate"; readonly update: SessionUpdate; readonly diff?: DiffSource } + | { readonly _tag: "ChildUpdate"; readonly update: ACPChild.Update; readonly diff?: DiffSource } | { readonly _tag: "PermissionAsk" readonly event: PermissionEvent @@ -374,7 +381,10 @@ function sessionEvent( content: event.data.content, cwd: ctx.cwd, }), - }), + }).map((output) => ({ + ...output, + diff: { toolName: tool.name, input: tool.input, metadata: event.data.metadata }, + })), } } case "session.tool.failed": { diff --git a/packages/cli/src/acp/turn.ts b/packages/cli/src/acp/turn.ts index 586e5dc9a085..34b7164f9a4b 100644 --- a/packages/cli/src/acp/turn.ts +++ b/packages/cli/src/acp/turn.ts @@ -135,17 +135,22 @@ export const make = Effect.fnUntraced(function* (input: { switch (output._tag) { case "SessionUpdate": if (turn.background) return Effect.void - return input.connection.sessionUpdate({ sessionId: turn.ctx.sessionID, update: output.update }) + return ACPPermission.withCompletedDiffs(output.update, output.diff, turn.ctx.cwd).pipe( + Effect.flatMap((update) => input.connection.sessionUpdate({ sessionId: turn.ctx.sessionID, update })), + ) case "ChildUpdate": - return input.connection - .extNotification(ACPChild.UpdateMethod, output.update) - .pipe( - Effect.catchCause((cause) => - Cause.hasInterruptsOnly(cause) - ? Effect.void - : Effect.logWarning("ACP child session update failed", cause), - ), - ) + return Effect.gen(function* () { + if (output.update.type !== "update") + return yield* input.connection.extNotification(ACPChild.UpdateMethod, output.update) + const update = yield* ACPPermission.withCompletedDiffs(output.update.update, output.diff, turn.ctx.cwd) + return yield* input.connection.extNotification(ACPChild.UpdateMethod, { ...output.update, update }) + }).pipe( + Effect.catchCause((cause) => + Cause.hasInterruptsOnly(cause) + ? Effect.void + : Effect.logWarning("ACP child session update failed", cause), + ), + ) case "PermissionAsk": { const permission = { client: input.client, diff --git a/packages/cli/test/acp/permission.test.ts b/packages/cli/test/acp/permission.test.ts index 0d54817969c9..ea7bdaccb99f 100644 --- a/packages/cli/test/acp/permission.test.ts +++ b/packages/cli/test/acp/permission.test.ts @@ -1,12 +1,12 @@ import { describe, expect, test } from "bun:test" import type { AnyRequest, CreateElicitationResponse, RequestPermissionResponse } from "@agentclientprotocol/sdk" import type { OpenCodeEventEncoded } from "@opencode/protocol/groups/event" -import { createTwoFilesPatch } from "diff" import fs from "node:fs/promises" import path from "node:path" import { tmpdir } from "../fixture/tmpdir" import { delivered, + fileDiff, ephemeralEvent, interrupted, permissionAsked, @@ -284,6 +284,8 @@ describe("acp edit previews over the wire", () => { [{ path: file("folder") }], [{ path: file("unpatched.ts") }], ]) + expect(acp.permissions[0]?.toolCall.name).toBe("edit") + expect(acp.permissions[4]?.toolCall).not.toHaveProperty("name") expect(decisions(acp)).toHaveLength(8) }) }) @@ -304,6 +306,3 @@ function decisions(acp: Wire) { return acp.server.replies.map((reply) => [reply.requestID, reply.decision]) } -function fileDiff(file: string, before: string, after: string, status: "added" | "deleted" | "modified" = "modified") { - return { file, patch: createTwoFilesPatch(file, file, before, after), additions: 1, deletions: 1, status } -} diff --git a/packages/cli/test/acp/prompt.test.ts b/packages/cli/test/acp/prompt.test.ts index ae242997272c..18b65d97f89e 100644 --- a/packages/cli/test/acp/prompt.test.ts +++ b/packages/cli/test/acp/prompt.test.ts @@ -8,6 +8,7 @@ import { pathToFileURL } from "node:url" import { tmpdir } from "../fixture/tmpdir" import { delivered, + fileDiff, durableEvent, ephemeralEvent, failed, @@ -144,6 +145,97 @@ describe("acp prompt turns over the wire", () => { }) }) + test("reports full-file diffs for a completed edit and patch", async () => { + await using dir = await tmpdir() + const file = (name: string) => path.resolve(dir.path, name) + const edited = "one\r\nthree\r\n" + const indented = " const x = 1\n const y = 3\n" + const formatted = "keep\nnew\n// formatted\n" + const added = "two\n" + await Promise.all([ + Bun.write(file("edited.ts"), edited), + Bun.write(file("indented.ts"), indented), + Bun.write(file("formatted.ts"), formatted), + Bun.write(file("added.ts"), added), + ]) + const patch = [ + "*** Begin Patch", + "*** Update File: indented.ts", + "@@", + " const x = 1", + "- const y = 2", + "+ const y = 3", + "*** Update File: formatted.ts", + "@@", + " keep", + "-old", + "+new", + "*** Add File: added.ts", + "+two", + "*** Delete File: gone.ts", + "*** End Patch", + ].join("\n") + await using acp = await startWire({ + onPrompt: ({ sessionID, id }) => + turn( + sessionID, + id, + toolStarted(sessionID, "call_edit", "edit"), + toolCalled(sessionID, "call_edit", { path: "edited.ts", oldString: "two", newString: "three" }), + toolSucceeded(sessionID, "call_edit", { files: [fileDiff("edited.ts", "one\r\ntwo\r\n", edited)] }, "edited"), + toolStarted(sessionID, "call_patch", "patch"), + toolCalled(sessionID, "call_patch", { patchText: patch }), + toolSucceeded( + sessionID, + "call_patch", + { + files: [ + trimmed("indented.ts", " const x = 1\n const y = 2\n", indented), + fileDiff("formatted.ts", "keep\nold\n", formatted), + fileDiff("added.ts", "", added, "added"), + fileDiff("gone.ts", "gone\n", "", "deleted"), + fileDiff("missing.ts", "one\n", "two\n"), + ], + }, + "patched", + ), + toolStarted(sessionID, "call_snippet", "edit"), + toolCalled(sessionID, "call_snippet", { path: "edited.ts", oldString: "two", newString: "three" }), + toolSucceeded(sessionID, "call_snippet", {}, "edited"), + ), + }) + await acp.initialize() + const session = await acp.newSession(dir.path) + + await acp.prompt(session.sessionId, "edit the files") + + const completed = (id: string) => + turnUpdates(acp.updates).find( + (item) => + item.update.sessionUpdate === "tool_call_update" && + item.update.toolCallId === id && + item.update.status === "completed", + )?.update + expect(completed("call_edit")).toMatchObject({ + content: [ + { type: "content", content: { type: "text", text: "edited" } }, + { type: "diff", path: file("edited.ts"), oldText: "one\r\ntwo\r\n", newText: edited }, + ], + }) + expect(completed("call_patch")).toMatchObject({ + content: [ + { type: "content", content: { type: "text", text: "patched" } }, + { type: "diff", path: file("indented.ts"), oldText: " const x = 1\n const y = 2\n", newText: indented }, + { type: "diff", path: file("formatted.ts"), oldText: "keep\nold\n", newText: formatted }, + { type: "diff", path: file("added.ts"), oldText: null, newText: added }, + { type: "diff", path: file("gone.ts"), oldText: "gone\n", newText: "" }, + ], + }) + expect(completed("call_snippet")).toMatchObject({ + content: [{ type: "content", content: { type: "text", text: "edited" } }], + }) + }) + test("routes slash commands and compact through their session endpoints", async () => { await using acp = await startWire() acp.server.catalog.commands = [reviewCommand, { name: "compact", description: "Server compact" }] @@ -439,6 +531,28 @@ function receivedBeforeResponse(acp: Wire) { return acp.updates.slice(0, count).map((item) => item.update) } +function trimmed(file: string, before: string, after: string) { + const recorded = fileDiff(file, before, after) + const lines = recorded.patch.split("\n") + const body = lines.filter((line) => /^[ +-]/.test(line) && !line.startsWith("---") && !line.startsWith("+++")) + const indent = body.reduce((result, line) => { + const value = line.slice(1) + if (value.trim().length === 0) return result + return Math.min(result, value.match(/^(\s*)/)?.[1].length ?? result) + }, Infinity) + if (indent === Infinity || indent === 0) return recorded + return { + ...recorded, + patch: lines + .map((line) => + /^[ +-]/.test(line) && !line.startsWith("---") && !line.startsWith("+++") + ? line[0] + line.slice(1 + indent) + : line, + ) + .join("\n"), + } +} + function turnUpdates(updates: readonly SessionNotification[]) { return updates.filter( (item) => item.update.sessionUpdate !== "available_commands_update" && item.update.sessionUpdate !== "usage_update", diff --git a/packages/cli/test/acp/replay.test.ts b/packages/cli/test/acp/replay.test.ts index 8e702cf8509d..9d4c0f083996 100644 --- a/packages/cli/test/acp/replay.test.ts +++ b/packages/cli/test/acp/replay.test.ts @@ -1,12 +1,17 @@ import { describe, expect, test } from "bun:test" import type { SessionMessage } from "@opencode/schema/session-message" -import { assistantMessage, makeSession, startWire } from "./wire-fixture" +import path from "node:path" +import { tmpdir } from "../fixture/tmpdir" +import { assistantMessage, fileDiff, makeSession, startWire } from "./wire-fixture" describe("acp session replay over the wire", () => { test("replays user, text, reasoning, and tool messages in order on session/load", async () => { + await using dir = await tmpdir() + const edited = path.resolve(dir.path, "edited.ts") + await Bun.write(edited, "one\r\nthree\r\n") await using acp = await startWire() acp.server.sessions.set("ses_replay", makeSession("ses_replay")) - acp.server.messages.set("ses_replay", replayFixtureMessages()) + acp.server.messages.set("ses_replay", replayFixtureMessages(edited)) await acp.initialize() await acp.request("session/load", { cwd: "/workspace", sessionId: "ses_replay", mcpServers: [] }) @@ -26,6 +31,8 @@ describe("acp session replay over the wire", () => { "tool_call", "tool_call_update", "tool_call", + "tool_call", + "tool_call_update", ]) expect(updates[1]?.update).toMatchObject({ content: { type: "resource_link", uri: "file:///workspace/note.md", name: "note.md", mimeType: "text/markdown" }, @@ -57,11 +64,20 @@ describe("acp session replay over the wire", () => { { type: "content", content: { type: "text", text: "failed hard" } }, ], }) + expect(updates[5]?.update).toMatchObject({ name: "shell" }) expect(updates[11]?.update).toMatchObject({ toolCallId: "call_streaming", status: "pending", rawInput: {} }) + expect(updates[13]?.update).toMatchObject({ + toolCallId: "call_edit", + status: "completed", + content: [ + { type: "content", content: { type: "text", text: "edited" } }, + { type: "diff", path: edited, oldText: "one\r\ntwo\r\n", newText: "one\r\nthree\r\n" }, + ], + }) }) }) -function replayFixtureMessages(): Array { +function replayFixtureMessages(edited: string): Array { return [ { id: "msg_user", @@ -120,6 +136,18 @@ function replayFixtureMessages(): Array { time: { created: 2 }, state: { status: "streaming", input: '{"command":' }, }, + { + type: "tool", + id: "call_edit", + name: "edit", + time: { created: 2, completed: 3 }, + state: { + status: "completed", + input: { path: edited, oldString: "two", newString: "three" }, + metadata: { files: [fileDiff(edited, "one\r\ntwo\r\n", "one\r\nthree\r\n")] }, + content: [{ type: "text", text: "edited" }], + }, + }, ], }), ] diff --git a/packages/cli/test/acp/translate.test.ts b/packages/cli/test/acp/translate.test.ts index 2a6d2e7358a7..b024a3d8988f 100644 --- a/packages/cli/test/acp/translate.test.ts +++ b/packages/cli/test/acp/translate.test.ts @@ -225,7 +225,7 @@ const rows: Row[] = [ ), expected: { updates: [ - { sessionUpdate: "tool_call", toolCallId: "call_ok", status: "pending", kind: "execute" }, + { sessionUpdate: "tool_call", toolCallId: "call_ok", name: "shell", status: "pending", kind: "execute" }, { sessionUpdate: "tool_call_update", status: "in_progress", diff --git a/packages/cli/test/acp/wire-fixture.ts b/packages/cli/test/acp/wire-fixture.ts index fe66a985e0eb..f0fc098ed3a1 100644 --- a/packages/cli/test/acp/wire-fixture.ts +++ b/packages/cli/test/acp/wire-fixture.ts @@ -28,6 +28,7 @@ import type { Session } from "@opencode/schema/session" import type { SessionMessage } from "@opencode/schema/session-message" import type { TokenUsage } from "@opencode/schema/token-usage" import type { BunRequest } from "bun" +import { createTwoFilesPatch } from "diff" import { Duration, Effect, Exit, Logger, Option, Schema, Scope } from "effect" import { FetchHttpClient } from "effect/unstable/http" import { ACP } from "../../src/acp/agent" @@ -337,6 +338,15 @@ export function toolProgress(sessionID: string, id: string, metadata: EventData< return ephemeralEvent("session.tool.progress", { sessionID, assistantMessageID: "msg_tools", id, metadata }) } +export function fileDiff( + file: string, + before: string, + after: string, + status: "added" | "deleted" | "modified" = "modified", +) { + return { file, patch: createTwoFilesPatch(file, file, before, after), additions: 1, deletions: 1, status } +} + export function toolSucceeded( sessionID: string, id: string,