1
0
Эх сурвалжийг харах

refactor(core): simplify filesystem mutation protocol (#31059)

Kit Langton 2 сар өмнө
parent
commit
ceccde7e84

+ 64 - 72
packages/core/src/file-mutation.ts

@@ -4,15 +4,19 @@ import { Context, Effect, Layer, Schema } from "effect"
 import { dirname } from "path"
 import { KeyedMutex } from "./effect/keyed-mutex"
 import { FSUtil } from "./fs-util"
-import { LocationMutation } from "./location-mutation"
+
+export interface Target {
+  readonly canonical: string
+  readonly resource: string
+}
 
 export interface WriteInput {
-  readonly plan: LocationMutation.Plan
+  readonly target: Target
   readonly content: string | Uint8Array
 }
 
 export interface TextWriteInput {
-  readonly plan: LocationMutation.Plan
+  readonly target: Target
   readonly content: string
 }
 
@@ -21,7 +25,7 @@ export interface ConditionalWriteInput extends WriteInput {
 }
 
 export interface RemoveInput {
-  readonly plan: LocationMutation.Plan
+  readonly target: Target
 }
 
 export class StaleContentError extends Schema.TaggedErrorClass<StaleContentError>()("FileMutation.StaleContentError", {
@@ -34,143 +38,131 @@ export class TargetExistsError extends Schema.TaggedErrorClass<TargetExistsError
 
 export interface WriteResult {
   readonly operation: "write"
-  /** Canonical target actually passed to the filesystem mutation. */
   readonly target: string
-  /** Permission resource captured during planning. */
   readonly resource: string
   readonly existed: boolean
 }
 
 export interface RemoveResult {
   readonly operation: "remove"
-  /** Canonical target actually passed to the filesystem mutation. */
   readonly target: string
-  /** Permission resource captured during planning. */
   readonly resource: string
   readonly existed: boolean
 }
 
 export interface Interface {
-  /** Create only while the planned target remains absent. */
-  readonly create: (
-    input: WriteInput,
-  ) => Effect.Effect<WriteResult, TargetExistsError | LocationMutation.RevalidationError | FSUtil.Error>
-  /** Write after immediately revalidating the planned target. */
-  readonly write: (input: WriteInput) => Effect.Effect<WriteResult, LocationMutation.RevalidationError | FSUtil.Error>
+  /** Create without replacing an existing target. */
+  readonly create: (input: WriteInput) => Effect.Effect<WriteResult, TargetExistsError | FSUtil.Error>
+  readonly write: (input: WriteInput) => Effect.Effect<WriteResult, FSUtil.Error>
   /** Write text while retaining an existing UTF-8 BOM and emitting at most one BOM. */
-  readonly writeTextPreservingBom: (
-    input: TextWriteInput,
-  ) => Effect.Effect<WriteResult, LocationMutation.RevalidationError | FSUtil.Error>
+  readonly writeTextPreservingBom: (input: TextWriteInput) => Effect.Effect<WriteResult, FSUtil.Error>
   /** Commit only if an existing target still has the expected bytes. */
   readonly writeIfUnchanged: (
     input: ConditionalWriteInput,
-  ) => Effect.Effect<WriteResult, StaleContentError | LocationMutation.RevalidationError | FSUtil.Error>
-  /** Remove after immediately revalidating the planned target. */
-  readonly remove: (
-    input: RemoveInput,
-  ) => Effect.Effect<RemoveResult, LocationMutation.RevalidationError | FSUtil.Error>
+  ) => Effect.Effect<WriteResult, StaleContentError | FSUtil.Error>
+  readonly remove: (input: RemoveInput) => Effect.Effect<RemoveResult, FSUtil.Error>
 }
 
 export class Service extends Context.Service<Service, Interface>()("@opencode/v2/FileMutation") {}
 
 /**
- * Commit planned file changes.
- *
- *   resolve(path) -> approve -> lock target -> revalidate(plan) -> mutate
- *
- * The caller approves the plan first. This service locks the canonical target,
- * revalidates the plan immediately before the filesystem operation, then mutates.
- *
- * `writeIfUnchanged` compares and writes while holding the same in-memory lock,
- * so cooperating calls in this process cannot overwrite from the same stale
- * content. Locks apply only within this service layer and only to identical
- * canonical targets.
- *
- * Revalidation reduces the race window but is not atomic with the next
- * path-based filesystem operation. A hostile local process can still race it.
- *
- * TODO: Use descriptor-relative no-follow operations where supported to close
- * the final race.
+ * Serialize file changes by canonical target. Conditional writes compare and
+ * write under the same process-local lock so cooperating OpenCode mutations do
+ * not overwrite changes made from the same stale content.
  */
 export const layer = Layer.effect(
   Service,
   Effect.gen(function* () {
     const fs = yield* FSUtil.Service
-    const mutation = yield* LocationMutation.Service
     const locks = KeyedMutex.makeUnsafe<string>()
     const withTargetLock =
-      (target: string) =>
+      (target: Target) =>
       <A, E, R>(effect: Effect.Effect<A, E, R>) =>
-        locks.withLock(target)(Effect.uninterruptible(effect))
+        locks.withLock(target.canonical)(Effect.uninterruptible(effect))
 
-    const withValidatedTarget =
-      (plan: LocationMutation.Plan) =>
-      <A, E, R>(commit: (target: LocationMutation.Target) => Effect.Effect<A, E, R>) =>
-        withTargetLock(plan.target.canonical)(mutation.revalidate(plan).pipe(Effect.flatMap(commit)))
-
-    const writeResult = (target: LocationMutation.Target, existed = target.exists): WriteResult => ({
+    const writeResult = (target: Target, existed: boolean): WriteResult => ({
       operation: "write",
       target: target.canonical,
       resource: target.resource,
       existed,
     })
 
-    const removeResult = (target: LocationMutation.Target): RemoveResult => ({
+    const removeResult = (target: Target, existed: boolean): RemoveResult => ({
       operation: "remove",
       target: target.canonical,
       resource: target.resource,
-      existed: target.exists,
+      existed,
     })
 
     const write = Effect.fn("FileMutation.write")((input: WriteInput) =>
-      withValidatedTarget(input.plan)((target) =>
+      withTargetLock(input.target)(
         Effect.gen(function* () {
-          yield* fs.writeWithDirs(target.canonical, input.content)
-          return writeResult(target)
+          const existed = yield* fs.exists(input.target.canonical)
+          yield* fs.writeWithDirs(input.target.canonical, input.content)
+          return writeResult(input.target, existed)
         }),
       ),
     )
 
     const writeTextPreservingBom = Effect.fn("FileMutation.writeTextPreservingBom")((input: TextWriteInput) =>
-      withValidatedTarget(input.plan)((target) =>
+      withTargetLock(input.target)(
         Effect.gen(function* () {
           const next = splitBom(input.content)
-          const preserveBom = target.exists && hasUtf8Bom(yield* fs.readFile(target.canonical))
-          yield* fs.writeWithDirs(target.canonical, joinBom(next.text, preserveBom || next.bom))
-          return writeResult(target)
+          const current = yield* fs
+            .readFile(input.target.canonical)
+            .pipe(Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(undefined)))
+          yield* fs.writeWithDirs(
+            input.target.canonical,
+            joinBom(next.text, Boolean(current && hasUtf8Bom(current)) || next.bom),
+          )
+          return writeResult(input.target, current !== undefined)
         }),
       ),
     )
 
     const create = Effect.fn("FileMutation.create")((input: WriteInput) =>
-      withValidatedTarget(input.plan)((target) =>
+      withTargetLock(input.target)(
         Effect.gen(function* () {
-          if (target.exists) return yield* new TargetExistsError({ path: target.canonical })
-          yield* fs.ensureDir(dirname(target.canonical))
-          if (typeof input.content === "string")
-            yield* fs.writeFileString(target.canonical, input.content, { flag: "wx" })
-          else yield* fs.writeFile(target.canonical, input.content, { flag: "wx" })
-          return writeResult(target, false)
+          const write =
+            typeof input.content === "string"
+              ? fs.writeFileString(input.target.canonical, input.content, { flag: "wx" })
+              : fs.writeFile(input.target.canonical, input.content, { flag: "wx" })
+          yield* write.pipe(
+            Effect.catchReason("PlatformError", "NotFound", () =>
+              fs.ensureDir(dirname(input.target.canonical)).pipe(Effect.andThen(write)),
+            ),
+            Effect.catchReason("PlatformError", "AlreadyExists", () =>
+              Effect.fail(new TargetExistsError({ path: input.target.canonical })),
+            ),
+          )
+          return writeResult(input.target, false)
         }),
       ),
     )
 
     const writeIfUnchanged = Effect.fn("FileMutation.writeIfUnchanged")((input: ConditionalWriteInput) =>
-      withValidatedTarget(input.plan)((target) =>
+      withTargetLock(input.target)(
         Effect.gen(function* () {
-          const current = yield* fs.readFile(target.canonical)
-          if (!sameBytes(current, input.expected)) return yield* new StaleContentError({ path: target.canonical })
-          yield* fs.writeWithDirs(target.canonical, input.content)
-          return writeResult(target)
+          const current = yield* fs.readFile(input.target.canonical)
+          if (!sameBytes(current, input.expected)) {
+            return yield* new StaleContentError({ path: input.target.canonical })
+          }
+          yield* typeof input.content === "string"
+            ? fs.writeFileString(input.target.canonical, input.content)
+            : fs.writeFile(input.target.canonical, input.content)
+          return writeResult(input.target, true)
         }),
       ),
     )
 
     const remove = Effect.fn("FileMutation.remove")((input: RemoveInput) =>
-      withValidatedTarget(input.plan)((target) =>
+      withTargetLock(input.target)(
         Effect.gen(function* () {
-          yield* fs.remove(target.canonical)
-          return removeResult(target)
+          const existed = yield* fs.remove(input.target.canonical).pipe(
+            Effect.as(true),
+            Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(false)),
+          )
+          return removeResult(input.target, existed)
         }),
       ),
     )

+ 3 - 3
packages/core/src/location-layer.ts

@@ -71,14 +71,14 @@ export class LocationServiceMap extends LayerMap.Service<LocationServiceMap>()("
       Layer.provide(base),
     )
     const services = Layer.mergeAll(base, resources, permissionsAndTools)
-    const commits = FileMutation.locationLayer.pipe(Layer.provide(services))
+    const mutation = FileMutation.locationLayer.pipe(Layer.provide(services))
     const searches = LocationSearch.layer.pipe(Layer.provide(Ripgrep.layer), Layer.provide(services))
     const skillGuidance = SkillGuidance.locationLayer.pipe(Layer.provide(services))
     const todos = SessionTodo.layer.pipe(Layer.provide(services))
     const questions = QuestionV2.locationLayer.pipe(Layer.provide(services))
     const builtInTools = BuiltInTools.locationLayer.pipe(
       Layer.provide(services),
-      Layer.provide(commits),
+      Layer.provide(mutation),
       Layer.provide(searches),
       Layer.provide(resources),
       Layer.provide(todos),
@@ -90,7 +90,7 @@ export class LocationServiceMap extends LayerMap.Service<LocationServiceMap>()("
       Layer.provide(model),
       Layer.provide(skillGuidance),
     )
-    return Layer.mergeAll(services, commits, searches, resources, todos, questions, model, runner, builtInTools).pipe(
+    return Layer.mergeAll(services, mutation, searches, resources, todos, questions, model, runner, builtInTools).pipe(
       Layer.fresh,
     )
   },

+ 34 - 190
packages/core/src/location-mutation.ts

@@ -1,7 +1,7 @@
 export * as LocationMutation from "./location-mutation"
 
 import path from "path"
-import { Context, Effect, Layer, Option, Schema } from "effect"
+import { Context, Effect, Layer, Schema } from "effect"
 import { FSUtil } from "./fs-util"
 import { Location } from "./location"
 
@@ -22,30 +22,9 @@ export type ResolveInput = typeof ResolveInput.Type
 
 export class PathError extends Schema.TaggedErrorClass<PathError>()("LocationMutation.PathError", {
   path: Schema.String,
-  reason: Schema.Literals([
-    "relative_escape",
-    "location_escape",
-    "non_directory_ancestor",
-    "unresolved_symlink",
-    "location_identity_changed",
-  ]),
+  reason: Schema.Literals(["relative_escape", "location_escape", "non_directory_ancestor"]),
 }) {}
 
-export class RevalidationError extends Schema.TaggedErrorClass<RevalidationError>()(
-  "LocationMutation.RevalidationError",
-  {
-    path: Schema.String,
-    reason: Schema.String,
-  },
-) {}
-
-export interface Identity {
-  /** Canonical path for this saved filesystem identity. */
-  readonly canonical: string
-  readonly dev: number
-  readonly ino?: number
-}
-
 export interface ExternalDirectoryAuthorization {
   readonly action: "external_directory"
   /** Canonical existing directory used as the external approval boundary. */
@@ -53,11 +32,8 @@ export interface ExternalDirectoryAuthorization {
   /** `external_directory` permission resource. */
   readonly resource: string
   readonly save: string
-  /** Saved identity checked again after approval to detect swaps. */
-  readonly authority: Identity
 }
 
-/** Build the `external_directory` permission request. */
 export const externalDirectoryPermission = (input: ExternalDirectoryAuthorization) => ({
   action: input.action,
   resources: [input.resource],
@@ -67,61 +43,34 @@ export const externalDirectoryPermission = (input: ExternalDirectoryAuthorizatio
 export interface Target {
   /** Canonical existing path, or missing path below a canonical directory. */
   readonly canonical: string
-  readonly exists: boolean
-  readonly type?:
-    | "File"
-    | "Directory"
-    | "SymbolicLink"
-    | "BlockDevice"
-    | "CharacterDevice"
-    | "FIFO"
-    | "Socket"
-    | "Unknown"
   /** Permission resource: Location-relative for internal paths, canonical for external paths. */
   readonly resource: string
   readonly externalDirectory?: ExternalDirectoryAuthorization
 }
 
-/**
- * A path checked before permission approval.
- *
- *   resolve(path) -> Plan -> approve -> revalidate(plan) -> mutate immediately
- *
- * Tools must approve `target.externalDirectory`, when present, and their normal
- * mutation action before calling `revalidate`. Revalidation rejects escapes,
- * symlinks in missing suffixes, and changes made while approval is pending. It
- * cannot be atomic with the next filesystem call, so mutate immediately afterward.
- */
-export interface Plan {
-  readonly input: ResolveInput
-  readonly target: Target
-  /** Saved identity of the existing target or nearest existing ancestor. */
-  readonly authority: Identity
-}
-
 export interface Interface {
   /**
-   * Check a path before approval and derive its permission resources. Relative
-   * paths must stay inside the Location. Absolute paths outside it require
-   * separate `external_directory` approval. This does not approve the tool's
-   * mutation action.
+   * Resolve a path and derive its permission resources. Relative paths must
+   * stay inside the Location. Absolute paths outside it require separate
+   * `external_directory` approval. This does not approve the mutation.
    */
-  readonly resolve: (input: ResolveInput) => Effect.Effect<Plan, PathError | FSUtil.Error>
-  /**
-   * Check the plan again immediately before mutation. Reject changes to the
-   * target, its saved identity, or approval resources. Mutate the returned
-   * target immediately.
-   */
-  readonly revalidate: (plan: Plan) => Effect.Effect<Target, RevalidationError | FSUtil.Error>
+  readonly resolve: (input: ResolveInput) => Effect.Effect<Target, PathError | FSUtil.Error>
 }
 
 export class Service extends Context.Service<Service, Interface>()("@opencode/v2/LocationMutation") {}
 
 interface ResolvedPath {
   readonly canonical: string
-  readonly exists: boolean
-  readonly type?: Target["type"]
-  readonly authority: Identity
+  readonly type?:
+    | "File"
+    | "Directory"
+    | "SymbolicLink"
+    | "BlockDevice"
+    | "CharacterDevice"
+    | "FIFO"
+    | "Socket"
+    | "Unknown"
+  readonly directory: string
 }
 
 const slash = (value: string) => value.replaceAll("\\", "/")
@@ -132,76 +81,19 @@ export const layer = Layer.effect(
     const fs = yield* FSUtil.Service
     const location = yield* Location.Service
     const locationRoot = yield* fs.realPath(location.directory)
-    const locationAuthority = yield* identity(locationRoot)
-
-    function identityFrom(canonical: string, info: Effect.Success<ReturnType<typeof fs.stat>>): Identity {
-      return {
-        canonical,
-        dev: info.dev,
-        ino: Option.getOrUndefined(info.ino),
-      }
-    }
-
-    function identity(canonical: string) {
-      return fs.stat(canonical).pipe(Effect.map((info) => identityFrom(canonical, info)))
-    }
 
     function notFound<A>(effect: Effect.Effect<A, FSUtil.Error>) {
       return effect.pipe(Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(undefined)))
     }
 
-    function sameIdentity(left: Identity, right: Identity) {
-      return left.canonical === right.canonical && left.dev === right.dev && left.ino === right.ino
-    }
-
-    /** Check whether a saved path still points to the same filesystem object. */
-    const assertIdentity = Effect.fnUntraced(function* (expected: Identity) {
-      const canonical = yield* notFound(fs.realPath(expected.canonical))
-      if (canonical === undefined) return false
-      const actual = yield* notFound(identity(canonical))
-      if (actual === undefined) return false
-      return canonical === expected.canonical && sameIdentity(expected, actual)
-    })
-
-    const assertLocationIdentity = Effect.fnUntraced(function* (requested: string) {
-      if (yield* assertIdentity(locationAuthority)) return
-      return yield* new PathError({ path: requested, reason: "location_identity_changed" })
-    })
-
-    const hasUnresolvedSymlink = Effect.fnUntraced(function* (anchor: string, suffix: string) {
-      let current = anchor
-      for (const part of suffix.split(path.sep)) {
-        if (!part) continue
-        current = path.join(current, part)
-        if (
-          yield* fs.readLink(current).pipe(
-            Effect.as(true),
-            Effect.catch(() => Effect.succeed(false)),
-          )
-        )
-          return true
-      }
-      return false
-    })
-
-    /**
-     * Resolve a path to a canonical target and save an existing filesystem
-     * identity for later revalidation.
-     *
-     *   existing path -> save target identity
-     *   missing path  -> save nearest existing directory identity
-     *
-     * Missing suffixes must not contain symlinks.
-     */
     const resolvePath = Effect.fnUntraced(function* (absolute: string) {
       const existing = yield* notFound(fs.realPath(absolute))
       if (existing !== undefined) {
         const info = yield* fs.stat(existing)
         return {
           canonical: existing,
-          exists: true,
           type: info.type,
-          authority: identityFrom(existing, info),
+          directory: info.type === "Directory" ? existing : path.dirname(existing),
         } satisfies ResolvedPath
       }
 
@@ -210,16 +102,12 @@ export const layer = Layer.effect(
         const canonical = yield* notFound(fs.realPath(anchor))
         if (canonical !== undefined) {
           const info = yield* fs.stat(canonical)
-          if (info.type !== "Directory")
+          if (info.type !== "Directory") {
             return yield* new PathError({ path: absolute, reason: "non_directory_ancestor" })
-          const suffix = path.relative(anchor, absolute)
-          if (yield* hasUnresolvedSymlink(anchor, suffix)) {
-            return yield* new PathError({ path: absolute, reason: "unresolved_symlink" })
           }
           return {
-            canonical: path.resolve(canonical, suffix),
-            exists: false,
-            authority: identityFrom(canonical, info),
+            canonical: path.resolve(canonical, path.relative(anchor, absolute)),
+            directory: canonical,
           } satisfies ResolvedPath
         }
         const parent = path.dirname(anchor)
@@ -228,30 +116,7 @@ export const layer = Layer.effect(
       }
     })
 
-    /**
-     * Choose the existing directory used for separate external approval.
-     *
-     *   existing directory target -> "<target>/*"
-     *   file or missing target    -> "<nearest existing parent>/*"
-     */
-    const externalDirectory = Effect.fnUntraced(function* (resolved: ResolvedPath, kind: Kind) {
-      const candidate =
-        kind === "directory" && resolved.type === "Directory" ? resolved.canonical : path.dirname(resolved.canonical)
-      const boundary = yield* resolvePath(candidate)
-      const directory =
-        boundary.exists && boundary.type === "Directory" ? boundary.canonical : boundary.authority.canonical
-      const resource = slash(path.join(directory, "*"))
-      return {
-        action: "external_directory" as const,
-        directory,
-        resource,
-        save: resource,
-        authority: boundary.authority,
-      }
-    })
-
     const resolve = Effect.fn("LocationMutation.resolve")(function* (input: ResolveInput) {
-      yield* assertLocationIdentity(input.path)
       const relative = !path.isAbsolute(input.path)
       const absolute = path.resolve(location.directory, input.path)
       const lexicallyInternal = FSUtil.contains(location.directory, absolute)
@@ -266,45 +131,24 @@ export const layer = Layer.effect(
       const resource = external
         ? slash(resolved.canonical)
         : slash(path.relative(locationRoot, resolved.canonical) || ".")
-      const target: Target = {
+      const externalDirectory =
+        input.kind === "directory" && resolved.type === "Directory" ? resolved.canonical : resolved.directory
+      const externalResource = slash(path.join(externalDirectory, "*"))
+      return {
         canonical: resolved.canonical,
-        exists: resolved.exists,
-        type: resolved.type,
         resource,
-        externalDirectory: external ? yield* externalDirectory(resolved, input.kind ?? "file") : undefined,
-      }
-      return { input, target, authority: resolved.authority } satisfies Plan
-    })
-
-    /**
-     * Re-resolve a plan immediately before mutation and reject any changed
-     * identity, target, or approval resource. This reduces the race window but
-     * cannot make the next filesystem call atomic.
-     */
-    const revalidate = Effect.fn("LocationMutation.revalidate")(function* (plan: Plan) {
-      const invalid = (reason: string) => new RevalidationError({ path: plan.input.path, reason })
-      const fresh = yield* resolve(plan.input).pipe(
-        Effect.mapError((error) => (error instanceof PathError ? invalid(error.reason) : error)),
-      )
-      if (!sameIdentity(fresh.authority, plan.authority)) return yield* invalid("mutation authority changed")
-      if (fresh.target.canonical !== plan.target.canonical) return yield* invalid("canonical mutation target changed")
-      if (fresh.target.resource !== plan.target.resource) return yield* invalid("mutation resource changed")
-      if (Boolean(fresh.target.externalDirectory) !== Boolean(plan.target.externalDirectory)) {
-        return yield* invalid("external directory authority changed")
-      }
-      if (
-        fresh.target.externalDirectory &&
-        plan.target.externalDirectory &&
-        (fresh.target.externalDirectory.directory !== plan.target.externalDirectory.directory ||
-          fresh.target.externalDirectory.resource !== plan.target.externalDirectory.resource ||
-          !sameIdentity(fresh.target.externalDirectory.authority, plan.target.externalDirectory.authority))
-      ) {
-        return yield* invalid("external directory authority changed")
-      }
-      return fresh.target
+        externalDirectory: external
+          ? {
+              action: "external_directory",
+              directory: externalDirectory,
+              resource: externalResource,
+              save: externalResource,
+            }
+          : undefined,
+      } satisfies Target
     })
 
-    return Service.of({ resolve, revalidate })
+    return Service.of({ resolve })
   }),
 )
 

+ 60 - 70
packages/core/src/tool/apply-patch.ts

@@ -41,25 +41,13 @@ const definition = Tool.make({
   toModelOutput: ({ output }) => [toolText({ type: "text", text: toModelOutput(output) })],
 })
 
-type Planned = { readonly hunk: Patch.Hunk; readonly plan: LocationMutation.Plan }
 type Prepared =
-  | {
-      readonly type: "add"
-      readonly hunk: Extract<Patch.Hunk, { readonly type: "add" }>
-      readonly plan: LocationMutation.Plan
-    }
-  | {
-      readonly type: "delete"
-      readonly hunk: Extract<Patch.Hunk, { readonly type: "delete" }>
-      readonly plan: LocationMutation.Plan
-    }
-  | {
-      readonly type: "update"
-      readonly hunk: Extract<Patch.Hunk, { readonly type: "update" }>
-      readonly plan: LocationMutation.Plan
+  | (Extract<Patch.Hunk, { readonly type: "add" | "delete" }> & { readonly target: LocationMutation.Target })
+  | (Extract<Patch.Hunk, { readonly type: "update" }> & {
+      readonly target: LocationMutation.Target
       readonly source: Uint8Array
       readonly content: string
-    }
+    })
 
 export const layer = Layer.effectDiscard(
   Effect.gen(function* () {
@@ -90,12 +78,12 @@ export const layer = Layer.effectDiscard(
             const move = hunks.find((hunk) => hunk.type === "update" && hunk.movePath !== undefined)
             if (move) return yield* new ToolFailure({ message: "apply_patch moves are not supported yet" })
 
-            const planned: Planned[] = []
+            const targets: Array<{ readonly hunk: Patch.Hunk; readonly target: LocationMutation.Target }> = []
             for (const hunk of hunks)
-              planned.push({ hunk, plan: yield* mutation.resolve({ path: hunk.path, kind: "file" }) })
+              targets.push({ hunk, target: yield* mutation.resolve({ path: hunk.path, kind: "file" }) })
             const externalDirectories = new Map<string, LocationMutation.ExternalDirectoryAuthorization>()
-            for (const { plan } of planned) {
-              const external = plan.target.externalDirectory
+            for (const { target } of targets) {
+              const external = target.externalDirectory
               if (external) externalDirectories.set(external.resource, external)
             }
             for (const external of externalDirectories.values()) {
@@ -103,64 +91,66 @@ export const layer = Layer.effectDiscard(
             }
             yield* assertPermission({
               action: "edit",
-              resources: [...new Set(planned.map(({ plan }) => plan.target.resource))],
+              resources: [...new Set(targets.map(({ target }) => target.resource))],
               save: ["*"],
             })
 
             const prepared: Prepared[] = []
-            for (const { hunk, plan } of planned) {
-              if (hunk.type === "add") {
-                const target = yield* mutation.revalidate(plan)
-                if (target.exists) return yield* fail(hunk.path, new Error("Target file already exists"))
-                prepared.push({ type: hunk.type, hunk, plan })
-                continue
-              }
-              const target = yield* mutation.revalidate(plan)
-              if (!target.exists || target.type !== "File")
-                return yield* fail(hunk.path, new Error("Target file does not exist"))
-              if (hunk.type === "delete") {
-                prepared.push({ type: hunk.type, hunk, plan })
-                continue
-              }
-              const source = yield* fs.readFile(target.canonical)
-              const update = Patch.derive(
-                hunk.path,
-                hunk.chunks,
-                new TextDecoder("utf-8", { ignoreBOM: true }).decode(source),
-              )
-              prepared.push({ type: hunk.type, hunk, plan, source, content: Patch.joinBom(update.content, update.bom) })
+            for (const { hunk, target } of targets) {
+              yield* Effect.gen(function* () {
+                if (hunk.type === "add") {
+                  prepared.push({ ...hunk, target })
+                  return
+                }
+                if ((yield* fs.stat(target.canonical)).type !== "File")
+                  yield* fail(hunk.path, new Error("Target file does not exist"))
+                if (hunk.type === "delete") {
+                  prepared.push({ ...hunk, target })
+                  return
+                }
+                const source = yield* fs.readFile(target.canonical)
+                const update = Patch.derive(
+                  hunk.path,
+                  hunk.chunks,
+                  new TextDecoder("utf-8", { ignoreBOM: true }).decode(source),
+                )
+                prepared.push({
+                  ...hunk,
+                  target,
+                  source,
+                  content: Patch.joinBom(update.content, update.bom),
+                })
+              }).pipe(Effect.catchCause((cause) => Effect.fail(fail(hunk.path, Cause.squash(cause)))))
             }
 
-            yield* Effect.uninterruptible(
-              Effect.forEach(
-                prepared,
-                (change) =>
-                  Effect.gen(function* () {
-                    if (change.type === "add") {
-                      const result = yield* files.create({
-                        plan: change.plan,
-                        content:
-                          change.hunk.contents.endsWith("\n") || change.hunk.contents === ""
-                            ? change.hunk.contents
-                            : `${change.hunk.contents}\n`,
-                      })
-                      applied.push({ type: change.type, resource: result.resource, target: result.target })
-                      return
-                    }
-                    if (change.type === "delete") {
-                      const result = yield* files.remove({ plan: change.plan })
-                      applied.push({ type: change.type, resource: result.resource, target: result.target })
-                      return
-                    }
-                    const result = yield* files.writeIfUnchanged({
-                      plan: change.plan,
-                      expected: change.source,
-                      content: change.content,
+            yield* Effect.forEach(
+              prepared,
+              (change) =>
+                Effect.gen(function* () {
+                  if (change.type === "add") {
+                    const result = yield* files.create({
+                      target: change.target,
+                      content:
+                        change.contents.endsWith("\n") || change.contents === ""
+                          ? change.contents
+                          : `${change.contents}\n`,
                     })
                     applied.push({ type: change.type, resource: result.resource, target: result.target })
-                  }).pipe(Effect.catchCause((cause) => Effect.fail(fail(change.hunk.path, Cause.squash(cause))))),
-                { discard: true },
-              ),
+                    return
+                  }
+                  if (change.type === "delete") {
+                    const result = yield* files.remove({ target: change.target })
+                    applied.push({ type: change.type, resource: result.resource, target: result.target })
+                    return
+                  }
+                  const result = yield* files.writeIfUnchanged({
+                    target: change.target,
+                    expected: change.source,
+                    content: change.content,
+                  })
+                  applied.push({ type: change.type, resource: result.resource, target: result.target })
+                }).pipe(Effect.catchCause((cause) => Effect.fail(fail(change.path, Cause.squash(cause))))),
+              { discard: true },
             )
             return { applied }
           }).pipe(

+ 5 - 5
packages/core/src/tool/bash.ts

@@ -114,6 +114,7 @@ export const layer = Layer.effectDiscard(
   Effect.gen(function* () {
     const registry = yield* ToolRegistry.Service
     const mutation = yield* LocationMutation.Service
+    const fs = yield* FSUtil.Service
     const appProcess = yield* AppProcess.Service
     const resources = yield* ToolOutputStore.Service
     const config = yield* Config.Service
@@ -124,17 +125,16 @@ export const layer = Layer.effectDiscard(
         outputPaths: (output) => (output.outputPath ? [output.outputPath] : []),
         execute: ({ parameters, sessionID, call, assertPermission }) =>
           Effect.gen(function* () {
-            const plan = yield* mutation.resolve({ path: parameters.workdir ?? ".", kind: "directory" })
-            const external = plan.target.externalDirectory
+            const target = yield* mutation.resolve({ path: parameters.workdir ?? ".", kind: "directory" })
+            const external = target.externalDirectory
             if (external) yield* assertPermission(LocationMutation.externalDirectoryPermission(external))
-            const warnings = externalCommandDirectories(parameters.command, plan.target.canonical).map(
+            const warnings = externalCommandDirectories(parameters.command, target.canonical).map(
               (directory) =>
                 `Command argument references external directory ${path.join(directory, "*").replaceAll("\\", "/")}. Bash runs with host-user filesystem, process, and network authority; this scan is advisory only.`,
             )
             yield* assertPermission({ action: name, resources: [parameters.command], save: [parameters.command] })
 
-            const target = yield* mutation.revalidate(plan)
-            if (!target.exists || target.type !== "Directory")
+            if ((yield* fs.stat(target.canonical)).type !== "Directory")
               throw new Error(`Working directory is not a directory: ${target.canonical}`)
 
             const entries = yield* config.entries()

+ 5 - 6
packages/core/src/tool/edit.ts

@@ -130,15 +130,14 @@ export const layer = Layer.effectDiscard(
               })
             }
 
-            const plan = yield* unableToEdit(mutation.resolve({ path: parameters.path, kind: "file" }))
-            const external = plan.target.externalDirectory
+            const target = yield* unableToEdit(mutation.resolve({ path: parameters.path, kind: "file" }))
+            const external = target.externalDirectory
             if (external) {
               yield* unableToEdit(assertPermission(LocationMutation.externalDirectoryPermission(external)))
             }
 
-            yield* unableToEdit(assertPermission({ action: "edit", resources: [plan.target.resource], save: ["*"] }))
-            const readable = yield* unableToEdit(mutation.revalidate(plan))
-            const source = decodeUtf8(yield* unableToEdit(fs.readFile(readable.canonical)))
+            yield* unableToEdit(assertPermission({ action: "edit", resources: [target.resource], save: ["*"] }))
+            const source = decodeUtf8(yield* unableToEdit(fs.readFile(target.canonical)))
             const ending = detectLineEnding(source.text)
             const oldString = convertToLineEnding(parameters.oldString, ending)
             const newString = convertToLineEnding(parameters.newString, ending)
@@ -163,7 +162,7 @@ export const layer = Layer.effectDiscard(
             const next = splitBom(replaced)
             const result = yield* unableToEdit(
               files.writeIfUnchanged({
-                plan,
+                target,
                 expected: source.content,
                 content: joinBom(next.text, source.bom || next.bom),
               }),

+ 4 - 4
packages/core/src/tool/write.ts

@@ -60,11 +60,11 @@ export const layer = Layer.effectDiscard(
         tool: definition,
         execute: ({ parameters, assertPermission }) =>
           Effect.gen(function* () {
-            const plan = yield* mutation.resolve({ path: parameters.path, kind: "file" })
-            const external = plan.target.externalDirectory
+            const target = yield* mutation.resolve({ path: parameters.path, kind: "file" })
+            const external = target.externalDirectory
             if (external) yield* assertPermission(LocationMutation.externalDirectoryPermission(external))
-            yield* assertPermission({ action: "edit", resources: [plan.target.resource], save: ["*"] })
-            return yield* files.writeTextPreservingBom({ plan, content: parameters.content })
+            yield* assertPermission({ action: "edit", resources: [target.resource], save: ["*"] })
+            return yield* files.writeTextPreservingBom({ target, content: parameters.content })
           }).pipe(
             Effect.catchCause((cause) =>
               Effect.fail(

+ 74 - 68
packages/core/test/file-mutation.test.ts

@@ -16,9 +16,9 @@ function provide(directory: string, filesystem = FSUtil.defaultLayer) {
     Location.Service,
     Location.Service.of(location({ directory: AbsolutePath.make(directory) })),
   )
-  const planning = LocationMutation.layer.pipe(Layer.provide(filesystem), Layer.provide(activeLocation))
-  const commits = FileMutation.layer.pipe(Layer.provide(filesystem), Layer.provide(planning))
-  return Effect.provide(Layer.mergeAll(planning, commits))
+  const resolution = LocationMutation.layer.pipe(Layer.provide(filesystem), Layer.provide(activeLocation))
+  const mutation = FileMutation.layer.pipe(Layer.provide(filesystem))
+  return Effect.provide(Layer.mergeAll(resolution, mutation))
 }
 
 function withTmp<A, E, R>(f: (directory: string) => Effect.Effect<A, E, R>) {
@@ -34,11 +34,11 @@ describe("FileMutation", () => {
       Effect.gen(function* () {
         const targetPath = path.join(directory, "hello.txt")
         yield* Effect.promise(() => fs.writeFile(targetPath, "before"))
-        const plan = yield* (yield* LocationMutation.Service).resolve({ path: "hello.txt" })
+        const target = yield* (yield* LocationMutation.Service).resolve({ path: "hello.txt" })
 
-        expect(yield* (yield* FileMutation.Service).write({ plan, content: "after" })).toEqual({
+        expect(yield* (yield* FileMutation.Service).write({ target, content: "after" })).toEqual({
           operation: "write",
-          target: plan.target.canonical,
+          target: target.canonical,
           resource: "hello.txt",
           existed: true,
         })
@@ -50,12 +50,14 @@ describe("FileMutation", () => {
   it.live("writes a prospective internal file and creates parent directories", () =>
     withTmp((directory) =>
       Effect.gen(function* () {
-        const plan = yield* (yield* LocationMutation.Service).resolve({ path: path.join("src", "nested", "hello.txt") })
-        const result = yield* (yield* FileMutation.Service).write({ plan, content: "hello" })
+        const target = yield* (yield* LocationMutation.Service).resolve({
+          path: path.join("src", "nested", "hello.txt"),
+        })
+        const result = yield* (yield* FileMutation.Service).write({ target, content: "hello" })
 
         expect(result).toEqual({
           operation: "write",
-          target: plan.target.canonical,
+          target: target.canonical,
           resource: "src/nested/hello.txt",
           existed: false,
         })
@@ -73,43 +75,62 @@ describe("FileMutation", () => {
         const created = yield* (yield* LocationMutation.Service).resolve({ path: "created.txt" })
         const files = yield* FileMutation.Service
 
-        yield* files.writeTextPreservingBom({ plan: preserved, content: "\uFEFFafter" })
-        yield* files.writeTextPreservingBom({ plan: created, content: "\uFEFF\uFEFF\uFEFFcreated" })
+        yield* files.writeTextPreservingBom({ target: preserved, content: "\uFEFFafter" })
+        yield* files.writeTextPreservingBom({ target: created, content: "\uFEFF\uFEFF\uFEFFcreated" })
 
         expect(yield* Effect.promise(() => fs.readFile(preservedPath, "utf8"))).toBe("\uFEFFafter")
-        expect(yield* Effect.promise(() => fs.readFile(created.target.canonical, "utf8"))).toBe("\uFEFFcreated")
+        expect(yield* Effect.promise(() => fs.readFile(created.canonical, "utf8"))).toBe("\uFEFFcreated")
       }).pipe(provide(directory)),
     ),
   )
 
-  it.live("rejects create when a prospective target appears after planning", () =>
+  it.live("rejects create when a prospective target appears after resolution", () =>
     withTmp((directory) =>
       Effect.gen(function* () {
         const targetPath = path.join(directory, "appeared.txt")
-        const plan = yield* (yield* LocationMutation.Service).resolve({ path: "appeared.txt" })
+        const target = yield* (yield* LocationMutation.Service).resolve({ path: "appeared.txt" })
         yield* Effect.promise(() => fs.writeFile(targetPath, "winner"))
 
         expect(
-          yield* (yield* FileMutation.Service).create({ plan, content: "replacement" }).pipe(Effect.flip),
+          yield* (yield* FileMutation.Service).create({ target, content: "replacement" }).pipe(Effect.flip),
         ).toMatchObject({
-          _tag: "LocationMutation.RevalidationError",
+          _tag: "FileMutation.TargetExistsError",
         })
         expect(yield* Effect.promise(() => fs.readFile(targetPath, "utf8"))).toBe("winner")
       }).pipe(provide(directory)),
     ),
   )
 
+  it.live("creates when an existing target disappears after resolution", () =>
+    withTmp((directory) =>
+      Effect.gen(function* () {
+        const targetPath = path.join(directory, "removed.txt")
+        yield* Effect.promise(() => fs.writeFile(targetPath, "before"))
+        const target = yield* (yield* LocationMutation.Service).resolve({ path: "removed.txt" })
+        yield* Effect.promise(() => fs.rm(targetPath))
+
+        expect(yield* (yield* FileMutation.Service).create({ target, content: "after" })).toEqual({
+          operation: "write",
+          target: target.canonical,
+          resource: "removed.txt",
+          existed: false,
+        })
+        expect(yield* Effect.promise(() => fs.readFile(targetPath, "utf8"))).toBe("after")
+      }).pipe(provide(directory)),
+    ),
+  )
+
   it.live("removes an existing internal file", () =>
     withTmp((directory) =>
       Effect.gen(function* () {
         const targetPath = path.join(directory, "remove.txt")
         yield* Effect.promise(() => fs.writeFile(targetPath, "remove"))
-        const plan = yield* (yield* LocationMutation.Service).resolve({ path: "remove.txt" })
-        const result = yield* (yield* FileMutation.Service).remove({ plan })
+        const target = yield* (yield* LocationMutation.Service).resolve({ path: "remove.txt" })
+        const result = yield* (yield* FileMutation.Service).remove({ target })
 
         expect(result).toEqual({
           operation: "remove",
-          target: plan.target.canonical,
+          target: target.canonical,
           resource: "remove.txt",
           existed: true,
         })
@@ -125,18 +146,18 @@ describe("FileMutation", () => {
     ),
   )
 
-  it.live("writes an explicitly planned external target", () =>
+  it.live("writes an explicitly resolved external target", () =>
     withTmp((directory) =>
       withTmp((outside) =>
         Effect.gen(function* () {
           const targetPath = path.join(outside, "external.txt")
-          const plan = yield* (yield* LocationMutation.Service).resolve({ path: targetPath })
-          const result = yield* (yield* FileMutation.Service).write({ plan, content: "external" })
+          const target = yield* (yield* LocationMutation.Service).resolve({ path: targetPath })
+          const result = yield* (yield* FileMutation.Service).write({ target, content: "external" })
 
           expect(result).toEqual({
             operation: "write",
-            target: plan.target.canonical,
-            resource: plan.target.resource,
+            target: target.canonical,
+            resource: target.resource,
             existed: false,
           })
           expect(yield* Effect.promise(() => fs.readFile(targetPath, "utf8"))).toBe("external")
@@ -145,19 +166,19 @@ describe("FileMutation", () => {
     ),
   )
 
-  it.live("removes an explicitly planned external target", () =>
+  it.live("removes an explicitly resolved external target", () =>
     withTmp((directory) =>
       withTmp((outside) =>
         Effect.gen(function* () {
           const targetPath = path.join(outside, "external.txt")
           yield* Effect.promise(() => fs.writeFile(targetPath, "external"))
-          const plan = yield* (yield* LocationMutation.Service).resolve({ path: targetPath })
-          const result = yield* (yield* FileMutation.Service).remove({ plan })
+          const target = yield* (yield* LocationMutation.Service).resolve({ path: targetPath })
+          const result = yield* (yield* FileMutation.Service).remove({ target })
 
           expect(result).toEqual({
             operation: "remove",
-            target: plan.target.canonical,
-            resource: plan.target.resource,
+            target: target.canonical,
+            resource: target.resource,
             existed: true,
           })
           expect(
@@ -173,34 +194,18 @@ describe("FileMutation", () => {
     ),
   )
 
-  it.live("propagates revalidation rejection after an ancestor swap", () =>
+  it.live("reports a missing target as not removed without checking existence first", () =>
     withTmp((directory) =>
-      withTmp((outside) =>
-        Effect.gen(function* () {
-          if (process.platform === "win32") return
-          const parent = path.join(directory, "parent")
-          yield* Effect.promise(() => fs.mkdir(parent))
-          const plan = yield* (yield* LocationMutation.Service).resolve({ path: path.join("parent", "new.txt") })
-          yield* Effect.promise(async () => {
-            await fs.rmdir(parent)
-            await fs.symlink(outside, parent)
-          })
+      Effect.gen(function* () {
+        const target = yield* (yield* LocationMutation.Service).resolve({ path: "missing.txt" })
 
-          expect(
-            yield* (yield* FileMutation.Service).write({ plan, content: "escape" }).pipe(Effect.flip),
-          ).toMatchObject({
-            _tag: "LocationMutation.RevalidationError",
-          })
-          expect(
-            yield* Effect.promise(() =>
-              fs.stat(path.join(outside, "new.txt")).then(
-                () => true,
-                () => false,
-              ),
-            ),
-          ).toBe(false)
-        }).pipe(provide(directory)),
-      ),
+        expect(yield* (yield* FileMutation.Service).remove({ target })).toEqual({
+          operation: "remove",
+          target: target.canonical,
+          resource: "missing.txt",
+          existed: false,
+        })
+      }).pipe(provide(directory)),
     ),
   )
 
@@ -231,9 +236,9 @@ describe("FileMutation", () => {
           const files = yield* FileMutation.Service
           const firstPlan = yield* mutation.resolve({ path: "shared.txt" })
           const secondPlan = yield* mutation.resolve({ path: "shared.txt" })
-          const first = yield* files.write({ plan: firstPlan, content: "first" }).pipe(Effect.forkChild)
+          const first = yield* files.write({ target: firstPlan, content: "first" }).pipe(Effect.forkChild)
           yield* Deferred.await(firstStarted)
-          const second = yield* files.write({ plan: secondPlan, content: "second" }).pipe(Effect.forkChild)
+          const second = yield* files.write({ target: secondPlan, content: "second" }).pipe(Effect.forkChild)
           yield* Effect.yieldNow
           expect(yield* Deferred.isDone(secondStarted)).toBe(false)
 
@@ -269,12 +274,12 @@ describe("FileMutation", () => {
         yield* Effect.gen(function* () {
           const mutation = yield* LocationMutation.Service
           const files = yield* FileMutation.Service
-          const plan = yield* mutation.resolve({ path: "shared.txt" })
+          const target = yield* mutation.resolve({ path: "shared.txt" })
           const expected = new TextEncoder().encode("initial")
-          const first = yield* files.writeIfUnchanged({ plan, expected, content: "first" }).pipe(Effect.forkChild)
+          const first = yield* files.writeIfUnchanged({ target, expected, content: "first" }).pipe(Effect.forkChild)
           yield* Deferred.await(firstStarted)
           const second = yield* files
-            .writeIfUnchanged({ plan, expected, content: "second" })
+            .writeIfUnchanged({ target, expected, content: "second" })
             .pipe(Effect.flip, Effect.forkChild)
 
           yield* Deferred.succeed(releaseFirst, undefined)
@@ -292,13 +297,13 @@ describe("FileMutation", () => {
       Effect.gen(function* () {
         const targetPath = path.join(directory, "stale.txt")
         yield* Effect.promise(() => fs.writeFile(targetPath, "current"))
-        const plan = yield* (yield* LocationMutation.Service).resolve({ path: "stale.txt" })
+        const target = yield* (yield* LocationMutation.Service).resolve({ path: "stale.txt" })
 
         expect(
           yield* (yield* FileMutation.Service)
-            .writeIfUnchanged({ plan, expected: new TextEncoder().encode("older"), content: "replacement" })
+            .writeIfUnchanged({ target, expected: new TextEncoder().encode("older"), content: "replacement" })
             .pipe(Effect.flip),
-        ).toMatchObject({ _tag: "FileMutation.StaleContentError", path: plan.target.canonical })
+        ).toMatchObject({ _tag: "FileMutation.StaleContentError", path: target.canonical })
         expect(yield* Effect.promise(() => fs.readFile(targetPath, "utf8"))).toBe("current")
       }).pipe(provide(directory)),
     ),
@@ -326,9 +331,9 @@ describe("FileMutation", () => {
           const files = yield* FileMutation.Service
           const firstPlan = yield* mutation.resolve({ path: "first.txt" })
           const secondPlan = yield* mutation.resolve({ path: "second.txt" })
-          const first = yield* files.write({ plan: firstPlan, content: "first" }).pipe(Effect.forkChild)
+          const first = yield* files.write({ target: firstPlan, content: "first" }).pipe(Effect.forkChild)
           yield* Deferred.await(firstStarted)
-          const second = yield* files.write({ plan: secondPlan, content: "second" }).pipe(Effect.forkChild)
+          const second = yield* files.write({ target: secondPlan, content: "second" }).pipe(Effect.forkChild)
           yield* Deferred.await(secondFinished)
           expect(yield* Effect.promise(() => fs.readFile(secondPath, "utf8"))).toBe("second")
 
@@ -341,9 +346,7 @@ describe("FileMutation", () => {
   )
 })
 
-function instrumentWrites(
-  run: (write: Effect.Effect<void, FSUtil.Error>, target: string) => Effect.Effect<void, FSUtil.Error>,
-) {
+function instrumentWrites(run: <E>(write: Effect.Effect<void, E>, target: string) => Effect.Effect<void, E>) {
   return Layer.effect(
     FSUtil.Service,
     Effect.gen(function* () {
@@ -351,6 +354,9 @@ function instrumentWrites(
       return FSUtil.Service.of({
         ...filesystem,
         writeWithDirs: (target, content, mode) => run(filesystem.writeWithDirs(target, content, mode), target),
+        writeFile: (target, content, options) => run(filesystem.writeFile(target, content, options), target),
+        writeFileString: (target, content, options) =>
+          run(filesystem.writeFileString(target, content, options), target),
       })
     }),
   ).pipe(Layer.provide(FSUtil.defaultLayer))

+ 33 - 87
packages/core/test/location-mutation.test.ts

@@ -36,17 +36,13 @@ describe("LocationMutation", () => {
       Effect.gen(function* () {
         const targetPath = path.join(directory, "hello.txt")
         yield* Effect.promise(() => fs.writeFile(targetPath, "hello"))
-        const plan = yield* (yield* LocationMutation.Service).resolve({ path: "hello.txt" })
+        const target = yield* (yield* LocationMutation.Service).resolve({ path: "hello.txt" })
 
-        expect(plan.target).toMatchObject({
+        expect(target).toMatchObject({
           canonical: yield* Effect.promise(() => fs.realpath(targetPath)),
-          exists: true,
           resource: "hello.txt",
         })
-        expect(plan.target.externalDirectory).toBeUndefined()
-        expect(yield* (yield* LocationMutation.Service).revalidate(plan)).toMatchObject({
-          canonical: plan.target.canonical,
-        })
+        expect(target.externalDirectory).toBeUndefined()
       }).pipe(provide(directory)),
     ),
   )
@@ -55,18 +51,13 @@ describe("LocationMutation", () => {
     withTmp((directory) =>
       Effect.gen(function* () {
         yield* Effect.promise(() => fs.mkdir(path.join(directory, "src")))
-        const plan = yield* (yield* LocationMutation.Service).resolve({ path: path.join("src", "new.txt") })
+        const target = yield* (yield* LocationMutation.Service).resolve({ path: path.join("src", "new.txt") })
         const root = yield* Effect.promise(() => fs.realpath(directory))
 
-        expect(plan.target).toMatchObject({
+        expect(target).toMatchObject({
           canonical: path.join(root, "src", "new.txt"),
-          exists: false,
           resource: "src/new.txt",
         })
-        expect(plan.authority.canonical).toBe(path.join(root, "src"))
-        expect(yield* (yield* LocationMutation.Service).revalidate(plan)).toMatchObject({
-          canonical: plan.target.canonical,
-        })
       }).pipe(provide(directory)),
     ),
   )
@@ -98,16 +89,33 @@ describe("LocationMutation", () => {
     }),
   )
 
+  it.live("follows an in-location symlink using ordinary filesystem semantics", () =>
+    withTmp((directory) =>
+      Effect.gen(function* () {
+        if (process.platform === "win32") return
+        yield* Effect.promise(async () => {
+          await fs.mkdir(path.join(directory, "actual"))
+          await fs.symlink(path.join(directory, "actual"), path.join(directory, "linked"))
+        })
+
+        expect(yield* (yield* LocationMutation.Service).resolve({ path: "linked/new.txt" })).toMatchObject({
+          canonical: path.join(yield* Effect.promise(() => fs.realpath(directory)), "actual", "new.txt"),
+          resource: "actual/new.txt",
+        })
+      }).pipe(provide(directory)),
+    ),
+  )
+
   it.live("accepts an explicit absolute in-location target without external approval", () =>
     withTmp((directory) =>
       Effect.gen(function* () {
         const targetPath = path.join(directory, "new.txt")
-        const plan = yield* (yield* LocationMutation.Service).resolve({ path: targetPath })
-        expect(plan.target).toMatchObject({
+        const target = yield* (yield* LocationMutation.Service).resolve({ path: targetPath })
+        expect(target).toMatchObject({
           canonical: path.join(yield* Effect.promise(() => fs.realpath(directory)), "new.txt"),
           resource: "new.txt",
         })
-        expect(plan.target.externalDirectory).toBeUndefined()
+        expect(target.externalDirectory).toBeUndefined()
       }).pipe(provide(directory)),
     ),
   )
@@ -117,13 +125,13 @@ describe("LocationMutation", () => {
       withTmp((outside) =>
         Effect.gen(function* () {
           const targetPath = path.join(outside, "new.txt")
-          const plan = yield* (yield* LocationMutation.Service).resolve({ path: targetPath })
+          const target = yield* (yield* LocationMutation.Service).resolve({ path: targetPath })
           const root = yield* Effect.promise(() => fs.realpath(outside))
-          expect(plan.target).toMatchObject({
+          expect(target).toMatchObject({
             canonical: path.join(root, "new.txt"),
             resource: path.join(root, "new.txt").replaceAll("\\", "/"),
           })
-          expect(plan.target.externalDirectory).toMatchObject({
+          expect(target.externalDirectory).toMatchObject({
             directory: root,
             resource: path.join(root, "*").replaceAll("\\", "/"),
           })
@@ -138,11 +146,10 @@ describe("LocationMutation", () => {
         Effect.gen(function* () {
           const targetPath = path.join(outside, "existing.txt")
           yield* Effect.promise(() => fs.writeFile(targetPath, "existing"))
-          const plan = yield* (yield* LocationMutation.Service).resolve({ path: targetPath })
+          const target = yield* (yield* LocationMutation.Service).resolve({ path: targetPath })
           const root = yield* Effect.promise(() => fs.realpath(outside))
-          expect(plan.target).toMatchObject({ canonical: path.join(root, "existing.txt"), exists: true })
-          expect(plan.authority.canonical).toBe(path.join(root, "existing.txt"))
-          expect(plan.target.externalDirectory?.directory).toBe(root)
+          expect(target).toMatchObject({ canonical: path.join(root, "existing.txt") })
+          expect(target.externalDirectory?.directory).toBe(root)
         }).pipe(provide(directory)),
       ),
     ),
@@ -153,10 +160,9 @@ describe("LocationMutation", () => {
       withTmp((outside) =>
         Effect.gen(function* () {
           const targetPath = path.join(outside, "new", "nested", "file.txt")
-          const plan = yield* (yield* LocationMutation.Service).resolve({ path: targetPath })
+          const target = yield* (yield* LocationMutation.Service).resolve({ path: targetPath })
           const root = yield* Effect.promise(() => fs.realpath(outside))
-          expect(plan.authority.canonical).toBe(root)
-          expect(plan.target.externalDirectory).toMatchObject({
+          expect(target.externalDirectory).toMatchObject({
             directory: root,
             resource: path.join(root, "*").replaceAll("\\", "/"),
           })
@@ -165,66 +171,6 @@ describe("LocationMutation", () => {
     ),
   )
 
-  it.live("rejects a symlink-ancestor swap during post-approval revalidation", () =>
-    withTmp((directory) =>
-      withTmp((outside) =>
-        Effect.gen(function* () {
-          if (process.platform === "win32") return
-          const parent = path.join(directory, "parent")
-          yield* Effect.promise(() => fs.mkdir(parent))
-          const service = yield* LocationMutation.Service
-          const plan = yield* service.resolve({ path: path.join("parent", "new.txt") })
-          yield* Effect.promise(async () => {
-            await fs.rmdir(parent)
-            await fs.symlink(outside, parent)
-          })
-
-          const error = yield* Effect.flip(service.revalidate(plan))
-          expect(error).toMatchObject({ _tag: "LocationMutation.RevalidationError" })
-        }).pipe(provide(directory)),
-      ),
-    ),
-  )
-
-  it.live("rejects an existing target identity swap during post-approval revalidation", () =>
-    withTmp((directory) =>
-      Effect.gen(function* () {
-        const targetPath = path.join(directory, "existing.txt")
-        yield* Effect.promise(() => fs.writeFile(targetPath, "first"))
-        const service = yield* LocationMutation.Service
-        const plan = yield* service.resolve({ path: "existing.txt" })
-        yield* Effect.promise(async () => {
-          const replacementPath = path.join(directory, "replacement.txt")
-          await fs.writeFile(replacementPath, "second")
-          await fs.rm(targetPath)
-          await fs.rename(replacementPath, targetPath)
-        })
-
-        const error = yield* Effect.flip(service.revalidate(plan))
-        expect(error).toMatchObject({
-          _tag: "LocationMutation.RevalidationError",
-          reason: "mutation authority changed",
-        })
-      }).pipe(provide(directory)),
-    ),
-  )
-
-  it.live("rejects a nearer prospective ancestor introduced after approval", () =>
-    withTmp((directory) =>
-      Effect.gen(function* () {
-        const service = yield* LocationMutation.Service
-        const plan = yield* service.resolve({ path: path.join("new", "nested", "file.txt") })
-        yield* Effect.promise(() => fs.mkdir(path.join(directory, "new")))
-
-        const error = yield* Effect.flip(service.revalidate(plan))
-        expect(error).toMatchObject({
-          _tag: "LocationMutation.RevalidationError",
-          reason: "mutation authority changed",
-        })
-      }).pipe(provide(directory)),
-    ),
-  )
-
   test("keeps project references outside the mutation input API", () => {
     expect(Object.keys(LocationMutation.ResolveInput.fields)).toEqual(["path", "kind"])
     expect(Schema.decodeUnknownSync(LocationMutation.ResolveInput)({ path: "README.md", reference: "docs" })).toEqual({

+ 28 - 5
packages/core/test/tool-apply-patch.test.ts

@@ -24,6 +24,7 @@ let editApproved = false
 let blockRemoveTarget: string | undefined
 let removeStarted: Deferred.Deferred<void> | undefined
 let releaseRemove: Deferred.Deferred<void> | undefined
+let afterEditApproval = (): Effect.Effect<void> => Effect.void
 
 const permission = Layer.succeed(
   PermissionV2.Service,
@@ -33,6 +34,7 @@ const permission = Layer.succeed(
         assertions.push(input)
         if (input.action === "edit") editApproved = true
       }).pipe(
+        Effect.andThen(input.action === "edit" ? Effect.suspend(afterEditApproval) : Effect.void),
         Effect.andThen(
           input.action === denyAction ? Effect.fail(new PermissionV2.DeniedError({ rules: [] })) : Effect.void,
         ),
@@ -54,6 +56,7 @@ const reset = () => {
   blockRemoveTarget = undefined
   removeStarted = undefined
   releaseRemove = undefined
+  afterEditApproval = () => Effect.void
 }
 
 const filesystem = Layer.effect(
@@ -84,18 +87,18 @@ const withTool = <A, E, R>(directory: string, body: (registry: ToolRegistry.Inte
     Location.Service,
     Location.Service.of(location({ directory: AbsolutePath.make(directory) })),
   )
-  const planning = LocationMutation.layer.pipe(Layer.provide(filesystem), Layer.provide(activeLocation))
-  const commits = FileMutation.layer.pipe(Layer.provide(filesystem), Layer.provide(planning))
+  const resolution = LocationMutation.layer.pipe(Layer.provide(filesystem), Layer.provide(activeLocation))
+  const mutation = FileMutation.layer.pipe(Layer.provide(filesystem))
   const registry = ToolRegistry.defaultLayer.pipe(Layer.provide(permission))
   const patch = ApplyPatchTool.layer.pipe(
     Layer.provide(registry),
-    Layer.provide(planning),
-    Layer.provide(commits),
+    Layer.provide(resolution),
+    Layer.provide(mutation),
     Layer.provide(filesystem),
   )
   return Effect.gen(function* () {
     return yield* body(yield* ToolRegistry.Service)
-  }).pipe(Effect.provide(Layer.mergeAll(registry, planning, commits, patch)))
+  }).pipe(Effect.provide(Layer.mergeAll(registry, resolution, mutation, patch)))
 }
 
 const call = (patchText: string, id = "call-apply-patch") => ({
@@ -302,6 +305,26 @@ describe("ApplyPatchTool", () => {
     ),
   )
 
+  it.live("rejects an add target that appears during permission approval", () =>
+    Effect.acquireUseRelease(
+      Effect.promise(() => tmpdir()),
+      (tmp) => {
+        reset()
+        const target = path.join(tmp.path, "appeared.txt")
+        afterEditApproval = () => Effect.promise(() => fs.writeFile(target, "winner\n")).pipe(Effect.orDie)
+        return withTool(tmp.path, (registry) =>
+          Effect.gen(function* () {
+            expect(
+              yield* registry.execute(call("*** Begin Patch\n*** Add File: appeared.txt\n+replacement\n*** End Patch")),
+            ).toEqual({ type: "error", value: "Unable to apply patch at appeared.txt" })
+            expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("winner\n")
+          }),
+        )
+      },
+      (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
+    ),
+  )
+
   it.live("reports earlier sequential applications when a later commit fails", () =>
     Effect.acquireUseRelease(
       Effect.promise(() => tmpdir()),

+ 31 - 0
packages/core/test/tool-bash.test.ts

@@ -38,6 +38,7 @@ let result: AppProcess.RunResult = {
   stderrTruncated: false,
 }
 let runFailure: AppProcess.AppProcessError | undefined
+let afterPermission = (_input: PermissionV2.AssertInput): Effect.Effect<void> => Effect.void
 let truncate = (input: ToolOutputStore.TruncateInput): Effect.Effect<ToolOutputStore.TruncateResult> =>
   Effect.succeed({ content: input.content, truncated: false })
 
@@ -46,6 +47,7 @@ const permission = Layer.succeed(
   PermissionV2.Service.of({
     assert: (input) =>
       Effect.sync(() => assertions.push(input)).pipe(
+        Effect.andThen(Effect.suspend(() => afterPermission(input))),
         Effect.andThen(
           input.action === denyAction ? Effect.fail(new PermissionV2.DeniedError({ rules: [] })) : Effect.void,
         ),
@@ -91,6 +93,7 @@ const reset = () => {
   truncations.length = 0
   denyAction = undefined
   runFailure = undefined
+  afterPermission = () => Effect.void
   result = {
     command: "mock",
     exitCode: 0,
@@ -118,6 +121,7 @@ const withTool = <A, E, R>(
     Layer.provide(registry),
     Layer.provide(permission),
     Layer.provide(mutation),
+    Layer.provide(filesystem),
     Layer.provide(processLayer),
     Layer.provide(resources),
     Layer.provide(config),
@@ -187,6 +191,33 @@ describe("BashTool", () => {
     ),
   )
 
+  it.live("rejects a workdir that stops being a directory during approval", () =>
+    Effect.acquireUseRelease(
+      Effect.promise(() => tmpdir()),
+      (tmp) => {
+        reset()
+        const workdir = path.join(tmp.path, "src")
+        afterPermission = (input) =>
+          input.action === "bash"
+            ? Effect.promise(async () => {
+                await fs.rm(workdir, { recursive: true })
+                await fs.writeFile(workdir, "not a directory")
+              }).pipe(Effect.orDie)
+            : Effect.void
+        return Effect.promise(() => fs.mkdir(workdir)).pipe(
+          Effect.andThen(withTool(tmp.path, (registry) => registry.execute(call({ command: "pwd", workdir: "src" })))),
+          Effect.andThen(
+            Effect.sync(() => {
+              expect(runs).toEqual([])
+              expect(assertions.map((input) => input.action)).toEqual(["bash"])
+            }),
+          ),
+        )
+      },
+      (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
+    ),
+  )
+
   if (process.platform !== "win32") {
     it.live("executes a real shell command through AppProcess", () =>
       Effect.acquireUseRelease(

+ 10 - 59
packages/core/test/tool-edit.test.ts

@@ -21,7 +21,6 @@ const assertions: PermissionV2.AssertInput[] = []
 const writes: string[] = []
 let reads = 0
 let denyAction: string | undefined
-let afterAssertion = (_input: PermissionV2.AssertInput): Effect.Effect<void> => Effect.void
 let afterRead = (_target: string, _content: Uint8Array): Effect.Effect<void> => Effect.void
 
 const permission = Layer.succeed(
@@ -30,9 +29,7 @@ const permission = Layer.succeed(
     assert: (input) =>
       Effect.sync(() => assertions.push(input)).pipe(
         Effect.andThen(
-          input.action === denyAction
-            ? Effect.fail(new PermissionV2.DeniedError({ rules: [] }))
-            : afterAssertion(input),
+          input.action === denyAction ? Effect.fail(new PermissionV2.DeniedError({ rules: [] })) : Effect.void,
         ),
       ),
     ask: () => Effect.die("unused"),
@@ -48,7 +45,6 @@ const reset = () => {
   writes.length = 0
   reads = 0
   denyAction = undefined
-  afterAssertion = () => Effect.void
   afterRead = () => Effect.void
 }
 
@@ -68,6 +64,10 @@ const filesystem = Layer.effect(
           ),
       writeWithDirs: (target, content, mode) =>
         Effect.sync(() => writes.push(target)).pipe(Effect.andThen(fs.writeWithDirs(target, content, mode))),
+      writeFile: (target, content, options) =>
+        Effect.sync(() => writes.push(target)).pipe(Effect.andThen(fs.writeFile(target, content, options))),
+      writeFileString: (target, content, options) =>
+        Effect.sync(() => writes.push(target)).pipe(Effect.andThen(fs.writeFileString(target, content, options))),
     })
   }),
 ).pipe(Layer.provide(FSUtil.defaultLayer))
@@ -77,18 +77,18 @@ const withTool = <A, E, R>(directory: string, body: (registry: ToolRegistry.Inte
     Location.Service,
     Location.Service.of(location({ directory: AbsolutePath.make(directory) })),
   )
-  const planning = LocationMutation.layer.pipe(Layer.provide(filesystem), Layer.provide(activeLocation))
-  const commits = FileMutation.layer.pipe(Layer.provide(filesystem), Layer.provide(planning))
+  const resolution = LocationMutation.layer.pipe(Layer.provide(filesystem), Layer.provide(activeLocation))
+  const mutation = FileMutation.layer.pipe(Layer.provide(filesystem))
   const registry = ToolRegistry.defaultLayer.pipe(Layer.provide(permission))
   const edit = EditTool.layer.pipe(
     Layer.provide(registry),
-    Layer.provide(planning),
-    Layer.provide(commits),
+    Layer.provide(resolution),
+    Layer.provide(mutation),
     Layer.provide(filesystem),
   )
   return Effect.gen(function* () {
     return yield* body(yield* ToolRegistry.Service)
-  }).pipe(Effect.provide(Layer.mergeAll(registry, planning, commits, edit)))
+  }).pipe(Effect.provide(Layer.mergeAll(registry, resolution, mutation, edit)))
 }
 
 const call = (input: typeof EditTool.Parameters.Type, id = "call-edit") => ({
@@ -384,55 +384,6 @@ describe("EditTool", () => {
       (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
     ),
   )
-
-  if (process.platform !== "win32") {
-    it.live("delegates post-approval revalidation to FileMutation before writing", () =>
-      Effect.acquireUseRelease(
-        Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
-        ([active, outside]) => {
-          reset()
-          const parent = path.join(active.path, "parent")
-          const detached = path.join(active.path, "detached")
-          afterAssertion = (input) =>
-            input.action === "edit"
-              ? Effect.promise(async () => {
-                  await fs.rename(parent, detached)
-                  await fs.symlink(outside.path, parent)
-                })
-              : Effect.void
-          return Effect.promise(async () => {
-            await fs.mkdir(parent)
-            await fs.writeFile(path.join(parent, "escape.txt"), "before")
-          }).pipe(
-            Effect.andThen(
-              withTool(active.path, (registry) =>
-                registry.execute(call({ path: "parent/escape.txt", oldString: "before", newString: "after" })),
-              ),
-            ),
-            Effect.andThen((result) =>
-              Effect.gen(function* () {
-                expect(result).toEqual({ type: "error", value: "Unable to edit parent/escape.txt" })
-                expect(assertions.map((input) => input.action)).toEqual(["edit"])
-                expect(writes).toEqual([])
-                expect(
-                  yield* Effect.promise(() =>
-                    fs.stat(path.join(outside.path, "escape.txt")).then(
-                      () => true,
-                      () => false,
-                    ),
-                  ),
-                ).toBe(false)
-              }),
-            ),
-          )
-        },
-        ([active, outside]) =>
-          Effect.promise(() =>
-            Promise.all([active[Symbol.asyncDispose](), outside[Symbol.asyncDispose]()]).then(() => undefined),
-          ),
-      ),
-    )
-  }
 })
 
 test("keeps the locked edit schema, semantics docstring, and deferred TODOs visible", async () => {

+ 5 - 54
packages/core/test/tool-write.test.ts

@@ -20,7 +20,6 @@ const sessionID = SessionV2.ID.make("ses_write_tool_test")
 const assertions: PermissionV2.AssertInput[] = []
 const writes: string[] = []
 let denyAction: string | undefined
-let afterAssertion = (_input: PermissionV2.AssertInput): Effect.Effect<void> => Effect.void
 
 const permission = Layer.succeed(
   PermissionV2.Service,
@@ -28,9 +27,7 @@ const permission = Layer.succeed(
     assert: (input) =>
       Effect.sync(() => assertions.push(input)).pipe(
         Effect.andThen(
-          input.action === denyAction
-            ? Effect.fail(new PermissionV2.DeniedError({ rules: [] }))
-            : afterAssertion(input),
+          input.action === denyAction ? Effect.fail(new PermissionV2.DeniedError({ rules: [] })) : Effect.void,
         ),
       ),
     ask: () => Effect.die("unused"),
@@ -45,7 +42,6 @@ const reset = () => {
   assertions.length = 0
   writes.length = 0
   denyAction = undefined
-  afterAssertion = () => Effect.void
 }
 
 const filesystem = Layer.effect(
@@ -65,13 +61,13 @@ const withTool = <A, E, R>(directory: string, body: (registry: ToolRegistry.Inte
     Location.Service,
     Location.Service.of(location({ directory: AbsolutePath.make(directory) })),
   )
-  const planning = LocationMutation.layer.pipe(Layer.provide(filesystem), Layer.provide(activeLocation))
-  const commits = FileMutation.layer.pipe(Layer.provide(filesystem), Layer.provide(planning))
+  const resolution = LocationMutation.layer.pipe(Layer.provide(filesystem), Layer.provide(activeLocation))
+  const mutation = FileMutation.layer.pipe(Layer.provide(filesystem))
   const registry = ToolRegistry.defaultLayer.pipe(Layer.provide(permission))
-  const write = WriteTool.layer.pipe(Layer.provide(registry), Layer.provide(planning), Layer.provide(commits))
+  const write = WriteTool.layer.pipe(Layer.provide(registry), Layer.provide(resolution), Layer.provide(mutation))
   return Effect.gen(function* () {
     return yield* body(yield* ToolRegistry.Service)
-  }).pipe(Effect.provide(Layer.mergeAll(registry, planning, commits, write)))
+  }).pipe(Effect.provide(Layer.mergeAll(registry, resolution, mutation, write)))
 }
 
 const call = (input: typeof WriteTool.Parameters.Type, id = "call-write") => ({
@@ -258,51 +254,6 @@ describe("WriteTool", () => {
         ),
     ),
   )
-
-  if (process.platform !== "win32") {
-    it.live("delegates post-approval revalidation to FileMutation before writing", () =>
-      Effect.acquireUseRelease(
-        Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
-        ([active, outside]) => {
-          reset()
-          const parent = path.join(active.path, "parent")
-          afterAssertion = (input) =>
-            input.action === "edit"
-              ? Effect.promise(async () => {
-                  await fs.rmdir(parent)
-                  await fs.symlink(outside.path, parent)
-                })
-              : Effect.void
-          return Effect.promise(() => fs.mkdir(parent)).pipe(
-            Effect.andThen(
-              withTool(active.path, (registry) =>
-                registry.execute(call({ path: "parent/escape.txt", content: "blocked" })),
-              ),
-            ),
-            Effect.andThen((result) =>
-              Effect.gen(function* () {
-                expect(result).toEqual({ type: "error", value: "Unable to write parent/escape.txt" })
-                expect(assertions.map((input) => input.action)).toEqual(["edit"])
-                expect(writes).toEqual([])
-                expect(
-                  yield* Effect.promise(() =>
-                    fs.stat(path.join(outside.path, "escape.txt")).then(
-                      () => true,
-                      () => false,
-                    ),
-                  ),
-                ).toBe(false)
-              }),
-            ),
-          )
-        },
-        ([active, outside]) =>
-          Effect.promise(() =>
-            Promise.all([active[Symbol.asyncDispose](), outside[Symbol.asyncDispose]()]).then(() => undefined),
-          ),
-      ),
-    )
-  }
 })
 
 test("keeps the locked write schema, semantics docstring, and deferred UX TODOs visible", async () => {