fix+test(#756): missing/invalid flag-value errors now emit typed error_kind and non-null hint

This commit is contained in:
YeonGyu-Kim
2026-05-26 21:37:28 +09:00
parent 0e8a449ea9
commit 4df146188f
3 changed files with 77 additions and 7 deletions

View File

@@ -286,6 +286,10 @@ fn classify_error_kind(message: &str) -> &'static str {
"unsupported_skills_action"
} else if message.contains("unrecognized argument") || message.contains("unknown option") {
"cli_parse"
} else if message.starts_with("missing_flag_value:") {
"missing_flag_value"
} else if message.starts_with("invalid_flag_value:") {
"invalid_flag_value"
} else if message.contains("invalid model syntax") {
"invalid_model_syntax"
} else if message.contains("is not yet implemented") {
@@ -776,7 +780,7 @@ fn parse_args(args: &[String]) -> Result<CliAction, String> {
"--model" => {
let value = args
.get(index + 1)
.ok_or_else(|| "missing value for --model".to_string())?;
.ok_or_else(|| "missing_flag_value: missing value for --model.\nUsage: --model <provider/model> e.g. --model anthropic/claude-opus-4-7".to_string())?;
let resolved = resolve_model_alias_with_config(value);
debug!("Resolved --model '{}' -> '{}'", value, resolved);
validate_model_syntax(&resolved)?;
@@ -796,14 +800,14 @@ fn parse_args(args: &[String]) -> Result<CliAction, String> {
"--output-format" => {
let value = args
.get(index + 1)
.ok_or_else(|| "missing value for --output-format".to_string())?;
.ok_or_else(|| "missing_flag_value: missing value for --output-format.\nUsage: --output-format text or --output-format json".to_string())?;
output_format = CliOutputFormat::parse(value)?;
index += 2;
}
"--permission-mode" => {
let value = args
.get(index + 1)
.ok_or_else(|| "missing value for --permission-mode".to_string())?;
.ok_or_else(|| "missing_flag_value: missing value for --permission-mode.\nUsage: --permission-mode default|acceptEdits|bypassPermissions|dangerFullAccess".to_string())?;
permission_mode_override = Some(parse_permission_mode_arg(value)?);
index += 2;
}
@@ -826,7 +830,7 @@ fn parse_args(args: &[String]) -> Result<CliAction, String> {
"--base-commit" => {
let value = args
.get(index + 1)
.ok_or_else(|| "missing value for --base-commit".to_string())?;
.ok_or_else(|| "missing_flag_value: missing value for --base-commit.\nUsage: --base-commit <git-sha>".to_string())?;
base_commit = Some(value.clone());
index += 2;
}
@@ -837,10 +841,10 @@ fn parse_args(args: &[String]) -> Result<CliAction, String> {
"--reasoning-effort" => {
let value = args
.get(index + 1)
.ok_or_else(|| "missing value for --reasoning-effort".to_string())?;
.ok_or_else(|| "missing_flag_value: missing value for --reasoning-effort.\nUsage: --reasoning-effort low|medium|high".to_string())?;
if !matches!(value.as_str(), "low" | "medium" | "high") {
return Err(format!(
"invalid value for --reasoning-effort: '{value}'; must be low, medium, or high"
"invalid_flag_value: invalid value for --reasoning-effort: '{value}'.\nUsage: --reasoning-effort low|medium|high"
));
}
reasoning_effort = Some(value.clone());
@@ -850,7 +854,7 @@ fn parse_args(args: &[String]) -> Result<CliAction, String> {
let value = &flag[19..];
if !matches!(value, "low" | "medium" | "high") {
return Err(format!(
"invalid value for --reasoning-effort: '{value}'; must be low, medium, or high"
"invalid_flag_value: invalid value for --reasoning-effort: '{value}'.\nUsage: --reasoning-effort low|medium|high"
));
}
reasoning_effort = Some(value.to_string());

View File

@@ -1504,6 +1504,70 @@ fn prompt_no_arg_json_error_kind_750() {
);
}
#[test]
fn flag_value_errors_have_error_kind_and_hint_756() {
// #756: missing/invalid flag-value errors must emit typed error_kind + non-null hint.
// Before #756: all returned error_kind:"unknown" + hint:null.
use std::process::Command;
let root = unique_temp_dir("flag-value-errors");
fs::create_dir_all(&root).expect("temp dir");
let bin = env!("CARGO_BIN_EXE_claw");
// Case 1: --reasoning-effort with invalid value
let out = Command::new(bin)
.current_dir(&root)
.args(["--output-format", "json", "--reasoning-effort", "HIGH"])
.output()
.expect("claw --reasoning-effort HIGH should run");
assert!(
!out.status.success(),
"invalid reasoning-effort must exit non-zero"
);
let raw = String::from_utf8_lossy(&out.stderr)
.lines()
.filter(|l| l.starts_with('{'))
.collect::<Vec<_>>()
.join("");
let parsed: serde_json::Value = serde_json::from_str(&raw)
.unwrap_or_else(|_| panic!("invalid --reasoning-effort must emit JSON; got: {raw}"));
assert_eq!(
parsed["error_kind"], "invalid_flag_value",
"invalid --reasoning-effort must be invalid_flag_value (#756): {parsed}"
);
assert!(
parsed["hint"].as_str().map_or(false, |h| h.contains("low")
|| h.contains("medium")
|| h.contains("high")),
"hint must mention valid values (#756): {parsed}"
);
// Case 2: --model flag with missing value (trailing flag)
let out2 = Command::new(bin)
.current_dir(&root)
.args(["--output-format", "json", "--model"])
.output()
.expect("claw --model (no value) should run");
assert!(
!out2.status.success(),
"missing --model value must exit non-zero"
);
let raw2 = String::from_utf8_lossy(&out2.stderr)
.lines()
.filter(|l| l.starts_with('{'))
.collect::<Vec<_>>()
.join("");
let parsed2: serde_json::Value = serde_json::from_str(&raw2)
.unwrap_or_else(|_| panic!("missing --model value must emit JSON; got: {raw2}"));
assert_eq!(
parsed2["error_kind"], "missing_flag_value",
"missing --model value must be missing_flag_value (#756): {parsed2}"
);
assert!(
parsed2["hint"].as_str().map_or(false, |h| !h.is_empty()),
"missing --model hint must be non-empty (#756): {parsed2}"
);
}
#[test]
fn short_p_flag_swallows_no_flags_755() {
// #755: `claw -p hello --output-format json` must parse --output-format json