Sign up
View stargazersSign up

fix(server): status polling no longer locks the git index#14718

merged1
main
GitHub fork: fix/git-status-optional-locks
openedOct 2, 2026byAahalekelly
1 files changed
View on GitHubTwo-way sync enabledPR last imported 12d ago

🤖 Generated with Claude Code

Problem

The server polls git status --porcelain=2 and git diff HEAD --numstat in each open project. Both save their refreshed index under .git/index.lock. So a user's own git checkout or git commit fails with Unable to create '.git/index.lock': File exists when it runs during a poll. #9845 stops polling while someone else holds the lock, but polling can still take the lock itself.

Seen in practice: a build script's git checkout --detach <sha> failed. The server trace shows GitVcsDriver.statusDetails.status running in that repo during the same ~170 ms.

Change

diff
1 git subprocess env
2+ GIT_OPTIONAL_LOCKS=0 # git status skips saving its refreshed index
3
4 statusDetails numstat
5- git diff HEAD --numstat --
6+ git diff-index -M --numstat HEAD --
7 unborn HEAD
8- git diff --numstat
9+ git diff-files --numstat

Porcelain git diff rewrites the index whenever it finds stat-dirty entries, and it ignores GIT_OPTIONAL_LOCKS (verified on git 2.56). The plumbing equivalents never write the index. -M keeps porcelain's default rename detection, so the numstat output doesn't change.

Scope and approval

This is a small fix for an obvious bug in existing behavior: background polling shouldn't make the user's own git commands fail. Git's documentation recommends GIT_OPTIONAL_LOCKS=0 for exactly this case (background status polling).

Verification

  • Repo with a stat-dirty tracked file (touch -t 202001010000 f): the index checksum stays the same after GIT_OPTIONAL_LOCKS=0 git status and after git diff-index -M --numstat HEAD --. It changes after GIT_OPTIONAL_LOCKS=0 git diff HEAD --numstat --.
  • A throwaway integration test did the same: make a tracked file stat-dirty, call statusDetails, and assert .git/index is byte-identical. It failed before this change and passed after. I left it out of this PR.
  • diff-index -M --numstat HEAD matches git diff HEAD --numstat on renamed, edited, and staged files. On an unborn HEAD it fails with the same bad revision 'HEAD' stderr, so the existing fallback still triggers.
  • vp test run src/vcs/GitVcsDriverCore.test.ts src/vcs/GitVcsDriver.test.ts: 154 pass. The one failure, "still creates the worktree when submodule checkout fails", fails without this change too, on this machine.

Claude Opus 5.5 in Claude Code.

Checks

25 successful · 11 skipped
All checks passed
3711421

Added 1 commit

  • fix(server): status polling no longer locks the git index3711421
apps/server/src/vcs/GitVcsDriverCore.tsline 1782Open thread
……@@ -1771,10 +1774,12 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
17751780 "GitVcsDriver.statusDetails.numstat",
17761781 cwd,
1777- ["diff", "HEAD", "--numstat", "--"],
1782+ ["diff-index", "-M", "--numstat", "HEAD", "--"],
MmacroscopeappBoton line 1782Oct 2, 2026

🟡 Medium vcs/GitVcsDriverCore.ts:1782

Status reports zero insertions/deletions for files handled by configured textconv drivers, even when the converted content has changes. git diff-index does not enable textconv by default, unlike the replaced git diff; pass --textconv to preserve the previous statistics.

🤖 Copy this AI Prompt to have your agent fix this:
text
1In file @apps/server/src/vcs/GitVcsDriverCore.ts around line 1782:
2
3Status reports zero insertions/deletions for files handled by configured textconv drivers, even when the converted content has changes. `git diff-index` does not enable textconv by default, unlike the replaced `git diff`; pass `--textconv` to preserve the previous statistics.
AahalekellyOct 2, 2026

🤖 Generated with Claude Code

False positive: porcelain git diff --numstat doesn't apply textconv either; textconv only affects patch output. Checked on git 2.56. With an uppercasing textconv driver, a→A gives 1 1 from both git diff HEAD --numstat and git diff-index -M --numstat HEAD. With a strings driver on a binary file, both report - - (--stat shows Bin 12 -> 12 bytes). So diff-index produces the same counts as before, and adding --textconv doesn't change them.

MmacroscopeappBotOct 2, 2026

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

MmacroscopeappBotOct 2, 2026
Synced from GitHub

Approvability

Verdict: Approved at 3711421

Macroscope's review found this PR approvable — This is a focused one-file server bug fix that prevents background Git status polling from taking optional index locks while preserving existing status and rename-detection behavior. An unresolved Medium finding raises a separate textconv statistics concern, but the implementation itself is small and self-contained.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

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

CcoderabbitaiBotOct 2, 2026
Synced from GitHub

Review in Change Stack →

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

🧰 Additional context used
📚 Code guidelines (1)
1docs/internals/effect-services.md — configured

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: cda610d5-a1c4-403e-be3d-08b6df598f35

📥 Commits

Reviewing files that changed from the base of the PR and between 2731929610ecb64617ff5e066980302da328f658 and 3711421bf1e61425d0ba9414b8b8449af04cecc1.

📒 Files selected for processing (1)
  • apps/server/src/vcs/GitVcsDriverCore.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

Git child processes now disable optional locks by default unless input.env overrides the setting. Local status numstat collection uses updated Git diff commands, and its failure diagnostic names the command and arguments.

Changes

Git VCS updates

Layer / File(s)Summary
Git child process environment
apps/server/src/vcs/GitVcsDriverCore.ts		Git child processes receive GIT_OPTIONAL_LOCKS=0 by default. Values from input.env can override this setting.
Local status numstat commands
apps/server/src/vcs/GitVcsDriverCore.ts		Local status statistics use git diff-index -M --numstat HEAD --. For an unborn HEAD, unstaged statistics use git diff-files --numstat, while staged statistics still use cached diff. The failure diagnostic identifies the git diff-index command and arguments.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: t3dotgg, juliusmarminge

Merge Risk: ⚪ Minimal · up to 37114

The alleged textconv undercount is unchanged from the prior behavior, so no supported merge-blocking risk remains.

Architecture Summary

Architecture risk: 🔵 Low · up to 37114

The change affects 1 system.

Changed systems: apps/server

Architecture concerns No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/server (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/server/src/vcs/GitVcsDriverCore.ts: Git child processes now receive GIT_OPTIONAL_LOCKS=0 by default; input.env is spread afterward and can override it.
  • observed — Modified behavior in apps/server/src/vcs/GitVcsDriverCore.ts: Local status statistics switch from git diff HEAD --numstat -- to git diff-index -M --numstat HEAD --. On the unborn-HEAD fallback, unstaged statistics switch from git diff --numstat to git diff-files --numstat; staged statistics remain obtained from cached diff.
  • observed — Modified behavior in apps/server/src/vcs/GitVcsDriverCore.ts: The unborn-HEAD unstaged-statistics command changes from git diff to git diff-files.
  • observed — Modified behavior in apps/server/src/vcs/GitVcsDriverCore.ts: The numstat failure reports git diff-index -M --numstat HEAD -- as the failing command and updates its detail from a git diff failure to a git diff-index failure.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description check✅ PassedThe description covers the problem, implementation, scope rationale, verification steps, observed results, and the known test failure. It also identifies the agent and harness used.
Title check✅ PassedThe title clearly and concisely describes the primary change: preventing server status polling from locking the Git index.
Docstring Coverage✅ PassedNo 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 1…
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

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