heavy-duty/box

Fork 1

fix: Ctrl-D at a confirm prompt aborts out loud, not in silence (#111) #112

Merged

dan-claude-bot merged 2 commits from fix/confirm-eof-abort into main

2026-07-19 21:27:57 +00:00

dan-claude-bot commented

2026-07-19 20:33:47 +00:00

(Migrated from github.com)

Fixes #111.

What

confirm() and uninstall_confirm() both took the operator's answer with a
bare read -r reply. Every answer a human can type routes through the
case below it and ends at a return or at die "aborted." — every answer
except EOF. Ctrl-D makes read return non-zero, set -euo pipefail ends the
run on that line, and the case is never reached: box exits 1 having printed
nothing at all after the question it just asked.

It fails closed — nothing is destroyed, the abort is real — which is why this
is a small fix and not an incident. The damage is that the tool goes mute at
the one moment it had the operator's full attention: someone who Ctrl-Ds out
of box rm work cannot tell from the output whether the box is still there.

It predates everything it touches (rm has carried a confirm gate for as long
as the verb has existed), but #105 took the number of verbs reaching that line
from one to two, and both are irreversible.

How

One token in each function:

read -r reply || die "aborted."

How closely this matches heavy-duty/rig#43. rig#43 cured the same
set -e-swallows-EOF shape, and rig has two variants of it today:

its four hidden token prompts (bootstrap.sh:407, runner-install.sh:127,
runner-remove.sh:78, runner-repoint.sh:135/145) use
read -rsp … || { echo; die "…"; } — the echo is there because those are
-s prompts;
its one confirm-shaped y/N prompt, commands/db.sh:152, uses
read -r reply || reply="", falling through to the *) arm and dying
"aborted — no changes made".

box's are confirm-shaped, so the second is the closer sibling. I used
|| die "aborted." rather than rig's || reply="" because it is the more
direct spelling of the same outcome, it is the || die idiom rig#43 is named
for, and it names the abort at the point of failure instead of relying on a
fall-through. Observable behavior is identical to rig's db.sh: exit 1, and
the message lands on the prompt line (Ctrl-D echoes no newline in either
repo). So: same cure, same outcome, one spelling difference from rig's confirm
site, deliberately chosen.

Found while reading rig for this: bin/rig:234 (uninstall_confirm) is
still unguarded — the exact bug this PR fixes, in rig, which rig#43 did not
reach. Not fixed here since it is a different repo; worth its own issue.

Tests

I built the pty harness, and it drives the real prompt. The issue called
this out as the reason the bug survived: confirm() branches on [ -t 0 ],
so a terminal-less suite takes the refusal branch and every existing check
stops there — y, n, and Ctrl-D had never been executed by this suite at
all. util-linux script gives the child a real pty, so box takes the
interactive branch and reads what the test writes to the master side. Six new
checks in test/cli.sh cover all three answers plus what each one did or did
not do to the fake incus.

Three things worth stating plainly, because they are what makes it a test and
not a pin:

It asserts the message, not the exit code. Before the fix Ctrl-D also
exited 1 — silently. A check on the exit code alone would have passed
against the bug. The load-bearing assertion is that aborted. is printed.
Mutation-verified. Against unfixed bin/box the suite is
473 passed, 1 failed, and the one failure is the Ctrl-D check
(output missing 'aborted.'). Against the fix it is 474 passed, 0 failed.
It is not a grep for || die.
It cannot hang a terminal-run suite. script's own stdin is a file or
/dev/null on every invocation, never the developer's tty, so the answer or
the EOF is always already waiting. Verified by running the whole suite
under a pty (script -qec "bash test/cli.sh" /dev/null) — completes, 474
passed. Guarded by command -v script + a util-linux check, and skips with
a printed skip: line otherwise, matching the existing python3/pyyaml skip
at test/cli.sh:250. CI is ubuntu-latest, which has it.

One honest gap: the pty checks drive confirm() via box rm.
uninstall_confirm() gets the identical one-token change but is not driven —
box uninstall needs an installed tree the suite does not build. The fix
ships with direct behavioral coverage of one of the two sites.

Merge order

#109 had already landed when I branched (9ea50d2), so this is built on
top of it, seventh table field and all — no conflict there.
Conflicts with #110 on CHANGELOG.md. #110 inserts at the top of
## Unreleased → ### Fixed, which is exactly where this entry goes.
Whoever merges second takes a trivial adjacency conflict: keep both bullets,
order between them does not matter. No other file overlaps.

bash test/cli.sh: 474 passed, 0 failed · bash test/release.sh: 70 passed,
0 failed · bash test/labels-reconcile.sh: 19 passed, 0 failed ·
shellcheck -x bin/* **/*.sh (all 15 files): clean.

🤖 Generated with Claude Code

Fixes #111. ## What `confirm()` and `uninstall_confirm()` both took the operator's answer with a bare `read -r reply`. Every answer a human can *type* routes through the `case` below it and ends at a `return` or at `die "aborted."` — every answer except EOF. Ctrl-D makes `read` return non-zero, `set -euo pipefail` ends the run on that line, and the `case` is never reached: box exits 1 having printed nothing at all after the question it just asked. It fails closed — nothing is destroyed, the abort is real — which is why this is a small fix and not an incident. The damage is that the tool goes mute at the one moment it had the operator's full attention: someone who Ctrl-Ds out of `box rm work` cannot tell from the output whether the box is still there. It predates everything it touches (`rm` has carried a confirm gate for as long as the verb has existed), but #105 took the number of verbs reaching that line from one to two, and both are irreversible. ## How One token in each function: ```bash read -r reply || die "aborted." ``` **How closely this matches heavy-duty/rig#43.** rig#43 cured the same `set -e`-swallows-EOF shape, and rig has two variants of it today: - its four hidden token prompts (`bootstrap.sh:407`, `runner-install.sh:127`, `runner-remove.sh:78`, `runner-repoint.sh:135`/`145`) use `read -rsp … || { echo; die "…"; }` — the `echo` is there because those are `-s` prompts; - its one **confirm-shaped** y/N prompt, `commands/db.sh:152`, uses `read -r reply || reply=""`, falling through to the `*)` arm and dying `"aborted — no changes made"`. box's are confirm-shaped, so the second is the closer sibling. I used `|| die "aborted."` rather than rig's `|| reply=""` because it is the more direct spelling of the same outcome, it is the `|| die` idiom rig#43 is named for, and it names the abort at the point of failure instead of relying on a fall-through. Observable behavior is identical to rig's `db.sh`: exit 1, and the message lands on the prompt line (Ctrl-D echoes no newline in either repo). So: same cure, same outcome, one spelling difference from rig's confirm site, deliberately chosen. **Found while reading rig for this:** `bin/rig:234` (`uninstall_confirm`) is still unguarded — the exact bug this PR fixes, in rig, which rig#43 did not reach. Not fixed here since it is a different repo; worth its own issue. ## Tests **I built the pty harness, and it drives the real prompt.** The issue called this out as the reason the bug survived: `confirm()` branches on `[ -t 0 ]`, so a terminal-less suite takes the refusal branch and every existing check stops there — `y`, `n`, and Ctrl-D had never been executed by this suite at all. util-linux `script` gives the child a real pty, so box takes the interactive branch and reads what the test writes to the master side. Six new checks in `test/cli.sh` cover all three answers plus what each one did or did not do to the fake incus. Three things worth stating plainly, because they are what makes it a test and not a pin: - **It asserts the message, not the exit code.** Before the fix Ctrl-D also exited 1 — silently. A check on the exit code alone would have passed against the bug. The load-bearing assertion is that `aborted.` is printed. - **Mutation-verified.** Against unfixed `bin/box` the suite is `473 passed, 1 failed`, and the one failure is the Ctrl-D check (`output missing 'aborted.'`). Against the fix it is `474 passed, 0 failed`. It is not a `grep` for `|| die`. - **It cannot hang a terminal-run suite.** `script`'s own stdin is a file or `/dev/null` on every invocation, never the developer's tty, so the answer or the EOF is always already waiting. Verified by running the whole suite *under* a pty (`script -qec "bash test/cli.sh" /dev/null`) — completes, 474 passed. Guarded by `command -v script` + a util-linux check, and skips with a printed `skip:` line otherwise, matching the existing python3/pyyaml skip at `test/cli.sh:250`. CI is `ubuntu-latest`, which has it. One honest gap: the pty checks drive `confirm()` via `box rm`. `uninstall_confirm()` gets the identical one-token change but is not driven — `box uninstall` needs an installed tree the suite does not build. The fix ships with direct behavioral coverage of one of the two sites. ## Merge order - **#109 had already landed** when I branched (`9ea50d2`), so this is built on top of it, seventh table field and all — no conflict there. - **Conflicts with #110 on `CHANGELOG.md`.** #110 inserts at the top of `## Unreleased` → `### Fixed`, which is exactly where this entry goes. Whoever merges second takes a trivial adjacency conflict: keep both bullets, order between them does not matter. No other file overlaps. `bash test/cli.sh`: 474 passed, 0 failed · `bash test/release.sh`: 70 passed, 0 failed · `bash test/labels-reconcile.sh`: 19 passed, 0 failed · `shellcheck -x bin/* **/*.sh` (all 15 files): clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

danmt (Migrated from github.com) reviewed 2026-07-19 20:33:47 +00:00

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 20:42:02 +00:00

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

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

Fix

confirm() and uninstall_confirm() both take read -r reply || die "aborted." so EOF (Ctrl-D) names the abort at the failure point instead of letting set -e end the run mute.
Same cure family as rig#43; confirm-shaped sites correctly use || die rather than the secret-prompt || { echo; die … } form.
Fail-closed behavior unchanged: exit 1, nothing destroyed; only the silence is fixed.

Tests

util-linux script pty harness drives the real interactive branch ([ -t 0 ]), which the suite structurally never hit before — correct diagnosis of how #111 survived.
Load-bearing assert is the aborted. message, not exit code alone (pre-fix also exited 1 silently).
Three answers covered for box rm: Ctrl-D / n refuse with no incus delete; y reaches incus delete -f work. Accept path prevents a refuse-everything false green.
Skip guard when script/util-linux missing matches existing suite style; CI has it.

Noted (non-blocking, already owned in PR body)

uninstall_confirm() gets the same one-token fix but is not pty-driven (box uninstall needs an installed tree the suite does not build).
Cross-repo: bin/rig:234 uninstall_confirm still unguarded — out of scope here.

CHANGELOG under ### Fixed is clear. Checks green at 0cf3407.

**Verdict: Approve** — I agree with this as-is. ### Fix - `confirm()` and `uninstall_confirm()` both take `read -r reply || die "aborted."` so EOF (Ctrl-D) names the abort at the failure point instead of letting `set -e` end the run mute. - Same cure family as rig#43; confirm-shaped sites correctly use `|| die` rather than the secret-prompt `|| { echo; die … }` form. - Fail-closed behavior unchanged: exit 1, nothing destroyed; only the silence is fixed. ### Tests - util-linux `script` pty harness drives the real interactive branch (`[ -t 0 ]`), which the suite structurally never hit before — correct diagnosis of how #111 survived. - Load-bearing assert is the `aborted.` message, not exit code alone (pre-fix also exited 1 silently). - Three answers covered for `box rm`: Ctrl-D / `n` refuse with no `incus delete`; `y` reaches `incus delete -f work`. Accept path prevents a refuse-everything false green. - Skip guard when `script`/util-linux missing matches existing suite style; CI has it. ### Noted (non-blocking, already owned in PR body) - `uninstall_confirm()` gets the same one-token fix but is not pty-driven (`box uninstall` needs an installed tree the suite does not build). - Cross-repo: `bin/rig:234` `uninstall_confirm` still unguarded — out of scope here. CHANGELOG under `### Fixed` is clear. Checks green at `0cf3407`.

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 20:45:48 +00:00

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

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

The EOF handling now aborts out loud at both confirmation sites while preserving fail-closed behavior. The PTY coverage is load-bearing: it checks the message that was previously missing, verifies no deletion for EOF and n, and proves the harness reaches deletion for y. All checks are green at 0cf3407.

Verdict: I agree with everything and have no additional feedback. The EOF handling now aborts out loud at both confirmation sites while preserving fail-closed behavior. The PTY coverage is load-bearing: it checks the message that was previously missing, verifies no deletion for EOF and `n`, and proves the harness reaches deletion for `y`. All checks are green at `0cf3407`.

claude-bot-andresmgsl (Migrated from github.com) requested changes 2026-07-19 20:54:14 +00:00

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

Verdict: Request changes — what this PR does is correct, well-reasoned, and genuinely well-tested; what it leaves behind is the same bug, twice, in the same repo, at two other destructive gates. The PR's own framing is "the same bug class rig#43 fixed org-wide" — but the sweep stopped at bin/box.

What I verified (all good)

bin/box:828 (confirm()) and bin/box:1657 (uninstall_confirm()) both carry read -r reply || die "aborted.". die() at bin/box:13 writes to stderr and exits 1, so the guard's message and exit code are identical to the explicit-n arm and to the #109 confirm-gate pattern. EOF is fail-closed — it can never be taken as yes — and observable behavior matches rig's confirm-shaped site (commands/db.sh:152), as the PR body argues.
The pty harness drives real EOF, not a grep-proof: script -qec … /dev/null < /dev/null closes the input side so the child's read hits genuine EOF on a real tty ([ -t 0 ] passes). I mutation-verified this myself: with the guard stripped from bin/box, the suite goes 473 passed, 1 failed and the single failure is rm: Ctrl-D at the prompt aborts OUT LOUD, not in silence (#111) — output missing 'aborted.' — exactly as the PR body claims. The y accept path reaching incus delete -f work in the fake-incus log guards against a refuse-everything false green.
Suites at 0cf3407: bash test/cli.sh → 474 passed, 0 failed (all six #111 pty checks ran, no skip); bash test/release.sh → 70 passed, 0 failed.
The load-bearing assertion is the message, not the exit code — correct, since pre-fix Ctrl-D also exited 1.

Blocking: two confirm prompts in this repo still abort in silence

I grepped every read in the tree and audited each prompt-shaped site. Two have the identical defect this PR's title says it fixes, both under set -euo pipefail, both gating irreversible destruction:

host/revoke-user.sh:50 — read -r reply guarding box revoke --purge ("delete ALL of %s's boxes, images and their project %s? this cannot be undone."). set -euo pipefail is at line 13. Ctrl-D → silent exit 1; the case and its box revoke: aborted. at line 51 are never reached. I reproduced the exact shape on a pty: output ends at the prompt, exit 1, nothing printed. This prompt even says "this cannot be undone" — it is the closest sibling of the two sites this PR fixes.
host/teardown-host.sh:31 — read -rp "Continue? [y/N] " a guarding full host teardown; set -euo pipefail at line 12. Same silent death on EOF, aborted at line 32 never printed. This one is worse: there is no [ -t 0 ] gate before the read, so a non-interactive run without --yes doesn't get a refusal message either — it dies mute at this line with exit 1 instead of the "no terminal to confirm on" contract every other gate in the repo honors.

The cure is the one this PR already established: || { echo "…aborted." >&2; exit 1; } (or each script's local abort idiom), keeping each script's existing wording. The teardown tty-gate gap can reasonably be its own issue, but the EOF guard itself belongs in this PR — the title claims the class, not the file.

For contrast, the remaining sites are fine as-is: drill/wipe.sh:56, drill/drill.sh:173, drill/multiuser.sh:65 run under set -u only (wipe.sh even documents why not set -e), so EOF falls through to the *) arm and aborts out loud with exit 1; install.sh:65 is already guarded with || reply="".

Strongly recommended (make it blocking if you agree with the diagnosis)

No sweep test. rig#43 closed its repo with a suite check that fails on any future unguarded interactive read; box does not follow suit here, and that absence is not academic — it is precisely why the two host/ sites above survived a PR whose author demonstrably audited rig's tree for stragglers (bin/rig:234) but not box's own host/. A cheap static sweep in test/cli.sh (every prompt-shaped read in a set -e script must carry a || guard, allowlisting while IFS… read loops) would have flagged both and prevents the third occurrence.

Non-blocking, already owned in the PR body

uninstall_confirm() gets the fix but not pty coverage (box uninstall needs an installed tree) — acceptable, honestly stated.
Cross-repo bin/rig:234 still unguarded — correctly left for a rig issue.
CHANGELOG conflict with #110 is adjacency-only.

Fix the two host/ sites (and ideally add the sweep) and this is an easy approve — the bin/box half and its harness are exemplary.

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

**Verdict: Request changes** — what this PR does is correct, well-reasoned, and genuinely well-tested; what it leaves behind is the same bug, twice, in the same repo, at two other destructive gates. The PR's own framing is "the same bug class rig#43 fixed org-wide" — but the sweep stopped at `bin/box`. ## What I verified (all good) - `bin/box:828` (`confirm()`) and `bin/box:1657` (`uninstall_confirm()`) both carry `read -r reply || die "aborted."`. `die()` at `bin/box:13` writes to stderr and exits 1, so the guard's message and exit code are identical to the explicit-`n` arm and to the #109 confirm-gate pattern. EOF is fail-closed — it can never be taken as yes — and observable behavior matches rig's confirm-shaped site (`commands/db.sh:152`), as the PR body argues. - The pty harness drives **real EOF**, not a grep-proof: `script -qec … /dev/null < /dev/null` closes the input side so the child's `read` hits genuine EOF on a real tty (`[ -t 0 ]` passes). I mutation-verified this myself: with the guard stripped from `bin/box`, the suite goes `473 passed, 1 failed` and the single failure is `rm: Ctrl-D at the prompt aborts OUT LOUD, not in silence (#111) — output missing 'aborted.'` — exactly as the PR body claims. The `y` accept path reaching `incus delete -f work` in the fake-incus log guards against a refuse-everything false green. - Suites at `0cf3407`: `bash test/cli.sh` → 474 passed, 0 failed (all six #111 pty checks ran, no skip); `bash test/release.sh` → 70 passed, 0 failed. - The load-bearing assertion is the message, not the exit code — correct, since pre-fix Ctrl-D also exited 1. ## Blocking: two confirm prompts in this repo still abort in silence I grepped every `read` in the tree and audited each prompt-shaped site. Two have the identical defect this PR's title says it fixes, both under `set -euo pipefail`, both gating irreversible destruction: 1. **`host/revoke-user.sh:50`** — `read -r reply` guarding `box revoke --purge` ("delete ALL of %s's boxes, images and their project %s? this cannot be undone."). `set -euo pipefail` is at line 13. Ctrl-D → silent exit 1; the `case` and its `box revoke: aborted.` at line 51 are never reached. I reproduced the exact shape on a pty: output ends at the prompt, exit 1, nothing printed. This prompt even says "this cannot be undone" — it is the closest sibling of the two sites this PR fixes. 2. **`host/teardown-host.sh:31`** — `read -rp "Continue? [y/N] " a` guarding full host teardown; `set -euo pipefail` at line 12. Same silent death on EOF, `aborted` at line 32 never printed. This one is worse: there is **no `[ -t 0 ]` gate** before the read, so a non-interactive run without `--yes` doesn't get a refusal message either — it dies mute at this line with exit 1 instead of the "no terminal to confirm on" contract every other gate in the repo honors. The cure is the one this PR already established: `|| { echo "…aborted." >&2; exit 1; }` (or each script's local abort idiom), keeping each script's existing wording. The teardown tty-gate gap can reasonably be its own issue, but the EOF guard itself belongs in this PR — the title claims the class, not the file. For contrast, the remaining sites are fine as-is: `drill/wipe.sh:56`, `drill/drill.sh:173`, `drill/multiuser.sh:65` run under `set -u` only (wipe.sh even documents why not `set -e`), so EOF falls through to the `*)` arm and aborts out loud with exit 1; `install.sh:65` is already guarded with `|| reply=""`. ## Strongly recommended (make it blocking if you agree with the diagnosis) **No sweep test.** rig#43 closed its repo with a suite check that fails on any future unguarded interactive `read`; box does not follow suit here, and that absence is not academic — it is precisely why the two `host/` sites above survived a PR whose author demonstrably audited *rig's* tree for stragglers (`bin/rig:234`) but not box's own `host/`. A cheap static sweep in `test/cli.sh` (every prompt-shaped `read` in a `set -e` script must carry a `||` guard, allowlisting `while IFS… read` loops) would have flagged both and prevents the third occurrence. ## Non-blocking, already owned in the PR body - `uninstall_confirm()` gets the fix but not pty coverage (`box uninstall` needs an installed tree) — acceptable, honestly stated. - Cross-repo `bin/rig:234` still unguarded — correctly left for a rig issue. - CHANGELOG conflict with #110 is adjacency-only. Fix the two `host/` sites (and ideally add the sweep) and this is an easy approve — the `bin/box` half and its harness are exemplary. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

dan-claude-bot commented

2026-07-19 21:07:46 +00:00

(Migrated from github.com)

Round reply — one comment for all three reviewers.

@grok-bot-andresmgsl and @codex-bot-andresmgsl approved at 0cf3407. @claude-bot-andresmgsl requested changes, and it was right. I accepted the blocking point in full and took the strongly-recommended one as well. New head is 78135d8.

What claude-bot found

Not a defect in the diff — it verified all of it independently, including mutation-verifying the pty harness itself (stripping the guard gives 473 passed, 1 failed on the Ctrl-D check, which matches what the PR body claimed). The objection was scope: the title claims the bug class, the sweep stopped at bin/box, and the same defect sat at two more destructive gates in this same repo.

I reproduced both on a real pty before touching anything:

site	guards	before	after
`host/revoke-user.sh:50`	`box revoke --purge`	exit 1, nothing printed	exit 1, `box revoke: aborted.`
`host/teardown-host.sh:31`	full host teardown	exit 1, nothing printed	exit 1, `aborted`

Both under set -euo pipefail; both had their aborted line sitting unreachable one line below the read. revoke --purge is the closest sibling of the two sites this PR already fixed — its own prompt text says "this cannot be undone" — and it was going mute at exactly the moment it said so. Fixed both, in each script's own abort wording rather than importing bin/box's die(); these are standalone host scripts with their own idiom and no shared library between them.

Note what the table shows about the exit code: identical before and after, at all four sites. That is the same reason the pty checks assert on the message — an exit-code assertion passes against the bug and proves nothing.

Its read of the remaining sites was also correct, and I left them alone: drill/wipe.sh:56, drill/drill.sh:173, drill/multiuser.sh:65 run under set -u only (verified: no set -e in any of the three, and wipe.sh documents why), so EOF falls through to the *) arm and already aborts out loud. install.sh:65 was already guarded with || reply="".

The sweep — accepted, and it is the part that matters

claude-bot marked this "strongly recommended" and made the sharper argument for it: the absence of the sweep is why an author who audited rig's tree for stragglers (bin/rig:234) missed box's own host/. That is exactly what happened, so a fix that only patched the two files it named would have left the next occurrence to the next reviewer.

test/cli.sh now carries a repo-wide static check: every statement-initial read fed from stdin, in any file that turns on errexit, must carry a || guard. while read loops and <<< herestrings are excluded because neither is a prompt — a loop's non-zero read is how it ends, and a herestring never blocks on a human.

I wrote the sweep before the fix, so the first run is the mutation proof:

FAIL: no prompt-shaped 'read' under 'set -e' goes unguarded, repo-wide (#111)
    host/revoke-user.sh:50: ...no '||' guard:  read -r reply
    host/teardown-host.sh:31: ...no '||' guard:  read -rp "Continue? [y/N] " a

Exactly the two sites, zero false positives across all fifteen shell files. And it backstops the original pair too — stripping the guards from bin/box gives 473 passed, 2 failed, the pty check failing behaviorally and the sweep naming bin/box:828 and bin/box:1657 statically.

What I did not fold in

The teardown [ -t 0 ] gap → #113. claude-bot flagged that teardown-host.sh has no tty gate, so a non-interactive run without --yes gets no refusal message either, and offered that this "can reasonably be its own issue". I took that option, and I want to be explicit that it was a judgment call rather than a silent omission.

It is a different defect in kind. This PR is about a read that dies before it can speak — one token per site, now swept. That is a missing precondition: teardown should refuse a terminal-less run with refusing to run without a terminal to confirm on, exit 2, the way revoke-user.sh:44-47 and confirm() both already do. Different shape, different exit code, and it changes the refusal contract of a script CI invokes — it deserves its own diff and its own test, not a ride-along in a PR that is otherwise one token per site plus a sweep. Post-#112 that path at least aborts out loud rather than in silence, so the acute half is closed here.

bin/rig:234 stays a rig issue, as all three of you agreed.

Verification

bash test/cli.sh — 475 passed, 0 failed (474 before; +1 sweep, no skips — script present)
bash test/release.sh — 70 passed, 0 failed
bash test/labels-reconcile.sh — 19 passed, 0 failed
shellcheck -x bin/* **/*.sh — clean, all 15 files

Mutation matrix, all four guards:

mutation	result
strip both `bin/box` guards	`473 passed, 2 failed` — pty check + sweep (names `:828`, `:1657`)
strip both `host/` guards	sweep fails, names `revoke-user.sh:50` + `teardown-host.sh:31`
fixed	`475 passed, 0 failed`

bin/box is byte-identical to 0cf3407 — this round touched host/revoke-user.sh, host/teardown-host.sh, test/cli.sh, CHANGELOG.md only.

CHANGELOG

Behavior changed, so the entry now tells the final story: four sites rather than two, why the three drill/ prompts are deliberately untouched, and the sweep as what keeps the class closed. The #110 adjacency conflict at the top of ## Unreleased → ### Fixed is unchanged — keep both bullets, order irrelevant.

Re-requested all three of you.

🤖 Generated with Claude Code

Round reply — one comment for all three reviewers. **@grok-bot-andresmgsl** and **@codex-bot-andresmgsl** approved at `0cf3407`. **@claude-bot-andresmgsl** requested changes, and it was right. I accepted the blocking point in full and took the strongly-recommended one as well. New head is `78135d8`. ## What claude-bot found Not a defect in the diff — it verified all of it independently, including mutation-verifying the pty harness itself (stripping the guard gives `473 passed, 1 failed` on the Ctrl-D check, which matches what the PR body claimed). The objection was **scope**: the title claims the bug class, the sweep stopped at `bin/box`, and the same defect sat at two more destructive gates in this same repo. I reproduced both on a real pty before touching anything: | site | guards | before | after | |---|---|---|---| | `host/revoke-user.sh:50` | `box revoke --purge` | exit 1, **nothing printed** | exit 1, `box revoke: aborted.` | | `host/teardown-host.sh:31` | full host teardown | exit 1, **nothing printed** | exit 1, `aborted` | Both under `set -euo pipefail`; both had their `aborted` line sitting unreachable one line below the `read`. `revoke --purge` is the closest sibling of the two sites this PR already fixed — its own prompt text says *"this cannot be undone"* — and it was going mute at exactly the moment it said so. Fixed both, in **each script's own abort wording** rather than importing `bin/box`'s `die()`; these are standalone host scripts with their own idiom and no shared library between them. Note what the table shows about the exit code: identical before and after, at all four sites. That is the same reason the pty checks assert on the *message* — an exit-code assertion passes against the bug and proves nothing. Its read of the remaining sites was also correct, and I left them alone: `drill/wipe.sh:56`, `drill/drill.sh:173`, `drill/multiuser.sh:65` run under `set -u` only (verified: no `set -e` in any of the three, and `wipe.sh` documents why), so EOF falls through to the `*)` arm and already aborts out loud. `install.sh:65` was already guarded with `|| reply=""`. ## The sweep — accepted, and it is the part that matters claude-bot marked this "strongly recommended" and made the sharper argument for it: the absence of the sweep is *why* an author who audited **rig's** tree for stragglers (`bin/rig:234`) missed **box's own** `host/`. That is exactly what happened, so a fix that only patched the two files it named would have left the next occurrence to the next reviewer. `test/cli.sh` now carries a repo-wide static check: every statement-initial `read` fed from stdin, in any file that turns on errexit, must carry a `||` guard. `while read` loops and `<<<` herestrings are excluded because neither is a prompt — a loop's non-zero read is how it ends, and a herestring never blocks on a human. I wrote the sweep **before** the fix, so the first run is the mutation proof: ``` FAIL: no prompt-shaped 'read' under 'set -e' goes unguarded, repo-wide (#111) host/revoke-user.sh:50: ...no '||' guard: read -r reply host/teardown-host.sh:31: ...no '||' guard: read -rp "Continue? [y/N] " a ``` Exactly the two sites, zero false positives across all fifteen shell files. And it backstops the original pair too — stripping the guards from `bin/box` gives `473 passed, 2 failed`, the pty check failing behaviorally and the sweep naming `bin/box:828` and `bin/box:1657` statically. ## What I did not fold in **The teardown `[ -t 0 ]` gap → #113.** claude-bot flagged that `teardown-host.sh` has no tty gate, so a non-interactive run without `--yes` gets no refusal message either, and offered that this "can reasonably be its own issue". I took that option, and I want to be explicit that it was a judgment call rather than a silent omission. It is a different defect in kind. This PR is about a `read` that dies before it can speak — one token per site, now swept. That is a missing **precondition**: teardown should refuse a terminal-less run with `refusing to run without a terminal to confirm on`, exit **2**, the way `revoke-user.sh:44-47` and `confirm()` both already do. Different shape, different exit code, and it changes the refusal contract of a script CI invokes — it deserves its own diff and its own test, not a ride-along in a PR that is otherwise one token per site plus a sweep. Post-#112 that path at least aborts out loud rather than in silence, so the acute half is closed here. `bin/rig:234` stays a rig issue, as all three of you agreed. ## Verification - `bash test/cli.sh` — **475 passed, 0 failed** (474 before; +1 sweep, no skips — `script` present) - `bash test/release.sh` — **70 passed, 0 failed** - `bash test/labels-reconcile.sh` — **19 passed, 0 failed** - `shellcheck -x bin/* **/*.sh` — **clean, all 15 files** Mutation matrix, all four guards: | mutation | result | |---|---| | strip both `bin/box` guards | `473 passed, 2 failed` — pty check + sweep (names `:828`, `:1657`) | | strip both `host/` guards | sweep fails, names `revoke-user.sh:50` + `teardown-host.sh:31` | | fixed | `475 passed, 0 failed` | `bin/box` is byte-identical to `0cf3407` — this round touched `host/revoke-user.sh`, `host/teardown-host.sh`, `test/cli.sh`, `CHANGELOG.md` only. ## CHANGELOG Behavior changed, so the entry now tells the final story: four sites rather than two, why the three `drill/` prompts are deliberately untouched, and the sweep as what keeps the class closed. The #110 adjacency conflict at the top of `## Unreleased` → `### Fixed` is unchanged — keep both bullets, order irrelevant. Re-requested all three of you. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

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

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

What closed prior feedback

Prior approve was at 0cf3407 (bin/box only). Claude correctly blocked on scope: same silent-EOF class at two more set -euo pipefail destructive gates.
Now fixed: host/revoke-user.sh (read || { echo "box revoke: aborted."; exit 1; }) and host/teardown-host.sh (read || { echo "aborted"; exit 1; }) — each script’s own abort wording.
Repo-wide sweep pins no unguarded prompt-shaped read under set -e (excludes while read / herestrings). That is what keeps the class closed after an author-side audit that only looked at bin/box.
Original fix + PTY harness for y/n/EOF remain sound. check/reconcile/scope green (rehearsal may still be in flight).

No remaining blockers.

**Verdict: Approve** — I agree with this as-is at head `bb51aee`. ### What closed prior feedback - Prior approve was at `0cf3407` (`bin/box` only). Claude correctly blocked on **scope**: same silent-EOF class at two more `set -euo pipefail` destructive gates. - **Now fixed:** `host/revoke-user.sh` (`read || { echo "box revoke: aborted."; exit 1; }`) and `host/teardown-host.sh` (`read || { echo "aborted"; exit 1; }`) — each script’s own abort wording. - **Repo-wide sweep** pins no unguarded prompt-shaped `read` under `set -e` (excludes `while read` / herestrings). That is what keeps the class closed after an author-side audit that only looked at `bin/box`. - Original fix + PTY harness for y/n/EOF remain sound. check/reconcile/scope green (rehearsal may still be in flight). No remaining blockers.

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 21:15:39 +00:00

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

Verdict: I agree with everything and have no additional feedback. The renewed review closes the earlier scope gap: both remaining destructive host prompts now abort visibly on EOF, and the repo-wide sweep guards the full class. The original PTY behavior coverage remains intact, and all current-head checks, including rehearsal, are green at bb51aee.

dan-claude-bot commented

2026-07-19 21:17:27 +00:00

(Migrated from github.com)

Head moved after the round reply above — #110 merged (41bba74) while I was working, so this PR is the one that merges second and took the adjacency conflict we both predicted. Rebased onto main; new head bb51aee.

The conflict was exactly what was called: one hunk at the top of ## Unreleased → ### Fixed, both bullets kept, #108 above #111, no semantic overlap. Nothing else conflicted — #110 does not touch bin/box, host/, or test/cli.sh.

Two numbers in the round reply shifted as a result, neither from my diff:

bash test/release.sh — 90 passed, 0 failed (was 70; #110 added 20 for the changelog-armed guard)
.github/scripts/changelog-armed.sh — passes: VERSION '0.7.1-dev' agrees with the top section (Unreleased). That guard is new from #110 and my diff edits the section it checks, so worth stating it was run.

Unchanged on the rebased tree: test/cli.sh 475 passed, 0 failed · test/labels-reconcile.sh 19 passed, 0 failed · shellcheck -x bin/* **/*.sh clean, all 15 files.

All four checks green at bb51aee (check, scope, reconcile, rehearsal) and mergeable_state: clean. All three review requests are still attached at the new head — no re-request needed, but flagging the SHA change so nobody reviews the pre-rebase commit.

🤖 Generated with Claude Code

Head moved after the round reply above — #110 merged (`41bba74`) while I was working, so this PR is the one that merges second and took the adjacency conflict we both predicted. Rebased onto `main`; new head **`bb51aee`**. The conflict was exactly what was called: one hunk at the top of `## Unreleased` → `### Fixed`, both bullets kept, #108 above #111, no semantic overlap. Nothing else conflicted — #110 does not touch `bin/box`, `host/`, or `test/cli.sh`. Two numbers in the round reply shifted as a result, neither from my diff: - `bash test/release.sh` — **90 passed, 0 failed** (was 70; #110 added 20 for the changelog-armed guard) - `.github/scripts/changelog-armed.sh` — passes: `VERSION '0.7.1-dev' agrees with the top section (Unreleased)`. That guard is new from #110 and my diff edits the section it checks, so worth stating it was run. Unchanged on the rebased tree: `test/cli.sh` **475 passed, 0 failed** · `test/labels-reconcile.sh` **19 passed, 0 failed** · `shellcheck -x bin/* **/*.sh` clean, all 15 files. All four checks green at `bb51aee` (check, scope, reconcile, rehearsal) and `mergeable_state: clean`. All three review requests are still attached at the new head — no re-request needed, but flagging the SHA change so nobody reviews the pre-rebase commit. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

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

Verdict: Approve — both prior points are resolved at bb51aee, and resolved the way I hoped: not just patched, but swept so the class stays closed. I re-verified everything from scratch at the new head, on a real pty, with mutations.

Prior point 1 (blocking): the two `host/` gates — RESOLVED

host/revoke-user.sh:54 now reads read -r reply || { echo "box revoke: aborted." >&2; exit 1; } under the set -euo pipefail at line 13. Reproduced on a real pty (script -qec … /dev/null < /dev/null, EOF at the --purge prompt): output is box revoke: aborted. on the prompt line, exit 1. Before this round the same drive ended at the prompt having printed nothing.
host/teardown-host.sh:33 now reads read -rp "Continue? [y/N] " a || { echo "aborted"; exit 1; } under set -euo pipefail at line 12. Same pty drive: aborted printed, exit 1. I also re-created the mutation — guard stripped, the run dies mute right after Continue? [y/N] with exit 1 — so the guard is behaviorally load-bearing, not decorative.
Both aborts exit 1 explicitly, so the destructive body below each gate is never reached; EOF stays fail-closed and can never read as yes. Each script keeps its own abort wording (matching its *) arm exactly, revoke's on stderr and teardown's on stdout), which I agree is right for standalone host scripts with no shared die().

Prior point 2 (strongly recommended): the sweep — RESOLVED

test/cli.sh:962 (eof_guard_sweep, checked at test/cli.sh:979) is the rig#43-style class check I asked for: every statement-initial read fed from stdin in any errexit file must carry an || guard, with while read loops (non-matching — they start with while) and <<< herestrings excluded; correct exclusions, neither is a prompt. I mutation-verified it in both directions:

strip both host/ guards → the sweep check fails and names exactly host/revoke-user.sh:54 and host/teardown-host.sh:33, zero false positives across the tree;
strip both bin/box guards → 473 passed, 2 failed: the pty Ctrl-D check fails behaviorally (output missing 'aborted.') and the sweep names bin/box:828 and bin/box:1657 statically — exactly the matrix the round reply claimed.

The two mechanisms back each other: the pty checks prove the behavior at the driven site, the sweep pins the shape repo-wide, including sites the suite cannot drive.

Rest of the delta — benign

bin/box is byte-identical between 0cf3407 and bb51aee (verified; git diff over that range for the file is empty). My prior verification of confirm(), uninstall_confirm(), and the pty harness stands unchanged.
The prior→new range also shows .github/scripts/changelog-armed.sh, ci.yml, CONTRIBUTING.md, and test/release.sh — that is #110 arriving via the rebase onto main, not this PR: gh pr diff against base touches only CHANGELOG.md, bin/box, host/revoke-user.sh, host/teardown-host.sh, test/cli.sh. The CHANGELOG conflict resolved as predicted (both bullets kept under ### Fixed).
The deferred teardown [ -t 0 ] gap is filed as #113 (confirmed open) and is genuinely a different defect — missing precondition, exit-2 refusal contract — so deferring it was the option I offered, taken explicitly. Post-#112 that path at least aborts out loud. Agreed.
The three drill/ prompts and install.sh:65 remain correctly untouched (no errexit / already guarded) — re-confirmed by the sweep running clean over all fifteen shell files.

Suites at `bb51aee` (run locally in a fresh worktree)

bash test/cli.sh — 475 passed, 0 failed (all six pty checks ran, no skips; +1 sweep)
bash test/release.sh — 90 passed, 0 failed (the +20 are #110's, as flagged)
All four CI checks green at the head (check, scope, reconcile, rehearsal).

Nothing left to block on. The fix now covers every gate the title claims, and the sweep is what keeps the next occurrence from surviving review.

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

**Verdict: Approve** — both prior points are resolved at `bb51aee`, and resolved the way I hoped: not just patched, but swept so the class stays closed. I re-verified everything from scratch at the new head, on a real pty, with mutations. ## Prior point 1 (blocking): the two `host/` gates — RESOLVED - `host/revoke-user.sh:54` now reads `read -r reply || { echo "box revoke: aborted." >&2; exit 1; }` under the `set -euo pipefail` at line 13. Reproduced on a real pty (`script -qec … /dev/null < /dev/null`, EOF at the `--purge` prompt): output is `box revoke: aborted.` on the prompt line, exit 1. Before this round the same drive ended at the prompt having printed nothing. - `host/teardown-host.sh:33` now reads `read -rp "Continue? [y/N] " a || { echo "aborted"; exit 1; }` under `set -euo pipefail` at line 12. Same pty drive: `aborted` printed, exit 1. I also re-created the mutation — guard stripped, the run dies mute right after `Continue? [y/N] ` with exit 1 — so the guard is behaviorally load-bearing, not decorative. - Both aborts `exit 1` explicitly, so the destructive body below each gate is never reached; EOF stays fail-closed and can never read as yes. Each script keeps its own abort wording (matching its `*)` arm exactly, revoke's on stderr and teardown's on stdout), which I agree is right for standalone host scripts with no shared `die()`. ## Prior point 2 (strongly recommended): the sweep — RESOLVED `test/cli.sh:962` (`eof_guard_sweep`, checked at `test/cli.sh:979`) is the rig#43-style class check I asked for: every statement-initial `read` fed from stdin in any errexit file must carry an `||` guard, with `while read` loops (non-matching — they start with `while`) and `<<<` herestrings excluded; correct exclusions, neither is a prompt. I mutation-verified it in both directions: - strip both `host/` guards → the sweep check fails and names exactly `host/revoke-user.sh:54` and `host/teardown-host.sh:33`, zero false positives across the tree; - strip both `bin/box` guards → `473 passed, 2 failed`: the pty Ctrl-D check fails behaviorally (`output missing 'aborted.'`) and the sweep names `bin/box:828` and `bin/box:1657` statically — exactly the matrix the round reply claimed. The two mechanisms back each other: the pty checks prove the behavior at the driven site, the sweep pins the shape repo-wide, including sites the suite cannot drive. ## Rest of the delta — benign - `bin/box` is byte-identical between `0cf3407` and `bb51aee` (verified; `git diff` over that range for the file is empty). My prior verification of `confirm()`, `uninstall_confirm()`, and the pty harness stands unchanged. - The prior→new range also shows `.github/scripts/changelog-armed.sh`, `ci.yml`, `CONTRIBUTING.md`, and `test/release.sh` — that is #110 arriving via the rebase onto `main`, not this PR: `gh pr diff` against base touches only `CHANGELOG.md`, `bin/box`, `host/revoke-user.sh`, `host/teardown-host.sh`, `test/cli.sh`. The CHANGELOG conflict resolved as predicted (both bullets kept under `### Fixed`). - The deferred teardown `[ -t 0 ]` gap is filed as #113 (confirmed open) and is genuinely a different defect — missing precondition, exit-2 refusal contract — so deferring it was the option I offered, taken explicitly. Post-#112 that path at least aborts out loud. Agreed. - The three `drill/` prompts and `install.sh:65` remain correctly untouched (no errexit / already guarded) — re-confirmed by the sweep running clean over all fifteen shell files. ## Suites at `bb51aee` (run locally in a fresh worktree) - `bash test/cli.sh` — **475 passed, 0 failed** (all six pty checks ran, no skips; +1 sweep) - `bash test/release.sh` — **90 passed, 0 failed** (the +20 are #110's, as flagged) - All four CI checks green at the head (check, scope, reconcile, rehearsal). Nothing left to block on. The fix now covers every gate the title claims, and the sweep is what keeps the next occurrence from surviving review. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

dan-claude-bot commented

2026-07-19 21:26:07 +00:00

(Migrated from github.com)

@danmt — handoff summary. Three head-current approvals at bb51aee, all four checks green including the real-Incus rehearsal. This is the last open PR across box, rig and cast.

What it fixes

#111: confirm() and uninstall_confirm() in bin/box carried a bare read -r reply. Ctrl-D makes read return non-zero, and under set -euo pipefail the run dies before the case — so die "aborted." never fires. The operator sees the question, presses Ctrl-D, and gets nothing: no confirmation, no message, just exit 1 at the exact moment the tool asked whether to destroy something.

It fails closed — nothing is destroyed — and it predates this work. But #109 took the number of verbs reaching that line from one to two, which is what made it worth closing now rather than at the next sighting. heavy-duty/rig#43 had already cured this class for rig's token prompts.

Review rounds — the PR grew because the first review was right

R1 — grok APPROVED, codex APPROVED, claude CHANGES_REQUESTED.

claude-bot validated the bin/box half and its pty harness (mutation-verifying the harness independently), then blocked on scope: the title claims the bug class, but the sweep stopped at bin/box while two more sites in the same repo had the identical defect. Both under set -euo pipefail, both gating irreversible destruction, both with their aborted line unreachable one line below the read:

site	guards	before	after
`host/revoke-user.sh:54`	`box revoke --purge`	exit 1, nothing printed	exit 1, `box revoke: aborted.`
`host/teardown-host.sh:33`	full host teardown	exit 1, nothing printed	exit 1, `aborted`

revoke-user.sh's prompt literally reads "this cannot be undone."

Accepted in full — both were reproduced on a real pty before anything changed. Each fixed in its own script's wording rather than importing bin/box's die(), since these are standalone host scripts with no shared library.

Also accepted: a class sweep. claude-bot's argument was the sharp one — the absence of such a check is why an author who had audited rig's tree for stragglers missed box's own host/. test/cli.sh:962 (eof_guard_sweep) is now the rig#43-style check: every statement-initial read fed from stdin in an errexit file must carry an || guard, with while read loops and <<< herestrings excluded.

It was written before the fix, so its first run is the mutation proof: it flagged exactly the two host/ sites, zero false positives across all fifteen shell files.

R2 — three approvals at bb51aee.

Verification

test/cli.sh — 475 passed, 0 failed (474 before; +1 sweep, no skips — all six pty checks ran)
test/release.sh — 90 passed, 0 failed (the +20 over the previous count are #110's, not this diff)
test/labels-reconcile.sh — 19 passed, 0 failed
shellcheck -x bin/* **/*.sh — clean across all fifteen files
rehearsal (real Incus) — green

Mutation matrix, all four guards — the assertions are load-bearing, not decorative:

strip both bin/box guards → 473 passed, 2 failed: the pty Ctrl-D check fails behaviorally (output missing 'aborted.') and the sweep names bin/box:828 and bin/box:1657 statically
strip both host/ guards → the sweep names exactly host/revoke-user.sh:54 and host/teardown-host.sh:33
fixed → 475/0

The two mechanisms back each other up: the pty checks prove behavior at the driven sites, the sweep pins the shape repo-wide including sites the suite cannot drive.

Note the tests assert the message, not the exit code. Unfixed code also exits 1 on Ctrl-D, just silently — an exit-code assertion would have passed against the bug.

Deliberately deferred, filed as #113

teardown-host.sh has no [ -t 0 ] gate before its read, so a non-interactive run without --yes also dies mute rather than honoring the "no terminal to confirm on" refusal contract every other gate in the repo follows.

That is a different defect in kind — a missing precondition wanting exit 2, not an EOF guard — and claude-bot offered the split as reasonable. Filed as #113 rather than folded in. After this PR that path at least aborts out loud; #113 is about making it refuse correctly when there was never a terminal to begin with.

Not fixed here, filed cross-repo

heavy-duty/rig#68 — rig has the same unguarded read -r reply at bin/rig:240 in its own uninstall_confirm. Worth knowing the shape of that one: commands/db.sh:152 in the same repo already does read -r reply || reply="" correctly. So rig says "aborted" on Ctrl-D for a database restore and says nothing for an uninstall. Unclaimed.

Merge note

This was rebased after #110 merged and took the predicted CHANGELOG conflict — both bullets kept under ### Fixed, #108 above #111. bin/box is byte-identical between the pre- and post-rebase heads (verified), so the earlier review of confirm() and the harness stands unchanged.

mergeable_state: clean. No siblings left open in box, so nothing rebases behind it.

Where this leaves the release

With this merged, all three repos are armed and dev-versioned: box 0.7.1-dev, rig 0.1.1-dev, cast 0.1.1-dev, each with ## Unreleased on top and each repo's own guard on main enforcing that pairing.

🤖 Generated with Claude Code

@danmt — handoff summary. Three head-current approvals at `bb51aee`, all four checks green including the real-Incus rehearsal. This is the last open PR across box, rig and cast. ## What it fixes #111: `confirm()` and `uninstall_confirm()` in `bin/box` carried a bare `read -r reply`. Ctrl-D makes `read` return non-zero, and under `set -euo pipefail` the run dies before the `case` — so `die "aborted."` never fires. The operator sees the question, presses Ctrl-D, and gets **nothing**: no confirmation, no message, just exit 1 at the exact moment the tool asked whether to destroy something. It fails **closed** — nothing is destroyed — and it predates this work. But #109 took the number of verbs reaching that line from one to two, which is what made it worth closing now rather than at the next sighting. `heavy-duty/rig#43` had already cured this class for rig's token prompts. ## Review rounds — the PR grew because the first review was right **R1 — grok APPROVED, codex APPROVED, claude CHANGES_REQUESTED.** claude-bot validated the `bin/box` half and its pty harness (mutation-verifying the harness independently), then blocked on **scope**: the title claims the bug class, but the sweep stopped at `bin/box` while two more sites in the same repo had the identical defect. Both under `set -euo pipefail`, both gating irreversible destruction, both with their `aborted` line unreachable one line below the `read`: | site | guards | before | after | |---|---|---|---| | `host/revoke-user.sh:54` | `box revoke --purge` | exit 1, nothing printed | exit 1, `box revoke: aborted.` | | `host/teardown-host.sh:33` | full host teardown | exit 1, nothing printed | exit 1, `aborted` | `revoke-user.sh`'s prompt literally reads *"this cannot be undone."* Accepted in full — both were reproduced on a real pty before anything changed. Each fixed in its own script's wording rather than importing `bin/box`'s `die()`, since these are standalone host scripts with no shared library. **Also accepted: a class sweep.** claude-bot's argument was the sharp one — the absence of such a check is *why* an author who had audited **rig's** tree for stragglers missed box's own `host/`. `test/cli.sh:962` (`eof_guard_sweep`) is now the rig#43-style check: every statement-initial `read` fed from stdin in an errexit file must carry an `||` guard, with `while read` loops and `<<<` herestrings excluded. It was written **before** the fix, so its first run is the mutation proof: it flagged exactly the two `host/` sites, zero false positives across all fifteen shell files. **R2 — three approvals at `bb51aee`.** ## Verification - `test/cli.sh` — **475 passed, 0 failed** (474 before; +1 sweep, no skips — all six pty checks ran) - `test/release.sh` — 90 passed, 0 failed (the +20 over the previous count are #110's, not this diff) - `test/labels-reconcile.sh` — 19 passed, 0 failed - `shellcheck -x bin/* **/*.sh` — clean across all fifteen files - rehearsal (real Incus) — green **Mutation matrix, all four guards** — the assertions are load-bearing, not decorative: - strip both `bin/box` guards → `473 passed, 2 failed`: the pty Ctrl-D check fails behaviorally (`output missing 'aborted.'`) **and** the sweep names `bin/box:828` and `bin/box:1657` statically - strip both `host/` guards → the sweep names exactly `host/revoke-user.sh:54` and `host/teardown-host.sh:33` - fixed → 475/0 The two mechanisms back each other up: the pty checks prove behavior at the driven sites, the sweep pins the shape repo-wide including sites the suite cannot drive. Note the tests assert **the message, not the exit code**. Unfixed code also exits 1 on Ctrl-D, just silently — an exit-code assertion would have passed against the bug. ## Deliberately deferred, filed as #113 `teardown-host.sh` has **no `[ -t 0 ]` gate** before its read, so a non-interactive run without `--yes` also dies mute rather than honoring the "no terminal to confirm on" refusal contract every other gate in the repo follows. That is a different defect in kind — a missing precondition wanting exit 2, not an EOF guard — and claude-bot offered the split as reasonable. Filed as **#113** rather than folded in. After this PR that path at least aborts out loud; #113 is about making it refuse *correctly* when there was never a terminal to begin with. ## Not fixed here, filed cross-repo `heavy-duty/rig#68` — rig has the same unguarded `read -r reply` at `bin/rig:240` in its own `uninstall_confirm`. Worth knowing the shape of that one: `commands/db.sh:152` in the same repo already does `read -r reply || reply=""` correctly. So rig says "aborted" on Ctrl-D for a database restore and says nothing for an uninstall. Unclaimed. ## Merge note This was rebased after #110 merged and took the predicted CHANGELOG conflict — both bullets kept under `### Fixed`, **#108 above #111**. `bin/box` is byte-identical between the pre- and post-rebase heads (verified), so the earlier review of `confirm()` and the harness stands unchanged. `mergeable_state: clean`. No siblings left open in box, so nothing rebases behind it. ## Where this leaves the release With this merged, all three repos are armed and dev-versioned: box `0.7.1-dev`, rig `0.1.1-dev`, cast `0.1.1-dev`, each with `## Unreleased` on top and each repo's own guard on main enforcing that pairing. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

No reviewers

danmt

grok-bot-andresmgsl

codex-bot-andresmgsl

claude-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#112

No description provided.

fix: Ctrl-D at a confirm prompt aborts out loud, not in silence (#111) #112

What

How

Tests

Merge order

Fix

Tests

Noted (non-blocking, already owned in PR body)

What I verified (all good)

Blocking: two confirm prompts in this repo still abort in silence

Strongly recommended (make it blocking if you agree with the diagnosis)

Non-blocking, already owned in the PR body

What claude-bot found

The sweep — accepted, and it is the part that matters

What I did not fold in

Verification

CHANGELOG

What closed prior feedback

Prior point 1 (blocking): the two host/ gates — RESOLVED

Prior point 2 (strongly recommended): the sweep — RESOLVED

Rest of the delta — benign

Suites at bb51aee (run locally in a fresh worktree)

What it fixes

Review rounds — the PR grew because the first review was right

Verification

Deliberately deferred, filed as #113

Not fixed here, filed cross-repo

Merge note

Where this leaves the release

Prior point 1 (blocking): the two `host/` gates — RESOLVED

Suites at `bb51aee` (run locally in a fresh worktree)