tool card UI redesign: semantic icons, inline diff persistence, tool detail page, MCP-friendly titles

crates/skald-core/src/session/handler/dispatch.rs

@@ -12,11 +12,30 @@ use tokio_util::sync::CancellationToken;
 use tracing::warn;
 
 use crate::events::ServerEvent;
-use crate::tools::{drive_execution, tool_names as tn, ExecutionOutcome, ToolResult};
+use crate::tools::{drive_execution, is_file_write_tool, tool_names as tn, ExecutionOutcome, ToolResult};
 
 use super::ChatSessionHandler;
 use super::interface_tools::AgentRunConfig;
 
+/// Max bytes captured per side of a file-write diff preview. Beyond this the side is
+/// dropped (`None`) so a huge file never bloats a row or the WS payload — the detail
+/// page then shows no diff for it.
+const MAX_PREVIEW_BYTES: usize = 256 * 1024;
+
+/// A file-write tool's before/after snapshot, captured by `execute_tool_call` around
+/// the write so the diff renders inline and survives a reload (Phase 2). `None` sides
+/// mean unreadable / new file / over the cap.
+pub(super) struct WritePreview {
+    pub old: Option<String>,
+    pub new: Option<String>,
+}
+
+/// Drops a captured snapshot over the size cap (a truncated snapshot would render a
+/// misleading diff, so omit it entirely).
+fn cap_preview(s: Option<String>) -> Option<String> {
+    s.filter(|c| c.len() <= MAX_PREVIEW_BYTES)
+}
+
 /// Whether a tool call is a synchronous sub-agent dispatch, i.e. one intercepted
 /// by `execute_tool_call` and routed to `dispatch_sub_agent` rather than the
 /// registry. Covers `execute_task` (mode=sync), `execute_subtask`, and the legacy
@@ -30,8 +49,12 @@ pub(super) fn is_sync_sub_agent(tool_name: &str, args: &Value) -> bool {
 
 /// Result of routing a single tool call to its executor.
 pub(super) enum DispatchResult {
-    /// Normal completion / failure / cancellation — the caller records it.
-    Outcome(ExecutionOutcome),
+    /// Normal completion / failure / cancellation — the caller records it. `preview`
+    /// carries a file-write's before/after snapshot (else `None`) for the diff card.
+    Outcome {
+        outcome: ExecutionOutcome,
+        preview: Option<WritePreview>,
+    },
     /// The turn must end now and the tool row must stay `pending`: the
     /// `ask_user_clarification` WS channel closed while awaiting an answer. The
     /// caller returns `TurnOutcome::Cancelled` **without** recording the tool, so
@@ -84,12 +107,38 @@ impl ChatSessionHandler {
             // Unified cancellable path. The execution owns its in-flight state and
             // its own stop(); on /stop the work future is dropped (aborting I/O /
             // killing the child) and the tool is recorded as Cancelled, not Failed.
-            match self.build_execution(tool_name, args.clone(), config) {
+            //
+            // For a file-write tool, bracket the execution with a before/after
+            // snapshot so its diff renders inline and survives a reload (Phase 2).
+            // The reads route memory-vs-disk exactly like the write itself
+            // (`read_current_content`); `new` is captured only on success.
+            let write_path = if is_file_write_tool(tool_name) {
+                args["path"].as_str().map(str::to_string)
+            } else {
+                None
+            };
+            let preview_old = match &write_path {
+                Some(p) => cap_preview(self.read_current_content(p).await),
+                None    => None,
+            };
+            let outcome = match self.build_execution(tool_name, args.clone(), config) {
                 Some(exec) => drive_execution(exec.as_ref(), token).await,
                 None        => ExecutionOutcome::Failed(format!("Unknown tool: {tool_name}")),
-            }
+            };
+            let preview = match &write_path {
+                Some(p) => {
+                    let new = if matches!(outcome, ExecutionOutcome::Completed(_)) {
+                        cap_preview(self.read_current_content(p).await)
+                    } else {
+                        None
+                    };
+                    Some(WritePreview { old: preview_old, new })
+                }
+                None => None,
+            };
+            return DispatchResult::Outcome { outcome, preview };
         };
-        DispatchResult::Outcome(outcome)
+        DispatchResult::Outcome { outcome, preview: None }
     }
 }
 

crates/skald-core/src/session/handler/emitter.rs

@@ -70,17 +70,26 @@ impl<'a> TurnEmitter<'a> {
         message_id: i64,
         name: String,
         arguments: Value,
+        display_name: String,
+        icon: String,
         label_short: String,
         label_full: String,
         path: Option<String>,
     ) {
         self.emit(ServerEvent::ToolStart {
-            tool_call_id, message_id, name, arguments, label_short, label_full, path,
+            tool_call_id, message_id, name, arguments, display_name, icon, label_short, label_full, path,
         }).await;
     }
 
-    pub(super) async fn tool_done(&self, tool_call_id: i64, result: String, result_type: String) {
-        self.emit(ServerEvent::ToolDone { tool_call_id, result, result_type }).await;
+    pub(super) async fn tool_done(
+        &self,
+        tool_call_id: i64,
+        result: String,
+        result_type: String,
+        preview_old: Option<String>,
+        preview_new: Option<String>,
+    ) {
+        self.emit(ServerEvent::ToolDone { tool_call_id, result, result_type, preview_old, preview_new }).await;
     }
 
     pub(super) async fn tool_error(&self, tool_call_id: i64, error: String) {

crates/skald-core/src/session/handler/llm_loop.rs

@@ -207,6 +207,21 @@ impl ChatSessionHandler {
     /// Handles a single tool call within a round: persists the call row, emits
     /// `ToolStart`, resolves the working directory, runs the approval gate, handles
     /// `restart`, dispatches, and records the outcome. Returns [`CallFlow::Continue`]
+    /// Card metadata (friendly display name + semantic icon key) for a tool call.
+    /// Delegates to the registry seam [`ToolRegistry::display_meta`], then layers the
+    /// MCP display-name override on for an `mcp__server__tool` name (manifest title >
+    /// live MCP `title` > the prettified name the seam already produced). The single
+    /// place the live loop resolves a card title, mirroring `describe_call`.
+    pub(super) fn tool_ui_meta(&self, name: &str, args: &serde_json::Value) -> (String, String) {
+        let mut meta = self.tools.display_meta(name, args);
+        if let Some((server, tool)) = crate::mcp::parse_mcp_tool_name(name) {
+            if let Some(friendly) = self.mcp.tool_display_name(server, tool) {
+                meta.display_name = friendly;
+            }
+        }
+        (meta.display_name, meta.icon)
+    }
+
     /// to move on to the next call, or [`CallFlow::End`] to end the whole turn.
     #[allow(clippy::too_many_arguments)]
     async fn handle_tool_call(
@@ -225,10 +240,12 @@ impl ChatSessionHandler {
         let args_str = serde_json::to_string(&call.arguments)
             .unwrap_or_else(|_| "{}".to_string());
         let tool_call_id = chat_llm_tools::append(pool, message_id, &call.name, &args_str).await?;
+        let (display_name, icon) = self.tool_ui_meta(&call.name, &call.arguments);
         em.tool_start(
             tool_call_id, message_id,
             call.name.clone(),
             call.arguments.clone(),
+            display_name, icon,
             self.tools.describe_call(&call.name, &call.arguments, ToolDescriptionLength::Short),
             self.tools.describe_call(&call.name, &call.arguments, ToolDescriptionLength::Full),
             self.tools.target_path(&call.name, &call.arguments),
@@ -252,7 +269,7 @@ impl ChatSessionHandler {
         if call.name == tn::RESTART {
             info!(session_id = self.session_id, tool_call_id, "restart approved — marking done then exiting");
             chat_llm_tools::complete(pool, tool_call_id, "Riavvio avviato.", "string").await?;
-            em.tool_done(tool_call_id, "Riavvio avviato.".to_string(), "string".to_string()).await;
+            em.tool_done(tool_call_id, "Riavvio avviato.".to_string(), "string".to_string(), None, None).await;
             // Use _exit() to skip C atexit handlers (e.g. Metal GPU cleanup in
             // whisper-rs/ggml, which aborts with SIGABRT and yields exit code 134
             // instead of 255 — breaking the run.sh restart supervisor).
@@ -262,15 +279,15 @@ impl ChatSessionHandler {
         // Route the approved call to its executor. `AbortPending` means the
         // clarification WS channel closed — end the turn and leave the tool
         // `pending` for resume to re-ask.
-        let outcome = match self.execute_tool_call(
+        let (outcome, preview) = match self.execute_tool_call(
             stack_id, config, tool_call_id, &call.name, &call.arguments, token, tx,
         ).await {
-            DispatchResult::Outcome(o)   => o,
+            DispatchResult::Outcome { outcome, preview } => (outcome, preview),
             DispatchResult::AbortPending => return Ok(CallFlow::End(TurnOutcome::Cancelled)),
         };
 
         match self.record_tool_outcome(
-            tool_call_id, &call.name, &call.arguments, outcome, em, Some(all_tool_calls),
+            tool_call_id, &call.name, &call.arguments, outcome, preview, em, Some(all_tool_calls),
         ).await? {
             RecordFlow::Continue => Ok(CallFlow::Continue),
             RecordFlow::Abort    => Ok(CallFlow::End(TurnOutcome::Cancelled)),
@@ -312,10 +329,12 @@ impl ChatSessionHandler {
             let args_str = serde_json::to_string(&call.arguments)
                 .unwrap_or_else(|_| "{}".to_string());
             let tool_call_id = chat_llm_tools::append(pool, message_id, &call.name, &args_str).await?;
+            let (display_name, icon) = self.tool_ui_meta(&call.name, &call.arguments);
             em.tool_start(
                 tool_call_id, message_id,
                 call.name.clone(),
                 call.arguments.clone(),
+                display_name, icon,
                 self.tools.describe_call(&call.name, &call.arguments, ToolDescriptionLength::Short),
                 self.tools.describe_call(&call.name, &call.arguments, ToolDescriptionLength::Full),
                 self.tools.target_path(&call.name, &call.arguments),
@@ -346,8 +365,9 @@ impl ChatSessionHandler {
                         Ok(GateOutcome::Proceed) => match self.execute_tool_call(
                             stack_id, config, tool_call_id, &name, &arguments, token, tx,
                         ).await {
-                            DispatchResult::Outcome(outcome) => Ok(GatedExec::Done { arguments, outcome }),
-                            DispatchResult::AbortPending      => Ok(GatedExec::AbortTurn),
+                            // Sub-agent batches never carry a file-write preview.
+                            DispatchResult::Outcome { outcome, .. } => Ok(GatedExec::Done { arguments, outcome }),
+                            DispatchResult::AbortPending            => Ok(GatedExec::AbortTurn),
                         },
                         Ok(GateOutcome::Rejected)      => Ok(GatedExec::Rejected),
                         Ok(GateOutcome::ChannelClosed) => Ok(GatedExec::AbortTurn),
@@ -371,7 +391,7 @@ impl ChatSessionHandler {
                 GatedExec::AbortTurn => abort = true,
                 GatedExec::Done { arguments, outcome } => {
                     match self.record_tool_outcome(
-                        *tool_call_id, &call.name, &arguments, outcome, em, Some(all_tool_calls),
+                        *tool_call_id, &call.name, &arguments, outcome, None, em, Some(all_tool_calls),
                     ).await? {
                         RecordFlow::Continue => {}
                         RecordFlow::Abort    => abort = true,

crates/skald-core/src/session/handler/outcome.rs

@@ -13,6 +13,7 @@ use crate::db::chat_llm_tools;
 use crate::tools::{is_file_write_tool, ExecutionOutcome};
 
 use super::ChatSessionHandler;
+use super::dispatch::WritePreview;
 use super::emitter::TurnEmitter;
 
 /// Whether the enclosing loop should keep going after an outcome is recorded.
@@ -38,6 +39,7 @@ impl ChatSessionHandler {
         tool_name:    &str,
         args:         &Value,
         outcome:      ExecutionOutcome,
+        preview:      Option<WritePreview>,
         em:           &TurnEmitter<'_>,
         accumulate:   Option<&mut Vec<ToolCallEvent>>,
     ) -> anyhow::Result<RecordFlow> {
@@ -48,6 +50,15 @@ impl ChatSessionHandler {
                 let kind = result.kind();
                 debug!(session_id = self.session_id, tool = %tool_name, tool_call_id, result_len = wire.len(), "tool done");
                 chat_llm_tools::complete(pool, tool_call_id, &wire, kind).await?;
+                // Persist a file-write's diff snapshot so it re-renders after a reload,
+                // and carry it on the event so an auto-allowed write shows the diff live.
+                let (preview_old, preview_new) = match preview {
+                    Some(WritePreview { old, new }) => {
+                        chat_llm_tools::set_preview(pool, tool_call_id, old.as_deref(), new.as_deref()).await?;
+                        (old, new)
+                    }
+                    None => (None, None),
+                };
                 if let Some(acc) = accumulate {
                     if is_file_write_tool(tool_name)
                         && let Some(p) = args["path"].as_str()
@@ -61,7 +72,7 @@ impl ChatSessionHandler {
                         status:    "done".to_string(),
                     });
                 }
-                em.tool_done(tool_call_id, wire, kind.to_string()).await;
+                em.tool_done(tool_call_id, wire, kind.to_string(), preview_old, preview_new).await;
                 Ok(RecordFlow::Continue)
             }
             ExecutionOutcome::Failed(msg) => {

crates/skald-core/src/session/handler/resume.rs

@@ -164,7 +164,7 @@ impl ChatSessionHandler {
             if is_error {
                 em.tool_error(parent_tool_call_id, result_str).await;
             } else {
-                em.tool_done(parent_tool_call_id, result_str, "string".to_string()).await;
+                em.tool_done(parent_tool_call_id, result_str, "string".to_string(), None, None).await;
             }
 
             // Now the parent is the deepest active stack.
@@ -305,11 +305,13 @@ impl ChatSessionHandler {
             // Re-dispatch it directly so the question is re-asked to the user.
             if tc.name == tn::ASK_USER_CLARIFICATION {
                 info!(session_id = self.session_id, tool_call_id = tc.id, "resume: re-asking clarification question");
+                let (display_name, icon) = self.tool_ui_meta(&tc.name, &args);
                 em.tool_start(
                     tc.id,
                     tc.message_id,
                     tc.name.clone(),
                     args.clone(),
+                    display_name, icon,
                     self.tools.describe_call(&tc.name, &args, ToolDescriptionLength::Short),
                     self.tools.describe_call(&tc.name, &args, ToolDescriptionLength::Full),
                     self.tools.target_path(&tc.name, &args),
@@ -318,7 +320,7 @@ impl ChatSessionHandler {
                 match result {
                     Ok(answer) => {
                         chat_llm_tools::complete(pool, tc.id, &answer, "string").await?;
-                        em.tool_done(tc.id, answer, "string".to_string()).await;
+                        em.tool_done(tc.id, answer, "string".to_string(), None, None).await;
                     }
                     Err(e) if matches!(e.downcast_ref::<super::AgentFlowSignal>(), Some(super::AgentFlowSignal::QuestionChannelClosed)) => {
                         // WS disconnected again mid-resume. Tool stays 'pending' — next resume re-asks.
@@ -335,11 +337,13 @@ impl ChatSessionHandler {
             }
 
             // Announce the tool is being re-tried.
+            let (display_name, icon) = self.tool_ui_meta(&tc.name, &args);
             em.tool_start(
                 tc.id,
                 tc.message_id,
                 tc.name.clone(),
                 args.clone(),
+                display_name, icon,
                 self.tools.describe_call(&tc.name, &args, ToolDescriptionLength::Short),
                 self.tools.describe_call(&tc.name, &args, ToolDescriptionLength::Full),
                 self.tools.target_path(&tc.name, &args),
@@ -358,7 +362,7 @@ impl ChatSessionHandler {
             if tc.name == tn::RESTART {
                 info!(session_id = self.session_id, tool_call_id = tc.id, "restart approved (resume) — marking done then exiting");
                 chat_llm_tools::complete(pool, tc.id, "Riavvio avviato.", "string").await?;
-                em.tool_done(tc.id, "Riavvio avviato.".to_string(), "string".to_string()).await;
+                em.tool_done(tc.id, "Riavvio avviato.".to_string(), "string".to_string(), None, None).await;
                 // Use _exit() to skip C atexit handlers (e.g. Metal GPU cleanup in
                 // whisper-rs/ggml, which aborts with SIGABRT and yields exit code 134
                 // instead of 255 — breaking the run.sh restart supervisor).
@@ -371,17 +375,18 @@ impl ChatSessionHandler {
             // `run_subtask`) through the recursive interception in `dispatch.rs`;
             // `build_execution` alone does not know them and would fail with
             // "Unknown tool: execute_task". Args are passed through unchanged.
-            let outcome = match self.execute_tool_call(
+            let (outcome, preview) = match self.execute_tool_call(
                 stack_id, config, tc.id, &tc.name, &args, token, tx,
             ).await {
-                super::dispatch::DispatchResult::Outcome(o) => o,
+                super::dispatch::DispatchResult::Outcome { outcome, preview } => (outcome, preview),
                 // Clarification WS channel closed mid-resume — leave the tool pending
                 // so the next resume re-asks (mirrors the live turn's AbortPending).
                 super::dispatch::DispatchResult::AbortPending => return Ok(true),
             };
-            // resume passes `None`: it does not accumulate ToolCallEvents nor re-emit
-            // FileChanged (only a live turn does). A /stop mid-resume returns Abort.
-            match self.record_tool_outcome(tc.id, &tc.name, &args, outcome, &em, None).await? {
+            // resume passes `None` for accumulate: it does not accumulate ToolCallEvents
+            // nor re-emit FileChanged (only a live turn does). The write preview IS
+            // persisted so a re-run write's diff survives. A /stop mid-resume returns Abort.
+            match self.record_tool_outcome(tc.id, &tc.name, &args, outcome, preview, &em, None).await? {
                 RecordFlow::Continue => {}
                 RecordFlow::Abort    => return Ok(true),
             }