diff --git a/FEATURE_PARITY.md b/FEATURE_PARITY.md index d00ff5e5df..85348de539 100644 --- a/FEATURE_PARITY.md +++ b/FEATURE_PARITY.md @@ -68,7 +68,7 @@ This document tracks feature parity between IronClaw (Rust implementation) and O | REPL (simple) | ✅ | ✅ | - | For testing | | WASM channels | ❌ | ✅ | - | IronClaw innovation; host resolves owner scope vs sender identity | | WhatsApp | ✅ | ❌ | P1 | Baileys (Web), same-phone mode with echo detection | -| Telegram | ✅ | ✅ | - | WASM channel(MTProto), DM pairing, caption, /start, bot_username, DM topics, setup-time owner verification, owner-scoped persistence | +| Telegram | ✅ | ✅ | - | WASM channel(MTProto), DM pairing, caption, /start, bot_username, DM topics, setup-time owner auto-verification, owner-scoped persistence | | Discord | ✅ | ❌ | P2 | discord.js, thread parent binding inheritance | | Signal | ✅ | ✅ | P2 | signal-cli daemonPC, SSE listener HTTP/JSON-R, user/group allowlists, DM pairing | | Slack | ✅ | ✅ | - | WASM tool | diff --git a/src/agent/agent_loop.rs b/src/agent/agent_loop.rs index aaaad879d1..83d971ef1a 100644 --- a/src/agent/agent_loop.rs +++ b/src/agent/agent_loop.rs @@ -54,16 +54,65 @@ pub(crate) fn truncate_for_preview(output: &str, max_chars: usize) -> String { } } +#[cfg(test)] fn resolve_routine_notification_user(metadata: &serde_json::Value) -> Option { - metadata - .get("notify_user") - .and_then(|value| value.as_str()) - .or_else(|| metadata.get("owner_id").and_then(|value| value.as_str())) + resolve_owner_scope_notification_user( + metadata.get("notify_user").and_then(|value| value.as_str()), + metadata.get("owner_id").and_then(|value| value.as_str()), + ) +} + +fn trimmed_option(value: Option<&str>) -> Option { + value .map(str::trim) .filter(|value| !value.is_empty()) .map(ToOwned::to_owned) } +fn resolve_owner_scope_notification_user( + explicit_user: Option<&str>, + owner_fallback: Option<&str>, +) -> Option { + trimmed_option(explicit_user).or_else(|| trimmed_option(owner_fallback)) +} + +async fn resolve_channel_notification_user( + extension_manager: Option<&Arc>, + channel: Option<&str>, + explicit_user: Option<&str>, + owner_fallback: Option<&str>, +) -> Option { + if let Some(user) = trimmed_option(explicit_user) { + return Some(user); + } + + if let Some(channel_name) = trimmed_option(channel) + && let Some(extension_manager) = extension_manager + && let Some(target) = extension_manager + .notification_target_for_channel(&channel_name) + .await + { + return Some(target); + } + + resolve_owner_scope_notification_user(explicit_user, owner_fallback) +} + +async fn resolve_routine_notification_target( + extension_manager: Option<&Arc>, + metadata: &serde_json::Value, +) -> Option { + resolve_channel_notification_user( + extension_manager, + metadata + .get("notify_channel") + .and_then(|value| value.as_str()), + metadata.get("notify_user").and_then(|value| value.as_str()), + metadata.get("owner_id").and_then(|value| value.as_str()), + ) + .await +} + fn should_fallback_routine_notification(error: &ChannelError) -> bool { !matches!(error, ChannelError::MissingRoutingTarget { .. }) } @@ -395,11 +444,13 @@ impl Agent { .timezone .clone() .or_else(|| Some(self.config.default_timezone.clone())); - if let Some(channel) = &hb_config.notify_channel { - let user = hb_config - .notify_user - .clone() - .unwrap_or_else(|| self.owner_id().to_string()); + let heartbeat_notify_user = resolve_owner_scope_notification_user( + hb_config.notify_user.as_deref(), + Some(self.owner_id()), + ); + if let Some(channel) = &hb_config.notify_channel + && let Some(user) = heartbeat_notify_user.as_deref() + { config = config.with_notify(user, channel); } @@ -409,26 +460,32 @@ impl Agent { // Spawn notification forwarder that routes through channel manager let notify_channel = hb_config.notify_channel.clone(); - let notify_user = hb_config - .notify_user - .clone() - .unwrap_or_else(|| self.owner_id().to_string()); + let notify_target = resolve_channel_notification_user( + self.deps.extension_manager.as_ref(), + hb_config.notify_channel.as_deref(), + hb_config.notify_user.as_deref(), + Some(self.owner_id()), + ) + .await; + let notify_user = heartbeat_notify_user; let channels = self.channels.clone(); tokio::spawn(async move { while let Some(response) = notify_rx.recv().await { // Try the configured channel first, fall back to // broadcasting on all channels. - let targeted_ok = if let Some(ref channel) = notify_channel { + let targeted_ok = if let Some(ref channel) = notify_channel + && let Some(ref user) = notify_target + { channels - .broadcast(channel, ¬ify_user, response.clone()) + .broadcast(channel, user, response.clone()) .await .is_ok() } else { false }; - if !targeted_ok { - let results = channels.broadcast_all(¬ify_user, response).await; + if !targeted_ok && let Some(ref user) = notify_user { + let results = channels.broadcast_all(user, response).await; for (ch, result) in results { if let Err(e) = result { tracing::warn!( @@ -496,6 +553,7 @@ impl Agent { // Spawn notification forwarder (mirrors heartbeat pattern) let channels = self.channels.clone(); + let extension_manager = self.deps.extension_manager.clone(); tokio::spawn(async move { while let Some(response) = notify_rx.recv().await { let notify_channel = response @@ -503,7 +561,18 @@ impl Agent { .get("notify_channel") .and_then(|v| v.as_str()) .map(|s| s.to_string()); - let Some(user) = resolve_routine_notification_user(&response.metadata) + let fallback_user = resolve_owner_scope_notification_user( + response + .metadata + .get("notify_user") + .and_then(|v| v.as_str()), + response.metadata.get("owner_id").and_then(|v| v.as_str()), + ); + let Some(user) = resolve_routine_notification_target( + extension_manager.as_ref(), + &response.metadata, + ) + .await else { tracing::warn!( notify_channel = ?notify_channel, @@ -537,7 +606,7 @@ impl Agent { false }; - if !targeted_ok { + if !targeted_ok && let Some(user) = fallback_user { let results = channels.broadcast_all(&user, response).await; for (ch, result) in results { if let Err(e) = result { @@ -624,6 +693,29 @@ impl Agent { // Store successfully extracted document text in workspace for indexing self.store_extracted_documents(&message).await; + // Event-triggered routines consume plain user input before it enters + // the normal chat/tool pipeline. This avoids a duplicate turn where + // the main agent responds and the routine also fires on the same + // inbound message. + if !message.is_internal + && matches!( + SubmissionParser::parse(&message.content), + Submission::UserInput { .. } + ) + && let Some(ref engine) = routine_engine_for_loop + { + let fired = engine.check_event_triggers(&message).await; + if fired > 0 { + tracing::debug!( + channel = %message.channel, + user = %message.user_id, + fired, + "Consumed inbound user message with matching event-triggered routine(s)" + ); + continue; + } + } + match self.handle_message(&message).await { Ok(Some(response)) if !response.is_empty() => { // Hook: BeforeOutbound — allow hooks to modify or suppress outbound @@ -696,14 +788,6 @@ impl Agent { } } } - - // Check event triggers (cheap in-memory regex, fires async if matched) - if let Some(ref engine) = routine_engine_for_loop { - let fired = engine.check_event_triggers(&message).await; - if fired > 0 { - tracing::debug!("Fired {} event-triggered routines", fired); - } - } } // Cleanup diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs index 1eb49e3cf5..27ef7cdce9 100644 --- a/src/channels/web/server.rs +++ b/src/channels/web/server.rs @@ -2243,14 +2243,7 @@ async fn extensions_setup_submit_handler( resp.auth_url = result.auth_url.clone(); resp.verification = result.verification.clone(); resp.instructions = result.verification.as_ref().map(|v| v.instructions.clone()); - if result.verification.is_some() { - state.sse.broadcast(SseEvent::AuthRequired { - extension_name: name.clone(), - instructions: resp.instructions.clone(), - auth_url: None, - setup_url: None, - }); - } else { + if result.verification.is_none() { // Broadcast auth_completed so the chat UI can dismiss any in-progress // auth card or setup modal that was triggered by tool_auth/tool_activate. state.sse.broadcast(SseEvent::AuthCompleted { @@ -2981,6 +2974,92 @@ mod tests { ); } + #[tokio::test] + async fn test_extensions_setup_submit_telegram_verification_does_not_broadcast_auth_required() { + use axum::body::Body; + use tokio::time::{Duration, timeout}; + use tower::ServiceExt; + + let secrets = test_secrets_store(); + let (ext_mgr, _wasm_tools_dir, wasm_channels_dir) = test_ext_mgr(secrets); + + std::fs::write( + wasm_channels_dir.path().join("telegram.wasm"), + b"\0asm fake", + ) + .expect("write fake telegram wasm"); + let caps = serde_json::json!({ + "type": "channel", + "name": "telegram", + "setup": { + "required_secrets": [ + { + "name": "telegram_bot_token", + "prompt": "Enter your Telegram Bot API token (from @BotFather)" + } + ] + } + }); + std::fs::write( + wasm_channels_dir.path().join("telegram.capabilities.json"), + serde_json::to_string(&caps).expect("serialize telegram caps"), + ) + .expect("write telegram caps"); + + ext_mgr + .set_test_telegram_pending_verification("iclaw-7qk2m9", Some("test_hot_bot")) + .await; + + let state = test_gateway_state(Some(ext_mgr)); + let mut receiver = state.sse.sender().subscribe(); + let app = Router::new() + .route( + "/api/extensions/{name}/setup", + post(extensions_setup_submit_handler), + ) + .with_state(state); + + let req_body = serde_json::json!({ + "secrets": { + "telegram_bot_token": "123456789:ABCdefGhI" + } + }); + let req = axum::http::Request::builder() + .method("POST") + .uri("/api/extensions/telegram/setup") + .header("content-type", "application/json") + .body(Body::from(req_body.to_string())) + .expect("request"); + + let resp = ServiceExt::>::oneshot(app, req) + .await + .expect("response"); + assert_eq!(resp.status(), StatusCode::OK); + + let body = axum::body::to_bytes(resp.into_body(), 1024 * 64) + .await + .expect("body"); + let parsed: serde_json::Value = serde_json::from_slice(&body).expect("json response"); + assert_eq!(parsed["success"], serde_json::Value::Bool(true)); + assert_eq!(parsed["activated"], serde_json::Value::Bool(false)); + assert_eq!(parsed["verification"]["code"], "iclaw-7qk2m9"); + + let deadline = tokio::time::Instant::now() + Duration::from_millis(100); + loop { + let remaining = deadline.saturating_duration_since(tokio::time::Instant::now()); + if remaining.is_zero() { + break; + } + match timeout(remaining, receiver.recv()).await { + Ok(Ok(crate::channels::web::types::SseEvent::AuthRequired { .. })) => { + panic!("verification responses should not emit auth_required SSE events") + } + Ok(Ok(_)) => continue, + Ok(Err(_)) | Err(_) => break, + } + } + } + fn expired_flow_created_at() -> Option { std::time::Instant::now() .checked_sub(oauth_defaults::OAUTH_FLOW_EXPIRY + std::time::Duration::from_secs(1)) diff --git a/src/channels/web/static/app.js b/src/channels/web/static/app.js index 127c18fa0c..9d931500cd 100644 --- a/src/channels/web/static/app.js +++ b/src/channels/web/static/app.js @@ -527,7 +527,6 @@ function enableChatInput() { const btn = document.getElementById('send-btn'); if (input) { input.disabled = false; - input.placeholder = I18n.t('chat.inputPlaceholder'); } if (btn) btn.disabled = false; } @@ -1205,11 +1204,13 @@ function showJobCard(data) { // --- Auth card --- function handleAuthRequired(data) { - setAuthFlowPending(true, data.instructions); if (data.auth_url) { + setAuthFlowPending(true, data.instructions); // OAuth flow: show the global auth prompt with an OAuth button + optional token paste field. showAuthCard(data); } else { + if (getConfigureOverlay(data.extension_name)) return; + setAuthFlowPending(true, data.instructions); // Setup flow: fetch the extension's credential schema and show the multi-field // configure modal (the same UI used by the Extensions tab "Setup" button). showConfigureModal(data.extension_name); @@ -1433,13 +1434,11 @@ function setAuthFlowPending(pending, instructions) { if (authFlowPending) { input.disabled = true; btn.disabled = true; - input.placeholder = instructions || 'Complete extension auth to continue chatting'; return; } if (!currentThreadIsReadOnly) { input.disabled = false; btn.disabled = false; - input.placeholder = I18n.t('chat.inputPlaceholder'); } } @@ -2712,8 +2711,11 @@ function renderConfigureModal(name, secrets) { const overlay = document.createElement('div'); overlay.className = 'configure-overlay'; overlay.setAttribute('data-extension-name', name); + overlay.dataset.telegramVerificationState = 'idle'; overlay.addEventListener('click', (e) => { - if (e.target === overlay) closeConfigureModal(); + if (e.target !== overlay) return; + if (name === 'telegram' && overlay.dataset.telegramVerificationState === 'waiting') return; + closeConfigureModal(); }); const modal = document.createElement('div'); @@ -2737,6 +2739,7 @@ function renderConfigureModal(name, secrets) { for (const secret of secrets) { const field = document.createElement('div'); field.className = 'configure-field'; + field.dataset.secretName = secret.name; const label = document.createElement('label'); label.textContent = secret.prompt; @@ -2781,6 +2784,16 @@ function renderConfigureModal(name, secrets) { modal.appendChild(form); + const error = document.createElement('div'); + error.className = 'configure-inline-error'; + error.style.display = 'none'; + modal.appendChild(error); + + const status = document.createElement('div'); + status.className = 'configure-inline-status'; + status.style.display = 'none'; + modal.appendChild(status); + const actions = document.createElement('div'); actions.className = 'configure-actions'; @@ -2807,12 +2820,20 @@ function renderTelegramVerificationChallenge(overlay, verification) { if (!overlay || !verification) return; const modal = overlay.querySelector('.configure-modal'); if (!modal) return; + const telegramField = modal.querySelector('.configure-field[data-secret-name="telegram_bot_token"]'); let panel = modal.querySelector('.configure-verification'); if (!panel) { panel = document.createElement('div'); panel.className = 'configure-verification'; - modal.insertBefore(panel, modal.querySelector('.configure-actions')); + } + if (telegramField && telegramField.parentNode) { + telegramField.insertAdjacentElement('afterend', panel); + } else { + modal.insertBefore( + panel, + modal.querySelector('.configure-inline-error') || modal.querySelector('.configure-actions') + ); } panel.innerHTML = ''; @@ -2827,10 +2848,15 @@ function renderTelegramVerificationChallenge(overlay, verification) { instructions.textContent = verification.instructions; panel.appendChild(instructions); - const code = document.createElement('code'); - code.className = 'configure-verification-code'; - code.textContent = verification.code; - panel.appendChild(code); + const commandLabel = document.createElement('div'); + commandLabel.className = 'configure-verification-instructions'; + commandLabel.textContent = I18n.t('config.telegramCommandLabel'); + panel.appendChild(commandLabel); + + const command = document.createElement('code'); + command.className = 'configure-verification-code'; + command.textContent = '/start ' + verification.code; + panel.appendChild(command); if (verification.deep_link) { const link = document.createElement('a'); @@ -2843,7 +2869,57 @@ function renderTelegramVerificationChallenge(overlay, verification) { } } -function submitConfigureModal(name, fields) { +function getConfigurePrimaryButton(overlay) { + return overlay && overlay.querySelector('.configure-actions button.btn-ext.activate'); +} + +function getConfigureCancelButton(overlay) { + return overlay && overlay.querySelector('.configure-actions button.btn-ext.remove'); +} + +function setConfigureInlineError(overlay, message) { + const error = overlay && overlay.querySelector('.configure-inline-error'); + if (!error) return; + error.textContent = message || ''; + error.style.display = message ? 'block' : 'none'; +} + +function clearConfigureInlineError(overlay) { + setConfigureInlineError(overlay, ''); +} + +function setConfigureInlineStatus(overlay, message) { + const status = overlay && overlay.querySelector('.configure-inline-status'); + if (!status) return; + status.textContent = message || ''; + status.style.display = message ? 'block' : 'none'; +} + +function setTelegramConfigureState(overlay, fields, state) { + if (!overlay) return; + overlay.dataset.telegramVerificationState = state; + + const primaryBtn = getConfigurePrimaryButton(overlay); + const cancelBtn = getConfigureCancelButton(overlay); + const waiting = state === 'waiting'; + const retry = state === 'retry'; + + setConfigureInlineStatus(overlay, waiting ? I18n.t('config.telegramOwnerWaiting') : ''); + + if (primaryBtn) { + primaryBtn.style.display = waiting ? 'none' : ''; + primaryBtn.disabled = false; + primaryBtn.textContent = retry ? I18n.t('config.telegramStartOver') : I18n.t('config.save'); + } + if (cancelBtn) cancelBtn.disabled = waiting; +} + +function startTelegramAutoVerify(name, fields) { + window.setTimeout(() => submitConfigureModal(name, fields, { telegramAutoVerify: true }), 0); +} + +function submitConfigureModal(name, fields, options) { + options = options || {}; const secrets = {}; for (const f of fields) { if (f.input.value.trim()) { @@ -2851,13 +2927,15 @@ function submitConfigureModal(name, fields) { } } - // Disable buttons to prevent double-submit const overlay = getConfigureOverlay(name) || document.querySelector('.configure-overlay'); + const isTelegram = name === 'telegram'; + clearConfigureInlineError(overlay); + + // Disable buttons to prevent double-submit var btns = overlay ? overlay.querySelectorAll('.configure-actions button') : []; btns.forEach(function(b) { b.disabled = true; }); - if (overlay && name === 'telegram') { - const submitBtn = overlay.querySelector('.configure-actions button.btn-ext.activate'); - if (submitBtn) submitBtn.textContent = I18n.t('config.telegramOwnerWaiting'); + if (overlay && isTelegram) { + setTelegramConfigureState(overlay, fields, 'waiting'); } apiFetch('/api/extensions/' + encodeURIComponent(name) + '/setup', { @@ -2866,13 +2944,20 @@ function submitConfigureModal(name, fields) { }) .then((res) => { if (res.success) { - if (res.verification && name === 'telegram') { - btns.forEach(function(b) { b.disabled = false; }); + if (res.verification && isTelegram) { renderTelegramVerificationChallenge(overlay, res.verification); fields.forEach(function(f) { f.input.value = ''; }); - const submitBtn = overlay.querySelector('.configure-actions button.btn-ext.activate'); - if (submitBtn) submitBtn.textContent = I18n.t('config.telegramVerifyOwner'); - showToast(res.message || res.verification.instructions, 'info'); + setTelegramConfigureState(overlay, fields, 'waiting'); + // Once the verification challenge is rendered inline, the global auth lock + // should not keep the chat composer disabled for this setup-driven flow. + setAuthFlowPending(false); + enableChatInput(); + if (!options.telegramAutoVerify) { + startTelegramAutoVerify(name, fields); + return; + } + setTelegramConfigureState(overlay, fields, 'retry'); + setConfigureInlineError(overlay, I18n.t('config.telegramStartOverHint')); return; } @@ -2891,13 +2976,13 @@ function submitConfigureModal(name, fields) { } else { // Keep modal open so the user can correct their input and retry. btns.forEach(function(b) { b.disabled = false; }); - if (name === 'telegram') { - const submitBtn = overlay && overlay.querySelector('.configure-actions button.btn-ext.activate'); + setConfigureInlineError(overlay, res.message || 'Configuration failed'); + if (isTelegram) { const hasVerification = overlay && overlay.querySelector('.configure-verification'); - if (submitBtn) { - submitBtn.textContent = hasVerification - ? I18n.t('config.telegramVerifyOwner') - : I18n.t('config.save'); + if (options.telegramAutoVerify || hasVerification) { + setTelegramConfigureState(overlay, fields, 'retry'); + } else { + setTelegramConfigureState(overlay, fields, 'idle'); } } showToast(res.message || 'Configuration failed', 'error'); @@ -2905,13 +2990,13 @@ function submitConfigureModal(name, fields) { }) .catch((err) => { btns.forEach(function(b) { b.disabled = false; }); - if (name === 'telegram') { - const submitBtn = overlay && overlay.querySelector('.configure-actions button.btn-ext.activate'); + setConfigureInlineError(overlay, 'Configuration failed: ' + err.message); + if (isTelegram) { const hasVerification = overlay && overlay.querySelector('.configure-verification'); - if (submitBtn) { - submitBtn.textContent = hasVerification - ? I18n.t('config.telegramVerifyOwner') - : I18n.t('config.save'); + if (options.telegramAutoVerify || hasVerification) { + setTelegramConfigureState(overlay, fields, 'retry'); + } else { + setTelegramConfigureState(overlay, fields, 'idle'); } } showToast('Configuration failed: ' + err.message, 'error'); @@ -2922,6 +3007,10 @@ function closeConfigureModal(extensionName) { if (typeof extensionName !== 'string') extensionName = null; const existing = getConfigureOverlay(extensionName); if (existing) existing.remove(); + if (!document.querySelector('.configure-overlay') && !document.querySelector('.auth-card')) { + setAuthFlowPending(false); + enableChatInput(); + } } // Validate that a server-supplied OAuth URL is HTTPS before opening a popup. diff --git a/src/channels/web/static/i18n/en.js b/src/channels/web/static/i18n/en.js index 42e996da0a..49bec76204 100644 --- a/src/channels/web/static/i18n/en.js +++ b/src/channels/web/static/i18n/en.js @@ -342,10 +342,12 @@ I18n.register('en', { // Configure 'config.title': 'Configure {name}', - 'config.telegramOwnerHint': 'After saving, IronClaw will show a one-time code. Send `/start CODE` to your bot in Telegram, then click Verify owner.', + 'config.telegramOwnerHint': 'After saving, IronClaw will show a one-time code. Send `/start CODE` to your bot in Telegram and IronClaw will finish setup automatically.', 'config.telegramChallengeTitle': 'Telegram owner verification', 'config.telegramOwnerWaiting': 'Waiting for Telegram owner verification...', - 'config.telegramVerifyOwner': 'Verify owner', + 'config.telegramCommandLabel': 'Send this in Telegram:', + 'config.telegramStartOver': 'Start over', + 'config.telegramStartOverHint': 'Telegram verification did not complete. Click Start over to generate a new code and try again.', 'config.telegramOpenBot': 'Open bot in Telegram', 'config.optional': ' (optional)', 'config.alreadySet': '(already set — leave empty to keep)', diff --git a/src/channels/web/static/i18n/zh-CN.js b/src/channels/web/static/i18n/zh-CN.js index 8a7fd520c4..d31cc0df91 100644 --- a/src/channels/web/static/i18n/zh-CN.js +++ b/src/channels/web/static/i18n/zh-CN.js @@ -342,6 +342,12 @@ I18n.register('zh-CN', { // 配置 'config.title': '配置 {name}', + 'config.telegramOwnerHint': '保存后,IronClaw 会显示一次性验证码。将 `/start CODE` 发送给你的 Telegram 机器人,IronClaw 会自动完成设置。', + 'config.telegramChallengeTitle': 'Telegram 所有者验证', + 'config.telegramOwnerWaiting': '正在等待 Telegram 所有者验证...', + 'config.telegramCommandLabel': '请在 Telegram 中发送:', + 'config.telegramStartOver': '重新开始', + 'config.telegramStartOverHint': 'Telegram 验证未完成。点击“重新开始”以生成新的验证码并重试。', 'config.optional': '(可选)', 'config.alreadySet': '(已设置 — 留空以保持不变)', 'config.alreadyConfigured': '已配置', diff --git a/src/channels/web/static/style.css b/src/channels/web/static/style.css index 44fd91762f..06d9665a20 100644 --- a/src/channels/web/static/style.css +++ b/src/channels/web/static/style.css @@ -2952,6 +2952,28 @@ body { text-decoration: underline; } +.configure-inline-error { + margin: 16px 0 0 0; + padding: 10px 12px; + border-radius: 8px; + background: rgba(220, 38, 38, 0.12); + border: 1px solid rgba(220, 38, 38, 0.35); + color: #fca5a5; + font-size: 13px; + line-height: 1.5; +} + +.configure-inline-status { + margin: 16px 0 0 0; + padding: 10px 12px; + border-radius: 8px; + background: var(--bg-secondary); + border: 1px solid var(--border); + color: var(--text-secondary); + font-size: 13px; + line-height: 1.5; +} + .configure-form { display: flex; flex-direction: column; diff --git a/src/extensions/manager.rs b/src/extensions/manager.rs index 471f10cf89..00d787a5a3 100644 --- a/src/extensions/manager.rs +++ b/src/extensions/manager.rs @@ -140,6 +140,13 @@ struct TelegramGetUpdatesResponse { description: Option, } +#[derive(Debug, serde::Deserialize)] +struct TelegramApiOkResponse { + ok: bool, + #[serde(default)] + description: Option, +} + #[derive(Debug, serde::Deserialize)] struct TelegramUpdate { update_id: i64, @@ -204,7 +211,7 @@ fn channel_auth_instructions( ) -> String { if channel_name == TELEGRAM_CHANNEL_NAME && secret.name == "telegram_bot_token" { return format!( - "{} After you submit it, IronClaw will show a one-time verification code. Send `/start CODE` to your bot in Telegram, then verify again to bind the owner.", + "{} After you submit it, IronClaw will show a one-time verification code. Send `/start CODE` to your bot in Telegram and IronClaw will finish setup automatically.", secret.prompt ); } @@ -237,10 +244,12 @@ fn telegram_verification_deep_link(bot_username: Option<&str>, code: &str) -> Op fn telegram_verification_instructions(bot_username: Option<&str>, code: &str) -> String { if let Some(username) = bot_username.filter(|username| !username.trim().is_empty()) { - return format!("Send `/start {code}` to @{username}, then click Verify owner."); + return format!( + "Send `/start {code}` to @{username} in Telegram. IronClaw will finish setup automatically." + ); } - format!("Send `/start {code}` to your Telegram bot, then click Verify owner.") + format!("Send `/start {code}` to your Telegram bot. IronClaw will finish setup automatically.") } fn telegram_message_matches_verification_code(text: &str, code: &str) -> bool { @@ -253,6 +262,42 @@ fn telegram_message_matches_verification_code(text: &str, code: &str) -> bool { .any(|token| token == code) } +async fn send_telegram_text_message( + client: &reqwest::Client, + endpoint: &str, + chat_id: i64, + text: &str, +) -> Result<(), ExtensionError> { + let response = client + .post(endpoint) + .json(&serde_json::json!({ + "chat_id": chat_id, + "text": text, + })) + .send() + .await + .map_err(|e| telegram_request_error("sendMessage", &e))?; + + if !response.status().is_success() { + return Err(ExtensionError::Other(format!( + "Telegram sendMessage failed (HTTP {})", + response.status() + ))); + } + + let payload: TelegramApiOkResponse = response + .json() + .await + .map_err(|e| telegram_response_parse_error("sendMessage", &e))?; + if !payload.ok { + return Err(ExtensionError::Other(payload.description.unwrap_or_else( + || "Telegram sendMessage returned ok=false".to_string(), + ))); + } + + Ok(()) +} + /// Central manager for extension lifecycle operations. /// /// # Initialization Order @@ -421,6 +466,29 @@ impl ExtensionManager { *self.test_telegram_binding_resolver.write().await = Some(resolver); } + #[cfg(test)] + pub(crate) async fn set_test_telegram_pending_verification( + &self, + code: &str, + bot_username: Option<&str>, + ) { + let code = code.to_string(); + let bot_username = bot_username.map(str::to_string); + self.set_test_telegram_binding_resolver(Arc::new(move |_token, existing_owner_id| { + if existing_owner_id.is_some() { + return Err(ExtensionError::Other( + "unexpected existing owner binding".to_string(), + )); + } + Ok(TelegramBindingResult::Pending(VerificationChallenge { + code: code.clone(), + instructions: telegram_verification_instructions(bot_username.as_deref(), &code), + deep_link: telegram_verification_deep_link(bot_username.as_deref(), &code), + })) + })) + .await; + } + /// Enable gateway mode so OAuth flows return auth URLs to the frontend /// instead of calling `open::that()` on the server. /// @@ -597,6 +665,12 @@ impl ExtensionManager { self.current_channel_owner_id(name).await.is_some() } + pub(crate) async fn notification_target_for_channel(&self, name: &str) -> Option { + self.current_channel_owner_id(name) + .await + .map(|owner_id| owner_id.to_string()) + } + async fn get_pending_telegram_verification( &self, name: &str, @@ -1074,7 +1148,7 @@ impl ExtensionManager { active, tools: Vec::new(), needs_setup: auth_state == ToolAuthState::NeedsSetup, - has_auth: false, + has_auth: auth_state != ToolAuthState::NoAuth, installed: true, activation_error, version, @@ -4336,6 +4410,22 @@ impl ExtensionManager { } if let Some(owner_id) = bound_owner_id { + if let Err(err) = send_telegram_text_message( + &client, + &format!("https://api.telegram.org/bot{bot_token}/sendMessage"), + owner_id, + "Verification received. Finishing setup...", + ) + .await + { + tracing::warn!( + channel = name, + owner_id, + error = %err, + "Failed to send Telegram verification acknowledgment" + ); + } + self.clear_pending_telegram_verification(name).await; if offset > 0 { let _ = client @@ -4355,10 +4445,10 @@ impl ExtensionManager { } } - Err(ExtensionError::ValidationFailed(format!( - "Telegram owner verification timed out. Send `/start {}` to your bot, then click Verify owner again.", - challenge.code - ))) + self.clear_pending_telegram_verification(name).await; + Err(ExtensionError::ValidationFailed( + "Telegram owner verification timed out. Request a new code and try again.".to_string(), + )) } async fn notify_telegram_owner_verified( @@ -5120,7 +5210,7 @@ mod tests { use crate::extensions::manager::{ ChannelRuntimeState, FallbackDecision, TelegramBindingData, TelegramBindingResult, TelegramOwnerBindingState, build_wasm_channel_runtime_config_updates, - combine_install_errors, fallback_decision, infer_kind_from_url, + combine_install_errors, fallback_decision, infer_kind_from_url, send_telegram_text_message, telegram_message_matches_verification_code, }; use crate::extensions::{ @@ -5923,7 +6013,7 @@ mod tests { Ok(TelegramBindingResult::Pending(VerificationChallenge { code: "iclaw-7qk2m9".to_string(), instructions: - "Send `/start iclaw-7qk2m9` to @test_hot_bot, then click Verify owner." + "Send `/start iclaw-7qk2m9` to @test_hot_bot in Telegram. IronClaw will finish setup automatically." .to_string(), deep_link: Some("https://t.me/test_hot_bot?start=iclaw-7qk2m9".to_string()), })) @@ -6875,11 +6965,75 @@ mod tests { )?; require( instructions.contains("one-time verification code") - && instructions.contains("/start CODE"), + && instructions.contains("/start CODE") + && instructions.contains("finish setup automatically"), "telegram auth instructions should explain the owner verification step", ) } + #[tokio::test] + async fn test_send_telegram_text_message_posts_expected_payload() -> Result<(), String> { + use axum::{Json, Router, extract::State, routing::post}; + + let payloads = Arc::new(tokio::sync::Mutex::new(Vec::::new())); + + async fn handler( + State(payloads): State>>>, + Json(payload): Json, + ) -> Json { + payloads.lock().await.push(payload); + Json(serde_json::json!({ "ok": true, "result": {} })) + } + + let app = Router::new() + .route("/sendMessage", post(handler)) + .with_state(Arc::clone(&payloads)); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .map_err(|err| format!("bind listener: {err}"))?; + let addr = listener + .local_addr() + .map_err(|err| format!("listener addr: {err}"))?; + let server = tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + + let client = reqwest::Client::new(); + send_telegram_text_message( + &client, + &format!("http://{addr}/sendMessage"), + 424242, + "Verification received. Finishing setup...", + ) + .await + .map_err(|err| format!("send message: {err}"))?; + + let captured = tokio::time::timeout(std::time::Duration::from_secs(1), async { + loop { + let maybe_payload = { payloads.lock().await.first().cloned() }; + if let Some(payload) = maybe_payload { + break payload; + } + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + }) + .await + .map_err(|_| "timed out waiting for sendMessage payload".to_string())?; + + server.abort(); + + require_eq( + captured["chat_id"].clone(), + serde_json::json!(424242), + "chat_id", + )?; + require_eq( + captured["text"].clone(), + serde_json::json!("Verification received. Finishing setup..."), + "text", + ) + } + #[test] fn test_telegram_message_matches_verification_code_variants() -> Result<(), String> { require( diff --git a/src/main.rs b/src/main.rs index ae864bed9b..745cae09b4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -620,7 +620,7 @@ async fn async_main() -> anyhow::Result<()> { // Register message tool for sending messages to connected channels components .tools - .register_message_tools(Arc::clone(&channels)) + .register_message_tools(Arc::clone(&channels), components.extension_manager.clone()) .await; // Wire up channel runtime for hot-activation of WASM channels. diff --git a/src/tools/builtin/message.rs b/src/tools/builtin/message.rs index b150c951e1..1d2ed0594a 100644 --- a/src/tools/builtin/message.rs +++ b/src/tools/builtin/message.rs @@ -10,6 +10,7 @@ use async_trait::async_trait; use crate::bootstrap::ironclaw_base_dir; use crate::channels::{ChannelManager, OutgoingResponse}; use crate::context::JobContext; +use crate::extensions::ExtensionManager; use crate::tools::tool::{ ApprovalRequirement, Tool, ToolError, ToolOutput, ToolRateLimitConfig, require_str, }; @@ -17,6 +18,7 @@ use crate::tools::tool::{ /// Tool for sending messages to channels. pub struct MessageTool { channel_manager: Arc, + extension_manager: Option>, /// Default channel for current conversation (set per-turn). /// Uses std::sync::RwLock because requires_approval() is sync and called from async context. default_channel: Arc>>, @@ -32,12 +34,18 @@ impl MessageTool { Self { channel_manager, + extension_manager: None, default_channel: Arc::new(RwLock::new(None)), default_target: Arc::new(RwLock::new(None)), base_dir, } } + pub fn with_extension_manager(mut self, extension_manager: Arc) -> Self { + self.extension_manager = Some(extension_manager); + self + } + /// Set the base directory for attachment validation. /// This is primarily used for testing or future configuration. pub fn with_base_dir(mut self, dir: PathBuf) -> Self { @@ -111,39 +119,69 @@ impl Tool for MessageTool { let content = require_str(¶ms, "content")?; + let explicit_channel = params + .get("channel") + .and_then(|v| v.as_str()) + .map(|value| value.to_string()); + let default_channel = self + .default_channel + .read() + .unwrap_or_else(|e| e.into_inner()) + .clone(); + let metadata_channel = ctx + .metadata + .get("notify_channel") + .and_then(|v| v.as_str()) + .map(|value| value.to_string()); + // Get channel: use param → conversation default → job metadata → None (broadcast all) - let channel: Option = - if let Some(c) = params.get("channel").and_then(|v| v.as_str()) { - Some(c.to_string()) - } else if let Some(c) = self - .default_channel - .read() - .unwrap_or_else(|e| e.into_inner()) - .clone() - { - Some(c) - } else { - ctx.metadata - .get("notify_channel") - .and_then(|v| v.as_str()) - .map(|c| c.to_string()) - }; + let channel: Option = explicit_channel + .clone() + .or_else(|| default_channel.clone()) + .or_else(|| metadata_channel.clone()); + + let can_use_default_target = match (explicit_channel.as_deref(), default_channel.as_deref()) + { + (None, _) => true, + (Some(explicit), Some(current)) if explicit == current => true, + _ => false, + }; + let can_use_metadata_target = match (channel.as_deref(), metadata_channel.as_deref()) { + (None, _) => true, + (Some(resolved), Some(current)) if resolved == current => true, + _ => false, + }; // Get target: use param → conversation default → job metadata → owner scope // fallback when a specific channel is known. let target = if let Some(t) = params.get("target").and_then(|v| v.as_str()) { Some(t.to_string()) - } else if let Some(t) = self - .default_target - .read() - .unwrap_or_else(|e| e.into_inner()) - .clone() + } else if can_use_default_target + && let Some(t) = self + .default_target + .read() + .unwrap_or_else(|e| e.into_inner()) + .clone() { Some(t) - } else if let Some(t) = ctx.metadata.get("notify_user").and_then(|v| v.as_str()) { + } else if can_use_metadata_target + && let Some(t) = ctx.metadata.get("notify_user").and_then(|v| v.as_str()) + { Some(t.to_string()) } else if channel.is_some() { - Some(ctx.user_id.clone()) + if let Some(channel_name) = channel.as_deref() { + if let Some(extension_manager) = self.extension_manager.as_ref() + && let Some(target) = extension_manager + .notification_target_for_channel(channel_name) + .await + { + Some(target) + } else { + Some(ctx.user_id.clone()) + } + } else { + Some(ctx.user_id.clone()) + } } else { None }; @@ -742,4 +780,33 @@ mod tests { err ); } + + #[tokio::test] + async fn message_tool_does_not_apply_metadata_target_to_different_default_channel() { + let tool = MessageTool::new(Arc::new(ChannelManager::new())); + tool.set_context(Some("telegram".to_string()), None).await; + + let mut ctx = crate::context::JobContext::with_user("owner-scope", "test", "test"); + ctx.metadata = serde_json::json!({ + "notify_channel": "signal", + "notify_user": "metadata-user", + }); + + let result = tool + .execute(serde_json::json!({"content": "hello"}), &ctx) + .await; + + assert!(result.is_err()); + let err = result.unwrap_err().to_string(); + assert!( + !err.contains("metadata-user"), + "metadata target should not be applied to a different default channel: {}", + err + ); + assert!( + err.contains("owner-scope"), + "expected owner-scope fallback target when metadata channel differs: {}", + err + ); + } } diff --git a/src/tools/registry.rs b/src/tools/registry.rs index 754869c8cf..0c457a6d3c 100644 --- a/src/tools/registry.rs +++ b/src/tools/registry.rs @@ -501,9 +501,14 @@ impl ToolRegistry { pub async fn register_message_tools( &self, channel_manager: Arc, + extension_manager: Option>, ) { use crate::tools::builtin::MessageTool; - let tool = Arc::new(MessageTool::new(channel_manager)); + let mut tool = MessageTool::new(channel_manager); + if let Some(extension_manager) = extension_manager { + tool = tool.with_extension_manager(extension_manager); + } + let tool = Arc::new(tool); *self.message_tool.write().await = Some(Arc::clone(&tool)); self.tools .write() diff --git a/tests/e2e/CLAUDE.md b/tests/e2e/CLAUDE.md index c977b6fdf8..0cf5e6dc32 100644 --- a/tests/e2e/CLAUDE.md +++ b/tests/e2e/CLAUDE.md @@ -52,7 +52,7 @@ HEADED=1 pytest scenarios/ | `test_html_injection.py` | XSS vectors injected directly via `page.evaluate("addMessage('assistant', ...)")` are sanitized by `renderMarkdown`; user messages are shown as escaped plain text | | `test_skills.py` | Skills tab UI visibility, ClawHub search (skipped if registry unreachable), install + remove lifecycle | | `test_sse_reconnect.py` | SSE reconnects after programmatic `eventSource.close()` + `connectSSE()`; history is reloaded after reconnect | -| `test_tool_approval.py` | Approval card appears, buttons disable on approve/deny, parameters toggle; all triggered via `page.evaluate("showApproval(...)")` — no real tool call needed | +| `test_tool_approval.py` | Approval card appears, buttons disable on approve/deny, parameters toggle via `page.evaluate("showApproval(...)")`; the waiting-approval regression uses a real HTTP tool call | ## `helpers.py` @@ -164,7 +164,7 @@ async def test_my_ui_feature(page): - **`asyncio_default_fixture_loop_scope = "session"`** — all async fixtures share one event loop. Do not use `asyncio.run()` inside fixtures; use `await` directly. - **The `page` fixture navigates with `/?token=e2e-test-token` and waits for `#auth-screen` to be hidden.** Tests receive a page that is already past the auth screen and has SSE connected. - **`test_skills.py` makes real network calls to ClawHub.** Tests skip (not fail) if the registry is unreachable via `pytest.skip()`. -- **`test_html_injection.py` and `test_tool_approval.py` inject state via `page.evaluate(...)`.** They test the browser-side rendering pipeline and do not depend on the LLM or backend tool execution. +- **`test_html_injection.py` injects state via `page.evaluate(...)`, and most of `test_tool_approval.py` does too.** The waiting-approval regression in `test_tool_approval.py` intentionally uses a real tool approval flow so it can verify backend thread-state handling. - **Browser is Chromium only.** `conftest.py` uses `p.chromium.launch()`; there is no Firefox or WebKit variant. - **Default timeout is 120 seconds** (pyproject.toml). Individual `wait_for` calls inside tests use shorter timeouts (5–20s) for faster failure messages. - **The libsql database is a temp directory** created fresh per `pytest` invocation; tests do not share state across runs. diff --git a/tests/e2e/README.md b/tests/e2e/README.md index 5aac9613fc..17e1378b73 100644 --- a/tests/e2e/README.md +++ b/tests/e2e/README.md @@ -164,5 +164,7 @@ await page.evaluate(""" """) ``` -This is the pattern used in `test_tool_approval.py` and parts of -`test_extensions.py` (auth card, configure modal). +This is the pattern used in most of `test_tool_approval.py` and parts of +`test_extensions.py` (auth card, configure modal). The waiting-approval +regression in `test_tool_approval.py` uses a real tool call instead so it can +exercise backend approval state. diff --git a/tests/e2e/mock_llm.py b/tests/e2e/mock_llm.py index b091fc1739..c27f276265 100644 --- a/tests/e2e/mock_llm.py +++ b/tests/e2e/mock_llm.py @@ -25,6 +25,15 @@ TOOL_CALL_PATTERNS = [ (re.compile(r"echo (.+)", re.IGNORECASE), "echo", lambda m: {"message": m.group(1)}), + ( + re.compile(r"make approval post (?P