node:fs: keep the last real stat as previous when a watched file reappears

[robobun]

Repro

import fs from "node:fs";
import os from "node:os";
import path from "node:path";

const dir = fs.mkdtempSync(path.join(os.tmpdir(), "wfr-"));
const f = path.join(dir, "g.txt");
fs.writeFileSync(f, "x");

const sleep = ms => new Promise(r => setTimeout(r, ms));
const log = [];
fs.watchFile(f, { interval: 10 }, (cur, prev) => log.push([prev.ino, cur.ino, prev.size, cur.size]));

await sleep(150);
fs.unlinkSync(f); // "gone" callback
await sleep(250);
fs.writeFileSync(f, "yy"); // "back" callback: what is `previous` here?
await sleep(400);
fs.unwatchFile(f);
for (const l of log) console.log(`cb ino ${l[0]}->${l[1]} size ${l[2]}->${l[3]}`);

node: cb ino 168559930->0 size 1->0   then   cb ino 168559930->168559930 size 1->2
bun : cb ino 168559932->0 size 1->0   then   cb ino 0->168559932          size 0->2

On the reappearance callback, previous is the zeroed ENOENT placeholder instead of the last real stat. The fs docs pin this exact case: "When a file being watched by fs.watchFile() disappears and reappears, the contents of previous in the second callback event (the file's reappearance) will be the same as the contents of previous in the first callback event (its disappearance)."

Code comparing previous.ino/previous.mtime across a gone→back transition (checking whether the file was replaced while it was missing, delta-size accounting, debouncers riding out atomic saves) computes against an all-zero baseline on Bun.

Cause

StatWatcher stores the result of each poll in last_stat, substituting a zeroed PosixStat when the stat() fails. swap_and_call_listener_on_main_thread then unconditionally copies that value into the prevStat cached slot that populates previous for the next callback, so a disappearance overwrites the last real stat with zeroes.

libuv's uv_fs_poll (what node's StatWatcher is built on) only writes poll_ctx.statbuf on a successful stat; on an error it calls back with the untouched statbuf as prev and a zeroed statbuf as curr, which is why node's previous survives the disappearance.

Fix

last_stat becomes Option<PosixStat> (None = the stat failed), so "the file is gone" is no longer conflated with "the stat is all zeroes". JS still sees a zeroed stat as current while the file is missing, but the cached previous only advances on a successful stat. Change detection is unchanged: None compares equal to None, so a file that stays missing still fires exactly one callback.

Verification

test/js/node/watch/fs.watchFile.test.ts gains a case that watches a nonexistent path, creates the file, deletes it, and recreates it, waiting on the watcher for each event. It asserts previous on the reappearance matches previous from the disappearance, and that only current is zeroed while the file is gone. It fails on main with previous = { ino: 0, size: 0, mtimeMs: 0 } and passes with the fix; the repro above now prints the same lines as node v26.3.0.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 20 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4edfb9df-523b-499d-99b7-b3854e167e13

📥 Commits

Reviewing files that changed from the base of the PR and between 48ff9eb and c08924b.

📒 Files selected for processing (2)

• src/runtime/node/node_fs_stat_watcher.rs
• test/js/node/watch/fs.watchFile.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 10:35 AM PT - Jul 6th, 2026}

❌ @robobun, your commit c08924b has some failures in Build #68958 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 33451

That installs a local version of the PR into your bun-33451 executable, so you can run:

bun-33451 --bun

[claude]

LGTM — clean, well-scoped fix that matches libuv's documented uv_fs_poll semantics.

Extended reasoning...

Overview

This PR fixes fs.watchFile to preserve the last successful stat as previous when a watched file disappears and reappears, matching the behavior explicitly documented in the Node.js fs docs and implemented by libuv's uv_fs_poll. The change wraps StatWatcher.last_stat in Option<PosixStat> so "stat failed" is no longer conflated with "stat is all zeroes", and gates the cached prev_stat update on is_some(). The old inline field-by-field comparison is extracted verbatim into stats_eq(), and stat_for_js() centralizes the None → zeroed mapping for the JS-visible current value.

Security risks

None. This is a pure Node-compat behavioral fix in the stat-polling watcher; no untrusted input parsing, no auth/crypto/permissions, no new syscalls or FFI surface.

Level of scrutiny

Moderate — it touches native Rust with cross-thread state (Guarded<Option<PosixStat>>) and a GC-cached JS value. I traced each state transition (initial ENOENT, exists→gone→back, stays-missing) through restat and swap_and_call_listener_on_main_thread and they all behave correctly. Option<PosixStat> is Copy so the *value copies in get/set_last_stat remain valid; (None, None) ⇒ unchanged preserves the single-callback-while-missing behavior; stats_eq is a byte-for-byte extraction of the previous inline comparison. The only real behavioral change is skipping one prev_stat::set when the stat failed, which is exactly what libuv does.

Other factors

The new test is event-driven (predicate-based until(), no sleeps), uses atomic rename via the existing updateFile helper to avoid 0-byte intermediates, cleans up in try/finally, and asserts the precise documented invariant (back.prev equals gone.prev). The PR description cites the Node docs verbatim and the libuv mechanism. The bug hunter found nothing, and no CODEOWNERS cover these paths.

[robobun]

Rebased onto main (no change to the diff).

The red lane on the previous build was test/js/bun/cookie/cookie-map.test.ts, which was already failing on the base commit 9f29e551 and is fixed on main by #33425 (test-only). It reproduces on Linux with a plain bun bd test test/js/bun/cookie/cookie-map.test.ts at that base and passes after the rebase. The other two lanes (test/cli/update_interactive_install.test.ts, test-net-dns-custom-lookup.js) were annotated flaky and passed on retry.

[claude]

LGTM — targeted Node compat fix matching libuv's documented uv_fs_poll semantics, with a solid event-driven test.

Extended reasoning...

Overview

This PR fixes a Node.js compatibility gap in fs.watchFile(): when a watched file disappears and reappears, previous in the reappearance callback should be the last real stat (from before it vanished), not a zeroed placeholder. The Node docs explicitly specify this, and it falls out of libuv's uv_fs_poll only writing poll_ctx.statbuf on a successful stat.

The change is confined to src/runtime/node/node_fs_stat_watcher.rs:

• last_stat becomes Guarded<Option<PosixStat>> so "stat failed" is distinguishable from "stat is all zeroes".
• swap_and_call_listener_on_main_thread only advances the cached prevStat slot when last_stat.is_some().
• The field-by-field comparison is extracted verbatim into stats_eq() (same fields, still ignores atime), and stat_for_js() maps None → zeroed for the JS-visible current.
• restat()'s change-detection is behaviorally identical: (None, None) → unchanged, (Some, Some) → same comparison as before, mixed → changed.

All callers of get_last_stat/set_last_stat are in this one file, so the signature change is fully contained. The initial-stat paths (initial_stat_success/error_on_main_thread, InitialStatTask::run_owned) are updated mechanically and preserve their existing observable behavior (zeroed curr/prev on initial ENOENT).

Security risks

None. This is stat-polling bookkeeping in the Node compat layer — no parsing of untrusted input, no auth/crypto/permissions, no new syscalls or FFI surface. The threading model (work-pool restat → main-thread callback via Guarded) is unchanged.

Level of scrutiny

Moderate is appropriate: it touches cross-thread state in a refcounted watcher, but the change is a type refinement (PosixStat → Option<PosixStat>) plus one conditional guard, not a structural or lifetime change. The extracted helpers are pure refactors. The pre-existing TOCTOU between restat() writing last_stat and the main-thread callback reading it is neither introduced nor worsened.

Other factors

The new test in fs.watchFile.test.ts is well-constructed: it awaits watcher callbacks via a predicate-driven until() (no sleeps), uses the existing updateFile atomic-rename helper to avoid observing truncated intermediates, cleans up in finally, and asserts the exact documented invariant (back.prev deep-equals gone.prev on ino/size/mtimeMs, and only curr is zeroed while gone). The PR description traces the fix directly to libuv source behavior. No CODEOWNERS cover these paths and there are no outstanding human review comments.

[claude]

LGTM — focused Node compat fix that matches libuv's documented previous semantics on file reappearance.

Extended reasoning...

Overview

This PR fixes fs.watchFile() to preserve the last real stat as previous when a watched file disappears and then reappears, matching Node.js/libuv semantics. The change is confined to src/runtime/node/node_fs_stat_watcher.rs (state representation + one call-site guard) and adds a targeted test in test/js/node/watch/fs.watchFile.test.ts.

The core change is representational: last_stat becomes Guarded<Option<PosixStat>> so "stat failed" is distinguishable from "stat is all-zeroes". swap_and_call_listener_on_main_thread then only advances the cached prevStat slot when last_stat.is_some(), mirroring libuv's uv_fs_poll which leaves poll_ctx.statbuf untouched on a failed stat. Two small helpers (stat_for_js, stats_eq) are extracted; stats_eq is a byte-identical lift of the previous inline field comparison, and stat_for_js centralizes the "None → zeroed" mapping so JS still sees a zeroed current while the file is missing.

Security risks

None. This is a Node.js compatibility fix to stat-polling callback semantics. No parsing of untrusted input, no auth/crypto/permissions surface, no new syscalls or FFI. The Option<PosixStat> wrapper is Copy (PosixStat is POD #[repr(C)]), so the Guarded locking pattern is unchanged and no new memory-safety concerns are introduced.

Level of scrutiny

Low-to-moderate. The change is small, mechanical, and directly traceable to a documented Node.js behavior (quoted in both the PR description and the test comment). I traced the state machine through all four transitions (initial-ENOENT, appear, disappear, reappear) and each produces the expected (curr, prev) pair. Change detection is preserved: (None, None) short-circuits so a file that stays missing still fires exactly once. The initial-ENOENT path still writes a zeroed prevStat (via stat_for_js(&None)), matching both the old behavior and Node.

Other factors

• The new test awaits watcher events via a predicate helper rather than sleeping, uses the existing updateFile rename-based writer to avoid the O_TRUNC race, and cleans up in try/finally — consistent with the file's conventions.
• No CODEOWNERS cover the touched paths.
• The bug-hunting system found no issues.
• The pre-existing cross-thread read of last_stat in the main-thread callback is now a single get_last_stat() into a local (one lock acquisition), which is a minor improvement over the prior inline call; the underlying work-pool/main-thread interleaving is unchanged by this PR.
• The author already rebased onto main to pick up an unrelated test fix; no outstanding reviewer comments.

[robobun]

CI status

The diff is green on every lane that actually runs a test. The red lanes are a macOS agent that cannot download the build artifact.

Both builds fail the same two jobs, on the same agent, before any test executes:

:darwin: 26 aarch64 - test-bun   (agent darwin-aarch64-26.5.1-1)

Error: buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'.
Refusing to continue with a partial download (would silently fall back to the wrong binary).
    at getExecPathFromBuildKite (scripts/runner.node.mjs:2182:13)

:darwin: aarch64 - build-bun passes in both builds, so the artifact exists; the download into that one macOS 26 agent times out. That step runs before bun test, so the failure is independent of this diff.

• Build 68872 (f1b55423): 278 jobs passed, 2 failed (the above). One test/bake/dev-and-prod.test.ts entry was annotated flaky and passed on retry.
• Build 68958 (c08924ba, identical diff): the same two jobs failed the same way on the same agent.

An earlier red on test/js/bun/cookie/cookie-map.test.ts was a stale assertion on the old base commit, fixed on main by #33425 and picked up by rebasing.

Local verification

• bun bd test test/js/node/watch/ — 58 pass, 9 skip, 0 fail (debug + ASAN)
• bun bd test/js/node/test/parallel/test-fs-watchfile.js and test-fs-watchfile-ref-unref.js — exit 0
• The new test fails on the unfixed build with previous = { ino: 0, size: 0, mtimeMs: 0 } and passes with the fix
• cargo clippy -p bun_runtime and cargo fmt clean

Ready for review; the darwin lane needs either an agent that can reach the artifact store or a retry of those two jobs.

[robobun]

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-06, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.