Merge remote-tracking branch 'origin/dori/tools-parity'

This commit is contained in:
YeonGyu-Kim
2026-04-02 18:12:10 +09:00
2 changed files with 32 additions and 19 deletions

View File

@@ -3732,7 +3732,8 @@ fn build_runtime(
progress_reporter, progress_reporter,
)?, )?,
CliToolExecutor::new(allowed_tools.clone(), emit_output, tool_registry.clone()), CliToolExecutor::new(allowed_tools.clone(), emit_output, tool_registry.clone()),
permission_policy(permission_mode, &feature_config, &tool_registry), permission_policy(permission_mode, &feature_config, &tool_registry)
.map_err(std::io::Error::other)?,
system_prompt, system_prompt,
&feature_config, &feature_config,
); );
@@ -4731,13 +4732,13 @@ fn permission_policy(
mode: PermissionMode, mode: PermissionMode,
feature_config: &runtime::RuntimeFeatureConfig, feature_config: &runtime::RuntimeFeatureConfig,
tool_registry: &GlobalToolRegistry, tool_registry: &GlobalToolRegistry,
) -> PermissionPolicy { ) -> Result<PermissionPolicy, String> {
tool_registry.permission_specs(None).into_iter().fold( Ok(tool_registry.permission_specs(None)?.into_iter().fold(
PermissionPolicy::new(mode).with_permission_rules(feature_config.permission_rules()), PermissionPolicy::new(mode).with_permission_rules(feature_config.permission_rules()),
|policy, (name, required_permission)| { |policy, (name, required_permission)| {
policy.with_tool_requirement(name, required_permission) policy.with_tool_requirement(name, required_permission)
}, },
) ))
} }
fn convert_messages(messages: &[ConversationMessage]) -> Vec<InputMessage> { fn convert_messages(messages: &[ConversationMessage]) -> Vec<InputMessage> {
@@ -5391,7 +5392,8 @@ mod tests {
PermissionMode::ReadOnly, PermissionMode::ReadOnly,
&feature_config, &feature_config,
&registry_with_plugin_tool(), &registry_with_plugin_tool(),
); )
.expect("permission policy should build");
let required = policy.required_mode_for("plugin_echo"); let required = policy.required_mode_for("plugin_echo");
assert_eq!(required, PermissionMode::WorkspaceWrite); assert_eq!(required, PermissionMode::WorkspaceWrite);
} }

View File

@@ -173,7 +173,7 @@ impl GlobalToolRegistry {
pub fn permission_specs( pub fn permission_specs(
&self, &self,
allowed_tools: Option<&BTreeSet<String>>, allowed_tools: Option<&BTreeSet<String>>,
) -> Vec<(String, PermissionMode)> { ) -> Result<Vec<(String, PermissionMode)>, String> {
let builtin = mvp_tool_specs() let builtin = mvp_tool_specs()
.into_iter() .into_iter()
.filter(|spec| allowed_tools.is_none_or(|allowed| allowed.contains(spec.name))) .filter(|spec| allowed_tools.is_none_or(|allowed| allowed.contains(spec.name)))
@@ -186,12 +186,11 @@ impl GlobalToolRegistry {
.is_none_or(|allowed| allowed.contains(tool.definition().name.as_str())) .is_none_or(|allowed| allowed.contains(tool.definition().name.as_str()))
}) })
.map(|tool| { .map(|tool| {
( permission_mode_from_plugin(tool.required_permission())
tool.definition().name.clone(), .map(|permission| (tool.definition().name.clone(), permission))
permission_mode_from_plugin(tool.required_permission()), })
) .collect::<Result<Vec<_>, _>>()?;
}); Ok(builtin.chain(plugin).collect())
builtin.chain(plugin).collect()
} }
pub fn execute(&self, name: &str, input: &Value) -> Result<String, String> { pub fn execute(&self, name: &str, input: &Value) -> Result<String, String> {
@@ -211,12 +210,12 @@ fn normalize_tool_name(value: &str) -> String {
value.trim().replace('-', "_").to_ascii_lowercase() value.trim().replace('-', "_").to_ascii_lowercase()
} }
fn permission_mode_from_plugin(value: &str) -> PermissionMode { fn permission_mode_from_plugin(value: &str) -> Result<PermissionMode, String> {
match value { match value {
"read-only" => PermissionMode::ReadOnly, "read-only" => Ok(PermissionMode::ReadOnly),
"workspace-write" => PermissionMode::WorkspaceWrite, "workspace-write" => Ok(PermissionMode::WorkspaceWrite),
"danger-full-access" => PermissionMode::DangerFullAccess, "danger-full-access" => Ok(PermissionMode::DangerFullAccess),
other => panic!("unsupported plugin permission: {other}"), other => Err(format!("unsupported plugin permission: {other}")),
} }
} }
@@ -3102,8 +3101,9 @@ mod tests {
use super::{ use super::{
agent_permission_policy, allowed_tools_for_subagent, execute_agent_with_spawn, agent_permission_policy, allowed_tools_for_subagent, execute_agent_with_spawn,
execute_tool, final_assistant_text, mvp_tool_specs, persist_agent_terminal_state, execute_tool, final_assistant_text, mvp_tool_specs, permission_mode_from_plugin,
push_output_block, AgentInput, AgentJob, SubagentToolExecutor, persist_agent_terminal_state, push_output_block, AgentInput, AgentJob,
SubagentToolExecutor,
}; };
use api::OutputContentBlock; use api::OutputContentBlock;
use runtime::{ApiRequest, AssistantEvent, ConversationRuntime, RuntimeError, Session}; use runtime::{ApiRequest, AssistantEvent, ConversationRuntime, RuntimeError, Session};
@@ -3151,6 +3151,17 @@ mod tests {
assert!(error.contains("unsupported tool")); assert!(error.contains("unsupported tool"));
} }
#[test]
fn permission_mode_from_plugin_rejects_invalid_inputs() {
let unknown_permission = permission_mode_from_plugin("admin")
.expect_err("unknown plugin permission should fail");
assert!(unknown_permission.contains("unsupported plugin permission: admin"));
let empty_permission = permission_mode_from_plugin("")
.expect_err("empty plugin permission should fail");
assert!(empty_permission.contains("unsupported plugin permission: "));
}
#[test] #[test]
fn web_fetch_returns_prompt_aware_summary() { fn web_fetch_returns_prompt_aware_summary() {
let server = TestServer::spawn(Arc::new(|request_line: &str| { let server = TestServer::spawn(Arc::new(|request_line: &str| {