feat: add extensible owned-node command system - #1002
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds typed owner-control scan refresh across protocol, client, server, REST, CLI, and documentation layers. It also reorganizes mesh runtime modules and adds connection, identity, admission, gossip, stage transport, artifact transfer, plugin, persistence, and validation implementations. ChangesOwner-control scan refresh
Mesh runtime
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant CLI
participant RuntimeAPI
participant OwnerControlClient
participant OwnerControlNode
participant InventoryScanner
CLI->>RuntimeAPI: POST /api/runtime/control/scan-refresh
RuntimeAPI->>OwnerControlClient: Connect with endpoint token
OwnerControlClient->>OwnerControlNode: Send typed refresh_inventory request
OwnerControlNode->>InventoryScanner: Coalesce inventory scan
InventoryScanner-->>OwnerControlNode: Return snapshot and disposition
OwnerControlNode-->>OwnerControlClient: Return snapshot and optional inventory
OwnerControlClient-->>RuntimeAPI: Map scan-refresh result
RuntimeAPI-->>CLI: Return human-readable or JSON output
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Contributor
|
This pull request is currently a draft. Reviews will not take place until the PR is marked as ready for review. |
ndizazzo
changed the title
ndizazzo
force-pushed
the
codex/owned-node-commands-plan
branch
from
July 18, 2026 04:20
5c66f88 to
11795d1
Compare
ndizazzo
marked this pull request as ready for review
July 18, 2026 04:20
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/mesh-llm-host-runtime/src/protocol/tests.rs (1)
1-1760: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftSplit this near-limit test module by responsibility.
At 1,760 lines, this new file is already approaching the 2,000-line limit. Move the owner-control, peer lifecycle, announcement, and config-roundtrip suites into named test submodules.
As per coding guidelines, “If a file is approaching that size, split it by responsibility into an owning module instead of adding more code to the oversized file.” <coding_guidelines>
🤖 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/tests.rs` around lines 1 - 1760, Split the oversized tests module by responsibility: move the owner-control tests, peer lifecycle tests, peer announcement/protocol conversion tests, and configuration round-trip tests into separate named test submodules. Keep shared fixtures and helpers such as make_valid_gossip_frame, make_test_peer_info, make_config_snapshot, and make_nested_mesh_config in a shared scope or expose them as needed, and leave only shared wiring or broadly applicable tests in the parent module.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/mesh/gossip.rs (1)
365-449: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winValidate the direct sender before applying any announcement.
When
direct_peer_requirements_validatedis false, sender-controlled ordering allows transitive peers to be applied before the direct announcement is checked. The direct branch also callsset_mesh_idbefore validation. A rejected peer can therefore seed mesh identity or peer state.Pre-locate and validate the remote peer’s direct announcement, then adopt its mesh ID and process the remaining announcements.
🤖 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/mesh/gossip.rs` around lines 365 - 449, Update apply_announced_peers to locate the announcement whose endpoint ID matches remote and, when direct_peer_requirements_validated is false, validate it with validate_direct_peer_requirements before processing any announcements. Only after successful validation should it adopt the announcement’s mesh ID and process the remaining entries, ensuring rejected senders cannot seed mesh identity or transitive peer state; preserve apply_announced_peer’s existing handling for already-validated direct peers.
🟠 Major comments (23)
crates/mesh-llm-host-runtime/src/mesh/owner_control/mod.rs-337-355 (1)
337-355: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound server response writes, including the timeout fallback.
If the peer stops reading, the original response write can consume the command deadline; the fallback then calls the same unbounded writer and may hang indefinitely. Apply a write timeout and close/reset the stream when it expires.
Also applies to: 862-869
🤖 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/mesh/owner_control/mod.rs` around lines 337 - 355, The owner-control response writers send_owner_control_envelope and send_owner_control_terminal_envelope must enforce a write timeout, including the terminal fallback path. Wrap the length-prefixed write with the command deadline, and when it expires close or reset the SendStream before returning the timeout error so neither the initial response nor the fallback can hang indefinitely.crates/mesh-client/src/client/control_plane.rs-383-386 (1)
383-386: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winLeave delivery margin between the server deadline and client timeout.
Both sides currently use 30 seconds, so a scan completing near the server deadline cannot reliably deliver its result or timeout envelope before the client reports a transport timeout.
crates/mesh-client/src/client/control_plane.rs#L383-L386: increase the response timeout above the complete server-side execution budget.crates/mesh-llm-host-runtime/src/mesh/owner_control/commands/mod.rs#L145-L145: retain or centralize the 30-second execution deadline while ensuring clients add transport margin.🤖 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-client/src/client/control_plane.rs` around lines 383 - 386, The client response timeout in send_unary_request must exceed the server’s 30-second execution deadline by a delivery margin; update the timeout used by the control-plane request in crates/mesh-client/src/client/control_plane.rs (lines 383-386) while retaining or centralizing the 30-second execution deadline in crates/mesh-llm-host-runtime/src/mesh/owner_control/commands/mod.rs (line 145), which requires no direct increase.crates/mesh-llm-host-runtime/src/mesh/identity_persistence.rs-3-15 (1)
3-15: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftUse the documented stable mesh-ID algorithm.
This produces a 64-bit
DefaultHashervalue, not the documented SHA-256 digest. Named meshes therefore have a different format, weaker collision resistance, and no explicit cross-version algorithm contract. Introduce a fixed digest plus compatibility handling for existing IDs.🤖 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/mesh/identity_persistence.rs` around lines 3 - 15, Update generate_mesh_id to compute named mesh IDs using the documented SHA-256 input format and return the full stable digest rather than a DefaultHasher value. Preserve unnamed mesh UUID persistence, and add compatibility handling so existing generated IDs remain recognized or resolvable while new named IDs use the fixed algorithm.crates/mesh-llm-host-runtime/src/mesh/stage_artifacts.rs-121-137 (1)
121-137: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winLocal stage-control waits have no deadline, so a stalled control loop can retain callers indefinitely. Apply the command-specific timeout consistently.
crates/mesh-llm-host-runtime/src/mesh/stage_artifacts.rs#L121-L137: bound inbound peer command execution.crates/mesh-llm-host-runtime/src/mesh/stage_transport.rs#L814-L822: bound local API command execution.🤖 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/mesh/stage_artifacts.rs` around lines 121 - 137, Apply the command-specific timeout to both stage-control execution paths: wrap the response wait in execute_stage_control_request in stage_artifacts.rs and the corresponding local API command wait in stage_transport.rs (lines 814-822) with the established timeout mechanism, returning the timeout error consistently while preserving existing send and response-drop errors.crates/mesh-llm-host-runtime/src/mesh/stage_transport.rs-966-1011 (1)
966-1011: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftMake stage-bridge creation atomic per key.
Two concurrent calls can both pass
contains_key, spawn listeners, and then overwrite oneJoinHandle; the losing bridge remains running but cannot be stopped through the map. Reserve the key under the lock before binding/spawning, and roll back the reservation on 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/mesh-llm-host-runtime/src/mesh/stage_transport.rs` around lines 966 - 1011, Make stage-bridge creation in the surrounding method atomic by reserving the computed key in stage_transport_bridges before binding the listener or spawning the task, rejecting an existing reservation. If listener binding or task setup fails, remove the reservation before returning the error; otherwise replace the reservation with the running JoinHandle without allowing concurrent calls to overwrite it.crates/mesh-llm-host-runtime/src/mesh/node_requirements.rs-218-243 (1)
218-243: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftRequirement-aware mesh state is published and read through independent locks, allowing partial snapshots. Replace these fields with one synchronized state object.
crates/mesh-llm-host-runtime/src/mesh/node_requirements.rs#L218-L243: commit mesh ID, policy/hash, signed policy, and bootstrap token atomically.crates/mesh-llm-host-runtime/src/mesh/node_identity.rs#L117-L139: read that atomic snapshot so requirement-aware setup cannot emit a legacy invite token.🤖 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/mesh/node_requirements.rs` around lines 218 - 243, The requirement-aware mesh fields currently use independent locks and can expose partial state. Introduce or reuse one synchronized state object, update install_requirement_aware_mesh_state in crates/mesh-llm-host-runtime/src/mesh/node_requirements.rs:218-243 to atomically commit mesh ID, policy/hash, signed policy, and bootstrap token, and update the corresponding read path in crates/mesh-llm-host-runtime/src/mesh/node_identity.rs:117-139 to consume one atomic snapshot and avoid emitting a legacy invite token.crates/mesh-llm-host-runtime/src/mesh/stage_artifacts.rs-783-795 (1)
783-795: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd an idle timeout to artifact-transfer writes.
A peer that stops reading can block
write_allindefinitely while retaining the file, stream, and task. Bound each write or the transfer loop with a write-idle timeout.🤖 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/mesh/stage_artifacts.rs` around lines 783 - 795, Update the artifact transfer loop around the write_all call to enforce a write-idle timeout, so a peer that stops reading cannot retain the file, stream, and task indefinitely. Wrap each send.write_all operation, or the transfer loop, with the existing timeout mechanism and propagate a contextual timeout error while preserving normal chunked transfer behavior.crates/mesh-llm-host-runtime/src/mesh/identity_persistence.rs-106-111 (1)
106-111: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftIdentity persistence fails open because filesystem errors are discarded. Propagate these failures so startup cannot continue with identity state that was never recorded.
crates/mesh-llm-host-runtime/src/mesh/identity_persistence.rs#L106-L111: require successful public-mode marker persistence before continuing.crates/mesh-llm-host-runtime/src/mesh/identity_persistence.rs#L25-L35: return an error instead of publishing an unpersisted random mesh ID.🤖 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/mesh/identity_persistence.rs` around lines 106 - 111, Update crates/mesh-llm-host-runtime/src/mesh/identity_persistence.rs:106-111 so mark_was_public propagates create_dir_all and write failures, requiring successful marker persistence before continuing; update crates/mesh-llm-host-runtime/src/mesh/identity_persistence.rs:25-35 so the identity-loading flow returns an error instead of publishing a random mesh ID when persistence fails.crates/mesh-llm-host-runtime/src/mesh/stage_transport.rs-1153-1155 (1)
1153-1155: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winTimeout the stream-kind read before accepting the next stream.
The accept loop waits here serially, so one admitted peer can open a bidirectional stream, send no type byte, and block every subsequent stage stream on that 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/mesh-llm-host-runtime/src/mesh/stage_transport.rs` around lines 1153 - 1155, Update the stream-kind read in the accept loop to apply the established stream-read timeout before awaiting recv.read_exact, and treat timeout or other read failure as StageStreamAccept::Continue. Ensure a peer that sends no type byte cannot block acceptance of subsequent stage streams.crates/mesh-llm-host-runtime/src/mesh/node_requirements.rs-69-105 (1)
69-105: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not silently rotate genesis policy after read or validation failures.
Any I/O, parse, or signature failure falls through to generating a new timestamped policy, changing the policy-derived mesh ID and partitioning the node from its existing mesh. Only
NotFoundshould create a policy; surface corruption and read failures.🤖 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/mesh/node_requirements.rs` around lines 69 - 105, Update load_or_create_signed_genesis_policy so only a missing policy file generates a new signed policy. Propagate filesystem read errors, and surface JSON parse or existing-policy validation failures—including verify, owner ID, requirements, and signing-key mismatches—instead of silently creating a timestamped replacement; preserve returning the valid existing policy unchanged.crates/mesh-llm-host-runtime/src/mesh/node_identity.rs-217-223 (1)
217-223: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAllow the valid cached-token fallback after signing fails.
Returning
Some(String::new())makes the caller return immediately, so its cached-token fallback is unreachable. ReturnNoneon failure.Proposed fix
Err(error) => { tracing::warn!( error = %error, "failed to sign requirement-aware bootstrap token; refusing to emit legacy invite token" ); - Some(String::new()) + None }🤖 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/mesh/node_identity.rs` around lines 217 - 223, Update the signing failure branch in the requirement-aware bootstrap token flow to return None instead of Some(String::new()), allowing the caller’s cached-token fallback to execute. Preserve the existing warning log and successful signing behavior.crates/mesh-llm-host-runtime/src/mesh/node.rs-1854-1877 (1)
1854-1877: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse saturating age calculations for gossiped timestamps.
merge_remote_demandaccepts peer-providedlast_activevalues. A future timestamp makesnow - d.last_activeunderflow, panicking with overflow checks or becoming a huge age and incorrectly dropping demand.Proposed fix
- demand.retain(|model, d| pinned.contains(model) || (now - d.last_active) < DEMAND_TTL_SECS); + demand.retain(|model, d| { + pinned.contains(model) + || now.saturating_sub(d.last_active) < DEMAND_TTL_SECS + }); ... - .filter(|(model, d)| pinned.contains(model) || (now - d.last_active) < DEMAND_TTL_SECS) + .filter(|(model, d)| { + pinned.contains(model) + || now.saturating_sub(d.last_active) < DEMAND_TTL_SECS + })Also applies to: 1879-1900
🤖 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/mesh/node.rs` around lines 1854 - 1877, Update the demand age calculation in gc_demand, and any corresponding filtering logic in the adjacent demand code, to use saturating subtraction when comparing now against peer-provided last_active timestamps. Ensure future timestamps produce age zero rather than underflowing, while preserving the existing TTL and pinned-model retention behavior.crates/mesh-llm-host-runtime/src/mesh/connections.rs-1469-1484 (1)
1469-1484: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftPublish replacement connections only after gossip succeeds.
Both paths install and dispatch a connection before its gossip exchange is validated, with no rollback on failure. This leaves unadmitted connections tracked or replaces a healthy connection with a failed reverse dial.
crates/mesh-llm-host-runtime/src/mesh/connections.rs#L1469-L1484: on gossip failure, remove and close the inserted connection, or delay insertion/dispatch until gossip succeeds.crates/mesh-llm-host-runtime/src/mesh/direct_path.rs#L429-L461: retain the previous connection until reverse-dial gossip succeeds; close the new connection and preserve/restore the old one on 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/mesh-llm-host-runtime/src/mesh/connections.rs` around lines 1469 - 1484, Ensure connection publication and stream dispatch occur only after initiate_gossip succeeds: in crates/mesh-llm-host-runtime/src/mesh/connections.rs lines 1469-1484, either defer insertion and dispatch until gossip completes or remove and close the inserted connection on failure. In crates/mesh-llm-host-runtime/src/mesh/direct_path.rs lines 429-461, retain the existing connection while reverse-dial gossip runs, close the replacement on failure, and preserve or restore the previous connection; publish the replacement only after successful gossip.crates/mesh-llm-host-runtime/src/mesh/node.rs-57-76 (1)
57-76: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winCorrelate and validate STUN responses before advertising their address.
The current flow accepts any sufficiently shaped UDP packet received on the socket. It does not verify the sender, Binding Success response type, declared length, magic cookie, or transaction ID.
crates/mesh-llm-host-runtime/src/mesh/node.rs#L57-L76: retain therecv_fromsource and reject responses not originating from the probed server.crates/mesh-llm-host-runtime/src/mesh/connections.rs#L209-L228: validate the STUN header and request transaction ID before parsing mapped-address attributes.🤖 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/mesh/node.rs` around lines 57 - 76, Validate STUN responses across probe_stun_server and parse_stun_public_addr: in crates/mesh-llm-host-runtime/src/mesh/node.rs:57-76, retain recv_from’s source and reject packets not from the resolved server; in crates/mesh-llm-host-runtime/src/mesh/connections.rs:209-228, require a valid STUN Binding Success header, declared message length, magic cookie, and matching request transaction ID before parsing mapped-address attributes.crates/mesh-llm-host-runtime/src/mesh/node.rs-923-1114 (1)
923-1114: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftSplit this module before it crosses the crate’s file-size limit.
This new file is already 1,934 lines. Move startup construction and one or more state domains—capture, metrics, model state, or control-listener lifecycle—into owning modules before further growth.
As per coding guidelines, “Do not add Rust source files over 2,000 lines. If a file is approaching that size, split it by responsibility into an owning module.”
🤖 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/mesh/node.rs` around lines 923 - 1114, Split the oversized node module before adding further code: extract the startup construction centered on Node::start and at least one cohesive state domain such as capture, metrics, model state, or control-listener lifecycle into an owning module. Move its related fields, initialization, and methods together, update imports and call sites, and keep Node::start behavior unchanged while keeping the original file below the 2,000-line limit.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/mesh/connections.rs-317-330 (1)
317-330: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReturn an error for malformed relay URLs instead of panicking.
RelayConfig.urlsis configuration input; one invalid URL reachesexpectand aborts node startup. Make this builder returnResult<RelayMap>and propagate URL-specific parse context.🤖 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/mesh/connections.rs` around lines 317 - 330, Update relay_map_from_urls to return Result<iroh::RelayMap> instead of panicking on malformed configuration. Replace the expect in URL parsing with error propagation that includes the invalid URL as context, and collect or propagate the iterator’s parse errors before constructing the RelayMap with iroh::RelayMap::from_iter.crates/mesh-llm-host-runtime/src/mesh/plugin_mesh.rs-204-217 (1)
204-217: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound plugin-frame reads with deadlines.
An admitted peer can send a length prefix and indefinitely stall the body, retaining one spawned task and QUIC stream per request. Apply timeouts to both the prefix and payload reads for channel and bulk frames.
Also applies to: 284-297
🤖 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/mesh/plugin_mesh.rs` around lines 204 - 217, Update handle_plugin_channel_stream and the corresponding bulk-frame read path to apply bounded deadlines to both recv.read_exact calls: the 4-byte length prefix and the allocated payload body. Reuse the existing timeout/deadline configuration where available, and propagate timeout errors so stalled admitted peers cannot retain the spawned task and QUIC stream indefinitely.crates/mesh-llm-host-runtime/src/mesh/plugin_mesh.rs-247-279 (1)
247-279: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftRoute targeted plugin messages beyond one intermediary.
The forwarding path only checks direct connections. In a chain such as A→B→C→D, a message from A targeting D stops at B. Resolve the target endpoint and use the admission-aware on-demand connection path, or implement bounded next-hop forwarding.
Also applies to: 327-356
🤖 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/mesh/plugin_mesh.rs` around lines 247 - 279, Update the targeted-message forwarding logic in the plugin frame handler so messages continue beyond a single intermediary instead of being sent only when the target is already in state.connections. Resolve message.target_peer_id and use the existing admission-aware on-demand connection mechanism to reach the target through multi-hop paths such as A→B→C→D, while preserving local delivery and avoiding broadcast flooding.crates/mesh-llm-host-runtime/src/mesh/peer_state.rs-1131-1173 (1)
1131-1173: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not reuse a connection before local admission completes.
Lines 1133-1134 return any tracked connection immediately, although unadmitted peers can already exist in
state.connections. This skips the gossip verification performed by the fallback branch and lets local trust-policy rejection be bypassed for outbound mesh streams. Reuse the connection only whenPeerInfo::is_admitted()is true; otherwise complete gossip first.🤖 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/mesh/peer_state.rs` around lines 1131 - 1173, The connection lookup in connection_to_peer must only return a tracked connection when its associated PeerInfo::is_admitted() is true. For an existing but unadmitted connection, retain the connection/address flow needed to run initiate_gossip_inner before returning it, ensuring local admission completes before reuse and trust-policy rejection cannot be bypassed.crates/mesh-llm-host-runtime/src/mesh/peer_state.rs-197-209 (1)
197-209: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove the unexplained dead-code suppression.
Delete these unused helpers, use them from production code, or restrict them to tests instead of suppressing the warnings.
As per coding guidelines, “Do not silence Rust warnings with
#[allow(...)]unless there is a clear reason and the developer has asked for that tradeoff.”🤖 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/mesh/peer_state.rs` around lines 197 - 209, Remove the #[allow(dead_code)] suppression from descriptor_from_model_path and descriptor_from_identity, then either delete these unused helpers, integrate them into production code, or limit their definitions to test-only code so they no longer require warning suppression.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/mesh/tests/protocol_compat.rs-185-267 (1)
185-267: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftTest production protocol and state-transition boundaries rather than reproducing their expected behavior. The current surrogate structs and manual map/timestamp mutations can remain green after the real compatibility or lifecycle implementation regresses.
crates/mesh-llm-host-runtime/src/mesh/tests/protocol_compat.rs#L185-L267: deserialize legacy payloads through the production/0adapter.crates/mesh-llm-host-runtime/src/mesh/tests/protocol_compat.rs#L395-L433: validate production field defaults and aliases rather than local Serde definitions.crates/mesh-llm-host-runtime/src/mesh/tests/protocol_frames.rs#L321-L332: invoke the production transitive-peer update path.crates/mesh-llm-host-runtime/src/mesh/tests/protocol_frames.rs#L414-L450: decode and assert rejection for every listed stream type.crates/mesh-llm-host-runtime/src/mesh/tests/protocol_frames.rs#L825-L839: exercise the actual dead-peer reconnect gate.crates/mesh-llm-host-runtime/src/mesh/tests/protocol_frames.rs#L990-L1015: drive the real reconnect-gossip failure cleanup.As per coding guidelines, mixed-version
/1protobuf and/0legacy JSON operation must remain compatible.🤖 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/mesh/tests/protocol_compat.rs` around lines 185 - 267, Replace the surrogate compatibility structs in crates/mesh-llm-host-runtime/src/mesh/tests/protocol_compat.rs:185-267 with legacy-payload deserialization through the production /0 adapter, preserving mixed-version /1 protobuf and /0 JSON compatibility; likewise validate production defaults and aliases at crates/mesh-llm-host-runtime/src/mesh/tests/protocol_compat.rs:395-433. At crates/mesh-llm-host-runtime/src/mesh/tests/protocol_frames.rs:321-332, invoke the production transitive-peer update path; at :414-450, decode and assert rejection for every listed stream type; at :825-839, exercise the actual dead-peer reconnect gate; and at :990-1015, drive the real reconnect-gossip failure cleanup instead of manually reproducing state transitions.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/mesh/peer_state.rs-745-757 (1)
745-757: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve the Hugging Face revision in public model IDs.
Line 756 always passes
None, sorepo@revisionidentities collapse together and revision-qualified routing requests cannot match.Proposed fix
- model_ref::format_model_ref(repo, None, selector.as_deref()) + model_ref::format_model_ref( + repo, + identity.revision.as_deref(), + selector.as_deref(), + )🤖 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/mesh/peer_state.rs` around lines 745 - 757, Update public_model_id_from_identity for the ModelSourceKind::HuggingFace branch to pass identity.revision to model_ref::format_model_ref instead of always passing None, preserving repo@revision in the generated public model ID while leaving artifact selector handling unchanged.crates/mesh-llm-host-runtime/src/mesh/peer_state.rs-3-34 (1)
3-34: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSelect exactly one primary model.
When
primary_model_nameis not first, Line 13 marks both index zero and the matching model as primary. Select the matching index, falling back to zero only when no match exists.Proposed fix
+ let primary_idx = serving_models + .iter() + .position(|name| name == primary_model_name) + .unwrap_or(0); serving_models .iter() .enumerate() .map(|(idx, model_name)| { - let identity = if idx == 0 || model_name == primary_model_name { + let identity = if idx == primary_idx {🤖 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/mesh/peer_state.rs` around lines 3 - 34, Update infer_remote_served_descriptors to compute one primary index by finding the entry matching primary_model_name, falling back to index zero when no match exists. Use only that index in the identity selection condition so exactly one ServedModelDescriptor has is_primary set to true, while preserving the existing primary identity and unknown-identity handling.
🟡 Minor comments (3)
crates/mesh-llm-host-runtime/src/runtime_data/mod.rs-1079-1081 (1)
1079-1081: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winBound waiter polling so regressions fail instead of hanging CI.
Each loop can run forever if waiter registration breaks. Wrap the polling phase in a short
tokio::time::timeoutor a shared bounded helper.Also applies to: 1094-1096, 1154-1156, 1168-1170
🤖 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/runtime_data/mod.rs` around lines 1079 - 1081, Bound each waiter-count polling loop around inventory_scan_waiter_count with a short tokio::time::timeout or existing shared bounded helper, including the additional loops at the referenced locations. Preserve the current yield-and-wait behavior while ensuring registration regressions terminate with a timeout failure instead of hanging indefinitely.crates/mesh-llm-host-runtime/src/mesh/mod.rs-113-116 (1)
113-116: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove or explicitly justify the warning suppressions.
Narrow/remove unused imports and re-exports instead of masking them; if any are temporary compatibility shims, document that reason locally.
As per coding guidelines: “Do not silence Rust warnings with
#[allow(...)]unless there is a clear reason and the developer has asked for that tradeoff.”Also applies to: 121-122, 128-129, 136-137, 147-148, 153-157
🤖 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/mesh/mod.rs` around lines 113 - 116, Remove the #[allow(unused_imports)] attributes around the imports and re-exports in the mesh module, including the additional locations noted in the review, then narrow each import to only the symbols actually used. If any import must remain as a compatibility shim, retain it only with a local comment documenting the specific reason.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/mesh/tests/owner_control.rs-71-109 (1)
71-109: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDocument the env-mutation safety invariant
EnvVarGuardis only used by a#[serial]test, but the unsafestd::envcalls still need an explicitSAFETY:explanation that no concurrent code in this process reads or mutates env while the guard is live.🤖 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/mesh/tests/owner_control.rs` around lines 71 - 109, Add explicit SAFETY comments to every unsafe environment operation in EnvVarGuard, documenting that its callers run under #[serial] and that no concurrent code reads or mutates the environment while the guard is live. Replace the existing TODO comments in set, set_str, unset, and Drop without changing the guard’s behavior.
🧹 Nitpick comments (4)
crates/mesh-client/src/lib.rs (1)
19-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the new result type in its owning module.
Remove this crate-root re-export and use
mesh_client::client::OwnerControlScanRefreshResultinstead.As per coding guidelines, “Minimize crate-root re-exports… new code should prefer importing from the owning module directly.”
Proposed change
- MeshClient, Model, OwnerControlClient, OwnerControlRemoteError, OwnerControlScanRefreshResult, + MeshClient, Model, OwnerControlClient, OwnerControlRemoteError,🤖 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-client/src/lib.rs` around lines 19 - 20, Remove OwnerControlScanRefreshResult from the crate-root export list in lib.rs. Update consumers to import it from the owning mesh_client::client module directly, leaving the other exports unchanged.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/runtime_data/inventory.rs (1)
11-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the test-only error variant conditional instead of suppressing warnings.
Use
#[cfg(test)]on both the variant and itsDisplayarm.As per coding guidelines, “Do not silence Rust warnings with
#[allow(...)]unless there is a clear reason and the developer has asked for that tradeoff.”Proposed change
- #[allow(dead_code)] + #[cfg(test)] LoaderFailed(String), ... + #[cfg(test)] Self::LoaderFailed(message) => { write!(formatter, "local inventory scan failed: {message}") }Also applies to: 19-22
🤖 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/runtime_data/inventory.rs` around lines 11 - 12, Update the LoaderFailed variant in the relevant error enum to use #[cfg(test)] instead of #[allow(dead_code)], and apply the same #[cfg(test)] condition to its corresponding Display match arm. Keep the variant and arm consistently available only during tests.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/runtime_data/mod.rs (1)
1052-1188: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the inventory tests out of this 1,700-plus-line module.
These additions move
runtime_data/mod.rscloser to the 2,000-line limit and already form a cohesive inventory-test unit.As per coding guidelines, “If a file is approaching [2,000 lines], split it by responsibility into an owning module instead of adding more code to the oversized file.”
🤖 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/runtime_data/mod.rs` around lines 1052 - 1188, Move the inventory-focused tests runtime_data_inventory_single_flight_scan_coalesces and runtime_data_inventory_scan_panic_fans_out_error_and_preserves_snapshot out of runtime_data/mod.rs into a dedicated inventory test module owned by the runtime_data implementation. Update module declarations and imports so both tests retain access to RuntimeDataCollector, inventory result types, and synchronization utilities, while preserving their existing behavior and assertions.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/mesh/tests/admission.rs (1)
688-754: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit this 1,722-line test module by responsibility.
Move owner-control, direct-proof, bootstrap-token, and RTT tests into dedicated submodules before this file crosses the 2,000-line limit.
As per coding guidelines, “If a file is approaching that size, split it by responsibility into an owning module instead of adding more code to the oversized file.”
Also applies to: 997-1077, 1611-1700
🤖 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/mesh/tests/admission.rs` around lines 688 - 754, Split the oversized admission test module by responsibility before adding further tests: move owner-control, direct-proof, bootstrap-token, and RTT tests into dedicated submodules, including the tests represented by assert_mesh_requirements_outbound_admits_compliant_peer_after_requirements_pass and assert_mesh_requirements_inbound_rejects_before_topology_announcement where applicable. Preserve shared helpers and test behavior, and update module declarations/imports so all tests continue compiling and running.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@crates/mesh-llm-host-runtime/src/mesh/gossip.rs`:
- Around line 365-449: Update apply_announced_peers to locate the announcement
whose endpoint ID matches remote and, when direct_peer_requirements_validated is
false, validate it with validate_direct_peer_requirements before processing any
announcements. Only after successful validation should it adopt the
announcement’s mesh ID and process the remaining entries, ensuring rejected
senders cannot seed mesh identity or transitive peer state; preserve
apply_announced_peer’s existing handling for already-validated direct peers.
In `@crates/mesh-llm-host-runtime/src/protocol/tests.rs`:
- Around line 1-1760: Split the oversized tests module by responsibility: move
the owner-control tests, peer lifecycle tests, peer announcement/protocol
conversion tests, and configuration round-trip tests into separate named test
submodules. Keep shared fixtures and helpers such as make_valid_gossip_frame,
make_test_peer_info, make_config_snapshot, and make_nested_mesh_config in a
shared scope or expose them as needed, and leave only shared wiring or broadly
applicable tests in the parent module.
---
Major comments:
In `@crates/mesh-client/src/client/control_plane.rs`:
- Around line 383-386: The client response timeout in send_unary_request must
exceed the server’s 30-second execution deadline by a delivery margin; update
the timeout used by the control-plane request in
crates/mesh-client/src/client/control_plane.rs (lines 383-386) while retaining
or centralizing the 30-second execution deadline in
crates/mesh-llm-host-runtime/src/mesh/owner_control/commands/mod.rs (line 145),
which requires no direct increase.
In `@crates/mesh-llm-host-runtime/src/mesh/connections.rs`:
- Around line 1469-1484: Ensure connection publication and stream dispatch occur
only after initiate_gossip succeeds: in
crates/mesh-llm-host-runtime/src/mesh/connections.rs lines 1469-1484, either
defer insertion and dispatch until gossip completes or remove and close the
inserted connection on failure. In
crates/mesh-llm-host-runtime/src/mesh/direct_path.rs lines 429-461, retain the
existing connection while reverse-dial gossip runs, close the replacement on
failure, and preserve or restore the previous connection; publish the
replacement only after successful gossip.
- Around line 317-330: Update relay_map_from_urls to return
Result<iroh::RelayMap> instead of panicking on malformed configuration. Replace
the expect in URL parsing with error propagation that includes the invalid URL
as context, and collect or propagate the iterator’s parse errors before
constructing the RelayMap with iroh::RelayMap::from_iter.
In `@crates/mesh-llm-host-runtime/src/mesh/identity_persistence.rs`:
- Around line 3-15: Update generate_mesh_id to compute named mesh IDs using the
documented SHA-256 input format and return the full stable digest rather than a
DefaultHasher value. Preserve unnamed mesh UUID persistence, and add
compatibility handling so existing generated IDs remain recognized or resolvable
while new named IDs use the fixed algorithm.
- Around line 106-111: Update
crates/mesh-llm-host-runtime/src/mesh/identity_persistence.rs:106-111 so
mark_was_public propagates create_dir_all and write failures, requiring
successful marker persistence before continuing; update
crates/mesh-llm-host-runtime/src/mesh/identity_persistence.rs:25-35 so the
identity-loading flow returns an error instead of publishing a random mesh ID
when persistence fails.
In `@crates/mesh-llm-host-runtime/src/mesh/node_identity.rs`:
- Around line 217-223: Update the signing failure branch in the
requirement-aware bootstrap token flow to return None instead of
Some(String::new()), allowing the caller’s cached-token fallback to execute.
Preserve the existing warning log and successful signing behavior.
In `@crates/mesh-llm-host-runtime/src/mesh/node_requirements.rs`:
- Around line 218-243: The requirement-aware mesh fields currently use
independent locks and can expose partial state. Introduce or reuse one
synchronized state object, update install_requirement_aware_mesh_state in
crates/mesh-llm-host-runtime/src/mesh/node_requirements.rs:218-243 to atomically
commit mesh ID, policy/hash, signed policy, and bootstrap token, and update the
corresponding read path in
crates/mesh-llm-host-runtime/src/mesh/node_identity.rs:117-139 to consume one
atomic snapshot and avoid emitting a legacy invite token.
- Around line 69-105: Update load_or_create_signed_genesis_policy so only a
missing policy file generates a new signed policy. Propagate filesystem read
errors, and surface JSON parse or existing-policy validation failures—including
verify, owner ID, requirements, and signing-key mismatches—instead of silently
creating a timestamped replacement; preserve returning the valid existing policy
unchanged.
In `@crates/mesh-llm-host-runtime/src/mesh/node.rs`:
- Around line 1854-1877: Update the demand age calculation in gc_demand, and any
corresponding filtering logic in the adjacent demand code, to use saturating
subtraction when comparing now against peer-provided last_active timestamps.
Ensure future timestamps produce age zero rather than underflowing, while
preserving the existing TTL and pinned-model retention behavior.
- Around line 57-76: Validate STUN responses across probe_stun_server and
parse_stun_public_addr: in crates/mesh-llm-host-runtime/src/mesh/node.rs:57-76,
retain recv_from’s source and reject packets not from the resolved server; in
crates/mesh-llm-host-runtime/src/mesh/connections.rs:209-228, require a valid
STUN Binding Success header, declared message length, magic cookie, and matching
request transaction ID before parsing mapped-address attributes.
- Around line 923-1114: Split the oversized node module before adding further
code: extract the startup construction centered on Node::start and at least one
cohesive state domain such as capture, metrics, model state, or control-listener
lifecycle into an owning module. Move its related fields, initialization, and
methods together, update imports and call sites, and keep Node::start behavior
unchanged while keeping the original file below the 2,000-line limit.
In `@crates/mesh-llm-host-runtime/src/mesh/owner_control/mod.rs`:
- Around line 337-355: The owner-control response writers
send_owner_control_envelope and send_owner_control_terminal_envelope must
enforce a write timeout, including the terminal fallback path. Wrap the
length-prefixed write with the command deadline, and when it expires close or
reset the SendStream before returning the timeout error so neither the initial
response nor the fallback can hang indefinitely.
In `@crates/mesh-llm-host-runtime/src/mesh/peer_state.rs`:
- Around line 1131-1173: The connection lookup in connection_to_peer must only
return a tracked connection when its associated PeerInfo::is_admitted() is true.
For an existing but unadmitted connection, retain the connection/address flow
needed to run initiate_gossip_inner before returning it, ensuring local
admission completes before reuse and trust-policy rejection cannot be bypassed.
- Around line 197-209: Remove the #[allow(dead_code)] suppression from
descriptor_from_model_path and descriptor_from_identity, then either delete
these unused helpers, integrate them into production code, or limit their
definitions to test-only code so they no longer require warning suppression.
- Around line 745-757: Update public_model_id_from_identity for the
ModelSourceKind::HuggingFace branch to pass identity.revision to
model_ref::format_model_ref instead of always passing None, preserving
repo@revision in the generated public model ID while leaving artifact selector
handling unchanged.
- Around line 3-34: Update infer_remote_served_descriptors to compute one
primary index by finding the entry matching primary_model_name, falling back to
index zero when no match exists. Use only that index in the identity selection
condition so exactly one ServedModelDescriptor has is_primary set to true, while
preserving the existing primary identity and unknown-identity handling.
In `@crates/mesh-llm-host-runtime/src/mesh/plugin_mesh.rs`:
- Around line 204-217: Update handle_plugin_channel_stream and the corresponding
bulk-frame read path to apply bounded deadlines to both recv.read_exact calls:
the 4-byte length prefix and the allocated payload body. Reuse the existing
timeout/deadline configuration where available, and propagate timeout errors so
stalled admitted peers cannot retain the spawned task and QUIC stream
indefinitely.
- Around line 247-279: Update the targeted-message forwarding logic in the
plugin frame handler so messages continue beyond a single intermediary instead
of being sent only when the target is already in state.connections. Resolve
message.target_peer_id and use the existing admission-aware on-demand connection
mechanism to reach the target through multi-hop paths such as A→B→C→D, while
preserving local delivery and avoiding broadcast flooding.
In `@crates/mesh-llm-host-runtime/src/mesh/stage_artifacts.rs`:
- Around line 121-137: Apply the command-specific timeout to both stage-control
execution paths: wrap the response wait in execute_stage_control_request in
stage_artifacts.rs and the corresponding local API command wait in
stage_transport.rs (lines 814-822) with the established timeout mechanism,
returning the timeout error consistently while preserving existing send and
response-drop errors.
- Around line 783-795: Update the artifact transfer loop around the write_all
call to enforce a write-idle timeout, so a peer that stops reading cannot retain
the file, stream, and task indefinitely. Wrap each send.write_all operation, or
the transfer loop, with the existing timeout mechanism and propagate a
contextual timeout error while preserving normal chunked transfer behavior.
In `@crates/mesh-llm-host-runtime/src/mesh/stage_transport.rs`:
- Around line 966-1011: Make stage-bridge creation in the surrounding method
atomic by reserving the computed key in stage_transport_bridges before binding
the listener or spawning the task, rejecting an existing reservation. If
listener binding or task setup fails, remove the reservation before returning
the error; otherwise replace the reservation with the running JoinHandle without
allowing concurrent calls to overwrite it.
- Around line 1153-1155: Update the stream-kind read in the accept loop to apply
the established stream-read timeout before awaiting recv.read_exact, and treat
timeout or other read failure as StageStreamAccept::Continue. Ensure a peer that
sends no type byte cannot block acceptance of subsequent stage streams.
In `@crates/mesh-llm-host-runtime/src/mesh/tests/protocol_compat.rs`:
- Around line 185-267: Replace the surrogate compatibility structs in
crates/mesh-llm-host-runtime/src/mesh/tests/protocol_compat.rs:185-267 with
legacy-payload deserialization through the production /0 adapter, preserving
mixed-version /1 protobuf and /0 JSON compatibility; likewise validate
production defaults and aliases at
crates/mesh-llm-host-runtime/src/mesh/tests/protocol_compat.rs:395-433. At
crates/mesh-llm-host-runtime/src/mesh/tests/protocol_frames.rs:321-332, invoke
the production transitive-peer update path; at :414-450, decode and assert
rejection for every listed stream type; at :825-839, exercise the actual
dead-peer reconnect gate; and at :990-1015, drive the real reconnect-gossip
failure cleanup instead of manually reproducing state transitions.
---
Minor comments:
In `@crates/mesh-llm-host-runtime/src/mesh/mod.rs`:
- Around line 113-116: Remove the #[allow(unused_imports)] attributes around the
imports and re-exports in the mesh module, including the additional locations
noted in the review, then narrow each import to only the symbols actually used.
If any import must remain as a compatibility shim, retain it only with a local
comment documenting the specific reason.
In `@crates/mesh-llm-host-runtime/src/mesh/tests/owner_control.rs`:
- Around line 71-109: Add explicit SAFETY comments to every unsafe environment
operation in EnvVarGuard, documenting that its callers run under #[serial] and
that no concurrent code reads or mutates the environment while the guard is
live. Replace the existing TODO comments in set, set_str, unset, and Drop
without changing the guard’s behavior.
In `@crates/mesh-llm-host-runtime/src/runtime_data/mod.rs`:
- Around line 1079-1081: Bound each waiter-count polling loop around
inventory_scan_waiter_count with a short tokio::time::timeout or existing shared
bounded helper, including the additional loops at the referenced locations.
Preserve the current yield-and-wait behavior while ensuring registration
regressions terminate with a timeout failure instead of hanging indefinitely.
---
Nitpick comments:
In `@crates/mesh-client/src/lib.rs`:
- Around line 19-20: Remove OwnerControlScanRefreshResult from the crate-root
export list in lib.rs. Update consumers to import it from the owning
mesh_client::client module directly, leaving the other exports unchanged.
In `@crates/mesh-llm-host-runtime/src/mesh/tests/admission.rs`:
- Around line 688-754: Split the oversized admission test module by
responsibility before adding further tests: move owner-control, direct-proof,
bootstrap-token, and RTT tests into dedicated submodules, including the tests
represented by
assert_mesh_requirements_outbound_admits_compliant_peer_after_requirements_pass
and assert_mesh_requirements_inbound_rejects_before_topology_announcement where
applicable. Preserve shared helpers and test behavior, and update module
declarations/imports so all tests continue compiling and running.
In `@crates/mesh-llm-host-runtime/src/runtime_data/inventory.rs`:
- Around line 11-12: Update the LoaderFailed variant in the relevant error enum
to use #[cfg(test)] instead of #[allow(dead_code)], and apply the same
#[cfg(test)] condition to its corresponding Display match arm. Keep the variant
and arm consistently available only during tests.
In `@crates/mesh-llm-host-runtime/src/runtime_data/mod.rs`:
- Around line 1052-1188: Move the inventory-focused tests
runtime_data_inventory_single_flight_scan_coalesces and
runtime_data_inventory_scan_panic_fans_out_error_and_preserves_snapshot out of
runtime_data/mod.rs into a dedicated inventory test module owned by the
runtime_data implementation. Update module declarations and imports so both
tests retain access to RuntimeDataCollector, inventory result types, and
synchronization utilities, while preserving their existing behavior and
assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5fd24668-33eb-4e9f-b5c4-0a97ca0434d6
📒 Files selected for processing (55)
crates/mesh-client/src/client/control_plane.rscrates/mesh-client/src/client/mod.rscrates/mesh-client/src/lib.rscrates/mesh-client/tests/control_plane_client.rscrates/mesh-llm-cli/src/runtime.rscrates/mesh-llm-host-runtime/src/api/mod.rscrates/mesh-llm-host-runtime/src/api/routes/mod.rscrates/mesh-llm-host-runtime/src/api/routes/runtime.rscrates/mesh-llm-host-runtime/src/mesh/artifact_transfer_io.rscrates/mesh-llm-host-runtime/src/mesh/connections.rscrates/mesh-llm-host-runtime/src/mesh/direct_path.rscrates/mesh-llm-host-runtime/src/mesh/gossip.rscrates/mesh-llm-host-runtime/src/mesh/heartbeat.rscrates/mesh-llm-host-runtime/src/mesh/identity_persistence.rscrates/mesh-llm-host-runtime/src/mesh/lan_bootstrap.rscrates/mesh-llm-host-runtime/src/mesh/mod.rscrates/mesh-llm-host-runtime/src/mesh/node.rscrates/mesh-llm-host-runtime/src/mesh/node_identity.rscrates/mesh-llm-host-runtime/src/mesh/node_requirements.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/commands/mod.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/commands/scan_refresh.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/mod.rscrates/mesh-llm-host-runtime/src/mesh/peer_state.rscrates/mesh-llm-host-runtime/src/mesh/plugin_mesh.rscrates/mesh-llm-host-runtime/src/mesh/plugin_streams.rscrates/mesh-llm-host-runtime/src/mesh/public_identity_tests.rscrates/mesh-llm-host-runtime/src/mesh/requirements.rscrates/mesh-llm-host-runtime/src/mesh/stage_artifacts.rscrates/mesh-llm-host-runtime/src/mesh/stage_transport.rscrates/mesh-llm-host-runtime/src/mesh/tests.rscrates/mesh-llm-host-runtime/src/mesh/tests/admission.rscrates/mesh-llm-host-runtime/src/mesh/tests/connections.rscrates/mesh-llm-host-runtime/src/mesh/tests/gossip.rscrates/mesh-llm-host-runtime/src/mesh/tests/owner_control.rscrates/mesh-llm-host-runtime/src/mesh/tests/peer_state.rscrates/mesh-llm-host-runtime/src/mesh/tests/protocol_compat.rscrates/mesh-llm-host-runtime/src/mesh/tests/protocol_frames.rscrates/mesh-llm-host-runtime/src/mesh/tests/requirements.rscrates/mesh-llm-host-runtime/src/mesh/tests/stage_transport.rscrates/mesh-llm-host-runtime/src/protocol/mod.rscrates/mesh-llm-host-runtime/src/protocol/tests.rscrates/mesh-llm-host-runtime/src/runtime_data/collector.rscrates/mesh-llm-host-runtime/src/runtime_data/inventory.rscrates/mesh-llm-host-runtime/src/runtime_data/mod.rscrates/mesh-llm-protocol/proto/node.protocrates/mesh-llm-protocol/src/proto/node.rscrates/mesh-llm-protocol/src/protocol/mod.rscrates/mesh-llm/src/commands/runtime.rscrates/mesh-llm/tests/owned_node_commands_cli.rsdocs/USAGE.mddocs/design/TESTING.mddocs/design/message_protocol.mdscripts/qa-control-plane-mixed-version.shwebsite/src/docs/pages/CLI.mdwebsite/src/docs/pages/api-reference.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/mesh-llm-host-runtime/src/mesh/peer_state.rs (1)
774-872: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftSplit
peer_state.rsbefore it reaches the file-size limit.At roughly 1,631 lines, this module now combines model identity, peer domain types, connection reservations, routing, stage connectivity, and lifecycle ingestion. Move these responsibilities into owning submodules.
As per coding guidelines, “If a file is approaching [2,000 lines], split it by responsibility into an owning module instead of adding more code to the oversized file.”
🤖 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/mesh/peer_state.rs` around lines 774 - 872, Split the responsibilities currently concentrated in `peer_state.rs` into owning submodules before adding further functionality: keep model identity and peer domain types together, move connection reservation types and methods such as `PendingConnectionHandshake`, `PendingConnectionWaiter`, and `MeshState::pending_connection_is_active` into a connection-reservation module, and separate routing, stage connectivity, and lifecycle-ingestion logic similarly. Update module declarations and visibility/imports so existing callers retain the same behavior and APIs.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/mesh/node_requirements.rs (1)
233-275: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake requirement-policy transitions atomic.
Lines 241-259 check the current mesh and publish under separate lock acquisitions, so concurrent joins can both observe no policy and install conflicting states. Similarly, Lines 304-325 verify against snapshot A but may attach its signed policy to a newly installed snapshot B.
Perform compare-and-set and signed-policy enrichment while holding
requirement_mesh_state; verify the mesh ID, policy hash, and policy still match before mutation.Also applies to: 299-325
🤖 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/mesh/node_requirements.rs` around lines 233 - 275, Make requirement-policy transitions atomic in install_requirement_aware_mesh_state and the signed-policy enrichment flow around the corresponding snapshot validation. Hold requirement_mesh_state for the entire compare-and-set operation, verify the existing mesh ID, policy hash, and policy still match the requested values, then install or enrich the state while retaining the guard; do not validate against a released snapshot before mutation.crates/mesh-llm-host-runtime/src/mesh/connections.rs (1)
800-823: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound the stream-type read before continuing dispatch.
Line 821 waits indefinitely for the discriminator. Because the dispatcher processes streams sequentially, a peer can open one bi-stream without sending this byte and block all subsequent gossip/control streams on the connection. Apply a read deadline and reset the stalled stream.
🤖 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/mesh/connections.rs` around lines 800 - 823, Update accept_mesh_stream’s discriminator read after type_buf is created to use a bounded read deadline; when the deadline expires or the read fails, reset the stalled receive stream before returning Err(()), while preserving normal dispatch for successfully read stream types.
🧹 Nitpick comments (1)
crates/mesh-llm-host-runtime/src/network/openai/transport.rs (1)
5122-5136: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest request routing with the new revision-qualified ID.
The test verifies
/v1/modelsoutput but not that this exactrepo@revision:selectorID rewrites to the internal model name. Add an alias-rewrite regression test so every advertised ID is demonstrably routable.🤖 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/network/openai/transport.rs` around lines 5122 - 5136, Extend the regression coverage around models_list_uses_public_huggingface_model_ref_ids to pass the advertised repo@revision:selector ID through the request alias-rewrite path and assert it resolves to the internal model name. Reuse the existing alias-rewrite function and test fixtures, ensuring the exact ID emitted by models_list_json is demonstrably routable.
🤖 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/connections.rs`:
- Around line 951-956: Handle the Err result from await_pending_connection in
the PendingConnectionReservation::Waiter branch instead of discarding it. When
the competing pending connection fails, remove or retry the peer so no stale
peer remains after dispatcher exit; preserve the existing close-and-return
behavior when awaiting succeeds.
In `@crates/mesh-llm-host-runtime/src/mesh/gossip.rs`:
- Around line 499-504: Update the direct-peer admission flow around
add_peer_after_direct_requirements_validated so it returns whether the sender
was accepted; only after successful admission should set_mesh_id and
merge_remote_demand be invoked, while rejected peers must not mutate local
identity or demand.
In `@crates/mesh-llm-host-runtime/src/mesh/peer_state.rs`:
- Around line 161-164: Remove the dead-code suppression and either delete the
unused helper or integrate it into the model-path identity reconstruction flow.
Update the relevant callers around the helper in peer state handling so the
implementation no longer relies on the #[expect(dead_code)] attribute.
- Around line 9-17: Update the primary_index handling in the serving-model
identity mapping so a missing primary_model_name does not default to index 0 or
assign the primary identity to another model. Only mark a model as primary when
position finds an actual match; preserve the existing identity behavior for
matching and non-primary models.
In `@crates/mesh-llm-host-runtime/src/mesh/stun.rs`:
- Around line 39-61: Update probe_stun_server and receive_matching_stun_response
so the returned SocketAddr uses the port decoded from the STUN mapped-address
response rather than advertised_port. Probe with the QUIC endpoint’s actual UDP
socket when available, and only return an advertised port after independently
verifying it; otherwise return the discovered IP without claiming an unverified
port.
In `@crates/mesh-llm-host-runtime/src/mesh/tests/admission/requirements.rs`:
- Around line 1155-1169: Update assert_persisted_random_mesh_id_is_preserved and
the similarly structured helpers at the referenced locations to use an RAII drop
guard immediately after changing HOME. Have the guard restore the saved HOME
value on drop, including during assertion or expect unwinding, and remove the
manual restore_env calls that rely on reaching the function end.
In `@crates/mesh-llm-host-runtime/src/plugin/transport.rs`:
- Around line 499-510: Update the mesh stream request flow around mesh_tx.send
and response_rx so one deadline covers both broker enqueue and response waiting,
including when the bounded queue is full and the receiver is stalled. Return the
existing mesh stream error for either enqueue or response timeout, preserving
the response handling for successful sends. Add a test that fills the queue and
verifies the request times out while enqueue is blocked.
In `@crates/mesh-llm-host-runtime/src/runtime/mod.rs`:
- Around line 3285-3300: Split the oversized modules by responsibility before
adding further logic: in
crates/mesh-llm-host-runtime/src/runtime/mod.rs:3285-3300, extract
handle_public_identity_transition and related identity/persistence and startup
phases into an owning module, updating callers and visibility as needed; in
crates/mesh-llm-host-runtime/src/mesh/connections.rs:1403-1725, extract the
relay, dispatch/admission, and pending-connection state machines into an owning
module. Keep behavior unchanged and ensure no Rust source file exceeds 2,000
lines.
---
Outside diff comments:
In `@crates/mesh-llm-host-runtime/src/mesh/connections.rs`:
- Around line 800-823: Update accept_mesh_stream’s discriminator read after
type_buf is created to use a bounded read deadline; when the deadline expires or
the read fails, reset the stalled receive stream before returning Err(()), while
preserving normal dispatch for successfully read stream types.
In `@crates/mesh-llm-host-runtime/src/mesh/node_requirements.rs`:
- Around line 233-275: Make requirement-policy transitions atomic in
install_requirement_aware_mesh_state and the signed-policy enrichment flow
around the corresponding snapshot validation. Hold requirement_mesh_state for
the entire compare-and-set operation, verify the existing mesh ID, policy hash,
and policy still match the requested values, then install or enrich the state
while retaining the guard; do not validate against a released snapshot before
mutation.
In `@crates/mesh-llm-host-runtime/src/mesh/peer_state.rs`:
- Around line 774-872: Split the responsibilities currently concentrated in
`peer_state.rs` into owning submodules before adding further functionality: keep
model identity and peer domain types together, move connection reservation types
and methods such as `PendingConnectionHandshake`, `PendingConnectionWaiter`, and
`MeshState::pending_connection_is_active` into a connection-reservation module,
and separate routing, stage connectivity, and lifecycle-ingestion logic
similarly. Update module declarations and visibility/imports so existing callers
retain the same behavior and APIs.
---
Nitpick comments:
In `@crates/mesh-llm-host-runtime/src/network/openai/transport.rs`:
- Around line 5122-5136: Extend the regression coverage around
models_list_uses_public_huggingface_model_ref_ids to pass the advertised
repo@revision:selector ID through the request alias-rewrite path and assert it
resolves to the internal model name. Reuse the existing alias-rewrite function
and test fixtures, ensuring the exact ID emitted by models_list_json is
demonstrably routable.
🪄 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: 2fc1c05f-fae5-4cea-9991-b0b96e97910f
📒 Files selected for processing (49)
crates/mesh-client/src/client/control_plane.rscrates/mesh-llm-host-runtime/src/api/routes/runtime.rscrates/mesh-llm-host-runtime/src/exact_test_wrappers.rscrates/mesh-llm-host-runtime/src/mesh/artifact_transfer_io.rscrates/mesh-llm-host-runtime/src/mesh/connections.rscrates/mesh-llm-host-runtime/src/mesh/direct_path.rscrates/mesh-llm-host-runtime/src/mesh/gossip.rscrates/mesh-llm-host-runtime/src/mesh/identity_persistence.rscrates/mesh-llm-host-runtime/src/mesh/mod.rscrates/mesh-llm-host-runtime/src/mesh/node.rscrates/mesh-llm-host-runtime/src/mesh/node_identity.rscrates/mesh-llm-host-runtime/src/mesh/node_requirements.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/commands/mod.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/mod.rscrates/mesh-llm-host-runtime/src/mesh/peer_state.rscrates/mesh-llm-host-runtime/src/mesh/plugin_mesh.rscrates/mesh-llm-host-runtime/src/mesh/plugin_streams.rscrates/mesh-llm-host-runtime/src/mesh/public_identity_tests.rscrates/mesh-llm-host-runtime/src/mesh/stage_artifacts.rscrates/mesh-llm-host-runtime/src/mesh/stage_transport.rscrates/mesh-llm-host-runtime/src/mesh/stage_transport_bridge.rscrates/mesh-llm-host-runtime/src/mesh/stun.rscrates/mesh-llm-host-runtime/src/mesh/tests.rscrates/mesh-llm-host-runtime/src/mesh/tests/admission.rscrates/mesh-llm-host-runtime/src/mesh/tests/admission/connectivity.rscrates/mesh-llm-host-runtime/src/mesh/tests/admission/helpers.rscrates/mesh-llm-host-runtime/src/mesh/tests/admission/requirements.rscrates/mesh-llm-host-runtime/src/mesh/tests/connections.rscrates/mesh-llm-host-runtime/src/mesh/tests/direct_path.rscrates/mesh-llm-host-runtime/src/mesh/tests/gossip.rscrates/mesh-llm-host-runtime/src/mesh/tests/owner_control.rscrates/mesh-llm-host-runtime/src/mesh/tests/peer_state.rscrates/mesh-llm-host-runtime/src/mesh/tests/protocol_compat.rscrates/mesh-llm-host-runtime/src/mesh/tests/protocol_frames.rscrates/mesh-llm-host-runtime/src/mesh/tests/stage_transport.rscrates/mesh-llm-host-runtime/src/models/inventory.rscrates/mesh-llm-host-runtime/src/models/mod.rscrates/mesh-llm-host-runtime/src/network/openai/transport.rscrates/mesh-llm-host-runtime/src/plugin/transport.rscrates/mesh-llm-host-runtime/src/protocol/tests.rscrates/mesh-llm-host-runtime/src/protocol/tests/announcements.rscrates/mesh-llm-host-runtime/src/protocol/tests/config.rscrates/mesh-llm-host-runtime/src/protocol/tests/control_frames.rscrates/mesh-llm-host-runtime/src/protocol/tests/mesh_timestamps.rscrates/mesh-llm-host-runtime/src/protocol/tests/owner_control.rscrates/mesh-llm-host-runtime/src/runtime/mod.rscrates/mesh-llm-host-runtime/src/runtime_data/inventory.rscrates/mesh-llm-host-runtime/src/runtime_data/mod.rscrates/mesh-llm-host-runtime/src/runtime_data/tests/inventory.rs
🚧 Files skipped from review as they are similar to previous changes (18)
- crates/mesh-llm-host-runtime/src/mesh/public_identity_tests.rs
- crates/mesh-llm-host-runtime/src/mesh/tests.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/connections.rs
- crates/mesh-llm-host-runtime/src/mesh/plugin_mesh.rs
- crates/mesh-llm-host-runtime/src/mesh/plugin_streams.rs
- crates/mesh-llm-host-runtime/src/mesh/identity_persistence.rs
- crates/mesh-llm-host-runtime/src/mesh/artifact_transfer_io.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/owner_control.rs
- crates/mesh-llm-host-runtime/src/runtime_data/inventory.rs
- crates/mesh-llm-host-runtime/src/mesh/owner_control/mod.rs
- crates/mesh-llm-host-runtime/src/api/routes/runtime.rs
- crates/mesh-llm-host-runtime/src/mesh/owner_control/commands/mod.rs
- crates/mesh-llm-host-runtime/src/mesh/mod.rs
- crates/mesh-llm-host-runtime/src/mesh/stage_transport.rs
- crates/mesh-llm-host-runtime/src/mesh/node_identity.rs
- crates/mesh-client/src/client/control_plane.rs
- crates/mesh-llm-host-runtime/src/mesh/stage_artifacts.rs
- crates/mesh-llm-host-runtime/src/mesh/node.rs
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/mesh-llm-host-runtime/src/mesh/stun.rs (1)
22-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEnsure timeout warnings are consistently logged.
If the deadline has already passed just before
checked_duration_sinceis called, the?operator will silently returnNonefrom the function, bypassing the timeout warning on line 26.Using
unwrap_or_default()ensures that a zero duration is passed totokio::time::timeout, which will immediately fail with anElapsederror and guarantee the warning is logged correctly.♻️ Proposed fix
- let remaining = deadline.checked_duration_since(tokio::time::Instant::now())?; + let remaining = deadline + .checked_duration_since(tokio::time::Instant::now()) + .unwrap_or_default(); match tokio::time::timeout(remaining, addresses.updated()).await {🤖 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/mesh/stun.rs` around lines 22 - 29, Update the deadline duration calculation in the surrounding address-discovery flow to replace the fallible `checked_duration_since` propagation with a zero-duration fallback such as `unwrap_or_default()`. Ensure an expired deadline reaches `tokio::time::timeout`, triggering the existing warning and `None` return path consistently.crates/mesh-llm-host-runtime/src/mesh/model_identity.rs (1)
182-190: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSimplify URL parsing and fix support for top-level repositories.
Formatting a canonical string and passing it back to
parse_hf_ref_partsis not only inefficient but also causes parsing to fail for top-level Hugging Face models (likegpt2). This happens becauseparse_hf_ref_partsexplicitly requires exactly three slash-separated segments (e.g.,owner/repo).Since
split_oncehas already isolated the repository, revision, and filename components from the URL structure, you can return them directly.♻️ Proposed fix
pub(crate) fn parse_hf_resolve_url_parts(url: &str) -> Option<(String, Option<String>, String)> { let path = url .strip_prefix("https://huggingface.co/") .or_else(|| url.strip_prefix("http://huggingface.co/"))?; let (repo, rest) = path.split_once("/resolve/")?; let (revision, file) = rest.split_once('/')?; - let canonical = format!("{repo}@{revision}/{file}"); - parse_hf_ref_parts(&canonical) + if repo.is_empty() || file.is_empty() { + return None; + } + Some((repo.to_string(), Some(revision.to_string()), file.to_string())) }🤖 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/mesh/model_identity.rs` around lines 182 - 190, Update parse_hf_resolve_url_parts to return the repository, revision, and file components directly after the existing URL prefix and split_once validations; remove the canonical string construction and parse_hf_ref_parts call so top-level repositories such as “gpt2” are supported.
🤖 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.
Nitpick comments:
In `@crates/mesh-llm-host-runtime/src/mesh/model_identity.rs`:
- Around line 182-190: Update parse_hf_resolve_url_parts to return the
repository, revision, and file components directly after the existing URL prefix
and split_once validations; remove the canonical string construction and
parse_hf_ref_parts call so top-level repositories such as “gpt2” are supported.
In `@crates/mesh-llm-host-runtime/src/mesh/stun.rs`:
- Around line 22-29: Update the deadline duration calculation in the surrounding
address-discovery flow to replace the fallible `checked_duration_since`
propagation with a zero-duration fallback such as `unwrap_or_default()`. Ensure
an expired deadline reaches `tokio::time::timeout`, triggering the existing
warning and `None` return path consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6f079e72-23f0-4710-8191-1091851bac58
📒 Files selected for processing (19)
crates/mesh-llm-host-runtime/src/exact_test_wrappers.rscrates/mesh-llm-host-runtime/src/mesh/connection_reservation.rscrates/mesh-llm-host-runtime/src/mesh/connections.rscrates/mesh-llm-host-runtime/src/mesh/gossip.rscrates/mesh-llm-host-runtime/src/mesh/mod.rscrates/mesh-llm-host-runtime/src/mesh/model_identity.rscrates/mesh-llm-host-runtime/src/mesh/node.rscrates/mesh-llm-host-runtime/src/mesh/node_identity.rscrates/mesh-llm-host-runtime/src/mesh/node_requirements.rscrates/mesh-llm-host-runtime/src/mesh/peer_state.rscrates/mesh-llm-host-runtime/src/mesh/stun.rscrates/mesh-llm-host-runtime/src/mesh/tests/admission/connectivity.rscrates/mesh-llm-host-runtime/src/mesh/tests/admission/requirements.rscrates/mesh-llm-host-runtime/src/mesh/tests/gossip.rscrates/mesh-llm-host-runtime/src/mesh/tests/peer_state.rscrates/mesh-llm-host-runtime/src/network/openai/transport.rscrates/mesh-llm-host-runtime/src/plugin/transport.rscrates/mesh-llm-host-runtime/src/runtime/mod.rscrates/mesh-llm-host-runtime/src/runtime/startup_identity.rs
💤 Files with no reviewable changes (1)
- crates/mesh-llm-host-runtime/src/mesh/peer_state.rs
🚧 Files skipped from review as they are similar to previous changes (13)
- crates/mesh-llm-host-runtime/src/exact_test_wrappers.rs
- crates/mesh-llm-host-runtime/src/runtime/mod.rs
- crates/mesh-llm-host-runtime/src/network/openai/transport.rs
- crates/mesh-llm-host-runtime/src/mesh/mod.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/admission/connectivity.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/gossip.rs
- crates/mesh-llm-host-runtime/src/mesh/node_identity.rs
- crates/mesh-llm-host-runtime/src/mesh/node_requirements.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/peer_state.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/admission/requirements.rs
- crates/mesh-llm-host-runtime/src/mesh/gossip.rs
- crates/mesh-llm-host-runtime/src/mesh/node.rs
- crates/mesh-llm-host-runtime/src/mesh/connections.rs
Validation: just test-all passed before commit.
ndizazzo
force-pushed
the
codex/owned-node-commands-plan
branch
from
July 18, 2026 17:25
ce38f49 to
fe437aa
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
crates/mesh-llm-host-runtime/src/runtime_data/tests/inventory.rs (1)
5-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImprove test stability by replacing
yield_now()with a short sleep.Using
tokio::task::yield_now().awaitin a polling loop can lead to test flakiness in CI environments. If the executor thread is lightly loaded,yield_nowcan return almost instantly, exhausting the 100 iterations before the background tasks have had enough wall-clock time to execute and update the waiter count.Consider using a short
tokio::time::sleepto give background tasks reliable scheduling time.♻️ Proposed refactor
async fn wait_for_inventory_scan_waiters(collector: &RuntimeDataCollector, expected: usize) { for _ in 0..WAITER_POLL_LIMIT { if collector.inventory_scan_waiter_count() == expected { return; } - tokio::task::yield_now().await; + tokio::time::sleep(std::time::Duration::from_millis(10)).await; } panic!( "inventory scan waiter count did not reach {expected}; got {}", collector.inventory_scan_waiter_count() ); }🤖 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/runtime_data/tests/inventory.rs` around lines 5 - 16, Update wait_for_inventory_scan_waiters to replace tokio::task::yield_now().await with a short tokio::time::sleep delay between polling attempts, while preserving the existing waiter-count checks, limit, and panic 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/tests/peer_state.rs`:
- Line 305: Split the oversized peer_state test file into
responsibility-specific modules under peer_state/, separating the relay, LAN
targeting, admission, gossip, tunnel-map, and route-table test groups. Update
the parent test module declarations and any imports or shared fixtures so all
tests continue compiling and running without changing their behavior.
- Around line 561-588: Replace the direct peer-map mutation in the admission
test with valid gossip submitted through the production gossip handler, then
assert promotion via is_peer_admitted. In
crates/mesh-llm-host-runtime/src/mesh/tests/peer_state.rs:652-657, submit
malformed gossip through that handler and assert rejection with no state
mutation; at 691-723, process the route request and assert it does not admit the
peer; at 1134-1150, submit mismatched sender identity and assert the production
validator rejects it. Use the relevant production handlers and validators rather
than reconstructing their effects.
In `@docs/design/message_protocol.md`:
- Around line 50-54: Update the owned-node command protocol description to scope
the “exactly one typed request and its response” rule to unary commands only.
Document that accepted watch_config streams have no unary deadline and carry the
appropriate ongoing watch message sequence instead of a single response.
- Line 46: Update the Skippy protocol description in the design document to
state that gossip advertises skippy-stage major version 1, while stream 0x0d
opens major version 2. Preserve the existing ownership and framing details, but
remove wording that implies major version 2 is advertised directly.
---
Nitpick comments:
In `@crates/mesh-llm-host-runtime/src/runtime_data/tests/inventory.rs`:
- Around line 5-16: Update wait_for_inventory_scan_waiters to replace
tokio::task::yield_now().await with a short tokio::time::sleep delay between
polling attempts, while preserving the existing waiter-count checks, limit, and
panic 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: e84035b3-04dd-4c63-80ae-d0e46243e045
📒 Files selected for processing (75)
crates/mesh-client/src/client/control_plane.rscrates/mesh-client/src/client/mod.rscrates/mesh-client/tests/control_plane_client.rscrates/mesh-llm-cli/src/runtime.rscrates/mesh-llm-host-runtime/src/api/mod.rscrates/mesh-llm-host-runtime/src/api/routes/mod.rscrates/mesh-llm-host-runtime/src/api/routes/runtime.rscrates/mesh-llm-host-runtime/src/exact_test_wrappers.rscrates/mesh-llm-host-runtime/src/mesh/artifact_transfer_io.rscrates/mesh-llm-host-runtime/src/mesh/connection_reservation.rscrates/mesh-llm-host-runtime/src/mesh/connections.rscrates/mesh-llm-host-runtime/src/mesh/direct_path.rscrates/mesh-llm-host-runtime/src/mesh/gossip.rscrates/mesh-llm-host-runtime/src/mesh/heartbeat.rscrates/mesh-llm-host-runtime/src/mesh/identity_persistence.rscrates/mesh-llm-host-runtime/src/mesh/lan_bootstrap.rscrates/mesh-llm-host-runtime/src/mesh/mod.rscrates/mesh-llm-host-runtime/src/mesh/model_identity.rscrates/mesh-llm-host-runtime/src/mesh/node.rscrates/mesh-llm-host-runtime/src/mesh/node_identity.rscrates/mesh-llm-host-runtime/src/mesh/node_requirements.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/commands/mod.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/commands/scan_refresh.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/mod.rscrates/mesh-llm-host-runtime/src/mesh/peer_state.rscrates/mesh-llm-host-runtime/src/mesh/plugin_mesh.rscrates/mesh-llm-host-runtime/src/mesh/plugin_streams.rscrates/mesh-llm-host-runtime/src/mesh/public_identity_tests.rscrates/mesh-llm-host-runtime/src/mesh/requirements.rscrates/mesh-llm-host-runtime/src/mesh/stage_artifacts.rscrates/mesh-llm-host-runtime/src/mesh/stage_transport.rscrates/mesh-llm-host-runtime/src/mesh/stage_transport_bridge.rscrates/mesh-llm-host-runtime/src/mesh/stun.rscrates/mesh-llm-host-runtime/src/mesh/tests.rscrates/mesh-llm-host-runtime/src/mesh/tests/admission.rscrates/mesh-llm-host-runtime/src/mesh/tests/admission/connectivity.rscrates/mesh-llm-host-runtime/src/mesh/tests/admission/helpers.rscrates/mesh-llm-host-runtime/src/mesh/tests/admission/requirements.rscrates/mesh-llm-host-runtime/src/mesh/tests/connections.rscrates/mesh-llm-host-runtime/src/mesh/tests/direct_path.rscrates/mesh-llm-host-runtime/src/mesh/tests/gossip.rscrates/mesh-llm-host-runtime/src/mesh/tests/owner_control.rscrates/mesh-llm-host-runtime/src/mesh/tests/peer_state.rscrates/mesh-llm-host-runtime/src/mesh/tests/protocol_compat.rscrates/mesh-llm-host-runtime/src/mesh/tests/protocol_frames.rscrates/mesh-llm-host-runtime/src/mesh/tests/requirements.rscrates/mesh-llm-host-runtime/src/mesh/tests/stage_transport.rscrates/mesh-llm-host-runtime/src/models/inventory.rscrates/mesh-llm-host-runtime/src/models/mod.rscrates/mesh-llm-host-runtime/src/network/openai/transport.rscrates/mesh-llm-host-runtime/src/plugin/transport.rscrates/mesh-llm-host-runtime/src/protocol/mod.rscrates/mesh-llm-host-runtime/src/protocol/tests.rscrates/mesh-llm-host-runtime/src/protocol/tests/announcements.rscrates/mesh-llm-host-runtime/src/protocol/tests/config.rscrates/mesh-llm-host-runtime/src/protocol/tests/control_frames.rscrates/mesh-llm-host-runtime/src/protocol/tests/mesh_timestamps.rscrates/mesh-llm-host-runtime/src/protocol/tests/owner_control.rscrates/mesh-llm-host-runtime/src/runtime/mod.rscrates/mesh-llm-host-runtime/src/runtime/startup_identity.rscrates/mesh-llm-host-runtime/src/runtime_data/collector.rscrates/mesh-llm-host-runtime/src/runtime_data/inventory.rscrates/mesh-llm-host-runtime/src/runtime_data/mod.rscrates/mesh-llm-host-runtime/src/runtime_data/tests/inventory.rscrates/mesh-llm-protocol/proto/node.protocrates/mesh-llm-protocol/src/proto/node.rscrates/mesh-llm-protocol/src/protocol/mod.rscrates/mesh-llm/src/commands/runtime.rscrates/mesh-llm/tests/owned_node_commands_cli.rsdocs/USAGE.mddocs/design/TESTING.mddocs/design/message_protocol.mdscripts/qa-control-plane-mixed-version.shwebsite/src/docs/pages/CLI.mdwebsite/src/docs/pages/api-reference.md
🚧 Files skipped from review as they are similar to previous changes (66)
- crates/mesh-llm-host-runtime/src/mesh/tests/admission.rs
- crates/mesh-llm-host-runtime/src/models/mod.rs
- crates/mesh-llm-host-runtime/src/api/routes/mod.rs
- crates/mesh-llm-host-runtime/src/mesh/public_identity_tests.rs
- crates/mesh-client/src/client/mod.rs
- crates/mesh-llm-host-runtime/src/protocol/tests/mesh_timestamps.rs
- crates/mesh-llm-host-runtime/src/api/mod.rs
- crates/mesh-llm-host-runtime/src/mesh/connection_reservation.rs
- crates/mesh-llm-host-runtime/src/mesh/owner_control/commands/scan_refresh.rs
- website/src/docs/pages/api-reference.md
- crates/mesh-llm-host-runtime/src/runtime/startup_identity.rs
- crates/mesh-llm-host-runtime/src/protocol/tests/owner_control.rs
- crates/mesh-llm-host-runtime/src/exact_test_wrappers.rs
- crates/mesh-llm-host-runtime/src/protocol/tests.rs
- crates/mesh-llm-host-runtime/src/mesh/lan_bootstrap.rs
- crates/mesh-llm-host-runtime/src/mesh/stun.rs
- crates/mesh-llm-host-runtime/src/runtime/mod.rs
- crates/mesh-llm/src/commands/runtime.rs
- crates/mesh-llm-host-runtime/src/mesh/owner_control/commands/mod.rs
- crates/mesh-llm-host-runtime/src/runtime_data/collector.rs
- crates/mesh-llm-host-runtime/src/api/routes/runtime.rs
- crates/mesh-llm-protocol/proto/node.proto
- crates/mesh-llm-host-runtime/src/models/inventory.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/protocol_compat.rs
- crates/mesh-llm/tests/owned_node_commands_cli.rs
- crates/mesh-llm-host-runtime/src/mesh/mod.rs
- website/src/docs/pages/CLI.md
- crates/mesh-llm-host-runtime/src/protocol/tests/control_frames.rs
- crates/mesh-llm-host-runtime/src/mesh/requirements.rs
- crates/mesh-llm-host-runtime/src/runtime_data/inventory.rs
- docs/design/TESTING.md
- crates/mesh-llm-protocol/src/proto/node.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/connections.rs
- crates/mesh-llm-host-runtime/src/runtime_data/mod.rs
- crates/mesh-llm-host-runtime/src/plugin/transport.rs
- crates/mesh-llm-host-runtime/src/mesh/plugin_streams.rs
- crates/mesh-llm-protocol/src/protocol/mod.rs
- crates/mesh-llm-host-runtime/src/protocol/tests/announcements.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/gossip.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/requirements.rs
- crates/mesh-llm-host-runtime/src/mesh/model_identity.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/direct_path.rs
- crates/mesh-llm-host-runtime/src/mesh/plugin_mesh.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/admission/helpers.rs
- scripts/qa-control-plane-mixed-version.sh
- crates/mesh-llm-host-runtime/src/network/openai/transport.rs
- crates/mesh-llm-host-runtime/src/protocol/mod.rs
- crates/mesh-client/tests/control_plane_client.rs
- crates/mesh-llm-host-runtime/src/protocol/tests/config.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/protocol_frames.rs
- crates/mesh-llm-host-runtime/src/mesh/artifact_transfer_io.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/stage_transport.rs
- crates/mesh-llm-host-runtime/src/mesh/peer_state.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/owner_control.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/admission/requirements.rs
- crates/mesh-llm-host-runtime/src/mesh/node_requirements.rs
- crates/mesh-llm-host-runtime/src/mesh/direct_path.rs
- crates/mesh-llm-host-runtime/src/mesh/identity_persistence.rs
- crates/mesh-llm-host-runtime/src/mesh/owner_control/mod.rs
- crates/mesh-client/src/client/control_plane.rs
- crates/mesh-llm-host-runtime/src/mesh/stage_artifacts.rs
- crates/mesh-llm-host-runtime/src/mesh/stage_transport.rs
- crates/mesh-llm-host-runtime/src/mesh/node.rs
- crates/mesh-llm-host-runtime/src/mesh/node_identity.rs
- crates/mesh-llm-host-runtime/src/mesh/gossip.rs
- crates/mesh-llm-host-runtime/src/mesh/connections.rs
Address current CodeRabbit feedback by routing peer-state coverage through production admission and decoder seams, and clarify owner-control watch protocol semantics.
This was referenced Aug 7, 2026
This was referenced Aug 13, 2026
Closed
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
1 participant
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.
#991 Should merge first!
Purpose
Provide an extensible, private command path for operating one explicitly targeted, owner-attested node without adding another transport or coupling node management to the public mesh plane.
This makes remote inventory scan/refresh the first supported owned-node command and hardens the existing owner-control lifecycle before future mutating operations are added.
What changed
refresh_inventory = 5operation additively with:executedversuscoalesceddispositionmesh-client::scan_refresh()POST /api/runtime/control/scan-refreshmesh-llm runtime scan-refresh --endpoint <token>Why
The existing private owner-control lane already had the correct authentication and transport boundary, but inventory refresh only returned a legacy snapshot and several lifecycle phases were not sufficiently bounded for a broader command surface. In particular, command deadlines were defined but not enforced server-side, and oversized outbound payloads needed rejection before any frame bytes were emitted.
Extending the shipped typed protobuf envelope keeps ownership and compatibility explicit while avoiding a second control protocol, opaque command registry, endpoint inference, or changes to public discovery/routing.
Compatibility and security
/api/statusschema changerefresh_inventory = 5and responsesnapshot = 1remain unchanged; rich inventory is additive tag 2Validation
just test-alljust buildjust website-buildv0.73.1/current public-mesh QA:Summary by CodeRabbit
New Features
runtime scan-refreshCLI command and loopback REST endpoint for owner-controlled inventory rescans.target_node_id,disposition(executed/coalesced), and optional typedinventorydetails; older servers may returninventory: nullwhile the request still succeeds.Bug Fixes
Documentation