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
14 changes: 7 additions & 7 deletions packages/opencode/specs/effect-migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -229,24 +229,24 @@ Still open:

## Tool interface → Effect

Once individual tools are effectified, change `Tool.Info` (`tool/tool.ts`) so `init` and `execute` return `Effect` instead of `Promise`. This lets tool implementations compose natively with the Effect pipeline rather than being wrapped in `Effect.promise()` at the call site. Requires:
`Tool.Def.execute` and `Tool.Info.init` already return `Effect` on this branch. Tool definitions should now stay Effect-native all the way through initialization instead of using Promise-returning init callbacks. Tools can still use lazy init callbacks when they need instance-bound state at init time, but those callbacks should return `Effect`, not `Promise`. Remaining work is:

1. Migrate each tool to return Effects
2. Update `Tool.define()` factory to work with Effects
3. Update `SessionPrompt` to `yield*` tool results instead of `await`ing
1. Migrate each tool body to return Effects
2. Keep `Tool.define()` inputs Effect-native
3. Update remaining callers to `yield*` tool initialization instead of `await`ing

### Tool migration details

Until the tool interface itself returns `Effect`, use this transitional pattern for migrated tools:
With `Tool.Info.init()` now effectful, use this transitional pattern for migrated tools that still need Promise-based boundaries internally:

- `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.
- Keep the bridge at the Promise boundary only inside the tool body when required by external APIs. Do not return Promise-based init callbacks from `Tool.define()`.
- 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())`.
- Yield the real tool export, then initialize it: `const info = yield* ReadTool`, `const tool = yield* 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.
Expand Down
95 changes: 48 additions & 47 deletions packages/opencode/src/tool/bash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -454,52 +454,53 @@ export const BashTool = Tool.define(
}
})

return async () => {
const shell = Shell.acceptable()
const name = Shell.name(shell)
const chain =
name === "powershell"
? "If the commands depend on each other and must run sequentially, avoid '&&' in this shell because Windows PowerShell 5.1 does not support it. Use PowerShell conditionals such as `cmd1; if ($?) { cmd2 }` when later commands must depend on earlier success."
: "If the commands depend on each other and must run sequentially, use a single Bash call with '&&' to chain them together (e.g., `git add . && git commit -m \"message\" && git push`). For instance, if one operation must complete before another starts (like mkdir before cp, Write before Bash for git operations, or git add before git commit), run these operations sequentially instead."
log.info("bash tool using shell", { shell })

return {
description: DESCRIPTION.replaceAll("${directory}", Instance.directory)
.replaceAll("${os}", process.platform)
.replaceAll("${shell}", name)
.replaceAll("${chaining}", chain)
.replaceAll("${maxLines}", String(Truncate.MAX_LINES))
.replaceAll("${maxBytes}", String(Truncate.MAX_BYTES)),
parameters: Parameters,
execute: (params: z.infer<typeof Parameters>, ctx: Tool.Context) =>
Effect.gen(function* () {
const cwd = params.workdir
? yield* resolvePath(params.workdir, Instance.directory, shell)
: Instance.directory
if (params.timeout !== undefined && params.timeout < 0) {
throw new Error(`Invalid timeout value: ${params.timeout}. Timeout must be a positive number.`)
}
const timeout = params.timeout ?? DEFAULT_TIMEOUT
const ps = PS.has(name)
const root = yield* parse(params.command, ps)
const scan = yield* collect(root, cwd, ps, shell)
if (!Instance.containsPath(cwd)) scan.dirs.add(cwd)
yield* ask(ctx, scan)

return yield* run(
{
shell,
name,
command: params.command,
cwd,
env: yield* shellEnv(ctx, cwd),
timeout,
description: params.description,
},
ctx,
)
}),
}
}
return () =>
Effect.sync(() => {
const shell = Shell.acceptable()
const name = Shell.name(shell)
const chain =
name === "powershell"
? "If the commands depend on each other and must run sequentially, avoid '&&' in this shell because Windows PowerShell 5.1 does not support it. Use PowerShell conditionals such as `cmd1; if ($?) { cmd2 }` when later commands must depend on earlier success."
: "If the commands depend on each other and must run sequentially, use a single Bash call with '&&' to chain them together (e.g., `git add . && git commit -m \"message\" && git push`). For instance, if one operation must complete before another starts (like mkdir before cp, Write before Bash for git operations, or git add before git commit), run these operations sequentially instead."
log.info("bash tool using shell", { shell })

return {
description: DESCRIPTION.replaceAll("${directory}", Instance.directory)
.replaceAll("${os}", process.platform)
.replaceAll("${shell}", name)
.replaceAll("${chaining}", chain)
.replaceAll("${maxLines}", String(Truncate.MAX_LINES))
.replaceAll("${maxBytes}", String(Truncate.MAX_BYTES)),
parameters: Parameters,
execute: (params: z.infer<typeof Parameters>, ctx: Tool.Context) =>
Effect.gen(function* () {
const cwd = params.workdir
? yield* resolvePath(params.workdir, Instance.directory, shell)
: Instance.directory
if (params.timeout !== undefined && params.timeout < 0) {
throw new Error(`Invalid timeout value: ${params.timeout}. Timeout must be a positive number.`)
}
const timeout = params.timeout ?? DEFAULT_TIMEOUT
const ps = PS.has(name)
const root = yield* parse(params.command, ps)
const scan = yield* collect(root, cwd, ps, shell)
if (!Instance.containsPath(cwd)) scan.dirs.add(cwd)
yield* ask(ctx, scan)

return yield* run(
{
shell,
name,
command: params.command,
cwd,
env: yield* shellEnv(ctx, cwd),
timeout,
description: params.description,
},
ctx,
)
}),
}
})
}),
)
2 changes: 1 addition & 1 deletion packages/opencode/src/tool/multiedit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export const MultiEditTool = Tool.define(
"multiedit",
Effect.gen(function* () {
const editInfo = yield* EditTool
const edit = yield* Effect.promise(() => editInfo.init())
const edit = yield* editInfo.init()

return {
description: DESCRIPTION,
Expand Down
144 changes: 72 additions & 72 deletions packages/opencode/src/tool/skill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,84 +17,84 @@ export const SkillTool = Tool.define(
Effect.gen(function* () {
const skill = yield* Skill.Service
const rg = yield* Ripgrep.Service
return () =>
Effect.gen(function* () {
const list = yield* skill.available().pipe(Effect.provide(EffectLogger.layer))

return async () => {
const list = await Effect.runPromise(skill.available().pipe(Effect.provide(EffectLogger.layer)))

const description =
list.length === 0
? "Load a specialized skill that provides domain-specific instructions and workflows. No skills are currently available."
: [
"Load a specialized skill that provides domain-specific instructions and workflows.",
"",
"When you recognize that a task matches one of the available skills listed below, use this tool to load the full skill instructions.",
"",
"The skill will inject detailed instructions, workflows, and access to bundled resources (scripts, references, templates) into the conversation context.",
"",
'Tool output includes a `<skill_content name="...">` block with the loaded content.',
"",
"The following skills provide specialized sets of instructions for particular tasks",
"Invoke this tool to load a skill when a task matches one of the available skills listed below:",
"",
Skill.fmt(list, { verbose: false }),
].join("\n")
const description =
list.length === 0
? "Load a specialized skill that provides domain-specific instructions and workflows. No skills are currently available."
: [
"Load a specialized skill that provides domain-specific instructions and workflows.",
"",
"When you recognize that a task matches one of the available skills listed below, use this tool to load the full skill instructions.",
"",
"The skill will inject detailed instructions, workflows, and access to bundled resources (scripts, references, templates) into the conversation context.",
"",
'Tool output includes a `<skill_content name="...">` block with the loaded content.',
"",
"The following skills provide specialized sets of instructions for particular tasks",
"Invoke this tool to load a skill when a task matches one of the available skills listed below:",
"",
Skill.fmt(list, { verbose: false }),
].join("\n")

return {
description,
parameters: Parameters,
execute: (params: z.infer<typeof Parameters>, ctx: Tool.Context) =>
Effect.gen(function* () {
const info = yield* skill.get(params.name)
return {
description,
parameters: Parameters,
execute: (params: z.infer<typeof Parameters>, ctx: Tool.Context) =>
Effect.gen(function* () {
const info = yield* skill.get(params.name)

if (!info) {
const all = yield* skill.all()
const available = all.map((s) => s.name).join(", ")
throw new Error(`Skill "${params.name}" not found. Available skills: ${available || "none"}`)
}
if (!info) {
const all = yield* skill.all()
const available = all.map((s) => s.name).join(", ")
throw new Error(`Skill "${params.name}" not found. Available skills: ${available || "none"}`)
}

yield* ctx.ask({
permission: "skill",
patterns: [params.name],
always: [params.name],
metadata: {},
})
yield* ctx.ask({
permission: "skill",
patterns: [params.name],
always: [params.name],
metadata: {},
})

const dir = path.dirname(info.location)
const base = pathToFileURL(dir).href
const dir = path.dirname(info.location)
const base = pathToFileURL(dir).href

const limit = 10
const files = yield* rg.files({ cwd: dir, follow: false, hidden: true }).pipe(
Stream.filter((file) => !file.includes("SKILL.md")),
Stream.map((file) => path.resolve(dir, file)),
Stream.take(limit),
Stream.runCollect,
Effect.map((chunk) => [...chunk].map((file) => `<file>${file}</file>`).join("\n")),
)
const limit = 10
const files = yield* rg.files({ cwd: dir, follow: false, hidden: true }).pipe(
Stream.filter((file) => !file.includes("SKILL.md")),
Stream.map((file) => path.resolve(dir, file)),
Stream.take(limit),
Stream.runCollect,
Effect.map((chunk) => [...chunk].map((file) => `<file>${file}</file>`).join("\n")),
)

return {
title: `Loaded skill: ${info.name}`,
output: [
`<skill_content name="${info.name}">`,
`# Skill: ${info.name}`,
"",
info.content.trim(),
"",
`Base directory for this skill: ${base}`,
"Relative paths in this skill (e.g., scripts/, reference/) are relative to this base directory.",
"Note: file list is sampled.",
"",
"<skill_files>",
files,
"</skill_files>",
"</skill_content>",
].join("\n"),
metadata: {
name: info.name,
dir,
},
}
}).pipe(Effect.orDie),
}
}
return {
title: `Loaded skill: ${info.name}`,
output: [
`<skill_content name="${info.name}">`,
`# Skill: ${info.name}`,
"",
info.content.trim(),
"",
`Base directory for this skill: ${base}`,
"Relative paths in this skill (e.g., scripts/, reference/) are relative to this base directory.",
"Note: file list is sampled.",
"",
"<skill_files>",
files,
"</skill_files>",
"</skill_content>",
].join("\n"),
metadata: {
name: info.name,
dir,
},
}
}).pipe(Effect.orDie),
}
})
}),
)
Loading
Loading