test(engine): the lease-seam alarm fired downward — re-point it, and close two ways it could pass without a fix (#2987)
## What happened Both of my source-level audits went red on the advance that landed #2975. They are exact counters, not floors, so this is the alarm working **downward** — the direction it was written for. #2975 converted the two self-healing `shouldHoldActiveFileScopeLease` call sites and closed the 2-of-4 seam that `workflow-file-scope-lease-caller-gap-live-e2e.pg.test.ts` was measuring. I judged the conversion real before updating anything. Both sites now derive their answers from `resolveProjectColumnsForRoles(...)` sets the sweep had already resolved a few lines above (`self-healing.ts:4754`, `:5825`) — trait membership, not a literal. So the numbers moved on purpose. ## The part that is not bookkeeping Re-pointing an audit to whatever the code now says is how a guard goes dead. Both assertions could have been satisfied by something that is **not** a fix, so both were tightened: | Way it could pass without a fix | Old assertion | Now | |---|---|---| | `isWipColumn: true` hardcoded at a self-healing site — the original defect wearing the converted call shape, answering "yes" for a blocker resting anywhere | only checked the key was *absent* | requires resolved-set membership: `/isWipColumn:\s*\w+\.has\(\w+\.column\)/` | | a site answering one of the two independent role questions and not the other | `includes(a) \|\| includes(b)` counted it as converted | `&&` | The scheduler's own two sites *do* pass literal `true`, correctly — they have already filtered to a role-resolved bucket, so there the answer is a fact about the loop, not about the card. The form check is scoped to `self-healing.ts` for that reason. ## Mutation evidence Not reasoned — measured. Each mutant applied to `self-healing.ts`, suite re-run, file restored: | Mutant | Result | |---|---| | baseline | 8 passed | | M1 — hardcode `isWipColumn: true` at one site | **1 failed** | | M2 — drop `isReviewColumn` at one site (half-converted) | **2 failed** | | M3 — revert both sites to pre-#2975 | **2 failed** | M2 is the one that justifies the `&&`: re-running it with the counter reverted to `||` leaves the file **green (4/4 passing)**. The old counter provably could not distinguish a closed seam from a half-closed one — the exact blind spot this file exists to remove. ## Verification `test:gate` exit 0 · live-PG E2E surface **171/171** · lifecycle-column census exit 0 · FNXC date ratchet exit 0 · `pnpm lint` clean. Production files untouched (`git status` clean on `self-healing.ts` after every mutant). ## Scope The other measured seam, `evaluateParkedAgentTaskLink`, is **unchanged at 2-of-6** — four callers still omit the resolved columns, so the class is not closed, only one of its two instances is. That remains characterized, not fixed, in the same file; converting those four is the capacity worker's file, not mine. The call-site facts are still asserted against source text rather than driven through the self-healing sweep, which would need the full dependency-lease reconcile harness. That limit was stated in the original file and still is. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated end-to-end workflow checks to verify resolved workflow-role values at active lease call sites. * Strengthened assertions for WIP and review column detection. * Updated audit coverage to reflect conversion of all active-file-scope lease callers. * Preserved tracking for the remaining parked-link integration points. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -14,20 +14,37 @@ parameters with literal defaults:
|
||||
A caller that has resolved the column's traits passes the answer; a caller that has not gets exactly
|
||||
the pre-conversion behaviour. That shape is a deliberate migration device and the source says so.
|
||||
|
||||
But the migration was only half made. Of the four production call sites:
|
||||
The migration was only half made. Of the four production call sites, as this file found them:
|
||||
|
||||
scheduler.ts:1986 passes { isWipColumn: true } -> converted
|
||||
scheduler.ts:2006 passes { isReviewColumn: true } -> converted
|
||||
self-healing.ts:4525 passes NEITHER -> still the literals
|
||||
self-healing.ts:5443 passes NEITHER -> still the literals
|
||||
|
||||
So the same predicate is right on the scheduler's path and wrong on self-healing's, and the harm is
|
||||
So the same predicate was right on the scheduler's path and wrong on self-healing's, and the harm is
|
||||
the one the function's own FNXC note describes: on a renamed board both branches fall through, the
|
||||
predicate returns false for every card, `activeScopes` stays empty, and the dispatch path sees no
|
||||
overlap — two agents editing the same files, which is what the overlap machinery exists to prevent.
|
||||
At the self-healing sites the consequence is narrower but the same shape: a stale-lease reconciler
|
||||
concludes a live blocker holds no lease and proceeds to clear state the scheduler would have honoured.
|
||||
|
||||
FNXC:OverlapScheduling 2026-07-30-23:30 (the alarm fired DOWNWARD — the seam is now 4-of-4):
|
||||
#2975 converted both self-healing sites, and this file's last case failed on the advance that landed
|
||||
it. That is the alarm working in the direction it was written for: the source-level case is a counter,
|
||||
so it fails when someone CLOSES the gap as well as when someone widens it, and whoever closes it is
|
||||
sent here to update the number deliberately.
|
||||
|
||||
Both sites now derive their answers from `resolveProjectColumnsForRoles(...)` sets the sweep had
|
||||
already resolved a few lines above — trait membership, not a literal — so the conversion is real
|
||||
rather than a call-shape that merely looks converted. The last case now asserts that FORM, not just
|
||||
the presence of the two keys: a site that satisfied it by hardcoding `isWipColumn: true` would be the
|
||||
same defect wearing the converted shape, and would restore exactly the pre-#2975 behaviour on any
|
||||
board where the blocker is not actually in a wip column.
|
||||
|
||||
The three behavioural cases below are UNCHANGED and still meaningful: the optional parameters and
|
||||
their literal defaults still exist, so the differential they drive is still the live behaviour of the
|
||||
predicate for any future caller that omits them.
|
||||
|
||||
WHY THIS IS NOT VISIBLE TO THE EXISTING INSTRUMENTS. There is no column literal at the self-healing
|
||||
call sites — the literal lives inside the callee's default, one function away. A census that counts
|
||||
`=== "in-progress"` occurrences sees the callee's two (correctly marked DELIBERATE-LITERAL, since for
|
||||
@@ -35,11 +52,11 @@ an unconverted caller they ARE the intended behaviour) and nothing at all at the
|
||||
conversion therefore reads as complete from every angle except running it.
|
||||
|
||||
SCOPE, STATED HONESTLY. The differential below is driven end to end: real persisted rows from a live
|
||||
PostgreSQL store, the real exported predicate, both call shapes. The CALL-SITE fact — that the two
|
||||
self-healing sites pass neither option — is asserted against source text, not driven through the
|
||||
self-healing sweep, which would need the full dependency-lease reconcile harness. That is a real limit
|
||||
and it is why the last case reads the file rather than pretending otherwise. It doubles as the alarm:
|
||||
when someone converts those call sites, that case fails and points at this file.
|
||||
PostgreSQL store, the real exported predicate, both call shapes. The CALL-SITE fact — what the two
|
||||
self-healing sites pass — is asserted against source text, not driven through the self-healing sweep,
|
||||
which would need the full dependency-lease reconcile harness. That is a real limit and it is why the
|
||||
last case reads the file rather than pretending otherwise. It doubles as the alarm, in both
|
||||
directions: dropping an option fails it, and so does closing the gap.
|
||||
|
||||
LANE. `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is
|
||||
unaffected. Throwaway per-file database; never port 4040.
|
||||
@@ -128,14 +145,21 @@ pgDescribe("file-scope lease: the converted call site against the unconverted on
|
||||
expect(shouldHoldActiveFileScopeLease(card, [card], { isWipColumn: true })).toBe(true);
|
||||
});
|
||||
|
||||
it("SOURCE-LEVEL — the two self-healing call sites still pass neither option", async () => {
|
||||
it("SOURCE-LEVEL — both self-healing call sites now pass both options, RESOLVED not hardcoded", async () => {
|
||||
/*
|
||||
NOT driven through the self-healing sweep, deliberately: reaching those sites needs the full
|
||||
dependency-lease reconcile harness. Asserted against source text instead, and labelled as such
|
||||
rather than dressed up as an end-to-end result.
|
||||
|
||||
Its job is to be an alarm. When those call sites are converted this fails, and whoever converted
|
||||
them is pointed at the three cases above, which describe what changes.
|
||||
Updated on purpose when #2975 closed the gap and this case failed — see the header. The value
|
||||
check is the part that matters now: presence of the two keys alone would be satisfied by
|
||||
`isWipColumn: true`, which is the ORIGINAL defect wearing the converted call shape (it answers
|
||||
"yes" for a blocker resting anywhere, not just in a wip column). Requiring the answer to come from
|
||||
a resolved set keeps the assertion attached to the property that made the fix a fix.
|
||||
|
||||
The scheduler's own sites DO pass literal `true`, correctly — they have already filtered to a
|
||||
role-resolved bucket, so the answer is a fact about the loop rather than about the card. The check
|
||||
below is scoped to `self-healing.ts` for exactly that reason.
|
||||
*/
|
||||
const source = readFileSync(join(__dirname, "..", "self-healing.ts"), "utf8");
|
||||
|
||||
@@ -144,10 +168,13 @@ pgDescribe("file-scope lease: the converted call site against the unconverted on
|
||||
|
||||
for (const site of callSites) {
|
||||
/* The options object literal ends at the first `})` that closes the call. Bounding the window
|
||||
keeps a later, unrelated `isWipColumn` in the file from masking an omission here. */
|
||||
keeps a later, unrelated `isWipColumn` in the file from reading as this site's argument. */
|
||||
const optionsWindow = site.slice(0, site.indexOf("})"));
|
||||
expect(optionsWindow).not.toContain("isWipColumn");
|
||||
expect(optionsWindow).not.toContain("isReviewColumn");
|
||||
|
||||
/* `<someSet>.has(<card>.column)` — membership in a set the sweep resolved by trait. Written as
|
||||
a shape rather than a fixed identifier so renaming the local set does not fail this. */
|
||||
expect(optionsWindow).toMatch(/isWipColumn:\s*\w+\.has\(\w+\.column\)/);
|
||||
expect(optionsWindow).toMatch(/isReviewColumn:\s*\w+\.has\(\w+\.column\)/);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -11,11 +11,17 @@ no column literal at all — the literal lives one function away. So the census
|
||||
literals (correctly annotated) and sees nothing at the call sites, and the conversion reads as
|
||||
complete from every angle except running it.
|
||||
|
||||
TWO MEASURED SEAMS, both with a minority of callers converted:
|
||||
TWO MEASURED SEAMS:
|
||||
|
||||
shouldHoldActiveFileScopeLease 2 of 4 call sites pass the resolved answer (#2795)
|
||||
shouldHoldActiveFileScopeLease 4 of 4 call sites pass the resolved answer (#2795, closed by #2975)
|
||||
evaluateParkedAgentTaskLink 2 of 6 call sites pass the resolved answer (this file)
|
||||
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-23:30 (first seam CLOSED; number updated deliberately):
|
||||
The lease seam was 2-of-4 when measured. #2975 converted both self-healing sites, this counter failed
|
||||
on the advance that landed it, and the number below moved on purpose — which is the whole reason the
|
||||
audit asserts an exact count instead of a floor. The parked-link seam is UNCHANGED at 2-of-6, so the
|
||||
class this file exists to measure is not closed; one of its two instances is.
|
||||
|
||||
The second is the more damaging. Its own FNXC note states the consequence exactly: without the
|
||||
resolved columns "the card would be treated as unparked and its live agent link cleared" — a
|
||||
stale-link bug turned into a DROPPED-link bug. The four unconverted callers are the two in
|
||||
@@ -127,7 +133,7 @@ pgDescribe("optional-role-parameter conversions, measured on a live store", () =
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("AUDIT — the call-site split for both measured seams is 2-of-6 and 2-of-4", async () => {
|
||||
it("AUDIT — the call-site split for both measured seams is 2-of-6 and 4-of-4", async () => {
|
||||
/*
|
||||
NOT driven: reaching all six sites needs the heartbeat and self-healing harnesses. Asserted
|
||||
against source text and labelled as such rather than dressed up as an end-to-end result.
|
||||
@@ -152,13 +158,23 @@ pgDescribe("optional-role-parameter conversions, measured on a live store", () =
|
||||
expect(parkedCalls.length).toBe(6);
|
||||
expect(parkedConverted.length).toBe(2);
|
||||
|
||||
/* The lease seam's two SELF-HEALING sites. Its other two are in `scheduler.ts` and were converted
|
||||
from the start, so 2 converted here is the seam at 4-of-4.
|
||||
|
||||
`&&`, not the `||` this replaces. The predicate asks two INDEPENDENT role questions, so a site
|
||||
answering only one is still half-converted — and `||` counted it as converted. Measured, not
|
||||
reasoned: with `||`, deleting one site's `isReviewColumn` leaves this whole file green (4/4
|
||||
passing); with `&&` it fails. An audit that cannot tell a closed seam from a half-closed one is
|
||||
the exact blind spot this file was written to remove. */
|
||||
const leaseCalls = read("self-healing.ts").split("shouldHoldActiveFileScopeLease(").slice(1);
|
||||
const leaseConverted = leaseCalls.filter((s) => {
|
||||
const w = s.slice(0, s.indexOf("})"));
|
||||
return w.includes("isWipColumn") || w.includes("isReviewColumn");
|
||||
return w.includes("isWipColumn") && w.includes("isReviewColumn");
|
||||
});
|
||||
|
||||
expect(leaseCalls.length).toBe(2);
|
||||
expect(leaseConverted.length).toBe(0); // both scheduler sites are converted; both here are not
|
||||
expect(leaseConverted.length).toBe(2); // closed by #2975. The FORM of the answer — resolved set
|
||||
// membership, not a hardcoded `true` — is asserted in
|
||||
// workflow-file-scope-lease-caller-gap-live-e2e.pg.test.ts
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user