huskies: merge 555_bug_agent_permission_prompts_flood_matrix_chat_instead_of_being_auto_denied

This commit is contained in:
dave
2026-04-13 14:58:39 +00:00
parent d618bc3b32
commit 7977b7c5f8
+59 -8
View File
@@ -148,11 +148,31 @@ pub(super) async fn tool_prompt_permission(
.to_string(); .to_string();
let tool_input = args.get("input").cloned().unwrap_or(json!({})); let tool_input = args.get("input").cloned().unwrap_or(json!({}));
// Auto-deny immediately if no interactive session is currently listening on
// perm_rx. Interactive sessions (WebSocket, Matrix bot chat) hold the
// perm_rx lock for the duration of a chat. If try_lock succeeds, nobody is
// listening — this is a background agent call that should never reach chat.
//
// Without this check, agent permission requests queue in the channel and
// get forwarded to Matrix/Slack/etc. at the start of the next user session,
// flooding chat with stale agent prompts.
if ctx.perm_rx.try_lock().is_ok() {
crate::slog!(
"[permission] Auto-denied '{tool_name}' (no interactive session — agent mode)"
);
return serde_json::to_string_pretty(&json!({
"behavior": "deny",
"message": format!(
"Permission denied for '{tool_name}'. No interactive session active."
)
}))
.map_err(|e| format!("Serialization error: {e}"));
}
let request_id = uuid::Uuid::new_v4().to_string(); let request_id = uuid::Uuid::new_v4().to_string();
let (response_tx, response_rx) = tokio::sync::oneshot::channel(); let (response_tx, response_rx) = tokio::sync::oneshot::channel();
// Try to forward to the interactive session (WebSocket/Matrix). // Forward to the active interactive session.
// If no session is active (headless agent), auto-deny the permission.
if ctx if ctx
.perm_tx .perm_tx
.send(crate::http::context::PermissionForward { .send(crate::http::context::PermissionForward {
@@ -163,12 +183,10 @@ pub(super) async fn tool_prompt_permission(
}) })
.is_err() .is_err()
{ {
crate::slog!( crate::slog!("[permission] Auto-denied '{tool_name}' (perm_tx send failed)");
"[permission] Auto-denied '{tool_name}' (no interactive session — agent mode)"
);
return serde_json::to_string_pretty(&json!({ return serde_json::to_string_pretty(&json!({
"behavior": "deny", "behavior": "deny",
"message": format!("Permission denied for '{tool_name}'. Use the appropriate MCP tool instead (e.g. run_tests, run_build, run_lint).") "message": format!("Permission denied for '{tool_name}'.")
})) }))
.map_err(|e| format!("Serialization error: {e}")); .map_err(|e| format!("Serialization error: {e}"));
} }
@@ -443,15 +461,40 @@ mod tests {
assert_eq!(parsed["totals"]["records"], 1); assert_eq!(parsed["totals"]["records"], 1);
} }
#[tokio::test]
async fn tool_prompt_permission_auto_denies_without_interactive_session() {
let tmp = tempfile::tempdir().unwrap();
let ctx = test_ctx(tmp.path());
// No task holds perm_rx — simulates a background agent with no
// interactive session active. Should auto-deny immediately.
let result = tool_prompt_permission(
&json!({"tool_name": "Bash", "input": {"command": "echo hello"}}),
&ctx,
)
.await
.expect("auto-deny must return Ok");
let parsed: Value = serde_json::from_str(&result).expect("result should be valid JSON");
assert_eq!(
parsed["behavior"], "deny",
"must auto-deny when no interactive session holds perm_rx"
);
}
#[tokio::test] #[tokio::test]
async fn tool_prompt_permission_approved_returns_updated_input() { async fn tool_prompt_permission_approved_returns_updated_input() {
let tmp = tempfile::tempdir().unwrap(); let tmp = tempfile::tempdir().unwrap();
let ctx = test_ctx(tmp.path()); let ctx = test_ctx(tmp.path());
// Spawn a task that immediately sends approval through the channel. // Simulate an interactive session: lock perm_rx first, signal readiness,
// then respond with approval. The try_lock() inside tool_prompt_permission
// must fail (lock held) so the request is forwarded rather than auto-denied.
let (ready_tx, ready_rx) = tokio::sync::oneshot::channel::<()>();
let perm_rx = ctx.perm_rx.clone(); let perm_rx = ctx.perm_rx.clone();
tokio::spawn(async move { tokio::spawn(async move {
let mut rx = perm_rx.lock().await; let mut rx = perm_rx.lock().await;
let _ = ready_tx.send(()); // signal: lock is held
if let Some(forward) = rx.recv().await { if let Some(forward) = rx.recv().await {
let _ = forward let _ = forward
.response_tx .response_tx
@@ -459,6 +502,9 @@ mod tests {
} }
}); });
// Wait until the spawned task holds the perm_rx lock.
ready_rx.await.unwrap();
let result = tool_prompt_permission( let result = tool_prompt_permission(
&json!({"tool_name": "Bash", "input": {"command": "echo hello"}}), &json!({"tool_name": "Bash", "input": {"command": "echo hello"}}),
&ctx, &ctx,
@@ -482,10 +528,12 @@ mod tests {
let tmp = tempfile::tempdir().unwrap(); let tmp = tempfile::tempdir().unwrap();
let ctx = test_ctx(tmp.path()); let ctx = test_ctx(tmp.path());
// Spawn a task that immediately sends denial through the channel. // Simulate an interactive session: lock perm_rx first, then deny.
let (ready_tx, ready_rx) = tokio::sync::oneshot::channel::<()>();
let perm_rx = ctx.perm_rx.clone(); let perm_rx = ctx.perm_rx.clone();
tokio::spawn(async move { tokio::spawn(async move {
let mut rx = perm_rx.lock().await; let mut rx = perm_rx.lock().await;
let _ = ready_tx.send(()); // signal: lock is held
if let Some(forward) = rx.recv().await { if let Some(forward) = rx.recv().await {
let _ = forward let _ = forward
.response_tx .response_tx
@@ -493,6 +541,9 @@ mod tests {
} }
}); });
// Wait until the spawned task holds the perm_rx lock.
ready_rx.await.unwrap();
let result = tool_prompt_permission(&json!({"tool_name": "Write", "input": {}}), &ctx) let result = tool_prompt_permission(&json!({"tool_name": "Write", "input": {}}), &ctx)
.await .await
.expect("denial must return Ok, not Err"); .expect("denial must return Ok, not Err");