Skip to content

fix(review): format-independent no-change detection; skip --open in composed mode #164

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.

Sign up for GitHub

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

Merged
reuvenharrison merged 5 commits into from
Jun 14, 2026
Merged

fix(review): format-independent no-change detection; skip --open in composed mode #164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b303ef1
fix(review): format-independent no-change detection; skip --open in c…
reuvenharrison Jun 14, 2026
5944c0d
docs(review): drop stale "reuse the same semantic flags" comment
reuvenharrison Jun 14, 2026
9654f43
fix(changelog): detect changes via --fail-on=INFO exit code, not json…
reuvenharrison Jun 14, 2026
2ee6583
fix(breaking): detect changes via --fail-on=WARN exit code, not "^No …
reuvenharrison Jun 14, 2026
d6ce0d1
fix(review): harden actions against disruptive .oasdiff config
reuvenharrison Jun 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 47 additions & 24 deletions breaking/entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,10 @@ fi
if [ -n "$filter_extension" ]; then
flags="$flags --filter-extension $filter_extension"
fi
if [ "$composed" = "true" ]; then
flags="$flags -c"
fi
# Pin composed to the action input so a 'composed' setting in .oasdiff.yaml can't
# desync oasdiff's mode from the action's logic (the --open guard and flag
# building both key off this input). A cmd-line flag overrides config.
flags="$flags --composed=$composed"
if [ "$flatten_allof" = "true" ]; then
flags="$flags --flatten-allof"
fi
Expand Down Expand Up @@ -186,8 +187,10 @@ rm -f "$_err"
# Run 2: render annotations to stdout via --format githubactions so
# GitHub parses them onto the PR's "Files changed" tab. Tolerate
# non-zero exit (could be triggered by oasdiff.yaml fail-on); the
# authoritative exit code is from Run 1.
oasdiff breaking "$base" "$revision" $flags --format githubactions || true
# authoritative exit code is from Run 1. --template= overrides a 'template'
# set in .oasdiff.yaml, which is rejected for the githubactions format and
# would (via || true) silently suppress the annotations.
oasdiff breaking "$base" "$revision" $flags --format githubactions --template= || true

# *** GitHub Action step output ***

Expand All @@ -199,7 +202,16 @@ oasdiff breaking "$base" "$revision" $flags --format githubactions || true
delimiter=$(cat /proc/sys/kernel/random/uuid | tr -d '-')
echo "breaking<<$delimiter" >>"$GITHUB_OUTPUT"

if [ -n "$breaking_changes" ] && ! echo "$breaking_changes" | head -n 1 | grep -q "^No "; then
# Whether there are breaking changes is read off oasdiff's exit code with
# --fail-on=WARN (breaking renders WARN-and-above, and WARN is the lowest level
# it accepts for fail-on): exit 1 means at least one breaking change, 0 means
# none. This is format-proof; parsing $breaking_changes is not, because its shape
# follows any 'format' set in .oasdiff.yaml (e.g. json renders "[]", not "No ...").
# The explicit --fail-on overrides a config fail-on for this probe only; Run 1's
# authoritative gate exit code is untouched.
changes_exit=0
oasdiff breaking "$base" "$revision" $flags --fail-on=WARN --template= >/dev/null 2>&1 || changes_exit=$?
if [ "$changes_exit" -eq 1 ]; then
write_output "$(echo "$breaking_changes" | head -n 1)" "$breaking_changes"

free_review_url=""
Expand All @@ -211,25 +223,36 @@ if [ -n "$breaking_changes" ] && ! echo "$breaking_changes" | head -n 1 | grep -
# entirely, so no spec ever leaves CI; the breaking-change detection and
# the inline annotations are unaffected either way.
if [ "$review" != "false" ]; then
# Reuse the same semantic flags as the diff above so the uploaded
# comparison matches. --open prints the review URL on stdout; in CI the
# browser-open step soft-fails. We grep the /review/e/ URL out by its
# stable path shape (not by surrounding prose). Tolerate a non-zero
# exit / no match so `set -e` doesn't abort the run.
free_review_url=$(oasdiff breaking "$base" "$revision" $flags --open 2>/dev/null \
| grep -oE 'https://[^[:space:]]+/review/e/[^[:space:]]+' | head -n 1) || true
if [ -n "$free_review_url" ]; then
echo "### 📋 [View these breaking changes in a side-by-side review](${free_review_url})" >> "$GITHUB_STEP_SUMMARY"
# Also surface the link on the PR itself (best-effort) so reviewers
# don't have to find the job summary.
post_review_comment "$free_review_url"
if [ "$composed" = "true" ]; then
# Composed mode (-c) diffs globs of many files; the side-by-side
# review represents exactly two specs, so --open can't build it.
# Say so once instead of running --open only to hit the generic
# "couldn't upload" warning below.
echo "::notice::oasdiff: the side-by-side review isn't available in composed mode (-c). The breaking-change report above is unaffected."
else
# review was requested but no link came back: an offline runner,
# oasdiff.com unreachable, or an older oasdiff in the base image.
# Warn rather than emit a link -- there's no useful local fallback
# (if the upload failed because the host is unreachable, a manual
# run would fail the same way), and the report above still stands.
echo "::warning::oasdiff: couldn't upload the side-by-side review (the breaking-change report still ran). Re-run the job, or set 'review: false' to skip the upload."
# --open prints the review URL on stdout; in CI the browser-open
# step soft-fails. We grep the /review/e/ URL out by its stable path
# shape (not by surrounding prose). Tolerate a non-zero exit / no
# match so `set -e` doesn't abort the run. --template= overrides a
# 'template' set in .oasdiff.yaml, which would otherwise error this
# render (templates are rejected for the default text format) and
# yield no URL.
free_review_url=$(oasdiff breaking "$base" "$revision" $flags --open --template= 2>/dev/null \
| grep -oE 'https://[^[:space:]]+/review/e/[^[:space:]]+' | head -n 1) || true
if [ -n "$free_review_url" ]; then
echo "### 📋 [View these breaking changes in a side-by-side review](${free_review_url})" >> "$GITHUB_STEP_SUMMARY"
# Also surface the link on the PR itself (best-effort) so
# reviewers don't have to find the job summary.
post_review_comment "$free_review_url"
else
# review was requested but no link came back: an offline runner,
# oasdiff.com unreachable, or an older oasdiff in the base image.
# Warn rather than emit a link -- there's no useful local
# fallback (if the upload failed because the host is unreachable,
# a manual run would fail the same way), and the report above
# still stands.
echo "::warning::oasdiff: couldn't upload the side-by-side review (the breaking-change report still ran). Re-run the job, or set 'review: false' to skip the upload."
fi
fi
fi
else
Expand Down
103 changes: 64 additions & 39 deletions changelog/entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,10 @@ fi
if [ -n "$filter_extension" ]; then
flags="$flags --filter-extension $filter_extension"
fi
if [ "$composed" = "true" ]; then
flags="$flags -c"
fi
# Pin composed to the action input so a 'composed' setting in .oasdiff.yaml can't
# desync oasdiff's mode from the action's logic (the --open guard and flag
# building both key off this input). A cmd-line flag overrides config.
flags="$flags --composed=$composed"
if [ "$flatten_allof" = "true" ]; then
flags="$flags --flatten-allof"
fi
Expand All @@ -149,16 +150,22 @@ fi
if [ "$case_insensitive_headers" = "true" ]; then
flags="$flags --case-insensitive-headers"
fi
if [ -n "$level" ]; then
flags="$flags --level $level"
fi
# format and template are presentation-only. Keep them out of $flags so the
# changes probe and the --open upload below run on the semantic flags only: the
# probe doesn't need the user's format, and --open renders the changelog as json
# internally where --template is rejected (it would error the upload). $fmt_flags
# is applied only to the user-facing changelog render.
fmt_flags=""
if [ -n "$format" ]; then
flags="$flags --format $format"
fmt_flags="$fmt_flags --format $format"
fi
if [ -n "$template" ]; then
flags="$flags --template $template"
fi
if [ -n "$level" ]; then
flags="$flags --level $level"
fmt_flags="$fmt_flags --template $template"
fi
echo "flags: $flags"
echo "flags: $flags, presentation flags: $fmt_flags"

# *** github action step output ***

Expand All @@ -172,18 +179,14 @@ echo "changelog<<$delimiter" >>"$GITHUB_OUTPUT"

exit_code=0
_err=$(mktemp)
if [ -n "$flags" ]; then
output=$(oasdiff changelog "$base" "$revision" $flags 2>"$_err") || exit_code=$?
else
output=$(oasdiff changelog "$base" "$revision" 2>"$_err") || exit_code=$?
fi
if [ "$exit_code" -ne 0 ]; then
output=$(oasdiff changelog "$base" "$revision" $flags $fmt_flags 2>"$_err") || exit_code=$?
# Only codes >=2 are real errors. Exit 1 is a fail-on result (the changelog
# action has no fail-on input, but .oasdiff.yaml can set one); the changelog
# action doesn't gate, so don't let it abort the run or suppress the review below.
if [ "$exit_code" -ge 2 ]; 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
# Promote a genuine failure to a Checks-tab annotation.
[ -s "$_err" ] && echo "::error::$(tr '\n' ' ' < "$_err")"
# 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
Expand All @@ -194,7 +197,18 @@ if [ "$exit_code" -ne 0 ]; then
fi
rm -f "$_err"

if [ -n "$output" ] && ! echo "$output" | head -n 1 | grep -q "^No "; then
# Decide whether there are changes independently of --format. The user-facing
# $output varies by format (json/yaml render "[]", markup adds a header line), so
# parsing it is fragile. Instead read oasdiff's exit code with --fail-on=INFO:
# exit 1 means the changelog has at least one change at or above --level, exit 0
# means it's empty. --level stays in $flags so detection matches what the user
# sees; the explicit --fail-on overrides any config fail-on for this probe only,
# and the rendered run above is untouched. --template= overrides a 'template' set
# in .oasdiff.yaml, which would otherwise error this probe (templates are rejected
# for the default text format) and make it a false negative.
changes_exit=0
oasdiff changelog "$base" "$revision" $flags --fail-on=INFO --template= >/dev/null 2>&1 || changes_exit=$?
if [ "$changes_exit" -eq 1 ]; then
write_output "$output"

free_review_url=""
Expand All @@ -206,25 +220,36 @@ if [ -n "$output" ] && ! echo "$output" | head -n 1 | grep -q "^No "; then
# entirely, so no spec ever leaves CI; the changelog output and the inline
# annotations are unaffected either way.
if [ "$review" != "false" ]; then
# Reuse the same semantic flags as the diff above so the uploaded
# comparison matches. --open prints the review URL on stdout; in CI the
# browser-open step soft-fails. We grep the /review/e/ URL out by its
# stable path shape (not by surrounding prose). Tolerate a non-zero
# exit / no match so `set -e` doesn't abort the run.
free_review_url=$(oasdiff changelog "$base" "$revision" $flags --open 2>/dev/null \
| grep -oE 'https://[^[:space:]]+/review/e/[^[:space:]]+' | head -n 1) || true
if [ -n "$free_review_url" ]; then
echo "### 📋 [View these API changes in a side-by-side review](${free_review_url})" >> "$GITHUB_STEP_SUMMARY"
# Also surface the link on the PR itself (best-effort) so reviewers
# don't have to find the job summary.
post_review_comment "$free_review_url"
if [ "$composed" = "true" ]; then
# Composed mode (-c) diffs globs of many files; the side-by-side
# review represents exactly two specs, so --open can't build it.
# Say so once instead of running --open only to hit the generic
# "couldn't upload" warning below.
echo "::notice::oasdiff: the side-by-side review isn't available in composed mode (-c). The changelog above is unaffected."
else
# review was requested but no link came back: an offline runner,
# oasdiff.com unreachable, or an older oasdiff in the base image.
# Warn rather than emit a link -- there's no useful local fallback
# (if the upload failed because the host is unreachable, a manual
# run would fail the same way), and the changelog above still stands.
echo "::warning::oasdiff: couldn't upload the side-by-side review (the changelog still ran). Re-run the job, or set 'review: false' to skip the upload."
# --open prints the review URL on stdout; in CI the browser-open
# step soft-fails. We grep the /review/e/ URL out by its stable path
# shape (not by surrounding prose). Tolerate a non-zero exit / no
# match so `set -e` doesn't abort the run. --template= overrides a
# 'template' set in .oasdiff.yaml, which would otherwise error this
# render (templates are rejected for the default text format) and
# yield no URL.
free_review_url=$(oasdiff changelog "$base" "$revision" $flags --open --template= 2>/dev/null \
| grep -oE 'https://[^[:space:]]+/review/e/[^[:space:]]+' | head -n 1) || true
if [ -n "$free_review_url" ]; then
echo "### 📋 [View these API changes in a side-by-side review](${free_review_url})" >> "$GITHUB_STEP_SUMMARY"
# Also surface the link on the PR itself (best-effort) so
# reviewers don't have to find the job summary.
post_review_comment "$free_review_url"
else
# review was requested but no link came back: an offline runner,
# oasdiff.com unreachable, or an older oasdiff in the base image.
# Warn rather than emit a link -- there's no useful local
# fallback (if the upload failed because the host is unreachable,
# a manual run would fail the same way), and the changelog above
# still stands.
echo "::warning::oasdiff: couldn't upload the side-by-side review (the changelog still ran). Re-run the job, or set 'review: false' to skip the upload."
fi
fi
fi
else
Expand Down
Loading