fix(scripts): make the Check Changeset refusal say which refusal it is - #19117
Merged
os-try-charles merged 1 commit intoSep 18, 2026
Merged
Conversation
`check-changeset-no-major.mjs` already computes why it is refusing — a near-miss `Clause-②:` line, a declaration it could not read at all, a declared `yes` with no package graded `minor` — and then prints it only to the job log, which is not the check run. Measured on this repository: a failing `Check Changeset` answers `output.title` = null with 0-byte `summary`/`text` and carries one annotation, the runner's generic `Process completed with exit code 1.` So a near miss and a missing changeset are the same event from outside. Two changes, both inside the script: * every near miss now renders the REASON `readClause2Line` returned, the offending line, and the remedy that reason owes. The reason is interpolated, never matched against a list here, so a reason the reader grows later prints and names itself instead of falling through. * every refusing lane emits exactly one `::error::` workflow command, so the diagnosis crosses the check-run boundary onto the annotation channel the run already carries. Greens emit none. No workflow file is touched, and which bodies are ACCEPTED is unchanged: `CLAUSE2_KEY_LINE` and `readClause2Line` are not edited and no verdict moves. Only what the refusal SAYS changes. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk
os-try-charles
marked this pull request as ready for review
September 18, 2026 21:11
os-try-charles
deleted the
claude/issue-18263-changeset-refusal-says-its-reason
branch
September 18, 2026 21:41
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
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.
Fixes #18263
Clause-②: no
The defect, stated as it measures rather than as the title spells it
The card's title says "an entirely empty check-run output". Measured later on the same card (comment
5705401851, PR #18524, run104955982460), the failingCheck Changesetanswersoutput.title= null with 0-bytesummaryandtextand carriesannotations_count= 1 — the runner's own generic exit-code annotation (path .github,level failure,title '',message 'Process completed with exit code 1.'). So the accurate description is not "an empty output"; it is "one generic annotation that states no cause".That distinction is what makes this fix cheap. The annotation channel is already open and already carried by this job, and a plain
run:step owns exactly one way onto it — the::error::workflow command, whose sibling::noticethis script has emitted for years. The gate's prose refusals — among the best in the repo, naming the missing reading, quoting the offending line and spelling the remedy down to "this red clears with no push and no re-run" — reach the job log and are then discarded at the check-run boundary. The reason is produced and thrown away; the repair is to say it.What changed, entirely inside
scripts/check-changeset-no-major.mjsreadClause2Linehas always returnedspelling/inline-key/describingbeside the offending line; this gate printed "a near miss" and dropped the reason. It now prints the reason, the line, and the remedy that reason owes — three different sentences, becausecheck-clause2-carriers.mjssays in its own words that "⛔ The reason changes the sentence, never the state".::error::annotation, so the diagnosis crosses the check-run boundary onto the channel the run already carries. Greens emit none.The reason is interpolated, never matched against a list held here.
CLAUSE2_NEAR_MISS_REMEDIESis a lookup from whatever the reader produced, and its miss path is loud: a reason this file has never been taught still prints, still names the offending line, and still says where the reason came from. That direction is deliberate — the reader is a live surface (PR #18903 is open on it, +225/-41), and a gate that matched reasons against a frozen list would answer a new one with exactly the silence this card is about.Exactly one annotation per refusal, deliberately: a check run caps annotations at ten per level, and this script has been past that cap before (its stock-scoped predecessor emitted 171
::noticelines on PRs that introduced none of them). And the annotation is not conditioned onGITHUB_ACTIONS—renderandrenderLevelare pure by design, which is what lets the self-test assert the MESSAGE rather than the exit code, and an env read inside them would make the one thing this PR adds the one thing the fixtures cannot see.What did NOT change
.github/workflows/**file is touched. The fix lives in the.mjs, as the card asked.CLAUSE2_KEY_LINEandreadClause2Lineare not edited,scripts/pm/check-clause2-carriers.mjsis not edited, and no verdict moves. [finding] Declarations that read correctly to a human but not to the gate — two near-miss spellings measured this session, each silently costing the reading they were written to provide #16303 remains open on the accept-set question and no arm of it is implemented here. This PR makes the existing verdict legible and nothing else.Post-condition 1 — the real predicate drives every row, with a negative control that can fail
Every row below was produced by importing
readClause2Line(the same function the gate calls) and running the realjudgeLevel/renderLevel. ⛔ No hand-written matcher anywhere.readClause2Line## Clause-②: no — …(heading)near-miss/spellingspelling+ remedy`Clause-②: no` · `skip-changeset`near-miss/describingdescribing+ remedyDomain: `domain:devx` · Clause-②: nonear-miss/inline-keyinline-key+ remedyClause-②: no(bare, own line)declared/nonothing herenullClause-②:line" — and no remedy, because there is no line to remedyThe emitted text, for the heading shape (one line, escaped, abridged):
Post-condition 2 — a reason the code has no message for still prints
Driven through the exported
nearMissReadings('a-reason-this-file-has-never-been-taught', …):Control: the same call with a KNOWN reason returns a different sentence, so the miss path is not silently borrowing a known remedy — a wrong prescription is worse than a named gap.
nullandundefinedreasons also return a sentence; an empty remedy would be this card's defect moved one function along.Post-condition 3 — the two refusals, side by side
::error title=Check Changeset (level axis)%3A no readableClause-②%3Adeclaration…with the reason, the line and the remedy in the messageyes, no moved package gradedminor+::error title=Check Changeset (level axis)%3A clause-② declares YES while no moved package is gradedminoror above…pr-automation.yml,Require a changesetstep, unchanged by this PR::error::This PR adds no changeset. FIRST: …A near miss and an absent line share a verdict (
not-measured-material) and used to share every byte anyone outside the run could read; they now differ in the message, which the self-test pins. The missing-changeset refusal is the workflow's own and already carried an::error::; the self-test now pins that it keeps one, because without it that refusal and this script's are once again one event from outside.Post-condition 4 — self-test and battery floor, before and after
07c6f822e)node scripts/check-changeset-no-major.mjs --self-testSELF_TEST_BATTERY_FLOOR(pinned roster size)'Missing input is a failure, never a pass (#4690 / #7006)'floorBoth floor moves are reported rather than absorbed, and neither is a battery shrinking.
'#18263: the refusal says its reason, and says it where the API can read it'(35 cases). The floor pin is the roster's own size, so declaring a battery necessarily moves it; leaving it at 18 would let the new battery be deleted later with nothing going red. The mechanism was exercised in the process: the run before the roster entry existed failed with "registered 35 case(s) but is not declared in SELF_TEST_BATTERIES".#4690 / #7006battery goes 5 → 6 because one assertion there was split into two. The old one wasrender(unreadable).stdout.length === 0; what check:react-declaration-parity 是唯一没接进任何 workflow 的源码审计门禁,且无 MANIFEST 时静默 skip 退出 0 —— 它现在永远不可能红 #4690 forbids on stdout is a tick, and the::error::annotation is the opposite of one, so the pin is now spelled as what it always meant — every stdout line must start with::error, and there must be exactly one of them. It is strictly stronger than the line it replaces, not a relaxation.Reverse verification — three ablations, each proving the new battery can fail
Each leg mutates the committed file, proves the mutation reached disk by an anchor count, runs the self-test, then restores with
git checkout HEAD -- …and proves the restore bygit hash-objectagainst the HEAD blob. Atrap … EXIT INT TERMcarries the restore on the crash path. Predicted direction for all three: RED.::error::annotation from thenot-measured-materiallanetitle: 'Check Changeset (level axis): no readable1 → 0Object.prototype.hasOwnProperty.call(CLAUSE2_NEAR_MISS_REMEDIES, reason)1 → 0return (held at 1 → 1The failures name themselves. Leg A reds nine cases including "a body that ALMOST declared and a body that never tried produce different text". Leg B reds "the three near-miss reasons owe three DIFFERENT remedies — one shared sentence would pass every assertion above while reading no reason at all". Leg C reds "a reason of
nullorundefinedstill returns a sentence — an empty remedy is this card's defect moved one function along".Leg C's first attempt was a proven no-op and its reading was discarded, not retried quietly. Its anchor was the text the mutation inserts a copy of, so
grep -cread 1 before and 1 after and the harness refused to read a self-test result it could not prove had run. It was re-run with an injected unique marker (0 → 1) and a control on the text that must NOT vanish. The first attempt produced no reading at all; the row above is the second.Restores are proven, not assumed: each leg ends with
git checkout HEAD -- …(never a baregit checkout --, which would take the mutation back out of the index) and thengit hash-objectagainst the HEAD blobaf36b25de84a4e55c34ba323c83097c61a3f474c, plusgit diff HEADempty and the injected marker counted back to 0. All ran in a throwaway detached worktree off this branch's commit, since the file under test is the file the ablation mutates; that worktree is removed.Scope, changeset and labels
skip-changeset, measured rather than assumed. The diff is one file,scripts/check-changeset-no-major.mjs. Resolving every trackedpackage.json(83 tracked, 70 publishable — not private and carrying afiles[]) and asking whichfiles[]entry would ship that path: zero. Positive controls through the same resolver:packages/spec/dist/index.jsresolves to@objectstack/spec'sdistentry andpackages/cli/dist/index.jsto@objectstack/cli's, so the resolver does find a shipped path when one exists;packages/spec/src/index.tscorrectly resolves to nothing. The repo-root manifest isprivate: truewith nofiles. Nothing published moves, so this takes the label and not an empty changeset (workflow route 2), and route 0 does not apply — this PR touches no.changeset/*.mdat all.One measured correction to the card's reproduction table
The card's table gives PR #18959 as
`Clause-②: no`on its own line, backtick-wrapped, reading{"kind":"near-miss","reason":"describing"}. Driven againstorigin/maintoday, that exact line reads{"kind":"declared","value":"no","arm":null}— a declaration, not a near miss.clause2LineDescribes's QUOTED-AND-CONTINUED tell fires only when the backtick span opened at the key continues past its closing tick; a span that closes with nothing after it is not describing. The five PR bodies have all since been corrected, so the historical bytes are no longer readable through the REST API and the exact line #18959 carried could not be recovered — most likely it carried trailing content after the closing tick, which is thedescribingshape and is covered by the #18946 row.This narrows one row of the reproduction table; it moves nothing about the card. Both near-miss spellings the card names are reproduced above from the real reader, the third (
inline-key) with them, and the remedy is unchanged.Out of scope, noted here rather than filed
.github/workflows/pr-automation.ymlis untouched. ItsRequire a changesetstep's::error::is now pinned by this script's self-test, which is a new coupling in the direction the card wants: it is what keeps a missing-changeset refusal distinguishable from this script's.check-empty-changeset.mjsandcheck-adr-0087-registration.mjs— refuse to the job log with no annotation of their own, exactly as this one did. Same shape, different surface, and out of this card's file surface.Generated by Claude Code