fix(iii-worker): route 127.0.0.1 to host and fix ERROR-level VM console logs#1451
Merged
andersonleal merged 4 commits intomainfrom Apr 10, 2026
Merged
fix(iii-worker): route 127.0.0.1 to host and fix ERROR-level VM console logs#1451andersonleal merged 4 commits intomainfrom
andersonleal merged 4 commits intomainfrom
Conversation
…le logs Two bugs in local-path worker support: 1. VM console output logged at ERROR level: run_dev did not pass --console-output to __vm-boot, so all guest output went through msb_krun's PortOutputLog which hardcodes log::Level::Error for every line. Now passes --console-output pointing to stdout.log, matching the managed worker path. 2. 127.0.0.1 unreachable from guest: the loopback interface inside the VM captured traffic to 127.0.0.1, preventing it from reaching the host via the smoltcp proxy. Now skips bringing up lo so 127.0.0.1 falls through to the default route via eth0. Also writes /etc/hosts mapping localhost to the gateway IP. Also fixes cargo env sourcing to guard against missing .cargo/env and ensures HOME is set in the dev run script.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughOmit bringing up loopback in network setup, map Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/iii-init/tests/init_integration.rs (1)
3-8:⚠️ Potential issue | 🟡 MinorUpdate the module doc to match actual test gating.
Line 5 says every test is Linux-gated, but the new tests at Line 11 and Line 40 are intentionally cross-platform. The top-level comment should be updated to avoid confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/iii-init/tests/init_integration.rs` around lines 3 - 8, Update the module-level doc comment at the top of the file so it accurately describes which tests are Linux-only and which are cross-platform: change the sentence that currently states "All iii-init functionality is Linux-only, so every test is gated with `#[cfg(target_os = \"linux\")]`" to reflect that only some tests are Linux-gated while others (the tests added around Line 11 and Line 40) are intentionally cross-platform; make the comment clearly state that Linux-specific tests are gated with `#[cfg(target_os = "linux")]` but that other tests in this file run on all platforms.
🧹 Nitpick comments (2)
crates/iii-worker/src/cli/project.rs (1)
151-153: Deduplicate the cargo-env guard string to avoid drift.The same shell guard is now repeated in multiple places; extracting a shared constant keeps inferred and auto-detected Rust paths consistent over time.
Refactor sketch
+const CARGO_ENV_SOURCE_IF_PRESENT: &str = r#"[ -f "$HOME/.cargo/env" ] && . "$HOME/.cargo/env";"#; ... - "[ -f \"$HOME/.cargo/env\" ] && . \"$HOME/.cargo/env\"; cargo build".to_string(), - "[ -f \"$HOME/.cargo/env\" ] && . \"$HOME/.cargo/env\"; cargo run".to_string(), + format!("{CARGO_ENV_SOURCE_IF_PRESENT} cargo build"), + format!("{CARGO_ENV_SOURCE_IF_PRESENT} cargo run"), ... - install_cmd: "[ -f \"$HOME/.cargo/env\" ] && . \"$HOME/.cargo/env\"; cargo build --release".into(), - run_cmd: "[ -f \"$HOME/.cargo/env\" ] && . \"$HOME/.cargo/env\"; cargo run --release".into(), + install_cmd: format!("{CARGO_ENV_SOURCE_IF_PRESENT} cargo build --release"), + run_cmd: format!("{CARGO_ENV_SOURCE_IF_PRESENT} cargo run --release"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/iii-worker/src/cli/project.rs` around lines 151 - 153, Extract the repeated shell guard into a single constant (e.g. const CARGO_ENV_GUARD = "[ -f \"$HOME/.cargo/env\" ] && . \"$HOME/.cargo/env\"; ") and use that constant when constructing the install_cmd and run_cmd strings instead of repeating the literal; update the assignments for install_cmd and run_cmd to concatenate or format with CARGO_ENV_GUARD so the guard is defined once and reused (refer to the install_cmd and run_cmd fields in the same struct where setup_cmd is set).crates/iii-init/tests/init_integration.rs (1)
13-35: Source-string assertions are brittle and can false-pass.The check at Line 33 (
contains("eth0")) can pass from comments alone, and the/etc/hostsassertion depends on exact formatting text. Prefer asserting concrete call patterns (or testing a factored pure helper) to reduce fragile tests.Targeted hardening within current approach
- // It MUST still configure eth0. - assert!( - closure_body.contains("eth0"), - "configure_network must still configure eth0" - ); + // It MUST still configure eth0 via concrete operations. + assert!( + closure_body.contains(r#"set_ip_address(sock, b"eth0\0", ip)"#), + "configure_network must set eth0 IP" + ); + assert!( + closure_body.contains(r#"set_netmask(sock, b"eth0\0", mask)"#), + "configure_network must set eth0 netmask" + ); + assert!( + closure_body.contains(r#"set_interface_up(sock, b"eth0\0")"#), + "configure_network must bring eth0 up" + );Also applies to: 43-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/iii-init/tests/init_integration.rs` around lines 13 - 35, The assertions in the test read the source as a raw string, so contains("eth0") and similar checks are brittle and can match comments or /etc/hosts text; update the test for function configure_network to either (A) strip Rust comments (both // and /* */) from the included source before searching, then use precise regexes to assert the concrete call patterns like set_interface_up\s*\(\s*sock\s*,\s*b"eth0\\0"\s*\) and to assert absence of set_interface_up\s*\(\s*sock\s*,\s*b"lo\\0"\s*\), or (B) factor the logic into a pure helper function and unit-test that helper directly; apply the same stronger check to the other assertions mentioned (the checks around lines 43-50) and reference configure_network, closure_body, and set_interface_up when locating the code to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/iii-init/src/network.rs`:
- Around line 80-84: The current code calls std::fs::write("/etc/hosts", ...)
and ignores errors, which clobbers the file and swallows failures; change this
to safely update /etc/hosts by reading the existing file, preserving all other
entries, replacing any existing "localhost" line that maps to 127.0.0.1 or the
gateway, or appending a gateway->localhost line if none exists, and handle
errors instead of ignoring them (return a Result or log the error). Locate the
std::fs::write("/etc/hosts", format!("{gw}\tlocalhost\n")) call and replace it
with logic that uses std::fs::read_to_string("/etc/hosts") -> modify string
(regex or line-scan) -> write back with std::fs::write or use OpenOptions to
update atomically (write to tmp and rename), and propagate or log any io::Error
rather than discarding it.
---
Outside diff comments:
In `@crates/iii-init/tests/init_integration.rs`:
- Around line 3-8: Update the module-level doc comment at the top of the file so
it accurately describes which tests are Linux-only and which are cross-platform:
change the sentence that currently states "All iii-init functionality is
Linux-only, so every test is gated with `#[cfg(target_os = \"linux\")]`" to
reflect that only some tests are Linux-gated while others (the tests added
around Line 11 and Line 40) are intentionally cross-platform; make the comment
clearly state that Linux-specific tests are gated with `#[cfg(target_os =
"linux")]` but that other tests in this file run on all platforms.
---
Nitpick comments:
In `@crates/iii-init/tests/init_integration.rs`:
- Around line 13-35: The assertions in the test read the source as a raw string,
so contains("eth0") and similar checks are brittle and can match comments or
/etc/hosts text; update the test for function configure_network to either (A)
strip Rust comments (both // and /* */) from the included source before
searching, then use precise regexes to assert the concrete call patterns like
set_interface_up\s*\(\s*sock\s*,\s*b"eth0\\0"\s*\) and to assert absence of
set_interface_up\s*\(\s*sock\s*,\s*b"lo\\0"\s*\), or (B) factor the logic into a
pure helper function and unit-test that helper directly; apply the same stronger
check to the other assertions mentioned (the checks around lines 43-50) and
reference configure_network, closure_body, and set_interface_up when locating
the code to change.
In `@crates/iii-worker/src/cli/project.rs`:
- Around line 151-153: Extract the repeated shell guard into a single constant
(e.g. const CARGO_ENV_GUARD = "[ -f \"$HOME/.cargo/env\" ] && .
\"$HOME/.cargo/env\"; ") and use that constant when constructing the install_cmd
and run_cmd strings instead of repeating the literal; update the assignments for
install_cmd and run_cmd to concatenate or format with CARGO_ENV_GUARD so the
guard is defined once and reused (refer to the install_cmd and run_cmd fields in
the same struct where setup_cmd is set).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 29a128ce-9bb8-41fa-8184-7f2584c92fe1
📒 Files selected for processing (6)
crates/iii-init/src/network.rscrates/iii-init/tests/init_integration.rscrates/iii-worker/src/cli/local_worker.rscrates/iii-worker/src/cli/project.rscrates/iii-worker/src/cli/worker_manager/libkrun.rscrates/iii-worker/tests/vm_args_integration.rs
…ding The `let _ =` pattern swallowed io::Error, making failures invisible. Use eprintln warning consistent with the write_resolv_conf pattern.
…line command arguments Changed the script path for the local worker from `/tmp/iii-dev-run.sh` to `/opt/iii/dev-run.sh` and adjusted the directory structure accordingly. Additionally, streamlined the command argument formatting in the `run_dev` function for improved readability.
sergiofilhowz
approved these changes
Apr 10, 2026
This was referenced Apr 16, 2026
6 tasks
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
2 participants
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
run_dev(local-path workers) did not pass--console-outputto__vm-boot, so all guest output (cargo downloads, compiles, app logs) went throughmsb_krun'sPortOutputLogwhich hardcodeslog::Level::Errorfor every line. Now passes--console-outputpointing tostdout.log, matching the managed worker path.127.0.0.1, preventing it from reaching the host via the smoltcp proxy. Now skips bringing uploso127.0.0.1falls through to the default route viaeth0. Also writes/etc/hostsmappinglocalhostto the gateway IP..cargo/envsourcing with existence check and ensuresHOMEis set in the dev run script.Test plan
cargo test -p iii-init -p iii-worker)configure_network_source_omits_loopback_setup— verifies lo is not brought upetc_hosts_format_maps_localhost_to_gateway— verifies /etc/hosts formatrun_dev_console_output_path_convention— verifies stdout.log path conventionrun_dev_console_output_parses_as_vm_boot_arg— verifies --console-output parsingiii worker logs <name> -fshould show clean output without ERROR prefixws://127.0.0.1:49134inside VM should reach engineSummary by CodeRabbit
New Features
Improvements
Tests