Repository navigation
T12 — Keep a name unique only among running boxes - #2756
Conversation
A name is now an alias that many retained records share. Only a pending, materializing or running box holds it, so a stopped or exited box keeps its name and a new box can reuse it (BOX-004). The index keys names on the box id, and a name lookup returns the running box, else the latest one (BOX-005, BOX-143). Rename refuses self and a name a running box holds, and records a renamed event (BOX-010). A pure cross-host resolution refuses an ambiguous name with exit 2.
The actor refuses a rename to self before it touches the box's route, logs every refusal with its exit code, and writes one renamed event with the old and new names once the store accepts the rename (BOX-010). The store makes the running-holder check and names the holder.
…e-unique-only-among-running-boxes
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
|
Approved: every review thread resolved at @mitodrummer resolved the last thread, which releases the approval this review had withheld. Nothing to do. |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The change conforms to the Addressing rules in architecture.md line 389. A name is unique among running boxes only, and a stopped or exited box keeps its name. Resolution returns the running box, else the latest creation. A rename refuses self and a running-held name, and it writes one renamed event with both names, as line 382 says. The index keys on the id, not the name. Nothing calls the cross-host resolver yet. The BOX spec gives that rendering to BCLI-002.
Ruled at c2ea913 against gominimal/arch@42d214dc8ac8 by claude-opus-5-5.
The refusal carries exit_code() 2, but rename returns an io::Error, so the code reaches only the daemon log. The RPC and min rename do not carry it yet.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The change since the last PASS is one doc comment on BoxActor::rename. The comment is accurate. The refusal has exit code 2, but the RPC reply and min rename do not carry it yet. The code did not change, so the earlier ruling stands. The CLI exit path belongs to BCLI-002, not to this task.
Ruled at e638ca0 against gominimal/arch@42d214dc8ac8 by claude-opus-5-5.
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
With two boxes named dev retained on a host, an unqualified Notes from the agent: Revise round 1. The rename doc comment in naming.rs now says where the exit code goes: the refusal's exit_code() is 2, but it reaches only the daemon log. To carry exit 2 to the user, a later task must change the rename RPC error and cmd_rename in crates/minimal/src/cmd/session.rs. Those files are outside this task's layers. The exit2 test names come from the spec verify lines, so they stay. They assert RenameRefusal::exit_code() == 2 in the boxes crate (lib.rs and store.rs tests). |
A stopped Dev hid a running dev, because resolution took exact-case matches first. Fold case first, let the running box answer, and use the exact case only to break a tie among retained boxes.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare fixes name resolution so that it folds case first. A running box that holds a name now answers for that name in any case, as architecture.md line 389 requires. The exact case only breaks a tie among boxes that do not run, and the design does not state a case rule. The rest of the PR keeps the PASS on e638ca08.
Ruled at 12cbb73 against gominimal/arch@42d214dc8ac8 by claude-opus-5-5.
|
Round 2 of 10 on a failed check suite, pushed as
Verify Review threads the coder judged:
Two boxes named dev are retained on one host and one of them runs. Notes from the agent: Failed checks: the job log excerpt does not name a failing test. The full boxes suite (622) and the full minimald suite (1241) pass on this branch. No other crate uses the changed store code. The boxes and minimald doctests did not run here, because rustdoc is not executable on this box. So the failing test in the CI |
Loader::rename now logs a failed renamed-event write and returns Ok, as the actor path does. The new name is already saved at that point.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The new commit changes one function: DiskLoader::rename in crates/boxes/src/store.rs. When the event write fails after the new name is saved, the store now logs the failure and keeps the rename. The actor path already does this. The design does not say that a lost lifecycle event must undo a rename. The earlier PASS stands for the rest of the change.
Ruled at 733d780 against gominimal/arch@42d214dc8ac8 by claude-opus-5-5.
|
Round 3 of 10 on a failed check suite, pushed as
Verify Review threads the coder judged:
Two boxes named dev can be retained on one host. A lookup of dev answers the running one, and only a running box holds the name. Notes from the agent: Revise round 3. Loader::rename now logs a failed renamed-event write and returns Ok, the same rule as the actor path. Failed checks: ci-macos-success is the lane aggregator. Its excerpt shows only |
|
/revise Fix the open thread in crates/minimald/src/box_actor/naming.rs ( |
…e-unique-only-among-running-boxes
Only a RenameRefusal logs 'rename refused' at info. A record write or index flush error now logs 'rename failed' at warn.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The change conforms. The new commit logs a failed rename apart from a refused rename. It does not change what rename returns or the renamed event. The merge of main is clean. The earlier PASS on 733d7806 stands for the rest of the change.
Ruled at b598e99 against gominimal/arch@42d214dc8ac8 by claude-opus-5-5.
|
Round 4 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
Two boxes named dev are retained, and the running one answers to the name. In the daemon log, a rename that the name rules refuse shows as 'rename refused' with its exit code. A rename that fails on a write shows as 'rename failed' at warn. Notes from the agent: This revise round changes only the Err arm of rename in naming.rs, as the note asks. No test asserts on the log line, so no test changed. The boxes test lines were not run again this round, because no file in crates/boxes changed. cargo clippy -p minimald --all-targets is clean. |
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 b598e99fb4ff 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 b598e99. Name uniqueness among running boxes, case-folded resolution preferring the running box, and the rename refusal path are correct; the open logging thread is fixed. Disclosed and accepted for follow-up: exit 2 is not yet carried through the RPC and min rename (BCLI), resolve_across_hosts has no caller yet, and find_by_name fails closed on an unreadable record. The per-test port lease in box_actor tests replaces the fixed 18080 that collided under nextest.
T12 — Keep a name unique only among running boxes
slice: S3
kind: task
covers: BOX-004, BOX-005, BOX-010, BOX-143
size: L
est: 360 code, 480 tests
executor: agent
difficulty: needs care
model: strongest
diagnostics: daemon log names holders and candidates
observability: renamed event carries both names
layers:
- names:
- crates/boxes/src/store.rs: one name, many records
- crates/boxes/src/lib.rs: running, latest, candidates
- crates/boxes/Cargo.toml: proptest dev-dependency
- Cargo.lock: proptest for boxes
test: cargo nextest run -p boxes name_unique_among_running_on_host_reusable_after_stop
test: cargo nextest run -p boxes unqualified_name_resolves_running_else_latest
test: cargo nextest run -p boxes unqualified_name_running_on_two_hosts_refused_exit2_lists_candidates
test: cargo nextest run -p boxes unqualified_name_retained_on_two_hosts_none_running_refused_exit2_lists_candidates
test: cargo nextest run -p boxes rename_changes_only_alias_and_records_event
test: cargo nextest run -p boxes rename_to_running_held_name_or_self_refused_exit2_names_holder
test: cargo nextest run -p boxes nothing_keys_on_box_name
- actor:
- crates/minimald/src/box_actor/naming.rs: rename refusals, renamed event
- crates/minimald/src/box_actor.rs: routes rename
- crates/minimald/src/box_actor/tests.rs: tests
test: cargo nextest run -p minimald rename_through_the_actor_writes_one_renamed_event
produces: none
consumes:
- crates/boxes/src/events.rs
proof: Two boxes named dev retained; the running one answers.
depends on:
- blocked by #2394
- blocked by #2389
Proof
In this branch, the store can keep two boxes named dev. When one of them is stopped and the other is running, a lookup of
devreturns the running box. When neither runs, the lookup returns the box created last. A new box can take a name that only a stopped or exited box has. A rename to a name that a running box has, or toself, is refused with exit 2, and the message names the running box orself. A successful rename writes onerenamedevent withold_nameandnew_name. The tests unqualified_name_resolves_running_else_latest and rename_through_the_actor_writes_one_renamed_event show this.Notes from the agent: Design 8000 : the store index no longer has the name-keyed
name_to_idmap. It now hasnames, keyed on box id. An old index.json is still read: the first start ignoresname_to_idand rebuildsnamesfrom the records (a test covers this). A daemon from before this change cannot read the new index.json, because it requiresname_to_id. A box holds its name while it is pending, materializing or running. A box that is still starting counts, so two concurrent creates of one name cannot both reach running. The 'latest' box is the one with the greatest id. That is creation order for UUIDv7 ids, but not for a legacy record with a v4 id. Cross-host resolution (box_name::resolve_across_hosts) is a pure function. Nothing calls it yet, because no caller in this tree reads more than one host. BCLI work must connect it.Layers with no change: crates/boxes/Cargo.toml and Cargo.lock already have proptest as a dev-dependency of boxes, so neither changed. crates/minimald/src/box_actor.rs already routes BoxMessage::Rename to the actor's rename, so it did not change. Behaviour change in the store's DiskLoader::rename: it now appends the renamed event. Only boxes tests call it. The actor writes its event through box_manager::write_box_event.
Printed text (new; the CLI does not show it yet except through the rename RPC error): the rename refusal says "…[truncated]
Tests
cargo nextest run -p boxes name_unique_among_running_on_host_reusable_after_stopcargo nextest run -p boxes unqualified_name_resolves_running_else_latestcargo nextest run -p boxes unqualified_name_running_on_two_hosts_refused_exit2_lists_candidatescargo nextest run -p boxes unqualified_name_retained_on_two_hosts_none_running_refused_exit2_lists_candidatescargo nextest run -p boxes rename_changes_only_alias_and_records_eventcargo nextest run -p boxes rename_to_running_held_name_or_self_refused_exit2_names_holdercargo nextest run -p boxes nothing_keys_on_box_namecargo nextest run -p minimald rename_through_the_actor_writes_one_renamed_eventVerify
min task run agent-verifypassed ([tasks.verify]).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/minimald/src/box_actor/tests.rsCloses #2396