From baa1d987159784922b8e04e447515efe773d108f Mon Sep 17 00:00:00 2001 From: Ryan Duguid <152749594+ryanduguid@users.noreply.github.com> Date: Thu, 13 Aug 2026 02:31:09 +1000 Subject: [PATCH 1/4] ci: fail closed on API compatibility checks --- .github/workflows/api-diff.yml | 19 ++--- scripts/api-diff/api-diff.sh | 115 +++++++-------------------- scripts/api-diff/api-diff.test.sh | 124 ++++++++++-------------------- 3 files changed, 74 insertions(+), 184 deletions(-) diff --git a/.github/workflows/api-diff.yml b/.github/workflows/api-diff.yml index 3ba967cc4..543deaa2f 100644 --- a/.github/workflows/api-diff.yml +++ b/.github/workflows/api-diff.yml @@ -12,32 +12,23 @@ concurrency: cancel-in-progress: true jobs: - test-conventional-commit-logic: - runs-on: ubuntu-latest - permissions: - contents: read - steps: - - name: Checkout code - uses: actions/checkout@v4 - - - name: Run conventional commit logic unit tests - run: ./scripts/api-diff/api-diff.test.sh - api-diff: runs-on: ubuntu-latest - needs: test-conventional-commit-logic + timeout-minutes: 20 permissions: contents: read - pull-requests: write steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.2.2 with: fetch-depth: 0 - name: Make script executable run: chmod +x scripts/api-diff/api-diff.sh + - name: Test fail-closed behaviour + run: bash scripts/api-diff/api-diff.test.sh + - name: Run API diff check run: ./scripts/api-diff/api-diff.sh diff --git a/scripts/api-diff/api-diff.sh b/scripts/api-diff/api-diff.sh index b5c09823c..0a72b5f69 100755 --- a/scripts/api-diff/api-diff.sh +++ b/scripts/api-diff/api-diff.sh @@ -1,20 +1,19 @@ #!/bin/bash # Script to check API diffs using oasdiff -# Usage: ./scripts/api-diff/api-diff.sh [--fail-on-breaking] [filename.yaml] +# Usage: ./scripts/api-diff/api-diff.sh [filename.yaml] # Assumes you have Docker installed and the repo is checked out with master branch available -set -e # Exit on error -set -o pipefail # Catch errors in pipes +set -euo pipefail # Change to repo root cd "$(dirname "$0")/../.." # Configuration -DOCKER_IMAGE="${OASDIFF_DOCKER_IMAGE:-tufin/oasdiff:latest}" +DOCKER_IMAGE="${OASDIFF_DOCKER_IMAGE:-tufin/oasdiff:v1.28.0@sha256:86830f988eaafcf589acb2794ee5ab78e3300ded071d6517bf085469300cbf36}" # Detect base branch from GitHub Actions environment or fallback to local defaults -if [ -n "$GITHUB_BASE_REF" ]; then +if [ -n "${GITHUB_BASE_REF:-}" ]; then # In GitHub Actions PR, use the base ref (e.g., "master") BASE_BRANCH="${BASE_BRANCH:-origin/$GITHUB_BASE_REF}" else @@ -22,76 +21,17 @@ else BASE_BRANCH="${BASE_BRANCH:-origin/master}" fi -FAIL_ON_BREAKING=false TARGET_FILE="" -DRY_RUN=false - -detect_breaking_commit_marker() { - local commits="$1" - - # Conventional Commits breaking indicators: - # 1) An exclamation mark in the type/scope header, e.g. feat!: ... or feat(api)!: ... - # 2) A BREAKING CHANGE footer in the commit body - if echo "$commits" | grep -Eiq '^[[:space:]]*[a-z]+(\([^)]+\))?!:'; then - return 0 - fi - - if echo "$commits" | grep -Eiq 'BREAKING[ -]CHANGE:'; then - return 0 - fi - - return 1 -} - -get_commit_messages() { - # For tests and local overrides - if [ -n "$COMMIT_MESSAGES" ]; then - echo "$COMMIT_MESSAGES" - return 0 - fi - - # In GitHub Actions PRs, scan all commit subjects + bodies in the PR range. - if [ -n "$GITHUB_BASE_REF" ]; then - git log --format='%s%n%b%n----' "origin/$GITHUB_BASE_REF..HEAD" 2>/dev/null || true - return 0 - fi - - # Local default: inspect HEAD commit only. - git log -1 --format='%s%n%b' 2>/dev/null || true -} # Parse arguments for arg in "$@"; do - if [ "$arg" = "--fail-on-breaking" ]; then - FAIL_ON_BREAKING=true - elif [ "$arg" = "--dry-run" ]; then - DRY_RUN=true - elif [[ "$arg" == *.yaml ]]; then + if [[ "$arg" == *.yaml ]] && [ -z "$TARGET_FILE" ]; then TARGET_FILE="$arg" - fi -done - -# If --fail-on-breaking not explicitly set, determine based on conventional commit markers. -if [ "$FAIL_ON_BREAKING" = false ]; then - COMMIT_TEXT=$(get_commit_messages) - if detect_breaking_commit_marker "$COMMIT_TEXT"; then - echo "Detected conventional commit breaking marker ('!' in header or 'BREAKING CHANGE:' footer), allowing breaking changes" - FAIL_ON_BREAKING=false else - echo "No conventional commit breaking marker found, failing on breaking changes" - FAIL_ON_BREAKING=true + echo "Error: unsupported argument '$arg'" >&2 + exit 2 fi -fi - -if [ "$DRY_RUN" = true ]; then - if [ "$FAIL_ON_BREAKING" = true ]; then - echo "Mode: Failing on breaking changes" - else - echo "Mode: Allowing breaking changes" - fi - echo "Dry run mode, exiting after commit message check" - exit 0 -fi +done echo "Starting API diff check..." @@ -101,12 +41,14 @@ if [ ! -f "xero_accounting.yaml" ]; then exit 1 fi -# Fetch master if not already done -git fetch "${BASE_BRANCH%%/*}" "${BASE_BRANCH##*/}" 2>/dev/null || echo "Warning: Could not fetch ${BASE_BRANCH}" +# Refresh and verify the exact base. A missing or stale base must not turn this +# compatibility check into a pass. +git fetch "${BASE_BRANCH%%/*}" "${BASE_BRANCH##*/}" +git rev-parse --verify "${BASE_BRANCH}^{commit}" >/dev/null # Create temp directory for master branch files (outside repo to avoid overlap with /current mount) TEMP_DIR=$(mktemp -d) -trap "rm -rf $TEMP_DIR" EXIT +trap 'rm -rf "$TEMP_DIR"' EXIT # Get list of xero*.yaml files (excluding any master_*.yaml files) if [ -n "$TARGET_FILE" ]; then @@ -127,6 +69,7 @@ else fi BREAKING_CHANGES_FOUND=false +EXECUTION_FAILED=false FILES_WITH_BREAKING_CHANGES=() TOTAL_FILES=0 PROCESSED_FILES=0 @@ -143,16 +86,13 @@ for file in $files; do echo "========== $file ==========" # Get the file from master branch - if ! git show "$BASE_BRANCH:$file" > "$TEMP_DIR/$file" 2>/dev/null; then + if ! git cat-file -e "$BASE_BRANCH:$file" 2>/dev/null; then echo "ℹ️ New file (does not exist in master branch)" + PROCESSED_FILES=$((PROCESSED_FILES + 1)) continue fi - # Verify the temp file was created - if [ ! -f "$TEMP_DIR/$file" ]; then - echo "❌ Failed to create temp file" - continue - fi + git show "$BASE_BRANCH:$file" > "$TEMP_DIR/$file" # Note: oasdiff has some non-deterministic behavior in change counts due to # unordered map iteration in Go. Error counts are consistent, but warning @@ -170,7 +110,8 @@ for file in $files; do if [ $CHANGELOG_EXIT -eq 0 ]; then echo "✓ Changelog generated successfully" else - echo "⚠ Could not generate changelog (exit code: $CHANGELOG_EXIT)" + echo "❌ Could not generate changelog (exit code: $CHANGELOG_EXIT)" + EXECUTION_FAILED=true fi # Run breaking changes check @@ -201,6 +142,11 @@ echo "Processed: $PROCESSED_FILES/$TOTAL_FILES files" echo "========================================" # Summary +if [ "$EXECUTION_FAILED" = true ]; then + echo "❌ API diff execution failed" + exit 1 +fi + if [ "$BREAKING_CHANGES_FOUND" = true ]; then echo "" echo "❌ Breaking changes detected in the following files:" @@ -212,15 +158,10 @@ if [ "$BREAKING_CHANGES_FOUND" = true ]; then fi done - if [ "$FAIL_ON_BREAKING" = true ]; then - echo "" - echo "Exiting with error due to breaking changes" - exit 1 - else - echo "" - echo "Note: Not failing build (use --fail-on-breaking to fail on breaking changes)" - fi + echo "" + echo "Exiting with error due to breaking changes" + exit 1 else echo "" echo "✓ No breaking changes detected across all files" -fi \ No newline at end of file +fi diff --git a/scripts/api-diff/api-diff.test.sh b/scripts/api-diff/api-diff.test.sh index bce73e8a9..e0ddd1e5f 100755 --- a/scripts/api-diff/api-diff.test.sh +++ b/scripts/api-diff/api-diff.test.sh @@ -1,91 +1,49 @@ -#!/bin/bash +#!/usr/bin/env bash -# Unit test for api-diff.sh conventional commit logic -# Tests commit message detection and FAIL_ON_BREAKING setting +set -euo pipefail -set -e +cd "$(dirname "$0")/../.." -echo "=== Unit Test: api-diff.sh Conventional Commit Logic ===" -echo +FAKE_BIN=$(mktemp -d) +trap 'rm -rf "$FAKE_BIN"' EXIT -TESTS_PASSED=0 -TESTS_FAILED=0 - -SCRIPT_PATH="scripts/api-diff/api-diff.sh" - -# Helper function to test script with commit messages -test_commit_messages() { - local commit_messages="$1" - local expected_mode="$2" # "allow-breaking-changes" or "block-breaking-changes" - local test_name="$3" - - echo "Testing: $test_name (commit: $commit_messages)" - - # Run the script in dry-run mode with COMMIT_MESSAGES set - local output - output=$(COMMIT_MESSAGES="$commit_messages" "$SCRIPT_PATH" --dry-run 2>&1) - - if [[ "$expected_mode" == "allow-breaking-changes" ]]; then - if echo "$output" | grep -q "Mode: Allowing breaking changes"; then - echo " ✓ PASS: Correctly allows breaking changes" - TESTS_PASSED=$((TESTS_PASSED + 1)) - else - echo " ✗ FAIL: Expected to allow breaking changes, but output was:" - echo "$output" - TESTS_FAILED=$((TESTS_FAILED + 1)) - fi - elif [[ "$expected_mode" == "block-breaking-changes" ]]; then - if echo "$output" | grep -q "Mode: Failing on breaking changes"; then - echo " ✓ PASS: Correctly fails on breaking changes" - TESTS_PASSED=$((TESTS_PASSED + 1)) - else - echo " ✗ FAIL: Expected to fail on breaking changes, but output was:" - echo "$output" - TESTS_FAILED=$((TESTS_FAILED + 1)) - fi +cat > "$FAKE_BIN/docker" <<'EOF' +#!/usr/bin/env bash +if [[ " $* " == *" changelog "* ]] && [ "${FAKE_DOCKER_MODE:-pass}" = "changelog-error" ]; then + echo "simulated changelog error" >&2 + exit 2 +fi +if [[ " $* " == *" breaking "* ]] && [ "${FAKE_DOCKER_MODE:-pass}" = "breaking" ]; then + echo "simulated breaking change" >&2 + exit 1 +fi +exit 0 +EOF +chmod +x "$FAKE_BIN/docker" + +run_check() { + local expected_exit=$1 + local mode=$2 + shift 2 + + set +e + PATH="$FAKE_BIN:$PATH" \ + BASE_BRANCH=origin/master \ + FAKE_DOCKER_MODE="$mode" \ + OASDIFF_DOCKER_IMAGE=test-image \ + bash scripts/api-diff/api-diff.sh "$@" >/dev/null 2>&1 + local actual_exit=$? + set -e + + if [ "$actual_exit" -ne "$expected_exit" ]; then + echo "Expected exit $expected_exit for mode '$mode', got $actual_exit" >&2 + exit 1 fi - echo } -# Test cases: test_commit_messages "commit_messages" "expected_mode" "test_name" -# expected_mode: "allow-breaking-changes" = allows breaking changes, "block-breaking-changes" = fails on breaking +run_check 0 pass xero_accounting.yaml +run_check 1 breaking xero_accounting.yaml +run_check 1 changelog-error xero_accounting.yaml +run_check 2 pass --unsupported -echo "--- Commit messages that SHOULD allow breaking changes ---" -test_commit_messages "feat!: remove deprecated endpoint" "allow-breaking-changes" "Header with ! marker" -test_commit_messages "feat(api)!: remove deprecated endpoint" "allow-breaking-changes" "Header with scope and ! marker" -test_commit_messages "feat: refactor API\n\nBREAKING CHANGE: response schema updated" "allow-breaking-changes" "BREAKING CHANGE footer" -test_commit_messages "fix: patch bug\n\nSome details\nBREAKING CHANGE: removed old field" "allow-breaking-changes" "BREAKING CHANGE footer after body" - -echo "--- Commit messages that SHOULD fail on breaking changes ---" -test_commit_messages "feat: add optional field" "block-breaking-changes" "Normal feature commit" -test_commit_messages "fix: resolve null issue" "block-breaking-changes" "Normal fix commit" -test_commit_messages "chore: update docs" "block-breaking-changes" "Chore commit" -test_commit_messages "docs: mention breaking behaviour in description" "block-breaking-changes" "Contains word breaking but no marker" - -echo "--- Test override with --fail-on-breaking ---" -# Test that --fail-on-breaking overrides commit message logic -echo "Testing override: commit with breaking marker and --fail-on-breaking" -output=$(COMMIT_MESSAGES="feat!: breaking api update" "$SCRIPT_PATH" --dry-run --fail-on-breaking 2>&1) -if echo "$output" | grep -q "Mode: Failing on breaking changes"; then - echo " ✓ PASS: --fail-on-breaking overrides commit logic" - TESTS_PASSED=$((TESTS_PASSED + 1)) -else - echo " ✗ FAIL: --fail-on-breaking did not override, output:" - echo "$output" - TESTS_FAILED=$((TESTS_FAILED + 1)) -fi -echo - -echo "========================================" -echo "Test Results:" -echo " Passed: $TESTS_PASSED" -echo " Failed: $TESTS_FAILED" -echo "========================================" - -if [ $TESTS_FAILED -gt 0 ]; then - echo "❌ Some tests failed!" - exit 1 -else - echo "✅ All tests passed!" - exit 0 -fi +echo "API diff fail-closed tests passed" From 8d62cdabf8736d381fcc63ba95355426ca561ed8 Mon Sep 17 00:00:00 2001 From: Ryan Duguid <152749594+ryanduguid@users.noreply.github.com> Date: Thu, 13 Aug 2026 02:53:31 +1000 Subject: [PATCH 2/4] test: cover unset Actions environment --- scripts/api-diff/api-diff.sh | 2 +- scripts/api-diff/api-diff.test.sh | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/api-diff/api-diff.sh b/scripts/api-diff/api-diff.sh index 0a72b5f69..6df24460a 100755 --- a/scripts/api-diff/api-diff.sh +++ b/scripts/api-diff/api-diff.sh @@ -153,7 +153,7 @@ if [ "$BREAKING_CHANGES_FOUND" = true ]; then for file in "${FILES_WITH_BREAKING_CHANGES[@]}"; do echo " - $file" # Output GitHub Actions annotation - if [ -n "$GITHUB_ACTIONS" ]; then + if [ -n "${GITHUB_ACTIONS:-}" ]; then echo "::warning file=${file}::Breaking changes detected in this API spec file" fi done diff --git a/scripts/api-diff/api-diff.test.sh b/scripts/api-diff/api-diff.test.sh index e0ddd1e5f..d02963e32 100755 --- a/scripts/api-diff/api-diff.test.sh +++ b/scripts/api-diff/api-diff.test.sh @@ -27,7 +27,8 @@ run_check() { shift 2 set +e - PATH="$FAKE_BIN:$PATH" \ + env -u GITHUB_ACTIONS \ + PATH="$FAKE_BIN:$PATH" \ BASE_BRANCH=origin/master \ FAKE_DOCKER_MODE="$mode" \ OASDIFF_DOCKER_IMAGE=test-image \ From 3311e7b524af463bd8f88925593737cba6886782 Mon Sep 17 00:00:00 2001 From: Ryan Duguid <152749594+ryanduguid@users.noreply.github.com> Date: Thu, 13 Aug 2026 03:52:42 +1000 Subject: [PATCH 3/4] docs: describe the pinned oasdiff image --- scripts/api-diff/README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/api-diff/README.md b/scripts/api-diff/README.md index 75d9e6f47..4824603ba 100644 --- a/scripts/api-diff/README.md +++ b/scripts/api-diff/README.md @@ -23,9 +23,11 @@ Main script that compares OpenAPI specifications against the master branch. ``` **Environment Variables:** -- `OASDIFF_DOCKER_IMAGE` - Docker image to use (default: `tufin/oasdiff:latest`) +- `OASDIFF_DOCKER_IMAGE` - Docker image to use (default: oasdiff 1.28.0 pinned by image digest in `api-diff.sh`) - `BASE_BRANCH` - Branch to compare against (default: `origin/master`) +When updating oasdiff, verify the release tag and the image manifest digest from the publisher, then update both the tag and digest together. Do not replace the default with a mutable tag such as `latest`. + ### `api-diff.test.sh` Unit tests for conventional commit breaking marker detection used in GitHub Actions. From a84ce6ffcc284d49d0084ab1cd09cec7095592eb Mon Sep 17 00:00:00 2001 From: Ryan Duguid <152749594+ryanduguid@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:12:57 +1000 Subject: [PATCH 4/4] fix(api-diff): close fail-open on deleted specs and nested base branches 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. --- .github/workflows/api-diff.yml | 2 +- scripts/api-diff/README.md | 61 +++++++++++++++++++--------------- scripts/api-diff/api-diff.sh | 27 +++++++++++++-- 3 files changed, 59 insertions(+), 31 deletions(-) diff --git a/.github/workflows/api-diff.yml b/.github/workflows/api-diff.yml index 543deaa2f..6f512106b 100644 --- a/.github/workflows/api-diff.yml +++ b/.github/workflows/api-diff.yml @@ -20,7 +20,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.2.2 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 with: fetch-depth: 0 diff --git a/scripts/api-diff/README.md b/scripts/api-diff/README.md index 4824603ba..fbe8a3e04 100644 --- a/scripts/api-diff/README.md +++ b/scripts/api-diff/README.md @@ -10,18 +10,23 @@ Main script that compares OpenAPI specifications against the master branch. **Usage:** ```bash # From the repo root -./scripts/api-diff/api-diff.sh [--fail-on-breaking] [filename.yaml] +./scripts/api-diff/api-diff.sh [filename.yaml] # Check all xero*.yaml files ./scripts/api-diff/api-diff.sh # Check a single file ./scripts/api-diff/api-diff.sh xero_accounting.yaml - -# Fail on breaking changes (CI mode) -./scripts/api-diff/api-diff.sh --fail-on-breaking ``` +The script always fails on breaking changes. There is no flag to waive that, and +any unrecognised argument is rejected with exit code 2. + +**Exit Codes:** +- `0` - No breaking changes detected +- `1` - Breaking changes detected, or the check could not be completed +- `2` - Unsupported argument + **Environment Variables:** - `OASDIFF_DOCKER_IMAGE` - Docker image to use (default: oasdiff 1.28.0 pinned by image digest in `api-diff.sh`) - `BASE_BRANCH` - Branch to compare against (default: `origin/master`) @@ -29,7 +34,8 @@ Main script that compares OpenAPI specifications against the master branch. When updating oasdiff, verify the release tag and the image manifest digest from the publisher, then update both the tag and digest together. Do not replace the default with a mutable tag such as `latest`. ### `api-diff.test.sh` -Unit tests for conventional commit breaking marker detection used in GitHub Actions. +Fail-closed tests for `api-diff.sh`. They stub `docker` on `PATH` so the script's +exit codes can be checked without running oasdiff. **Usage:** ```bash @@ -37,31 +43,32 @@ Unit tests for conventional commit breaking marker detection used in GitHub Acti ``` Tests validate that: -- Commits with `!` in the conventional commit header are correctly identified -- Commits with `BREAKING CHANGE:` footer are correctly identified -- Other commits are handled with breaking change enforcement +- A clean comparison exits `0` +- A breaking change reported by oasdiff exits `1` +- A changelog generation failure exits `1` instead of being ignored +- An unsupported argument exits `2` ## Integration -These scripts are integrated into the GitHub Actions workflow at `.github/workflows/api-diff.yml`: -- **test-conventional-commit-logic** job - Runs unit tests -- **api-diff** job - Runs API diff checks with conditional breaking change enforcement - -### Conventional Commit Breaking Markers -The API diff script automatically adjusts behavior based on commit messages: - -**Allow Breaking Changes:** -- Commit header with `!`, for example: `feat!: remove deprecated endpoint` -- Commit header with scope and `!`, for example: `feat(api)!: remove deprecated endpoint` -- Commit body/footer containing `BREAKING CHANGE: ...` -- The `--fail-on-breaking` flag is NOT passed to the script - -**Fail on Breaking Changes:** -- Commits without these conventional commit breaking markers -- The `--fail-on-breaking` flag IS passed to the script -- Build will fail if breaking changes are detected - -This keeps enforcement aligned with [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/#summary) and semantic-release expectations. +These scripts run in the GitHub Actions workflow at `.github/workflows/api-diff.yml`: +- **api-diff** job - Runs the fail-closed tests, then the API diff check + +### Fail-Closed Behaviour +The check is built so that no failure mode is reported as a pass: + +- Breaking changes always fail the build. Conventional Commit markers such as + `feat!:` in the header or a `BREAKING CHANGE:` footer no longer waive the + check, and the `--fail-on-breaking` flag has been removed. +- A spec deleted or renamed in the pull request counts as a breaking change. + Specs are enumerated from the union of the base ref and the working tree, so + removing a file cannot skip the comparison. +- A base ref that cannot be fetched or resolved aborts the run rather than + comparing against nothing. +- A changelog that cannot be generated fails the run rather than logging a + warning and continuing. + +If a breaking change is intentional, coordinate the major version bump and the +release notes. There is no in-repo way to waive the check. ## Known Limitations diff --git a/scripts/api-diff/api-diff.sh b/scripts/api-diff/api-diff.sh index 6df24460a..0f072a9cc 100755 --- a/scripts/api-diff/api-diff.sh +++ b/scripts/api-diff/api-diff.sh @@ -43,7 +43,11 @@ fi # Refresh and verify the exact base. A missing or stale base must not turn this # compatibility check into a pass. -git fetch "${BASE_BRANCH%%/*}" "${BASE_BRANCH##*/}" +# Strip only the remote prefix: a base such as origin/feature/prism-changes +# must fetch "feature/prism-changes", not just the last path segment. +BASE_REMOTE="${BASE_BRANCH%%/*}" +BASE_REF="${BASE_BRANCH#"$BASE_REMOTE"/}" +git fetch "$BASE_REMOTE" "$BASE_REF" git rev-parse --verify "${BASE_BRANCH}^{commit}" >/dev/null # Create temp directory for master branch files (outside repo to avoid overlap with /current mount) @@ -60,8 +64,14 @@ if [ -n "$TARGET_FILE" ]; then files="$TARGET_FILE" echo "Running diff for single file: $TARGET_FILE" else - # All xero*.yaml files - files=$(ls xero*.yaml 2>/dev/null | grep -v "^master_") + # Union of the specs present in the base ref and in the working tree. + # Enumerating the working tree alone would silently skip a spec deleted or + # renamed in this PR, so the removal of an entire API would never be + # compared and the check would pass. + files=$( { + ls xero*.yaml 2>/dev/null || true + git ls-tree -r --name-only "$BASE_BRANCH" 2>/dev/null || true + } | grep -E '^xero[^/]*\.yaml$' | grep -v "^master_" | sort -u || true ) if [ -z "$files" ]; then echo "No xero*.yaml files found" exit 1 @@ -85,6 +95,17 @@ for file in $files; do echo "" echo "========== $file ==========" + # A spec present in the base but absent at head removes the entire API + # surface it described. That is the most breaking change possible and must + # never be reported as a pass. + if [ ! -f "$file" ]; then + echo "❌ Spec removed (present in $BASE_BRANCH, absent at HEAD)" + BREAKING_CHANGES_FOUND=true + FILES_WITH_BREAKING_CHANGES+=("$file") + PROCESSED_FILES=$((PROCESSED_FILES + 1)) + continue + fi + # Get the file from master branch if ! git cat-file -e "$BASE_BRANCH:$file" 2>/dev/null; then echo "ℹ️ New file (does not exist in master branch)"