Skip to content

fix(ci): openapi generator merge-preserves hand-authored paths#4795

Closed
bokelley wants to merge 2 commits into
mainfrom
fix/openapi-freshness-hand-edited-paths
Closed

fix(ci): openapi generator merge-preserves hand-authored paths#4795
bokelley wants to merge 2 commits into
mainfrom
fix/openapi-freshness-hand-edited-paths

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Why

The TypeScript Build CI step has been red on main since #4771 merged. That PR added 685 lines of brand-registry endpoint docs directly to `static/openapi/registry.yaml` because the routes are docs-only — they exist in Express but were never given Zod schemas. The freshness lint regenerates the yaml from Zod and `git diff --exit-code`s, so the hand-authored content gets clobbered on every regen and fails.

Verified currently failing on main:

```
gh api repos/adcontextprotocol/adcp/commits/main/check-runs --jq '.check_runs[] | select(.name | contains("TypeScript Build")) | .conclusion'

→ failure

```

Every open PR carries the inherited red regardless of whether it touched openapi at all.

The fix

Generator now merge-preserves hand-authored content:

  1. Read existing `registry.yaml` from disk before emit.
  2. For each path / component schema / tag in the existing yaml that's NOT in the generator's tracked output, copy it forward.
  3. Generator wins on conflicts so Zod-backed paths remain authoritative.

This means docs teams can keep landing pure-yaml PRs (the brand-registry case, and any future docs-only surfaces) without first wiring Zod schemas, AND the freshness lint stops blocking unrelated PRs.

Also moved `Brand Logos` and `Brand Wiki` tag descriptions into `TAG_DESCRIPTIONS` so they emit between `Brand Resolution` and `Property Resolution` (their documented position) rather than appended at the tag list's tail.

Why not write Zod schemas for the brand-registry surface?

That's the structurally correct fix and worth doing eventually. But it's 9 operations with multipart upload + 4 error variants each + moderator queue + ownership endpoint — real engineering, separate scope. Filing this PR unblocks repo-wide CI today.

Test plan

  • WORKOS_API_KEY=test_dummy WORKOS_CLIENT_ID=client_test npm run build:openapi runs cleanly
  • git diff --exit-code static/openapi/registry.yaml clean after the regen
  • Brand-registry paths still present in regenerated yaml (preserved by the merge)
  • Tag order matches TAG_DESCRIPTIONS (Brand Logos / Brand Wiki between Brand Resolution and Property Resolution)
  • npm run test:schemas / npm run test:json-schema clean (this PR only touches the generator and registry.yaml)

Followup

A separate issue should track wiring real Zod schemas for the brand-registry surface so they're code-authoritative and inspectable. Not blocking this fix.

🤖 Generated with Claude Code

bokelley and others added 2 commits May 19, 2026 09:07
The TypeScript Build CI step regenerates static/openapi/registry.yaml
from Zod source and fails on any git diff. PR #4771 added 685 lines of
brand-registry endpoint documentation directly to registry.yaml because
those routes (brand.json, brand-logos, brand-ownership, brand-wiki) are
docs-only — the Express routes exist but were never given Zod schemas.
That hand-edit fails the freshness lint on every PR, and main itself is
currently red on the same step.

Two ways to fix: (a) write Zod schemas for the 7 brand-registry paths
(real engineering, ~9 operations with multipart upload + full error
matrices), or (b) make the generator merge-preserve hand-authored
content. This PR takes (b) — generator now reads the on-disk yaml and
unions its tracked output with anything already there. Paths, component
schemas, and tag descriptors are all preserved when the generator
doesn't own them; generator output wins on conflicts so Zod-backed
paths remain authoritative.

Brand Logos and Brand Wiki tag descriptions moved into TAG_DESCRIPTIONS
so they emit in their documented position (between Brand Resolution and
Property Resolution) rather than appended at the tag list's tail.

registry.yaml carries a 2-line whitespace normalization from YAML re-
serialization — quoted strings collapse to unquoted where YAML permits,
semantically identical.

After this PR: build:openapi && git diff --exit-code is clean on main
and the lint stops blocking PRs that haven't touched openapi at all.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeQL flagged scripts/generate-openapi.ts:128 (existsSync followed by
readFileSync). The file could in principle change between the check and
the read. Trivial in this generator's actual environment, but the fix
is a single-syscall pattern: readFileSync directly, treat ENOENT as
"no prior yaml," rethrow anything else.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley

Copy link
Copy Markdown
Contributor Author

Closing — superseded by #4793 which took the policy answer (hand-edited yaml entries are not supported; the path is Zod schemas, not workarounds in the generator).

This PR would have made the freshness lint tolerate hand-edited paths by merge-preserving them on regen. That's defensively useful, but it also re-enables the exact pattern #4793 deliberately closed: it lets future contributors bypass writing Zod schemas for new routes, which is the right long-term answer.

The TOCTOU fix on existsSync/readFileSync in this PR is moot too, since the merge-preserve code that introduced the race is being dropped.

If a future need for yaml-first docs surfaces, it should come with that policy conversation rather than silently being unblocked by this PR's machinery.

@bokelley bokelley closed this May 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant