From 60054aab0a773beb4ab958fcfe5ffb66a27011ab Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 10:59:57 -0700 Subject: [PATCH] test(engine): live-PG evidence of an inert conversion at the CALL SITE (#2795) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What One new live-PostgreSQL E2E suite, 4 tests. **No production file is touched** — evidence, per the E2E worker's remit. `packages/engine/src/__tests__/workflow-file-scope-lease-caller-gap-live-e2e.pg.test.ts` ## Why this is a different finding, not a sixth of the same one #2789/#2791/#2792/#2793/#2794 all concern **one** mechanism: a site resolves the workflow synchronously and silently gets the default board. This is a **second** mechanism, and neither the lifecycle-column census nor the sync-resolver allow-list can see it. `shouldHoldActiveFileScopeLease` was converted by turning its two role questions into optional parameters with literal defaults: ```ts const isWipColumn = options?.isWipColumn ?? task.column === "in-progress"; const isReviewColumn = options?.isReviewColumn ?? task.column === "in-review"; ``` A caller that resolved the traits passes the answer; a caller that has not gets exactly the pre-conversion behaviour. That is a deliberate migration device and the source says so — correctly marked `DELIBERATE-LITERAL`. **But the migration was only half made:** | call site | passes the resolved answer? | |---|---| | `scheduler.ts:1986` | ✅ `{ isWipColumn: true }` | | `scheduler.ts:2006` | ✅ `{ isReviewColumn: true }` | | `self-healing.ts:4525` | ❌ neither | | `self-healing.ts:5443` | ❌ neither | So the same predicate is right on the scheduler's path and wrong on self-healing's. 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 identical in shape: a stale-lease reconciler concludes a live blocker holds no lease and proceeds to clear state the scheduler would have honoured. ### Why the existing instruments are blind to it **There is no column literal at the self-healing call sites.** The literal lives inside the callee's default, one function away — and there it is correct, because for an unconverted caller it *is* the intended behaviour. A census counting `=== "in-progress"` occurrences sees the callee's two (properly marked) and nothing at all at the call sites. The conversion reads as complete from every angle except running it. This generalizes: **any conversion that migrates behaviour behind an optional parameter leaves a residue the census scores as done.** Worth a sweep for the same shape elsewhere — `agent-assignment.ts`'s `activeColumns?` and `restart-recovery-coordinator.ts`'s `isReviewColumn?` are the same pattern; I have not checked whether their callers supply them. ## Scope, stated honestly Three cases are driven end to end: real persisted rows from a live store, the real exported predicate, both call shapes. The **call-site fact is asserted against source text, not driven** — reaching those sites needs the full dependency-lease reconcile harness, which I did not build. The last case reads the file and says so in its own comment rather than dressing it up as an end-to-end result. It doubles as an alarm: when those call sites are converted it fails and points at the three cases above, which describe exactly what changes. ## Mutation-verified Flipping the callee's default from `"in-progress"` to `"building"`: | case | result | |---|---| | CONTROL (default board, no options) | **fails** | | CHARACTERIZATION (renamed board, no options) | **fails** | | BOUND (renamed board, option passed) | passes — correct, the option overrides the default | | SOURCE-LEVEL | passes — correct, it is a source assertion | The two default-dependent cases bind to the default; the bound case proves the override; nothing passes for the wrong reason. ## Not done, and why **No fix.** Passing the resolved answers at the two self-healing sites requires resolving each blocker's column traits there — an async resolution inside a reconcile path that already holds locks, and `self-healing.ts` is another worker's file. Flagging with a differential that says exactly what the fix should make true. ## Verification - new suite — **4/4 passed**, mutation matrix above - full live-PG E2E surface — **137/137 passed** (133 on main + 4) - `pnpm lint` — clean 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) --- ...scope-lease-caller-gap-live-e2e.pg.test.ts | 153 ++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 packages/engine/src/__tests__/workflow-file-scope-lease-caller-gap-live-e2e.pg.test.ts 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 new file mode 100644 index 0000000000..c025172471 --- /dev/null +++ b/packages/engine/src/__tests__/workflow-file-scope-lease-caller-gap-live-e2e.pg.test.ts @@ -0,0 +1,153 @@ +/* +FNXC:OverlapScheduling 2026-07-31-03:15 (E2E evidence — a SECOND inert-conversion mechanism, at the call site): + +The previous five files in this series (#2789, #2791, #2792, #2793, #2794) all concern one mechanism: +a site resolves the workflow synchronously and silently gets the default board. This file is about a +different one, and neither the lifecycle-column census nor the sync-resolver allow-list can see it. + +`shouldHoldActiveFileScopeLease` was converted by turning its two role questions into OPTIONAL +parameters with literal defaults: + + const isWipColumn = options?.isWipColumn ?? task.column === "in-progress"; + const isReviewColumn = options?.isReviewColumn ?? task.column === "in-review"; + +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: + + 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 +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. + +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 +an unconverted caller they ARE the intended behaviour) and nothing at all at the call sites. 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. + +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. +*/ +import { beforeAll, beforeEach, afterEach, afterAll, expect, it } from "vitest"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import "@fusion/core"; // registers the built-in column traits +import type { Task, TaskStore } from "@fusion/core"; + +import { + pgDescribe, + createSharedPgTaskStoreTestHarness, + type SharedPgTaskStoreHarness, +} from "../../../core/src/__test-utils__/pg-test-harness.js"; + +import { shouldHoldActiveFileScopeLease } from "../scheduler.js"; +import { DEFAULT_VOCAB, RENAMED_VOCAB, lifecycleIr, type Vocabulary } from "./_workflow-vocabulary-fixture.js"; + +pgDescribe("file-scope lease: the converted call site against the unconverted one", () => { + const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({ + prefix: "fusion_lease_gap", + }); + + beforeAll(h.beforeAll); + afterAll(h.afterAll); + beforeEach(async () => { await h.beforeEach(); }); + afterEach(async () => { await h.afterEach(); }); + + /** A real persisted card parked in its workflow's WIP column, with a worktree and no unmet + * dependencies — i.e. a card that genuinely holds an active lease. */ + async function wipCard(store: TaskStore, v: Vocabulary, key: string): Promise { + const created = await store.createWorkflowDefinition({ + name: `Lease ${key}`, + kind: "workflow", + ir: lifecycleIr(v, `custom:${key}`), + } as never); + const workflowId = (created as { id: string }).id; + + const task = await store.createTask({ description: `lease probe ${key}` }); + await store.writeTaskWorkflowSelection(task.id, workflowId, []); + store.taskCache.delete(task.id); + await store.updateTask(task.id, { worktree: `/tmp/does-not-need-to-exist/${key}` }); + await store.moveTask(task.id, v.wip as never, { recoveryRehome: true } as never); + + store.taskCache.delete(task.id); + const row = await store.getTask(task.id); + if (!row) throw new Error("fixture: card not persisted"); + return row; + } + + it("CONTROL — on the DEFAULT board the unconverted call shape is CORRECT", async () => { + /* + The literal defaults are not a bug in themselves: for a card in `in-progress` they give the right + answer, which is exactly why the optional-parameter shape was chosen and why the omission at the + self-healing sites went unnoticed. + */ + const store = h.store(); + const card = await wipCard(store, DEFAULT_VOCAB, "wf-default-lease"); + + expect(card.column).toBe(DEFAULT_VOCAB.wip); // ...which is literally "in-progress" + expect(shouldHoldActiveFileScopeLease(card, [card])).toBe(true); + }); + + it("CHARACTERIZATION — on a RENAMED board the unconverted call shape says NO LEASE", async () => { + /* + Same predicate, same card, no resolved answers supplied — the self-healing call shape. Both + branches fall through and a card actively holding a worktree is reported as holding no lease. + */ + const store = h.store(); + const card = await wipCard(store, RENAMED_VOCAB, "wf-renamed-lease"); + + expect(card.column).toBe(RENAMED_VOCAB.wip); + expect(shouldHoldActiveFileScopeLease(card, [card])).toBe(false); + }); + + it("BOUND — the SAME card holds the lease once the resolved answer is passed", async () => { + /* + The differential that attributes the failure above to the call shape and nothing else: identical + card, identical predicate, one extra argument. This is what the two scheduler call sites do and + the two self-healing call sites do not. + */ + const store = h.store(); + const card = await wipCard(store, RENAMED_VOCAB, "wf-renamed-resolved"); + + expect(shouldHoldActiveFileScopeLease(card, [card], { isWipColumn: true })).toBe(true); + }); + + it("SOURCE-LEVEL — the two self-healing call sites still pass neither option", 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. + */ + const source = readFileSync(join(__dirname, "..", "self-healing.ts"), "utf8"); + + const callSites = source.split("shouldHoldActiveFileScopeLease(").slice(1); + expect(callSites.length).toBe(2); + + 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. */ + const optionsWindow = site.slice(0, site.indexOf("})")); + expect(optionsWindow).not.toContain("isWipColumn"); + expect(optionsWindow).not.toContain("isReviewColumn"); + } + }); +});