Test hygiene: make root src/__tests__/ CI-able (pollution + infra deps), then widen the CI Test scope

[TerrysPOV]

Context

CI now runs bun test on the clean module suites (src/bus, src/tuner, src/observability, src/skills-tuner, src/governance — ~700 tests, all green) via the new Test workflow. The root src/__tests__/ suite is excluded because a full bun test on main is 2518 pass / 41 fail, and all 41 failures live in src/__tests__/.

The 41 failures split two ways

1. Cross-test pollution — tests that pass in isolation but fail in the shared process (e.g. Phase 18: runner modelOverride wiring passes alone, fails in the pack). Root cause is shared module/singleton/env/file state between test files. This is the same class the SDC mark-tests-passing.sh works around with TEST_ARGS scoping.
2. Infra-dependent — tests that fail even in isolation because they need a DB/queue/service not present in CI (e.g. Event Processor → 9 fail alone).

Representative failing files: event-processor, jobs, escalation/*, gateway/*, policy/*, mcp_proxy_*, pty-*, runner, integration/*.

Goal

Make src/__tests__/ green (fix pollution via proper per-test isolation/teardown; gate or mock the infra-dependent tests), then widen the CI Test job's paths toward the full bun test. Landmark: CI runs the whole suite green.

Why it matters

The reason this surfaced: PR #290's 3 hanging collectObservations tests showed green in CI because CI ran no tests at all. The scoped Test job catches that class in the module dirs now; closing this issue extends the safety net to the whole codebase.

[TerrysPOV]

Re-baselined at 36321ad — the original diagnosis was wrong, and the scope is smaller than it looks

Full bun test on current main: 2893 pass / 6 skip / 41 fail across 2940 tests in 180 files. The failure count is unchanged since this issue was filed, but the composition is not what the issue describes.

Finding 1 — all failures live in 10 files, not scattered

Failures	File
9	src/__tests__/event-processor.test.ts
9	src/__tests__/runner.test.ts
7	src/__tests__/mcp-multiplexer-integration.test.ts
7	src/__tests__/session-persistence-integration.test.ts
3	src/__tests__/jobs.test.ts
2	src/__tests__/integration/model-override.test.ts
2	src/__tests__/policy/wiring.test.ts
1	src/__tests__/agents.test.ts
1	src/__tests__/pty-boot-dialog.test.ts
1	src/__tests__/integration/escalation-wiring.test.ts

(The per-file tally sums to 42 against a reported 41 — one test appears to double-report or flake. Not material to the plan.)

Finding 2 — it is NOT mostly cross-test pollution

Running each of the 10 files in isolation:

event-processor.test.ts                 4 pass   9 fail
runner.test.ts                         15 pass   9 fail
mcp-multiplexer-integration.test.ts     0 pass   7 fail
session-persistence-integration.test.ts 0 pass   7 fail
jobs.test.ts                           33 pass   3 fail
model-override.test.ts                  3 pass   2 fail
wiring.test.ts                          8 pass   0 fail   <-- only pollution case
agents.test.ts                        117 pass   1 fail
pty-boot-dialog.test.ts                 3 pass   1 fail
escalation-wiring.test.ts              14 pass   1 fail

41 of 42 failures reproduce in isolation. Genuine cross-test pollution accounts for exactly one file — policy/wiring.test.ts, 2 failures. The issue's framing of pollution as a primary cause does not hold.

Finding 3 — it is NOT infra dependency either

No DB, queue, or service is involved. event-processor.test.ts and jobs.test.ts contain zero infra references. The two files that look infra-ish fail before reaching any I/O.

The real cause is test rot: these tests were excluded from CI, so nothing ran them, so they drifted from the code they cover.

Root cause	Failures	Evidence
settings.cache.cacheable is undefined	14	src/plugins/mcp-multiplexer/index.ts:296 reads it unguarded; the hand-rolled makeSettingsView fixtures (duplicated at mcp-multiplexer-integration.test.ts:64 and session-persistence-integration.test.ts:73) predate the #69 cache field and never gained it
dedupeKey is not defined	9	event-processor.test.ts:300 references an undeclared variable — a ReferenceError in the test itself
primaryConfig.model.startsWith is not a function	~4	model became an object; runner and model-override still treat it as a string
Assertion drift — stale expected values	~13	jobs, agents, pty-boot-dialog, escalation-wiring
Cross-test pollution	2	policy/wiring

Finding 4 — several of these are statically detectable today

bunx tsc --noEmit already flags the stale fixture:

src/__tests__/mcp-multiplexer-integration.test.ts(65,9): error TS2322:
  Type '{ ... }' is not assignable to type 'MuxSettingsView'.

There is no typecheck script in package.json and no typecheck step in any workflow, so nothing catches this class. That is the same gap tracked in #324, and it is part of the answer here: a typecheck step would have prevented the largest cluster from ever rotting.

---

Revised scope

The goal stands — make src/__tests__/ green and widen the CI Test job — but the work is fixing rotted tests, not building infra or unpicking shared state.

Delivered as one PR per root-cause cluster, each widening the CI path list as it lands, so every step permanently locks in its gain:

1. settings.cache fixtures — 14 failures, 2 files. Largest win, likely smallest diff.
2. event-processor dedupeKey — 9 failures, 1 file.
3. model shape drift — ~4 failures across runner and model-override.
4. Assertion drift — ~13 failures across 4 files. Each needs judgement on whether the test or the code is wrong.
5. policy/wiring pollution — 2 failures. The only one needing real isolation work.
6. Widen CI to the full suite + add a typecheck step, closing this issue.

Note that CI scope already widened once since this issue was filed: #321 added src/adapters (1357 tests now run, up from ~1100).

Each PR will state which paths it adds to the CI list, so progress is visible in .github/workflows/test.yml rather than only in this thread.

[TerrysPOV]

Correction to my scope comment: cluster 2 was a SOURCE bug, not test rot

My root-cause table above says:

| dedupeKey is not defined | 9 | event-processor.test.ts:300 references an undeclared variable — a ReferenceError in the test itself |

Wrong file. I read line 300 off a stack trace and attributed it to the test. Verified against povai/main:

src/event-processor.ts:300        ->  await recordProcessed(nextEvent, dedupeKey);
src/__tests__/…test.ts:300        ->  await append(createTestEntry({ type: "type-c" }));

The undeclared reference was in source. All 9 failures were one ReferenceError thrown from processNext(), not from test code.

Why it matters for the remaining clusters

The framing of this issue — and of my own comment — is "test rot: tests drifted from the code". That held for cluster 1 (stale fixtures). It does not hold for cluster 2: the tests were correct the whole time and were correctly reporting a real defect in src/event-processor.ts, introduced by 8cbd850 on 5 April and unnoticed for 3.5 months.

So the revised scope needs a third category alongside "pollution" and "test rot": source bugs that only the excluded tests were catching. Whoever takes clusters 3-5 should check which side the fix belongs on before assuming the test is at fault. The model shape drift cluster (primaryConfig.model.startsWith is not a function) looks like it may be the same shape.

Related detail worth recording

processNext has never had a production caller — the daemon reaches this module via processPersistedEvent instead (start.ts → initGatewayProcessor → getGatewayProcessor). So the defect was never user-visible, which is precisely why it survived: nothing but the excluded tests ever ran that path.

That also strengthens the case for a typecheck step (#324) — tsc would have caught dedupeKey is not defined in April.

[TerrysPOV]

Reframing: the high-value part is done, the rest has thin returns

Three clusters in, I want to be straight about what this issue has actually bought and what remains, rather than marching through the remaining failures because the plan said so.

What it bought — real

CI now runs 1378 tests, up from ~1100. That is the whole point, and it is banked:

• test: share one MuxSettingsView fixture, reclaim 14 rotted tests (#304) #328 (cluster 1) — 14 rotted tests reclaimed, three divergent fixtures consolidated into one, src/__tests__/mcp-multiplexer-integration.test.ts added to CI
• fix(event-processor): record the dedupe key the event was selected with (#304) #329 (cluster 2) — a source bug, dedupeKey is not defined, which sat unnoticed for 3.5 months because nothing ran that file. src/__tests__/event-processor.test.ts added to CI
• test: seed settings in runner + model-override rather than inheriting it (#304) #331 (cluster 3a) — a real cross-test ordering dependency removed

Cluster 2 is the strongest evidence for this issue's premise: excluding tests from CI is exactly how a source defect survives a quarter.

What remains — thinner than the plan implies

The remaining ~18 failures are largely in code that is not live:

• processNext (fixed in fix(event-processor): record the dedupe key the event was selected with (#304) #329) has no production caller
• modelOverride (runner: modelOverride is untested — RunOptions declared but never wired, and the test harness cannot intercept #330) — RunOptions was declared and never wired, so there is no working feature to protect
• the session-persistence GC test (flaky: session-persistence GC test races the async audit capture (~25% under load) #326) is a broken test, not broken production code

Driving the failure count to zero would mostly be tidying. Cluster 2 took several hours and five review passes to fix a defect on a dead code path — correct work, but low return, and I should have said so at the time rather than after.

What I think is actually worth doing next

Ordered by leverage:

1. tsc: 338 type errors repo-wide block a typecheck step; 14 are the it() timeout overload #327 / bus: handleKill is not type-checked — Extract<StallDecision, {action:"kill"}> resolves to never #324 — typecheck step. tsc --noEmit would have caught cluster 2's dedupeKey is not defined in April, and runner: modelOverride is untested — RunOptions declared but never wired, and the test harness cannot intercept #330's object-into-a-string-parameter immediately. It prevents the entire class rather than fixing instances. Blocked on triaging the 338-error backlog, which is the real work.
2. flaky: session-persistence GC test races the async audit capture (~25% under load) #326 — the GC test. Genuinely broken 60% of the time on a quiet machine (re-measured; my earlier numbers were contaminated by my own runaway processes). Localised to a write race before the GC tick even runs. One real bug, worth fixing.
3. runner: modelOverride is untested — RunOptions declared but never wired, and the test harness cannot intercept #330 — modelOverride. Needs a decision first: is modelOverride a feature worth keeping? If dormant, delete the eleven non-functional tests and record the gap. If it is meant to work, add a seam in runner.ts and write tests that can actually intercept.

Proposal for this issue

Keep it open as the CI-widening tracker, but drop "make the whole root suite green" as the success condition. That target rewards deleting or patching tests that were never meaningful, which is the opposite of the intent.

Better success condition: every test file that CI runs is one that would catch a real regression, and the path list grows only as files earn their place. Three have earned it so far.

The remaining root-suite files can be triaged individually against that bar — some will be worth fixing, some worth deleting, some worth leaving excluded with a recorded reason. That is a smaller and more honest scope than "41 failures to zero".