fix(cli): suppress watch-mode errors for paths under .git - #11305
fix(cli): suppress watch-mode errors for paths under .git#11305wahajahmed010 wants to merge 1 commit into
Conversation
Fixes biomejs#11110. `biome lint --watch` (and other watch-mode commands) emitted `internalError/io` diagnostics whenever `.git/index.lock` or any other bookkeeping file inside the project's `.git` directory changed, even when VCS integration was disabled and the directory was already covered by the project's ignore rules. This happens because the CLI's default watcher forwarded every `notify` event -- including `Err` events from the OS watcher and `Ok` events whose paths lived inside `.git` -- straight to the workspace crawler, which would then try to index or otherwise react to them. Filter paths under `.git/` in `DefaultWatcher::poll` and convert `Err` events into a tracing warning instead of a user-facing diagnostic. The workspace scanner's existing ignore contract is left untouched; this only stops the watcher itself from re-feeding events it never should have re-fed.
🦋 Changeset detectedLatest commit: 9a2c9b1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Organic activityNo automation signals detected in the analyzed events. This is an automated analysis by AgentScan |
WalkthroughThe default watcher now logs and suppresses Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@crates/biome_cli/src/runner/impls/watchers/default.rs`:
- Around line 104-129: Import Utf8Path in the tests module so the existing
helper tests compile, then add regression tests for DefaultWatcher::poll
covering error events, mixed internal VCS and non-VCS paths, and batches
containing only .git paths. Construct each event scenario through the watcher’s
public poll flow and assert the resulting behavior, rather than testing only
is_internal_vcs_path.
- Around line 104-106: Add a camino::Utf8Path import inside the cfg(test) tests
module alongside is_internal_vcs_path so its Utf8Path::new(...) usages compile;
do not modify the parent module imports.
- Around line 55-65: Update the watcher event error handling around the notify
error match to suppress errors only when they are demonstrably transient and all
affected paths are internal VCS paths. Return WatcherEvent::Error for errors
with no paths, any non-VCS path, or otherwise non-transient conditions, and add
regression tests covering these cases; import camino::Utf8Path in the test
module.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 57459e1c-bed5-451a-94db-b2c6f78ff4ad
📒 Files selected for processing (2)
.changeset/fix-watcher-git-internal-errors.mdcrates/biome_cli/src/runner/impls/watchers/default.rs
| Err(err) => { | ||
| // `notify` surfaces filesystem errors that occur on the | ||
| // watched paths (for example, transient permission | ||
| // failures while reading a file inside `.git`). Those | ||
| // events should never reach Biome's error reporter, since | ||
| // they do not reflect problems with the user's code or | ||
| // configuration. Drop the event so the watcher keeps | ||
| // running. | ||
| warn!("Watcher event error (suppressed): {err}"); | ||
| None | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 'WatcherEvent::Error|notify::Error|ErrorKind|event\.paths|fn poll' \
crates/biome_cliRepository: biomejs/biome
Length of output: 25778
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- watcher implementation ---'
cat -n crates/biome_cli/src/runner/impls/watchers/default.rs | sed -n '1,150p'
printf '%s\n' '--- watcher event and consumer contract ---'
cat -n crates/biome_cli/src/runner/watcher.rs | sed -n '1,100p'
cat -n crates/biome_cli/src/runner/mod.rs | sed -n '350,415p'
printf '%s\n' '--- notify dependency declarations and uses ---'
rg -n -C 5 'notify|RecommendedWatcher|WatcherEvent' Cargo.toml Cargo.lock crates/biome_cliRepository: biomejs/biome
Length of output: 28535
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- relevant tests and imports ---'
cat -n crates/biome_cli/src/runner/impls/watchers/default.rs | sed -n '85,160p'
rg -n -C 6 'is_internal_vcs_path|Utf8Path::|DefaultWatcher|watcher' crates/biome_cli/src/runner crates/biome_cli/tests 2>/dev/null || true
printf '%s\n' '--- notify source availability ---'
find . -path '*/notify-*' -o -path '*/notify' | head -30Repository: biomejs/biome
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- focused watcher tests ---'
sed -n '100,135p' crates/biome_cli/src/runner/impls/watchers/default.rs
printf '%s\n' '--- watcher diagnostics ---'
fd -t f . crates/biome_cli/src/runner | sort | xargs rg -n -C 5 'WatcherDiagnostic|notify|internalError/io'
printf '%s\n' '--- exact test imports and symbols ---'
python3 - <<'PY'
from pathlib import Path
p = Path("crates/biome_cli/src/runner/impls/watchers/default.rs")
text = p.read_text()
test = text.split("#[cfg(test)]", 1)[1]
print(test)
PY
printf '%s\n' '--- local cargo registry notify sources, if present ---'
find "${CARGO_HOME:-$HOME/.cargo}/registry/src" -maxdepth 3 \
\( -path '*/notify-8.2.0/src/error.rs' -o -path '*/notify-types-2.0.0/src/*' \) \
-print 2>/dev/null | head -20Repository: biomejs/biome
Length of output: 7111
🌐 Web query:
notify 8.2.0 Error type paths ErrorKind documentation
💡 Result:
In the notify crate version 8.2.0, errors are represented by the notify::Error struct, which contains two primary fields: kind and paths [1][2]. The kind field is an instance of the notify::ErrorKind enum, which categorizes the type of error that occurred [3][1]. The paths field is a Vec that contains any filesystem paths relevant to the error, if applicable [1][2]. The ErrorKind enum includes the following variants: - Generic(String): Used for platform-specific mappings or opaque internal errors [3][4]. - Io(std::io::Error): Used for standard I/O errors [3][4]. - PathNotFound: Indicates that a specified path does not exist [3][4]. - WatchNotFound: Used when attempting to remove a watch that does not exist [3][4]. - InvalidConfig(Config): Indicates an invalid value was provided in the runtime configuration [3][4]. - MaxFilesWatch: Indicates the limit on the total number of inotify watches has been reached [3][4]. The Error struct provides methods to manage these paths, such as add_path(PathBuf) and set_paths(Vec), allowing users to associate specific filesystem targets with a given error instance [2]. Errors can be generated either during the creation of a Watcher or during the event stream [1].
Citations:
- 1: https://docs.rs/notify/latest/notify/struct.Error.html
- 2: https://docs.rs/notify/%3E=5.0.0,%20%3C9.0.0/struct.Error.html
- 3: https://docs.rs/notify/latest/notify/enum.ErrorKind.html
- 4: https://docs.rs/notify/%3E=5.0.0,%20%3C9.0.0/enum.ErrorKind.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- check Rust child-module name resolution with a standalone snippet ---'
if command -v rustc >/dev/null 2>&1; then
rustc --version
if rustc - --crate-type=lib <<'RS'
use std::path::Path;
mod tests {
fn uses_parent_import() {
let _ = Path::new("x");
}
}
RS
then
echo 'parent import is visible in the child module'
else
echo 'parent import is not visible in the child module'
fi
else
echo 'rustc unavailable'
fi
printf '%s\n' '--- diagnostic conversion declarations ---'
rg -n -C 8 'impl.*(From|Into).*notify|WatcherDiagnostic|pub struct Error|notify' \
crates/biome_cli/src crates/biome_diagnostics/src crates/biome_*/* 2>/dev/null | head -250Repository: biomejs/biome
Length of output: 20442
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- existing notify error handling ---'
sed -n '200,270p' crates/biome_service/src/scanner/watcher.rs
sed -n '135,165p' crates/biome_cli/src/runner/diagnostics.rs
printf '%s\n' '--- all watcher diagnostic constructors and conversions ---'
rg -n -C 8 'WatcherDiagnostic|WatchError|NotifyError|NotifyResult|Error::from|source:' \
crates/biome_cli/src crates/biome_service/src | head -300Repository: biomejs/biome
Length of output: 32747
Preserve non-VCS watcher errors.
If a notify::Error has no paths, includes a non-VCS path, or is not demonstrably transient, return WatcherEvent::Error. Suppress only transient errors limited to internal VCS paths. Add regression tests and import camino::Utf8Path in the test module so the tests compile.
🤖 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 `@crates/biome_cli/src/runner/impls/watchers/default.rs` around lines 55 - 65,
Update the watcher event error handling around the notify error match to
suppress errors only when they are demonstrably transient and all affected paths
are internal VCS paths. Return WatcherEvent::Error for errors with no paths, any
non-VCS path, or otherwise non-transient conditions, and add regression tests
covering these cases; import camino::Utf8Path in the test module.
| #[cfg(test)] | ||
| mod tests { | ||
| use super::is_internal_vcs_path; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'mod tests|use super::is_internal_vcs_path|use camino::Utf8Path|Utf8Path::new' \
crates/biome_cli/src/runner/impls/watchers/default.rsRepository: biomejs/biome
Length of output: 1025
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- imports and tests ---'
sed -n '1,145p' crates/biome_cli/src/runner/impls/watchers/default.rs
printf '%s\n' '--- related watcher symbols and call sites ---'
rg -n -C 4 'fn poll|notify::Error|Utf8PathBuf|is_internal_vcs_path|changed_paths|poll\(' \
crates/biome_cli/src/runner/impls/watchers/default.rs crates/biome_cli/src/runner/impls/watchers
printf '%s\n' '--- standalone Rust name-resolution probe ---'
if command -v rustc >/dev/null 2>&1; then
tmp="$(mktemp --suffix=.rs)"
trap 'rm -f "$tmp" "${tmp%.rs}"' EXIT
cat >"$tmp" <<'RS'
mod parent {
use std::path::Path;
mod child {
fn uses_parent_import() {
let _ = Path::new(".");
}
}
}
fn main() {}
RS
if rustc "$tmp" -o "${tmp%.rs}" 2>&1; then
echo 'child can use the parent use binding unqualified'
else
echo 'child cannot use the parent use binding unqualified'
fi
else
echo 'rustc unavailable'
fiRepository: biomejs/biome
Length of output: 23059
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp="$(mktemp /tmp/rust-scope-XXXXXX.rs)"
bin="${tmp%.rs}"
trap 'rm -f "$tmp" "$bin"' EXIT
cat >"$tmp" <<'RS'
mod parent {
use std::path::Path;
mod child {
fn uses_parent_import() {
let _ = Path::new(".");
}
}
}
fn main() {}
RS
if rustc --crate-name rust_scope_probe "$tmp" -o "$bin" 2>/tmp/rust-scope-probe.err; then
echo 'child can use the parent use binding unqualified'
else
echo 'child cannot use the parent use binding unqualified'
cat /tmp/rust-scope-probe.err
fiRepository: biomejs/biome
Length of output: 852
Import Utf8Path inside mod tests.
The parent module’s import is not inherited by the child module, so Utf8Path::new(...) does not compile. Add use camino::Utf8Path;.
🤖 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 `@crates/biome_cli/src/runner/impls/watchers/default.rs` around lines 104 -
106, Add a camino::Utf8Path import inside the cfg(test) tests module alongside
is_internal_vcs_path so its Utf8Path::new(...) usages compile; do not modify the
parent module imports.
| #[cfg(test)] | ||
| mod tests { | ||
| use super::is_internal_vcs_path; | ||
|
|
||
| #[test] | ||
| fn detects_path_inside_dot_git_directory() { | ||
| assert!(is_internal_vcs_path(Utf8Path::new( | ||
| "/repo/.git/index.lock" | ||
| ))); | ||
| assert!(is_internal_vcs_path(Utf8Path::new("repo/.git/HEAD"))); | ||
| assert!(is_internal_vcs_path(Utf8Path::new( | ||
| "/repo/sub/.git/index.lock" | ||
| ))); | ||
| } | ||
|
|
||
| #[test] | ||
| fn leaves_non_git_paths_alone() { | ||
| assert!(!is_internal_vcs_path(Utf8Path::new("/repo/src/index.js"))); | ||
| assert!(!is_internal_vcs_path(Utf8Path::new( | ||
| "/repo/.github/workflows/ci.yml" | ||
| ))); | ||
| assert!(!is_internal_vcs_path(Utf8Path::new( | ||
| "/repo/some/path/git-notes.txt" | ||
| ))); | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 'DefaultWatcher|fn poll|WatcherEvent::Changed|WatcherEvent::Error|notify::Error' \
crates/biome_cli/src/runner/impls/watchersRepository: biomejs/biome
Length of output: 7546
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- default watcher ---'
cat -n crates/biome_cli/src/runner/impls/watchers/default.rs
printf '%s\n' '--- watcher trait and consumers ---'
rg -n -C 10 'pub trait Watcher|enum WatcherEvent|WatcherEvent::Error|poll\(\)' \
crates/biome_cli/src/runner crates/biome_cli/src | head -n 260
printf '%s\n' '--- watcher module files and test imports ---'
git ls-files crates/biome_cli/src/runner/impls/watchers
rg -n -C 5 'Utf8Path|DefaultWatcher|WatcherEvent::Changed|WatcherEvent::Error' \
crates/biome_cli/src/runner/impls/watchersRepository: biomejs/biome
Length of output: 25580
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- targeted change history for the reviewed file ---'
git diff --unified=25 -- crates/biome_cli/src/runner/impls/watchers/default.rs | head -n 240
printf '%s\n' '--- watcher diagnostics and error construction ---'
rg -n -C 12 'struct WatcherDiagnostic|enum WatcherDiagnostic|WatcherDiagnostic|WatcherEvent::Error' \
crates/biome_cli/src/runner crates/biome_cli/src | head -n 260
printf '%s\n' '--- deterministic checks for the test module and poll branches ---'
python3 - <<'PY'
from pathlib import Path
import re
p = Path("crates/biome_cli/src/runner/impls/watchers/default.rs")
text = p.read_text()
tests = text[text.index("#[cfg(test)]"):]
print("test module imports Utf8Path:",
bool(re.search(r'^\s*use\s+[^;]*\bUtf8Path\b', tests, re.M)))
print("test module references Utf8Path:",
bool(re.search(r'\bUtf8Path::new\s*\(', tests)))
print("poll suppresses Err branches:",
bool(re.search(r'Err\(err\)\s*=>\s*\{.*?None', text, re.S)))
print("poll emits Changed only after non-empty paths:",
bool(re.search(r'if paths\.is_empty\(\)\s*\{\s*None\s*\}\s*else\s*\{\s*Some\(WatcherEvent::Changed\(paths\)\)', text, re.S)))
print("poll filters .git paths:",
bool(re.search(r'\.filter\(\|path\| !is_internal_vcs_path\(path\)\)', text)))
PYRepository: biomejs/biome
Length of output: 8939
Add the missing import and DefaultWatcher::poll regression tests.
The test module uses Utf8Path but imports only is_internal_vcs_path, so the tests do not compile. Add the import, then cover error events, mixed .git and non-.git paths, and events containing only .git paths. The tests must exercise poll, not only the helper.
🤖 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 `@crates/biome_cli/src/runner/impls/watchers/default.rs` around lines 104 -
129, Import Utf8Path in the tests module so the existing helper tests compile,
then add regression tests for DefaultWatcher::poll covering error events, mixed
internal VCS and non-VCS paths, and batches containing only .git paths.
Construct each event scenario through the watcher’s public poll flow and assert
the resulting behavior, rather than testing only is_internal_vcs_path.
Source: Coding guidelines
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
Just because the bug was found for the git folder, doesn't mean the fix must be specifically made for that
3 participants
Summary
Fixes #11110.
biome lint --watch(and any other Biome command running in watch mode) used to emitinternalError/iodiagnostics whenever the OS watcher saw a change under.git/— most visibly.git/index.lockchurn. The same kind of spurious diagnostic was emitted for anyErrevent thatnotifyraised on a watched path, regardless of whether Biome had any interest in the path itself.The CLI's default watcher forwarded every event straight to the workspace crawler. The crawler then tried to index or otherwise react to paths it should never have been told about, which is where the IO error ultimately surfaced to the user.
Test Plan
Two small unit tests live in
crates/biome_cli/src/runner/impls/watchers/default.rs:detects_path_inside_dot_git_directoryexercises the newis_internal_vcs_pathhelper with paths that should be filtered (/repo/.git/index.lock,repo/.git/HEAD, a nested.git).leaves_non_git_paths_alonemakes sure that.github/, plain source files, andgit-notes.txt(which only contains the substringgit) still flow through.Manual reproduction for the original report:
biome lint --watchin a project whosebiome.jsonhas VCS integration disabled.touch ui/something.jsto trigger a normal re-lint (still works)..git/index.lock.Before this change, step 3 produced a
internalError/iodiagnostic on stderr. After this change, no diagnostic is emitted, the watcher keeps polling, and re-lints from step 2 continue to fire normally.Docs
No documentation change is needed. The watcher's ignore contract already includes VCS bookkeeping directories in the workspace scanner; this PR just extends the same expectation to the CLI-side event filter.
AI Assistance Notice
I used an AI coding assistant to explore the repository structure, locate the watcher implementation, draft the filter helper, and write this description. I reviewed the resulting code and description myself and made changes where I thought they were warranted (renaming the helper to make its scope explicit, tightening the comments, dropping the now-unused imports).