Repository navigation
fix(widget-dev): install a granted build first, and wait on the engine rather than the clock in its tests - #669
Merged
Merged
Conversation
…d removed folder tests A folder remade by copying straight into place can be read half-written once a loaded runner stretches the copy past the debounce, and the engine rightly builds what it found. The child now copies into a sibling and renames it into place. The grace test drained by sleeping past the catch-up, so a change build a late watcher event ran (FSEvents on macOS) was counted as a report about the removal; it now drains the build queue with a rebuild before removing the folder. Refs #656, #658
The session's answer poll saw an inbox grant before the inbox finished installing it, installed the newer build on that consent and pruned the granted build's snapshot while the inbox was still copying it, so the grant failed with PACKAGE_CACHE_UNAVAILABLE. The session now waits, still asking, until the install the grant runs has recorded its outcome, bounded so an unrecorded ending never holds it for good. Refs #656
This was referenced Oct 8, 2026
# Conflicts: # docs/manifest.json
Contributor
Author
|
Review attestation: ready to merge at A push to this PR makes this attestation stale; the new head needs its own review. |
mrgoonie
enabled auto-merge (squash)
October 8, 2026 06:49
Contributor
Author
|
The review found nothing wrong with the code; the only blocker was a conflict in docs/manifest.json. I merged main (which now includes #665 and #668) and regenerated the manifest. On that tree, invariants pass 15/15, typecheck passes, and both focused specs pass 108/108. I attested head 9bde5b6 and enabled auto-merge. The non-blocking follow-ups (logging when the 60 s wait runs out, and the overlong doc lines) are folded into #670. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #656
Fixes #658
Summary
Three widget dev tests failed under load or on slow runners. Two of them had a test-side cause, and one exposed a real product race.
Engine: a remade folder was read half-written (#656)
The tests "builds a folder another process makes again exactly once…" and "builds a folder another process made again once…" had a child process copy the folder straight back into place with
cpSync. Under load, that copy can take longer than the 30 ms debounce. The engine then sees the folder back and builds what it finds, which is half the files. That build is correct engine behaviour, so the fault was in the test.I reproduced both CI signatures deterministically. A child that creates the folder and only some of its files, and finishes 300 ms later, gives four
failedrebuilds,["change:failed","change:generation"], andclarkcant.json does not exist.The child now copies into a sibling folder and renames it into place, retrying briefly on Windows
EPERM/EBUSY. The folder is therefore whole the moment it reappears. Both tests also assert that the first build was a generation.Engine: the grace test counted a build from before the removal (#658)
The test "does not call a watched folder gone while it may still come back…" slept for catch-up + 100 ms before removing the folder, but never cleared
built. On macOS, FSEvents can report the writes that made the folder after the watcher has started. The change build that runs because of that event is reported asunchanged, and a slow runner pushes it past the sleep.I reproduced the exact CI failure (
expected [ 'unchanged' ] to deeply equal []) by saving the same content once just before the removal.The test no longer sleeps. Builds run one at a time, so
await engine.rebuild()first waits for every build already started to end. The test then clearsbuiltand removes the folder in the same synchronous step, and a change reported later finds the folder missing. The assertionexpect(built).toEqual([])is unchanged.Runtime: an inbox grant could fail while a newer build was waiting (#656)
"keeps the newest build's snapshot…" sometimes got 503
PACKAGE_CACHE_UNAVAILABLEfrom the decision. The cause is a race in the product:In the reverse order, the older generation could instead be activated over the newer one.
follownow waits, still asking, while the install that the grant started has not recorded its outcome on thepackage.install-approvalstream. The wait is capped at 60 s, so a missing outcome record cannot hold the session forever.A new test holds the inbox install's first read of the granted snapshot and posts a rebuild meanwhile. It then checks that the grant succeeds, that generation 2 becomes active afterwards, and that the granted snapshot survives. On the base code it fails with
expected 400 to be 200. The EN and VI docs indocs/widget-development*.mdnow say the granted build installs first.Verification
Stress used 8 parallel vitest processes × 20 rounds on Windows (
node vitest.mjs run <spec>):packages/core/test/widget-dev-engine.spec.tsapps/runtime/test/widget-dev-sessions.spec.tspnpm verifypasses locally.