Repository navigation
feat(minimald): refuse an activation whose declared port something else holds (T100) - #2725
Conversation
…T100) Finalize refuses a declared ingress host port that a program outside Minimal holds, or that another box holds at the 127.0.0.1 fallback once a landed range verdict keeps the box there. The refusal is a typed PortInUseRefusal that names the port and its holder, and the daemon logs one warn line with the session, port, address and holder.
min session activate ends with the daemon's refusal text alone when a declared host port is already in use: the port, its holder and the --ingress remedy, in place of the RPC context around it.
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
|
Reviewed: 1 finding, none blocking at Address the comments on the diff. Clobber: none seen. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 3 conditions · the full ruling
The fallback refusal conforms to §7.1. The first box keeps the port, and the later box gets a surfaced error (lines 249 and 258). The holder text is wrong. Every holder that is not a box in this daemon's registry prints as "a program outside Minimal". Line 258 says the daemon's own proxy and answerer hold ports on that address, and lines 256 and 258 add other nodes' boxes. C1 fixes the text. C2 and C3 file the two cases that the PR notes leave undone, the VM-backed host and the Pending verdict, as residuals with issues.
Ruled at e62e585 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 1 of 10 on a failed check suite: the coder changed nothing. Revise round 1: I changed nothing. The directive lists no review threads. Only two checks failed: cargo-deny and its aggregator ci-success. cargo-deny fails on a new RustSec advisory, 'Exponential event growth with ENABLE_WIKILINKS', against pulldown-cmark 0.13.4 in Cargo.lock. This branch does not change Cargo.lock or deny.toml, so the same advisory fails main too. The fix is a pulldown-cmark bump in Cargo.lock or an ignore entry in deny.toml. Both files are outside this task's layers, so this needs a separate pull request on main. After that lands, a merge of main will turn this pull request green. The log also shows 'override toolchain ... is not installed'. This branch does not change rust-toolchain.toml, and the advisories step still ran and reported its result after that line. |
|
/revise Fix arch-review condition C1 at e62e585 (arch_rulings 516). Change nothing else.
Generated by Claude Code |
No box in the daemon's hostname registry holding the port does not prove the holder is outside Minimal: the daemon's own proxy and a co-resident node's box also arrive as the host holder. Arch-review condition C1.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare meets the baseline's condition C1. When no box in this daemon's registry holds the port, the refusal now says "another program on this machine". Section 7.1 lines 256 and 258 say that the daemon's own listeners and the boxes of a co-resident node also occupy the shared address. The new text is true for each of these causes. The compare touches nothing else, so the earlier ruling stands for the rest of the change. NET-144 conforms.
Ruled at 8ed2d67 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Revise round 1 ran against |
There was a problem hiding this comment.
APPROVE withheld: 3 unresolved threads — minimal-arch[bot] crates/minimald-rpc/src/lib.rs, minimal-arch[bot] crates/minimald/src/box_actor/setup.rs:579, minimal-arch[bot] crates/minimald/src/box_actor/setup.rs:596.
minimal-review — advisory review
Verdict COMMENT — No findings.
Panel: 2 of 3 personas ran (security, supply-chain); did not complete: correctness.
|
Round 2 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
Activating a box whose --ingress port another box or a host program already holds fails with the port and the holder named, and min ls shows no box. When no box holds the port, the refusal now reads "Port is already in use on this machine by another program on this machine." Notes from the agent: Round 2 addresses only C1, as the note asks. PORT_IN_USE_HOST is now "another program on this machine.", the PortHolder::Host doc comment says what the code knows, and the pinned text in session.rs and the policy.rs apply_ingress test is updated. The log label stays |
|
/revise Two things, in order. Change nothing else.
Generated by Claude Code |
…-refuses-a-declared-port-that-somethin
On a VM-backed host the daemon cannot probe the host's ports, so a box went Active with a held declared port and failed only at its first attach. Finalize now binds the declared forwards toward the handed switch address before the Active write. A held port fails with the same refusal and holder text as the native probe. The first attach takes the host's "already bound" answer as the box's own forward. A box that finalizes at the 127.0.0.1 fallback under a pending range verdict still yields a port another box holds there. A watcher now waits for the verdict to land, and an abs 10000 ent landing reports the held port, the address and the holder in the daemon log.
…-refuses-a-declared-port-that-somethin
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The change still conforms to §7.1. The new commit binds the declared forwards at publish on a VM-backed host. A held port now fails the finalize before the box goes Active, as line 249 requires. The "already running" answer is read as the box's own forward only behind the host gate, and yielded ports are skipped first. The absent-landing watcher adds a log line and changes no decision.
Ruled at 3e62ed6 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 3 of 10 on a /revise note, pushed as
Verify Activating a box whose --ingress port another box or a host program already holds fails with the port and the holder named, and min ls shows no box. This now also holds on a VM-backed host, because finalize binds the declared forwards before the box goes Active. A box that finalized at 127.0.0.1 while the range verdict was pending gets a daemon log report with the port, the address and the holding box if the verdict then lands absent. Notes from the agent: Item 1 (merge origin/main): already done on this branch by bac3f1d, which includes #2726. No conflicts, no new merge commit. C2: on a VM-backed host, finalize now calls policy::bind_declared_at_publish before the Active write. It posts the real exposes toward the switch_address the host handed in record.box_addresses. A held port fails with the same PortInUseRefusal text and holder as the native probe. The forwards stay on the host, which unbinds them when it withdraws the box row. On a VM-backed host the first attach now takes the switch's 'proxy already running' answer as the box's own forward. minvmd's gate sends that answer (forward_revoke::held_answer) for a listener that the same address already holds. A VM-backed box with no box_addresses has no switch address, so its first attach still binds the forwards, as before. The finalize path through a vsock control channel has no test, because TestServer only spawns a local switch. The tests drive the VM branch through apply_declared with host_holds_listeners=true over a fake unix forwarder. C3: I chose report, not refuse, which the directive allows. The box is already Active when the verdict lands, and a refusal there would destroy a running box. Under a pending verdict, finalize starts a detached watcher (LoopbackLeaseBook::await_landing). On an absent landing, the watcher re-reads fallback_port_refusal. If the box still holds a refused port, it writes one warn line with action=host-port-in-use, session, port, address=127.0.0.1 and holder=box:<na…[truncated] |
There was a problem hiding this comment.
Babysitter review at 3e62ed64. I read the daemon and client changes against T100's plan block and arch C1–C3 (arch_rulings 516). Finalize now binds the declared ingress forwards before the record goes Active, and a held port fails activation with a typed refusal naming the port and its holder; the client prints it and exits non-zero. A port another box holds at the 127.0.0.1 fallback is a refusal, not a yielded port. C1: PortHolder::Host reads "another program on this machine." (minimald-rpc PORT_IN_USE_HOST), with the pinned texts updated. C2: on a VM-backed host bind_declared_at_publish binds the forwards at publish, so a held port refuses finalize with the same holder text. C3: a box finalized at the fallback under a pending range verdict is watched (watch_fallback_landing); when the verdict lands absent the conflict is reported instead of passing silently. Main is merged in. No findings.
Generated by Claude Code
There was a problem hiding this comment.
Babysitter approval at 3e62ed6: every merge gate passes at this head (arch PASS at 3e62ed6, zero open threads, every lane aggregator green). Squash title feat(minimald): refuse an activation whose declared port something else holds (T100) and the squashed message pass commitlint. Queued for squash merge.
Generated by Claude Code
…e holds (#2725) T100. Finalize refuses a declared ingress host port that a program outside Minimal holds, or that another box holds at the 127.0.0.1 fallback once a landed range verdict keeps the box there. The typed PortInUseRefusal names the port and its holder, the daemon logs one warn line, and min session activate ends with that refusal text alone. An unknown holder is named as another program. On a VM-backed host, finalize binds the declared forwards toward the handed switch address before the Active write, and the first attach takes the host's "already bound" answer as the box's own forward. A box that finalizes at the fallback under a pending verdict gets a watcher that reports an absent landing. Backport of #2725, folded from #2738. Adapted to the line: - The finalize code lives in session.rs, not box_actor/setup.rs, and registry_name is the session module's own helper. In the CLI, activate_refusal applies to activate_session. - boxes:: becomes sessions:: and switch::BoxGate becomes switch::SessionGate. - The fallback watcher spawns with tokio::spawn under the same root span. - OwnAddressReporter::registry comes without NET-129's dynamic-hold tracking. The attach path's holder lookup needs only the accessor. - The line's DestroySessionRequest has no only_if_unfinalized, so the refusal tests assert the refused box is listed and not Active, destroy it plainly, and assert no record stays. (cherry picked from commit 703bb7f)
…e holds (#2725) T100. Finalize refuses a declared ingress host port that a program outside Minimal holds, or that another box holds at the 127.0.0.1 fallback once a landed range verdict keeps the box there. The typed PortInUseRefusal names the port and its holder, the daemon logs one warn line, and min session activate ends with that refusal text alone. An unknown holder is named as another program. On a VM-backed host, finalize binds the declared forwards toward the handed switch address before the Active write, and the first attach takes the host's "already bound" answer as the box's own forward. A box that finalizes at the fallback under a pending verdict gets a watcher that reports an absent landing. Backport of #2725, folded from #2738. Adapted to the line: - The finalize code lives in session.rs, not box_actor/setup.rs, and registry_name is the session module's own helper. In the CLI, activate_refusal applies to activate_session. - boxes:: becomes sessions:: and switch::BoxGate becomes switch::SessionGate. - The fallback watcher spawns with tokio::spawn under the same root span. - OwnAddressReporter::registry comes without NET-129's dynamic-hold tracking. The attach path's holder lookup needs only the accessor. - The line's DestroySessionRequest has no only_if_unfinalized, so the refusal tests assert the refused box is listed and not Active, destroy it plainly, and assert no record stays. (cherry picked from commit 703bb7f)
T100 — Activation refuses a declared port that somethin 8000 g else holds
slice: S0
kind: task
covers: NET-144
size: M
est: 300 code, 180 tests
executor: agent
difficulty: needs care
model: standard
diagnostics: the refusal itself is what a person reads:
min session activateexits non-zero and prints "Port is already in use on this machine by box ." or, when no box holds it, "Port is already in use on this machine by a program outside Minimal.", then "Choose another host port in --ingress (for example --ingress <port+1>:) or in [network] ingress in minimal.toml.", pinned by activation_refused_when_declared_port_is_held and activate_refusal_names_port_and_holderobservability: one warn line in the daemon log per refused activation with the session id, the port, the address and the holder (
box:<name>orhost)layers:
- daemon:
- crates/minimald/src/box_actor/setup.rs: finalize binds the declared ingress forwards before it writes the record
Active, and a bind that fails fails the finalize, so the client's activate cleanup destroys the box; today the forwards bind only at the first attach and the box activates with the port dead- crates/minimald/src/net/gvproxy_network.rs:
finish_own_ip_attachreturns the failed port and its holder instead of a bare io error- crates/minimald/src/net/policy.rs:
apply_ingressnames the holder: the box whose publish stands at that address and port, read from the hostname registry, or the host when none does- crates/minimald/src/net/dns.rs: a declared port that another box already holds at the shared
127.0.0.1fallback is a refusal at activation, not a yielded port; a host-address box sharing its node's address keeps the shared-address rule- crates/minimald/src/box_actor/tests.rs: a box whose declared port is held by a host listener, and one whose port another box holds at the fallback, both fail activation and leave no record
test: cargo nextest run -p minimald activation_refused_when_declared_port_is_held
- client:
- crates/minimald-rpc/src/lib.rs: a typed activation refusal carrying the port and the holder
- crates/minimal/src/cmd/session.rs: prints the refusal text in the diagnostics line and exits non-zero
test: cargo nextest run -p minimal activate_refusal_names_port_and_holder
produces:
- crates/minimald-rpc/src/lib.rs
consumes: none
depends on:
- blocked by #2688
proof: Activating a box whose
--ingressport another box or a host program already holds fails with the port and the holder named, andmin lsshows no box.Proof
A box activated with an --ingress host port that a host program holds, or that another box holds at the shared 127.0.0.1 fallback, fails min session activate with a non-zero exit. It prints "Port is already in use on this machine by box ." or "... by a program outside Minimal.", then the --ingress remedy line. The daemon refuses before the record goes Active, and the activate cleanup destroys the box, so min ls shows no box. The daemon tests prove the destroy-and-list part over the RPC, not through the CLI end to end.
Notes from the agent: 1. NET-144 is not in .implement/spec.md. I worked from the task text only.
2. Finalize does not bind the real forwards before Active. The forwards need the box's switch lease, and that exists only after a launch. Finalize keeps the existing bind probe for host programs and adds the box-holder check at the fallback. Both fail the finalize before the Active write. With activate hooks, the hook launch binds the forwards, and a taken port fails with the same typed refusal.
3. Fallback rule: another box that holds the port at 127.0.0.1 refuses the activation only when the range verdict has landed (Absent or Present). Under a Pending verdict the interim is a short stop before the box's own hand, so the port is yielded as before. Without this, handed_finalizes_share_one_verdict_deadline fails, and a normal start under a pending verdict breaks. The new LoopbackLeaseBook::range_verdict in dns.rs gives finalize the verdict.
4. apply_ingress keeps its 6-argument signature because crates/minimald/tests/netns_root_integration.rs (outside the layers) calls it. The holder-naming version is the new apply_ingress_naming_holders, which the attach path uses. apply_ingress names the host as the holder.
5. The refusal travels as the daemon's error text. PortInUseRefusal formats the text and parses it back with find_in, so the two cannot drift. The client replaces the whole error chain with that text, only in cmd_activate (ses…[truncated]
Tests
cargo nextest run -p minimald activation_refused_when_declared_port_is_heldcargo nextest run -p minimal activate_refusal_names_port_and_holderVerify
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 #2703