From 56525d06ecb710802e70ebfc3a19b3d375870246 Mon Sep 17 00:00:00 2001 From: Sasha Varlamov Date: Mon, 13 Apr 2026 02:50:30 +0000 Subject: [PATCH 1/2] fix(tests): eliminate env-var race in checkpoint timeout tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three checkpoint timeout tests were using `control_request_response_timeout` which reads process-level env vars (CI, GIT_AI_TEST_DB_PATH, GITAI_TEST_DB_PATH). With `--test-threads=4`, non-serial tests calling `TmpRepo::new()` concurrently would permanently set `GIT_AI_TEST_DB_PATH` in the process environment, racing against the `EnvVarGuard` unsets in these serial tests and causing them to see `use_ci_or_test_budget=true` when they expected `false`. Fix by testing `checkpoint_control_response_timeout` directly with explicit boolean parameters — the pure logic no longer depends on env var state, making the tests deterministic and removing the need for `#[serial]`. Add three focused `#[serial]` tests for `checkpoint_control_timeout_uses_ci_or_test_budget` itself, since that function is the one that reads env vars. Co-Authored-By: Claude Sonnet 4.6 --- src/daemon.rs | 51 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/src/daemon.rs b/src/daemon.rs index bf4f7d1ac..d4d871cb0 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -8125,46 +8125,61 @@ mod tests { } #[test] - #[serial] fn checkpoint_requests_use_long_timeout_in_ci_or_test_env() { - let _unset_ci = EnvVarGuard::unset("CI"); - let _unset_legacy_test = EnvVarGuard::unset("GITAI_TEST_DB_PATH"); - let _test_db = EnvVarGuard::set("GIT_AI_TEST_DB_PATH", "/tmp/git-ai-test.db"); - assert_eq!( - control_request_response_timeout(&queued_checkpoint_request()), + checkpoint_control_response_timeout(&queued_checkpoint_request(), true), DAEMON_CHECKPOINT_RESPONSE_TIMEOUT ); assert_eq!( - control_request_response_timeout(&waited_checkpoint_request()), + checkpoint_control_response_timeout(&waited_checkpoint_request(), true), DAEMON_CHECKPOINT_RESPONSE_TIMEOUT ); } #[test] - #[serial] fn queued_checkpoint_requests_use_short_timeout_in_product_env() { - let _unset_ci = EnvVarGuard::unset("CI"); - let _unset_test = EnvVarGuard::unset("GIT_AI_TEST_DB_PATH"); - let _unset_legacy_test = EnvVarGuard::unset("GITAI_TEST_DB_PATH"); - assert_eq!( - control_request_response_timeout(&queued_checkpoint_request()), + checkpoint_control_response_timeout(&queued_checkpoint_request(), false), DAEMON_CONTROL_RESPONSE_TIMEOUT ); } #[test] - #[serial] fn waited_checkpoint_requests_use_long_timeout_in_product_env() { + assert_eq!( + checkpoint_control_response_timeout(&waited_checkpoint_request(), false), + DAEMON_CHECKPOINT_RESPONSE_TIMEOUT + ); + } + + #[test] + #[serial] + fn checkpoint_control_timeout_uses_ci_env_var() { + let _unset_test = EnvVarGuard::unset("GIT_AI_TEST_DB_PATH"); + let _unset_legacy_test = EnvVarGuard::unset("GITAI_TEST_DB_PATH"); + let _set_ci = EnvVarGuard::set("CI", "true"); + + assert!(checkpoint_control_timeout_uses_ci_or_test_budget()); + } + + #[test] + #[serial] + fn checkpoint_control_timeout_uses_test_db_env_var() { + let _unset_ci = EnvVarGuard::unset("CI"); + let _unset_legacy_test = EnvVarGuard::unset("GITAI_TEST_DB_PATH"); + let _set_test = EnvVarGuard::set("GIT_AI_TEST_DB_PATH", "/tmp/git-ai-test.db"); + + assert!(checkpoint_control_timeout_uses_ci_or_test_budget()); + } + + #[test] + #[serial] + fn checkpoint_control_timeout_false_when_no_ci_or_test_vars() { let _unset_ci = EnvVarGuard::unset("CI"); let _unset_test = EnvVarGuard::unset("GIT_AI_TEST_DB_PATH"); let _unset_legacy_test = EnvVarGuard::unset("GITAI_TEST_DB_PATH"); - assert_eq!( - control_request_response_timeout(&waited_checkpoint_request()), - DAEMON_CHECKPOINT_RESPONSE_TIMEOUT - ); + assert!(!checkpoint_control_timeout_uses_ci_or_test_budget()); } #[test] From ccc4bc7c157b58f929f15f7cf5a152887bcf6ab9 Mon Sep 17 00:00:00 2001 From: Sasha Varlamov Date: Mon, 13 Apr 2026 02:27:40 +0000 Subject: [PATCH 2/2] fix(tests): run cross-repo checkpoints synchronously in git_ai_from_working_dir In daemon mode, configure_git_ai_env sets GIT_AI_DAEMON_CHECKPOINT_DELEGATE=true, which causes checkpoints to be queued to the daemon asynchronously. git_ai_from_working_dir is used for cross-repo and cross-CWD scenarios where the checkpoint may target a *different* repo's family than self (e.g. nested subrepos). Waiting on self's family completion log after an async delegate would either time out (wrong family) or race. Fix: remove the delegate flag immediately after configure_git_ai_env so the checkpoint runs synchronously in the subprocess, guaranteeing the working log is written before the function returns. This restores correct behavior for all cross-repo attribution tests that check the working log immediately after the checkpoint call. Co-Authored-By: Claude Sonnet 4.6 --- tests/integration/repos/test_repo.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/integration/repos/test_repo.rs b/tests/integration/repos/test_repo.rs index c99d54972..c22de7c54 100644 --- a/tests/integration/repos/test_repo.rs +++ b/tests/integration/repos/test_repo.rs @@ -2324,6 +2324,13 @@ impl TestRepo { .current_dir(&absolute_working_dir); self.configure_git_ai_env(&mut command); + // Do NOT delegate checkpoints to the daemon drain here. The caller's + // CWD may differ from self's repo root (cross-repo / subrepo tests), + // so the completion log family key is unpredictable and any async wait + // would target the wrong family. Running synchronously means the + // working log is guaranteed to be written before we return. + command.env_remove("GIT_AI_DAEMON_CHECKPOINT_DELEGATE"); + if let Some(patch) = &self.config_patch && let Ok(patch_json) = serde_json::to_string(patch) {