From 0a0fc09533371f49d3d97d0c8fc7ff3c3017d522 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Wed, 5 Aug 2026 12:33:20 -0400 Subject: [PATCH] refactor(core): remove unused file mutation methods (#40667) --- packages/core/src/file-mutation.ts | 94 +------------ packages/core/test/file-mutation.test.ts | 162 ----------------------- 2 files changed, 2 insertions(+), 254 deletions(-) diff --git a/packages/core/src/file-mutation.ts b/packages/core/src/file-mutation.ts index b5aee9a5263..03812261601 100644 --- a/packages/core/src/file-mutation.ts +++ b/packages/core/src/file-mutation.ts @@ -1,8 +1,7 @@ export * as FileMutation from "./file-mutation" import { makeLocationNode } from "@opencode-ai/util/effect/app-node" -import { Context, Effect, Layer, Schema } from "effect" -import { dirname } from "path" +import { Context, Effect, Layer } from "effect" import { KeyedMutex } from "./effect/keyed-mutex" import { FSUtil } from "@opencode-ai/util/fs-util" import { Bom } from "@opencode-ai/util/bom" @@ -22,22 +21,6 @@ export interface TextWriteInput { readonly content: string } -export interface ConditionalWriteInput extends WriteInput { - readonly expected: Uint8Array -} - -export interface RemoveInput { - readonly target: Target -} - -export class StaleContentError extends Schema.TaggedErrorClass()("FileMutation.StaleContentError", { - path: Schema.String, -}) {} - -export class TargetExistsError extends Schema.TaggedErrorClass()("FileMutation.TargetExistsError", { - path: Schema.String, -}) {} - export interface WriteResult { readonly operation: "write" readonly target: string @@ -45,24 +28,10 @@ export interface WriteResult { readonly existed: boolean } -export interface RemoveResult { - readonly operation: "remove" - readonly target: string - readonly resource: string - readonly existed: boolean -} - export interface Interface { - /** Create without replacing an existing target. */ - readonly create: (input: WriteInput) => 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 - /** Commit only if an existing target still has the expected bytes. */ - readonly writeIfUnchanged: ( - input: ConditionalWriteInput, - ) => Effect.Effect - readonly remove: (input: RemoveInput) => Effect.Effect } export class Service extends Context.Service()("@opencode/FileMutation") {} @@ -89,13 +58,6 @@ const layer = Layer.effect( existed, }) - const removeResult = (target: Target, existed: boolean): RemoveResult => ({ - operation: "remove", - target: target.canonical, - resource: target.resource, - existed, - }) - const write = Effect.fn("FileMutation.write")((input: WriteInput) => withTargetLock(input.target)( Effect.gen(function* () { @@ -122,62 +84,10 @@ const layer = Layer.effect( ), ) - const create = Effect.fn("FileMutation.create")((input: WriteInput) => - withTargetLock(input.target)( - Effect.gen(function* () { - 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) => - withTargetLock(input.target)( - Effect.gen(function* () { - 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) => - withTargetLock(input.target)( - Effect.gen(function* () { - const existed = yield* fs.remove(input.target.canonical).pipe( - Effect.as(true), - Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(false)), - ) - return removeResult(input.target, existed) - }), - ), - ) - - return Service.of({ create, write, writeTextPreservingBom, writeIfUnchanged, remove }) + return Service.of({ write, writeTextPreservingBom }) }), ) -function sameBytes(left: Uint8Array, right: Uint8Array) { - if (left.length !== right.length) return false - return left.every((byte, index) => byte === right[index]) -} - export const node = makeLocationNode({ service: Service, layer, deps: [FSUtil.node] }) /** diff --git a/packages/core/test/file-mutation.test.ts b/packages/core/test/file-mutation.test.ts index 73f5a7a4dcf..e24c711c872 100644 --- a/packages/core/test/file-mutation.test.ts +++ b/packages/core/test/file-mutation.test.ts @@ -89,68 +89,6 @@ describe("FileMutation", () => { ), ) - it.live("rejects create when a prospective target appears after resolution", () => - withTmp((directory) => - Effect.gen(function* () { - const targetPath = path.join(directory, "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({ target, content: "replacement" }).pipe(Effect.flip), - ).toMatchObject({ - _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 target = yield* (yield* LocationMutation.Service).resolve({ path: "remove.txt" }) - const result = yield* (yield* FileMutation.Service).remove({ target }) - - expect(result).toEqual({ - operation: "remove", - target: target.canonical, - resource: "remove.txt", - existed: true, - }) - expect( - yield* Effect.promise(() => - fs.stat(targetPath).then( - () => true, - () => false, - ), - ), - ).toBe(false) - }).pipe(provide(directory)), - ), - ) - it.live("writes an explicitly resolved external target", () => withTmp((directory) => withTmp((outside) => @@ -171,49 +109,6 @@ describe("FileMutation", () => { ), ) - 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 target = yield* (yield* LocationMutation.Service).resolve({ path: targetPath }) - const result = yield* (yield* FileMutation.Service).remove({ target }) - - expect(result).toEqual({ - operation: "remove", - target: target.canonical, - resource: target.resource, - existed: true, - }) - expect( - yield* Effect.promise(() => - fs.stat(targetPath).then( - () => true, - () => false, - ), - ), - ).toBe(false) - }).pipe(provide(directory)), - ), - ), - ) - - it.live("reports a missing target as not removed without checking existence first", () => - withTmp((directory) => - Effect.gen(function* () { - const target = yield* (yield* LocationMutation.Service).resolve({ path: "missing.txt" }) - - expect(yield* (yield* FileMutation.Service).remove({ target })).toEqual({ - operation: "remove", - target: target.canonical, - resource: "missing.txt", - existed: false, - }) - }).pipe(provide(directory)), - ), - ) - it.live("serializes concurrent writes to the same canonical target", () => withTmp((directory) => Effect.gen(function* () { @@ -257,63 +152,6 @@ describe("FileMutation", () => { ), ) - it.live("allows only one concurrent conditional write based on the same bytes", () => - withTmp((directory) => - Effect.gen(function* () { - const targetPath = path.join(directory, "shared.txt") - yield* Effect.promise(() => fs.writeFile(targetPath, "initial")) - const firstStarted = yield* Deferred.make() - const releaseFirst = yield* Deferred.make() - let writes = 0 - const filesystem = instrumentWrites((write) => - Effect.gen(function* () { - writes++ - if (writes === 1) { - yield* Deferred.succeed(firstStarted, undefined) - yield* Deferred.await(releaseFirst) - } - yield* write - }), - ) - - yield* Effect.gen(function* () { - const mutation = yield* LocationMutation.Service - const files = yield* FileMutation.Service - const target = yield* mutation.resolve({ path: "shared.txt" }) - const expected = new TextEncoder().encode("initial") - const first = yield* files.writeIfUnchanged({ target, expected, content: "first" }).pipe(Effect.forkChild) - yield* Deferred.await(firstStarted) - const second = yield* files - .writeIfUnchanged({ target, expected, content: "second" }) - .pipe(Effect.flip, Effect.forkChild) - - yield* Deferred.succeed(releaseFirst, undefined) - yield* Fiber.join(first) - expect(yield* Fiber.join(second)).toMatchObject({ _tag: "FileMutation.StaleContentError" }) - expect(yield* Effect.promise(() => fs.readFile(targetPath, "utf8"))).toBe("first") - expect(writes).toBe(1) - }).pipe(provide(directory, filesystem)) - }), - ), - ) - - it.live("rejects a conditional write when target content is already stale", () => - withTmp((directory) => - Effect.gen(function* () { - const targetPath = path.join(directory, "stale.txt") - yield* Effect.promise(() => fs.writeFile(targetPath, "current")) - const target = yield* (yield* LocationMutation.Service).resolve({ path: "stale.txt" }) - - expect( - yield* (yield* FileMutation.Service) - .writeIfUnchanged({ target, expected: new TextEncoder().encode("older"), content: "replacement" }) - .pipe(Effect.flip), - ).toMatchObject({ _tag: "FileMutation.StaleContentError", path: target.canonical }) - expect(yield* Effect.promise(() => fs.readFile(targetPath, "utf8"))).toBe("current") - }).pipe(provide(directory)), - ), - ) - it.live("allows distinct canonical targets to proceed independently", () => withTmp((directory) => Effect.gen(function* () {