mirror of
https://github.com/anomalyco/opencode.git
synced 2026-05-30 00:00:29 +00:00
refactor: finish small effect service adoption cleanups (#22094)
This commit is contained in:
@@ -178,7 +178,9 @@ That is fine for leaf files like `schema.ts`. Keep the service surface in the ow
|
||||
|
||||
## Migration checklist
|
||||
|
||||
Fully migrated (single namespace, InstanceState where needed, flattened facade):
|
||||
Service-shape migrated (single namespace, traced methods, `InstanceState` where needed).
|
||||
|
||||
This checklist is only about the service shape migration. Many of these services still keep `makeRuntime(...)` plus async facade exports; that facade-removal phase is tracked separately in [Destroying the facades](#destroying-the-facades).
|
||||
|
||||
- [x] `Account` — `account/index.ts`
|
||||
- [x] `Agent` — `agent/agent.ts`
|
||||
@@ -221,20 +223,22 @@ Fully migrated (single namespace, InstanceState where needed, flattened facade):
|
||||
- [x] `Provider` — `provider/provider.ts`
|
||||
- [x] `Storage` — `storage/storage.ts`
|
||||
- [x] `ShareNext` — `share/share-next.ts`
|
||||
|
||||
Still open:
|
||||
|
||||
- [x] `SessionTodo` — `session/todo.ts`
|
||||
- [ ] `SyncEvent` — `sync/index.ts`
|
||||
- [ ] `Workspace` — `control-plane/workspace.ts`
|
||||
|
||||
Still open at the service-shape level:
|
||||
|
||||
- [ ] `SyncEvent` — `sync/index.ts` (deferred pending sync with James)
|
||||
- [ ] `Workspace` — `control-plane/workspace.ts` (deferred pending sync with James)
|
||||
|
||||
## Tool interface → Effect
|
||||
|
||||
`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:
|
||||
`Tool.Def.execute` and `Tool.Info.init` already return `Effect` on this branch, and the current tools in `src/tool/*.ts` have been migrated to the Effect-native `Tool.define(...)` shape.
|
||||
|
||||
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
|
||||
The remaining work here is follow-on cleanup rather than the top-level tool interface migration:
|
||||
|
||||
1. Remove internal `Effect.promise(...)` bridges where practical
|
||||
2. Keep replacing raw platform helpers with Effect services inside tool bodies
|
||||
3. Update remaining callers and tests to prefer `yield* info.init()` / `Tool.init(...)` over older Promise-oriented patterns
|
||||
|
||||
### Tool migration details
|
||||
|
||||
@@ -254,26 +258,27 @@ This keeps migrated tool tests aligned with the production service graph today,
|
||||
|
||||
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
|
||||
- [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
|
||||
- [ ] `codesearch.ts` — MEDIUM: HTTP + SSE + manual timeout → HttpClient + Effect.timeout
|
||||
- [ ] `webfetch.ts` — MEDIUM: fetch with UA retry, size limits → HttpClient
|
||||
- [ ] `websearch.ts` — MEDIUM: MCP over HTTP → HttpClient
|
||||
- [ ] `batch.ts` — MEDIUM: parallel execution, per-call error recovery → Effect.all
|
||||
- [ ] `task.ts` — MEDIUM: task state management
|
||||
- [ ] `ls.ts` — MEDIUM: bounded directory listing over ripgrep-backed traversal
|
||||
- [ ] `multiedit.ts` — MEDIUM: sequential edit orchestration over `edit.ts`
|
||||
- [ ] `glob.ts` — LOW: simple async generator
|
||||
- [ ] `lsp.ts` — LOW: dispatch switch over LSP operations
|
||||
- [ ] `question.ts` — LOW: prompt wrapper
|
||||
- [ ] `skill.ts` — LOW: skill tool adapter
|
||||
- [ ] `todo.ts` — LOW: todo persistence wrapper
|
||||
- [ ] `invalid.ts` — LOW: invalid-tool fallback
|
||||
- [ ] `plan.ts` — LOW: plan file operations
|
||||
- [x] `apply_patch.ts` — HIGH: multi-step orchestration, error accumulation, Bus events
|
||||
- [x] `bash.ts` — HIGH: shell orchestration, quoting, timeout handling, output capture
|
||||
- [x] `read.ts` — HIGH: effectful interface migrated; still has raw fs/readline internals tracked below
|
||||
- [x] `edit.ts` — HIGH: multi-step diff/format/publish pipeline, FileWatcher lock
|
||||
- [x] `grep.ts` — MEDIUM: spawns ripgrep → ChildProcessSpawner, timeout handling
|
||||
- [x] `write.ts` — MEDIUM: permission checks, diagnostics polling, Bus events
|
||||
- [x] `codesearch.ts` — MEDIUM: HTTP + SSE + manual timeout → HttpClient + Effect.timeout
|
||||
- [x] `webfetch.ts` — MEDIUM: fetch with UA retry, size limits → HttpClient
|
||||
- [x] `websearch.ts` — MEDIUM: MCP over HTTP → HttpClient
|
||||
- [x] `task.ts` — MEDIUM: task state management
|
||||
- [x] `ls.ts` — MEDIUM: bounded directory listing over ripgrep-backed traversal
|
||||
- [x] `multiedit.ts` — MEDIUM: sequential edit orchestration over `edit.ts`
|
||||
- [x] `glob.ts` — LOW: simple async generator
|
||||
- [x] `lsp.ts` — LOW: dispatch switch over LSP operations
|
||||
- [x] `question.ts` — LOW: prompt wrapper
|
||||
- [x] `skill.ts` — LOW: skill tool adapter
|
||||
- [x] `todo.ts` — LOW: todo persistence wrapper
|
||||
- [x] `invalid.ts` — LOW: invalid-tool fallback
|
||||
- [x] `plan.ts` — LOW: plan file operations
|
||||
|
||||
`batch.ts` was removed from `src/tool/` and is no longer tracked here.
|
||||
|
||||
## Effect service adoption in already-migrated code
|
||||
|
||||
@@ -281,25 +286,21 @@ Some already-effectified areas still use raw `Filesystem.*` or `Process.spawn` i
|
||||
|
||||
### `Filesystem.*` → `AppFileSystem.Service` (yield in layer)
|
||||
|
||||
- [ ] `file/index.ts` — 1 remaining `Filesystem.readText()` call in untracked diff handling
|
||||
- [ ] `config/config.ts` — 5 remaining `Filesystem.*` calls in `installDependencies()`
|
||||
- [ ] `provider/provider.ts` — 1 remaining `Filesystem.readJson()` call for recent model state
|
||||
- [x] `config/config.ts` — `installDependencies()` now uses `AppFileSystem`
|
||||
- [x] `provider/provider.ts` — recent model state now reads via `AppFileSystem.Service`
|
||||
|
||||
### `Process.spawn` → `ChildProcessSpawner` (yield in layer)
|
||||
|
||||
- [ ] `format/formatter.ts` — 2 remaining `Process.spawn()` checks (`air`, `uv`)
|
||||
- [x] `format/formatter.ts` — direct `Process.spawn()` checks removed (`air`, `uv`)
|
||||
- [ ] `lsp/server.ts` — multiple `Process.spawn()` installs/download helpers
|
||||
|
||||
## Filesystem consolidation
|
||||
|
||||
`util/filesystem.ts` (raw fs wrapper) is currently imported by **34 files**. The effectified `AppFileSystem` service (`filesystem/index.ts`) is currently imported by **15 files**. As services and tools are effectified, they should switch from `Filesystem.*` to yielding `AppFileSystem.Service` — this happens naturally during each migration, not as a separate effort.
|
||||
|
||||
Similarly, **21 files** still import raw `fs` or `fs/promises` directly. These should migrate to `AppFileSystem` or `Filesystem.*` as they're touched.
|
||||
`util/filesystem.ts` is still used widely across `src/`, and raw `fs` / `fs/promises` imports still exist in multiple tooling and infrastructure files. As services and tools are effectified, they should switch from `Filesystem.*` to yielding `AppFileSystem.Service` where possible — this should happen naturally during each migration, not as a separate sweep.
|
||||
|
||||
Current raw fs users that will convert during tool migration:
|
||||
|
||||
- `tool/read.ts` — fs.createReadStream, readline
|
||||
- `tool/apply_patch.ts` — fs/promises
|
||||
- `file/ripgrep.ts` — fs/promises
|
||||
- `patch/index.ts` — fs, fs/promises
|
||||
|
||||
@@ -312,7 +313,9 @@ Current raw fs users that will convert during tool migration:
|
||||
|
||||
## Destroying the facades
|
||||
|
||||
Every service currently exports async facade functions at the bottom of its namespace — `export async function read(...) { return runPromise(...) }` — backed by a per-service `makeRuntime`. These exist because cyclic imports used to force each service to build its own independent runtime. Now that the layer DAG is acyclic and `AppRuntime` (`src/effect/app-runtime.ts`) composes everything into one `ManagedRuntime`, we're removing them.
|
||||
This phase is still broadly open. As of 2026-04-11 there are still 31 `makeRuntime(...)` call sites under `src/`, and many service namespaces still export async facade helpers like `export async function read(...) { return runPromise(...) }`.
|
||||
|
||||
These facades exist because cyclic imports used to force each service to build its own independent runtime. Now that the layer DAG is acyclic and `AppRuntime` (`src/effect/app-runtime.ts`) composes everything into one `ManagedRuntime`, we're removing them.
|
||||
|
||||
### Process
|
||||
|
||||
|
||||
Reference in New Issue
Block a user