From eb4b1ebc9b0cce8b177f692d247f9156b5be3397 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Thu, 23 Apr 2026 00:34:03 +0900 Subject: [PATCH] fix(#249): Add kind+hint to resumed-session slash error JSON envelopes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Was Broken (ROADMAP #249) Two Err arms in the resumed-session slash command dispatcher were emitting JSON envelopes WITHOUT the `kind` and `hint` fields that the typed-error contract requires: - main.rs:~2747 (parse_command_token failure path) - main.rs:~2783 (run_resume_command failure path) Adjacent code paths (e.g., the Ok(None) error branch at main.rs:2658) were already threading classify_error_kind + split_error_hint through. These two arms skipped the helpers, so claws routing on error class couldn't distinguish parse failures from other error classes for resumed-session slash command operations. This is an ARM-LEVEL LEAK pattern: the contract exists, the helpers exist, but two specific arms didn't call them. Cycle #38 identified this pattern ("Contract + helpers exist; the remaining work is finding every code path that didn't call the helpers"). ## What This Fix Does Thread classify_error_kind + split_error_hint through both arms: ```rust Err(error) => { if output_format == CliOutputFormat::Json { let full_message = error.to_string(); let kind = classify_error_kind(&full_message); let (short_reason, hint) = split_error_hint(&full_message); eprintln!( "{}", serde_json::json!({ "type": "error", "error": short_reason, "kind": kind, "hint": hint, "command": raw_command, }) ); } else { ... } std::process::exit(2); } ``` This matches the envelope shape used elsewhere in the same function (main.rs:2658) and satisfies the typed-error contract. ## Tests Added resumed_slash_error_envelope_includes_kind_and_hint_249 — regression guard in crates/rusty-claude-cli/src/main.rs that: - Documents the contract: both arms must carry kind and hint - Verifies classify_error_kind returns a non-empty class name - Verifies split_error_hint returns a non-empty short_reason - Provides an explicit comment documenting the structural contract (integration test would require real session file setup; this unit-level test verifies the building blocks work correctly) ## Test Results - All 181 tests pass (180 original + 1 new #249 regression test) - cargo build succeeds with no warnings - No regressions in existing tests ## Template This fix follows the exact pattern of the Ok(None) error branch at main.rs:2658 which was added for #77. The #247 classifier sweep (cycle #33-#36) and #248 sweep (cycle #41) also use this pattern. Each fix extends the typed-error contract to a previously-missed arm. Part of the classifier/dispatcher sweep cluster: - #247 (closed): prompt-related parse errors - #248 (review-ready): verb-qualified unknown-option errors - #249 (this): resumed slash-command Err arms - #130 (still open): filesystem errno strings (classifier part) - #251 (filed): dispatch-order on session-management verbs --- rust/crates/rusty-claude-cli/src/main.rs | 61 +++++++++++++++++++++++- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index 52915f0..2acb336 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -2745,11 +2745,20 @@ fn resume_session(session_path: &Path, commands: &[String], output_format: CliOu } Err(error) => { if output_format == CliOutputFormat::Json { + // #249: thread classify_error_kind + split_error_hint through this arm + // so the JSON envelope carries the same `kind` and `hint` fields as + // the Ok(None) path's error branch at main.rs:2658. Without these, claws + // routing on `kind` couldn't distinguish parse errors from other classes. + let full_message = error.to_string(); + let kind = classify_error_kind(&full_message); + let (short_reason, hint) = split_error_hint(&full_message); eprintln!( "{}", serde_json::json!({ "type": "error", - "error": error.to_string(), + "error": short_reason, + "kind": kind, + "hint": hint, "command": raw_command, }) ); @@ -2782,11 +2791,19 @@ fn resume_session(session_path: &Path, commands: &[String], output_format: CliOu } Err(error) => { if output_format == CliOutputFormat::Json { + // #249: mirror the Err arm above — emit the typed-error contract + // (kind + hint) for the run_resume_command failure path so claws + // routing on `kind` can distinguish parse/classification errors. + let full_message = error.to_string(); + let kind = classify_error_kind(&full_message); + let (short_reason, hint) = split_error_hint(&full_message); eprintln!( "{}", serde_json::json!({ "type": "error", - "error": error.to_string(), + "error": short_reason, + "kind": kind, + "hint": hint, "command": raw_command, }) ); @@ -10475,6 +10492,46 @@ mod tests { ); } + #[test] + fn resumed_slash_error_envelope_includes_kind_and_hint_249() { + // #249: the resumed-session slash command Err arms at main.rs:~2747 + // and main.rs:~2783 previously emitted JSON envelopes without `kind` + // or `hint` fields, breaking the typed-error contract for claws + // routing on error class. This test documents the contract: typical + // error messages produced through these paths (parse_command_token + // failures, run_resume_command failures) must classify correctly + // via the existing classify_error_kind() helper. + + // parse_command_token path (main.rs:~2747): unknown slash commands + // surface as cli_parse errors. + assert_eq!( + classify_error_kind("unknown slash command outside the REPL: /blargh"), + "unknown", + "unknown slash command without verb+option shape is currently unknown (may be tightened later via #248-family classifier work)" + ); + + // run_resume_command path (main.rs:~2783): common filesystem errors + // propagated from save_to_path(), write_session_clear_backup(), etc. + // These will classify via classify_error_kind; what matters is the + // envelope now carries the kind and hint fields instead of omitting them. + // Test the contract: whatever string is passed, it gets a kind and hint. + let full_message = "compact failed: I/O error during save"; + let kind = classify_error_kind(full_message); + let (short_reason, hint) = split_error_hint(full_message); + // Envelope building block must not panic and must produce usable values. + assert!(!kind.is_empty(), "classify_error_kind must return a non-empty class name"); + assert!(!short_reason.is_empty(), "split_error_hint must return a non-empty short reason"); + // hint may be None (single-line message has no hint), that's allowed. + let _ = hint; + + // Regression guard for the envelope SHAPE: the two arms must now include + // `kind` and `hint` fields (along with the pre-existing `type`, `error`, + // `command` fields). This is a structural contract — if anyone reverts + // the envelope to drop these fields, the code review must reject it. + // (Direct JSON inspection requires integration test infrastructure; this + // unit test verifies the building blocks work correctly.) + } + #[test] fn split_error_hint_separates_reason_from_runbook() { // #77: short reason / hint separation for JSON error payloads