Repository navigation
T58 — Stop min inventing box names and carry the held-name refusal to the user - #2766
mitodrummer merged 20 commits into
Conversation
CreateSession gains fresh_instance. The daemon then names the box <name>-<n> through create_fresh_box_with_spec. A create under a name a running box holds is refused with a reply that names the holder, which a client reads back with HeldNameRefusal::from_error (BOX-163).
min no longer mints <dir>-<hex> names. A create carries a name only for --name, and the daemon names the box from its entry. A held entry name makes the create a fresh instance, <name>-<n>. Task runs are always fresh instances. A --name that a running box holds ends min with exit 2, naming the holder (BOX-163). The VM host row is registered under the name the daemon will give the box, and a held row name is no longer re-minted. cmd/mod.rs loses the minting re-exports, main.rs maps the refusal to exit 2, and the vm_host test that pinned the re-mint now pins the refusal.
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
…nventing-box-names-and-carry-the-held
|
/revise The maintainer accepts these files outside the layers: the four already edited (crates/minimal/src/cmd/boxes/vm_host.rs, crates/minimal/src/cmd/mod.rs, crates/minimal/src/main.rs, crates/minimald-rpc/src/tests.rs), plus crates/minimald/src/rpc/tests.rs, scripts/session-e2e.sh and crates/minimald/src/box_manager.rs. Touch nothing outside those and the layers.
|
|
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. |
Task runs no longer mint task-<task>-<hex>. The daemon names a task box from its entry with the smallest unused suffix.
|
Round 1 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. min creates a box named from its entry and invents no name. A second task run of a held name gets -, as e2e-echo-1 in the e2e index shows. A create under a running box's name exits 2, and the refusal names the holder by name and id. Notes from the agent: Revise round 1 addressed two failed checks. (1) tests: minimald rpc::tests::create_session_errors_if_name_not_unique still pinned the old text "A session with that name already exists". It now pins the new refusal that names the holder. The file crates/minimald/src/rpc/tests.rs is the test module of the crates/minimald/src/rpc.rs layer. (2) native-daemon-e2e: scripts/session-e2e.sh matched task boxes by the old minted name task--. This task changes that name to -, so the script now matches e2e-echo-, e2e-fail- and e2e-port-run-. scripts/session-e2e.sh is outside the task's layers. I touched it because the task's intended behaviour change broke its name pins, and no other fix keeps the e2e lane green. ci-linux-native-success is the aggregator and fails because of the two jobs above. Before the fix, a full run of -p minimal -p minimal-client -p minimald-rpc -p minimald gave 2260 passed and 1 failed. I did not run the e2e script on this box, so the native-daemon-e2e lane still owes that evidence. |
An entry such as build_all or ci.lint is not a DNS label. The daemon now maps an entry-derived name onto one, so min task run build_all creates build-all-1. A [box] name or --name stays unchanged. The held-name test checks the holder and the rename hint, not the whole sentence.
|
/revise Within the files already accepted. |
|
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. |
|
/revise Also merge origin/main first: #2723 (node-plane baseline set) and #2767 (bep re-certify) landed after this branch and conflict with it in create.rs, vm_host.rs, dash/rpc.rs, task.rs, box_registration.rs, minimald-rpc lib.rs, minimald rpc.rs, minimal tests.rs and scripts/session-e2e.sh. Resolve by keeping both sides: main's additions stay, and this branch's BOX-163 changes (no invented names, the fresh-instance field, the holder refusal and exit 2) apply on top. Then do the |
|
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. |
|
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. min creates a box named from its entry and invents no name. A second run of a held name gets -. A create under a running box's name exits 2 and names the holder. A task entry such as build_all now creates the box build-all-1 and is not refused. Notes from the agent: Revise round 2. Item 1: the commit 5b891c4 was already on the branch, but it pinned the full sentence. The test create_session_errors_if_name_not_unique now checks the holder's name, its id and the rename hint, not the exact sentence. Item 2: commit 9ac0487 already changed every minted-name check in scripts/session-e2e.sh to -, with the comment and the e2e-port-run checks too. A grep found no other task-- or hex-suffix pattern on minted names, so this round did not change the script. Item 3: box_manager.rs adds entry_box_name. It lowercases, maps -, _ and . to -, drops other characters, caps the name at 48 octets and trims - at the ends. An empty result becomes |
…nventing-box-names-and-carry-the-held # Conflicts: # crates/minimal/src/cmd/boxes/create.rs # crates/minimal/src/task.rs
creation_name() cleaned spec.box_name(), which is the [box] name when set. It now reads spec.entry, so an invalid [box] name is refused, not rewritten.
|
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. Earlier threads cover the name mismatch between the VM host row and the daemon, and the fresh-instance retry branch. The current code resolves both. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 1 condition · the full ruling
The change stops min from inventing box names, and the daemon names a box from its entry as BOX-163 requires. One path deviates. activate_box() sends a refused create again as a fresh instance when the person typed no --name. The Addressing section, line 389, says start refuses a held name with exit 2 and names the holder. Only run and --new create <name>-<n>. Remove that fallback as C1 describes. Then the change conforms.
Ruled at f760828 against gominimal/arch@8640f85e193d by claude-opus-5-5.
|
Round 3 of 10 on a /revise note, pushed as Resolved conflicts in:
Verify Outside the task's
Converted to a draft: something above needs a person before review. min creates a box named from its entry with no invented name. A second run of a held name gets -. A create under a running box's name exits 2 and names the holder. A [box] name that is not a valid label, such as My_Box, is now refused and not rewritten to my-box. Notes from the agent: Merge of origin/main: only create.rs and task.rs had conflict markers. In both files I kept main's HostIpResidency::admit and put this branch's daemon_box_name read-back after it. In task.rs, main's |
|
/revise Two fixes. The maintainer decided both and accepts one more file outside the layers: crates/boxes/src/lib.rs (its
minimal-review PRRT_kwDOPdfcxM6yMdS8 (the VM-host path refusing where native renamed) is resolved by fix 1, because both paths now refuse. Touch nothing else. |
…nventing-box-names-and-carry-the-held
A plain create under a name that a running box holds no longer retries as a fresh instance. It ends with exit 2 and names the holder, with or without --name. Only min task run asks for a fresh instance.
|
/revise One item only, minimal-review PRRT_kwDOPdfcxM6yTLwG: in crates/minimal/src/task.rs (~line 560), |
…nventing-box-names-and-carry-the-held # Conflicts: # crates/minimal/src/cmd/boxes/create.rs
interactive_task_box_name sat between the comment and the function, so rustdoc attached the comment to the helper.
|
/revise Also merge origin/main: the branch conflicts with main again. Resolve by keeping both sides: main's changes stay, and this branch's BOX-163 changes apply on top. Then do the doc-comment move above. Change nothing else. |
|
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. |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The new commits do not change the ruling. The merge of main has one resolved conflict in activate_box. That resolution keeps the name that the daemon gave the box, and it keeps the T110 at_own_address argument. The other commit only moves a doc comment in task.rs. BOX-163 conforms to Addressing line 389, as it did on 47dbf1b2.
Ruled at 3f582ac against gominimal/arch@8640f85e193d by claude-opus-5-5.
|
/revise One item, minimal-review PRRT_kwDOPdfcxM6yTu83: in crates/minimal/src/cmd/boxes/create.rs |
|
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. |
|
Round 1 of 10 on a /revise note, pushed as Resolved conflicts in:
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
min creates a box named from its entry with no invented name. A second run of a held name gets -. A create under a running box's name exits 2 and names the holder. In the rendered docs, the cmd_task_run doc comment is on cmd_task_run again. Notes from the agent: Merge of origin/main: create.rs had two conflicts. In the first, main reworded the autogen re-mint retry block, and this branch removes that block. I kept the removal. In the second, I kept this branch's box_name and added main's at_own_address, which host_line_beside now takes. cargo clippy -p minimal --all-targets is clean after the merge. The directive asked for one change, the task.rs doc comment move, and I changed nothing else. The commitlint failure is in an earlier commit on the branch ("fix(minimald): take a [box] name unchanged and clean only the entry"): one body line is longer than 100 characters. A fix needs a history rewrite, and these instructions forbid that, so the check stays red until a person rewords that commit or the PR is squash-merged with a new message. |
|
The runtime will not revise this pull request again on its own: the diff is +960/-549 (1509 lines), past 3x the plan's estimate of 500 lines (est: 300 code, 200 tests, as #2762 reads now), at round 1 (the last trigger was a /revise note). A person decides now. Remove |
mint_into_request withdrew under request.config.name. That is None for an activation with no --name, so the row stayed published. It now withdraws under the name the row was registered under.
|
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:
min creates a box named from its entry and invents no name. A second run of a held name gets -. A create under a running box's name exits 2 and names the holder. Now an activation with no --name whose grant mint fails also withdraws its VM host row. Notes from the agent: Round 2. mint_into_request takes the name the row was registered under (box_name), and its failure arm withdraws with that 10BC8 name, not request.config.name. The new test unnamed_activation_withdraws_its_row_when_the_grant_mint_fails is in create.rs. It runs a real minvmd control server, registers an own-address row for "web", and calls mint_into_request with a request that has no name. The mint fails before it reads the home directory, because the request's network mode (none) gives the proxy no stance. I checked that the test fails when the old line is restored. Merge of origin/main: nothing to do. origin/main has no commits that this branch lacks, because the merge f6c22f0 is already on the branch. The doc-comment move is already done in 3f582ac. The commitlint failure is in two commits already on the branch (f-prefixed 'take a [box] name unchanged' and 3f582ac), whose bodies have lines over 100 characters. I cannot fix them without a rewrite of pushed commits, which the rules forbid. A squash merge, or a person who rewords them, clears that check. My new commit keeps body lines under 72 characters. |
|
/revise One line: CI |
The new unit test builds a CreateSession that is never sent, so the inventory names it as one that asserts nothing.
|
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. min creates a box named from its entry with no invented name. A second run of a held name with a fresh instance gets -. A create under a running box's name exits 2 and names the holder. This round also makes the create-site guard list the new row-withdrawal test, so the CI tests check passes again. Notes from the agent: This round has one change, as the note asks: the guard list in every_create_session_asserts_the_daemon_build now has unnamed_activation_withdraws_its_row_when_the_grant_mint_fails, in its sorted place. The commitlint failure was not fixed. It comes from two commits already on the branch (f760828 and 3f582ac), and their body lines are longer than 100 characters. A fix for it needs a rewrite of those commits, and the instructions do not permit that. The new commit's lines are all shorter than 100 characters. ci-linux-native-success only aggregates the tests job, so it will pass when tests passes. |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare fixes a leak. An unnamed box has no name on its create request. So a failed grant mint withdrew nothing and left the host row published. mint_into_request now withdraws under the registered name, as the other withdrawal sites do. The new test proves the path. The change conforms to Addressing line 389, and the PASS on 3f582acd stands.
Ruled at 3f3c8e7 against gominimal/arch@8640f85e193d by claude-opus-5-5.
There was a problem hiding this comment.
Reviewed at 3f3c8e7. min invents no box name on any create path. A held name is refused with exit 2 naming the holder, from both the daemon and the VM host registration, and every refusal path withdraws its row. Only min task run asks for a fresh -. The client and the daemon share boxes::box_name::entry_box_name, so the VM host row and the box carry one name, and a user's [box] name or --name is never rewritten. The e2e script matches the new names without vacuous checks.
|
A lab note from the day this landed, nothing to act on in the PR: two min-regress scenarios (690, 690b) went ENV on every run from this commit on, and bisecting the release pinned it here. It was the scenarios' oracle, not the daemon: they read the session's name from the "session created" record and counted hook lines by it, and since this change the record logs |
T58 — Stop min inventing box names and carry the held-name refusal to the user
slice: S3
kind: task
covers: BOX-163
size: M
est: 300 code, 200 tests
executor: agent
difficulty: known ground
model: strongest
diagnostics: min's create log names the requested name or "fresh instance"; the refusal names the holder
observability: create span carries name source (entry, --name, fresh) and the holder on refusal
layers:
- client:
- crates/minimal-client/src/box_name.rs: no invented name
- crates/minimal-client/src/box_registration.rs: register without a minted name
- crates/minimal/src/cmd/boxes/create.rs: name only for --name; fresh-instance field; exit 2 naming the holder
- crates/minimal/src/task.rs: task runs take the fresh-instance path
- crates/minimal/src/dash/rpc.rs: dashboard create sends no minted name
- crates/minimal/src/tests.rs: pins on minted names
test: cargo nextest run -p minimal-client no_invented_directory_hex_name
test: cargo nextest run -p minimal held_name_refusal_exits_2_and_names_the_holder
- wire:
- crates/minimald-rpc/src/lib.rs: fresh-instance field; refusal carries the holder
- crates/minimald/src/rpc.rs: holder refusal passes through; fresh-instance maps to create_fresh_box_with_spec
test: cargo nextest run -p minimald create_session_reply_names_the_holder_of_a_held_name
test: cargo nextest run -p minimald create_session_fresh_instance_takes_the_smallest_unused_suffix
produces: none
consumes: none
proof: min creates a box named from its entry with no invented name; a second run of a held name gets -; a create under a running box's name exits 2 naming the holder.
depends on:
- blocked by #2400
- blocked by #2758
Proof
min session activate with no --name sends no name, and the daemon names the box from its entry (
- name is minted anywhere in min, min dash or min task run. When a running box holds the entry's name, min sends the create again as a fresh instance and the box is named -. Every min task run is a fresh instance, -. A create under a --name that a running box holds ends min with exit 2 and prints: Cannot create a session nameddefaultfor a session). No<name>: the running session<holder>() holds that name. Rename that session withmin session rename <id> <new-name>, or choose another name.Notes from the agent: Files touched outside the layers, and why: (1) crates/minimal/src/main.rs: failure_exit_code maps minimald_rpc::HeldNameRefusal to exit 2. Without it min cannot exit 2, because the exit codes are decided only there. (2) crates/minimal/src/cmd/mod.rs: it re-exported autogen_session_name, random_hex4, sanitize_name_component, should_retry_autogen and AUTOGEN_NAME_RETRIES. Those are removed, so the re-export lines go. (3) crates/minimal/src/cmd/boxes/vm_host.rs: its test autogen_registration_reminted_on_a_held_name called the removed register_box_reminting_autogen. It is now registration_under_a_held_name_is_refused. (4) crates/minimald-rpc/src/tests.rs: two CreateSessionRequest struct literals need the new fresh_instance field to compile.
Design: the holder rides in the reply's error text. Errorable has only a string error, and a typed error variant would change every CreateSession call site in the workspace. minimald_rpc::HeldNameRefusal spells the text (Display) and parses it back (from_error), so one definition owns both. The new minimald tests live in an inline mod create_naming_tests in rpc.rs, not in rpc/tests.rs, to stay inside the layers. After a create, min reads the name the daemon gave the box with GetSessionRecord, for the start lines, the name hold and the cleanup. CreateSessionResponse has no name…[truncated]
Tests
cargo nextest run -p minimal-client no_invented_directory_hex_namecargo nextest run -p minimal held_name_refusal_exits_2_and_names_the_holdercargo nextest run -p minimald create_session_reply_names_the_holder_of_a_held_namecargo nextest run -p minimald create_session_fresh_instance_takes_the_smallest_unused_suffixVerify
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/vm_host.rscrates/minimal/src/cmd/mod.rscrates/minimal/src/main.rscrates/minimald-rpc/src/tests.rsCloses #2762
Undrafted by the maintainer: the edits outside the layers are accepted. They are crates/minimal/src/cmd/boxes/vm_host.rs, crates/minimal/src/cmd/mod.rs, crates/minimal/src/main.rs, crates/minimald-rpc/src/tests.rs, crates/minimald/src/rpc/tests.rs, scripts/session-e2e.sh (the daemon's
<entry>-<n>names) and crates/minimald/src/box_manager.rs (an entry cleaned into a valid name).Round 4 adds crates/boxes/src/lib.rs (the shared
entry_box_name, accepted) and crates/minimald/src/box_manager/tests.rs (the test the maintainer asked for, for the accepted box_manager.rs change).