fix: FS watcher when file does not exist yet (#18492)

The initial goal of this PR was to stabilise the test `fs_watch_allows_missing_file_targets`. After further investigation, it turns out that this test was always failing and the unstability was coming from a race between timeouts mostly The goal of the test was to test what happens if a notifier gets subscribed while a file does not exist yet. But actually the main code was broken and in case of a file not existing yet, the notifier used to never notify anything (even if the file ended up being created) This PR fixes the main code (and the test). For this, we basically watch the sup-directory when a file does not exist and refresh on it when the files gets created
2026-07-01 00:31:56 +08:00 · 2026-04-20 11:23:00 +01:00
parent 2a17b32dfa
commit 7d8bd69283
2 changed files with 561 additions and 112 deletions
@@ -113,46 +113,86 @@ fn is_mutating_event_filters_non_mutating_event_kinds() {

 #[test]
 fn register_dedupes_by_path_and_scope() {
+    let temp_dir = tempfile::tempdir().expect("temp dir");
+    let skills = temp_dir.path().join("skills");
+    let other_skills = temp_dir.path().join("other-skills");
+    std::fs::create_dir(&skills).expect("create skills dir");
+    std::fs::create_dir(&other_skills).expect("create other skills dir");
+
    let watcher = Arc::new(FileWatcher::noop());
    let (subscriber, _rx) = watcher.add_subscriber();
-    let _first = subscriber.register_path(path("/tmp/skills"), /*recursive*/ false);
-    let _second = subscriber.register_path(path("/tmp/skills"), /*recursive*/ false);
-    let _third = subscriber.register_path(path("/tmp/skills"), /*recursive*/ true);
-    let _fourth = subscriber.register_path(path("/tmp/other-skills"), /*recursive*/ true);
+    let _first = subscriber.register_path(skills.clone(), /*recursive*/ false);
+    let _second = subscriber.register_path(skills.clone(), /*recursive*/ false);
+    let _third = subscriber.register_path(skills.clone(), /*recursive*/ true);
+    let _fourth = subscriber.register_path(other_skills.clone(), /*recursive*/ true);

-    assert_eq!(
-        watcher.watch_counts_for_test(&path("/tmp/skills")),
-        Some((2, 1))
-    );
-    assert_eq!(
-        watcher.watch_counts_for_test(&path("/tmp/other-skills")),
-        Some((0, 1))
-    );
+    assert_eq!(watcher.watch_counts_for_test(&skills), Some((2, 1)));
+    assert_eq!(watcher.watch_counts_for_test(&other_skills), Some((0, 1)));
 }

 #[test]
 fn watch_registration_drop_unregisters_paths() {
+    let temp_dir = tempfile::tempdir().expect("temp dir");
+    let skills = temp_dir.path().join("skills");
+    std::fs::create_dir(&skills).expect("create skills dir");
+
    let watcher = Arc::new(FileWatcher::noop());
    let (subscriber, _rx) = watcher.add_subscriber();
-    let registration = subscriber.register_path(path("/tmp/skills"), /*recursive*/ true);
+    let registration = subscriber.register_path(skills.clone(), /*recursive*/ true);

    drop(registration);

-    assert_eq!(watcher.watch_counts_for_test(&path("/tmp/skills")), None);
+    assert_eq!(watcher.watch_counts_for_test(&skills), None);
 }

 #[test]
 fn subscriber_drop_unregisters_paths() {
+    let temp_dir = tempfile::tempdir().expect("temp dir");
+    let skills = temp_dir.path().join("skills");
+    std::fs::create_dir(&skills).expect("create skills dir");
+
    let watcher = Arc::new(FileWatcher::noop());
    let registration = {
        let (subscriber, _rx) = watcher.add_subscriber();
-        subscriber.register_path(path("/tmp/skills"), /*recursive*/ true)
+        subscriber.register_path(skills.clone(), /*recursive*/ true)
    };

-    assert_eq!(watcher.watch_counts_for_test(&path("/tmp/skills")), None);
+    assert_eq!(watcher.watch_counts_for_test(&skills), None);
    drop(registration);
 }

+#[test]
+fn missing_path_registers_nearest_existing_parent() {
+    // Missing targets start with a bounded non-recursive parent fallback.
+    let temp_dir = tempfile::tempdir().expect("temp dir");
+    let missing_file = temp_dir.path().join("FETCH_HEAD");
+
+    let watcher = Arc::new(FileWatcher::noop());
+    let (subscriber, _rx) = watcher.add_subscriber();
+    let registration = subscriber.register_path(missing_file.clone(), /*recursive*/ false);
+
+    assert_eq!(watcher.watch_counts_for_test(temp_dir.path()), Some((1, 0)));
+    assert_eq!(watcher.watch_counts_for_test(&missing_file), None);
+
+    drop(registration);
+
+    assert_eq!(watcher.watch_counts_for_test(temp_dir.path()), None);
+}
+
+#[test]
+fn deeply_missing_path_registers_nearest_existing_directory_ancestor() {
+    // Missing nested targets skip file prefixes and keep the fallback non-recursive.
+    let temp_dir = tempfile::tempdir().expect("temp dir");
+    std::fs::write(temp_dir.path().join("refs"), "not a dir").expect("write refs file");
+    let missing_file = temp_dir.path().join("refs").join("heads").join("main");
+
+    let watcher = Arc::new(FileWatcher::noop());
+    let (subscriber, _rx) = watcher.add_subscriber();
+    let _registration = subscriber.register_path(missing_file, /*recursive*/ false);
+
+    assert_eq!(watcher.watch_counts_for_test(temp_dir.path()), Some((1, 0)));
+}
+
 #[tokio::test]
 async fn receiver_closes_when_subscriber_drops() {
    let watcher = Arc::new(FileWatcher::noop());
@@ -299,13 +339,20 @@ async fn non_recursive_watch_ignores_grandchildren() {

 #[tokio::test]
 async fn ancestor_events_notify_child_watches() {
+    let temp_dir = tempfile::tempdir().expect("temp dir");
+    let skills_dir = temp_dir.path().join("skills");
+    let rust_dir = skills_dir.join("rust");
+    let skill_file = rust_dir.join("SKILL.md");
+    std::fs::create_dir(&skills_dir).expect("create skills dir");
+    std::fs::create_dir(&rust_dir).expect("create rust dir");
+    std::fs::write(&skill_file, "name: rust\n").expect("write skill file");
+
    let watcher = Arc::new(FileWatcher::noop());
    let (subscriber, rx) = watcher.add_subscriber();
-    let _registration =
-        subscriber.register_path(path("/tmp/skills/rust/SKILL.md"), /*recursive*/ false);
+    let _registration = subscriber.register_path(skill_file, /*recursive*/ false);
    let mut rx = ThrottledWatchReceiver::new(rx, TEST_THROTTLE_INTERVAL);

-    watcher.send_paths_for_test(vec![path("/tmp/skills")]).await;
+    watcher.send_paths_for_test(vec![skills_dir.clone()]).await;

    let event = timeout(Duration::from_secs(1), rx.recv())
        .await
@@ -314,7 +361,131 @@ async fn ancestor_events_notify_child_watches() {
    assert_eq!(
        event,
        FileWatcherEvent {
-            paths: vec![path("/tmp/skills")],
+            paths: vec![skills_dir],
+        }
+    );
+}
+
+#[tokio::test]
+async fn missing_file_watch_reports_requested_path_when_parent_changes() {
+    // Parent events for a newly-created target should report the requested file.
+    let temp_dir = tempfile::tempdir().expect("temp dir");
+    let missing_file = temp_dir.path().join("FETCH_HEAD");
+
+    let watcher = Arc::new(FileWatcher::noop());
+    let (subscriber, rx) = watcher.add_subscriber();
+    let _registration = subscriber.register_path(missing_file.clone(), /*recursive*/ false);
+    let mut rx = ThrottledWatchReceiver::new(rx, TEST_THROTTLE_INTERVAL);
+
+    watcher
+        .send_paths_for_test(vec![temp_dir.path().join("FETCH_HEAD.lock")])
+        .await;
+    let sibling_event = timeout(TEST_THROTTLE_INTERVAL, rx.recv()).await;
+    assert_eq!(sibling_event.is_err(), true);
+
+    std::fs::write(&missing_file, "origin/main\n").expect("write missing file");
+    watcher
+        .send_paths_for_test(vec![temp_dir.path().into()])
+        .await;
+
+    let event = timeout(Duration::from_secs(1), rx.recv())
+        .await
+        .expect("missing file change timeout")
+        .expect("missing file change");
+    assert_eq!(
+        event,
+        FileWatcherEvent {
+            paths: vec![missing_file],
+        }
+    );
+}
+
+#[tokio::test]
+async fn missing_file_watch_reports_requested_path_when_parent_delete_event_arrives() {
+    // Parent events should report both creation and deletion of a fallback target.
+    let temp_dir = tempfile::tempdir().expect("temp dir");
+    let missing_file = temp_dir.path().join("FETCH_HEAD");
+
+    let watcher = Arc::new(FileWatcher::noop());
+    let (subscriber, rx) = watcher.add_subscriber();
+    let _registration = subscriber.register_path(missing_file.clone(), /*recursive*/ false);
+    let mut rx = ThrottledWatchReceiver::new(rx, TEST_THROTTLE_INTERVAL);
+
+    std::fs::write(&missing_file, "origin/main\n").expect("write missing file");
+    watcher
+        .send_paths_for_test(vec![temp_dir.path().into()])
+        .await;
+    let created = timeout(Duration::from_secs(1), rx.recv())
+        .await
+        .expect("created event timeout")
+        .expect("created event");
+    assert_eq!(
+        created,
+        FileWatcherEvent {
+            paths: vec![missing_file.clone()],
+        }
+    );
+
+    std::fs::remove_file(&missing_file).expect("remove missing file");
+    watcher
+        .send_paths_for_test(vec![temp_dir.path().into()])
+        .await;
+    let deleted = timeout(Duration::from_secs(1), rx.recv())
+        .await
+        .expect("deleted event timeout")
+        .expect("deleted event");
+    assert_eq!(
+        deleted,
+        FileWatcherEvent {
+            paths: vec![missing_file],
+        }
+    );
+}
+
+#[tokio::test]
+async fn missing_directory_watch_moves_to_created_directory_for_child_events() {
+    // Missing directory watches move closer as components appear, without recursive fallback.
+    let temp_dir = tempfile::tempdir().expect("temp dir");
+    let skills_dir = temp_dir.path().join("skills");
+    let skill_file = skills_dir.join("SKILL.md");
+
+    let watcher = Arc::new(FileWatcher::noop());
+    let (subscriber, rx) = watcher.add_subscriber();
+    let _registration = subscriber.register_path(skills_dir.clone(), /*recursive*/ false);
+    let mut rx = ThrottledWatchReceiver::new(rx, TEST_THROTTLE_INTERVAL);
+
+    assert_eq!(watcher.watch_counts_for_test(temp_dir.path()), Some((1, 0)));
+    assert_eq!(watcher.watch_counts_for_test(&skills_dir), None);
+
+    std::fs::create_dir(&skills_dir).expect("create skills dir");
+    watcher
+        .send_paths_for_test(vec![temp_dir.path().into()])
+        .await;
+
+    let created = timeout(Duration::from_secs(1), rx.recv())
+        .await
+        .expect("created dir event timeout")
+        .expect("created dir event");
+    assert_eq!(
+        created,
+        FileWatcherEvent {
+            paths: vec![skills_dir.clone()],
+        }
+    );
+    assert_eq!(watcher.watch_counts_for_test(temp_dir.path()), None);
+    assert_eq!(watcher.watch_counts_for_test(&skills_dir), Some((1, 0)));
+
+    std::fs::write(&skill_file, "name: rust\n").expect("write skill file");
+    watcher.send_paths_for_test(vec![skill_file.clone()]).await;
+
+    let changed_child = timeout(Duration::from_secs(1), rx.recv())
+        .await
+        .expect("changed child event timeout")
+        .expect("changed child event");
+    assert_eq!(
+        changed_child,
+        FileWatcherEvent {
+            paths: vec![skill_file],
        }
    );
 }
@@ -354,3 +525,13 @@ async fn spawn_event_loop_filters_non_mutating_events() {
        }
    );
 }
+
+#[tokio::test]
+async fn dropping_live_watcher_releases_inner_watcher() {
+    let watcher = FileWatcher::new().expect("watcher");
+    let weak_inner = Arc::downgrade(watcher.inner.as_ref().expect("watcher inner"));
+
+    drop(watcher);
+
+    assert_eq!(weak_inner.upgrade().is_none(), true);
+}