diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 93109fa94d..3c39d7a302 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -13,13 +13,29 @@ use tokio::io::AsyncWriteExt; use tokio::process::{Child, ChildStdin, ChildStdout}; use tokio_util::codec::{FramedRead, LinesCodec, LinesCodecError}; -use crate::observer::{ObserverContext, ObserverHandle}; +use crate::config::{PermissionMode, PermissionPolicy, ResolvedPermissionConfig}; +use crate::observer::{AuthorizationEnvelope, ObserverContext, ObserverEvent, ObserverHandle}; use crate::usage::{TurnUsage, UsageTracker}; +use buzz_core::observer::OBSERVER_MAX_PLAINTEXT_LEN; /// Maximum allowed size of a single NDJSON line from the agent's stdout. /// Lines exceeding this limit are rejected to prevent OOM from rogue agents. const MAX_LINE_SIZE: usize = 10_000_000; // 10 MB +/// Maximum number of `session/request_permission` requests that may be +/// simultaneously pending under the `ask` policy. New requests beyond this +/// cap are denied immediately (fail closed) so the map remains bounded. +pub const PERMISSION_MAP_CAP: usize = 8; + +/// Maximum number of options in a single `session/request_permission` request. +/// Requests with more options are denied immediately (admission preflight). +const PERMISSION_OPTIONS_MAX: usize = 16; + +/// Per-request timeout under the `ask` policy. The desktop has at most this +/// long to deliver a `permission_decision` control frame before the harness +/// fails closed with the denial response. +const PERMISSION_ASK_TIMEOUT_SECS: u64 = 300; + /// An MCP server configuration passed to `session/new`. /// /// Corresponds to the `McpServerStdio` variant in the ACP schema. @@ -106,6 +122,16 @@ pub enum AcpError { #[error("Agent reported error (code {code}): {message}")] AgentError { code: i64, message: String }, + + /// A permission response write was interrupted mid-flight by a cancel. + /// + /// The process may have received the response bytes but may not have acted + /// on them — state is irrecoverably uncertain. The agent process MUST be + /// replaced (not returned to the pool) after this error. The cancel path + /// surfaces this through `cancel_with_cleanup_grace` so + /// `classify_control_cancel_failure` in `pool.rs` triggers respawn. + #[error("Permission response write was interrupted — process state uncertain")] + PermissionPoisoned, } /// Build an [`AcpError::AgentError`] from a JSON-RPC error object, @@ -132,6 +158,45 @@ fn build_initialize_params() -> serde_json::Value { }) } +/// A decision delivered by the desktop via a `permission_decision` control frame. +#[derive(Debug, Clone)] +pub struct PermissionDecision { + /// The nonce that was advertised in the `authorization` envelope of the + /// `acp_read` frame for this request. + pub request_nonce: String, + /// The `optionId` the owner chose. Must exactly match one of the options in + /// the original request. + pub option_id: String, +} + +/// Lifecycle state of a single `session/request_permission` request under +/// the `ask` policy. +#[derive(Debug)] +enum PermissionEntryState { + /// Registered and waiting for an owner decision. + Pending, + /// A decision arrived; we are in the process of writing the response. + /// Cancel during this state → `PermissionPoisoned`. + Writing, + /// Fully resolved — write confirmed. Kept in map until turn end to guard + /// against duplicate delivery. + Resolved, +} + +/// Per-request state tracked in `AcpClient::pending_permissions` under `ask`. +#[derive(Debug)] +struct PermissionEntry { + /// Nonce bound to this request — must match the desktop's decision. + nonce: String, + /// The exact options snapshot from the original request. + options_snapshot: Vec, + /// Current lifecycle state. + state: PermissionEntryState, + /// Per-request hard deadline: `min(registered_at + 300s, turn hard deadline)`. + /// Expiry → fail closed (denial + `cancelled` outcome). + deadline: tokio::time::Instant, +} + /// ACP client that owns an agent subprocess and communicates over its stdio. /// /// One `AcpClient` per agent process. Multiple sessions can be created on the @@ -153,11 +218,39 @@ pub struct AcpClient { /// permits both numeric and string IDs from the agent. /// Used by [`cancel_with_cleanup`](AcpClient::cancel_with_cleanup) to send /// a `cancelled` outcome before the agent returns from `session/prompt`. + /// + /// Under `reject` and `allow` policies only one request can be in-flight + /// (synchronous handling), so a single Option suffices. + /// Under `ask` the full map is `pending_permissions` below. pending_permission_id: Option, /// Whether we have already sent a response to the pending permission request. /// Guards against double-response if a timeout fires after the rejection /// response was written but before `pending_permission_id` was cleared. permission_responded: bool, + /// Pending `session/request_permission` entries under the `ask` policy. + /// + /// Keyed by request id (as JSON Value). Bounded at `PERMISSION_MAP_CAP`. + /// Entries transition: `Pending → Writing(optionId) → Resolved`. + /// Cancel during `Writing` → `PermissionPoisoned`. + /// Cleared at turn end. + pending_permissions: std::collections::HashMap, + /// Whether this process is poisoned due to a cancel-during-write. + /// + /// When `true` the process MUST NOT be returned to the pool — it must be + /// respawned. The cancel path surfaces this via `PermissionPoisoned`. + permission_poisoned: bool, + /// Resolved permission configuration. Determines how `handle_permission_request` + /// answers ACP `session/request_permission` frames. + permission_config: ResolvedPermissionConfig, + /// Whether an agent owner pubkey was resolved at startup. + /// + /// Used by the `ask` availability gate: `ask` without a known owner downgrades + /// to `reject` (the desktop needs an owner to route the permission card to). + owner_pubkey_known: bool, + /// Channel for delivering `permission_decision` control frames from the + /// observer dispatch loop into the read loop's decision arm. + /// Installed by `install_permission_decision_rx`; consumed by the read loop. + permission_decision_rx: Option>, /// The JSON-RPC id of the most recently sent `session/prompt` request. /// Used by [`cancel_with_cleanup`] to drain the correct response. /// Set in [`session_prompt_with_idle_timeout`]; consumed in [`cancel_with_cleanup`]. @@ -541,6 +634,16 @@ impl AcpClient { next_id: 0, pending_permission_id: None, permission_responded: false, + pending_permissions: std::collections::HashMap::new(), + permission_poisoned: false, + permission_config: ResolvedPermissionConfig { + policy: crate::config::PermissionPolicy::Reject, + effective_mode: PermissionMode::DontAsk, + mode_source: crate::config::ModeSource::Derived, + transmit_mode: true, + }, + owner_pubkey_known: false, + permission_decision_rx: None, last_prompt_id: None, current_hard_deadline: None, observer: None, @@ -559,6 +662,33 @@ impl AcpClient { self.observer_agent_index = Some(agent_index); } + /// Set the resolved permission configuration for this agent process. + /// + /// Called once after spawn (like `set_observer`) by `pool_lifecycle`. + pub fn set_permission_config(&mut self, config: ResolvedPermissionConfig) { + self.permission_config = config; + } + + /// Record whether the agent owner pubkey is known at startup. + /// + /// The `ask` availability gate downgrades to `reject` when the owner is + /// unknown — the desktop needs an owner to route the permission card. + pub fn set_owner_pubkey_known(&mut self, known: bool) { + self.owner_pubkey_known = known; + } + + /// Install the per-session `permission_decision` receiver. + /// + /// The matching `Sender` is held by `handle_observer_control` in `lib.rs` + /// and delivers `permission_decision` control frames into the read loop's + /// decision arm. Idempotent — replaces any previously installed receiver. + pub fn install_permission_decision_rx( + &mut self, + rx: tokio::sync::mpsc::Receiver, + ) { + self.permission_decision_rx = Some(rx); + } + /// Update metadata that will be attached to subsequent raw wire events. pub fn set_observer_context(&mut self, context: ObserverContext) { self.observer_context = context; @@ -586,6 +716,24 @@ impl AcpClient { } } + /// Emit a semantic event with an authorization envelope, if observer enabled. + fn observe_authorized( + &self, + kind: impl Into, + authorization: AuthorizationEnvelope, + payload: serde_json::Value, + ) { + if let Some(observer) = &self.observer { + observer.emit_authorized( + kind, + self.observer_agent_index, + &self.observer_context, + authorization, + payload, + ); + } + } + /// Send the `initialize` request and return the agent's response result value. /// /// Must be called exactly once, before any other ACP method. @@ -811,6 +959,10 @@ impl AcpClient { Ok(_) => { self.last_prompt_id = None; self.current_hard_deadline = None; + // Turn completed normally — drain resolved/expired permission entries. + // Pending entries are unexpected here (should be Resolved or expired), + // but drain unconditionally to guarantee the map never leaks across turns. + self.pending_permissions.clear(); } Err(AcpError::IdleTimeout(_) | AcpError::HardTimeout { .. }) => { // Leave last_prompt_id and current_hard_deadline set — @@ -819,6 +971,10 @@ impl AcpClient { Err(_) => { self.last_prompt_id = None; self.current_hard_deadline = None; + // Non-recoverable error — drain the map to prevent capacity leak + // if the pool reuses this process (poisoned processes are respawned, + // but clean error exits may be returned to the pool). + self.pending_permissions.clear(); } } self.parse_stop_reason(&result?) @@ -1003,8 +1159,81 @@ impl AcpClient { AcpError::Protocol("cancel_with_cleanup called with no in-flight prompt".into()) })?; - // Step 1: respond to any pending permission request with "cancelled", - // but only if we haven't already responded (guards against double-response race). + // Check for poisoning first: if a permission write is in progress we + // must not send any more bytes to this process — return the dedicated + // error so `classify_control_cancel_failure` triggers respawn. + if self.permission_poisoned { + tracing::error!( + target: "acp::cancel", + "cancel on poisoned process — triggering respawn" + ); + return Err(AcpError::PermissionPoisoned); + } + + // Step 1: respond to any pending permission request with "cancelled". + // + // Under `ask` policy: drain all pending entries (cancel each one); + // check for any entry currently in `Writing` state → that's a + // cancel-during-write, so poison the process. + // + // Under `reject`/`allow` policy: use the old single-id path. + let mut cancel_during_write = false; + + // Ask-policy pending map: drain every Pending entry with cancelled; + // Writing entries poison the process. + let ids_to_cancel: Vec = self.pending_permissions.keys().cloned().collect(); + for req_id_str in ids_to_cancel { + let entry = self.pending_permissions.remove(&req_id_str).unwrap(); + match entry.state { + PermissionEntryState::Writing => { + tracing::error!( + target: "acp::cancel", + "cancel during permission write for req_id={req_id_str} — poisoning process" + ); + cancel_during_write = true; + // Don't try to write anything to this process. + } + PermissionEntryState::Pending => { + // Parse id back to JSON value for the wire response. + let perm_id: serde_json::Value = serde_json::from_str(&req_id_str) + .unwrap_or_else(|_| serde_json::Value::String(req_id_str.clone())); + let response = permission_response_cancelled(&perm_id); + if let Err(e) = self.write_ndjson_no_observe(&response).await { + tracing::warn!( + target: "acp::cancel", + "failed to write cancelled for pending perm id={req_id_str}: {e}" + ); + // Best-effort; continue to session/cancel. + } else { + // Emit one authorized acp_write with the original nonce + // so the desktop can retire the card by nonce correlation. + self.observe_authorized( + "acp_write", + AuthorizationEnvelope { + request_nonce: entry.nonce.clone(), + actionable: false, + reason: Some("cancelled".to_string()), + }, + response, + ); + tracing::debug!( + target: "acp::cancel", + "responded cancelled to pending permission id={req_id_str}" + ); + } + } + PermissionEntryState::Resolved => { + // Already resolved — nothing to do. + } + } + } + + if cancel_during_write { + self.permission_poisoned = true; + return Err(AcpError::PermissionPoisoned); + } + + // Old single-id path (reject/allow policy). if let Some(perm_id) = self.pending_permission_id.clone() { if !self.permission_responded { let response = permission_response_cancelled(&perm_id); @@ -1038,6 +1267,9 @@ impl AcpClient { remaining, ) .await?; + // Cancel completed — drain any remaining permission entries (they were + // answered with cancelled above, but drain Resolved ones to free capacity). + self.pending_permissions.clear(); self.parse_stop_reason(&result) } @@ -1045,7 +1277,26 @@ impl AcpClient { /// /// Bounded by a 30-second write timeout. If the agent stops reading stdin /// (e.g., it's stuck or dead), the write would otherwise block forever. + /// + /// Emits a generic `acp_write` observer event. For permission response paths + /// that emit their own authorized event, use `write_ndjson_no_observe`. async fn write_ndjson(&mut self, value: &serde_json::Value) -> Result<(), AcpError> { + self.write_ndjson_inner(value, true).await + } + + /// Write NDJSON without emitting a generic `acp_write` observer event. + /// + /// Used for permission response paths that emit a single authorized event + /// themselves — prevents duplicate generic+authorized telemetry. + async fn write_ndjson_no_observe(&mut self, value: &serde_json::Value) -> Result<(), AcpError> { + self.write_ndjson_inner(value, false).await + } + + async fn write_ndjson_inner( + &mut self, + value: &serde_json::Value, + emit_observe: bool, + ) -> Result<(), AcpError> { const WRITE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); let line = serde_json::to_string(value)?; tokio::time::timeout(WRITE_TIMEOUT, async { @@ -1057,7 +1308,9 @@ impl AcpClient { .await .map_err(|_| AcpError::WriteTimeout(WRITE_TIMEOUT))? .map_err(AcpError::Io)?; - self.observe("acp_write", value.clone()); + if emit_observe { + self.observe("acp_write", value.clone()); + } Ok(()) } @@ -1239,7 +1492,26 @@ impl AcpClient { self.handle_goose_usage_update(&msg); } "session/request_permission" => { - self.handle_permission_request(&msg).await?; + // Pre-turn (session/new) path: no decision arm installed. + // Force reject regardless of policy — ask requests would + // register map entries that can never be resolved without + // the turn reader's decision arm. + let saved_policy = self.permission_config.policy; + if matches!(saved_policy, PermissionPolicy::Ask) { + // Temporarily downgrade to reject for this request only. + let saved = std::mem::replace( + &mut self.permission_config.policy, + PermissionPolicy::Reject, + ); + let deadline = tokio::time::Instant::now() + + std::time::Duration::from_secs(PERMISSION_ASK_TIMEOUT_SECS); + let _ = self.handle_permission_request(&msg, true, deadline).await; + self.permission_config.policy = saved; + } else { + let deadline = tokio::time::Instant::now() + + std::time::Duration::from_secs(PERMISSION_ASK_TIMEOUT_SECS); + self.handle_permission_request(&msg, true, deadline).await?; + } } other => { // If the unknown message has an id, it's a request expecting a reply. @@ -1309,6 +1581,11 @@ impl AcpClient { // so the ack_tx oneshot is never leaked silently). let mut steer_rx = self.steer_rx.take(); + // Take the per-session permission decision receiver into a local for + // the same reason: `self.reader` and `decision_rx` cannot both be + // borrowed inside `select!` via `self`. + let mut decision_rx = self.permission_decision_rx.take(); + // Tracks the in-flight steer write: `(request_id, transport, ack_tx)`. // While `Some`, the steer arm is gated off so we don't stack writes, // and a response matching `id` is routed to the ack_tx instead @@ -1328,14 +1605,48 @@ impl AcpClient { let mut last_activity_at = now; loop { + // If the process was poisoned by a cancel-during-write, surface the + // error immediately so the caller can respawn. + if self.permission_poisoned { + if let Some((_, _, ack_tx)) = pending_steer.take() { + let _ = ack_tx.send(crate::pool::SteerAck::PromptCompletedNeutral); + } + return Err(AcpError::PermissionPoisoned); + } + // Determine which deadline fires first BEFORE sleeping — this is // the classification we'll use on timeout, immune to scheduler jitter. - let idle_fires_first = idle_deadline < hard_deadline; - let next_deadline = if idle_fires_first { - idle_deadline + // + // Deadline logic: + // - When any Pending permission entries exist, suspend the idle + // deadline (owner is deciding; agent silence is expected) and + // wake on the earliest permission deadline instead. + // - Otherwise wake on min(idle, hard) as normal. + let has_pending_permissions = self + .pending_permissions + .values() + .any(|e| matches!(e.state, PermissionEntryState::Pending)); + let next_deadline; + let idle_fires_first; + if has_pending_permissions { + // Suspend idle; find earliest permission deadline (capped by hard). + let earliest_perm = self + .pending_permissions + .values() + .filter(|e| matches!(e.state, PermissionEntryState::Pending)) + .map(|e| e.deadline) + .min() + .unwrap_or(hard_deadline); + next_deadline = earliest_perm.min(hard_deadline); + idle_fires_first = false; // hard deadline governs if we wake } else { - hard_deadline - }; + idle_fires_first = idle_deadline < hard_deadline; + next_deadline = if idle_fires_first { + idle_deadline + } else { + hard_deadline + }; + } // Pre-select deadline check — required by Max's review. Under // `biased`, a continuously-ready reader arm wins every poll and @@ -1345,20 +1656,79 @@ impl AcpClient { // exists). Check the classified deadline here so a steady- // stream agent is still bounded. if Instant::now() >= next_deadline { - if let Some((_, _, ack_tx)) = pending_steer.take() { - // Prompt is timing out — release the withheld event via - // PromptCompletedNeutral (no fallback signal: there is - // no in-flight turn to signal once we return, and - // normal dispatch handles redelivery). - let _ = ack_tx.send(crate::pool::SteerAck::PromptCompletedNeutral); + // When we woke for a permission deadline (not the hard deadline), + // skip the error return — let the expiry block below process the + // timed-out entries, then continue the loop. + let is_permission_wake = has_pending_permissions && next_deadline != hard_deadline; + if !is_permission_wake { + if let Some((_, _, ack_tx)) = pending_steer.take() { + // Prompt is timing out — release the withheld event via + // PromptCompletedNeutral (no fallback signal: there is + // no in-flight turn to signal once we return, and + // normal dispatch handles redelivery). + let _ = ack_tx.send(crate::pool::SteerAck::PromptCompletedNeutral); + } + if idle_fires_first { + tracing::warn!("idle timeout ({idle_timeout:?}) — no agent activity"); + return Err(AcpError::IdleTimeout(idle_timeout)); + } else { + let silence = Instant::now().saturating_duration_since(last_activity_at); + tracing::warn!("hard turn timeout exceeded (silence {silence:?})"); + return Err(AcpError::HardTimeout { silence }); + } } - if idle_fires_first { - tracing::warn!("idle timeout ({idle_timeout:?}) — no agent activity"); - return Err(AcpError::IdleTimeout(idle_timeout)); - } else { - let silence = Instant::now().saturating_duration_since(last_activity_at); - tracing::warn!("hard turn timeout exceeded (silence {silence:?})"); - return Err(AcpError::HardTimeout { silence }); + } + + // Expire any pending `ask` permission entries whose per-request + // deadline has passed. Fail closed: write denial response for each + // expired entry and transition to Resolved. + { + let now = Instant::now(); + let expired: Vec<(String, serde_json::Value, Vec, String)> = + self.pending_permissions + .iter() + .filter(|(_, e)| { + matches!(e.state, PermissionEntryState::Pending) && now >= e.deadline + }) + .map(|(id_str, e)| { + ( + id_str.clone(), + serde_json::from_str(id_str) + .unwrap_or_else(|_| serde_json::Value::String(id_str.clone())), + e.options_snapshot.clone(), + e.nonce.clone(), + ) + }) + .collect(); + for (id_str, id_val, opts, nonce) in expired { + tracing::warn!( + target: "acp::permission", + "ask timeout for permission id={id_val} — failing closed" + ); + // Transition to Resolved so cancel doesn't drain twice. + if let Some(entry) = self.pending_permissions.get_mut(&id_str) { + entry.state = PermissionEntryState::Resolved; + } + if let Ok(response) = permission_denial_response(&id_val, &opts) { + // Write the denial without the generic observer (avoids duplicate). + // Best-effort; ignore error (we're already timing out). + let write_ok = self.write_ndjson_no_observe(&response).await.is_ok(); + // Emit one authorized acp_write correlated by nonce so the + // desktop can retire the card. Only emitted when the write + // actually reached the pipe — otherwise emit nothing rather + // than claim a response was delivered. + if write_ok { + self.observe_authorized( + "acp_write", + AuthorizationEnvelope { + request_nonce: nonce, + actionable: false, + reason: Some("timed_out".to_string()), + }, + response, + ); + } + } } } @@ -1366,6 +1736,113 @@ impl AcpClient { // read level — the buffer never grows beyond the limit. let read_result = tokio::select! { biased; + // Decision arm — must be FIRST in the biased select! (spec §9) so + // owner decisions are not starved by a continuously-ready stdout. + // Cancel-safe: `mpsc::Receiver::recv` does not lose messages on drop. + Some(decision) = async { + match decision_rx.as_mut() { + Some(rx) => rx.recv().await, + None => None, + } + } => { + // Find the pending entry by nonce match. + let entry_id = self.pending_permissions + .iter() + .find(|(_, e)| { + matches!(e.state, PermissionEntryState::Pending) + && e.nonce == decision.request_nonce + }) + .map(|(k, _)| k.clone()); + + if let Some(id_str) = entry_id { + // Validate the chosen option_id is in the snapshot. + let opt_valid = self.pending_permissions + .get(&id_str) + .map(|e| { + e.options_snapshot.iter().any(|opt| { + opt.get("optionId") + .and_then(|v| v.as_str()) + == Some(decision.option_id.as_str()) + }) + }) + .unwrap_or(false); + + if !opt_valid { + tracing::warn!( + target: "acp::permission", + "permission_decision optionId {:?} not in snapshot for id={id_str} — ignoring", + decision.option_id + ); + } else { + // Transition Pending → Writing. + let (nonce, opts, id_val) = { + let entry = self.pending_permissions.get_mut(&id_str).unwrap(); + entry.state = PermissionEntryState::Writing; + ( + entry.nonce.clone(), + entry.options_snapshot.clone(), + serde_json::from_str::(&id_str) + .unwrap_or_else(|_| serde_json::Value::String(id_str.clone())), + ) + }; + + let response = permission_response_selected(&id_val, &decision.option_id); + // Write bounded by min(30s, remaining hard deadline). + let write_deadline = (Instant::now() + + std::time::Duration::from_secs(30)) + .min(hard_deadline); + let write_result = tokio::time::timeout_at(write_deadline, self.write_ndjson_no_observe(&response)).await; + + match write_result { + Ok(Ok(())) => { + // Transition Writing → Resolved. + if let Some(entry) = self.pending_permissions.get_mut(&id_str) { + entry.state = PermissionEntryState::Resolved; + } + // Emit single authorized acp_write after confirmed write. + self.observe_authorized( + "acp_write", + AuthorizationEnvelope { + request_nonce: nonce.clone(), + actionable: false, + reason: Some("applied".to_string()), + }, + response, + ); + let _ = opts; // used above for validation + tracing::info!( + target: "acp::permission", + "permission id={id_val} answered: optionId={:?}", + decision.option_id + ); + } + Ok(Err(write_err)) => { + // Write failed — poison the process. + tracing::error!( + target: "acp::permission", + "permission write failed for id={id_val}: {write_err} — poisoning process" + ); + self.permission_poisoned = true; + } + Err(_timeout) => { + // Write timed out — poison the process. + tracing::error!( + target: "acp::permission", + "permission write timed out for id={id_val} — poisoning process" + ); + self.permission_poisoned = true; + } + } + } + } else { + tracing::warn!( + target: "acp::permission", + "permission_decision nonce {:?} has no matching pending entry — ignoring", + decision.request_nonce + ); + } + None // loop back; don't set read_result + } read_result = self.reader.next() => Some(read_result), // Steer arm: gated off whenever a steer write is already in // flight so we don't stack two writes against the same @@ -1470,16 +1947,24 @@ impl AcpClient { // would catch this anyway, but firing the deadline arm // here makes the wakeup immediate (no extra reader poll // round-trip when stdout is idle). - if let Some((_, _, ack_tx)) = pending_steer.take() { - let _ = ack_tx.send(crate::pool::SteerAck::PromptCompletedNeutral); - } - if idle_fires_first { - tracing::warn!("idle timeout ({idle_timeout:?}) — no agent activity"); - return Err(AcpError::IdleTimeout(idle_timeout)); + // For a permission-deadline wake, loop back to let the + // expiry block process timed-out entries. + let is_permission_wake = + has_pending_permissions && next_deadline != hard_deadline; + if is_permission_wake { + None // loop back; expiry block will fire } else { - let silence = Instant::now().saturating_duration_since(last_activity_at); - tracing::warn!("hard turn timeout exceeded (silence {silence:?})"); - return Err(AcpError::HardTimeout { silence }); + if let Some((_, _, ack_tx)) = pending_steer.take() { + let _ = ack_tx.send(crate::pool::SteerAck::PromptCompletedNeutral); + } + if idle_fires_first { + tracing::warn!("idle timeout ({idle_timeout:?}) — no agent activity"); + return Err(AcpError::IdleTimeout(idle_timeout)); + } else { + let silence = Instant::now().saturating_duration_since(last_activity_at); + tracing::warn!("hard turn timeout exceeded (silence {silence:?})"); + return Err(AcpError::HardTimeout { silence }); + } } } }; @@ -1538,7 +2023,16 @@ impl AcpClient { continue; } }; - self.observe("acp_read", msg.clone()); + // Suppress the generic `acp_read` for `session/request_permission` + // under the `ask` policy — `handle_permission_request` emits the + // single enveloped frame instead (spec §6 "one frame per request"). + let is_ask_permission_request = + matches!(self.permission_config.policy, PermissionPolicy::Ask) + && msg.get("method").and_then(|v| v.as_str()) + == Some("session/request_permission"); + if !is_ask_permission_request { + self.observe("acp_read", msg.clone()); + } let activity_now = Instant::now(); idle_deadline = activity_now + idle_timeout; @@ -1683,7 +2177,12 @@ impl AcpClient { self.handle_goose_usage_update(&msg); } "session/request_permission" => { - self.handle_permission_request(&msg).await?; + self.handle_permission_request( + &msg, + is_ask_permission_request, + hard_deadline, + ) + .await?; } other => { // If the unknown message has an id, it's a request expecting a reply. @@ -1871,57 +2370,294 @@ impl AcpClient { } } - /// Reject a `session/request_permission` request from the agent. + /// Handle a `session/request_permission` request from the agent. /// - /// Buzz has no human permission prompt in this harness, so selecting - /// `allow_once` would turn any admitted prompt into an implicit approval. - /// Find `reject_once` by kind when the adapter offers it; otherwise use the - /// protocol's cancelled outcome, which is also fail-closed. + /// Dispatches based on the resolved permission policy: + /// - `reject` — deny via `reject_once`/`cancelled` (byte-for-byte old behaviour). + /// - `allow` — auto-select the unique validated `allow_once` option; fail closed. + /// - `ask` — register in the pending map, emit an actionable frame, and return. + /// The read loop's decision arm (added to `select!`) delivers the owner + /// decision. This call is intentionally **non-blocking** for `ask`; + /// the actual response is written asynchronously via the decision arm. /// - /// The request `id` is stored as `serde_json::Value` to support both numeric - /// and string IDs per JSON-RPC 2.0. - async fn handle_permission_request(&mut self, msg: &serde_json::Value) -> Result<(), AcpError> { + /// **Admission preflight (always runs before any policy dispatch):** + /// options nonempty, count ≤ PERMISSION_OPTIONS_MAX, every optionId unique + + /// nonempty, required kind/name fields present, no duplicate live requestId, + /// plaintext size ≤ OBSERVER_MAX_PLAINTEXT_LEN. Fail → immediate denial + emit + /// with `actionable: false`. + /// + /// Under `ask`, the generic pre-dispatch `acp_read` (acp.rs:1697 seam) is + /// **suppressed** for permission requests; this method emits the single + /// post-preflight enveloped frame instead. + /// + /// Returns `Ok(true)` when the caller should suppress the normal `acp_read` emit + /// (i.e. this method already emitted the enveloped frame), `Ok(false)` otherwise. + pub(crate) async fn handle_permission_request( + &mut self, + msg: &serde_json::Value, + // When `true`, caller has NOT yet emitted acp_read for this message — + // this method emits it (enveloped) for permission frames under `ask`. + // When `false` (read_until_response, non-idle path), the caller already + // emitted it; we must not double-emit. + caller_will_emit_read: bool, + // Hard deadline for the current turn. Used to bound per-request ask timeouts. + hard_deadline: tokio::time::Instant, + ) -> Result { // Extract id as a Value — JSON-RPC 2.0 allows both numeric and string IDs. let id = msg .get("id") .cloned() .ok_or_else(|| AcpError::Protocol("permission request missing id".into()))?; - // Store pending permission id so cancel_with_cleanup can respond to it. - self.pending_permission_id = Some(id.clone()); - // Mark as not yet responded — guards against double-response race. - self.permission_responded = false; + let options = match msg["params"]["options"].as_array() { + Some(o) => o.clone(), + None => { + // Missing options — emit non-actionable frame and deny. + let reason = "missing or non-array options field"; + tracing::warn!(target: "acp::permission", "{reason}, id={id}"); + self.emit_permission_read_non_actionable(&id, msg, reason, caller_will_emit_read); + let response = permission_denial_response(&id, &[])?; + self.write_ndjson(&response).await?; + return Ok(true); + } + }; + + // ── Admission preflight ──────────────────────────────────────────────── + let preflight_result = run_admission_preflight( + &id, + &options, + msg, + self.permission_config.policy, + // Check for duplicate live requestId under ask. + if matches!(self.permission_config.policy, PermissionPolicy::Ask) { + let id_str = id.to_string(); + self.pending_permissions.contains_key(&id_str) + } else { + false + }, + if matches!(self.permission_config.policy, PermissionPolicy::Ask) { + self.pending_permissions.len() >= PERMISSION_MAP_CAP + } else { + false + }, + &self.observer_context, + self.observer_agent_index, + ); - let options = msg["params"]["options"] - .as_array() - .ok_or_else(|| AcpError::Protocol("permission request missing options".into()))?; + if let Err(reason) = preflight_result { + tracing::warn!(target: "acp::permission", "preflight failed: {reason}, id={id}"); + self.emit_permission_read_non_actionable(&id, msg, &reason, caller_will_emit_read); + let response = permission_denial_response(&id, &options)?; + self.write_ndjson(&response).await?; + return Ok(true); + } + // ── Preflight passed ─────────────────────────────────────────────────── tracing::debug!( target: "acp::permission", - "session/request_permission id={id}, {} options", - options.len() + "session/request_permission id={id}, {} options, policy={}", + options.len(), + self.permission_config.policy ); - let response = permission_denial_response(&id, options)?; + match self.permission_config.policy { + PermissionPolicy::Reject => { + // Byte-for-byte old behaviour: deny, track pending id for cancel. + self.pending_permission_id = Some(id.clone()); + self.permission_responded = false; + + // For reject, the caller already emitted acp_read unconditionally; + // emit a non-actionable authorization envelope alongside. + let nonce = new_permission_nonce(); + self.emit_permission_read_with_nonce( + &id, + msg, + &nonce, + false, + Some("policy=reject"), + caller_will_emit_read, + ); - // Write the response first, then mark as responded. - // - // Previous ordering (flag-before-write) was intended to guard against a - // double-response if a timeout fires between write and flag-set. However, - // the deadlock risk is worse: if write_ndjson fails (e.g. WriteTimeout), - // the flag would be true but no response was actually sent. Then - // cancel_with_cleanup would see permission_responded=true, skip sending - // the cancelled outcome, and the agent would hang waiting for a reply - // that never arrives — a guaranteed deadlock. - // - // The correct fix: set the flag AFTER a successful write. The double- - // response window (between write completion and flag-set) is negligibly - // small and bounded by a single memory store; the deadlock window was - // unbounded. - self.write_ndjson(&response).await?; - self.permission_responded = true; - self.pending_permission_id = None; - Ok(()) + let response = permission_denial_response(&id, &options)?; + self.write_ndjson(&response).await?; + self.permission_responded = true; + self.pending_permission_id = None; + Ok(true) + } + PermissionPolicy::Allow => { + // Auto-select the unique allow_once option; fail closed otherwise. + self.pending_permission_id = Some(id.clone()); + self.permission_responded = false; + + match select_allow_once(&options) { + Ok(option_id) => { + tracing::info!( + target: "acp::permission", + "allow: selecting allow_once optionId={option_id:?} for id={id}" + ); + let nonce = new_permission_nonce(); + // Emit enveloped acp_read (non-actionable: auto-approved). + self.emit_permission_read_with_nonce( + &id, + msg, + &nonce, + false, + Some("policy=allow; auto-approved"), + caller_will_emit_read, + ); + let response = permission_response_selected(&id, &option_id); + self.write_ndjson(&response).await?; + // Emit enveloped acp_write after confirmed write. + self.observe_authorized( + "acp_write", + AuthorizationEnvelope { + request_nonce: nonce, + actionable: false, + reason: Some("auto-approved by policy=allow".to_string()), + }, + response, + ); + self.permission_responded = true; + self.pending_permission_id = None; + } + Err(reason) => { + // Fail closed. + tracing::warn!( + target: "acp::permission", + "allow: fail closed — {reason}, id={id}" + ); + let nonce = new_permission_nonce(); + self.emit_permission_read_with_nonce( + &id, + msg, + &nonce, + false, + Some(&format!("policy=allow; fail closed: {reason}")), + caller_will_emit_read, + ); + let response = permission_denial_response(&id, &options)?; + self.write_ndjson(&response).await?; + self.permission_responded = true; + self.pending_permission_id = None; + } + } + Ok(true) + } + PermissionPolicy::Ask => { + // Availability gate (spec §10): `ask` requires both an active observer + // and a known owner. Without either, downgrade to `reject` with a loud + // warning — never sideways to `allow`. + let observer_active = self.observer.is_some(); + if !observer_active || !self.owner_pubkey_known { + tracing::warn!( + target: "acp::permission", + "ask policy unavailable (observer={}, owner_known={}) — downgrading to reject for id={id}", + observer_active, + self.owner_pubkey_known + ); + // Fall through to the Reject arm's logic. + self.pending_permission_id = Some(id.clone()); + self.permission_responded = false; + let nonce = new_permission_nonce(); + self.emit_permission_read_with_nonce( + &id, + msg, + &nonce, + false, + Some("policy=ask unavailable (no observer/owner); downgraded to reject"), + caller_will_emit_read, + ); + let response = permission_denial_response(&id, &options)?; + self.write_ndjson(&response).await?; + self.permission_responded = true; + self.pending_permission_id = None; + return Ok(true); + } + + // Register in the pending map and emit the actionable frame. + // The read loop's decision arm delivers the response asynchronously. + let id_str = id.to_string(); + let nonce = new_permission_nonce(); + + // Emit the single enveloped acp_read — suppresses the caller's + // generic emit via the Ok(true) return. + self.observe_authorized( + "acp_read", + AuthorizationEnvelope { + request_nonce: nonce.clone(), + actionable: true, + reason: None, + }, + msg.clone(), + ); + + // Per-request deadline: min(now + 300s, turn hard deadline). + let ask_deadline = tokio::time::Instant::now() + + std::time::Duration::from_secs(PERMISSION_ASK_TIMEOUT_SECS); + let entry_deadline = ask_deadline.min(hard_deadline); + + self.pending_permissions.insert( + id_str, + PermissionEntry { + nonce, + options_snapshot: options.clone(), + state: PermissionEntryState::Pending, + deadline: entry_deadline, + }, + ); + + // Do NOT set pending_permission_id for ask — the map is the + // sole source of truth. The legacy single-id slot is only used + // by reject/allow (synchronous paths). + Ok(true) + } + } + } + + /// Emit a non-actionable `acp_read` authorization frame for a permission request. + fn emit_permission_read_non_actionable( + &self, + id: &serde_json::Value, + msg: &serde_json::Value, + reason: &str, + _caller_will_emit_read: bool, + ) { + let nonce = new_permission_nonce(); + self.observe_authorized( + "acp_read", + AuthorizationEnvelope { + request_nonce: nonce, + actionable: false, + reason: Some(reason.to_string()), + }, + msg.clone(), + ); + tracing::debug!(target: "acp::permission", "non-actionable permission read id={id}"); + } + + /// Emit an `acp_read` with an authorization envelope. + /// + /// When `caller_will_emit_read` is `false` the caller already emitted the + /// raw `acp_read`; we emit only the enveloped version. When `true` we emit + /// the enveloped version (the caller suppresses its normal emit). + fn emit_permission_read_with_nonce( + &self, + _id: &serde_json::Value, + msg: &serde_json::Value, + nonce: &str, + actionable: bool, + reason: Option<&str>, + _caller_will_emit_read: bool, + ) { + self.observe_authorized( + "acp_read", + AuthorizationEnvelope { + request_nonce: nonce.to_string(), + actionable, + reason: reason.map(str::to_string), + }, + msg.clone(), + ); } /// Parse `stopReason` from a `session/prompt` result value. @@ -2040,9 +2776,16 @@ fn permission_denial_response( return Ok(permission_response_cancelled(id)); }; - let option_id = opt["optionId"] - .as_str() - .ok_or_else(|| AcpError::Protocol("reject_once option missing optionId".into()))?; + let Some(option_id) = opt["optionId"].as_str().filter(|s| !s.is_empty()) else { + // reject_once found but optionId is missing or empty — malformed request; + // fall back to `cancelled` rather than returning a Protocol error so the + // adapter still receives a valid JSON-RPC response. + tracing::warn!( + target: "acp::permission", + "reject_once option has missing or empty optionId for id={id}, cancelling" + ); + return Ok(permission_response_cancelled(id)); + }; tracing::info!( target: "acp::permission", "rejecting permission id={id} with reject_once optionId={option_id:?}" @@ -2050,6 +2793,177 @@ fn permission_denial_response( Ok(permission_response_selected(id, option_id)) } +/// Generate a cryptographically random, URL-safe nonce string. +/// +/// Used as the `requestNonce` in [`crate::observer::AuthorizationEnvelope`]. +/// The nonce is single-use and bound to a specific permission request. +fn new_permission_nonce() -> String { + uuid::Uuid::new_v4().to_string() +} + +/// Select the unique `allow_once` option from a permission request's option list. +/// +/// Returns `Ok(option_id)` when there is exactly one option with `kind = +/// "allow_once"` and a non-empty `optionId`. Returns `Err(reason)` (fail +/// closed) when: +/// - zero `allow_once` options are present, +/// - multiple `allow_once` options are present (ambiguous), +/// - the matching option has a missing or empty `optionId`. +/// +/// `allow_always` options are deliberately not selected — they would grant +/// indefinite access without a per-request human decision. +fn select_allow_once(options: &[serde_json::Value]) -> Result { + let candidates: Vec<&serde_json::Value> = options + .iter() + .filter(|opt| { + opt.get("kind") + .and_then(|k| k.as_str()) + .map(|k| k == "allow_once") + .unwrap_or(false) + }) + .collect(); + + match candidates.len() { + 0 => Err("no allow_once option found".to_string()), + 2.. => Err(format!( + "multiple allow_once options found ({}); ambiguous", + candidates.len() + )), + 1 => { + let opt = candidates[0]; + let option_id = opt + .get("optionId") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .ok_or_else(|| "allow_once option has missing or empty optionId".to_string())?; + Ok(option_id.to_string()) + } + } +} + +/// Validate a `session/request_permission` request before it touches the +/// pending map or policy dispatch. +/// +/// Returns `Ok(())` on a clean request; `Err(reason)` on the first violation. +/// +/// Checks (in order): +/// 1. `options` nonempty. +/// 2. `options` count ≤ `PERMISSION_OPTIONS_MAX`. +/// 3. Every `optionId` is present and non-empty. +/// 4. Every `optionId` is unique across the request. +/// 5. Every option has a non-empty `kind` and `name`. +/// 6. Duplicate live `requestId` (only relevant under `ask`, caller passes flag). +/// 7. Permission map at capacity (only relevant under `ask`, caller passes flag). +/// 8. Full serialised `ObserverEvent` (raw payload + all envelope fields + real +/// context) fits within `OBSERVER_MAX_PLAINTEXT_LEN` — no leaf surgery on frames. +fn run_admission_preflight( + _id: &serde_json::Value, + options: &[serde_json::Value], + msg: &serde_json::Value, + _policy: PermissionPolicy, + is_duplicate_id: bool, + is_map_at_cap: bool, + observer_context: &ObserverContext, + agent_index: Option, +) -> Result<(), String> { + // 1. options nonempty + if options.is_empty() { + return Err("options array is empty".to_string()); + } + + // 2. count ≤ PERMISSION_OPTIONS_MAX + if options.len() > PERMISSION_OPTIONS_MAX { + return Err(format!( + "too many options: {} > {}", + options.len(), + PERMISSION_OPTIONS_MAX + )); + } + + // 3 & 4. optionId present, non-empty, unique + let mut seen_ids = std::collections::HashSet::new(); + for opt in options { + let option_id = opt + .get("optionId") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .ok_or_else(|| "option has missing or empty optionId".to_string())?; + if !seen_ids.insert(option_id) { + return Err(format!("duplicate optionId: {option_id:?}")); + } + } + + // 5. required kind and name fields + for opt in options { + if opt + .get("kind") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .is_none() + { + return Err("option has missing or empty kind".to_string()); + } + if opt + .get("name") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .is_none() + { + return Err("option has missing or empty name".to_string()); + } + } + + // 6. duplicate live requestId (ask only — caller computes flag) + if is_duplicate_id { + return Err("duplicate live requestId".to_string()); + } + + // 7. map at capacity (ask only — caller computes flag) + if is_map_at_cap { + return Err(format!( + "pending permission map at capacity ({})", + PERMISSION_MAP_CAP + )); + } + + // 8. Full annotated `ObserverEvent` fits within `OBSERVER_MAX_PLAINTEXT_LEN`. + // + // Construct the exact production `ObserverEvent` with the real observer context + // and a representative nonce. Serialise it and reject if over cap. This is the + // same construction path the observer uses at emit time, so any payload that + // passes here is guaranteed to fit in the final frame — no leaf surgery needed. + // + // A UUID nonce is used for sizing; the actual nonce is generated after the + // preflight passes, but all nonces are the same UUID length. + let candidate_event = ObserverEvent { + seq: u64::MAX, // worst-case seq (19 digits) + timestamp: "2026-01-01T00:00:00.000000000+00:00".to_string(), // max RFC3339 len + kind: "acp_read".to_string(), + agent_index, + channel_id: observer_context.channel_id.clone(), + session_id: observer_context.session_id.clone(), + turn_id: observer_context.turn_id.clone(), + started_at: observer_context.started_at.clone(), + authorization: Some(AuthorizationEnvelope { + // UUID nonce — all production nonces are this length. + request_nonce: "00000000-0000-0000-0000-000000000000".to_string(), + actionable: true, + reason: None, + }), + payload: msg.clone(), + }; + let annotated_len = serde_json::to_string(&candidate_event) + .map(|s| s.len()) + .unwrap_or(usize::MAX); + if annotated_len > OBSERVER_MAX_PLAINTEXT_LEN { + return Err(format!( + "permission request payload too large: annotated size {annotated_len} > {OBSERVER_MAX_PLAINTEXT_LEN}" + )); + } + + Ok(()) +} + /// Full `session/new` response — session ID plus the raw JSON result. /// /// Callers use the extractor helpers to pull model info from `raw`. @@ -2259,6 +3173,7 @@ fn configure_no_window(cmd: &mut tokio::process::Command) { #[cfg(test)] mod tests { use super::*; + use crate::config::ModeSource; #[test] fn stop_reason_parses_all_known_values() { @@ -2369,17 +3284,21 @@ mod tests { assert_eq!(outcome(&response), Some("cancelled")); } - /// A `reject_once` option missing its `optionId` is a protocol violation. - /// Erroring propagates to the caller, which tears the turn down — still no - /// approval is ever sent. + /// A `reject_once` option missing its `optionId` falls back to a `cancelled` + /// response rather than propagating a Protocol error. This ensures the adapter + /// always receives a valid JSON-RPC response, even for malformed requests. #[test] - fn reject_once_without_option_id_is_a_protocol_error() { + fn reject_once_without_option_id_falls_back_to_cancelled() { let options = options(r#"[{"name": "Reject", "kind": "reject_once"}]"#); - let err = permission_denial_response(&serde_json::json!(1), &options) - .expect_err("missing optionId must error"); + let response = permission_denial_response(&serde_json::json!(1), &options) + .expect("malformed reject_once must not error"); - assert!(matches!(err, AcpError::Protocol(_)), "got {err:?}"); + assert_eq!( + response["result"]["outcome"]["outcome"].as_str(), + Some("cancelled"), + "malformed reject_once must produce cancelled, got: {response}" + ); } #[test] @@ -4648,4 +5567,1330 @@ mod tests { "error must mention sandbox_workspace_write" ); } + + // ══════════════════════════════════════════════════════════════════════════ + // ── Permission policy: pinned tests (#4938) ─────────────────────────────── + // ══════════════════════════════════════════════════════════════════════════ + // + // Tests are grouped by the pinned requirement they cover, labelled as + // "Pinned §N" matching the spec's numbered list. + // + // These tests use: + // • `spawn_inert_client()` (cat) for pure unit coverage of `handle_permission_request`. + // • `spawn_script(s)` for end-to-end coverage of `read_until_response_with_idle_timeout`. + // • `AcpClient::set_permission_config` / `set_owner_pubkey_known` helpers. + // + // "observer" is left None for tests that only care about deny/allow path; + // an in-process observer is installed for tests that verify acp_write events. + + // ── Helpers ─────────────────────────────────────────────────────────────── + + /// Build a minimal `session/request_permission` JSON-RPC message. + fn perm_request(id: u64, options: &[(&str, &str, &str)]) -> serde_json::Value { + let opts: Vec = options + .iter() + .map(|(opt_id, kind, name)| { + serde_json::json!({"optionId": opt_id, "kind": kind, "name": name}) + }) + .collect(); + serde_json::json!({ + "jsonrpc": "2.0", + "id": id, + "method": "session/request_permission", + "params": { + "sessionId": "sess-test", + "options": opts, + } + }) + } + + /// Canonical 3-option set used in most tests. + fn default_opts() -> &'static [(&'static str, &'static str, &'static str)] { + &[ + ("opt-allow", "allow_once", "Allow once"), + ("opt-reject", "reject_once", "Reject once"), + ("opt-always", "allow_always", "Always allow"), + ] + } + + /// Set policy=allow on a client and mark owner known. + fn set_policy(client: &mut AcpClient, policy: PermissionPolicy) { + let config = ResolvedPermissionConfig::resolve(policy, None).expect("valid policy"); + client.set_permission_config(config); + client.set_owner_pubkey_known(true); + } + + // ── Pinned §2: allow selector — unique/zero/multiple/malformed ──────────── + + #[test] + fn allow_selector_picks_unique_allow_once() { + // Unique allow_once → Ok with that optionId. + let opts = serde_json::from_str::>( + r#"[{"optionId":"opt-a","kind":"allow_once","name":"Allow"}, + {"optionId":"opt-r","kind":"reject_once","name":"Reject"}]"#, + ) + .unwrap(); + assert_eq!(select_allow_once(&opts), Ok("opt-a".to_string())); + } + + #[test] + fn allow_selector_fails_closed_on_zero_allow_once() { + // No allow_once options → fail closed. + let opts = serde_json::from_str::>( + r#"[{"optionId":"opt-r","kind":"reject_once","name":"Reject"}]"#, + ) + .unwrap(); + assert!(select_allow_once(&opts).is_err()); + } + + #[test] + fn allow_selector_fails_closed_on_multiple_allow_once() { + // Two allow_once candidates → ambiguous, fail closed. + let opts = serde_json::from_str::>( + r#"[{"optionId":"opt-a1","kind":"allow_once","name":"A1"}, + {"optionId":"opt-a2","kind":"allow_once","name":"A2"}]"#, + ) + .unwrap(); + assert!(select_allow_once(&opts).is_err()); + } + + #[test] + fn allow_selector_fails_closed_on_missing_option_id() { + // allow_once present but optionId absent → malformed, fail closed. + let opts = serde_json::from_str::>( + r#"[{"kind":"allow_once","name":"Allow"}]"#, + ) + .unwrap(); + assert!(select_allow_once(&opts).is_err()); + } + + #[test] + fn allow_selector_never_selects_allow_always() { + // allow_always must NOT be selected even when it is the only option + // with an "allow" kind — indefinite access without per-request approval. + let opts = serde_json::from_str::>( + r#"[{"optionId":"opt-aa","kind":"allow_always","name":"Always"}]"#, + ) + .unwrap(); + assert!( + select_allow_once(&opts).is_err(), + "allow_always must never be auto-selected" + ); + } + + // ── Pinned §3: duplicate option IDs ────────────────────────────────────── + + #[test] + fn admission_preflight_rejects_duplicate_option_ids() { + let id = serde_json::json!(1); + let msg = perm_request( + 1, + &[("dup", "allow_once", "A"), ("dup", "reject_once", "R")], + ); + let opts = msg["params"]["options"].as_array().unwrap().clone(); + let result = run_admission_preflight(&id, &opts, &msg, PermissionPolicy::Ask, false, false, &ObserverContext::default(), None); + assert!(result.is_err(), "duplicate optionId must fail preflight"); + let reason = result.unwrap_err(); + assert!( + reason.contains("duplicate optionId"), + "reason must name the check, got: {reason}" + ); + } + + // ── Pinned §2: duplicate request ID ────────────────────────────────────── + + #[tokio::test] + async fn handle_permission_request_denies_duplicate_live_request_id() { + // Under ask policy, a second request with the same id while the first + // is still pending must be denied immediately without disturbing the original. + let mut client = spawn_inert_client().await; + set_policy(&mut client, PermissionPolicy::Ask); + // Simulate an already-registered pending entry with the same id. + client.pending_permissions.insert( + "1".to_string(), + PermissionEntry { + nonce: "nonce-abc".to_string(), + options_snapshot: vec![], + state: PermissionEntryState::Pending, + deadline: tokio::time::Instant::now() + std::time::Duration::from_secs(300), + }, + ); + let msg = perm_request(1, default_opts()); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + let result = client + .handle_permission_request(&msg, true, hard_deadline) + .await; + // Must succeed (Ok) — denial was written and the call itself doesn't error. + assert!( + result.is_ok(), + "duplicate-id must not propagate as Err, got {result:?}" + ); + // The original entry must still be in the map, untouched. + assert!( + client.pending_permissions.contains_key("1"), + "original pending entry must survive the duplicate-id rejection" + ); + // Only one entry should exist (the duplicate was denied, not registered). + assert_eq!( + client.pending_permissions.len(), + 1, + "no new entry should be added for the duplicate id" + ); + } + + // ── Pinned §4: oversize subject → plaintext cap exceeded ───────────────── + + #[test] + fn admission_preflight_rejects_oversize_msg_exceeding_plaintext_cap() { + // Construct a message large enough to exceed OBSERVER_MAX_PLAINTEXT_LEN. + // We embed the large payload directly in the msg so that + // `serde_json::to_string(msg).len() > OBSERVER_MAX_PLAINTEXT_LEN`. + let id = serde_json::json!(42); + let oversize_subject = "x".repeat(OBSERVER_MAX_PLAINTEXT_LEN + 1); + let msg = serde_json::json!({ + "jsonrpc": "2.0", + "id": 42, + "method": "session/request_permission", + "params": { + "sessionId": "sess", + "subject": oversize_subject, + "options": [{"optionId":"opt","kind":"allow_once","name":"A"}] + } + }); + let opts = vec![serde_json::json!({"optionId":"opt","kind":"allow_once","name":"A"})]; + let result = run_admission_preflight(&id, &opts, &msg, PermissionPolicy::Ask, false, false, &ObserverContext::default(), None); + assert!(result.is_err(), "oversize msg must fail preflight"); + let reason = result.unwrap_err(); + assert!( + reason.contains("too large") || reason.contains("payload"), + "reason should mention payload size, got: {reason}" + ); + } + + #[test] + fn admission_preflight_rejects_payload_overflowing_after_full_event_construction() { + // Construct a context matching production (UUID-sized IDs) and compute the + // maximum msg payload that fits within OBSERVER_MAX_PLAINTEXT_LEN when + // serialised as the actual ObserverEvent. Then submit a payload one byte + // larger and verify the preflight rejects it. + // + // This exercises the production code path: the check constructs the + // exact ObserverEvent with real context fields, not an estimate. + use crate::observer::ObserverContext; + + let ctx = ObserverContext { + channel_id: Some("00000000-0000-0000-0000-000000000000".to_string()), + session_id: Some("sess-00000000-0000-0000-0000-000000000000".to_string()), + turn_id: Some("00000000-0000-0000-0000-000000000000".to_string()), + started_at: Some("2026-01-01T00:00:00.000000000+00:00".to_string()), + }; + + // Binary-search for the exact max subject length that still fits. + // We wrap it in a minimal msg structure to simulate a real request. + let template = |subject: &str| { + serde_json::json!({ + "jsonrpc": "2.0", + "id": 42, + "method": "session/request_permission", + "params": { + "sessionId": "sess", + "subject": subject, + "options": [{"optionId":"opt","kind":"allow_once","name":"A"}] + } + }) + }; + let opts = vec![serde_json::json!({"optionId":"opt","kind":"allow_once","name":"A"})]; + let id = serde_json::json!(42); + + // Build the ObserverEvent exactly as the preflight does to find where the + // boundary is — then make a msg one byte over that boundary. + let make_candidate = |msg: &serde_json::Value| { + ObserverEvent { + seq: u64::MAX, + timestamp: "2026-01-01T00:00:00.000000000+00:00".to_string(), + kind: "acp_read".to_string(), + agent_index: None, + channel_id: ctx.channel_id.clone(), + session_id: ctx.session_id.clone(), + turn_id: ctx.turn_id.clone(), + started_at: ctx.started_at.clone(), + authorization: Some(AuthorizationEnvelope { + request_nonce: "00000000-0000-0000-0000-000000000000".to_string(), + actionable: true, + reason: None, + }), + payload: msg.clone(), + } + }; + + // Find a subject length that overflows after event wrapping. + // Start with a large subject known to overflow (cap worth of padding). + let overflow_subject = "z".repeat(OBSERVER_MAX_PLAINTEXT_LEN); + let overflow_msg = template(&overflow_subject); + let overflow_event_len = serde_json::to_string(&make_candidate(&overflow_msg)) + .unwrap() + .len(); + assert!( + overflow_event_len > OBSERVER_MAX_PLAINTEXT_LEN, + "test setup: overflow_event_len ({overflow_event_len}) must exceed cap" + ); + + // The preflight must reject this payload. + let result = run_admission_preflight( + &id, + &opts, + &overflow_msg, + PermissionPolicy::Ask, + false, + false, + &ctx, + None, + ); + assert!( + result.is_err(), + "payload overflowing after event construction must fail preflight (event_len={overflow_event_len})" + ); + let reason = result.unwrap_err(); + assert!( + reason.contains("too large") || reason.contains("payload"), + "reason should mention payload size, got: {reason}" + ); + + // Sanity-check: an empty subject (tiny msg) must pass the preflight. + let tiny_msg = template(""); + let tiny_event_len = serde_json::to_string(&make_candidate(&tiny_msg)) + .unwrap() + .len(); + assert!( + tiny_event_len <= OBSERVER_MAX_PLAINTEXT_LEN, + "test setup: tiny_event_len ({tiny_event_len}) must be within cap" + ); + let ok_result = run_admission_preflight( + &id, + &opts, + &tiny_msg, + PermissionPolicy::Ask, + false, + false, + &ctx, + None, + ); + assert!( + ok_result.is_ok(), + "small payload must pass preflight, got: {ok_result:?}" + ); + } + + #[test] + fn denial_response_with_malformed_reject_once_falls_back_to_cancelled() { + // A reject_once option with a missing optionId must produce a `cancelled` + // response, not a Protocol error — the adapter must always receive a valid + // JSON-RPC response. + let id = serde_json::json!(7); + let opts = vec![ + serde_json::json!({"kind": "reject_once", "name": "Reject"}), // no optionId + ]; + let response = permission_denial_response(&id, &opts) + .expect("malformed reject_once must not return Err"); + // The response must be a cancelled frame (no optionId in result.outcome). + let outcome = &response["result"]["outcome"]; + assert_eq!( + outcome["outcome"].as_str(), + Some("cancelled"), + "malformed reject_once must produce cancelled response, got: {response}" + ); + } + + // ── Pinned §5: map overflow ─────────────────────────────────────────────── + + #[tokio::test] + async fn handle_permission_request_denies_when_map_at_capacity() { + let mut client = spawn_inert_client().await; + set_policy(&mut client, PermissionPolicy::Ask); + + // Fill the map to PERMISSION_MAP_CAP. + for i in 0..PERMISSION_MAP_CAP { + client.pending_permissions.insert( + format!("{i}"), + PermissionEntry { + nonce: format!("nonce-{i}"), + options_snapshot: vec![], + state: PermissionEntryState::Pending, + deadline: tokio::time::Instant::now() + std::time::Duration::from_secs(300), + }, + ); + } + assert_eq!(client.pending_permissions.len(), PERMISSION_MAP_CAP); + + // One more request with a new id → must be denied. + let msg = perm_request(99, default_opts()); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + let result = client + .handle_permission_request(&msg, true, hard_deadline) + .await; + assert!( + result.is_ok(), + "map-at-cap must not propagate Err, got {result:?}" + ); + // Map must not have grown. + assert_eq!( + client.pending_permissions.len(), + PERMISSION_MAP_CAP, + "map must not grow beyond capacity after denial" + ); + } + + // ── Pinned §7: mode matrix — unset + every explicit mode × 3 policies ──── + + #[test] + fn resolved_permission_config_reject_unset_derives_dont_ask() { + let cfg = ResolvedPermissionConfig::resolve(PermissionPolicy::Reject, None).unwrap(); + assert_eq!(cfg.effective_mode, PermissionMode::DontAsk); + assert_eq!(cfg.mode_source, ModeSource::Derived); + assert!(cfg.transmit_mode, "transmit_mode must always be true"); + } + + #[test] + fn resolved_permission_config_ask_unset_derives_default() { + let cfg = ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(); + assert_eq!(cfg.effective_mode, PermissionMode::Default); + assert_eq!(cfg.mode_source, ModeSource::Derived); + } + + #[test] + fn resolved_permission_config_allow_unset_derives_default_not_dont_ask() { + // allow + unset → default (NOT dontAsk — dontAsk self-denies before Buzz can answer) + let cfg = ResolvedPermissionConfig::resolve(PermissionPolicy::Allow, None).unwrap(); + assert_eq!(cfg.effective_mode, PermissionMode::Default); + assert!( + cfg.effective_mode != PermissionMode::DontAsk, + "allow policy must NOT derive dontAsk" + ); + } + + #[test] + fn resolved_permission_config_reject_plus_explicit_dont_ask_is_ok() { + // reject + dontAsk explicit is valid: both say "deny". + let cfg = ResolvedPermissionConfig::resolve( + PermissionPolicy::Reject, + Some(PermissionMode::DontAsk), + ) + .unwrap(); + assert_eq!(cfg.effective_mode, PermissionMode::DontAsk); + assert_eq!(cfg.mode_source, ModeSource::Explicit); + } + + #[test] + fn resolved_permission_config_ask_plus_explicit_dont_ask_is_startup_error() { + let result = + ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, Some(PermissionMode::DontAsk)); + assert!(result.is_err(), "ask + dontAsk must be a startup error"); + let msg = format!("{}", result.unwrap_err()); + assert!( + msg.contains("dontAsk"), + "error must mention dontAsk, got: {msg}" + ); + } + + #[test] + fn resolved_permission_config_allow_plus_explicit_dont_ask_is_startup_error() { + let result = ResolvedPermissionConfig::resolve( + PermissionPolicy::Allow, + Some(PermissionMode::DontAsk), + ); + assert!(result.is_err(), "allow + dontAsk must be a startup error"); + } + + #[test] + fn resolved_permission_config_ask_plus_explicit_accept_edits_is_ok() { + let cfg = ResolvedPermissionConfig::resolve( + PermissionPolicy::Ask, + Some(PermissionMode::AcceptEdits), + ) + .unwrap(); + assert_eq!(cfg.effective_mode, PermissionMode::AcceptEdits); + assert_eq!(cfg.mode_source, ModeSource::Explicit); + } + + #[test] + fn resolved_permission_config_allow_plus_explicit_plan_is_ok() { + let cfg = + ResolvedPermissionConfig::resolve(PermissionPolicy::Allow, Some(PermissionMode::Plan)) + .unwrap(); + assert_eq!(cfg.effective_mode, PermissionMode::Plan); + assert_eq!(cfg.mode_source, ModeSource::Explicit); + } + + #[test] + fn resolved_permission_config_transmit_mode_always_true() { + // transmit_mode is always true regardless of policy/mode combination. + for policy in [ + PermissionPolicy::Reject, + PermissionPolicy::Ask, + PermissionPolicy::Allow, + ] { + let cfg = ResolvedPermissionConfig::resolve(policy, None).unwrap(); + assert!(cfg.transmit_mode, "transmit_mode must be true for {policy}"); + } + } + + // ── Pinned §10: ask availability gate — no observer → downgrade to reject ─ + + #[tokio::test] + async fn ask_without_observer_downgrades_to_reject() { + // ask policy but no observer installed → must downgrade to reject, + // never sideways to allow. + let mut client = spawn_inert_client().await; + let config = ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(); + client.set_permission_config(config); + client.set_owner_pubkey_known(true); + // No observer installed (default). + + let msg = perm_request(1, default_opts()); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + let result = client + .handle_permission_request(&msg, true, hard_deadline) + .await; + // Denial was written — Ok(true) means caller should suppress generic emit. + assert!( + result.is_ok(), + "ask downgrade to reject must not propagate Err" + ); + // Confirm nothing was left pending in the map — it was denied synchronously. + assert!( + client.pending_permissions.is_empty(), + "downgraded-to-reject must not leave a pending entry" + ); + } + + #[tokio::test] + async fn ask_without_owner_known_downgrades_to_reject() { + // ask policy with observer but unknown owner → downgrade to reject. + let mut client = spawn_inert_client().await; + let config = ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(); + client.set_permission_config(config); + client.set_owner_pubkey_known(false); // explicitly unknown + + let msg = perm_request(2, default_opts()); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + let result = client + .handle_permission_request(&msg, true, hard_deadline) + .await; + assert!(result.is_ok()); + assert!(client.pending_permissions.is_empty()); + } + + // ── Pinned §1: ask path success — decision arrives → response written ───── + // + // This test verifies the biased select! decision arm: + // 1. permission request emitted on stdout + // 2. decision injected via permission_decision_tx + // 3. read loop writes the permission response + // 4. loop continues and the final id=999 response is matched → Ok + + #[tokio::test] + async fn ask_decision_consumed_writes_response_and_continues() { + // Setup: ask policy, observer + owner active, permission_decision channel installed. + // A Pending entry is pre-planted with a known nonce so we can deliver a matching + // decision without needing access to nonce generation inside the loop. + // The script immediately emits the terminal id=999 response (simulating the + // adapter continuing after the permission response was written to its stdin). + let script = r#"echo '{"jsonrpc":"2.0","id":999,"result":{"done":true}}'"#; + let mut client = spawn_script(script).await; + + let config = ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(); + client.set_permission_config(config); + client.set_owner_pubkey_known(true); + let obs = crate::observer::ObserverHandle::in_process(); + client.set_observer(Some(obs.clone()), 0); + + let (perm_tx, perm_rx) = tokio::sync::mpsc::channel::(8); + client.install_permission_decision_rx(perm_rx); + + // Plant a Pending entry with a known nonce. + let known_nonce = "test-nonce-loop-success".to_string(); + let req_id_str = "42".to_string(); + client.pending_permissions.insert( + req_id_str.clone(), + PermissionEntry { + nonce: known_nonce.clone(), + options_snapshot: vec![ + serde_json::json!({"optionId":"opt-allow","kind":"allow_once","name":"Allow"}), + ], + state: PermissionEntryState::Pending, + deadline: tokio::time::Instant::now() + std::time::Duration::from_secs(300), + }, + ); + + // Deliver a matching decision (by nonce) with a valid optionId. + // The decision is already in the channel before the loop starts; the biased + // select! arm reads it on the first iteration. + perm_tx + .send(PermissionDecision { + request_nonce: known_nonce, + option_id: "opt-allow".to_string(), + }) + .await + .unwrap(); + + // Drive the loop. It should: (1) find the pre-delivered decision, write the + // permission response, transition entry → Resolved; (2) continue and read the + // id=999 terminal response from the script. + let idle = std::time::Duration::from_secs(5); + let max_dur = std::time::Duration::from_secs(10); + let hard_deadline = tokio::time::Instant::now() + max_dur; + let result = client + .read_until_response_with_idle_timeout( + "sess-ask-success", + 999, + idle, + hard_deadline, + max_dur, + ) + .await; + + assert!( + result.is_ok(), + "loop must succeed after decision is consumed, got: {result:?}" + ); + + // The entry must have been transitioned to Resolved (decision was applied). + let entry = client.pending_permissions.get(&req_id_str); + match entry { + Some(e) => assert!( + matches!(e.state, PermissionEntryState::Resolved), + "entry must be Resolved after decision applied, got: {:?}", + e.state + ), + None => { + // Entry may have been drained at turn end — also acceptable. + } + } + } + + // ── Production-path tests: real loop emits request, captures nonce ────── + + /// Full end-to-end production path test for the `ask` decision flow: + /// + /// 1. Script emits a real `session/request_permission` on stdout. + /// 2. The read loop processes it via `handle_permission_request()` — + /// no state is pre-planted. + /// 3. The nonce is captured from the observer. + /// 4. A valid decision is sent through the decision channel. + /// 5. The loop writes the permission response to the script's stdin. + /// 6. The script reads the response and emits the terminal id=999 reply. + /// 7. The loop returns `Ok` — the wire flow completes end-to-end. + #[tokio::test] + async fn ask_production_path_emits_request_captures_nonce_and_delivers_decision() { + // Script: emit permission request, wait for any stdin line (the harness's + // response), then emit the terminal session/prompt response. + let perm_req = r#"{"jsonrpc":"2.0","id":42,"method":"session/request_permission","params":{"sessionId":"sess","requestId":"req-prod","subject":"read a file","options":[{"optionId":"opt-allow","kind":"allow_once","name":"Allow"},{"optionId":"opt-deny","kind":"reject_once","name":"Deny"}]}}"#; + let terminal = r#"{"jsonrpc":"2.0","id":999,"result":{"stopReason":"end_turn"}}"#; + // Print the permission request, wait for one line of stdin (the harness's + // response), then print the terminal response. + let script = format!( + r#"printf '{perm_req}\n'; read -r _resp; printf '{terminal}\n'"#, + perm_req = perm_req, + terminal = terminal, + ); + + let mut client = spawn_script(&script).await; + let config = ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(); + client.set_permission_config(config); + client.set_owner_pubkey_known(true); + + // Subscribe to the observer BEFORE starting the loop so we capture all events. + let obs = crate::observer::ObserverHandle::in_process(); + let mut obs_rx = obs.subscribe(); + client.set_observer(Some(obs.clone()), 0); + + let (perm_tx, perm_rx) = tokio::sync::mpsc::channel::(8); + client.install_permission_decision_rx(perm_rx); + + // Spawn a task that waits for the observer to emit the actionable acp_read + // (the permission request), then delivers a matching decision. + let decision_task = tokio::spawn(async move { + // Wait for the actionable acp_read from the observer. + let mut found_nonce: Option = None; + loop { + match tokio::time::timeout( + std::time::Duration::from_secs(5), + obs_rx.recv(), + ) + .await + { + Ok(Ok(event)) => { + if event.kind == "acp_read" { + if let Some(auth) = &event.authorization { + if auth.actionable { + found_nonce = Some(auth.request_nonce.clone()); + break; + } + } + } + } + _ => break, + } + } + let nonce = found_nonce.expect("actionable acp_read must be emitted"); + // Deliver a valid decision by the captured nonce. + perm_tx + .send(PermissionDecision { + request_nonce: nonce, + option_id: "opt-allow".to_string(), + }) + .await + .expect("decision channel must accept"); + }); + + let idle = std::time::Duration::from_secs(5); + let max_dur = std::time::Duration::from_secs(15); + let hard_deadline = tokio::time::Instant::now() + max_dur; + let result = client + .read_until_response_with_idle_timeout("sess", 999, idle, hard_deadline, max_dur) + .await; + + assert!( + result.is_ok(), + "production-path ask loop must succeed after decision is delivered, got: {result:?}" + ); + assert_eq!( + result.unwrap().get("stopReason").and_then(|v| v.as_str()), + Some("end_turn"), + ); + + // Verify the observer emitted an authorized acp_write (the decision response). + let _ = decision_task.await; + let events = obs.snapshot(); + let write_events: Vec<_> = events + .iter() + .filter(|e| e.kind == "acp_write" && e.authorization.is_some()) + .collect(); + assert!( + !write_events.is_empty(), + "observer must emit at least one authorized acp_write after decision applied" + ); + } + + /// Cancel test: asserts exactly one JSON-RPC response per pending id, + /// no replay on subsequent cancel. + #[tokio::test] + async fn cancel_writes_exactly_one_response_per_pending_id_no_replay() { + // Script that stays alive but produces no output (simulates a hung agent). + let mut client = spawn_script("sleep 5").await; + client.set_permission_config( + ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(), + ); + client.set_owner_pubkey_known(true); + + // Subscribe to observer to capture writes. + let obs = crate::observer::ObserverHandle::in_process(); + client.set_observer(Some(obs.clone()), 0); + + let (_tx, perm_rx) = tokio::sync::mpsc::channel::(8); + client.install_permission_decision_rx(perm_rx); + + // Plant two distinct Pending entries directly — this tests the cancel + // drain path without needing a live protocol exchange. + for i in 0..2u64 { + let hard_deadline = + tokio::time::Instant::now() + std::time::Duration::from_secs(300); + let msg = perm_request(i, default_opts()); + client + .handle_permission_request(&msg, true, hard_deadline) + .await + .expect("ask registration must succeed"); + } + assert_eq!( + client.pending_permissions.len(), + 2, + "two pending entries must be registered before cancel" + ); + client.last_prompt_id = Some(999); + + // First cancel: must drain both entries. + let _ = client + .cancel_with_cleanup_grace("sess-exact-once", std::time::Duration::from_millis(200)) + .await; + assert!( + client.pending_permissions.is_empty(), + "all pending entries must be drained after cancel" + ); + + // Count authorized acp_write events (each must correspond to one drained entry). + let events_after_first = obs.snapshot(); + let write_count_first = events_after_first + .iter() + .filter(|e| e.kind == "acp_write" && e.authorization.is_some()) + .count(); + assert_eq!( + write_count_first, 2, + "cancel must emit exactly one authorized acp_write per pending id (got {write_count_first})" + ); + + // Second cancel on the same client: no pending entries remain, must not + // re-emit any additional acp_write (no replay). + let _ = client + .cancel_with_cleanup_grace("sess-exact-once", std::time::Duration::from_millis(200)) + .await; + let events_after_second = obs.snapshot(); + let write_count_second = events_after_second + .iter() + .filter(|e| e.kind == "acp_write" && e.authorization.is_some()) + .count(); + assert_eq!( + write_count_second, write_count_first, + "second cancel must not emit additional acp_writes (no replay): before={write_count_first}, after={write_count_second}" + ); + } + + /// Paused-time test: the permission deadline fires at exactly 300 seconds, + /// idle is suspended while a Pending entry exists, and capacity recovers + /// after more than eight sequential requests. + #[tokio::test(start_paused = true)] + async fn ask_permission_deadline_idle_suspension_and_capacity_recovery() { + // Script that emits nothing (simulates an agent waiting for permission response). + let mut client = spawn_script("sleep 600").await; + let config = ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(); + client.set_permission_config(config); + client.set_owner_pubkey_known(true); + let obs = crate::observer::ObserverHandle::in_process(); + client.set_observer(Some(obs.clone()), 0); + let (_tx, perm_rx) = tokio::sync::mpsc::channel::(8); + client.install_permission_decision_rx(perm_rx); + + // ── Part 1: permission deadline fires before idle ──────────────────── + // Register one pending entry. + let msg = perm_request(1, default_opts()); + let hard_deadline = tokio::time::Instant::now() + + std::time::Duration::from_secs(PERMISSION_ASK_TIMEOUT_SECS + 10); + client + .handle_permission_request(&msg, true, hard_deadline) + .await + .expect("ask registration must succeed"); + assert_eq!(client.pending_permissions.len(), 1); + + // Drive the loop with a long idle timeout — idle must be SUSPENDED while + // the permission entry is pending; only the 300s permission deadline fires. + let idle = std::time::Duration::from_secs(5); // would fire immediately without suspension + let max_dur = std::time::Duration::from_secs(PERMISSION_ASK_TIMEOUT_SECS + 10); + let hard_deadline = tokio::time::Instant::now() + max_dur; + + // Advance time to just before the 300s deadline — idle should NOT fire. + tokio::time::advance( + std::time::Duration::from_secs(PERMISSION_ASK_TIMEOUT_SECS - 1), + ) + .await; + // Spawn a task to advance time past the deadline and check the loop exits + // via permission expiry (not idle timeout). + let advance_task = tokio::spawn(async { + tokio::time::advance(std::time::Duration::from_secs(2)).await; + }); + + let result = client + .read_until_response_with_idle_timeout("sess-tdl", 999, idle, hard_deadline, max_dur) + .await; + let _ = advance_task.await; + + // The loop must have processed the expired entry (transitioned to Resolved) + // and then continued. Because the script produces no output, after the + // permission entry expires the idle timeout fires next (5s). + // Either an IdleTimeout or HardTimeout is acceptable — the key check is + // that no PermissionPoisoned or unexpected error occurred AND the entry + // was processed (Resolved or drained). + assert!( + !matches!(result, Err(AcpError::PermissionPoisoned)), + "permission expiry must not poison the process, got: {result:?}" + ); + // After the deadline, the entry must have been transitioned to Resolved. + let entry_state = client.pending_permissions.get("1"); + let was_resolved = entry_state + .map(|e| matches!(e.state, PermissionEntryState::Resolved)) + .unwrap_or(true); // drain on turn exit is also acceptable + assert!( + was_resolved, + "entry must be Resolved or drained after permission deadline, got: {entry_state:?}" + ); + + // ── Part 2: capacity recovery after 8 sequential requests ─────────── + // Clear any stale entries and verify 8+ sequential requests can succeed + // when previous resolved entries are drained between turns. + let mut client2 = spawn_script("sleep 600").await; + client2.set_permission_config( + ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(), + ); + client2.set_owner_pubkey_known(true); + let obs2 = crate::observer::ObserverHandle::in_process(); + client2.set_observer(Some(obs2), 0); + let (perm_tx2, perm_rx2) = tokio::sync::mpsc::channel::(16); + client2.install_permission_decision_rx(perm_rx2); + + // Send 9 sequential requests, processing each before sending the next. + // The map is bounded at PERMISSION_MAP_CAP = 8, but Resolved entries do + // not count toward the live-entry cap check — only Pending ones do. + // After each decision is applied (Resolved), the next request must succeed. + for i in 0..9u64 { + let hard = tokio::time::Instant::now() + std::time::Duration::from_secs(300); + let msg = perm_request(i + 100, default_opts()); + let result = client2 + .handle_permission_request(&msg, true, hard) + .await; + // If still Pending from prior iterations, the cap check blocks — this + // tests the case after decisions have been applied (Resolved). + // For this sequential test we deliver decisions immediately. + if result.is_ok() && result.unwrap() { + // Entry is now Pending; deliver a decision immediately. + // Capture the nonce from the freshly-inserted entry. + let id_str = (i + 100).to_string(); + let nonce = client2 + .pending_permissions + .get(&id_str) + .map(|e| e.nonce.clone()); + if let Some(nonce) = nonce { + perm_tx2 + .send(PermissionDecision { + request_nonce: nonce, + option_id: "opt-allow".to_string(), + }) + .await + .ok(); + } + } + } + // Drive the loop to process all queued decisions. + let hard = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + let _ = tokio::time::timeout( + std::time::Duration::from_millis(500), + client2.read_until_response_with_idle_timeout( + "sess-cap", + 9999, + std::time::Duration::from_millis(100), + hard, + std::time::Duration::from_secs(10), + ), + ) + .await; + // After processing, no Pending entries should remain (all should be Resolved + // or the map may have been drained). This proves the capacity map doesn't + // permanently block after 8 requests. + let pending_count = client2 + .pending_permissions + .values() + .filter(|e| matches!(e.state, PermissionEntryState::Pending)) + .count(); + assert_eq!( + pending_count, 0, + "no Pending entries must remain after all decisions applied (capacity recovery confirmed)" + ); + } + + // ── Pinned §1 (simpler): ask entry registered synchronously ────────────── + + #[tokio::test] + async fn ask_registers_entry_in_pending_map() { + // Verify that handle_permission_request under ask policy inserts + // a Pending entry into the map (without needing a live decision loop). + let mut client = spawn_inert_client().await; + let config = ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(); + client.set_permission_config(config); + client.set_owner_pubkey_known(true); + // Install an observer so the ask arm doesn't downgrade. + let obs = crate::observer::ObserverHandle::in_process(); + client.set_observer(Some(obs), 0); + // Install a permission decision channel (must be installed or take() panics). + let (_perm_tx, perm_rx) = tokio::sync::mpsc::channel::(8); + client.install_permission_decision_rx(perm_rx); + + let msg = perm_request(42, default_opts()); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + let result = client + .handle_permission_request(&msg, true, hard_deadline) + .await; + assert!( + result.is_ok(), + "ask must return Ok to suppress generic emit" + ); + assert_eq!( + result.unwrap(), + true, + "ask must return Ok(true) to suppress generic emit" + ); + assert_eq!( + client.pending_permissions.len(), + 1, + "exactly one entry must be registered after ask" + ); + let entry = client + .pending_permissions + .get("42") + .expect("entry under id=42"); + assert!( + matches!(entry.state, PermissionEntryState::Pending), + "entry must start in Pending state" + ); + } + + // ── Pinned §1 (cancel during write path): poison process test ──────────── + + #[test] + fn cancel_during_writing_poisons_process() { + // Simulate a process that has an entry in Writing state at cancel time. + // cancel_with_cleanup_until must return PermissionPoisoned and set the flag. + // + // We test this synchronously because cancel_with_cleanup_until is async + // and we need to manipulate state directly. We use a tokio runtime. + let rt = tokio::runtime::Runtime::new().unwrap(); + rt.block_on(async { + // Use a "sleep" script so the process is alive but won't emit responses. + let mut client = spawn_script("sleep 10").await; + client.set_permission_config( + ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(), + ); + client.set_owner_pubkey_known(true); + + // Manually plant an entry in Writing state — this simulates cancel + // arriving while the harness was in the middle of writing. + client.pending_permissions.insert( + "99".to_string(), + PermissionEntry { + nonce: "n99".to_string(), + options_snapshot: vec![], + state: PermissionEntryState::Writing, + deadline: tokio::time::Instant::now() + std::time::Duration::from_secs(300), + }, + ); + // cancel_with_cleanup needs last_prompt_id to be Some. + client.last_prompt_id = Some(999); + + let err = client + .cancel_with_cleanup_grace("sess-poison", std::time::Duration::from_millis(500)) + .await + .expect_err("cancel during write must return Err"); + + assert!( + matches!(err, AcpError::PermissionPoisoned), + "expected PermissionPoisoned, got {err:?}" + ); + assert!( + client.permission_poisoned, + "poisoned flag must be set after cancel-during-write" + ); + }); + } + + #[test] + fn poisoned_process_surfaces_immediately_on_next_cancel() { + // Once poisoned, every subsequent cancel must immediately return PermissionPoisoned + // without writing anything — the process is unsafe to use. + let rt = tokio::runtime::Runtime::new().unwrap(); + rt.block_on(async { + let mut client = spawn_script("sleep 10").await; + client.permission_poisoned = true; + client.last_prompt_id = Some(1); + + let err = client + .cancel_with_cleanup_grace("sess", std::time::Duration::from_millis(200)) + .await + .expect_err("poisoned process must error immediately"); + assert!(matches!(err, AcpError::PermissionPoisoned)); + }); + } + + #[test] + fn poisoned_process_check_in_read_loop_returns_poison_error() { + // Once permission_poisoned is set, read_until_response_with_idle_timeout + // must return PermissionPoisoned on the next loop iteration. + let rt = tokio::runtime::Runtime::new().unwrap(); + rt.block_on(async { + let mut client = spawn_script("sleep 10").await; + client.permission_poisoned = true; + client.last_prompt_id = Some(42); + + let idle = std::time::Duration::from_secs(5); + let max_dur = std::time::Duration::from_secs(10); + let hard_deadline = tokio::time::Instant::now() + max_dur; + let result = client + .read_until_response_with_idle_timeout("sess", 42, idle, hard_deadline, max_dur) + .await; + assert!( + matches!(result, Err(AcpError::PermissionPoisoned)), + "expected PermissionPoisoned from poisoned-flag check, got {result:?}" + ); + }); + } + + // ── Pinned §5: cancel drains pending entries with cancelled ─────────────── + + #[test] + fn cancel_drains_pending_entries_with_cancelled_response() { + // Under ask policy: cancel must drain all Pending entries and write + // "cancelled" responses for each, then proceed to session/cancel. + // Verifies: + // - Map is empty after cancel (entries were drained). + // - Cancel result is NOT PermissionPoisoned (no Writing entries present). + // - Cancel exits normally (Ok or CancelDrainTimeout — sleep script never + // emits a response, so this exits via timeout, which is expected). + // + // We can verify that Pending entries are removed by checking the map post-cancel. + // We don't verify the wire bytes here (that requires a live script) — we verify + // the state machine: Pending entries disappear after cancel. + let rt = tokio::runtime::Runtime::new().unwrap(); + rt.block_on(async { + // Use a "sleep" script — stays alive but ignores stdin. + let mut client = spawn_script("sleep 5").await; + client.set_permission_config( + ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(), + ); + + // Plant two Pending entries. + for i in 0..2u64 { + client.pending_permissions.insert( + format!("{i}"), + PermissionEntry { + nonce: format!("n{i}"), + options_snapshot: vec![ + serde_json::json!({"optionId":"opt","kind":"reject_once","name":"R"}), + ], + state: PermissionEntryState::Pending, + deadline: tokio::time::Instant::now() + std::time::Duration::from_secs(300), + }, + ); + } + client.last_prompt_id = Some(999); + + // cancel_with_cleanup_grace with short grace — the sleep script will + // never emit a response, so this exits via CancelDrainTimeout. + let result = client + .cancel_with_cleanup_grace("sess-drain", std::time::Duration::from_millis(200)) + .await; + + // Should NOT be PermissionPoisoned (no Writing entries). + assert!( + !matches!(result, Err(AcpError::PermissionPoisoned)), + "no Writing entries — must not be PermissionPoisoned" + ); + // Map must be empty — Pending entries were drained. + assert!( + client.pending_permissions.is_empty(), + "all Pending entries must be removed from the map after cancel" + ); + }); + } + + // ── Pinned §2: reject policy is byte-for-byte unchanged ─────────────────── + + #[tokio::test] + async fn reject_policy_denies_synchronously_and_returns_ok_true() { + let mut client = spawn_inert_client().await; + set_policy(&mut client, PermissionPolicy::Reject); + + let msg = perm_request(7, default_opts()); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + let result = client + .handle_permission_request(&msg, true, hard_deadline) + .await; + // Reject is synchronous — no pending entry, Ok(true) to suppress generic emit. + assert!(result.is_ok(), "reject must return Ok"); + assert_eq!(result.unwrap(), true, "reject must return Ok(true)"); + assert!( + client.pending_permissions.is_empty(), + "reject must not leave pending entries" + ); + // Legacy single-id slot must also be cleared after the synchronous response. + assert!( + client.pending_permission_id.is_none(), + "pending_permission_id must be None after reject completes" + ); + assert!( + client.permission_responded, + "permission_responded must be true after reject completes" + ); + } + + // ── Pinned §2: allow policy auto-selects allow_once ─────────────────────── + + #[tokio::test] + async fn allow_policy_auto_selects_allow_once_and_returns_ok_true() { + let mut client = spawn_inert_client().await; + set_policy(&mut client, PermissionPolicy::Allow); + + let msg = perm_request(8, default_opts()); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + let result = client + .handle_permission_request(&msg, true, hard_deadline) + .await; + assert!(result.is_ok(), "allow auto-select must return Ok"); + assert_eq!( + result.unwrap(), + true, + "allow auto-select must return Ok(true)" + ); + // No pending entries — handled synchronously. + assert!(client.pending_permissions.is_empty()); + } + + #[tokio::test] + async fn allow_policy_fails_closed_with_no_allow_once_option() { + let mut client = spawn_inert_client().await; + set_policy(&mut client, PermissionPolicy::Allow); + + // Only reject_once offered — allow policy must fail closed. + let msg = perm_request(9, &[("opt-r", "reject_once", "Reject")]); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + let result = client + .handle_permission_request(&msg, true, hard_deadline) + .await; + // Fail closed: denial written, Ok(true) returned. + assert!(result.is_ok(), "fail-closed allow must return Ok"); + assert_eq!( + result.unwrap(), + true, + "fail-closed allow must return Ok(true)" + ); + assert!(client.pending_permissions.is_empty()); + } + + // ── Pinned §6: decision arm — validated option_id must be in snapshot ───── + + #[tokio::test] + async fn decision_with_unknown_option_id_is_ignored() { + // A decision carrying an optionId not in the snapshot must be ignored + // (no response written, entry stays Pending) — the loop continues. + // After the bad decision is processed, the loop times out on idle (since the + // script produces no output after the initial response) and the entry is + // still Pending at that point. + // + // The script produces the terminal id=999 response only AFTER a short delay, + // giving the loop time to process the bad decision and leave the entry Pending. + // We verify the entry is still Pending by running the loop until idle timeout. + let script = "sleep 2; echo '{\"jsonrpc\":\"2.0\",\"id\":999,\"result\":{\"done\":true}}'"; + let mut client = spawn_script(script).await; + client.set_owner_pubkey_known(true); + set_policy(&mut client, PermissionPolicy::Ask); + let obs = crate::observer::ObserverHandle::in_process(); + client.set_observer(Some(obs), 0); + + let nonce = "test-nonce-bad-opt".to_string(); + let req_id_str = "5".to_string(); + client.pending_permissions.insert( + req_id_str.clone(), + PermissionEntry { + nonce: nonce.clone(), + options_snapshot: vec![ + serde_json::json!({"optionId":"valid-opt","kind":"allow_once","name":"A"}), + ], + state: PermissionEntryState::Pending, + deadline: tokio::time::Instant::now() + std::time::Duration::from_secs(300), + }, + ); + + // Deliver a decision with a nonce that matches but an invalid optionId. + let bad_decision = PermissionDecision { + request_nonce: nonce, + option_id: "nonexistent-option".to_string(), + }; + + let (tx, rx) = tokio::sync::mpsc::channel::(1); + client.install_permission_decision_rx(rx); + // Send the bad decision; then close the sender so the channel is exhausted. + tx.send(bad_decision).await.unwrap(); + drop(tx); + + // Drive the loop with a short idle timeout — the bad decision is processed + // on the first iteration (entry stays Pending), then the loop idles. + let idle = std::time::Duration::from_millis(300); + let max_dur = std::time::Duration::from_secs(5); + let hard_deadline = tokio::time::Instant::now() + max_dur; + let result = client + .read_until_response_with_idle_timeout("sess-bad-opt", 5, idle, hard_deadline, max_dur) + .await; + + // The loop exits via idle timeout (script sleeps; bad decision was ignored, + // so no terminal response for id=5 was written, and idle fires). + // We accept either idle timeout OR id=999 match (if the script's sleep was short). + // The critical assertion is on the entry state. + let _ = result; // exit reason is not the focus + + // Entry must still be Pending — the bad decision did not mutate it. + let entry = client.pending_permissions.get(&req_id_str); + // The loop drains on non-recoverable errors; on idle timeout (recoverable) it + // does NOT drain — entry must still be there and Pending. + match entry { + Some(e) => assert!( + matches!(e.state, PermissionEntryState::Pending), + "entry must still be Pending after bad decision, got: {:?}", + e.state + ), + None => panic!("entry was removed — idle timeout should not drain the map"), + } + } + + // ── Pinned §7 (wire transmission): transmit_mode drives set_config_option ─ + + #[test] + fn resolved_permission_config_effective_mode_wire_string_is_correct() { + // Verify that effective_mode.as_wire_str() returns the correct ACP wire value. + let cfg = ResolvedPermissionConfig::resolve(PermissionPolicy::Reject, None).unwrap(); + assert_eq!(cfg.effective_mode.as_wire_str(), "dontAsk"); + + let cfg = ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(); + assert_eq!(cfg.effective_mode.as_wire_str(), "default"); + + let cfg = ResolvedPermissionConfig::resolve(PermissionPolicy::Allow, None).unwrap(); + assert_eq!(cfg.effective_mode.as_wire_str(), "default"); + } + + // ── Pinned amendment: PermissionMode::Auto matrix row ──────────────────── + // + // `auto` = model-gated classifier — the adapter may self-approve most tool + // calls internally but can still forward residual permission requests to ACP. + // - allow + auto → compatible (transmit as-is; both want unattended approval) + // - ask + auto → compatible with warning (residual escalations surface cards; + // internally-approved calls bypass ask silently) + // - reject + auto → startup error (inverted security: policy says deny, adapter + // auto-approves everything) + + #[test] + fn resolved_permission_config_allow_plus_explicit_auto_is_ok() { + // allow + auto is compatible: both want unattended approval. + let cfg = + ResolvedPermissionConfig::resolve(PermissionPolicy::Allow, Some(PermissionMode::Auto)) + .unwrap(); + assert_eq!(cfg.effective_mode, PermissionMode::Auto); + assert_eq!(cfg.effective_mode.as_wire_str(), "auto"); + assert_eq!(cfg.mode_source, ModeSource::Explicit); + } + + #[test] + fn resolved_permission_config_ask_plus_explicit_auto_is_ok_with_warning() { + // ask + auto is compatible-with-warning: residual escalations still surface + // cards; internally-approved calls bypass the ask flow silently. + // `auto` is a model classifier, not a bypass — some requests still escalate. + let result = + ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, Some(PermissionMode::Auto)); + assert!( + result.is_ok(), + "ask + auto must succeed (warn only), got: {result:?}" + ); + let cfg = result.unwrap(); + assert_eq!(cfg.effective_mode, PermissionMode::Auto); + assert_eq!(cfg.mode_source, ModeSource::Explicit); + } + + #[test] + fn resolved_permission_config_reject_plus_explicit_auto_is_startup_error() { + // reject + auto: inverted-security worst case — policy says deny but + // adapter auto-approves everything internally. + let result = + ResolvedPermissionConfig::resolve(PermissionPolicy::Reject, Some(PermissionMode::Auto)); + assert!(result.is_err(), "reject + auto must be a startup error"); + let msg = format!("{}", result.unwrap_err()); + assert!(msg.contains("auto"), "error must mention auto, got: {msg}"); + } + + #[test] + fn permission_mode_auto_wire_string_is_correct() { + assert_eq!(PermissionMode::Auto.as_wire_str(), "auto"); + assert!(!PermissionMode::Auto.is_default()); + } } diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index d959685846..50cf9038e8 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -115,6 +115,10 @@ impl std::fmt::Display for RespondTo { /// `configId: "mode"` (e.g. `claude-agent-acp`). /// /// - `default` — agent's built-in behaviour (permission requests per tool call). +/// - `auto` — fully autonomous execution; model-gated (requires `supportsAutoMode`); +/// the adapter degrades gracefully to `default` when the active model does not +/// support it. The adapter self-approves all tool calls internally — no +/// `session/request_permission` ever crosses ACP under this mode. /// - `acceptEdits` — auto-approve file edits, still ask for other tools. /// - `dontAsk` — never prompt; reject anything that would require permission. /// - `plan` — planning-only mode (no tool execution). @@ -123,6 +127,22 @@ pub enum PermissionMode { /// Agent default — permission requests per tool call. #[value(alias = "default")] Default, + /// Fully autonomous execution; model-gated (requires `supportsAutoMode`). + /// + /// `auto` is a model-gated classifier — the adapter self-approves most tool + /// calls internally, but can fall back to forwarding residual + /// `session/request_permission` requests to ACP when the model chooses manual + /// approval for a specific call. It is therefore **not** a hard bypass. + /// + /// Policy compatibility: + /// - `allow + auto` — compatible; both want unattended approval. + /// - `ask + auto` — compatible with a startup warning; residual escalations + /// still surface permission cards, but internally approved calls bypass the + /// ask flow silently. + /// - `reject + auto` — startup contradiction; adapter auto-approves + /// internally while the policy intends to deny — inverted-security worst case. + #[value(alias = "auto")] + Auto, /// Auto-approve file edits, still ask for other tools. #[value(alias = "acceptEdits")] AcceptEdits, @@ -140,6 +160,7 @@ impl PermissionMode { pub fn as_wire_str(&self) -> &'static str { match self { Self::Default => "default", + Self::Auto => "auto", Self::AcceptEdits => "acceptEdits", Self::DontAsk => "dontAsk", Self::Plan => "plan", @@ -148,6 +169,7 @@ impl PermissionMode { /// Returns `true` when the mode is the agent's built-in default and /// therefore doesn't need to be explicitly set. + #[cfg(test)] pub fn is_default(&self) -> bool { matches!(self, Self::Default) } @@ -159,6 +181,173 @@ impl std::fmt::Display for PermissionMode { } } +/// How Buzz responds to an ACP `session/request_permission` request. +/// +/// Injected as `BUZZ_ACP_PERMISSION_POLICY`. Desktop injects the resolved +/// per-agent or fleet-wide value; headless defaults to `reject`. +/// +/// - `allow` — auto-select the unique `allow_once` option; fail closed if +/// zero or multiple `allow_once` candidates, malformed options, +/// or any validation error. +/// - `ask` — surface the request as an actionable card for the owner; +/// fail closed on timeout (300 s) or if the observer / owner is +/// unavailable. +/// - `reject` — deny every request (today's behaviour, headless default). +#[derive(Debug, Clone, Copy, PartialEq, clap::ValueEnum)] +pub enum PermissionPolicy { + /// Auto-approve via the unique `allow_once` option; fail closed otherwise. + #[value(alias = "allow")] + Allow, + /// Surface as an actionable card; fail closed on timeout or unavailability. + #[value(alias = "ask")] + Ask, + /// Deny all requests — headless default, byte-for-byte today's behaviour. + #[value(alias = "reject")] + Reject, +} + +impl std::fmt::Display for PermissionPolicy { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match self { + Self::Allow => "allow", + Self::Ask => "ask", + Self::Reject => "reject", + }) + } +} + +/// Whether an effective `PermissionMode` was derived by the harness or +/// supplied explicitly by the operator. +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum ModeSource { + /// No `--permission-mode` was supplied; the harness derived the mode from + /// the active `PermissionPolicy`. + Derived, + /// An explicit `--permission-mode` / `BUZZ_ACP_PERMISSION_MODE` value was + /// supplied by the operator. + Explicit, +} + +impl std::fmt::Display for ModeSource { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match self { + Self::Derived => "derived", + Self::Explicit => "explicit", + }) + } +} + +/// Resolved, immutable per-startup permission configuration. +/// +/// Computed once in `Config::from_args` from `policy` + optional `mode` and +/// carried through `PromptContext` (via `Arc`) so every task reads the same +/// value without re-deriving it. +/// +/// `transmit_mode` — set `session/set_config_option` for this mode whenever +/// the agent advertises it. **Always set** (including for `PermissionMode::Default`); +/// the caller decides whether to skip based on advertisement, not derivation. +#[derive(Debug, Clone)] +pub struct ResolvedPermissionConfig { + /// The high-level policy governing how permission requests are answered. + pub policy: PermissionPolicy, + /// The ACP mode that will be sent to the agent after session creation. + pub effective_mode: PermissionMode, + /// Whether `effective_mode` was derived or supplied explicitly. + pub mode_source: ModeSource, + /// `true` when the effective mode should be transmitted to the agent via + /// `session/set_config_option`, i.e. whenever the agent advertises it. + pub transmit_mode: bool, +} + +impl ResolvedPermissionConfig { + /// Derive the config from a `policy` and an optional explicit `mode`. + /// + /// Returns `Err` for contradictory combinations: + /// - `ask` + explicit `dontAsk` — harness would want the agent to + /// escalate, but `dontAsk` makes the agent self-deny internally. + /// - `allow` + explicit `dontAsk` — same contradiction. + /// - `reject` + explicit `auto` — inverted-security worst case: policy says + /// "deny" but the adapter auto-approves everything internally. + /// + /// Emits a warning (not an error) for `ask + auto`: internally-approved tool + /// calls bypass the ask flow silently, but residual escalations still surface + /// cards — the combination works, with the caveat that not all requests are seen. + pub fn resolve( + policy: PermissionPolicy, + explicit_mode: Option, + ) -> Result { + // Fail on contradictory ask/allow + dontAsk combinations. + if matches!(policy, PermissionPolicy::Ask | PermissionPolicy::Allow) + && explicit_mode == Some(PermissionMode::DontAsk) + { + return Err(ConfigError::ConfigFile(format!( + "permission_policy={policy} conflicts with permission_mode=dontAsk: \ + dontAsk makes the agent self-deny internally before Buzz can answer" + ))); + } + // Fail on reject + auto: inverted-security worst case — policy says "deny" + // but the adapter auto-approves everything internally. + // `ask` + auto is a warning-only case: the adapter MAY still forward residual + // permission requests to ACP (auto is a model classifier, not bypass mode); + // warn and transmit rather than fail startup. + // `allow` + auto is compatible: both policies want unattended approval. + if policy == PermissionPolicy::Reject && explicit_mode == Some(PermissionMode::Auto) { + return Err(ConfigError::ConfigFile(format!( + "permission_policy={policy} conflicts with permission_mode=auto: \ + auto makes the adapter self-approve internally, which bypasses the \ + reject policy — inverted-security worst case" + ))); + } + // Warn on ask + auto: residual permission requests may still reach ACP + // (auto is a model classifier, not bypass mode) so ask can still surface + // cards — but internally-approved calls will bypass the ask flow silently. + if policy == PermissionPolicy::Ask && explicit_mode == Some(PermissionMode::Auto) { + tracing::warn!( + "permission_policy=ask with permission_mode=auto: internally-approved \ + tool calls bypass Buzz ask flow; residual escalations will still \ + surface cards. Consider policy=allow if unattended approval is intended." + ); + } + + let (effective_mode, mode_source) = match explicit_mode { + Some(m) => (m, ModeSource::Explicit), + None => { + // Mode matrix — derived from policy when no explicit mode given: + // reject → dontAsk (harness rejects; adapter also self-denies for + // consistency — byte-for-byte today's behaviour) + // ask → default (keep the adapter escalating to Buzz) + // allow → default (keep the adapter escalating to Buzz; + // dontAsk would silently self-deny before we + // could auto-select allow_once) + let derived = match policy { + PermissionPolicy::Reject => PermissionMode::DontAsk, + PermissionPolicy::Ask | PermissionPolicy::Allow => PermissionMode::Default, + }; + (derived, ModeSource::Derived) + } + }; + + Ok(Self { + policy, + effective_mode, + mode_source, + // Always transmit — the caller skips based on agent advertisement, + // not on whether the mode is the default. + transmit_mode: true, + }) + } +} + +impl std::fmt::Display for ResolvedPermissionConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "policy={} mode={}({})", + self.policy, self.effective_mode, self.mode_source + ) + } +} + /// CLI args for `buzz-acp models` — query available models from an agent. /// /// This is a standalone `Parser` (not a subcommand variant) because the @@ -424,18 +613,32 @@ pub struct CliArgs { #[arg(long, env = "BUZZ_ACP_SESSION_TITLE")] pub session_title: Option, - /// Permission mode for agents that support `session/set_config_option` - /// with `configId: "mode"` (e.g. `claude-agent-acp`). + /// How Buzz responds to ACP `session/request_permission` requests. /// - /// Defaults to `dontAsk`, which rejects operations that need interactive - /// approval because Buzz does not expose a human permission prompt. + /// - `reject` (headless default) — deny all permission requests. + /// - `ask` — surface as an actionable card; auto-deny on timeout (300 s) + /// or when the observer / owner is unavailable. + /// - `allow` — auto-approve via the unique `allow_once` option; + /// fail closed if zero or multiple `allow_once` candidates. + /// + /// Desktop injects the resolved per-agent or fleet-wide value. + /// Headless installations should leave this unset (defaults to `reject`). #[arg( long, - env = "BUZZ_ACP_PERMISSION_MODE", - default_value = "dont-ask", + env = "BUZZ_ACP_PERMISSION_POLICY", + default_value = "reject", value_enum )] - pub permission_mode: PermissionMode, + pub permission_policy: PermissionPolicy, + + /// ACP permission mode sent to the agent via `session/set_config_option`. + /// + /// When unset the harness derives a sensible default from `permission_policy`: + /// `reject` → `dontAsk`, `ask` / `allow` → `default`. + /// Explicit values are validated: `ask` or `allow` + `dontAsk` is a startup + /// error because `dontAsk` makes the agent self-deny before Buzz can answer. + #[arg(long, env = "BUZZ_ACP_PERMISSION_MODE", value_enum)] + pub permission_mode: Option, /// Inbound author gate: which authors' events the harness forwards. /// Modes: owner-only (default), allowlist, anyone, nobody. @@ -530,8 +733,10 @@ pub struct Config { /// Sanitized session title, sent as `_meta.sessionTitle` on `session/new`. /// `None` when unset or when the configured value sanitized to empty. pub session_title: Option, - /// Permission mode to apply after session creation. `Default` = skip. - pub permission_mode: PermissionMode, + /// Resolved permission configuration — policy, effective ACP mode, and + /// how to transmit it. Computed once from `PermissionPolicy` + optional + /// explicit `PermissionMode` in `from_args`. + pub permission_config: ResolvedPermissionConfig, /// Inbound author gate mode. pub respond_to: RespondTo, /// Validated allowlist of pubkey hex strings (used when respond_to == Allowlist). @@ -1054,6 +1259,9 @@ impl Config { validate_multiple_event_handling(args.multiple_event_handling, args.dedup)?; + let permission_config = + ResolvedPermissionConfig::resolve(args.permission_policy, args.permission_mode)?; + let config = Config { keys, relay_url: args.relay_url, @@ -1092,7 +1300,7 @@ impl Config { .session_title .as_deref() .and_then(sanitize_session_title), - permission_mode: args.permission_mode, + permission_config, respond_to: args.respond_to, respond_to_allowlist, allowed_respond_to, @@ -1125,7 +1333,7 @@ impl Config { format!(" allowed_respond_to=[{}]", modes.join(",")) }; format!( - "relay={} pubkey={} agent_cmd={} {} mcp_cmd={} idle_timeout={}s max_turn={}s agents={} heartbeat={}s subscribe={:?} dedup={:?} meh={:?} ignore_self={} context_limit={} max_turns_per_session={} presence={} typing={} memory={} model={} permission_mode={} {}{}", + "relay={} pubkey={} agent_cmd={} {} mcp_cmd={} idle_timeout={}s max_turn={}s agents={} heartbeat={}s subscribe={:?} dedup={:?} meh={:?} ignore_self={} context_limit={} max_turns_per_session={} presence={} typing={} memory={} model={} permission_mode={}({}) {}{}", self.relay_url, self.keys.public_key().to_hex(), self.agent_command, @@ -1145,7 +1353,8 @@ impl Config { self.typing_enabled, self.memory_enabled, self.model.as_deref().unwrap_or("(agent default)"), - self.permission_mode, + self.permission_config.effective_mode, + self.permission_config.mode_source, respond_to_detail, allowed_respond_to_detail, ) @@ -1463,7 +1672,11 @@ mod tests { memory_enabled: true, model: None, session_title: None, - permission_mode: PermissionMode::DontAsk, + permission_config: ResolvedPermissionConfig::resolve( + PermissionPolicy::Reject, + Some(PermissionMode::DontAsk), + ) + .expect("test config"), respond_to: RespondTo::Anyone, respond_to_allowlist: HashSet::new(), allowed_respond_to: Vec::new(), @@ -2285,7 +2498,11 @@ channels = "ALL" #[test] fn test_summary_includes_permission_mode() { let mut config = test_config(SubscribeMode::Mentions); - config.permission_mode = PermissionMode::DontAsk; + config.permission_config = ResolvedPermissionConfig::resolve( + PermissionPolicy::Reject, + Some(PermissionMode::DontAsk), + ) + .expect("test config"); let s = config.summary(); assert!( s.contains("permission_mode=dontAsk"), @@ -2296,7 +2513,8 @@ channels = "ALL" #[test] fn test_summary_permission_mode_default() { let mut config = test_config(SubscribeMode::Mentions); - config.permission_mode = PermissionMode::Default; + config.permission_config = + ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).expect("test config"); let s = config.summary(); assert!( s.contains("permission_mode=default"), @@ -2307,7 +2525,10 @@ channels = "ALL" #[test] fn test_default_config_rejects_interactive_permissions() { let config = test_config(SubscribeMode::Mentions); - assert_eq!(config.permission_mode, PermissionMode::DontAsk); + assert_eq!( + config.permission_config.effective_mode, + PermissionMode::DontAsk + ); } #[test] diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 65c9dd6203..9fb1d1e131 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -594,6 +594,7 @@ fn batch_envelope(events: &[observer::ObserverEvent]) -> observer::ObserverEvent session_id: last.session_id.clone(), turn_id: last.turn_id.clone(), started_at: last.started_at.clone(), + authorization: None, payload: serde_json::json!({ "events": serde_json::to_value(events).unwrap_or_default(), }), @@ -1116,6 +1117,9 @@ fn handle_relay_observer_control_event( Some("switch_model") => { handle_switch_model_control(&payload, pool, observer); } + Some("permission_decision") => { + handle_permission_decision_control(&payload, pool, observer); + } _ => { tracing::debug!(payload = %payload, "ignoring unknown observer control frame"); } @@ -1235,6 +1239,120 @@ fn handle_switch_model_control( } } +/// Handle a `permission_decision` control frame. +/// +/// Extracts `channelId`, `requestNonce`, and `optionId` from the payload and +/// delivers a [`crate::acp::PermissionDecision`] to the in-flight read loop +/// via the per-task `permission_decision_tx` mpsc channel. +/// +/// If there is no in-flight task for the channel, or the sender is gone, the +/// frame is dropped silently (the per-request 300s timeout will fail the entry +/// closed on its own). +fn handle_permission_decision_control( + payload: &serde_json::Value, + pool: &mut AgentPool, + observer: Option<&observer::ObserverHandle>, +) { + let Some(channel_id) = payload + .get("channelId") + .and_then(|v| v.as_str()) + .and_then(|v| v.parse::().ok()) + else { + tracing::warn!("observer permission_decision control frame missing valid channelId"); + return; + }; + + let Some(request_nonce) = payload + .get("requestNonce") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + else { + tracing::warn!("observer permission_decision control frame missing requestNonce"); + return; + }; + + let Some(option_id) = payload + .get("optionId") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + else { + tracing::warn!("observer permission_decision control frame missing optionId"); + return; + }; + + let decision = crate::acp::PermissionDecision { + request_nonce: request_nonce.to_string(), + option_id: option_id.to_string(), + }; + + // Find the in-flight task for this channel and deliver via its mpsc. + let entry = pool + .task_map_mut() + .values_mut() + .find(|m| m.channel_id == Some(channel_id)); + + let status = if let Some(meta) = entry { + if let Some(tx) = &meta.permission_decision_tx { + match tx.try_send(decision) { + Ok(()) => { + tracing::info!( + channel = %channel_id, + nonce = %request_nonce, + option_id = %option_id, + "permission_decision delivered to read loop" + ); + "sent" + } + Err(tokio::sync::mpsc::error::TrySendError::Full(_)) => { + tracing::warn!( + channel = %channel_id, + "permission_decision channel full — dropping (will timeout)" + ); + "channel_full" + } + Err(tokio::sync::mpsc::error::TrySendError::Closed(_)) => { + tracing::warn!( + channel = %channel_id, + "permission_decision channel closed — read loop already exited" + ); + "channel_closed" + } + } + } else { + tracing::warn!( + channel = %channel_id, + "permission_decision_tx not installed for in-flight task" + ); + "no_channel" + } + } else { + tracing::warn!( + channel = %channel_id, + "permission_decision control frame for channel with no in-flight task" + ); + "no_active_turn" + }; + + if let Some(observer) = observer { + observer.emit( + "control_result", + None, + &observer::ObserverContext { + channel_id: Some(channel_id.to_string()), + session_id: None, + turn_id: None, + started_at: None, + }, + serde_json::json!({ + "type": "permission_decision", + "status": status, + "requestNonce": request_nonce, + "optionId": option_id, + }), + ); + } +} + /// Maximum crashes in a 60-second window before a slot's circuit opens. const CIRCUIT_BREAKER_THRESHOLD: usize = 3; /// Window for circuit-breaker crash counting. @@ -1835,7 +1953,7 @@ async fn tokio_main() -> Result<()> { channel_info: pool::ChannelInfoResolver::new(channel_info_map, relay.rest_client()), context_message_limit: config.context_message_limit, max_turns_per_session: config.max_turns_per_session, - permission_mode: config.permission_mode, + permission_config: config.permission_config.clone(), agent_keys: config.keys.clone(), agent_owner_pubkey: startup_owner .as_deref() @@ -3290,6 +3408,17 @@ fn dispatch_pending( agent.acp.install_steer_rx(rx); let steer_tx = Some(tx); + // Permission decision channel: delivers `permission_decision` control + // frames into the read loop's decision arm (spec §4). Installed + // per-session (the receiver is taken by the read loop and dropped + // when the turn ends; the next turn installs a fresh pair). Capacity + // matches PERMISSION_MAP_CAP so each pending entry gets a slot. + let (perm_tx, perm_rx) = tokio::sync::mpsc::channel::( + crate::acp::PERMISSION_MAP_CAP, + ); + agent.acp.install_permission_decision_rx(perm_rx); + let permission_decision_tx = Some(perm_tx); + // Prompt text is now built inside run_prompt_task (needs async for // context fetching). Pass None for prompt_text; batch carries the data. let (control_tx, control_rx) = tokio::sync::oneshot::channel::(); @@ -3318,6 +3447,7 @@ fn dispatch_pending( recoverable_batch, control_tx: Some(control_tx), steer_tx, + permission_decision_tx, }, ); dispatched_channels.push((channel_id, typing_scope)); @@ -3703,6 +3833,10 @@ fn handle_prompt_result( | acp::AcpError::WriteTimeout(_) | acp::AcpError::Timeout(_) | acp::AcpError::Protocol(_) + // A poisoned process wrote a partial permission response + // and must NOT be returned to the pool — the pipe state is + // uncertain and re-use would corrupt the next turn's writes. + | acp::AcpError::PermissionPoisoned ); let error_code = match &e { acp::AcpError::AgentError { code, .. } => Some(*code), @@ -3932,6 +4066,7 @@ fn dispatch_heartbeat( recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); *heartbeat_in_flight = true; @@ -4703,6 +4838,7 @@ mod owner_control_command_tests { recoverable_batch: None, control_tx: Some(control_tx), steer_tx: None, + permission_decision_tx: None, }, ); @@ -5208,6 +5344,7 @@ mod observer_publish_queue_tests { session_id: Some("session-1".to_string()), turn_id: Some("turn-1".to_string()), started_at: None, + authorization: None, payload: serde_json::json!({ "seq": seq }), } } @@ -6076,6 +6213,7 @@ mod observer_chunk_coalescer_tests { session_id: Some("session-1".to_string()), turn_id: Some("turn-1".to_string()), started_at: None, + authorization: None, payload: serde_json::json!({ "jsonrpc": "2.0", "method": "session/update", @@ -6104,6 +6242,7 @@ mod observer_chunk_coalescer_tests { session_id: Some("session-1".to_string()), turn_id: Some("turn-1".to_string()), started_at: None, + authorization: None, payload: serde_json::json!({ "type": "turn_started" }), } } @@ -6199,7 +6338,11 @@ mod build_mcp_servers_tests { memory_enabled: false, model: None, session_title: None, - permission_mode: config::PermissionMode::DontAsk, + permission_config: config::ResolvedPermissionConfig::resolve( + config::PermissionPolicy::Reject, + None, + ) + .expect("test config"), respond_to: config::RespondTo::Anyone, respond_to_allowlist: std::collections::HashSet::new(), allowed_respond_to: vec![], @@ -6421,7 +6564,11 @@ mod error_outcome_emission_tests { memory_enabled: false, model: None, session_title: None, - permission_mode: config::PermissionMode::DontAsk, + permission_config: config::ResolvedPermissionConfig::resolve( + config::PermissionPolicy::Reject, + None, + ) + .expect("test config"), respond_to: config::RespondTo::Anyone, respond_to_allowlist: HashSet::new(), allowed_respond_to: vec![], @@ -6493,6 +6640,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); @@ -6569,6 +6717,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); started_rx.await.unwrap(); @@ -6661,6 +6810,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -6752,6 +6902,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -6857,6 +7008,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -6933,6 +7085,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -7027,6 +7180,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let config = test_config(); @@ -7143,6 +7297,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -7282,6 +7437,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -7470,6 +7626,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -7555,6 +7712,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -7617,6 +7775,7 @@ mod observer_payload_trim_tests { session_id: Some("sess-1".to_string()), turn_id: Some("turn-1".to_string()), started_at: None, + authorization: None, payload, } } diff --git a/crates/buzz-acp/src/observer.rs b/crates/buzz-acp/src/observer.rs index 7029e5af6d..104b604caf 100644 --- a/crates/buzz-acp/src/observer.rs +++ b/crates/buzz-acp/src/observer.rs @@ -30,6 +30,25 @@ pub struct ObserverContext { pub started_at: Option, } +/// Authorization envelope attached to permission-related observer events. +/// +/// Present on the single `acp_read` emitted after a permission request passes +/// the admission preflight, and on the corresponding `acp_write` after the +/// response is confirmed written. +#[derive(Clone, Debug, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct AuthorizationEnvelope { + /// Single-use nonce bound to this request — delivered to the desktop and + /// consumed exactly once when the owner makes a decision. + pub request_nonce: String, + /// `true` when the owner can take action (policy=ask, preflight passed, + /// owner/observer available). `false` for auto-deny / fail-closed paths. + pub actionable: bool, + /// Human-readable reason when `actionable` is `false`. + #[serde(skip_serializing_if = "Option::is_none")] + pub reason: Option, +} + /// Handle used by the harness to publish local observer events. #[derive(Clone)] pub struct ObserverHandle { @@ -74,6 +93,10 @@ pub struct ObserverEvent { /// RFC3339 timestamp at which the current turn began, when known. #[serde(skip_serializing_if = "Option::is_none")] pub started_at: Option, + /// Authorization envelope — present only on permission `acp_read` / + /// `acp_write` frames. `None` on all other event kinds. + #[serde(skip_serializing_if = "Option::is_none")] + pub authorization: Option, /// Raw or semantic event payload. pub payload: serde_json::Value, } @@ -107,6 +130,31 @@ impl ObserverHandle { agent_index: Option, context: &ObserverContext, payload: serde_json::Value, + ) { + self.emit_inner(kind, agent_index, context, None, payload); + } + + /// Emit a local observer event with an authorization envelope. + /// + /// Used for permission `acp_read` and `acp_write` frames. + pub fn emit_authorized( + &self, + kind: impl Into, + agent_index: Option, + context: &ObserverContext, + authorization: AuthorizationEnvelope, + payload: serde_json::Value, + ) { + self.emit_inner(kind, agent_index, context, Some(authorization), payload); + } + + fn emit_inner( + &self, + kind: impl Into, + agent_index: Option, + context: &ObserverContext, + authorization: Option, + payload: serde_json::Value, ) { let event = ObserverEvent { seq: self.inner.seq.fetch_add(1, Ordering::Relaxed), @@ -117,6 +165,7 @@ impl ObserverHandle { session_id: context.session_id.clone(), turn_id: context.turn_id.clone(), started_at: context.started_at.clone(), + authorization, payload, }; diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 8430307d9c..a4869924b8 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -34,7 +34,7 @@ use crate::acp::{ resolve_model_switch_method, AcpClient, AcpError, EnvVar, McpServer, ModelSwitchMethod, StopReason, SystemPromptTransport, }; -use crate::config::{compose_session_title, DedupMode, PermissionMode}; +use crate::config::{compose_session_title, DedupMode, PermissionMode, ResolvedPermissionConfig}; use crate::observer; use crate::queue::{ CancelReason, ContextMessage, ConversationContext, FlushBatch, PromptChannelInfo, @@ -67,6 +67,13 @@ pub struct TaskMeta { /// tasks only — all prompt tasks install a steer channel regardless /// of the agent's name. pub steer_tx: Option>, + /// Permission decision channel — delivers `permission_decision` control + /// frames from the observer dispatch loop into the read loop's decision + /// arm. `None` until the first `ask`-policy permission request arrives + /// (installed per-session by the pool dispatch path). Cloned from the + /// sender end of the channel installed on `AcpClient` via + /// `install_permission_decision_rx`. + pub permission_decision_tx: Option>, } /// Agent-level model capabilities. Populated on first session creation. @@ -543,8 +550,8 @@ pub struct PromptContext { pub context_message_limit: u32, /// Max turns per session before proactive rotation. 0 = disabled. pub max_turns_per_session: u32, - /// Permission mode to apply after session creation. `Default` = skip. - pub permission_mode: PermissionMode, + /// Resolved permission configuration — policy, effective ACP mode, and how to transmit. + pub permission_config: ResolvedPermissionConfig, /// Agent identity — used to derive the NIP-AE conversation key at /// session creation for core injection. pub agent_keys: nostr::Keys, @@ -1014,14 +1021,20 @@ async fn create_session_and_apply_model( }), ); - // Apply permission mode if not the agent's built-in default AND the agent - // advertises the requested mode in session/new. Agents that don't support - // the mode (e.g., goose crashes on unrecognized set_config_option values) - // are safely skipped — the harness rejects interactive permission requests. - if !ctx.permission_mode.is_default() - && agent_supports_mode(&resp.raw, ctx.permission_mode.as_wire_str()) + // Apply permission mode whenever the agent advertises it (including `default`). + // The `transmit_mode` flag handles any future cases where transmission should be skipped. + if ctx.permission_config.transmit_mode + && agent_supports_mode( + &resp.raw, + ctx.permission_config.effective_mode.as_wire_str(), + ) { - apply_permission_mode(&mut agent.acp, &resp.session_id, &ctx.permission_mode).await?; + apply_permission_mode( + &mut agent.acp, + &resp.session_id, + &ctx.permission_config.effective_mode, + ) + .await?; } Ok(resp.session_id) @@ -1412,6 +1425,18 @@ pub async fn run_prompt_task( turn_id.clone(), turn_started_at.clone(), )); + + // Wire permission configuration and owner-knowledge into the ACP client so + // `handle_permission_request` can evaluate the ask availability gate. These + // values come from `PromptContext` (resolved once at startup from CLI args and + // desktop-injected env vars) and are idempotent to re-apply across turns. + agent + .acp + .set_permission_config(ctx.permission_config.clone()); + agent + .acp + .set_owner_pubkey_known(ctx.agent_owner_pubkey.is_some()); + let triggering_event_ids: Vec = batch .as_ref() .map(|b| b.events.iter().map(|be| be.event.id.to_hex()).collect()) @@ -6536,7 +6561,11 @@ mod tests { ), context_message_limit: 0, max_turns_per_session: 0, - permission_mode: PermissionMode::Default, + permission_config: ResolvedPermissionConfig::resolve( + crate::config::PermissionPolicy::Reject, + None, + ) + .expect("test config"), agent_keys: agent_keys.clone(), agent_owner_pubkey: owner_pubkey, memory_enabled: false, diff --git a/crates/buzz-backend-kubernetes/tests/fixtures/provider-wire/deploy-full-launch.request.json b/crates/buzz-backend-kubernetes/tests/fixtures/provider-wire/deploy-full-launch.request.json index beffc29440..8c243a0c34 100644 --- a/crates/buzz-backend-kubernetes/tests/fixtures/provider-wire/deploy-full-launch.request.json +++ b/crates/buzz-backend-kubernetes/tests/fixtures/provider-wire/deploy-full-launch.request.json @@ -25,6 +25,7 @@ "BUZZ_ACP_DISPLAY_NAME": "worker", "BUZZ_ACP_LAZY_POOL": "true", "BUZZ_ACP_MODEL": "gpt-5", + "BUZZ_ACP_PERMISSION_POLICY": "ask", "BUZZ_ACP_RELAY_OBSERVER": "true", "BUZZ_ACP_SESSION_TITLE": "worker", "GOOSE_MODE": "auto" diff --git a/desktop/src-tauri/src/commands/agent_config_tests.rs b/desktop/src-tauri/src/commands/agent_config_tests.rs index 5519153578..119eac8c43 100644 --- a/desktop/src-tauri/src/commands/agent_config_tests.rs +++ b/desktop/src-tauri/src/commands/agent_config_tests.rs @@ -116,6 +116,7 @@ fn agent_record() -> ManagedAgentRecord { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, agent_command_override: None, persona_source_version: None, provider: None, diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index 4704582372..94a464e30a 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -852,6 +852,18 @@ pub async fn update_managed_agent( record.respond_to_allowlist = prospective_allowlist; } + // Per-agent permission policy. `None` = clear override; remote agents are read-only. + if let Some(policy_opt) = input.permission_policy { + if matches!( + record.backend, + crate::managed_agents::BackendKind::Provider { .. } + ) && record.backend_agent_id.is_some() + { + return Err("permission_policy is read-only while the agent is deployed remotely; shut down and redeploy to change it".to_string()); + } + record.permission_policy = policy_opt; + } + record.updated_at = now_iso(); save_managed_agents(&app, &records)?; diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index dd61fc9398..23cf7b0b75 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -913,6 +913,7 @@ pub async fn create_managed_agent( } else { relay_mesh.clone() }, + permission_policy: None, // inherits global default or built-in `ask` }; records.push(record); diff --git a/desktop/src-tauri/src/commands/agents_deploy.rs b/desktop/src-tauri/src/commands/agents_deploy.rs index 47ee5f92d4..8e425dd079 100644 --- a/desktop/src-tauri/src/commands/agents_deploy.rs +++ b/desktop/src-tauri/src/commands/agents_deploy.rs @@ -46,6 +46,10 @@ pub(crate) fn resolve_deploy_model_provider( /// `descriptor.env` is the authoritative six-layer environment. Policy values /// are deliberately separate because providers apply them below that layered /// environment, preserving the local spawn's power-user override semantics. +/// +/// `effective_permission_policy` is the already-resolved per-agent → global → +/// built-in policy. Pass it from the caller so that this function does not need +/// the global config; tests can pass `None` to get the built-in default. pub(super) fn build_launch_block( record: &ManagedAgentRecord, descriptor: &crate::managed_agents::readiness::EffectiveHarnessDescriptor, @@ -53,6 +57,7 @@ pub(super) fn build_launch_block( effective_prompt: Option<&str>, effective_model: Option<&str>, owner_pubkey: &str, + effective_permission_policy: Option, ) -> serde_json::Value { use crate::managed_agents::{ known_acp_runtime, resolve_session_title, DISPLAY_NAME_ENV_VAR, SESSION_TITLE_ENV_VAR, @@ -101,6 +106,20 @@ pub(super) fn build_launch_block( policy_env.insert("BUZZ_ACP_TEAM_INSTRUCTIONS".into(), value); } + // Permission policy: use the caller-resolved value (per-agent → global → + // built-in), falling back to the built-in default if the caller did not + // provide one. Tests pass `None`; production callers pass the result of + // `resolve_effective_permission_policy(record, global_config)`. + { + let policy = effective_permission_policy.unwrap_or_else( + crate::managed_agents::permission_policy::PermissionPolicy::desktop_default, + ); + policy_env.insert( + "BUZZ_ACP_PERMISSION_POLICY".into(), + policy.as_str().to_string(), + ); + } + serde_json::json!({ "command": descriptor.command, "args": descriptor.args, @@ -149,6 +168,10 @@ pub(super) fn build_deploy_payload( crate::managed_agents::resolve_effective_harness_descriptor(record, &personas, &global) .map_err(|error| crate::managed_agents::user_facing_harness_error(&error))?; let owner_pubkey = super::workspace_owner_hex(state)?; + let (effective_policy, _) = + crate::managed_agents::permission_policy::resolve_effective_permission_policy( + record, &global, + ); let launch = build_launch_block( record, &descriptor, @@ -156,6 +179,7 @@ pub(super) fn build_deploy_payload( effective.system_prompt.value.as_deref(), effective.model.value.as_deref(), &owner_pubkey, + Some(effective_policy), ); let effective_parallelism = @@ -267,6 +291,7 @@ mod tests { Some("prompt"), Some("model"), "owner-hex", + None, ); assert_eq!(launch["command"], "goose"); @@ -305,7 +330,7 @@ mod tests { env: BTreeMap::new(), }; - let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex"); + let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex", None); assert_eq!( launch["policy_env"]["BUZZ_ACP_AGENTS"], @@ -327,7 +352,7 @@ mod tests { env: BTreeMap::new(), }; - let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex"); + let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex", None); assert_eq!( launch["policy_env"]["BUZZ_ACP_AGENTS"], "8", @@ -357,7 +382,7 @@ mod tests { }; let cap = crate::managed_agents::parallelism::OPENCLAW_MAX_PARALLELISM; - let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex"); + let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex", None); let effective_parallelism = crate::managed_agents::effective_parallelism(&descriptor.command, record.parallelism); let payload = deploy_payload_json( @@ -402,7 +427,7 @@ mod tests { env: BTreeMap::new(), }; - let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex"); + let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex", None); let effective_parallelism = crate::managed_agents::effective_parallelism(&descriptor.command, record.parallelism); let payload = deploy_payload_json( @@ -448,7 +473,7 @@ mod tests { }; let cap = crate::managed_agents::parallelism::OPENCLAW_MAX_PARALLELISM; - let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex"); + let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex", None); let effective_parallelism = crate::managed_agents::effective_parallelism(&descriptor.command, record.parallelism); let payload = deploy_payload_json( @@ -475,4 +500,78 @@ mod tests { "legacy top-level parallelism must match launch.policy_env — both must be {cap}" ); } + + /// `build_launch_block` with an explicit `allow` policy injects `allow`. + #[test] + fn launch_block_explicit_allow_policy_injected() { + let record = record(); + let descriptor = EffectiveHarnessDescriptor { + command: "goose".into(), + args: vec![], + env: BTreeMap::new(), + }; + let launch = build_launch_block( + &record, + &descriptor, + &[], + None, + None, + "owner-hex", + Some(crate::managed_agents::permission_policy::PermissionPolicy::Allow), + ); + assert_eq!( + launch["policy_env"]["BUZZ_ACP_PERMISSION_POLICY"], "allow", + "explicit allow policy must be injected into policy_env" + ); + } + + /// `build_launch_block` with `None` (test callers / no global) falls back to + /// the built-in desktop default (`ask`). + #[test] + fn launch_block_none_policy_falls_back_to_built_in_ask() { + let record = record(); + let descriptor = EffectiveHarnessDescriptor { + command: "goose".into(), + args: vec![], + env: BTreeMap::new(), + }; + let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex", None); + assert_eq!( + launch["policy_env"]["BUZZ_ACP_PERMISSION_POLICY"], "ask", + "None effective_permission_policy must fall back to built-in ask" + ); + } + + /// Production deploy path: global `allow` override is respected when the + /// record has no per-agent policy, matching the local-spawn resolver. + #[test] + fn launch_block_global_allow_policy_used_when_record_has_none() { + let mut record = record(); + record.permission_policy = None; + let descriptor = EffectiveHarnessDescriptor { + command: "goose".into(), + args: vec![], + env: BTreeMap::new(), + }; + let mut global = crate::managed_agents::global_config::GlobalAgentConfig::default(); + global.permission_policy = + Some(crate::managed_agents::permission_policy::PermissionPolicy::Allow); + let (effective_policy, _) = + crate::managed_agents::permission_policy::resolve_effective_permission_policy( + &record, &global, + ); + let launch = build_launch_block( + &record, + &descriptor, + &[], + None, + None, + "owner-hex", + Some(effective_policy), + ); + assert_eq!( + launch["policy_env"]["BUZZ_ACP_PERMISSION_POLICY"], "allow", + "global allow policy must be injected when record has no per-agent policy" + ); + } } diff --git a/desktop/src-tauri/src/commands/agents_tests.rs b/desktop/src-tauri/src/commands/agents_tests.rs index 54a03e2bab..ec52fc1832 100644 --- a/desktop/src-tauri/src/commands/agents_tests.rs +++ b/desktop/src-tauri/src/commands/agents_tests.rs @@ -58,6 +58,7 @@ fn bare_agent_record( source_team_persona_slug: None, catalog_source: None, relay_mesh: None, + permission_policy: None, auto_restart_on_config_change: false, definition_respond_to: None, definition_respond_to_allowlist: vec![], @@ -482,6 +483,7 @@ fn deploy_payload_matches_the_shared_full_launch_fixture() { None, Some("gpt-5"), "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + None, ); let agent = deploy_payload_json( &record, diff --git a/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs b/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs index 8ff7cfbd9b..b33c7feaa9 100644 --- a/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs +++ b/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs @@ -66,6 +66,7 @@ fn make_agent( source_team_persona_slug: None, catalog_source: None, relay_mesh: None, + permission_policy: None, auto_restart_on_config_change: false, definition_respond_to: None, definition_respond_to_allowlist: vec![], diff --git a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs index 1005a83432..327aef8a5b 100644 --- a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs @@ -215,6 +215,7 @@ fn local_agent() -> ManagedAgentRecord { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs b/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs index b769d74d7b..49d828b1e6 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs @@ -64,6 +64,7 @@ fn make_definition(slug: &str) -> ManagedAgentRecord { definition_respond_to_allowlist: vec![], definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/commands/personas/snapshot/import.rs b/desktop/src-tauri/src/commands/personas/snapshot/import.rs index d7f0323304..f4a4201029 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/import.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/import.rs @@ -654,6 +654,7 @@ pub async fn confirm_agent_snapshot_import( relay_mesh: None, runtime: snapshot.definition.runtime.clone(), name_pool: snapshot.definition.name_pool.clone(), + permission_policy: None, }; records.push(record.clone()); diff --git a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs index c453b09a9d..8e000e926c 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs @@ -73,6 +73,7 @@ fn make_definition(slug: &str) -> ManagedAgentRecord { definition_respond_to_allowlist: vec![], definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs b/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs index c60215ae4d..9a066156e3 100644 --- a/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs +++ b/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs @@ -58,6 +58,7 @@ fn agent(persona_id: &str, name: &str, display_name: Option<&str>) -> ManagedAge definition_respond_to_allowlist: vec![], definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/commands/team_snapshot.rs b/desktop/src-tauri/src/commands/team_snapshot.rs index 97cd11933d..c8990738fd 100644 --- a/desktop/src-tauri/src/commands/team_snapshot.rs +++ b/desktop/src-tauri/src/commands/team_snapshot.rs @@ -609,6 +609,7 @@ pub async fn confirm_team_snapshot_import( definition_respond_to_allowlist: definition.respond_to_allowlist.clone(), definition_parallelism: minted_parallelism, relay_mesh: None, + permission_policy: None, runtime: member.definition.runtime.clone(), name_pool: member.definition.name_pool.clone(), }; diff --git a/desktop/src-tauri/src/commands/team_snapshot/tests.rs b/desktop/src-tauri/src/commands/team_snapshot/tests.rs index c9a6d8812a..3ec5ec16a8 100644 --- a/desktop/src-tauri/src/commands/team_snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/team_snapshot/tests.rs @@ -229,6 +229,7 @@ fn team_export_with_instance_and_memory_level_uses_supplied_entries() { definition_respond_to_allowlist: vec![], definition_parallelism: None, relay_mesh: None, + permission_policy: None, runtime: None, name_pool: vec![], }; diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index 4a7b80079d..13f75c2eff 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -216,6 +216,7 @@ mod tests { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs index 8508c27073..e553916c88 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs @@ -416,6 +416,7 @@ mod tests { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, agent_command_override: None, persona_source_version: None, provider: None, diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs index b4492418e5..fc9759657c 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs @@ -72,6 +72,7 @@ fn minimal_record() -> ManagedAgentRecord { definition_respond_to_allowlist: vec!["abc123def".to_string()], definition_parallelism: Some(4), relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs index 62caffeb2e..08ce59cf5a 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs @@ -115,6 +115,7 @@ fn test_record() -> ManagedAgentRecord { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, agent_command_override: None, persona_source_version: None, provider: None, diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 6fe6a77521..1fd0c95ef1 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -283,6 +283,7 @@ fn record_with( definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/managed_agents/effective_config/tests.rs b/desktop/src-tauri/src/managed_agents/effective_config/tests.rs index c8e437809c..230b945644 100644 --- a/desktop/src-tauri/src/managed_agents/effective_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/effective_config/tests.rs @@ -88,6 +88,7 @@ fn record( source_team_persona_slug: None, catalog_source: None, relay_mesh: None, + permission_policy: None, auto_restart_on_config_change: false, definition_respond_to: None, definition_respond_to_allowlist: vec![], diff --git a/desktop/src-tauri/src/managed_agents/global_config/mod.rs b/desktop/src-tauri/src/managed_agents/global_config/mod.rs index 162f447981..50e3082394 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/mod.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/mod.rs @@ -70,6 +70,14 @@ pub struct GlobalAgentConfig { /// Preferred ACP runtime for definitions without an explicit runtime. #[serde(default)] pub preferred_runtime: Option, + /// Fleet-wide permission policy default. `None` = use the built-in + /// desktop default (`ask`). Per-agent `permission_policy` takes precedence. + /// + /// Semantics match the per-agent field: `ask` shows the Allow/Deny card, + /// `allow` auto-approves the unique `allow_once` option (explicit opt-in + /// only), `reject` auto-denies. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub permission_policy: Option, } /// Validate a `GlobalAgentConfig` before persisting it. diff --git a/desktop/src-tauri/src/managed_agents/global_config/tests.rs b/desktop/src-tauri/src/managed_agents/global_config/tests.rs index 553596e226..d9eb6f4c1d 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/tests.rs @@ -267,6 +267,7 @@ fn roundtrip_serialization() { provider: Some("anthropic".to_string()), model: Some("claude-opus-4".to_string()), preferred_runtime: Some("claude".to_string()), + permission_policy: None, }; let json = serde_json::to_string(&config).expect("serialize"); let back: GlobalAgentConfig = serde_json::from_str(&json).expect("deserialize"); @@ -348,6 +349,7 @@ fn bare_record() -> ManagedAgentRecord { source_team_persona_slug: None, catalog_source: None, relay_mesh: None, + permission_policy: None, auto_restart_on_config_change: false, definition_respond_to: None, definition_respond_to_allowlist: vec![], @@ -592,6 +594,7 @@ fn populated_global_config_round_trips() { provider: Some("anthropic".to_string()), model: Some("claude-opus-4-5".to_string()), preferred_runtime: None, + permission_policy: None, }; let json = serde_json::to_string(&original).expect("serialization must not fail"); let decoded: GlobalAgentConfig = diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index fe90ce430f..a0f9c6f9f5 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -3,6 +3,7 @@ mod agent_env; pub(crate) mod agent_events; pub(crate) mod agent_snapshot; pub(crate) mod agent_snapshot_envelope; +pub(crate) mod permission_policy; pub(crate) mod team_snapshot; pub(crate) use access_policy::{owner_only, owner_only_access_build, projected_access_with_policy}; pub(crate) use agent_env::{ diff --git a/desktop/src-tauri/src/managed_agents/nest/tests.rs b/desktop/src-tauri/src/managed_agents/nest/tests.rs index cbef171f6f..1288c5ceb3 100644 --- a/desktop/src-tauri/src/managed_agents/nest/tests.rs +++ b/desktop/src-tauri/src/managed_agents/nest/tests.rs @@ -502,6 +502,7 @@ fn make_agent(name: &str, persona_id: Option<&str>) -> ManagedAgentRecord { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/managed_agents/parallelism.rs b/desktop/src-tauri/src/managed_agents/parallelism.rs index e1691575b1..0253d48158 100644 --- a/desktop/src-tauri/src/managed_agents/parallelism.rs +++ b/desktop/src-tauri/src/managed_agents/parallelism.rs @@ -117,6 +117,7 @@ mod tests { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/managed_agents/permission_policy.rs b/desktop/src-tauri/src/managed_agents/permission_policy.rs new file mode 100644 index 0000000000..cc6fb92f01 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/permission_policy.rs @@ -0,0 +1,154 @@ +//! Permission policy enum, source attribution, and the precedence resolver. +//! +//! `BUZZ_ACP_PERMISSION_POLICY` is in `RESERVED_ENV_KEYS` so users cannot +//! override it via the env-vars UI — a manual override would make the running +//! harness use a different policy than the saved/UI-visible setting. + +use serde::{Deserialize, Serialize}; + +use super::types::ManagedAgentRecord; + +/// How the agent answers `session/request_permission` requests. +/// +/// - `Ask` — show an Allow/Deny card; auto-deny after 300 s (desktop default). +/// - `Allow` — auto-select the unique `allow_once` option; explicit opt-in. +/// - `Reject` — deny immediately; headless/CLI default. +/// +/// Wire format is lowercase to match the harness CLI vocabulary and the +/// `BUZZ_ACP_PERMISSION_POLICY` env var the harness reads. +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "lowercase")] +pub enum PermissionPolicy { + Ask, + Allow, + Reject, +} + +impl PermissionPolicy { + /// The env-var wire string consumed by the harness + /// (`BUZZ_ACP_PERMISSION_POLICY`). + pub fn as_str(self) -> &'static str { + match self { + Self::Ask => "ask", + Self::Allow => "allow", + Self::Reject => "reject", + } + } + + /// The built-in desktop default: show the Allow/Deny card. + /// + /// Headless / bare-CLI callers use `Reject` — they never have a UI to + /// answer a card. The desktop injects the resolved effective policy so + /// headless sessions spawned by the desktop still pick up the user's + /// choice. + pub fn desktop_default() -> Self { + Self::Ask + } +} + +/// Where the effective [`PermissionPolicy`] came from. Serialized as a +/// `snake_case` string for TypeScript's exhaustive-switch pattern. +#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum PermissionPolicySource { + /// Set explicitly on this agent record. + Agent, + /// Inherited from the global agent config. + GlobalDefault, + /// Neither per-agent nor global is set; using the built-in desktop default. + BuiltIn, +} + +/// Resolve the effective permission policy for an agent. +/// +/// Precedence (highest first): +/// 1. `record.permission_policy` — per-agent override. +/// 2. `global.permission_policy` — fleet-wide default. +/// 3. [`PermissionPolicy::desktop_default`] — built-in. +pub fn resolve_effective_permission_policy( + record: &ManagedAgentRecord, + global: &super::global_config::GlobalAgentConfig, +) -> (PermissionPolicy, PermissionPolicySource) { + if let Some(policy) = record.permission_policy { + return (policy, PermissionPolicySource::Agent); + } + if let Some(policy) = global.permission_policy { + return (policy, PermissionPolicySource::GlobalDefault); + } + ( + PermissionPolicy::desktop_default(), + PermissionPolicySource::BuiltIn, + ) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::managed_agents::global_config::GlobalAgentConfig; + + fn empty_record() -> ManagedAgentRecord { + serde_json::from_value(serde_json::json!({ + "pubkey": "abcd1234", + "name": "test", + "display_name": "Test", + "private_key_nsec": "nsec1fake", + "relay_url": "wss://relay.example", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 300, + "idle_timeout_seconds": 900, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" + })) + .expect("minimal ManagedAgentRecord") + } + + #[test] + fn test_per_agent_policy_beats_global_and_built_in() { + let mut record = empty_record(); + record.permission_policy = Some(PermissionPolicy::Allow); + let mut global = GlobalAgentConfig::default(); + global.permission_policy = Some(PermissionPolicy::Reject); + + let (policy, source) = resolve_effective_permission_policy(&record, &global); + assert_eq!(policy, PermissionPolicy::Allow); + assert_eq!(source, PermissionPolicySource::Agent); + } + + #[test] + fn test_global_policy_beats_built_in_when_no_per_agent() { + let mut record = empty_record(); + record.permission_policy = None; + let mut global = GlobalAgentConfig::default(); + global.permission_policy = Some(PermissionPolicy::Allow); + + let (policy, source) = resolve_effective_permission_policy(&record, &global); + assert_eq!(policy, PermissionPolicy::Allow); + assert_eq!(source, PermissionPolicySource::GlobalDefault); + } + + #[test] + fn test_built_in_used_when_neither_per_agent_nor_global_is_set() { + let mut record = empty_record(); + record.permission_policy = None; + let global = GlobalAgentConfig::default(); // permission_policy = None + + let (policy, source) = resolve_effective_permission_policy(&record, &global); + assert_eq!(policy, PermissionPolicy::Ask); // desktop_default + assert_eq!(source, PermissionPolicySource::BuiltIn); + } + + #[test] + fn test_per_agent_reject_beats_global_allow() { + let mut record = empty_record(); + record.permission_policy = Some(PermissionPolicy::Reject); + let mut global = GlobalAgentConfig::default(); + global.permission_policy = Some(PermissionPolicy::Allow); + + let (policy, source) = resolve_effective_permission_policy(&record, &global); + assert_eq!(policy, PermissionPolicy::Reject); + assert_eq!(source, PermissionPolicySource::Agent); + } +} diff --git a/desktop/src-tauri/src/managed_agents/persona_events/tests.rs b/desktop/src-tauri/src/managed_agents/persona_events/tests.rs index 0580b12ce2..91110836b4 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events/tests.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events/tests.rs @@ -58,6 +58,7 @@ pub(super) fn sample_record() -> ManagedAgentRecord { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index c072448ff1..041ad029a4 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -1530,6 +1530,7 @@ mod tests { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, }; let runtime = known_acp_runtime_exact("buzz-agent"); diff --git a/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs b/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs index 8698d3a51d..3a972ed849 100644 --- a/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs +++ b/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs @@ -70,6 +70,11 @@ pub(crate) const RESERVED_ENV_KEYS: &[&str] = &[ // for same-session sweep decisions. "BUZZ_MANAGED_AGENT", "BUZZ_MANAGED_AGENT_START_NONCE", + // Permission policy gate: Desktop resolves the effective policy + // (per-agent > global > built-in) and injects it here. A user-supplied + // override would make the running harness use a different policy than the + // saved/UI-visible setting — exactly the truthfulness failure #4938 fixes. + "BUZZ_ACP_PERMISSION_POLICY", ]; pub(crate) fn is_reserved_env_key(key: &str) -> bool { diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index ec804869c4..adb2327947 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -7,9 +7,10 @@ use super::agent_env::build_buzz_agent_provider_defaults; use crate::{ managed_agents::{ append_log_marker, known_acp_runtime, login_shell_path, managed_agent_log_path, - missing_command_message, normalize_agent_args, open_log_file, resolve_command, - spawn_key_refusal, KnownAcpRuntime, ManagedAgentPairRuntime, ManagedAgentRecord, - ManagedAgentRuntimeKey, ManagedAgentSummary, + missing_command_message, normalize_agent_args, open_log_file, + permission_policy::resolve_effective_permission_policy, resolve_command, spawn_key_refusal, + KnownAcpRuntime, ManagedAgentPairRuntime, ManagedAgentRecord, ManagedAgentRuntimeKey, + ManagedAgentSummary, }, util::now_iso, }; @@ -296,6 +297,9 @@ pub fn build_managed_agent_summary( .unwrap_or("") .to_string(); + let (effective_permission_policy_summary, effective_permission_policy_source) = + resolve_effective_permission_policy(record, global_config); + Ok(ManagedAgentSummary { pubkey: record.pubkey.clone(), name: record.name.clone(), @@ -338,6 +342,8 @@ pub fn build_managed_agent_summary( log_path, respond_to: record.respond_to, respond_to_allowlist: record.respond_to_allowlist.clone(), + permission_policy: effective_permission_policy_summary, + permission_policy_source: effective_permission_policy_source, }) } @@ -761,6 +767,14 @@ pub fn spawn_agent_child( command.env_remove(key); } + // Inject BUZZ_ACP_PERMISSION_POLICY — resolved here so the running process + // and the UI-visible setting are always in sync. + let (effective_permission_policy, _) = resolve_effective_permission_policy(record, &global); + command.env( + "BUZZ_ACP_PERMISSION_POLICY", + effective_permission_policy.as_str(), + ); + command.env("BUZZ_ACP_RELAY_OBSERVER", "true"); // ── Git credential helper for Buzz relay ────────────────────────── @@ -844,6 +858,7 @@ pub fn spawn_agent_child( system_prompt: effective_prompt.as_deref(), model: effective_model.as_deref(), provider: effective_provider.as_deref(), + permission_policy: effective_permission_policy, }, ); diff --git a/desktop/src-tauri/src/managed_agents/runtime/test_fixtures.rs b/desktop/src-tauri/src/managed_agents/runtime/test_fixtures.rs index 9836d983ed..70ed9db6ab 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/test_fixtures.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/test_fixtures.rs @@ -89,5 +89,6 @@ pub(super) fn fixture( definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/managed_agents/spawn_snapshot.rs b/desktop/src-tauri/src/managed_agents/spawn_snapshot.rs index ba2129c984..238b263f6f 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_snapshot.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_snapshot.rs @@ -72,6 +72,8 @@ pub(crate) struct SpawnConfigInputs<'a> { pub system_prompt: Option<&'a str>, pub model: Option<&'a str>, pub provider: Option<&'a str>, + /// Resolved effective permission policy (per-agent > global > built-in). + pub permission_policy: super::permission_policy::PermissionPolicy, } /// The effective spawn configuration of one managed-agent process. @@ -123,6 +125,10 @@ pub(crate) struct SpawnConfigSnapshot { pub idle_timeout_seconds: Option, pub max_turn_duration_seconds: Option, pub parallelism: u32, + /// Effective permission policy at spawn time. Reaches the harness via + /// `BUZZ_ACP_PERMISSION_POLICY`. Tracked in the snapshot so an edit shows + /// in the `needsRestart` diff. + pub permission_policy: String, } impl SpawnConfigSnapshot { @@ -136,6 +142,7 @@ impl SpawnConfigSnapshot { system_prompt, model, provider, + permission_policy, } = inputs; Self { acp_command: record.acp_command.clone(), @@ -174,6 +181,7 @@ impl SpawnConfigSnapshot { // pool and must badge. The diff surface consequently displays the // effective value — that is correct, it is what actually runs. parallelism: super::effective_parallelism(&descriptor.command, record.parallelism), + permission_policy: permission_policy.as_str().to_string(), } } @@ -262,6 +270,10 @@ pub(crate) fn prospective_spawn_config_snapshot( system_prompt: prompt.as_deref(), model: model.as_deref(), provider: provider.as_deref(), + permission_policy: super::permission_policy::resolve_effective_permission_policy( + record, global, + ) + .0, }) } diff --git a/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff/tests.rs b/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff/tests.rs index a7a8cab93e..ca9999f17e 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff/tests.rs @@ -28,6 +28,7 @@ fn base() -> SpawnConfigSnapshot { idle_timeout_seconds: Some(600), max_turn_duration_seconds: Some(7200), parallelism: 1, + permission_policy: "ask".into(), } } @@ -70,6 +71,9 @@ fn mutations() -> Vec { s.max_turn_duration_seconds = None }), ("parallelism", |s| s.parallelism = 8), + ("permission_policy", |s| { + s.permission_policy = "allow".into() + }), ] } diff --git a/desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs b/desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs index 1ceeee372f..00acfe7bde 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs @@ -70,6 +70,7 @@ fn record() -> ManagedAgentRecord { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/managed_agents/team_snapshot.rs b/desktop/src-tauri/src/managed_agents/team_snapshot.rs index 96082acc76..9db79e4c03 100644 --- a/desktop/src-tauri/src/managed_agents/team_snapshot.rs +++ b/desktop/src-tauri/src/managed_agents/team_snapshot.rs @@ -309,6 +309,7 @@ mod tests { definition_respond_to_allowlist: vec![], definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/managed_agents/teams_tests.rs b/desktop/src-tauri/src/managed_agents/teams_tests.rs index 1ffa60eda9..4d4297ee27 100644 --- a/desktop/src-tauri/src/managed_agents/teams_tests.rs +++ b/desktop/src-tauri/src/managed_agents/teams_tests.rs @@ -213,6 +213,7 @@ fn managed_agent(name: &str) -> ManagedAgentRecord { source_team_persona_slug: None, catalog_source: None, relay_mesh: None, + permission_policy: None, definition_respond_to: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index e5be105fed..6cd7366383 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -1,6 +1,5 @@ use serde::{Deserialize, Serialize}; use std::{collections::BTreeMap, path::PathBuf, process::Child}; - #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] #[serde(tag = "type", rename_all = "snake_case")] pub enum BackendKind { @@ -153,6 +152,7 @@ impl AgentDefinition { definition_respond_to_allowlist: self.respond_to_allowlist, definition_parallelism: self.parallelism, relay_mesh: None, + permission_policy: None, } } } @@ -352,6 +352,8 @@ pub struct ManagedAgentRecord { /// Preserved across mode toggles so users don't lose state. #[serde(default)] pub respond_to_allowlist: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub permission_policy: Option, /// Optional display name distinct from the unique `name` handle. Absorbed /// from `AgentDefinition.display_name` (unified agent model, Phase 1A). #[serde(default, skip_serializing_if = "Option::is_none")] @@ -566,6 +568,8 @@ pub struct ManagedAgentSummary { pub log_path: String, pub respond_to: RespondTo, pub respond_to_allowlist: Vec, + pub permission_policy: super::permission_policy::PermissionPolicy, + pub permission_policy_source: super::permission_policy::PermissionPolicySource, } #[derive(Debug, Serialize)] diff --git a/desktop/src-tauri/src/managed_agents/types/requests.rs b/desktop/src-tauri/src/managed_agents/types/requests.rs index e28b0bd461..ae90375297 100644 --- a/desktop/src-tauri/src/managed_agents/types/requests.rs +++ b/desktop/src-tauri/src/managed_agents/types/requests.rs @@ -253,6 +253,12 @@ pub struct UpdateManagedAgentRequest { /// normalized server-side). #[serde(default)] pub respond_to_allowlist: Option>, + /// Absent = don't touch. `null` = clear per-agent override (revert to + /// global/built-in). Present string = set per-agent override. + /// Remote deployed agents: rejected server-side (displayed read-only in UI). + #[serde(default, deserialize_with = "crate::util::double_option")] + pub permission_policy: + Option>, } #[cfg(test)] diff --git a/desktop/src-tauri/src/managed_agents/types/tests.rs b/desktop/src-tauri/src/managed_agents/types/tests.rs index 1db7b9b524..914568cf12 100644 --- a/desktop/src-tauri/src/managed_agents/types/tests.rs +++ b/desktop/src-tauri/src/managed_agents/types/tests.rs @@ -744,6 +744,9 @@ fn summary_fixture( log_path: String::new(), respond_to: RespondTo::OwnerOnly, respond_to_allowlist: Vec::new(), + permission_policy: crate::managed_agents::permission_policy::PermissionPolicy::Ask, + permission_policy_source: + crate::managed_agents::permission_policy::PermissionPolicySource::BuiltIn, } } diff --git a/desktop/src/features/agents/ui/AgentConfigFields.tsx b/desktop/src/features/agents/ui/AgentConfigFields.tsx index 11f68e8a56..67b5513bfd 100644 --- a/desktop/src/features/agents/ui/AgentConfigFields.tsx +++ b/desktop/src/features/agents/ui/AgentConfigFields.tsx @@ -68,6 +68,7 @@ export const EMPTY_GLOBAL_CONFIG: GlobalAgentConfig = { provider: null, model: null, preferred_runtime: null, + permission_policy: null, }; const BAKED_STRUCTURED_KEYS = new Set([ diff --git a/desktop/src/features/agents/ui/AgentDefaultsEditor.tsx b/desktop/src/features/agents/ui/AgentDefaultsEditor.tsx index 69e8b3a5b4..71b7b25446 100644 --- a/desktop/src/features/agents/ui/AgentDefaultsEditor.tsx +++ b/desktop/src/features/agents/ui/AgentDefaultsEditor.tsx @@ -17,7 +17,7 @@ import { getGlobalAgentConfig, setGlobalAgentConfig, } from "@/shared/api/tauriGlobalAgentConfig"; -import type { GlobalAgentConfig } from "@/shared/api/types"; +import type { GlobalAgentConfig, PermissionPolicy } from "@/shared/api/types"; import { getBakedBuildEnv, type BakedEnvEntry } from "@/shared/api/tauri"; import { globalAgentConfigQueryKey } from "@/features/agents/useGlobalAgentConfig"; import { @@ -294,6 +294,36 @@ export function AgentDefaultsEditor({ value={selectedRuntime?.id ?? ""} /> + {/* Fleet-wide permission policy default */} +
+ + +
{flatLayout ? ( {configFields ? ( diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index f7ee098833..05f7a136d0 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -15,6 +15,7 @@ import { useAgentAccessOwnerOnlyQuery } from "@/features/agents/useAgentAccessOw import { isManagedAgentActive } from "@/features/agents/lib/managedAgentControlActions"; import type { ManagedAgent, + PermissionPolicy, RespondToMode, UpdateManagedAgentInput, } from "@/shared/api/types"; @@ -160,6 +161,12 @@ export function AgentInstanceEditDialog({ const [respondToAllowlist, setRespondToAllowlist] = React.useState( agent.respondToAllowlist, ); + // `null` means "inherit from global/built-in". Local state mirrors the record's + // per-agent override field. Remote deployed agents: this is read-only. + const [permissionPolicy, setPermissionPolicy] = + React.useState( + agent.permissionPolicySource === "agent" ? agent.permissionPolicy : null, + ); const [showAdvancedFields, setShowAdvancedFields] = React.useState(false); const [avatarUrl, setAvatarUrl] = React.useState(agent.avatarUrl ?? ""); const [isAvatarUploadPending, setIsAvatarUploadPending] = @@ -196,6 +203,11 @@ export function AgentInstanceEditDialog({ setAutoRestartOnConfigChange(agent.autoRestartOnConfigChange); setRespondTo(agent.respondTo); setRespondToAllowlist(agent.respondToAllowlist); + setPermissionPolicy( + agent.permissionPolicySource === "agent" + ? agent.permissionPolicy + : null, + ); setAvatarUrl(agent.avatarUrl ?? ""); setShowAdvancedFields(false); setIsAvatarUploadPending(false); @@ -725,6 +737,15 @@ export function AgentInstanceEditDialog({ respondToAllowlist.join(",") !== agent.respondToAllowlist.join(",") ? respondToAllowlist : undefined, + // `null` = clear the per-agent override (revert to global/built-in). + // `undefined` = don't touch. Only include when the value actually changed. + permissionPolicy: (() => { + const saved = + agent.permissionPolicySource === "agent" + ? agent.permissionPolicy + : null; + return permissionPolicy !== saved ? permissionPolicy : undefined; + })(), }; const result = await updateMutation.mutateAsync(input); @@ -944,6 +965,63 @@ export function AgentInstanceEditDialog({ onAllowlistChange={setRespondToAllowlist} onModeChange={setRespondTo} /> + {/* Permission policy */} + {(() => { + const isRemoteDeployed = + agent.backend.type === "provider" && + agent.backendAgentId !== null; + const sourceLabelMap: Record = { + agent: "agent override", + global_default: "global default", + built_in: "built-in", + }; + const sourceLabel = + sourceLabelMap[agent.permissionPolicySource] ?? + agent.permissionPolicySource; + return ( +
+
+ + + ({agent.permissionPolicy} · from {sourceLabel}) + +
+ {isRemoteDeployed ? ( +

+ Read-only while deployed. To change, shut down and + redeploy the agent. +

+ ) : ( + + )} +
+ ); + })()} {/* Provider (runtime) */} diff --git a/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx b/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx index 53a00e6463..4bafe00709 100644 --- a/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx +++ b/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx @@ -1,5 +1,7 @@ import { AlertCircle, CheckCircle2, ShieldCheck, XCircle } from "lucide-react"; +import * as React from "react"; +import { sendPermissionDecision } from "@/shared/api/agentControl"; import { formatTranscriptTimestampTitle } from "../agentSessionUtils"; import { ActivityRow, ActivityRowLabel } from "./ActivityRow"; import { ToolActivity } from "./ToolActivity"; @@ -29,7 +31,7 @@ function splitPermissionText(text: string): { /** * Derive the visual tone and icon for a resolved permission outcome string. * Outcome strings come from describePermissionOutcome: - * "Approved (...)" | "Denied (...)" | "Cancelled" + * "Approved (...)" | "Denied (...)" | "Cancelled" | "uncertain" pinned copy */ function permissionOutcomeTone(outcome: string): "approve" | "deny" | "cancel" { if (outcome.startsWith("Approved")) return "approve"; @@ -37,6 +39,86 @@ function permissionOutcomeTone(outcome: string): "approve" | "deny" | "cancel" { return "cancel"; } +/** + * Allow/Deny buttons for an actionable permission card. + * Renders the agent's exact options as labeled buttons; a click sends the + * `permission_decision` control event (fire-and-forget). + * + * On send failure (relay reject or non-`sent` delivery status), buttons are + * re-enabled so the user can retry. The harness's 300 s fail-closed timeout + * is the backstop for permanently lost frames. + */ +function PermissionDecisionButtons({ + agentPubkey, + channelId, + options, + requestNonce, + deliveryFailed, +}: { + agentPubkey: string; + channelId: string; + options: Array<{ optionId: string; kind: string; label?: string }>; + requestNonce: string; + /** + * Monotonically increasing failure token from the reducer — incremented on + * every non-`sent` `control_result`. Keying the effect on this number (not a + * boolean) ensures a second failure after a retry also re-enables buttons. + */ + deliveryFailed?: number; +}) { + const [pending, setPending] = React.useState(null); + + // Re-enable buttons when the reducer signals delivery failure (non-`sent` + // control_result status). The relay send succeeded but the harness couldn't + // route the click — the user should be able to retry. + React.useEffect(() => { + if (deliveryFailed) { + setPending(null); + } + }, [deliveryFailed]); + + if (options.length === 0) { + return null; + } + + return ( +
+ {options.map(({ optionId, kind, label }) => { + const isDeny = kind.startsWith("reject"); + const displayLabel = label ?? (isDeny ? "Deny" : "Allow"); + return ( + + ); + })} +
+ ); +} + export function LifecycleActivity(props: ActivityRenderClassItemProps) { if (props.item.type === "tool") { return ; @@ -55,6 +137,11 @@ export function LifecycleActivity(props: ActivityRenderClassItemProps) { const { requestLines, optionsLine } = splitPermissionText(props.item.text); const outcome = props.item.outcome; const tone = outcome ? permissionOutcomeTone(outcome) : null; + const actionable = props.item.actionable ?? false; + const requestNonce = props.item.requestNonce; + const options = props.item.options ?? []; + const authorizationReason = props.item.authorizationReason; + const deliveryFailed = props.item.deliveryFailed; return (
· {requestLines} ) : null}
- {/* Row 2: options (muted sub-line) */} - {optionsLine ? ( + {/* Row 2: authorization reason (from envelope), if present */} + {authorizationReason ? ( +
{authorizationReason}
+ ) : null} + {/* Row 3: options sub-line (legacy fallback) */} + {optionsLine && !authorizationReason ? (
{optionsLine}
) : null} - {/* Row 3: decision — only when outcome is resolved */} + {/* Row 4: Allow/Deny buttons (actionable card awaiting decision) */} + {actionable && requestNonce && !outcome ? ( + + ) : null} + {/* Row 5: decision — only when outcome is resolved */} {outcome && tone ? ( <>
diff --git a/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs b/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs index b4a139eb0e..f4e520b0a6 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs +++ b/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs @@ -2077,3 +2077,318 @@ test("buildTranscript session/new bare systemPrompt field takes precedence over "_meta.systemPrompt.append must not appear when bare field is present", ); }); + +// ── authorization envelope + nonce-keyed cards ──────────────────────────────── + +/** Build an acp_read permission event with a full authorization envelope. */ +function makePermissionRequestWithAuth( + seq, + requestId, + nonce, + { actionable = true, reason, turnId = "turn-1", channelId = "ch-1" } = {}, +) { + return { + seq, + timestamp: "2026-07-01T10:00:00.000Z", + kind: "acp_read", + agentIndex: 0, + channelId, + sessionId: "session-1", + turnId, + payload: { + jsonrpc: "2.0", + id: requestId, + method: "session/request_permission", + params: { + title: "Confirm push", + toolCallId: "tool-1", + options: [ + { optionId: "allow_once", kind: "allow_once", name: "Allow" }, + { optionId: "reject_once", kind: "reject_once", name: "Reject" }, + ], + }, + }, + authorization: { requestNonce: nonce, actionable, reason }, + }; +} + +test("buildTranscript_nonce_keyed_card_is_actionable_with_options", () => { + // An acp_read with an authorization envelope should produce one card + // keyed by nonce, with actionable=true and the parsed options attached. + const transcript = buildTranscript([ + makePermissionRequestWithAuth(1, "req-n1", "nonce-abc"), + ]); + + assert.equal(transcript.length, 1); + const item = transcript[0]; + assert.equal(item.type, "lifecycle"); + assert.equal(item.renderClass, "permission"); + assert.equal(item.requestNonce, "nonce-abc"); + assert.equal(item.actionable, true); + assert.equal(item.channelId, "ch-1"); + assert.ok(Array.isArray(item.options)); + assert.equal(item.options.length, 2); + assert.equal(item.options[0].optionId, "allow_once"); + // Card is keyed by nonce, not by turn. + assert.ok( + item.id.includes("nonce-abc"), + `expected nonce in id, got ${item.id}`, + ); +}); + +test("buildTranscript_actionable_false_envelope_produces_read_only_card", () => { + const transcript = buildTranscript([ + makePermissionRequestWithAuth(1, "req-n2", "nonce-readonly", { + actionable: false, + reason: "auto-rejected: reject policy", + }), + ]); + + assert.equal(transcript.length, 1); + const item = transcript[0]; + assert.equal(item.actionable, false); + assert.equal(item.authorizationReason, "auto-rejected: reject policy"); +}); + +test("buildTranscript_concurrent_requests_same_turn_produce_separate_cards", () => { + // Two permission requests in the same turn with different nonces must each + // get their own card — nonce is the unique key. + const transcript = buildTranscript([ + makePermissionRequestWithAuth(1, "req-c1", "nonce-c1", { + turnId: "turn-1", + }), + makePermissionRequestWithAuth(2, "req-c2", "nonce-c2", { + turnId: "turn-1", + }), + ]); + + // Two distinct cards. + const cards = transcript.filter((i) => i.renderClass === "permission"); + assert.equal(cards.length, 2, "expected two separate permission cards"); + const nonces = cards.map((c) => c.requestNonce).sort(); + assert.deepEqual(nonces, ["nonce-c1", "nonce-c2"]); + // Each card id is unique. + assert.notEqual(cards[0].id, cards[1].id); +}); + +test("buildTranscript_without_auth_envelope_falls_back_to_turn_keyed_card", () => { + // A permission request without an authorization envelope (legacy / reject + // policy path) still produces a card using the turn-based key. + const transcript = buildTranscript([ + { + seq: 1, + timestamp: "2026-07-01T10:00:00.000Z", + kind: "acp_read", + agentIndex: 0, + channelId: "ch-1", + sessionId: "session-1", + turnId: "turn-legacy", + payload: { + jsonrpc: "2.0", + id: "req-leg", + method: "session/request_permission", + params: { + title: "Confirm push", + toolCallId: "tool-1", + options: [ + { optionId: "allow_once", kind: "allow_once", name: "Allow" }, + ], + }, + }, + // No authorization field. + }, + ]); + + assert.equal(transcript.length, 1); + const item = transcript[0]; + assert.equal(item.renderClass, "permission"); + assert.equal(item.requestNonce, undefined); + assert.equal(item.actionable, undefined); + // Fall-back key uses turn id. + assert.ok( + item.id.includes("turn-legacy"), + `expected turn id in fallback key, got ${item.id}`, + ); +}); + +test("buildTranscript_uncertain_outcome_uses_pinned_copy", () => { + // The 'uncertain' terminal state must use the verbatim pinned copy, never + // "denied" or "failed closed". + const transcript = buildTranscript([ + makePermissionRequest(1, "req-unc"), + makePermissionResponse(2, "req-unc", "uncertain"), + ]); + + assert.equal(transcript.length, 1); + const item = transcript[0]; + assert.equal(item.renderClass, "permission"); + assert.match( + item.outcome ?? "", + /Approval outcome unknown.*agent process stopped/i, + "uncertain must use the pinned copy", + ); + // Must not use 'denied' or 'failed closed'. + assert.doesNotMatch(item.outcome ?? "", /denied/i); + assert.doesNotMatch(item.outcome ?? "", /failed closed/i); +}); + +test("buildTranscript_timed_out_outcome_renders_correctly", () => { + const transcript = buildTranscript([ + makePermissionRequest(1, "req-to"), + makePermissionResponse(2, "req-to", "timed_out"), + ]); + + const item = transcript[0]; + assert.equal(item.renderClass, "permission"); + assert.ok(item.outcome, "timed_out should produce an outcome string"); + assert.doesNotMatch(item.outcome ?? "", /Approved/i); +}); + +test("buildTranscript_nonce_card_channelId_is_threaded_from_event", () => { + // The channelId on the card must come from the event, not a hard-coded value, + // so PermissionDecisionButtons can pass it to sendPermissionDecision. + const transcript = buildTranscript([ + makePermissionRequestWithAuth(1, "req-ch", "nonce-ch", { + channelId: "specific-channel-id", + }), + ]); + + const item = transcript[0]; + assert.equal(item.channelId, "specific-channel-id"); +}); + +test("buildTranscript_control_result_non_sent_marks_card_delivery_failed", () => { + // A `control_result` with non-`sent` status must set deliveryFailed on the + // matching card so PermissionDecisionButtons can re-enable buttons for retry. + const nonce = "nonce-delivery-fail"; + const events = [ + // First: the permission request that creates the card. + makePermissionRequestWithAuth(1, "req-df", nonce), + // Second: a control_result with non-sent status. + { + seq: 2, + timestamp: "2026-07-01T10:00:01.000Z", + kind: "control_result", + agentIndex: 0, + channelId: "ch-1", + sessionId: "session-1", + turnId: "turn-1", + payload: { + type: "permission_decision", + status: "no_active_turn", + requestNonce: nonce, + optionId: "allow_once", + }, + }, + ]; + const transcript = buildTranscript(events); + + const card = transcript.find( + (i) => i.renderClass === "permission" && i.requestNonce === nonce, + ); + assert.ok(card, "permission card must exist"); + assert.equal( + card.deliveryFailed, + 1, + "deliveryFailed must be 1 after first non-sent control_result", + ); + // Card must still be actionable so the user can retry. + assert.equal( + card.actionable, + true, + "card must remain actionable after delivery failure", + ); +}); + +test("buildTranscript_control_result_second_failure_increments_delivery_failed", () => { + // A second non-`sent` control_result must increment deliveryFailed so the + // useEffect([deliveryFailed]) dependency in PermissionDecisionButtons + // re-fires and re-enables the buttons for a second retry attempt. + const nonce = "nonce-delivery-fail-2"; + const events = [ + makePermissionRequestWithAuth(1, "req-df2", nonce), + // First failure. + { + seq: 2, + timestamp: "2026-07-01T10:00:01.000Z", + kind: "control_result", + agentIndex: 0, + channelId: "ch-1", + sessionId: "session-1", + turnId: "turn-1", + payload: { + type: "permission_decision", + status: "no_active_turn", + requestNonce: nonce, + optionId: "allow_once", + }, + }, + // Second failure (user retried; harness still unavailable). + { + seq: 3, + timestamp: "2026-07-01T10:00:02.000Z", + kind: "control_result", + agentIndex: 0, + channelId: "ch-1", + sessionId: "session-1", + turnId: "turn-1", + payload: { + type: "permission_decision", + status: "channel_closed", + requestNonce: nonce, + optionId: "allow_once", + }, + }, + ]; + const transcript = buildTranscript(events); + + const card = transcript.find( + (i) => i.renderClass === "permission" && i.requestNonce === nonce, + ); + assert.ok(card, "permission card must exist"); + assert.equal( + card.deliveryFailed, + 2, + "deliveryFailed must be 2 after two non-sent control_results — each failure must increment the token", + ); + assert.equal( + card.actionable, + true, + "card must remain actionable after second delivery failure", + ); +}); + +test("buildTranscript_control_result_sent_does_not_mark_delivery_failed", () => { + // A `control_result` with `sent` status must NOT set deliveryFailed — the + // click reached the harness successfully. + const nonce = "nonce-delivery-ok"; + const events = [ + makePermissionRequestWithAuth(1, "req-ok", nonce), + { + seq: 2, + timestamp: "2026-07-01T10:00:01.000Z", + kind: "control_result", + agentIndex: 0, + channelId: "ch-1", + sessionId: "session-1", + turnId: "turn-1", + payload: { + type: "permission_decision", + status: "sent", + requestNonce: nonce, + optionId: "allow_once", + }, + }, + ]; + const transcript = buildTranscript(events); + + const card = transcript.find( + (i) => i.renderClass === "permission" && i.requestNonce === nonce, + ); + assert.ok(card, "permission card must exist"); + assert.equal( + card.deliveryFailed, + undefined, + "deliveryFailed must not be set on sent control_result", + ); +}); diff --git a/desktop/src/features/agents/ui/agentSessionTranscript.ts b/desktop/src/features/agents/ui/agentSessionTranscript.ts index e371bf5fc3..7dfb7dd8df 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscript.ts +++ b/desktop/src/features/agents/ui/agentSessionTranscript.ts @@ -47,6 +47,13 @@ export type TranscriptState = { string, { itemId: string; optionNames: Map } >; + /** + * Maps `requestNonce` → `itemId` for actionable permission cards. + * Populated alongside `pendingPermissions` when the `authorization` envelope + * is present on the `acp_read` frame. Used by the `permission_decision` + * `control_result` handler to retire the card on any terminal outcome. + */ + pendingPermissionsByNonce: Map; continuationSeq: number; latestSessionId: string | null; }; @@ -59,6 +66,7 @@ export function createEmptyTranscriptState(): TranscriptState { sealedKeys: new Set(), triggeringEventIdsByTurn: new Map(), pendingPermissions: new Map(), + pendingPermissionsByNonce: new Map(), continuationSeq: 0, latestSessionId: null, }; @@ -79,6 +87,7 @@ type TranscriptDraft = { string, { itemId: string; optionNames: Map } >; + pendingPermissionsByNonce: Map; continuationSeq: number; latestSessionId: string | null; changed: boolean; @@ -92,6 +101,7 @@ function draftFrom(state: TranscriptState): TranscriptDraft { sealedKeys: state.sealedKeys, triggeringEventIdsByTurn: state.triggeringEventIdsByTurn, pendingPermissions: state.pendingPermissions, + pendingPermissionsByNonce: state.pendingPermissionsByNonce, continuationSeq: state.continuationSeq, latestSessionId: state.latestSessionId, changed: false, @@ -180,40 +190,47 @@ function describePermissionRequest(payload: Record) { "Permission requested"; const toolCallId = asString(params.toolCallId) ?? asString(params.tool_call_id); - const options = Array.isArray(params.options) - ? params.options - .map((option) => { - const record = asRecord(option); - return ( - asString(record.name) ?? - asString(record.kind) ?? - asString(record.optionId) - ); - }) - .filter((option): option is string => Boolean(option)) - : []; - const detail: string[] = []; - if (title !== "Permission requested") detail.push(title); - if (toolCallId) detail.push(`Tool call: ${toolCallId}`); - if (options.length > 0) detail.push(`Options: ${options.join(", ")}`); - // Build optionId → kind map for outcome labeling on the response. + // Build both the display-string list and the structured options list in + // a single pass over params.options. const optionNames = new Map(); + const structuredOptions: Array<{ + optionId: string; + kind: string; + label?: string; + }> = []; + const optionDisplayNames: string[] = []; if (Array.isArray(params.options)) { for (const option of params.options) { - const record = asRecord(option); - const optionId = asString(record.optionId); - const kind = asString(record.kind); + const rec = asRecord(option); + const optionId = asString(rec.optionId); + const kind = asString(rec.kind); + const label = asString(rec.label) ?? asString(rec.name); + const displayName = + asString(rec.name) ?? asString(rec.kind) ?? asString(rec.optionId); + if (displayName) optionDisplayNames.push(displayName); if (optionId && kind) { optionNames.set(optionId, kind); + structuredOptions.push({ + optionId, + kind, + ...(label ? { label } : {}), + }); } } } + const detail: string[] = []; + if (title !== "Permission requested") detail.push(title); + if (toolCallId) detail.push(`Tool call: ${toolCallId}`); + if (optionDisplayNames.length > 0) + detail.push(`Options: ${optionDisplayNames.join(", ")}`); + return { title, text: detail.join("\n"), optionNames, + options: structuredOptions, descriptor: { renderClass: "permission" as const, label: "Permission requested", @@ -241,6 +258,13 @@ function describePermissionOutcome( if (outcome === "cancelled") { return "Cancelled"; } + if (outcome === "timed_out") { + return "Timed out"; + } + if (outcome === "uncertain") { + // Pinned verbatim copy — must never say "denied" or "failed closed". + return "Approval outcome unknown; agent process stopped before it could continue."; + } if (outcome === "selected" && optionId) { const kind = optionNames.get(optionId) ?? optionId; const isDenial = kind.startsWith("reject"); @@ -792,7 +816,13 @@ export function processTranscriptEvent( if (method === "session/request_permission") { const request = describePermissionRequest(payload); - const itemId = `permission:${ch}:${event.turnId ?? event.seq}`; + // Key by nonce when the authorization envelope is present — this gives + // each concurrent ACP request its own card. Fall back to the turn-based + // key for legacy/non-ask paths where no nonce is emitted. + const auth = event.authorization; + const itemId = auth?.requestNonce + ? `permission:${ch}:nonce:${auth.requestNonce}` + : `permission:${ch}:${event.turnId ?? event.seq}`; upsertLifecycleItem( d, itemId, @@ -804,6 +834,26 @@ export function processTranscriptEvent( "permission_request", request.descriptor, ); + + // Attach authorization-envelope fields to the item. The `authorization` + // object is on the ObserverEvent itself (not the payload — payloads are + // raw ACP with no `_buzz` wrapper). + if (auth) { + const existing = d.itemsById.get(itemId); + if (existing?.type === "lifecycle") { + replaceItem(d, itemId, { + ...existing, + requestNonce: auth.requestNonce, + actionable: auth.actionable, + authorizationReason: auth.reason, + options: request.options, + }); + } + // Index by nonce so acp_write terminal frames can retire the card. + d.pendingPermissionsByNonce = new Map(d.pendingPermissionsByNonce); + d.pendingPermissionsByNonce.set(auth.requestNonce, itemId); + } + // Index by JSON-RPC id so the response (acp_write with result.outcome, // no method) can correlate by id rather than by turn/seq. const requestId = jsonRpcId(payload.id); @@ -1138,6 +1188,45 @@ export function processTranscriptEvent( ); } } + } else if (event.kind === "control_result") { + // `control_result` for `permission_decision` is a **delivery confirmation**, + // not a terminal outcome. Status values are: sent | no_active_turn | + // channel_full | channel_closed | no_channel. + // + // A non-"sent" status means the click did not reach the harness — mark the + // card with `deliveryFailed = true` so buttons re-enable for retry. Terminal + // outcomes (applied, denied, timed_out, cancelled, uncertain) arrive as + // enveloped acp_write frames correlated by requestNonce. + const payload = asRecord(event.payload); + const frameType = asString(payload.type); + if (frameType === "permission_decision") { + const deliveryStatus = asString(payload.status); + if (deliveryStatus !== "sent") { + // Delivery failed — find the card by nonce and mark it retryable. + const nonce = asString(payload.requestNonce); + if (nonce) { + const itemId = d.pendingPermissionsByNonce.get(nonce); + if (itemId) { + const existing = d.itemsById.get(itemId); + if ( + existing?.type === "lifecycle" && + existing.renderClass === "permission" && + existing.actionable + ) { + replaceItem(d, itemId, { + ...existing, + // Increment the failure token so the effect in + // PermissionDecisionButtons re-fires even when a prior + // failure already set deliveryFailed (a sticky boolean + // value would not change on the second failure and the + // useEffect dependency would not trigger). + deliveryFailed: (existing.deliveryFailed ?? 0) + 1, + }); + } + } + } + } + } } if (!d.changed && d.latestSessionId === state.latestSessionId) { @@ -1151,6 +1240,7 @@ export function processTranscriptEvent( sealedKeys: d.sealedKeys, triggeringEventIdsByTurn: d.triggeringEventIdsByTurn, pendingPermissions: d.pendingPermissions, + pendingPermissionsByNonce: d.pendingPermissionsByNonce, continuationSeq: d.continuationSeq, latestSessionId: d.latestSessionId, }; diff --git a/desktop/src/features/agents/ui/agentSessionTypes.ts b/desktop/src/features/agents/ui/agentSessionTypes.ts index 578f98076c..39168e0cd4 100644 --- a/desktop/src/features/agents/ui/agentSessionTypes.ts +++ b/desktop/src/features/agents/ui/agentSessionTypes.ts @@ -10,6 +10,17 @@ export type ObserverEvent = { turnId: string | null; startedAt?: string | null; payload: unknown; + /** + * Present on `acp_read` permission frames (kind === "acp_read" + method === + * "session/request_permission"). Carries the harness-level permission gate + * metadata — `requestNonce`, `actionable`, and an optional human-readable + * `reason`. Payloads are raw ACP; there is no `_buzz` wrapper field. + */ + authorization?: { + requestNonce: string; + actionable: boolean; + reason?: string; + }; }; export type ConnectionState = @@ -112,6 +123,37 @@ export type TranscriptItem = timestamp: string; descriptor?: AgentActivityDescriptor; acpSource?: TranscriptAcpSource; + /** + * Nonce from the `authorization` envelope on an `acp_read` permission + * frame. Present only on `renderClass === "permission"` items; used to + * correlate the `permission_decision` control response and to match + * incoming `control_result` frames back to this card. + */ + requestNonce?: string; + /** + * When `true`, this card is waiting for a user Allow/Deny decision. + * `false` (or absent) means the card is read-only (auto-handled, or the + * policy is not `ask`). + */ + actionable?: boolean; + /** + * Human-readable reason string from the `authorization` envelope. + * Displayed as context below the request description. + */ + authorizationReason?: string; + /** + * Parsed options from the request params, passed back for Allow/Deny + * button rendering. + */ + options?: Array<{ optionId: string; kind: string; label?: string }>; + /** + * Monotonically increasing token incremented on every `control_result` + * with a non-`sent` delivery status. The `PermissionDecisionButtons` + * component keys its re-enable effect on this value, so a second failure + * after a retry (same boolean value would not re-trigger the effect) + * still re-enables the buttons. `undefined` when no failure has occurred. + */ + deliveryFailed?: number; } & TranscriptItemIdentity) | ({ id: string; diff --git a/desktop/src/features/agents/useGlobalAgentConfig.ts b/desktop/src/features/agents/useGlobalAgentConfig.ts index 4b90beb43d..294427742e 100644 --- a/desktop/src/features/agents/useGlobalAgentConfig.ts +++ b/desktop/src/features/agents/useGlobalAgentConfig.ts @@ -19,6 +19,7 @@ const EMPTY_CONFIG: GlobalAgentConfig = { provider: null, model: null, preferred_runtime: null, + permission_policy: null, }; export const globalAgentConfigQueryKey = ["globalAgentConfig"] as const; diff --git a/desktop/src/shared/api/agentControl.ts b/desktop/src/shared/api/agentControl.ts index 677f0ffad4..47db543d85 100644 --- a/desktop/src/shared/api/agentControl.ts +++ b/desktop/src/shared/api/agentControl.ts @@ -29,3 +29,29 @@ export async function switchManagedAgentModel( modelId, }); } + +/** + * Send a permission decision to a running agent's ACP harness. The decision + * is fire-and-forget: the harness receives it via the observer control channel + * and updates the permission card asynchronously via a `control_result` frame. + * + * @param pubkey - Agent's public key (hex or npub). + * @param channelId - The channel from which the permission request was issued. + * The harness validates this before looking up the nonce. + * @param nonce - `requestNonce` from the `authorization` envelope on the + * corresponding `acp_read` permission frame. + * @param optionId - The chosen option's `optionId` (e.g. `"allow_once"`). + */ +export async function sendPermissionDecision( + pubkey: string, + channelId: string, + nonce: string, + optionId: string, +): Promise { + await sendAgentObserverControl(pubkey, { + type: "permission_decision", + channelId, + requestNonce: nonce, + optionId, + }); +} diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index 6e29f77c14..fd1df56c88 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -40,6 +40,8 @@ import type { InstallRuntimeResult, GitBashPrerequisite, RuntimeConfigSurface, + PermissionPolicy, + PermissionPolicySource, } from "@/shared/api/types"; export * from "@/shared/api/tauriChannels"; @@ -162,6 +164,9 @@ export type RawManagedAgent = { // Pre-feature fixtures may omit these; mapped to "owner-only"/[] in fromRawManagedAgent. respond_to?: ManagedAgent["respondTo"]; respond_to_allowlist?: string[]; + // Pre-feature fixtures may omit these; defaults applied in fromRawManagedAgent. + permission_policy?: PermissionPolicy; + permission_policy_source?: PermissionPolicySource; }; type RawCreateManagedAgentResponse = { @@ -730,6 +735,8 @@ export function fromRawManagedAgent(agent: RawManagedAgent): ManagedAgent { backendAgentId: agent.backend_agent_id, respondTo: agent.respond_to ?? "owner-only", respondToAllowlist: agent.respond_to_allowlist ?? [], + permissionPolicy: agent.permission_policy ?? "ask", + permissionPolicySource: agent.permission_policy_source ?? "built_in", }; } diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index 24ef625783..3c2efd6026 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -384,11 +384,40 @@ export type ManagedAgent = { * `"allowlist"`. Preserved across mode toggles. */ respondToAllowlist: string[]; + /** + * Effective permission policy at the last spawn. Determines how the ACP + * harness answers `session/request_permission` calls. + */ + permissionPolicy: PermissionPolicy; + /** + * Where the effective `permissionPolicy` value came from: a per-agent + * override, the fleet-wide global default, or the built-in desktop default. + */ + permissionPolicySource: PermissionPolicySource; }; /** Inbound author gate mode. Mirrors buzz-acp's --respond-to CLI flag. */ export type RespondToMode = "owner-only" | "allowlist" | "anyone"; +/** + * Permission policy controlling how the ACP harness answers + * `session/request_permission` calls. + * + * - `ask`: Show an actionable Allow/Deny card in the transcript (desktop default). + * - `allow`: Auto-approve the unique `allow_once` option (explicit opt-in). + * - `reject`: Auto-deny all requests without surfacing a card. + */ +export type PermissionPolicy = "ask" | "allow" | "reject"; + +/** + * Where the effective permission policy value came from. + * + * - `agent`: Per-agent override set on this specific agent record. + * - `global_default`: Fleet-wide default from the global agent config. + * - `built_in`: Neither layer had a value; the desktop built-in default (`ask`) applies. + */ +export type PermissionPolicySource = "agent" | "global_default" | "built_in"; + export type BackendProviderCandidate = { id: string; binaryPath: string; @@ -443,6 +472,8 @@ export type CreateManagedAgentInput = { */ respondToAllowlist?: string[]; relayMesh?: RelayMeshConfig; + /** Per-agent permission policy override. Omitted = inherit from global or built-in default. */ + permissionPolicy?: PermissionPolicy; }; export type CreateManagedAgentResponse = { @@ -475,9 +506,11 @@ export type SwitchManagedAgentModelStatus = | "no_active_turn"; export type ControlResultFrame = { - type: "cancel_turn" | "switch_model"; + type: "cancel_turn" | "switch_model" | "permission_decision"; status: string; modelId?: string; + /** Present on `permission_decision` results — identifies the request card to retire. */ + requestNonce?: string; }; export type GitBashPrerequisite = { @@ -706,6 +739,11 @@ export type UpdateManagedAgentInput = { * (validated & normalized server-side). */ respondToAllowlist?: string[]; + /** + * Absent = don't touch. Present = override (or `null` to clear back to inherit). + * Remote deployed agents: read-only; edit the deploy config and redeploy. + */ + permissionPolicy?: PermissionPolicy | null; }; export type AgentPersona = { id: string; @@ -1012,6 +1050,12 @@ export type GlobalAgentConfig = { model: string | null; /** Preferred ACP runtime for agents without a persona-specific runtime. */ preferred_runtime: string | null; + /** + * Fleet-wide permission policy fallback. Null = no fleet default; agents + * without a per-agent policy use the built-in desktop default (`ask`). + * Mirrors `GlobalAgentConfig.permission_policy` in Rust. + */ + permission_policy: PermissionPolicy | null; }; /** diff --git a/docs/nips/NIP-AO.md b/docs/nips/NIP-AO.md index 36adea0487..1b29ab59e0 100644 --- a/docs/nips/NIP-AO.md +++ b/docs/nips/NIP-AO.md @@ -24,6 +24,11 @@ It is strictly scoped to the agent↔owner relationship and carries no durable s - **Owner**: The human (or system) whose pubkey the agent was provisioned under. - **Observer Frame**: A single kind 24200 event carrying one unit of telemetry or control. - **Session**: A bounded agent execution correlated by a shared `sessionId`. +- **Request nonce**: A single-use random token bound to one `session/request_permission` + call. The harness generates it on arrival of the request, embeds it in the + `authorization` envelope of the emitted `acp_read` telemetry frame, and consumes it + exactly once when a matching `permission_decision` control frame is received. A nonce + that is never matched expires with the per-request fail-closed timeout. ## Event Kinds @@ -58,8 +63,8 @@ Events MUST have exactly one `p` tag, exactly one `agent` tag, and exactly one `frame` MUST be `"telemetry"` or `"control"`. Relays SHOULD silently drop events with unrecognized `frame` values (returning OK to the publisher for forward -compatibility). Clients MUST ignore events with unrecognized `frame` values. An `h` tag MAY be included when the session runs within a NIP-29 group -context. +compatibility). Clients MUST ignore events with unrecognized `frame` values. An `h` +tag MAY be included when the session runs within a NIP-29 group context. ## Encryption @@ -80,14 +85,15 @@ The `content` field decrypts to an `ObserverEvent` JSON object: ```json { - "seq": , - "timestamp": "", - "kind": "", - "agentIndex": | null, - "channelId": "" | null, - "sessionId": "" | null, - "turnId": "" | null, - "payload": { ... } + "seq": , + "timestamp": "", + "kind": "", + "agentIndex": | null, + "channelId": "" | null, + "sessionId": "" | null, + "turnId": "" | null, + "authorization": { ... } | omitted, + "payload": { ... } } ``` @@ -99,21 +105,84 @@ gracefully. `seq` is monotonically increasing per session (drop detection). `timestamp` is an RFC 3339 datetime string with sub-second precision (e.g., `"2026-04-29T12:00:41.500Z"`). `agentIndex` identifies the agent in multi-agent scenarios. `sessionId`/`turnId` -correlate frames across a session and turn. `payload` is kind-specific (MAY be `{}`). -Unknown `kind` values MUST be ignored. +correlate frames across a session and turn. `payload` carries the raw ACP JSON frame +byte-for-byte — it is NEVER mutated by the harness. Unknown `kind` values MUST be +ignored. + +`authorization` is present only on `acp_read` and `acp_write` frames that correspond +to `session/request_permission` calls (see [Authorization Envelope](#authorization-envelope) +below). It is omitted on all other frame kinds. ### Frame Kinds -| `kind` | Description | -|--------------------|----------------------------------------------------------| -| `acp_read` | Inbound ACP protocol frame (model → harness) | -| `acp_write` | Outbound ACP protocol frame (harness → model) | -| `turn_started` | A new agent turn has begun | -| `session_resolved` | Session completed or terminated | +| `kind` | Description | +|--------------------|--------------------------------------------------------------------| +| `acp_read` | Inbound ACP protocol frame (model → harness) | +| `acp_write` | Outbound ACP protocol frame (harness → model) | +| `turn_started` | A new agent turn has begun | +| `session_resolved` | Session completed or terminated | +| `control_result` | Acknowledgement telemetry emitted after processing a control frame | + +Permission `acp_read` frames (carrying `session/request_permission` calls) always +include an `authorization` envelope. The corresponding `acp_write` (the harness +response) also includes an `authorization` envelope correlated by the same nonce — +this pairs the challenge and answer in the observer log. + +**One-write / one-observe contract.** Each pending permission entry produces at most +one ACP wire write and at most one authorized `acp_write` observer event. The write +and the observer event are always emitted together; if the write fails the observer +event is suppressed. The sole exception is the `uncertain` terminal (see below) in +which neither is emitted. + +### Authorization Envelope + +When an `acp_read` or `acp_write` frame relates to a `session/request_permission` +call, the `ObserverEvent` carries an `authorization` field: + +```json +{ + "requestNonce": "", + "actionable": true | false, + "reason": "" | omitted +} +``` + +- `requestNonce`: a single-use random token generated by the harness for this request. + It is embedded in the `acp_read` emit and MUST be echoed verbatim in the + `permission_decision` control frame sent by the desktop. The harness consumes the + nonce exactly once — a second `permission_decision` carrying the same nonce is + silently ignored. If no matching decision arrives before the per-request timeout, + the harness fails the request closed. +- `actionable`: `true` when the owner can act (policy=`ask`, preflight passed, owner + and observer available). `false` for auto-deny, fail-closed, and terminal outcomes. +- `reason`: present on every `acp_write` authorization envelope. Identifies the + terminal outcome for this request. Defined values: + + | Value | Meaning | + |-------|---------| + | `"applied"` | Owner decision was received and written to the agent pipe. | + | `"timed_out"` | No decision arrived before the 300-second per-request deadline; request failed closed (denial). | + | `"cancelled"` | The turn was cancelled while the request was pending; request failed closed (denial). | + + The `uncertain` terminal (cancel arriving while the write is in flight) does NOT + produce an `acp_write` observer event — the process is irrecoverably poisoned and + will be respawned by the pool. Desktop clients MUST NOT expect an `acp_write` for + every `acp_read` they receive; a missing `acp_write` after a `session_resolved` + frame with a poisoned outcome indicates the `uncertain` path. + +**Nonce binding.** The nonce is bound to the agent, channel, session, turn, request +ID, and exact option snapshot at generation time. It MUST NOT be reused across +requests, turns, or sessions. The harness rejects a `permission_decision` whose nonce +does not match any live pending entry. ### Control (`frame=control`) -The `content` field decrypts to: +The `content` field decrypts to a JSON object with a required `type` field. +Implementations MUST ignore events with unrecognized `type` values. + +#### `cancel_turn` + +Cancel the in-flight agent turn for the given channel. ```json { @@ -122,8 +191,84 @@ The `content` field decrypts to: } ``` -The only defined control type is `cancel_turn`. Implementations MUST ignore -events with unrecognized `type` values. +#### `switch_model` + +Switch the active model for the agent session in the given channel. + +- **Busy turn:** delivers `ControlSignal::SwitchModel` over the per-turn oneshot, + which triggers the harness to cancel the current turn and requeue with the new model. + If the oneshot is already consumed (a prior cancel/interrupt is in flight), the + switch cannot land and the current turn is left to complete with the old model. +- **Idle session:** validates the model against the cached catalog and, if valid, + invalidates and reapplies the agent's model config immediately. + +```json +{ + "type": "switch_model", + "channelId": "", + "modelId": "" +} +``` + +#### `permission_decision` + +Deliver the owner's decision for a pending `session/request_permission` call. +The harness matches `requestNonce` to a live pending entry and, if found, transitions +the entry from `pending` to `writing` and writes the ACP response. + +```json +{ + "type": "permission_decision", + "channelId": "", + "requestNonce": "", + "optionId": "" +} +``` + +The harness MUST: +1. Verify `requestNonce` matches a live pending entry (else ignore silently). +2. Verify `optionId` is present in the exact option snapshot recorded at nonce + generation time (else ignore silently — prevents replay with an altered option). +3. Transition the entry to `writing` atomically before performing the ACP write. +4. Emit an `acp_write` telemetry frame with a matching `authorization` envelope only + after the write is confirmed. + +**Best-effort delivery.** `permission_decision` frames ride the ordinary observer +control path — they are NOT guaranteed to arrive before the per-request timeout. +If no matching `permission_decision` is received within `min(300s, remaining hard +deadline)`, the harness fails the request closed (deny). The owner SHOULD respond +before this deadline; the desktop MAY surface the deadline to the owner in the +permission card UI. + +### `control_result` Telemetry + +After processing any control frame, the harness emits a `control_result` telemetry +event to confirm receipt. This is an `acp_read`-style telemetry frame (kind = +`control_result`) that carries a `payload` describing the outcome: + +**`cancel_turn`:** +```json +{ "type": "cancel_turn", "status": "sent" | "no_active_turn" } +``` + +**`switch_model`:** +```json +{ "type": "switch_model", "status": "sent" | "turn_ending" | "switched" | "unsupported_model" | "no_active_turn", "modelId": "..." } +``` + +**`permission_decision`:** +```json +{ + "type": "permission_decision", + "status": "sent" | "no_active_turn" | "channel_full" | "channel_closed" | "no_channel", + "requestNonce": "", + "optionId": "" +} +``` + +`status: "sent"` means the decision was delivered to the in-flight read loop. +Other statuses indicate delivery failure; the per-request timeout will fail the +entry closed. ## Ephemerality Contract @@ -132,7 +277,9 @@ events with unrecognized `type` values. - Relays MUST NOT include kind 24200 events in audit logs. - Relays SHOULD fan out kind 24200 events only via in-memory pub/sub, never via a database write path. -- Clients SHOULD subscribe with `since=`; historical replay is not supported. +- Clients SHOULD subscribe with `since=` to recover frames from the past + five minutes (e.g., after a brief reconnect); historical replay beyond this window + is not supported. - Clients SHOULD buffer received events in a bounded in-memory ring buffer. ## Authorization @@ -152,6 +299,9 @@ Both directions require relay confirmation of the agent-owner relationship via database lookup. `#p` tag matching alone is insufficient. Unauthorized publish or subscribe attempts MUST be rejected with `AUTH required`. +The harness additionally enforces a ±5-minute `created_at` freshness window on +incoming control frames as defense-in-depth against relay-captured replay. + ## Relay Behavior On receiving a kind 24200 event, a relay MUST: @@ -170,9 +320,12 @@ freshness window to prevent replay of captured events. Clients subscribe with: ```json -{"kinds": [24200], "#p": [""], "since": } +{"kinds": [24200], "#p": [""], "since": } ``` +The `since` lookback of 300 seconds (5 minutes) allows recovery of recent frames +after brief reconnects without enabling unbounded historical replay. + On receiving an event, a client MUST: 1. Verify the event signature. @@ -184,8 +337,8 @@ Clients SHOULD verify that the `agent` tag matches a known/trusted agent pubkey before decrypting. Clients SHOULD buffer events in a bounded ring buffer (RECOMMENDED maximum: 800 events). -Clients MUST NOT request historical kind 24200 events (no `since` in the past, no -`until`, no `ids` queries). +Clients MUST NOT request historical kind 24200 events beyond the 5-minute lookback +window (no `since` further in the past, no `until`, no `ids` queries). ## Security Considerations @@ -197,19 +350,34 @@ rate. For maximum metadata privacy, implementors MAY wrap events in NIP-59 gift agent's private key allows decryption of any captured ciphertext. **Replay attacks.** A captured, signed event could be replayed without a freshness -check. Relays are RECOMMENDED to enforce a `created_at` freshness window. +check. Relays are RECOMMENDED to enforce a `created_at` freshness window. The harness +enforces this as defense-in-depth on incoming control frames. **Rogue relays.** The ephemerality contract is relay policy, not cryptography. NIP-44 encryption ensures stored events remain opaque to the relay operator absent key compromise. **Best-effort delivery.** Control frames can be dropped during reconnect or queue -overflow. Control commands SHOULD be treated as advisory with idempotent semantics. -Agents MUST NOT rely on guaranteed delivery of control frames. +overflow. `permission_decision` frames follow the same best-effort path; the +mandatory per-request fail-closed timeout (max 300 seconds) ensures the harness never +blocks indefinitely waiting for a decision that never arrives. + +**Permission nonce security.** Request nonces are single-use and generated fresh per +request. A `permission_decision` carrying a nonce that does not match an active +pending entry is silently ignored. The harness verifies that the chosen `optionId` is +present in the exact option snapshot captured at nonce generation — preventing a +replayed or modified decision from selecting an option not offered in the original +request. + +**Cancel during write (poison).** If a cancel arrives while the harness is writing +an ACP permission response mid-flight, the process state is irrecoverably uncertain. +The harness surfaces a dedicated `PermissionPoisoned` error through `cancel_with_cleanup_grace`, +which causes the pool to respawn the agent process rather than return it. All other +pending permission entries for that session are drained with `cancelled` responses. **Operational persistence vectors.** Telemetry may transiently exist in process memory, crash dumps, and application logs. Implementations SHOULD minimize logging -of decrypted payloads and MUST NOT log it at INFO level or above. +of decrypted payloads and MUST NOT log them at INFO level or above. ## Relationship to Other NIPs @@ -295,6 +463,82 @@ of decrypted payloads and MUST NOT log it at INFO level or above. } ``` +--- + +### 3. Permission request (ask policy) — challenge + decision round trip + +**Step 1 — agent emits `session/request_permission`; harness emits `acp_read` telemetry:** + +```json +{ + "seq": 101, + "timestamp": "2026-08-01T10:00:00.000Z", + "kind": "acp_read", + "agentIndex": 0, + "channelId": "52a85618-0f8f-4542-94ec-599e6e1c6f2e", + "sessionId": "sess-abc", + "turnId": "turn-xyz", + "authorization": { + "requestNonce": "a9f3b2c1d4e5...", + "actionable": true + }, + "payload": { + "jsonrpc": "2.0", + "id": "req-17", + "method": "session/request_permission", + "params": { + "sessionId": "sess-abc", + "options": [ + { "optionId": "opt-allow", "kind": "allow_once", "name": "Allow once" }, + { "optionId": "opt-deny", "kind": "reject_once", "name": "Deny" } + ] + } + } +} +``` + +**Step 2 — desktop sends `permission_decision` control frame:** + +```json +{ + "type": "permission_decision", + "channelId": "52a85618-0f8f-4542-94ec-599e6e1c6f2e", + "requestNonce": "a9f3b2c1d4e5...", + "optionId": "opt-allow" +} +``` + +**Step 3 — harness writes ACP response and emits `acp_write` telemetry:** + +```json +{ + "seq": 102, + "timestamp": "2026-08-01T10:00:04.120Z", + "kind": "acp_write", + "agentIndex": 0, + "channelId": "52a85618-0f8f-4542-94ec-599e6e1c6f2e", + "sessionId": "sess-abc", + "turnId": "turn-xyz", + "authorization": { + "requestNonce": "a9f3b2c1d4e5...", + "actionable": false, + "reason": "applied" + }, + "payload": { + "jsonrpc": "2.0", + "id": "req-17", + "result": { "outcome": { "outcome": "selected", "optionId": "opt-allow" } } + } +} +``` + +Note: `actionable` is `false` on the `acp_write` telemetry frame — the decision has +been applied and the card is no longer actionable. `reason: "applied"` is the +standard terminal annotation for a successfully delivered decision. When the request +expires without a decision, the harness emits `reason: "timed_out"`. When the turn +is cancelled while the request is pending, the harness emits `reason: "cancelled"`. +If the cancel arrives mid-write (`uncertain`), no `acp_write` frame is emitted at all. + ## Reference Implementation -[block/sprout PR #421](https://github.com/block/sprout/pull/421) +[block/buzz PR #4938](https://github.com/block/buzz/pull/4938)