Repository navigation
fix(git): avoid index.lock contention by using --no-optional-locks - #4805
SpencerJung wants to merge 1 commit into
Conversation
When cmux polls git status in the background, it can create index.lock and interfere with user-initiated git operations such as rebase, causing 'Unable to create .git/index.lock: File exists' errors. Add --no-optional-locks to the git status invocation in GitStatusProvider so that status checks become read-only and never take an optional lock, giving user git operations priority over background polling. Fixes manaflow-ai#4779
|
@SpencerJung is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughA single parameter addition to the ChangesGit Status Lock Prevention
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors)
✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR adds
Confidence Score: 3/5Safe to merge for local repositories, but the SSH code path retains the original lock-contention bug and should be updated before merging if SSH-backed repos are a supported configuration. The fix is correct and minimal for local git repositories, which is likely the most common case. However, fetchStatusSSH still shells out git status --porcelain without --no-optional-locks, meaning users working in SSH-backed repositories can still hit index.lock contention during background polling. The omission is a straightforward one-word fix, but it is a present gap in the stated goal of the PR. Sources/FileExplorerStore.swift — specifically the fetchStatusSSH method around line 1200, where the SSH shell command still uses git status --porcelain without the lock-avoidance flag. Important Files Changed
Sequence DiagramsequenceDiagram
participant DW as DirectoryWatcher
participant FES as FileExplorerStore
participant GPS as GitStatusProvider
participant Git as git process
DW->>FES: filesystem change event
FES->>FES: refreshGitStatus()
alt local repo
FES->>GPS: fetchStatus(directory:)
GPS->>Git: git status --no-optional-locks --porcelain ✅
Git-->>GPS: porcelain output
GPS-->>FES: [String: GitFileStatus]
else SSH repo
FES->>GPS: fetchStatusSSH(directory:destination:...)
GPS->>Git: git status --porcelain ❌ (missing --no-optional-locks)
Git-->>GPS: porcelain output
GPS-->>FES: [String: GitFileStatus]
end
FES->>FES: "gitStatusByPath = status (on MainActor)"
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/FileExplorerStore.swift (1)
1200-1200:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winApply the same
--no-optional-locksfix to the SSH git status command.The local git status invocation at line 1189 now includes
--no-optional-locks, but this SSH code path still runsgit status --porcelainwithout the flag. SincerefreshGitStatus()(lines 725-738) callsfetchStatusSSHfor SSH providers, background polling over SSH can still create.git/index.lockon the remote host and interfere with user git operations, leaving issue#4779unresolved for SSH workspaces.🔧 Proposed fix
- let cmd = "cd '\(escapedDir)' 2>/dev/null && git rev-parse --show-toplevel 2>/dev/null && echo '---GIT_STATUS---' && git status --porcelain 2>/dev/null" + let cmd = "cd '\(escapedDir)' 2>/dev/null && git rev-parse --show-toplevel 2>/dev/null && echo '---GIT_STATUS---' && git status --no-optional-locks --porcelain 2>/dev/null"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/FileExplorerStore.swift` at line 1200, The SSH path builds a shell command string named cmd (in fetchStatusSSH called from refreshGitStatus) that runs "git status --porcelain" without the --no-optional-locks flag; update that command to run git with --no-optional-locks (e.g. change "git status --porcelain" to "git --no-optional-locks status --porcelain") so remote SSH polling won't create .git/index.lock on the remote host.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/FileExplorerStore.swift`:
- Line 1200: The SSH path builds a shell command string named cmd (in
fetchStatusSSH called from refreshGitStatus) that runs "git status --porcelain"
without the --no-optional-locks flag; update that command to run git with
--no-optional-locks (e.g. change "git status --porcelain" to "git
--no-optional-locks status --porcelain") so remote SSH polling won't create
.git/index.lock on the remote host.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e0dae918-bb22-49ac-a544-9a93502687fa
📒 Files selected for processing (1)
Sources/FileExplorerStore.swift
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Superseded by #7173 (merged in 553fe35), which applies the same lock-avoidance to the local git status probe and extends it to the SSH remote variant, with regression coverage. Thanks @SpencerJung for the original diagnosis and fix — credited in the merged PR. |
This branch was successfully deployed
2 participants
Problem
When cmux polls
git statusin the background (e.g. for sidebar file-explorer badges), it can createindex.lockand interfere with user-initiated git operations such asgit rebase, causing:This is reproducible quite reliably when cmux's GitStatusProvider runs concurrently with user git commands.
Root Cause
GitStatusProvider.fetchStatusinvokesgit status --porcelainwithout any lock-avoidance flags. Git may take optional locks during status, which contend with user operations.Fix
Add
--no-optional-locksto thegit statusinvocation:This makes the status check read-only and prevents it from taking any optional lock, giving user git operations priority over background polling.
Verification
--no-optional-locksis supported in Git 2.15+ (cmux targets recent macOS)References
Fixes #4779
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by cubic
Run git status with --no-optional-locks to prevent background polling from creating index.lock and blocking user operations (e.g., rebase). This makes status checks read-only and avoids contention. Fixes #4779.
Written for commit 73b0bfd. Summary will update on new commits. Review in cubic
Summary by CodeRabbit