From c11702c3d36ffa29a9147427e279531a3174148e Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Tue, 21 Jul 2026 23:39:41 +0100 Subject: [PATCH] tool card UI redesign: semantic icons, inline diff persistence, tool detail page, MCP-friendly titles --- crates/core-api/src/events.rs | 13 ++ crates/core-api/src/tool.rs | 24 +++ crates/skald-core/src/db/chat_llm_tools.rs | 60 +++++++- crates/skald-core/src/db/mcp_catalog.rs | 34 ++++- crates/skald-core/src/db/mod.rs | 8 + crates/skald-core/src/mcp/mod.rs | 39 +++++ crates/skald-core/src/mcp/provider.rs | 15 ++ .../src/session/handler/dispatch.rs | 61 +++++++- .../skald-core/src/session/handler/emitter.rs | 15 +- .../src/session/handler/llm_loop.rs | 34 ++++- .../skald-core/src/session/handler/outcome.rs | 13 +- .../skald-core/src/session/handler/resume.rs | 21 ++- crates/skald-core/src/tools/exec.rs | 2 + crates/skald-core/src/tools/fs/edit_file.rs | 2 + crates/skald-core/src/tools/fs/grep_files.rs | 2 + .../skald-core/src/tools/fs/insert_at_line.rs | 2 + crates/skald-core/src/tools/fs/list_files.rs | 2 + .../skald-core/src/tools/fs/memory_search.rs | 2 + crates/skald-core/src/tools/fs/read_file.rs | 2 + .../skald-core/src/tools/fs/replace_lines.rs | 2 + crates/skald-core/src/tools/fs/search_file.rs | 2 + crates/skald-core/src/tools/fs/write_file.rs | 2 + crates/skald-core/src/tools/image_generate.rs | 2 + crates/skald-core/src/tools/mod.rs | 65 ++++++++ src/frontend/api/marketplace.rs | 16 ++ src/frontend/api/mcp.rs | 3 + src/frontend/api/mod.rs | 2 + src/frontend/api/sessions.rs | 95 +++++++++++- web/app.js | 5 +- web/components/copilot-render.js | 108 +++++++++++++- web/components/copilot.js | 2 +- web/components/mobile-app.js | 33 ++++- web/components/shared/tool-detail-mobile.js | 80 ++++++++++ web/components/shared/tool-detail-view.js | 77 ++++++++++ web/components/sidebar.js | 2 +- web/components/tool-detail-page.js | 99 +++++++++++++ web/css/copilot-messages.css | 139 +++++++++++++++++- web/css/variables.css | 26 ++++ web/i18n/en.js | 5 + web/i18n/fr.js | 5 + web/i18n/it.js | 5 + web/index.html | 1 + web/lib/chat-session.js | 9 +- web/lib/open-tool.js | 17 +++ 44 files changed, 1103 insertions(+), 50 deletions(-) create mode 100644 web/components/shared/tool-detail-mobile.js create mode 100644 web/components/shared/tool-detail-view.js create mode 100644 web/components/tool-detail-page.js create mode 100644 web/lib/open-tool.js diff --git a/crates/core-api/src/events.rs b/crates/core-api/src/events.rs index e1bb435..dc6fc54 100644 --- a/crates/core-api/src/events.rs +++ b/crates/core-api/src/events.rs @@ -43,6 +43,12 @@ pub enum ServerEvent { message_id: i64, name: String, arguments: Value, + /// Friendly, static card title ("Edit File", "Read File", or an MCP tool's + /// resolved friendly name). Separate from `name` (the raw LLM function id). + display_name: String, + /// Semantic icon key (`edit`/`read`/`shell`/`mcp`/…) the frontend maps to a + /// glyph + accent color. Never a glyph — the core commits to meaning, not look. + icon: String, /// Concise human-readable label (≤60 chars): tool + primary argument. label_short: String, /// Verbose human-readable label (≤120 chars): tool + all meaningful arguments. @@ -62,6 +68,13 @@ pub enum ServerEvent { /// server; the frontend treats an absent/unknown value as plain text, so /// older clients degrade gracefully. result_type: String, + /// For a file-write tool: the file content before/after the write, so the + /// card renders the diff inline even for an auto-allowed write (one that + /// never emitted a `PendingWrite`). Absent for non-write tools. + #[serde(skip_serializing_if = "Option::is_none")] + preview_old: Option, + #[serde(skip_serializing_if = "Option::is_none")] + preview_new: Option, }, /// A tool call failed. DB status: error. ToolError { diff --git a/crates/core-api/src/tool.rs b/crates/core-api/src/tool.rs index 005e3f6..08d05d5 100644 --- a/crates/core-api/src/tool.rs +++ b/crates/core-api/src/tool.rs @@ -72,6 +72,30 @@ pub trait Tool: Send + Sync { self.name().to_string() } + /// Friendly, **static** display name for this tool ("Edit File", "Read File"), + /// shown as the card title in the chat UI — separate from [`name`](Self::name), + /// which stays the raw LLM function id. Several tools map to the same friendly + /// verb (e.g. `write_file`/`edit_file`/`insert_at_line` → "Edit File"). The + /// default returns the raw name so unmapped tools still render something. + fn display_name(&self) -> &str { + self.name() + } + + /// Semantic icon key for the chat card — **not** a glyph. The frontend maps the + /// key to a concrete icon + accent color (themeable), so the core commits to a + /// meaning, never a look. Known keys: `edit`, `read`, `list`, `search`, `shell`, + /// `subagent`, `image`, `config`, `introspection`. The default derives from + /// [`category`](Self::category). + fn icon(&self) -> &str { + match self.category() { + ToolCategory::Filesystem => "file", + ToolCategory::Shell => "shell", + ToolCategory::Subagent => "subagent", + ToolCategory::Introspection => "introspection", + ToolCategory::Config => "config", + } + } + /// If this invocation targets a single file the user can open in the file /// viewer, return its path (relative to the project root, or absolute). /// Tools that target a directory (list/grep) or no file at all return diff --git a/crates/skald-core/src/db/chat_llm_tools.rs b/crates/skald-core/src/db/chat_llm_tools.rs index e7bf07f..ee2b7ea 100644 --- a/crates/skald-core/src/db/chat_llm_tools.rs +++ b/crates/skald-core/src/db/chat_llm_tools.rs @@ -11,6 +11,13 @@ pub struct LlmToolCall { /// payload, e.g. MCP `structuredContent`). Drives frontend rendering. pub result_type: String, pub status: String, + /// For a file-write tool: the file content **before**/**after** the write, + /// captured at execution time so the diff renders inline in the chat card and + /// survives a page reload (it was previously only on the transient `PendingWrite` + /// event). `None` for non-write tools, an unreadable path, or content over the + /// size cap (no diff shown then). Only populated by `for_message` (history). + pub preview_old: Option, + pub preview_new: Option, } /// Inserts a tool call in `running` state and returns its id. @@ -55,6 +62,25 @@ pub async fn complete(pool: &SqlitePool, id: i64, result: &str, result_type: &st Ok(()) } +/// Persists a file-write tool's before/after snapshot (the diff preview) on its row. +/// Both `None` is a valid no-op state (non-write tool, unreadable path, or content +/// over the size cap). Separate from [`complete`] so the status/result write and the +/// preview write stay independent, and so it can run for both the live and resume paths. +pub async fn set_preview( + pool: &SqlitePool, + id: i64, + old: Option<&str>, + new: Option<&str>, +) -> anyhow::Result<()> { + sqlx::query("UPDATE chat_llm_tools SET preview_old = ?, preview_new = ? WHERE id = ?") + .bind(old) + .bind(new) + .bind(id) + .execute(pool) + .await?; + Ok(()) +} + pub async fn fail(pool: &SqlitePool, id: i64, error: &str) -> anyhow::Result<()> { sqlx::query( "UPDATE chat_llm_tools SET result = ?, status = 'failed' WHERE id = ?", @@ -117,13 +143,15 @@ pub async fn pending_for_stack( Ok(rows.into_iter().map(row_to_tool).collect()) } -/// All tool calls for a single assistant message, ordered chronologically. +/// All tool calls for a single assistant message, ordered chronologically. Unlike +/// [`pending_for_stack`], this also reads the diff-preview columns, so the history +/// projection can re-render a write's diff after a page reload. pub async fn for_message( pool: &SqlitePool, message_id: i64, ) -> anyhow::Result> { - let rows = sqlx::query_as::<_, (i64, i64, String, Option, Option, String, String)>( - "SELECT id, message_id, name, arguments, result, result_type, status + let rows = sqlx::query_as::<_, (i64, i64, String, Option, Option, String, String, Option, Option)>( + "SELECT id, message_id, name, arguments, result, result_type, status, preview_old, preview_new FROM chat_llm_tools WHERE message_id = ? ORDER BY id ASC", @@ -132,7 +160,28 @@ pub async fn for_message( .fetch_all(pool) .await?; - Ok(rows.into_iter().map(row_to_tool).collect()) + Ok(rows.into_iter() + .map(|(id, message_id, name, arguments, result, result_type, status, preview_old, preview_new)| { + LlmToolCall { id, message_id, name, arguments, result, result_type, status, preview_old, preview_new } + }) + .collect()) +} + +/// A single tool call by id, with its diff-preview columns. Backs the tool-detail +/// page (`GET /api/tools/{id}`). Returns `None` when the id is unknown in this pool. +pub async fn get(pool: &SqlitePool, id: i64) -> anyhow::Result> { + let row = sqlx::query_as::<_, (i64, i64, String, Option, Option, String, String, Option, Option)>( + "SELECT id, message_id, name, arguments, result, result_type, status, preview_old, preview_new + FROM chat_llm_tools + WHERE id = ?", + ) + .bind(id) + .fetch_optional(pool) + .await?; + + Ok(row.map(|(id, message_id, name, arguments, result, result_type, status, preview_old, preview_new)| { + LlmToolCall { id, message_id, name, arguments, result, result_type, status, preview_old, preview_new } + })) } fn row_to_tool( @@ -140,5 +189,6 @@ fn row_to_tool( i64, i64, String, Option, Option, String, String, ), ) -> LlmToolCall { - LlmToolCall { id, message_id, name, arguments, result, result_type, status } + // The resume path (`pending_for_stack`) never needs the diff preview. + LlmToolCall { id, message_id, name, arguments, result, result_type, status, preview_old: None, preview_new: None } } diff --git a/crates/skald-core/src/db/mcp_catalog.rs b/crates/skald-core/src/db/mcp_catalog.rs index f4b18bd..0fa1090 100644 --- a/crates/skald-core/src/db/mcp_catalog.rs +++ b/crates/skald-core/src/db/mcp_catalog.rs @@ -54,6 +54,11 @@ pub struct McpCatalogRow { pub icon_large_path: Option, pub friendly_name: Option, pub description: Option, + /// Manifest-declared friendly tool names: a JSON array of `{name, display_name}` + /// snapshotting the connector's `tools[]` block. Drives the UI card title for an + /// `mcp____` call (override > live MCP `title` > prettified name). + /// NULL when the manifest declares none. + pub tool_meta_json: Option, /// Marketplace build number — the **comparison key** for updates (a feed entry /// with a higher `version` than this installed one is "update available"). /// Monotonic per connector; `version_string`/`version_release_date` are display @@ -105,9 +110,26 @@ const SELECT: &str = script_path, config_schema_json, auth_kind, oauth_provider, oauth_scopes_json, \ deliver_json, role_filter, verify_command, \ verify_script_path, icon_small_path, icon_large_path, friendly_name, \ - description, version, version_string, version_release_date, created_at \ + description, tool_meta_json, version, version_string, version_release_date, created_at \ FROM mcp_catalog"; +/// Parses a catalog row's `tool_meta_json` (a `[{name, display_name}]` array) into +/// the `tool name → display title` override map the runtime feeds into +/// [`McpServerSpec::tool_titles`]. Empty on NULL or malformed JSON — the runtime then +/// falls back to the server's live MCP `title` and finally a prettified raw name. +pub fn parse_tool_titles(tool_meta_json: Option<&str>) -> HashMap { + #[derive(serde::Deserialize)] + struct ToolMeta { name: String, display_name: Option } + tool_meta_json + .and_then(|s| serde_json::from_str::>(s).ok()) + .map(|metas| { + metas.into_iter() + .filter_map(|m| m.display_name.map(|dn| (m.name, dn))) + .collect() + }) + .unwrap_or_default() +} + // ── Reads ──────────────────────────────────────────────────────────────────── pub async fn list(pool: &SqlitePool) -> Result> { @@ -167,6 +189,8 @@ pub struct UpsertCatalog<'a> { pub icon_large_path: Option<&'a str>, pub friendly_name: Option<&'a str>, pub description: Option<&'a str>, + /// JSON array of `{name, display_name}` snapshotting the manifest's `tools[]`. + pub tool_meta_json: Option, /// Versioning (from the feed). All three `None` for the admin's manual form, /// which COALESCEs them away rather than blanking an installed entry's version. pub version: Option, @@ -181,8 +205,8 @@ pub async fn upsert(pool: &SqlitePool, e: UpsertCatalog<'_>) -> Result { script_path, config_schema_json, auth_kind, oauth_provider, oauth_scopes_json, deliver_json, role_filter, verify_command, verify_script_path, icon_small_path, icon_large_path, friendly_name, description, - version, version_string, version_release_date) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18, ?19, ?20, ?21, ?22, ?23, ?24) + tool_meta_json, version, version_string, version_release_date) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18, ?19, ?20, ?21, ?22, ?23, ?24, ?25) ON CONFLICT(name) DO UPDATE SET scope = excluded.scope, source = excluded.source, @@ -208,6 +232,9 @@ pub async fn upsert(pool: &SqlitePool, e: UpsertCatalog<'_>) -> Result { icon_large_path = COALESCE(excluded.icon_large_path, mcp_catalog.icon_large_path), friendly_name = excluded.friendly_name, description = excluded.description, + -- Manifest tool titles are installer-owned like icons: COALESCE so the + -- admin's manual catalog form (which never sends them) can't blank them. + tool_meta_json = COALESCE(excluded.tool_meta_json, mcp_catalog.tool_meta_json), -- Version fields come from the feed on (re)install; the admin's manual -- form passes NULL, so COALESCE keeps the installed version rather than -- wiping it (same rationale as icons above). @@ -237,6 +264,7 @@ pub async fn upsert(pool: &SqlitePool, e: UpsertCatalog<'_>) -> Result { .bind(e.icon_large_path) .bind(e.friendly_name) .bind(e.description) + .bind(e.tool_meta_json) .bind(e.version) .bind(e.version_string) .bind(e.version_release_date) diff --git a/crates/skald-core/src/db/mod.rs b/crates/skald-core/src/db/mod.rs index b833930..2a6ca37 100644 --- a/crates/skald-core/src/db/mod.rs +++ b/crates/skald-core/src/db/mod.rs @@ -557,6 +557,7 @@ async fn create_registry_tables(pool: &SqlitePool) -> Result<()> { icon_large_path TEXT, friendly_name TEXT, description TEXT, + tool_meta_json TEXT, -- [{name,display_name}] friendly tool names from the manifest version INTEGER, -- marketplace build number: the update-comparison key version_string TEXT, -- semver, display only version_release_date TEXT, -- ISO date, display only @@ -569,6 +570,8 @@ async fn create_registry_tables(pool: &SqlitePool) -> Result<()> { ensure_column(pool, "mcp_catalog", "oauth_provider", "TEXT").await?; ensure_column(pool, "mcp_catalog", "oauth_scopes_json", "TEXT").await?; ensure_column(pool, "mcp_catalog", "deliver_json", "TEXT").await?; + // Manifest-declared friendly tool names (UI card titles) — additive. + ensure_column(pool, "mcp_catalog", "tool_meta_json", "TEXT").await?; // Versioning columns are additive: the installed `version` integer is compared // against the feed's to surface "update available" in the marketplace UI. ensure_column(pool, "mcp_catalog", "version", "INTEGER").await?; @@ -745,11 +748,16 @@ pub async fn create_owner_tables(pool: &SqlitePool) -> Result<()> { result TEXT, status TEXT NOT NULL DEFAULT 'running' CHECK(status IN ('running', 'pending', 'done', 'failed', 'cancelled', 'rejected')), result_type TEXT NOT NULL DEFAULT 'string' CHECK(result_type IN ('string', 'json')), + preview_old TEXT, -- file-write diff: content before the write + preview_new TEXT, -- file-write diff: content after the write created_at TEXT NOT NULL DEFAULT (datetime('now')) )", ) .execute(pool) .await?; + // Diff-preview columns are additive — reach an already-created table in place. + ensure_column(pool, "chat_llm_tools", "preview_old", "TEXT").await?; + ensure_column(pool, "chat_llm_tools", "preview_new", "TEXT").await?; sqlx::query( "CREATE INDEX IF NOT EXISTS idx_stack_session ON chat_sessions_stack(session_id)", diff --git a/crates/skald-core/src/mcp/mod.rs b/crates/skald-core/src/mcp/mod.rs index 7bb0961..98d540d 100644 --- a/crates/skald-core/src/mcp/mod.rs +++ b/crates/skald-core/src/mcp/mod.rs @@ -44,6 +44,10 @@ pub struct McpManager { servers: RwLock>>, errors: RwLock>, descriptions: RwLock>>, + /// Per-server manifest-declared friendly tool names (`server → tool → title`), + /// the authoritative override for a tool's UI display name (`tool_display_name`). + /// Populated from each spec's `tool_titles` at connect, forgotten on stop. + titles: RwLock>>, notification_tx: mpsc::UnboundedSender, /// Feeds per-server diagnostic lines (stderr, `notifications/message`, /// lifecycle) to the `logs::log_consumer`, which writes `logs/mcp/.log`. @@ -69,6 +73,7 @@ impl McpManager { servers: RwLock::new(HashMap::new()), errors: RwLock::new(HashMap::new()), descriptions: RwLock::new(HashMap::new()), + titles: RwLock::new(HashMap::new()), notification_tx, log_tx, elicitation_handler: RwLock::new(None), @@ -179,8 +184,10 @@ impl McpManager { } { let mut descs = self.descriptions.write().unwrap(); + let mut titles = self.titles.write().unwrap(); for spec in &specs { descs.insert(spec.config.name.clone(), spec.description.clone()); + titles.insert(spec.config.name.clone(), spec.tool_titles.clone()); } } if boot { @@ -256,6 +263,7 @@ impl McpManager { self.log_lifecycle(&name, format!("connected — {} tool(s)", tool_names.len())); self.errors.write().unwrap().remove(&name); self.descriptions.write().unwrap().insert(name.clone(), spec.description); + self.titles.write().unwrap().insert(name.clone(), spec.tool_titles); self.servers.write().unwrap().insert(name, client); Ok(tool_names) } @@ -266,6 +274,7 @@ impl McpManager { self.servers.write().unwrap().remove(name); self.errors.write().unwrap().remove(name); self.descriptions.write().unwrap().remove(name); + self.titles.write().unwrap().remove(name); } /// Stops **every** running server (each dropped client → `kill_on_drop` kills @@ -277,6 +286,7 @@ impl McpManager { self.servers.write().unwrap().clear(); self.errors.write().unwrap().clear(); self.descriptions.write().unwrap().clear(); + self.titles.write().unwrap().clear(); } /// Whether a server by this name currently has a live connection in the @@ -299,6 +309,18 @@ impl McpManager { .collect() } + /// Best friendly name for a tool for UI display: the manifest-declared override + /// (`tool_titles`) wins, else the server's live MCP `title` (2025-06-18+), else + /// `None` — the caller falls back to a prettified raw name. Cheap: an O(tools) + /// scan per call, run once per tool-call event. + pub fn tool_display_name(&self, server: &str, tool: &str) -> Option { + if let Some(t) = self.titles.read().unwrap().get(server).and_then(|m| m.get(tool)) { + return Some(t.clone()); + } + self.servers.read().unwrap().get(server) + .and_then(|s| s.tools().iter().find(|t| t.name == tool).and_then(|t| t.title.clone())) + } + pub fn server_descriptions(&self) -> HashMap> { self.descriptions.read().unwrap().clone() } @@ -407,6 +429,11 @@ impl McpManager { pub struct McpServerSpec { pub config: McpServerConfig, pub description: Option, + /// Manifest-declared friendly tool names (`tool name → display title`), the + /// authoritative override for a connector's UI display names. Empty for globals + /// and for per-user rows with no catalog `tool_meta_json`; the runtime then + /// falls back to the server's live MCP `title` and finally a prettified name. + pub tool_titles: HashMap, } fn transport_of(s: &str) -> McpTransport { @@ -532,6 +559,7 @@ pub fn global_row_spec(row: &crate::db::mcp_global_servers::McpGlobalServerRow) launch_in: None, }, description: row.description.clone(), + tool_titles: HashMap::new(), } } @@ -616,6 +644,9 @@ pub fn user_row_spec( // A per-user connector's description falls back to its catalog name; the // catalog's friendly description can be injected by the caller if richer. description: row.catalog_name.clone(), + // Manifest tool titles are loaded from the catalog by `user_row_spec_resolved`, + // which has registry access; the bare sync builder leaves them empty. + tool_titles: HashMap::new(), } } @@ -632,6 +663,14 @@ pub async fn user_row_spec_resolved( registry: &SqlitePool, ) -> McpServerSpec { let mut spec = user_row_spec(row, container); + // Manifest-declared friendly tool names (UI card titles): snapshot the catalog's + // `tool_meta_json` for this activation so `tool_display_name` can override the raw + // name. Best-effort — a missing/failed lookup just leaves the live `title` path. + if let Some(catalog_name) = row.catalog_name.as_deref() { + if let Ok(Some(entry)) = crate::db::mcp_catalog::get_by_name(registry, catalog_name).await { + spec.tool_titles = crate::db::mcp_catalog::parse_tool_titles(entry.tool_meta_json.as_deref()); + } + } if let (Some(provider), Some(deliver), Some(refresh)) = (row.oauth_provider.as_deref(), row.deliver(), row.api_key.as_deref()) { diff --git a/crates/skald-core/src/mcp/provider.rs b/crates/skald-core/src/mcp/provider.rs index dcedf5f..d7ca767 100644 --- a/crates/skald-core/src/mcp/provider.rs +++ b/crates/skald-core/src/mcp/provider.rs @@ -25,6 +25,10 @@ pub trait McpProvider: Send + Sync { fn tools_for(&self, names: &[String]) -> Vec; fn server_descriptions(&self) -> HashMap>; fn server_infos(&self) -> Vec; + /// Best friendly name for a `server`/`tool` pair for the chat card (manifest + /// override > live MCP `title` > `None`, the caller then prettifies the raw + /// name). Routed to whichever runtime owns the server. + fn tool_display_name(&self, server: &str, tool: &str) -> Option; async fn call(&self, server: &str, tool: &str, args: Value) -> Result; } @@ -34,6 +38,9 @@ impl McpProvider for McpManager { fn tools_for(&self, names: &[String]) -> Vec { McpManager::tools_for(self, names) } fn server_descriptions(&self) -> HashMap> { McpManager::server_descriptions(self) } fn server_infos(&self) -> Vec { McpManager::server_infos(self) } + fn tool_display_name(&self, server: &str, tool: &str) -> Option { + McpManager::tool_display_name(self, server, tool) + } async fn call(&self, server: &str, tool: &str, args: Value) -> Result { McpManager::call(self, server, tool, args).await } @@ -97,6 +104,14 @@ impl McpProvider for UserMcpView { v } + fn tool_display_name(&self, server: &str, tool: &str) -> Option { + if self.accessible_global.contains(server) { + self.global.tool_display_name(server, tool) + } else { + self.user.tool_display_name(server, tool) + } + } + async fn call(&self, server: &str, tool: &str, args: Value) -> Result { if self.accessible_global.contains(server) { self.global.call(server, tool, args).await diff --git a/crates/skald-core/src/session/handler/dispatch.rs b/crates/skald-core/src/session/handler/dispatch.rs index 3adbab6..3c71065 100644 --- a/crates/skald-core/src/session/handler/dispatch.rs +++ b/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, + pub new: Option, +} + +/// 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) -> Option { + 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, + }, /// 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 } } } diff --git a/crates/skald-core/src/session/handler/emitter.rs b/crates/skald-core/src/session/handler/emitter.rs index 7427c1a..a5304f4 100644 --- a/crates/skald-core/src/session/handler/emitter.rs +++ b/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, ) { 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, + preview_new: Option, + ) { + 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) { diff --git a/crates/skald-core/src/session/handler/llm_loop.rs b/crates/skald-core/src/session/handler/llm_loop.rs index 7a226b3..6623688 100644 --- a/crates/skald-core/src/session/handler/llm_loop.rs +++ b/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, diff --git a/crates/skald-core/src/session/handler/outcome.rs b/crates/skald-core/src/session/handler/outcome.rs index a6becdc..c820afb 100644 --- a/crates/skald-core/src/session/handler/outcome.rs +++ b/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, em: &TurnEmitter<'_>, accumulate: Option<&mut Vec>, ) -> anyhow::Result { @@ -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) => { diff --git a/crates/skald-core/src/session/handler/resume.rs b/crates/skald-core/src/session/handler/resume.rs index d10b053..5c1e8fa 100644 --- a/crates/skald-core/src/session/handler/resume.rs +++ b/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::(), 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), } diff --git a/crates/skald-core/src/tools/exec.rs b/crates/skald-core/src/tools/exec.rs index 23d3420..6252463 100644 --- a/crates/skald-core/src/tools/exec.rs +++ b/crates/skald-core/src/tools/exec.rs @@ -26,6 +26,8 @@ pub struct ExecuteCmd; impl Tool for ExecuteCmd { fn name(&self) -> &str { crate::tools::tool_names::EXECUTE_CMD } + fn display_name(&self) -> &str { "Run Command" } + fn icon(&self) -> &str { "shell" } fn category(&self) -> crate::tools::ToolCategory { crate::tools::ToolCategory::Shell } fn description(&self) -> &str { diff --git a/crates/skald-core/src/tools/fs/edit_file.rs b/crates/skald-core/src/tools/fs/edit_file.rs index be307c3..192fdeb 100644 --- a/crates/skald-core/src/tools/fs/edit_file.rs +++ b/crates/skald-core/src/tools/fs/edit_file.rs @@ -114,6 +114,8 @@ impl EditFile { impl Tool for EditFile { fn name(&self) -> &str { "edit_file" } + fn display_name(&self) -> &str { "Edit File" } + fn icon(&self) -> &str { "edit" } fn category(&self) -> crate::tools::ToolCategory { crate::tools::ToolCategory::Filesystem } fn description(&self) -> &str { diff --git a/crates/skald-core/src/tools/fs/grep_files.rs b/crates/skald-core/src/tools/fs/grep_files.rs index 7461847..8e47041 100644 --- a/crates/skald-core/src/tools/fs/grep_files.rs +++ b/crates/skald-core/src/tools/fs/grep_files.rs @@ -16,6 +16,8 @@ impl GrepFiles { impl Tool for GrepFiles { fn name(&self) -> &str { "grep_files" } + fn display_name(&self) -> &str { "Search" } + fn icon(&self) -> &str { "search" } fn category(&self) -> crate::tools::ToolCategory { crate::tools::ToolCategory::Filesystem } fn description(&self) -> &str { diff --git a/crates/skald-core/src/tools/fs/insert_at_line.rs b/crates/skald-core/src/tools/fs/insert_at_line.rs index 39735d8..ac5d8c4 100644 --- a/crates/skald-core/src/tools/fs/insert_at_line.rs +++ b/crates/skald-core/src/tools/fs/insert_at_line.rs @@ -48,6 +48,8 @@ fn apply_insert(text: &str, args: &Value, display: &str) -> Result<(String, Stri impl Tool for InsertAtLine { fn name(&self) -> &str { "insert_at_line" } + fn display_name(&self) -> &str { "Edit File" } + fn icon(&self) -> &str { "edit" } fn category(&self) -> crate::tools::ToolCategory { crate::tools::ToolCategory::Filesystem } fn description(&self) -> &str { diff --git a/crates/skald-core/src/tools/fs/list_files.rs b/crates/skald-core/src/tools/fs/list_files.rs index 06d86df..0957b9a 100644 --- a/crates/skald-core/src/tools/fs/list_files.rs +++ b/crates/skald-core/src/tools/fs/list_files.rs @@ -25,6 +25,8 @@ impl ListFiles { impl Tool for ListFiles { fn name(&self) -> &str { "list_files" } + fn display_name(&self) -> &str { "List Files" } + fn icon(&self) -> &str { "list" } fn category(&self) -> crate::tools::ToolCategory { crate::tools::ToolCategory::Filesystem } fn description(&self) -> &str { diff --git a/crates/skald-core/src/tools/fs/memory_search.rs b/crates/skald-core/src/tools/fs/memory_search.rs index 6344ae4..2ba6b3b 100644 --- a/crates/skald-core/src/tools/fs/memory_search.rs +++ b/crates/skald-core/src/tools/fs/memory_search.rs @@ -46,6 +46,8 @@ fn render_hits(store: &str, hits: &[MemoryHit], out: &mut String) { impl Tool for MemorySearch { fn name(&self) -> &str { "memory_search" } + fn display_name(&self) -> &str { "Search Memory" } + fn icon(&self) -> &str { "search" } fn category(&self) -> crate::tools::ToolCategory { crate::tools::ToolCategory::Introspection } fn description(&self) -> &str { diff --git a/crates/skald-core/src/tools/fs/read_file.rs b/crates/skald-core/src/tools/fs/read_file.rs index 3be4074..6f57303 100644 --- a/crates/skald-core/src/tools/fs/read_file.rs +++ b/crates/skald-core/src/tools/fs/read_file.rs @@ -49,6 +49,8 @@ fn number_lines(content: &str, start: usize, end_line: Option, limit: Opt impl Tool for ReadFile { fn name(&self) -> &str { "read_file" } + fn display_name(&self) -> &str { "Read File" } + fn icon(&self) -> &str { "read" } fn category(&self) -> crate::tools::ToolCategory { crate::tools::ToolCategory::Filesystem } fn description(&self) -> &str { diff --git a/crates/skald-core/src/tools/fs/replace_lines.rs b/crates/skald-core/src/tools/fs/replace_lines.rs index eb6c808..1d8aefb 100644 --- a/crates/skald-core/src/tools/fs/replace_lines.rs +++ b/crates/skald-core/src/tools/fs/replace_lines.rs @@ -55,6 +55,8 @@ fn apply_replace(content: &str, args: &Value, display: &str) -> Result<(String, impl Tool for ReplaceLines { fn name(&self) -> &str { "replace_lines" } + fn display_name(&self) -> &str { "Edit File" } + fn icon(&self) -> &str { "edit" } fn category(&self) -> crate::tools::ToolCategory { crate::tools::ToolCategory::Filesystem } fn description(&self) -> &str { diff --git a/crates/skald-core/src/tools/fs/search_file.rs b/crates/skald-core/src/tools/fs/search_file.rs index af6dce8..a71e4ab 100644 --- a/crates/skald-core/src/tools/fs/search_file.rs +++ b/crates/skald-core/src/tools/fs/search_file.rs @@ -66,6 +66,8 @@ fn render_search(text: &str, args: &Value, display: &str) -> Result { impl Tool for SearchFile { fn name(&self) -> &str { "search_file" } + fn display_name(&self) -> &str { "Search" } + fn icon(&self) -> &str { "search" } fn category(&self) -> crate::tools::ToolCategory { crate::tools::ToolCategory::Filesystem } fn description(&self) -> &str { diff --git a/crates/skald-core/src/tools/fs/write_file.rs b/crates/skald-core/src/tools/fs/write_file.rs index f791865..a2f4804 100644 --- a/crates/skald-core/src/tools/fs/write_file.rs +++ b/crates/skald-core/src/tools/fs/write_file.rs @@ -21,6 +21,8 @@ impl WriteFile { impl Tool for WriteFile { fn name(&self) -> &str { "write_file" } + fn display_name(&self) -> &str { "Edit File" } + fn icon(&self) -> &str { "edit" } fn category(&self) -> crate::tools::ToolCategory { crate::tools::ToolCategory::Filesystem } fn description(&self) -> &str { diff --git a/crates/skald-core/src/tools/image_generate.rs b/crates/skald-core/src/tools/image_generate.rs index cb7e743..038c06b 100644 --- a/crates/skald-core/src/tools/image_generate.rs +++ b/crates/skald-core/src/tools/image_generate.rs @@ -47,6 +47,8 @@ pub struct ImageGenerateTool { impl Tool for ImageGenerateTool { fn name(&self) -> &str { "image_generate" } + fn display_name(&self) -> &str { "Generate Image" } + fn icon(&self) -> &str { "image" } fn category(&self) -> ToolCategory { ToolCategory::Config } fn description(&self) -> &str { diff --git a/crates/skald-core/src/tools/mod.rs b/crates/skald-core/src/tools/mod.rs index b66300d..4df59b8 100644 --- a/crates/skald-core/src/tools/mod.rs +++ b/crates/skald-core/src/tools/mod.rs @@ -62,6 +62,33 @@ pub use core_api::tool::{ pub const MAX_LABEL_SHORT: usize = 60; pub const MAX_LABEL_FULL: usize = 120; +/// UI metadata for one tool call — the friendly card title plus a **semantic** icon +/// key (never a glyph; the frontend maps the key to an icon + accent color). Computed +/// by [`ToolRegistry::display_meta`], the single seam shared by the live WS event and +/// the history projection so the two can't drift. +#[derive(Debug, Clone)] +pub struct ToolUiMeta { + pub display_name: String, + pub icon: String, +} + +/// Turns a raw tool id (`snake_case` / `kebab-case`) into a Title-Cased phrase for a +/// UI label when no friendly name is declared — `list_recent_files` → "List Recent +/// Files". The last-resort fallback for MCP and unknown tools. +pub fn prettify_tool_name(name: &str) -> String { + name.split(|c| c == '_' || c == '-') + .filter(|s| !s.is_empty()) + .map(|w| { + let mut chars = w.chars(); + match chars.next() { + Some(f) => f.to_uppercase().collect::() + chars.as_str(), + None => String::new(), + } + }) + .collect::>() + .join(" ") +} + /// Registry of all available tools. pub struct ToolRegistry { tools: HashMap>, @@ -173,6 +200,44 @@ impl ToolRegistry { name.to_string() } + /// Friendly card metadata (display name + semantic icon key) for any tool call, + /// including non-registry tools. Registry tools delegate to + /// [`Tool::display_name`]/[`Tool::icon`]; sub-agent and interface tools are + /// handled inline; an `mcp__server__tool` name gets `icon = "mcp"` and a + /// prettified tool name — the caller (which holds the [`McpProvider`]) may then + /// override `display_name` with the connector's resolved friendly name. + /// + /// [`McpProvider`]: crate::mcp::provider::McpProvider + pub fn display_meta(&self, name: &str, args: &Value) -> ToolUiMeta { + if let Some(tool) = self.tools.get(name) { + return ToolUiMeta { + display_name: tool.display_name().to_string(), + icon: tool.icon().to_string(), + }; + } + // Sub-agent delegation tools (InterfaceTools, not in the registry). + if name == tool_names::EXECUTE_TASK + || name == tool_names::EXECUTE_SUBTASK + || name == "run_subtask" + { + let dn = match args["agent_id"].as_str().map(str::trim).filter(|s| !s.is_empty()) { + Some(a) => format!("Sub-agent: {a}"), + None => "Sub-agent".to_string(), + }; + return ToolUiMeta { display_name: dn, icon: "subagent".to_string() }; + } + if name == tool_names::SHOW_FILE_TO_USER { + return ToolUiMeta { display_name: "Show File".to_string(), icon: "read".to_string() }; + } + // MCP tool `mcp__server__tool`: default to a prettified tool name; the caller + // overrides with the connector's manifest/`title` friendly name when known. + if let Some(rest) = name.strip_prefix("mcp__") { + let tool = rest.split_once("__").map(|(_, t)| t).unwrap_or(rest); + return ToolUiMeta { display_name: prettify_tool_name(tool), icon: "mcp".to_string() }; + } + ToolUiMeta { display_name: prettify_tool_name(name), icon: "tool".to_string() } + } + /// Returns the category of a registered tool, or `None` for unknown tools /// (MCP tools, interface tools, call_agent, etc.). pub fn category_of(&self, name: &str) -> Option { diff --git a/src/frontend/api/marketplace.rs b/src/frontend/api/marketplace.rs index 01415a6..8b2d36e 100644 --- a/src/frontend/api/marketplace.rs +++ b/src/frontend/api/marketplace.rs @@ -201,6 +201,16 @@ struct VerifySpec { #[serde(default)] timeout_secs: Option, } +/// One entry of the manifest's optional `tools[]` block: a friendly display name +/// for a raw MCP tool. Snapshotted into `mcp_catalog.tool_meta_json` and used as the +/// authoritative UI card title (override > live MCP `title` > prettified raw name). +/// Icons are not per-tool — a connector's own icon covers all its tools. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +struct ToolMeta { + name: String, + #[serde(default)] display_name: Option, +} + #[derive(Debug, Clone, Default, Deserialize)] struct Manifest { #[serde(default)] name: Option, @@ -226,6 +236,8 @@ struct Manifest { #[serde(default)] env: Vec, /// Optional verify-before-save command. #[serde(default)] verify: Option, + /// Optional friendly display names for the connector's tools (UI card titles). + #[serde(default)] tools: Vec, } #[derive(Debug, Clone)] @@ -762,6 +774,10 @@ pub async fn install( .llm_short_description .as_deref() .or(h.entry.user_description.as_deref()), + // Snapshot the manifest's friendly tool names for the UI card titles. + tool_meta_json: (!h.manifest.tools.is_empty()) + .then(|| serde_json::to_string(&h.manifest.tools).ok()) + .flatten(), // Snapshot the feed's version so a later listing can compare it against a // newer feed and surface "update available". Manifest wins over index. version: h.manifest.version.or(h.entry.version), diff --git a/src/frontend/api/mcp.rs b/src/frontend/api/mcp.rs index bb90e06..754a2fe 100644 --- a/src/frontend/api/mcp.rs +++ b/src/frontend/api/mcp.rs @@ -322,6 +322,9 @@ pub async fn catalog_upsert( icon_large_path: None, friendly_name: body.friendly_name.as_deref(), description: body.description.as_deref(), + // Friendly tool names are the feed's to set; the manual form leaves them + // untouched (COALESCE in `upsert`). + tool_meta_json: None, // Versioning is the feed's to set (marketplace install); the manual form // leaves it untouched (COALESCE in `upsert`). version: None, diff --git a/src/frontend/api/mod.rs b/src/frontend/api/mod.rs index 28f0753..b5b22e9 100644 --- a/src/frontend/api/mod.rs +++ b/src/frontend/api/mod.rs @@ -68,6 +68,8 @@ pub fn router() -> Router> { // File attachments: streamed to disk, so the default body-size limit is // disabled on this route only. .route("/{source}/uploads", post(uploads::upload).layer(DefaultBodyLimit::disable())) + // Full execution detail for one tool call (the tool-detail page). + .route("/tools/{tool_call_id}", get(sessions::tool_detail)) // Source-agnostic approval resolve, keyed by globally-unique tool_call_id. .route("/tools/{tool_call_id}/resolve", post(sessions::resolve_tool)) // Back-compat alias for older web clients that POST to /web/tools/... diff --git a/src/frontend/api/sessions.rs b/src/frontend/api/sessions.rs index 732106b..e0be7ef 100644 --- a/src/frontend/api/sessions.rs +++ b/src/frontend/api/sessions.rs @@ -17,6 +17,7 @@ use std::sync::Arc; use skald_core::skald::{Skald, UserContext}; use skald_core::session::handler::ApprovalDecision; use skald_core::approval::ApprovalManager; +use skald_core::mcp::{McpManager, parse_mcp_tool_name}; use skald_core::tools::{ToolRegistry, ToolDescriptionLength, tool_names as tn}; use super::{ApiError, guard::AuthUser, require_context}; @@ -112,11 +113,35 @@ async fn messages_for_source(skald: &Arc, ctx: &UserContext, source: &str .collect(); let mut items: Vec = Vec::new(); - build_items(db, skald.tools(), &ctx.approval, &main_stack, &subagent_map, &mut items).await?; + build_items(db, skald.tools(), skald.mcp(), &ctx.user_mcp, &ctx.approval, &main_stack, &subagent_map, &mut items).await?; Ok(Json(items)) } +/// Card metadata (friendly display name + semantic icon key) for a persisted tool +/// call, mirroring the live loop's `tool_ui_meta`: the registry seam, with the MCP +/// display-name override layered on (per-user runtime first, then global) for an +/// `mcp__server__tool` name. History resolves against the running runtimes, so the +/// friendly name survives a refresh; a stopped server falls back to the prettified +/// name the seam produced. +fn tool_card_meta( + tools: &ToolRegistry, + global_mcp: &McpManager, + user_mcp: &McpManager, + name: &str, + args: &Value, +) -> (String, String) { + let mut meta = tools.display_meta(name, args); + if let Some((server, tool)) = parse_mcp_tool_name(name) { + if let Some(friendly) = user_mcp.tool_display_name(server, tool) + .or_else(|| global_mcp.tool_display_name(server, tool)) + { + meta.display_name = friendly; + } + } + (meta.display_name, meta.icon) +} + // ── POST /api/tools/:tool_call_id/resolve — approve/reject a pending tool ───── // (source-agnostic; /api/web/tools/... kept as a back-compat alias) @@ -269,6 +294,56 @@ pub async fn resolve_tool( } } +// ── GET /api/tools/:tool_call_id — full execution detail for the detail page ── + +/// One tool call's full record — input args, result, and (for a file-write) the +/// before/after snapshot — for the dedicated tool-detail page. Read from the +/// caller's own pool (the `tool_call_id` is local to it), so ownership is implicit. +pub async fn tool_detail( + State(skald): State>, + Extension(auth): Extension, + Path(p): Path, +) -> Result, ApiError> { + let ctx = require_context(&skald, &auth.user_id).await?; + let tc = chat_llm_tools::get(&ctx.pool, p.tool_call_id).await? + .ok_or_else(|| ApiError::not_found(format!("tool_call_id {} not found", p.tool_call_id)))?; + + let args: Value = tc.arguments.as_deref() + .and_then(|s| serde_json::from_str(s).ok()) + .unwrap_or(Value::Null); + + // Normalize the DB status the same way as the history projection: an interrupted + // `running` row surfaces as an error, terminal states pass through. + let (status, result, error) = match tc.status.as_str() { + "done" => ("done", tc.result.clone(), None), + "pending" => ("pending", None, None), + "running" => ("error", None, Some("Interrupted.".to_string())), + "cancelled" => ("cancelled", None, tc.result.clone()), + "rejected" => ("rejected", None, tc.result.clone()), + _ => ("error", None, tc.result.clone()), + }; + + let (display_name, icon) = tool_card_meta(skald.tools(), skald.mcp(), &ctx.user_mcp, &tc.name, &args); + let label_full = skald.tools().describe_call(&tc.name, &args, ToolDescriptionLength::Full); + let target_path = skald.tools().target_path(&tc.name, &args); + + Ok(Json(json!({ + "tool_call_id": tc.id, + "name": tc.name, + "display_name": display_name, + "icon": icon, + "label_full": label_full, + "path": target_path, + "arguments": args, + "status": status, + "result": result, + "result_type": tc.result_type, + "error": error, + "preview_old": tc.preview_old, + "preview_new": tc.preview_new, + }))) +} + // ── GET /api/sessions — list sessions by source (paginated) ────────────────── #[derive(Deserialize)] @@ -473,10 +548,16 @@ fn build_debug_items<'a>( let label_short = tools.describe_call(&tc.name, &args, ToolDescriptionLength::Short); let label_full = tools.describe_call(&tc.name, &args, ToolDescriptionLength::Full); let target_path = tools.target_path(&tc.name, &args); + // Debug view has no MCP runtime handy: use the registry seam + // directly (MCP tools fall back to a prettified name + `mcp` + // icon, no live/manifest title override). + let meta = tools.display_meta(&tc.name, &args); items.push(json!({ "kind": "tool", "tool_call_id": tc.id, "name": tc.name, + "display_name": meta.display_name, + "icon": meta.icon, "label_short": label_short, "label_full": label_full, "path": target_path, @@ -485,6 +566,8 @@ fn build_debug_items<'a>( "result": result, "result_type": tc.result_type, "error": error, + "preview_old": tc.preview_old, + "preview_new": tc.preview_new, })); if let Some(sub_stack) = subagent_map.get(&tc.id) { @@ -513,9 +596,12 @@ fn build_debug_items<'a>( // ── Recursive message-tree builder ──────────────────────────────────────────── +#[allow(clippy::too_many_arguments)] fn build_items<'a>( db: &'a SqlitePool, tools: &'a ToolRegistry, + global_mcp: &'a McpManager, + user_mcp: &'a McpManager, approval: &'a ApprovalManager, stack: &'a SessionStack, subagent_map: &'a HashMap, @@ -601,11 +687,14 @@ fn build_items<'a>( let label_short = tools.describe_call(&tc.name, &args, ToolDescriptionLength::Short); let label_full = tools.describe_call(&tc.name, &args, ToolDescriptionLength::Full); let target_path = tools.target_path(&tc.name, &args); + let (display_name, icon) = tool_card_meta(tools, global_mcp, user_mcp, &tc.name, &args); items.push(json!({ "kind": "tool", "tool_call_id": tc.id, "request_id": request_id, "name": tc.name, + "display_name": display_name, + "icon": icon, "label_short": label_short, "label_full": label_full, "path": target_path, @@ -614,6 +703,8 @@ fn build_items<'a>( "result": result, "result_type": tc.result_type, "error": error, + "preview_old": tc.preview_old, + "preview_new": tc.preview_new, })); if let Some(sub_stack) = subagent_map.get(&tc.id) { @@ -624,7 +715,7 @@ fn build_items<'a>( "depth": sub_stack.depth, "done": true, })); - build_items(db, tools, approval, sub_stack, subagent_map, items).await?; + build_items(db, tools, global_mcp, user_mcp, approval, sub_stack, subagent_map, items).await?; items.push(json!({ "kind": "agent_end", "agent_id": sub_stack.agent_id, diff --git a/web/app.js b/web/app.js index b066d70..c8a8025 100644 --- a/web/app.js +++ b/web/app.js @@ -32,11 +32,13 @@ import { SessionDetailPage } from './components/session-detail.js'; import { TicSessionsPage } from './components/tic-sessions.js'; import { ProjectsPage } from './components/projects/index.js'; import { FileViewerPage } from './components/file-viewer-page.js'; +import { ToolDetailPage } from './components/tool-detail-page.js'; import { SetupPage } from './components/setup-page.js'; import { LoginPage } from './components/login-page.js'; -// Register the global `openFile(path)` helper (window.openFile → location.hash). +// Register the global `openFile(path)` / `openToolDetail(id)` helpers. import './lib/open-file.js'; +import './lib/open-tool.js'; import { initI18n } from './lib/i18n.js'; customElements.define('app-topbar', AppTopbar); @@ -73,6 +75,7 @@ customElements.define('session-detail-page', SessionDetailPage); customElements.define('tic-sessions-page', TicSessionsPage); customElements.define('projects-page', ProjectsPage); customElements.define('file-viewer-page', FileViewerPage); +customElements.define('tool-detail-page', ToolDetailPage); customElements.define('setup-page', SetupPage); customElements.define('login-page', LoginPage); diff --git a/web/components/copilot-render.js b/web/components/copilot-render.js index b4c17f7..7cfadae 100644 --- a/web/components/copilot-render.js +++ b/web/components/copilot-render.js @@ -2,7 +2,93 @@ import { html, nothing } from 'lit'; import { unsafeHTML } from 'lit/directives/unsafe-html.js'; import { renderMarkdown } from '../lib/base.js'; import { openFile } from '../lib/open-file.js'; +import { openToolDetail } from '../lib/open-tool.js'; import { t } from '../lib/i18n.js'; +import { connectorIconUrl } from './shared/connector-common.js'; + +// ── Tool icons ───────────────────────────────────────────────────────────────── + +/** + * Maps a tool's semantic `icon` key (from the backend `Tool::icon`) to a Bootstrap + * glyph + a CSS accent class. The key commits to meaning; the look lives here and in + * `copilot-messages.css` (theme-aware, no hardcoded colors). Unknown keys fall back + * to the generic wrench. + */ +const TOOL_ICON = { + edit: { glyph: 'bi-pencil-square', cls: 'tool-ico--edit' }, + read: { glyph: 'bi-file-earmark-text', cls: 'tool-ico--read' }, + list: { glyph: 'bi-folder2-open', cls: 'tool-ico--list' }, + search: { glyph: 'bi-search', cls: 'tool-ico--search' }, + shell: { glyph: 'bi-terminal', cls: 'tool-ico--shell' }, + subagent: { glyph: 'bi-diagram-3', cls: 'tool-ico--subagent' }, + image: { glyph: 'bi-image', cls: 'tool-ico--image' }, + config: { glyph: 'bi-sliders', cls: 'tool-ico--config' }, + introspection: { glyph: 'bi-info-circle', cls: 'tool-ico--introspection' }, + file: { glyph: 'bi-file-earmark', cls: 'tool-ico--file' }, + mcp: { glyph: 'bi-plug', cls: 'tool-ico--mcp' }, + tool: { glyph: 'bi-wrench', cls: 'tool-ico--tool' }, +}; + +/** Whether a tool call carries a file-write diff snapshot to render. */ +function hasPreview(msg) { + return msg.preview_new != null || msg.preview_old != null; +} + +/** + * Whether to render the diff inline on the tool card. Suppressed while a sibling + * `pending_write` card for the same call is present (it already shows the diff — an + * approval-gated write). After a reload there is no such card, so the tool card + * becomes the single place the diff lives. `host` may be absent in bare renders. + */ +function showInlineDiff(host, msg) { + if (!hasPreview(msg)) return false; + const siblings = host && host._messages; + if (Array.isArray(siblings) + && siblings.some(m => m.kind === 'pending_write' && m.tool_call_id === msg.tool_call_id)) { + return false; + } + return true; +} + +/** The MCP server name embedded in an `mcp____` id, or null. */ +function mcpServerOf(name) { + if (typeof name !== 'string' || !name.startsWith('mcp__')) return null; + const rest = name.slice(5); + const i = rest.indexOf('__'); + return i === -1 ? rest : rest.slice(0, i); +} + +/** + * The leading tool icon for a card. MCP tools show their connector's own icon + * (parsed from the `mcp__server__tool` id), falling back to a plug glyph if the + * connector shipped none; every other tool shows its semantic glyph + accent. + */ +function renderToolIcon(msg) { + const server = mcpServerOf(msg.name); + if (server) { + return html` + { const w = e.target.closest('.copilot-tool-ico-wrap'); if (w) w.classList.add('img-failed'); }}> + + `; + } + const ic = TOOL_ICON[msg.icon] || TOOL_ICON.tool; + return html``; +} + +/** + * The muted secondary detail beside a tool's friendly title: the target path + * (clickable) or the primary argument. Derived from `label_full` by stripping the + * leading raw tool-name token — which the friendly `display_name` now replaces — so + * an MCP tool (whose label is just its raw id) shows no redundant secondary. + */ +function toolSecondary(msg) { + let rest = msg.label_full || ''; + if (msg.name && rest.startsWith(msg.name)) rest = rest.slice(msg.name.length); + rest = rest.trim(); + if (!rest) return nothing; + return html`${renderLabel(rest, msg.path)}`; +} // ── Utilities ──────────────────────────────────────────────────────────────── @@ -197,9 +283,21 @@ export function renderTool(host, msg) {
${isOpen ? html`
@@ -209,6 +307,12 @@ export function renderTool(host, msg) { 
${argsStr}
` : nothing} + ${showInlineDiff(host, msg) ? html` +
+ ${t('copilot.changes')} + 
${renderDiff(msg.preview_old || '', msg.preview_new || '')}
+
+ ` : nothing} ${isPending ? (msg.name === 'ask_user_clarification' ? html`
${msg.question_title ? html`
${msg.question_title}
` : nothing} diff --git a/web/components/copilot.js b/web/components/copilot.js index 95a8e50..4eacf6a 100644 --- a/web/components/copilot.js +++ b/web/components/copilot.js @@ -78,7 +78,7 @@ export class AppCopilot extends I18nMixin(ChatSession) { _pageFromHash() { const m = location.hash.slice(1).match(/^([^/?]+)/); const seg = m ? m[1] : ''; - const known = ['inbox', 'dashboard', 'tasks', 'projects', 'models', 'providers', 'approval', 'agents', 'users', 'roles', 'connectors', 'connector', 'catalog', 'marketplace', 'profile', 'config', 'llm-requests', 'session', 'tic', 'file_viewer']; + const known = ['inbox', 'dashboard', 'tasks', 'projects', 'models', 'providers', 'approval', 'agents', 'users', 'roles', 'connectors', 'connector', 'catalog', 'marketplace', 'profile', 'config', 'llm-requests', 'session', 'tic', 'file_viewer', 'tool_detail']; return known.includes(seg) ? seg : 'home'; } diff --git a/web/components/mobile-app.js b/web/components/mobile-app.js index d1db168..0d666c5 100644 --- a/web/components/mobile-app.js +++ b/web/components/mobile-app.js @@ -5,14 +5,16 @@ import './shared/chat-page.js'; import './shared/projects-page.js'; import './shared/settings-page.js'; import './shared/file-viewer-mobile.js'; +import './shared/tool-detail-mobile.js'; // Sections addressable via the URL hash — same routing style as the desktop // sidebar (web/components/sidebar.js). The native iOS shell and mobile browsers // share this router so the URL always reflects the active section: native menu // sync, deep links, and back/refresh restoration all flow from one place. -// `file_viewer` is not a tab — it's opened from content (a clickable tool path -// via openFile() → `#file_viewer?path=...`) and so has no bottom-nav entry. -const VALID_SECTIONS = ['inbox', 'projects', 'chat', 'notifications', 'settings', 'file_viewer']; +// `file_viewer` / `tool_detail` are not tabs — they're opened from content (a +// clickable tool path via openFile() → `#file_viewer?path=...`, or a tool card's +// eye via openToolDetail() → `#tool_detail?id=...`) and have no bottom-nav entry. +const VALID_SECTIONS = ['inbox', 'projects', 'chat', 'notifications', 'settings', 'file_viewer', 'tool_detail']; class MobileApp extends LitElement { // No shadow DOM — lets external CSS and Bootstrap Icons apply directly. @@ -26,6 +28,8 @@ class MobileApp extends LitElement { _chatLabel: { state: true }, // File shown by the file_viewer section (from `#file_viewer?path=...`). _filePath: { state: true }, + // Tool call shown by the tool_detail section (from `#tool_detail?id=...`). + _toolId: { state: true }, }; constructor() { @@ -34,6 +38,7 @@ class MobileApp extends LitElement { this._chatSource = 'mobile'; this._chatLabel = ''; this._filePath = null; + this._toolId = null; // id → name cache, so a cold deep-link (#chat/project- opened by the // native shell) can resolve its header label without the project list open. this._projectLabels = {}; @@ -69,9 +74,9 @@ class MobileApp extends LitElement { // #file_viewer?path= → section 'file_viewer' showing a file _readHash() { const raw = location.hash.slice(1); - if (!raw) return { section: 'chat', projectId: null, filePath: null }; + if (!raw) return { section: 'chat', projectId: null, filePath: null, toolId: null }; // Segment ends at the first `/` (project sub-route) or `?` (query, e.g. the - // file viewer's `?path=`). + // file viewer's `?path=` / the tool detail's `?id=`). const cut = raw.search(/[/?]/); const seg = cut === -1 ? raw : raw.slice(0, cut); const section = VALID_SECTIONS.includes(seg) ? seg : 'chat'; @@ -79,7 +84,13 @@ class MobileApp extends LitElement { let filePath = null; const m = raw.match(/[?&]path=([^&]*)/); if (m) { try { filePath = decodeURIComponent(m[1]); } catch { /* keep null */ } } - return { section, projectId: null, filePath }; + return { section, projectId: null, filePath, toolId: null }; + } + if (section === 'tool_detail') { + let toolId = null; + const m = raw.match(/[?&]id=([^&]*)/); + if (m) { try { toolId = decodeURIComponent(m[1]); } catch { /* keep null */ } } + return { section, projectId: null, filePath: null, toolId }; } const slash = raw.indexOf('/'); const sub = slash === -1 ? '' : raw.slice(slash + 1); @@ -87,13 +98,14 @@ class MobileApp extends LitElement { if (section === 'chat' && sub.startsWith('project-')) { projectId = sub.slice('project-'.length) || null; } - return { section, projectId, filePath: null }; + return { section, projectId, filePath: null, toolId: null }; } _applyHash() { - const { section, projectId, filePath } = this._readHash(); + const { section, projectId, filePath, toolId } = this._readHash(); this._section = section; this._filePath = filePath; + this._toolId = toolId; if (projectId) { const source = 'project-' + projectId; if (this._chatSource !== source) this._chatSource = source; @@ -198,6 +210,11 @@ class MobileApp extends LitElement { .path=${this._filePath} style=${s === 'file_viewer' ? 'flex:1;min-height:0;overflow:hidden' : 'display:none'} > + `, but prop-driven: `` binds `visible` / `toolId` + * from its hash router (`#tool_detail?id=...`) instead of the component listening to + * the hash. The back button returns to the previous mobile section via history. + */ +export class MobileToolDetailPage extends LitElement { + // No shadow DOM — inherit the app's global CSS + Bootstrap Icons. + createRenderRoot() { return this; } + + static properties = { + visible: { type: Boolean }, + toolId: { attribute: 'tool-id' }, + _loading: { state: true }, + _error: { state: true }, + _tool: { state: true }, + }; + + constructor() { + super(); + this.visible = false; + this.toolId = null; + this._loading = false; + this._error = null; + this._tool = null; + } + + updated(changed) { + if (changed.has('visible') || changed.has('toolId')) { + if (this.visible && this.toolId != null) this._load(); + } + } + + async _load() { + this._loading = true; + this._error = null; + this._tool = null; + try { + this._tool = await fetchToolDetail(this.toolId); + } catch (e) { + this._error = e.message || String(e); + } finally { + this._loading = false; + } + } + + _back() { history.back(); } + + render() { + if (!this.visible) return nothing; + const tl = this._tool; + const si = tl ? (STATUS_ICON[tl.status] || STATUS_ICON.done) : null; + return html` +
+
+ + + + ${si ? html`` : nothing} + ${tl ? (tl.display_name || tl.name) : t('tool_detail.title')} + + +
+
+ ${this._loading ? html`
${t('common.loading')}
` : nothing} + ${this._error ? html`
${this._error}
` : nothing} + ${renderToolBody(tl)} +
+
+ `; + } +} + +customElements.define('mobile-tool-detail-page', MobileToolDetailPage); diff --git a/web/components/shared/tool-detail-view.js b/web/components/shared/tool-detail-view.js new file mode 100644 index 0000000..8bf5c81 --- /dev/null +++ b/web/components/shared/tool-detail-view.js @@ -0,0 +1,77 @@ +import { html, nothing } from 'lit'; +import { t } from '../../lib/i18n.js'; +import { openFile } from '../../lib/open-file.js'; +import { renderDiff } from '../copilot-render.js'; + +// Shared engine for the tool-execution detail view — the fetch + the center-panel +// body — so the desktop (`tool-detail-page.js`) and mobile +// (`shared/tool-detail-mobile.js`) surfaces render identically and only supply +// their own chrome (header / back button). + +export const STATUS_ICON = { + done: { glyph: 'bi-check-circle-fill', cls: 'text-success' }, + error: { glyph: 'bi-x-circle-fill', cls: 'text-danger' }, + cancelled: { glyph: 'bi-slash-circle-fill', cls: 'text-secondary' }, + rejected: { glyph: 'bi-shield-fill-x', cls: 'text-warning' }, + pending: { glyph: 'bi-hourglass-split', cls: 'text-warning' }, +}; + +function prettyJson(v) { + if (v == null) return ''; + try { return JSON.stringify(v, null, 2); } + catch { return String(v); } +} + +/** Fetches one tool call's full detail from `GET /api/tools/{id}`. Throws on error. */ +export async function fetchToolDetail(id) { + const r = await fetch(`/api/tools/${encodeURIComponent(id)}`, { credentials: 'same-origin' }); + if (!r.ok) throw new Error((await r.text()) || `HTTP ${r.status}`); + return r.json(); +} + +function renderResult(tl) { + if (tl.status === 'error' || tl.status === 'cancelled' || tl.status === 'rejected') { + return html`
${tl.error ?? tl.result ?? ''}
`; + } + if (tl.status === 'pending') { + return html`
${t('approval.pending')}
`; + } + let body = tl.result ?? ''; + if (tl.result_type === 'json') { + try { body = prettyJson(JSON.parse(tl.result ?? 'null')); } catch { /* keep raw */ } + } + return html`
${body}
`; +} + +/** The center-panel body: target path, input args, diff (writes), and result. */ +export function renderToolBody(tl) { + if (!tl) return nothing; + const hasPreview = tl.preview_new != null || tl.preview_old != null; + return html` + ${tl.path ? html` +
+ ${t('tool_detail.target')} + +
+ ` : nothing} + +
+ ${t('tool_detail.input')} + 
${prettyJson(tl.arguments)}
+
+ + ${hasPreview ? html` +
+ ${t('copilot.changes')} + 
${renderDiff(tl.preview_old || '', tl.preview_new || '')}
+
+ ` : nothing} + +
+ ${t('copilot.result')} + ${renderResult(tl)} +
+ `; +} diff --git a/web/components/sidebar.js b/web/components/sidebar.js index 0209f04..08c9cc7 100644 --- a/web/components/sidebar.js +++ b/web/components/sidebar.js @@ -219,7 +219,7 @@ export class AppSidebar extends I18nMixin(LightElement) { return m ? `plugin/${m[1]}/${m[2]}` : 'home'; } // `connector` (singular) is the per-connector detail page, `connectors` the list. - return ['inbox', 'dashboard', 'tasks', 'projects', 'models', 'providers', 'approval', 'agents', 'users', 'roles', 'shared-folders', 'connectors', 'connector', 'plugins', 'plugin-catalog', 'plugin-detail', 'catalog', 'marketplace', 'profile', 'config', 'llm-requests', 'session', 'tic', 'file_viewer'].includes(segment) ? segment : 'home'; + return ['inbox', 'dashboard', 'tasks', 'projects', 'models', 'providers', 'approval', 'agents', 'users', 'roles', 'shared-folders', 'connectors', 'connector', 'plugins', 'plugin-catalog', 'plugin-detail', 'catalog', 'marketplace', 'profile', 'config', 'llm-requests', 'session', 'tic', 'file_viewer', 'tool_detail'].includes(segment) ? segment : 'home'; } _tasksSectionFromHash() { diff --git a/web/components/tool-detail-page.js b/web/components/tool-detail-page.js new file mode 100644 index 0000000..dd21534 --- /dev/null +++ b/web/components/tool-detail-page.js @@ -0,0 +1,99 @@ +import { html, nothing } from 'lit'; +import { LightElement } from '../lib/base.js'; +import { t } from '../lib/i18n.js'; +import { fetchToolDetail, renderToolBody, STATUS_ICON } from './shared/tool-detail-view.js'; + +const PAGE_ID = 'tool_detail'; + +function idFromHash() { + const h = location.hash; + const prefix = `#${PAGE_ID}?id=`; + if (!h.startsWith(prefix)) return null; + try { + return decodeURIComponent(h.slice(prefix.length)); + } catch { + return null; + } +} + +/** + * Desktop tool-execution detail page. Self-routes off the hash + * (`#tool_detail?id=...`), mirroring `file-viewer-page.js`: the sidebar's + * `llm-page-change` event toggles visibility and `hashchange` re-loads. It hydrates + * from `GET /api/tools/{id}` (via the shared `tool-detail-view` engine) so a tool's + * input / result / diff is readable in the center panel even after a page reload. + */ +export class ToolDetailPage extends LightElement { + static properties = { + _open: { state: true }, + _loading: { state: true }, + _error: { state: true }, + _tool: { state: true }, + }; + + constructor() { + super(); + this._open = false; + this._loading = false; + this._error = null; + this._tool = null; + } + + connectedCallback() { + super.connectedCallback(); + window.addEventListener('llm-page-change', (e) => { + this._open = e.detail.page === PAGE_ID; + this.style.display = this._open ? 'flex' : 'none'; + if (this._open) this._loadFromHash(); + }); + window.addEventListener('hashchange', () => { + if (this._open) this._loadFromHash(); + }); + } + + async _loadFromHash() { + const id = idFromHash(); + if (id == null) return; + this._loading = true; + this._error = null; + this._tool = null; + try { + this._tool = await fetchToolDetail(id); + } catch (e) { + this._error = e.message || String(e); + } finally { + this._loading = false; + } + } + + _back() { history.back(); } + + render() { + if (!this._open) return nothing; + const tl = this._tool; + const si = tl ? (STATUS_ICON[tl.status] || STATUS_ICON.done) : null; + return html` +
+
+
+ +

+ ${tl ? html` + ${si ? html`` : nothing} + ${tl.display_name || tl.name} + ` : t('tool_detail.title')} +

+
+
+ +
+ ${this._loading ? html`
${t('common.loading')}
` : nothing} + ${this._error ? html`
${this._error}
` : nothing} + ${renderToolBody(tl)} +
+
+ `; + } +} diff --git a/web/css/copilot-messages.css b/web/css/copilot-messages.css index 18073d9..cdb0bd9 100644 --- a/web/css/copilot-messages.css +++ b/web/css/copilot-messages.css @@ -131,13 +131,67 @@ font-size: 0.7rem; } +/* The leading tool-type icon: a colored glyph, or an MCP connector's own icon. */ +.copilot-tool-ico { + flex-shrink: 0; + font-size: 0.9rem; + line-height: 1; + color: var(--tool-generic); +} +.copilot-tool-ico-wrap { + display: inline-flex; + align-items: center; + flex-shrink: 0; +} +.copilot-tool-ico-img { + width: 15px; + height: 15px; + object-fit: contain; + border-radius: 3px; +} +/* Fallback: hide the glyph unless the connector image failed to load. */ +.copilot-tool-ico-wrap .copilot-tool-ico { display: none; } +.copilot-tool-ico-wrap.img-failed .copilot-tool-ico-img { display: none; } +.copilot-tool-ico-wrap.img-failed .copilot-tool-ico { display: inline; } + +.tool-ico--edit { color: var(--tool-edit); } +.tool-ico--read { color: var(--tool-read); } +.tool-ico--list { color: var(--tool-list); } +.tool-ico--search { color: var(--tool-search); } +.tool-ico--shell { color: var(--tool-shell); } +.tool-ico--subagent { color: var(--tool-subagent); } +.tool-ico--image { color: var(--tool-image); } +.tool-ico--config { color: var(--tool-config); } +.tool-ico--introspection { color: var(--tool-introspection); } +.tool-ico--mcp { color: var(--tool-mcp); } +.tool-ico--file, +.tool-ico--tool { color: var(--tool-generic); } + .copilot-tool-name { - font-weight: 400; + display: flex; + align-items: baseline; + gap: 0.4rem; + min-width: 0; + overflow: hidden; +} + +/* The friendly, static tool title ("Edit File") — the primary label. */ +.copilot-tool-title { + font-weight: 500; + white-space: nowrap; + flex-shrink: 0; + color: var(--msg-assistant-text); +} + +/* The muted secondary detail (target path / command / primary arg). */ +.copilot-tool-detail { font-family: var(--bs-font-monospace); + font-size: 0.72rem; white-space: nowrap; overflow: hidden; text-overflow: ellipsis; min-width: 0; + color: var(--placeholder-color); } .copilot-tool-name code { @@ -583,3 +637,86 @@ } .attach-chip-remove:hover { background: var(--sidebar-hover); color: #dc2626; } + +/* ── Tool card "view details" (eye) ────────────────────────────────────────── */ + +.copilot-tool-eye { + display: inline-flex; + align-items: center; + padding: 0 0.25rem; + color: var(--placeholder-color); + border-radius: 0.25rem; + flex-shrink: 0; +} +.copilot-tool-eye:hover { color: var(--accent); background: var(--accent-soft); } + +/* ── Tool-detail page (#tool_detail) ───────────────────────────────────────── */ + +.tool-detail-page { + display: flex; + flex-direction: column; + height: 100%; + width: 100%; +} + +.tool-detail-body { + flex: 1; + overflow-y: auto; + padding: 1rem 1.25rem; + display: flex; + flex-direction: column; + gap: 1rem; +} + +.tool-detail-section { + display: flex; + flex-direction: column; + gap: 0.35rem; +} + +.tool-detail-label { + font-size: 0.7rem; + font-weight: 700; + text-transform: uppercase; + letter-spacing: 0.08em; + color: var(--placeholder-color); +} + +.tool-detail-pre { + font-family: var(--bs-font-monospace); + font-size: 0.82rem; + white-space: pre-wrap; + word-break: break-word; + margin: 0; + padding: 0.7rem 0.9rem; + background: rgba(0, 0, 0, 0.04); + border: 1px solid var(--toolbar-border); + border-radius: var(--radius-sm); + overflow-x: auto; +} +.tool-detail-pre--error { color: #dc2626; } + +@media (prefers-color-scheme: dark) { + .tool-detail-pre { background: rgba(255, 255, 255, 0.04); } +} + +.tool-detail-muted { color: var(--placeholder-color); font-size: 0.85rem; } + +.tool-detail-path { + align-self: flex-start; + font-family: var(--bs-font-monospace); + font-size: 0.82rem; + color: var(--accent); + background: var(--accent-soft); + border: none; + border-radius: var(--radius-sm); + padding: 0.3rem 0.6rem; + cursor: pointer; +} +.tool-detail-path:hover { text-decoration: underline; } + +/* The detail page's diff fills the panel rather than the compact card height. */ +.tool-detail-page .copilot-diff { + max-height: none; + font-size: 0.82rem; +} diff --git a/web/css/variables.css b/web/css/variables.css index 8a6a98b..d8efea4 100644 --- a/web/css/variables.css +++ b/web/css/variables.css @@ -12,6 +12,20 @@ --accent-soft: rgba(217, 93, 78, 0.12); --accent-ring: rgba(217, 93, 78, 0.28); + /* Tool-card accents — one hue per tool category (semantic icon key). + Light mode; the dark block below lifts them for contrast. */ + --tool-edit: #2563eb; + --tool-read: #0891b2; + --tool-list: #7c3aed; + --tool-search: #9333ea; + --tool-shell: #475569; + --tool-subagent: #0d9488; + --tool-image: #db2777; + --tool-config: #ca8a04; + --tool-introspection: #6b7280; + --tool-mcp: var(--accent); + --tool-generic: #8a7a66; + /* Radius scale — friendly, generous */ --radius-sm: 8px; --radius-md: 12px; @@ -66,6 +80,18 @@ --accent-soft: rgba(232, 131, 111, 0.16); --accent-ring: rgba(232, 131, 111, 0.35); + /* Tool-card accents — lifted for dark backgrounds */ + --tool-edit: #60a5fa; + --tool-read: #22d3ee; + --tool-list: #a78bfa; + --tool-search: #c084fc; + --tool-shell: #94a3b8; + --tool-subagent: #2dd4bf; + --tool-image: #f472b6; + --tool-config: #eab308; + --tool-introspection: #9ca3af; + --tool-generic: #b3a28c; + --sidebar-bg: #241f1a; --sidebar-hover: rgba(236, 225, 211, 0.06); --sidebar-active-bg: rgba(232, 131, 111, 0.16); diff --git a/web/i18n/en.js b/web/i18n/en.js index 8ef7378..15dc549 100644 --- a/web/i18n/en.js +++ b/web/i18n/en.js @@ -91,6 +91,11 @@ export default { 'copilot.not_sent_to_llm': 'This message is not sent to the LLM', 'copilot.remove': 'Remove', 'copilot.result_json': 'result · json', + 'copilot.changes': 'changes', + 'copilot.view_details': 'View details', + 'tool_detail.title': 'Tool call', + 'tool_detail.target': 'target', + 'tool_detail.input': 'input', 'copilot.agent_done': 'done', 'copilot.agent_running': 'running…', 'copilot.agent_finished': 'finished', diff --git a/web/i18n/fr.js b/web/i18n/fr.js index f6c7c85..181b5ee 100644 --- a/web/i18n/fr.js +++ b/web/i18n/fr.js @@ -91,6 +91,11 @@ export default { 'copilot.not_sent_to_llm': 'Ce message n\'est pas envoyé au LLM', 'copilot.remove': 'Supprimer', 'copilot.result_json': 'résultat · json', + 'copilot.changes': 'modifications', + 'copilot.view_details': 'Voir les détails', + 'tool_detail.title': 'Appel d\'outil', + 'tool_detail.target': 'cible', + 'tool_detail.input': 'entrée', 'copilot.agent_done': 'terminé', 'copilot.agent_running': 'en cours…', 'copilot.agent_finished': 'fini', diff --git a/web/i18n/it.js b/web/i18n/it.js index 5b1b266..4baae73 100644 --- a/web/i18n/it.js +++ b/web/i18n/it.js @@ -91,6 +91,11 @@ export default { 'copilot.not_sent_to_llm': 'Questo messaggio non viene inviato all\'LLM', 'copilot.remove': 'Rimuovi', 'copilot.result_json': 'risultato · json', + 'copilot.changes': 'modifiche', + 'copilot.view_details': 'Vedi dettagli', + 'tool_detail.title': 'Chiamata tool', + 'tool_detail.target': 'target', + 'tool_detail.input': 'input', 'copilot.agent_done': 'completato', 'copilot.agent_running': 'in esecuzione…', 'copilot.agent_finished': 'finito', diff --git a/web/index.html b/web/index.html index b798a29..cc88992 100644 --- a/web/index.html +++ b/web/index.html @@ -116,6 +116,7 @@ +
diff --git a/web/lib/chat-session.js b/web/lib/chat-session.js index e7b57e6..c0244da 100644 --- a/web/lib/chat-session.js +++ b/web/lib/chat-session.js @@ -314,6 +314,8 @@ export class ChatSession extends LightElement { kind: 'tool', tool_call_id: msg.tool_call_id, name: msg.name, + display_name: msg.display_name, + icon: msg.icon, label_short: msg.label_short, label_full: msg.label_full, path: msg.path, @@ -327,7 +329,12 @@ export class ChatSession extends LightElement { } case 'tool_done': - this._updateTool(msg.tool_call_id, { status: 'done', result: msg.result, result_type: msg.result_type }); + // `preview_old`/`preview_new` are present only for a file-write; they let the + // card render the diff inline even for an auto-allowed write (no PendingWrite). + this._updateTool(msg.tool_call_id, { + status: 'done', result: msg.result, result_type: msg.result_type, + preview_old: msg.preview_old, preview_new: msg.preview_new, + }); break; case 'tool_error': diff --git a/web/lib/open-tool.js b/web/lib/open-tool.js new file mode 100644 index 0000000..97da739 --- /dev/null +++ b/web/lib/open-tool.js @@ -0,0 +1,17 @@ +/** + * Global tool-detail opener helper. + * + * `window.openToolDetail(id)` is the single entry point for "show this tool + * call's full input / result / diff in the dedicated detail page". It navigates + * to `#tool_detail?id=`, which the hash router in `sidebar.js` resolves to + * the `` element. Back/forward navigation works naturally. + * + * Mirrors `open-file.js` — the URL format lives in one place, so a tool card's + * "details" (eye) affordance calls this rather than setting the hash directly. + */ +export function openToolDetail(id) { + if (id == null) return; + location.hash = `tool_detail?id=${encodeURIComponent(id)}`; +} + +window.openToolDetail = openToolDetail;