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

[thewrz]

Why

The OpenAPI contract test suite (src/api/contract.integration.test.ts) had six response schemas that were never actually validated against assertResponseExact — a sweep bug meant they silently passed regardless of shape drift. Issue #649 asked us to close that gap and prove the fix with tests that fail red on the pre-fix code.

What

• Validates the six previously-unvalidated response schemas against real route responses.
• Switches the contract test's OpenAPI loading from $RefParser.dereference to $RefParser.bundle, and updates resolveResponseSchema in test-utils to resolve local $ref pointers explicitly, since bundling keeps $refs instead of inlining them.
• Drops the undocumented specCount field from the GET /revisions/{id} response documentation (cross-fix surfaced while auditing schema exactness).
• Adds regression tests pinning: sweep-vacuity and per-op depth invariants, local $ref context resolution across six ops, and a no-mutation invariant for the qualifyRef/in-place $ref-qualification path.

Design decisions

• Dual-mirror necessity confirmed via a real 12-component both-context survey. Before committing to bundle-over-dereference, we surveyed all 12 schema components that appear in both request and response contexts to confirm a single shared mirror could not silently pass a case where request and response actually diverge — the survey showed dereference's inlining was masking exactly this class of bug for the six previously-unvalidated ops.
• Bundle over dereference. $RefParser.bundle() preserves $ref pointers (resolving only across files, not inline), so the contract test now walks the same schema shape the OpenAPI document itself describes, rather than a fully-inlined copy that can drift from what client tooling actually sees. resolveResponseSchema was extended to resolve local (#/...) pointers explicitly against the bundled document.
• The components-retention gap. Bundling — unlike dereferencing — retains the components section as live, referenced structure. We had to make sure the sweep-vacuity check still walks all reachable schemas via $ref, not just top-level paths, or the six ops would have gone right back to being invisible to the checker under the new loading strategy.
• Mutation-verify coincidence risk found and documented. While pinning the no-mutation invariant for the qualifyRef/in-place path, we found that a naive before/after deep-equal check on the return value could pass even if the input object were mutated in place, because the function also returns the same reference — a coincidental green. The regression test instead snapshots the input independently before the call and diffs it against the post-call input, not the return value, so it can't pass by that coincidence.
• No ADR filed — explicit sprint override (test-only + one-field API-doc correction, no architectural decision).

Verification transcript (mutation-verified — #649's mandated bar)

The risk this PR had to disprove: bundling leaves $ref pointers in place, and
unevaluated-properties.ts previously assumed they were already inlined. If the walker had
silently stopped traversing at a $ref, INV-6's exact-key-match gate would have degraded to a
no-op for exactly the six operations this issue exists to close — and the suite would still be
green. Every claim below was produced by running the mutation, not by inspection.

1. Walker applicator lists — remove a keyword, confirm a case goes red

Each keyword deleted independently from a pristine copy, then
vitest run --project unit src/test-utils/contract/validate-response.test.ts:

Mutation	Result
CHILD_SINGLES − items	2 failed | 71 passed
CHILD_SINGLES − contains	1 failed | 72 passed
CHILD_SINGLES − additionalProperties	1 failed | 72 passed
CHILD_SINGLES − unevaluatedProperties	73 passed — SURVIVING MUTANT
CHILD_SINGLES − unevaluatedItems	1 failed | 72 passed
IN_PLACE_MAPS − dependentSchemas	1 failed | 72 passed
CHILD_MAPS − patternProperties	1 failed | 72 passed

The sweep found one real gap: unevaluatedProperties was unpinned — that traversal edge could
have been deleted with nothing going red. Fixed in 08bb5454, which adds the object-side twin of
the existing unevaluatedItems case. Re-verified red/green: 74/74 passed on current code, and
with the keyword removed that exact case is the single failure (1 failed | 73 passed).

2. The $ref walk is load-bearing, not a no-op

Bundled $refs are handled by rewriting the pointer to one of two pre-marked component mirrors
rather than by recursing (which is what avoids materializing SpecNode's circular object). To prove
those mirrors actually carry the marking, registerComponentMirrors was mutated to register the
components unmarked:

2 failed | 20 passed (22)
  × checkpoint, pending-summary, and reject responses match their documented schemas end to end
  × GET /revisions/{id} matches its documented schema exactly, including its embedded SpecTree

Those two blocks hold all six operations' exact-match assertions. The gate goes red when the walk
goes blind — it is not passing vacuously.

3. The adapted $ref-resolving tests are not vacuous

resolveIfRef mutated to an identity function (never resolves a pointer):
28 failed | 2 passed (30) across path-id-status-openapi.test.ts and
revision-parent-openapi.test.ts (baseline: 30/30 passed).

4. All six operations genuinely validate — nothing exempted

Each of the six carries, against a real driven response body: assertResponse +
assertResponseExact + a top-level spliced undocumented key + a key spliced one level down inside
data.children (the rogueChild helper), both asserted .rejects.toThrow(/does not document/).
The nested splice is what proves the recursive mirror walk reaches past data rather than stopping
there.

Bucket accounting confirms burn-down rather than exemption:

• RESPONSE_COVERED 47 → 53 (+6), RESPONSE_ALLOWLIST 91 → 85 (−6) — all six moved from
  the not-yet-verified allowlist into the asserted set.
• src/mcp/contract-write-response.integration.test.ts — which owns INV6_WRITE_EXEMPT and
  INV6_WRITE_PENDING — is untouched by this PR. Nothing was added to any exemption, pending,
  or allowlist bucket.

5. Full local run under bundling

pnpm lint clean · pnpm test 3657 passed (255 files) · pnpm test:integration
1812 passed, 141 skipped (170 files). The #640 non-vacuity sweep and INV-1/2/3/5/6 all still
hold under bundle.

Testing

• Unit tests pass — pnpm test (255 files, 3657 tests, all green)
• Integration tests pass — pnpm test:integration 1812 passed / 141 skipped (170 files), run locally against this branch
• Manual verification: n/a (contract-test-only change)
• CI green — all 6 checks passing on 08bb545

🤖 Co-authored by Claude Sonnet 5. Closes #649

Summary by CodeRabbit

• Bug Fixes

  • Corrected revision response data so summary and detailed revision views return the appropriate fields.
  • Improved handling of referenced API response definitions, including error responses and revision-related requests.

• Tests

  • Expanded API contract coverage for recursive specification data, exact responses, nested fields, and undocumented properties.
  • Strengthened validation of request and response schemas to catch inconsistencies more reliably.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3b21366d-c963-464e-ad16-ff6cefdc4733

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Changes

Recursive contract validation

Layer / File(s)	Summary
Schema reference walking src/test-utils/contract/schema-refs.ts, src/test-utils/contract/unevaluated-properties.ts	Bundled $ref values now use context-specific mirrors during recursive schema traversal.
Validator reference integration src/test-utils/contract/validate-response.ts, src/test-utils/contract/validate-response.test.ts	The validator bundles and registers OpenAPI schemas, resolves referenced branches, and validates exact recursive responses.
OpenAPI contract consumers src/api/path-id-status-openapi.test.ts, src/api/revision-parent-openapi.test.ts	Error, revision, and request-schema assertions resolve $ref values before inspection.
Recursive response coverage src/api/contract.integration.test.ts, src/db/queries/revisions.ts	Integration tests cover six recursive response operations and nested undocumented-key rejection. Revision mapping separates shared fields from specCount.

Sequence Diagram(s)

sequenceDiagram
  participant API as API operation
  participant Contract as Contract validator
  participant OpenAPI as Bundled OpenAPI document
  participant AJV
  API->>Contract: submit response
  Contract->>OpenAPI: resolve response and schema references
  Contract->>AJV: compile recursive schema
  AJV-->>Contract: validate exact response
  Contract-->>API: accept or reject response

Loading

Possibly related PRs

• wrzonance/SpecR#645: Earlier exact response-key validation changes in the same contract utilities.
• wrzonance/SpecR#586: Related OpenAPI response validation changes in the path-status contract tests.
• wrzonance/SpecR#542: Related recursive revision-tree response schema changes.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main changes: validating six response schemas and bundling the OpenAPI document.
Linked Issues check	✅ Passed	The changes satisfy #649 by bundling schemas, supporting recursive references, validating all six operations, and preserving exact-key and non-vacuity checks.
Out of Scope Changes check	✅ Passed	The changes support #649, including required validator, schema-walker, OpenAPI test, and revision-response updates; no unrelated scope is evident.

---

_{Comment @coderabbitai help to get the list of available commands.}

[thewrz]

This was written agentically; verify its assertions and edit accordingly:

Adversarial cross-review — Codex gpt-5.6-sol (reasoning effort xhigh)

Ran once as the final draft-phase gate, against origin/main, after CI went green on 08bb5454.

Result: 0 actionable findings.

The bundled-schema registration and dual-context mirror handling correctly preserve recursive
$ref validation, while the revision response now matches its documented shape. No actionable
regressions were found.

Because a sibling PR in this sprint reported a false "0 findings" while the reviewer had actually
produced real P2s, this zero was verified against the raw transcript rather than taken from the
summary line: the run is 603,773 bytes, exits 0, contains a terminal verdict block, and carries
zero [P1]/[P2] tags and zero severity-graded findings anywhere in the transcript. The
reviewer's own checklist confirms it completed "trace affected call sites and tests for concrete
regressions" before concluding.

Independent verification run alongside the review

The review was not the only gate. The specific risk this PR carries — that switching loadSpec from
dereference to bundle leaves $ref pointers the exact-key-match walker might silently stop at,
degrading INV-6 to a no-op while staying green — was mutation-tested rather than reasoned about.
Full transcript is in the PR body; the load-bearing results:

• One real gap found and fixed. Deleting unevaluatedProperties from the walker's
  CHILD_SINGLES left the entire suite green — a surviving mutant, i.e. an unpinned traversal edge.
  Fixed in 08bb5454; re-verified red/green (74/74 green; with the keyword removed that one case is
  the sole failure).
• The $ref path is not a no-op. Registering the component mirrors unmarked turns both
  exact-match blocks red, so the six operations' gates genuinely gate.
• Nothing was exempted to make the suite pass. RESPONSE_COVERED 47→53, RESPONSE_ALLOWLIST
  91→85 — a straight burn-down — and the file owning INV6_WRITE_EXEMPT/INV6_WRITE_PENDING is
  untouched by this PR.

No ADR was added or amended (git diff origin/main...HEAD -- docs/adr/ is empty); rationale lives in
the PR body and in code comments/test names per this sprint's instruction.

🤖 Co-authored by Claude Opus 5 (1M context).

[thewrz]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

🧹 Nitpick comments (5)

src/db/queries/revisions.ts (1)

178-190: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider declaring the shared core as its own interface.

Omit<RevisionSummary, 'specCount'> expresses the shared shape indirectly. It is correct today, because RevisionSummary minus specCount equals RevisionWithTrees minus specs. That equality is not stated anywhere, so a future field added to only one of the two interfaces makes the return type misleading before it makes it a compile error.

An explicit RevisionCore interface that both RevisionSummary and RevisionWithTrees extend would make the relationship declarative and keep mapRevisionCore's return type stable.

♻️ Proposed shape

+export interface RevisionCore {
+  readonly revisionId: string;
+  readonly packageId: string;
+  readonly label: string;
+  readonly displayName: string;
+  readonly type: string;
+  readonly date: string;
+  readonly sortOrder: number;
+  readonly number: string | null;
+  readonly attributes: RevisionAttributes;
+  readonly issuedAt: string;
+  readonly parentRevisionId: string | null;
+  readonly baseRevisionId: string | null;
+}
+
-export interface RevisionSummary {
-  readonly revisionId: string;
-  // ... 12 shared fields ...
+export interface RevisionSummary extends RevisionCore {
   readonly specCount: number;
 }
 
-export interface RevisionWithTrees {
-  readonly revisionId: string;
-  // ... 12 shared fields ...
+export interface RevisionWithTrees extends RevisionCore {
   readonly specs: readonly RevisionSpecEntry[];
 }

Then mapRevisionCore returns RevisionCore.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/db/queries/revisions.ts` around lines 178 - 190, Declare a shared
RevisionCore interface for the fields common to RevisionSummary and
RevisionWithTrees, and update mapRevisionCore to return RevisionCore instead of
Omit<RevisionSummary, 'specCount'>. Adjust RevisionSummary and RevisionWithTrees
to extend RevisionCore so the relationship is explicit and the shared-core shape
stays stable if either type changes.

src/api/contract.integration.test.ts (1)

846-861: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Move the package creation and the project_specs INSERT inside the try block.

Both setup statements run before try. If the project_specs INSERT at Lines 857-860 fails, the finally block never runs, so the created design_packages row survives the test. That orphan row can then make the outer afterAll cleanup fail with a project_specs RESTRICT violation, which masks the original failure.

Enter the try immediately after baseUrl is known, and keep the existing finally cleanup. The cleanup already tolerates missing rows, because both statements are unconditional DELETEs.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/api/contract.integration.test.ts` around lines 846 - 861, Move the
package creation in the contract integration test into the existing try/finally
flow so both the POST to /projects/${projectId}/packages and the project_specs
INSERT execute after the try starts, using the test block around packageId and
setPackageSpecs setup as the anchor. Keep the current finally cleanup unchanged
so any failure during pkg creation or the INSERT still triggers the DELETE
cleanup and prevents orphaned design_packages rows from leaking into afterAll.

src/test-utils/contract/schema-refs.ts (1)

54-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the as AnySchemaObject assertion at this boundary.

qualifyRefValue returns unknown, so line 55 asserts over unknown at a public export. The sibling markUnevaluatedPropertiesFalse in unevaluated-properties.ts (line 238) documents the opposite approach: it types the inner helper so the return type is proven, not asserted. Add a small object-only helper and let the type follow.

♻️ Proposed refactor

 export function qualifyRefs(schema: AnySchemaObject, toId: string): AnySchemaObject {
-  return qualifyRefValue(schema, toId) as AnySchemaObject;
+  return qualifyRefObject(schema, toId);
 }
 
+function qualifyRefObject(value: object, toId: string): Record<string, unknown> {
+  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;
+}
+
 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;
+  return qualifyRefObject(value, toId);
 }

As per coding guidelines: "Do not use any, as unknown as, cross-boundary type assertions, or non-null assertions (!) outside tests."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/test-utils/contract/schema-refs.ts` around lines 54 - 56, Update
qualifyRefs and its qualifyRefValue helper to eliminate the public-boundary as
AnySchemaObject assertion. Add a small object-only helper, following
markUnevaluatedPropertiesFalse, that proves and returns the AnySchemaObject type
before qualifyRefs returns it; do not replace the assertion with any, as unknown
as, or a non-null assertion.

Source: Coding guidelines

src/test-utils/contract/validate-response.ts (1)

343-352: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value

Consider a per-branch guard so a partially-derivable oneOf cannot weaken INV-4 silently.

The fail-loud guard at lines 392-398 fires only when the derived key set is completely empty. A oneOf branch that resolves to a component composed with allOf (or a nested oneOf) parses cleanly into RequestBodyBranchObject, yields properties: undefined, and contributes zero keys. If the other branches still contribute keys, body.size stays non-zero and the guard never fires. INV-4 then checks a partial key set with no signal.

No current operation reaches this state: the request bodies named in the comment above are all plain properties-bearing objects. The gap is forward-looking, and it is the same vacuity class this reader already guards against one level up.

🛡️ Proposed per-branch guard

   const keys = new Set(Object.keys(schema.properties ?? {}));
   for (const rawBranch of schema.oneOf ?? []) {
     const branch = RequestBodyBranchObject.parse(resolveIfRef(doc, rawBranch));
+    if (branch.properties === undefined) {
+      throw new Error(
+        'a requestBody `oneOf` branch declares no top-level `properties` — an unsupported ' +
+          'composition (allOf, nested oneOf, non-object branch) would contribute zero keys and ' +
+          'weaken INV-4 without emptying the key set. Extend bodyPropertyKeys() to handle it.'
+      );
+    }
     for (const key of Object.keys(branch.properties ?? {})) keys.add(key);
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/test-utils/contract/validate-response.ts` around lines 343 - 352, Update
bodyPropertyKeys to detect any oneOf branch that cannot yield properties,
including branches resolved through allOf or nested oneOf, and fail loudly
instead of silently adding only keys from derivable branches. Preserve the
existing top-level empty-key guard while ensuring partially derivable oneOf
schemas cannot produce a partial key set.

src/test-utils/contract/unevaluated-properties.ts (1)

207-211: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Align the comment with the code: $ref nodes are not skipped.

The comment says the walker rewrites the pointer "and stop". The code does not return early. It falls through to walkSubschemas and shouldMark. For a pointer-only node both are effectively no-ops, so the behavior matches the intent. For a $ref node that carries sibling applicators — legal and evaluated in JSON Schema 2020-12 / OpenAPI 3.1, for example { $ref: '…/Foo', properties: { extra: … } } — the walker correctly descends into the siblings and can mark the node. The comment reads as if an early return were the contract, which could invite a future change that silently drops those sibling applicators.

📝 Proposed comment change

-  // A bundled `$ref` pointer: rewrite it to the mirror-qualified id for THIS local context and stop
-  // — it has no other subschema-bearing keywords worth walking (siblings like `description` are
-  // plain data), and `shouldMark` below naturally leaves it unmarked since it evaluates no
-  // properties of its own (see the `$ref` note in the applicator-classification comment).
+  // A bundled `$ref` pointer: rewrite it to the mirror-qualified id for THIS local context. The
+  // walk below deliberately CONTINUES rather than returning early — `$ref` siblings are legal and
+  // evaluated in 2020-12, so a `{ $ref, properties }` node's own applicators must still be walked
+  // and marked. For a pointer-only node both `walkSubschemas` and `shouldMark` are no-ops, since it
+  // evaluates no properties of its own (see the `$ref` note in the applicator-classification
+  // comment). Do not add an early `return` here: it would silently drop sibling applicators.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/test-utils/contract/unevaluated-properties.ts` around lines 207 - 211,
Update the comment above the `$ref` handling in the schema walker to accurately
state that the pointer is rewritten before normal traversal continues, rather
than claiming the walker stops. Preserve traversal through sibling applicators
and the existing `walkSubschemas` and `shouldMark` behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/api/contract.integration.test.ts`:
- Around line 846-861: Move the package creation in the contract integration
test into the existing try/finally flow so both the POST to
/projects/${projectId}/packages and the project_specs INSERT execute after the
try starts, using the test block around packageId and setPackageSpecs setup as
the anchor. Keep the current finally cleanup unchanged so any failure during pkg
creation or the INSERT still triggers the DELETE cleanup and prevents orphaned
design_packages rows from leaking into afterAll.

In `@src/db/queries/revisions.ts`:
- Around line 178-190: Declare a shared RevisionCore interface for the fields
common to RevisionSummary and RevisionWithTrees, and update mapRevisionCore to
return RevisionCore instead of Omit<RevisionSummary, 'specCount'>. Adjust
RevisionSummary and RevisionWithTrees to extend RevisionCore so the relationship
is explicit and the shared-core shape stays stable if either type changes.

In `@src/test-utils/contract/schema-refs.ts`:
- Around line 54-56: Update qualifyRefs and its qualifyRefValue helper to
eliminate the public-boundary as AnySchemaObject assertion. Add a small
object-only helper, following markUnevaluatedPropertiesFalse, that proves and
returns the AnySchemaObject type before qualifyRefs returns it; do not replace
the assertion with any, as unknown as, or a non-null assertion.

In `@src/test-utils/contract/unevaluated-properties.ts`:
- Around line 207-211: Update the comment above the `$ref` handling in the
schema walker to accurately state that the pointer is rewritten before normal
traversal continues, rather than claiming the walker stops. Preserve traversal
through sibling applicators and the existing `walkSubschemas` and `shouldMark`
behavior.

In `@src/test-utils/contract/validate-response.ts`:
- Around line 343-352: Update bodyPropertyKeys to detect any oneOf branch that
cannot yield properties, including branches resolved through allOf or nested
oneOf, and fail loudly instead of silently adding only keys from derivable
branches. Preserve the existing top-level empty-key guard while ensuring
partially derivable oneOf schemas cannot produce a partial key set.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5fa5211d-b22d-4f05-b74b-8e02a0036ee8

📥 Commits

Reviewing files that changed from the base of the PR and between 14f06e3 and 08bb545.

📒 Files selected for processing (8)

• src/api/contract.integration.test.ts
• src/api/path-id-status-openapi.test.ts
• src/api/revision-parent-openapi.test.ts
• src/db/queries/revisions.ts
• src/test-utils/contract/schema-refs.ts
• src/test-utils/contract/unevaluated-properties.ts
• src/test-utils/contract/validate-response.test.ts
• src/test-utils/contract/validate-response.ts

[thewrz]

All five nitpicks handled in 9857806d — 4 fixed, 1 declined. Body-level nitpicks, so there are no threads to resolve.

1. validate-response.ts — per-branch INV-4 guard · fixed

The most substantive of the five, and it earns more than its 🔵 Trivial label: this is the vacuous-gate class inside the gate that exists to prevent vacuous gates.

Verified reachable rather than assumed. RequestBodyBranchObject is a plain z.object, not .strict(), so an allOf/nested-oneOf branch parses cleanly and yields properties: undefined instead of throwing. The whole-body guard in operationParamKeys fires only on body.size === 0, so any union with one derivable branch keeps the set non-empty and hides the rest — INV-4 then compares a partial key set with no signal.

Keyed the check on "could not derive" (properties === undefined), never on "derived nothing": an explicit properties: {} is a legal, genuinely-empty branch and must still pass. Both directions pinned by new tests.

Mutation-verified, because a guard against vacuity that is itself vacuous would be the whole joke:

× throws when only SOME oneOf branches are derivable, rather than checking a partial key set
AssertionError: expected [Function] to throw an error
Test Files  1 failed (1) | Tests  1 failed | 75 passed (76)

Reverted → 76/76.

2. contract.integration.test.ts — fixture leak · fixed

Real, and it compounds. The package POST ran before the try, and the package is created first — so an INSERT failure (a re-run where project_specs already has the row) skipped the finally entirely and orphaned the design_packages row just created. Both statements now sit inside the try, with the finally guarding on packageId (undefined only when the POST itself threw, when there is nothing to delete). The project_specs delete stays unconditional — id-scoped and a no-op if the INSERT never landed.

3. unevaluated-properties.ts — comment drift · fixed

Confirmed: the comment said the $ref branch rewrites "and stop", but there is no early return and the walk continues. Behaviourally harmless — a $ref node carries no other subschema-bearing keywords — but it described control flow that does not exist.

4. schema-refs.ts — as AnySchemaObject · fixed

Replaced with a real runtime narrowing check rather than a cast, per this repo's no-cross-boundary-assertions rule. tsc --noEmit accepts it without the assertion.

5. revisions.ts — extract RevisionCore · declined, with reasons

The stated risk does not hold. It argues a future field added to one interface "makes the return type misleading before it makes it a compile error" — but both directions fail immediately:

• a new RevisionSummary field lands inside Omit<RevisionSummary,'specCount'>, so mapRevisionCore must return it → compile error;
• a new RevisionWithTrees field breaks { ...mapRevisionCore(...), specs } at the construction site → compile error.

The claim that the equality "is not stated anywhere" is also inaccurate — the 10-line comment directly above states it, including why the core was extracted (the specCount leak this PR fixes). Adding a third interface would restate in types what is already enforced by Omit<> and explained in prose, for no behavioural gain, in a non-test file. Happy to revisit if you'd rather have it declarative.

Verified: pnpm lint clean, 3659/3659 unit, contract gate 22/22 against real Postgres.