From 3c8781eee1c416a7548dcc5129b59ce27b64f1f3 Mon Sep 17 00:00:00 2001 From: Hashem Khalifa Date: Mon, 27 Jul 2026 03:57:04 +0200 Subject: [PATCH 1/5] fix(worktrees): avoid blocking git discovery --- src/daemon.rs | 16 +++-- src/daemon/git_watch.rs | 3 +- src/git.rs | 145 +++++++++++++++++++++++++++++++++++-- src/sessions/claude.rs | 39 +++++++--- src/sessions/codex.rs | 108 +++++++++++++++++++++++++--- src/sessions/shared.rs | 98 +++++++++++++++++++++---- src/worktree.rs | 155 ++++++++++++++++++++++++++++++++++++++++ 7 files changed, 518 insertions(+), 46 deletions(-) diff --git a/src/daemon.rs b/src/daemon.rs index 3d8368b8c..e900de1e3 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -1227,15 +1227,17 @@ async fn resolve_daemon_initialize_route( break; } } - if let Some(git_root) = crate::worktree::git_worktree_root(root) { - let git_common_dir = crate::worktree::git_common_dir(&git_root); + if let Some(identity) = crate::worktree::git_repo_identity(root) { if registry - .project_registry_context_by_identity(&git_root, git_common_dir.as_deref()) + .project_registry_context_by_identity( + &identity.worktree_root, + Some(&identity.common_dir), + ) .await .is_some() { return Some(InitializeRouteMetadata { - project_path: git_root, + project_path: identity.worktree_root, allow_init: false, }); } @@ -1258,10 +1260,10 @@ async fn resolve_daemon_initialize_route( allow_init: false, }); } - if let Some(git_root) = crate::worktree::git_worktree_root(&root) { - let allow_init = crate::config::load_sync_config(&git_root).auto_init; + if let Some(identity) = crate::worktree::git_repo_identity(&root) { + let allow_init = crate::config::load_sync_config(&identity.worktree_root).auto_init; return Some(InitializeRouteMetadata { - project_path: git_root, + project_path: identity.worktree_root, allow_init, }); } diff --git a/src/daemon/git_watch.rs b/src/daemon/git_watch.rs index 334dea7bb..86d803a44 100644 --- a/src/daemon/git_watch.rs +++ b/src/daemon/git_watch.rs @@ -467,13 +467,14 @@ async fn supervise_project(inner: Arc, state: Arc) /// debounce raw events into coalesced syncs. On watcher construction/death, /// fall back to a 5-minute mtime poll for THIS project only. async fn project_task(inner: Arc, state: Arc) { - let Some(common_dir) = crate::worktree::git_common_dir(&state.project_root) else { + let Some(identity) = crate::worktree::git_repo_identity(&state.project_root) else { // Not a resolvable git repo (yet). Degrade to polling so a later `git // init` / clone is still eventually covered. state.health.set_degraded(true); degraded_poll_loop(&inner, &state, None).await; return; }; + let common_dir = identity.common_dir; // Build the raw watcher. Its callback pushes into the dirty set and wakes // the debounce loop — it never blocks and never syncs inline. diff --git a/src/git.rs b/src/git.rs index 9ae6d2f09..30b992a97 100644 --- a/src/git.rs +++ b/src/git.rs @@ -6,19 +6,21 @@ //! absolute path exactly once (cached in a [`OnceLock`]) and hands every product //! spawn site that cached path, so the long-running daemon never re-walks `PATH`. //! -//! The gix-first read paths in [`crate::branch`] and [`crate::worktree`] are -//! unaffected: they still prefer in-process `gix` and only reach a `git` -//! subprocess as a gated fallback. This module only changes *which* program those -//! fallbacks (and the one-shot spawn sites) exec. +//! The public gix-first read paths in [`crate::branch`] and [`crate::worktree`] +//! are unaffected: they still prefer in-process `gix` and only reach a `git` +//! subprocess as a gated fallback. use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; -use std::process::{Command, Output}; +use std::process::{Child, Command, Output, Stdio}; use std::sync::OnceLock; +use std::time::{Duration, Instant}; /// The literal used when resolution fails, preserving today's behavior (the OS /// PATH-walks per spawn, but callers keep working). const GIT_LITERAL: &str = "git"; +const GIT_CAPTURE_AT_TIMEOUT: Duration = Duration::from_secs(2); +const CHILD_WAIT_POLL_INTERVAL: Duration = Duration::from_millis(10); /// Returns the resolved `git` program to spawn, as a cached `&'static OsStr`. /// @@ -129,6 +131,92 @@ pub(crate) fn git_capture(repo_root: &Path, args: &[&str]) -> Option { (!trimmed.is_empty()).then(|| trimmed.to_string()) } +/// Outcome of the bounded `git -C` capture used by repository identity lookup. +#[derive(Debug)] +pub(crate) enum GitCaptureAtResult { + Captured(String), + Failed, + TimedOut, +} + +/// Runs `git -C ` without setting the child process working +/// directory to `repo_root`. +/// +/// Some network-backed or otherwise unhealthy project roots can block inside +/// the child's initial `getcwd` when passed through [`Command::current_dir`]. +/// Git's `-C` resolves the repository after process startup and avoids that +/// pre-argument cwd lookup. The child is killed and reaped at the hard deadline. +pub(crate) fn git_capture_at(repo_root: &Path, args: &[&str]) -> GitCaptureAtResult { + let mut command = git_command_at(repo_root, args); + command.stdout(Stdio::piped()).stderr(Stdio::piped()); + let Ok(child) = command.spawn() else { + return GitCaptureAtResult::Failed; + }; + match capture_child_with_deadline(child, GIT_CAPTURE_AT_TIMEOUT) { + ChildCaptureResult::Completed(output) if output.status.success() => { + let Ok(text) = String::from_utf8(output.stdout) else { + return GitCaptureAtResult::Failed; + }; + let trimmed = text.trim(); + if trimmed.is_empty() { + GitCaptureAtResult::Failed + } else { + GitCaptureAtResult::Captured(trimmed.to_string()) + } + } + ChildCaptureResult::TimedOut => GitCaptureAtResult::TimedOut, + ChildCaptureResult::Completed(_) | ChildCaptureResult::Failed => GitCaptureAtResult::Failed, + } +} + +fn git_command_at(repo_root: &Path, args: &[&str]) -> Command { + let mut command = Command::new(git_program()); + command.arg("-C").arg(repo_root).args(args); + command +} + +#[derive(Debug)] +enum ChildCaptureResult { + Completed(Output), + Failed, + TimedOut, +} + +fn capture_child_with_deadline(mut child: Child, timeout: Duration) -> ChildCaptureResult { + let deadline = Instant::now() + timeout; + loop { + match child.try_wait() { + Ok(Some(_)) => { + return child + .wait_with_output() + .map(ChildCaptureResult::Completed) + .unwrap_or(ChildCaptureResult::Failed); + } + Ok(None) => {} + Err(_) => { + let _ = child.kill(); + let _ = child.wait(); + return ChildCaptureResult::Failed; + } + } + + let now = Instant::now(); + if now >= deadline { + let _ = child.kill(); + return if child.wait().is_ok() { + ChildCaptureResult::TimedOut + } else { + ChildCaptureResult::Failed + }; + } + std::thread::sleep( + deadline + .saturating_duration_since(now) + .min(CHILD_WAIT_POLL_INTERVAL), + ); + } +} + #[cfg(test)] #[allow(clippy::unwrap_used, clippy::expect_used)] mod tests { @@ -151,6 +239,53 @@ mod tests { ); } + #[test] + fn git_at_command_uses_dash_c_without_target_current_dir() { + let repo_root = Path::new("/problematic/project/root"); + let command = git_command_at( + repo_root, + &["rev-parse", "--show-toplevel", "--git-common-dir"], + ); + + assert!( + command.get_current_dir().is_none(), + "git -C must inherit the safe daemon cwd instead of entering the target root" + ); + assert_eq!( + command + .get_args() + .map(std::ffi::OsStr::to_os_string) + .collect::>(), + vec![ + OsString::from("-C"), + repo_root.as_os_str().to_os_string(), + OsString::from("rev-parse"), + OsString::from("--show-toplevel"), + OsString::from("--git-common-dir"), + ] + ); + } + + #[cfg(unix)] + #[test] + fn git_capture_deadline_kills_and_reaps_child() { + let child = Command::new("/bin/sleep") + .arg("30") + .spawn() + .expect("spawn sleeping child"); + let started = std::time::Instant::now(); + + let result = capture_child_with_deadline(child, std::time::Duration::from_millis(25)); + + let ChildCaptureResult::TimedOut = result else { + panic!("sleeping child should time out, got {result:?}"); + }; + assert!( + started.elapsed() < std::time::Duration::from_secs(2), + "deadline must stop and reap the child promptly" + ); + } + #[test] fn git_env_override_is_honored() { // resolve_git_program() reads GIT directly; test it in isolation so the diff --git a/src/sessions/claude.rs b/src/sessions/claude.rs index d6639585e..b867e8e8d 100644 --- a/src/sessions/claude.rs +++ b/src/sessions/claude.rs @@ -26,10 +26,9 @@ use serde_json::{Map, Value}; use crate::accounting::parser::parse_timestamp; use crate::sessions::SessionMessageRecord; use crate::sessions::shared::{ - StoredCursor, TranscriptLocation, TranscriptLocationMetadataKeys, append_location_metadata, - append_tool_calls_metadata, append_tool_event_metadata, append_usage_metadata, - content_storage_text_and_tools, path_belongs_to_project, preview_truncated, - title_from_messages, + ProjectRootMatcherCache, StoredCursor, TranscriptLocation, TranscriptLocationMetadataKeys, + append_location_metadata, append_tool_calls_metadata, append_tool_event_metadata, + append_usage_metadata, content_storage_text_and_tools, preview_truncated, title_from_messages, }; use crate::sessions::source::{ ParsedTranscript, SessionDraft, TranscriptSource, collect_files_with_ext, stream_new_jsonl, @@ -76,6 +75,7 @@ pub(crate) const CWD_PROBE_LINES: usize = 8; pub struct ClaudeSource { projects_dir: PathBuf, user_scope: Option, + project_matchers: ProjectRootMatcherCache, } struct UserClaudeScope { @@ -96,6 +96,7 @@ impl ClaudeSource { Self { projects_dir: home.join(".claude").join("projects"), user_scope: None, + project_matchers: ProjectRootMatcherCache::default(), } } @@ -191,21 +192,37 @@ impl TranscriptSource for ClaudeSource { // summary alongside the per-row marker rows / metadata. let mut accumulator = SessionAccumulator::default(); let mut messages = Vec::new(); + let project_matcher = self + .user_scope + .is_none() + .then(|| self.project_matchers.get(project_root)); + let registered_root_matchers = self + .user_scope + .as_ref() + .map(|scope| { + scope + .registered_roots + .iter() + .map(|root| self.project_matchers.get(root)) + .collect::>() + }) + .unwrap_or_default(); for line in &new.lines { let record = &line.value; let line_cwd = record_cwd(record).or_else(|| session_cwd.clone()); let include = self.user_scope.as_ref().map_or_else( || { - line_cwd - .as_deref() - .is_some_and(|cwd| path_belongs_to_project(cwd, project_root)) + line_cwd.as_deref().is_some_and(|cwd| { + project_matcher + .as_ref() + .is_some_and(|matcher| matcher.contains(cwd)) + }) }, - |scope| { + |_scope| { line_cwd.as_deref().is_none_or(|cwd| { - !scope - .registered_roots + !registered_root_matchers .iter() - .any(|root| path_belongs_to_project(cwd, root)) + .any(|matcher| matcher.contains(cwd)) }) }, ); diff --git a/src/sessions/codex.rs b/src/sessions/codex.rs index e2d3250d7..814009fce 100644 --- a/src/sessions/codex.rs +++ b/src/sessions/codex.rs @@ -56,8 +56,8 @@ use serde_json::Value; use crate::accounting::parser::parse_timestamp; use crate::sessions::SessionMessageRecord; use crate::sessions::shared::{ - StoredCursor, append_tool_calls_metadata, content_storage_text_and_tools, - path_belongs_to_project, title_from_messages, + ProjectRootMatcherCache, StoredCursor, append_tool_calls_metadata, + content_storage_text_and_tools, title_from_messages, }; use crate::sessions::source::{ ParsedTranscript, SessionDraft, TranscriptSource, collect_files_with_ext, stream_new_jsonl, @@ -94,6 +94,7 @@ pub struct CodexSource { sessions_dir: PathBuf, archived_sessions_dir: PathBuf, user_scope: Option, + project_matchers: ProjectRootMatcherCache, } struct UserCodexScope { @@ -116,6 +117,7 @@ impl CodexSource { sessions_dir: codex_home.join("sessions"), archived_sessions_dir: codex_home.join("archived_sessions"), user_scope: None, + project_matchers: ProjectRootMatcherCache::default(), } } @@ -185,23 +187,38 @@ impl TranscriptSource for CodexSource { } else { CodexContextState::from_meta(&meta) }; + let project_matcher = self + .user_scope + .is_none() + .then(|| self.project_matchers.get(project_root)); + let registered_root_matchers = self + .user_scope + .as_ref() + .map(|scope| { + scope + .registered_roots + .iter() + .map(|root| self.project_matchers.get(root)) + .collect::>() + }) + .unwrap_or_default(); let mut last_in_scope_cwd = None; let mut last_in_scope_git = None; for line in &new.lines { let is_context_record = context_state.observe_context_record(&line.value, path, &meta); let in_scope = self.user_scope.as_ref().map_or_else( || { - context_state - .cwd - .as_deref() - .is_some_and(|cwd| path_belongs_to_project(cwd, project_root)) + context_state.cwd.as_deref().is_some_and(|cwd| { + project_matcher + .as_ref() + .is_some_and(|matcher| matcher.contains(cwd)) + }) }, - |scope| { + |_scope| { context_state.cwd.as_deref().is_none_or(|cwd| { - !scope - .registered_roots + !registered_root_matchers .iter() - .any(|root| path_belongs_to_project(cwd, root)) + .any(|matcher| matcher.contains(cwd)) }) }, ); @@ -1576,3 +1593,74 @@ mod goal_event_tests { assert!(codex_goal_event_from_line(&user).is_none()); } } + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +mod source_matcher_cache_tests { + use super::*; + use serde_json::json; + use tempfile::TempDir; + + fn write_rollout(path: &Path, session_id: &str, cwd: &Path) { + let lines = [ + json!({ + "timestamp": "2026-01-01T00:00:00.000Z", + "type": "session_meta", + "payload": { + "id": session_id, + "cwd": cwd, + "model": "gpt-5.5" + } + }), + json!({ + "timestamp": "2026-01-01T00:00:01.000Z", + "type": "event_msg", + "payload": { + "type": "user_message", + "message": format!("message from {session_id}") + } + }), + ]; + std::fs::write( + path, + lines + .iter() + .map(ToString::to_string) + .collect::>() + .join("\n") + + "\n", + ) + .unwrap(); + } + + #[test] + fn codex_source_reuses_project_matcher_across_parse_calls() { + let temp = TempDir::new().unwrap(); + let project_root = temp.path().join("repo"); + let nested_cwd = project_root.join("packages/app"); + std::fs::create_dir_all(&nested_cwd).unwrap(); + let status = std::process::Command::new("git") + .args(["init", "-q"]) + .current_dir(&project_root) + .status() + .unwrap(); + assert!(status.success()); + + let first_path = temp.path().join("first.jsonl"); + let second_path = temp.path().join("second.jsonl"); + write_rollout(&first_path, "first-session", &nested_cwd); + write_rollout(&second_path, "second-session", &nested_cwd); + let source = CodexSource::with_home(temp.path()); + + let first = source + .parse_new(&first_path, StoredCursor::default(), &project_root, None) + .unwrap(); + assert_eq!(first.messages.len(), 1); + + std::fs::rename(project_root.join(".git"), project_root.join(".git.hidden")).unwrap(); + let second = source + .parse_new(&second_path, StoredCursor::default(), &project_root, None) + .unwrap(); + assert_eq!(second.messages.len(), 1); + } +} diff --git a/src/sessions/shared.rs b/src/sessions/shared.rs index 6999dbc07..a1264c8c1 100644 --- a/src/sessions/shared.rs +++ b/src/sessions/shared.rs @@ -4,8 +4,10 @@ //! file-backed [`crate::sessions::source`] drivers and the Hermes `SQLite` sweep //! both depend on them so they do not need to import from each other. +use std::collections::HashMap; use std::io; use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; use serde_json::Value; @@ -139,43 +141,66 @@ pub(crate) fn path_belongs_to_project(path: &Path, project_root: &Path) -> bool /// re-run `git_worktree_root`/`git_common_dir` on the fixed project side. A /// single [`ProjectRootMatcher::contains`] call is exactly equivalent to /// [`path_belongs_to_project`], which is a thin wrapper over it. +#[derive(Debug)] pub(crate) struct ProjectRootMatcher { root: PathBuf, worktree: Option, common_dir: Option, + path_membership: Mutex>, } impl ProjectRootMatcher { /// Resolve the fixed project-side git identity once. pub(crate) fn new(project_root: &Path) -> Self { + let identity = crate::worktree::git_repo_identity(project_root); Self { root: project_root.to_path_buf(), - worktree: crate::worktree::git_worktree_root(project_root), - common_dir: crate::worktree::git_common_dir(project_root), + worktree: identity + .as_ref() + .map(|identity| identity.worktree_root.clone()), + common_dir: identity.map(|identity| identity.common_dir), + path_membership: Mutex::new(HashMap::new()), } } /// True when `path` belongs to this project: it is the root, shares the /// project's git worktree or common dir, or discovers back to the root. - /// Only the varying `path` side is git-resolved here. + /// Each distinct path is resolved once for this matcher, so repeated + /// transcript rows with the same cwd do not repeatedly discover/open git. pub(crate) fn contains(&self, path: &Path) -> bool { + if let Some(belongs) = self + .path_membership + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .get(path) + .copied() + { + return belongs; + } + + let belongs = self.contains_uncached(path); + self.path_membership + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .insert(path.to_path_buf(), belongs); + belongs + } + + fn contains_uncached(&self, path: &Path) -> bool { if paths_equal(path, &self.root) { return true; } - if let (Some(path_worktree), Some(project_worktree)) = ( - crate::worktree::git_worktree_root(path).as_ref(), + if let (Some(path_identity), Some(project_worktree)) = ( + crate::worktree::git_repo_identity(path), self.worktree.as_ref(), ) { - if paths_equal(path_worktree, project_worktree) { + if paths_equal(&path_identity.worktree_root, project_worktree) { return true; } - return crate::worktree::git_common_dir(path) - .as_ref() - .zip(self.common_dir.as_ref()) - .is_some_and(|(path_common, project_common)| { - paths_equal(path_common, project_common) - }); + return self.common_dir.as_ref().is_some_and(|project_common| { + paths_equal(&path_identity.common_dir, project_common) + }); } crate::config::discover_project_root(path) @@ -184,6 +209,30 @@ impl ProjectRootMatcher { } } +/// Source-lifetime cache of project matchers keyed by canonical project root. +/// +/// A source parses many transcript files for the same project. Keeping the +/// matcher here avoids reopening the same git repository once per file while +/// retaining per-path membership caching inside [`ProjectRootMatcher`]. +#[derive(Clone, Debug, Default)] +pub(crate) struct ProjectRootMatcherCache { + matchers: Arc>>>, +} + +impl ProjectRootMatcherCache { + pub(crate) fn get(&self, project_root: &Path) -> Arc { + let key = project_root + .canonicalize() + .unwrap_or_else(|_| project_root.to_path_buf()); + self.matchers + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .entry(key) + .or_insert_with(|| Arc::new(ProjectRootMatcher::new(project_root))) + .clone() + } +} + #[cfg(windows)] fn normalized_paths_equal(a: &Path, b: &Path) -> bool { fn normalize(path: &Path) -> String { @@ -551,10 +600,35 @@ pub(crate) fn title_from_messages(messages: &[SessionMessageRecord]) -> Option Option { + if !git_may_resolve_repo(dir) { + return None; + } + resolve_git_identity_with( + dir, + || crate::git::git_capture_at(dir, &["rev-parse", "--show-toplevel", "--git-common-dir"]), + || git_repo_identity_from_gix(dir), + ) +} + +fn resolve_git_identity_with( + dir: &Path, + cli_output: impl FnOnce() -> crate::git::GitCaptureAtResult, + gix_fallback: impl FnOnce() -> Option, +) -> Option { + match cli_output() { + crate::git::GitCaptureAtResult::Captured(output) => { + git_repo_identity_from_cli_output(dir, &output).or_else(gix_fallback) + } + crate::git::GitCaptureAtResult::Failed => gix_fallback(), + crate::git::GitCaptureAtResult::TimedOut => None, + } +} + +fn git_repo_identity_from_cli_output(dir: &Path, output: &str) -> Option { + let mut lines = output.lines(); + let worktree_root = PathBuf::from(lines.next()?.trim()); + let common_dir = PathBuf::from(lines.next()?.trim()); + if worktree_root.as_os_str().is_empty() || common_dir.as_os_str().is_empty() { + return None; + } + let worktree_root = if worktree_root.is_absolute() { + worktree_root + } else { + dir.join(worktree_root) + }; + let common_dir = if common_dir.is_absolute() { + common_dir + } else { + dir.join(common_dir) + }; + Some(GitRepoIdentity { + worktree_root: realpath(&worktree_root)?, + common_dir: common_dir.canonicalize().unwrap_or(common_dir), + }) +} + +fn git_repo_identity_from_gix(dir: &Path) -> Option { + let repo = gix::discover(dir).ok()?; + let worktree_root = realpath(repo.workdir()?)?; + let common_dir = repo.common_dir().to_path_buf(); + let common_dir = if common_dir.is_absolute() { + common_dir + } else { + dir.join(common_dir) + }; + Some(GitRepoIdentity { + worktree_root, + common_dir: common_dir.canonicalize().unwrap_or(common_dir), + }) +} + /// Absolute, symlink-resolved toplevel of the git working tree that `dir` /// belongs to, or `None` when `dir` isn't inside a git repo (or `git` is /// missing on PATH). @@ -228,6 +305,84 @@ mod tests { assert!(detect_worktree_index_mismatch(&sub, &project).is_none()); } + #[test] + fn paired_cli_identity_resolves_relative_common_dir_without_gix() { + let tmp = tempdir().unwrap(); + let worktree = tmp.path().join("worktree"); + let nested = worktree.join("src/deep"); + let common_dir = tmp.path().join("main/.git"); + fs::create_dir_all(&nested).unwrap(); + fs::create_dir_all(&common_dir).unwrap(); + let output = format!("{}\n../../../main/.git", worktree.display()); + + let identity = resolve_git_identity_with( + &nested, + || crate::git::GitCaptureAtResult::Captured(output), + || panic!("valid CLI identity must short-circuit gix discovery"), + ) + .expect("paired CLI identity"); + + assert_eq!( + identity.worktree_root, + std::fs::canonicalize(&worktree).unwrap() + ); + assert_eq!( + identity.common_dir, + std::fs::canonicalize(&common_dir).unwrap() + ); + } + + #[test] + fn timed_out_cli_identity_does_not_fallback_to_gix() { + let tmp = tempdir().unwrap(); + let identity = resolve_git_identity_with( + tmp.path(), + || crate::git::GitCaptureAtResult::TimedOut, + || panic!("timed-out CLI identity must not fall through to gix"), + ); + assert!(identity.is_none()); + } + + #[test] + fn paired_identity_resolves_nested_linked_worktree() { + let tmp = tempdir().unwrap(); + let main = tmp.path().join("main"); + fs::create_dir_all(&main).unwrap(); + run_git(&main, &["init", "--quiet"]); + fs::write(main.join("README.md"), "hi").unwrap(); + run_git(&main, &["add", "."]); + run_git( + &main, + &[ + "-c", + "user.email=t@t", + "-c", + "user.name=t", + "commit", + "--quiet", + "-m", + "init", + ], + ); + let worktree = tmp.path().join("wt"); + run_git( + &main, + &["worktree", "add", "--detach", worktree.to_str().unwrap()], + ); + let nested = worktree.join("src/deep"); + fs::create_dir_all(&nested).unwrap(); + + let identity = git_repo_identity(&nested).expect("linked worktree identity"); + assert_eq!( + identity.worktree_root, + std::fs::canonicalize(&worktree).unwrap() + ); + assert_eq!( + identity.common_dir, + std::fs::canonicalize(main.join(".git")).unwrap() + ); + } + #[test] fn flags_mismatch_when_started_from_linked_worktree() { // Two real git working trees: a main checkout and a linked From 9ade11aa99382e31be53c1f9a7ee7b8f074d6e60 Mon Sep 17 00:00:00 2001 From: Hashem Khalifa Date: Mon, 27 Jul 2026 12:20:14 +0200 Subject: [PATCH 2/5] perf(sessions): cache worktree metadata resolution --- src/sessions/claude.rs | 76 +++++++++++++++++++++++-- src/sessions/codex.rs | 16 ++++++ src/sessions/codex/context.rs | 11 +++- src/sessions/shared.rs | 101 +++++++++++++++++++++++++++++++++- 4 files changed, 193 insertions(+), 11 deletions(-) diff --git a/src/sessions/claude.rs b/src/sessions/claude.rs index b867e8e8d..51b8bc7b4 100644 --- a/src/sessions/claude.rs +++ b/src/sessions/claude.rs @@ -27,7 +27,7 @@ use crate::accounting::parser::parse_timestamp; use crate::sessions::SessionMessageRecord; use crate::sessions::shared::{ ProjectRootMatcherCache, StoredCursor, TranscriptLocation, TranscriptLocationMetadataKeys, - append_location_metadata, append_tool_calls_metadata, append_tool_event_metadata, + append_location_metadata_cached, append_tool_calls_metadata, append_tool_event_metadata, append_usage_metadata, content_storage_text_and_tools, preview_truncated, title_from_messages, }; use crate::sessions::source::{ @@ -240,6 +240,7 @@ impl TranscriptSource for ClaudeSource { line.offset, session_cwd.as_deref(), &mut accumulator, + &self.project_matchers, ) .or_else(|| { system_hook_message_from_line( @@ -289,6 +290,7 @@ impl TranscriptSource for ClaudeSource { session_cwd.as_deref(), subagent.as_ref(), &accumulator, + &self.project_matchers, )) .ok(), parent_session_id: subagent.as_ref().map(|info| info.parent_session_id.clone()), @@ -517,6 +519,7 @@ fn message_from_line( offset: i64, session_cwd: Option<&Path>, accumulator: &mut SessionAccumulator, + location_cache: &ProjectRootMatcherCache, ) -> Option { let kind = record.get("type").and_then(Value::as_str)?; if kind != "user" && kind != "assistant" { @@ -590,6 +593,7 @@ fn message_from_line( content, session_cwd, accumulator, + location_cache, )) .ok(), }) @@ -1123,16 +1127,18 @@ fn session_metadata( session_cwd: Option<&Path>, subagent: Option<&ClaudeSubagentInfo>, accumulator: &SessionAccumulator, + location_cache: &ProjectRootMatcherCache, ) -> Value { let mut metadata = Map::new(); metadata.insert( "source".to_string(), Value::String("claude_transcript".to_string()), ); - append_location_metadata( + append_location_metadata_cached( &mut metadata, CLAUDE_SESSION_LOCATION_KEYS, TranscriptLocation::new(session_cwd, "transcript_session"), + location_cache, ); // Subagent spawn provenance (from the sibling agent-.meta.json and the @@ -1184,6 +1190,7 @@ fn message_metadata( content: &Value, session_cwd: Option<&Path>, accumulator: &mut SessionAccumulator, + location_cache: &ProjectRootMatcherCache, ) -> Value { let mut metadata = Map::new(); metadata.insert( @@ -1197,10 +1204,11 @@ fn message_metadata( } else { (session_cwd, "transcript_session") }; - append_location_metadata( + append_location_metadata_cached( &mut metadata, CLAUDE_MESSAGE_LOCATION_KEYS, TranscriptLocation::new(location_cwd, location_provenance), + location_cache, ); if let Some(branch) = record .get("gitBranch") @@ -1412,8 +1420,16 @@ mod tests { let path = Path::new("/tmp/sess.jsonl"); let mut accumulator = SessionAccumulator::default(); - let message = message_from_line(&record, "sess", path, 10, None, &mut accumulator) - .expect("assistant message row"); + let message = message_from_line( + &record, + "sess", + path, + 10, + None, + &mut accumulator, + &ProjectRootMatcherCache::default(), + ) + .expect("assistant message row"); assert_eq!(message.message_id, "msg_1"); assert_eq!(message.kind.as_deref(), Some("message")); assert!(!message.text.contains("First I inspect the parser")); @@ -1442,6 +1458,56 @@ mod tests { assert!(metadata.get("redacted_thinking_blocks").is_none()); } + #[test] + fn claude_message_metadata_reuses_worktree_for_repeated_cwd() { + let temp = tempfile::TempDir::new().expect("temp dir"); + let project_root = temp.path().join("repo"); + let nested_cwd = project_root.join("packages/app"); + std::fs::create_dir_all(&nested_cwd).expect("nested cwd"); + let status = std::process::Command::new("git") + .args(["init", "-q"]) + .current_dir(&project_root) + .status() + .expect("git init"); + assert!(status.success()); + + let record = assistant_record(&json!([{"type": "text", "text": "Repeated cwd metadata."}])); + let path = temp.path().join("session.jsonl"); + let cache = ProjectRootMatcherCache::default(); + let mut accumulator = SessionAccumulator::default(); + let first = message_from_line( + &record, + "sess", + &path, + 10, + Some(&nested_cwd), + &mut accumulator, + &cache, + ) + .expect("first message"); + let first_metadata: Value = + serde_json::from_str(first.metadata_json.as_deref().unwrap()).unwrap(); + let first_worktree = first_metadata["claude_message_worktree"].clone(); + assert!(first_worktree.is_string()); + + std::fs::rename(project_root.join(".git"), project_root.join(".git.hidden")) + .expect("hide git metadata after first lookup"); + + let second = message_from_line( + &record, + "sess", + &path, + 20, + Some(&nested_cwd), + &mut accumulator, + &cache, + ) + .expect("second message"); + let second_metadata: Value = + serde_json::from_str(second.metadata_json.as_deref().unwrap()).unwrap(); + assert_eq!(second_metadata["claude_message_worktree"], first_worktree); + } + #[test] fn redacted_only_thinking_records_no_reasoning_row() { // Matches Codex's encrypted-reasoning convention: no plaintext, no row. diff --git a/src/sessions/codex.rs b/src/sessions/codex.rs index 814009fce..480708411 100644 --- a/src/sessions/codex.rs +++ b/src/sessions/codex.rs @@ -260,6 +260,7 @@ impl TranscriptSource for CodexSource { &mut message, context_state.cwd.as_deref(), context_state.git.as_ref(), + &self.project_matchers, ); messages.push(message); } @@ -283,6 +284,7 @@ impl TranscriptSource for CodexSource { &mut message, context_state.cwd.as_deref(), context_state.git.as_ref(), + &self.project_matchers, ); messages.push(message); continue; @@ -298,6 +300,7 @@ impl TranscriptSource for CodexSource { &mut message, context_state.cwd.as_deref(), context_state.git.as_ref(), + &self.project_matchers, ); messages.push(message); continue; @@ -313,6 +316,7 @@ impl TranscriptSource for CodexSource { &mut message, context_state.cwd.as_deref(), context_state.git.as_ref(), + &self.project_matchers, ); messages.push(message); continue; @@ -331,6 +335,7 @@ impl TranscriptSource for CodexSource { &mut message, context_state.cwd.as_deref(), context_state.git.as_ref(), + &self.project_matchers, ); messages.push(message); continue; @@ -346,6 +351,7 @@ impl TranscriptSource for CodexSource { &mut message, context_state.cwd.as_deref(), context_state.git.as_ref(), + &self.project_matchers, ); messages.push(message); continue; @@ -366,6 +372,7 @@ impl TranscriptSource for CodexSource { &mut message, context_state.cwd.as_deref(), context_state.git.as_ref(), + &self.project_matchers, ); messages.push(message); } @@ -380,6 +387,7 @@ impl TranscriptSource for CodexSource { &mut message, last_in_scope_cwd.as_deref(), last_in_scope_git.as_ref(), + &self.project_matchers, ); messages.push(message); } @@ -399,6 +407,7 @@ impl TranscriptSource for CodexSource { metadata_json: context::session_metadata_json( &meta, self.user_scope.is_none().then_some(&structured.summary), + &self.project_matchers, ), parent_session_id: meta.parent_session_id.clone(), is_subagent: meta.is_subagent, @@ -1656,11 +1665,18 @@ mod source_matcher_cache_tests { .parse_new(&first_path, StoredCursor::default(), &project_root, None) .unwrap(); assert_eq!(first.messages.len(), 1); + let first_metadata: Value = + serde_json::from_str(first.messages[0].metadata_json.as_deref().unwrap()).unwrap(); + let first_worktree = first_metadata["codex_turn_worktree"].clone(); + assert!(first_worktree.is_string()); std::fs::rename(project_root.join(".git"), project_root.join(".git.hidden")).unwrap(); let second = source .parse_new(&second_path, StoredCursor::default(), &project_root, None) .unwrap(); assert_eq!(second.messages.len(), 1); + let second_metadata: Value = + serde_json::from_str(second.messages[0].metadata_json.as_deref().unwrap()).unwrap(); + assert_eq!(second_metadata["codex_turn_worktree"], first_worktree); } } diff --git a/src/sessions/codex/context.rs b/src/sessions/codex/context.rs index 93690ed72..8ff6fb383 100644 --- a/src/sessions/codex/context.rs +++ b/src/sessions/codex/context.rs @@ -6,7 +6,8 @@ use serde_json::Value; use super::{CodexMeta, session_meta_from_record, turn_context_from_record}; use crate::sessions::SessionMessageRecord; use crate::sessions::shared::{ - TranscriptLocation, TranscriptLocationMetadataKeys, append_location_metadata, + ProjectRootMatcherCache, TranscriptLocation, TranscriptLocationMetadataKeys, + append_location_metadata_cached, }; const CODEX_SESSION_LOCATION_KEYS: TranscriptLocationMetadataKeys = @@ -113,6 +114,7 @@ impl CodexContextState { pub(super) fn session_metadata_json( meta: &CodexMeta, summary: Option<&super::events::CodexSessionSummary>, + location_cache: &ProjectRootMatcherCache, ) -> Option { let mut metadata = serde_json::Map::new(); metadata.insert( @@ -134,10 +136,11 @@ pub(super) fn session_metadata_json( Value::String(agent_nickname.clone()), ); } - append_location_metadata( + append_location_metadata_cached( &mut metadata, CODEX_SESSION_LOCATION_KEYS, TranscriptLocation::new(Some(&meta.cwd), "session_meta"), + location_cache, ); insert_git_metadata(&mut metadata, meta.git.as_ref()); if let Some(summary) = summary { @@ -150,6 +153,7 @@ pub(super) fn annotate_message( message: &mut SessionMessageRecord, cwd: Option<&Path>, git: Option<&Value>, + location_cache: &ProjectRootMatcherCache, ) { let mut metadata = message .metadata_json @@ -161,10 +165,11 @@ pub(super) fn annotate_message( }) .unwrap_or_default(); - append_location_metadata( + append_location_metadata_cached( &mut metadata, CODEX_TURN_LOCATION_KEYS, TranscriptLocation::new(cwd, "codex_context"), + location_cache, ); insert_git_metadata(&mut metadata, git); message.metadata_json = serde_json::to_string(&Value::Object(metadata)).ok(); diff --git a/src/sessions/shared.rs b/src/sessions/shared.rs index a1264c8c1..83bf3afed 100644 --- a/src/sessions/shared.rs +++ b/src/sessions/shared.rs @@ -7,7 +7,7 @@ use std::collections::HashMap; use std::io; use std::path::{Path, PathBuf}; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, OnceLock}; use serde_json::Value; @@ -217,6 +217,7 @@ impl ProjectRootMatcher { #[derive(Clone, Debug, Default)] pub(crate) struct ProjectRootMatcherCache { matchers: Arc>>>, + location_worktrees: Arc>>>>>, } impl ProjectRootMatcherCache { @@ -231,6 +232,27 @@ impl ProjectRootMatcherCache { .or_insert_with(|| Arc::new(ProjectRootMatcher::new(project_root))) .clone() } + + /// Resolve a transcript cwd's worktree once for this ingest source. + /// + /// Location metadata is added per message, so one transcript can otherwise + /// repeat git discovery thousands of times for the same cwd. Keep this + /// source-lifetime like the project matchers and use the bounded CLI-first + /// identity path instead of opening the repository object database. + pub(crate) fn git_worktree_root(&self, cwd: &Path) -> Option { + let resolution = self + .location_worktrees + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .entry(cwd.to_path_buf()) + .or_insert_with(|| Arc::new(OnceLock::new())) + .clone(); + resolution + .get_or_init(|| { + crate::worktree::git_repo_identity(cwd).map(|identity| identity.worktree_root) + }) + .clone() + } } #[cfg(windows)] @@ -441,6 +463,34 @@ pub(crate) fn append_location_metadata( map: &mut serde_json::Map, keys: TranscriptLocationMetadataKeys, location: TranscriptLocation<'_>, +) { + append_location_metadata_with_worktree( + map, + keys, + location, + location.cwd.and_then(crate::worktree::git_worktree_root), + ); +} + +pub(crate) fn append_location_metadata_cached( + map: &mut serde_json::Map, + keys: TranscriptLocationMetadataKeys, + location: TranscriptLocation<'_>, + cache: &ProjectRootMatcherCache, +) { + append_location_metadata_with_worktree( + map, + keys, + location, + location.cwd.and_then(|cwd| cache.git_worktree_root(cwd)), + ); +} + +fn append_location_metadata_with_worktree( + map: &mut serde_json::Map, + keys: TranscriptLocationMetadataKeys, + location: TranscriptLocation<'_>, + worktree: Option, ) { let Some(cwd) = location.cwd else { return; @@ -449,7 +499,7 @@ pub(crate) fn append_location_metadata( keys.cwd.to_string(), Value::String(cwd.to_string_lossy().to_string()), ); - if let Some(worktree) = crate::worktree::git_worktree_root(cwd) { + if let Some(worktree) = worktree { map.insert( keys.worktree.to_string(), Value::String(worktree.to_string_lossy().to_string()), @@ -599,13 +649,58 @@ pub(crate) fn title_from_messages(messages: &[SessionMessageRecord]) -> Option Date: Mon, 27 Jul 2026 12:44:04 +0200 Subject: [PATCH 3/5] fix(sessions): preserve unknown worktree membership --- src/sessions/claude.rs | 102 +++++++++++++++---- src/sessions/codex.rs | 85 ++++++++++++---- src/sessions/shared.rs | 216 ++++++++++++++++++++++++++++++++++------- src/worktree.rs | 52 +++++++--- 4 files changed, 374 insertions(+), 81 deletions(-) diff --git a/src/sessions/claude.rs b/src/sessions/claude.rs index 51b8bc7b4..2f9b441c1 100644 --- a/src/sessions/claude.rs +++ b/src/sessions/claude.rs @@ -26,9 +26,10 @@ use serde_json::{Map, Value}; use crate::accounting::parser::parse_timestamp; use crate::sessions::SessionMessageRecord; use crate::sessions::shared::{ - ProjectRootMatcherCache, StoredCursor, TranscriptLocation, TranscriptLocationMetadataKeys, - append_location_metadata_cached, append_tool_calls_metadata, append_tool_event_metadata, - append_usage_metadata, content_storage_text_and_tools, preview_truncated, title_from_messages, + ProjectMembership, ProjectRootMatcherCache, StoredCursor, TranscriptLocation, + TranscriptLocationMetadataKeys, append_location_metadata_cached, append_tool_calls_metadata, + append_tool_event_metadata, append_usage_metadata, content_storage_text_and_tools, + preview_truncated, title_from_messages, }; use crate::sessions::source::{ ParsedTranscript, SessionDraft, TranscriptSource, collect_files_with_ext, stream_new_jsonl, @@ -210,22 +211,26 @@ impl TranscriptSource for ClaudeSource { for line in &new.lines { let record = &line.value; let line_cwd = record_cwd(record).or_else(|| session_cwd.clone()); - let include = self.user_scope.as_ref().map_or_else( - || { - line_cwd.as_deref().is_some_and(|cwd| { - project_matcher - .as_ref() - .is_some_and(|matcher| matcher.contains(cwd)) - }) - }, - |_scope| { - line_cwd.as_deref().is_none_or(|cwd| { - !registered_root_matchers - .iter() - .any(|matcher| matcher.contains(cwd)) - }) - }, - ); + let include = if self.user_scope.is_none() { + line_cwd.as_deref().map_or(Some(false), |cwd| { + project_matcher + .as_ref() + .map(|matcher| matcher.contains_status(cwd).definitive()) + .unwrap_or(Some(false)) + }) + } else { + line_cwd.as_deref().map_or(Some(true), |cwd| { + let mut unknown = false; + for matcher in ®istered_root_matchers { + match matcher.contains_status(cwd) { + ProjectMembership::Match => return Some(false), + ProjectMembership::NoMatch => {} + ProjectMembership::Unknown => unknown = true, + } + } + (!unknown).then_some(true) + }) + }?; if !include { continue; } @@ -1362,9 +1367,27 @@ fn record_cwd(record: &Value) -> Option { #[cfg(test)] mod tests { + use std::sync::atomic::{AtomicUsize, Ordering}; + use super::*; use serde_json::json; + static UNKNOWN_PATH_ATTEMPTS: AtomicUsize = AtomicUsize::new(0); + + fn retrying_identity(path: &Path) -> crate::worktree::GitRepoIdentityOutcome { + let root = path + .ancestors() + .find(|ancestor| ancestor.file_name().is_some_and(|name| name == "repo")) + .unwrap_or(path); + if UNKNOWN_PATH_ATTEMPTS.fetch_add(1, Ordering::SeqCst) == 0 { + return crate::worktree::GitRepoIdentityOutcome::Unknown; + } + crate::worktree::GitRepoIdentityOutcome::Resolved(crate::worktree::GitRepoIdentity { + worktree_root: root.to_path_buf(), + common_dir: root.join(".git"), + }) + } + #[test] fn structured_git_operation_becomes_host_commit_evidence() { let mut metadata = Map::new(); @@ -1508,6 +1531,47 @@ mod tests { assert_eq!(second_metadata["claude_message_worktree"], first_worktree); } + #[test] + fn claude_unknown_membership_retries_without_advancing_cursor() { + UNKNOWN_PATH_ATTEMPTS.store(0, Ordering::SeqCst); + let temp = tempfile::TempDir::new().expect("temp dir"); + let project_root = temp.path().join("repo"); + let nested_cwd = project_root.join("packages/app"); + std::fs::create_dir_all(&nested_cwd).expect("nested cwd"); + let transcript = temp.path().join("retry.jsonl"); + std::fs::write( + &transcript, + format!( + "{}\n", + json!({ + "type": "user", + "sessionId": "retry", + "cwd": nested_cwd, + "message": {"role": "user", "content": "retry me"} + }) + ), + ) + .expect("write transcript"); + let mut source = ClaudeSource::with_home(temp.path()); + source.project_matchers = + ProjectRootMatcherCache::with_identity_resolver(retrying_identity); + + let previous = StoredCursor::default(); + assert!( + source + .parse_new(&transcript, previous, &project_root, None) + .is_none(), + "unknown membership must abort before a new cursor can be persisted" + ); + + let retried = source + .parse_new(&transcript, previous, &project_root, None) + .expect("unknown membership must be resolved again on retry"); + assert_eq!(retried.messages.len(), 1); + assert!(retried.new_cursor.position > previous.position); + assert_eq!(UNKNOWN_PATH_ATTEMPTS.load(Ordering::SeqCst), 3); + } + #[test] fn redacted_only_thinking_records_no_reasoning_row() { // Matches Codex's encrypted-reasoning convention: no plaintext, no row. diff --git a/src/sessions/codex.rs b/src/sessions/codex.rs index 480708411..f45c7945c 100644 --- a/src/sessions/codex.rs +++ b/src/sessions/codex.rs @@ -56,7 +56,7 @@ use serde_json::Value; use crate::accounting::parser::parse_timestamp; use crate::sessions::SessionMessageRecord; use crate::sessions::shared::{ - ProjectRootMatcherCache, StoredCursor, append_tool_calls_metadata, + ProjectMembership, ProjectRootMatcherCache, StoredCursor, append_tool_calls_metadata, content_storage_text_and_tools, title_from_messages, }; use crate::sessions::source::{ @@ -206,22 +206,26 @@ impl TranscriptSource for CodexSource { let mut last_in_scope_git = None; for line in &new.lines { let is_context_record = context_state.observe_context_record(&line.value, path, &meta); - let in_scope = self.user_scope.as_ref().map_or_else( - || { - context_state.cwd.as_deref().is_some_and(|cwd| { - project_matcher - .as_ref() - .is_some_and(|matcher| matcher.contains(cwd)) - }) - }, - |_scope| { - context_state.cwd.as_deref().is_none_or(|cwd| { - !registered_root_matchers - .iter() - .any(|matcher| matcher.contains(cwd)) - }) - }, - ); + let in_scope = if self.user_scope.is_none() { + context_state.cwd.as_deref().map_or(Some(false), |cwd| { + project_matcher + .as_ref() + .map(|matcher| matcher.contains_status(cwd).definitive()) + .unwrap_or(Some(false)) + }) + } else { + context_state.cwd.as_deref().map_or(Some(true), |cwd| { + let mut unknown = false; + for matcher in ®istered_root_matchers { + match matcher.contains_status(cwd) { + ProjectMembership::Match => return Some(false), + ProjectMembership::NoMatch => {} + ProjectMembership::Unknown => unknown = true, + } + } + (!unknown).then_some(true) + }) + }?; if !in_scope { if compacted_summary_from_line( &line.value, @@ -1606,10 +1610,28 @@ mod goal_event_tests { #[cfg(test)] #[allow(clippy::unwrap_used)] mod source_matcher_cache_tests { + use std::sync::atomic::{AtomicUsize, Ordering}; + use super::*; use serde_json::json; use tempfile::TempDir; + static UNKNOWN_PATH_ATTEMPTS: AtomicUsize = AtomicUsize::new(0); + + fn retrying_identity(path: &Path) -> crate::worktree::GitRepoIdentityOutcome { + let root = path + .ancestors() + .find(|ancestor| ancestor.file_name().is_some_and(|name| name == "repo")) + .unwrap_or(path); + if UNKNOWN_PATH_ATTEMPTS.fetch_add(1, Ordering::SeqCst) == 0 { + return crate::worktree::GitRepoIdentityOutcome::Unknown; + } + crate::worktree::GitRepoIdentityOutcome::Resolved(crate::worktree::GitRepoIdentity { + worktree_root: root.to_path_buf(), + common_dir: root.join(".git"), + }) + } + fn write_rollout(path: &Path, session_id: &str, cwd: &Path) { let lines = [ json!({ @@ -1679,4 +1701,33 @@ mod source_matcher_cache_tests { serde_json::from_str(second.messages[0].metadata_json.as_deref().unwrap()).unwrap(); assert_eq!(second_metadata["codex_turn_worktree"], first_worktree); } + + #[test] + fn codex_unknown_membership_retries_without_advancing_cursor() { + UNKNOWN_PATH_ATTEMPTS.store(0, Ordering::SeqCst); + let temp = TempDir::new().unwrap(); + let project_root = temp.path().join("repo"); + let nested_cwd = project_root.join("packages/app"); + std::fs::create_dir_all(&nested_cwd).unwrap(); + let transcript = temp.path().join("retry.jsonl"); + write_rollout(&transcript, "retry-session", &nested_cwd); + let mut source = CodexSource::with_home(temp.path()); + source.project_matchers = + ProjectRootMatcherCache::with_identity_resolver(retrying_identity); + + let previous = StoredCursor::default(); + assert!( + source + .parse_new(&transcript, previous, &project_root, None) + .is_none(), + "unknown membership must abort before a new cursor can be persisted" + ); + + let retried = source + .parse_new(&transcript, previous, &project_root, None) + .expect("unknown membership must be resolved again on retry"); + assert_eq!(retried.messages.len(), 1); + assert!(retried.new_cursor.position > previous.position); + assert_eq!(UNKNOWN_PATH_ATTEMPTS.load(Ordering::SeqCst), 3); + } } diff --git a/src/sessions/shared.rs b/src/sessions/shared.rs index 83bf3afed..f6fe542c3 100644 --- a/src/sessions/shared.rs +++ b/src/sessions/shared.rs @@ -136,6 +136,29 @@ pub(crate) fn path_belongs_to_project(path: &Path, project_root: &Path) -> bool ProjectRootMatcher::new(project_root).contains(path) } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ProjectMembership { + Match, + NoMatch, + Unknown, +} + +impl ProjectMembership { + fn from_bool(value: bool) -> Self { + if value { Self::Match } else { Self::NoMatch } + } + + pub(crate) fn definitive(self) -> Option { + match self { + Self::Match => Some(true), + Self::NoMatch => Some(false), + Self::Unknown => None, + } + } +} + +type GitIdentityResolver = fn(&Path) -> crate::worktree::GitRepoIdentityOutcome; + /// A project root with its git worktree/common-dir resolutions computed once, /// so repeated membership tests (e.g. one per discovered workflow run) do not /// re-run `git_worktree_root`/`git_common_dir` on the fixed project side. A @@ -144,21 +167,25 @@ pub(crate) fn path_belongs_to_project(path: &Path, project_root: &Path) -> bool #[derive(Debug)] pub(crate) struct ProjectRootMatcher { root: PathBuf, - worktree: Option, - common_dir: Option, + identity: crate::worktree::GitRepoIdentityOutcome, + identity_resolver: GitIdentityResolver, path_membership: Mutex>, } impl ProjectRootMatcher { /// Resolve the fixed project-side git identity once. pub(crate) fn new(project_root: &Path) -> Self { - let identity = crate::worktree::git_repo_identity(project_root); + Self::new_with_identity_resolver(project_root, crate::worktree::git_repo_identity_outcome) + } + + fn new_with_identity_resolver( + project_root: &Path, + identity_resolver: GitIdentityResolver, + ) -> Self { Self { root: project_root.to_path_buf(), - worktree: identity - .as_ref() - .map(|identity| identity.worktree_root.clone()), - common_dir: identity.map(|identity| identity.common_dir), + identity: identity_resolver(project_root), + identity_resolver, path_membership: Mutex::new(HashMap::new()), } } @@ -168,6 +195,10 @@ impl ProjectRootMatcher { /// Each distinct path is resolved once for this matcher, so repeated /// transcript rows with the same cwd do not repeatedly discover/open git. pub(crate) fn contains(&self, path: &Path) -> bool { + self.contains_status(path) == ProjectMembership::Match + } + + pub(crate) fn contains_status(&self, path: &Path) -> ProjectMembership { if let Some(belongs) = self .path_membership .lock() @@ -175,37 +206,69 @@ impl ProjectRootMatcher { .get(path) .copied() { - return belongs; + return ProjectMembership::from_bool(belongs); } let belongs = self.contains_uncached(path); - self.path_membership - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .insert(path.to_path_buf(), belongs); + if let Some(definitive) = belongs.definitive() { + self.path_membership + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .insert(path.to_path_buf(), definitive); + } belongs } - fn contains_uncached(&self, path: &Path) -> bool { + fn contains_uncached(&self, path: &Path) -> ProjectMembership { + self.contains_uncached_with( + path, + self.identity_resolver, + crate::config::discover_project_root, + ) + } + + fn contains_uncached_with( + &self, + path: &Path, + identity_resolver: impl FnOnce(&Path) -> crate::worktree::GitRepoIdentityOutcome, + discover_project_root: impl FnOnce(&Path) -> Option, + ) -> ProjectMembership { if paths_equal(path, &self.root) { - return true; + return ProjectMembership::Match; + } + if self.identity == crate::worktree::GitRepoIdentityOutcome::Unknown { + return ProjectMembership::Unknown; } - if let (Some(path_identity), Some(project_worktree)) = ( - crate::worktree::git_repo_identity(path), - self.worktree.as_ref(), - ) { - if paths_equal(&path_identity.worktree_root, project_worktree) { - return true; + let path_identity = identity_resolver(path); + match (&self.identity, path_identity) { + ( + crate::worktree::GitRepoIdentityOutcome::Resolved(project_identity), + crate::worktree::GitRepoIdentityOutcome::Resolved(path_identity), + ) => { + if paths_equal( + &path_identity.worktree_root, + &project_identity.worktree_root, + ) { + return ProjectMembership::Match; + } + return ProjectMembership::from_bool(paths_equal( + &path_identity.common_dir, + &project_identity.common_dir, + )); + } + (crate::worktree::GitRepoIdentityOutcome::Unknown, _) + | (_, crate::worktree::GitRepoIdentityOutcome::Unknown) => { + return ProjectMembership::Unknown; } - return self.common_dir.as_ref().is_some_and(|project_common| { - paths_equal(&path_identity.common_dir, project_common) - }); + _ => {} } - crate::config::discover_project_root(path) - .as_ref() - .is_some_and(|discovered| paths_equal(discovered, &self.root)) + ProjectMembership::from_bool( + discover_project_root(path) + .as_ref() + .is_some_and(|discovered| paths_equal(discovered, &self.root)), + ) } } @@ -214,22 +277,60 @@ impl ProjectRootMatcher { /// A source parses many transcript files for the same project. Keeping the /// matcher here avoids reopening the same git repository once per file while /// retaining per-path membership caching inside [`ProjectRootMatcher`]. -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug)] pub(crate) struct ProjectRootMatcherCache { matchers: Arc>>>, - location_worktrees: Arc>>>>>, + location_worktrees: + Arc>>>>, + identity_resolver: GitIdentityResolver, +} + +impl Default for ProjectRootMatcherCache { + fn default() -> Self { + Self { + matchers: Arc::default(), + location_worktrees: Arc::default(), + identity_resolver: crate::worktree::git_repo_identity_outcome, + } + } } impl ProjectRootMatcherCache { + #[cfg(test)] + pub(crate) fn with_identity_resolver(identity_resolver: GitIdentityResolver) -> Self { + Self { + identity_resolver, + ..Self::default() + } + } + pub(crate) fn get(&self, project_root: &Path) -> Arc { let key = project_root .canonicalize() .unwrap_or_else(|_| project_root.to_path_buf()); + if let Some(matcher) = self + .matchers + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .get(&key) + .cloned() + { + return matcher; + } + + let matcher = Arc::new(ProjectRootMatcher::new_with_identity_resolver( + project_root, + self.identity_resolver, + )); + if matcher.identity == crate::worktree::GitRepoIdentityOutcome::Unknown { + return matcher; + } + self.matchers .lock() .unwrap_or_else(std::sync::PoisonError::into_inner) .entry(key) - .or_insert_with(|| Arc::new(ProjectRootMatcher::new(project_root))) + .or_insert_with(|| matcher.clone()) .clone() } @@ -247,11 +348,28 @@ impl ProjectRootMatcherCache { .entry(cwd.to_path_buf()) .or_insert_with(|| Arc::new(OnceLock::new())) .clone(); - resolution - .get_or_init(|| { - crate::worktree::git_repo_identity(cwd).map(|identity| identity.worktree_root) - }) + match resolution + .get_or_init(|| crate::worktree::git_repo_identity_outcome(cwd)) .clone() + { + crate::worktree::GitRepoIdentityOutcome::Resolved(identity) => { + Some(identity.worktree_root) + } + crate::worktree::GitRepoIdentityOutcome::NotFound => None, + crate::worktree::GitRepoIdentityOutcome::Unknown => { + let mut worktrees = self + .location_worktrees + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if worktrees + .get(cwd) + .is_some_and(|cached| Arc::ptr_eq(cached, &resolution)) + { + worktrees.remove(cwd); + } + None + } + } } } @@ -649,9 +767,12 @@ pub(crate) fn title_from_messages(messages: &[SessionMessageRecord]) -> Option crate::worktree::GitRepoIdentityOutcome { + let root = path + .ancestors() + .find(|ancestor| ancestor.file_name().is_some_and(|name| name == "repo")) + .unwrap_or(path); + crate::worktree::GitRepoIdentityOutcome::Resolved(crate::worktree::GitRepoIdentity { + worktree_root: root.to_path_buf(), + common_dir: root.join(".git"), + }) + } + + #[test] + fn matcher_timeout_is_unknown_without_downstream_discovery() { + let temp = TempDir::new().expect("temp dir"); + let project_root = temp.path().join("repo"); + let nested_cwd = project_root.join("packages/app"); + std::fs::create_dir_all(&nested_cwd).expect("nested cwd"); + let matcher = + ProjectRootMatcher::new_with_identity_resolver(&project_root, resolved_test_identity); + + let membership = matcher.contains_uncached_with( + &nested_cwd, + |_| crate::worktree::GitRepoIdentityOutcome::Unknown, + |_| panic!("timeout must not fall through to project discovery/gix"), + ); + + assert_eq!(membership, ProjectMembership::Unknown); + } + #[test] fn location_metadata_cache_reuses_worktree_root_for_repeated_cwd() { let temp = TempDir::new().expect("temp dir"); diff --git a/src/worktree.rs b/src/worktree.rs index efa37b906..badd3cfbd 100644 --- a/src/worktree.rs +++ b/src/worktree.rs @@ -38,6 +38,18 @@ pub(crate) struct GitRepoIdentity { pub(crate) common_dir: PathBuf, } +/// Tri-state repository identity used by session ingest. +/// +/// `Unknown` is reserved for bounded git timeouts. Callers that decide whether +/// to persist a transcript cursor must retry later rather than treating it as +/// the definitive absence of a repository. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum GitRepoIdentityOutcome { + Resolved(GitRepoIdentity), + NotFound, + Unknown, +} + /// Resolve both halves of a repository identity with one cheap git subprocess. /// /// `gix::discover` can open and scan every pack index in a large repository. @@ -45,27 +57,41 @@ pub(crate) struct GitRepoIdentity { /// without opening the object database. If git is unavailable or rejects the /// path, one gix discovery preserves the previous fail-open behavior. pub(crate) fn git_repo_identity(dir: &Path) -> Option { + match git_repo_identity_outcome(dir) { + GitRepoIdentityOutcome::Resolved(identity) => Some(identity), + GitRepoIdentityOutcome::NotFound | GitRepoIdentityOutcome::Unknown => None, + } +} + +pub(crate) fn git_repo_identity_outcome(dir: &Path) -> GitRepoIdentityOutcome { if !git_may_resolve_repo(dir) { - return None; + return GitRepoIdentityOutcome::NotFound; } - resolve_git_identity_with( + resolve_git_identity_outcome_with( dir, || crate::git::git_capture_at(dir, &["rev-parse", "--show-toplevel", "--git-common-dir"]), || git_repo_identity_from_gix(dir), ) } -fn resolve_git_identity_with( +fn resolve_git_identity_outcome_with( dir: &Path, cli_output: impl FnOnce() -> crate::git::GitCaptureAtResult, gix_fallback: impl FnOnce() -> Option, -) -> Option { +) -> GitRepoIdentityOutcome { + let fallback = || { + gix_fallback().map_or( + GitRepoIdentityOutcome::NotFound, + GitRepoIdentityOutcome::Resolved, + ) + }; match cli_output() { crate::git::GitCaptureAtResult::Captured(output) => { - git_repo_identity_from_cli_output(dir, &output).or_else(gix_fallback) + git_repo_identity_from_cli_output(dir, &output) + .map_or_else(fallback, GitRepoIdentityOutcome::Resolved) } - crate::git::GitCaptureAtResult::Failed => gix_fallback(), - crate::git::GitCaptureAtResult::TimedOut => None, + crate::git::GitCaptureAtResult::Failed => fallback(), + crate::git::GitCaptureAtResult::TimedOut => GitRepoIdentityOutcome::Unknown, } } @@ -315,12 +341,14 @@ mod tests { fs::create_dir_all(&common_dir).unwrap(); let output = format!("{}\n../../../main/.git", worktree.display()); - let identity = resolve_git_identity_with( + let outcome = resolve_git_identity_outcome_with( &nested, || crate::git::GitCaptureAtResult::Captured(output), || panic!("valid CLI identity must short-circuit gix discovery"), - ) - .expect("paired CLI identity"); + ); + let GitRepoIdentityOutcome::Resolved(identity) = outcome else { + panic!("paired CLI identity should resolve"); + }; assert_eq!( identity.worktree_root, @@ -335,12 +363,12 @@ mod tests { #[test] fn timed_out_cli_identity_does_not_fallback_to_gix() { let tmp = tempdir().unwrap(); - let identity = resolve_git_identity_with( + let outcome = resolve_git_identity_outcome_with( tmp.path(), || crate::git::GitCaptureAtResult::TimedOut, || panic!("timed-out CLI identity must not fall through to gix"), ); - assert!(identity.is_none()); + assert_eq!(outcome, GitRepoIdentityOutcome::Unknown); } #[test] From f6c57edf14d059b0c6e04be4247792181ef51d3d Mon Sep 17 00:00:00 2001 From: Hashem Khalifa Date: Mon, 27 Jul 2026 13:31:43 +0200 Subject: [PATCH 4/5] fix: retry uncertain session discovery --- .changeset/session-worktree-discovery.md | 5 + src/daemon/git_watch.rs | 51 +++++++- src/daemon/git_watch/tests.rs | 12 ++ src/sessions/hermes.rs | 144 ++++++++++++++++++++--- src/sessions/shared.rs | 142 +++++++++++++++++----- 5 files changed, 299 insertions(+), 55 deletions(-) create mode 100644 .changeset/session-worktree-discovery.md diff --git a/.changeset/session-worktree-discovery.md b/.changeset/session-worktree-discovery.md new file mode 100644 index 000000000..c33bbba9b --- /dev/null +++ b/.changeset/session-worktree-discovery.md @@ -0,0 +1,5 @@ +--- +"tracedecay": patch +--- + +Bound session worktree discovery so Git timeouts remain retryable without re-entering expensive repository scans or advancing transcript cursors. diff --git a/src/daemon/git_watch.rs b/src/daemon/git_watch.rs index 86d803a44..2e039b07a 100644 --- a/src/daemon/git_watch.rs +++ b/src/daemon/git_watch.rs @@ -466,13 +466,52 @@ async fn supervise_project(inner: Arc, state: Arc) /// One project's event loop: build the notify watcher over git metadata, then /// debounce raw events into coalesced syncs. On watcher construction/death, /// fall back to a 5-minute mtime poll for THIS project only. +enum IdentityDiscoveryDisposition { + Watch(crate::worktree::GitRepoIdentity), + Degraded, + Retry, +} + +fn identity_discovery_disposition( + outcome: crate::worktree::GitRepoIdentityOutcome, +) -> IdentityDiscoveryDisposition { + match outcome { + crate::worktree::GitRepoIdentityOutcome::Resolved(identity) => { + IdentityDiscoveryDisposition::Watch(identity) + } + crate::worktree::GitRepoIdentityOutcome::NotFound => IdentityDiscoveryDisposition::Degraded, + crate::worktree::GitRepoIdentityOutcome::Unknown => IdentityDiscoveryDisposition::Retry, + } +} + async fn project_task(inner: Arc, state: Arc) { - let Some(identity) = crate::worktree::git_repo_identity(&state.project_root) else { - // Not a resolvable git repo (yet). Degrade to polling so a later `git - // init` / clone is still eventually covered. - state.health.set_degraded(true); - degraded_poll_loop(&inner, &state, None).await; - return; + let mut discovery_backoff = Duration::from_millis(500); + let identity = loop { + match identity_discovery_disposition(crate::worktree::git_repo_identity_outcome( + &state.project_root, + )) { + IdentityDiscoveryDisposition::Watch(identity) => break identity, + IdentityDiscoveryDisposition::Degraded => { + // Definitively not a git repo (yet). Degrade to polling so a + // later `git init` / clone is still eventually covered. + state.health.set_degraded(true); + degraded_poll_loop(&inner, &state, None).await; + return; + } + IdentityDiscoveryDisposition::Retry => { + state.health.set_degraded(true); + state.health.beat(); + log_daemon_event( + "git_watch_discovery_retry", + &[ + ("project", state.project_root.display().to_string()), + ("backoff_ms", discovery_backoff.as_millis().to_string()), + ], + ); + tokio::time::sleep(discovery_backoff).await; + discovery_backoff = (discovery_backoff * 2).min(RESTART_BACKOFF_MAX); + } + } }; let common_dir = identity.common_dir; diff --git a/src/daemon/git_watch/tests.rs b/src/daemon/git_watch/tests.rs index 5cf26734e..178b3c254 100644 --- a/src/daemon/git_watch/tests.rs +++ b/src/daemon/git_watch/tests.rs @@ -102,6 +102,18 @@ fn heartbeat_staleness() { assert!(old.heartbeat_stale()); } +#[test] +fn timed_out_identity_discovery_retries_instead_of_degrading_forever() { + assert!(matches!( + identity_discovery_disposition(crate::worktree::GitRepoIdentityOutcome::Unknown), + IdentityDiscoveryDisposition::Retry + )); + assert!(matches!( + identity_discovery_disposition(crate::worktree::GitRepoIdentityOutcome::NotFound), + IdentityDiscoveryDisposition::Degraded + )); +} + /// The shared coordinator must not start a second store-writing lifetime while /// the first one is held. Paused Tokio time plus Notify/oneshot handshakes make /// this a scheduling-state assertion rather than a wall-clock sleep. diff --git a/src/sessions/hermes.rs b/src/sessions/hermes.rs index f56f747a2..923c33680 100644 --- a/src/sessions/hermes.rs +++ b/src/sessions/hermes.rs @@ -37,9 +37,9 @@ use serde_json::{Map, Value}; use crate::agents::hermes::read_config_pinned_project_root; use crate::global_db::{GlobalDb, ParseOffset, TranscriptBatch}; use crate::sessions::shared::{ - NewRows, ProjectRootMatcher, StoredCursor, TranscriptIngestStats, TranscriptLocation, - TranscriptLocationMetadataKeys, append_location_metadata, content_storage_text_and_tools, - path_belongs_to_project, preview_title, title_from_messages, + NewRows, ProjectMembership, ProjectRootMatcher, StoredCursor, TranscriptIngestStats, + TranscriptLocation, TranscriptLocationMetadataKeys, append_location_metadata, + content_storage_text_and_tools, path_belongs_to_project, preview_title, title_from_messages, }; use crate::sessions::{SessionMessageRecord, SessionRecord}; @@ -539,7 +539,13 @@ async fn try_ingest_state_db_for_projects( &destination_matchers, source, &mut destination_routes, - ); + ) + .map_err(|_| { + format!( + "could not classify Hermes rows from '{}' because project membership is unknown", + state_db.display() + ) + })?; for (state_index, state) in states .iter_mut() .enumerate() @@ -980,12 +986,17 @@ struct DestinationTurnLocations { row_indices: Vec, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum DestinationRoutingError { + UnknownMembership, +} + fn turn_project_locations_for_destinations( rows: &[HermesRow], destination_matchers: &[ProjectRootMatcher], source: &HermesProfileSource, destination_routes: &mut HashMap>, -) -> Vec { +) -> Result, DestinationRoutingError> { let mut by_session: HashMap<&str, Vec<&HermesRow>> = HashMap::new(); let row_indices = rows .iter() @@ -1017,7 +1028,7 @@ fn turn_project_locations_for_destinations( let mut fallbacks = vec![None; destination_matchers.len()]; for (cwd, provenance) in fallback_candidates { for destination_index in - matching_destinations(&cwd, destination_matchers, destination_routes) + matching_destinations(&cwd, destination_matchers, destination_routes)? { fallbacks[destination_index].get_or_insert_with(|| HermesSessionLocation { cwd: cwd.clone(), @@ -1035,7 +1046,7 @@ fn turn_project_locations_for_destinations( &row_indices, &mut locations, destination_routes, - ); + )?; turn.clear(); } turn.push(row); @@ -1047,12 +1058,12 @@ fn turn_project_locations_for_destinations( &row_indices, &mut locations, destination_routes, - ); + )?; } for destination in &mut locations { destination.row_indices.sort_unstable(); } - locations + Ok(locations) } fn assign_turn_locations_for_destinations( @@ -1062,7 +1073,7 @@ fn assign_turn_locations_for_destinations( row_indices: &HashMap, locations: &mut [DestinationTurnLocations], destination_routes: &mut HashMap>, -) { +) -> Result<(), DestinationRoutingError> { let explicit_paths = rows .iter() .rev() @@ -1074,7 +1085,7 @@ fn assign_turn_locations_for_destinations( } else { for path in explicit_paths { for destination_index in - matching_destinations(&path, destination_matchers, destination_routes) + matching_destinations(&path, destination_matchers, destination_routes)? { selected[destination_index].get_or_insert_with(|| HermesSessionLocation { cwd: path.clone(), @@ -1094,23 +1105,29 @@ fn assign_turn_locations_for_destinations( } } } + Ok(()) } fn matching_destinations( path: &Path, destination_matchers: &[ProjectRootMatcher], destination_routes: &mut HashMap>, -) -> Vec { +) -> Result, DestinationRoutingError> { if let Some(indices) = destination_routes.get(path) { - return indices.clone(); + return Ok(indices.clone()); + } + let mut indices = Vec::new(); + for (index, matcher) in destination_matchers.iter().enumerate() { + match matcher.contains_status(path) { + ProjectMembership::Match => indices.push(index), + ProjectMembership::NoMatch => {} + ProjectMembership::Unknown => { + return Err(DestinationRoutingError::UnknownMembership); + } + } } - let indices = destination_matchers - .iter() - .enumerate() - .filter_map(|(index, matcher)| matcher.contains(path).then_some(index)) - .collect::>(); destination_routes.insert(path.to_path_buf(), indices.clone()); - indices + Ok(indices) } fn assign_turn_location( @@ -1422,3 +1439,92 @@ fn file_mtime_secs(path: &Path) -> u64 { .and_then(|time| time.duration_since(std::time::UNIX_EPOCH).ok()) .map_or(0, |duration| duration.as_secs()) } + +#[cfg(test)] +mod tests { + use std::sync::atomic::{AtomicUsize, Ordering}; + + use super::*; + + static IDENTITY_ATTEMPTS: AtomicUsize = AtomicUsize::new(0); + + fn retrying_identity(path: &Path) -> crate::worktree::GitRepoIdentityOutcome { + let root = path + .ancestors() + .find(|ancestor| ancestor.file_name().is_some_and(|name| name == "repo")) + .unwrap_or(path); + if IDENTITY_ATTEMPTS.fetch_add(1, Ordering::SeqCst) == 0 { + return crate::worktree::GitRepoIdentityOutcome::Unknown; + } + crate::worktree::GitRepoIdentityOutcome::Resolved(crate::worktree::GitRepoIdentity { + worktree_root: root.to_path_buf(), + common_dir: root.join(".git"), + }) + } + + fn row_with_cwd(cwd: &Path) -> HermesRow { + HermesRow { + id: 1, + session_id: "session".to_string(), + role: "user".to_string(), + content: Some("retry me".to_string()), + tool_name: None, + tool_calls: None, + timestamp: None, + session_title: None, + session_model: None, + parent_session_id: None, + session_started_at: None, + session_ended_at: None, + session_source: None, + session_cwd: Some(cwd.to_string_lossy().to_string()), + session_input_tokens: None, + session_output_tokens: None, + session_cache_read_tokens: None, + session_cache_write_tokens: None, + session_reasoning_tokens: None, + active: 1, + } + } + + #[test] + fn unknown_destination_route_retries_without_advancing_cursor() { + IDENTITY_ATTEMPTS.store(0, Ordering::SeqCst); + let temp = tempfile::TempDir::new().expect("temp dir"); + let project_root = temp.path().join("repo"); + let cwd = project_root.join("packages/app"); + std::fs::create_dir_all(&cwd).expect("cwd"); + let rows = vec![row_with_cwd(&cwd)]; + let source = HermesProfileSource { + state_db: temp.path().join("state.db"), + profile: None, + legacy_project_pin: None, + }; + let previous = StoredCursor::default(); + let mut persisted = previous; + let mut routes = HashMap::new(); + + let first_matchers = vec![ProjectRootMatcher::new_with_identity_resolver( + &project_root, + retrying_identity, + )]; + assert!( + turn_project_locations_for_destinations(&rows, &first_matchers, &source, &mut routes,) + .is_err() + ); + assert_eq!(persisted, previous); + assert!(routes.is_empty(), "unknown routes must not be cached"); + + let retry_matchers = vec![ProjectRootMatcher::new_with_identity_resolver( + &project_root, + retrying_identity, + )]; + let locations = + turn_project_locations_for_destinations(&rows, &retry_matchers, &source, &mut routes) + .expect("the same source rows should route after identity recovers"); + assert_eq!(locations[0].row_indices, vec![0]); + persisted.position = rows[0].id as u64; + assert!(persisted.position > previous.position); + assert_eq!(IDENTITY_ATTEMPTS.load(Ordering::SeqCst), 3); + } +} diff --git a/src/sessions/shared.rs b/src/sessions/shared.rs index f6fe542c3..ee634e29b 100644 --- a/src/sessions/shared.rs +++ b/src/sessions/shared.rs @@ -8,6 +8,7 @@ use std::collections::HashMap; use std::io; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex, OnceLock}; +use std::time::{Duration, Instant}; use serde_json::Value; @@ -157,7 +158,14 @@ impl ProjectMembership { } } -type GitIdentityResolver = fn(&Path) -> crate::worktree::GitRepoIdentityOutcome; +pub(crate) type GitIdentityResolver = fn(&Path) -> crate::worktree::GitRepoIdentityOutcome; +const LOCATION_WORKTREE_UNKNOWN_RETRY_COOLDOWN: Duration = Duration::from_secs(30); + +#[derive(Debug, Default)] +struct LocationWorktreeCacheEntry { + outcome: OnceLock, + unknown_retry_after: Mutex>, +} /// A project root with its git worktree/common-dir resolutions computed once, /// so repeated membership tests (e.g. one per discovered workflow run) do not @@ -178,7 +186,7 @@ impl ProjectRootMatcher { Self::new_with_identity_resolver(project_root, crate::worktree::git_repo_identity_outcome) } - fn new_with_identity_resolver( + pub(crate) fn new_with_identity_resolver( project_root: &Path, identity_resolver: GitIdentityResolver, ) -> Self { @@ -280,8 +288,7 @@ impl ProjectRootMatcher { #[derive(Clone, Debug)] pub(crate) struct ProjectRootMatcherCache { matchers: Arc>>>, - location_worktrees: - Arc>>>>, + location_worktrees: Arc>>>, identity_resolver: GitIdentityResolver, } @@ -341,33 +348,61 @@ impl ProjectRootMatcherCache { /// source-lifetime like the project matchers and use the bounded CLI-first /// identity path instead of opening the repository object database. pub(crate) fn git_worktree_root(&self, cwd: &Path) -> Option { - let resolution = self - .location_worktrees - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .entry(cwd.to_path_buf()) - .or_insert_with(|| Arc::new(OnceLock::new())) - .clone(); - match resolution - .get_or_init(|| crate::worktree::git_repo_identity_outcome(cwd)) - .clone() - { - crate::worktree::GitRepoIdentityOutcome::Resolved(identity) => { - Some(identity.worktree_root) - } - crate::worktree::GitRepoIdentityOutcome::NotFound => None, - crate::worktree::GitRepoIdentityOutcome::Unknown => { - let mut worktrees = self - .location_worktrees - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - if worktrees - .get(cwd) - .is_some_and(|cached| Arc::ptr_eq(cached, &resolution)) - { - worktrees.remove(cwd); + self.git_worktree_root_at( + cwd, + Instant::now(), + &crate::worktree::git_repo_identity_outcome, + ) + } + + fn git_worktree_root_at( + &self, + cwd: &Path, + now: Instant, + identity_resolver: &impl Fn(&Path) -> crate::worktree::GitRepoIdentityOutcome, + ) -> Option { + loop { + let resolution = self + .location_worktrees + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .entry(cwd.to_path_buf()) + .or_insert_with(|| Arc::new(LocationWorktreeCacheEntry::default())) + .clone(); + match resolution + .outcome + .get_or_init(|| identity_resolver(cwd)) + .clone() + { + crate::worktree::GitRepoIdentityOutcome::Resolved(identity) => { + return Some(identity.worktree_root); + } + crate::worktree::GitRepoIdentityOutcome::NotFound => return None, + crate::worktree::GitRepoIdentityOutcome::Unknown => { + let should_retry = { + let mut retry_after = resolution + .unknown_retry_after + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let retry_after = retry_after + .get_or_insert(now + LOCATION_WORKTREE_UNKNOWN_RETRY_COOLDOWN); + now >= *retry_after + }; + if !should_retry { + return None; + } + + let mut worktrees = self + .location_worktrees + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if worktrees + .get(cwd) + .is_some_and(|cached| Arc::ptr_eq(cached, &resolution)) + { + worktrees.remove(cwd); + } } - None } } } @@ -768,10 +803,13 @@ pub(crate) fn title_from_messages(messages: &[SessionMessageRecord]) -> Option Date: Mon, 27 Jul 2026 13:54:51 +0200 Subject: [PATCH 5/5] fix(storage): bound legacy worktree probing --- src/storage.rs | 118 ++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 103 insertions(+), 15 deletions(-) diff --git a/src/storage.rs b/src/storage.rs index 5a14994f3..9d2878953 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -448,25 +448,25 @@ pub(crate) fn matching_legacy_profile_layouts( profile_root: &Path, excluded_project_id: Option<&str>, ) -> Result<(Vec, bool)> { - matching_legacy_profile_layouts_with_git_resolver( + matching_legacy_profile_layouts_with_git_identity_resolver( project_root, profile_root, excluded_project_id, crate::worktree::is_detached_linked_worktree, - crate::worktree::git_common_dir, + crate::worktree::git_repo_identity_outcome, ) } -fn matching_legacy_profile_layouts_with_git_resolver( +fn matching_legacy_profile_layouts_with_git_identity_resolver( project_root: &Path, profile_root: &Path, excluded_project_id: Option<&str>, mut is_detached_linked_worktree: D, - mut git_common_dir: G, + mut git_identity: G, ) -> Result<(Vec, bool)> where D: FnMut(&Path) -> bool, - G: FnMut(&Path) -> Option, + G: FnMut(&Path) -> crate::worktree::GitRepoIdentityOutcome, { let projects_root = profile_root.join("projects"); let Ok(entries) = fs::read_dir(&projects_root) else { @@ -507,7 +507,13 @@ where selected_manifest_matches_exact_root && exact_manifests.is_empty(); let matching_manifests = if exact_manifests.is_empty() { let project_git_common_dir = (!is_detached_linked_worktree(project_root)) - .then(|| git_common_dir(project_root)) + .then(|| match git_identity(project_root) { + crate::worktree::GitRepoIdentityOutcome::Resolved(identity) => { + Some(identity.common_dir) + } + crate::worktree::GitRepoIdentityOutcome::Unknown + | crate::worktree::GitRepoIdentityOutcome::NotFound => None, + }) .flatten(); let mut legacy_git_common_dirs = HashMap::>::new(); non_exact_manifests @@ -520,7 +526,13 @@ where manifest .project_root .is_dir() - .then(|| git_common_dir(&manifest.project_root)) + .then(|| match git_identity(&manifest.project_root) { + crate::worktree::GitRepoIdentityOutcome::Resolved(identity) => { + Some(identity.common_dir) + } + crate::worktree::GitRepoIdentityOutcome::Unknown + | crate::worktree::GitRepoIdentityOutcome::NotFound => None, + }) .flatten() }) .as_deref() @@ -1314,6 +1326,67 @@ mod tests { use std::cell::RefCell; use std::sync::{Arc, Barrier}; + #[test] + fn legacy_shared_git_probe_skips_timeout_and_unresolvable_roots() { + let dir = tempfile::tempdir().unwrap(); + let project_root = dir.path().join("repo"); + let legacy_root = dir.path().join("legacy"); + let profile_root = dir.path().join("profile"); + let data_root = profile_root.join("projects").join("proj_legacy"); + for root in [&project_root, &legacy_root, &data_root] { + fs::create_dir_all(root).unwrap(); + } + write_store_manifest_to_path( + &data_root.join(STORE_MANIFEST_FILENAME), + &StoreManifest { + schema_version: STORE_MANIFEST_SCHEMA_VERSION, + project_id: Some("proj_legacy".to_string()), + store_kind: StoreKind::CodeProject, + storage_mode: StorageMode::ProfileSharded, + project_root: legacy_root.clone(), + data_root, + graph_db_relpath: "tracedecay.db".into(), + sessions_db_relpath: "sessions.db".into(), + branch_meta_relpath: "branch-meta.json".into(), + }, + ) + .unwrap(); + + let (layouts, _) = matching_legacy_profile_layouts_with_git_identity_resolver( + &project_root, + &profile_root, + None, + |_| false, + |_| crate::worktree::GitRepoIdentityOutcome::Unknown, + ) + .unwrap(); + assert!(layouts.is_empty(), "a timed-out current root cannot match"); + + let (layouts, _) = matching_legacy_profile_layouts_with_git_identity_resolver( + &project_root, + &profile_root, + None, + |_| false, + |root| { + if root == project_root { + crate::worktree::GitRepoIdentityOutcome::Resolved( + crate::worktree::GitRepoIdentity { + worktree_root: project_root.clone(), + common_dir: dir.path().join("shared.git"), + }, + ) + } else { + crate::worktree::GitRepoIdentityOutcome::NotFound + } + }, + ) + .unwrap(); + assert!( + layouts.is_empty(), + "an unresolvable legacy root cannot be selected" + ); + } + #[test] fn exact_root_manifest_overrides_shared_git_discovery() { fn write_manifest(profile_root: &Path, project_id: &str, project_root: &Path) { @@ -1347,14 +1420,19 @@ mod tests { let resolver_calls = RefCell::new(Vec::new()); let (layouts, selected_is_sole_exact_root) = - matching_legacy_profile_layouts_with_git_resolver( + matching_legacy_profile_layouts_with_git_identity_resolver( &project_root, &profile_root, None, |_| false, |root| { resolver_calls.borrow_mut().push(root.to_path_buf()); - Some(dir.path().join("shared.git")) + crate::worktree::GitRepoIdentityOutcome::Resolved( + crate::worktree::GitRepoIdentity { + worktree_root: root.to_path_buf(), + common_dir: dir.path().join("shared.git"), + }, + ) }, ) .unwrap(); @@ -1371,14 +1449,19 @@ mod tests { resolver_calls.borrow_mut().clear(); let (layouts, selected_is_sole_exact_root) = - matching_legacy_profile_layouts_with_git_resolver( + matching_legacy_profile_layouts_with_git_identity_resolver( &project_root, &profile_root, Some("proj_exact"), |_| false, |root| { resolver_calls.borrow_mut().push(root.to_path_buf()); - Some(dir.path().join("shared.git")) + crate::worktree::GitRepoIdentityOutcome::Resolved( + crate::worktree::GitRepoIdentity { + worktree_root: root.to_path_buf(), + common_dir: dir.path().join("shared.git"), + }, + ) }, ) .unwrap(); @@ -1422,12 +1505,12 @@ mod tests { ) .unwrap(); - let error = matching_legacy_profile_layouts_with_git_resolver( + let error = matching_legacy_profile_layouts_with_git_identity_resolver( &project_root, &profile_root, None, |_| false, - |_| None, + |_| crate::worktree::GitRepoIdentityOutcome::NotFound, ) .expect_err("missing project_id must fail closed"); assert!(error.to_string().contains("project_id is missing")); @@ -1468,14 +1551,19 @@ mod tests { let resolver_calls = RefCell::new(Vec::new()); let (layouts, selected_is_sole_exact_root) = - matching_legacy_profile_layouts_with_git_resolver( + matching_legacy_profile_layouts_with_git_identity_resolver( &worktree_root, &profile_root, Some("proj_selected"), |_| false, |root| { resolver_calls.borrow_mut().push(root.to_path_buf()); - Some(dir.path().join("shared.git")) + crate::worktree::GitRepoIdentityOutcome::Resolved( + crate::worktree::GitRepoIdentity { + worktree_root: root.to_path_buf(), + common_dir: dir.path().join("shared.git"), + }, + ) }, ) .unwrap();