Skip to content

Commit 6fdb8ab

Browse files
authored
refactor(file): add ripgrep search service (#22295)
1 parent 321bf1f commit 6fdb8ab

12 files changed

Lines changed: 665 additions & 348 deletions

File tree

packages/opencode/AGENTS.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
Use these rules when writing or migrating Effect code.
1515

16-
See `specs/effect-migration.md` for the compact pattern reference and examples.
16+
See `specs/effect/migration.md` for the compact pattern reference and examples.
1717

1818
## Core
1919

@@ -51,7 +51,7 @@ See `specs/effect-migration.md` for the compact pattern reference and examples.
5151

5252
## Effect.cached for deduplication
5353

54-
Use `Effect.cached` when multiple concurrent callers should share a single in-flight computation rather than storing `Fiber | undefined` or `Promise | undefined` manually. See `specs/effect-migration.md` for the full pattern.
54+
Use `Effect.cached` when multiple concurrent callers should share a single in-flight computation rather than storing `Fiber | undefined` or `Promise | undefined` manually. See `specs/effect/migration.md` for the full pattern.
5555

5656
## Instance.bind — ALS for native callbacks
5757

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
# HttpApi migration
2+
3+
Practical notes for an eventual migration of `packages/opencode` server routes from the current Hono handlers to Effect `HttpApi`, either as a full replacement or as a parallel surface.
4+
5+
## Goal
6+
7+
Use Effect `HttpApi` where it gives us a better typed contract for:
8+
9+
- route definition
10+
- request decoding and validation
11+
- typed success and error responses
12+
- OpenAPI generation
13+
- handler composition inside Effect
14+
15+
This should be treated as a later-stage HTTP boundary migration, not a prerequisite for ongoing service, route-handler, or schema work.
16+
17+
## Core model
18+
19+
`HttpApi` is definition-first.
20+
21+
- `HttpApi` is the root API
22+
- `HttpApiGroup` groups related endpoints
23+
- `HttpApiEndpoint` defines a single route and its request / response schemas
24+
- handlers are implemented separately from the contract
25+
26+
This is a better fit once route inputs and outputs are already moving toward Effect Schema-first models.
27+
28+
## Why it is relevant here
29+
30+
The current route-effectification work is already pushing handlers toward:
31+
32+
- one `AppRuntime.runPromise(Effect.gen(...))` body
33+
- yielding services from context
34+
- using typed Effect errors instead of Promise wrappers
35+
36+
That work is a good prerequisite for `HttpApi`. Once the handler body is already a composed Effect, the remaining migration is mostly about replacing the Hono route declaration and validator layer.
37+
38+
## What HttpApi gives us
39+
40+
### Contracts
41+
42+
Request params, query, payload, success payloads, and typed error payloads are declared in one place using Effect Schema.
43+
44+
### Validation and decoding
45+
46+
Incoming data is decoded through Effect Schema instead of hand-maintained Zod validators per route.
47+
48+
### OpenAPI
49+
50+
`HttpApi` can derive OpenAPI from the API definition, which overlaps with the current `describeRoute(...)` and `resolver(...)` pattern.
51+
52+
### Typed errors
53+
54+
`Schema.TaggedErrorClass` maps naturally to endpoint error contracts.
55+
56+
## Likely fit for opencode
57+
58+
Best fit first:
59+
60+
- JSON request / response endpoints
61+
- route groups that already mostly delegate into services
62+
- endpoints whose request and response models can be defined with Effect Schema
63+
64+
Harder / later fit:
65+
66+
- SSE endpoints
67+
- websocket endpoints
68+
- streaming handlers
69+
- routes with heavy Hono-specific middleware assumptions
70+
71+
## Current blockers and gaps
72+
73+
### Schema split
74+
75+
Many route boundaries still use Zod-first validators. That does not block all experimentation, but full `HttpApi` adoption is easier after the domain and boundary types are more consistently Schema-first with `.zod` compatibility only where needed.
76+
77+
### Mixed handler styles
78+
79+
Many current `server/instance/*.ts` handlers still call async facades directly. Migrating those to composed `Effect.gen(...)` handlers is the low-risk step to do first.
80+
81+
### Non-JSON routes
82+
83+
The server currently includes SSE, websocket, and streaming-style endpoints. Those should not be the first `HttpApi` targets.
84+
85+
### Existing Hono integration
86+
87+
The current server composition, middleware, and docs flow are Hono-centered today. That suggests a parallel or incremental adoption plan is safer than a flag day rewrite.
88+
89+
## Recommended strategy
90+
91+
### 1. Finish the prerequisites first
92+
93+
- continue route-handler effectification in `server/instance/*.ts`
94+
- continue schema migration toward Effect Schema-first DTOs and errors
95+
- keep removing service facades
96+
97+
### 2. Start with one parallel group
98+
99+
Introduce one small `HttpApi` group for plain JSON endpoints only. Good initial candidates are the least stateful endpoints in:
100+
101+
- `server/instance/question.ts`
102+
- `server/instance/provider.ts`
103+
- `server/instance/permission.ts`
104+
105+
Avoid `session.ts`, SSE, websocket, and TUI-facing routes first.
106+
107+
### 3. Reuse existing services
108+
109+
Do not re-architect business logic during the HTTP migration. `HttpApi` handlers should call the same Effect services already used by the Hono handlers.
110+
111+
### 4. Run in parallel before replacing
112+
113+
Prefer mounting an experimental `HttpApi` surface alongside the existing Hono routes first. That lowers migration risk and lets us compare:
114+
115+
- handler ergonomics
116+
- OpenAPI output
117+
- auth and middleware integration
118+
- test ergonomics
119+
120+
### 5. Migrate JSON route groups gradually
121+
122+
If the parallel slice works well, migrate additional JSON route groups one at a time. Leave streaming-style endpoints on Hono until there is a clear reason to move them.
123+
124+
## Proposed first steps
125+
126+
- [ ] add one small spike that defines an `HttpApi` group for a simple JSON route set
127+
- [ ] use Effect Schema request / response types for that slice
128+
- [ ] keep the underlying service calls identical to the current handlers
129+
- [ ] compare generated OpenAPI against the current Hono/OpenAPI setup
130+
- [ ] document how auth, instance lookup, and error mapping would compose in the new stack
131+
- [ ] decide after the spike whether `HttpApi` should stay parallel, replace only some groups, or become the long-term default
132+
133+
## Rule of thumb
134+
135+
Do not start with the hardest route file.
136+
137+
If `HttpApi` is adopted here, it should arrive after the handler body is already Effect-native and after the relevant request / response models have moved to Effect Schema.

packages/opencode/specs/effect-migration.md renamed to packages/opencode/specs/effect/migration.md

Lines changed: 7 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -230,55 +230,9 @@ Still open at the service-shape level:
230230
- [ ] `SyncEvent``sync/index.ts` (deferred pending sync with James)
231231
- [ ] `Workspace``control-plane/workspace.ts` (deferred pending sync with James)
232232

233-
## Tool interface → Effect
233+
## Tool migration
234234

235-
`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.
236-
237-
The remaining work here is follow-on cleanup rather than the top-level tool interface migration:
238-
239-
1. Remove internal `Effect.promise(...)` bridges where practical
240-
2. Keep replacing raw platform helpers with Effect services inside tool bodies
241-
3. Update remaining callers and tests to prefer `yield* info.init()` / `Tool.init(...)` over older Promise-oriented patterns
242-
243-
### Tool migration details
244-
245-
With `Tool.Info.init()` now effectful, use this transitional pattern for migrated tools that still need Promise-based boundaries internally:
246-
247-
- `Tool.defineEffect(...)` should `yield*` the services the tool depends on and close over them in the returned tool definition.
248-
- 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()`.
249-
- If a tool starts requiring new services, wire them into `ToolRegistry.defaultLayer` so production callers resolve the same dependencies as tests.
250-
251-
Tool tests should use the existing Effect helpers in `packages/opencode/test/lib/effect.ts`:
252-
253-
- Use `testEffect(...)` / `it.live(...)` instead of creating fake local wrappers around effectful tools.
254-
- Yield the real tool export, then initialize it: `const info = yield* ReadTool`, `const tool = yield* info.init()`.
255-
- Run tests inside a real instance with `provideTmpdirInstance(...)` or `provideInstance(tmpdirScoped(...))` so instance-scoped services resolve exactly as they do in production.
256-
257-
This keeps migrated tool tests aligned with the production service graph today, and makes the eventual `Tool.Info``Effect` cleanup mostly mechanical later.
258-
259-
Individual tools, ordered by value:
260-
261-
- [x] `apply_patch.ts` — HIGH: multi-step orchestration, error accumulation, Bus events
262-
- [x] `bash.ts` — HIGH: shell orchestration, quoting, timeout handling, output capture
263-
- [x] `read.ts` — HIGH: effectful interface migrated; still has raw fs/readline internals tracked below
264-
- [x] `edit.ts` — HIGH: multi-step diff/format/publish pipeline, FileWatcher lock
265-
- [x] `grep.ts` — MEDIUM: spawns ripgrep → ChildProcessSpawner, timeout handling
266-
- [x] `write.ts` — MEDIUM: permission checks, diagnostics polling, Bus events
267-
- [x] `codesearch.ts` — MEDIUM: HTTP + SSE + manual timeout → HttpClient + Effect.timeout
268-
- [x] `webfetch.ts` — MEDIUM: fetch with UA retry, size limits → HttpClient
269-
- [x] `websearch.ts` — MEDIUM: MCP over HTTP → HttpClient
270-
- [x] `task.ts` — MEDIUM: task state management
271-
- [x] `ls.ts` — MEDIUM: bounded directory listing over ripgrep-backed traversal
272-
- [x] `multiedit.ts` — MEDIUM: sequential edit orchestration over `edit.ts`
273-
- [x] `glob.ts` — LOW: simple async generator
274-
- [x] `lsp.ts` — LOW: dispatch switch over LSP operations
275-
- [x] `question.ts` — LOW: prompt wrapper
276-
- [x] `skill.ts` — LOW: skill tool adapter
277-
- [x] `todo.ts` — LOW: todo persistence wrapper
278-
- [x] `invalid.ts` — LOW: invalid-tool fallback
279-
- [x] `plan.ts` — LOW: plan file operations
280-
281-
`batch.ts` was removed from `src/tool/` and is no longer tracked here.
235+
Tool-specific migration guidance and checklist live in `tools.md`.
282236

283237
## Effect service adoption in already-migrated code
284238

@@ -298,11 +252,7 @@ Some already-effectified areas still use raw `Filesystem.*` or `Process.spawn` i
298252

299253
`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.
300254

301-
Current raw fs users that will convert during tool migration:
302-
303-
- `tool/read.ts` — fs.createReadStream, readline
304-
- `file/ripgrep.ts` — fs/promises
305-
- `patch/index.ts` — fs, fs/promises
255+
Tool-specific filesystem cleanup notes live in `tools.md`.
306256

307257
## Primitives & utilities
308258

@@ -344,47 +294,14 @@ For each service, the migration is roughly:
344294
- `ShareNext` — migrated 2026-04-11. Swapped remaining async callers to `AppRuntime.runPromise(ShareNext.Service.use(...))`, removed the `makeRuntime(...)` facade, and kept instance bootstrap on the shared app runtime.
345295
- `SessionTodo` — migrated 2026-04-10. Already matched the target service shape in `session/todo.ts`: single namespace, traced Effect methods, and no `makeRuntime(...)` facade remained; checklist updated to reflect the completed migration.
346296
- `Storage` — migrated 2026-04-10. One production caller (`Session.diff`) and all storage.test.ts tests converted to effectful style. Facades and `makeRuntime` removed.
347-
- `SessionRunState` — migrated 2026-04-11. Single caller in `server/routes/session.ts` converted; facade removed.
348-
- `Account` — migrated 2026-04-11. Callers in `server/routes/experimental.ts` and `cli/cmd/account.ts` converted; facade removed.
297+
- `SessionRunState` — migrated 2026-04-11. Single caller in `server/instance/session.ts` converted; facade removed.
298+
- `Account` — migrated 2026-04-11. Callers in `server/instance/experimental.ts` and `cli/cmd/account.ts` converted; facade removed.
349299
- `Instruction` — migrated 2026-04-11. Test-only callers converted; facade removed.
350300
- `FileTime` — migrated 2026-04-11. Test-only callers converted; facade removed.
351301
- `FileWatcher` — migrated 2026-04-11. Callers in `project/bootstrap.ts` and test converted; facade removed.
352-
- `Question` — migrated 2026-04-11. Callers in `server/routes/question.ts` and test converted; facade removed.
302+
- `Question` — migrated 2026-04-11. Callers in `server/instance/question.ts` and test converted; facade removed.
353303
- `Truncate` — migrated 2026-04-11. Caller in `tool/tool.ts` and test converted; facade removed.
354304

355305
## Route handler effectification
356306

357-
Route handlers should wrap their entire body in a single `AppRuntime.runPromise(Effect.gen(...))` call, yielding services from context rather than calling facades one-by-one. This eliminates multiple `runPromise` round-trips and lets handlers compose naturally.
358-
359-
```ts
360-
// Before — one facade call per service
361-
;async (c) => {
362-
await SessionRunState.assertNotBusy(id)
363-
await Session.removeMessage({ sessionID: id, messageID })
364-
return c.json(true)
365-
}
366-
367-
// After — one Effect.gen, yield services from context
368-
;async (c) => {
369-
await AppRuntime.runPromise(
370-
Effect.gen(function* () {
371-
const state = yield* SessionRunState.Service
372-
const session = yield* Session.Service
373-
yield* state.assertNotBusy(id)
374-
yield* session.removeMessage({ sessionID: id, messageID })
375-
}),
376-
)
377-
return c.json(true)
378-
}
379-
```
380-
381-
When migrating, always use `{ concurrency: "unbounded" }` with `Effect.all` — route handlers should run independent service calls in parallel, not sequentially.
382-
383-
Route files to convert (each handler that calls facades should be wrapped):
384-
385-
- [ ] `server/routes/session.ts` — heaviest; uses Session, SessionPrompt, SessionRevert, SessionCompaction, SessionShare, SessionSummary, SessionRunState, Agent, Permission, Bus
386-
- [ ] `server/routes/global.ts` — uses Config, Project, Provider, Vcs, Snapshot, Agent
387-
- [ ] `server/routes/provider.ts` — uses Provider, Auth, Config
388-
- [ ] `server/routes/question.ts` — uses Question
389-
- [ ] `server/routes/pty.ts` — uses Pty
390-
- [ ] `server/routes/experimental.ts` — uses Account, ToolRegistry, Agent, MCP, Config
307+
Route-handler migration guidance and checklist live in `routes.md`.
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
# Route handler effectification
2+
3+
Practical reference for converting server route handlers in `packages/opencode` to a single `AppRuntime.runPromise(Effect.gen(...))` body.
4+
5+
## Goal
6+
7+
Route handlers should wrap their entire body in a single `AppRuntime.runPromise(Effect.gen(...))` call, yielding services from context rather than calling facades one-by-one.
8+
9+
This eliminates multiple `runPromise` round-trips and lets handlers compose naturally.
10+
11+
```ts
12+
// Before - one facade call per service
13+
;async (c) => {
14+
await SessionRunState.assertNotBusy(id)
15+
await Session.removeMessage({ sessionID: id, messageID })
16+
return c.json(true)
17+
}
18+
19+
// After - one Effect.gen, yield services from context
20+
;async (c) => {
21+
await AppRuntime.runPromise(
22+
Effect.gen(function* () {
23+
const state = yield* SessionRunState.Service
24+
const session = yield* Session.Service
25+
yield* state.assertNotBusy(id)
26+
yield* session.removeMessage({ sessionID: id, messageID })
27+
}),
28+
)
29+
return c.json(true)
30+
}
31+
```
32+
33+
## Rules
34+
35+
- Wrap the whole handler body in one `AppRuntime.runPromise(Effect.gen(...))` call when the handler is service-heavy.
36+
- Yield services from context instead of calling async facades repeatedly.
37+
- When independent service calls can run in parallel, use `Effect.all(..., { concurrency: "unbounded" })`.
38+
- Prefer one composed Effect body over multiple separate `runPromise(...)` calls in the same handler.
39+
40+
## Current route files
41+
42+
Current instance route files live under `src/server/instance`, not `server/routes`.
43+
44+
The main migration targets are:
45+
46+
- [ ] `server/instance/session.ts` — heaviest; still has many direct facade calls for Session, SessionPrompt, SessionRevert, SessionCompaction, SessionShare, SessionSummary, Agent, Bus
47+
- [ ] `server/instance/global.ts` — still has direct facade calls for Config and instance lifecycle actions
48+
- [ ] `server/instance/provider.ts` — still has direct facade calls for Config and Provider
49+
- [ ] `server/instance/question.ts` — partially converted; still worth tracking here until it consistently uses the composed style
50+
- [ ] `server/instance/pty.ts` — still calls Pty facades directly
51+
- [ ] `server/instance/experimental.ts` — mixed state; some handlers are already composed, others still use facades
52+
53+
Additional route files that still participate in the migration:
54+
55+
- [ ] `server/instance/index.ts` — Vcs, Agent, Skill, LSP, Format
56+
- [ ] `server/instance/file.ts` — Ripgrep, File, LSP
57+
- [ ] `server/instance/mcp.ts` — MCP facade-heavy
58+
- [ ] `server/instance/permission.ts` — Permission
59+
- [ ] `server/instance/workspace.ts` — Workspace
60+
- [ ] `server/instance/tui.ts` — Bus and Session
61+
- [ ] `server/instance/middleware.ts` — Session and Workspace lookups
62+
63+
## Notes
64+
65+
- Some handlers already use `AppRuntime.runPromise(Effect.gen(...))` in isolated places. Keep pushing those files toward one consistent style.
66+
- Route conversion is closely tied to facade removal. As services lose `makeRuntime`-backed async exports, route handlers should switch to yielding the service directly.

0 commit comments

Comments
 (0)