Skip to content

fix(server): retain checkpoint index metadata for specially flagged entries - #13768

Open
XiaChuerwu wants to merge 1 commit into
pingdotgg:mainfrom
XiaChuerwu:fix/checkpoint-special-index-flags
Open

XiaChuerwu wants to merge 1 commit into
pingdotgg:mainfrom
XiaChuerwu:fix/checkpoint-special-index-flags

Conversation

@XiaChuerwu

@XiaChuerwu XiaChuerwu commented Sep 26, 2026 •

Copy link
Copy Markdown

Problem

Fixes #13741. Follow-up to #10792 and #12154: manual skip-worktree or assume-unchanged entries still discard the copied checkpoint index and force a cold rebuild. The reported non-sparse mapped-drive worktree repeatedly hit the 30-second Git timeout. Manual flags in sparse checkouts have the same fallback.

Fix

Keep ordinary entries' cached metadata and reinsert flagged entries into the private index using update-index --index-info, clearing their flags/stat without losing tracked status. Use sparse rules to distinguish manual in-cone flags from ordinary exclusions. Preserve skip bits during private inspection so Git cannot silently clear them before we invalidate stale metadata; final staging retains its existing behavior.

When repairing manual sparse entries, invalidate remaining excluded file entries' stat data too, then restore their skip bits. This captures present exclusions with same-size/same-mtime edits without deleting absent exclusions or expanding sparse directories. Restore the conservative index timestamp after all index writes. The real index/config remain untouched.

Keep the existing fallback for malformed/truncated inspection output, failed repair, excluded assume-unchanged entries, and unsupported flagged gitlinks. This composes with #13543 rather than replacing its saved-index optimization.

Verification

  • vp test run apps/server/src/vcs/GitVcsDriver.test.ts -t 'checkpoint (capture|index|refreshes|falls back)|sparse checkpoint': 49 passed, 20 skipped on Windows (exit 0).
  • Targeted lint and formatting for both changed files: passed.
  • pnpm --filter t3 typecheck: passed.
  • git diff --check: passed.

Earlier full-file run before the sparse extension: 55 passed, 1 failed. The unchanged restores empty checkpoints without changing paths outside the workspace test fails with ENOENT writing nested/added.txt; the same failure was reproduced with both original HEAD files. That unrelated fixture is not modified here.

Regressions cover ordinary index-cache reuse, h/S/s flags, streamed inspection, ignored-but-tracked edits/deletions, same-size/same-mtime changes in flagged entries, sparse exclusions, nested workspaces, split indexes, failure fallback, unchanged real index, and racy timestamps. An unflagged file with deliberately preserved stat metadata remains subject to Git's pre-existing stat-cache limitations; this is not a general fix for that case.

An earlier non-sparse packaged-app patch completed one previously failing checkpoint in 11.09 seconds after restart. This is one observation, not a benchmark guarantee or runtime verification of the later sparse extension.

Model: GPT-6 Astra. Harness: Claude Code via T3 Code.

Summary by CodeRabbit

  • Bug Fixes
    • Improved checkpoint handling for sparse checkouts and files with special Git index flags, including fallback behavior when index data cannot be safely reused.
    • Preserved edits made during checkpoint capture and ensured the user’s index and working files remain unchanged.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 26, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This production-path fix adds nontrivial Git index parsing, sparse-checkout handling, and private-index repair logic to alter when checkpoint captures reuse metadata versus rebuild it. Although the tests are extensive and the scope is localized, the runtime decision and implementation complexity merit human review.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 26, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: 665d6fb7-3acb-4a06-9f57-3a3ebd421e70

📥 Commits

Reviewing files that changed from the base of the PR and between a21b42c and 76fb14b.

📒 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 index reuse now handles assume-unchanged and skip-worktree flags by inspecting staged records and repairing flagged entries in the private index. Sparse-rule and repair failures can trigger a fresh-index fallback. Tests cover capture contents, source-index preservation, racy edits, and split indexes.

Changes

Checkpoint index reuse

Layer / File(s) Summary
Inspect and classify index flags
apps/server/src/vcs/GitVcsDriver.ts, apps/server/src/vcs/GitVcsDriver.test.ts
Inspection scans staged index records for special flags and sparse skipped entries. Tests cover separate and combined flags, including a non-ASCII path.
Repair flagged entries and validate capture
apps/server/src/vcs/GitVcsDriver.ts, apps/server/src/vcs/GitVcsDriver.test.ts
Sparse rules guide selective repair in the private index or fallback. Tests cover sparse cases, flagged-file refresh and deletion, repair failures, racy edits, and split-index modes.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge, t3dotgg

Merge Risk: ⚪ Minimal · up to 76fb1

No actionable issue remains from this review. The gitlink case rebuilds the private index as intended, and the checked sparse-rule behavior matches the implementation.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 76fb1

Checkpoint capture now repairs more flagged entries instead of rebuilding its private index. The review found no new access boundary or material security risk, and the existing fallback remains in place. Simultaneous captures were not covered by the available tests.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed handling is bounded to the repository being checkpointed and its private temporary index; the reviewed ranges show no new cross-service dependency or production entrypoint.

Trust Boundaries and Controls

  • observed — Index inspection rejects malformed or unsupported flagged states; repair errors fall back rather than using the partially repaired index, and sparse non-cone fallback refuses publication where rebuilding would lose exclusions.

Resilience and Maintainability Implications

  • observed — Tests cover injected repair failures and racy edits while checking checkpoint contents and original-index preservation; they do not establish behavior for simultaneous capture calls.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the problem, the fix, fallback behavior, scope, and verification results. It does not use the template headings “What Changed” and “Why” or include the checklist, but …
Title check ✅ Passed The title clearly identifies the server fix and the main change: preserving checkpoint index metadata for specially flagged entries.
Linked Issues check ✅ Passed The changes satisfy the coding objectives in [#13741]. GitVcsDriver.ts keeps the copied private index for reusable cases and repairs only flagged entries with update-index --index-info. This prese…
Out of Scope Changes check ✅ Passed The reviewed changes are limited to checkpoint index inspection, private-index repair, sparse-checkout handling, and automated tests for these behaviors. The sparse rule handling and expanded flag cov…
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…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 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]: Non-sparse skip-worktree entries still force full checkpoint index rebuild after #10792 / #12154

1 participant