From c643d62e85b4ee40108016e5a662e010e6b57312 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 09:28:41 -0700 Subject: [PATCH] fix(executor): wipDeclared must ask ALL six lifecycle roles, not two (#2777) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What this fixes `resolveResumeLanes` returns `wipDeclared`, which gates whether `routeGraphFailureToExecutionResume` may route a graph failure back into execution resume. Getting it wrong terminalizes tasks on boards that should resume. Two prior versions were wrong, both caught in review rather than by me at write time: **1. Two-state (`lifecycle?.wip !== undefined`)** — greptile P1 on #2760. A v1-upgraded board terminalizes: `synthesizeDefaultColumns` emits `{ id, name: id, traits: [] }`, so *every* role resolves `undefined` even though those columns literally are the legacy lanes. Verified by parsing a real v1 IR. **2. Proxying "synthesized" as "hold and review are both undefined"** — my own fix for (1), and also wrong. I caught this against #2765 rather than shipping it. A **v2** board that declares only `intake` + `complete` has hold and review undefined too, so it would be misread as synthesized and treated as declaring wip when it deliberately does not. The failure mode both versions share: reading a *sample* of the roles and treating the answer as a verdict about the whole IR. #2765 says it directly — an empty result has two meanings, and you cannot tell them apart from a subset. ## The rule ```ts wipDeclared: lifecycle?.wip !== undefined || !declaresAnyLifecycleRole(lifecycle), ``` Three states, asking all six roles: - **wip declared** → true, the board says so. - **some role declared but not wip** → false. A v2 board that omits wip means it; do not resume into a lane it did not define. - **no role declared at all** → true. That is the synthesized/v1-upgraded shape, whose columns *are* the legacy lanes; the pre-existing behaviour is correct there and must not regress. `declaresAnyLifecycleRole` iterates `Object.values(lifecycle)` rather than naming roles, so a seventh role added later is included automatically instead of silently falling into the wrong branch. ## Evidence - `executor-resume-lanes-resolved.test.ts`: **7 passed**, +23 lines covering the v1-synthesized board and the declares-some-but-not-wip board. - **Mutation:** restoring the naive two-state rule → **1 failed / 6 passed**. The added coverage is load-bearing and pins exactly the regression greptile caught. - Gate **732 green** · `pnpm lint` clean · engine `tsc --noEmit` **0 errors**. - Rebased on current main. ## Scope `executor.ts` (+22) and its test (+23). One predicate; no other behavior touched. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- .../executor-resume-lanes-resolved.test.ts | 23 ++++++++++++++++++ packages/engine/src/executor.ts | 24 +++++++++++++++++-- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/packages/engine/src/__tests__/executor-resume-lanes-resolved.test.ts b/packages/engine/src/__tests__/executor-resume-lanes-resolved.test.ts index bf28779f4c..11fa99a39e 100644 --- a/packages/engine/src/__tests__/executor-resume-lanes-resolved.test.ts +++ b/packages/engine/src/__tests__/executor-resume-lanes-resolved.test.ts @@ -152,6 +152,29 @@ describe("resume lanes come from the task's own workflow", () => { }); }); + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-18:10 (the edge my first discriminator missed): + DECLARED AND EMPTY, per #2765. This board is a real v2 workflow that declares intake and complete + but no hold, wip or review. My first rule proxied "synthesized" as "hold and review are both + undefined", so it read this board as v1-upgraded and let the resume router proceed into a wip lane + that does not exist. Checking whether ANY of the six roles resolved is the actual question. + */ + it("treats a v2 board that declares OTHER roles but no wip as genuinely having no wip lane", async () => { + const intakeAndCompleteOnly = { + version: "v2", + id: "WF-sparse", + nodes: [], + edges: [], + columns: [ + { id: "inbox", label: "Inbox", traits: [{ trait: "intake" }] }, + { id: "shipped", label: "Shipped", traits: [{ trait: "complete" }] }, + ], + } as unknown as WorkflowIr; + const h = harness(intakeAndCompleteOnly); + + await expect(h.lanes("FN-1")).resolves.toMatchObject({ wipDeclared: false }); + }); + it("falls back to the legacy trio when no workflow resolves", async () => { // A v1 / column-less workflow has no vocabulary to read, so the legacy names ARE the answer // and the default lineage behaves exactly as before. diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 44ad342b49..119c6b498c 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -1869,6 +1869,27 @@ docs/plans/2026-07-19-002-u5e-remaining-deletions-handoff.md. */ export type GraphCompletionCallback = (info: { modifiedFiles: string[] }) => void; +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-18:10 (tightening my own rule against #2765): +Does this IR express ANY lifecycle intent? #2765 published the general form of the distinction I hit +in the no-wip fix: an empty role result means either DECLARED AND EMPTY (a v2 board the operator +wrote that genuinely lacks the lane — a guard should act on it) or SYNTHESIZED (a v1 graph upgraded +in place; `synthesizeDefaultColumns` emits `{ id, name: id, traits: [] }` for the five default ids, +so every role resolves undefined even though those columns ARE the legacy lanes). + +My first discriminator proxied this with "hold and review are both undefined". That is right for the +boards under test and wrong in general: a v2 workflow declaring, say, intake and complete but no +hold/wip/review would read as SYNTHESIZED and the resume router would proceed into a wip lane the +board does not have — the same failure the guard exists to stop, one case narrower. + +`resolveLifecycleColumns` returns all six roles, so the honest question is whether ANY of them +resolved. Checking two of six was a proxy for that; this checks the thing. +*/ +function declaresAnyLifecycleRole(lifecycle: ReturnType): boolean { + if (!lifecycle) return false; + return Object.values(lifecycle).some((columnId) => columnId !== undefined); +} + export class TaskExecutor { /* FNXC:Workspace 2026-06-21-12:00: @@ -10719,8 +10740,7 @@ export class TaskExecutor { The discriminator is whether the IR expresses lifecycle intent AT ALL. An untraited legacy board expresses none, so the legacy trio is the honest answer and today's behaviour is preserved. */ - wipDeclared: lifecycle?.wip !== undefined - || (lifecycle?.hold === undefined && lifecycle?.review === undefined), + wipDeclared: lifecycle?.wip !== undefined || !declaresAnyLifecycleRole(lifecycle), }; if (memo) memo.lanes = lanes; return lanes;