Skip to content

Commit faca24d

Browse files
authored
fix(httpapi): align session boolean query parsing (#24693)
1 parent c103202 commit faca24d

5 files changed

Lines changed: 146 additions & 90 deletions

File tree

packages/opencode/src/server/routes/instance/experimental.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ const ConsoleSwitchBody = z.object({
3737
orgID: z.string(),
3838
})
3939

40+
const QueryBoolean = z.enum(["true", "false"]).transform((value) => value === "true")
41+
4042
export const ExperimentalRoutes = lazy(() =>
4143
new Hono()
4244
.get(
@@ -346,7 +348,7 @@ export const ExperimentalRoutes = lazy(() =>
346348
"query",
347349
z.object({
348350
directory: z.string().optional().meta({ description: "Filter sessions by project directory" }),
349-
roots: z.coerce.boolean().optional().meta({ description: "Only return root sessions (no parentID)" }),
351+
roots: QueryBoolean.optional().meta({ description: "Only return root sessions (no parentID)" }),
350352
start: z.coerce
351353
.number()
352354
.optional()
@@ -357,7 +359,7 @@ export const ExperimentalRoutes = lazy(() =>
357359
.meta({ description: "Return sessions updated before this timestamp (milliseconds since epoch)" }),
358360
search: z.string().optional().meta({ description: "Filter sessions by title (case-insensitive)" }),
359361
limit: z.coerce.number().optional().meta({ description: "Maximum number of sessions to return" }),
360-
archived: z.coerce.boolean().optional().meta({ description: "Include archived sessions (default false)" }),
362+
archived: QueryBoolean.optional().meta({ description: "Include archived sessions (default false)" }),
361363
}),
362364
),
363365
async (c) => {

packages/opencode/src/server/routes/instance/httpapi/experimental.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { Session } from "@/session/session"
1010
import { ToolRegistry } from "@/tool/registry"
1111
import * as EffectZod from "@/util/effect-zod"
1212
import { Worktree } from "@/worktree"
13-
import { Effect, Layer, Option, Schema } from "effect"
13+
import { Effect, Layer, Option, Schema, SchemaGetter } from "effect"
1414
import * as HttpServerResponse from "effect/unstable/http/HttpServerResponse"
1515
import { HttpApi, HttpApiBuilder, HttpApiEndpoint, HttpApiError, HttpApiGroup, OpenApi } from "effect/unstable/httpapi"
1616
import { Authorization } from "./auth"
@@ -51,15 +51,21 @@ const ToolListQuery = Schema.Struct({
5151
model: ModelID,
5252
})
5353

54+
const QueryBoolean = Schema.Literals(["true", "false"]).pipe(
55+
Schema.decodeTo(Schema.Boolean, {
56+
decode: SchemaGetter.transform((value) => value === "true"),
57+
encode: SchemaGetter.transform((value) => (value ? "true" : "false")),
58+
}),
59+
)
5460
const WorktreeList = Schema.Array(Schema.String).annotate({ identifier: "WorktreeList" })
5561
const SessionListQuery = Schema.Struct({
5662
directory: Schema.optional(Schema.String),
57-
roots: Schema.optional(Schema.Literals(["true", "false"])),
63+
roots: Schema.optional(QueryBoolean),
5864
start: Schema.optional(Schema.NumberFromString),
5965
cursor: Schema.optional(Schema.NumberFromString),
6066
search: Schema.optional(Schema.String),
6167
limit: Schema.optional(Schema.NumberFromString),
62-
archived: Schema.optional(Schema.Literals(["true", "false"])),
68+
archived: Schema.optional(QueryBoolean),
6369
})
6470

6571
export const ExperimentalPaths = {
@@ -307,12 +313,12 @@ export const experimentalHandlers = Layer.unwrap(
307313
const sessions = Array.from(
308314
Session.listGlobal({
309315
directory: ctx.query.directory,
310-
roots: ctx.query.roots === "true" ? true : undefined,
316+
roots: ctx.query.roots,
311317
start: ctx.query.start,
312318
cursor: ctx.query.cursor,
313319
search: ctx.query.search,
314320
limit: limit + 1,
315-
archived: ctx.query.archived === "true" ? true : undefined,
321+
archived: ctx.query.archived,
316322
}),
317323
)
318324
const list = sessions.length > limit ? sessions.slice(0, limit) : sessions

packages/opencode/src/server/routes/instance/httpapi/session.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { MessageID, PartID, SessionID } from "@/session/schema"
2121
import { Snapshot } from "@/snapshot"
2222
import * as Log from "@opencode-ai/core/util/log"
2323
import { NamedError } from "@opencode-ai/core/util/error"
24-
import { Effect, Layer, Schema, Struct } from "effect"
24+
import { Effect, Layer, Schema, SchemaGetter, Struct } from "effect"
2525
import * as Stream from "effect/Stream"
2626
import { HttpServerRequest, HttpServerResponse } from "effect/unstable/http"
2727
import {
@@ -37,9 +37,15 @@ import { Authorization } from "./auth"
3737

3838
const log = Log.create({ service: "server" })
3939
const root = "/session"
40+
const QueryBoolean = Schema.Literals(["true", "false"]).pipe(
41+
Schema.decodeTo(Schema.Boolean, {
42+
decode: SchemaGetter.transform((value) => value === "true"),
43+
encode: SchemaGetter.transform((value) => (value ? "true" : "false")),
44+
}),
45+
)
4046
const ListQuery = Schema.Struct({
4147
directory: Schema.optional(Schema.String),
42-
roots: Schema.optional(Schema.Literals(["true", "false"])),
48+
roots: Schema.optional(QueryBoolean),
4349
start: Schema.optional(Schema.NumberFromString),
4450
search: Schema.optional(Schema.String),
4551
limit: Schema.optional(Schema.NumberFromString),
@@ -436,7 +442,7 @@ export const sessionHandlers = Layer.unwrap(
436442
Array.from(
437443
Session.list({
438444
directory: ctx.query.directory,
439-
roots: ctx.query.roots === "true" ? true : undefined,
445+
roots: ctx.query.roots,
440446
start: ctx.query.start,
441447
search: ctx.query.search,
442448
limit: ctx.query.limit,
@@ -472,8 +478,8 @@ export const sessionHandlers = Layer.unwrap(
472478
params: { sessionID: SessionID }
473479
query: typeof MessagesQuery.Type
474480
}) {
475-
if (ctx.query.before !== undefined && ctx.query.limit === undefined) return yield* new HttpApiError.BadRequest({})
476-
if (ctx.query.before !== undefined) {
481+
if (ctx.query.before && ctx.query.limit === undefined) return yield* new HttpApiError.BadRequest({})
482+
if (ctx.query.before) {
477483
const before = ctx.query.before
478484
yield* Effect.try({
479485
try: () => MessageV2.cursor.decode(before),

packages/opencode/src/server/routes/instance/session.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ import { jsonRequest, runRequest } from "./trace"
3030

3131
const log = Log.create({ service: "server" })
3232

33+
const QueryBoolean = z.enum(["true", "false"]).transform((value) => value === "true")
34+
3335
export const SessionRoutes = lazy(() =>
3436
new Hono()
3537
.get(
@@ -53,7 +55,7 @@ export const SessionRoutes = lazy(() =>
5355
"query",
5456
z.object({
5557
directory: z.string().optional().meta({ description: "Filter sessions by project directory" }),
56-
roots: z.coerce.boolean().optional().meta({ description: "Only return root sessions (no parentID)" }),
58+
roots: QueryBoolean.optional().meta({ description: "Only return root sessions (no parentID)" }),
5759
start: z.coerce
5860
.number()
5961
.optional()
Lines changed: 117 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { afterEach, describe, expect, test } from "bun:test"
1+
import { afterEach, describe, expect } from "bun:test"
22
import type { UpgradeWebSocket } from "hono/ws"
33
import { Effect } from "effect"
44
import { Flag } from "@opencode-ai/core/flag/flag"
@@ -11,7 +11,8 @@ import { MessageID, PartID } from "../../src/session/schema"
1111
import { Session } from "@/session/session"
1212
import * as Log from "@opencode-ai/core/util/log"
1313
import { resetDatabase } from "../fixture/db"
14-
import { tmpdir } from "../fixture/fixture"
14+
import { provideInstance, tmpdir } from "../fixture/fixture"
15+
import { it } from "../lib/effect"
1516

1617
void Log.init({ print: false })
1718

@@ -23,70 +24,63 @@ function app(experimental: boolean) {
2324
return InstanceRoutes(websocket)
2425
}
2526

26-
function runSession<A, E>(fx: Effect.Effect<A, E, Session.Service>) {
27-
return Effect.runPromise(fx.pipe(Effect.provide(Session.defaultLayer)))
28-
}
29-
3027
function pathFor(path: string, params: Record<string, string>) {
3128
return Object.entries(params).reduce((result, [key, value]) => result.replace(`:${key}`, value), path)
3229
}
3330

34-
async function seedSessions(directory: string) {
35-
return await Instance.provide({
36-
directory,
37-
fn: () =>
38-
runSession(
39-
Effect.gen(function* () {
40-
const svc = yield* Session.Service
41-
const parent = yield* svc.create({ title: "parent" })
42-
yield* svc.create({ title: "child", parentID: parent.id })
43-
const message = yield* svc.updateMessage({
44-
id: MessageID.ascending(),
45-
role: "user",
46-
sessionID: parent.id,
47-
agent: "build",
48-
model: { providerID: ProviderID.make("test"), modelID: ModelID.make("test") },
49-
time: { created: Date.now() },
50-
})
51-
yield* svc.updatePart({
52-
id: PartID.ascending(),
53-
sessionID: parent.id,
54-
messageID: message.id,
55-
type: "text",
56-
text: "hello",
57-
})
58-
return { parent, message }
59-
}),
60-
),
31+
const seedSessions = Effect.gen(function* () {
32+
const svc = yield* Session.Service
33+
const parent = yield* svc.create({ title: "parent" })
34+
yield* svc.create({ title: "child", parentID: parent.id })
35+
const message = yield* svc.updateMessage({
36+
id: MessageID.ascending(),
37+
role: "user",
38+
sessionID: parent.id,
39+
agent: "build",
40+
model: { providerID: ProviderID.make("test"), modelID: ModelID.make("test") },
41+
time: { created: Date.now() },
6142
})
62-
}
43+
yield* svc.updatePart({
44+
id: PartID.ascending(),
45+
sessionID: parent.id,
46+
messageID: message.id,
47+
type: "text",
48+
text: "hello",
49+
})
50+
return { parent, message }
51+
})
6352

64-
async function readJson(
65-
label: string,
66-
app: ReturnType<typeof InstanceRoutes>,
67-
directory: string,
68-
path: string,
69-
headers: HeadersInit,
53+
function withTmp<A, E, R>(
54+
options: Parameters<typeof tmpdir>[0],
55+
fn: (tmp: Awaited<ReturnType<typeof tmpdir>>) => Effect.Effect<A, E, R>,
7056
) {
71-
const response = await Instance.provide({
72-
directory,
73-
fn: () => app.request(path, { headers }),
57+
return Effect.acquireRelease(
58+
Effect.promise(() => tmpdir(options)),
59+
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
60+
).pipe(Effect.flatMap((tmp) => fn(tmp).pipe(provideInstance(tmp.path))))
61+
}
62+
63+
function readJson(label: string, app: ReturnType<typeof InstanceRoutes>, path: string, headers: HeadersInit) {
64+
return Effect.promise(async () => {
65+
const response = await app.request(path, { headers })
66+
if (response.status !== 200) throw new Error(`${label} returned ${response.status}: ${await response.text()}`)
67+
return await response.json()
7468
})
75-
if (response.status !== 200) throw new Error(`${label} returned ${response.status}: ${await response.text()}`)
76-
return await response.json()
7769
}
7870

79-
async function expectJsonParity(input: {
71+
function expectJsonParity(input: {
8072
label: string
8173
legacy: ReturnType<typeof InstanceRoutes>
8274
httpapi: ReturnType<typeof InstanceRoutes>
83-
directory: string
8475
path: string
8576
headers: HeadersInit
8677
}) {
87-
const legacy = await readJson(input.label, input.legacy, input.directory, input.path, input.headers)
88-
const httpapi = await readJson(input.label, input.httpapi, input.directory, input.path, input.headers)
89-
expect({ label: input.label, body: httpapi }).toEqual({ label: input.label, body: legacy })
78+
return Effect.gen(function* () {
79+
const legacy = yield* readJson(input.label, input.legacy, input.path, input.headers)
80+
const httpapi = yield* readJson(input.label, input.httpapi, input.path, input.headers)
81+
expect({ label: input.label, body: httpapi }).toEqual({ label: input.label, body: legacy })
82+
return httpapi
83+
})
9084
}
9185

9286
afterEach(async () => {
@@ -96,32 +90,78 @@ afterEach(async () => {
9690
})
9791

9892
describe("HttpApi JSON parity", () => {
99-
test("matches legacy JSON shape for session read endpoints", async () => {
100-
await using tmp = await tmpdir({ git: true, config: { formatter: false, lsp: false } })
101-
const headers = { "x-opencode-directory": tmp.path }
102-
const seeded = await seedSessions(tmp.path)
103-
const legacy = app(false)
104-
const httpapi = app(true)
93+
it.live(
94+
"matches legacy JSON shape for session read endpoints",
95+
withTmp({ git: true, config: { formatter: false, lsp: false } }, (tmp) =>
96+
Effect.gen(function* () {
97+
const headers = { "x-opencode-directory": tmp.path }
98+
const seeded = yield* seedSessions.pipe(Effect.provide(Session.defaultLayer))
99+
const legacy = app(false)
100+
const httpapi = app(true)
105101

106-
await [
107-
{ label: "session.list roots", path: `${SessionPaths.list}?roots=true`, headers },
108-
{ label: "session.list all", path: SessionPaths.list, headers },
109-
{ label: "session.get", path: pathFor(SessionPaths.get, { sessionID: seeded.parent.id }), headers },
110-
{ label: "session.children", path: pathFor(SessionPaths.children, { sessionID: seeded.parent.id }), headers },
111-
{ label: "session.messages", path: pathFor(SessionPaths.messages, { sessionID: seeded.parent.id }), headers },
112-
{
113-
label: "session.message",
114-
path: pathFor(SessionPaths.message, { sessionID: seeded.parent.id, messageID: seeded.message.id }),
115-
headers,
116-
},
117-
{
118-
label: "experimental.session",
119-
path: `${ExperimentalPaths.session}?${new URLSearchParams({ directory: tmp.path, limit: "10" })}`,
120-
headers,
121-
},
122-
].reduce(
123-
(promise, input) => promise.then(() => expectJsonParity({ ...input, legacy, httpapi, directory: tmp.path })),
124-
Promise.resolve(),
125-
)
126-
})
102+
const rootsFalse = yield* expectJsonParity({
103+
label: "session.list roots false",
104+
legacy,
105+
httpapi,
106+
path: `${SessionPaths.list}?roots=false`,
107+
headers,
108+
})
109+
expect((rootsFalse as Session.Info[]).map((session) => session.id)).toContain(seeded.parent.id)
110+
expect((rootsFalse as Session.Info[]).length).toBe(2)
111+
112+
const experimentalRootsFalse = yield* expectJsonParity({
113+
label: "experimental.session roots false",
114+
legacy,
115+
httpapi,
116+
path: `${ExperimentalPaths.session}?${new URLSearchParams({ directory: tmp.path, limit: "10", roots: "false" })}`,
117+
headers,
118+
})
119+
expect((experimentalRootsFalse as Session.GlobalInfo[]).length).toBe(2)
120+
121+
const experimentalArchivedFalse = yield* expectJsonParity({
122+
label: "experimental.session archived false",
123+
legacy,
124+
httpapi,
125+
path: `${ExperimentalPaths.session}?${new URLSearchParams({ directory: tmp.path, limit: "10", archived: "false" })}`,
126+
headers,
127+
})
128+
expect((experimentalArchivedFalse as Session.GlobalInfo[]).length).toBe(2)
129+
130+
yield* Effect.forEach(
131+
[
132+
{ label: "session.list roots", path: `${SessionPaths.list}?roots=true`, headers },
133+
{ label: "session.list all", path: SessionPaths.list, headers },
134+
{ label: "session.get", path: pathFor(SessionPaths.get, { sessionID: seeded.parent.id }), headers },
135+
{
136+
label: "session.children",
137+
path: pathFor(SessionPaths.children, { sessionID: seeded.parent.id }),
138+
headers,
139+
},
140+
{
141+
label: "session.messages",
142+
path: pathFor(SessionPaths.messages, { sessionID: seeded.parent.id }),
143+
headers,
144+
},
145+
{
146+
label: "session.messages empty before",
147+
path: `${pathFor(SessionPaths.messages, { sessionID: seeded.parent.id })}?before=`,
148+
headers,
149+
},
150+
{
151+
label: "session.message",
152+
path: pathFor(SessionPaths.message, { sessionID: seeded.parent.id, messageID: seeded.message.id }),
153+
headers,
154+
},
155+
{
156+
label: "experimental.session",
157+
path: `${ExperimentalPaths.session}?${new URLSearchParams({ directory: tmp.path, limit: "10" })}`,
158+
headers,
159+
},
160+
],
161+
(input) => expectJsonParity({ ...input, legacy, httpapi }),
162+
{ concurrency: 1 },
163+
)
164+
}),
165+
),
166+
)
127167
})

0 commit comments

Comments
 (0)