From 56b5cdfeed5138972cfb73f216b8b15391b5bd0f Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 05:09:36 -0700 Subject: [PATCH] test(notifications): cover the wedge-episode renamed-lane clear that main's conversion lacked (#3096) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Rebased onto current `main`, and it shrank to a test.** Was "close the wedge-episode race, then resolve its lanes." ## What happened Another worker landed **both halves of this PR independently** while it was open. Rebasing showed their versions are better, so I took theirs and dropped mine: - **The serialisation** — theirs is `enqueueWedgeHandling(taskId, run)`, a general callback; mine was wedge-specific. - **The conversion** — theirs is **project-union membership** over the four roles; mine was first-match-per-role via `resolveLifecycleColumns`. Membership is correct: more than one lane can fill a role on a renamed board, and first-match silently ignores the rest. My rebased branch initially compiled to a **duplicate `wedgeHandlingChains` field and duplicate method** — caught by `tsc`, removed. Nothing of my implementation survives, and it shouldn't. ## What's left is worth landing Their conversion has **no renamed-board test**. This adds one. A card recovering into a renamed hold lane must **clear** its episode. Asserted through the *second* notification, because a stale active episode also **refuses the next genuine wedge its claim** — so the visible symptom is a real wedge going unannounced, not merely a stale alert. The fixture needed `listWorkflowDefinitions`: `resolveProjectColumnsForRoles` unions across the project's workflows, so the per-task selection readers alone leave it resolving nothing and the test would pass for the wrong reason. That's how I found the mismatch — my original test failed against their implementation. ## Verification - Green as written against **their** implementation - **Revert-proof against theirs:** restoring the four literals fails it — 1 delivered, 2 expected - 7 notification suites — **80 green**; `tsc` clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit * **Bug Fixes** * Fixed wedge notifications so they can trigger again after a task recovers into a renamed workflow’s hold lane. * **Tests** * Added regression coverage confirming that recovered tasks correctly clear their wedge state and support subsequent notifications. Co-authored-by: Claude Opus 5 (1M context) --- .../__tests__/task-wedge-notification.test.ts | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/packages/engine/src/notification/__tests__/task-wedge-notification.test.ts b/packages/engine/src/notification/__tests__/task-wedge-notification.test.ts index 0b4d5786d5..29b28e8ca2 100644 --- a/packages/engine/src/notification/__tests__/task-wedge-notification.test.ts +++ b/packages/engine/src/notification/__tests__/task-wedge-notification.test.ts @@ -67,6 +67,53 @@ describe("task wedge notifications", () => { await service.stop(); }); + /* + FNXC:WorkflowResolvedColumns 2026-07-31-18:50 (fleet): + Clearing a wedge episode asks "has the card moved on?", which was four column literals. On a renamed + board none matched, so a RECOVERED card never cleared its episode and the operator kept an alert for + work that had already progressed — and, because the stale episode stays active, the NEXT genuine wedge + is refused its claim and never announced at all. + + The board below recovers into `backlog` (hold). The assertion is the second message: with the literals + it never arrives, because the first episode was never resolved. + */ + it("clears a wedge episode when the card recovers into a RENAMED hold lane", async () => { + const RENAMED_IR = { + version: "v2", + name: "renamed-notify", + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "hold" }] }, + { id: "building", name: "Building", traits: [{ trait: "wip" }] }, + { id: "signoff", name: "Signoff", traits: [{ trait: "merge" }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], + nodes: [], + edges: [], + }; + const { store, service, sendMessageOnce, task } = fixture(); + /* `resolveProjectColumnsForRoles` unions lanes across the PROJECT's workflows, so the fake needs + `listWorkflowDefinitions` — the per-task selection readers alone leave it resolving nothing. */ + Object.assign(store, { + getTaskWorkflowSelection: () => ({ workflowId: "custom:renamed", stepIds: [] }), + getTaskWorkflowSelectionAsync: async () => ({ workflowId: "custom:renamed", stepIds: [] }), + getWorkflowDefinition: async () => ({ ir: RENAMED_IR }), + listWorkflowDefinitions: async () => [{ id: "custom:renamed", ir: RENAMED_IR }], + }); + + await service.start(); + store.emit(task({ column: "signoff" })); + await vi.waitFor(() => expect(sendMessageOnce).toHaveBeenCalledTimes(1)); + + // Recovered into the board's own hold lane — the episode must close. + /* status stays "failed" ON PURPOSE: the status disjunct (`status !== "failed"`) would otherwise + satisfy hasProgressed on its own and the column term would never decide anything. */ + store.emit(task({ status: "failed", error: undefined, column: "backlog", updatedAt: "2026-07-22T12:02:00.000Z" })); + // Wedged again: only claimable if the previous episode actually cleared. + store.emit(task({ column: "signoff", updatedAt: "2026-07-22T12:03:00.000Z" })); + await vi.waitFor(() => expect(sendMessageOnce).toHaveBeenCalledTimes(2)); + await service.stop(); + }); + it("does not re-deliver an unchanged durable episode after service restart", async () => { const { store, service, sendNotification, sendMessageOnce, task } = fixture(); await service.start();