Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion packages/opencode/specs/effect-migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,27 @@ Once individual tools are effectified, change `Tool.Info` (`tool/tool.ts`) so `i
2. Update `Tool.define()` factory to work with Effects
3. Update `SessionPrompt` to `yield*` tool results instead of `await`ing

### Tool migration details

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. 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`:

- Use `testEffect(...)` / `it.live(...)` instead of creating fake local wrappers around effectful tools.
- Yield the real tool export, then initialize it: `const info = yield* ReadTool`, `const tool = yield* Effect.promise(() => info.init())`.
- Run tests inside a real instance with `provideTmpdirInstance(...)` or `provideInstance(tmpdirScoped(...))` so instance-scoped services resolve exactly as they do in production.

This keeps migrated tool tests aligned with the production service graph today, and makes the eventual `Tool.Info` → `Effect` cleanup mostly mechanical later.

Individual tools, ordered by value:

- [ ] `apply_patch.ts` — HIGH: multi-step orchestration, error accumulation, Bus events
- [ ] `bash.ts` — HIGH: shell orchestration, quoting, timeout handling, output capture
- [ ] `read.ts` — HIGH: streaming I/O, readline, binary detection → FileSystem + Stream
- [x] `read.ts` — HIGH: streaming I/O, readline, binary detection → FileSystem + Stream
- [ ] `edit.ts` — HIGH: multi-step diff/format/publish pipeline, FileWatcher lock
- [ ] `grep.ts` — MEDIUM: spawns ripgrep → ChildProcessSpawner, timeout handling
- [ ] `write.ts` — MEDIUM: permission checks, diagnostics polling, Bus events
Expand Down
14 changes: 12 additions & 2 deletions packages/opencode/src/filesystem/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,23 @@ export namespace AppFileSystem {

export function normalizePath(p: string): string {
if (process.platform !== "win32") return p
const resolved = pathResolve(windowsPath(p))
try {
return realpathSync.native(p)
return realpathSync.native(resolved)
} catch {
return p
return resolved
}
}

export function normalizePathPattern(p: string): string {
if (process.platform !== "win32") return p
if (p === "*") return p
const match = p.match(/^(.*)[\\/]\*$/)
if (!match) return normalizePath(p)
const dir = /^[A-Za-z]:$/.test(match[1]) ? match[1] + "\\" : match[1]
return join(normalizePath(dir), "*")
}

export function resolve(p: string): string {
const resolved = pathResolve(windowsPath(p))
try {
Expand Down
15 changes: 12 additions & 3 deletions packages/opencode/src/tool/external-directory.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import path from "path"
import { Effect } from "effect"
import type { Tool } from "./tool"
import { Instance } from "../project/instance"
import { Filesystem } from "@/util/filesystem"
import { AppFileSystem } from "../filesystem"

type Kind = "file" | "directory"

Expand All @@ -15,14 +16,14 @@ export async function assertExternalDirectory(ctx: Tool.Context, target?: string

if (options?.bypass) return

const full = process.platform === "win32" ? Filesystem.normalizePath(target) : target
const full = process.platform === "win32" ? AppFileSystem.normalizePath(target) : target
if (Instance.containsPath(full)) return

const kind = options?.kind ?? "file"
const dir = kind === "directory" ? full : path.dirname(full)
const glob =
process.platform === "win32"
? Filesystem.normalizePathPattern(path.join(dir, "*"))
? AppFileSystem.normalizePathPattern(path.join(dir, "*"))
: path.join(dir, "*").replaceAll("\\", "/")

await ctx.ask({
Expand All @@ -35,3 +36,11 @@ export async function assertExternalDirectory(ctx: Tool.Context, target?: string
},
})
}

export const assertExternalDirectoryEffect = Effect.fn("Tool.assertExternalDirectory")(function* (
ctx: Tool.Context,
target?: string,
options?: Options,
) {
yield* Effect.promise(() => assertExternalDirectory(ctx, target, options))
})
Loading
Loading