Four findings on watchTree / watchStudio in tools/sync-studio.mjs, raised in review of #209 and judged non-blocking there. All of them are confined to --watch, which is an opt-in developer convenience; the one-shot sync, the tests and CI are unaffected. Findings 1 and 2 only bite on the non-recursive fallback path, which runs on Linux with Node < 20 and therefore on no machine CI uses.
1. Stale watcher entries are never pruned. watched is a Map keyed by directory path and mount() skips any path already in it. Delete a watched directory and recreate it, and the entry survives while the underlying watcher is dead — events from the recreated directory are lost for the rest of the session, and the dead watcher stays open until close(). Reproduced in review with a real fs.watch on the fallback path: the fire count stayed flat across the recreate.
2. mount() can throw out of an event listener. It is called unguarded inside every per-directory callback. Its catch closes every watcher and rethrows — and a throw from inside a listener is an uncaught exception, so the watch process dies, having silently torn down the whole watch first. The realistic triggers are the ENOSPC the code comment itself cites, and an ENOENT race in listDirectories against a directory deleted between the walk and the watch. The current test only exercises the throw at construction time, where the caller can still catch it.
3. Error visibility was narrowed, and in the wrong commit. The debounce handler used to rethrow anything that was not a SyncError; it now swallows everything and reports through log. Under --quiet, log is null, so an EACCES or ENOSPC on copy turns the watcher into a permanent silent no-op with no signal at all. Errors should go to stderr regardless of --quiet, which suppresses the per-run report, not failures. This was a production behaviour change that landed inside a commit typed test(tools), which is the part worth not repeating.
4. A doc overstatement. The header of tools/sync-studio.mjs says TextureStudio's discovery does not consume rasters. discoverActivePack() actually matches /\.(svg|png)$/i. The conclusion the header draws is still correct — the Studio's own compiler filters .svg, the server types /textures/* as image/svg+xml, and the viewport rasterizes in the browser — but the claim is absolute where the code is not. Reword to say the Studio's vector pipeline consumes SVG, rather than that discovery cannot see a PNG.
Suggested order: 3 first (smallest, and it is the one that can hide a real failure), then 2, then 1, then 4.
Four findings on
watchTree/watchStudiointools/sync-studio.mjs, raised in review of #209 and judged non-blocking there. All of them are confined to--watch, which is an opt-in developer convenience; the one-shot sync, the tests and CI are unaffected. Findings 1 and 2 only bite on the non-recursive fallback path, which runs on Linux with Node < 20 and therefore on no machine CI uses.1. Stale watcher entries are never pruned.
watchedis aMapkeyed by directory path andmount()skips any path already in it. Delete a watched directory and recreate it, and the entry survives while the underlying watcher is dead — events from the recreated directory are lost for the rest of the session, and the dead watcher stays open untilclose(). Reproduced in review with a realfs.watchon the fallback path: the fire count stayed flat across the recreate.2.
mount()can throw out of an event listener. It is called unguarded inside every per-directory callback. Itscatchcloses every watcher and rethrows — and a throw from inside a listener is an uncaught exception, so the watch process dies, having silently torn down the whole watch first. The realistic triggers are the ENOSPC the code comment itself cites, and an ENOENT race inlistDirectoriesagainst a directory deleted between the walk and the watch. The current test only exercises the throw at construction time, where the caller can still catch it.3. Error visibility was narrowed, and in the wrong commit. The debounce handler used to rethrow anything that was not a
SyncError; it now swallows everything and reports throughlog. Under--quiet,logisnull, so an EACCES or ENOSPC on copy turns the watcher into a permanent silent no-op with no signal at all. Errors should go tostderrregardless of--quiet, which suppresses the per-run report, not failures. This was a production behaviour change that landed inside a commit typedtest(tools), which is the part worth not repeating.4. A doc overstatement. The header of
tools/sync-studio.mjssays TextureStudio's discovery does not consume rasters.discoverActivePack()actually matches/\.(svg|png)$/i. The conclusion the header draws is still correct — the Studio's own compiler filters.svg, the server types/textures/*asimage/svg+xml, and the viewport rasterizes in the browser — but the claim is absolute where the code is not. Reword to say the Studio's vector pipeline consumes SVG, rather than that discovery cannot see a PNG.Suggested order: 3 first (smallest, and it is the one that can hide a real failure), then 2, then 1, then 4.