Skip to content

(changes): run a local status() one git call at a time (#421) - #430

Merged
devsuitup merged 2 commits into
mainfrom
fix/421-status-sequential
Oct 3, 2026
Merged

devsuitup merged 2 commits into
mainfrom
fix/421-status-sequential

Conversation

@devsuitup

Copy link
Copy Markdown
Owner

Closes #421

Cause

status() in git-changes-runner.js ran git status, git diff --numstat and git diff --cached --numstat at the same time. Each may refresh a stale index and rename a new .git/index over the old one. On Windows a git that opens .git/index while another renames it fails with fatal: .git/index: index file open failed: Permission denied, and status() returned { ok: false, error }. Which test of test/git-changes-runner-real-git.test.js failed depended on which one hit the race (captured: that exact message in "a leaf symlink to a DIRECTORY ..."; Cannot read properties of undefined (reading 'some') in the .. test, from status.files being undefined).

This is a production bug: the Changes panel could show that error on a busy machine.

Change

A local status() now runs its three git calls one after the other. The remote transport keeps them parallel (the host is not Windows). Rationale: .ai/contexts/changes-view.md, "Local reads run one at a time". One Fixed line under Unreleased in CHANGELOG.md.

GIT_OPTIONAL_LOCKS=0 was tried and rejected: measured, git diff still rewrites a stale index with it set.

Evidence

  • Git-level reproduction (scratch repo, 4 processes at once, commit + edit before each round, the three calls in parallel): 4 rounds in 400 failed with the exact message above.
  • Real-git file, 4 copies at once, 12 runs: 31/31 each after the change. Before: failures in isolated runs, 1 in 6 sequential runs.
  • New test local runner .status(): never two git calls in flight at once fails on the Promise.all version (peak 3) and passes now.
  • task -d <wt> check: exit 0. The two git-changes-runner test files under Node 22 + c8: 168 pass, 0 fail, 0 cancelled.

Not verified

The race is probabilistic: 12 green runs are consistent with the fix but do not prove it alone. The proof is the mechanism plus the message captured on the old code. The 400-round git-level loop was not repeated against the fixed status().

status(), git diff --numstat and git diff --cached --numstat ran in
parallel. Each may refresh the index and rename a new .git/index over the
old one; on Windows a concurrent git then fails with "index file open
failed: Permission denied" and the panel showed an error. The remote
transport keeps its parallel calls.

Closes #421

@devsuitup devsuitup left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Review at d9bc7cd (merged with main): 0 blocking. The root cause is a production race, not a test bug. status() ran git status and both git diff --numstat calls in parallel. Each can refresh a stale index and rename .git/index over the old one, and on Windows a concurrent open then fails with "index file open failed: Permission denied". This was captured on the old code and reproduced at git level, 4 rounds in 400. The fix runs the three calls sequentially for local runners only; remote keeps them parallel. A new test asserts that two git calls are never in flight at once, and it is red on the Promise.all version. GIT_OPTIONAL_LOCKS=0 was measured and rejected.

@devsuitup
devsuitup merged commit b859660 into main Oct 3, 2026
11 checks passed
@devsuitup
devsuitup deleted the fix/421-status-sequential branch October 3, 2026 09:50
@devsuitup devsuitup mentioned this pull request Oct 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(test): git-changes-runner-real-git fails intermittently under load on Windows

1 participant