From 3d18cc0ff494ded984d81f0a0140475dfec9a550 Mon Sep 17 00:00:00 2001
From: Yogthos <yogthos@gmail.com>
Date: Thu, 21 May 2026 00:04:41 -0400
Subject: [PATCH] fix(F1): route ACP permission asks to auto-deny instead of
 dropping
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Track F-CRITICAL #1 from ROADMAP.md.

## Problem

`extras/acp/mod.rs::build_acp_permission` constructed
`(ask_tx, _ask_rx)` and immediately dropped `ask_rx`. When a tool
needed `Ask` confirmation in ACP mode (Zed / editor client), the
permission check called `ask_tx.send()`, awaited the oneshot
reply, and waited 30 seconds before timing out. The user saw a
generic tool failure with no context.

Tools requiring permission were effectively unrunnable from ACP
clients unless the user configured `--yolo` or explicit allow
rules in `permission` config.

## Fix

New `spawn_acp_ask_drain` task takes ownership of the receiver
and responds to every `AskRequest` with
`UserDecision::Deny` immediately. Fail-fast with a clear deny
beats a 30s timeout:
- The LLM sees the denial in the tool result and can re-plan.
- The error message is "Permission denied by user", not a
  generic timeout.
- Explicit allow rules (set via config or `/allow add` in
  interactive mode beforehand) still work — they short-circuit
  before the ask channel.

Routing the ask through the ACP protocol as a real
`requestPermission` notification (so the editor surfaces a dialog)
is a larger Phase C5-ish feature; F1 is the minimum-viable
non-hang fix.

## Tests

Two new tests in `extras::acp::tests`:

- `acp_ask_drain_responds_with_deny`: send one `AskRequest`,
  assert the reply arrives within 200ms with `Deny`.
- `acp_ask_drain_handles_multiple_concurrent_asks`: send 5
  asks in a loop, assert all 5 replies arrive promptly with
  Deny — guards against a single-shot drain bug.

## Test plan

- [x] `cargo test --features acp` — 2 new tests pass.
- [x] `cargo test --features plugin` — 649 pass (full suite).
- [x] `cargo build --all-features` — compiles, no warnings.
---
 src/extras/acp/mod.rs | 99 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 97 insertions(+), 2 deletions(-)

diff --git a/src/extras/acp/mod.rs b/src/extras/acp/mod.rs
index baf11d75..4c876f30 100644
--- a/src/extras/acp/mod.rs
+++ b/src/extras/acp/mod.rs
@@ -284,11 +284,106 @@ fn build_acp_permission(state: &AcpState) -> (Option<PermCheck>, Option<AskSende
     let checker = PermissionChecker::new(&perm_config, mode, None);
     let perm: PermCheck = Arc::new(Mutex::new(checker));
 
-    let (ask_tx, _ask_rx) = tokio::sync::mpsc::channel(64);
-
+    let (ask_tx, ask_rx) = tokio::sync::mpsc::channel(64);
+    spawn_acp_ask_drain(ask_rx);
     (Some(perm), Some(ask_tx))
 }
 
+/// Drain `ask_rx` by responding to every permission ask with
+/// `Deny`. ACP runs are non-interactive — there's no human at a
+/// keyboard to confirm prompts. Previously the receiver was simply
+/// dropped (`_ask_rx`), so any tool needing `Ask` confirmation
+/// hit the 30s permission timeout and surfaced as a generic
+/// failure to the editor client. Fail-fast with a clear deny is
+/// strictly better: the LLM sees the denial immediately and can
+/// re-plan, or the user can configure explicit allow rules.
+///
+/// **Future work**: route the ask through the ACP protocol as a
+/// `requestPermission` notification so the editor client can
+/// surface a real dialog. Out of scope for the F1 fix; that's a
+/// Phase C5-ish feature requiring ACP protocol wiring.
+fn spawn_acp_ask_drain(
+    mut ask_rx: tokio::sync::mpsc::Receiver<crate::permission::ask::AskRequest>,
+) {
+    tokio::spawn(async move {
+        while let Some(req) = ask_rx.recv().await {
+            // The tool's caller is awaiting on `req.reply`. Dropping
+            // it without sending would also surface as a tool error
+            // ("Permission system unavailable"), but Deny is a
+            // clearer signal that the call was *refused* rather
+            // than the system being broken.
+            let _ = req.reply.send(crate::permission::ask::UserDecision::Deny);
+        }
+    });
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Regression for F1: any `AskRequest` sent through the ACP
+    /// ask channel must be promptly responded to with `Deny`,
+    /// rather than hanging. Without `spawn_acp_ask_drain`, the
+    /// `reply` oneshot is dropped on receiver drop → tool sees
+    /// `Permission system unavailable` (technically OK, but slower
+    /// and worse signal). With the drain, the tool sees `Deny`
+    /// within a tick.
+    #[tokio::test]
+    async fn acp_ask_drain_responds_with_deny() {
+        let (ask_tx, ask_rx) = tokio::sync::mpsc::channel::<crate::permission::ask::AskRequest>(8);
+        spawn_acp_ask_drain(ask_rx);
+
+        let (reply_tx, reply_rx) = tokio::sync::oneshot::channel();
+        ask_tx
+            .send(crate::permission::ask::AskRequest {
+                tool: "bash".to_string(),
+                input: "rm -rf /".to_string(),
+                reply: reply_tx,
+            })
+            .await
+            .expect("send must succeed");
+
+        let resp = tokio::time::timeout(std::time::Duration::from_millis(200), reply_rx)
+            .await
+            .expect("must reply within 200ms — F1 regression")
+            .expect("reply channel must not be dropped");
+        assert!(
+            matches!(resp, crate::permission::ask::UserDecision::Deny),
+            "ACP ask must auto-deny; got {:?}",
+            resp,
+        );
+    }
+
+    /// Multiple concurrent asks all get responded to.
+    #[tokio::test]
+    async fn acp_ask_drain_handles_multiple_concurrent_asks() {
+        let (ask_tx, ask_rx) = tokio::sync::mpsc::channel::<crate::permission::ask::AskRequest>(8);
+        spawn_acp_ask_drain(ask_rx);
+
+        let mut replies = Vec::new();
+        for i in 0..5 {
+            let (reply_tx, reply_rx) = tokio::sync::oneshot::channel();
+            ask_tx
+                .send(crate::permission::ask::AskRequest {
+                    tool: format!("bash-{i}"),
+                    input: format!("cmd-{i}"),
+                    reply: reply_tx,
+                })
+                .await
+                .unwrap();
+            replies.push(reply_rx);
+        }
+
+        for reply_rx in replies {
+            let resp = tokio::time::timeout(std::time::Duration::from_millis(500), reply_rx)
+                .await
+                .expect("each reply must arrive promptly")
+                .expect("reply channel dropped");
+            assert!(matches!(resp, crate::permission::ask::UserDecision::Deny));
+        }
+    }
+}
+
 fn resolve_acp_mode(cli: &Cli, cfg: &Config) -> SecurityMode {
     if cli.yolo || cfg.yolo.unwrap_or(false) {
         SecurityMode::Yolo