story-219: add Always Allow button to web UI permission dialog
Cherry-pick from feature branch — code was never squash-merged despite story being accepted (bug 226). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -7,6 +7,18 @@ use poem::http::StatusCode;
|
||||
use std::sync::Arc;
|
||||
use tokio::sync::{broadcast, mpsc, oneshot};
|
||||
|
||||
/// The user's decision when responding to a permission dialog.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum PermissionDecision {
|
||||
/// One-time denial.
|
||||
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.
|
||||
AlwaysAllow,
|
||||
}
|
||||
|
||||
/// A permission request forwarded from the MCP `prompt_permission` tool to the
|
||||
/// active WebSocket session. The MCP handler blocks on `response_tx` until the
|
||||
/// user approves or denies via the frontend dialog.
|
||||
@@ -14,7 +26,7 @@ pub struct PermissionForward {
|
||||
pub request_id: String,
|
||||
pub tool_name: String,
|
||||
pub tool_input: serde_json::Value,
|
||||
pub response_tx: oneshot::Sender<bool>,
|
||||
pub response_tx: oneshot::Sender<PermissionDecision>,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
@@ -82,4 +94,13 @@ mod tests {
|
||||
let err = bad_request(String::new());
|
||||
assert_eq!(err.status(), StatusCode::BAD_REQUEST);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permission_decision_equality() {
|
||||
assert_eq!(PermissionDecision::Deny, PermissionDecision::Deny);
|
||||
assert_eq!(PermissionDecision::Approve, PermissionDecision::Approve);
|
||||
assert_eq!(PermissionDecision::AlwaysAllow, PermissionDecision::AlwaysAllow);
|
||||
assert_ne!(PermissionDecision::Deny, PermissionDecision::Approve);
|
||||
assert_ne!(PermissionDecision::Approve, PermissionDecision::AlwaysAllow);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1793,6 +1793,93 @@ fn tool_get_server_logs(args: &Value) -> Result<String, String> {
|
||||
Ok(all_lines[start..].join("\n"))
|
||||
}
|
||||
|
||||
/// Generate a Claude Code permission rule string for the given tool name and input.
|
||||
///
|
||||
/// - `Edit` / `Write` / `Read` / `Grep` / `Glob` etc. → just the tool name
|
||||
/// - `Bash` → `Bash(first_word *)` derived from the `command` field in `tool_input`
|
||||
/// - `mcp__*` → the full tool name (e.g. `mcp__story-kit__create_story`)
|
||||
fn generate_permission_rule(tool_name: &str, tool_input: &Value) -> String {
|
||||
if tool_name == "Bash" {
|
||||
// Extract command from tool_input.command and use first word as prefix
|
||||
let command_str = tool_input
|
||||
.get("command")
|
||||
.and_then(|v| v.as_str())
|
||||
.unwrap_or("");
|
||||
let first_word = command_str.split_whitespace().next().unwrap_or("unknown");
|
||||
format!("Bash({first_word} *)")
|
||||
} else {
|
||||
// For Edit, Write, Read, Glob, Grep, MCP tools, etc. — use the tool name directly
|
||||
tool_name.to_string()
|
||||
}
|
||||
}
|
||||
|
||||
/// Add a permission rule to `.claude/settings.json` in the project root.
|
||||
/// Does nothing if the rule already exists. Creates the file if missing.
|
||||
fn add_permission_rule(project_root: &std::path::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 => {
|
||||
// Ensure the structure exists
|
||||
settings
|
||||
.as_object_mut()
|
||||
.unwrap()
|
||||
.entry("permissions")
|
||||
.or_insert(json!({ "allow": [] }));
|
||||
settings
|
||||
.pointer_mut("/permissions/allow")
|
||||
.unwrap()
|
||||
.as_array_mut()
|
||||
.unwrap()
|
||||
}
|
||||
};
|
||||
|
||||
// Check for duplicates — exact string match
|
||||
let rule_value = Value::String(rule.to_string());
|
||||
if allow.contains(&rule_value) {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// Also check for wildcard coverage: if "mcp__story-kit__*" exists, don't add
|
||||
// a more specific "mcp__story-kit__create_story".
|
||||
let dominated = allow.iter().any(|existing| {
|
||||
if let Some(pat) = existing.as_str()
|
||||
&& let Some(prefix) = pat.strip_suffix('*')
|
||||
{
|
||||
return rule.starts_with(prefix);
|
||||
}
|
||||
false
|
||||
});
|
||||
if dominated {
|
||||
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(())
|
||||
}
|
||||
|
||||
/// MCP tool called by Claude Code via `--permission-prompt-tool`.
|
||||
///
|
||||
/// Forwards the permission request through the shared channel to the active
|
||||
@@ -1821,7 +1908,9 @@ async fn tool_prompt_permission(args: &Value, ctx: &AppContext) -> Result<String
|
||||
})
|
||||
.map_err(|_| "No active WebSocket session to receive permission request".to_string())?;
|
||||
|
||||
let approved = tokio::time::timeout(
|
||||
use crate::http::context::PermissionDecision;
|
||||
|
||||
let decision = tokio::time::timeout(
|
||||
std::time::Duration::from_secs(300),
|
||||
response_rx,
|
||||
)
|
||||
@@ -1833,7 +1922,19 @@ async fn tool_prompt_permission(args: &Value, ctx: &AppContext) -> Result<String
|
||||
})?
|
||||
.map_err(|_| "Permission response channel closed unexpectedly".to_string())?;
|
||||
|
||||
if approved {
|
||||
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}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if decision == PermissionDecision::Approve || decision == PermissionDecision::AlwaysAllow {
|
||||
// Claude Code SDK expects:
|
||||
// Allow: { behavior: "allow", updatedInput: <record> }
|
||||
// Deny: { behavior: "deny", message: string }
|
||||
@@ -3382,7 +3483,7 @@ stage = "coder"
|
||||
tokio::spawn(async move {
|
||||
let mut rx = perm_rx.lock().await;
|
||||
if let Some(forward) = rx.recv().await {
|
||||
let _ = forward.response_tx.send(true);
|
||||
let _ = forward.response_tx.send(crate::http::context::PermissionDecision::Approve);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -3414,7 +3515,7 @@ stage = "coder"
|
||||
tokio::spawn(async move {
|
||||
let mut rx = perm_rx.lock().await;
|
||||
if let Some(forward) = rx.recv().await {
|
||||
let _ = forward.response_tx.send(false);
|
||||
let _ = forward.response_tx.send(crate::http::context::PermissionDecision::Deny);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -3508,4 +3609,134 @@ stage = "coder"
|
||||
let pct = read_coverage_percent_from_json(tmp.path());
|
||||
assert!(pct.is_none());
|
||||
}
|
||||
|
||||
// ── Permission rule generation tests ─────────────────────────
|
||||
|
||||
#[test]
|
||||
fn generate_rule_for_edit_tool() {
|
||||
let rule = generate_permission_rule("Edit", &json!({}));
|
||||
assert_eq!(rule, "Edit");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn generate_rule_for_write_tool() {
|
||||
let rule = generate_permission_rule("Write", &json!({}));
|
||||
assert_eq!(rule, "Write");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn generate_rule_for_bash_git() {
|
||||
let rule =
|
||||
generate_permission_rule("Bash", &json!({"command": "git status"}));
|
||||
assert_eq!(rule, "Bash(git *)");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn generate_rule_for_bash_cargo() {
|
||||
let rule =
|
||||
generate_permission_rule("Bash", &json!({"command": "cargo test --all"}));
|
||||
assert_eq!(rule, "Bash(cargo *)");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn generate_rule_for_bash_empty_command() {
|
||||
let rule = generate_permission_rule("Bash", &json!({}));
|
||||
assert_eq!(rule, "Bash(unknown *)");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn generate_rule_for_mcp_tool() {
|
||||
let rule = generate_permission_rule(
|
||||
"mcp__story-kit__create_story",
|
||||
&json!({"name": "foo"}),
|
||||
);
|
||||
assert_eq!(rule, "mcp__story-kit__create_story");
|
||||
}
|
||||
|
||||
// ── Settings.json writing tests ──────────────────────────────
|
||||
|
||||
#[test]
|
||||
fn add_rule_creates_settings_file_when_missing() {
|
||||
let tmp = tempfile::tempdir().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();
|
||||
assert!(allow.contains(&json!("Edit")));
|
||||
}
|
||||
|
||||
#[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__story-kit__*"]}}"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
add_permission_rule(tmp.path(), "mcp__story-kit__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__story-kit__*");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn add_rule_appends_to_existing_rules() {
|
||||
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"]}}"#,
|
||||
)
|
||||
.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 allow = settings["permissions"]["allow"].as_array().unwrap();
|
||||
assert_eq!(allow.len(), 2);
|
||||
assert!(allow.contains(&json!("Edit")));
|
||||
assert!(allow.contains(&json!("Write")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn add_rule_preserves_other_settings_fields() {
|
||||
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":["story-kit"]}"#,
|
||||
)
|
||||
.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], "story-kit");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
use crate::http::context::AppContext;
|
||||
use crate::http::context::{AppContext, PermissionDecision};
|
||||
use crate::http::workflow::{PipelineState, load_pipeline_state};
|
||||
use crate::io::onboarding;
|
||||
use crate::io::watcher::WatcherEvent;
|
||||
@@ -29,6 +29,8 @@ enum WsRequest {
|
||||
PermissionResponse {
|
||||
request_id: String,
|
||||
approved: bool,
|
||||
#[serde(default)]
|
||||
always_allow: bool,
|
||||
},
|
||||
/// Heartbeat ping from the client. The server responds with `Pong` so the
|
||||
/// client can detect stale (half-closed) connections.
|
||||
@@ -250,7 +252,7 @@ pub async fn ws_handler(ws: WebSocket, ctx: Data<&Arc<AppContext>>) -> impl poem
|
||||
// Permission requests arrive from the MCP `prompt_permission` tool via
|
||||
// `ctx.perm_rx` and are forwarded to the client as `PermissionRequest`.
|
||||
// When the client responds, we resolve the corresponding oneshot.
|
||||
let mut pending_perms: HashMap<String, oneshot::Sender<bool>> = HashMap::new();
|
||||
let mut pending_perms: HashMap<String, oneshot::Sender<PermissionDecision>> = HashMap::new();
|
||||
|
||||
loop {
|
||||
// Outer loop: wait for the next WebSocket message.
|
||||
@@ -324,9 +326,16 @@ pub async fn ws_handler(ws: WebSocket, ctx: Data<&Arc<AppContext>>) -> impl poem
|
||||
// (permission responses and cancellations).
|
||||
Some(Ok(WsMessage::Text(inner_text))) = stream.next() => {
|
||||
match serde_json::from_str::<WsRequest>(&inner_text) {
|
||||
Ok(WsRequest::PermissionResponse { request_id, approved }) => {
|
||||
Ok(WsRequest::PermissionResponse { request_id, approved, always_allow }) => {
|
||||
if let Some(resp_tx) = pending_perms.remove(&request_id) {
|
||||
let _ = resp_tx.send(approved);
|
||||
let decision = if always_allow {
|
||||
PermissionDecision::AlwaysAllow
|
||||
} else if approved {
|
||||
PermissionDecision::Approve
|
||||
} else {
|
||||
PermissionDecision::Deny
|
||||
};
|
||||
let _ = resp_tx.send(decision);
|
||||
}
|
||||
}
|
||||
Ok(WsRequest::Cancel) => {
|
||||
@@ -457,9 +466,11 @@ mod tests {
|
||||
WsRequest::PermissionResponse {
|
||||
request_id,
|
||||
approved,
|
||||
always_allow,
|
||||
} => {
|
||||
assert_eq!(request_id, "req-42");
|
||||
assert!(approved);
|
||||
assert!(!always_allow);
|
||||
}
|
||||
_ => panic!("expected PermissionResponse variant"),
|
||||
}
|
||||
@@ -477,9 +488,34 @@ mod tests {
|
||||
WsRequest::PermissionResponse {
|
||||
request_id,
|
||||
approved,
|
||||
always_allow,
|
||||
} => {
|
||||
assert_eq!(request_id, "req-99");
|
||||
assert!(!approved);
|
||||
assert!(!always_allow);
|
||||
}
|
||||
_ => panic!("expected PermissionResponse variant"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserialize_permission_response_always_allow() {
|
||||
let json = r#"{
|
||||
"type": "permission_response",
|
||||
"request_id": "req-100",
|
||||
"approved": true,
|
||||
"always_allow": true
|
||||
}"#;
|
||||
let req: WsRequest = serde_json::from_str(json).unwrap();
|
||||
match req {
|
||||
WsRequest::PermissionResponse {
|
||||
request_id,
|
||||
approved,
|
||||
always_allow,
|
||||
} => {
|
||||
assert_eq!(request_id, "req-100");
|
||||
assert!(approved);
|
||||
assert!(always_allow);
|
||||
}
|
||||
_ => panic!("expected PermissionResponse variant"),
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user