[codex] Prevent background Git fetches from leaking temp packs

[juliusmarminge]

Summary

• remove only newly-created tmp_pack_* files after a failed background status fetch
• fail closed when a root or linked-worktree FETCH_HEAD.lock indicates concurrent fetch activity
• expose upstream refresh failures separately from stale status reads so the existing scheduler applies exponential backoff
• extend the status-fetch timeout from 5 seconds to 1 minute and cache failures for 30 seconds

Root cause

The status refresher killed every fetch after 5 seconds. Repositories whose fetches took longer could leave .git/objects/pack/tmp_pack_* files behind on every attempt. The driver then swallowed the refresh failure while returning stale status, so the broadcaster observed a successful status read and never activated its failure backoff.

Impact

Long-running or failed background fetches no longer accumulate newly-created temporary packs. Existing temporary packs are deliberately preserved, and cleanup is skipped whenever concurrent Git fetch activity cannot be ruled out.

This PR is stacked on #2679.

Closes #3525.

Validation

• vp check
• vp run typecheck
• vp test apps/server/src/vcs/GitVcsDriverCore.test.ts apps/server/src/vcs/VcsStatusBroadcaster.test.ts apps/server/src/git/GitWorkflowService.test.ts (47 tests)

Note

Prevent background Git fetches from leaking temporary pack files on failure

• Adds cleanup logic in fetchRemoteForStatus that snapshots tmp_pack_* files before a fetch and removes any newly created ones on failure, skipping cleanup if a FETCH_HEAD.lock is active (linked worktree in progress).
• Increases STATUS_UPSTREAM_REFRESH_TIMEOUT to 1 minute and STATUS_UPSTREAM_REFRESH_FAILURE_COOLDOWN to 30 seconds.
• Adds GitVcsDriver.refreshStatusUpstream and GitWorkflowService.refreshStatusUpstream, wired into VcsStatusBroadcaster so remote status refreshes run a single explicit fetch before invalidating the cache.
• Behavioral Change: non-zero exit codes from git fetch during status refresh now surface as GitCommandError instead of being silently swallowed.

^{Macroscope summarized 5834060.}

---

Note

Medium Risk
Touches background git fetch and filesystem cleanup in .git/objects/pack; incorrect lock or cleanup logic could interfere with concurrent Git activity, though tests cover lock skip and pack removal.

Overview
Background upstream fetches for VCS status no longer leave behind new tmp_pack_* files when they fail or time out. fetchRemoteForStatus snapshots pack dir contents before fetch, surfaces non-zero exits as GitCommandError (instead of swallowing them), and on failure removes only new tmp_pack_* files—skipping cleanup when FETCH_HEAD.lock is present on the common dir or linked worktrees.

Fetch tuning changes: status upstream timeout 5s → 1 minute, failure cache cooldown 5s → 30s.

A dedicated refreshStatusUpstream path is added on GitVcsDriver / GitWorkflowService. VcsStatusBroadcaster calls it before invalidating remote cache and reads remote status with refreshUpstream: false, so fetch failures can drive backoff without discarding cached snapshots.

^{Reviewed by Cursor Bugbot for commit 5834060. Bugbot is set up for automated code reviews on this repo. Configure here.}

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 464bbb3b-a772-4a68-9e19-6cc604e2b870

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/fix-git-fetch-temp-pack-leaks

---

_{Comment @coderabbitai help to get the list of available commands.}

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Deletes unrelated fetch temp packs
  • Expanded the lock check to also detect gc.pid, preventing the cleanup from deleting tmp_pack_* files created by concurrent git gc or git repack operations that don't hold FETCH_HEAD.lock.

Or push these changes by commenting:

@cursor push dbbfa6798f

Preview (dbbfa6798f)

diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts
--- a/apps/server/src/vcs/GitVcsDriverCore.ts
+++ b/apps/server/src/vcs/GitVcsDriverCore.ts
@@ -968,7 +968,7 @@
     ];
   });
 
-  const hasActiveFetchLock = Effect.fn("GitVcsDriver.hasActiveFetchLock")(function* (
+  const hasActiveGitPackLock = Effect.fn("GitVcsDriver.hasActiveGitPackLock")(function* (
     gitCommonDir: string,
   ): Effect.fn.Return<boolean> {
     const lockPaths = yield* listFetchHeadLockPaths(gitCommonDir);
@@ -988,6 +988,14 @@
         return true;
       }
     }
+    const gcPidExists = yield* fileSystem.exists(path.join(gitCommonDir, "gc.pid")).pipe(
+      Effect.catchTags({
+        PlatformError: () => Effect.succeed(false),
+      }),
+    );
+    if (gcPidExists) {
+      return true;
+    }
     return false;
   });
 
@@ -1004,13 +1012,10 @@
       if (newFiles.length === 0) {
         return;
       }
-      if (yield* hasActiveFetchLock(gitCommonDir)) {
-        yield* Effect.logWarning(
-          "Skipped temporary Git pack cleanup while a fetch lock is active",
-          {
-            temporaryPackCount: newFiles.length,
-          },
-        );
+      if (yield* hasActiveGitPackLock(gitCommonDir)) {
+        yield* Effect.logWarning("Skipped temporary Git pack cleanup while a git lock is active", {
+          temporaryPackCount: newFiles.length,
+        });
         return;
       }

_{You can send follow-ups to the cloud agent here.}

^{Reviewed by Cursor Bugbot for commit 5834060. Configure here.}

[cursor]

Deletes unrelated fetch temp packs

Medium Severity

After a failed status fetchRemoteForStatus, cleanup removes every new tmp_pack_* name versus a pre-fetch snapshot, not packs created only by that fetch. Another git fetch (for example fetchRemote) can fail, release FETCH_HEAD.lock, and leave temp packs that this path then deletes.

 

^{Reviewed by Cursor Bugbot for commit 5834060. Configure here.}

[macroscopeapp]

Approvability

Verdict: Needs human review

This bug fix introduces new file cleanup logic with an unresolved review comment identifying a potential race condition where temp packs from concurrent fetches could be incorrectly deleted. The substantive concern about correctness warrants human review.

^{You can customize Macroscope's approvability policy. Learn more.}