Fix: Root-cause the flake cluster that made "rerun once" standard advice

[cheapsteak]

What's broken

A recurring cluster of tests fails under machine load and passes on targeted rerun: both ProviderEventsSupervisorTests, the MarkdownStylesheetTests directory-watch test, the ThemeStoreTests watcher-reload test, and (analyzed but not changed here, see below) the AppearanceDebounceTests/SearchQueryDebouncerTests burst tests. Across four full runs on a loaded dev machine, every run had at least one of these red; each went green on rerun. The standing advice calcified into "rerun once", which taxes every branch and trains people to wave past red suites.

Why it happens

Swift Testing runs every non-serialized test in one process with no concurrency cap, so per-test scheduling latency scales with the total population (now 5417 tests) plus external machine load — mined runs put p50 per-test reported duration at ~1/3 of total wall time (Tests/CLAUDE.md, "Population is the scheduler"). Each flaking test waited on that real scheduling behind a wall-clock bound sized for a smaller population or a quieter environment. Per-test root causes, each traced through the production pipeline (none is a race in the code under test):

• ProviderEventsSupervisorTests (tier 3) — 15s bounded-poll deadlines were sized for CI's quiet serial pass; a loaded parallel local run starves the supervision task past them (observed: pid file never read in 15s; mirror rows still empty at 15s). On top of that, one assertion had a genuine logic race: snapshotAppliedAndRestartResyncs demanded the exact transient set {live-a, run-1}, but the stub goes silent after its snapshot and silenceLimit is 5, so the supervisor's own watchdog legitimately kills and respawns it every few seconds, replacing run-1 with run-2 — an observer starved past the first cycle could never match, at any deadline.
• MarkdownStylesheetTests watch test (tier 2) — waited out a real 150ms FileWatcher debounce plus AsyncStream delivery under one 8s bound. FileWatcher already takes an injected clock, and the sibling FileWatcherTests already proves the real-dispatch-source + TestClock pattern; this test predated it.
• ThemeStoreTests watcher test (tier 2) — FSEvents cannot be virtualized (the 0.1s is a stream latency parameter, not a sleep; events are journaled by fseventsd, so none can be lost) and bounded polling is the sanctioned shape; the 30s bound predated the population re-derivation that took the shared guards to 45s/90s. Its wait rides the single MainActor executor that every @MainActor suite queues on.

When it broke

Not one regression commit: each bound was correct for the population it was derived against (the ThemeStore 30s in #441 against ~3000 tests; the supervisor 15s with #514 assuming the quiet tier-3 pass). Population growth to 5417 plus multi-agent load on the dev box moved the environment out from under them. The run-1 transient-set race has been latent since #514 — it needs a >5s observation delay to bite, so only load exposed it.

What this PR does

• MarkdownStylesheetTests: rewrites the watch test onto the FileWatcherTests idiom — real dispatch source delivers the event, the debounce timer is virtual (FileWatcher(clock: TestClock) via an arm-counting delegating clock). No wall bound covers the debounce any more; the remaining bounded polls guard only genuinely real-time legs (kqueue registration/arming, AsyncStream delivery) and throw named diagnostics.
• ProviderEventsSupervisorTests: rewrites runAssertions against the contract instead of run indices — a snapshot applied (live-a + any run-N), invocation count strictly above its value at the kill, then some run-M with M > maxRunAtKill (a snapshot only a post-kill connection can produce). Re-derives deadlines with the reasoning inline: bounded-poll guards 15→90s (anchoring to the ciSafeDeadline figure for the same contention class), zombie-reap guards 5→30s, the stop() promptness bound 10→60s, and pins a 6-minute suite hang limit so a regressed stop() cannot wedge a whole local run.
• ThemeStoreTests: re-derives the reload hang guard 30→90s (same anchor), and upgrades the timeout to a thrown diagnostic reporting userThemes, loadErrors, and the actual directory listing — discriminating "watcher never fired" from "reload ran but decode failed" from "file never landed".

On the raised numbers, explicitly rather than quietly: every one is a positive wait that breaks on its first satisfying probe — healthy runs pay ~0.1s, only a genuinely failing run pays the deadline — and each carries its derivation in a comment so the next re-derivation has an anchor. This is the repo's established hang-guard doctrine (Tests/CLAUDE.md), not tolerance-window inflation; the transient-state fix above is the part no deadline could have fixed.

Deliberately not changed: the AppearanceDebounceTests/SearchQueryDebouncerTests burst tests. They are already fully virtual-time and .serialized; their failure is the arming handshake starving inside shared machinery (TestClock.checkSuspension's background-QoS megaYield under process-global saturation), and every suite-local remedy is already measured-and-refuted in Tests/CLAUDE.md. The real fix is the megaYield-free virtual clock that ClockTestSupport names as a deliberate shared-contract non-goal — that reshapes shared test infrastructure, so it needs a human decision first rather than being bundled here. Also checked per the burn-down list: all five production types in the cluster already carry the clock seam; no no_raw_task_sleep suppressions are involved.

Evidence & verification

Mutation checks (each applied to Sources/, observed red with its named diagnostic, restored, re-observed green):

• FileWatcher eventMask minus .write/.extend → a FileWatcher on a directory armed no debounce timer for an entry being created — observed armed=0 after 4 attempts of 2.0 seconds each
• ThemeStore.startWatching() without w.start(directory:) → thrown diagnostic at 90s whose directory listing shows ext.json present, pinning "watcher never fired"
• Supervisor restart loop removed → supervisor did not restart the events process within 90.0s; observed invocation count=1, was 1 at the kill
• SIGKILL escalation removed from killTree → red via the leak check at the 30s reap guard (stop() returned promptly; the child survived)

Targeted suites green before and after: 32 tests across the three files (the rewritten markdown watch test now runs in ~0.1s instead of riding a real debounce). scripts/swift-safe build clean, swiftlint --strict clean. Full-suite under-load soak runs are executing on the loaded dev box now; results will follow as a PR comment.

🔗 open in tbd

[tbd-claude-reviewer]

✅ Looks good

Both review lenses — correctness and CLAUDE.md conventions — came back clean on this PR.

Correctness. The diff is confined to three test files (Tests/TBDAppTests/MarkdownStylesheetTests.swift, Tests/TBDAppTests/ThemeStoreTests.swift, Tests/TBDDaemonLiveTests/ProviderEventsSupervisorTests.swift), confirmed against the pinned merge base with no production Sources/ changes slipped in. The specialist spot-checked the PR's three most load-bearing factual claims about existing code and found each accurate: FileWatcher's injected-clock seam is real and the new test clock wrapper mirrors the established FileWatcherTests idiom; ThemeDirectoryWatcher's FSEvents latency genuinely isn't virtualizable; and the supervisor's watchdog/silenceLimit behavior matches the premise behind the rewritten transient-set assertions. The rewritten ProviderEventsSupervisorTests assertions were traced against the PR's own mutation-testing scenarios (restart loop removed, resync broken, SIGKILL escalation removed) and still catch each regression — the loosened "contract not run-index" assertion isn't a permissiveness regression.

Conventions. None of the flag/TUI-scraping/private-context/migration rules apply to a test-only PR, and that scope was verified rather than assumed. The claimed anchor doctrine in Tests/CLAUDE.md ("Population is the scheduler", ciSafeDeadline figures) was confirmed to actually exist and actually match the PR's re-derived constants and inline derivation comments — this reads as a bug fix restoring tests to the suite's existing hang-guard doctrine, not a new theory needing a spec.

0 findings were filtered out as invalid during the merge (both specialists returned empty findings arrays).

Finding dispositions

No specialist findings were reported by either lens — nothing to dispose of.

Review diagnostics

No tool calls failed or were denied for either specialist or the orchestrator. One environmental note: .git/shallow in this checkout lists a commit that is not the pinned merge base SHA, so both specialists correctly scoped their git log/git blame-derived conclusions to not cross that boundary, relying on the pinned-SHA diff and direct file reads instead. Both specialists also independently confirmed the diff stat (three test files, no Sources/ or .swiftlint.yml changes) before proceeding.

Posted by the claude-review check — the review of this PR's diff at patch-id 72eaa9705198a587b33c50a20a946781932e31bd. A newer review comment supersedes this one.

[cheapsteak]

Under-load soak results (the failure condition from the PR description)

Three consecutive full-suite runs (5411 tests in one process) on the loaded dev box, load averages 15–44 on 12 cores at run starts:

• Fixed cluster: 12/12 green. All four rewritten tests passed in all three runs. Their reported durations show the regime they survived — mostly time suspended waiting for a scheduler turn, exactly the population effect the fixes were derived against:
  • ThemeStore watcher reload: 131s / 228s / 92s reported (old bound: 30s — every run would have been red before this PR)
  • MarkdownStylesheet directory watch: 44s / 63s / 33s reported (old end-to-end bound: 8s)
  • ProviderEventsSupervisor: 33–55s reported (old bounds: 15s)
• The deliberately-unfixed burst pair reproduced its diagnosed failure in 2 of 3 runs (AppearanceDebounceTests "rapid scheme changes…", SearchQueryDebouncerTests "a burst within one window…"): Issue recorded at the advanceWhenSuspended arming handshake, then fired.values → [], at ~131s and ~228s. Live confirmation of the TestClock arming-starvation root cause described in the PR body; the megaYield-free clock lands as a stacked follow-up PR.
• Out-of-cluster load flakes observed during the soak, not touched by this PR: ReplayLiveIntegrationMatrixTests firehose-overflow (run 1) and the TerminalTeardownReapTests/ChildReaperTests PTY-reap suites (run 2). Same family, separate work.

Also for the record: this PR's first CI run flaked on WorktreeArchiveDeletionQueueTests.theFallbackRemovalArmsItsOwnRaisedTimeout — a test outside this diff, same load-sensitive class — rerun in progress at time of writing (this line will not be edited again; check the checks tab). The tax this PR is about, paid one more time on its own gate.