project: Explicitly scan on git reset and fetch - #60590
Merged
Merged
Conversation
This PR fixes a bug where uncomitting would not reflect changes in the git panel, especially evident on Linux which has non-recursive watches.
cla-bot
Bot
added
the
cla-signed
zed-community-bot
Bot
added
the
staff
NeelChotai
changed the title
5 tasks
Member
Author
|
Still getting this on nightly, reopening. |
ChristopherBiscardi
approved these changes
Jul 22, 2026
5 tasks
pull Bot
pushed a commit
to Jaleel-zhu/zed
that referenced
this pull request
Jul 24, 2026
…ed-industries#61541) # Objective When the OS file watcher loses sync (e.g. its event queue overflows under heavy fs churn), it drops pending events and reports a single `Rescan` event for the watched root. Git changes hidden behind such a rescan were silently lost, leaving the git panel and branch indicator stale until something else touched `.git`. Contributes to zed-industries#13176. May fix zed-industries#60102, though that report predates zed-industries#60660 and may already be addressed by it on nightly (see Related PRs below). Two bugs caused this, and they masked each other: 1. **A rescan never triggered a git reload.** The `Rescan` event's path is the worktree root, not something inside `.git`, so it never populated `dot_git_abs_paths` in `process_events` — and since `.git` is excluded from entry scanning, the rescan produced no `.git` events either. The dropped git changes were simply never picked up. 2. **Re-scanning reset `git_dir_scan_id` to 0.** The snapshot diff detects git changes by comparing scan ids, so re-inserting the repository with a fresh id could wipe out a bump made earlier in the same scan cycle, swallowing the corresponding `UpdatedGitRepositories` signal. The masking is why these fixes land together rather than as two PRs: the root-rescan case in `test_dot_git_dir_event_does_not_suppress_children` only passed on `main` because the buggy scan-id reset made the snapshot diff fire spuriously. Fixing either bug alone turns that (currently green) test red. ## Solution - `process_events`: a `Rescan` event now schedules a git state reload for every repository whose git directory (`dot_git`, `common_dir`, or `repository_dir`) lies under the rescanned path, covering linked worktrees and gitfile repositories, including git dirs watched outside the worktree root. - `insert_git_repository_for_path`: carry the existing `git_dir_scan_id` forward when re-inserting a repository instead of resetting it to 0. Deliberately *not* bumped either: re-insertion is snapshot bookkeeping, not evidence of a git change — bumping would trigger spurious full reloads on non-lossy paths (explicit refreshes, path-prefix scans). Only `update_git_repositories` claims that git state changed. - `changed_repos`: a `debug_assert` enforcing that `git_dir_scan_id` never regresses, so future violations of this invariant fail loudly in tests instead of manifesting as a stale git panel. ## Testing New fault-injection infrastructure and tests: - `FakeFs::simulate_watcher_overflow` models the kernel's watch queue overflowing: buffered (undelivered) events are discarded and replaced by a single `Rescan` for the given root, mirroring FSEvents `kFSEventStreamEventFlagMustScanSubDirs`, inotify `IN_Q_OVERFLOW`, and Windows `ERROR_NOTIFY_ENUM_DIR`. - `test_watcher_overflow_rescan_reloads_git_state`: a git change whose events are lost to an overflow must still be picked up via the rescan (reproduces bug 1; fails on `main`). - `test_git_update_in_same_batch_as_rescan_is_not_lost`: a git event processed in the same batch as a rescan must not lose its scan-id bump to the repository re-insertion (reproduces bug 2; fails on `main`). - `test_random_git_updates_with_watcher_overflows`: randomized property test (100 iterations) asserting that every git state change is eventually signaled via `UpdatedGitRepositories` under random event batching, delays, and overflows. Fails on `main` within the first few seeds. - `test_random_worktree_changes` now also injects watcher overflows, extending its existing convergence property to rescan reconciliation of worktree entries (this already passed; the injection guards it going forward). Full `worktree` and `fs` suites pass. Verified the directed tests exercise the intended code paths via trace logging (the same-batch test hits `update_git_repositories` stamping followed by re-insertion, distinct from the overflow test where no `.git` event arrives at all). ## Related PRs The recent stale-git-state reports trace back to three distinct mechanisms that share one symptom. This PR addresses the third: - **Events never generated** — zed-industries#60660 (merged, in this PR's base): Linux's non-recursive watcher missed nested `refs/` directories, so external commits/fetches produced no events at all. That PR (together with zed-industries#60590, which explicitly rescans after Zed-initiated reset/fetch and touches only `git_store.rs`) fixed zed-industries#60348. No overlap with this PR; a merge against current `main` is clean, and the refs-watching tests are disjoint from the overflow/rescan tests added here. - **Events coalesced** — zed-industries#59876 (open, complementary): FSEvents can merge `.git` child events into a bare `.git` `Changed` event; the signal arrives, in a shape Zed ignores. @RemcoSmitsDev's review comment there describes this PR's failure mode and calls the two fixes complementary; this is effectively the follow-up promised in that comment. Both PRs touch the same region of `process_events`, so whichever lands second needs a trivial rebase, and zed-industries#59876 flips the bare-`.git` expectation in `test_dot_git_dir_event_does_not_suppress_children` Case 2, which this PR preserves. - **Events dropped** — this PR: the watcher generated events but lost them to a queue overflow, and the resulting `Rescan` did not reach the git reload path. zed-industries#59976 and zed-industries#60098 (merged) reduced how often this happens; this PR makes git state recover correctly when it does. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed the git panel and branch indicator showing stale state after heavy file-system activity caused the file watcher to lose events
0arm
pushed a commit
to 0arm/zed
that referenced
this pull request
Jul 26, 2026
…ed-industries#61541) # Objective When the OS file watcher loses sync (e.g. its event queue overflows under heavy fs churn), it drops pending events and reports a single `Rescan` event for the watched root. Git changes hidden behind such a rescan were silently lost, leaving the git panel and branch indicator stale until something else touched `.git`. Contributes to zed-industries#13176. May fix zed-industries#60102, though that report predates zed-industries#60660 and may already be addressed by it on nightly (see Related PRs below). Two bugs caused this, and they masked each other: 1. **A rescan never triggered a git reload.** The `Rescan` event's path is the worktree root, not something inside `.git`, so it never populated `dot_git_abs_paths` in `process_events` — and since `.git` is excluded from entry scanning, the rescan produced no `.git` events either. The dropped git changes were simply never picked up. 2. **Re-scanning reset `git_dir_scan_id` to 0.** The snapshot diff detects git changes by comparing scan ids, so re-inserting the repository with a fresh id could wipe out a bump made earlier in the same scan cycle, swallowing the corresponding `UpdatedGitRepositories` signal. The masking is why these fixes land together rather than as two PRs: the root-rescan case in `test_dot_git_dir_event_does_not_suppress_children` only passed on `main` because the buggy scan-id reset made the snapshot diff fire spuriously. Fixing either bug alone turns that (currently green) test red. ## Solution - `process_events`: a `Rescan` event now schedules a git state reload for every repository whose git directory (`dot_git`, `common_dir`, or `repository_dir`) lies under the rescanned path, covering linked worktrees and gitfile repositories, including git dirs watched outside the worktree root. - `insert_git_repository_for_path`: carry the existing `git_dir_scan_id` forward when re-inserting a repository instead of resetting it to 0. Deliberately *not* bumped either: re-insertion is snapshot bookkeeping, not evidence of a git change — bumping would trigger spurious full reloads on non-lossy paths (explicit refreshes, path-prefix scans). Only `update_git_repositories` claims that git state changed. - `changed_repos`: a `debug_assert` enforcing that `git_dir_scan_id` never regresses, so future violations of this invariant fail loudly in tests instead of manifesting as a stale git panel. ## Testing New fault-injection infrastructure and tests: - `FakeFs::simulate_watcher_overflow` models the kernel's watch queue overflowing: buffered (undelivered) events are discarded and replaced by a single `Rescan` for the given root, mirroring FSEvents `kFSEventStreamEventFlagMustScanSubDirs`, inotify `IN_Q_OVERFLOW`, and Windows `ERROR_NOTIFY_ENUM_DIR`. - `test_watcher_overflow_rescan_reloads_git_state`: a git change whose events are lost to an overflow must still be picked up via the rescan (reproduces bug 1; fails on `main`). - `test_git_update_in_same_batch_as_rescan_is_not_lost`: a git event processed in the same batch as a rescan must not lose its scan-id bump to the repository re-insertion (reproduces bug 2; fails on `main`). - `test_random_git_updates_with_watcher_overflows`: randomized property test (100 iterations) asserting that every git state change is eventually signaled via `UpdatedGitRepositories` under random event batching, delays, and overflows. Fails on `main` within the first few seeds. - `test_random_worktree_changes` now also injects watcher overflows, extending its existing convergence property to rescan reconciliation of worktree entries (this already passed; the injection guards it going forward). Full `worktree` and `fs` suites pass. Verified the directed tests exercise the intended code paths via trace logging (the same-batch test hits `update_git_repositories` stamping followed by re-insertion, distinct from the overflow test where no `.git` event arrives at all). ## Related PRs The recent stale-git-state reports trace back to three distinct mechanisms that share one symptom. This PR addresses the third: - **Events never generated** — zed-industries#60660 (merged, in this PR's base): Linux's non-recursive watcher missed nested `refs/` directories, so external commits/fetches produced no events at all. That PR (together with zed-industries#60590, which explicitly rescans after Zed-initiated reset/fetch and touches only `git_store.rs`) fixed zed-industries#60348. No overlap with this PR; a merge against current `main` is clean, and the refs-watching tests are disjoint from the overflow/rescan tests added here. - **Events coalesced** — zed-industries#59876 (open, complementary): FSEvents can merge `.git` child events into a bare `.git` `Changed` event; the signal arrives, in a shape Zed ignores. @RemcoSmitsDev's review comment there describes this PR's failure mode and calls the two fixes complementary; this is effectively the follow-up promised in that comment. Both PRs touch the same region of `process_events`, so whichever lands second needs a trivial rebase, and zed-industries#59876 flips the bare-`.git` expectation in `test_dot_git_dir_event_does_not_suppress_children` Case 2, which this PR preserves. - **Events dropped** — this PR: the watcher generated events but lost them to a queue overflow, and the resulting `Rescan` did not reach the git reload path. zed-industries#59976 and zed-industries#60098 (merged) reduced how often this happens; this PR makes git state recover correctly when it does. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed the git panel and branch indicator showing stale state after heavy file-system activity caused the file watcher to lose events
This was referenced Jul 31, 2026
jolutz
pushed a commit
to jolutz/zed
that referenced
this pull request
Aug 8, 2026
This PR fixes a bug where uncomitting would not reflect changes in the git panel, and fetching would not update the upstream ahead/behind counts in the git panel. This was especially evident on Linux which has non-recursive watches. --- Release Notes: - Fixed changes not appearing in the git panel after uncommitting - Fixed the git panel not updating ahead/behind counts after a fetch
jolutz
pushed a commit
to jolutz/zed
that referenced
this pull request
Aug 8, 2026
…ed-industries#61541) # Objective When the OS file watcher loses sync (e.g. its event queue overflows under heavy fs churn), it drops pending events and reports a single `Rescan` event for the watched root. Git changes hidden behind such a rescan were silently lost, leaving the git panel and branch indicator stale until something else touched `.git`. Contributes to zed-industries#13176. May fix zed-industries#60102, though that report predates zed-industries#60660 and may already be addressed by it on nightly (see Related PRs below). Two bugs caused this, and they masked each other: 1. **A rescan never triggered a git reload.** The `Rescan` event's path is the worktree root, not something inside `.git`, so it never populated `dot_git_abs_paths` in `process_events` — and since `.git` is excluded from entry scanning, the rescan produced no `.git` events either. The dropped git changes were simply never picked up. 2. **Re-scanning reset `git_dir_scan_id` to 0.** The snapshot diff detects git changes by comparing scan ids, so re-inserting the repository with a fresh id could wipe out a bump made earlier in the same scan cycle, swallowing the corresponding `UpdatedGitRepositories` signal. The masking is why these fixes land together rather than as two PRs: the root-rescan case in `test_dot_git_dir_event_does_not_suppress_children` only passed on `main` because the buggy scan-id reset made the snapshot diff fire spuriously. Fixing either bug alone turns that (currently green) test red. ## Solution - `process_events`: a `Rescan` event now schedules a git state reload for every repository whose git directory (`dot_git`, `common_dir`, or `repository_dir`) lies under the rescanned path, covering linked worktrees and gitfile repositories, including git dirs watched outside the worktree root. - `insert_git_repository_for_path`: carry the existing `git_dir_scan_id` forward when re-inserting a repository instead of resetting it to 0. Deliberately *not* bumped either: re-insertion is snapshot bookkeeping, not evidence of a git change — bumping would trigger spurious full reloads on non-lossy paths (explicit refreshes, path-prefix scans). Only `update_git_repositories` claims that git state changed. - `changed_repos`: a `debug_assert` enforcing that `git_dir_scan_id` never regresses, so future violations of this invariant fail loudly in tests instead of manifesting as a stale git panel. ## Testing New fault-injection infrastructure and tests: - `FakeFs::simulate_watcher_overflow` models the kernel's watch queue overflowing: buffered (undelivered) events are discarded and replaced by a single `Rescan` for the given root, mirroring FSEvents `kFSEventStreamEventFlagMustScanSubDirs`, inotify `IN_Q_OVERFLOW`, and Windows `ERROR_NOTIFY_ENUM_DIR`. - `test_watcher_overflow_rescan_reloads_git_state`: a git change whose events are lost to an overflow must still be picked up via the rescan (reproduces bug 1; fails on `main`). - `test_git_update_in_same_batch_as_rescan_is_not_lost`: a git event processed in the same batch as a rescan must not lose its scan-id bump to the repository re-insertion (reproduces bug 2; fails on `main`). - `test_random_git_updates_with_watcher_overflows`: randomized property test (100 iterations) asserting that every git state change is eventually signaled via `UpdatedGitRepositories` under random event batching, delays, and overflows. Fails on `main` within the first few seeds. - `test_random_worktree_changes` now also injects watcher overflows, extending its existing convergence property to rescan reconciliation of worktree entries (this already passed; the injection guards it going forward). Full `worktree` and `fs` suites pass. Verified the directed tests exercise the intended code paths via trace logging (the same-batch test hits `update_git_repositories` stamping followed by re-insertion, distinct from the overflow test where no `.git` event arrives at all). ## Related PRs The recent stale-git-state reports trace back to three distinct mechanisms that share one symptom. This PR addresses the third: - **Events never generated** — zed-industries#60660 (merged, in this PR's base): Linux's non-recursive watcher missed nested `refs/` directories, so external commits/fetches produced no events at all. That PR (together with zed-industries#60590, which explicitly rescans after Zed-initiated reset/fetch and touches only `git_store.rs`) fixed zed-industries#60348. No overlap with this PR; a merge against current `main` is clean, and the refs-watching tests are disjoint from the overflow/rescan tests added here. - **Events coalesced** — zed-industries#59876 (open, complementary): FSEvents can merge `.git` child events into a bare `.git` `Changed` event; the signal arrives, in a shape Zed ignores. @RemcoSmitsDev's review comment there describes this PR's failure mode and calls the two fixes complementary; this is effectively the follow-up promised in that comment. Both PRs touch the same region of `process_events`, so whichever lands second needs a trivial rebase, and zed-industries#59876 flips the bare-`.git` expectation in `test_dot_git_dir_event_does_not_suppress_children` Case 2, which this PR preserves. - **Events dropped** — this PR: the watcher generated events but lost them to a queue overflow, and the resulting `Rescan` did not reach the git reload path. zed-industries#59976 and zed-industries#60098 (merged) reduced how often this happens; this PR makes git state recover correctly when it does. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed the git panel and branch indicator showing stale state after heavy file-system activity caused the file watcher to lose events
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes a bug where uncomitting would not reflect changes in the git panel, and fetching would not update the upstream ahead/behind counts in the git panel. This was especially evident on Linux which has non-recursive watches.
Release Notes: