From 6f2f759fbb7ecce6c28bac6ab0d72891370827ac Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Mon, 11 May 2026 13:20:54 -0400 Subject: [PATCH] Clean up post-Hono references (#26903) --- bun.lock | 6 - packages/console/function/package.json | 2 - packages/opencode/specs/effect/errors.md | 15 +- packages/opencode/specs/effect/routes.md | 27 +- packages/opencode/specs/effect/schema.md | 40 +- .../opencode/specs/effect/server-package.md | 694 ++---------------- .../instance/httpapi/handlers/provider.ts | 2 +- .../routes/instance/httpapi/handlers/pty.ts | 2 +- .../instance/httpapi/handlers/session.ts | 2 +- .../httpapi/middleware/compression.ts | 2 +- .../server/routes/instance/httpapi/public.ts | 8 +- .../test/project/instance-bootstrap.test.ts | 4 +- .../test/server/httpapi-config.test.ts | 2 +- .../test/server/httpapi-experimental.test.ts | 9 +- .../test/server/httpapi-instance.test.ts | 5 +- specs/v2/todo.md | 6 +- 16 files changed, 90 insertions(+), 736 deletions(-) diff --git a/bun.lock b/bun.lock index c3758e2326..37df44b0b5 100644 --- a/bun.lock +++ b/bun.lock @@ -152,12 +152,10 @@ "@ai-sdk/anthropic": "3.0.64", "@ai-sdk/openai": "3.0.48", "@ai-sdk/openai-compatible": "2.0.37", - "@hono/zod-validator": "catalog:", "@openauthjs/openauth": "0.0.0-20250322224806", "@opencode-ai/console-core": "workspace:*", "@opencode-ai/console-resource": "workspace:*", "ai": "catalog:", - "hono": "catalog:", "zod": "catalog:", }, "devDependencies": { @@ -1235,8 +1233,6 @@ "@hono/standard-validator": ["@hono/standard-validator@0.1.5", "", { "peerDependencies": { "@standard-schema/spec": "1.0.0", "hono": ">=3.9.0" } }, "sha512-EIyZPPwkyLn6XKwFj5NBEWHXhXbgmnVh2ceIFo5GO7gKI9WmzTjPDKnppQB0KrqKeAkq3kpoW4SIbu5X1dgx3w=="], - "@hono/zod-validator": ["@hono/zod-validator@0.4.2", "", { "peerDependencies": { "hono": ">=3.9.0", "zod": "^3.19.1" } }, "sha512-1rrlBg+EpDPhzOV4hT9pxr5+xDVmKuz6YJl+la7VCwK6ass5ldyKm5fD+umJdV2zhHD6jROoCCv8NbTwyfhT0g=="], - "@ibm/plex": ["@ibm/plex@6.4.1", "", { "dependencies": { "@ibm/telemetry-js": "^1.5.1" } }, "sha512-fnsipQywHt3zWvsnlyYKMikcVI7E2fEwpiPnIHFqlbByXVfQfANAAeJk1IV4mNnxhppUIDlhU0TzwYwL++Rn2g=="], "@ibm/telemetry-js": ["@ibm/telemetry-js@1.11.0", "", { "bin": { "ibmtelemetry": "dist/collect.js" } }, "sha512-RO/9j+URJnSfseWg9ZkEX9p+a3Ousd33DBU7rOafoZB08RqdzxFVYJ2/iM50dkBuD0o7WX7GYt1sLbNgCoE+pA=="], @@ -5469,8 +5465,6 @@ "@hey-api/openapi-ts/semver": ["semver@7.7.3", "", { "bin": { "semver": "bin/semver.js" } }, "sha512-SdsKMrI9TdgjdweUSR9MweHA4EJ8YxHn8DFaDisvhVlUOe4BF1tLD7GAj0lIqWVl+dPb/rExr0Btby5loQm20Q=="], - "@hono/zod-validator/zod": ["zod@3.25.76", "", {}, "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ=="], - "@jimp/core/mime": ["mime@3.0.0", "", { "bin": { "mime": "cli.js" } }, "sha512-jSCU7/VB1loIWBZe14aEYHU/+1UMEHoaO7qxCOVJOw9GgH72VAWppxNcjU+x9a2k3GSIBXNKxXQFqRvvZ7vr3A=="], "@jimp/plugin-blit/zod": ["zod@3.25.76", "", {}, "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ=="], diff --git a/packages/console/function/package.json b/packages/console/function/package.json index 41487f845a..5c1d1ba223 100644 --- a/packages/console/function/package.json +++ b/packages/console/function/package.json @@ -20,12 +20,10 @@ "@ai-sdk/anthropic": "3.0.64", "@ai-sdk/openai": "3.0.48", "@ai-sdk/openai-compatible": "2.0.37", - "@hono/zod-validator": "catalog:", "@opencode-ai/console-core": "workspace:*", "@opencode-ai/console-resource": "workspace:*", "@openauthjs/openauth": "0.0.0-20250322224806", "ai": "catalog:", - "hono": "catalog:", "zod": "catalog:" } } diff --git a/packages/opencode/specs/effect/errors.md b/packages/opencode/specs/effect/errors.md index 746e658693..e19199ef49 100644 --- a/packages/opencode/specs/effect/errors.md +++ b/packages/opencode/specs/effect/errors.md @@ -23,8 +23,9 @@ contracts. - Some services already use `Schema.TaggedErrorClass`, for example `Account`, `Auth`, `Permission`, `Question`, `Installation`, and parts of `Workspace`. -- Legacy Hono error handling recognizes `NamedError`, `Session.BusyError`, and a - few name-based cases, then emits the legacy `{ name, data }` JSON body. +- The temporary HttpApi compatibility middleware recognizes `NamedError`, + `Session.BusyError`, and a few name-based cases, then emits the legacy + `{ name, data }` JSON body. - Effect `HttpApi` only knows how to encode errors that are declared on the endpoint, group, or middleware. Undeclared expected errors become defects and eventually fall through to generic HTTP handling. @@ -127,7 +128,7 @@ Create an HttpApi-local error module, likely That module should provide: - Legacy-compatible public schemas for `{ name, data }` error bodies that must - remain SDK-compatible during the Hono migration. + remain SDK-compatible while route groups declare typed errors. - Small constructors or mapping helpers for common API errors such as not found, bad request, conflict, and unknown internal errors. - Route-group-specific adapters only when they encode domain-specific public @@ -173,7 +174,7 @@ Add the `httpapi/errors.ts` module before converting route groups. - Define a legacy `{ name, data }` body helper for SDK-compatible errors. - Define `UnknownError` for generic internal failures with a safe public message. - Define `BadRequestError` and `NotFoundError` equivalents only if the actual - wire body must match the legacy Hono SDK surface. + wire body must match the existing SDK surface. - Put the HTTP status on the public schema with `HttpApiSchema.status(...)` or `{ httpApiStatus: code }`; do not keep a separate name-to-status table. - Keep conversion helpers pure and small. They should not inspect `Cause` or @@ -238,7 +239,7 @@ Suggested route order: 2. `experimental` worktree mutations. 3. `provider` auth and model selection errors. 4. `mcp` OAuth and connection errors. -5. Remaining route groups as Hono deletion work progresses. +5. Remaining route groups as typed error contracts are declared. ### 6. Remove Defect Recovery @@ -286,8 +287,8 @@ For HttpApi conversions: errors. - Add a regression test that the temporary middleware is no longer needed for the migrated route. -- Keep bridge/parity tests aligned with legacy Hono behavior until Hono is - deleted or the SDK contract intentionally changes. +- Keep compatibility tests aligned with the existing SDK contract until the + public error shape intentionally changes. ## Verification Commands diff --git a/packages/opencode/specs/effect/routes.md b/packages/opencode/specs/effect/routes.md index 3bf7e1b556..8066bda346 100644 --- a/packages/opencode/specs/effect/routes.md +++ b/packages/opencode/specs/effect/routes.md @@ -39,26 +39,19 @@ This eliminates multiple `runPromise` round-trips and lets handlers compose natu ## Current route files -Current instance route files live under `src/server/routes/instance`. - -Files that are already mostly on the intended service-yielding shape: - -- [x] `server/routes/instance/question.ts` — handlers yield `Question.Service` -- [x] `server/routes/instance/provider.ts` — handlers yield `Provider.Service`, `ProviderAuth.Service`, and `Config.Service` -- [x] `server/routes/instance/permission.ts` — handlers yield `Permission.Service` -- [x] `server/routes/instance/mcp.ts` — handlers mostly yield `MCP.Service` -- [x] `server/routes/instance/pty.ts` — handlers yield `Pty.Service` +Current instance route files live under `src/server/routes/instance/httpapi`. +Most handlers already yield stable services at route-layer construction and then +close over those services in endpoint implementations. Files still worth tracking here: -- [ ] `server/routes/instance/session.ts` — still the heaviest mixed file; many handlers are composed, but the file still mixes patterns and has direct `Bus.publish(...)` / `Session.list(...)` usage -- [ ] `server/routes/instance/index.ts` — mostly converted, but still has direct `Instance.dispose()` / `Instance.*` reads for `/instance/dispose` and `/path` -- [ ] `server/routes/instance/file.ts` — most handlers yield services, but `/find` still passes `Instance.directory` directly into ripgrep and `/find/symbol` is still stubbed -- [ ] `server/routes/instance/experimental.ts` — mixed state; many handlers are composed, but some still rely on `runRequest(...)` or direct `Instance.project` reads -- [ ] `server/routes/instance/middleware.ts` — still enters the instance via `Instance.provide(...)` -- [ ] `server/routes/global.ts` — still uses `Instance.disposeAll()` and remains partly outside the fully-composed style +- [ ] `handlers/session.ts` — still the heaviest mixed file; some paths keep compatibility translations and direct event publication +- [ ] `handlers/experimental.ts` — mixed state; some handlers still rely on request-local context reads +- [ ] `middleware/*` — still contains compatibility policy for auth, compression, errors, instance context, and workspace routing +- [ ] `public.ts` — still owns SDK/OpenAPI compatibility translation shims +- [ ] raw route modules — WebSocket and catch-all routes should stay explicit and avoid rebuilding stable layers per request ## Notes -- Route conversion is now less about facade removal and more about removing the remaining direct `Instance.*` reads, `Instance.provide(...)` boundaries, and small Promise-style bridges inside route files. -- `jsonRequest(...)` / `runRequest(...)` already provide a good intermediate shape for many handlers. The remaining cleanup is mostly consistency work in the heavier files. +- Route conversion is now less about backend migration and more about removing the remaining direct `Instance.*` reads, request-local service plumbing, and OpenAPI compatibility shims. +- Prefer route-layer service capture over rebuilding or providing stable layers inside individual handlers. diff --git a/packages/opencode/specs/effect/schema.md b/packages/opencode/specs/effect/schema.md index e20605c3bc..20b3e70e7b 100644 --- a/packages/opencode/specs/effect/schema.md +++ b/packages/opencode/specs/effect/schema.md @@ -305,38 +305,18 @@ emitted JSON Schema must stay byte-identical. ### HTTP route boundaries -Every file in `src/server/routes/` uses hono-openapi with zod validators for -route inputs/outputs. Migrating these individually is the last step; most -will switch to `.zod` derived from the Schema-migrated domain types above, -which means touching them is largely mechanical once the domain side is -done. +The server route tree now lives under `src/server/routes/instance/httpapi` and +uses Effect HttpApi contracts for request and response schemas. Remaining schema +work is no longer a Hono route migration; it is compatibility cleanup around +derived `.zod` statics, OpenAPI translation shims, and route groups that still +need explicit SDK-visible error contracts. -- [ ] `src/server/error.ts` -- [x] `src/server/event.ts` -- [x] `src/server/projectors.ts` -- [ ] `src/server/routes/control/index.ts` -- [ ] `src/server/routes/control/workspace.ts` -- [ ] `src/server/routes/global.ts` -- [ ] `src/server/routes/instance/index.ts` -- [ ] `src/server/routes/instance/config.ts` -- [ ] `src/server/routes/instance/event.ts` -- [ ] `src/server/routes/instance/experimental.ts` -- [ ] `src/server/routes/instance/file.ts` -- [ ] `src/server/routes/instance/mcp.ts` -- [ ] `src/server/routes/instance/permission.ts` -- [ ] `src/server/routes/instance/project.ts` -- [ ] `src/server/routes/instance/provider.ts` -- [ ] `src/server/routes/instance/pty.ts` -- [ ] `src/server/routes/instance/question.ts` -- [ ] `src/server/routes/instance/session.ts` -- [ ] `src/server/routes/instance/sync.ts` -- [ ] `src/server/routes/instance/tui.ts` +Good follow-up targets: -The bigger prize for this group is the `@effect/platform` HTTP migration -described in `specs/effect/http-api.md`. Once that lands, every one of -these files changes shape entirely (`HttpApi.endpoint(...)` and friends), -so the Schema-first domain types become a prerequisite rather than a -sibling task. +- shrink `public.ts` legacy OpenAPI translation shims one SDK-compatible slice at a time +- replace production `.zod.safeParse(...)` call sites with Effect Schema decoders +- remove derived `.zod` statics after their production consumers are gone +- declare route-group errors directly instead of relying on compatibility middleware ### Everything else diff --git a/packages/opencode/specs/effect/server-package.md b/packages/opencode/specs/effect/server-package.md index 06e89c18de..036472337e 100644 --- a/packages/opencode/specs/effect/server-package.md +++ b/packages/opencode/specs/effect/server-package.md @@ -1,668 +1,58 @@ -# Server package extraction +# Server Package Extraction -Practical reference for extracting a future `packages/server` from the current `packages/opencode` monolith while `packages/core` is still being migrated to Effect. +Practical reference for a future `packages/server` split after the opencode +server moved to the Effect HttpApi backend. -This document is intentionally execution-oriented. +## Current State -It should give an agent enough context to land one incremental PR at a time without needing to rediscover the package strategy, route migration rules, or current constraints. +- The server still lives in `packages/opencode`. +- The runtime and app layer are centralized in `src/effect/app-runtime.ts` and + `src/effect/run-service.ts`. +- The route tree lives under `src/server/routes/instance/httpapi` and is hosted + from `src/server/server.ts`. +- OpenAPI generation is based on the HttpApi contract plus compatibility + translation in `src/server/routes/instance/httpapi/public.ts`. +- There is no standalone `packages/server` workspace yet. -## Goal - -Create `packages/server` as the home for: - -- HTTP contract definitions -- HTTP handler implementations -- OpenAPI generation -- eventual embeddable server APIs for Node apps - -Do this without blocking on the full `packages/core` extraction. - -## Future state +## Future State Target package layout: -- `packages/core` - all opencode services, Effect-first source of truth -- `packages/server` - opencode server, with separate contract and implementation, still producing `openapi.json` -- `packages/cli` - TUI + CLI entrypoints -- `packages/sdk` - generated from the server OpenAPI spec, may add higher-level wrappers -- `packages/plugin` - generated or semi-hand-rolled non-Effect package built from core plugin definitions +- `packages/core` - shared domain services and schemas +- `packages/server` - HTTP contracts, handlers, OpenAPI generation, and an + embeddable server API +- `packages/cli` - TUI and CLI entrypoints +- `packages/sdk` - generated from the server OpenAPI spec +- `packages/plugin` - plugin authoring surface -Desired user stories: +## Extraction Rule -- import from `core` and build a custom agent or app-specific runtime -- import from `server` and embed the full opencode server into an existing Node app -- spawn the CLI and talk to the server through that boundary +Do not create a package cycle. -## Current state +Until enough shared service code lives outside `packages/opencode`, a future +`packages/server` should either: -Everything still lives in `packages/opencode`. +- own pure HttpApi contracts only, or +- accept host-provided services/layers/callbacks from `packages/opencode` -Important current facts: +It should not import `packages/opencode` services while `packages/opencode` +imports it to host routes. -- there is no `packages/core` or `packages/cli` workspace yet -- there is no `packages/server` workspace yet on this branch -- the main host server is still Hono-based in `src/server/server.ts` -- current OpenAPI generation is Hono-based through `Server.openapi()` and `cli/cmd/generate.ts` -- the Effect runtime and app layer are centralized in `src/effect/app-runtime.ts` and `src/effect/run-service.ts` -- there are already bridged Effect `HttpApi` slices under `src/server/routes/instance/httpapi/*` -- those slices are mounted into the Hono server behind `OPENCODE_EXPERIMENTAL_HTTPAPI` -- the bridge currently covers `question`, `permission`, `provider`, partial `config`, and partial `project` routes +## Suggested PR Sequence -This means the package split should start from an extraction path, not from greenfield package ownership. +1. Keep shrinking OpenAPI compatibility shims in `httpapi/public.ts`. +2. Move stable domain schemas into shared packages only when they no longer + depend on opencode-local runtime modules. +3. Extract pure HttpApi contract modules into `packages/server` once the contract + can compile without importing `packages/opencode` implementation details. +4. Extract handler factories after their service dependencies can be supplied by + a host layer instead of imported directly. +5. Move server hosting last, after package ownership is clear. -## Structural reference +## Non-Goals -Use `anomalyco/opentunnel` as the structural reference for `packages/server`. - -The important pattern there is: - -- `packages/core` owns services and domain schemas -- `packages/server/src/definition/*` owns pure `HttpApi` contracts -- `packages/server/src/api/*` owns `HttpApiBuilder.group(...)` implementations and server-side middleware wiring -- `packages/server/src/index.ts` becomes the composition root only after the server package really owns runtime hosting - -Relevant `opentunnel` files: - -- `packages/server/src/definition/index.ts` -- `packages/server/src/definition/tunnel.ts` -- `packages/server/src/api/index.ts` -- `packages/server/src/api/tunnel.ts` -- `packages/server/src/api/client.ts` -- `packages/server/src/index.ts` - -The intended direction here is the same, but the current `opencode` package split is earlier in the migration. - -That means: - -- we should follow the same `definition` and `api` naming -- we should keep contract and implementation as separate modules from the start -- we should postpone the runtime composition root until `packages/core` exists enough to support it cleanly - -## Key decision - -Start `packages/server` as a contract and implementation package only. - -Do not make it the runtime host yet. - -Why: - -- `packages/core` does not exist yet -- the current server host still lives in `packages/opencode` -- moving host ownership immediately would force a large package and runtime shuffle while Effect service extraction is still in flight -- if `packages/server` imports services from `packages/opencode` while `packages/opencode` imports `packages/server` to host routes, we create a package cycle immediately - -Short version: - -1. create `packages/server` -2. move pure `HttpApi` contracts there -3. move handler factories there -4. keep `packages/opencode` as the temporary Hono host -5. merge `packages/server` OpenAPI with the legacy Hono OpenAPI during the transition -6. move server hosting later, after `packages/core` exists enough - -## Dependency rule - -Phase 1 rule: - -- `packages/server` must not import from `packages/opencode` - -Allowed in phase 1: - -- `packages/opencode` imports `packages/server` -- `packages/server` accepts host-provided services, layers, or callbacks as inputs -- `packages/server` may temporarily own transport-local placeholder schemas when a canonical shared schema does not exist yet - -Future rule after `packages/core` exists: - -- `packages/server` imports from `packages/core` -- `packages/cli` imports from `packages/server` and `packages/core` -- `packages/opencode` shrinks or disappears as package responsibilities are fully split - -## HttpApi model - -Use Effect v4 `HttpApi` as the source of truth for migrated HTTP routes. - -Important properties from the current `effect` / `effect-smol` model: - -- `HttpApi`, `HttpApiGroup`, and `HttpApiEndpoint` are pure contract definitions -- handlers are implemented separately with `HttpApiBuilder.group(...)` -- OpenAPI can be generated from the contract alone -- auth and middleware can later be modeled with `HttpApiMiddleware.Service` -- SSE and websocket routes are not good first-wave `HttpApi` targets - -This package split should preserve that separation explicitly. - -Default shape for migrated routes: - -- contract lives in `packages/server/src/definition/*` -- implementation lives in `packages/server/src/api/*` -- host mounting stays outside for now - -## OpenAPI rule - -During the transition there is still one spec artifact. - -Default rule: - -- `packages/server` generates OpenAPI from `HttpApi` contract -- `packages/opencode` keeps generating legacy OpenAPI from Hono routes -- the temporary exported server spec is a merged document -- `packages/sdk` continues consuming one `openapi.json` - -Merge safety rules: - -- fail on duplicate `path + method` -- fail on duplicate `operationId` -- prefer explicit summary, description, and operation ids on all new `HttpApi` endpoints - -Practical implication: - -- do not make the SDK consume two specs -- do not switch SDK generation to `packages/server` only until enough of the route surface has moved - -## Package shape - -Minimum viable `packages/server`: - -- `src/index.ts` -- `src/definition/index.ts` -- `src/definition/api.ts` -- `src/definition/question.ts` -- `src/api/index.ts` -- `src/api/question.ts` -- `src/openapi.ts` -- `src/bridge/hono.ts` -- `src/types.ts` - -Later additions, once there is enough real contract surface: - -- `src/api/client.ts` -- runtime composition in `src/index.ts` - -Suggested initial exports: - -- `api` -- `openapi` -- `questionApi` -- `makeQuestionHandler` - -Phase 1 responsibilities: - -- own pure API contracts -- own handler factories for migrated slices -- own contract-generated OpenAPI -- expose host adapters needed by `packages/opencode` - -Phase 1 non-goals: - -- do not own `listen()` -- do not own adapter selection -- do not own global server middleware -- do not own websocket or SSE transport -- do not own process bootstrapping for CLI entrypoints - -## Current source inventory - -These files matter for the first phase. - -Current host and route composition: - -- `src/server/server.ts` -- `src/server/control/index.ts` -- `src/server/routes/instance/index.ts` -- `src/server/middleware.ts` -- `src/server/adapter.bun.ts` -- `src/server/adapter.node.ts` - -Current bridged `HttpApi` slices: - -- `src/server/routes/instance/httpapi/question.ts` -- `src/server/routes/instance/httpapi/permission.ts` -- `src/server/routes/instance/httpapi/provider.ts` -- `src/server/routes/instance/httpapi/config.ts` -- `src/server/routes/instance/httpapi/project.ts` -- `src/server/routes/instance/httpapi/server.ts` - -Current OpenAPI flow: - -- `src/server/server.ts` via `Server.openapi()` -- `src/cli/cmd/generate.ts` -- `packages/sdk/js/script/build.ts` - -Current runtime and service layer: - -- `src/effect/app-runtime.ts` -- `src/effect/run-service.ts` - -## Ownership rules - -Move first into `packages/server`: - -- the experimental `question` `HttpApi` slice -- future `provider` and `config` JSON read slices -- any new `HttpApi` route groups -- transport-local OpenAPI generation for migrated routes - -Keep in `packages/opencode` for now: - -- `src/server/server.ts` -- `src/server/control/index.ts` -- `src/server/routes/**/*.ts` -- `src/server/middleware.ts` -- `src/server/adapter.*.ts` -- `src/effect/app-runtime.ts` -- `src/effect/run-service.ts` -- all Effect services until they move to `packages/core` - -## Placeholder schema rule - -`packages/core` is allowed to lag behind. - -Until shared canonical schemas move to `packages/core`: - -- prefer importing existing Effect Schema DTOs from current locations when practical -- if a route only needs a transport-local type and moving the canonical schema would create unrelated churn, allow a temporary server-local placeholder schema -- if a placeholder is introduced, leave a short note so it does not become permanent - -The default rule from `schema.md` still applies: - -- Effect Schema owns the type -- `.zod` is compatibility only -- avoid parallel hand-written Zod and Effect definitions for the same migrated route shape - -## Host boundary rule - -Until host ownership moves: - -- auth stays at the outer Hono app level -- compression stays at the outer Hono app level -- CORS stays at the outer Hono app level -- instance and workspace lookup stay at the current middleware layer -- `packages/server` handlers should assume the host already provided the right request context -- do not redesign host middleware just to land the package split - -This matches the current guidance in `http-api.md`: - -- keep auth outside the first parallel `HttpApi` slices -- keep instance lookup outside the first parallel `HttpApi` slices -- keep the first migrations transport-focused and semantics-preserving - -## Route selection rules - -Good early migration targets: - -- `question` -- `provider` auth read endpoint -- `config` providers read endpoint -- small read-only instance routes - -Bad early migration targets: - -- `session` -- `event` -- `pty` -- most `global` streaming or process-heavy routes -- anything requiring websocket upgrade handling -- anything that mixes many mutations and streaming in one file - -## First vertical slice - -The first slice for the package split is still the existing `question` `HttpApi` group. - -Why `question` first: - -- it already exists as an experimental `HttpApi` slice -- it already follows the desired contract and implementation split in one file -- it is already mounted through the current Hono host -- it is JSON-only -- it has low blast radius - -Use the first slice to prove: - -- package boundary -- contract and implementation split -- host mounting from `packages/opencode` -- merged OpenAPI output -- test ergonomics for future slices - -Do not broaden scope in the first slice. - -## Incremental migration order - -Use small PRs. - -Each PR should be easy to review, easy to revert, and should not mix extraction work with unrelated service refactors. - -### PR 1. Create `packages/server` - -Scope: - -- add the new workspace package -- add package manifest and tsconfig -- add empty `src/index.ts`, `src/definition/api.ts`, `src/definition/index.ts`, `src/api/index.ts`, `src/openapi.ts`, and supporting scaffolding - -Rules: - -- no production behavior changes -- no host server changes yet -- no imports from `packages/opencode` inside `packages/server` -- prefer `opentunnel`-style naming from the start: `definition` for contracts, `api` for implementations - -Done means: - -- `packages/server` typechecks -- the workspace can import it -- the package boundary is in place for follow-up PRs - -### PR 2. Move the experimental question contract - -Scope: - -- extract the pure `HttpApi` contract from `src/server/routes/instance/httpapi/question.ts` -- place it in `packages/server/src/definition/question.ts` -- aggregate it in `packages/server/src/definition/api.ts` -- generate OpenAPI in `packages/server/src/openapi.ts` - -Rules: - -- contract only in this PR -- no handler movement yet if that keeps the diff simpler -- keep operation ids and docs metadata stable - -Done means: - -- question contract lives in `packages/server` -- OpenAPI can be generated from contract alone -- no runtime behavior changes yet - -### PR 3. Move the experimental question handler factory - -Scope: - -- extract the question `HttpApiBuilder.group(...)` implementation into `packages/server/src/api/question.ts` -- expose it as a factory that accepts host-provided dependencies or wiring -- add a small Hono bridge in `packages/server/src/bridge/hono.ts` if needed - -Rules: - -- `packages/server` must still not import from `packages/opencode` -- handler code should stay thin and service-delegating -- do not redesign the question service itself in this PR - -Done means: - -- `packages/server` can produce the experimental question handler -- the package still stays cycle-free - -### PR 4. Mount `packages/server` question from `packages/opencode` - -Scope: - -- replace local experimental question route wiring in `packages/opencode` -- keep the same mount path: -- `/question` -- `/question/:requestID/reply` -- `/question/:requestID/reject` - -Rules: - -- no behavior change -- preserve existing docs path -- preserve current request and response shapes - -Done means: - -- existing question `HttpApi` test still passes -- runtime behavior is unchanged -- the current host server is now consuming `packages/server` - -### PR 5. Merge legacy and contract OpenAPI - -Scope: - -- keep `Server.openapi()` as the temporary spec entrypoint -- generate legacy Hono spec -- generate `packages/server` contract spec -- merge them into one document -- keep `cli/cmd/generate.ts` and `packages/sdk/js/script/build.ts` consuming one spec - -Rules: - -- fail loudly on duplicate `path + method` -- fail loudly on duplicate `operationId` -- do not silently overwrite one source with the other - -Done means: - -- one merged spec is produced -- migrated question paths can come from `packages/server` -- existing SDK generation path still works - -### PR 6. Add merged OpenAPI coverage - -Scope: - -- add one test for merged OpenAPI -- assert both a legacy Hono route and a migrated `HttpApi` route exist - -Rules: - -- test the merged document, not just the `packages/server` contract spec in isolation -- pick one stable legacy route and one stable migrated route - -Done means: - -- the merged-spec path is covered -- future route migrations have a guardrail - -### PR 7. Migrate `GET /provider/auth` - -Scope: - -- add `GET /provider/auth` as the next `HttpApi` slice in `packages/server` -- mount it in parallel from `packages/opencode` - -Why this route: - -- JSON-only -- simple service delegation -- small response shape -- already listed as the best next `provider` candidate in `http-api.md` - -Done means: - -- route works through the current host -- route appears in merged OpenAPI -- no semantic change to provider auth behavior - -### PR 8. Migrate `GET /config/providers` - -Scope: - -- add `GET /config/providers` as a `HttpApi` slice in `packages/server` -- mount it in parallel from `packages/opencode` - -Why this route: - -- JSON-only -- read-only -- low transport complexity -- already listed as the best next `config` candidate in `http-api.md` - -Done means: - -- route works unchanged -- route appears in merged OpenAPI - -### PR 9+. Migrate small read-only instance routes - -Candidate order: - -1. `GET /path` -2. `GET /vcs` -3. `GET /vcs/diff` -4. `GET /command` -5. `GET /agent` -6. `GET /skill` - -Rules: - -- one or two endpoints per PR -- prefer read-only routes first -- keep outer middleware unchanged -- keep business logic in the existing service layer - -Done means for each PR: - -- contract lives in `packages/server` -- handler lives in `packages/server` -- route is mounted from the current host -- route appears in merged OpenAPI -- behavior remains unchanged - -### Later PR. Move host ownership into `packages/server` - -Only start this after there is enough `packages/core` surface to depend on directly. - -Scope: - -- move server composition into `packages/server` -- add embeddable APIs such as `createServer(...)`, `listen(...)`, or `createApp(...)` -- move adapter selection and server startup out of `packages/opencode` - -Rules: - -- do not start this while `packages/server` still depends on `packages/opencode` -- do not mix this with route migration PRs - -Done means: - -- `packages/server` can be embedded in another Node app -- `packages/cli` can depend on `packages/server` -- host logic no longer lives in `packages/opencode` - -## PR sizing rule - -Every migration PR should satisfy all of these: - -- one route group or one to two endpoints -- no unrelated service refactor -- no auth redesign -- no middleware redesign -- OpenAPI updated -- at least one route test or spec test added or updated - -## Done means for a migrated route group - -A route group migration is complete only when: - -1. the `HttpApi` contract lives in `packages/server` -2. handler implementation lives in `packages/server` -3. the route is mounted from the current host in `packages/opencode` -4. the route appears in merged OpenAPI -5. request and response schemas are Effect Schema-first or clearly temporary placeholders -6. existing behavior remains unchanged -7. the route has straightforward test coverage - -## Validation expectations - -For package-split PRs, validate the smallest useful thing. - -Typical validation for the first waves: - -- `bun typecheck` in the touched package directory or directories -- the relevant server / route coverage for the migrated slice -- merged OpenAPI coverage if the PR touches spec generation - -Do not run tests from repo root. - -## Main risks - -### Package cycle - -This is the biggest risk. - -Bad state: - -- `packages/server` imports services or runtime from `packages/opencode` -- `packages/opencode` imports route definitions or handlers from `packages/server` - -Avoid by: - -- keeping phase-1 `packages/server` free of `packages/opencode` imports -- using factories and host-provided wiring instead of direct service imports - -### Spec drift - -During the transition there are two route-definition sources. - -Avoid by: - -- one merged spec -- collision checks -- explicit `operationId`s -- merged OpenAPI tests - -### Middleware mismatch - -Current auth, compression, CORS, and instance selection are Hono-centered. - -Avoid by: - -- leaving them where they are during the first wave -- not trying to solve `HttpApiMiddleware.Service` globally in the package-split PRs - -### Core lag - -`packages/core` will not be ready everywhere. - -Avoid by: - -- allowing small transport-local placeholder schemas where necessary -- keeping those placeholders clearly temporary -- not blocking the server extraction on full schema movement - -### Scope creep - -The first vertical slice is easy to overload. - -Avoid by: - -- proving the package boundary first -- not mixing package creation, route migration, host redesign, and core extraction in the same change - -## Non-goals for the first wave - -- do not replace all Hono routes at once -- do not migrate SSE or websocket routes first -- do not redesign auth -- do not redesign instance lookup -- do not wait for full `packages/core` before starting `packages/server` -- do not change SDK generation to consume multiple specs - -## Checklist - -- [x] create `packages/server` -- [x] add package-level exports for contract and OpenAPI -- [ ] extract `question` contract into `packages/server` -- [ ] extract `question` handler factory into `packages/server` -- [ ] mount `question` from `packages/opencode` -- [ ] merge legacy and contract OpenAPI into one document -- [ ] add merged-spec coverage -- [ ] migrate `GET /provider/auth` -- [ ] migrate `GET /config/providers` -- [ ] migrate small read-only instance routes one or two at a time -- [ ] move host ownership into `packages/server` only after `packages/core` is ready enough -- [ ] split `packages/cli` after server and core boundaries are stable - -## Rule of thumb - -The fastest correct path is: - -1. establish `packages/server` as the contract-first boundary -2. keep `packages/opencode` as the temporary host -3. migrate a few safe JSON routes -4. keep one merged OpenAPI document -5. move actual host ownership only after `packages/core` can support it cleanly - -If a proposed PR would make `packages/server` import from `packages/opencode`, stop and restructure the boundary first. +- Do not revive the old dual-backend migration shape. +- Do not split server hosting before service dependencies have a clean package + boundary. +- Do not switch SDK generation to a new package until generated output is known + to remain compatible. diff --git a/packages/opencode/src/server/routes/instance/httpapi/handlers/provider.ts b/packages/opencode/src/server/routes/instance/httpapi/handlers/provider.ts index 7027e666ca..b9d5b5af15 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/provider.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/provider.ts @@ -61,7 +61,7 @@ export const providerHandlers = HttpApiBuilder.group(InstanceHttpApi, "provider" const payload = yield* Schema.decodeUnknownEffect(Schema.fromJsonString(ProviderAuth.AuthorizeInput))(body).pipe( Effect.mapError(() => new HttpApiError.BadRequest({})), ) - // Match legacy Hono behavior: when authorize() resolves without a + // Match legacy route behavior: when authorize() resolves without a // result (e.g. no further redirect), serialize as JSON `null` instead // of an empty body so clients can `.json()` parse the response. const result = yield* authorize({ params: ctx.params, payload }) diff --git a/packages/opencode/src/server/routes/instance/httpapi/handlers/pty.ts b/packages/opencode/src/server/routes/instance/httpapi/handlers/pty.ts index 369ca91d02..f4d6adb935 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/pty.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/pty.ts @@ -153,7 +153,7 @@ export const ptyConnectRoute = HttpRouter.use((router) => return HttpServerResponse.empty() } - // No `pending[]`-style early-frame buffer (the legacy Hono handler had one). + // No `pending[]`-style early-frame buffer (the legacy handler had one). // `request.upgrade` returns a Socket without running the WS handshake; the // handshake fires inside `socket.runRaw` below, AFTER `pty.connect` resolves // and the message callback is registered. The client therefore can't fire diff --git a/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts b/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts index 9230a6fe57..99645f3da3 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts @@ -234,7 +234,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", // share/unshare errors aren't all client-induced — storage and network // failures from SessionShare are real possibilities. Map to a typed 500 - // (matches the legacy Hono path which routed any failure through + // (matches the legacy route behavior which routed any failure through // ErrorMiddleware → NamedError.Unknown 500) instead of blanket-mapping // every failure to a 400 BadRequest. const share = Effect.fn("SessionHttpApi.share")(function* (ctx: { params: { sessionID: SessionID } }) { diff --git a/packages/opencode/src/server/routes/instance/httpapi/middleware/compression.ts b/packages/opencode/src/server/routes/instance/httpapi/middleware/compression.ts index 9dc9bc01ec..9187bfea34 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/middleware/compression.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/middleware/compression.ts @@ -2,7 +2,7 @@ import { deflateSync, gzipSync } from "node:zlib" import { Effect } from "effect" import { HttpBody, HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/unstable/http" -// Mirror of Hono's compressible content-type set so wire behavior matches. +// Keep the server's compressible content-type set stable across HTTP backend changes. const COMPRESSIBLE_CONTENT_TYPE_REGEX = /^\s*(?:text\/(?!event-stream(?:[;\s]|$))[^;\s]+|application\/(?:javascript|json|xml|xml-dtd|ecmascript|dart|postscript|rtf|tar|toml|vnd\.dart|vnd\.ms-fontobject|vnd\.ms-opentype|wasm|x-httpd-php|x-javascript|x-ns-proxy-autoconfig|x-sh|x-tar|x-www-form-urlencoded)|font\/(?:otf|ttf)|image\/(?:bmp|vnd\.adobe\.photoshop|vnd\.microsoft\.icon|vnd\.ms-dds|x-icon|x-ms-bmp)|message\/rfc822|model\/gltf-binary|x-shader\/x-fragment|x-shader\/x-vertex|[^;\s]+?\+(?:json|text|xml|yaml))(?:[;\s]|$)/i diff --git a/packages/opencode/src/server/routes/instance/httpapi/public.ts b/packages/opencode/src/server/routes/instance/httpapi/public.ts index 460a2be7a5..12d3791ecc 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/public.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/public.ts @@ -111,8 +111,8 @@ function matchLegacyOpenApi(input: Record) { const operation = item[method] if (!operation) continue if (operation.requestBody) { - // Hono's generated OpenAPI never marked request bodies as required. Keep - // that SDK surface stable during the HttpApi migration. + // The legacy OpenAPI surface never marked request bodies as required. + // Keep that SDK surface stable while the HttpApi spec is tightened. delete operation.requestBody.required const body = operation.requestBody.content?.["application/json"] if (body?.schema) body.schema = stripOptionalNull(structuredClone(body.schema)) @@ -146,8 +146,8 @@ function matchLegacyOpenApi(input: Record) { if (content.schema) content.schema = stripOptionalNull(structuredClone(content.schema)) } } - // Hono applied auth as runtime middleware outside OpenAPI metadata, so the - // legacy SDK did not expose auth schemes or generated 401 error unions. + // Auth is still runtime middleware outside the public OpenAPI metadata, so + // the SDK should not expose auth schemes or generated 401 error unions. delete operation.security delete operation.responses?.["401"] normalizeLegacyErrorResponses(operation) diff --git a/packages/opencode/test/project/instance-bootstrap.test.ts b/packages/opencode/test/project/instance-bootstrap.test.ts index baad8df592..71521a765a 100644 --- a/packages/opencode/test/project/instance-bootstrap.test.ts +++ b/packages/opencode/test/project/instance-bootstrap.test.ts @@ -13,9 +13,7 @@ import { disposeAllInstances, tmpdir } from "../fixture/fixture" // bodies deliberately avoid Plugin/config directly. The marker only // appears if InstanceBootstrap ran at the instance boundary. // -// The Hono variant of this check lived alongside these tests and is -// going away with the Hono backend. The boundaries below are backend- -// agnostic and stay. +// The boundaries below are transport-agnostic and stay. afterEach(async () => { await disposeAllInstances() diff --git a/packages/opencode/test/server/httpapi-config.test.ts b/packages/opencode/test/server/httpapi-config.test.ts index 509a067d08..26c0fe03e6 100644 --- a/packages/opencode/test/server/httpapi-config.test.ts +++ b/packages/opencode/test/server/httpapi-config.test.ts @@ -25,7 +25,7 @@ afterEach(async () => { }) describe("config HttpApi", () => { - test("serves config update through Hono bridge", async () => { + test("serves config update through the default server app", async () => { await using tmp = await tmpdir({ config: { formatter: false, lsp: false } }) const disposed = waitDisposed(tmp.path) diff --git a/packages/opencode/test/server/httpapi-experimental.test.ts b/packages/opencode/test/server/httpapi-experimental.test.ts index 0b8d8051bc..383442e00e 100644 --- a/packages/opencode/test/server/httpapi-experimental.test.ts +++ b/packages/opencode/test/server/httpapi-experimental.test.ts @@ -1,6 +1,5 @@ import { afterEach, describe, expect, test } from "bun:test" import { Effect } from "effect" -import { Instance } from "../../src/project/instance" import { WithInstance } from "../../src/project/with-instance" import { Server } from "../../src/server/server" import { ExperimentalPaths } from "../../src/server/routes/instance/httpapi/groups/experimental" @@ -41,7 +40,7 @@ afterEach(async () => { }) describe("experimental HttpApi", () => { - test("serves read-only experimental endpoints through Hono bridge", async () => { + test("serves read-only experimental endpoints through the default server app", async () => { await using tmp = await tmpdir({ config: { formatter: false, @@ -94,7 +93,7 @@ describe("experimental HttpApi", () => { expect(await resources.json()).toEqual({}) }) - test("serves Console org switch through Hono bridge", async () => { + test("serves Console org switch through the default server app", async () => { await using tmp = await tmpdir({ config: { formatter: false, lsp: false } }) Database.Client() .$client.prepare( @@ -120,7 +119,7 @@ describe("experimental HttpApi", () => { expect(await switched.json()).toBe(true) }) - test("serves global session list through Hono bridge", async () => { + test("serves global session list through the default server app", async () => { await using tmp = await tmpdir({ git: true, config: { formatter: false, lsp: false } }) const first = await WithInstance.provide({ @@ -157,7 +156,7 @@ describe("experimental HttpApi", () => { expect(((await next.json()) as Session.GlobalInfo[]).map((session) => session.id)).toContain(first.id) }) - testWorktreeMutations("serves worktree mutations through Hono bridge", async () => { + testWorktreeMutations("serves worktree mutations through the default server app", async () => { await using tmp = await tmpdir({ git: true, config: { formatter: false, lsp: false } }) const headers = { "x-opencode-directory": tmp.path, "content-type": "application/json" } diff --git a/packages/opencode/test/server/httpapi-instance.test.ts b/packages/opencode/test/server/httpapi-instance.test.ts index 946de2835c..5ec4acb877 100644 --- a/packages/opencode/test/server/httpapi-instance.test.ts +++ b/packages/opencode/test/server/httpapi-instance.test.ts @@ -11,7 +11,7 @@ import { SessionPaths } from "../../src/server/routes/instance/httpapi/groups/se import { ExperimentalHttpApiServer } from "../../src/server/routes/instance/httpapi/server" import { HEADER as FenceHeader } from "../../src/server/shared/fence" import { resetDatabase } from "../fixture/db" -import { disposeAllInstances, tmpdirScoped } from "../fixture/fixture" +import { tmpdirScoped } from "../fixture/fixture" import { testEffect } from "../lib/effect" // Flip the experimental workspaces flag so SyncEvent.run actually writes to @@ -35,8 +35,7 @@ const testStateLayer = Layer.effectDiscard( // Mount the production HttpApi route tree on a real Node HTTP server bound to // 127.0.0.1:0 and a fetch-based HttpClient that prepends the server URL. This -// keeps the test wired through the same route layer production uses, without -// going through Server.Default()/Hono. +// keeps the test wired directly through the same route layer production uses. const servedRoutes: Layer.Layer = HttpRouter.serve( ExperimentalHttpApiServer.routes, { disableListenLog: true, disableLogger: true }, diff --git a/specs/v2/todo.md b/specs/v2/todo.md index 77c650e55f..ca38931f8f 100644 --- a/specs/v2/todo.md +++ b/specs/v2/todo.md @@ -2,9 +2,11 @@ ok we need to work towards a launch of v2 so we can get out of this rebuild phase -## Kill Hono - Kit +## Post-Hono cleanup - Kit -Hono needs to go away so zod can go away. this is almost done +The opencode server has moved to the Effect HttpApi backend. Remaining work is +mostly cleanup: delete compatibility shims, shrink Zod surfaces, and simplify +test harnesses that used to compare Hono and HttpApi behavior. ## New Data Mode - Dax