Keep plugin-aware CLI validation aligned with the shared registry

The shared /plugins command flow already routes through the plugin registry, but
allowed-tool normalization still fell back to builtin tools when registry
construction failed. This keeps plugin-related validation errors visible at the
CLI boundary and updates tools tests to use the enum-based plugin permission
API so workspace verification remains green.

Constraint: Plugin tool permissions are now strongly typed in the plugins crate
Rejected: Restore string-based permission arguments in tests | weakens the plugin API contract
Rejected: Keep builtin fallback in normalize_allowed_tools | masks plugin registry integration failures
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Do not silently bypass current_tool_registry() failures unless plugin-aware allowed-tool validation is intentionally being disabled
Tested: cargo test -p commands -- --nocapture; cargo test --workspace
Not-tested: Manual REPL /plugins interaction in a live session
This commit is contained in:
Yeachan-Heo
2026-04-01 07:22:41 +00:00
parent f967484b9a
commit a10bbaf8de
2 changed files with 51 additions and 10 deletions

View File

@@ -301,9 +301,7 @@ fn resolve_model_alias(model: &str) -> &str {
} }
fn normalize_allowed_tools(values: &[String]) -> Result<Option<AllowedToolSet>, String> { fn normalize_allowed_tools(values: &[String]) -> Result<Option<AllowedToolSet>, String> {
current_tool_registry() current_tool_registry()?.normalize_allowed_tools(values)
.unwrap_or_else(|_| GlobalToolRegistry::builtin())
.normalize_allowed_tools(values)
} }
fn current_tool_registry() -> Result<GlobalToolRegistry, String> { fn current_tool_registry() -> Result<GlobalToolRegistry, String> {
@@ -3292,17 +3290,42 @@ mod tests {
filter_tool_specs, format_compact_report, format_cost_report, format_model_report, filter_tool_specs, format_compact_report, format_cost_report, format_model_report,
format_model_switch_report, format_permissions_report, format_permissions_switch_report, format_model_switch_report, format_permissions_report, format_permissions_switch_report,
format_resume_report, format_status_report, format_tool_call_start, format_tool_result, format_resume_report, format_status_report, format_tool_call_start, format_tool_result,
normalize_permission_mode, parse_args, parse_git_status_metadata, print_help_to, normalize_permission_mode, parse_args, parse_git_status_metadata, permission_policy,
push_output_block, render_config_report, render_memory_report, render_repl_help, print_help_to, push_output_block, render_config_report, render_memory_report,
resolve_model_alias, response_to_events, resume_supported_slash_commands, status_context, render_repl_help, resolve_model_alias, response_to_events, resume_supported_slash_commands,
CliAction, CliOutputFormat, SlashCommand, StatusUsage, DEFAULT_MODEL, status_context, CliAction, CliOutputFormat, SlashCommand, StatusUsage, DEFAULT_MODEL,
}; };
use api::{MessageResponse, OutputContentBlock, Usage}; use api::{MessageResponse, OutputContentBlock, Usage};
use plugins::{PluginTool, PluginToolDefinition, PluginToolPermission};
use runtime::{AssistantEvent, ContentBlock, ConversationMessage, MessageRole, PermissionMode}; use runtime::{AssistantEvent, ContentBlock, ConversationMessage, MessageRole, PermissionMode};
use serde_json::json; use serde_json::json;
use std::path::PathBuf; use std::path::PathBuf;
use tools::GlobalToolRegistry; use tools::GlobalToolRegistry;
fn registry_with_plugin_tool() -> GlobalToolRegistry {
GlobalToolRegistry::with_plugin_tools(vec![PluginTool::new(
"plugin-demo@external",
"plugin-demo",
PluginToolDefinition {
name: "plugin_echo".to_string(),
description: Some("Echo plugin payload".to_string()),
input_schema: json!({
"type": "object",
"properties": {
"message": { "type": "string" }
},
"required": ["message"],
"additionalProperties": false
}),
},
"echo".to_string(),
Vec::new(),
PluginToolPermission::WorkspaceWrite,
None,
)])
.expect("plugin tool registry should build")
}
#[test] #[test]
fn defaults_to_repl_when_no_args() { fn defaults_to_repl_when_no_args() {
assert_eq!( assert_eq!(
@@ -3523,6 +3546,24 @@ mod tests {
assert_eq!(names, vec!["read_file", "grep_search"]); assert_eq!(names, vec!["read_file", "grep_search"]);
} }
#[test]
fn filtered_tool_specs_include_plugin_tools() {
let filtered = filter_tool_specs(&registry_with_plugin_tool(), None);
let names = filtered
.into_iter()
.map(|definition| definition.name)
.collect::<Vec<_>>();
assert!(names.contains(&"bash".to_string()));
assert!(names.contains(&"plugin_echo".to_string()));
}
#[test]
fn permission_policy_uses_plugin_tool_permissions() {
let policy = permission_policy(PermissionMode::ReadOnly, &registry_with_plugin_tool());
let required = policy.required_mode_for("plugin_echo");
assert_eq!(required, PermissionMode::WorkspaceWrite);
}
#[test] #[test]
fn shared_help_uses_resume_annotation_copy() { fn shared_help_uses_resume_annotation_copy() {
let help = commands::render_slash_command_help(); let help = commands::render_slash_command_help();

View File

@@ -3099,7 +3099,7 @@ mod tests {
execute_tool, final_assistant_text, mvp_tool_specs, persist_agent_terminal_state, execute_tool, final_assistant_text, mvp_tool_specs, persist_agent_terminal_state,
AgentInput, AgentJob, GlobalToolRegistry, SubagentToolExecutor, AgentInput, AgentJob, GlobalToolRegistry, SubagentToolExecutor,
}; };
use plugins::{PluginTool, PluginToolDefinition}; use plugins::{PluginTool, PluginToolDefinition, PluginToolPermission};
use runtime::{ApiRequest, AssistantEvent, ConversationRuntime, RuntimeError, Session}; use runtime::{ApiRequest, AssistantEvent, ConversationRuntime, RuntimeError, Session};
use serde_json::json; use serde_json::json;
@@ -3181,7 +3181,7 @@ mod tests {
}, },
script.display().to_string(), script.display().to_string(),
Vec::new(), Vec::new(),
"workspace-write", PluginToolPermission::WorkspaceWrite,
script.parent().map(PathBuf::from), script.parent().map(PathBuf::from),
)]) )])
.expect("registry should build"); .expect("registry should build");
@@ -3217,7 +3217,7 @@ mod tests {
}, },
"echo".to_string(), "echo".to_string(),
Vec::new(), Vec::new(),
"read-only", PluginToolPermission::ReadOnly,
None, None,
)]) )])
.expect_err("conflicting plugin tool should fail"); .expect_err("conflicting plugin tool should fail");