Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 43 additions & 2 deletions packages/cli/src/acp/permission.ts
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -17,6 +17,7 @@ import {
pendingToolCall,
stringValue,
toLocations,
type DiffSource,
type ToolInput,
} from "./tool"

Expand Down Expand Up @@ -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 } : {}),
Expand All @@ -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,
Expand Down
13 changes: 12 additions & 1 deletion packages/cli/src/acp/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 },
),
),
Expand Down Expand Up @@ -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"
16 changes: 8 additions & 8 deletions packages/cli/src/acp/tool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ import { Result } from "effect"

export type ToolInput = Record<string, unknown>

export type DiffSource = {
readonly toolName: string
readonly input: ToolInput
readonly metadata?: Readonly<Record<string, unknown>>
}

function toToolKind(toolName: string): ToolKind {
switch (canonicalName(toolName)) {
case "shell":
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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 }),
},
Expand Down
18 changes: 14 additions & 4 deletions packages/cli/src/acp/translate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -56,8 +63,8 @@ type FormEvent = Extract<OpenCodeEvent, { type: "form.created" }>
type CreatedEvent = Extract<OpenCodeEvent, { type: "session.created" }>

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
Expand Down Expand Up @@ -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": {
Expand Down
25 changes: 15 additions & 10 deletions packages/cli/src/acp/turn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 3 additions & 4 deletions packages/cli/test/acp/permission.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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)
})
})
Expand All @@ -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 }
}
114 changes: 114 additions & 0 deletions packages/cli/test/acp/prompt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { pathToFileURL } from "node:url"
import { tmpdir } from "../fixture/tmpdir"
import {
delivered,
fileDiff,
durableEvent,
ephemeralEvent,
failed,
Expand Down Expand Up @@ -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" }]
Expand Down Expand Up @@ -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",
Expand Down
Loading
Loading