From 69790dc3e79a5fccbec97b3d9845fcea6c9734ba Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 29 Jul 2026 09:32:34 -0700 Subject: [PATCH] test(U9): revive two permanently-red testMode guards in reviewer.test.ts (#2547) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **U9, PR7.** One test file, +15 lines, no production change. ## Two safety tests that could never pass `reviewer.test.ts`'s `vi.mock("../pi.js")` is missing `wrapToolsWithOutputBudget`, which `wrapCustomToolsForPluginRuntime` (`agent-session-helpers.ts:104`) calls as the outermost tool wrapper. Both test-mode-forcing cases therefore threw: ``` No "wrapToolsWithOutputBudget" export is defined on the "../pi.js" mock ``` They have been **permanently red on main** — dead enforcement on the invariant that **testMode never issues real AI calls**. `reviewer.test.ts`: 83 passed | 2 failed → **85 passed**. Found while characterizing the reviewer lane for U9: they surfaced as pre-existing baseline failures under an unrelated mutation run. This is exactly why the delta harness records a baseline — under the old absolute-count method these two would have been silently credited to whatever mutation was running. **Not a product bug.** testMode forcing works correctly; its guard did not. ## Verified the revived tests actually guard something A dead test can also be a vacuous one, so passing again is not sufficient evidence. Mutating `isTestModeActive` in `model-resolution.ts` to ignore `settings.testMode` fails **exactly these two** (`NEW-failures=2`). Both assert `expect(mockedCreateFnAgent).not.toHaveBeenCalled()` — no live agent spawn. ## Why the existing gate didn't catch it `pnpm check:mock-completeness` runs in the merge gate and passes. It inspects only the `@fusion/engine` and `@fusion/dashboard` **barrels**, under `cli/` and `dashboard/` test dirs — never a relative intra-package mock like `"../pi.js"`. So the whole class of engine-internal mock drift is outside it. **Deliberately not fixed here.** Extending the checker is its own change and I want the violation count measured before proposing it, rather than opening a PR that turns out to touch dozens of files. That's the next PR. ## Also observed, stated rather than buried Mutating `useMockRuntime` in `agent-session-helpers.ts` produces **no** failure in this file — the reviewer path routes through model resolution instead. That downstream seam has its own coverage question which I have not answered; flagging it rather than implying this PR closes it. ## Scope note This was initially committed onto #2541's branch. I split it onto its own branch so each PR stays independently revertable — #2541 is now one commit (the FN-7720 verdict assertion) and this is one commit. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) --- packages/engine/src/__tests__/reviewer.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/engine/src/__tests__/reviewer.test.ts b/packages/engine/src/__tests__/reviewer.test.ts index 54c8b6e0c5..70f681da30 100644 --- a/packages/engine/src/__tests__/reviewer.test.ts +++ b/packages/engine/src/__tests__/reviewer.test.ts @@ -2,6 +2,21 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; vi.mock("../pi.js", () => ({ createFnAgent: vi.fn(), + /* + FNXC:TestInfrastructure 2026-07-29-10:15 (U9): + `wrapCustomToolsForPluginRuntime` (agent-session-helpers.ts) calls this as the + outermost tool wrapper, so its absence here threw + "No wrapToolsWithOutputBudget export is defined on the ../pi.js mock" and made + BOTH test-mode-forcing cases below permanently red — a dead guard on the + invariant that testMode never issues real AI calls. Identity stub: the real + function returns tools byte-identical for a null budget, and these cases assert + model/provider resolution, not output budgeting. + + Not caught by `pnpm check:mock-completeness` — that gate only inspects the + @fusion/engine and @fusion/dashboard BARRELS in cli/dashboard test dirs, never a + relative intra-package mock like this one. + */ + wrapToolsWithOutputBudget: vi.fn((tools: unknown[]) => tools), describeModel: vi.fn().mockReturnValue("mock-provider/mock-model"), formatModelMarkerDetails: vi.fn((model: string, thinking?: string | null, annotations: string[] = []) => { const suffixes = [thinking ? `thinking effort: ${thinking}` : "", ...annotations].filter(Boolean);