From 7536d26f36ad2196e5150698797c72a73db94513 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Mon, 13 Apr 2026 19:23:58 -0400 Subject: [PATCH] add experimental question HttpApi slice --- packages/opencode/specs/effect/http-api.md | 151 +++++++++++++++++ packages/opencode/src/question/index.ts | 156 ++++++++++-------- .../src/server/instance/experimental.ts | 2 + .../src/server/instance/httpapi/index.ts | 7 + .../src/server/instance/httpapi/question.ts | 90 ++++++++++ .../opencode/src/server/instance/question.ts | 4 +- packages/opencode/src/tool/question.ts | 4 +- .../opencode/test/question/question.test.ts | 4 +- .../test/server/question-httpapi.test.ts | 78 +++++++++ 9 files changed, 425 insertions(+), 71 deletions(-) create mode 100644 packages/opencode/src/server/instance/httpapi/index.ts create mode 100644 packages/opencode/src/server/instance/httpapi/question.ts create mode 100644 packages/opencode/test/server/question-httpapi.test.ts diff --git a/packages/opencode/specs/effect/http-api.md b/packages/opencode/specs/effect/http-api.md index a18d805a3c..a0814a7d12 100644 --- a/packages/opencode/specs/effect/http-api.md +++ b/packages/opencode/specs/effect/http-api.md @@ -104,6 +104,19 @@ Introduce one small `HttpApi` group for plain JSON endpoints only. Good initial Avoid `session.ts`, SSE, websocket, and TUI-facing routes first. +Recommended first slice: + +- start with `question` +- start with `GET /question` +- start with `POST /question/:requestID/reply` + +Why `question` first: + +- already JSON-only +- already delegates into an Effect service +- proves list + mutation + params + payload + OpenAPI in one small slice +- avoids the harder streaming and middleware cases + ### 3. Reuse existing services 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,6 +134,144 @@ Prefer mounting an experimental `HttpApi` surface alongside the existing Hono ro 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. +## Schema rule for HttpApi work + +Every `HttpApi` slice should follow `specs/effect/schema.md` and the Schema -> Zod interop rule in `specs/effect/migration.md`. + +Default rule: + +- Effect Schema owns the type +- `.zod` exists only as a compatibility surface +- do not introduce a new hand-written Zod schema for a type that is already migrating to Effect Schema + +Practical implication for `HttpApi` migration: + +- 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 +- if an existing Hono route or tool still needs Zod, derive it with `@/util/effect-zod` +- avoid maintaining parallel Zod and Effect definitions for the same request or response type + +Ordering for a route-group migration: + +1. move implicated shared `schema.ts` leaf types to Effect Schema first +2. move exported `Info` / `Input` / `Output` route DTOs to Effect Schema +3. move tagged route-facing errors to `Schema.TaggedErrorClass` where needed +4. switch existing Zod boundary validators to derived `.zod` +5. define the `HttpApi` contract from the canonical Effect schemas + +Temporary exception: + +- 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 +- if that happens, leave a short note so the type does not become a permanent second source of truth + +## First vertical slice + +The first `HttpApi` spike should be intentionally small and repeatable. + +Chosen slice: + +- group: `question` +- endpoints: `GET /question` and `POST /question/:requestID/reply` + +Non-goals: + +- no `session` routes +- no SSE or websocket routes +- no auth redesign +- no broad service refactor + +Behavior rule: + +- preserve current runtime behavior first +- treat semantic changes such as introducing new `404` behavior as a separate follow-up unless they are required to make the contract honest + +Add `POST /question/:requestID/reject` only after the first two endpoints work cleanly. + +## Repeatable slice template + +Use the same sequence for each route group. + +1. Pick one JSON-only route group that already mostly delegates into services. +2. Identify the shared DTOs, IDs, and errors implicated by that slice. +3. Apply the schema migration ordering above so those types are Effect Schema-first. +4. Define the `HttpApi` contract separately from the handlers. +5. Implement handlers by yielding the existing service from context. +6. Mount the new surface in parallel under an experimental prefix. +7. Add one end-to-end test and one OpenAPI-focused test. +8. Compare ergonomics before migrating the next endpoint. + +Rule of thumb: + +- migrate one route group at a time +- migrate one or two endpoints first, not the whole file +- keep business logic in the existing service +- keep the first spike easy to delete if the experiment is not worth continuing + +## Example structure + +Placement rule: + +- keep `HttpApi` code under `src/server`, not `src/effect` +- `src/effect` should stay focused on runtimes, layers, instance state, and shared Effect plumbing +- place each `HttpApi` slice next to the HTTP boundary it serves +- for instance-scoped routes, prefer `src/server/instance/httpapi/*` +- if control-plane routes ever migrate, prefer `src/server/control/httpapi/*` + +Suggested file layout for a repeatable spike: + +- `src/server/instance/httpapi/question.ts` +- `src/server/instance/httpapi/index.ts` +- `test/server/question-httpapi.test.ts` +- `test/server/question-httpapi-openapi.test.ts` + +Suggested responsibilities: + +- `question.ts` defines the `HttpApi` contract and `HttpApiBuilder.group(...)` handlers for the experimental slice +- `index.ts` combines experimental `HttpApi` groups and exposes the mounted handler or layer +- `question-httpapi.test.ts` proves the route works end-to-end against the real service +- `question-httpapi-openapi.test.ts` proves the generated OpenAPI is acceptable for the migrated endpoints + +## Example migration shape + +Each route-group spike should follow the same shape. + +### 1. Contract + +- define an experimental `HttpApi` +- define one `HttpApiGroup` +- define endpoint params, payload, success, and error schemas from canonical Effect schemas +- annotate summary, description, and operation ids explicitly so generated docs are stable + +### 2. Handler layer + +- implement with `HttpApiBuilder.group(api, groupName, ...)` +- yield the existing Effect service from context +- keep handler bodies thin +- keep transport mapping at the HTTP boundary only + +### 3. Mounting + +- mount under an experimental prefix such as `/experimental/httpapi` +- keep existing Hono routes unchanged +- expose separate OpenAPI output for the experimental slice first + +### 4. Verification + +- seed real state through the existing service +- call the experimental endpoints +- assert that the service behavior is unchanged +- assert that the generated OpenAPI contains the migrated paths and schemas + +## Exit criteria for the spike + +The first slice is successful if: + +- the endpoints run in parallel with the current Hono routes +- the handlers reuse the existing Effect service +- request decoding and response shapes are schema-defined from canonical Effect schemas +- any remaining Zod boundary usage is derived from `.zod` or clearly temporary +- OpenAPI is generated from the `HttpApi` contract +- the tests are straightforward enough that the next slice feels mechanical + ## Proposed first steps - [ ] add one small spike that defines an `HttpApi` group for a simple JSON route set diff --git a/packages/opencode/src/question/index.ts b/packages/opencode/src/question/index.ts index 178bc7943c..f409ed8bb4 100644 --- a/packages/opencode/src/question/index.ts +++ b/packages/opencode/src/question/index.ts @@ -3,8 +3,8 @@ import { Bus } from "@/bus" import { BusEvent } from "@/bus/bus-event" import { InstanceState } from "@/effect/instance-state" import { SessionID, MessageID } from "@/session/schema" +import { zod } from "@/util/effect-zod" import { Log } from "@/util/log" -import z from "zod" import { QuestionID } from "./schema" export namespace Question { @@ -12,67 +12,90 @@ export namespace Question { // Schemas - export const Option = z - .object({ - label: z.string().describe("Display text (1-5 words, concise)"), - description: z.string().describe("Explanation of choice"), - }) - .meta({ ref: "QuestionOption" }) - export type Option = z.infer + const _Option = Schema.Struct({ + label: Schema.String.annotate({ + description: "Display text (1-5 words, concise)", + }), + description: Schema.String.annotate({ + description: "Explanation of choice", + }), + }).annotate({ identifier: "QuestionOption" }) + export const Option = Object.assign(_Option, { zod: zod(_Option) }) + export type Option = Schema.Schema.Type - export const Info = z - .object({ - question: z.string().describe("Complete question"), - header: z.string().describe("Very short label (max 30 chars)"), - options: z.array(Option).describe("Available choices"), - multiple: z.boolean().optional().describe("Allow selecting multiple choices"), - custom: z.boolean().optional().describe("Allow typing a custom answer (default: true)"), - }) - .meta({ ref: "QuestionInfo" }) - export type Info = z.infer + const base = { + question: Schema.String.annotate({ + description: "Complete question", + }), + header: Schema.String.annotate({ + description: "Very short label (max 30 chars)", + }), + options: Schema.Array(Option).annotate({ + description: "Available choices", + }), + multiple: Schema.optional(Schema.Boolean).annotate({ + description: "Allow selecting multiple choices", + }), + } - export const Request = z - .object({ - id: QuestionID.zod, - sessionID: SessionID.zod, - questions: z.array(Info).describe("Questions to ask"), - tool: z - .object({ - messageID: MessageID.zod, - callID: z.string(), - }) - .optional(), - }) - .meta({ ref: "QuestionRequest" }) - export type Request = z.infer + const _Info = Schema.Struct({ + ...base, + custom: Schema.optional(Schema.Boolean).annotate({ + description: "Allow typing a custom answer (default: true)", + }), + }).annotate({ identifier: "QuestionInfo" }) + export const Info = Object.assign(_Info, { zod: zod(_Info) }) + export type Info = Schema.Schema.Type - export const Answer = z.array(z.string()).meta({ ref: "QuestionAnswer" }) - export type Answer = z.infer + const _Prompt = Schema.Struct(base).annotate({ identifier: "QuestionPrompt" }) + export const Prompt = Object.assign(_Prompt, { zod: zod(_Prompt) }) + export type Prompt = Schema.Schema.Type - export const Reply = z.object({ - answers: z - .array(Answer) - .describe("User answers in order of questions (each answer is an array of selected labels)"), + const _Tool = Schema.Struct({ + messageID: MessageID, + callID: Schema.String, + }).annotate({ identifier: "QuestionTool" }) + export const Tool = Object.assign(_Tool, { zod: zod(_Tool) }) + export type Tool = Schema.Schema.Type + + const _Request = Schema.Struct({ + id: QuestionID, + sessionID: SessionID, + questions: Schema.Array(Info).annotate({ + description: "Questions to ask", + }), + tool: Schema.optional(Tool), + }).annotate({ identifier: "QuestionRequest" }) + export const Request = Object.assign(_Request, { zod: zod(_Request) }) + export type Request = Schema.Schema.Type + + const _Answer = Schema.Array(Schema.String).annotate({ identifier: "QuestionAnswer" }) + export const Answer = Object.assign(_Answer, { zod: zod(_Answer) }) + export type Answer = Schema.Schema.Type + + const _Reply = Schema.Struct({ + answers: Schema.Array(Answer).annotate({ + description: "User answers in order of questions (each answer is an array of selected labels)", + }), + }).annotate({ identifier: "QuestionReply" }) + export const Reply = Object.assign(_Reply, { zod: zod(_Reply) }) + export type Reply = Schema.Schema.Type + + const replied = Schema.Struct({ + sessionID: SessionID, + requestID: QuestionID, + answers: Schema.Array(Answer), + }) + + const rejected = Schema.Struct({ + sessionID: SessionID, + requestID: QuestionID, }) - export type Reply = z.infer export const Event = { - Asked: BusEvent.define("question.asked", Request), - Replied: BusEvent.define( - "question.replied", - z.object({ - sessionID: SessionID.zod, - requestID: QuestionID.zod, - answers: z.array(Answer), - }), - ), - Rejected: BusEvent.define( - "question.rejected", - z.object({ - sessionID: SessionID.zod, - requestID: QuestionID.zod, - }), - ), + Asked: BusEvent.define("question.asked", Request.zod), + Replied: BusEvent.define("question.replied", zod(replied)), + Rejected: BusEvent.define("question.rejected", zod(rejected)), } export class RejectedError extends Schema.TaggedErrorClass()("QuestionRejectedError", {}) { @@ -83,7 +106,7 @@ export namespace Question { interface PendingEntry { info: Request - deferred: Deferred.Deferred + deferred: Deferred.Deferred, RejectedError> } interface State { @@ -95,12 +118,12 @@ export namespace Question { export interface Interface { readonly ask: (input: { sessionID: SessionID - questions: Info[] - tool?: { messageID: MessageID; callID: string } - }) => Effect.Effect - readonly reply: (input: { requestID: QuestionID; answers: Answer[] }) => Effect.Effect + questions: ReadonlyArray + tool?: Tool + }) => Effect.Effect, RejectedError> + readonly reply: (input: { requestID: QuestionID; answers: ReadonlyArray }) => Effect.Effect readonly reject: (requestID: QuestionID) => Effect.Effect - readonly list: () => Effect.Effect + readonly list: () => Effect.Effect> } export class Service extends Context.Service()("@opencode/Question") {} @@ -130,14 +153,14 @@ export namespace Question { const ask = Effect.fn("Question.ask")(function* (input: { sessionID: SessionID - questions: Info[] - tool?: { messageID: MessageID; callID: string } + questions: ReadonlyArray + tool?: Tool }) { const pending = (yield* InstanceState.get(state)).pending const id = QuestionID.ascending() log.info("asking", { id, questions: input.questions.length }) - const deferred = yield* Deferred.make() + const deferred = yield* Deferred.make, RejectedError>() const info: Request = { id, sessionID: input.sessionID, @@ -155,7 +178,10 @@ export namespace Question { ) }) - const reply = Effect.fn("Question.reply")(function* (input: { requestID: QuestionID; answers: Answer[] }) { + const reply = Effect.fn("Question.reply")(function* (input: { + requestID: QuestionID + answers: ReadonlyArray + }) { const pending = (yield* InstanceState.get(state)).pending const existing = pending.get(input.requestID) if (!existing) { diff --git a/packages/opencode/src/server/instance/experimental.ts b/packages/opencode/src/server/instance/experimental.ts index ca8b89fa6a..4cec7dff2a 100644 --- a/packages/opencode/src/server/instance/experimental.ts +++ b/packages/opencode/src/server/instance/experimental.ts @@ -18,6 +18,7 @@ import { lazy } from "../../util/lazy" import { Effect, Option } from "effect" import { WorkspaceRoutes } from "./workspace" import { Agent } from "@/agent/agent" +import { HttpApiRoutes } from "./httpapi" const ConsoleOrgOption = z.object({ accountID: z.string(), @@ -39,6 +40,7 @@ const ConsoleSwitchBody = z.object({ export const ExperimentalRoutes = lazy(() => new Hono() + .route("/httpapi", HttpApiRoutes()) .get( "/console", describeRoute({ diff --git a/packages/opencode/src/server/instance/httpapi/index.ts b/packages/opencode/src/server/instance/httpapi/index.ts new file mode 100644 index 0000000000..523041de84 --- /dev/null +++ b/packages/opencode/src/server/instance/httpapi/index.ts @@ -0,0 +1,7 @@ +import { lazy } from "@/util/lazy" +import { Hono } from "hono" +import { QuestionHttpApiHandler } from "./question" + +export const HttpApiRoutes = lazy(() => + new Hono().all("/question", QuestionHttpApiHandler).all("/question/*", QuestionHttpApiHandler), +) diff --git a/packages/opencode/src/server/instance/httpapi/question.ts b/packages/opencode/src/server/instance/httpapi/question.ts new file mode 100644 index 0000000000..c6033427d1 --- /dev/null +++ b/packages/opencode/src/server/instance/httpapi/question.ts @@ -0,0 +1,90 @@ +import { AppLayer } from "@/effect/app-runtime" +import { memoMap } from "@/effect/run-service" +import { Question } from "@/question" +import { QuestionID } from "@/question/schema" +import { lazy } from "@/util/lazy" +import { Effect, Layer, Schema } from "effect" +import { HttpRouter, HttpServer } from "effect/unstable/http" +import { HttpApi, HttpApiBuilder, HttpApiEndpoint, HttpApiGroup, OpenApi } from "effect/unstable/httpapi" +import type { Handler } from "hono" + +const root = "/experimental/httpapi/question" + +const Api = HttpApi.make("question") + .add( + HttpApiGroup.make("question") + .add( + HttpApiEndpoint.get("list", root, { + success: Schema.Array(Question.Request), + }).annotateMerge( + OpenApi.annotations({ + identifier: "question.list", + summary: "List pending questions", + description: "Get all pending question requests across all sessions.", + }), + ), + HttpApiEndpoint.post("reply", `${root}/:requestID/reply`, { + params: { requestID: QuestionID }, + payload: Question.Reply, + success: Schema.Boolean, + }).annotateMerge( + OpenApi.annotations({ + identifier: "question.reply", + summary: "Reply to question request", + description: "Provide answers to a question request from the AI assistant.", + }), + ), + ) + .annotateMerge( + OpenApi.annotations({ + title: "question", + description: "Experimental HttpApi question routes.", + }), + ), + ) + .annotateMerge( + OpenApi.annotations({ + title: "opencode experimental HttpApi", + version: "0.0.1", + description: "Experimental HttpApi surface for selected instance routes.", + }), + ) + +const list = Effect.fn("QuestionHttpApi.list")(function* () { + const svc = yield* Question.Service + return yield* svc.list() +}) + +const reply = Effect.fn("QuestionHttpApi.reply")(function* (ctx: { + params: { requestID: QuestionID } + payload: Question.Reply +}) { + const svc = yield* Question.Service + yield* svc.reply({ + requestID: ctx.params.requestID, + answers: ctx.payload.answers, + }) + return true +}) + +const QuestionLive = HttpApiBuilder.group(Api, "question", (handlers) => + handlers.handle("list", list).handle("reply", reply), +) + +const web = lazy(() => + HttpRouter.toWebHandler( + Layer.mergeAll( + AppLayer, + HttpApiBuilder.layer(Api, { openapiPath: `${root}/doc` }).pipe( + Layer.provide(QuestionLive), + Layer.provide(HttpServer.layerServices), + ), + ), + { + disableLogger: true, + memoMap, + }, + ), +) + +export const QuestionHttpApiHandler: Handler = (c, _next) => web().handler(c.req.raw) diff --git a/packages/opencode/src/server/instance/question.ts b/packages/opencode/src/server/instance/question.ts index 501ae21816..1375f12e71 100644 --- a/packages/opencode/src/server/instance/question.ts +++ b/packages/opencode/src/server/instance/question.ts @@ -21,7 +21,7 @@ export const QuestionRoutes = lazy(() => description: "List of pending questions", content: { "application/json": { - schema: resolver(Question.Request.array()), + schema: resolver(Question.Request.zod.array()), }, }, }, @@ -56,7 +56,7 @@ export const QuestionRoutes = lazy(() => requestID: QuestionID.zod, }), ), - validator("json", Question.Reply), + validator("json", Question.Reply.zod), async (c) => { const params = c.req.valid("param") const json = c.req.valid("json") diff --git a/packages/opencode/src/tool/question.ts b/packages/opencode/src/tool/question.ts index 8cfa700a5a..50e4b1c511 100644 --- a/packages/opencode/src/tool/question.ts +++ b/packages/opencode/src/tool/question.ts @@ -5,11 +5,11 @@ import { Question } from "../question" import DESCRIPTION from "./question.txt" const parameters = z.object({ - questions: z.array(Question.Info.omit({ custom: true })).describe("Questions to ask"), + questions: z.array(Question.Prompt.zod).describe("Questions to ask"), }) type Metadata = { - answers: Question.Answer[] + answers: ReadonlyArray } export const QuestionTool = Tool.define( diff --git a/packages/opencode/test/question/question.test.ts b/packages/opencode/test/question/question.test.ts index 7c101ce285..d44f41f1aa 100644 --- a/packages/opencode/test/question/question.test.ts +++ b/packages/opencode/test/question/question.test.ts @@ -6,12 +6,12 @@ import { tmpdir } from "../fixture/fixture" import { SessionID } from "../../src/session/schema" import { AppRuntime } from "../../src/effect/app-runtime" -const ask = (input: { sessionID: SessionID; questions: Question.Info[]; tool?: { messageID: any; callID: string } }) => +const ask = (input: { sessionID: SessionID; questions: ReadonlyArray; tool?: Question.Tool }) => AppRuntime.runPromise(Question.Service.use((svc) => svc.ask(input))) const list = () => AppRuntime.runPromise(Question.Service.use((svc) => svc.list())) -const reply = (input: { requestID: QuestionID; answers: Question.Answer[] }) => +const reply = (input: { requestID: QuestionID; answers: ReadonlyArray }) => AppRuntime.runPromise(Question.Service.use((svc) => svc.reply(input))) const reject = (id: QuestionID) => AppRuntime.runPromise(Question.Service.use((svc) => svc.reject(id))) diff --git a/packages/opencode/test/server/question-httpapi.test.ts b/packages/opencode/test/server/question-httpapi.test.ts new file mode 100644 index 0000000000..00cc32f59e --- /dev/null +++ b/packages/opencode/test/server/question-httpapi.test.ts @@ -0,0 +1,78 @@ +import { afterEach, describe, expect, test } from "bun:test" +import { AppRuntime } from "../../src/effect/app-runtime" +import { Instance } from "../../src/project/instance" +import { Question } from "../../src/question" +import { Server } from "../../src/server/server" +import { SessionID } from "../../src/session/schema" +import { Log } from "../../src/util/log" +import { tmpdir } from "../fixture/fixture" + +Log.init({ print: false }) + +const ask = (input: { sessionID: SessionID; questions: ReadonlyArray }) => + AppRuntime.runPromise(Question.Service.use((svc) => svc.ask(input))) + +afterEach(async () => { + await Instance.disposeAll() +}) + +describe("experimental question httpapi", () => { + test("lists pending questions, replies, and serves docs", async () => { + await using tmp = await tmpdir({ git: true }) + const app = Server.Default().app + const headers = { + "content-type": "application/json", + "x-opencode-directory": tmp.path, + } + const questions: ReadonlyArray = [ + { + question: "What would you like to do?", + header: "Action", + options: [ + { label: "Option 1", description: "First option" }, + { label: "Option 2", description: "Second option" }, + ], + }, + ] + + let pending!: ReturnType + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + pending = ask({ + sessionID: SessionID.make("ses_test"), + questions, + }) + }, + }) + + const list = await app.request("/experimental/httpapi/question", { + headers, + }) + + expect(list.status).toBe(200) + const items = await list.json() + expect(items).toHaveLength(1) + expect(items[0]).toMatchObject({ questions }) + + const doc = await app.request("/experimental/httpapi/question/doc", { + headers, + }) + + expect(doc.status).toBe(200) + const spec = await doc.json() + expect(spec.paths["/experimental/httpapi/question"]?.get?.operationId).toBe("question.list") + expect(spec.paths["/experimental/httpapi/question/{requestID}/reply"]?.post?.operationId).toBe("question.reply") + + const reply = await app.request(`/experimental/httpapi/question/${items[0].id}/reply`, { + method: "POST", + headers, + body: JSON.stringify({ answers: [["Option 1"]] }), + }) + + expect(reply.status).toBe(200) + expect(await reply.json()).toBe(true) + expect(await pending).toEqual([["Option 1"]]) + }) +})