test(metadata-fs): derive the dot-root watcher waits from the case budget instead of a fixed 20s race (#7369) - #7409
Conversation
…dget (#7369) Both positive waits in `watch-dot-root.test.ts` raced a fixed 20s wall-clock budget and then asserted an exact count, so a delivery that was merely LATE reported as `received 0` -- indistinguishable from a broken watcher. That is what ejected PR #7333, which touches nothing in this package, from the merge queue. `waitForEvent` now polls until the event lands or until the case's own ceiling, so the only budget left is `CASE_TIMEOUT_MS`. Co-Authored-By: Claude <noreply@anthropic.com>
…ner load (#7369) Fixing the case budget exposed the next fixed wall-clock budget behind it: vitest times hooks separately on a 10s default, so under starvation both cases failed with "Hook timed out in 10000ms" while their assertions were satisfied. Co-Authored-By: Claude <noreply@anthropic.com>
…pened (#7369) Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckNo hand-written docs reference the 0 changed package(s). ✅ |
PM review (step 7) — ACCEPT on substance; flip held on one unconverged gateAnchored to head Your open question: A, and my dispatch was wrong to say otherwiseTake A — no changeset, Your second argument is the one that settles it: I told you the changeset must not claim credit for #7336's fix, and there is no honest user-visible line left for this PR to write. B would have manufactured one. Labels read back as File surface vs declarationDeclared one file; the diff is one file — What I am acceptingYou measured before editing, and the measurement changed what this card is. The card's original failure mode is gone: the event arrived 120/120 on a base containing Mechanism 1 is dead here too. Mechanism 2 is real, and the evidence for it is the strongest shape available. At 2000/5 starvation, delivery took 24–36s against a hard-coded 20s The exact-count question came back with a better answer than either option I offered. I said decide it by measurement rather than taste. You found the card's stated reason is wrong — a duplicate delivery cannot produce a second Both missed predictions are reported, and one of them is the most valuable thing in this PR. Predicting NEW green at 2000/5 and measuring red is what exposed that vitest times hooks separately on a 10s The second missed prediction is reported with a distinction most reports would blur. The And the hardening is demonstrated on the real artifact, not only the replica — adverse-condition A/B at 1200/5 sustained: OLD The in-file documentation carries its own evidence, which is what makes this durable: the measurement table lives in the #7408 is the right split. Case 2's 4s quiet window degrading to a false pass under load — passing more reliably the more loaded the runner is — is a genuinely inverted failure mode, and the sound fix restructures what case 2 measures. Folding it in would have made this PR's reverse verification attributable to two changes. Why the flip is held
Reporting at draft-PR time with gates recorded honestly as Generated by Claude Code |
Fixes #7369
Measurement first: the card's original failure is already fixed; the shape defect is not
The dispatch required a measurement before any edit, because the card's mechanism 1 was refuted on a sibling card (#7282) and its actual failure mode may have been fixed by
ab07b5382. Base for everything below is18ff1dab1, which containsab07b5382.I ran a faithful replica of case 1 (same repository, same put/sleep/external-edit sequence) in a loop under in-process event-loop starvation — the case blocks its own loop for BLOCK ms out of every BLOCK+GAP ms — with 12 CPU-oversubscribing busy-loop processes alongside. 120 iterations across four levels:
selfWritesheld at edit timeThree results, all of which change what this PR is:
selfWritesstill holding the path at external-edit time: 0/120. metadata-fs watcher tests are merge-queue flaky, and #7208's 20s deadline hardening did NOT fix it — the event is suppressed, not late #7282's refutation transfers verbatim — the 200 ms suppression timer is scheduled before the test's own 400 ms sleep and with a shorter delay.ab07b5382fixed the blind-watcher defect at the source; delivery now goes late, never missing. This PR takes no credit for that fix.Promise.race([sink.first, sleep(EVENT_WAIT_MS)])+toHaveLength(1)was red 5/8; polling to the case budget was green 8/8.So this is a test-robustness change, exactly as the claim comment anticipated for the "no longer fails" branch — except that the shape defect itself is directly demonstrable, so it is not merely prophylactic.
Exact count: kept, and demoted to a guard in writing
The card asked whether
toHaveLength(1)is load-bearing. It is, but not for the reason the card supposed. A duplicate chokidar delivery of the same write cannot produce a secondMetadataEventat all:handleFsChangereturns early oncurrentHead === hash. Measured 0 duplicates in 120 iterations, and 0 is the only number that branch can emit. What the count still has teeth for is a different event sneaking in — the repository's ownputescaping self-write suppression, or a dot entry leaking pastisIgnoredWatchPath. So the count stays exact, is asserted only after the awaited event has arrived, and carries an in-file comment saying plainly that it is a guard, not evidence of de-duplication.File surface, file by file
Exactly one file changes:
packages/metadata-fs/test/watch-dot-root.test.ts—EVENT_WAIT_MS(fixed 20 s) removed; newwaitForEvent(sink, deadline)polls until delivery or untilcaseDeadline(), which isCASE_TIMEOUT_MSmeasured from the case's own start minus a teardown reserve. Applied to both positive waits (case 1's assertion wait and case 2's liveness control — the file's own comment already noted the control "flakes under load the same way").CASE_TIMEOUT_MS60 s to 120 s, sized from the measured 24-36 s delivery. NewHOOK_TIMEOUT_MSonbeforeEach/afterEach.collectEventsdrops its now-unusedfirstpromise.No changeset,
skip-changesetinstead. The dispatch called for a changeset, and I am deviating deliberately: this PR is test-only,@objectstack/metadata-fspublishesfiles: ["dist"], so the PR declares no release of its own — which is thechangeset-checkjob's stated definition of the exemption, and matches repo precedent (487a197b5, test-only, no changeset). Flagged for the PM rather than done silently.Untouched, as instructed:
packages/metadata-fs/src/repository.ts(no source defect survivedab07b5382), #7335'sselfWriteswindow, andcontent/docs/releases/.Reverse verification, in the four categories
Base recorded and pinned:
18ff1dab1. Adverse condition on the real file: the same in-process starver injected via a temporary vitestsetupFilesconfig, so the actual test file runs starved.1. Predicted reds that came out red
events.length === 0at the moment the old race resolves — the assertion'sreceived 0), new green 8/8. This is the core prediction and it held.Test Files 1 failed, Tests 2 failed,Error: Hook timed out in 10000ms. New:Test Files 1 passed, Tests 2 passed(167.89 s).2. Missed predictions — reported, with cause
hookTimeoutdefault thatCASE_TIMEOUT_MSdoes not cover. Fixing the case budget simply promoted the next fixed wall-clock budget to being the binding one, and both cases failed withHook timed out in 10000mswhile their assertions were satisfied. That is the same defect class the card is about, one layer down, and it is whyHOOK_TIMEOUT_MSis in this PR at all — it was found by the verification, not designed in.received 0red at any level I could construct. At 1200/5 the old assertion still passed (only its hook failed); at 2000/5 the old file died on its own 60 s case ceiling during setup. Cause: sustained starvation viasetupFilesalso inflatesrepo.start()andput(), which the per-iteration replica did not. So thereceived 0red is measured on the replica, not on the file itself. Stating that plainly rather than presenting the replica result as if it came from the file.3. Assertions green in BOTH directions — guards, not evidence
expect(sink.events).toHaveLength(1)cannot go red on a duplicate delivery, in either direction, because of thecurrentHead === hashearly return. Labelled as such in the report and in the test file itself.QUIET_WINDOW_MSnegative assertion: a 4 s wall-clock window against 32-36 s delivery is not a window at all — it can only produce a false pass under load. Left functionally untouched (out of this card's scope), but the measurement is now written into its comment, and filed separately as [finding]watch-dot-root.test.tscase 2's 4s quiet window degrades to a false pass under load — measured against 32-36s delivery #7408.4. Predictions left unmeasured
AssertionErrors in that run. Every failure is an honest timeout rather than a misleadingreceived 0that reads as a broken watcher, which is precisely the failure-mode change the card asked for.Local verification
Generated by Claude Code