From 4414a571ac34ca04196516a72998558ced837cc7 Mon Sep 17 00:00:00 2001 From: Maxime Tolos Date: Mon, 3 Aug 2026 01:33:27 +0200 Subject: [PATCH 1/2] test(acp): pin permission mode protocol bounds Signed-off-by: Maxime Tolos --- crates/buzz-acp/src/pool.rs | 141 ++++++++++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 64edf68ee2..7e44b8981c 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -6905,4 +6905,145 @@ mod tests { ); server.abort(); } + + // ── Permission-mode bounds (#4098 / #3729 regression pins) ────────────── + // + // These pin the two halves of the permission-mode fix class: + // 1. `agent_supports_mode` — the advertisement gate behind the #3729 + // (OpenCode hang) fix: never send a mode the agent did not list. + // 2. `apply_permission_mode` — the 5s PERMISSION_MODE_TIMEOUT bound behind + // the #4098 hypothesis: a silent agent yields a fast fatal Timeout, + // never the 60s REQUEST_TIMEOUT stall from the issue log. + + #[test] + fn test_agent_supports_mode_true_when_mode_advertised() { + let session_new = json!({ + "sessionId": "ses_modes", + "modes": { + "currentModeId": "default", + "availableModes": [ + { "id": "default", "name": "Default" }, + { "id": "bypassPermissions", "name": "Bypass" } + ] + } + }); + assert!(agent_supports_mode(&session_new, "bypassPermissions")); + assert!(agent_supports_mode(&session_new, "default")); + } + + #[test] + fn test_agent_supports_mode_false_when_modes_absent() { + // Hermes-shaped session/new: no `modes` member at all. + let session_new = json!({ "sessionId": "ses_no_modes" }); + assert!(!agent_supports_mode(&session_new, "bypassPermissions")); + // An explicit `modes: null` behaves the same as absent. + let session_new_null = json!({ "sessionId": "ses_no_modes", "modes": null }); + assert!(!agent_supports_mode(&session_new_null, "bypassPermissions")); + } + + #[test] + fn test_agent_supports_mode_false_when_mode_unlisted() { + // The #3729 OpenCode arm: `availableModes` exists but does not list the + // requested mode — sending it must be suppressed. + let session_new = json!({ + "sessionId": "ses_modes", + "modes": { + "currentModeId": "default", + "availableModes": [ { "id": "default", "name": "Default" } ] + } + }); + assert!(!agent_supports_mode(&session_new, "bypassPermissions")); + } + + /// A Hermes-shaped agent that answers `session/new` advertising the mode and + /// then goes silent on `session/set_config_option` must fail fast: the outer + /// PERMISSION_MODE_TIMEOUT (5s) fires and maps to a fatal AcpError::Timeout — + /// never the 60s REQUEST_TIMEOUT stall from the #4098 log. + #[tokio::test] + async fn test_apply_permission_mode_silent_agent_fails_fast_with_timeout() { + // Reads stdin forever, never writes a response. + let mut acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "cat > /dev/null".to_string()], + &[], + false, + ) + .await + .expect("failed to spawn silent test agent"); + + let started = std::time::Instant::now(); + let result = + apply_permission_mode(&mut acp, "ses_silent", &PermissionMode::BypassPermissions).await; + let elapsed = started.elapsed(); + + let err = result.expect_err("a silent agent must fail the permission-mode set"); + assert!( + matches!(err, AcpError::Timeout(d) if d == PERMISSION_MODE_TIMEOUT), + "expected AcpError::Timeout(PERMISSION_MODE_TIMEOUT), got {err:?}" + ); + assert!( + elapsed >= Duration::from_secs(4), + "the ~5s bound must actually elapse, not fail fast on a transport error: {elapsed:?}" + ); + assert!( + elapsed < Duration::from_secs(10), + "permission-mode set must bound far below the 60s REQUEST_TIMEOUT (#4098): {elapsed:?}" + ); + } + + /// Gate (#3729): when `session/new` advertises no modes, a non-default + /// permission mode must NOT emit `session/set_config_option`, while session + /// setup still succeeds. The fake agent records every inbound frame to a + /// capture file; absence of the method string is the assertion. + #[tokio::test] + async fn test_unadvertised_permission_mode_sends_no_set_config_option() { + let capture = + std::env::temp_dir().join(format!("buzz-acp-gate-capture-{}.jsonl", Uuid::new_v4())); + + // Respond to session/new with a Hermes-shaped result (no `modes` + // member), recording every inbound frame first. Stay silent on anything + // else, so an (unwanted) set_config_option would be both captured and + // left to time out. + let script = r#"while IFS= read -r -t 10 line; do + printf '%s\n' "$line" >> __CAPTURE__ + case "$line" in + *session/new*) printf '%s\n' '{"jsonrpc":"2.0","id":0,"result":{"sessionId":"ses_gate"}}' ;; + esac +done"# + .replace("__CAPTURE__", &capture.display().to_string()); + + let acp = AcpClient::spawn("bash", &["-c".to_string(), script], &[], false) + .await + .expect("failed to spawn gate test agent"); + let mut agent = OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + agent_name: "unknown".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }; + let mut ctx = make_prompt_context_no_owner(); + ctx.permission_mode = PermissionMode::BypassPermissions; + + let session_id = create_session_and_apply_model(&mut agent, &ctx, None, None, None) + .await + .expect("session setup must succeed when the mode is gated out"); + + assert_eq!(session_id, "ses_gate"); + let sent = std::fs::read_to_string(&capture).expect("capture file must exist"); + assert!( + sent.contains("session/new"), + "sanity: the capture must contain the session/new frame: {sent}" + ); + assert!( + !sent.contains("session/set_config_option"), + "no set_config_option frame may be sent for an unadvertised mode: {sent}" + ); + + let _ = std::fs::remove_file(&capture); + } } From b5f6635b47543bde8672ba6c367c9ed3640344ab Mon Sep 17 00:00:00 2001 From: Maxime Tolos Date: Mon, 3 Aug 2026 23:51:05 +0200 Subject: [PATCH 2/2] test(acp): harden permission mode regression coverage Signed-off-by: Maxime Tolos --- crates/buzz-acp/src/pool.rs | 167 ++++++++++++++++++++++-------------- 1 file changed, 104 insertions(+), 63 deletions(-) diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 7e44b8981c..47edc829e6 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -6915,53 +6915,15 @@ mod tests { // the #4098 hypothesis: a silent agent yields a fast fatal Timeout, // never the 60s REQUEST_TIMEOUT stall from the issue log. - #[test] - fn test_agent_supports_mode_true_when_mode_advertised() { - let session_new = json!({ - "sessionId": "ses_modes", - "modes": { - "currentModeId": "default", - "availableModes": [ - { "id": "default", "name": "Default" }, - { "id": "bypassPermissions", "name": "Bypass" } - ] - } - }); - assert!(agent_supports_mode(&session_new, "bypassPermissions")); - assert!(agent_supports_mode(&session_new, "default")); - } - - #[test] - fn test_agent_supports_mode_false_when_modes_absent() { - // Hermes-shaped session/new: no `modes` member at all. - let session_new = json!({ "sessionId": "ses_no_modes" }); - assert!(!agent_supports_mode(&session_new, "bypassPermissions")); - // An explicit `modes: null` behaves the same as absent. - let session_new_null = json!({ "sessionId": "ses_no_modes", "modes": null }); - assert!(!agent_supports_mode(&session_new_null, "bypassPermissions")); - } - - #[test] - fn test_agent_supports_mode_false_when_mode_unlisted() { - // The #3729 OpenCode arm: `availableModes` exists but does not list the - // requested mode — sending it must be suppressed. - let session_new = json!({ - "sessionId": "ses_modes", - "modes": { - "currentModeId": "default", - "availableModes": [ { "id": "default", "name": "Default" } ] - } - }); - assert!(!agent_supports_mode(&session_new, "bypassPermissions")); - } - /// A Hermes-shaped agent that answers `session/new` advertising the mode and /// then goes silent on `session/set_config_option` must fail fast: the outer /// PERMISSION_MODE_TIMEOUT (5s) fires and maps to a fatal AcpError::Timeout — /// never the 60s REQUEST_TIMEOUT stall from the #4098 log. + #[cfg(unix)] #[tokio::test] async fn test_apply_permission_mode_silent_agent_fails_fast_with_timeout() { - // Reads stdin forever, never writes a response. + // Reads stdin forever, never writes a response. The test watchdog keeps + // a broken timeout implementation from hanging the suite indefinitely. let mut acp = AcpClient::spawn( "bash", &["-c".to_string(), "cat > /dev/null".to_string()], @@ -6971,50 +6933,127 @@ mod tests { .await .expect("failed to spawn silent test agent"); - let started = std::time::Instant::now(); - let result = - apply_permission_mode(&mut acp, "ses_silent", &PermissionMode::BypassPermissions).await; - let elapsed = started.elapsed(); + let result = tokio::time::timeout( + Duration::from_secs(15), + apply_permission_mode(&mut acp, "ses_silent", &PermissionMode::BypassPermissions), + ) + .await; + // Do this before assertions so normal assertion failures do not leave a + // live subprocess behind; AcpClient::Drop remains only the panic-path + // fallback. + acp.shutdown().await; + let result = result.expect("permission-mode test exceeded its 15s watchdog"); let err = result.expect_err("a silent agent must fail the permission-mode set"); assert!( matches!(err, AcpError::Timeout(d) if d == PERMISSION_MODE_TIMEOUT), "expected AcpError::Timeout(PERMISSION_MODE_TIMEOUT), got {err:?}" ); + } + + /// Positive end-to-end pin: an advertised non-default mode must emit the + /// expected `session/set_config_option` frame, not merely pass the helper + /// advertisement unit test. + #[cfg(unix)] + #[tokio::test] + async fn test_advertised_permission_mode_emits_set_config_option() { + let capture = std::env::temp_dir().join(format!( + "buzz-acp-positive-capture-{}.jsonl", + Uuid::new_v4() + )); + let extra_env = vec![( + "CAPTURE_FILE".to_string(), + capture.to_string_lossy().into_owned(), + )]; + + // Record each inbound frame and answer the two requests used by this + // test. The path comes through the environment so arbitrary temp paths + // cannot change the Bash program's syntax. + let script = r#"while IFS= read -r -t 10 line; do + printf '%s\n' "$line" >> "$CAPTURE_FILE" + case "$line" in + *session/new*) printf '%s\n' '{"jsonrpc":"2.0","id":0,"result":{"sessionId":"ses_advertised","modes":{"availableModes":[{"id":"bypassPermissions","name":"Bypass"}]}}}' ;; + *session/set_config_option*) printf '%s\n' '{"jsonrpc":"2.0","id":1,"result":{}}' ;; + esac +done"#; + + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), script.to_string()], + &extra_env, + false, + ) + .await + .expect("failed to spawn advertised-mode test agent"); + let mut agent = OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + agent_name: "unknown".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }; + let mut ctx = make_prompt_context_no_owner(); + ctx.permission_mode = PermissionMode::BypassPermissions; + + let session_result = + create_session_and_apply_model(&mut agent, &ctx, None, None, None).await; + agent.acp.shutdown().await; + let session_id = session_result.expect("advertised mode session setup must succeed"); + let sent = std::fs::read_to_string(&capture).expect("capture file must exist"); + + assert_eq!(session_id, "ses_advertised"); + assert!( + sent.contains("session/set_config_option"), + "advertised mode must emit session/set_config_option: {sent}" + ); assert!( - elapsed >= Duration::from_secs(4), - "the ~5s bound must actually elapse, not fail fast on a transport error: {elapsed:?}" + sent.contains("\"configId\":\"mode\""), + "permission mode must use configId=mode: {sent}" ); assert!( - elapsed < Duration::from_secs(10), - "permission-mode set must bound far below the 60s REQUEST_TIMEOUT (#4098): {elapsed:?}" + sent.contains("\"value\":\"bypassPermissions\""), + "permission mode must emit the requested wire value: {sent}" ); + + let _ = std::fs::remove_file(&capture); } /// Gate (#3729): when `session/new` advertises no modes, a non-default /// permission mode must NOT emit `session/set_config_option`, while session - /// setup still succeeds. The fake agent records every inbound frame to a - /// capture file; absence of the method string is the assertion. + /// setup still succeeds. These Bash/`read -t` subprocess tests are Unix-only + /// because they intentionally exercise the repository's POSIX agent-spawn path. + #[cfg(unix)] #[tokio::test] async fn test_unadvertised_permission_mode_sends_no_set_config_option() { let capture = std::env::temp_dir().join(format!("buzz-acp-gate-capture-{}.jsonl", Uuid::new_v4())); + let extra_env = vec![( + "CAPTURE_FILE".to_string(), + capture.to_string_lossy().into_owned(), + )]; // Respond to session/new with a Hermes-shaped result (no `modes` // member), recording every inbound frame first. Stay silent on anything - // else, so an (unwanted) set_config_option would be both captured and - // left to time out. + // else, so an (unwanted) set_config_option would be captured. let script = r#"while IFS= read -r -t 10 line; do - printf '%s\n' "$line" >> __CAPTURE__ + printf '%s\n' "$line" >> "$CAPTURE_FILE" case "$line" in *session/new*) printf '%s\n' '{"jsonrpc":"2.0","id":0,"result":{"sessionId":"ses_gate"}}' ;; esac -done"# - .replace("__CAPTURE__", &capture.display().to_string()); +done"#; - let acp = AcpClient::spawn("bash", &["-c".to_string(), script], &[], false) - .await - .expect("failed to spawn gate test agent"); + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), script.to_string()], + &extra_env, + false, + ) + .await + .expect("failed to spawn gate test agent"); let mut agent = OwnedAgent { index: 0, acp, @@ -7029,12 +7068,14 @@ done"# let mut ctx = make_prompt_context_no_owner(); ctx.permission_mode = PermissionMode::BypassPermissions; - let session_id = create_session_and_apply_model(&mut agent, &ctx, None, None, None) - .await - .expect("session setup must succeed when the mode is gated out"); + let session_result = + create_session_and_apply_model(&mut agent, &ctx, None, None, None).await; + agent.acp.shutdown().await; + let session_id = + session_result.expect("session setup must succeed when the mode is gated out"); + let sent = std::fs::read_to_string(&capture).expect("capture file must exist"); assert_eq!(session_id, "ses_gate"); - let sent = std::fs::read_to_string(&capture).expect("capture file must exist"); assert!( sent.contains("session/new"), "sanity: the capture must contain the session/new frame: {sent}"