From a2bc5353672ef7da67536b025fcd84bf562f4a05 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sun, 3 May 2026 00:07:01 -0400 Subject: [PATCH] sketch: typed SessionNotFound (design validation) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Demonstrates the typed-errors-at-the-service-layer pattern on a single method (`Session.Service.get`): - Define `Session.SessionNotFound` as `Schema.ErrorClass` with `httpApiStatus: 404` and a body shape that matches the existing Hono `NamedError` envelope (`name: "NotFoundError"`, `data: { message }`). - Update `Session.Service.Interface.get` and the implementation to fail in the E channel via `yield* new SessionNotFound(...)`. - HttpApi endpoint declares the typed error directly. The handler drops its `mapNotFound` wrapper for `get` — Effect HttpApi auto-routes the 404 status from the schema annotation and serializes the body fields. - Hono adapter (`runRequest`) bridges the typed error back to the legacy `NotFoundError` defect so the existing `ErrorMiddleware` keeps rendering 404 + body until that adapter is retired. End-to-end validation: the unskipped parity test (`Error JSON shape parity > HttpApi 404 body matches NamedError shape`) now passes against the HttpApi adapter. This is a sketch, not for merge — most cascading consumers are absorbed via `Effect.orDie` (legacy never-E surface preserved). Real rollout migrates each method/handler explicitly. --- .../routes/instance/httpapi/groups/session.ts | 5 +- .../instance/httpapi/handlers/session.ts | 101 +++++++++++------- .../httpapi/middleware/workspace-routing.ts | 7 +- .../src/server/routes/instance/trace.ts | 21 +++- packages/opencode/src/session/prompt.ts | 7 +- packages/opencode/src/session/revert.ts | 8 +- packages/opencode/src/session/session.ts | 28 ++++- .../test/control-plane/workspace.test.ts | 4 +- .../test/server/httpapi-parity.test.ts | 6 +- 9 files changed, 134 insertions(+), 53 deletions(-) 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..71150ae6c2 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts @@ -123,7 +123,10 @@ 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], + // Sketch: typed `Session.SessionNotFound` declared directly. Effect + // HttpApi auto-routes the 404 status (`httpApiStatus` annotation) + // and serializes the schema fields (`name`, `data.message`). + error: [HttpApiError.BadRequest, Session.SessionNotFound], }).annotateMerge( OpenApi.annotations({ identifier: "session.get", 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..e737cf7d7f 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts @@ -38,13 +38,22 @@ import { UpdatePayload, } from "../groups/session" -const mapNotFound = (self: Effect.Effect) => +const mapNotFound = ( + self: Effect.Effect, +): Effect.Effect | HttpApiError.NotFound, R> => self.pipe( Effect.catchIf(NotFoundError.isInstance, () => Effect.fail(new HttpApiError.NotFound({}))), + // Sketch: bridge typed `SessionNotFound` into the legacy wrapper so + // handlers that internally call `session.get` (whose endpoint + // declarations have not been migrated yet) keep returning 404. + Effect.catchIf( + (e: unknown) => e instanceof Session.SessionNotFound, + () => Effect.fail(new HttpApiError.NotFound({})), + ), Effect.catchDefect((error) => NotFoundError.isInstance(error) ? Effect.fail(new HttpApiError.NotFound({})) : Effect.die(error), ), - ) + ) as Effect.Effect | HttpApiError.NotFound, R> export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", (handlers) => Effect.gen(function* () { @@ -78,8 +87,11 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", return Object.fromEntries(yield* statusSvc.list()) }) + // Sketch: typed `SessionNotFound` flows through the E channel — HttpApi + // maps the status (404 via `httpApiStatus`) and renders the body via the + // schema annotations. No `mapNotFound` wrapper required. const get = Effect.fn("SessionHttpApi.get")(function* (ctx: { params: { sessionID: SessionID } }) { - return yield* mapNotFound(session.get(ctx.params.sessionID)) + return yield* session.get(ctx.params.sessionID) }) const children = Effect.fn("SessionHttpApi.children")(function* (ctx: { params: { sessionID: SessionID } }) { @@ -178,20 +190,24 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", params: { sessionID: SessionID } payload: typeof UpdatePayload.Type }) { - const current = yield* session.get(ctx.params.sessionID) - if (ctx.payload.title !== undefined) { - yield* session.setTitle({ sessionID: ctx.params.sessionID, title: ctx.payload.title }) - } - if (ctx.payload.permission !== undefined) { - yield* session.setPermission({ - sessionID: ctx.params.sessionID, - permission: Permission.merge(current.permission ?? [], ctx.payload.permission), - }) - } - 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* mapNotFound( + Effect.gen(function* () { + const current = yield* session.get(ctx.params.sessionID) + if (ctx.payload.title !== undefined) { + yield* session.setTitle({ sessionID: ctx.params.sessionID, title: ctx.payload.title }) + } + if (ctx.payload.permission !== undefined) { + yield* session.setPermission({ + sessionID: ctx.params.sessionID, + permission: Permission.merge(current.permission ?? [], ctx.payload.permission), + }) + } + 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) + }), + ) }) const fork = Effect.fn("SessionHttpApi.fork")(function* (ctx: { @@ -221,35 +237,48 @@ 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* mapNotFound( + Effect.gen(function* () { + yield* shareSvc.share(ctx.params.sessionID).pipe(Effect.mapError(() => new HttpApiError.BadRequest({}))) + return yield* 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* mapNotFound( + Effect.gen(function* () { + yield* shareSvc.unshare(ctx.params.sessionID).pipe(Effect.mapError(() => new HttpApiError.BadRequest({}))) + return yield* 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)) - 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 + return yield* mapNotFound( + Effect.gen(function* () { + yield* revertSvc.cleanup(yield* 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 - yield* compactSvc.create({ - sessionID: ctx.params.sessionID, - agent: currentAgent, - model: { - providerID: ctx.payload.providerID, - modelID: ctx.payload.modelID, - }, - auto: ctx.payload.auto ?? false, - }) - yield* promptSvc.loop({ sessionID: ctx.params.sessionID }) - return true + yield* compactSvc.create({ + sessionID: ctx.params.sessionID, + agent: currentAgent, + model: { + providerID: ctx.payload.providerID, + modelID: ctx.payload.modelID, + }, + auto: ctx.payload.auto ?? false, + }) + yield* promptSvc.loop({ sessionID: ctx.params.sessionID }) + return true + }), + ) }) const prompt = Effect.fn("SessionHttpApi.prompt")(function* (ctx: { 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 4a07aaf11c..b6fb560201 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 @@ -178,7 +178,12 @@ 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( + // 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)), + ) : undefined const plan = yield* planRequest(request, session?.workspaceID) return yield* routeWorkspace(client, effect, plan) diff --git a/packages/opencode/src/server/routes/instance/trace.ts b/packages/opencode/src/server/routes/instance/trace.ts index 4c7119ef3a..e76bdc32c8 100644 --- a/packages/opencode/src/server/routes/instance/trace.ts +++ b/packages/opencode/src/server/routes/instance/trace.ts @@ -1,6 +1,8 @@ import type { Context } from "hono" import { Effect } from "effect" import { AppRuntime } from "@/effect/app-runtime" +import { NotFoundError } from "@/storage/storage" +import { Session } from "@/session/session" type AppEnv = Parameters[0] extends Effect.Effect ? R : never @@ -40,8 +42,25 @@ 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. +const adaptTypedErrors = ( + self: Effect.Effect, +): Effect.Effect, R> => + self.pipe( + Effect.catchIf( + (e: unknown) => e instanceof Session.SessionNotFound, + (e) => Effect.die(new NotFoundError({ message: (e as Session.SessionNotFound).data.message })), + ), + ) as unknown as Effect.Effect, R> + export function runRequest(name: string, c: Context, effect: Effect.Effect) { - return AppRuntime.runPromise(effect.pipe(Effect.withSpan(name, { attributes: requestAttributes(c) }))) + return AppRuntime.runPromise( + adaptTypedErrors(effect).pipe(Effect.withSpan(name, { attributes: requestAttributes(c) })), + ) } export async function jsonRequest( diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 0590fc3827..83c4e70c62 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -745,7 +745,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* Effect.orDie(sessions.get(input.sessionID)) if (session.revert) { yield* revert.cleanup(session) } @@ -1369,7 +1369,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* Effect.orDie(sessions.get(input.sessionID)) yield* revert.cleanup(session) const message = yield* createUserMessage(input) yield* sessions.touch(input.sessionID) @@ -1402,7 +1402,8 @@ NOTE: At any point in time through this workflow you should feel free to ask the const slog = elog.with({ sessionID }) let structured: unknown | undefined let step = 0 - const session = yield* sessions.get(sessionID) + // Sketch: keep public never-E surface; defect on missing session. + const session = yield* Effect.orDie(sessions.get(sessionID)) 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..24bf469203 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* Effect.orDie(sessions.get(input.sessionID)) let rev: Session.Info["revert"] const patches: Snapshot.Patch[] = [] @@ -89,17 +89,17 @@ export const layer = Layer.effect( files: diffs.length, }, }) - return yield* sessions.get(input.sessionID) + return yield* Effect.orDie(sessions.get(input.sessionID)) }) 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* Effect.orDie(sessions.get(input.sessionID)) if (!session.revert) return session if (session.revert.snapshot) yield* snap.restore(session.revert!.snapshot!) yield* sessions.clearRevert(input.sessionID) - return yield* sessions.get(input.sessionID) + return yield* Effect.orDie(sessions.get(input.sessionID)) }) 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..3e7651a0e8 100644 --- a/packages/opencode/src/session/session.ts +++ b/packages/opencode/src/session/session.ts @@ -434,7 +434,7 @@ export interface Interface { }) => 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 @@ -472,6 +472,22 @@ export interface Interface { ) => Effect.Effect> } +/** + * Typed not-found error for `Session.Service.get`. + * + * The `httpApiStatus: 404` annotation lets the Effect HttpApi auto-route the + * status. The schema fields (`name` literal, `data.message`) match Hono's + * `NamedError` JSON envelope so SDK consumers see the same body shape on both + * adapters. + */ +export class SessionNotFound extends Schema.ErrorClass("opencode/Session/NotFound")( + { + name: Schema.tag("NotFoundError"), + data: Schema.Struct({ message: Schema.String }), + }, + { httpApiStatus: 404 }, +) {} + export class Service extends Context.Service()("@opencode/Session") {} export type Patch = Types.DeepMutable["data"]["info"]> @@ -534,7 +550,7 @@ 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* new SessionNotFound({ data: { message: `Session not found: ${id}` } }) return fromRow(row) }) @@ -556,7 +572,9 @@ export const layer: Layer.Layer { yield* eventuallyEffect( Effect.gen(function* () { - expect((yield* sessionSvc.get(session.id)).title).toBe("from history") + expect((yield* Effect.orDie(sessionSvc.get(session.id))).title).toBe("from history") }), ) expect(historyBodies).toEqual([{ [session.id]: historyNextSeq - 1 }]) @@ -1106,7 +1106,7 @@ describe("workspace-old sync state", () => { yield* eventuallyEffect( Effect.gen(function* () { - expect((yield* sessionSvc.get(session.id)).title).toBe("from sse") + expect((yield* Effect.orDie(sessionSvc.get(session.id))).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..5f77a09970 100644 --- a/packages/opencode/test/server/httpapi-parity.test.ts +++ b/packages/opencode/test/server/httpapi-parity.test.ts @@ -113,7 +113,11 @@ describe("404 mapping for missing session", () => { // FIXME: unskip when error JSON shape policy is decided + applied (separate PR). // ────────────────────────────────────────────────────────────────────────────── describe("Error JSON shape parity", () => { - test.todo("HttpApi 404 body matches NamedError shape", async () => { + // 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 } }) const response = await app(true).request("/session/ses_does_not_exist", {