diff --git a/src/daemon.rs b/src/daemon.rs index 6adb325..bc51502 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -8,8 +8,8 @@ use crate::git::cli_parser::{ }; use crate::git::find_repository_in_path; use crate::git::repo_state::{ - HeadState, common_dir_for_worktree, git_dir_for_worktree, latest_reflog_old_oid_for_worktree, - read_head_state_for_worktree, read_ref_oid_for_worktree, + HeadState, common_dir_for_git_dir, common_dir_for_worktree, git_dir_for_worktree, + latest_reflog_old_oid_for_worktree, read_head_state_for_worktree, read_ref_oid_for_worktree, resolve_linear_head_commit_chain_for_worktree, resolve_rebase_segment_for_worktree, resolve_reflog_old_oid_for_ref_new_oid_in_worktree, resolve_squash_source_head_for_worktree, resolve_stash_target_oid_for_worktree, resolve_worktree_head_reflog_old_oid_for_new_head, @@ -358,6 +358,34 @@ fn is_missing_working_dir_error(error: &AutterError) -> bool { } } +/// Returns true when a side effect failed because its repository was deleted +/// mid-operation. git reports this race in more shapes than +/// `is_missing_working_dir_error` knows (for example `git diff` exits 1 with +/// "Could not access " when the objects are already gone), so any git or +/// IO failure counts when the repository is no longer on disk. +fn is_vanished_repo_error(error: &AutterError, worktree: Option<&Path>) -> bool { + is_missing_working_dir_error(error) + || (matches!( + error, + AutterError::GitCliError { .. } | AutterError::IoError(_) + ) && worktree.is_some_and(repository_is_gone)) +} + +/// Returns true when the worktree, its git dir, or its object store no longer +/// exists. +fn repository_is_gone(worktree: &Path) -> bool { + if !worktree.is_dir() { + return true; + } + let git_dir = match git_dir_for_worktree(worktree) { + Some(git_dir) => git_dir, + None if worktree.join("HEAD").is_file() => worktree.to_path_buf(), + None => return true, + }; + let common_dir = common_dir_for_git_dir(&git_dir).unwrap_or_else(|| git_dir.clone()); + !git_dir.join("HEAD").is_file() || !common_dir.join("objects").is_dir() +} + fn trace_root_sid(sid: &str) -> &str { sid.split('/').next().unwrap_or(sid) } @@ -4275,6 +4303,13 @@ impl ActorDaemonCoordinator { Ok(()) } + fn inflight_family_effects(&self, family: &str) -> usize { + self.inflight_effects_by_family + .lock() + .map(|map| map.get(family).copied().unwrap_or(0)) + .unwrap_or(0) + } + /// Garbage-collect empty or idle entries from per-family and per-root maps /// to prevent unbounded memory growth in long-running daemon processes. fn gc_stale_family_state(&self) { @@ -7216,12 +7251,12 @@ impl ActorDaemonCoordinator { .maybe_apply_side_effects_for_applied_command_inner(family, applied) .await { - // A working directory that vanished mid-operation (e.g. a temp repo - // cleaned up while this async side effect was still in flight) makes - // git commands fail with exit 128 / "No such file or directory". - // That is a benign race, not a real fault, so skip quietly instead - // of surfacing a reported "command side effect failed" error. - Err(error) if is_missing_working_dir_error(&error) => { + // A repository that vanished mid-operation (e.g. a temp repo cleaned + // up while this async side effect was still in flight) makes git + // commands fail. That is a benign race, not a real fault, so skip + // quietly instead of surfacing a reported "command side effect + // failed" error. + Err(error) if is_vanished_repo_error(&error, applied.command.worktree.as_deref()) => { tracing::debug!( %error, ?family, @@ -7834,6 +7869,7 @@ impl ActorDaemonCoordinator { last_error: status .last_error .or_else(|| self.latest_side_effect_error(&family_key).ok().flatten()), + inflight_effects: self.inflight_family_effects(&family_key), }) } @@ -9351,6 +9387,52 @@ mod tests { ))); } + fn init_fake_repo(worktree: &Path) { + fs::create_dir_all(worktree.join(".git").join("objects")).unwrap(); + fs::write(worktree.join(".git").join("HEAD"), "ref: refs/heads/main\n").unwrap(); + } + + #[test] + fn vanished_repo_error_detects_any_git_failure_once_repo_is_gone() { + // `git diff` exits 1 with "Could not access " when the repo is + // deleted while the post-commit side effect runs. + let could_not_access = AutterError::GitCliError { + code: Some(1), + stderr: "error: Could not access '0123456789abcdef0123456789abcdef01234567'" + .to_string(), + args: vec!["diff".to_string()], + }; + let temp = tempfile::tempdir().unwrap(); + let worktree = temp.path().join("repo"); + init_fake_repo(&worktree); + + assert!(!is_vanished_repo_error(&could_not_access, Some(&worktree))); + + fs::remove_dir_all(worktree.join(".git").join("objects")).unwrap(); + assert!(is_vanished_repo_error(&could_not_access, Some(&worktree))); + + fs::remove_dir_all(&worktree).unwrap(); + assert!(is_vanished_repo_error(&could_not_access, Some(&worktree))); + + // Non-git errors never qualify, even when the repo is gone. + assert!(!is_vanished_repo_error( + &AutterError::Generic("boom".to_string()), + Some(&worktree) + )); + } + + #[test] + fn repository_is_gone_keeps_subdirectory_of_live_repo() { + let temp = tempfile::tempdir().unwrap(); + let worktree = temp.path().join("repo"); + init_fake_repo(&worktree); + let subdir = worktree.join("src"); + fs::create_dir_all(&subdir).unwrap(); + + assert!(!repository_is_gone(&worktree)); + assert!(!repository_is_gone(&subdir)); + } + struct EnvVarGuard { key: &'static str, original: Option, diff --git a/src/daemon/control_api.rs b/src/daemon/control_api.rs index 87f2775..3f86690 100644 --- a/src/daemon/control_api.rs +++ b/src/daemon/control_api.rs @@ -111,6 +111,10 @@ pub struct FamilyStatus { pub family_key: String, pub latest_seq: u64, pub last_error: Option, + /// Side effects that are still running for this family. Older daemons do + /// not send this field, so it reads as 0. + #[serde(default)] + pub inflight_effects: usize, } /// A telemetry envelope sent from client to daemon. diff --git a/src/diagnostics.rs b/src/diagnostics.rs index 060a944..9c581c0 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -465,6 +465,7 @@ pub fn run_attribution_self_check(target: &GitDiagnosticTarget) -> DiagnosticChe match result { Ok(details) => { + wait_for_daemon_family_idle(&repo_path, deadline); let _ = fs::remove_dir_all(&repo_path); DiagnosticCheckResult::passed("attribution self-check completed", details, commands) } @@ -1024,6 +1025,21 @@ fn wait_for_daemon_family_status( )) } +/// Waits until the daemon has no side effects in flight for the repo, so that +/// deleting the repo does not race them. Returns at once when the daemon is not +/// reachable or is too old to report in-flight side effects. +fn wait_for_daemon_family_idle(repo_path: &Path, deadline: Instant) { + let Ok(config) = crate::daemon::DaemonConfig::from_env_or_default_paths() else { + return; + }; + while Instant::now() < deadline { + match read_daemon_family_status(&config, repo_path) { + Ok(status) if status.inflight_effects > 0 => std::thread::sleep(POLL_INTERVAL), + _ => return, + } + } +} + fn read_daemon_family_status( config: &crate::daemon::DaemonConfig, repo_path: &Path,