fix(tools): name a refused built-in override to the user, not only the log

D4 refuses a [tools.overrides] script/command entry keyed by a built-in and
keeps the built-in active. A user who had routed a built-in through such an
override (the old audit-wrapper example did exactly this for Bash) lost the
wrapper with only a runtime-log line to say so. Misconfiguration fails loud:
apply_overrides now returns what it refused, and the engine emits one status
line per refused key per process (the registry is rebuilt every turn), using
the same sentence as the log.

The engine wiring test asserts the refusal reaches the engine.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: CodeWhale Bot <[email protected]>
This commit is contained in:
CodeWhale BotandClaude Opus 5.5 committed 2026-09-30 11:02:40 -07:00
1 parent ccb9adfcc4
commit 078ba5597c
7 files changed
+65 -25

No files matched your search

+2 -2
View File
@@ -15,8 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
the session's approval setting like a script with no `approval:` line, and
the runtime log and `/plugin tools` name each script that still declares it.
A `[tools.overrides]` entry of `type = "script"` or `type = "command"` keyed
by a built-in tool name is refused with an error in the runtime log naming
the key, and the built-in stays active. `type = "disabled"` still turns a
by a built-in tool name is refused, and the built-in stays active; a status
line names the key once per session, and the runtime log records it. `type = "disabled"` still turns a
built-in off, and script or command overrides under a new name still work.
To keep a wrapper such as an audited shell, disable the built-in and give
the wrapper its own name
+2 -2
View File
@@ -15,8 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
the session's approval setting like a script with no `approval:` line, and
the runtime log and `/plugin tools` name each script that still declares it.
A `[tools.overrides]` entry of `type = "script"` or `type = "command"` keyed
by a built-in tool name is refused with an error in the runtime log naming
the key, and the built-in stays active. `type = "disabled"` still turns a
by a built-in tool name is refused, and the built-in stays active; a status
line names the key once per session, and the runtime log records it. `type = "disabled"` still turns a
built-in off, and script or command overrides under a new name still work.
To keep a wrapper such as an audited shell, disable the built-in and give
the wrapper its own name
+32 -8
View File
@@ -5319,8 +5319,19 @@ impl Engine {
.note_native_names(tool_registry.names());
attachment.sync_in_background();
}
let mut plugin_tool_names =
let (mut plugin_tool_names, refused_overrides) =
configure_plugin_tools(&mut tool_registry, self.config.tools.as_ref());
// The registry is rebuilt every turn: name each refused override to
// the user once per process, not once per turn.
for name in refused_overrides {
if first_override_refusal(&name) {
let _ = self
.send_event(Event::status(
crate::tools::registry::override_refusal_notice(&name),
))
.await;
}
}
// Extension tools go in last and never replace a name already present
// (`ToolRegistry::register` would overwrite it silently). Only this
// engine's own plugins' tools are installed.
@@ -7947,10 +7958,24 @@ fn plugin_tools_dir(tools_config: Option<&crate::config::ToolsConfig>) -> PathBu
default_plugin_tools_dir()
}
/// Whether this process has not yet told the user about the refused
/// `[tools.overrides.<name>]`.
fn first_override_refusal(name: &str) -> bool {
static NOTIFIED: std::sync::OnceLock<std::sync::Mutex<std::collections::HashSet<String>>> =
std::sync::OnceLock::new();
NOTIFIED
.get_or_init(Default::default)
.lock()
.unwrap_or_else(|poisoned| poisoned.into_inner())
.insert(name.to_string())
}
/// Load drop-in scripts and apply `[tools.overrides]`. Returns the tool names
/// this added and the overrides refused for naming a built-in (D4).
fn configure_plugin_tools(
tool_registry: &mut crate::tools::ToolRegistry,
tools_config: Option<&crate::config::ToolsConfig>,
) -> std::collections::HashSet<String> {
) -> (std::collections::HashSet<String>, Vec<String>) {
// Everything registered before the plugin directory loads is built in
// (native and host-dynamic tools); no script tool may replace it (D4).
let builtin_names: std::collections::HashSet<String> = tool_registry
@@ -7962,18 +7987,17 @@ fn configure_plugin_tools(
let plugin_dir = plugin_tools_dir(tools_config);
tool_registry.load_plugins(&plugin_dir);
if let Some(tools_config) = tools_config
&& let Some(ref overrides) = tools_config.overrides
{
tool_registry.apply_overrides(overrides, &plugin_dir, &builtin_names);
}
let refused = match tools_config.and_then(|config| config.overrides.as_ref()) {
Some(overrides) => tool_registry.apply_overrides(overrides, &plugin_dir, &builtin_names),
None => Vec::new(),
};
let names_after: std::collections::HashSet<String> = tool_registry
.names()
.into_iter()
.map(|s| s.to_string())
.collect();
&names_after - &builtin_names
(&names_after - &builtin_names, refused)
}
fn system_prompt_hash(prompt: Option<&SystemPrompt>) -> u64 {
+6 -1
View File
@@ -7768,7 +7768,7 @@ fn configure_plugin_tools_applies_overrides_after_discovered_plugins() {
.build(ctx);
let file = registry.get("File").expect("built-in File");
let plugin_names = configure_plugin_tools(&mut registry, Some(&tools_config));
let (plugin_names, refused) = configure_plugin_tools(&mut registry, Some(&tools_config));
let tool = registry.get("same_tool").expect("same_tool registered");
assert!(tool.description().contains("configured-command"));
@@ -7778,6 +7778,11 @@ fn configure_plugin_tools_applies_overrides_after_discovered_plugins() {
&file
));
assert!(!plugin_names.contains("File"));
// The refusal reaches the engine so it can name it to the user.
assert_eq!(refused, vec!["File".to_string()]);
assert!(
crate::tools::registry::override_refusal_notice("File").contains("[tools.overrides.File]")
);
}
fn make_plan(
+18 -8
View File
@@ -38,6 +38,15 @@ pub struct ToolRegistry {
api_cache: OnceLock<Vec<Tool>>,
}
/// The one sentence naming a `[tools.overrides]` entry that D4 refused, shared
/// by the runtime log and the engine's user-facing status line.
pub(crate) fn override_refusal_notice(tool_name: &str) -> String {
format!(
"Refused [tools.overrides.{name}]: a script or command override cannot replace the built-in tool '{name}', which stays active. Set type = \"disabled\" to turn it off, or key the override by a new tool name.",
name = crate::safe_label::SafeLabel::identifier(tool_name)
)
}
impl ToolRegistry {
/// Create a new empty registry with the given context.
#[must_use]
@@ -375,9 +384,10 @@ impl ToolRegistry {
/// the built-in stays active: script tools cannot shadow built-ins (D4,
/// CURRENT_DECISIONS §26).
///
/// Known limitation: like a drop-in name collision in [`Self::load_plugins`],
/// the refusal is an error in the runtime log only. `/plugin` does not list
/// `[tools.overrides]` entries, so it cannot show the refusal there.
/// Returns the refused override names so the engine can name each one to
/// the user ([`override_refusal_notice`]); the runtime log records every
/// refusal. `/plugin` does not list `[tools.overrides]` entries, so it
/// cannot show them there.
///
/// `plugin_dir` is used as the base for relative script paths.
pub fn apply_overrides(
@@ -385,7 +395,8 @@ impl ToolRegistry {
overrides: &std::collections::HashMap<String, crate::config::ToolOverride>,
plugin_dir: &Path,
builtin_names: &std::collections::HashSet<String>,
) {
) -> Vec<String> {
let mut refused = Vec::new();
for (tool_name, override_cfg) in overrides {
match override_cfg {
crate::config::ToolOverride::Disabled => {
@@ -396,10 +407,8 @@ impl ToolRegistry {
}
}
_ if builtin_names.contains(tool_name) => {
tracing::error!(
"Refusing [tools.overrides.{name}]: a script or command override cannot replace the built-in tool '{name}', which stays active; set type = \"disabled\" to turn it off, or key the override by a new tool name",
name = crate::safe_label::SafeLabel::identifier(tool_name)
);
tracing::error!("{}", override_refusal_notice(tool_name));
refused.push(tool_name.clone());
}
_ => {
// Script and Command overrides create replacement tools.
@@ -426,6 +435,7 @@ impl ToolRegistry {
}
}
}
refused
}
/// Load and register plugin tools from a directory.
+3 -2
View File
@@ -3091,8 +3091,9 @@ or add a script or command tool under a name of its own:
"Bash" = { type = "script", path = "audit-shell.sh" } # refused: Bash is built in
```
A `script` or `command` override keyed by a built-in is refused with an error
in the runtime log naming the key, and the built-in stays active. To route a
A `script` or `command` override keyed by a built-in is refused, and the
built-in stays active. A status line names the key once per session, and the
runtime log records it. To route a
built-in through your own wrapper, disable the built-in and register the
wrapper under a new name. An override keyed by a drop-in script's name still
replaces that script. Relative `path` values resolve against the plugin
+2 -2
View File
@@ -15,8 +15,8 @@ D9 describe the old behaviour. In `crates/tui/src/tools/plugin.rs` and
`ToolRegistry::apply_overrides`: a script's `# approval: auto` is ignored and
the tool gets the default a script with no `approval:` line gets (`Suggest`),
reported in the runtime log and `/plugin tools`; a `[tools.overrides]`
`script` / `command` entry keyed by a built-in is refused with a logged error
and the built-in stays active (`disabled` still works). D9(a) planned
`script` / `command` entry keyed by a built-in is refused, named once in a
status line and in the runtime log, and the built-in stays active (`disabled` still works). D9(a) planned
`Required`, rememberable per tool: `Suggest` and `Required` resolve the same
way in `resolve_tool_permission`, and per-tool remembering was not built.