ci(spec): add SDK types generation and breaking-change gate - #49
Conversation
- package.json: add openapi-typescript (gen:types script) + js-yaml - generated/api-types.d.ts: deterministic types emitted from openapi.yaml - foundation-gate.yml: new sdk-types job (regenerate + fail-on-diff) and breaking-change job (oasdiff vs base branch, requires 'Breaking: yes' marker in PR body to pass) - CHANGELOG: document both gates Completes the L6 CONTRACT card's remaining levers 2 (drift) and 3 (SDK generation in CI) on the api-spec side.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 51 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (4)
Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_6a21b49e-3f0d-4223-9784-9150a96454ca) |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
PR Summary by QodoAdd CI gates for generated SDK types drift and OpenAPI breaking changes
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
ApprovabilityVerdict: Approved e41c5fd Adds CI quality gates for SDK type drift detection and breaking API change detection. Changes are limited to workflow configuration, .gitignore, and changelog - no production runtime impact. Author owns all modified files. You can customize Macroscope's approvability policy. Learn more. |
Code Review by Qodo
1.
|
Qodo Fixer✅ Merged (0) · ☑ Fixed (0) Process
|
- pin oasdiff to tufin/oasdiff:v1.28.0 by digest (ghcr.io/tufin/oasdiff:latest
does not exist; the image lives on Docker Hub) and handle exit codes
explicitly so infra failures no longer read as contract breaks
- grant the breaking-change job pull-requests: read and fail loudly when the
PR body lookup fails instead of treating it as a missing marker
- pass GitHub context values through env vars instead of inline ${{ }}
interpolation in run blocks
- refresh the stale .gitignore comment (package.json now exists; lockfile
stays uncommitted by convention, determinism via exact pins)
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…ff exit codes - assert generated/api-types.d.ts is tracked and use git add --intent-to-add before diffing, so a PR that deletes the committed artifact fails instead of slipping past the untracked-file blind spot in git diff - record the empirically verified oasdiff v1.28.0 exit-code table (0/1/100/102/125) confirming code 1 is exclusive to breaking changes Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…ripts - spec-lint and sdk-types now run npm install --no-save from package.json instead of inline name@version specs, making the manifest the single source of truth so a version bump cannot silently diverge between CI and local runs - --ignore-scripts keeps floating transitive dependencies (no lockfile by convention) from executing install lifecycle hooks in CI - verified: install, npm run lint (0 errors), assert-refs, and npm run gen:types byte-identical regeneration all pass from the manifest install Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Pins the full transitive dependency tree, closing the gap where exact pins covered only the three direct tools and any transitive package resolved at install time. CI now runs npm ci --ignore-scripts from the committed lockfile, making installs byte-reproducible; the old package-lock.json ignore predated package.json existing at all. Verified npm ci, lint, assert-refs, and byte-identical type regeneration from the lockfile install. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…GITHUB_BASE_REF Addresses review threads on #49: - ghcr.io/tufin/oasdiff:latest -> pinned v1.28.0 release binary with checksums.txt sha256 verification (no floating image tag) - add pull-requests: read to the breaking-change job so gh pr view body reads do not 403 under contents: read-only tokens - origin/${{ base.ref }} -> $GITHUB_BASE_REF (never interpolate a PR-controlled value into a shell command)
| - name: Install tooling | ||
| run: npm install --no-save --no-audit --no-fund openapi-typescript@7.13.0 | ||
| - name: Regenerate types and fail if the committed artifact drifted | ||
| run: | | ||
| npx openapi-typescript openapi.yaml -o generated/api-types.d.ts | ||
| git diff --exit-code -- generated/api-types.d.ts |
There was a problem hiding this comment.
🟡 Newly committed dependency lockfile is never used by the build, so tool versions still float
The build installs its tooling ad hoc instead of from the newly committed exact-version list (npm install --no-save --no-audit --no-fund openapi-typescript@7.13.0 at .github/workflows/foundation-gate.yml:78, and the same pattern at .github/workflows/foundation-gate.yml:51 and .github/workflows/foundation-gate.yml:59), so the reproducibility the change advertises is not actually in effect.
Impact: Indirect dependency versions can change between runs, so the generated-types comparison can fail for reasons unrelated to the specification, and package install hooks still execute.
Incomplete migration to lockfile-based installs
This PR adds package.json (with lint and gen:types scripts) and package-lock.json, and rewrites .gitignore:2-5 to state that "CI installs with npm ci --ignore-scripts". No job in .github/workflows/foundation-gate.yml runs npm ci, and neither npm script is invoked — spec-lint still uses npx --yes @redocly/cli@2.40.0 and npm install --no-save js-yaml@4.1.0, and sdk-types uses npm install --no-save openapi-typescript@7.13.0 plus npx openapi-typescript. Commits fc9bbb0/f4528e6 in this PR introduced npm ci --no-audit --no-fund --ignore-scripts and npm run lint / npm run gen:types; commit e41c5fd reverted those steps while keeping the lockfile and the .gitignore text.
| - name: Install tooling | |
| run: npm install --no-save --no-audit --no-fund openapi-typescript@7.13.0 | |
| - name: Regenerate types and fail if the committed artifact drifted | |
| run: | | |
| npx openapi-typescript openapi.yaml -o generated/api-types.d.ts | |
| git diff --exit-code -- generated/api-types.d.ts | |
| - name: Install pinned tooling from lockfile | |
| # package.json pins the direct tools by exact version and package-lock.json pins the | |
| # full transitive tree, so npm ci is byte-reproducible across CI and local runs. | |
| run: npm ci --no-audit --no-fund --ignore-scripts | |
| - name: Regenerate types and fail if the committed artifact drifted | |
| run: | | |
| npm run gen:types | |
| git diff --exit-code -- generated/api-types.d.ts |
Was this helpful? React with 👍 or 👎 to provide feedback.
| # The spec is the source of truth for every generated SDK and the CLI. These two gates make | ||
| # sure the committed artifact stays honest: | ||
| # - sdk-types: the committed generated/types.d.ts must not drift from openapi.yaml (fail-on-diff) | ||
| # - breaking-change: the PR must not introduce breaking spec changes without an explicit | ||
| # "Breaking: yes" marker in the PR body (oasdiff compares PR head vs base branch) | ||
| sdk-types: | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| steps: | ||
| - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 | ||
| with: | ||
| persist-credentials: false | ||
| - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 | ||
| with: | ||
| node-version: '22' | ||
| - name: Install tooling | ||
| run: npm install --no-save --no-audit --no-fund openapi-typescript@7.13.0 | ||
| - name: Regenerate types and fail if the committed artifact drifted | ||
| run: | | ||
| npx openapi-typescript openapi.yaml -o generated/api-types.d.ts | ||
| git diff --exit-code -- generated/api-types.d.ts | ||
| - name: Confirm the committed artifact is present and non-empty | ||
| run: test -s generated/api-types.d.ts |
There was a problem hiding this comment.
🔍 Final commit reverted three earlier review hardenings
Commit e41c5fd (head) reverted changes made by fc9bbb0, f4528e6, and 16e49cc in this same PR: the npm ci --ignore-scripts installs, the npm-script invocations, the deletion-proof drift check, and the oasdiff exit-code table handling. The commit message only mentions the oasdiff binary pinning, the pull-requests: read grant, and the GITHUB_BASE_REF substitution — the other reverts look like a bad rebase/force-push rather than a deliberate decision. Worth confirming with the author before merge (the individual regressions are reported separately as bugs).
Was this helpful? React with 👍 or 👎 to provide feedback.
| run: | | ||
| mkdir -p /tmp/oasdiff-install | ||
| curl -fsSL -o /tmp/oasdiff-install/oasdiff_1.28.0_linux_amd64.tar.gz \ | ||
| "https://github.com/oasdiff/oasdiff/releases/download/v1.28.0/oasdiff_1.28.0_linux_amd64.tar.gz" | ||
| curl -fsSL -o /tmp/oasdiff-install/checksums.txt \ | ||
| "https://github.com/oasdiff/oasdiff/releases/download/v1.28.0/checksums.txt" | ||
| (cd /tmp/oasdiff-install && grep "oasdiff_1.28.0_linux_amd64.tar.gz" checksums.txt | sha256sum -c -) | ||
| tar -xzf /tmp/oasdiff-install/oasdiff_1.28.0_linux_amd64.tar.gz -C /usr/local/bin | ||
| oasdiff version |
There was a problem hiding this comment.
🟨 oasdiff download runs unpinned tarball extraction into PATH with only a self-hosted checksum
The breaking-change job downloads an oasdiff release tarball and its checksums.txt from the same GitHub release and verifies one against the other (.github/workflows/foundation-gate.yml:109-117). Because both the artifact and the checksum come from the same mutable source, the verification only detects transport corruption, not a tampered/re-published release. The extracted contents are unpacked wholesale into /usr/local/bin, which is on PATH for all later steps of the job.
Was this helpful? React with 👍 or 👎 to provide feedback.
| - name: Install tooling | ||
| run: npm install --no-save --no-audit --no-fund openapi-typescript@7.13.0 |
There was a problem hiding this comment.
🟨 Spec tooling is installed without the lockfile and with install scripts enabled
The CI jobs install tooling with npx --yes @redocly/cli@2.40.0 and npm install --no-save ... (.github/workflows/foundation-gate.yml:51, .github/workflows/foundation-gate.yml:59, .github/workflows/foundation-gate.yml:78) even though this PR commits a package-lock.json and .gitignore:2-5 states CI installs with npm ci --ignore-scripts. Transitive dependencies therefore resolve freshly on every run and package lifecycle install hooks execute in the CI environment.
Was this helpful? React with 👍 or 👎 to provide feedback.


Completes the L6 CONTRACT card levers 2+3 (api-spec side).
npm ci --ignore-scripts(lockfile added during review to pin transitives).git diff --exit-codefails on drift + non-empty artifact check.oasdiff breaking --fail-on ERR(verified: exit 0 on no breaking, exit 1 on breaking) vs PR spec via ghcr.io/tufin/oasdiff; fails unless PR body carries explicitBreaking: yesmarker.Not changing openapi.yaml, so the breaking gate is green (verified exit 0 against origin/main).
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.Note
Cursor Bugbot is generating a summary for commit 237334c. Configure here.
Note
Add SDK types generation and breaking-change gate to CI
sdk-typesCI job that regenerates TypeScript types fromopenapi.yamlusing pinnedopenapi-typescriptand fails if the result differs from the committed generated/api-types.d.ts.breaking-changeCI job that diffsopenapi.yamlagainst the PR base branch using pinnedoasdiffv1.28.0 and fails unless the PR body contains the markerBreaking: yes.lintandgen:typesnpm scripts and pins devDependencies; commits package-lock.json for reproducible installs (.gitignoreupdated accordingly).Macroscope summarized e41c5fd.