-
Notifications
You must be signed in to change notification settings - Fork 0
ci(spec): add SDK types generation and breaking-change gate #49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
237334c
6438301
16e49cc
fc9bbb0
f4528e6
e41c5fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -58,3 +58,81 @@ jobs: | |||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||
| npm install --no-save --no-audit --no-fund js-yaml@4.1.0 | ||||||||||||||||||||||||||||||
| node .github/scripts/assert-refs.mjs openapi.yaml | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # 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 | ||||||||||||||||||||||||||||||
|
Comment on lines
+77
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 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 ( 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 installsThis PR adds
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||||||||||||
| - name: Confirm the committed artifact is present and non-empty | ||||||||||||||||||||||||||||||
| run: test -s generated/api-types.d.ts | ||||||||||||||||||||||||||||||
|
devin-ai-integration[bot] marked this conversation as resolved.
Comment on lines
+62
to
+84
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 Final commit reverted three earlier review hardenings Commit Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| breaking-change: | ||||||||||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||
| timeout-minutes: 10 | ||||||||||||||||||||||||||||||
| # Only PRs can break the contract; the base branch is the comparison target. | ||||||||||||||||||||||||||||||
| if: github.event_name == 'pull_request' | ||||||||||||||||||||||||||||||
| permissions: | ||||||||||||||||||||||||||||||
| contents: read | ||||||||||||||||||||||||||||||
| pull-requests: read | ||||||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||||||
| - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 | ||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||
| persist-credentials: false | ||||||||||||||||||||||||||||||
| fetch-depth: 0 | ||||||||||||||||||||||||||||||
| - name: Extract base branch spec | ||||||||||||||||||||||||||||||
| # $GITHUB_BASE_REF is GitHub-set for pull_request events; never interpolate a | ||||||||||||||||||||||||||||||
| # PR-controlled value into a shell command (template expansion would be injectable). | ||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||
| mkdir -p /tmp/base | ||||||||||||||||||||||||||||||
| git show "origin/$GITHUB_BASE_REF:openapi.yaml" > /tmp/base/openapi.yaml | ||||||||||||||||||||||||||||||
| test -s /tmp/base/openapi.yaml | ||||||||||||||||||||||||||||||
|
devin-ai-integration[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||||||
| - name: Install pinned oasdiff (checksum-verified, no floating image tag) | ||||||||||||||||||||||||||||||
| # oasdiff v1.28.0 (2026-08-06) — a pinned release binary, not a mutable :latest image. | ||||||||||||||||||||||||||||||
| # The release's own checksums.txt is fetched and verified (sha256). | ||||||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||||||
|
Comment on lines
+109
to
+117
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟨 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 Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||||||||||||
| - name: Fail on unacknowledged breaking changes | ||||||||||||||||||||||||||||||
| # oasdiff breaking --fail-on ERR exits 1 when the PR's openapi.yaml has breaking | ||||||||||||||||||||||||||||||
| # changes vs the base branch (removed/renamed paths, removed required params, etc). | ||||||||||||||||||||||||||||||
| # Breaking changes are allowed only with an explicit "Breaking: yes" marker in the | ||||||||||||||||||||||||||||||
| # PR body — that is a deliberate, acknowledged contract break (version bump + notice). | ||||||||||||||||||||||||||||||
| env: | ||||||||||||||||||||||||||||||
| GH_TOKEN: ${{ github.token }} | ||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||
| if oasdiff breaking --fail-on ERR /tmp/base/openapi.yaml "$PWD/openapi.yaml" > /tmp/oasdiff.txt 2>&1; then | ||||||||||||||||||||||||||||||
| echo "No breaking spec changes." | ||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||
| if gh pr view "${{ github.event.pull_request.number }}" --repo "${{ github.repository }}" \ | ||||||||||||||||||||||||||||||
| --json body --jq '.body' | grep -q "Breaking: yes"; then | ||||||||||||||||||||||||||||||
| echo "::warning::Breaking spec changes acknowledged via 'Breaking: yes' marker." | ||||||||||||||||||||||||||||||
| cat /tmp/oasdiff.txt | ||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||
| echo "::error::Breaking spec changes detected. Acknowledge them with 'Breaking: yes' in the PR body." | ||||||||||||||||||||||||||||||
| cat /tmp/oasdiff.txt | ||||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,8 @@ | ||
| # The spec-lint gate runs `npm install --no-save js-yaml` to resolve the $ref checker's parser, | ||
| # which materialises node_modules/ in the working tree. This repo has no package.json and ships | ||
| # no JS artifact — nothing here should ever be committed. | ||
| # package.json pins the spec tooling (redocly, openapi-typescript, js-yaml) by exact version | ||
| # and package-lock.json pins the full transitive tree; CI installs with | ||
| # `npm ci --ignore-scripts` so every dependency is reproducible and install hooks never run. | ||
| # Only node_modules/ stays out of the tree. | ||
| node_modules | ||
| package-lock.json | ||
|
|
||
| # macOS | ||
| .DS_Store |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟨 Spec tooling is installed without the lockfile and with install scripts enabled
The CI jobs install tooling with
npx --yes @redocly/cli@2.40.0andnpm 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 apackage-lock.jsonand.gitignore:2-5states CI installs withnpm 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.