diff --git a/apps/rocm/src/automations.rs b/apps/rocm/src/automations.rs index 3727c39ae..168916e5e 100644 --- a/apps/rocm/src/automations.rs +++ b/apps/rocm/src/automations.rs @@ -2,19 +2,53 @@ // // SPDX-License-Identifier: MIT -//! `rocm automations` command handler. +//! `rocm automations` command handler, the automation-status/proposal +//! rendering it prints, and the `rocm daemon --status` background-helper +//! summary. //! //! Mechanically relocated from `main.rs` with no behavior change — the -//! `dispatch()` call site stays `automations(command)` (re-imported via -//! `use crate::automations::automations;`). Render/policy helpers remain in the -//! crate root and are reached through `crate::` (root items are visible to this -//! descendant module). +//! `dispatch()` call site stays `automations(command)`, re-imported with +//! `render_daemon_text` via +//! `use crate::automations::{automations, render_daemon_text};`. As of +//! Phase 6f (ROCMAI-210, see `docs/architecture.md`) this module also owns the +//! rendering/classification +//! helpers behind `render_automations_text` (the `rocm automations` status +//! report: watcher status/policy lines, recent automation activity, and +//! plain-language proposal/audit summaries) and `render_daemon_text` (the +//! `rocm daemon --status` summary) — both folded in from `main.rs` alongside +//! the `update_cmd.rs` split in the same PR. +//! +//! This module owns: `render_automations_text`, the `ProposalKind` +//! classification it renders through (`proposal_kind`, +//! `fallback_tool_for_automation_action`, `proposal_plain_summary`, +//! `proposal_plain_reason`), the small plain-language label helpers +//! (`proposal_status_label`, `proposal_bool_argument`, +//! `automation_event_plain_summary`, `audit_event_plain_summary`, +//! `watcher_mode_plain_label`, `watcher_last_check`, `watcher_plain_name`, +//! `watcher_plain_trigger`, `watcher_plain_action`, `watcher_policy_note`), +//! and `render_daemon_text`. `render_automations_text` and +//! `render_daemon_text` are `pub(crate)`: `main.rs`'s `dispatch()` calls +//! `render_daemon_text` directly (via the re-import above), and `chat.rs`'s +//! deterministic tool summaries call both. Everything else here has no +//! caller outside this module (including `watcher_policy_note`, now that the +//! `automations()` handler above reaches it locally rather than through a +//! crate-root re-import) and stays private. +//! +//! The cluster's regression tests live in this module's own `mod tests`, +//! reaching the shared `test_paths` fixture through `crate::tests::` the +//! same way other mechanically relocated modules do. + +use std::fmt::Write as _; use anyhow::{Result, bail}; -use rocm_core::{AppPaths, RocmCliConfig, builtin_watcher}; +use rocm_core::{ + AppPaths, AuditEventRecord, AutomationEventRecord, AutomationProposalRecord, + AutomationRuntimeState, RocmCliConfig, WatcherMode, builtin_watcher, builtin_watchers, + load_recent_audit_events, load_recent_automation_events, load_recent_automation_proposals, +}; use crate::service_launch::ensure_background_helper_running; -use crate::{AutomationsCommand, render_automations_text, watcher_policy_note}; +use crate::{AutomationsCommand, format_bytes}; pub(crate) fn automations(command: Option) -> Result<()> { let paths = AppPaths::discover()?; @@ -66,3 +100,555 @@ pub(crate) fn automations(command: Option) -> Result<()> { } Ok(()) } + +pub(crate) fn render_automations_text(paths: &AppPaths, config: &RocmCliConfig) -> Result { + let runtime_state = AutomationRuntimeState::load(paths).unwrap_or(None); + let recent_events = load_recent_automation_events(paths, 5).unwrap_or_default(); + let recent_proposals = load_recent_automation_proposals(paths, 5).unwrap_or_default(); + let recent_audit_events = load_recent_audit_events(paths, 5).unwrap_or_default(); + let mut output = String::new(); + let _ = writeln!(output, "automation checks"); + let _ = writeln!(output, " config: {}", paths.config_path().display()); + let _ = writeln!( + output, + " background checks: {}", + if config.automation_daemon_enabled() { + "on" + } else { + "off" + } + ); + if let Some(state) = runtime_state.as_ref() { + let _ = writeln!( + output, + " background service: {}", + if state.running { "running" } else { "stopped" } + ); + let _ = writeln!( + output, + " local event intake: {}", + state + .local_webhook_endpoint + .as_deref() + .unwrap_or("disabled") + ); + } else { + let _ = writeln!(output, " background service: not running"); + let _ = writeln!(output, " local event intake: disabled"); + } + for watcher in builtin_watchers() { + let runtime_snapshot = runtime_state.as_ref().and_then(|state| { + state + .active_watchers + .iter() + .find(|item| item.id == watcher.id) + }); + let _ = writeln!( + output, + " {} ({})", + watcher_plain_name(watcher.id), + if config.watcher_enabled(watcher) { + "on" + } else { + "off" + } + ); + let _ = writeln!( + output, + " setting: {}", + watcher_mode_plain_label(config.effective_watcher_mode(watcher)) + ); + let _ = writeln!( + output, + " listens for: {}", + watcher_plain_trigger(watcher.id) + ); + let _ = writeln!(output, " does: {}", watcher_plain_action(watcher.id)); + if let Some(note) = watcher_policy_note(watcher.id) { + let _ = writeln!(output, " policy: {note}"); + } + if let Some(snapshot) = runtime_snapshot { + let _ = writeln!(output, " last check: {}", watcher_last_check(snapshot)); + } + } + if !recent_events.is_empty() { + let _ = writeln!(output, " recent automation activity:"); + for event in recent_events { + let _ = writeln!(output, " {}", automation_event_plain_summary(&event)); + if let Some(service_id) = event.service_id.as_deref() { + let _ = writeln!(output, " server: {service_id}"); + } + } + } + if !recent_proposals.is_empty() { + let _ = writeln!(output, " recent review requests:"); + for proposal in recent_proposals { + let _ = writeln!( + output, + " {} [{}] {}", + proposal.proposal_id, + proposal_status_label(&proposal.status), + proposal_plain_summary(&proposal) + ); + let _ = writeln!(output, " why: {}", proposal_plain_reason(&proposal)); + if let Some(service_id) = proposal.service_id.as_deref() { + let _ = writeln!(output, " server: {service_id}"); + } + if let Some(artifact_ref) = proposal + .arguments + .get("artifact_ref") + .and_then(serde_json::Value::as_str) + { + let _ = writeln!(output, " model file: {artifact_ref}"); + } + if proposal_bool_argument(&proposal, "allow_artifact_download") { + let limit = proposal + .arguments + .get("artifact_max_bytes") + .and_then(serde_json::Value::as_u64) + .map_or_else(|| "not set".to_owned(), format_bytes); + let _ = writeln!(output, " download: approved up to {limit}"); + } else if proposal_kind(&proposal) == ProposalKind::PrefetchArtifact { + let _ = writeln!(output, " download: not approved yet"); + } + if proposal_kind(&proposal) == ProposalKind::DriverPlan { + let _ = writeln!( + output, + " effect: show a driver plan only; no driver install" + ); + } + if proposal.status == "pending" { + let _ = writeln!( + output, + " controls: /automations approve {} | /automations reject {}", + proposal.proposal_id, proposal.proposal_id + ); + } + } + } + if !recent_audit_events.is_empty() { + let _ = writeln!(output, " recent background activity:"); + for event in recent_audit_events { + let _ = writeln!(output, " {}", audit_event_plain_summary(&event)); + if let Some(service_id) = event.service_id.as_deref() { + let _ = writeln!(output, " server: {service_id}"); + } + } + } + Ok(output) +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ProposalKind { + RestartServer, + StopServer, + CheckUpdates, + PrefetchArtifact, + DriverPlan, + Other, +} + +fn proposal_kind(proposal: &AutomationProposalRecord) -> ProposalKind { + match proposal + .tool + .as_deref() + .or_else(|| fallback_tool_for_automation_action(&proposal.action)) + { + Some("restart_server") => ProposalKind::RestartServer, + Some("stop_server") => ProposalKind::StopServer, + Some("check_updates") => ProposalKind::CheckUpdates, + Some("prefetch_artifact") => ProposalKind::PrefetchArtifact, + Some("driver_plan") => ProposalKind::DriverPlan, + _ => ProposalKind::Other, + } +} + +fn fallback_tool_for_automation_action(action: &str) -> Option<&'static str> { + match action { + "queue_restart_proposal" => Some("restart_server"), + "queue_stop_server_proposal" => Some("stop_server"), + "queue_update_proposal" => Some("check_updates"), + "queue_prefetch_proposal" => Some("prefetch_artifact"), + "prepare_driver_plan" => Some("driver_plan"), + _ => None, + } +} + +fn proposal_plain_summary(proposal: &AutomationProposalRecord) -> &'static str { + match proposal_kind(proposal) { + ProposalKind::RestartServer => "Restart a model server", + ProposalKind::StopServer => "Stop a model server", + ProposalKind::CheckUpdates => "Check for ROCm updates", + ProposalKind::PrefetchArtifact => "Prepare a model file", + ProposalKind::DriverPlan => "Show a driver plan", + ProposalKind::Other => "Review an automation request", + } +} + +fn proposal_plain_reason(proposal: &AutomationProposalRecord) -> &'static str { + match proposal_kind(proposal) { + ProposalKind::RestartServer => "A managed server looks stopped or unhealthy.", + ProposalKind::StopServer => "GPU pressure is high and serving should be reviewed.", + ProposalKind::CheckUpdates => "A scheduled update check is due.", + ProposalKind::PrefetchArtifact => "rocm-cli was asked to prepare this model file.", + ProposalKind::DriverPlan => "A driver update signal was received.", + ProposalKind::Other => "An enabled automation asked for review.", + } +} + +fn proposal_status_label(status: &str) -> &'static str { + match status { + "pending" => "waiting for review", + "approved" => "approved", + "completed" => "done", + "rejected" => "rejected", + "failed" => "failed", + _ => "status unknown", + } +} + +fn proposal_bool_argument(proposal: &AutomationProposalRecord, key: &str) -> bool { + proposal + .arguments + .get(key) + .and_then(serde_json::Value::as_bool) + .unwrap_or(false) +} + +fn automation_event_plain_summary(event: &AutomationEventRecord) -> &'static str { + match event.watcher_id.as_str() { + "therock-update" => "ROCm update check recorded.", + "server-recover" => "Server recovery check recorded.", + "gpu-metrics" => "GPU status check recorded.", + "gpu-thermal-protect" => "GPU pressure check recorded.", + "cache-warm" => "Model file preparation request recorded.", + "driver-upgrade" => "Driver update review recorded.", + _ => "Automation activity recorded.", + } +} + +fn audit_event_plain_summary(event: &AuditEventRecord) -> &'static str { + match event.category.as_str() { + "automation" => "Automation activity was recorded.", + "proposal" => "A review request changed status.", + "provider" => "Provider request completed.", + "service" => "Managed server activity was recorded.", + "install" => "Install activity was recorded.", + "update" => "Update activity was recorded.", + "runtime" => "ROCm runtime activity was recorded.", + "engine" => "Engine activity was recorded.", + _ => "Background activity was recorded.", + } +} + +const fn watcher_mode_plain_label(mode: WatcherMode) -> &'static str { + match mode { + WatcherMode::Observe => "record only", + WatcherMode::Propose => "ask before taking action", + WatcherMode::Contained => "ask before changes; keep actions limited", + } +} + +fn watcher_last_check(snapshot: &rocm_core::WatcherRuntimeSnapshot) -> &'static str { + let Some(event) = snapshot.last_event.as_deref() else { + return "not yet"; + }; + match event { + "remind_update_check" => "ROCm update check recorded", + "queue_update_proposal" => "ROCm update review requested", + "collect_failure_snapshot" => "server health check recorded", + "restart_managed_service" | "queue_restart_proposal" => "server restart review requested", + "record_gpu_metrics" => "GPU status check recorded", + "queue_stop_server_proposal" => "GPU pressure review requested", + "queue_prefetch_proposal" => "model file review requested", + "prepare_driver_plan" => "driver plan review requested", + _ => "activity recorded", + } +} + +fn watcher_plain_name(watcher_id: &str) -> &'static str { + match watcher_id { + "therock-update" => "ROCm update checks", + "server-recover" => "Server recovery", + "gpu-metrics" => "GPU status checks", + "gpu-thermal-protect" => "GPU pressure protection", + "cache-warm" => "Model file preparation", + "driver-upgrade" => "Driver update review", + _ => "Automation check", + } +} + +fn watcher_plain_trigger(watcher_id: &str) -> &'static str { + match watcher_id { + "therock-update" => "a scheduled check", + "server-recover" => "a server that stops or becomes unhealthy", + "gpu-metrics" => "local GPU status updates", + "gpu-thermal-protect" => "high GPU temperature or memory pressure", + "cache-warm" => "a request to prepare a model file", + "driver-upgrade" => "a driver update signal", + _ => "local automation activity", + } +} + +fn watcher_plain_action(watcher_id: &str) -> &'static str { + match watcher_id { + "therock-update" => "checks for ROCm updates without installing them", + "server-recover" => "asks before restarting a managed server", + "gpu-metrics" => "records local GPU status only", + "gpu-thermal-protect" => "asks before stopping a managed server", + "cache-warm" => "asks before preparing or downloading a model file", + "driver-upgrade" => "shows a driver plan only", + _ => "records the event", + } +} + +fn watcher_policy_note(watcher_id: &str) -> Option<&'static str> { + match watcher_id { + "gpu-metrics" => Some( + "read-only telemetry; propose/contained modes record events only and do not create review requests or mutate services", + ), + "gpu-thermal-protect" => Some( + "GPU pressure protection is review-gated; propose/contained modes queue a stop review only and never stop servers automatically", + ), + "cache-warm" => Some( + "artifact prefetch stays review-gated; contained mode creates a review request instead of downloading without reviewed source policy", + ), + "driver-upgrade" => Some( + "local driver update signals stay review-gated; review requests run a read-only driver plan and never install drivers automatically", + ), + _ => None, + } +} + +pub(crate) fn render_daemon_text(paths: &AppPaths, config: &RocmCliConfig) -> String { + let runtime_state = AutomationRuntimeState::load(paths).unwrap_or(None); + let mut output = String::new(); + let _ = writeln!(output, "Background helper"); + let _ = writeln!(output); + let _ = writeln!( + output, + "Status: {}", + match runtime_state { + Some(ref state) if state.running => "running", + _ => "not running", + } + ); + let _ = writeln!(output, "Startup: on demand"); + let _ = writeln!( + output, + "Why it runs: automation checks and local model servers use it when needed." + ); + let _ = writeln!( + output, + "Automation checks: {}", + if config.automation_daemon_enabled() { + "on" + } else { + "off" + } + ); + let _ = writeln!(output, "Saved state: kept on this computer"); + let _ = writeln!(output); + let _ = writeln!(output, "Choose Automations to review background checks."); + let _ = writeln!(output, "Choose Local servers to see local model servers."); + output +} + +#[cfg(test)] +mod tests { + use std::fs; + + use anyhow::Result; + + use super::*; + use crate::tests::test_paths; + + #[test] + fn render_automations_text_marks_gpu_metrics_read_only() -> Result<()> { + let (root, paths) = test_paths("automation-status-policy"); + let mut config = RocmCliConfig::default(); + let watcher = config.watcher_config_mut("gpu-metrics"); + watcher.enabled = true; + watcher.mode = Some(WatcherMode::Contained); + config.automations.daemon_enabled = true; + + let rendered = render_automations_text(&paths, &config)?; + let _ = fs::remove_dir_all(root); + + assert!(rendered.contains("GPU status checks (on)")); + assert!(rendered.contains("setting: ask before changes; keep actions limited")); + assert!(rendered.contains("listens for: local GPU status updates")); + assert!(rendered.contains("does: records local GPU status only")); + assert!(rendered.contains("policy: read-only telemetry")); + assert!(rendered.contains("do not create review requests or mutate services")); + assert!(!rendered.contains("command id:")); + assert!(!rendered.contains("mode:")); + assert!(!rendered.contains("gpu-metrics")); + Ok(()) + } + + #[test] + fn render_automations_text_marks_gpu_thermal_protect_review_gated() -> Result<()> { + let (root, paths) = test_paths("automation-gpu-thermal-policy"); + let mut config = RocmCliConfig::default(); + let watcher = config.watcher_config_mut("gpu-thermal-protect"); + watcher.enabled = true; + watcher.mode = Some(WatcherMode::Contained); + config.automations.daemon_enabled = true; + + let rendered = render_automations_text(&paths, &config)?; + let _ = fs::remove_dir_all(root); + + assert!(rendered.contains("GPU pressure protection (on)")); + assert!(rendered.contains("setting: ask before changes; keep actions limited")); + assert!(rendered.contains("listens for: high GPU temperature or memory pressure")); + assert!(rendered.contains("does: asks before stopping a managed server")); + assert!(rendered.contains("policy: GPU pressure protection is review-gated")); + assert!(rendered.contains("never stop servers automatically")); + assert!(!rendered.contains("command id:")); + assert!(!rendered.contains("mode:")); + assert!(!rendered.contains("gpu-thermal-protect")); + Ok(()) + } + + #[test] + fn render_automations_text_surfaces_local_webhook_endpoint() -> Result<()> { + let (root, paths) = test_paths("automation-local-webhook"); + AutomationRuntimeState { + running: true, + automations_enabled: true, + daemon_pid: 123, + started_at_unix_ms: 1, + last_tick_unix_ms: 2, + local_webhook_endpoint: Some("http://127.0.0.1:19191/automation-events".to_owned()), + active_watchers: vec![rocm_core::WatcherRuntimeSnapshot { + id: "gpu-metrics".to_owned(), + enabled: true, + mode: WatcherMode::Observe, + summary: "record metrics".to_owned(), + last_event: Some("record_gpu_metrics".to_owned()), + last_event_unix_ms: Some(2), + }], + } + .write(&paths)?; + + let rendered = render_automations_text(&paths, &RocmCliConfig::default())?; + let _ = fs::remove_dir_all(root); + + assert!(rendered.contains("background service: running")); + assert!(rendered.contains("local event intake: http://127.0.0.1:19191/automation-events")); + assert!(rendered.contains("last check: GPU status check recorded")); + assert!(!rendered.contains("pid=123")); + assert!(!rendered.contains("last_tick_unix_ms")); + assert!(!rendered.contains("local_webhook_endpoint")); + assert!(!rendered.contains("record_gpu_metrics")); + assert!(!rendered.contains("gpu-metrics")); + Ok(()) + } + + #[test] + fn render_automations_text_uses_plain_proposal_history() -> Result<()> { + let (root, paths) = test_paths("automation-plain-proposal-history"); + rocm_core::append_automation_proposal( + &paths, + &AutomationProposalRecord { + at_unix_ms: 42, + proposal_id: "proposal-plain".to_owned(), + watcher_id: "server-recover".to_owned(), + action: "queue_restart_proposal".to_owned(), + title: "queue restart proposal".to_owned(), + message: "backend-authored recovery message".to_owned(), + status: "pending".to_owned(), + service_id: Some("svc-plain".to_owned()), + tool: Some("restart_server".to_owned()), + arguments: serde_json::json!({ "service_id": "svc-plain" }), + reviewed_at_unix_ms: None, + }, + )?; + rocm_core::append_automation_proposal( + &paths, + &AutomationProposalRecord { + at_unix_ms: 43, + proposal_id: "proposal-file".to_owned(), + watcher_id: "cache-warm".to_owned(), + action: "queue_prefetch_proposal".to_owned(), + title: "queue prefetch proposal".to_owned(), + message: "cache warm requested internal artifact prefetch".to_owned(), + status: "pending".to_owned(), + service_id: None, + tool: Some("prefetch_artifact".to_owned()), + arguments: serde_json::json!({ + "artifact_ref": "tiny/model#gguf", + "allow_artifact_download": true, + // One byte under 1 MiB: `{:.1}` rounds it up to a full + // 1024 KiB, so the approved cap must read as "1.0 MiB", + // not "1024.0 KiB". It must also read as `MiB`, not `MB` + // — this is base-1024 math, not base-1000 (issue #190). + "artifact_max_bytes": 1_048_575 + }), + reviewed_at_unix_ms: None, + }, + )?; + rocm_core::append_automation_event( + &paths, + &rocm_core::AutomationEventRecord { + at_unix_ms: 44, + watcher_id: "gpu-metrics".to_owned(), + level: "info".to_owned(), + action: "record_gpu_metrics".to_owned(), + message: "raw amd-smi telemetry event".to_owned(), + service_id: None, + }, + )?; + rocm_core::append_audit_event( + &paths, + &AuditEventRecord { + at_unix_ms: 45, + source: "test".to_owned(), + category: "proposal".to_owned(), + actor: "watcher:server-recover".to_owned(), + level: "info".to_owned(), + action: "proposal_approved".to_owned(), + message: "approved proposal-plain with backend detail".to_owned(), + watcher_id: Some("server-recover".to_owned()), + service_id: Some("svc-plain".to_owned()), + }, + )?; + + let rendered = render_automations_text(&paths, &RocmCliConfig::default())?; + let _ = fs::remove_dir_all(root); + + assert!(rendered.contains("recent automation activity:")); + assert!(rendered.contains("GPU status check recorded.")); + assert!(rendered.contains("recent review requests:")); + assert!(rendered.contains("proposal-plain [waiting for review] Restart a model server")); + assert!(rendered.contains("why: A managed server looks stopped or unhealthy.")); + assert!(rendered.contains("server: svc-plain")); + assert!(rendered.contains("proposal-file [waiting for review] Prepare a model file")); + assert!(rendered.contains("model file: tiny/model#gguf")); + assert!(rendered.contains("download: approved up to 1.0 MiB")); + assert!(rendered.contains( + "controls: /automations approve proposal-plain | /automations reject proposal-plain" + )); + assert!(rendered.contains("recent background activity:")); + assert!(rendered.contains("A review request changed status.")); + assert!(!rendered.contains("queue_restart_proposal")); + assert!(!rendered.contains("restart_server")); + assert!(!rendered.contains("queue restart proposal")); + assert!(!rendered.contains("backend-authored recovery message")); + assert!(!rendered.contains("record_gpu_metrics")); + assert!(!rendered.contains("raw amd-smi telemetry event")); + assert!(!rendered.contains("proposal_approved")); + assert!(!rendered.contains("approved proposal-plain with backend detail")); + assert!(!rendered.contains("created_unix_ms")); + assert!(!rendered.contains("last_tick_unix_ms")); + assert!(!rendered.contains("local_webhook_endpoint")); + assert!(!rendered.contains("command id:")); + assert!(!rendered.contains("mode:")); + assert!(!rendered.contains("1048576 bytes")); + assert!(!rendered.contains("recent events:")); + assert!(!rendered.contains("recent audit:")); + assert!(!rendered.contains("tool:")); + Ok(()) + } +} diff --git a/apps/rocm/src/chat.rs b/apps/rocm/src/chat.rs index ed6a054a9..114cad975 100644 --- a/apps/rocm/src/chat.rs +++ b/apps/rocm/src/chat.rs @@ -24,6 +24,7 @@ //! module; none of the module's types need to be, bar the one exception //! above. +use crate::automations::{render_automations_text, render_daemon_text}; use crate::freeform_planner::{ format_structured_tool_call, infer_engine_from_request, plan_freeform_request_with_context, serve_args_request_cpu_device, @@ -34,14 +35,15 @@ use crate::managed_service_status::{ use crate::runtimes_cmd::render_runtimes_text; use crate::service_launch::is_loopback_host; use crate::uninstall_plan::render_uninstall_dry_run; +use crate::update_cmd::render_update_text; use crate::{ ChatInferenceParams, apply_app_path_env, build_codex_bridge_gpu_snapshot, build_codex_bridge_snapshot, builtin_codex_bridge_engine_inventory, cli_version_string, - read_tail_lines, record_cli_audit_event, render_automations_text, render_config_text, - render_daemon_text, render_engine_inventory_text_with_paths, render_examine_text, - render_examine_text_with_paths, render_logs_browser_text, render_logs_text, + read_tail_lines, record_cli_audit_event, render_config_text, + render_engine_inventory_text_with_paths, render_examine_text, render_examine_text_with_paths, + render_logs_browser_text, render_logs_text, render_model_registry_verbose_text_with_context_and_host, render_service_logs_text, - render_services_text, render_setup_status_text, render_update_text, service_model_names_match, + render_services_text, render_setup_status_text, service_model_names_match, therock_install_version_selector, validate_service_id, }; use crate::{chat_host_facts, comfyui, providers, therock}; diff --git a/apps/rocm/src/main.rs b/apps/rocm/src/main.rs index de84cbd43..710318db9 100644 --- a/apps/rocm/src/main.rs +++ b/apps/rocm/src/main.rs @@ -46,6 +46,7 @@ mod test_support; mod therock; mod uninstall; mod uninstall_plan; +mod update_cmd; // Per-command handlers mechanically relocated into modules — fns, plus the // types (e.g. `ServeArgs`, driver-plan types) that moved with their cluster. @@ -64,11 +65,16 @@ mod uninstall_plan; // `render_freeform_execution_header` are re-imported for the same reason: // `execute_freeform_next_action` (itself only called from `run_freeform`, // outside `dispatch()`) calls them. `install()`'s SDK path calls into -// `runtime_install_finalize` directly (not via `dispatch()`), -// and `apply_runtime_update`/`append_update_activate_summary` and several -// root-level chat/examine/update helpers call into `runtimes_cmd` directly for -// the same reason `engines_cmd`'s runtime-resolution helpers are re-imported. -use crate::automations::automations; +// `runtime_install_finalize` directly (not via `dispatch()`), and several +// root-level chat/examine helpers call into `runtimes_cmd` directly for the +// same reason `engines_cmd`'s runtime-resolution helpers are re-imported; +// `current_runtime_manifest`/`select_runtime_manifest` stay re-imported here +// for `single_ready_runtime_key`/`resolve_runtime_selector_to_exact_key` +// below, also outside `dispatch()`. `update_cmd.rs`'s +// `apply_runtime_update`/`append_update_activate_summary` call into +// `runtimes_cmd` directly too, via their own imports rather than this +// re-export. +use crate::automations::{automations, render_daemon_text}; use crate::driver_install::{install_driver, reconcile_driver_install}; use crate::engine_ipc::{ builtin_engine_available, builtin_engine_serve_http_args, engine_request, @@ -94,10 +100,10 @@ use crate::runtime_install_finalize::{ print_sdk_install_success, }; use crate::runtimes_cmd::{ - RuntimeActivationResult, activate_runtime, active_runtime_marker_path, - current_runtime_manifest, default_runtime_id_matches, recover_setup_runtime_registration, - runtime_keys_text, runtime_usability_status, runtimes, select_runtime_manifest, - setup_runtime_paths, toolchain_state_text, validate_runtime_manifest_for_activation, + active_runtime_marker_path, current_runtime_manifest, default_runtime_id_matches, + recover_setup_runtime_registration, runtime_keys_text, runtime_usability_status, runtimes, + select_runtime_manifest, setup_runtime_paths, toolchain_state_text, + validate_runtime_manifest_for_activation, }; use crate::serve_cmd::{ServeArgs, select_serve_engine, serve}; use crate::service_launch::ensure_public_service_has_endpoint_key; @@ -108,6 +114,7 @@ use crate::service_launch::{ }; use crate::uninstall::uninstall; use crate::uninstall_plan::UninstallOptions; +use crate::update_cmd::{apply_runtime_update, render_update_text, update_should_preview_or_apply}; use anyhow::{Context, Result, bail}; use clap::{CommandFactory, FromArgMatches, Parser, Subcommand, ValueEnum}; @@ -116,19 +123,18 @@ use rocm_core::model_readiness::{ AcceleratorMemory, HostEngineChoice, HostFacts, ModelCatalogSource, ModelReadiness, }; use rocm_core::{ - AppPaths, AuditEventRecord, AutomationEventRecord, AutomationProposalRecord, - AutomationRuntimeState, CodexBridgeEngine, CodexBridgeGpuSnapshot, CodexBridgeSnapshot, - DEFAULT_LOCAL_HOST, EndpointReadiness, ExamineSummary, ManagedServiceRecord, ModelRecipeRecord, - ModelRecipeRegistry, ModelRecipeRegistrySource, PERMISSIONS_MODE_ASK, - PERMISSIONS_MODE_FULL_ACCESS, RocmCliConfig, TELEMETRY_MODE_LOCAL, TELEMETRY_MODE_OFF, - WatcherMode, append_audit_event, builtin_model_recipes, builtin_watchers, connect_tcp_stream, + AppPaths, AuditEventRecord, AutomationRuntimeState, CodexBridgeEngine, CodexBridgeGpuSnapshot, + CodexBridgeSnapshot, DEFAULT_LOCAL_HOST, EndpointReadiness, ExamineSummary, + ManagedServiceRecord, ModelRecipeRecord, ModelRecipeRegistry, ModelRecipeRegistrySource, + PERMISSIONS_MODE_ASK, PERMISSIONS_MODE_FULL_ACCESS, RocmCliConfig, TELEMETRY_MODE_LOCAL, + TELEMETRY_MODE_OFF, WatcherMode, append_audit_event, builtin_model_recipes, connect_tcp_stream, daemon_binary_path, default_engine_for_platform, detect_host_gfx_target, detect_host_gpu_summary, engine_plugin_dirs, format_host_port, format_http_base_url, - interactive_terminal, load_model_recipe_registry, load_recent_audit_events, - load_recent_automation_events, load_recent_automation_proposals, managed_pip_cache_dir, - model_artifact_cache_status, model_catalog_platforms, model_recipe_featured, - model_recipe_target_platform_label, normalize_therock_family, platform_matches_gfx_family, - process_is_running, read_http_response_bounded, uv_cache_source, write_all_tcp_stream, + interactive_terminal, load_model_recipe_registry, load_recent_automation_events, + managed_pip_cache_dir, model_artifact_cache_status, model_catalog_platforms, + model_recipe_featured, model_recipe_target_platform_label, normalize_therock_family, + platform_matches_gfx_family, process_is_running, read_http_response_bounded, uv_cache_source, + write_all_tcp_stream, }; use rocm_engine_protocol::{ DEFAULT_LOG_TAIL_LINES, DetectRequest, DetectResponse, DevicePolicy, @@ -7473,197 +7479,6 @@ fn read_tail_lines(path: &Path, limit: usize, label: &str) -> Result Ok(lines.into_iter().collect()) } -pub(crate) fn render_update_text(paths: &AppPaths) -> Result { - let mut output = therock::render_update_report(paths)?; - append_update_surfaces(&mut output); - Ok(output) -} - -fn append_update_surfaces(output: &mut String) { - let _ = writeln!(output, " update_surfaces:"); - let _ = writeln!( - output, - " cli: installed={} status=not_configured reason=repository-owned CLI update feed is not published yet", - env!("CARGO_PKG_VERSION") - ); - let engine_ids = engine_inventory() - .iter() - .map(|(engine, _)| *engine) - .collect::>() - .join(","); - let _ = writeln!( - output, - " engines: status=package_managed packaged=[{engine_ids}] reason=first-party engine binaries update with the rocm-cli package; data-dir plugins are user-managed" - ); - match load_model_recipe_registry() { - Ok(registry) => match registry.source { - ModelRecipeRegistrySource::BuiltIn => { - let _ = writeln!( - output, - " model_recipes: status=built_in count={} reason=external signed recipe index is not configured", - registry.recipes.len() - ); - } - ModelRecipeRegistrySource::SignedIndex { - index_path, - signature_path, - public_key_path, - } => { - let _ = writeln!( - output, - " model_recipes: status=signed_index count={} index={} signature={} public_key={} reason=loaded signed recipe index; recipe update feed is not live in this build", - registry.recipes.len(), - index_path.display(), - signature_path.display(), - public_key_path.display() - ); - } - }, - Err(error) => { - let _ = writeln!( - output, - " model_recipes: status=error reason={}", - sanitize_log_value(&error.to_string()) - ); - } - } - let runtime_status = if output.contains(" managed runtimes: none") { - "none_configured" - } else { - "checked_above" - }; - let _ = writeln!( - output, - " runtimes: status={runtime_status} reason=TheRock runtime update checks above are the only live update checks in this build" - ); - let _ = writeln!( - output, - " note: `rocm update --apply` applies runtime updates only; CLI, engine, and recipe update feeds require published metadata before they can mutate state" - ); -} - -/// Whether `rocm update` should route into the runtime update path -/// (`apply_runtime_update`) instead of the read-only status report. -/// -/// `--dry-run` alone must take this path too, since `apply_runtime_update` -/// only mutates anything when `dry_run` is false — a plain status report -/// would silently ignore `--dry-run` and never show what `--apply` would do. -const fn update_should_preview_or_apply(apply: bool, dry_run: bool) -> bool { - apply || dry_run -} - -fn apply_runtime_update( - paths: &AppPaths, - config: &mut RocmCliConfig, - runtime_selector: Option<&str>, - activate: bool, - dry_run: bool, -) -> Result { - let manifests = therock::load_runtime_manifests(paths)?; - let source = select_runtime_update_source(&manifests, config, runtime_selector)?; - let plan = therock::runtime_update_plan(paths, source, &manifests, None)?; - let mut output = String::new(); - let _ = writeln!(output, "runtime update"); - let _ = writeln!(output, " source_runtime_key: {}", source.runtime_key); - let _ = writeln!(output, " source_runtime_id: {}", source.runtime_id); - let _ = writeln!(output, " channel: {}", source.channel); - let _ = writeln!(output, " format: {}", source.format); - let _ = writeln!(output, " family: {}", source.family); - let _ = writeln!( - output, - " installed_version: {}", - therock::runtime_version_display(&source.version) - ); - let _ = writeln!( - output, - " latest_version: {}", - therock::runtime_version_display(&plan.latest_version) - ); - let _ = writeln!(output, " status: {}", plan.status); - let _ = writeln!(output, " target_runtime_key: {}", plan.target_runtime_key); - let _ = writeln!(output, " activate_after_install: {activate}"); - if !plan.update_available { - let _ = writeln!(output, " result: no newer runtime found"); - return Ok(output); - } - - if dry_run { - let _ = writeln!(output, " mode: dry-run"); - let install_plan = therock::install_sdk_for_update( - paths, - &source.channel, - &source.format, - &source.family, - plan.device_target.as_deref(), - plan.source_layout_generation.as_deref(), - source.includes_devel(), - true, - activate, - )?; - let _ = writeln!(output, " install_plan:"); - for line in install_plan.output.lines() { - let _ = writeln!(output, " {line}"); - } - return Ok(output); - } - - // `activate` rather than a bare `true`: the update path is preapproved either - // way (its approval comes from the runtime the user selected, not from a - // flag, and `rocm update` has no terminal contract), but the approval line it - // prints must not promise an activation that only `--activate` performs - // below. - let install_output = therock::install_sdk_for_update( - paths, - &source.channel, - &source.format, - &source.family, - plan.device_target.as_deref(), - plan.source_layout_generation.as_deref(), - // Reinstall what the user originally chose rather than silently - // adding or dropping the compiler toolchain on update. - source.includes_devel(), - false, - activate, - )?; - let manifests_after = therock::load_runtime_manifests(paths)?; - // By exact key, never by version: a same-version repair installs a sibling - // that shares the source's channel, format, family AND version, so a - // version match would just as happily return the stale runtime this update - // was meant to replace, and then activate it. - let installed = select_installed_update_runtime(&manifests_after, &plan.target_runtime_key) - .with_context(|| { - format!( - "runtime install completed but no manifest was written for the planned runtime key `{}`", - plan.target_runtime_key - ) - })?; - let _ = writeln!(output, " installed_runtime_key: {}", installed.runtime_key); - let _ = writeln!( - output, - " installed_runtime_root: {}", - installed.install_root.display() - ); - if activate { - // No `config.save` after this: `activate_runtime` already persisted the - // same struct and nothing mutates it in between, so a second write can - // only fail after an activation that fully succeeded — sending the user - // to re-run a command that already worked. - let activation = activate_runtime(paths, config, &installed.runtime_key)?; - append_update_activate_summary(&mut output, &activation); - } else { - let _ = writeln!( - output, - " next step: rocm runtimes activate {}", - installed.runtime_key - ); - } - let _ = writeln!(output, " install_output:"); - for line in install_output.output.lines() { - let _ = writeln!(output, " {line}"); - } - Ok(output) -} - /// Shown after any activation (direct `runtimes activate`, `update --apply /// --activate`, or an `install sdk` that switches the active runtime) that /// recorded a previous runtime, so every path that reaches this state gives @@ -7673,422 +7488,6 @@ fn apply_runtime_update( const ROLLBACK_RECOVERY_HINT: &str = " next step: if this causes problems, run `rocm runtimes rollback` \ (no history - undoes only this one activation)"; -/// Appends the `--activate` branch of the `update --apply` summary. The -/// rollback hint is only shown when a previous runtime was actually recorded — -/// `rocm runtimes rollback` hard-errors otherwise (no prior runtime to return -/// to), so hinting it unconditionally would point users at a command that -/// immediately fails on their very first activation. -fn append_update_activate_summary(output: &mut String, activation: &RuntimeActivationResult) { - let _ = writeln!( - output, - " activated_runtime_key: {}", - activation.runtime_key - ); - let _ = writeln!( - output, - " previous_runtime_key: {}", - activation - .previous_runtime_key - .as_deref() - .unwrap_or("") - ); - let _ = write!( - output, - "{}", - runtime_services::render_runtime_service_reconciliation( - &activation.services, - &activation.runtime_key - ) - ); - if activation.previous_runtime_key.is_some() { - let _ = writeln!(output, "{ROLLBACK_RECOVERY_HINT}"); - } -} - -fn select_runtime_update_source<'a>( - manifests: &'a [therock::InstalledRuntimeManifest], - config: &RocmCliConfig, - runtime_selector: Option<&str>, -) -> Result<&'a therock::InstalledRuntimeManifest> { - if let Some(selector) = runtime_selector { - return select_runtime_manifest(manifests, selector); - } - if let Some(active) = current_runtime_manifest(config, manifests) { - return Ok(active); - } - match manifests { - [] => bail!( - "no managed runtimes are registered; run `rocm install sdk --channel release --format wheel` first" - ), - [only] => Ok(only), - _ => bail!( - "multiple runtimes are registered and no active runtime is configured; pass `--runtime `" - ), - } -} - -fn select_installed_update_runtime<'a>( - manifests: &'a [therock::InstalledRuntimeManifest], - target_runtime_key: &str, -) -> Option<&'a therock::InstalledRuntimeManifest> { - manifests - .iter() - .find(|manifest| manifest.runtime_key == target_runtime_key) -} - -pub(crate) fn render_automations_text(paths: &AppPaths, config: &RocmCliConfig) -> Result { - let runtime_state = AutomationRuntimeState::load(paths).unwrap_or(None); - let recent_events = load_recent_automation_events(paths, 5).unwrap_or_default(); - let recent_proposals = load_recent_automation_proposals(paths, 5).unwrap_or_default(); - let recent_audit_events = load_recent_audit_events(paths, 5).unwrap_or_default(); - let mut output = String::new(); - let _ = writeln!(output, "automation checks"); - let _ = writeln!(output, " config: {}", paths.config_path().display()); - let _ = writeln!( - output, - " background checks: {}", - if config.automation_daemon_enabled() { - "on" - } else { - "off" - } - ); - if let Some(state) = runtime_state.as_ref() { - let _ = writeln!( - output, - " background service: {}", - if state.running { "running" } else { "stopped" } - ); - let _ = writeln!( - output, - " local event intake: {}", - state - .local_webhook_endpoint - .as_deref() - .unwrap_or("disabled") - ); - } else { - let _ = writeln!(output, " background service: not running"); - let _ = writeln!(output, " local event intake: disabled"); - } - for watcher in builtin_watchers() { - let runtime_snapshot = runtime_state.as_ref().and_then(|state| { - state - .active_watchers - .iter() - .find(|item| item.id == watcher.id) - }); - let _ = writeln!( - output, - " {} ({})", - watcher_plain_name(watcher.id), - if config.watcher_enabled(watcher) { - "on" - } else { - "off" - } - ); - let _ = writeln!( - output, - " setting: {}", - watcher_mode_plain_label(config.effective_watcher_mode(watcher)) - ); - let _ = writeln!( - output, - " listens for: {}", - watcher_plain_trigger(watcher.id) - ); - let _ = writeln!(output, " does: {}", watcher_plain_action(watcher.id)); - if let Some(note) = watcher_policy_note(watcher.id) { - let _ = writeln!(output, " policy: {note}"); - } - if let Some(snapshot) = runtime_snapshot { - let _ = writeln!(output, " last check: {}", watcher_last_check(snapshot)); - } - } - if !recent_events.is_empty() { - let _ = writeln!(output, " recent automation activity:"); - for event in recent_events { - let _ = writeln!(output, " {}", automation_event_plain_summary(&event)); - if let Some(service_id) = event.service_id.as_deref() { - let _ = writeln!(output, " server: {service_id}"); - } - } - } - if !recent_proposals.is_empty() { - let _ = writeln!(output, " recent review requests:"); - for proposal in recent_proposals { - let _ = writeln!( - output, - " {} [{}] {}", - proposal.proposal_id, - proposal_status_label(&proposal.status), - proposal_plain_summary(&proposal) - ); - let _ = writeln!(output, " why: {}", proposal_plain_reason(&proposal)); - if let Some(service_id) = proposal.service_id.as_deref() { - let _ = writeln!(output, " server: {service_id}"); - } - if let Some(artifact_ref) = proposal - .arguments - .get("artifact_ref") - .and_then(serde_json::Value::as_str) - { - let _ = writeln!(output, " model file: {artifact_ref}"); - } - if proposal_bool_argument(&proposal, "allow_artifact_download") { - let limit = proposal - .arguments - .get("artifact_max_bytes") - .and_then(serde_json::Value::as_u64) - .map_or_else(|| "not set".to_owned(), format_bytes); - let _ = writeln!(output, " download: approved up to {limit}"); - } else if proposal_kind(&proposal) == ProposalKind::PrefetchArtifact { - let _ = writeln!(output, " download: not approved yet"); - } - if proposal_kind(&proposal) == ProposalKind::DriverPlan { - let _ = writeln!( - output, - " effect: show a driver plan only; no driver install" - ); - } - if proposal.status == "pending" { - let _ = writeln!( - output, - " controls: /automations approve {} | /automations reject {}", - proposal.proposal_id, proposal.proposal_id - ); - } - } - } - if !recent_audit_events.is_empty() { - let _ = writeln!(output, " recent background activity:"); - for event in recent_audit_events { - let _ = writeln!(output, " {}", audit_event_plain_summary(&event)); - if let Some(service_id) = event.service_id.as_deref() { - let _ = writeln!(output, " server: {service_id}"); - } - } - } - Ok(output) -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum ProposalKind { - RestartServer, - StopServer, - CheckUpdates, - PrefetchArtifact, - DriverPlan, - Other, -} - -fn proposal_kind(proposal: &AutomationProposalRecord) -> ProposalKind { - match proposal - .tool - .as_deref() - .or_else(|| fallback_tool_for_automation_action(&proposal.action)) - { - Some("restart_server") => ProposalKind::RestartServer, - Some("stop_server") => ProposalKind::StopServer, - Some("check_updates") => ProposalKind::CheckUpdates, - Some("prefetch_artifact") => ProposalKind::PrefetchArtifact, - Some("driver_plan") => ProposalKind::DriverPlan, - _ => ProposalKind::Other, - } -} - -fn fallback_tool_for_automation_action(action: &str) -> Option<&'static str> { - match action { - "queue_restart_proposal" => Some("restart_server"), - "queue_stop_server_proposal" => Some("stop_server"), - "queue_update_proposal" => Some("check_updates"), - "queue_prefetch_proposal" => Some("prefetch_artifact"), - "prepare_driver_plan" => Some("driver_plan"), - _ => None, - } -} - -fn proposal_plain_summary(proposal: &AutomationProposalRecord) -> &'static str { - match proposal_kind(proposal) { - ProposalKind::RestartServer => "Restart a model server", - ProposalKind::StopServer => "Stop a model server", - ProposalKind::CheckUpdates => "Check for ROCm updates", - ProposalKind::PrefetchArtifact => "Prepare a model file", - ProposalKind::DriverPlan => "Show a driver plan", - ProposalKind::Other => "Review an automation request", - } -} - -fn proposal_plain_reason(proposal: &AutomationProposalRecord) -> &'static str { - match proposal_kind(proposal) { - ProposalKind::RestartServer => "A managed server looks stopped or unhealthy.", - ProposalKind::StopServer => "GPU pressure is high and serving should be reviewed.", - ProposalKind::CheckUpdates => "A scheduled update check is due.", - ProposalKind::PrefetchArtifact => "rocm-cli was asked to prepare this model file.", - ProposalKind::DriverPlan => "A driver update signal was received.", - ProposalKind::Other => "An enabled automation asked for review.", - } -} - -fn proposal_status_label(status: &str) -> &'static str { - match status { - "pending" => "waiting for review", - "approved" => "approved", - "completed" => "done", - "rejected" => "rejected", - "failed" => "failed", - _ => "status unknown", - } -} - -fn proposal_bool_argument(proposal: &AutomationProposalRecord, key: &str) -> bool { - proposal - .arguments - .get(key) - .and_then(serde_json::Value::as_bool) - .unwrap_or(false) -} - -fn automation_event_plain_summary(event: &AutomationEventRecord) -> &'static str { - match event.watcher_id.as_str() { - "therock-update" => "ROCm update check recorded.", - "server-recover" => "Server recovery check recorded.", - "gpu-metrics" => "GPU status check recorded.", - "gpu-thermal-protect" => "GPU pressure check recorded.", - "cache-warm" => "Model file preparation request recorded.", - "driver-upgrade" => "Driver update review recorded.", - _ => "Automation activity recorded.", - } -} - -fn audit_event_plain_summary(event: &AuditEventRecord) -> &'static str { - match event.category.as_str() { - "automation" => "Automation activity was recorded.", - "proposal" => "A review request changed status.", - "provider" => "Provider request completed.", - "service" => "Managed server activity was recorded.", - "install" => "Install activity was recorded.", - "update" => "Update activity was recorded.", - "runtime" => "ROCm runtime activity was recorded.", - "engine" => "Engine activity was recorded.", - _ => "Background activity was recorded.", - } -} - -const fn watcher_mode_plain_label(mode: WatcherMode) -> &'static str { - match mode { - WatcherMode::Observe => "record only", - WatcherMode::Propose => "ask before taking action", - WatcherMode::Contained => "ask before changes; keep actions limited", - } -} - -fn watcher_last_check(snapshot: &rocm_core::WatcherRuntimeSnapshot) -> &'static str { - let Some(event) = snapshot.last_event.as_deref() else { - return "not yet"; - }; - match event { - "remind_update_check" => "ROCm update check recorded", - "queue_update_proposal" => "ROCm update review requested", - "collect_failure_snapshot" => "server health check recorded", - "restart_managed_service" | "queue_restart_proposal" => "server restart review requested", - "record_gpu_metrics" => "GPU status check recorded", - "queue_stop_server_proposal" => "GPU pressure review requested", - "queue_prefetch_proposal" => "model file review requested", - "prepare_driver_plan" => "driver plan review requested", - _ => "activity recorded", - } -} - -fn watcher_plain_name(watcher_id: &str) -> &'static str { - match watcher_id { - "therock-update" => "ROCm update checks", - "server-recover" => "Server recovery", - "gpu-metrics" => "GPU status checks", - "gpu-thermal-protect" => "GPU pressure protection", - "cache-warm" => "Model file preparation", - "driver-upgrade" => "Driver update review", - _ => "Automation check", - } -} - -fn watcher_plain_trigger(watcher_id: &str) -> &'static str { - match watcher_id { - "therock-update" => "a scheduled check", - "server-recover" => "a server that stops or becomes unhealthy", - "gpu-metrics" => "local GPU status updates", - "gpu-thermal-protect" => "high GPU temperature or memory pressure", - "cache-warm" => "a request to prepare a model file", - "driver-upgrade" => "a driver update signal", - _ => "local automation activity", - } -} - -fn watcher_plain_action(watcher_id: &str) -> &'static str { - match watcher_id { - "therock-update" => "checks for ROCm updates without installing them", - "server-recover" => "asks before restarting a managed server", - "gpu-metrics" => "records local GPU status only", - "gpu-thermal-protect" => "asks before stopping a managed server", - "cache-warm" => "asks before preparing or downloading a model file", - "driver-upgrade" => "shows a driver plan only", - _ => "records the event", - } -} - -fn watcher_policy_note(watcher_id: &str) -> Option<&'static str> { - match watcher_id { - "gpu-metrics" => Some( - "read-only telemetry; propose/contained modes record events only and do not create review requests or mutate services", - ), - "gpu-thermal-protect" => Some( - "GPU pressure protection is review-gated; propose/contained modes queue a stop review only and never stop servers automatically", - ), - "cache-warm" => Some( - "artifact prefetch stays review-gated; contained mode creates a review request instead of downloading without reviewed source policy", - ), - "driver-upgrade" => Some( - "local driver update signals stay review-gated; review requests run a read-only driver plan and never install drivers automatically", - ), - _ => None, - } -} - -pub(crate) fn render_daemon_text(paths: &AppPaths, config: &RocmCliConfig) -> String { - let runtime_state = AutomationRuntimeState::load(paths).unwrap_or(None); - let mut output = String::new(); - let _ = writeln!(output, "Background helper"); - let _ = writeln!(output); - let _ = writeln!( - output, - "Status: {}", - match runtime_state { - Some(ref state) if state.running => "running", - _ => "not running", - } - ); - let _ = writeln!(output, "Startup: on demand"); - let _ = writeln!( - output, - "Why it runs: automation checks and local model servers use it when needed." - ); - let _ = writeln!( - output, - "Automation checks: {}", - if config.automation_daemon_enabled() { - "on" - } else { - "off" - } - ); - let _ = writeln!(output, "Saved state: kept on this computer"); - let _ = writeln!(output); - let _ = writeln!(output, "Choose Automations to review background checks."); - let _ = writeln!(output, "Choose Local servers to see local model servers."); - output -} - pub(crate) const fn engine_inventory() -> &'static [(&'static str, &'static str)] { &[ ( @@ -10757,23 +10156,6 @@ pub(crate) mod tests { let _ = fs::remove_dir_all(&root); } - #[test] - fn render_update_text_reports_all_update_surfaces() -> Result<()> { - let (root, paths) = test_paths("update-surfaces"); - - let rendered = render_update_text(&paths)?; - fs::remove_dir_all(root).ok(); - - assert!(rendered.contains("update_surfaces:")); - assert!(rendered.contains("cli: installed=")); - assert!(rendered.contains("status=not_configured")); - assert!(rendered.contains("engines: status=package_managed")); - assert!(rendered.contains("model_recipes: status=")); - assert!(rendered.contains("runtimes: status=none_configured")); - assert!(rendered.contains("`rocm update --apply` applies runtime updates only")); - Ok(()) - } - #[test] fn render_logs_text_preserves_directory_summary() { let (_root, paths) = test_paths("logs-summary"); @@ -12535,25 +11917,6 @@ pub(crate) mod tests { ); } - // This only pins the free predicate's truth table. The actual dispatch - // wiring — that `rocm update --dry-run` really does reach the preview - // path without requiring --apply — is covered by e2e scenario - // `update-dry-run-reaches-preview-path-without-apply` - // (tests/e2e-cucumber/features/update.feature). - #[test] - fn update_should_preview_or_apply_includes_dry_run() { - assert!( - !update_should_preview_or_apply(false, false), - "plain `rocm update` should stay on the read-only status report" - ); - assert!( - update_should_preview_or_apply(false, true), - "the predicate must say dry-run alone should preview" - ); - assert!(update_should_preview_or_apply(true, false)); - assert!(update_should_preview_or_apply(true, true)); - } - #[test] fn remote_is_structured_not_freeform() { // `rocm remote …` reads like a plain-English request, so without an @@ -14080,7 +13443,7 @@ pub(crate) mod tests { result.err() ); - let actions: Vec = load_recent_audit_events(&paths, 10)? + let actions: Vec = rocm_core::load_recent_audit_events(&paths, 10)? .into_iter() .map(|event| event.action) .collect(); @@ -14242,195 +13605,6 @@ pub(crate) mod tests { assert!(missing_packaged_engine_reason("vllm").is_none()); } - #[test] - fn render_automations_text_marks_gpu_metrics_read_only() -> Result<()> { - let (root, paths) = test_paths("automation-status-policy"); - let mut config = RocmCliConfig::default(); - let watcher = config.watcher_config_mut("gpu-metrics"); - watcher.enabled = true; - watcher.mode = Some(WatcherMode::Contained); - config.automations.daemon_enabled = true; - - let rendered = render_automations_text(&paths, &config)?; - let _ = fs::remove_dir_all(root); - - assert!(rendered.contains("GPU status checks (on)")); - assert!(rendered.contains("setting: ask before changes; keep actions limited")); - assert!(rendered.contains("listens for: local GPU status updates")); - assert!(rendered.contains("does: records local GPU status only")); - assert!(rendered.contains("policy: read-only telemetry")); - assert!(rendered.contains("do not create review requests or mutate services")); - assert!(!rendered.contains("command id:")); - assert!(!rendered.contains("mode:")); - assert!(!rendered.contains("gpu-metrics")); - Ok(()) - } - - #[test] - fn render_automations_text_marks_gpu_thermal_protect_review_gated() -> Result<()> { - let (root, paths) = test_paths("automation-gpu-thermal-policy"); - let mut config = RocmCliConfig::default(); - let watcher = config.watcher_config_mut("gpu-thermal-protect"); - watcher.enabled = true; - watcher.mode = Some(WatcherMode::Contained); - config.automations.daemon_enabled = true; - - let rendered = render_automations_text(&paths, &config)?; - let _ = fs::remove_dir_all(root); - - assert!(rendered.contains("GPU pressure protection (on)")); - assert!(rendered.contains("setting: ask before changes; keep actions limited")); - assert!(rendered.contains("listens for: high GPU temperature or memory pressure")); - assert!(rendered.contains("does: asks before stopping a managed server")); - assert!(rendered.contains("policy: GPU pressure protection is review-gated")); - assert!(rendered.contains("never stop servers automatically")); - assert!(!rendered.contains("command id:")); - assert!(!rendered.contains("mode:")); - assert!(!rendered.contains("gpu-thermal-protect")); - Ok(()) - } - - #[test] - fn render_automations_text_surfaces_local_webhook_endpoint() -> Result<()> { - let (root, paths) = test_paths("automation-local-webhook"); - AutomationRuntimeState { - running: true, - automations_enabled: true, - daemon_pid: 123, - started_at_unix_ms: 1, - last_tick_unix_ms: 2, - local_webhook_endpoint: Some("http://127.0.0.1:19191/automation-events".to_owned()), - active_watchers: vec![rocm_core::WatcherRuntimeSnapshot { - id: "gpu-metrics".to_owned(), - enabled: true, - mode: WatcherMode::Observe, - summary: "record metrics".to_owned(), - last_event: Some("record_gpu_metrics".to_owned()), - last_event_unix_ms: Some(2), - }], - } - .write(&paths)?; - - let rendered = render_automations_text(&paths, &RocmCliConfig::default())?; - let _ = fs::remove_dir_all(root); - - assert!(rendered.contains("background service: running")); - assert!(rendered.contains("local event intake: http://127.0.0.1:19191/automation-events")); - assert!(rendered.contains("last check: GPU status check recorded")); - assert!(!rendered.contains("pid=123")); - assert!(!rendered.contains("last_tick_unix_ms")); - assert!(!rendered.contains("local_webhook_endpoint")); - assert!(!rendered.contains("record_gpu_metrics")); - assert!(!rendered.contains("gpu-metrics")); - Ok(()) - } - - #[test] - fn render_automations_text_uses_plain_proposal_history() -> Result<()> { - let (root, paths) = test_paths("automation-plain-proposal-history"); - rocm_core::append_automation_proposal( - &paths, - &AutomationProposalRecord { - at_unix_ms: 42, - proposal_id: "proposal-plain".to_owned(), - watcher_id: "server-recover".to_owned(), - action: "queue_restart_proposal".to_owned(), - title: "queue restart proposal".to_owned(), - message: "backend-authored recovery message".to_owned(), - status: "pending".to_owned(), - service_id: Some("svc-plain".to_owned()), - tool: Some("restart_server".to_owned()), - arguments: serde_json::json!({ "service_id": "svc-plain" }), - reviewed_at_unix_ms: None, - }, - )?; - rocm_core::append_automation_proposal( - &paths, - &AutomationProposalRecord { - at_unix_ms: 43, - proposal_id: "proposal-file".to_owned(), - watcher_id: "cache-warm".to_owned(), - action: "queue_prefetch_proposal".to_owned(), - title: "queue prefetch proposal".to_owned(), - message: "cache warm requested internal artifact prefetch".to_owned(), - status: "pending".to_owned(), - service_id: None, - tool: Some("prefetch_artifact".to_owned()), - arguments: serde_json::json!({ - "artifact_ref": "tiny/model#gguf", - "allow_artifact_download": true, - // One byte under 1 MiB: `{:.1}` rounds it up to a full - // 1024 KiB, so the approved cap must read as "1.0 MiB", - // not "1024.0 KiB". It must also read as `MiB`, not `MB` - // — this is base-1024 math, not base-1000 (issue #190). - "artifact_max_bytes": 1_048_575 - }), - reviewed_at_unix_ms: None, - }, - )?; - rocm_core::append_automation_event( - &paths, - &rocm_core::AutomationEventRecord { - at_unix_ms: 44, - watcher_id: "gpu-metrics".to_owned(), - level: "info".to_owned(), - action: "record_gpu_metrics".to_owned(), - message: "raw amd-smi telemetry event".to_owned(), - service_id: None, - }, - )?; - rocm_core::append_audit_event( - &paths, - &AuditEventRecord { - at_unix_ms: 45, - source: "test".to_owned(), - category: "proposal".to_owned(), - actor: "watcher:server-recover".to_owned(), - level: "info".to_owned(), - action: "proposal_approved".to_owned(), - message: "approved proposal-plain with backend detail".to_owned(), - watcher_id: Some("server-recover".to_owned()), - service_id: Some("svc-plain".to_owned()), - }, - )?; - - let rendered = render_automations_text(&paths, &RocmCliConfig::default())?; - let _ = fs::remove_dir_all(root); - - assert!(rendered.contains("recent automation activity:")); - assert!(rendered.contains("GPU status check recorded.")); - assert!(rendered.contains("recent review requests:")); - assert!(rendered.contains("proposal-plain [waiting for review] Restart a model server")); - assert!(rendered.contains("why: A managed server looks stopped or unhealthy.")); - assert!(rendered.contains("server: svc-plain")); - assert!(rendered.contains("proposal-file [waiting for review] Prepare a model file")); - assert!(rendered.contains("model file: tiny/model#gguf")); - assert!(rendered.contains("download: approved up to 1.0 MiB")); - assert!(rendered.contains( - "controls: /automations approve proposal-plain | /automations reject proposal-plain" - )); - assert!(rendered.contains("recent background activity:")); - assert!(rendered.contains("A review request changed status.")); - assert!(!rendered.contains("queue_restart_proposal")); - assert!(!rendered.contains("restart_server")); - assert!(!rendered.contains("queue restart proposal")); - assert!(!rendered.contains("backend-authored recovery message")); - assert!(!rendered.contains("record_gpu_metrics")); - assert!(!rendered.contains("raw amd-smi telemetry event")); - assert!(!rendered.contains("proposal_approved")); - assert!(!rendered.contains("approved proposal-plain with backend detail")); - assert!(!rendered.contains("created_unix_ms")); - assert!(!rendered.contains("last_tick_unix_ms")); - assert!(!rendered.contains("local_webhook_endpoint")); - assert!(!rendered.contains("command id:")); - assert!(!rendered.contains("mode:")); - assert!(!rendered.contains("1048576 bytes")); - assert!(!rendered.contains("recent events:")); - assert!(!rendered.contains("recent audit:")); - assert!(!rendered.contains("tool:")); - Ok(()) - } - #[test] fn linux_only_engine_runtime_status_is_explicit_on_native_windows() { let detect = DetectResponse { @@ -14489,7 +13663,7 @@ pub(crate) mod tests { None, ); - let events = load_recent_audit_events(&paths, 1)?; + let events = rocm_core::load_recent_audit_events(&paths, 1)?; assert_eq!(events.len(), 1); assert_eq!(events[0].source, "rocm"); assert_eq!(events[0].actor, "cli"); @@ -15158,131 +14332,6 @@ pub(crate) mod tests { let _ = fs::remove_dir_all(root); } - #[test] - fn runtime_update_source_uses_active_runtime_and_requires_selector_when_ambiguous() -> Result<()> - { - let (root, paths) = test_paths("runtime-update-source"); - let first = write_test_pip_runtime( - &paths, - "release-pip-gfx110x-all", - "therock-release:gfx110X-all", - "7.13.0a20260416", - 1, - )?; - let second = write_test_pip_runtime( - &paths, - "release-pip-gfx120x-all", - "therock-release:gfx120X-all", - "7.13.0a20260416", - 2, - )?; - let manifests = therock::load_runtime_manifests(&paths)?; - let active_config = RocmCliConfig { - default_runtime_id: Some(second.runtime_id.clone()), - active_runtime_key: Some(second.runtime_key.clone()), - ..RocmCliConfig::default() - }; - - let selected = select_runtime_update_source(&manifests, &active_config, None)?; - assert_eq!(selected.runtime_key, second.runtime_key); - - let explicit = select_runtime_update_source( - &manifests, - &RocmCliConfig::default(), - Some(&first.runtime_key), - )?; - assert_eq!(explicit.runtime_key, first.runtime_key); - - let error = select_runtime_update_source(&manifests, &RocmCliConfig::default(), None) - .unwrap_err() - .to_string(); - assert!(error.contains("multiple runtimes")); - assert!(error.contains("--runtime ")); - let _ = fs::remove_dir_all(root); - Ok(()) - } - - #[test] - fn installed_update_runtime_is_selected_by_exact_target_key() { - // Everything a version match would have keyed on is identical here: - // same channel, format, family and version. Only the composition-keyed - // runtime key tells the freshly installed repair apart from the stale - // runtime it was installed to replace. - let stale = test_runtime_manifest_for_update( - "release-wheel-multi-arch-7-14-0", - "therock-release:gfx120X-all", - "gfx120X-all", - "7.14.0", - ); - let wrong_family = test_runtime_manifest_for_update( - "release-wheel-multi-arch-7-14-0-ffffffffffffffff", - "therock-release:gfx110X-all", - "gfx110X-all", - "7.14.0", - ); - let repaired = test_runtime_manifest_for_update( - "release-wheel-multi-arch-7-14-0-0123456789abcdef", - "therock-release:gfx120X-all", - "gfx120X-all", - "7.14.0", - ); - let manifests = vec![stale, wrong_family, repaired.clone()]; - - let selected = select_installed_update_runtime(&manifests, &repaired.runtime_key) - .expect("the side-by-side repair must be selected by its exact key"); - assert_eq!(selected.runtime_key, repaired.runtime_key); - - assert!( - select_installed_update_runtime(&manifests, "release-wheel-multi-arch-7-15-0") - .is_none(), - "an install that wrote no manifest for the planned key must not resolve to a sibling" - ); - } - - #[test] - fn update_activate_summary_hints_rollback_only_when_a_previous_runtime_exists() { - let with_previous = RuntimeActivationResult { - runtime_id: "therock-release:gfx942".to_owned(), - runtime_key: "release-wheel-gfx942".to_owned(), - previous_runtime_key: Some("release-wheel-gfx942-old".to_owned()), - reactivated: false, - // No live services: this test is about the rollback hint, and an - // empty reconciliation is what an update on a machine with nothing - // serving produces. - services: runtime_services::RuntimeServiceReconciliation::default(), - }; - let mut rendered = String::new(); - append_update_activate_summary(&mut rendered, &with_previous); - assert!( - rendered.contains("services_on_previous_runtime: 0"), - "the update summary reports service state the same way `runtimes \ - activate` does, including when there is none:\n{rendered}" - ); - assert!( - rendered.contains("next step: if this causes problems, run `rocm runtimes rollback`"), - "a previous runtime is recorded, so rollback is a valid recovery path:\n{rendered}" - ); - assert!( - rendered.contains("undoes only this one activation"), - "the hint should state the single-step limit up front, not just in --help:\n{rendered}" - ); - - let without_previous = RuntimeActivationResult { - runtime_id: "therock-release:gfx942".to_owned(), - runtime_key: "release-wheel-gfx942".to_owned(), - previous_runtime_key: None, - reactivated: false, - services: runtime_services::RuntimeServiceReconciliation::default(), - }; - let mut rendered = String::new(); - append_update_activate_summary(&mut rendered, &without_previous); - assert!( - !rendered.contains("rocm runtimes rollback"), - "no previous runtime is recorded, so `rocm runtimes rollback` would hard-error; \ - must not hint at a command that immediately fails:\n{rendered}" - ); - } - pub(crate) fn test_runtime_manifest_for_update( runtime_key: &str, runtime_id: &str, diff --git a/apps/rocm/src/runtimes_cmd.rs b/apps/rocm/src/runtimes_cmd.rs index 120ab4dd0..27545eb13 100644 --- a/apps/rocm/src/runtimes_cmd.rs +++ b/apps/rocm/src/runtimes_cmd.rs @@ -13,8 +13,8 @@ //! way `EnginesCommand` stayed behind when `engines()` moved to //! `engines_cmd.rs` in Phase 6a. `RuntimeActivationResult` and //! `ActiveRuntimeMarker` stay `pub(crate)`: the former is read by -//! `append_update_activate_summary` in `main.rs`'s `apply_runtime_update` -//! flow, the latter by `storage.rs`. `select_runtime_manifest`, +//! `append_update_activate_summary` in `update_cmd.rs`'s +//! `apply_runtime_update` flow, the latter by `storage.rs`. `select_runtime_manifest`, //! `validate_runtime_manifest_for_activation`, `normalize_path_for_compare`, //! `recover_setup_runtime_registration`, `active_runtime_marker_path`, //! `should_remove_runtime_install_root`, and `runtime_usability_status` are diff --git a/apps/rocm/src/therock_version_proptests.rs b/apps/rocm/src/therock_version_proptests.rs index 52b13fc11..9843f264a 100644 --- a/apps/rocm/src/therock_version_proptests.rs +++ b/apps/rocm/src/therock_version_proptests.rs @@ -610,8 +610,8 @@ proptest! { } config.default_runtime_id = Some(anchor.runtime_id.clone()); - let left = crate::select_runtime_update_source(&manifests, &config, None); - let right = crate::select_runtime_update_source(&reordered, &config, None); + let left = crate::update_cmd::select_runtime_update_source(&manifests, &config, None); + let right = crate::update_cmd::select_runtime_update_source(&reordered, &config, None); match (left, right) { (Ok(left), Ok(right)) => prop_assert_eq!( &left.runtime_key, diff --git a/apps/rocm/src/update_cmd.rs b/apps/rocm/src/update_cmd.rs new file mode 100644 index 000000000..2b596bbf8 --- /dev/null +++ b/apps/rocm/src/update_cmd.rs @@ -0,0 +1,481 @@ +// Copyright © Advanced Micro Devices, Inc., or its affiliates. +// +// SPDX-License-Identifier: MIT + +//! `rocm update`'s own rendering and activation logic: the read-only status +//! report (`render_update_text`, `append_update_surfaces`), the predicate +//! deciding whether `update` should route into the mutating path instead of +//! that report (`update_should_preview_or_apply`), applying a runtime update +//! (`apply_runtime_update`, `append_update_activate_summary`), and the +//! update-source/installed-runtime pickers behind it +//! (`select_runtime_update_source`, `select_installed_update_runtime`). +//! +//! Mechanically relocated from `main.rs` with no behavior change — Phase 6f +//! (ROCMAI-210) of the `main.rs` modularization (see `docs/architecture.md`). +//! `dispatch()` calls directly into this module at three points in the +//! `update` command arm — `update_should_preview_or_apply(apply, dry_run)`, +//! `apply_runtime_update(...)`, and `render_update_text(&paths)` — all +//! re-imported via `use crate::update_cmd::{...};`; `dispatch()`'s own body +//! is otherwise unaffected. `chat.rs`'s deterministic `rocm update` tool +//! summary also calls `render_update_text` the same way, outside `dispatch()`. +//! +//! One item physically interleaved in this span in `main.rs` stayed there +//! instead of moving: `const ROLLBACK_RECOVERY_HINT` sits between +//! `apply_runtime_update` and `append_update_activate_summary`, but it +//! already had callers outside this cluster — `runtimes_cmd.rs`'s +//! `RuntimesCommand::Activate` handler and `runtime_install_finalize.rs`'s +//! `render_sdk_install_success` both reach it unqualified via +//! `use crate::{...}` — so it stays a shared crate-root constant, reached +//! from here the same way via `use crate::ROLLBACK_RECOVERY_HINT;`. +//! +//! `render_update_text`, `apply_runtime_update`, and +//! `update_should_preview_or_apply` are `pub(crate)` for `dispatch()`'s own +//! calls above. `select_runtime_update_source` is also `pub(crate)`: it is +//! reached from `therock_version_proptests.rs`'s property tests via +//! `crate::update_cmd::select_runtime_update_source`. `append_update_surfaces`, +//! `append_update_activate_summary`, and `select_installed_update_runtime` +//! have no callers outside this module and stay private. +//! +//! The cluster's regression tests live in this module's own `mod tests`, +//! reaching shared fixtures (`test_paths`, `test_runtime_manifest_for_update`) +//! through `crate::tests::` the same way other mechanically relocated +//! modules do. + +use std::fmt::Write as _; + +use anyhow::{Context, Result, bail}; +use rocm_core::{AppPaths, ModelRecipeRegistrySource, RocmCliConfig, load_model_recipe_registry}; + +use crate::runtimes_cmd::{ + RuntimeActivationResult, activate_runtime, current_runtime_manifest, select_runtime_manifest, +}; +use crate::{ + ROLLBACK_RECOVERY_HINT, engine_inventory, runtime_services, sanitize_log_value, therock, +}; + +pub(crate) fn render_update_text(paths: &AppPaths) -> Result { + let mut output = therock::render_update_report(paths)?; + append_update_surfaces(&mut output); + Ok(output) +} + +fn append_update_surfaces(output: &mut String) { + let _ = writeln!(output, " update_surfaces:"); + let _ = writeln!( + output, + " cli: installed={} status=not_configured reason=repository-owned CLI update feed is not published yet", + env!("CARGO_PKG_VERSION") + ); + let engine_ids = engine_inventory() + .iter() + .map(|(engine, _)| *engine) + .collect::>() + .join(","); + let _ = writeln!( + output, + " engines: status=package_managed packaged=[{engine_ids}] reason=first-party engine binaries update with the rocm-cli package; data-dir plugins are user-managed" + ); + match load_model_recipe_registry() { + Ok(registry) => match registry.source { + ModelRecipeRegistrySource::BuiltIn => { + let _ = writeln!( + output, + " model_recipes: status=built_in count={} reason=external signed recipe index is not configured", + registry.recipes.len() + ); + } + ModelRecipeRegistrySource::SignedIndex { + index_path, + signature_path, + public_key_path, + } => { + let _ = writeln!( + output, + " model_recipes: status=signed_index count={} index={} signature={} public_key={} reason=loaded signed recipe index; recipe update feed is not live in this build", + registry.recipes.len(), + index_path.display(), + signature_path.display(), + public_key_path.display() + ); + } + }, + Err(error) => { + let _ = writeln!( + output, + " model_recipes: status=error reason={}", + sanitize_log_value(&error.to_string()) + ); + } + } + let runtime_status = if output.contains(" managed runtimes: none") { + "none_configured" + } else { + "checked_above" + }; + let _ = writeln!( + output, + " runtimes: status={runtime_status} reason=TheRock runtime update checks above are the only live update checks in this build" + ); + let _ = writeln!( + output, + " note: `rocm update --apply` applies runtime updates only; CLI, engine, and recipe update feeds require published metadata before they can mutate state" + ); +} + +/// Whether `rocm update` should route into the runtime update path +/// (`apply_runtime_update`) instead of the read-only status report. +/// +/// `--dry-run` alone must take this path too, since `apply_runtime_update` +/// only mutates anything when `dry_run` is false — a plain status report +/// would silently ignore `--dry-run` and never show what `--apply` would do. +pub(crate) const fn update_should_preview_or_apply(apply: bool, dry_run: bool) -> bool { + apply || dry_run +} + +pub(crate) fn apply_runtime_update( + paths: &AppPaths, + config: &mut RocmCliConfig, + runtime_selector: Option<&str>, + activate: bool, + dry_run: bool, +) -> Result { + let manifests = therock::load_runtime_manifests(paths)?; + let source = select_runtime_update_source(&manifests, config, runtime_selector)?; + let plan = therock::runtime_update_plan(paths, source, &manifests, None)?; + let mut output = String::new(); + let _ = writeln!(output, "runtime update"); + let _ = writeln!(output, " source_runtime_key: {}", source.runtime_key); + let _ = writeln!(output, " source_runtime_id: {}", source.runtime_id); + let _ = writeln!(output, " channel: {}", source.channel); + let _ = writeln!(output, " format: {}", source.format); + let _ = writeln!(output, " family: {}", source.family); + let _ = writeln!( + output, + " installed_version: {}", + therock::runtime_version_display(&source.version) + ); + let _ = writeln!( + output, + " latest_version: {}", + therock::runtime_version_display(&plan.latest_version) + ); + let _ = writeln!(output, " status: {}", plan.status); + let _ = writeln!(output, " target_runtime_key: {}", plan.target_runtime_key); + let _ = writeln!(output, " activate_after_install: {activate}"); + if !plan.update_available { + let _ = writeln!(output, " result: no newer runtime found"); + return Ok(output); + } + + if dry_run { + let _ = writeln!(output, " mode: dry-run"); + let install_plan = therock::install_sdk_for_update( + paths, + &source.channel, + &source.format, + &source.family, + plan.device_target.as_deref(), + plan.source_layout_generation.as_deref(), + source.includes_devel(), + true, + activate, + )?; + let _ = writeln!(output, " install_plan:"); + for line in install_plan.output.lines() { + let _ = writeln!(output, " {line}"); + } + return Ok(output); + } + + // `activate` rather than a bare `true`: the update path is preapproved either + // way (its approval comes from the runtime the user selected, not from a + // flag, and `rocm update` has no terminal contract), but the approval line it + // prints must not promise an activation that only `--activate` performs + // below. + let install_output = therock::install_sdk_for_update( + paths, + &source.channel, + &source.format, + &source.family, + plan.device_target.as_deref(), + plan.source_layout_generation.as_deref(), + // Reinstall what the user originally chose rather than silently + // adding or dropping the compiler toolchain on update. + source.includes_devel(), + false, + activate, + )?; + let manifests_after = therock::load_runtime_manifests(paths)?; + // By exact key, never by version: a same-version repair installs a sibling + // that shares the source's channel, format, family AND version, so a + // version match would just as happily return the stale runtime this update + // was meant to replace, and then activate it. + let installed = select_installed_update_runtime(&manifests_after, &plan.target_runtime_key) + .with_context(|| { + format!( + "runtime install completed but no manifest was written for the planned runtime key `{}`", + plan.target_runtime_key + ) + })?; + let _ = writeln!(output, " installed_runtime_key: {}", installed.runtime_key); + let _ = writeln!( + output, + " installed_runtime_root: {}", + installed.install_root.display() + ); + if activate { + // No `config.save` after this: `activate_runtime` already persisted the + // same struct and nothing mutates it in between, so a second write can + // only fail after an activation that fully succeeded — sending the user + // to re-run a command that already worked. + let activation = activate_runtime(paths, config, &installed.runtime_key)?; + append_update_activate_summary(&mut output, &activation); + } else { + let _ = writeln!( + output, + " next step: rocm runtimes activate {}", + installed.runtime_key + ); + } + let _ = writeln!(output, " install_output:"); + for line in install_output.output.lines() { + let _ = writeln!(output, " {line}"); + } + Ok(output) +} + +/// Appends the `--activate` branch of the `update --apply` summary. The +/// rollback hint is only shown when a previous runtime was actually recorded — +/// `rocm runtimes rollback` hard-errors otherwise (no prior runtime to return +/// to), so hinting it unconditionally would point users at a command that +/// immediately fails on their very first activation. +fn append_update_activate_summary(output: &mut String, activation: &RuntimeActivationResult) { + let _ = writeln!( + output, + " activated_runtime_key: {}", + activation.runtime_key + ); + let _ = writeln!( + output, + " previous_runtime_key: {}", + activation + .previous_runtime_key + .as_deref() + .unwrap_or("") + ); + let _ = write!( + output, + "{}", + runtime_services::render_runtime_service_reconciliation( + &activation.services, + &activation.runtime_key + ) + ); + if activation.previous_runtime_key.is_some() { + let _ = writeln!(output, "{ROLLBACK_RECOVERY_HINT}"); + } +} + +pub(crate) fn select_runtime_update_source<'a>( + manifests: &'a [therock::InstalledRuntimeManifest], + config: &RocmCliConfig, + runtime_selector: Option<&str>, +) -> Result<&'a therock::InstalledRuntimeManifest> { + if let Some(selector) = runtime_selector { + return select_runtime_manifest(manifests, selector); + } + if let Some(active) = current_runtime_manifest(config, manifests) { + return Ok(active); + } + match manifests { + [] => bail!( + "no managed runtimes are registered; run `rocm install sdk --channel release --format wheel` first" + ), + [only] => Ok(only), + _ => bail!( + "multiple runtimes are registered and no active runtime is configured; pass `--runtime `" + ), + } +} + +fn select_installed_update_runtime<'a>( + manifests: &'a [therock::InstalledRuntimeManifest], + target_runtime_key: &str, +) -> Option<&'a therock::InstalledRuntimeManifest> { + manifests + .iter() + .find(|manifest| manifest.runtime_key == target_runtime_key) +} + +#[cfg(test)] +mod tests { + use std::fs; + + use anyhow::Result; + use rocm_core::RocmCliConfig; + + use super::*; + use crate::test_support::write_test_pip_runtime; + use crate::tests::{test_paths, test_runtime_manifest_for_update}; + + #[test] + fn render_update_text_reports_all_update_surfaces() -> Result<()> { + let (root, paths) = test_paths("update-surfaces"); + + let rendered = render_update_text(&paths)?; + fs::remove_dir_all(root).ok(); + + assert!(rendered.contains("update_surfaces:")); + assert!(rendered.contains("cli: installed=")); + assert!(rendered.contains("status=not_configured")); + assert!(rendered.contains("engines: status=package_managed")); + assert!(rendered.contains("model_recipes: status=")); + assert!(rendered.contains("runtimes: status=none_configured")); + assert!(rendered.contains("`rocm update --apply` applies runtime updates only")); + Ok(()) + } + + // This only pins the free predicate's truth table. The actual dispatch + // wiring — that `rocm update --dry-run` really does reach the preview + // path without requiring --apply — is covered by e2e scenario + // `update-dry-run-reaches-preview-path-without-apply` + // (tests/e2e-cucumber/features/update.feature). + #[test] + fn update_should_preview_or_apply_includes_dry_run() { + assert!( + !update_should_preview_or_apply(false, false), + "plain `rocm update` should stay on the read-only status report" + ); + assert!( + update_should_preview_or_apply(false, true), + "the predicate must say dry-run alone should preview" + ); + assert!(update_should_preview_or_apply(true, false)); + assert!(update_should_preview_or_apply(true, true)); + } + + #[test] + fn runtime_update_source_uses_active_runtime_and_requires_selector_when_ambiguous() -> Result<()> + { + let (root, paths) = test_paths("runtime-update-source"); + let first = write_test_pip_runtime( + &paths, + "release-pip-gfx110x-all", + "therock-release:gfx110X-all", + "7.13.0a20260416", + 1, + )?; + let second = write_test_pip_runtime( + &paths, + "release-pip-gfx120x-all", + "therock-release:gfx120X-all", + "7.13.0a20260416", + 2, + )?; + let manifests = therock::load_runtime_manifests(&paths)?; + let active_config = RocmCliConfig { + default_runtime_id: Some(second.runtime_id.clone()), + active_runtime_key: Some(second.runtime_key.clone()), + ..RocmCliConfig::default() + }; + + let selected = select_runtime_update_source(&manifests, &active_config, None)?; + assert_eq!(selected.runtime_key, second.runtime_key); + + let explicit = select_runtime_update_source( + &manifests, + &RocmCliConfig::default(), + Some(&first.runtime_key), + )?; + assert_eq!(explicit.runtime_key, first.runtime_key); + + let error = select_runtime_update_source(&manifests, &RocmCliConfig::default(), None) + .unwrap_err() + .to_string(); + assert!(error.contains("multiple runtimes")); + assert!(error.contains("--runtime ")); + let _ = fs::remove_dir_all(root); + Ok(()) + } + + #[test] + fn installed_update_runtime_is_selected_by_exact_target_key() { + // Everything a version match would have keyed on is identical here: + // same channel, format, family and version. Only the composition-keyed + // runtime key tells the freshly installed repair apart from the stale + // runtime it was installed to replace. + let stale = test_runtime_manifest_for_update( + "release-wheel-multi-arch-7-14-0", + "therock-release:gfx120X-all", + "gfx120X-all", + "7.14.0", + ); + let wrong_family = test_runtime_manifest_for_update( + "release-wheel-multi-arch-7-14-0-ffffffffffffffff", + "therock-release:gfx110X-all", + "gfx110X-all", + "7.14.0", + ); + let repaired = test_runtime_manifest_for_update( + "release-wheel-multi-arch-7-14-0-0123456789abcdef", + "therock-release:gfx120X-all", + "gfx120X-all", + "7.14.0", + ); + let manifests = vec![stale, wrong_family, repaired.clone()]; + + let selected = select_installed_update_runtime(&manifests, &repaired.runtime_key) + .expect("the side-by-side repair must be selected by its exact key"); + assert_eq!(selected.runtime_key, repaired.runtime_key); + + assert!( + select_installed_update_runtime(&manifests, "release-wheel-multi-arch-7-15-0") + .is_none(), + "an install that wrote no manifest for the planned key must not resolve to a sibling" + ); + } + + #[test] + fn update_activate_summary_hints_rollback_only_when_a_previous_runtime_exists() { + let with_previous = RuntimeActivationResult { + runtime_id: "therock-release:gfx942".to_owned(), + runtime_key: "release-wheel-gfx942".to_owned(), + previous_runtime_key: Some("release-wheel-gfx942-old".to_owned()), + reactivated: false, + // No live services: this test is about the rollback hint, and an + // empty reconciliation is what an update on a machine with nothing + // serving produces. + services: runtime_services::RuntimeServiceReconciliation::default(), + }; + let mut rendered = String::new(); + append_update_activate_summary(&mut rendered, &with_previous); + assert!( + rendered.contains("services_on_previous_runtime: 0"), + "the update summary reports service state the same way `runtimes \ + activate` does, including when there is none:\n{rendered}" + ); + assert!( + rendered.contains("next step: if this causes problems, run `rocm runtimes rollback`"), + "a previous runtime is recorded, so rollback is a valid recovery path:\n{rendered}" + ); + assert!( + rendered.contains("undoes only this one activation"), + "the hint should state the single-step limit up front, not just in --help:\n{rendered}" + ); + + let without_previous = RuntimeActivationResult { + runtime_id: "therock-release:gfx942".to_owned(), + runtime_key: "release-wheel-gfx942".to_owned(), + previous_runtime_key: None, + reactivated: false, + services: runtime_services::RuntimeServiceReconciliation::default(), + }; + let mut rendered = String::new(); + append_update_activate_summary(&mut rendered, &without_previous); + assert!( + !rendered.contains("rocm runtimes rollback"), + "no previous runtime is recorded, so `rocm runtimes rollback` would hard-error; \ + must not hint at a command that immediately fails:\n{rendered}" + ); + } +} diff --git a/docs/architecture.md b/docs/architecture.md index e16d11764..61a480895 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -15,7 +15,7 @@ This is the living module map for rocm-cli. It's a contributor-facing reference New subcommands and subsystems default to their own file from day one — they should not grow inside `main.rs`/`lib.rs` waiting for a future extraction pass. Two extraction patterns already exist in the codebase; use whichever fits: - **Full domain extraction** — a subsystem's domain implementation moves into its own file that owns its own types (structs/enums), not just relocated functions; that ownership is usually what distinguishes this pattern from mechanical relocation below, though the Phase 6a variant described next shows ownership alone doesn't fully tell them apart — every `apps/rocm` module is a private `mod x;` regardless of pattern, so module privacy alone doesn't tell the two apart either. Full-domain-extraction modules are typically reached via qualified paths (e.g. `comfyui::render_status(...)`); mechanically relocated modules with no owned types are reached the same way (e.g. [`endpoint_keys.rs`](../apps/rocm/src/endpoint_keys.rs), [`logging.rs`](../apps/rocm/src/logging.rs)). The exception — reached unqualified through `use crate::x::{...}` re-imports, matching their dispatch call sites' pre-extraction form — is limited to the clusters that moved as a dispatch-adjacent group: [`automations.rs`](../apps/rocm/src/automations.rs), [`uninstall.rs`](../apps/rocm/src/uninstall.rs), (as of Phase 6a, ROCMAI-82) [`driver_install.rs`](../apps/rocm/src/driver_install.rs), [`engines_cmd.rs`](../apps/rocm/src/engines_cmd.rs), [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs), (as of Phase 6e's follow-up, ROCMAI-726) [`freeform_execution.rs`](../apps/rocm/src/freeform_execution.rs), and (as of Phase 6f, ROCMAI-213) [`engine_ipc.rs`](../apps/rocm/src/engine_ipc.rs). [`service_launch.rs`](../apps/rocm/src/service_launch.rs) (as of Phase 6b, ROCMAI-91) uses the same unqualified re-import form for its cross-module callers, but it is not a dispatch-adjacent cluster in the same sense — one of its callers, `automations.rs`, is not a dispatch call site, and its pre-extraction form was a qualified `crate::` call rather than an unqualified import, so the re-import is new rather than kept. This call-site form, not type ownership or dispatch-fn placement, is what actually separates the Phase 6a variant below from full domain extraction. Where a subsystem has a dedicated clap subcommand, its command *enum* usually stays in `main.rs` as part of the `Command`/`Cli` definition (e.g. `ComfyuiCommand`, `RuntimesCommand`) — but the dispatch function itself does not always stay with it: `comfyui()` does, while `engines()` (Phase 6a) and `runtimes()` (Phase 6c, ROCMAI-89) moved out to `engines_cmd.rs`/`runtimes_cmd.rs` respectively, following the mechanical-relocation re-import form below rather than full domain extraction's qualified-path form. Not every domain-extracted module has a dedicated command enum at all (`providers.rs` has none; it's invoked from the existing chat/config command flows). In library crates (`crates/rocm-core`) the module is `pub mod x;` plus a `pub use x::{...};` re-export, since it's part of the crate's public API. This is the default for new subsystems. Examples: [`apps/rocm/src/therock.rs`](../apps/rocm/src/therock.rs) (`RuntimesCommand`'s domain types), [`comfyui.rs`](../apps/rocm/src/comfyui.rs) (`ComfyuiCommand`), [`providers.rs`](../apps/rocm/src/providers.rs) (no dedicated command enum); `crates/rocm-core`'s [`diagnose.rs`](../crates/rocm-core/src/diagnose.rs)/[`examine.rs`](../crates/rocm-core/src/examine.rs). -- **Mechanical relocation** — a `pub(crate) fn` moves out verbatim, with shared types/config staying at the crate root and reached via `crate::`. Used for dispatch-adjacent clusters where a minimal, easy-to-review diff matters more than full extraction. Example: [`apps/rocm/src/automations.rs`](../apps/rocm/src/automations.rs). Variant: when the cluster is large enough to own private types (part of `main.rs`'s Phase 6a split, ROCMAI-82, `main.rs`'s Phase 6b split, ROCMAI-91, `main.rs`'s Phase 6c split, ROCMAI-89, and Phase 6e's follow-up, ROCMAI-726), those types move with it instead of staying in `main.rs` — [`engines_cmd.rs`](../apps/rocm/src/engines_cmd.rs)'s types are used nowhere else, but [`driver_install.rs`](../apps/rocm/src/driver_install.rs)'s `DriverInstallResult`/`DriverInstallError` are `pub(crate)` and read from `main.rs`'s `install()` (`result.output`/`result.executed`, `error.source`/`error.executed`); [`freeform_execution.rs`](../apps/rocm/src/freeform_execution.rs)'s `FreeformExecution` is `pub(crate)` and read from `main.rs`'s `execute_freeform_next_action()` (`execution.action`) — it was already `pub(crate)` before the split, unlike the two functions that return/consume it, which widened from module-private; [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs)'s `ServeEngineSelection`/`ServeArgs` both stay `pub(crate)` and are both used from `main.rs` — `assess_model_for_host` calls `select_serve_engine` and reads the resulting `ServeEngineSelection`'s `.engine`/`.source` fields — and `ServeGenerationDefaults` is private; [`service_launch.rs`](../apps/rocm/src/service_launch.rs)'s `AttachOutcome`/`RawModeGuard`/`ManagedSpawn`/`AutostartClaim` are private, and its `ManagedLaunchReport` is `pub(crate)` and read from [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs) for the interactive summary; [`runtimes_cmd.rs`](../apps/rocm/src/runtimes_cmd.rs)'s `RuntimeActivationResult` and `ActiveRuntimeMarker` are `pub(crate)` and read from `main.rs`'s `apply_runtime_update`/`append_update_activate_summary` and from [`storage.rs`](../apps/rocm/src/storage.rs) respectively; its sibling [`runtime_install_finalize.rs`](../apps/rocm/src/runtime_install_finalize.rs)'s `SdkInstallConsents` is `pub(crate)` and read from `main.rs`'s `install()`. The pattern is still "mechanical relocation" rather than full domain extraction because call sites keep their pre-extraction unqualified-re-import form — the exception named above — rather than switching to the qualified paths full-domain-extraction modules use. The top-level dispatch fns (`install()`/`dispatch()`) staying in `main.rs` is not what makes it so: full domain extraction usually keeps its dispatch fn in `main.rs` too (see `comfyui()` above). +- **Mechanical relocation** — a `pub(crate) fn` moves out verbatim, with shared types/config staying at the crate root and reached via `crate::`. Used for dispatch-adjacent clusters where a minimal, easy-to-review diff matters more than full extraction. Example: [`apps/rocm/src/automations.rs`](../apps/rocm/src/automations.rs). Variant: when the cluster is large enough to own private types (part of `main.rs`'s Phase 6a split, ROCMAI-82, `main.rs`'s Phase 6b split, ROCMAI-91, `main.rs`'s Phase 6c split, ROCMAI-89, and Phase 6e's follow-up, ROCMAI-726), those types move with it instead of staying in `main.rs` — [`engines_cmd.rs`](../apps/rocm/src/engines_cmd.rs)'s types are used nowhere else, but [`driver_install.rs`](../apps/rocm/src/driver_install.rs)'s `DriverInstallResult`/`DriverInstallError` are `pub(crate)` and read from `main.rs`'s `install()` (`result.output`/`result.executed`, `error.source`/`error.executed`); [`freeform_execution.rs`](../apps/rocm/src/freeform_execution.rs)'s `FreeformExecution` is `pub(crate)` and read from `main.rs`'s `execute_freeform_next_action()` (`execution.action`) — it was already `pub(crate)` before the split, unlike the two functions that return/consume it, which widened from module-private; [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs)'s `ServeEngineSelection`/`ServeArgs` both stay `pub(crate)` and are both used from `main.rs` — `assess_model_for_host` calls `select_serve_engine` and reads the resulting `ServeEngineSelection`'s `.engine`/`.source` fields — and `ServeGenerationDefaults` is private; [`service_launch.rs`](../apps/rocm/src/service_launch.rs)'s `AttachOutcome`/`RawModeGuard`/`ManagedSpawn`/`AutostartClaim` are private, and its `ManagedLaunchReport` is `pub(crate)` and read from [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs) for the interactive summary; [`runtimes_cmd.rs`](../apps/rocm/src/runtimes_cmd.rs)'s `RuntimeActivationResult` and `ActiveRuntimeMarker` are `pub(crate)` and read from [`update_cmd.rs`](../apps/rocm/src/update_cmd.rs)'s `apply_runtime_update`/`append_update_activate_summary` and from [`storage.rs`](../apps/rocm/src/storage.rs) respectively; its sibling [`runtime_install_finalize.rs`](../apps/rocm/src/runtime_install_finalize.rs)'s `SdkInstallConsents` is `pub(crate)` and read from `main.rs`'s `install()`. The pattern is still "mechanical relocation" rather than full domain extraction because call sites keep their pre-extraction unqualified-re-import form — the exception named above — rather than switching to the qualified paths full-domain-extraction modules use. The top-level dispatch fns (`install()`/`dispatch()`) staying in `main.rs` is not what makes it so: full domain extraction usually keeps its dispatch fn in `main.rs` too (see `comfyui()` above). There is no file-line-count CI gate enforcing this — `too_many_lines = "allow"` in the workspace `Cargo.toml` is a deliberate, function-level choice, not an oversight. This convention is the guardrail instead. @@ -29,7 +29,7 @@ Each per-crate module inventory lists one file per line. Flat inventories (every [`main.rs`](../apps/rocm/src/main.rs) itself is **partially modularized** — Phase 6a (ROCMAI-82) extracted the driver-install/engines/serve clusters, Phase 6b (ROCMAI-91) extracted the managed-process-launch cluster, Phase 6c (ROCMAI-89) extracted the `runtimes` command/uninstall/rollback/SDK-install-finalization clusters, Phase 6d (ROCMAI-88) extracted the chat/assistant and MCP tool-bridge cluster below across three PRs, Phase 6e (ROCMAI-87) extracted the freeform natural-language planner's plan-*building* half below, with a same-phase follow-up (ROCMAI-726) extracting the plan-*execution*/consent half it deliberately left behind; the rest of the split is planned across several more PRs, one cluster at a time (see EAI-7768). -- [`automations.rs`](../apps/rocm/src/automations.rs) — mechanical relocation, dispatch-adjacent, no owned types. +- [`automations.rs`](../apps/rocm/src/automations.rs) — mechanical relocation, owns private types (Phase 6f, ROCMAI-210): the `rocm automations` dispatch handler (`automations()`) plus the `rocm automations` status report it prints (`render_automations_text`), the `ProposalKind` classification that report renders through (`proposal_kind`, `fallback_tool_for_automation_action`, `proposal_plain_summary`, `proposal_plain_reason`), the small plain-language label helpers (`proposal_status_label`, `proposal_bool_argument`, `automation_event_plain_summary`, `audit_event_plain_summary`, `watcher_mode_plain_label`, `watcher_last_check`, `watcher_plain_name`, `watcher_plain_trigger`, `watcher_plain_action`, `watcher_policy_note`), and the unrelated `rocm daemon --status` summary (`render_daemon_text`). `render_automations_text` and `render_daemon_text` are `pub(crate)`: [`main.rs`](../apps/rocm/src/main.rs)'s `dispatch()` calls `render_daemon_text` directly (re-imported alongside `automations` via `use crate::automations::{automations, render_daemon_text};`; `render_automations_text` has no `dispatch()` call site of its own, reached only through `automations()` above, now local to this file), and [`chat.rs`](../apps/rocm/src/chat.rs)'s deterministic tool summaries call both. Every other item in this list, including `watcher_policy_note` (its only caller is `automations()`'s `Enable` arm in this same file), has no caller outside this module and stays private. The cluster's regression tests live in this module's own `mod tests`, reaching the shared `test_paths` fixture through `crate::tests::` the same way other mechanically relocated modules do. - [`bootstrap.rs`](../apps/rocm/src/bootstrap.rs) — a further-extracted variant of full domain extraction: it owns its clap command enum (`BootstrapCommand`) and dispatch function too, rather than leaving them in [`main.rs`](../apps/rocm/src/main.rs). - [`chat.rs`](../apps/rocm/src/chat.rs) — full domain extraction (Phase 6d, ROCMAI-88), mirrors [`providers.rs`](../apps/rocm/src/providers.rs). Owns the chat/MCP-bridge cluster's types (`ChatRocmCommandAction`, `CommandCapture`, `DeterministicModelRecipe`, `DeterministicServiceRow`, `DeterministicEngineRow`, and others — all module-private except `ChatToolApprovalRequest`, reached by `dash_seam.rs` as `chat_tool_approval_request`'s return type), its validation/approval/execution logic (`validate_chat_tool_call` and the `validate_chat_*_tool_call` family), deterministic tool-output summarization (including `render_services_tool_result_text`), and the MCP tool-bridge entry points (`run_internal_mcp_call` and its helpers). [`main.rs`](../apps/rocm/src/main.rs), [`dash.rs`](../apps/rocm/src/dash.rs), and [`dash_seam.rs`](../apps/rocm/src/dash_seam.rs) call into it in production; [`comfyui.rs`](../apps/rocm/src/comfyui.rs) and [`runtime_install_finalize.rs`](../apps/rocm/src/runtime_install_finalize.rs) reference it only from their own `#[cfg(test)]` modules (`deterministic_comfyui_tool_summary` and `rocm_chat_tool_requested_args` respectively) — every one of these five files' references still goes through the `chat::` module boundary. As a child module of the crate root (`mod chat;` in `main.rs`), Rust's module-privacy rule already makes most of `main.rs`'s private helpers visible to `chat.rs` without any visibility change, when `chat.rs` calls into `main.rs`. The other direction needs real widening: `chat.rs`'s own functions reached from those five files — `run_internal_mcp_call` (a production call from `main.rs`), `rocm_chat_tool_requested_args` (reached only by `runtime_install_finalize.rs`'s own test module), among others — stay `pub(crate)`; the third PR's visibility audit tightened every other function to module-private, leaving 20 of `chat.rs`'s 123 functions `pub(crate)`, each with a confirmed external caller, production or test-only. The cluster's regression tests live in `chat.rs`'s own `mod tests`, moved out of `main.rs`'s shared `mod tests` block across the second and third PRs; two `main.rs` tests that reference `chat::` only incidentally — structural checks on `main.rs`'s own `dispatch()` source (`command_chat_honors_chat_mock_and_keeps_prompt_passthrough`, `command_chat_reads_prompt_from_piped_stdin`) — plus `serve_negative_sampling_space_form_reaches_range_validator`, a shared clap value-parser wiring test living alongside its sibling command-parsing tests, stay in `main.rs`. - [`chat_host_facts.rs`](../apps/rocm/src/chat_host_facts.rs) — full domain extraction. @@ -60,6 +60,7 @@ Each per-crate module inventory lists one file per line. Flat inventories (every - [`therock.rs`](../apps/rocm/src/therock.rs) — full domain extraction; owns TheRock SDK install/update and the runtime-manifest model/loaders. The `runtimes` subcommand's own command enum, dispatch fn, and most of its domain logic — rendering, activation, and manifest import — stay in [`main.rs`](../apps/rocm/src/main.rs). - [`uninstall.rs`](../apps/rocm/src/uninstall.rs) — mechanical relocation, owns private types: `uninstall()`, plus the installer `PATH` undo and running-executable deletion. The shared plan type and its file-removal helpers live in [`uninstall_plan.rs`](../apps/rocm/src/uninstall_plan.rs) below instead, because [`storage.rs`](../apps/rocm/src/storage.rs) reuses them too. - [`uninstall_plan.rs`](../apps/rocm/src/uninstall_plan.rs) — mechanical relocation, shares crate-visible types (Phase 6f, ROCMAI-212): the uninstall-plan types (`UninstallOptions`, `UninstallPlan`, `UninstallPlanEntry`), building the plan (`build_uninstall_plan`, `plan_installed_binaries`, and the installed-binary/shared-cache discovery helpers behind them), rendering it (`render_uninstall_dry_run`, `render_uninstall_plan`), the `[y/N]` confirmation prompt (`confirm_uninstall`), and the filesystem removal primitive both [`uninstall.rs`](../apps/rocm/src/uninstall.rs) and [`storage.rs`](../apps/rocm/src/storage.rs) call to carry a plan out (`remove_path`, and the entry-resolution helpers behind it). Kept separate from `uninstall.rs` rather than folded into it, since `uninstall.rs`'s own doc comment and name both describe `uninstall()`'s exclusive logic (the installer `PATH` undo and running-executable deletion), while this module's plan/render/removal helpers are shared with `storage.rs`. `uninstall.rs`, `storage.rs`, `runtimes_cmd.rs`, `chat.rs`, and `deletion_properties.rs` each import the subset of these `pub(crate)` types and helpers they need; see this module's own doc comment for which symbol each one uses. +- [`update_cmd.rs`](../apps/rocm/src/update_cmd.rs) — mechanical relocation, dispatch-adjacent, no owned types (Phase 6f, ROCMAI-210): `rocm update`'s own rendering/activation logic — the read-only status report (`render_update_text`, `append_update_surfaces`), the predicate deciding whether `update` should route into the mutating path instead of that report (`update_should_preview_or_apply`), applying a runtime update (`apply_runtime_update`, `append_update_activate_summary`), and the update-source/installed-runtime pickers behind it (`select_runtime_update_source`, `select_installed_update_runtime`). `const ROLLBACK_RECOVERY_HINT` stays in `main.rs` as a shared crate-root constant, because callers outside this module also use it ([`runtimes_cmd.rs`](../apps/rocm/src/runtimes_cmd.rs)'s `RuntimesCommand::Activate` handler and [`runtime_install_finalize.rs`](../apps/rocm/src/runtime_install_finalize.rs)'s `render_sdk_install_success`); this module reaches it via `use crate::ROLLBACK_RECOVERY_HINT;`, the same way `gpu_policy.rs`'s `device_policy_name` stays in `main.rs`. `render_update_text`, `apply_runtime_update`, and `update_should_preview_or_apply` are `pub(crate)` and read from [`main.rs`](../apps/rocm/src/main.rs)'s `dispatch()` (the `update` command arm, all three re-imported via `use crate::update_cmd::{...};`); [`chat.rs`](../apps/rocm/src/chat.rs) also calls into it for `render_update_text` (the deterministic `rocm update` tool summary); `select_runtime_update_source` is also `pub(crate)`, read from `therock_version_proptests.rs`'s property tests via `crate::update_cmd::select_runtime_update_source`. `append_update_surfaces`, `append_update_activate_summary`, and `select_installed_update_runtime` have no callers outside this module and stay private. The cluster's regression tests live in `update_cmd.rs`'s own `mod tests`, reaching shared fixtures (`test_paths`, `test_runtime_manifest_for_update`, `write_test_pip_runtime`) through `crate::tests::`/`crate::test_support::` the same way other mechanically relocated modules do. ### `apps/rocmd` — background daemon