diff --git a/src/agent/agent_loop.rs b/src/agent/agent_loop.rs index bac4df94..9b66a6b4 100644 --- a/src/agent/agent_loop.rs +++ b/src/agent/agent_loop.rs @@ -2,29 +2,6 @@ use std::sync::Arc; -/// Escape special characters for Telegram's legacy Markdown. -/// -/// In Telegram's Markdown mode, these characters have special meaning: -/// - `_` starts/ends italic -/// - `*` starts/ends bold -/// - `` ` `` starts/ends code -/// - `[` starts a link -/// -/// We escape them with backslash so dynamic content doesn't break formatting. -fn escape_telegram_markdown(s: &str) -> String { - let mut result = String::with_capacity(s.len()); - for c in s.chars() { - match c { - '_' | '*' | '`' | '[' => { - result.push('\\'); - result.push(c); - } - _ => result.push(c), - } - } - result -} - use futures::StreamExt; use tokio::sync::Mutex; use uuid::Uuid; @@ -339,17 +316,37 @@ impl Agent { // Main message loop tracing::info!("Agent {} ready and listening", self.config.name); - while let Some(message) = message_stream.next().await { + loop { + let message = tokio::select! { + biased; + _ = tokio::signal::ctrl_c() => { + tracing::info!("Ctrl+C received, shutting down..."); + break; + } + msg = message_stream.next() => { + match msg { + Some(m) => m, + None => { + tracing::info!("All channel streams ended, shutting down..."); + break; + } + } + } + }; + match self.handle_message(&message).await { - Ok(Some(response)) => { + Ok(Some(response)) if !response.is_empty() => { let _ = self .channels .respond(&message, OutgoingResponse::text(response)) .await; } + Ok(Some(_)) => { + // Empty response, nothing to send (e.g. approval handled via send_status) + } Ok(None) => { - // Shutdown signal received - tracing::info!("Shutdown signal received, exiting..."); + // Shutdown signal received (/quit, /exit, /shutdown) + tracing::info!("Shutdown command received, exiting..."); break; } Err(e) => { @@ -439,6 +436,7 @@ impl Agent { Submission::Heartbeat => self.process_heartbeat().await, Submission::Summarize => self.process_summarize(session, thread_id).await, Submission::Suggest => self.process_suggest(session, thread_id).await, + Submission::Quit => return Ok(None), Submission::SwitchThread { thread_id: target } => { self.process_switch_thread(message, target).await } @@ -478,34 +476,24 @@ impl Agent { description, parameters, } => { - // Format approval request for user - let params_preview = serde_json::to_string_pretty(¶meters) - .unwrap_or_else(|_| parameters.to_string()); - let params_truncated = if params_preview.chars().count() > 200 { - format!( - "{}...", - params_preview.chars().take(200).collect::() + // Each channel renders the approval prompt via send_status. + // Web gateway shows an inline card, REPL prints a formatted prompt, etc. + let _ = self + .channels + .send_status( + &message.channel, + StatusUpdate::ApprovalNeeded { + request_id: request_id.to_string(), + tool_name, + description, + parameters, + }, + &message.metadata, ) - } else { - params_preview - }; - // Escape Markdown special chars in dynamic values to avoid breaking - // Telegram's Markdown parser (underscores, asterisks, backticks, brackets) - let tool_name_escaped = escape_telegram_markdown(&tool_name); - let description_escaped = escape_telegram_markdown(&description); - // Params go inside a code block, so no escaping needed there - Ok(Some(format!( - "🔒 Tool requires approval:\n\n\ - *Tool:* {}\n\ - *Description:* {}\n\ - *Parameters:*\n```\n{}\n```\n\n\ - Reply with:\n\ - • yes or approve to allow this tool\n\ - • always to always allow this tool in this session\n\ - • no or deny to reject\n\n\ - Request ID: {}", - tool_name_escaped, description_escaped, params_truncated, request_id - ))) + .await; + + // Empty string signals the caller to skip respond() (no duplicate text) + Ok(Some(String::new())) } } } @@ -663,7 +651,7 @@ impl Agent { // Run the agentic tool execution loop let result = self - .run_agentic_loop(message, session.clone(), thread_id, turn_messages) + .run_agentic_loop(message, session.clone(), thread_id, turn_messages, false) .await; // Re-acquire lock and check if interrupted @@ -732,12 +720,17 @@ impl Agent { /// /// Returns `AgenticLoopResult::Response` on completion, or /// `AgenticLoopResult::NeedApproval` if a tool requires user approval. + /// + /// When `resume_after_tool` is true the loop already knows a tool was + /// executed earlier in this turn (e.g. an approved tool), so it won't + /// force the LLM to use tools if it responds with text. async fn run_agentic_loop( &self, message: &IncomingMessage, session: Arc>, thread_id: Uuid, initial_messages: Vec, + resume_after_tool: bool, ) -> Result { // Load workspace system prompt (identity files: AGENTS.md, SOUL.md, etc.) let system_prompt = if let Some(ws) = self.workspace() { @@ -766,7 +759,7 @@ impl Agent { const MAX_TOOL_ITERATIONS: usize = 10; let mut iteration = 0; - let mut tools_executed = false; + let mut tools_executed = resume_after_tool; loop { iteration += 1; @@ -824,6 +817,14 @@ impl Agent { } RespondResult::ToolCalls(tool_calls) => { tools_executed = true; + + // Add the assistant message with tool_calls to context. + // OpenAI-compatible APIs require this before tool-result messages. + context_messages.push(ChatMessage::assistant_with_tool_calls( + "", + tool_calls.clone(), + )); + // Execute tools and add results to context let _ = self .channels @@ -1323,9 +1324,9 @@ impl Agent { result_content, )); - // Continue the agentic loop + // Continue the agentic loop (a tool was already executed this turn) let result = self - .run_agentic_loop(message, session.clone(), thread_id, context_messages) + .run_agentic_loop(message, session.clone(), thread_id, context_messages, true) .await; // Handle the result @@ -1441,15 +1442,38 @@ impl Agent { .channels .send_status( &message.channel, - StatusUpdate::Status("Authenticated".into()), + StatusUpdate::Status("Authenticated, loading tools...".into()), &message.metadata, ) .await; - Ok(Some(format!( - "{} authenticated successfully.", - pending.extension_name - ))) + // Auto-activate so tools are available immediately after auth + match ext_mgr.activate(&pending.extension_name).await { + Ok(activate_result) => { + let tool_count = activate_result.tools_loaded.len(); + let tool_list = if activate_result.tools_loaded.is_empty() { + String::new() + } else { + format!("\n\nTools: {}", activate_result.tools_loaded.join(", ")) + }; + Ok(Some(format!( + "{} authenticated and activated ({} tools loaded).{}", + pending.extension_name, tool_count, tool_list + ))) + } + Err(e) => { + tracing::warn!( + "Extension '{}' authenticated but activation failed: {}", + pending.extension_name, + e + ); + Ok(Some(format!( + "{} authenticated successfully, but activation failed: {}. \ + Try activating manually.", + pending.extension_name, e + ))) + } + } } Ok(result) => { // Unexpected state, re-enter auth mode @@ -1862,11 +1886,6 @@ impl Agent { Ok(Some(format!("Available tools: {}", tools.join(", ")))) } - "quit" | "exit" | "shutdown" => { - // Signal shutdown - return None to indicate no response needed - Ok(None) - } - _ => Ok(Some(format!("Unknown command: {}. Try /help", command))), } } diff --git a/src/agent/submission.rs b/src/agent/submission.rs index f1c36d68..7a28356c 100644 --- a/src/agent/submission.rs +++ b/src/agent/submission.rs @@ -43,6 +43,9 @@ impl SubmissionParser { if lower == "/thread new" || lower == "/new" { return Submission::NewThread; } + if lower == "/quit" || lower == "/exit" || lower == "/shutdown" { + return Submission::Quit; + } // /thread - switch thread if let Some(rest) = lower.strip_prefix("/thread ") { @@ -61,6 +64,15 @@ impl SubmissionParser { } } + // Try structured JSON approval (from web gateway's /api/chat/approval endpoint) + if trimmed.starts_with('{') { + if let Ok(submission) = serde_json::from_str::(trimmed) { + if matches!(submission, Submission::ExecApproval { .. }) { + return submission; + } + } + } + // Approval responses (simple yes/no/always for pending approvals) // These are short enough to check explicitly match lower.as_str() { @@ -157,6 +169,9 @@ pub enum Submission { /// Suggest next steps based on the current thread. Suggest, + + /// Quit the agent. Bypasses thread-state checks. + Quit, } impl Submission { @@ -407,4 +422,97 @@ mod tests { let submission = SubmissionParser::parse("/unknown"); assert!(matches!(submission, Submission::UserInput { content } if content == "/unknown")); } + + #[test] + fn test_parser_json_exec_approval() { + let req_id = Uuid::new_v4(); + let json = serde_json::to_string(&Submission::ExecApproval { + request_id: req_id, + approved: true, + always: false, + }) + .expect("serialize"); + + let submission = SubmissionParser::parse(&json); + assert!( + matches!(submission, Submission::ExecApproval { request_id, approved, always } + if request_id == req_id && approved && !always) + ); + } + + #[test] + fn test_parser_json_exec_approval_always() { + let req_id = Uuid::new_v4(); + let json = serde_json::to_string(&Submission::ExecApproval { + request_id: req_id, + approved: true, + always: true, + }) + .expect("serialize"); + + let submission = SubmissionParser::parse(&json); + assert!( + matches!(submission, Submission::ExecApproval { request_id, approved, always } + if request_id == req_id && approved && always) + ); + } + + #[test] + fn test_parser_json_exec_approval_deny() { + let req_id = Uuid::new_v4(); + let json = serde_json::to_string(&Submission::ExecApproval { + request_id: req_id, + approved: false, + always: false, + }) + .expect("serialize"); + + let submission = SubmissionParser::parse(&json); + assert!( + matches!(submission, Submission::ExecApproval { request_id, approved, always } + if request_id == req_id && !approved && !always) + ); + } + + #[test] + fn test_parser_json_non_approval_stays_user_input() { + // A JSON UserInput should NOT be intercepted, it should be treated as text + let json = r#"{"UserInput":{"content":"hello"}}"#; + let submission = SubmissionParser::parse(json); + assert!(matches!(submission, Submission::UserInput { .. })); + } + + #[test] + fn test_parser_json_roundtrip_matches_approval_handler() { + // Simulate exactly what chat_approval_handler does: serialize a Submission::ExecApproval + // and verify the parser picks it up correctly. + let request_id = Uuid::new_v4(); + let approval = Submission::ExecApproval { + request_id, + approved: true, + always: false, + }; + let json = serde_json::to_string(&approval).expect("serialize"); + eprintln!("Serialized approval JSON: {}", json); + + let parsed = SubmissionParser::parse(&json); + assert!( + matches!(parsed, Submission::ExecApproval { request_id: rid, approved, always } + if rid == request_id && approved && !always), + "Expected ExecApproval, got {:?}", + parsed + ); + } + + #[test] + fn test_parser_quit() { + assert!(matches!(SubmissionParser::parse("/quit"), Submission::Quit)); + assert!(matches!(SubmissionParser::parse("/exit"), Submission::Quit)); + assert!(matches!( + SubmissionParser::parse("/shutdown"), + Submission::Quit + )); + assert!(matches!(SubmissionParser::parse("/QUIT"), Submission::Quit)); + assert!(matches!(SubmissionParser::parse("/Exit"), Submission::Quit)); + } } diff --git a/src/channels/channel.rs b/src/channels/channel.rs index c94140a4..8292f6be 100644 --- a/src/channels/channel.rs +++ b/src/channels/channel.rs @@ -112,6 +112,13 @@ pub enum StatusUpdate { StreamChunk(String), /// General status message. Status(String), + /// Tool requires user approval before execution. + ApprovalNeeded { + request_id: String, + tool_name: String, + description: String, + parameters: serde_json::Value, + }, } /// Trait for message channels. diff --git a/src/channels/repl.rs b/src/channels/repl.rs index 35fdcb07..dc3aed96 100644 --- a/src/channels/repl.rs +++ b/src/channels/repl.rs @@ -286,7 +286,9 @@ impl Channel for ReplChannel { } } Err(ReadlineError::Eof) => { - // Ctrl+D: quit + // Ctrl+D: send /quit so the agent loop runs graceful shutdown + let msg = IncomingMessage::new("repl", "user", "/quit"); + let _ = tx.blocking_send(msg); break; } Err(e) => { @@ -362,6 +364,37 @@ impl Channel for ReplChannel { eprintln!("\x1b[90m[status] {msg}\x1b[0m"); } } + StatusUpdate::ApprovalNeeded { + request_id, + tool_name, + description, + parameters, + } => { + let params_preview = serde_json::to_string_pretty(¶meters) + .unwrap_or_else(|_| parameters.to_string()); + let params_truncated = if params_preview.chars().count() > 200 { + format!( + "{}...", + params_preview.chars().take(200).collect::() + ) + } else { + params_preview + }; + eprintln!(); + eprintln!("\x1b[33m Tool requires approval\x1b[0m"); + eprintln!(" \x1b[1mTool:\x1b[0m {tool_name}"); + eprintln!(" \x1b[1mDesc:\x1b[0m {description}"); + eprintln!( + " \x1b[1mParams:\x1b[0m\n {}", + params_truncated.replace('\n', "\n ") + ); + eprintln!(); + eprintln!( + " Reply: \x1b[32myes\x1b[0m / \x1b[34malways\x1b[0m / \x1b[31mno\x1b[0m" + ); + eprintln!(" \x1b[90mRequest ID: {request_id}\x1b[0m"); + eprintln!(); + } } Ok(()) } diff --git a/src/channels/wasm/wrapper.rs b/src/channels/wasm/wrapper.rs index 550e6932..c70e9845 100644 --- a/src/channels/wasm/wrapper.rs +++ b/src/channels/wasm/wrapper.rs @@ -1844,6 +1844,15 @@ fn status_to_wit(status: &StatusUpdate, metadata: &serde_json::Value) -> wit_cha metadata_json, } } + StatusUpdate::ApprovalNeeded { + tool_name, + description, + .. + } => wit_channel::StatusUpdate { + status: wit_channel::StatusType::Thinking, + message: format!("Approval needed: {} - {}", tool_name, description), + metadata_json, + }, } } diff --git a/src/channels/web/log_layer.rs b/src/channels/web/log_layer.rs index f4f6b227..02a8e4ed 100644 --- a/src/channels/web/log_layer.rs +++ b/src/channels/web/log_layer.rs @@ -89,15 +89,32 @@ impl Default for LogBroadcaster { } } -/// Visitor that extracts the `message` field from a tracing event. +/// Visitor that extracts the `message` field and all extra key-value +/// fields from a tracing event. +/// +/// The terminal formatter shows something like: +/// INFO ironclaw::agent: Request completed url="http://..." status=200 +/// +/// We replicate that by capturing both the message and the extra fields. struct MessageVisitor { message: String, + fields: Vec, } impl MessageVisitor { fn new() -> Self { Self { message: String::new(), + fields: Vec::new(), + } + } + + /// Build the final message string: "message key=val key=val ..." + fn finish(self) -> String { + if self.fields.is_empty() { + self.message + } else { + format!("{} {}", self.message, self.fields.join(" ")) } } } @@ -110,12 +127,16 @@ impl Visit for MessageVisitor { if self.message.starts_with('"') && self.message.ends_with('"') { self.message = self.message[1..self.message.len() - 1].to_string(); } + } else { + self.fields.push(format!("{}={:?}", field.name(), value)); } } fn record_str(&mut self, field: &Field, value: &str) { if field.name() == "message" { self.message = value.to_string(); + } else { + self.fields.push(format!("{}={}", field.name(), value)); } } } @@ -153,7 +174,7 @@ impl Layer for WebLogLayer { let entry = LogEntry { level: metadata.level().to_string().to_uppercase(), target: metadata.target().to_string(), - message: visitor.message, + message: visitor.finish(), timestamp: chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Millis, true), }; @@ -261,4 +282,35 @@ mod tests { assert_eq!(recent.len(), 1); assert_eq!(recent[0].message, "before anyone listened"); } + + #[test] + fn test_message_visitor_finish_message_only() { + let v = MessageVisitor { + message: "hello world".to_string(), + fields: vec![], + }; + assert_eq!(v.finish(), "hello world"); + } + + #[test] + fn test_message_visitor_finish_with_fields() { + let v = MessageVisitor { + message: "Request completed".to_string(), + fields: vec![ + "url=http://localhost:8080".to_string(), + "status=200".to_string(), + ], + }; + let result = v.finish(); + assert_eq!( + result, + "Request completed url=http://localhost:8080 status=200" + ); + } + + #[test] + fn test_message_visitor_finish_empty() { + let v = MessageVisitor::new(); + assert_eq!(v.finish(), ""); + } } diff --git a/src/channels/web/mod.rs b/src/channels/web/mod.rs index 33cdbb0d..64fc9611 100644 --- a/src/channels/web/mod.rs +++ b/src/channels/web/mod.rs @@ -205,6 +205,18 @@ impl Channel for GatewayChannel { } StatusUpdate::StreamChunk(content) => SseEvent::StreamChunk { content }, StatusUpdate::Status(msg) => SseEvent::Status { message: msg }, + StatusUpdate::ApprovalNeeded { + request_id, + tool_name, + description, + parameters, + } => SseEvent::ApprovalNeeded { + request_id, + tool_name, + description, + parameters: serde_json::to_string_pretty(¶meters) + .unwrap_or_else(|_| parameters.to_string()), + }, }; self.state.sse.broadcast(event); diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs index 4020b9a4..c862ba4c 100644 --- a/src/channels/web/server.rs +++ b/src/channels/web/server.rs @@ -78,6 +78,7 @@ pub async fn start_server( let protected = Router::new() // Chat .route("/api/chat/send", post(chat_send_handler)) + .route("/api/chat/approval", post(chat_approval_handler)) .route("/api/chat/events", get(chat_events_handler)) .route("/api/chat/history", get(chat_history_handler)) .route("/api/chat/threads", get(chat_threads_handler)) @@ -204,6 +205,68 @@ async fn chat_send_handler( )) } +async fn chat_approval_handler( + State(state): State>, + Json(req): Json, +) -> Result<(StatusCode, Json), (StatusCode, String)> { + let (approved, always) = match req.action.as_str() { + "approve" => (true, false), + "always" => (true, true), + "deny" => (false, false), + other => { + return Err(( + StatusCode::BAD_REQUEST, + format!("Unknown action: {}", other), + )); + } + }; + + let request_id = Uuid::parse_str(&req.request_id).map_err(|_| { + ( + StatusCode::BAD_REQUEST, + "Invalid request_id (expected UUID)".to_string(), + ) + })?; + + // Build a structured ExecApproval submission as JSON, sent through the + // existing message pipeline so the agent loop picks it up. + let approval = crate::agent::submission::Submission::ExecApproval { + request_id, + approved, + always, + }; + let content = serde_json::to_string(&approval).map_err(|e| { + ( + StatusCode::INTERNAL_SERVER_ERROR, + format!("Failed to serialize approval: {}", e), + ) + })?; + + let msg = IncomingMessage::new("gateway", &state.user_id, content); + let msg_id = msg.id; + + let tx_guard = state.msg_tx.read().await; + let tx = tx_guard.as_ref().ok_or(( + StatusCode::SERVICE_UNAVAILABLE, + "Channel not started".to_string(), + ))?; + + tx.send(msg).await.map_err(|_| { + ( + StatusCode::INTERNAL_SERVER_ERROR, + "Channel closed".to_string(), + ) + })?; + + Ok(( + StatusCode::ACCEPTED, + Json(SendMessageResponse { + message_id: msg_id, + status: "accepted", + }), + )) +} + async fn chat_events_handler(State(state): State>) -> impl IntoResponse { // subscribe() returns Sse> so no lifetime issues state.sse.subscribe() @@ -650,6 +713,7 @@ async fn extensions_list_handler( name: ext.name, kind: ext.kind.to_string(), description: ext.description, + url: ext.url, authenticated: ext.authenticated, active: ext.active, tools: ext.tools, @@ -699,14 +763,8 @@ async fn extensions_install_handler( .install(&req.name, req.url.as_deref(), kind_hint) .await { - Ok(result) => Ok(Json(ActionResponse { - success: true, - message: result.message, - })), - Err(e) => Ok(Json(ActionResponse { - success: false, - message: e.to_string(), - })), + Ok(result) => Ok(Json(ActionResponse::ok(result.message))), + Err(e) => Ok(Json(ActionResponse::fail(e.to_string()))), } } @@ -720,14 +778,45 @@ async fn extensions_activate_handler( ))?; match ext_mgr.activate(&name).await { - Ok(result) => Ok(Json(ActionResponse { - success: true, - message: result.message, - })), - Err(e) => Ok(Json(ActionResponse { - success: false, - message: e.to_string(), - })), + Ok(result) => Ok(Json(ActionResponse::ok(result.message))), + Err(activate_err) => { + let err_str = activate_err.to_string(); + let needs_auth = err_str.contains("authentication") + || err_str.contains("401") + || err_str.contains("Unauthorized"); + + if !needs_auth { + return Ok(Json(ActionResponse::fail(err_str))); + } + + // Activation failed due to auth; try authenticating first. + match ext_mgr.auth(&name, None).await { + Ok(auth_result) if auth_result.status == "authenticated" => { + // Auth succeeded, retry activation. + match ext_mgr.activate(&name).await { + Ok(result) => Ok(Json(ActionResponse::ok(result.message))), + Err(e) => Ok(Json(ActionResponse::fail(e.to_string()))), + } + } + Ok(auth_result) => { + // Auth in progress (OAuth URL or awaiting manual token). + let mut resp = ActionResponse::fail( + auth_result + .instructions + .clone() + .unwrap_or_else(|| format!("'{}' requires authentication.", name)), + ); + resp.auth_url = auth_result.auth_url; + resp.awaiting_token = Some(auth_result.awaiting_token); + resp.instructions = auth_result.instructions; + Ok(Json(resp)) + } + Err(auth_err) => Ok(Json(ActionResponse::fail(format!( + "Authentication failed: {}", + auth_err + )))), + } + } } } @@ -741,13 +830,7 @@ async fn extensions_remove_handler( ))?; match ext_mgr.remove(&name).await { - Ok(message) => Ok(Json(ActionResponse { - success: true, - message, - })), - Err(e) => Ok(Json(ActionResponse { - success: false, - message: e.to_string(), - })), + Ok(message) => Ok(Json(ActionResponse::ok(message))), + Err(e) => Ok(Json(ActionResponse::fail(e.to_string()))), } } diff --git a/src/channels/web/static/app.js b/src/channels/web/static/app.js index 78431c35..a7ac57dd 100644 --- a/src/channels/web/static/app.js +++ b/src/channels/web/static/app.js @@ -71,7 +71,6 @@ function connectSSE() { const data = JSON.parse(e.data); addMessage('assistant', data.content); setStatus(''); - hideApproval(); }); eventSource.addEventListener('thinking', (e) => { @@ -134,21 +133,47 @@ function sendMessage() { }); } -function sendApproval(response) { - apiFetch('/api/chat/send', { +function sendApprovalAction(requestId, action) { + apiFetch('/api/chat/approval', { method: 'POST', - body: { content: response }, + body: { request_id: requestId, action: action }, }).catch((err) => { addMessage('system', 'Failed to send approval: ' + err.message); }); - hideApproval(); + + // Disable buttons and show confirmation on the card + const card = document.querySelector('.approval-card[data-request-id="' + requestId + '"]'); + if (card) { + const buttons = card.querySelectorAll('.approval-actions button'); + buttons.forEach((btn) => { + btn.disabled = true; + }); + const actions = card.querySelector('.approval-actions'); + const label = document.createElement('span'); + label.className = 'approval-resolved'; + const labelText = action === 'approve' ? 'Approved' : action === 'always' ? 'Always approved' : 'Denied'; + label.textContent = labelText; + actions.appendChild(label); + } +} + +function renderMarkdown(text) { + if (typeof marked !== 'undefined') { + return marked.parse(text); + } + return escapeHtml(text); } function addMessage(role, content) { const container = document.getElementById('chat-messages'); const div = document.createElement('div'); div.className = 'message ' + role; - div.textContent = content; + if (role === 'user') { + div.textContent = content; + } else { + div.setAttribute('data-raw', content); + div.innerHTML = renderMarkdown(content); + } container.appendChild(div); container.scrollTop = container.scrollHeight; } @@ -158,7 +183,9 @@ function appendToLastAssistant(chunk) { const messages = container.querySelectorAll('.message.assistant'); if (messages.length > 0) { const last = messages[messages.length - 1]; - last.textContent += chunk; + const raw = (last.getAttribute('data-raw') || '') + chunk; + last.setAttribute('data-raw', raw); + last.innerHTML = renderMarkdown(raw); container.scrollTop = container.scrollHeight; } else { addMessage('assistant', chunk); @@ -175,14 +202,70 @@ function setStatus(text, spinning) { } function showApproval(data) { - const banner = document.getElementById('approval-banner'); - const info = document.getElementById('approval-info'); - info.textContent = 'Tool "' + data.tool_name + '" requires approval: ' + data.description; - banner.classList.add('visible'); -} + const container = document.getElementById('chat-messages'); + const card = document.createElement('div'); + card.className = 'approval-card'; + card.setAttribute('data-request-id', data.request_id); -function hideApproval() { - document.getElementById('approval-banner').classList.remove('visible'); + const header = document.createElement('div'); + header.className = 'approval-header'; + header.textContent = 'Tool requires approval'; + card.appendChild(header); + + const toolName = document.createElement('div'); + toolName.className = 'approval-tool-name'; + toolName.textContent = data.tool_name; + card.appendChild(toolName); + + if (data.description) { + const desc = document.createElement('div'); + desc.className = 'approval-description'; + desc.textContent = data.description; + card.appendChild(desc); + } + + if (data.parameters) { + const paramsToggle = document.createElement('button'); + paramsToggle.className = 'approval-params-toggle'; + paramsToggle.textContent = 'Show parameters'; + const paramsBlock = document.createElement('pre'); + paramsBlock.className = 'approval-params'; + paramsBlock.textContent = data.parameters; + paramsBlock.style.display = 'none'; + paramsToggle.addEventListener('click', () => { + const visible = paramsBlock.style.display !== 'none'; + paramsBlock.style.display = visible ? 'none' : 'block'; + paramsToggle.textContent = visible ? 'Show parameters' : 'Hide parameters'; + }); + card.appendChild(paramsToggle); + card.appendChild(paramsBlock); + } + + const actions = document.createElement('div'); + actions.className = 'approval-actions'; + + const approveBtn = document.createElement('button'); + approveBtn.className = 'approve'; + approveBtn.textContent = 'Approve'; + approveBtn.addEventListener('click', () => sendApprovalAction(data.request_id, 'approve')); + + const alwaysBtn = document.createElement('button'); + alwaysBtn.className = 'always'; + alwaysBtn.textContent = 'Always'; + alwaysBtn.addEventListener('click', () => sendApprovalAction(data.request_id, 'always')); + + const denyBtn = document.createElement('button'); + denyBtn.className = 'deny'; + denyBtn.textContent = 'Deny'; + denyBtn.addEventListener('click', () => sendApprovalAction(data.request_id, 'deny')); + + actions.appendChild(approveBtn); + actions.appendChild(alwaysBtn); + actions.appendChild(denyBtn); + card.appendChild(actions); + + container.appendChild(card); + container.scrollTop = container.scrollHeight; } function loadHistory() { @@ -454,6 +537,8 @@ function appendLogEntry(entry) { msg.textContent = entry.message; div.appendChild(msg); + div.addEventListener('click', () => div.classList.toggle('expanded')); + // Apply current filters as visibility const matchesLevel = levelFilter === 'all' || entry.level === levelFilter; const matchesTarget = !targetFilter || entry.target.toLowerCase().includes(targetFilter); @@ -574,6 +659,14 @@ function renderExtensionCard(ext) { card.appendChild(desc); } + if (ext.url) { + const url = document.createElement('div'); + url.className = 'ext-url'; + url.textContent = ext.url; + url.title = ext.url; + card.appendChild(url); + } + if (ext.tools.length > 0) { const tools = document.createElement('div'); tools.className = 'ext-tools'; @@ -610,7 +703,24 @@ function renderExtensionCard(ext) { function activateExtension(name) { apiFetch('/api/extensions/' + encodeURIComponent(name) + '/activate', { method: 'POST' }) .then((res) => { - if (!res.success) { + if (res.success) { + loadExtensions(); + return; + } + + if (res.auth_url) { + addMessage( + 'system', + 'Opening authentication for **' + name + '**. Complete the flow in the opened tab, then click Activate again.' + ); + window.open(res.auth_url, '_blank'); + } else if (res.awaiting_token) { + addMessage( + 'system', + (res.instructions || 'Please provide an API token for **' + name + '**.') + + '\n\nYou can authenticate via chat: type `Authenticate ' + name + '` and follow the instructions.' + ); + } else { addMessage('system', 'Activate failed: ' + res.message); } loadExtensions(); diff --git a/src/channels/web/static/index.html b/src/channels/web/static/index.html index 9d68e2a4..8ae5be51 100644 --- a/src/channels/web/static/index.html +++ b/src/channels/web/static/index.html @@ -5,6 +5,7 @@ IronClaw + @@ -38,14 +39,6 @@
-
-
-
- - - -
-
diff --git a/src/channels/web/static/style.css b/src/channels/web/static/style.css index 3d276707..bb30ccb2 100644 --- a/src/channels/web/static/style.css +++ b/src/channels/web/static/style.css @@ -184,7 +184,6 @@ body { font-size: 14px; line-height: 1.5; word-wrap: break-word; - white-space: pre-wrap; } .message.user { @@ -192,6 +191,7 @@ body { background: var(--accent); color: #fff; border-bottom-right-radius: 2px; + white-space: pre-wrap; } .message.assistant { @@ -229,6 +229,34 @@ body { padding: 0; } +.message p { margin: 0 0 8px 0; } +.message p:last-child { margin-bottom: 0; } +.message ul, .message ol { margin: 4px 0; padding-left: 20px; } +.message li { margin: 2px 0; } +.message blockquote { + margin: 6px 0; + padding: 4px 12px; + border-left: 3px solid var(--border); + color: var(--text-secondary); +} +.message h1, .message h2, .message h3, +.message h4, .message h5, .message h6 { + margin: 8px 0 4px 0; + line-height: 1.3; +} +.message h1 { font-size: 1.3em; } +.message h2 { font-size: 1.2em; } +.message h3 { font-size: 1.1em; } +.message a { color: var(--accent); } +.message hr { border: none; border-top: 1px solid var(--border); margin: 8px 0; } +.message table { border-collapse: collapse; margin: 6px 0; } +.message th, .message td { + border: 1px solid var(--border); + padding: 4px 8px; + font-size: 13px; +} +.message th { background: var(--bg-tertiary); } + /* Status bar */ .chat-status { padding: 6px 16px; @@ -255,31 +283,75 @@ body { to { transform: rotate(360deg); } } -/* Approval banner */ -.approval-banner { - display: none; - padding: 12px 16px; - background: var(--bg-tertiary); - border-top: 1px solid var(--warning); - gap: 8px; +/* Approval card (inline in chat) */ +.approval-card { + align-self: flex-start; + max-width: 80%; + background: var(--bg-secondary); + border: 1px solid var(--warning); + border-radius: var(--radius); + padding: 14px; + display: flex; flex-direction: column; -} - -.approval-banner.visible { - display: flex; -} - -.approval-banner .approval-info { - font-size: 13px; - color: var(--warning); -} - -.approval-banner .approval-actions { - display: flex; gap: 8px; } -.approval-banner button { +.approval-header { + font-size: 12px; + font-weight: 600; + color: var(--warning); + text-transform: uppercase; + letter-spacing: 0.5px; +} + +.approval-tool-name { + font-size: 14px; + font-weight: 600; + color: var(--text); + font-family: "SFMono-Regular", Consolas, "Liberation Mono", Menlo, monospace; +} + +.approval-description { + font-size: 13px; + color: var(--text-secondary); + line-height: 1.4; +} + +.approval-params-toggle { + background: none; + border: none; + color: var(--accent); + cursor: pointer; + font-size: 12px; + padding: 0; + text-align: left; +} + +.approval-params-toggle:hover { + text-decoration: underline; +} + +.approval-params { + background: var(--code-bg); + padding: 8px 12px; + border-radius: var(--radius); + font-size: 12px; + font-family: "SFMono-Regular", Consolas, "Liberation Mono", Menlo, monospace; + line-height: 1.4; + overflow-x: auto; + color: var(--text-secondary); + margin: 0; + white-space: pre-wrap; + word-break: break-all; +} + +.approval-card .approval-actions { + display: flex; + gap: 8px; + align-items: center; +} + +.approval-card .approval-actions button { padding: 6px 14px; border: 1px solid var(--border); border-radius: var(--radius); @@ -289,24 +361,36 @@ body { color: var(--text); } -.approval-banner button.approve { +.approval-card .approval-actions button:disabled { + opacity: 0.4; + cursor: not-allowed; +} + +.approval-card .approval-actions button.approve { background: var(--success); border-color: var(--success); color: #fff; } -.approval-banner button.always { +.approval-card .approval-actions button.always { background: var(--accent); border-color: var(--accent); color: #fff; } -.approval-banner button.deny { +.approval-card .approval-actions button.deny { background: var(--danger); border-color: var(--danger); color: #fff; } +.approval-resolved { + font-size: 12px; + font-weight: 500; + color: var(--text-secondary); + font-style: italic; +} + /* Chat input */ .chat-input { display: flex; @@ -736,6 +820,7 @@ body { gap: 8px; padding: 1px 12px; white-space: nowrap; + cursor: pointer; } .log-entry:hover { @@ -764,10 +849,22 @@ body { .log-msg { color: var(--text); - white-space: pre-wrap; - word-break: break-all; flex: 1; min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.log-entry.expanded { + white-space: normal; +} + +.log-entry.expanded .log-msg { + white-space: pre-wrap; + word-break: break-all; + overflow: visible; + text-overflow: unset; } /* Log level coloring */ @@ -870,6 +967,15 @@ body { line-height: 1.4; } +.ext-url { + font-size: 12px; + color: var(--text-secondary); + font-family: "SFMono-Regular", Consolas, "Liberation Mono", Menlo, monospace; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + .ext-tools { font-size: 12px; color: var(--text-secondary); diff --git a/src/channels/web/types.rs b/src/channels/web/types.rs index e86d4d9c..1159acb4 100644 --- a/src/channels/web/types.rs +++ b/src/channels/web/types.rs @@ -56,6 +56,15 @@ pub struct HistoryResponse { pub turns: Vec, } +// --- Approval --- + +#[derive(Debug, Deserialize)] +pub struct ApprovalRequest { + pub request_id: String, + /// "approve", "always", or "deny" + pub action: String, +} + // --- SSE Event Types --- #[derive(Debug, Clone, Serialize)] @@ -184,6 +193,8 @@ pub struct ExtensionInfo { pub name: String, pub kind: String, pub description: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub url: Option, pub authenticated: bool, pub active: bool, pub tools: Vec, @@ -216,6 +227,37 @@ pub struct InstallExtensionRequest { pub struct ActionResponse { pub success: bool, pub message: String, + /// Auth URL to open (when activation requires OAuth). + #[serde(skip_serializing_if = "Option::is_none")] + pub auth_url: Option, + /// Whether the extension is waiting for a manual token. + #[serde(skip_serializing_if = "Option::is_none")] + pub awaiting_token: Option, + /// Instructions for manual token entry. + #[serde(skip_serializing_if = "Option::is_none")] + pub instructions: Option, +} + +impl ActionResponse { + pub fn ok(message: impl Into) -> Self { + Self { + success: true, + message: message.into(), + auth_url: None, + awaiting_token: None, + instructions: None, + } + } + + pub fn fail(message: impl Into) -> Self { + Self { + success: false, + message: message.into(), + auth_url: None, + awaiting_token: None, + instructions: None, + } + } } // --- Health --- diff --git a/src/extensions/manager.rs b/src/extensions/manager.rs index 89c30426..f79d7d95 100644 --- a/src/extensions/manager.rs +++ b/src/extensions/manager.rs @@ -215,6 +215,7 @@ impl ExtensionManager { name: server.name.clone(), kind: ExtensionKind::McpServer, description: server.description.clone(), + url: Some(server.url.clone()), authenticated, active, tools, @@ -240,6 +241,7 @@ impl ExtensionManager { name: name.clone(), kind: ExtensionKind::WasmTool, description: None, + url: None, authenticated: true, // WASM tools don't always need auth active, tools: if active { vec![name] } else { Vec::new() }, @@ -263,6 +265,7 @@ impl ExtensionManager { name, kind: ExtensionKind::WasmChannel, description: None, + url: None, authenticated: true, active: true, // If loaded at startup, they're active tools: Vec::new(), diff --git a/src/extensions/mod.rs b/src/extensions/mod.rs index 4d3b8512..7f1a43f2 100644 --- a/src/extensions/mod.rs +++ b/src/extensions/mod.rs @@ -176,6 +176,9 @@ pub struct InstalledExtension { pub kind: ExtensionKind, #[serde(skip_serializing_if = "Option::is_none")] pub description: Option, + /// Server or source URL (e.g. MCP server endpoint). + #[serde(skip_serializing_if = "Option::is_none")] + pub url: Option, pub authenticated: bool, pub active: bool, /// Tool names if active. diff --git a/src/llm/nearai_chat.rs b/src/llm/nearai_chat.rs index 35fd7593..5a76e7b6 100644 --- a/src/llm/nearai_chat.rs +++ b/src/llm/nearai_chat.rs @@ -332,12 +332,25 @@ impl From for ChatCompletionMessage { Role::Assistant => "assistant", Role::Tool => "tool", }; + let tool_calls = msg.tool_calls.map(|calls| { + calls + .into_iter() + .map(|tc| ChatCompletionToolCall { + id: tc.id, + call_type: "function".to_string(), + function: ChatCompletionToolCallFunction { + name: tc.name, + arguments: tc.arguments.to_string(), + }, + }) + .collect() + }); Self { role: role.to_string(), content: Some(msg.content), tool_call_id: msg.tool_call_id, name: msg.name, - tool_calls: None, + tool_calls, } } } @@ -423,4 +436,61 @@ mod tests { assert_eq!(chat_msg.tool_call_id, Some("call_123".to_string())); assert_eq!(chat_msg.name, Some("my_tool".to_string())); } + + #[test] + fn test_assistant_with_tool_calls_conversion() { + use crate::llm::ToolCall; + + let tool_calls = vec![ + ToolCall { + id: "call_1".to_string(), + name: "list_issues".to_string(), + arguments: serde_json::json!({"owner": "foo", "repo": "bar"}), + }, + ToolCall { + id: "call_2".to_string(), + name: "search".to_string(), + arguments: serde_json::json!({"query": "test"}), + }, + ]; + + let msg = ChatMessage::assistant_with_tool_calls("", tool_calls); + let chat_msg: ChatCompletionMessage = msg.into(); + + assert_eq!(chat_msg.role, "assistant"); + + let tc = chat_msg.tool_calls.expect("tool_calls present"); + assert_eq!(tc.len(), 2); + assert_eq!(tc[0].id, "call_1"); + assert_eq!(tc[0].function.name, "list_issues"); + assert_eq!(tc[0].call_type, "function"); + assert_eq!(tc[1].id, "call_2"); + assert_eq!(tc[1].function.name, "search"); + } + + #[test] + fn test_assistant_without_tool_calls_has_none() { + let msg = ChatMessage::assistant("Hello"); + let chat_msg: ChatCompletionMessage = msg.into(); + assert!(chat_msg.tool_calls.is_none()); + } + + #[test] + fn test_tool_call_arguments_serialized_to_string() { + use crate::llm::ToolCall; + + let tc = ToolCall { + id: "call_1".to_string(), + name: "test".to_string(), + arguments: serde_json::json!({"key": "value"}), + }; + let msg = ChatMessage::assistant_with_tool_calls("", vec![tc]); + let chat_msg: ChatCompletionMessage = msg.into(); + + let calls = chat_msg.tool_calls.unwrap(); + // Arguments should be a JSON string, not a nested object + let parsed: serde_json::Value = + serde_json::from_str(&calls[0].function.arguments).expect("valid JSON string"); + assert_eq!(parsed["key"], "value"); + } } diff --git a/src/llm/provider.rs b/src/llm/provider.rs index 63469664..bb6dcdcb 100644 --- a/src/llm/provider.rs +++ b/src/llm/provider.rs @@ -27,6 +27,11 @@ pub struct ChatMessage { /// Name of the tool for tool results. #[serde(skip_serializing_if = "Option::is_none")] pub name: Option, + /// Tool calls requested by the assistant (for conversation replay). + /// OpenAI-compatible APIs require the assistant message to include + /// tool_calls when followed by tool result messages. + #[serde(skip_serializing_if = "Option::is_none")] + pub tool_calls: Option>, } impl ChatMessage { @@ -37,6 +42,7 @@ impl ChatMessage { content: content.into(), tool_call_id: None, name: None, + tool_calls: None, } } @@ -47,6 +53,7 @@ impl ChatMessage { content: content.into(), tool_call_id: None, name: None, + tool_calls: None, } } @@ -57,6 +64,28 @@ impl ChatMessage { content: content.into(), tool_call_id: None, name: None, + tool_calls: None, + } + } + + /// Create an assistant message that requested tool calls. + /// + /// OpenAI-compatible APIs require the assistant message to carry the + /// `tool_calls` array when followed by tool-result messages. + pub fn assistant_with_tool_calls( + content: impl Into, + tool_calls: Vec, + ) -> Self { + Self { + role: Role::Assistant, + content: content.into(), + tool_call_id: None, + name: None, + tool_calls: if tool_calls.is_empty() { + None + } else { + Some(tool_calls) + }, } } @@ -71,6 +100,7 @@ impl ChatMessage { content: content.into(), tool_call_id: Some(tool_call_id.into()), name: Some(name.into()), + tool_calls: None, } } } diff --git a/src/llm/reasoning.rs b/src/llm/reasoning.rs index efad83e3..6d8bf183 100644 --- a/src/llm/reasoning.rs +++ b/src/llm/reasoning.rs @@ -5,6 +5,7 @@ use std::sync::Arc; use serde::{Deserialize, Serialize}; use crate::error::LlmError; + use crate::llm::{ ChatMessage, CompletionRequest, LlmProvider, ToolCall, ToolCompletionRequest, ToolDefinition, }; @@ -309,10 +310,10 @@ Respond in JSON format: return Ok(RespondResult::ToolCalls(response.tool_calls)); } - // No tool calls - clean up the response let content = response .content .unwrap_or_else(|| "I'm not sure how to respond to that.".to_string()); + Ok(RespondResult::Text(clean_response(&content))) } else { // No tools, use simple completion diff --git a/src/tools/builtin/extension_tools.rs b/src/tools/builtin/extension_tools.rs index 41c9d708..6b4ca532 100644 --- a/src/tools/builtin/extension_tools.rs +++ b/src/tools/builtin/extension_tools.rs @@ -226,6 +226,38 @@ impl Tool for ToolAuthTool { .await .map_err(|e| ToolError::ExecutionFailed(e.to_string()))?; + // Auto-activate after successful auth so tools are available immediately + if result.status == "authenticated" { + match self.manager.activate(name).await { + Ok(activate_result) => { + let output = serde_json::json!({ + "status": "authenticated_and_activated", + "name": name, + "tools_loaded": activate_result.tools_loaded, + "message": activate_result.message, + }); + return Ok(ToolOutput::success(output, start.elapsed())); + } + Err(e) => { + tracing::warn!( + "Extension '{}' authenticated but activation failed: {}", + name, + e + ); + let output = serde_json::json!({ + "status": "authenticated", + "name": name, + "activation_error": e.to_string(), + "message": format!( + "Authenticated but activation failed: {}. Try tool_activate.", + e + ), + }); + return Ok(ToolOutput::success(output, start.elapsed())); + } + } + } + let output = serde_json::to_value(&result) .unwrap_or_else(|_| serde_json::json!({"error": "serialization failed"})); diff --git a/src/tools/mcp/protocol.rs b/src/tools/mcp/protocol.rs index 4f5b5d98..ec1d367d 100644 --- a/src/tools/mcp/protocol.rs +++ b/src/tools/mcp/protocol.rs @@ -15,7 +15,12 @@ pub struct McpTool { pub description: String, /// JSON Schema for input parameters. /// Defaults to empty object schema if not provided. - #[serde(default = "default_input_schema")] + /// MCP protocol uses camelCase `inputSchema`. + #[serde( + default = "default_input_schema", + rename = "inputSchema", + alias = "input_schema" + )] pub input_schema: serde_json::Value, /// Optional annotations from the MCP server. #[serde(default)] @@ -285,3 +290,95 @@ impl ContentBlock { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_mcp_tool_deserialize_camel_case_input_schema() { + // MCP protocol uses camelCase "inputSchema" + let json = serde_json::json!({ + "name": "list_issues", + "description": "List GitHub issues", + "inputSchema": { + "type": "object", + "properties": { + "owner": { "type": "string" }, + "repo": { "type": "string" } + }, + "required": ["owner", "repo"] + } + }); + + let tool: McpTool = serde_json::from_value(json).expect("deserialize McpTool"); + assert_eq!(tool.name, "list_issues"); + assert_eq!(tool.description, "List GitHub issues"); + + // The schema must have the properties, not the empty default + let props = tool.input_schema.get("properties").expect("has properties"); + assert!(props.get("owner").is_some()); + assert!(props.get("repo").is_some()); + } + + #[test] + fn test_mcp_tool_deserialize_snake_case_alias() { + // Also accept snake_case "input_schema" for flexibility + let json = serde_json::json!({ + "name": "search", + "description": "Search", + "input_schema": { + "type": "object", + "properties": { + "query": { "type": "string" } + } + } + }); + + let tool: McpTool = serde_json::from_value(json).expect("deserialize McpTool"); + let props = tool.input_schema.get("properties").expect("has properties"); + assert!(props.get("query").is_some()); + } + + #[test] + fn test_mcp_tool_missing_schema_gets_default() { + let json = serde_json::json!({ + "name": "ping", + "description": "Ping" + }); + + let tool: McpTool = serde_json::from_value(json).expect("deserialize McpTool"); + assert_eq!(tool.input_schema["type"], "object"); + assert!(tool.input_schema["properties"].is_object()); + } + + #[test] + fn test_mcp_tool_roundtrip_preserves_schema() { + // Simulate what list_tools returns from a real MCP server + let server_response = serde_json::json!({ + "tools": [{ + "name": "github-copilot_list_issues", + "description": "List issues for a repository", + "inputSchema": { + "type": "object", + "properties": { + "owner": { "type": "string", "description": "Repository owner" }, + "repo": { "type": "string", "description": "Repository name" }, + "state": { "type": "string", "enum": ["open", "closed", "all"] } + }, + "required": ["owner", "repo"] + } + }] + }); + + let result: ListToolsResult = + serde_json::from_value(server_response).expect("deserialize ListToolsResult"); + assert_eq!(result.tools.len(), 1); + + let tool = &result.tools[0]; + assert_eq!(tool.name, "github-copilot_list_issues"); + + let required = tool.input_schema.get("required").expect("has required"); + assert!(required.as_array().expect("is array").len() == 2); + } +}