Repository navigation
T2 — Stop and remove a box - #2961
Conversation
`min box stop <box> [--force]` stops a box over StopBox and keeps its record. `min box rm <box> [--force]` removes a box over ReapBox. A running box without `--force` is refused on stderr with exit 2, and the refusal names `--force` (BCLI-059, BCLI-060, BCLI-004). The command's root span records the verb, the resolved box id and the outcome.
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
|
Approved: no findings at Nothing to do. Verdict APPROVE, posted as a comment: the App cannot review its own pull request; the check and the label carry it. Clobber: none seen. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 1 condition · the full ruling
min box stop and the running-box refusal of min box rm conform. min box rm does not do the creator's host-side release that min session destroy does. Architecture line 458 makes the type nouns aliases of the min box forms, so the two must end a box the same way. On a LocalVM, a removed box keeps its row, its reserved address and its host_ip name hold with the VM host daemon (networking §7.1 line 244). Make the change in C1. Separately, add daemon_with_boxes to the connect-site inventory in crates/minimal/src/tests.rs, because every_daemon_connection_is_classified fails at this head.
Ruled at 8d5ac60 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
The race branch in min box rm matched the daemon's text with no test. Extract the match into reap_error_is_running and check it against a real unforced ReapBox of a running box.
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
min box stop and the running-box refusal of min box rm conform. min box rm stops at the in-VM ReapBox. On a minvmd host, the host row, its address, the host_ip egress and the zone hold stay behind. Networking §7.1 lines 244 and 255 give that withdrawal to the host-side creator, never in-VM minimald. Share the retirement in destroy_box with the rm path, as C1 says. File the exit-code follow-up as an issue, or remove the sentence, as C2 says. Separately, the verify run fails every_daemon_connection_is_classified because of the new daemon_with_boxes connect site.
Ruled at 95fd359 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
A person stops a box with Notes from the agent: Revise round 1. The match on the daemon's running-box text moved into reap_error_is_running. rm_running_box_refuses_naming_force now sends an unforced ReapBox for a running box to the test daemon and asserts that the helper maps the daemon's answer to the refusal. cargo clippy -p minimal --all-targets printed no warnings. |
Read the box record before the reap and, after it, withdraw the row or release the held name as destroy_box does. Classify the test helper's daemon connection in the connect-site inventory.
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
min box rm now withdraws the row or releases the name hold after the reap, as destroy_box does. That closes the open point from the last head. One gap stays on min box stop. A stopped host_ip box keeps its rules in the VM host daemon's cohort union, because only a client retires them. Section 4.3 line 84 admits the cohort by live boxes only, and §5.5 line 183 says a stopped box has no live egress. C1 adds one best-effort retire call after the stop. The gap is filed as #2747, so the merge can go ahead with C1 open.
Ruled at 4229238 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 2 of 10 on a failed check suite, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
A person stops a box with Notes from the agent: Revise round 2. The |
|
/revise From the maintainer's pre-merge review at
|
The running-box refusal now ends with exit 2 from failure_exit_code, so a machine-mode run gets the min/v1/error object. The root span owns verb, box_id and outcome: box stop and box rm join command_verb and record the resolved id through output::record_box_id. min box rm prints its line after the row and name cleanup, so a closed stdout cannot skip it. min box stop retires a host_ip box's egress rules on the VM host daemon. The stop test reads the daemon's log to prove the forced stop, and covers the line for a box that exited before the stop.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
This head conforms. min box stop now retires a host_ip box's egress on the VM host daemon, and that meets the open condition from 4229238a (§5.5 line 183). The running-box refusal of min box rm now ends through failure_exit_code with exit 2, so a machine mode gets the min/v1/error object (lines 363 and 383). Close #2982 with this merge.
Ruled at 2f568b7 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 8 unresolved threads — minimal-arch[bot] crates/minimal/src/cmd/boxes/lifecycle.rs, minimal-arch[bot] crates/minimal/src/cmd/boxes/lifecycle.rs:805, minimal-foundry[bot] crates/minimal/src/cmd/boxes/lifecycle.rs, minimal-arch[bot] crates/minimal/src/cmd/boxes/lifecycle.rs, minimal-arch[bot] crates/minimal/src/cmd/boxes/lifecycle.rs, …. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
|
Round 3 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
A person stops a box with Notes from the agent: Revise round 3. Main was already merged at d20ec86, so no merge was necessary. Item 1: |
There was a problem hiding this comment.
minimal-review — approved
Verdict APPROVE — every review thread is resolved; @mitodrummer closed the last one. The review of 2f568b7232e2 withheld this approval on the threads open then. Posted as a comment: the App cannot approve its own pull request; the check and the label carry it.
There was a problem hiding this comment.
Reviewed at 2f568b72. A whole-PR review at 4229238a found one blocker (the build broke against main after T5), and this round fixes it.
- Build: main is merged, and
command_verbhasStop→ "box stop" andRm→ "box rm". - Exit code: the running-box refusal goes through
failure_exit_code(exit 2), replacingTaskExit(2). - Cleanup:
withdraw_box_rowandrelease_held_box_namerun before the stdout line. - Stop:
--forcereaches StopBox. The record stays after a stop (BOX-018). - Rm: the reap's running refusal and the listing check map to the same message.
- Residuals: #2982 (
host_ipretire after a stop). The stopped-box exit code and wording are noted on T33 #2923. - Follow-up: the forced half of
box_stop_stops_as_box_013_force_forcesstill provesforceonly on the request.
|
/revise The merge queue removed this PR at 04:45 ( T1 widened |
T1 widened find_listed_box to take the host and the not-found remedy. min box stop and min box rm now look the box up on the selected host, as min box show does, and their refusal names the host and the verb.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare since the PASS on 2f568b7 is one branch commit and a merge from main. The commit makes min box stop and min box rm look the box up on the selected host, as min box show does. Line 389 and line 353 of architecture.md address a box on a host, so this conforms. The stop, reap, --force refusal and exit 2 do not change. The residual #2982 stays open.
Ruled at 899be48 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 4 of 10 on a /revise note, pushed as
Verify A person stops a box with Notes from the agent: The merge with main (2b46c80) was already on the branch and had no conflicts. stop_listed_box and rm_listed_box now take a BoxLookup (host and not-found remedy). The callers build it from local_hosts with the selected host, the same as cmd_box_show. With one host the remedy is not_found_remedy's text. With several hosts the remedy names the verb that ran ( |
There was a problem hiding this comment.
Re-approved at 899be482. It merges main with T1 (#2963) and passes host and remedy to find_listed_box in stop_listed_box and rm_listed_box, which fixes the E0061 that ejected the merge-queue entry at 04:45. Every lane is green on this head, and arch passes.
T2 — Stop and remove a box
slice: S0
kind: task
covers: BCLI-059, BCLI-060, BCLI-004
size: M
est: 200 code, 180 tests
executor: agent
difficulty: routine
model: standard
diagnostics: when the box is running and
--forceis absent,min box rmprints " is running. Stop it with min box stop , or remove it now with min box rm --force." on stderr and exits 2, pinned by rm_running_box_refuses_naming_forceobservability: 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/mod.rs: dispatches the verb
- crates/minimal/src/cmd/boxes.rs: exports the stop and remove verbs
- crates/minimal/src/cmd/boxes/lifecycle.rs:
min box stop [--force]over StopBox andmin box rm [--force]over ReapBoxtest: cargo nextest run -p minimal box_stop_stops_as_box_013_force_forces
test: cargo nextest run -p minimal box_rm_reaps_as_box_019_force_forces
test: cargo nextest run -p minimal rm_running_box_refuses_naming_force
produces: none
consumes: none
proof: A person stops a box with
min box stopand removes it withmin box rm, and a running box refuses removal without--force.Proof
A person runs
min box stop <box>to stop a box and keep its record.--forcekills its processes at once. The person runsmin box rm <box>to remove a stopped or exited box. For a running box without--force,min box rmprints " is running. Stop it with min box stop , or remove it now with min box rm --force." on stderr, exits 2 and leaves the box running. With--force, it stops the box and removes it.Notes from the agent: crates/minimal/src/cmd/boxes.rs needs no change: it already re-exports everything in lifecycle.rs with
pub(crate) use lifecycle::*, so the newcmd_box_stopandcmd_box_rmreach the dispatch through that line. Exit 2: main.rs (outside the layers) maps error types to exit codes. To stay inside the layers,cmd_box_rmprints the refusal to stderr itself and returnsminimal::task::TaskExit(2), which main.rs relays as the exit code without printing more. A follow-up can giveBoxRunningRefusalits own arm infailure_exit_codein main.rs. The client checks for a running box with the state from the ListBoxes listing before it calls ReapBox. It also maps the daemon's own running-box refusal to the same message, in case the box starts between the two calls. Observability: the rootcmdspan in cmd/mod.rs now declaresverb,box_idandoutcome, and the two verbs record them.min box stop --exec(also in BCLI-059) is not part of this task and is not implemented.min box rmdoes not withdraw a VM host row or release a held name the waymin session destroydoes. It relies on the daemon's ReapBox for that, as BOX-019 defines the reap. Success lines are "Stopped .", " exited before the stop, so the stop changed nothing." and "Removed ." on stdout. The tests use the in-process minimald test harness. They check the request each verb sends, the state ListBoxes reports after each verb, and the exact refusal text. I did not runjust cior a run of the whole crate.Tests
cargo nextest run -p minimal box_stop_stops_as_box_013_force_forcescargo nextest run -p minimal box_rm_reaps_as_box_019_force_forcescargo nextest run -p minimal rm_running_box_refuses_naming_forceVerify
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.Closes #2890
Out-of-layer edit accepted by the maintainer (2026-11-04):
crates/minimal/src/tests.rsholds the in-process minimald test harness that these tests use. The plan should have named it in the layers.Residuals
min box stopdoes not yet retire the box'shost_ipegress on the VM host daemon (arch C1 on 4229238). Arch rules this can merge with C1 open.