fix(review): format-independent no-change detection; skip --open in composed mode#164
Merged
Merged
Conversation
…omposed mode
Two fixes to the free side-by-side review the breaking/changelog actions post.
1. changelog: detect "no changes" independently of --format. The old test
grepped the first output line for "^No ", which only holds for the default
text format. With format: json or yaml an empty changelog renders as "[]",
and with markup it's a header line followed by "No changes detected" -- both
slipped past the test, so every clean PR got a spurious review link (and an
uploaded comparison). Detection now uses a json probe ("[]" == no changes),
which is unambiguous across formats. To make that probe (and the --open
upload) safe, format/template are pulled out of the semantic $flags into a
separate $fmt_flags used only for the user-facing render -- otherwise the
probe would pass --format twice and, worse, error out when a template is set
(--template is rejected for json).
2. breaking + changelog: in composed mode (-c) the action no longer runs
--open. The encrypted review carries exactly two specs and can't represent a
glob of many files, so --open now returns a clear non-fatal message in the
CLI; the action would otherwise surface the generic "couldn't upload, re-run"
warning. Emit a single ::notice:: explaining the review isn't available in
composed mode instead.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The sentence was a remnant of an abandoned approach and added no information (passing $flags to the --open run is plain from the code). Keep the part of the comment that still explains the real subtleties: --open prints the URL on stdout, the browser-open soft-fails in CI, we grep the URL by its path shape, and the run is tolerated as non-fatal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… parsing Replace the json-render "[]" probe with oasdiff's own changes-present signal: run changelog with --fail-on=INFO and read the exit code (1 = at least one change at or above --level, 0 = empty). This is format-proof by construction (no output parsing) and is the canonical "are there changes" check. --level stays in the probe so detection matches what the user sees; the explicit --fail-on overrides any config fail-on for the probe only, and the user-facing render is untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…" text The "^No " first-line heuristic assumed the breaking output is always text, but .oasdiff.yaml can set 'format' (the CLI flag exists even though the action has no format input), so a config format=json/html/markup makes an empty result render as "[]" / "<!DOCTYPE...>" / a header line and the heuristic flags a clean PR as having changes (spurious review + upload every run). Detect via oasdiff's exit code with --fail-on=WARN instead (WARN is breaking's display level and the lowest fail-on it accepts): exit 1 = at least one breaking change, 0 = none. Matches the changelog approach. The explicit --fail-on overrides any config fail-on for the probe only; Run 1's authoritative gate exit code is untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
.oasdiff.yaml can set most oasdiff flags, and the action's control flow assumed some of them. Override the ones the logic depends on, on the command line (cmd-line beats config): - composed: pin --composed=<input> in $flags (all runs). A config composed:true would otherwise run composed mode while the action's --open guard / flag building still think it's off, desyncing them (the review run would fail the new CLI composed check or build a bogus review). - template: pin --template= on the runs that force/assume a non-template format and whose output the action depends on -- the changelog/breaking change-probes (default text), both --open runs (default text), and breaking's githubactions annotations run. A config 'template' is rejected for those formats and would make a probe a false negative, drop the review URL, or (via || true) silently suppress the breaking annotations. - changelog: treat exit 1 as non-fatal (>=2 are real errors), matching breaking. A fail-on set in .oasdiff.yaml would otherwise make the changelog main run exit 1 and abort before the review is posted. (--fail-on= can't be pinned: empty is rejected.) Note: 'open' cannot be set via config -- it's not in oasdiff's Config struct, so UnmarshalExact rejects it; no override needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two fixes to the free side-by-side review the
breaking/changelogactions produce.1.
changelog: detect "no changes" independently offormatThe old test grepped the first output line for
^No, which only holds for the default text format. Withformat: jsonoryamlan empty changelog renders as[], and withmarkupit's a header line followed by "No changes detected". Both slipped past the test, so every clean PR got a spurious review link (and an uploaded comparison).Detection now uses a json probe (
[]means no changes), which is unambiguous across formats. To make that probe (and the--openupload) safe,format/templateare pulled out of the semantic$flagsinto a separate$fmt_flagsused only for the user-facing render. Otherwise the probe would pass--formattwice and, worse, error out when a template is set (--templateis rejected for json).2.
breaking+changelog: skip--openin composed modeIn composed mode (
-c) the action no longer runs--open. The encrypted review carries exactly two specs and can't represent a glob of many files, so--opennow returns a clear non-fatal message in the CLI (oasdiff/oasdiff#1009); without this change the action would surface the generic "couldn't upload, re-run" warning. It now emits a single::notice::explaining the review isn't available in composed mode.Test
sh -npasses on both entrypoints. The json last-value-wins behavior and the per-format empty representations ([]for json/yaml, header + "No changes detected" for markup) were verified against the local oasdiff build.Related
--opennon-fatal + composed-mode message)🤖 Generated with Claude Code