From faca24d487e328ec595bc15896fe35bcfeb9c6c9 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Mon, 27 Apr 2026 20:53:27 -0400 Subject: [PATCH] fix(httpapi): align session boolean query parsing (#24693) --- .../server/routes/instance/experimental.ts | 6 +- .../routes/instance/httpapi/experimental.ts | 16 +- .../server/routes/instance/httpapi/session.ts | 16 +- .../src/server/routes/instance/session.ts | 4 +- .../test/server/httpapi-json-parity.test.ts | 196 +++++++++++------- 5 files changed, 147 insertions(+), 91 deletions(-) diff --git a/packages/opencode/src/server/routes/instance/experimental.ts b/packages/opencode/src/server/routes/instance/experimental.ts index 9c50abd628..ec46298f5a 100644 --- a/packages/opencode/src/server/routes/instance/experimental.ts +++ b/packages/opencode/src/server/routes/instance/experimental.ts @@ -37,6 +37,8 @@ const ConsoleSwitchBody = z.object({ orgID: z.string(), }) +const QueryBoolean = z.enum(["true", "false"]).transform((value) => value === "true") + export const ExperimentalRoutes = lazy(() => new Hono() .get( @@ -346,7 +348,7 @@ export const ExperimentalRoutes = lazy(() => "query", z.object({ directory: z.string().optional().meta({ description: "Filter sessions by project directory" }), - roots: z.coerce.boolean().optional().meta({ description: "Only return root sessions (no parentID)" }), + roots: QueryBoolean.optional().meta({ description: "Only return root sessions (no parentID)" }), start: z.coerce .number() .optional() @@ -357,7 +359,7 @@ export const ExperimentalRoutes = lazy(() => .meta({ description: "Return sessions updated before this timestamp (milliseconds since epoch)" }), search: z.string().optional().meta({ description: "Filter sessions by title (case-insensitive)" }), limit: z.coerce.number().optional().meta({ description: "Maximum number of sessions to return" }), - archived: z.coerce.boolean().optional().meta({ description: "Include archived sessions (default false)" }), + archived: QueryBoolean.optional().meta({ description: "Include archived sessions (default false)" }), }), ), async (c) => { diff --git a/packages/opencode/src/server/routes/instance/httpapi/experimental.ts b/packages/opencode/src/server/routes/instance/httpapi/experimental.ts index caf32bcbba..97bad74bf7 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/experimental.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/experimental.ts @@ -10,7 +10,7 @@ import { Session } from "@/session/session" import { ToolRegistry } from "@/tool/registry" import * as EffectZod from "@/util/effect-zod" import { Worktree } from "@/worktree" -import { Effect, Layer, Option, Schema } from "effect" +import { Effect, Layer, Option, Schema, SchemaGetter } from "effect" import * as HttpServerResponse from "effect/unstable/http/HttpServerResponse" import { HttpApi, HttpApiBuilder, HttpApiEndpoint, HttpApiError, HttpApiGroup, OpenApi } from "effect/unstable/httpapi" import { Authorization } from "./auth" @@ -51,15 +51,21 @@ const ToolListQuery = Schema.Struct({ model: ModelID, }) +const QueryBoolean = Schema.Literals(["true", "false"]).pipe( + Schema.decodeTo(Schema.Boolean, { + decode: SchemaGetter.transform((value) => value === "true"), + encode: SchemaGetter.transform((value) => (value ? "true" : "false")), + }), +) const WorktreeList = Schema.Array(Schema.String).annotate({ identifier: "WorktreeList" }) const SessionListQuery = Schema.Struct({ directory: Schema.optional(Schema.String), - roots: Schema.optional(Schema.Literals(["true", "false"])), + roots: Schema.optional(QueryBoolean), start: Schema.optional(Schema.NumberFromString), cursor: Schema.optional(Schema.NumberFromString), search: Schema.optional(Schema.String), limit: Schema.optional(Schema.NumberFromString), - archived: Schema.optional(Schema.Literals(["true", "false"])), + archived: Schema.optional(QueryBoolean), }) export const ExperimentalPaths = { @@ -307,12 +313,12 @@ export const experimentalHandlers = Layer.unwrap( const sessions = Array.from( Session.listGlobal({ directory: ctx.query.directory, - roots: ctx.query.roots === "true" ? true : undefined, + roots: ctx.query.roots, start: ctx.query.start, cursor: ctx.query.cursor, search: ctx.query.search, limit: limit + 1, - archived: ctx.query.archived === "true" ? true : undefined, + archived: ctx.query.archived, }), ) const list = sessions.length > limit ? sessions.slice(0, limit) : sessions diff --git a/packages/opencode/src/server/routes/instance/httpapi/session.ts b/packages/opencode/src/server/routes/instance/httpapi/session.ts index dccfb3ecbd..8b1ab9c71e 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/session.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/session.ts @@ -21,7 +21,7 @@ import { MessageID, PartID, SessionID } from "@/session/schema" import { Snapshot } from "@/snapshot" import * as Log from "@opencode-ai/core/util/log" import { NamedError } from "@opencode-ai/core/util/error" -import { Effect, Layer, Schema, Struct } from "effect" +import { Effect, Layer, Schema, SchemaGetter, Struct } from "effect" import * as Stream from "effect/Stream" import { HttpServerRequest, HttpServerResponse } from "effect/unstable/http" import { @@ -37,9 +37,15 @@ import { Authorization } from "./auth" const log = Log.create({ service: "server" }) const root = "/session" +const QueryBoolean = Schema.Literals(["true", "false"]).pipe( + Schema.decodeTo(Schema.Boolean, { + decode: SchemaGetter.transform((value) => value === "true"), + encode: SchemaGetter.transform((value) => (value ? "true" : "false")), + }), +) const ListQuery = Schema.Struct({ directory: Schema.optional(Schema.String), - roots: Schema.optional(Schema.Literals(["true", "false"])), + roots: Schema.optional(QueryBoolean), start: Schema.optional(Schema.NumberFromString), search: Schema.optional(Schema.String), limit: Schema.optional(Schema.NumberFromString), @@ -436,7 +442,7 @@ export const sessionHandlers = Layer.unwrap( Array.from( Session.list({ directory: ctx.query.directory, - roots: ctx.query.roots === "true" ? true : undefined, + roots: ctx.query.roots, start: ctx.query.start, search: ctx.query.search, limit: ctx.query.limit, @@ -472,8 +478,8 @@ export const sessionHandlers = Layer.unwrap( params: { sessionID: SessionID } query: typeof MessagesQuery.Type }) { - if (ctx.query.before !== undefined && ctx.query.limit === undefined) return yield* new HttpApiError.BadRequest({}) - if (ctx.query.before !== undefined) { + 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), diff --git a/packages/opencode/src/server/routes/instance/session.ts b/packages/opencode/src/server/routes/instance/session.ts index 5791a0cd76..a5958d37ad 100644 --- a/packages/opencode/src/server/routes/instance/session.ts +++ b/packages/opencode/src/server/routes/instance/session.ts @@ -30,6 +30,8 @@ import { jsonRequest, runRequest } from "./trace" const log = Log.create({ service: "server" }) +const QueryBoolean = z.enum(["true", "false"]).transform((value) => value === "true") + export const SessionRoutes = lazy(() => new Hono() .get( @@ -53,7 +55,7 @@ export const SessionRoutes = lazy(() => "query", z.object({ directory: z.string().optional().meta({ description: "Filter sessions by project directory" }), - roots: z.coerce.boolean().optional().meta({ description: "Only return root sessions (no parentID)" }), + roots: QueryBoolean.optional().meta({ description: "Only return root sessions (no parentID)" }), start: z.coerce .number() .optional() diff --git a/packages/opencode/test/server/httpapi-json-parity.test.ts b/packages/opencode/test/server/httpapi-json-parity.test.ts index 728a8ffb27..d7d252a5e1 100644 --- a/packages/opencode/test/server/httpapi-json-parity.test.ts +++ b/packages/opencode/test/server/httpapi-json-parity.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, test } from "bun:test" +import { afterEach, describe, expect } from "bun:test" import type { UpgradeWebSocket } from "hono/ws" import { Effect } from "effect" import { Flag } from "@opencode-ai/core/flag/flag" @@ -11,7 +11,8 @@ import { MessageID, PartID } from "../../src/session/schema" import { Session } from "@/session/session" import * as Log from "@opencode-ai/core/util/log" import { resetDatabase } from "../fixture/db" -import { tmpdir } from "../fixture/fixture" +import { provideInstance, tmpdir } from "../fixture/fixture" +import { it } from "../lib/effect" void Log.init({ print: false }) @@ -23,70 +24,63 @@ function app(experimental: boolean) { return InstanceRoutes(websocket) } -function runSession(fx: Effect.Effect) { - return Effect.runPromise(fx.pipe(Effect.provide(Session.defaultLayer))) -} - function pathFor(path: string, params: Record) { return Object.entries(params).reduce((result, [key, value]) => result.replace(`:${key}`, value), path) } -async function seedSessions(directory: string) { - return await Instance.provide({ - directory, - fn: () => - runSession( - Effect.gen(function* () { - const svc = yield* Session.Service - const parent = yield* svc.create({ title: "parent" }) - yield* svc.create({ title: "child", parentID: parent.id }) - const message = yield* svc.updateMessage({ - id: MessageID.ascending(), - role: "user", - sessionID: parent.id, - agent: "build", - model: { providerID: ProviderID.make("test"), modelID: ModelID.make("test") }, - time: { created: Date.now() }, - }) - yield* svc.updatePart({ - id: PartID.ascending(), - sessionID: parent.id, - messageID: message.id, - type: "text", - text: "hello", - }) - return { parent, message } - }), - ), +const seedSessions = Effect.gen(function* () { + const svc = yield* Session.Service + const parent = yield* svc.create({ title: "parent" }) + yield* svc.create({ title: "child", parentID: parent.id }) + const message = yield* svc.updateMessage({ + id: MessageID.ascending(), + role: "user", + sessionID: parent.id, + agent: "build", + model: { providerID: ProviderID.make("test"), modelID: ModelID.make("test") }, + time: { created: Date.now() }, }) -} + yield* svc.updatePart({ + id: PartID.ascending(), + sessionID: parent.id, + messageID: message.id, + type: "text", + text: "hello", + }) + return { parent, message } +}) -async function readJson( - label: string, - app: ReturnType, - directory: string, - path: string, - headers: HeadersInit, +function withTmp( + options: Parameters[0], + fn: (tmp: Awaited>) => Effect.Effect, ) { - const response = await Instance.provide({ - directory, - fn: () => app.request(path, { headers }), - }) - if (response.status !== 200) throw new Error(`${label} returned ${response.status}: ${await response.text()}`) - return await response.json() + return Effect.acquireRelease( + Effect.promise(() => tmpdir(options)), + (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()), + ).pipe(Effect.flatMap((tmp) => fn(tmp).pipe(provideInstance(tmp.path)))) } -async function expectJsonParity(input: { +function readJson(label: string, app: ReturnType, path: string, headers: HeadersInit) { + return Effect.promise(async () => { + const response = await app.request(path, { headers }) + if (response.status !== 200) throw new Error(`${label} returned ${response.status}: ${await response.text()}`) + return await response.json() + }) +} + +function expectJsonParity(input: { label: string legacy: ReturnType httpapi: ReturnType - directory: string path: string headers: HeadersInit }) { - const legacy = await readJson(input.label, input.legacy, input.directory, input.path, input.headers) - const httpapi = await readJson(input.label, input.httpapi, input.directory, input.path, input.headers) - expect({ label: input.label, body: httpapi }).toEqual({ label: input.label, body: legacy }) + return Effect.gen(function* () { + const legacy = yield* readJson(input.label, input.legacy, input.path, input.headers) + const httpapi = yield* readJson(input.label, input.httpapi, input.path, input.headers) + expect({ label: input.label, body: httpapi }).toEqual({ label: input.label, body: legacy }) + return httpapi + }) } afterEach(async () => { @@ -96,32 +90,78 @@ afterEach(async () => { }) describe("HttpApi JSON parity", () => { - test("matches legacy JSON shape for session read endpoints", async () => { - await using tmp = await tmpdir({ git: true, config: { formatter: false, lsp: false } }) - const headers = { "x-opencode-directory": tmp.path } - const seeded = await seedSessions(tmp.path) - const legacy = app(false) - const httpapi = app(true) + it.live( + "matches legacy JSON shape for session read endpoints", + withTmp({ git: true, config: { formatter: false, lsp: false } }, (tmp) => + Effect.gen(function* () { + const headers = { "x-opencode-directory": tmp.path } + const seeded = yield* seedSessions.pipe(Effect.provide(Session.defaultLayer)) + const legacy = app(false) + const httpapi = app(true) - await [ - { label: "session.list roots", path: `${SessionPaths.list}?roots=true`, headers }, - { label: "session.list all", path: SessionPaths.list, headers }, - { label: "session.get", path: pathFor(SessionPaths.get, { sessionID: seeded.parent.id }), headers }, - { label: "session.children", path: pathFor(SessionPaths.children, { sessionID: seeded.parent.id }), headers }, - { label: "session.messages", path: pathFor(SessionPaths.messages, { sessionID: seeded.parent.id }), headers }, - { - label: "session.message", - path: pathFor(SessionPaths.message, { sessionID: seeded.parent.id, messageID: seeded.message.id }), - headers, - }, - { - label: "experimental.session", - path: `${ExperimentalPaths.session}?${new URLSearchParams({ directory: tmp.path, limit: "10" })}`, - headers, - }, - ].reduce( - (promise, input) => promise.then(() => expectJsonParity({ ...input, legacy, httpapi, directory: tmp.path })), - Promise.resolve(), - ) - }) + const rootsFalse = yield* expectJsonParity({ + label: "session.list roots false", + legacy, + httpapi, + path: `${SessionPaths.list}?roots=false`, + headers, + }) + expect((rootsFalse as Session.Info[]).map((session) => session.id)).toContain(seeded.parent.id) + expect((rootsFalse as Session.Info[]).length).toBe(2) + + const experimentalRootsFalse = yield* expectJsonParity({ + label: "experimental.session roots false", + legacy, + httpapi, + path: `${ExperimentalPaths.session}?${new URLSearchParams({ directory: tmp.path, limit: "10", roots: "false" })}`, + headers, + }) + expect((experimentalRootsFalse as Session.GlobalInfo[]).length).toBe(2) + + const experimentalArchivedFalse = yield* expectJsonParity({ + label: "experimental.session archived false", + legacy, + httpapi, + path: `${ExperimentalPaths.session}?${new URLSearchParams({ directory: tmp.path, limit: "10", archived: "false" })}`, + headers, + }) + expect((experimentalArchivedFalse as Session.GlobalInfo[]).length).toBe(2) + + yield* Effect.forEach( + [ + { label: "session.list roots", path: `${SessionPaths.list}?roots=true`, headers }, + { label: "session.list all", path: SessionPaths.list, headers }, + { label: "session.get", path: pathFor(SessionPaths.get, { sessionID: seeded.parent.id }), headers }, + { + label: "session.children", + path: pathFor(SessionPaths.children, { sessionID: seeded.parent.id }), + headers, + }, + { + label: "session.messages", + path: pathFor(SessionPaths.messages, { sessionID: seeded.parent.id }), + headers, + }, + { + label: "session.messages empty before", + path: `${pathFor(SessionPaths.messages, { sessionID: seeded.parent.id })}?before=`, + headers, + }, + { + label: "session.message", + path: pathFor(SessionPaths.message, { sessionID: seeded.parent.id, messageID: seeded.message.id }), + headers, + }, + { + label: "experimental.session", + path: `${ExperimentalPaths.session}?${new URLSearchParams({ directory: tmp.path, limit: "10" })}`, + headers, + }, + ], + (input) => expectJsonParity({ ...input, legacy, httpapi }), + { concurrency: 1 }, + ) + }), + ), + ) })