test(engine): un-red the reliability-interactions tier (5 → 0) + a guard that could not fire (#2707)

## How this was found

Full-suite **shard 3/4** reports no `Tests N failed` summary at all —
its log ends mid-`@fusion/engine [1/2]` on a watchdog heartbeat, so the
red reads as infrastructure noise. It is not.

Facts that ruled out the infrastructure explanations, before touching
any test:

- watchdog budget is **1500s**; the engine slice died after **~6.5–10.5
min** (varies run to run) — not a timeout, and not a fixed one
- job `timeout-minutes: 60`, ran **9.7 min** — not the job timeout
- `concurrency.cancel-in-progress: false` — not cancellation
- annotation says **exit code 1**, not 137 — not an OOM kill
- across **four consecutive runs** the last file named is always
`reliability-interactions/explicit-duplicate-marker-sweep.test.ts`

Running that file locally reproduces real failures. The summary line is
simply missing from the CI log's final chunk (the last ~5s of output
never appears), which is what disguised a normal test failure as a
crash.

## Four causes, none a product defect

| File | Failures | Cause |
|---|---:|---|
| `explicit-duplicate-marker-sweep` | 2 | fixture seeds `column:
"triage"` |
| `starved-refinement-x-approval-gate` | 1 | same |
| `starved-refinement-x-triage-poll` | 1 | same |
| `executor-pending-review-skip-retry` | 1 | review handoff now passes
move **options** |

`triage` is no longer declared on any workflow post-U11, and these
sweeps filter by **role** — so cards seeded there carried no intake role
and the sweeps reported 0.

## Measured

| Check | Result |
|---|---|
| `reliability-interactions` | 5 failed → **0** (103 files, **530
passed**) |
| `pnpm test:gate` | **726 passed** |
| `pnpm lint`, engine `tsc --noEmit` | clean |

Census unchanged — test files only.

## The real find: "honors the disable flag" could not fail

Forcing `enabled = true` in `resolveExplicitDuplicateMarkerTasks` — i.e.
making the sweep **ignore the disable flag entirely** — left all 16
cases **green**.

The fixture never set `triageDuplicateResolution`, so the sweep had no
resolution action to take and the duplicate survived whether the flag
was honoured or ignored. The assertion held for a reason unrelated to
the test's name. It had been red only because of the column literal,
which would have made "fix the literal, go green" a repair that left a
guard guarding nothing.

Setting the resolution mode makes that same mutation delete the
duplicate and the case fail. Verified both directions:

| | mutation applied |
|---|---|
| before | 16 passed — **guard cannot fire** |
| after | **1 failed** / 15 passed |

Recorded in-file with the measurement, so nobody strips the setting back
out as redundant.

## Two assertions strengthened rather than relaxed

- The disable-flag and failed-delete cases now assert the column is
**unchanged from a value read before the sweep**, rather than equal to a
literal. They cannot pass because a seed happened to land where the
assertion looked, and they survive the next column rename. The
invariants those cases own are "the flag stops the sweep" and "the task
whose delete threw survives" — the column id was always incidental.
- The handoff move asserts its **provenance** (`nodeId:
"review-pending-handoff"`, `preserveProgress: true`) instead of the
`expect.anything()` its siblings in that file use, so a move to the same
column by another path cannot satisfy it.

## Still open for whoever owns CI

Shard 3/4's log loses its final chunk, which is why a plain test failure
presented as a crash and stayed unexplained across at least four runs.
Anyone triaging full-suite from shard conclusions alone will keep
mis-reading this one; the failure has to be reproduced locally to be
visible.
This commit is contained in:
gsxdsm
2026-07-30 03:59:13 -07:00
committed by GitHub
parent 339f6e7830
commit a037ca93c7
4 changed files with 53 additions and 9 deletions

View File

@@ -144,7 +144,17 @@ describe("reliability interactions: FN-5436 executor pending-review skip", () =>
error: "executor-exit-while-review-pending",
});
expect(store.updateTask).not.toHaveBeenCalledWith("FN-5436-RI-C", expect.objectContaining({ taskDoneRetryCount: 3 }));
expect(store.moveTask).toHaveBeenCalledWith("FN-5436-RI-C", "in-review");
/*
FNXC:WorkflowResolvedColumns 2026-07-30-22:00:
The review handoff now carries move options, so the two-argument form no longer matches. Asserting
the PROVENANCE rather than `expect.anything()` (which the sibling assertions in this file use):
this pins that the move came from the review-pending-handoff node with progress preserved, so a
move made by some other path to the same destination cannot satisfy it.
*/
expect(store.moveTask).toHaveBeenCalledWith("FN-5436-RI-C", "in-review", expect.objectContaining({
preserveProgress: true,
workflowMoveMetadata: expect.objectContaining({ nodeId: "review-pending-handoff" }),
}));
});
it("FN-5436 composition: recoverApprovedStepsOnResume leaves pending-review skip disabled after approval resolves step", async () => {

View File

@@ -201,15 +201,34 @@ const canRun = hasGit && hasPg;
});
it("honors the disable flag", async () => {
const fx = await makeReliabilityFixture({ settings: { resolveExplicitDuplicateMarkerEnabled: false, taskPrefix: "FN" } as never });
/*
FNXC:WorkflowResolvedColumns 2026-07-30-21:45 (this guard could not fire):
`triageDuplicateResolution: "delete"` is REQUIRED for this case to mean anything. Without it the
sweep has no resolution action to take, so the duplicate survives whether the disable flag is
honoured or ignored — the assertion held for a reason unrelated to the flag. Proven by mutation:
forcing `enabled = true` in resolveExplicitDuplicateMarkerTasks left all 16 cases green. With the
resolution mode set, that same mutation deletes the duplicate and this case fails.
*/
const fx = await makeReliabilityFixture({ settings: { resolveExplicitDuplicateMarkerEnabled: false, taskPrefix: "FN", triageDuplicateResolution: "delete" } as never });
fixtures.push(fx);
const canonical = await fx.store.createTask({ title: "Canonical", description: "canonical", column: "todo" });
const duplicate = await createPromptTask(fx, { id: "FN-5303", column: "triage", prompt: duplicateStub(canonical.id) });
const duplicate = await createPromptTask(fx, { id: "FN-5303", column: "todo", prompt: duplicateStub(canonical.id) });
/*
FNXC:WorkflowResolvedColumns 2026-07-30-21:30:
Assert the card DID NOT MOVE, rather than that it sits in a named column. The invariant this
case owns is "the disable flag stops the sweep"; the column id was incidental, and pinning
`triage` broke it post-U11 (that column is no longer declared, so the seed lands in the merged
Planning column `todo`). Reading the column back BEFORE the sweep also means this cannot pass
because the seed happened to land where the assertion looked.
*/
const columnBefore = (await fx.store.getTask(duplicate.id)).column;
await (fx.manager as any).resolveExplicitDuplicateMarkerTasks();
expect((await fx.store.getTask(duplicate.id)).column).toBe("triage");
// Survives at all (the flag blocked the delete) AND did not move.
expect((await fx.store.getTask(duplicate.id)).column).toBe(columnBefore);
});
it("caps work at 50 tasks per sweep", async () => {
@@ -241,8 +260,11 @@ const canRun = hasGit && hasPg;
fixtures.push(fx);
const canonical = await fx.store.createTask({ title: "Canonical", description: "canonical", column: "todo" });
const first = await createPromptTask(fx, { id: "FN-5304", column: "triage", prompt: duplicateStub(canonical.id) });
const second = await createPromptTask(fx, { id: "FN-5305", column: "triage", prompt: duplicateStub(canonical.id) });
const first = await createPromptTask(fx, { id: "FN-5304", column: "todo", prompt: duplicateStub(canonical.id) });
const second = await createPromptTask(fx, { id: "FN-5305", column: "todo", prompt: duplicateStub(canonical.id) });
// Same reason as the disable-flag case: what matters is that `first` SURVIVES the thrown
// delete, not which column it sits in.
const firstColumnBefore = (await fx.store.getTask(first.id)).column;
const originalDeleteTask = fx.store.deleteTask.bind(fx.store);
const deleteSpy = vi.spyOn(fx.store, "deleteTask").mockImplementation(async (taskId, options) => {
@@ -254,7 +276,7 @@ const canRun = hasGit && hasPg;
expect(await (fx.manager as any).resolveExplicitDuplicateMarkerTasks()).toBe(1);
expect(deleteSpy).toHaveBeenCalled();
expect((await fx.store.getTask(first.id)).column).toBe("triage");
expect((await fx.store.getTask(first.id)).column).toBe(firstColumnBefore);
await expect(fx.store.getTask(second.id)).rejects.toThrow(`Task ${second.id} not found`);
});
});

View File

@@ -13,7 +13,13 @@ function task(overrides: Partial<Task> & Pick<Task, "id">): Task {
title: overrides.id,
description: overrides.id,
priority: "normal",
column: "triage",
/*
FNXC:WorkflowResolvedColumns 2026-07-30-22:00:
The INTAKE column post-U11 is `todo` (the merged Planning column). `triage` is no longer declared
on any workflow, and the starved-refinement sweep filters by ROLE — so a card seeded in `triage`
carried no intake role, the filter found no candidates, and the sweep reported 0 escalations.
*/
column: "todo",
dependencies: [],
steps: [],
currentStep: 0,

View File

@@ -13,7 +13,13 @@ function triageTask(overrides: Partial<Task> & Pick<Task, "id">): Task {
title: overrides.id,
description: overrides.id,
priority: "low",
column: "triage",
/*
FNXC:WorkflowResolvedColumns 2026-07-30-22:00:
The INTAKE column post-U11 is `todo` (the merged Planning column). `triage` is no longer declared
on any workflow, and the starved-refinement sweep filters by ROLE — so a card seeded in `triage`
carried no intake role, the filter found no candidates, and the sweep reported 0 escalations.
*/
column: "todo",
dependencies: [],
steps: [],
currentStep: 0,