Repository navigation
T22 — Rename and prune boxes - #3008
Conversation
min box rename renames a box as BOX-010 defines and ends with exit 2 on the daemon's refusal, which names the holder. min box prune reaps the stopped and exited boxes through PruneBoxes, narrowed by --older-than and --parent. Off a terminal it needs --yes and ends with exit 2.
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
There was a problem hiding this comment.
Reviewed at 96e10b1 against #2907 (BCLI-058, BCLI-061, BCLI-005). min box rename resolves through resolve_box_on_hosts, prints 'Renamed to .', and maps the daemon's refusals to exit 2. min box prune gates on --yes off a TTY or under --no-input (exit 2), asks [y/N] on a TTY, then calls PruneBoxes with --older-than/--parent and prints one line per removed box. The three plan tests exist and are deterministic. Two problems predate this task and are raised separately: rename of a running host_ip box on a VM host retires its egress (copied from min session rename), and the prune refusal names --dry-run, which BCLI-006 has not added yet (the text is the plan's).
|
/review |
|
Approved: no findings at Nothing to do. Clobber: none seen. |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The change conforms to the design. min box rename and min box prune live under min box, as principle 8 (line 338) requires. A held-name rename and a prune off a te
8000
rminal without --yes both exit 2 (lines 333 and 363). The daemon does the reap, and it removes only stopped or exited boxes (line 501). The --dry-run flag from line 485 is not in this change. Spec 28 files it under #2834 as the residual. The summary notes one gap in the box_id span field for --parent, which the design does not govern.
Ruled at 96e10b1 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
The terminal question said every stopped or exited box, even when --older-than limits the prune to older boxes.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare since the PASS on 96e10b15 changes only the question that min box prune asks on a terminal. The question now names the --older-than age as the time since the box stopped or exited. That matches architecture.md line 501. The off-terminal refusal is unchanged and still follows design principle 3. The earlier PASS stands for the rest of the change.
Ruled at 13ef8b9 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
Notes from the agent: Revise round 1. A new pure function prune_question builds the terminal question. box_prune_flags_map_to_box_021_selectors pins its four wordings. cargo clippy -p minimal --all-targets is clean. |
|
/revise Fix thread PRRT_kwDOPdfcxM6zz-tU (MEDIUM) at head 13ef8b9.
Collect each host's error and go on to the next host. At the end:
Fix the summary's plural while you are there: "of 1 boxes" must read "of 1 box". Add a test with two hosts where one fails: the other host is still pruned, and the command exits non-zero naming the failed host. Change nothing else. |
cmd_box_prune stopped at the first host error, so later hosts were never pruned and the summary was skipped. Collect each host's error, name each failed host on stderr, count it in the summary and exit non-zero. The box count now says "1 box" for one.
|
Round 2 of 10 on a /revise note, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
Notes from the agent: The host loop moved from cmd_box_prune into a new prune_hosts function, and the summary moved into finish_prune, so the new two-host test can drive them. The daemon-connection inventory in crates/minimal/src/tests.rs names the function that probes. Its entry for cmd_box_prune now names prune_hosts. The summary now says "1 of 1 box" for one box. A host failure also writes a tracing warn record, and the |
|
Round 3 of 10 on a failed check suite: the coder changed nothing. Revise round 3: the only directive items are the failed |
There was a problem hiding this comment.
Re-reviewed at acfcef1. acfcef1: prune_hosts no longer stops at the first failing host; each host's error goes in the tally, the remaining hosts are pruned, finish_prune prints one 'Could not prune host : ' line per failure and exits non-zero (thread zz-tU). 669a7be: the prune question names --older-than. The failed tests run was otel_cli::exec_run_attach_and_task_run_share_the_commands_trace, a flake (main green; rerun passed).
|
/review |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The new commit makes min box prune continue past a host that fails. It names each failed host on stderr and exits non-zero. Each host still passes the version gate before its prune, so the walk fails closed. The request still reaps only stopped or exited boxes, as architecture.md line 501 says. The design does not say how a prune across several hosts handles one failed host. The rest of the PR keeps the PASS on 13ef8b9.
Ruled at acfcef1 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
/revise Merge main into this branch, then fix the compile error it brings. On the merge-queue commit (run 41929133950, clippy): #2984 (T27, now on main) added |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
This head merges main and adapts one test call to the new answer_failure signature. The merge keeps both sides in cli.rs and cmd/mod.rs. The rename and prune code does not change, so the PASS on acfcef13 stands. Rename and prune conform to architecture.md lines 333, 363, 485–486 and 501. --dry-run stays a filed residual on #2834.
Ruled at 84a9a6c against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 4 of 10 on a /revise note, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review.
Notes from the agent: The merge of origin/main was already on the branch (a115ed7) with no conflict markers. The only change this round passes None for alias_hint in the answer_failure call in the prune_off_tty_needs_yes_exits_2 test. cargo clippy -p minimal --all-targets is clean. |
|
/review |
T22 — Rename and prune boxes
slice: S6
kind: task
covers: BCLI-058, BCLI-061, BCLI-005
size: M
est: 200 code, 180 tests
executor: agent
difficulty: routine
model: standard
diagnostics: off a terminal,
min box pruneprints "Prune removes boxes. Pass --yes to confirm, or --dry-run to see what it would remove." and exits 2, pinned by prune_off_tty_needs_yesobservability: the command's existing tracing span records the verb, the box id it resolved and the outcome
layers:
- client:
- crates/minimal/src/cli.rs: the verb's arguments and help in the one command model
- crates/minimal/src/cmd/boxes/lifecycle.rs:
min box renameandmin box prune [--older-than] [--parent] [--yes]- crates/minimal/src/main.rs:
failure_exit_codemaps this verb's refusals to their exit codes- crates/minimal/src/tests.rs: the in-process minimald test harness gains this layer's tests
test: cargo nextest run -p minimal box_rename_renames_as_box_010
test: cargo nextest run -p minimal box_prune_flags_map_to_box_021_selectors
test: cargo nextest run -p minimal prune_off_tty_needs_yes
produces: none
consumes:
- crates/minimal/src/cmd/boxes/address.rs
proof:
min box renamerenames a box, andmin box pruneremoves old boxes after--yes.depends on:
- blocked by #2895
Proof
A person can run
min box rename <box> <name>. It printsRenamed <box> to <name>., and a name that a running box holds is refused with exit 2 and names the holder.min box prune --yesremoves every stopped or exited box, prints oneRemoved <name> (<id>).line for each box, and never removes a running box.--older-than <d>and--parent <box>narrow the prune. Off a terminal without--yes, it exits 2 with the pinned message.Notes from the agent: Files outside the layers: crates/minimal/src/cmd/mod.rs gets the verb names and the dispatch arms for the two new BoxCommand variants, because the matches are exhaustive. crates/minimal/src/cmd/boxes.rs re-exports BoxRenameRefusal and PruneNeedsYes so that main.rs can map them to exit 2. The tests are in the lifecycle.rs test module beside the stop and rm tests, because the harness helpers are there. tests.rs only gets the entry for the new gated daemon connection in cmd_box_prune, which every_daemon_connection_is_classified requires. One full
cargo nextest run -p minimalpassed: 880 tests. Printed-text issues: (1) The task fixes the BCLI-005 message, so I copied it. It names--dry-run, butmin box prunehas no--dry-runflag yet, because BCLI-006 waits on #2834. Today the message tells the person to use a flag that clap rejects. (2) The rename refusal text comes from boxes::box_name::RenameRefusal, outside this task's layers. It saysThe running sessionand suggestsmin session rename, notmin box rename. The client detects that text by its lead, and the test pins the detection against the daemon's real reply. Behaviour notes: without --parent, the prune runs on every local host that is running. With --parent, it runs only on the host that keeps the parent box. On a terminal without --yes, the prune asks one [y/N] question before it connects. --no-input works the same as no terminal. The tracing records verb, box_id and outcome for rename, and verb, reaped, failed and …[truncated]Tests
cargo nextest run -p minimal box_rename_renames_as_box_010cargo nextest run -p minimal box_prune_flags_map_to_box_021_selectorscargo nextest run -p minimal prune_off_tty_needs_yesVerify
min task run agent-verifyexited 100 under rustc 1.99.0, which the repository does not pin (1.99.1 in rust-toolchain.toml); the PR is not a draft on that alone — CI, which honours the pin, is the gate.Out of scope
These files are outside the task's
layers:, so this PR is a draft:crates/minimal/src/cmd/boxes.rscrates/minimal/src/cmd/mod.rsCloses #2907
Undrafted by the operator (agent-137) at 96e10b1: the out-of-layer edits are wiring only. cmd/boxes.rs re-exports BoxRenameRefusal and PruneNeedsYes for failure_exit_code, and cmd/mod.rs adds the box rename/prune arms to the exhaustive command_verb and run_command matches.