Skip to content

Clean up leaked tmp_pack files after failed status refresh#2026

Open
SAKETH11111 wants to merge 2 commits into
pingdotgg:mainfrom
SAKETH11111:fix-git-refresh-tmp-pack-leak
Open

Clean up leaked tmp_pack files after failed status refresh#2026
SAKETH11111 wants to merge 2 commits into
pingdotgg:mainfrom
SAKETH11111:fix-git-refresh-tmp-pack-leak

Conversation

@SAKETH11111
Copy link
Copy Markdown
Contributor

@SAKETH11111 SAKETH11111 commented Apr 15, 2026

Closes #1965

What was broken

Background git status refreshes could time out and leave new tmp_pack_* files behind in .git/objects/pack.
Repeated failures could accumulate those temporary pack files until the repository disk usage became destructive.

Root cause

The background upstream refresh path stopped at the failed fetch result and never cleaned up temp pack artifacts created by that refresh attempt.

What changed

  • snapshot existing tmp_pack_* files before each background refresh fetch
  • remove only newly created tmp_pack_* files when that refresh fails
  • skip cleanup when FETCH_HEAD.lock indicates another fetch is active
  • add regression coverage for both cleanup and lock-guard behavior

Validation

  • bun fmt
  • bun lint
  • bun run --cwd apps/server test src/git/Layers/GitCore.test.ts

Repo-wide bun typecheck is currently failing in untouched apps/server/src/provider/Layers/ClaudeAdapter.ts on this branch base, so I did not widen this bugfix PR to include unrelated provider changes.

Note

Clean up leaked tmp_pack_ files after failed git fetch during status refresh

  • When a git fetch fails during fetchRemoteForStatus, any tmp_pack_* files created during the failed fetch are removed from objects/pack
  • Cleanup is skipped if a FETCH_HEAD.lock exists in the main git dir or any linked worktree, indicating a concurrent fetch is in progress
  • Introduces TEMPORARY_PACK_FILE_PREFIX constant and helper functions: readTemporaryPackFiles, listFetchHeadLockPaths, hasConcurrentFetchLock, and removeLeakedTemporaryPackFiles in GitCore.ts
  • Removal errors are tolerated and logged as warnings; pre-existing tmp_pack_* files and non-temporary pack files are preserved
  • Behavioral Change: fetchRemoteForStatus no longer passes allowNonZeroExit and now uses a fallbackErrorMessage for the fetch call

Macroscope summarized 622083d.


Note

Medium Risk
Adds filesystem cleanup and lock detection around background git fetch during statusDetails, which could accidentally remove files or behave incorrectly under unusual repo layouts/concurrent operations if the heuristics are wrong.

Overview
Prevents disk bloat from failed background upstream refreshes by snapshotting pre-existing .git/objects/pack/tmp_pack_* files before the refresh git fetch and deleting only newly created tmp_pack_* artifacts when the fetch fails.

Cleanup is skipped when FETCH_HEAD.lock is present in the main repo or any linked worktree (indicating another fetch is in progress), and new tests cover both the cleanup behavior and the lock-guard scenarios.

Reviewed by Cursor Bugbot for commit 622083d. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced error recovery with improved cleanup of temporary files following unsuccessful git operations
    • Added concurrent operation detection to prevent cleanup conflicts during simultaneous git operations
  • Tests

    • Added test coverage for failure recovery and cleanup scenarios
    • Added test coverage for concurrent operation handling

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 15, 2026

📝 Walkthrough

Walkthrough

Implement cleanup mechanism for leaked temporary pack files created during failed git fetch operations. Records existing tmp_pack_* files before fetch, then removes newly-created ones on fetch failure, with concurrency protection via FETCH_HEAD.lock detection.

Changes

Cohort / File(s) Summary
Core fetch cleanup logic
apps/server/src/git/Layers/GitCore.ts
Modified fetchRemoteForStatus to track pre-fetch temporary pack files and clean up new ones on fetch failure, respecting concurrent fetch locks and logging warnings.
Test coverage for cleanup
apps/server/src/git/Layers/GitCore.test.ts
Added two it.effect tests: one verifying cleanup removes newly-created tmp_pack_new while preserving existing files, another verifying cleanup is skipped when FETCH_HEAD.lock indicates concurrent fetch.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A fetch that failed would leave its trace,
Temp pack files scattered all over the place—
But now we're clever, we cleanup with care,
Recording before, removing what's spare!
No more disk-filling despair!
hops away satisfied

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: cleaning up leaked tmp_pack files after failed status refresh attempts.
Linked Issues check ✅ Passed The PR fulfills all coding requirements from issue #1965: snapshots tmp_pack files before fetch, removes only newly created ones on failure, skips cleanup on concurrent fetch lock, and adds regression tests.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the tmp_pack file leak issue; no out-of-scope changes detected in GitCore.ts or GitCore.test.ts.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The pull request description is well-structured, comprehensive, and aligns with the template requirements.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Apr 15, 2026
@SAKETH11111 SAKETH11111 marked this pull request as ready for review April 15, 2026 03:40
@macroscopeapp
Copy link
Copy Markdown
Contributor

macroscopeapp Bot commented Apr 15, 2026

Approvability

Verdict: Approved

This is a straightforward bug fix that adds cleanup of leaked temporary git pack files after failed fetch operations. The change is well-isolated, includes appropriate safety checks (concurrent fetch lock detection), and has comprehensive test coverage for the new behavior. It only affects error paths and doesn't modify normal operation.

You can customize Macroscope's approvability policy. Learn more.

@SAKETH11111
Copy link
Copy Markdown
Contributor Author

@codex review
@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 15, 2026

@SAKETH11111 I'll review the changes in this PR right away!

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e0d5ee6bc9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/server/src/git/Layers/GitCore.ts Outdated
roni-estein added a commit to roni-estein/t3code that referenced this pull request Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Background git fetch retries leak tmp_pack files in .git/objects/pack and can fill disk

1 participant