diff --git a/packages/opencode/specs/effect/errors.md b/packages/opencode/specs/effect/errors.md new file mode 100644 index 0000000000..746e658693 --- /dev/null +++ b/packages/opencode/specs/effect/errors.md @@ -0,0 +1,329 @@ +# Typed error migration + +Plan for moving `packages/opencode` from temporary defect/`NamedError` +compatibility toward typed Effect service errors and explicit HTTP error +contracts. + +## Goal + +- Expected service failures live on the Effect error channel. +- Service interfaces expose those failures in their return types. +- Domain errors are authored with Effect Schema so they are reusable by services, + tests, HTTP routes, tools, and OpenAPI generation. +- HTTP status codes and wire compatibility are handled at the HTTP boundary, not + inside service modules. +- `Effect.die`, `throw`, `catchDefect`, and global cause inspection are reserved + for defects, compatibility bridges, or final fallback behavior. + +## Current State + +- Many migrated services use Effect internally, but expected failures are still a + mix of `NamedError.create(...)`, `namedSchemaError(...)`, `class extends Error`, + `throw`, and `Effect.die(...)`. +- 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. +- 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. +- The temporary HttpApi error middleware catches defect-wrapped legacy errors to + preserve runtime behavior, but it is intentionally a bridge rather than the + final model. + +## End State + +Service modules own domain failures. + +```ts +export class SessionBusyError extends Schema.TaggedErrorClass()("SessionBusyError", { + sessionID: SessionID, + message: Schema.String, +}) {} + +export type Error = Storage.Error | SessionBusyError + +export interface Interface { + readonly get: (id: SessionID) => Effect.Effect +} +``` + +HTTP modules own transport mapping. + +```ts +const get = Effect.fn("SessionHttpApi.get")(function* (ctx: { params: { sessionID: SessionID } }) { + return yield* session + .get(ctx.params.sessionID) + .pipe( + Effect.catchTag("StorageNotFoundError", () => new SessionNotFoundHttpError({ sessionID: ctx.params.sessionID })), + ) +}) +``` + +HTTP-visible error schemas carry their own response status through Effect +HttpApi's `httpApiStatus` annotation. Prefer `HttpApiSchema.status(...)`, or the +equivalent declaration annotation, instead of maintaining a parallel status map. + +```ts +export class SessionNotFoundHttpError extends Schema.TaggedErrorClass()( + "SessionNotFoundHttpError", + { + sessionID: SessionID, + message: Schema.String, + }, + { httpApiStatus: 404 }, +) {} +``` + +Endpoint definitions still declare which HTTP-visible error schemas can be +emitted. The status annotation is only used if the error is part of the endpoint, +group, or middleware error schema and the handler fails with that error on the +typed error channel. + +```ts +HttpApiEndpoint.get("get", SessionPaths.get, { + success: Session.Info, + error: [SessionNotFoundHttpError, SessionBusyHttpError], +}) +``` + +The service error and HTTP error may be the same class when the wire shape is a +deliberate public contract. They should be different classes when the service +error contains internals, low-level causes, retry hints, or anything that should +not be exposed to API clients. + +## Rules + +- Use `Schema.TaggedErrorClass` for new expected domain errors. +- Include `cause: Schema.optional(Schema.Defect)` only when preserving an + underlying unknown failure is useful for logs or callers. +- Export a domain-level error union from each service module, for example + `export type Error = NotFoundError | BusyError | Storage.Error`. +- Put expected errors in service method signatures, for example + `Effect.Effect`. +- Use `yield* new DomainError(...)` for direct early failures inside + `Effect.gen` / `Effect.fn`. +- Use `Effect.try({ try, catch })`, `Effect.mapError`, or `Effect.catchTag` to + convert external exceptions into domain errors. +- Use `HttpApiSchema.status(...)` or `{ httpApiStatus: code }` on HTTP-visible + error schemas so Effect `HttpApiBuilder` and OpenAPI generation get the status + from the schema itself. +- Do not use `Effect.die(...)` for user, IO, validation, missing-resource, auth, + provider, worktree, or busy-state failures. +- Do not use `catchDefect` to recover expected domain errors. If recovery is + needed, the upstream effect should fail with a typed error instead. +- Do not make service modules import `HttpApiError`, `HttpServerResponse`, HTTP + status codes, or route-specific error schemas. +- Keep raw `HttpRouter` routes free to use `HttpServerRespondable` when that is + the right transport abstraction, but prefer declared `HttpApi` errors for + normal JSON API endpoints. + +## HTTP Boundary Shape + +Create an HttpApi-local error module, likely +`src/server/routes/instance/httpapi/errors.ts`. + +That module should provide: + +- Legacy-compatible public schemas for `{ name, data }` error bodies that must + remain SDK-compatible during the Hono migration. +- 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 + data. +- A single place to document which public error shape is legacy-compatible and + which shape is new Effect-native API surface. + +Avoid one giant `unknown -> status` mapper. Prefer small, explicit mappers close +to the handler or route group. + +```ts +const mapSessionError = (effect: Effect.Effect) => + effect.pipe( + Effect.catchTag("StorageNotFoundError", (error) => new SessionNotFoundHttpError({ message: error.message })), + Effect.catchTag("SessionBusyError", (error) => new SessionBusyHttpError({ message: error.message })), + ) +``` + +Use built-in `HttpApiError.BadRequest`, `HttpApiError.NotFound`, and related +types only when their generated response body and SDK surface are intentionally +acceptable. Use a custom schema-backed error when clients need the legacy +`{ name, data }` body or a domain-specific error payload. + +## Migration Phases + +### 1. Stabilize The Bridge + +Keep the temporary HttpApi error middleware only as a compatibility bridge while +typed errors are introduced. + +- Add tests that prove the bridge catches legacy `NamedError` defects. +- Add tests that prove declared HttpApi errors still use the declared endpoint + contract. +- Stop returning stack traces in unknown HTTP `500` responses; log the full + `Cause.pretty(cause)` server-side instead. +- Add a comment or TODO that names this plan and states the bridge must shrink + as route groups migrate. + +### 2. Define The Shared HTTP Error Helpers + +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. +- 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 + accept `unknown` unless they are final fallback helpers. + +### 3. Convert One Vertical Slice + +Start with session read routes because they already have local `mapNotFound` +logic and are heavily covered by existing HttpApi tests. + +- Convert `Session.BusyError` from a plain `Error` to a typed service error, or + add a typed wrapper while preserving the old constructor until callers are + migrated. +- Replace `catchDefect` in `httpapi/handlers/session.ts` with typed error + mapping. +- Add endpoint error schemas for the affected session endpoints. +- Prove behavior with focused tests in `test/server/httpapi-session.test.ts`. +- Remove the migrated cases from the global compatibility middleware. + +### 4. Convert Legacy NamedError Domains + +Move legacy `NamedError.create(...)` services to Effect Schema-backed errors in +small domain PRs. + +Priority order: + +1. `storage/storage.ts` and `storage/db.ts` not-found errors. +2. `worktree/index.ts` `Worktree*` errors. +3. `provider/auth.ts` validation failures and `provider/provider.ts` model-not-found errors. +4. `mcp/index.ts`, `skill/index.ts`, `lsp/client.ts`, and `ide/index.ts` service errors. +5. Config and CLI-only errors after HTTP-facing domains are stable. + +For each domain: + +- Replace `NamedError.create(...)` with `Schema.TaggedErrorClass` when the error + is primarily a service error. +- Keep or add a separate HTTP error schema when the legacy `{ name, data }` wire + shape must remain stable. +- Update service interface return types to include the new error union. +- Replace `throw new X(...)` inside `Effect.fn` with `yield* new X(...)`. +- Replace async exceptions with `Effect.try({ catch })` or explicit `mapError`. +- Add service-level tests that assert the error tag and data, not just the HTTP + status. + +### 5. Declare HttpApi Errors Group By Group + +For each HttpApi group: + +- Inventory every service call and the typed errors it can return. +- Add only the public error schemas that endpoint can actually emit. +- Map service errors to HTTP errors in the handler file. +- Keep built-in `HttpApiError` only for generic request/validation failures where + the generated contract is accepted. +- Update `httpapi/public.ts` compatibility transforms only when the generated + spec cannot represent the desired source shape directly. +- Regenerate the SDK after OpenAPI-visible changes and verify the diff is + intentional. + +Suggested route order: + +1. `session` not-found and busy-state reads. +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. + +### 6. Remove Defect Recovery + +After enough route groups declare their expected errors: + +- Delete `catchDefect` recovery for domain errors. +- Delete name-prefix checks such as `error.name.startsWith("Worktree")` from + HTTP middleware. +- Delete `NamedError` branches from the Effect HttpApi compatibility middleware + once no Effect route depends on them. +- Leave one final unknown-defect fallback that logs server-side and returns a + safe generic `500` body. + +## Inventory Checklist + +Use this checklist when touching a service or route group. + +- [ ] Does the service interface expose every expected failure in the Effect + error type? +- [ ] Are user-caused, provider-caused, IO, auth, missing-resource, and busy-state + failures modeled as typed errors instead of defects? +- [ ] Does the service avoid importing HTTP status, `HttpApiError`, or response + classes? +- [ ] Does the handler map each service error into a declared endpoint error? +- [ ] Does the endpoint `error` field include every public error the handler can + emit? +- [ ] Does OpenAPI/SDK output either stay byte-identical or have an explicitly + reviewed diff? +- [ ] Do tests cover both service-level error typing and HTTP-level status/body? +- [ ] Did the PR remove any now-unneeded case from the temporary compatibility + middleware? + +## Testing Requirements + +For service conversions: + +- Test the service method directly with `testEffect(...)`. +- Assert on `_tag` or class identity and the structured fields. +- Avoid testing by string-matching `Cause.pretty(...)`. + +For HttpApi conversions: + +- Add or update the focused `test/server/httpapi-*.test.ts` file. +- Assert status code, content type, and exact JSON body for declared public + 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. + +## Verification Commands + +Run from `packages/opencode` unless noted otherwise. + +```bash +bun run prettier --write +bunx oxlint +bun typecheck +bun run test -- test/server/httpapi-session.test.ts +``` + +Run SDK generation from the repo root when schemas or OpenAPI-visible errors +change. + +```bash +./packages/sdk/js/script/build.ts +``` + +## Open Questions + +- Should legacy V1 routes keep `{ name, data }` forever while V2 routes expose a + more Effect-native tagged error body? +- Should storage not-found remain generic, or should callers map it to + domain-specific not-found errors before crossing service boundaries? +- Should `namedSchemaError(...)` stay as a long-term public-wire helper, or only + as a migration bridge for old `NamedError` contracts? +- Which SDK version boundary lets us stop remapping built-in Effect HttpApi error + schemas in `httpapi/public.ts`? + +## Success Criteria + +- New service code no longer uses `die` for expected failures. +- A route reviewer can read an endpoint definition and see every public error it + can return. +- The temporary HttpApi error middleware shrinks over time instead of gaining new + name-based cases. +- Service tests prove domain error types without going through HTTP. +- HTTP tests prove status/body contracts without relying on defect recovery. diff --git a/packages/opencode/src/cli/cmd/session.ts b/packages/opencode/src/cli/cmd/session.ts index 08c0df929c..33f1e78ac0 100644 --- a/packages/opencode/src/cli/cmd/session.ts +++ b/packages/opencode/src/cli/cmd/session.ts @@ -9,6 +9,7 @@ import { Locale } from "@/util/locale" import { Flag } from "@opencode-ai/core/flag/flag" import { Filesystem } from "@/util/filesystem" import { Process } from "@/util/process" +import { NotFoundError } from "@/storage/storage" import { EOL } from "os" import path from "path" import { which } from "../../util/which" @@ -59,9 +60,9 @@ export const SessionDeleteCommand = effectCmd({ handler: Effect.fn("Cli.session.delete")(function* (args) { const svc = yield* Session.Service const sessionID = SessionID.make(args.sessionID) - // Match legacy try/catch — Session.get surfaces NotFoundError as a defect. - yield* svc.get(sessionID).pipe(Effect.catchCause(() => fail(`Session not found: ${args.sessionID}`))) - yield* svc.remove(sessionID) + yield* svc.remove(sessionID).pipe( + Effect.catchIf(NotFoundError.isInstance, () => fail(`Session not found: ${args.sessionID}`)), + ) UI.println(UI.Style.TEXT_SUCCESS_BOLD + `Session ${args.sessionID} deleted` + UI.Style.TEXT_NORMAL) }), }) diff --git a/packages/opencode/src/control-plane/workspace.ts b/packages/opencode/src/control-plane/workspace.ts index fe651fe3e3..24ca0e61bf 100644 --- a/packages/opencode/src/control-plane/workspace.ts +++ b/packages/opencode/src/control-plane/workspace.ts @@ -24,11 +24,12 @@ import { Session } from "@/session/session" import { SessionPrompt } from "@/session/prompt" import { SessionTable } from "@/session/session.sql" import { SessionID } from "@/session/schema" +import { NotFoundError } from "@/storage/storage" import { errorData } from "@/util/error" import { waitEvent } from "./util" import { WorkspaceContext } from "./workspace-context" import { EffectBridge } from "@/effect/bridge" -import { NonNegativeInt, withStatics } from "@/util/schema" +import { withStatics } from "@/util/schema" import { zod as effectZod, zodObject } from "@/util/effect-zod" export const Info = WorkspaceInfoSchema @@ -739,9 +740,19 @@ export const layer = Layer.effect( const remove = Effect.fn("Workspace.remove")(function* (id: WorkspaceID) { const sessions = yield* db((db) => - db.select({ id: SessionTable.id }).from(SessionTable).where(eq(SessionTable.workspace_id, id)).all(), + db + .select({ id: SessionTable.id, parentID: SessionTable.parent_id }) + .from(SessionTable) + .where(eq(SessionTable.workspace_id, id)) + .all(), + ) + const sessionIDs = new Set(sessions.map((sessionInfo) => sessionInfo.id)) + yield* Effect.forEach( + sessions.filter((sessionInfo) => !sessionInfo.parentID || !sessionIDs.has(sessionInfo.parentID)), + (sessionInfo) => + session.remove(sessionInfo.id).pipe(Effect.catchIf(NotFoundError.isInstance, () => Effect.void)), + { discard: true }, ) - yield* Effect.forEach(sessions, (sessionInfo) => session.remove(sessionInfo.id), { discard: true }) const row = yield* db((db) => db.select().from(WorkspaceTable).where(eq(WorkspaceTable.id, id)).get()) if (!row) return diff --git a/packages/opencode/src/server/routes/instance/httpapi/AGENTS.md b/packages/opencode/src/server/routes/instance/httpapi/AGENTS.md index 757d7aed0c..a6ccf794dd 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/AGENTS.md +++ b/packages/opencode/src/server/routes/instance/httpapi/AGENTS.md @@ -32,4 +32,6 @@ Avoid `HttpRouter.provideRequest(...)` unless the dependency is intentionally re Use `Effect.provideService(...)` in middleware only for request-derived context, such as `WorkspaceRouteContext`, `InstanceRef`, or `WorkspaceRef`. Do not use it to smuggle stable services through request effects when they can be yielded at layer construction. +Public JSON errors should be explicit `Schema.ErrorClass` contracts declared on each endpoint. Use built-in `HttpApiError.*` classes only when their empty/tagged body is the intended wire shape; for SDK-visible errors with messages, define an API error schema such as `ApiNotFoundError` and fail with that exact declared error. Keep domain and storage services free of HttpApi types, and translate expected domain errors at the handler boundary. + When adding middleware, compose it at the layer boundary and keep the route tree explicit in `server.ts`. Shared router middleware such as auth, workspace routing, and instance context should stay visible where routes are assembled. diff --git a/packages/opencode/src/server/routes/instance/httpapi/errors.ts b/packages/opencode/src/server/routes/instance/httpapi/errors.ts new file mode 100644 index 0000000000..e5df6f5abf --- /dev/null +++ b/packages/opencode/src/server/routes/instance/httpapi/errors.ts @@ -0,0 +1,18 @@ +import { Schema } from "effect" + +export class ApiNotFoundError extends Schema.ErrorClass("NotFoundError")( + { + name: Schema.Literal("NotFoundError"), + data: Schema.Struct({ + message: Schema.String, + }), + }, + { httpApiStatus: 404 }, +) {} + +export function notFound(message: string) { + return new ApiNotFoundError({ + name: "NotFoundError", + data: { message }, + }) +} diff --git a/packages/opencode/src/server/routes/instance/httpapi/groups/pty.ts b/packages/opencode/src/server/routes/instance/httpapi/groups/pty.ts index 3304ab9fbf..ad513e0ad4 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/groups/pty.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/groups/pty.ts @@ -6,6 +6,7 @@ import { HttpApi, HttpApiEndpoint, HttpApiError, HttpApiGroup, OpenApi } from "e import { Authorization } from "../middleware/authorization" import { InstanceContextMiddleware } from "../middleware/instance-context" import { WorkspaceRoutingMiddleware } from "../middleware/workspace-routing" +import { ApiNotFoundError } from "../errors" import { described } from "./metadata" const root = "/pty" @@ -64,7 +65,7 @@ export const PtyApi = HttpApi.make("pty") HttpApiEndpoint.get("get", PtyPaths.get, { params: { ptyID: PtyID }, success: described(Pty.Info, "Session info"), - error: HttpApiError.NotFound, + error: ApiNotFoundError, }).annotateMerge( OpenApi.annotations({ identifier: "pty.get", @@ -76,7 +77,7 @@ export const PtyApi = HttpApi.make("pty") params: { ptyID: PtyID }, payload: Pty.UpdateInput, success: described(Pty.Info, "Updated session"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "pty.update", @@ -87,7 +88,7 @@ export const PtyApi = HttpApi.make("pty") HttpApiEndpoint.delete("remove", PtyPaths.remove, { params: { ptyID: PtyID }, success: described(Schema.Boolean, "Session removed"), - error: HttpApiError.NotFound, + error: ApiNotFoundError, }).annotateMerge( OpenApi.annotations({ identifier: "pty.remove", @@ -98,7 +99,7 @@ export const PtyApi = HttpApi.make("pty") HttpApiEndpoint.post("connectToken", PtyPaths.connectToken, { params: { ptyID: PtyID }, success: described(PtyTicket.ConnectToken, "WebSocket connect token"), - error: [HttpApiError.Forbidden, HttpApiError.NotFound], + error: [HttpApiError.Forbidden, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "pty.connectToken", diff --git a/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts b/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts index 77d064ff5a..1159c88030 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts @@ -15,6 +15,7 @@ import { HttpApi, HttpApiEndpoint, HttpApiError, HttpApiGroup, HttpApiSchema, Op import { Authorization } from "../middleware/authorization" import { InstanceContextMiddleware } from "../middleware/instance-context" import { WorkspaceRoutingMiddleware } from "../middleware/workspace-routing" +import { ApiNotFoundError } from "../errors" import { described } from "./metadata" const root = "/session" @@ -123,7 +124,7 @@ export const SessionApi = HttpApi.make("session") HttpApiEndpoint.get("get", SessionPaths.get, { params: { sessionID: SessionID }, success: described(Session.Info, "Get session"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.get", @@ -168,7 +169,7 @@ export const SessionApi = HttpApi.make("session") params: { sessionID: SessionID }, query: MessagesQuery, success: described(Schema.Array(MessageV2.WithParts), "List of messages"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.messages", @@ -179,7 +180,7 @@ export const SessionApi = HttpApi.make("session") HttpApiEndpoint.get("message", SessionPaths.message, { params: { sessionID: SessionID, messageID: MessageID }, success: described(MessageV2.WithParts, "Message"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.message", @@ -201,7 +202,7 @@ export const SessionApi = HttpApi.make("session") HttpApiEndpoint.delete("remove", SessionPaths.remove, { params: { sessionID: SessionID }, success: described(Schema.Boolean, "Successfully deleted session"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.delete", @@ -213,7 +214,7 @@ export const SessionApi = HttpApi.make("session") params: { sessionID: SessionID }, payload: UpdatePayload, success: described(Session.Info, "Successfully updated session"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.update", @@ -225,6 +226,7 @@ export const SessionApi = HttpApi.make("session") params: { sessionID: SessionID }, payload: ForkPayload, success: described(Session.Info, "200"), + error: ApiNotFoundError, }).annotateMerge( OpenApi.annotations({ identifier: "session.fork", @@ -259,7 +261,7 @@ export const SessionApi = HttpApi.make("session") HttpApiEndpoint.post("share", SessionPaths.share, { params: { sessionID: SessionID }, success: described(Session.Info, "Successfully shared session"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.share", @@ -270,7 +272,7 @@ export const SessionApi = HttpApi.make("session") HttpApiEndpoint.delete("unshare", SessionPaths.share, { params: { sessionID: SessionID }, success: described(Session.Info, "Successfully unshared session"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.unshare", @@ -282,7 +284,7 @@ export const SessionApi = HttpApi.make("session") params: { sessionID: SessionID }, payload: SummarizePayload, success: described(Schema.Boolean, "Summarized session"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.summarize", diff --git a/packages/opencode/src/server/routes/instance/httpapi/groups/tui.ts b/packages/opencode/src/server/routes/instance/httpapi/groups/tui.ts index efe73d95d1..8ab43f6654 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/groups/tui.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/groups/tui.ts @@ -4,6 +4,7 @@ import { HttpApi, HttpApiEndpoint, HttpApiError, HttpApiGroup, OpenApi } from "e import { Authorization } from "../middleware/authorization" import { InstanceContextMiddleware } from "../middleware/instance-context" import { WorkspaceRoutingMiddleware } from "../middleware/workspace-routing" +import { ApiNotFoundError } from "../errors" import { described } from "./metadata" const root = "/tui" @@ -155,7 +156,7 @@ export const TuiApi = HttpApi.make("tui") HttpApiEndpoint.post("selectSession", TuiPaths.selectSession, { payload: TuiEvent.SessionSelect.properties, success: described(Schema.Boolean, "Session selected successfully"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "tui.selectSession", 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 e5ff300a2a..7b8395d809 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/pty.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/pty.ts @@ -15,6 +15,7 @@ import { HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/unstab import { HttpApiBuilder, HttpApiError } from "effect/unstable/httpapi" import * as Socket from "effect/unstable/socket/Socket" import { InstanceHttpApi } from "../api" +import * as ApiError from "../errors" import { CursorQuery, Params, PtyPaths } from "../groups/pty" import { WebSocketTracker } from "../websocket-tracker" @@ -46,7 +47,7 @@ export const ptyHandlers = HttpApiBuilder.group(InstanceHttpApi, "pty", (handler const get = Effect.fn("PtyHttpApi.get")(function* (ctx: { params: { ptyID: PtyID } }) { const info = yield* pty.get(ctx.params.ptyID) - if (!info) return yield* new HttpApiError.NotFound({}) + if (!info) return yield* ApiError.notFound("Session not found") return info }) @@ -58,7 +59,7 @@ export const ptyHandlers = HttpApiBuilder.group(InstanceHttpApi, "pty", (handler ...ctx.payload, size: ctx.payload.size ? { ...ctx.payload.size } : undefined, }) - if (!info) return yield* new HttpApiError.NotFound({}) + if (!info) return yield* ApiError.notFound("Session not found") return info }) @@ -71,7 +72,7 @@ export const ptyHandlers = HttpApiBuilder.group(InstanceHttpApi, "pty", (handler const request = yield* HttpServerRequest.HttpServerRequest if (request.headers[PTY_CONNECT_TOKEN_HEADER] !== PTY_CONNECT_TOKEN_HEADER_VALUE || !validOrigin(request, cors)) return yield* new HttpApiError.Forbidden({}) - if (!(yield* pty.get(ctx.params.ptyID))) return yield* new HttpApiError.NotFound({}) + if (!(yield* pty.get(ctx.params.ptyID))) return yield* ApiError.notFound("Session not found") return yield* tickets.issue({ ptyID: ctx.params.ptyID, ...(yield* PtyTicket.scope) }) }) diff --git a/packages/opencode/src/server/routes/instance/httpapi/handlers/session-errors.ts b/packages/opencode/src/server/routes/instance/httpapi/handlers/session-errors.ts new file mode 100644 index 0000000000..98ac2b9ad6 --- /dev/null +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/session-errors.ts @@ -0,0 +1,9 @@ +import type { NotFoundError as StorageNotFoundError } from "@/storage/storage" +import { Effect } from "effect" +import * as ApiError from "../errors" + +type StorageNotFound = InstanceType + +export function mapStorageNotFound(self: Effect.Effect) { + return self.pipe(Effect.mapError((error) => ApiError.notFound(error.data.message))) +} 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 4a67ba036e..56fa7adb15 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts @@ -37,14 +37,7 @@ import { SummarizePayload, UpdatePayload, } from "../groups/session" - -const mapNotFound = (self: Effect.Effect) => - self.pipe( - Effect.catchIf(NotFoundError.isInstance, () => Effect.fail(new HttpApiError.NotFound({}))), - Effect.catchDefect((error) => - NotFoundError.isInstance(error) ? Effect.fail(new HttpApiError.NotFound({})) : Effect.die(error), - ), - ) +import * as SessionError from "./session-errors" export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", (handlers) => Effect.gen(function* () { @@ -79,7 +72,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", }) const get = Effect.fn("SessionHttpApi.get")(function* (ctx: { params: { sessionID: SessionID } }) { - return yield* mapNotFound(session.get(ctx.params.sessionID)) + return yield* SessionError.mapStorageNotFound(session.get(ctx.params.sessionID)) }) const children = Effect.fn("SessionHttpApi.children")(function* (ctx: { params: { sessionID: SessionID } }) { @@ -101,51 +94,49 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", params: { sessionID: SessionID } query: typeof MessagesQuery.Type }) { - return yield* mapNotFound( - Effect.gen(function* () { - if (ctx.query.before && ctx.query.limit === undefined) return yield* new HttpApiError.BadRequest({}) - if (ctx.query.before) { - const before = ctx.query.before - yield* Effect.try({ - try: () => MessageV2.cursor.decode(before), - catch: () => new HttpApiError.BadRequest({}), - }) - } - if (ctx.query.limit === undefined || ctx.query.limit === 0) { - yield* session.get(ctx.params.sessionID) - return yield* session.messages({ sessionID: ctx.params.sessionID }) - } + if (ctx.query.before && ctx.query.limit === undefined) return yield* new HttpApiError.BadRequest({}) + if (ctx.query.before) { + const before = ctx.query.before + yield* Effect.try({ + try: () => MessageV2.cursor.decode(before), + catch: () => new HttpApiError.BadRequest({}), + }) + } + yield* SessionError.mapStorageNotFound(session.get(ctx.params.sessionID)) + if (ctx.query.limit === undefined || ctx.query.limit === 0) { + return yield* session.messages({ sessionID: ctx.params.sessionID }) + } - yield* session.get(ctx.params.sessionID) - const page = MessageV2.page({ - sessionID: ctx.params.sessionID, - limit: ctx.query.limit, - before: ctx.query.before, - }) - if (!page.cursor) return page.items + const page = MessageV2.page({ + sessionID: ctx.params.sessionID, + limit: ctx.query.limit, + before: ctx.query.before, + }) + if (!page.cursor) return page.items - const request = yield* HttpServerRequest.HttpServerRequest - // toURL() honors the Host + x-forwarded-proto headers, so the Link - // header echoes the real origin instead of a hard-coded localhost. - const url = Option.getOrElse(HttpServerRequest.toURL(request), () => new URL(request.url, "http://localhost")) - url.searchParams.set("limit", ctx.query.limit.toString()) - url.searchParams.set("before", page.cursor) - return HttpServerResponse.jsonUnsafe(page.items, { - headers: { - "Access-Control-Expose-Headers": "Link, X-Next-Cursor", - Link: `<${url.toString()}>; rel="next"`, - "X-Next-Cursor": page.cursor, - }, - }) - }), - ) + const request = yield* HttpServerRequest.HttpServerRequest + // toURL() honors the Host + x-forwarded-proto headers, so the Link + // header echoes the real origin instead of a hard-coded localhost. + const url = Option.getOrElse(HttpServerRequest.toURL(request), () => new URL(request.url, "http://localhost")) + url.searchParams.set("limit", ctx.query.limit.toString()) + url.searchParams.set("before", page.cursor) + return HttpServerResponse.jsonUnsafe(page.items, { + headers: { + "Access-Control-Expose-Headers": "Link, X-Next-Cursor", + Link: `<${url.toString()}>; rel="next"`, + "X-Next-Cursor": page.cursor, + }, + }) }) const message = Effect.fn("SessionHttpApi.message")(function* (ctx: { params: { sessionID: SessionID; messageID: MessageID } }) { - return yield* mapNotFound( - Effect.sync(() => MessageV2.get({ sessionID: ctx.params.sessionID, messageID: ctx.params.messageID })), + return yield* SessionError.mapStorageNotFound( + Effect.try({ + try: () => MessageV2.get({ sessionID: ctx.params.sessionID, messageID: ctx.params.messageID }), + catch: (error) => error, + }).pipe(Effect.catch((error) => (NotFoundError.isInstance(error) ? Effect.fail(error) : Effect.die(error)))), ) }) @@ -170,7 +161,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", }) const remove = Effect.fn("SessionHttpApi.remove")(function* (ctx: { params: { sessionID: SessionID } }) { - yield* session.remove(ctx.params.sessionID) + yield* SessionError.mapStorageNotFound(session.remove(ctx.params.sessionID)) return true }) @@ -178,7 +169,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", params: { sessionID: SessionID } payload: typeof UpdatePayload.Type }) { - const current = yield* session.get(ctx.params.sessionID) + const current = yield* SessionError.mapStorageNotFound(session.get(ctx.params.sessionID)) if (ctx.payload.title !== undefined) { yield* session.setTitle({ sessionID: ctx.params.sessionID, title: ctx.payload.title }) } @@ -191,14 +182,16 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", if (ctx.payload.time?.archived !== undefined) { yield* session.setArchived({ sessionID: ctx.params.sessionID, time: ctx.payload.time.archived }) } - return yield* session.get(ctx.params.sessionID) + return yield* SessionError.mapStorageNotFound(session.get(ctx.params.sessionID)) }) const fork = Effect.fn("SessionHttpApi.fork")(function* (ctx: { params: { sessionID: SessionID } payload: typeof ForkPayload.Type }) { - return yield* session.fork({ sessionID: ctx.params.sessionID, messageID: ctx.payload.messageID }) + return yield* SessionError.mapStorageNotFound( + session.fork({ sessionID: ctx.params.sessionID, messageID: ctx.payload.messageID }), + ) }) const abort = Effect.fn("SessionHttpApi.abort")(function* (ctx: { params: { sessionID: SessionID } }) { @@ -222,19 +215,19 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", const share = Effect.fn("SessionHttpApi.share")(function* (ctx: { params: { sessionID: SessionID } }) { yield* shareSvc.share(ctx.params.sessionID).pipe(Effect.mapError(() => new HttpApiError.BadRequest({}))) - return yield* session.get(ctx.params.sessionID) + return yield* SessionError.mapStorageNotFound(session.get(ctx.params.sessionID)) }) const unshare = Effect.fn("SessionHttpApi.unshare")(function* (ctx: { params: { sessionID: SessionID } }) { yield* shareSvc.unshare(ctx.params.sessionID).pipe(Effect.mapError(() => new HttpApiError.BadRequest({}))) - return yield* session.get(ctx.params.sessionID) + return yield* SessionError.mapStorageNotFound(session.get(ctx.params.sessionID)) }) const summarize = Effect.fn("SessionHttpApi.summarize")(function* (ctx: { params: { sessionID: SessionID } payload: typeof SummarizePayload.Type }) { - yield* revertSvc.cleanup(yield* session.get(ctx.params.sessionID)) + yield* revertSvc.cleanup(yield* SessionError.mapStorageNotFound(session.get(ctx.params.sessionID))) const messages = yield* session.messages({ sessionID: ctx.params.sessionID }) const defaultAgent = yield* agentSvc.defaultAgent() const currentAgent = messages.findLast((message) => message.info.role === "user")?.info.agent ?? defaultAgent diff --git a/packages/opencode/src/server/routes/instance/httpapi/handlers/tui.ts b/packages/opencode/src/server/routes/instance/httpapi/handlers/tui.ts index cc85321685..0ecebf451f 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/tui.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/tui.ts @@ -1,13 +1,12 @@ import { Bus } from "@/bus" import { TuiEvent } from "@/cli/cmd/tui/event" -import { SessionTable } from "@/session/session.sql" -import * as Database from "@/storage/db" -import { eq } from "drizzle-orm" +import { Session } from "@/session/session" import { Effect } from "effect" import { HttpApiBuilder, HttpApiError } from "effect/unstable/httpapi" import { nextTuiRequest, submitTuiResponse } from "@/server/shared/tui-control" import { InstanceHttpApi } from "../api" import { CommandPayload, TuiPublishPayload } from "../groups/tui" +import * as SessionError from "./session-errors" const commandAliases = { session_new: "session.new", @@ -28,6 +27,7 @@ const commandAliases = { export const tuiHandlers = HttpApiBuilder.group(InstanceHttpApi, "tui", (handlers) => Effect.gen(function* () { const bus = yield* Bus.Service + const session = yield* Session.Service const publishCommand = (command: typeof TuiEvent.CommandExecute.properties.Type.command | undefined) => bus.publish(TuiEvent.CommandExecute, { command } as typeof TuiEvent.CommandExecute.properties.Type) @@ -98,12 +98,7 @@ export const tuiHandlers = HttpApiBuilder.group(InstanceHttpApi, "tui", (handler payload: typeof TuiEvent.SessionSelect.properties.Type }) { if (!ctx.payload.sessionID.startsWith("ses")) return yield* new HttpApiError.BadRequest({}) - const row = yield* Effect.sync(() => - Database.use((db) => - db.select({ id: SessionTable.id }).from(SessionTable).where(eq(SessionTable.id, ctx.payload.sessionID)).get(), - ), - ) - if (!row) return yield* new HttpApiError.NotFound({}) + yield* SessionError.mapStorageNotFound(session.get(ctx.payload.sessionID)) yield* bus.publish(TuiEvent.SessionSelect, ctx.payload) return true }) diff --git a/packages/opencode/src/server/routes/instance/httpapi/middleware/workspace-routing.ts b/packages/opencode/src/server/routes/instance/httpapi/middleware/workspace-routing.ts index a91a9992df..8ec9f74860 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/middleware/workspace-routing.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/middleware/workspace-routing.ts @@ -7,6 +7,7 @@ import { Session } from "@/session/session" import { HttpApiProxy } from "./proxy" import * as Fence from "@/server/shared/fence" import { getWorkspaceRouteSessionID, isLocalWorkspaceRoute, workspaceProxyURL } from "@/server/shared/workspace-routing" +import { NotFoundError } from "@/storage/storage" import { Flag } from "@opencode-ai/core/flag/flag" import { Context, Data, Effect, Layer } from "effect" import { HttpClient, HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/unstable/http" @@ -178,7 +179,10 @@ function routeHttpApiWorkspace( const request = yield* HttpServerRequest.HttpServerRequest const sessionID = getWorkspaceRouteSessionID(requestURL(request)) const session = sessionID - ? yield* Session.Service.use((svc) => svc.get(sessionID)).pipe(Effect.catchDefect(() => Effect.void)) + ? yield* Session.Service.use((svc) => svc.get(sessionID)).pipe( + Effect.catchIf(NotFoundError.isInstance, () => Effect.succeed(undefined)), + Effect.catchDefect(() => Effect.succeed(undefined)), + ) : undefined const plan = yield* planRequest(request, session?.workspaceID) return yield* routeWorkspace(client, effect, plan) diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 8286ecf8e6..fef8c43836 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -744,7 +744,7 @@ NOTE: At any point in time through this workflow you should feel free to ask the const markReady = ready ? ready.open.pipe(Effect.asVoid) : Effect.void const { msg, part, cwd } = yield* Effect.gen(function* () { const ctx = yield* InstanceState.context - const session = yield* sessions.get(input.sessionID) + const session = yield* sessions.get(input.sessionID).pipe(Effect.orDie) if (session.revert) { yield* revert.cleanup(session) } @@ -1370,7 +1370,7 @@ NOTE: At any point in time through this workflow you should feel free to ask the const prompt: (input: PromptInput) => Effect.Effect = Effect.fn("SessionPrompt.prompt")( function* (input: PromptInput) { - const session = yield* sessions.get(input.sessionID) + const session = yield* sessions.get(input.sessionID).pipe(Effect.orDie) yield* revert.cleanup(session) const message = yield* createUserMessage(input) yield* sessions.touch(input.sessionID) @@ -1401,9 +1401,9 @@ NOTE: At any point in time through this workflow you should feel free to ask the function* (sessionID: SessionID) { const ctx = yield* InstanceState.context const slog = elog.with({ sessionID }) - let structured: unknown | undefined + let structured: unknown let step = 0 - const session = yield* sessions.get(sessionID) + const session = yield* sessions.get(sessionID).pipe(Effect.orDie) while (true) { yield* status.set(sessionID, { type: "busy" }) diff --git a/packages/opencode/src/session/revert.ts b/packages/opencode/src/session/revert.ts index 58d69a2040..abf7c3441f 100644 --- a/packages/opencode/src/session/revert.ts +++ b/packages/opencode/src/session/revert.ts @@ -44,7 +44,7 @@ export const layer = Layer.effect( yield* state.assertNotBusy(input.sessionID) const all = yield* sessions.messages({ sessionID: input.sessionID }) let lastUser: MessageV2.User | undefined - const session = yield* sessions.get(input.sessionID) + const session = yield* sessions.get(input.sessionID).pipe(Effect.orDie) let rev: Session.Info["revert"] const patches: Snapshot.Patch[] = [] @@ -75,8 +75,8 @@ export const layer = Layer.effect( rev.snapshot = session.revert?.snapshot ?? (yield* snap.track()) if (session.revert?.snapshot) yield* snap.restore(session.revert.snapshot) yield* snap.revert(patches) - if (rev.snapshot) rev.diff = yield* snap.diff(rev.snapshot as string) - const range = all.filter((msg) => msg.info.id >= rev!.messageID) + if (rev.snapshot) rev.diff = yield* snap.diff(rev.snapshot) + const range = all.filter((msg) => msg.info.id >= rev.messageID) const diffs = yield* summary.computeDiff({ messages: range }) yield* storage.write(["session_diff", input.sessionID], diffs).pipe(Effect.ignore) yield* bus.publish(Session.Event.Diff, { sessionID: input.sessionID, diff: diffs }) @@ -89,17 +89,17 @@ export const layer = Layer.effect( files: diffs.length, }, }) - return yield* sessions.get(input.sessionID) + return yield* sessions.get(input.sessionID).pipe(Effect.orDie) }) const unrevert = Effect.fn("SessionRevert.unrevert")(function* (input: { sessionID: SessionID }) { log.info("unreverting", input) yield* state.assertNotBusy(input.sessionID) - const session = yield* sessions.get(input.sessionID) + const session = yield* sessions.get(input.sessionID).pipe(Effect.orDie) if (!session.revert) return session - if (session.revert.snapshot) yield* snap.restore(session.revert!.snapshot!) + if (session.revert.snapshot) yield* snap.restore(session.revert.snapshot) yield* sessions.clearRevert(input.sessionID) - return yield* sessions.get(input.sessionID) + return yield* sessions.get(input.sessionID).pipe(Effect.orDie) }) const cleanup = Effect.fn("SessionRevert.cleanup")(function* (session: Session.Info) { diff --git a/packages/opencode/src/session/session.ts b/packages/opencode/src/session/session.ts index 09d2c8c3c3..5c938ff693 100644 --- a/packages/opencode/src/session/session.ts +++ b/packages/opencode/src/session/session.ts @@ -3,7 +3,6 @@ import path from "path" import { BusEvent } from "@/bus/bus-event" import { Bus } from "@/bus" import { Decimal } from "decimal.js" -import z from "zod" import { type ProviderMetadata, type LanguageModelUsage } from "ai" import { Flag } from "@opencode-ai/core/flag/flag" import { InstallationVersion } from "@opencode-ai/core/installation/version" @@ -422,6 +421,8 @@ export class BusyError extends Error { } } +export type NotFound = InstanceType + export interface Interface { readonly list: (input?: ListInput) => Effect.Effect readonly create: (input?: { @@ -432,9 +433,9 @@ export interface Interface { permission?: Permission.Ruleset workspaceID?: WorkspaceID }) => Effect.Effect - readonly fork: (input: { sessionID: SessionID; messageID?: MessageID }) => Effect.Effect + readonly fork: (input: { sessionID: SessionID; messageID?: MessageID }) => Effect.Effect readonly touch: (sessionID: SessionID) => Effect.Effect - readonly get: (id: SessionID) => Effect.Effect + readonly get: (id: SessionID) => Effect.Effect readonly setTitle: (input: { sessionID: SessionID; title: string }) => Effect.Effect readonly setArchived: (input: { sessionID: SessionID; time?: number }) => Effect.Effect readonly setPermission: (input: { sessionID: SessionID; permission: Permission.Ruleset }) => Effect.Effect @@ -448,7 +449,7 @@ export interface Interface { readonly diff: (sessionID: SessionID) => Effect.Effect readonly messages: (input: { sessionID: SessionID; limit?: number }) => Effect.Effect readonly children: (parentID: SessionID) => Effect.Effect - readonly remove: (sessionID: SessionID) => Effect.Effect + readonly remove: (sessionID: SessionID) => Effect.Effect readonly updateMessage: (msg: T) => Effect.Effect readonly removeMessage: (input: { sessionID: SessionID; messageID: MessageID }) => Effect.Effect readonly removePart: (input: { sessionID: SessionID; messageID: MessageID; partID: PartID }) => Effect.Effect @@ -534,13 +535,13 @@ export const layer: Layer.Layer d.select().from(SessionTable).where(eq(SessionTable.id, id)).get()) - if (!row) throw new NotFoundError({ message: `Session not found: ${id}` }) + if (!row) return yield* Effect.fail(new NotFoundError({ message: `Session not found: ${id}` })) return fromRow(row) }) const list = Effect.fn("Session.list")(function* (input?: ListInput) { const ctx = yield* InstanceState.context - return Array.from(listByProject({ projectID: ctx.project.id, ...(input ?? {}) })) + return Array.from(listByProject({ projectID: ctx.project.id, ...input })) }) const children = Effect.fn("Session.children")(function* (parentID: SessionID) { @@ -555,8 +556,8 @@ export const layer: Layer.Layer { yield* eventuallyEffect( Effect.gen(function* () { - expect((yield* sessionSvc.get(session.id)).title).toBe("from history") + expect((yield* sessionSvc.get(session.id).pipe(Effect.orDie)).title).toBe("from history") }), ) expect(historyBodies).toEqual([{ [session.id]: historyNextSeq - 1 }]) @@ -1208,7 +1208,7 @@ describe("workspace-old sync state", () => { yield* eventuallyEffect( Effect.gen(function* () { - expect((yield* sessionSvc.get(session.id)).title).toBe("from sse") + expect((yield* sessionSvc.get(session.id).pipe(Effect.orDie)).title).toBe("from sse") }), ) expect( diff --git a/packages/opencode/test/server/httpapi-parity.test.ts b/packages/opencode/test/server/httpapi-parity.test.ts index 6922d8c43f..9d7eff4964 100644 --- a/packages/opencode/test/server/httpapi-parity.test.ts +++ b/packages/opencode/test/server/httpapi-parity.test.ts @@ -105,23 +105,22 @@ describe("404 mapping for missing session", () => { }) // ────────────────────────────────────────────────────────────────────────────── -// Reproducer 3: 404 response body shape should match Hono's NamedError -// envelope `{ name, data: { message } }`. HttpApi returns the typed-error -// shape `{ _tag }` instead. SDK consumers reading `error.data.message` -// see undefined. -// -// FIXME: unskip when error JSON shape policy is decided + applied (separate PR). +// Reproducer 3: 404 response body shape should match Hono's public NamedError +// envelope `{ name, data: { message } }`. SDK consumers read +// `error.data.message`, so returning an Effect built-in `{ _tag }` body is a +// compatibility break. // ────────────────────────────────────────────────────────────────────────────── describe("Error JSON shape parity", () => { - test.todo("HttpApi 404 body matches NamedError shape", async () => { + test("HttpApi 404 body matches Hono shape", async () => { await using tmp = await tmpdir({ config: { formatter: false, lsp: false } }) + const headers = { "x-opencode-directory": tmp.path } - const response = await app(true).request("/session/ses_does_not_exist", { - headers: { "x-opencode-directory": tmp.path }, - }) + const hono = await app(false).request("/session/ses_does_not_exist", { headers }) + const httpapi = await app(true).request("/session/ses_does_not_exist", { headers }) - expect(response.status).toBe(404) - const body = (await response.json()) as { name?: string; data?: { message?: string } } + expect(httpapi.status).toBe(hono.status) + const body = (await httpapi.json()) as { name?: string; data?: { message?: string } } + expect(body).toEqual(await hono.json()) expect(body.name).toBe("NotFoundError") expect(typeof body.data?.message).toBe("string") }) diff --git a/packages/opencode/test/server/httpapi-pty.test.ts b/packages/opencode/test/server/httpapi-pty.test.ts index 2b6284a310..5e63eae61c 100644 --- a/packages/opencode/test/server/httpapi-pty.test.ts +++ b/packages/opencode/test/server/httpapi-pty.test.ts @@ -50,9 +50,9 @@ const effectIt = testEffect( ), ) -function app() { - Flag.OPENCODE_EXPERIMENTAL_HTTPAPI = true - return Server.Default().app +function app(experimental = true) { + Flag.OPENCODE_EXPERIMENTAL_HTTPAPI = experimental + return experimental ? Server.Default().app : Server.Legacy().app } function serverUrl() { @@ -121,6 +121,18 @@ describe("pty HttpApi bridge", () => { expect(missing.status).toBe(404) }) + test("matches Hono missing PTY error body", async () => { + await using tmp = await tmpdir({ git: true, config: { formatter: false, lsp: false } }) + const headers = { "x-opencode-directory": tmp.path } + const path = PtyPaths.get.replace(":ptyID", PtyID.ascending()) + + const hono = await app(false).request(path, { headers }) + const httpapi = await app().request(path, { headers }) + + expect(httpapi.status).toBe(hono.status) + expect(await httpapi.json()).toEqual(await hono.json()) + }) + test("returns 404 for missing PTY websocket before upgrade", async () => { await using tmp = await tmpdir({ git: true, config: { formatter: false, lsp: false } }) const response = await app().request(PtyPaths.connect.replace(":ptyID", PtyID.ascending()), { diff --git a/packages/opencode/test/server/httpapi-sdk.test.ts b/packages/opencode/test/server/httpapi-sdk.test.ts index ce774ccfd0..6d2df45078 100644 --- a/packages/opencode/test/server/httpapi-sdk.test.ts +++ b/packages/opencode/test/server/httpapi-sdk.test.ts @@ -4,6 +4,7 @@ import type * as Scope from "effect/Scope" import { HttpRouter } from "effect/unstable/http" import { Flag } from "@opencode-ai/core/flag/flag" import { createOpencodeClient } from "@opencode-ai/sdk/v2" +import { validateSession } from "../../src/cli/cmd/tui/validate-session" import { Instance } from "../../src/project/instance" import { WithInstance } from "../../src/project/with-instance" import { ExperimentalHttpApiServer } from "../../src/server/routes/instance/httpapi/server" @@ -13,6 +14,7 @@ import { MessageV2 } from "../../src/session/message-v2" import { ModelID, ProviderID } from "../../src/provider/schema" import type { Config } from "@/config/config" import { Session as SessionNs } from "@/session/session" +import { errorMessage } from "../../src/util/error" import { TestLLMServer } from "../lib/llm-server" import path from "path" import { resetDatabase } from "../fixture/db" @@ -64,20 +66,23 @@ function client( directory?: string, input?: { password?: string; username?: string; headers?: Record }, ) { - const serverApp = app(backend, input) - const fetch = Object.assign( - async (request: RequestInfo | URL, init?: RequestInit) => - await serverApp.fetch(request instanceof Request ? request : new Request(request, init)), - { preconnect: globalThis.fetch.preconnect }, - ) satisfies typeof globalThis.fetch return createOpencodeClient({ baseUrl: "http://localhost", directory, headers: input?.headers, - fetch, + fetch: serverFetch(backend, input), }) } +function serverFetch(backend: Backend, input?: { password?: string; username?: string }) { + const serverApp = app(backend, input) + return Object.assign( + async (request: RequestInfo | URL, init?: RequestInit) => + await serverApp.fetch(request instanceof Request ? request : new Request(request, init)), + { preconnect: globalThis.fetch.preconnect }, + ) satisfies typeof globalThis.fetch +} + function authorization(username: string, password: string) { return `Basic ${Buffer.from(`${username}:${password}`).toString("base64")}` } @@ -129,6 +134,16 @@ function capture(request: () => Promise) { ) } +function captureThrown(request: () => Promise) { + return call(async () => { + try { + await request() + } catch (error) { + return error + } + }) +} + function expectStatus(request: () => Promise<{ response: Response }>, status: number) { return call(request).pipe( Effect.tap((result) => Effect.sync(() => expect(result.response.status).toBe(status))), @@ -338,6 +353,46 @@ describe("HttpApi SDK", () => { ), ) + parity("matches generated SDK missing session errors across backends", (backend) => + withStandardProject(backend, ({ sdk }) => + Effect.gen(function* () { + const sessionID = "ses_missing" + const expected = { + name: "NotFoundError", + data: { message: `Session not found: ${sessionID}` }, + } + const missing = yield* capture(() => sdk.session.get({ sessionID })) + const thrown = yield* captureThrown(() => sdk.session.get({ sessionID }, { throwOnError: true })) + + expect(missing.error).toEqual(expected) + expect(thrown).toEqual(expected) + return { + status: missing.status, + error: missing.error, + thrown, + } + }), + ), + ) + + parity("formats missing session validation errors for -s", (backend) => + withStandardProject(backend, ({ directory }) => + Effect.gen(function* () { + const sessionID = "ses_206f84f18ffeZ6hhD7pFYAiW5T" + const thrown = yield* captureThrown(() => + validateSession({ + url: "http://localhost", + directory, + sessionID, + fetch: serverFetch(backend), + }), + ) + expect(errorMessage(thrown)).toBe(`Session not found: ${sessionID}`) + return errorMessage(thrown) + }), + ), + ) + parity("matches generated SDK basic auth behavior across backends", (backend) => withStandardProject(backend, ({ directory }) => Effect.gen(function* () { diff --git a/packages/opencode/test/server/httpapi-session.test.ts b/packages/opencode/test/server/httpapi-session.test.ts index 34cecd80d0..c45aacce75 100644 --- a/packages/opencode/test/server/httpapi-session.test.ts +++ b/packages/opencode/test/server/httpapi-session.test.ts @@ -8,13 +8,12 @@ import type { WorkspaceAdapter } from "../../src/control-plane/types" import { Workspace } from "../../src/control-plane/workspace" import { PermissionID } from "../../src/permission/schema" import { ModelID, ProviderID } from "../../src/provider/schema" -import { Instance } from "../../src/project/instance" import { WithInstance } from "../../src/project/with-instance" import { Project } from "../../src/project/project" import { Server } from "../../src/server/server" import { SessionPaths } from "../../src/server/routes/instance/httpapi/groups/session" import { Session } from "@/session/session" -import { MessageID, PartID, type SessionID } from "../../src/session/schema" +import { MessageID, PartID, SessionID, type SessionID as SessionIDType } from "../../src/session/schema" import { MessageV2 } from "../../src/session/message-v2" import { Database } from "@/storage/db" import { SessionMessageTable, SessionTable } from "@/session/session.sql" @@ -55,7 +54,7 @@ function createSession(directory: string, input?: Session.CreateInput) { ) } -function createTextMessage(directory: string, sessionID: SessionID, text: string) { +function createTextMessage(directory: string, sessionID: SessionIDType, text: string) { return Effect.promise( async () => await WithInstance.provide({ @@ -125,6 +124,10 @@ function json(response: Response) { }) } +function responseJson(response: Response) { + return Effect.promise(() => response.json()) +} + function requestJson(path: string, init?: RequestInit) { return request(path, init).pipe(Effect.flatMap(json)) } @@ -147,6 +150,47 @@ afterEach(async () => { }) describe("session HttpApi", () => { + it.live( + "returns declared not found errors for read routes", + withTmp({ git: true, config: { formatter: false, lsp: false } }, (tmp) => + Effect.gen(function* () { + const headers = { "x-opencode-directory": tmp.path } + const missingSession = SessionID.descending() + const missingSessionBody = { + name: "NotFoundError", + data: { message: `Session not found: ${missingSession}` }, + } + + const get = yield* request(pathFor(SessionPaths.get, { sessionID: missingSession }), { headers }) + expect(get.status).toBe(404) + expect(yield* responseJson(get)).toEqual(missingSessionBody) + + const messages = yield* request(pathFor(SessionPaths.messages, { sessionID: missingSession }), { headers }) + expect(messages.status).toBe(404) + expect(yield* responseJson(messages)).toEqual(missingSessionBody) + + const remove = yield* request(pathFor(SessionPaths.remove, { sessionID: missingSession }), { + headers, + method: "DELETE", + }) + expect(remove.status).toBe(404) + expect(yield* responseJson(remove)).toEqual(missingSessionBody) + + const session = yield* createSession(tmp.path, { title: "missing message" }) + const missingMessage = MessageID.ascending() + const message = yield* request( + pathFor(SessionPaths.message, { sessionID: session.id, messageID: missingMessage }), + { headers }, + ) + expect(message.status).toBe(404) + expect(yield* responseJson(message)).toEqual({ + name: "NotFoundError", + data: { message: `Message not found: ${missingMessage}` }, + }) + }), + ), + ) + it.live( "serves read routes through Hono bridge", withTmp({ git: true, config: { formatter: false, lsp: false } }, (tmp) => diff --git a/packages/opencode/test/server/httpapi-tui.test.ts b/packages/opencode/test/server/httpapi-tui.test.ts index 8d2670c492..91cad362a9 100644 --- a/packages/opencode/test/server/httpapi-tui.test.ts +++ b/packages/opencode/test/server/httpapi-tui.test.ts @@ -72,14 +72,27 @@ describe("tui HttpApi bridge", () => { properties: { text: "from publish" }, }) + const missingSessionID = SessionID.descending() const missing = await app().request(TuiPaths.selectSession, { method: "POST", headers: { ...headers, "content-type": "application/json" }, - body: JSON.stringify({ sessionID: SessionID.descending() }), + body: JSON.stringify({ sessionID: missingSessionID }), }) expect(missing.status).toBe(404) }) + test("matches Hono missing selected session error body", async () => { + await using tmp = await tmpdir({ git: true, config: { formatter: false, lsp: false } }) + const headers = { "x-opencode-directory": tmp.path, "content-type": "application/json" } + const body = JSON.stringify({ sessionID: SessionID.descending() }) + + const hono = await app(false).request(TuiPaths.selectSession, { method: "POST", headers, body }) + const httpapi = await app().request(TuiPaths.selectSession, { method: "POST", headers, body }) + + expect(httpapi.status).toBe(hono.status) + expect(await httpapi.json()).toEqual(await hono.json()) + }) + test("matches legacy unknown execute command behavior", async () => { await using tmp = await tmpdir({ git: true, config: { formatter: false, lsp: false } }) const headers = { "x-opencode-directory": tmp.path, "content-type": "application/json" }