fix(tools): remove invalid override fallthrough (#2428)
Hardens the tool override path from #2420 so a broken replacement override cannot silently fall through to the original built-in tool. Validation: - git diff --check - CARGO_TARGET_DIR=/Volumes/VIXinSSD/codewhale-target/fix-plugin-override cargo test -p codewhale-tui tools::registry --all-features - CARGO_TARGET_DIR=/Volumes/VIXinSSD/codewhale-target/fix-plugin-override cargo test -p codewhale-tui tools::plugin --all-features
This commit is contained in:
@@ -425,11 +425,24 @@ impl ToolRegistry {
|
||||
_ => {
|
||||
// Script and Command overrides create replacement tools.
|
||||
use crate::tools::plugin::tool_from_override;
|
||||
if let Some(replacement) =
|
||||
tool_from_override(tool_name, override_cfg, plugin_dir)
|
||||
{
|
||||
self.register(replacement);
|
||||
tracing::info!("Tool '{}' replaced via config override", tool_name);
|
||||
match tool_from_override(tool_name, override_cfg, plugin_dir) {
|
||||
Some(replacement) => {
|
||||
self.register(replacement);
|
||||
tracing::info!("Tool '{}' replaced via config override", tool_name);
|
||||
}
|
||||
None => {
|
||||
if self.remove_tool(tool_name) {
|
||||
tracing::warn!(
|
||||
"Tool '{}' override did not create a replacement; removed the original tool to avoid override fallthrough",
|
||||
tool_name
|
||||
);
|
||||
} else {
|
||||
tracing::warn!(
|
||||
"Tool '{}' override did not create a replacement and no registered tool existed",
|
||||
tool_name
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1082,11 +1095,13 @@ impl ToolSpec for McpToolAdapter {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
|
||||
use serde_json::{Value, json};
|
||||
use tempfile::tempdir;
|
||||
|
||||
use crate::config::ToolOverride;
|
||||
use crate::tools::ToolRegistryBuilder;
|
||||
use crate::tools::spec::{
|
||||
ToolCapability, ToolContext, ToolError, ToolResult, ToolSpec, required_str,
|
||||
@@ -1155,6 +1170,32 @@ mod tests {
|
||||
assert_eq!(registry.len(), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_overrides_removes_original_when_replacement_is_missing() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let ctx = ToolContext::new(tmp.path().to_path_buf());
|
||||
let mut registry = ToolRegistryBuilder::new()
|
||||
.with_read_only_file_tools()
|
||||
.build(ctx);
|
||||
|
||||
assert!(registry.contains("read_file"));
|
||||
assert!(registry.contains("list_dir"));
|
||||
|
||||
let mut overrides = HashMap::new();
|
||||
overrides.insert(
|
||||
"read_file".to_string(),
|
||||
ToolOverride::Script {
|
||||
path: "missing-wrapper.sh".to_string(),
|
||||
args: None,
|
||||
},
|
||||
);
|
||||
|
||||
registry.apply_overrides(&overrides, tmp.path());
|
||||
|
||||
assert!(!registry.contains("read_file"));
|
||||
assert!(registry.contains("list_dir"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_registry_names() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
|
||||
Reference in New Issue
Block a user