fix(codemode): keep directional projection linear and single-pass

2026-07-17 00:04:47 -05:00
parent b14a88227f
commit cc1fdb45bd
4 changed files with 165 additions and 35 deletions
@@ -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.
@@ -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<string>()
  const namespaces = new Set<string>()
@@ -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<unknown, boolean>
+  readonly names: Map<unknown, ReadonlySet<string>>
+}
+const projectionCaches = new WeakMap<Document, Record<SchemaDirection, DirectionCache>>()
+
+const projectionCache = (document: Document, direction: SchemaDirection): DirectionCache => {
+  const existing = projectionCaches.get(document)
+  if (existing !== undefined) return existing[direction]
+  const created = {
+    request: { hidden: new Map<unknown, boolean>(), names: new Map<unknown, ReadonlySet<string>>() },
+    response: { hidden: new Map<unknown, boolean>(), names: new Map<unknown, ReadonlySet<string>>() },
+  }
+  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<Document, boolean>()
+
+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<object> = 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<object> = new Set(),
-): ReadonlySet<string> => {
-  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<string> => {
+  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,
      })),
@@ -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({