Deflake the unit-test cluster: seed HNSW routing, signal-based waits, quarantine two unreproduced hangs

[kriszyp]

Deflakes the cluster of unit-test failures that took the Unit Test workflow red on 7 of its last 30 main runs (four distinct tests across 2026-08-21 alone), plus the Windows-only risk-query integration failure. Three tests are fixed with demonstrated root causes; two could not be root-caused within this round and are quarantined, each behind a filed, prioritized issue under epic [Epic] CI test flakiness (#1655). No product behavior changes beyond a test seam (details below).

What changed

Fixed (root cause demonstrated):

1. HNSW greedy routing (unitTests/resources/vectorIndex.test.js, resources/indexes/HierarchicalNavigableSmallWorld.ts) — graph levels were drawn from unseeded Math.random(), and greedy-vs-full-ef result equality is only statistically true across random graphs: ~2-3% of random 600-node graphs legitimately route to a different layer-0 entry point and displace the top-10 tail. Added a random property to HierarchicalNavigableSmallWorld (test seam, defaults to Math.random) and the test now pins the graph with a seeded PRNG. One pinned graph samples that statistical property exactly once, so the test sweeps eight fixed seeds, each asserted for exact equality — measured over 40 arbitrary seeds, 4 produce a graph that legitimately diverges, and all eight in SEEDS are non-divergent at this head. Product-side this also clamps the entry-point-creation level to MAX_LEVEL, matching the other assignment site — previously -Math.log(random()) at that one site was unclamped, and a random() returning exactly 0 (possible for both Math.random and any test PRNG) would produce level = Infinity and an infinite loop in the node-initialization for loop; a high finite draw persists an entry point dozens of empty layers above the graph for the life of the index. And it is not just a lottery ticket: with a schema-configured mL (e.g. mL: 5, which the constructor accepts), 13.5% of first-node draws exceed MAX_LEVEL — every such index got a permanently over-tall entry point while later nodes were clamped. The clamp now has its own regression test (HNSW entry-point level clamp), driven with a finite pathological draw so a clamp regression fails fast instead of wedging the runner. The statistical nature of greedy-equals-full, and the seed-sweep contract, are recorded in DESIGN.md.
2. Txn expiration (unitTests/resources/txn-tracking.test.js) — the test raced a fixed 50ms window (Promise.race([delay(50), result])) against a 40ms sleep + two real DB operations + an expiry that structurally needs two ~20ms monitor ticks. Ten milliseconds of slack loses on a contended runner. Now waits on the actual signals (waitFor), and proves the transaction left trackedTxns while the slow get() was still pending — so removal-by-expiry can't be confused with removal-by-completion. The suite's teardown also restores both engines' expiration globals: it set 20ms through whichever engine is active but restored only the rocksdb one, so on the lmdb pass a 20ms open-transaction limit leaked into every later test in the unitTests/resources process.
3. Subscription replay (unitTests/resources/subscriptionReplay.test.js) — !omitCurrent: updates to passed keys arrive via queue still used collect()'s quiet-period timer, the exact race its two sibling tests were already migrated off (their in-file comments document it). Under contention the subscription can go quiet longer than the window while queued updates are in flight, so the final-value assertion reads stale values. Nine tests in this file are now converted to condition waits: the five caught in the first review round, the 600-id notify-batch burst test (still on a 300ms quiet window and still flaking — 1 failure in 32 lmdb runs, missing 7 of 600), and the three remaining tests that write after subscribe and then assert completeness. collect()'s quiet window remains only where a quiet window is the semantically right tool — asserting that no events arrive, and history-replay tests whose writes all precede subscribe().
4. Vacuous duplicate checks (unitTests/resources/subscriptionReplay.test.js) — the two in-flight duplicate-detection tests swallowed their waitFor timeout, leaving no duplicate (id,version) as the only assertion, which is trivially true over a partial event set. Run alone, the count test timed out on every run and went green in 5.3s. Waiting for all 30 in-flight ids is not the fix: an in-flight write that commits before the cursor's snapshot and falls outside previousCount is legitimately never delivered (measured: 17000-17002 never arrive in an isolated run). Both tests now write a record after the in-flight writes settle and wait for it with a terminal timeout. The ordering that makes this sound is code-traced, not assumed: the previousCount cursor collects its history from a reverse audit getRange under the default snapshot, bounded by count = 10 well under REPLAY_YIELD_INTERVAL = 100, so it completes in the subscribe IIFE's first synchronous segment — before subscribe() resolves and long before the sentinel commits; the non-collection shape captures entry in that same segment and never re-reads it. Neither cursor can deliver the sentinel, so it can only arrive through pendingRealTimeQueue, which drains in commit order after every in-flight delivery.

Quarantined (issue filed, skip links to it):

5. risk-query on Windows (integrationTests/components/risk-query.test.ts) — #2273: deploy_component (restart:true) hung server-side after npm pack on Windows CI — the instance log goes silent and the client fetch dies on undici's 300s headers timeout, cancelling all 9 children at 319s. This is a hang, not runner slowness (npm pack itself finished in <1s; then 5+ minutes of zero log output), so a timeout bump would not help — the suite is skipped on win32 until the deploy hang is fixed. The readiness poll also now carries AbortSignal.timeout(5s) per probe so one hung fetch can't consume the whole budget. Suite verified green on Linux.
6. MQTT non-clean session (unitTests/apiTests/mqtt-test.mjs) — #2274: silent 20s mocha timeout on CI (lmdb pass, Node 26), zero server-side log output, and the hang is provably in one of the steps with no individual timeout (the test's own bounded 15s wait never fired). Not reproduced in 30 contended local full-file runs. Skipped on the lmdb pass only — where the hang was observed — so the rocksdb pass keeps this durable-session coverage. Hypotheses and a reinstatement path are in the issue. Most of this file's diff is indentation from wrapping the test in the conditional skip; the body is unchanged.

Found but not touched (out of scope): the repro loops surfaced a sixth flaky test, MQTT subscribe to retained record with patch operations — ~10% failure rate under 2-core contention, duplicate retained/live delivery asserted as an uncaught exception. Filed as #2275 with mechanism and repro instructions.

For the human reviewer

• The red check is not this branch. Integration Tests 2/6 (Windows, Node.js v24) fails at integrationTests/apiTests/describe-metadata-upgrade.test.ts:62 with Probe /SeoPageCache/ did not become ready within 120000ms (ECONNREFUSED) after restart_service, and phase 2 then cascades. This branch touches no integration file in that shard, no test util and no restart path. main run 32812674585 (05:23 UTC, 2026-08-25) fails identically; main run 32820775289 (07:16) passes. #2309 is the fix and names the mechanism: libuv's fs-event callback asserts that the GetLongPathNameW expansion of an event path still starts with the directory stored when the watch was armed, and the Windows runner's os.tmpdir() (C:\Users\RUNNER~1\AppData\Local\Temp) never satisfies it, so libuv aborts the process with no JS-observable seam. The instance logs from both failed attempts here match that exactly and rule out the "120s probe budget is too tight after a slow npm install" reading recorded on #2273: the restarted http worker resolves its middleware chains (attempt 1 at 06:49:17.3, attempt 2 at 07:19:42.6) and then logs nothing at all for the full 120s while every connection is refused — a dead process, not a slow one — and phase 2 boots a fresh process on the same runner, against the same component directory and its 541-package node_modules, fully up with both tables initialized 2.5s later. Attempt 1 also died after a 3-minute npm install, not a 10-minute one. No test change was made here for it, deliberately: the fix belongs in Canonicalize watch paths so a Windows 8.3 short path cannot abort the process #2309, and quarantining the suite would cost Windows coverage of the describe_all shows schema_defined: false and omits expiration for eviction-configured tables on upgraded clusters #1245 metadata-upgrade regression for a defect that is already understood.

• The one product-source file touched is HierarchicalNavigableSmallWorld.ts, and the judgment call is the random instance property as a test seam plus the MAX_LEVEL clamp on the entry-point site. The seam is inert in production (this.random defaults to Math.random; both call sites previously called Math.random() directly). The clamp changes behavior only for random() === 0 or graphs demanding level > 10, i.e. probability ~2^-32 per insert — but the unclamped site could infinite-loop, so it is a genuine (if theoretical) hardening, called out here rather than buried.

• Two review suggestions were implemented differently than proposed, both because the proposed form would have failed or flaked — worth a look because the alternatives are still live:

  • Duplicate-detection tests: the suggested fix was to make the "all 30 in-flight ids arrived" wait terminal (assert.fail on timeout). That completeness condition is unreachable by design — previousCount: 10 means an in-flight write committed before the cursor snapshot is legitimately never delivered — so it would have failed on every isolated run. The sentinel wait is terminal instead, on a condition that must hold.
  • Routing seed: the suggested fix was to sweep ~20 seeds and assert at most one diverges. At the measured divergence rate (4 of 40 arbitrary seeds) that bound fails ~20% of the time — a worse flake than the one being removed. Eight non-divergent seeds asserted exactly keeps determinism and still samples the property eight times instead of once. The cost is that an intentional index change can force a seed re-pin; DESIGN.md says so.

• The txn test's new "expired-before-settled" outcome check is deliberately stricter than the old assertion: it fails with a distinct message if the slow get() completes before expiry is observed, which would indicate the timing assumption broke in a new way rather than passing vacuously.

• The subscription-replay conversions keep .catch(() => {}) on waits whose own completeness assert follows immediately (the per-key/per-id asserts report exactly which key went stale). The two sentinel waits do not swallow: there the wait is the only thing standing between a slow runner and a vacuous pass.

• Windows quarantine means Windows loses risk-query coverage entirely until Windows CI: deploy_component (restart:true) hangs after npm pack — risk-query integration suite cancelled at 319s #2273 is fixed; that felt better than a suite that red-flags the whole Integration workflow on a 300s hang, but it is a coverage regression on that platform. Same shape for the lmdb MQTT durable-session skip: engine-specific offline-queue persistence is exactly the code path most likely to differ per engine, and it is unguarded on lmdb until Flaky unit test: MQTT 'subscribe with QoS=1 and reconnect with non-clean session' silent 20s timeout (lmdb pass) #2274 is closed.

• Round-7 review threads (Barber AI), both declined — the evidence is here because the alternatives are still live:

  • Quarantine FIRST-subscription-on-fresh-DB while writes are in flight on lmdb. Declined. That test is byte-identical at the merge base and does not reproduce on this box: 0 failures in 46 lmdb runs at this head — 12 sequential full-file, 24 six-way-contended full-file, 10 grep-isolated under 16 busy cores — and it prints ✔ in every recent main unit-test run on CI, including the seven that failed for other reasons. Skipping it would drop the only coverage of a claimed lost delivery on the one backend where it is claimed to happen, for a failure CI has never observed. What was applied is the thread's "at minimum": an explicit 5000 ms timeout plus .catch(() => {}), so a genuine loss now fails as missing in-flight id N rather than a bare Timed out after 2000ms (verified by widening the wait to an id that can never arrive). If it reproduces for you, the quarantine is one line — and the trace worth starting from is that the startTime collection branch nulls pendingRealTimeQueue and sets dropDuringReplay = true (resources/Table.ts:4151), so that branch depends entirely on the forward snapshot: false audit cursor never being overtaken by a commit that lands during replay.
  • Assert a divergence bound over a larger fixed seed list instead of curating eight non-divergent ones. Declined as yours to call, not a bot-driven redesign. One correction to the bullet above, in fairness to the suggestion: over a fixed seed list the divergence count is deterministic, so a bound would not flake the way the arbitrary-seed version would — the reviewer is right that it measures the property while exact equality only samples it. The reasons to keep the current form are cost (40 seeds is ~5× the 600-node graph builds, roughly +12s on a rocksdb-only suite) and that the baseline count then has to hold across machines and index versions. Cheap to switch if you want the measured form.

• Round-8 (this push): the lost delivery is root-caused and filed as #2311 — P1 Bug under [Epic] Audit-log subsystem hardening (#1651). The review thread's remaining ask was a ticket for the lost delivery itself rather than a quarantine, and the mechanism turned out to be demonstrable, not just a hypothesis. Table.ts:4149-4188's startTime collection branch is the one replay branch that discards live events (pendingRealTimeQueue = null) instead of buffering them, on the assumption that its snapshot: false cursor picks up the tail — but getRange() returns a terminating iterator, so the window between cursor exhausted and dropDuringReplay = false is covered by neither path. Forced by inserting a 100 ms delay between the replay loop and its finally (in dist only, nothing else changed): 200 of 200 in-flight events hit the dropDuringReplay early return, the cursor recovered none, and the test failed with missing in-flight id 20000 — the reviewer's exact signature. The mid-replay portion of the window is genuinely covered and a fix should not over-correct there: with the artificial delay at the await rest() yield instead, 200 events were dropped by the listener and all 200 were then delivered by the cursor. A second, narrower hole is code-traced but unreproduced: replay advances subscription.startTime to the last record it saw (Table.ts:4184), and transactionBroadcast.ts:202 skips any live event with subscription.startTime >= timestamp. The test is still not quarantined — same reasoning as round 7, now with a linked issue so a missing in-flight id N failure reads as Subscription replay: startTime collection branch silently drops events committed after the audit cursor terminates #2311 rather than as runner slowness.

  • One correction to round 7's non-repro, and to the reviewer's explanation of it. The suggestion was that the grep-isolated leg had silently run on the default engine. It had not: probing the store path shows test2.mdb (lmdb) on exactly the command in question, and lmdb is the faster pass here (80 ms vs 143 ms on rocksdb), not the slower one. This head is now 41/41 green on lmdb on this box (15 sequential grep-isolated, 18 six-way contended, 8 pinned to a single core). The real difference is which side of the window the writes land on: instrumented here, the replay cursor sees an empty audit range and there are zero listener drops, so the uncovered window is never entered at all. On a box where the in-flight writes straddle the end of replay it is deterministic. Both results are correct and Subscription replay: startTime collection branch silently drops events committed after the audit cursor terminates #2311 explains the gap between them.

• Review decision ledger, left as author calls for you to override: pin-the-graph vs a tolerance assertion on random graphs (pinned, now eight of them); the random seam as a public field vs threading through schema options (field — options are persisted schema config, a function doesn't round-trip); MAX_LEVEL clamp bundled here vs its own PR (bundled — one line, named in the commit body, now with its own regression test); quarantine now vs holding for root cause (quarantine — greening main is the task); converting the three unflagged same-class collect() tests vs leaving them (converted — the PR's claim about the idiom is only true if the class is gone).

Verification

• HNSW: pre-fix 1/40 sequential failures; post-fix 0/120 (4-way parallel) on the single-seed form, and the eight-seed sweep passes at 3.9s for the suite (up from 0.9s; ~0.47s per 600-node graph). Intermediate result worth knowing: seeding alone appeared to still fail 5/100 — because unit tests load dist/, and the seam wasn't built yet; after npm run build, graph state (level histograms) is byte-identical across runs.

• Seed divergence measured directly: a 40-seed sweep at this head diverges on 4 ([1, 4, 6, 33] of an imul(s+1, 0x9e3779b9) series), i.e. 4 of 160 target-graph pairs — matching the ~2-3% the DESIGN.md note records.

• Txn expiration: pre-fix 2/80 failures with 8 workers pinned to 2 cores (same assertion family as CI); post-fix 0/160 under the identical harness.

• Subscription replay: 0 failures in 24 contended full-file lmdb runs (4-way parallel pinned to 2 cores) after the conversions, plus 0 in isolated single-test runs. The count in-flight test went from a 5.3s vacuous pass in isolation to a 0.25s real one. Sentinel ordering was instrumented over those runs: the sentinel was the last event delivered every time, with zero in-flight events after it.

• Gates: npm run test:unit:resources on both engines — rocksdb 1672 passing / 3 failing, lmdb 1423 passing / 2 failing, where every failure is a pre-existing randomAccessFields / replayStructures failure that reproduces standalone in a fresh mocha process on files this branch does not touch. lint:required and prettier clean. npm run test:unit:main could not be run on this box: a stale local Harper install (~/.harperdb/hdb_boot_properties.file pointing at another checkout) makes security/auth.ts fail at module load, before any test runs. That gate --excludes unitTests/resources/**, which is the only area this round touches; forced onto a synthesized clean root it reports 4840 passing with 14 failures, all in cliOperations / Login / configValidator, i.e. artifacts of the synthesized root.

• Round-7 follow-up, at 510058687: the fresh-DB in-flight test was run 46 times on lmdb at this head with zero failures (12 sequential full-file, 24 six-way-contended full-file, 10 grep-isolated under 16 busy cores). The new failure message was proved rather than assumed — widening the wait to an id that can never arrive produces AssertionError: missing in-flight id 20200 after 5s instead of the old bare timeout. test:unit:resources re-run on both engines at this head (rocksdb 1672 passing / 3 failing, lmdb 1423 passing / 2 failing — the same pre-existing randomAccessFields / replayStructures failures on files this branch does not touch), plus lint:required and prettier clean.

• Round-8 follow-up, at 47f9b8993: the change itself is a five-line comment, so the gates re-run rather than expanded — test:unit:resources on both engines at this head (rocksdb 1672 passing / 3 failing, lmdb 1423 passing / 2 failing — the same pre-existing randomAccessFields / replayStructures failures on files this branch does not touch), lint:required and prettier clean. The Subscription replay: startTime collection branch silently drops events committed after the audit cursor terminates #2311 mechanism was established by direct instrumentation of dist/resources/Table.js and dist/resources/transactionBroadcast.js (drop counter, replay start/end markers, and the startTime >= timestamp skip counter), each reverted and dist rebuilt afterwards; the counts quoted above are from those runs.

• test:integration:all not run in full: this branch touches a single integration file (integrationTests/components/risk-query.test.ts, run individually, 9/9 on Linux) and the Integration workflow on main is currently failing for unrelated reasons (17 of its last 30 main runs), so a full local run has no usable baseline.

• Round-9 (rebase onto main@df5355aaa, at 56e544505): no conflicts — the branch's 7 changed files did not overlap anything new on main, so the rebase replayed cleanly. npm run build clean; npx mocha unitTests/resources/vectorIndex.test.js unitTests/resources/subscriptionReplay.test.js unitTests/resources/txn-tracking.test.js — 153 passing / 11 pending (pending are the pre-existing Read Txn Expiration and count-branch skips, unrelated to this rebase). unitTests/apiTests/** could not be run in this worktree (no installed Harper system database at HDB_ROOT, an environment gap unrelated to the diff). A full independent pre-push re-review ran (force-push breaks the ancestor chain the delta mode needs) and raised nothing not already covered above — the two Harper-domain findings it surfaced (a timing-dependent branch in the txn-tracking assert.rejects, and the lmdb MQTT quarantine's coverage cost) restate the "txn test's stricter outcome check" and "MQTT durable-session skip" points already called out in this section, so no code changed.

Refs [Epic] CI test flakiness (#1655)

Complexity: low — test-only changes plus one inert test seam and a theoretical-edge clamp in HNSW level assignment.

— Claude Opus 5

🤖 Generated with Claude Code

_{Review-Coverage: authored=claude; ran=gemini,codex; adjudicated=domain; declined=cursor-grok,cursor-composer; rounds=10 @ d894e40}

_{Human-Review-Need: 3 (decisions: test-seam-on-production-class, pinned-seed-sweep-vs-recall-tolerance, known-defect-test-left-red, windows-skip-whole-suite, lmdb-mqtt-quarantine-before-root-cause) @ d894e40}

[gemini-code-assist]

Code Review

This pull request addresses test flakiness and environment-specific hangs by introducing condition-based polling (using a waitFor helper), adding abort timeouts to fetch calls, and quarantining problematic tests on Windows and LMDB. It also introduces a random function seam in the HNSW index to allow seeding the PRNG for deterministic graph generation in tests, and clamps the entry-point level to MAX_LEVEL. The reviewer feedback recommends replacing the remaining fixed delays in subscription replay tests with a sentinel record pattern to ensure all prior events are fully processed before assertions, which aligns with best practices for testing ordered streams.

[claude]

Reviewed; no blockers found.

[kriszyp]

All four review findings are addressed in 01b6f79 + c44777a. Two were implemented differently than suggested — both because the suggested form would have failed or flaked — so the reasoning is here rather than only in the diff.

Swallowed waitFor in the two duplicate-detection tests — real, and worse than described: run alone, the count test timed out on every run and went green in 5.3s. But "all 30 in-flight ids arrived" is unreachable by design, so making that wait terminal would fail the test on every isolated run rather than fix the vacuity: with previousCount: 10, an in-flight write that commits before the cursor's snapshot and falls outside the last 10 is legitimately never delivered (measured: 17000-17002 never arrive in isolation).

Both tests now write a record after the in-flight writes settle and wait for that, with a terminal timeout. The ordering is code-traced, not assumed: the previousCount cursor collects history from a reverse audit getRange under the default snapshot, bounded by count = 10 — well under REPLAY_YIELD_INTERVAL = 100 — so it completes inside the subscribe IIFE's first synchronous segment, before subscribe() resolves and long before the sentinel commits; the non-collection shape captures entry in that same segment and never re-reads it. Neither cursor can deliver the sentinel, so it can only arrive through pendingRealTimeQueue, which drains in commit order after every in-flight delivery. Instrumented over 6 contended full-file lmdb runs plus isolated runs: the sentinel was the last event every time, with zero in-flight events after it. Isolated run of the count test: 5.3s vacuous pass → 0.25s real one.

collect() quiet window at the burst test — confirmed and converted, as suggested. The three other tests in the file with the same shape (writes issued after subscribe(), then a completeness assert — the two replay-race tests and the current-state update test) are converted too, so the claim about the idiom is actually true now rather than nearly true.

One pinned seed samples the property once — confirmed by direct measurement: sweeping 40 arbitrary seeds at this head, 4 produce a graph that legitimately diverges. The suggested remedy would itself flake, though: with ~10% of graphs diverging on at least one of the 4 targets, "at most 1 of 20 diverges" fails about 20% of the time — a worse flake than the one being removed, and "at most 3" still fails ~1%. Instead the test now sweeps 8 fixed seeds, each asserted for exact equality, all verified non-divergent at this head — deterministic, 8× the sampling, +3s of suite time. The cost is that an intentional index change can force a seed re-pin; DESIGN.md says so.

Teardown restores only the rocksdb expiration — correct, fixed exactly as suggested; both process-wide globals are restored.

A pre-push cross-model round on the result (codex + gemini + harper-domain) found one further defect worth naming: the per-seed T.dropTable() in the new sweep loop was unawaited, which would have raced the next graph's 600 puts and turned a LOCK_TIMEOUT rejection into an unhandled rejection. Fixed in c44777a.

— Claude Opus 5

[kriszyp]

Round-8 push (47f9b899). The remaining ask from the last thread was the ticket for the lost delivery rather than the quarantine — and chasing it turned the hypothesis in the PR body into a demonstrated mechanism, so the ticket is a root-cause report rather than a placeholder.

Subscription replay: startTime collection branch silently drops events committed after the audit cursor terminates (#2311) — P1 Bug, parented to [Epic] Audit-log subsystem hardening (#1651).

Table.ts:4149-4188 is the one replay branch that discards live events (pendingRealTimeQueue = null) rather than buffering them, on the assumption its snapshot: false cursor picks up the tail. getRange() returns a terminating iterator, so the window between cursor exhausted and dropDuringReplay = false is covered by neither path. Forced by inserting a 100 ms delay between the replay loop and its finally (in dist only): 200 of 200 in-flight events hit the dropDuringReplay early return, the cursor recovered none, and the test failed as missing in-flight id 20000 — your exact signature. Worth recording for whoever fixes it: the mid-replay part of the window is genuinely covered, so a fix should not over-correct there. With the artificial delay at the await rest() yield instead, 200 events were dropped by the listener and all 200 were then delivered by the cursor.

One correction, on the engine hypothesis. The grep-isolated leg was not running on the default engine. Probing the store path on exactly that command shows test2.mdb, and lmdb is the faster pass here (80 ms vs 143 ms on rocksdb), not the ~20× slower one. This head is now 41/41 green on lmdb on this box — 15 sequential grep-isolated, 18 six-way contended, 8 pinned to a single core.

The real difference is which side of the window the writes land on. Instrumented here, the replay cursor sees an empty audit range and there are zero listener drops, so the uncovered window is never entered at all. On a box where the in-flight writes straddle the end of replay it is deterministic. Both of our measurements are right; #2311 is what sits between them. Thanks for pushing on this one — the 6/6-vs-0/10 disagreement is what made the window visible.

The test stays unquarantined, for the round-7 reasons, but the comment at the wait now points at #2311 so a missing in-flight id N failure reads as the product defect rather than as runner slowness.

— Claude Opus 5

[cb1kenobi]

Reviewed 56e54450 (was 47f9b899) — no issues found. This PR looks good, nice job!

The branch was rebased (f8a5aa90 → df5355aa), same 8 commits 1:1. Isolating the PR's own content at each head, the only real change is in unitTests/resources/subscriptionReplay.test.js, in startTime: heavy concurrent writes during replay all arrive without loss. Four changes there, all strict improvements:

1. The data listener now attaches before the concurrent writes are kicked off. Previously subscription.on('data', …) was registered after the concurrentWrites IIFE had already started, so any event delivered in that window was dropped on the floor — in a test whose entire purpose is proving nothing is lost. Attaching immediately after subscribe() closes that.

2. .catch(() => {}) is gone from the waitFor. A delivery timeout used to be swallowed, so the run continued into the uniqueness and chronology assertions with a partial event set. It now throws, with message: 'not all replayed ids were delivered', which is the difference between "this test failed and told you why" and "this test failed somewhere downstream for reasons you have to reconstruct."

3. The body is wrapped in try/finally so subscription.return?.() still runs. This is required by (2): now that waitFor can throw, the old straight-line teardown would have leaked the subscription on exactly the path that just started failing.

4. The additive settle went 100ms → 200ms, matching the quiet window the sibling tests use, so a duplicate trailing the last expected delivery has a wider gap in which to surface.

I did check the one thing that looked like a risk from the diff shape — that a collect() conversion had been reverted, since retiring those quiet windows is the point of this PR. It had not been. The collect() helper at this head is genuinely condition-driven, not a sleep:

const deadline = Date.now() + timeout;
while (Date.now() < deadline) {
    if (events.length >= minEvents && Date.now() - lastEventAt >= quietMs) break;
    await delay(Math.min(quietMs, 20));
}

minEvents plus a quiet tail, capped by timeout — so the remaining collect(subscription, N) call sites bound the tail rather than racing a fixed window from t=0, which is the stated goal.

CI is fully green (43 success, 4 skipped, 0 failures) and all six findings from the earlier rounds are resolved.

—
Generated by Barber AI