Update WatchFilter API#3
Merged
Merged
Conversation
2 tasks
kevinyang372
approved these changes
May 21, 2026
| )?; | ||
| watch_self = false; | ||
| .into_iter(); | ||
| while let Some(entry_result) = iter.next() { |
Member
There was a problem hiding this comment.
ooc why do we need to rewrite the for iterator into while let?
My understanding is we should just change should_watch in the original filter to should_watch_directory right?
Author
There was a problem hiding this comment.
iiuc the previous filter_map and filter would evaluate the entry/directory after it's already been explored, whereas WalkDir::skip_current_dir ensures that we're skipping entries before exploring them
I believe we can similarly use WalkDir::filter_entry to be slightly more ergonomic but I don't have a strong opinion
| } | ||
| } | ||
|
|
||
| /// return `DirEntry` when it is a directory |
Member
There was a problem hiding this comment.
See above but I think we should be able to keep the original logic here
MaggieShan
added a commit
to warpdotdev/warp
that referenced
this pull request
May 21, 2026
## Description * Based on change to the `notify` crate warpdotdev/notify#3 (will update cargo.toml once it's merged) * Which introduces 2 filter fns within the watch filter - `should_emit_event` and `should_register_directory` * Updates all callers of `WatchFilter::with_filter` to include the right predicates * Where `should_emit_event` filters out specific files while `should_register_directory` includes ancestors to allowlisted paths such that the directories are correctly registered * Today we use a filesystem watcher to detect git changes (branch checkouts, commits, etc.) using an allowlist - this includes paths like `HEAD`, `refs/heads/*`, `index.lock`, and ignores paths like `.git/logs/` to reduce noice * The issue is that on linux, the notify crate uses `inotify` which uses the filter to decide which directories to register/watch * This meant that with out filter `should_ignore_git_path`, directories like `./git` and `./git/refs` were not getting registered * Better fix to #11321 ## Linked Issue * [APP-4528](https://linear.app/warpdotdev/issue/APP-4528/bug-code-review-shows-inaccurate-diffs-when-switching-commits) * I believe this also fixes the flaky test mentioned in for the same reasons [CODE-1492](https://linear.app/warpdotdev/issue/CODE-1492/fix-flaky-test-commit-related-files-excluded-from-update-lists) ## Testing Tested various git commands and repo updates in a linux devbox Tested original repro steps where diffs weren't getting the events for checkout: 1. git checkout HEAD~1 2. update base branch to master in code review view 3. git checkout master 4. Bug: code review renders old/inaccurate diffs 5. Fix: code review refresh is triggered and shows clean state https://www.loom.com/share/ea1a81e5c0654fc9a6d0f20b68f7d63e Also verified that we're not properly watching changes to remote refs - so pushing commits to a remote branch is tracked: 1. git commit and git push changes to a branch 2. Bug: code review git button shows "push" as the primary action 3. Fix: code review git button shows "create pr" as the primary action - [x] I have manually tested my changes locally with `./script/run` ## Agent Mode - [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode Oz plan: https://staging.warp.dev/drive/notebook/Linux-inotify-dual-predicate-watch-filter-Warp-caller-updates-5HQUBSdF5ZqO5e8GxJmuAe ## Changelog Entries for Stable CHANGELOG-BUG-FIX: Fixes linux file watcher bug where certain git events were not getting sent
zouyonghe
pushed a commit
to zouyonghe/warp
that referenced
this pull request
May 22, 2026
* Based on change to the `notify` crate warpdotdev/notify#3 (will update cargo.toml once it's merged) * Which introduces 2 filter fns within the watch filter - `should_emit_event` and `should_register_directory` * Updates all callers of `WatchFilter::with_filter` to include the right predicates * Where `should_emit_event` filters out specific files while `should_register_directory` includes ancestors to allowlisted paths such that the directories are correctly registered * Today we use a filesystem watcher to detect git changes (branch checkouts, commits, etc.) using an allowlist - this includes paths like `HEAD`, `refs/heads/*`, `index.lock`, and ignores paths like `.git/logs/` to reduce noice * The issue is that on linux, the notify crate uses `inotify` which uses the filter to decide which directories to register/watch * This meant that with out filter `should_ignore_git_path`, directories like `./git` and `./git/refs` were not getting registered * Better fix to warpdotdev#11321 * [APP-4528](https://linear.app/warpdotdev/issue/APP-4528/bug-code-review-shows-inaccurate-diffs-when-switching-commits) * I believe this also fixes the flaky test mentioned in for the same reasons [CODE-1492](https://linear.app/warpdotdev/issue/CODE-1492/fix-flaky-test-commit-related-files-excluded-from-update-lists) Tested various git commands and repo updates in a linux devbox Tested original repro steps where diffs weren't getting the events for checkout: 1. git checkout HEAD~1 2. update base branch to master in code review view 3. git checkout master 4. Bug: code review renders old/inaccurate diffs 5. Fix: code review refresh is triggered and shows clean state https://www.loom.com/share/ea1a81e5c0654fc9a6d0f20b68f7d63e Also verified that we're not properly watching changes to remote refs - so pushing commits to a remote branch is tracked: 1. git commit and git push changes to a branch 2. Bug: code review git button shows "push" as the primary action 3. Fix: code review git button shows "create pr" as the primary action - [x] I have manually tested my changes locally with `./script/run` - [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode Oz plan: https://staging.warp.dev/drive/notebook/Linux-inotify-dual-predicate-watch-filter-Warp-caller-updates-5HQUBSdF5ZqO5e8GxJmuAe CHANGELOG-BUG-FIX: Fixes linux file watcher bug where certain git events were not getting sent (cherry picked from commit cb4fe42)
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
2 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.
Description
WatchFilterAPI to support more granular filter fns -should_emit_eventandshould_watch_directoryOz plan: https://staging.warp.dev/drive/notebook/Linux-inotify-dual-predicate-watch-filter-Warp-caller-updates-5HQUBSdF5ZqO5e8GxJmuAe
Testing