Skip to content

watch mode: re-run files replaced by an atomic save (write temp + rename) #33071

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 4 commits into
base: main
Choose a base branch
from
+183 −118
Open

watch mode: re-run files replaced by an atomic save (write temp + rename) #33071

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2a65f4d
watcher: map directory events to watched files without the resolver d…
robobun Jun 29, 2026
27e3f63
watcher: keep the SAFETY comment adjacent to its unsafe block
robobun Jun 29, 2026
01eb708
test: use the file-level default timeout and assert the re-run ran cl…
robobun Jun 29, 2026
ad81bf7
test: rely on the await using disposer for subprocess teardown
robobun Jun 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
218 changes: 102 additions & 116 deletions src/jsc/hot_reloader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1155,132 +1155,118 @@ where
}
}

if let Some(dir_ent) = entries_option {
// SAFETY: dir_ent points into rfs.entries (or a tombstoned copy);
// both outlive this loop iteration.
let dir_ent = unsafe { &mut *dir_ent };
let mut last_file_hash: bun_watcher::HashType =
bun_watcher::HashType::MAX;

for i in 0..affected_len {
let changed_name: &[u8] = if IS_KQUEUE {
affected_kqueue[i]
} else {
affected_inotify[i].unwrap().as_bytes()
};
if changed_name.is_empty()
|| changed_name[0] == b'~'
|| changed_name[0] == b'.'
{
continue;
// Map each changed name onto the watchlist by joining it to the
// watched directory's path. The per-file watch cannot be relied
// on here: it follows the inode, so a file replaced by an atomic
// save (write temp + rename) stops reporting the moment the
// rename lands, and this directory event is the only signal left.
// The resolver's directory cache (`entries_option`) can't be the
// source of truth either: its `abs_path` is populated lazily per
// cache generation, so after any unrelated write in the directory
// busts it, already-watched files map to an empty path and the
// reload used to be dropped (leaving the file unwatched forever).
let mut last_file_hash: bun_watcher::HashType = bun_watcher::HashType::MAX;
let dir_path = strings::trim_right(file_path, &[SEP]);

for i in 0..affected_len {
let changed_name: &[u8] = if IS_KQUEUE {
affected_kqueue[i]
} else {
match affected_inotify[i] {
Some(z) => z.as_bytes(),
None => continue,
}
};
if changed_name.is_empty()
|| changed_name[0] == b'~'
|| changed_name[0] == b'.'
{
continue;
}

// `ctx` is a BACKREF that outlives the reloader.
let loader = self
.ctx
.get_loaders()
.get(PathName::find_extname(changed_name))
.copied()
.unwrap_or(bun_ast::Loader::File);
// Note: the post-assignment `_ = prev_entry_id`
// below documents the intentional dead store.
let mut prev_entry_id: usize = usize::MAX;
if loader != bun_ast::Loader::File {
// Both arms of `'brk` assign these before
// any read.
let path_string: bun_ptr::Interned;
let file_hash: bun_watcher::HashType;
let abs_path: &[u8] = 'brk: {
if let Some(file_ent) = dir_ent.entries().get(changed_name)
{
// reset the file descriptor
let ent = file_ent.entry();
{
// Every cached-`Entry` rewrite takes
// the per-entry mutex.
let _entry_guard = ent.mutex.lock_guard();
ent.set_cache_fd(Fd::INVALID);
ent.need_stat.set(true);
}
path_string = ent.abs_path;
file_hash = Watcher::get_hash(path_string.as_bytes());
for (entry_id, hash) in hashes.iter().enumerate() {
if *hash == file_hash {
if file_descriptors[entry_id].is_valid() {
if prev_entry_id != entry_id {
record_changed_path(
path_string.as_bytes(),
);
current_task.append(hashes[entry_id]);
if self.verbose {
Self::debug(format_args!(
"Removing file: {}",
bstr::BStr::new(
path_string.as_bytes()
)
));
}
ctx.remove_at_index(
bun_watcher::Kind::File,
entry_id as u16,
0,
&[],
);
}
}

prev_entry_id = entry_id;
_ = prev_entry_id;
break;
}
}
// `ctx` is a BACKREF that outlives the reloader.
let loader = self
.ctx
.get_loaders()
.get(PathName::find_extname(changed_name))
.copied()
.unwrap_or(bun_ast::Loader::File);
if loader == bun_ast::Loader::File {
continue;
}

break 'brk path_string.as_bytes();
} else {
let file_path_without_trailing_slash =
strings::trim_right(file_path, &[SEP]);
_on_file_update_path_buf
[0..file_path_without_trailing_slash.len()]
.copy_from_slice(file_path_without_trailing_slash);
_on_file_update_path_buf
[file_path_without_trailing_slash.len()] = SEP;

// The separator written at index `len` is
// immediately overwritten by the
// `changed_name` copy, and the slice takes
// one stale byte past the copy. Deliberate:
// changing it would change the resulting
// path hash.
_on_file_update_path_buf
[file_path_without_trailing_slash.len()
..file_path_without_trailing_slash.len()
+ changed_name.len()]
.copy_from_slice(changed_name);
let path_slice = &_on_file_update_path_buf[0
..file_path_without_trailing_slash.len()
+ changed_name.len()
+ 1];
file_hash = Watcher::get_hash(path_slice);
break 'brk path_slice;
}
};
// The resolver must re-stat this entry on its next
// resolution. Bookkeeping only; never gates the reload.
if let Some(dir_ent) = entries_option {
// SAFETY: dir_ent points into rfs.entries (or a tombstoned
// copy); both outlive this loop iteration.
let dir_ent = unsafe { &*dir_ent };
if let Fs::EntriesOption::Entries(dir_entries) = dir_ent
&& let Some(file_ent) = dir_entries.get(changed_name)
{
let ent = file_ent.entry();
// Every cached-`Entry` rewrite takes the per-entry mutex.
let _entry_guard = ent.mutex.lock_guard();
ent.set_cache_fd(Fd::INVALID);
ent.need_stat.set(true);
}
}

// skip consecutive duplicates
if last_file_hash == file_hash {
continue;
}
last_file_hash = file_hash;
if dir_path.len() + 1 + changed_name.len()
>= _on_file_update_path_buf.len()
{
continue;
}
_on_file_update_path_buf[0..dir_path.len()].copy_from_slice(dir_path);
_on_file_update_path_buf[dir_path.len()] = SEP;
_on_file_update_path_buf
[dir_path.len() + 1..dir_path.len() + 1 + changed_name.len()]
.copy_from_slice(changed_name);
let changed_path: &[u8] = &_on_file_update_path_buf
[0..dir_path.len() + 1 + changed_name.len()];
let file_hash = Watcher::get_hash(changed_path);

// skip consecutive duplicates
if last_file_hash == file_hash {
continue;
}
last_file_hash = file_hash;

for (entry_id, hash) in hashes.iter().enumerate() {
if *hash != file_hash {
continue;
}
if file_descriptors[entry_id].is_valid() {
let watched_path: &[u8] = &file_paths[entry_id];
record_changed_path(watched_path);
current_task.append(file_hash);
if self.verbose {
Self::debug(format_args!(
"File change: {}",
bstr::BStr::new(bun_paths::resolve_path::relative(
fs.top_level_dir,
abs_path,
))
"Removing file: {}",
bstr::BStr::new(watched_path)
));
}
// The entry's watch (and cached fd) may point at the
// replaced inode; evict it so the reload re-arms a
// watch on the current one.
ctx.remove_at_index(
bun_watcher::Kind::File,
entry_id as u16,
0,
&[],
);
}
break;
}

if self.verbose {
Self::debug(format_args!(
"File change: {}",
bstr::BStr::new(bun_paths::resolve_path::relative(
fs.top_level_dir,
changed_path
))
));
}
}

Expand Down
83 changes: 81 additions & 2 deletions test/cli/hot/watch.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import { spawn } from "bun";
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, forEachLine, isBroken, isWindows, tempDirWithFiles } from "harness";
import { describe, expect, setDefaultTimeout, test } from "bun:test";
import { bunEnv, bunExe, forEachLine, isASAN, isBroken, isWindows, tempDir, tempDirWithFiles } from "harness";
import { renameSync, writeFileSync } from "node:fs";
import { writeFile } from "node:fs/promises";
import { join } from "node:path";

// Every case spawns a watch-mode bun process and waits on several of its
// runs; give them headroom on slow ASAN/CI machines.
setDefaultTimeout(isASAN ? 120_000 : 30_000);

describe.todoIf(isBroken && isWindows)("--watch works", async () => {
for (const watchedFile of ["entry.js", "tmp.js"]) {
test(`with ${watchedFile}`, async () => {
Expand Down Expand Up @@ -48,3 +53,77 @@ describe.todoIf(isBroken && isWindows)("--watch works", async () => {
});
}
});

// On Windows `bun test --watch` runs under a watcher-manager parent process,
// which makes the stderr sync points below racy there.
describe.skipIf(isWindows)("watch mode detects atomic saves", () => {
// A test file that writes into the watched directory while it runs. The
// watcher busts the resolver's directory cache on any event in that
// directory, so the *next* test file's load re-reads the directory. That
// rebuilt cache generation is the one consulted when the rename below is
// seen, and it has never resolved the first file (editors, loggers, and
// bun's own terminal output all produce this kind of activity).
const noisyTestFile = (name: string) => `
import { test, expect } from "bun:test";
import { writeFileSync } from "node:fs";
import { join } from "node:path";

test("${name} initial", async () => {
writeFileSync(join(import.meta.dir, "noise.log"), "${name}");
// Give the watcher thread time to see the event before the next test
// file is loaded.
await Bun.sleep(500);
expect(1).toBe(1);
});
`;

test("bun test --watch re-runs a test file replaced by write-temp-then-rename", async () => {
using dir = tempDir("watch-atomic-save", {
"package.json": JSON.stringify({ name: "watch-atomic-save", type: "module" }),
"a.test.ts": noisyTestFile("a"),
"b.test.ts": noisyTestFile("b"),
});

await using proc = Bun.spawn({
cmd: [bunExe(), "test", "--watch", "--no-clear-screen"],
cwd: String(dir),
env: bunEnv,
stdout: "ignore",
stderr: "pipe",
stdin: "ignore",
});

const reader = proc.stderr.getReader();
const decoder = new TextDecoder();
let buf = "";
async function waitFor(needle: string, from = 0): Promise<void> {
while (!buf.slice(from).includes(needle)) {
const { value, done } = await reader.read();
if (done) throw new Error(`stream closed before seeing ${JSON.stringify(needle)}\n${buf}`);
buf += decoder.decode(value, { stream: true });
}
}

// Wait for the first full run so both files are loaded and watched.
await waitFor("Ran 2 tests across 2 files");

// Replace whichever file was loaded first the way editors save: write a
// temp file, then rename it over the original. The original inode (and
// its per-file watch) is gone, so only the directory event can report
// this change.
const first = buf.indexOf("a.test.ts:") < buf.indexOf("b.test.ts:") ? "a" : "b";
const target = join(String(dir), `${first}.test.ts`);
const before = buf.length;
writeFileSync(
`${target}.tmp`,
`import { test, expect } from "bun:test";\ntest("${first} atomically saved", () => { expect(1).toBe(1); });\n`,
);
renameSync(`${target}.tmp`, target);

await waitFor(`${first} atomically saved`, before);
await waitFor("Ran 2 tests across 2 files", before);
// The re-run executed cleanly: it picked up the replaced file and none of
// the re-ran tests failed.
expect(buf.slice(before)).not.toContain("(fail)");
});
});
Loading