From 4834dfd28029e0c2c9bcae9156a4badc9d32505b Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Wed, 8 Apr 2026 14:57:46 -0700 Subject: [PATCH] feat: add ecc2 worktree merge actions --- ecc2/src/main.rs | 89 +++++++++++++++++++ ecc2/src/session/manager.rs | 122 ++++++++++++++++++++++++-- ecc2/src/tui/app.rs | 1 + ecc2/src/tui/dashboard.rs | 152 ++++++++++++++++++++++++++++++-- ecc2/src/worktree/mod.rs | 167 +++++++++++++++++++++++++++++++++++- 5 files changed, 513 insertions(+), 18 deletions(-) diff --git a/ecc2/src/main.rs b/ecc2/src/main.rs index 2e73f992..008b2094 100644 --- a/ecc2/src/main.rs +++ b/ecc2/src/main.rs @@ -203,6 +203,17 @@ enum Commands { #[arg(long)] check: bool, }, + /// Merge a session worktree branch into its base branch + MergeWorktree { + /// Session ID or alias + session_id: Option, + /// Emit machine-readable JSON instead of the human summary + #[arg(long)] + json: bool, + /// Keep the worktree attached after a successful merge + #[arg(long)] + keep_worktree: bool, + }, /// Prune worktrees for inactive sessions and report any active sessions still holding one PruneWorktrees { /// Emit machine-readable JSON instead of the human summary @@ -690,6 +701,21 @@ async fn main() -> Result<()> { std::process::exit(worktree_status_reports_exit_code(&reports)); } } + Some(Commands::MergeWorktree { + session_id, + json, + keep_worktree, + }) => { + let id = session_id.unwrap_or_else(|| "latest".to_string()); + let resolved_id = resolve_session_id(&db, &id)?; + let outcome = + session::manager::merge_session_worktree(&db, &resolved_id, !keep_worktree).await?; + if json { + println!("{}", serde_json::to_string_pretty(&outcome)?); + } else { + println!("{}", format_worktree_merge_human(&outcome)); + } + } Some(Commands::PruneWorktrees { json }) => { let outcome = session::manager::prune_inactive_worktrees(&db).await?; if json { @@ -1054,6 +1080,28 @@ fn format_worktree_status_reports_human(reports: &[WorktreeStatusReport]) -> Str .join("\n\n") } +fn format_worktree_merge_human(outcome: &session::manager::WorktreeMergeOutcome) -> String { + let mut lines = vec![format!( + "Merged worktree for {}", + short_session(&outcome.session_id) + )]; + lines.push(format!( + "Branch {} -> {}", + outcome.branch, outcome.base_branch + )); + lines.push(if outcome.already_up_to_date { + "Result already up to date".to_string() + } else { + "Result merged into base".to_string() + }); + lines.push(if outcome.cleaned_worktree { + "Cleanup removed worktree and branch".to_string() + } else { + "Cleanup kept worktree attached".to_string() + }); + lines.join("\n") +} + fn worktree_status_exit_code(report: &WorktreeStatusReport) -> i32 { report.check_exit_code } @@ -1513,6 +1561,31 @@ mod tests { } } + #[test] + fn cli_parses_merge_worktree_flags() { + let cli = Cli::try_parse_from([ + "ecc", + "merge-worktree", + "deadbeef", + "--json", + "--keep-worktree", + ]) + .expect("merge-worktree flags should parse"); + + match cli.command { + Some(Commands::MergeWorktree { + session_id, + json, + keep_worktree, + }) => { + assert_eq!(session_id.as_deref(), Some("deadbeef")); + assert!(json); + assert!(keep_worktree); + } + _ => panic!("expected merge-worktree subcommand"), + } + } + #[test] fn format_worktree_status_human_includes_readiness_and_conflicts() { let report = WorktreeStatusReport { @@ -1560,6 +1633,22 @@ mod tests { assert!(text.contains("- active facefeed")); } + #[test] + fn format_worktree_merge_human_reports_merge_and_cleanup() { + let text = format_worktree_merge_human(&session::manager::WorktreeMergeOutcome { + session_id: "deadbeefcafefeed".to_string(), + branch: "ecc/deadbeef".to_string(), + base_branch: "main".to_string(), + already_up_to_date: false, + cleaned_worktree: true, + }); + + assert!(text.contains("Merged worktree for deadbeef")); + assert!(text.contains("Branch ecc/deadbeef -> main")); + assert!(text.contains("Result merged into base")); + assert!(text.contains("Cleanup removed worktree and branch")); + } + #[test] fn format_worktree_status_human_handles_missing_worktree() { let report = WorktreeStatusReport { diff --git a/ecc2/src/session/manager.rs b/ecc2/src/session/manager.rs index 4ec104a6..24f2ab0b 100644 --- a/ecc2/src/session/manager.rs +++ b/ecc2/src/session/manager.rs @@ -603,13 +603,57 @@ pub async fn cleanup_session_worktree(db: &StateStore, id: &str) -> Result<()> { } if let Some(worktree) = session.worktree.as_ref() { - crate::worktree::remove(&worktree.path)?; + crate::worktree::remove(worktree)?; db.clear_worktree(&session.id)?; } Ok(()) } +#[derive(Debug, Clone, Serialize)] +pub struct WorktreeMergeOutcome { + pub session_id: String, + pub branch: String, + pub base_branch: String, + pub already_up_to_date: bool, + pub cleaned_worktree: bool, +} + +pub async fn merge_session_worktree( + db: &StateStore, + id: &str, + cleanup_worktree: bool, +) -> Result { + let session = resolve_session(db, id)?; + + if matches!(session.state, SessionState::Pending | SessionState::Running) { + anyhow::bail!( + "Cannot merge active session {} while it is {}", + session.id, + session.state + ); + } + + let worktree = session + .worktree + .clone() + .ok_or_else(|| anyhow::anyhow!("Session {} has no attached worktree", session.id))?; + let outcome = crate::worktree::merge_into_base(&worktree)?; + + if cleanup_worktree { + crate::worktree::remove(&worktree)?; + db.clear_worktree(&session.id)?; + } + + Ok(WorktreeMergeOutcome { + session_id: session.id, + branch: outcome.branch, + base_branch: outcome.base_branch, + already_up_to_date: outcome.already_up_to_date, + cleaned_worktree: cleanup_worktree, + }) +} + #[derive(Debug, Clone, Serialize)] pub struct WorktreePruneOutcome { pub cleaned_session_ids: Vec, @@ -659,7 +703,7 @@ pub async fn delete_session(db: &StateStore, id: &str) -> Result<()> { } if let Some(worktree) = session.worktree.as_ref() { - let _ = crate::worktree::remove(&worktree.path); + let _ = crate::worktree::remove(worktree); } db.delete_session(&session.id)?; @@ -758,7 +802,7 @@ async fn queue_session_in_dir_with_runner_program( db.update_state(&session.id, &SessionState::Failed)?; if let Some(worktree) = session.worktree.as_ref() { - let _ = crate::worktree::remove(&worktree.path); + let _ = crate::worktree::remove(worktree); } Err(error.context(format!("Failed to queue session {}", session.id))) @@ -829,7 +873,7 @@ async fn create_session_in_dir( db.update_state(&session.id, &SessionState::Failed)?; if let Some(worktree) = session.worktree.as_ref() { - let _ = crate::worktree::remove(&worktree.path); + let _ = crate::worktree::remove(worktree); } Err(error.context(format!("Failed to start session {}", session.id))) @@ -1029,7 +1073,7 @@ async fn stop_session_with_options( if cleanup_worktree { if let Some(worktree) = session.worktree.as_ref() { - crate::worktree::remove(&worktree.path)?; + crate::worktree::remove(worktree)?; } } @@ -1525,15 +1569,13 @@ mod tests { fn init_git_repo(path: &Path) -> Result<()> { fs::create_dir_all(path)?; run_git(path, ["init", "-q"])?; + run_git(path, ["config", "user.name", "ECC Tests"])?; + run_git(path, ["config", "user.email", "ecc-tests@example.com"])?; fs::write(path.join("README.md"), "hello\n")?; run_git(path, ["add", "README.md"])?; run_git( path, [ - "-c", - "user.name=ECC Tests", - "-c", - "user.email=ecc-tests@example.com", "commit", "-qm", "init", @@ -1856,6 +1898,68 @@ mod tests { Ok(()) } + #[tokio::test(flavor = "current_thread")] + async fn merge_session_worktree_merges_branch_and_cleans_worktree() -> Result<()> { + let tempdir = TestDir::new("manager-merge-worktree")?; + let repo_root = tempdir.path().join("repo"); + init_git_repo(&repo_root)?; + + let cfg = build_config(tempdir.path()); + let db = StateStore::open(&cfg.db_path)?; + let (fake_claude, _) = write_fake_claude(tempdir.path())?; + + let session_id = create_session_in_dir( + &db, + &cfg, + "merge later", + "claude", + true, + &repo_root, + &fake_claude, + ) + .await?; + + stop_session_with_options(&db, &session_id, false).await?; + let stopped = db + .get_session(&session_id)? + .context("stopped session should exist")?; + let worktree = stopped + .worktree + .clone() + .context("stopped session worktree missing")?; + + fs::write(worktree.path.join("feature.txt"), "ready to merge\n")?; + run_git(&worktree.path, ["add", "feature.txt"])?; + run_git(&worktree.path, ["commit", "-qm", "feature work"])?; + + let outcome = merge_session_worktree(&db, &session_id, true).await?; + + assert_eq!(outcome.session_id, session_id); + assert_eq!(outcome.branch, worktree.branch); + assert_eq!(outcome.base_branch, worktree.base_branch); + assert!(outcome.cleaned_worktree); + assert!(!outcome.already_up_to_date); + assert_eq!(fs::read_to_string(repo_root.join("feature.txt"))?, "ready to merge\n"); + + let merged = db + .get_session(&outcome.session_id)? + .context("merged session should still exist")?; + assert!(merged.worktree.is_none(), "worktree metadata should be cleared"); + assert!(!worktree.path.exists(), "worktree path should be removed"); + + let branch_output = StdCommand::new("git") + .arg("-C") + .arg(&repo_root) + .args(["branch", "--list", &worktree.branch]) + .output()?; + assert!( + String::from_utf8_lossy(&branch_output.stdout).trim().is_empty(), + "merged worktree branch should be deleted" + ); + + Ok(()) + } + #[tokio::test(flavor = "current_thread")] async fn delete_session_removes_inactive_session_and_worktree() -> Result<()> { let tempdir = TestDir::new("manager-delete-session")?; diff --git a/ecc2/src/tui/app.rs b/ecc2/src/tui/app.rs index aaad30b2..de9e1c7c 100644 --- a/ecc2/src/tui/app.rs +++ b/ecc2/src/tui/app.rs @@ -46,6 +46,7 @@ pub async fn run(db: StateStore, cfg: Config) -> Result<()> { (_, KeyCode::Char('g')) => dashboard.auto_dispatch_backlog().await, (_, KeyCode::Char('G')) => dashboard.coordinate_backlog().await, (_, KeyCode::Char('v')) => dashboard.toggle_output_mode(), + (_, KeyCode::Char('m')) => dashboard.merge_selected_worktree().await, (_, KeyCode::Char('p')) => dashboard.toggle_auto_dispatch_policy(), (_, KeyCode::Char(',')) => dashboard.adjust_auto_dispatch_limit(-1), (_, KeyCode::Char('.')) => dashboard.adjust_auto_dispatch_limit(1), diff --git a/ecc2/src/tui/dashboard.rs b/ecc2/src/tui/dashboard.rs index f9661d00..3fda8541 100644 --- a/ecc2/src/tui/dashboard.rs +++ b/ecc2/src/tui/dashboard.rs @@ -21,8 +21,6 @@ use crate::worktree; use crate::session::output::OutputStream; #[cfg(test)] use crate::session::{SessionMetrics, WorktreeInfo}; -#[cfg(test)] -use std::path::Path; const DEFAULT_PANE_SIZE_PERCENT: u16 = 35; const DEFAULT_GRID_SIZE_PERCENT: u16 = 50; @@ -516,6 +514,7 @@ impl Dashboard { " g Auto-dispatch unread handoffs across lead sessions", " G Dispatch then rebalance backlog across lead teams", " v Toggle selected worktree diff in output pane", + " m Merge selected ready worktree into base and clean it up", " p Toggle daemon auto-dispatch policy and persist config", " ,/. Decrease/increase auto-dispatch limit per lead", " s Stop selected session", @@ -1115,6 +1114,43 @@ impl Dashboard { )); } + pub async fn merge_selected_worktree(&mut self) { + let Some(session) = self.sessions.get(self.selected_session) else { + return; + }; + + if session.worktree.is_none() { + self.set_operator_note("selected session has no worktree to merge".to_string()); + return; + } + + let session_id = session.id.clone(); + let outcome = match manager::merge_session_worktree(&self.db, &session_id, true).await { + Ok(outcome) => outcome, + Err(error) => { + tracing::warn!("Failed to merge session {} worktree: {error}", session.id); + self.set_operator_note(format!( + "merge failed for {}: {error}", + format_session_id(&session_id) + )); + return; + } + }; + + self.refresh(); + self.set_operator_note(format!( + "merged {} into {} for {}{}", + outcome.branch, + outcome.base_branch, + format_session_id(&session_id), + if outcome.already_up_to_date { + " (already up to date)" + } else { + "" + } + )); + } + pub async fn prune_inactive_worktrees(&mut self) { match manager::prune_inactive_worktrees(&self.db).await { Ok(outcome) => { @@ -2327,14 +2363,16 @@ fn format_duration(duration_secs: u64) -> String { #[cfg(test)] mod tests { - use anyhow::Result; + use anyhow::{Context, Result}; use chrono::Utc; use ratatui::{Terminal, backend::TestBackend}; - use std::path::PathBuf; + use std::fs; + use std::path::{Path, PathBuf}; + use std::process::Command; use uuid::Uuid; use super::*; - use crate::config::PaneLayout; + use crate::config::{Config, PaneLayout, Theme}; #[test] fn render_sessions_shows_summary_headers_and_selected_row() { @@ -3235,6 +3273,68 @@ mod tests { Ok(()) } + #[tokio::test(flavor = "current_thread")] + async fn merge_selected_worktree_sets_operator_note_when_ready() -> Result<()> { + let tempdir = std::env::temp_dir().join(format!("dashboard-merge-{}", Uuid::new_v4())); + let repo_root = tempdir.join("repo"); + init_git_repo(&repo_root)?; + + let cfg = build_config(&tempdir); + let db = StateStore::open(&cfg.db_path)?; + let worktree = worktree::create_for_session_in_repo("merge1234", &cfg, &repo_root)?; + let session_id = "merge1234".to_string(); + let now = Utc::now(); + db.insert_session(&Session { + id: session_id.clone(), + task: "merge via dashboard".to_string(), + agent_type: "claude".to_string(), + working_dir: worktree.path.clone(), + state: SessionState::Completed, + pid: None, + worktree: Some(worktree.clone()), + created_at: now, + updated_at: now, + metrics: SessionMetrics::default(), + })?; + + std::fs::write(worktree.path.join("dashboard.txt"), "dashboard merge\n")?; + Command::new("git") + .arg("-C") + .arg(&worktree.path) + .args(["add", "dashboard.txt"]) + .status()?; + Command::new("git") + .arg("-C") + .arg(&worktree.path) + .args(["commit", "-qm", "dashboard work"]) + .status()?; + + let mut dashboard = Dashboard::new(db, cfg); + dashboard.sync_selection_by_id(Some(&session_id)); + dashboard.merge_selected_worktree().await; + + let note = dashboard + .operator_note + .clone() + .context("operator note should be set")?; + assert!(note.contains("merged ecc/merge1234 into")); + assert!(note.contains(&format!("for {}", format_session_id(&session_id)))); + + let session = dashboard + .db + .get_session(&session_id)? + .context("merged session should still exist")?; + assert!(session.worktree.is_none(), "worktree metadata should be cleared"); + assert!(!worktree.path.exists(), "worktree path should be removed"); + assert_eq!( + std::fs::read_to_string(repo_root.join("dashboard.txt"))?, + "dashboard merge\n" + ); + + let _ = std::fs::remove_dir_all(&tempdir); + Ok(()) + } + #[tokio::test] async fn delete_selected_session_removes_inactive_session() -> Result<()> { let db_path = std::env::temp_dir().join(format!("ecc2-dashboard-{}.db", Uuid::new_v4())); @@ -3501,6 +3601,48 @@ mod tests { } } + fn build_config(root: &Path) -> Config { + Config { + db_path: root.join("state.db"), + worktree_root: root.join("worktrees"), + max_parallel_sessions: 4, + max_parallel_worktrees: 4, + session_timeout_secs: 60, + heartbeat_interval_secs: 5, + default_agent: "claude".to_string(), + auto_dispatch_unread_handoffs: false, + auto_dispatch_limit_per_session: 5, + cost_budget_usd: 10.0, + token_budget: 500_000, + theme: Theme::Dark, + pane_layout: PaneLayout::Horizontal, + risk_thresholds: Config::RISK_THRESHOLDS, + } + } + + fn init_git_repo(path: &Path) -> Result<()> { + fs::create_dir_all(path)?; + run_git(path, &["init", "-q"])?; + run_git(path, &["config", "user.name", "ECC Tests"])?; + run_git(path, &["config", "user.email", "ecc-tests@example.com"])?; + fs::write(path.join("README.md"), "hello\n")?; + run_git(path, &["add", "README.md"])?; + run_git(path, &["commit", "-qm", "init"])?; + Ok(()) + } + + fn run_git(path: &Path, args: &[&str]) -> Result<()> { + let output = Command::new("git") + .arg("-C") + .arg(path) + .args(args) + .output()?; + if !output.status.success() { + anyhow::bail!("{}", String::from_utf8_lossy(&output.stderr)); + } + Ok(()) + } + fn sample_session( id: &str, agent_type: &str, diff --git a/ecc2/src/worktree/mod.rs b/ecc2/src/worktree/mod.rs index dac77b6c..574cf22d 100644 --- a/ecc2/src/worktree/mod.rs +++ b/ecc2/src/worktree/mod.rs @@ -1,5 +1,6 @@ use anyhow::{Context, Result}; -use std::path::Path; +use serde::Serialize; +use std::path::{Path, PathBuf}; use std::process::Command; use crate::config::Config; @@ -25,6 +26,13 @@ pub enum WorktreeHealth { Conflicted, } +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] +pub struct MergeOutcome { + pub branch: String, + pub base_branch: String, + pub already_up_to_date: bool, +} + /// Create a new git worktree for an agent session. pub fn create_for_session(session_id: &str, cfg: &Config) -> Result { let repo_root = std::env::current_dir().context("Failed to resolve repository root")?; @@ -73,18 +81,59 @@ pub(crate) fn create_for_session_in_repo( } /// Remove a worktree and its branch. -pub fn remove(path: &Path) -> Result<()> { +pub fn remove(worktree: &WorktreeInfo) -> Result<()> { + let repo_root = match base_checkout_path(worktree) { + Ok(path) => path, + Err(error) => { + tracing::warn!( + "Falling back to filesystem-only cleanup for {}: {error}", + worktree.path.display() + ); + if worktree.path.exists() { + if let Err(remove_error) = std::fs::remove_dir_all(&worktree.path) { + tracing::warn!( + "Fallback worktree directory cleanup warning for {}: {remove_error}", + worktree.path.display() + ); + } + } + return Ok(()); + } + }; let output = Command::new("git") .arg("-C") - .arg(path) + .arg(&repo_root) .args(["worktree", "remove", "--force"]) - .arg(path) + .arg(&worktree.path) .output() .context("Failed to remove worktree")?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr); tracing::warn!("Worktree removal warning: {stderr}"); + if worktree.path.exists() { + if let Err(remove_error) = std::fs::remove_dir_all(&worktree.path) { + tracing::warn!( + "Fallback worktree directory cleanup warning for {}: {remove_error}", + worktree.path.display() + ); + } + } + } + + let branch_output = Command::new("git") + .arg("-C") + .arg(&repo_root) + .args(["branch", "-D", &worktree.branch]) + .output() + .context("Failed to delete worktree branch")?; + + if !branch_output.status.success() { + let stderr = String::from_utf8_lossy(&branch_output.stderr); + tracing::warn!( + "Worktree branch deletion warning for {}: {stderr}", + worktree.branch + ); } Ok(()) @@ -248,6 +297,61 @@ pub fn health(worktree: &WorktreeInfo) -> Result { } } +pub fn merge_into_base(worktree: &WorktreeInfo) -> Result { + let readiness = merge_readiness(worktree)?; + if readiness.status == MergeReadinessStatus::Conflicted { + anyhow::bail!(readiness.summary); + } + + if !git_status_short(&worktree.path)?.is_empty() { + anyhow::bail!( + "Worktree {} has uncommitted changes; commit or discard them before merging", + worktree.branch + ); + } + + let repo_root = base_checkout_path(worktree)?; + let current_branch = get_current_branch(&repo_root)?; + if current_branch != worktree.base_branch { + anyhow::bail!( + "Base branch {} is not checked out in repo root (currently {})", + worktree.base_branch, + current_branch + ); + } + + if !git_status_short(&repo_root)?.is_empty() { + anyhow::bail!( + "Repository root {} has uncommitted changes; commit or stash them before merging", + repo_root.display() + ); + } + + let output = Command::new("git") + .arg("-C") + .arg(&repo_root) + .args(["merge", "--no-edit", &worktree.branch]) + .output() + .context("Failed to merge worktree branch into base")?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + anyhow::bail!("git merge failed: {stderr}"); + } + + let merged_output = format!( + "{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + + Ok(MergeOutcome { + branch: worktree.branch.clone(), + base_branch: worktree.base_branch.clone(), + already_up_to_date: merged_output.contains("Already up to date."), + }) +} + fn git_diff_shortstat(worktree_path: &Path, extra_args: &[&str]) -> Result> { let mut command = Command::new("git"); command @@ -387,6 +491,61 @@ fn get_current_branch(repo_root: &Path) -> Result { Ok(String::from_utf8_lossy(&output.stdout).trim().to_string()) } +fn base_checkout_path(worktree: &WorktreeInfo) -> Result { + let output = Command::new("git") + .arg("-C") + .arg(&worktree.path) + .args(["worktree", "list", "--porcelain"]) + .output() + .context("Failed to resolve git worktree list")?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + anyhow::bail!("git worktree list --porcelain failed: {stderr}"); + } + + let target_branch = format!("refs/heads/{}", worktree.base_branch); + let mut current_path: Option = None; + let mut current_branch: Option = None; + let mut fallback: Option = None; + + for line in String::from_utf8_lossy(&output.stdout).lines() { + if line.is_empty() { + if let Some(path) = current_path.take() { + if fallback.is_none() && path != worktree.path { + fallback = Some(path.clone()); + } + if current_branch.as_deref() == Some(target_branch.as_str()) && path != worktree.path + { + return Ok(path); + } + } + current_branch = None; + continue; + } + + if let Some(path) = line.strip_prefix("worktree ") { + current_path = Some(PathBuf::from(path.trim())); + } else if let Some(branch) = line.strip_prefix("branch ") { + current_branch = Some(branch.trim().to_string()); + } + } + + if let Some(path) = current_path.take() { + if fallback.is_none() && path != worktree.path { + fallback = Some(path.clone()); + } + if current_branch.as_deref() == Some(target_branch.as_str()) && path != worktree.path { + return Ok(path); + } + } + + fallback.context(format!( + "Failed to locate base checkout for {} from git worktree list", + worktree.base_branch + )) +} + #[cfg(test)] mod tests { use super::*;