refactor(tool): Tool.Def.execute returns Effect, rename defineEffect → define#21961
Merged
kitlangton merged 9 commits intodevfrom Apr 11, 2026
Merged
refactor(tool): Tool.Def.execute returns Effect, rename defineEffect → define#21961kitlangton merged 9 commits intodevfrom
kitlangton merged 9 commits intodevfrom
Conversation
kitlangton
force-pushed
the
fix-skill
branch
2 times, most recently
from
36b6510 to
ee9ec7c
Compare
…→ define - Change Tool.Def.execute return type from Promise to Effect.Effect - Rename Tool.defineEffect to Tool.define, delete old Promise-based Tool.define - Remove Effect.runPromise from all 18 tool execute boundaries - Update wrap() to use Effect.gen for validation and truncation - Convert prompt.ts dispatch to yield* item.execute() directly - Wrap plugin tool execute in Effect.promise in registry - Wire AbortController + Effect.onInterrupt for execRead abort propagation - Update all tool tests to use Effect.runPromise(tool.execute(...))
The bash tool needs ctx.abort to handle user cancellation gracefully — returning partial output through normal truncation. Pure Effect interruption can't produce a return value (effect-smol's Effect.exit doesn't consume interrupts). Moving to Effect interruption for bash requires taking tool dispatch from the AI SDK (forkChild), which is a separate change.
The task tool passes ctx.abort to sub-sessions. Without a real signal, bash in sub-sessions can't handle cancellation. Use AbortController + onInterrupt to fire the signal when the task fiber is interrupted.
Remove Effect.promise wrappers around ctx.ask() in all tools — now just yield* ctx.ask(...) directly. Update prompt.ts boundary to return Effect from permission.ask (with orDie). Plugin tool context converts back to Promise at the boundary. Update all test mocks.
…annel wrap() in tool.ts already applies Effect.orDie. Tools whose execute has typed errors (codesearch, webfetch, etc.) still need their own orDie to satisfy the Def.execute type. Tools with never errors (edit, multiedit, todo, lsp) had redundant orDie — removed. Also merge double .pipe() chain in prompt.ts subtask handling.
Close over Truncate.Service in registry layer gen and use truncate.output() directly instead of the static Truncate.output() facade. Also convert fromPlugin execute from Effect.promise to Effect.gen for cleaner service usage.
- FileTime.withLock now accepts () => Effect<T> instead of () => Promise<T> - edit.ts withLock callback converted from async to Effect.gen, uses AppFileSystem.Service, Format.Service, and FileTime.Service directly - edit/write/apply_patch tools use Bus.Service instead of Bus.publish facade - write.ts uses Format.Service instead of Format.file facade
Bare Effect.runPromise/runFork calls used Effect's default logger, causing raw pino-style log output to leak to the terminal during subtask execution. Capture the current ServiceMap via Effect.services() and use Effect.runPromiseWith/runForkWith so all sub-effects inherit the app's logger and other services.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Tool.Def.executereturn type fromPromisetoEffect.EffectTool.defineEffect→Tool.define, delete old Promise-basedTool.defineEffect.runPromisefrom all 18 tool execute boundarieswrap()to useEffect.genfor validation and truncationprompt.tsdispatch toyield* item.execute()directlyEffect.promisein registryAbortController+Effect.onInterruptforexecReadabort propagationBash keeps
ctx.abort+Effect.callbackrace for cancel support — the full output needs to go through normal truncation, which requires the tool to return a value. Moving to pure Effect interruption for bash requires taking tool dispatch from the AI SDK (forkChild), tracked separately.Test plan
bun run typecheckcleanbun run test test/tool/— 167 passbun run test test/session/prompt-effect.test.ts— 34 pass