test(U9): gate the review-lane leniency guard (prose rejection never becomes APPROVE) (#2564)
**U9, PR9.** Config only — one line added to the `engine-core` allow-list, plus its justification. The merge half of U9's safeguards now fires in blocking CI (#2526). **This is the review half, none of which did.** ## What's admitted `workflow-step-verdict-parsing.test.ts` holds `proseSignalsClearApproval`'s leniency guard: **a prose REJECTION must never be promoted to APPROVE.** Removing the REVISE/RETHINK/negated-approval disqualifiers fails **11** of its cases. This is a **fail-open** defect on the path to an irreversible merge — a review saying *"looks good, but this must be fixed before merging"* would read as an approval. That belongs in the gate, not in a non-blocking run hours after the merge. Measured across 3 runs: | | Files | Tests | Wall | |---|---|---|---| | before | 19 | 414 | 6.16 / 6.25 / 6.21s | | after | 20 | 482 | 6.28 / 6.55 / 6.33s | **+~0.2s** against a ~60s ceiling. **Gate fires — verified, not assumed:** removing the disqualifiers → `pnpm test:gate` exits 1 (11 failed / 471 passed); restored → exits 0. ## What is deliberately NOT admitted, and why `reviewer.test.ts` holds the sibling family — *"a provider outage is not a review verdict"*. I verified by mutation that it genuinely guards this: removing the escalation branch fails **5** tests covering "escalates a rate limit as `ReviewerProviderError` instead of an `UNAVAILABLE` verdict", "does not burn the reviewer fallback retry budget on a provider outage", and "escalates as transient once the network retry budget is exhausted". That budget exists to bound *bad reviews*; spending it on an outage fails tasks that have nothing wrong with them. It is green in `engine-default` but **fails 72 cases under `engine-core`**, because that project resolves `@fusion/core` through the **reduced** `index.gate.ts` barrel/bundle and the suite reaches exports it does not carry (`__vite_ssr_import_0__.has…` TypeError). Admitting it would mean widening the gate barrel — which trades away the bundle's entire reason for existing (FN-7669 measured the barrel import phase as the gate's dominant wall-time cost). **I tried it, measured the 72 failures, and backed it out** rather than either shipping a red gate or — the tempting version — loosening the test until it passed under the reduced barrel. The reason is recorded in the config next to the allow-list so the next person doesn't rediscover it. Widening the barrel for this suite is a real option, but it is a gate-performance decision with its own measurement, not a side effect of a test-coverage PR. ## Review-lane characterization status By-name coverage search performed first in every case, per the lesson from #2520: | Invariant | Verdict | |---|---| | FN-8492 orphaned pending results rewritten, never deleted | covered (NEW=2) | | FN-7720 bypass writes `skipped` | covered (NEW=1) | | FN-7720 bypass never fabricates a verdict | **was vacuous** — fixed in #2541 | | Provider outage escalates, never becomes a verdict | covered (NEW=5), outside the gate — see above | | Prose rejection never promoted to APPROVE | covered (NEW=11) — **now gated** | | testMode never issues real AI calls | **was permanently red** — fixed in #2547 | Still uncharacterized, stated rather than implied: branch-group member integration and promotion sequencing (the FN-5819 scoped exception). 🤖 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:
@@ -193,6 +193,13 @@ export default defineConfig({
|
||||
*/
|
||||
"src/__tests__/merge-single-flight-invariant.test.ts",
|
||||
/*
|
||||
FNXC:EngineTests 2026-07-29-12:40 (U9 review lane):
|
||||
The merge half of U9's safeguards fires in blocking CI; this is the review half, which did not. `workflow-step-verdict-parsing.test.ts` holds the leniency guard: a prose REJECTION must never be promoted to APPROVE. Removing the REVISE/RETHINK/negated-approval disqualifiers in `proseSignalsClearApproval` fails 11 of its cases — a fail-OPEN defect on the path to an irreversible merge, so it belongs in the gate rather than a non-blocking run hours later. Deterministic, pure parser assertions, no mocks/git/network.
|
||||
|
||||
NOT admitted: `reviewer.test.ts`, which holds the sibling "a provider outage is not a review verdict" family. It is green in engine-default but fails 72 cases under engine-core, because that project resolves @fusion/core through the REDUCED `index.gate.ts` barrel/bundle and the suite reaches exports it does not carry (`__vite_ssr_import_0__.has…` TypeError). Admitting it needs the gate barrel widened, which trades away the bundle's whole reason for existing; left outside deliberately rather than papered over.
|
||||
*/
|
||||
"src/__tests__/workflow-step-verdict-parsing.test.ts",
|
||||
/*
|
||||
FNXC:EngineTests 2026-07-28-10:20:
|
||||
Gate admission evidence (U9): this pins which authority actually decides merge-region policy — the built-in IR declares `merge-retry.maxAttempts` / `manual-merge-hold.release` that no handler reads, while the live budgets sit in `settings.maxAutoMergeRetries` and `ProjectEngine.MAX_AUTO_MERGE_TRANSIENT_RETRIES`. Merge is where irreversible work happens, and the drift it guards is SILENT: a handler-only edit can quietly make the dead IR config live (or move the live budget) with no other test failing. Outside the gate the ratchet cannot fire on the defect it exists for. Deterministic and pure — no git subprocesses, no timers, no network, no store; 3 ms of assertions.
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user