test(e2e): Machine networking — first exercise of the network plane

[AprilNEA]

The Machines feature had lifecycle e2e (machine.rs) but its network
plane was never tested — machine.rs asserts an agent-reported IP over
vsock and never drives a packet. This drives real traffic from inside a
Machine over the machines.exec vsock channel (the Machine's docker exec). No product code changed.

cargo test -p arcbox-e2e --test machine_network -- --ignored --nocapture

Datapath (source-verified)

A Machine's primary NIC is the same socketpair userspace netstack +
TcpBridge as the System VM's (darwin.rs: gateway 10.0.2.1, guest
10.0.2.2, DHCP + DnsForwarder at the gateway). Egress is pure in-process
host-socket proxying — no privileged helper, no host route (the
172.16/12 route_reconciler is System-VM-only and never wired to
Machines) — so this runs in the isolated e2e daemon. The agent channel is
vsock, independent of the network plane, so exec works even while the
network is under test.

Scenarios (one Machine, one boot)

• M1 egress TCP — wget a host-local origin at 10.0.2.1:<port>,
  byte-exact: first proof a Machine reaches the network at all.
• M2 DNS — host.docker.internal / gateway.docker.internal resolve
  to the gateway via the in-VMM DnsForwarder.
• M3 egress volume — 16 MiB download, byte-exact and bounded.
• M4 metadata — inspect reports gateway 10.0.2.1 + it as a DNS
  server; IP is a valid routable IPv4.
• M5 SSH contract — ssh_info is still Unimplemented; pins the gap
  so a future SSH feature trips this test into growing real coverage.

Not covered — by architecture, not omission

Machine↔Machine, Machine→container, Machine→System VM are isolated per-VM
netstacks (the vmnet bridge NIC is never brought up guest-side for
Machines; two Machines can both be 10.0.2.2 — no shared segment). host→
Machine inbound/SSH doesn't exist. Documented in the plan; no active test
(would be flaky/meaningless today). M5's assert-the-gap-then-grow is the
template for when cross-machine networking lands.

Finding: reported IP ≠ datapath IP on a fake-IP host

The datapath logs gateway=10.0.2.1, guest=10.0.2.2 and egress/DNS work
through it (M1–M3 pass), but inspect().network.ip_address came back
198.18.11.51 — the Surge/Clash fake-IP range this host runs, not
10.0.2.2. select_routable_ip picked a non-datapath address the agent
enumerated; the desktop UI shows this field, so a user on such a host sees
a bogus IP. M4 WARN-logs it rather than gating (host-environment-tangled).
Worth a clean-host repro to decide whether select_routable_ip should
prefer the datapath subnet.

Verification

5/5 green on a real VZ VM (86 s incl. machine boot + live CDN alpine pull).

[pullfrog]

Important

The M2 DNS scenario passes trivially — its assertion matches busybox nslookup's server-echo line, not the resolved answer, so it can't distinguish a working DnsForwarder from a broken one. Worth fixing before merge so the scenario provides the coverage it claims.

Reviewed changes — a new #[ignore]d VZ e2e (machine_network.rs) that drives real traffic from inside a Machine over the machines.exec vsock channel, plus a plan doc. No product code changed.

• Add machine_network.rs e2e — five sub-scenarios on one Machine boot (egress TCP, DNS, 16 MiB egress volume, inspect metadata, SSH-unimplemented contract), failures aggregated so one boot reports the full picture.
• Add machine-network-e2e.md plan — documents the datapath (same socketpair netstack + TcpBridge as the System VM), what's out of scope by architecture (Machine↔Machine, host→Machine inbound), and the select_routable_ip fake-IP finding.
• Reuse spawn_blob_server — host-local 127.0.0.1:0 origin the guest reaches via the 10.0.2.1 gateway→loopback translation, keeping the test on the isolated e2e daemon.

ℹ️ Nitpicks

• m1_egress_tcp/m3_egress_volume (lines 274, 309): wget … | wc -c makes the pipeline's exit status wc's (always 0), so the if exit != 0 guard never fires on a wget failure. The byte-count check is what actually catches a failed/truncated transfer — the exit check is effectively dead. Harmless, but the bail!("wget exited {exit}…") message will never be seen.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d525765c48

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[greptile-apps]

Greptile Summary

The PR adds the first end-to-end exercise of the Machine network plane.

• Adds TCP egress, DNS resolution, volume integrity, network metadata, and SSH-contract scenarios.
• Adds BusyBox nslookup parsing fixtures that distinguish DNS answers from resolver preambles.
• Updates the patterned HTTP fixture to keep connections open until clients close them.
• Documents the tested datapath, architectural exclusions, and known metadata producer limitation.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
tests/e2e/tests/machine_network.rs	Adds a real-Machine networking suite and focused parser tests; the prior DNS-preamble and metadata-contract concerns are addressed at current HEAD.
tests/e2e/src/net_fixtures.rs	Keeps the patterned HTTP fixture socket alive until the client closes after receiving the complete response body.
internal-docs/plans/machine-network-e2e.md	Documents the Machine datapath, scenario scope, unsupported paths, and known IP metadata producer defect.

Sequence Diagram

sequenceDiagram
    participant Test as machine_network E2E
    participant Daemon as Arcbox daemon
    participant Agent as Machine agent (vsock)
    participant Guest as Machine guest
    participant Netstack as Userspace netstack / DNS
    participant Origin as Host-local origin
    Test->>Daemon: Create and start Machine
    Test->>Daemon: machines.exec(command)
    Daemon->>Agent: Exec over vsock
    Agent->>Guest: Run wget / nslookup
    Guest->>Netstack: TCP or DNS traffic
    Netstack->>Origin: Proxy host-socket request
    Origin-->>Netstack: Response bytes
    Netstack-->>Guest: Network response
    Guest-->>Agent: stdout and exit status
    Agent-->>Daemon: Exec stream
    Daemon-->>Test: Captured result
    Test->>Daemon: inspect / ssh_info
    Daemon-->>Test: Metadata / Unimplemented

Loading

_{Reviews (8): Last reviewed commit: "docs(e2e): correct the nslookup exit-cod..." | Re-trigger Greptile}

[pullfrog]

ℹ️ No critical issues — minor suggestions inline.

The M2 tautology from the previous review is genuinely fixed: nslookup_answer_addrs gates on the Name: boundary before parsing Address lines, so the resolver preamble can no longer satisfy the assertion, and an unrecognised format yields an empty vec (fails closed). Thread replied and resolved. One fixture-fidelity nit inline — M2 behaves correctly against real Alpine output today, so it's not blocking.

Reviewed changes — the incremental commit ed779e79 on top of the initial review's d525765. Still test-only; no product code changed.

• Rewrote the M2 DNS assertion — added nslookup_answer_addrs, which skips to the Name: boundary and parses each following Address… line's first whitespace token as an Ipv4Addr (the first-token split handles busybox's reverse-lookup shape Address 1: 10.0.2.1 host.docker.internal). m2_dns now requires the gateway among the parsed answers, and the per-name loop bail!s so one name can't mask the other.
• Added three parser unit tests that run without a VM, so the parser stays honest even when the #[ignore]d scenario isn't run. Two of them (nslookup_preamble_is_not_an_answer, nslookup_wrong_answer_is_distinguishable) are the regression guards against reverting to whole-output matching.
• Root-caused the M4 ip_address finding — the field isn't an interface address at all. Traced and confirmed against source: the guest agent fills SystemInfo.ip_addresses by shelling out to hostname -I then hostname -i (guest/arcbox-agent/src/agent/linux/system_info.rs:103-119), busybox has no -I, and its -i resolves the guest's own hostname through the resolver rather than enumerating interfaces. So select_routable_ip (app/arcbox-core/src/machine.rs:1376-1401) faithfully returns a DNS answer. The writeup and the decision to characterize rather than gate are both correct.
• Rewrote the plan doc's M2/M4 rows and Finding section to match.

Also independently confirmed M2's assertion is reachable for Machines, not just containers: register_host_dns (app/arcbox-daemon/src/services.rs:235-252) writes both names at the gateway IP into the LocalHostsTable that MachineManager threads through to the VMM's DnsForwarder.

ℹ️ M4's deliberate weakening has no expiry

M5 is built as a tripwire — it asserts the gap so a future SSH feature trips the test and prompts real coverage. M4's ip_address check is the same kind of deliberate concession but has no equivalent trip: it only warn!s on a non-datapath address, so when the guest-agent producer is fixed, nothing fails to prompt the tightening the code comment and plan doc both call for ("tighten M4 to the datapath subnet once that lands"). Relatedly, that comment says the producer fix is "tracked separately" but names no ticket, so the follow-up has no anchor outside this file. Your call on whether that's worth a ticket reference or an inverted assertion — flagging it because the two scenarios in the same file resolve the same tension in opposite ways.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

ℹ️ No critical issues — minor suggestions inline.

Reviewed changes — bea15be, a merge of origin/master into test/machine-network (167 files, +14,126/−2,336). It touches neither of the PR's two files, so the review question this round isn't "what did the author change" but "did master create a semantic conflict under the test's feet". I traced every assumption M1–M5 rest on and they all still hold:

• M5 tripwire intact — ssh_info still returns Status::unimplemented (app/arcbox-api/src/grpc/machine.rs:464-469).
• M2 reachable — register_host_dns still registers host.docker.internal / gateway.docker.internal at the gateway IP (app/arcbox-daemon/src/services.rs:452-469). The merge's dns.rs rewrite (+225: Mutex-backed cache, zeroed NSCOUNT/ARCOUNT for EDNS, QTYPE-honoring NODATA, per-upstream connect() + txid validation) improves M2's environment — busybox nslookup issues A queries and the local mapping is IPv4, so qtype_matches passes and the normal A answer is returned (virt/arcbox-net/src/dns.rs:454-463).
• M4 writeup still accurate — guest/arcbox-agent/src/agent/linux/system_info.rs and app/arcbox-core/src/machine.rs are both untouched, so the hostname -i-resolves-via-DNS root cause is unchanged.
• Plan doc's DHCP claim holds — machine_init() is still lo up → configure_primary_interface_dhcp() → write_machine_resolv_conf() (guest/arcbox-agent/src/init.rs:491-500). The merge's new is_uplink_interface (eth*/en* only, init.rs:779-791) narrows detect_primary_interface's old negative-filter, but Alpine-under-VZ names virtio-net eth0, so it passes. The bridge-NIC FORWARD-rule removal doesn't reach Machines — machine_init never calls configure_bridge_nic().
• M1/M3 still compile and can't be reaped mid-flight — spawn_blob_server's signature is unchanged despite net_fixtures.rs growing +174, and splicetcp's DEAD_FLOW_TIMEOUT is 300s (common/splicetcp/src/tcp_bridge/mod.rs:1336) against the test's NET_BUDGET = 60s.

One thing the merge genuinely did change for this PR is inline below: it landed the integrity-check fixtures M3 was written without.

ℹ️ No green run exists against this merge base

The PR's evidence of correctness is the author's reported 5/5 on a real VZ VM — but that run predates both ed779e79 (the M2 parser rewrite, so the current nslookup_answer_addrs has only ever been validated against hand-written sample strings, never captured busybox output) and this merge, which pulled ~2,500 lines of splicetcp/TcpBridge change onto the very datapath M1/M3 drive, plus the guest-agent init and DNS-forwarder rewrites above.

Every conflict I could name, I falsified by reading — that's what the list above is. But "I found no semantic conflict" and "the scenario is green" aren't the same claim, and for a test-only PR the passing run is the deliverable. Worth one cargo test -p arcbox-e2e --test machine_network -- --ignored on the merge commit before merging, and worth pasting the result into the PR so the next reader doesn't have to re-derive this.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

ℹ️ No critical issues — one carried-over comment inaccuracy inline.

The rebuttal in this commit is correct and my previous round was wrong. I re-verified busybox 1.37.0 networking/nslookup.c against upstream: the FEATURE_NSLOOKUP_BIG preamble formats through xmalloc_sockaddr2dotted (with the :53 port), while the legacy path goes through xmalloc_sockaddr2dotted_noport → print_host (bare, numbered). The previous review asserted the opposite and used that to call the :53 fixture unfaithful. Keeping both fixtures and attributing each to its build is the better resolution than the swap I proposed.

I re-traced the gate-guard question by hand rather than taking the doc comment's word for it — deleting skip_while(… "Name:").skip(1) now makes nslookup_bare_preamble_address_is_not_an_answer return [10.0.2.1] and nslookup_legacy_answer_is_parsed return [10.0.2.1, 10.0.2.1], so both go red. The Name: gate is genuinely load-bearing now, which it wasn't before this commit.

Reviewed changes — the single commit 01d1d78 on top of the merge bea15be. Touches only the machine_network.rs parser doc comment and its unit-test fixtures; no product code, no scenario-logic change.

• Documented both busybox nslookup output shapes — the parser doc comment now shows the BIG and legacy formats side by side and names the xmalloc_sockaddr2dotted / _noport split as the reason the preamble address differs, which is what makes the Name: gate necessary rather than incidental.
• Split the preamble fixture in two — nslookup_bare_preamble_address_is_not_an_answer (legacy, bare IP) now carries the regression, and nslookup_ported_preamble_address_is_not_an_answer (BIG, :53) pins the shape M2 actually runs against. The second one's doc comment states outright that it passes with or without the gate, so the split is honest about which test does the work.
• Added nslookup_legacy_answer_is_parsed — covers the numbered Address 1: <ip> <revhost> shape and is the only remaining test that exercises the first-token split, since nslookup_wrong_answer_is_distinguishable was retargeted to the BIG shape.
• Retargeted nslookup_wrong_answer_is_distinguishable to BIG's unnumbered Address: <ip>, keeping it as the guard against reverting to whole-output substring matching.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

ℹ️ No critical issues — the incremental changes are correct and strictly strengthen M3. One non-anchorable concern in the body.

Reviewed changes — 7e22638 + acbe5f07, the delta since my last review at 01d1d78. Note the supplied incremental diff was baselined at 7e22638 rather than the prior review's head, so I re-derived scope by hand (git diff 01d1d78..acbe5f07); the M3 rewrite is only visible in that wider range. Both commits are test-only.

• Converted M3 from a length check into a content check — spawn_pattern_server(VOLUME_BYTES, VOLUME_SEED) + in-guest wget -q -O - | sha256sum | cut -d' ' -f1 compared against PatternServer::sha256(), which is W15 download_integrity's idiom. This retires the bea15be integrity nit; M1 correctly keeps wc -c as the reachability probe.
• Named the empty-input digest as a distinct failure — EMPTY_SHA256 (verified correct against the existing sha256_hex_is_lowercase_64_hex assertion at tests/e2e/src/net_fixtures.rs:604-616), so a zero-byte delivery masked by the pipeline's exit status reports "no data at all" rather than generic corruption. Good call given that shape is exactly what misled the first run.
• Made spawn_pattern_server linger until the client closes, and error-check the second write_all, matching spawn_blob_server's existing loop at net_fixtures.rs:159-160. I checked both callers — W15 (network_workload.rs:597) and M3 (machine_network.rs:371) — and the accept loop runs on its own thread, so a per-connection thread parked in the drain loop can't stall accepts. The only residual divergence from spawn_blob_server is the timings push (net_fixtures.rs:161-164), which W15's assert_no_established_flows doesn't depend on. No issue.
• Updated the plan doc's M3 row and the module doc to describe the hashed check.

⚠️ The fixture fix closed the last accidental coverage of a real datapath behaviour, and the deferred issue doesn't exist yet

The measurement in your reply is the interesting artifact here, more than the fix: a Machine downloading 16 MiB from an origin that closes promptly after its last write received zero bytes on two runs in three. Whatever the correct answer is, "the guest gets nothing" is not it — a well-behaved HTTP origin closing right after writing its body is completely ordinary.

You wrote that this "belongs in its own issue rather than this test PR," and I agree. But no such issue exists — I found no ABX reference, nothing in docs/net-perf-limits.md, and nothing in the AGENTS.md files covering it. And after this fix, no e2e fixture exercises a prompt clean close at all: spawn_blob_server lingers, spawn_pattern_server now lingers too, and spawn_chaos_origin sends a RST mid-body (SO_LINGER{1,0}) — a different case. The one thing that happened to reproduce this has been removed, which is the right call for a test PR and the wrong outcome for the underlying behaviour.

Worth noting it isn't Machine-specific: W15 runs the same origin at 32 MiB over the container path, so the same shape was latent there — your fixture fix covering both is what makes that invisible from now on.

Technical details

# Prompt clean origin close vs. buffered egress response

## What the PR established empirically
- 16 MiB download, pattern origin closing immediately after the last queued
  write: 2 failures / 3 runs, received hash == empty-input digest (zero bytes).
- Same download against the lingering blob origin: never failed.
- With the linger added: 3 / 3 green.

## Why the fix is right but incomplete
`spawn_pattern_server` now matches `spawn_blob_server`, so the fixture is
consistent and W15 is no longer exposed. That is the correct scope for a
test-only PR. What it does not do is answer whether the datapath should be
losing an already-buffered response when the origin closes promptly.

## Where I looked (both directions read clean)
- `common/splicetcp/src/tcp_bridge/fast_path.rs:760-786` — `poll_fast_path`
  treats `Ok(0)` as `host_eof`, forwards FIN, leaves the entry alive.
- `common/splicetcp/src/direct_rx.rs:285-300` — `read_promoted_conn` treats
  `Ok(0)` as FIN without setting `dead`; the regression
  `clean_eof_sends_fin_without_setting_dead` (`direct_rx.rs:548`) pins it.

So the obvious "clean EOF kills the flow" reading is already excluded. One
shape I could not exclude by reading, offered as a hypothesis rather than a
diagnosis: at `direct_rx.rs:279-281` a `send_budget == 0` inline task sleeps
1 ms and `continue`s, re-checking `dead` at `262-264`. If `close_fast_path`
sets `dead` in that window before the task has drained upstream, the task
exits without ever reading the response. I have not instrumented this and it
should not be treated as the cause.

## Why prior fixes don't cover it
- `49db7ac6` — keep inline flows alive after a clean EOF.
- `33595640` (ABX-431) — RST on upstream death.
Each addresses a different aspect; neither is about losing data already
buffered at the point of a clean close.

## Suggested outcome
File the issue you named, with the 2/3 measurement and the fixture diff as
the reproducer, so the reproduction survives even though the fixture no
longer performs it. No change requested in this PR.

_{Mode IncrementalReview · files reviewed 3 · base master · head test/machine-network (acbe5f0) · reviewed commits 7e22638 (make M3 a content check, not a length check), acbe5f0 (let the pattern origin linger before closing) · prior pullfrog review at 01d1d78 (#pullrequestreview-4832720720). Not approving: the NXDOMAIN exit-code/stream inaccuracy threads (machine_network.rs:288, :443-444, internal-docs/plans/machine-network-e2e.md:38) are still open and untouched by these commits.}

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

Run failed. View the logs →

  ^{｜ Rerun failed job ➔ ｜ View workflow run ｜ via Pullfrog ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

ℹ️ No critical issues — one missed site from the last round, inline.

Reviewed changes — the true delta since my last review is two commits, not one. acbe5f07 was force-pushed away and rewritten as a96c8b12, so the harness' incremental diff (baselined at a96c8b1) omits that rewrite; I re-derived scope from the commit itself.

• a96c8b12 (rewrite of acbe5f07) — the spawn_pattern_server linger change is byte-identical to what I already reviewed, but the rewrite adds M3's EMPTY_SHA256 branch and, with it, the CORE-66 attribution. The commit message correction is the right call: it now says outright that this is a consistency fix and not a fix for the M3 flake, and that the 3 green runs it previously leaned on carry almost no signal at the measured ~1-in-4 failure rate. Retracting a claim on your own evidence is the harder thing to write.
• 9469f28 — corrects the nslookup exit-code and error-stream claims. Two of the three sites my previous thread named are fixed; the third is not (inline).

The empty-digest branch is a good shape in its own right: a pipeline reports its last stage's status, so a wget that delivered nothing still exits 0 and hands sha256sum an empty stream whose digest is a perfectly valid hash. Naming that case beats reporting it as generic corruption.

ℹ️ The known ~1-in-4 M3 failure isn't recorded anywhere a reader looks first

This PR knowingly ships an e2e whose M3 scenario fails roughly one run in four, due to open defect CORE-66 (the Machine's guest route table goes empty mid-session → ENETUNREACH). That's the right call — the tripwire is more valuable than the green — but the fact currently lives in exactly two places: the a96c8b12 commit message, and a bail! string that is only visible after you've already hit the failure.

grep -rn CORE-66 across the repo returns only machine_network.rs:395,399. Meanwhile both documents a reader would consult beforehand present the test as reliable:

• the PR body's Verification section still reads "5/5 green on a real VZ VM (86 s incl. machine boot + live CDN alpine pull)";
• internal-docs/plans/machine-network-e2e.md never mentions the flake or CORE-66 at all.

So the next person to run this — quite possibly not you — burns a debugging session on a failure you have already root-caused and filed. The natural home is tests/e2e/AGENTS.md's "Known failure signatures" list, which exists for precisely this ("symptom → first commands → likely cause") and would make cat /proc/net/route the first move rather than the tenth. A line in the plan doc and a correction to the PR body's 5/5 claim would close the other two gaps.

Informational rather than blocking: the test is #[ignore]d, so it cannot redden CI, and no product code is affected.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

Third site from the exit-code thread, still stale. 9469f28 fixed the two machine_network.rs sites the thread named (:288 and :443-444), but this M2 row still says busybox "exits 0 on NXDOMAIN" — which is wrong for both builds: BIG sets G.exitcode = EXIT_FAILURE, legacy returns (rc != 0). Either way, exit 1.

Worth catching now specifically because the thread that tracked it is resolved, so nothing else is watching this line. Same reasoning as last round applies here too — this doc is the plan of record for the network-plane work, so a wrong busybox claim in it will be read as authoritative.

The rest of the row is accurate and the conclusion it draws is unaffected: matching whole output is a tautology when the resolver is the expected answer, and m2_dns gating on parsed answers only remains correct regardless of the exit code.