fix(codemode): correct OpenAPI response fallthrough and cookie handling
This commit is contained in:
@@ -176,13 +176,11 @@ const runtime = CodeMode.make({ tools: { opencode: api.tools } })
|
||||
const result = await Effect.runPromise(runtime.execute(code).pipe(Effect.provide(FetchHttpClient.layer)))
|
||||
```
|
||||
|
||||
`fromSpec` is synchronous and returns `{ tools, skipped }`. It throws on structurally invalid specs; operations it cannot represent (non-JSON request bodies, unresolved server URL templates) are reported in `skipped` with a reason instead of producing broken tools. `baseUrl` overrides the spec's `servers`; `serverVariables` fills templated server URLs; `operations` filters which operations become tools; `headers` adds static host headers to every request (not model-visible, though a spec-declared header parameter with the same name may override the value; auth always wins).
|
||||
`fromSpec` is synchronous and returns `{ tools, skipped }`. It throws on structurally invalid specs; operations it cannot represent (non-JSON request bodies, unresolved server URL templates) are reported in `skipped` with a reason instead of producing broken tools. See the `Options` and `AuthResolver` docstrings in `src/openapi.ts` for the full option and auth semantics; the essentials:
|
||||
|
||||
Tool inputs group parameters by OpenAPI location - `{ path, query, headers, cookies, body }` - and never include auth. Output schemas come from the first 2xx JSON response; `#/components/schemas/*` refs are shared with the signature renderer, so catalog signatures and search results expand referenced types. Operations with no response content at all (e.g. 204) resolve to `null`; declared non-JSON responses (e.g. `text/plain`) advertise `unknown` and return the raw body. Non-2xx responses become safe tool failures carrying the status and a size-capped body summary, so programs can `catch` and read them.
|
||||
|
||||
Auth follows OpenAPI `security` semantics: root-level security is the default, operation-level security replaces it, `security: []` (and an empty `{}` alternative) means unauthenticated. The requirement list is OR - the adapter picks the first alternative whose every scheme (AND) resolves. `auth.resolve` is called per scheme per invocation with `{ schemeName, scheme, scopes, operation }` and returns credential material: `{ type: "bearer" | "basic" | "apiKey" | "header", ... }`. The carrier for an `apiKey` credential comes from the scheme declaration (header, query, or cookie), never from the host. Returning `undefined` skips to the next alternative; failing aborts the call - an expired refresh token must not silently fall through to an unauthenticated alternative. Two credentials colliding on one carrier - the same header, query parameter, or cookie - fail with a clear error.
|
||||
|
||||
Credential storage, OAuth flows, and token refresh stay host-side behind `resolve`; the adapter only asks for a valid credential at call time. Generated tools require `HttpClient.HttpClient` (from `effect/unstable/http`) in the Effect environment - provide `FetchHttpClient.layer` or a custom/test client layer at execution.
|
||||
- Tool inputs group parameters by OpenAPI location - `{ path, query, headers, cookies, body }` - and never include auth. Output schemas come from the first success (2xx/`2XX`) JSON response; `#/components/schemas/*` refs are expanded by the signature renderer. No-content success responses (e.g. 204) resolve to `null`; declared non-JSON responses advertise `unknown` and return the raw body. Non-2xx responses become safe tool failures carrying the status and a size-capped body summary, so programs can `catch` and read them.
|
||||
- Auth follows OpenAPI `security` semantics (root default, operation override, `security: []` = unauthenticated, OR across requirements, AND within one). `auth.resolve` supplies credential material per scheme per invocation; the apiKey carrier comes from the scheme declaration, never from the host. Returning `undefined` tries the next alternative; failing aborts the call; two credentials colliding on one carrier fail clearly. Credential storage, OAuth flows, and token refresh stay host-side behind `resolve`.
|
||||
- `headers` adds static host headers (not model-visible; declared header parameters may override them, auth always wins). Generated tools require `HttpClient.HttpClient` (from `effect/unstable/http`) in the Effect environment - provide `FetchHttpClient.layer` or a custom/test client layer at execution.
|
||||
|
||||
## Discovery
|
||||
|
||||
|
||||
@@ -375,13 +375,14 @@ const outputSchema = (
|
||||
): JsonSchema | undefined => {
|
||||
if (!isRecord(operation.responses)) return undefined
|
||||
const entries = Object.entries(operation.responses)
|
||||
// Literal 2xx codes, then the 2XX wildcard range, then default.
|
||||
const preferred = [
|
||||
// Literal 2xx codes, then the 2XX wildcard range. `default` typically
|
||||
// describes errors, so it is consulted only when no success response exists.
|
||||
const successes = [
|
||||
...entries.filter(([status]) => /^2\d\d$/.test(status)).sort(([a], [b]) => a.localeCompare(b)),
|
||||
...entries.filter(([status]) => status.toUpperCase() === "2XX"),
|
||||
...entries.filter(([status]) => status === "default"),
|
||||
]
|
||||
for (const [, ref] of preferred) {
|
||||
const candidates = successes.length > 0 ? successes : entries.filter(([status]) => status === "default")
|
||||
for (const [, ref] of candidates) {
|
||||
const response = resolve(document, ref)
|
||||
if (!isRecord(response)) continue
|
||||
const content = isRecord(response.content) ? response.content : {}
|
||||
@@ -392,10 +393,11 @@ const outputSchema = (
|
||||
// Declared content without a usable JSON schema (e.g. text/plain): the tool
|
||||
// returns the raw body, so advertise unknown rather than a wrong null.
|
||||
if (Object.keys(content).length > 0) return undefined
|
||||
// A success response declared with no content at all (e.g. 204) resolves
|
||||
// to null; do not fall through to a later (likely error) response shape.
|
||||
return { type: "null" }
|
||||
}
|
||||
// Success responses declared with no content at all (e.g. 204) resolve to
|
||||
// null. Without any recognized success/default response the shape is unknown.
|
||||
return preferred.length > 0 ? { type: "null" } : undefined
|
||||
return undefined
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -534,14 +536,20 @@ const invoke = (plan: Plan, input: unknown): Effect.Effect<unknown, unknown, Htt
|
||||
if (item === undefined || item === null) continue
|
||||
request = HttpClientRequest.setHeader(request, parameter.name, renderPrimitive(item))
|
||||
}
|
||||
// Auth cookies come first and shadow model cookie parameters with the same
|
||||
// name (servers take the first occurrence). Model values are encoded so a
|
||||
// value containing ';' or '=' cannot inject extra cookie pairs; host
|
||||
// credentials are trusted and sent verbatim.
|
||||
const cookiePairs = [
|
||||
...Object.entries(auth.cookies).map(([name, item]) => `${name}=${item}`),
|
||||
...plan.parameters
|
||||
.filter((parameter) => parameter.location === "cookie")
|
||||
.filter((parameter) => parameter.location === "cookie" && auth.cookies[parameter.name] === undefined)
|
||||
.flatMap((parameter) => {
|
||||
const item = cookies[parameter.name]
|
||||
return item === undefined || item === null ? [] : [`${parameter.name}=${renderPrimitive(item)}`]
|
||||
return item === undefined || item === null
|
||||
? []
|
||||
: [`${parameter.name}=${encodeURIComponent(renderPrimitive(item))}`]
|
||||
}),
|
||||
...Object.entries(auth.cookies).map(([name, item]) => `${name}=${item}`),
|
||||
]
|
||||
if (cookiePairs.length > 0) request = HttpClientRequest.setHeader(request, "cookie", cookiePairs.join("; "))
|
||||
request = HttpClientRequest.setHeaders(request, auth.headers)
|
||||
|
||||
@@ -445,6 +445,72 @@ describe("OpenAPI.fromSpec", () => {
|
||||
expect(requests).toHaveLength(0)
|
||||
})
|
||||
|
||||
test("a no-content success response resolves to null even when default declares an error shape", () => {
|
||||
const mixed = {
|
||||
openapi: "3.1.0",
|
||||
info: { title: "Mixed", version: "1.0.0" },
|
||||
servers: [{ url: "https://mixed.example" }],
|
||||
paths: {
|
||||
"/thing": {
|
||||
delete: {
|
||||
operationId: "deleteThing",
|
||||
responses: {
|
||||
"204": { description: "deleted" },
|
||||
default: {
|
||||
description: "error",
|
||||
content: {
|
||||
"application/json": { schema: { type: "object", properties: { message: { type: "string" } } } },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
const runtime = CodeMode.make({ tools: { api: OpenAPI.fromSpec({ spec: mixed }).tools } })
|
||||
expect(runtime.instructions()).toContain("tools.api.deleteThing(input: {}): Promise<null>")
|
||||
})
|
||||
|
||||
test("auth cookies shadow model cookie parameters and model values are encoded", async () => {
|
||||
const cookieSpec = {
|
||||
openapi: "3.1.0",
|
||||
info: { title: "Cookies", version: "1.0.0" },
|
||||
servers: [{ url: "https://cookies.example" }],
|
||||
paths: {
|
||||
"/c": {
|
||||
get: {
|
||||
operationId: "c",
|
||||
security: [{ CookieKey: [] }],
|
||||
parameters: [
|
||||
{ name: "session", in: "cookie", schema: { type: "string" } },
|
||||
{ name: "theme", in: "cookie", schema: { type: "string" } },
|
||||
],
|
||||
responses: { "200": { description: "ok" } },
|
||||
},
|
||||
},
|
||||
},
|
||||
components: { securitySchemes: { CookieKey: { type: "apiKey", in: "cookie", name: "session" } } },
|
||||
}
|
||||
const { requests, layer } = recordingClient(() => json({}))
|
||||
const runtime = CodeMode.make({
|
||||
tools: {
|
||||
api: OpenAPI.fromSpec({
|
||||
spec: cookieSpec,
|
||||
auth: { resolve: () => Effect.succeed({ type: "apiKey", value: "real" } as const) },
|
||||
}).tools,
|
||||
},
|
||||
})
|
||||
|
||||
const result = await Effect.runPromise(
|
||||
runtime
|
||||
.execute(`return await tools.api.c({ cookies: { session: "forged", theme: "dark; session=forged" } })`)
|
||||
.pipe(Effect.provide(layer)),
|
||||
)
|
||||
|
||||
expect(result).toMatchObject({ ok: true })
|
||||
expect(requests[0]!.headers["cookie"]).toBe("session=real; theme=dark%3B%20session%3Dforged")
|
||||
})
|
||||
|
||||
test("spec server variables substitute defaults and explicit values", () => {
|
||||
const templated = {
|
||||
openapi: "3.1.0",
|
||||
|
||||
Reference in New Issue
Block a user