From 0afd84e8eb5b02c3a0dba7468e5a22ab0b981cc0 Mon Sep 17 00:00:00 2001 From: Goransh walia Date: Fri, 4 Sep 2026 13:05:26 +0530 Subject: [PATCH 1/4] =?UTF-8?q?fix:=20Tools:=20atomic=20commit=20splitting?= =?UTF-8?q?=20=E2=80=94=20order=20unrelated=20changes=20by=20=20(#3999)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/tui/src/tools/git.rs | 804 +++++++++++++++++++++++++++++++++++- 1 file changed, 799 insertions(+), 5 deletions(-) diff --git a/crates/tui/src/tools/git.rs b/crates/tui/src/tools/git.rs index e3bdb44ab9..8be4d6c625 100644 --- a/crates/tui/src/tools/git.rs +++ b/crates/tui/src/tools/git.rs @@ -3,6 +3,7 @@ //! These tools are read-only wrappers around common git inspection commands, //! scoped to the workspace and optionally to a sub-path within it. +use std::collections::{HashMap, HashSet}; use std::fs; use std::path::{Path, PathBuf}; @@ -209,6 +210,355 @@ impl ToolSpec for GitDiffTool { } } +// === GitCommitSplitTool === + +/// Tool for splitting unstaged and staged changes into atomic commits. +pub struct GitCommitSplitTool; + +#[async_trait] +impl ToolSpec for GitCommitSplitTool { + fn name(&self) -> &'static str { + "commit_split" + } + + fn model_visible(&self) -> bool { + true + } + + fn description(&self) -> &'static str { + "Analyze the working tree, group changes into logical commits, order them by dependency (rejecting cycles), and apply the split commits." + } + + fn input_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "dry_run": { + "type": "boolean", + "description": "If true, only return the proposed split commits and dependency graph without writing." + }, + "path": { + "type": "string", + "description": "Optional subdirectory or file to scope the split to (must be within the workspace)." + } + }, + "additionalProperties": false + }) + } + + fn capabilities(&self) -> Vec { + vec![ToolCapability::Sandboxable] + } + + fn approval_requirement(&self) -> ApprovalRequirement { + ApprovalRequirement::Manual + } + + fn supports_parallel(&self) -> bool { + false + } + + async fn execute(&self, input: Value, context: &ToolContext) -> Result { + let git_ctx = resolve_git_context(context, optional_str(&input, "path")?)?; + let dry_run = optional_bool(&input, "dry_run", false)?; + + // 1. Prepare untracked files so they are included in the diff + let status_args = vec![ + "-c".to_string(), + "core.quotepath=false".to_string(), + "status".to_string(), + "--porcelain=v1".to_string(), + ]; + let output = run_git_command(&git_ctx.working_dir, &status_args)?; + if output.status.success() { + let stdout = String::from_utf8_lossy(&output.stdout); + for line in stdout.lines() { + if line.starts_with("?? ") { + let path = &line[3..]; + let add_args = vec![ + "add".to_string(), + "-N".to_string(), + path.to_string(), + ]; + let _ = run_git_command(&git_ctx.working_dir, &add_args); + } + } + } + + // 2. Run git diff HEAD to get all staged and unstaged changes + let mut diff_args = vec![ + "-c".to_string(), + "core.quotepath=false".to_string(), + "diff".to_string(), + "HEAD".to_string(), + "--no-color".to_string(), + "--no-ext-diff".to_string(), + "-U3".to_string(), + ]; + if let Some(pathspec) = &git_ctx.pathspec { + diff_args.push("--".to_string()); + diff_args.push(pathspec.display().to_string()); + } + + let output = run_git_command(&git_ctx.working_dir, &diff_args)?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Ok(ToolResult::error(format!("git diff HEAD failed: {}", stderr.trim()))); + } + + let stdout = String::from_utf8_lossy(&output.stdout); + let hunks = parse_diff(&stdout); + if hunks.is_empty() { + return Ok(ToolResult::success("No changes to commit.")); + } + + // 3. Group hunks into logical components + let mut groups: Vec = Vec::new(); + let mut file_to_hunks: HashMap> = HashMap::new(); + let mut lock_files: Vec<(String, Vec)> = Vec::new(); + + for hunk in hunks { + if is_lock_file(&hunk.file_path) { + let mut found = false; + for (lf_path, lf_hunks) in &mut lock_files { + if lf_path == &hunk.file_path { + lf_hunks.push(hunk.clone()); + found = true; + break; + } + } + if !found { + lock_files.push((hunk.file_path.clone(), vec![hunk])); + } + } else { + file_to_hunks.entry(hunk.file_path.clone()).or_default().push(hunk); + } + } + + for (file_path, hunks) in file_to_hunks { + let mut defined_symbols = HashSet::new(); + let mut referenced_symbols = HashSet::new(); + for h in &hunks { + defined_symbols.extend(extract_defined_symbols(h)); + referenced_symbols.extend(extract_referenced_symbols(h)); + } + for sym in &defined_symbols { + referenced_symbols.remove(sym); + } + let mut files = HashMap::new(); + files.insert(file_path, hunks); + groups.push(CommitGroup { + files, + defined_symbols, + referenced_symbols, + }); + } + + // Associate lock files with matching manifest groups, or their own group + for (lf_path, lf_hunks) in lock_files { + let mut assigned = false; + for g in &mut groups { + let mut match_found = false; + for existing_file in g.files.keys() { + if matches_lock_file(existing_file, &lf_path) { + match_found = true; + break; + } + } + if match_found { + g.files.insert(lf_path.clone(), lf_hunks.clone()); + assigned = true; + break; + } + } + if !assigned { + let mut files = HashMap::new(); + files.insert(lf_path, lf_hunks); + groups.push(CommitGroup { + files, + defined_symbols: HashSet::new(), + referenced_symbols: HashSet::new(), + }); + } + } + + // Merge groups with closely related file names (e.g. tests, specs, docs) + let mut merged_groups: Vec = Vec::new(); + 'outer: for g in groups { + for mg in &mut merged_groups { + let mut should_merge = false; + for f1 in g.files.keys() { + for f2 in mg.files.keys() { + if are_files_related(f1, f2) { + should_merge = true; + break; + } + } + if should_merge { + break; + } + } + if should_merge { + for (k, v) in g.files { + mg.files.insert(k, v); + } + mg.defined_symbols.extend(g.defined_symbols); + mg.referenced_symbols.extend(g.referenced_symbols); + for sym in &mg.defined_symbols { + mg.referenced_symbols.remove(sym); + } + continue 'outer; + } + } + merged_groups.push(g); + } + let mut groups = merged_groups; + + // 4. Build the dependency graph + let n = groups.len(); + let mut adj = vec![vec![]; n]; + let mut in_degree = vec![0; n]; + + for i in 0..n { + for j in 0..n { + if i == j { + continue; + } + let mut depends = false; + for ref_sym in &groups[i].referenced_symbols { + if groups[j].defined_symbols.contains(ref_sym) { + depends = true; + break; + } + } + + if !depends { + let i_is_source = groups[i].files.keys().any(|f| is_source_file(f)); + let j_is_source = groups[j].files.keys().any(|f| is_source_file(f)); + if !i_is_source && j_is_source { + for f1 in groups[i].files.keys() { + for f2 in groups[j].files.keys() { + if share_context(f1, f2) { + depends = true; + break; + } + } + if depends { + break; + } + } + } + } + + if depends { + adj[j].push(i); + in_degree[i] += 1; + } + } + } + + // 5. Order the commits using topological sort + let mut ready = Vec::new(); + for i in 0..n { + if in_degree[i] == 0 { + ready.push(i); + } + } + + let mut sorted_order = Vec::new(); + while !ready.is_empty() { + ready.sort_by(|&idx_a, &idx_b| { + let a_is_source = groups[idx_a].files.keys().any(|f| is_source_file(f)); + let b_is_source = groups[idx_b].files.keys().any(|f| is_source_file(f)); + if a_is_source != b_is_source { + b_is_source.cmp(&a_is_source) + } else { + let a_first_file = groups[idx_a].files.keys().next().unwrap(); + let b_first_file = groups[idx_b].files.keys().next().unwrap(); + a_first_file.cmp(b_first_file) + } + }); + + let curr = ready.remove(0); + sorted_order.push(curr); + + for &next in &adj[curr] { + in_degree[next] -= 1; + if in_degree[next] == 0 { + ready.push(next); + } + } + } + + // Cycle detection! Reject immediately if cycle is present + if sorted_order.len() < n { + let mut cyclic_files = Vec::new(); + for i in 0..n { + if in_degree[i] > 0 { + cyclic_files.extend(groups[i].files.keys().cloned()); + } + } + let message = format!( + "Dependency cycle detected among changes in the following files: {}. Atomic commit splitting rejected.", + cyclic_files.join(", ") + ); + return Ok(ToolResult::error(message).with_metadata(json!({ + "cycle_detected": true, + "cyclic_files": cyclic_files, + }))); + } + + // 6. Propose or Apply the split commits + if dry_run { + let mut proposed_commits = Vec::new(); + for (idx, &g_idx) in sorted_order.iter().enumerate() { + let g = &groups[g_idx]; + let commit_message = generate_commit_message(g); + let files_list: Vec = g.files.keys().cloned().collect(); + proposed_commits.push(json!({ + "order": idx + 1, + "message": commit_message, + "files": files_list, + })); + } + return Ok(ToolResult::success( + serde_json::to_string_pretty(&proposed_commits).unwrap_or_default() + ).with_metadata(json!({ + "dry_run": true, + "commits": proposed_commits, + }))); + } + + let mut committed = Vec::new(); + for (idx, &g_idx) in sorted_order.iter().enumerate() { + let g = &groups[g_idx]; + let commit_message = generate_commit_message(g); + + let mut hunks_to_apply = Vec::new(); + for file_hunks in g.files.values() { + hunks_to_apply.extend(file_hunks.clone()); + } + + apply_hunks_and_commit(&git_ctx.working_dir, &hunks_to_apply, &commit_message)?; + + let files_list: Vec = g.files.keys().cloned().collect(); + committed.push(json!({ + "order": idx + 1, + "message": commit_message, + "files": files_list, + })); + } + + Ok(ToolResult::success(format!( + "Successfully split and applied {} commits.", + committed.len() + )).with_metadata(json!({ + "dry_run": false, + "commits": committed, + }))) + } +} + // === Helpers === struct GitContext { @@ -319,6 +669,337 @@ fn char_boundary_index(text: &str, max_chars: usize) -> usize { text.len() } +// === Commit Split Specific Types & Helpers === + +#[derive(Debug, Clone)] +pub struct Hunk { + pub file_path: String, + pub old_range: (usize, usize), + pub new_range: (usize, usize), + pub header: String, + pub lines: Vec, +} + +struct CommitGroup { + files: HashMap>, + defined_symbols: HashSet, + referenced_symbols: HashSet, +} + +fn parse_diff(diff_output: &str) -> Vec { + let mut hunks = Vec::new(); + let mut current_file = String::new(); + let mut current_hunk_header = String::new(); + let mut current_hunk_lines = Vec::new(); + let mut in_hunk = false; + let mut old_range = (0, 0); + let mut new_range = (0, 0); + + for line in diff_output.lines() { + if line.starts_with("diff --git ") { + if in_hunk { + hunks.push(Hunk { + file_path: current_file.clone(), + old_range, + new_range, + header: current_hunk_header.clone(), + lines: current_hunk_lines.clone(), + }); + in_hunk = false; + current_hunk_lines.clear(); + } + if let Some(pos) = line.rfind(" b/") { + let path = &line[pos + 3..]; + current_file = path.trim_matches('"').to_string(); + } else { + let parts: Vec<&str> = line.split_whitespace().collect(); + if parts.len() >= 4 { + current_file = parts[3].strip_prefix("b/").unwrap_or(parts[3]).to_string(); + } + } + } else if line.starts_with("@@ ") { + if in_hunk { + hunks.push(Hunk { + file_path: current_file.clone(), + old_range, + new_range, + header: current_hunk_header.clone(), + lines: current_hunk_lines.clone(), + }); + current_hunk_lines.clear(); + } + in_hunk = true; + current_hunk_header = line.to_string(); + let parts: Vec<&str> = line.split_whitespace().collect(); + if parts.len() >= 3 { + old_range = parse_range(parts[1].strip_prefix('-').unwrap_or(parts[1])); + new_range = parse_range(parts[2].strip_prefix('+').unwrap_or(parts[2])); + } + } else if in_hunk { + current_hunk_lines.push(line.to_string()); + } + } + + if in_hunk { + hunks.push(Hunk { + file_path: current_file, + old_range, + new_range, + header: current_hunk_header, + lines: current_hunk_lines, + }); + } + + hunks +} + +fn parse_range(s: &str) -> (usize, usize) { + let parts: Vec<&str> = s.split(',').collect(); + let start = parts.get(0).and_then(|x| x.parse().ok()).unwrap_or(0); + let count = parts.get(1).and_then(|x| x.parse().ok()).unwrap_or(1); + (start, count) +} + +fn is_lock_file(path: &str) -> bool { + let name = Path::new(path).file_name().and_then(|n| n.to_str()).unwrap_or(""); + name.ends_with(".lock") || name == "go.sum" || name == "package-lock.json" || name == "pnpm-lock.yaml" || name == "yarn.lock" +} + +fn matches_lock_file(manifest: &str, lock: &str) -> bool { + let m_path = Path::new(manifest); + let l_path = Path::new(lock); + let m_dir = m_path.parent().unwrap_or(Path::new("")); + let l_dir = l_path.parent().unwrap_or(Path::new("")); + if m_dir != l_dir { + return false; + } + let m_name = m_path.file_name().and_then(|n| n.to_str()).unwrap_or(""); + let l_name = l_path.file_name().and_then(|n| n.to_str()).unwrap_or(""); + match (m_name, l_name) { + ("Cargo.toml", "Cargo.lock") => true, + ("package.json", "package-lock.json" | "yarn.lock" | "pnpm-lock.yaml") => true, + ("go.mod", "go.sum") => true, + _ => { + let m_stem = m_path.file_stem().and_then(|s| s.to_str()).unwrap_or(""); + let l_stem = l_path.file_stem().and_then(|s| s.to_str()).unwrap_or(""); + m_stem == l_stem || (m_name.ends_with(".json") && l_name.ends_with(".json")) + } + } +} + +fn are_files_related(f1: &str, f2: &str) -> bool { + let p1 = Path::new(f1); + let p2 = Path::new(f2); + let stem1 = p1.file_stem().and_then(|s| s.to_str()).unwrap_or("").to_lowercase(); + let stem2 = p2.file_stem().and_then(|s| s.to_str()).unwrap_or("").to_lowercase(); + if stem1 == stem2 { + return true; + } + let clean_stem = |s: &str| { + s.replace("_test", "") + .replace("test_", "") + .replace("_spec", "") + .replace("spec_", "") + .replace("test", "") + }; + clean_stem(&stem1) == clean_stem(&stem2) && !clean_stem(&stem1).is_empty() +} + +fn is_source_file(path: &str) -> bool { + let p = Path::new(path); + let ext = p.extension().and_then(|e| e.to_str()).unwrap_or(""); + let name = p.file_name().and_then(|n| n.to_str()).unwrap_or("").to_lowercase(); + if name.contains("test") || name.contains("spec") || name.contains("mock") { + return false; + } + matches!(ext, "rs" | "py" | "go" | "js" | "ts" | "cpp" | "h" | "c" | "java" | "cs" | "rb" | "php") +} + +fn share_context(f1: &str, f2: &str) -> bool { + let p1 = Path::new(f1); + let p2 = Path::new(f2); + p1.parent() == p2.parent() +} + +fn extract_defined_symbols(hunk: &Hunk) -> HashSet { + let mut symbols = HashSet::new(); + for line in &hunk.lines { + if line.starts_with('+') && !line.starts_with("+++") { + let content = &line[1..]; + let tokens = tokenize(content); + for i in 0..tokens.len() { + let tok = &tokens[i]; + if tok == "fn" || tok == "func" || tok == "def" || tok == "function" || tok == "struct" || tok == "enum" || tok == "trait" || tok == "class" || tok == "interface" || tok == "type" || tok == "const" || tok == "let" || tok == "mod" { + if i + 1 < tokens.len() { + let sym = &tokens[i + 1]; + if is_valid_identifier(sym) { + symbols.insert(sym.clone()); + } + } + } + } + } + } + symbols +} + +fn extract_referenced_symbols(hunk: &Hunk) -> HashSet { + let mut symbols = HashSet::new(); + for line in &hunk.lines { + if line.starts_with('+') && !line.starts_with("+++") { + let content = &line[1..]; + let tokens = tokenize(content); + for tok in tokens { + if is_valid_identifier(&tok) && !is_keyword(&tok) { + symbols.insert(tok); + } + } + } + } + symbols +} + +fn tokenize(s: &str) -> Vec { + let mut tokens = Vec::new(); + let mut current = String::new(); + for c in s.chars() { + if c.is_alphanumeric() || c == '_' { + current.push(c); + } else { + if !current.is_empty() { + tokens.push(current.clone()); + current.clear(); + } + } + } + if !current.is_empty() { + tokens.push(current); + } + tokens +} + +fn is_valid_identifier(s: &str) -> bool { + if s.is_empty() { + return false; + } + let first = s.chars().next().unwrap(); + (first.is_alphabetic() || first == '_') && s.chars().all(|c| c.is_alphanumeric() || c == '_') +} + +fn is_keyword(s: &str) -> bool { + matches!( + s, + "if" | "else" | "while" | "for" | "return" | "import" | "use" | "pub" | "impl" | "crate" + | "self" | "true" | "false" | "let" | "mut" | "match" | "var" | "void" | "int" + | "string" | "bool" | "float" | "double" | "public" | "private" | "protected" + | "static" | "final" | "class" | "fn" | "struct" | "enum" | "trait" | "interface" + | "type" | "const" | "mod" | "def" | "func" | "function" | "and" | "or" | "not" + | "in" | "as" | "break" | "continue" | "new" | "this" | "super" + ) +} + +fn generate_commit_message(group: &CommitGroup) -> String { + let files: Vec<&String> = group.files.keys().collect(); + if files.len() == 1 { + let file = files[0]; + let p = Path::new(file); + let name = p.file_name().and_then(|n| n.to_str()).unwrap_or(file); + if !group.defined_symbols.is_empty() { + let syms: Vec = group.defined_symbols.iter().take(3).cloned().collect(); + format!("refactor({}): define {}", name, syms.join(", ")) + } else { + format!("style/update: changes in {}", name) + } + } else { + if !group.defined_symbols.is_empty() { + let syms: Vec = group.defined_symbols.iter().take(3).cloned().collect(); + format!("feat: implement {}", syms.join(", ")) + } else { + format!("chore: update multiple files including {}", files[0]) + } + } +} + +fn apply_hunks_and_commit( + working_dir: &Path, + hunks: &[Hunk], + commit_message: &str, +) -> Result<(), ToolError> { + let mut patch = String::new(); + let mut files_map: HashMap> = HashMap::new(); + for hunk in hunks { + files_map.entry(hunk.file_path.clone()).or_default().push(hunk); + } + + for (file_path, file_hunks) in files_map { + patch.push_str(&format!("diff --git a/{file_path} b/{file_path}\n")); + patch.push_str(&format!("--- a/{file_path}\n")); + patch.push_str(&format!("+++ b/{file_path}\n")); + for hunk in file_hunks { + patch.push_str(&hunk.header); + patch.push('\n'); + for line in &hunk.lines { + patch.push_str(line); + patch.push('\n'); + } + } + } + + let mut child = { + let mut cmd = crate::dependencies::Git::command().ok_or_else(|| { + ToolError::not_available("git is not installed or not in PATH") + })?; + cmd.args(&["-c", "core.quotepath=false", "apply", "--cached", "-"]) + .current_dir(working_dir) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()); + cmd.spawn().map_err(|e| { + ToolError::execution_failed(format!("Failed to spawn git apply: {e}")) + })? + }; + + { + use std::io::Write; + let mut stdin = child.stdin.take().ok_or_else(|| { + ToolError::execution_failed("Failed to open stdin for git apply") + })?; + stdin.write_all(patch.as_bytes()).map_err(|e| { + ToolError::execution_failed(format!("Failed to write to git apply: {e}")) + })?; + } + + let output = child.wait_with_output().map_err(|e| { + ToolError::execution_failed(format!("Failed to wait for git apply: {e}")) + })?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(ToolError::execution_failed(format!( + "git apply --cached failed: {}", + stderr.trim() + ))); + } + + let commit_args = vec![ + "-c".to_string(), + "core.quotepath=false".to_string(), + "commit".to_string(), + "-m".to_string(), + commit_message.to_string(), + ]; + let output = run_git_command(working_dir, &commit_args)?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(ToolError::execution_failed(format!( + "git commit failed: {}", + stderr.trim() + ))); + } + + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -486,9 +1167,6 @@ mod tests { #[test] fn format_command_joins_args_without_intermediate_vec() { - // Locks the output shape after dropping the collect-before-join - // allocation: joining the `&[String]` slice directly must be byte-for-byte - // identical to the previous `.map(String::as_str).collect().join(" ")`. let args = vec![ "-c".to_string(), "core.quotepath=false".to_string(), @@ -502,7 +1180,6 @@ mod tests { "git -C /tmp/repo -c core.quotepath=false status --porcelain=v1 -b" ); - // Empty args still render cleanly (trailing space, matching prior behavior). assert_eq!( format_command(Path::new("/tmp/repo"), &[]), "git -C /tmp/repo " @@ -517,4 +1194,121 @@ mod tests { assert!(omitted > 0); assert!(truncated.contains("output truncated")); } -} + + #[test] + fn test_parse_diff() { + let diff = r#"diff --git a/src/lib.rs b/src/lib.rs +index e69de29..4b2a8d3 100644 +--- a/src/lib.rs ++++ b/src/lib.rs +@@ -1,3 +1,4 @@ + line1 +-line2 ++line2 modified + line3 ++line4 added +"#; + let hunks = parse_diff(diff); + assert_eq!(hunks.len(), 1); + assert_eq!(hunks[0].file_path, "src/lib.rs"); + assert_eq!(hunks[0].old_range, (1, 3)); + assert_eq!(hunks[0].new_range, (1, 4)); + assert_eq!(hunks[0].header, "@@ -1,3 +1,4 @@"); + assert_eq!(hunks[0].lines.len(), 5); + } + + #[test] + fn test_dependency_extraction() { + let hunk = Hunk { + file_path: "src/lib.rs".to_string(), + old_range: (1, 1), + new_range: (1, 2), + header: "@@ -1 +1,2 @@".to_string(), + lines: vec![ + " pub fn add(a: i32, b: i32) -> i32 {".to_string(), + "+ let sum = a + b;".to_string(), + "+ struct Answer;".to_string(), + " sum".to_string(), + ], + }; + let defined = extract_defined_symbols(&hunk); + let referenced = extract_referenced_symbols(&hunk); + + assert!(defined.contains("Answer")); + assert!(defined.contains("sum")); + assert!(referenced.contains("sum")); + assert!(referenced.contains("Answer")); + } + + #[tokio::test] + async fn test_git_commit_split_success() { + if !git_available() { + return; + } + let tmp = tempdir().expect("tempdir"); + init_git_repo(tmp.path()); + + let math_file = tmp.path().join("math.rs"); + let main_file = tmp.path().join("main.rs"); + + fs::write(&math_file, "pub fn add(a: i32, b: i32) -> i32 { a + b }\n").expect("write math"); + fs::write(&main_file, "fn main() { let x = math::add(1, 2); }\n").expect("write main"); + + commit_all(tmp.path(), "init"); + + fs::write(&math_file, "pub fn add(a: i32, b: i32) -> i32 { a + b }\npub fn sub(a: i32, b: i32) -> i32 { a - b }\n").expect("modify math"); + fs::write(&main_file, "fn main() { let x = math::add(1, 2); let y = math::sub(3, 4); }\n").expect("modify main"); + + let ctx = ToolContext::new(tmp.path()); + let tool = GitCommitSplitTool; + + let result = tool.execute(json!({ "dry_run": true }), &ctx).await.expect("execute"); + assert!(result.success); + let val: Value = serde_json::from_str(&result.content).expect("parse response"); + let commits = val.as_array().expect("array of commits"); + assert_eq!(commits.len(), 2); + + let first_commit = &commits[0]; + let first_files = first_commit.get("files").unwrap().as_array().unwrap(); + assert!(first_files.iter().any(|f| f.as_str().unwrap().contains("math.rs"))); + + let second_commit = &commits[1]; + let second_files = second_commit.get("files").unwrap().as_array().unwrap(); + assert!(second_files.iter().any(|f| f.as_str().unwrap().contains("main.rs"))); + + let run_result = tool.execute(json!({ "dry_run": false }), &ctx).await.expect("execute"); + assert!(run_result.success); + + let log_output = run_git_command(tmp.path(), &["log".to_string(), "--oneline".to_string()]).expect("git log"); + let log_stdout = String::from_utf8_lossy(&log_output.stdout); + let lines: Vec<&str> = log_stdout.lines().collect(); + assert_eq!(lines.len(), 3); + } + + #[tokio::test] + async fn test_git_commit_split_cycle() { + if !git_available() { + return; + } + let tmp = tempdir().expect("tempdir"); + init_git_repo(tmp.path()); + + let a_file = tmp.path().join("a.rs"); + let b_file = tmp.path().join("b.rs"); + + fs::write(&a_file, "pub fn func_a() {}\n").expect("write a"); + fs::write(&b_file, "pub fn func_b() {}\n").expect("write b"); + + commit_all(tmp.path(), "init"); + + fs::write(&a_file, "pub fn func_a() {}\npub fn func_a2() { b::func_b2(); }\n").expect("modify a"); + fs::write(&b_file, "pub fn func_b() {}\npub fn func_b2() { a::func_a2(); }\n").expect("modify b"); + + let ctx = ToolContext::new(tmp.path()); + let tool = GitCommitSplitTool; + + let result = tool.execute(json!({ "dry_run": true }), &ctx).await.expect("execute"); + assert!(!result.success); + assert!(result.content.contains("Dependency cycle detected")); + } +} \ No newline at end of file From f94d4aa95014ec779f15b4c9fd25b987ff36a94a Mon Sep 17 00:00:00 2001 From: CodeWhale Bot Date: Sun, 6 Sep 2026 17:06:45 -0700 Subject: [PATCH 2/4] fix(tools): commit_plan is propose-only (#3999 rework) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maintainer rework of #5870 (fixes #3999) on top of @goransh-walia's commit, per review: - The atomic-commit planner (Git action `commit_plan`, GitCommitPlanTool) is propose-only: it returns a dependency-ordered split plan with messages and writes nothing. The old mutation paths — `git add -N` on untracked files, `git apply --cached` plus a spawned `git commit` — are deleted. Untracked files are discovered with `ls-files --others` and read from disk for symbol analysis; landing commits stays on the ordinary `git add` / `git commit` shell path, where the approval gate already applies. - The non-existent `ApprovalRequirement::Manual` override is gone. The tool declares ReadOnly + Sandboxable and derives ApprovalRequirement::Auto explicitly, matching its git_status/git_diff siblings. - Every alias consumer classes commit_plan read-only: the execution envelope (classify_call -> Bounded, is_read_only_for), the approval policy (Safe / Benign), the hooks tool_category gate ("safe"), the tool card family (Read), and history activity (File). - Removed the now-dead Hunk old_range/new_range fields and parse_range (nothing rebuilds a patch from them anymore); a group of a source file plus its test scopes its commit message by the shared stem, not the directory. - Tests: grouping, dependency ordering, and cycle rejection kept; new propose-only proof (index, HEAD, and the untracked list untouched), staged-changes warning, clean-tree report, and envelope classification. Gates, final tree (rebased onto origin/main f9746854c): - cargo fmt --all: clean - cargo clippy --workspace --all-targets --all-features --locked (-D warnings, allowing uninlined_format_args/too_many_arguments/ unnecessary_map_or): exit 0 - cargo test -p codewhale-tui --lib --locked -- tools::git git_tool commit_plan canonical_action: 49 passed, 0 failed, 0 ignored - cargo test -p codewhale-tui --lib --locked: 11880 passed, 0 failed, 13 ignored Note on the shared target dir: two earlier full-suite runs on the pre-rebase tree reported 2143 failures and one intermediate run 1 failure; all were traced to the shared CARGO_TARGET_DIR serving this worktree codewhale-config / test-binary artifacts built from other in-flight worktrees (e.g. a 49-variant ProviderKind rlib against this tree's 48-entry FROM_KIND_LOOKUP). The counts above are from a run on this tree's own binary. Signed-off-by: CodeWhale Bot --- CHANGELOG.md | 11 + crates/tui/CHANGELOG.md | 11 + crates/tui/src/hooks/executor.rs | 6 +- crates/tui/src/tools/canonical_action.rs | 1 + crates/tui/src/tools/git.rs | 1514 ++++++++++++++-------- crates/tui/src/tools/git_tool.rs | 31 +- crates/tui/src/tui/approval/policy.rs | 2 + crates/tui/src/tui/history/tool_run.rs | 4 +- crates/tui/src/tui/widgets/tool_card.rs | 3 +- 9 files changed, 1002 insertions(+), 581 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cd3b2cb3e..7bf9799879 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -117,6 +117,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 per-World session container before each command — full tree first, then only changes and deletions — so remote builds and tests run on the files just edited locally and their outputs persist across commands. +- `Git` grows a `commit_plan` action: a propose-only planner that splits the + working tree into ordered atomic commits (#3999). It groups whole files — + lock files ride with their manifest, tests ride with the source they name — + orders the groups so a commit that defines a symbol lands before the commit + that uses it, and refuses the whole plan when that dependency graph has a + cycle. It reads `git diff HEAD` plus the untracked-file list and writes + nothing: no `git add -N`, no `git apply --cached`, no `git commit`, so + staging and committing stay with the ordinary `git add` / `git commit` shell + path where the approval gate already applies. Thanks + [@goransh-walia](https://github.com/goransh-walia) for the original + implementation (PR #5870, fixes #3999). ## [0.9.12] - 2026-09-03 diff --git a/crates/tui/CHANGELOG.md b/crates/tui/CHANGELOG.md index 47025c6680..7a109eb34e 100644 --- a/crates/tui/CHANGELOG.md +++ b/crates/tui/CHANGELOG.md @@ -117,6 +117,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 per-World session container before each command — full tree first, then only changes and deletions — so remote builds and tests run on the files just edited locally and their outputs persist across commands. +- `Git` grows a `commit_plan` action: a propose-only planner that splits the + working tree into ordered atomic commits (#3999). It groups whole files — + lock files ride with their manifest, tests ride with the source they name — + orders the groups so a commit that defines a symbol lands before the commit + that uses it, and refuses the whole plan when that dependency graph has a + cycle. It reads `git diff HEAD` plus the untracked-file list and writes + nothing: no `git add -N`, no `git apply --cached`, no `git commit`, so + staging and committing stay with the ordinary `git add` / `git commit` shell + path where the approval gate already applies. Thanks + [@goransh-walia](https://github.com/goransh-walia) for the original + implementation (PR #5870, fixes #3999). ## [0.9.12] - 2026-09-03 diff --git a/crates/tui/src/hooks/executor.rs b/crates/tui/src/hooks/executor.rs index 3bbbe36293..3bfa1e1115 100644 --- a/crates/tui/src/hooks/executor.rs +++ b/crates/tui/src/hooks/executor.rs @@ -2517,7 +2517,7 @@ fn tool_category_for(tool_name: &str, tool_args: Option<&str>) -> &'static str { "Git" | "git" => match action.as_deref() { // Every shipped Git action is read-only today; classify by action // anyway so adding a mutating one cannot silently inherit `safe`. - Some("status" | "diff" | "log" | "show" | "blame") => "safe", + Some("status" | "diff" | "log" | "show" | "blame" | "commit_plan") => "safe", _ => "other", }, // `Run` executes test/verifier commands — closer to shell than safe. @@ -5821,6 +5821,10 @@ command = "echo project" tool_category_for("Git", Some(r#"{"action":"log"}"#)), "safe" ); + assert_eq!( + tool_category_for("Git", Some(r#"{"action":"commit_plan"}"#)), + "safe" + ); assert_eq!(tool_category_for("web.run", None), "other"); } } diff --git a/crates/tui/src/tools/canonical_action.rs b/crates/tui/src/tools/canonical_action.rs index a4f3c1c746..51c14bb65f 100644 --- a/crates/tui/src/tools/canonical_action.rs +++ b/crates/tui/src/tools/canonical_action.rs @@ -37,6 +37,7 @@ pub(crate) const CANONICAL_ACTION_ALIASES: &[(&str, &str, &str)] = &[ ("Git", "log", "git_log"), ("Git", "show", "git_show"), ("Git", "blame", "git_blame"), + ("Git", "commit_plan", "git_commit_plan"), ("Run", "tests", "run_tests"), ("Run", "verifiers", "run_verifiers"), ("Web", "search", "web_search"), diff --git a/crates/tui/src/tools/git.rs b/crates/tui/src/tools/git.rs index 8be4d6c625..ccd3070754 100644 --- a/crates/tui/src/tools/git.rs +++ b/crates/tui/src/tools/git.rs @@ -1,9 +1,12 @@ -//! Git power tools: `git_status` and `git_diff`. +//! Git power tools: `git_status`, `git_diff`, and the propose-only +//! `git_commit_plan`. //! //! These tools are read-only wrappers around common git inspection commands, -//! scoped to the workspace and optionally to a sub-path within it. +//! scoped to the workspace and optionally to a sub-path within it. The commit +//! planner (#3999) is read-only too: it proposes an ordered atomic split and +//! leaves staging and committing to the ordinary shell write path. -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::fs; use std::path::{Path, PathBuf}; @@ -210,36 +213,42 @@ impl ToolSpec for GitDiffTool { } } -// === GitCommitSplitTool === - -/// Tool for splitting unstaged and staged changes into atomic commits. -pub struct GitCommitSplitTool; +// === GitCommitPlanTool === + +/// Propose-only planner that splits the working tree into ordered atomic +/// commits (#3999). +/// +/// The planner reads `git diff HEAD` plus the untracked-file list, groups +/// whole files into logical commits, orders the groups so a commit that +/// defines a symbol lands before the commit that uses it, and refuses the +/// whole plan when that dependency graph has a cycle. It never touches the +/// index or the object store — no `git add -N`, no `git apply --cached`, no +/// `git commit`. The model lands each proposed commit through the ordinary +/// `git add` / `git commit` shell path, which is where the approval gate +/// already lives: one commit authority, not two. +pub struct GitCommitPlanTool; #[async_trait] -impl ToolSpec for GitCommitSplitTool { +impl ToolSpec for GitCommitPlanTool { fn name(&self) -> &'static str { - "commit_split" + "git_commit_plan" } fn model_visible(&self) -> bool { - true + false } fn description(&self) -> &'static str { - "Analyze the working tree, group changes into logical commits, order them by dependency (rejecting cycles), and apply the split commits." + "Propose how to split the working tree into ordered atomic commits. Read-only: returns the plan and writes nothing." } fn input_schema(&self) -> Value { json!({ "type": "object", "properties": { - "dry_run": { - "type": "boolean", - "description": "If true, only return the proposed split commits and dependency graph without writing." - }, "path": { "type": "string", - "description": "Optional subdirectory or file to scope the split to (must be within the workspace)." + "description": "Optional subdirectory or file to scope the plan to (must be within the workspace)." } }, "additionalProperties": false @@ -247,45 +256,27 @@ impl ToolSpec for GitCommitSplitTool { } fn capabilities(&self) -> Vec { - vec![ToolCapability::Sandboxable] + vec![ToolCapability::ReadOnly, ToolCapability::Sandboxable] } fn approval_requirement(&self) -> ApprovalRequirement { - ApprovalRequirement::Manual + ApprovalRequirement::Auto } fn supports_parallel(&self) -> bool { - false + true } async fn execute(&self, input: Value, context: &ToolContext) -> Result { let git_ctx = resolve_git_context(context, optional_str(&input, "path")?)?; - let dry_run = optional_bool(&input, "dry_run", false)?; + let working_dir = &git_ctx.working_dir; - // 1. Prepare untracked files so they are included in the diff - let status_args = vec![ - "-c".to_string(), - "core.quotepath=false".to_string(), - "status".to_string(), - "--porcelain=v1".to_string(), - ]; - let output = run_git_command(&git_ctx.working_dir, &status_args)?; - if output.status.success() { - let stdout = String::from_utf8_lossy(&output.stdout); - for line in stdout.lines() { - if line.starts_with("?? ") { - let path = &line[3..]; - let add_args = vec![ - "add".to_string(), - "-N".to_string(), - path.to_string(), - ]; - let _ = run_git_command(&git_ctx.working_dir, &add_args); - } - } - } + let root_args = vec!["rev-parse".to_string(), "--show-toplevel".to_string()]; + let repo_root = match git_stdout(working_dir, &root_args)? { + Ok(stdout) => PathBuf::from(String::from_utf8_lossy(&stdout).trim_end()), + Err(failure) => return Ok(failure), + }; - // 2. Run git diff HEAD to get all staged and unstaged changes let mut diff_args = vec![ "-c".to_string(), "core.quotepath=false".to_string(), @@ -299,262 +290,113 @@ impl ToolSpec for GitCommitSplitTool { diff_args.push("--".to_string()); diff_args.push(pathspec.display().to_string()); } + let command = format_command(working_dir, &diff_args); + let mut files = match git_stdout(working_dir, &diff_args)? { + Ok(stdout) => parse_diff(&String::from_utf8_lossy(&stdout)), + Err(failure) => return Ok(failure), + }; - let output = run_git_command(&git_ctx.working_dir, &diff_args)?; - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Ok(ToolResult::error(format!("git diff HEAD failed: {}", stderr.trim()))); - } - - let stdout = String::from_utf8_lossy(&output.stdout); - let hunks = parse_diff(&stdout); - if hunks.is_empty() { - return Ok(ToolResult::success("No changes to commit.")); - } - - // 3. Group hunks into logical components - let mut groups: Vec = Vec::new(); - let mut file_to_hunks: HashMap> = HashMap::new(); - let mut lock_files: Vec<(String, Vec)> = Vec::new(); - - for hunk in hunks { - if is_lock_file(&hunk.file_path) { - let mut found = false; - for (lf_path, lf_hunks) in &mut lock_files { - if lf_path == &hunk.file_path { - lf_hunks.push(hunk.clone()); - found = true; - break; - } - } - if !found { - lock_files.push((hunk.file_path.clone(), vec![hunk])); - } - } else { - file_to_hunks.entry(hunk.file_path.clone()).or_default().push(hunk); - } - } - - for (file_path, hunks) in file_to_hunks { - let mut defined_symbols = HashSet::new(); - let mut referenced_symbols = HashSet::new(); - for h in &hunks { - defined_symbols.extend(extract_defined_symbols(h)); - referenced_symbols.extend(extract_referenced_symbols(h)); - } - for sym in &defined_symbols { - referenced_symbols.remove(sym); - } - let mut files = HashMap::new(); - files.insert(file_path, hunks); - groups.push(CommitGroup { - files, - defined_symbols, - referenced_symbols, - }); - } - - // Associate lock files with matching manifest groups, or their own group - for (lf_path, lf_hunks) in lock_files { - let mut assigned = false; - for g in &mut groups { - let mut match_found = false; - for existing_file in g.files.keys() { - if matches_lock_file(existing_file, &lf_path) { - match_found = true; - break; - } - } - if match_found { - g.files.insert(lf_path.clone(), lf_hunks.clone()); - assigned = true; - break; - } - } - if !assigned { - let mut files = HashMap::new(); - files.insert(lf_path, lf_hunks); - groups.push(CommitGroup { - files, - defined_symbols: HashSet::new(), - referenced_symbols: HashSet::new(), - }); - } - } - - // Merge groups with closely related file names (e.g. tests, specs, docs) - let mut merged_groups: Vec = Vec::new(); - 'outer: for g in groups { - for mg in &mut merged_groups { - let mut should_merge = false; - for f1 in g.files.keys() { - for f2 in mg.files.keys() { - if are_files_related(f1, f2) { - should_merge = true; - break; - } - } - if should_merge { - break; - } - } - if should_merge { - for (k, v) in g.files { - mg.files.insert(k, v); - } - mg.defined_symbols.extend(g.defined_symbols); - mg.referenced_symbols.extend(g.referenced_symbols); - for sym in &mg.defined_symbols { - mg.referenced_symbols.remove(sym); - } - continue 'outer; - } - } - merged_groups.push(g); - } - let mut groups = merged_groups; - - // 4. Build the dependency graph - let n = groups.len(); - let mut adj = vec![vec![]; n]; - let mut in_degree = vec![0; n]; - - for i in 0..n { - for j in 0..n { - if i == j { - continue; - } - let mut depends = false; - for ref_sym in &groups[i].referenced_symbols { - if groups[j].defined_symbols.contains(ref_sym) { - depends = true; - break; - } - } - - if !depends { - let i_is_source = groups[i].files.keys().any(|f| is_source_file(f)); - let j_is_source = groups[j].files.keys().any(|f| is_source_file(f)); - if !i_is_source && j_is_source { - for f1 in groups[i].files.keys() { - for f2 in groups[j].files.keys() { - if share_context(f1, f2) { - depends = true; - break; - } - } - if depends { - break; - } - } - } - } - - if depends { - adj[j].push(i); - in_degree[i] += 1; - } - } - } - - // 5. Order the commits using topological sort - let mut ready = Vec::new(); - for i in 0..n { - if in_degree[i] == 0 { - ready.push(i); - } + // Untracked files are listed by path and read for symbol analysis. + // Never `git add -N` them: intent-to-add mutates the index, and a + // planner that mutates the index is not propose-only. + let mut untracked_args = vec![ + "-c".to_string(), + "core.quotepath=false".to_string(), + "ls-files".to_string(), + "--others".to_string(), + "--exclude-standard".to_string(), + "--full-name".to_string(), + "-z".to_string(), + ]; + if let Some(pathspec) = &git_ctx.pathspec { + untracked_args.push("--".to_string()); + untracked_args.push(pathspec.display().to_string()); } - - let mut sorted_order = Vec::new(); - while !ready.is_empty() { - ready.sort_by(|&idx_a, &idx_b| { - let a_is_source = groups[idx_a].files.keys().any(|f| is_source_file(f)); - let b_is_source = groups[idx_b].files.keys().any(|f| is_source_file(f)); - if a_is_source != b_is_source { - b_is_source.cmp(&a_is_source) - } else { - let a_first_file = groups[idx_a].files.keys().next().unwrap(); - let b_first_file = groups[idx_b].files.keys().next().unwrap(); - a_first_file.cmp(b_first_file) - } - }); - - let curr = ready.remove(0); - sorted_order.push(curr); - - for &next in &adj[curr] { - in_degree[next] -= 1; - if in_degree[next] == 0 { - ready.push(next); + match git_stdout(working_dir, &untracked_args)? { + Ok(stdout) => { + for path in String::from_utf8_lossy(&stdout) + .split('\0') + .filter(|path| !path.is_empty()) + { + files.push(ChangedFile { + path: path.to_string(), + hunks: untracked_hunk(&repo_root, path).into_iter().collect(), + untracked: true, + }); } } + Err(failure) => return Ok(failure), } - // Cycle detection! Reject immediately if cycle is present - if sorted_order.len() < n { - let mut cyclic_files = Vec::new(); - for i in 0..n { - if in_degree[i] > 0 { - cyclic_files.extend(groups[i].files.keys().cloned()); - } - } - let message = format!( - "Dependency cycle detected among changes in the following files: {}. Atomic commit splitting rejected.", - cyclic_files.join(", ") + if files.is_empty() { + return Ok( + ToolResult::success("No changes to plan: the working tree matches HEAD.") + .with_metadata(json!({ + "command": command, + "propose_only": true, + "commits": [], + })), ); - return Ok(ToolResult::error(message).with_metadata(json!({ - "cycle_detected": true, - "cyclic_files": cyclic_files, - }))); } - // 6. Propose or Apply the split commits - if dry_run { - let mut proposed_commits = Vec::new(); - for (idx, &g_idx) in sorted_order.iter().enumerate() { - let g = &groups[g_idx]; - let commit_message = generate_commit_message(g); - let files_list: Vec = g.files.keys().cloned().collect(); - proposed_commits.push(json!({ - "order": idx + 1, - "message": commit_message, - "files": files_list, - })); - } - return Ok(ToolResult::success( - serde_json::to_string_pretty(&proposed_commits).unwrap_or_default() - ).with_metadata(json!({ - "dry_run": true, - "commits": proposed_commits, - }))); - } - - let mut committed = Vec::new(); - for (idx, &g_idx) in sorted_order.iter().enumerate() { - let g = &groups[g_idx]; - let commit_message = generate_commit_message(g); - - let mut hunks_to_apply = Vec::new(); - for file_hunks in g.files.values() { - hunks_to_apply.extend(file_hunks.clone()); + let staged_args = vec![ + "diff".to_string(), + "--cached".to_string(), + "--quiet".to_string(), + ]; + let index_has_staged_changes = + run_git_command(working_dir, &staged_args)?.status.code() == Some(1); + + let commits = match plan_commits(files) { + Ok(commits) => commits, + Err(cycle) => { + let message = format!( + "Dependency cycle detected among changes in: {}. Atomic commit split rejected; nothing was written.\nCycle edges:\n{}", + cycle.files.join(", "), + cycle + .edges + .iter() + .map(|edge| format!(" {edge}")) + .collect::>() + .join("\n") + ); + return Ok(ToolResult::error(message).with_metadata(json!({ + "command": command, + "propose_only": true, + "cycle_detected": true, + "cyclic_files": cycle.files, + "cycle_edges": cycle.edges, + }))); } + }; - apply_hunks_and_commit(&git_ctx.working_dir, &hunks_to_apply, &commit_message)?; - - let files_list: Vec = g.files.keys().cloned().collect(); - committed.push(json!({ - "order": idx + 1, - "message": commit_message, - "files": files_list, - })); - } + let content = render_commit_plan(&repo_root, index_has_staged_changes, &commits); + let (content, truncated, omitted_chars) = truncate_with_note(&content, MAX_OUTPUT_CHARS); + let metadata_commits: Vec = commits + .iter() + .enumerate() + .map(|(idx, commit)| { + json!({ + "order": idx + 1, + "message": commit.message, + "files": commit.files.iter().filter(|f| !f.untracked).map(|f| &f.path).collect::>(), + "untracked": commit.files.iter().filter(|f| f.untracked).map(|f| &f.path).collect::>(), + "hunks": commit.files.iter().flat_map(|f| f.hunks.iter().map(|h| json!({"file": h.file_path, "header": h.header}))).collect::>(), + "defines": commit.defines, + "depends_on": commit.depends_on.iter().map(|(order, reason)| json!({"order": order, "reason": reason})).collect::>(), + }) + }) + .collect(); - Ok(ToolResult::success(format!( - "Successfully split and applied {} commits.", - committed.len() - )).with_metadata(json!({ - "dry_run": false, - "commits": committed, + Ok(ToolResult::success(content).with_metadata(json!({ + "command": command, + "repo_root": repo_root.display().to_string(), + "propose_only": true, + "cycle_detected": false, + "index_has_staged_changes": index_has_staged_changes, + "commits": metadata_commits, + "truncated": truncated, + "omitted_chars": omitted_chars, }))) } } @@ -671,172 +513,494 @@ fn char_boundary_index(text: &str, max_chars: usize) -> usize { // === Commit Split Specific Types & Helpers === +// === Commit plan: types, parsing, grouping, ordering === + +/// Largest untracked file the planner reads for symbol analysis. Bigger or +/// binary files are still listed by path; they just carry no hunks. +const MAX_UNTRACKED_BYTES: u64 = 1 << 20; + +/// One hunk of a unified diff, kept for symbol analysis and for the plan's +/// per-file hunk listing. The `@@` ranges stay in `header`: nothing rebuilds +/// a patch from them anymore, so parsed copies would be dead weight. #[derive(Debug, Clone)] pub struct Hunk { pub file_path: String, - pub old_range: (usize, usize), - pub new_range: (usize, usize), pub header: String, pub lines: Vec, } +/// One file the working tree changed relative to HEAD. Tracked binary and +/// mode-only changes carry no hunks; untracked files carry a synthesized +/// all-additions hunk when they are readable text. +#[derive(Debug, Clone)] +pub struct ChangedFile { + pub path: String, + pub hunks: Vec, + pub untracked: bool, +} + +/// One proposed commit in dependency order. +#[derive(Debug, Clone)] +pub struct PlannedCommit { + pub message: String, + /// Sorted by path; every hunk of a file stays in the same commit. + pub files: Vec, + pub defines: Vec, + /// `(order, reason)` pairs naming the earlier commits this one builds on. + pub depends_on: Vec<(usize, String)>, +} + +/// Why a plan was refused: the files on the cycle and the edges that close it. +#[derive(Debug, Clone)] +pub struct CycleDiagnostic { + pub files: Vec, + pub edges: Vec, +} + struct CommitGroup { - files: HashMap>, - defined_symbols: HashSet, - referenced_symbols: HashSet, + files: BTreeMap, + defined_symbols: BTreeSet, + referenced_symbols: BTreeSet, } -fn parse_diff(diff_output: &str) -> Vec { - let mut hunks = Vec::new(); - let mut current_file = String::new(); - let mut current_hunk_header = String::new(); - let mut current_hunk_lines = Vec::new(); - let mut in_hunk = false; - let mut old_range = (0, 0); - let mut new_range = (0, 0); +impl CommitGroup { + fn from_file(file: ChangedFile) -> Self { + let mut defined_symbols = BTreeSet::new(); + let mut referenced_symbols = BTreeSet::new(); + for hunk in &file.hunks { + defined_symbols.extend(extract_defined_symbols(hunk)); + referenced_symbols.extend(extract_referenced_symbols(hunk)); + } + let mut group = Self { + files: BTreeMap::from([(file.path.clone(), file)]), + defined_symbols, + referenced_symbols, + }; + group.drop_self_references(); + group + } + + fn absorb(&mut self, other: CommitGroup) { + self.files.extend(other.files); + self.defined_symbols.extend(other.defined_symbols); + self.referenced_symbols.extend(other.referenced_symbols); + self.drop_self_references(); + } + + fn drop_self_references(&mut self) { + for symbol in &self.defined_symbols { + self.referenced_symbols.remove(symbol); + } + } + + fn has_source_file(&self) -> bool { + self.files.keys().any(|path| is_source_file(path)) + } + + fn first_path(&self) -> &str { + self.files.keys().next().map_or("", String::as_str) + } +} + +/// Run git and hand back stdout, or the operator-facing failure result. +fn git_stdout( + working_dir: &Path, + args: &[String], +) -> Result, ToolResult>, ToolError> { + let output = run_git_command(working_dir, args)?; + if output.status.success() { + return Ok(Ok(output.stdout)); + } + let stderr = String::from_utf8_lossy(&output.stderr); + Ok(Err(ToolResult::error(format!( + "{} failed: {}", + format_command(working_dir, args), + stderr.trim() + )))) +} + +/// Parse `git diff` output into per-file hunks. Every `diff --git` section +/// yields a file even when it has no hunks (binary or mode-only change), so +/// no changed file can silently drop out of the plan. +fn parse_diff(diff_output: &str) -> Vec { + let mut files: Vec = Vec::new(); + let mut current_hunk: Option = None; + + let flush = |files: &mut Vec, hunk: Option| { + if let (Some(hunk), Some(file)) = (hunk, files.last_mut()) { + file.hunks.push(hunk); + } + }; for line in diff_output.lines() { - if line.starts_with("diff --git ") { - if in_hunk { - hunks.push(Hunk { - file_path: current_file.clone(), - old_range, - new_range, - header: current_hunk_header.clone(), - lines: current_hunk_lines.clone(), - }); - in_hunk = false; - current_hunk_lines.clear(); - } - if let Some(pos) = line.rfind(" b/") { - let path = &line[pos + 3..]; - current_file = path.trim_matches('"').to_string(); - } else { - let parts: Vec<&str> = line.split_whitespace().collect(); - if parts.len() >= 4 { - current_file = parts[3].strip_prefix("b/").unwrap_or(parts[3]).to_string(); - } - } + if let Some(rest) = line.strip_prefix("diff --git ") { + flush(&mut files, current_hunk.take()); + let path = rest + .rfind(" b/") + .map(|pos| &rest[pos + 3..]) + .unwrap_or(rest) + .trim_matches('"') + .to_string(); + files.push(ChangedFile { + path, + hunks: Vec::new(), + untracked: false, + }); } else if line.starts_with("@@ ") { - if in_hunk { - hunks.push(Hunk { - file_path: current_file.clone(), - old_range, - new_range, - header: current_hunk_header.clone(), - lines: current_hunk_lines.clone(), + flush(&mut files, current_hunk.take()); + let Some(file) = files.last() else { continue }; + current_hunk = Some(Hunk { + file_path: file.path.clone(), + header: line.to_string(), + lines: Vec::new(), + }); + } else if let Some(hunk) = current_hunk.as_mut() { + hunk.lines.push(line.to_string()); + } + } + flush(&mut files, current_hunk.take()); + files +} + +/// Synthesize an all-additions hunk for an untracked text file so its +/// symbols take part in grouping. Binary, oversized, or unreadable files +/// yield `None` and are listed by path only. +fn untracked_hunk(repo_root: &Path, path: &str) -> Option { + let full = repo_root.join(path); + if fs::metadata(&full).ok()?.len() > MAX_UNTRACKED_BYTES { + return None; + } + let bytes = fs::read(&full).ok()?; + if bytes.iter().take(8000).any(|byte| *byte == 0) { + return None; + } + let text = String::from_utf8(bytes).ok()?; + let lines: Vec = text.lines().map(|line| format!("+{line}")).collect(); + Some(Hunk { + file_path: path.to_string(), + header: format!("@@ -0,0 +1,{} @@", lines.len()), + lines, + }) +} + +/// Group changed files into commits and order them by dependency. +/// +/// Pure: reads nothing from git and writes nothing anywhere. Returns the +/// cycle diagnostic instead of a plan when the dependency graph is not a DAG. +fn plan_commits(files: Vec) -> Result, CycleDiagnostic> { + let (lock_files, files): (Vec, Vec) = + files.into_iter().partition(|file| is_lock_file(&file.path)); + + let mut groups: Vec = files.into_iter().map(CommitGroup::from_file).collect(); + + // Lock files are excluded from symbol analysis and ride with the manifest + // change that moved them; a lock file with no manifest change stands alone. + for lock in lock_files { + let lock_path = lock.path.clone(); + let mut group = CommitGroup::from_file(lock); + group.defined_symbols.clear(); + group.referenced_symbols.clear(); + match groups.iter_mut().find(|existing| { + existing + .files + .keys() + .any(|manifest| matches_lock_file(manifest, &lock_path)) + }) { + Some(manifest_group) => manifest_group.files.extend(group.files), + None => groups.push(group), + } + } + + // Merge files with closely related names (source with its test/spec). + let mut merged: Vec = Vec::new(); + for group in groups { + let related = merged.iter_mut().find(|existing| { + group + .files + .keys() + .any(|a| existing.files.keys().any(|b| are_files_related(a, b))) + }); + match related { + Some(existing) => existing.absorb(group), + None => merged.push(group), + } + } + let groups = merged; + + // Dependency graph: `edges[j]` lists the groups that must land after j. + let n = groups.len(); + let mut edges: Vec> = vec![Vec::new(); n]; + let mut in_degree = vec![0usize; n]; + for i in 0..n { + for j in 0..n { + if i == j { + continue; + } + let reason = groups[i] + .referenced_symbols + .iter() + .find(|symbol| groups[j].defined_symbols.contains(*symbol)) + .map(|symbol| format!("uses `{symbol}`")) + .or_else(|| { + // Tests, docs, and configs follow the source change that + // lives beside them. + (!groups[i].has_source_file() && groups[j].has_source_file()) + .then(|| { + groups[i] + .files + .keys() + .any(|a| groups[j].files.keys().any(|b| share_context(a, b))) + }) + .filter(|shares| *shares) + .map(|_| "follows the source change in the same directory".to_string()) }); - current_hunk_lines.clear(); + if let Some(reason) = reason { + edges[j].push((i, reason)); + in_degree[i] += 1; } - in_hunk = true; - current_hunk_header = line.to_string(); - let parts: Vec<&str> = line.split_whitespace().collect(); - if parts.len() >= 3 { - old_range = parse_range(parts[1].strip_prefix('-').unwrap_or(parts[1])); - new_range = parse_range(parts[2].strip_prefix('+').unwrap_or(parts[2])); + } + } + + // Kahn's algorithm; among ready groups, source changes land first, then + // path order, so the plan is deterministic. + let mut ready: Vec = (0..n).filter(|&i| in_degree[i] == 0).collect(); + let mut order: Vec = Vec::with_capacity(n); + while !ready.is_empty() { + ready.sort_by(|&a, &b| { + groups[b] + .has_source_file() + .cmp(&groups[a].has_source_file()) + .then_with(|| groups[a].first_path().cmp(groups[b].first_path())) + }); + let current = ready.remove(0); + order.push(current); + for (next, _) in &edges[current] { + in_degree[*next] -= 1; + if in_degree[*next] == 0 { + ready.push(*next); } - } else if in_hunk { - current_hunk_lines.push(line.to_string()); } } - if in_hunk { - hunks.push(Hunk { - file_path: current_file, - old_range, - new_range, - header: current_hunk_header, - lines: current_hunk_lines, + if order.len() < n { + let cyclic: Vec = (0..n).filter(|&i| in_degree[i] > 0).collect(); + let files = cyclic + .iter() + .flat_map(|&i| groups[i].files.keys().cloned()) + .collect(); + let mut cycle_edges = Vec::new(); + for &j in &cyclic { + for (i, reason) in &edges[j] { + if cyclic.contains(i) { + cycle_edges.push(format!( + "{} -> {} ({reason})", + groups[*i].first_path(), + groups[j].first_path() + )); + } + } + } + return Err(CycleDiagnostic { + files, + edges: cycle_edges, }); } - hunks + let mut position = vec![0usize; n]; + for (idx, &group) in order.iter().enumerate() { + position[group] = idx + 1; + } + let mut depends_on: Vec> = vec![Vec::new(); n]; + for (j, outgoing) in edges.iter().enumerate() { + for (i, reason) in outgoing { + depends_on[*i].push((position[j], reason.clone())); + } + } + + Ok(order + .into_iter() + .map(|idx| { + let mut deps = std::mem::take(&mut depends_on[idx]); + deps.sort(); + let group = &groups[idx]; + PlannedCommit { + message: generate_commit_message(group), + files: group.files.values().cloned().collect(), + defines: group.defined_symbols.iter().cloned().collect(), + depends_on: deps, + } + }) + .collect()) } -fn parse_range(s: &str) -> (usize, usize) { - let parts: Vec<&str> = s.split(',').collect(); - let start = parts.get(0).and_then(|x| x.parse().ok()).unwrap_or(0); - let count = parts.get(1).and_then(|x| x.parse().ok()).unwrap_or(1); - (start, count) +fn render_commit_plan( + repo_root: &Path, + index_has_staged_changes: bool, + commits: &[PlannedCommit], +) -> String { + let mut out = format!( + "Commit plan for {}: {} commit{} (propose-only; nothing was staged or committed).\n\ + Groups are whole files. Land each in order from the repo root with \ + `git add -- ` then `git commit -m ''`; those commands go \ + through the normal shell approval gate.\n", + repo_root.display(), + commits.len(), + if commits.len() == 1 { "" } else { "s" } + ); + if index_has_staged_changes { + out.push_str( + "WARNING: the index already holds staged changes. Run `git reset` before \ + staging commit 1, or those hunks will ride into it.\n", + ); + } + for (idx, commit) in commits.iter().enumerate() { + out.push_str(&format!("\n{}. {}\n", idx + 1, commit.message)); + for file in &commit.files { + let hunks = match file.hunks.len() { + 0 if file.untracked => "untracked; listed by path".to_string(), + 0 => "no text hunks (binary or mode change)".to_string(), + 1 => format!("1 hunk: {}", file.hunks[0].header), + count => format!( + "{count} hunks: {}", + file.hunks + .iter() + .map(|hunk| hunk.header.as_str()) + .collect::>() + .join(" ") + ), + }; + let flag = if file.untracked { " (untracked)" } else { "" }; + out.push_str(&format!(" {}{flag} — {hunks}\n", file.path)); + } + if !commit.defines.is_empty() { + out.push_str(&format!(" defines: {}\n", commit.defines.join(", "))); + } + if commit.depends_on.is_empty() { + out.push_str(" depends on: none\n"); + } else { + let deps: Vec = commit + .depends_on + .iter() + .map(|(order, reason)| format!("{order} ({reason})")) + .collect(); + out.push_str(&format!(" depends on: {}\n", deps.join(", "))); + } + } + out } fn is_lock_file(path: &str) -> bool { - let name = Path::new(path).file_name().and_then(|n| n.to_str()).unwrap_or(""); - name.ends_with(".lock") || name == "go.sum" || name == "package-lock.json" || name == "pnpm-lock.yaml" || name == "yarn.lock" + let name = file_name(path); + name.ends_with(".lock") + || matches!( + name, + "go.sum" | "package-lock.json" | "pnpm-lock.yaml" | "yarn.lock" + ) +} + +fn is_manifest_file(path: &str) -> bool { + matches!(file_name(path), "Cargo.toml" | "package.json" | "go.mod") +} + +fn file_name(path: &str) -> &str { + Path::new(path) + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or(path) +} + +fn file_stem(path: &str) -> &str { + Path::new(path) + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or(path) } fn matches_lock_file(manifest: &str, lock: &str) -> bool { let m_path = Path::new(manifest); let l_path = Path::new(lock); - let m_dir = m_path.parent().unwrap_or(Path::new("")); - let l_dir = l_path.parent().unwrap_or(Path::new("")); - if m_dir != l_dir { + if m_path.parent() != l_path.parent() { return false; } - let m_name = m_path.file_name().and_then(|n| n.to_str()).unwrap_or(""); - let l_name = l_path.file_name().and_then(|n| n.to_str()).unwrap_or(""); - match (m_name, l_name) { - ("Cargo.toml", "Cargo.lock") => true, + match (file_name(manifest), file_name(lock)) { + ("Cargo.toml", "Cargo.lock") | ("go.mod", "go.sum") => true, ("package.json", "package-lock.json" | "yarn.lock" | "pnpm-lock.yaml") => true, - ("go.mod", "go.sum") => true, - _ => { - let m_stem = m_path.file_stem().and_then(|s| s.to_str()).unwrap_or(""); - let l_stem = l_path.file_stem().and_then(|s| s.to_str()).unwrap_or(""); - m_stem == l_stem || (m_name.ends_with(".json") && l_name.ends_with(".json")) + (m_name, l_name) => { + file_stem(manifest) == file_stem(lock) + || (m_name.ends_with(".json") && l_name.ends_with(".json")) } } } +/// Lowercased file stem with test/spec markers removed — the name two +/// related files share (`math.rs` and `math_test.rs` both reduce to `math`). +fn related_stem(path: &str) -> String { + file_stem(path) + .to_lowercase() + .replace("_test", "") + .replace("test_", "") + .replace("_spec", "") + .replace("spec_", "") + .replace("test", "") +} + fn are_files_related(f1: &str, f2: &str) -> bool { - let p1 = Path::new(f1); - let p2 = Path::new(f2); - let stem1 = p1.file_stem().and_then(|s| s.to_str()).unwrap_or("").to_lowercase(); - let stem2 = p2.file_stem().and_then(|s| s.to_str()).unwrap_or("").to_lowercase(); + let stem1 = file_stem(f1).to_lowercase(); + let stem2 = file_stem(f2).to_lowercase(); if stem1 == stem2 { return true; } - let clean_stem = |s: &str| { - s.replace("_test", "") - .replace("test_", "") - .replace("_spec", "") - .replace("spec_", "") - .replace("test", "") - }; - clean_stem(&stem1) == clean_stem(&stem2) && !clean_stem(&stem1).is_empty() + let clean1 = related_stem(f1); + !clean1.is_empty() && clean1 == related_stem(f2) } fn is_source_file(path: &str) -> bool { let p = Path::new(path); let ext = p.extension().and_then(|e| e.to_str()).unwrap_or(""); - let name = p.file_name().and_then(|n| n.to_str()).unwrap_or("").to_lowercase(); + let name = file_name(path).to_lowercase(); if name.contains("test") || name.contains("spec") || name.contains("mock") { return false; } - matches!(ext, "rs" | "py" | "go" | "js" | "ts" | "cpp" | "h" | "c" | "java" | "cs" | "rb" | "php") + matches!( + ext, + "rs" | "py" | "go" | "js" | "ts" | "cpp" | "h" | "c" | "java" | "cs" | "rb" | "php" + ) +} + +fn is_doc_file(path: &str) -> bool { + matches!( + Path::new(path).extension().and_then(|e| e.to_str()), + Some("md" | "rst" | "txt" | "adoc") + ) } fn share_context(f1: &str, f2: &str) -> bool { - let p1 = Path::new(f1); - let p2 = Path::new(f2); - p1.parent() == p2.parent() + Path::new(f1).parent() == Path::new(f2).parent() } +const DEFINING_KEYWORDS: &[&str] = &[ + "fn", + "func", + "def", + "function", + "struct", + "enum", + "trait", + "class", + "interface", + "type", + "const", + "let", + "mod", +]; + fn extract_defined_symbols(hunk: &Hunk) -> HashSet { let mut symbols = HashSet::new(); - for line in &hunk.lines { - if line.starts_with('+') && !line.starts_with("+++") { - let content = &line[1..]; - let tokens = tokenize(content); - for i in 0..tokens.len() { - let tok = &tokens[i]; - if tok == "fn" || tok == "func" || tok == "def" || tok == "function" || tok == "struct" || tok == "enum" || tok == "trait" || tok == "class" || tok == "interface" || tok == "type" || tok == "const" || tok == "let" || tok == "mod" { - if i + 1 < tokens.len() { - let sym = &tokens[i + 1]; - if is_valid_identifier(sym) { - symbols.insert(sym.clone()); - } - } - } + for content in added_lines(hunk) { + let tokens = tokenize(content); + for pair in tokens.windows(2) { + if DEFINING_KEYWORDS.contains(&pair[0].as_str()) && is_valid_identifier(&pair[1]) { + symbols.insert(pair[1].clone()); } } } @@ -845,159 +1009,150 @@ fn extract_defined_symbols(hunk: &Hunk) -> HashSet { fn extract_referenced_symbols(hunk: &Hunk) -> HashSet { let mut symbols = HashSet::new(); - for line in &hunk.lines { - if line.starts_with('+') && !line.starts_with("+++") { - let content = &line[1..]; - let tokens = tokenize(content); - for tok in tokens { - if is_valid_identifier(&tok) && !is_keyword(&tok) { - symbols.insert(tok); - } + for content in added_lines(hunk) { + for token in tokenize(content) { + if is_valid_identifier(&token) && !is_keyword(&token) { + symbols.insert(token); } } } symbols } +fn added_lines(hunk: &Hunk) -> impl Iterator { + hunk.lines + .iter() + .filter(|line| line.starts_with('+') && !line.starts_with("+++")) + .map(|line| &line[1..]) +} + fn tokenize(s: &str) -> Vec { - let mut tokens = Vec::new(); - let mut current = String::new(); - for c in s.chars() { - if c.is_alphanumeric() || c == '_' { - current.push(c); - } else { - if !current.is_empty() { - tokens.push(current.clone()); - current.clear(); - } - } - } - if !current.is_empty() { - tokens.push(current); - } - tokens + s.split(|c: char| !(c.is_alphanumeric() || c == '_')) + .filter(|token| !token.is_empty()) + .map(str::to_string) + .collect() } fn is_valid_identifier(s: &str) -> bool { - if s.is_empty() { - return false; - } - let first = s.chars().next().unwrap(); - (first.is_alphabetic() || first == '_') && s.chars().all(|c| c.is_alphanumeric() || c == '_') + let mut chars = s.chars(); + chars + .next() + .is_some_and(|first| first.is_alphabetic() || first == '_') + && chars.all(|c| c.is_alphanumeric() || c == '_') } fn is_keyword(s: &str) -> bool { matches!( s, - "if" | "else" | "while" | "for" | "return" | "import" | "use" | "pub" | "impl" | "crate" - | "self" | "true" | "false" | "let" | "mut" | "match" | "var" | "void" | "int" - | "string" | "bool" | "float" | "double" | "public" | "private" | "protected" - | "static" | "final" | "class" | "fn" | "struct" | "enum" | "trait" | "interface" - | "type" | "const" | "mod" | "def" | "func" | "function" | "and" | "or" | "not" - | "in" | "as" | "break" | "continue" | "new" | "this" | "super" + "if" | "else" + | "while" + | "for" + | "return" + | "import" + | "use" + | "pub" + | "impl" + | "crate" + | "self" + | "true" + | "false" + | "let" + | "mut" + | "match" + | "var" + | "void" + | "int" + | "string" + | "bool" + | "float" + | "double" + | "public" + | "private" + | "protected" + | "static" + | "final" + | "class" + | "fn" + | "struct" + | "enum" + | "trait" + | "interface" + | "type" + | "const" + | "mod" + | "def" + | "func" + | "function" + | "and" + | "or" + | "not" + | "in" + | "as" + | "break" + | "continue" + | "new" + | "this" + | "super" ) } +/// A conventional-commit proposal for one group. The model is expected to +/// refine it; the point is that it names the group's single concern rather +/// than "wip". fn generate_commit_message(group: &CommitGroup) -> String { - let files: Vec<&String> = group.files.keys().collect(); - if files.len() == 1 { - let file = files[0]; - let p = Path::new(file); - let name = p.file_name().and_then(|n| n.to_str()).unwrap_or(file); - if !group.defined_symbols.is_empty() { - let syms: Vec = group.defined_symbols.iter().take(3).cloned().collect(); - format!("refactor({}): define {}", name, syms.join(", ")) - } else { - format!("style/update: changes in {}", name) - } - } else { - if !group.defined_symbols.is_empty() { - let syms: Vec = group.defined_symbols.iter().take(3).cloned().collect(); - format!("feat: implement {}", syms.join(", ")) - } else { - format!("chore: update multiple files including {}", files[0]) - } - } -} - -fn apply_hunks_and_commit( - working_dir: &Path, - hunks: &[Hunk], - commit_message: &str, -) -> Result<(), ToolError> { - let mut patch = String::new(); - let mut files_map: HashMap> = HashMap::new(); - for hunk in hunks { - files_map.entry(hunk.file_path.clone()).or_default().push(hunk); - } - - for (file_path, file_hunks) in files_map { - patch.push_str(&format!("diff --git a/{file_path} b/{file_path}\n")); - patch.push_str(&format!("--- a/{file_path}\n")); - patch.push_str(&format!("+++ b/{file_path}\n")); - for hunk in file_hunks { - patch.push_str(&hunk.header); - patch.push('\n'); - for line in &hunk.lines { - patch.push_str(line); - patch.push('\n'); + let paths: Vec<&str> = group.files.keys().map(String::as_str).collect(); + let scope = match paths.as_slice() { + [single] => file_stem(single).to_string(), + _ => { + // A test or spec rides with the source it names; when every file + // in the group shares that stem, the stem is the subject. An + // unrelated group falls back to its directory name. + let shared = related_stem(paths[0]); + if !shared.is_empty() && paths.iter().all(|path| related_stem(path) == shared) { + shared + } else { + paths + .iter() + .map(|path| Path::new(path).parent()) + .reduce(|a, b| if a == b { a } else { None }) + .flatten() + .and_then(|dir| dir.file_name().and_then(|n| n.to_str())) + .map_or_else(|| "repo".to_string(), str::to_string) } } - } - - let mut child = { - let mut cmd = crate::dependencies::Git::command().ok_or_else(|| { - ToolError::not_available("git is not installed or not in PATH") - })?; - cmd.args(&["-c", "core.quotepath=false", "apply", "--cached", "-"]) - .current_dir(working_dir) - .stdin(std::process::Stdio::piped()) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()); - cmd.spawn().map_err(|e| { - ToolError::execution_failed(format!("Failed to spawn git apply: {e}")) - })? }; - - { - use std::io::Write; - let mut stdin = child.stdin.take().ok_or_else(|| { - ToolError::execution_failed("Failed to open stdin for git apply") - })?; - stdin.write_all(patch.as_bytes()).map_err(|e| { - ToolError::execution_failed(format!("Failed to write to git apply: {e}")) - })?; - } - - let output = child.wait_with_output().map_err(|e| { - ToolError::execution_failed(format!("Failed to wait for git apply: {e}")) - })?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(ToolError::execution_failed(format!( - "git apply --cached failed: {}", - stderr.trim() - ))); - } - - let commit_args = vec![ - "-c".to_string(), - "core.quotepath=false".to_string(), - "commit".to_string(), - "-m".to_string(), - commit_message.to_string(), - ]; - let output = run_git_command(working_dir, &commit_args)?; - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(ToolError::execution_failed(format!( - "git commit failed: {}", - stderr.trim() - ))); + if !group.defined_symbols.is_empty() { + let shown: Vec<&str> = group + .defined_symbols + .iter() + .take(3) + .map(String::as_str) + .collect(); + let more = group.defined_symbols.len().saturating_sub(shown.len()); + let suffix = if more > 0 { + format!(" (+{more} more)") + } else { + String::new() + }; + return format!("feat({scope}): add {}{suffix}", shown.join(", ")); } - - Ok(()) + let names: Vec<&str> = paths.iter().map(|path| file_name(path)).collect(); + let kind = if paths + .iter() + .all(|path| is_lock_file(path) || is_manifest_file(path)) + { + return format!("chore(deps): update {}", names.join(", ")); + } else if paths.iter().all(|path| is_doc_file(path)) { + "docs" + } else if paths + .iter() + .all(|path| !is_source_file(path) && file_name(path).to_lowercase().contains("test")) + { + "test" + } else { + "chore" + }; + format!("{kind}({scope}): update {}", names.join(", ")) } #[cfg(test)] @@ -1195,6 +1350,8 @@ mod tests { assert!(truncated.contains("output truncated")); } + // === Commit plan (#3999) === + #[test] fn test_parse_diff() { let diff = r#"diff --git a/src/lib.rs b/src/lib.rs @@ -1207,22 +1364,26 @@ index e69de29..4b2a8d3 100644 +line2 modified line3 +line4 added +diff --git a/image.png b/image.png +index 1111111..2222222 100644 +Binary files a/image.png and b/image.png differ "#; - let hunks = parse_diff(diff); + let files = parse_diff(diff); + assert_eq!(files.len(), 2); + let hunks = &files[0].hunks; assert_eq!(hunks.len(), 1); assert_eq!(hunks[0].file_path, "src/lib.rs"); - assert_eq!(hunks[0].old_range, (1, 3)); - assert_eq!(hunks[0].new_range, (1, 4)); assert_eq!(hunks[0].header, "@@ -1,3 +1,4 @@"); assert_eq!(hunks[0].lines.len(), 5); + // A binary change has no hunks but must not vanish from the plan. + assert_eq!(files[1].path, "image.png"); + assert!(files[1].hunks.is_empty()); } #[test] fn test_dependency_extraction() { let hunk = Hunk { file_path: "src/lib.rs".to_string(), - old_range: (1, 1), - new_range: (1, 2), header: "@@ -1 +1,2 @@".to_string(), lines: vec![ " pub fn add(a: i32, b: i32) -> i32 {".to_string(), @@ -1240,8 +1401,101 @@ index e69de29..4b2a8d3 100644 assert!(referenced.contains("Answer")); } + fn text_file(path: &str, added: &[&str]) -> ChangedFile { + ChangedFile { + path: path.to_string(), + hunks: vec![Hunk { + file_path: path.to_string(), + header: format!("@@ -1 +1,{} @@", added.len()), + lines: added.iter().map(|line| format!("+{line}")).collect(), + }], + untracked: false, + } + } + + fn paths(commit: &PlannedCommit) -> Vec<&str> { + commit.files.iter().map(|f| f.path.as_str()).collect() + } + + #[test] + fn plan_orders_definition_before_use_and_tests_after_source() { + let commits = plan_commits(vec![ + text_file("src/main.rs", &["fn main() { let y = math::sub(3, 4); }"]), + text_file( + "src/math.rs", + &["pub fn sub(a: i32, b: i32) -> i32 { a - b }"], + ), + text_file("src/math_test.rs", &["#[test] fn sub_works() {}"]), + text_file("docs/notes.md", &["Some notes"]), + ]) + .expect("acyclic"); + + assert_eq!(commits.len(), 3, "{commits:#?}"); + // The test rides with the source file it names. + assert_eq!(paths(&commits[0]), vec!["src/math.rs", "src/math_test.rs"]); + assert!(commits[0].depends_on.is_empty()); + assert_eq!(paths(&commits[1]), vec!["src/main.rs"]); + assert_eq!(commits[1].depends_on, vec![(1, "uses `sub`".to_string())]); + assert_eq!(paths(&commits[2]), vec!["docs/notes.md"]); + assert!( + commits[0].message.starts_with("feat(math): add sub"), + "{}", + commits[0].message + ); + assert_eq!(commits[2].message, "docs(notes): update notes.md"); + } + + #[test] + fn plan_rejects_cycles_with_a_diagnostic() { + let cycle = plan_commits(vec![ + text_file("a.rs", &["pub fn func_a2() { b::func_b2(); }"]), + text_file("b.rs", &["pub fn func_b2() { a::func_a2(); }"]), + ]) + .expect_err("cycle"); + assert_eq!(cycle.files, vec!["a.rs", "b.rs"]); + assert!( + cycle + .edges + .iter() + .any(|edge| edge.contains("uses `func_b2`")), + "{:?}", + cycle.edges + ); + } + + #[test] + fn lock_file_rides_with_its_manifest_and_stays_out_of_analysis() { + let mut lock = text_file("Cargo.lock", &["name = \"serde\"", "fn sub() {}"]); + lock.hunks[0].file_path = "Cargo.lock".to_string(); + let commits = plan_commits(vec![ + text_file("Cargo.toml", &["serde = \"1\""]), + lock, + text_file("src/math.rs", &["pub fn sub() {}"]), + ]) + .expect("acyclic"); + assert_eq!(commits.len(), 2, "{commits:#?}"); + let deps = commits + .iter() + .find(|c| paths(c).contains(&"Cargo.lock")) + .expect("lock group"); + assert_eq!(paths(deps), vec!["Cargo.lock", "Cargo.toml"]); + assert_eq!(deps.message, "chore(deps): update Cargo.lock, Cargo.toml"); + // The lock file's tokens never create a dependency edge. + assert!( + commits.iter().all(|c| c.depends_on.is_empty()), + "{commits:#?}" + ); + } + + fn git_out(root: &Path, args: &[&str]) -> String { + let args: Vec = args.iter().map(|s| s.to_string()).collect(); + let output = run_git_command(root, &args).expect("git"); + assert!(output.status.success(), "git {args:?} failed"); + String::from_utf8_lossy(&output.stdout).to_string() + } + #[tokio::test] - async fn test_git_commit_split_success() { + async fn commit_plan_proposes_without_touching_the_index() { if !git_available() { return; } @@ -1250,43 +1504,82 @@ index e69de29..4b2a8d3 100644 let math_file = tmp.path().join("math.rs"); let main_file = tmp.path().join("main.rs"); - - fs::write(&math_file, "pub fn add(a: i32, b: i32) -> i32 { a + b }\n").expect("write math"); - fs::write(&main_file, "fn main() { let x = math::add(1, 2); }\n").expect("write main"); - + fs::write(&math_file, "pub fn add(a: i32, b: i32) -> i32 { a + b }\n").expect("write"); + fs::write(&main_file, "fn main() { let x = math::add(1, 2); }\n").expect("write"); commit_all(tmp.path(), "init"); - fs::write(&math_file, "pub fn add(a: i32, b: i32) -> i32 { a + b }\npub fn sub(a: i32, b: i32) -> i32 { a - b }\n").expect("modify math"); - fs::write(&main_file, "fn main() { let x = math::add(1, 2); let y = math::sub(3, 4); }\n").expect("modify main"); + fs::write( + &math_file, + "pub fn add(a: i32, b: i32) -> i32 { a + b }\npub fn sub(a: i32, b: i32) -> i32 { a - b }\n", + ) + .expect("modify"); + fs::write( + &main_file, + "fn main() { let x = math::add(1, 2); let y = math::sub(3, 4); }\n", + ) + .expect("modify"); + fs::write(tmp.path().join("NOTES.md"), "untracked notes\n").expect("write"); let ctx = ToolContext::new(tmp.path()); - let tool = GitCommitSplitTool; - - let result = tool.execute(json!({ "dry_run": true }), &ctx).await.expect("execute"); - assert!(result.success); - let val: Value = serde_json::from_str(&result.content).expect("parse response"); - let commits = val.as_array().expect("array of commits"); - assert_eq!(commits.len(), 2); - - let first_commit = &commits[0]; - let first_files = first_commit.get("files").unwrap().as_array().unwrap(); - assert!(first_files.iter().any(|f| f.as_str().unwrap().contains("math.rs"))); - - let second_commit = &commits[1]; - let second_files = second_commit.get("files").unwrap().as_array().unwrap(); - assert!(second_files.iter().any(|f| f.as_str().unwrap().contains("main.rs"))); - - let run_result = tool.execute(json!({ "dry_run": false }), &ctx).await.expect("execute"); - assert!(run_result.success); + let result = GitCommitPlanTool + .execute(json!({}), &ctx) + .await + .expect("execute"); + assert!(result.success, "{}", result.content); + assert!( + result.content.contains("propose-only"), + "{}", + result.content + ); + let metadata = result.metadata.expect("metadata"); + let commits = metadata["commits"].as_array().expect("commits"); + assert_eq!(commits.len(), 3, "{}", result.content); + assert_eq!(commits[0]["files"], json!(["math.rs"])); + assert_eq!(commits[1]["files"], json!(["main.rs"])); + assert_eq!(commits[1]["depends_on"][0]["order"], json!(1)); + assert_eq!(commits[2]["untracked"], json!(["NOTES.md"])); + assert_eq!(metadata["index_has_staged_changes"], json!(false)); + + // Nothing was staged, intent-added, or committed. + assert_eq!( + git_out(tmp.path(), &["diff", "--cached", "--name-only"]), + "" + ); + assert_eq!( + git_out(tmp.path(), &["rev-list", "--count", "HEAD"]).trim(), + "1" + ); + assert_eq!( + git_out(tmp.path(), &["ls-files", "--others", "--exclude-standard"]).trim(), + "NOTES.md" + ); - let log_output = run_git_command(tmp.path(), &["log".to_string(), "--oneline".to_string()]).expect("git log"); - let log_stdout = String::from_utf8_lossy(&log_output.stdout); - let lines: Vec<&str> = log_stdout.lines().collect(); - assert_eq!(lines.len(), 3); + // The plan lands through the ordinary write path, in order. + for commit in commits { + let mut add = vec!["add", "--"]; + let files: Vec = commit["files"] + .as_array() + .unwrap() + .iter() + .chain(commit["untracked"].as_array().unwrap()) + .map(|f| f.as_str().unwrap().to_string()) + .collect(); + add.extend(files.iter().map(String::as_str)); + git_out(tmp.path(), &add); + git_out( + tmp.path(), + &["commit", "-q", "-m", commit["message"].as_str().unwrap()], + ); + } + assert_eq!( + git_out(tmp.path(), &["rev-list", "--count", "HEAD"]).trim(), + "4" + ); + assert_eq!(git_out(tmp.path(), &["status", "--porcelain"]), ""); } #[tokio::test] - async fn test_git_commit_split_cycle() { + async fn commit_plan_rejects_cycles_and_writes_nothing() { if !git_available() { return; } @@ -1295,20 +1588,103 @@ index e69de29..4b2a8d3 100644 let a_file = tmp.path().join("a.rs"); let b_file = tmp.path().join("b.rs"); - fs::write(&a_file, "pub fn func_a() {}\n").expect("write a"); fs::write(&b_file, "pub fn func_b() {}\n").expect("write b"); - commit_all(tmp.path(), "init"); - fs::write(&a_file, "pub fn func_a() {}\npub fn func_a2() { b::func_b2(); }\n").expect("modify a"); - fs::write(&b_file, "pub fn func_b() {}\npub fn func_b2() { a::func_a2(); }\n").expect("modify b"); + fs::write( + &a_file, + "pub fn func_a() {}\npub fn func_a2() { b::func_b2(); }\n", + ) + .expect("modify a"); + fs::write( + &b_file, + "pub fn func_b() {}\npub fn func_b2() { a::func_a2(); }\n", + ) + .expect("modify b"); let ctx = ToolContext::new(tmp.path()); - let tool = GitCommitSplitTool; - - let result = tool.execute(json!({ "dry_run": true }), &ctx).await.expect("execute"); + let result = GitCommitPlanTool + .execute(json!({}), &ctx) + .await + .expect("execute"); assert!(!result.success); - assert!(result.content.contains("Dependency cycle detected")); + assert!( + result.content.contains("Dependency cycle detected"), + "{}", + result.content + ); + assert!( + result.content.contains("nothing was written"), + "{}", + result.content + ); + assert_eq!(result.metadata.unwrap()["cycle_detected"], json!(true)); + assert_eq!( + git_out(tmp.path(), &["diff", "--cached", "--name-only"]), + "" + ); + assert_eq!( + git_out(tmp.path(), &["rev-list", "--count", "HEAD"]).trim(), + "1" + ); } -} \ No newline at end of file + + #[tokio::test] + async fn commit_plan_warns_when_the_index_already_holds_changes() { + if !git_available() { + return; + } + let tmp = tempdir().expect("tempdir"); + init_git_repo(tmp.path()); + let file = tmp.path().join("a.rs"); + fs::write(&file, "pub fn a() {}\n").expect("write"); + commit_all(tmp.path(), "init"); + fs::write(&file, "pub fn a() {}\npub fn a2() {}\n").expect("modify"); + git_out(tmp.path(), &["add", "a.rs"]); + + let ctx = ToolContext::new(tmp.path()); + let result = GitCommitPlanTool + .execute(json!({}), &ctx) + .await + .expect("execute"); + assert!(result.success, "{}", result.content); + assert!( + result + .content + .contains("WARNING: the index already holds staged changes") + ); + assert_eq!( + result.metadata.unwrap()["index_has_staged_changes"], + json!(true) + ); + // Still staged exactly as the user left it. + assert_eq!( + git_out(tmp.path(), &["diff", "--cached", "--name-only"]).trim(), + "a.rs" + ); + } + + #[tokio::test] + async fn commit_plan_reports_a_clean_tree() { + if !git_available() { + return; + } + let tmp = tempdir().expect("tempdir"); + init_git_repo(tmp.path()); + fs::write(tmp.path().join("a.rs"), "pub fn a() {}\n").expect("write"); + commit_all(tmp.path(), "init"); + + let ctx = ToolContext::new(tmp.path()); + let result = GitCommitPlanTool + .execute(json!({}), &ctx) + .await + .expect("execute"); + assert!(result.success); + assert!( + result.content.contains("No changes to plan"), + "{}", + result.content + ); + } +} diff --git a/crates/tui/src/tools/git_tool.rs b/crates/tui/src/tools/git_tool.rs index 975034e1fd..f093ab3e99 100644 --- a/crates/tui/src/tools/git_tool.rs +++ b/crates/tui/src/tools/git_tool.rs @@ -1,14 +1,16 @@ //! Canonical action-based wrapper for git inspection tools. //! //! The model sees one tool: `Git` with an `action` parameter -//! (status | diff | log | show | blame). The per-action legacy execution -//! aliases were removed in v0.9.3. +//! (status | diff | log | show | blame | commit_plan). The per-action legacy +//! execution aliases were removed in v0.9.3. `commit_plan` (#3999) is the +//! propose-only atomic-commit planner: it returns a split plan and writes +//! nothing, so the family stays read-only end to end. use async_trait::async_trait; use serde_json::{Value, json}; use super::canonical_action::required_action; -use super::git::{GitDiffTool, GitStatusTool}; +use super::git::{GitCommitPlanTool, GitDiffTool, GitStatusTool}; use super::git_history::{GitBlameTool, GitLogTool, GitShowTool}; use super::spec::{ ApprovalRequirement, ToolCapability, ToolContext, ToolError, ToolResult, ToolSpec, @@ -27,7 +29,8 @@ impl GitTool { } } - const ACTIONS: &'static [&'static str] = &["status", "diff", "log", "show", "blame"]; + const ACTIONS: &'static [&'static str] = + &["status", "diff", "log", "show", "blame", "commit_plan"]; fn required_action(&self, input: &Value) -> Result { if let Some(forced) = self.forced_action { @@ -60,7 +63,7 @@ impl ToolSpec for GitTool { } fn description(&self) -> &'static str { - "Inspect repository state and history with status, diff, log, show, or blame. All actions are read-only and parallel-safe." + "Inspect repository state and history with status, diff, log, show, or blame; commit_plan proposes an ordered atomic-commit split of the working tree. All actions are read-only and parallel-safe." } fn input_schema(&self) -> Value { @@ -69,8 +72,8 @@ impl ToolSpec for GitTool { "properties": { "action": { "type": "string", - "enum": ["status", "diff", "log", "show", "blame"], - "description": "Action to perform" + "enum": ["status", "diff", "log", "show", "blame", "commit_plan"], + "description": "Action to perform. commit_plan returns a proposed split of the working tree into dependency-ordered commits (rejecting cycles) and writes nothing; land each group with git add/commit." }, "path": { "type": "string", @@ -159,6 +162,7 @@ impl ToolSpec for GitTool { "log" => GitLogTool.execute(input, context).await, "show" => GitShowTool.execute(input, context).await, "blame" => GitBlameTool.execute(input, context).await, + "commit_plan" => GitCommitPlanTool.execute(input, context).await, other => Err(ToolError::invalid_input(format!( "Unknown Git action \"{other}\"; nothing was run. Pass one of: {}.", Self::ACTIONS.join(", ") @@ -196,11 +200,22 @@ mod tests { let message = err(json!({"action": "commit"})).await; assert!(message.contains("commit"), "{message}"); assert!( - message.contains("status, diff, log, show, blame"), + message.contains("status, diff, log, show, blame, commit_plan"), "{message}" ); } + /// `commit_plan` proposes and never writes, so the envelope must class it + /// with the other read-only Git actions rather than as a mutation (#3999). + #[test] + fn commit_plan_is_bounded_read_only_for_the_envelope() { + use crate::tools::execution_envelope::{CallClass, classify_call}; + let tool = GitTool::new("Git"); + let input = json!({"action": "commit_plan"}); + assert!(tool.is_read_only_for(&input)); + assert_eq!(classify_call("Git", &input, &tool), CallClass::Bounded); + } + #[test] fn advertised_actions_match_the_actions_that_dispatch() { let schema = GitTool::new("Git").input_schema(); diff --git a/crates/tui/src/tui/approval/policy.rs b/crates/tui/src/tui/approval/policy.rs index 5ed8cdb878..c6096d80b0 100644 --- a/crates/tui/src/tui/approval/policy.rs +++ b/crates/tui/src/tui/approval/policy.rs @@ -120,6 +120,7 @@ pub fn get_tool_category(name: &str) -> ToolCategory { | "git_log" | "git_show" | "git_blame" + | "git_commit_plan" | "project" | "diagnostics" ) || name.starts_with("read_") @@ -386,6 +387,7 @@ mod tests { ("Git", "log", ToolCategory::Safe, RiskLevel::Benign), ("Git", "show", ToolCategory::Safe, RiskLevel::Benign), ("Git", "blame", ToolCategory::Safe, RiskLevel::Benign), + ("Git", "commit_plan", ToolCategory::Safe, RiskLevel::Benign), ( "Run", "tests", diff --git a/crates/tui/src/tui/history/tool_run.rs b/crates/tui/src/tui/history/tool_run.rs index 93f1ce0045..08b2c84a87 100644 --- a/crates/tui/src/tui/history/tool_run.rs +++ b/crates/tui/src/tui/history/tool_run.rs @@ -187,7 +187,7 @@ fn classify_tool_name_activity(name: &str) -> ToolRunActivity { let normalized = name.trim().to_ascii_lowercase(); match normalized.as_str() { "read_file" | "list_dir" | "view_image" | "explore" | "git_status" | "git_diff" - | "git_log" | "git_show" | "git_blame" => ToolRunActivity::File, + | "git_log" | "git_show" | "git_blame" | "git_commit_plan" => ToolRunActivity::File, "grep_files" | "file_search" | "web_search" | "fetch_url" | "registry_sync" => { ToolRunActivity::Search } @@ -369,7 +369,7 @@ mod tests { #[test] fn normalized_git_and_run_actions_keep_truthful_activity_buckets() { - for action in ["status", "diff", "log", "show", "blame"] { + for action in ["status", "diff", "log", "show", "blame", "commit_plan"] { let input = json!({"action": action}); assert_eq!( classify_tool_name_activity(canonical_action_alias("Git", &input)), diff --git a/crates/tui/src/tui/widgets/tool_card.rs b/crates/tui/src/tui/widgets/tool_card.rs index 06fe099f17..efbd348dfe 100644 --- a/crates/tui/src/tui/widgets/tool_card.rs +++ b/crates/tui/src/tui/widgets/tool_card.rs @@ -80,7 +80,7 @@ pub fn tool_family_for_title(title: &str) -> ToolFamily { pub fn tool_family_for_name(name: &str) -> ToolFamily { match name { "read_file" | "list_dir" | "view_image" | "git_status" | "git_diff" | "git_log" - | "git_show" | "git_blame" => ToolFamily::Read, + | "git_show" | "git_blame" | "git_commit_plan" => ToolFamily::Read, "edit_file" | "apply_patch" | "write_file" => ToolFamily::Patch, "exec_shell" | "exec_shell_wait" @@ -403,6 +403,7 @@ mod tests { ("Git", "log", ToolFamily::Read), ("Git", "show", ToolFamily::Read), ("Git", "blame", ToolFamily::Read), + ("Git", "commit_plan", ToolFamily::Read), ("Run", "tests", ToolFamily::Verify), ("Run", "verifiers", ToolFamily::Verify), ("Web", "search", ToolFamily::Find), From d7799eb7b88abe0d1ea4793a575a2ee8b5eaee0b Mon Sep 17 00:00:00 2001 From: CodeWhale Bot Date: Sun, 6 Sep 2026 22:02:21 -0700 Subject: [PATCH 3/4] chore(web): regenerate facts for the new git_commit_plan tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `git_commit_plan` (#5870, fixes #3999) is the 76th model-visible tool, so the committed `web/lib/facts.generated.ts` went stale at toolCount 75 and `npm run check:facts` failed the Lint & Type Check gate. Regenerated with `cd web && npm run prebuild`; only the facts file is committed. `changelog.generated.ts` also moves under that script, but its drift comes from CHANGELOG entries merged in from main and is unrelated to this PR, so it is left alone. Gate: `npm run check:facts` → OK, committed facts.generated.ts matches workspace. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01D4rk4NXwyy6wmvii9Lp84P Signed-off-by: CodeWhale Bot --- web/lib/facts.generated.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/lib/facts.generated.ts b/web/lib/facts.generated.ts index 37afd23507..fe2c673b10 100644 --- a/web/lib/facts.generated.ts +++ b/web/lib/facts.generated.ts @@ -27,7 +27,7 @@ export interface RepoFacts { } export const FACTS: RepoFacts = { - "generatedAt": "2026-09-06T05:58:50.132Z", + "generatedAt": "2026-09-07T05:01:30.152Z", "sourceRevision": null, "sourceCommittedAt": null, "version": "0.9.12", @@ -297,7 +297,7 @@ export const FACTS: RepoFacts = { ], "defaultModel": "deepseek-v4-pro", "nodeEngines": ">=18", - "toolCount": 75, + "toolCount": 76, "license": "MIT", "latestPublishedRelease": { "tag": "v0.9.12", From 7da6064e88fe41c6cc87463a0245c55d5db9a334 Mon Sep 17 00:00:00 2001 From: CodeWhale Bot Date: Sun, 6 Sep 2026 22:14:32 -0700 Subject: [PATCH 4/4] chore(docs): align public-surface sourceCandidate toolCount with the new tool `git_commit_plan` (#5870, fixes #3999) is the 76th model-visible tool. `facts.generated.ts` was regenerated in the parent commit, but `docs/public-surface-facts.json` carries a second, hand-maintained `sourceCandidate.toolCount` that the web suite pins against it, so `public-surface-contract.test.ts` failed with `expected 75 to be 76`. Bumped that one field; no other value in the matrix changes. Gates run locally in the Lint & Type Check job's own order: - npm run check:facts -> OK (committed facts match workspace) - npm run prebuild -> tools=76 - npm test -> 47 files / 407 tests passed, 0 failed - npm run lint -> 0 errors (2 pre-existing next/image warnings) - npx tsc --noEmit -> clean Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01D4rk4NXwyy6wmvii9Lp84P Signed-off-by: CodeWhale Bot --- docs/public-surface-facts.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/public-surface-facts.json b/docs/public-surface-facts.json index 4ff69769b0..5354b9a902 100644 --- a/docs/public-surface-facts.json +++ b/docs/public-surface-facts.json @@ -22,7 +22,7 @@ "sourceCandidate": { "version": "0.9.12", "providerCount": 47, - "toolCount": 75, + "toolCount": 76, "sandboxBackends": [ "seatbelt (macOS, when available)", "bubblewrap (Linux, opt-in when installed)"