diff --git a/apps/rocm/src/dash_seam.rs b/apps/rocm/src/dash_seam.rs index b9a5c577..472b2dc7 100644 --- a/apps/rocm/src/dash_seam.rs +++ b/apps/rocm/src/dash_seam.rs @@ -53,12 +53,25 @@ impl RocmToolExecutor for BinToolExecutor { } b }, - args: req.args, + name: name.to_owned(), + arguments: args.clone(), }), Err(e) => RocmToolOutcome::Error(e.to_string()), } } } + + fn execute_approved(&self, name: &str, args: &serde_json::Value) -> RocmToolOutcome { + // Replay the approved mutating call via the captured-subprocess path + // (`allow_mutation = true`). `run_internal_mcp_call` re-validates the + // call first, so the safety validators stay the single gate; it runs the + // action with piped stdout/stderr (TUI-safe) — no `dispatch`, no stdout + // corruption. + match crate::run_internal_mcp_call(&self.paths, name, args.clone(), true) { + Ok(v) => RocmToolOutcome::Result(v), + Err(e) => RocmToolOutcome::Error(e.to_string()), + } + } } #[cfg(test)] @@ -118,14 +131,62 @@ mod tests { ); match outcome { RocmToolOutcome::ApprovalRequired(intent) => { + assert_eq!( + intent.name, "install_sdk", + "approval intent carries the tool name for re-execution" + ); assert!( - intent.args.len() >= 2 - && intent.args[..2] == ["install".to_owned(), "sdk".to_owned()], - "install_sdk approval args should start with [install, sdk], got: {:?}", - intent.args + !intent.body.is_empty(), + "approval body should carry human-readable lines, got: {:?}", + intent.body ); + // The replayable payload is the same args object we passed in. + assert_eq!(intent.arguments["channel"], "release"); } other => panic!("expected ApprovalRequired for `install_sdk`, got {other:?}"), } } + + #[test] + fn seam_execute_rejects_public_bind_before_approval() { + // (d) an UNSAFE mutating call fails validation in execute() → Error, NOT + // ApprovalRequired. The approval modal never opens for a rejected call. + let exec = BinToolExecutor::new(temp_paths()); + let outcome = exec.execute( + "launch_server", + &serde_json::json!({ "model": "m", "host": "0.0.0.0" }), + ); + assert!( + matches!(outcome, RocmToolOutcome::Error(_)), + "public-bind launch_server must be rejected, got {outcome:?}" + ); + } + + #[test] + fn seam_execute_rejects_cpu_device_before_approval() { + let exec = BinToolExecutor::new(temp_paths()); + let outcome = exec.execute( + "launch_server", + &serde_json::json!({ "model": "m", "host": "127.0.0.1", "device": "cpu" }), + ); + assert!( + matches!(outcome, RocmToolOutcome::Error(_)), + "CPU-device launch_server must be rejected, got {outcome:?}" + ); + } + + #[test] + fn seam_execute_approved_rejects_unsafe_call_via_validator() { + // execute_approved re-validates: a public-bind launch_server is rejected + // (the validators stay the single gate even on the approved path). + let exec = BinToolExecutor::new(temp_paths()); + let outcome = exec.execute_approved( + "launch_server", + &serde_json::json!({ "model": "m", "host": "0.0.0.0" }), + ); + assert!( + matches!(outcome, RocmToolOutcome::Error(_)), + "approved-path execution must still reject an unsafe call, got {outcome:?}" + ); + } } diff --git a/crates/rocm-dash-tui/src/agent.rs b/crates/rocm-dash-tui/src/agent.rs index f2e8e9cd..a4b9ab73 100644 --- a/crates/rocm-dash-tui/src/agent.rs +++ b/crates/rocm-dash-tui/src/agent.rs @@ -29,9 +29,12 @@ use rocm_dash_core::bench_schema::BenchmarkRow; use rocm_dash_core::metrics::{GpuMetrics, Instance, Snapshot}; use crate::app::{ChatRole, ChatTurn}; +use crate::client::ClientMsg; use crate::llm::LlmConfig; use crate::tool_exec::{RocmToolOutcome, SharedRocmToolExecutor}; +use tokio::sync::mpsc::UnboundedSender; + /// One-shot request budget. A hung backend becomes a timeout error turn, never /// a frozen pane. pub const REQUEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(45); @@ -628,6 +631,178 @@ pub const ROCM_READ_TOOL_NAMES: [&str; 12] = [ RocmCommandRocmTool::NAME, ]; +// --------------------------------------------------------------------------- +// Mutating ROCm tools (group D, Phase 4). These do NOT execute inside the rig +// tool loop. `execute()` returns `ApprovalRequired(intent)` (a descriptor that +// the bin's validators already accepted); the tool posts the intent to the app +// via `approval_tx` (a `ClientMsg::ChatApprovalRequired`) and returns a +// "surfaced for approval" note to the model. The actual action runs only after +// the operator approves the modal, via `execute_approved` off the event loop. +// agent.rs stays the sole `rig` namer; the seam types are plain data. +// --------------------------------------------------------------------------- + +/// Declare one mutating ROCm tool type. Mirrors [`rocm_read_tool!`] but its +/// `call()` surfaces the approval intent rather than executing: on +/// `ApprovalRequired` it forwards the descriptor over `approval_tx` and returns +/// a terse "surfaced" note (no execution, no retry); on `Error` it returns the +/// validator error; a `Result` (shouldn't happen for a mutating tool, handled +/// defensively) is passed through. Args are raw JSON (the bin validates them). +macro_rules! rocm_mutating_tool { + ($ty:ident, $name:literal, $desc:literal, $params:tt) => { + pub struct $ty { + pub executor: Option, + pub approval_tx: Option>, + pub fired: FiredLog, + } + impl Tool for $ty { + const NAME: &'static str = $name; + type Error = ToolError; + type Args = serde_json::Value; + type Output = Value; + async fn definition(&self, _p: String) -> ToolDefinition { + ToolDefinition { + name: $name.to_string(), + description: $desc.to_string(), + parameters: json!($params), + } + } + async fn call(&self, args: Self::Args) -> Result { + record(&self.fired, $name); + match self.executor.as_ref() { + None => Ok(json!({ "error": "ROCm tools unavailable in this mode." })), + Some(e) => match e.execute($name, &args) { + RocmToolOutcome::ApprovalRequired(intent) => { + if let Some(tx) = self.approval_tx.as_ref() { + let _ = tx.send(ClientMsg::ChatApprovalRequired { intent }); + } + Ok(json!({ + "status": "surfaced_for_approval", + "note": "This action needs operator approval; it has been \ + surfaced to the operator. Do not retry.", + })) + } + RocmToolOutcome::Error(s) => Ok(json!({ "error": s })), + RocmToolOutcome::Result(v) => Ok(v), + }, + } + } + } + }; +} + +rocm_mutating_tool!( + InstallSdkRocmTool, + "install_sdk", + "Install the TheRock ROCm SDK. MUTATING — this is surfaced for operator \ + approval before anything runs; it does NOT install immediately. The user \ + must supply an install `prefix` (folder); ask for it first.", + { + "type": "object", + "properties": { + "channel": { "type": "string", "description": "Release channel: 'release' or 'nightly'." }, + "format": { "type": "string", "description": "Artifact format: 'wheel' or 'tarball'." }, + "prefix": { "type": "string", "description": "Install folder (required; never a system path)." }, + "version": { "type": "string", "description": "Optional explicit wheel version selector." } + } + } +); +rocm_mutating_tool!( + InstallEngineRocmTool, + "install_engine", + "Install an inference engine (e.g. vllm, llama-cpp, comfyui). MUTATING — \ + surfaced for operator approval before anything runs.", + { + "type": "object", + "properties": { + "engine": { "type": "string", "description": "Engine to install, e.g. 'vllm'." }, + "runtime_id": { "type": "string", "description": "Optional ROCm runtime id to target." }, + "python_version": { "type": "string", "description": "Optional Python version for the engine env." }, + "reinstall": { "type": "boolean", "description": "Reinstall even if already present." } + }, + "required": ["engine"] + } +); +rocm_mutating_tool!( + LaunchServerRocmTool, + "launch_server", + "Start a local managed model server. MUTATING — surfaced for operator \ + approval before anything runs. Host is loopback-only (no public bind); CPU \ + execution is rejected (ROCm GPU required).", + { + "type": "object", + "properties": { + "model": { "type": "string", "description": "Model id/name to serve." }, + "engine": { "type": "string", "description": "Optional engine, e.g. 'vllm'." }, + "host": { "type": "string", "description": "Loopback host only (e.g. 127.0.0.1)." }, + "port": { "type": "integer", "description": "Optional TCP port." }, + "device": { "type": "string", "description": "GPU device selector (CPU is rejected)." } + }, + "required": ["model"] + } +); +rocm_mutating_tool!( + StopServerRocmTool, + "stop_server", + "Stop a running local managed model server by service id. MUTATING — \ + surfaced for operator approval before anything runs.", + { + "type": "object", + "properties": { + "service_id": { "type": "string", "description": "Managed service identifier to stop." } + }, + "required": ["service_id"] + } +); + +/// All mutating ROCm tool names (mirrors [`ROCM_READ_TOOL_NAMES`]). +/// +/// Used for uniqueness/registration checks and the parity map. Phase 4 ships +/// exactly the install/engine/serve/services mutating set; +/// update/comfyui/uninstall/setup and automations toggles are later phases. +pub const ROCM_MUTATING_TOOL_NAMES: [&str; 4] = [ + InstallSdkRocmTool::NAME, + InstallEngineRocmTool::NAME, + LaunchServerRocmTool::NAME, + StopServerRocmTool::NAME, +]; + +/// Register every mutating ROCm tool on a Rig `AgentBuilder`, cloning the +/// optional executor + approval channel + the shared `fired` log into each. +/// Generic over the builder's model + preamble so both client paths reuse one +/// registration site (DRY). Called after [`register_rocm_read_tools`]. +fn register_rocm_mutating_tools( + builder: rig::agent::AgentBuilder, + executor: Option<&SharedRocmToolExecutor>, + approval_tx: Option<&UnboundedSender>, + fired: &FiredLog, +) -> rig::agent::AgentBuilder +where + M: rig::completion::CompletionModel, + P: rig::agent::PromptHook, +{ + builder + .tool(InstallSdkRocmTool { + executor: executor.cloned(), + approval_tx: approval_tx.cloned(), + fired: fired.clone(), + }) + .tool(InstallEngineRocmTool { + executor: executor.cloned(), + approval_tx: approval_tx.cloned(), + fired: fired.clone(), + }) + .tool(LaunchServerRocmTool { + executor: executor.cloned(), + approval_tx: approval_tx.cloned(), + fired: fired.clone(), + }) + .tool(StopServerRocmTool { + executor: executor.cloned(), + approval_tx: approval_tx.cloned(), + fired: fired.clone(), + }) +} + /// Live Rig-backed client for an OpenAI-compatible endpoint. The Rig client is /// constructed once; the agent + tools are rebuilt per request from the /// captured snapshot. @@ -635,14 +810,18 @@ pub struct RigAgentClient { client: rig::providers::openai::CompletionsClient, model: String, preamble: String, - /// Bin-injected read-only tool executor (None for tests / no live seam). + /// Bin-injected tool executor (None for tests / no live seam). executor: Option, + /// Channel to surface mutating-tool approval intents to the app (None for + /// tests / no live seam). Mutating tools post here instead of executing. + approval_tx: Option>, } impl RigAgentClient { pub fn new( cfg: LlmConfig, executor: Option, + approval_tx: Option>, ) -> Result { // Custom-auth gateway (e.g. Azure APIM `Ocp-Apim-Subscription-Key`): // the key goes in a custom header, NOT `Authorization: Bearer`. Rig @@ -675,6 +854,7 @@ impl RigAgentClient { model: cfg.model, preamble: DEFAULT_PREAMBLE.to_string(), executor, + approval_tx, }) } } @@ -739,7 +919,15 @@ impl AgentClient for RigAgentClient { fired: fired.clone(), }); // Read-only ROCm machine-inspection tools (forward across the seam). - let agent = register_rocm_read_tools(agent, self.executor.as_ref(), &fired).build(); + let agent = register_rocm_read_tools(agent, self.executor.as_ref(), &fired); + // Mutating ROCm tools (surface approval; never execute in the rig loop). + let agent = register_rocm_mutating_tools( + agent, + self.executor.as_ref(), + self.approval_tx.as_ref(), + &fired, + ) + .build(); let req = agent .prompt(last.content.clone()) @@ -835,8 +1023,11 @@ pub struct ChatGptAgentClient { client: rig::providers::chatgpt::Client, model: String, preamble: String, - /// Bin-injected read-only tool executor (None for tests / no live seam). + /// Bin-injected tool executor (None for tests / no live seam). executor: Option, + /// Channel to surface mutating-tool approval intents to the app (None for + /// tests / no live seam). + approval_tx: Option>, } impl ChatGptAgentClient { @@ -848,6 +1039,7 @@ impl ChatGptAgentClient { model: Option, on_device_code: F, executor: Option, + approval_tx: Option>, ) -> Result where F: Fn(String, String) + Send + Sync + 'static, @@ -908,6 +1100,7 @@ impl ChatGptAgentClient { model: model.unwrap_or_else(|| chatgpt::GPT_5_3_CODEX.to_string()), preamble: DEFAULT_PREAMBLE.to_string(), executor, + approval_tx, }) } } @@ -966,7 +1159,15 @@ impl AgentClient for ChatGptAgentClient { fired: fired.clone(), }); // Read-only ROCm machine-inspection tools (forward across the seam). - let agent = register_rocm_read_tools(agent, self.executor.as_ref(), &fired).build(); + let agent = register_rocm_read_tools(agent, self.executor.as_ref(), &fired); + // Mutating ROCm tools (surface approval; never execute in the rig loop). + let agent = register_rocm_mutating_tools( + agent, + self.executor.as_ref(), + self.approval_tx.as_ref(), + &fired, + ) + .build(); let req = agent .prompt(last.content.clone()) @@ -1290,6 +1491,9 @@ mod tests { fn execute(&self, _name: &str, _args: &Value) -> RocmToolOutcome { RocmToolOutcome::Result(self.0.clone()) } + fn execute_approved(&self, _name: &str, _args: &Value) -> RocmToolOutcome { + RocmToolOutcome::Result(self.0.clone()) + } } #[tokio::test] @@ -1310,6 +1514,100 @@ mod tests { ); } + /// Recording executor for the mutating-tool surfacing test: `execute` + /// returns `ApprovalRequired`; `execute_approved` records a call (which must + /// NOT happen during the rig tool loop). + #[derive(Debug)] + struct RecordingMutatingExec { + approved: Arc>>, + } + impl crate::tool_exec::RocmToolExecutor for RecordingMutatingExec { + fn execute(&self, name: &str, args: &Value) -> RocmToolOutcome { + RocmToolOutcome::ApprovalRequired(crate::tool_exec::ApprovalIntent { + title: "T".to_string(), + body: vec!["cmd".to_string()], + name: name.to_string(), + arguments: args.clone(), + }) + } + fn execute_approved(&self, name: &str, _args: &Value) -> RocmToolOutcome { + self.approved.lock().unwrap().push(name.to_string()); + RocmToolOutcome::Result(json!({ "ok": true })) + } + } + + #[tokio::test] + async fn mutating_tool_surfaces_approval_not_execution() { + // (f) a mutating rig tool's call() posts ChatApprovalRequired over the + // approval channel and returns a "surfaced" note — it must NOT execute + // (execute_approved is never called from the rig loop). + let approved = Arc::new(Mutex::new(Vec::::new())); + let exec: SharedRocmToolExecutor = Arc::new(RecordingMutatingExec { + approved: approved.clone(), + }); + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::(); + let tool = InstallSdkRocmTool { + executor: Some(exec), + approval_tx: Some(tx), + fired: Arc::new(Mutex::new(Vec::new())), + }; + let out = tool + .call(json!({ "channel": "release", "format": "wheel", "prefix": "/tmp/x" })) + .await + .expect("tool call ok"); + assert_eq!(out["status"], "surfaced_for_approval"); + // The intent was posted to the app (modal would open). + match rx.try_recv().expect("approval intent posted") { + ClientMsg::ChatApprovalRequired { intent } => { + assert_eq!(intent.name, "install_sdk"); + assert_eq!(intent.arguments["channel"], "release"); + } + other => panic!("expected ChatApprovalRequired, got {other:?}"), + } + // Crucially, nothing executed in the rig loop. + assert!( + approved.lock().unwrap().is_empty(), + "mutating tool must not execute in the rig loop" + ); + } + + #[tokio::test] + async fn mutating_tool_none_executor_is_graceful() { + let tool = LaunchServerRocmTool { + executor: None, + approval_tx: None, + fired: Arc::new(Mutex::new(Vec::new())), + }; + let out = tool.call(json!({ "model": "m" })).await.expect("ok"); + assert!(out.get("error").and_then(Value::as_str).is_some()); + } + + #[test] + fn mutating_tool_names_are_complete_and_disjoint() { + // The registry is the source of truth: every entry is non-empty and the + // Phase 4 mutating set is present in full. + for expected in [ + "install_sdk", + "install_engine", + "launch_server", + "stop_server", + ] { + assert!( + ROCM_MUTATING_TOOL_NAMES.contains(&expected), + "missing mutating tool: {expected}" + ); + } + // Disjoint from read-only + skill names (iterates the registry itself). + for n in ROCM_MUTATING_TOOL_NAMES { + assert!(!SKILL_NAMES.contains(&n), "collision with skill: {n}"); + // install_sdk_dry_run (read-only) must not clash with install_sdk. + assert!( + !ROCM_READ_TOOL_NAMES.contains(&n), + "collision with read tool: {n}" + ); + } + } + #[tokio::test] async fn read_only_tool_none_executor_is_graceful() { // No seam (demo/replay/mock): a clear error object, never a panic. @@ -1412,7 +1710,7 @@ mod tests { api_key: None, auth_header: None, }; - let client = RigAgentClient::new(cfg, None).expect("build rig client"); + let client = RigAgentClient::new(cfg, None, None).expect("build rig client"); let history = vec![ChatTurn::user("What's GPU-2 doing? Use the tools.")]; let reply = client .complete(&history, fixture_snapshot()) @@ -1443,7 +1741,7 @@ mod tests { api_key: Some(key), auth_header, }; - let client = RigAgentClient::new(cfg, None).expect("build rig client"); + let client = RigAgentClient::new(cfg, None, None).expect("build rig client"); let history = vec![ChatTurn::user("Reply with exactly: gateway ok")]; let reply = client .complete(&history, fixture_snapshot()) @@ -1467,6 +1765,7 @@ mod tests { sink.lock().unwrap().push(format!("{url}|{code}")); }, None, + None, ) .expect("build chatgpt oauth client"); assert_eq!(client.model, "gpt-5.3-codex"); @@ -1476,7 +1775,7 @@ mod tests { #[test] fn chatgpt_oauth_client_defaults_model_when_none() { - let client = ChatGptAgentClient::new(None, |_url, _code| {}, None) + let client = ChatGptAgentClient::new(None, |_url, _code| {}, None, None) .expect("build chatgpt oauth client"); assert_eq!( client.model, @@ -1498,6 +1797,7 @@ mod tests { eprintln!("Sign in: open {url} and enter code {code}"); }, None, + None, ) .expect("build chatgpt oauth client"); let history = vec![ChatTurn::user("Reply with exactly: oauth ok")]; diff --git a/crates/rocm-dash-tui/src/app.rs b/crates/rocm-dash-tui/src/app.rs index 57bacc94..64a5bcc6 100644 --- a/crates/rocm-dash-tui/src/app.rs +++ b/crates/rocm-dash-tui/src/app.rs @@ -280,6 +280,21 @@ pub(crate) struct SlashToolRequest { pub label: String, } +/// A surfaced mutating-tool approval awaiting the operator's decision (Phase 4). +/// Reusable for any [`crate::tool_exec::ApprovalIntent`] (the same modal serves +/// later phases: update/uninstall, permissions, plan). The modal owns keyboard +/// focus while `Some`; on Approve the `(name, arguments)` are replayed through +/// `execute_approved`; on Deny/Cancel nothing runs. +#[derive(Debug, Clone)] +pub(crate) struct PendingApproval { + pub req: crate::ui::approval::ApprovalRequest, + pub choice: crate::ui::approval::ApprovalChoice, + /// Tool name to re-execute on Approve (the validator already accepted it). + pub name: String, + /// JSON args for the approved re-execution. + pub arguments: serde_json::Value, +} + /// Modal overlays. Only one is shown at a time, on top of the active tab body. #[derive(Debug, Clone, Default, PartialEq, Eq)] pub enum Modal { @@ -401,6 +416,9 @@ pub struct AppState { /// Edge: a pending executor-backed read-only slash command. Raised by /// `handle_slash_command`, drained once by the event loop (spawn_blocking). pub(crate) slash_tool: Option, + /// A surfaced mutating-tool approval awaiting the operator's decision + /// (Phase 4). `Some` ⇒ the approval modal is open and owns keyboard focus. + pub(crate) approval: Option, } impl AppState { @@ -460,6 +478,7 @@ impl AppState { tool_executor: None, should_quit: false, slash_tool: None, + approval: None, } } @@ -479,6 +498,7 @@ impl AppState { self.automations_manager = None; self.command_screen = None; self.config_manager = None; + self.approval = None; } /// Open the theme picker modal, positioning the cursor on the active theme. @@ -694,6 +714,108 @@ impl AppState { label: "daemon status".to_string(), }); } + // --- 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(), + }); + } + } + } // Unknown slash command: an error turn, never sent to the LLM. other => { self.chat.push(ChatTurn::error(format!( @@ -737,6 +859,68 @@ impl AppState { self.chat.push(ChatTurn::agent(text)); } + /// Open the approval modal for a surfaced mutating-tool intent (Phase 4). + /// Closes any operational overlay first so the modal owns focus alone. + pub(crate) fn open_approval(&mut self, intent: crate::tool_exec::ApprovalIntent) { + if self.approval.is_some() { + self.chat.push(ChatTurn::error( + "An action is already awaiting approval; the new request was discarded. Resolve the open approval first.", + )); + return; + } + self.close_overlays(); + self.approval = Some(PendingApproval { + req: crate::ui::approval::ApprovalRequest::new(intent.title, intent.body), + choice: crate::ui::approval::ApprovalChoice::Approve, + name: intent.name, + arguments: intent.arguments, + }); + } + + /// Route a key to the open approval modal: move the cursor and return a + /// verdict if the key confirmed one. Pure w.r.t. I/O — the caller maps the + /// verdict onto execution (Approve) or a declined turn (Deny/Cancel). No-op + /// returning `None` when no modal is open. + pub(crate) fn on_approval_key( + &mut self, + code: crossterm::event::KeyCode, + ) -> Option { + let pa = self.approval.as_mut()?; + let (choice, verdict) = crate::ui::approval::approval_key(code, pa.choice); + pa.choice = choice; + verdict + } + + /// Take the pending approval's `(name, arguments)` for off-thread execution, + /// clearing the modal. Returns `None` if no modal is open. + pub(crate) fn take_approval(&mut self) -> Option<(String, serde_json::Value)> { + self.approval.take().map(|pa| (pa.name, pa.arguments)) + } + + /// Handle a Deny/Cancel verdict: clear the modal and append a declined turn. + /// Nothing executes. + pub(crate) fn on_approval_declined(&mut self) { + self.approval = None; + self.chat.push(ChatTurn::agent("Action declined.")); + } + + /// Append the approved-action result turn AND raise the one-shot + /// `chat_dispatch` edge so the agent does EXACTLY ONE automatic follow-up + /// turn that incorporates the result. The result is pushed as an agent turn + /// (so `build_messages` sends it as conversational context); `chat_dispatch` + /// is consumed once by the event loop, so this never loops. A further + /// mutating request from that follow-up re-surfaces approval (user-gated), + /// so there is no unbounded execution. Clears any open modal defensively. + pub(crate) fn on_approval_result(&mut self, text: String) { + self.approval = None; + self.chat.push(ChatTurn::agent(text)); + // Exactly one follow-up: raise the edge once. `chat_sending` mirrors a + // normal submit so the UI shows the in-flight state and a double key + // can't race a second dispatch. + self.chat_sending = true; + self.chat_dispatch = true; + } + /// Apply the currently-highlighted picker entry and close the modal. pub fn apply_theme_pick(&mut self) { let names = crate::ui::theme::theme_names(); @@ -1017,19 +1201,20 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu }); }, state.tool_executor.clone(), + Some(chat_tx.clone()), ) .ok() .map(|c| std::sync::Arc::new(c) as std::sync::Arc) } else { // A build failure leaves `agent` None; a submit surfaces an error turn. match &state.chat_llm { - Some(cfg) => { - crate::agent::RigAgentClient::new(cfg.clone(), state.tool_executor.clone()) - .ok() - .map(|c| { - std::sync::Arc::new(c) as std::sync::Arc - }) - } + Some(cfg) => crate::agent::RigAgentClient::new( + cfg.clone(), + state.tool_executor.clone(), + Some(chat_tx.clone()), + ) + .ok() + .map(|c| std::sync::Arc::new(c) as std::sync::Arc), None => None, } } @@ -1061,6 +1246,12 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu Some(ClientMsg::SlashToolReply { text }) => state.on_slash_tool_reply(text), Some(ClientMsg::ChatError { message }) => state.on_chat_error(message), Some(ClientMsg::ChatDetectResult { offer }) => state.set_detect_result(offer), + // A mutating tool (or slash command) surfaced an approval — + // open the modal; nothing executes until the operator approves. + Some(ClientMsg::ChatApprovalRequired { intent }) => state.open_approval(intent), + // An approved action finished: append the result turn and + // fire exactly one automatic follow-up agent turn. + Some(ClientMsg::ChatApprovalResult { text }) => state.on_approval_result(text), None => break, } } @@ -1073,6 +1264,38 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu } maybe_ev = events.next() => { match maybe_ev { + // The approval modal, when open, owns ALL keys with the + // highest priority (above every operational overlay and the + // general handler) so the operator's decision can't be + // pre-empted. On Approve: replay the approved action off the + // event loop (spawn_blocking) and post ChatApprovalResult. + // On Deny/Cancel: a declined turn, no execution. + Some(Ok(CtEvent::Key(k))) if state.approval.is_some() => { + use crate::ui::approval::ApprovalVerdict; + match state.on_approval_key(k.code) { + Some(ApprovalVerdict::Approve) => { + if let Some((name, args)) = state.take_approval() { + match state.tool_executor.clone() { + Some(executor) => { + let reply_tx = chat_tx.clone(); + tokio::task::spawn_blocking(move || { + let text = run_approved(&executor, &name, &args); + let _ = reply_tx + .send(ClientMsg::ChatApprovalResult { text }); + }); + } + None => state.on_approval_result( + "ROCm tools unavailable in this mode".to_string(), + ), + } + } + } + Some(ApprovalVerdict::Deny | ApprovalVerdict::Cancel) => { + state.on_approval_declined(); + } + None => { /* cursor moved or key ignored — modal stays open */ } + } + } // The services-manager overlay, when open, owns all keys // (and may spawn lifecycle jobs through the job-bridge). Some(Ok(CtEvent::Key(k))) if state.services.is_some() => { @@ -1238,9 +1461,19 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu Some(executor) => { let reply_tx = chat_tx.clone(); tokio::task::spawn_blocking(move || { - let outcome = executor.execute(&req.name, &req.args); - let text = summarize_slash_tool(&req.label, &outcome); - let _ = reply_tx.send(ClientMsg::SlashToolReply { text }); + // One path for read-only AND mutating slash commands: + // `Result`/`Error` → a concise reply turn; an + // `ApprovalRequired` (mutating) → open the approval modal + // via ChatApprovalRequired (nothing executes yet). + let msg = match executor.execute(&req.name, &req.args) { + crate::tool_exec::RocmToolOutcome::ApprovalRequired(intent) => { + ClientMsg::ChatApprovalRequired { intent } + } + outcome => ClientMsg::SlashToolReply { + text: summarize_slash_tool(&req.label, &outcome), + }, + }; + let _ = reply_tx.send(msg); }); } None => { @@ -1350,6 +1583,33 @@ fn persist_chat_endpoint(base_url: &str, model: &str) -> Result String { + use crate::tool_exec::RocmToolOutcome; + match executor.execute_approved(name, args) { + RocmToolOutcome::Result(v) => { + let body = summarize_json_value(&v); + if body.is_empty() { + format!("Approved · {name}: done") + } else { + format!("Approved · {name}:\n{body}") + } + } + RocmToolOutcome::Error(e) => format!("Approved · {name} failed: {e}"), + // A mutating tool's approved replay should not re-request approval; if it + // somehow does, surface it plainly rather than silently looping. + RocmToolOutcome::ApprovalRequired(_) => { + format!("Approved · {name}: unexpected second approval request (not run)") + } + } +} + fn summarize_slash_tool(label: &str, outcome: &crate::tool_exec::RocmToolOutcome) -> String { use crate::tool_exec::RocmToolOutcome; match outcome { @@ -2816,6 +3076,316 @@ mod tests { ); } + // --- Phase 4: mutating slash dispatch + approval modal flow --- + + #[test] + fn slash_install_raises_install_sdk_request() { + let mut s = st(); + assert_eq!( + s.handle_slash_command("/install ~/rocm"), + SlashOutcome::Handled + ); + let req = s.slash_tool.expect("install raises a slash_tool request"); + assert_eq!(req.name, "install_sdk"); + assert_eq!(req.args["channel"], "release"); + assert_eq!(req.args["format"], "wheel"); + // The validator REQUIRES a prefix; the slash path must supply one or the + // modal never opens. + assert_eq!(req.args["prefix"], "~/rocm"); + } + + #[test] + fn slash_install_without_prefix_hints_not_dispatch() { + let mut s = st(); + assert_eq!(s.handle_slash_command("/install"), SlashOutcome::Handled); + assert!( + s.slash_tool.is_none(), + "no dispatch without an install folder" + ); + assert_eq!(s.chat.last().unwrap().role, ChatRole::Error); + } + + #[test] + fn slash_engine_raises_install_engine_request() { + let mut s = st(); + assert_eq!( + s.handle_slash_command("/engine vllm"), + SlashOutcome::Handled + ); + let req = s.slash_tool.expect("engine raises a slash_tool request"); + assert_eq!(req.name, "install_engine"); + assert_eq!(req.args, serde_json::json!({ "engine": "vllm" })); + } + + #[test] + fn slash_engine_without_name_hints_not_dispatch() { + let mut s = st(); + assert_eq!(s.handle_slash_command("/engine"), SlashOutcome::Handled); + assert!(s.slash_tool.is_none(), "no dispatch without an engine name"); + assert_eq!(s.chat.last().unwrap().role, ChatRole::Error); + } + + #[test] + fn slash_serve_raises_launch_server_request() { + let mut s = st(); + assert_eq!( + s.handle_slash_command("/serve deepseek-r1"), + SlashOutcome::Handled + ); + let req = s.slash_tool.expect("serve raises a slash_tool request"); + assert_eq!(req.name, "launch_server"); + assert_eq!(req.args["model"], "deepseek-r1"); + // Loopback host is forced so the validator never rejects the slash path. + assert_eq!(req.args["host"], "127.0.0.1"); + } + + #[test] + fn slash_services_stop_raises_stop_server_request() { + let mut s = st(); + assert_eq!( + s.handle_slash_command("/services stop svc-1"), + SlashOutcome::Handled + ); + let req = s + .slash_tool + .expect("services stop raises a slash_tool request"); + assert_eq!(req.name, "stop_server"); + assert_eq!(req.args, serde_json::json!({ "service_id": "svc-1" })); + } + + #[test] + fn slash_services_restart_is_guided_not_stop() { + // restart is NOT wired through the chat seam yet; it must guide the + // operator instead of silently running stop_server (a semantic lie). + let mut s = st(); + assert_eq!( + s.handle_slash_command("/services restart svc-1"), + SlashOutcome::Handled + ); + assert!( + s.slash_tool.is_none(), + "restart must NOT dispatch a stop_server request" + ); + assert_eq!(s.chat.last().unwrap().role, ChatRole::Error); + } + + #[test] + fn slash_services_bare_is_read_only_list() { + let mut s = st(); + assert_eq!(s.handle_slash_command("/services"), SlashOutcome::Handled); + let req = s.slash_tool.expect("bare services lists managed services"); + assert_eq!(req.name, "services"); + } + + /// Recording executor: mutating names surface `ApprovalRequired`; the + /// approved replay records `(name, args)` and returns a success Result. Used + /// to drive the approve/deny/follow-up tests offline (no real installs). + #[derive(Debug)] + struct RecordingExecutor { + approved: std::sync::Arc>>, + } + impl RecordingExecutor { + fn new() -> Self { + Self { + approved: std::sync::Arc::new(std::sync::Mutex::new(Vec::new())), + } + } + } + impl crate::tool_exec::RocmToolExecutor for RecordingExecutor { + fn execute( + &self, + name: &str, + args: &serde_json::Value, + ) -> crate::tool_exec::RocmToolOutcome { + crate::tool_exec::RocmToolOutcome::ApprovalRequired(crate::tool_exec::ApprovalIntent { + title: "T".to_string(), + body: vec!["cmd".to_string()], + name: name.to_string(), + arguments: args.clone(), + }) + } + fn execute_approved( + &self, + name: &str, + args: &serde_json::Value, + ) -> crate::tool_exec::RocmToolOutcome { + self.approved + .lock() + .unwrap() + .push((name.to_string(), args.clone())); + crate::tool_exec::RocmToolOutcome::Result(serde_json::json!({ "ok": true })) + } + } + + #[test] + fn approval_required_opens_modal() { + let mut s = st(); + let intent = crate::tool_exec::ApprovalIntent { + title: "Install ROCm".to_string(), + body: vec!["rocm install sdk".to_string()], + name: "install_sdk".to_string(), + arguments: serde_json::json!({ "channel": "release" }), + }; + s.open_approval(intent); + let pa = s.approval.as_ref().expect("modal opened"); + assert_eq!(pa.name, "install_sdk"); + assert_eq!(pa.req.title, "Install ROCm"); + } + + #[test] + fn close_overlays_clears_pending_approval() { + // A stale approval modal must not survive close_overlays (focus trap). + let mut s = st(); + s.open_approval(crate::tool_exec::ApprovalIntent { + title: "Install ROCm".to_string(), + body: vec!["rocm install sdk".to_string()], + name: "install_sdk".to_string(), + arguments: serde_json::json!({}), + }); + assert!(s.approval.is_some(), "approval pending before close"); + s.close_overlays(); + assert!(s.approval.is_none(), "close_overlays must clear the modal"); + } + + #[test] + fn second_approval_request_is_discarded_while_one_pending() { + // Two mutating calls in one turn must not clobber: the operator could + // otherwise approve args they never saw. + let mut s = st(); + let first = crate::tool_exec::ApprovalIntent { + title: "Install ROCm".to_string(), + body: vec!["rocm install sdk".to_string()], + name: "install_sdk".to_string(), + arguments: serde_json::json!({ "prefix": "~/rocm" }), + }; + s.open_approval(first); + s.open_approval(crate::tool_exec::ApprovalIntent { + title: "Stop server".to_string(), + body: vec!["rocm services stop svc-1".to_string()], + name: "stop_server".to_string(), + arguments: serde_json::json!({ "service_id": "svc-1" }), + }); + // The original intent survives intact; the second was discarded. + let pa = s.approval.as_ref().expect("first approval still pending"); + assert_eq!(pa.name, "install_sdk"); + assert_eq!(pa.arguments["prefix"], "~/rocm"); + assert_eq!(s.chat.last().unwrap().role, ChatRole::Error); + } + + #[test] + fn approve_path_runs_execute_approved_with_expected_args() { + // (a) approve: a ChatApprovalRequired opens the modal; an Approve verdict + // drives execute_approved with the exact name + args. We exercise the + // same sync code path the spawn_blocking uses (`run_approved`). + let exec = std::sync::Arc::new(RecordingExecutor::new()); + let recorded = exec.approved.clone(); + let shared: crate::tool_exec::SharedRocmToolExecutor = exec; + + let mut s = st(); + s.open_approval(crate::tool_exec::ApprovalIntent { + title: "T".to_string(), + body: vec!["cmd".to_string()], + name: "install_sdk".to_string(), + arguments: serde_json::json!({ "channel": "release", "format": "wheel" }), + }); + // Enter on the default (Approve) choice yields an Approve verdict. + let verdict = s.on_approval_key(crossterm::event::KeyCode::Enter); + assert_eq!(verdict, Some(crate::ui::approval::ApprovalVerdict::Approve)); + let (name, args) = s.take_approval().expect("approval taken on approve"); + assert!(s.approval.is_none(), "modal cleared after taking approval"); + + let summary = run_approved(&shared, &name, &args); + let log = recorded.lock().unwrap(); + assert_eq!(log.len(), 1, "execute_approved ran exactly once"); + assert_eq!(log[0].0, "install_sdk"); + assert_eq!(log[0].1["channel"], "release"); + assert_eq!(log[0].1["format"], "wheel"); + assert!( + summary.contains("Approved"), + "concise summary, not raw JSON" + ); + } + + #[test] + fn deny_path_runs_nothing_and_appends_declined_turn() { + // (b) deny: a Deny/Cancel verdict appends a declined turn and never + // touches execute_approved. + let exec = std::sync::Arc::new(RecordingExecutor::new()); + let recorded = exec.approved.clone(); + + let mut s = st(); + s.open_approval(crate::tool_exec::ApprovalIntent { + title: "T".to_string(), + body: vec!["cmd".to_string()], + name: "launch_server".to_string(), + arguments: serde_json::json!({ "model": "m" }), + }); + // 'n' is a direct Deny verdict. + let verdict = s.on_approval_key(crossterm::event::KeyCode::Char('n')); + assert_eq!(verdict, Some(crate::ui::approval::ApprovalVerdict::Deny)); + s.on_approval_declined(); + assert!(s.approval.is_none(), "modal cleared on deny"); + assert_eq!(s.chat.last().unwrap().role, ChatRole::Agent); + assert!(s.chat.last().unwrap().content.contains("declined")); + assert!( + recorded.lock().unwrap().is_empty(), + "deny must not execute the action" + ); + + // Esc also cancels (no execution). + let mut s2 = st(); + s2.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_eq!( + s2.on_approval_key(crossterm::event::KeyCode::Esc), + Some(crate::ui::approval::ApprovalVerdict::Cancel) + ); + } + + #[test] + fn approval_result_fires_exactly_one_follow_up_no_loop() { + // (c) exactly one follow-up: on_approval_result appends the result turn + // AND raises chat_dispatch exactly once; it must not re-trigger itself. + let mut s = st(); + assert!(!s.chat_dispatch); + s.on_approval_result("Approved · install_sdk: done".to_string()); + assert_eq!(s.chat.last().unwrap().role, ChatRole::Agent); + assert!(s.chat_dispatch, "exactly one follow-up edge raised"); + assert!(s.chat_sending, "in-flight mirrors a normal submit"); + + // Simulate the event loop consuming the edge once. + s.chat_dispatch = false; + // A subsequent tick must NOT re-raise it on its own (no self-loop): the + // only thing that re-raises is another explicit result/submit. + assert!(!s.chat_dispatch, "follow-up does not re-trigger itself"); + } + + #[test] + fn approval_key_tab_moves_choice_without_verdict() { + let mut s = st(); + s.open_approval(crate::tool_exec::ApprovalIntent { + title: "T".to_string(), + body: vec!["cmd".to_string()], + name: "install_sdk".to_string(), + arguments: serde_json::json!({}), + }); + // Tab toggles the cursor to Deny without producing a verdict. + assert_eq!(s.on_approval_key(crossterm::event::KeyCode::Tab), None); + assert_eq!( + s.approval.as_ref().unwrap().choice, + crate::ui::approval::ApprovalChoice::Deny + ); + // Enter now confirms Deny. + assert_eq!( + s.on_approval_key(crossterm::event::KeyCode::Enter), + Some(crate::ui::approval::ApprovalVerdict::Deny) + ); + } + #[test] fn slash_tool_reply_does_not_disturb_chat_sending() { // The slash-tool reply path is decoupled from the agent state machine: diff --git a/crates/rocm-dash-tui/src/client.rs b/crates/rocm-dash-tui/src/client.rs index dcc89357..d72d1ed6 100644 --- a/crates/rocm-dash-tui/src/client.rs +++ b/crates/rocm-dash-tui/src/client.rs @@ -62,6 +62,19 @@ pub enum ClientMsg { ChatDetectResult { offer: Option, }, + /// A mutating tool surfaced an approval request (Phase 4). Posted by the + /// mutating rig tool (or the slash-tool path) when `execute()` returns + /// `ApprovalRequired`; the app event loop opens the approval modal. The tool + /// itself does NOT execute — execution waits for the operator's Approve. + ChatApprovalRequired { + intent: crate::tool_exec::ApprovalIntent, + }, + /// Result of an *approved* mutating action (Phase 4). Posted off-thread after + /// `execute_approved` runs; the app appends a concise result turn and fires + /// exactly one automatic follow-up agent turn. + ChatApprovalResult { + text: String, + }, } pub fn spawn(connect: String, tx: UnboundedSender) { diff --git a/crates/rocm-dash-tui/src/tool_exec.rs b/crates/rocm-dash-tui/src/tool_exec.rs index 8985ad0d..c25039a6 100644 --- a/crates/rocm-dash-tui/src/tool_exec.rs +++ b/crates/rocm-dash-tui/src/tool_exec.rs @@ -7,16 +7,26 @@ //! Plain-data signatures ONLY (serde_json / std / serde). The bin (`apps/rocm`, //! which owns `rocm-core` and the tool engine) implements [`RocmToolExecutor`]; //! the dash holds it as `Option>` and never depends on -//! `rocm-core`. Phase 2 only stores the seam; Phase 3 will use it. +//! `rocm-core`. Phase 2 stored the seam; Phase 3 used it for read-only tools; +//! Phase 4 adds the mutating "execute approved" path + the approval descriptor. use std::sync::Arc; -/// Plain-data approval descriptor surfaced to the app event loop (Phase 4 renders it). +/// Plain-data approval descriptor surfaced to the app event loop, which renders +/// it in the approval modal and — only on Approve — replays it via +/// [`RocmToolExecutor::execute_approved`]. +/// +/// `title` + `body` are the human-readable display (the rendered command and an +/// optional explanation); `name` + `arguments` are the actionable payload used +/// to re-execute the *same* call the validator already accepted. Re-executing by +/// `(name, arguments)` (not by a free-form command) keeps the safety validators +/// the single gate — `execute_approved` re-validates before running. #[derive(Debug, Clone)] pub struct ApprovalIntent { pub title: String, pub body: Vec, - pub args: Vec, + pub name: String, + pub arguments: serde_json::Value, } /// Outcome of a tool-call intent executed by the bin across the seam. @@ -33,16 +43,19 @@ pub enum RocmToolOutcome { /// The bin implements this; the dash holds it as /// `Option>` (None for demo/replay/mock). The `Debug` /// supertrait keeps `ResolvedArgs`/`AppState` deriving Debug. -/// -/// NOTE: the mutating "execute approved" path is intentionally deferred to -/// Phase 4, where it will be added alongside the approval modal with proper -/// stdout/stderr capture (TUI-safe), spawn_blocking off the async loop, and an -/// approval-provenance barrier so only descriptors from `execute()`'s -/// ApprovalRequired can be run. pub trait RocmToolExecutor: std::fmt::Debug + Send + Sync { /// Execute a tool-call intent: read-only → Result(json); mutating → ApprovalRequired; failure → Error. /// (Return value carries `#[must_use]` via the `RocmToolOutcome` enum.) fn execute(&self, name: &str, args: &serde_json::Value) -> RocmToolOutcome; + + /// Run an *approved* mutating action via the bin's captured-subprocess path + /// (piped stdout/stderr → JSON; no printing to the TUI terminal, so it is + /// TUI-safe). Called only after the operator approves the modal, with the + /// `(name, arguments)` taken from the [`ApprovalIntent`] that `execute()` + /// returned. It re-validates the call, so the safety validators remain the + /// single gate and an unapproved/invalid call can never run here. This is a + /// blocking call — invoke it off the async event loop (spawn_blocking). + fn execute_approved(&self, name: &str, args: &serde_json::Value) -> RocmToolOutcome; } /// Arc-wrapped executor as stored in `ResolvedArgs`/`AppState`. diff --git a/crates/rocm-dash-tui/src/ui/mod.rs b/crates/rocm-dash-tui/src/ui/mod.rs index c88e11a3..adf3a926 100644 --- a/crates/rocm-dash-tui/src/ui/mod.rs +++ b/crates/rocm-dash-tui/src/ui/mod.rs @@ -133,6 +133,12 @@ pub fn draw(f: &mut Frame, state: &mut AppState) { } else if let Some(cm) = &state.config_manager { config_manager::draw_config_manager(f, outer[2], cm, &state.jobs, &theme); } + + // Approval modal (Phase 4): drawn LAST so it sits on top of every overlay + // and owns the screen while a mutating-tool approval is pending. + if let Some(pa) = &state.approval { + approval::draw_approval(f, outer[2], &pa.req, pa.choice, &theme); + } } fn draw_header(f: &mut Frame, area: Rect, state: &AppState, theme: &Theme) { diff --git a/crates/rocm-dash-tui/src/ui/tabs/instances.rs b/crates/rocm-dash-tui/src/ui/tabs/instances.rs index 5b7394a4..9800c6e8 100644 --- a/crates/rocm-dash-tui/src/ui/tabs/instances.rs +++ b/crates/rocm-dash-tui/src/ui/tabs/instances.rs @@ -809,6 +809,7 @@ mod tests { tool_executor: None, should_quit: false, slash_tool: None, + approval: None, } } diff --git a/docs/dash-parity-map.md b/docs/dash-parity-map.md index 5b2e8328..6e8fe146 100644 --- a/docs/dash-parity-map.md +++ b/docs/dash-parity-map.md @@ -24,10 +24,10 @@ Groups: **A** nav/session · **B** read-only · **C** approvals/automations · | logs | B | covered (Phase 3) | `/logs` slash → opens the logs overlay (`logs_view`); the read-only `service_logs` tool is covered separately via the LLM tool-call seam | `slash_logs_opens_overlay` / `read_only_tool_round_trips_to_json` | | gpu | B | covered (Phase 3) | `/gpu` slash → `ActiveTab::Hardware`; tool `gpu_snapshot` via seam | `slash_gpu_switches_to_hardware` | | daemon | B | covered (Phase 3) | `/daemon` slash → `slash_tool` `rocm_command ["daemon","status"]` (off-thread) | `slash_daemon_raises_executor_request` | -| install | D | pending (Phase 4) | install overlay + mutating tool (approval-gated) | — | -| engine | D | pending (Phase 4) | engine-manager overlay + mutating tool (approval-gated) | — | -| serve | D | pending (Phase 4) | serve wizard + mutating tool (approval-gated) | — | -| services | D | pending (Phase 4) | services-manager overlay + mutating actions (approval-gated) | — | +| install | D | covered (Phase 4) | `/install` slash → `install_sdk` mutating tool → approval modal → `execute_approved` (captured subprocess); also LLM tool-call seam | `slash_install_raises_install_sdk_request` / `approve_path_runs_execute_approved` | +| engine | D | covered (Phase 4) | `/engine ` slash → `install_engine` mutating tool → approval modal → `execute_approved`; also LLM tool-call seam | `slash_engine_raises_install_engine_request` | +| serve | D | covered (Phase 4) | `/serve ` slash (loopback host) → `launch_server` mutating tool → approval modal → `execute_approved`; also LLM tool-call seam | `slash_serve_raises_launch_server_request` / `seam_execute_approved_rejects_unsafe_call_via_validator` | +| services | D | covered (Phase 4) | `/services stop ` slash → `stop_server` mutating tool → approval modal → `execute_approved`; `restart` is guided (not yet wired through the chat seam — points to stop + `/serve`); bare `/services` is read-only | `slash_services_stop_raises_stop_server_request` / `slash_services_restart_is_guided_not_stop` | | update | D | pending (Phase 5) | update overlay + mutating apply (approval-gated) | — | | comfyui | D | pending (Phase 5) | ComfyUI serve/launch flow (approval-gated) | — | | uninstall | D | pending (Phase 5) | uninstall flow (approval-gated) | — |