From d6ab61815500fbff89cc1adfd10eb8caad00fa00 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Thu, 6 Aug 2026 11:45:50 -0400 Subject: [PATCH] refactor(core): make tools hosting-blind --- packages/core/src/file-mutation.ts | 123 +++++++++-- packages/core/src/location-mutation.ts | 8 + packages/core/src/location-services.ts | 1 - packages/core/src/plugin/internal.ts | 10 +- packages/core/src/plugin/supervisor.ts | 8 - packages/core/src/session.ts | 2 +- packages/core/src/shell.ts | 50 ++++- packages/core/src/tool/plugin/edit.ts | 31 +-- packages/core/src/tool/plugin/patch.ts | 225 ++++++--------------- packages/core/src/tool/plugin/shell.ts | 23 +-- packages/core/src/tool/plugin/write.ts | 28 ++- packages/core/test/tool-patch.test.ts | 10 +- packages/core/test/tool-read.test.ts | 1 + packages/core/test/workspace-patch.test.ts | 44 ++-- packages/server/src/handlers/session.ts | 3 + packages/server/src/handlers/shell.ts | 8 +- 16 files changed, 284 insertions(+), 291 deletions(-) diff --git a/packages/core/src/file-mutation.ts b/packages/core/src/file-mutation.ts index 0fd4f779fc4..3f883fb4c93 100644 --- a/packages/core/src/file-mutation.ts +++ b/packages/core/src/file-mutation.ts @@ -1,17 +1,30 @@ export * as FileMutation from "./file-mutation" import { makeLocationNode } from "@opencode-ai/util/effect/app-node" -import { Context, Effect, Layer } from "effect" +import { Context, Effect, Layer, Schema } from "effect" import { KeyedMutex } from "./effect/keyed-mutex" import { FSUtil } from "@opencode-ai/util/fs-util" import { Bom } from "@opencode-ai/util/bom" +import { Formatter } from "./formatter" import { WorkspaceEnvironment } from "./workspace/environment" export interface Target { readonly canonical: string + /** Lexical path for entry operations; remove unlinks the name, not the referent. */ + readonly absolute?: string readonly resource: string } +/** Seam-owned absence so tools never see backend error vocabularies. */ +export class NotFoundError extends Schema.TaggedErrorClass()("FileMutation.NotFoundError", { + path: Schema.String, +}) {} + +/** The target resolved to a directory where a file operation was required. */ +export class NotAFileError extends Schema.TaggedErrorClass()("FileMutation.NotAFileError", { + path: Schema.String, +}) {} + export interface WriteInput { readonly target: Target readonly content: string | Uint8Array @@ -29,14 +42,29 @@ export interface WriteResult { readonly existed: boolean } +export interface TextWriteResult extends WriteResult { + /** Final text on disk after BOM handling and formatting. */ + readonly content: string +} + export interface Interface { + /** Read a text file with the BOM stripped. BOM handling stays inside the seam. */ + readonly read: (target: Target) => Effect.Effect readonly write: (input: WriteInput) => Effect.Effect - /** Write text while retaining an existing UTF-8 BOM and emitting at most one BOM. */ - readonly writeTextPreservingBom: (input: TextWriteInput) => Effect.Effect + /** + * Write text while retaining an existing UTF-8 BOM and emitting at most one + * BOM. Runs configured formatters where the files live and reports the + * final text. + */ + readonly writeTextPreservingBom: (input: TextWriteInput) => Effect.Effect + readonly remove: (target: Target) => Effect.Effect } export class Service extends Context.Service()("@opencode/FileMutation") {} +/** Normalize model-provided text to the BOM-free representation tools consume. */ +export const normalizeText = (content: string) => Bom.split(content).text + const writeResult = (target: Target, existed: boolean): WriteResult => ({ operation: "write", target: target.canonical, @@ -53,12 +81,31 @@ const layer = Layer.effect( Service, Effect.gen(function* () { const fs = yield* FSUtil.Service + const formatter = yield* Formatter.Service const locks = KeyedMutex.makeUnsafe() const withTargetLock = (target: Target) => (effect: Effect.Effect) => locks.withLock(target.canonical)(Effect.uninterruptible(effect)) + // Happy-path reads are one operation; only the failure path stats to + // classify a directory target. + const read = Effect.fn("FileMutation.read")((target: Target) => + Bom.readFile(fs, target.canonical).pipe( + Effect.map((content) => content.text), + Effect.catchTag("PlatformError", (error): Effect.Effect => + error.reason._tag === "NotFound" + ? Effect.fail(new NotFoundError({ path: target.canonical })) + : fs.stat(target.canonical).pipe( + Effect.catchTag("PlatformError", () => Effect.succeed(undefined)), + Effect.flatMap((info) => + Effect.fail(info?.type === "Directory" ? new NotAFileError({ path: target.canonical }) : error), + ), + ), + ), + ), + ) + const write = Effect.fn("FileMutation.write")((input: WriteInput) => withTargetLock(input.target)( Effect.gen(function* () { @@ -76,23 +123,37 @@ const layer = Layer.effect( const current = yield* fs .readFile(input.target.canonical) .pipe(Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(undefined))) - yield* fs.writeWithDirs( - input.target.canonical, - Bom.join(next.text, Boolean(current && Bom.has(current)) || next.bom), - ) - return writeResult(input.target, current !== undefined) + const bom = Boolean(current && Bom.has(current)) || next.bom + yield* fs.writeWithDirs(input.target.canonical, Bom.join(next.text, bom)) + // Formatters may rewrite the file, so re-sync the BOM and report the + // final text. + const content = (yield* formatter.file(input.target.canonical)) + ? yield* Bom.syncFile(fs, input.target.canonical, bom) + : next.text + return { ...writeResult(input.target, current !== undefined), content } }), ), ) - return Service.of({ write, writeTextPreservingBom }) + // Removing a symlink unlinks the link itself, never its referent. + const remove = Effect.fn("FileMutation.remove")((target: Target) => + withTargetLock(target)( + fs + .remove(target.absolute ?? target.canonical) + .pipe(Effect.catchReason("PlatformError", "NotFound", () => new NotFoundError({ path: target.canonical }))), + ), + ) + + return Service.of({ read, write, writeTextPreservingBom, remove }) }), ) -export const node = makeLocationNode({ service: Service, layer, deps: [FSUtil.node] }) +export const node = makeLocationNode({ service: Service, layer, deps: [FSUtil.node, Formatter.node] }) -// Same cooperative locking, writes through WorkspaceEnvironment.Files. The +// Same cooperative locking, verbs through WorkspaceEnvironment.Files. The // environment write reports prior existence, so no stat pre-check round trip. +// No formatting: formatters are host binaries and cannot run against provider +// paths until an environment formatter runtime exists. const hostedLayer = Layer.effect( Service, Effect.gen(function* () { @@ -106,6 +167,34 @@ const hostedLayer = Layer.effect( const bytes = (content: string | Uint8Array) => (typeof content === "string" ? encoder.encode(content) : content) + // Absence stays typed; other environment failures surface as filesystem + // errors so tool-level messages stay uniform across backends. + const mapError = (method: string) => (error: WorkspaceEnvironment.Error | WorkspaceEnvironment.NotFoundError) => + error._tag === "WorkspaceEnvironment.NotFoundError" + ? new NotFoundError({ path: error.path }) + : new FSUtil.FileSystemError({ method, cause: error }) + + const read = Effect.fn("FileMutation.read")((target: Target) => + env.files.read(target.canonical).pipe( + Effect.map((content) => Bom.fromBytes(content).text), + Effect.catchTag("WorkspaceEnvironment.NotFoundError", () => + Effect.fail(new NotFoundError({ path: target.canonical })), + ), + Effect.catchTag("WorkspaceEnvironment.Error", (error) => + env.files.stat(target.canonical).pipe( + Effect.catch(() => Effect.succeed(undefined)), + Effect.flatMap((info) => + Effect.fail( + info?.type === "Directory" + ? new NotAFileError({ path: target.canonical }) + : new FSUtil.FileSystemError({ method: "read", cause: error }), + ), + ), + ), + ), + ), + ) + const write = Effect.fn("FileMutation.write")((input: WriteInput) => withTargetLock(input.target)( env.files @@ -124,12 +213,19 @@ const hostedLayer = Layer.effect( const current = yield* WorkspaceEnvironment.optional(env.files.read(input.target.canonical)) const text = Bom.join(next.text, Boolean(current && Bom.has(current)) || next.bom) const result = yield* env.files.write(input.target.canonical, bytes(text)).pipe(Effect.orDie) - return writeResult(input.target, result.existed) + return { ...writeResult(input.target, result.existed), content: next.text } }), ), ) - return Service.of({ write, writeTextPreservingBom }) + // Removing a symlink unlinks the link itself, never its referent. + const remove = Effect.fn("FileMutation.remove")((target: Target) => + withTargetLock(target)( + env.files.remove(target.absolute ?? target.canonical).pipe(Effect.mapError(mapError("remove"))), + ), + ) + + return Service.of({ read, write, writeTextPreservingBom, remove }) }), ) @@ -142,7 +238,6 @@ export const hostedNode = makeLocationNode({ /** * Deferred until the corresponding integrations exist. */ -// TODO: Add formatter integration after formatter runtime exists. // TODO: Publish watcher/file-edit events after watcher integration exists. // TODO: Add snapshots / undo after snapshot design exists. // TODO: Notify LSP and collect diagnostics after LSP runtime exists. diff --git a/packages/core/src/location-mutation.ts b/packages/core/src/location-mutation.ts index b2cf512e427..1820961d269 100644 --- a/packages/core/src/location-mutation.ts +++ b/packages/core/src/location-mutation.ts @@ -47,6 +47,12 @@ export const externalDirectoryPermission = (input: ExternalDirectoryAuthorizatio export interface Target { /** Canonical existing path, or missing path below a canonical directory. */ readonly canonical: string + /** + * Lexical resolved path before symlink canonicalization. Reads and writes + * address the referent (canonical); entry operations like remove address + * the name itself. + */ + readonly absolute: string /** Permission resource: Location-relative for internal paths, canonical for external paths. */ readonly resource: string readonly externalDirectory?: ExternalDirectoryAuthorization @@ -133,6 +139,7 @@ const layer = Layer.effect( const externalResource = slash(path.join(externalDirectory, "*")) return { canonical: resolved.canonical, + absolute, resource, externalDirectory: external ? { @@ -205,6 +212,7 @@ const hostedLayer = Layer.effect( const resolved = yield* resolvePath(absolute) return { canonical: resolved.canonical, + absolute, resource: path.posix.relative(location.directory, absolute) || ".", } satisfies Target }) diff --git a/packages/core/src/location-services.ts b/packages/core/src/location-services.ts index 542a941b55d..3cf8059b3b7 100644 --- a/packages/core/src/location-services.ts +++ b/packages/core/src/location-services.ts @@ -134,7 +134,6 @@ export function buildLocationServiceMap( [LocationMutation.node, LocationMutation.hostedNode], [FileMutation.node, FileMutation.hostedNode], [Shell.node, Shell.hostedNode], - [PluginSupervisor.node, PluginSupervisor.hostedNode], ] : [[Location.node, Location.boundNode(ref)]], ) diff --git a/packages/core/src/plugin/internal.ts b/packages/core/src/plugin/internal.ts index 004ef880e58..defc4c27094 100644 --- a/packages/core/src/plugin/internal.ts +++ b/packages/core/src/plugin/internal.ts @@ -1,8 +1,7 @@ export * as PluginInternal from "./internal" import type { Plugin } from "@opencode-ai/plugin/effect/plugin" -import { Context, Effect, Option, Scope } from "effect" -import { WorkspaceEnvironment } from "../workspace/environment" +import { Context, Effect, Scope } from "effect" import { HttpClient } from "effect/unstable/http" import { Agent } from "../agent" import { Catalog } from "../catalog" @@ -96,14 +95,7 @@ const services = Effect.fn("PluginInternal.services")(function* () { const skill = yield* Skill.Service const tools = yield* Tool.Service const wellknown = yield* WellKnown.Service - // Bound only in hosted Location graphs; plugins read it with serviceOption - // to route filesystem checks at the provider instead of the host. - const environment = yield* Effect.serviceOption(WorkspaceEnvironment.Service) return Context.mergeAll( - Option.match(environment, { - onSome: (value) => Context.make(WorkspaceEnvironment.Service, value), - onNone: () => Context.empty(), - }), Context.make(Agent.Service, agent), Context.make(Catalog.Service, catalog), Context.make(Command.Service, command), diff --git a/packages/core/src/plugin/supervisor.ts b/packages/core/src/plugin/supervisor.ts index 79ec0c0d9b4..a7d79a8cef4 100644 --- a/packages/core/src/plugin/supervisor.ts +++ b/packages/core/src/plugin/supervisor.ts @@ -40,7 +40,6 @@ import { ReadToolFileSystem } from "../tool/read-filesystem" import { Tool } from "../tool" import { WebSearch } from "../websearch" import { WellKnown } from "../wellknown" -import { WorkspaceEnvironment } from "../workspace/environment" import { PluginInternal } from "./internal" import { PluginRuntime } from "./runtime" import { SdkPlugins } from "./sdk" @@ -354,11 +353,4 @@ export const node = makeLocationNode({ deps: nodeDeps, }) -/** Hosted graphs bind the workspace environment so internal plugins can reach it. */ -export const hostedNode = makeLocationNode({ - service: Service, - layer: nodeLayer, - deps: [...nodeDeps, WorkspaceEnvironment.node], -}) - export { layer } diff --git a/packages/core/src/session.ts b/packages/core/src/session.ts index bce4322bc60..7f86fc3922c 100644 --- a/packages/core/src/session.ts +++ b/packages/core/src/session.ts @@ -233,7 +233,7 @@ export interface Interface { id?: Event.ID sessionID: SessionSchema.ID command: string - }) => Effect.Effect + }) => Effect.Effect readonly skill: (input: { id?: SessionMessage.ID sessionID: SessionSchema.ID diff --git a/packages/core/src/shell.ts b/packages/core/src/shell.ts index 7bfd29da8fc..1592e6483c6 100644 --- a/packages/core/src/shell.ts +++ b/packages/core/src/shell.ts @@ -6,6 +6,7 @@ import { ChildProcess } from "effect/unstable/process" import { produce } from "immer" import { Shell } from "@opencode-ai/schema/shell" import { makeLocationNode } from "@opencode-ai/util/effect/app-node" +import { FSUtil } from "@opencode-ai/util/fs-util" import { AppProcess } from "@opencode-ai/util/process" import { Config } from "./config" import { Bus } from "./bus" @@ -20,6 +21,18 @@ export class NotFoundError extends Schema.TaggedErrorClass()("She id: Shell.ID, }) {} +export class InvalidCwdError extends Schema.TaggedErrorClass()("Shell.InvalidCwdError", { + path: Schema.String, + reason: Schema.Literals(["not_found", "not_directory", "unavailable"]), + cause: Schema.optional(Schema.Defect()), +}) { + override get message() { + if (this.reason === "not_found") return `Working directory does not exist: ${this.path}` + if (this.reason === "not_directory") return `Working directory is not a directory: ${this.path}` + return `Unable to inspect working directory: ${this.path}` + } +} + // Exited processes stay observable (status, exit code, retained output) until removed explicitly. // Cap retention so abandoned commands do not accumulate unbounded state and output files. const EXITED_LIMIT = 25 @@ -51,7 +64,7 @@ export interface Interface { readonly create: ( input: Shell.CreateInput, before?: (input: ShellCreateBefore) => Effect.Effect, - ) => Effect.Effect + ) => Effect.Effect // Currently running commands only; exited shells are retained for get/output but excluded here. readonly list: () => Effect.Effect readonly get: (id: Shell.ID) => Effect.Effect @@ -75,6 +88,7 @@ interface Backend { /** Base environment for spawned commands. Hosted backends must not leak host process.env. */ readonly env: Readonly> readonly detached: boolean + readonly validateCwd: (cwd: string) => Effect.Effect } const layerWith = (backend: Effect.Effect) => Layer.effect( @@ -201,6 +215,7 @@ const layerWith = (backend: Effect.Effect) => Layer.effect( } yield* hooks.trigger("shell", "create.before", invocation) if (before) yield* before(invocation) + yield* spawner.validateCwd(invocation.cwd) const id = Shell.ID.ascending() const args = spawner.args(invocation.shell, invocation.command) @@ -350,6 +365,7 @@ export const layer = (options?: ShellSelect.Options) => Effect.gen(function* () { const config = yield* Config.Service const appProcess = yield* AppProcess.Service + const fs = yield* FSUtil.Service return { spawn: (command) => appProcess.spawn(command), shell: config @@ -358,6 +374,21 @@ export const layer = (options?: ShellSelect.Options) => args: ShellSelect.args, env: process.env, detached: process.platform !== "win32", + validateCwd: (cwd) => + fs.stat(cwd).pipe( + Effect.mapError((error) => + new InvalidCwdError({ + path: cwd, + reason: error.reason._tag === "NotFound" ? "not_found" : "unavailable", + cause: error, + }), + ), + Effect.flatMap((info) => + info.type === "Directory" + ? Effect.void + : Effect.fail(new InvalidCwdError({ path: cwd, reason: "not_directory" })), + ), + ), } satisfies Backend }), ) @@ -366,7 +397,7 @@ export function configured(options?: ShellSelect.Options) { return makeLocationNode({ service: Service, layer: layer(options), - deps: [Bus.node, Location.node, Config.node, Global.node, AppProcess.node, PluginHooks.node], + deps: [Bus.node, Location.node, Config.node, Global.node, AppProcess.node, FSUtil.node, PluginHooks.node], }) } @@ -385,6 +416,21 @@ export const hostedNode = makeLocationNode({ args: (_shell, command) => env.shell.args(command), env: env.shell.environmentOverrides, detached: env.shell.detached, + validateCwd: (cwd) => + env.files.stat(cwd).pipe( + Effect.mapError((error) => + new InvalidCwdError({ + path: cwd, + reason: error._tag === "WorkspaceEnvironment.NotFoundError" ? "not_found" : "unavailable", + cause: error, + }), + ), + Effect.flatMap((info) => + info.type === "Directory" + ? Effect.void + : Effect.fail(new InvalidCwdError({ path: cwd, reason: "not_directory" })), + ), + ), } satisfies Backend }), ), diff --git a/packages/core/src/tool/plugin/edit.ts b/packages/core/src/tool/plugin/edit.ts index 1fc5d55461e..9c0facc80c6 100644 --- a/packages/core/src/tool/plugin/edit.ts +++ b/packages/core/src/tool/plugin/edit.ts @@ -12,8 +12,6 @@ import { FileDiff } from "@opencode-ai/schema/file-diff" import { Bom } from "@opencode-ai/util/bom" import { Effect, Schema } from "effect" import { FileMutation } from "../../file-mutation" -import { Formatter } from "../../formatter" -import { FSUtil } from "@opencode-ai/util/fs-util" import { LocationMutation } from "../../location-mutation" import { Permission } from "../../permission" import { fileDiff } from "./file-diff" @@ -110,8 +108,6 @@ export const Plugin = { effect: Effect.fn("EditTool.Plugin")(function* (ctx: PluginContext) { const mutation = yield* LocationMutation.Service const files = yield* FileMutation.Service - const formatter = yield* Formatter.Service - const fs = yield* FSUtil.Service const permission = yield* Permission.Service yield* ctx.tool @@ -153,16 +149,14 @@ export const Plugin = { }) } - const info = yield* fs.stat(target.canonical).pipe( - Effect.catchReason("PlatformError", "NotFound", () => + const source = yield* files.read(target).pipe( + Effect.catchTag("FileMutation.NotFoundError", () => Effect.fail(new ToolFailure({ message: `File not found: ${input.path}` })), ), + Effect.catchTag("FileMutation.NotAFileError", () => + Effect.fail(new ToolFailure({ message: `Path is a directory, not a file: ${input.path}` })), + ), ) - if (info.type === "Directory") { - return yield* new ToolFailure({ message: `Path is a directory, not a file: ${input.path}` }) - } - const original = yield* Bom.readFile(fs, target.canonical) - const source = original.text const ending = source.includes(crlf) ? crlf : "\n" const oldString = input.oldString.replaceAll(crlf, "\n").replaceAll("\n", ending) const newString = input.newString.replaceAll(crlf, "\n").replaceAll("\n", ending) @@ -183,9 +177,10 @@ export const Plugin = { `${content.slice(0, match.start)}${newString}${content.slice(match.end)}`, source, ) + const content = FileMutation.normalizeText(replaced) const preview = replacements > 0 && (replacements === 1 || input.replaceAll === true) - ? fileDiff(target.resource, source, replaced) + ? fileDiff(target.resource, source, content) : undefined yield* permission.assert({ action: "edit", @@ -206,17 +201,9 @@ export const Plugin = { message: `Found ${replacements} matches for oldString, but expected exactly one. Add more surrounding context to make oldString unique, or set replaceAll to true to replace every occurrence.`, }) } - const replacementBom = replaced.startsWith("\uFEFF") - const result = yield* files.write({ - target, - content: Bom.join(replaced, original.bom || replacementBom), - }) - const bom = original.bom || replacementBom - const formatted = (yield* formatter.file(target.canonical)) - ? yield* Bom.syncFile(fs, target.canonical, bom) - : (yield* Bom.readFile(fs, target.canonical)).text + const result = yield* files.writeTextPreservingBom({ target, content: replaced }) return { - files: [fileDiff(result.resource, source, formatted)], + files: [fileDiff(result.resource, source, result.content)], replacements, } satisfies Output }).pipe( diff --git a/packages/core/src/tool/plugin/patch.ts b/packages/core/src/tool/plugin/patch.ts index c5b0fc66859..66a8050cc06 100644 --- a/packages/core/src/tool/plugin/patch.ts +++ b/packages/core/src/tool/plugin/patch.ts @@ -4,16 +4,12 @@ import type { Context as PluginContext } from "@opencode-ai/plugin/effect/plugin import { ToolFailure } from "@opencode-ai/ai" import { FileDiff } from "@opencode-ai/schema/file-diff" import { createTwoFilesPatch, diffLines } from "diff" -import { Effect, FileSystem, Option, Schema } from "effect" +import { Effect, Schema } from "effect" import { PlatformError } from "effect/PlatformError" -import path from "path" -import { Bom } from "@opencode-ai/util/bom" -import { FSUtil } from "@opencode-ai/util/fs-util" -import { Formatter } from "../../formatter" -import { Location } from "../../location" +import { FileMutation } from "../../file-mutation" +import { LocationMutation } from "../../location-mutation" import { Patch } from "@opencode-ai/util/patch" import { Permission } from "../../permission" -import { WorkspaceEnvironment } from "../../workspace/environment" import DESCRIPTION from "../patch.txt" export const name = "patch" @@ -58,54 +54,14 @@ type Prepared = readonly moveTarget?: Target }) -interface Target { - readonly canonical: string - readonly resource: string - readonly externalDirectory?: { - readonly directory: string - readonly resource: string - } -} - -type BackendError = PlatformError | globalThis.Error | WorkspaceEnvironment.Error | WorkspaceEnvironment.NotFoundError - -/** The four verbs patch needs, provided by the host filesystem or the workspace environment. */ -interface Backend { - readonly read: (path: string) => Effect.Effect<{ readonly bom: boolean; readonly text: string }, BackendError> - readonly stat: (path: string) => Effect.Effect<{ readonly type: FileSystem.File.Type }, BackendError> - readonly write: (path: string, content: string) => Effect.Effect - readonly remove: (path: string) => Effect.Effect -} - -const hostBackend = (fs: FSUtil.Interface): Backend => ({ - read: (target) => Bom.readFile(fs, target), - stat: (target) => fs.stat(target), - write: (target, content) => fs.writeWithDirs(target, content), - remove: (target) => fs.remove(target), -}) - -const environmentBackend = (environment: WorkspaceEnvironment.Interface): Backend => { - const encoder = new TextEncoder() - return { - read: (target) => Effect.map(environment.files.read(target), Bom.fromBytes), - stat: (target) => environment.files.stat(target), - write: (target, content) => Effect.asVoid(environment.files.write(target, encoder.encode(content))), - remove: (target) => environment.files.remove(target), - } -} +type Target = LocationMutation.Target export const Plugin = { id: "opencode.tool.patch", effect: Effect.fn("PatchTool.Plugin")(function* (ctx: PluginContext) { - const fs = yield* FSUtil.Service - const formatter = yield* Formatter.Service - const location = yield* Location.Service + const mutation = yield* LocationMutation.Service + const files = yield* FileMutation.Service const permission = yield* Permission.Service - // Hosted Locations bind the workspace environment: file verbs run at the - // provider, paths resolve as posix, and host formatters never run. - const environment = Option.getOrUndefined(yield* Effect.serviceOption(WorkspaceEnvironment.Service)) - const hosted = environment !== undefined - const backend = environment ? environmentBackend(environment) : hostBackend(fs) yield* ctx.tool .transform((draft) => @@ -142,19 +98,20 @@ export const Plugin = { const prepared: Prepared[] = [] const targets: Target[] = [] const updates = new Map() + const resolveHunkTarget = (value: string) => + mutation.resolve({ path: value, kind: "file" }).pipe( + Effect.mapError( + (error) => + new ToolFailure({ message: `patch verification failed: ${errorMessage(error)}` }), + ), + ) for (const hunk of hunks) { yield* Effect.gen(function* () { - const target = resolveTarget(location, hunk.path, hosted) + const target = yield* resolveHunkTarget(hunk.path) targets.push(target) if (target.externalDirectory) { yield* permission.assert({ - action: "external_directory", - resources: [target.externalDirectory.resource], - save: [target.externalDirectory.resource], - metadata: { - filepath: target.canonical, - parentDir: target.externalDirectory.directory, - }, + ...LocationMutation.externalDirectoryPermission(target.externalDirectory), sessionID: context.sessionID, agent: context.agent, source, @@ -165,16 +122,16 @@ export const Plugin = { ...hunk, target, before: "", - after: Bom.split( + after: FileMutation.normalizeText( hunk.contents.endsWith("\n") || hunk.contents === "" ? hunk.contents : `${hunk.contents}\n`, - ).text, + ), }) return } if (hunk.type === "delete") { - const content = yield* backend.read(target.canonical).pipe( + const content = yield* files.read(target).pipe( Effect.mapError( (error) => new ToolFailure({ @@ -182,53 +139,32 @@ export const Plugin = { }), ), ) - prepared.push({ ...hunk, target, before: content.text, after: "" }) + prepared.push({ ...hunk, target, before: content, after: "" }) return } const previous = updates.get(target.canonical) const original = previous ?? - (yield* Effect.gen(function* () { - const stats = yield* backend.stat(target.canonical).pipe( - Effect.mapError( - (error) => - new ToolFailure({ - message: `patch verification failed: Failed to read file to update ${target.canonical}: ${errorMessage(error)}`, - }), - ), - ) - if (stats.type === "Directory") { - return yield* new ToolFailure({ - message: `patch verification failed: Failed to read file to update ${target.canonical}: path is a directory`, - }) - } - const content = yield* backend.read(target.canonical).pipe( - Effect.mapError( - (error) => - new ToolFailure({ - message: `patch verification failed: Failed to read file to update ${target.canonical}: ${errorMessage(error)}`, - }), - ), - ) - return Bom.join(content.text, content.bom) - })) - const before = Bom.split(original).text + (yield* files.read(target).pipe( + Effect.mapError( + (error) => + new ToolFailure({ + message: `patch verification failed: Failed to read file to update ${target.canonical}: ${errorMessage(error)}`, + }), + ), + )) + const before = original const update = yield* Effect.try({ try: () => Patch.derive(hunk.path, hunk.chunks, original), catch: (error) => new ToolFailure({ message: `patch verification failed: ${errorMessage(error)}` }), }) - const moveTarget = hunk.movePath ? resolveTarget(location, hunk.movePath, hosted) : undefined + const content = FileMutation.normalizeText(update.content) + const moveTarget = hunk.movePath ? yield* resolveHunkTarget(hunk.movePath) : undefined if (moveTarget) targets.push(moveTarget) if (moveTarget?.externalDirectory) { yield* permission.assert({ - action: "external_directory", - resources: [moveTarget.externalDirectory.resource], - save: [moveTarget.externalDirectory.resource], - metadata: { - filepath: moveTarget.canonical, - parentDir: moveTarget.externalDirectory.directory, - }, + ...LocationMutation.externalDirectoryPermission(moveTarget.externalDirectory), sessionID: context.sessionID, agent: context.agent, source, @@ -239,10 +175,10 @@ export const Plugin = { target, content: Patch.joinBom(update.content, update.bom), before, - after: update.content, + after: content, moveTarget, }) - if (!moveTarget) updates.set(target.canonical, Patch.joinBom(update.content, update.bom)) + if (!moveTarget) updates.set(target.canonical, content) }).pipe( Effect.mapError((error) => error instanceof ToolFailure @@ -267,21 +203,26 @@ export const Plugin = { source, }) + // writeTextPreservingBom formats where the files live and + // reports the final text, so the diff output reflects disk. + const formatted = new Map() yield* Effect.forEach( prepared, (change) => Effect.gen(function* () { if (change.type === "add") { - yield* backend - .write( - change.target.canonical, - change.contents.endsWith("\n") || change.contents === "" - ? change.contents - : `${change.contents}\n`, - ) + const result = yield* files + .writeTextPreservingBom({ + target: change.target, + content: + change.contents.endsWith("\n") || change.contents === "" + ? change.contents + : `${change.contents}\n`, + }) .pipe( Effect.mapError((error) => fail(`Failed to write ${change.target.resource}`, error)), ) + formatted.set(change.target.canonical, result.content) applied.push({ type: change.type, resource: change.target.resource, @@ -290,8 +231,8 @@ export const Plugin = { return } if (change.type === "delete") { - yield* backend - .remove(change.target.canonical) + yield* files + .remove(change.target) .pipe( Effect.mapError((error) => fail(`Failed to delete ${change.target.resource}`, error)), ) @@ -304,10 +245,11 @@ export const Plugin = { } if (change.moveTarget) { const moveTarget = change.moveTarget - yield* backend - .write(moveTarget.canonical, change.content) + const result = yield* files + .writeTextPreservingBom({ target: moveTarget, content: change.content }) .pipe(Effect.mapError((error) => fail(`Failed to write ${moveTarget.resource}`, error))) - yield* backend.remove(change.target.canonical).pipe( + formatted.set(moveTarget.canonical, result.content) + yield* files.remove(change.target).pipe( Effect.mapError((error) => fail(`Wrote ${moveTarget.resource} but failed to remove ${change.target.resource}`, error), ), @@ -319,9 +261,10 @@ export const Plugin = { }) return } - yield* backend - .write(change.target.canonical, change.content) + const result = yield* files + .writeTextPreservingBom({ target: change.target, content: change.content }) .pipe(Effect.mapError((error) => fail(`Failed to write ${change.target.resource}`, error))) + formatted.set(change.target.canonical, result.content) applied.push({ type: change.type, resource: change.target.resource, @@ -330,35 +273,12 @@ export const Plugin = { }), { discard: true }, ) - // Hosted Locations skip formatting: formatters are host - // binaries and cannot run against provider paths. patchFile - // then falls back to the in-memory after-content. - const formatted = new Map() - if (!hosted) - yield* Effect.forEach( - [...new Set(applied.filter((item) => item.type !== "delete").map((item) => item.target))], - (target) => - Effect.gen(function* () { - const current = yield* Bom.readFile(fs, target).pipe( - Effect.mapError((error) => fail(`Failed to read ${target}`, error)), - ) - formatted.set( - target, - (yield* formatter.file(target)) - ? yield* Bom.syncFile(fs, target, current.bom).pipe( - Effect.mapError((error) => fail(`Failed to sync ${target}`, error)), - ) - : current.text, - ) - }), - { discard: true }, - ) - const files = yield* Effect.forEach(prepared, (change) => { + const fileDiffs = yield* Effect.forEach(prepared, (change) => { if (change.type === "delete") return Effect.succeed(patchFile(change)) const target = change.type === "update" && change.moveTarget ? change.moveTarget : change.target return Effect.succeed(patchFile(change, formatted.get(target.canonical))) }) - return { applied, files } + return { applied, files: fileDiffs } }).pipe( Effect.map((output) => ({ output, @@ -391,9 +311,13 @@ export const Plugin = { ) }), } - function errorMessage(error: unknown) { - if (error instanceof WorkspaceEnvironment.NotFoundError) return "file does not exist" + if (error instanceof FileMutation.NotFoundError) return "file does not exist" + if (error instanceof FileMutation.NotAFileError) return "path is a directory" + if (error instanceof LocationMutation.PathError) + return error.reason === "outside_workspace" + ? `path is outside the workspace: ${error.path}` + : `ancestor is not a directory: ${error.path}` if (error instanceof PlatformError) { if (error.reason._tag === "NotFound") return "file does not exist" return error.reason.description ?? error.reason.message @@ -452,28 +376,3 @@ function trimDiff(diff: string) { }) .join("\n") } - -function resolveTarget(location: Location.Interface, value: string, hosted: boolean): Target { - // Hosted paths live in the provider filesystem: always posix, regardless of - // the host platform. - const paths = hosted ? path.posix : path - const contains = hosted ? FSUtil.containsPosix : FSUtil.contains - const canonical = - !hosted && process.platform === "win32" - ? FSUtil.normalizePath(path.resolve(location.directory, value)) - : paths.resolve(location.directory, value) - const projectRoot = paths.parse(location.project.directory).root - const external = - !contains(location.directory, canonical) && - (location.project.directory === projectRoot || !contains(location.project.directory, canonical)) - const directory = paths.dirname(canonical) - const resource = - !hosted && process.platform === "win32" - ? FSUtil.normalizePathPattern(path.join(directory, "*")) - : paths.join(directory, "*").replaceAll("\\", "/") - return { - canonical, - resource: paths.relative(location.project.directory, canonical).replaceAll("\\", "/") || ".", - externalDirectory: external ? { directory, resource } : undefined, - } -} diff --git a/packages/core/src/tool/plugin/shell.ts b/packages/core/src/tool/plugin/shell.ts index ab9efc41290..4bc7dc0e96c 100644 --- a/packages/core/src/tool/plugin/shell.ts +++ b/packages/core/src/tool/plugin/shell.ts @@ -4,9 +4,7 @@ import path from "path" import { ToolFailure } from "@opencode-ai/ai" import type { Content } from "@opencode-ai/schema/tool" import type { Context as PluginContext } from "@opencode-ai/plugin/effect/plugin" -import { Deferred, Effect, Option, Schema, Scope } from "effect" -import { FSUtil } from "@opencode-ai/util/fs-util" -import { WorkspaceEnvironment } from "../../workspace/environment" +import { Deferred, Effect, Schema, Scope } from "effect" import { LocationMutation } from "../../location-mutation" import { Permission } from "../../permission" import { PluginRuntime } from "../../plugin/runtime" @@ -83,22 +81,6 @@ export const Plugin = { effect: Effect.fn("ShellTool.Plugin")(function* (ctx: PluginContext) { const runtime = yield* PluginRuntime.Service const scope = yield* Scope.Scope - const fsUtil = yield* FSUtil.Service - // Hosted Locations bind the workspace environment; the workdir check must - // stat the provider filesystem there, never the host's. - const environment = Option.getOrUndefined(yield* Effect.serviceOption(WorkspaceEnvironment.Service)) - const statWorkdir = (canonical: string) => - environment - ? environment.files.stat(canonical).pipe( - Effect.catchTag("WorkspaceEnvironment.NotFoundError", () => - Effect.fail(new Error(`Working directory does not exist: ${canonical}`)), - ), - ) - : fsUtil.stat(canonical).pipe( - Effect.catchReason("PlatformError", "NotFound", () => - Effect.fail(new Error(`Working directory does not exist: ${canonical}`)), - ), - ) const mutation = yield* LocationMutation.Service const shell = yield* Shell.Service const permission = yield* Permission.Service @@ -192,9 +174,6 @@ export const Plugin = { agent: context.agent, source, }) - const workdir = yield* statWorkdir(target.canonical) - if (workdir.type !== "Directory") - return yield* Effect.fail(new Error(`Working directory is not a directory: ${target.canonical}`)) }), ) yield* context.progress({ shellID: info.id }) diff --git a/packages/core/src/tool/plugin/write.ts b/packages/core/src/tool/plugin/write.ts index e04d7ef7ad9..83b32b8ef93 100644 --- a/packages/core/src/tool/plugin/write.ts +++ b/packages/core/src/tool/plugin/write.ts @@ -9,10 +9,7 @@ export * as WriteTool from "./write" import type { Context as PluginContext } from "@opencode-ai/plugin/effect/plugin" import { ToolFailure } from "@opencode-ai/ai" import { Effect, Schema } from "effect" -import { Bom } from "@opencode-ai/util/bom" -import { FSUtil } from "@opencode-ai/util/fs-util" import { FileMutation } from "../../file-mutation" -import { Formatter } from "../../formatter" import { LocationMutation } from "../../location-mutation" import { Permission } from "../../permission" import { fileDiff } from "./file-diff" @@ -48,8 +45,6 @@ export const Plugin = { effect: Effect.fn("WriteTool.Plugin")(function* (ctx: PluginContext) { const mutation = yield* LocationMutation.Service const files = yield* FileMutation.Service - const formatter = yield* Formatter.Service - const fs = yield* FSUtil.Service const permission = yield* Permission.Service yield* ctx.tool @@ -78,15 +73,15 @@ export const Plugin = { agent: context.agent, source, }) - const current = yield* Bom.readFile(fs, target.canonical).pipe( - Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(undefined)), - ) - const next = Bom.split(input.content) + const current = yield* files + .read(target) + .pipe(Effect.catchTag("FileMutation.NotFoundError", () => Effect.succeed(undefined))) + const content = FileMutation.normalizeText(input.content) const preview = fileDiff( target.resource, - current?.text ?? "", - next.text, - current ? "modified" : "added", + current ?? "", + content, + current === undefined ? "added" : "modified", ) yield* permission.assert({ action: "edit", @@ -98,9 +93,12 @@ export const Plugin = { source, }) const result = yield* files.writeTextPreservingBom({ target, content: input.content }) - const bom = (yield* Bom.readFile(fs, target.canonical)).bom - if (yield* formatter.file(target.canonical)) yield* Bom.syncFile(fs, target.canonical, bom) - return result + return { + operation: result.operation, + target: result.target, + resource: result.resource, + existed: result.existed, + } }).pipe( Effect.map((output) => ({ output, content: toModelOutput(output) })), Effect.mapError((error) => new ToolFailure({ message: `Unable to write ${input.path}`, error })), diff --git a/packages/core/test/tool-patch.test.ts b/packages/core/test/tool-patch.test.ts index 4a1a2e095f8..33e1dd5217c 100644 --- a/packages/core/test/tool-patch.test.ts +++ b/packages/core/test/tool-patch.test.ts @@ -13,6 +13,8 @@ import { AbsolutePath } from "@opencode-ai/core/schema" import { Session } from "@opencode-ai/core/session" import { Tool } from "@opencode-ai/core/tool" import { PatchTool } from "@opencode-ai/core/tool/plugin/patch" +import { FileMutation } from "@opencode-ai/core/file-mutation" +import { LocationMutation } from "@opencode-ai/core/location-mutation" import { location } from "./fixture/location" import { tmpdir } from "./fixture/tmpdir" import { makeLocationNode } from "@opencode-ai/util/effect/app-node" @@ -22,7 +24,7 @@ import { toolIdentity, executeTool, registerToolPlugin, toolDefinitions } from " const patchToolNode = makeLocationNode({ name: "test/patch-tool-plugin", layer: Layer.effectDiscard(registerToolPlugin(PatchTool.Plugin)), - deps: [Tool.node, Formatter.node, FSUtil.node, Location.node, Permission.node], + deps: [Tool.node, LocationMutation.node, FileMutation.node, Permission.node], }) const sessionID = Session.ID.make("ses_patch_tool_test") @@ -828,7 +830,9 @@ describe("PatchTool", () => { ), ) - it.live("treats a sibling path inside the project worktree as internal", () => + // Paths outside the active Location require external approval — the same + // boundary the edit and write tools derive from LocationMutation. + it.live("requires external approval for sibling paths outside the location", () => Effect.acquireUseRelease( Effect.promise(() => tmpdir()), (tmp) => { @@ -847,7 +851,7 @@ describe("PatchTool", () => { call("*** Begin Patch\n*** Update File: ../sibling.txt\n@@\n-before\n+after\n*** End Patch"), ), ).toMatchObject({ status: "completed" }) - expect(assertions.map((input) => input.action)).toEqual(["edit"]) + expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"]) expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\n") }), tmp.path, diff --git a/packages/core/test/tool-read.test.ts b/packages/core/test/tool-read.test.ts index c9b549646e4..5e86061b4e8 100644 --- a/packages/core/test/tool-read.test.ts +++ b/packages/core/test/tool-read.test.ts @@ -154,6 +154,7 @@ const mutation = Layer.succeed( const externalResource = path.join(directory, "*").replaceAll("\\", "/") return Effect.succeed({ canonical, + absolute: canonical, resource, externalDirectory: external ? { diff --git a/packages/core/test/workspace-patch.test.ts b/packages/core/test/workspace-patch.test.ts index 10df6630b9d..3033f216fe8 100644 --- a/packages/core/test/workspace-patch.test.ts +++ b/packages/core/test/workspace-patch.test.ts @@ -2,9 +2,9 @@ import { describe, expect } from "bun:test" import { Effect, Layer } from "effect" import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" import { LayerNode } from "@opencode-ai/util/effect/layer-node" -import { FSUtil } from "@opencode-ai/util/fs-util" -import { Formatter } from "@opencode-ai/core/formatter" +import { FileMutation } from "@opencode-ai/core/file-mutation" import { Location } from "@opencode-ai/core/location" +import { LocationMutation } from "@opencode-ai/core/location-mutation" import { Permission } from "@opencode-ai/core/permission" import { Session } from "@opencode-ai/core/session" import { Tool } from "@opencode-ai/core/tool" @@ -35,32 +35,12 @@ const permission = Layer.succeed( }), ) -// Hosted patch must never format nor touch the host filesystem: both die. -const formatter = Layer.mock(Formatter.Service, { - file: () => Effect.die("hosted patch must not run host formatters"), -}) - -const poisoned = () => Effect.die("hosted patch must not touch the host filesystem") -const filesystem = Layer.effect( - FSUtil.Service, - Effect.gen(function* () { - const fs = yield* FSUtil.Service - return FSUtil.Service.of({ - ...fs, - stat: poisoned, - readFile: poisoned, - writeWithDirs: poisoned, - remove: poisoned, - }) - }), -).pipe(Layer.provide(LayerNode.compile(FSUtil.node))) - const withTool = (memory: MemoryEnvironment, body: (registry: Tool.Interface) => Effect.Effect) => { const environment = Layer.succeed(WorkspaceEnvironment.Service, memory.environment) const patchToolNode = makeLocationNode({ name: "test/workspace-patch-plugin", layer: Layer.effectDiscard(registerToolPlugin(PatchTool.Plugin)), - deps: [Tool.node, Formatter.node, FSUtil.node, Location.node, Permission.node, WorkspaceEnvironment.node], + deps: [Tool.node, LocationMutation.node, FileMutation.node, Permission.node], }) return Effect.gen(function* () { assertions.length = 0 @@ -68,9 +48,9 @@ const withTool = (memory: MemoryEnvironment, body: (registry: Tool.Inte }).pipe( Effect.provide( AppNodeBuilder.build(LayerNode.group([Tool.node, patchToolNode]), [ - [FSUtil.node, filesystem], [Location.node, hostedLocationLayer()], - [Formatter.node, formatter], + [LocationMutation.node, LocationMutation.hostedNode], + [FileMutation.node, FileMutation.hostedNode], [Permission.node, permission], [WorkspaceEnvironment.node, environment], ]), @@ -148,13 +128,19 @@ describe("PatchTool on a hosted location", () => { ) }) - it.effect("resolves external targets with posix containment", () => { + it.effect("rejects targets outside the workspace", () => { const memory = memoryEnvironment({}) return withTool(memory, (registry) => Effect.gen(function* () { - yield* executeTool(registry, call("*** Begin Patch\n*** Add File: /outside/new.txt\n+created\n*** End Patch")) - expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"]) - expect(assertions[0]?.metadata).toMatchObject({ filepath: "/outside/new.txt", parentDir: "/outside" }) + const settled = yield* executeTool( + registry, + call("*** Begin Patch\n*** Add File: /outside/new.txt\n+created\n*** End Patch"), + ) + expect(settled.status).toBe("error") + if (settled.status !== "error") return + expect(settled.error?.message).toContain("path is outside the workspace") + expect(assertions).toEqual([]) + expect(memory.contents("/outside/new.txt")).toBeUndefined() }), ) }) diff --git a/packages/server/src/handlers/session.ts b/packages/server/src/handlers/session.ts index 8d145165664..ac95f290033 100644 --- a/packages/server/src/handlers/session.ts +++ b/packages/server/src/handlers/session.ts @@ -402,6 +402,9 @@ export const SessionHandler = HttpApiBuilder.group(Api, "server.session", (handl }), ), ), + Effect.catchTag("Shell.InvalidCwdError", (error) => + Effect.fail(new InvalidRequestError({ message: error.message })), + ), ) return HttpApiSchema.NoContent.make() }), diff --git a/packages/server/src/handlers/shell.ts b/packages/server/src/handlers/shell.ts index ac1ae09a8c2..90052aa64ba 100644 --- a/packages/server/src/handlers/shell.ts +++ b/packages/server/src/handlers/shell.ts @@ -2,7 +2,7 @@ import { Shell } from "@opencode-ai/core/shell" import { Location } from "@opencode-ai/core/location" import { Effect } from "effect" import { HttpApiBuilder, HttpApiSchema } from "effect/unstable/httpapi" -import { ShellNotFoundError } from "@opencode-ai/protocol/errors" +import { InvalidRequestError, ShellNotFoundError } from "@opencode-ai/protocol/errors" import { Api } from "../api" import { response } from "../location" @@ -21,7 +21,11 @@ export const ShellHandler = HttpApiBuilder.group(Api, "server.shell", (handlers) Effect.fn(function* (ctx) { const shell = yield* Shell.Service const location = yield* Location.Service - return yield* response(shell.create({ ...ctx.payload, cwd: ctx.payload.cwd || location.directory })) + return yield* response( + shell + .create({ ...ctx.payload, cwd: ctx.payload.cwd || location.directory }) + .pipe(Effect.mapError((error) => new InvalidRequestError({ message: error.message }))), + ) }), ) .handle(