Skip to content

The fleet watcher announces an active watch it never started on Linux #1098

Description

@williamthorsen

Problem

startWatcher treats fs.watch() returning without throwing as proof that the watch is live, then logs recursive watch active. On Linux that inference is false for a missing directory: Node returns an inert watcher and discards the ENOENT. No error event ever fires, so the process announces a mode that is not in effect and never corrects it, contradicting the module's stated contract that degradation to rescan-only is "announced, never silent".

Fleet deploys on macOS, where fs.watch() throws synchronously and the announcement is correct. The exposure is to contributors developing on Linux, and to CI, where packages/fleet/src/store/__tests__/watcher.test.ts fails:

AssertionError: expected "vi.fn()" to be called once with arguments: [ StringContaining "rescan-only" ]
Received: "recursive watch active on /tmp/watcher-ao3e0S/missing; rescan backstop every 10ms"

The test broke because it depends on fs.watch's failure contract while exercising none of its event contract. Nothing writes into the watched directory, so the debounce path and the error-event downgrade are both uncovered. The suite starts a real OS watch, never uses it, and binds itself to the one part of the contract that varies by platform.

Context

23d9143e bumped Node from 24.14.1 to 24.18.0. The behavior changed in 24.16.0, via nodejs/node#61870, which added a throwIfNoEntry option defaulting to true and gated the existing rethrow on it:

// 24.14.1
} catch (error) { if (error.code === 'ENOENT') { throw error; } }
// 24.16.0+
} catch (error) { if (!this.#options.throwIfNoEntry && error.code === 'ENOENT') { throw error; } }

The condition is inverted against the option's name and default, so the default path swallows ENOENT. Verified against 24.18.0: kFSWatchStart returns without throwing and emits no error event. Node's recursive watch is native only on macOS and Windows; on Linux it is the JS shim in lib/internal/fs/recursive_watch.js, which is why one call carries two contracts.

Platform watch(missingDir, { recursive: true }) on 24.18.0
macOS throws ENOENT synchronously
Linux returns an inert watcher, discards the ENOENT, and emits no error event

No upstream issue reports the inversion, and 24.18.0 is the newest 24.x, so the behavior stands.

The failure was masked. CI has been red at the agents package since that same commit (#1094), and pnpm --recursive stops at the first failing package, so fleet had not run since the bump. #1097 fixes the agents failure; until it merges, CI cannot reach fleet, and the result here is unobservable on a PR from this branch.

#1099 generalizes the test-seam requirement from subprocesses to OS facilities, with this defect as its motivating case.

Proposed solution

Decide degraded mode from a statSync probe of the directory rather than from whether fs.watch() threw, so the startup line names the true mode on every platform and stops depending on the contract Node changed. Announce both failure causes through a single line naming the watch target and the underlying reason, replacing fs.watch unavailable (...), which the probe makes false in the only case that fires.

Then resolve the watch through an injectable seam, consistent with createGithubAdapter's ProcessRunner and createGitAdapter's probe, so the announcement and degradation tests run against a fake and the debounce and error-event paths become reachable. The probe lands first: injecting a seam while degraded mode still keys off fs.watch throwing would make the test pass against a stub while production stayed broken on Linux.

This closes the missing-directory case, not the general weakness. A directory that exists at probe time but is unwatchable, or is removed immediately after, still announces recursive watch active and self-corrects only when the async error handler fires. Deferring the announcement until the watch is proven live would close that too, but fs.watch exposes no ready signal, so it would change what the startup line promises. Out of scope, with a caveat: a commit already on v24.x-staging silences ENOENT during the recursive scan as well, which will remove that self-correction in a later 24.x.

Acceptance criteria

Must have

  • startWatcher announces rescan-only mode for a non-existent directory on Linux as well as macOS.
  • The degraded-mode line names the watch target and the underlying reason, and does not attribute the failure to fs.watch when the directory is merely absent.
  • startWatcher resolves its watch through an injectable seam that production omits.
  • The debounce path and the error-event downgrade are covered through the seam rather than by real filesystem events.
  • One test exercises the default watch against an existing directory, so the production path stays covered.
  • packages/fleet/src/store/__tests__/watcher.test.ts passes on Linux.

Should have

  • Packages ordered after fleet in the recursive test run are confirmed green under Node 24.18.0.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions