test(metadata-fs): bracket case 2's negative assertion instead of waiting 4s (#7408) - #7472
Draft
os-zhuang wants to merge 1 commit into
Draft
test(metadata-fs): bracket case 2's negative assertion instead of waiting 4s (#7408)#7472os-zhuang wants to merge 1 commit into
os-zhuang wants to merge 1 commit into
Conversation
…ting 4s (#7408) `watch-dot-root.test.ts` case 2 proves a negative — the watcher must NOT report dot entries under the root. It proved it by sleeping `QUIET_WINDOW_MS = 4_000` and asserting `sink.events` was empty. Measured for #7369: under event-loop starvation delivery of a single external edit takes 24-36s. Past that load level the array is empty because nothing has arrived yet, not because the noise was ignored — so the assertion passes *more* reliably the more loaded the runner is. Re-measured here against the real defect (`isIgnoredWatchPath` reduced to `return false` at the source): the old assertion passed with the defect live in 9 of 13 loaded runs, and 5 of 5 when `.cache/x.json` was the only noise. A bigger number is not the fix — any "sleep, then assert empty" shape has the same defect at some load level. You cannot wait for an absence, so the case now brackets the noise between deliveries it can observe: - an OPENER (an external edit that must arrive) proves the watcher is awake BEFORE the noise exists, which nothing checked before — a late-armed watcher swallows the noise into its own baseline and produces exactly the empty array a correct one does; - two CLOSERS, each a new entry placed so that the same directory read which discovers it must also enumerate one of the leaks — a new file in `view/` for `view/.scratch.json`, a new type directory for `.cache/x.json`; - the bracket closes on the closers' IDENTITY, never on an event count: with a count a leaked event satisfies the count itself, so the run that should fail loudest is the one that stops listening soonest. Against the same defect and the same load the new shape goes red 9/9 at BLOCK=4000/GAP=5 and 8/8 at 2000/5, and stays green 5/5 with the source intact. `.cache/x.json`'s bracket is tight rather than exact (two async directory reads from one root read can finish in either order): caught 28/29, the one escape a late delivery rather than a lost one. Recorded in the file, with the reason a sleep must not be used to close it. `CASE_TIMEOUT_MS` 120s -> 180s: the case now makes three sequential deliveries where the ceiling was sized for one. Measured at 4000/5, case 2 takes 40-44s (72s when the broken matcher adds two events); 120s would have held, at 2.6x margin instead of the ~3.3x #7369 chose. A healthy run pays none of it — case 2 is 2.1s unloaded, down from 5.5s. Test-only; no runtime behaviour changes. Fixes #7408 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XMgfc1Q56eBEH9s6yCHf6t
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckNo hand-written docs reference the 0 changed package(s). ✅ |
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 #7408
Real file surface
packages/metadata-fs/test/watch-dot-root.test.tswaitForEvent→waitForEvents+ newwaitForNames;QUIET_WINDOW_MSdeleted;CASE_TIMEOUT_MS120s → 180sThat is the whole diff — one file, test-only (+176 / −45). No source file is touched;
packages/metadata-fs/src/repository.tsis byte-identical tomain.The defect
Case 2 proves a negative: the watcher must not report dot entries under the root. It proved it by sleeping
QUIET_WINDOW_MS = 4_000and assertingsink.eventswas empty.#7369 measured delivery of a single external edit at 24–36s under event-loop starvation. Past that load level the array is empty because nothing has arrived yet — not because the noise was ignored. The assertion passes more reliably the more loaded the runner is, which is the one correlation a guard must never have.
Re-measured here against the real defect (
isIgnoredWatchPathreduced toreturn falseat the source, reverted before commit): the old assertion passed with the defect live in 9 of 13 loaded runs, and 5 of 5 when.cache/x.jsonwas the only noise.⛔ A bigger number is not the fix — every "sleep, then assert empty" shape has the same defect at some load level, and #7369 already measured what moving one fixed budget buys (the 10s
hookTimeoutimmediately became the next binding one).The shape
You cannot wait for an absence, so the case brackets the noise between deliveries it can actually observe:
view/forview/.scratch.json, a new type directory for.cache/x.json.Reverse verification
Break:
isIgnoredWatchPath→return false(the "letparseItemPathdecide" variant the source docblock describes). Load: in-process event-loop starvation,BLOCKms blocked out of everyBLOCK+GAPms. Base SHA08363a0.Missed prediction (P6), with cause. I predicted the old case would go green under load. It did not, 13/13. What false-passed was the assertion this card is about —
expect(sink.events).toEqual([])passed with the defect live in 9 of 13 loaded runs (0/1 unloaded), 5/5 on the isolated.cacheleak. The case was rescued each time by its trailingtoHaveLength(1), an assertion documented as a liveness control, which caught the leak only because the leak (written ~24–28s earlier under starvation) happened to arrive before the control event. That rescue is an ordering accident, not a design — and it is #7369's deadline-driven half doing the work, not the quiet window.Guards (green in BOTH directions), labelled as such in the test file:
.objectstack/.log/main.jsonlappend cannot become aMetadataEventeven with the matcher removed entirely —parseItemPathrejects the.objectstackprefix. Measured: with the matcher broken, the leak set is exactly.cache/x+view/.scratch, never the log. It stays as the traffic the matcher exists to keep out of the poll set.Residual, reported rather than hidden.
view/.scratch.json's bracket is exact (oneview/read enumerates both) — caught 11/11..cache/x.json's is tight, not exact: the root read that discoversdashboard/enumerates.cache/in the same call, but each is then descended into by its own async read and those can finish in either order. Caught 28/29 (9/9 at 2000/5, 19/20 at 4000/5). The single escape was a late delivery, not a lost one — draining 30s past the bracket found the event 6/6. The file records this and states why a sleep must not be used to close it; the direction that would is another event-driven round, traded against one more delivery inside the ceiling.Unmeasured: whether the opener catches a genuinely late-arming watcher. That needs a different break (#7000/#7282 arming defects) than the matcher break used here; the opener's teeth against it are argued from
ignoreInitial, not measured.CASE_TIMEOUT_MS120s → 180sNot a quiet window — this is the deadline #7369 made the single source of every wait's budget, and it was sized for one delivery. Case 2 now makes three sequential ones. Measured at 4000/5 (heavier than any row #7369 ran): case 1 28–32s, case 2 40–44s, up to 72s when the broken matcher adds two events. 120s would have held at 2.6x margin; 180s restores the ~3.3x #7369 chose. A healthy run pays none of it — case 2 is 2.1s unloaded, down from 5.5s.
Changeset
skip-changesetlabel, not an empty changeset. This is test-only and releases nothing, so it is the textbook case for the label; an empty changeset would linger in the release pipeline. Label applied to this PR.Gates
pnpm check:nul-bytespackages/metadata-fssuitepnpm typecheck(metadata-fs)Per instruction, the whole lint farm was not run locally.
Generated by Claude Code