refactor(tool): convert write tool to Tool.defineEffect (#21901)

packages/opencode/src/tool/registry.ts

@@ -110,6 +110,7 @@ export namespace ToolRegistry {
       const bash = yield* BashTool
       const codesearch = yield* CodeSearchTool
       const globtool = yield* GlobTool
+      const writetool = yield* WriteTool
 
       const state = yield* InstanceState.make<State>(
         Effect.fn("ToolRegistry.state")(function* (ctx) {
@@ -173,7 +174,7 @@ export namespace ToolRegistry {
             glob: Tool.init(globtool),
             grep: Tool.init(GrepTool),
             edit: Tool.init(EditTool),
-            write: Tool.init(WriteTool),
+            write: Tool.init(writetool),
             task: Tool.init(task),
             fetch: Tool.init(webfetch),
             todo: Tool.init(todo),

packages/opencode/src/tool/write.ts

@@ -1,5 +1,6 @@
 import z from "zod"
 import * as path from "path"
+import { Effect } from "effect"
 import { Tool } from "./tool"
 import { LSP } from "../lsp"
 import { createTwoFilesPatch } from "diff"
@@ -9,30 +10,39 @@ import { File } from "../file"
 import { FileWatcher } from "../file/watcher"
 import { Format } from "../format"
 import { FileTime } from "../file/time"
-import { Filesystem } from "../util/filesystem"
+import { AppFileSystem } from "../filesystem"
 import { Instance } from "../project/instance"
 import { trimDiff } from "./edit"
-import { assertExternalDirectory } from "./external-directory"
+import { assertExternalDirectoryEffect } from "./external-directory"
 
 const MAX_DIAGNOSTICS_PER_FILE = 20
 const MAX_PROJECT_DIAGNOSTICS_FILES = 5
 
-export const WriteTool = Tool.define("write", {
+export const WriteTool = Tool.defineEffect(
+  "write",
+  Effect.gen(function* () {
+    const lsp = yield* LSP.Service
+    const fs = yield* AppFileSystem.Service
+    const filetime = yield* FileTime.Service
+
+    return {
       description: DESCRIPTION,
       parameters: z.object({
         content: z.string().describe("The content to write to the file"),
         filePath: z.string().describe("The absolute path to the file to write (must be absolute, not relative)"),
       }),
-  async execute(params, ctx) {
+      execute: (params: { content: string; filePath: string }, ctx: Tool.Context) =>
+        Effect.gen(function* () {
           const filepath = path.isAbsolute(params.filePath) ? params.filePath : path.join(Instance.directory, params.filePath)
-    await assertExternalDirectory(ctx, filepath)
+          yield* assertExternalDirectoryEffect(ctx, filepath)
 
-    const exists = await Filesystem.exists(filepath)
+          const exists = yield* fs.existsSafe(filepath)
-    const contentOld = exists ? await Filesystem.readText(filepath) : ""
+          const contentOld = exists ? yield* fs.readFileString(filepath) : ""
-    if (exists) await FileTime.assert(ctx.sessionID, filepath)
+          if (exists) yield* filetime.assert(ctx.sessionID, filepath)
 
           const diff = trimDiff(createTwoFilesPatch(filepath, filepath, contentOld, params.content))
-    await ctx.ask({
+          yield* Effect.promise(() =>
+            ctx.ask({
               permission: "edit",
               patterns: [path.relative(Instance.worktree, filepath)],
               always: ["*"],
@@ -40,21 +50,24 @@ export const WriteTool = Tool.define("write", {
                 filepath,
                 diff,
               },
-    })
+            }),
+          )
 
-    await Filesystem.write(filepath, params.content)
+          yield* fs.writeWithDirs(filepath, params.content)
-    await Format.file(filepath)
+          yield* Effect.promise(() => Format.file(filepath))
           Bus.publish(File.Event.Edited, { file: filepath })
-    await Bus.publish(FileWatcher.Event.Updated, {
+          yield* Effect.promise(() =>
+            Bus.publish(FileWatcher.Event.Updated, {
               file: filepath,
               event: exists ? "change" : "add",
-    })
+            }),
-    await FileTime.read(ctx.sessionID, filepath)
+          )
+          yield* filetime.read(ctx.sessionID, filepath)
 
           let output = "Wrote file successfully."
-    await LSP.touchFile(filepath, true)
+          yield* lsp.touchFile(filepath, true)
-    const diagnostics = await LSP.diagnostics()
+          const diagnostics = yield* lsp.diagnostics()
-    const normalizedFilepath = Filesystem.normalizePath(filepath)
+          const normalizedFilepath = AppFileSystem.normalizePath(filepath)
           let projectDiagnosticsCount = 0
           for (const [file, issues] of Object.entries(diagnostics)) {
             const errors = issues.filter((item) => item.severity === 1)
@@ -80,5 +93,7 @@ export const WriteTool = Tool.define("write", {
             },
             output,
           }
-  },
+        }).pipe(Effect.orDie, Effect.runPromise),
-})
+    }
+  }),
+)

packages/opencode/test/tool/write.test.ts

@@ -1,10 +1,17 @@
-import { afterEach, describe, test, expect } from "bun:test"
+import { afterEach, describe, expect } from "bun:test"
+import { Effect, Layer } from "effect"
 import path from "path"
 import fs from "fs/promises"
 import { WriteTool } from "../../src/tool/write"
 import { Instance } from "../../src/project/instance"
-import { tmpdir } from "../fixture/fixture"
+import { LSP } from "../../src/lsp"
+import { AppFileSystem } from "../../src/filesystem"
+import { FileTime } from "../../src/file/time"
+import { Tool } from "../../src/tool/tool"
 import { SessionID, MessageID } from "../../src/session/schema"
+import * as CrossSpawnSpawner from "../../src/effect/cross-spawn-spawner"
+import { provideTmpdirInstance } from "../fixture/fixture"
+import { testEffect } from "../lib/effect"
 
 const ctx = {
   sessionID: SessionID.make("ses_test-write-session"),
@@ -21,333 +28,216 @@ afterEach(async () => {
   await Instance.disposeAll()
 })
 
+const it = testEffect(
+  Layer.mergeAll(LSP.defaultLayer, AppFileSystem.defaultLayer, FileTime.defaultLayer, CrossSpawnSpawner.defaultLayer),
+)
+
+const init = Effect.fn("WriteToolTest.init")(function* () {
+  const info = yield* WriteTool
+  return yield* Effect.promise(() => info.init())
+})
+
+const run = Effect.fn("WriteToolTest.run")(function* (
+  args: Tool.InferParameters<typeof WriteTool>,
+  next: Tool.Context = ctx,
+) {
+  const tool = yield* init()
+  return yield* Effect.promise(() => tool.execute(args, next))
+})
+
+const markRead = Effect.fn("WriteToolTest.markRead")(function* (sessionID: string, filepath: string) {
+  const ft = yield* FileTime.Service
+  yield* ft.read(sessionID as any, filepath)
+})
+
 describe("tool.write", () => {
   describe("new file creation", () => {
-    test("writes content to new file", async () => {
+    it.live("writes content to new file", () =>
-      await using tmp = await tmpdir()
+      provideTmpdirInstance((dir) =>
-      const filepath = path.join(tmp.path, "newfile.txt")
+        Effect.gen(function* () {
-
+          const filepath = path.join(dir, "newfile.txt")
-      await Instance.provide({
+          const result = yield* run({ filePath: filepath, content: "Hello, World!" })
-        directory: tmp.path,
-        fn: async () => {
-          const write = await WriteTool.init()
-          const result = await write.execute(
-            {
-              filePath: filepath,
-              content: "Hello, World!",
-            },
-            ctx,
-          )
 
           expect(result.output).toContain("Wrote file successfully")
           expect(result.metadata.exists).toBe(false)
 
-          const content = await fs.readFile(filepath, "utf-8")
+          const content = yield* Effect.promise(() => fs.readFile(filepath, "utf-8"))
           expect(content).toBe("Hello, World!")
-        },
+        }),
-      })
+      ),
-    })
-
-    test("creates parent directories if needed", async () => {
-      await using tmp = await tmpdir()
-      const filepath = path.join(tmp.path, "nested", "deep", "file.txt")
-
-      await Instance.provide({
-        directory: tmp.path,
-        fn: async () => {
-          const write = await WriteTool.init()
-          await write.execute(
-            {
-              filePath: filepath,
-              content: "nested content",
-            },
-            ctx,
     )
 
-          const content = await fs.readFile(filepath, "utf-8")
+    it.live("creates parent directories if needed", () =>
+      provideTmpdirInstance((dir) =>
+        Effect.gen(function* () {
+          const filepath = path.join(dir, "nested", "deep", "file.txt")
+          yield* run({ filePath: filepath, content: "nested content" })
+
+          const content = yield* Effect.promise(() => fs.readFile(filepath, "utf-8"))
           expect(content).toBe("nested content")
-        },
+        }),
-      })
+      ),
-    })
-
-    test("handles relative paths by resolving to instance directory", async () => {
-      await using tmp = await tmpdir()
-
-      await Instance.provide({
-        directory: tmp.path,
-        fn: async () => {
-          const write = await WriteTool.init()
-          await write.execute(
-            {
-              filePath: "relative.txt",
-              content: "relative content",
-            },
-            ctx,
     )
 
-          const content = await fs.readFile(path.join(tmp.path, "relative.txt"), "utf-8")
+    it.live("handles relative paths by resolving to instance directory", () =>
+      provideTmpdirInstance((dir) =>
+        Effect.gen(function* () {
+          yield* run({ filePath: "relative.txt", content: "relative content" })
+
+          const content = yield* Effect.promise(() => fs.readFile(path.join(dir, "relative.txt"), "utf-8"))
           expect(content).toBe("relative content")
-        },
+        }),
-      })
+      ),
-    })
+    )
   })
 
   describe("existing file overwrite", () => {
-    test("overwrites existing file content", async () => {
+    it.live("overwrites existing file content", () =>
-      await using tmp = await tmpdir()
+      provideTmpdirInstance((dir) =>
-      const filepath = path.join(tmp.path, "existing.txt")
+        Effect.gen(function* () {
-      await fs.writeFile(filepath, "old content", "utf-8")
+          const filepath = path.join(dir, "existing.txt")
+          yield* Effect.promise(() => fs.writeFile(filepath, "old content", "utf-8"))
+          yield* markRead(ctx.sessionID, filepath)
 
-      // First read the file to satisfy FileTime requirement
+          const result = yield* run({ filePath: filepath, content: "new content" })
-      await Instance.provide({
-        directory: tmp.path,
-        fn: async () => {
-          const { FileTime } = await import("../../src/file/time")
-          await FileTime.read(ctx.sessionID, filepath)
-
-          const write = await WriteTool.init()
-          const result = await write.execute(
-            {
-              filePath: filepath,
-              content: "new content",
-            },
-            ctx,
-          )
 
           expect(result.output).toContain("Wrote file successfully")
           expect(result.metadata.exists).toBe(true)
 
-          const content = await fs.readFile(filepath, "utf-8")
+          const content = yield* Effect.promise(() => fs.readFile(filepath, "utf-8"))
           expect(content).toBe("new content")
-        },
+        }),
-      })
+      ),
-    })
-
-    test("returns diff in metadata for existing files", async () => {
-      await using tmp = await tmpdir()
-      const filepath = path.join(tmp.path, "file.txt")
-      await fs.writeFile(filepath, "old", "utf-8")
-
-      await Instance.provide({
-        directory: tmp.path,
-        fn: async () => {
-          const { FileTime } = await import("../../src/file/time")
-          await FileTime.read(ctx.sessionID, filepath)
-
-          const write = await WriteTool.init()
-          const result = await write.execute(
-            {
-              filePath: filepath,
-              content: "new",
-            },
-            ctx,
     )
 
-          // Diff should be in metadata
+    it.live("returns diff in metadata for existing files", () =>
+      provideTmpdirInstance((dir) =>
+        Effect.gen(function* () {
+          const filepath = path.join(dir, "file.txt")
+          yield* Effect.promise(() => fs.writeFile(filepath, "old", "utf-8"))
+          yield* markRead(ctx.sessionID, filepath)
+
+          const result = yield* run({ filePath: filepath, content: "new" })
+
           expect(result.metadata).toHaveProperty("filepath", filepath)
           expect(result.metadata).toHaveProperty("exists", true)
-        },
+        }),
-      })
+      ),
-    })
+    )
   })
 
   describe("file permissions", () => {
-    test("sets file permissions when writing sensitive data", async () => {
+    it.live("sets file permissions when writing sensitive data", () =>
-      await using tmp = await tmpdir()
+      provideTmpdirInstance((dir) =>
-      const filepath = path.join(tmp.path, "sensitive.json")
+        Effect.gen(function* () {
+          const filepath = path.join(dir, "sensitive.json")
+          yield* run({ filePath: filepath, content: JSON.stringify({ secret: "data" }) })
 
-      await Instance.provide({
-        directory: tmp.path,
-        fn: async () => {
-          const write = await WriteTool.init()
-          await write.execute(
-            {
-              filePath: filepath,
-              content: JSON.stringify({ secret: "data" }),
-            },
-            ctx,
-          )
-
-          // On Unix systems, check permissions
           if (process.platform !== "win32") {
-            const stats = await fs.stat(filepath)
+            const stats = yield* Effect.promise(() => fs.stat(filepath))
             expect(stats.mode & 0o777).toBe(0o644)
           }
-        },
+        }),
-      })
+      ),
-    })
+    )
   })
 
   describe("content types", () => {
-    test("writes JSON content", async () => {
+    it.live("writes JSON content", () =>
-      await using tmp = await tmpdir()
+      provideTmpdirInstance((dir) =>
-      const filepath = path.join(tmp.path, "data.json")
+        Effect.gen(function* () {
+          const filepath = path.join(dir, "data.json")
           const data = { key: "value", nested: { array: [1, 2, 3] } }
+          yield* run({ filePath: filepath, content: JSON.stringify(data, null, 2) })
 
-      await Instance.provide({
+          const content = yield* Effect.promise(() => fs.readFile(filepath, "utf-8"))
-        directory: tmp.path,
-        fn: async () => {
-          const write = await WriteTool.init()
-          await write.execute(
-            {
-              filePath: filepath,
-              content: JSON.stringify(data, null, 2),
-            },
-            ctx,
-          )
-
-          const content = await fs.readFile(filepath, "utf-8")
           expect(JSON.parse(content)).toEqual(data)
-        },
+        }),
-      })
+      ),
-    })
+    )
 
-    test("writes binary-safe content", async () => {
+    it.live("writes binary-safe content", () =>
-      await using tmp = await tmpdir()
+      provideTmpdirInstance((dir) =>
-      const filepath = path.join(tmp.path, "binary.bin")
+        Effect.gen(function* () {
+          const filepath = path.join(dir, "binary.bin")
           const content = "Hello\x00World\x01\x02\x03"
+          yield* run({ filePath: filepath, content })
 
-      await Instance.provide({
+          const buf = yield* Effect.promise(() => fs.readFile(filepath))
-        directory: tmp.path,
-        fn: async () => {
-          const write = await WriteTool.init()
-          await write.execute(
-            {
-              filePath: filepath,
-              content,
-            },
-            ctx,
-          )
-
-          const buf = await fs.readFile(filepath)
           expect(buf.toString()).toBe(content)
-        },
+        }),
-      })
+      ),
-    })
-
-    test("writes empty content", async () => {
-      await using tmp = await tmpdir()
-      const filepath = path.join(tmp.path, "empty.txt")
-
-      await Instance.provide({
-        directory: tmp.path,
-        fn: async () => {
-          const write = await WriteTool.init()
-          await write.execute(
-            {
-              filePath: filepath,
-              content: "",
-            },
-            ctx,
     )
 
-          const content = await fs.readFile(filepath, "utf-8")
+    it.live("writes empty content", () =>
+      provideTmpdirInstance((dir) =>
+        Effect.gen(function* () {
+          const filepath = path.join(dir, "empty.txt")
+          yield* run({ filePath: filepath, content: "" })
+
+          const content = yield* Effect.promise(() => fs.readFile(filepath, "utf-8"))
           expect(content).toBe("")
 
-          const stats = await fs.stat(filepath)
+          const stats = yield* Effect.promise(() => fs.stat(filepath))
           expect(stats.size).toBe(0)
-        },
+        }),
-      })
+      ),
-    })
+    )
 
-    test("writes multi-line content", async () => {
+    it.live("writes multi-line content", () =>
-      await using tmp = await tmpdir()
+      provideTmpdirInstance((dir) =>
-      const filepath = path.join(tmp.path, "multiline.txt")
+        Effect.gen(function* () {
+          const filepath = path.join(dir, "multiline.txt")
           const lines = ["Line 1", "Line 2", "Line 3", ""].join("\n")
+          yield* run({ filePath: filepath, content: lines })
 
-      await Instance.provide({
+          const content = yield* Effect.promise(() => fs.readFile(filepath, "utf-8"))
-        directory: tmp.path,
-        fn: async () => {
-          const write = await WriteTool.init()
-          await write.execute(
-            {
-              filePath: filepath,
-              content: lines,
-            },
-            ctx,
-          )
-
-          const content = await fs.readFile(filepath, "utf-8")
           expect(content).toBe(lines)
-        },
+        }),
-      })
+      ),
-    })
-
-    test("handles different line endings", async () => {
-      await using tmp = await tmpdir()
-      const filepath = path.join(tmp.path, "crlf.txt")
-      const content = "Line 1\r\nLine 2\r\nLine 3"
-
-      await Instance.provide({
-        directory: tmp.path,
-        fn: async () => {
-          const write = await WriteTool.init()
-          await write.execute(
-            {
-              filePath: filepath,
-              content,
-            },
-            ctx,
     )
 
-          const buf = await fs.readFile(filepath)
+    it.live("handles different line endings", () =>
+      provideTmpdirInstance((dir) =>
+        Effect.gen(function* () {
+          const filepath = path.join(dir, "crlf.txt")
+          const content = "Line 1\r\nLine 2\r\nLine 3"
+          yield* run({ filePath: filepath, content })
+
+          const buf = yield* Effect.promise(() => fs.readFile(filepath))
           expect(buf.toString()).toBe(content)
-        },
+        }),
-      })
+      ),
-    })
+    )
   })
 
   describe("error handling", () => {
-    test("throws error when OS denies write access", async () => {
+    it.live("throws error when OS denies write access", () =>
-      await using tmp = await tmpdir()
+      provideTmpdirInstance((dir) =>
-      const readonlyPath = path.join(tmp.path, "readonly.txt")
+        Effect.gen(function* () {
+          const readonlyPath = path.join(dir, "readonly.txt")
+          yield* Effect.promise(() => fs.writeFile(readonlyPath, "test", "utf-8"))
+          yield* Effect.promise(() => fs.chmod(readonlyPath, 0o444))
+          yield* markRead(ctx.sessionID, readonlyPath)
 
-      // Create a read-only file
+          const exit = yield* run({ filePath: readonlyPath, content: "new content" }).pipe(Effect.exit)
-      await fs.writeFile(readonlyPath, "test", "utf-8")
+          expect(exit._tag).toBe("Failure")
-      await fs.chmod(readonlyPath, 0o444)
+        }),
-
-      await Instance.provide({
-        directory: tmp.path,
-        fn: async () => {
-          const { FileTime } = await import("../../src/file/time")
-          await FileTime.read(ctx.sessionID, readonlyPath)
-
-          const write = await WriteTool.init()
-          await expect(
-            write.execute(
-              {
-                filePath: readonlyPath,
-                content: "new content",
-              },
-              ctx,
       ),
-          ).rejects.toThrow()
+    )
-        },
-      })
-    })
   })
 
   describe("title generation", () => {
-    test("returns relative path as title", async () => {
+    it.live("returns relative path as title", () =>
-      await using tmp = await tmpdir()
+      provideTmpdirInstance((dir) =>
-      const filepath = path.join(tmp.path, "src", "components", "Button.tsx")
+        Effect.gen(function* () {
-      await fs.mkdir(path.dirname(filepath), { recursive: true })
+          const filepath = path.join(dir, "src", "components", "Button.tsx")
-
+          yield* Effect.promise(() => fs.mkdir(path.dirname(filepath), { recursive: true }))
-      await Instance.provide({
-        directory: tmp.path,
-        fn: async () => {
-          const write = await WriteTool.init()
-          const result = await write.execute(
-            {
-              filePath: filepath,
-              content: "export const Button = () => {}",
-            },
-            ctx,
-          )
 
+          const result = yield* run({ filePath: filepath, content: "export const Button = () => {}" })
           expect(result.title).toEndWith(path.join("src", "components", "Button.tsx"))
-        },
+        }),
-      })
+      ),
-    })
+    )
   })
 })