From 68fd781eb8eb5cee609e6cb7066564931c4469e2 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 15:27:22 -0700 Subject: [PATCH] test(engine): stop engine-default exiting 1 with zero failing tests (#2855) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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) --- .../self-healing-db-corruption.test.ts | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/packages/engine/src/__tests__/self-healing-db-corruption.test.ts b/packages/engine/src/__tests__/self-healing-db-corruption.test.ts index 8d42d6d549..439f79b9ac 100644 --- a/packages/engine/src/__tests__/self-healing-db-corruption.test.ts +++ b/packages/engine/src/__tests__/self-healing-db-corruption.test.ts @@ -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); }