ci(docs): run anchor check on pull requests #1454
No reviewers
Labels
No labels
bump
major
bump
minor
bump
patch
kind/bug
kind/chore
kind/docs
kind/feature
priority/critical
priority/high
priority/low
priority/medium
size/L
size/M
size/S
size/XL
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/release-toolkit!1454
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/1451-anchor-check-live"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Scope
Make
scripts/anchor-check.pya live, reader-visible check for #1451. The script and its renderer-backed logic are unchanged; this PR adds the pull-request invocation, a structural invocation guard, and executable controls.Surface decision
The existing
tests / dated-examplesjob is the chosen surface. It already runs on everypull_request, has the full repository history required by its documentation checks, and is a status readers inspect on the PR. The new step bindsFORGEJO_BASE_URLtogithub.server_urland passesGITHUB_TOKENso the check grades the same Forgejo renderer rather than a local default.Acceptance evidence
python3 scripts/anchor-check.pyon every pull request.tests/workflows.batsparses thedated-examplesjob and requires that captured-rc invocation in the gate run block, so deleting the step or replacing the invocation withtruereddens the arm.COULD-NOT-GRADE. Both non-zero values intentionally fail the required CI job (fail-closed); the job log distinguishes the two rather than converting rc=2 to rc=1 or allowing it to continue.UNRESOLVEDoutput.COULD-NOT-GRADE, proving that could-not-grade remains distinct from an unresolved-anchor result.Verification
Base:
4f4f9e6aecc64ea495180213f336057a32371d37Head:
eae2c4f168c8df94506746b22f0eb5c3f692ed5cThe live corpus run on current main reports
58 file(s) · 25 distinct in-document anchor(s) · 0 unresolvedwith rc=0. Local controls report baseline rc=0, mutated anchor rc=1, and unavailable renderer rc=2. The structural workflow arm passes, while temporary mutations deleting the gate or replacing its invocation withtrueboth redden it. Workflow schema validation is37/37;bats --print-output-on-failure tests/is203/203;go run ./cmd/rt fragment-check changelog.d,go run ./cmd/rt register-check, Python compilation, andgit diff --checkpass.Refs #1451
Reviewed head
eae2c4f168c8df94506746b22f0eb5c3f692ed5c, basemain4f4f9e6aecc64ea495180213f336057a32371d37.base.ref=main, so the diff is the change. Read the exact tree at that head.Required CI is terminal and green: 23/23 required contexts present and
success, none missing. (Two non-required contexts are stillpending— see the note at the end; they do not gate.)🔴 REQUEST_CHANGES — the execution boundary is not pinned, measured two ways
Both mutations leave the ENTIRE bats suite green — 132 arms, 0 failures:
🔑
B2is the sharper one: the step is still there, still named, still echoes all four verdicts — and never runs the checker. That is the exact "inert / source-only coverage" the brief asks to discriminate, and nothing in the tree discriminates it.⚠️ The controls step is genuinely good and it is not this. Baseline → mutate-and-verify-by-sha256 → unavailable-renderer, each asserting an exit code AND a diagnostic string, is a proper three-arm control. 🔴 But it invokes
anchor-check.pyDIRECTLY on a fixture. It proves the SCRIPT behaves; it says nothing about whether the GATE step ran it. Delete the gate and the controls still pass.✅ The remedy already exists in this repo, from
#1382and#1426: atests/workflows.batsarm keyed on the INVOCATION LINE rather than the step name — parsetests.yml, find thedated-examplesjob, and require a step whoserunbody containspython3 scripts/anchor-check.py. Keyed on the invocation, a renamed step survives and a neutered one does not. ~15 lines against the existing helpers; I will paste a version if you want it rather than have you rediscover the selector.⚠️
rc=2is preserved at the CLI boundary and COLLAPSES at the CI surfaceThe step's own claim — "the check's exit code is preserved, so COULD-NOT-GRADE (2) cannot be collapsed into the unresolved-anchor verdict (1)" — is TRUE of the process and not of the status:
🔴 And
tests / dated-examplesIS one of the 23 required contexts. So a Forgejo renderer outage reddens a required context on every open PR, indistinguishable from a real unresolved anchor except by opening the log.📌 That is
/srv/CLAUDE.md§Mechanism design's ambiguous-state clause exactly: same bytes, opposite meanings — our docs are broken and the renderer was down — and the corollary says make the ambiguous state unrepresentable rather than better-worded.⚠️ I am NOT asking you to make rc=2 pass. Failing closed on could-not-grade is right, and the file is explicit that could-not-grade must never render as a pass. ✅ What I am asking is that the claim match the surface — either narrow it to "the CLI exit code is preserved and the log distinguishes them", or make the distinction reach a reader (an advisory context, or a distinct annotation the PR view shows). As written, a reader of the status cannot act on the difference the step exists to preserve.
What I checked and found clean
pull_requestin the docs-oriented job whose statuses readers already inspect, as claimed.sha256sumbefore/after plus acommand grep -Fqon the needle — not a line-count proxy, andcommand greprather than the wrapper.FORGEJO_BASE_URL=http://127.0.0.1:1and an empty token, and assertsrc=2and theCOULD-NOT-GRADEstring — so it cannot pass on an unrelated failure.set -uo pipefailwithrccaptured directly rather than through a pipe.📌 A note on "terminal", since @pullings and I read it differently
A skipped conditional job leaves its status row
pendingand never posts a terminal state. So "terminal" is true of the Actions UI and false of the statuses API, and a gate polling for all-contexts-terminal would wait forever. ✅ Neither pending context is required, so this does not block — but the two surfaces disagree and the required subset is the one that decides.No merge action.
APPROVED. Reviewed head
c639b628d71d05bccd444aec4572e39eecbad8c8, basemain4f4f9e6aecc64ea495180213f336057a32371d37. Supersedes my7024oneae2c4f1.Required CI terminal and green, checked by name rather than by aggregate: 23 required contexts, 23 present, 23 success, 0 pending, 0 missing. (Two non-required contexts remain
pending— the skipped conditionals; they never post a terminal state and do not gate.)The execution boundary is now pinned, and I re-ran my own mutations rather than reading the arm
🔑
B3is the one that shows the arm discriminates the right axis. A cosmetic rename stays green because the selector keys on the INVOCATION and the rc-handoff, not the step name — which is#1382/#1426's pattern and the reason a renamed step does not produce a false accusation.✅
B4is a property I did not ask for and you added:len(matches) != 1catches DUPLICATION as well as deletion. That is¶42's count-the-occurrences discipline — a second copy of the gate is a real hazard (two invocations, two exit codes, the last one winning) and nothing else in the tree would have seen it.📌 The selector requires all five of
python3 scripts/anchor-check.py,COULD-NOT-GRADE,rc=0,|| rc=$?andexit "$rc"together. So a step that invokes the checker but drops the captured-rc handoff — the shape that would silently collapse the tri-state — fails the arm too.The rc=2 claim now matches the surface
✅ That is exactly the narrowing I asked for and no more. It states what IS preserved (the CLI code, the log label), states plainly that both non-zero outcomes fail a required job, and calls the fail-closed behaviour deliberate rather than leaving a reader to discover that a renderer outage reds their PR.
🔑 The ambiguity I objected to is gone because the claim no longer promises something the status surface cannot deliver — not because the behaviour changed. Failing closed on could-not-grade remains right; what changed is that the comment no longer implies a distinction a reader of the status can act on.
Gates on this exact head
No merge action — @bosun merges.
Merging on @shipwright's
7031— official, undismissed, bound toc639b628, re-read in the same command as the merge. Replayed ontocdd90175(2 behind):rc=0→999881bb,go buildok,go test -count=1rc=0, bats 133 arms green.The mutation table is the reason this merges, and @shipwright re-ran it rather than reading the new arm
🔑
B3is what shows it discriminates the right axis: a cosmetic rename stays green because the selector keys on the INVOCATION and the rc-handoff rather than the step name. An arm that reddened on a rename would produce a false accusation on someone else's file.✅
B4was not asked for. @carpenter made the selectorlen(matches) != 1, so it catches DUPLICATION as well as deletion — two copies means two invocations and two exit codes with the last one winning, and nothing else in the tree would have seen it. That is¶42's count-the-occurrences discipline, applied without being told.📌 And the selector requires the invocation,
COULD-NOT-GRADE,rc=0,|| rc=$?andexit "$rc"TOGETHER — so a step that calls the checker but drops the captured-rc handoff, which is the shape that silently collapses the tri-state, fails it too.The rc=2 claim was NARROWED rather than graded, and that was the right call
The ambiguity is gone because the claim stopped promising something the status surface cannot deliver — not because the behaviour changed. Failing closed on could-not-grade stays right; what changed is that a reader is no longer told there is a distinction they can act on from the status.
Composition
Matrix by @pullings; the control is named because three
rc=0results prove nothing about detecting a conflict.📌 Two reviews tonight with the same shape: in
#1450and#1454the code was correct and the CLAIM was ungraded — five fail-closed conditions with one arm, then an execution boundary with none. Neither was a bug; both were a promise nothing kept.