feat(runtime): add daemon model lifecycle reconciliation - #1082
Conversation
|
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 PR adds owner-control model lifecycle commands, runtime intent reconciliation, per-instance draining, host-activity admission controls, runtime status APIs, CLI/UI support, CUDA setup improvements, certification scripts, and test-all Rust crate coverage validation. ChangesRuntime lifecycle and owner control
Activity admission and runtime status
Tooling and supporting surfaces
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Suggested labels: Suggested reviewers: 🚥 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 |
|
This pull request is currently a draft. Reviews will not take place until the PR is marked as ready for review. |
Native CUDA and daemon lifecycle certificationVerdict: PASS on Test matrix
All three remote checkouts were pulled to the exact PR head in Native build resultsThe definitive builds used the platform's installed compiler and native GPU
Build artifacts:
Failures found and fixes appliedThe initial Orin builds found that Two fixes were implemented, tested, and pushed:
Validation for those fixes:
Daemon lifecycle certificationThe repository's
Evidence:
Real-model load, pause, resume, and unloadEach host ran an isolated daemon with activity policy enabled
Pause was admission control, not an unload: resident model memory remained Evidence:
Additional workload evidenceThe first Artifact: During the first broad universal-architecture build attempt on The pre-existing LM Studio GitHub CI
After the PR was marked ready, GitHub's Final assessmentThe PR builds natively on x86_64/Ada+Blackwell and aarch64/Orin, the complete |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
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 (1)
crates/mesh-llm-host-runtime/src/runtime/model_lifecycle.rs (1)
206-245: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winShutdown path leaves lifecycle records registered on the node.
The new
..discardsentry.lifecycle, so unlikerun_auto_unload_runtime_entry(unload.rslines 209-216) this loop never callsnode.unregister_runtime_instance_lifecycle(handle.port)nor transitions the record toStopped. Since the daemon now outlives model teardown,runtime_instance_lifecycleskeeps stale port→record entries; a later instance bound to a recycled port would be admitted/rejected against the dead record.🐛 Proposed fix
let RuntimeModelHandleEntry { model_name: name, handle, capacity_reservation, - .. + lifecycle, } = entry; @@ remove_dashboard_context_usage(dashboard_context_usage, &name, &handle).await; + node.unregister_runtime_instance_lifecycle(handle.port); + let _ = lifecycle + .lock() + .await + .transition_to(InstanceLifecycleState::Stopped);🤖 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/model_lifecycle.rs` around lines 206 - 245, Preserve the lifecycle record when destructuring each RuntimeModelHandleEntry in the shutdown loop instead of discarding entry.lifecycle. Before or during handle shutdown, mirror run_auto_unload_runtime_entry’s lifecycle cleanup by unregistering the record for handle.port and transitioning it to Stopped, ensuring runtime_instance_lifecycles no longer retains stale entries.
🟠 Major comments (25)
crates/mesh-llm-system/src/activity/macos.rs-11-37 (1)
11-37: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftLoad average is a poor proxy for "user is actively using this machine".
getloadavgreflects total system load including this daemon's own inference work, so serving a model will itself push the host intoActiveand triggerpause_all/pause_remote— a self-sustaining pause loop. Consider excluding own-process CPU, or documenting that macOS detection is load-based and recommendingreduce_prioritythere.🤖 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-system/src/activity/macos.rs` around lines 11 - 37, Replace the load-average-based logic in MacHostActivityDetector::sample with a user-activity signal that excludes this daemon’s own inference work, preventing model serving from triggering a self-sustaining pause loop. If that cannot be implemented, explicitly document the load-based limitation and recommend using reduce_priority for macOS instead of treating the result as direct user activity.crates/mesh-llm-host-runtime/src/api/routes/runtime.rs-1562-1578 (1)
1562-1578: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGlobal API mutex is held across the response write.
state.inner.lock().awaitstays alive until the end of the function becauserespond_jsonis called inside the guard's scope. Any slow or stalled client on/api/runtime/activity(or the override routes) blocks every other request that needsstate.inner.handle_get_intentsgets this right by scoping the lock in a block — do the same here.🔒 Proposed fix (apply the same shape to all three handlers)
- let inner = state.inner.lock().await; - let guard = &inner.node.activity_policy_guard; - let effective_state = map_policy_state(guard.effective_state()); - let override_mode = map_override_mode(guard.manual_override()); - let detector_category = map_detector_category(guard.detector_state()); + let (effective_state, override_mode, detector_category) = { + let inner = state.inner.lock().await; + let guard = &inner.node.activity_policy_guard; + ( + guard.effective_state().into(), + map_override_mode(guard.manual_override()), + map_detector_category(guard.detector_state()), + ) + };Also applies to: 1600-1617, 1627-1644
🤖 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/api/routes/runtime.rs` around lines 1562 - 1578, Limit the lifetime of the state.inner mutex guard in all three activity-policy handlers, including the status handler and the handlers around the referenced ranges. Build ActivityPolicyStatus or the corresponding response data inside a scoped lock block, then call respond_json after the block ends so no mutex is held during response writes; follow the existing handle_get_intents scoping pattern.crates/mesh-llm-system/src/activity/mod.rs-257-288 (1)
257-288: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPriority degradation is latched permanently.
PrioritySessionnever restoresPriorityStatus::Healthyafter a successfulreduce/restore, andclear_degraded()has no callers, so a single failure (very likely on non-root Linux/macOS restore, and guaranteed on Windows/unsupported) pins the runtime into aDegradeddaemon state and disablesreduce_priorityfor the process lifetime.
crates/mesh-llm-system/src/activity/mod.rs#L257-L288: setself.status = PriorityStatus::Healthyon the success paths of bothreduceandrestore, and updateactivity_detector_and_priority_fail_closed_without_leakaccordingly.crates/mesh-llm-host-runtime/src/runtime/activity_policy.rs#L513-L529: once the session can recover, this loop'smark_priority_degraded/clear_priority_degradedtoggle becomes meaningful; verifyclear_priority_degradedis reached after a recovered tick.🤖 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-system/src/activity/mod.rs` around lines 257 - 288, PrioritySession keeps a degraded status after priority operations recover. In PrioritySession::reduce and PrioritySession::restore, set status to Healthy on successful completion, update activity_detector_and_priority_fail_closed_without_leak for the recovered state, and verify the recovered-tick path reaches clear_priority_degraded in crates/mesh-llm-host-runtime/src/runtime/activity_policy.rs lines 513-529; no direct change is required there unless verification exposes a failure.crates/mesh-llm-host-runtime/src/api/routes/runtime.rs-1510-1518 (1)
1510-1518: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
.expect()on a poisoned mutex panics the request handler.A panic anywhere else while holding
runtime_intentsturns every subsequentGET /api/runtime/intentsinto a panicking task instead of an error response. Recover the guard and return an empty/500 result.🛡️ Proposed fix
- inner - .node - .runtime_intents - .lock() - .expect("runtime intent history mutex poisoned") - .clone() + match inner.node.runtime_intents.lock() { + Ok(history) => history.clone(), + Err(poisoned) => poisoned.into_inner().clone(), + }🤖 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/api/routes/runtime.rs` around lines 1510 - 1518, Update the runtime intent history retrieval in the GET /api/runtime/intents handler to avoid calling expect on the runtime_intents mutex. Handle a poisoned lock by recovering the guard when possible or returning an appropriate empty/500 response, ensuring subsequent requests do not panic.crates/mesh-llm-system/src/activity/linux.rs-73-93 (1)
73-93: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle Linux restore failures as non-degrading or gate deprioritization on restore ability.
Reducing priority on Linux raises the nice value, which unprivileged processes can almost always do; restoring it lowers nice, which requires
CAP_SYS_NICEor a sufficientRLIMIT_NICE soft.EPERM/EACCEShere should not permanently makePrioritySessionreportDegraded; clearoriginal_nicefor unprivileged restore failures, or only enablereduce_prioritywhen the restore path is guaranteed to work.🤖 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-system/src/activity/linux.rs` around lines 73 - 93, Update PrioritySession::restore_priority so Linux restore failures caused by insufficient permissions (EPERM/EACCES) clear original_nice and do not leave the session reporting Degraded, while preserving PriorityFailure::RestoreFailed for other restore errors. Keep successful restores clearing original_nice as they do now.crates/mesh-llm-host-runtime/src/protocol/convert.rs-701-701 (1)
701-701: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve
inference_admission_statein the gossip proto encoder.
local_ann_to_proto_annis the export path for gossip announcements and currently serializes the field asNone, whileproto_ann_to_localexpects it to restore the peer’s state. This dropsCoarseStateadvertising; if privacy is required for private modes, the sanitizer should explicitly strip it for the relevant policy rather than leaving the advertised state on the local peer state but omitting it from the peer-to-peer proto.🤖 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/convert.rs` at line 701, Update local_ann_to_proto_ann to serialize the local announcement’s inference_admission_state instead of always setting it to None, preserving CoarseState through the gossip proto round trip expected by proto_ann_to_local. Keep any privacy sanitization explicit and policy-specific rather than dropping the field unconditionally.crates/mesh-llm-host-runtime/src/protocol/control_frames.rs-725-741 (1)
725-741: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
validate_owner_control_model_for_load_or_ensuremishandles empty-string instance ids and the "both present" case.Unlike its sibling
validate_owner_control_model_for_unload_or_drain(743-758), which normalizes "instance present" viainstance_id.as_deref().is_some_and(|id| !id.trim().is_empty()), this function checksmodel.instance_id.is_none()strictly. As a result:
canonical_model_refset +instance_id = Some("")incorrectly returnsMissingModelRef(should beOk, since an empty instance id is equivalent to absent).canonical_model_refset +instance_id = Some("abc")(both present) also incorrectly returnsMissingModelRefinstead ofInvalidModelRefCombination(a ref is present, it's just ambiguous).This can incorrectly reject valid load/ensure requests and report a misleading error code, undermining the new model-lifecycle control-frame validation this PR introduces.
🐛 Proposed fix
fn validate_owner_control_model_for_load_or_ensure( model: &crate::proto::node::OwnerControlModelRef, ) -> Result<(), ControlFrameError> { - if !model.canonical_model_ref.trim().is_empty() && model.instance_id.is_none() { - return Ok(()); - } - if model.canonical_model_ref.trim().is_empty() - && model - .instance_id - .as_deref() - .is_some_and(|id| !id.trim().is_empty()) - { - return Err(ControlFrameError::InvalidModelRefCombination); - } - Err(ControlFrameError::MissingModelRef) + let canonical = !model.canonical_model_ref.trim().is_empty(); + let instance = model + .instance_id + .as_deref() + .is_some_and(|id| !id.trim().is_empty()); + match (canonical, instance) { + (true, false) => Ok(()), + (false, false) => Err(ControlFrameError::MissingModelRef), + _ => Err(ControlFrameError::InvalidModelRefCombination), + } }🤖 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/control_frames.rs` around lines 725 - 741, Update validate_owner_control_model_for_load_or_ensure to normalize instance_id presence using a trimmed, non-empty value. Return Ok when canonical_model_ref is non-empty and instance_id is absent or empty; return InvalidModelRefCombination when both canonical_model_ref and a non-empty instance_id are present; preserve MissingModelRef when neither valid reference is provided.crates/mesh-llm-host-runtime/src/api/mod.rs-927-950 (1)
927-950: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not map normal unload
"exited"status to lifecycle"failed".The API treats status
"exited"as lifecycle state"failed", while graceful shutdown paths transition the instance lifecycle to"stopped"and only unexpected unloads use"exited"/unexpected exit telemetry. Map runtime status"stopped"→ lifecycle"stopped"so clean unloads are not surfaced as daemon/runtime 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/api/mod.rs` around lines 927 - 950, Update the lifecycle_state mapping in the local_processes transformation to map runtime status "stopped" to lifecycle state "stopped" instead of treating normal unloads as failures. Preserve the existing mappings for "ready", "serving", "shutting down", "exited", and "starting" in the match within the lifecycle_instances construction.tools/xtask/src/repo_consistency.rs-264-298 (1)
264-298: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftThis consistency check is tautological — it can never fail except on the subset check.
dynamic_targetsandexcluded_targetsare built from the same constant, so:
ensure_set_eq(&dynamic_targets, &excluded_targets, ...)is true by construction;covered_crates = (workspace − excluded) ∪ dynamicequalsworkspacewheneverdynamic ⊆ workspace, which the precedingcheck_subsetalready guarantees, so the finalensure_set_eqis also true by construction.The only real assertion left is "these four crate names exist in the workspace". For the check to actually catch drift, at least one side must be parsed from the real source of truth (the
test-allrecipe in theJustfile: the dynamically-tested targets and thecargo test --workspace --exclude ...list), then compared against the other and againstcargo metadata.#!/bin/bash # Inspect the actual test-all recipe to see the dynamic targets and excludes rg -n -A 40 '^test-all' Justfile🤖 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 `@tools/xtask/src/repo_consistency.rs` around lines 264 - 298, Update the consistency check around TEST_ALL_DYNAMIC_TEST_TARGETS so it parses the test-all recipe in the Justfile for both dynamically tested targets and cargo test exclusions, rather than deriving both sets from the same constant. Compare the independently extracted sets for parity and validate their coverage against workspace_crates, while preserving the existing subset and coverage checks where applicable.scripts/qa-runtime-daemon-lifecycle.sh-357-378 (1)
357-378: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winA
cleanupFAIL can never influence the harness exit code.
exit "$EXIT_STATUS"at line 1067 evaluates the variable before the EXIT trap runs, so whencleanuprecordsFAIL cleanup "harness-owned processes remain"and setsEXIT_STATUS=1, the process still exits 0. Leaked processes — one of the documented checks (clean_process_teardown) — would be reported inresults.jsonlbut the script would be green in CI. Havecleanupre-exit explicitly.🐛 Proposed fix
write_summary_json + if [[ "$EXIT_STATUS" -ne 0 ]]; then + exit "$EXIT_STATUS" + fi } trap cleanup EXITAlso applies to: 1059-1067
🤖 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 `@scripts/qa-runtime-daemon-lifecycle.sh` around lines 357 - 378, Update cleanup() so that when it detects harness-owned processes remain and records the cleanup failure, it also preserves that failure in EXIT_STATUS and explicitly exits with the final EXIT_STATUS after write_summary_json. Ensure the EXIT trap does not allow the script to return the pre-cleanup status.scripts/qa-runtime-daemon-lifecycle.sh-933-975 (1)
933-975: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftThe privacy check is vacuous — the sentinels are never introduced into the system under test.
alice,SecretEditor, andraw epochare scanned for, but nothing in this harness configures a node with those identities/values (nodes start headless with ephemeral keys and no model), so the check passes unconditionally and would keep passing even if the runtime started leaking real identity data. To be meaningful it needs to first seed an identifiable value (e.g. an owner/display name and a known model ref) and then assert that value does not appear in status/intent payloads.Related:
"raw epoch"as a literal substring will never appear in JSON; a raw epoch leak would surface as a bare 10-digit integer field, which needs a structural assertion rather than a grep.🤖 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 `@scripts/qa-runtime-daemon-lifecycle.sh` around lines 933 - 975, Make check_privacy_no_raw_data meaningful by first seeding the running daemon with identifiable owner/display-name and known model-reference values, then scan the resulting status and intent payloads to ensure those values are absent. Replace the ineffective literal "raw epoch" grep with a structural JSON assertion that rejects bare 10-digit epoch integer fields. Preserve the existing prerequisite handling and result reporting.crates/mesh-llm-host-runtime/src/runtime/control_loop.rs-700-721 (1)
700-721: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winStartup load completion bypasses the completion-channel fan-out. Stacked
oneshotwaiters are only released bynotify_load_success/notify_load_failure, but the startup path reports throughRuntimeEvent::StartupModelLoadFinished, which records state without notifying — so any API load that piggybacks on an in-flight startup load hangs and its sender leaks.
crates/mesh-llm-host-runtime/src/runtime/control_loop.rs#L700-L721: callnotify_load_success/notify_load_failurein both arms of theStartupModelLoadFinishedhandler, mirroring whatrun_auto_handle_model_intentdoes at Lines 479-497.crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/state.rs#L525-L577: add a test that stacks a load completion for a(model_ref, profile)marked in flight viamark_load_startedand asserts the waiter resolves once the startup result is recorded, so this fan-out gap cannot regress.🤖 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/control_loop.rs` around lines 700 - 721, Update the StartupModelLoadFinished handler in control_loop.rs to call notify_load_success and notify_load_failure in the respective result arms, matching run_auto_handle_model_intent while preserving state recording and error propagation. Add a regression test in state.rs covering a waiter stacked after mark_load_started, asserting it resolves when the startup load completion is recorded for the same model_ref and profile.crates/mesh-llm-host-runtime/src/runtime/control_loop.rs-533-542 (1)
533-542: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftUnload intents are recorded under the empty profile regardless of the loaded model's profile.
suppress_desired_with_id(&intent_model, "", ...)hardcodes"", but load intents are keyed by the real profile (Line 460,add_desired_with_id(&spec, &profile, ...)) anddesired_models/effective_intent/is_desiredare all(model_ref, profile)-keyed. Unloading a model that was loaded under a non-default profile therefore writes anAbsentintent for(model, "")and leaves the(model, "low-ctx")Presentintent effective — the reconciler is free to reload it on the next tick, andis_desiredin the startup filter (Line 158) still reports the model as desired.The profile-independence test in
model_reconciliation/planner.rs(Line 729) documents that profiles are tracked separately, which makes this key mismatch a correctness gap rather than a simplification.The unload target needs to carry the profile (resolved from the runtime instance registry for
UnloadTarget::Instance, or from the desired-state entry forUnloadTarget::Model) so suppression lands on the same key the load used.🤖 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/control_loop.rs` around lines 533 - 542, Update the unload handling around suppress_desired_with_id so it uses the loaded model’s profile instead of hardcoding an empty profile. Resolve the profile from the runtime instance registry for UnloadTarget::Instance and from the desired-state entry for UnloadTarget::Model, then pass that profile to suppress_desired_with_id so suppression targets the same (model, profile) key used by add_desired_with_id.crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/state.rs-460-467 (1)
460-467: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPending checks use exact string equality while suppression uses
model_identity_matches.
cooldown_active(Line 586) deliberately treatsrepo@main:selandrepo:selas the same model, butis_load_pendingkeys on raw(model_ref, profile)equality. A second load request that spells the same model differently therefore sees no pending load and starts a concurrent duplicate, and its completion channel lands under a key the first load will never notify.🤖 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/model_reconciliation/state.rs` around lines 460 - 467, Update is_load_pending to use model_identity_matches when comparing model references, rather than exact raw-string key equality, so equivalent forms such as repo@main:sel and repo:sel share pending state. Apply the same identity-aware lookup to pending_load_completions and ensure load completion registration/notification uses the matching canonical identity so equivalent requests share the existing completion channel.crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/state.rs-343-351 (1)
343-351: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPoisoned-mutex
expectturns a reader panic into a control-loop panic.
shared_historyis handed to the API status surface (node.runtime_intents). If any reader panics while holding the lock, every subsequent intent record — i.e. every load, unload, drain, and startup seed — panics the runtime control loop. The guarded value is a plainVecthat is wholly overwritten here, so poisoning carries no invariant worth aborting on.🔒️ Proposed fix: recover the guard instead of panicking
fn publish_history(&self) { let Some(shared) = &self.shared_history else { return; }; - *shared - .lock() - .expect("runtime intent history mutex poisoned") = - self.intent_history.iter().cloned().collect(); + let mut guard = match shared.lock() { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + }; + *guard = self.intent_history.iter().cloned().collect(); }🤖 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/model_reconciliation/state.rs` around lines 343 - 351, Update publish_history to recover the mutex guard from shared_history when the mutex is poisoned instead of calling expect and panicking. Continue overwriting the guarded Vec with the collected intent_history, preserving the existing early return when shared_history is absent.crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/planner.rs-80-82 (1)
80-82: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winIntra-tick dedup key ignores
profileandmodel_name, diverging from the rest of the planner's identity rules.Everywhere else identity is profile-scoped (
state.suppressed(model_ref, profile, ...),record_load_failure(.., profile, ..), and thereconciliation_tracks_profiles_independentlytest) or name-aware (loaded_targetalso matchestarget.model_name). This new filter usesmodel_refalone, so withmax_loads_per_tick > 1:
- two candidates for the same
model_refunder different profiles collapse to one action, contradicting profile-independent reconciliation;- two candidates with different
model_refs that resolve to the samemodel_nameboth get scheduled, which is the exact duplicateloaded_targetguards against.Default
max_loads_per_tickis 1, so this is latent rather than currently firing — but the new test at Line 500 raises it to 3.🐛 Proposed fix: align the dedup key with existing identity handling
|| actions .iter() - .any(|action| model_identity_matches(&action.model_ref, &target.model_ref)) + .any(|action| { + action.profile == target.profile + && (model_identity_matches(&action.model_ref, &target.model_ref) + || match (action.model_name.as_deref(), target.model_name.as_deref()) { + (Some(left), Some(right)) => model_identity_matches(left, right), + _ => false, + }) + })🤖 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/model_reconciliation/planner.rs` around lines 80 - 82, Update the intra-tick action deduplication predicate in the planner to use the same profile-scoped, name-aware identity rules as the surrounding reconciliation logic. Include both the action and target profiles and model names when comparing candidates, reusing the existing identity helper or matching behavior used by loaded_target, so different profiles remain independent while equivalent model names are deduplicated.crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/state.rs-214-218 (1)
214-218: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winHistory eviction can strand
desired_modelsentries.When the 256-entry cap evicts the front of
intent_history, onlyretired_intentsis cleaned up. If the evicted entry was the effective intent for some(model_ref, profile),desired_modelsstill holds that key becauserefresh_desired_modelis never re-run for it — the reconciler will keep treating the model as desired with no backing intent, and the status surface will disagree with history.The bounded-history test at Line 801 pushes 300 intents but only asserts the length, so this path is untested.
🐛 Proposed fix: refresh the projection for evicted keys
- while self.intent_history.len() > 256 { - if let Some(removed) = self.intent_history.pop_front() { - self.retired_intents.remove(&removed.intent_id); - } - } + let mut evicted = Vec::new(); + while self.intent_history.len() > 256 { + if let Some(removed) = self.intent_history.pop_front() { + self.retired_intents.remove(&removed.intent_id); + evicted.push((removed.canonical_model_ref, removed.profile)); + } + } + for (model_ref, profile) in evicted { + self.refresh_desired_model(&model_ref, &profile); + }🤖 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/model_reconciliation/state.rs` around lines 214 - 218, Update the intent-history eviction loop to refresh the desired-model projection for each evicted intent’s (model_ref, profile) key after removing it from retired_intents. Reuse the existing refresh_desired_model mechanism and ensure the bounded-history test also verifies that desired_models no longer retains projections whose effective intents were evicted.crates/mesh-llm-config/src/model.rs-73-79 (1)
73-79: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject invalid drain timeout configs and avoid exposing a no-op default.
RuntimeConfig.drain_timeout_secsis deserialized directly and not used by node startup or owner-control drains, whileexecute_drainfalls back to the crate default30instead of rejecting omitted timeouts. Config values likeruntime.drain_timeout_secs = 0, orruntime.drain_timeout_secs = 60withruntime.drain_timeout_max_secs = 30, can still be accepted at config load time. Add cross-field validation and either plumb the configured default into drains or reject/normalize omitted drain timeouts consistently.🤖 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-config/src/model.rs` around lines 73 - 79, Add cross-field validation for RuntimeConfig so drain_timeout_secs is positive and does not exceed drain_timeout_max_secs, rejecting invalid values during deserialization/config loading. Ensure omitted drain timeouts are handled consistently by either propagating the configured default into execute_drain or rejecting/normalizing omission, rather than silently using an unrelated crate default. Avoid exposing a no-op default through default_drain_timeout_secs.crates/mesh-llm-host-runtime/src/runtime/daemon_startup.rs-51-51 (1)
51-51: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUnjustified
#[allow(dead_code)]on three new public functions.Use
#[expect(dead_code, reason = "...")]so the attribute self-documents and fails once the function is actually wired up — the same pattern already used inruntime/interactive.rsin this PR. Also,apply_startup_failure_policyprefixes used parameters with_, which signals the opposite of the truth.♻️ Proposed change
-#[allow(dead_code)] +#[expect( + dead_code, + reason = "wired up by daemon startup integration in a follow-up (Todo 8+)" +)] pub fn apply_startup_failure_policy( - _failure_mode: mesh_llm_config::StartupFailurePolicy, - _error_msg: &str, + failure_mode: mesh_llm_config::StartupFailurePolicy, + error_msg: &str, ) -> Option<String> { - match _failure_mode { - mesh_llm_config::StartupFailurePolicy::FailFast => Some(_error_msg.to_string()), + match failure_mode { + mesh_llm_config::StartupFailurePolicy::FailFast => Some(error_msg.to_string()), mesh_llm_config::StartupFailurePolicy::BestEffort => None, } }As per coding guidelines: "do not use
#[allow(...)]to silence warnings without a clear reason and developer approval."Also applies to: 65-65, 76-85
🤖 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/daemon_startup.rs` at line 51, Replace the unjustified #[allow(dead_code)] attributes on the three public startup functions, including apply_startup_failure_policy, with #[expect(dead_code, reason = "...")] using clear reasons consistent with runtime/interactive.rs. Remove leading underscores from parameters that apply_startup_failure_policy actually uses, updating all references accordingly.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/mesh/owner_control/mod.rs-909-923 (1)
909-923: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftLifecycle response cache only dedupes sequential retries, and can return a stale envelope.
Two problems in the check-then-execute window:
- TOCTOU: the lookup at Line 909 releases the lock before execution at Line 952. Two concurrent frames carrying the same
(requester, request_id)both miss, both executeexecute_load/execute_drain, and both insert. Retry-on-timeout by a client is exactly the case that produces this.- Duplicate keys:
cache_owner_lifecycle_responsealwayspush_backs without replacing an existing entry for the same key, while the lookup usesiter().find(...), which returns the oldest match. After (1) happens, subsequent retries get the first envelope — a differentintent_idthan the one that actually won.Reserving the key under the same lock before execution (and replacing rather than appending on completion) fixes both. Separately, the global 256 cap has no per-requester bound, so one chatty owner can evict every other peer's entries.
🔒 Sketch: reserve-then-fill
- if is_model_lifecycle { - let cached = self - .owner_lifecycle_response_cache - .lock() - .await - .iter() - .find(|((requester, cached_id), _)| { - *requester == remote && *cached_id == command_request_id - }) - .map(|(_, envelope)| envelope.clone()); - if let Some(envelope) = cached { - self.send_owner_control_envelope(send, envelope).await?; - return Ok(execution_shape); - } - } + if is_model_lifecycle { + // Single critical section: hit -> replay, miss -> reserve the key so a + // concurrent duplicate cannot execute the command a second time. + match self + .reserve_or_replay_lifecycle_response(remote, command_request_id) + .await + { + LifecycleCacheSlot::Replay(envelope) => { + self.send_owner_control_envelope(send, envelope).await?; + return Ok(execution_shape); + } + LifecycleCacheSlot::Reserved => {} + } + }and in the writer, overwrite an existing key instead of appending:
let mut cache = self.owner_lifecycle_response_cache.lock().await; - cache.push_back(((requester, request_id), envelope.clone())); + if let Some(slot) = cache + .iter_mut() + .find(|((cached_requester, cached_id), _)| { + *cached_requester == requester && *cached_id == request_id + }) + { + slot.1 = envelope.clone(); + } else { + cache.push_back(((requester, request_id), envelope.clone())); + } while cache.len() > MAX_CACHED_LIFECYCLE_RESPONSES { cache.pop_front(); }Also applies to: 1008-1020
🤖 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 909 - 923, Update the lifecycle response cache flow around the lookup and execution path to reserve each (requester, command_request_id) key under the cache lock before executing, so concurrent retries share one in-flight result instead of executing twice. Modify cache_owner_lifecycle_response to replace an existing key when completion stores the envelope, ensuring lookups cannot return an older duplicate; also enforce a per-requester cache bound in addition to the global 256-entry cap.crates/mesh-llm-host-runtime/src/runtime/instance_lifecycle.rs-141-144 (1)
141-144: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Draininghas no transition toFailed, so a failure mid-drain wedges the instance.Every other non-terminal state can reach
Failed;Drainingcan only reachUnloading. If the drain or the subsequent unload preparation errors out,transition_to(Failed)returnsInvalidTransitionand the record staysDraining— not accepting work, not terminal, never reaped. Confirm this is deliberate rather than an omission.🐛 Suggested table entry
( InstanceLifecycleState::Draining, - &[InstanceLifecycleState::Unloading], + &[ + InstanceLifecycleState::Unloading, + InstanceLifecycleState::Failed, + ], ),🤖 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/instance_lifecycle.rs` around lines 141 - 144, Update the transition table containing InstanceLifecycleState::Draining to allow a transition to InstanceLifecycleState::Failed in addition to Unloading. Preserve the existing Unloading transition and ensure transition_to(Failed) succeeds when drain or unload preparation fails.crates/mesh-llm-host-runtime/src/runtime/instance_lifecycle.rs-308-331 (1)
308-331: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
mark_draining_forcecannot escalate an already-draining instance.The idempotency guard at Line 313 returns
Ok(())before the deadline is touched, so the second call's deadline is discarded. The realistic sequence — operator issues a graceful drain with a 300s timeout, then issues a force drain because it's hanging — silently no-ops and returns success, leavingdrain_deadlineat the original value. The caller has no way to distinguish "escalated" from "ignored".Shortening the deadline on a repeat call keeps the operation idempotent while making force meaningful.
🐛 Proposed fix
pub(crate) fn mark_draining( &mut self, deadline: Instant, ) -> Result<(), InstanceLifecycleError> { - // Idempotency guard: if already draining, return Ok. + // Idempotency guard: repeat drains are accepted, and may only tighten + // the deadline (so force-drain can escalate a graceful drain). if self.draining_initiated { + self.drain_deadline = Some(match self.drain_deadline { + Some(current) => current.min(deadline), + None => deadline, + }); return Ok(()); }🤖 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/instance_lifecycle.rs` around lines 308 - 331, Update mark_draining so repeated calls can shorten an existing drain_deadline instead of returning immediately. Preserve idempotency for equal or later deadlines, but when the new deadline is earlier—especially mark_draining_force’s Instant::now()—store it and return Ok without re-running the state transition; retain the existing validation and initial-drain behavior.crates/mesh-llm-protocol/src/protocol/mod.rs-735-750 (1)
735-750: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLoad/ensure model-ref validation misclassifies errors and rejects a blank
instance_id.
validate_owner_control_model_for_load_or_ensuretreatsinstance_id: Some("")as "present" (onlyis_none()is accepted on the happy path), so a peer that serializes an empty-string instance id together with a valid canonical ref is rejected withMissingModelRef— even thoughvalidate_owner_control_model_for_unload_or_draincorrectly treats a blank instance id as absent. The same fall-through also reportsMissingModelReffor the genuinely invalidcanonical + instancecombination, which should beInvalidModelRefCombination.Mirroring the trimmed-boolean shape used by the unload/drain helper fixes both.
🐛 Proposed fix
fn validate_owner_control_model_for_load_or_ensure( model: &crate::proto::node::OwnerControlModelRef, ) -> Result<(), ControlFrameError> { - if !model.canonical_model_ref.trim().is_empty() && model.instance_id.is_none() { - return Ok(()); - } - if model.canonical_model_ref.trim().is_empty() - && model - .instance_id - .as_deref() - .is_some_and(|id| !id.trim().is_empty()) - { - return Err(ControlFrameError::InvalidModelRefCombination); - } - Err(ControlFrameError::MissingModelRef) + let canonical = !model.canonical_model_ref.trim().is_empty(); + let instance = model + .instance_id + .as_deref() + .is_some_and(|id| !id.trim().is_empty()); + match (canonical, instance) { + (true, false) => Ok(()), + (false, false) => Err(ControlFrameError::MissingModelRef), + _ => Err(ControlFrameError::InvalidModelRefCombination), + } }🤖 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-protocol/src/protocol/mod.rs` around lines 735 - 750, Update validate_owner_control_model_for_load_or_ensure to use trimmed, non-empty booleans for both canonical_model_ref and instance_id, matching validate_owner_control_model_for_unload_or_drain. Accept a valid canonical reference with a missing or blank instance_id, return InvalidModelRefCombination when both trimmed values are present, and return MissingModelRef when neither is present.crates/mesh-llm-host-runtime/src/runtime/model_lifecycle/unload.rs-43-60 (1)
43-60: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftDrain is awaited while the per-instance lifecycle mutex is held. Both unload paths lock
lifecycleand thenawaitDrainCoordinator::wait_for_unload_ready, keeping thetokio::sync::Mutexheld for up todrain_timeout.Node::begin_runtime_instance_requestlocks that same record for every ingress admission check on the port, so admission blocks for the whole drain instead of being rejected immediately.
crates/mesh-llm-host-runtime/src/runtime/model_lifecycle/unload.rs#L43-L60: mark draining under a short-lived guard, drop it, await the drain on cloned/Arcdrain state, then re-lock only fortransition_to_unloading.crates/mesh-llm-host-runtime/src/runtime/model_lifecycle/unload.rs#L193-L208: apply the identical restructuring inrun_auto_unload_runtime_entry.🤖 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/model_lifecycle/unload.rs` around lines 43 - 60, Both unload paths hold the per-instance lifecycle mutex across the asynchronous drain wait, blocking admission checks. In unload.rs lines 43-60 and 193-208, update the relevant unload flow and run_auto_unload_runtime_entry to mark draining under a short-lived lock, release it before calling DrainCoordinator::wait_for_unload_ready using cloned/Arc drain state, then re-lock only for transition_to_unloading; apply the same restructuring at both sites.crates/mesh-llm-host-runtime/src/mesh/owner_control/commands/model_lifecycle.rs-143-193 (1)
143-193: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidation error envelopes drop
request_id, breaking response correlation.Both helpers build
owner_control_error_envelope(..., None, ...), so every bad-request rejection fromexecute_load/execute_unload/execute_ensure/execute_drain(lines 18-21, 46-49, 77-80, 116-119) returns an error with norequest_id— unlike theControlUnavailable/drain-timeout paths which passSome(request_id). A client multiplexing owner-control requests cannot match these failures to the originating call and will likely time out instead.🐛 Proposed fix: thread `request_id` through the validators
fn extract_present_model_ref( + request_id: u64, model: Option<crate::proto::node::OwnerControlModelRef>, ) -> Result<crate::proto::node::OwnerControlModelRef, Box<OwnerControlEnvelope>> { match model { Some(ref_) => { if ref_.canonical_model_ref.trim().is_empty() || ref_.instance_id.is_some() { return Err(Box::new(owner_control_error_envelope( OwnerControlErrorCode::BadRequest, - None, + Some(request_id), None, "load and ensure require a canonical model reference only", ))); } Ok(ref_) } None => Err(Box::new(owner_control_error_envelope( OwnerControlErrorCode::BadRequest, - None, + Some(request_id), None, "model field is required", ))), } }Apply the same change to
extract_absent_model_refand update the four call sites to passrequest_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/owner_control/commands/model_lifecycle.rs` around lines 143 - 193, Thread request_id through extract_present_model_ref and extract_absent_model_ref, and pass it from execute_load, execute_unload, execute_ensure, and execute_drain at every validation call site. Use Some(request_id) when constructing validation error envelopes so rejected requests retain response correlation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d9479f8b-8ac4-4993-9099-46a6f3ee9037
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (106)
.github/workflows/ci.yml.github/workflows/pr_builds.ymlJustfilecrates/mesh-client/src/client/control_plane.rscrates/mesh-client/tests/control_plane_client.rscrates/mesh-client/tests/protocol_wire.rscrates/mesh-llm-cli/src/runtime.rscrates/mesh-llm-config/Cargo.tomlcrates/mesh-llm-config/src/lib.rscrates/mesh-llm-config/src/model.rscrates/mesh-llm-config/src/model/built_in_schema.rscrates/mesh-llm-config/src/model/built_in_schema/control_behavior/runtime_controls.rscrates/mesh-llm-config/src/model/runtime.rscrates/mesh-llm-gpu-bench/build.rscrates/mesh-llm-host-runtime/src/api/access.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/api/state.rscrates/mesh-llm-host-runtime/src/api/status.rscrates/mesh-llm-host-runtime/src/api/status/runtime.rscrates/mesh-llm-host-runtime/src/api/tests/mod.rscrates/mesh-llm-host-runtime/src/api/tests/node_state.rscrates/mesh-llm-host-runtime/src/api/tests/runtime_lifecycle_routes.rscrates/mesh-llm-host-runtime/src/api/tests/support.rscrates/mesh-llm-host-runtime/src/mesh/gossip.rscrates/mesh-llm-host-runtime/src/mesh/node.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/commands/mod.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/commands/model_lifecycle.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/owner_control_response.rscrates/mesh-llm-host-runtime/src/mesh/peer_state.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/control_plane_unique.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_frames.rscrates/mesh-llm-host-runtime/src/network/nostr/model_packs.rscrates/mesh-llm-host-runtime/src/network/openai/ingress.rscrates/mesh-llm-host-runtime/src/network/openai/response/routing.rscrates/mesh-llm-host-runtime/src/network/openai/transport.rscrates/mesh-llm-host-runtime/src/network/tunnel.rscrates/mesh-llm-host-runtime/src/protocol/control_frames.rscrates/mesh-llm-host-runtime/src/protocol/convert.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/mesh_timestamps.rscrates/mesh-llm-host-runtime/src/runtime/activity_policy.rscrates/mesh-llm-host-runtime/src/runtime/auto_join.rscrates/mesh-llm-host-runtime/src/runtime/control_loop.rscrates/mesh-llm-host-runtime/src/runtime/daemon_startup.rscrates/mesh-llm-host-runtime/src/runtime/instance_lifecycle.rscrates/mesh-llm-host-runtime/src/runtime/interactive.rscrates/mesh-llm-host-runtime/src/runtime/local.rscrates/mesh-llm-host-runtime/src/runtime/local_split/test_support.rscrates/mesh-llm-host-runtime/src/runtime/mod.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/load.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/reconciliation.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/unload.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/intent.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/mod.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/planner.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/state.rscrates/mesh-llm-host-runtime/src/runtime/publication.rscrates/mesh-llm-host-runtime/src/runtime/run_auto.rscrates/mesh-llm-host-runtime/src/runtime/serving_surface.rscrates/mesh-llm-host-runtime/src/runtime/startup_handles.rscrates/mesh-llm-host-runtime/src/runtime/startup_models.rscrates/mesh-llm-host-runtime/src/runtime/tests/startup_models.rscrates/mesh-llm-host-runtime/src/runtime_data/api_views.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-system/src/activity/linux.rscrates/mesh-llm-system/src/activity/macos.rscrates/mesh-llm-system/src/activity/mod.rscrates/mesh-llm-system/src/activity/unsupported.rscrates/mesh-llm-system/src/activity/windows.rscrates/mesh-llm-system/src/lib.rscrates/mesh-llm-ui/src/features/app-tabs/types.tscrates/mesh-llm-ui/src/features/configuration/api/config-adapter.tscrates/mesh-llm-ui/src/features/configuration/api/runtime-settings.test.tsxcrates/mesh-llm-ui/src/features/configuration/api/runtime-settings.tscrates/mesh-llm/src/commands/runtime.rscrates/mesh-llm/tests/protocol_convert_matrix.rsdocs/CLI.mddocs/MESHES.mddocs/USAGE.mddocs/design/TESTING.mddocs/design/message_protocol.mdscripts/build-linux.shscripts/build-llama.shscripts/ci-sdk-fixture.shscripts/lib/cuda-toolkit.shscripts/qa-control-plane-mixed-version.shscripts/qa-runtime-daemon-lifecycle.shscripts/tests/test_cuda_toolkit.pytools/xtask/src/main.rstools/xtask/src/repo_consistency.rswebsite/src/docs/pages/CLI.md
|
very nice. |
i386
left a comment
i386
left a comment
There was a problem hiding this comment.
Requesting changes because several lifecycle contracts can currently hang callers, reload an explicitly unloaded profiled model, block admission during drain, or fail to propagate the advertised admission state. I also found an owner-control idempotency race and a priority-restore state bug. Please address the inline findings and the existing unresolved ShellCheck finding for log_command. The current CI matrix is green, but these concurrency/profile/round-trip cases are not covered by it.
cb49f3e to
1894c44
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (9)
crates/mesh-llm-host-runtime/src/runtime/model_lifecycle.rs (2)
311-313: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
#[expect(dead_code, reason = "...")]instead of a bare#[allow(dead_code)].The rest of this PR (e.g.
model_reconciliation/intent.rs) consistently attaches an explicitreason, andexpectwill fire once the field gains a real reader.♻️ Proposed change
/// Per-instance lifecycle state machine for admission and drain control. - #[allow(dead_code)] + #[expect( + dead_code, + reason = "per-instance lifecycle record is consumed by admission/drain control wiring" + )] pub(super) lifecycle: std::sync::Arc<tokio::sync::Mutex<InstanceLifecycleRecord>>,As per coding guidelines: "do not use
#[allow(...)]to silence warnings without a clear reason and developer approval."🤖 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/model_lifecycle.rs` around lines 311 - 313, Replace the bare #[allow(dead_code)] on the lifecycle field in InstanceLifecycleRecord with #[expect(dead_code, reason = "...")], providing a concise reason consistent with the explicit expectation style used elsewhere in the PR.Source: Coding guidelines
209-221: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRound-tripping the error string through
anyhow!is redundant.
resultalready carries aString; wrapping it only to call.to_string()on line 217 recreates the same text. Onlynotify_load_failureneeds an&anyhow::Error.♻️ Proposed tidy
Err(error) => { - let error = anyhow::anyhow!(error); state.record_load_failure(model_ref, profile, now_secs, policy); - state.set_effective_intent_error(model_ref, profile, error.to_string()); - state.notify_load_failure(model_ref, profile, &error); + state.set_effective_intent_error(model_ref, profile, error.clone()); + state.notify_load_failure(model_ref, profile, &anyhow::anyhow!(error)); }🤖 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/model_lifecycle.rs` around lines 209 - 221, In the Err branch of the result handling, keep the original error String for record_load_failure and set_effective_intent_error, avoiding the redundant anyhow::anyhow! conversion and error.to_string() round trip. Create an anyhow::Error only at the notify_load_failure call, passing it by reference as required.crates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache/tests.rs (1)
213-239: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMissing coverage: same
request_idfrom different requesters must not collide.The key includes
requester, but nothing asserts it. A one-liner test (twoendpoint_idseeds, samerequest_id, both must lead) would lock that in.🤖 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_lifecycle_cache/tests.rs` around lines 213 - 239, The tests for OwnerLifecycleResponseCache lack coverage that requester identity participates in cache keys. Add a focused test near completed_lifecycle_responses_are_bounded using two distinct endpoint_id seeds with the same request_id, and assert cache.reserve returns Leader for both requester values.crates/mesh-llm-host-runtime/src/mesh/owner_control/mod.rs (1)
958-1001: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFour lifecycle arms are byte-identical except for the
execute_*call.Consider collapsing to a single arm that resolves the envelope future first, then shares the publish/send tail.
♻️ Sketch
- OwnedNodeCommand::LoadModel { request_id, request } => { - let envelope = - commands::model_lifecycle::execute_load(self, request_id, request).await; - if let Some(leader) = lifecycle_leader.take() { - leader.publish(envelope.clone()); - } - self.send_owner_control_envelope(send, envelope).await?; - } - // ...three more identical arms + lifecycle @ (OwnedNodeCommand::LoadModel { .. } + | OwnedNodeCommand::UnloadModel { .. } + | OwnedNodeCommand::EnsureModel { .. } + | OwnedNodeCommand::DrainModel { .. }) => { + let envelope = commands::model_lifecycle::execute(self, lifecycle).await; + if let Some(leader) = lifecycle_leader.take() { + leader.publish(envelope.clone()); + } + self.send_owner_control_envelope(send, envelope).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/owner_control/mod.rs` around lines 958 - 1001, Collapse the LoadModel, UnloadModel, EnsureModel, and DrainModel branches in the owner command dispatch into one shared lifecycle arm that selects and awaits the appropriate execute_* function, then reuses the common lifecycle_leader.publish and send_owner_control_envelope flow. Preserve each command’s existing request_id/request handling and executor mapping.crates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache.rs (1)
118-136: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
complete_keyunconditionally overwrites aReadyentry.Currently unreachable (a published leader's
Dropis a no-op), but if a fallback path ever fires after a real publish it would silently replace the authoritative response. Cheap to make explicit.🛡️ Proposed guard
let notify = match entry { OwnerLifecycleResponseEntry::Pending { tx, .. } => Some(tx.clone()), - OwnerLifecycleResponseEntry::Ready { .. } => None, + OwnerLifecycleResponseEntry::Ready { .. } => return true, };🤖 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_lifecycle_cache.rs` around lines 118 - 136, Update complete_key so it only transitions a Pending OwnerLifecycleResponseEntry to Ready and returns true after notifying its waiter; if the matching entry is already Ready, leave its authoritative envelope unchanged and return false. Preserve the existing missing-key behavior and synchronization flow.crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/intent.rs (1)
146-166: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
dedup_keyis dead code whose Unload keying is already known-wrong.Model names and instance ids share one key space with an empty profile, so a model named the same as an instance id would collide — the inline comment admits dedup actually happens at execution time. Prefer deleting it (and the
#[expect(dead_code)]) until a real consumer exists, rather than shipping a placeholder that a future caller may trust.🤖 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/model_reconciliation/intent.rs` around lines 146 - 166, Remove the unused ModelIntent::dedup_key method and its #[expect(dead_code)] annotation. Also delete its placeholder UnloadTarget model/instance keying logic and related comments, leaving deduplication to the existing execution-time path.scripts/qa-control-plane-mixed-version.sh (1)
1060-1063: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
auth_logis now dead.Since the owner keystore is created up front (line 619+), this local is never used; shellcheck flags SC2034. Drop it.
♻️ Proposed cleanup
local wrong_owner_key="$WORK_ROOT/wrong-owner-keystore.json" - local auth_log="$LOG_DIR/config-auth-init.log" local wrong_auth_log="$LOG_DIR/config-wrong-owner-auth-init.log"🤖 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 `@scripts/qa-control-plane-mixed-version.sh` around lines 1060 - 1063, Remove the unused auth_log local declaration from the surrounding setup block, while preserving owner_key, wrong_owner_key, and wrong_auth_log unchanged.Source: Linters/SAST tools
docs/design/TESTING.md (1)
950-950: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a language to the fence (MD040).
📝 Proposed fix
-``` +```text .sisyphus/evidence/runtime-daemon-lifecycle-20260724T123456Z-1234/🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/TESTING.md` at line 950, Update the fenced code block containing the .sisyphus evidence path in TESTING.md to specify the text language, using a ```text fence while preserving the block contents.Source: Linters/SAST tools
scripts/tests/test_cuda_toolkit.py (1)
79-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider asserting the helper's exit status.
run_bashhere only checksprintf's status, soconfigure_cuda_toolkit_envreturning non-zero (which it currently does whenever no matching lib dir exists — seescripts/lib/cuda-toolkit.sh) goes unnoticed. Addingconfigure_cuda_toolkit_env || exit 3to the inline script would pin the contract once the explicitreturn 0is in place.🤖 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 `@scripts/tests/test_cuda_toolkit.py` around lines 79 - 89, Update the inline bash script in the test around run_bash to propagate configure_cuda_toolkit_env failures by exiting immediately with status 3 when the helper returns non-zero, before printing the environment variables. Keep the existing output assertions and success-status check unchanged.
🤖 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-config/src/model/runtime.rs`:
- Around line 144-164: Update validate_config for RuntimeActivityConfig so it
enforces the documented ranges for idle_after_secs, poll_interval_secs, and
resume_debounce_secs, rejecting values outside 30..=86400, 1..=60, and 0..=300
respectively. Reuse the existing built-in control behavior schema or add an
equivalent runtime activity validation path, ensuring serialized
poll_interval_secs = 0 is rejected.
In `@crates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache.rs`:
- Around line 79-116: Update the owner lifecycle cache lookup in the reservation
method containing OwnerLifecycleCacheKey so Ready entries are only reused while
their command deadline remains valid. Store or otherwise retain the response
deadline when publishing and compare it against the current time before
returning OwnerLifecycleResponseReservation::Ready; expired entries must be
removed or bypassed so the request follows normal execution with its own
deadline, while Pending coordination remains unchanged.
In `@crates/mesh-llm-ui/e2e/perf/meshviz-200.spec.ts`:
- Around line 178-183: Move the waitForLivePacketLayerScale(page) call in the
wheel interaction test to after the four page.mouse.wheel calls, so the promise
begins observing only after the wheel-driven update is triggered. Keep the
existing liveScaleTransform assertion and hover setup unchanged.
In `@docs/design/TESTING.md`:
- Around line 917-921: Update the lifecycle and activity entries in the testing
table to match qa-runtime-daemon-lifecycle.sh: use the authenticated
owner-control routes POST /api/runtime/control/{load,unload,ensure,drain}-model
with {"endpoint":"...","model":"..."} for lifecycle checks, and document the
activity override request as a bare JSON string ("active") rather than an
object.
In `@scripts/lib/cuda-toolkit.sh`:
- Around line 91-101: Ensure CUDA environment configuration reports success
after applying variables, even when optional toolkit or library paths are
unresolved: add an explicit successful return to configure_cuda_toolkit_env in
scripts/lib/cuda-toolkit.sh (lines 91-101). In both branches of
scripts/build-linux.sh (lines 166-175), invoke configure_cuda_toolkit_env with
failure suppression and explicitly return success afterward, matching
build-llama.sh so locate_nvcc does not report a false failure.
- Around line 11-16: Update cuda_canonical_path() to provide a portable fallback
when readlink -f and realpath are unavailable, especially on older macOS/BSD
systems. Normalize the supplied path using an existing cross-platform mechanism
such as Python or an available GNU coreutils utility before allowing the helper
to return an uncanonicalized path, preserving the current fast paths where
supported.
In `@scripts/qa-control-plane-mixed-version.sh`:
- Around line 1378-1385: Update run_lifecycle_probes so the no-current-host path
records PREREQ for all five planned lifecycle checks, including
lifecycle-legacy-unsupported, before returning. Preserve the existing skip
message pattern and ensure results.jsonl remains consistent with the plan and
documentation.
In `@scripts/qa-runtime-daemon-lifecycle.sh`:
- Around line 357-366: Update cleanup() to safely handle an empty PIDS array
under set -u on bash 3.2 by guarding each PIDS iteration or using a nounset-safe
expansion such as "${PIDS[@]:-}". Preserve the existing kill_tree and
liveness-check behavior when PIDS contains entries.
- Around line 196-198: Update the required-tool declarations near the existing
require_tool calls to include both python3 and pgrep, ensuring prerequisite
validation covers the commands used by result/manifest/summary handling and
teardown/kill_tree.
- Around line 993-996: In the graceful-stop loop, replace the full-path
`${home_dir/h-/r-}` substitution with runtime-root derivation based only on the
basename of `home_dir`, preserving its parent directory and converting the home
directory name from h- to r-. Remove the unused `leaked` variable and its
computation near the loop.
---
Nitpick comments:
In `@crates/mesh-llm-host-runtime/src/mesh/owner_control/mod.rs`:
- Around line 958-1001: Collapse the LoadModel, UnloadModel, EnsureModel, and
DrainModel branches in the owner command dispatch into one shared lifecycle arm
that selects and awaits the appropriate execute_* function, then reuses the
common lifecycle_leader.publish and send_owner_control_envelope flow. Preserve
each command’s existing request_id/request handling and executor mapping.
In `@crates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache.rs`:
- Around line 118-136: Update complete_key so it only transitions a Pending
OwnerLifecycleResponseEntry to Ready and returns true after notifying its
waiter; if the matching entry is already Ready, leave its authoritative envelope
unchanged and return false. Preserve the existing missing-key behavior and
synchronization flow.
In `@crates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache/tests.rs`:
- Around line 213-239: The tests for OwnerLifecycleResponseCache lack coverage
that requester identity participates in cache keys. Add a focused test near
completed_lifecycle_responses_are_bounded using two distinct endpoint_id seeds
with the same request_id, and assert cache.reserve returns Leader for both
requester values.
In `@crates/mesh-llm-host-runtime/src/runtime/model_lifecycle.rs`:
- Around line 311-313: Replace the bare #[allow(dead_code)] on the lifecycle
field in InstanceLifecycleRecord with #[expect(dead_code, reason = "...")],
providing a concise reason consistent with the explicit expectation style used
elsewhere in the PR.
- Around line 209-221: In the Err branch of the result handling, keep the
original error String for record_load_failure and set_effective_intent_error,
avoiding the redundant anyhow::anyhow! conversion and error.to_string() round
trip. Create an anyhow::Error only at the notify_load_failure call, passing it
by reference as required.
In `@crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/intent.rs`:
- Around line 146-166: Remove the unused ModelIntent::dedup_key method and its
#[expect(dead_code)] annotation. Also delete its placeholder UnloadTarget
model/instance keying logic and related comments, leaving deduplication to the
existing execution-time path.
In `@docs/design/TESTING.md`:
- Line 950: Update the fenced code block containing the .sisyphus evidence path
in TESTING.md to specify the text language, using a ```text fence while
preserving the block contents.
In `@scripts/qa-control-plane-mixed-version.sh`:
- Around line 1060-1063: Remove the unused auth_log local declaration from the
surrounding setup block, while preserving owner_key, wrong_owner_key, and
wrong_auth_log unchanged.
In `@scripts/tests/test_cuda_toolkit.py`:
- Around line 79-89: Update the inline bash script in the test around run_bash
to propagate configure_cuda_toolkit_env failures by exiting immediately with
status 3 when the helper returns non-zero, before printing the environment
variables. Keep the existing output assertions and success-status check
unchanged.
🪄 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: 5a1864a2-ad9a-4fe2-979d-37904e783243
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (109)
Justfilecrates/mesh-client/src/client/control_plane.rscrates/mesh-client/tests/control_plane_client.rscrates/mesh-client/tests/protocol_wire.rscrates/mesh-llm-cli/src/runtime.rscrates/mesh-llm-config/Cargo.tomlcrates/mesh-llm-config/src/lib.rscrates/mesh-llm-config/src/model.rscrates/mesh-llm-config/src/model/built_in_schema.rscrates/mesh-llm-config/src/model/built_in_schema/control_behavior/runtime_controls.rscrates/mesh-llm-config/src/model/runtime.rscrates/mesh-llm-gpu-bench/build.rscrates/mesh-llm-host-runtime/src/api/access.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/api/state.rscrates/mesh-llm-host-runtime/src/api/status.rscrates/mesh-llm-host-runtime/src/api/status/runtime.rscrates/mesh-llm-host-runtime/src/api/tests/mod.rscrates/mesh-llm-host-runtime/src/api/tests/node_state.rscrates/mesh-llm-host-runtime/src/api/tests/runtime_lifecycle_routes.rscrates/mesh-llm-host-runtime/src/api/tests/support.rscrates/mesh-llm-host-runtime/src/mesh/gossip.rscrates/mesh-llm-host-runtime/src/mesh/mod.rscrates/mesh-llm-host-runtime/src/mesh/node.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/commands/mod.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/commands/model_lifecycle.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/owner_control_response.rscrates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache.rscrates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache/tests.rscrates/mesh-llm-host-runtime/src/mesh/peer_state.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/control_plane_unique.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_frames.rscrates/mesh-llm-host-runtime/src/network/nostr/model_packs.rscrates/mesh-llm-host-runtime/src/network/openai/ingress.rscrates/mesh-llm-host-runtime/src/network/openai/response/routing.rscrates/mesh-llm-host-runtime/src/network/openai/transport.rscrates/mesh-llm-host-runtime/src/network/tunnel.rscrates/mesh-llm-host-runtime/src/protocol/control_frames.rscrates/mesh-llm-host-runtime/src/protocol/convert.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/mesh_timestamps.rscrates/mesh-llm-host-runtime/src/runtime/activity_policy.rscrates/mesh-llm-host-runtime/src/runtime/auto_join.rscrates/mesh-llm-host-runtime/src/runtime/control_loop.rscrates/mesh-llm-host-runtime/src/runtime/daemon_startup.rscrates/mesh-llm-host-runtime/src/runtime/instance_lifecycle.rscrates/mesh-llm-host-runtime/src/runtime/interactive.rscrates/mesh-llm-host-runtime/src/runtime/local.rscrates/mesh-llm-host-runtime/src/runtime/local_split/test_support.rscrates/mesh-llm-host-runtime/src/runtime/mod.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/load.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/reconciliation.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/unload.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/intent.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/mod.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/planner.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/state.rscrates/mesh-llm-host-runtime/src/runtime/publication.rscrates/mesh-llm-host-runtime/src/runtime/run_auto.rscrates/mesh-llm-host-runtime/src/runtime/serving_surface.rscrates/mesh-llm-host-runtime/src/runtime/startup_handles.rscrates/mesh-llm-host-runtime/src/runtime/startup_models.rscrates/mesh-llm-host-runtime/src/runtime/tests/model_lifecycle.rscrates/mesh-llm-host-runtime/src/runtime/tests/startup_models.rscrates/mesh-llm-host-runtime/src/runtime_data/api_views.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-system/src/activity/linux.rscrates/mesh-llm-system/src/activity/macos.rscrates/mesh-llm-system/src/activity/mod.rscrates/mesh-llm-system/src/activity/unsupported.rscrates/mesh-llm-system/src/activity/windows.rscrates/mesh-llm-system/src/lib.rscrates/mesh-llm-ui/e2e/perf/meshviz-200.spec.tscrates/mesh-llm-ui/src/features/app-tabs/types.tscrates/mesh-llm-ui/src/features/configuration/api/config-adapter.tscrates/mesh-llm-ui/src/features/configuration/api/runtime-settings.test.tsxcrates/mesh-llm-ui/src/features/configuration/api/runtime-settings.tscrates/mesh-llm/src/commands/runtime.rscrates/mesh-llm/tests/protocol_convert_matrix.rsdocs/CLI.mddocs/MESHES.mddocs/USAGE.mddocs/design/TESTING.mddocs/design/message_protocol.mdscripts/build-linux.shscripts/build-llama.shscripts/ci-sdk-fixture.shscripts/lib/cuda-toolkit.shscripts/qa-control-plane-mixed-version.shscripts/qa-runtime-daemon-lifecycle.shscripts/tests/test_cuda_toolkit.pytools/xtask/src/main.rstools/xtask/src/repo_consistency.rswebsite/src/docs/pages/CLI.md
🚧 Files skipped from review as they are similar to previous changes (84)
- crates/mesh-llm-system/src/activity/windows.rs
- crates/mesh-llm-host-runtime/src/protocol/tests/mesh_timestamps.rs
- crates/mesh-llm-ui/src/features/app-tabs/types.ts
- crates/mesh-llm-host-runtime/src/mesh/owner_control/commands/scan_refresh.rs
- crates/mesh-llm-host-runtime/src/mesh/owner_control_response.rs
- crates/mesh-llm-host-runtime/src/api/tests/node_state.rs
- crates/mesh-llm-host-runtime/src/protocol/tests.rs
- crates/mesh-llm-host-runtime/src/runtime/publication.rs
- crates/mesh-llm-host-runtime/src/runtime/local_split/test_support.rs
- crates/mesh-llm-host-runtime/src/api/tests/mod.rs
- website/src/docs/pages/CLI.md
- crates/mesh-llm-system/src/lib.rs
- crates/mesh-llm/tests/protocol_convert_matrix.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/control_plane_unique.rs
- crates/mesh-llm-host-runtime/src/api/state.rs
- crates/mesh-llm-host-runtime/src/network/openai/transport.rs
- crates/mesh-client/tests/protocol_wire.rs
- crates/mesh-llm-config/src/model/built_in_schema.rs
- crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/mod.rs
- docs/CLI.md
- crates/mesh-llm-system/src/activity/unsupported.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/admission/helpers.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/connections.rs
- crates/mesh-llm-host-runtime/src/api/tests/runtime_lifecycle_routes.rs
- docs/MESHES.md
- crates/mesh-llm-config/src/lib.rs
- crates/mesh-llm-host-runtime/src/runtime_data/api_views.rs
- crates/mesh-llm-host-runtime/src/api/access.rs
- crates/mesh-llm-host-runtime/src/api/tests/support.rs
- crates/mesh-llm-config/Cargo.toml
- crates/mesh-llm-host-runtime/src/mesh/tests/peer_state.rs
- scripts/ci-sdk-fixture.sh
- crates/mesh-llm-host-runtime/src/runtime_data/mod.rs
- crates/mesh-llm-config/src/model.rs
- crates/mesh-llm-host-runtime/src/network/openai/response/routing.rs
- crates/mesh-llm-host-runtime/src/api/mod.rs
- crates/mesh-llm-host-runtime/src/network/nostr/model_packs.rs
- crates/mesh-llm-ui/src/features/configuration/api/runtime-settings.test.tsx
- crates/mesh-llm-ui/src/features/configuration/api/runtime-settings.ts
- crates/mesh-llm-host-runtime/src/runtime/auto_join.rs
- crates/mesh-llm-host-runtime/src/runtime/interactive.rs
- docs/USAGE.md
- docs/design/message_protocol.md
- crates/mesh-llm-host-runtime/src/network/tunnel.rs
- crates/mesh-llm-cli/src/runtime.rs
- crates/mesh-llm-system/src/activity/macos.rs
- crates/mesh-llm-config/src/model/built_in_schema/control_behavior/runtime_controls.rs
- crates/mesh-llm-host-runtime/src/mesh/owner_control/commands/model_lifecycle.rs
- crates/mesh-llm-host-runtime/src/mesh/peer_state.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/gossip.rs
- crates/mesh-llm-protocol/src/proto/node.rs
- crates/mesh-llm/src/commands/runtime.rs
- crates/mesh-llm-host-runtime/src/protocol/control_frames.rs
- crates/mesh-llm-host-runtime/src/runtime/model_lifecycle/unload.rs
- crates/mesh-llm-host-runtime/src/api/status.rs
- crates/mesh-llm-host-runtime/src/runtime/model_lifecycle/reconciliation.rs
- crates/mesh-llm-host-runtime/src/runtime/daemon_startup.rs
- crates/mesh-llm-host-runtime/src/runtime/model_lifecycle/load.rs
- crates/mesh-llm-host-runtime/src/mesh/gossip.rs
- crates/mesh-llm-protocol/proto/node.proto
- crates/mesh-llm-system/src/activity/linux.rs
- crates/mesh-llm-host-runtime/src/api/status/runtime.rs
- crates/mesh-llm-host-runtime/src/runtime/mod.rs
- crates/mesh-client/tests/control_plane_client.rs
- crates/mesh-client/src/client/control_plane.rs
- crates/mesh-llm-host-runtime/src/api/routes/runtime.rs
- tools/xtask/src/repo_consistency.rs
- Justfile
- crates/mesh-llm-host-runtime/src/runtime/control_loop.rs
- crates/mesh-llm-host-runtime/src/network/openai/ingress.rs
- crates/mesh-llm-host-runtime/src/runtime/instance_lifecycle.rs
- crates/mesh-llm-system/src/activity/mod.rs
- crates/mesh-llm-host-runtime/src/runtime/startup_models.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/owner_control.rs
- crates/mesh-llm-host-runtime/src/runtime/activity_policy.rs
- crates/mesh-llm-host-runtime/src/mesh/owner_control/commands/mod.rs
- crates/mesh-llm-host-runtime/src/runtime/serving_surface.rs
- crates/mesh-llm-host-runtime/src/mesh/node.rs
- crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/planner.rs
- crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/state.rs
- crates/mesh-llm-host-runtime/src/runtime/startup_handles.rs
- crates/mesh-llm-protocol/src/protocol/mod.rs
- crates/mesh-llm-host-runtime/src/runtime/tests/startup_models.rs
- crates/mesh-llm-host-runtime/src/runtime/run_auto.rs
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/tests/test_cuda_toolkit.py (1)
52-55: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winQuote the toolkit script path before embedding it in Bash.
CUDA_TOOLKIT_LIBis unquoted in threesourcecommands, so checkouts located under paths containing spaces or shell metacharacters can fail before testing CUDA behavior. Apply the sameshlex.quote(str(CUDA_TOOLKIT_LIB))pattern already used at Line 90.Proposed fix
- source {CUDA_TOOLKIT_LIB} + source {shlex.quote(str(CUDA_TOOLKIT_LIB))}Apply this change at Lines 54, 108, and 128.
Also applies to: 106-109, 126-129
🤖 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 `@scripts/tests/test_cuda_toolkit.py` around lines 52 - 55, Quote the CUDA toolkit script path in each Bash snippet using the existing shlex.quote(str(CUDA_TOOLKIT_LIB)) pattern. Update the source commands in the test cases around run_bash at the three indicated locations, preserving the surrounding CUDA environment setup and test behavior.
🧹 Nitpick comments (1)
scripts/tests/test_cuda_toolkit.py (1)
42-58: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert
LD_LIBRARY_PATHpropagation too.
configure_cuda_toolkit_envprepends the resolved library directory to bothLIBRARY_PATHandLD_LIBRARY_PATH, but this test verifies only the former. Add the missing variable to prevent regressions in the runtime library-path export.Proposed fix
"CUDA_LIBRARY_PATH", "LIBRARY_PATH", + "LD_LIBRARY_PATH", ... - printf '%s\n%s\n%s\n%s\n%s\n' \ + printf '%s\n%s\n%s\n%s\n%s\n%s\n' \ "$CUDACXX" "$CUDAToolkit_ROOT" "$NVCC" \ - "$CUDA_LIBRARY_PATH" "$LIBRARY_PATH" + "$CUDA_LIBRARY_PATH" "$LIBRARY_PATH" "$LD_LIBRARY_PATH" ... str(library_dir.resolve()), + str(library_dir.resolve()),Also applies to: 64-70
🤖 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 `@scripts/tests/test_cuda_toolkit.py` around lines 42 - 58, Extend the configure_cuda_toolkit_env test in the result output and assertions to include LD_LIBRARY_PATH alongside LIBRARY_PATH. Preserve the existing ordering and verify that the resolved CUDA library directory is propagated to both variables.
🤖 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 `@scripts/tests/test_cuda_toolkit.py`:
- Around line 102-104: Update
test_propagates_helper_failure_when_nvcc_is_missing to remove inherited CUDACXX
before invoking the CUDA toolkit configuration, ensuring the test exercises the
missing-nvcc lookup path. For full environment isolation, also clear the other
CUDA-related environment variables used by configure_cuda_toolkit_env while
preserving the empty PATH setup.
---
Outside diff comments:
In `@scripts/tests/test_cuda_toolkit.py`:
- Around line 52-55: Quote the CUDA toolkit script path in each Bash snippet
using the existing shlex.quote(str(CUDA_TOOLKIT_LIB)) pattern. Update the source
commands in the test cases around run_bash at the three indicated locations,
preserving the surrounding CUDA environment setup and test behavior.
---
Nitpick comments:
In `@scripts/tests/test_cuda_toolkit.py`:
- Around line 42-58: Extend the configure_cuda_toolkit_env test in the result
output and assertions to include LD_LIBRARY_PATH alongside LIBRARY_PATH.
Preserve the existing ordering and verify that the resolved CUDA library
directory is propagated to both variables.
🪄 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: 57cb5bde-748e-4a4b-a95f-c4769304a552
📒 Files selected for processing (13)
crates/mesh-llm-config/src/validate.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/mod.rscrates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache.rscrates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache/tests.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/intent.rscrates/mesh-llm-ui/e2e/perf/meshviz-200.spec.tsdocs/design/TESTING.mdscripts/build-linux.shscripts/lib/cuda-toolkit.shscripts/qa-control-plane-mixed-version.shscripts/qa-runtime-daemon-lifecycle.shscripts/tests/test_cuda_toolkit.py
💤 Files with no reviewable changes (1)
- crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/intent.rs
🚧 Files skipped from review as they are similar to previous changes (9)
- scripts/build-linux.sh
- crates/mesh-llm-ui/e2e/perf/meshviz-200.spec.ts
- crates/mesh-llm-host-runtime/src/mesh/owner_control/mod.rs
- scripts/lib/cuda-toolkit.sh
- docs/design/TESTING.md
- crates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache.rs
- scripts/qa-control-plane-mixed-version.sh
- scripts/qa-runtime-daemon-lifecycle.sh
- crates/mesh-llm-host-runtime/src/runtime/model_lifecycle.rs
05a1e22 to
8892d54
Compare
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (10)
crates/mesh-llm-host-runtime/src/runtime/mod.rs (2)
43-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBlanket
#[allow(unused_imports)]over crate-root re-export blocks. Both blocks suppress the very warning that would tell you which re-exports are unnecessary, working against the guideline to keep crate-root re-exports minimal.
crates/mesh-llm-host-runtime/src/runtime/mod.rs#L43-L47: drop the suppression and keep only theinstance_lifecycletypes with real consumers.crates/mesh-llm-host-runtime/src/runtime/mod.rs#L60-L67: drop the suppression and keep only themodel_reconciliationtypes with real consumers.As per coding guidelines: "Minimize crate-root re-exports; new code should import from the owning module directly".
🤖 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/mod.rs` around lines 43 - 47, Remove the blanket #[allow(unused_imports)] attributes from both crate-root re-export blocks in crates/mesh-llm-host-runtime/src/runtime/mod.rs:43-47 and crates/mesh-llm-host-runtime/src/runtime/mod.rs:60-67, then remove any unused symbols from the instance_lifecycle and model_reconciliation re-exports respectively. Retain only types with real consumers and have new code import directly from the owning modules.Source: Coding guidelines
75-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale comment.
The comment says the types are accessed via
crate::runtime::activity_policy::*, but the next line re-exports them at the module root. Drop the comment or the re-export.🤖 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/mod.rs` around lines 75 - 76, Remove the stale comment above the activity_policy re-export, or remove the pub(crate) re-export itself; ensure the remaining API and its documented access path are consistent. Prefer retaining the existing re-export from the runtime module root unless it is no longer needed.crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/state.rs (1)
387-398: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
suppressedignoresprofilefor cooldown checks.
compound_keyis profile-scoped for the in-flight check, butcooldown_activematches on model identity only, so a failed/manually-unloaded load of one profile suppresses every other profile of the same model. If that is intended, a short comment would help; otherwise pass and compare the profile too.🤖 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/model_reconciliation/state.rs` around lines 387 - 398, Update suppressed and cooldown_active so failed_models and manual_unload_models cooldown checks include profile in their matching criteria, preserving profile-scoped suppression consistently with the compound_key in_flight check. Ensure a cooldown for one profile does not suppress other profiles of the same model.crates/mesh-llm-host-runtime/src/runtime/daemon_startup.rs (1)
50-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the bare
#[allow]with#[expect(..., reason = "...")].The rest of this PR consistently uses
#[expect]with an explicit reason; this lint suppression carries none. Alternatively, construct the test options with struct-update syntax and drop the suppression entirely.As per coding guidelines: "do not use
#[allow(...)]to silence warnings without a clear reason and developer approval".♻️ Proposed change
#[cfg(test)] -#[allow(clippy::field_reassign_with_default)] +#[expect( + clippy::field_reassign_with_default, + reason = "tests tweak individual RuntimeOptions fields for readability" +)] mod tests {🤖 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/daemon_startup.rs` at line 50, Replace the bare #[allow(clippy::field_reassign_with_default)] near the test-options construction with #[expect(..., reason = "...")] and provide a clear justification, or rewrite that construction using struct-update syntax and remove the suppression entirely. Follow the surrounding #[expect] convention.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/runtime/model_lifecycle/unload.rs (2)
5-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffTwo cognitive-complexity suppressions in one file.
Both unload paths repeat the same ordered sequence (drain → transition → deregister → telemetry/dashboard cleanup → event). Extracting the shared drain-and-transition step and the dashboard/console cleanup into named helpers would remove the need for the suppressions.
As per coding guidelines: "Do not add Rust methods or functions over the configured Clippy line-count or cognitive-complexity limits; split complex logic into semantically named helpers."
Also applies to: 132-135
🤖 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/model_lifecycle/unload.rs` around lines 5 - 8, Remove both clippy::cognitive_complexity suppressions by refactoring the unload paths into semantically named helpers: extract the shared ordered drain-and-transition operation, and extract dashboard/console cleanup. Update each unload flow to call these helpers while preserving the existing ordering through deregistration, telemetry cleanup, and event emission.Source: Coding guidelines
68-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant double lock acquisition.
lifecycle.lock().await.state()then re-locking to transition opens a needless TOCTOU window. Hold one guard for the check and the transition.♻️ Proposed change
- if lifecycle.lock().await.state() == InstanceLifecycleState::Unloading { - lifecycle - .lock() - .await - .transition_to(InstanceLifecycleState::Stopped) - .map_err(|error| anyhow::anyhow!(error))?; - } + { + let mut record = lifecycle.lock().await; + if record.state() == InstanceLifecycleState::Unloading { + record + .transition_to(InstanceLifecycleState::Stopped) + .map_err(|error| anyhow::anyhow!(error))?; + } + }🤖 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/model_lifecycle/unload.rs` around lines 68 - 74, Update the lifecycle handling around the state check and transition to acquire a single lock guard, use it to check whether the state is Unloading, and perform transition_to on that same guard. Preserve the existing Stopped transition and error propagation.scripts/qa-runtime-daemon-lifecycle.sh (3)
488-524: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead helpers:
json_field,json_len,json_queryare never called.All JSON extraction in the harness is done with inline
python3heredocs, so these three functions are unused. Drop them (or start using them in place of the inline heredocs) to avoid drift.🤖 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 `@scripts/qa-runtime-daemon-lifecycle.sh` around lines 488 - 524, Remove the unused json_field, json_len, and json_query helper functions from the harness, leaving the existing inline python3 JSON extraction logic unchanged.Source: Linters/SAST tools
729-733: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid globally toggling
errexit; capture the status inline.- local exit_code=0 - set +e - wait "$pid" - exit_code=$? - set -e + local exit_code=0 + wait "$pid" || exit_code=$?🤖 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 `@scripts/qa-runtime-daemon-lifecycle.sh` around lines 729 - 733, Update the wait logic around wait "$pid" to capture its exit status inline without disabling and re-enabling errexit via set +e and set -e. Preserve the existing exit_code assignment and subsequent error-handling behavior.Source: Linters/SAST tools
605-609: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueComment promises a mode assertion that isn't performed.
Line 605 says the status is verified to show serve mode, but the check records PASS based only on process liveness. Either assert the mode field from
$status_fileor reword the comment.🤖 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 `@scripts/qa-runtime-daemon-lifecycle.sh` around lines 605 - 609, The serve-mode verification in the runtime lifecycle check records PASS without inspecting the status file. Update the check around status_file and record_result to parse and assert that the status mode field is "serve" before reporting runtime_mode_serve as passing; retain the existing liveness/API validation separately.crates/mesh-llm-host-runtime/src/api/routes/runtime_activity.rs (1)
31-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCollapse the duplicated
ActivityOverrideMode⇄ManualActivityOverridemapping intoFromimpls.The same three-arm match appears twice; a pair of
Fromimpls keeps the two directions adjacent and prevents them drifting when a fourth mode is added.Also applies to: 71-75
🤖 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/api/routes/runtime_activity.rs` around lines 31 - 38, Replace the duplicated three-variant mappings in the runtime activity handler with adjacent From implementations between ActivityOverrideMode and ManualActivityOverride. Update both conversion sites to use these conversions while preserving the existing invalid-input response and all variant mappings.
🤖 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/owner_lifecycle_cache.rs`:
- Around line 44-49: Update OwnerLifecycleCacheEntry::is_live_ready_at to
evaluate Pending entries using their stored expires_at timestamp, returning true
only while the deadline is in the future. Preserve the existing Ready expiry
behavior so stale pending entries can be removed and followers use the fallback
path after expiry.
In `@crates/mesh-llm-host-runtime/src/runtime/activity_policy.rs`:
- Line 35: Review every #[allow(dead_code)] attribute in the activity policy
implementation, including the methods used by runtime_activity routes and guard
logic such as effective_state, manual_override, detector_state, to_proto,
is_blocking, apply_manual_override, and update_detector_state. Remove attributes
that are no longer needed; retain only genuinely unused API surface and add a
concise rationale comment explaining its intended future use, such as reserved
CLI wiring.
- Around line 466-495: Move the synchronous work in the tokio::spawn loop off
the async executor by wrapping monitor.sample() and priority.reduce()/restore()
in tokio::task::spawn_blocking, while preserving the existing interval cadence
and guard state updates. Handle the blocking task result so sampling or priority
failures do not break the monitoring loop.
In `@crates/mesh-llm-host-runtime/src/runtime/model_lifecycle/unload.rs`:
- Around line 28-74: Ensure unload transition failures cannot orphan running
instances after removal from their registries. In unload.rs lines 28-74, update
the flow around ManagedModelController to either reinsert the controller on
mark_draining or transition_to_unloading failure, or continue through
stop_tx.send(true), await_managed_model_stop, and
unregister_runtime_instance_lifecycle. Apply the same handling at lines 141-160
for RuntimeModelHandleEntry: reinsert it on lifecycle transition errors or still
execute handle.shutdown() and registry/dashboard cleanup.
In `@crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/state.rs`:
- Around line 216-235: The effective intent lookup in effective_intent() uses
exact canonical_model_ref matching while suppression state uses
model_identity_matches(), causing equivalent model references to disagree.
Replace the direct model_ref comparison in effective_intent() and any related
effective-state lookup paths with the existing model identity matching behavior,
while preserving profile and retired-intent filtering.
- Around line 195-214: Update record_desired_intent_with_id() so active
maintained intents (such as StartupConfig or OwnerEnsure) remain selectable in
intent_history despite the 256-entry cap. Deduplicate replacement entries for
the same (model_ref, profile), or adjust eviction to skip effective non-retired
maintained intents while evicting ephemeral MeshDemand entries; preserve
retired_intents cleanup and history publication.
In `@crates/mesh-llm-system/src/activity/macos.rs`:
- Around line 64-85: Update macOS reduce_priority to remove the geteuid root
check and use libc::getpriority/setpriority with PRIO_PROCESS and the process id
type. Preserve the existing one-time original_nice tracking and increase the
nice value for non-root users; ensure restoration failures do not block the
reduction attempt and the caller still records degraded state.
In `@docs/design/TESTING.md`:
- Around line 911-923: Update the testing table entries for
zero_model_serve_ready, fail_fast_startup, best_effort_startup, and
privacy_no_raw_data to match the harness commands, explicit startup-failure
policy configuration, and exact activity response assertions. Add the missing
prereq.owner-identity row using the harness plan’s description.
- Around line 971-979: Correct the missing-binary prerequisite example in the
testing documentation: either describe the nonexistent --current-binary case as
an exit-2 fail_usage failure without expecting evidence files, or revise the
harness flow so executability validation occurs after evidence setup and is
recorded as PREREQ. Update the example commands and expected outcome
consistently, preserving the existing prerequisite-reporting behavior for checks
that reach execution.
In `@scripts/qa-runtime-daemon-lifecycle.sh`:
- Around line 620-637: Update the on-demand startup invocation in the lifecycle
check around start_node so it explicitly selects on-demand runtime mode using
the supported mode flag or established [runtime] configuration pattern. Keep the
existing PREREQ handling when startup rejects the mode, ensuring
runtime_mode_on_demand cannot pass while exercising default serve mode.
- Around line 1010-1029: Update the final teardown check around the existing
WORK_ROOT pgrep sweep to also inspect the tracked PIDS and all of their
descendants, since daemon command lines may not contain WORK_ROOT. Include any
matching tracked processes in remaining and target them in both termination
passes, while preserving the existing pgrep fallback and PASS/FAIL reporting
through clean_process_teardown.
---
Nitpick comments:
In `@crates/mesh-llm-host-runtime/src/api/routes/runtime_activity.rs`:
- Around line 31-38: Replace the duplicated three-variant mappings in the
runtime activity handler with adjacent From implementations between
ActivityOverrideMode and ManualActivityOverride. Update both conversion sites to
use these conversions while preserving the existing invalid-input response and
all variant mappings.
In `@crates/mesh-llm-host-runtime/src/runtime/daemon_startup.rs`:
- Line 50: Replace the bare #[allow(clippy::field_reassign_with_default)] near
the test-options construction with #[expect(..., reason = "...")] and provide a
clear justification, or rewrite that construction using struct-update syntax and
remove the suppression entirely. Follow the surrounding #[expect] convention.
In `@crates/mesh-llm-host-runtime/src/runtime/mod.rs`:
- Around line 43-47: Remove the blanket #[allow(unused_imports)] attributes from
both crate-root re-export blocks in
crates/mesh-llm-host-runtime/src/runtime/mod.rs:43-47 and
crates/mesh-llm-host-runtime/src/runtime/mod.rs:60-67, then remove any unused
symbols from the instance_lifecycle and model_reconciliation re-exports
respectively. Retain only types with real consumers and have new code import
directly from the owning modules.
- Around line 75-76: Remove the stale comment above the activity_policy
re-export, or remove the pub(crate) re-export itself; ensure the remaining API
and its documented access path are consistent. Prefer retaining the existing
re-export from the runtime module root unless it is no longer needed.
In `@crates/mesh-llm-host-runtime/src/runtime/model_lifecycle/unload.rs`:
- Around line 5-8: Remove both clippy::cognitive_complexity suppressions by
refactoring the unload paths into semantically named helpers: extract the shared
ordered drain-and-transition operation, and extract dashboard/console cleanup.
Update each unload flow to call these helpers while preserving the existing
ordering through deregistration, telemetry cleanup, and event emission.
- Around line 68-74: Update the lifecycle handling around the state check and
transition to acquire a single lock guard, use it to check whether the state is
Unloading, and perform transition_to on that same guard. Preserve the existing
Stopped transition and error propagation.
In `@crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/state.rs`:
- Around line 387-398: Update suppressed and cooldown_active so failed_models
and manual_unload_models cooldown checks include profile in their matching
criteria, preserving profile-scoped suppression consistently with the
compound_key in_flight check. Ensure a cooldown for one profile does not
suppress other profiles of the same model.
In `@scripts/qa-runtime-daemon-lifecycle.sh`:
- Around line 488-524: Remove the unused json_field, json_len, and json_query
helper functions from the harness, leaving the existing inline python3 JSON
extraction logic unchanged.
- Around line 729-733: Update the wait logic around wait "$pid" to capture its
exit status inline without disabling and re-enabling errexit via set +e and set
-e. Preserve the existing exit_code assignment and subsequent error-handling
behavior.
- Around line 605-609: The serve-mode verification in the runtime lifecycle
check records PASS without inspecting the status file. Update the check around
status_file and record_result to parse and assert that the status mode field is
"serve" before reporting runtime_mode_serve as passing; retain the existing
liveness/API validation separately.
🪄 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: 16e5e601-1588-444e-b23f-2630e0924d34
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (112)
Justfilecrates/mesh-client/src/client/control_plane.rscrates/mesh-client/tests/control_plane_client.rscrates/mesh-client/tests/protocol_wire.rscrates/mesh-llm-cli/src/runtime.rscrates/mesh-llm-config/Cargo.tomlcrates/mesh-llm-config/src/lib.rscrates/mesh-llm-config/src/model.rscrates/mesh-llm-config/src/model/built_in_schema.rscrates/mesh-llm-config/src/model/built_in_schema/control_behavior/runtime_controls.rscrates/mesh-llm-config/src/model/runtime.rscrates/mesh-llm-config/src/validate.rscrates/mesh-llm-gpu-bench/build.rscrates/mesh-llm-host-runtime/src/api/access.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/api/routes/runtime_activity.rscrates/mesh-llm-host-runtime/src/api/state.rscrates/mesh-llm-host-runtime/src/api/status.rscrates/mesh-llm-host-runtime/src/api/status/runtime.rscrates/mesh-llm-host-runtime/src/api/tests/mod.rscrates/mesh-llm-host-runtime/src/api/tests/node_state.rscrates/mesh-llm-host-runtime/src/api/tests/runtime_lifecycle_routes.rscrates/mesh-llm-host-runtime/src/api/tests/support.rscrates/mesh-llm-host-runtime/src/mesh/gossip.rscrates/mesh-llm-host-runtime/src/mesh/mod.rscrates/mesh-llm-host-runtime/src/mesh/node.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/commands/mod.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/commands/model_lifecycle.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/owner_control_response.rscrates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache.rscrates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache/tests.rscrates/mesh-llm-host-runtime/src/mesh/peer_state.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/control_plane_unique.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_frames.rscrates/mesh-llm-host-runtime/src/network/nostr/model_packs.rscrates/mesh-llm-host-runtime/src/network/openai/ingress.rscrates/mesh-llm-host-runtime/src/network/openai/response/routing.rscrates/mesh-llm-host-runtime/src/network/openai/transport.rscrates/mesh-llm-host-runtime/src/network/tunnel.rscrates/mesh-llm-host-runtime/src/protocol/control_frames.rscrates/mesh-llm-host-runtime/src/protocol/convert.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/mesh_timestamps.rscrates/mesh-llm-host-runtime/src/runtime/activity_policy.rscrates/mesh-llm-host-runtime/src/runtime/auto_join.rscrates/mesh-llm-host-runtime/src/runtime/control_loop.rscrates/mesh-llm-host-runtime/src/runtime/daemon_startup.rscrates/mesh-llm-host-runtime/src/runtime/instance_lifecycle.rscrates/mesh-llm-host-runtime/src/runtime/interactive.rscrates/mesh-llm-host-runtime/src/runtime/local.rscrates/mesh-llm-host-runtime/src/runtime/local_split/test_support.rscrates/mesh-llm-host-runtime/src/runtime/mod.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/load.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/reconciliation.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/unload.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/intent.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/mod.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/planner.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/state.rscrates/mesh-llm-host-runtime/src/runtime/publication.rscrates/mesh-llm-host-runtime/src/runtime/run_auto.rscrates/mesh-llm-host-runtime/src/runtime/serving_surface.rscrates/mesh-llm-host-runtime/src/runtime/startup_handles.rscrates/mesh-llm-host-runtime/src/runtime/startup_models.rscrates/mesh-llm-host-runtime/src/runtime/tests/model_lifecycle.rscrates/mesh-llm-host-runtime/src/runtime/tests/startup_models.rscrates/mesh-llm-host-runtime/src/runtime_data/api_views.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-system/src/activity/linux.rscrates/mesh-llm-system/src/activity/macos.rscrates/mesh-llm-system/src/activity/mod.rscrates/mesh-llm-system/src/activity/unsupported.rscrates/mesh-llm-system/src/activity/windows.rscrates/mesh-llm-system/src/lib.rscrates/mesh-llm-ui/e2e/perf/meshviz-200.spec.tscrates/mesh-llm-ui/src/features/app-tabs/types.tscrates/mesh-llm-ui/src/features/configuration/api/config-adapter.tscrates/mesh-llm-ui/src/features/configuration/api/runtime-settings.test.tsxcrates/mesh-llm-ui/src/features/configuration/api/runtime-settings.tscrates/mesh-llm/src/commands/runtime.rscrates/mesh-llm/tests/protocol_convert_matrix.rsdocs/CLI.mddocs/MESHES.mddocs/USAGE.mddocs/design/TESTING.mddocs/design/message_protocol.mdscripts/build-linux.shscripts/build-llama.shscripts/ci-sdk-fixture.shscripts/lib/cuda-toolkit.shscripts/qa-control-plane-mixed-version.shscripts/qa-runtime-daemon-lifecycle.shscripts/tests/test_cuda_toolkit.pytools/xtask/src/main.rstools/xtask/src/repo_consistency.rswebsite/src/docs/pages/CLI.md
💤 Files with no reviewable changes (1)
- crates/mesh-llm-host-runtime/src/protocol/control_frames.rs
🚧 Files skipped from review as they are similar to previous changes (89)
- crates/mesh-llm-system/src/lib.rs
- crates/mesh-llm-host-runtime/src/protocol/tests/mesh_timestamps.rs
- crates/mesh-llm-host-runtime/src/mesh/mod.rs
- crates/mesh-llm-config/Cargo.toml
- tools/xtask/src/main.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/protocol_frames.rs
- crates/mesh-llm-host-runtime/src/runtime/local_split/test_support.rs
- crates/mesh-llm-ui/src/features/app-tabs/types.ts
- crates/mesh-llm-host-runtime/src/mesh/owner_control_response.rs
- crates/mesh-llm-host-runtime/src/runtime_data/api_views.rs
- crates/mesh-client/tests/protocol_wire.rs
- crates/mesh-llm-host-runtime/src/api/routes/mod.rs
- crates/mesh-llm-host-runtime/src/api/tests/node_state.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/admission/requirements.rs
- crates/mesh-llm-host-runtime/src/runtime/publication.rs
- crates/mesh-llm-host-runtime/src/network/openai/response/routing.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/control_plane_unique.rs
- crates/mesh-llm-system/src/activity/unsupported.rs
- crates/mesh-llm-host-runtime/src/network/nostr/model_packs.rs
- docs/MESHES.md
- crates/mesh-llm-host-runtime/src/mesh/tests/admission/helpers.rs
- crates/mesh-llm-host-runtime/src/api/tests/support.rs
- crates/mesh-llm-host-runtime/src/protocol/tests.rs
- crates/mesh-llm-host-runtime/src/api/access.rs
- crates/mesh-llm/tests/protocol_convert_matrix.rs
- crates/mesh-llm-system/src/activity/windows.rs
- crates/mesh-llm-host-runtime/src/runtime_data/mod.rs
- crates/mesh-llm-config/src/model/built_in_schema.rs
- crates/mesh-llm-host-runtime/src/runtime/startup_models.rs
- crates/mesh-llm-ui/src/features/configuration/api/config-adapter.ts
- crates/mesh-llm-host-runtime/src/api/state.rs
- crates/mesh-llm-host-runtime/src/mesh/tests/connections.rs
- crates/mesh-llm-host-runtime/src/protocol/tests/announcements.rs
- crates/mesh-llm-host-runtime/src/network/openai/transport.rs
- crates/mesh-llm-host-runtime/src/api/status/runtime.rs
- tools/xtask/src/repo_consistency.rs
- website/src/docs/pages/CLI.md
- crates/mesh-llm-host-runtime/src/runtime/auto_join.rs
- crates/mesh-llm-ui/src/features/configuration/api/runtime-settings.ts
- crates/mesh-llm-host-runtime/src/mesh/tests/peer_state.rs
- crates/mesh-llm-host-runtime/src/runtime/tests/startup_models.rs
- crates/mesh-llm-host-runtime/src/runtime/tests/model_lifecycle.rs
- crates/mesh-llm-host-runtime/src/network/tunnel.rs
- crates/mesh-llm-host-runtime/src/mesh/owner_control/mod.rs
- crates/mesh-client/tests/control_plane_client.rs
- crates/mesh-llm-config/src/model/runtime.rs
- crates/mesh-llm-host-runtime/src/protocol/convert.rs
- crates/mesh-llm-host-runtime/src/runtime/interactive.rs
- crates/mesh-llm-config/src/lib.rs
- scripts/ci-sdk-fixture.sh
- docs/CLI.md
- crates/mesh-llm-host-runtime/src/runtime/local.rs
- docs/design/message_protocol.md
- docs/USAGE.md
- crates/mesh-llm-protocol/src/proto/node.rs
- crates/mesh-llm-ui/src/features/configuration/api/runtime-settings.test.tsx
- crates/mesh-llm-cli/src/runtime.rs
- Justfile
- crates/mesh-llm-host-runtime/src/mesh/tests/gossip.rs
- crates/mesh-llm-host-runtime/src/mesh/owner_control/commands/model_lifecycle.rs
- crates/mesh-llm-host-runtime/src/api/mod.rs
- crates/mesh-llm-protocol/proto/node.proto
- crates/mesh-llm-host-runtime/src/mesh/tests/owner_control.rs
- crates/mesh-llm-host-runtime/src/api/routes/runtime.rs
- crates/mesh-llm-host-runtime/src/api/tests/runtime_lifecycle_routes.rs
- crates/mesh-llm-system/src/activity/linux.rs
- crates/mesh-llm-ui/e2e/perf/meshviz-200.spec.ts
- crates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache/tests.rs
- crates/mesh-llm-host-runtime/src/mesh/node.rs
- crates/mesh-llm-host-runtime/src/runtime/serving_surface.rs
- scripts/qa-control-plane-mixed-version.sh
- crates/mesh-client/src/client/control_plane.rs
- crates/mesh-llm-config/src/model.rs
- crates/mesh-llm-system/src/activity/mod.rs
- crates/mesh-llm/src/commands/runtime.rs
- crates/mesh-llm-host-runtime/src/runtime/model_lifecycle/reconciliation.rs
- crates/mesh-llm-host-runtime/src/mesh/gossip.rs
- crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/planner.rs
- crates/mesh-llm-host-runtime/src/mesh/peer_state.rs
- crates/mesh-llm-host-runtime/src/mesh/owner_control/commands/mod.rs
- crates/mesh-llm-host-runtime/src/runtime/control_loop.rs
- crates/mesh-llm-host-runtime/src/runtime/model_lifecycle.rs
- crates/mesh-llm-protocol/src/protocol/mod.rs
- crates/mesh-llm-host-runtime/src/runtime/instance_lifecycle.rs
- crates/mesh-llm-config/src/model/built_in_schema/control_behavior/runtime_controls.rs
- crates/mesh-llm-host-runtime/src/network/openai/ingress.rs
- crates/mesh-llm-host-runtime/src/runtime/run_auto.rs
- crates/mesh-llm-host-runtime/src/runtime/startup_handles.rs
- crates/mesh-llm-host-runtime/src/runtime/model_lifecycle/load.rs
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/qa-runtime-daemon-lifecycle.sh`:
- Line 975: Update the PIDS iteration in the cleanup loop to remain safe with
set -u on Bash 3.2 when the array is empty. Use a nounset-safe expansion or
guard the loop with the array length, while preserving normal iteration over
recorded process IDs.
- Around line 581-590: Update the on-demand startup flow around start_node and
its polling loop to track whether a successful /api/status response is received.
Require that readiness flag before recording PASS; if the daemon remains alive
but the endpoint never responds, record FAIL instead, while preserving the
existing PREREQ handling for unsupported on-demand configuration.
- Around line 972-995: Update the final cleanup flow around the PIDS sweep and
remaining_pids handling so kill -0 alone does not establish process ownership.
Retire tracked PIDs after their processes exit, or capture and verify a stable
identity such as start time before adding them to remaining_pids and issuing
kill or kill -9; only escalate against verified matching processes.
🪄 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: ea6468b4-cc13-4947-88da-d1df276cc7c3
📒 Files selected for processing (15)
crates/mesh-llm-host-runtime/src/api/routes/runtime_activity.rscrates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache.rscrates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache/tests.rscrates/mesh-llm-host-runtime/src/runtime/activity_policy.rscrates/mesh-llm-host-runtime/src/runtime/daemon_startup.rscrates/mesh-llm-host-runtime/src/runtime/mod.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/unload.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/intent.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/mod.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/planner.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/state.rscrates/mesh-llm-host-runtime/src/runtime/tests/model_lifecycle.rscrates/mesh-llm-system/src/activity/macos.rsdocs/design/TESTING.mdscripts/qa-runtime-daemon-lifecycle.sh
🚧 Files skipped from review as they are similar to previous changes (9)
- crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/mod.rs
- crates/mesh-llm-host-runtime/src/runtime/daemon_startup.rs
- crates/mesh-llm-host-runtime/src/api/routes/runtime_activity.rs
- crates/mesh-llm-host-runtime/src/runtime/tests/model_lifecycle.rs
- crates/mesh-llm-host-runtime/src/runtime/mod.rs
- crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/intent.rs
- docs/design/TESTING.md
- crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/state.rs
- crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/planner.rs
a466c70 to
60813d9
Compare
|
@CodeRabbit full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (8)
crates/mesh-llm/src/commands/runtime.rs-277-291 (1)
277-291: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate mutually-exclusive
model/instance_idbefore building the request.
build_lifecycle_request's fallback arm (_ => json!({ "endpoint": endpoint })) silently drops bothmodelandinstance_idwhenever the CLI caller supplies both or neither forunload-model/drain-model. The request still reaches the server and gets rejected, but with a generic message that doesn't reflect that conflicting/missing flags were actually the cause.🐛 Proposed fix: validate before dispatch
pub(crate) async fn run_control_unload_model( endpoint: &str, model: Option<&str>, instance_id: Option<&str>, port: u16, json_output: bool, ) -> Result<()> { + if model.is_some() == instance_id.is_some() { + anyhow::bail!("unload-model requires exactly one of --model or --instance-id"); + } let body = post_runtime_payload( port, "/api/runtime/control/unload-model", &build_lifecycle_request(endpoint, model, instance_id, None), ) .await?; print_control_response("Unload model (owner-control)", &body, json_output) }Apply the same guard to
run_control_drain_model.Also applies to: 309-323, 614-628
🤖 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/src/commands/runtime.rs` around lines 277 - 291, Validate that exactly one of model or instance_id is provided at the start of run_control_unload_model and run_control_drain_model, returning a clear error before calling build_lifecycle_request or dispatching the request. Preserve the existing request and response handling when validation succeeds.docs/design/TESTING.md-1031-1031 (1)
1031-1031: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe probes don't discover nodes by scanning console ports.
run_lifecycle_probesusesCURRENT_HOST_CONSOLE/RELEASED_HOST_CONSOLE, which are assigned directly when the nodes start (run_local_directionand afterconfig-current-controller) — there is no port scan and noBASE_PORT+81probe. Also only the fourlifecycle-*-modelchecks pluslifecycle-legacy-unsupportedare recorded PREREQ whenCURRENT_HOST_CONSOLEis empty; when onlyRELEASED_HOST_CONSOLEis missing, the four command probes still execute.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/TESTING.md` at line 1031, Correct the run_lifecycle_probes documentation to state that probes use CURRENT_HOST_CONSOLE and RELEASED_HOST_CONSOLE assigned by run_local_direction and config-current-controller, not console-port scanning or BASE_PORT+81. Document that only the four lifecycle-*-model checks and lifecycle-legacy-unsupported are recorded as PREREQ when CURRENT_HOST_CONSOLE is empty, while the four command probes still run when only RELEASED_HOST_CONSOLE is missing.scripts/qa-runtime-daemon-lifecycle.sh-796-799 (1)
796-799: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInconsistent result vocabulary when the zero-model daemon is unavailable.
check_activity_override(Line 922) andcheck_privacy_no_raw_data(Line 982) recordPREREQfor the exact same missing-daemon precondition, while the four lifecycle checks recordFAIL. Whenzero_model_serve_readyis blocked by an environment prerequisite (e.g. the owner keystore step at Line 506 clearedOWNER_KEY), the evidence bundle reports four hard failures whose root cause is a prerequisite, not a regression.🐛 Proposed fix
if [[ -z "$console_port" ]] || ! kill -0 "${ZERO_MODEL_PID:-}" 2>/dev/null; then - record_result "FAIL" "$result_name" "zero-model daemon is not running" + record_result "PREREQ" "$result_name" \ + "zero-model daemon is not running; authenticated lifecycle path cannot be exercised" return 0 fi🤖 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 `@scripts/qa-runtime-daemon-lifecycle.sh` around lines 796 - 799, Update the missing zero-model daemon handling in the four lifecycle checks around the shown kill -0 guard to record "PREREQ" instead of "FAIL", matching check_activity_override and check_privacy_no_raw_data. Preserve the existing result message and return behavior so environment-blocked readiness is reported consistently as a prerequisite.crates/mesh-llm-host-runtime/src/network/openai/ingress.rs-629-641 (1)
629-641: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winNo-model path can silently close the connection without any HTTP response.
first_available_targetreturnsInferenceTarget::Nonewhen nothing is routable, and theroute_to_targetresult is discarded. Unlike the model-specified branch, the client then gets no status line at all. A 503 on theNonecase would keep behavior consistent with the rest of this rewrite.🤖 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/ingress.rs` around lines 629 - 641, Handle the no-model fallback in the route_to_target flow by checking when first_available_target(ctx.targets) yields InferenceTarget::None and returning an HTTP 503 response instead of silently discarding the result. Preserve normal routing for available targets and align the behavior with the model-specified branch.crates/mesh-llm-host-runtime/src/mesh/gossip.rs-1019-1025 (1)
1019-1025: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRemove
resume_debounce_secsfrom the activity policy.
resume_debounce_secsaccepts0, which lets short idle samples remain active indefinitely and prevents activity-based pauses from ever reaching the withdrawal/clear path instead only adding an extra config surface with that behavior.🤖 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 1019 - 1025, Remove the resume_debounce_secs configuration and all related activity-policy logic, including its validation and propagation, so short idle samples no longer remain active indefinitely. Preserve the existing activity-policy withdrawal behavior in advertisement_decision and the serving_models/hosted_models clearing path.crates/mesh-llm-host-runtime/src/mesh/owner_control/commands/model_lifecycle.rs-105-115 (1)
105-115: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
drain_timeout_secs = 0is accepted from the wire.Config validation rejects a zero drain timeout, but an owner-supplied
Some(0)passes straight through toDuration::from_secs(0), turning a drain into an immediate unload with no grace period. Reject zero alongside the max check.🐛 Proposed fix
- if drain_timeout_secs > node.drain_timeout_max_secs { + if drain_timeout_secs == 0 || drain_timeout_secs > node.drain_timeout_max_secs { return owner_control_error_envelope( OwnerControlErrorCode::BadRequest, Some(request_id), None, - "drain_timeout_secs exceeds configured maximum", + "drain_timeout_secs must be between 1 and the configured maximum", ); }🤖 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/commands/model_lifecycle.rs` around lines 105 - 115, Update the drain_timeout_secs validation in the model lifecycle command to reject a resolved timeout of zero in addition to values exceeding node.drain_timeout_max_secs. Preserve the existing BadRequest response and message behavior for invalid owner-supplied values, while allowing positive values within the configured maximum.crates/mesh-llm-host-runtime/src/runtime/control_loop.rs-459-472 (1)
459-472: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winA rejected load leaves its
Presentintent in history.When
is_effective_intentis false the caller gets an error, but the intent added at Line 459 stays inintent_historyun-retired. Once the suppressing higher-precedence intent is evicted by the 256-entry bound (both are non-maintained), this orphanedPresentintent can become effective and cause reconciliation to load a model whose request was explicitly rejected. Retire it on the rejection path.🛡️ Proposed fix
if !ctx .model_target_reconciliation_state .is_effective_intent(&intent_id, &spec, &profile) { + ctx.model_target_reconciliation_state + .retire_one_shot_present(&intent_id); if let Some(tx) = completion { let _ = tx.send(Err(anyhow::anyhow!( "model load intent is suppressed by a higher-priority desired state" ))); } return; }🤖 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/control_loop.rs` around lines 459 - 472, On the !is_effective_intent rejection path in the control loop, retire the intent_id just added by add_desired_with_id before sending the error and returning. Ensure the rejected Present intent is removed from intent_history while preserving the existing completion error behavior.crates/mesh-llm-host-runtime/src/runtime/instance_lifecycle.rs-540-556 (1)
540-556: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
wait_for_unload_readycan spin forever if the instance leavesDraining.
is_drain_deadline_expired()only reportstruewhilestate == Draining.Draining → Failedis a valid transition, so if another task fails the instance while in-flight is still non-zero, neither exit condition can ever be satisfied and this loop polls indefinitely, leaking a task and blocking the unload path. Bail out when the record is no longer draining.🛡️ Proposed fix
{ let record = record.lock().await; if record.in_flight_count() == 0 { return DrainResult::Graceful; } if record.is_drain_deadline_expired() { return DrainResult::ForceCancelled; } + if record.state() != InstanceLifecycleState::Draining { + return DrainResult::ForceCancelled; + } }🤖 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/instance_lifecycle.rs` around lines 540 - 556, Update wait_for_unload_ready to return ForceCancelled when the locked InstanceLifecycleRecord is no longer in the Draining state, including transitions such as Draining to Failed. Preserve the existing Graceful result for zero in-flight work and the deadline-based ForceCancelled result while still draining.
🧹 Nitpick comments (29)
crates/mesh-llm-ui/src/features/configuration/api/runtime-settings.ts (2)
41-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate pure utilities also defined in config-adapter.ts.
titleCaseIdentifier,sortCategories,sortSettings, andDEFAULT_CATEGORY_ORDER/DEFAULT_SETTING_ORDERhere are essentially identical to the ones already inconfig-adapter.ts. Extracting these into a shared module would remove the duplication captured in the consolidated comment below.Also applies to: 52-59, 206-222
🤖 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-ui/src/features/configuration/api/runtime-settings.ts` around lines 41 - 42, Extract the duplicated utilities titleCaseIdentifier, sortCategories, sortSettings, DEFAULT_CATEGORY_ORDER, and DEFAULT_SETTING_ORDER from runtime-settings.ts and config-adapter.ts into a shared module, then update both consumers to import and reuse those definitions while preserving their current behavior.
41-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShared schema-settings helpers duplicated between config-adapter.ts and the new runtime-settings.ts. Both files independently implement sorting, title-casing, order constants, and icon/control lookups for the same underlying schema-derived settings pipeline, which will drift over time if only one copy is updated.
crates/mesh-llm-ui/src/features/configuration/api/runtime-settings.ts#L41-L59: extracttitleCaseIdentifier,sortCategories,sortSettings, andDEFAULT_CATEGORY_ORDER/DEFAULT_SETTING_ORDERinto a shared helper module imported by both files instead of redefining them here.crates/mesh-llm-ui/src/features/configuration/api/config-adapter.ts#L283: either import the shared icon map from the new common module (dropping this local copy) or keep it file-local (non-exported) if runtime-settings.ts's separate map is intentional — the current export with no external consumer is unclear intent.crates/mesh-llm-ui/src/features/configuration/api/config-adapter.ts#L731-L741: after extracting shared helpers, confirmcombineSettingsHarnessDatastill produces a globally consistent sort order across the two composed settings groups.🤖 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-ui/src/features/configuration/api/runtime-settings.ts` around lines 41 - 59, Extract titleCaseIdentifier, sortCategories, sortSettings, DEFAULT_CATEGORY_ORDER, and DEFAULT_SETTING_ORDER from runtime-settings.ts into a shared helper module, then import and reuse them from both configuration files. In config-adapter.ts, resolve the exported icon-map duplication by importing the shared map or making the local map non-exported if it is intentionally separate. Verify combineSettingsHarnessData preserves one globally consistent sort order across both composed settings groups; apply these changes in crates/mesh-llm-ui/src/features/configuration/api/runtime-settings.ts (41-59) and crates/mesh-llm-ui/src/features/configuration/api/config-adapter.ts (283, 731-741).tools/xtask/src/repo_consistency.rs (2)
332-340: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueFlag parsing only recognizes the space-separated form.
command_flag_valuesmatchespair[0] == flag, so-p=crate/--exclude=cratewould be silently ignored and the crate would drop out of both sets — parity and coverage still pass while the Justfile actually skips a crate. Cheap to harden.♻️ Proposed hardening
fn command_flag_values(command: &str, flag: &str) -> BTreeSet<String> { - command - .split_whitespace() - .collect::<Vec<_>>() - .windows(2) - .filter(|pair| pair[0] == flag) - .map(|pair| pair[1].to_string()) - .collect() + let tokens = command.split_whitespace().collect::<Vec<_>>(); + let mut values = BTreeSet::new(); + for (index, token) in tokens.iter().enumerate() { + if let Some(value) = token.strip_prefix(&format!("{flag}=")) { + values.insert(value.to_string()); + } else if *token == flag { + if let Some(value) = tokens.get(index + 1) { + values.insert((*value).to_string()); + } + } + } + values }🤖 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 `@tools/xtask/src/repo_consistency.rs` around lines 332 - 340, Update command_flag_values to parse both space-separated flags and equals-form flags such as -p=crate and --exclude=crate. Normalize each matching argument into its flag value before collecting it, while preserving the existing behavior for flags followed by a separate value.
277-286: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe coverage assertion is tautological once parity passes.
ensure_set_eq(&dynamic_targets, &excluded_targets, ...)at Line 271 already guaranteesdynamic == excluded, socovered = workspace - excluded + dynamicis always exactlyworkspace. The secondensure_set_eqcan never fail and gives false confidence in the check.If the intent is only to enforce parity plus subset membership, drop the derived set; if the intent is to catch a future Justfile that excludes crates without re-adding them, compute coverage before the parity check (or make parity a soft check).
🤖 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 `@tools/xtask/src/repo_consistency.rs` around lines 277 - 286, Remove the tautological coverage calculation and its second ensure_set_eq assertion after the dynamic/excluded parity check. Keep the existing ensure_set_eq(&dynamic_targets, &excluded_targets, ...) parity validation and rely on the earlier subset-membership checks, unless coverage must be computed before parity to detect missing re-additions.Justfile (1)
394-396: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNit: preflight skips
just with-lldused by every other xtask invocation.Lines 425-426 wrap the same binary with
with-lld; running the preflight without it buildsxtaskwith a different linker config and can cost an extra link.🤖 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 `@Justfile` around lines 394 - 396, Update the test-all-rust-crate-coverage preflight invocation in the Justfile to run through the existing with-lld wrapper, matching the other xtask invocations around it. Preserve the repo-consistency test-all-rust-crate-coverage arguments while ensuring the same linker configuration is used.crates/mesh-llm-gpu-bench/build.rs (1)
178-180: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueNit: an empty
NVCC/CUDACXXis treated as a valid compiler path.
env::var("NVCC")returnsOk("")when the variable is exported but empty (common in CI matrices), producing a confusingCommand::new("")failure instead of falling back tonvcc.🛡️ Proposed hardening
- let nvcc = std::env::var("NVCC") - .or_else(|_| std::env::var("CUDACXX")) - .unwrap_or_else(|_| "nvcc".to_string()); + let nvcc = ["NVCC", "CUDACXX"] + .iter() + .filter_map(|name| std::env::var(name).ok()) + .find(|value| !value.trim().is_empty()) + .unwrap_or_else(|| "nvcc".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-gpu-bench/build.rs` around lines 178 - 180, Update the NVCC compiler selection around the NVCC/CUDACXX environment lookups to ignore empty values and continue to the next fallback, ultimately using "nvcc" when both variables are unset or empty. Preserve non-empty NVCC precedence over CUDACXX.scripts/tests/test_cuda_toolkit.py (1)
42-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNit: hoist the CUDA env-var list into a module constant.
The same eight-name tuple is duplicated in two tests; a shared
CUDA_ENV_VARSconstant (plus a smallclean_env()helper) keeps future additions in one place and would also lettest_preserves_explicit_cuda_environmentstart from a known-clean base.Also applies to: 107-117
🤖 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 `@scripts/tests/test_cuda_toolkit.py` around lines 42 - 52, Extract the duplicated eight-name CUDA environment tuple into a module-level CUDA_ENV_VARS constant, and add a small clean_env() helper that removes each listed variable. Update both affected tests, including test_preserves_explicit_cuda_environment, to use the helper so each starts from a known-clean environment.docs/design/message_protocol.md (1)
160-162: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: name the concrete deadline for tags 6-9 in "Bounds and deadlines".
That list enumerates get/apply (5s) and inventory (35s) but not the lifecycle commands, so "the unary deadline" here is unresolved for a reader implementing a client.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/message_protocol.md` around lines 160 - 162, Update the “Bounds and deadlines” section to define the concrete unary deadline for lifecycle command tags 6–9, and revise the response description to reference that named deadline instead of the ambiguous “existing unary deadline.”scripts/qa-control-plane-mixed-version.sh (1)
1329-1393: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueResult messages attribute the response to the released host, but the request goes to the current host.
Line 1373 POSTs to
current_console_portwith the released node's bootstrap endpoint — i.e. the current binary is the controller and the released node is the target. That's the right shape for the compatibility probe, but "released host returned a typed unsupported lifecycle response" reads as though the released host served the HTTP call. Consider "current controller reported CONTROL_UNSUPPORTED for the released target" so the evidence bundle is unambiguous.Also, Line 1341 and Line 1375 both redirect stderr to
$control_log, so a bootstrap-stage diagnostic is overwritten by the POST stage before the FAIL record references it.🤖 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 `@scripts/qa-control-plane-mixed-version.sh` around lines 1329 - 1393, Update probe_lifecycle_legacy_unsupported so result messages identify the current controller as issuing the typed control_unsupported response for the released target, rather than attributing the HTTP response to the released host. Preserve the request through current_console_port using the released endpoint. Prevent the bootstrap curl diagnostics from being overwritten by the later POST by using separate log files or appending the POST stderr.crates/mesh-llm-system/src/activity/unsupported.rs (1)
20-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
restore_priorityshould reportUnsupported, notRestoreFailed.Nothing was ever reduced on this backend, so
RestoreFailedmislabels the cause and diverges fromwindows.rs, which returnsUnsupportedfor both directions.♻️ Proposed change
fn restore_priority(&mut self) -> Result<(), PriorityFailure> { - Err(PriorityFailure::RestoreFailed) + Err(PriorityFailure::Unsupported) }🤖 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-system/src/activity/unsupported.rs` around lines 20 - 22, Update restore_priority in the unsupported backend to return PriorityFailure::Unsupported instead of PriorityFailure::RestoreFailed, matching the unsupported behavior used by windows.rs for both priority directions.crates/mesh-llm-system/src/activity/mod.rs (1)
204-207: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueA single
Unknownsample discards the accumulated idle timer.Transient detector failures are common (
/proc/statread failure,total_delta == 0,ioregnon-zero exit), and each one resetsdetected_idle_since, so the host must re-accumulate the fullidle_after + resume_debouncewindow before it can go idle again. On a noisy detector this can prevent the idle transition indefinitely. Consider preservingdetected_idle_sinceacrossUnknownand only clearing it on an observedActive.🤖 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-system/src/activity/mod.rs` around lines 204 - 207, Update the HostActivity::Unknown branch to preserve the existing detected_idle_since value instead of clearing it. Continue returning HostActivity::Unknown, and retain clearing detected_idle_since only when an Active state is observed.crates/mesh-llm-system/src/activity/macos.rs (1)
5-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
use libc;.Bare extern-crate imports trip
clippy::single_component_path_imports, which is warn-by-default. Thelibc::paths below resolve without it. This only surfaces when compiling for macOS, so Linux CI won't catch it.As per coding guidelines: "Do not leave compiler warnings in touched Rust code".
🤖 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-system/src/activity/macos.rs` around lines 5 - 6, Remove the redundant cfg-gated `use libc;` import from the macOS-specific code in `macos.rs`. Keep the existing `libc::` references unchanged, relying on direct crate-path resolution to avoid the Clippy warning.Source: Coding guidelines
crates/mesh-llm-system/src/activity/linux.rs (1)
55-74: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winLinux
reduce_priorityis gated behind root/CAP_SYS_NICE, unlike macOS.Raising a nice value never requires privilege; only lowering it back does. As written, unprivileged Linux hosts always return
Unsupported, soActivityResponse::ReducePrioritypermanently collapses toAccepting(seecompute_effective_stateinruntime/activity_policy.rs). The macOS backend had the equivalent root gate removed in an earlier round. If the gate is deliberate (so the reduction is reversible), a short comment stating that rationale would prevent it from reading as an oversight; otherwise attempt the reduction and let the restore path reportRestoreFailed.🤖 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-system/src/activity/linux.rs` around lines 55 - 74, Remove the can_restore_priority() gate from reduce_priority so Linux can attempt the non-privileged nice-value increase; preserve read_process_nice, set_process_nice, and original_nice tracking. Let failures from the reduction or later restoration propagate through their existing PriorityFailure paths, including RestoreFailed.crates/mesh-llm-host-runtime/src/runtime/activity_policy.rs (1)
100-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winManagement-API admission is only modelled in test builds.
IngressType::ManagementApi,is_inference(), and the escape-hatch branch insidecheck_admission'sAllPausedarm are all#[cfg(test)]. The tests at Lines 674-677 and 825-828 therefore assert behavior that does not exist in a release binary — they validate the test scaffolding, not the shipped policy. Either make the variant real (management routes then pass it explicitly and the branch is exercised in production) or drop the assertions and rely on the "management routes don't call this guard" invariant being enforced elsewhere.Also applies to: 110-113, 308-317
🤖 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/activity_policy.rs` around lines 100 - 103, Make ManagementApi admission behavior part of the release policy: remove the #[cfg(test)] gating from IngressType::ManagementApi, is_inference(), and the ManagementApi escape-hatch branch in check_admission’s AllPaused arm. Update management API route callers to pass IngressType::ManagementApi explicitly, preserving the existing tests as coverage of production behavior.crates/mesh-llm-host-runtime/src/api/tests/runtime_lifecycle_routes.rs (1)
76-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for the missing-
endpointrejection.
required_control_endpointreturnscontrol_endpoint_requiredwhenendpointis absent or blank, but every row here supplies"endpoint":"unused", so that branch is untested. A row like{"model":"model/test"}againstload-modelwould cover it.🤖 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/api/tests/runtime_lifecycle_routes.rs` around lines 76 - 102, Add a missing-endpoint case to the invalid inputs table used by the runtime lifecycle route tests, targeting load-model with a payload such as {"model":"model/test"} and expecting the control_endpoint_required rejection. Keep the existing invalid cases unchanged and ensure the new row exercises required_control_endpoint for an absent endpoint.crates/mesh-llm-host-runtime/src/network/openai/ingress.rs (1)
20-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
#[expect(dead_code, reason = "...")]over#[allow(...)], or drop the single-variant enum.
#[expect]is used elsewhere in this crate (e.g.api/mod.rsline 371) and will itself warn once the variant is genuinely used, so it doesn't rot. Alternatively, sinceMissingModelRouteResultnow has one variant and every caller ignores or unconditionally returnsRouted, the type could be replaced with().As per coding guidelines, "do not use
#[allow(...)]to silence warnings without a clear reason and developer approval."🤖 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/ingress.rs` around lines 20 - 21, Replace the #[allow(dead_code)] annotation on MissingModelRouteResult with #[expect(dead_code, reason = "...")] using a clear justification, or remove the single-variant enum and update its callers to use (). Prefer the expect-based change if preserving the type for future routing extensions.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/network/tunnel.rs (1)
107-118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate admission-match blocks; extract a shared helper.
The same
match check_admission(...)shape appears here, at Lines 134-148, and (in stream-consuming form) ascheck_activity_admissioninnetwork/openai/ingress.rs. A smallfn ensure_admitted(node: &Node, ingress: IngressType) -> Result<()>would collapse both call sites in this 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/network/tunnel.rs` around lines 107 - 118, Extract the repeated admission handling into a shared ensure_admitted helper accepting Node and IngressType and returning Result. Move the Allowed/Paused matching, debug logging, and pause error into that helper, then replace both admission-match blocks in this file with calls using the appropriate ingress type; keep the existing check_activity_admission behavior in network/openai/ingress.rs unchanged unless it can directly reuse the new helper.crates/mesh-llm-host-runtime/src/mesh/owner_control/commands/mod.rs (1)
201-214: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMerge the duplicate unary-deadline arms.
GetConfig/ApplyConfigand the four lifecycle variants both yieldUnary(5s); one arm reads cleaner.🤖 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/commands/mod.rs` around lines 201 - 214, Merge the two `OwnedNodeCommand::deadline` match arms that return `OwnedNodeCommandDeadline::Unary(Duration::from_secs(5))` into one arm containing `GetConfig`, `ApplyConfig`, `LoadModel`, `UnloadModel`, `EnsureModel`, and `DrainModel`; leave the `ScanRefresh` and `WatchConfig` arms unchanged.crates/mesh-llm-host-runtime/src/mesh/owner_control/commands/model_lifecycle.rs (1)
143-200: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueModel-ref legality is now validated twice.
validate_owner_control_model_for_load_or_ensure/..._for_unload_or_drainincrates/mesh-llm-protocol/src/protocol/mod.rsalready reject these exact combinations before dispatch. Keeping the handler-side copies is fine as defense in depth, but the two rule sets can drift; consider reusing the protocol helpers and mapping their error toBadRequest.🤖 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/commands/model_lifecycle.rs` around lines 143 - 200, Update extract_present_model_ref and extract_absent_model_ref to reuse validate_owner_control_model_for_load_or_ensure and validate_owner_control_model_for_unload_or_drain from the protocol module instead of duplicating their legality checks. Preserve the existing handler behavior by mapping validation failures to a BadRequest owner-control error envelope with the request_id and operation-appropriate message.crates/mesh-llm-protocol/src/protocol/mod.rs (1)
750-767: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the nested
else { if ... }to avoid a clippy style warning.
clippy::collapsible_else_ifis warn-by-default and fires on this shape. Amatchalso mirrorsvalidate_owner_control_model_for_load_or_ensurefor symmetry.♻️ Proposed refactor
- if canonical ^ instance { - Ok(()) - } else { - if canonical || instance { - Err(ControlFrameError::InvalidModelRefCombination) - } else { - Err(ControlFrameError::MissingModelRef) - } - } + match (canonical, instance) { + (true, false) | (false, true) => Ok(()), + (true, true) => Err(ControlFrameError::InvalidModelRefCombination), + (false, false) => Err(ControlFrameError::MissingModelRef), + }As per coding guidelines: "Do not leave compiler warnings in touched Rust code".
🤖 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-protocol/src/protocol/mod.rs` around lines 750 - 767, Update validate_owner_control_model_for_unload_or_drain to remove the nested else-if shape triggering clippy::collapsible_else_if. Flatten the conditional or use a match while preserving the existing MissingModelRef and InvalidModelRefCombination error outcomes and the valid exclusive-reference case.Source: Coding guidelines
crates/mesh-client/src/client/control_plane.rs (1)
551-568: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSilently discarding
model_refwheninstance_idis set hides caller mistakes.Both
unload_modelanddrain_modelblank the canonical ref whenever an instance id is present, so a caller that passes both gets the model ref dropped without any signal. Since the protocol enforces exactly-one, returning aProtocolerror for the ambiguous input would be clearer on this public client API.Also applies to: 653-672
🤖 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 551 - 568, Validate the inputs at the start of both unload_model and drain_model, returning a ControlPlaneClientError::Protocol error when model_ref and instance_id are both provided. Only construct the request after this exactly-one-of validation, preserving the existing behavior for model-ref-only and instance-id-only calls.crates/mesh-llm-host-runtime/src/api/status/runtime.rs (1)
12-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winJustify or remove the
#[allow(dead_code)]onDaemonState/derive_daemon_state.Nothing in the shipped status path consumes these —
api/mod.rsderiveslifecycle_statefrom raw process status strings instead. Either wire the derivation into the status payload or add a short comment explaining why the type is landed ahead of its consumer.As per coding guidelines: "do not use
#[allow(...)]to silence warnings without a clear reason and developer approval".Also applies to: 30-32
🤖 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/api/status/runtime.rs` around lines 12 - 14, Remove the unjustified #[allow(dead_code)] from DaemonState and derive_daemon_state, or wire derive_daemon_state into the shipped status payload so it is consumed. If retaining the unused type as intentional groundwork, replace the suppression with a concise comment explaining its planned consumer and rationale.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/runtime/instance_lifecycle.rs (2)
24-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBlanket
#![allow(dead_code)]hides warnings across the whole module.A file-scoped allow will keep suppressing dead code long after the TODO is resolved, and it also masks genuinely unused items added later. The rest of this PR uses targeted
#[expect(dead_code, reason = "...")]per item, which self-clears once the symbol is used. Prefer that here.Want me to open an issue to track removing this once admission/drain integration lands?
As per coding guidelines, "do not use
#[allow(...)]to silence warnings without a clear reason and developer approval."🤖 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/instance_lifecycle.rs` around lines 24 - 25, Remove the file-scoped #![allow(dead_code)] from instance_lifecycle.rs and replace it with targeted #[expect(dead_code, reason = "...")] attributes only on the currently unused items that require suppression. Keep the reasons specific to the pending admission/drain integration and avoid suppressing dead-code warnings for the entire module.Source: Coding guidelines
985-996: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
history_is_boundeddoesn't exercise the bound.One transition against
max_history = 3can never overflow, so the assertion is vacuous. Drive the record through a full lifecycle (Planned → Resolving → Loading → Warming → Serving → Draining → Unloading → Stopped) and assertfull_history().len() == 3plus that the oldest entry was evicted.🤖 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/instance_lifecycle.rs` around lines 985 - 996, Update the test history_is_bounded to perform the full valid lifecycle sequence Planned → Resolving → Loading → Warming → Serving → Draining → Unloading → Stopped, using the existing transition and drain-reset APIs as needed. Then assert full_history().len() == 3 and verify the oldest history entry was evicted, rather than only checking that the current length is at most three.crates/mesh-llm-host-runtime/src/runtime/model_reconciliation/planner.rs (1)
727-738: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssertion doesn't prove which profile was selected.
enabled_policy()keeps the defaultmax_loads_per_tick(1, permax_loads_per_tick_caps_eligible_targets), soactions.len() == 1holds regardless of whether the planner picked the default or the cooled-downlow-ctxprofile. Assert the profile to actually cover the claim in the message.💚 Proposed test tightening
- assert_eq!( - actions.len(), - 1, - "low-ctx cooldown must not suppress the default profile" - ); + assert_eq!(actions.len(), 1); + assert_eq!( + actions[0].profile, "", + "low-ctx cooldown must not suppress the default profile" + );🤖 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/model_reconciliation/planner.rs` around lines 727 - 738, Strengthen the assertion in the reconciliation test around plan_model_target_reconciliation so it verifies that the sole action targets the default profile, rather than checking only actions.len(). Inspect the returned action’s profile identifier and assert it is not the cooled-down “low-ctx” profile, preserving the existing cooldown scenario and message.crates/mesh-llm-host-runtime/src/runtime/model_lifecycle/load.rs (1)
5-206: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider splitting
run_auto_load_runtime_modelinto smaller named helpers.This single function now spans path resolution, planning-byte computation with a spawn_blocking fallback, capacity reservation, launch + error-path cleanup, survey telemetry, instance/target registration, dashboard/console updates, exit-event wiring, and lifecycle-record setup — roughly 200 lines and a dozen-plus responsibilities. As per coding guidelines,
**/*.rs: "Do not add Rust methods or functions over the configured Clippy line-count or cognitive-complexity limits; split complex logic into semantically named helpers." Breaking this into stages (e.g. plan → launch → register → publish) would make each stage independently testable and easier to reason about, particularly since this is a core model-loading path.The error-cleanup ordering itself (dropping the capacity reservation and removing the serving assignment on launch failure) looks correct as written.
🤖 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/model_lifecycle/load.rs` around lines 5 - 206, Split run_auto_load_runtime_model into semantically named helpers for the distinct stages of path/model planning, capacity reservation, launch with failure cleanup, registration, publication, exit-event wiring, and lifecycle setup. Keep the existing behavior and data flow unchanged, especially dropping capacity_reservation before removing the serving assignment when start_runtime_local_model fails. Leave run_auto_load_runtime_model as a concise orchestration function that invokes these helpers.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/runtime/model_lifecycle/unload.rs (1)
251-262: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGuard the
Stoppedtransition like the managed path to avoid spurious warnings.When
mark_drainingfails (draining == false), the record never reachesUnloading, so this unconditionaltransition_to(Stopped)will log a stale-state warning on every forced unload. The managed path at Lines 84-87 already checksstate() == Unloadingfirst.♻️ Suggested alignment
- if let Err(error) = lifecycle - .lock() - .await - .transition_to(InstanceLifecycleState::Stopped) - { - tracing::warn!( - model, - instance_id = unload.instance_id, - %error, - "runtime instance stopped with a stale lifecycle state" - ); + { + let mut record = lifecycle.lock().await; + if record.state() == InstanceLifecycleState::Unloading + && let Err(error) = record.transition_to(InstanceLifecycleState::Stopped) + { + tracing::warn!( + model, + instance_id = unload.instance_id, + %error, + "runtime instance stopped with a stale lifecycle state" + ); + } }🤖 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/model_lifecycle/unload.rs` around lines 251 - 262, Guard the `transition_to(InstanceLifecycleState::Stopped)` call in the forced-unload path by first checking the lifecycle state is `Unloading`, matching the managed path’s existing state check. Only attempt the transition and emit its warning when that state is present; otherwise skip both.crates/mesh-llm-host-runtime/src/mesh/node.rs (1)
1566-1586: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer a named error over
Err(()).
Result<Option<InstanceRequestGuard>, ()>forces callers to invent their own rejection reason and cannot distinguish draining from paused/stopped, which matters for the admission responses this PR adds. A small enum (InstanceAdmissionError::NotAccepting { state }) keeps the ingress error mapping honest.🤖 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 1566 - 1586, Replace the unit error in begin_runtime_instance_request with a named InstanceAdmissionError that carries the current lifecycle state via a NotAccepting variant. Update the function’s Result type and construct this error when record.is_accepting_work() is false, preserving Ok(None) for missing lifecycles and the existing guard creation for accepted requests; adjust callers to map the named state into their admission responses.crates/mesh-llm-host-runtime/src/mesh/tests/owner_control.rs (1)
249-252: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a builder/
..Default::default()forOwnerControlRequestin tests.The four new
Nonefields are now repeated across ~9 envelopes; every future protocol field addition will require the same mechanical churn. A smallowner_control_request(request_id)helper (or..Default::default()if prost derives it) would localize that.Also applies to: 976-979
🤖 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 249 - 252, Reduce repeated OwnerControlRequest initialization in the tests by introducing a shared owner_control_request(request_id) helper or using ..Default::default() if OwnerControlRequest supports it. Centralize the four optional model fields and update all repeated envelopes, including the additional occurrence, to use the shared construction so future protocol fields require changes in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e55a187e-6db1-474e-926e-1e197e17f159
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (112)
Justfilecrates/mesh-client/src/client/control_plane.rscrates/mesh-client/tests/control_plane_client.rscrates/mesh-client/tests/protocol_wire.rscrates/mesh-llm-cli/src/runtime.rscrates/mesh-llm-config/Cargo.tomlcrates/mesh-llm-config/src/lib.rscrates/mesh-llm-config/src/model.rscrates/mesh-llm-config/src/model/built_in_schema.rscrates/mesh-llm-config/src/model/built_in_schema/control_behavior/runtime_controls.rscrates/mesh-llm-config/src/model/runtime.rscrates/mesh-llm-config/src/validate.rscrates/mesh-llm-gpu-bench/build.rscrates/mesh-llm-host-runtime/src/api/access.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/api/routes/runtime_activity.rscrates/mesh-llm-host-runtime/src/api/state.rscrates/mesh-llm-host-runtime/src/api/status.rscrates/mesh-llm-host-runtime/src/api/status/runtime.rscrates/mesh-llm-host-runtime/src/api/tests/mod.rscrates/mesh-llm-host-runtime/src/api/tests/node_state.rscrates/mesh-llm-host-runtime/src/api/tests/runtime_lifecycle_routes.rscrates/mesh-llm-host-runtime/src/api/tests/support.rscrates/mesh-llm-host-runtime/src/mesh/gossip.rscrates/mesh-llm-host-runtime/src/mesh/mod.rscrates/mesh-llm-host-runtime/src/mesh/node.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/commands/mod.rscrates/mesh-llm-host-runtime/src/mesh/owner_control/commands/model_lifecycle.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/owner_control_response.rscrates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache.rscrates/mesh-llm-host-runtime/src/mesh/owner_lifecycle_cache/tests.rscrates/mesh-llm-host-runtime/src/mesh/peer_state.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/control_plane_unique.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_frames.rscrates/mesh-llm-host-runtime/src/network/nostr/model_packs.rscrates/mesh-llm-host-runtime/src/network/openai/ingress.rscrates/mesh-llm-host-runtime/src/network/openai/response/routing.rscrates/mesh-llm-host-runtime/src/network/openai/transport.rscrates/mesh-llm-host-runtime/src/network/tunnel.rscrates/mesh-llm-host-runtime/src/protocol/control_frames.rscrates/mesh-llm-host-runtime/src/protocol/convert.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/mesh_timestamps.rscrates/mesh-llm-host-runtime/src/runtime/activity_policy.rscrates/mesh-llm-host-runtime/src/runtime/auto_join.rscrates/mesh-llm-host-runtime/src/runtime/control_loop.rscrates/mesh-llm-host-runtime/src/runtime/daemon_startup.rscrates/mesh-llm-host-runtime/src/runtime/instance_lifecycle.rscrates/mesh-llm-host-runtime/src/runtime/interactive.rscrates/mesh-llm-host-runtime/src/runtime/local.rscrates/mesh-llm-host-runtime/src/runtime/local_split/test_support.rscrates/mesh-llm-host-runtime/src/runtime/mod.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/load.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/reconciliation.rscrates/mesh-llm-host-runtime/src/runtime/model_lifecycle/unload.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/intent.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/mod.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/planner.rscrates/mesh-llm-host-runtime/src/runtime/model_reconciliation/state.rscrates/mesh-llm-host-runtime/src/runtime/publication.rscrates/mesh-llm-host-runtime/src/runtime/run_auto.rscrates/mesh-llm-host-runtime/src/runtime/serving_surface.rscrates/mesh-llm-host-runtime/src/runtime/startup_handles.rscrates/mesh-llm-host-runtime/src/runtime/startup_models.rscrates/mesh-llm-host-runtime/src/runtime/tests/model_lifecycle.rscrates/mesh-llm-host-runtime/src/runtime/tests/startup_models.rscrates/mesh-llm-host-runtime/src/runtime_data/api_views.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-system/src/activity/linux.rscrates/mesh-llm-system/src/activity/macos.rscrates/mesh-llm-system/src/activity/mod.rscrates/mesh-llm-system/src/activity/unsupported.rscrates/mesh-llm-system/src/activity/windows.rscrates/mesh-llm-system/src/lib.rscrates/mesh-llm-ui/e2e/perf/meshviz-200.spec.tscrates/mesh-llm-ui/src/features/app-tabs/types.tscrates/mesh-llm-ui/src/features/configuration/api/config-adapter.tscrates/mesh-llm-ui/src/features/configuration/api/runtime-settings.test.tsxcrates/mesh-llm-ui/src/features/configuration/api/runtime-settings.tscrates/mesh-llm/src/commands/runtime.rscrates/mesh-llm/tests/protocol_convert_matrix.rsdocs/CLI.mddocs/MESHES.mddocs/USAGE.mddocs/design/TESTING.mddocs/design/message_protocol.mdscripts/build-linux.shscripts/build-llama.shscripts/ci-sdk-fixture.shscripts/lib/cuda-toolkit.shscripts/qa-control-plane-mixed-version.shscripts/qa-runtime-daemon-lifecycle.shscripts/tests/test_cuda_toolkit.pytools/xtask/src/main.rstools/xtask/src/repo_consistency.rswebsite/src/docs/pages/CLI.md
💤 Files with no reviewable changes (1)
- crates/mesh-llm-host-runtime/src/protocol/control_frames.rs
Introduce persistent runtime lifecycle reconciliation with authenticated owner controls, profile-aware load, unload, ensure, and drain semantics, activity-based admission and priority policy, and additive gossip/protocol support. Expose the lifecycle through config, CLI, UI, and management APIs; consolidate shared owner-control protocol handling; and add cross-platform build and QA coverage for CUDA setup, mixed versions, process teardown, and SDK/platform paths.
98b183c to
1ae8f99
Compare
i386
left a comment
i386
left a comment
There was a problem hiding this comment.
Found one concurrency/identity issue in the reconciliation load path.
i386
left a comment
i386
left a comment
There was a problem hiding this comment.
Happy for you to merge after that one comment is resolved
|
@i386 SG! let's let this one soak a bit this week and schedule it for 0.75.0 |
* origin/main: Hand npm publishing to mesh-packaging feat(runtime): add daemon model lifecycle reconciliation (#1082) Accept bounded external token proposals in local Skippy generation (#1081) Expose target-authoritative local generation receipts (#1080) # Conflicts: # crates/mesh-llm-host-runtime/src/runtime/model_lifecycle.rs
2 participants
Summary
Audit fixes
Validation
cargo test -p mesh-llm-host-runtime --lib— 1810 passed, 8 ignoredcargo test -p mesh-llm-client -p mesh-llm-protocol -p mesh-llm --tests— passedcargo fmt --all -- --checkand full branchgit diff --check— passedjust build— passed; clean commit binary built and codesignedEvidence roots:
/tmp/mesh-llm-lifecycle-rebased/runtime-daemon-lifecycle-20260726T044251Z-23316/tmp/mesh-llm-mixed-rebased/control-plane-mixed-version-20260726T044342Z-23957Summary by CodeRabbit
New Features
load-model,unload-model,ensure-model, anddrain-modelvia CLI and runtime REST endpoints./api/status, runtime intents, and mesh gossip with coarse inference admission state.Bug Fixes
Chores