diff --git a/ROADMAP.md b/ROADMAP.md index 5b2621d3..972b7aa0 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -9,19 +9,46 @@ Bench / Chat), the no-key ChatGPT-OAuth chat backend, the unified `config.json` Windows-clean build, and `rocm dash --demo/--replay/--chat-mock`. This file captures (1) the remaining work to reach full **feature parity** with -the two surfaces the merge replaces, and (2) the scoped **Per-process VRAM → -model** dashboard item. +the two surfaces the merge replaces, (2) the **background-helper (`rocm daemon`) +wiring** regression, and (3) the scoped **Per-process VRAM → model** dashboard +item. + +> **What shipped (Supergoal §1 + §2).** Items (1) and (2) are now done. The dash +> Chat tab reached agentic parity (read-only ROCm tools, approval-gated mutating +> tools, `/plan`, `/provider` + Anthropic, a 30-command parity map) and bare +> `rocm`/`rocm chat` reroute to it; `tui.rs` is retained behind an explicit +> retire gate (deletion not performed). `rocm daemon` runs the real loop +> in-process via the `rocmd` lib with on-demand, double-spawn-guarded autostart. +> Item (3) (per-model VRAM) remains the open dashboard follow-up. --- ## 1. Feature parity — retire `tui.rs` without regression -**Status: the only real parity gap.** Bare `rocm` and `rocm chat` still launch -the legacy chat-first assistant (`apps/rocm/src/tui.rs`); its retirement was -**deferred** because the new dashboard **Chat** tab is a read-only telemetry -chat and lacks the legacy assistant's agentic capabilities. Closing this gap is -the prerequisite for deleting `tui.rs` and routing bare `rocm`/`rocm chat` to -the unified TUI. +**Status: parity REACHED; bare `rocm`/`rocm chat` REROUTED to the dash; +deletion GATED (not performed).** The dash **Chat** tab now matches the legacy +assistant's agentic capabilities, and bare `rocm` + interactive `rocm chat` +route to the unified dash chat (`dash::run_chat`). `apps/rocm/src/tui.rs` is +**retained** (anchored by `_RETAINED_TUI_ENTRY`) behind an explicit accept/retire +gate — the actual file deletion is intentionally not done in this supergoal. + +**What shipped (Supergoal §1, Phases 3–9):** +- Read-only ROCm tools through a `rocm-core`-free execution seam + (`tool_exec.rs` / `dash_seam.rs`): doctor, engines, services, logs, snapshots, + automations, path/port checks, update-check, `rocm_command`. +- Mutating tools gated by the existing `ui/approval.rs` modal (`install_sdk`, + `install_engine`, `launch_server`, `stop_server`, `watcher_enable/disable`) + plus update/comfyui/uninstall/setup and automations review/approve/reject/edit. +- Natural-language **`/plan`** (Ask → Plan → Review → Run). +- **`/provider`** live backend switching + the **Anthropic** backend; the full + slash-command set. +- A **30-command parity map** + accept/retire checklists: + [`docs/dash-parity-map.md`](docs/dash-parity-map.md), + [`docs/dash-parity-checklist.md`](docs/dash-parity-checklist.md), + [`docs/tui-retirement-checklist.md`](docs/tui-retirement-checklist.md). + +The original gap analysis and build order below are kept for historical context; +all stages are now done except the **gated** Stage-8 deletion. **What the dash Chat tab is missing vs `tui.rs`:** - Mutating ROCm tool-calls with **in-chat approval** (`install_sdk`, @@ -77,7 +104,67 @@ ureq/reqwest partition intact. --- -## 2. Per-process VRAM → model attribution (rocm dash) +## 2. Background-helper (`rocm daemon`) wiring — regression + +**Status: DONE.** `rocm daemon` now runs the real foreground loop **in-process** +via the `rocmd` library (`rocm` → `rocmd` lib; acyclic), matching the "built into +rocm" policy. The helper is **autostarted on demand**, detached and +double-spawn-guarded (`ensure_background_helper_running` / +`background_helper_already_running`), from both `automations enable` and +`rocm serve --managed`. `render_daemon_text` is retained as the `--status` view +only. The original regression analysis below is kept for historical context. + +**Status (historical): regression. The background helper is dormant.** +`rocm daemon` is documented as *"Start the background helper in the foreground"* +(`apps/rocm/src/main.rs:250`), but its handler only renders a status panel: + +```rust +// apps/rocm/src/main.rs:1298 +Some(Command::Daemon) => { + print!("{}", render_daemon_text(&paths, &config)); // status only — no loop + Ok(()) +} +``` + +The real foreground loop, `run_daemon()`, exists **only in the separate `rocmd` +binary** (`apps/rocmd/src/lib.rs:2808`, reached via `rocmd run`). But: + +- `apps/rocm` does **not** depend on `rocmd` (its `Cargo.toml` pulls + `rocm-core`, `rocm-dash-daemon`, `rocm-dash-tui` — not `rocmd`). +- **Nothing in `apps/rocm` ever spawns `rocmd run` or `rocm daemon` as a running + loop.** The only `"daemon"` references are status renderers and tests. +- `daemon_binary_path()` (`crates/rocm-core/src/lib.rs:5859`) resolves to the + **`rocm`** binary itself, and `main.rs:10666` states *"policy: built into + rocm; no separate rocmd binary is required"* — i.e. the intended design is + that `rocm daemon` **is** the helper loop, re-executing itself. That loop + logic is missing; it is stubbed to the status panel. + +**Effect:** automation checks and on-demand local model servers have no helper +to run, regardless of the §1 chat-parity work. `run_daemon()` in `rocmd` is +orphaned code the product never invokes. + +**Build order:** +1. **Decide the model** — either (a) fold `rocmd`'s `run_daemon()` into the + `rocm` crate so `rocm daemon` runs the loop in-process (matches the + "built into rocm" policy), or (b) have `rocm daemon` spawn `rocmd run`, with + automations/managed-serve starting it on demand. (a) is the stated direction. +2. **Wire the chosen entry** so `rocm daemon` actually starts the loop in the + foreground; keep `render_daemon_text` as the *status* view only. +3. **On-demand start** — have automation-enable and `rocm serve --managed` + ensure the helper is running (spawn detached if not), then update + `AutomationRuntimeState` so the status panel reflects reality. +4. **Reconcile `rocmd`** — once the loop lives in `rocm`, either remove the + orphaned `rocmd` binary or make it a thin alias; don't ship two daemons. +5. **Verify** — `rocm daemon` runs and accepts work; automation checks fire; + status panel shows `running`; clean shutdown. + +**Invariants:** keep the user-owned unix socket (mode 0600) hardening from #17; +no second listener/socket competing with the `rocm dash` telemetry daemon +(`rocm-dash-daemon`) — these are distinct daemons and must stay distinct. + +--- + +## 3. Per-process VRAM → model attribution (rocm dash) **Goal:** attribute each running model/serving instance's GPU VRAM by joining `amd-smi` per-process VRAM to the owning model/container — so the dashboard diff --git a/apps/rocm/src/dash.rs b/apps/rocm/src/dash.rs index 4b766b08..1e5a45f4 100644 --- a/apps/rocm/src/dash.rs +++ b/apps/rocm/src/dash.rs @@ -166,15 +166,15 @@ fn automation_summaries(config: &RocmCliConfig) -> Vec { /// Resolve the TUI args from the unified config + environment. /// -/// `anthropic_api_key` is resolved by the caller *before* any tokio runtime is -/// entered: the secure-store fallback uses a blocking zbus client that spins its -/// own runtime, which panics ("cannot start a runtime from within a runtime") if -/// invoked from inside `run_async`. See [`anthropic_api_key_for_dash`]. +/// MUST be called on a synchronous thread *before* any tokio runtime is entered: +/// the Anthropic-key secure-store fallback ([`anthropic_api_key_for_dash`]) uses +/// a blocking zbus client that spins its own runtime, which panics ("cannot +/// start a runtime from within a runtime") if invoked from inside `run_async`. +/// The sync entry points `run`/`run_chat` call this and pass the result in. pub fn resolved_args( config: &RocmCliConfig, paths: &AppPaths, initial_tab: ActiveTab, - anthropic_api_key: Option, ) -> ResolvedArgs { let t = &config.dashboard.tui; ResolvedArgs { @@ -190,7 +190,7 @@ pub fn resolved_args( .ok() .filter(|v| !v.is_empty()), chat_api_key: chat_api_key_from_env(), - anthropic_api_key, + anthropic_api_key: anthropic_api_key_for_dash(), chat_auto_consent: false, chat_mock: false, model_recipes: model_recipe_summaries(), @@ -202,6 +202,16 @@ pub fn resolved_args( } } +/// Build the multi-thread tokio runtime the async daemon/TUI run on. Shared by +/// the synchronous [`run`] and [`run_chat`] entry points (the rest of `rocm` is +/// synchronous; only the dashboard needs an async reactor). +fn build_dashboard_runtime() -> Result { + tokio::runtime::Builder::new_multi_thread() + .enable_all() + .build() + .context("building tokio runtime for the dashboard") +} + /// Entry point for `rocm dash`. Builds a tokio runtime and runs the dashboard. pub fn run(replay: Option, demo: bool, chat_mock: bool) -> Result<()> { let paths = AppPaths::discover()?; @@ -219,21 +229,15 @@ pub fn run(replay: Option, demo: bool, chat_mock: bool) -> Result<()> { } else { replay }; - // Resolve the Anthropic key before the runtime exists; the secure-store - // fallback blocks on its own zbus runtime and would panic inside `run_async`. - let anthropic_api_key = anthropic_api_key_for_dash(); - let rt = tokio::runtime::Builder::new_multi_thread() - .enable_all() - .build() - .context("building tokio runtime for the dashboard")?; - rt.block_on(run_async( - config, - paths, - replay, - chat_mock, - ActiveTab::Overview, - anthropic_api_key, - )) + // Resolve TUI args — including the OS secure-store (keyring) lookup for the + // Anthropic key — on this plain synchronous thread, BEFORE entering the tokio + // runtime. The secure-store path (`provider_keys` → secret-service) uses + // `zbus::blocking`, which builds its own runtime and `block_on`s internally; + // doing that on a dash runtime worker thread panics with "Cannot start a + // runtime from within a runtime". See `run_async`. + let args = resolved_args(&config, &paths, ActiveTab::Overview); + let rt = build_dashboard_runtime()?; + rt.block_on(run_async(config, paths, args, replay, chat_mock)) } /// Entry point for bare `rocm` and interactive `rocm chat`. Opens the unified @@ -242,32 +246,23 @@ pub fn run(replay: Option, demo: bool, chat_mock: bool) -> Result<()> { pub fn run_chat(chat_mock: bool) -> Result<()> { let paths = AppPaths::discover()?; let config = RocmCliConfig::load(&paths)?; - // Resolve the Anthropic key before the runtime exists; the secure-store - // fallback blocks on its own zbus runtime and would panic inside `run_async`. - let anthropic_api_key = anthropic_api_key_for_dash(); - let rt = tokio::runtime::Builder::new_multi_thread() - .enable_all() - .build() - .context("building tokio runtime for the dashboard")?; - rt.block_on(run_async( - config, - paths, - None, - chat_mock, - ActiveTab::Chat, - anthropic_api_key, - )) + // See `run`: resolve args (incl. the keyring lookup) before the runtime so the + // secure-store `zbus::blocking` path never runs on a runtime worker thread. + let args = resolved_args(&config, &paths, ActiveTab::Chat); + let rt = build_dashboard_runtime()?; + rt.block_on(run_async(config, paths, args, None, chat_mock)) } async fn run_async( config: RocmCliConfig, paths: AppPaths, + mut args: ResolvedArgs, replay: Option, chat_mock: bool, - initial_tab: ActiveTab, - anthropic_api_key: Option, ) -> Result<()> { - let mut args = resolved_args(&config, &paths, initial_tab, anthropic_api_key); + // `args` is built by the synchronous caller (`run`/`run_chat`) so the keyring + // lookup inside `resolved_args` never runs on a runtime worker thread (it uses + // `zbus::blocking`, which would otherwise panic: runtime-within-a-runtime). args.replay = replay.clone(); args.chat_mock = chat_mock; // Inject the bin-side tool-execution seam for a live dash only. Demo/replay @@ -375,7 +370,7 @@ mod tests { #[test] fn resolved_args_take_connect_and_theme_from_config() { let c = cfg(); - let args = resolved_args(&c, &paths(), ActiveTab::Overview, None); + let args = resolved_args(&c, &paths(), ActiveTab::Overview); assert_eq!(args.connect, c.dashboard.tui.connect); assert_eq!(args.theme, c.dashboard.tui.theme); assert!(!args.chat_mock); diff --git a/apps/rocm/src/main.rs b/apps/rocm/src/main.rs index 646676d3..4b9ac26d 100644 --- a/apps/rocm/src/main.rs +++ b/apps/rocm/src/main.rs @@ -1347,6 +1347,13 @@ fn dispatch(cli: Cli) -> Result<()> { // honored only on the non-interactive render path below. The // legacy `tui::run` assistant is retained but no longer invoked // here (see docs/tui-retirement-checklist.md). + if provider.is_some() { + // --provider isn't threaded into the interactive dash; say so + // instead of dropping it silently — the user can switch live. + eprintln!( + "note: launching the dash chat; switch providers with /provider " + ); + } return dash::run_chat(chat_mock); } match prompt { @@ -4216,6 +4223,16 @@ fn start_managed_service( Ok(()) } +/// The "should we spawn?" decision for [`ensure_background_helper_running`], +/// factored out so it is testable hermetically (no spawn side effect). Returns +/// `true` when the file-based runtime state says the daemon is `running` AND its +/// recorded `daemon_pid` is a live process — i.e. a second spawn must be guarded. +/// A missing state file, `running=false`, or a dead/zero pid returns `false`. +pub(crate) fn background_helper_already_running(paths: &AppPaths) -> Result { + Ok(AutomationRuntimeState::load(paths)? + .is_some_and(|state| state.running && rocm_core::process_is_running(state.daemon_pid))) +} + /// Shared daemon-lifecycle entrypoint: ensures the background automation helper /// (`rocm daemon`) is running, spawning it detached if not. Liveness is read from /// the file-based automation runtime state. Intentionally `pub(crate)` — reused by @@ -4224,10 +4241,7 @@ fn start_managed_service( /// propagated; setup errors (path discovery, stdio attach) still return `Err`. pub(crate) fn ensure_background_helper_running() -> Result<()> { let paths = AppPaths::discover()?; - if let Some(state) = AutomationRuntimeState::load(&paths)? - && state.running - && rocm_core::process_is_running(state.daemon_pid) - { + if background_helper_already_running(&paths)? { return Ok(()); } @@ -22263,6 +22277,64 @@ VERSION_ID="41" ) } + /// Build an `AutomationRuntimeState` for the no-double-spawn guard tests. + fn runtime_state(running: bool, daemon_pid: u32) -> AutomationRuntimeState { + AutomationRuntimeState { + running, + automations_enabled: true, + daemon_pid, + started_at_unix_ms: 1, + last_tick_unix_ms: 1, + local_webhook_endpoint: None, + active_watchers: Vec::new(), + } + } + + #[test] + fn background_helper_already_running_true_for_live_pid() { + // Phase-10 daemon no-double-spawn: a runtime-state.json with running=true + // and a LIVE daemon_pid (this very test process) means the helper is + // already up — the "should spawn?" decision must say NO (true ⇒ skip). + // Hermetic + offline: no spawn, just the file-based liveness check. + let (root, paths) = test_paths("helper-live-pid"); + runtime_state(true, std::process::id()) + .write(&paths) + .expect("write runtime state"); + assert!( + background_helper_already_running(&paths).expect("liveness check ok"), + "live recorded pid + running=true ⇒ do not spawn a second daemon" + ); + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn background_helper_already_running_false_for_dead_or_missing() { + // The inverse guard cases — each must report NOT running (false ⇒ spawn): + // (1) no state file at all, (2) running=true but a dead/zero pid, + // (3) a live pid but running=false. None must spawn from this decision. + let (root, paths) = test_paths("helper-dead-or-missing"); + // (1) No state file yet. + assert!( + !background_helper_already_running(&paths).expect("missing state ⇒ ok"), + "no runtime state ⇒ not running" + ); + // (2) running=true but pid 0 is never a live process. + runtime_state(true, 0).write(&paths).expect("write state"); + assert!( + !background_helper_already_running(&paths).expect("dead pid ⇒ ok"), + "running=true + dead pid ⇒ not running (spawn)" + ); + // (3) live pid but running flag is false. + runtime_state(false, std::process::id()) + .write(&paths) + .expect("write state"); + assert!( + !background_helper_already_running(&paths).expect("not-running flag ⇒ ok"), + "running=false ⇒ not running even with a live pid" + ); + let _ = fs::remove_dir_all(&root); + } + // ---- Phase 9: reroute dispatch (bare `rocm` + interactive `rocm chat`) ---- // // The interactive branches require a real TTY (`interactive_terminal()`), @@ -22404,6 +22476,24 @@ VERSION_ID="41" ); } + #[test] + fn command_chat_interactive_notes_dropped_provider_flag() { + // Phase-9 polish: --provider on interactive `rocm chat` is no longer + // silently ignored — the handler emits a one-line note when provider is + // set before rerouting to the dash. Proven by reading the handler body + // (the interactive branch requires a TTY, unavailable in CI). + let src = main_rs_source(); + let body = strip_line_comments(&command_chat_handler_body(&src)); + assert!( + body.contains("provider.is_some()"), + "handler must gate the note on a set --provider; body:\n{body}" + ); + assert!( + body.contains("/provider"), + "the note must point the user at /provider for live switching; body:\n{body}" + ); + } + #[test] fn command_chat_honors_chat_mock_and_keeps_prompt_passthrough() { let src = main_rs_source(); diff --git a/crates/rocm-dash-tui/src/agent.rs b/crates/rocm-dash-tui/src/agent.rs index 7531a0b1..458f6559 100644 --- a/crates/rocm-dash-tui/src/agent.rs +++ b/crates/rocm-dash-tui/src/agent.rs @@ -42,6 +42,11 @@ pub const REQUEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs( /// Max tool-calling turns the model may take before producing a final answer. const MAX_TOOL_TURNS: usize = 5; +/// Max tokens the model may emit in its final answer. Shared by all three +/// backends (RigAgentClient, ChatGptAgentClient, AnthropicAgentClient) so the +/// budget is defined once. `u64` to match rig's `AgentBuilder::max_tokens`. +const MAX_AGENT_TOKENS: u64 = 1024; + /// Default system preamble for the dashboard assistant. const DEFAULT_PREAMBLE: &str = "You are the rocm-dash assistant, embedded in a terminal dashboard for AMD \ Instinct GPU telemetry and benchmarks. Use the provided tools (gpu_status, \ @@ -114,6 +119,29 @@ pub fn annotate_reply(reply: String, skills: &[String]) -> String { format!("{reply}\n⚙ via: {}", seen.join(", ")) } +/// Drive a built rig prompt request to completion under the shared +/// [`REQUEST_TIMEOUT`], then annotate the reply with the Skills that fired. +/// +/// The shared `complete()` tail for all three backends (RigAgentClient, +/// ChatGptAgentClient, AnthropicAgentClient): their `req` types differ (rig +/// typestate), so this is generic over `IntoFuture>` +/// with a `Display` error. A timeout maps to [`AgentError::Timeout`]; a backend +/// error maps to [`AgentError::Request`]; success is annotated with the fired +/// Skills. ONE definition of the tail, used by all three. +async fn finish_agent_request(req: F, fired: &FiredLog) -> Result +where + F: std::future::IntoFuture>, + E: std::fmt::Display, +{ + let reply = match tokio::time::timeout(REQUEST_TIMEOUT, req.into_future()).await { + Err(_) => return Err(AgentError::Timeout), + Ok(Ok(reply)) => reply, + Ok(Err(e)) => return Err(AgentError::Request(e.to_string())), + }; + let skills = fired.lock().map(|g| g.clone()).unwrap_or_default(); + Ok(annotate_reply(reply, &skills)) +} + // --------------------------------------------------------------------------- // Pure tool computations over the snapshot (testable without Rig / async). // --------------------------------------------------------------------------- @@ -944,7 +972,6 @@ impl AgentClient for RigAgentClient { ) -> Result { use rig::client::CompletionClient; use rig::completion::Prompt; - use std::future::IntoFuture; let Some((last, prior)) = history.split_last() else { return Err(AgentError::Empty); @@ -956,7 +983,7 @@ impl AgentClient for RigAgentClient { .client .agent(&self.model) .preamble(&self.preamble) - .max_tokens(1024); + .max_tokens(MAX_AGENT_TOKENS); // Telemetry + skill registry tools (shared registration site). let agent = register_telemetry_tools(agent, &snap, &fired); // Read-only ROCm machine-inspection tools (forward across the seam). @@ -975,14 +1002,7 @@ impl AgentClient for RigAgentClient { .max_turns(MAX_TOOL_TURNS) .with_history(build_messages(prior)); - let reply = match tokio::time::timeout(REQUEST_TIMEOUT, req.into_future()).await { - Err(_) => return Err(AgentError::Timeout), - Ok(Ok(reply)) => reply, - Ok(Err(e)) => return Err(AgentError::Request(e.to_string())), - }; - - let skills = fired.lock().map(|g| g.clone()).unwrap_or_default(); - Ok(annotate_reply(reply, &skills)) + finish_agent_request(req, &fired).await } } @@ -1206,7 +1226,6 @@ impl AgentClient for ChatGptAgentClient { use rig::agent::AgentBuilder; use rig::completion::Prompt; use rig::providers::chatgpt::ResponsesCompletionModel; - use std::future::IntoFuture; let Some((last, prior)) = history.split_last() else { return Err(AgentError::Empty); @@ -1225,7 +1244,7 @@ impl AgentClient for ChatGptAgentClient { let model = ResponsesCompletionModel::new(self.client.clone(), self.model.clone()); let agent = AgentBuilder::new(model) .preamble(&self.preamble) - .max_tokens(1024); + .max_tokens(MAX_AGENT_TOKENS); // Telemetry + skill registry tools (shared registration site). let agent = register_telemetry_tools(agent, &snap, &fired); // Read-only ROCm machine-inspection tools (forward across the seam). @@ -1244,14 +1263,7 @@ impl AgentClient for ChatGptAgentClient { .max_turns(MAX_TOOL_TURNS) .with_history(build_messages(prior)); - let reply = match tokio::time::timeout(REQUEST_TIMEOUT, req.into_future()).await { - Err(_) => return Err(AgentError::Timeout), - Ok(Ok(reply)) => reply, - Ok(Err(e)) => return Err(AgentError::Request(e.to_string())), - }; - - let skills = fired.lock().map(|g| g.clone()).unwrap_or_default(); - Ok(annotate_reply(reply, &skills)) + finish_agent_request(req, &fired).await } } @@ -1319,7 +1331,6 @@ impl AgentClient for AnthropicAgentClient { ) -> Result { use rig::client::CompletionClient; use rig::completion::Prompt; - use std::future::IntoFuture; let Some((last, prior)) = history.split_last() else { return Err(AgentError::Empty); @@ -1334,7 +1345,7 @@ impl AgentClient for AnthropicAgentClient { .client .agent(&self.model) .preamble(&self.preamble) - .max_tokens(1024); + .max_tokens(MAX_AGENT_TOKENS); // Telemetry + skill registry tools (shared registration site). let agent = register_telemetry_tools(agent, &snap, &fired); // Read-only ROCm machine-inspection tools (forward across the seam). @@ -1353,14 +1364,7 @@ impl AgentClient for AnthropicAgentClient { .max_turns(MAX_TOOL_TURNS) .with_history(build_messages(prior)); - let reply = match tokio::time::timeout(REQUEST_TIMEOUT, req.into_future()).await { - Err(_) => return Err(AgentError::Timeout), - Ok(Ok(reply)) => reply, - Ok(Err(e)) => return Err(AgentError::Request(e.to_string())), - }; - - let skills = fired.lock().map(|g| g.clone()).unwrap_or_default(); - Ok(annotate_reply(reply, &skills)) + finish_agent_request(req, &fired).await } } @@ -1800,6 +1804,54 @@ mod tests { assert!(out.get("error").and_then(Value::as_str).is_some()); } + /// Executor whose `execute`/`execute_approved` both return a seam-level + /// `Error`, exercising the recoverable error path (not None, not Approval). + #[derive(Debug)] + struct FakeErrorExec; + impl crate::tool_exec::RocmToolExecutor for FakeErrorExec { + fn execute(&self, _name: &str, _args: &Value) -> RocmToolOutcome { + RocmToolOutcome::Error("boom".to_string()) + } + fn execute_approved(&self, _name: &str, _args: &Value) -> RocmToolOutcome { + RocmToolOutcome::Error("boom".to_string()) + } + } + + #[tokio::test] + async fn read_only_tool_seam_error_is_recoverable() { + // Edge: the injected executor returns RocmToolOutcome::Error("boom"). + // call() must return a Value carrying an `error` key (recoverable), + // never panic — the model can read and recover from it. + let exec: SharedRocmToolExecutor = Arc::new(FakeErrorExec); + let tool = DoctorRocmTool { + executor: Some(exec), + fired: Arc::new(Mutex::new(Vec::new())), + }; + let out = tool.call(json!({})).await.expect("tool call ok (no panic)"); + assert_eq!(out.get("error").and_then(Value::as_str), Some("boom")); + } + + #[tokio::test] + async fn mutating_tool_seam_error_is_recoverable() { + // Edge: a mutating tool whose executor returns Error("boom") on the + // validate step (e.g. bad args) returns the error as a recoverable + // Value — no approval surfaced, no panic. + let exec: SharedRocmToolExecutor = Arc::new(FakeErrorExec); + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::(); + let tool = LaunchServerRocmTool { + executor: Some(exec), + approval_tx: Some(tx), + fired: Arc::new(Mutex::new(Vec::new())), + }; + let out = tool + .call(json!({ "model": "m" })) + .await + .expect("tool call ok (no panic)"); + assert_eq!(out.get("error").and_then(Value::as_str), Some("boom")); + // No approval intent is surfaced on the error path. + assert!(rx.try_recv().is_err(), "error path surfaces no approval"); + } + #[test] fn rocm_read_tool_names_are_complete_and_disjoint_from_skills() { // Every expected read-only tool is registered… diff --git a/crates/rocm-dash-tui/src/app/chat.rs b/crates/rocm-dash-tui/src/app/chat.rs new file mode 100644 index 00000000..a8db0335 --- /dev/null +++ b/crates/rocm-dash-tui/src/app/chat.rs @@ -0,0 +1,394 @@ +// Copyright Advanced Micro Devices, Inc. +// +// SPDX-License-Identifier: MIT + +//! Chat-backend construction, detection, and persistence. +//! +//! The provider→agent factory ([`build_chat_agent`]), the local-engine probe +//! ([`detect_local_chat`] + [`fetch_first_model`]), and the config persistence +//! for an accepted endpoint ([`persist_chat_endpoint`] + [`config_with_chat`]). +//! Split out of `app/mod.rs` to keep the core reducer + event loop focused. The +//! one reducer method here, [`AppState::set_chat_config`], lives with the rest +//! of the chat-backend resolution group it configures. + +use tokio::sync::mpsc; + +use super::{AppState, ChatConsent, ChatProvider, ResolvedArgs}; +use crate::client::ClientMsg; + +/// OpenAI default base URL when the `Openai` provider is selected. +const OPENAI_BASE_URL: &str = "https://api.openai.com/v1"; +/// Default OpenAI model when none is configured via `chat_model`. +const OPENAI_DEFAULT_MODEL: &str = "gpt-5"; + +impl AppState { + pub fn set_chat_config(&mut self, llm: Option, pre_consent: bool) { + self.chat_consent = match (&llm, pre_consent) { + (None, _) => ChatConsent::Unavailable, + (Some(_), true) => ChatConsent::Accepted, + (Some(_), false) => ChatConsent::Pending, + }; + self.chat_llm = llm; + } +} + +/// Construct the chat backend for an explicitly-selected provider (Phase 8). +/// +/// Construction only — NO network I/O happens here (rig clients defer the +/// request to `complete()`). Handles ONLY `Openai` and `Anthropic`; `Local` +/// returns `None` because its build (auto-detect probe → Rig/ChatGPT) is owned +/// by `event_loop`'s inline path and can't be reproduced from `ResolvedArgs` +/// alone. Keys come from `ResolvedArgs` (in-process seam), never argv. `None` +/// signals "couldn't build" (e.g. a missing key) so the caller surfaces an +/// actionable error turn instead of switching to a dead backend. +pub(super) fn build_chat_agent( + provider: ChatProvider, + args: &ResolvedArgs, + executor: Option, + approval_tx: mpsc::UnboundedSender, +) -> Option> { + match provider { + // Local is rebuilt by the caller's inline path (it needs the live probe). + ChatProvider::Local => None, + ChatProvider::Openai => { + // Require a real key. Without this, `RigAgentClient::new` falls back to + // a dummy `sk-no-key` bearer and still builds, so the switch reports + // success and then 401s at request time. Returning `None` here makes + // the caller surface an actionable error and stay on the current + // backend instead of switching to a dead one. + let api_key = args.chat_api_key.clone().filter(|k| !k.trim().is_empty())?; + let cfg = crate::llm::LlmConfig { + base_url: OPENAI_BASE_URL.to_string(), + model: args + .chat_model + .clone() + .filter(|m| !m.is_empty()) + .unwrap_or_else(|| OPENAI_DEFAULT_MODEL.to_string()), + api_key: Some(api_key), + auth_header: None, + }; + crate::agent::RigAgentClient::new(cfg, executor, Some(approval_tx)) + .ok() + .map(|c| std::sync::Arc::new(c) as std::sync::Arc) + } + ChatProvider::Anthropic => { + // Leave base_url empty → the Anthropic backend uses rig's default + // host. model "" → CLAUDE_SONNET_4_6 (resolved inside the backend). + let cfg = crate::llm::LlmConfig { + base_url: String::new(), + model: args.chat_model.clone().unwrap_or_default(), + api_key: args.anthropic_api_key.clone(), + auth_header: None, + }; + crate::agent::AnthropicAgentClient::new(cfg, executor, Some(approval_tx)) + .ok() + .map(|c| std::sync::Arc::new(c) as std::sync::Arc) + } + } +} + +/// Local engines that expose an OpenAI-compatible `/v1` surface the dash chat +/// can talk to directly. A managed service running one of these is a valid +/// auto-detected chat endpoint regardless of which port it bound. +const OPENAI_COMPATIBLE_ENGINES: &[&str] = &["vllm", "lemonade", "llama.cpp", "sglang", "pytorch"]; + +/// A managed-service endpoint the dash chat can route to, picked from the +/// read-only `services` tool payload. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct ManagedChatEndpoint { + /// `endpoint_url` from the registry — already includes the `/v1` suffix. + pub base_url: String, + /// The service's model id (`canonical_model_id`, then `model_ref`), if any. + pub model: Option, +} + +/// Pick the best **ready**, OpenAI-compatible managed-service endpoint from the +/// `services` tool's JSON envelope (`structuredContent.services`). +/// +/// "Ready" mirrors the bin's own HTTP readiness check (what `rocm services` +/// reports), so a selected endpoint has been verified to actually serve. Among +/// ready candidates the most recently created wins. Pure — no I/O; the anchor +/// for the port-detection unit tests. +pub(crate) fn pick_managed_chat_endpoint( + services_result: &serde_json::Value, +) -> Option { + let services = services_result + .get("structuredContent") + .and_then(|s| s.get("services")) + .and_then(serde_json::Value::as_array)?; + + let mut best: Option<(&serde_json::Value, u64)> = None; + for record in services { + let engine = record + .get("engine") + .and_then(serde_json::Value::as_str) + .unwrap_or_default(); + if !OPENAI_COMPATIBLE_ENGINES + .iter() + .any(|known| engine.eq_ignore_ascii_case(known)) + { + continue; + } + let ready = record + .get("status") + .and_then(serde_json::Value::as_str) + .is_some_and(|s| s.eq_ignore_ascii_case("ready")); + if !ready { + continue; + } + let endpoint_present = record + .get("endpoint_url") + .and_then(serde_json::Value::as_str) + .is_some_and(|e| !e.is_empty()); + if !endpoint_present { + continue; + } + let created = record + .get("created_at_unix_ms") + .and_then(serde_json::Value::as_u64) + .unwrap_or(0); + if best + .as_ref() + .is_none_or(|(_, best_created)| created >= *best_created) + { + best = Some((record, created)); + } + } + + let (record, _) = best?; + let base_url = record + .get("endpoint_url") + .and_then(serde_json::Value::as_str)? + .to_string(); + let model = record + .get("canonical_model_id") + .and_then(serde_json::Value::as_str) + .filter(|m| !m.is_empty()) + .or_else(|| { + record + .get("model_ref") + .and_then(serde_json::Value::as_str) + .filter(|m| !m.is_empty()) + }) + .map(str::to_string); + Some(ManagedChatEndpoint { base_url, model }) +} + +/// Query the bin's read-only `services` tool and, if a ready OpenAI-compatible +/// managed service exists, return its endpoint as an [`LlmConfig`]. +/// +/// This is how the dash learns the *actual* port it launched an engine on (e.g. +/// a tool-launched vLLM on a non-default port) instead of guessing the +/// well-known defaults. The seam call is blocking, so it runs off the reactor. +/// A best-effort `/v1/models` fetch both confirms the endpoint is live and +/// supplies the served model id; on fetch failure the registry-reported model +/// is used (the service was already readiness-verified by the bin). +pub(super) async fn detect_managed_chat( + executor: Option, +) -> Option { + let executor = executor?; + let outcome = + tokio::task::spawn_blocking(move || executor.execute("services", &serde_json::json!({}))) + .await + .ok()?; + let crate::tool_exec::RocmToolOutcome::Result(value) = outcome else { + return None; + }; + let picked = pick_managed_chat_endpoint(&value)?; + match fetch_first_model(&picked.base_url).await { + Some(model) => Some(crate::llm::detected_llm_config(&picked.base_url, &model)), + None => picked + .model + .map(|model| crate::llm::detected_llm_config(&picked.base_url, &model)), + } +} + +/// Probe for a local chat engine, returning a ready [`LlmConfig`] or `None`. +/// +/// Registry-first: an engine we launched ourselves (known via the managed- +/// services registry, on whatever port it bound) takes priority over the +/// well-known default ports. Falls back to the TCP probe of the well-known +/// Lemonade/vLLM endpoints when no managed service is available. +pub(super) async fn detect_local_chat( + executor: Option, +) -> Option { + if let Some(cfg) = detect_managed_chat(executor).await { + return Some(cfg); + } + + // TCP probe is blocking; keep it off the async reactor. + let base = tokio::task::spawn_blocking(crate::llm::detect_local_endpoint) + .await + .ok() + .flatten()?; + + // Best-effort model query; fall back to the neutral default on any failure. + let model = fetch_first_model(base) + .await + .unwrap_or_else(|| crate::llm::DEFAULT_CHAT_MODEL.to_string()); + Some(crate::llm::detected_llm_config(base, &model)) +} + +/// Persist an accepted local endpoint to the user's `config.toml`: load the +/// existing config (or defaults), set `tui.chat_url`/`tui.chat_model`, and write +/// it back. Best-effort — returns a human error string on failure. +/// +/// Uses [`default_config_path`] (a `--config` override is not honored by this +/// in-TUI save; that's a documented limitation). All I/O lives here. +pub(super) fn persist_chat_endpoint( + base_url: &str, + model: &str, +) -> Result { + use rocm_dash_core::config::{Config, default_config_path}; + let path = default_config_path().ok_or_else(|| "no config path available".to_string())?; + let cfg = Config::load(&path).unwrap_or_default(); + let next = config_with_chat(cfg, base_url, model); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).map_err(|e| e.to_string())?; + } + let toml = toml::to_string_pretty(&next).map_err(|e| e.to_string())?; + std::fs::write(&path, toml).map_err(|e| e.to_string())?; + Ok(path) +} + +/// Pure immutable transform: return a copy of `cfg` with the chat endpoint set +/// to a local engine (base_url + model), clearing any gateway auth header since +/// local engines need none. +pub(super) fn config_with_chat( + mut cfg: rocm_dash_core::config::Config, + base_url: &str, + model: &str, +) -> rocm_dash_core::config::Config { + cfg.tui.chat_url = Some(base_url.to_string()); + cfg.tui.chat_model = Some(model.to_string()); + cfg.tui.chat_auth_header = None; + cfg +} + +/// GET `{base}/models` and return the first served model id, or `None`. +async fn fetch_first_model(base_url: &str) -> Option { + let url = format!("{}/models", base_url.trim_end_matches('/')); + let client = reqwest::Client::builder() + .timeout(std::time::Duration::from_secs(3)) + .build() + .ok()?; + let resp = client.get(&url).send().await.ok()?; + let json: serde_json::Value = resp.json().await.ok()?; + crate::llm::pick_first_model(&json) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Wrap service records in the `services` tool's success envelope shape + /// (`structuredContent.services`), matching `internal_mcp_tool_success`. + fn services_envelope(records: serde_json::Value) -> serde_json::Value { + serde_json::json!({ + "content": [{ "type": "text", "text": "services" }], + "structuredContent": { "services": records }, + "isError": false, + }) + } + + fn service( + engine: &str, + endpoint_url: &str, + status: &str, + model: &str, + created: u64, + ) -> serde_json::Value { + serde_json::json!({ + "engine": engine, + "endpoint_url": endpoint_url, + "status": status, + "canonical_model_id": model, + "created_at_unix_ms": created, + }) + } + + #[test] + fn picks_ready_vllm_on_nondefault_port() { + // The headline case: a tool-launched vLLM bound a non-default port; the + // registry knows the real endpoint and it must be detected. + let env = services_envelope(serde_json::json!([service( + "vllm", + "http://127.0.0.1:11435/v1", + "ready", + "Qwen3-8B", + 100 + )])); + let picked = pick_managed_chat_endpoint(&env).expect("ready vLLM is picked"); + assert_eq!(picked.base_url, "http://127.0.0.1:11435/v1"); + assert_eq!(picked.model.as_deref(), Some("Qwen3-8B")); + } + + #[test] + fn skips_non_ready_services() { + let env = services_envelope(serde_json::json!([service( + "vllm", + "http://127.0.0.1:11435/v1", + "starting", + "Qwen3-8B", + 100 + )])); + assert_eq!(pick_managed_chat_endpoint(&env), None); + } + + #[test] + fn skips_non_openai_compatible_engines() { + // A hypothetical non-OpenAI engine must not be offered as a chat endpoint. + let env = services_envelope(serde_json::json!([service( + "comfyui", + "http://127.0.0.1:8188", + "ready", + "sd", + 100 + )])); + assert_eq!(pick_managed_chat_endpoint(&env), None); + } + + #[test] + fn prefers_most_recently_created_among_ready() { + let env = services_envelope(serde_json::json!([ + service("vllm", "http://127.0.0.1:8000/v1", "ready", "old", 100), + service("lemonade", "http://127.0.0.1:13305/v1", "ready", "new", 200), + ])); + let picked = pick_managed_chat_endpoint(&env).expect("a ready endpoint"); + assert_eq!(picked.base_url, "http://127.0.0.1:13305/v1"); + assert_eq!(picked.model.as_deref(), Some("new")); + } + + #[test] + fn falls_back_to_model_ref_when_canonical_missing() { + let env = services_envelope(serde_json::json!([{ + "engine": "vllm", + "endpoint_url": "http://127.0.0.1:11435/v1", + "status": "ready", + "canonical_model_id": "", + "model_ref": "org/Model-Ref", + "created_at_unix_ms": 1u64, + }])); + let picked = pick_managed_chat_endpoint(&env).expect("ready endpoint"); + assert_eq!(picked.model.as_deref(), Some("org/Model-Ref")); + } + + #[test] + fn none_on_empty_or_malformed() { + assert_eq!( + pick_managed_chat_endpoint(&services_envelope(serde_json::json!([]))), + None + ); + assert_eq!(pick_managed_chat_endpoint(&serde_json::json!({})), None); + assert_eq!( + pick_managed_chat_endpoint(&serde_json::json!({ "structuredContent": {} })), + None + ); + } + + #[test] + fn skips_ready_record_with_empty_endpoint() { + let env = services_envelope(serde_json::json!([service("vllm", "", "ready", "m", 100)])); + assert_eq!(pick_managed_chat_endpoint(&env), None); + } +} diff --git a/crates/rocm-dash-tui/src/app.rs b/crates/rocm-dash-tui/src/app/mod.rs similarity index 82% rename from crates/rocm-dash-tui/src/app.rs rename to crates/rocm-dash-tui/src/app/mod.rs index 790deed8..617fe0cc 100644 --- a/crates/rocm-dash-tui/src/app.rs +++ b/crates/rocm-dash-tui/src/app/mod.rs @@ -31,6 +31,16 @@ use crate::client::{self, ClientMsg}; use crate::ui; use crate::ui::theme::Theme; +// Submodules holding cohesive pieces of `AppState` + free fns split out of this +// file to keep the core reducer + event loop focused (a file→dir module move: +// `crate::app::*` paths are unchanged). +mod chat; +mod slash; +mod summary; + +use chat::{build_chat_agent, detect_local_chat, detect_managed_chat, persist_chat_endpoint}; +use summary::{parse_plan_result, summarize_json_value, summarize_slash_tool}; + /// Args after CLI + config resolution. Consumed by `run`. #[derive(Debug, Clone)] pub struct ResolvedArgs { @@ -299,6 +309,12 @@ impl ChatProvider { } } +/// Actionable empty-state shown when a chat is submitted with no agent built +/// (no detected endpoint and no provider key). Surfaced as an error turn — never +/// an error dump or a panic — and names the two concrete recovery actions. +pub(crate) const NO_CHAT_BACKEND_MSG: &str = "no chat backend is configured. Press d to detect a local engine, or use \ + /provider openai|anthropic with the matching API key set."; + /// Result of routing a chat-input line through the slash-command handler. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum SlashOutcome { @@ -444,7 +460,7 @@ pub struct AppState { pub serve_wizard: Option, /// Engine manager overlay (Phase 3 Wave 1). `None` = closed. pub engine_manager: Option, - /// Examine overlay (Phase 3 Wave 2). `None` = closed. + /// Doctor overlay (Phase 3 Wave 2). `None` = closed. pub examine_manager: Option, /// Update overlay (Phase 3 Wave 2). `None` = closed. pub update_manager: Option, @@ -489,8 +505,21 @@ pub struct AppState { /// The chat LLM backend currently selected (Phase 8). Defaults to `Local`. pub(crate) active_provider: ChatProvider, /// Edge: a pending `/provider` switch. Raised by `handle_slash_command`, - /// drained once by the event loop which rebuilds the live `agent`. - pub(crate) provider_switch: Option, + /// drained once by the event loop which rebuilds the live `agent`. Carries + /// both the target and the provider that was active BEFORE the optimistic + /// switch, so a failed build (missing key) reverts to the prior provider + /// rather than unconditionally to `Local`. + pub(crate) provider_switch: Option, +} + +/// A pending `/provider` switch edge: the `target` backend plus the `previous` +/// provider captured before the optimistic `active_provider` set. The event-loop +/// drain rebuilds the agent for `target`; on failure it reverts `active_provider` +/// to `previous` (honest display) instead of forcing `Local`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct ProviderSwitch { + pub(crate) previous: ChatProvider, + pub(crate) target: ChatProvider, } impl AppState { @@ -624,15 +653,6 @@ impl AppState { /// Install the resolved chat endpoint and set the initial consent state. /// `None` → `Unavailable`; `Some` → `Accepted` when pre-consented (e.g. /// `--chat-yes`), otherwise `Pending` (the one-time in-TUI prompt). - pub fn set_chat_config(&mut self, llm: Option, pre_consent: bool) { - self.chat_consent = match (&llm, pre_consent) { - (None, _) => ChatConsent::Unavailable, - (Some(_), true) => ChatConsent::Accepted, - (Some(_), false) => ChatConsent::Pending, - }; - self.chat_llm = llm; - } - /// Accept the detected endpoint and enable chat. No-op when no endpoint is /// available. Focuses the input so the user can type immediately. pub const fn accept_chat_consent(&mut self) { @@ -734,481 +754,6 @@ impl AppState { self.chat_dispatch = true; } - /// Route a chat-input line that may be a slash command. Returns - /// [`SlashOutcome::NotCommand`] for plain text (caller dispatches to the - /// agent); otherwise handles it in-reducer and returns - /// [`SlashOutcome::Handled`]. Stays I/O-free: executor-backed commands raise - /// the `slash_tool` edge for the event loop to drain off-thread. - pub(crate) fn handle_slash_command(&mut self, text: &str) -> SlashOutcome { - /// Build a `rocm_command` slash-tool request from an argv slice and a - /// chat-turn label. Centralizes the repeated `name: "rocm_command"` + - /// `{"args": argv}` construction shared by the lifecycle read/mutate arms. - fn rocm_cmd_request(argv: &[&str], label: impl Into) -> SlashToolRequest { - SlashToolRequest { - name: "rocm_command".to_string(), - args: serde_json::json!({ "args": argv }), - label: label.into(), - } - } - - let trimmed = text.trim(); - let Some(rest) = trimmed.strip_prefix('/') else { - return SlashOutcome::NotCommand; - }; - // First whitespace-delimited word after '/', lowercased. - let cmd = rest.split_whitespace().next().unwrap_or("").to_lowercase(); - - match cmd.as_str() { - // --- Group A: nav / session (deterministic, no executor) --- - "home" => self.active_tab = ActiveTab::Overview, - "gpu" => self.active_tab = ActiveTab::Hardware, - "help" | "?" => self.modal = Modal::Help, - "clear" => self.chat.clear(), - "quit" | "exit" => self.should_quit = true, - // --- Group B: read-only overlays (mirror the keybind handlers) --- - "doctor" => { - self.close_overlays(); - self.examine_manager = - Some(crate::ui::examine_manager::ExamineManagerState::default()); - } - "runtimes" => { - self.close_overlays(); - self.runtime_manager = - Some(crate::ui::runtime_manager::RuntimeManagerState::default()); - } - "config" => { - self.close_overlays(); - self.config_manager = - Some(crate::ui::config_manager::ConfigManagerState::default()); - } - "logs" => { - self.close_overlays(); - self.logs_view = Some(crate::ui::logs_view::LogsViewState::default()); - } - // --- Group B: read-only executor-backed (no overlay; off-thread) --- - "model" => { - self.slash_tool = Some(rocm_cmd_request(&["model"], "model")); - } - "daemon" => { - self.slash_tool = Some(rocm_cmd_request(&["daemon", "status"], "daemon status")); - } - // --- Group D: mutating ops (approval-gated; surfaced via the modal) --- - // Each raises a `slash_tool` request whose `execute()` returns - // `ApprovalRequired`; the event loop opens the approval modal. The - // safety validators run inside `execute()` (and again on the approved - // replay), so an unsafe call surfaces an error instead of the modal. - "install" => { - // `/install ` — the install folder is required by the - // validator (it never installs to a system path). - match rest.split_whitespace().nth(1) { - Some(prefix) => { - self.slash_tool = Some(SlashToolRequest { - name: "install_sdk".to_string(), - args: serde_json::json!({ - "channel": "release", - "format": "wheel", - "prefix": prefix, - }), - label: format!("install {prefix}"), - }); - } - None => { - self.chat.push(ChatTurn::error( - "usage: /install (install folder, e.g. /install ~/rocm)" - .to_string(), - )); - } - } - } - "engine" => { - // `/engine ` — the engine name is required by the validator. - match rest.split_whitespace().nth(1) { - Some(engine) => { - self.slash_tool = Some(SlashToolRequest { - name: "install_engine".to_string(), - args: serde_json::json!({ "engine": engine }), - label: format!("engine {engine}"), - }); - } - None => { - self.chat.push(ChatTurn::error( - "usage: /engine (e.g. /engine vllm)".to_string(), - )); - } - } - } - "serve" => { - // `/serve ` — loopback host only (validator rejects public). - match rest.split_whitespace().nth(1) { - Some(model) => { - self.slash_tool = Some(SlashToolRequest { - name: "launch_server".to_string(), - args: serde_json::json!({ "model": model, "host": "127.0.0.1" }), - label: format!("serve {model}"), - }); - } - None => { - self.chat.push(ChatTurn::error( - "usage: /serve (e.g. /serve deepseek-r1)".to_string(), - )); - } - } - } - "services" => { - // `/services stop ` mutates (stop_server, approval-gated); a - // bare `/services` is read-only and lists managed services. - // `restart` is NOT yet wired through the chat seam, so it is - // guided rather than silently running stop (a semantic lie). - let mut words = rest.split_whitespace().skip(1); - match words.next() { - Some("stop") => match words.next() { - Some(id) => { - self.slash_tool = Some(SlashToolRequest { - name: "stop_server".to_string(), - args: serde_json::json!({ "service_id": id }), - label: format!("services stop {id}"), - }); - } - None => { - self.chat - .push(ChatTurn::error("usage: /services stop ".to_string())); - } - }, - Some("restart") => { - self.chat.push(ChatTurn::error( - "services restart via chat is not supported yet; use /services stop then /serve " - .to_string(), - )); - } - Some(other) => { - self.chat.push(ChatTurn::error(format!( - "unknown /services action `{other}` (try stop, or /services to list)" - ))); - } - None => { - self.slash_tool = Some(SlashToolRequest { - name: "services".to_string(), - args: serde_json::json!({}), - label: "services".to_string(), - }); - } - } - } - // --- Group D-rest: lifecycle ops (read/mutate split via rocm_command) --- - // Read paths classify as ReadOnly in the bin and return a result turn; - // mutating paths classify as ApprovalRequired and open the Phase-4 modal. - "update" => { - // `/update` reports available updates (read-only); `/update --apply` - // applies them (approval-gated). Scan all tokens for the dash flag - // (matching `/uninstall`) so the trigger isn't position-sensitive. - let apply = rest.split_whitespace().skip(1).any(|tok| tok == "--apply"); - let argv: Vec<&str> = if apply { - vec!["update", "--apply"] - } else { - vec!["update"] - }; - self.slash_tool = Some(rocm_cmd_request( - &argv, - if apply { "update --apply" } else { "update" }, - )); - } - "comfyui" | "comfy" => { - // Bare `/comfyui` is read-only status. status/logs read; install/ - // start/stop mutate (approval-gated). - let sub = rest.split_whitespace().nth(1).map(str::to_lowercase); - match sub.as_deref() { - None | Some("status") => { - self.slash_tool = - Some(rocm_cmd_request(&["comfyui", "status"], "comfyui status")); - } - Some(action @ ("logs" | "install" | "start" | "stop")) => { - self.slash_tool = Some(rocm_cmd_request( - &["comfyui", action], - format!("comfyui {action}"), - )); - } - Some(other) => { - self.chat.push(ChatTurn::error(format!( - "unknown /comfyui action `{other}` (try status, logs, install, start, stop)" - ))); - } - } - } - "uninstall" => { - // SAFE default: bare `/uninstall` is a dry-run (read-only). A real - // uninstall needs `/uninstall --apply` and is approval-gated (the - // bin auto-adds --yes on approval). - let flags: Vec<&str> = rest.split_whitespace().skip(1).collect(); - let saw_apply = flags.contains(&"--apply"); - let saw_dry_run = flags.contains(&"--dry-run"); - if saw_apply && saw_dry_run { - self.chat.push(ChatTurn::error( - "conflicting /uninstall flags: choose either --dry-run (safe) or --apply (real uninstall)" - .to_string(), - )); - } else { - let real = saw_apply; - let argv: Vec<&str> = if real { - vec!["uninstall"] - } else { - vec!["uninstall", "--dry-run"] - }; - self.slash_tool = Some(rocm_cmd_request( - &argv, - if real { - "uninstall" - } else { - "uninstall --dry-run" - }, - )); - } - } - "setup" => { - // Bare `/setup` (or `/setup status`) reports first-time setup - // (read-only); `/setup reset` re-arms it (approval-gated). The CLI - // only has status + reset — anything else is guided, not run. - let sub = rest.split_whitespace().nth(1).map(str::to_lowercase); - match sub.as_deref() { - None | Some("status") => { - self.slash_tool = - Some(rocm_cmd_request(&["setup", "status"], "setup status")); - } - Some("reset") => { - self.slash_tool = - Some(rocm_cmd_request(&["setup", "reset"], "setup reset")); - } - Some(other) => { - self.chat.push(ChatTurn::error(format!( - "unknown /setup action `{other}` (try status or reset)" - ))); - } - } - } - // --- Group C: automations / reviews (read list; toggles + proposal - // actions are approval-gated via the Phase-4 modal) --- - "automations" => { - // Bare `/automations` (or `list`) lists configured automations - // (read-only). `enable`/`disable` toggle a watcher (approval-gated - // via the watcher_enable/watcher_disable mutating tools). - let mut words = rest.split_whitespace().skip(1); - match words.next().map(str::to_lowercase).as_deref() { - None | Some("list") => { - self.slash_tool = - Some(rocm_cmd_request(&["automations", "list"], "automations")); - } - Some("enable") => match words.next() { - Some(watcher) => { - // Optional `--mode `: when present, include it so the - // validator can accept observe|propose|contained. - let mode = match words.next() { - Some("--mode") => words.next(), - _ => None, - }; - let args = match mode { - Some(m) => { - serde_json::json!({ "watcher": watcher, "mode": m }) - } - None => serde_json::json!({ "watcher": watcher }), - }; - self.slash_tool = Some(SlashToolRequest { - name: "watcher_enable".to_string(), - args, - label: format!("automations enable {watcher}"), - }); - } - None => { - self.chat.push(ChatTurn::error( - "usage: /automations enable [--mode observe|propose|contained]" - .to_string(), - )); - } - }, - Some("disable") => match words.next() { - Some(watcher) => { - self.slash_tool = Some(SlashToolRequest { - name: "watcher_disable".to_string(), - args: serde_json::json!({ "watcher": watcher }), - label: format!("automations disable {watcher}"), - }); - } - None => { - self.chat.push(ChatTurn::error( - "usage: /automations disable ".to_string(), - )); - } - }, - Some(other) => { - self.chat.push(ChatTurn::error(format!( - "unknown /automations action `{other}` (try list, enable, disable)" - ))); - } - } - } - "reviews" => { - // Bare `/reviews` lists pending reviews (read-only, via the - // automations list). `/reviews ` shows one proposal's detail. - match rest.split_whitespace().nth(1) { - None => { - self.slash_tool = - Some(rocm_cmd_request(&["automations", "list"], "reviews")); - } - Some(id) => { - self.slash_tool = Some(SlashToolRequest { - name: "proposal_action".to_string(), - args: serde_json::json!({ "proposal_id": id, "action": "show" }), - label: format!("reviews {id}"), - }); - } - } - } - "approve" => match rest.split_whitespace().nth(1) { - Some(id) => { - self.slash_tool = Some(SlashToolRequest { - name: "proposal_action".to_string(), - args: serde_json::json!({ "proposal_id": id, "action": "approve" }), - label: format!("approve {id}"), - }); - } - None => { - self.chat - .push(ChatTurn::error("usage: /approve ".to_string())); - } - }, - "reject" => match rest.split_whitespace().nth(1) { - Some(id) => { - self.slash_tool = Some(SlashToolRequest { - name: "proposal_action".to_string(), - args: serde_json::json!({ "proposal_id": id, "action": "reject" }), - label: format!("reject {id}"), - }); - } - None => { - self.chat - .push(ChatTurn::error("usage: /reject ".to_string())); - } - }, - "edit" => match rest.split_whitespace().nth(1) { - Some(id) => { - // Editing a proposal's CONTENT isn't supported by the bin; show - // the proposal (read) so the operator can /approve or /reject. - self.slash_tool = Some(SlashToolRequest { - name: "proposal_action".to_string(), - args: serde_json::json!({ "proposal_id": id, "action": "show" }), - label: format!("edit {id}"), - }); - self.chat.push(ChatTurn::agent(format!( - "Editing a proposal's content isn't supported; showing {id}. Use /approve {id} or /reject {id}." - ))); - } - None => { - self.chat - .push(ChatTurn::error("usage: /edit ".to_string())); - } - }, - // --- Group E: permissions (read status; escalation is approval-gated) --- - "permissions" => { - // Bare `/permissions` (or `status`) shows the current mode - // (read-only via `config show`). `full-access`/`ask` change the - // mode — escalation MUST route through the approval modal. - let sub = rest.split_whitespace().nth(1).map(str::to_lowercase); - match sub.as_deref() { - None | Some("status") => { - self.slash_tool = - Some(rocm_cmd_request(&["config", "show"], "permissions")); - } - Some("full-access" | "full_access") => { - self.slash_tool = Some(rocm_cmd_request( - &["config", "set-permissions", "full_access"], - "permissions full-access", - )); - } - Some("ask") => { - self.slash_tool = Some(rocm_cmd_request( - &["config", "set-permissions", "ask"], - "permissions ask", - )); - } - Some(other) => { - self.chat.push(ChatTurn::error(format!( - "unknown /permissions action `{other}` (try status, full-access, ask)" - ))); - } - } - } - // --- Group F: natural-language planner (Ask→Plan→Review→Run) --- - // `/plan ` raises the `plan_request` edge; the event loop - // calls the read-only `natural_language_plan` tool off-thread and - // posts `PlanReady`. The plan is rendered for review; a complete - // mutating action is then handed to the Phase-4 approval modal. - "plan" => { - // Split off the command word (`plan`, any case) and take the - // free-form tail; case-insensitive, unlike `strip_prefix`. - let request = rest - .split_once(char::is_whitespace) - .map(|(_, tail)| tail.trim()) - .unwrap_or_default(); - if request.is_empty() { - self.chat.push(ChatTurn::agent( - "usage: /plan (e.g. /plan install rocm into /opt/rocm)" - .to_string(), - )); - } else { - self.plan_request = Some(request.to_string()); - } - } - // --- Group G: provider switch + chat entry (Phase 8) --- - // `/provider [local|openai|anthropic]` switches the live chat - // backend. A bare/unknown arg shows the current provider (or hints); - // a valid one raises the `provider_switch` edge for the event loop to - // rebuild the agent. Every backend calls the SAME ROCm tools. - "provider" => match rest.split_whitespace().nth(1) { - None => { - self.chat.push(ChatTurn::agent(format!( - "current provider: {} (usage: /provider [local|openai|anthropic])", - self.active_provider.label() - ))); - } - Some(arg) => match ChatProvider::parse(arg) { - Some(p) => { - self.active_provider = p; - self.provider_switch = Some(p); - } - None => { - self.chat.push(ChatTurn::error(format!( - "unknown provider `{arg}` (try local, openai, or anthropic)" - ))); - } - }, - }, - // `/chat [prompt]`: with a prompt, send it to the agent (passthrough, - // exactly as a plain line would); bare `/chat` focuses the Chat tab. - "chat" => { - let prompt = rest - .split_once(char::is_whitespace) - .map(|(_, tail)| tail.trim()) - .unwrap_or_default(); - if prompt.is_empty() { - self.active_tab = ActiveTab::Chat; - self.chat_focused = true; - } else if !self.chat_sending { - // Mirror `submit_chat`'s dispatch tail (the slash already - // consumed `chat_input`); guarded so an in-flight request is - // never double-spawned. - self.chat.push(ChatTurn::user(prompt.to_string())); - self.chat_sending = true; - self.chat_dispatch = true; - } - } - // Unknown slash command: an error turn, never sent to the LLM. - other => { - self.chat.push(ChatTurn::error(format!( - "unknown command: /{other} (try /help)" - ))); - } - } - SlashOutcome::Handled - } - /// Capture a read-only telemetry snapshot for the chat tools. Plain owned /// clones — tools read this without touching the reducer or `&AppState`. pub fn state_snapshot(&self) -> crate::agent::StateSnapshot { @@ -1499,66 +1044,6 @@ impl AppState { } } -/// OpenAI's hosted Chat Completions base URL (the standard, no-gateway case). -const OPENAI_BASE_URL: &str = "https://api.openai.com/v1"; -/// Default OpenAI model when none is configured via `chat_model`. -const OPENAI_DEFAULT_MODEL: &str = "gpt-5"; - -/// Construct the chat backend for an explicitly-selected provider (Phase 8). -/// -/// Construction only — NO network I/O happens here (rig clients defer the -/// request to `complete()`). Handles ONLY `Openai` and `Anthropic`; `Local` -/// returns `None` because its build (auto-detect probe → Rig/ChatGPT) is owned -/// by `event_loop`'s inline path and can't be reproduced from `ResolvedArgs` -/// alone. Keys come from `ResolvedArgs` (in-process seam), never argv. `None` -/// signals "couldn't build" (e.g. a missing key) so the caller surfaces an -/// actionable error turn instead of switching to a dead backend. -fn build_chat_agent( - provider: ChatProvider, - args: &ResolvedArgs, - executor: Option, - approval_tx: mpsc::UnboundedSender, -) -> Option> { - match provider { - // Local is rebuilt by the caller's inline path (it needs the live probe). - ChatProvider::Local => None, - ChatProvider::Openai => { - // Require a real key. Without this, `RigAgentClient::new` falls back to - // a dummy `sk-no-key` bearer and still builds, so the switch reports - // success and then 401s at request time. Returning `None` here makes - // the caller surface an actionable error and stay on the current - // backend instead of switching to a dead one. - let api_key = args.chat_api_key.clone().filter(|k| !k.trim().is_empty())?; - let cfg = crate::llm::LlmConfig { - base_url: OPENAI_BASE_URL.to_string(), - model: args - .chat_model - .clone() - .filter(|m| !m.is_empty()) - .unwrap_or_else(|| OPENAI_DEFAULT_MODEL.to_string()), - api_key: Some(api_key), - auth_header: None, - }; - crate::agent::RigAgentClient::new(cfg, executor, Some(approval_tx)) - .ok() - .map(|c| std::sync::Arc::new(c) as std::sync::Arc) - } - ChatProvider::Anthropic => { - // Leave base_url empty → the Anthropic backend uses rig's default - // host. model "" → CLAUDE_SONNET_4_6 (resolved inside the backend). - let cfg = crate::llm::LlmConfig { - base_url: String::new(), - model: args.chat_model.clone().unwrap_or_default(), - api_key: args.anthropic_api_key.clone(), - auth_header: None, - }; - crate::agent::AnthropicAgentClient::new(cfg, executor, Some(approval_tx)) - .ok() - .map(|c| std::sync::Arc::new(c) as std::sync::Arc) - } - } -} - pub async fn run(args: ResolvedArgs) -> color_eyre::Result<()> { enable_raw_mode()?; let mut stdout = io::stdout(); @@ -1642,26 +1127,44 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu )) as std::sync::Arc, ) } else { + // An endpoint we launched ourselves (managed-services registry) takes + // priority over the well-known default port — this is how a tool-launched + // engine on a non-default port (e.g. vLLM on :11435) is found. It does + // NOT override an explicitly configured `chat_url`/env URL, so config + // precedence is preserved (we only consult the registry when neither is + // set, i.e. where the well-known default would otherwise be probed). + let managed = if args.chat_url.is_none() && args.chat_env_url.is_none() { + detect_managed_chat(state.tool_executor.clone()).await + } else { + None + }; let probe_target = args .chat_url .clone() .or_else(|| args.chat_env_url.clone()) .unwrap_or_else(|| crate::llm::DEFAULT_CHAT_BASE_URL.to_string()); - let probe_ok = tokio::task::spawn_blocking(move || { - crate::llm::probe_endpoint(&probe_target, crate::llm::PROBE_TIMEOUT) - }) - .await - .unwrap_or(false); - let llm = crate::llm::resolve_llm_config( - args.chat_url.as_deref(), - args.chat_model.as_deref(), - None, - None, - args.chat_api_key.as_deref(), - args.chat_env_url.as_deref(), - args.chat_auth_header.as_deref(), - probe_ok, - ); + // A managed endpoint is already readiness-verified; otherwise TCP-probe. + let probe_ok = if managed.is_some() { + true + } else { + tokio::task::spawn_blocking(move || { + crate::llm::probe_endpoint(&probe_target, crate::llm::PROBE_TIMEOUT) + }) + .await + .unwrap_or(false) + }; + let llm = managed.or_else(|| { + crate::llm::resolve_llm_config( + args.chat_url.as_deref(), + args.chat_model.as_deref(), + None, + None, + args.chat_api_key.as_deref(), + args.chat_env_url.as_deref(), + args.chat_auth_header.as_deref(), + probe_ok, + ) + }); state.set_chat_config(llm, args.chat_auto_consent); // No reachable local endpoint AND no key/url configured → the no-key // ChatGPT OAuth default (device-code login surfaced in the chat tab). @@ -1823,8 +1326,8 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu ); crate::jobs::run_effects(fx, &job_tx); } - // The examine overlay, when open, owns all keys (read-only - // `rocm examine` job through the job-bridge). + // The doctor overlay, when open, owns all keys (read-only + // `rocm doctor` job through the job-bridge). Some(Ok(CtEvent::Key(k))) if state.examine_manager.is_some() => { let fx = crate::ui::examine_manager::on_key( &mut state.examine_manager, @@ -2037,7 +1540,7 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu // from `ResolvedArgs` keys (in-process seam, never argv). A build failure // (e.g. missing key) leaves `agent` unchanged and surfaces an actionable // error turn. Construction only — no network until the next submit. - if let Some(target) = state.provider_switch.take() { + if let Some(ProviderSwitch { previous, target }) = state.provider_switch.take() { match target { ChatProvider::Local => { // Restore the auto-detected local backend saved before the @@ -2058,13 +1561,15 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu .chat .push(ChatTurn::agent(format!("switched to {}", target.label()))); } else { - // Revert the optimistic `active_provider` set by the - // slash handler so the displayed provider stays honest, and - // reset the live `agent` to local too — otherwise a failed - // switch from a remote backend would leave requests routed - // to the old remote while the UI claims "local". - state.active_provider = ChatProvider::Local; - agent = local_agent.clone(); + // Revert the optimistic `active_provider` set by the slash + // handler back to the provider active BEFORE the switch + // attempt — not unconditionally Local — so the displayed + // provider stays honest (e.g. a failed openai→anthropic + // switch stays on openai). `agent` is never reassigned on a + // failed build, so it already matches `previous`; the two + // stay consistent (no stale-remote routing under a wrong + // label). + state.active_provider = previous; let hint = if target == ChatProvider::Anthropic { "anthropic requires ANTHROPIC_API_KEY in env or secure store" } else { @@ -2099,7 +1604,7 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu let _ = reply_tx.send(msg); }); } - None => state.on_chat_error("chat backend unavailable".to_string()), + None => state.on_chat_error(NO_CHAT_BACKEND_MSG.to_string()), } } @@ -2109,8 +1614,9 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu if state.chat_detect_dispatch { state.chat_detect_dispatch = false; let reply_tx = chat_tx.clone(); + let executor = state.tool_executor.clone(); tokio::spawn(async move { - let offer = detect_local_chat().await; + let offer = detect_local_chat(executor).await; let _ = reply_tx.send(ClientMsg::ChatDetectResult { offer }); }); } @@ -2136,50 +1642,6 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu Ok(()) } -/// Probe for a local engine (TCP), then query its `/v1/models` to choose a -/// model. Returns a ready-to-use [`LlmConfig`], or `None` when nothing is -/// reachable. All I/O lives here, outside the reducer. -async fn detect_local_chat() -> Option { - // TCP probe is blocking; keep it off the async reactor. - let base = tokio::task::spawn_blocking(crate::llm::detect_local_endpoint) - .await - .ok() - .flatten()?; - - // Best-effort model query; fall back to the neutral default on any failure. - let model = fetch_first_model(base) - .await - .unwrap_or_else(|| crate::llm::DEFAULT_CHAT_MODEL.to_string()); - Some(crate::llm::detected_llm_config(base, &model)) -} - -/// Persist an accepted local endpoint to the user's `config.toml`: load the -/// existing config (or defaults), set `tui.chat_url`/`tui.chat_model`, and write -/// it back. Best-effort — returns a human error string on failure. -/// -/// Uses [`default_config_path`] (a `--config` override is not honored by this -/// in-TUI save; that's a documented limitation). All I/O lives here. -fn persist_chat_endpoint(base_url: &str, model: &str) -> Result { - use rocm_dash_core::config::{Config, default_config_path}; - let path = default_config_path().ok_or_else(|| "no config path available".to_string())?; - let cfg = Config::load(&path).unwrap_or_default(); - let next = config_with_chat(cfg, base_url, model); - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent).map_err(|e| e.to_string())?; - } - let toml = toml::to_string_pretty(&next).map_err(|e| e.to_string())?; - std::fs::write(&path, toml).map_err(|e| e.to_string())?; - Ok(path) -} - -/// Render a CONCISE, human summary of a read-only slash-tool outcome — never a -/// raw JSON transcript dump (per docs/ux-guidelines.md). Errors and approval -/// notes are surfaced plainly; a success object is reduced to a short headline -/// plus a few key/value lines, with arrays/objects collapsed to counts. -/// Max object fields a slash-tool summary surfaces before truncating with an -/// ellipsis line. Keeps `summarize_json_value` output terse and length-bounded. -const SUMMARY_MAX_FIELDS: usize = 8; - /// Run an approved mutating action across the seam and render a concise summary /// (never a raw JSON dump). Sync + executor-generic so the approve path is /// unit-testable without tokio; the event loop calls it inside spawn_blocking. @@ -2207,127 +1669,6 @@ fn run_approved( } } -/// Parse the `natural_language_plan` tool result into the rendered plan text and -/// the structured next action. The bin returns -/// `structuredContent: { request, text, action: {...}|null }`; we surface the -/// concise rendered `text` (the review) and map `action` to [`PlannedAction`]. -/// Returns `None` only when the structured payload is missing/unusable. -fn parse_plan_result(v: &serde_json::Value) -> Option<(String, Option)> { - let text = v - .pointer("/structuredContent/text") - .and_then(serde_json::Value::as_str)? - .to_string(); - let action = v - .pointer("/structuredContent/action") - .filter(|a| a.is_object()) - .map(|a| PlannedAction { - args: a - .get("args") - .and_then(serde_json::Value::as_array) - .map(|arr| { - arr.iter() - .filter_map(|x| x.as_str().map(str::to_string)) - .collect() - }) - .unwrap_or_default(), - approval_required: a - .get("approval_required") - .and_then(serde_json::Value::as_bool) - .unwrap_or(false), - has_placeholders: a - .get("has_placeholders") - .and_then(serde_json::Value::as_bool) - .unwrap_or(false), - provider_assisted: a - .get("provider_assisted") - .and_then(serde_json::Value::as_bool) - .unwrap_or(false), - }); - Some((text, action)) -} - -fn summarize_slash_tool(label: &str, outcome: &crate::tool_exec::RocmToolOutcome) -> String { - use crate::tool_exec::RocmToolOutcome; - match outcome { - RocmToolOutcome::Error(e) => format!("/{label} failed: {e}"), - RocmToolOutcome::ApprovalRequired(_) => { - format!("/{label}: this action needs approval (read-only command did not expect it)") - } - RocmToolOutcome::Result(v) => { - let body = summarize_json_value(v); - if body.is_empty() { - format!("/{label}: done") - } else { - format!("/{label}:\n{body}") - } - } - } -} - -/// Collapse a JSON value into at most a handful of readable lines. Scalars print -/// inline; objects list their top-level fields (nested containers shown as -/// counts); arrays show a length. Keeps slash-tool output terse and scannable. -fn summarize_json_value(v: &serde_json::Value) -> String { - use serde_json::Value; - match v { - Value::Object(map) => { - let mut lines = Vec::new(); - for (k, val) in map.iter().take(SUMMARY_MAX_FIELDS) { - lines.push(format!(" {k}: {}", scalar_or_shape(val))); - } - if map.len() > SUMMARY_MAX_FIELDS { - lines.push(format!( - " … ({} more fields)", - map.len() - SUMMARY_MAX_FIELDS - )); - } - lines.join("\n") - } - Value::Array(arr) => format!(" {} item(s)", arr.len()), - other => format!(" {}", scalar_or_shape(other)), - } -} - -/// A scalar's plain text, or a shape hint (`{N fields}` / `[N items]`) for a -/// nested container — used so summaries never inline a whole subtree. -fn scalar_or_shape(v: &serde_json::Value) -> String { - use serde_json::Value; - match v { - Value::String(s) => s.clone(), - Value::Null => "null".to_string(), - Value::Bool(b) => b.to_string(), - Value::Number(n) => n.to_string(), - Value::Array(a) => format!("[{} items]", a.len()), - Value::Object(o) => format!("{{{} fields}}", o.len()), - } -} - -/// Pure immutable transform: return a copy of `cfg` with the chat endpoint set -/// to a local engine (base_url + model), clearing any gateway auth header since -/// local engines need none. -fn config_with_chat( - mut cfg: rocm_dash_core::config::Config, - base_url: &str, - model: &str, -) -> rocm_dash_core::config::Config { - cfg.tui.chat_url = Some(base_url.to_string()); - cfg.tui.chat_model = Some(model.to_string()); - cfg.tui.chat_auth_header = None; - cfg -} - -/// GET `{base}/models` and return the first served model id, or `None`. -async fn fetch_first_model(base_url: &str) -> Option { - let url = format!("{}/models", base_url.trim_end_matches('/')); - let client = reqwest::Client::builder() - .timeout(std::time::Duration::from_secs(3)) - .build() - .ok()?; - let resp = client.get(&url).send().await.ok()?; - let json: serde_json::Value = resp.json().await.ok()?; - crate::llm::pick_first_model(&json) -} - /// Apply a `KeyAction` to mutable state. Returns `true` when the action /// requests application exit (Quit). fn apply_action(state: &mut AppState, action: KeyAction) -> bool { @@ -2571,7 +1912,7 @@ pub enum KeyAction { OpenServeWizard, /// Open the engine-manager overlay (Phase 3 Wave 1). OpenEngineManager, - /// Open the examine overlay (Phase 3 Wave 2). + /// Open the doctor overlay (Phase 3 Wave 2). OpenExamine, /// Open the update overlay (Phase 3 Wave 2). OpenUpdate, @@ -2739,7 +2080,7 @@ fn handle_key(k: KeyEvent, current: ActiveTab, modal: &Modal, chat: ChatKeyCtx) KeyCode::Char('e') if matches!(current, ActiveTab::Overview | ActiveTab::Instances) => { KeyAction::OpenEngineManager } - // Examine: read-only environment check. + // Doctor: read-only environment check. KeyCode::Char('d') if matches!(current, ActiveTab::Overview | ActiveTab::Instances) => { KeyAction::OpenExamine } @@ -2976,7 +2317,7 @@ mod tests { KeyAction::Nothing ); assert_eq!(hk(KeyCode::Char('e'), ActiveTab::Bench), KeyAction::Nothing); - // Examine / update open from Overview + Instances. + // Doctor / update open from Overview + Instances. assert_eq!( hk(KeyCode::Char('d'), ActiveTab::Overview), KeyAction::OpenExamine @@ -3478,7 +2819,7 @@ mod tests { fn config_with_chat_sets_local_endpoint_and_clears_auth() { let mut cfg = rocm_dash_core::config::Config::default(); cfg.tui.chat_auth_header = Some("Ocp-Apim-Subscription-Key".into()); - let next = config_with_chat(cfg, "http://localhost:8000/v1", "qwen"); + let next = super::chat::config_with_chat(cfg, "http://localhost:8000/v1", "qwen"); assert_eq!( next.tui.chat_url.as_deref(), Some("http://localhost:8000/v1") @@ -3798,17 +3139,80 @@ mod tests { SlashOutcome::Handled ); assert_eq!(s.active_provider, ChatProvider::Anthropic); - assert_eq!(s.provider_switch, Some(ChatProvider::Anthropic)); + assert_eq!( + s.provider_switch, + Some(ProviderSwitch { + previous: ChatProvider::Local, + target: ChatProvider::Anthropic, + }) + ); // /provider openai → openai. let mut s2 = st(); s2.handle_slash_command("/provider openai"); assert_eq!(s2.active_provider, ChatProvider::Openai); - assert_eq!(s2.provider_switch, Some(ChatProvider::Openai)); + assert_eq!( + s2.provider_switch, + Some(ProviderSwitch { + previous: ChatProvider::Local, + target: ChatProvider::Openai, + }) + ); // /provider local → local (matched case-insensitively). let mut s3 = st(); s3.handle_slash_command("/Provider LOCAL"); assert_eq!(s3.active_provider, ChatProvider::Local); - assert_eq!(s3.provider_switch, Some(ChatProvider::Local)); + assert_eq!( + s3.provider_switch, + Some(ProviderSwitch { + previous: ChatProvider::Local, + target: ChatProvider::Local, + }) + ); + } + + #[test] + fn slash_provider_switch_captures_previous_provider() { + // (Phase-8 polish) A failed switch must revert to the provider that was + // active BEFORE the attempt, not unconditionally to Local. Prove the + // slash handler snapshots the prior provider in the edge: switch to + // openai (optimistic), then attempt anthropic — the edge carries + // previous=Openai so the drain can revert there on a build failure. + let mut s = st(); + s.handle_slash_command("/provider openai"); + assert_eq!(s.active_provider, ChatProvider::Openai); + s.handle_slash_command("/provider anthropic"); + assert_eq!( + s.provider_switch, + Some(ProviderSwitch { + previous: ChatProvider::Openai, + target: ChatProvider::Anthropic, + }), + "the failed-switch revert target is the prior provider, not Local" + ); + } + + #[test] + fn no_provider_no_key_chat_surfaces_actionable_message() { + // Edge: agent is None (no endpoint, no provider key). Submitting chat + // must surface a clear, ACTIONABLE message (the recovery affordances), + // routed through `on_chat_error` as an error turn — not an error dump, + // not a panic. This mirrors the event-loop None-agent branch, which + // emits exactly `NO_CHAT_BACKEND_MSG`. + let mut s = AppState::new("t".into(), "default-dark".into()); + s.set_chat_config(None, false); + assert_eq!(s.chat_consent, ChatConsent::Unavailable); + // Drive the same surface the event loop uses for the None-agent case. + s.on_chat_error(NO_CHAT_BACKEND_MSG.to_string()); + let last = s.chat.last().expect("an error turn was pushed"); + assert_eq!(last.role, ChatRole::Error); + // Actionable: names both concrete recovery paths. + assert!( + last.content.contains("detect") && last.content.contains("/provider"), + "empty-state must be actionable, got: {}", + last.content + ); + // Not a panic and not in-flight afterwards (sending cleared). + assert!(!s.chat_sending); } #[test] @@ -4671,6 +4075,48 @@ mod tests { ); } + #[test] + fn approval_modal_escape_is_not_a_focus_trap() { + // Edge: the approval modal must be escapable — Esc and 'n' both yield a + // closing verdict, and routing that verdict through the deny/cancel path + // clears the modal (`approval` → None) without executing. The covered + // active tab is preserved across open → escape (the modal overlays it). + for key in [ + crossterm::event::KeyCode::Esc, + crossterm::event::KeyCode::Char('n'), + ] { + let mut s = st(); + s.active_tab = ActiveTab::Hardware; + s.open_approval(crate::tool_exec::ApprovalIntent { + title: "T".to_string(), + body: vec!["cmd".to_string()], + name: "stop_server".to_string(), + arguments: serde_json::json!({ "service_id": "x" }), + }); + assert!(s.approval.is_some(), "modal open before escape"); + let verdict = s.on_approval_key(key); + // Esc → Cancel, 'n' → Deny; both are closing (non-Approve) verdicts. + assert!( + matches!( + verdict, + Some( + crate::ui::approval::ApprovalVerdict::Cancel + | crate::ui::approval::ApprovalVerdict::Deny + ) + ), + "key {key:?} must yield a closing verdict, got {verdict:?}" + ); + // The event loop routes Deny|Cancel through on_approval_declined. + s.on_approval_declined(); + assert!( + s.approval.is_none(), + "escape must clear the modal (no focus trap) for {key:?}" + ); + // The covered tab is preserved — the modal never navigated away. + assert_eq!(s.active_tab, ActiveTab::Hardware); + } + } + #[test] fn approval_result_fires_exactly_one_follow_up_no_loop() { // (c) exactly one follow-up: on_approval_result appends the result turn diff --git a/crates/rocm-dash-tui/src/app/slash.rs b/crates/rocm-dash-tui/src/app/slash.rs new file mode 100644 index 00000000..44b3e6e9 --- /dev/null +++ b/crates/rocm-dash-tui/src/app/slash.rs @@ -0,0 +1,501 @@ +// Copyright Advanced Micro Devices, Inc. +// +// SPDX-License-Identifier: MIT + +//! Slash-command routing for the chat input. +//! +//! [`AppState::handle_slash_command`] classifies a `/`-prefixed chat line and +//! either mutates reducer state directly (nav / overlays), raises a one-shot +//! executor edge (`slash_tool` / `plan_request` / `provider_switch`) for the +//! event loop to drain off-thread, or pushes a usage/error turn. Stays I/O-free. +//! Split out of `app/mod.rs` to keep the core reducer focused; the slash-command +//! payload types it raises (`SlashOutcome`, `SlashToolRequest`, `ProviderSwitch`) +//! stay in `mod.rs` alongside the `AppState` fields that carry them. + +use super::{ + ActiveTab, AppState, ChatProvider, ChatTurn, Modal, ProviderSwitch, SlashOutcome, + SlashToolRequest, +}; + +impl AppState { + /// Route a chat-input line that may be a slash command. Returns + /// [`SlashOutcome::NotCommand`] for plain text (caller dispatches to the + /// agent); otherwise handles it in-reducer and returns + /// [`SlashOutcome::Handled`]. Stays I/O-free: executor-backed commands raise + /// the `slash_tool` edge for the event loop to drain off-thread. + pub(crate) fn handle_slash_command(&mut self, text: &str) -> SlashOutcome { + /// Build a `rocm_command` slash-tool request from an argv slice and a + /// chat-turn label. Centralizes the repeated `name: "rocm_command"` + + /// `{"args": argv}` construction shared by the lifecycle read/mutate arms. + fn rocm_cmd_request(argv: &[&str], label: impl Into) -> SlashToolRequest { + SlashToolRequest { + name: "rocm_command".to_string(), + args: serde_json::json!({ "args": argv }), + label: label.into(), + } + } + + let trimmed = text.trim(); + let Some(rest) = trimmed.strip_prefix('/') else { + return SlashOutcome::NotCommand; + }; + // First whitespace-delimited word after '/', lowercased. + let cmd = rest.split_whitespace().next().unwrap_or("").to_lowercase(); + + match cmd.as_str() { + // --- Group A: nav / session (deterministic, no executor) --- + "home" => self.active_tab = ActiveTab::Overview, + "gpu" => self.active_tab = ActiveTab::Hardware, + "help" | "?" => self.modal = Modal::Help, + "clear" => self.chat.clear(), + "quit" | "exit" => self.should_quit = true, + // --- Group B: read-only overlays (mirror the keybind handlers) --- + "doctor" => { + self.close_overlays(); + self.examine_manager = + Some(crate::ui::examine_manager::ExamineManagerState::default()); + } + "runtimes" => { + self.close_overlays(); + self.runtime_manager = + Some(crate::ui::runtime_manager::RuntimeManagerState::default()); + } + "config" => { + self.close_overlays(); + self.config_manager = + Some(crate::ui::config_manager::ConfigManagerState::default()); + } + "logs" => { + self.close_overlays(); + self.logs_view = Some(crate::ui::logs_view::LogsViewState::default()); + } + // --- Group B: read-only executor-backed (no overlay; off-thread) --- + "model" => { + self.slash_tool = Some(rocm_cmd_request(&["model"], "model")); + } + "daemon" => { + self.slash_tool = Some(rocm_cmd_request(&["daemon", "status"], "daemon status")); + } + // --- Group D: mutating ops (approval-gated; surfaced via the modal) --- + // Each raises a `slash_tool` request whose `execute()` returns + // `ApprovalRequired`; the event loop opens the approval modal. The + // safety validators run inside `execute()` (and again on the approved + // replay), so an unsafe call surfaces an error instead of the modal. + "install" => { + // `/install ` — the install folder is required by the + // validator (it never installs to a system path). + match rest.split_whitespace().nth(1) { + Some(prefix) => { + self.slash_tool = Some(SlashToolRequest { + name: "install_sdk".to_string(), + args: serde_json::json!({ + "channel": "release", + "format": "wheel", + "prefix": prefix, + }), + label: format!("install {prefix}"), + }); + } + None => { + self.chat.push(ChatTurn::error( + "usage: /install (install folder, e.g. /install ~/rocm)" + .to_string(), + )); + } + } + } + "engine" => { + // `/engine ` — the engine name is required by the validator. + match rest.split_whitespace().nth(1) { + Some(engine) => { + self.slash_tool = Some(SlashToolRequest { + name: "install_engine".to_string(), + args: serde_json::json!({ "engine": engine }), + label: format!("engine {engine}"), + }); + } + None => { + self.chat.push(ChatTurn::error( + "usage: /engine (e.g. /engine vllm)".to_string(), + )); + } + } + } + "serve" => { + // `/serve ` — loopback host only (validator rejects public). + match rest.split_whitespace().nth(1) { + Some(model) => { + self.slash_tool = Some(SlashToolRequest { + name: "launch_server".to_string(), + args: serde_json::json!({ "model": model, "host": "127.0.0.1" }), + label: format!("serve {model}"), + }); + } + None => { + self.chat.push(ChatTurn::error( + "usage: /serve (e.g. /serve deepseek-r1)".to_string(), + )); + } + } + } + "services" => { + // `/services stop ` mutates (stop_server, approval-gated); a + // bare `/services` is read-only and lists managed services. + // `restart` is NOT yet wired through the chat seam, so it is + // guided rather than silently running stop (a semantic lie). + let mut words = rest.split_whitespace().skip(1); + match words.next() { + Some("stop") => match words.next() { + Some(id) => { + self.slash_tool = Some(SlashToolRequest { + name: "stop_server".to_string(), + args: serde_json::json!({ "service_id": id }), + label: format!("services stop {id}"), + }); + } + None => { + self.chat + .push(ChatTurn::error("usage: /services stop ".to_string())); + } + }, + Some("restart") => { + self.chat.push(ChatTurn::error( + "services restart via chat is not supported yet; use /services stop then /serve " + .to_string(), + )); + } + Some(other) => { + self.chat.push(ChatTurn::error(format!( + "unknown /services action `{other}` (try stop, or /services to list)" + ))); + } + None => { + self.slash_tool = Some(SlashToolRequest { + name: "services".to_string(), + args: serde_json::json!({}), + label: "services".to_string(), + }); + } + } + } + // --- Group D-rest: lifecycle ops (read/mutate split via rocm_command) --- + // Read paths classify as ReadOnly in the bin and return a result turn; + // mutating paths classify as ApprovalRequired and open the Phase-4 modal. + "update" => { + // `/update` reports available updates (read-only); `/update --apply` + // applies them (approval-gated). Scan all tokens for the dash flag + // (matching `/uninstall`) so the trigger isn't position-sensitive. + let apply = rest.split_whitespace().skip(1).any(|tok| tok == "--apply"); + let argv: Vec<&str> = if apply { + vec!["update", "--apply"] + } else { + vec!["update"] + }; + self.slash_tool = Some(rocm_cmd_request( + &argv, + if apply { "update --apply" } else { "update" }, + )); + } + "comfyui" | "comfy" => { + // Bare `/comfyui` is read-only status. status/logs read; install/ + // start/stop mutate (approval-gated). + let sub = rest.split_whitespace().nth(1).map(str::to_lowercase); + match sub.as_deref() { + None | Some("status") => { + self.slash_tool = + Some(rocm_cmd_request(&["comfyui", "status"], "comfyui status")); + } + Some(action @ ("logs" | "install" | "start" | "stop")) => { + self.slash_tool = Some(rocm_cmd_request( + &["comfyui", action], + format!("comfyui {action}"), + )); + } + Some(other) => { + self.chat.push(ChatTurn::error(format!( + "unknown /comfyui action `{other}` (try status, logs, install, start, stop)" + ))); + } + } + } + "uninstall" => { + // SAFE default: bare `/uninstall` is a dry-run (read-only). A real + // uninstall needs `/uninstall --apply` and is approval-gated (the + // bin auto-adds --yes on approval). + let flags: Vec<&str> = rest.split_whitespace().skip(1).collect(); + let saw_apply = flags.contains(&"--apply"); + let saw_dry_run = flags.contains(&"--dry-run"); + if saw_apply && saw_dry_run { + self.chat.push(ChatTurn::error( + "conflicting /uninstall flags: choose either --dry-run (safe) or --apply (real uninstall)" + .to_string(), + )); + } else { + let real = saw_apply; + let argv: Vec<&str> = if real { + vec!["uninstall"] + } else { + vec!["uninstall", "--dry-run"] + }; + self.slash_tool = Some(rocm_cmd_request( + &argv, + if real { + "uninstall" + } else { + "uninstall --dry-run" + }, + )); + } + } + "setup" => { + // Bare `/setup` (or `/setup status`) reports first-time setup + // (read-only); `/setup reset` re-arms it (approval-gated). The CLI + // only has status + reset — anything else is guided, not run. + let sub = rest.split_whitespace().nth(1).map(str::to_lowercase); + match sub.as_deref() { + None | Some("status") => { + self.slash_tool = + Some(rocm_cmd_request(&["setup", "status"], "setup status")); + } + Some("reset") => { + self.slash_tool = + Some(rocm_cmd_request(&["setup", "reset"], "setup reset")); + } + Some(other) => { + self.chat.push(ChatTurn::error(format!( + "unknown /setup action `{other}` (try status or reset)" + ))); + } + } + } + // --- Group C: automations / reviews (read list; toggles + proposal + // actions are approval-gated via the Phase-4 modal) --- + "automations" => { + // Bare `/automations` (or `list`) lists configured automations + // (read-only). `enable`/`disable` toggle a watcher (approval-gated + // via the watcher_enable/watcher_disable mutating tools). + let mut words = rest.split_whitespace().skip(1); + match words.next().map(str::to_lowercase).as_deref() { + None | Some("list") => { + self.slash_tool = + Some(rocm_cmd_request(&["automations", "list"], "automations")); + } + Some("enable") => match words.next() { + Some(watcher) => { + // Optional `--mode `: when present, include it so the + // validator can accept observe|propose|contained. + let mode = match words.next() { + Some("--mode") => words.next(), + _ => None, + }; + let args = match mode { + Some(m) => { + serde_json::json!({ "watcher": watcher, "mode": m }) + } + None => serde_json::json!({ "watcher": watcher }), + }; + self.slash_tool = Some(SlashToolRequest { + name: "watcher_enable".to_string(), + args, + label: format!("automations enable {watcher}"), + }); + } + None => { + self.chat.push(ChatTurn::error( + "usage: /automations enable [--mode observe|propose|contained]" + .to_string(), + )); + } + }, + Some("disable") => match words.next() { + Some(watcher) => { + self.slash_tool = Some(SlashToolRequest { + name: "watcher_disable".to_string(), + args: serde_json::json!({ "watcher": watcher }), + label: format!("automations disable {watcher}"), + }); + } + None => { + self.chat.push(ChatTurn::error( + "usage: /automations disable ".to_string(), + )); + } + }, + Some(other) => { + self.chat.push(ChatTurn::error(format!( + "unknown /automations action `{other}` (try list, enable, disable)" + ))); + } + } + } + "reviews" => { + // Bare `/reviews` lists pending reviews (read-only, via the + // automations list). `/reviews ` shows one proposal's detail. + match rest.split_whitespace().nth(1) { + None => { + self.slash_tool = + Some(rocm_cmd_request(&["automations", "list"], "reviews")); + } + Some(id) => { + self.slash_tool = Some(SlashToolRequest { + name: "proposal_action".to_string(), + args: serde_json::json!({ "proposal_id": id, "action": "show" }), + label: format!("reviews {id}"), + }); + } + } + } + "approve" => match rest.split_whitespace().nth(1) { + Some(id) => { + self.slash_tool = Some(SlashToolRequest { + name: "proposal_action".to_string(), + args: serde_json::json!({ "proposal_id": id, "action": "approve" }), + label: format!("approve {id}"), + }); + } + None => { + self.chat + .push(ChatTurn::error("usage: /approve ".to_string())); + } + }, + "reject" => match rest.split_whitespace().nth(1) { + Some(id) => { + self.slash_tool = Some(SlashToolRequest { + name: "proposal_action".to_string(), + args: serde_json::json!({ "proposal_id": id, "action": "reject" }), + label: format!("reject {id}"), + }); + } + None => { + self.chat + .push(ChatTurn::error("usage: /reject ".to_string())); + } + }, + "edit" => match rest.split_whitespace().nth(1) { + Some(id) => { + // Editing a proposal's CONTENT isn't supported by the bin; show + // the proposal (read) so the operator can /approve or /reject. + self.slash_tool = Some(SlashToolRequest { + name: "proposal_action".to_string(), + args: serde_json::json!({ "proposal_id": id, "action": "show" }), + label: format!("review {id}"), + }); + self.chat.push(ChatTurn::agent(format!( + "Editing a proposal's content isn't supported; showing {id}. Use /approve {id} or /reject {id}." + ))); + } + None => { + self.chat + .push(ChatTurn::error("usage: /edit ".to_string())); + } + }, + // --- Group E: permissions (read status; escalation is approval-gated) --- + "permissions" => { + // Bare `/permissions` (or `status`) shows the current mode + // (read-only via `config show`). `full-access`/`ask` change the + // mode — escalation MUST route through the approval modal. + let sub = rest.split_whitespace().nth(1).map(str::to_lowercase); + match sub.as_deref() { + None | Some("status") => { + self.slash_tool = + Some(rocm_cmd_request(&["config", "show"], "permissions")); + } + Some("full-access" | "full_access") => { + self.slash_tool = Some(rocm_cmd_request( + &["config", "set-permissions", "full_access"], + "permissions full-access", + )); + } + Some("ask") => { + self.slash_tool = Some(rocm_cmd_request( + &["config", "set-permissions", "ask"], + "permissions ask", + )); + } + Some(other) => { + self.chat.push(ChatTurn::error(format!( + "unknown /permissions action `{other}` (try status, full-access, ask)" + ))); + } + } + } + // --- Group F: natural-language planner (Ask→Plan→Review→Run) --- + // `/plan ` raises the `plan_request` edge; the event loop + // calls the read-only `natural_language_plan` tool off-thread and + // posts `PlanReady`. The plan is rendered for review; a complete + // mutating action is then handed to the Phase-4 approval modal. + "plan" => { + // Split off the command word (`plan`, any case) and take the + // free-form tail; case-insensitive, unlike `strip_prefix`. + let request = rest + .split_once(char::is_whitespace) + .map(|(_, tail)| tail.trim()) + .unwrap_or_default(); + if request.is_empty() { + self.chat.push(ChatTurn::agent( + "usage: /plan (e.g. /plan install rocm into /opt/rocm)" + .to_string(), + )); + } else { + self.plan_request = Some(request.to_string()); + } + } + // --- Group G: provider switch + chat entry (Phase 8) --- + // `/provider [local|openai|anthropic]` switches the live chat + // backend. A bare/unknown arg shows the current provider (or hints); + // a valid one raises the `provider_switch` edge for the event loop to + // rebuild the agent. Every backend calls the SAME ROCm tools. + "provider" => match rest.split_whitespace().nth(1) { + None => { + self.chat.push(ChatTurn::agent(format!( + "current provider: {} (usage: /provider [local|openai|anthropic])", + self.active_provider.label() + ))); + } + Some(arg) => match ChatProvider::parse(arg) { + Some(p) => { + // Snapshot the prior provider BEFORE the optimistic set so + // a failed switch (missing key) can revert to it. + let previous = self.active_provider; + self.active_provider = p; + self.provider_switch = Some(ProviderSwitch { + previous, + target: p, + }); + } + None => { + self.chat.push(ChatTurn::error(format!( + "unknown provider `{arg}` (try local, openai, or anthropic)" + ))); + } + }, + }, + // `/chat [prompt]`: with a prompt, send it to the agent (passthrough, + // exactly as a plain line would); bare `/chat` focuses the Chat tab. + "chat" => { + let prompt = rest + .split_once(char::is_whitespace) + .map(|(_, tail)| tail.trim()) + .unwrap_or_default(); + if prompt.is_empty() { + self.active_tab = ActiveTab::Chat; + self.chat_focused = true; + } else if !self.chat_sending { + // Mirror `submit_chat`'s dispatch tail (the slash already + // consumed `chat_input`); guarded so an in-flight request is + // never double-spawned. + self.chat.push(ChatTurn::user(prompt.to_string())); + self.chat_sending = true; + self.chat_dispatch = true; + } + } + // Unknown slash command: an error turn, never sent to the LLM. + other => { + self.chat.push(ChatTurn::error(format!( + "unknown command: /{other} (try /help)" + ))); + } + } + SlashOutcome::Handled + } +} diff --git a/crates/rocm-dash-tui/src/app/summary.rs b/crates/rocm-dash-tui/src/app/summary.rs new file mode 100644 index 00000000..ea870b42 --- /dev/null +++ b/crates/rocm-dash-tui/src/app/summary.rs @@ -0,0 +1,119 @@ +// Copyright Advanced Micro Devices, Inc. +// +// SPDX-License-Identifier: MIT + +//! Pure display utilities for slash-tool outcomes and natural-language plans. +//! +//! Concise, length-bounded summaries of read-only slash-tool results (never a +//! raw JSON transcript dump, per docs/ux-guidelines.md) plus the parser that +//! maps the `natural_language_plan` tool result into a [`PlannedAction`]. All +//! free functions — no `AppState` access — split out of `app/mod.rs` to keep the +//! core reducer + event loop focused. + +use super::PlannedAction; + +/// Max object fields a slash-tool summary surfaces before truncating with an +/// ellipsis line. Keeps `summarize_json_value` output terse and length-bounded. +pub(super) const SUMMARY_MAX_FIELDS: usize = 8; + +/// Parse the `natural_language_plan` tool result into the rendered plan text and +/// the structured next action. The bin returns +/// `structuredContent: { request, text, action: {...}|null }`; we surface the +/// concise rendered `text` (the review) and map `action` to [`PlannedAction`]. +/// Returns `None` only when the structured payload is missing/unusable. +pub(super) fn parse_plan_result(v: &serde_json::Value) -> Option<(String, Option)> { + let text = v + .pointer("/structuredContent/text") + .and_then(serde_json::Value::as_str)? + .to_string(); + let action = v + .pointer("/structuredContent/action") + .filter(|a| a.is_object()) + .map(|a| PlannedAction { + args: a + .get("args") + .and_then(serde_json::Value::as_array) + .map(|arr| { + arr.iter() + .filter_map(|x| x.as_str().map(str::to_string)) + .collect() + }) + .unwrap_or_default(), + approval_required: a + .get("approval_required") + .and_then(serde_json::Value::as_bool) + .unwrap_or(false), + has_placeholders: a + .get("has_placeholders") + .and_then(serde_json::Value::as_bool) + .unwrap_or(false), + provider_assisted: a + .get("provider_assisted") + .and_then(serde_json::Value::as_bool) + .unwrap_or(false), + }); + Some((text, action)) +} + +/// Render a CONCISE, human summary of a read-only slash-tool outcome — never a +/// raw JSON transcript dump (per docs/ux-guidelines.md). Errors and approval +/// notes are surfaced plainly; a success object is reduced to a short headline +/// plus a few key/value lines, with arrays/objects collapsed to counts. +pub(super) fn summarize_slash_tool( + label: &str, + outcome: &crate::tool_exec::RocmToolOutcome, +) -> String { + use crate::tool_exec::RocmToolOutcome; + match outcome { + RocmToolOutcome::Error(e) => format!("/{label} failed: {e}"), + RocmToolOutcome::ApprovalRequired(_) => { + format!("/{label}: this action needs approval (read-only command did not expect it)") + } + RocmToolOutcome::Result(v) => { + let body = summarize_json_value(v); + if body.is_empty() { + format!("/{label}: done") + } else { + format!("/{label}:\n{body}") + } + } + } +} + +/// Collapse a JSON value into at most a handful of readable lines. Scalars print +/// inline; objects list their top-level fields (nested containers shown as +/// counts); arrays show a length. Keeps slash-tool output terse and scannable. +pub(super) fn summarize_json_value(v: &serde_json::Value) -> String { + use serde_json::Value; + match v { + Value::Object(map) => { + let mut lines = Vec::new(); + for (k, val) in map.iter().take(SUMMARY_MAX_FIELDS) { + lines.push(format!(" {k}: {}", scalar_or_shape(val))); + } + if map.len() > SUMMARY_MAX_FIELDS { + lines.push(format!( + " … ({} more fields)", + map.len() - SUMMARY_MAX_FIELDS + )); + } + lines.join("\n") + } + Value::Array(arr) => format!(" {} item(s)", arr.len()), + other => format!(" {}", scalar_or_shape(other)), + } +} + +/// A scalar's plain text, or a shape hint (`{N fields}` / `[N items]`) for a +/// nested container — used so summaries never inline a whole subtree. +pub(super) fn scalar_or_shape(v: &serde_json::Value) -> String { + use serde_json::Value; + match v { + Value::String(s) => s.clone(), + Value::Null => "null".to_string(), + Value::Bool(b) => b.to_string(), + Value::Number(n) => n.to_string(), + Value::Array(a) => format!("[{} items]", a.len()), + Value::Object(o) => format!("{{{} fields}}", o.len()), + } +}