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.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..2e039b07a 100644 --- a/src/daemon/git_watch.rs +++ b/src/daemon/git_watch.rs @@ -466,14 +466,54 @@ 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(common_dir) = crate::worktree::git_common_dir(&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; // 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/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/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..2f9b441c1 100644 --- a/src/sessions/claude.rs +++ b/src/sessions/claude.rs @@ -26,10 +26,10 @@ 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, + 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, @@ -76,6 +76,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 +97,7 @@ impl ClaudeSource { Self { projects_dir: home.join(".claude").join("projects"), user_scope: None, + project_matchers: ProjectRootMatcherCache::default(), } } @@ -191,24 +193,44 @@ 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)) - }, - |scope| { - line_cwd.as_deref().is_none_or(|cwd| { - !scope - .registered_roots - .iter() - .any(|root| path_belongs_to_project(cwd, root)) - }) - }, - ); + 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; } @@ -223,6 +245,7 @@ impl TranscriptSource for ClaudeSource { line.offset, session_cwd.as_deref(), &mut accumulator, + &self.project_matchers, ) .or_else(|| { system_hook_message_from_line( @@ -272,6 +295,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()), @@ -500,6 +524,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" { @@ -573,6 +598,7 @@ fn message_from_line( content, session_cwd, accumulator, + location_cache, )) .ok(), }) @@ -1106,16 +1132,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 @@ -1167,6 +1195,7 @@ fn message_metadata( content: &Value, session_cwd: Option<&Path>, accumulator: &mut SessionAccumulator, + location_cache: &ProjectRootMatcherCache, ) -> Value { let mut metadata = Map::new(); metadata.insert( @@ -1180,10 +1209,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") @@ -1337,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(); @@ -1395,8 +1443,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")); @@ -1425,6 +1481,97 @@ 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 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 e2d3250d7..f45c7945c 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, + ProjectMembership, 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,26 +187,45 @@ 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)) - }, - |scope| { - context_state.cwd.as_deref().is_none_or(|cwd| { - !scope - .registered_roots - .iter() - .any(|root| path_belongs_to_project(cwd, root)) - }) - }, - ); + 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, @@ -243,6 +264,7 @@ impl TranscriptSource for CodexSource { &mut message, context_state.cwd.as_deref(), context_state.git.as_ref(), + &self.project_matchers, ); messages.push(message); } @@ -266,6 +288,7 @@ impl TranscriptSource for CodexSource { &mut message, context_state.cwd.as_deref(), context_state.git.as_ref(), + &self.project_matchers, ); messages.push(message); continue; @@ -281,6 +304,7 @@ impl TranscriptSource for CodexSource { &mut message, context_state.cwd.as_deref(), context_state.git.as_ref(), + &self.project_matchers, ); messages.push(message); continue; @@ -296,6 +320,7 @@ impl TranscriptSource for CodexSource { &mut message, context_state.cwd.as_deref(), context_state.git.as_ref(), + &self.project_matchers, ); messages.push(message); continue; @@ -314,6 +339,7 @@ impl TranscriptSource for CodexSource { &mut message, context_state.cwd.as_deref(), context_state.git.as_ref(), + &self.project_matchers, ); messages.push(message); continue; @@ -329,6 +355,7 @@ impl TranscriptSource for CodexSource { &mut message, context_state.cwd.as_deref(), context_state.git.as_ref(), + &self.project_matchers, ); messages.push(message); continue; @@ -349,6 +376,7 @@ impl TranscriptSource for CodexSource { &mut message, context_state.cwd.as_deref(), context_state.git.as_ref(), + &self.project_matchers, ); messages.push(message); } @@ -363,6 +391,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); } @@ -382,6 +411,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, @@ -1576,3 +1606,128 @@ 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 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!({ + "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); + 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); + } + + #[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/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/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 6999dbc07..ee634e29b 100644 --- a/src/sessions/shared.rs +++ b/src/sessions/shared.rs @@ -4,8 +4,11 @@ //! 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, OnceLock}; +use std::time::{Duration, Instant}; use serde_json::Value; @@ -134,53 +137,274 @@ 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, + } + } +} + +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 /// 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, + 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 { + Self::new_with_identity_resolver(project_root, crate::worktree::git_repo_identity_outcome) + } + + pub(crate) fn new_with_identity_resolver( + project_root: &Path, + identity_resolver: GitIdentityResolver, + ) -> Self { Self { root: project_root.to_path_buf(), - worktree: crate::worktree::git_worktree_root(project_root), - common_dir: crate::worktree::git_common_dir(project_root), + identity: identity_resolver(project_root), + identity_resolver, + 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 { + self.contains_status(path) == ProjectMembership::Match + } + + pub(crate) fn contains_status(&self, path: &Path) -> ProjectMembership { + if let Some(belongs) = self + .path_membership + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .get(path) + .copied() + { + return ProjectMembership::from_bool(belongs); + } + + let belongs = self.contains_uncached(path); + 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) -> 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_worktree), Some(project_worktree)) = ( - crate::worktree::git_worktree_root(path).as_ref(), - self.worktree.as_ref(), - ) { - if paths_equal(path_worktree, 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 crate::worktree::git_common_dir(path) + _ => {} + } + + ProjectMembership::from_bool( + discover_project_root(path) .as_ref() - .zip(self.common_dir.as_ref()) - .is_some_and(|(path_common, project_common)| { - paths_equal(path_common, project_common) - }); + .is_some_and(|discovered| paths_equal(discovered, &self.root)), + ) + } +} + +/// 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)] +pub(crate) struct ProjectRootMatcherCache { + matchers: 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; } - crate::config::discover_project_root(path) - .as_ref() - .is_some_and(|discovered| paths_equal(discovered, &self.root)) + self.matchers + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .entry(key) + .or_insert_with(|| matcher.clone()) + .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 { + 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); + } + } + } + } } } @@ -392,6 +616,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; @@ -400,7 +652,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()), @@ -550,11 +802,160 @@ 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_unknown_uses_cooldown_then_retries() { + let temp = TempDir::new().expect("temp dir"); + let cwd = temp.path().join("repo"); + std::fs::create_dir_all(&cwd).expect("cwd"); + let cache = ProjectRootMatcherCache::default(); + let calls = AtomicUsize::new(0); + let now = Instant::now(); + let resolver = |path: &Path| { + if calls.fetch_add(1, Ordering::SeqCst) == 0 { + crate::worktree::GitRepoIdentityOutcome::Unknown + } else { + crate::worktree::GitRepoIdentityOutcome::Resolved( + crate::worktree::GitRepoIdentity { + worktree_root: path.to_path_buf(), + common_dir: path.join(".git"), + }, + ) + } + }; + + assert!(cache.git_worktree_root_at(&cwd, now, &resolver).is_none()); + assert!( + cache + .git_worktree_root_at( + &cwd, + now + LOCATION_WORKTREE_UNKNOWN_RETRY_COOLDOWN / 2, + &resolver, + ) + .is_none() + ); + assert_eq!(calls.load(Ordering::SeqCst), 1); + + assert_eq!( + cache.git_worktree_root_at( + &cwd, + now + LOCATION_WORKTREE_UNKNOWN_RETRY_COOLDOWN, + &resolver, + ), + Some(cwd) + ); + assert_eq!(calls.load(Ordering::SeqCst), 2); + } + + #[test] + fn location_metadata_cache_reuses_worktree_root_for_repeated_cwd() { + 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 status = std::process::Command::new("git") + .args(["init", "-q"]) + .current_dir(&project_root) + .status() + .expect("git init"); + assert!(status.success()); + + let cache = ProjectRootMatcherCache::default(); + let keys = TranscriptLocationMetadataKeys::new("cwd", "worktree", "provenance"); + let location = TranscriptLocation::new(Some(&nested_cwd), "test"); + let mut first = serde_json::Map::new(); + append_location_metadata_cached(&mut first, keys, location, &cache); + assert_eq!( + first.get("worktree").and_then(Value::as_str), + Some( + project_root + .canonicalize() + .expect("canonical project root") + .to_string_lossy() + .as_ref() + ) + ); + + std::fs::rename(project_root.join(".git"), project_root.join(".git.hidden")) + .expect("hide git metadata after first lookup"); + + let mut second = serde_json::Map::new(); + append_location_metadata_cached(&mut second, keys, location, &cache); + assert_eq!( + second.get("worktree").and_then(Value::as_str), + first.get("worktree").and_then(Value::as_str), + "repeated cwd should reuse the source-lifetime worktree resolution" + ); + } + + #[test] + fn project_root_matcher_caches_repeated_path_membership() { + 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 status = std::process::Command::new("git") + .args(["init", "-q"]) + .current_dir(&project_root) + .status() + .expect("git init"); + assert!(status.success()); + + let matcher = ProjectRootMatcher::new(&project_root); + assert!(matcher.contains(&nested_cwd)); + + // A repeated lookup should use the result already resolved for this + // cwd, rather than discovering/opening the same repository again. + std::fs::rename(project_root.join(".git"), project_root.join(".git.hidden")) + .expect("hide git metadata after first lookup"); + assert!(matcher.contains(&nested_cwd)); + } + #[test] fn one_line_truncated_collapses_and_clips() { assert_eq!(one_line_truncated("a\n b\t c", 100), "a b c"); 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(); diff --git a/src/worktree.rs b/src/worktree.rs index e564fa8d9..badd3cfbd 100644 --- a/src/worktree.rs +++ b/src/worktree.rs @@ -30,6 +30,109 @@ pub struct WorktreeIndexMismatch { pub index_root: PathBuf, } +/// Paired git identity used by hot paths that need both the per-worktree root +/// and repository-wide common directory. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct GitRepoIdentity { + pub(crate) worktree_root: PathBuf, + 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. +/// Session ingestion only needs these two paths, which `rev-parse` returns +/// 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 GitRepoIdentityOutcome::NotFound; + } + 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_outcome_with( + dir: &Path, + cli_output: impl FnOnce() -> crate::git::GitCaptureAtResult, + gix_fallback: impl FnOnce() -> 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) + .map_or_else(fallback, GitRepoIdentityOutcome::Resolved) + } + crate::git::GitCaptureAtResult::Failed => fallback(), + crate::git::GitCaptureAtResult::TimedOut => GitRepoIdentityOutcome::Unknown, + } +} + +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 +331,86 @@ 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 outcome = resolve_git_identity_outcome_with( + &nested, + || crate::git::GitCaptureAtResult::Captured(output), + || panic!("valid CLI identity must short-circuit gix discovery"), + ); + let GitRepoIdentityOutcome::Resolved(identity) = outcome else { + panic!("paired CLI identity should resolve"); + }; + + 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 outcome = resolve_git_identity_outcome_with( + tmp.path(), + || crate::git::GitCaptureAtResult::TimedOut, + || panic!("timed-out CLI identity must not fall through to gix"), + ); + assert_eq!(outcome, GitRepoIdentityOutcome::Unknown); + } + + #[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