From 709aef3ff75b8854059475371b686dcaa3c6a148 Mon Sep 17 00:00:00 2001 From: Paco Preciado Mendoza Date: Mon, 8 Jun 2026 10:57:03 -0600 Subject: [PATCH 1/9] feat(core): add ReviewOverlay for ACP review staging In-memory overlay that holds staged file content during an ACP turn so add/update writes can be sent to the client for native review instead of hitting disk immediately. --- packages/core/src/review-overlay.ts | 79 +++++++++++++++++++++++ packages/core/test/review-overlay.test.ts | 56 ++++++++++++++++ 2 files changed, 135 insertions(+) create mode 100644 packages/core/src/review-overlay.ts create mode 100644 packages/core/test/review-overlay.test.ts diff --git a/packages/core/src/review-overlay.ts b/packages/core/src/review-overlay.ts new file mode 100644 index 000000000000..4a55fb396c48 --- /dev/null +++ b/packages/core/src/review-overlay.ts @@ -0,0 +1,79 @@ +export * as ReviewOverlay from "./review-overlay" + +import { FSUtil } from "./fs-util" + +// In-memory staging area for ACP review mode. While enabled, file writes are +// kept here instead of going to disk, and are later sent to the client so they +// show up in the native review UI. `entries` holds the current staged content +// (or a delete marker) per path; `pending` is the queue of writes still to send. +export type Entry = { readonly content: string } | { readonly deleted: true } + +export type PendingWrite = { sessionID: string; path: string; content: string } + +let enabled = false +let activeSession: string | undefined +const entries = new Map() +const pending: PendingWrite[] = [] + +export function setEnabled(value: boolean) { + enabled = value +} + +export function isEnabled() { + return enabled +} + +export function setActiveSession(sessionID: string | undefined) { + activeSession = sessionID +} + +export function stage(path: string, content: string) { + const key = FSUtil.resolve(path) + entries.set(key, { content }) + if (activeSession) pending.push({ sessionID: activeSession, path: key, content }) +} + +export function markDeleted(path: string) { + entries.set(FSUtil.resolve(path), { deleted: true }) +} + +export function get(path: string) { + return entries.get(FSUtil.resolve(path)) +} + +export function has(path: string) { + return entries.has(FSUtil.resolve(path)) +} + +// Recover staged edits that were written before a session was active, so a late +// flush still sends them. Skips paths already queued and delete markers. +export function enqueueUnflushed(sessionID: string) { + const queued = new Set(pending.map((item) => item.path)) + for (const [path, entry] of entries) { + if (!("content" in entry)) continue + if (queued.has(path)) continue + pending.push({ sessionID, path, content: entry.content }) + queued.add(path) + } +} + +export function drainPendingWrites() { + const drained = [...pending] + pending.length = 0 + return drained +} + +// Drop staged state at the end of a turn but keep review mode enabled. +export function clear() { + entries.clear() + pending.length = 0 + activeSession = undefined +} + +// Full reset, including disabling review mode. Used on shutdown and in tests. +export function reset() { + enabled = false + activeSession = undefined + entries.clear() + pending.length = 0 +} diff --git a/packages/core/test/review-overlay.test.ts b/packages/core/test/review-overlay.test.ts new file mode 100644 index 000000000000..697383c5c383 --- /dev/null +++ b/packages/core/test/review-overlay.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it } from "bun:test" +import { ReviewOverlay } from "../src/review-overlay" + +describe("ReviewOverlay", () => { + ReviewOverlay.reset() + + it("stage and get", () => { + ReviewOverlay.setEnabled(true) + ReviewOverlay.setActiveSession("sess-1") + ReviewOverlay.stage("/tmp/foo.ts", "hello") + expect(ReviewOverlay.get("/tmp/foo.ts")).toEqual({ content: "hello" }) + expect(ReviewOverlay.has("/tmp/foo.ts")).toBe(true) + }) + + it("drainPendingWrites", () => { + ReviewOverlay.reset() + ReviewOverlay.setEnabled(true) + ReviewOverlay.setActiveSession("sess-1") + ReviewOverlay.stage("/tmp/a.ts", "a") + ReviewOverlay.stage("/tmp/b.ts", "b") + const drained = ReviewOverlay.drainPendingWrites() + expect(drained).toEqual([ + { sessionID: "sess-1", path: expect.stringContaining("a.ts"), content: "a" }, + { sessionID: "sess-1", path: expect.stringContaining("b.ts"), content: "b" }, + ]) + expect(ReviewOverlay.drainPendingWrites()).toEqual([]) + }) + + it("markDeleted", () => { + ReviewOverlay.reset() + ReviewOverlay.stage("/tmp/gone.ts", "soon deleted") + ReviewOverlay.markDeleted("/tmp/gone.ts") + expect(ReviewOverlay.get("/tmp/gone.ts")).toEqual({ deleted: true }) + expect(ReviewOverlay.has("/tmp/gone.ts")).toBe(true) + }) + + it("enqueueUnflushed recovers staged content without pending queue", () => { + ReviewOverlay.reset() + ReviewOverlay.setEnabled(true) + ReviewOverlay.stage("/tmp/recover.ts", "staged") + expect(ReviewOverlay.drainPendingWrites()).toEqual([]) + ReviewOverlay.enqueueUnflushed("sess-2") + expect(ReviewOverlay.drainPendingWrites()).toEqual([ + { sessionID: "sess-2", path: expect.stringContaining("recover.ts"), content: "staged" }, + ]) + }) + + it("clear", () => { + ReviewOverlay.reset() + ReviewOverlay.setActiveSession("sess-1") + ReviewOverlay.stage("/tmp/x.ts", "x") + ReviewOverlay.clear() + expect(ReviewOverlay.has("/tmp/x.ts")).toBe(false) + expect(ReviewOverlay.drainPendingWrites()).toEqual([]) + }) +}) From 5ac60e3911de45a2ca142590d2363b9e34adbbee Mon Sep 17 00:00:00 2001 From: Paco Preciado Mendoza Date: Mon, 8 Jun 2026 10:57:07 -0600 Subject: [PATCH 2/9] feat(opencode): add review mode and ReviewFs layer Gate review staging on the client writeTextFile capability, flush staged edits through ACP, and wrap FSUtil so tool writes land in the overlay instead of disk while review mode is active. --- packages/opencode/src/acp/review-mode.ts | 37 +++++ packages/opencode/src/acp/review-staging.ts | 40 +++++ .../opencode/src/effect/review-fs-layer.ts | 140 ++++++++++++++++++ .../opencode/test/acp/review-mode.test.ts | 72 +++++++++ .../opencode/test/acp/review-staging.test.ts | 83 +++++++++++ .../test/effect/review-fs-layer.test.ts | 97 ++++++++++++ 6 files changed, 469 insertions(+) create mode 100644 packages/opencode/src/acp/review-mode.ts create mode 100644 packages/opencode/src/acp/review-staging.ts create mode 100644 packages/opencode/src/effect/review-fs-layer.ts create mode 100644 packages/opencode/test/acp/review-mode.test.ts create mode 100644 packages/opencode/test/acp/review-staging.test.ts create mode 100644 packages/opencode/test/effect/review-fs-layer.test.ts diff --git a/packages/opencode/src/acp/review-mode.ts b/packages/opencode/src/acp/review-mode.ts new file mode 100644 index 000000000000..543f5a836b1d --- /dev/null +++ b/packages/opencode/src/acp/review-mode.ts @@ -0,0 +1,37 @@ +export * as ACPReviewMode from "./review-mode" + +import { Flag, truthy } from "@opencode-ai/core/flag/flag" +import { ReviewOverlay } from "@opencode-ai/core/review-overlay" + +let clientWriteTextFile = false +let forcedForAcp = false + +export function forceEnableForAcp() { + forcedForAcp = true + syncEnabled() +} + +export function setClientWriteTextFileSupported(supported: boolean) { + clientWriteTextFile = supported + syncEnabled() +} + +// Review staging only works when the ACP client can receive staged edits via +// `fs/write_text_file`. Without that capability we must fall through to normal +// disk writes; otherwise edits would be staged in-memory and silently dropped +// (never written to disk nor sent to the client). +export function isActive() { + if (Flag.OPENCODE_CLIENT !== "acp") return false + if (!clientWriteTextFile) return false + return forcedForAcp || truthy("OPENCODE_ACP_REVIEW") +} + +export function syncEnabled() { + ReviewOverlay.setEnabled(isActive()) +} + +export function reset() { + clientWriteTextFile = false + forcedForAcp = false + ReviewOverlay.reset() +} diff --git a/packages/opencode/src/acp/review-staging.ts b/packages/opencode/src/acp/review-staging.ts new file mode 100644 index 000000000000..5783a549e48f --- /dev/null +++ b/packages/opencode/src/acp/review-staging.ts @@ -0,0 +1,40 @@ +export * as ACPReviewStaging from "./review-staging" + +import type { AgentSideConnection } from "@agentclientprotocol/sdk" +import * as Log from "@opencode-ai/core/util/log" +import { ReviewOverlay } from "@opencode-ai/core/review-overlay" +import { isActive } from "./review-mode" + +const log = Log.create({ service: "acp-review-staging" }) + +type Connection = Partial> + +export async function flushPendingWrites(connection: Connection | undefined, sessionID?: string) { + if (!isActive()) return + if (!connection?.writeTextFile) return + if (sessionID) { + ReviewOverlay.setActiveSession(sessionID) + ReviewOverlay.enqueueUnflushed(sessionID) + } + + const pending = ReviewOverlay.drainPendingWrites() + if (pending.length > 0) { + log.info("flushing staged writes", { count: pending.length }) + } + + for (const item of pending) { + await connection + .writeTextFile({ + sessionId: item.sessionID, + path: item.path, + content: item.content, + }) + .catch((error: unknown) => { + log.error("failed to write staged edit through ACP", { + error, + path: item.path, + sessionID: item.sessionID, + }) + }) + } +} diff --git a/packages/opencode/src/effect/review-fs-layer.ts b/packages/opencode/src/effect/review-fs-layer.ts new file mode 100644 index 000000000000..2c3f14fe7cc6 --- /dev/null +++ b/packages/opencode/src/effect/review-fs-layer.ts @@ -0,0 +1,140 @@ +import { Effect, FileSystem, Layer, Option } from "effect" +import * as PlatformError from "effect/PlatformError" +import { FSUtil } from "@opencode-ai/core/fs-util" +import { ReviewOverlay } from "@opencode-ai/core/review-overlay" + +// FSUtil layer for ACP review mode. It wraps the real FSUtil: writes are sent to +// the ReviewOverlay (staged in memory) and reads return staged content, so the +// agent sees its own pending edits without touching disk. When review mode is +// off, every call passes straight through to the real FSUtil. + +const overlayNotFound = (method: string, path: string) => + PlatformError.systemError({ + _tag: "NotFound", + module: "FileSystem", + method, + pathOrDescriptor: path, + syscall: method, + }) + +function overlayFileStat(content: string): FileSystem.File.Info { + const now = Option.some(new Date()) + return { + type: "File", + mtime: now, + atime: now, + birthtime: now, + dev: 0, + rdev: Option.some(0), + ino: Option.none(), + mode: 0o644, + nlink: Option.some(1), + uid: Option.none(), + gid: Option.none(), + size: FileSystem.Size(new TextEncoder().encode(content).length), + blksize: Option.none(), + blocks: Option.none(), + } +} + +export const layer = Layer.effect( + FSUtil.Service, + Effect.gen(function* () { + const fs = yield* FSUtil.Service + + const readFileString = Effect.fn("ReviewFs.readFileString")(function* (path: string) { + if (!ReviewOverlay.isEnabled()) return yield* fs.readFileString(path) + const entry = ReviewOverlay.get(path) + if (entry) { + if ("deleted" in entry) return yield* Effect.fail(overlayNotFound("readFileString", path)) + return entry.content + } + return yield* fs.readFileString(path) + }) + + const readFile = Effect.fn("ReviewFs.readFile")(function* (path: string) { + if (!ReviewOverlay.isEnabled()) return yield* fs.readFile(path) + const entry = ReviewOverlay.get(path) + if (entry) { + if ("deleted" in entry) return yield* Effect.fail(overlayNotFound("readFile", path)) + return new TextEncoder().encode(entry.content) + } + return yield* fs.readFile(path) + }) + + const readFileStringSafe = Effect.fn("ReviewFs.readFileStringSafe")(function* (path: string) { + if (!ReviewOverlay.isEnabled()) return yield* fs.readFileStringSafe(path) + const entry = ReviewOverlay.get(path) + if (entry) { + if ("deleted" in entry) return undefined + return entry.content + } + return yield* fs.readFileStringSafe(path) + }) + + const exists = Effect.fn("ReviewFs.exists")(function* (path: string) { + if (!ReviewOverlay.isEnabled()) return yield* fs.exists(path) + const entry = ReviewOverlay.get(path) + if (entry) return !("deleted" in entry) + return yield* fs.exists(path) + }) + + const stat = Effect.fn("ReviewFs.stat")(function* (path: string) { + if (!ReviewOverlay.isEnabled()) return yield* fs.stat(path) + const entry = ReviewOverlay.get(path) + if (entry) { + if ("deleted" in entry) return yield* Effect.fail(overlayNotFound("stat", path)) + const disk = yield* fs.stat(path).pipe(Effect.catch(() => Effect.succeed(undefined))) + if (disk?.type === "File") { + return { ...disk, size: FileSystem.Size(new TextEncoder().encode(entry.content).length) } + } + if (disk) return disk + return overlayFileStat(entry.content) + } + return yield* fs.stat(path) + }) + + const isFile = Effect.fn("ReviewFs.isFile")(function* (path: string) { + if (!ReviewOverlay.isEnabled()) return yield* fs.isFile(path) + const entry = ReviewOverlay.get(path) + if (entry) return !("deleted" in entry) + return yield* fs.isFile(path) + }) + + const isDir = Effect.fn("ReviewFs.isDir")(function* (path: string) { + if (!ReviewOverlay.isEnabled()) return yield* fs.isDir(path) + const entry = ReviewOverlay.get(path) + if (entry) return false + return yield* fs.isDir(path) + }) + + const writeWithDirs = Effect.fn("ReviewFs.writeWithDirs")(function* ( + path: string, + content: string | Uint8Array, + mode?: number, + ) { + // Only text writes can be staged and shown in the review UI. Binary writes + // (and any write while review mode is off) go straight to disk. + if (!ReviewOverlay.isEnabled() || typeof content !== "string") { + return yield* fs.writeWithDirs(path, content, mode) + } + ReviewOverlay.stage(path, content) + }) + + return FSUtil.Service.of({ + ...fs, + readFileString, + readFile, + readFileStringSafe, + exists, + stat, + isFile, + isDir, + writeWithDirs, + }) + }), +).pipe(Layer.provide(FSUtil.defaultLayer)) + +export const defaultLayer = layer + +export * as ReviewFs from "./review-fs-layer" diff --git a/packages/opencode/test/acp/review-mode.test.ts b/packages/opencode/test/acp/review-mode.test.ts new file mode 100644 index 000000000000..281806c71fad --- /dev/null +++ b/packages/opencode/test/acp/review-mode.test.ts @@ -0,0 +1,72 @@ +import { afterEach, describe, expect, it } from "bun:test" +import { ReviewOverlay } from "@opencode-ai/core/review-overlay" +import { forceEnableForAcp, isActive, reset, setClientWriteTextFileSupported, syncEnabled } from "@/acp/review-mode" + +describe("ACPReviewMode", () => { + afterEach(() => { + delete process.env.OPENCODE_ACP_REVIEW + delete process.env.OPENCODE_CLIENT + reset() + }) + + it("is inactive without flag even when capability is present", () => { + process.env.OPENCODE_CLIENT = "acp" + setClientWriteTextFileSupported(true) + syncEnabled() + expect(isActive()).toBe(false) + expect(ReviewOverlay.isEnabled()).toBe(false) + }) + + it("is inactive outside acp client", () => { + process.env.OPENCODE_ACP_REVIEW = "1" + process.env.OPENCODE_CLIENT = "cli" + setClientWriteTextFileSupported(true) + syncEnabled() + expect(isActive()).toBe(false) + }) + + it("is inactive when the client lacks writeTextFile capability, even with the env flag", () => { + process.env.OPENCODE_ACP_REVIEW = "1" + process.env.OPENCODE_CLIENT = "acp" + setClientWriteTextFileSupported(false) + syncEnabled() + expect(isActive()).toBe(false) + expect(ReviewOverlay.isEnabled()).toBe(false) + }) + + it("is inactive when forced but the client lacks writeTextFile capability", () => { + process.env.OPENCODE_CLIENT = "acp" + forceEnableForAcp() + setClientWriteTextFileSupported(false) + expect(isActive()).toBe(false) + expect(ReviewOverlay.isEnabled()).toBe(false) + }) + + it("is active when flag, client, and capability are set", () => { + process.env.OPENCODE_ACP_REVIEW = "1" + process.env.OPENCODE_CLIENT = "acp" + setClientWriteTextFileSupported(true) + syncEnabled() + expect(isActive()).toBe(true) + expect(ReviewOverlay.isEnabled()).toBe(true) + }) + + it("is active when forced and the client supports writeTextFile, without the env flag", () => { + process.env.OPENCODE_CLIENT = "acp" + forceEnableForAcp() + setClientWriteTextFileSupported(true) + expect(isActive()).toBe(true) + expect(ReviewOverlay.isEnabled()).toBe(true) + }) + + it("deactivates when capability is later reported missing", () => { + process.env.OPENCODE_CLIENT = "acp" + forceEnableForAcp() + setClientWriteTextFileSupported(true) + expect(isActive()).toBe(true) + + setClientWriteTextFileSupported(false) + expect(isActive()).toBe(false) + expect(ReviewOverlay.isEnabled()).toBe(false) + }) +}) diff --git a/packages/opencode/test/acp/review-staging.test.ts b/packages/opencode/test/acp/review-staging.test.ts new file mode 100644 index 000000000000..9d26c1da7247 --- /dev/null +++ b/packages/opencode/test/acp/review-staging.test.ts @@ -0,0 +1,83 @@ +import { afterEach, describe, expect, it } from "bun:test" +import { ReviewOverlay } from "@opencode-ai/core/review-overlay" +import { flushPendingWrites } from "@/acp/review-staging" +import { reset, setClientWriteTextFileSupported, syncEnabled } from "@/acp/review-mode" + +describe("ACPReviewStaging", () => { + afterEach(() => { + delete process.env.OPENCODE_ACP_REVIEW + delete process.env.OPENCODE_CLIENT + reset() + }) + + it("awaits one writeTextFile per staged path", async () => { + process.env.OPENCODE_ACP_REVIEW = "1" + process.env.OPENCODE_CLIENT = "acp" + setClientWriteTextFileSupported(true) + syncEnabled() + + ReviewOverlay.setActiveSession("sess-1") + ReviewOverlay.stage("/tmp/a.ts", "a") + ReviewOverlay.stage("/tmp/b.ts", "b") + + const calls: Array<{ sessionId: string; path: string; content: string }> = [] + await flushPendingWrites({ + writeTextFile: async (input) => { + calls.push(input) + return {} + }, + }) + + expect(calls).toHaveLength(2) + expect(calls[0]?.sessionId).toBe("sess-1") + expect(calls[0]?.content).toBe("a") + expect(calls[1]?.content).toBe("b") + expect(ReviewOverlay.drainPendingWrites()).toEqual([]) + }) + + it("recovers staged files when session was set late", async () => { + process.env.OPENCODE_ACP_REVIEW = "1" + process.env.OPENCODE_CLIENT = "acp" + setClientWriteTextFileSupported(true) + syncEnabled() + + ReviewOverlay.stage("/tmp/late.ts", "late content") + + const calls: Array<{ sessionId: string; path: string; content: string }> = [] + await flushPendingWrites( + { + writeTextFile: async (input) => { + calls.push(input) + return {} + }, + }, + "sess-late", + ) + + expect(calls).toEqual([ + { + sessionId: "sess-late", + path: expect.stringContaining("late.ts"), + content: "late content", + }, + ]) + }) + + it("skips flush when review mode is inactive", async () => { + ReviewOverlay.setEnabled(true) + ReviewOverlay.setActiveSession("sess-1") + ReviewOverlay.stage("/tmp/a.ts", "a") + + let called = false + await flushPendingWrites({ + writeTextFile: async () => { + called = true + return {} + }, + }) + + expect(called).toBe(false) + expect(ReviewOverlay.drainPendingWrites()).toHaveLength(1) + ReviewOverlay.reset() + }) +}) diff --git a/packages/opencode/test/effect/review-fs-layer.test.ts b/packages/opencode/test/effect/review-fs-layer.test.ts new file mode 100644 index 000000000000..747d578c722e --- /dev/null +++ b/packages/opencode/test/effect/review-fs-layer.test.ts @@ -0,0 +1,97 @@ +import { afterEach, describe, expect } from "bun:test" +import fs from "fs/promises" +import path from "path" +import { Effect } from "effect" +import { FSUtil } from "@opencode-ai/core/fs-util" +import { ReviewOverlay } from "@opencode-ai/core/review-overlay" +import { ReviewFs } from "@/effect/review-fs-layer" +import { TestInstance } from "../fixture/fixture" +import { testEffect } from "../lib/effect" + +const it = testEffect(ReviewFs.defaultLayer) + +afterEach(() => { + ReviewOverlay.reset() +}) + +const readDisk = (filepath: string) => Effect.promise(() => fs.readFile(filepath, "utf-8").catch(() => "")) + +describe("ReviewFs overlay", () => { + it.instance("stages writes without mutating disk", () => + Effect.gen(function* () { + const test = yield* TestInstance + const afs = yield* FSUtil.Service + ReviewOverlay.setEnabled(true) + ReviewOverlay.setActiveSession("sess-1") + + const filepath = path.join(test.directory, "staged.txt") + yield* afs.writeWithDirs(filepath, "staged content") + + expect(yield* readDisk(filepath)).toBe("") + expect(yield* afs.readFileString(filepath)).toBe("staged content") + expect(yield* afs.exists(filepath)).toBe(true) + expect(yield* afs.isFile(filepath)).toBe(true) + const info = yield* afs.stat(filepath) + expect(info.type).toBe("File") + }), + ) + + it.instance("read-after-write returns latest staged content", () => + Effect.gen(function* () { + const test = yield* TestInstance + const afs = yield* FSUtil.Service + ReviewOverlay.setEnabled(true) + + const filepath = path.join(test.directory, "twice.txt") + yield* afs.writeWithDirs(filepath, "first") + yield* afs.writeWithDirs(filepath, "second") + expect(yield* afs.readFileString(filepath)).toBe("second") + expect(yield* readDisk(filepath)).toBe("") + }), + ) + + it.instance("readFile returns staged bytes for Bom.readFile", () => + Effect.gen(function* () { + const test = yield* TestInstance + const afs = yield* FSUtil.Service + ReviewOverlay.setEnabled(true) + + const filepath = path.join(test.directory, "bytes.txt") + yield* afs.writeWithDirs(filepath, "utf8 body") + const bytes = yield* afs.readFile(filepath) + expect(new TextDecoder().decode(bytes)).toBe("utf8 body") + }), + ) + + it.instance("markDeleted hides overlay paths", () => + Effect.gen(function* () { + const test = yield* TestInstance + const afs = yield* FSUtil.Service + ReviewOverlay.setEnabled(true) + + const filepath = path.join(test.directory, "gone.txt") + yield* Effect.promise(() => fs.writeFile(filepath, "on disk", "utf-8")) + ReviewOverlay.markDeleted(filepath) + + expect(yield* afs.exists(filepath)).toBe(false) + expect(yield* afs.isFile(filepath)).toBe(false) + const stat = yield* afs.stat(filepath).pipe(Effect.catch(() => Effect.succeed(undefined))) + expect(stat).toBeUndefined() + }), + ) + + it.instance("updates see staged content from disk baseline", () => + Effect.gen(function* () { + const test = yield* TestInstance + const afs = yield* FSUtil.Service + ReviewOverlay.setEnabled(true) + + const filepath = path.join(test.directory, "base.txt") + yield* Effect.promise(() => fs.writeFile(filepath, "line1\nline2\n", "utf-8")) + yield* afs.writeWithDirs(filepath, "line1\nchanged\n") + + expect(yield* afs.readFileString(filepath)).toBe("line1\nchanged\n") + expect(yield* readDisk(filepath)).toBe("line1\nline2\n") + }), + ) +}) From 3ffb147f08edb21951a2083d40e08f98853c1ffd Mon Sep 17 00:00:00 2001 From: Paco Preciado Mendoza Date: Mon, 8 Jun 2026 10:57:10 -0600 Subject: [PATCH 3/9] feat(acp): stage file edits for native review in ACP clients Wire review mode into ACP session lifecycle, tool registry, and edit/write/ apply_patch tools so add/update changes reach the client review UI via writeTextFile. Delete/move still hit disk and show as tool_call diffs. --- packages/opencode/src/acp/event.ts | 8 + packages/opencode/src/acp/permission.ts | 5 +- packages/opencode/src/acp/service.ts | 159 ++++++++------- packages/opencode/src/acp/tool.ts | 36 ++++ packages/opencode/src/cli/cmd/acp.ts | 5 + packages/opencode/src/tool/apply_patch.ts | 66 ++++--- packages/opencode/src/tool/edit.ts | 54 +++-- packages/opencode/src/tool/write.ts | 54 +++-- packages/opencode/test/acp/event.test.ts | 54 ++++- .../opencode/test/acp/service-session.test.ts | 59 ++++++ packages/opencode/test/acp/tool.test.ts | 64 ++++++ .../test/tool/review-mode-tools.test.ts | 187 ++++++++++++++++++ 12 files changed, 619 insertions(+), 132 deletions(-) create mode 100644 packages/opencode/test/tool/review-mode-tools.test.ts diff --git a/packages/opencode/src/acp/event.ts b/packages/opencode/src/acp/event.ts index 7d05fa6ee58c..c19974add5ef 100644 --- a/packages/opencode/src/acp/event.ts +++ b/packages/opencode/src/acp/event.ts @@ -12,6 +12,7 @@ import { Effect } from "effect" import { ACPSession } from "./session" import { ACPPermission } from "./permission" import { partsToContentChunks, type ReplayPart } from "./content" +import { ReviewOverlay } from "@opencode-ai/core/review-overlay" import { duplicateRunningToolUpdate, errorToolUpdate, @@ -20,6 +21,7 @@ import { shellOutputSnapshot, completedToolUpdate, } from "./tool" +import { flushPendingWrites } from "./review-staging" type Connection = Pick & Partial> @@ -233,6 +235,7 @@ export class Subscription { } private async handleToolPart(sessionId: string, part: ToolPart, cwd: string) { + ReviewOverlay.setActiveSession(sessionId) await this.toolStart(sessionId, part, cwd) switch (part.state.status) { @@ -258,6 +261,11 @@ export class Subscription { }), }, }) + // These tools may have staged edits in the overlay. Send them to the + // client now so they show up in the native review UI as the tool finishes. + if (part.tool === "edit" || part.tool === "write" || part.tool === "apply_patch") { + await flushPendingWrites(this.input.connection, sessionId) + } return case "error": diff --git a/packages/opencode/src/acp/permission.ts b/packages/opencode/src/acp/permission.ts index 4eeca28f09f5..de8050b6c313 100644 --- a/packages/opencode/src/acp/permission.ts +++ b/packages/opencode/src/acp/permission.ts @@ -12,6 +12,7 @@ import { exists, readText } from "@/util/filesystem" import type { ACPSession } from "./session" import { pendingToolCall, toLocations, type ToolInput } from "./tool" import { Effect } from "effect" +import { isActive } from "./review-mode" type PermissionEvent = Extract type Reply = "once" | "always" | "reject" @@ -81,7 +82,9 @@ export class Handler { return } - if (permission.permission === "edit") { + // In review mode the overlay already sends edits to the client, so skip the + // proposed-edit message to avoid showing the same change twice. + if (permission.permission === "edit" && !isActive()) { await this.writeProposedEdit(session.id, permission.metadata).catch(() => {}) } diff --git a/packages/opencode/src/acp/service.ts b/packages/opencode/src/acp/service.ts index 7257efa7658c..c9d32dc1bc7c 100644 --- a/packages/opencode/src/acp/service.ts +++ b/packages/opencode/src/acp/service.ts @@ -41,6 +41,9 @@ import { ACPEvent } from "./event" import { ACPSession } from "./session" import { UsageService } from "./usage" import { ACPProfile } from "./profile" +import { ReviewOverlay } from "@opencode-ai/core/review-overlay" +import { syncEnabled, setClientWriteTextFileSupported } from "./review-mode" +import { flushPendingWrites } from "./review-staging" import { ProviderV2 } from "@opencode-ai/core/provider" import { ModelV2 } from "@opencode-ai/core/model" import { Provider } from "@/provider/provider" @@ -107,6 +110,14 @@ export function make(input: { } } + // Review mode needs the client to accept staged edits via writeTextFile. + // Record the capability from initialize so review activates only when usable. + setClientWriteTextFileSupported(params.clientCapabilities?.fs?.writeTextFile === true) + syncEnabled() + if (ReviewOverlay.isEnabled()) { + log.info("ACP review-at-end active") + } + const response = { protocolVersion: 1, agentCapabilities: { @@ -340,6 +351,7 @@ export function make(input: { const removed = yield* session.remove(params.sessionId) registeredMcp.delete(params.sessionId) sessionSnapshots.delete(params.sessionId) + ReviewOverlay.clear() if (!removed) return {} yield* abortBackingSession(removed) @@ -348,6 +360,7 @@ export function make(input: { const cancel = Effect.fn("ACP.cancel")(function* (params: CancelNotification) { const current = yield* session.get(params.sessionId) + ReviewOverlay.clear() yield* abortBackingSession(current) }) @@ -491,79 +504,89 @@ export function make(input: { setSessionModel, prompt: Effect.fn("ACP.prompt")(function* (params: PromptRequest) { const current = yield* session.get(params.sessionId) - const snapshot = yield* directorySnapshot(current.cwd) - const selected = current.model ?? selectDefaultModel(snapshot) - if (!current.model) { - yield* session.setModel(params.sessionId, selected) - } - const variant = current.variant ?? selectVariant(snapshot, selected) - const modeId = current.modeId ?? (snapshot.availableModes.length > 0 ? snapshot.defaultModeID : undefined) - const parts = promptContentToParts(params.prompt) - const command = detectSlashCommand(parts) - - if (!command) { - const response = yield* request( - () => - input.sdk.session.prompt( - { - sessionID: current.id, - model: { + ReviewOverlay.setActiveSession(current.id) + syncEnabled() + // Always clear the staging overlay when the turn settles — success, + // failure, or interruption — so partial edits from a failed turn never + // leak into the next prompt. + return yield* Effect.gen(function* () { + const snapshot = yield* directorySnapshot(current.cwd) + const selected = current.model ?? selectDefaultModel(snapshot) + if (!current.model) { + yield* session.setModel(params.sessionId, selected) + } + const variant = current.variant ?? selectVariant(snapshot, selected) + const modeId = current.modeId ?? (snapshot.availableModes.length > 0 ? snapshot.defaultModeID : undefined) + const parts = promptContentToParts(params.prompt) + const command = detectSlashCommand(parts) + + if (!command) { + const response = yield* request( + () => + input.sdk.session.prompt( + { + sessionID: current.id, + model: { + providerID: selected.providerID, + modelID: selected.modelID, + }, + ...(variant ? { variant } : {}), + parts, + ...(modeId ? { agent: modeId } : {}), + directory: current.cwd, + }, + { throwOnError: true }, + ), + "session", + ) + yield* sendUsageUpdate(input.usage, input.sdk, input.connection, current.id, current.cwd) + yield* Effect.promise(() => flushPendingWrites(input.connection, current.id)) + return yield* promptResponse(response.info, params.messageId) + } + + const known = snapshot.availableCommands.find((item) => item.name === command.name) + if (known) { + const response = yield* request( + () => + input.sdk.session.command( + { + sessionID: current.id, + command: known.name, + arguments: command.args, + model: `${selected.providerID}/${selected.modelID}`, + ...(variant ? { variant } : {}), + ...(modeId ? { agent: modeId } : {}), + directory: current.cwd, + }, + { throwOnError: true }, + ), + "session", + ) + yield* sendUsageUpdate(input.usage, input.sdk, input.connection, current.id, current.cwd) + yield* Effect.promise(() => flushPendingWrites(input.connection, current.id)) + return yield* promptResponse(response.info, params.messageId) + } + + if (command.name === "compact") { + yield* request( + () => + input.sdk.session.summarize( + { + sessionID: current.id, + directory: current.cwd, providerID: selected.providerID, modelID: selected.modelID, }, - ...(variant ? { variant } : {}), - parts, - ...(modeId ? { agent: modeId } : {}), - directory: current.cwd, - }, - { throwOnError: true }, - ), - "session", - ) - yield* sendUsageUpdate(input.usage, input.sdk, input.connection, current.id, current.cwd) - return yield* promptResponse(response.info, params.messageId) - } + { throwOnError: true }, + ), + "session", + ) + } - const known = snapshot.availableCommands.find((item) => item.name === command.name) - if (known) { - const response = yield* request( - () => - input.sdk.session.command( - { - sessionID: current.id, - command: known.name, - arguments: command.args, - model: `${selected.providerID}/${selected.modelID}`, - ...(variant ? { variant } : {}), - ...(modeId ? { agent: modeId } : {}), - directory: current.cwd, - }, - { throwOnError: true }, - ), - "session", - ) yield* sendUsageUpdate(input.usage, input.sdk, input.connection, current.id, current.cwd) - return yield* promptResponse(response.info, params.messageId) - } - - if (command.name === "compact") { - yield* request( - () => - input.sdk.session.summarize( - { - sessionID: current.id, - directory: current.cwd, - providerID: selected.providerID, - modelID: selected.modelID, - }, - { throwOnError: true }, - ), - "session", - ) - } - - yield* sendUsageUpdate(input.usage, input.sdk, input.connection, current.id, current.cwd) - return yield* promptResponse(undefined, params.messageId) + yield* Effect.promise(() => flushPendingWrites(input.connection, current.id)) + return yield* promptResponse(undefined, params.messageId) + }).pipe(Effect.ensuring(Effect.sync(() => ReviewOverlay.clear()))) }), cancel, } diff --git a/packages/opencode/src/acp/tool.ts b/packages/opencode/src/acp/tool.ts index 309c126d6e99..43f00cab9931 100644 --- a/packages/opencode/src/acp/tool.ts +++ b/packages/opencode/src/acp/tool.ts @@ -113,6 +113,10 @@ export function completedToolContent(toolName: string, state: CompletedToolState }, ] + if (toolName.toLocaleLowerCase() === "apply_patch") { + content.push(...applyPatchDiffContent(state.metadata)) + } + if (toToolKind(toolName) === "edit") { content.push(...diffContent(state.input)) } @@ -307,6 +311,14 @@ export const buildDuplicateRunningToolUpdate = duplicateRunningToolUpdate export const buildCompletedToolUpdate = completedToolUpdate export const buildErrorToolUpdate = errorToolUpdate +export function deleteDiffContent(path: string, oldText: string): ToolCallContent[] { + return [{ type: "diff", path, oldText, newText: "" }] +} + +export function moveDiffContent(dest: string, oldText: string, newText: string): ToolCallContent[] { + return [{ type: "diff", path: dest, oldText, newText }] +} + function locationFrom(...values: unknown[]): ToolCallLocation[] { return Array.from( new Set( @@ -322,6 +334,30 @@ function locationFrom(...values: unknown[]): ToolCallLocation[] { ) } +// apply_patch deletes and moves go straight to disk, so the client never gets a +// writeTextFile for them. Build diff content from the tool metadata so those +// changes still show up as a diff in the tool call. Metadata is untrusted, so +// every field is checked before use. +function applyPatchDiffContent(metadata: unknown): ToolCallContent[] { + if (!metadata || typeof metadata !== "object") return [] + const files = (metadata as Record).files + if (!Array.isArray(files)) return [] + + return files.flatMap((file): ToolCallContent[] => { + if (!file || typeof file !== "object") return [] + const info = file as Record + const type = stringValue(info.type) + const path = stringValue(info.diffPath) ?? stringValue(info.movePath) ?? stringValue(info.filePath) + const oldText = stringValue(info.oldText) + if (!path || oldText === undefined) return [] + const newText = stringValue(info.newText) ?? "" + + if (type === "delete") return deleteDiffContent(path, oldText) + if (type === "move") return moveDiffContent(path, oldText, newText) + return [] + }) +} + function diffContent(input: ToolInput): ToolCallContent[] { const oldText = stringValue(input.oldString) const newText = stringValue(input.newString) ?? stringValue(input.content) diff --git a/packages/opencode/src/cli/cmd/acp.ts b/packages/opencode/src/cli/cmd/acp.ts index da47d9579578..b5c067ba0783 100644 --- a/packages/opencode/src/cli/cmd/acp.ts +++ b/packages/opencode/src/cli/cmd/acp.ts @@ -5,6 +5,7 @@ import { ServerAuth } from "@/server/auth" import { createOpencodeClient } from "@opencode-ai/sdk/v2" import { withNetworkOptions, resolveNetworkOptions } from "../network" import { ACPProfile } from "@/acp/profile" +import { forceEnableForAcp } from "@/acp/review-mode" export const AcpCommand = effectCmd({ command: "acp", @@ -21,6 +22,10 @@ export const AcpCommand = effectCmd({ const { ACP } = yield* Effect.promise(() => import("@/acp/agent")) ACPProfile.mark("cli.acp.handler") process.env.OPENCODE_CLIENT = "acp" + // Opt this ACP session into review mode. It still only activates if the + // client reports the writeTextFile capability during initialize. + forceEnableForAcp() + log.info("ACP review-at-end enabled") const opts = yield* resolveNetworkOptions(args) const server = yield* Effect.promise(() => ACPProfile.measure("cli.acp.server.listen", () => Server.listen(opts))) diff --git a/packages/opencode/src/tool/apply_patch.ts b/packages/opencode/src/tool/apply_patch.ts index f9201be8a7db..bf963b934883 100644 --- a/packages/opencode/src/tool/apply_patch.ts +++ b/packages/opencode/src/tool/apply_patch.ts @@ -14,6 +14,8 @@ import DESCRIPTION from "./apply_patch.txt" import { FileSystem } from "@opencode-ai/core/filesystem" import { Format } from "../format" import * as Bom from "@/util/bom" +import { isActive } from "@/acp/review-mode" +import { ReviewOverlay } from "@opencode-ai/core/review-overlay" export const Parameters = Schema.Struct({ patchText: Schema.String.annotate({ description: "The full patch text that describes all changes to be made" }), @@ -199,6 +201,9 @@ export const ApplyPatchTool = Tool.define( additions: change.additions, deletions: change.deletions, movePath: change.movePath, + diffPath: change.movePath ?? change.filePath, + oldText: change.oldContent, + newText: change.type === "delete" ? "" : change.newContent, })) // Check permissions if needed @@ -214,6 +219,11 @@ export const ApplyPatchTool = Tool.define( }, }) + // In review mode, add/update writes are staged in the overlay (see ReviewFs) + // instead of going to disk. Skip the disk-only steps below: formatting, + // file events, and LSP diagnostics. + const review = isActive() + // Apply the changes const updates: Array<{ file: string; event: "add" | "change" | "unlink" }> = [] @@ -221,8 +231,6 @@ export const ApplyPatchTool = Tool.define( const edited = change.type === "delete" ? undefined : (change.movePath ?? change.filePath) switch (change.type) { case "add": - // Create parent directories (recursive: true is safe on existing/root dirs) - yield* afs.writeWithDirs(change.filePath, Bom.join(change.newContent, change.bom)) updates.push({ file: change.filePath, event: "add" }) break @@ -234,10 +242,9 @@ export const ApplyPatchTool = Tool.define( case "move": if (change.movePath) { - // Create parent directories (recursive: true is safe on existing/root dirs) - yield* afs.writeWithDirs(change.movePath!, Bom.join(change.newContent, change.bom)) yield* afs.remove(change.filePath) + if (review) ReviewOverlay.markDeleted(change.filePath) updates.push({ file: change.filePath, event: "unlink" }) updates.push({ file: change.movePath, event: "add" }) } @@ -245,11 +252,12 @@ export const ApplyPatchTool = Tool.define( case "delete": yield* afs.remove(change.filePath) + if (review) ReviewOverlay.markDeleted(change.filePath) updates.push({ file: change.filePath, event: "unlink" }) break } - if (edited) { + if (edited && !review) { if (yield* format.file(edited)) { yield* Bom.syncFile(afs, edited, change.bom) } @@ -257,18 +265,22 @@ export const ApplyPatchTool = Tool.define( } } - // Publish file change events - for (const update of updates) { - yield* events.publish(Watcher.Event.Updated, update) + if (!review) { + for (const update of updates) { + yield* events.publish(Watcher.Event.Updated, update) + } } - // Notify LSP of file changes and collect diagnostics - for (const change of fileChanges) { - if (change.type === "delete") continue - const target = change.movePath ?? change.filePath - yield* lsp.touchFile(target, "document") - } - const diagnostics = yield* lsp.diagnostics() + const diagnostics = review + ? {} + : yield* Effect.gen(function* () { + for (const change of fileChanges) { + if (change.type === "delete") continue + const target = change.movePath ?? change.filePath + yield* lsp.touchFile(target, "document") + } + return yield* lsp.diagnostics() + }) // Generate output summary const summaryLines = fileChanges.map((change) => { @@ -283,13 +295,23 @@ export const ApplyPatchTool = Tool.define( }) let output = `Success. Updated the following files:\n${summaryLines.join("\n")}` - for (const change of fileChanges) { - if (change.type === "delete") continue - const target = change.movePath ?? change.filePath - const block = LSP.Diagnostic.report(target, diagnostics[FSUtil.normalizePath(target)] ?? []) - if (!block) continue - const rel = path.relative(instance.worktree, target).replaceAll("\\", "/") - output += `\n\nLSP errors detected in ${rel}, please fix:\n${block}` + // ACP has no deleteFile/renameFile, so deletions and moves are applied to + // disk immediately even in review mode (only add/update are staged for + // review). Surface this so a reviewer knows those changes are not undone + // by rejecting the staged edits. + if (review && fileChanges.some((change) => change.type === "delete" || change.type === "move")) { + output += `\n\nNote: deletions and moves were applied directly to disk and are not part of the staged review.` + } + + if (!review) { + for (const change of fileChanges) { + if (change.type === "delete") continue + const target = change.movePath ?? change.filePath + const block = LSP.Diagnostic.report(target, diagnostics[FSUtil.normalizePath(target)] ?? []) + if (!block) continue + const rel = path.relative(instance.worktree, target).replaceAll("\\", "/") + output += `\n\nLSP errors detected in ${rel}, please fix:\n${block}` + } } return { diff --git a/packages/opencode/src/tool/edit.ts b/packages/opencode/src/tool/edit.ts index a92e4720c0fc..5a3480fb3c73 100644 --- a/packages/opencode/src/tool/edit.ts +++ b/packages/opencode/src/tool/edit.ts @@ -18,6 +18,7 @@ import { Snapshot } from "@/snapshot" import { assertExternalDirectoryEffect } from "./external-directory" import { FSUtil } from "@opencode-ai/core/fs-util" import * as Bom from "@/util/bom" +import { isActive } from "@/acp/review-mode" function normalizeLineEndings(text: string): string { return text.replaceAll("\r\n", "\n") @@ -109,14 +110,18 @@ export const EditTool = Tool.define( }, }) yield* afs.writeWithDirs(filePath, Bom.join(contentNew, desiredBom)) - if (yield* format.file(filePath)) { - contentNew = yield* Bom.syncFile(afs, filePath, desiredBom) + // In review mode the write above is staged in the overlay, not on + // disk. Skip disk-only follow-ups (format + file events) here. + if (!isActive()) { + if (yield* format.file(filePath)) { + contentNew = yield* Bom.syncFile(afs, filePath, desiredBom) + } + yield* events.publish(FileSystem.Event.Edited, { file: filePath }) + yield* events.publish(Watcher.Event.Updated, { + file: filePath, + event: "add", + }) } - yield* events.publish(FileSystem.Event.Edited, { file: filePath }) - yield* events.publish(Watcher.Event.Updated, { - file: filePath, - event: "add", - }) return } @@ -153,14 +158,17 @@ export const EditTool = Tool.define( }) yield* afs.writeWithDirs(filePath, Bom.join(contentNew, desiredBom)) - if (yield* format.file(filePath)) { - contentNew = yield* Bom.syncFile(afs, filePath, desiredBom) + // Staged in review mode: skip format + file events (disk only). + if (!isActive()) { + if (yield* format.file(filePath)) { + contentNew = yield* Bom.syncFile(afs, filePath, desiredBom) + } + yield* events.publish(FileSystem.Event.Edited, { file: filePath }) + yield* events.publish(Watcher.Event.Updated, { + file: filePath, + event: "change", + }) } - yield* events.publish(FileSystem.Event.Edited, { file: filePath }) - yield* events.publish(Watcher.Event.Updated, { - file: filePath, - event: "change", - }) diff = trimDiff( createTwoFilesPatch( filePath, @@ -194,11 +202,19 @@ export const EditTool = Tool.define( }) let output = "Edit applied successfully." - yield* lsp.touchFile(filePath, "document") - const diagnostics = yield* lsp.diagnostics() - const normalizedFilePath = FSUtil.normalizePath(filePath) - const block = LSP.Diagnostic.report(filePath, diagnostics[normalizedFilePath] ?? []) - if (block) output += `\n\nLSP errors detected in this file, please fix:\n${block}` + // LSP needs the file on disk, so skip diagnostics while the edit is + // staged in review mode. + const diagnostics = isActive() + ? {} + : yield* Effect.gen(function* () { + yield* lsp.touchFile(filePath, "document") + return yield* lsp.diagnostics() + }) + if (!isActive()) { + const normalizedFilePath = FSUtil.normalizePath(filePath) + const block = LSP.Diagnostic.report(filePath, diagnostics[normalizedFilePath] ?? []) + if (block) output += `\n\nLSP errors detected in this file, please fix:\n${block}` + } return { metadata: { diff --git a/packages/opencode/src/tool/write.ts b/packages/opencode/src/tool/write.ts index 37be6d8c47bc..7211684f4d26 100644 --- a/packages/opencode/src/tool/write.ts +++ b/packages/opencode/src/tool/write.ts @@ -14,6 +14,7 @@ import { InstanceState } from "@/effect/instance-state" import { trimDiff } from "./edit" import { assertExternalDirectoryEffect } from "./external-directory" import * as Bom from "@/util/bom" +import { isActive } from "@/acp/review-mode" const MAX_PROJECT_DIAGNOSTICS_FILES = 5 @@ -62,31 +63,42 @@ export const WriteTool = Tool.define( }) yield* fs.writeWithDirs(filepath, Bom.join(contentNew, desiredBom)) - if (yield* format.file(filepath)) { - yield* Bom.syncFile(fs, filepath, desiredBom) + // In review mode the write above is staged in the overlay, not on disk. + // Skip disk-only follow-ups (format + file events) here. + if (!isActive()) { + if (yield* format.file(filepath)) { + yield* Bom.syncFile(fs, filepath, desiredBom) + } + yield* events.publish(FileSystem.Event.Edited, { file: filepath }) + yield* events.publish(Watcher.Event.Updated, { + file: filepath, + event: exists ? "change" : "add", + }) } - yield* events.publish(FileSystem.Event.Edited, { file: filepath }) - yield* events.publish(Watcher.Event.Updated, { - file: filepath, - event: exists ? "change" : "add", - }) let output = "Wrote file successfully." - yield* lsp.touchFile(filepath, "document") - const diagnostics = yield* lsp.diagnostics() - const normalizedFilepath = FSUtil.normalizePath(filepath) - let projectDiagnosticsCount = 0 - for (const [file, issues] of Object.entries(diagnostics)) { - const current = file === normalizedFilepath - if (!current && projectDiagnosticsCount >= MAX_PROJECT_DIAGNOSTICS_FILES) continue - const block = LSP.Diagnostic.report(current ? filepath : file, issues) - if (!block) continue - if (current) { - output += `\n\nLSP errors detected in this file, please fix:\n${block}` - continue + // LSP needs the file on disk, so skip diagnostics while staged. + const diagnostics = isActive() + ? {} + : yield* Effect.gen(function* () { + yield* lsp.touchFile(filepath, "document") + return yield* lsp.diagnostics() + }) + if (!isActive()) { + const normalizedFilepath = FSUtil.normalizePath(filepath) + let projectDiagnosticsCount = 0 + for (const [file, issues] of Object.entries(diagnostics)) { + const current = file === normalizedFilepath + if (!current && projectDiagnosticsCount >= MAX_PROJECT_DIAGNOSTICS_FILES) continue + const block = LSP.Diagnostic.report(current ? filepath : file, issues) + if (!block) continue + if (current) { + output += `\n\nLSP errors detected in this file, please fix:\n${block}` + continue + } + projectDiagnosticsCount++ + output += `\n\nLSP errors detected in other files:\n${block}` } - projectDiagnosticsCount++ - output += `\n\nLSP errors detected in other files:\n${block}` } return { diff --git a/packages/opencode/test/acp/event.test.ts b/packages/opencode/test/acp/event.test.ts index 8271b28265c2..e9032e1ae687 100644 --- a/packages/opencode/test/acp/event.test.ts +++ b/packages/opencode/test/acp/event.test.ts @@ -1,9 +1,11 @@ -import { describe, expect, it } from "bun:test" +import { afterEach, describe, expect, it } from "bun:test" import type { AgentSideConnection } from "@agentclientprotocol/sdk" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import type { Event, Message, OpencodeClient, Part, SessionMessageResponse, ToolPart } from "@opencode-ai/sdk/v2" import { Effect, ManagedRuntime } from "effect" import { ACPEvent } from "@/acp/event" +import { ReviewOverlay } from "@opencode-ai/core/review-overlay" +import { reset, setClientWriteTextFileSupported, syncEnabled } from "@/acp/review-mode" import * as ACPService from "@/acp/service" import { Directory } from "@/acp/directory" import { ACPSession } from "@/acp/session" @@ -748,4 +750,54 @@ describe("acp event routing", () => { ], ]) }) + + afterEach(() => { + delete process.env.OPENCODE_ACP_REVIEW + delete process.env.OPENCODE_CLIENT + reset() + }) + + it("flushes staged writes after completed edit tool", async () => { + process.env.OPENCODE_ACP_REVIEW = "1" + process.env.OPENCODE_CLIENT = "acp" + setClientWriteTextFileSupported(true) + syncEnabled() + + const writes: Array<{ sessionId: string; path: string; content: string }> = [] + const harness = createHarness() + const subscription = new ACPEvent.Subscription({ + sdk: harness.sdk, + connection: { + ...harness.connection, + writeTextFile: async (input) => { + writes.push(input) + return {} + }, + }, + session: harness.session, + }) + + ReviewOverlay.setActiveSession("ses_flush") + ReviewOverlay.stage("/workspace/file.ts", "staged body") + + await Effect.runPromise(harness.session.create({ id: "ses_flush", cwd: "/workspace" })) + await subscription.handle( + toolUpdated( + completedTool("ses_flush", "call_edit", "edited", [], { + tool: "edit", + input: { filePath: "/workspace/file.ts", oldString: "a", newString: "b" }, + }), + ), + ) + + expect(writes).toEqual([ + { + sessionId: "ses_flush", + path: expect.stringContaining("file.ts"), + content: "staged body", + }, + ]) + subscription.stop() + harness.events.close() + }) }) diff --git a/packages/opencode/test/acp/service-session.test.ts b/packages/opencode/test/acp/service-session.test.ts index 8dd25492c093..85bf918c3d3e 100644 --- a/packages/opencode/test/acp/service-session.test.ts +++ b/packages/opencode/test/acp/service-session.test.ts @@ -18,6 +18,8 @@ import * as ACPService from "@/acp/service" import * as ACPError from "@/acp/error" import { UsageService } from "@/acp/usage" import type { Provider } from "@/provider/provider" +import { ReviewOverlay } from "@opencode-ai/core/review-overlay" +import { forceEnableForAcp, reset as resetReviewMode, setClientWriteTextFileSupported } from "@/acp/review-mode" const providerID = ProviderV2.ID.make("test") const modelID = ModelV2.ID.make("test-model") @@ -1213,6 +1215,63 @@ describe("ACP service sessions", () => { expect(error.code).toBe(-32000) }) + + it("clears the review overlay when a prompt fails mid-turn", async () => { + process.env.OPENCODE_CLIENT = "acp" + forceEnableForAcp() + setClientWriteTextFileSupported(true) + try { + const failing = ACPService.make({ + sdk: { + config: { + providers: () => Promise.resolve({ data: { providers: [provider], default: { test: modelID } } }), + get: () => Promise.resolve({ data: {} }), + }, + app: { + agents: () => Promise.resolve({ data: [{ name: "build", mode: "primary", permission: [], options: {} }] }), + skills: () => Promise.resolve({ data: [] }), + }, + command: { + list: () => Promise.resolve({ data: [] }), + }, + session: { + create: () => Promise.resolve({ data: { id: "ses_review_fail" } }), + list: () => Promise.resolve({ data: [] }), + prompt: () => Promise.reject(new Error("provider exploded")), + }, + mcp: { + add: () => Promise.resolve({ data: {} }), + }, + } as unknown as OpencodeClient, + connection: { + sessionUpdate: () => Promise.resolve(), + writeTextFile: () => Promise.resolve({}), + } as unknown as Pick, + usage: UsageService.Service.of({ + buildUsage: UsageService.buildUsage, + latestAssistantMessage: UsageService.latestAssistantMessage, + totalSessionCost: UsageService.totalSessionCost, + contextLimit: () => Effect.succeed(128000), + sendUpdate: () => Effect.void, + }), + }) + + const session = await Effect.runPromise(failing.newSession({ cwd: "/workspace", mcpServers: [] })) + ReviewOverlay.setActiveSession(session.sessionId) + ReviewOverlay.stage("/workspace/leftover.ts", "staged but turn fails") + expect(ReviewOverlay.get("/workspace/leftover.ts")).toBeDefined() + + const exit = await Effect.runPromiseExit( + failing.prompt({ sessionId: session.sessionId, prompt: [{ type: "text", text: "hello" }] }), + ) + expect(exit._tag).toBe("Failure") + + expect(ReviewOverlay.get("/workspace/leftover.ts")).toBeUndefined() + } finally { + delete process.env.OPENCODE_CLIENT + resetReviewMode() + } + }) }) function assistantInfo( diff --git a/packages/opencode/test/acp/tool.test.ts b/packages/opencode/test/acp/tool.test.ts index a243e10cc783..5cb76f0f090c 100644 --- a/packages/opencode/test/acp/tool.test.ts +++ b/packages/opencode/test/acp/tool.test.ts @@ -4,9 +4,11 @@ import { completedToolContent, completedToolUpdate, completedToolRawOutput, + deleteDiffContent, extractImageAttachments, imageContents, pendingToolCall, + moveDiffContent, shellOutputSnapshot, toLocations, toToolKind, @@ -295,4 +297,66 @@ describe("acp tool conversion", () => { expect(shellOutputSnapshot({ metadata: { output: 42 } })).toBeUndefined() expect(shellOutputSnapshot({ metadata: undefined })).toBeUndefined() }) + + test("builds Codex-style delete and move diff helpers", () => { + expect(deleteDiffContent("/tmp/remove.ts", "gone")).toEqual([ + { type: "diff", path: "/tmp/remove.ts", oldText: "gone", newText: "" }, + ]) + expect(moveDiffContent("/tmp/dest.ts", "old body", "new body")).toEqual([ + { type: "diff", path: "/tmp/dest.ts", oldText: "old body", newText: "new body" }, + ]) + }) + + test("builds apply_patch completed content with delete and move diffs from metadata", () => { + expect( + completedToolContent("apply_patch", { + status: "completed", + input: { patchText: "*** Begin Patch\n*** End Patch" }, + output: "Success.", + metadata: { + files: [ + { + type: "delete", + filePath: "/tmp/remove.ts", + diffPath: "/tmp/remove.ts", + oldText: "remove me", + newText: "", + }, + { + type: "move", + filePath: "/tmp/src.ts", + movePath: "/tmp/dest.ts", + diffPath: "/tmp/dest.ts", + oldText: "before move", + newText: "after move", + }, + { + type: "update", + filePath: "/tmp/keep.ts", + diffPath: "/tmp/keep.ts", + oldText: "old", + newText: "new", + }, + ], + }, + }), + ).toEqual([ + { + type: "content", + content: { type: "text", text: "Success." }, + }, + { + type: "diff", + path: "/tmp/remove.ts", + oldText: "remove me", + newText: "", + }, + { + type: "diff", + path: "/tmp/dest.ts", + oldText: "before move", + newText: "after move", + }, + ]) + }) }) diff --git a/packages/opencode/test/tool/review-mode-tools.test.ts b/packages/opencode/test/tool/review-mode-tools.test.ts new file mode 100644 index 000000000000..cf87bcb5dcdf --- /dev/null +++ b/packages/opencode/test/tool/review-mode-tools.test.ts @@ -0,0 +1,187 @@ +import { afterEach, describe, expect } from "bun:test" +import path from "path" +import fs from "fs/promises" +import { Effect, Layer } from "effect" +import { EditTool } from "../../src/tool/edit" +import { WriteTool } from "../../src/tool/write" +import { ApplyPatchTool } from "../../src/tool/apply_patch" +import { disposeAllInstances, TestInstance } from "../fixture/fixture" +import { LSP } from "@/lsp/lsp" +import { Format } from "../../src/format" +import { Agent } from "../../src/agent/agent" +import { EventV2Bridge } from "../../src/event-v2-bridge" +import { Truncate } from "@/tool/truncate" +import { SessionID, MessageID } from "../../src/session/schema" +import { testEffect } from "../lib/effect" +import { ReviewFs } from "@/effect/review-fs-layer" +import { ReviewOverlay } from "@opencode-ai/core/review-overlay" +import { forceEnableForAcp, reset, setClientWriteTextFileSupported } from "@/acp/review-mode" +import { completedToolContent } from "@/acp/tool" +import { ToolRegistry } from "@/tool/registry" +import { ProviderV2 } from "@opencode-ai/core/provider" +import { ModelV2 } from "@opencode-ai/core/model" + +const baseCtx = { + sessionID: SessionID.make("ses_review"), + messageID: MessageID.make("msg_review"), + callID: "", + agent: "build", + abort: AbortSignal.any([]), + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, +} + +const layer = Layer.mergeAll( + LSP.defaultLayer, + ReviewFs.defaultLayer, + Format.defaultLayer, + EventV2Bridge.defaultLayer, + Truncate.defaultLayer, + Agent.defaultLayer, +) + +const it = testEffect(layer) + +afterEach(async () => { + delete process.env.OPENCODE_ACP_REVIEW + delete process.env.OPENCODE_CLIENT + reset() + await disposeAllInstances() +}) + +const enableReview = () => { + process.env.OPENCODE_CLIENT = "acp" + forceEnableForAcp() + setClientWriteTextFileSupported(true) + ReviewOverlay.setActiveSession("ses_review") +} + +const readDisk = (filepath: string) => Effect.promise(() => fs.readFile(filepath, "utf-8").catch(() => "")) + +describe("review mode tools", () => { + it.instance("edit stages without disk write", () => + Effect.gen(function* () { + enableReview() + const test = yield* TestInstance + const edit = yield* EditTool + const tool = yield* edit.init() + + const filepath = path.join(test.directory, "edit.txt") + yield* Effect.promise(() => fs.writeFile(filepath, "before\n", "utf-8")) + + yield* tool.execute( + { + filePath: filepath, + oldString: "before", + newString: "after", + }, + baseCtx, + ) + + expect(yield* readDisk(filepath)).toBe("before\n") + const staged = ReviewOverlay.get(filepath) + expect(staged && "content" in staged ? staged.content : undefined).toBe("after\n") + }), + ) + + it.instance("write stages new files without disk write", () => + Effect.gen(function* () { + enableReview() + const test = yield* TestInstance + const write = yield* WriteTool + const tool = yield* write.init() + + const filepath = path.join(test.directory, "new.txt") + yield* tool.execute({ filePath: filepath, content: "created" }, baseCtx) + + expect(yield* readDisk(filepath)).toBe("") + expect(ReviewOverlay.get(filepath)).toEqual({ content: "created" }) + }), + ) + + it.instance("apply_patch stages add/update and deletes on disk", () => + Effect.gen(function* () { + enableReview() + const test = yield* TestInstance + const patch = yield* ApplyPatchTool + const tool = yield* patch.init() + + const updatePath = path.join(test.directory, "update.txt") + const deletePath = path.join(test.directory, "delete.txt") + yield* Effect.promise(() => fs.writeFile(updatePath, "old\n", "utf-8")) + yield* Effect.promise(() => fs.writeFile(deletePath, "gone\n", "utf-8")) + + const patchText = [ + "*** Begin Patch", + "*** Add File: added.txt", + "+added", + "*** Update File: update.txt", + "@@", + "-old", + "+new", + "*** Delete File: delete.txt", + "*** End Patch", + ].join("\n") + + const result = yield* tool.execute({ patchText }, baseCtx) + + expect(yield* readDisk(path.join(test.directory, "added.txt"))).toBe("") + expect(yield* readDisk(updatePath)).toBe("old\n") + expect(yield* readDisk(deletePath)).toBe("") + + expect(ReviewOverlay.get(path.join(test.directory, "added.txt"))).toEqual({ content: "added\n" }) + expect(ReviewOverlay.get(updatePath)).toEqual({ content: "new\n" }) + + // delete/move bypass staging (ACP has no deleteFile), so the output must + // warn that those changes already hit disk and won't be undone on reject. + expect(result.output).toContain("applied directly to disk") + + const content = completedToolContent("apply_patch", { + status: "completed", + input: { patchText }, + output: result.output, + metadata: result.metadata, + }) + expect(content.some((item) => item.type === "diff" && item.path === deletePath && item.newText === "")).toBe( + true, + ) + }), + ) +}) + +// Regression: tools resolved through the production ToolRegistry wiring must +// stage instead of writing to disk. The direct-tool tests above can pass even +// when ToolRegistry injects the plain FSUtil layer, so this exercises the real +// path that ACP prompts go through. +const registryIt = testEffect(Layer.mergeAll(ToolRegistry.defaultLayer, Agent.defaultLayer)) + +describe("review mode via ToolRegistry", () => { + registryIt.instance("edit staged through registry does not touch disk", () => + Effect.gen(function* () { + enableReview() + const test = yield* TestInstance + const agent = yield* Agent.Service + const build = yield* agent.get("build") + if (!build) throw new Error("build agent not found") + + const registry = yield* ToolRegistry.Service + const tools = yield* registry.tools({ + providerID: ProviderV2.ID.opencode, + modelID: ModelV2.ID.make("test"), + agent: build, + }) + const edit = tools.find((tool) => tool.id === "edit") + if (!edit) throw new Error("edit tool not registered") + + const filepath = path.join(test.directory, "registry-edit.txt") + yield* Effect.promise(() => fs.writeFile(filepath, "before\n", "utf-8")) + + yield* edit.execute({ filePath: filepath, oldString: "before", newString: "after" }, baseCtx) + + expect(yield* readDisk(filepath)).toBe("before\n") + const staged = ReviewOverlay.get(filepath) + expect(staged && "content" in staged ? staged.content : undefined).toBe("after\n") + }), + ) +}) From 6f3687d28cb1f353bc16bb1035283bb456db1163 Mon Sep 17 00:00:00 2001 From: Paco Preciado Mendoza Date: Mon, 8 Jun 2026 14:44:09 -0600 Subject: [PATCH 4/9] fix(acp): replace removed log module with Effect logging Upstream dropped @opencode-ai/core/util/log; migrate ACP review paths to Effect.logInfo/logError so typecheck passes after the rebase. --- packages/opencode/src/acp/permission.ts | 10 ++++++++- packages/opencode/src/acp/review-staging.ts | 23 +++++++++------------ packages/opencode/src/acp/service.ts | 2 +- packages/opencode/src/cli/cmd/acp.ts | 2 +- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/packages/opencode/src/acp/permission.ts b/packages/opencode/src/acp/permission.ts index de8050b6c313..efc6e8524077 100644 --- a/packages/opencode/src/acp/permission.ts +++ b/packages/opencode/src/acp/permission.ts @@ -85,7 +85,15 @@ export class Handler { // In review mode the overlay already sends edits to the client, so skip the // proposed-edit message to avoid showing the same change twice. if (permission.permission === "edit" && !isActive()) { - await this.writeProposedEdit(session.id, permission.metadata).catch(() => {}) + await this.writeProposedEdit(session.id, permission.metadata).catch((error: unknown) => + Effect.runPromise( + Effect.logError("failed to write proposed edit through ACP", { + error, + permissionID: permission.id, + sessionID: permission.sessionID, + }), + ), + ) } await this.reply(permission.id, reply, session.cwd) diff --git a/packages/opencode/src/acp/review-staging.ts b/packages/opencode/src/acp/review-staging.ts index 5783a549e48f..86c1d054874d 100644 --- a/packages/opencode/src/acp/review-staging.ts +++ b/packages/opencode/src/acp/review-staging.ts @@ -1,12 +1,10 @@ export * as ACPReviewStaging from "./review-staging" import type { AgentSideConnection } from "@agentclientprotocol/sdk" -import * as Log from "@opencode-ai/core/util/log" +import { Effect } from "effect" import { ReviewOverlay } from "@opencode-ai/core/review-overlay" import { isActive } from "./review-mode" -const log = Log.create({ service: "acp-review-staging" }) - type Connection = Partial> export async function flushPendingWrites(connection: Connection | undefined, sessionID?: string) { @@ -18,9 +16,6 @@ export async function flushPendingWrites(connection: Connection | undefined, ses } const pending = ReviewOverlay.drainPendingWrites() - if (pending.length > 0) { - log.info("flushing staged writes", { count: pending.length }) - } for (const item of pending) { await connection @@ -29,12 +24,14 @@ export async function flushPendingWrites(connection: Connection | undefined, ses path: item.path, content: item.content, }) - .catch((error: unknown) => { - log.error("failed to write staged edit through ACP", { - error, - path: item.path, - sessionID: item.sessionID, - }) - }) + .catch((error: unknown) => + Effect.runPromise( + Effect.logError("failed to write staged edit through ACP", { + error, + path: item.path, + sessionID: item.sessionID, + }), + ), + ) } } diff --git a/packages/opencode/src/acp/service.ts b/packages/opencode/src/acp/service.ts index c9d32dc1bc7c..09087ca0b59d 100644 --- a/packages/opencode/src/acp/service.ts +++ b/packages/opencode/src/acp/service.ts @@ -115,7 +115,7 @@ export function make(input: { setClientWriteTextFileSupported(params.clientCapabilities?.fs?.writeTextFile === true) syncEnabled() if (ReviewOverlay.isEnabled()) { - log.info("ACP review-at-end active") + yield* Effect.logInfo("ACP review-at-end active") } const response = { diff --git a/packages/opencode/src/cli/cmd/acp.ts b/packages/opencode/src/cli/cmd/acp.ts index b5c067ba0783..93a02a91f39b 100644 --- a/packages/opencode/src/cli/cmd/acp.ts +++ b/packages/opencode/src/cli/cmd/acp.ts @@ -25,7 +25,7 @@ export const AcpCommand = effectCmd({ // Opt this ACP session into review mode. It still only activates if the // client reports the writeTextFile capability during initialize. forceEnableForAcp() - log.info("ACP review-at-end enabled") + yield* Effect.logInfo("ACP review-at-end enabled") const opts = yield* resolveNetworkOptions(args) const server = yield* Effect.promise(() => ACPProfile.measure("cli.acp.server.listen", () => Server.listen(opts))) From cd7c2727e916c0d79027b8e41e81269ade10a5ba Mon Sep 17 00:00:00 2001 From: Paco Preciado Mendoza Date: Tue, 9 Jun 2026 20:31:03 -0600 Subject: [PATCH 5/9] test(opencode): fix review-mode ToolRegistry test after rebase ToolRegistry.defaultLayer already provides Agent; align the registry test layer with skill.test and use an inline build agent to satisfy Effect types. --- packages/opencode/test/tool/review-mode-tools.test.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/opencode/test/tool/review-mode-tools.test.ts b/packages/opencode/test/tool/review-mode-tools.test.ts index cf87bcb5dcdf..5e905664d221 100644 --- a/packages/opencode/test/tool/review-mode-tools.test.ts +++ b/packages/opencode/test/tool/review-mode-tools.test.ts @@ -13,6 +13,8 @@ import { EventV2Bridge } from "../../src/event-v2-bridge" import { Truncate } from "@/tool/truncate" import { SessionID, MessageID } from "../../src/session/schema" import { testEffect } from "../lib/effect" +import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { Ripgrep } from "@opencode-ai/core/ripgrep" import { ReviewFs } from "@/effect/review-fs-layer" import { ReviewOverlay } from "@opencode-ai/core/review-overlay" import { forceEnableForAcp, reset, setClientWriteTextFileSupported } from "@/acp/review-mode" @@ -154,22 +156,21 @@ describe("review mode tools", () => { // stage instead of writing to disk. The direct-tool tests above can pass even // when ToolRegistry injects the plain FSUtil layer, so this exercises the real // path that ACP prompts go through. -const registryIt = testEffect(Layer.mergeAll(ToolRegistry.defaultLayer, Agent.defaultLayer)) +const registryIt = testEffect( + Layer.mergeAll(ToolRegistry.defaultLayer, CrossSpawnSpawner.defaultLayer).pipe(Layer.provide(Ripgrep.defaultLayer)), +) describe("review mode via ToolRegistry", () => { registryIt.instance("edit staged through registry does not touch disk", () => Effect.gen(function* () { enableReview() const test = yield* TestInstance - const agent = yield* Agent.Service - const build = yield* agent.get("build") - if (!build) throw new Error("build agent not found") const registry = yield* ToolRegistry.Service const tools = yield* registry.tools({ providerID: ProviderV2.ID.opencode, modelID: ModelV2.ID.make("test"), - agent: build, + agent: { name: "build", mode: "primary", permission: [], options: {} }, }) const edit = tools.find((tool) => tool.id === "edit") if (!edit) throw new Error("edit tool not registered") From 685697dab732b6898b496793d4f32143e4f6aef9 Mon Sep 17 00:00:00 2001 From: Paco Preciado Mendoza Date: Fri, 12 Jun 2026 13:18:26 -0600 Subject: [PATCH 6/9] fix(opencode): restore ReviewFs wiring for ACP review after LayerNode migration Re-add ReviewFs to ToolRegistry.node and createRoutes so tool writes stage in memory again. Remove misplaced ReviewFs from AppRuntime, which ACP does not use. --- packages/opencode/src/effect/review-fs-layer.ts | 3 +++ .../opencode/src/server/routes/instance/httpapi/server.ts | 7 +++++-- packages/opencode/src/tool/registry.ts | 3 ++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/opencode/src/effect/review-fs-layer.ts b/packages/opencode/src/effect/review-fs-layer.ts index 2c3f14fe7cc6..385dcaadcc43 100644 --- a/packages/opencode/src/effect/review-fs-layer.ts +++ b/packages/opencode/src/effect/review-fs-layer.ts @@ -1,5 +1,6 @@ import { Effect, FileSystem, Layer, Option } from "effect" import * as PlatformError from "effect/PlatformError" +import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { FSUtil } from "@opencode-ai/core/fs-util" import { ReviewOverlay } from "@opencode-ai/core/review-overlay" @@ -137,4 +138,6 @@ export const layer = Layer.effect( export const defaultLayer = layer +export const node = LayerNode.make(layer, [FSUtil.node]) + export * as ReviewFs from "./review-fs-layer" diff --git a/packages/opencode/src/server/routes/instance/httpapi/server.ts b/packages/opencode/src/server/routes/instance/httpapi/server.ts index fb9d2db65621..292a1f2543e6 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/server.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/server.ts @@ -110,6 +110,7 @@ import { instanceContextLayer } from "./middleware/instance-context" import { workspaceRoutingLayer } from "./middleware/workspace-routing" import { disposeMiddleware } from "./lifecycle" import { memoMap } from "@opencode-ai/core/effect/memo-map" +import { ReviewFs } from "@/effect/review-fs-layer" import { compressionLayer } from "./middleware/compression" import { corsVaryFix } from "./middleware/cors-vary" import { errorLayer } from "./middleware/error" @@ -209,6 +210,8 @@ type RouteRequirements = | HttpRouter.Request<"Requires", unknown> | HttpRouter.Request<"GlobalRequires", never> +const reviewFsReplacements = [[FSUtil.node, ReviewFs.node]] as const + const app = LayerNode.group([ Npm.node, FSUtil.node, @@ -271,7 +274,7 @@ const app = LayerNode.group([ export function createRoutes( corsOptions?: CorsOptions, ): Layer.Layer { - const locationServiceMapV2 = buildLocationServiceMap() + const locationServiceMapV2 = buildLocationServiceMap(reviewFsReplacements) return Layer.mergeAll( rootApiRoutes, @@ -303,7 +306,7 @@ export function createRoutes( ), Layer.provide(locationServiceMapV2), - Layer.provide(AppNodeBuilderV1.build(app)), + Layer.provide(AppNodeBuilderV1.build(app, reviewFsReplacements)), // Must stay last: layers provided later in this pipe build beneath earlier ones, // so Observability must come after every service graph. Otherwise eagerly forked // fibers (e.g. the ModelsDev background refresh) capture Effect's default stdout diff --git a/packages/opencode/src/tool/registry.ts b/packages/opencode/src/tool/registry.ts index 15acc757f3d4..75f965a1dadd 100644 --- a/packages/opencode/src/tool/registry.ts +++ b/packages/opencode/src/tool/registry.ts @@ -43,6 +43,7 @@ import { Todo } from "../session/todo" import { LSP } from "@/lsp/lsp" import { Instruction } from "../session/instruction" import { FSUtil } from "@opencode-ai/core/fs-util" +import { ReviewFs } from "@/effect/review-fs-layer" import { EventV2Bridge } from "@/event-v2-bridge" import { Agent } from "../agent/agent" import { Skill } from "../skill" @@ -421,7 +422,7 @@ function isJsonSchemaObject(value: unknown): value is Record { export const node = LayerNode.make({ service: Service, - layer, + layer: layer.pipe(Layer.provide(ReviewFs.defaultLayer)), deps: [ Config.node, Plugin.node, From 5261f417ef6ec697fc312febdcbeb31494c2b4db Mon Sep 17 00:00:00 2001 From: Paco Preciado Mendoza Date: Fri, 12 Jun 2026 13:18:30 -0600 Subject: [PATCH 7/9] test(opencode): add HTTP app ReviewFs regression test Verify ToolRegistry.node with ReviewFs stages edits through the same layer wiring used by the HTTP server after the LayerNode migration. --- .../test/effect/httpapi-review-fs.test.ts | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 packages/opencode/test/effect/httpapi-review-fs.test.ts diff --git a/packages/opencode/test/effect/httpapi-review-fs.test.ts b/packages/opencode/test/effect/httpapi-review-fs.test.ts new file mode 100644 index 000000000000..19fddaa28997 --- /dev/null +++ b/packages/opencode/test/effect/httpapi-review-fs.test.ts @@ -0,0 +1,78 @@ +import { afterEach, describe, expect } from "bun:test" +import path from "path" +import fs from "fs/promises" +import { Effect, Layer } from "effect" +import { LayerNode } from "@opencode-ai/core/effect/layer-node" +import { FSUtil } from "@opencode-ai/core/fs-util" +import { ReviewFs } from "@/effect/review-fs-layer" +import { ReviewOverlay } from "@opencode-ai/core/review-overlay" +import { ToolRegistry } from "@/tool/registry" +import { disposeAllInstances, TestInstance } from "../fixture/fixture" +import { testEffect } from "../lib/effect" +import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { Ripgrep } from "@opencode-ai/core/ripgrep" +import { forceEnableForAcp, reset, setClientWriteTextFileSupported } from "@/acp/review-mode" +import { SessionID, MessageID } from "@/session/schema" +import { ProviderV2 } from "@opencode-ai/core/provider" +import { ModelV2 } from "@opencode-ai/core/model" + +// Mirrors HttpApiApp.createRoutes: buildLayer(app) + ReviewFs.defaultLayer on top. +const httpAppLayer = LayerNode.buildLayer(LayerNode.group([FSUtil.node, ToolRegistry.node])).pipe( + Layer.provide(ReviewFs.defaultLayer), + Layer.provide(Ripgrep.defaultLayer), + Layer.provide(CrossSpawnSpawner.defaultLayer), +) + +const it = testEffect(httpAppLayer) + +const baseCtx = { + sessionID: SessionID.make("ses_http"), + messageID: MessageID.make("msg_http"), + callID: "", + agent: "build", + abort: AbortSignal.any([]), + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, +} + +const enableReview = () => { + process.env.OPENCODE_CLIENT = "acp" + forceEnableForAcp() + setClientWriteTextFileSupported(true) + ReviewOverlay.setActiveSession("ses_http") +} + +afterEach(async () => { + delete process.env.OPENCODE_CLIENT + reset() + await disposeAllInstances() +}) + +describe("HTTP app ReviewFs wiring", () => { + it.instance("ToolRegistry.node with ReviewFs stages edits through the registry", () => + Effect.gen(function* () { + enableReview() + const test = yield* TestInstance + + const registry = yield* ToolRegistry.Service + const tools = yield* registry.tools({ + providerID: ProviderV2.ID.opencode, + modelID: ModelV2.ID.make("test"), + agent: { name: "build", mode: "primary", permission: [], options: {} }, + }) + const edit = tools.find((tool) => tool.id === "edit") + if (!edit) throw new Error("edit tool not registered") + + const filepath = path.join(test.directory, "http-edit.txt") + yield* Effect.promise(() => fs.writeFile(filepath, "before\n", "utf-8")) + + yield* edit.execute({ filePath: filepath, oldString: "before", newString: "after" }, baseCtx) + + const disk = yield* Effect.promise(() => fs.readFile(filepath, "utf-8").catch(() => "")) + const staged = ReviewOverlay.get(filepath) + expect(disk).toBe("before\n") + expect(staged && "content" in staged ? staged.content : undefined).toBe("after\n") + }), + ) +}) From 0b22e5724aebbfa51bd70be1140455641acba70b Mon Sep 17 00:00:00 2001 From: Paco Preciado Mendoza Date: Sat, 27 Jun 2026 14:01:17 -0600 Subject: [PATCH 8/9] fix(opencode): update ReviewFs node for LayerNode.compile API Adapt ReviewFs.node and the HTTP review regression test to the new LayerNode.make object form and compile() after upstream rebase. --- packages/opencode/src/effect/review-fs-layer.ts | 6 +++++- packages/opencode/test/effect/httpapi-review-fs.test.ts | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/opencode/src/effect/review-fs-layer.ts b/packages/opencode/src/effect/review-fs-layer.ts index 385dcaadcc43..0ecead81977c 100644 --- a/packages/opencode/src/effect/review-fs-layer.ts +++ b/packages/opencode/src/effect/review-fs-layer.ts @@ -138,6 +138,10 @@ export const layer = Layer.effect( export const defaultLayer = layer -export const node = LayerNode.make(layer, [FSUtil.node]) +export const node = LayerNode.make({ + name: "ReviewFs", + layer, + deps: [FSUtil.node], +}) export * as ReviewFs from "./review-fs-layer" diff --git a/packages/opencode/test/effect/httpapi-review-fs.test.ts b/packages/opencode/test/effect/httpapi-review-fs.test.ts index 19fddaa28997..a4ac8fa16155 100644 --- a/packages/opencode/test/effect/httpapi-review-fs.test.ts +++ b/packages/opencode/test/effect/httpapi-review-fs.test.ts @@ -16,8 +16,8 @@ import { SessionID, MessageID } from "@/session/schema" import { ProviderV2 } from "@opencode-ai/core/provider" import { ModelV2 } from "@opencode-ai/core/model" -// Mirrors HttpApiApp.createRoutes: buildLayer(app) + ReviewFs.defaultLayer on top. -const httpAppLayer = LayerNode.buildLayer(LayerNode.group([FSUtil.node, ToolRegistry.node])).pipe( +// Mirrors HttpApiApp.createRoutes: compile(app) + ReviewFs.defaultLayer on top. +const httpAppLayer = LayerNode.compile(LayerNode.group([FSUtil.node, ToolRegistry.node])).pipe( Layer.provide(ReviewFs.defaultLayer), Layer.provide(Ripgrep.defaultLayer), Layer.provide(CrossSpawnSpawner.defaultLayer), From a8d3d1ae69345fc46d1cc7a8826883abac56ff73 Mon Sep 17 00:00:00 2001 From: Paco Preciado Mendoza Date: Wed, 1 Jul 2026 10:37:43 -0600 Subject: [PATCH 9/9] fix(opencode): adapt ReviewFs wiring after dev layer-node rebase Replace removed FSUtil.defaultLayer usage with compile-based ReviewFs nodes and global FSUtil replacement in the HTTP app graph so ACP review staging keeps working on the current dev stack. --- .../opencode/src/effect/review-fs-layer.ts | 20 +++++++++++-------- packages/opencode/src/tool/registry.ts | 3 +-- .../test/effect/httpapi-review-fs.test.ts | 10 +++++----- .../test/tool/review-mode-tools.test.ts | 15 +++++++------- 4 files changed, 25 insertions(+), 23 deletions(-) diff --git a/packages/opencode/src/effect/review-fs-layer.ts b/packages/opencode/src/effect/review-fs-layer.ts index 0ecead81977c..41cd94233595 100644 --- a/packages/opencode/src/effect/review-fs-layer.ts +++ b/packages/opencode/src/effect/review-fs-layer.ts @@ -1,6 +1,8 @@ import { Effect, FileSystem, Layer, Option } from "effect" import * as PlatformError from "effect/PlatformError" import { LayerNode } from "@opencode-ai/core/effect/layer-node" +import { makeGlobalNode } from "@opencode-ai/core/effect/app-node" +import { filesystem } from "@opencode-ai/core/effect/app-node-platform" import { FSUtil } from "@opencode-ai/core/fs-util" import { ReviewOverlay } from "@opencode-ai/core/review-overlay" @@ -38,7 +40,7 @@ function overlayFileStat(content: string): FileSystem.File.Info { } } -export const layer = Layer.effect( +const reviewLayer = Layer.effect( FSUtil.Service, Effect.gen(function* () { const fs = yield* FSUtil.Service @@ -134,14 +136,16 @@ export const layer = Layer.effect( writeWithDirs, }) }), -).pipe(Layer.provide(FSUtil.defaultLayer)) +) -export const defaultLayer = layer - -export const node = LayerNode.make({ - name: "ReviewFs", - layer, - deps: [FSUtil.node], +export const node = makeGlobalNode({ + service: FSUtil.Service, + layer: reviewLayer.pipe(Layer.provide(LayerNode.compile(FSUtil.node))), + deps: [filesystem], }) +export const defaultLayer = LayerNode.compile(node) + +export const layer = defaultLayer + export * as ReviewFs from "./review-fs-layer" diff --git a/packages/opencode/src/tool/registry.ts b/packages/opencode/src/tool/registry.ts index 75f965a1dadd..15acc757f3d4 100644 --- a/packages/opencode/src/tool/registry.ts +++ b/packages/opencode/src/tool/registry.ts @@ -43,7 +43,6 @@ import { Todo } from "../session/todo" import { LSP } from "@/lsp/lsp" import { Instruction } from "../session/instruction" import { FSUtil } from "@opencode-ai/core/fs-util" -import { ReviewFs } from "@/effect/review-fs-layer" import { EventV2Bridge } from "@/event-v2-bridge" import { Agent } from "../agent/agent" import { Skill } from "../skill" @@ -422,7 +421,7 @@ function isJsonSchemaObject(value: unknown): value is Record { export const node = LayerNode.make({ service: Service, - layer: layer.pipe(Layer.provide(ReviewFs.defaultLayer)), + layer, deps: [ Config.node, Plugin.node, diff --git a/packages/opencode/test/effect/httpapi-review-fs.test.ts b/packages/opencode/test/effect/httpapi-review-fs.test.ts index a4ac8fa16155..11f993b8a688 100644 --- a/packages/opencode/test/effect/httpapi-review-fs.test.ts +++ b/packages/opencode/test/effect/httpapi-review-fs.test.ts @@ -11,16 +11,16 @@ import { disposeAllInstances, TestInstance } from "../fixture/fixture" import { testEffect } from "../lib/effect" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { Ripgrep } from "@opencode-ai/core/ripgrep" +import { AppNodeBuilderV1 } from "@/effect/app-node-builder-v1" import { forceEnableForAcp, reset, setClientWriteTextFileSupported } from "@/acp/review-mode" import { SessionID, MessageID } from "@/session/schema" import { ProviderV2 } from "@opencode-ai/core/provider" import { ModelV2 } from "@opencode-ai/core/model" -// Mirrors HttpApiApp.createRoutes: compile(app) + ReviewFs.defaultLayer on top. -const httpAppLayer = LayerNode.compile(LayerNode.group([FSUtil.node, ToolRegistry.node])).pipe( - Layer.provide(ReviewFs.defaultLayer), - Layer.provide(Ripgrep.defaultLayer), - Layer.provide(CrossSpawnSpawner.defaultLayer), +// Mirrors HttpApiApp.createRoutes: AppNodeBuilderV1.build(app) + ReviewFs.defaultLayer on top. +const httpAppLayer = AppNodeBuilderV1.build( + LayerNode.group([FSUtil.node, ToolRegistry.node, CrossSpawnSpawner.node, Ripgrep.node]), + [[FSUtil.node, ReviewFs.node]], ) const it = testEffect(httpAppLayer) diff --git a/packages/opencode/test/tool/review-mode-tools.test.ts b/packages/opencode/test/tool/review-mode-tools.test.ts index 5e905664d221..e637b8969e66 100644 --- a/packages/opencode/test/tool/review-mode-tools.test.ts +++ b/packages/opencode/test/tool/review-mode-tools.test.ts @@ -2,11 +2,13 @@ import { afterEach, describe, expect } from "bun:test" import path from "path" import fs from "fs/promises" import { Effect, Layer } from "effect" +import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { EditTool } from "../../src/tool/edit" import { WriteTool } from "../../src/tool/write" import { ApplyPatchTool } from "../../src/tool/apply_patch" import { disposeAllInstances, TestInstance } from "../fixture/fixture" import { LSP } from "@/lsp/lsp" +import { FSUtil } from "@opencode-ai/core/fs-util" import { Format } from "../../src/format" import { Agent } from "../../src/agent/agent" import { EventV2Bridge } from "../../src/event-v2-bridge" @@ -34,13 +36,8 @@ const baseCtx = { ask: () => Effect.void, } -const layer = Layer.mergeAll( - LSP.defaultLayer, - ReviewFs.defaultLayer, - Format.defaultLayer, - EventV2Bridge.defaultLayer, - Truncate.defaultLayer, - Agent.defaultLayer, +const layer = LayerNode.compile( + LayerNode.group([LSP.node, ReviewFs.node, Format.node, EventV2Bridge.node, Truncate.node, Agent.node]), ) const it = testEffect(layer) @@ -157,7 +154,9 @@ describe("review mode tools", () => { // when ToolRegistry injects the plain FSUtil layer, so this exercises the real // path that ACP prompts go through. const registryIt = testEffect( - Layer.mergeAll(ToolRegistry.defaultLayer, CrossSpawnSpawner.defaultLayer).pipe(Layer.provide(Ripgrep.defaultLayer)), + LayerNode.compile(LayerNode.group([ToolRegistry.node, CrossSpawnSpawner.node, Ripgrep.node]), [ + [FSUtil.node, ReviewFs.node], + ]), ) describe("review mode via ToolRegistry", () => {