Skip to content

Commit f2525a6

Browse files
authored
add experimental question HttpApi slice (#22357)
1 parent 8c42d39 commit f2525a6

10 files changed

Lines changed: 549 additions & 83 deletions

File tree

packages/opencode/specs/effect/http-api.md

Lines changed: 263 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,19 @@ Introduce one small `HttpApi` group for plain JSON endpoints only. Good initial
104104

105105
Avoid `session.ts`, SSE, websocket, and TUI-facing routes first.
106106

107+
Recommended first slice:
108+
109+
- start with `question`
110+
- start with `GET /question`
111+
- start with `POST /question/:requestID/reply`
112+
113+
Why `question` first:
114+
115+
- already JSON-only
116+
- already delegates into an Effect service
117+
- proves list + mutation + params + payload + OpenAPI in one small slice
118+
- avoids the harder streaming and middleware cases
119+
107120
### 3. Reuse existing services
108121

109122
Do not re-architect business logic during the HTTP migration. `HttpApi` handlers should call the same Effect services already used by the Hono handlers.
@@ -121,13 +134,257 @@ Prefer mounting an experimental `HttpApi` surface alongside the existing Hono ro
121134

122135
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.
123136

124-
## Proposed first steps
137+
## Schema rule for HttpApi work
138+
139+
Every `HttpApi` slice should follow `specs/effect/schema.md` and the Schema -> Zod interop rule in `specs/effect/migration.md`.
140+
141+
Default rule:
142+
143+
- Effect Schema owns the type
144+
- `.zod` exists only as a compatibility surface
145+
- do not introduce a new hand-written Zod schema for a type that is already migrating to Effect Schema
146+
147+
Practical implication for `HttpApi` migration:
148+
149+
- if a route boundary already depends on a shared DTO, ID, input, output, or tagged error, migrate that model to Effect Schema first or in the same change
150+
- if an existing Hono route or tool still needs Zod, derive it with `@/util/effect-zod`
151+
- avoid maintaining parallel Zod and Effect definitions for the same request or response type
152+
153+
Ordering for a route-group migration:
154+
155+
1. move implicated shared `schema.ts` leaf types to Effect Schema first
156+
2. move exported `Info` / `Input` / `Output` route DTOs to Effect Schema
157+
3. move tagged route-facing errors to `Schema.TaggedErrorClass` where needed
158+
4. switch existing Zod boundary validators to derived `.zod`
159+
5. define the `HttpApi` contract from the canonical Effect schemas
160+
161+
Temporary exception:
162+
163+
- it is acceptable to keep a route-local Zod schema for the first spike only when the type is boundary-local and migrating it would create unrelated churn
164+
- if that happens, leave a short note so the type does not become a permanent second source of truth
165+
166+
## First vertical slice
167+
168+
The first `HttpApi` spike should be intentionally small and repeatable.
169+
170+
Chosen slice:
171+
172+
- group: `question`
173+
- endpoints: `GET /question` and `POST /question/:requestID/reply`
174+
175+
Non-goals:
176+
177+
- no `session` routes
178+
- no SSE or websocket routes
179+
- no auth redesign
180+
- no broad service refactor
181+
182+
Behavior rule:
183+
184+
- preserve current runtime behavior first
185+
- treat semantic changes such as introducing new `404` behavior as a separate follow-up unless they are required to make the contract honest
186+
187+
Add `POST /question/:requestID/reject` only after the first two endpoints work cleanly.
188+
189+
## Repeatable slice template
190+
191+
Use the same sequence for each route group.
192+
193+
1. Pick one JSON-only route group that already mostly delegates into services.
194+
2. Identify the shared DTOs, IDs, and errors implicated by that slice.
195+
3. Apply the schema migration ordering above so those types are Effect Schema-first.
196+
4. Define the `HttpApi` contract separately from the handlers.
197+
5. Implement handlers by yielding the existing service from context.
198+
6. Mount the new surface in parallel under an experimental prefix.
199+
7. Add one end-to-end test and one OpenAPI-focused test.
200+
8. Compare ergonomics before migrating the next endpoint.
201+
202+
Rule of thumb:
203+
204+
- migrate one route group at a time
205+
- migrate one or two endpoints first, not the whole file
206+
- keep business logic in the existing service
207+
- keep the first spike easy to delete if the experiment is not worth continuing
208+
209+
## Example structure
210+
211+
Placement rule:
212+
213+
- keep `HttpApi` code under `src/server`, not `src/effect`
214+
- `src/effect` should stay focused on runtimes, layers, instance state, and shared Effect plumbing
215+
- place each `HttpApi` slice next to the HTTP boundary it serves
216+
- for instance-scoped routes, prefer `src/server/instance/httpapi/*`
217+
- if control-plane routes ever migrate, prefer `src/server/control/httpapi/*`
218+
219+
Suggested file layout for a repeatable spike:
220+
221+
- `src/server/instance/httpapi/question.ts`
222+
- `src/server/instance/httpapi/index.ts`
223+
- `test/server/question-httpapi.test.ts`
224+
- `test/server/question-httpapi-openapi.test.ts`
225+
226+
Suggested responsibilities:
227+
228+
- `question.ts` defines the `HttpApi` contract and `HttpApiBuilder.group(...)` handlers for the experimental slice
229+
- `index.ts` combines experimental `HttpApi` groups and exposes the mounted handler or layer
230+
- `question-httpapi.test.ts` proves the route works end-to-end against the real service
231+
- `question-httpapi-openapi.test.ts` proves the generated OpenAPI is acceptable for the migrated endpoints
232+
233+
## Example migration shape
234+
235+
Each route-group spike should follow the same shape.
236+
237+
### 1. Contract
238+
239+
- define an experimental `HttpApi`
240+
- define one `HttpApiGroup`
241+
- define endpoint params, payload, success, and error schemas from canonical Effect schemas
242+
- annotate summary, description, and operation ids explicitly so generated docs are stable
243+
244+
### 2. Handler layer
245+
246+
- implement with `HttpApiBuilder.group(api, groupName, ...)`
247+
- yield the existing Effect service from context
248+
- keep handler bodies thin
249+
- keep transport mapping at the HTTP boundary only
250+
251+
### 3. Mounting
252+
253+
- mount under an experimental prefix such as `/experimental/httpapi`
254+
- keep existing Hono routes unchanged
255+
- expose separate OpenAPI output for the experimental slice first
256+
257+
### 4. Verification
258+
259+
- seed real state through the existing service
260+
- call the experimental endpoints
261+
- assert that the service behavior is unchanged
262+
- assert that the generated OpenAPI contains the migrated paths and schemas
263+
264+
## Boundary composition
265+
266+
The first slices should keep the existing outer server composition and only replace the route contract and handler layer.
267+
268+
### Auth
269+
270+
- keep `AuthMiddleware` at the outer Hono app level
271+
- do not duplicate auth checks inside each `HttpApi` group for the first parallel slices
272+
- treat auth as an already-satisfied transport concern before the request reaches the `HttpApi` handler
273+
274+
Practical rule:
275+
276+
- if a route is currently protected by the shared server middleware stack, the experimental `HttpApi` route should stay mounted behind that same stack
277+
278+
### Instance and workspace lookup
279+
280+
- keep `WorkspaceRouterMiddleware` as the source of truth for resolving `directory`, `workspace`, and session-derived workspace context
281+
- let that middleware provide `Instance.current` and `WorkspaceContext` before the request reaches the `HttpApi` handler
282+
- keep the `HttpApi` handlers unaware of path-to-instance lookup details when the existing Hono middleware already handles them
283+
284+
Practical rule:
285+
286+
- `HttpApi` handlers should yield services from context and assume the correct instance has already been provided
287+
- only move instance lookup into the `HttpApi` layer if we later decide to migrate the outer middleware boundary itself
288+
289+
### Error mapping
290+
291+
- keep domain and service errors typed in the service layer
292+
- declare typed transport errors on the endpoint only when the route can actually return them intentionally
293+
- prefer explicit endpoint-level error schemas over relying on the outer Hono `ErrorMiddleware` for expected route behavior
294+
295+
Practical rule:
296+
297+
- request decoding failures should remain transport-level `400`s
298+
- storage or lookup failures that are part of the route contract should be declared as typed endpoint errors
299+
- unexpected defects can still fall through to the outer error middleware while the slice is experimental
300+
301+
For the current parallel slices, this means:
302+
303+
- auth still composes outside `HttpApi`
304+
- instance selection still composes outside `HttpApi`
305+
- success payloads should be schema-defined from canonical Effect schemas
306+
- known route errors should be modeled at the endpoint boundary incrementally instead of all at once
307+
308+
## Exit criteria for the spike
309+
310+
The first slice is successful if:
311+
312+
- the endpoints run in parallel with the current Hono routes
313+
- the handlers reuse the existing Effect service
314+
- request decoding and response shapes are schema-defined from canonical Effect schemas
315+
- any remaining Zod boundary usage is derived from `.zod` or clearly temporary
316+
- OpenAPI is generated from the `HttpApi` contract
317+
- the tests are straightforward enough that the next slice feels mechanical
318+
319+
## Learnings from the question slice
320+
321+
The first parallel `question` spike gave us a concrete pattern to reuse.
322+
323+
- `Schema.Class` works well for route DTOs such as `Question.Request`, `Question.Info`, and `Question.Reply`.
324+
- scalar or collection schemas such as `Question.Answer` should stay as schemas and use helpers like `withStatics(...)` instead of being forced into classes.
325+
- if an `HttpApi` success schema uses `Schema.Class`, the handler or underlying service needs to return real schema instances rather than plain objects.
326+
- internal event payloads can stay anonymous when we want to avoid adding extra named OpenAPI component churn for non-route shapes.
327+
- the experimental slice should stay mounted in parallel and keep calling the existing service layer unchanged.
328+
- compare generated OpenAPI semantically at the route and schema level; in the current setup the exported OpenAPI paths do not include the outer Hono mount prefix.
329+
330+
## Route inventory
331+
332+
Status legend:
333+
334+
- `done` - parallel `HttpApi` slice exists
335+
- `next` - good near-term candidate
336+
- `later` - possible, but not first wave
337+
- `defer` - not a good early `HttpApi` target
338+
339+
Current instance route inventory:
125340

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
341+
- `question` - `done`
342+
endpoints in slice: `GET /question`, `POST /question/:requestID/reply`
343+
- `permission` - `done`
344+
endpoints in slice: `GET /permission`, `POST /permission/:requestID/reply`
345+
- `provider` - `next`
346+
best next endpoint: `GET /provider/auth`
347+
later endpoint: `GET /provider`
348+
defer first-wave OAuth mutations
349+
- `config` - `next`
350+
best next endpoint: `GET /config/providers`
351+
later endpoint: `GET /config`
352+
defer `PATCH /config` for now
353+
- `project` - `later`
354+
best small reads: `GET /project`, `GET /project/current`
355+
defer git-init mutation first
356+
- `workspace` - `later`
357+
best small reads: `GET /experimental/workspace/adaptor`, `GET /experimental/workspace`, `GET /experimental/workspace/status`
358+
defer create/remove mutations first
359+
- `file` - `later`
360+
good JSON-only candidate set, but larger than the current first-wave slices
361+
- `mcp` - `later`
362+
has JSON-only endpoints, but interactive OAuth/auth flows make it a worse early fit
363+
- `session` - `defer`
364+
large, stateful, mixes CRUD with prompt/shell/command/share/revert flows and a streaming route
365+
- `event` - `defer`
366+
SSE only
367+
- `global` - `defer`
368+
mixed bag with SSE and process-level side effects
369+
- `pty` - `defer`
370+
websocket-heavy route surface
371+
- `tui` - `defer`
372+
queue-style UI bridge, weak early `HttpApi` fit
373+
374+
Recommended near-term sequence after the first spike:
375+
376+
1. `provider` auth read endpoint
377+
2. `config` providers read endpoint
378+
3. `project` read endpoints
379+
4. `workspace` read endpoints
380+
381+
## Checklist
382+
383+
- [x] add one small spike that defines an `HttpApi` group for a simple JSON route set
384+
- [x] use Effect Schema request / response types for that slice
385+
- [x] keep the underlying service calls identical to the current handlers
386+
- [x] compare generated OpenAPI against the current Hono/OpenAPI setup
387+
- [x] document how auth, instance lookup, and error mapping would compose in the new stack
131388
- [ ] decide after the spike whether `HttpApi` should stay parallel, replace only some groups, or become the long-term default
132389

133390
## Rule of thumb

packages/opencode/src/cli/cmd/tui/routes/session/index.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2195,7 +2195,7 @@ function Question(props: ToolProps<typeof QuestionTool>) {
21952195
const { theme } = useTheme()
21962196
const count = createMemo(() => props.input.questions?.length ?? 0)
21972197

2198-
function format(answer?: string[]) {
2198+
function format(answer?: ReadonlyArray<string>) {
21992199
if (!answer?.length) return "(no answer)"
22002200
return answer.join(", ")
22012201
}

0 commit comments

Comments
 (0)