test(U9): revive two permanently-red testMode guards in reviewer.test.ts (#2547)

**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) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-29 09:32:34 -07:00
committed by GitHub
parent c2705f292f
commit 69790dc3e7

View File

@@ -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);