mirror of
https://github.com/instructkr/claude-code.git
synced 2026-05-28 00:16:45 +00:00
Fail closed for compact without an interactive session
Route bare compact invocations to a typed interactive-only error before prompt/provider startup so non-TTY JSON probes terminate predictably. Keep the existing resume-session /compact path as the supported automation surface.\n\nConstraint: ROADMAP pinpoint #696 requires no spinner/hang for closed stdin and --output-format json.\nRejected: Broad help-schema rewrites | #699/#700/#701 are outside this PR scope.\nConfidence: high\nScope-risk: narrow\nDirective: Do not route bare slash-command names through the prompt fallback when they require a session.\nTested: cargo fmt --manifest-path rust/Cargo.toml --all -- --check; cargo test --manifest-path rust/Cargo.toml -p rusty-claude-cli --test compact_output compact_subcommand_ -- --nocapture; cargo build --manifest-path rust/Cargo.toml --workspace; cargo build --manifest-path rust/Cargo.toml --workspace --locked; timeout probes for compact JSON/text with stdin closed.\nNot-tested: Full workspace test suite.
This commit is contained in:
1
rust/Cargo.lock
generated
1
rust/Cargo.lock
generated
@@ -2124,6 +2124,7 @@ dependencies = [
|
||||
"serde_json",
|
||||
"sha2",
|
||||
"telemetry",
|
||||
"tempfile",
|
||||
"tokio",
|
||||
"walkdir",
|
||||
]
|
||||
|
||||
@@ -217,16 +217,22 @@ fn main() {
|
||||
.any(|w| w[0] == "--output-format" && w[1] == "json")
|
||||
|| argv.iter().any(|a| a == "--output-format=json");
|
||||
if json_output {
|
||||
// #77: classify error by prefix so downstream claws can route without
|
||||
// regex-scraping the prose. Split short-reason from hint-runbook.
|
||||
// #77/#696: classify error by prefix so downstream claws can route
|
||||
// without regex-scraping prose. Keep the legacy `type`/`kind`
|
||||
// fields and add the stable status/error_kind/action contract used
|
||||
// by non-interactive command guards.
|
||||
let kind = classify_error_kind(&message);
|
||||
let (short_reason, hint) = split_error_hint(&message);
|
||||
eprintln!(
|
||||
"{}",
|
||||
serde_json::json!({
|
||||
"type": "error",
|
||||
"error": short_reason,
|
||||
"kind": kind,
|
||||
"status": "error",
|
||||
"error_kind": kind,
|
||||
"error": short_reason,
|
||||
"message": short_reason,
|
||||
"action": "abort",
|
||||
"hint": hint,
|
||||
"exit_code": 1,
|
||||
})
|
||||
@@ -968,6 +974,16 @@ fn parse_args(args: &[String]) -> Result<CliAction, String> {
|
||||
if let Some(action) = parse_local_help_action(&rest, output_format) {
|
||||
return action;
|
||||
}
|
||||
// #696: `claw compact` is the bare name of the interactive `/compact`
|
||||
// slash command, not a prompt. When extra args such as `--help` appear
|
||||
// after the word `compact`, the generic prompt fallback used to send
|
||||
// `compact --help` to provider startup and could hang under closed stdin /
|
||||
// JSON output. Fail closed before any provider, prompt, TUI, or spinner
|
||||
// startup. `claw --resume SESSION.jsonl /compact` remains the supported
|
||||
// non-interactive session compaction path.
|
||||
if rest.first().map(String::as_str) == Some("compact") {
|
||||
return Err(compact_interactive_only_error());
|
||||
}
|
||||
if let Some(action) = parse_single_word_command_alias(
|
||||
&rest,
|
||||
&model,
|
||||
@@ -1303,6 +1319,11 @@ fn bare_slash_command_guidance(command_name: &str) -> Option<String> {
|
||||
Some(guidance)
|
||||
}
|
||||
|
||||
fn compact_interactive_only_error() -> String {
|
||||
"interactive_only: `claw compact` is an interactive/session command. Start `claw` and run `/compact`, or use `claw --resume SESSION.jsonl /compact` to compact an existing session."
|
||||
.to_string()
|
||||
}
|
||||
|
||||
fn removed_auth_surface_error(command_name: &str) -> String {
|
||||
format!(
|
||||
"`claw {command_name}` has been removed. Set ANTHROPIC_API_KEY or ANTHROPIC_AUTH_TOKEN instead."
|
||||
|
||||
@@ -2,9 +2,9 @@
|
||||
|
||||
use std::fs;
|
||||
use std::path::PathBuf;
|
||||
use std::process::{Command, Output};
|
||||
use std::process::{Command, Output, Stdio};
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
use std::time::{SystemTime, UNIX_EPOCH};
|
||||
use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
|
||||
|
||||
use mock_anthropic_service::{MockAnthropicService, SCENARIO_PREFIX};
|
||||
use serde_json::Value;
|
||||
@@ -245,6 +245,84 @@ stderr:
|
||||
fs::remove_dir_all(&workspace).expect("workspace cleanup should succeed");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn compact_subcommand_json_help_fails_fast_when_stdin_closed() {
|
||||
let workspace = unique_temp_dir("compact-nontty-json-help");
|
||||
let config_home = workspace.join("config-home");
|
||||
let home = workspace.join("home");
|
||||
fs::create_dir_all(&workspace).expect("workspace should exist");
|
||||
fs::create_dir_all(&config_home).expect("config home should exist");
|
||||
fs::create_dir_all(&home).expect("home should exist");
|
||||
|
||||
let output = run_claw_closed_stdin_with_timeout(
|
||||
&workspace,
|
||||
&config_home,
|
||||
&home,
|
||||
&["compact", "--output-format", "json", "--help"],
|
||||
Duration::from_secs(2),
|
||||
);
|
||||
|
||||
assert!(
|
||||
!output.status.success(),
|
||||
"compact json help should fail non-zero"
|
||||
);
|
||||
assert!(
|
||||
output.stdout.is_empty(),
|
||||
"compact json help should not start a prompt/spinner on stdout: {}",
|
||||
String::from_utf8_lossy(&output.stdout)
|
||||
);
|
||||
let stderr = String::from_utf8(output.stderr).expect("stderr should be utf8");
|
||||
let parsed: Value = serde_json::from_str(stderr.trim()).expect("stderr should be JSON error");
|
||||
assert_eq!(parsed["status"], "error");
|
||||
assert_eq!(parsed["error_kind"], "interactive_only");
|
||||
assert_eq!(parsed["action"], "abort");
|
||||
assert!(
|
||||
parsed["message"]
|
||||
.as_str()
|
||||
.unwrap_or_default()
|
||||
.contains("claw compact"),
|
||||
"message should name compact: {parsed}"
|
||||
);
|
||||
|
||||
fs::remove_dir_all(&workspace).expect("workspace cleanup should succeed");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn compact_subcommand_text_fails_fast_when_stdin_closed() {
|
||||
let workspace = unique_temp_dir("compact-nontty-text");
|
||||
let config_home = workspace.join("config-home");
|
||||
let home = workspace.join("home");
|
||||
fs::create_dir_all(&workspace).expect("workspace should exist");
|
||||
fs::create_dir_all(&config_home).expect("config home should exist");
|
||||
fs::create_dir_all(&home).expect("home should exist");
|
||||
|
||||
let output = run_claw_closed_stdin_with_timeout(
|
||||
&workspace,
|
||||
&config_home,
|
||||
&home,
|
||||
&["compact"],
|
||||
Duration::from_secs(2),
|
||||
);
|
||||
|
||||
assert!(
|
||||
!output.status.success(),
|
||||
"compact text should fail non-zero"
|
||||
);
|
||||
assert!(
|
||||
output.stdout.is_empty(),
|
||||
"compact text should not start a prompt/spinner on stdout: {}",
|
||||
String::from_utf8_lossy(&output.stdout)
|
||||
);
|
||||
let stderr = String::from_utf8(output.stderr).expect("stderr should be utf8");
|
||||
assert!(
|
||||
stderr.contains("[error-kind: interactive_only]"),
|
||||
"{stderr}"
|
||||
);
|
||||
assert!(stderr.contains("claw compact"), "{stderr}");
|
||||
|
||||
fs::remove_dir_all(&workspace).expect("workspace cleanup should succeed");
|
||||
}
|
||||
|
||||
fn run_claw(
|
||||
cwd: &std::path::Path,
|
||||
config_home: &std::path::Path,
|
||||
@@ -266,6 +344,48 @@ fn run_claw(
|
||||
command.output().expect("claw should launch")
|
||||
}
|
||||
|
||||
fn run_claw_closed_stdin_with_timeout(
|
||||
cwd: &std::path::Path,
|
||||
config_home: &std::path::Path,
|
||||
home: &std::path::Path,
|
||||
args: &[&str],
|
||||
timeout: Duration,
|
||||
) -> Output {
|
||||
let mut child = Command::new(env!("CARGO_BIN_EXE_claw"))
|
||||
.current_dir(cwd)
|
||||
.env_clear()
|
||||
.env("CLAW_CONFIG_HOME", config_home)
|
||||
.env("HOME", home)
|
||||
.env("NO_COLOR", "1")
|
||||
.env("PATH", "/usr/bin:/bin")
|
||||
.stdin(Stdio::null())
|
||||
.stdout(Stdio::piped())
|
||||
.stderr(Stdio::piped())
|
||||
.args(args)
|
||||
.spawn()
|
||||
.expect("claw should launch");
|
||||
|
||||
let start = Instant::now();
|
||||
loop {
|
||||
if child.try_wait().expect("try_wait should succeed").is_some() {
|
||||
return child.wait_with_output().expect("output should collect");
|
||||
}
|
||||
if start.elapsed() > timeout {
|
||||
let _ = child.kill();
|
||||
let output = child
|
||||
.wait_with_output()
|
||||
.expect("killed output should collect");
|
||||
panic!(
|
||||
"claw did not exit within {:?}\nstdout:\n{}\nstderr:\n{}",
|
||||
timeout,
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
}
|
||||
std::thread::sleep(Duration::from_millis(10));
|
||||
}
|
||||
}
|
||||
|
||||
fn unique_temp_dir(label: &str) -> PathBuf {
|
||||
let millis = SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
|
||||
Reference in New Issue
Block a user