fix(#3129): handle trailing json format for diff errors (#3161)

Keep malformed diff invocations with trailing JSON format flags on the parser error path and lock the contract with focused output-format regressions.

Constraint: Do not touch tracked .omx state files.

Rejected: Repeating direct binary smoke loops | local auth/provider configuration intercepts those invocations and obscures parser behavior.

Confidence: high

Scope-risk: narrow

Tested: git diff --check; cargo fmt --check; cargo test -p rusty-claude-cli diff_extra_args_have_typed_error_kind_and_hint_766 --test output_format_contract; cargo test -p rusty-claude-cli diff_trailing_json_after_malformed_args_is_bounded_json_3129 --test output_format_contract; cargo test -p rusty-claude-cli diff_non_git_dir_has_error_kind_and_hint_801 --test output_format_contract
This commit is contained in:
Bellman
2026-05-27 21:57:26 +09:00
committed by GitHub
parent efd34c151a
commit ae6a207d4e
2 changed files with 78 additions and 12 deletions

View File

@@ -1225,10 +1225,10 @@ fn parse_args(args: &[String]) -> Result<CliAction, String> {
// `git diff`). No session needed to inspect the working tree.
"diff" => {
if rest.len() > 1 {
return Err(format!(
"unexpected extra arguments after `claw diff`: {}\nUsage: claw diff",
rest[1..].join(" ")
));
// #3129: keep malformed `diff ... --output-format json` on the
// parser/error path, not the prompt/TUI fallback. The newline
// before Usage is part of the JSON hint contract.
return Err(unexpected_diff_args_error(&rest[1..]));
}
Ok(CliAction::Diff { output_format })
}
@@ -1625,6 +1625,13 @@ fn removed_auth_surface_error(command_name: &str) -> String {
)
}
fn unexpected_diff_args_error(extra: &[String]) -> String {
format!(
"unexpected extra arguments after `claw diff`: {}\nUsage: claw diff",
extra.join(" ")
)
}
fn parse_acp_args(args: &[String], output_format: CliOutputFormat) -> Result<CliAction, String> {
match args {
[] => Ok(CliAction::Acp { output_format }),

View File

@@ -1916,36 +1916,95 @@ fn login_logout_removed_subcommands_have_error_kind_and_hint_765() {
fn diff_extra_args_have_typed_error_kind_and_hint_766() {
// #766: `claw diff --bogus` returned error_kind:"unknown" + hint:null.
// `diff` takes no arguments; extra args were unclassified with no remediation.
let root = unique_temp_dir("diff-extra-args-766");
let root = git_temp_dir("diff-extra-args-766");
assert_diff_unexpected_extra_args_json(
&root,
&["--output-format", "json", "diff", "--bogus"],
"claw diff --bogus",
);
}
#[test]
fn diff_trailing_json_after_malformed_args_is_bounded_json_3129() {
// #3129: when --output-format json appeared after malformed `diff` args,
// the parser fell through to the interactive/prompt path and emitted zero
// JSON stdout. These forms must fail before any provider or TUI path starts.
let root = git_temp_dir("diff-trailing-json-3129");
for (args, label) in [
(
&["diff", "--bogus-flag", "--output-format", "json"][..],
"claw diff --bogus-flag --output-format json",
),
(
&["diff", "does-not-exist", "--output-format", "json"][..],
"claw diff does-not-exist --output-format json",
),
(
&[
"diff",
"--cached",
"--bogus-flag",
"--output-format",
"json",
][..],
"claw diff --cached --bogus-flag --output-format json",
),
] {
assert_diff_unexpected_extra_args_json(&root, args, label);
}
}
fn git_temp_dir(prefix: &str) -> PathBuf {
let root = unique_temp_dir(prefix);
fs::create_dir_all(&root).expect("temp dir should exist");
// Need a git repo for diff to parse past arg validation
// Need a git repo so `diff` reaches argument validation before git checks.
std::process::Command::new("git")
.args(["init", "-q"])
.current_dir(&root)
.output()
.ok();
.expect("git init should launch");
root
}
let output = run_claw(&root, &["--output-format", "json", "diff", "--bogus"], &[]);
fn assert_diff_unexpected_extra_args_json(root: &Path, args: &[&str], label: &str) {
let output = run_claw(root, args, &[]);
assert!(
!output.status.success(),
"claw diff --bogus should exit non-zero"
"{label} should exit non-zero; stdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
assert!(
output.stdout.is_empty(),
"{label} should not enter the spinner/prompt path; stdout:\n{}",
String::from_utf8_lossy(&output.stdout)
);
let stderr = String::from_utf8_lossy(&output.stderr);
let json_line = stderr
.lines()
.find(|l| l.trim_start().starts_with('{'))
.expect("stderr should contain a JSON error envelope");
.unwrap_or_else(|| {
panic!("{label} stderr should contain a JSON error envelope; stderr:\n{stderr}")
});
let parsed: serde_json::Value =
serde_json::from_str(json_line).expect("error envelope should be valid JSON");
assert_eq!(
parsed["error_kind"], "unexpected_extra_args",
"claw diff --bogus must return error_kind:unexpected_extra_args (#766)"
"{label} must return error_kind:unexpected_extra_args"
);
let hint = parsed["hint"].as_str().unwrap_or("");
assert!(
!hint.is_empty(),
"claw diff --bogus must return non-null hint (#766), got: {hint:?}"
"{label} must return non-null hint, got: {hint:?}"
);
assert!(
parsed["message"]
.as_str()
.is_some_and(|message| !message.is_empty()),
"{label} must return non-empty message"
);
}