From cc1fdb45bd93660d37da492638e7c8d62cf38566 Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Fri, 17 Jul 2026 00:04:47 -0500 Subject: [PATCH] fix(codemode): keep directional projection linear and single-pass --- packages/codemode/src/openapi/TODO.md | 1 + packages/codemode/src/openapi/index.ts | 5 +- packages/codemode/src/openapi/spec.ts | 112 +++++++++++++++++-------- packages/codemode/test/openapi.test.ts | 82 ++++++++++++++++++ 4 files changed, 165 insertions(+), 35 deletions(-) diff --git a/packages/codemode/src/openapi/TODO.md b/packages/codemode/src/openapi/TODO.md index 9cd536172c..107ec75fef 100644 --- a/packages/codemode/src/openapi/TODO.md +++ b/packages/codemode/src/openapi/TODO.md @@ -7,6 +7,7 @@ The initial adapter intentionally skips operations it cannot execute correctly. - External references and complete nested `$defs` support. - `$anchor` and nested `$id` resource resolution in directional (`readOnly`/`writeOnly`) projection. - Use-site cleanup for `allOf` branches that reference shared component schemas: per-direction component definitions are projected globally, so a directional annotation declared only at one use site cannot remove the property from a referenced component's definition. +- Hidden-name cleanup inside `then`/`else`/`dependentSchemas`/`dependentRequired`, which constrain the same instance as `allOf`; a hidden property may remain named in those keywords. - Relative or templated server URLs and server variables. - Base URLs containing query strings or fragments. - Runtime response-schema validation and full content negotiation. diff --git a/packages/codemode/src/openapi/index.ts b/packages/codemode/src/openapi/index.ts index 4649d884ef..5ea688129e 100644 --- a/packages/codemode/src/openapi/index.ts +++ b/packages/codemode/src/openapi/index.ts @@ -3,6 +3,7 @@ import { make, type Definition } from "../tool.js" import { invoke } from "./runtime.js" import { componentDefinitions, + hasDirectionalSchemas, inputSchema, isRecord, methods, @@ -39,7 +40,9 @@ export const fromSpec = (options: Options): Result => { const schemes = securitySchemes(document) const defaultSecurity = securityRequirements(document.security) const requestDefinitions = componentDefinitions(document, "request") - const responseDefinitions = componentDefinitions(document, "response") + const responseDefinitions = hasDirectionalSchemas(document) + ? componentDefinitions(document, "response") + : requestDefinitions const paths = isRecord(document.paths) ? document.paths : {} const used = new Set() const namespaces = new Set() diff --git a/packages/codemode/src/openapi/spec.ts b/packages/codemode/src/openapi/spec.ts index 8a3ea5ef94..69101758bd 100644 --- a/packages/codemode/src/openapi/spec.ts +++ b/packages/codemode/src/openapi/spec.ts @@ -70,38 +70,71 @@ const resolveResource = (document: Document, resource: SchemaResource): SchemaRe return next(resource, new Set()) } +// Hidden-ness is memoized per schema object and direction so that diamond-shaped +// reference graphs (the same component referenced from many sites) stay linear; +// path-scoped visited sets would re-traverse shared subtrees exponentially. Entries +// are seeded before recursion so reference cycles terminate as not hidden. A schema +// reachable under multiple resolution roots reuses the first root's result. +type DirectionCache = { + readonly hidden: Map + readonly names: Map> +} +const projectionCaches = new WeakMap>() + +const projectionCache = (document: Document, direction: SchemaDirection): DirectionCache => { + const existing = projectionCaches.get(document) + if (existing !== undefined) return existing[direction] + const created = { + request: { hidden: new Map(), names: new Map>() }, + response: { hidden: new Map(), names: new Map>() }, + } + projectionCaches.set(document, created) + return created[direction] +} + +// Most documents never use directional keywords; one cached linear scan lets +// projection and the doubled per-direction component normalization be skipped entirely. +const directionalDocuments = new WeakMap() + +export const hasDirectionalSchemas = (document: Document): boolean => { + const cached = directionalDocuments.get(document) + if (cached !== undefined) return cached + const contains = (value: unknown): boolean => { + if (Array.isArray(value)) return value.some(contains) + if (!isRecord(value)) return false + if (own(value, "readOnly") === true || own(value, "writeOnly") === true) return true + return Object.values(value).some(contains) + } + const result = contains(document) + directionalDocuments.set(document, result) + return result +} + // OpenAPI 3.1 allows keywords as siblings of `$ref`, so a schema's own declarations // are inspected before following the reference. -const isHidden = ( - document: Document, - resource: SchemaResource, - direction: SchemaDirection, - seen: ReadonlySet = new Set(), -): boolean => { - if (!isRecord(resource.value) || seen.has(resource.value)) return false +const isHidden = (document: Document, resource: SchemaResource, direction: SchemaDirection): boolean => { + if (!isRecord(resource.value)) return false if (own(resource.value, hiddenKeyword[direction]) === true) return true - const nextSeen = new Set([...seen, resource.value]) - if ( - asArray(own(resource.value, "allOf")).some((item) => - isHidden(document, { ...resource, value: item }, direction, nextSeen), - ) - ) { - return true - } + const cache = projectionCache(document, direction).hidden + const cached = cache.get(resource.value) + if (cached !== undefined) return cached + cache.set(resource.value, false) const target = resolveResource(document, resource) - return target.value !== resource.value && isHidden(document, target, direction, nextSeen) + const result = + asArray(own(resource.value, "allOf")).some((item) => isHidden(document, { ...resource, value: item }, direction)) || + (target.value !== resource.value && isHidden(document, target, direction)) + cache.set(resource.value, result) + return result } // Hidden property names declared by a schema itself or inherited through `$ref` and // `allOf` composition, so sibling `required` lists stay consistent after projection. -const hiddenNames = ( - document: Document, - resource: SchemaResource, - direction: SchemaDirection, - seen: ReadonlySet = new Set(), -): ReadonlySet => { - if (!isRecord(resource.value) || seen.has(resource.value)) return new Set() - const nextSeen = new Set([...seen, resource.value]) +const hiddenNames = (document: Document, resource: SchemaResource, direction: SchemaDirection): ReadonlySet => { + if (!isRecord(resource.value)) return new Set() + const cache = projectionCache(document, direction).names + const cached = cache.get(resource.value) + if (cached !== undefined) return cached + cache.set(resource.value, new Set()) const properties = own(resource.value, "properties") const declared = isRecord(properties) ? Object.entries(properties) @@ -109,11 +142,13 @@ const hiddenNames = ( .map(([name]) => name) : [] const composed = asArray(own(resource.value, "allOf")).flatMap((item) => [ - ...hiddenNames(document, { ...resource, value: item }, direction, nextSeen), + ...hiddenNames(document, { ...resource, value: item }, direction), ]) const target = resolveResource(document, resource) - const referenced = target.value === resource.value ? [] : hiddenNames(document, target, direction, nextSeen) - return new Set([...declared, ...composed, ...referenced]) + const referenced = target.value === resource.value ? [] : hiddenNames(document, target, direction) + const result = new Set([...declared, ...composed, ...referenced]) + cache.set(resource.value, result) + return result } const nestedSchemas = new Set([ @@ -167,17 +202,22 @@ const directionalSchema = ( ) } -const projectSchema = (document: Document, value: unknown, direction: SchemaDirection): JsonSchema => { - const projected = directionalSchema(document, { value, root: value }, direction) - if (!isRecord(projected)) return {} +const normalizeSchema = (document: Document, value: unknown): JsonSchema => { + if (!isRecord(value)) return {} const normalized = nonEmptyString(document.openapi)?.startsWith("3.0") - ? fromSchemaOpenApi3_0(projected) - : fromSchemaOpenApi3_1(projected) + ? fromSchemaOpenApi3_0(value) + : fromSchemaOpenApi3_1(value) return Object.keys(normalized.definitions).length === 0 ? normalized.schema : { ...normalized.schema, $defs: normalized.definitions } } +const projectSchema = (document: Document, value: unknown, direction: SchemaDirection): JsonSchema => + normalizeSchema( + document, + hasDirectionalSchemas(document) ? directionalSchema(document, { value, root: value }, direction) : value, + ) + export const componentDefinitions = ( document: Document, direction: SchemaDirection, @@ -323,7 +363,9 @@ const operationBody = ( } } const resolvedSchema = resolve(document, selected.schema) - const schema = directionalSchema(document, { value: resolvedSchema, root: resolvedSchema }, "request") + const schema = hasDirectionalSchemas(document) + ? directionalSchema(document, { value: resolvedSchema, root: resolvedSchema }, "request") + : resolvedSchema const required = resolved.required === true if (!isFlattenableObjectBody(schema, required)) { return { @@ -349,11 +391,13 @@ const operationBody = ( return { ok: true, value: { + // Field schemas were already projected with the body as resolution root; a second + // directional pass rooted at the field would misresolve shadowed local $defs. fields: Object.entries(schema.properties).map(([name, value]) => ({ name, location: "body" as const, required: required && requiredProperties.has(name), - schema: projectSchema(document, value, "request"), + schema: normalizeSchema(document, value), style: undefined, explode: undefined, })), diff --git a/packages/codemode/test/openapi.test.ts b/packages/codemode/test/openapi.test.ts index 98f879ce24..db121be691 100644 --- a/packages/codemode/test/openapi.test.ts +++ b/packages/codemode/test/openapi.test.ts @@ -573,6 +573,88 @@ describe("OpenAPI.fromSpec", () => { expect(node.required).toEqual(["name", "child"]) }) + test("projects diamond-shaped reference graphs in linear time", () => { + // Each component references the next twice; without memoized hidden-ness this is 2^30 work. + const depth = 30 + const schemas = Object.fromEntries( + Array.from({ length: depth }, (_, index) => [ + `C${index}`, + index === depth - 1 + ? { type: "object", properties: { id: { type: "string", readOnly: true }, name: { type: "string" } } } + : { allOf: [{ $ref: `#/components/schemas/C${index + 1}` }, { $ref: `#/components/schemas/C${index + 1}` }] }, + ]), + ) + const tool = toolAt( + OpenAPI.fromSpec({ + baseUrl, + spec: { + openapi: "3.1.0", + paths: { + "/test": { + post: { + operationId: "test", + responses: { 200: { description: "Success" } }, + requestBody: { + required: true, + content: { "application/json": { schema: { $ref: "#/components/schemas/C0" } } }, + }, + }, + }, + }, + components: { schemas }, + }, + }).tools, + "test", + ) + if (!Tool.isDefinition(tool) || !isRecord(tool.input)) throw new Error("test was not generated") + const definitions = isRecord(tool.input.$defs) ? tool.input.$defs : {} + const leaf = isRecord(definitions[`C${depth - 1}`]) ? definitions[`C${depth - 1}`] : {} + + expect(Object.keys(isRecord(leaf.properties) ? leaf.properties : {})).toEqual(["name"]) + }) + + test("does not misresolve shadowed local $defs when flattening body fields", () => { + const tool = toolAt( + OpenAPI.fromSpec({ + baseUrl, + spec: singleOperation( + { + requestBody: { + required: true, + content: { + "application/json": { + schema: { + type: "object", + additionalProperties: false, + required: ["record"], + $defs: { Value: { type: "string" } }, + properties: { + record: { + type: "object", + required: ["x"], + properties: { x: { $ref: "#/$defs/Value" } }, + // Shadows the body-level Value; must not affect the body-rooted projection. + $defs: { Value: { type: "string", readOnly: true } }, + }, + }, + }, + }, + }, + }, + }, + "post", + ), + }).tools, + "test", + ) + if (!Tool.isDefinition(tool) || !isRecord(tool.input)) throw new Error("test was not generated") + const properties = isRecord(tool.input.properties) ? tool.input.properties : {} + const record = isRecord(properties.record) ? properties.record : {} + + expect(Object.keys(isRecord(record.properties) ? record.properties : {})).toEqual(["x"]) + expect(record.required).toEqual(["x"]) + }) + test("projects directional annotations inside parameter schemas", () => { const tool = toolAt( OpenAPI.fromSpec({