From 4837bb8904a483e5151887fcaa323f7525a228d9 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sun, 3 May 2026 00:17:59 -0400 Subject: [PATCH] refactor(sketch): tighten review feedback - workspace-routing best-effort probe: catch ONLY SessionNotFound (was Effect.catch(() => undefined) which swallowed all E-channel errors, broader than the prior catchDefect-only behavior). Drop the now-redundant catchDefect since session.get fails typed instead of throwing. - trace.ts: TODO(typed-errors) marker on the legacy Hono shim so it greps as a known retirement target. - httpapi-parity.test.ts: drop stale FIXME comment above the now- passing reproducer. --- .../httpapi/middleware/workspace-routing.ts | 10 ++++++---- .../opencode/src/server/routes/instance/trace.ts | 9 ++++----- .../opencode/test/server/httpapi-parity.test.ts | 14 ++++---------- 3 files changed, 14 insertions(+), 19 deletions(-) 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 b6fb560201..062c272ed5 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 @@ -179,10 +179,12 @@ function routeHttpApiWorkspace( const sessionID = getWorkspaceRouteSessionID(requestURL(request)) const session = sessionID ? yield* Session.Service.use((svc) => svc.get(sessionID)).pipe( - // Sketch: also swallow the typed `SessionNotFound` so this routing - // probe stays best-effort (matches the previous defect-only catch). - Effect.catch(() => Effect.succeed(undefined)), - Effect.catchDefect(() => Effect.succeed(undefined)), + // Best-effort routing probe: swallow only the typed SessionNotFound; + // any other error must propagate. + Effect.catchIf( + (err): err is Session.SessionNotFound => err instanceof Session.SessionNotFound, + () => Effect.succeed(undefined), + ), ) : undefined const plan = yield* planRequest(request, session?.workspaceID) diff --git a/packages/opencode/src/server/routes/instance/trace.ts b/packages/opencode/src/server/routes/instance/trace.ts index e76bdc32c8..afd0dcff24 100644 --- a/packages/opencode/src/server/routes/instance/trace.ts +++ b/packages/opencode/src/server/routes/instance/trace.ts @@ -42,11 +42,10 @@ export function requestAttributes(c: RequestLike): Record { return attributes } -// Bridge typed service errors to the legacy Hono `NamedError` ErrorMiddleware. -// The HttpApi adapter consumes typed errors directly via schema annotations, -// but Hono's ErrorMiddleware switches on `instanceof NamedError`. Until the -// legacy adapter is retired, catch the new typed errors here and rethrow the -// equivalent NamedError so the existing 404 wiring keeps working. +// TODO(typed-errors): bridge typed service errors to the legacy Hono +// `NamedError` ErrorMiddleware. HttpApi consumes typed errors directly via +// schema annotations, but Hono's ErrorMiddleware switches on +// `instanceof NamedError`. Delete this shim once the legacy adapter retires. const adaptTypedErrors = ( self: Effect.Effect, ): Effect.Effect, R> => diff --git a/packages/opencode/test/server/httpapi-parity.test.ts b/packages/opencode/test/server/httpapi-parity.test.ts index 5f77a09970..4e9dd184ce 100644 --- a/packages/opencode/test/server/httpapi-parity.test.ts +++ b/packages/opencode/test/server/httpapi-parity.test.ts @@ -105,18 +105,12 @@ 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 matches Hono's NamedError envelope +// `{ name, data: { message } }`. `Session.get` now fails with a typed +// `SessionNotFound` annotated `httpApiStatus: 404`. The endpoint declares the +// schema; HttpApi auto-routes status + body — no `mapNotFound` wrapper. // ────────────────────────────────────────────────────────────────────────────── describe("Error JSON shape parity", () => { - // Sketch: validates the typed-error pattern end-to-end on `session.get`. The - // service now fails with a typed `SessionNotFound` error annotated - // `httpApiStatus: 404`. The endpoint declares the schema, and HttpApi auto- - // routes the status + body — no `mapNotFound` wrapper required. test("HttpApi 404 body matches NamedError shape (sketch: session.get)", async () => { await using tmp = await tmpdir({ config: { formatter: false, lsp: false } })