Repository navigation
T111 — Host-side commands find a box on any VM, a stopped VM included, and attach errors stay on screen - #2811
Conversation
A stopped VM's boxes can now be named and listed by the CLI. The read never writes or sets aside a file it cannot use.
Policy and destroy resolve the box's VM from its name. A box no running VM holds names --vm, and a box on a stopped VM names the command that starts it. min ls lists a stopped VM's boxes as stopped. A relay failure writes the unwind before the attach error prints.
Record the new connect sites in the daemon connection inventory.
…commands-find-a-box-on-any-vm-a-stoppe
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
There was a problem hiding this comment.
Babysitter review of 375f68a against T111 (#2705).
- A box name now picks its VM for attach (
resolve_box_on_another_vm), exec, run andmin net forward(resolve_box_vm), andmin session policyandmin session destroy(resolve_box_on_any_vm). - Both refusals match the plan text.
box_on_stopped_vmdocuments whymin --vm <vm> session attach <box>starts the stopped VM (ensure_daemon→autospawn::ensure_minvmd_running). min lslists a stopped VM's boxes from its persisted records.- Each named test exists.
Notes:
- The plan names
min stop <box>, butmin stoptakes no box name (StopArgshas only--force), solifecycle.rsneeds no change. That item of the plan is void. - The out-of-layer
crates/minimal/tests/cli.rschange is threerunning: truefield inits that the new listing field forces.
|
Approved: every review thread resolved at @norrietaylor resolved the last thread, which releases the approval this review had withheld. Nothing to do. |
…commands-find-a-box-on-any-vm-a-stoppe
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The change resolves a box's VM from its name for exec, run, net forward, policy, destroy and attach, and min ls lists a stopped VM's boxes. One case does not conform. A non-selected running VM that owns the name silently wins over a stopped VM that also holds it. architecture.md line 389 says a name that matches on more than one host is an error listing the candidates. C1 makes resolve_box_vm return that error. C2 files the undelivered min stop <box> part of NET-058 as an issue.
Ruled at 375f68a against gominimal/arch@8640f85e193d by claude-opus-5-5.
|
/revise Two items on 375f68a, from the arch-review check and minimal-review.
Arch C2 ( |
|
That Nothing to type again: the command gets a rocket when its round starts, or a refusal that names the reason. No round was counted for it yet. |
A name that more than one VM holds now fails with AmbiguousBoxName. min session policy -o json maps it to SessionNotFound, not DaemonUnreachable.
There was a problem hiding this comment.
arch-review: CONDITIONS · 3 conditions · the full ruling
The resolution across VMs follows the Addressing rule in most cases, but one new case picks a VM when it must refuse. In resolve_box_vm, a stopped record on a running VM wins over a stopped VM that holds the same name. Line 389 makes that an error that lists the candidates (C3). Because min session destroy uses this path, the wrong pick can destroy the wrong box. The new typed refusals exit 1, but the Exit codes table gives 4 for not found and 2 for ambiguous (C1). The min stop <box> leg is not delivered and the PR body links no issue for it (C2).
Ruled at 87dda39 against gominimal/arch@8640f85e193d 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:
With two VMs, Notes from the agent: Revise round 1. Added the typed error |
…commands-find-a-box-on-any-vm-a-stoppe
A running owner and a stopped VM's record of the same name are now an ambiguity that lists both VMs and names --vm. The listing comments cite NET-152.
…commands-find-a-box-on-any-vm-a-stoppe
|
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:
With two VMs, min session policy beta, min net forward beta 8080:3000, exec and session destroy find beta on its VM without --vm. After min --vm alpha stop, min ls lists alpha's boxes as stopped, and min session attach beta names the VM to start and the command that starts it. The error stays on screen. A name that a running VM and a stopped VM both hold is now refused. The refusal lists both VMs, marks the stopped one, and names --vm. Notes from the agent: Item 1: resolve_box_vm now returns AmbiguousBoxName when one running VM owns the name and a stopped VM's records hold it. The text is "box name 'api' exists on more than one VM (default, gamma (stopped)); name the one you mean with --vm". The several-owners arm uses the same text and also lists stopped holders. The test also pins that the selected VM's own box still resolves through resolve_box_on_any_vm. Item 2 was already on the branch in 87dda39 (policy.rs maps AmbiguousBoxName to SessionNotFound, and box_on_stopped_vm_names_start_command asserts the typed marker). This round made no further change for it. Item 3: list.rs and the ls_lists_boxes_on_stopped_vm doc now cite NET-152. The NET-148 citations in crates/minvmd/src/box_registry.rs were not in the note and stay. Residuals for a person to file: exit codes 4 (BoxNotFound) and 2 (AmbiguousBoxName) need crates/minimal/src/main.rs failure_exit_code, which is outside the layers. A Relay::start failure needs a not-taken marker in crates/minimal-client/src/tty_relay.rs, which is outside the layers. I have no credential to open issues. just clippy-strict was not run. |
|
/revise One item on 0ca0427, arch C3 from the review of 87dda39.
Update the doc comment to say "a running box wins". In Do not edit the PR body. It already lists #2815 and #2816 as box CLI residuals. |
A running VM that owns the name with an active box resolves there even when a stopped VM's records hold the same name. A non-active owner plus a stopped holder still refuses as ambiguous and names --vm.
|
Round 3 of 10 on a /revise note, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. With two VMs, Notes from the agent: Round 3 addresses the /revise note (arch C3). In |
…commands-find-a-box-on-any-vm-a-stoppe
A name that more than one VM holds, running or stopped, is now an ambiguity that lists every VM and names --vm, though the running box is active. The arm that let an active box win is gone. An attach whose relay fails before ssh's session reaches the terminal (the pty, the ssh spawn or raw mode fail) no longer writes the unwind. The relay calls its suspend hook only once it started, so a failure before that call is marked and the guard stands down.
There was a problem hiding this comment.
arch-review: CONDITIONS · 1 condition · the full ruling
The compare fixes the cross-VM tie. resolve_box_vm now refuses a name that two VMs hold, as Addressing line 389 requires. One path still picks without an error. resolve_box_on_any_vm and resolve_named_box_version_gated return the selected VM's box on a hit. They never ask the other VMs. So with no --vm, min session destroy shared acts on the default VM's box when alpha also holds shared. Line 389 says an unqualified name on more than one host is an error. C1 sends a hit through resolve_box_vm when several VMs exist, and adds a test on the command path.
Ruled at af11107 against gominimal/arch@8640f85e193d 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. Review threads the coder judged:
With two VMs, Notes from the agent: Directive items 1 to 3: removed the active-box-wins arm in resolve_box_vm, rewrote its doc comment, and restored the 385b33c 'api' assertion in box_on_stopped_vm_names_start_command. The 'web' case stays. Item 4 is real. Every error that Relay::start returns (pty, ssh spawn, raw mode, pump thread) comes before ssh's session reaches the real terminal, and the relay puts back the termios itself, so the guard wrote a blind unwind. The fix stays inside the layers: a new marker crate::attach::RelayNotStarted wraps the error and prints the same text. The new mark_relay_not_started in cmd/boxes/attach.rs always passes the relay a suspend hook, and the relay calls that hook only after Relay::start returns. A failure before that call gets the marker, and interactive_attach disarms the guard on it. One limit: the hook runs on its own thread. So if Relay::join fails at once (only the wait on ssh's exit can fail), the guard can stand down although the session ran. The termios is still restored in that case. A marker set in minimal-client's Relay::start is the exact fix, but that crate is outside this task's layers. attach_error_printed_after_terminal_restore now pins that an ssh spawn failure writes no unwind and prints 'failed to run ssh: spawning the ssh attach: ...'. just clippy-strict was not run. |
|
/revise Arch C1 on af11107. This follows from the person's ruling that a name held by more than one VM is ambiguous. A hit on the selected VM still returns at once, so a second VM that holds the same name is never seen. In
Do not edit the PR body. |
…commands-find-a-box-on-any-vm-a-stoppe
A hit on the selected VM returned at once, so a second VM that holds the same name was never seen. With several VMs and no --vm, policy, destroy, attach, exec, run and net forward now check every VM and refuse the name with the ambiguity error.
|
Round 5 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:
With two VMs, Notes from the agent: Round 5 (Arch C1): a new |
There was a problem hiding this comment.
Babysitter review of 6d670cc. Arch C1 is closed. With several VMs and no --vm, a hit on the selected VM now runs ensure_selected_holds_box_alone. That walks every VM through resolve_box_vm, which returns AmbiguousBoxName (naming each VM and --vm) when another VM, running or stopped, holds the name. With one VM, or with --vm, no extra lookup runs. Tests cover both destroy and attach. Undrafting: the out-of-layer tests/cli.rs change was accepted earlier.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The change now conforms. A hit on the selected VM is checked against every VM before the command uses it. A name that two VMs hold is refused with every candidate listed, as Addressing line 389 requires. All box-naming commands go through the two checked helpers. The exit codes (#2816) and min stop <box> (#2815) stay as filed residuals.
Ruled at 6d670cc against gominimal/arch@8640f85e193d by claude-opus-5-5.
A daemon matches an exact name before an id prefix. The walk across VMs now does the same: when any VM holds the name as a name, a VM that matches only an id prefix is not a candidate.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare conforms. A box name now wins over an id prefix across VMs, the same order the daemon uses in match_id_prefix. A name that two VMs hold as a name is still refused with the list of candidates, as architecture.md line 389 requires. The rest of the change keeps the status of the PASS on 6d670cc. Residuals #2816 and #2815 stay open.
Ruled at f721ac4 against gominimal/arch@8640f85e193d by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 10 unresolved threads — minimal-arch[bot] crates/minimal/src/attach.rs, minimal-arch[bot] crates/minimal/src/attach.rs, minimal-arch[bot] crates/minimal/src/tests.rs:6007, minimal-arch[bot] crates/minimal/src/attach.rs, minimal-arch[bot] crates/minimal/src/attach.rs, …. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
|
Round 6 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:
With two VMs, min session policy beta, min net forward beta 8080:3000, exec and stop find beta on its VM without --vm. After min --vm alpha stop, min ls lists the boxes of alpha as stopped. min session attach beta then says which VM to start and how, and the error stays on screen. In this round, a box name such as 'cafe' on a running VM also resolves when a stopped VM holds a box whose id starts with 'cafe'. Notes from the agent: Revise round 6. resolve_box_vm now records how each VM holds the name: as a name or only as an id prefix. When any VM holds it as a name, the id-prefix-only holders are dropped, as match_id_prefix does on one daemon. ensure_selected_holds_box_alone in cmd/mod.rs calls resolve_box_vm, so the selected-VM path gets the same rule. The new case is in box_on_stopped_vm_names_start_command. |
There was a problem hiding this comment.
minimal-review — approved
Verdict APPROVE — every review thread is resolved; @norrietaylor closed the last one. The review of f721ac4f1039 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.
Babysitter review of f721ac4. Round 6 makes a name match win over an id prefix across VMs, as match_id_prefix does on one daemon. Stopped and running holders are both classified, and id-prefix holders drop out only when some VM holds the name as a name. The cafe test pins it. All 10 threads are answered at this head. Non-blocking: the (0, several) message says "more than one stopped VM" when one holder is a running VM with a stopped box. The ambiguity is right and only the label is loose.
Policy and destroy resolve a box's VM from its name, as attach, exec, run and net forward do. A box no running VM holds names --vm, a box on a stopped VM names the command that starts it, and a name any two VMs hold, running or stopped, is refused with every VM listed. A relay failure writes the unwind before the attach error prints, so the error stays on screen. Backport of #2811. Adapted to the line: cmd/boxes/attach.rs and policy.rs hunks land in cmd/session.rs. Dropped, because the line has no stopped session status (#2676) and keeps box ids apart from session ids (#2485): min ls rows for a stopped VM's boxes, vm_running in its JSON, and id-prefix matching against a stopped VM's records, which match by name only here. (cherry picked from commit 2e3962f)
Policy and destroy resolve a box's VM from its name, as attach, exec, run and net forward do. A box no running VM holds names --vm, a box on a stopped VM names the command that starts it, and a name any two VMs hold, running or stopped, is refused with every VM listed. A relay failure writes the unwind before the attach error prints, so the error stays on screen. Backport of #2811. Adapted to the line: cmd/boxes/attach.rs and policy.rs hunks land in cmd/session.rs. Dropped, because the line has no stopped session status (#2676) and keeps box ids apart from session ids (#2485): min ls rows for a stopped VM's boxes, vm_running in its JSON, and id-prefix matching against a stopped VM's records, which match by name only here. (cherry picked from commit 2e3962f)
T111 — Host-side commands find a box on any VM, a stopped VM included, and attach errors stay on screen
slice: S17
kind: task
covers: NET-058, NET-152, NET-148
size: L
est: 380 code, 420 tests
executor: agent
difficulty: needs care
model: strongest
diagnostics: what a person reads is the error itself: a box on no running VM names
--vm, a box on a stopped VM names that VM and the command that starts it, and a failed attach leaves its error on screen;min lsshows the stopped VM's boxes as stoppedobservability: the existing debug line
box name resolved to a VMgains the outcomeon a stopped VMwith the VM's name, and one debug line per stopped VM read records how many boxes its persisted records holddepends on:
- blocked by #2696
layers:
- resolution:
- crates/minimal/src/attach.rs: builds on open pull request #2626, which already resolves a box's VM for exec, task run and
min net forward;resolve_box_vmalso reads each stopped VM's persisted box records, and answers a box found only there as on a stopped VM; the attach path restores the terminal (the unwind and the attach-start termios) before it prints any error that endsmin session attach, so the error stays on screen after exit 1- crates/minimal/src/cmd/session.rs: from #2626,
min session execandmin session runresolve the VM from the box name- crates/minimal/src/cmd/net.rs: from #2626,
min net forwardresolves the VM from the box name- crates/minimal/src/cmd/boxes/attach.rs: from #2626; prints the resolution's error only after the terminal is restored
- crates/minimal/src/cmd/boxes/policy.rs:
min session policy <box>resolves the VM from the box name- crates/minimal/src/cmd/boxes/lifecycle.rs:
min stop <box>andmin session destroy <box>resolve the VM from the box name- crates/minimal/src/cmd/mod.rs: the not-found error for a box no running VM holds reads
error: no running VM has a box named 'beta'. A box on another VM is named with --vm, for example: min --vm <vm> session policy beta, using the command the user ran in place ofsession policy; a box held only by a stopped VM readserror: box 'beta' is on VM 'alpha', which is stopped. Start it with: min --vm alpha session attach beta; the implementer confirms againstcrates/minimal/src/autospawn.rsthat the named command starts a stopped VM and pins the text in the testtest: cargo nextest run -p minimal box_name_resolves_vm_without_flag
test: cargo nextest run -p minimal unknown_box_names_vm_flag
test: cargo nextest run -p minimal box_on_stopped_vm_names_start_command
test: cargo nextest run -p minimal attach_error_printed_after_terminal_restore
- stopped-vm-records:
- crates/minvmd/src/box_registry.rs: a read-only accessor over a VM state directory's persisted creations file that returns each box's name and id without a running daemon; a file of another version, or one that does not parse, reads as no boxes and is never written
test: cargo nextest run -p minvmd persisted_box_names_read_without_a_daemon
- listing:
- crates/minimal/src/cmd/list.rs:
min lslists the boxes of every VM the user created; a stopped VM's boxes come from its persisted records, each row's STATUS readsstoppedand its VM cell readsalpha (stopped);--jsoncarries them with"status": "stopped"and"vm_running": false; the table rule extends under the VM column- crates/minimal/src/tests.rs: pins both refusal texts, the stopped rows in the table and in
--json, and that an attach error prints after the restore sequencetest: cargo nextest run -p minimal ls_lists_boxes_on_stopped_vm
produces: none
consumes: none
proof: With two VMs,
min session policy beta,min net forward beta 8080:3000, exec and stop findbetaon its VM without--vm; aftermin --vm alpha stop,min lslists alpha's boxes as stopped andmin session attach betasays which VM to start and how, with the error still on screen.Proof
On a host with two VMs,
min session policy beta,min net forward beta 8080:3000,min session exec beta ...,min session run beta <task>,min session attach betaandmin session destroy betafindbetaon its VM without--vm. A name that no running VM holds gets:error: no running VM has a box named 'beta'. A box on another VM is named with --vm, for example: min --vm <vm> session policy beta, with the command the user ran in place ofsession policy. Aftermin --vm alpha stop,min lslists alpha's boxes with STATUSstoppedand the VM cellalpha (stopped), and the rule line now runs under the VM column.min ls --jsongives those boxes"status": "stopped"and"vm_running": false.min session attach betathen printserror: box 'beta' is on VM 'alpha', which is stopped. Start it with: min --vm alpha session attach beta. This error comes before the attach touches the terminal, so it stays on screen.min stop <box>is not delivered: see notes.Notes from the agent: 1.
min stoptakes no box argument. It stops the selected VM, andStopArgsin crates/minimal/src/cli.rs has no box field. cli.rs is outside the layers, somin stop <box>does not resolve a VM from a box name.min session destroy <box>does. The proof'sstoppart is true only asmin --vm alpha stop, which works already. A follow-up task that adds a box argument tomin stopmust name cli.rs. Also, cmd_destroy and cmd_session_policy (text) live in cmd/session.rs, not in cmd/boxes/lifecycle.rs or cmd/boxes/policy.rs. So lifecycle.rs needed no change. policy.rs changed only formin session policy -o json.2. Out of scope: crates/minimal/tests/cli.rs.
VmListingis a public struct with a newrunningfield, and that file builds it with struct literals at three sites. The only change there isrunning: trueat each site.3. Confirmed against autospawn.rs:
--vm alphais published before path resolution (cli.rs publish_vm_name).cmd_attachcalls `e…[truncated]Residuals
failure_exit_codeincrates/minimal/src/main.rs). Box CLI work, by the same ruling. This PR keeps exit 1, asmaindoes.min stop <box>.min stoptakes no box name (StopArgsincrates/minimal/src/cli.rs), so the proof'sstopleg is not delivered here. By the person's ruling of 2026-10-31, this belongs to the box CLI work, not to NET.Tests
cargo nextest run -p minimal box_name_resolves_vm_without_flagcargo nextest run -p minimal unknown_box_names_vm_flagcargo nextest run -p minimal box_on_stopped_vm_names_start_commandcargo nextest run -p minimal attach_error_printed_after_terminal_restorecargo nextest run -p minvmd persisted_box_names_read_without_a_daemoncargo nextest run -p minimal ls_lists_boxes_on_stopped_vmVerify
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.Base
The base branch moved while the coder worked and was merged in before the tests ran. These files changed on BOTH sides since the branch point — read them first, because a rewrite from a stale reading can undo a merge without a conflict:
crates/minimal/src/tests.rsOut of scope
These files are outside the task's
layers:, so this PR is a draft:crates/minimal/tests/cli.rsCloses #2705