Repository navigation
test(docs): extend networking test plan to all five deployment models - #625
Closed
norrietaylor wants to merge 21 commits into
Closed
norrietaylor wants to merge 21 commits into
norrietaylor wants to merge 21 commits into
Conversation
Restructure the #478 CLI test plan from DM1-centric to a DM1-DM5 coverage plan, and replace the expect PTY driver with a heredoc-piped stdin driver. test-plan.md: - status-marker legend (RUNNABLE / BLOCKED(impl-gap) / SPEC-GAP / N/A) and a DM x TC coverage matrix (TC1-TC17 x DM1-DM5), every cell marked - per-DM bring-up/teardown/topology sections (just dm1|dm2|dm3; DM4 composed as dm3+dm2; DM5 spec-gap) and a CI-workflow mapping - new TCs: TC10 topology preflight, TC11 DM3 parity, TC12 vm_egress, TC13 per-PTask egress, TC14 dynamic port-map, TC15 DM4 co-residency, TC16 DM5 remote access, TC17 mesh data path - gap register G-N1..G-N7 with file:line citations for every blocked cell - TC7 corrected to the real :7655 routing rule (Host: header authority -> published port; SNI stays localhost) - per-DM results table; DM1 column filled from 2026-07-02 runs test-plan.sh: - DM selector (DM=dm1|dm2|dm3|dm4, auto-detected) with clean cross-OS SKIP exits; --minimal-dir wiring for DM2's dedicated state dir - heredoc driver replaces expect: session_out (one-shot, ends with exit) and session_hold (held-open stdin keeps the shell + own-ip lease + in-session server alive while the host probes); exact-line nonce markers and anchored sentinels defeat the pre-stty echo replay - TC10 preflight, gap-cited SKIP banners for TC12-14/16/17, TC15 for DM4 DM1 validation (macOS/HVF): TC1/1b/2/5/6/9/10 PASS incl. UC6 peer routing; TC3/TC4/TC7-with-cert/TC8 all fail on the same host->PTask published-loopback-port -> lease leg with a confirmed-live backend, under both drivers - recorded as a driver-independent finding. Refs: #478 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThe PR rewrites the networking test plan and shell harness around full-session ChangesNetworking Test Plan Documentation and Harness
Minimald own-IP routing and proxy plumbing
Sequence Diagram(s)sequenceDiagram
participant Session as Session
participant HostnameRegistry as HostnameRegistry
participant GvproxyNetwork as GvproxyNetwork
participant Router as Router
participant ConnectionHandler as ConnectionHandler
Session->>HostnameRegistry: publish session lease and port map
GvproxyNetwork->>HostnameRegistry: clear lease on teardown
Router->>HostnameRegistry: resolve_route(host)
ConnectionHandler->>HostnameRegistry: look up active lease target
ConnectionHandler->>Router: use published port mapping for direct-tcpip
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/specs/03-spec-networking/test-plan.md`:
- Around line 330-345: The `#553` reference in the TC12 description is being
parsed as a Markdown heading because it starts a line with `#` and no space.
Update the wording in this test plan entry so the issue reference is not
line-initial, either by rephrasing it within the sentence or escaping the `#`
character. Keep the rest of the `VmConfig`/`validate_for`/`vmm_child` context
unchanged.
In `@docs/specs/03-spec-networking/test-plan.sh`:
- Around line 319-331: The ssh-forward test is writing its held-session log to a
fixed /tmp path instead of the per-run scratch directory. Update the log target
used in the session_hold call for the TC8 ssh-forward scenario to use $SCRATCH
consistently with the other session logs in this script, so the logging path is
unique per run and not a shared /tmp file.
- Around line 101-124: The trap-driven cleanup in cleanup() misses tc15b when
running under DM=dm4 because mcli() still targets only the primary daemon state,
so destroy $ALL_SESSIONS cannot reach the native daemon. Update the dm4 path to
ensure tc15b is destroyed through the same $DM2_DIR-aware mechanism used when it
is created (for example by routing cleanup through the raw "$M" --minimal-dir
"$DM2_DIR" destroy path or adding a dedicated tc15b teardown helper invoked from
cleanup()). Keep the existing cleanup() and destroy() flow, but make tc15b
reachable from the EXIT/INT/TERM trap regardless of where the script stops.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f4194d3f-9a7d-407d-9d49-158b54981ff9
📒 Files selected for processing (2)
docs/specs/03-spec-networking/test-plan.mddocs/specs/03-spec-networking/test-plan.sh
Fill the previously-empty DM2/DM3/DM4 result columns with real 2026-07-03 end-to-end runs on a native Linux/aarch64 host (DM=dm2, DM=dm3, plus TC15 for DM4). AppArmor's unprivileged-userns restriction was disabled and the binaries rebuilt first (the prior build predated the #589 DM2 own-ip feature under test). Key results: - DM2 (native, no VM): TC1/1b/2/5/6/9/10 PASS; TC3/TC4/TC8 FAIL and TC7 with-cert FAIL, all with the in-session backend confirmed live. - DM3 (KVM VM): same PASS set; TC3/TC7 return 502 matching DM1. - DM4: TC15 PASS; G-N1 proxy-port contention confirmed observably (second daemon loses the bind with EADDRINUSE, one listener remains). Findings added to the gap register: - G-N9: the host->PTask ingress forward reaches the backend in the daemon netns, not the session's sandbox netns. Code-confirmed for TC8 (direct-tcpip validates the session UUID then connects to 127.0.0.1:<int> in the daemon netns). Reproduced with a live backend and no VM on DM2, ruling the VM out. - G-N8: on the KVM VM, attach of an own-ip session carrying --ingress hangs (shell never starts, >200s vs ~8s without ingress), so the backend never comes up. DM3's 502 is thus a different mechanism than DM1/DM2 -- TC11 parity holds on the verdict, not the mechanism. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Wjns5t5SQFRvNWmYqXdQ5
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/specs/03-spec-networking/test-plan.md (1)
229-230: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRewrite the
#542reference.This is the same markdownlint issue as the earlier
#553note: a line-initial#542will be read as an ATX heading. Rephrase it inline or escape the hash.✏️ Proposed fix
-#542 landed the hostname → `127.0.0.1` registration, +issue `#542` landed the hostname → `127.0.0.1` registration,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/specs/03-spec-networking/test-plan.md` around lines 229 - 230, The markdown in the test plan contains a line-initial issue in the note mentioning `#542`, which markdownlint will parse as an ATX heading. Update the wording in the affected paragraph to avoid starting the line with the hash, either by rephrasing the reference inline or escaping the hash, and keep the surrounding networking/proxy description intact.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@docs/specs/03-spec-networking/test-plan.md`:
- Around line 229-230: The markdown in the test plan contains a line-initial
issue in the note mentioning `#542`, which markdownlint will parse as an ATX
heading. Update the wording in the affected paragraph to avoid starting the line
with the hash, either by rephrasing the reference inline or escaping the hash,
and keep the surrounding networking/proxy description intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fe8f865f-242d-4a55-af41-e0318c998bf4
📒 Files selected for processing (1)
docs/specs/03-spec-networking/test-plan.md
norrietaylor
marked this pull request as draft
July 6, 2026 16:37
- test-plan.sh: sweep tc15b on the DM4 native daemon from cleanup(), so an abnormal exit between activate and the explicit destroy can't leak it (the trap-driven destroy only reaches the primary/MARGS daemon). - test-plan.sh: write the TC8 ssh-forward log under $SCRATCH instead of a predictable /tmp path, matching every other held-session log. - test-plan.md: reflow the #542 and #553 issue references off line start so markdownlint (MD018) no longer reads them as malformed ATX headings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`just up`/`just dm1` failed on a clean macOS checkout at the `artifacts`
step: the recipe passed `arch()` (aarch64) to `minimal materialize`, which
rejects it ("unsupported architecture: aarch64; use amd64 or arm64"), and
the macOS `minimal` shim silently builds the wrong arch when given an
absolute `--output`, dropping the kernel Image.
Add a `guest-arch` variable mapping the host arch to OCI naming
(arm64/amd64) for every materialize consumer, keep the Rust-named `arch`
for the musl target, and pass a repo-relative `--output` on the macOS
shim path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ress `channel_open_direct_tcpip` validated the session UUID but then dialed the raw client-supplied host:port in the daemon's own netns, so a forward to an own-ip PTask's backend never reached it (and any authenticated client could make the daemon connect anywhere in its namespace). Resolve the requested PTask-side (internal) port against the session's static ingress mapping and dial the published port, rejecting a target with no published TCP mapping. Extract `published_external_port` with unit tests. Interim step: this dials the published host-loopback port, correct on the shared-netns (DM2) path but not the in-VM path, where consumers must dial the lease over the switch. See docs/specs/03-spec-networking/gn9-forwarding-findings.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sion The in-session server built its HTTP reply inside socat's `SYSTEM:` address, which mangled the printf and served a non-HTTP response; strict curl scored the delivered-but-malformed reply as 000/502, masquerading as a forwarding failure. Write the response to a file (interpreted by the sandbox shell) and serve it verbatim with `cat`. TC8 also used the ingress external port (18080) as the ssh -L local port, colliding with gvproxy's own :18080 listener so the tunnel failed to bind and the curl silently hit the forwarder; use a distinct local port. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
8BD0
A live re-run with a valid backend shows the host->PTask forward works end-to-end (TC4 = 200); the recorded TC3/TC4/TC7/TC8 failures were largely a broken test backend. The real remaining defect is narrow: in-VM proxies and direct-tcpip dial in-VM loopback while gvproxy's forwarder listens on the host loopback, though the daemon can reach the lease over the switch. Add gn9-forwarding-findings.md (evidence + lease-on-sandbox fix design), correct the DM1 verdict table (TC4 passes), rewrite the cross-DM finding, flag the DM2/DM3/DM4 columns as suspect (same backend), and re-characterize G-N9 in the gap register. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Completes G-N9. The in-VM :7654/:7655 proxies and the SSH direct-tcpip
forward dialed 127.0.0.1:<external>, but gvproxy's forwarder listener is
on the host loopback and is unreachable from inside the VM, so those
consumers got 502 / connect-fail. Route them to the PTask's switch
lease instead: the daemon is on the switch and can reach
lease:<internal>.
The lease is ephemeral runtime state of the live sandbox. Its
authoritative owner is OwnIpGuard; it is published into the shared
HostnameRegistry at own-ip attach and reverted at teardown, so it exists
in the routing table only while the sandbox runs and is never persisted
to the on-disk Record. Publishing is gated on the Vsock (VM) transport;
DM2 (Unix, shared-netns forwarder) keeps the loopback placeholder, so
its verified behaviour is unchanged.
- net/dns.rs: registry entry becomes Route{target,port_map}; add
set_own_ip_lease / clear_own_ip_lease / route_for_session (by stable
SessionId, rename-safe) and resolve_route.
- net/proxy.rs: Router::route remaps the authority's published external
port to the PTask-internal port via Route::dial_port.
- net/gvproxy_network.rs: OwnIpGuard owns the lease; finish_own_ip_attach
publishes it on the Vsock path; teardown reverts before switch detach.
- connection.rs: direct-tcpip dials lease:<internal> when a live lease is
published, else falls back to 127.0.0.1:<external>.
- Thread the shared registry + session_id through Session ->
SandboxLauncher -> the attach tail; make the manager's hostnames
registry unconditional so it can be threaded on every platform.
Verified: cargo test -p minimald green (incl. new lease-lifecycle and
lease-routing unit tests); clean DM2 regression (TC3/4/7/8 =
200/200/200+401/200). The in-VM lease branch is unit-tested only — DM3/DM4
runtime verification is blocked by the separate G-N8 attach-hang.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mark G-N9 as implemented in gn9-forwarding-findings.md (fix, transport- aware DM2/VM branch, and verification) and in the test-plan gap register. Refresh the per-DM verdict table from the 2026-07-07 re-runs: DM2 TC3/4/7/8 now PASS (200/200/200+401/200) with the fixed backend; DM3 TC3/4/7/8 still fail via the distinct G-N8 attach-hang, so the in-VM lease path stays unit-tested pending a G-N8 fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The DM1 re-run (2026-07-08, HEAD 9443bec with the G-N9 fix built in) surfaced two issues and qualified a third: - G-N10: in-sandbox `curl` vanished after #650, which made the session rootfs compose only declared packages; `curl` was only incidental to the old base rootfs. TC1b/TC2 now provision it per-session with `min add curl` (egress 200, peer YES), restoring the egress probes. - G-N11 (new): the G-N9 `:7654`/`:7655` proxy lease-routing returns 502 reproducibly once a prior own-ip session has driven real egress; from a churn-free VM the same TCs are 200. TC4 (direct ingress) is unaffected, so the fault is the proxy lease resolution under prior switch usage. - G-N12 (new): TC7 with-cert fails at the host `curl`/LibreSSL, which rejects the valid P-256 client cert from `minimal login`; the proxy mTLS gate (no-cert 401) works. So G-N9's proxy fix is confirmed on DM1 only in isolation. test-plan.sh wires `min add curl` into TC1b/TC2; test-plan.md records the 4-pass verdicts and adds G-N11/G-N12 to the gap register. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The forwarder control verbs (POST /services/forwarder/expose and the
:7654/:7655 proxy-publish) sent HTTP/1.0 with `Connection: close` and
read the reply to EOF. Over the KVM libkrun `add_vsock_port2(listen =
false)` shuttle, gvproxy's response is dropped when it closes the socket,
so the guest reads 0 bytes ("malformed gvproxy status line") even though
the forward was applied. apply_ingress then rolls back a *successful*
own-ip ingress attach, so the session's shell never starts (gap G-N8);
own-ip without ingress is unaffected.
Send HTTP/1.1 keep-alive (no `Connection: close`) and frame the reply by
Content-Length instead of read-to-EOF, so the server never closes first
and the response drains to the guest, which closes from its own side.
This also repairs the in-VM :7654/:7655 proxy-publish (same leg). The
native unix path (DM2) is unaffected.
Verified live on DM3 (KVM): own-ip + --ingress attach starts the shell
and publishes the lease; TC3/TC4/TC7/TC8 = 200/200/(401+200)/200, with
no startup proxy-publish warnings. Adds a regression test that hangs if
the framing reverts to read-to-EOF.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
G-N8 was mis-recorded as an own-ip + --ingress attach hang. Pin it as a fast attach failure: gvproxy's forwarder HTTP response is dropped over the KVM vsock shuttle (the forward is applied, the reply lost), and the ">200 s" was only the suite's hold_wait ceiling. Record the keep-alive / Content-Length fix, mark G-N8 fixed in the gap register, and update the per-DM table and prose with the 2026-07-08 DM3 (KVM) run: TC3/TC4/TC7/TC8 = 200/200/(401+200)/200, which also runtime-confirms the G-N9 lease path on the VM (TC8 exercises direct-tcpip -> lease:8080). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Re-ran DM1 on HEAD a686f70 (with the be71172 Content-Length forwarder fix built into the initramfs), 2 passes, both with the `min add curl` own-ip churn that had forced 502 on 9443bec: - G-N11 RESOLVED. TC3 :7654 = 200 after own-ip egress (2/2), not 502. The proxy-publish rides the same forwarder control leg as the G-N8 ingress attach, so be71172 (HTTP/1.1 + Content-Length) repairs both; there was no distinct lease-routing defect. G-N9's in-VM proxy path is now DM1-runtime-verified unconditionally. - G-N12 not reproduced: TC7 with-cert = 200 (2/2). The earlier LibreSSL rejection was host/PKI-instance dependent; downgraded to a watch item. - TC8 still flaky on DM1 (000 then 200) — residual G-N9 direct-tcpip; be71172 verified TC8=200 on DM3. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
|
@CodeRabbit review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/minimald/src/net/policy.rs (1)
206-209: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale comment: says HTTP/1.0 after the keep-alive switch.
This comment now contradicts the request just built above (HTTP/1.1, no
Connection: close) and the whole G-N8 change; it reads as "both speak … HTTP/1.0", which is misleading in the exact path being reworked.✏️ Suggested wording
- // socket is local (DM2) or the host gvproxy over vsock (DM1/3/4); both speak - // the same HTTP/1.0 request/response on a fresh connection. + // socket is local (DM2) or the host gvproxy over vsock (DM1/3/4); both speak + // the same HTTP/1.1 keep-alive request/response on a fresh connection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/minimald/src/net/policy.rs` around lines 206 - 209, Update the stale comment in the request-building path in policy logic so it matches the new keep-alive behavior. The note around the bounded exchange should no longer say the control socket and gvproxy “speak HTTP/1.0”; revise the wording near the relevant request/setup code in the policy exchange flow to describe the current HTTP/1.1 behavior without implying Connection: close.
🧹 Nitpick comments (1)
crates/minimald/src/connection.rs (1)
654-655: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueSilent fallback on a poisoned registry lock hides a real misroute on the VM path.
registry.read().ok()?maps a poisonedRwLocktoNone, so the caller silently falls back to127.0.0.1:<external>. On DM1/3/4 that host-loopback target is unreachable from inside the VM, so every forward would fail with a 10s timeout and no log explaining why. Consider logging (warn) when the lock read fails so the poisoning is observable rather than masquerading as an upstream-connect failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/minimald/src/connection.rs` around lines 654 - 655, The registry lookup in `route_for_session` is silently swallowing a poisoned `RwLock` by converting `registry.read()` failures into `None`, which can hide a bad VM routing state. Update this path to explicitly handle the lock-read failure on `mngr.hostnames()`/`route_for_session(session_ 10BC8 id)` and emit a warning log when the read is poisoned, while preserving the existing fallback behavior only after the failure is recorded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/minimald/src/connection.rs`:
- Around line 589-611: The direct-tcpip handling in channel_open_direct_tcpip is
blocking the SSH session while waiting on TcpStream::connect and timeout before
reply.accept(). Move the upstream connect logic out of the handler path by
spawning or otherwise offloading it, accept the channel immediately, and then
complete the connection asynchronously so other requests on the same russh
session remain responsive. Keep the existing error handling and logging around
the upstream_addr connect attempt, but ensure it happens in the background task
rather than before accepting the channel.
In `@crates/minimald/src/sessions.rs`:
- Around line 14-16: The RwLock import is incorrectly gated to Linux even though
Manager, ManagerHandle, and hostnames() use it on every target. Remove the
#[cfg(target_os = "linux")] gate from the std::sync::RwLock import in
sessions.rs so the symbol is always in scope, and verify the affected
types/functions compile cleanly on non-Linux builds.
In `@docs/specs/03-spec-networking/test-plan.md`:
- Line 185: The blockquote in the test-plan note contains an empty quoted line
that triggers MD028; update the quoted section so the note stays continuous, or
close the blockquote before any blank line. Locate the affected Markdown block
by its blockquote and the adjacent details/summary section, and remove the extra
blank quoted line while keeping the surrounding note text intact.
---
Outside diff comments:
In `@crates/minimald/src/net/policy.rs`:
- Around line 206-209: Update the stale comment in the request-building path in
policy logic so it matches the new keep-alive behavior. The note around the
bounded exchange should no longer say the control socket and gvproxy “speak
HTTP/1.0”; revise the wording near the relevant request/setup code in the policy
exchange flow to describe the current HTTP/1.1 behavior without implying
Connection: close.
---
Nitpick comments:
In `@crates/minimald/src/connection.rs`:
- Around line 654-655: The registry lookup in `route_for_session` is silently
swallowing a poisoned `RwLock` by converting `registry.read()` failures into
`None`, which can hide a bad VM routing state. Update this path to explicitly
handle the lock-read failure on
`mngr.hostnames()`/`route_for_session(session_id)` and emit a warning log when
the read is poisoned, while preserving the existing fallback behavior only after
the failure is recorded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 238c1aa6-477e-48e4-ad2c-d899aca794b5
📒 Files selected for processing (12)
crates/minimald/src/connection.rscrates/minimald/src/net/dns.rscrates/minimald/src/net/gvproxy_network.rscrates/minimald/src/net/policy.rscrates/minimald/src/net/proxy.rscrates/minimald/src/session.rscrates/minimald/src/session_host.rscrates/minimald/src/sessions.rsdocs/specs/03-spec-networking/gn9-forwarding-findings.mddocs/specs/03-spec-networking/test-plan.mddocs/specs/03-spec-networking/test-plan.shjustfile
✅ Files skipped from review due to trivial changes (1)
- docs/specs/03-spec-networking/gn9-forwarding-findings.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/specs/03-spec-networking/test-plan.sh
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/minimald/src/net/policy.rs (1)
206-209: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale comment: says HTTP/1.0 after the keep-alive switch.
This comment now contradicts the request just built above (HTTP/1.1, no
Connection: close) and the whole G-N8 change; it reads as "both speak … HTTP/1.0", which is misleading in the exact path being reworked.✏️ Suggested wording
- // socket is local (DM2) or the host gvproxy over vsock (DM1/3/4); both speak - // the same HTTP/1.0 request/response on a fresh connection. + // socket is local (DM2) or the host gvproxy over vsock (DM1/3/4); both speak + // the same HTTP/1.1 keep-alive request/response on a fresh connection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/minimald/src/net/policy.rs` around lines 206 - 209, Update the stale comment in the request-building path in policy logic so it matches the new keep-alive behavior. The note around the bounded exchange should no longer say the control socket and gvproxy “speak HTTP/1.0”; revise the wording near the relevant request/setup code in the policy exchange flow to describe the current HTTP/1.1 behavior without implying Connection: close.
🧹 Nitpick comments (1)
crates/minimald/src/connection.rs (1)
654-655: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueSilent fallback on a poisoned registry lock hides a real misroute on the VM path.
registry.read().ok()?maps a poisonedRwLocktoNone, so the caller silently falls back to127.0.0.1:<external>. On DM1/3/4 that host-loopback target is unreachable from inside the VM, so every forward would fail with a 10s timeout and no log explaining why. Consider logging (warn) when the lock read fails so the poisoning is observable rather than masquerading as an upstream-connect failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/minimald/src/connection.rs` around lines 654 - 655, The registry lookup in `route_for_session` is silently swallowing a poisoned `RwLock` by converting `registry.read()` failures into `None`, which can hide a bad VM routing state. Update this path to explicitly handle the lock-read failure on `mngr.hostnames()`/`route_for_session(session_id)` and emit a warning log when the read is poisoned, while preserving the existing fallback behavior only after the failure is recorded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/minimald/src/connection.rs`:
- Around line 589-611: The direct-tcpip handling in channel_open_direct_tcpip is
blocking the SSH session while waiting on TcpStream::connect and timeout before
reply.accept(). Move the upstream connect logic out of the handler path by
spawning or otherwise offloading it, accept the channel immediately, and then
complete the connection asynchronously so other requests on the same russh
session remain responsive. Keep the existing error handling and logging around
the upstream_addr connect attempt, but ensure it happens in the background task
rather than before accepting the channel.
In `@crates/minimald/src/sessions.rs`:
- Around line 14-16: The RwLock import is incorrectly gated to Linux even though
Manager, ManagerHandle, and hostnames() use it on every target. Remove the
#[cfg(target_os = "linux")] gate from the std::sync::RwLock import in
sessions.rs so the symbol is always in scope, and verify the affected
types/functions compile cleanly on non-Linux builds.
In `@docs/specs/03-spec-networking/test-plan.md`:
- Line 185: The blockquote in the test-plan note contains an empty quoted line
that triggers MD028; update the quoted section so the note stays continuous, or
close the blockquote before any blank line. Locate the affected Markdown block
by its blockquote and the adjacent details/summary section, and remove the extra
blank quoted line while keeping the surrounding note text intact.
---
Outside diff comments:
In `@crates/minimald/src/net/policy.rs`:
- Around line 206-209: Update the stale comment in the request-building path in
policy logic so it matches the new keep-alive behavior. The note around the
bounded exchange should no longer say the control socket and gvproxy “speak
HTTP/1.0”; revise the wording near the relevant request/setup code in the policy
exchange flow to describe the current HTTP/1.1 behavior without implying
Connection: close.
---
Nitpick comments:
In `@crates/minimald/src/connection.rs`:
- Around line 654-655: The registry lookup in `route_for_session` is silently
swallowing a poisoned `RwLock` by converting `registry.read()` failures into
`None`, which can hide a bad VM routing state. Update this path to explicitly
handle the lock-read failure on
`mngr.hostnames()`/`route_for_session(session_id)` and emit a warning log when
the read is poisoned, while preserving the existing fallback behavior only after
the failure is recorded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 238c1aa6-477e-48e4-ad2c-d899aca794b5
📒 Files selected for processing (12)
crates/minimald/src/connection.rscrates/minimald/src/net/dns.rscrates/minimald/src/net/gvproxy_network.rscrates/minimald/src/net/policy.rscrates/minimald/src/net/proxy.rscrates/minimald/src/session.rscrates/minimald/src/session_host.rscrates/minimald/src/sessions.rsdocs/specs/03-spec-networking/gn9-forwarding-findings.mddocs/specs/03-spec-networking/test-plan.mddocs/specs/03-spec-networking/test-plan.shjustfile
✅ Files skipped from review due to trivial changes (1)
- docs/specs/03-spec-networking/gn9-forwarding-findings.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/specs/03-spec-networking/test-plan.sh
🛑 Comments failed to post (3)
crates/minimald/src/connection.rs (1)
589-611: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
russh server Handler channel_open concurrent dispatch per session serialized💡 Result:
In Russh, the dispatch of handler methods for a single SSH connection is serialized, meaning that only one handler method execution proceeds at a time per session [1]. The Russh server architecture utilizes an asynchronous event loop that processes incoming SSH messages sequentially for a given connection [1]. When a new channel is requested (e.g., via channel_open_session), the corresponding Handler method is invoked within this sequential event loop [2][3]. Because the Handler trait methods take &mut self, they hold a mutable reference to your server implementation instance, which inherently serializes access to that instance's state while the handler method is executing [3][4]. While the dispatch itself is serialized, you can achieve concurrent behavior for channel operations by spawning asynchronous tasks within your handler methods using tokio::spawn [5][6]. Since the Session handle and Channel objects are Send and Sync (and designed to be passed across task boundaries), you can offload long-running operations or I/O work to separate tasks, allowing the main event loop to remain free to process subsequent SSH messages [5][7][6]. Summary of behavior: - Dispatch serialization: The Russh server dispatches events (including channel_open requests) to your Handler serially per session [1]. - State protection: The &mut self signature of Handler methods ensures exclusive access to the handler's state during the duration of each call [3][4]. - Concurrency strategy: To prevent blocking the server's event loop during a channel handler call, spawn asynchronous tasks for heavy processing or concurrent I/O operations [5][6].
Citations:
- 1: https://docs.rs/russh/latest/russh/index.html
- 2: https://docs.rs/russh/latest/src/russh/server/mod.rs.html
- 3: https://docs.rs/bssh-russh/latest/russh/server/trait.Handler.html
- 4: https://docs.rs/russh/latest/russh/server/trait.Handler.html
- 5: https://github.com/warp-tech/russh/blob/48d898d99808b1c7d4b7b5a9e118d00156920c7d/russh/examples/echoserver.rs
- 6: https://github.com/warp-tech/russh/blob/main/russh/examples/echoserver.rs
- 7: https://docs.rs/russh/latest/russh/struct.Channel.html
Avoid blocking the session handler on the upstream connect In
channel_open_direct_tcpip, awaiting the 10sTcpStream::connectbeforereply.accept()can block other channel/forward requests on the same SSH session, since russh serializes handler dispatch per connection. Offload the connect so the session stays responsive.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/minimald/src/connection.rs` around lines 589 - 611, The direct-tcpip handling in channel_open_direct_tcpip is blocking the SSH session while waiting on TcpStream::connect and timeout before reply.accept(). Move the upstream connect logic out of the handler path by spawning or otherwise offloading it, accept the channel immediately, and then complete the connection asynchronously so other requests on the same russh session remain responsive. Keep the existing error handling and logging around the upstream_addr connect attempt, but ensure it happens in the background task rather than before accepting the channel.crates/minimald/src/sessions.rs (1)
14-16: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Confirm the import is linux-gated while usages are unconditional. rg -n -B1 'use std::sync::RwLock' crates/minimald/src/sessions.rs echo '--- unconditional RwLock usages (no cfg on same/adjacent line) ---' rg -n 'RwLock' crates/minimald/src/sessions.rsRepository: gominimal/minimal
Length of output: 699
🏁 Script executed:
#!/bin/bash set -euo pipefail # Show the file context around the import and the first/last uses. sed -n '1,40p' crates/minimald/src/sessions.rs echo '--- around manager struct/init ---' sed -n '220,310p' crates/minimald/src/sessions.rs echo '--- around ManagerHandle ---' sed -n '900,950p' crates/minimald/src/sessions.rs # Check whether the whole file or these items are cfg-gated elsewhere. rg -n 'cfg\(target_os = "linux"\)|cfg\(target_os="linux"\)|cfg\(linux\)' crates/minimald/src/sessions.rsRepository: gominimal/minimal
Length of output: 8168
Ungate
RwLockimport
RwLockis used inManager,ManagerHandle, andhostnames()on all targets, but the import is still behind#[cfg(target_os = "linux")]. That breaks non-Linux builds becauseRwLockis out of scope.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/minimald/src/sessions.rs` around lines 14 - 16, The RwLock import is incorrectly gated to Linux even though Manager, ManagerHandle, and hostnames() use it on every target. Remove the #[cfg(target_os = "linux")] gate from the std::sync::RwLock import in sessions.rs so the symbol is always in scope, and verify the affected types/functions compile cleanly on non-Linux builds.docs/specs/03-spec-networking/test-plan.md (1)
185-185: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the blank line inside the blockquote.
MD028 will fl 10BC8 ag the empty quoted line here. Keep the quoted note continuous, or end the blockquote before the blank line.
✏️ Proposed fix
> [`gn9-forwarding-findings.md`](gn9-forwarding-findings.md). ->📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.> [`gn9-forwarding-findings.md`](gn9-forwarding-findings.md).🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 185-185: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/specs/03-spec-networking/test-plan.md` at line 185, The blockquote in the test-plan note contains an empty quoted line that triggers MD028; update the quoted section so the note stays continuous, or close the blockquote before any blank line. Locate the affected Markdown block by its blockquote and the adjacent details/summary section, and remove the extra blank quoted line while keeping the surrounding note text intact.Source: Linters/SAST tools
Expert review of the G-N9/be711722 networking changes surfaced five defects. Fixes: - direct-tcpip only reached ports in the static ingress policy and ignored the request otherwise, so an own-ip forward without --ingress (reachable over the live switch lease) was rejected. Move the published-mapping gate to the no-lease branch only; the lease path dials lease:<internal> for any port the PTask listens on. - Renaming a running own-ip PTask deregistered then re-registered its hostname, resetting the route to the loopback placeholder and never re-publishing the lease, stranding in-VM consumers. Capture the live lease and re-publish it under the new name after the swap. - active_lease_target mapped a poisoned registry lock to None (dialing an unreachable loopback on the VM path); recover from poison like the proxy's HostRoute impl. - The Content-Length reply framing discarded a body sent without a Content-Length header, losing gvproxy error diagnostics. Fall back to a bounded, idle-timeout best-effort read (never read-to-EOF, which would hang on the keep-alive connection). - Remove the dead HostnameRegistry::resolve wrapper (all call sites moved to resolve_route); rewrite its tests in terms of resolve_route().target. Verified: musl build + cross clippy clean; minimald lib unit tests pass (net::dns/net::policy/connection); DM1 test plan no-regression (TC3/TC4/TC7 = 200). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "residual G-N9 direct-tcpip" framing for TC8's DM1 000 was wrong. The 000 is `direct-tcpip rejected: session not found`: ssh-forward resolves `dev` to its UUID, but `dev`'s session is reaped ~5s after its held attach (deterministically, and only in the full-plan context — in isolation the identical attach + forward is 6/6 green), so the forward's channel validates a destroyed session and the client resets. direct-tcpip is correctly failing closed and never reaches the lease-dial path, so it is unrelated to G-N9 (whose data path is fully verified: DM1 TC3/4/7, DM3 TC3/4/7/8). Harness hardening did not fix it (in-sandbox hold: no effect; settle+retry worsened it to 0/16 by pushing the curl past the ~5s reap, which pinned the mechanism). Tracked as G-N13 pending daemon-side teardown tracing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The two host-side proxy ports were module-level constants (EGRESS_PROXY_PORT=7654, HTTPS_PROXY_PORT=7655) bound warn-only best-effort, so a second co-resident minimald on one host (the DM4 deployment: a native DM2 daemon alongside an in-VM DM3 daemon) silently lost its proxy bind to whichever daemon started first — its UC2a/UC2b hostname surface vanished. Add `--egress-proxy-port`/`--https-proxy-port` daemon flags (Option<u16>, both-or-neither via clap `requires`) threaded through `Config` into `start_host_proxies`, which now binds and host-publishes the resolved ports. Unset resolves bit-for-bit to the well-known 7654/7655, so every existing client (host curl, browsers, the CLI hint, the single-tenant DM1/DM2/DM3 paths) is unchanged; a co-resident daemon takes a disjoint pair (e.g. 7656/7657) and both control planes serve their proxies side by side. The in-VM (DM1/DM3) gvproxy host-publish uses the same derived port on bind, local, and remote, so DM3 TC3/TC7 are unaffected. `Config` gains `egress_proxy_port`/`https_proxy_port: Option<u16>` (`#[serde(default)]`, backward-compatible); the mTLS proxy stays behind `networking-proxy`, so `https_port` carries a cfg-gated `allow(unused)`. Refs: #627, #626 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a `just dm4` target (dm3, then dm2 on the disjoint proxy pair :7656/:7657) and parameterize the `dm2` recipe's proxy ports (default :7654/:7655, so DM2 standalone is unchanged). Rework the DM4 arm of `test-plan.sh`: TC10 now asserts a listener on BOTH the VM daemon's :7654 and the native daemon's :7656 (no silent bind loss), and TC15 asserts the native daemon's proxy bind succeeds (no "address already in use") and reaches an own-ip ingress session by hostname through its derived :7656 proxy — the acceptance's "TC3 passes against BOTH control planes". Refs: #627, #626 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The networking spec's deployment-model list said DM1/DM3 have "one or more" VMs, but minvmd supervises exactly one named VM (`default`) — a single state.toml/lifecycle.lock/vmm.pid and one host UDS bridge — which is an explicit v0.1 Non-Goal of the minvmd host-daemon spec. Add a v0.1 single-VM scope note reconciling the two specs and mark the G-N7 gap resolved wontfix-by-spec-scope. No code change. Closes: #633 Refs: #626 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mark G-N1 fixed in the gap register and matrix, rewrite the DM4 section for the `just dm4` bring-up (native daemon on the disjoint :7656/:7657), and add the 2026-07-08 re-run note: G-N1 verified on DM4 (one listener each on the VM's :7654/:7655 and the native's :7656/:7657; TC15_NATIVE_PROXY=200), DM2/DM3 unregressed. The VM daemon's host->PTask legs (TC4/TC7-cert/TC8) flake run-to-run under DM4's heavier co-resident load — consistent with the G-N13 session-liveness race (held session reaped before the host probe), not a G-N1 regression (proxy binding is orthogonal; TC3 and the native :7656 proxy are 200 throughout; the fresh single-VM DM3 pass is the clean baseline). Refs: #627, #626 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends the #478 networking CLI test plan (
docs/specs/03-spec-networking/test-plan.{md,sh}) from DM1-centric to comprehensive coverage of all five deployment models, per user decision: full test specs for every DM with status markers, so blocked tests activate as implementation lands. It also lands the G-N9 fix (sandbox-owned lease routing) and the G-N1 fix (per-daemon proxy-port override, unblocking DM4 co-residency) surfaced by these runs.test-plan.md— status-marker legend, DM×TC coverage matrix, per-DM bring-up/teardown/topology sections, eight new test cases (TC10–TC17), CI-workflow mapping, gap register with file:line citations, per-DM results table.test-plan.sh—DM=dm1|dm2|dm3|dm4selector (auto-detected; clean cross-OS SKIP exits),--minimal-dirwiring for DM2, TC10 preflight, gap-cited SKIP banners, TC15 for DM4. Backend harness bug fixed: the in-session server now serves a file-built HTTP reply (printf … > /tmp/rsp; socat … SYSTEM:'cat /tmp/rsp') instead of building it inside socat'sSYSTEM:address, which mangled theprintfinto a non-HTTP (HTTP/0.9) reply that scored000/502regardless of the data path; TC8 now uses a distinctssh -Llocal port so it exercisesdirect-tcpiprather than colliding with gvproxy's ingress listener.expectPTY driver (intermittent readiness wedges) is replaced by a heredoc-piped stdin driver —session_outfor one-shot assertions,session_holdfor server TCs (held-open stdin keeps the shell + own-ip lease + in-session server alive while the host probes). Exact-line nonce markers and anchored sentinels defeat the pre-stty -echoinput replay on slow first attaches.crates/minimald): route in-VM host→PTask consumers to the PTask's switch lease instead of the host-loopback forwarder they cannot reach from inside the VM. See G-N9 fix.crates/minimald):minimald --egress-proxy-port/--https-proxy-port(both-or-neither) let a co-resident second daemon (DM4) serve its own*.min.internalproxy surface on a disjoint port pair; unset stays bit-for-bit on the well-known 7654/7655.just dm4brings the two daemons up on 7654/7655 + 7656/7657. See G-N1 fix.Update (2026-07-07): backend artifact corrected + G-N9 fixed
A live DM1 re-investigation (
gn9-forwarding-findings.md) and 2026-07-07 DM2/DM3 re-runs established that the 2026-07-03 "host→PTask ingress broken on every DM" conclusion conflated two things:socat … SYSTEM:'printf …'served a non-HTTP (HTTP/0.9) reply, so strictcurl -w %{http_code}scored000/502even though the bytes were delivered. Fixed intest-plan.sh. With the fix, DM2 TC3/TC4/TC7-cert/TC8 all PASS (200/200/200+401/200) — the native shared-netns forwarder works end-to-end.:7654/:7655proxies anddirect-tcpipdialled127.0.0.1:<external>, but gvproxy's forwarder listener lives on the host loopback, unreachable from inside the VM → 502 / connect-fail. The daemon can reachlease:<internal>over the switch, so the fix routes those consumers to the lease.Update (2026-07-08): DM1 re-run on
a686f703— proxy path verified, G-N11 closedThe G-N9 lease path was DM1-unit-tested only (DM1 is off the Linux dev host at commit time). A live DM1 re-run (macOS/HVF) now closes that, and along the way surfaced then resolved an interaction and a tooling gap:
curlregression (G-N10) — resolved in-harness. #650 made the session rootfs compose only declared packages;curlwas incidental to the old base rootfs and is not declared, so TC1b/TC2's in-sandbox egress probes broke (curl: command not found).test-plan.shnow provisions it per-session withmin add curl— TC1b/TC2 restored (egress 200,TC2_PEER=YES).be711722. On9443becb, oncemin add curldrove real own-ip egress, TC3:7654regressed to 502 (2/2) while churn-free passes were 200. That was not a separate defect: the proxy-publish rides the same forwarder control leg as the G-N8 ingress attach, so the HTTP/1.0 read-to-EOF framing that dropped over the vsock shuttle also dropped the proxy-publish.be711722(HTTP/1.1 + Content-Length) fixes both. Re-run ona686f703with the same churn: TC3 = 200 (2/2).a686f703(2 passes, with churn): TC3 200/200 · TC4 200/200 · TC7 no-cert 401 / with-cert 200 (2/2) · TC8 000 then 200 (flaky) · TC1b/TC2 200 /PEER=YES. So the in-VM:7654/:7655proxy path is now DM1-runtime-verified unconditionally, and the G-N9 data path is fully verified (DM1 TC3/4/7 + DM3 TC3/4/7/8). TC8's DM1 flake is not a G-N9 residual — root-caused 2026-07-08 as G-N13, a session-liveness race (dev's session is reaped ~5 s after its held attach in the full plan, sossh-forward'sdirect-tcpipvalidates a destroyed session →session not found→ reset;direct-tcpipfails closed and never reaches the lease-dial path; in isolation TC8 is 6/6 green). The earlier host-TLS with-cert failure (G-N12) did not reproduce here (instance-dependent; downgraded to a watch item).Update (2026-07-08): G-N1 fixed + DM2/DM3/DM4 re-run
Fixes the last bounded, here-testable sub-issue of #626: G-N1 — both
minimaldinstances on a DM4 host bound the same hardcoded:7654/:7655(warn-only best-effort), so whichever started first won and the loser's UC2a/UC2b hostname surface silently vanished.Fix — a per-daemon proxy-port override.
minimald rungains--egress-proxy-port/--https-proxy-port(Option<u16>, both-or-neither via claprequires), threaded throughConfigintostart_host_proxies. Unset resolves bit-for-bit to the well-known 7654/7655, so every single-tenant client is unchanged; a co-resident second daemon takes a disjoint pair. A dedicated flag rather thaninstance_num-derivation, because the CLI hardcodesproviders/local-0/ssh.sock— deriving ports frominstance_numwould cascade into socket resolution. Client-side discovery of a co-resident daemon's ports is intentionally out of scope (minimald is single-tenant per host).just dm4(=just dm3thenjust dm2 7656 7657) brings both daemons up on disjoint ports; the DM4 arm oftest-plan.shasserts both proxies bind and drives UC2a through the native:7656. Reviewed by an adversarial multi-lens pass (cross-DM regression / Rust API / shell harness) before commit.Also closes G-N7 (#633) as wontfix-by-spec-scope: single-VM is an explicit v0.1 Non-Goal of
01-spec-minvmd-host-daemon; the networking spec is amended with a v0.1 single-VM scope note.Full DM2/DM3/DM4 re-run (Linux/aarch64 KVM host):
:7654/:7655and native:7656/:7657(native bindstatus="reachable", noAddress already in use);TC15_NATIVE_PROXY=200reaches an own-ip ingress session by hostname through the native daemon's:7656.:7656proxy are 200 throughout).DM × TC coverage matrix
DM5 is an overlay ("any of the above, network-exposed"), so per-session TCs are N/A in its column; TC16 tests the DM5 delta. DM1 = 2026-07-08 macOS/HVF re-run on
a686f703(proxy path now verified — see the DM1 update); DM2 = 2026-07-07 native Linux/aarch64; DM3 = 2026-07-08 KVM VM re-run (TC3/4/7/8 with the G-N8 fix). DM4 TC1–TC9 run against the VM daemon and mirror DM3 (only TC10/TC15 re-executed underDM=dm4, 2026-07-03).TC2_PEER=YES)TC2_PEER=YES)TC2_PEER=YES)a686f703; 502→200 afterbe711722, G-N11 closed)a686f703, 2/2; G-N12 host-TLS not reproduced)direct-tcpip/lease defect)curlabsent from the sandbox rootfs — tooling gap, not networking; DNS itself resolves)vm_egress(R2.5)TC15_NATIVE_PROXY=200via native :7656† DM4 (G-N1 fixed): the VM daemon owns
:7654/:7655and the native daemon the disjoint:7656/:7657(--egress-proxy-port/--https-proxy-port); TC3/TC7 pass against the VM daemon, and TC15 additionally drives the native daemon's:7656.‡ DM2's half of TC12 is the R2.5 rejection — unit-tested in
VmConfig::validate_for, no runtime trigger yet.Gap register: G-N1 — FIXED (2026-07-08, #627) per-daemon
--egress-proxy-port/--https-proxy-portoverride (unset ⇒ well-known 7654/7655); DM4 verified — 1 listener each on VM :7654/:7655 + native :7656/:7657,TC15_NATIVE_PROXY=200· G-N2 DM5 unimplemented (no--server, UDS-only control plane) · G-N3vm_egresshas no runtime config surface (enforcement pending #553) · G-N4 CLI hardcodesegress: None(main.rs:459) · G-N5 dynamic port-map RPC has no daemon handler · G-N6 daemon never consumes mesh enrolment (set_meshtest-only) · G-N7 — resolved wontfix-by-spec-scope (#633) single-VM is an explicit v0.1 Non-Goal of01-spec-minvmd-host-daemon; networking spec amended with a v0.1 single-VM scope note · G-N8 — FIXED (2026-07-08) not a hang: the own-ip+--ingressattach failed fast because gvproxy's forwarder response is dropped over the KVMadd_vsock_port2(listen=false)shuttle (the forward was applied — it shows in the host forwarder table — but theConnection: closereply is lost, soapply_ingressrolls back a successful expose). Fix: HTTP/1.1 keep-alive +Content-Lengthframing innet::policy. Live DM3: TC3/4/7/8 = 200/200/(401+200)/200; also closed the DM1 own-ip-churn 502 (G-N11) · G-N9 — FIXED (2026-07-07); data path fully verified (DM1 TC3/4/7, DM3 TC3/4/7/8) — in-VM host→PTask consumers dial the switchlease:<internal>(see below) · G-N10 — RESOLVED in-harness in-sandboxcurlgone after #650; TC1b/TC2 now provision it withmin add curl· G-N11 — RESOLVED bybe711722the DM1 proxy-publish 502-after-churn was the same forwarder-framing leg as G-N8 · G-N12 — watch hostcurl/LibreSSL once rejected a validminimal logincert on one VM instance; not reproduced ona686f703(no-cert 401 always works) · G-N13 — open TC8's DM1 000 is a session-liveness race, not adirect-tcpip/lease defect:dev's session is reaped ~5 s after its held attach in the full plan, sossh-forward'sdirect-tcpipvalidates a destroyed session (session not found→ reset) and correctly fails closed; isolation is 6/6 green. Needs daemon-side teardown tracing.G-N9 fix (2026-07-07) — sandbox-owned lease routing
The in-VM
:7654/:7655proxies and the SSHdirect-tcpipforward dialled127.0.0.1:<external>, but gvproxy's forwarder listener is on the host loopback and is unreachable from inside the VM, so those consumers got 502 / connect-fail. They now dial the PTask's switch lease (lease:<internal>), which the daemon can reach over the switch.The lease is ephemeral runtime state of the live sandbox — published into the shared routing table for exactly the sandbox's lifetime and never persisted to the on-disk
Record:net/dns.rs— the hostname-registry entry is now aRoute { target, port_map }.set_own_ip_lease/clear_own_ip_lease(keyed by stableSessionId, rename-safe) publish/revert the live lease + its TCP external→internal ingress map;route_for_sessionreads it.net/proxy.rs—Router::routeremaps the authority's published external port to the PTask-internal port viaRoute::dial_port, diallinglease:<internal>.net/gvproxy_network.rs—OwnIpGuardowns the ephemeral lease;finish_own_ip_attachpublishes it only on theControlChannel::Vsock(VM) path and teardown reverts it before the switch detach. DM2 (Unix, shared-netns forwarder) keeps the loopback placeholder — its verified behaviour is untouched.connection.rs—direct-tcpipkeeps its published-mapping validation, then dialslease:<internal>when a live lease exists, else falls back to127.0.0.1:<external>.hostnamesregistry is made unconditional and passed throughSession→SandboxLauncher→ the attach tail with thesession_id.Verification:
cargo test -p minimald --features networking-proxy,networking-wggreen (incl. newown_ip_lease_is_published_only_between_attach_and_teardownandown_ip_lease_routes_to_switch_lease_with_internal_port);fmt+clippy -D warningsclean; DM2 regression re-run all-PASS (the fix is gated to the VM path, so DM2 is byte-identical). The in-VM lease branch was initially unit-tested only because DM3/DM4 were blocked by the separate G-N8 vsock-forwarder response-drop — now pinned and fixed (2026-07-08), so a live DM3 (KVM) run confirms the lease path end-to-end (TC3/4/7/8 = 200/200/(401+200)/200; TC8=200 exercisesdirect-tcpip → lease:8080). DM1 (macOS/HVF) is off the Linux dev host and was never G-N8-affected.Superseded: 2026-07-03 "broken on every runnable DM" finding
The earlier cross-DM finding (host→PTask ingress broken on DM2 and DM3/DM4) is superseded: the DM2 half was the test-backend artifact (DM2 now PASSES with the fixed backend), and the real G-N9 defect was VM-only (the in-VM loopback/host split), now fixed. The remaining DM3/DM4 failures were G-N8 (the vsock-forwarder response-drop), now also fixed (2026-07-08) — DM3 TC3/4/7/8 = 200. Full evidence:
gn9-forwarding-findings.md(§ "G-N8 pinned + fixed") + the Per-DM results / gap register intest-plan.md.Test plan
bash -n test-plan.shDM=dm4/dm5/bogus on macOS → clean SKIP banners / exit 2DM=dm1run on macOS/HVF (expect + heredoc drivers; identical verdicts)DM=dm2/DM=dm3on native Linux/aarch64 (2026-07-03) — full suites + DM4/TC15; surfaced G-N8/G-N9DM=dm2/DM=dm3re-run (2026-07-07) with the fixed backend: DM2 all-PASS incl. TC3/4/7/8; DM3 TC3/4/7/8 re-confirmed G-N8-blockedcargo test -p minimaldgreen,fmt/clippy -D warningsclean, DM2 regression all-PASSContent-Lengthframing innet::policy. LiveDM=dm3full suite: TC3/4/7/8 = 200/200/(401+200)/200 (also runtime-confirms the G-N9 lease path);fmt/clippy/cargo test -p minimaldclean incl. new regression testDM=dm1re-run (2026-07-08,a686f703), 2 passes: proxy path DM1-runtime-verified — TC3/TC4/TC7 = 200 even after own-ip egress (G-N11 closed bybe711722); TC1b/TC2 restored viamin add curl(G-N10); TC8 flaky 000/200 root-caused as the G-N13 session-liveness race (not G-N9); G-N12 host-TLS not reproducedcargo test -p minimald/fmt/clippy -D warningsclean; adversarial cross-DM/API/harness review pre-commit. DM4: 1 listener each on VM :7654/:7655 + native :7656/:7657,TC15_NATIVE_PROXY=200; DM2 all-PASS (TC3/4/7/8 = 200/200/(401+200)/200); DM3 TC1–7/9/10 PASS (TC3/4/7 = 200/200/(401+200)), TC8 flaky (G-N13). Also closed G-N7 (wontfix-by-spec-scope, G-N7: minvmd supervises exactly one VM; spec says DM1/DM3 support one or more #633)dev~5 s into TC8 under full-plan load); wireDM=dm3intoci-linux-kvm.yml; land the sandboxcurlcompose gap (G-N10) somin add curlisn't needed🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Tests
Bug Fixes
Refactor