Add bounded direct path repair - #846
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds DirectPathRequest protocol and a periodic direct-path maintenance controller that selects peers for reverse-dial repair, mesh wiring/state for cooldown and filtering, receiver-side validation and reverse-dial installation, and unit tests covering planning, cooldown, and candidate filtering. ChangesDirect Path Maintenance Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
e3cd7e0 to
08bb730
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
crates/mesh-llm-host-runtime/src/protocol/mod.rs (1)
280-292: ⚡ Quick winUse a dedicated validation error for missing direct-path address.
Line 290 currently maps an empty
serialized_addrtoInvalidEndpointId { got: 0 }, which misreports the fault and makes logs harder to triage. Use a direct-path-specific error (and keep it aligned with the protocol crate’s validator behavior).Suggested direction
- if self.serialized_addr.is_empty() { - return Err(ControlFrameError::InvalidEndpointId { got: 0 }); - } + if self.serialized_addr.is_empty() { + return Err(ControlFrameError::MissingDirectPathAddress); + }(Plus add
MissingDirectPathAddresstoControlFrameErrorand itsDisplayimpl.)🤖 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/mesh-llm-host-runtime/src/protocol/mod.rs` around lines 280 - 292, The validate_frame implementation for crate::proto::node::DirectPathRequest currently returns ControlFrameError::InvalidEndpointId when serialized_addr is empty; change this to return a new, dedicated variant ControlFrameError::MissingDirectPathAddress (add that variant to the ControlFrameError enum and update its Display implementation) and update validate_frame to return MissingDirectPathAddress when serialized_addr.is_empty() so the error accurately reports a missing direct-path address and stays aligned with the protocol crate's validator behavior.
🤖 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/mesh-llm-host-runtime/src/mesh/direct_path.rs`:
- Around line 204-210: The mapping that builds DirectPathObservation currently
iterates over state.connections without ensuring the peer is an admitted mesh
peer; update the closure that produces DirectPathObservation to first check the
peer admission flag (e.g., only proceed when peer.is_admitted or peer.admitted
== true depending on your Peer struct) and skip non-admitted peers so requests
are only planned for admitted peers; keep use of
heartbeat::selected_path_snapshot(conn) and
endpoint_addr_has_direct_candidate(&peer.addr) unchanged for admitted peers.
In `@crates/mesh-llm-host-runtime/src/mesh/mod.rs`:
- Around line 4528-4532: The call to filter_endpoint_addr_for_bind_ip currently
passes self.relay_policy.uses_relay(), which causes
RelayPolicy::ExplicitlyDisabled to be treated like relay-enabled and drops
public candidates; change those call sites (where
filter_endpoint_addr_for_bind_ip is invoked, including the occurrences around
the current context and the similar site ~4670-4674) to instead gate on
raw-STUN/public-discovery mode: use an existing uses_raw_stun() method or add a
dedicated helper (e.g., preserve_public_ipv4_candidates) on RelayPolicy and pass
its boolean result to filter_endpoint_addr_for_bind_ip so ExplicitlyDisabled
preserves public IPv4 candidates. Ensure the change is applied to every call
site mentioned.
- Line 3892: The helper new_test_node_from_endpoint constructs an Endpoint with
RelayMode::Disabled but assigns RelayPolicy::DefaultPublic, causing mismatched
behavior for relay/invite/token tests; update the helper so the Endpoint's
relay_policy matches the intended RelayMode (e.g., set relay_policy to
RelayPolicy::Disabled) or add a parameter to new_test_node_from_endpoint to
accept the desired RelayPolicy and thread it through when building the Endpoint;
ensure the change uses the same symbols (new_test_node_from_endpoint, Endpoint,
RelayMode::Disabled, RelayPolicy::DefaultPublic/Disabled) so tests that exercise
invite-token or advertisement filtering take the correct branch.
In `@crates/mesh-llm-host-runtime/src/mesh/tests.rs`:
- Around line 3158-3280: These three direct-path tests
(direct_path_maintenance_requires_candidate_and_grace_period,
direct_path_maintenance_cooldown_and_inflight_suppress_requests,
direct_path_request_keeps_only_previously_advertised_direct_candidates) should
be moved out of the large tests.rs into a new semantically named test module
(e.g., mod direct_path) so the direct-path responsibility is isolated; create
the new test module file, paste the three tests there, add a mod declaration in
the original tests module to include it, and update imports/visibility so the
tests can see DirectPathMaintenanceController, DirectPathObservation,
RelayPathSnapshot, SelectedPathKind, DIRECT_PATH_REPAIR_GRACE_SECS,
DIRECT_PATH_REPAIR_COOLDOWN_SECS,
endpoint_addr_with_previously_advertised_direct_candidates,
make_test_endpoint_id and TransportAddr/EndpointAddr types (adjust to pub(crate)
or re-export helpers if needed) so the tests compile and cargo test passes.
---
Nitpick comments:
In `@crates/mesh-llm-host-runtime/src/protocol/mod.rs`:
- Around line 280-292: The validate_frame implementation for
crate::proto::node::DirectPathRequest currently returns
ControlFrameError::InvalidEndpointId when serialized_addr is empty; change this
to return a new, dedicated variant ControlFrameError::MissingDirectPathAddress
(add that variant to the ControlFrameError enum and update its Display
implementation) and update validate_frame to return MissingDirectPathAddress
when serialized_addr.is_empty() so the error accurately reports a missing
direct-path address and stays aligned with the protocol crate's validator
behavior.
🪄 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 Plus
Run ID: 54b06bef-7ae1-464c-a2ff-7ef87b65f527
📒 Files selected for processing (9)
crates/mesh-llm-host-runtime/src/mesh/direct_path.rscrates/mesh-llm-host-runtime/src/mesh/heartbeat.rscrates/mesh-llm-host-runtime/src/mesh/mod.rscrates/mesh-llm-host-runtime/src/mesh/tests.rscrates/mesh-llm-host-runtime/src/protocol/mod.rscrates/mesh-llm-host-runtime/src/runtime/mod.rscrates/mesh-llm-protocol/proto/node.protocrates/mesh-llm-protocol/src/proto/node.rscrates/mesh-llm-protocol/src/protocol/mod.rs
|
Wow. Going to try this out. |
08bb730 to
4431fcf
Compare
4431fcf to
d053842
Compare
* origin/main: Add bounded direct path repair (#846) Fix skippy smoke PR gate (#850) Stabilize skippy smoke chain startup (#849) # Conflicts: # crates/mesh-llm-host-runtime/src/mesh/direct_path.rs # crates/mesh-llm-host-runtime/src/mesh/mod.rs # crates/mesh-llm-host-runtime/src/protocol/mod.rs # crates/mesh-llm-protocol/src/protocol/mod.rs
* origin/main: (29 commits) MoA: don't let small-model consensus pre-empt a still-running large model (#837) fix(console): render thinking traces as markdown Add bounded direct path repair (#846) Fix skippy smoke PR gate (#850) Stabilize skippy smoke chain startup (#849) fix(ci): switch back to auto-assign workflow fix(website): polish longform visual explainer (#843) fix: gemma thinking Carry GLM llama MTP patches (#840) Refresh llama.cpp canary patch queue (#839) Add transport-aware Skippy stage ordering (#814) Share Skippy stage wire byte accounting (#818) Report Skippy artifact cold-start costs (#815) fix: debug output capturing for TUI / panics (#827) fix(hero): visual corrections for iPhone SE size devices (#838) Add Skippy stage role metadata (#816) Add Skippy request cache epoch telemetry (#817) Consolidate agent skills and fix stale docs (Windows deploy, repo map, design docs) (#836) feature(version): normalize version markers for different build types (#831) fix(website): fix visual regressions (#835) ... # Conflicts: # AGENTS.md
Summary
Adds a targeted mesh-level direct-path repair flow for peers whose selected iroh path has fallen back to relay or is unknown even though a direct UDP candidate is available.
The repair path uses a dedicated mesh stream byte (
0x0e) and aDirectPathRequestframe. It does not useSTREAM_SUBPROTOCOL, does not gossip, and only asks one peer per maintenance tick to reverse-dial the requester’s current advertised endpoint address.Why
In private/mDNS lab meshes, one side may need the other side to initiate the UDP attempt for iroh to converge on a direct path. The previous behavior could sit on relay/unknown path state even when a usable LAN candidate existed. This keeps the fix close to mesh networking and iroh connection management without adding broad discovery chatter.
This follows the iroh layering model: iroh authenticates the endpoint reached by a dial, while mesh-llm constrains which peer-advertised candidates this mesh-level repair path is allowed to hand to iroh.
Flow Diagram
sequenceDiagram autonumber participant A as Node A maintenance loop participant Conn as Existing mesh QUIC connection participant B as Node B mesh dispatcher participant Iroh as iroh endpoint Note over A,B: Existing admitted mesh connection is already alive A->>A: Observe selected path = relay/unknown<br/>and peer has direct UDP candidate A->>A: Apply grace period, one-at-a-time gate,<br/>sender cooldown, inflight suppression A->>Conn: Open bi stream with byte 0x0e Conn->>B: DirectPathRequest{requester_id, gen, EndpointAddr} B->>B: Validate generation, requester id,<br/>admitted peer, known direct candidate, receiver cooldown B->>Iroh: Dial A using only previously advertised direct candidates Iroh-->>B: New QUIC connection if endpoint identity verifies B->>B: Install connection, dispatch streams,<br/>initiate gossipSecurity / Layering Note
This is only reachable from an already-admitted mesh peer over an existing QUIC connection. iroh still verifies the remote endpoint identity during the reverse dial; a candidate cannot produce an accepted connection unless the remote proves it owns the requested endpoint id.
mesh-llm still has to authorize candidate eligibility before handing addresses to iroh. Otherwise this repair frame could become a new low-rate UDP egress primitive where an admitted peer asks us to try socket candidates that did not come through normal mesh membership. To keep the repair path aligned with iroh’s design, the receiver intersects the requested
EndpointAddrwith that peer’s already-known membership address and keeps only previously advertised direct IP candidates. Unknown candidates and relay candidates in the request are ignored before any dial attempt or receiver cooldown is recorded.Details
DirectPathRequestto the mesh protocol surface and validates generation/requester identity.STREAM_DIRECT_PATH_REQUEST = 0x0einto the mesh stream dispatcher.Validation
cargo fmt --allgit diff --checkcargo test -p mesh-llm-host-runtime endpoint_addr_filter --libcargo test -p mesh-llm-host-runtime direct_path_maintenance --libcargo test -p mesh-llm-host-runtime direct_path --libcargo test -p mesh-llm-protocol control_plane_messages_constants_are_stable --libcargo check -p mesh-llmcargo clippy -p mesh-llm-host-runtime --all-targets -- -D warningscargo clippy -p mesh-llm --all-targets -- -D warningscargo clippy -p mesh-llm-protocol --all-targets -- -D warningsjust buildSummary by CodeRabbit
New Features
Bug Fixes / Reliability
Documentation
Tests
Chores