From 4df146188fb9dde5f5c2a2c527f4b1edce041d3e Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Tue, 26 May 2026 21:37:28 +0900 Subject: [PATCH] fix+test(#756): missing/invalid flag-value errors now emit typed error_kind and non-null hint --- ROADMAP.md | 2 + rust/crates/rusty-claude-cli/src/main.rs | 18 ++++-- .../tests/output_format_contract.rs | 64 +++++++++++++++++++ 3 files changed, 77 insertions(+), 7 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index bc746614..c1499b29 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -7677,3 +7677,5 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed) 754. **`missing_credentials` JSON envelope always had `hint: null` even when a contextual hint was available** — dogfooded 2026-05-26 on `e9327135`. `ApiError::Display` for `MissingCredentials` appended the hint via ` — hint: {hint}` (inline, no `\n`), so `split_error_hint()` could not extract it and left the JSON `hint` field null. Fix: change delimiter from ` — hint: ` to `\n` in `api/src/error.rs` Display impl; update two tests in `api/src/error.rs` and `api/src/providers/mod.rs` to assert newline separator. Source: Jobdori dogfood on `e9327135`, 2026-05-26. 755. **`claw -p hello --model sonnet` swallowed `--model sonnet` into the prompt string** — gaebal-gajae pinpoint on `e9327135` (#117 revival). `-p` used `args[index+1..].join(" ")`, consuming all remaining tokens as prompt. Fix: capture exactly one token via `args.get(index+1)`, reject flag-like tokens (`starts_with('-')`) as `missing_prompt`, support `--` sentinel for literal flag-text, then `continue` the flag loop so `--model`/`--output-format`/etc. parse normally. Dispatch via `short_p_prompt` after full flag scan. Regression guard: `short_p_flag_swallows_no_flags_755` asserts `--output-format json` is parsed (not swallowed) and `--model` as prompt-arg is rejected. Source: gaebal-gajae dogfood on `e9327135`, 2026-05-26. + +756. **`--reasoning-effort bogus`, `--model` (no value), and sibling missing/invalid flag-value errors all returned `error_kind:"unknown"` + `hint:null`** — gaebal-gajae pinpoint on `0e8a449e`. All `missing value for --X` and `invalid value for --reasoning-effort` error strings were single-line with no classifier arm. Fix: (a) prefix all with `missing_flag_value:` / `invalid_flag_value:` + `\n` usage hint; (b) add `message.starts_with("missing_flag_value:")` → `"missing_flag_value"` and `message.starts_with("invalid_flag_value:")` → `"invalid_flag_value"` classifier arms. Covers `--model`, `--output-format`, `--permission-mode`, `--base-commit`, `--reasoning-effort`. Regression guard: `flag_value_errors_have_error_kind_and_hint_756` — invalid `--reasoning-effort HIGH` → `invalid_flag_value` + hint with valid values; missing `--model` → `missing_flag_value` + non-null hint. Source: gaebal-gajae dogfood on `0e8a449e`, 2026-05-26. diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index 120d4498..830d07cf 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -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 { "--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 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 { "--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 { "--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 ".to_string())?; base_commit = Some(value.clone()); index += 2; } @@ -837,10 +841,10 @@ fn parse_args(args: &[String]) -> Result { "--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 { 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()); diff --git a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs index 87b11d8a..90f740be 100644 --- a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs +++ b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs @@ -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::>() + .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::>() + .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