feat(tools): hide legacy subagent and shell aliases from model catalog (#2683)
Subagent aliases: - Legacy names (agent_spawn, agent_result, agent_cancel, resume_agent, agent_list, agent_send_input, agent_assign, agent_wait, delegate_to_agent) are already NOT registered — they exist as dead code with #[allow(dead_code)] since v0.8.33 - Add test verifying model catalog only advertises canonical subagent tools: agent_open, agent_eval, agent_close, tool_agent Shell aliases: - Hide exec_wait from model catalog (legacy alias for exec_shell_wait) - Hide exec_interact from model catalog (legacy alias for exec_shell_interact) - Both remain callable for saved transcript replay - Add test verifying shell aliases are hidden but callable Verification: cargo test -p codewhale-tui --locked (4040 passed), cargo clippy -D warnings
This commit is contained in:
@@ -1670,4 +1670,42 @@ mod tests {
|
||||
"task_shell_wait should be included when allow_shell is true"
|
||||
);
|
||||
}
|
||||
|
||||
/// #2683 — `exec_wait` and `exec_interact` are legacy aliases for
|
||||
/// `exec_shell_wait` and `exec_shell_interact`. They must remain
|
||||
/// callable (for saved transcript replay) but hidden from the
|
||||
/// model-facing catalog.
|
||||
#[test]
|
||||
fn shell_alias_tools_hidden_from_model_catalog() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let ctx = ToolContext::new(tmp.path().to_path_buf());
|
||||
let registry = ToolRegistryBuilder::new().with_shell_tools().build(ctx);
|
||||
|
||||
// Legacy aliases stay callable.
|
||||
for alias in ["exec_wait", "exec_interact"] {
|
||||
assert!(registry.contains(alias), "{alias} should remain callable");
|
||||
}
|
||||
|
||||
let api_names: Vec<String> = registry
|
||||
.to_api_tools()
|
||||
.into_iter()
|
||||
.map(|tool| tool.name)
|
||||
.collect();
|
||||
|
||||
// Canonical names are model-visible.
|
||||
for canonical in ["exec_shell_wait", "exec_shell_interact"] {
|
||||
assert!(
|
||||
api_names.iter().any(|n| n == canonical),
|
||||
"{canonical} should be model-visible"
|
||||
);
|
||||
}
|
||||
|
||||
// Legacy aliases are hidden.
|
||||
for alias in ["exec_wait", "exec_interact"] {
|
||||
assert!(
|
||||
api_names.iter().all(|n| n != alias),
|
||||
"{alias} should be hidden from the model catalog"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2755,6 +2755,11 @@ impl ToolSpec for ShellWaitTool {
|
||||
self.name
|
||||
}
|
||||
|
||||
fn model_visible(&self) -> bool {
|
||||
// `exec_wait` is a legacy alias; only `exec_shell_wait` is model-visible.
|
||||
self.name == "exec_shell_wait"
|
||||
}
|
||||
|
||||
fn description(&self) -> &'static str {
|
||||
"Wait for a background shell task and return incremental output. Turn cancellation stops waiting but leaves the background task running."
|
||||
}
|
||||
@@ -2836,6 +2841,11 @@ impl ToolSpec for ShellInteractTool {
|
||||
self.name
|
||||
}
|
||||
|
||||
fn model_visible(&self) -> bool {
|
||||
// `exec_interact` is a legacy alias; only `exec_shell_interact` is model-visible.
|
||||
self.name == "exec_shell_interact"
|
||||
}
|
||||
|
||||
fn description(&self) -> &'static str {
|
||||
"Send input to a background shell task and return incremental output."
|
||||
}
|
||||
|
||||
@@ -2658,3 +2658,50 @@ fn subagent_completion_payload_carries_existing_sentinel_format() {
|
||||
"sentinel should not duplicate the human summary line"
|
||||
);
|
||||
}
|
||||
|
||||
/// #2683 — Verify the model-facing tool catalog only advertises canonical
|
||||
/// subagent tools and never exposes legacy superseded names.
|
||||
#[test]
|
||||
fn model_catalog_only_advertises_canonical_subagent_tools() {
|
||||
use crate::tools::ToolRegistryBuilder;
|
||||
|
||||
let tmp = tempfile::tempdir().expect("tempdir");
|
||||
let runtime = stub_runtime();
|
||||
let manager = runtime.manager.clone();
|
||||
let ctx = crate::tools::spec::ToolContext::new(tmp.path().to_path_buf());
|
||||
let registry = ToolRegistryBuilder::new()
|
||||
.with_subagent_tools(manager, runtime)
|
||||
.build(ctx);
|
||||
|
||||
let api_names: Vec<String> = registry
|
||||
.to_api_tools()
|
||||
.into_iter()
|
||||
.map(|t| t.name)
|
||||
.collect();
|
||||
|
||||
// Canonical tools must be model-visible.
|
||||
for canonical in ["agent_open", "agent_eval", "agent_close", "tool_agent"] {
|
||||
assert!(
|
||||
api_names.iter().any(|n| n == canonical),
|
||||
"{canonical} should be in the model-facing catalog"
|
||||
);
|
||||
}
|
||||
|
||||
// Legacy/superseded names must NOT appear in the model catalog.
|
||||
for legacy in [
|
||||
"agent_spawn",
|
||||
"agent_result",
|
||||
"agent_cancel",
|
||||
"resume_agent",
|
||||
"agent_list",
|
||||
"agent_send_input",
|
||||
"agent_assign",
|
||||
"agent_wait",
|
||||
"delegate_to_agent",
|
||||
] {
|
||||
assert!(
|
||||
api_names.iter().all(|n| n != legacy),
|
||||
"{legacy} should be hidden from the model-facing catalog"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user