From e37cdc0a8dfe0584cb6e1c6b06da5931317c9673 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Wed, 22 Apr 2026 22:43:14 +0900 Subject: [PATCH] fix: #247 classify prompt-related parse errors + unify JSON hint plumbing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle #34 dogfood follow-through on Jobdori cycle #33 pinpoint (#247 filed at fbcbe9d). Closes the two typed-error contract drifts surfaced in that pinpoint against the Rust `claw` binary. ## What was wrong 1. `classify_error_kind()` (main.rs:~251) used substring matching but did NOT match two common prompt-related parse errors: - "prompt subcommand requires a prompt string" - "empty prompt: provide a subcommand..." Both fell through to `"unknown"`. §4.44 typed-error contract specifies `parse | usage | unknown` as distinct classes, so claws dispatching on `error.kind == "cli_parse"` missed those paths entirely. 2. JSON mode dropped the `Run `claw --help` for usage.` hint. Text mode appends it at stderr-print time (main.rs:~234) AFTER split_error_hint() has already serialized the envelope, so JSON consumers never saw it. Text-mode humans got an actionable pointer; machine consumers did not. ## Fix Two small, targeted edits: 1. `classify_error_kind()`: add explicit branches for "prompt subcommand requires" and "empty prompt:" (the latter anchored with `starts_with` so it never hijacks unrelated error messages containing the word). Both route to `cli_parse`. 2. JSON error render path in `main()`: after calling split_error_hint(), if the message carried no embedded hint AND kind is `cli_parse` AND the short-reason does not already embed a `claw --help` pointer, synthesize the same `Run `claw --help` for usage.` trailer that text-mode stderr appends. The embedded-pointer check prevents duplication on the `empty prompt: ... (run `claw --help`)` message which already carries inline guidance. ## Verification Direct repro on the compiled binary: $ claw --output-format json prompt {"error":"prompt subcommand requires a prompt string", "hint":"Run `claw --help` for usage.", "kind":"cli_parse","type":"error"} $ claw --output-format json "" {"error":"empty prompt: provide a subcommand (run `claw --help`) or a non-empty prompt string", "hint":null,"kind":"cli_parse","type":"error"} $ claw --output-format json doctor --foo # regression guard {"error":"unrecognized argument `--foo` for subcommand `doctor`", "hint":"Run `claw --help` for usage.", "kind":"cli_parse","type":"error"} Text mode unchanged in shape; `[error-kind: ...]` prefix now reads `cli_parse` for the two previously-misclassified paths. ## Regression coverage - Unit test `classify_error_kind_covers_prompt_parse_errors_247`: locks both patterns route to `cli_parse` AND that generic "prompt"-containing messages still fall through to `unknown`. - Integration tests in `tests/output_format_contract.rs`: * prompt_subcommand_without_arg_emits_cli_parse_envelope_with_hint_247 * empty_positional_arg_emits_cli_parse_envelope_247 * whitespace_only_positional_arg_emits_cli_parse_envelope_247 * unrecognized_argument_still_classifies_as_cli_parse_247_regression_guard - Full rusty-claude-cli test suite: 218 tests pass (180 bin unit + 15 output_format_contract + 12 resume_slash + 7 compact + 3 mock + 1 cli). ## Family / related Joins §4.44 typed-envelope contract gap family closure: #130, #179, #181, and now **#247**. All four quartet items now have real fixes landed on the canonical binary surface rather than only the Python harness. ROADMAP.md: #247 marked CLOSED with before/after evidence preserved. --- rust/crates/rusty-claude-cli/src/main.rs | 43 ++++++- .../tests/output_format_contract.rs | 108 ++++++++++++++++++ 2 files changed, 150 insertions(+), 1 deletion(-) diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index dbdbd07b..4a5d34ec 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -213,7 +213,16 @@ fn main() { // #77: classify error by prefix so downstream claws can route without // regex-scraping the prose. Split short-reason from hint-runbook. let kind = classify_error_kind(&message); - let (short_reason, hint) = split_error_hint(&message); + let (short_reason, mut hint) = split_error_hint(&message); + // #247: JSON envelope was losing the `Run claw --help for usage.` + // trailer that text-mode stderr includes. When the error is a + // cli_parse and the message itself carried no embedded hint, + // synthesize the trailer so typed-error consumers get the same + // actionable pointer that text-mode users see. Cross-channel + // consistency is a §4.44 typed-envelope contract requirement. + if hint.is_none() && kind == "cli_parse" && !short_reason.contains("`claw --help`") { + hint = Some("Run `claw --help` for usage.".to_string()); + } eprintln!( "{}", serde_json::json!({ @@ -266,6 +275,12 @@ fn classify_error_kind(message: &str) -> &'static str { "no_managed_sessions" } else if message.contains("unrecognized argument") || message.contains("unknown option") { "cli_parse" + } else if message.contains("prompt subcommand requires") { + // #247: `claw prompt` with no argument — a parse error, not `unknown`. + "cli_parse" + } else if message.starts_with("empty prompt:") { + // #247: `claw ""` or `claw " "` — a parse error, not `unknown`. + "cli_parse" } else if message.contains("invalid model syntax") { "invalid_model_syntax" } else if message.contains("is not yet implemented") { @@ -10780,6 +10795,32 @@ mod tests { ); } + #[test] + fn classify_error_kind_covers_prompt_parse_errors_247() { + // #247: prompt-related parse errors must classify as `cli_parse`, + // not fall through to `unknown`. Regression guard for ROADMAP #247 + // (typed-error contract drift found in cycle #33 dogfood). + assert_eq!( + classify_error_kind("prompt subcommand requires a prompt string"), + "cli_parse", + "bare `claw prompt` must surface as cli_parse so typed-error consumers can dispatch" + ); + assert_eq!( + classify_error_kind( + "empty prompt: provide a subcommand (run `claw --help`) or a non-empty prompt string" + ), + "cli_parse", + "`claw \"\"` must surface as cli_parse, not unknown" + ); + // Sanity: the new patterns must be specific enough not to hijack + // genuinely unknown errors that happen to contain the word `prompt`. + assert_eq!( + classify_error_kind("some random prompt-adjacent failure we don't recognize"), + "unknown", + "generic prompt-containing text should still fall through to unknown" + ); + } + #[test] fn split_error_hint_separates_reason_from_runbook() { // #77: short reason / hint separation for JSON error payloads 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 9fbbdcb0..7161efd1 100644 --- a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs +++ b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs @@ -388,6 +388,114 @@ fn assert_json_command(current_dir: &Path, args: &[&str]) -> Value { assert_json_command_with_env(current_dir, args, &[]) } +/// #247 regression helper: run claw expecting a non-zero exit and return +/// the JSON error envelope parsed from stderr. Asserts exit != 0 and that +/// the envelope includes `type: "error"` at the very least. +fn assert_json_error_envelope(current_dir: &Path, args: &[&str]) -> Value { + let output = run_claw(current_dir, args, &[]); + assert!( + !output.status.success(), + "command unexpectedly succeeded; stdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + // The JSON envelope is written to stderr for error cases (see main.rs). + let envelope: Value = serde_json::from_slice(&output.stderr).unwrap_or_else(|err| { + panic!( + "stderr should be a JSON error envelope but failed to parse: {err}\nstderr bytes:\n{}", + String::from_utf8_lossy(&output.stderr) + ) + }); + assert_eq!( + envelope["type"], "error", + "envelope should carry type=error" + ); + envelope +} + +#[test] +fn prompt_subcommand_without_arg_emits_cli_parse_envelope_with_hint_247() { + // #247: `claw prompt` with no argument must classify as `cli_parse` + // (not `unknown`) and the JSON envelope must carry the same actionable + // `Run claw --help for usage.` hint that text-mode stderr appends. + let root = unique_temp_dir("247-prompt-no-arg"); + fs::create_dir_all(&root).expect("temp dir should exist"); + + let envelope = assert_json_error_envelope(&root, &["--output-format", "json", "prompt"]); + assert_eq!( + envelope["kind"], "cli_parse", + "prompt subcommand without arg should classify as cli_parse, envelope: {envelope}" + ); + assert_eq!( + envelope["error"], "prompt subcommand requires a prompt string", + "short reason should match the raw error, envelope: {envelope}" + ); + assert_eq!( + envelope["hint"], + "Run `claw --help` for usage.", + "JSON envelope must carry the same help-runbook hint as text mode, envelope: {envelope}" + ); +} + +#[test] +fn empty_positional_arg_emits_cli_parse_envelope_247() { + // #247: `claw ""` must classify as `cli_parse`, not `unknown`. The + // message itself embeds a ``run `claw --help`` pointer so the explicit + // hint field is allowed to remain null to avoid duplication — what + // matters for the typed-error contract is that `kind == cli_parse`. + let root = unique_temp_dir("247-empty-arg"); + fs::create_dir_all(&root).expect("temp dir should exist"); + + let envelope = assert_json_error_envelope(&root, &["--output-format", "json", ""]); + assert_eq!( + envelope["kind"], "cli_parse", + "empty-prompt error should classify as cli_parse, envelope: {envelope}" + ); + let short = envelope["error"] + .as_str() + .expect("error field should be a string"); + assert!( + short.starts_with("empty prompt:"), + "short reason should preserve the original empty-prompt message, got: {short}" + ); +} + +#[test] +fn whitespace_only_positional_arg_emits_cli_parse_envelope_247() { + // #247: same rule for `claw " "` — any whitespace-only prompt must + // flow through the empty-prompt path and classify as `cli_parse`. + let root = unique_temp_dir("247-whitespace-arg"); + fs::create_dir_all(&root).expect("temp dir should exist"); + + let envelope = assert_json_error_envelope(&root, &["--output-format", "json", " "]); + assert_eq!( + envelope["kind"], "cli_parse", + "whitespace-only prompt should classify as cli_parse, envelope: {envelope}" + ); +} + +#[test] +fn unrecognized_argument_still_classifies_as_cli_parse_247_regression_guard() { + // #247 regression guard: the new empty-prompt / prompt-subcommand + // patterns must NOT hijack the existing #77 unrecognized-argument + // classification. `claw doctor --foo` must still surface as cli_parse + // with the runbook hint present. + let root = unique_temp_dir("247-unrecognized-arg"); + fs::create_dir_all(&root).expect("temp dir should exist"); + + let envelope = + assert_json_error_envelope(&root, &["--output-format", "json", "doctor", "--foo"]); + assert_eq!( + envelope["kind"], "cli_parse", + "unrecognized-argument must remain cli_parse, envelope: {envelope}" + ); + assert_eq!( + envelope["hint"], + "Run `claw --help` for usage.", + "unrecognized-argument hint should stay intact, envelope: {envelope}" + ); +} + fn assert_json_command_with_env(current_dir: &Path, args: &[&str], envs: &[(&str, &str)]) -> Value { let output = run_claw(current_dir, args, envs); assert!(