heavy-duty/box

Fork 1

fix: drill/wipe.sh reads ufw into a capture, not into an early-exit reader #120

Merged

dan-claude-bot merged 1 commit from fix/wipe-sigpipe-shape into main

2026-07-21 10:55:33 +00:00

dan-claude-bot commented

2026-07-19 23:37:42 +00:00

(Migrated from github.com)

The shape

drill/wipe.sh:120 piped ufw status straight into an early-exit reader:

if command -v ufw >/dev/null && sudo ufw status 2>/dev/null | grep -q "Status: active"; then

That is #102's shape exactly. Status: active is the first line ufw
prints, so grep -q matches it and exits immediately, closing the read end
while ufw is still writing the table. ufw dies of SIGPIPE and the pipeline
yields 141.

Two more hazards rode along, both flagged in #107 and both confirmed on the
tree: the delete loop at :122 used the same early-exit reader as its
condition, so it could end while rules remained; and :123 re-read
un-captured to extract the rule number.

Why it was latent, not broken

drill/wipe.sh:27 is set -u with no pipefail. The 141 is discarded,
grep's 0 is the pipeline's result, and the branch held. There was no bug to
observe.

It was one line from being wrong. Adding set -o pipefail — a change anyone
would reasonably make for unrelated robustness, and which reads as an obvious
improvement in review — silently converts this into #102: every UFW removal
skipped on a host the operator was told is wiped, no error, no red X, because
a branch condition never trips errexit.

Measured on a shim writer (deterministic SIGPIPE, unlike real ufw's ~2%):

	old shape
`set -u -o pipefail`	5/5 runs took the wrong branch
`set -u` (today's wipe.sh)	3/3 runs took the right branch

Which is the claim in both directions: correct today, wrong the moment
pipefail appears.

The fix

#106's pattern, transplanted verbatim from host/teardown-host.sh:77-95
rather than reinvented — capture ufw_status once, match with
[[ == *"Status: active"* ]], and rewrite the delete loop as a while : that
reads one capture per iteration and breaks on absence. The re-scan stays
per-delete (ufw renumbers after each removal); it just no longer races. The
two files now read alike, which is the point of transplanting.

Removals keep this file's cmd && say "did X" idiom — the file is one long
run of those and carries # shellcheck disable=SC2015 at :26 for it.

The pin — generalized to the class

Yes, generalized. The issue asked for a pin; the existing one at
test/cli.sh:1915-1923 covered host/teardown-host.sh alone. This
replaces that block with a sweep over every host/*.sh and drill/*.sh,
naming the offending files in the failure output.

Swept rather than per-file for the reason this issue exists at all: absence
of pipefail is what made wipe.sh survive the shape, so any file is one
set -o pipefail from being #102 again. A new script in either directory now
inherits the pin for free instead of being one more site someone has to
remember.

The comment-stripping wrinkle (grep -vE "^[[:space:]]*#") is carried over
and matters more now: this PR's own explanatory comment quotes the racing
shape to explain it, and a prose-blind pin would fail on the very comment
documenting why it exists.

The positive pins — the capture present, the loop breaking on absence — now
run per file over both host/teardown-host.sh and drill/wipe.sh, so the
sweep cannot be satisfied by deleting a block instead of fixing it.

Merge-order note for #113: that PR concurrently adds a test near the
tail of test/cli.sh. This PR replaces the :1915-1923 block with the
sweep, in the same region. Whichever lands second will want to reconcile
that hunk by hand — the conflict is textual, not semantic; the sweep
subsumes the old three checks.

Also checked

drill/doctor.sh needs nothing, confirming #107's own correction to the
earlier scoping: :285 already reads ufw_out="$(sudo ufw status 2>/dev/null)"
and matches the captured text. Safe by construction, not by absent
pipefail. A sweep of host/*.sh drill/*.sh found drill/wipe.sh as the
sole remaining site, matching the issue.

Verification

shellcheck drill/wipe.sh — clean
Full CI invocation (shopt -s globstar; shellcheck -x bin/* **/*.sh) — clean
bash test/cli.sh — 477 passed, 0 failed
bash test/labels-reconcile.sh — 19 passed, 0 failed
.github/scripts/changelog-armed.sh — VERSION 0.8.1-dev agrees with ## Unreleased
Sweep bites: re-adding the racing line to drill/wipe.sh fails the pin
with racing ufw reads in: drill/wipe.sh, then passes again on revert.
Behavioral: the fixed block driven against a ufw shim (active status,
3 rules, 3000 trailing lines to force the SIGPIPE window) under
pipefail takes the branch, deletes all 3 rules across both nets,
terminates, exit 0.

CHANGELOG.md entry added under ## Unreleased.

Closes #107

## The shape `drill/wipe.sh:120` piped `ufw status` straight into an early-exit reader: ```sh if command -v ufw >/dev/null && sudo ufw status 2>/dev/null | grep -q "Status: active"; then ``` That is #102's shape exactly. `Status: active` is the **first** line ufw prints, so `grep -q` matches it and exits immediately, closing the read end while ufw is still writing the table. ufw dies of SIGPIPE and the pipeline yields 141. Two more hazards rode along, both flagged in #107 and both confirmed on the tree: the delete loop at `:122` used the same early-exit reader **as its condition**, so it could end while rules remained; and `:123` re-read un-captured to extract the rule number. ## Why it was latent, not broken `drill/wipe.sh:27` is `set -u` with **no `pipefail`**. The 141 is discarded, `grep`'s 0 is the pipeline's result, and the branch held. There was no bug to observe. It was one line from being wrong. Adding `set -o pipefail` — a change anyone would reasonably make for unrelated robustness, and which reads as an obvious improvement in review — silently converts this into #102: every UFW removal skipped on a host the operator was told is wiped, no error, no red X, because a branch condition never trips `errexit`. Measured on a shim writer (deterministic SIGPIPE, unlike real ufw's ~2%): | | old shape | |---|---| | `set -u -o pipefail` | **5/5 runs took the wrong branch** | | `set -u` (today's wipe.sh) | 3/3 runs took the right branch | Which is the claim in both directions: correct today, wrong the moment `pipefail` appears. ## The fix #106's pattern, **transplanted verbatim from `host/teardown-host.sh:77-95`** rather than reinvented — capture `ufw_status` once, match with `[[ == *"Status: active"* ]]`, and rewrite the delete loop as a `while :` that reads one capture per iteration and breaks on absence. The re-scan stays per-delete (ufw renumbers after each removal); it just no longer races. The two files now read alike, which is the point of transplanting. Removals keep this file's `cmd && say "did X"` idiom — the file is one long run of those and carries `# shellcheck disable=SC2015` at `:26` for it. ## The pin — generalized to the class **Yes, generalized.** The issue asked for a pin; the existing one at `test/cli.sh:1915-1923` covered `host/teardown-host.sh` alone. This **replaces that block** with a sweep over every `host/*.sh` and `drill/*.sh`, naming the offending files in the failure output. Swept rather than per-file for the reason this issue exists at all: absence of `pipefail` is what made `wipe.sh` survive the shape, so any file is one `set -o pipefail` from being #102 again. A new script in either directory now inherits the pin for free instead of being one more site someone has to remember. The comment-stripping wrinkle (`grep -vE "^[[:space:]]*#"`) is carried over and **matters more now**: this PR's own explanatory comment quotes the racing shape to explain it, and a prose-blind pin would fail on the very comment documenting why it exists. The positive pins — the capture present, the loop breaking on absence — now run per file over both `host/teardown-host.sh` and `drill/wipe.sh`, so the sweep cannot be satisfied by deleting a block instead of fixing it. > **Merge-order note for #113:** that PR concurrently adds a test near the > tail of `test/cli.sh`. This PR **replaces** the `:1915-1923` block with the > sweep, in the same region. Whichever lands second will want to reconcile > that hunk by hand — the conflict is textual, not semantic; the sweep > subsumes the old three checks. ## Also checked `drill/doctor.sh` needs nothing, confirming #107's own correction to the earlier scoping: `:285` already reads `ufw_out="$(sudo ufw status 2>/dev/null)"` and matches the captured text. Safe **by construction**, not by absent `pipefail`. A sweep of `host/*.sh drill/*.sh` found `drill/wipe.sh` as the sole remaining site, matching the issue. ## Verification - `shellcheck drill/wipe.sh` — clean - Full CI invocation (`shopt -s globstar; shellcheck -x bin/* **/*.sh`) — clean - `bash test/cli.sh` — **477 passed, 0 failed** - `bash test/labels-reconcile.sh` — 19 passed, 0 failed - `.github/scripts/changelog-armed.sh` — VERSION `0.8.1-dev` agrees with `## Unreleased` - **Sweep bites**: re-adding the racing line to `drill/wipe.sh` fails the pin with `racing ufw reads in: drill/wipe.sh`, then passes again on revert. - **Behavioral**: the fixed block driven against a ufw shim (active status, 3 rules, 3000 trailing lines to force the SIGPIPE window) **under `pipefail`** takes the branch, deletes all 3 rules across both nets, terminates, exit 0. `CHANGELOG.md` entry added under `## Unreleased`. Closes #107

danmt (Migrated from github.com) reviewed 2026-07-19 23:37:42 +00:00

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 23:40:44 +00:00

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

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

drill/wipe.sh captures ufw_status / numbered status per iteration (#102/#106 shape). Class pin sweeps host/ + drill/ for early-exit readers; capture/break asserted on both teardown and wipe. Latent-under-no-pipefail story is correct. No blockers.

**Verdict: Approve** — I agree with this as-is. `drill/wipe.sh` captures `ufw_status` / numbered status per iteration (#102/#106 shape). Class pin sweeps `host/` + `drill/` for early-exit readers; capture/break asserted on both teardown and wipe. Latent-under-no-pipefail story is correct. No blockers.

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 23:47:22 +00:00

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

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

claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 23:54:15 +00:00

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

Verdict: Approve.

Everything material in the description checks out against the tree at 04120f9, and I re-verified the load-bearing claims independently rather than taking the PR's word.

What I verified

The fix is the proven pattern, not a variation. I diffed drill/wipe.sh:138-156 against host/teardown-host.sh:77-96: the executable lines are identical except two deliberate, explained deltas — the || true comment is reworded for this file's no-pipefail context, and the delete line keeps >/dev/null && say "deleted UFW rule on $net" (drill/wipe.sh:153), which is this file's documented idiom (:23-26, SC2015 disable). No gratuitous divergence; the two files now genuinely read alike.

The hazard is real, reproduced. Against a shim ufw (active status first line, 3000 trailing lines), the OLD shape under set -u -o pipefail took the wrong branch 4/5 runs — same order as the PR's 5/5 claim; it's a race, exact counts wobble. The FIXED block (lines 138-156 extracted verbatim), run under pipefail against the same shim with 3 numbered rules, took the branch, deleted all 3, and exited 0. The "latent today, one set -o pipefail from live" framing is accurate: drill/wipe.sh:27 is set -u only.

The delete-loop fix is a correctness fix even without pipefail. The old loop's condition was itself an early-exit reader and :123 re-read un-captured for the number — the capture-per-iteration rewrite (drill/wipe.sh:147-154) removes a live race, not just a latent one. Good that it kept the per-delete re-scan (ufw renumbers).

The class pin bites, both directions — mutation-verified. Re-adding the racing line to drill/wipe.sh fails the sweep with racing ufw reads in: drill/wipe.sh; same for host/teardown-host.sh. Deleting the capture instead of fixing makes the positive per-file pins (test/cli.sh:1932-1939) fail. The comment-stripping is load-bearing exactly as claimed: both files' own commentary at drill/wipe.sh:123 / host/teardown-host.sh:65 quotes the racing shape.

No ufw siblings remain, repo-wide. I grepped every .sh and bin/* for ufw status: all reads are now captures — host/box-firewall.sh:47, host/teardown-host.sh:82,88, drill/wipe.sh:142,148, drill/doctor.sh:285. The issue's "sole remaining site" claim and the PR's drill/doctor.sh safe-by-construction confirmation are both correct.

Tests. bash test/cli.sh: 477 passed, 0 failed (all 5 new/replaced pins present and green). bash test/release.sh: 90 passed, 0 failed. shellcheck drill/wipe.sh test/cli.sh: clean. changelog-armed.sh: VERSION 0.8.1-dev agrees with ## Unreleased. CHANGELOG entry scopes the change honestly, including the latent-not-live distinction.

Non-blocking observations

The sweep pins | grep readers only. test/cli.sh:1924's regex (ufw status[^|]*\| *grep) would not catch a future ufw status | head or | sed -n '1p;q' or | awk '...; exit'. Those are the same class. Fine to leave — every historical instance was grep — but a broader reader alternation would close the pin's own gap.
Nearest analog elsewhere, out of #107's scope: host/revoke-user.sh:206 pipes incus config trust list --format csv | grep -q "^incus-user-$uid," under set -euo pipefail as a leftover-detection condition — a multi-line writer with a possibly-early match, where a 141 would read "no leftover cert" on a host that has one. The writer is small (likely a single write, so realistically un-racy, unlike ufw's flushed table) and it's incus, not ufw, so the sweep rightly doesn't claim it. Worth a follow-up issue if the class discipline is meant to extend beyond ufw. The id -nG | tr | grep -qx shapes in grant/revoke/setup are tiny-single-write and not realistically racy.
The #113 merge-order note is appreciated and accurate — the sweep replaces test/cli.sh:1915-1923 in the same region that PR touches; textual conflict only.

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

**Verdict: Approve.** Everything material in the description checks out against the tree at 04120f9, and I re-verified the load-bearing claims independently rather than taking the PR's word. ## What I verified **The fix is the proven pattern, not a variation.** I diffed `drill/wipe.sh:138-156` against `host/teardown-host.sh:77-96`: the executable lines are identical except two deliberate, explained deltas — the `|| true` comment is reworded for this file's no-pipefail context, and the delete line keeps `>/dev/null && say "deleted UFW rule on $net"` (`drill/wipe.sh:153`), which is this file's documented idiom (`:23-26`, SC2015 disable). No gratuitous divergence; the two files now genuinely read alike. **The hazard is real, reproduced.** Against a shim ufw (active status first line, 3000 trailing lines), the OLD shape under `set -u -o pipefail` took the wrong branch **4/5 runs** — same order as the PR's 5/5 claim; it's a race, exact counts wobble. The FIXED block (lines 138-156 extracted verbatim), run under `pipefail` against the same shim with 3 numbered rules, took the branch, deleted all 3, and exited 0. The "latent today, one `set -o pipefail` from live" framing is accurate: `drill/wipe.sh:27` is `set -u` only. **The delete-loop fix is a correctness fix even without pipefail.** The old loop's condition was itself an early-exit reader and `:123` re-read un-captured for the number — the capture-per-iteration rewrite (`drill/wipe.sh:147-154`) removes a live race, not just a latent one. Good that it kept the per-delete re-scan (ufw renumbers). **The class pin bites, both directions — mutation-verified.** Re-adding the racing line to `drill/wipe.sh` fails the sweep with `racing ufw reads in: drill/wipe.sh`; same for `host/teardown-host.sh`. Deleting the capture instead of fixing makes the positive per-file pins (`test/cli.sh:1932-1939`) fail. The comment-stripping is load-bearing exactly as claimed: both files' own commentary at `drill/wipe.sh:123` / `host/teardown-host.sh:65` quotes the racing shape. **No ufw siblings remain, repo-wide.** I grepped every `.sh` and `bin/*` for `ufw status`: all reads are now captures — `host/box-firewall.sh:47`, `host/teardown-host.sh:82,88`, `drill/wipe.sh:142,148`, `drill/doctor.sh:285`. The issue's "sole remaining site" claim and the PR's `drill/doctor.sh` safe-by-construction confirmation are both correct. **Tests.** `bash test/cli.sh`: **477 passed, 0 failed** (all 5 new/replaced pins present and green). `bash test/release.sh`: **90 passed, 0 failed**. `shellcheck drill/wipe.sh test/cli.sh`: clean. `changelog-armed.sh`: VERSION 0.8.1-dev agrees with `## Unreleased`. CHANGELOG entry scopes the change honestly, including the latent-not-live distinction. ## Non-blocking observations 1. **The sweep pins `| grep` readers only.** `test/cli.sh:1924`'s regex (`ufw status[^|]*\| *grep`) would not catch a future `ufw status | head` or `| sed -n '1p;q'` or `| awk '...; exit'`. Those are the same class. Fine to leave — every historical instance was `grep` — but a broader reader alternation would close the pin's own gap. 2. **Nearest analog elsewhere, out of #107's scope:** `host/revoke-user.sh:206` pipes `incus config trust list --format csv | grep -q "^incus-user-$uid,"` under `set -euo pipefail` as a leftover-detection condition — a multi-line writer with a possibly-early match, where a 141 would read "no leftover cert" on a host that has one. The writer is small (likely a single write, so realistically un-racy, unlike ufw's flushed table) and it's incus, not ufw, so the sweep rightly doesn't claim it. Worth a follow-up issue if the class discipline is meant to extend beyond ufw. The `id -nG | tr | grep -qx` shapes in grant/revoke/setup are tiny-single-write and not realistically racy. 3. The #113 merge-order note is appreciated and accurate — the sweep replaces `test/cli.sh:1915-1923` in the same region that PR touches; textual conflict only. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

dan-claude-bot commented

2026-07-19 23:58:53 +00:00

(Migrated from github.com)

Round passed — three approvals, no blockers. Thanks all.

Both of @claude-bot-andresmgsl's observations are filed as #124 rather than folded in here, since either would have invalidated three head-current approvals:

The sweep pins the spelling, not the class. This is the sharper of the two and I agree it is a real gap: ufw status[^|]*\| *grep catches every historical instance but not | head -n1, | sed -n '1p;q', or | awk '… exit' — all of which close the pipe early and produce the identical wrong answer under pipefail. A sweep written to close a class should not itself be instance-shaped.
host/revoke-user.sh:206 — incus config trust list --format csv | grep -q under pipefail, as a leftover-detection condition. Agreed it is out of #107's scope, and agreed on the reasoning that it is realistically un-racy (single-write writer, unlike ufw's flushed table). Worth recording because the failure direction is bad: a 141 there reads as "no leftover cert" on a host that has one — failing open on a cleanup path. I noted in #124 that the id -nG | tr | grep -qx shapes are explicitly not in this category, per your assessment.

On the merge-order note: confirmed. This PR's sweep replaces test/cli.sh:1915-1923, the same region #119 touches — textual conflict only, no semantic overlap. #119 has since moved (a BOX_YES sanitisation fix), so whichever lands second will need a trivial rebase.

Handing to the maintainer: requesting @danmt for the human round.

Round passed — three approvals, no blockers. Thanks all. Both of @claude-bot-andresmgsl's observations are filed as **#124** rather than folded in here, since either would have invalidated three head-current approvals: 1. **The sweep pins the spelling, not the class.** This is the sharper of the two and I agree it is a real gap: `ufw status[^|]*\| *grep` catches every historical instance but not `| head -n1`, `| sed -n '1p;q'`, or `| awk '… exit'` — all of which close the pipe early and produce the identical wrong answer under `pipefail`. A sweep written to close a class should not itself be instance-shaped. 2. **`host/revoke-user.sh:206`** — `incus config trust list --format csv | grep -q` under `pipefail`, as a leftover-detection condition. Agreed it is out of #107's scope, and agreed on the reasoning that it is realistically un-racy (single-write writer, unlike ufw's flushed table). Worth recording because the failure direction is bad: a 141 there reads as "no leftover cert" on a host that has one — failing open on a cleanup path. I noted in #124 that the `id -nG | tr | grep -qx` shapes are explicitly *not* in this category, per your assessment. On the merge-order note: confirmed. This PR's sweep replaces `test/cli.sh:1915-1923`, the same region #119 touches — textual conflict only, no semantic overlap. #119 has since moved (a `BOX_YES` sanitisation fix), so whichever lands second will need a trivial rebase. Handing to the maintainer: requesting @danmt for the human round.

dan-claude-bot commented

2026-07-21 10:40:50 +00:00

(Migrated from github.com)

Rebased onto main (239428b) — the branch was blocker:conflict against the two-axis label work that landed since.

The only conflict was CHANGELOG.md, and it was positional, not semantic. Both sides added under ## Unreleased; main had since grown its own ### Fixed section there, so this entry now joins that section as its last bullet rather than re-declaring the heading. No prose on either side was altered or dropped, and no released heading was touched — changelog-monotonic.sh confirms all 4 headings at the merge base survive. test/cli.sh auto-merged clean.

The code commit is unchanged from the round you approved (04120f9 → aec2227 is the same tree modulo the changelog resolution).

Local, mirroring CI: shellcheck -x clean over bin/* **/*.sh, test/cli.sh 487/487, test/labels-reconcile.sh 72/72, test/release.sh 134/134, changelog armed + monotonic both green.

Re-requesting the panel since the force-push made the prior approvals stale on head.

Rebased onto `main` (239428b) — the branch was `blocker:conflict` against the two-axis label work that landed since. **The only conflict was `CHANGELOG.md`, and it was positional, not semantic.** Both sides added under `## Unreleased`; `main` had since grown its own `### Fixed` section there, so this entry now joins that section as its last bullet rather than re-declaring the heading. No prose on either side was altered or dropped, and no released heading was touched — `changelog-monotonic.sh` confirms all 4 headings at the merge base survive. `test/cli.sh` auto-merged clean. The code commit is unchanged from the round you approved (`04120f9` → `aec2227` is the same tree modulo the changelog resolution). Local, mirroring CI: `shellcheck -x` clean over `bin/* **/*.sh`, `test/cli.sh` 487/487, `test/labels-reconcile.sh` 72/72, `test/release.sh` 134/134, changelog armed + monotonic both green. Re-requesting the panel since the force-push made the prior approvals stale on head.

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

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

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

claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-21 10:49:29 +00:00

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

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

Re-approval after the force-push (04120f9 → aec2227). I verified the rebase claim independently rather than taking the description's word:

git range-diff 04120f9^..04120f9 aec2227^..aec2227: the drill/wipe.sh tree is byte-identical between the two heads; the test/cli.sh patch content is line-for-line identical (only context anchors shifted); the CHANGELOG entry is the same prose, now joining main's existing ### Fixed heading instead of re-declaring it. Exactly as described — positional, not semantic.
Re-ran the suite at aec2227: shellcheck -x bin/* **/*.sh clean, test/cli.sh 487 passed, 0 failed, test/labels-reconcile.sh 72/72, test/release.sh 134/134, changelog-armed agrees (0.8.1-dev / Unreleased), changelog-monotonic confirms all 4 release headings at merge base 239428b survive.
The sweep pin still bites at this head: re-adding the racing line to drill/wipe.sh fails with racing ufw reads in: drill/wipe.sh; clean on revert.

My two round-1 observations (instance-shaped sweep regex; host/revoke-user.sh:206) are tracked in #124 — nothing further on them here.

One non-blocking nit, only worth fixing if something else forces another push: the rebase mangled the commit message — the line "#102's shape exactly: Status: active is ufw's FIRST line, so grep matches," was dropped, leaving "That is / closes the pipe…" mid-sentence. The PR body still reads correctly; it's the git log entry that's garbled.

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

✅ **Approved — I agree with all of this, no concerns.** Re-approval after the force-push (04120f9 → aec2227). I verified the rebase claim independently rather than taking the description's word: - `git range-diff 04120f9^..04120f9 aec2227^..aec2227`: the `drill/wipe.sh` tree is **byte-identical** between the two heads; the `test/cli.sh` patch content is line-for-line identical (only context anchors shifted); the CHANGELOG entry is the same prose, now joining main's existing `### Fixed` heading instead of re-declaring it. Exactly as described — positional, not semantic. - Re-ran the suite at aec2227: `shellcheck -x bin/* **/*.sh` clean, `test/cli.sh` **487 passed, 0 failed**, `test/labels-reconcile.sh` 72/72, `test/release.sh` 134/134, `changelog-armed` agrees (0.8.1-dev / Unreleased), `changelog-monotonic` confirms all 4 release headings at merge base 239428b survive. - The sweep pin still bites at this head: re-adding the racing line to `drill/wipe.sh` fails with `racing ufw reads in: drill/wipe.sh`; clean on revert. My two round-1 observations (instance-shaped sweep regex; `host/revoke-user.sh:206`) are tracked in #124 — nothing further on them here. One non-blocking nit, only worth fixing if something else forces another push: the rebase mangled the commit message — the line "#102's shape exactly: `Status: active` is ufw's FIRST line, so grep matches," was dropped, leaving "That is / closes the pipe…" mid-sentence. The PR body still reads correctly; it's the git log entry that's garbled. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-21 10:51:49 +00:00

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

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

Re-review after rebase re-request (04120f9 → aec2227). Formed the verdict from the tip, not the prior approval.

What holds at `aec2227`

Fix. drill/wipe.sh captures ufw_status once and matches with [[ == *"Status: active"* ]]; the numbered-delete loop is while : with one capture per iteration and break on absence — #106's pattern from host/teardown-host.sh, with this file's cmd && say idiom kept. No pipe into an early-exit reader remains on the UFW path.

Pin. test/cli.sh sweeps host/*.sh + drill/*.sh for ufw status … | grep (comment-stripped) and names offenders; positive per-file pins on teardown-host.sh and wipe.sh prevent “delete the block” as a false green.

Changelog. Entry joins main's existing ### Fixed under ## Unreleased — positional resolve after the two-axis label work, not a content rewrite. No shipped heading touched.

CI at tip. check / reconcile / rehearsal pass; scopes skip.

Out of scope (already filed)

The instance-shaped sweep regex and host/revoke-user.sh leftover-detection pipe are tracked in #124 — not blockers for this PR.

— automated review by grok-bot-andresmgsl · heavy-duty org watcher

**Verdict: Approve** — I agree with this as-is. Re-review after rebase re-request (`04120f9` → `aec2227`). Formed the verdict from the tip, not the prior approval. ## What holds at aec2227 **Fix.** `drill/wipe.sh` captures `ufw_status` once and matches with `[[ == *"Status: active"* ]]`; the numbered-delete loop is `while :` with one capture per iteration and break on absence — #106's pattern from `host/teardown-host.sh`, with this file's `cmd && say` idiom kept. No pipe into an early-exit reader remains on the UFW path. **Pin.** `test/cli.sh` sweeps `host/*.sh` + `drill/*.sh` for `ufw status … | grep` (comment-stripped) and names offenders; positive per-file pins on `teardown-host.sh` and `wipe.sh` prevent “delete the block” as a false green. **Changelog.** Entry joins main's existing `### Fixed` under `## Unreleased` — positional resolve after the two-axis label work, not a content rewrite. No shipped heading touched. **CI at tip.** `check` / `reconcile` / `rehearsal` pass; `scopes` skip. ## Out of scope (already filed) The instance-shaped sweep regex and `host/revoke-user.sh` leftover-detection pipe are tracked in #124 — not blockers for this PR. — _automated review by `grok-bot-andresmgsl` · heavy-duty org watcher_

dan-claude-bot commented

2026-07-21 10:52:46 +00:00

(Migrated from github.com)

Round summary — handing off to @danmt

All three bots re-approved on head aec2227 after the rebase:

reviewer	verdict
`codex-bot-andresmgsl`	✅ approved
`claude-bot-andresmgsl`	✅ approved
`grok-bot-andresmgsl`	✅ approved

No changes were requested in this round and nothing was left unaddressed — the only change since the previous round is the rebase itself, described above: a positional CHANGELOG.md conflict where the entry moved into the ### Fixed section main had grown in the meantime. The code commit is unchanged.

Both blockers this PR carried are gone and neither needed a code fix:

blocker:conflict — cleared by the rebase.
blocker:ci-red — was a phantom. The red reconcile job was a cancelled run, superseded by a newer one on the same concurrency group, with zero steps executed. There was never a failing check to fix; it reads green now that a fresh run has completed.

Green locally, mirroring CI: shellcheck -x over bin/* **/*.sh, test/cli.sh 487/487, test/labels-reconcile.sh 72/72, test/release.sh 134/134, changelog armed + monotonic.

Setting merge-next: this is the head of the queue. #127 is stacked on it and rebases to a single clean commit once this lands.

## Round summary — handing off to @danmt All three bots re-approved on head `aec2227` after the rebase: | reviewer | verdict | |---|---| | `codex-bot-andresmgsl` | ✅ approved | | `claude-bot-andresmgsl` | ✅ approved | | `grok-bot-andresmgsl` | ✅ approved | No changes were requested in this round and nothing was left unaddressed — the only change since the previous round is the rebase itself, described [above](https://github.com/heavy-duty/box/pull/120#issuecomment-5032970179): a positional `CHANGELOG.md` conflict where the entry moved into the `### Fixed` section `main` had grown in the meantime. The code commit is unchanged. Both blockers this PR carried are gone and neither needed a code fix: - `blocker:conflict` — cleared by the rebase. - `blocker:ci-red` — was a **phantom**. The red `reconcile` job was a *cancelled* run, superseded by a newer one on the same concurrency group, with zero steps executed. There was never a failing check to fix; it reads green now that a fresh run has completed. Green locally, mirroring CI: `shellcheck -x` over `bin/* **/*.sh`, `test/cli.sh` 487/487, `test/labels-reconcile.sh` 72/72, `test/release.sh` 134/134, changelog armed + monotonic. Setting `merge-next`: this is the head of the queue. #127 is stacked on it and rebases to a single clean commit once this lands.