Skip to content

Commit bbc6f75

Browse files
committed
fix(acp): preserve permission requests by default
Signed-off-by: KC <79471844+wolfyy970@users.noreply.github.com>
1 parent 548d31c commit bbc6f75

3 files changed

Lines changed: 118 additions & 17 deletions

File tree

crates/buzz-acp/src/acp.rs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2359,6 +2359,66 @@ mod tests {
23592359
assert_eq!(reject_once.unwrap()["optionId"].as_str(), Some("rej-x"));
23602360
}
23612361

2362+
/// Exercise the production prompt read loop rather than duplicating its
2363+
/// option-selection logic. In the default adapter mode, the adapter asks
2364+
/// for each permission and Buzz must answer that exact request before the
2365+
/// prompt can complete.
2366+
#[cfg(unix)]
2367+
#[tokio::test]
2368+
async fn prompt_loop_answers_allow_once_permission_on_wire() {
2369+
let capture = std::env::temp_dir().join(format!(
2370+
"buzz-acp-default-mode-permission-{}.json",
2371+
uuid::Uuid::new_v4()
2372+
));
2373+
let script = format!(
2374+
"read -r _prompt; \
2375+
printf '%s\\n' '{request}'; \
2376+
read -r decision; \
2377+
printf '%s' \"$decision\" > \"$1\"; \
2378+
printf '%s\\n' '{result}'; \
2379+
sleep 1",
2380+
request = r#"{"jsonrpc":"2.0","id":"permission-7","method":"session/request_permission","params":{"options":[{"optionId":"deny-1","name":"Deny","kind":"reject_once"},{"optionId":"grant-7","name":"Allow once","kind":"allow_once"}]}}"#,
2381+
result = r#"{"jsonrpc":"2.0","id":0,"result":{"stopReason":"end_turn"}}"#,
2382+
);
2383+
let mut client = AcpClient::spawn(
2384+
"bash",
2385+
&[
2386+
"-c".into(),
2387+
script,
2388+
"buzz-acp-permission-test".into(),
2389+
capture.to_string_lossy().into_owned(),
2390+
],
2391+
&[],
2392+
false,
2393+
)
2394+
.await
2395+
.expect("spawn permission test adapter");
2396+
2397+
let stop = client
2398+
.session_prompt_with_idle_timeout(
2399+
"session-1",
2400+
"reply",
2401+
std::time::Duration::from_secs(2),
2402+
std::time::Duration::from_secs(5),
2403+
)
2404+
.await
2405+
.expect("permission response should let the prompt complete");
2406+
assert_eq!(stop, StopReason::EndTurn);
2407+
2408+
let written = std::fs::read_to_string(&capture).expect("captured permission response");
2409+
let response: serde_json::Value =
2410+
serde_json::from_str(&written).expect("permission response is valid JSON");
2411+
assert_eq!(response["id"].as_str(), Some("permission-7"));
2412+
assert_eq!(response["result"]["outcome"]["outcome"], "selected");
2413+
assert_eq!(
2414+
response["result"]["outcome"]["optionId"].as_str(),
2415+
Some("grant-7")
2416+
);
2417+
2418+
client.shutdown().await;
2419+
std::fs::remove_file(capture).expect("remove permission capture");
2420+
}
2421+
23622422
#[test]
23632423
fn request_has_id_field() {
23642424
let id: u64 = 42;

crates/buzz-acp/src/config.rs

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -432,13 +432,13 @@ pub struct CliArgs {
432432
/// Permission mode for agents that support `session/set_config_option`
433433
/// with `configId: "mode"` (e.g. `claude-agent-acp`).
434434
///
435-
/// Defaults to `bypassPermissions` which skips the per-tool-call
436-
/// permission flow. Set to `default` to restore the agent's built-in
437-
/// behaviour.
435+
/// Defaults to `default`, which preserves the per-tool permission flow so
436+
/// the harness can observe and answer each request. Operators can still
437+
/// select `bypassPermissions` explicitly for legacy unattended behavior.
438438
#[arg(
439439
long,
440440
env = "BUZZ_ACP_PERMISSION_MODE",
441-
default_value = "bypass-permissions",
441+
default_value = "default",
442442
value_enum
443443
)]
444444
pub permission_mode: PermissionMode,
@@ -1435,7 +1435,7 @@ fn rule_applies_to_channel(rule: &SubscriptionRule, channel_id: Uuid) -> bool {
14351435
mod tests {
14361436
use super::*;
14371437
use crate::filter::{ChannelScope, SubscriptionRule};
1438-
use clap::{Parser, ValueEnum};
1438+
use clap::{CommandFactory, Parser, ValueEnum};
14391439

14401440
/// Build a minimal Config for testing without CLI parsing.
14411441
fn test_config(mode: SubscribeMode) -> Config {
@@ -1469,7 +1469,7 @@ mod tests {
14691469
memory_enabled: true,
14701470
model: None,
14711471
session_title: None,
1472-
permission_mode: PermissionMode::BypassPermissions,
1472+
permission_mode: PermissionMode::Default,
14731473
respond_to: RespondTo::Anyone,
14741474
respond_to_allowlist: HashSet::new(),
14751475
allowed_respond_to: Vec::new(),
@@ -2319,9 +2319,18 @@ channels = "ALL"
23192319
}
23202320

23212321
#[test]
2322-
fn test_default_config_uses_bypass_permissions() {
2323-
let config = test_config(SubscribeMode::Mentions);
2324-
assert_eq!(config.permission_mode, PermissionMode::BypassPermissions);
2322+
fn test_default_config_preserves_permission_requests() {
2323+
let command = CliArgs::command();
2324+
let permission_mode = command
2325+
.get_arguments()
2326+
.find(|arg| arg.get_id() == "permission_mode")
2327+
.expect("permission-mode argument");
2328+
let defaults: Vec<_> = permission_mode
2329+
.get_default_values()
2330+
.iter()
2331+
.map(|value| value.to_string_lossy())
2332+
.collect();
2333+
assert_eq!(defaults, ["default"]);
23252334
}
23262335

23272336
#[test]

crates/buzz-acp/src/pool.rs

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,9 +1018,7 @@ async fn create_session_and_apply_model(
10181018
// advertises the requested mode in session/new. Agents that don't support
10191019
// the mode (e.g., goose crashes on unrecognized set_config_option values)
10201020
// are safely skipped — the harness auto-approves via handle_permission_request.
1021-
if !ctx.permission_mode.is_default()
1022-
&& agent_supports_mode(&resp.raw, ctx.permission_mode.as_wire_str())
1023-
{
1021+
if should_apply_permission_mode(&resp.raw, &ctx.permission_mode) {
10241022
apply_permission_mode(&mut agent.acp, &resp.session_id, &ctx.permission_mode).await?;
10251023
}
10261024

@@ -1130,10 +1128,6 @@ async fn apply_model_switch(
11301128
Ok(())
11311129
}
11321130

1133-
/// Set the session permission mode via `session/set_config_option`.
1134-
///
1135-
/// Non-fatal for most errors: logs and proceeds. The agent falls back
1136-
/// to its default permission mode (`"default"`), which still works via
11371131
/// Check if the agent's `session/new` response advertises a given mode ID
11381132
/// in `result.modes.availableModes[].id`. Returns `false` if the modes
11391133
/// field is absent or the mode isn't listed.
@@ -1150,7 +1144,20 @@ fn agent_supports_mode(session_new_result: &serde_json::Value, mode_wire: &str)
11501144
.unwrap_or(false)
11511145
}
11521146

1153-
/// per-tool auto-approval in `handle_permission_request`.
1147+
/// Decide whether Buzz should override the adapter's session permission mode.
1148+
/// The default deliberately leaves the adapter untouched so its individual
1149+
/// permission requests reach the harness.
1150+
fn should_apply_permission_mode(
1151+
session_new_result: &serde_json::Value,
1152+
mode: &PermissionMode,
1153+
) -> bool {
1154+
!mode.is_default() && agent_supports_mode(session_new_result, mode.as_wire_str())
1155+
}
1156+
1157+
/// Set the session permission mode via `session/set_config_option`.
1158+
///
1159+
/// Non-fatal for application-level errors: the agent remains in its default
1160+
/// mode and the harness handles its per-tool permission requests.
11541161
///
11551162
/// **Fatal exception:** if the agent process exits (e.g., goose crashes on
11561163
/// unrecognized methods), returns `Err(AgentExited)` so the caller can respawn.
@@ -4039,6 +4046,31 @@ mod tests {
40394046
}
40404047
}
40414048

4049+
#[test]
4050+
fn default_permission_mode_never_sends_a_session_override() {
4051+
let session = json!({
4052+
"modes": {
4053+
"availableModes": [
4054+
{"id": "default"},
4055+
{"id": "bypassPermissions"}
4056+
]
4057+
}
4058+
});
4059+
4060+
assert!(!should_apply_permission_mode(
4061+
&session,
4062+
&PermissionMode::Default
4063+
));
4064+
assert!(should_apply_permission_mode(
4065+
&session,
4066+
&PermissionMode::BypassPermissions
4067+
));
4068+
assert!(!should_apply_permission_mode(
4069+
&json!({}),
4070+
&PermissionMode::BypassPermissions
4071+
));
4072+
}
4073+
40424074
#[test]
40434075
fn public_session_forwards_channel_origin_to_mcp() {
40444076
let channel_id = Uuid::new_v4();

0 commit comments

Comments
 (0)