From fb53a96eaa574df9c31f1d3bf98ccaaada317aaf Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 23:38:10 -0700 Subject: [PATCH] =?UTF-8?q?test(engine):=20the=20lease-seam=20alarm=20fire?= =?UTF-8?q?d=20downward=20=E2=80=94=20re-point=20it,=20and=20close=20two?= =?UTF-8?q?=20ways=20it=20could=20pass=20without=20a=20fix=20(#2987)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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. ## 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. --- ...scope-lease-caller-gap-live-e2e.pg.test.ts | 53 ++++++++++++++----- ...ole-param-caller-audit-live-e2e.pg.test.ts | 26 +++++++-- 2 files changed, 61 insertions(+), 18 deletions(-) diff --git a/packages/engine/src/__tests__/workflow-file-scope-lease-caller-gap-live-e2e.pg.test.ts b/packages/engine/src/__tests__/workflow-file-scope-lease-caller-gap-live-e2e.pg.test.ts index c025172471..e49f5c70b7 100644 --- a/packages/engine/src/__tests__/workflow-file-scope-lease-caller-gap-live-e2e.pg.test.ts +++ b/packages/engine/src/__tests__/workflow-file-scope-lease-caller-gap-live-e2e.pg.test.ts @@ -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"); + + /* `.has(.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\)/); } }); }); diff --git a/packages/engine/src/__tests__/workflow-optional-role-param-caller-audit-live-e2e.pg.test.ts b/packages/engine/src/__tests__/workflow-optional-role-param-caller-audit-live-e2e.pg.test.ts index 48f5cbe2ba..6c5a472367 100644 --- a/packages/engine/src/__tests__/workflow-optional-role-param-caller-audit-live-e2e.pg.test.ts +++ b/packages/engine/src/__tests__/workflow-optional-role-param-caller-audit-live-e2e.pg.test.ts @@ -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 }); });