Skip to content

Commit dac81cd

Browse files
authored
fix(httpapi): preserve v2 openapi errors (#28298)
1 parent 2c3bcf3 commit dac81cd

2 files changed

Lines changed: 122 additions & 10 deletions

File tree

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

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -99,17 +99,13 @@ function matchLegacyOpenApi(input: Record<string, unknown>) {
9999
applyLegacySchemaOverrides(spec)
100100
normalizeComponentDescriptions(spec)
101101
addLegacyErrorSchemas(spec)
102-
delete spec.components?.schemas?.Unauthorized
103-
delete spec.components?.schemas?.EffectHttpApiErrorBadRequest
104-
delete spec.components?.schemas?.EffectHttpApiErrorNotFound
105-
delete spec.components?.schemas?.effect_HttpApiError_BadRequest
106-
delete spec.components?.schemas?.effect_HttpApiError_NotFound
107102
delete spec.components?.securitySchemes
108103

109104
for (const [path, item] of Object.entries(spec.paths ?? {})) {
110105
for (const method of ["get", "post", "put", "delete", "patch"] as const) {
111106
const operation = item[method]
112107
if (!operation) continue
108+
const isV2Api = isV2ApiPath(path)
113109
if (operation.requestBody) {
114110
// The legacy OpenAPI surface never marked request bodies as required.
115111
// Keep that SDK surface stable while the HttpApi spec is tightened.
@@ -146,11 +142,14 @@ function matchLegacyOpenApi(input: Record<string, unknown>) {
146142
if (content.schema) content.schema = stripOptionalNull(structuredClone(content.schema))
147143
}
148144
}
149-
// Auth is still runtime middleware outside the public OpenAPI metadata, so
150-
// the SDK should not expose auth schemes or generated 401 error unions.
151-
delete operation.security
152-
delete operation.responses?.["401"]
153-
normalizeLegacyErrorResponses(operation)
145+
if (!isV2Api) {
146+
// Auth is still runtime middleware outside the legacy public OpenAPI
147+
// metadata, so the legacy SDK should not expose auth schemes or
148+
// generated 401 error unions.
149+
delete operation.security
150+
delete operation.responses?.["401"]
151+
normalizeLegacyErrorResponses(operation)
152+
}
154153
normalizeLegacyOperation(operation, path, method)
155154
if ((path === "/event" || path === "/global/event") && method === "get") {
156155
// HttpApi has no first-class SSE response schema, and these handlers are
@@ -171,9 +170,14 @@ function matchLegacyOpenApi(input: Record<string, unknown>) {
171170
for (const param of operation.parameters ?? []) normalizeParameter(param, route)
172171
}
173172
}
173+
deleteUnusedLegacyErrorComponents(spec)
174174
return input
175175
}
176176

177+
function isV2ApiPath(path: string) {
178+
return path === "/api" || path.startsWith("/api/")
179+
}
180+
177181
function addLegacyErrorSchemas(spec: OpenApiSpec) {
178182
if (!spec.components?.schemas) return
179183
spec.components.schemas.BadRequestError = {
@@ -345,6 +349,26 @@ function normalizeLegacyErrorResponses(operation: OpenApiOperation) {
345349
}
346350
}
347351

352+
function deleteUnusedLegacyErrorComponents(spec: OpenApiSpec) {
353+
for (const name of [
354+
"Unauthorized",
355+
"EffectHttpApiErrorBadRequest",
356+
"EffectHttpApiErrorNotFound",
357+
"effect_HttpApiError_BadRequest",
358+
"effect_HttpApiError_NotFound",
359+
]) {
360+
if (referencesComponent(spec.paths, name)) continue
361+
delete spec.components?.schemas?.[name]
362+
}
363+
}
364+
365+
function referencesComponent(input: unknown, name: string): boolean {
366+
if (Array.isArray(input)) return input.some((item) => referencesComponent(item, name))
367+
if (!input || typeof input !== "object") return false
368+
if ((input as OpenApiSchema).$ref === `#/components/schemas/${name}`) return true
369+
return Object.values(input).some((value) => referencesComponent(value, name))
370+
}
371+
348372
function normalizeLegacyOperation(operation: OpenApiOperation, path: string, method: string) {
349373
if (path === "/experimental/console/switch" && method === "post") delete operation.responses?.["400"]
350374
if (path === "/pty/{ptyID}" && method === "put") delete operation.responses?.["404"]
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import { describe, expect, test } from "bun:test"
2+
import { OpenApi } from "effect/unstable/httpapi"
3+
import { PublicApi } from "../../src/server/routes/instance/httpapi/public"
4+
5+
type Method = "get" | "post" | "put" | "delete" | "patch"
6+
type OpenApiSchema = { readonly $ref?: string }
7+
type OpenApiResponse = {
8+
readonly description?: string
9+
readonly content?: Record<string, { readonly schema?: OpenApiSchema }>
10+
}
11+
type OpenApiOperation = {
12+
readonly responses?: Record<string, OpenApiResponse>
13+
readonly security?: unknown
14+
}
15+
type OpenApiPathItem = Partial<Record<Method, OpenApiOperation>>
16+
type OpenApiSpec = { readonly paths: Record<string, OpenApiPathItem> }
17+
18+
const methods = ["get", "post", "put", "delete", "patch"] as const
19+
20+
const allowedV2BuiltInEndpointErrors = [
21+
"GET /api/session 400 effect_HttpApiError_BadRequest",
22+
"GET /api/session/{sessionID}/message 400 effect_HttpApiError_BadRequest",
23+
]
24+
25+
function v2Operations(spec: OpenApiSpec) {
26+
return Object.entries(spec.paths).flatMap(([path, item]) =>
27+
path.startsWith("/api/")
28+
? methods.flatMap((method) => {
29+
const operation = item[method]
30+
return operation ? [{ method, path, operation }] : []
31+
})
32+
: [],
33+
)
34+
}
35+
36+
function responseRef(response: OpenApiResponse | undefined) {
37+
return response?.content?.["application/json"]?.schema?.$ref
38+
}
39+
40+
function componentName(ref: string) {
41+
return ref.replace("#/components/schemas/", "")
42+
}
43+
44+
function isBuiltInEndpointError(name: string) {
45+
return name.startsWith("EffectHttpApiError") || name.startsWith("effect_HttpApiError_")
46+
}
47+
48+
describe("PublicApi OpenAPI v2 errors", () => {
49+
test("preserves /api auth responses", () => {
50+
const spec = OpenApi.fromApi(PublicApi) as OpenApiSpec
51+
52+
for (const route of v2Operations(spec)) {
53+
expect(route.operation.responses?.["401"], `${route.method.toUpperCase()} ${route.path}`).toBeDefined()
54+
expect(route.operation.security, `${route.method.toUpperCase()} ${route.path}`).toEqual([])
55+
}
56+
})
57+
58+
test("does not rewrite /api endpoint errors to legacy error components", () => {
59+
const spec = OpenApi.fromApi(PublicApi) as OpenApiSpec
60+
const refs = v2Operations(spec)
61+
.flatMap((route) =>
62+
Object.entries(route.operation.responses ?? {}).flatMap(([status, response]) => {
63+
const ref = responseRef(response)
64+
return ref ? [`${route.method.toUpperCase()} ${route.path} ${status} ${componentName(ref)}`] : []
65+
}),
66+
)
67+
.filter((entry) => entry.includes("BadRequestError") || entry.includes("NotFoundError"))
68+
69+
expect(refs).toEqual(["GET /api/provider/{providerID} 404 NotFoundError"])
70+
})
71+
72+
test("new /api endpoint errors cannot use built-in components without an explicit allowlist", () => {
73+
const spec = OpenApi.fromApi(PublicApi) as OpenApiSpec
74+
const builtInEndpointErrors = v2Operations(spec)
75+
.flatMap((route) =>
76+
Object.entries(route.operation.responses ?? {}).flatMap(([status, response]) => {
77+
if (status === "401") return []
78+
const ref = responseRef(response)
79+
if (!ref) return []
80+
const name = componentName(ref)
81+
return isBuiltInEndpointError(name) ? [`${route.method.toUpperCase()} ${route.path} ${status} ${name}`] : []
82+
}),
83+
)
84+
.sort()
85+
86+
expect(builtInEndpointErrors).toEqual(allowedV2BuiltInEndpointErrors)
87+
})
88+
})

0 commit comments

Comments
 (0)