Have agent servers respect always_allow_tool_actions
This commit is contained in:
parent
45af1fcc2f
commit
c655428282
4 changed files with 160 additions and 1 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
@ -153,6 +153,7 @@ version = "0.1.0"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"acp_thread",
|
"acp_thread",
|
||||||
"agent-client-protocol",
|
"agent-client-protocol",
|
||||||
|
"agent_settings",
|
||||||
"agentic-coding-protocol",
|
"agentic-coding-protocol",
|
||||||
"anyhow",
|
"anyhow",
|
||||||
"collections",
|
"collections",
|
||||||
|
|
|
@ -19,6 +19,7 @@ doctest = false
|
||||||
[dependencies]
|
[dependencies]
|
||||||
acp_thread.workspace = true
|
acp_thread.workspace = true
|
||||||
agent-client-protocol.workspace = true
|
agent-client-protocol.workspace = true
|
||||||
|
agent_settings.workspace = true
|
||||||
agentic-coding-protocol.workspace = true
|
agentic-coding-protocol.workspace = true
|
||||||
anyhow.workspace = true
|
anyhow.workspace = true
|
||||||
collections.workspace = true
|
collections.workspace = true
|
||||||
|
|
|
@ -3,6 +3,7 @@ use std::path::PathBuf;
|
||||||
use crate::claude::tools::{ClaudeTool, EditToolParams, ReadToolParams};
|
use crate::claude::tools::{ClaudeTool, EditToolParams, ReadToolParams};
|
||||||
use acp_thread::AcpThread;
|
use acp_thread::AcpThread;
|
||||||
use agent_client_protocol as acp;
|
use agent_client_protocol as acp;
|
||||||
|
use agent_settings::AgentSettings;
|
||||||
use anyhow::{Context, Result};
|
use anyhow::{Context, Result};
|
||||||
use collections::HashMap;
|
use collections::HashMap;
|
||||||
use context_server::listener::{McpServerTool, ToolResponse};
|
use context_server::listener::{McpServerTool, ToolResponse};
|
||||||
|
@ -13,6 +14,7 @@ use context_server::types::{
|
||||||
use gpui::{App, AsyncApp, Task, WeakEntity};
|
use gpui::{App, AsyncApp, Task, WeakEntity};
|
||||||
use schemars::JsonSchema;
|
use schemars::JsonSchema;
|
||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
|
use settings::Settings;
|
||||||
|
|
||||||
pub struct ClaudeZedMcpServer {
|
pub struct ClaudeZedMcpServer {
|
||||||
server: context_server::listener::McpServer,
|
server: context_server::listener::McpServer,
|
||||||
|
@ -114,6 +116,7 @@ pub struct PermissionToolParams {
|
||||||
|
|
||||||
#[derive(Serialize)]
|
#[derive(Serialize)]
|
||||||
#[serde(rename_all = "camelCase")]
|
#[serde(rename_all = "camelCase")]
|
||||||
|
#[cfg_attr(test, derive(serde::Deserialize))]
|
||||||
pub struct PermissionToolResponse {
|
pub struct PermissionToolResponse {
|
||||||
behavior: PermissionToolBehavior,
|
behavior: PermissionToolBehavior,
|
||||||
updated_input: serde_json::Value,
|
updated_input: serde_json::Value,
|
||||||
|
@ -121,7 +124,8 @@ pub struct PermissionToolResponse {
|
||||||
|
|
||||||
#[derive(Serialize)]
|
#[derive(Serialize)]
|
||||||
#[serde(rename_all = "snake_case")]
|
#[serde(rename_all = "snake_case")]
|
||||||
enum PermissionToolBehavior {
|
#[cfg_attr(test, derive(serde::Deserialize))]
|
||||||
|
pub enum PermissionToolBehavior {
|
||||||
Allow,
|
Allow,
|
||||||
Deny,
|
Deny,
|
||||||
}
|
}
|
||||||
|
@ -141,6 +145,26 @@ impl McpServerTool for PermissionTool {
|
||||||
input: Self::Input,
|
input: Self::Input,
|
||||||
cx: &mut AsyncApp,
|
cx: &mut AsyncApp,
|
||||||
) -> Result<ToolResponse<Self::Output>> {
|
) -> Result<ToolResponse<Self::Output>> {
|
||||||
|
// Check if we should automatically allow tool actions
|
||||||
|
let always_allow =
|
||||||
|
cx.update(|cx| AgentSettings::get_global(cx).always_allow_tool_actions)?;
|
||||||
|
|
||||||
|
if always_allow {
|
||||||
|
// If always_allow_tool_actions is true, immediately return Allow without prompting
|
||||||
|
let response = PermissionToolResponse {
|
||||||
|
behavior: PermissionToolBehavior::Allow,
|
||||||
|
updated_input: input.input,
|
||||||
|
};
|
||||||
|
|
||||||
|
return Ok(ToolResponse {
|
||||||
|
content: vec![ToolResponseContent::Text {
|
||||||
|
text: serde_json::to_string(&response)?,
|
||||||
|
}],
|
||||||
|
structured_content: (),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
// Otherwise, proceed with the normal permission flow
|
||||||
let mut thread_rx = self.thread_rx.clone();
|
let mut thread_rx = self.thread_rx.clone();
|
||||||
let Some(thread) = thread_rx.recv().await?.upgrade() else {
|
let Some(thread) = thread_rx.recv().await?.upgrade() else {
|
||||||
anyhow::bail!("Thread closed");
|
anyhow::bail!("Thread closed");
|
||||||
|
@ -300,3 +324,78 @@ impl McpServerTool for EditTool {
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
use gpui::TestAppContext;
|
||||||
|
use project::Project;
|
||||||
|
use settings::{Settings, SettingsStore};
|
||||||
|
|
||||||
|
#[gpui::test]
|
||||||
|
async fn test_permission_tool_respects_always_allow_setting(cx: &mut TestAppContext) {
|
||||||
|
// Initialize settings
|
||||||
|
cx.update(|cx| {
|
||||||
|
let settings_store = SettingsStore::test(cx);
|
||||||
|
cx.set_global(settings_store);
|
||||||
|
agent_settings::init(cx);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Create a test thread
|
||||||
|
let project = cx.update(|cx| gpui::Entity::new(cx, |_cx| Project::local()));
|
||||||
|
let thread = cx.update(|cx| {
|
||||||
|
gpui::Entity::new(cx, |_cx| {
|
||||||
|
acp_thread::AcpThread::new(
|
||||||
|
acp::ConnectionId("test".into()),
|
||||||
|
project,
|
||||||
|
std::path::Path::new("/tmp"),
|
||||||
|
)
|
||||||
|
})
|
||||||
|
});
|
||||||
|
|
||||||
|
let (tx, rx) = watch::channel(thread.downgrade());
|
||||||
|
let tool = PermissionTool { thread_rx: rx };
|
||||||
|
|
||||||
|
// Test with always_allow_tool_actions = true
|
||||||
|
cx.update(|cx| {
|
||||||
|
AgentSettings::override_global(
|
||||||
|
AgentSettings {
|
||||||
|
always_allow_tool_actions: true,
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
cx,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
let input = PermissionToolParams {
|
||||||
|
tool_name: "test_tool".to_string(),
|
||||||
|
input: serde_json::json!({"test": "data"}),
|
||||||
|
tool_use_id: Some("test_id".to_string()),
|
||||||
|
};
|
||||||
|
|
||||||
|
let result = tool.run(input.clone(), &mut cx.to_async()).await.unwrap();
|
||||||
|
|
||||||
|
// Should return Allow without prompting
|
||||||
|
assert_eq!(result.content.len(), 1);
|
||||||
|
if let ToolResponseContent::Text { text } = &result.content[0] {
|
||||||
|
let response: PermissionToolResponse = serde_json::from_str(text).unwrap();
|
||||||
|
assert!(matches!(response.behavior, PermissionToolBehavior::Allow));
|
||||||
|
} else {
|
||||||
|
panic!("Expected text response");
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test with always_allow_tool_actions = false
|
||||||
|
cx.update(|cx| {
|
||||||
|
AgentSettings::override_global(
|
||||||
|
AgentSettings {
|
||||||
|
always_allow_tool_actions: false,
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
cx,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
// This test would require mocking the permission prompt response
|
||||||
|
// In the real scenario, it would wait for user input
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
@ -7,6 +7,7 @@ use std::{
|
||||||
use crate::{AgentServer, AgentServerSettings, AllAgentServersSettings};
|
use crate::{AgentServer, AgentServerSettings, AllAgentServersSettings};
|
||||||
use acp_thread::{AcpThread, AgentThreadEntry, ToolCall, ToolCallStatus};
|
use acp_thread::{AcpThread, AgentThreadEntry, ToolCall, ToolCallStatus};
|
||||||
use agent_client_protocol as acp;
|
use agent_client_protocol as acp;
|
||||||
|
use agent_settings::AgentSettings;
|
||||||
|
|
||||||
use futures::{FutureExt, StreamExt, channel::mpsc, select};
|
use futures::{FutureExt, StreamExt, channel::mpsc, select};
|
||||||
use gpui::{Entity, TestAppContext};
|
use gpui::{Entity, TestAppContext};
|
||||||
|
@ -241,6 +242,57 @@ pub async fn test_tool_call_with_confirmation(
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub async fn test_tool_call_always_allow(
|
||||||
|
server: impl AgentServer + 'static,
|
||||||
|
cx: &mut TestAppContext,
|
||||||
|
) {
|
||||||
|
let fs = init_test(cx).await;
|
||||||
|
let project = Project::test(fs, [path!("/private/tmp").as_ref()], cx).await;
|
||||||
|
|
||||||
|
// Enable always_allow_tool_actions
|
||||||
|
cx.update(|cx| {
|
||||||
|
AgentSettings::override_global(
|
||||||
|
AgentSettings {
|
||||||
|
always_allow_tool_actions: true,
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
cx,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
let thread = new_test_thread(server, project.clone(), "/private/tmp", cx).await;
|
||||||
|
let full_turn = thread.update(cx, |thread, cx| {
|
||||||
|
thread.send_raw(
|
||||||
|
r#"Run `touch hello.txt && echo "Hello, world!" | tee hello.txt`"#,
|
||||||
|
cx,
|
||||||
|
)
|
||||||
|
});
|
||||||
|
|
||||||
|
// Wait for the tool call to complete
|
||||||
|
full_turn.await.unwrap();
|
||||||
|
|
||||||
|
thread.read_with(cx, |thread, _cx| {
|
||||||
|
// With always_allow_tool_actions enabled, the tool call should be immediately allowed
|
||||||
|
// without waiting for confirmation
|
||||||
|
let tool_call_entry = thread
|
||||||
|
.entries()
|
||||||
|
.iter()
|
||||||
|
.find(|entry| matches!(entry, AgentThreadEntry::ToolCall(_)))
|
||||||
|
.expect("Expected a tool call entry");
|
||||||
|
|
||||||
|
let AgentThreadEntry::ToolCall(tool_call) = tool_call_entry else {
|
||||||
|
panic!("Expected tool call entry");
|
||||||
|
};
|
||||||
|
|
||||||
|
// Should be allowed, not waiting for confirmation
|
||||||
|
assert!(
|
||||||
|
matches!(tool_call.status, ToolCallStatus::Allowed { .. }),
|
||||||
|
"Expected tool call to be allowed automatically, but got {:?}",
|
||||||
|
tool_call.status
|
||||||
|
);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
pub async fn test_cancel(server: impl AgentServer + 'static, cx: &mut TestAppContext) {
|
pub async fn test_cancel(server: impl AgentServer + 'static, cx: &mut TestAppContext) {
|
||||||
let fs = init_test(cx).await;
|
let fs = init_test(cx).await;
|
||||||
|
|
||||||
|
@ -351,6 +403,12 @@ macro_rules! common_e2e_tests {
|
||||||
async fn cancel(cx: &mut ::gpui::TestAppContext) {
|
async fn cancel(cx: &mut ::gpui::TestAppContext) {
|
||||||
$crate::e2e_tests::test_cancel($server, cx).await;
|
$crate::e2e_tests::test_cancel($server, cx).await;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[::gpui::test]
|
||||||
|
#[cfg_attr(not(feature = "e2e"), ignore)]
|
||||||
|
async fn tool_call_always_allow(cx: &mut ::gpui::TestAppContext) {
|
||||||
|
$crate::e2e_tests::test_tool_call_always_allow($server, cx).await;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue