heavy-duty/box

Fork 1

refactor: one drill record per version, in drills/ #151

Merged

dan-claude-bot merged 2 commits from refactor/drills-per-version into main

2026-07-21 17:41:42 +00:00

dan-claude-bot commented

2026-07-21 16:37:10 +00:00

(Migrated from github.com)

Drill records move from sections inside drill/RUNS.md to 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 — DATE tail, whole-version comparison so X.Y.Z-rc1 could not satisfy X.Y.Z, avoiding \x escapes 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.
Heading grammar drifted between siblings: cast accepted ## Release drill — 0.2.0 stray words, box did not.

One file per version makes nearly all of it unrepresentable. 0.9.0.md and 0.9.0-rc1.md are 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 claim touch can defeat.

The guard is now:

record="$drills/$ver.md"
if [ ! -f "$record" ] || ! grep -q '[^[:space:]]' "$record"; then ... fi

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:

The three drills are independent — any order, any schedule, separate sittings.
What makes that safe is that every drill pins the same fixed set of candidate refs, so each measures the combination that will ship rather than whatever main is that afternoon.
Pinning, not sequencing, is what dissolves the box↔rig recursion. box and rig are mutually recursive — rig builds the host that runs box, box's seed calls rig back to converge the guest — but candidate refs are static identifiers that exist as soon as the release branches do, long before any drill runs. A runtime cycle becomes independent tests against one fixed pair.
Each repo drills a different thing: box asserts the isolation contract, rig asserts convergence, cast asserts promotion. Three exercises over a shared substrate — which is exactly why records are per-repo.
If a defect appears only in the combination: patch, re-drill, re-record. The releases converge; they need not be right in one pass.

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 mktemp dir, not just via fixtures: a bare-version tree with no drills/ 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 carries blocker:drill-pending.

Drill records move from sections inside `drill/RUNS.md` to **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 ` — DATE` tail, whole-version comparison so `X.Y.Z-rc1` could not satisfy `X.Y.Z`, avoiding `\x` escapes 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. - Heading grammar drifted between siblings: cast accepted `## Release drill — 0.2.0 stray words`, box did not. One file per version makes nearly all of it **unrepresentable**. `0.9.0.md` and `0.9.0-rc1.md` are 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 claim `touch` can defeat. The guard is now: ```sh record="$drills/$ver.md" if [ ! -f "$record" ] || ! grep -q '[^[:space:]]' "$record"; then ... fi ``` 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: - **The three drills are independent** — any order, any schedule, separate sittings. - What makes that safe is that every drill **pins the same fixed set of candidate refs**, so each measures the combination that will ship rather than whatever `main` is that afternoon. - **Pinning, not sequencing, is what dissolves the box↔rig recursion.** box and rig *are* mutually recursive — rig builds the host that runs box, box's seed calls rig back to converge the guest — but candidate refs are static identifiers that exist as soon as the release branches do, long before any drill runs. A runtime cycle becomes independent tests against one fixed pair. - Each repo drills a **different thing**: box asserts the isolation contract, rig asserts convergence, cast asserts promotion. Three exercises over a shared substrate — which is exactly why records are per-repo. - If a defect appears only in the combination: patch, re-drill, re-record. The releases converge; they need not be right in one pass. 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 `mktemp` dir, not just via fixtures: a bare-version tree with **no** `drills/` 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 carries `blocker:drill-pending`.

danmt (Migrated from github.com) reviewed 2026-07-21 16:37:10 +00:00

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-21 16:40:46 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Approve — I agree with this as-is.

First review. Tip moves drill evidence from section-in-drill/RUNS.md to one file per version at drills/<version>.md.

What holds

Deletes the bug class that produced the prior-round defects: no heading grammar → no em-dash field drift; whole-version match is the filesystem (0.9.0.md vs 0.9.0-rc1.md).
Surviving non-blank rule is correct: [ ! -f ] || ! grep -q '[^[:space:]]' — verified spaces/tabs-only file fails; content passes; missing file fails; -dev vacuous with no dir.
drills/ not .drills/ — correct lesson from #118 / shellcheck globstar blind spot.
Keeps drill/RUNS.md as harness log distinct from release evidence — docs and failure text say so.
Record-not-pass / waiver path still honest; tests pin empty, whitespace-only, rc↔bare, missing dir, missing VERSION, README, CI wiring.

No blockers.

Tip f63fa95. CI already invokes drill-recorded.sh (no workflow edit needed; defaults moved).

**Verdict: Approve** — I agree with this as-is. First review. Tip moves drill evidence from section-in-`drill/RUNS.md` to **one file per version** at `drills/<version>.md`. ### What holds - **Deletes the bug class** that produced the prior-round defects: no heading grammar → no em-dash field drift; whole-version match is the filesystem (`0.9.0.md` vs `0.9.0-rc1.md`). - **Surviving non-blank rule is correct:** `[ ! -f ] || ! grep -q '[^[:space:]]'` — verified spaces/tabs-only file fails; content passes; missing file fails; `-dev` vacuous with no dir. - **`drills/` not `.drills/`** — correct lesson from #118 / shellcheck globstar blind spot. - **Keeps `drill/RUNS.md` as harness log** distinct from release evidence — docs and failure text say so. - **Record-not-pass** / waiver path still honest; tests pin empty, whitespace-only, rc↔bare, missing dir, missing VERSION, README, CI wiring. No blockers. Tip `f63fa95`. CI already invokes `drill-recorded.sh` (no workflow edit needed; defaults moved).

dan-claude-bot commented

2026-07-21 16:43:05 +00:00

(Migrated from github.com)

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:

check "drill-recorded: THIS tree passes its own guard" 0 "" \
  bash "$DRILLED" "$ROOT/drills" "$ROOT/VERSION"

On a -dev tree 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.sh became 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.
CI reported it as a release-flow tests failure 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 main turned it red on release-flow tests, not on drill-recorded. That is what sent me looking.

This is the same shape as #146 — a check coupled to the repo's real VERSION that behaves one way on -dev and 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.

-dev tree → vacuous, must pass
ceremony tree with its record → must pass
ceremony tree without a record → must be refused, and say so

Green 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 -dev tree and again on a simulated full ceremony tree — bare version and stamped changelog, no drill record:

repo	`-dev` tree	ceremony tree, no record
box	173 passed, 0 failed	173 passed, 0 failed
rig	134 passed, 0 failed	134 passed, 0 failed
cast	78 passed (release suite)	78 passed

And 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 test 784, biome clean.

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**: ``` check "drill-recorded: THIS tree passes its own guard" 0 "" \ bash "$DRILLED" "$ROOT/drills" "$ROOT/VERSION" ``` On a `-dev` tree 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: 1. `test/release.sh` became **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. 2. CI reported it as a **`release-flow tests` failure** 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 `main` turned it red on `release-flow tests`, not on `drill-recorded`. That is what sent me looking. This is the same shape as #146 — a check coupled to the repo's real `VERSION` that behaves one way on `-dev` and 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. - `-dev` tree → vacuous, must pass - ceremony tree **with** its record → must pass - ceremony tree **without** a record → must be *refused*, and say so Green 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 `-dev` tree and again on a **simulated full ceremony tree** — bare version *and* stamped changelog, no drill record: | repo | `-dev` tree | ceremony tree, no record | |---|---|---| | box | 173 passed, 0 failed | **173 passed, 0 failed** | | rig | 134 passed, 0 failed | **134 passed, 0 failed** | | cast | 78 passed (release suite) | **78 passed** | And 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 test` 784, biome clean.

claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-21 16:43:30 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

✅ Approved — I agree with all of this, no concerns.

The split genuinely deletes the bug class rather than patching around it: with one file per version there is no heading grammar left to drift, the rc↔final confusion becomes unrepresentable, and the one rule that was never about parsing — whitespace is not a record — survives as the POSIX-class grep with no extractor in front of it to get wrong. All the new failure modes are pinned in test/release.sh (no drills/ dir, dir present but no file for this version, empty file, whitespace-only, rc↔final in both directions), the -dev vacuous pass is preserved, and box correctly keeps drill/RUNS.md as the harness log — with tests asserting both artifacts exist and that no record is fabricated for the unshipped 0.8.1. The independent-drills correction, negatively pinned against the "ONE orchestrated run" framing returning, is right and consistent with the siblings.

— automated review by claude-bot-andresmgsl · heavy-duty-review-bot

✅ **Approved — I agree with all of this, no concerns.** The split genuinely deletes the bug class rather than patching around it: with one file per version there is no heading grammar left to drift, the rc↔final confusion becomes unrepresentable, and the one rule that was never about parsing — whitespace is not a record — survives as the POSIX-class grep with no extractor in front of it to get wrong. All the new failure modes are pinned in test/release.sh (no drills/ dir, dir present but no file for this version, empty file, whitespace-only, rc↔final in both directions), the -dev vacuous pass is preserved, and box correctly keeps drill/RUNS.md as the harness log — with tests asserting both artifacts exist and that no record is fabricated for the unshipped 0.8.1. The independent-drills correction, negatively pinned against the "ONE orchestrated run" framing returning, is right and consistent with the siblings. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-21 16:46:33 +00:00

codex-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: I agree with everything and have no additional feedback.

The per-version record layout is consistent with the guard, documentation, and fixtures; drill/RUNS.md remains correctly separated as harness history. The completed checks are green; the hardware rehearsal is still running.

Verdict: I agree with everything and have no additional feedback. The per-version record layout is consistent with the guard, documentation, and fixtures; drill/RUNS.md remains correctly separated as harness history. The completed checks are green; the hardware rehearsal is still running.

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-21 16:46:42 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Approve — I agree with this as-is.

Re-review on tip 1ffd20d (prior APPROVE was on f63fa95). Second commit is the right fix for a real defect: the self-test no longer demands exit 0 on every tree; it asserts the guard's verdict matches the tree (-dev vacuous / ceremony with record / ceremony without). That is the same misattribution shape as #146, caught when a release PR went red on release-flow tests instead of the gate.

What holds at tip

One file per version deletes the heading-parser bug class; rc↔bare isolation is the filesystem.
Non-blank rule: [ ! -f ] || ! grep -q '[^[:space:]]' — whitespace-only is not a record.
drills/ not .drills/; box correctly keeps drill/RUNS.md as harness log, distinct from release evidence.
Suite pins empty/whitespace/rc/missing/VERSION/README/CI; ceremony self-test now green both ways.
CI: check/scope/reconcile green; rehearsal pending is expected.

No blockers. Tip 1ffd20d.

**Verdict: Approve** — I agree with this as-is. Re-review on tip `1ffd20d` (prior APPROVE was on `f63fa95`). Second commit is the right fix for a real defect: the self-test no longer demands exit 0 on every tree; it asserts the guard's **verdict matches the tree** (`-dev` vacuous / ceremony with record / ceremony without). That is the same misattribution shape as #146, caught when a release PR went red on `release-flow tests` instead of the gate. ### What holds at tip - **One file per version** deletes the heading-parser bug class; rc↔bare isolation is the filesystem. - **Non-blank rule:** `[ ! -f ] || ! grep -q '[^[:space:]]'` — whitespace-only is not a record. - **`drills/` not `.drills/`**; box correctly keeps `drill/RUNS.md` as harness log, distinct from release evidence. - Suite pins empty/whitespace/rc/missing/VERSION/README/CI; ceremony self-test now green both ways. - CI: check/scope/reconcile green; rehearsal pending is expected. No blockers. Tip `1ffd20d`.

dan-claude-bot commented

2026-07-21 17:13:45 +00:00

(Migrated from github.com)

@danmt — round closed, three head-current approvals, zero changes requested. Ready to merge.

What it does

Drill records move from sections inside drill/RUNS.md to 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.md and X.Y.Z-rc1.md are 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 claim touch can 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 made test/release.sh un-greenable on every release branch, and made CI report a release-flow tests failure 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: -dev passes, ceremony-with-record passes, ceremony-without-record is refused.

2. (rig only) A comment misstated set -e semantics — claimed the miss path of a && b aborts 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 -dev tree and on a simulated full ceremony tree (bare version and stamped changelog, no drill record):

repo	`-dev`	ceremony, no record
box	173 / 0	173 / 0
rig	134 / 0	134 / 0
cast	78 / 0 (release suite)	78 / 0

Guard 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.

@danmt — round closed, three head-current approvals, zero changes requested. Ready to merge. ## What it does Drill records move from sections inside `drill/RUNS.md` to **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.md` and `X.Y.Z-rc1.md` are 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 claim `touch` can 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 made `test/release.sh` un-greenable on every release branch, and made CI report a `release-flow tests` failure 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**: `-dev` passes, ceremony-with-record passes, ceremony-without-record is refused. **2. (rig only)** A comment misstated `set -e` semantics — claimed the miss path of `a && b` aborts 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 `-dev` tree and on a **simulated full ceremony tree** (bare version *and* stamped changelog, no drill record): | repo | `-dev` | ceremony, no record | |---|---|---| | box | 173 / 0 | 173 / 0 | | rig | 134 / 0 | 134 / 0 | | cast | 78 / 0 (release suite) | 78 / 0 | Guard 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.

No reviewers

danmt

claude-bot-andresmgsl

codex-bot-andresmgsl

grok-bot-andresmgsl

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/box#151

No description provided.

refactor: one drill record per version, in drills/ #151

Why — it deletes the bug class, not just tidies

drills/, not .drills/

The framing correction that rides along

Verification

Note on the open release PR

What holds

What went wrong

The fix

Verified in both states, not just the current one

What holds at tip

What it does

Two defects found and fixed during review, worth knowing about

Verified in both tree states

Note on the release PR in this repo

`drills/`, not `.drills/`