From 0e79ffaaa58f43ee305ea2745210220eb715ce12 Mon Sep 17 00:00:00 2001 From: BFlat Date: Tue, 3 Nov 2026 20:40:55 +0700 Subject: [PATCH] Remember "Always for this project" on a tool's consent card (#654) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Always for this project" on a consent card is remembered in the project, and a tool allowed that way isn't asked about again (fixes #563). Replays #575. Co-authored-by: Fábio Soares --- Cargo.lock | 1 + crates/mcp/Cargo.toml | 3 +++ crates/mcp/src/lib.rs | 24 +++++++++++++++++++-- crates/mcp/src/tests.rs | 46 +++++++++++++++++++++++++++++++++++++++++ docs/pitfalls.md | 6 ++++++ 5 files changed, 78 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6669c1d0..af6b0d62 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1320,6 +1320,7 @@ dependencies = [ "log", "serde", "serde_json", + "tempfile", ] [[package]] diff --git a/crates/mcp/Cargo.toml b/crates/mcp/Cargo.toml index 507416fc..1b3e3506 100644 --- a/crates/mcp/Cargo.toml +++ b/crates/mcp/Cargo.toml @@ -16,3 +16,6 @@ craft-anything-platform = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } log = { workspace = true } + +[dev-dependencies] +tempfile = "3" diff --git a/crates/mcp/src/lib.rs b/crates/mcp/src/lib.rs index 6d935009..c6143127 100644 --- a/crates/mcp/src/lib.rs +++ b/crates/mcp/src/lib.rs @@ -486,14 +486,34 @@ impl Server { /// Ask the person (through the session's inbox) to allow one consent-gated call. `None` when /// there is no inbox. + /// + /// "Always for this project" is remembered in the project (the inbox's folder is the project's, + /// `allowed_actions` as `tool:`), and a tool the person already always allowed isn't asked + /// about again (#563: it was asked every time, whatever was chosen). fn ask_consent(&self, name: &str, why: &str, token: &CancelToken) -> Option { let dir = self.engine.session().ext.get::()?; let inbox = craft_anything_engine::inbox::Inbox::new(dir.0.clone()); + let kind = format!("tool:{name}"); + let mut project = dir.0.parent().and_then(|root| craft_anything_engine::workspace::Workspace::open(root).ok()); + if project.as_ref().is_some_and(|p| p.manifest.allowed_actions.contains(&kind)) { + return Some(true); + } let title = self.engine.registry().get(name).map(|s| s.title).unwrap_or(name); let id = - inbox.post(craft_anything_engine::inbox::RequestKind::Approval, &format!("Allow: {title}"), why, Vec::new(), false, Some(name.to_string())).ok()?; + inbox.post(craft_anything_engine::inbox::RequestKind::Approval, &format!("Allow: {title}"), why, Vec::new(), false, Some(kind.clone())).ok()?; let answer = inbox.wait(&id, Duration::from_secs(30 * 60), &|| token.is_cancelled()); - Some(answer.and_then(|a| a.allow).unwrap_or(false)) + let allowed = answer.as_ref().and_then(|a| a.allow).unwrap_or(false); + if allowed + && answer.is_some_and(|a| a.always) + && let Some(p) = project.as_mut() + { + let _ = p.set_person(|m| { + if !m.allowed_actions.contains(&kind) { + m.allowed_actions.push(kind); + } + }); + } + Some(allowed) } fn success_body(&self, name: &str, out: &ToolOutput) -> Value { diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index 118e70ce..9f99c787 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -315,3 +315,49 @@ fn claude_code_connections_get_a_budget_that_fits_its_limit() { set.handle_request("initialize", &hello("claude-code")).unwrap(); assert_eq!(set.budget(), 500_000, "an explicit budget wins"); } + +/// #563: "Always for this project" on a consent card was dropped, so the same tool was asked about +/// every time. It is remembered in the project, and a tool it was said for isn't asked again. +#[test] +fn always_for_this_project_on_a_consent_card_is_remembered() { + use craft_anything_engine::inbox::{Answer, Inbox, InboxDir}; + use craft_anything_engine::workspace::{TargetRef, Workspace}; + let dir = tempfile::tempdir().unwrap(); + let ws = Workspace::create(dir.path(), "t", "r", TargetRef::None).unwrap(); + let engine = Arc::new(Engine::new(craft_anything_engine::testing::session())); + engine.session().ext.insert(Arc::new(InboxDir(ws.inbox_dir()))); + let server = Server::new(engine, ServerOptions::default(), Box::new(Buf::default())); + let inbox = Inbox::new(ws.inbox_dir()); + + // Allowed once: this call goes through, and nothing is remembered. + let root = ws.inbox_dir(); + let answerer = |always: bool| { + let inbox = Inbox::new(root.clone()); + std::thread::spawn(move || { + for _ in 0..400 { + if let Some(r) = inbox.pending().first() { + inbox.answer(&r.id, &Answer { allow: Some(true), always, ..Default::default() }).unwrap(); + return; + } + std::thread::sleep(Duration::from_millis(10)); + } + }) + }; + let once = answerer(false); + assert_eq!(server.ask_consent("web_session_run", "why", &CancelToken::default()), Some(true)); + once.join().unwrap(); + assert!(Workspace::open(&ws.root).unwrap().manifest.allowed_actions.is_empty()); + + // Always: remembered as `tool:`, and the next call isn't asked (the token is already + // cancelled, so asking would be refused, not waited on). + let always = answerer(true); + assert_eq!(server.ask_consent("web_session_run", "why", &CancelToken::default()), Some(true)); + always.join().unwrap(); + assert_eq!(Workspace::open(&ws.root).unwrap().manifest.allowed_actions, vec!["tool:web_session_run".to_string()]); + let cancelled = CancelToken::default(); + cancelled.cancel(); + assert_eq!(server.ask_consent("web_session_run", "why", &cancelled), Some(true), "not asked again"); + assert!(inbox.pending().is_empty()); + // Another tool is still asked about. + assert_eq!(server.ask_consent("browser_other", "why", &cancelled), Some(false)); +} diff --git a/docs/pitfalls.md b/docs/pitfalls.md index 0ae6c5e7..ff345998 100644 --- a/docs/pitfalls.md +++ b/docs/pitfalls.md @@ -188,6 +188,12 @@ every one into "isn't installed or isn't on your PATH": #95, #130 and #152 reins had. When a spawn says not found, look the program up yourself; if it's there, name the file and what it needs (`spawn_error_for`). Not run on Windows here. +**An answer with two parts, read as one.** An approval card has "Allow once" and "Always for this +project": the answer carries `allow` and `always`. The consent card our MCP server shows for a tool +that runs code read only `allow`, so "Always" was dropped and the same tool (a scripted web session) +was asked about every time (#563). When a card offers a choice, whoever reads the answer handles +every field of it, and a test answers it each way and asks again. + **A Mac app that couldn't find Claude Code, and offered a GitHub key instead.** An app opened from the Dock gets the system's short `PATH`, not the shell's, so `claude` in `~/.local/bin` wasn't found; "Built by" then fell back to "Claude Code on github.com-work", an `~/.ssh/config` alias