fix(server): failed checkpoints no longer leave temporary packs in the repository

[chhoumann]

Fixes the remaining case from #3646 (comment).

Problem

Checkpoint capture runs git add -A with a private index, but Git still writes new objects into the repository's shared object store. An untracked file larger than core.bigFileThreshold is streamed straight into .git/objects/pack/tmp_pack_*. When the 30 second VCS timeout kills git add, that temporary pack stays behind, and every later turn adds another one. Users reported 82 GB and 380 GB of abandoned packs.

Fix

Each capture now writes its objects to a private directory, <objects>/tmp_objdir-t3-checkpoint-<uuid>, through GIT_OBJECT_DIRECTORY, and reads existing objects from the real store through GIT_ALTERNATE_OBJECT_DIRECTORIES. This is the same quarantine git receive-pack uses. The private index lives in that directory too.

• On success, the capture hard-links its object files into the real store, then runs update-ref. It never replaces an existing object and publishes each pack before its index, the order Git uses for its own quarantine. Both steps are quick and run uninterruptibly, so an interruption cannot strand a pack. If publication itself fails (for example on a full disk), the files of any pack whose index was not published yet are removed again, because Git ignores a pack without its index and gc never removes one. A pack whose index is published stays, since another capture may already use it; gc reclaims it if nothing does.
• On success, failure, timeout, or interruption, the directory is removed. Nothing in the shared store is touched, so temporary packs of concurrent Git commands are safe.
• If the server crashes mid-capture, Git's own gc removes the leftover tmp_objdir-* directory once it is two weeks old.

Limits: if a capture's pack publication fails in the same instant another capture is publishing a byte-identical pack, the other capture can lose its pack; this needs a disk error plus identical concurrent work within milliseconds. A git prune without --expire run by the user mid-capture deletes the live quarantine and fails that capture, the same race the old code had on tmp_pack_*. Windows was reasoned about (; delimiter, backslash quoting, NTFS hard links) but not executed; server tests run on Linux only.

Out of scope, and unchanged: a capture over a huge file still times out every turn, so the CPU cost and the "missing checkpoint" result remain. That needs a separate policy decision.

Relation to #15297: that PR skips untracked files over 100 MiB, which changes what checkpoints capture and restore. This PR changes no behavior; it makes every killed or interrupted capture leave nothing behind, including timeouts from large repos, slow or network disks, and files under any size cap. The two are complementary.

Proof

Real server, real Claude turns. Fresh T3 home, disposable repo with one tracked file and one untracked 2 GiB incompressible file, two claude-sonnet-5-5 turns through the WebSocket API:

	main	this PR
Capture timeouts	6	6
tmp_pack_* after turn 1 / turn 2	2 / 4 (4.91 GB)	0 / 0
git count-objects garbage	4	0
Leftover quarantine directories	-	0
git fsck --strict	ok	ok

Production capture function, no mocks. The reporter's probe (real GitVcsDriver, VcsProcess, 30 s timeout):

Capture	main	this PR
Small repo	ok, 0 packs	ok, 0 packs
600 MiB untracked (bulk-checkin, succeeds)	ok, content byte-identical	ok, content byte-identical
2 GiB untracked, 1st	timeout, 1 pack (1.23 GB)	timeout, 0
2 GiB untracked, 2nd	timeout, 2 packs (2.43 GB)	timeout, 0
File now ignored	ok, 2 packs remain	ok, 0

A planted foreign tmp_pack_* survived every run, and the real index and HEAD never changed.

Scripts and raw JSON for both runs: t3-3646-evidence.zip.

Tests (GitVcsDriver.test.ts, real Git, no sleeps):

• A killed capture leaves none of its objects: a clean filter on a later file blocks git add while the large file's pack is open, then the capture is interrupted, and separately times out through the real 30 s path. The object store listing is unchanged, a concurrent command's tmp_pack_* and another capture's quarantine survive, existing refs and the real index are intact, and a later capture stores the file correctly. Fails on main with a leftover pack/tmp_pack_*.
• A publication that fails on a pack's index (ENOSPC injected) leaves no .pack behind, and one that fails on a later loose object keeps the complete pack readable. Both cases were found in review and reproduced before being fixed.
• Concurrent captures publish the same pack and both succeed, fsck --strict passes, and new object directories keep core.sharedRepository permissions. Fails if the permission copy is removed.

Fixes #3646

Made with Claude Opus 5.5 in Claude Code, running in T3 Code. Design iterated with Claude Fable 5.1 and GPT 6.0 Astra; code reviewed by both.

[coderabbitai]

Important

Review skipped

We couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting @coderabbitai full review.

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 76b834af-293f-41ce-bec6-504042f45777

📥 Commits

Reviewing files that changed from the base of the PR and between 2d81e4c and b71bfde.

📒 Files selected for processing (2)

• apps/server/src/vcs/GitVcsDriver.test.ts
• apps/server/src/vcs/GitVcsDriver.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

Checkpoint capture now writes new Git objects to a temporary quarantine in the repository object directory. It publishes recognized objects before updating the checkpoint ref, then removes the quarantine. Tests cover concurrent captures, publication failures, interruption, and timeout.

Changes

Checkpoint capture storage

Layer / File(s)	Summary
Create and configure the object quarantine apps/server/src/vcs/GitVcsDriver.ts, apps/server/src/vcs/GitVcsDriver.test.ts	Capture configures a temporary object quarantine and removes it during cleanup. The interruption test checks that private-index cleanup also removes its parent directory. The fsync test uses a scoped temporary object directory.
Publish objects and update checkpoint refs apps/server/src/vcs/GitVcsDriver.ts, apps/server/src/vcs/GitVcsDriver.test.ts	Capture publishes recognized loose objects and pack components before updating the checkpoint ref. POSIX-only tests check concurrent capture, publication failures, interruption, timeout, object-store contents, and successful capture after removing the blocker.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant GitVcsDriver
  participant Quarantine
  participant ObjectDirectory
  participant CheckpointRef
  GitVcsDriver->>Quarantine: Capture Git objects
  GitVcsDriver->>ObjectDirectory: Publish loose objects and pack components
  GitVcsDriver->>CheckpointRef: Update checkpoint ref
  GitVcsDriver->>Quarantine: Remove quarantine

Loading

Suggested reviewers: juliusmarminge, t3dotgg

Merge Risk: ⚪ Minimal · up to b71bf

Checkpoint capture now writes new Git objects to a temporary quarantine and removes it on failure, timeout or interruption. Objects are published before the checkpoint ref is updated, and the tests cover the failure paths. One narrow race in the rename fallback is documented by the author and is not a merge blocker.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to b71bf

The change substantially improves cleanup after failed captures. However, overlapping captures combined with a publication failure can remove a pack another capture needs, compromising checkpoint recovery. Normal same-workspace operations are serialized, which limits exposure, but that protection is not repository-wide.

Retained concerns

• Medium · reliability · inferred: Failure compensation can delete a shared pack adopted by another capture. After capture A creates the pack, capture B can treat its existing target as published. If A fails before recording index publication, A removes the pack without coordinating with B, which can subsequently publish the index and checkpoint ref. This newly introduced cleanup behavior can compromise rollback and recovery. Same-cwd serialization reduces normal exposure but does not cover all publishers sharing the repository object store.

Security review details

Security Blast Radius

• inferred — The identified failure-containment exposure is the selected shared Git object store and checkpoints depending on affected objects. Separate workspaces or processes using that same store can share the impact; this does not establish cross-tenant or cross-service reachability.

Trust Boundaries and Controls

• observed — Production capture derives checkpoint refs from scope identity and ordinal, delegates with the existing cwd, and serializes normal orchestration operations by cwd. These controls limit ordinary overlap but do not provide shared-object-store publication ownership.

Resilience and Maintainability Implications

• observed — Finalization removes the quarantine recursively and logs cleanup failures. Publication compensation retains completed packs and loose objects for later garbage collection, while removing components that this capture still records as incomplete.

Hardening Proposals

• proposed — Coordinate publication and compensation at shared-object-store scope, including independent processes, or use an ownership protocol that prevents removal after another publisher adopts a pack. Separately, consider recoverable tracking of incomplete shared-store publication across hard crashes.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Issue #3646 is closed and supplies historical context only. No active directly linked issue targets remain, so no linked-issue coding requirements apply.
Out of Scope Changes check	✅ Passed	The quarantine, object publication, cleanup, and related tests address the abandoned-pack problem described in #3646. The change summary identifies no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Title check	✅ Passed	The title clearly describes the main change: preventing failed checkpoint captures from leaving temporary packs in the repository.
Description check	✅ Passed	The description explains the problem, change, limitations, and detailed verification. It does not include a distinct Scope and approval section or state explicit maintainer approval, but it provides s…

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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