From 0e6a4689422e21d65d42aee533be5629f7afe985 Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Sat, 23 May 2026 15:47:20 +0300 Subject: [PATCH 1/4] breaking: add review_url output; test the free /review URL via it Add a `review_url` output to the breaking action (the free /review link it already emits as a notice) so tests can assert on it directly. Rewrite the two free-review URL tests to run the real action via `uses:` and check the output, instead of invoking the entrypoint on the runner under dash. They now run on the production platform (Alpine/busybox ash) with no shell-compat workarounds, and the strip test exercises real git-ref resolution (HEAD:specs/base.yaml), which had no CI coverage. Also align entrypoint drift: write_output now matches changelog/diff, add set -o pipefail (consistent with changelog/diff; safe now that every breaking test runs in-container), and drop an em-dash from a comment. --- .github/workflows/test-breaking.yaml | 115 +++++++++------------------ breaking/action.yml | 2 + breaking/entrypoint.sh | 21 ++--- 3 files changed, 51 insertions(+), 87 deletions(-) diff --git a/.github/workflows/test-breaking.yaml b/.github/workflows/test-breaking.yaml index 0f64b96..710813f 100644 --- a/.github/workflows/test-breaking.yaml +++ b/.github/workflows/test-breaking.yaml @@ -276,105 +276,66 @@ jobs: breaking_free_review_url_preserves_https_base: runs-on: ubuntu-latest name: Test breaking free review URL keeps the https:// scheme on URL-style base - # Regression test for the bug class fixed in pr-comment (#120): the naive - # base_path=$(echo "$base" | sed 's/.*://') strips "https:" from a - # URL-shaped base and leaves a broken "//raw.github..." in the free - # /review link, which the page renders as a misleading access-denied - # screen. The strip_ref_prefix helper passes URLs through unchanged. + # Regression test for the bug fixed in pr-comment (#120): a URL-shaped base + # must survive intact in the review URL's base_file, not be mangled to + # "//raw.github..." by the git-ref-prefix strip. Runs the real action in its + # Alpine container (the production platform) and asserts on the review_url + # output. openapi-test1 -> openapi-test3 has breaking changes, so the notice + # (and review_url) fire. steps: - uses: actions/checkout@v6 - - name: Stub oasdiff to report a change so the notice fires + - uses: ./breaking + id: brk + with: + base: 'https://raw.githubusercontent.com/oasdiff/oasdiff/main/data/openapi-test1.yaml' + revision: 'https://raw.githubusercontent.com/oasdiff/oasdiff/main/data/openapi-test3.yaml' + - name: Assert the full https:// URL is preserved in base_file + env: + REVIEW_URL: ${{ steps.brk.outputs.review_url }} run: | set -euo pipefail - mkdir -p /tmp/stub - cat > /tmp/stub/oasdiff <<'STUB' - #!/bin/sh - echo '1 breaking change(s)' - STUB - chmod +x /tmp/stub/oasdiff - - mkdir -p /tmp/run - export GITHUB_REPOSITORY=oasdiff-test/test - export GITHUB_SHA=deadbeef - export GITHUB_BASE_REF=main - export GITHUB_OUTPUT=/tmp/run/github-output - export GITHUB_STEP_SUMMARY=/tmp/run/step-summary - cat > /tmp/run/event.json <&1) - echo "--- entrypoint output ---" - echo "$out" - - if ! echo "$out" | grep -q "::notice::.*breaking changes"; then - echo "FAIL: review-URL notice line missing" >&2 + echo "review_url=$REVIEW_URL" + if [ -z "$REVIEW_URL" ]; then + echo "FAIL: review_url output is empty (expected breaking changes to fire the notice)" >&2 exit 1 fi - notice=$(echo "$out" | grep "::notice::.*breaking changes") - if echo "$notice" | grep -q 'base_file=https%3A%2F%2Fraw.githubusercontent.com'; then + if printf '%s' "$REVIEW_URL" | grep -q 'base_file=https%3A%2F%2Fraw.githubusercontent.com'; then echo "PASS: full https://raw... URL preserved in base_file" - elif echo "$notice" | grep -q 'base_file=%2F%2Fraw.githubusercontent.com'; then + elif printf '%s' "$REVIEW_URL" | grep -q 'base_file=%2F%2Fraw.githubusercontent.com'; then echo "FAIL: 'https:' was stripped from base, the strip_ref_prefix URL guard is missing" >&2 - echo "notice line: $notice" >&2 exit 1 else - echo "FAIL: base_file= param did not contain the raw.githubusercontent.com host as expected" >&2 - echo "notice line: $notice" >&2 + echo "FAIL: base_file= did not contain the raw.githubusercontent.com host as expected" >&2 exit 1 fi breaking_free_review_url_strips_git_ref_prefix: runs-on: ubuntu-latest - name: "Test breaking free review URL strips origin/main: prefix from git-ref base" - # Companion to breaking_free_review_url_preserves_https_base: a git-ref - # base (origin/main:openapi.yaml) must still have its prefix stripped so - # the /review page receives just the path. + name: "Test breaking free review URL strips the ref: prefix from a git-ref base" + # Companion to the preserve test: a git-ref base (HEAD:specs/base.yaml) must + # have its ref prefix stripped so base_file is just the path. Also exercises + # real git-ref resolution, the action's recommended base form. steps: - uses: actions/checkout@v6 - - name: Stub oasdiff to report a change so the notice fires + - uses: ./breaking + id: brk + with: + base: 'HEAD:specs/base.yaml' + revision: 'specs/revision-breaking.yaml' + - name: Assert the git-ref prefix is stripped from base_file + env: + REVIEW_URL: ${{ steps.brk.outputs.review_url }} run: | set -euo pipefail - mkdir -p /tmp/stub - cat > /tmp/stub/oasdiff <<'STUB' - #!/bin/sh - echo '1 breaking change(s)' - STUB - chmod +x /tmp/stub/oasdiff - - mkdir -p /tmp/run - export GITHUB_REPOSITORY=foo/bar - export GITHUB_SHA=deadbeef - export GITHUB_BASE_REF=main - export GITHUB_OUTPUT=/tmp/run/github-output - export GITHUB_STEP_SUMMARY=/tmp/run/step-summary - cat > /tmp/run/event.json <&1) - echo "--- entrypoint output ---" - echo "$out" - - if ! echo "$out" | grep -q "::notice::.*breaking changes"; then - echo "FAIL: review-URL notice line missing" >&2 + echo "review_url=$REVIEW_URL" + if [ -z "$REVIEW_URL" ]; then + echo "FAIL: review_url output is empty (expected breaking changes to fire the notice)" >&2 exit 1 fi - notice=$(echo "$out" | grep "::notice::.*breaking changes") - if echo "$notice" | grep -q 'base_file=multi-file%2Fopenapi.yaml'; then - echo "PASS: origin/main: prefix stripped from base" + if printf '%s' "$REVIEW_URL" | grep -q 'base_file=specs%2Fbase.yaml'; then + echo "PASS: ref prefix stripped from base" else - echo "FAIL: base_file= did not have the git-ref prefix stripped" >&2 - echo "notice line: $notice" >&2 + echo "FAIL: base_file= did not have the git-ref prefix stripped (got: $REVIEW_URL)" >&2 exit 1 fi diff --git a/breaking/action.yml b/breaking/action.yml index 2c6adf0..ef7c00f 100644 --- a/breaking/action.yml +++ b/breaking/action.yml @@ -55,6 +55,8 @@ inputs: outputs: breaking: description: 'Output summary of API breaking changes, encompassing both warnings and errors' + review_url: + description: 'URL of the free oasdiff review page for these changes (empty when there are no breaking changes)' runs: using: 'docker' image: 'Dockerfile' diff --git a/breaking/entrypoint.sh b/breaking/entrypoint.sh index 38869e9..5078f14 100755 --- a/breaking/entrypoint.sh +++ b/breaking/entrypoint.sh @@ -1,5 +1,6 @@ #!/bin/sh set -e +set -o pipefail if [ -n "$GITHUB_WORKSPACE" ]; then git config --global --get-all safe.directory | grep -q "$GITHUB_WORKSPACE" || \ @@ -22,23 +23,22 @@ readonly warn_ignore="${13}" readonly output_to_file="${14}" write_output () { - _write_output_output="$1" + local output="$1" if [ -n "$output_to_file" ]; then - _write_output_file_output="$2" - if [ -z "$_write_output_file_output" ]; then - _write_output_file_output=$_write_output_output - + local file_output="$2" + if [ -z "$file_output" ]; then + file_output=$output fi - echo "$_write_output_file_output" >> "$output_to_file" + echo "$file_output" >> "$output_to_file" fi # github-action limits output to 1MB # we count bytes because unicode has multibyte characters - size=$(echo "$_write_output_output" | wc -c) + size=$(echo "$output" | wc -c) if [ "$size" -ge "1000000" ]; then echo "WARN: diff exceeds the 1MB limit, truncating output..." >&2 - _write_output_output=$(echo "$_write_output_output" | head -c 1000000) + output=$(echo "$output" | head -c 1000000) fi - echo "$_write_output_output" >>"$GITHUB_OUTPUT" + echo "$output" >>"$GITHUB_OUTPUT" } echo "running oasdiff breaking... base: $base, revision: $revision, fail_on: $fail_on, include_checks: $include_checks, include_path_params: $include_path_params, deprecation_days_beta: $deprecation_days_beta, deprecation_days_stable: $deprecation_days_stable, exclude_elements: $exclude_elements, filter_extension: $filter_extension, composed: $composed, flatten_allof: $flatten_allof, err_ignore: $err_ignore, warn_ignore: $warn_ignore, output_to_file: $output_to_file" @@ -127,7 +127,7 @@ if [ -n "$breaking_changes" ] && ! echo "$breaking_changes" | head -n 1 | grep - if [ -z "$head_sha" ]; then head_sha="$GITHUB_SHA"; fi # base_sha must be an immutable commit SHA, not the branch name. Using # $GITHUB_BASE_REF (the branch) makes the URL decay whenever the branch - # advances past the file's commit — e.g. someone merges a rename of the + # advances past the file's commit, e.g. someone merges a rename of the # spec file and every previously-emitted /review URL starts 404'ing # because raw.githubusercontent.com now resolves the branch to a newer # commit where the file lives at a different path. @@ -136,6 +136,7 @@ if [ -n "$breaking_changes" ] && ! echo "$breaking_changes" | head -n 1 | grep - free_review_url="https://www.oasdiff.com/review?owner=${owner}&repo=${repo}&base_sha=$(urlencode "$base_sha")&rev_sha=${head_sha}&base_file=$(urlencode "$base_path")&rev_file=$(urlencode "$rev_path")" echo "::notice::📋 Review & approve these breaking changes → ${free_review_url}" echo "### 📋 [Review & approve these breaking changes](${free_review_url})" >> "$GITHUB_STEP_SUMMARY" + echo "review_url=${free_review_url}" >> "$GITHUB_OUTPUT" else write_output "No breaking changes" fi From 5dd6e173d374fd6c6126be8e76d37a087206fa03 Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Sat, 23 May 2026 15:50:23 +0300 Subject: [PATCH 2/4] breaking: write review_url output outside the breaking multiline block The review_url echo was between `breaking<<$delimiter` and its closing delimiter, so it got folded into the `breaking` output value (breaking the existing output-comparison tests) and never became its own output (so the URL tests saw it empty). Move it after the block closes. --- breaking/entrypoint.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/breaking/entrypoint.sh b/breaking/entrypoint.sh index 5078f14..0d06135 100755 --- a/breaking/entrypoint.sh +++ b/breaking/entrypoint.sh @@ -136,11 +136,14 @@ if [ -n "$breaking_changes" ] && ! echo "$breaking_changes" | head -n 1 | grep - free_review_url="https://www.oasdiff.com/review?owner=${owner}&repo=${repo}&base_sha=$(urlencode "$base_sha")&rev_sha=${head_sha}&base_file=$(urlencode "$base_path")&rev_file=$(urlencode "$rev_path")" echo "::notice::📋 Review & approve these breaking changes → ${free_review_url}" echo "### 📋 [Review & approve these breaking changes](${free_review_url})" >> "$GITHUB_STEP_SUMMARY" - echo "review_url=${free_review_url}" >> "$GITHUB_OUTPUT" else write_output "No breaking changes" fi echo "$delimiter" >>"$GITHUB_OUTPUT" +# review_url is a single-line output, written after the multiline `breaking` +# block is closed so it doesn't get folded into that value. Empty when there +# are no breaking changes (the notice/URL only fire then). +echo "review_url=${free_review_url:-}" >> "$GITHUB_OUTPUT" exit $exit_code From 89b697f2e8ba35bfbdbdf25aef0e05391f59041f Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Sat, 23 May 2026 15:54:43 +0300 Subject: [PATCH 3/4] changelog: add review_url output; test the free /review URL via it Mirror the breaking change: add a review_url output (written after the multiline changelog block closes) and rewrite the two free-review URL tests to run the real action via uses: + assert on the output, dropping the bash/dash workaround. Also expands the base_sha comment to match breaking's (drift cleanup). --- .github/workflows/test-changelog.yaml | 114 ++++++++------------------ changelog/action.yml | 2 + changelog/entrypoint.sh | 10 ++- 3 files changed, 46 insertions(+), 80 deletions(-) diff --git a/.github/workflows/test-changelog.yaml b/.github/workflows/test-changelog.yaml index 143ffec..0db568d 100644 --- a/.github/workflows/test-changelog.yaml +++ b/.github/workflows/test-changelog.yaml @@ -90,106 +90,64 @@ jobs: changelog_free_review_url_preserves_https_base: runs-on: ubuntu-latest name: Test changelog free review URL keeps the https:// scheme on URL-style base - # Regression test for the bug class fixed in pr-comment (#120): a - # URL-shaped base must survive intact in the free /review link rather - # than being mangled to "//raw.github..." by the naive ref-prefix strip. + # Regression test for the bug fixed in pr-comment (#120): a URL-shaped base + # must survive intact in the review URL's base_file. Runs the real action in + # its Alpine container (the production platform) and asserts on the + # review_url output. steps: - uses: actions/checkout@v6 - - name: Stub oasdiff to report a change so the notice fires + - uses: ./changelog + id: clog + with: + base: 'https://raw.githubusercontent.com/oasdiff/oasdiff/main/data/openapi-test1.yaml' + revision: 'https://raw.githubusercontent.com/oasdiff/oasdiff/main/data/openapi-test3.yaml' + - name: Assert the full https:// URL is preserved in base_file + env: + REVIEW_URL: ${{ steps.clog.outputs.review_url }} run: | set -euo pipefail - mkdir -p /tmp/stub - cat > /tmp/stub/oasdiff <<'STUB' - #!/bin/sh - echo '1 change(s)' - STUB - chmod +x /tmp/stub/oasdiff - - mkdir -p /tmp/run - export GITHUB_REPOSITORY=oasdiff-test/test - export GITHUB_SHA=deadbeef - export GITHUB_BASE_REF=main - export GITHUB_OUTPUT=/tmp/run/github-output - export GITHUB_STEP_SUMMARY=/tmp/run/step-summary - cat > /tmp/run/event.json <&1) - echo "--- entrypoint output ---" - echo "$out" - - if ! echo "$out" | grep -q "::notice::.*API changes"; then - echo "FAIL: review-URL notice line missing" >&2 + echo "review_url=$REVIEW_URL" + if [ -z "$REVIEW_URL" ]; then + echo "FAIL: review_url output is empty (expected changes to fire the notice)" >&2 exit 1 fi - notice=$(echo "$out" | grep "::notice::.*API changes") - if echo "$notice" | grep -q 'base_file=https%3A%2F%2Fraw.githubusercontent.com'; then + if printf '%s' "$REVIEW_URL" | grep -q 'base_file=https%3A%2F%2Fraw.githubusercontent.com'; then echo "PASS: full https://raw... URL preserved in base_file" - elif echo "$notice" | grep -q 'base_file=%2F%2Fraw.githubusercontent.com'; then + elif printf '%s' "$REVIEW_URL" | grep -q 'base_file=%2F%2Fraw.githubusercontent.com'; then echo "FAIL: 'https:' was stripped from base, the strip_ref_prefix URL guard is missing" >&2 - echo "notice line: $notice" >&2 exit 1 else - echo "FAIL: base_file= param did not contain the raw.githubusercontent.com host as expected" >&2 - echo "notice line: $notice" >&2 + echo "FAIL: base_file= did not contain the raw.githubusercontent.com host as expected" >&2 exit 1 fi changelog_free_review_url_strips_git_ref_prefix: runs-on: ubuntu-latest - name: "Test changelog free review URL strips origin/main: prefix from git-ref base" - # Companion: git-ref base must have its prefix stripped to a bare path. + name: "Test changelog free review URL strips the ref: prefix from a git-ref base" + # Companion to the preserve test: a git-ref base (HEAD:specs/base.yaml) must + # have its ref prefix stripped so base_file is just the path. Also exercises + # real git-ref resolution, the action's recommended base form. steps: - uses: actions/checkout@v6 - - name: Stub oasdiff to report a change so the notice fires + - uses: ./changelog + id: clog + with: + base: 'HEAD:specs/base.yaml' + revision: 'specs/revision-breaking.yaml' + - name: Assert the git-ref prefix is stripped from base_file + env: + REVIEW_URL: ${{ steps.clog.outputs.review_url }} run: | set -euo pipefail - mkdir -p /tmp/stub - cat > /tmp/stub/oasdiff <<'STUB' - #!/bin/sh - echo '1 change(s)' - STUB - chmod +x /tmp/stub/oasdiff - - mkdir -p /tmp/run - export GITHUB_REPOSITORY=foo/bar - export GITHUB_SHA=deadbeef - export GITHUB_BASE_REF=main - export GITHUB_OUTPUT=/tmp/run/github-output - export GITHUB_STEP_SUMMARY=/tmp/run/step-summary - cat > /tmp/run/event.json <&1) - echo "--- entrypoint output ---" - echo "$out" - - if ! echo "$out" | grep -q "::notice::.*API changes"; then - echo "FAIL: review-URL notice line missing" >&2 + echo "review_url=$REVIEW_URL" + if [ -z "$REVIEW_URL" ]; then + echo "FAIL: review_url output is empty (expected changes to fire the notice)" >&2 exit 1 fi - notice=$(echo "$out" | grep "::notice::.*API changes") - if echo "$notice" | grep -q 'base_file=multi-file%2Fopenapi.yaml'; then - echo "PASS: origin/main: prefix stripped from base" + if printf '%s' "$REVIEW_URL" | grep -q 'base_file=specs%2Fbase.yaml'; then + echo "PASS: ref prefix stripped from base" else - echo "FAIL: base_file= did not have the git-ref prefix stripped" >&2 - echo "notice line: $notice" >&2 + echo "FAIL: base_file= did not have the git-ref prefix stripped (got: $REVIEW_URL)" >&2 exit 1 fi diff --git a/changelog/action.yml b/changelog/action.yml index 691db26..3f14cd9 100644 --- a/changelog/action.yml +++ b/changelog/action.yml @@ -58,6 +58,8 @@ inputs: outputs: changelog: description: 'Output summary of API changelog' + review_url: + description: 'URL of the free oasdiff review page for these changes (empty when there are no changes)' runs: using: 'docker' image: 'Dockerfile' diff --git a/changelog/entrypoint.sh b/changelog/entrypoint.sh index 0088e0d..7cfc536 100755 --- a/changelog/entrypoint.sh +++ b/changelog/entrypoint.sh @@ -119,8 +119,10 @@ if [ -n "$output" ] && ! echo "$output" | head -n 1 | grep -q "^No "; then if [ -z "$head_sha" ]; then head_sha="$GITHUB_SHA"; fi # base_sha must be an immutable commit SHA, not the branch name. Using # $GITHUB_BASE_REF (the branch) makes the URL decay whenever the branch - # advances past the file's commit. See breaking/entrypoint.sh for the - # full rationale. + # advances past the file's commit, e.g. someone merges a rename of the + # spec file and every previously-emitted /review URL starts 404'ing + # because raw.githubusercontent.com now resolves the branch to a newer + # commit where the file lives at a different path. base_sha=$(jq -r '.pull_request.base.sha // empty' "$GITHUB_EVENT_PATH" 2>/dev/null || echo "") if [ -z "$base_sha" ]; then base_sha=$(git rev-parse "origin/$GITHUB_BASE_REF" 2>/dev/null || echo "$GITHUB_BASE_REF"); fi free_review_url="https://www.oasdiff.com/review?owner=${owner}&repo=${repo}&base_sha=$(urlencode "$base_sha")&rev_sha=${head_sha}&base_file=$(urlencode "$base_path")&rev_file=$(urlencode "$rev_path")" @@ -131,6 +133,10 @@ else fi echo "$delimiter" >>"$GITHUB_OUTPUT" +# review_url is a single-line output, written after the multiline `changelog` +# block is closed so it doesn't get folded into that value. Empty when there +# are no changes (the notice/URL only fire then). +echo "review_url=${free_review_url:-}" >> "$GITHUB_OUTPUT" # *** github action step output *** From 3bf47716aa16281043f57ca61b5ef79f6a6287b0 Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Sat, 23 May 2026 17:35:24 +0300 Subject: [PATCH 4/4] actions: drop non-POSIX set -o pipefail from all entrypoints The entrypoints are #!/bin/sh. pipefail is not POSIX (the runner's dash rejects it), and it bought nothing here: every pipeline is either inside an if-condition or ends in the command whose failure already propagates. Standardize all four on `set -e` only, which also makes them runnable directly under the runner's dash. --- breaking/entrypoint.sh | 1 - changelog/entrypoint.sh | 2 -- diff/entrypoint.sh | 2 -- 3 files changed, 5 deletions(-) diff --git a/breaking/entrypoint.sh b/breaking/entrypoint.sh index 0d06135..c5fdd88 100755 --- a/breaking/entrypoint.sh +++ b/breaking/entrypoint.sh @@ -1,6 +1,5 @@ #!/bin/sh set -e -set -o pipefail if [ -n "$GITHUB_WORKSPACE" ]; then git config --global --get-all safe.directory | grep -q "$GITHUB_WORKSPACE" || \ diff --git a/changelog/entrypoint.sh b/changelog/entrypoint.sh index 7cfc536..090475e 100755 --- a/changelog/entrypoint.sh +++ b/changelog/entrypoint.sh @@ -79,8 +79,6 @@ if [ -n "$level" ]; then fi echo "flags: $flags" -set -o pipefail - # *** github action step output *** # output name should be in the syntax of multiple lines: diff --git a/diff/entrypoint.sh b/diff/entrypoint.sh index c77c78c..186560b 100755 --- a/diff/entrypoint.sh +++ b/diff/entrypoint.sh @@ -73,8 +73,6 @@ echo "flags: $flags" delimiter=$(cat /proc/sys/kernel/random/uuid | tr -d '-') echo "diff<<$delimiter" >>"$GITHUB_OUTPUT" -set -o pipefail - # Capture the exit code from oasdiff command while still getting the output exit_code=0 if [ -n "$flags" ]; then