fix(acp): constrain owner permission choices

Signed-off-by: KC <79471844+wolfyy970@users.noreply.github.com>

Author: wolfyy970

crates/buzz-acp/src/acp.rs

@@ -1266,7 +1266,8 @@ fn handle_switch_model_control(
 
 /// Handle a `permission_decision` control frame.
 ///
-/// Extracts `channelId`, `requestNonce`, and `optionId` from the payload and
+/// Extracts `channelId`, `requestNonce`, and a selected/cancelled outcome from
+/// the payload and
 /// delivers a [`crate::acp::PermissionDecision`] to the in-flight read loop
 /// via the per-task `permission_decision_tx` mpsc channel.
 ///
@@ -1296,18 +1297,48 @@ fn handle_permission_decision_control(
         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 selected_outcome = || {
+        payload
+            .get("optionId")
+            .and_then(|value| value.as_str())
+            .filter(|value| !value.is_empty())
+            .map(
+                |option_id| crate::acp::PermissionDecisionOutcome::Selected {
+                    option_id: option_id.to_string(),
+                },
+            )
+    };
+    let outcome = match payload.get("outcome") {
+        Some(serde_json::Value::String(value)) if value == "cancelled" => {
+            crate::acp::PermissionDecisionOutcome::Cancelled
+        }
+        Some(serde_json::Value::String(value)) if value == "selected" => {
+            let Some(outcome) = selected_outcome() else {
+                tracing::warn!("observer permission_decision selected outcome missing optionId");
+                return;
+            };
+            outcome
+        }
+        None => {
+            let Some(outcome) = selected_outcome() else {
+                tracing::warn!("observer permission_decision selected outcome missing optionId");
+                return;
+            };
+            outcome
+        }
+        Some(other) => {
+            tracing::warn!(outcome = %other, "observer permission_decision has unknown outcome");
+            return;
+        }
     };
 
     let decision = crate::acp::PermissionDecision {
         request_nonce: request_nonce.to_string(),
-        option_id: option_id.to_string(),
+        outcome: outcome.clone(),
+    };
+    let outcome_name = match &outcome {
+        crate::acp::PermissionDecisionOutcome::Selected { .. } => "selected",
+        crate::acp::PermissionDecisionOutcome::Cancelled => "cancelled",
     };
 
     // Find the in-flight task for this channel and deliver via its mpsc.
@@ -1323,7 +1354,7 @@ fn handle_permission_decision_control(
                     tracing::info!(
                         channel = %channel_id,
                         nonce = %request_nonce,
-                        option_id = %option_id,
+                        outcome = outcome_name,
                         "permission_decision delivered to read loop"
                     );
                     "sent"
@@ -1372,7 +1403,7 @@ fn handle_permission_decision_control(
                 "type": "permission_decision",
                 "status": status,
                 "requestNonce": request_nonce,
-                "optionId": option_id,
+                "outcome": outcome_name,
             }),
         );
     }
@@ -4884,6 +4915,62 @@ mod owner_control_command_tests {
             ControlSignal::Rotate
         ));
     }
+
+    #[tokio::test]
+    async fn permission_control_delivers_cancel_and_legacy_selection() {
+        let mut pool = AgentPool::from_slots(vec![]);
+        let channel_id = Uuid::new_v4();
+        let (decision_tx, mut decision_rx) = tokio::sync::mpsc::channel(1);
+
+        let task = pool.join_set.spawn(std::future::pending::<()>());
+        pool.task_map_mut().insert(
+            task.id(),
+            pool::TaskMeta {
+                agent_index: 0,
+                channel_id: Some(channel_id),
+                turn_id: "permission-turn".to_string(),
+                recoverable_batch: None,
+                control_tx: None,
+                steer_tx: None,
+                permission_decision_tx: Some(decision_tx),
+            },
+        );
+
+        handle_permission_decision_control(
+            &serde_json::json!({
+                "type": "permission_decision",
+                "channelId": channel_id,
+                "requestNonce": "cancel-nonce",
+                "outcome": "cancelled"
+            }),
+            &mut pool,
+            None,
+        );
+        let cancelled = decision_rx.recv().await.expect("cancel decision");
+        assert_eq!(cancelled.request_nonce, "cancel-nonce");
+        assert!(matches!(
+            cancelled.outcome,
+            crate::acp::PermissionDecisionOutcome::Cancelled
+        ));
+
+        handle_permission_decision_control(
+            &serde_json::json!({
+                "type": "permission_decision",
+                "channelId": channel_id,
+                "requestNonce": "legacy-nonce",
+                "optionId": "allow-once"
+            }),
+            &mut pool,
+            None,
+        );
+        let selected = decision_rx.recv().await.expect("legacy selected decision");
+        assert_eq!(selected.request_nonce, "legacy-nonce");
+        assert!(matches!(
+            selected.outcome,
+            crate::acp::PermissionDecisionOutcome::Selected { option_id }
+                if option_id == "allow-once"
+        ));
+    }
 }
 
 #[cfg(test)]
@@ -8049,6 +8136,7 @@ mod observer_payload_trim_tests {
         event.authorization = Some(crate::observer::AuthorizationEnvelope {
             request_nonce: "test-nonce".to_string(),
             actionable: true,
+            can_cancel: Some(true),
             reason: None,
         });
 

crates/buzz-acp/src/lib.rs

@@ -44,6 +44,10 @@ pub struct AuthorizationEnvelope {
     /// `true` when the owner can take action (policy=ask, preflight passed,
     /// owner/observer available). `false` for auto-deny / fail-closed paths.
     pub actionable: bool,
+    /// `true` when this harness accepts an owner `cancelled` control outcome.
+    /// Omitted for older/non-actionable envelopes so Desktop never infers it.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub can_cancel: Option<bool>,
     /// Human-readable reason when `actionable` is `false`.
     #[serde(skip_serializing_if = "Option::is_none")]
     pub reason: Option<String>,

crates/buzz-acp/src/observer.rs

@@ -0,0 +1,80 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+import React from "react";
+import { renderToStaticMarkup } from "react-dom/server";
+
+import { LifecycleActivity } from "./LifecycleActivity.tsx";
+
+function renderPermission(options, { canCancel = false } = {}) {
+  return renderToStaticMarkup(
+    React.createElement(LifecycleActivity, {
+      agentAvatarUrl: null,
+      agentName: "Agent",
+      agentPubkey: "agent-pubkey",
+      item: {
+        id: "permission:test",
+        type: "lifecycle",
+        title: "Permission requested",
+        text: "Run the requested tool",
+        timestamp: "2026-08-08T12:00:00.000Z",
+        renderClass: "permission",
+        actionable: true,
+        requestNonce: "nonce",
+        canCancelPermission: canCancel,
+        channelId: "channel",
+        options,
+      },
+    }),
+  );
+}
+
+test("permission card renders only one-time decisions", () => {
+  const html = renderPermission([
+    { optionId: "allow", kind: "allow_once", label: "Allow once" },
+    { optionId: "reject", kind: "reject_once", label: "Reject once" },
+    {
+      optionId: "persistent",
+      kind: "allow_always",
+      label: "Always allow",
+    },
+    { optionId: "future", kind: "future_scope", label: "Future choice" },
+  ]);
+
+  assert.match(html, /permission-decision-allow/);
+  assert.match(html, /permission-decision-reject/);
+  assert.doesNotMatch(html, /permission-decision-persistent/);
+  assert.doesNotMatch(html, /permission-decision-future/);
+  assert.doesNotMatch(html, /Always allow/);
+  assert.doesNotMatch(html, /permission-decision-cancel/);
+});
+
+test("new harness allow-only permission card includes an immediate Cancel action", () => {
+  const html = renderPermission(
+    [{ optionId: "allow", kind: "allow_once", label: "Allow once" }],
+    { canCancel: true },
+  );
+
+  assert.match(html, /permission-decision-allow/);
+  assert.match(html, /permission-decision-cancel/);
+  assert.match(html, />Cancel</);
+});
+
+test("legacy allow-only permission card does not show an unsupported Cancel action", () => {
+  const html = renderPermission([
+    { optionId: "allow", kind: "allow_once", label: "Allow once" },
+  ]);
+
+  assert.match(html, /permission-decision-allow/);
+  assert.doesNotMatch(html, /permission-decision-cancel/);
+});
+
+test("permission card renders no action for persistent or unknown choices", () => {
+  const html = renderPermission([
+    { optionId: "persistent", kind: "allow_always" },
+    { optionId: "future", kind: "future_scope" },
+  ]);
+
+  assert.doesNotMatch(html, /permission-decision-/);
+  assert.doesNotMatch(html, />Allow</);
+});

desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.render.test.mjs

@@ -1,7 +1,10 @@
 import { AlertCircle, CheckCircle2, ShieldCheck, XCircle } from "lucide-react";
 import * as React from "react";
 
-import { sendPermissionDecision } from "@/shared/api/agentControl";
+import {
+  cancelPermissionRequest,
+  sendPermissionDecision,
+} from "@/shared/api/agentControl";
 import { formatTranscriptTimestampTitle } from "../agentSessionUtils";
 import { ActivityRow, ActivityRowLabel } from "./ActivityRow";
 import { ToolActivity } from "./ToolActivity";
@@ -40,9 +43,8 @@ function permissionOutcomeTone(outcome: string): "approve" | "deny" | "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).
+ * One-time Allow/Deny buttons for an actionable permission card. Persistent
+ * and unknown choices are filtered before they reach this component.
  *
  * 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
@@ -53,20 +55,31 @@ function PermissionDecisionButtons({
   channelId,
   options,
   requestNonce,
+  canCancel,
   deliveryFailed,
 }: {
   agentPubkey: string;
   channelId: string;
   options: Array<{ optionId: string; kind: string; label?: string }>;
   requestNonce: string;
+  /** Whether this harness explicitly accepts a protocol-level cancellation. */
+  canCancel: boolean;
   /**
    * 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<string | null>(null);
+  const [pending, setPending] = React.useState<
+    { kind: "option"; optionId: string } | { kind: "cancel" } | null
+  >(null);
+  const oneTimeOptions = options.filter(
+    ({ kind }) => kind === "allow_once" || kind === "reject_once",
+  );
+  const hasRejectOption = oneTimeOptions.some(
+    ({ kind }) => kind === "reject_once",
+  );
 
   // Re-enable buttons when the reducer signals delivery failure (non-`sent`
   // control_result status). The relay send succeeded but the harness couldn't
@@ -77,14 +90,14 @@ function PermissionDecisionButtons({
     }
   }, [deliveryFailed]);
 
-  if (options.length === 0) {
+  if (oneTimeOptions.length === 0) {
     return null;
   }
 
   return (
     <div className="mt-1.5 flex flex-wrap gap-1.5">
-      {options.map(({ optionId, kind, label }) => {
-        const isDeny = kind.startsWith("reject");
+      {oneTimeOptions.map(({ optionId, kind, label }) => {
+        const isDeny = kind === "reject_once";
         const displayLabel = label ?? (isDeny ? "Deny" : "Allow");
         return (
           <button
@@ -98,7 +111,7 @@ function PermissionDecisionButtons({
             data-testid={`permission-decision-${optionId}`}
             disabled={pending !== null}
             onClick={() => {
-              setPending(optionId);
+              setPending({ kind: "option", optionId });
               void sendPermissionDecision(
                 agentPubkey,
                 channelId,
@@ -111,10 +124,32 @@ function PermissionDecisionButtons({
               });
             }}
           >
-            {pending === optionId ? "…" : displayLabel}
+            {pending?.kind === "option" && pending.optionId === optionId
+              ? "…"
+              : displayLabel}
           </button>
         );
       })}
+      {canCancel && !hasRejectOption ? (
+        <button
+          type="button"
+          className="rounded px-2 py-0.5 text-xs font-medium border border-destructive/40 text-destructive hover:bg-destructive/10 disabled:opacity-50"
+          data-testid="permission-decision-cancel"
+          disabled={pending !== null}
+          onClick={() => {
+            setPending({ kind: "cancel" });
+            void cancelPermissionRequest(
+              agentPubkey,
+              channelId,
+              requestNonce,
+            ).catch(() => {
+              setPending(null);
+            });
+          }}
+        >
+          {pending?.kind === "cancel" ? "…" : "Cancel"}
+        </button>
+      ) : null}
     </div>
   );
 }
@@ -171,6 +206,7 @@ export function LifecycleActivity(props: ActivityRenderClassItemProps) {
             channelId={props.item.channelId ?? ""}
             options={options}
             requestNonce={requestNonce}
+            canCancel={props.item.canCancelPermission ?? false}
             deliveryFailed={deliveryFailed}
           />
         ) : null}