From 701677a2e5439f5cf127dc21052eb976fd62aeb3 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 04:09:48 -0700 Subject: [PATCH] =?UTF-8?q?test(engine):=20pin=20#3078's=20executor-owned?= =?UTF-8?q?=20skip=20=E2=80=94=20it=20merged=20without=20coverage=20(204?= =?UTF-8?q?=20tests=20passed=20against=20the=20reverted=20fix)=20(#3090)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #3078 merged its conversion of the orphaned-pending-step-results sweep **before this test landed**, so that sweep is on main with no coverage. This closes the gap. ## The gap was measured, not assumed With all three of #3078's conversions reverted, **all 204 self-healing tests still passed**. I had cited that number as verification when I opened it. It was meaningless for that change: every existing test in the file uses `in-review` / `in-progress`, where the literal is correct, so none of them could see the defect. This is the same "a green suite is not coverage" failure I flagged in other PRs today — in my own work, twice. The only reason I caught it is that I finally ran the revert check on myself. ## Two cases - **An executor-owned card in a renamed wip lane is SKIPPED.** Against the pre-#3078 sweep this fails: the sweep reaches a card an executor is actively running and rewrites its `pending` step results to `failed` — the one thing that file's header says it must never do. The liveness triple does not cover it; those legs prove an *in-process* session, and an executor on another node or between session handles is exactly what the column skip is for. - **A genuine orphan on that same renamed board is still recovered** — the skip must narrow, not disable. Passes either way, deliberately. ## What it pins, precisely The **invariant**, not a line. Reverting either single guard still passes, because the page-snapshot check and the fresh-row re-read protect independently. What fails is reverting the sweep's column handling as a whole — which is the condition worth pinning, and matches the project's "fix the invariant, not the repro" rule. ## Still uncovered, said plainly #3078's other two sweeps — worktree-metadata liveness and agent-link drift — have no dedicated case. The orphaned-step-results sweep got the test first because it is the one that can corrupt a live executor's state. The other two remain honest debt rather than implied coverage. ## Verification `self-healing-orphaned-pending-step-results` **10 passed** on current main · full self-healing suites 204 · `pnpm test:gate` 161 + 13 + 487 + 71 · lint — green. --- ...ling-orphaned-pending-step-results.test.ts | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/packages/engine/src/__tests__/self-healing-orphaned-pending-step-results.test.ts b/packages/engine/src/__tests__/self-healing-orphaned-pending-step-results.test.ts index 9dc140774f..94201be88e 100644 --- a/packages/engine/src/__tests__/self-healing-orphaned-pending-step-results.test.ts +++ b/packages/engine/src/__tests__/self-healing-orphaned-pending-step-results.test.ts @@ -256,4 +256,56 @@ describe("review-gate lease liveness (in-review gates)", () => { const after = await store.getTask("FN-NO-OWNER"); expect(after?.workflowStepResults?.[0]?.status).toBe("failed"); }); + + /* + FNXC:WorkflowResolvedColumns 2026-07-31-17:40: + THE EXECUTOR-OWNED SKIP IS A WIP-ROLE QUESTION, and it was asked with the id `in-progress`. + + On a board whose execution lane is named anything else, the skip never fired: this sweep reached a + card an executor is actively running and rewrote its `pending` step results to `failed` — the one + thing the header above says it must never do. The liveness triple does not save it either, because + those legs prove an in-process session, and an executor on another node or between session handles + is exactly the case the column skip exists to cover. + + Every other test in this file uses `in-review`/`in-progress`, where the literal is correct — which + is why 204 self-healing tests passed with this conversion reverted. + */ + const RENAMED_WIP_IR = { + version: "v2", id: "custom:renamed", nodes: [], edges: [], + columns: [ + { id: "building", name: "building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "checking", name: "checking", traits: [{ trait: "merge" }] }, + ], + }; + + it("skips an executor-owned card resting in a RENAMED wip lane", async () => { + const executing = task("FN-WIP", { + column: "building", + workflowStepResults: [stepResult({ status: "pending", workflowStepId: "code-review", workflowStepName: "Code Review" })], + }); + const store = storeFor([executing]); + (store as unknown as { listWorkflowDefinitions: unknown }).listWorkflowDefinitions = + vi.fn(async () => [{ ir: RENAMED_WIP_IR }]); + const manager = new SelfHealingManager(store, { rootDir: "/repo" }); + + expect(await manager.reconcileOrphanedPendingStepResults()).toBe(0); + /* The executor's lease is untouched. */ + expect((await store.getTask("FN-WIP"))?.workflowStepResults?.[0]?.status).toBe("pending"); + expect(recordRunAuditEventMock).not.toHaveBeenCalled(); + }); + + it("still recovers a genuine orphan on that same renamed board", async () => { + /* The skip must narrow, not disable: a card in the REVIEW lane is not executor-owned. */ + const stranded = task("FN-REV", { + column: "checking", + workflowStepResults: [stepResult({ status: "pending", workflowStepId: "code-review", workflowStepName: "Code Review" })], + }); + const store = storeFor([stranded]); + (store as unknown as { listWorkflowDefinitions: unknown }).listWorkflowDefinitions = + vi.fn(async () => [{ ir: RENAMED_WIP_IR }]); + const manager = new SelfHealingManager(store, { rootDir: "/repo" }); + + expect(await manager.reconcileOrphanedPendingStepResults()).toBe(1); + expect((await store.getTask("FN-REV"))?.workflowStepResults?.[0]?.status).toBe("failed"); + }); });