root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

test(engine): audit the auto-heal review-lane call sites (a DELIBERATE-LITERAL note that is not true) (#2802)

Browse Source 
## What

One new source-level audit test, 3 cases. **No production file is
touched.** Fourth instance of the optional-role-parameter class (#2795,
#2798, #2799) — and the only one so far where the source **annotation
asserts the opposite of the fact**.


`packages/engine/src/__tests__/auto-heal-review-lane-callsite-audit.test.ts`

## The finding

`project-engine.ts`'s `hasAutoHealableVerificationBufferFailure` takes
the review-lane answer as an optional parameter defaulting to
`task.column === "in-review"`. Its `DELIBERATE-LITERAL` note says:

> "Both call sites pass the resolved answer; the default exists so an
unconverted caller keeps exactly today's behaviour rather than silently
changing meaning."

**There are three call sites, not two:**

| site | passes the resolved lane? |
|---|---|
| `canMergeTask:2657` (threads its own param) | ✅ |
| ← `canMergeTask:2903` | ✅ `t.column === reviewLane` |
| ← `canMergeTask:3334` | ✅ `task.column === mergeLoopReviewLane` |
| **merge loop `:3655`** — direct call | ❌ **nothing** |

The note counts the two gating callers and misses the healing one. Note
which half is converted: **the sites deciding whether a card MAY merge
resolve the lane; the site that would RECOVER a stuck card does not.**

The consequence is in the same comment: on a renamed board *"a task
whose merge verification died on a buffer-overflow error was never
auto-healed — it sat retry-exhausted until a human reset it. The failure
is invisible because 'no auto-heal' looks identical to 'nothing to
heal'."*

## Why this is a source audit and not an E2E

The predicate and its caller are both **private methods** of
`ProjectEngine`. The three sibling files in this series each carry a
live behavioural differential because their predicates are exported;
this one cannot, and inventing a mock `ProjectEngine` to assert a
private method would prove only that the mock behaves as written. Stated
plainly rather than substituted for — the finding is a call-site fact,
and a call-site fact is what is asserted.

The third case deliberately pins the **false note itself**, so the audit
fails when someone corrects the sentence — forcing them to also decide
what to do about the third site rather than fixing the prose and leaving
the gap.

## A self-correction, forced by the mutation run

The first version filtered call sites on whether the argument text
contained `isReviewColumn` / `ReviewLane`. Converting the unconverted
site to pass a plain `true` left the count at one and **the suite stayed
green** — the "alarm in both directions" the header claims did not
exist.

Now it counts **arguments** (depth-aware, so nested calls and object
literals do not confuse it), which is the property actually being
asserted and cannot be spelled around. Re-verified:

| state | result |
|---|---|
| main | 3/3 pass |
| site 3655 converted to pass a third argument | **fails** |

Recorded in an FNXC note next to the helper, because the first version
is the exact mistake this series exists to catch.

## Not done, and why

**No fix.** Passing the resolved lane at `:3655` means resolving the
task's review column inside the merge loop; whether that resolution
belongs there or should be hoisted alongside `mergeLoopReviewLane`
(already computed nearby, which is what makes the omission look
accidental rather than considered) is a decision for the file's owner.

## Verification

- new suite — **3/3 passed**, mutation-verified in both directions
- `pnpm lint` — clean
- Unit lane, no PostgreSQL required; adds no gate surface.

🤖 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 11:19:12 -07:00
committed by GitHub
parent ea0af4826f
commit bb8be93c52
1 changed files with 105 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

105
packages/engine/src/__tests__/auto-heal-review-lane-callsite-audit.test.ts Normal file
View File

@@ -0,0 +1,105 @@
/*
FNXC:WorkflowLifecycleColumns 2026-07-31-08:45 (call-site audit — a DELIBERATE-LITERAL note that is not true):
Fourth instance of the optional-role-parameter class (#2795, #2798, #2799), and the only one so far
where the source ANNOTATION asserts the opposite of the fact.
`project-engine.ts`'s `hasAutoHealableVerificationBufferFailure` takes the review-lane answer as an
optional parameter defaulting to `task.column === "in-review"`, and its own note says:
"Both call sites pass the resolved answer; the default exists so an unconverted caller keeps
exactly today's behaviour rather than silently changing meaning."
There are THREE call sites, not two:
canMergeTask:2657 threads its own `isReviewColumn` through CONVERTED
<- canMergeTask:2903 passes `t.column === reviewLane` CONVERTED
<- canMergeTask:3334 passes `task.column === mergeLoopReviewLane` CONVERTED
merge loop:3655 calls it DIRECTLY with no review-lane answer unconverted
So the note counts the two gating callers and misses the healing one. On a renamed board the auto-heal
branch inside the merge loop keys on `in-review`, does not match, and — in the words of the same
comment — "a task whose merge verification died on a buffer-overflow error was never auto-healed; it
sat retry-exhausted until a human reset it. The failure is invisible because 'no auto-heal' looks
identical to 'nothing to heal'."
Note which half is converted: the sites deciding whether a card MAY merge resolve the lane; the site
that would RECOVER a stuck card does not.
WHY THIS FILE IS A SOURCE AUDIT AND NOT AN E2E. The predicate and its caller are both PRIVATE methods
of `ProjectEngine`, so there is no seam to drive them through without standing up the merge loop. The
three sibling files in this series each carry a behavioural differential because their predicates are
exported; this one cannot, and inventing a mock ProjectEngine to assert a private method would prove
only that the mock behaves as written. Stated plainly rather than substituted for — the finding is a
call-site fact, and a call-site fact is what is asserted.
It is an alarm in both directions: a new unconverted caller fails it, and converting site 3655 fails
it too, which is the moment to delete this file and record the fix.
SYNTAX, NOT TEXT. The call sites are found by parsing `project-engine.ts` and inspecting real call
expressions, matching the repo's existing precedent for the same problem
(`core/.../sync-workflow-ir-callsite-allowlist.test.ts`). An audit that reasons about the text
following a name can be broken — or silently misled — by a formatting-only change, which is the
failure mode this whole series is about. The one prose assertion below is unavoidably textual and is
whitespace-normalised for that reason.
*/
import { describe, expect, it } from "vitest";
import { readFileSync } from "node:fs";
import { join } from "node:path";
import ts from "typescript";
const FILE = join(__dirname, "..", "project-engine.ts");
const SOURCE = readFileSync(FILE, "utf8");
const PREDICATE = "hasAutoHealableVerificationBufferFailure";
/** Every call of the predicate, as its argument-expression list. A declaration is not a call
* expression, so it is excluded structurally rather than by guessing at its text. */
function callSites(): ts.NodeArray<ts.Expression>[] {
const sf = ts.createSourceFile(FILE, SOURCE, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS);
const found: ts.NodeArray<ts.Expression>[] = [];
const visit = (node: ts.Node) => {
if (ts.isCallExpression(node)) {
const callee = node.expression;
const name = ts.isPropertyAccessExpression(callee)
? callee.name.text
: ts.isIdentifier(callee) ? callee.text : undefined;
if (name === PREDICATE) found.push(node.arguments);
}
ts.forEachChild(node, visit);
};
ts.forEachChild(sf, visit);
return found;
}
describe("auto-heal review-lane call sites", () => {
it("finds every call site, so the audit cannot pass vacuously", () => {
/* A renamed predicate or a moved file would otherwise leave this suite green while measuring
nothing — the failure mode this whole series is about. */
expect(callSites().length).toBeGreaterThan(0);
});
it("has exactly one call site that does NOT pass the resolved review lane", () => {
/*
ARITY is the property asserted, deliberately. A first version of this file filtered on whether the
argument TEXT mentioned `isReviewColumn` / `ReviewLane`; converting the unconverted site to pass a
plain `true` then left the count at one and the suite stayed green, so the "alarm in both
directions" claimed above did not exist. Argument count cannot be spelled around, and parsing
means a reflowed call cannot be miscounted either.
*/
const unconverted = callSites().filter((args) => args.length < 3);
expect(unconverted).toHaveLength(1);
});
it("the DELIBERATE-LITERAL note still claims both call sites are converted", () => {
/*
Pinned deliberately. The note is the artefact that would stop a reviewer looking further, so the
audit fails when the note is corrected — forcing whoever corrects it to also decide what to do
about the third site, rather than fixing the sentence and leaving the gap.
Whitespace-normalised because the source wraps this sentence mid-phrase; a comment is prose and
has no syntax to parse, so this one assertion is textual by necessity rather than by choice.
*/
expect(SOURCE.replace(/\s+/g, " ")).toContain("Both call sites pass the resolved answer");
});
});