heavy-duty/box

Fork 1

fix: box restore asks before it destroys, in its own words (#105) #109

Merged

dan-claude-bot merged 3 commits from fix/restore-confirm into main

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

dan-claude-bot commented

2026-07-19 19:41:12 +00:00

(Migrated from github.com)

Fixes #105.

What

box restore destroyed user state with no prompt and no --force. The
table gave it box,arg2 — the instance is ours, a snapshot name is
present, go — so box restore work stale-label silently discarded
everything done in the box since that snapshot. rm, on the very next
row, has always asked.

The one-token fix the issue proposes does not work, and that is the real
finding here. confirm was already a precondition token, but the
dispatch line hardcoded the words:

case ",$pre," in *,confirm,*) confirm "delete $inst and all its snapshots" ;; esac

Adding confirm to restore would have gated it behind a prompt
offering to delete the box the operator was trying to rescue. That is
worse than no gate: it is how people learn to answer y without reading.
So the prompt had to become row-driven first.

How

The prompt is a seventh table field. Same ^-separated idiom, same
{}/{1} substitution as the ok message, documented in the table header
next to the other fields. {} is the resolved instance rather than the
name as typed — the ok message reports on what you asked for, a prompt
about to destroy something should say which machine (so --remote
prints lab:work, not work).

The two rows:

"restore^<box> <snapshot> [--force]^box,arg2,confirm^…^incus:snapshot restore^restored {} to {1}^roll {} back to snapshot '{1}' and discard everything in the box since it was taken"
"rm^<box> [--force]^box,confirm^…^incus:delete -f^removed {}^delete {} and all its snapshots"

confirm() wraps that into box: <prompt>? this cannot be undone. [y/N]
and into the no-TTY refusal, so --force, the TTY check and refusing
rather than assuming consent all come free — that function already had
them.

Three small consequences, kept minimal:

The {}/{1} substitution was inline in the dispatch's ok branch and
now has two callers, so it is a three-line fill(). No behaviour
change to the ok messages.
field() gained cnf, and the two other IFS='^' read sites gained a
trailing placeholder so a six-var read cannot silently swallow the new
field into ok.
A row marked confirm with no words dies as an internal error rather
than asking a blank question.

rm's wording is byte-identical. It was the string that lived in the
dispatch; it moved, it did not change. A test pins it verbatim, because
rewording the one verb that already worked would be a regression shipped
as a refactor.

Help. box help restore now documents the prompt and --force, and
the general --force line names restore alongside rm.

Not in this PR, deliberately

The stopped precondition. #105 raises it and explicitly wants it
decided on its own rather than smuggled in with the confirm. Agreed —
it is a UX cost on the common case. --help now states the situation
instead: snapshots here are stateless (nothing in bin/ or host/
passes --stateful, nothing mentions migration.stateful), so a
rollback of a running box is crash-consistent, like a machine coming
back from losing power. No user-facing text claims otherwise.
reset_identity. #105 asks that nobody add it. Nobody did.

Tests

Coverage for restore was two argument-validation checks
(test/cli.sh:58, :777); neither ever reached dispatch, which is why
four releases of an ungated destructive verb shipped green. The
destructive path is now driven against a fake incus that logs every
call — the same shim discipline the grant/revoke blocks use:

refusing without --force on a closed stdin exits 2, and the call log
is empty — the assertion that proves the gate actually held, not
merely that it printed
the prompt contains "discard everything in the box since it was taken"
and does not contain "delete work"
--force exits 0, prints restored work to authed, and produces
exactly one incus snapshot restore work authed
rm unchanged in both directions: refusal text pinned verbatim,
nothing deleted; --force still reaches incus delete -f work
a table invariant (awk over CMDS): every row whose preconditions
contain confirm has a seventh field and it is non-empty — and it
fails closed if it matches no rows at all
the dispatch pin: the prompt comes from $cnf, not a constant

Stdin is closed on every driven run on purpose — confirm() branches on
[ -t 0 ], and a suite run from a terminal would otherwise block waiting
for a human.

The rehearsal found the one caller. CI's multi-user rehearsal drives
box restore unattended on real Incus (drill/multiuser.sh:206) and
took the new no-TTY refusal: 56 passed, 1 failed, (b) restore failed. That is the gate working, so the fix is in the caller — it now
passes --force, which is the drill proving the gate is real rather
than working around it. Deliberately not BOX_YES=1: that variable
is installer-family only and confirm() does not honor it, by design
(bin/box:1646-1651), so the installer's own BOX_YES=1 cannot make
box rm skip its prompt. --force is the only correct answer.

The suite should have caught this and did not, so it now pins the exact
no-TTY wording too. Audited the rest of drill/, test/, host/,
install.sh and .github/: multiuser.sh:206 was the only
non-interactive box restore. drill/drill.sh already drives box rm
with --force and already asserts its no-TTY refusal.

Docs swept for anything implying rm is the only gated verb: the
--force option line, the restore row summary, box help restore,
and README's command table and prose.

Verified on this branch:

bash test/cli.sh — 468 passed, 0 failed (was 454)
bash test/release.sh — 70 passed, 0 failed
shellcheck -x bin/box test/cli.sh — clean

🤖 Generated with Claude Code

Fixes #105. ## What `box restore` destroyed user state with no prompt and no `--force`. The table gave it `box,arg2` — the instance is ours, a snapshot name is present, go — so `box restore work stale-label` silently discarded everything done in the box since that snapshot. `rm`, on the very next row, has always asked. The one-token fix the issue proposes does not work, and that is the real finding here. `confirm` was already a precondition token, but the dispatch line hardcoded the *words*: ```bash case ",$pre," in *,confirm,*) confirm "delete $inst and all its snapshots" ;; esac ``` Adding `confirm` to `restore` would have gated it behind a prompt offering to **delete** the box the operator was trying to rescue. That is worse than no gate: it is how people learn to answer `y` without reading. So the prompt had to become row-driven first. ## How **The prompt is a seventh table field.** Same `^`-separated idiom, same `{}`/`{1}` substitution as the ok message, documented in the table header next to the other fields. `{}` is the *resolved* instance rather than the name as typed — the ok message reports on what you asked for, a prompt about to destroy something should say which machine (so `--remote` prints `lab:work`, not `work`). The two rows: ``` "restore^<box> <snapshot> [--force]^box,arg2,confirm^…^incus:snapshot restore^restored {} to {1}^roll {} back to snapshot '{1}' and discard everything in the box since it was taken" "rm^<box> [--force]^box,confirm^…^incus:delete -f^removed {}^delete {} and all its snapshots" ``` `confirm()` wraps that into `box: <prompt>? this cannot be undone. [y/N]` and into the no-TTY refusal, so `--force`, the TTY check and refusing rather than assuming consent all come free — that function already had them. Three small consequences, kept minimal: - The `{}`/`{1}` substitution was inline in the dispatch's ok branch and now has two callers, so it is a three-line `fill()`. No behaviour change to the ok messages. - `field()` gained `cnf`, and the two other `IFS='^' read` sites gained a trailing placeholder so a six-var read cannot silently swallow the new field into `ok`. - A row marked `confirm` with no words dies as an internal error rather than asking a blank question. **`rm`'s wording is byte-identical.** It was the string that lived in the dispatch; it moved, it did not change. A test pins it verbatim, because rewording the one verb that already worked would be a regression shipped as a refactor. **Help.** `box help restore` now documents the prompt and `--force`, and the general `--force` line names `restore` alongside `rm`. ## Not in this PR, deliberately - **The `stopped` precondition.** #105 raises it and explicitly wants it decided on its own rather than smuggled in with the confirm. Agreed — it is a UX cost on the common case. `--help` now states the situation instead: snapshots here are stateless (nothing in `bin/` or `host/` passes `--stateful`, nothing mentions `migration.stateful`), so a rollback of a running box is crash-consistent, like a machine coming back from losing power. No user-facing text claims otherwise. - **`reset_identity`.** #105 asks that nobody add it. Nobody did. ## Tests Coverage for `restore` was two argument-validation checks (`test/cli.sh:58`, `:777`); neither ever reached dispatch, which is why four releases of an ungated destructive verb shipped green. The destructive path is now **driven** against a fake `incus` that logs every call — the same shim discipline the grant/revoke blocks use: - refusing without `--force` on a closed stdin exits 2, and the call log is **empty** — the assertion that proves the gate actually held, not merely that it printed - the prompt contains "discard everything in the box since it was taken" and does **not** contain "delete work" - `--force` exits 0, prints `restored work to authed`, and produces exactly one `incus snapshot restore work authed` - `rm` unchanged in both directions: refusal text pinned verbatim, nothing deleted; `--force` still reaches `incus delete -f work` - a table invariant (awk over `CMDS`): every row whose preconditions contain `confirm` has a seventh field and it is non-empty — and it fails closed if it matches no rows at all - the dispatch pin: the prompt comes from `$cnf`, not a constant Stdin is closed on every driven run on purpose — `confirm()` branches on `[ -t 0 ]`, and a suite run from a terminal would otherwise block waiting for a human. **The rehearsal found the one caller.** CI's multi-user rehearsal drives `box restore` unattended on real Incus (`drill/multiuser.sh:206`) and took the new no-TTY refusal: `56 passed, 1 failed`, `(b) restore failed`. That is the gate working, so the fix is in the caller — it now passes `--force`, which is the drill proving the gate is real rather than working around it. Deliberately **not** `BOX_YES=1`: that variable is installer-family only and `confirm()` does not honor it, by design (`bin/box:1646-1651`), so the installer's own `BOX_YES=1` cannot make `box rm` skip its prompt. `--force` is the only correct answer. The suite should have caught this and did not, so it now pins the exact no-TTY wording too. Audited the rest of `drill/`, `test/`, `host/`, `install.sh` and `.github/`: `multiuser.sh:206` was the only non-interactive `box restore`. `drill/drill.sh` already drives `box rm` with `--force` and already asserts its no-TTY refusal. Docs swept for anything implying `rm` is the only gated verb: the `--force` option line, the `restore` row summary, `box help restore`, and README's command table and prose. Verified on this branch: - `bash test/cli.sh` — 468 passed, 0 failed (was 454) - `bash test/release.sh` — 70 passed, 0 failed - `shellcheck -x bin/box test/cli.sh` — clean 🤖 Generated with [Claude Code](https://claude.com/claude-code)

danmt (Migrated from github.com) reviewed 2026-07-19 19:41:12 +00:00

codex-bot-andresmgsl (Migrated from github.com) requested changes 2026-07-19 19:46:17 +00:00

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

Verdict: I have feedback.

Blocking: update the required multi-user rehearsal for the new non-interactive contract. drill/multiuser.sh:209 still runs box restore mine s1 without --force; under the rehearsal's non-TTY execution the new confirmation gate correctly refuses it, and the current rehearsal check fails with FAIL: (b) restore failed. Pass --force there (and keep the lifecycle assertion) so the end-to-end required check exercises the intended explicit-consent path and returns green.

Verdict: I have feedback. Blocking: update the required multi-user rehearsal for the new non-interactive contract. `drill/multiuser.sh:209` still runs `box restore mine s1` without `--force`; under the rehearsal's non-TTY execution the new confirmation gate correctly refuses it, and the current `rehearsal` check fails with `FAIL: (b) restore failed`. Pass `--force` there (and keep the lifecycle assertion) so the end-to-end required check exercises the intended explicit-consent path and returns green.

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 19:49:20 +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 19:53:30 +00:00

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

Reviewed at 531803a. Approving — I verified each of the claims that matter for a destructive-action gate, and they hold.

What restore destroys vs. what the prompt says. incus snapshot restore discards all instance state newer than the snapshot, and these snapshots are stateless (no --stateful anywhere in the tree, confirmed). The row's prompt — "roll {} back to snapshot '{1}' and discard everything in the box since it was taken" (bin/box:81) — states exactly that, names the label (the actual risk is picking the wrong one), and uses the resolved instance so --remote prints lab:work. The issue's one-token fix would indeed have asked the operator to confirm deleting the box (confirm "delete $inst and all its snapshots" was hardcoded in the old dispatch); making the prompt a row field is the right cure, not gold-plating.

Prompt fires before any destructive step. Dispatch order at bin/box:1948-1957 is resolve (a read-only incus config get probe), arg check, then confirm, then the single incus snapshot restore call. No destructive work happens before or between — one gate, one action.

Headless behavior. With no TTY and no --force, confirm() (bin/box:821-828) refuses loudly via usage_error (exit 2, names both the act and the way out). --force/-f is the documented bypass — global parser at bin/box:726, documented in the --force option line, box help restore, and README. BOX_YES deliberately does not bypass it (installer-family only, bin/box:1643-1651) — the right call, and the drill fix at drill/multiuser.sh:215 correctly uses --force rather than reaching for it.

Consistency with rm. Both destructive verbs now go through the identical row-driven mechanism; rm's wording moved without changing a byte, pinned verbatim in both directions (refusal text + --force reaching incus delete -f). A confirm row with no words dies as an internal error (bin/box:1955), and the awk table invariant fails closed if it matches no confirm rows at all.

Tests are driven, not grepped. The load-bearing assertion is present: refusal leaves the fake-incus call log empty (test/cli.sh:837), --force produces exactly one incus snapshot restore work authed (test/cli.sh:850), the prompt text is pinned positively ("discard everything...") and negatively (no "delete work"), the no-TTY refusal wording is pinned (the regression test for this PR's own CI failure), and the rehearsal's explicit consent is pinned. Stdin is closed on every driven run so a terminal-run suite cannot hang — correct.

Verified locally on the PR head: bash test/cli.sh — 468 passed, 0 failed; bash test/release.sh — 70 passed, 0 failed; shellcheck -x bin/box test/cli.sh clean. Matches the PR's stated numbers.

One non-blocking note, for a follow-up. confirm()'s read -r reply at bin/box:826 is unguarded: Ctrl-D at a real interactive prompt makes read return non-zero and set -euo pipefail ends the run silently with exit 1 — no "aborted.", nothing after the question. This is exactly the pattern heavy-duty/rig#43 cured with read ... || die. It fails closed (nothing is destroyed) and it predates this PR — rm has always had it — so it is not this change's bug and I am not blocking on it. But this PR doubled the number of verbs that reach that line; a one-token read -r reply || die "aborted." (and the same in uninstall_confirm at bin/box:1654) would close the family's known silent-EOF gap. The interactive abort (n) and accept (y) paths are untested for the same structural reason — they need a pty — which is acceptable here, but worth a script-based check if one ever grows.

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

Reviewed at 531803a. Approving — I verified each of the claims that matter for a destructive-action gate, and they hold. **What restore destroys vs. what the prompt says.** `incus snapshot restore` discards all instance state newer than the snapshot, and these snapshots are stateless (no `--stateful` anywhere in the tree, confirmed). The row's prompt — "roll {} back to snapshot '{1}' and discard everything in the box since it was taken" (`bin/box:81`) — states exactly that, names the label (the actual risk is picking the wrong one), and uses the resolved instance so `--remote` prints `lab:work`. The issue's one-token fix would indeed have asked the operator to confirm *deleting* the box (`confirm "delete $inst and all its snapshots"` was hardcoded in the old dispatch); making the prompt a row field is the right cure, not gold-plating. **Prompt fires before any destructive step.** Dispatch order at `bin/box:1948-1957` is resolve (a read-only `incus config get` probe), arg check, then `confirm`, then the single `incus snapshot restore` call. No destructive work happens before or between — one gate, one action. **Headless behavior.** With no TTY and no `--force`, `confirm()` (`bin/box:821-828`) refuses loudly via `usage_error` (exit 2, names both the act and the way out). `--force`/`-f` is the documented bypass — global parser at `bin/box:726`, documented in the `--force` option line, `box help restore`, and README. `BOX_YES` deliberately does not bypass it (installer-family only, `bin/box:1643-1651`) — the right call, and the drill fix at `drill/multiuser.sh:215` correctly uses `--force` rather than reaching for it. **Consistency with `rm`.** Both destructive verbs now go through the identical row-driven mechanism; `rm`'s wording moved without changing a byte, pinned verbatim in both directions (refusal text + `--force` reaching `incus delete -f`). A confirm row with no words dies as an internal error (`bin/box:1955`), and the awk table invariant fails closed if it matches no confirm rows at all. **Tests are driven, not grepped.** The load-bearing assertion is present: refusal leaves the fake-incus call log empty (`test/cli.sh:837`), `--force` produces exactly one `incus snapshot restore work authed` (`test/cli.sh:850`), the prompt text is pinned positively ("discard everything...") and negatively (no "delete work"), the no-TTY refusal wording is pinned (the regression test for this PR's own CI failure), and the rehearsal's explicit consent is pinned. Stdin is closed on every driven run so a terminal-run suite cannot hang — correct. Verified locally on the PR head: `bash test/cli.sh` — 468 passed, 0 failed; `bash test/release.sh` — 70 passed, 0 failed; `shellcheck -x bin/box test/cli.sh` clean. Matches the PR's stated numbers. **One non-blocking note, for a follow-up.** `confirm()`'s `read -r reply` at `bin/box:826` is unguarded: Ctrl-D at a real interactive prompt makes `read` return non-zero and `set -euo pipefail` ends the run silently with exit 1 — no "aborted.", nothing after the question. This is exactly the pattern heavy-duty/rig#43 cured with `read ... || die`. It fails **closed** (nothing is destroyed) and it predates this PR — `rm` has always had it — so it is not this change's bug and I am not blocking on it. But this PR doubled the number of verbs that reach that line; a one-token `read -r reply || die "aborted."` (and the same in `uninstall_confirm` at `bin/box:1654`) would close the family's known silent-EOF gap. The interactive abort (`n`) and accept (`y`) paths are untested for the same structural reason — they need a pty — which is acceptable here, but worth a `script`-based check if one ever grows. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

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

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

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

Why this is the right shape

The one-token fix would have been worse than no gate: confirm was already a precondition, but dispatch hardcoded rm's "delete …" wording. Row-driven prompts (7th ^ field, same {}/{1} as ok) are the real fix — restore names the snapshot and the loss, not a deletion.

Load-bearing pieces checked at tip `531803a`

restore row: box,arg2,confirm + prompt roll {} back to snapshot '{1}' and discard everything in the box since it was taken
rm wording byte-identical to the old dispatch constant (pinned in suite)
fill() + confirm "$(fill "$cnf" "$inst")" — {} is resolved instance (so --remote names the machine)
empty cnf on a confirm row dies as internal error
Driven shim tests: no-TTY refuse leaves incus log empty; --force produces exactly one snapshot restore; prompt must not contain delete work
drill/multiuser.sh passes --force (codex R1 closed); suite pins that line

Deliberate non-goals (stopped precondition, no reset_identity) are correctly left out.

Checks: unit/scope green; rehearsal was pending at review time (the contract the suite now pins).

**Verdict: Approve** — I agree with this as-is. ### Why this is the right shape The one-token fix would have been worse than no gate: `confirm` was already a precondition, but dispatch hardcoded rm's "delete …" wording. Row-driven prompts (`7th` `^` field, same `{}`/`{1}` as ok) are the real fix — restore names the snapshot and the loss, not a deletion. ### Load-bearing pieces checked at tip `531803a` - `restore` row: `box,arg2,confirm` + prompt `roll {} back to snapshot '{1}' and discard everything in the box since it was taken` - `rm` wording byte-identical to the old dispatch constant (pinned in suite) - `fill()` + `confirm "$(fill "$cnf" "$inst")"` — `{}` is resolved instance (so `--remote` names the machine) - empty `cnf` on a confirm row dies as internal error - Driven shim tests: no-TTY refuse leaves incus log empty; `--force` produces exactly one `snapshot restore`; prompt must not contain `delete work` - `drill/multiuser.sh` passes `--force` (codex R1 closed); suite pins that line Deliberate non-goals (`stopped` precondition, no `reset_identity`) are correctly left out. Checks: unit/scope green; rehearsal was pending at review time (the contract the suite now pins).

dan-claude-bot commented

2026-07-19 20:21:42 +00:00

(Migrated from github.com)

@danmt — handoff summary. Three head-current approvals, all checks green including the real-Incus rehearsal.

What it fixes

box restore irreversibly discarded everything since the snapshot with no prompt and no --force (#105). rm and restore are the two verbs that destroy user state, and only one of them asked.

The reason it shipped ungated for four releases is the interesting part, and it is why this diff is larger than the issue implies. confirm already existed as a precondition token, but the dispatch hardcoded rm's words:

case ",$pre," in *,confirm,*) confirm "delete $inst and all its snapshots" ;; esac

So the one-token fix in the issue would have gated restore behind a prompt offering to delete the box being rescued. The prompt had to become a per-row field first. It is now a seventh ^ field using the same {}/{1} idiom as the ok message, with {} bound to the resolved instance — a destruction prompt should name the machine, so --remote prints lab:work.

rm's wording moved byte-identical and is pinned verbatim in both directions. A confirm row with no words is now a hard internal error, so this class cannot recur silently.

Review rounds

R1 — codex, CHANGES_REQUESTED. The rehearsal was red: drill/multiuser.sh drove box restore unattended, the new gate correctly refused a non-TTY invocation, and the required check failed FAIL: (b) restore failed (56 passed, 1 failed).

Fixed in the caller with --force, not by weakening the gate. Worth recording why that distinction mattered: BOX_YES=1 was the tempting shortcut and would have been wrong — it is installer-family only (bin/box:1643-1651), deliberately not honored by confirm(), because the installer exports it and must still have box rm refuse. Audited drill/, test/, host/, install.sh, .github/: that was the only non-interactive box restore in the tree.

The failure is also the strongest evidence the gate works — it proved caller-visible reach on live Incus, which the unit suite had not. test/cli.sh now pins the exact no-TTY refusal wording as the regression test for this PR's own CI failure.

R2 — codex APPROVED, claude APPROVED, grok APPROVED, all at 531803a. Rehearsal re-ran green: 57 passed, 0 failed.

Verification

bash test/cli.sh — 468 passed, 0 failed (454 before, 14 added)
bash test/release.sh — 70 passed, 0 failed
shellcheck -x bin/box test/cli.sh — clean
rehearsal (real Incus, container mode) — 57 passed, 0 failed

Tests are driven, not grepped: refusal leaves the fake-incus call log empty, --force produces exactly one incus snapshot restore work authed, and the prompt is pinned positively ("discard everything…") and negatively (must not contain "delete work"). Stdin is closed on every driven run so a terminal-run suite cannot hang.

Flagged for your judgment

A pre-existing silent-EOF gap that this PR doubles the exposure of. Raised by claude-bot as explicitly non-blocking; I agree it should not block, but it should not be lost either.

confirm()'s read -r reply (bin/box:826) is unguarded. Ctrl-D at a real interactive prompt makes read return non-zero, and under set -euo pipefail the run ends silently with exit 1 — no "aborted.", nothing after the question. This is the same defect heavy-duty/rig#43 already cured with read ... || die.

It fails closed (nothing is destroyed) and predates this PR — rm has always had it — so it is not this change's bug. But this PR takes the number of verbs reaching that line from one to two. The cure is one token in two places (confirm() and uninstall_confirm() at bin/box:1654).

Your call whether that rides a follow-up issue or gets folded in here. I lean follow-up: it is a different defect with a different blast radius, and rig#43 gives it a precedent to point at.

Related: the interactive accept/abort paths (y/n) are untested, structurally — they need a pty. Acceptable as-is, but a script-based check would close it if one ever grows.

Merge note

#109 and #110 conflict on one anchor — both insert at the top of ## Unreleased → ### Fixed. Pure adjacency, no semantic overlap; keep both entries. Whichever merges second needs a rebase, and that push retires its approvals. Suggest merging #109 first, since re-validating it costs a rehearsal run and #110's diff (script + prose + tests) is cheaper to re-check.

🤖 Generated with Claude Code

@danmt — handoff summary. Three head-current approvals, all checks green including the real-Incus rehearsal. ## What it fixes `box restore` irreversibly discarded everything since the snapshot with no prompt and no `--force` (#105). `rm` and `restore` are the two verbs that destroy user state, and only one of them asked. **The reason it shipped ungated for four releases is the interesting part**, and it is why this diff is larger than the issue implies. `confirm` already existed as a precondition token, but the dispatch hardcoded rm's words: ```bash case ",$pre," in *,confirm,*) confirm "delete $inst and all its snapshots" ;; esac ``` So the one-token fix in the issue would have gated restore behind a prompt offering to **delete the box being rescued**. The prompt had to become a per-row field first. It is now a seventh `^` field using the same `{}`/`{1}` idiom as the ok message, with `{}` bound to the *resolved* instance — a destruction prompt should name the machine, so `--remote` prints `lab:work`. `rm`'s wording moved byte-identical and is pinned verbatim in both directions. A `confirm` row with no words is now a hard internal error, so this class cannot recur silently. ## Review rounds **R1 — codex, CHANGES_REQUESTED.** The rehearsal was red: `drill/multiuser.sh` drove `box restore` unattended, the new gate correctly refused a non-TTY invocation, and the required check failed `FAIL: (b) restore failed` (56 passed, 1 failed). Fixed in the caller with `--force`, not by weakening the gate. Worth recording why that distinction mattered: `BOX_YES=1` was the tempting shortcut and would have been wrong — it is installer-family only (`bin/box:1643-1651`), deliberately not honored by `confirm()`, because the installer exports it and must still have `box rm` refuse. Audited `drill/`, `test/`, `host/`, `install.sh`, `.github/`: that was the only non-interactive `box restore` in the tree. The failure is also the strongest evidence the gate works — it proved caller-visible reach on live Incus, which the unit suite had not. `test/cli.sh` now pins the exact no-TTY refusal wording as the regression test for this PR's own CI failure. **R2 — codex APPROVED, claude APPROVED, grok APPROVED**, all at `531803a`. Rehearsal re-ran green: 57 passed, 0 failed. ## Verification - `bash test/cli.sh` — **468 passed, 0 failed** (454 before, 14 added) - `bash test/release.sh` — 70 passed, 0 failed - `shellcheck -x bin/box test/cli.sh` — clean - rehearsal (real Incus, container mode) — **57 passed, 0 failed** Tests are *driven*, not grepped: refusal leaves the fake-incus call log empty, `--force` produces exactly one `incus snapshot restore work authed`, and the prompt is pinned positively ("discard everything…") and negatively (must not contain "delete work"). Stdin is closed on every driven run so a terminal-run suite cannot hang. ## Flagged for your judgment **A pre-existing silent-EOF gap that this PR doubles the exposure of.** Raised by claude-bot as explicitly non-blocking; I agree it should not block, but it should not be lost either. `confirm()`'s `read -r reply` (`bin/box:826`) is unguarded. Ctrl-D at a real interactive prompt makes `read` return non-zero, and under `set -euo pipefail` the run ends silently with exit 1 — no "aborted.", nothing after the question. **This is the same defect heavy-duty/rig#43 already cured** with `read ... || die`. It fails **closed** (nothing is destroyed) and predates this PR — `rm` has always had it — so it is not this change's bug. But this PR takes the number of verbs reaching that line from one to two. The cure is one token in two places (`confirm()` and `uninstall_confirm()` at `bin/box:1654`). Your call whether that rides a follow-up issue or gets folded in here. I lean follow-up: it is a different defect with a different blast radius, and rig#43 gives it a precedent to point at. Related: the interactive accept/abort paths (`y`/`n`) are untested, structurally — they need a pty. Acceptable as-is, but a `script`-based check would close it if one ever grows. ## Merge note **#109 and #110 conflict on one anchor** — both insert at the top of `## Unreleased` → `### Fixed`. Pure adjacency, no semantic overlap; keep both entries. Whichever merges second needs a rebase, and that push retires its approvals. Suggest merging **#109 first**, since re-validating it costs a rehearsal run and #110's diff (script + prose + tests) is cheaper to re-check. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

No reviewers

danmt

codex-bot-andresmgsl

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

No description provided.