fix(server): return diagnosable body for schema rejections (#26631)
This commit is contained in:
@@ -0,0 +1,162 @@
|
||||
import { afterEach, describe, expect } from "bun:test"
|
||||
import { Effect } from "effect"
|
||||
import { eq } from "drizzle-orm"
|
||||
import * as Database from "@/storage/db"
|
||||
import { ModelID, ProviderID } from "../../src/provider/schema"
|
||||
import { WithInstance } from "../../src/project/with-instance"
|
||||
import { Server } from "../../src/server/server"
|
||||
import { Session } from "@/session/session"
|
||||
import { SessionPaths } from "../../src/server/routes/instance/httpapi/groups/session"
|
||||
import { SyncPaths } from "../../src/server/routes/instance/httpapi/groups/sync"
|
||||
import { MessageID, PartID } from "../../src/session/schema"
|
||||
import { PartTable } from "@/session/session.sql"
|
||||
import { resetDatabase } from "../fixture/db"
|
||||
import { disposeAllInstances, tmpdir } from "../fixture/fixture"
|
||||
import { it } from "../lib/effect"
|
||||
|
||||
afterEach(async () => {
|
||||
await disposeAllInstances()
|
||||
await resetDatabase()
|
||||
})
|
||||
|
||||
const withTmp = <A, E, R>(
|
||||
options: Parameters<typeof tmpdir>[0],
|
||||
fn: (tmp: Awaited<ReturnType<typeof tmpdir>>) => Effect.Effect<A, E, R>,
|
||||
) =>
|
||||
Effect.acquireRelease(
|
||||
Effect.promise(() => tmpdir(options)),
|
||||
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
|
||||
).pipe(Effect.flatMap(fn))
|
||||
|
||||
async function seedCorruptStepFinishPart(directory: string) {
|
||||
return WithInstance.provide({
|
||||
directory,
|
||||
fn: () =>
|
||||
Effect.runPromise(
|
||||
Effect.gen(function* () {
|
||||
const session = yield* Session.Service
|
||||
const info = yield* session.create({})
|
||||
const message = yield* session.updateMessage({
|
||||
id: MessageID.ascending(),
|
||||
role: "user",
|
||||
sessionID: info.id,
|
||||
agent: "build",
|
||||
model: { providerID: ProviderID.make("test"), modelID: ModelID.make("test") },
|
||||
time: { created: Date.now() },
|
||||
})
|
||||
const partID = PartID.ascending()
|
||||
yield* session.updatePart({
|
||||
id: partID,
|
||||
sessionID: info.id,
|
||||
messageID: message.id,
|
||||
type: "step-finish",
|
||||
reason: "stop",
|
||||
cost: 0,
|
||||
tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } },
|
||||
})
|
||||
// Schema.Finite still rejects NaN at encode — exact mirror of the
|
||||
// corrupt row that broke the user's session in the OMO/Windows bug.
|
||||
Database.use((db) =>
|
||||
db
|
||||
.update(PartTable)
|
||||
.set({
|
||||
data: {
|
||||
type: "step-finish",
|
||||
reason: "stop",
|
||||
cost: 0,
|
||||
tokens: { input: 0, output: NaN, reasoning: 0, cache: { read: 0, write: 0 } },
|
||||
} as never, // drizzle's .set() can't narrow the discriminated union
|
||||
})
|
||||
.where(eq(PartTable.id, partID))
|
||||
.run(),
|
||||
)
|
||||
return info.id
|
||||
}).pipe(Effect.provide(Session.defaultLayer)),
|
||||
),
|
||||
})
|
||||
}
|
||||
|
||||
describe("schema-rejection wire shape", () => {
|
||||
it.live(
|
||||
"Payload schema rejection returns NamedError-shaped JSON, not empty",
|
||||
withTmp({ git: true, config: { formatter: false, lsp: false } }, (tmp) =>
|
||||
Effect.gen(function* () {
|
||||
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)
|
||||
expect(parsed).toMatchObject({
|
||||
name: "BadRequest",
|
||||
data: { kind: expect.stringMatching(/^(Body|Payload)$/) },
|
||||
})
|
||||
expect(parsed.data.message).toEqual(expect.any(String))
|
||||
expect(parsed.data.message.length).toBeGreaterThan(0)
|
||||
}),
|
||||
),
|
||||
)
|
||||
|
||||
it.live(
|
||||
"Query schema rejection returns NamedError-shaped JSON",
|
||||
withTmp({ git: true, config: { formatter: false, lsp: false } }, (tmp) =>
|
||||
Effect.gen(function* () {
|
||||
// /find/file?limit=999999 violates the limit constraint check.
|
||||
const url = `/find/file?query=foo&limit=999999&directory=${encodeURIComponent(tmp.path)}`
|
||||
const res = yield* Effect.promise(async () => Server.Default().app.request(url))
|
||||
const body = yield* Effect.promise(async () => res.text())
|
||||
expect(res.status).toBe(400)
|
||||
const parsed = JSON.parse(body)
|
||||
expect(parsed).toMatchObject({ name: "BadRequest", data: { kind: "Query" } })
|
||||
}),
|
||||
),
|
||||
)
|
||||
|
||||
it.live(
|
||||
"rejected request body never echoes back unbounded — message is capped",
|
||||
// Defense against DoS-amplification + secret-echo: Effect's Issue formatter
|
||||
// dumps the rejected `actual` verbatim. A multi-MB invalid array would
|
||||
// become a multi-MB 400 response and log line. Cap kicks in around 1KB.
|
||||
withTmp({ git: true, config: { formatter: false, lsp: false } }, (tmp) =>
|
||||
Effect.gen(function* () {
|
||||
const huge = "X".repeat(50_000)
|
||||
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: huge }),
|
||||
}),
|
||||
)
|
||||
const body = yield* Effect.promise(async () => res.text())
|
||||
expect(res.status).toBe(400)
|
||||
// 1 KB cap + small JSON envelope ≈ <2 KB — never tens of KB.
|
||||
expect(body.length).toBeLessThan(2 * 1024)
|
||||
const parsed = JSON.parse(body)
|
||||
expect(parsed.data.message).not.toContain(huge)
|
||||
}),
|
||||
),
|
||||
)
|
||||
|
||||
it.live(
|
||||
"response-encode failure: corrupted stored row returns NamedError-shaped JSON with field path",
|
||||
withTmp({ config: { formatter: false, lsp: false } }, (tmp) =>
|
||||
Effect.gen(function* () {
|
||||
const sessionID = yield* Effect.promise(() => seedCorruptStepFinishPart(tmp.path))
|
||||
const url = `${SessionPaths.messages.replace(":sessionID", sessionID)}?limit=80&directory=${encodeURIComponent(tmp.path)}`
|
||||
const res = yield* Effect.promise(async () => Server.Default().app.request(url))
|
||||
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)
|
||||
expect(parsed).toMatchObject({ name: "BadRequest", data: { kind: "Body" } })
|
||||
// Field path in data.message — what made this PR worth shipping.
|
||||
expect(parsed.data.message).toMatch(/output/)
|
||||
}),
|
||||
),
|
||||
)
|
||||
})
|
||||
@@ -52,23 +52,33 @@ describe("v2 SDK error shape", () => {
|
||||
})
|
||||
})
|
||||
|
||||
test("400 with empty body throws a real Error naming the status", async () => {
|
||||
test("400 schema rejection: SDK extracts the field-level reason from the NamedError body", async () => {
|
||||
// Canary for the #26631 wire shape. Asserts the contract end-to-end:
|
||||
// server emits {name:"BadRequest", data:{message, kind}}, SDK's
|
||||
// wrapClientError extracts .data.message into Error.message. If either
|
||||
// side regresses (#26457 reverted because both layers were missing),
|
||||
// this test fails before users see (empty response body).
|
||||
await using tmp = await tmpdir({ config: { formatter: false, lsp: false } })
|
||||
const sdk = client(tmp.path)
|
||||
|
||||
let caught: unknown
|
||||
try {
|
||||
// POST /sync/history with `aggregate: -1` triggers schema validation
|
||||
// that returns an empty 400 body (verified via plan-mode probe).
|
||||
await sdk.sync.history.list({ aggregate: -1 } as any, { throwOnError: true })
|
||||
await sdk.sync.history.list({ body: { aggregate: -1 } as any }, { throwOnError: true })
|
||||
} catch (e) {
|
||||
caught = e
|
||||
}
|
||||
|
||||
expect(caught).toBeInstanceOf(Error)
|
||||
const err = caught as Error
|
||||
const cause = err.cause as { status?: number }
|
||||
expect(err.message.length).toBeGreaterThan(0)
|
||||
const cause = err.cause as { body?: any; status?: number }
|
||||
expect(cause.status).toBe(400)
|
||||
expect(cause.body).toMatchObject({
|
||||
name: "BadRequest",
|
||||
data: { kind: expect.stringMatching(/^(Body|Payload)$/) },
|
||||
})
|
||||
expect(typeof cause.body.data.message).toBe("string")
|
||||
expect(cause.body.data.message.length).toBeGreaterThan(0)
|
||||
// Whatever the server put in data.message must be what the user sees.
|
||||
expect(err.message).toBe(cause.body.data.message)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
// Smoke test: v1 SDK (the plugin contract) can actually reach core endpoints
|
||||
// against the current server. v1 generation has been frozen since #5216
|
||||
// (2025-12-07) so types may be stale, but runtime calls should still work
|
||||
// for endpoints the v1 SDK was generated against.
|
||||
import { afterEach, describe, expect, test } from "bun:test"
|
||||
import { createOpencodeClient } from "@opencode-ai/sdk"
|
||||
import { Server } from "../../src/server/server"
|
||||
import { tmpdir, disposeAllInstances } from "../fixture/fixture"
|
||||
import { resetDatabase } from "../fixture/db"
|
||||
import * as Log from "@opencode-ai/core/util/log"
|
||||
|
||||
void Log.init({ print: false })
|
||||
|
||||
afterEach(async () => {
|
||||
await disposeAllInstances()
|
||||
await resetDatabase()
|
||||
})
|
||||
|
||||
function client(directory: string) {
|
||||
return createOpencodeClient({
|
||||
baseUrl: "http://test",
|
||||
directory,
|
||||
fetch: ((req: Request) => Server.Default().app.fetch(req)) as unknown as typeof fetch,
|
||||
})
|
||||
}
|
||||
|
||||
describe("v1 SDK runtime smoke", () => {
|
||||
test("session.list reaches the server and returns 200", async () => {
|
||||
await using tmp = await tmpdir({ git: true, config: { formatter: false, lsp: false } })
|
||||
const sdk = client(tmp.path)
|
||||
const result = await sdk.session.list()
|
||||
expect(result.error).toBeUndefined()
|
||||
expect(Array.isArray(result.data)).toBe(true)
|
||||
})
|
||||
|
||||
test("path.get reaches the server and returns 200", async () => {
|
||||
await using tmp = await tmpdir({ git: true, config: { formatter: false, lsp: false } })
|
||||
const sdk = client(tmp.path)
|
||||
const result = await sdk.path.get()
|
||||
expect(result.error).toBeUndefined()
|
||||
expect(result.data).toBeDefined()
|
||||
})
|
||||
|
||||
test("config.get reaches the server and returns 200", async () => {
|
||||
await using tmp = await tmpdir({ git: true, config: { formatter: false, lsp: false } })
|
||||
const sdk = client(tmp.path)
|
||||
const result = await sdk.config.get()
|
||||
expect(result.error).toBeUndefined()
|
||||
expect(result.data).toBeDefined()
|
||||
})
|
||||
|
||||
test("session 404: result-tuple path returns the error body", async () => {
|
||||
await using tmp = await tmpdir({ git: true, config: { formatter: false, lsp: false } })
|
||||
const sdk = client(tmp.path)
|
||||
const result = await sdk.session.get({ path: { id: "ses_no_such" } as never })
|
||||
expect(result.error).toBeDefined()
|
||||
// wire body for 404 is NamedError-shaped
|
||||
expect(result.error).toMatchObject({ name: "NotFoundError" })
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user