diff --git a/crates/kild-core/src/cleanup/handler.rs b/crates/kild-core/src/cleanup/handler.rs index f492c5bb..30cc817a 100644 --- a/crates/kild-core/src/cleanup/handler.rs +++ b/crates/kild-core/src/cleanup/handler.rs @@ -1007,8 +1007,8 @@ mod tests { git::test_support::init_repo_with_commit(repo_dir.path()).unwrap(); - let branch_name = format!("kild/{branch_suffix}"); - let admin_name = format!("kild-{branch_suffix}"); + let branch_name = git::naming::kild_branch_name(branch_suffix); + let admin_name = git::naming::kild_worktree_admin_name(branch_suffix); git::test_support::create_branch(repo_dir.path(), &branch_name).unwrap(); let worktree_path = worktree_base.path().join(&admin_name); diff --git a/crates/kild-core/src/git/overlaps.rs b/crates/kild-core/src/git/overlaps.rs index 1f2c81ee..ac0b7735 100644 --- a/crates/kild-core/src/git/overlaps.rs +++ b/crates/kild-core/src/git/overlaps.rs @@ -283,7 +283,7 @@ mod tests { .current_dir(dir) .output() .unwrap(); - let kild_branch = format!("kild/{}", branch); + let kild_branch = crate::git::naming::kild_branch_name(branch); Command::new("git") .args(["checkout", "-b", &kild_branch]) .current_dir(dir) diff --git a/crates/kild-core/src/sessions/destroy.rs b/crates/kild-core/src/sessions/destroy.rs index 38753437..f444a4bf 100644 --- a/crates/kild-core/src/sessions/destroy.rs +++ b/crates/kild-core/src/sessions/destroy.rs @@ -38,6 +38,200 @@ pub fn cleanup_task_list(session_id: &str, task_list_id: &str, home_dir: &std::p } } +/// Kill all tracked agents for a session, closing their terminal windows or daemon PTYs. +/// +/// Returns `Ok(())` if all agents were killed (or if `force` is true). +/// Returns `Err(ProcessKillFailed)` if any agent could not be killed and `force` is false. +fn kill_tracked_agents(session: &Session, force: bool) -> Result<(), SessionError> { + if !session.has_agents() { + warn!( + event = "core.session.destroy_no_agents", + session_id = %session.id, + branch = %session.branch, + "Session has no tracked agents — skipping process/terminal cleanup" + ); + return Ok(()); + } + + let mut kill_errors: Vec<(u32, String)> = Vec::with_capacity(session.agent_count()); + for agent_proc in session.agents() { + if let Some(daemon_sid) = agent_proc.daemon_session_id() { + // Daemon-managed: destroy via IPC + info!( + event = "core.session.destroy_daemon_session", + daemon_session_id = daemon_sid, + agent = agent_proc.agent() + ); + if let Err(e) = crate::daemon::client::destroy_daemon_session(daemon_sid, force) { + warn!( + event = "core.session.destroy_daemon_failed_continue", + daemon_session_id = daemon_sid, + error = %e, + force = force, + ); + // Daemon cleanup failure is non-fatal — the kild session file + // is being removed regardless. Do NOT add to kill_errors. + } + + // Close the attach terminal window (if tracked) + if let (Some(terminal_type), Some(window_id)) = + (agent_proc.terminal_type(), agent_proc.terminal_window_id()) + { + info!( + event = "core.session.destroy_close_attach_window", + terminal_type = ?terminal_type, + agent = agent_proc.agent(), + ); + terminal::handler::close_terminal(terminal_type, Some(window_id)); + } + } else { + // Terminal-managed: close window + kill process + if let (Some(terminal_type), Some(window_id)) = + (agent_proc.terminal_type(), agent_proc.terminal_window_id()) + { + info!( + event = "core.session.destroy_close_terminal", + terminal_type = ?terminal_type, + agent = agent_proc.agent(), + ); + terminal::handler::close_terminal(terminal_type, Some(window_id)); + } + + let Some(pid) = agent_proc.process_id() else { + continue; + }; + + info!( + event = "core.session.destroy_kill_started", + pid = pid, + agent = agent_proc.agent() + ); + + let result = crate::process::kill_process( + pid, + agent_proc.process_name(), + agent_proc.process_start_time(), + ); + + match result { + Ok(()) => { + info!(event = "core.session.destroy_kill_completed", pid = pid); + } + Err(crate::process::ProcessError::NotFound { .. }) => { + info!(event = "core.session.destroy_kill_already_dead", pid = pid); + } + Err(e) if force => { + warn!( + event = "core.session.destroy_kill_failed_force_continue", + pid = pid, + error = %e + ); + } + Err(e) => { + kill_errors.push((pid, e.to_string())); + } + } + } + } + + if !kill_errors.is_empty() && !force { + for (pid, err) in &kill_errors { + error!( + event = "core.session.destroy_kill_failed", + pid = pid, + error = %err + ); + } + + let &(first_pid, ref first_msg) = kill_errors.first().unwrap(); + let error_count = kill_errors.len(); + + let message = if error_count == 1 { + format!( + "Process still running. Kill it manually or use --force flag: {}", + first_msg + ) + } else { + let pids: String = kill_errors + .iter() + .map(|(p, _)| p.to_string()) + .collect::>() + .join(", "); + format!( + "{} processes still running (PIDs: {}). Kill them manually or use --force flag.", + error_count, pids + ) + }; + + return Err(SessionError::ProcessKillFailed { + pid: first_pid, + message, + }); + } + + Ok(()) +} + +/// Sweep for untracked daemon sessions created by the UI. +/// +/// UI-created daemon sessions use the naming pattern `{kild_id}_ui_shell_{counter}` +/// and are not tracked in the session file. Queries the daemon for all sessions +/// with a matching prefix and destroys any remaining ones. +fn sweep_ui_daemon_sessions(session_id: &str) { + let prefix = format!("{}_ui_shell_", session_id); + match crate::daemon::client::list_daemon_sessions() { + Ok(sessions) => { + let ui_sessions: Vec<_> = sessions + .iter() + .filter(|s| s.id.starts_with(&prefix)) + .collect(); + + if !ui_sessions.is_empty() { + info!( + event = "core.session.destroy_ui_sessions_sweep_started", + session_id = session_id, + count = ui_sessions.len() + ); + } + + for daemon_session in ui_sessions { + info!( + event = "core.session.destroy_ui_session", + daemon_session_id = %daemon_session.id, + ); + if let Err(e) = + crate::daemon::client::destroy_daemon_session(&daemon_session.id, true) + { + warn!( + event = "core.session.destroy_ui_session_failed", + daemon_session_id = %daemon_session.id, + error = %e, + ); + eprintln!( + "Warning: Failed to clean up UI terminal session {}: {}", + daemon_session.id, e + ); + } + } + } + Err(crate::daemon::client::DaemonClientError::NotRunning { .. }) => { + debug!( + event = "core.session.destroy_ui_sessions_sweep_skipped", + session_id = session_id, + reason = "daemon_not_running" + ); + } + Err(e) => { + warn!( + event = "core.session.destroy_ui_sessions_sweep_failed", + session_id = session_id, + error = %e, + "Could not query daemon for orphaned UI sessions" + ); + } + } +} + /// Destroys a kild by removing its worktree, killing the process, and deleting the session file. /// /// # Arguments @@ -79,194 +273,10 @@ pub fn destroy_session(name: &str, force: bool) -> Result<(), SessionError> { ); // 2. Close all terminal windows and kill all processes - { - if !session.has_agents() { - warn!( - event = "core.session.destroy_no_agents", - session_id = %session.id, - branch = name, - "Session has no tracked agents — skipping process/terminal cleanup" - ); - } - - // Kill/stop all tracked agents — branch on daemon vs terminal - let mut kill_errors: Vec<(u32, String)> = Vec::with_capacity(session.agent_count()); - for agent_proc in session.agents() { - if let Some(daemon_sid) = agent_proc.daemon_session_id() { - // Daemon-managed: destroy via IPC - info!( - event = "core.session.destroy_daemon_session", - daemon_session_id = daemon_sid, - agent = agent_proc.agent() - ); - if let Err(e) = crate::daemon::client::destroy_daemon_session(daemon_sid, force) { - warn!( - event = "core.session.destroy_daemon_failed_continue", - daemon_session_id = daemon_sid, - error = %e, - force = force, - ); - // Don't add to kill_errors — daemon cleanup failure is non-fatal. - // The kild session file is being removed regardless. - } - - // Close the attach terminal window (if tracked) - if let (Some(terminal_type), Some(window_id)) = - (agent_proc.terminal_type(), agent_proc.terminal_window_id()) - { - info!( - event = "core.session.destroy_close_attach_window", - terminal_type = ?terminal_type, - agent = agent_proc.agent(), - ); - terminal::handler::close_terminal(terminal_type, Some(window_id)); - } - } else { - // Terminal-managed: close window + kill process - if let (Some(terminal_type), Some(window_id)) = - (agent_proc.terminal_type(), agent_proc.terminal_window_id()) - { - info!( - event = "core.session.destroy_close_terminal", - terminal_type = ?terminal_type, - agent = agent_proc.agent(), - ); - terminal::handler::close_terminal(terminal_type, Some(window_id)); - } - - let Some(pid) = agent_proc.process_id() else { - continue; - }; - - info!( - event = "core.session.destroy_kill_started", - pid = pid, - agent = agent_proc.agent() - ); - - let result = crate::process::kill_process( - pid, - agent_proc.process_name(), - agent_proc.process_start_time(), - ); - - match result { - Ok(()) => { - info!(event = "core.session.destroy_kill_completed", pid = pid); - } - Err(crate::process::ProcessError::NotFound { .. }) => { - info!(event = "core.session.destroy_kill_already_dead", pid = pid); - } - Err(e) if force => { - warn!( - event = "core.session.destroy_kill_failed_force_continue", - pid = pid, - error = %e - ); - } - Err(e) => { - kill_errors.push((pid, e.to_string())); - } - } - } - } - - if !kill_errors.is_empty() && !force { - for (pid, err) in &kill_errors { - error!( - event = "core.session.destroy_kill_failed", - pid = pid, - error = %err - ); - } - - let &(first_pid, ref first_msg) = kill_errors.first().unwrap(); - let error_count = kill_errors.len(); - - let message = if error_count == 1 { - format!( - "Process still running. Kill it manually or use --force flag: {}", - first_msg - ) - } else { - let pids: String = kill_errors - .iter() - .map(|(p, _)| p.to_string()) - .collect::>() - .join(", "); - format!( - "{} processes still running (PIDs: {}). Kill them manually or use --force flag.", - error_count, pids - ) - }; - - return Err(SessionError::ProcessKillFailed { - pid: first_pid, - message, - }); - } - } + kill_tracked_agents(&session, force)?; // 3a. Sweep for untracked daemon sessions (e.g., UI-created shells) - // - // UI-created daemon sessions use the naming pattern `{kild_id}_ui_shell_{counter}` - // and are not tracked in the session file. Query the daemon for all sessions - // with a matching prefix and destroy any remaining ones. - { - let prefix = format!("{}_ui_shell_", session.id); - match crate::daemon::client::list_daemon_sessions() { - Ok(sessions) => { - let ui_sessions: Vec<_> = sessions - .iter() - .filter(|s| s.id.starts_with(&prefix)) - .collect(); - - if !ui_sessions.is_empty() { - info!( - event = "core.session.destroy_ui_sessions_sweep_started", - session_id = %session.id, - count = ui_sessions.len() - ); - } - - for daemon_session in ui_sessions { - info!( - event = "core.session.destroy_ui_session", - daemon_session_id = %daemon_session.id, - ); - if let Err(e) = - crate::daemon::client::destroy_daemon_session(&daemon_session.id, true) - { - warn!( - event = "core.session.destroy_ui_session_failed", - daemon_session_id = %daemon_session.id, - error = %e, - ); - eprintln!( - "Warning: Failed to clean up UI terminal session {}: {}", - daemon_session.id, e - ); - } - } - } - Err(crate::daemon::client::DaemonClientError::NotRunning { .. }) => { - // Daemon not running — no UI sessions to clean up - debug!( - event = "core.session.destroy_ui_sessions_sweep_skipped", - session_id = %session.id, - reason = "daemon_not_running" - ); - } - Err(e) => { - warn!( - event = "core.session.destroy_ui_sessions_sweep_failed", - session_id = %session.id, - error = %e, - "Could not query daemon for orphaned UI sessions" - ); - } - } - } + sweep_ui_daemon_sessions(&session.id); // 3b. Clean up tmux shim state and destroy child shim panes match KildPaths::resolve() { diff --git a/crates/kild-core/src/sessions/open.rs b/crates/kild-core/src/sessions/open.rs index 20f8009b..5b6e1686 100644 --- a/crates/kild-core/src/sessions/open.rs +++ b/crates/kild-core/src/sessions/open.rs @@ -10,6 +10,54 @@ use super::daemon_helpers::{ spawn_and_save_attach_window, spawn_daemon_agent, spawn_terminal_agent, }; +/// Resolve the agent command and session ID for resume or fresh open. +/// +/// When `resume` is true, appends resume args using the session's stored agent_session_id. +/// When `resume` is false, generates a fresh session ID for resume-capable agents. +/// +/// Returns `(final_agent_command, new_agent_session_id)`. +fn resolve_resume_args( + resume: bool, + is_bare_shell: bool, + agent: &str, + agent_command: String, + session: &Session, +) -> Result<(String, Option), SessionError> { + if resume && !is_bare_shell { + if let Some(ref sid) = session.agent_session_id { + if agents::resume::supports_resume(agent) { + let extra = agents::resume::resume_session_args(agent, sid); + let cmd = format!("{} {}", agent_command, extra.join(" ")); + info!(event = "core.session.resume_started", session_id = %sid, agent = %agent); + Ok((cmd, Some(sid.clone()))) + } else { + error!(event = "core.session.resume_unsupported", agent = %agent); + Err(SessionError::ResumeUnsupported { + agent: agent.to_string(), + }) + } + } else { + error!(event = "core.session.resume_no_session_id", branch = %session.branch); + Err(SessionError::ResumeNoSessionId { + branch: session.branch.to_string(), + }) + } + } else if !is_bare_shell && agents::resume::supports_resume(agent) { + // Fresh open: generate new session ID for future resume capability + let sid = agents::resume::generate_session_id(); + let extra = agents::resume::create_session_args(agent, &sid); + let cmd = if extra.is_empty() { + agent_command + } else { + info!(event = "core.session.agent_session_id_set", session_id = %sid); + format!("{} {}", agent_command, extra.join(" ")) + }; + Ok((cmd, Some(sid))) + } else { + Ok((agent_command, None)) + } +} + /// Resolve the effective runtime mode for `open_session`. /// /// Priority: explicit CLI flag > session's stored mode > config > Terminal default. @@ -255,39 +303,8 @@ pub fn open_session(request: &super::types::OpenSessionRequest) -> Result String { /// The git branch namespace prefix used by KILD for worktree branches. pub const KILD_BRANCH_PREFIX: &str = "kild/"; +/// The worktree admin name prefix used for `.git/worktrees/` directories. +pub(crate) const WORKTREE_ADMIN_PREFIX: &str = "kild-"; + /// Constructs the KILD branch name for a given user branch name. /// /// Example: `"my-feature"` → `"kild/my-feature"` pub fn kild_branch_name(branch: &str) -> String { - format!("kild/{branch}") + format!("{}{branch}", KILD_BRANCH_PREFIX) } /// Constructs the worktree admin name (flat, filesystem-safe) for a given user branch name. @@ -31,7 +34,7 @@ pub fn kild_branch_name(branch: &str) -> String { /// - `"my-feature"` → `"kild-my-feature"` /// - `"feature/auth"` → `"kild-feature-auth"` pub fn kild_worktree_admin_name(branch: &str) -> String { - format!("kild-{}", sanitize_for_path(branch)) + format!("{}{}", WORKTREE_ADMIN_PREFIX, sanitize_for_path(branch)) } pub fn calculate_worktree_path(base_dir: &Path, project_name: &str, branch: &str) -> PathBuf { diff --git a/crates/kild-tmux-shim/src/commands.rs b/crates/kild-tmux-shim/src/commands.rs index bbcafc1b..02df0cad 100644 --- a/crates/kild-tmux-shim/src/commands.rs +++ b/crates/kild-tmux-shim/src/commands.rs @@ -282,10 +282,13 @@ fn create_pty_pane( Ok(pane_id) } +/// The tmux version string reported by the shim. +const SHIM_VERSION: &str = "tmux 3.4"; + // -- Command handlers -- fn handle_version() -> Result { - println!("tmux 3.4"); + println!("{}", SHIM_VERSION); Ok(0) } diff --git a/crates/kild-ui/src/views/detail_view.rs b/crates/kild-ui/src/views/detail_view.rs index 6360edaf..588fe71c 100644 --- a/crates/kild-ui/src/views/detail_view.rs +++ b/crates/kild-ui/src/views/detail_view.rs @@ -204,7 +204,7 @@ pub fn render_detail_view( .child(render_detail_row("Created", &created_at, theme::text())) .child(render_detail_row( "Branch", - &format!("kild/{}", branch), + &kild_core::git::kild_branch_name(&branch), theme::text(), )) .child(render_detail_row("Runtime", &runtime_text, theme::text())), diff --git a/crates/kild/src/commands/commits.rs b/crates/kild/src/commands/commits.rs index 0262eda2..206da390 100644 --- a/crates/kild/src/commands/commits.rs +++ b/crates/kild/src/commands/commits.rs @@ -38,7 +38,7 @@ pub(crate) fn handle_commits_command( if let Err(e) = std::io::stdout().write_all(commits.as_bytes()) && e.kind() != std::io::ErrorKind::BrokenPipe { - eprintln!("Write failed: {}", e); + eprintln!("{} {}", crate::color::error("Write failed:"), e); error!( event = "cli.commits_write_failed", branch = branch, diff --git a/crates/kild/src/commands/diff.rs b/crates/kild/src/commands/diff.rs index 11c78235..58b942cc 100644 --- a/crates/kild/src/commands/diff.rs +++ b/crates/kild/src/commands/diff.rs @@ -37,7 +37,7 @@ pub(crate) fn handle_diff_command(matches: &ArgMatches) -> Result<(), Box Result<(), Box { - eprintln!("Could not focus '{}': {}", branch, e); + super::helpers::display_operation_error("focus", branch, &e); error!(event = "cli.focus_failed", branch = branch, error = %e); Err(e.into()) } diff --git a/crates/kild/src/commands/health.rs b/crates/kild/src/commands/health.rs index 594c1899..70a71e00 100644 --- a/crates/kild/src/commands/health.rs +++ b/crates/kild/src/commands/health.rs @@ -103,7 +103,7 @@ fn run_health_once( if json_output { return Err(super::helpers::print_json_error(&e, e.error_code())); } - eprintln!("Health check failed for '{}': {}", branch_name, e); + super::helpers::display_operation_error("check health of", branch_name, &e); Err(e.into()) } } @@ -131,7 +131,7 @@ fn run_health_once( if json_output { return Err(super::helpers::print_json_error(&e, e.error_code())); } - eprintln!("Health check failed: {}", e); + eprintln!("{} {}", crate::color::error("Health check failed:"), e); Err(e.into()) } } diff --git a/crates/kild/src/commands/helpers.rs b/crates/kild/src/commands/helpers.rs index 1414b036..b823308a 100644 --- a/crates/kild/src/commands/helpers.rs +++ b/crates/kild/src/commands/helpers.rs @@ -9,6 +9,23 @@ use kild_core::{events, session_ops}; use super::json_types::JsonError; use crate::color; +/// Display a CLI operation error consistently. +/// +/// Format: `"Could not {operation} '{target}': {error}"` with colored "Could not {operation}" prefix. +/// All single-resource CLI errors should use this for consistent user-facing output. +pub(crate) fn display_operation_error( + operation: &str, + target: &str, + error: &dyn std::fmt::Display, +) { + eprintln!( + "{} '{}': {}", + color::error(&format!("Could not {}", operation)), + target, + error + ); +} + /// Resolve the branch name of the calling session, if running inside one. /// /// Tries `$KILD_SESSION_BRANCH` first (reliable for claude/codex agents), diff --git a/crates/kild/src/commands/hide.rs b/crates/kild/src/commands/hide.rs index 5d0960bf..1947d98c 100644 --- a/crates/kild/src/commands/hide.rs +++ b/crates/kild/src/commands/hide.rs @@ -54,7 +54,7 @@ pub(crate) fn handle_hide_command(matches: &ArgMatches) -> Result<(), Box { - eprintln!("Could not hide '{}': {}", branch, e); + super::helpers::display_operation_error("hide", branch, &e); error!(event = "cli.hide_failed", branch = branch, error = %e); events::log_app_error(&e); Err(e.into()) diff --git a/crates/kild/src/commands/open.rs b/crates/kild/src/commands/open.rs index fd94927f..a2028b44 100644 --- a/crates/kild/src/commands/open.rs +++ b/crates/kild/src/commands/open.rs @@ -101,7 +101,7 @@ pub(crate) fn handle_open_command(matches: &ArgMatches) -> Result<(), Box { - eprintln!("Could not open '{}': {}", branch, e); + super::helpers::display_operation_error("open", branch, &e); error!(event = "cli.open_failed", branch = branch, error = %e); events::log_app_error(&e); Err(e.into()) diff --git a/crates/kild/src/commands/pr.rs b/crates/kild/src/commands/pr.rs index 1d509bfa..836d8765 100644 --- a/crates/kild/src/commands/pr.rs +++ b/crates/kild/src/commands/pr.rs @@ -41,7 +41,7 @@ pub(crate) fn handle_pr_command(matches: &ArgMatches) -> Result<(), Box Result<(), Box Result<(), Box) -> Result<(), Box { error!(event = "cli.teammates_failed", branch = branch, error = %e); events::log_app_error(&e); - eprintln!("Failed to read pane registry: {}", e); + eprintln!( + "{} {}", + crate::color::error("Failed to read pane registry:"), + e + ); return Err(e.into()); } };