From 6a56bd5e79753e791bc38e0075dea3b450f2f715 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Fri, 3 Apr 2026 22:44:51 -0400 Subject: [PATCH] refactor(effect): simplify read tool boundaries Keep LSP warmup off the read critical path and use AppFileSystem helpers directly in the read tool. Update the migration note to describe the single-bridge pattern that the tool now follows. --- packages/opencode/specs/effect-migration.md | 2 +- packages/opencode/src/tool/read.ts | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/opencode/specs/effect-migration.md b/packages/opencode/specs/effect-migration.md index 60f4d5802f..9ca123d317 100644 --- a/packages/opencode/specs/effect-migration.md +++ b/packages/opencode/specs/effect-migration.md @@ -240,7 +240,7 @@ Once individual tools are effectified, change `Tool.Info` (`tool/tool.ts`) so `i Until the tool interface itself returns `Effect`, use this transitional pattern for migrated tools: - `Tool.defineEffect(...)` should `yield*` the services the tool depends on and close over them in the returned tool definition. -- Keep the bridge at the Promise boundary only. In the temporary `async execute(...)` implementation, call service methods with `await Effect.runPromise(...)` instead of falling back to static async facades. +- Keep the bridge at the Promise boundary only. Prefer a single `Effect.runPromise(...)` in the temporary `async execute(...)` implementation, and move the inner logic into `Effect.fn(...)` helpers instead of scattering `runPromise` islands through the tool body. - If a tool starts requiring new services, wire them into `ToolRegistry.defaultLayer` so production callers resolve the same dependencies as tests. Tool tests should use the existing Effect helpers in `packages/opencode/test/lib/effect.ts`: diff --git a/packages/opencode/src/tool/read.ts b/packages/opencode/src/tool/read.ts index 6a913549c5..0118c389df 100644 --- a/packages/opencode/src/tool/read.ts +++ b/packages/opencode/src/tool/read.ts @@ -12,7 +12,6 @@ import DESCRIPTION from "./read.txt" import { Instance } from "../project/instance" import { assertExternalDirectory } from "./external-directory" import { Instruction } from "../session/instruction" -import { Filesystem } from "../util/filesystem" const DEFAULT_READ_LIMIT = 2000 const MAX_LINE_LENGTH = 2000 @@ -78,7 +77,9 @@ export const ReadTool = Tool.defineEffect( }) const warm = Effect.fn("ReadTool.warm")(function* (filepath: string, sessionID: Tool.Context["sessionID"]) { - yield* lsp.touchFile(filepath, false) + yield* Effect.sync(() => { + void Effect.runFork(lsp.touchFile(filepath, false)) + }) yield* time.read(sessionID, filepath) }) @@ -92,7 +93,7 @@ export const ReadTool = Tool.defineEffect( filepath = path.resolve(Instance.directory, filepath) } if (process.platform === "win32") { - filepath = Filesystem.normalizePath(filepath) + filepath = AppFileSystem.normalizePath(filepath) } const title = path.relative(Instance.worktree, filepath) @@ -146,7 +147,7 @@ export const ReadTool = Tool.defineEffect( const loaded = yield* instruction.resolve(ctx.messages, filepath, ctx.messageID) - const mime = Filesystem.mimeType(filepath) + const mime = AppFileSystem.mimeType(filepath) const isImage = mime.startsWith("image/") && mime !== "image/svg+xml" && mime !== "image/vnd.fastbidsheet" const isPdf = mime === "application/pdf" if (isImage || isPdf) {