From 043287c62c88bf785233876272795bafb70aadef Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Sun, 31 May 2026 00:22:02 +0300 Subject: [PATCH 1/2] actions: surface genuine oasdiff errors as PR annotations Previously a failed oasdiff invocation only printed to stderr (visible in the raw log, not on the Checks tab) unless it was the disallowed-external-ref case. Each entrypoint now promotes a genuine failure to a ::error:: GitHub annotation so it shows on the PR. Gated on exit code >=2, not "!= 0": exit 0 is success and exit 1 is the intended fail-on / changes-found result (not an error), so a normal "breaking changes found" run is never mislabeled as an error. The disallowed-external-ref remedy (exit 123) is layered on top. Adds test-error-annotation.yaml: drives the validate (standalone) and changelog (early-exit) entrypoints with a stubbed oasdiff at exit 102/123/1 and asserts an annotation is emitted for genuine errors and the remedy for 123, but not for the fail-on exit. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/test-error-annotation.yaml | 62 ++++++++++++++++++++ breaking/entrypoint.sh | 6 ++ changelog/entrypoint.sh | 5 ++ diff/entrypoint.sh | 6 ++ pr-comment/entrypoint.sh | 5 ++ validate/entrypoint.sh | 6 ++ 6 files changed, 90 insertions(+) create mode 100644 .github/workflows/test-error-annotation.yaml diff --git a/.github/workflows/test-error-annotation.yaml b/.github/workflows/test-error-annotation.yaml new file mode 100644 index 0000000..f9b7b0a --- /dev/null +++ b/.github/workflows/test-error-annotation.yaml @@ -0,0 +1,62 @@ +name: error-annotation +on: + pull_request: + push: +jobs: + entrypoint_error_annotation: + runs-on: ubuntu-latest + name: Entrypoints surface genuine errors as annotations, not fail-on + steps: + - name: checkout + uses: actions/checkout@v6 + - name: Exercise entrypoints with a stubbed oasdiff + run: | + set -eu + + # Stub oasdiff: write $STUB_STDERR to stderr (if set) and exit + # $STUB_CODE. Lets us drive the entrypoints' exit-code handling + # deterministically without a real spec or network. + mkdir -p /tmp/bin + cat > /tmp/bin/oasdiff <<'STUB' + #!/bin/sh + [ -n "${STUB_STDERR:-}" ] && printf '%s\n' "$STUB_STDERR" >&2 + exit "${STUB_CODE:-0}" + STUB + chmod +x /tmp/bin/oasdiff + export PATH="/tmp/bin:$PATH" + export GITHUB_OUTPUT=/tmp/gh_output + export GITHUB_STEP_SUMMARY=/tmp/gh_summary + export GITHUB_REPOSITORY=o/r GITHUB_SHA=deadbeef + + fail() { echo "FAIL: $1" >&2; exit 1; } + + # $1=entrypoint $2=exit-code $3=stderr ; echoes the entrypoint's stdout. + # Trailing args after $3 are the positional inputs for that entrypoint. + run() { + ep="$1"; code="$2"; err="$3"; shift 3 + : > "$GITHUB_OUTPUT"; : > "$GITHUB_STEP_SUMMARY" + STUB_CODE="$code" STUB_STDERR="$err" sh "$ep" "$@" 2>/dev/null || true + } + + # validate (standalone shape, shared by breaking/diff): spec, fail-on, allow-external-refs + v() { run validate/entrypoint.sh "$1" "$2" valid.yaml "" "false"; } + # changelog (early-exit shape, shared by pr-comment): 15 positional inputs + c() { run changelog/entrypoint.sh "$1" "$2" base.yaml rev.yaml "" "" "" "" "" "" "" "" "" "" "" "" "false"; } + + # --- genuine load error (exit 102) -> generic ::error:: annotation --- + echo "$(v 102 'Error: failed to load spec from "x.yaml": no such file')" | grep -q '::error::' \ + || fail "validate: exit 102 should emit a ::error:: annotation" + echo "$(c 102 'Error: failed to load spec from "x.yaml": no such file')" | grep -q '::error::' \ + || fail "changelog: exit 102 should emit a ::error:: annotation" + + # --- disallowed external ref (exit 123) -> the allow-external-refs remedy --- + echo "$(v 123 'Error: external $ref not allowed: https://evil')" | grep -q "allow-external-refs: true" \ + || fail "validate: exit 123 should emit the allow-external-refs remedy" + echo "$(c 123 'Error: external $ref not allowed: https://evil')" | grep -q "allow-external-refs: true" \ + || fail "changelog: exit 123 should emit the allow-external-refs remedy" + + # --- fail-on / changes-found (exit 1, no stderr) -> NO ::error:: annotation --- + if echo "$(v 1 '')" | grep -q '::error::'; then fail "validate: exit 1 (fail-on) must not annotate"; fi + if echo "$(c 1 '')" | grep -q '::error::'; then fail "changelog: exit 1 (fail-on) must not annotate"; fi + + echo "all error-annotation assertions passed" diff --git a/breaking/entrypoint.sh b/breaking/entrypoint.sh index a1c4bc8..e80c584 100755 --- a/breaking/entrypoint.sh +++ b/breaking/entrypoint.sh @@ -95,6 +95,12 @@ exit_code=0 _err=$(mktemp) breaking_changes=$(oasdiff breaking "$base" "$revision" $flags $fail_on_flag 2>"$_err") || exit_code=$? [ -s "$_err" ] && cat "$_err" >&2 +# Promote a genuine oasdiff failure to a Checks-tab annotation. Exit 0 is +# success and exit 1 is the intended "breaking changes found" / fail-on result; +# only codes >=2 (load/parse/etc.) are real errors worth surfacing here. +if [ "$exit_code" -ge 2 ] && [ -s "$_err" ]; then + echo "::error::$(tr '\n' ' ' < "$_err")" +fi # Exit code 123 = oasdiff refused a disallowed external $ref (stable contract, # not message text). Surface the action-specific remedy. if [ "$exit_code" -eq 123 ]; then diff --git a/changelog/entrypoint.sh b/changelog/entrypoint.sh index e4318f2..0d22c00 100755 --- a/changelog/entrypoint.sh +++ b/changelog/entrypoint.sh @@ -104,6 +104,11 @@ else fi if [ "$exit_code" -ne 0 ]; then [ -s "$_err" ] && cat "$_err" >&2 + # Promote a genuine failure to a Checks-tab annotation. Exit 1 is the + # intended fail-on result (not an error); only codes >=2 are real errors. + if [ "$exit_code" -ge 2 ] && [ -s "$_err" ]; then + echo "::error::$(tr '\n' ' ' < "$_err")" + fi # Exit code 123 = oasdiff refused a disallowed external $ref (stable # contract, not message text). Surface the action-specific remedy. if [ "$exit_code" -eq 123 ]; then diff --git a/diff/entrypoint.sh b/diff/entrypoint.sh index 3e32d15..7b4e183 100755 --- a/diff/entrypoint.sh +++ b/diff/entrypoint.sh @@ -88,6 +88,12 @@ else output=$(oasdiff diff "$base" "$revision" 2>"$_err") || exit_code=$? fi [ -s "$_err" ] && cat "$_err" >&2 +# Promote a genuine oasdiff failure to a Checks-tab annotation. Exit 0 is +# success and exit 1 is the intended fail-on-diff result; only codes >=2 +# (load/parse/etc.) are real errors worth surfacing here. +if [ "$exit_code" -ge 2 ] && [ -s "$_err" ]; then + echo "::error::$(tr '\n' ' ' < "$_err")" +fi # Exit code 123 = oasdiff refused a disallowed external $ref (stable contract, # not message text). Surface the action-specific remedy. if [ "$exit_code" -eq 123 ]; then diff --git a/pr-comment/entrypoint.sh b/pr-comment/entrypoint.sh index cafd5c0..8c40639 100755 --- a/pr-comment/entrypoint.sh +++ b/pr-comment/entrypoint.sh @@ -46,6 +46,11 @@ _err=$(mktemp) changelog=$(oasdiff changelog "$base" "$revision" --format json $flags 2>"$_err") || oasdiff_exit=$? if [ "$oasdiff_exit" -ne 0 ] && [ -z "$changelog" ]; then [ -s "$_err" ] && cat "$_err" >&2 + # Promote a genuine failure to a Checks-tab annotation. Exit 1 is the + # intended fail-on result (not an error); only codes >=2 are real errors. + if [ "$oasdiff_exit" -ge 2 ] && [ -s "$_err" ]; then + echo "::error::$(tr '\n' ' ' < "$_err")" + fi # Exit code 123 = oasdiff refused a disallowed external $ref (stable # contract, not message text). Surface the action-specific remedy. if [ "$oasdiff_exit" -eq 123 ]; then diff --git a/validate/entrypoint.sh b/validate/entrypoint.sh index 6db87f1..331c144 100755 --- a/validate/entrypoint.sh +++ b/validate/entrypoint.sh @@ -35,6 +35,12 @@ exit_code=0 _err=$(mktemp) oasdiff validate $flags --format githubactions "$spec" 2>"$_err" || exit_code=$? [ -s "$_err" ] && cat "$_err" >&2 +# Promote a genuine oasdiff failure to a Checks-tab annotation. Exit 0 is +# success and exit 1 is the intended fail-on result; only codes >=2 +# (load/parse/etc.) are real errors worth surfacing here. +if [ "$exit_code" -ge 2 ] && [ -s "$_err" ]; then + echo "::error::$(tr '\n' ' ' < "$_err")" +fi # Exit code 123 = oasdiff refused a disallowed external $ref (stable contract, # not message text). Surface the action-specific remedy. if [ "$exit_code" -eq 123 ]; then From 88cd3ba6d11686efc0d8977bf28a20ad4a723fca Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Sun, 31 May 2026 07:57:37 +0300 Subject: [PATCH 2/2] test: cover all five entrypoints + the success case in error-annotation test Previously only validate + changelog were exercised, and the exit-0 success path was untested. Now loops over all five entrypoints (breaking, changelog, diff, pr-comment, validate) across four cases each: - exit 0 (success) -> no ::error:: annotation - exit 102 (genuine load error) -> ::error:: annotation surfaced - exit 123 (disallowed external ref) -> allow-external-refs remedy - exit 1 (fail-on/changes) -> no ::error:: annotation pr-comment is driven with an empty oasdiff-token so its success path skips the network POST; its error cases early-exit before it. 20 assertions, all hermetic (stubbed oasdiff, no network). Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/test-error-annotation.yaml | 64 +++++++++++--------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/.github/workflows/test-error-annotation.yaml b/.github/workflows/test-error-annotation.yaml index f9b7b0a..5bb4123 100644 --- a/.github/workflows/test-error-annotation.yaml +++ b/.github/workflows/test-error-annotation.yaml @@ -5,17 +5,17 @@ on: jobs: entrypoint_error_annotation: runs-on: ubuntu-latest - name: Entrypoints surface genuine errors as annotations, not fail-on + name: Entrypoints surface genuine errors as annotations, not success/fail-on steps: - name: checkout uses: actions/checkout@v6 - - name: Exercise entrypoints with a stubbed oasdiff + - name: Exercise every entrypoint with a stubbed oasdiff run: | set -eu - # Stub oasdiff: write $STUB_STDERR to stderr (if set) and exit - # $STUB_CODE. Lets us drive the entrypoints' exit-code handling - # deterministically without a real spec or network. + # Stub oasdiff: write $STUB_STDERR to stderr (if set), produce no + # stdout, and exit $STUB_CODE. Lets us drive each entrypoint's + # exit-code handling deterministically — no real spec, no network. mkdir -p /tmp/bin cat > /tmp/bin/oasdiff <<'STUB' #!/bin/sh @@ -24,39 +24,43 @@ jobs: STUB chmod +x /tmp/bin/oasdiff export PATH="/tmp/bin:$PATH" - export GITHUB_OUTPUT=/tmp/gh_output - export GITHUB_STEP_SUMMARY=/tmp/gh_summary - export GITHUB_REPOSITORY=o/r GITHUB_SHA=deadbeef + export GITHUB_OUTPUT=/tmp/gh_output GITHUB_STEP_SUMMARY=/tmp/gh_summary + export GITHUB_REPOSITORY=o/r GITHUB_SHA=deadbeef GITHUB_REF=refs/pull/1/merge + echo '{}' > /tmp/event.json; export GITHUB_EVENT_PATH=/tmp/event.json fail() { echo "FAIL: $1" >&2; exit 1; } - # $1=entrypoint $2=exit-code $3=stderr ; echoes the entrypoint's stdout. - # Trailing args after $3 are the positional inputs for that entrypoint. - run() { - ep="$1"; code="$2"; err="$3"; shift 3 + # invoke -> prints the entrypoint's stdout. + # pr-comment gets an empty oasdiff-token so it skips the network POST. + invoke() { + export STUB_CODE="$1" STUB_STDERR="$2"; cmd="$3" : > "$GITHUB_OUTPUT"; : > "$GITHUB_STEP_SUMMARY" - STUB_CODE="$code" STUB_STDERR="$err" sh "$ep" "$@" 2>/dev/null || true + case "$cmd" in + breaking) sh breaking/entrypoint.sh base.yaml rev.yaml "" "" "" "" "" "" "" "" "" "" "" "" "false" ;; + changelog) sh changelog/entrypoint.sh base.yaml rev.yaml "" "" "" "" "" "" "" "" "" "" "" "" "false" ;; + diff) sh diff/entrypoint.sh base.yaml rev.yaml "" "" "" "" "" "" "" "false" ;; + pr-comment) sh pr-comment/entrypoint.sh base.yaml rev.yaml "" "" "" "" "" "" "false" ;; + validate) sh validate/entrypoint.sh spec.yaml "" "false" ;; + esac } - # validate (standalone shape, shared by breaking/diff): spec, fail-on, allow-external-refs - v() { run validate/entrypoint.sh "$1" "$2" valid.yaml "" "false"; } - # changelog (early-exit shape, shared by pr-comment): 15 positional inputs - c() { run changelog/entrypoint.sh "$1" "$2" base.yaml rev.yaml "" "" "" "" "" "" "" "" "" "" "" "" "false"; } + for cmd in breaking changelog diff pr-comment validate; do + # success (exit 0): no ::error:: annotation + out=$(invoke 0 "" "$cmd" 2>/dev/null || true) + echo "$out" | grep -q '::error::' && fail "$cmd: success (exit 0) must not emit ::error::" - # --- genuine load error (exit 102) -> generic ::error:: annotation --- - echo "$(v 102 'Error: failed to load spec from "x.yaml": no such file')" | grep -q '::error::' \ - || fail "validate: exit 102 should emit a ::error:: annotation" - echo "$(c 102 'Error: failed to load spec from "x.yaml": no such file')" | grep -q '::error::' \ - || fail "changelog: exit 102 should emit a ::error:: annotation" + # genuine error (exit 102): ::error:: annotation surfaced + out=$(invoke 102 'Error: failed to load spec from "x.yaml": no such file' "$cmd" 2>/dev/null || true) + echo "$out" | grep -q '::error::' || fail "$cmd: exit 102 must emit a ::error:: annotation" - # --- disallowed external ref (exit 123) -> the allow-external-refs remedy --- - echo "$(v 123 'Error: external $ref not allowed: https://evil')" | grep -q "allow-external-refs: true" \ - || fail "validate: exit 123 should emit the allow-external-refs remedy" - echo "$(c 123 'Error: external $ref not allowed: https://evil')" | grep -q "allow-external-refs: true" \ - || fail "changelog: exit 123 should emit the allow-external-refs remedy" + # disallowed external ref (exit 123): the allow-external-refs remedy + out=$(invoke 123 'Error: external $ref not allowed: https://evil' "$cmd" 2>/dev/null || true) + echo "$out" | grep -q 'allow-external-refs: true' || fail "$cmd: exit 123 must emit the allow-external-refs remedy" - # --- fail-on / changes-found (exit 1, no stderr) -> NO ::error:: annotation --- - if echo "$(v 1 '')" | grep -q '::error::'; then fail "validate: exit 1 (fail-on) must not annotate"; fi - if echo "$(c 1 '')" | grep -q '::error::'; then fail "changelog: exit 1 (fail-on) must not annotate"; fi + # fail-on / changes-found (exit 1, no stderr): no ::error:: annotation + out=$(invoke 1 "" "$cmd" 2>/dev/null || true) + echo "$out" | grep -q '::error::' && fail "$cmd: exit 1 (fail-on) must not emit ::error::" + echo "ok: $cmd" + done echo "all error-annotation assertions passed"