Skip to content

fix(widgets): detect a replaced dev folder and ignore transient stat errors - #610

Merged
mrgoonie merged 3 commits into
mainfrom
fix/546-dev-folder-replaced
Oct 7, 2026
Merged

fix(widgets): detect a replaced dev folder and ignore transient stat errors#610
mrgoonie merged 3 commits into
mainfrom
fix/546-dev-folder-replaced

Conversation

@mrgoonie

@mrgoonie mrgoonie commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

What

This PR fixes all four follow-ups to the #531 review in #546: the two in the issue body and the two in the comment.

  1. A folder that is deleted and made again is detected. When the dev engine starts watching, it records the folder's dev/ino (bigint stats). A different folder at the same path now counts as gone, so the session stops as folder-gone instead of staying "live" but hearing nothing. DevEngine.rootGone() exposes this, and stoppedIfGone in apps/runtime/src/application/widget-dev-sessions.ts now asks the session's engine, so the engine and the session agree on which folder is watched.
  2. Only "not found" counts as gone. devRootState (core) returns gone for ENOENT/ENOTDIR, for a path that is not a folder, or for a changed identity. Any other stat error (EPERM, EBUSY) returns unknown. The engine's rootStillThere and the session's stoppedIfGone both treat unknown as still there and look again on the next tick.
  3. A catch-up build with no news no longer relabels the last build. When the result is unchanged or the same failure (same diagnostics), the previous last build stays, so live sessions no longer see a fake "change" build 500 ms after start. A catch-up that finds a different failure, or files back to the newest generation after a failure, is still reported.
  4. A failed first build slower than 500 ms is not reported twice. The catch-up now reads before inside its queued job, after every earlier build has settled, instead of when its timer fires. enqueue now takes a job rather than a trigger to make that possible.

Tests (real watchers)

packages/core/test/widget-dev-engine.spec.ts:

  • The folder is deleted and made again in one event-loop turn: onRootGone fires once, watching stops, and rootGone() is true.
  • EPERM, then EBUSY, on the folder's stat across several existence checks: the folder is not gone and is still watched, and the next save builds generation 2. This test uses a pass-through vi.mock("node:fs") that fails statSync for one path.
  • The catch-up finds nothing new: lastBuild() is still the start build and nothing is reported.
  • The catch-up fires while a failing first build (snapshot over its limits) is still pending, using fake setTimeout only: nothing is reported, and the last build is still the first one.

apps/runtime/test/widget-dev-sessions.spec.ts:

  • A watched session whose folder is deleted and made again stops as folder-gone.
  • A watched session whose folder stat fails with EPERM stays live through a rebuild, and the next save activates generation 2.

All six new tests fail on main and pass here. The core and runtime specs were run three times in a row on Windows with no flakes.

One finding along the way: on Windows, fs.watch also reports reads of new files, such as an antivirus scan a few seconds after they are written, as changes. That noise already existed before this PR: it triggers an extra build, not a loop. The two catch-up tests set a long debounce so that only the catch-up builds.

Verification on 4538343:

  • focused: both specs passed, 46 tests
  • pnpm typecheck: passed
  • pnpm invariants: all 14 checks passed
  • eslint on the changed files: clean
  • pnpm verify: passed (561 test files, 7501 tests passed, 37 skipped)

Docs

The folder-gone entry in docs/open-interfaces.md and docs/open-interfaces.vi.md now covers a folder that was replaced, and says that transient stat errors keep the session live. The catch-up changes are internal and need no doc. Official docs (clarkcant-web) do not describe stop reasons at this level.

Overlap

PR #609 (#583) also edits apps/runtime/test/widget-dev-sessions.spec.ts: its imports, and a new describe block appended at the end of the file. Both branches start from main, so whichever merges second needs a small rebase there. The source files do not overlap: #609 changes the install call in widget-dev-sessions.ts, and this PR changes stoppedIfGone and its imports.

Fixes #546
Refs #496, #531

@mrgoonie
fix(widgets): detect a replaced dev folder and ignore transient stat …
4538343 
…errors

A dev folder deleted and made again before the next existence check kept its
session live while the watcher still pointed at the deleted folder. The engine
now records the folder's device and file id when it starts watching and treats
a different folder at the path as gone; the session asks its engine rather than
statting the path itself.

Only ENOENT and ENOTDIR count as gone. A transient EPERM or EBUSY from an
antivirus or indexer no longer stops the session as folder-gone; the folder is
checked again on the next tick.

The macOS catch-up build no longer relabels the last build when it finds
nothing new or the same failure, and reads the previous build only after the
builds before it settle, so a slow failing first build is not reported twice.
@mrgoonie

mrgoonie commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Review attestation: ready to merge at 453834317485527af01087816062d529d6e5f17f, reviewed by agent:code-reviewer.

A push to this PR makes this attestation stale; the new head needs its own review.

@mrgoonie
mrgoonie enabled auto-merge (squash) October 7, 2026 20:28
This was referenced Oct 7, 2026
Closed
Closed
@mrgoonie
fix(widgets): hold a watched dev folder open so a folder made again t…
6c36b7f 
…here reads as a new one

On Linux a folder deleted and made again at the same path straight after is
usually given the freed file id back, so the identity check took it for the
same folder and built from it. Held open while watched, the deleted folder
keeps its id and the new one gets another. Windows already gives a new
folder a new id, and a folder held open there could not be deleted, so
nothing is held there.
@mrgoonie

mrgoonie commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Review attestation: ready to merge at 6c36b7fa809a1f74faf934a295af3e4d310cef78, reviewed by agent:code-reviewer.

A push to this PR makes this attestation stale; the new head needs its own review.

@mrgoonie

mrgoonie commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Review attestation: ready to merge at 4d36fb083a136830fc6f61b88fd1d4f56b594d15, reviewed by agent:code-reviewer.

A push to this PR makes this attestation stale; the new head needs its own review.

@mrgoonie
mrgoonie merged commit 28ded47 into main Oct 7, 2026
22 checks passed
@mrgoonie
mrgoonie deleted the fix/546-dev-folder-replaced branch October 7, 2026 21:03
This was referenced Oct 7, 2026
Merged
Merged
Closed
mrgoonie added a commit that referenced this pull request Oct 8, 2026
@mrgoonie
fix(widgets): re-arm a dev session on a changed folder id and bound p…
723a8e6 
…ersistent stat errors (#614)

* fix(widgets): re-arm a dev session on a changed folder id and bound persistent stat errors

A dev folder that is still a directory but reports another device or file id (made again by a build, or a FUSE or network drive that does not keep ids) is now watched again and built, instead of stopping the session as folder-gone. Only a path that is gone or no longer a directory stops it.

A folder that keeps failing to be looked at for a reason other than not found (EPERM, EBUSY) now stops the session as watch-failed after 30 seconds of continuous failures, and the node's log names the error and says what it ran keeps running.

Refs #546, #610

* fix(widgets): stop a dev session whose folder now leads elsewhere through a link

A folder found under a new file id is watched anew only while its path still
resolves to the canonical path watching started from and is not itself a link;
a link or junction swapped in at the folder or above it now stops the session
as folder-gone instead of building a tree nobody chose.

Re-arms are logged with the old and new ids and capped at 30 in 60 s, a
re-arm queues the catch-up build a new watcher needs, a look that re-arms no
longer builds twice, and the unreadable bound uses a monotonic clock. A folder
that is there but cannot be read is refused as ROOT_UNREADABLE and resumes as
watch-failed. The watch-failed copy says why, and the macOS tests stat the
canonical root the engine watches.

* fix(widgets): wait for a dev folder a build deletes and makes again

A watched folder that is missing now counts as gone only after 2 s; a folder
back at the same real path within that time is watched anew, so a build in
another process that deletes and rewrites its output folder keeps the session
live. Nothing is built while the folder is missing.

Only back-to-back id changes count toward the re-arm cap, the real path is
checked just before and after the files are copied, real paths are compared
without case on Windows and macOS, and a folder whose real path cannot be read
is refused as ROOT_UNREADABLE.

* fix(widgets): build a dev folder given through a link, and wait out a recreate during a build

The dev engine now resolves its root to the real path once, when it starts, so a
package folder that is itself a symlink or junction builds instead of failing
every build with FILES_LINK_REFUSED. rootGone() honours the missing-folder grace
while watching, so a build or rebuild that overlaps a folder being made again
fails on the missing files without stopping the session, and the folder is
built once it is back. A real path that differs only in case counts as the same
place only when no folder on the path is a link.
@mrgoonie mrgoonie mentioned this pull request Oct 8, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fix(widgets): detect a replaced dev folder and ignore transient stat errors

1 participant

@mrgoonie