huskies: merge 1218 story Remembered/sticky permission approvals so constrained agents stop re-prompting for the same action class
This commit is contained in:
@@ -135,7 +135,7 @@ export function PermissionDialog({
|
||||
fontSize: "0.9em",
|
||||
}}
|
||||
>
|
||||
Always Allow
|
||||
Don't ask again this session
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -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)),
|
||||
});
|
||||
|
||||
@@ -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)),
|
||||
})
|
||||
|
||||
@@ -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)),
|
||||
})
|
||||
|
||||
@@ -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)),
|
||||
});
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)),
|
||||
});
|
||||
|
||||
@@ -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 ─────────────────────────────────────
|
||||
|
||||
@@ -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<AppContext>>) -> 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<AppConte
|
||||
.and_then(|m| m.get("progressToken"))
|
||||
.cloned();
|
||||
if let (true, Some(token)) = (accepts_sse, progress_token) {
|
||||
return sse_tools_call(rpc.id, rpc.params, token, Arc::clone(&ctx)).await;
|
||||
return sse_tools_call(rpc.id, rpc.params, token, Arc::clone(&ctx), session_id).await;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -134,7 +143,11 @@ pub async fn mcp_post_handler(req: &Request, body: Body, ctx: Data<&Arc<AppConte
|
||||
"tools/list" => {
|
||||
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<AppContext>,
|
||||
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
|
||||
});
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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)),
|
||||
});
|
||||
|
||||
@@ -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(())
|
||||
}
|
||||
@@ -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 ────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
@@ -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", &[]));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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<HashMap<String, HashSet<String>>>,
|
||||
}
|
||||
|
||||
impl RememberedPermissions {
|
||||
/// Create an empty store.
|
||||
pub fn new() -> Arc<Self> {
|
||||
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"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<RememberedPermissions>,
|
||||
/// 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)),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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)),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user