test: pin the at-risk gate's floor at one operator #81
Labels
No labels
attention
blocked
blocker:ci-red
blocker:conflict
blocker:drill-pending
blocker:unrequested
bug
claimed
documentation
enhancement
epic
merge-next
needs-ruling
needs-triage
offsite
post-merge
ready
release
scope:bootstrap
scope:coolify
scope:db
scope:docs
scope:drill
scope:installer
scope:labels
scope:platform
scope:runner
scope:users
stale
state:addressing
state:bots-reviewing
state:building
state:needs-human
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: heavy-duty/rig#81
Loading…
Reference in a new issue
No description provided.
Delete branch "test/at-risk-gate-pin"
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?
commands/users-apply.sh's empty-file gate can have its floor moved from-gt 0to-gt 1and the entire suite stays green. That mutation silently un-gates the single-operator box: a users file that revokes the last remaining operator would proceed with no question asked, which is the case the gate exists for. Closes #78.Stacked on #73 — read this first
This branch is #73 plus one commit. The gate it pins is introduced by #73, which is still open, so the pin cannot exist on
mainyet — amain-based version of it would fail on a line that isn't there. The diff shown here therefore includes #73's commit until #73 merges, at which point GitHub collapses this to the singletest:commit below. Nothing in #73 is modified:git diff 9126bbe..HEADtouchestest/cli.shonly, +18/-0.If maintainers would rather this land as part of #73 than behind it, the commit cherry-picks onto that branch cleanly.
Why the two neighbouring pins miss it
Both are in the same block and both look like they should cover this. Neither does:
[ "${#USERS[@]}" -eq 0 ] && [ -r "$LEDGER" ] && [ "$ASSUME_YES" -eq 0 ]. It says the gate is entered for the right file, and nothing about what the gate then decides.AT_RISK … (-gt|-ge) \$— a comparison against a$-variable.-gt 1is a literal, so it slips through.So the suite pinned that the gate exists and that no threshold was introduced, but never where the gate's floor sits — the one number the gate is made of.
The pin, and why not the literal line
The issue suggested
grep -qF 'if [ "$AT_RISK" -gt 0 ]; then'. That catches the mutant, but it also fails on a correct gate that someone respelled${AT_RISK}or re-spaced — a false failure on a refactor is how a pin earns a reputation for being noise and gets deleted. So the pin matches the conditional as a pattern instead:Anchored to the
ifand to end-of-line, so it pins the gate's conditional rather than any line that happens to mentionAT_RISK. Brace-optional, quote-optional, whitespace-tolerant.-ge 1is accepted because it is the same statement in other words, and a pin that fails on a synonym is pinning the spelling, not the contract.Positioned with the counting pins rather than at the end of the block: the count and the floor it is measured against are one statement, and the pin above it (
the count is taken before the message quotes it) already reads as the first half of that pair.What I deliberately did NOT change
(-gt|-ge)[[:space:]]*\$to also match digits would catch-gt 1too, and in one edit — but it would then flag-ge 1, the legitimate spelling the new pin accepts, as a "threshold". The two pins would contradict each other. It would also report a floor regression under the name "partial mass revocation stays ungated (#65 open question)", sending the next reader to the wrong contract. Two contracts, two pins, two names./etc/rig/users; grep-pinning the shipped script is the house precedent for exactly that (the@rootkeyless-seed die, the invoker gate, and every other pin in this block).commands/users-apply.shis untouched. The code is correct; #78 is a regression pin, not a fix.CHANGELOG.mdentry. CONTRIBUTING's rule is that feature PRs land their changelog entry, and the Unreleased section becomes release notes verbatim. This changes no observable behaviour — the gate's entry is already in #73's changelog block, and "we also added a test for it" is not release-notes material. Flagging the judgment call explicitly since skipping the changelog is the kind of thing worth disagreeing with out loud.Evidence — observed, not asserted
Mutation applied to
commands/users-apply.sh(-gt 0→-gt 1), test file untouched:Exactly one failure, and the 419 is the same number #78 reports the mutant passing — direct confirmation that the new pin is the only thing standing between that mutation and a green board.
Mutation reverted:
Reformat tolerance checked the same way, since a pin claiming to survive reformatting should be made to prove it. With the gate rewritten to
if [ "${AT_RISK}" -ge 1 ] ; then— braced, double-spaced, space before the;:The literal-line pin from the issue fails that tree.
Checks
shellcheck -x— CI's exact block replayed locally (shopt -s globstar; files=(bin/* **/*.sh)), all 22 files, exit 0bash test/cli.sh— 420 passed, 0 failed (419 on #73's head; +1 is this pin)bash test/release.sh— 68 passed, 0 failedCloses #78
Verdict: Approve — I agree with this as-is.
Tip
3808758pins the one number the empty-file gate is made of: floor at one at-risk operator. Neighbouring pins miss it deliberately (trigger condition vs.$-variable threshold), and the pattern form correctly accepts-ge 1/${AT_RISK}respelling without calling a legitimate synonym a "threshold." Mutation evidence is load-bearing:-gt 0→-gt 1fails only this check; reformat tolerance proven.Stacked on #73 as stated — no production change, no changelog needed for a pure regression pin. CI green.
Verdict: I agree with everything and have no additional feedback.
The assertion pins the one-operator boundary while allowing equivalent test spellings, and the CLI suite passes (420/0).
✅ Reviewed — I agree with all of this, no concerns.
First review. The gap analysis is precise — the condition grep pins the trigger, the deferred-threshold negative only matches
$-variable comparisons, so a literal-gt 1slid between them — and the pattern-over-literal call is right: a pin that fails on${AT_RISK}or a respacing is pinning spelling, not contract, and earns deletion.I checked the regex against the mutations that matter:
-ge 2,-ge 10,-gt 01,-gt 00all fail to match (the digit must be exactly0/1followed by whitespace before]), so every floor-raising respelling goes red while-ge 1passes as the legitimate synonym. Declining to widen the #65 negative is also correct — it would flag-ge 1as a threshold and send a floor regression to the wrong contract's name. The one-failure mutation evidence (419+1, matching #78's reported count exactly) is the confirmation that this pin alone stands between the mutant and green. Skipping the changelog for a pure regression pin matches my reading of the convention too.— automated review by
claude-bot-andresmgsl· heavy-duty-review-bot✅ Approved — I agree with all of this, no concerns.
Formalizing my earlier ✅ review of this same revision as an approval — reviews from this bot now carry a formal verdict state instead of a comment.
— automated review by
claude-bot-andresmgsl· heavy-duty-review-botRebased onto
mainafter heavy-duty/rig#79, #80 and #83 merged.The conflict was
CHANGELOG.mdonly — those three landed their entries at the same position under## Unreleasedthat this PR writes to. The two sides were disjoint###sections, so they compose: both kept, in order, no prose edited on either side. Every other file rebased clean.Re-verified on the rebased head — suites and
shellcheck -xover CI's exact block (shopt -s globstar; files=(bin/* **/*.sh)) all green.Re-requesting all three bots: the force-push moved head, and the reconciler counts only head-current approvals. Leaving stale ones would let this reach @danmt looking reviewed against a tree that no longer exists. Rebase-only change, so the round should be short.
✅ Approved — I agree with all of this, no concerns.
Re-review of head
03b0131after the rebase onto main. Still #73's commit (now1b16821) plus the one-line floor pin, and the pin itself — pattern-form-g[te]against a literal so-ge 1and${AT_RISK}spellings stay legal — is unchanged from what I approved at3808758. The conflict was CHANGELOG-only.Same CI note as #73: the
reconcilejob on this head reads as failed but was actually cancelled by the labels workflow's concurrency group; the substantivecirun is green and this review triggers a fresh reconcile.— automated review by
claude-bot-andresmgsl· heavy-duty-review-botVerdict: Approve — I agree with this as-is at head
03b0131.Re-review after rebase onto main (prior approve was on
3808758). Stack is#73(1b16821) + the one-line floor pin.-gt 0or-ge 1as "one at-risk operator is enough" (#78); pattern-based so respacing/${AT_RISK}still pass.CI: check + db-integration green; reconcile red expected while stacked on unmerged base. No blockers.
Rebased again — heavy-duty/rig#71 merged after the previous rebase, so this needed one more pass.
CHANGELOG.mdonly; every other file rebased clean.This is the cascade of every open PR writing under
## Unreleased: each merge re-conflicts the ones below it. Suites andshellcheck -xre-verified green at the new head, and the bots are re-requested since the force-push moved head.Verdict: Approve — I agree with this as-is.
Tip
a1a0592is the floor pin rebased with #73 (b0bacd3+ pin). Prior approve was at03b0131.What holds
-gt 0/-ge 1and fails a mutant-gt 1that neighbouring pins missNo remaining blockers. CI green.
Verdict: I agree with everything and have no additional feedback.
✅ Approved — I agree with all of this, no concerns.
Re-review of head
8d4f6daafter the second rebase (CHANGELOG cascade, #71 merging under it). The stack shape is intact — #73's gate commit (now8d03184) plus the one floor-pin commit — and I diffed this PR against #73's: the only delta is the same 18-line pin block intest/cli.sh, with the pattern-form regex unchanged from what I approved at3808758and03b0131. CI is green across all four jobs on this head, reconcile included.— automated review by
claude-bot-andresmgsl· heavy-duty-review-bot