Fourth cold cross-family review: no runtime correctness finding this round
(single-flight held against synchronous throws, generation overlap,
StrictMode, unmount, and separate hook instances). Two test-gap findings
remained, both the same shape as prior rounds' review feedback: asserting on
something ADJACENT to the mechanism rather than the mechanism itself.
1. ChannelBuilder.test.tsx's page-0 single-flight test asserted on the
disabled BUTTON — during a page-0 refresh, `loadingMore` disables the
native button, so `fireEvent.click` never reaches `loadMore` at all.
Deleting `busyRef.current = true` at libraryBrowse.ts's generation-change
effect would leave that test green while direct hook calls could still
overlap page 0 and page 1. Added a hook-level test that calls
`result.current.loadMore()` directly (via `renderHook`, bypassing the
button/DOM layer) while a gated page-0 fetch is in flight, and asserts no
page-1 request is ever issued. Verified: removing `busyRef.current = true`
at that line makes the new test fail with
`expected [ +0, 1 ] to deeply equal [ +0 ]` (a page-1 request was issued
when the guard should have ignored the direct call); restored, green
again.
2. pageSizeScan.test.ts's `.mts`/`.cts` tests exercised the SCANNER'S
PARSING, not the guard's file-discovery glob — reverting the glob back to
`.ts`/`.tsx` left both those tests green (the scanner still parses a
`.mts`/`.cts` filename as plain TS regardless of extension) AND the
real-source guard green (this repo has no committed `.mts`/`.cts` file
for the reverted glob to miss). A prior verification planted a real
`.mts` file and watched the guard notice it, which proved the behavior
exists today but pinned nothing going forward. Extracted the inline glob
condition into a standalone, exported `isScannableSourceFileName`
predicate and added 14 parametrized cases asserting it BY FILENAME (no
filesystem involved) — `.ts`/`.tsx`/`.mts`/`.cts` accepted, their
`.test.*` and `.guard.test.ts` variants excluded, non-TS extensions
rejected. Verified: reverting the predicate's regex back to `.ts`/`.tsx`
only makes 2 of the 14 cases fail (`isScannableSourceFileName('*.mts')`
and `'*.cts'` both wrongly return `false`); restored, green again.
Nothing else changed — the reviewer confirmed no further findings (loader
async/throw handling, generation sequencing, the unconditional rollback,
StrictMode/unmount behavior, the disabled-during-refresh UX, type-only node
exclusion, computed-name exactness, wrapped-forwarded-call rejection, and
all existing test labelling were all confirmed correct as-is).
Full local gate: lint clean, tsc clean, full vitest run 110 files / 1078
tests passed, re-run twice for stability (no flakes).
timothy
937ee92a3f