test(engine): pin #3078's executor-owned skip — it merged without coverage (204 tests passed against the reverted fix) (#3090)
#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.
This commit is contained in:
@@ -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");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user