Allow ancestor directories of allowlisted git files#11321
Closed
MaggieShan wants to merge 2 commits into
Closed
Conversation
Contributor
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Overview
This PR broadens the Git watcher allowlist so Linux can register watches on ancestor directories of Git files, and it updates tests around the new behavior.
Concerns
- Ancestor
.git/refspaths are now classified as remote-tracking refs even though the watcher routing path only handles exact tracked-ref file paths, which can drop those directory events instead of routing them through commit/shared-ref handling.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
zerx-lab
added a commit
to zerx-lab/zap
that referenced
this pull request
May 20, 2026
…n-empty" This reverts commit 445a245, reversing changes made to 6b83612. 撤销原因:PR #123 修复未生效。事后排查发现真根因在 terminal/view.rs 的 `should_subscribe_to_git_status` 上 —— 该函数要求 `GitDiffStats` chip 在 prompt 配置里才订阅 `GitRepoStatusModel`,导致 `MetadataChanged` 事件链路 在很多场景下根本不建立,PR #123 修改的 `MetadataChanged` handler 永远进 不去。该 PR 改的另一面(autoupdate/linux_test.rs 孤儿测试清理)随之 回退,如需保留请单独 cherry-pick 18f34ef。 后续重新调研方向:对齐上游 warpdotdev#9970(无条件订阅) + warpdotdev#11321(Linux inotify 监听目录而非文件)。
2 tasks
Contributor
Author
|
Closing in favour of #11464 |
MaggieShan
added a commit
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
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
1 participant
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
HEAD,refs/heads/*,index.lock, and ignores paths like.git/logs/to reduce noiceinotifyunder the hood which is non-recursive by design and registers a watch handle per directoryshould_ignore_git_pathoriginally filtered out directories like.git/, many of the git events for the allowlisted paths were never getting firedshould_ignore_git_pathto allow the related ancestor directories to the allowlisted files and not just their filesLinked Issue
Testing
Repro steps:
bug: code review would render old/inaccurate diffs
fix: code review refresh is triggered and shows clean state
https://www.loom.com/share/ea1a81e5c0654fc9a6d0f20b68f7d63e
./script/runScreenshots / Videos
Agent Mode