From 5cb731420cd8d5a948c9c7ebcd2dbdff7dbf7190 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 16:00:41 -0700 Subject: [PATCH] test(engine): re-point the stale-spec ratchet at the improved guard (#2863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Main is red; this fixes it `executor-stale-spec-active-lanes.test.ts` — 2 failures on `main`: ``` × resolves the task's lifecycle columns before deciding the skip → expected source to contain 'const activeLifecycle = resolveLifecy…' × adds the wip, review and complete lanes to the active set ``` **Nothing regressed — the guard got better and the ratchet didn't follow.** ## What changed in the product The stale-spec skip used to build its active set from `resolveLifecycleColumns(...)`, taking `?.wip`, `?.review` and `?.complete`. That returns the **first** column carrying each trait, so a board with two wip lanes — or a review lane plus a second merge-blocking one — had only one of each recognised as active. A card in the other read as **inactive**, and its prompt file was treated as reclaimable. It now resolves the IR once and unions `columnsWithFlag` over five flags, which returns **every** column carrying each: ```ts const activeIr = await resolveWorkflowIrForTask(this.store, task.id); const activeColumns = new Set(["in-progress", "in-review", "done"]); if (activeIr) { for (const flag of ["countsTowardWip", "mergeOrchestration", "mergeBlocker", "humanReview", "complete"] as const) { for (const lane of columnsWithFlag(activeIr, flag)) activeColumns.add(lane); ``` That is a real fix to an arity bug, and the legacy trio stays unioned in for the documented reason: under-reporting active is the destructive direction. ## Why re-point rather than loosen This is a **source ratchet** in the `engine-no-blocking-shellout` style, and its entire value is that it fails on a revert. A substring loose enough to match both the old and new shapes would keep the file green through exactly the regression it exists to catch. So the assertions now name the new shape precisely, including the **flag list**, so dropping one of the five is caught here too. **Mutation-verified:** replacing the IR resolution with `undefined` fails the ratchet. It still does its job. ## Scope The file's own header notes this is *not* a behavioural proof — the guard sits inside `execute()` behind worktree and session setup a unit test has no business standing up, and it asks whoever next touches that scaffolding to add the end-to-end case. Re-pointing is maintenance; I have not taken on that harness here, and the note still stands. ## Verification - `executor-stale-spec-active-lanes.test.ts` — **4/4**, fails on revert - `pnpm lint` — clean Test-only; no changeset. Found by running the full `engine-default` project after #2855 merged — the remaining `main` red is my own audit case, fixed by the pending #2857. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) --- .../executor-stale-spec-active-lanes.test.ts | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/packages/engine/src/__tests__/executor-stale-spec-active-lanes.test.ts b/packages/engine/src/__tests__/executor-stale-spec-active-lanes.test.ts index 4c4ece5b82..1de82b7363 100644 --- a/packages/engine/src/__tests__/executor-stale-spec-active-lanes.test.ts +++ b/packages/engine/src/__tests__/executor-stale-spec-active-lanes.test.ts @@ -43,17 +43,37 @@ import { readFileSync } from "node:fs"; const source = readFileSync(new URL("../executor.ts", import.meta.url), "utf8"); describe("the stale-spec skip resolves the board's own active lanes", () => { - it("resolves the task's lifecycle columns before deciding the skip", () => { + it("resolves the task's own workflow IR before deciding the skip", () => { + /* + FNXC:WorkflowResolvedColumns 2026-08-02-02:20 (the ratchet drifted behind an IMPROVEMENT): + THE GUARD GOT BETTER AND THIS FILE WENT RED. It used to read + `resolveLifecycleColumns(...)` and add `?.wip / ?.review / ?.complete`, which returns the FIRST + column carrying each trait — so a board with two wip lanes, or a review lane plus a second + merge-blocking one, had only one of each recognised as active, and a card in the other read as + INACTIVE. That was fixed by resolving the IR once and unioning `columnsWithFlag` over five flags, + which returns EVERY column carrying each. + + The assertions are re-pointed at the new shape rather than loosened: this is a source ratchet in + the `engine-no-blocking-shellout` style, and its whole value is that it fails on a revert. A + substring that matched both shapes would keep the file green through exactly the regression it + exists to catch. + + The header's standing note still applies — this is not a behavioural proof, and the guard sits + behind worktree and session setup that a unit test has no business standing up. Re-pointing it is + maintenance, not the end-to-end case it asks for. + */ expect(source).toContain( - "const activeLifecycle = resolveLifecycleColumns(await resolveWorkflowIrForTask(this.store, task.id));", + "const activeIr = await resolveWorkflowIrForTask(this.store, task.id);", ); }); - it("adds the wip, review and complete lanes to the active set", () => { + it("unions EVERY column carrying each active-lane trait, not the first per role", () => { + /* `columnsWithFlag` over the five flags is the arity fix: `.has()` membership needs every lane, + not one per trait. Asserting the flag list too, so dropping one is caught here. */ expect(source).toContain( - "for (const lane of [activeLifecycle?.wip, activeLifecycle?.review, activeLifecycle?.complete]) {", + 'for (const flag of ["countsTowardWip", "mergeOrchestration", "mergeBlocker", "humanReview", "complete"] as const) {', ); - expect(source).toContain("if (lane !== undefined) activeColumns.add(lane);"); + expect(source).toContain("for (const lane of columnsWithFlag(activeIr, flag)) activeColumns.add(lane);"); }); it("UNIONS rather than replaces, so a degraded IR cannot narrow the set", () => {