From 4ab7673066e887b00a100eb65680225be7934b6d Mon Sep 17 00:00:00 2001
From: Andrew Kuttig <akuttig@squareup.com>
Date: Thu, 2 Apr 2026 18:45:02 -0600
Subject: [PATCH] Fall back to session/set_mode when set_config_option is
 unsupported

amp-acp implements session/set_mode (with modeId param) instead of
session/set_config_option (with configId 'mode'). The harness now
tries set_config_option first (Claude-style), and if the agent
returns a method-not-found error, falls back to set_mode (amp-acp
style). This fixes the (blocked-on-user) issue where amp-acp agents
never received the bypass mode and prompted for permissions.

Amp-Thread-ID: https://ampcode.com/threads/T-019d4e6c-6235-76b4-aaf3-63baf10304d0
Co-authored-by: Amp <amp@ampcode.com>
---
 crates/sprout-acp/src/acp.rs  | 13 ++++++++
 crates/sprout-acp/src/pool.rs | 58 ++++++++++++++++++++++++-----------
 2 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/crates/sprout-acp/src/acp.rs b/crates/sprout-acp/src/acp.rs
index 198228c74..1e753102c 100644
--- a/crates/sprout-acp/src/acp.rs
+++ b/crates/sprout-acp/src/acp.rs
@@ -287,6 +287,19 @@ impl AcpClient {
         self.send_request("session/set_config_option", params).await
     }
 
+    /// Send `session/set_mode` — newer ACP path used by adapters like amp-acp.
+    pub async fn session_set_mode(
+        &mut self,
+        session_id: &str,
+        mode_id: &str,
+    ) -> Result<serde_json::Value, AcpError> {
+        let params = serde_json::json!({
+            "sessionId": session_id,
+            "modeId": mode_id,
+        });
+        self.send_request("session/set_mode", params).await
+    }
+
     /// Send `session/set_model` (unstable ACP path).
     pub async fn session_set_model(
         &mut self,
diff --git a/crates/sprout-acp/src/pool.rs b/crates/sprout-acp/src/pool.rs
index b3f507585..ecd709b63 100644
--- a/crates/sprout-acp/src/pool.rs
+++ b/crates/sprout-acp/src/pool.rs
@@ -549,6 +549,9 @@ fn resolve_mode_id(
 
 /// per-tool auto-approval in `handle_permission_request`.
 ///
+/// Tries `session/set_config_option` first (Claude-style), and if the agent
+/// doesn't support it, falls back to `session/set_mode` (amp-acp style).
+///
 /// **Fatal exception:** if the agent process exits (e.g., goose crashes on
 /// unrecognized methods), returns `Err(AgentExited)` so the caller can respawn.
 async fn apply_permission_mode(
@@ -556,35 +559,59 @@ async fn apply_permission_mode(
     session_id: &str,
     mode_id: &str,
 ) -> Result<(), AcpError> {
-    let result = tokio::time::timeout(PERMISSION_MODE_TIMEOUT, async {
+    // Try session/set_config_option first (Claude-style).
+    let config_result = tokio::time::timeout(PERMISSION_MODE_TIMEOUT, async {
         acp.session_set_config_option(session_id, "mode", mode_id)
             .await
     })
     .await;
 
-    match result {
+    match config_result {
         Ok(Ok(_)) => {
             tracing::info!(
                 target: "pool::permission",
-                "applied permission mode {mode_id:?} on session {session_id}"
+                "applied permission mode {mode_id:?} via set_config_option on session {session_id}"
+            );
+            return Ok(());
+        }
+        // Transport-class errors — propagate so the caller can respawn.
+        Ok(Err(e @ AcpError::Io(_)))
+        | Ok(Err(e @ AcpError::WriteTimeout(_)))
+        | Ok(Err(e @ AcpError::Timeout(_)))
+        | Ok(Err(e @ AcpError::AgentExited)) => {
+            return Err(e);
+        }
+        Err(_) => {
+            return Err(AcpError::Timeout(PERMISSION_MODE_TIMEOUT));
+        }
+        // Application-level error (e.g. "method not found") — try set_mode.
+        Ok(Err(e)) => {
+            tracing::debug!(
+                target: "pool::permission",
+                "set_config_option not supported ({e}), trying session/set_mode"
+            );
+        }
+    }
+
+    // Fallback: session/set_mode (amp-acp style).
+    let mode_result = tokio::time::timeout(PERMISSION_MODE_TIMEOUT, async {
+        acp.session_set_mode(session_id, mode_id).await
+    })
+    .await;
+
+    match mode_result {
+        Ok(Ok(_)) => {
+            tracing::info!(
+                target: "pool::permission",
+                "applied permission mode {mode_id:?} via set_mode on session {session_id}"
             );
         }
-        // Transport-class errors may have corrupted the stdio stream — propagate
-        // so the caller can respawn the agent.
         Ok(Err(e @ AcpError::Io(_)))
         | Ok(Err(e @ AcpError::WriteTimeout(_)))
         | Ok(Err(e @ AcpError::Timeout(_)))
         | Ok(Err(e @ AcpError::AgentExited)) => {
-            tracing::error!(
-                target: "pool::permission",
-                "fatal error setting permission mode {mode_id:?}: {e}"
-            );
             return Err(e);
         }
-        // Application-level errors (including JSON-RPC error responses like
-        // "method not found" which surface as AcpError::Protocol) — agent is
-        // fine, just doesn't support this config method. Fall back to
-        // per-tool auto-approval.
         Ok(Err(e)) => {
             tracing::warn!(
                 target: "pool::permission",
@@ -592,11 +619,6 @@ async fn apply_permission_mode(
             );
         }
         Err(_) => {
-            // Outer timeout fired — stream may be in unknown state.
-            tracing::error!(
-                target: "pool::permission",
-                "permission mode set timed out ({PERMISSION_MODE_TIMEOUT:?}) — treating as fatal"
-            );
             return Err(AcpError::Timeout(PERMISSION_MODE_TIMEOUT));
         }
     }