test(contract): validate six unvalidated response schemas, bundle openapi.yaml

src/api/path-id-status-openapi.test.ts

@@ -15,16 +15,25 @@
 // disciplines .integration.test.ts, all tagged "(#568)").
 import { describe, it, expect } from 'vitest';
 import { z } from 'zod';
-import { loadSpec, type OpenApiDoc } from '../test-utils/contract/validate-response.js';
+import {
+  loadSpec,
+  resolveIfRef,
+  type OpenApiDoc,
+} from '../test-utils/contract/validate-response.js';
 
 const MediaObjectSchema = z.object({ schema: z.unknown() });
 const ContentSchema = z.record(z.string(), MediaObjectSchema);
 const ResponseObjectSchema = z.object({
   description: z.string().optional(),
   content: ContentSchema.optional(),
 });
+// `responses` values are `z.unknown()`, not narrowed directly: a status like 400/404/500 is
+// typically `$ref`'d to a shared `components/responses/*` entry (e.g. BadRequest) rather than
+// documented inline, which bundling (#649) leaves as a literal `{ $ref }` pointer instead of
+// dereference's fully-inlined response object — resolved per-status in operation()/
+// expectErrorResponseSchema() before parsing into ResponseObjectSchema.
 const OperationSchema = z.object({
-  responses: z.record(z.string(), ResponseObjectSchema).optional(),
+  responses: z.record(z.string(), z.unknown()).optional(),
 });
 
 function operation(doc: OpenApiDoc, path: string, method: string): z.infer<typeof OperationSchema> {
@@ -48,14 +57,17 @@ const ErrorResponseShapeSchema = z.object({
 });
 
 function expectErrorResponseSchema(
+  doc: OpenApiDoc,
   responses: z.infer<typeof OperationSchema>['responses'],
   status: string,
   label: string
 ): void {
-  const response = responses?.[status];
-  expect(response, `${label} does not document a ${status} response`).toBeDefined();
-  const schema = response?.content?.['application/json']?.schema;
-  expect(schema, `${label} ${status} has no application/json schema`).toBeDefined();
+  const rawResponse = responses?.[status];
+  expect(rawResponse, `${label} does not document a ${status} response`).toBeDefined();
+  const response = ResponseObjectSchema.parse(resolveIfRef(doc, rawResponse));
+  const rawSchema = response.content?.['application/json']?.schema;
+  expect(rawSchema, `${label} ${status} has no application/json schema`).toBeDefined();
+  const schema = resolveIfRef(doc, rawSchema);
 
   const parsed = ErrorResponseShapeSchema.safeParse(schema);
   expect(parsed.success, `${label} ${status} is not the ErrorResponse shape`).toBe(true);
@@ -100,7 +112,7 @@ describe('openapi.yaml documents the parsePathUuid 400 on every migrated operati
       const op = operation(doc, path, method);
       // ErrorResponse always requires `success: false` + `error: string` —
       // confirm the 400 targets that shape, not some ad hoc one-off object.
-      expectErrorResponseSchema(op.responses, '400', `${method} ${path}`);
+      expectErrorResponseSchema(doc, op.responses, '400', `${method} ${path}`);
     }
   );
 });
@@ -109,6 +121,6 @@ describe('openapi.yaml documents the shared-schema 422 on POST /templates/import
   it('documents 422 (not a second, ad hoc 400) for a name that fails CreateTemplateBodySchema', async () => {
     const doc = await loadSpec();
     const op = operation(doc, '/templates/import', 'post');
-    expectErrorResponseSchema(op.responses, '422', 'post /templates/import');
+    expectErrorResponseSchema(doc, op.responses, '422', 'post /templates/import');
   });
 });

src/api/revision-parent-openapi.test.ts

@@ -13,7 +13,11 @@
 // those modules without a matching openapi.yaml edit fails here first.
 import { describe, it, expect } from 'vitest';
 import { z } from 'zod';
-import { loadSpec, type OpenApiDoc } from '../test-utils/contract/validate-response.js';
+import {
+  loadSpec,
+  resolveIfRef,
+  type OpenApiDoc,
+} from '../test-utils/contract/validate-response.js';
 
 const JsonSchemaObjectSchema = z.object({
   type: z.union([z.string(), z.array(z.string())]).optional(),
@@ -46,26 +50,37 @@ function jsonSchemaOf(content: z.infer<typeof ContentSchema> | undefined): unkno
   return media.schema;
 }
 
-/** Picks the oneOf/allOf branch whose `required` list names `field` — order-independent. */
-function branchRequiring(branches: readonly unknown[], field: string): JsonSchemaObject {
+/** Picks the oneOf/allOf branch whose `required` list names `field` — order-independent. Each
+ * branch is resolved via {@link resolveIfRef} before narrowing: bundling (#649) leaves a branch
+ * that is itself a top-level `$ref` (e.g. CreateRevisionLegacyBody/CreateRevisionStructuredBody)
+ * as a literal `{ $ref }` pointer instead of dereference's fully-inlined object. */
+function branchRequiring(
+  doc: OpenApiDoc,
+  branches: readonly unknown[],
+  field: string
+): JsonSchemaObject {
   const match = branches
-    .map((b) => JsonSchemaObjectSchema.parse(b))
+    .map((b) => JsonSchemaObjectSchema.parse(resolveIfRef(doc, b)))
     .find((b) => (b.required ?? []).includes(field));
   if (match === undefined) throw new Error(`no schema branch requires "${field}"`);
   return match;
 }
 
-/** Unwraps `data` out of the `allOf: [SuccessResponse, { data }]` envelope. */
-function dataSchemaOf(schema: unknown): unknown {
+/** Unwraps `data` out of the `allOf: [SuccessResponse, { data }]` envelope, resolving a `data`
+ * schema that is itself a top-level `$ref` (e.g. `{ $ref: RevisionWithTrees }`, #649). */
+function dataSchemaOf(doc: OpenApiDoc, schema: unknown): unknown {
   const allOf = z.object({ allOf: z.array(z.unknown()) }).parse(schema).allOf;
-  const holder = branchRequiring(allOf, 'data');
+  const holder = branchRequiring(doc, allOf, 'data');
   const properties = holder.properties;
   if (properties === undefined) throw new Error('data-bearing branch has no properties');
-  return properties['data'];
+  return resolveIfRef(doc, properties['data']);
 }
 
-function itemsOf(schema: unknown): unknown {
-  return z.object({ items: z.unknown() }).parse(schema).items;
+/** Resolves an array schema's `items` — itself commonly a top-level `$ref` (e.g. `#/.../
+ * RevisionSummary`, #649) — to its actual component shape. */
+function itemsOf(doc: OpenApiDoc, schema: unknown): unknown {
+  const items = z.object({ items: z.unknown() }).parse(schema).items;
+  return resolveIfRef(doc, items);
 }
 
 function expectNullableUuidField(schema: JsonSchemaObject, field: string): void {
@@ -82,7 +97,7 @@ describe('openapi.yaml — package_revisions.parent_revision_id (ADR-066 #389)',
     const oneOf = z
       .object({ oneOf: z.array(z.unknown()) })
       .parse(jsonSchemaOf(op.requestBody?.content)).oneOf;
-    const structured = branchRequiring(oneOf, 'type');
+    const structured = branchRequiring(doc, oneOf, 'type');
     const prop = JsonSchemaObjectSchema.parse((structured.properties ?? {})['parentRevisionId']);
     expect(prop.type).toBe('string');
     expect(prop.format).toBe('uuid');
@@ -95,32 +110,32 @@ describe('openapi.yaml — package_revisions.parent_revision_id (ADR-066 #389)',
     const oneOf = z
       .object({ oneOf: z.array(z.unknown()) })
       .parse(jsonSchemaOf(op.requestBody?.content)).oneOf;
-    const legacy = branchRequiring(oneOf, 'label');
+    const legacy = branchRequiring(doc, oneOf, 'label');
     expect(legacy.properties ?? {}).not.toHaveProperty('parentRevisionId');
   });
 
   it('RevisionSummary requires parentRevisionId as a nullable uuid (POST .../revisions 201)', async () => {
     const doc = await loadSpec();
     const op = operation(doc, '/packages/{id}/revisions', 'post');
     const revisionSummary = JsonSchemaObjectSchema.parse(
-      dataSchemaOf(jsonSchemaOf(op.responses?.['201']?.content))
+      dataSchemaOf(doc, jsonSchemaOf(op.responses?.['201']?.content))
     );
     expectNullableUuidField(revisionSummary, 'parentRevisionId');
   });
 
   it('RevisionSummary requires parentRevisionId on every list item (GET .../revisions 200)', async () => {
     const doc = await loadSpec();
     const op = operation(doc, '/packages/{id}/revisions', 'get');
-    const arraySchema = dataSchemaOf(jsonSchemaOf(op.responses?.['200']?.content));
-    const itemSchema = JsonSchemaObjectSchema.parse(itemsOf(arraySchema));
+    const arraySchema = dataSchemaOf(doc, jsonSchemaOf(op.responses?.['200']?.content));
+    const itemSchema = JsonSchemaObjectSchema.parse(itemsOf(doc, arraySchema));
     expectNullableUuidField(itemSchema, 'parentRevisionId');
   });
 
   it('RevisionWithTrees requires parentRevisionId as a nullable uuid (GET /revisions/{id})', async () => {
     const doc = await loadSpec();
     const op = operation(doc, '/revisions/{id}', 'get');
     const revisionWithTrees = JsonSchemaObjectSchema.parse(
-      dataSchemaOf(jsonSchemaOf(op.responses?.['200']?.content))
+      dataSchemaOf(doc, jsonSchemaOf(op.responses?.['200']?.content))
     );
     expectNullableUuidField(revisionWithTrees, 'parentRevisionId');
   });
@@ -148,12 +163,12 @@ describe('openapi.yaml — package_revisions.base_revision_id (ADR-066 #390)', (
     const branches = z
       .object({ oneOf: z.array(z.unknown()) })
       .parse(jsonSchemaOf(op.requestBody?.content)).oneOf;
-    const structured = branchRequiring(branches, 'type');
+    const structured = branchRequiring(doc, branches, 'type');
     const base = JsonSchemaObjectSchema.parse((structured.properties ?? {})['baseRevisionId']);
     expect(base.type).toBe('string');
     expect(base.format).toBe('uuid');
     expect(structured.required ?? []).not.toContain('baseRevisionId');
-    expect(branchRequiring(branches, 'label').properties ?? {}).not.toHaveProperty(
+    expect(branchRequiring(doc, branches, 'label').properties ?? {}).not.toHaveProperty(
       'baseRevisionId'
     );
   });
@@ -168,9 +183,12 @@ describe('openapi.yaml — package_revisions.base_revision_id (ADR-066 #390)', (
       const doc = await loadSpec();
       const op = operation(doc, path, method);
       const dataSchema = dataSchemaOf(
+        doc,
         jsonSchemaOf(op.responses?.[method === 'post' ? '201' : '200']?.content)
       );
-      const responseSchema = JsonSchemaObjectSchema.parse(list ? itemsOf(dataSchema) : dataSchema);
+      const responseSchema = JsonSchemaObjectSchema.parse(
+        list ? itemsOf(doc, dataSchema) : dataSchema
+      );
       expectNullableUuidField(responseSchema, 'baseRevisionId');
     }
   );

src/db/queries/revisions.ts

@@ -175,11 +175,19 @@ function readinessInputFrom(input: string | CreatePackageRevisionInput): Readine
   return { mode: input.mode, overrideReadinessGate: input.overrideReadinessGate };
 }
 
-export function mapSummary(
+// The fields RevisionSummary and RevisionWithTrees share — everything except RevisionSummary's
+// `specCount` and RevisionWithTrees's `specs`. #649: getPackageRevision used to build its
+// RevisionWithTrees result as `{ ...mapSummary(...), specs }`, which spreads mapSummary's FULL
+// RevisionSummary (specCount included) and appends `specs` — TypeScript's excess-property check
+// never fires on a spread's inferred return type, so `specCount` silently rode along on every real
+// GET /revisions/{id} response despite openapi.yaml never documenting it on RevisionWithTrees. The
+// gap was invisible because that op had no response-schema validation at all until #649 fixed the
+// self-referential-schema stack overflow that had excluded it. Extracting the shared core here
+// makes each caller build the SPECIFIC shape its return type promises, not a superset of it.
+function mapRevisionCore(
   row: RevisionRow,
-  profile: RevisionNomenclatureProfile | null,
-  specCount: number
-): RevisionSummary {
+  profile: RevisionNomenclatureProfile | null
+): Omit<RevisionSummary, 'specCount'> {
   const date = revisionDateString(row.revision_date);
   const attributes = parseAttributes(row.attributes);
   const display = getRevisionDisplayIdentity(
@@ -200,12 +208,19 @@ export function mapSummary(
     number: display.number,
     attributes,
     issuedAt: row.issued_at.toISOString(),
-    specCount,
     parentRevisionId: row.parent_revision_id,
     baseRevisionId: row.base_revision_id,
   };
 }
 
+export function mapSummary(
+  row: RevisionRow,
+  profile: RevisionNomenclatureProfile | null,
+  specCount: number
+): RevisionSummary {
+  return { ...mapRevisionCore(row, profile), specCount };
+}
+
 async function profileForPackage(
   packageId: string,
   db: Queryable
@@ -347,7 +362,7 @@ export async function getPackageRevision(
       position: snap.position,
       tree: validateTree(snap.tree, snap.spec_id),
     }));
-    return { ...mapSummary(row, profile, specs.length), specs };
+    return { ...mapRevisionCore(row, profile), specs };
   } catch (err) {
     if (err instanceof DatabaseError) throw err;
     throw new DatabaseError(`getPackageRevision: query failed for ${revisionId}`, { cause: err });

src/test-utils/contract/schema-refs.ts

@@ -0,0 +1,110 @@
+// src/test-utils/contract/schema-refs.ts
+//
+// #649 — validate-response.ts switched loadSpec() from $RefParser.dereference to .bundle so ajv
+// can compile the self-referential SpecNode/SpecTree response schemas (dereference materializes a
+// real circular JS object for them, which blows ajv's compile-time traversal stack). Bundling keeps
+// every `$ref` as a literal `{ $ref: '#/...' }` pointer instead of inlining it, which shifts the
+// work here: registering documents with ajv under stable ids so those pointers resolve at
+// compile/validate time, and giving callers a way to resolve one level of `$ref` themselves when
+// they need to inspect a component's actual shape (not just validate a body against it). Split out
+// of validate-response.ts (400-line cap) as its own cohesive concern.
+import type { AnySchemaObject } from 'ajv';
+import { markUnevaluatedPropertiesFalse, buildMirrorQualifyRef } from './unevaluated-properties.js';
+import { CHILD_MIRROR_ID, IN_PLACE_MIRROR_ID } from './unevaluated-properties.js';
+import type { OpenApiDoc } from './validate-response.js';
+
+/** The bundled document itself, registered once with ajv so a plain-conformance (assertResponse)
+ * response schema's qualified `$ref` (see {@link qualifyRefs}) can resolve against it at compile
+ * time — the assertResponse counterpart to the two exact-match component mirrors below. */
+export const DOC_SCHEMA_ID = 'https://specr.internal/contract/validate-response/doc';
+
+type ComponentKind = 'schemas' | 'responses' | 'parameters';
+const COMPONENT_REF_PATTERN = /^#\/components\/(schemas|responses|parameters)\/([^/]+)$/;
+
+/** Resolves ONE level of a local `#/components/{schemas,responses,parameters}/Name` pointer
+ * against `doc`, or returns `value` unchanged when it isn't a `$ref` object. Exported for
+ * consumers that need to inspect a component's actual shape directly (rather than validate a real
+ * response body against it) — #649: bundling leaves these as literal `{ $ref }` pointers instead
+ * of dereference's fully-inlined target. Throws on a non-local or dangling ref rather than
+ * returning `undefined`, matching this module's fail-loud posture for a gate that would otherwise
+ * silently stop checking anything. */
+export function resolveIfRef(doc: OpenApiDoc, value: unknown): unknown {
+  if (typeof value !== 'object' || value === null || Array.isArray(value)) return value;
+  const ref = (value as Record<string, unknown>)['$ref'];
+  if (typeof ref !== 'string') return value;
+  const match = COMPONENT_REF_PATTERN.exec(ref);
+  if (match === null) {
+    throw new Error(`resolveIfRef: unsupported $ref "${ref}" — only local #/components/* refs`);
+  }
+  const kind = match[1] as ComponentKind;
+  const name = match[2] as string;
+  const target = doc.components?.[kind]?.[name];
+  if (target === undefined) {
+    throw new Error(`resolveIfRef: openapi.yaml has no components.${kind}.${name} (ref "${ref}")`);
+  }
+  return target;
+}
+
+/** Deep-clones `schema` and rewrites every local `$ref` string (`#/...`) to `${toId}#/...`, so it
+ * resolves against whatever document was registered with ajv under `toId`. Never inlines/resolves
+ * the ref target itself — inlining a self-referential target (e.g. SpecNode) would reproduce the
+ * exact circular-JS-object shape that made ajv's traversal stack overflow before #649. Throws on a
+ * non-local ref instead of silently leaving it unqualified (which would let it accidentally resolve
+ * against the wrong document by URI-shape coincidence). */
+export function qualifyRefs(schema: AnySchemaObject, toId: string): AnySchemaObject {
+  const qualified = qualifyRefValue(schema, toId);
+  // qualifyRefValue is typed `unknown -> unknown` because it recurses over arbitrary schema
+  // values. Narrowing with a real runtime check rather than an `as` assertion: the input is an
+  // object schema and the walk preserves object-ness, so this never fires — but a genuine check
+  // costs nothing here and keeps the boundary free of a cast the repo's conventions reject.
+  if (typeof qualified !== 'object' || qualified === null || Array.isArray(qualified)) {
+    throw new Error('qualifyRefs: expected the qualified result to be an object schema');
+  }
+  return qualified;
+}
+
+function qualifyRefValue(value: unknown, toId: string): unknown {
+  if (Array.isArray(value)) return value.map((item) => qualifyRefValue(item, toId));
+  if (typeof value !== 'object' || value === null) return value;
+  const out: Record<string, unknown> = {};
+  for (const [key, val] of Object.entries(value)) {
+    if (key === '$ref' && typeof val === 'string') {
+      if (!val.startsWith('#/')) {
+        throw new Error(`qualifyRefs: unsupported non-local $ref "${val}"`);
+      }
+      out[key] = `${toId}${val}`;
+    } else {
+      out[key] = qualifyRefValue(val, toId);
+    }
+  }
+  return out;
+}
+
+/** Minimal slice of the ajv instance this module needs — kept narrow so this file doesn't have to
+ * re-declare the CJS-interop typing dance validate-response.ts does for the full instance. */
+export interface AjvSchemaRegistry {
+  addSchema(schema: AnySchemaObject, key: string): unknown;
+}
+
+/** Builds and registers the two exact-match component mirrors (#649): every `components.schemas`
+ * entry marked once as if reached via a CHILD position (properties/items) and once as if reached
+ * via an IN_PLACE position (allOf/oneOf/anyOf branch). Both are built eagerly for every component
+ * name — no dependency worklist needed, since ajv resolves `$ref` lazily by registered id at
+ * compile/validate time, never at registration time. A single "mark once, context-agnostic" mirror
+ * was tried and rejected: at least 12 real components (SuccessResponse, ErrorResponse, and others)
+ * are referenced from BOTH contexts somewhere in openapi.yaml, and marking SuccessResponse
+ * standalone made it reject the sibling `data` branch's own keys — a false rejection of a fully
+ * documented payload, not just a missed detection. */
+export function registerComponentMirrors(ajv: AjvSchemaRegistry, doc: OpenApiDoc): void {
+  const componentSchemas = doc.components?.schemas ?? {};
+  const qualifyRef = buildMirrorQualifyRef();
+  const childMirror: Record<string, AnySchemaObject> = {};
+  const inPlaceMirror: Record<string, AnySchemaObject> = {};
+  for (const [name, componentSchema] of Object.entries(componentSchemas)) {
+    const schema = componentSchema as AnySchemaObject;
+    childMirror[name] = markUnevaluatedPropertiesFalse(schema, { inPlace: false, qualifyRef });
+    inPlaceMirror[name] = markUnevaluatedPropertiesFalse(schema, { inPlace: true, qualifyRef });
+  }
+  ajv.addSchema({ components: { schemas: childMirror } }, CHILD_MIRROR_ID);
+  ajv.addSchema({ components: { schemas: inPlaceMirror } }, IN_PLACE_MIRROR_ID);
+}