Skip to content

Commit 877e835

Browse files
authored
Merge pull request #820 from git-ai-project/fix/wrapper-state-timeout-flood
fix: eliminate wrapper state timeout flood in async daemon mode
2 parents 10cfc12 + be1c043 commit 877e835

File tree

2 files changed

+81
-34
lines changed

2 files changed

+81
-34
lines changed

src/commands/git_handlers.rs

Lines changed: 73 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,29 @@ pub fn handle_git(args: &[String]) {
113113
// Async mode: wrapper should behave as a pure passthrough to git,
114114
// but capture and send authoritative pre/post state to the daemon.
115115
if config::Config::get().feature_flags().async_mode {
116+
let parsed = parse_git_cli_args(args);
117+
118+
// Read-only commands don't need wrapper state (the daemon fast-paths
119+
// their trace events and never processes them through the normalizer).
120+
// Skip the invocation_id so we can also suppress trace2 for them,
121+
// avoiding unnecessary daemon work and wrapper_states memory leaks.
122+
let is_read_only = parsed
123+
.command
124+
.as_deref()
125+
.is_some_and(crate::git::command_classification::is_definitely_read_only_command);
126+
127+
if is_read_only {
128+
let exit_status = proxy_to_git(args, false, None, None);
129+
exit_with_status(exit_status);
130+
}
131+
116132
// Initialize the daemon telemetry handle so we can send wrapper state
117-
let _ = crate::daemon::telemetry_handle::init_daemon_telemetry_handle();
133+
if let crate::daemon::telemetry_handle::DaemonTelemetryInitResult::Failed(e) =
134+
crate::daemon::telemetry_handle::init_daemon_telemetry_handle()
135+
{
136+
debug_log(&format!("wrapper: daemon telemetry init failed: {}", e));
137+
}
118138

119-
let parsed = parse_git_cli_args(args);
120139
let repository = find_repository(&parsed.global_args).ok();
121140
let worktree = repository.as_ref().and_then(|r| r.workdir().ok());
122141

@@ -125,13 +144,17 @@ pub fn handle_git(args: &[String]) {
125144
.and_then(crate::git::repo_state::read_head_state_for_worktree);
126145
let invocation_id = uuid::Uuid::new_v4().to_string();
127146

147+
// Send pre-state BEFORE running git so it's available when the daemon
148+
// processes the atexit trace event and starts the wrapper state timeout.
149+
send_wrapper_pre_state_to_daemon(&invocation_id, worktree.as_deref(), &pre_state);
150+
128151
let exit_status = proxy_to_git(args, false, None, Some(&invocation_id));
129152

130153
let post_state = worktree
131154
.as_deref()
132155
.and_then(crate::git::repo_state::read_head_state_for_worktree);
133156

134-
send_wrapper_states_to_daemon(&invocation_id, worktree.as_deref(), pre_state, post_state);
157+
send_wrapper_post_state_to_daemon(&invocation_id, worktree.as_deref(), &post_state);
135158

136159
// After a successful commit, wait briefly for the daemon to produce an
137160
// authorship note so we can show stats inline (same UX as plain wrapper mode).
@@ -707,33 +730,55 @@ fn maybe_show_async_post_commit_stats(parsed: &ParsedGitInvocation, repo: &Repos
707730
}
708731
}
709732

710-
fn send_wrapper_states_to_daemon(
733+
fn head_state_to_repo_context(
734+
s: crate::git::repo_state::HeadState,
735+
) -> crate::daemon::domain::RepoContext {
736+
crate::daemon::domain::RepoContext {
737+
head: s.head,
738+
branch: s.branch,
739+
detached: s.detached,
740+
}
741+
}
742+
743+
fn send_wrapper_pre_state_to_daemon(
711744
invocation_id: &str,
712745
worktree: Option<&std::path::Path>,
713-
pre_state: Option<crate::git::repo_state::HeadState>,
714-
post_state: Option<crate::git::repo_state::HeadState>,
746+
pre_state: &Option<crate::git::repo_state::HeadState>,
715747
) {
716748
let Some(wt) = worktree else { return };
749+
let Some(pre) = pre_state.clone() else { return };
717750
let wt_str = wt.to_string_lossy().to_string();
718-
let to_repo_context =
719-
|s: crate::git::repo_state::HeadState| crate::daemon::domain::RepoContext {
720-
head: s.head,
721-
branch: s.branch,
722-
detached: s.detached,
723-
};
724-
if let Some(pre) = pre_state {
725-
crate::daemon::telemetry_handle::send_wrapper_pre_state(
726-
invocation_id,
727-
&wt_str,
728-
to_repo_context(pre),
729-
);
751+
if let Err(e) = crate::daemon::telemetry_handle::send_wrapper_pre_state(
752+
invocation_id,
753+
&wt_str,
754+
head_state_to_repo_context(pre),
755+
) {
756+
debug_log(&format!(
757+
"wrapper: failed to send pre-state for {}: {}",
758+
invocation_id, e
759+
));
730760
}
731-
if let Some(post) = post_state {
732-
crate::daemon::telemetry_handle::send_wrapper_post_state(
733-
invocation_id,
734-
&wt_str,
735-
to_repo_context(post),
736-
);
761+
}
762+
763+
fn send_wrapper_post_state_to_daemon(
764+
invocation_id: &str,
765+
worktree: Option<&std::path::Path>,
766+
post_state: &Option<crate::git::repo_state::HeadState>,
767+
) {
768+
let Some(wt) = worktree else { return };
769+
let Some(post) = post_state.clone() else {
770+
return;
771+
};
772+
let wt_str = wt.to_string_lossy().to_string();
773+
if let Err(e) = crate::daemon::telemetry_handle::send_wrapper_post_state(
774+
invocation_id,
775+
&wt_str,
776+
head_state_to_repo_context(post),
777+
) {
778+
debug_log(&format!(
779+
"wrapper: failed to send post-state for {}: {}",
780+
invocation_id, e
781+
));
737782
}
738783
}
739784

@@ -744,10 +789,10 @@ fn proxy_to_git(
744789
wrapper_invocation_id: Option<&str>,
745790
) -> std::process::ExitStatus {
746791
// Suppress trace2 for read-only commands to avoid hitting the daemon with
747-
// events that can never produce meaningful state changes. Skip suppression
748-
// when a wrapper_invocation_id is set (async mode) because the daemon
749-
// expects trace2 events to match wrapper state entries — suppressing them
750-
// would leak wrapper state entries that never get cleaned up.
792+
// events that can never produce meaningful state changes. In async mode,
793+
// read-only commands are handled before this point (no invocation_id set),
794+
// so wrapper_invocation_id is only Some for mutating commands that need
795+
// trace2 events for the daemon to match wrapper state entries.
751796
let suppress_trace2 = wrapper_invocation_id.is_none() && {
752797
let parsed = parse_git_cli_args(args);
753798
parsed

src/daemon/telemetry_handle.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -244,30 +244,32 @@ pub fn submit_cas(records: Vec<CasSyncPayload>) {
244244
let _ = send_via_daemon(&request);
245245
}
246246

247-
/// Send wrapper pre-command state to the daemon. Fire-and-forget.
247+
/// Send wrapper pre-command state to the daemon.
248+
/// Returns an error if the send fails (caller decides whether to log/ignore).
248249
pub fn send_wrapper_pre_state(
249250
invocation_id: &str,
250251
repo_working_dir: &str,
251252
repo_context: RepoContext,
252-
) {
253+
) -> Result<(), String> {
253254
let request = ControlRequest::WrapperPreState {
254255
invocation_id: invocation_id.to_string(),
255256
repo_working_dir: repo_working_dir.to_string(),
256257
repo_context,
257258
};
258-
let _ = send_via_daemon(&request);
259+
send_via_daemon(&request).map(|_| ())
259260
}
260261

261-
/// Send wrapper post-command state to the daemon. Fire-and-forget.
262+
/// Send wrapper post-command state to the daemon.
263+
/// Returns an error if the send fails (caller decides whether to log/ignore).
262264
pub fn send_wrapper_post_state(
263265
invocation_id: &str,
264266
repo_working_dir: &str,
265267
repo_context: RepoContext,
266-
) {
268+
) -> Result<(), String> {
267269
let request = ControlRequest::WrapperPostState {
268270
invocation_id: invocation_id.to_string(),
269271
repo_working_dir: repo_working_dir.to_string(),
270272
repo_context,
271273
};
272-
let _ = send_via_daemon(&request);
274+
send_via_daemon(&request).map(|_| ())
273275
}

0 commit comments

Comments
 (0)