From 6773f90bfe3a25cdc1fe6dcd0c4afc0d88bd3e6c Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Wed, 7 Oct 2026 19:49:06 +0000 Subject: [PATCH 1/2] fix(daemon): skip side-effect errors when the repo was deleted mid-operation The vanished-repo guard matched only exit-128 stderr text and NotFound IO errors. git also reports the race in other shapes, for example `git diff` exits 1 with "Could not access " after the objects are gone, so those errors still reached error tracking. The guard now also treats any git or IO error as benign when the command's worktree, its git dir HEAD, or its objects dir no longer exists. The daemon now reports in-flight side effects per family in the family status, and the `autter doctor` attribution self-check waits for them before it deletes its repo. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 509cd030-ac58-4e8e-84a3-7f359e5a5d9c --- src/daemon.rs | 138 +++++++++++++++++++++++++++++++++----- src/daemon/control_api.rs | 4 ++ src/diagnostics.rs | 16 +++++ 3 files changed, 140 insertions(+), 18 deletions(-) diff --git a/src/daemon.rs b/src/daemon.rs index 6adb325..366a1be 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, @@ -344,7 +344,19 @@ fn is_trace_payload(payload: &Value) -> bool { /// "not a git repository". /// - on Windows the directory read fails before git runs: an `IoError` with a /// not-found kind ("The system cannot find the path specified"). -fn is_missing_working_dir_error(error: &AutterError) -> bool { +/// +/// git reports the race in more shapes than these (for example `git diff` exits +/// 1 with "Could not access " when the objects are already gone). So any +/// git or IO failure also counts when the command's repository is no longer on +/// disk. +fn is_missing_working_dir_error(error: &AutterError, worktree: Option<&Path>) -> bool { + if matches!( + error, + AutterError::GitCliError { .. } | AutterError::IoError(_) + ) && worktree.is_some_and(repository_is_gone) + { + return true; + } match error { AutterError::GitCliError { code: Some(128), @@ -358,6 +370,30 @@ fn is_missing_working_dir_error(error: &AutterError) -> bool { } } +/// 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 dot_git = worktree.join(".git"); + let git_dir = if dot_git.is_dir() { + dot_git + } else if dot_git.is_file() { + match git_dir_for_worktree(worktree) { + Some(git_dir) => git_dir, + None => return false, + } + } else if worktree.join("HEAD").is_file() { + worktree.to_path_buf() + } else { + // A subdirectory of a live repo is not a vanished repo. + return worktree_root_for_path(worktree).is_none(); + }; + 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 +4311,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 +7259,14 @@ 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_missing_working_dir_error(&error, applied.command.worktree.as_deref()) => + { tracing::debug!( %error, ?family, @@ -7834,6 +7879,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), }) } @@ -9291,7 +9337,7 @@ mod tests { .to_string(), args: vec!["ls-tree".to_string(), "HEAD".to_string()], }; - assert!(is_missing_working_dir_error(&error)); + assert!(is_missing_working_dir_error(&error, None)); } #[test] @@ -9305,7 +9351,7 @@ mod tests { .to_string(), args: vec!["ls-tree".to_string(), "-r".to_string()], }; - assert!(is_missing_working_dir_error(&error)); + assert!(is_missing_working_dir_error(&error, None)); } #[test] @@ -9317,7 +9363,7 @@ mod tests { std::io::ErrorKind::NotFound, "The system cannot find the path specified. (os error 3)", )); - assert!(is_missing_working_dir_error(&error)); + assert!(is_missing_working_dir_error(&error, None)); } #[test] @@ -9328,7 +9374,7 @@ mod tests { stderr: "fatal: not a valid object name HEAD".to_string(), args: vec!["ls-tree".to_string(), "HEAD".to_string()], }; - assert!(!is_missing_working_dir_error(&bad_object)); + assert!(!is_missing_working_dir_error(&bad_object, None)); // Wrong exit code, even with matching stderr text. let other_code = AutterError::GitCliError { @@ -9336,19 +9382,75 @@ mod tests { stderr: "No such file or directory".to_string(), args: vec![], }; - assert!(!is_missing_working_dir_error(&other_code)); + assert!(!is_missing_working_dir_error(&other_code, None)); // An IoError with a different kind is a real fault, not a vanished dir. let permission_denied = AutterError::IoError(std::io::Error::new( std::io::ErrorKind::PermissionDenied, "access denied", )); - assert!(!is_missing_working_dir_error(&permission_denied)); + assert!(!is_missing_working_dir_error(&permission_denied, None)); // Non-git errors never qualify. - assert!(!is_missing_working_dir_error(&AutterError::Generic( - "No such file or directory".to_string() - ))); + assert!(!is_missing_working_dir_error( + &AutterError::Generic("No such file or directory".to_string()), + None + )); + } + + 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 missing_working_dir_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_missing_working_dir_error( + &could_not_access, + Some(&worktree) + )); + + fs::remove_dir_all(worktree.join(".git").join("objects")).unwrap(); + assert!(is_missing_working_dir_error( + &could_not_access, + Some(&worktree) + )); + + fs::remove_dir_all(&worktree).unwrap(); + assert!(is_missing_working_dir_error( + &could_not_access, + Some(&worktree) + )); + + // Non-git errors never qualify, even when the repo is gone. + assert!(!is_missing_working_dir_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 { 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, From 304a9cb06e0e7ccc4c58eb401a77043180e6f4aa Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Wed, 7 Oct 2026 19:51:42 +0000 Subject: [PATCH 2/2] refactor(daemon): keep the stderr classifier intact and reuse git_dir_for_worktree Move the repo-state check into a separate is_vanished_repo_error wrapper, so is_missing_working_dir_error and its tests stay as they were. Resolve the git dir with git_dir_for_worktree instead of a hand-written ladder. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 509cd030-ac58-4e8e-84a3-7f359e5a5d9c --- src/daemon.rs | 86 ++++++++++++++++++++------------------------------- 1 file changed, 33 insertions(+), 53 deletions(-) diff --git a/src/daemon.rs b/src/daemon.rs index 366a1be..bc51502 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -344,19 +344,7 @@ fn is_trace_payload(payload: &Value) -> bool { /// "not a git repository". /// - on Windows the directory read fails before git runs: an `IoError` with a /// not-found kind ("The system cannot find the path specified"). -/// -/// git reports the race in more shapes than these (for example `git diff` exits -/// 1 with "Could not access " when the objects are already gone). So any -/// git or IO failure also counts when the command's repository is no longer on -/// disk. -fn is_missing_working_dir_error(error: &AutterError, worktree: Option<&Path>) -> bool { - if matches!( - error, - AutterError::GitCliError { .. } | AutterError::IoError(_) - ) && worktree.is_some_and(repository_is_gone) - { - return true; - } +fn is_missing_working_dir_error(error: &AutterError) -> bool { match error { AutterError::GitCliError { code: Some(128), @@ -370,25 +358,29 @@ fn is_missing_working_dir_error(error: &AutterError, worktree: Option<&Path>) -> } } +/// 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 dot_git = worktree.join(".git"); - let git_dir = if dot_git.is_dir() { - dot_git - } else if dot_git.is_file() { - match git_dir_for_worktree(worktree) { - Some(git_dir) => git_dir, - None => return false, - } - } else if worktree.join("HEAD").is_file() { - worktree.to_path_buf() - } else { - // A subdirectory of a live repo is not a vanished repo. - return worktree_root_for_path(worktree).is_none(); + 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() @@ -7264,9 +7256,7 @@ impl ActorDaemonCoordinator { // 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_missing_working_dir_error(&error, applied.command.worktree.as_deref()) => - { + Err(error) if is_vanished_repo_error(&error, applied.command.worktree.as_deref()) => { tracing::debug!( %error, ?family, @@ -9337,7 +9327,7 @@ mod tests { .to_string(), args: vec!["ls-tree".to_string(), "HEAD".to_string()], }; - assert!(is_missing_working_dir_error(&error, None)); + assert!(is_missing_working_dir_error(&error)); } #[test] @@ -9351,7 +9341,7 @@ mod tests { .to_string(), args: vec!["ls-tree".to_string(), "-r".to_string()], }; - assert!(is_missing_working_dir_error(&error, None)); + assert!(is_missing_working_dir_error(&error)); } #[test] @@ -9363,7 +9353,7 @@ mod tests { std::io::ErrorKind::NotFound, "The system cannot find the path specified. (os error 3)", )); - assert!(is_missing_working_dir_error(&error, None)); + assert!(is_missing_working_dir_error(&error)); } #[test] @@ -9374,7 +9364,7 @@ mod tests { stderr: "fatal: not a valid object name HEAD".to_string(), args: vec!["ls-tree".to_string(), "HEAD".to_string()], }; - assert!(!is_missing_working_dir_error(&bad_object, None)); + assert!(!is_missing_working_dir_error(&bad_object)); // Wrong exit code, even with matching stderr text. let other_code = AutterError::GitCliError { @@ -9382,20 +9372,19 @@ mod tests { stderr: "No such file or directory".to_string(), args: vec![], }; - assert!(!is_missing_working_dir_error(&other_code, None)); + assert!(!is_missing_working_dir_error(&other_code)); // An IoError with a different kind is a real fault, not a vanished dir. let permission_denied = AutterError::IoError(std::io::Error::new( std::io::ErrorKind::PermissionDenied, "access denied", )); - assert!(!is_missing_working_dir_error(&permission_denied, None)); + assert!(!is_missing_working_dir_error(&permission_denied)); // Non-git errors never qualify. - assert!(!is_missing_working_dir_error( - &AutterError::Generic("No such file or directory".to_string()), - None - )); + assert!(!is_missing_working_dir_error(&AutterError::Generic( + "No such file or directory".to_string() + ))); } fn init_fake_repo(worktree: &Path) { @@ -9404,7 +9393,7 @@ mod tests { } #[test] - fn missing_working_dir_error_detects_any_git_failure_once_repo_is_gone() { + 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 { @@ -9417,25 +9406,16 @@ mod tests { let worktree = temp.path().join("repo"); init_fake_repo(&worktree); - assert!(!is_missing_working_dir_error( - &could_not_access, - Some(&worktree) - )); + assert!(!is_vanished_repo_error(&could_not_access, Some(&worktree))); fs::remove_dir_all(worktree.join(".git").join("objects")).unwrap(); - assert!(is_missing_working_dir_error( - &could_not_access, - Some(&worktree) - )); + assert!(is_vanished_repo_error(&could_not_access, Some(&worktree))); fs::remove_dir_all(&worktree).unwrap(); - assert!(is_missing_working_dir_error( - &could_not_access, - Some(&worktree) - )); + assert!(is_vanished_repo_error(&could_not_access, Some(&worktree))); // Non-git errors never qualify, even when the repo is gone. - assert!(!is_missing_working_dir_error( + assert!(!is_vanished_repo_error( &AutterError::Generic("boom".to_string()), Some(&worktree) ));