test(engine): stop engine-default exiting 1 with zero failing tests (#2855)

## The problem

On `main`, the whole `engine-default` project exits **1 while reporting
736 files passed and 0 failed**:

```
Test Files  736 passed | 1 skipped (737)
     Tests  9944 passed | 14 skipped | 1 todo (9959)
    Errors  9 errors
```

Nine unhandled rejections — `TypeError: this.store.listTasks is not a
function`, all from `self-healing-db-corruption.test.ts`, raised *after*
its tests had passed.

That is worse than a plain failure: the lane is red, **nothing names a
test**, and a genuine regression landing later arrives in an already-red
lane where it reads as more of the same noise.

## Why the existing isolation didn't hold

The suite stubs every other maintenance sweep so only the corruption
path runs. But `runMaintenance` invokes the surfacing family like this:

```ts
{ name: "surface-in-review-stalled", fn: () => this.surfaceInReviewStalled(maintenanceSurfacing()) },
```

`maintenanceSurfacing()` lazily calls `openSurfacingCycle()`, which does
`await this.store.listTasks({ slim: false })`. **Spying on
`surfaceInReviewStalled` replaces the method, but the call site still
evaluates its argument** — so the shared cycle opened regardless,
against a mock store that deliberately implements only what corruption
surfacing needs.

## Two fixes that look right and are not

Both tried, both reverted — recorded because each is the obvious next
move:

| attempt | result |
|---|---|
| add `listTasks` to the mock | **3 tests fail** — the sweeps then run
far enough to record audit events the assertions don't expect |
| stub the 27 maintenance methods missing from the suite's lists | **5
tests fail, and all 9 errors remain** — `openSurfacingCycle` isn't among
`runMaintenance`'s own calls, so the drift was a real but unrelated gap
|

The second result is what located the fault: stubbing everything
`runMaintenance` calls does not silence the rejections, so they don't
originate there. The stack confirmed it —
`SelfHealingManager.openSurfacingCycle` at `self-healing.ts:8015`. I
should have read that stack before guessing twice.

## The fix

Stub the cycle itself. `null` is exactly what `openSurfacingCycle`
returns when the engine is paused, so every consumer already handles it,
and the corruption-surfacing path this suite exists to test is
untouched.

## Verification

- `self-healing-db-corruption.test.ts` — **6/6, 0 errors, exit 0**
- full `engine-default` project — **736 files passed, 9944 tests, 0
errors, exit 0** (was exit 1)
- `pnpm lint` — clean

Test-only change; no production file is touched, so no changeset.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-30 15:27:22 -07:00
committed by GitHub
parent 92d82b7a17
commit 68fd781eb8

View File

@@ -123,6 +123,31 @@ function stubMaintenance(manager: SelfHealingManager) {
for (const method of BATCH2_METHODS) {
vi.spyOn(manager as never, method).mockResolvedValue(0 as never);
}
/*
FNXC:TestInfrastructure 2026-08-01-23:05 (the suite exited 1 with ZERO failing tests):
STUBBING THE SWEEPS IS NOT ENOUGH — the shared surfacing cycle is built in the ARGUMENT.
`runMaintenance` invokes the surfacing family as `() => this.surfaceInReviewStalled(maintenanceSurfacing())`,
and `maintenanceSurfacing()` lazily calls `openSurfacingCycle()`, which does
`await this.store.listTasks({ slim: false })`. Spying on `surfaceInReviewStalled` replaces the
METHOD but the call site still evaluates its argument, so the cycle opened anyway against a mock
store that deliberately implements only what corruption surfacing needs.
The result was nine `TypeError: this.store.listTasks is not a function` UNHANDLED REJECTIONS raised
after the tests had already passed: `engine-default` exited 1 while reporting 736 files passed and
0 failed. Worse than a plain failure — the lane is red, nothing names a test, and a real regression
arriving later reads as more of the same noise.
Two fixes that look right and are not, both tried and reverted:
adding `listTasks` to the mock -> 3 tests fail; the sweeps then run far enough to record audit
events the assertions do not expect.
stubbing the 27 maintenance -> 5 tests fail AND all nine errors remain, because
methods missing from the lists `openSurfacingCycle` is not among `runMaintenance`'s own calls.
Stubbing the cycle is the honest seam: `null` is exactly what it returns when the engine is paused,
so every consumer already handles it, and the corruption path this suite exists to test is
untouched.
*/
vi.spyOn(manager as never, "openSurfacingCycle").mockResolvedValue(null as never);
vi.spyOn(manager, "archiveStaleDoneTasks").mockResolvedValue(0);
}