Route nested CLI help requests to usage instead of operand fallthrough

The direct CLI wrappers for agents, skills, and mcp treated nested help flags as ordinary operands. That made commands like `claw mcp show --help` report a missing server and `claw skills install --help` fall into filesystem install logic instead of surfacing usage.

This change normalizes help-path arguments before dispatch so nested help stays on the help path. The regression tests cover both handler-level behavior and end-to-end CLI output for nested help and unknown subcommands with trailing help flags.

Constraint: Keep the fix scoped to direct CLI slash-command wrappers without changing unrelated parser behavior
Rejected: Rework top-level argument parsing for all subcommands | broader risk than needed for the regression
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: If more nested subcommands are added, extend the help-path normalization table before relying on raw operand dispatch
Tested: cargo build -p commands -p rusty-claude-cli
Tested: cargo test -p commands -p rusty-claude-cli
Not-tested: cargo clippy -p commands -p rusty-claude-cli --all-targets --no-deps -- -D warnings (pre-existing warnings in untouched files block clean run)
This commit is contained in:
Yeachan-Heo
2026-04-05 16:38:43 +00:00
parent b9c5cc118e
commit 1f53d961ff
2 changed files with 99 additions and 4 deletions

View File

@@ -2142,13 +2142,22 @@ pub fn handle_plugins_slash_command(
} }
pub fn handle_agents_slash_command(args: Option<&str>, cwd: &Path) -> std::io::Result<String> { pub fn handle_agents_slash_command(args: Option<&str>, cwd: &Path) -> std::io::Result<String> {
if let Some(args) = normalize_optional_args(args) {
if let Some(help_path) = help_path_from_args(args) {
return Ok(match help_path.as_slice() {
[] => render_agents_usage(None),
_ => render_agents_usage(Some(&help_path.join(" "))),
});
}
}
match normalize_optional_args(args) { match normalize_optional_args(args) {
None | Some("list") => { None | Some("list") => {
let roots = discover_definition_roots(cwd, "agents"); let roots = discover_definition_roots(cwd, "agents");
let agents = load_agents_from_roots(&roots)?; let agents = load_agents_from_roots(&roots)?;
Ok(render_agents_report(&agents)) Ok(render_agents_report(&agents))
} }
Some("-h" | "--help" | "help") => Ok(render_agents_usage(None)), Some(args) if is_help_arg(args) => Ok(render_agents_usage(None)),
Some(args) => Ok(render_agents_usage(Some(args))), Some(args) => Ok(render_agents_usage(Some(args))),
} }
} }
@@ -2162,6 +2171,16 @@ pub fn handle_mcp_slash_command(
} }
pub fn handle_skills_slash_command(args: Option<&str>, cwd: &Path) -> std::io::Result<String> { pub fn handle_skills_slash_command(args: Option<&str>, cwd: &Path) -> std::io::Result<String> {
if let Some(args) = normalize_optional_args(args) {
if let Some(help_path) = help_path_from_args(args) {
return Ok(match help_path.as_slice() {
[] => render_skills_usage(None),
["install", ..] => render_skills_usage(Some("install")),
_ => render_skills_usage(Some(&help_path.join(" "))),
});
}
}
match normalize_optional_args(args) { match normalize_optional_args(args) {
None | Some("list") => { None | Some("list") => {
let roots = discover_skill_roots(cwd); let roots = discover_skill_roots(cwd);
@@ -2177,7 +2196,7 @@ pub fn handle_skills_slash_command(args: Option<&str>, cwd: &Path) -> std::io::R
let install = install_skill(target, cwd)?; let install = install_skill(target, cwd)?;
Ok(render_skill_install_report(&install)) Ok(render_skill_install_report(&install))
} }
Some("-h" | "--help" | "help") => Ok(render_skills_usage(None)), Some(args) if is_help_arg(args) => Ok(render_skills_usage(None)),
Some(args) => Ok(render_skills_usage(Some(args))), Some(args) => Ok(render_skills_usage(Some(args))),
} }
} }
@@ -2187,6 +2206,16 @@ fn render_mcp_report_for(
cwd: &Path, cwd: &Path,
args: Option<&str>, args: Option<&str>,
) -> Result<String, runtime::ConfigError> { ) -> Result<String, runtime::ConfigError> {
if let Some(args) = normalize_optional_args(args) {
if let Some(help_path) = help_path_from_args(args) {
return Ok(match help_path.as_slice() {
[] => render_mcp_usage(None),
["show", ..] => render_mcp_usage(Some("show")),
_ => render_mcp_usage(Some(&help_path.join(" "))),
});
}
}
match normalize_optional_args(args) { match normalize_optional_args(args) {
None | Some("list") => { None | Some("list") => {
let runtime_config = loader.load()?; let runtime_config = loader.load()?;
@@ -2195,7 +2224,7 @@ fn render_mcp_report_for(
runtime_config.mcp().servers(), runtime_config.mcp().servers(),
)) ))
} }
Some("-h" | "--help" | "help") => Ok(render_mcp_usage(None)), Some(args) if is_help_arg(args) => Ok(render_mcp_usage(None)),
Some("show") => Ok(render_mcp_usage(Some("show"))), Some("show") => Ok(render_mcp_usage(Some("show"))),
Some(args) if args.split_whitespace().next() == Some("show") => { Some(args) if args.split_whitespace().next() == Some("show") => {
let mut parts = args.split_whitespace(); let mut parts = args.split_whitespace();
@@ -3036,6 +3065,16 @@ fn normalize_optional_args(args: Option<&str>) -> Option<&str> {
args.map(str::trim).filter(|value| !value.is_empty()) args.map(str::trim).filter(|value| !value.is_empty())
} }
fn is_help_arg(arg: &str) -> bool {
matches!(arg, "help" | "-h" | "--help")
}
fn help_path_from_args(args: &str) -> Option<Vec<&str>> {
let parts = args.split_whitespace().collect::<Vec<_>>();
let help_index = parts.iter().position(|part| is_help_arg(part))?;
Some(parts[..help_index].to_vec())
}
fn render_agents_usage(unexpected: Option<&str>) -> String { fn render_agents_usage(unexpected: Option<&str>) -> String {
let mut lines = vec![ let mut lines = vec![
"Agents".to_string(), "Agents".to_string(),
@@ -4005,7 +4044,17 @@ mod tests {
let skills_unexpected = let skills_unexpected =
super::handle_skills_slash_command(Some("show help"), &cwd).expect("skills usage"); super::handle_skills_slash_command(Some("show help"), &cwd).expect("skills usage");
assert!(skills_unexpected.contains("Unexpected show help")); assert!(skills_unexpected.contains("Unexpected show"));
let skills_install_help = super::handle_skills_slash_command(Some("install --help"), &cwd)
.expect("nested skills help");
assert!(skills_install_help.contains("Usage /skills [list|install <path>|help]"));
assert!(skills_install_help.contains("Unexpected install"));
let skills_unknown_help =
super::handle_skills_slash_command(Some("show --help"), &cwd).expect("skills help");
assert!(skills_unknown_help.contains("Usage /skills [list|install <path>|help]"));
assert!(skills_unknown_help.contains("Unexpected show"));
let _ = fs::remove_dir_all(cwd); let _ = fs::remove_dir_all(cwd);
} }
@@ -4022,6 +4071,16 @@ mod tests {
super::handle_mcp_slash_command(Some("show alpha beta"), &cwd).expect("mcp usage"); super::handle_mcp_slash_command(Some("show alpha beta"), &cwd).expect("mcp usage");
assert!(unexpected.contains("Unexpected show alpha beta")); assert!(unexpected.contains("Unexpected show alpha beta"));
let nested_help =
super::handle_mcp_slash_command(Some("show --help"), &cwd).expect("mcp help");
assert!(nested_help.contains("Usage /mcp [list|show <server>|help]"));
assert!(nested_help.contains("Unexpected show"));
let unknown_help =
super::handle_mcp_slash_command(Some("inspect --help"), &cwd).expect("mcp usage");
assert!(unknown_help.contains("Usage /mcp [list|show <server>|help]"));
assert!(unknown_help.contains("Unexpected inspect"));
let _ = fs::remove_dir_all(cwd); let _ = fs::remove_dir_all(cwd);
} }

View File

@@ -160,6 +160,42 @@ fn config_command_loads_defaults_from_standard_config_locations() {
fs::remove_dir_all(temp_dir).expect("cleanup temp dir"); fs::remove_dir_all(temp_dir).expect("cleanup temp dir");
} }
#[test]
fn nested_help_flags_render_usage_instead_of_falling_through() {
let temp_dir = unique_temp_dir("nested-help");
fs::create_dir_all(&temp_dir).expect("temp dir should exist");
let mcp_output = command_in(&temp_dir)
.args(["mcp", "show", "--help"])
.output()
.expect("claw should launch");
assert_success(&mcp_output);
let mcp_stdout = String::from_utf8(mcp_output.stdout).expect("stdout should be utf8");
assert!(mcp_stdout.contains("Usage /mcp [list|show <server>|help]"));
assert!(mcp_stdout.contains("Unexpected show"));
assert!(!mcp_stdout.contains("server `--help` is not configured"));
let skills_output = command_in(&temp_dir)
.args(["skills", "install", "--help"])
.output()
.expect("claw should launch");
assert_success(&skills_output);
let skills_stdout = String::from_utf8(skills_output.stdout).expect("stdout should be utf8");
assert!(skills_stdout.contains("Usage /skills [list|install <path>|help]"));
assert!(skills_stdout.contains("Unexpected install"));
let unknown_output = command_in(&temp_dir)
.args(["mcp", "inspect", "--help"])
.output()
.expect("claw should launch");
assert_success(&unknown_output);
let unknown_stdout = String::from_utf8(unknown_output.stdout).expect("stdout should be utf8");
assert!(unknown_stdout.contains("Usage /mcp [list|show <server>|help]"));
assert!(unknown_stdout.contains("Unexpected inspect"));
fs::remove_dir_all(temp_dir).expect("cleanup temp dir");
}
fn command_in(cwd: &Path) -> Command { fn command_in(cwd: &Path) -> Command {
let mut command = Command::new(env!("CARGO_BIN_EXE_claw")); let mut command = Command::new(env!("CARGO_BIN_EXE_claw"));
command.current_dir(cwd); command.current_dir(cwd);