From 62ff15264260ae35fdfe67cb40255d321dbc0c50 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sat, 9 May 2026 22:41:11 -0400 Subject: [PATCH] fix(server): return diagnosable body for schema rejections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Effect's default Respondable for HttpApiSchemaError returns 400 with an empty body. The renderer / SDK / curl get nothing actionable — just "GET /url → 400 Bad Request: (empty response body)". When a real user hit this on Windows yesterday (corrupted DB row → schema rejected the response), we spent ~an hour reverse-engineering the cause from the URL alone. PR #26457 previously tried to surface the reason in a structured body ({data, errors, success}) and got reverted in #26546 because some plugins broke. The proximate cause was the SDK throwing raw POJOs to plugins instead of Errors, which has since been fixed by `wrapClientError` (`50dcc4f1a`). Use the same NamedError shape every other 4xx/5xx in this API already uses (e.g. NotFoundError 404): {"name":"BadRequest","data":{"message":"...","kind":"Body"}} The SDK's wrapClientError extracts data.message automatically, so any caller that handles existing 404 NotFoundError bodies handles this identically — no new contract. Verified end-to-end: BEFORE status: 400 body: "" SDK Error.message: opencode server GET .../message?... → 400: (empty response body) AFTER status: 400 body: {"name":"BadRequest","data":{"message":"Expected number, got null at [0][\"parts\"][0][\"tokens\"][\"output\"]","kind":"Body"}} SDK Error.message: Expected number, got null at [0]["parts"][0]["tokens"]["output"] Includes a regression test that asserts the body shape on a real Body schema rejection (POST /sync/history with invalid aggregate). --- .../src/server/routes/instance/httpapi/api.ts | 3 ++ .../httpapi/middleware/schema-error.ts | 42 +++++++++++++++ .../server/routes/instance/httpapi/server.ts | 3 ++ .../server/httpapi-schema-error-body.test.ts | 51 +++++++++++++++++++ 4 files changed, 99 insertions(+) create mode 100644 packages/opencode/src/server/routes/instance/httpapi/middleware/schema-error.ts create mode 100644 packages/opencode/test/server/httpapi-schema-error-body.test.ts diff --git a/packages/opencode/src/server/routes/instance/httpapi/api.ts b/packages/opencode/src/server/routes/instance/httpapi/api.ts index bdd917e4e8..4c6e46a455 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/api.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/api.ts @@ -21,6 +21,7 @@ import { TuiApi } from "./groups/tui" import { WorkspaceApi } from "./groups/workspace" import { V2Api } from "./groups/v2" import { Authorization } from "./middleware/authorization" +import { SchemaErrorMiddleware } from "./middleware/schema-error" // SSE event schemas built from the BusEvent/SyncEvent registries. const EventSchema = Schema.Union(BusEvent.effectPayloads()).annotate({ identifier: "Event" }) @@ -29,6 +30,7 @@ const SyncEventSchemas = SyncEvent.effectPayloads() export const RootHttpApi = HttpApi.make("opencode-root") .addHttpApi(ControlApi) .addHttpApi(GlobalApi) + .middleware(SchemaErrorMiddleware) .middleware(Authorization) export const InstanceHttpApi = HttpApi.make("opencode-instance") @@ -47,6 +49,7 @@ export const InstanceHttpApi = HttpApi.make("opencode-instance") .addHttpApi(V2Api) .addHttpApi(TuiApi) .addHttpApi(WorkspaceApi) + .middleware(SchemaErrorMiddleware) export const OpenCodeHttpApi = HttpApi.make("opencode") .addHttpApi(RootHttpApi) diff --git a/packages/opencode/src/server/routes/instance/httpapi/middleware/schema-error.ts b/packages/opencode/src/server/routes/instance/httpapi/middleware/schema-error.ts new file mode 100644 index 0000000000..4f8fd23252 --- /dev/null +++ b/packages/opencode/src/server/routes/instance/httpapi/middleware/schema-error.ts @@ -0,0 +1,42 @@ +import { Effect } from "effect" +import { HttpServerResponse } from "effect/unstable/http" +import { HttpApiMiddleware } from "effect/unstable/httpapi" +import * as Log from "@opencode-ai/core/util/log" + +const log = Log.create({ service: "server" }) + +// Effect's default Respondable for HttpApiSchemaError returns 400 with an +// empty body. That gives the renderer / SDK / curl no information about +// what was actually rejected (Body field, Query param, etc.). PR #26457 +// previously tried `{data:{}, errors:[], success:false}` and broke a +// plugin (#26546) — root cause was the SDK throwing raw POJOs instead +// of Errors, which has since been fixed by `wrapClientError`. +// +// We use the same shape every other 4xx/5xx in the API already uses — +// NamedError serialization (`{name, data}`). The SDK's `wrapClientError` +// extracts `.data.message` automatically, so plugins that already handle +// 404 NotFoundError bodies handle this with no changes. +export class SchemaErrorMiddleware extends HttpApiMiddleware.Service()( + "@opencode/HttpApiSchemaError", +) {} + +export const schemaErrorLayer = HttpApiMiddleware.layerSchemaErrorTransform( + SchemaErrorMiddleware, + (error) => + Effect.gen(function* () { + log.warn("schema rejection", { + kind: error.kind, + reason: error.cause.message, + }) + return HttpServerResponse.jsonUnsafe( + { + name: "BadRequest", + data: { + message: error.cause.message, + kind: error.kind, + }, + }, + { status: 400 }, + ) + }), +) diff --git a/packages/opencode/src/server/routes/instance/httpapi/server.ts b/packages/opencode/src/server/routes/instance/httpapi/server.ts index 495497ecb4..7ce21dfadb 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/server.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/server.ts @@ -84,6 +84,7 @@ import { compressionLayer } from "./middleware/compression" import { corsVaryFix } from "./middleware/cors-vary" import { errorLayer } from "./middleware/error" import { fenceLayer } from "./middleware/fence" +import { schemaErrorLayer } from "./middleware/schema-error" export const context = Context.makeUnsafe(new Map()) @@ -114,6 +115,7 @@ const authOnlyRouterLayer = authorizationRouterMiddleware.layer.pipe(Layer.provi const httpApiAuthLayer = authorizationLayer.pipe(Layer.provide(ServerAuth.Config.defaultLayer)) const rootApiRoutes = HttpApiBuilder.layer(RootHttpApi).pipe( Layer.provide([controlHandlers, globalHandlers]), + Layer.provide(schemaErrorLayer), Layer.provide(httpApiAuthLayer), ) const instanceRouterLayer = authorizationRouterMiddleware @@ -150,6 +152,7 @@ const instanceRoutes = Layer.mergeAll(rawInstanceRoutes, instanceApiRoutes).pipe httpApiAuthLayer, workspaceRoutingLayer.pipe(Layer.provide(Socket.layerWebSocketConstructorGlobal)), instanceContextLayer, + schemaErrorLayer, ]), ) diff --git a/packages/opencode/test/server/httpapi-schema-error-body.test.ts b/packages/opencode/test/server/httpapi-schema-error-body.test.ts new file mode 100644 index 0000000000..f280e165f4 --- /dev/null +++ b/packages/opencode/test/server/httpapi-schema-error-body.test.ts @@ -0,0 +1,51 @@ +/** + * Regression: a schema rejection used to come back as `400` with an empty + * body, leaving the renderer / SDK / curl with no way to tell which field + * failed. The schemaErrorLayer now returns a NamedError-shaped JSON body + * (same shape as 404 NotFoundError) so callers see the actual reason. + */ +import { afterEach, describe, expect } from "bun:test" +import { Effect } from "effect" +import { Server } from "../../src/server/server" +import { SyncPaths } from "../../src/server/routes/instance/httpapi/groups/sync" +import { resetDatabase } from "../fixture/db" +import { disposeAllInstances, tmpdir } from "../fixture/fixture" +import { it } from "../lib/effect" + +afterEach(async () => { + await disposeAllInstances() + await resetDatabase() +}) + +describe("schema-rejection wire shape", () => { + it.live( + "Body schema rejection returns NamedError-shaped JSON, not empty", + Effect.acquireRelease( + Effect.promise(() => tmpdir({ git: true, config: { formatter: false, lsp: false } })), + (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()), + ).pipe( + Effect.flatMap((tmp) => + Effect.gen(function* () { + // POST /sync/history with `aggregate: -1` is an invalid Body shape + // (aggregate is a NamedString) and triggers the framework's + // HttpApiSchemaError on the Body kind. + const res = yield* Effect.promise(async () => + Server.Default().app.request(SyncPaths.history, { + method: "POST", + headers: { "x-opencode-directory": tmp.path, "content-type": "application/json" }, + body: JSON.stringify({ aggregate: -1 }), + }), + ) + const body = yield* Effect.promise(async () => res.text()) + expect(res.status).toBe(400) + expect(res.headers.get("content-type") ?? "").toContain("application/json") + const parsed = JSON.parse(body) as { name?: string; data?: { message?: string; kind?: string } } + expect(parsed.name).toBe("BadRequest") + expect(typeof parsed.data?.message).toBe("string") + expect(parsed.data?.message?.length ?? 0).toBeGreaterThan(0) + expect(parsed.data?.kind).toMatch(/^(Body|Payload)$/) + }), + ), + ), + ) +})