Skip to content

ci: fail closed on API compatibility checks - #837

Open
ryanduguid wants to merge 5 commits into
XeroAPI:masterfrom
ryanduguid:agent/fail-closed-api-diff
Open

ci: fail closed on API compatibility checks#837
ryanduguid wants to merge 5 commits into
XeroAPI:masterfrom
ryanduguid:agent/fail-closed-api-diff

Conversation

@ryanduguid

@ryanduguid ryanduguid commented Aug 12, 2026

Copy link
Copy Markdown

Summary

  • always fail when oasdiff reports a breaking change, instead of trusting a contributor-controlled commit marker to waive the check
  • fail when the base cannot be refreshed or when changelog generation fails
  • pin the oasdiff container by version and multi-platform digest
  • document the immutable default and the tag-plus-digest update procedure
  • reduce workflow permissions to read-only, pin checkout, and bound runtime
  • replace the old commit-message tests with pass, breaking-change, tool-error and invalid-argument behaviour tests

Validation

  • bash -n scripts/api-diff/api-diff.sh scripts/api-diff/api-diff.test.sh
  • bash scripts/api-diff/api-diff.test.sh
  • actionlint .github/workflows/api-diff.yml
  • git diff --check

Pin provenance was checked against the upstream oasdiff v1.28.0 release and the publisher's Docker registry manifest. The tufin/oasdiff:v1.28.0 multi-platform manifest resolved to sha256:86830f988eaafcf589acb2794ee5ab78e3300ded071d6517bf085469300cbf36, matching the workflow default.

Docker is unavailable in the local Windows environment, so the local test suite uses a deterministic fake Docker command to exercise each exit path. Trusted Codegen Repo Validation passed on exact head 3311e7b in 4m32s (run 31624823983).

@ryanduguid
ryanduguid marked this pull request as ready for review August 18, 2026 15:08
Enumerate the union of specs in the base ref and the working tree, and treat
a spec present in the base but absent at head as a breaking change. The list
was built only from `ls xero*.yaml` in the working tree, so a spec deleted or
renamed in a PR was never compared: deleting xero_bankfeeds.yaml reported
"No breaking changes detected across all files" and exited 0. That block was
byte-identical to master, so the fail-closed change did not cover it.

Strip only the remote prefix when deriving the ref to fetch. `${BASE_BRANCH##*/}`
kept just the last path segment, so a PR based on feature/prism-changes fetched
"prism-changes", which does not exist, and `set -e` aborted with exit 128 before
any spec was compared.

Correct the actions/checkout version comment to v4.4.0. The pinned SHA
11d5960a326750d5838078e36cf38b85af677262 is v4.4.0, not v4.2.2, which is
11bd71901bbe5b1630ceea73d27597364c9af683. The SHA is kept because it is
immutable and current; only the comment was wrong.

Update the README to match the enforced behaviour: drop the removed
--fail-on-breaking flag and the conventional-commit waiver, drop the deleted
test-conventional-commit-logic job, and document the real fail-closed rules
and exit codes.
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