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.pull/21016/head
parent
d9a07b5d96
commit
6a56bd5e79
|
|
@ -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`:
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue