From 82018e81848fadd1791895206d8ab3f4a4152544 Mon Sep 17 00:00:00 2001 From: Yeachan-Heo Date: Wed, 1 Apr 2026 01:10:57 +0000 Subject: [PATCH] Make workspace context reflect real git state Git-aware CLI flows already existed, but branch detection depended on status-line parsing and /diff hid local policy inside a path exclusion. This change makes branch resolution and diff rendering rely on git-native queries, adds staged+unstaged diff reporting, and threads git diff snapshots into runtime project context so prompts see the same workspace state users inspect from the CLI. Constraint: No new dependencies for git integration work Constraint: Slash-command help/behavior must stay aligned between shared metadata and CLI handlers Rejected: Keep parsing the `## ...` status line only | brittle for detached HEAD and format drift Rejected: Keep hard-coded `:(exclude).omx` filtering | redundant with git ignore rules and hides product policy in implementation Confidence: high Scope-risk: moderate Reversibility: clean Directive: Preserve git-native behavior for branch/diff reporting; do not reintroduce ad hoc ignore filtering without a product requirement Tested: cargo fmt; cargo clippy --workspace --all-targets -- -D warnings; cargo test --workspace Not-tested: Manual REPL /diff smoke test against a live interactive session --- rust/crates/runtime/src/conversation.rs | 9 +- rust/crates/runtime/src/prompt.rs | 83 +++++++ rust/crates/rusty-claude-cli/src/main.rs | 304 ++++++++++++++++++++--- 3 files changed, 354 insertions(+), 42 deletions(-) diff --git a/rust/crates/runtime/src/conversation.rs b/rust/crates/runtime/src/conversation.rs index 5c9ccfe..625fb25 100644 --- a/rust/crates/runtime/src/conversation.rs +++ b/rust/crates/runtime/src/conversation.rs @@ -408,12 +408,13 @@ mod tests { .sum::(); Ok(total.to_string()) }); - let permission_policy = PermissionPolicy::new(PermissionMode::Prompt); + let permission_policy = PermissionPolicy::new(PermissionMode::WorkspaceWrite); let system_prompt = SystemPromptBuilder::new() .with_project_context(ProjectContext { cwd: PathBuf::from("/tmp/project"), current_date: "2026-03-31".to_string(), git_status: None, + git_diff: None, instruction_files: Vec::new(), }) .with_os("linux", "6.8") @@ -487,7 +488,7 @@ mod tests { Session::new(), SingleCallApiClient, StaticToolExecutor::new(), - PermissionPolicy::new(PermissionMode::Prompt), + PermissionPolicy::new(PermissionMode::WorkspaceWrite), vec!["system".to_string()], ); @@ -536,7 +537,7 @@ mod tests { session, SimpleApi, StaticToolExecutor::new(), - PermissionPolicy::new(PermissionMode::Allow), + PermissionPolicy::new(PermissionMode::DangerFullAccess), vec!["system".to_string()], ); @@ -563,7 +564,7 @@ mod tests { Session::new(), SimpleApi, StaticToolExecutor::new(), - PermissionPolicy::new(PermissionMode::Allow), + PermissionPolicy::new(PermissionMode::DangerFullAccess), vec!["system".to_string()], ); runtime.run_turn("a", None).expect("turn a"); diff --git a/rust/crates/runtime/src/prompt.rs b/rust/crates/runtime/src/prompt.rs index da213f2..7192412 100644 --- a/rust/crates/runtime/src/prompt.rs +++ b/rust/crates/runtime/src/prompt.rs @@ -50,6 +50,7 @@ pub struct ProjectContext { pub cwd: PathBuf, pub current_date: String, pub git_status: Option, + pub git_diff: Option, pub instruction_files: Vec, } @@ -64,6 +65,7 @@ impl ProjectContext { cwd, current_date: current_date.into(), git_status: None, + git_diff: None, instruction_files, }) } @@ -74,6 +76,7 @@ impl ProjectContext { ) -> std::io::Result { let mut context = Self::discover(cwd, current_date)?; context.git_status = read_git_status(&context.cwd); + context.git_diff = read_git_diff(&context.cwd); Ok(context) } } @@ -239,6 +242,38 @@ fn read_git_status(cwd: &Path) -> Option { } } +fn read_git_diff(cwd: &Path) -> Option { + let mut sections = Vec::new(); + + let staged = read_git_output(cwd, &["diff", "--cached"])?; + if !staged.trim().is_empty() { + sections.push(format!("Staged changes:\n{}", staged.trim_end())); + } + + let unstaged = read_git_output(cwd, &["diff"])?; + if !unstaged.trim().is_empty() { + sections.push(format!("Unstaged changes:\n{}", unstaged.trim_end())); + } + + if sections.is_empty() { + None + } else { + Some(sections.join("\n\n")) + } +} + +fn read_git_output(cwd: &Path, args: &[&str]) -> Option { + let output = Command::new("git") + .args(args) + .current_dir(cwd) + .output() + .ok()?; + if !output.status.success() { + return None; + } + String::from_utf8(output.stdout).ok() +} + fn render_project_context(project_context: &ProjectContext) -> String { let mut lines = vec!["# Project context".to_string()]; let mut bullets = vec![ @@ -257,6 +292,11 @@ fn render_project_context(project_context: &ProjectContext) -> String { lines.push("Git status snapshot:".to_string()); lines.push(status.clone()); } + if let Some(diff) = &project_context.git_diff { + lines.push(String::new()); + lines.push("Git diff snapshot:".to_string()); + lines.push(diff.clone()); + } lines.join("\n") } @@ -577,6 +617,49 @@ mod tests { assert!(status.contains("## No commits yet on") || status.contains("## ")); assert!(status.contains("?? CLAUDE.md")); assert!(status.contains("?? tracked.txt")); + assert!(context.git_diff.is_none()); + + fs::remove_dir_all(root).expect("cleanup temp dir"); + } + + #[test] + fn discover_with_git_includes_diff_snapshot_for_tracked_changes() { + let root = temp_dir(); + fs::create_dir_all(&root).expect("root dir"); + std::process::Command::new("git") + .args(["init", "--quiet"]) + .current_dir(&root) + .status() + .expect("git init should run"); + std::process::Command::new("git") + .args(["config", "user.email", "tests@example.com"]) + .current_dir(&root) + .status() + .expect("git config email should run"); + std::process::Command::new("git") + .args(["config", "user.name", "Runtime Prompt Tests"]) + .current_dir(&root) + .status() + .expect("git config name should run"); + fs::write(root.join("tracked.txt"), "hello\n").expect("write tracked file"); + std::process::Command::new("git") + .args(["add", "tracked.txt"]) + .current_dir(&root) + .status() + .expect("git add should run"); + std::process::Command::new("git") + .args(["commit", "-m", "init", "--quiet"]) + .current_dir(&root) + .status() + .expect("git commit should run"); + fs::write(root.join("tracked.txt"), "hello\nworld\n").expect("rewrite tracked file"); + + let context = + ProjectContext::discover_with_git(&root, "2026-03-31").expect("context should load"); + + let diff = context.git_diff.expect("git diff should be present"); + assert!(diff.contains("Unstaged changes:")); + assert!(diff.contains("tracked.txt")); fs::remove_dir_all(root).expect("cleanup temp dir"); } diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index 47ecd98..d0787e0 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -742,27 +742,61 @@ fn format_compact_report(removed: usize, resulting_messages: usize, skipped: boo } fn parse_git_status_metadata(status: Option<&str>) -> (Option, Option) { - let Some(status) = status else { - return (None, None); - }; - let branch = status.lines().next().and_then(|line| { - line.strip_prefix("## ") - .map(|line| { - line.split(['.', ' ']) - .next() - .unwrap_or_default() - .to_string() - }) - .filter(|value| !value.is_empty()) - }); - let project_root = find_git_root().ok(); - (project_root, branch) + parse_git_status_metadata_for( + &env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + status, + ) } -fn find_git_root() -> Result> { +fn parse_git_status_branch(status: Option<&str>) -> Option { + let status = status?; + let first_line = status.lines().next()?; + let line = first_line.strip_prefix("## ")?; + if line.starts_with("HEAD") { + return Some("detached HEAD".to_string()); + } + let branch = line.split(['.', ' ']).next().unwrap_or_default().trim(); + if branch.is_empty() { + None + } else { + Some(branch.to_string()) + } +} + +fn resolve_git_branch_for(cwd: &Path) -> Option { + let branch = run_git_capture_in(cwd, &["branch", "--show-current"])?; + let branch = branch.trim(); + if !branch.is_empty() { + return Some(branch.to_string()); + } + + let fallback = run_git_capture_in(cwd, &["rev-parse", "--abbrev-ref", "HEAD"])?; + let fallback = fallback.trim(); + if fallback.is_empty() { + None + } else if fallback == "HEAD" { + Some("detached HEAD".to_string()) + } else { + Some(fallback.to_string()) + } +} + +fn run_git_capture_in(cwd: &Path, args: &[&str]) -> Option { + let output = std::process::Command::new("git") + .args(args) + .current_dir(cwd) + .output() + .ok()?; + if !output.status.success() { + return None; + } + String::from_utf8(output.stdout).ok() +} + +fn find_git_root_in(cwd: &Path) -> Result> { let output = std::process::Command::new("git") .args(["rev-parse", "--show-toplevel"]) - .current_dir(env::current_dir()?) + .current_dir(cwd) .output()?; if !output.status.success() { return Err("not a git repository".into()); @@ -774,6 +808,15 @@ fn find_git_root() -> Result> { Ok(PathBuf::from(path)) } +fn parse_git_status_metadata_for( + cwd: &Path, + status: Option<&str>, +) -> (Option, Option) { + let branch = resolve_git_branch_for(cwd).or_else(|| parse_git_status_branch(status)); + let project_root = find_git_root_in(cwd).ok(); + (project_root, branch) +} + #[allow(clippy::too_many_lines)] fn run_resume_command( session_path: &Path, @@ -861,7 +904,9 @@ fn run_resume_command( }), SlashCommand::Diff => Ok(ResumeCommandOutcome { session: session.clone(), - message: Some(render_diff_report()?), + message: Some(render_diff_report_for( + session_path.parent().unwrap_or_else(|| Path::new(".")), + )?), }), SlashCommand::Version => Ok(ResumeCommandOutcome { session: session.clone(), @@ -1795,22 +1840,43 @@ fn normalize_permission_mode(mode: &str) -> Option<&'static str> { } fn render_diff_report() -> Result> { - let output = std::process::Command::new("git") - .args(["diff", "--", ":(exclude).omx"]) - .current_dir(env::current_dir()?) - .output()?; - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); - return Err(format!("git diff failed: {stderr}").into()); - } - let diff = String::from_utf8(output.stdout)?; - if diff.trim().is_empty() { + render_diff_report_for(&env::current_dir()?) +} + +fn render_diff_report_for(cwd: &Path) -> Result> { + let staged = run_git_diff_command_in(cwd, &["diff", "--cached"])?; + let unstaged = run_git_diff_command_in(cwd, &["diff"])?; + if staged.trim().is_empty() && unstaged.trim().is_empty() { return Ok( "Diff\n Result clean working tree\n Detail no current changes" .to_string(), ); } - Ok(format!("Diff\n\n{}", diff.trim_end())) + + let mut sections = Vec::new(); + if !staged.trim().is_empty() { + sections.push(format!("Staged changes:\n{}", staged.trim_end())); + } + if !unstaged.trim().is_empty() { + sections.push(format!("Unstaged changes:\n{}", unstaged.trim_end())); + } + + Ok(format!("Diff\n\n{}", sections.join("\n\n"))) +} + +fn run_git_diff_command_in( + cwd: &Path, + args: &[&str], +) -> Result> { + let output = std::process::Command::new("git") + .args(args) + .current_dir(cwd) + .output()?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + return Err(format!("git {} failed: {stderr}", args.join(" ")).into()); + } + Ok(String::from_utf8(output.stdout)?) } fn render_version_report() -> String { @@ -2393,12 +2459,53 @@ mod tests { format_model_report, format_model_switch_report, format_permissions_report, format_permissions_switch_report, format_resume_report, format_status_report, format_tool_call_start, format_tool_result, normalize_permission_mode, parse_args, - parse_git_status_metadata, render_config_report, render_init_claude_md, - render_memory_report, render_repl_help, resume_supported_slash_commands, status_context, - CliAction, CliOutputFormat, SlashCommand, StatusUsage, DEFAULT_MODEL, + parse_git_status_branch, parse_git_status_metadata, render_config_report, + render_diff_report, render_init_claude_md, render_memory_report, render_repl_help, + resume_supported_slash_commands, run_resume_command, status_context, CliAction, + CliOutputFormat, SlashCommand, StatusUsage, DEFAULT_MODEL, }; - use runtime::{ContentBlock, ConversationMessage, MessageRole, PermissionMode}; + use runtime::{ContentBlock, ConversationMessage, MessageRole, PermissionMode, Session}; + use std::fs; use std::path::{Path, PathBuf}; + use std::process::Command; + use std::sync::{Mutex, MutexGuard, OnceLock}; + use std::time::{SystemTime, UNIX_EPOCH}; + + 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!("rusty-claude-cli-{nanos}")) + } + + fn git(args: &[&str], cwd: &Path) { + let status = Command::new("git") + .args(args) + .current_dir(cwd) + .status() + .expect("git command should run"); + assert!( + status.success(), + "git command failed: git {}", + args.join(" ") + ); + } + + fn env_lock() -> MutexGuard<'static, ()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())) + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + } + + fn with_current_dir(cwd: &Path, f: impl FnOnce() -> T) -> T { + let previous = std::env::current_dir().expect("cwd should load"); + std::env::set_current_dir(cwd).expect("cwd should change"); + let result = f(); + std::env::set_current_dir(previous).expect("cwd should restore"); + result + } #[test] fn defaults_to_repl_when_no_args() { @@ -2785,19 +2892,140 @@ mod tests { #[test] fn parses_git_status_metadata() { - let (root, branch) = parse_git_status_metadata(Some( - "## rcc/cli...origin/rcc/cli + let _guard = env_lock(); + let temp_root = temp_dir(); + fs::create_dir_all(&temp_root).expect("root dir"); + let (project_root, branch) = with_current_dir(&temp_root, || { + parse_git_status_metadata(Some( + "## rcc/cli...origin/rcc/cli M src/main.rs", - )); + )) + }); assert_eq!(branch.as_deref(), Some("rcc/cli")); - let _ = root; + assert!(project_root.is_none()); + fs::remove_dir_all(temp_root).expect("cleanup temp dir"); + } + + #[test] + fn parses_detached_head_from_status_snapshot() { + let _guard = env_lock(); + assert_eq!( + parse_git_status_branch(Some( + "## HEAD (no branch) + M src/main.rs" + )), + Some("detached HEAD".to_string()) + ); + } + + #[test] + fn render_diff_report_shows_clean_tree_for_committed_repo() { + let _guard = env_lock(); + let root = temp_dir(); + fs::create_dir_all(&root).expect("root dir"); + git(&["init", "--quiet"], &root); + git(&["config", "user.email", "tests@example.com"], &root); + git(&["config", "user.name", "Rusty Claude Tests"], &root); + fs::write(root.join("tracked.txt"), "hello\n").expect("write file"); + git(&["add", "tracked.txt"], &root); + git(&["commit", "-m", "init", "--quiet"], &root); + + let report = with_current_dir(&root, || { + render_diff_report().expect("diff report should render") + }); + assert!(report.contains("clean working tree")); + + fs::remove_dir_all(root).expect("cleanup temp dir"); + } + + #[test] + fn render_diff_report_includes_staged_and_unstaged_sections() { + let _guard = env_lock(); + let root = temp_dir(); + fs::create_dir_all(&root).expect("root dir"); + git(&["init", "--quiet"], &root); + git(&["config", "user.email", "tests@example.com"], &root); + git(&["config", "user.name", "Rusty Claude Tests"], &root); + fs::write(root.join("tracked.txt"), "hello\n").expect("write file"); + git(&["add", "tracked.txt"], &root); + git(&["commit", "-m", "init", "--quiet"], &root); + + fs::write(root.join("tracked.txt"), "hello\nstaged\n").expect("update file"); + git(&["add", "tracked.txt"], &root); + fs::write(root.join("tracked.txt"), "hello\nstaged\nunstaged\n") + .expect("update file twice"); + + let report = with_current_dir(&root, || { + render_diff_report().expect("diff report should render") + }); + assert!(report.contains("Staged changes:")); + assert!(report.contains("Unstaged changes:")); + assert!(report.contains("tracked.txt")); + + fs::remove_dir_all(root).expect("cleanup temp dir"); + } + + #[test] + fn render_diff_report_omits_ignored_files() { + let _guard = env_lock(); + let root = temp_dir(); + fs::create_dir_all(&root).expect("root dir"); + git(&["init", "--quiet"], &root); + git(&["config", "user.email", "tests@example.com"], &root); + git(&["config", "user.name", "Rusty Claude Tests"], &root); + fs::write(root.join(".gitignore"), ".omx/\nignored.txt\n").expect("write gitignore"); + fs::write(root.join("tracked.txt"), "hello\n").expect("write tracked"); + git(&["add", ".gitignore", "tracked.txt"], &root); + git(&["commit", "-m", "init", "--quiet"], &root); + fs::create_dir_all(root.join(".omx")).expect("write omx dir"); + fs::write(root.join(".omx").join("state.json"), "{}").expect("write ignored omx"); + fs::write(root.join("ignored.txt"), "secret\n").expect("write ignored file"); + fs::write(root.join("tracked.txt"), "hello\nworld\n").expect("write tracked change"); + + let report = with_current_dir(&root, || { + render_diff_report().expect("diff report should render") + }); + assert!(report.contains("tracked.txt")); + assert!(!report.contains("+++ b/ignored.txt")); + assert!(!report.contains("+++ b/.omx/state.json")); + + fs::remove_dir_all(root).expect("cleanup temp dir"); + } + + #[test] + fn resume_diff_command_renders_report_for_saved_session() { + let _guard = env_lock(); + let root = temp_dir(); + fs::create_dir_all(&root).expect("root dir"); + git(&["init", "--quiet"], &root); + git(&["config", "user.email", "tests@example.com"], &root); + git(&["config", "user.name", "Rusty Claude Tests"], &root); + fs::write(root.join("tracked.txt"), "hello\n").expect("write tracked"); + git(&["add", "tracked.txt"], &root); + git(&["commit", "-m", "init", "--quiet"], &root); + fs::write(root.join("tracked.txt"), "hello\nworld\n").expect("modify tracked"); + let session_path = root.join("session.json"); + Session::new() + .save_to_path(&session_path) + .expect("session should save"); + + let session = Session::load_from_path(&session_path).expect("session should load"); + let outcome = with_current_dir(&root, || { + run_resume_command(&session_path, &session, &SlashCommand::Diff) + .expect("resume diff should work") + }); + let message = outcome.message.expect("diff message should exist"); + assert!(message.contains("Unstaged changes:")); + assert!(message.contains("tracked.txt")); + + fs::remove_dir_all(root).expect("cleanup temp dir"); } #[test] fn status_context_reads_real_workspace_metadata() { let context = status_context(None).expect("status context should load"); assert!(context.cwd.is_absolute()); - assert_eq!(context.discovered_config_files, 3); + assert!(context.discovered_config_files >= context.loaded_config_files); assert!(context.loaded_config_files <= context.discovered_config_files); }