@sigitcodereview / sigit / commits / b705a71

Let the user deny a tool for the rest of the session

The permission prompt now offers a fourth choice, "Deny for this session" (ACP `reject_always`). It records a denial scoped the same way an "allow for this session" grant is: denying `git push origin main` stops every `git push ...` for the thread, not every command. `decision_for` checks these denials before grants, and they go away with the rest of the session's permission state. The TUI prompt gets the same choice on `d`, and `/permissions` lists the denials next to the grants. Closes #199 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Seto Elkahfi committed Oct 9, 2026 at 00:08 UTC b705a7141b2c7919c4e8269f5b618e63b3f9fda0

5 files changed +208 -29

.agents/AGENTS.md

+6 -5

@@ -289,11 +289,12 @@ feeds results back. Neither the loop nor ACP/TUI surfaces depend on a concrete b

289 - **`src/permissions.rs`** — tool permission policy. Every tool call passes through

290 `decision_for` before executing: read-only tools always run; mutating tools (and all

291 `mcp__*`/unknown tools) are governed by, in order: per-session plan mode (`/plan` — deny all

292 - mutating tools with a present-a-plan message), session "always allow" grants, per-tool

293 - overrides and the default mode from `[permissions]` in `settings.toml` (`allow`/`ask`/`deny`,

294 - default `ask`; `SIGIT_PERMISSIONS` env overrides the default). On `ask`, the ACP path sends

295 - `session/request_permission` (allow once / allow for session / deny) and the TUI pauses the

296 - inference task on a y/a/n prompt. On the ACP path the decision is taken *before* the call is

292 + mutating tools with a present-a-plan message), session "always deny" and "always allow"

293 + choices (deny checked first, both scoped by `session_grant_rule` and dropped with the session),

294 + per-tool overrides and the default mode from `[permissions]` in `settings.toml`

295 + (`allow`/`ask`/`deny`, default `ask`; `SIGIT_PERMISSIONS` env overrides the default). On `ask`,

296 + the ACP path sends `session/request_permission` (allow once / allow for session / deny / deny

297 + for session) and the TUI pauses the inference task on a y/a/n/d prompt. On the ACP path the decision is taken *before* the call is

298 announced, because it sets the announced status: a call that will ask starts `pending`, the

299 permission request carries that call's own id, and an `in_progress` update follows approval.

300 That update also puts the card's title back, since the permission request overwrites it

src/chat.rs

+18 -6

@@ -652,6 +652,9 @@ mod tui {

652 Session,

653 /// skip the call; the model gets an explanatory tool result

654 Deny,

655 + /// skip it and stop asking: deny calls like it for the rest of the

656 + /// session

657 + DenySession,

658 }

659

660 enum ModelLoadUpdate {

@@ -736,7 +739,7 @@ mod tui {

739 stream_buf: String,

740 inference_rx: Option<mpsc::Receiver<InferenceUpdate>>,

741 model_load_rx: Option<mpsc::Receiver<ModelLoadUpdate>>,

739 - /// a tool call waiting on the user's y/a/n answer; the inference task is

742 + /// a tool call waiting on the user's y/a/n/d answer; the inference task is

743 /// paused on the other end of the channel

744 pending_approval: Option<(String, oneshot::Sender<ApprovalChoice>)>,

745 thinking: bool,

@@ -2590,7 +2593,7 @@ mod tui {

2593

2594 if let Some((tool, _)) = &app.pending_approval {

2595 spans.push(Span::styled(

2593 - format!(" allow {tool}? [y]es · [a]lways · [n]o"),

2596 + format!(" allow {tool}? [y]es · [a]lways · [n]o · [d]eny always"),

2597 Style::default().fg(Color::Yellow),

2598 ));

2599 } else if app.thinking || app.switching_model || app.is_streaming() {

@@ -3272,7 +3275,7 @@ mod tui {

3275

3276 // Permission gate: read-only tools pass straight through; a

3277 // mutating tool consults policy and may pause on the user's

3275 - // y/a/n answer (delivered over a oneshot from the event loop).

3278 + // y/a/n/d answer (delivered over a oneshot from the event loop).

3279 use crate::permissions::{self, Decision, TUI_SESSION};

3280 let output = match permissions::decision_for(TUI_SESSION, &tc.name, &tc.arguments) {

3281 Decision::Allow => crate::tools::execute_tool(&tc.name, &tc.arguments).await,

@@ -3305,6 +3308,11 @@ mod tui {

3308 log::info!(" ✗ {} denied by user", tc.name);

3309 permissions::user_denial(&tc.name)

3310 }

3311 + Ok(ApprovalChoice::DenySession) => {

3312 + log::info!(" ✗ {} denied by user for the session", tc.name);

3313 + permissions::deny_for_session(TUI_SESSION, &tc.name, &tc.arguments);

3314 + permissions::user_denial(&tc.name)

3315 + }

3316 // The UI dropped the reply channel (Ctrl+C or

3317 // quit): the whole turn is over, not just this

3318 // call. Close out the round and stop instead of

@@ -3596,7 +3604,7 @@ mod tui {

3604 app.messages.push(ChatMessage::system(format!("error: {msg}")));

3605 }

3606 Some(InferenceUpdate::ApprovalRequest { tool, args, reply }) => {

3599 - // The y/a/n prompt lives on the Session tab; make

3607 + // The y/a/n/d prompt lives on the Session tab; make

3608 // sure the user can see what they're answering.

3609 app.active_tab = Tab::Session;

3610 let call = if args.is_empty() {

@@ -3605,7 +3613,7 @@ mod tui {

3613 format!("{tool}({args})")

3614 };

3615 app.messages.push(ChatMessage::system(format!(

3608 - "⚠ permission — allow {call}? [y]es · [a]lways this session · [n]o"

3616 + "⚠ permission — allow {call}? [y]es · [a]lways this session · [n]o · [d]eny this session"

3617 )));

3618 app.pending_approval = Some((tool, reply));

3619 }

@@ -3709,7 +3717,7 @@ mod tui {

3717 continue;

3718 }

3719

3712 - // pending tool approval — y/a/n answer the prompt; the

3720 + // pending tool approval — y/a/n/d answer the prompt; the

3721 // inference task is paused on the reply channel. Checked

3722 // before the busy gate because the app *is* busy here.

3723 if app.pending_approval.is_some() {

@@ -3736,6 +3744,9 @@ mod tui {

3744 KeyCode::Char('n') | KeyCode::Char('N') | KeyCode::Esc => {

3745 Some(ApprovalChoice::Deny)

3746 }

3747 + KeyCode::Char('d') | KeyCode::Char('D') => {

3748 + Some(ApprovalChoice::DenySession)

3749 + }

3750 _ => None,

3751 }

3752 };

@@ -3746,6 +3757,7 @@ mod tui {

3757 ApprovalChoice::Once => "allowed once",

3758 ApprovalChoice::Session => "allowed for this session",

3759 ApprovalChoice::Deny => "denied",

3760 + ApprovalChoice::DenySession => "denied for this session",

3761 };

3762 app.messages.push(ChatMessage::system(format!(

3763 "{tool}: {verdict}"

src/main.rs

+16 -2

@@ -3900,8 +3900,9 @@ impl SiGitAgent {

3900 /// Ask the ACP client for permission to run one tool call. The request

3901 /// names the call by `tool_call_id`, the id it was announced under, so the

3902 /// client updates that one card instead of drawing a second. Presents

3903 - /// allow-once / allow-for-session / deny; an "always allow" choice is

3904 - /// recorded via [`permissions::grant_for_session`]. Only safe to call from

3903 + /// allow-once / allow-for-session / deny / deny-for-session; an "always"

3904 + /// choice is recorded via [`permissions::grant_for_session`] or

3905 + /// [`permissions::deny_for_session`]. Only safe to call from

3906 /// a spawned task (see the handler registration in `run_acp_server`): the

3907 /// dispatch loop must be free to route the client's answer back to us.

3908 ///

@@ -3951,6 +3952,11 @@ impl SiGitAgent {

3952 PermissionOptionKind::AllowAlways,

3953 ),

3954 PermissionOption::new("reject_once", "Deny", PermissionOptionKind::RejectOnce),

3955 + PermissionOption::new(

3956 + "reject_session",

3957 + "Deny for this session",

3958 + PermissionOptionKind::RejectAlways,

3959 + ),

3960 ],

3961 );

3962

@@ -3967,6 +3973,14 @@ impl SiGitAgent {

3973 );

3974 PermissionVerdict::Approved

3975 }

3976 + "reject_session" => {

3977 + permissions::deny_for_session(

3978 + &session_id.to_string(),

3979 + tool_name,

3980 + arguments,

3981 + );

3982 + PermissionVerdict::Denied(permissions::user_denial(tool_name))

3983 + }

3984 _ => PermissionVerdict::Denied(permissions::user_denial(tool_name)),

3985 }

3986 }

src/permissions.rs

+71 -15

@@ -13,7 +13,9 @@

13 //! picks that option in an approval prompt. For `run_command` the grant is

14 //! scoped to the command's first two whitespace-separated tokens (approving

15 //! `git push origin main` records `run_command(git push)`); other tools

16 -//! record the bare tool name.

16 +//! record the bare tool name. "Always deny this session" records a rule

17 +//! scoped the same way, and a session denial is checked before a session

18 +//! grant.

19 //! 3. **Rule lists** — `[permissions.rules]` in `settings.toml`: ordered

20 //! `deny` and `allow` lists of rules shaped `tool_name` or

21 //! `tool_name(argument_pattern)`, e.g. `run_command(git status)`,

@@ -134,6 +136,8 @@ pub fn classify(tool_name: &str) -> ToolRisk {

136 struct SessionPerms {

137 /// Tools the user chose "always allow this session" for.

138 always_allow: HashSet<String>,

139 + /// Tools the user chose "always deny this session" for.

140 + always_deny: HashSet<String>,

141 /// When set, every mutating tool is denied with a plan-mode message.

142 plan_mode: bool,

143 /// Session override picked from the panel Permissions dropdown. `None`

@@ -294,15 +298,26 @@ pub fn decision_for(session: &str, tool_name: &str, arguments: &str) -> Decision

298 return Decision::Allow;

299 }

300

297 - let (plan_mode, granted) = with_session(session, |s| {

301 + let (plan_mode, denied, granted) = with_session(session, |s| {

302 + let matches = |set: &HashSet<String>| {

303 + set.iter()

304 + .find(|rule| rule_matches(rule, tool_name, argument.as_deref()))

305 + .cloned()

306 + };

307 (

308 s.plan_mode,

300 - s.always_allow

301 - .iter()

302 - .any(|grant| rule_matches(grant, tool_name, argument.as_deref())),

309 + matches(&s.always_deny),

310 + matches(&s.always_allow).is_some(),

311 )

312 });

313

314 + if let Some(rule) = denied {

315 + return Decision::Deny(format!(

316 + "The user chose to always deny `{rule}` in this session, so `{tool_name}` \

317 + was not executed. Do not retry it; continue with an approach that does \

318 + not need this tool, or ask the user how to proceed."

319 + ));

320 + }

321 if plan_mode {

322 return Decision::Deny(plan_mode_denial(tool_name));

323 }

@@ -345,6 +360,16 @@ pub fn grant_for_session(session: &str, tool_name: &str, arguments: &str) {

360 });

361 }

362

363 +/// Record an "always deny this session" choice for a tool call, scoped the

364 +/// same way as [`grant_for_session`]: denying `git push origin main` stops

365 +/// every `git push …` for the rest of the session, not every command.

366 +pub fn deny_for_session(session: &str, tool_name: &str, arguments: &str) {

367 + let rule = session_grant_rule(tool_name, arguments);

368 + with_session(session, |s| {

369 + s.always_deny.insert(rule);

370 + });

371 +}

372 +

373 /// The rule string recorded for one approved call (see [`grant_for_session`]).

374 /// Falls back to the bare tool name when the command is absent or empty, which

375 /// grants the whole tool — exactly what the pre-granular behavior was.

@@ -411,16 +436,18 @@ pub fn describe(session: &str) -> String {

436 Some(mode) => format!("{mode} (session override)"),

437 None => format!("{default} (stored default)"),

438 };

414 - let granted = with_session(session, |s| {

415 - let mut names: Vec<&str> = s.always_allow.iter().map(String::as_str).collect();

416 - names.sort_unstable();

417 - names.join(", ")

439 + let (granted, denied) = with_session(session, |s| {

440 + let render = |set: &HashSet<String>| {

441 + let mut names: Vec<&str> = set.iter().map(String::as_str).collect();

442 + names.sort_unstable();

443 + if names.is_empty() {

444 + "none".to_string()

445 + } else {

446 + names.join(", ")

447 + }

448 + };

449 + (render(&s.always_allow), render(&s.always_deny))

450 });

419 - let granted = if granted.is_empty() {

420 - "none".to_string()

421 - } else {

422 - granted

423 - };

451 let rules = settings::permission_rules();

452 let render = |list: &[String]| {

453 if list.is_empty() {

@@ -430,7 +457,8 @@ pub fn describe(session: &str) -> String {

457 }

458 };

459 format!(

433 - "permissions: mode={mode} | plan mode: {plan} | session grants: {granted}\n\

460 + "permissions: mode={mode} | plan mode: {plan} | session grants: {granted} \

461 + | session denials: {denied}\n\

462 rules: deny: {} | allow: {}\n\

463 read-only tools always run; configure [permissions] in settings.toml",

464 render(&rules.deny),

@@ -662,6 +690,34 @@ mod tests {

690 assert_ne!(decision_for(session, "edit_file", args), Decision::Allow);

691 }

692

693 + #[test]

694 + fn session_denial_stops_asking_and_outranks_a_grant() {

695 + let _guard = env_guard();

696 + let session = "t-deny";

697 + reset_session(session);

698 + let push = r#"{"command":"git push origin main"}"#;

699 + deny_for_session(session, "run_command", push);

700 + // Scoped like a grant: the whole `git push …` family, nothing else.

701 + assert!(matches!(

702 + decision_for(session, "run_command", r#"{"command":"git push --tags"}"#),

703 + Decision::Deny(_)

704 + ));

705 + assert_eq!(

706 + decision_for(session, "run_command", r#"{"command":"git status"}"#),

707 + Decision::Ask

708 + );

709 + // A denial is checked before a grant covering the same call.

710 + grant_for_session(session, "run_command", r#"{"command":"git push"}"#);

711 + assert!(matches!(

712 + decision_for(session, "run_command", push),

713 + Decision::Deny(_)

714 + ));

715 + assert!(describe(session).contains("session denials: run_command(git push)"));

716 + // Dropped with the rest of the session's state.

717 + reset_session(session);

718 + assert_eq!(decision_for(session, "run_command", push), Decision::Ask);

719 + }

720 +

721 #[test]

722 fn approval_preview_shows_short_arguments_in_full() {

723 let args = r#"{"command":"cargo test"}"#;

tests/acp_permissions.rs

+97 -1

@@ -384,7 +384,22 @@ fn permission_round_trip_cancel_then_allow() {

384 .iter()

385 .map(|option| option["optionId"].as_str().unwrap_or_default())

386 .collect();

387 - assert_eq!(option_ids, ["allow_once", "allow_session", "reject_once"]);

387 + assert_eq!(

388 + option_ids,

389 + [

390 + "allow_once",

391 + "allow_session",

392 + "reject_once",

393 + "reject_session"

394 + ]

395 + );

396 + let reject_always = params["options"]

397 + .as_array()

398 + .unwrap()

399 + .iter()

400 + .find(|option| option["optionId"] == "reject_session")

401 + .unwrap();

402 + assert_eq!(reject_always["kind"], "reject_always");

403

404 agent.respond(

405 permission["id"].clone(),

@@ -627,6 +642,87 @@ fn a_call_denied_at_the_prompt_ends_failed() {

642 let _ = std::fs::remove_dir_all(&scratch);

643 }

644

645 +/// "Deny for this session" answers the call it was picked for and every later

646 +/// call in the same family without asking again (issue #199).

647 +#[test]

648 +fn a_call_denied_for_the_session_is_not_asked_about_again() {

649 + let endpoint = start_fake_endpoint(vec![

650 + sse_tool_call("call_1", "run_command", r#"{"command":"echo sigit-never"}"#),

651 + sse_tool_call(

652 + "call_2",

653 + "run_command",

654 + r#"{"command":"echo sigit-never again"}"#,

655 + ),

656 + sse_text("ok, I will stop"),

657 + ]);

658 +

659 + let scratch =

660 + std::env::temp_dir().join(format!("sigit_acp_perm_deny_all_{}", std::process::id()));

661 + let config_dir = scratch.join("config");

662 + let cwd = scratch.join("cwd");

663 + std::fs::create_dir_all(&config_dir).unwrap();

664 + std::fs::create_dir_all(&cwd).unwrap();

665 +

666 + let mut agent = spawn_agent(endpoint.port, &config_dir);

667 +

668 + let id = agent.request(

669 + "initialize",

670 + json!({"protocolVersion": 1, "clientCapabilities": {}}),

671 + );

672 + agent.wait_for_response(id);

673 +

674 + let id = agent.request("session/new", json!({"cwd": cwd, "mcpServers": []}));

675 + let session_id = agent.wait_for_response(id)["result"]["sessionId"]

676 + .as_str()

677 + .expect("session id")

678 + .to_string();

679 +

680 + let prompt_id = agent.request(

681 + "session/prompt",

682 + json!({

683 + "sessionId": session_id,

684 + "prompt": [{"type": "text", "text": "run the command"}],

685 + }),

686 + );

687 +

688 + let permission = agent.wait_for_agent_request("session/request_permission");

689 + agent.respond(

690 + permission["id"].clone(),

691 + json!({"outcome": {"outcome": "selected", "optionId": "reject_session"}}),

692 + );

693 +

694 + // The second call is denied by the recorded choice: no second request,

695 + // which `wait_for_response_with_updates` would fail on.

696 + let (response, updates) = agent.wait_for_response_with_updates(prompt_id);

697 + assert_eq!(response["result"]["stopReason"], "end_turn");

698 + let second = updates

699 + .iter()

700 + .rev()

701 + .find(|update| {

702 + update["sessionUpdate"] == "tool_call_update" && update["toolCallId"] == "call_2"

703 + })

704 + .expect("the second call closes");

705 + assert_eq!(second["status"], "failed", "{second}");

706 +

707 + let requests = endpoint.requests.lock().unwrap();

708 + let result = requests[2]["messages"]

709 + .as_array()

710 + .expect("messages")

711 + .iter()

712 + .find(|message| message["role"] == "tool" && message["tool_call_id"] == "call_2")

713 + .expect("tool result for the second call");

714 + assert!(

715 + result["content"]

716 + .as_str()

717 + .unwrap_or_default()

718 + .contains("always deny"),

719 + "the model is told why: {result}"

720 + );

721 +

722 + drop(agent);

723 + let _ = std::fs::remove_dir_all(&scratch);

724 +}

725 +

726 #[test]

727 fn successful_config_option_changes_are_rendered_as_system_status_cards() {

728 let endpoint = start_fake_endpoint(vec![]);