From a10bbaf8de6287a72259616045347fb9ef1029b6 Mon Sep 17 00:00:00 2001 From: Yeachan-Heo Date: Wed, 1 Apr 2026 07:22:41 +0000 Subject: [PATCH] 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 --- rust/crates/rusty-claude-cli/src/main.rs | 55 +++++++++++++++++++++--- rust/crates/tools/src/lib.rs | 6 +-- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index 67e5965..b7188c2 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -301,9 +301,7 @@ fn resolve_model_alias(model: &str) -> &str { } fn normalize_allowed_tools(values: &[String]) -> Result, String> { - current_tool_registry() - .unwrap_or_else(|_| GlobalToolRegistry::builtin()) - .normalize_allowed_tools(values) + current_tool_registry()?.normalize_allowed_tools(values) } fn current_tool_registry() -> Result { @@ -3292,17 +3290,42 @@ mod tests { filter_tool_specs, format_compact_report, format_cost_report, format_model_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, - normalize_permission_mode, parse_args, parse_git_status_metadata, print_help_to, - push_output_block, render_config_report, render_memory_report, render_repl_help, - resolve_model_alias, response_to_events, resume_supported_slash_commands, status_context, - CliAction, CliOutputFormat, SlashCommand, StatusUsage, DEFAULT_MODEL, + normalize_permission_mode, parse_args, parse_git_status_metadata, permission_policy, + print_help_to, push_output_block, render_config_report, render_memory_report, + render_repl_help, resolve_model_alias, response_to_events, resume_supported_slash_commands, + status_context, CliAction, CliOutputFormat, SlashCommand, StatusUsage, DEFAULT_MODEL, }; use api::{MessageResponse, OutputContentBlock, Usage}; + use plugins::{PluginTool, PluginToolDefinition, PluginToolPermission}; use runtime::{AssistantEvent, ContentBlock, ConversationMessage, MessageRole, PermissionMode}; use serde_json::json; use std::path::PathBuf; 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] fn defaults_to_repl_when_no_args() { assert_eq!( @@ -3523,6 +3546,24 @@ mod tests { assert_eq!(names, vec!["read_file", "grep_search"]); } + #[test] + fn filtered_tool_specs_include_plugin_tools() { + let filtered = filter_tool_specs(®istry_with_plugin_tool(), None); + let names = filtered + .into_iter() + .map(|definition| definition.name) + .collect::>(); + 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, ®istry_with_plugin_tool()); + let required = policy.required_mode_for("plugin_echo"); + assert_eq!(required, PermissionMode::WorkspaceWrite); + } + #[test] fn shared_help_uses_resume_annotation_copy() { let help = commands::render_slash_command_help(); diff --git a/rust/crates/tools/src/lib.rs b/rust/crates/tools/src/lib.rs index 83b0b03..fea9ce1 100644 --- a/rust/crates/tools/src/lib.rs +++ b/rust/crates/tools/src/lib.rs @@ -3099,7 +3099,7 @@ mod tests { execute_tool, final_assistant_text, mvp_tool_specs, persist_agent_terminal_state, AgentInput, AgentJob, GlobalToolRegistry, SubagentToolExecutor, }; - use plugins::{PluginTool, PluginToolDefinition}; + use plugins::{PluginTool, PluginToolDefinition, PluginToolPermission}; use runtime::{ApiRequest, AssistantEvent, ConversationRuntime, RuntimeError, Session}; use serde_json::json; @@ -3181,7 +3181,7 @@ mod tests { }, script.display().to_string(), Vec::new(), - "workspace-write", + PluginToolPermission::WorkspaceWrite, script.parent().map(PathBuf::from), )]) .expect("registry should build"); @@ -3217,7 +3217,7 @@ mod tests { }, "echo".to_string(), Vec::new(), - "read-only", + PluginToolPermission::ReadOnly, None, )]) .expect_err("conflicting plugin tool should fail");