refactor: one drill record per version, in drills/ #139
Labels
No labels
blocked
blocker:ci-red
blocker:conflict
blocker:drill-pending
blocker:unrequested
bug
claimed
documentation
enhancement
epic
merge-next
needs-triage
ready
release
scope:apply
scope:capture
scope:coolify-api
scope:fleet
scope:manifest
scope:secrets
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/cast#139
Loading…
Reference in a new issue
No description provided.
Delete branch "refactor/drills-per-version"
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?
Drill records move from sections inside
drill/RUNS.mdto one file per version:drills/<version>.md.Why — it deletes the bug class, not just tidies
The guard shipped in the previous PR had to parse headings: em-dash field matching, an optional
— DATEtail, whole-version comparison soX.Y.Z-rc1could not satisfyX.Y.Z, avoiding\xescapes because CI runs mawk not gawk, and a non-blank body rule.Every one of those existed only because records shared a file. And that complexity produced two real defects in review, both caught by all three reviewers:
sed '/./,$!d'treated a whitespace-only body as content —.matches a space — so a heading followed by one tab satisfied the gate. An evidence-free release for the price of an invisible character, on the check whose entire job is to demand evidence.## Release drill — 0.2.0 stray words, box did not.One file per version makes nearly all of it unrepresentable.
0.9.0.mdand0.9.0-rc1.mdare simply different files, so whole-version matching is the filesystem's problem rather than a comparison anyone can get wrong. There is no heading left to parse, so there is no grammar left to drift.What survives is the one rule that was never really about parsing: a file of only whitespace is not a record.
[ -f ]is happy with it, so existence alone is a claimtouchcan defeat.The guard is now:
Executable logic dropped by roughly two thirds; the file barely shrank because the comments now carry the history of why the complexity is gone.
drills/, not.drills/Deliberate. A dot-directory is invisible to a glob without
dotglob, which is exactly the blind spot behind box#116/#118 and rig#70 — a sweep that reads green because it never descended into the directory holding the thing it was meant to check. Evidence a sweep cannot see is evidence that goes missing quietly.The framing correction that rides along
The docs previously described the drill as one orchestrated stack run, implying the three repos must be drilled together in sequence. That over-constrains it and is now corrected everywhere:
mainis that afternoon.Substrate-before-guest is framed as how you run a single drill, not an inter-repo ordering rule. Tests pin the corrected claims, including a negative that fails if fixed-order language returns.
Verification
Per-repo results are in the commit; all suites green, all guards pass on their own tree, shellcheck sweeps clean with the rewritten script covered.
Hand-verified in a
mktempdir, not just via fixtures: a bare-version tree with nodrills/exits 1 with the full unblock message; one with a real record exits 0.Note on the open release PR
It is currently red on
drill-recorded— correctly, since no drill has been run. This PR changes where the record goes, not whether one is required, so it stays red until the drill lands. It carriesblocker:drill-pending.Verdict: Approve — I agree with this as-is.
First review. Tip moves drill evidence from section-in-
drill/RUNS.mdto one file per version atdrills/<version>.md.What holds
0.9.0.mdvs0.9.0-rc1.md).[ ! -f ] || ! grep -q '[^[:space:]]'— verified spaces/tabs-only file fails; content passes; missing file fails;-devvacuous with no dir.drills/not.drills/— correct lesson from #118 / shellcheck globstar blind spot.drill/RUNS.mdas harness log distinct from release evidence — docs and failure text say so.No blockers.
Tip
e690c89. package.json version via sed; fixtures intest/release.test.tscover whitespace (#138) and rc isolation.Pushed a second commit after a real defect surfaced the moment the gate met a release branch.
What went wrong
The gate PR's self-test asserted the guard always passes on our own tree:
On a
-devtree the guard is vacuous, so it passed and looked fine. On a ceremony tree it cannot pass until a human has run the drill and written the record — which is the entire point of the gate.Consequences, both real:
test/release.shbecame un-greenable on every release branch before its drill. An author preparing a release could not get a green suite locally, through no fault of their own.release-flow testsfailure rather than as the drill gate doing its job. The signal was misattributed — you would go looking for a broken test.Observed live: rebasing the release PR onto the gated
mainturned it red onrelease-flow tests, not ondrill-recorded. That is what sent me looking.This is the same shape as #146 — a check coupled to the repo's real
VERSIONthat behaves one way on-devand another on the ceremony tree, so it only misbehaves during a release. I reintroduced it in the very PR that fixed the last instance.The fix
Assert the property that is actually true: the guard's verdict is correct for this tree, not that it always passes.
-devtree → vacuous, must passGreen in all three states, and still red if the guard ever disagrees with the tree in front of it — which is the thing worth testing.
The sibling "default arguments, as CI runs it" check got the same treatment: it now asserts the defaults reach the same verdict as the explicit call, rather than a hard-coded 0. What that test is for is proving the defaults are the paths this repo uses, and comparing verdicts says that without re-importing the bug.
Verified in both states, not just the current one
Each suite was run on its
-devtree and again on a simulated full ceremony tree — bare version and stamped changelog, no drill record:-devtreeAnd the gate still refuses the undrilled ceremony tree — that assertion is now what the suite checks, rather than something the suite trips over.
Full sweeps re-run: box and rig shellcheck clean under CI's exact globstar/dotglob invocation; cast
npm test784, biome clean.✅ Approved — I agree with all of this, no concerns.
Same design as box#151 and equally sound here. Deleting cast's drill/RUNS.md is the right call — it held only format documentation, never a record, and a stale second place to write records nothing reads would be a trap; its useful reasoning survives in drills/README.md with the 9.9.9 placeholder kept un-shippable. The guard keeps the usage check and the package.json version source, and the whitespace rule is the direct
grep -q '[^[:space:]]'with nothing in front of it to get wrong. Fixture coverage of the new failure modes is complete (no dir, others-only dir including a README.md decoy, empty file, whitespace-only, rc↔final both directions, each version still matching its own record), and the doc pins for the independent, ref-pinned drills match the siblings.— automated review by
claude-bot-andresmgsl· heavy-duty-review-botVerdict: I agree with everything and have no additional feedback.
The per-version record layout removes the heading-parser failure modes, preserves the non-whitespace requirement, and the revised real-tree test correctly distinguishes development and ceremony states. Current checks are green.
Verdict: Approve — I agree with this as-is.
Re-review on tip
9e6f343(prior APPROVE was one690c89). Second commit same family fix as box#151: real-tree test asserts verdict matches tree state, not unconditional exit 0 — so a ceremony branch is greenable before the drill and the failure attributes to the gate.What holds at tip
drills/<ver>.md+grep -q '[^[:space:]]'; usage check and package.json version source preserved.drill/RUNS.mdis correct here (docs-only; would be a second write path nothing reads); reasoning lives indrills/README.mdwith unshippable9.9.9example.No blockers. Tip
9e6f343.@danmt — round closed, three head-current approvals, zero changes requested. Ready to merge.
What it does
Drill records move from sections inside
drill/RUNS.mdto one file per version:drills/<version>.md.It deletes a bug class rather than tidying one. The heading parser existed only because records shared a file — em-dash field matching, an optional date tail, whole-version comparison, mawk-vs-gawk escapes, a non-blank body rule — and that complexity produced two defects in review (a
sed '/./,$!d'whitespace bypass where.matches a space, and heading-grammar drift between siblings). One file per version makes nearly all of it unrepresentable:X.Y.Z.mdandX.Y.Z-rc1.mdare simply different files.What survives is the one rule that was never about parsing: a file of only whitespace is not a record, since
[ -f ]is a claimtouchcan defeat.Two defects found and fixed during review, worth knowing about
1. The self-test asserted the guard always passes on our own tree. Vacuous on
-dev, so it looked fine — but a ceremony tree cannot pass until a human drills. That madetest/release.shun-greenable on every release branch, and made CI report arelease-flow testsfailure instead of the gate doing its job. Same shape as #146, reintroduced in the PR that fixed #146's last instance. Now asserts the verdict matches the tree:-devpasses, ceremony-with-record passes, ceremony-without-record is refused.2. (rig only) A comment misstated
set -esemantics — claimed the miss path ofa && baborts mid-script, when the LHS of&&is exempt and the opposite case is the hazard. Fixed by adopting box's and cast's form, so all three guards are now byte-identical in that construct. Cross-checked: this repo never had that claim.Verified in both tree states
Not just the current one — each suite was run on its
-devtree and on a simulated full ceremony tree (bare version and stamped changelog, no drill record):-devGuard behaviour re-verified by hand across all five states (no dir / dir-no-file / whitespace-only / real record /
-dev), and the sweeps are clean with the rewritten script covered.Note on the release PR in this repo
It is red on the drill gate and carries
blocker:drill-pending— correctly, since no drill has been run. This PR changes where a record goes, not whether one is required, so merging it does not unblock the release. That stays parked until the drill is recorded or explicitly waived.