The #2248 skills tests time out under load at the default 5s testTimeout

[cliffhall]

Found while running npm run local:gate repeatedly on #2264 (PR #2322), a branch touching zero files under clients/.

What happens

Two files in the web unit project fail intermittently, always with Error: Test timed out in 5000ms. — never with a failed assertion:

• clients/web/src/components/screens/SkillsScreen/SkillsScreen.test.tsx
• clients/web/src/test/core/mcp/skillsVerification.test.ts

Both are #2248 surfaces. Observed across five gate runs on the same branch:

Run	Result
1	green
2	red — 1 timeout (SkillsScreen > "offers Up only below the skill root, and returns to it")
3	green
4	red — 5 timeouts (2 in SkillsScreen, 3 in skillsVerification)
5	green

Why it is load, not the branch

• The branch changes five markdown/tooling files and nothing under clients/ — git diff origin/v2/main...HEAD --name-only on v2/chore/2264-testing-handoff.
• Re-running both files in isolation passes 156/156.
• In the red run, SkillsScreen.test.tsx took 91382ms for 111 tests; alone it takes ~48s. The suite was starved, and a 5s per-test cap is what turned that into a failure.

Why it is worth fixing rather than re-running

This is the same class as the Storybook-project-had-no-testTimeout finding: a per-test cap inherited from vitest's 5000ms default, on tests whose work is real enough to exceed it when the machine is busy. It is green on an idle machine and red on a loaded one, which means it fails in CI as a function of runner contention rather than of the code — and it trains people to re-run a red gate instead of reading it, which is exactly what the gate exists to prevent.

Suggested direction

Not asserted as the fix — whoever takes this should measure first:

1. Find out what actually takes the time. If a single waitFor in these files is doing real polling work, the honest fix may be a longer testTimeout for the unit project, or a per-test timeout on the specific cases.
2. If instead the tests are waiting on something that should be deterministic (an accordion panel, a listing read), the fix is to remove the wait rather than extend the budget.

WARNING: Do not close this by raising a waitFor timeout alone — a waitFor is capped by the project testTimeout, so raising only the inner wait changes nothing.

Acceptance

• The cause is identified as one of the two above, and said explicitly in the fix.
• Twenty consecutive unit project runs of both files, with the machine deliberately loaded, are green.

🤖 Generated with Claude Code

https://claude.ai/code/session_018zwYWanPhvgu6wiKoeGg4N

[cliffhall]

Status after #2323 (PR #2334): the 5s trigger is gone, the failure is not

Recording this so the next person starts here rather than re-deriving it.

#2323 raised the web unit project's testTimeout from Vitest's inherited
5000ms to a stated 15000ms, so the specific failures reported above cannot
recur in that form. This issue is not fixed. Across the gate runs on
v2/fix/2323-test-gate-timeouts, SkillsScreen.test.tsx failed three times,
every one of them:

Error: Test timed out in 15000ms.

• SkillsScreen directory browsing (#2248) > descends into a child directory
• SkillsScreen directory browsing (#2248) > pages manually, accumulating children rather than replacing them
• SkillsScreen frontmatter cross-check (#2248) > counts frontmatter findings in the Conformance badge

Plus one in a project this issue does not mention:
SkillsScreen.stories.tsx > Long Skill Document, in the storybook project
(which already had testTimeout: 15000 from #2292).

So a 3x wider window did not close it, and this issue's own warning — "do not
close this by raising a waitFor timeout alone" — turns out to apply one level
up as well: raising the project budget is the same move with more reach.
Neither acceptance criterion here is met: the cause was never identified, and
the twenty-run loaded check was never performed. #2334 raised a shared budget
across all six projects and never looked at these two files.

Measurements, for whoever takes it

SkillsScreen.test.tsx in isolation, on the branch with the raise applied:

Tests	111 passed
Duration	9.12s (8.24s in tests)
Mean per test	~74ms

To blow a 15000ms cap, a ~74ms test has to be starved by roughly 200x. That
is the file-level starvation this issue already documents (91382ms vs ~48s
alone), not a slow assertion — which means the budget was probably never the
right lever.

Two more numbers that point at this issue's second suggested branch
("remove the wait rather than extend the budget"):

• 77 waitFor / findBy* calls in SkillsScreen.test.tsx, against
  0 in skillsVerification.test.ts — yet both files were named in the
  original report, so whatever they share is not the waits.
• The SkillsScreen accordion sets transitionDuration={0} (established in
  Flaky Storybook test: SkillsScreen "Long Skill Document" geometry assertion races the collapse/reopen transition #2278), so several of those waits are polling for a React commit plus a layout
  pass rather than for an animation. That is the "should be deterministic" case.

A third possibility neither this issue nor #2323 considered

This may be a concurrency problem rather than a budget one. A file whose
mean test is 74ms only starves when workers contend, and no Vitest project in
this repo sets maxWorkers — all six inherit availableParallelism(), which is
8 on this hardware, of which only 4 are performance cores. Measured there, 8
workers burn 40% more CPU for identical work than 2. That is #2336, and it may
be the actual fix for this issue.

Worth testing that hypothesis before touching these tests: run the twenty
loaded iterations this issue asks for at --maxWorkers=4 and see whether the
timeouts simply stop. If they do, the tests were never the problem.

Tracking

Attached as a sub-issue of #2338, which holds the shared goal and — importantly
for this issue — the measurement standard: three arms, three runs each,
baseline / change / change reverted. #2323 shipped one change on reasoning alone
that measurement later showed made things worse, so please measure before
changing a number here.

[cliffhall]

Reproduced on the leased baseline: it does not fail, and the tests carry no defect to fix

Recommendation: dismiss (close as not planned, delete the card). The mechanism this issue describes — a 5s per-test cap turning file-level starvation into a red run — has been removed twice over since it was filed, and the residual at 15000ms that the previous comment recorded was measured under overlapping gates, which #2339's lease now prevents. Measured after #2339 and #2336 landed, both files and the Storybook story are green in every run, quiet or deliberately loaded, and no test in them contains any of the three shapes that would make a per-test budget the wrong lever.

Method

v2/main @ 4a522ebc, fresh worktree, real npm install, no gate and no other Vitest on the machine (checked with ps before each arm; ps -r during the loaded arms showed only the two Vitest trees below). Each run wrote a JSON report, so the per-test durations are Vitest's own.

• Quiet: the two files through vitest run --project=unit, back to back.
• Loaded: the same, looped for the entire lifetime of a sibling full vitest run --project=unit (345 files, 7 workers) started 8s earlier in the same worktree — the contention Vitest worker count is a library default that oversubscribes this team's machines #2336 used, 14 workers on 8 cores, 1-minute load average 16–62.
• The same two arms for SkillsScreen.stories.tsx in the storybook project, since the previous comment attributed a Long Skill Document failure to this surface.

Results

Arm	Runs	Failures	Wall (min / median / max)	Slowest single test
Two files, quiet	10	0 (156/156 each)	8.8 / 8.9 / 8.9s	327ms
Two files, loaded	20	0 (156/156 each)	18.3 / 21.9 / 26.7s	1033ms
Sibling full unit runs (the gate's own shape)	13	0 (6461/6461 each)	48.1 / 55.4 / 63.4s	1450ms
Stories file, quiet	5	0 (10/10 each)	3.4 / 3.4 / 3.7s	413ms
Stories file, loaded	15	0 (10/10 each)	3.6 / 6.7 / 7.1s	813ms

The tests named in the two reports, worst case across all 43 unit-project runs that contained them:

Test	Quiet max	Loaded max	Budget
descends into a child directory	203ms	709ms	15000ms
pages manually, accumulating children…	218ms	805ms	15000ms
counts frontmatter findings in the Conformance badge	59ms	202ms	15000ms
offers Up only below the skill root…	327ms	1033ms	15000ms
Long Skill Document (storybook)	182ms	279ms	15000ms

So the acceptance criterion's twenty loaded runs are green, and the worst any named test reached under 14 workers is 7% of its budget. Inflation under the sibling was ~2.5x at file level and ~3x on the slowest test — consistent with two saturating processes sharing eight cores, and nothing like the ~50–200x a 74–320ms test needs to hit 15s.

Why this is starvation and not a test defect

The three shapes that would make these files the problem are all absent, checked by reading both files rather than by inference:

• No wait that spends its budget on the passing path. Every waitFor / findBy* (77 in SkillsScreen.test.tsx, 0 in skillsVerification.test.ts) polls for a condition that holds within milliseconds of a mocked promise resolving; none is an absence assertion written as an expected rejection (the Web unit tests that assert absence by letting a Testing Library wait expire #2335 class), and the three .not.toBeInTheDocument() waits pass on the first tick once the state lands.
• No fixed sleep, no fake timers, no per-call timeout: in either file or in the story. Nothing scales with a global.
• No geometry race. The Long Skill Document play function already reads its post-reopen geometry under waitFor (Flaky Storybook test: SkillsScreen "Long Skill Document" geometry assertion races the collapse/reopen transition #2278), and the accordion sets transitionDuration={0}; in 20 runs its worst case was 279ms and the one failure seen during Storybook stage fails on a Vite dep-optimizer race, not a timeout — 15 stories cannot load a pre-bundled chunk #2340 was in a run whose SkillsScreen stories were also failing to load modules — the stale dep-cache Storybook stage fails on a Vite dep-optimizer race, not a timeout — 15 stories cannot load a pre-bundled chunk #2340 fixed, not this surface.

The tests that appear in every timeout report are the directory-browsing ones because they are the longest in the file (four sequential userEvent clicks, 200–330ms quiet) — the first to cross any ceiling when the whole worker is starved, which the openRoot helper's own comment already says. That is a property of which test is slowest, not of a wait inside it.

What actually produced the red runs is on the record: the original five runs were concurrent gates on a machine running several worktrees (91382ms for a ~48s file), and the three 15000ms timeouts in the previous comment came from gate runs on v2/fix/2323-test-gate-timeouts before #2339 existed. Overlapping gates are now serialized by the lease, and #2336 measured that the remaining sibling-session case produces 0 failures in these files (the one site it did find, AppRenderer.test.tsx's 5s inner waitFor, is not in this issue's scope).

What would reopen this

Test timed out in 15000ms in either file on a run whose only contention is a sibling session — i.e. under the lease — with the JSON report showing which test and how long the neighbours took. Until then there is nothing in these tests to change, and changing a budget for them would be #2323's move again with less reason.

Raw per-run data (JSON reports and the two campaign scripts) is available on request; nothing was committed, since the recommendation is that nothing changes.