mirror of
https://github.com/instructkr/claude-code.git
synced 2026-06-05 03:56:45 +00:00
fix: keep failed resume side-effect free
Generated with https://github.com/Yeachan-Heo/gajae-code Co-authored-by: Gajae Code <dev@gajae-code.com>
This commit is contained in:
@@ -28,7 +28,8 @@ pub struct SessionStore {
|
||||
impl SessionStore {
|
||||
/// Build a store from the server's current working directory.
|
||||
///
|
||||
/// The on-disk layout becomes `<cwd>/.claw/sessions/<workspace_hash>/`.
|
||||
/// The on-disk layout is `<cwd>/.claw/sessions/<workspace_hash>/`,
|
||||
/// created lazily on first successful session save.
|
||||
pub fn from_cwd(cwd: impl AsRef<Path>) -> Result<Self, SessionControlError> {
|
||||
let cwd = cwd.as_ref();
|
||||
// #151: canonicalize so equivalent paths (symlinks, relative vs
|
||||
@@ -40,7 +41,6 @@ impl SessionStore {
|
||||
.join(".claw")
|
||||
.join("sessions")
|
||||
.join(workspace_fingerprint(&canonical_cwd));
|
||||
fs::create_dir_all(&sessions_root)?;
|
||||
Ok(Self {
|
||||
sessions_root,
|
||||
workspace_root: canonical_cwd,
|
||||
@@ -49,7 +49,8 @@ impl SessionStore {
|
||||
|
||||
/// Build a store from an explicit `--data-dir` flag.
|
||||
///
|
||||
/// The on-disk layout becomes `<data_dir>/sessions/<workspace_hash>/`
|
||||
/// The on-disk layout is `<data_dir>/sessions/<workspace_hash>/`,
|
||||
/// created lazily on first successful session save.
|
||||
/// where `<workspace_hash>` is derived from `workspace_root`.
|
||||
pub fn from_data_dir(
|
||||
data_dir: impl AsRef<Path>,
|
||||
@@ -64,7 +65,6 @@ impl SessionStore {
|
||||
.as_ref()
|
||||
.join("sessions")
|
||||
.join(workspace_fingerprint(&canonical_workspace));
|
||||
fs::create_dir_all(&sessions_root)?;
|
||||
Ok(Self {
|
||||
sessions_root,
|
||||
workspace_root: canonical_workspace,
|
||||
@@ -760,14 +760,21 @@ mod tests {
|
||||
use crate::session::Session;
|
||||
use std::fs;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
use std::time::{SystemTime, UNIX_EPOCH};
|
||||
|
||||
static TEMP_COUNTER: AtomicU64 = AtomicU64::new(0);
|
||||
|
||||
fn temp_dir() -> PathBuf {
|
||||
let nanos = SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
.expect("time should be after epoch")
|
||||
.as_nanos();
|
||||
std::env::temp_dir().join(format!("runtime-session-control-{nanos}"))
|
||||
let counter = TEMP_COUNTER.fetch_add(1, Ordering::Relaxed);
|
||||
std::env::temp_dir().join(format!(
|
||||
"runtime-session-control-{}-{nanos}-{counter}",
|
||||
std::process::id()
|
||||
))
|
||||
}
|
||||
|
||||
fn persist_session(root: &Path, text: &str) -> Session {
|
||||
@@ -981,6 +988,38 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn session_store_from_cwd_is_side_effect_free_until_save() {
|
||||
// given
|
||||
let base = temp_dir();
|
||||
let workspace = base.join("fresh-workspace");
|
||||
fs::create_dir_all(&workspace).expect("workspace should exist");
|
||||
|
||||
// when
|
||||
let store = SessionStore::from_cwd(&workspace).expect("store should build");
|
||||
|
||||
// then — resolving the store must not create .claw/session partitions.
|
||||
assert!(
|
||||
!workspace.join(".claw").exists(),
|
||||
"session store construction must not create .claw side effects"
|
||||
);
|
||||
assert!(
|
||||
!store.sessions_dir().exists(),
|
||||
"session partition should be created lazily on save"
|
||||
);
|
||||
|
||||
let session = persist_session_via_store(&store, "first saved turn");
|
||||
assert!(
|
||||
store
|
||||
.sessions_dir()
|
||||
.join(format!("{}.jsonl", session.session_id))
|
||||
.exists(),
|
||||
"saving a managed session should create the lazy session partition"
|
||||
);
|
||||
|
||||
fs::remove_dir_all(base).expect("temp dir should clean up");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn session_store_from_cwd_isolates_sessions_by_workspace() {
|
||||
// given
|
||||
|
||||
@@ -382,9 +382,16 @@ fn main() {
|
||||
object.insert("available".to_string(), serde_json::json!(available));
|
||||
object.insert("tool_aliases".to_string(), aliases);
|
||||
}
|
||||
} else if kind == "missing_argument" && message.contains("--allowedTools") {
|
||||
} else if kind == "missing_argument" {
|
||||
if let Some(object) = error_json.as_object_mut() {
|
||||
object.insert("argument".to_string(), serde_json::json!("--allowedTools"));
|
||||
if message.contains("--allowedTools") {
|
||||
object.insert("argument".to_string(), serde_json::json!("--allowedTools"));
|
||||
} else if message.contains("prompt or subcommand") {
|
||||
object.insert(
|
||||
"argument".to_string(),
|
||||
serde_json::json!("prompt or subcommand"),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
// #819/#820/#823: JSON mode error envelopes must go to stdout so machine
|
||||
@@ -1751,11 +1758,15 @@ fn parse_args(args: &[String]) -> Result<CliAction, String> {
|
||||
|
||||
if rest.is_empty() {
|
||||
let permission_mode = permission_mode_override.unwrap_or_else(default_permission_mode);
|
||||
let stdin_is_terminal = std::io::stdin().is_terminal();
|
||||
if compact && stdin_is_terminal {
|
||||
return Err(compact_missing_argument_error());
|
||||
}
|
||||
// When stdin is not a terminal (pipe/redirect) and no prompt is given on the
|
||||
// command line, read stdin as the prompt and dispatch as a one-shot Prompt
|
||||
// rather than starting the interactive REPL (which would consume the pipe and
|
||||
// print the startup banner, then exit without sending anything to the API).
|
||||
if !std::io::stdin().is_terminal() {
|
||||
if !stdin_is_terminal {
|
||||
let mut buf = String::new();
|
||||
let _ = std::io::Read::read_to_string(&mut std::io::stdin(), &mut buf);
|
||||
let piped = buf.trim().to_string();
|
||||
@@ -1766,12 +1777,15 @@ fn parse_args(args: &[String]) -> Result<CliAction, String> {
|
||||
allowed_tools,
|
||||
permission_mode,
|
||||
output_format,
|
||||
compact: false,
|
||||
compact,
|
||||
base_commit,
|
||||
reasoning_effort,
|
||||
allow_broad_cwd,
|
||||
});
|
||||
}
|
||||
if compact {
|
||||
return Err(compact_missing_argument_error());
|
||||
}
|
||||
// Non-TTY stdin with no piped content: refuse to start the interactive
|
||||
// REPL (it would block forever waiting for input that will never arrive).
|
||||
// (#696: emit a typed error instead of hanging indefinitely)
|
||||
@@ -2087,7 +2101,8 @@ Usage: claw prompt <text> or echo '<text>' | claw prompt".to_string());
|
||||
allow_broad_cwd,
|
||||
),
|
||||
other => {
|
||||
if !other.starts_with('-')
|
||||
if !compact
|
||||
&& !other.starts_with('-')
|
||||
&& looks_like_subcommand_typo(other)
|
||||
&& (rest.len() == 1
|
||||
|| (output_format == CliOutputFormat::Json && model_flag_raw.is_none()))
|
||||
@@ -2850,6 +2865,11 @@ fn allowed_tools_missing_error() -> String {
|
||||
"missing_argument: --allowedTools requires a tool list before subcommands or flags.\nUsage: --allowedTools <tool-name>[,<tool-name>...] e.g. --allowedTools read,glob".to_string()
|
||||
}
|
||||
|
||||
fn compact_missing_argument_error() -> String {
|
||||
"missing_argument: --compact requires prompt text, piped stdin, or a subcommand. argument: prompt or subcommand\nUsage: claw --compact <prompt> or echo '<prompt>' | claw --compact"
|
||||
.to_string()
|
||||
}
|
||||
|
||||
fn allowed_tool_aliases_json(registry: &GlobalToolRegistry) -> Value {
|
||||
Value::Object(
|
||||
registry
|
||||
@@ -13558,6 +13578,21 @@ mod tests {
|
||||
allow_broad_cwd: false,
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
parse_args(&["--compact".to_string(), "hello".to_string()])
|
||||
.expect("compact single-word prompt should parse"),
|
||||
CliAction::Prompt {
|
||||
prompt: "hello".to_string(),
|
||||
model: DEFAULT_MODEL.to_string(),
|
||||
output_format: CliOutputFormat::Text,
|
||||
allowed_tools: None,
|
||||
permission_mode: PermissionMode::WorkspaceWrite,
|
||||
compact: true,
|
||||
base_commit: None,
|
||||
reasoning_effort: None,
|
||||
allow_broad_cwd: false,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -5424,6 +5424,54 @@ fn multi_word_unknown_subcommand_json_emits_command_not_found_826() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn compact_flag_missing_argument_and_shorthand_prompt_contract_435() {
|
||||
let root = unique_temp_dir("compact-flag-435");
|
||||
let config_home = root.join("config-home");
|
||||
let home = root.join("home");
|
||||
std::fs::create_dir_all(&root).expect("create temp dir");
|
||||
std::fs::create_dir_all(&config_home).expect("create config home");
|
||||
std::fs::create_dir_all(&home).expect("create home");
|
||||
let envs = [
|
||||
(
|
||||
"CLAW_CONFIG_HOME",
|
||||
config_home.to_str().expect("config home utf8"),
|
||||
),
|
||||
("HOME", home.to_str().expect("home utf8")),
|
||||
("ANTHROPIC_API_KEY", ""),
|
||||
("ANTHROPIC_AUTH_TOKEN", ""),
|
||||
("OPENAI_API_KEY", ""),
|
||||
];
|
||||
|
||||
let missing = run_claw(&root, &["--output-format", "json", "--compact"], &envs);
|
||||
assert_eq!(missing.status.code(), Some(1));
|
||||
assert!(
|
||||
missing.stderr.is_empty(),
|
||||
"compact missing-argument JSON should keep stderr empty: {}",
|
||||
String::from_utf8_lossy(&missing.stderr)
|
||||
);
|
||||
let missing_json = parse_json_stdout(&missing, "compact missing argument");
|
||||
assert_eq!(missing_json["error_kind"], "missing_argument");
|
||||
assert_eq!(missing_json["argument"], "prompt or subcommand");
|
||||
|
||||
let prompt = run_claw(
|
||||
&root,
|
||||
&["--output-format", "json", "--compact", "hello"],
|
||||
&envs,
|
||||
);
|
||||
assert_eq!(prompt.status.code(), Some(1));
|
||||
assert!(
|
||||
prompt.stderr.is_empty(),
|
||||
"compact prompt JSON should keep stderr empty: {}",
|
||||
String::from_utf8_lossy(&prompt.stderr)
|
||||
);
|
||||
let prompt_json = parse_json_stdout(&prompt, "compact shorthand prompt");
|
||||
assert_eq!(
|
||||
prompt_json["error_kind"], "missing_credentials",
|
||||
"--compact hello should stay on the prompt/provider path, not command_not_found: {prompt_json}"
|
||||
);
|
||||
}
|
||||
|
||||
// #827: direct /unknown-slash-command must emit typed error_kind, not "unknown"
|
||||
// Uses the direct-slash CLI path (no session load needed; reproducible on CI).
|
||||
#[test]
|
||||
|
||||
@@ -222,6 +222,73 @@ fn resume_latest_restores_the_most_recent_managed_session() {
|
||||
assert!(stdout.contains(newer_path.to_str().expect("utf8 path")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resume_latest_missing_session_fails_without_creating_session_dirs_435() {
|
||||
// given
|
||||
let temp_dir = unique_temp_dir("resume-latest-missing-435");
|
||||
let project_dir = temp_dir.join("project");
|
||||
let config_home = temp_dir.join("config-home");
|
||||
let home = temp_dir.join("home");
|
||||
fs::create_dir_all(&project_dir).expect("project dir should exist");
|
||||
fs::create_dir_all(&config_home).expect("config home should exist");
|
||||
fs::create_dir_all(&home).expect("home should exist");
|
||||
let envs = [
|
||||
(
|
||||
"CLAW_CONFIG_HOME",
|
||||
config_home.to_str().expect("utf8 config home"),
|
||||
),
|
||||
("HOME", home.to_str().expect("utf8 home")),
|
||||
("ANTHROPIC_API_KEY", ""),
|
||||
("ANTHROPIC_AUTH_TOKEN", ""),
|
||||
("OPENAI_API_KEY", ""),
|
||||
];
|
||||
|
||||
// when — both text and JSON resume failures should be non-zero and read-only.
|
||||
let text = run_claw_with_env(&project_dir, &["--resume", "latest"], &envs);
|
||||
let json = run_claw_with_env(
|
||||
&project_dir,
|
||||
&["--output-format", "json", "--resume", "latest"],
|
||||
&envs,
|
||||
);
|
||||
|
||||
// then
|
||||
assert_eq!(
|
||||
text.status.code(),
|
||||
Some(1),
|
||||
"text resume failure must be non-zero"
|
||||
);
|
||||
assert!(
|
||||
text.stdout.is_empty(),
|
||||
"text resume failure should not claim success on stdout: {}",
|
||||
String::from_utf8_lossy(&text.stdout)
|
||||
);
|
||||
let text_stderr = String::from_utf8_lossy(&text.stderr);
|
||||
assert!(
|
||||
text_stderr.contains("no managed sessions found"),
|
||||
"text failure should explain missing sessions: {text_stderr}"
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
json.status.code(),
|
||||
Some(1),
|
||||
"JSON resume failure must be non-zero"
|
||||
);
|
||||
assert!(
|
||||
json.stderr.is_empty(),
|
||||
"JSON resume failure should keep stderr empty: {}",
|
||||
String::from_utf8_lossy(&json.stderr)
|
||||
);
|
||||
let parsed: Value = serde_json::from_slice(&json.stdout)
|
||||
.expect("JSON resume failure should emit JSON to stdout");
|
||||
assert_eq!(parsed["status"], "error");
|
||||
assert_eq!(parsed["action"], "restore");
|
||||
assert_eq!(parsed["error_kind"], "no_managed_sessions");
|
||||
assert!(
|
||||
!project_dir.join(".claw").exists(),
|
||||
"failed resume must not create .claw/session directories"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resumed_status_command_emits_structured_json_when_requested() {
|
||||
// given
|
||||
|
||||
Reference in New Issue
Block a user