From 10a5bea2b1d67c08a33cc4146367d20fac019caf Mon Sep 17 00:00:00 2001 From: Dave Date: Wed, 18 Mar 2026 09:28:51 +0000 Subject: [PATCH] story-kit: merge 275_story_matrix_bot_surfaces_claude_code_permission_prompts_to_chat --- server/src/main.rs | 8 +- server/src/matrix/bot.rs | 208 ++++++++++++++++++++++++++++++++---- server/src/matrix/config.rs | 50 +++++++++ server/src/matrix/mod.rs | 16 ++- 4 files changed, 256 insertions(+), 26 deletions(-) diff --git a/server/src/main.rs b/server/src/main.rs index de026bf..cd2a3c4 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -167,6 +167,10 @@ async fn main() -> Result<(), std::io::Error> { // Clone watcher_tx for the Matrix bot before it is moved into AppContext. let watcher_tx_for_bot = watcher_tx.clone(); + // Wrap perm_rx in Arc so it can be shared with both the WebSocket + // handler (via AppContext) and the Matrix bot. + let perm_rx = Arc::new(tokio::sync::Mutex::new(perm_rx)); + let perm_rx_for_bot = Arc::clone(&perm_rx); // Capture project root, agents Arc, and reconciliation sender before ctx // is consumed by build_routes. @@ -183,7 +187,7 @@ async fn main() -> Result<(), std::io::Error> { watcher_tx, reconciliation_tx, perm_tx, - perm_rx: Arc::new(tokio::sync::Mutex::new(perm_rx)), + perm_rx, }; let app = build_routes(ctx); @@ -192,7 +196,7 @@ async fn main() -> Result<(), std::io::Error> { // Optional Matrix bot: connect to the homeserver and start listening for // messages if `.story_kit/bot.toml` is present and enabled. if let Some(ref root) = startup_root { - matrix::spawn_bot(root, watcher_tx_for_bot); + matrix::spawn_bot(root, watcher_tx_for_bot, perm_rx_for_bot); } // On startup: diff --git a/server/src/matrix/bot.rs b/server/src/matrix/bot.rs index 940e2a7..36d9b6a 100644 --- a/server/src/matrix/bot.rs +++ b/server/src/matrix/bot.rs @@ -1,3 +1,4 @@ +use crate::http::context::{PermissionDecision, PermissionForward}; use crate::llm::providers::claude_code::{ClaudeCodeProvider, ClaudeCodeResult}; use crate::slog; use matrix_sdk::{ @@ -19,8 +20,9 @@ use std::collections::{HashMap, HashSet}; use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; +use std::time::Duration; use tokio::sync::Mutex as TokioMutex; -use tokio::sync::watch; +use tokio::sync::{mpsc, oneshot, watch}; use futures::StreamExt; use matrix_sdk::encryption::verification::{ @@ -149,6 +151,17 @@ pub struct BotContext { /// bot so it can continue a conversation thread without requiring an /// explicit `@mention` on every follow-up. pub bot_sent_event_ids: Arc>>, + /// Receiver for permission requests from the MCP `prompt_permission` tool. + /// During an active chat the bot locks this to poll for incoming requests. + pub perm_rx: Arc>>, + /// Per-room pending permission reply senders. When a permission prompt is + /// posted to a room the oneshot sender is stored here; when the user + /// replies (yes/no) the event handler resolves it. + pub pending_perm_replies: + Arc>>>, + /// How long to wait for a user to respond to a permission prompt before + /// denying (fail-closed). + pub permission_timeout_secs: u64, } // --------------------------------------------------------------------------- @@ -162,6 +175,7 @@ pub async fn run_bot( config: BotConfig, project_root: PathBuf, watcher_rx: tokio::sync::broadcast::Receiver, + perm_rx: Arc>>, ) -> Result<(), String> { let store_path = project_root.join(".story_kit").join("matrix_store"); let client = Client::builder() @@ -307,6 +321,9 @@ pub async fn run_bot( history: Arc::new(TokioMutex::new(persisted)), history_size: config.history_size, bot_sent_event_ids: Arc::new(TokioMutex::new(HashSet::new())), + perm_rx, + pending_perm_replies: Arc::new(TokioMutex::new(HashMap::new())), + permission_timeout_secs: config.permission_timeout_secs, }; slog!("[matrix-bot] Cryptographic identity verification is always ON — commands from unencrypted rooms or unverified devices are rejected"); @@ -340,6 +357,24 @@ pub async fn run_bot( // Address-filtering helpers // --------------------------------------------------------------------------- +/// Returns `true` if the message body is an affirmative permission response. +/// +/// Recognised affirmative tokens (case-insensitive): `yes`, `y`, `approve`, +/// `allow`, `ok`. Anything else — including ambiguous text — is treated as +/// denial (fail-closed). +fn is_permission_approval(body: &str) -> bool { + // Strip a leading @mention (e.g. "@timmy yes") so the bot name doesn't + // interfere with the check. + let trimmed = body + .trim() + .trim_start_matches('@') + .split_whitespace() + .last() + .unwrap_or("") + .to_ascii_lowercase(); + matches!(trimmed.as_str(), "yes" | "y" | "approve" | "allow" | "ok") +} + /// Returns `true` if the message mentions the bot. /// /// Checks both the plain-text `body` and an optional `formatted_body` (HTML). @@ -637,6 +672,33 @@ async fn on_room_message( } } + // If there is a pending permission prompt for this room, interpret the + // message as a yes/no response instead of starting a new chat. + { + let mut pending = ctx.pending_perm_replies.lock().await; + if let Some(tx) = pending.remove(&incoming_room_id) { + let decision = if is_permission_approval(&body) { + PermissionDecision::Approve + } else { + PermissionDecision::Deny + }; + let _ = tx.send(decision); + let confirmation = if decision == PermissionDecision::Approve { + "Permission approved." + } else { + "Permission denied." + }; + let html = markdown_to_html(confirmation); + if let Ok(resp) = room + .send(RoomMessageEventContent::text_html(confirmation, html)) + .await + { + ctx.bot_sent_event_ids.lock().await.insert(resp.event_id); + } + return; + } + } + let sender = ev.sender.to_string(); let user_message = body; slog!("[matrix-bot] Message from {sender}: {user_message}"); @@ -692,6 +754,7 @@ async fn handle_message( // block the LLM stream while waiting for Matrix send round-trips. let post_room = room.clone(); let sent_ids = Arc::clone(&ctx.bot_sent_event_ids); + let sent_ids_for_post = Arc::clone(&sent_ids); let post_task = tokio::spawn(async move { while let Some(chunk) = msg_rx.recv().await { let html = markdown_to_html(&chunk); @@ -699,7 +762,7 @@ async fn handle_message( .send(RoomMessageEventContent::text_html(chunk, html)) .await { - sent_ids.lock().await.insert(response.event_id); + sent_ids_for_post.lock().await.insert(response.event_id); } } }); @@ -710,26 +773,84 @@ async fn handle_message( let sent_any_chunk = Arc::new(AtomicBool::new(false)); let sent_any_chunk_for_callback = Arc::clone(&sent_any_chunk); - let result = provider - .chat_stream( - &prompt, - &ctx.project_root.to_string_lossy(), - resume_session_id.as_deref(), - &mut cancel_rx, - move |token| { - let mut buf = buffer_for_callback.lock().unwrap(); - buf.push_str(token); - // Flush complete paragraphs as they arrive. - let paragraphs = drain_complete_paragraphs(&mut buf); - for chunk in paragraphs { - sent_any_chunk_for_callback.store(true, Ordering::Relaxed); - let _ = msg_tx_for_callback.send(chunk); + let project_root_str = ctx.project_root.to_string_lossy().to_string(); + let chat_fut = provider.chat_stream( + &prompt, + &project_root_str, + resume_session_id.as_deref(), + &mut cancel_rx, + move |token| { + let mut buf = buffer_for_callback.lock().unwrap(); + buf.push_str(token); + // Flush complete paragraphs as they arrive. + let paragraphs = drain_complete_paragraphs(&mut buf); + for chunk in paragraphs { + sent_any_chunk_for_callback.store(true, Ordering::Relaxed); + let _ = msg_tx_for_callback.send(chunk); + } + }, + |_thinking| {}, // Discard thinking tokens + |_activity| {}, // Discard activity signals + ); + tokio::pin!(chat_fut); + + // Lock the permission receiver for the duration of this chat session. + // Permission requests from the MCP `prompt_permission` tool arrive here. + let mut perm_rx_guard = ctx.perm_rx.lock().await; + + let result = loop { + tokio::select! { + r = &mut chat_fut => break r, + + Some(perm_fwd) = perm_rx_guard.recv() => { + // Post the permission prompt to the Matrix room. + let prompt_msg = format!( + "**Permission Request**\n\n\ + Tool: `{}`\n```json\n{}\n```\n\n\ + Reply **yes** to approve or **no** to deny.", + perm_fwd.tool_name, + serde_json::to_string_pretty(&perm_fwd.tool_input) + .unwrap_or_else(|_| perm_fwd.tool_input.to_string()), + ); + let html = markdown_to_html(&prompt_msg); + if let Ok(resp) = room + .send(RoomMessageEventContent::text_html(&prompt_msg, html)) + .await + { + sent_ids.lock().await.insert(resp.event_id); } - }, - |_thinking| {}, // Discard thinking tokens - |_activity| {}, // Discard activity signals - ) - .await; + + // Store the MCP oneshot sender so the event handler can + // resolve it when the user replies yes/no. + ctx.pending_perm_replies + .lock() + .await + .insert(room_id.clone(), perm_fwd.response_tx); + + // Spawn a timeout task: auto-deny if the user does not respond. + let pending = Arc::clone(&ctx.pending_perm_replies); + let timeout_room_id = room_id.clone(); + let timeout_room = room.clone(); + let timeout_sent_ids = Arc::clone(&ctx.bot_sent_event_ids); + let timeout_secs = ctx.permission_timeout_secs; + tokio::spawn(async move { + tokio::time::sleep(Duration::from_secs(timeout_secs)).await; + if let Some(tx) = pending.lock().await.remove(&timeout_room_id) { + let _ = tx.send(PermissionDecision::Deny); + let msg = "Permission request timed out — denied (fail-closed)."; + let html = markdown_to_html(msg); + if let Ok(resp) = timeout_room + .send(RoomMessageEventContent::text_html(msg, html)) + .await + { + timeout_sent_ids.lock().await.insert(resp.event_id); + } + } + }); + } + } + }; + drop(perm_rx_guard); // Flush any remaining text that didn't end with a paragraph boundary. let remaining = buffer.lock().unwrap().trim().to_string(); @@ -1071,6 +1192,7 @@ mod tests { fn bot_context_has_no_require_verified_devices_field() { // Verification is always on — BotContext no longer has a toggle field. // This test verifies the struct can be constructed and cloned without it. + let (_perm_tx, perm_rx) = mpsc::unbounded_channel(); let ctx = BotContext { bot_user_id: make_user_id("@bot:example.com"), target_room_ids: vec![], @@ -1079,6 +1201,9 @@ mod tests { history: Arc::new(TokioMutex::new(HashMap::new())), history_size: 20, bot_sent_event_ids: Arc::new(TokioMutex::new(HashSet::new())), + perm_rx: Arc::new(TokioMutex::new(perm_rx)), + pending_perm_replies: Arc::new(TokioMutex::new(HashMap::new())), + permission_timeout_secs: 120, }; // Clone must work (required by Matrix SDK event handler injection). let _cloned = ctx.clone(); @@ -1482,4 +1607,45 @@ mod tests { "user with no cross-signing setup should be rejected" ); } + + // -- is_permission_approval ----------------------------------------------- + + #[test] + fn is_permission_approval_accepts_yes_variants() { + assert!(is_permission_approval("yes")); + assert!(is_permission_approval("Yes")); + assert!(is_permission_approval("YES")); + assert!(is_permission_approval("y")); + assert!(is_permission_approval("Y")); + assert!(is_permission_approval("approve")); + assert!(is_permission_approval("allow")); + assert!(is_permission_approval("ok")); + assert!(is_permission_approval("OK")); + } + + #[test] + fn is_permission_approval_denies_no_and_other() { + assert!(!is_permission_approval("no")); + assert!(!is_permission_approval("No")); + assert!(!is_permission_approval("n")); + assert!(!is_permission_approval("deny")); + assert!(!is_permission_approval("reject")); + assert!(!is_permission_approval("maybe")); + assert!(!is_permission_approval("")); + assert!(!is_permission_approval("yes please do it")); + } + + #[test] + fn is_permission_approval_strips_at_mention_prefix() { + // "@botname yes" should still be treated as approval — the mention + // prefix is stripped before checking the token. + assert!(is_permission_approval("@timmy yes")); + assert!(!is_permission_approval("@timmy no")); + } + + #[test] + fn is_permission_approval_handles_whitespace() { + assert!(is_permission_approval(" yes ")); + assert!(is_permission_approval("\tyes\n")); + } } diff --git a/server/src/matrix/config.rs b/server/src/matrix/config.rs index 1fcb5aa..e58d607 100644 --- a/server/src/matrix/config.rs +++ b/server/src/matrix/config.rs @@ -5,6 +5,10 @@ fn default_history_size() -> usize { 20 } +fn default_permission_timeout_secs() -> u64 { + 120 +} + /// Configuration for the Matrix bot, read from `.story_kit/bot.toml`. #[derive(Deserialize, Clone, Debug)] pub struct BotConfig { @@ -35,6 +39,11 @@ pub struct BotConfig { /// dropped. Defaults to 20. #[serde(default = "default_history_size")] pub history_size: usize, + /// Timeout in seconds for permission prompts surfaced to the Matrix room. + /// If the user does not respond within this window the permission is denied + /// (fail-closed). Defaults to 120 seconds. + #[serde(default = "default_permission_timeout_secs")] + pub permission_timeout_secs: u64, /// Previously used to select an Anthropic model. Now ignored — the bot /// uses Claude Code which manages its own model selection. Kept for /// backwards compatibility so existing bot.toml files still parse. @@ -256,6 +265,47 @@ history_size = 50 assert_eq!(config.history_size, 50); } + #[test] + fn load_uses_default_permission_timeout() { + let tmp = tempfile::tempdir().unwrap(); + let sk = tmp.path().join(".story_kit"); + fs::create_dir_all(&sk).unwrap(); + fs::write( + sk.join("bot.toml"), + r#" +homeserver = "https://matrix.example.com" +username = "@bot:example.com" +password = "secret" +room_ids = ["!abc:example.com"] +enabled = true +"#, + ) + .unwrap(); + let config = BotConfig::load(tmp.path()).unwrap(); + assert_eq!(config.permission_timeout_secs, 120); + } + + #[test] + fn load_respects_custom_permission_timeout() { + let tmp = tempfile::tempdir().unwrap(); + let sk = tmp.path().join(".story_kit"); + fs::create_dir_all(&sk).unwrap(); + fs::write( + sk.join("bot.toml"), + r#" +homeserver = "https://matrix.example.com" +username = "@bot:example.com" +password = "secret" +room_ids = ["!abc:example.com"] +enabled = true +permission_timeout_secs = 60 +"#, + ) + .unwrap(); + let config = BotConfig::load(tmp.path()).unwrap(); + assert_eq!(config.permission_timeout_secs, 60); + } + #[test] fn load_ignores_legacy_require_verified_devices_key() { // Old bot.toml files that still have `require_verified_devices = true` diff --git a/server/src/matrix/mod.rs b/server/src/matrix/mod.rs index 3269f61..97920e6 100644 --- a/server/src/matrix/mod.rs +++ b/server/src/matrix/mod.rs @@ -21,9 +21,11 @@ pub mod notifications; pub use config::BotConfig; +use crate::http::context::PermissionForward; use crate::io::watcher::WatcherEvent; use std::path::Path; -use tokio::sync::broadcast; +use std::sync::Arc; +use tokio::sync::{Mutex as TokioMutex, broadcast, mpsc}; /// Attempt to start the Matrix bot. /// @@ -35,8 +37,16 @@ use tokio::sync::broadcast; /// posts stage-transition messages to all configured rooms whenever a work /// item moves between pipeline stages. /// +/// `perm_rx` is the permission-request receiver shared with the MCP +/// `prompt_permission` tool. The bot locks it during active chat sessions +/// to surface permission prompts to the Matrix room and relay user decisions. +/// /// Must be called from within a Tokio runtime context (e.g., from `main`). -pub fn spawn_bot(project_root: &Path, watcher_tx: broadcast::Sender) { +pub fn spawn_bot( + project_root: &Path, + watcher_tx: broadcast::Sender, + perm_rx: Arc>>, +) { let config = match BotConfig::load(project_root) { Some(c) => c, None => { @@ -54,7 +64,7 @@ pub fn spawn_bot(project_root: &Path, watcher_tx: broadcast::Sender