diff --git a/frontend/src/components/PermissionDialog.tsx b/frontend/src/components/PermissionDialog.tsx index 3c8afe73..cd9f4eef 100644 --- a/frontend/src/components/PermissionDialog.tsx +++ b/frontend/src/components/PermissionDialog.tsx @@ -135,7 +135,7 @@ export function PermissionDialog({ fontSize: "0.9em", }} > - Always Allow + Don't ask again this session diff --git a/server/src/agent_mode/context.rs b/server/src/agent_mode/context.rs index 6a8a595e..06140d9e 100644 --- a/server/src/agent_mode/context.rs +++ b/server/src/agent_mode/context.rs @@ -82,6 +82,7 @@ pub(super) fn build_agent_app_context( permission_registry, pending_perm_replies: crate::service::permission_router::PendingPermReplies::new(), permission_timeout_secs: 120, + remembered_permissions: crate::service::permission_router::RememberedPermissions::new(), status: agents.status_broadcaster(), chat_dispatcher: Arc::new(crate::chat::dispatcher::ChatDispatcher::new(1_500)), }); diff --git a/server/src/chat/transport/matrix/bot/context.rs b/server/src/chat/transport/matrix/bot/context.rs index 20c7398b..15160cc3 100644 --- a/server/src/chat/transport/matrix/bot/context.rs +++ b/server/src/chat/transport/matrix/bot/context.rs @@ -317,6 +317,7 @@ mod tests { permission_registry: ResponderRegistry::new(), pending_perm_replies: PendingPermReplies::new(), permission_timeout_secs: 120, + remembered_permissions: crate::service::permission_router::RememberedPermissions::new(), status: Arc::new(crate::service::status::StatusBroadcaster::new()), chat_dispatcher: Arc::new(crate::chat::dispatcher::ChatDispatcher::new(1_500)), }) diff --git a/server/src/chat/transport/matrix/bot/permission_listener.rs b/server/src/chat/transport/matrix/bot/permission_listener.rs index 5a609a76..aa06a465 100644 --- a/server/src/chat/transport/matrix/bot/permission_listener.rs +++ b/server/src/chat/transport/matrix/bot/permission_listener.rs @@ -173,6 +173,7 @@ mod tests { permission_registry: crate::service::permission_router::ResponderRegistry::new(), pending_perm_replies: PendingPermReplies::new(), permission_timeout_secs: 120, + remembered_permissions: crate::service::permission_router::RememberedPermissions::new(), status: Arc::new(crate::service::status::StatusBroadcaster::new()), chat_dispatcher: Arc::new(crate::chat::dispatcher::ChatDispatcher::new(1_500)), }) diff --git a/server/src/chat/transport/matrix/health.rs b/server/src/chat/transport/matrix/health.rs index 3a4a2cd9..2ccfce93 100644 --- a/server/src/chat/transport/matrix/health.rs +++ b/server/src/chat/transport/matrix/health.rs @@ -624,6 +624,7 @@ mod tests { permission_registry: registry, pending_perm_replies: PendingPermReplies::new(), permission_timeout_secs: 120, + remembered_permissions: crate::service::permission_router::RememberedPermissions::new(), status: Arc::new(crate::service::status::StatusBroadcaster::new()), chat_dispatcher: Arc::new(crate::chat::dispatcher::ChatDispatcher::new(1_500)), }); @@ -655,6 +656,7 @@ mod tests { permission_registry: ResponderRegistry::new(), pending_perm_replies: PendingPermReplies::new(), permission_timeout_secs: 120, + remembered_permissions: crate::service::permission_router::RememberedPermissions::new(), status: Arc::new(crate::service::status::StatusBroadcaster::new()), chat_dispatcher: Arc::new(crate::chat::dispatcher::ChatDispatcher::new(1_500)), }); diff --git a/server/src/chat/transport/whatsapp/commands/mod.rs b/server/src/chat/transport/whatsapp/commands/mod.rs index 9eca793f..8df30dde 100644 --- a/server/src/chat/transport/whatsapp/commands/mod.rs +++ b/server/src/chat/transport/whatsapp/commands/mod.rs @@ -314,6 +314,7 @@ mod tests { permission_registry: crate::service::permission_router::ResponderRegistry::new(), pending_perm_replies: crate::service::permission_router::PendingPermReplies::new(), permission_timeout_secs: 120, + remembered_permissions: crate::service::permission_router::RememberedPermissions::new(), chat_dispatcher: Arc::new(crate::chat::dispatcher::ChatDispatcher::new(1_500)), }); Arc::new(WhatsAppWebhookContext { diff --git a/server/src/http/context.rs b/server/src/http/context.rs index efad5f31..9b27748a 100644 --- a/server/src/http/context.rs +++ b/server/src/http/context.rs @@ -18,8 +18,12 @@ pub enum PermissionDecision { Deny, /// One-time approval. Approve, - /// Approve and persist the rule to `.claude/settings.json` so Claude Code's - /// built-in permission system handles future checks without prompting. + /// Approve, and remember `(tool, target-pattern)` for the rest of the + /// requesting agent's session (story 1218) — subsequent matching + /// requests auto-approve without forwarding to chat. Scoped in-memory to + /// the session that made the request (see + /// `service::permission_router::RememberedPermissions`); never persisted + /// to disk and never shared with another agent or story. AlwaysAllow, } @@ -125,6 +129,7 @@ impl AppContext { permission_registry, pending_perm_replies: crate::service::permission_router::PendingPermReplies::new(), permission_timeout_secs: 120, + remembered_permissions: crate::service::permission_router::RememberedPermissions::new(), status: agents.status_broadcaster(), chat_dispatcher: Arc::new(crate::chat::dispatcher::ChatDispatcher::new(1_500)), }); diff --git a/server/src/http/mcp/diagnostics/permission.rs b/server/src/http/mcp/diagnostics/permission.rs index 02aadf67..4e46d0e7 100644 --- a/server/src/http/mcp/diagnostics/permission.rs +++ b/server/src/http/mcp/diagnostics/permission.rs @@ -3,7 +3,8 @@ use serde_json::{Value, json}; use crate::http::context::AppContext; -use crate::service::diagnostics::{add_permission_rule, generate_permission_rule}; +use crate::http::mcp::session; +use crate::service::diagnostics::generate_permission_rule; use crate::slog; use crate::slog_warn; @@ -29,6 +30,23 @@ pub(crate) async fn tool_prompt_permission( return Ok(json!({"behavior": "allow", "updatedInput": tool_input}).to_string()); } + let session_id = session::current(); + let rule = generate_permission_rule(&tool_name, &tool_input); + + // Auto-approve without forwarding when this exact (tool, target-pattern) + // was already remembered for this agent's session (story 1218). Logged + // here for auditability since it bypasses the chat approval dialog. + if ctx + .services + .remembered_permissions + .is_remembered(&session_id, &rule) + { + crate::slog!( + "[permission] Auto-approved '{tool_name}' (remembered rule '{rule}' for session '{session_id}')" + ); + return Ok(json!({"behavior": "allow", "updatedInput": tool_input}).to_string()); + } + // Auto-deny immediately if no responder is currently registered to // receive forwarded permission requests. The Matrix bot's // permission_listener task, sled uplinks, and per-message chat transports @@ -83,15 +101,14 @@ pub(crate) async fn tool_prompt_permission( .map_err(|_| "Permission response channel closed unexpectedly".to_string())?; if decision == PermissionDecision::AlwaysAllow { - // Persist the rule so Claude Code won't prompt again for this tool. - if let Some(root) = ctx.state.project_root.lock().unwrap().clone() { - let rule = generate_permission_rule(&tool_name, &tool_input); - if let Err(e) = add_permission_rule(&root, &rule) { - slog_warn!("[permission] Failed to write always-allow rule: {e}"); - } else { - slog!("[permission] Added always-allow rule: {rule}"); - } - } + // Remember for the rest of this agent's session (story 1218) — never + // written to disk, never visible to another session's requests. + ctx.services + .remembered_permissions + .remember(&session_id, &rule); + slog!( + "[permission] Remembered rule '{rule}' for session '{session_id}' — future matches auto-approve without prompting" + ); } if decision == PermissionDecision::Approve || decision == PermissionDecision::AlwaysAllow { @@ -238,91 +255,86 @@ mod tests { assert_eq!(rule, "mcp__huskies__create_story"); } - // ── Settings.json writing tests ────────────────────────────── + // ── Remembered ("don't ask again this session") tests (story 1218) ── - #[test] - fn add_rule_creates_settings_file_when_missing() { + #[tokio::test] + async fn remembered_rule_auto_approves_without_forwarding() { let tmp = tempfile::tempdir().unwrap(); - add_permission_rule(tmp.path(), "Edit").unwrap(); + let ctx = test_ctx(tmp.path()); - let content = fs::read_to_string(tmp.path().join(".claude/settings.json")).unwrap(); - let settings: Value = serde_json::from_str(&content).unwrap(); - let allow = settings["permissions"]["allow"].as_array().unwrap(); - assert!(allow.contains(&json!("Edit"))); - } + // Pre-remember the rule for the default session (no X-Huskies-Session + // header scope installed in this test) — no responder is registered, + // so if the request were forwarded it would auto-deny instead. + ctx.services + .remembered_permissions + .remember(&session::current(), "Bash(git *)"); - #[test] - fn add_rule_does_not_duplicate_existing() { - let tmp = tempfile::tempdir().unwrap(); - add_permission_rule(tmp.path(), "Edit").unwrap(); - add_permission_rule(tmp.path(), "Edit").unwrap(); - - let content = fs::read_to_string(tmp.path().join(".claude/settings.json")).unwrap(); - let settings: Value = serde_json::from_str(&content).unwrap(); - let allow = settings["permissions"]["allow"].as_array().unwrap(); - let count = allow.iter().filter(|v| v == &&json!("Edit")).count(); - assert_eq!(count, 1); - } - - #[test] - fn add_rule_skips_when_wildcard_already_covers() { - let tmp = tempfile::tempdir().unwrap(); - let claude_dir = tmp.path().join(".claude"); - fs::create_dir_all(&claude_dir).unwrap(); - fs::write( - claude_dir.join("settings.json"), - r#"{"permissions":{"allow":["mcp__huskies__*"]}}"#, + let result = tool_prompt_permission( + &json!({"tool_name": "Bash", "input": {"command": "git status"}}), + &ctx, ) - .unwrap(); + .await + .expect("remembered rule must short-circuit before the no-responder auto-deny"); - add_permission_rule(tmp.path(), "mcp__huskies__create_story").unwrap(); - - let content = fs::read_to_string(claude_dir.join("settings.json")).unwrap(); - let settings: Value = serde_json::from_str(&content).unwrap(); - let allow = settings["permissions"]["allow"].as_array().unwrap(); - assert_eq!(allow.len(), 1); - assert_eq!(allow[0], "mcp__huskies__*"); + let parsed: Value = serde_json::from_str(&result).unwrap(); + assert_eq!(parsed["behavior"], "allow"); } - #[test] - fn add_rule_appends_to_existing_rules() { + #[tokio::test] + async fn always_allow_decision_remembers_rule_for_session_not_disk() { let tmp = tempfile::tempdir().unwrap(); - let claude_dir = tmp.path().join(".claude"); - fs::create_dir_all(&claude_dir).unwrap(); - fs::write( - claude_dir.join("settings.json"), - r#"{"permissions":{"allow":["Edit"]}}"#, + let ctx = test_ctx(tmp.path()); + + let (guard, mut rx) = ctx.services.permission_registry.register(); + tokio::spawn(async move { + if let Some(forward) = rx.recv().await { + let _ = forward + .response_tx + .send(crate::http::context::PermissionDecision::AlwaysAllow); + } + drop(guard); + }); + + tool_prompt_permission( + &json!({"tool_name": "Bash", "input": {"command": "git status"}}), + &ctx, ) - .unwrap(); + .await + .expect("always-allow must succeed"); - add_permission_rule(tmp.path(), "Write").unwrap(); - - let content = fs::read_to_string(claude_dir.join("settings.json")).unwrap(); - let settings: Value = serde_json::from_str(&content).unwrap(); - let allow = settings["permissions"]["allow"].as_array().unwrap(); - assert_eq!(allow.len(), 2); - assert!(allow.contains(&json!("Edit"))); - assert!(allow.contains(&json!("Write"))); + assert!( + ctx.services + .remembered_permissions + .is_remembered(&session::current(), "Bash(git *)"), + "AlwaysAllow must remember the rule in-memory for this session" + ); + assert!( + !tmp.path().join(".claude/settings.json").exists(), + "AlwaysAllow must not write to disk (story 1218: session-scoped only)" + ); } - #[test] - fn add_rule_preserves_other_settings_fields() { + #[tokio::test] + async fn remembered_rule_does_not_cross_sessions() { let tmp = tempfile::tempdir().unwrap(); - let claude_dir = tmp.path().join(".claude"); - fs::create_dir_all(&claude_dir).unwrap(); - fs::write( - claude_dir.join("settings.json"), - r#"{"permissions":{"allow":["Edit"]},"enabledMcpjsonServers":["huskies"]}"#, + let ctx = test_ctx(tmp.path()); + ctx.services + .remembered_permissions + .remember("story-a", "Bash(git *)"); + + // Current (default) session never remembered this rule, and no + // responder is registered, so it must fall through to auto-deny. + let result = tool_prompt_permission( + &json!({"tool_name": "Bash", "input": {"command": "git status"}}), + &ctx, ) + .await .unwrap(); - - add_permission_rule(tmp.path(), "Write").unwrap(); - - let content = fs::read_to_string(claude_dir.join("settings.json")).unwrap(); - let settings: Value = serde_json::from_str(&content).unwrap(); - let servers = settings["enabledMcpjsonServers"].as_array().unwrap(); - assert_eq!(servers.len(), 1); - assert_eq!(servers[0], "huskies"); + let parsed: Value = serde_json::from_str(&result).unwrap(); + assert_eq!( + parsed["behavior"], "deny", + "a rule remembered for a different session must not auto-approve this one" + ); } // ── move_story tool tests ───────────────────────────────────── diff --git a/server/src/http/mcp/mod.rs b/server/src/http/mcp/mod.rs index 9f30e93e..d3babc0b 100644 --- a/server/src/http/mcp/mod.rs +++ b/server/src/http/mcp/mod.rs @@ -19,6 +19,10 @@ pub mod merge_tools; pub mod progress; /// MCP tools for QA request, approve, and reject workflows. pub mod qa_tools; +/// Task-local session identifier for the requesting MCP client, read from +/// the `X-Huskies-Session` header so `tool_prompt_permission` can scope +/// remembered approvals per requesting agent. +pub mod session; /// MCP tools for running shell commands and test suites. pub mod shell_tools; /// MCP tools for pipeline status, story todos, and triage dump. @@ -71,6 +75,11 @@ pub async fn mcp_get_handler() -> Response { /// `tools/call`, and `notifications/*`. #[handler] pub async fn mcp_post_handler(req: &Request, body: Body, ctx: Data<&Arc>) -> Response { + let session_id = req + .header("x-huskies-session") + .unwrap_or(session::DEFAULT_SESSION) + .to_string(); + let content_type = req.header("content-type").unwrap_or(""); if !content_type.is_empty() && !content_type.contains("application/json") { return json_response(JsonRpcResponse::error( @@ -125,7 +134,7 @@ pub async fn mcp_post_handler(req: &Request, body: Body, ctx: Data<&Arc { JsonRpcResponse::success(rpc.id, json!({ "tools": tools_list::list_tools() })) } - "tools/call" => handle_tools_call(rpc.id, &rpc.params, &ctx).await, + "tools/call" => { + session::SESSION_ID + .scope(session_id, handle_tools_call(rpc.id, &rpc.params, &ctx)) + .await + } _ => JsonRpcResponse::error(rpc.id, -32601, format!("Unknown method: {}", rpc.method)), }; @@ -152,6 +165,7 @@ async fn sse_tools_call( params: Value, progress_token: Value, ctx: Arc, + session_id: String, ) -> Response { use tokio::sync::mpsc::unbounded_channel; @@ -174,9 +188,13 @@ async fn sse_tools_call( // its final state to the CRDT even on client disconnect). let dispatch_ctx = Arc::clone(&ctx); let dispatch_handle = tokio::spawn(async move { - progress::EMITTER - .scope(emitter, async move { - dispatch::dispatch_tool_call(&tool_name, args, &dispatch_ctx).await + session::SESSION_ID + .scope(session_id, async move { + progress::EMITTER + .scope(emitter, async move { + dispatch::dispatch_tool_call(&tool_name, args, &dispatch_ctx).await + }) + .await }) .await }); diff --git a/server/src/http/mcp/session.rs b/server/src/http/mcp/session.rs new file mode 100644 index 00000000..cb183b70 --- /dev/null +++ b/server/src/http/mcp/session.rs @@ -0,0 +1,51 @@ +//! Task-local session identifier for the requesting MCP client. +//! +//! Threaded ambiently through tool dispatch (same pattern as +//! [`super::progress::EMITTER`]) so a deeply-nested handler — currently only +//! `tool_prompt_permission` — can scope decisions per requesting agent +//! without adding a parameter to `dispatch_tool_call` and every one of its +//! ~40 match arms. +//! +//! The HTTP MCP handler installs the scope before dispatching a `tools/call` +//! request, populated from the `X-Huskies-Session` header that per-story +//! worktrees embed in their `.mcp.json` (see `worktree::write_mcp_json`). +//! Callers with no header (the main interactive chat CLI, API-based runtimes +//! that invoke `dispatch_tool_call` directly) fall back to a fixed +//! `"default"` key — there is only ever one such session per server process, +//! so no cross-story leakage results from sharing that bucket. + +/// Session key used when no `X-Huskies-Session` header was present. +pub const DEFAULT_SESSION: &str = "default"; + +tokio::task_local! { + /// Set by the MCP HTTP handler before dispatching a `tools/call` request. + /// Unset in tests and in non-HTTP dispatch paths, where [`current`] falls + /// back to [`DEFAULT_SESSION`]. + pub static SESSION_ID: String; +} + +/// Return the current request's session id, or [`DEFAULT_SESSION`] if no +/// scope is installed. +pub fn current() -> String { + SESSION_ID + .try_with(Clone::clone) + .unwrap_or_else(|_| DEFAULT_SESSION.to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn current_falls_back_to_default_without_scope() { + assert_eq!(current(), DEFAULT_SESSION); + } + + #[tokio::test] + async fn current_reads_installed_scope() { + let value = SESSION_ID + .scope("1218".to_string(), async { current() }) + .await; + assert_eq!(value, "1218"); + } +} diff --git a/server/src/main.rs b/server/src/main.rs index 88016603..ec8dee4c 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -256,6 +256,7 @@ async fn main() -> Result<(), std::io::Error> { .as_ref() .map(|c| c.permission_timeout_secs) .unwrap_or(120), + remembered_permissions: service::permission_router::RememberedPermissions::new(), status: agents.status_broadcaster(), chat_dispatcher: std::sync::Arc::new(chat::dispatcher::ChatDispatcher::new( bot_cfg diff --git a/server/src/service/bot_command/io.rs b/server/src/service/bot_command/io.rs index baa83f15..a15df154 100644 --- a/server/src/service/bot_command/io.rs +++ b/server/src/service/bot_command/io.rs @@ -141,6 +141,7 @@ pub(super) fn call_sync( permission_registry: ResponderRegistry::new(), pending_perm_replies: PendingPermReplies::new(), permission_timeout_secs: 120, + remembered_permissions: crate::service::permission_router::RememberedPermissions::new(), status: Arc::new(crate::service::status::StatusBroadcaster::new()), chat_dispatcher: Arc::new(crate::chat::dispatcher::ChatDispatcher::new(1_500)), }); diff --git a/server/src/service/diagnostics/io.rs b/server/src/service/diagnostics/io.rs deleted file mode 100644 index 6f1ed555..00000000 --- a/server/src/service/diagnostics/io.rs +++ /dev/null @@ -1,72 +0,0 @@ -//! Diagnostics I/O — the ONLY place in `service::diagnostics/` that may perform side effects. -//! -//! Side effects here include: reading and writing `.claude/settings.json` via `std::fs`. -//! Pure permission-rule logic (pattern derivation, wildcard domination checks) lives in -//! `permission.rs`. - -use serde_json::{Value, json}; -use std::fs; -use std::path::Path; - -/// Add a permission rule to `.claude/settings.json` in the project root. -/// -/// Does nothing if the rule already exists (exact match) or is already covered -/// by a wildcard pattern in the allow list. Creates the file and any missing -/// parent directories if they do not yet exist. -/// -/// # Errors -/// Returns `Err(String)` if the directory cannot be created, the file cannot be -/// read or written, or the JSON cannot be parsed or serialised. -pub fn add_permission_rule(project_root: &Path, rule: &str) -> Result<(), String> { - let claude_dir = project_root.join(".claude"); - fs::create_dir_all(&claude_dir) - .map_err(|e| format!("Failed to create .claude/ directory: {e}"))?; - - let settings_path = claude_dir.join("settings.json"); - let mut settings: Value = if settings_path.exists() { - let content = fs::read_to_string(&settings_path) - .map_err(|e| format!("Failed to read settings.json: {e}"))?; - serde_json::from_str(&content).map_err(|e| format!("Failed to parse settings.json: {e}"))? - } else { - json!({ "permissions": { "allow": [] } }) - }; - - let allow_arr = settings - .pointer_mut("/permissions/allow") - .and_then(|v| v.as_array_mut()); - - let allow = match allow_arr { - Some(arr) => arr, - None => { - settings - .as_object_mut() - .unwrap() - .entry("permissions") - .or_insert(json!({ "allow": [] })); - settings - .pointer_mut("/permissions/allow") - .unwrap() - .as_array_mut() - .unwrap() - } - }; - - let rule_value = Value::String(rule.to_string()); - - // Exact duplicate check. - if allow.contains(&rule_value) { - return Ok(()); - } - - // Wildcard-coverage check: if "mcp__huskies__*" exists, skip more-specific rules. - if super::permission::is_dominated_by_wildcard(rule, allow) { - return Ok(()); - } - - allow.push(rule_value); - - let pretty = - serde_json::to_string_pretty(&settings).map_err(|e| format!("Failed to serialize: {e}"))?; - fs::write(&settings_path, pretty).map_err(|e| format!("Failed to write settings.json: {e}"))?; - Ok(()) -} diff --git a/server/src/service/diagnostics/mod.rs b/server/src/service/diagnostics/mod.rs index bb32919d..75261338 100644 --- a/server/src/service/diagnostics/mod.rs +++ b/server/src/service/diagnostics/mod.rs @@ -3,20 +3,17 @@ //! Extracted from `http/mcp/diagnostics.rs` following the conventions in //! `docs/architecture/service-modules.md`: //! - `mod.rs` (this file) — public API, typed [`Error`], orchestration -//! - `io.rs` — the ONLY place that performs side effects (filesystem reads/writes) //! - `permission.rs` — pure permission-rule generation and wildcard checks +//! +//! Permission rules are remembered in-memory, per requesting-agent session +//! (`service::permission_router::RememberedPermissions`, story 1218) rather +//! than written to disk, so there is no side-effectful I/O submodule here. -/// Side-effectful diagnostics I/O — log reads, CRDT dumps, filesystem writes. -pub mod io; /// Pure permission-rule generation and wildcard matching. pub mod permission; -#[allow(unused_imports)] -pub use io::add_permission_rule; #[allow(unused_imports)] pub use permission::generate_permission_rule; -#[allow(unused_imports)] -pub use permission::is_dominated_by_wildcard; // ── Error type ──────────────────────────────────────────────────────────────── diff --git a/server/src/service/diagnostics/permission.rs b/server/src/service/diagnostics/permission.rs index 45b3aa63..b0253324 100644 --- a/server/src/service/diagnostics/permission.rs +++ b/server/src/service/diagnostics/permission.rs @@ -22,21 +22,6 @@ pub fn generate_permission_rule(tool_name: &str, tool_input: &Value) -> String { } } -/// Return `true` if `rule` is already covered by an existing wildcard in `allow_list`. -/// -/// For example, if `allow_list` contains `"mcp__huskies__*"`, then the more -/// specific rule `"mcp__huskies__create_story"` is already covered. -pub fn is_dominated_by_wildcard(rule: &str, allow_list: &[Value]) -> bool { - allow_list.iter().any(|existing| { - if let Some(pat) = existing.as_str() - && let Some(prefix) = pat.strip_suffix('*') - { - return rule.starts_with(prefix); - } - false - }) -} - // ── Tests ───────────────────────────────────────────────────────────────────── #[cfg(test)] @@ -79,27 +64,4 @@ mod tests { let rule = generate_permission_rule("mcp__huskies__create_story", &json!({"name": "foo"})); assert_eq!(rule, "mcp__huskies__create_story"); } - - #[test] - fn is_dominated_by_exact_wildcard() { - let allow = vec![json!("mcp__huskies__*")]; - assert!(is_dominated_by_wildcard( - "mcp__huskies__create_story", - &allow - )); - } - - #[test] - fn is_not_dominated_by_different_prefix() { - let allow = vec![json!("mcp__other__*")]; - assert!(!is_dominated_by_wildcard( - "mcp__huskies__create_story", - &allow - )); - } - - #[test] - fn is_not_dominated_when_list_is_empty() { - assert!(!is_dominated_by_wildcard("Edit", &[])); - } } diff --git a/server/src/service/gateway/io.rs b/server/src/service/gateway/io.rs index 26e69b5c..68a93c03 100644 --- a/server/src/service/gateway/io.rs +++ b/server/src/service/gateway/io.rs @@ -741,6 +741,7 @@ pub fn spawn_gateway_bot( .as_ref() .map(|c| c.permission_timeout_secs) .unwrap_or(120), + remembered_permissions: crate::service::permission_router::RememberedPermissions::new(), chat_dispatcher: std::sync::Arc::new(crate::chat::dispatcher::ChatDispatcher::new( bot_cfg .as_ref() diff --git a/server/src/service/permission_router.rs b/server/src/service/permission_router.rs index 04d4857b..6017ee77 100644 --- a/server/src/service/permission_router.rs +++ b/server/src/service/permission_router.rs @@ -20,7 +20,7 @@ //! part of the server. use crate::http::context::{PermissionDecision, PermissionForward}; -use std::collections::{HashMap, VecDeque}; +use std::collections::{HashMap, HashSet, VecDeque}; use std::sync::Arc; use std::sync::Mutex as StdMutex; use std::sync::atomic::{AtomicU64, Ordering}; @@ -241,6 +241,57 @@ impl PendingPermReplies { } } +// ── Remembered permissions ─────────────────────────────────────────────── + +/// In-memory "don't ask again for this tool + target-pattern" store, scoped +/// per requesting agent session (story 1218). +/// +/// Keyed by session id (see `http::mcp::session`) — for a per-story coding +/// agent this is the story id, stable across a `--resume` of the same story +/// since the worktree (and its `.mcp.json` header) doesn't change. Rules +/// remembered here are visible only within the session that recorded them, +/// so one agent's "don't ask again" never widens what another agent or +/// story is auto-approved for. +/// +/// Deliberately **not** persisted to disk: this store lives only as long as +/// the server process does. Restarting the server clears every remembered +/// rule (agents will be prompted again once), which is an explicit tradeoff +/// — durability across restarts would require writing into each worktree's +/// own `.claude/settings.json`, which adds a second persistence path for a +/// case (server restarts mid-story) rare enough not to justify the added +/// complexity. +pub struct RememberedPermissions { + inner: StdMutex>>, +} + +impl RememberedPermissions { + /// Create an empty store. + pub fn new() -> Arc { + Arc::new(Self { + inner: StdMutex::new(HashMap::new()), + }) + } + + /// Record that `rule` is approved for `session_id` going forward. + pub fn remember(&self, session_id: &str, rule: &str) { + self.inner + .lock() + .unwrap() + .entry(session_id.to_string()) + .or_default() + .insert(rule.to_string()); + } + + /// `true` if `rule` was previously remembered for `session_id`. + pub fn is_remembered(&self, session_id: &str, rule: &str) -> bool { + self.inner + .lock() + .unwrap() + .get(session_id) + .is_some_and(|rules| rules.contains(rule)) + } +} + // ── Tests ───────────────────────────────────────────────────────────────── #[cfg(test)] @@ -394,4 +445,36 @@ mod tests { let pending = PendingPermReplies::new(); assert!(pending.resolve_oldest("no-such-room").await.is_none()); } + + // ── RememberedPermissions ─────────────────────────────────────── + + #[test] + fn remembered_permissions_starts_empty() { + let store = RememberedPermissions::new(); + assert!(!store.is_remembered("1218", "Bash(git *)")); + } + + #[test] + fn remembered_permissions_recalls_after_remember() { + let store = RememberedPermissions::new(); + store.remember("1218", "Bash(git *)"); + assert!(store.is_remembered("1218", "Bash(git *)")); + } + + #[test] + fn remembered_permissions_scoped_per_session() { + let store = RememberedPermissions::new(); + store.remember("1218", "Bash(git *)"); + assert!( + !store.is_remembered("1216", "Bash(git *)"), + "a rule remembered for one story must not apply to another" + ); + } + + #[test] + fn remembered_permissions_scoped_per_rule() { + let store = RememberedPermissions::new(); + store.remember("1218", "Bash(git *)"); + assert!(!store.is_remembered("1218", "Write")); + } } diff --git a/server/src/services.rs b/server/src/services.rs index b2397b96..d082ae09 100644 --- a/server/src/services.rs +++ b/server/src/services.rs @@ -7,7 +7,9 @@ use crate::agents::AgentPool; use crate::chat::dispatcher::ChatDispatcher; -use crate::service::permission_router::{PendingPermReplies, ResponderRegistry}; +use crate::service::permission_router::{ + PendingPermReplies, RememberedPermissions, ResponderRegistry, +}; use crate::service::status::StatusBroadcaster; use std::collections::HashSet; use std::path::PathBuf; @@ -41,6 +43,11 @@ pub struct Services { /// Seconds to wait for a user to respond to a permission prompt before /// auto-denying (fail-closed). pub permission_timeout_secs: u64, + /// In-memory, per-session "don't ask again" permission rules (story + /// 1218). Checked by `tool_prompt_permission` before forwarding a + /// request to chat; never persisted to disk and never affects a + /// different session's agent. + pub remembered_permissions: Arc, /// Project-scoped status broadcaster. /// /// Consumers (chat transports, Web UI, agent context) call @@ -73,6 +80,7 @@ impl Services { permission_registry: ResponderRegistry::new(), pending_perm_replies: PendingPermReplies::new(), permission_timeout_secs: 120, + remembered_permissions: RememberedPermissions::new(), chat_dispatcher: std::sync::Arc::new(ChatDispatcher::new(1_500)), }) } diff --git a/server/src/sled_uplink.rs b/server/src/sled_uplink.rs index 8f732a25..8746a392 100644 --- a/server/src/sled_uplink.rs +++ b/server/src/sled_uplink.rs @@ -483,6 +483,7 @@ mod tests { permission_registry: ResponderRegistry::new(), pending_perm_replies: PendingPermReplies::new(), permission_timeout_secs: 120, + remembered_permissions: crate::service::permission_router::RememberedPermissions::new(), chat_dispatcher: Arc::new(crate::chat::dispatcher::ChatDispatcher::new(1_500)), }) } diff --git a/server/src/worktree/create.rs b/server/src/worktree/create.rs index 2bab753b..ae2d1ecd 100644 --- a/server/src/worktree/create.rs +++ b/server/src/worktree/create.rs @@ -53,7 +53,7 @@ pub async fn create_worktree( tokio::task::spawn_blocking(move || configure_sparse_checkout(&wt_clone)) .await .map_err(|e| format!("spawn_blocking: {e}"))??; - write_mcp_json(&wt_path, port)?; + write_mcp_json(&wt_path, port, story_id)?; return Ok(WorktreeInfo { path: wt_path, branch, @@ -68,7 +68,7 @@ pub async fn create_worktree( .await .map_err(|e| format!("spawn_blocking: {e}"))??; - write_mcp_json(&wt_path, port)?; + write_mcp_json(&wt_path, port, story_id)?; run_setup_commands(&wt_path, config).await; Ok(WorktreeInfo { diff --git a/server/src/worktree/mod.rs b/server/src/worktree/mod.rs index 611d05d5..d5a5bc12 100644 --- a/server/src/worktree/mod.rs +++ b/server/src/worktree/mod.rs @@ -40,10 +40,23 @@ pub fn worktree_path(project_root: &Path, story_id: &str) -> PathBuf { /// Write a `.mcp.json` file in the given directory pointing to the huskies /// HTTP MCP endpoint at the given port. -pub fn write_mcp_json(dir: &Path, port: u16) -> Result<(), String> { - let content = format!( - "{{\n \"mcpServers\": {{\n \"huskies\": {{\n \"type\": \"http\",\n \"url\": \"http://localhost:{port}/mcp\"\n }}\n }}\n}}\n" - ); +/// +/// Embeds an `X-Huskies-Session` header set to `story_id` so the server can +/// scope remembered permission approvals to this worktree's agent session +/// (see `http::mcp::session`) without affecting other stories' agents. +pub fn write_mcp_json(dir: &Path, port: u16, story_id: &str) -> Result<(), String> { + let value = serde_json::json!({ + "mcpServers": { + "huskies": { + "type": "http", + "url": format!("http://localhost:{port}/mcp"), + "headers": { "X-Huskies-Session": story_id } + } + } + }); + let content = serde_json::to_string_pretty(&value) + .map_err(|e| format!("Serialize .mcp.json: {e}"))? + + "\n"; std::fs::write(dir.join(".mcp.json"), content).map_err(|e| format!("Write .mcp.json: {e}")) } @@ -91,7 +104,7 @@ mod tests { #[test] fn write_mcp_json_uses_given_port() { let tmp = TempDir::new().unwrap(); - write_mcp_json(tmp.path(), 4242).unwrap(); + write_mcp_json(tmp.path(), 4242, "1218").unwrap(); let content = std::fs::read_to_string(tmp.path().join(".mcp.json")).unwrap(); assert!(content.contains("http://localhost:4242/mcp")); } @@ -99,11 +112,23 @@ mod tests { #[test] fn write_mcp_json_default_port() { let tmp = TempDir::new().unwrap(); - write_mcp_json(tmp.path(), 3001).unwrap(); + write_mcp_json(tmp.path(), 3001, "1218").unwrap(); let content = std::fs::read_to_string(tmp.path().join(".mcp.json")).unwrap(); assert!(content.contains("http://localhost:3001/mcp")); } + #[test] + fn write_mcp_json_embeds_session_header_with_story_id() { + let tmp = TempDir::new().unwrap(); + write_mcp_json(tmp.path(), 3001, "42_story_test").unwrap(); + let content = std::fs::read_to_string(tmp.path().join(".mcp.json")).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&content).unwrap(); + assert_eq!( + parsed["mcpServers"]["huskies"]["headers"]["X-Huskies-Session"], + "42_story_test" + ); + } + #[test] fn worktree_path_is_inside_project() { let project_root = Path::new("/home/user/my-project");