fix(open): make --open non-fatal and reject composed mode cleanly#1009
Merged
Conversation
--open is additive: the changelog/breaking output is already printed before the upload. Two problems made it behave otherwise: 1. An upload error (or unsupported source) returned exit code 130, which pre-empted --fail-on and changed the command's exit status. A side-by-side review that could not be opened should never alter the result of the check. Now the failure is warned to stderr (not stdout, so piped --format json/yaml output stays clean) and the command continues to its normal --fail-on exit. 2. Composed mode (-c) diffs globs of many files, which the two-spec encrypted payload and the review page cannot represent. Previously readSpecSource tried to read the literal glob string from disk and produced a confusing "no such file or directory". Now --open returns a clear "does not support composed mode" message (still non-fatal per the above). Removes the now-dead getErrUploadAndOpenFailed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1009 +/- ##
=======================================
Coverage 87.32% 87.33%
=======================================
Files 274 274
Lines 14141 14152 +11
=======================================
+ Hits 12348 12359 +11
+ Misses 1280 1278 -2
- Partials 513 515 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…F_URL) --open uploads to oasdiffSiteURL(), which defaults to oasdiff.com but follows OASDIFF_URL for a local dev server or a self-hosted deployment. The zero-knowledge comments hardcoded "oasdiff.com" as the upload target, which reads as if the destination were fixed. Reword to "the server" (noting oasdiff.com as the default) so the comments stay accurate for any OASDIFF_URL target. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the composed-mode rejection out of uploadAndOpen (which runs only after the full diff + changelog output) into getChangelog's flag validation, returning getErrInvalidFlags (exit 101). --open + -c is a static, user-fixable incompatibility, not a runtime upload problem, so it should fail fast with a clear message like any other invalid flag combination rather than running the whole diff and emitting a late warning. The non-fatal handling stays for genuine runtime/upload failures, which is what it was for. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the --open/-c incompatibility check into getParseArgs alongside the sibling argument validators (checkStdinWithComposed, checkColor) as checkOpenWithComposed, rather than in getChangelog. This is where flag-combination validation belongs: it fails before any diff work, returns the conventional exit 100 (matching the other Args validators), and keeps changelog/getChangelog free of the concern. checkOpenWithComposed no-ops on diff/summary, which share getParseArgs but don't define --open. Adds a run_test covering both breaking and changelog. 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
2 participants
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
--open(onbreakingandchangelog).1.
--openis now non-fatal for runtime failuresThe changelog/breaking output is already printed before the upload runs. Previously, an upload error (or an unsupported spec source) returned exit code
130, which pre-empted--fail-onand changed the command's exit status. A side-by-side review that could not be uploaded should never alter the result of the check.Now a runtime
--openfailure (upload error, unreachable host, unsupported source) is warned to stderr (so piped--format json/yamloutput on stdout stays clean) and the command continues to its normal--fail-onexit.Also removes the now-dead
getErrUploadAndOpenFailed.2.
--open+ composed (-c) is rejected at argument validationComposed mode diffs a glob of many files, which the two-spec review can't represent. This is a static, user-fixable incompatibility, so it's validated up front in
getParseArgs(alongside the existingcheckStdinWithComposed/checkColorchecks) ascheckOpenWithComposed, failing before any diff runs with exit100and a clear message:That distinction is deliberate: a transient upload blip can't be fixed by editing your command (so it warns and continues, per #1), but
-c --opencan (so it fails fast like any invalid flag combination).checkOpenWithComposedno-ops ondiff/summary, which sharegetParseArgsbut don't define--open.3. Comments generalized to "the server"
--openuploads tooasdiffSiteURL(), which defaults to oasdiff.com but followsOASDIFF_URLfor a local dev server or self-hosted deployment. The zero-knowledge comments hardcoded "oasdiff.com" as the destination; reworded to "the server" so they stay accurate for anyOASDIFF_URLtarget.Test
go build ./...,go vet, andgo test ./internal/pass. NewTest_OpenWithComposedRejectedcovers bothbreakingandchangelog; verifieddiff -c/summary -c(no--openflag) are unaffected.Related
--openin composed mode)🤖 Generated with Claude Code