Skip to content

Commit 861c9b7

Browse files
Merge pull request #481 from HashemKhalifa/fix/session-git-discovery-cpu
fix: bound session worktree discovery
2 parents f297a4c + 8897550 commit 861c9b7

12 files changed

Lines changed: 1398 additions & 119 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"tracedecay": patch
3+
---
4+
5+
Bound session worktree discovery so Git timeouts remain retryable without re-entering expensive repository scans or advancing transcript cursors.

src/daemon.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1227,15 +1227,17 @@ async fn resolve_daemon_initialize_route(
12271227
break;
12281228
}
12291229
}
1230-
if let Some(git_root) = crate::worktree::git_worktree_root(root) {
1231-
let git_common_dir = crate::worktree::git_common_dir(&git_root);
1230+
if let Some(identity) = crate::worktree::git_repo_identity(root) {
12321231
if registry
1233-
.project_registry_context_by_identity(&git_root, git_common_dir.as_deref())
1232+
.project_registry_context_by_identity(
1233+
&identity.worktree_root,
1234+
Some(&identity.common_dir),
1235+
)
12341236
.await
12351237
.is_some()
12361238
{
12371239
return Some(InitializeRouteMetadata {
1238-
project_path: git_root,
1240+
project_path: identity.worktree_root,
12391241
allow_init: false,
12401242
});
12411243
}
@@ -1258,10 +1260,10 @@ async fn resolve_daemon_initialize_route(
12581260
allow_init: false,
12591261
});
12601262
}
1261-
if let Some(git_root) = crate::worktree::git_worktree_root(&root) {
1262-
let allow_init = crate::config::load_sync_config(&git_root).auto_init;
1263+
if let Some(identity) = crate::worktree::git_repo_identity(&root) {
1264+
let allow_init = crate::config::load_sync_config(&identity.worktree_root).auto_init;
12631265
return Some(InitializeRouteMetadata {
1264-
project_path: git_root,
1266+
project_path: identity.worktree_root,
12651267
allow_init,
12661268
});
12671269
}

src/daemon/git_watch.rs

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -466,14 +466,54 @@ async fn supervise_project(inner: Arc<GitWatcherInner>, state: Arc<WatchState>)
466466
/// One project's event loop: build the notify watcher over git metadata, then
467467
/// debounce raw events into coalesced syncs. On watcher construction/death,
468468
/// fall back to a 5-minute mtime poll for THIS project only.
469+
enum IdentityDiscoveryDisposition {
470+
Watch(crate::worktree::GitRepoIdentity),
471+
Degraded,
472+
Retry,
473+
}
474+
475+
fn identity_discovery_disposition(
476+
outcome: crate::worktree::GitRepoIdentityOutcome,
477+
) -> IdentityDiscoveryDisposition {
478+
match outcome {
479+
crate::worktree::GitRepoIdentityOutcome::Resolved(identity) => {
480+
IdentityDiscoveryDisposition::Watch(identity)
481+
}
482+
crate::worktree::GitRepoIdentityOutcome::NotFound => IdentityDiscoveryDisposition::Degraded,
483+
crate::worktree::GitRepoIdentityOutcome::Unknown => IdentityDiscoveryDisposition::Retry,
484+
}
485+
}
486+
469487
async fn project_task(inner: Arc<GitWatcherInner>, state: Arc<WatchState>) {
470-
let Some(common_dir) = crate::worktree::git_common_dir(&state.project_root) else {
471-
// Not a resolvable git repo (yet). Degrade to polling so a later `git
472-
// init` / clone is still eventually covered.
473-
state.health.set_degraded(true);
474-
degraded_poll_loop(&inner, &state, None).await;
475-
return;
488+
let mut discovery_backoff = Duration::from_millis(500);
489+
let identity = loop {
490+
match identity_discovery_disposition(crate::worktree::git_repo_identity_outcome(
491+
&state.project_root,
492+
)) {
493+
IdentityDiscoveryDisposition::Watch(identity) => break identity,
494+
IdentityDiscoveryDisposition::Degraded => {
495+
// Definitively not a git repo (yet). Degrade to polling so a
496+
// later `git init` / clone is still eventually covered.
497+
state.health.set_degraded(true);
498+
degraded_poll_loop(&inner, &state, None).await;
499+
return;
500+
}
501+
IdentityDiscoveryDisposition::Retry => {
502+
state.health.set_degraded(true);
503+
state.health.beat();
504+
log_daemon_event(
505+
"git_watch_discovery_retry",
506+
&[
507+
("project", state.project_root.display().to_string()),
508+
("backoff_ms", discovery_backoff.as_millis().to_string()),
509+
],
510+
);
511+
tokio::time::sleep(discovery_backoff).await;
512+
discovery_backoff = (discovery_backoff * 2).min(RESTART_BACKOFF_MAX);
513+
}
514+
}
476515
};
516+
let common_dir = identity.common_dir;
477517

478518
// Build the raw watcher. Its callback pushes into the dirty set and wakes
479519
// the debounce loop — it never blocks and never syncs inline.

src/daemon/git_watch/tests.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,18 @@ fn heartbeat_staleness() {
102102
assert!(old.heartbeat_stale());
103103
}
104104

105+
#[test]
106+
fn timed_out_identity_discovery_retries_instead_of_degrading_forever() {
107+
assert!(matches!(
108+
identity_discovery_disposition(crate::worktree::GitRepoIdentityOutcome::Unknown),
109+
IdentityDiscoveryDisposition::Retry
110+
));
111+
assert!(matches!(
112+
identity_discovery_disposition(crate::worktree::GitRepoIdentityOutcome::NotFound),
113+
IdentityDiscoveryDisposition::Degraded
114+
));
115+
}
116+
105117
/// The shared coordinator must not start a second store-writing lifetime while
106118
/// the first one is held. Paused Tokio time plus Notify/oneshot handshakes make
107119
/// this a scheduling-state assertion rather than a wall-clock sleep.

src/git.rs

Lines changed: 140 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,21 @@
66
//! absolute path exactly once (cached in a [`OnceLock`]) and hands every product
77
//! spawn site that cached path, so the long-running daemon never re-walks `PATH`.
88
//!
9-
//! The gix-first read paths in [`crate::branch`] and [`crate::worktree`] are
10-
//! unaffected: they still prefer in-process `gix` and only reach a `git`
11-
//! subprocess as a gated fallback. This module only changes *which* program those
12-
//! fallbacks (and the one-shot spawn sites) exec.
9+
//! The public gix-first read paths in [`crate::branch`] and [`crate::worktree`]
10+
//! are unaffected: they still prefer in-process `gix` and only reach a `git`
11+
//! subprocess as a gated fallback.
1312
1413
use std::ffi::{OsStr, OsString};
1514
use std::path::{Path, PathBuf};
16-
use std::process::{Command, Output};
15+
use std::process::{Child, Command, Output, Stdio};
1716
use std::sync::OnceLock;
17+
use std::time::{Duration, Instant};
1818

1919
/// The literal used when resolution fails, preserving today's behavior (the OS
2020
/// PATH-walks per spawn, but callers keep working).
2121
const GIT_LITERAL: &str = "git";
22+
const GIT_CAPTURE_AT_TIMEOUT: Duration = Duration::from_secs(2);
23+
const CHILD_WAIT_POLL_INTERVAL: Duration = Duration::from_millis(10);
2224

2325
/// Returns the resolved `git` program to spawn, as a cached `&'static OsStr`.
2426
///
@@ -129,6 +131,92 @@ pub(crate) fn git_capture(repo_root: &Path, args: &[&str]) -> Option<String> {
129131
(!trimmed.is_empty()).then(|| trimmed.to_string())
130132
}
131133

134+
/// Outcome of the bounded `git -C` capture used by repository identity lookup.
135+
#[derive(Debug)]
136+
pub(crate) enum GitCaptureAtResult {
137+
Captured(String),
138+
Failed,
139+
TimedOut,
140+
}
141+
142+
/// Runs `git -C <repo_root> <args>` without setting the child process working
143+
/// directory to `repo_root`.
144+
///
145+
/// Some network-backed or otherwise unhealthy project roots can block inside
146+
/// the child's initial `getcwd` when passed through [`Command::current_dir`].
147+
/// Git's `-C` resolves the repository after process startup and avoids that
148+
/// pre-argument cwd lookup. The child is killed and reaped at the hard deadline.
149+
pub(crate) fn git_capture_at(repo_root: &Path, args: &[&str]) -> GitCaptureAtResult {
150+
let mut command = git_command_at(repo_root, args);
151+
command.stdout(Stdio::piped()).stderr(Stdio::piped());
152+
let Ok(child) = command.spawn() else {
153+
return GitCaptureAtResult::Failed;
154+
};
155+
match capture_child_with_deadline(child, GIT_CAPTURE_AT_TIMEOUT) {
156+
ChildCaptureResult::Completed(output) if output.status.success() => {
157+
let Ok(text) = String::from_utf8(output.stdout) else {
158+
return GitCaptureAtResult::Failed;
159+
};
160+
let trimmed = text.trim();
161+
if trimmed.is_empty() {
162+
GitCaptureAtResult::Failed
163+
} else {
164+
GitCaptureAtResult::Captured(trimmed.to_string())
165+
}
166+
}
167+
ChildCaptureResult::TimedOut => GitCaptureAtResult::TimedOut,
168+
ChildCaptureResult::Completed(_) | ChildCaptureResult::Failed => GitCaptureAtResult::Failed,
169+
}
170+
}
171+
172+
fn git_command_at(repo_root: &Path, args: &[&str]) -> Command {
173+
let mut command = Command::new(git_program());
174+
command.arg("-C").arg(repo_root).args(args);
175+
command
176+
}
177+
178+
#[derive(Debug)]
179+
enum ChildCaptureResult {
180+
Completed(Output),
181+
Failed,
182+
TimedOut,
183+
}
184+
185+
fn capture_child_with_deadline(mut child: Child, timeout: Duration) -> ChildCaptureResult {
186+
let deadline = Instant::now() + timeout;
187+
loop {
188+
match child.try_wait() {
189+
Ok(Some(_)) => {
190+
return child
191+
.wait_with_output()
192+
.map(ChildCaptureResult::Completed)
193+
.unwrap_or(ChildCaptureResult::Failed);
194+
}
195+
Ok(None) => {}
196+
Err(_) => {
197+
let _ = child.kill();
198+
let _ = child.wait();
199+
return ChildCaptureResult::Failed;
200+
}
201+
}
202+
203+
let now = Instant::now();
204+
if now >= deadline {
205+
let _ = child.kill();
206+
return if child.wait().is_ok() {
207+
ChildCaptureResult::TimedOut
208+
} else {
209+
ChildCaptureResult::Failed
210+
};
211+
}
212+
std::thread::sleep(
213+
deadline
214+
.saturating_duration_since(now)
215+
.min(CHILD_WAIT_POLL_INTERVAL),
216+
);
217+
}
218+
}
219+
132220
#[cfg(test)]
133221
#[allow(clippy::unwrap_used, clippy::expect_used)]
134222
mod tests {
@@ -151,6 +239,53 @@ mod tests {
151239
);
152240
}
153241

242+
#[test]
243+
fn git_at_command_uses_dash_c_without_target_current_dir() {
244+
let repo_root = Path::new("/problematic/project/root");
245+
let command = git_command_at(
246+
repo_root,
247+
&["rev-parse", "--show-toplevel", "--git-common-dir"],
248+
);
249+
250+
assert!(
251+
command.get_current_dir().is_none(),
252+
"git -C must inherit the safe daemon cwd instead of entering the target root"
253+
);
254+
assert_eq!(
255+
command
256+
.get_args()
257+
.map(std::ffi::OsStr::to_os_string)
258+
.collect::<Vec<_>>(),
259+
vec![
260+
OsString::from("-C"),
261+
repo_root.as_os_str().to_os_string(),
262+
OsString::from("rev-parse"),
263+
OsString::from("--show-toplevel"),
264+
OsString::from("--git-common-dir"),
265+
]
266+
);
267+
}
268+
269+
#[cfg(unix)]
270+
#[test]
271+
fn git_capture_deadline_kills_and_reaps_child() {
272+
let child = Command::new("/bin/sleep")
273+
.arg("30")
274+
.spawn()
275+
.expect("spawn sleeping child");
276+
let started = std::time::Instant::now();
277+
278+
let result = capture_child_with_deadline(child, std::time::Duration::from_millis(25));
279+
280+
let ChildCaptureResult::TimedOut = result else {
281+
panic!("sleeping child should time out, got {result:?}");
282+
};
283+
assert!(
284+
started.elapsed() < std::time::Duration::from_secs(2),
285+
"deadline must stop and reap the child promptly"
286+
);
287+
}
288+
154289
#[test]
155290
fn git_env_override_is_honored() {
156291
// resolve_git_program() reads GIT directly; test it in isolation so the

0 commit comments

Comments
 (0)