From 87a4dbdc60c738d79043c38ab7933305f6084f24 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 02:19:17 -0700 Subject: [PATCH] test(u9): a release-leg E2E fixture that diagnoses itself (same defect found 3x independently) (#2678) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What The planned-spec release-leg fixture defect has now been diagnosed **three times independently** — #2634 (`workflow-lifecycle`), #2643 (`workflow-merged-board`), and again in `workflow-planning-lane`. Each time it cost real time, because it presents as a *scheduler* bug rather than a fixture bug. The mechanism: `createTaskWithReservedId` leaves a bootstrap seed (`# \n\n`), `isUnplannedForExecution` (`hold-release.ts`) refuses to move an unplanned card out of any intake- or hold-trait column, and the sweep reports `held: [{ reason: "move-rejected-or-no-slot" }]` while releasing nothing. That is **the gate working.** This extracts the write into `seedPlannedSpec` (`_planned-spec-fixture.ts`) which **self-checks against the real predicate the gate uses** (`isUnplannedSeedPrompt`, imported — not restated) and throws naming the fixture as the cause. Three call sites converted; their ~12-line comments collapse to a pointer, so the diagnosis and both dead-end hypotheses live once, at the seam that causes them. ## Measured (real PostgreSQL, not estimated) | Check | Result | |---|---| | 3 converted families + new ratchet | **41 pass / 41** | | Guard neutered (`if (false)`) | **4 of 5** ratchet tests fail | | Original defect reproduced faithfully | **3 of 7** planning-lane tests fail | | `pnpm lint`, engine `tsc --noEmit` | clean | **The ratchet fails on the original defect.** It drives the two real seed shapes through the production builders (`buildBootstrapPrompt`, `buildRefinementSeedPrompt`) rather than local imitations, so it cannot keep passing if the seed shape drifts — which is exactly the drift the fixture absorbs. The one test that survives the neutered guard is the happy path, which should not move. **The fixture is load-bearing, not decorative.** Proven by reproducing the defect end-to-end: production `buildBootstrapPrompt` on the created row with the guard bypassed → 3 planning-lane tests fail, including its own control case. A false mutation is worth recording, because it nearly produced a wrong "not load-bearing" verdict: a *hand-written* seed passed 7/7. `isUnplannedSeedPrompt` is **byte-equality** against a prompt built from the task's own title/description, so only a byte-exact seed reproduces it. A *missing* prompt does not either — `isUnplannedForExecution` catches the read error and returns `false`. ## Deliberately not converted `workflow-rebound-family`'s `PROMPT.md` write. It is a content-preservation artifact asserted byte-identical across a re-home, not a release-gate fixture; the helper would overwrite the very bytes under assertion. ## Reversible decisions taken (per standing authority) - **`opts.content` seam.** Exists so the ratchet can drive a known seed and prove the throw fires. Without it the guard could only ever be observed passing — the "guard that reports success without checking anything" failure mode. Documented as test-only. - **`title`/`description` optional.** The check is shape-based: both recognised seed forms are `\n\n` with no section headings, so the written spec cannot match either for *any* description. Omitting them cannot mask a positive, and a test asserts that directly. - **`merged-board`'s spec text lengthened** to match the other two (both were already non-seed, so behaviour is unchanged; verified by the 41-pass run). ## Method correction worth propagating My collision scan was wrong and I nearly acted on it. `git diff origin/main origin/ -- ` reports a difference when a branch is merely **stale** (the file did not exist at its base), so it flagged dashboard and CLI PRs as touching engine E2E files. Diffing against each branch's **merge base** is correct. Re-run under the fixed method: all five files here are uncontested, and `feature/code-organization-wave17` genuinely does touch `packages/engine/src/triage.ts` (so that one stays hands-off). ## Lane `.pg.test.ts` under engine-default, `pgDescribe`-skipped without PostgreSQL — the merge gate is unaffected. Throwaway per-file database, never port 4040, no temp-root walk. --- .../__tests__/_planned-spec-fixture.test.ts | 74 ++++++++++++++++ .../src/__tests__/_planned-spec-fixture.ts | 87 +++++++++++++++++++ .../workflow-lifecycle-live-e2e.pg.test.ts | 36 ++------ .../workflow-merged-board-live-e2e.pg.test.ts | 29 ++----- ...workflow-planning-lane-live-e2e.pg.test.ts | 26 ++---- 5 files changed, 180 insertions(+), 72 deletions(-) create mode 100644 packages/engine/src/__tests__/_planned-spec-fixture.test.ts create mode 100644 packages/engine/src/__tests__/_planned-spec-fixture.ts diff --git a/packages/engine/src/__tests__/_planned-spec-fixture.test.ts b/packages/engine/src/__tests__/_planned-spec-fixture.test.ts new file mode 100644 index 0000000000..9b20384249 --- /dev/null +++ b/packages/engine/src/__tests__/_planned-spec-fixture.test.ts @@ -0,0 +1,74 @@ +import { mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { buildBootstrapPrompt, buildRefinementSeedPrompt, isUnplannedSeedPrompt } from "@fusion/core"; +import { afterEach, describe, expect, it } from "vitest"; +import { seedPlannedSpec } from "./_planned-spec-fixture.js"; + +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-12:20: +Ratchet for the shared release-leg fixture. The fixture's whole value is its self-check, so that +check must be shown FAILING on the defect it exists to catch — a fixture that silently writes a +bootstrap seed, which is the state that produced the same three-times-diagnosed "the release sweep +is broken" symptom (#2634, #2643, workflow-planning-lane). + +The two seed shapes are built with the PRODUCTION builders rather than restated here; a local +imitation would keep passing if the real seed shape changed, which is exactly the drift the +fixture is meant to absorb. +*/ + +const dirs: string[] = []; +function makeStore(): { getTasksDir(): string; taskCache: { delete(id: string): void } } { + const root = mkdtempSync(join(tmpdir(), "fusion-planned-spec-fixture-")); + dirs.push(root); + return { getTasksDir: () => root, taskCache: { delete: () => {} } }; +} + +afterEach(() => { + while (dirs.length > 0) rmSync(dirs.pop() as string, { recursive: true, force: true }); +}); + +describe("seedPlannedSpec", () => { + it("writes a spec the release gate's own predicate accepts as planned", () => { + const store = makeStore(); + seedPlannedSpec(store, "FN-1001", { title: "a title", description: "a description" }); + + const written = readFileSync(join(store.getTasksDir(), "FN-1001", "PROMPT.md"), "utf-8"); + expect(isUnplannedSeedPrompt(written, "FN-1001", "a title", "a description")).toBe(false); + }); + + it("RATCHET: throws when the spec it would write is a bootstrap seed", () => { + const store = makeStore(); + // The exact content `createTaskWithReservedId` leaves behind — the original defect. + const seed = buildBootstrapPrompt("FN-1002", "a title", "a description"); + + expect(() => seedPlannedSpec(store, "FN-1002", { title: "a title", description: "a description", content: seed })) + .toThrow(/still classified as an unplanned bootstrap seed/); + }); + + it("RATCHET: throws for the refineTask seed shape too", () => { + const store = makeStore(); + // hold-release.ts:265 notes this second shape is also held; the fixture must reject it as well. + const seed = buildRefinementSeedPrompt("a title", "a description"); + + expect(() => seedPlannedSpec(store, "FN-1003", { title: "a title", description: "a description", content: seed })) + .toThrow(/FIXTURE defect, not a scheduler defect/); + }); + + it("names the fixture and the task in the failure so the diagnosis does not restart", () => { + const store = makeStore(); + const seed = buildBootstrapPrompt("FN-1004", undefined, "a description"); + + expect(() => seedPlannedSpec(store, "FN-1004", { description: "a description", content: seed })) + .toThrow(/seedPlannedSpec\(FN-1004\)[\s\S]*_planned-spec-fixture\.ts/); + }); + + it("works without title/description — the check is shape-based, so omitting them cannot mask a seed", () => { + const store = makeStore(); + expect(() => seedPlannedSpec(store, "FN-1005")).not.toThrow(); + + // And the omission genuinely does not weaken it: a seed still throws with no title/description. + const seed = buildBootstrapPrompt("FN-1006", undefined, ""); + expect(() => seedPlannedSpec(store, "FN-1006", { content: seed })).toThrow(/unplanned bootstrap seed/); + }); +}); diff --git a/packages/engine/src/__tests__/_planned-spec-fixture.ts b/packages/engine/src/__tests__/_planned-spec-fixture.ts new file mode 100644 index 0000000000..bda38b8f70 --- /dev/null +++ b/packages/engine/src/__tests__/_planned-spec-fixture.ts @@ -0,0 +1,87 @@ +import { mkdirSync, writeFileSync } from "node:fs"; +import { dirname } from "node:path"; +import { isUnplannedSeedPrompt } from "@fusion/core"; +import { getPromptPath } from "../spec-staleness.js"; + +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-12:05: +Shared release-leg fixture: give a seeded card a spec that FN-7648 accepts as PLANNED. + +Why this exists as a helper rather than as a block copied into each E2E family: the same +fixture defect has now been diagnosed THREE times independently (#2634 in +workflow-lifecycle, #2643 in workflow-merged-board, and again in workflow-planning-lane), +and it presents as a scheduler bug rather than a fixture bug. `createTaskWithReservedId` +leaves a bootstrap seed ("# \n\n"), and `isUnplannedForExecution` +(hold-release.ts) refuses to move an unplanned card out of any intake- or hold-trait +column. The sweep therefore reports `held: [{ reason: "move-rejected-or-no-slot" }]` and +releases nothing — which is the GATE WORKING. + +The contract is stated outright in +`docs/solutions/architecture-patterns/workflow-node-column-placement-and-graph-entry-contract.md`: +"Scheduler/release test fixtures must model a card that cleared the gate ... A held +unreviewed card is the gate working." + +Dead ends recorded so a fourth worker does not re-walk them: adding +`maxConcurrent`/`maxWorktrees` to the E2E settings changes nothing (the in-transaction +capacity gate from #2488/#2499 is not the cause), a direct `moveTask(id, wip)` succeeds +(the move is not the blocker), and bisecting to #2613's task-creation.ts attributes the +change correctly but yields the wrong verdict — post-U11 a card created in `todo` genuinely +IS intake, so receiving a bootstrap seed is correct behaviour. +*/ + +/** The subset of TaskStore this fixture needs; keeps the helper usable from narrow PG fakes. */ +type PlannedSpecStore = { + getTasksDir(): string; + taskCache?: { delete(id: string): void }; +}; + +/** + * Write a PROMPT.md that `isUnplannedSeedPrompt` classifies as a real spec, then PROVE it did. + * + * The verification is the point. A silent write would let a future change to the seed-detection + * heuristic (or to the prompt shape below) reintroduce the exact failure this helper exists to + * prevent, and the symptom would again surface as "the release sweep is broken" in whichever + * E2E family happened to run. Instead the throw names the real cause at the seam that caused it. + */ +export function seedPlannedSpec( + store: PlannedSpecStore, + taskId: string, + opts: { title?: string; description?: string; content?: string } = {}, +): void { + const promptPath = getPromptPath(store.getTasksDir(), taskId); + /* + `opts.content` exists so `_planned-spec-fixture.test.ts` can drive a KNOWN bootstrap seed + through this function and prove the throw below fires. Without that seam the guard could only + ever be observed passing, which is the "guard that reports success without checking anything" + failure mode. Production fixtures must not pass it. + */ + const content = opts.content + ?? (`# ${taskId}\n\n## Context\nA planned spec, so the release sweep does not classify this card ` + + `as an unplanned seed.\n\n## Steps\n### Step 1\n- [ ] do the planned work\n`); + + mkdirSync(dirname(promptPath), { recursive: true }); + writeFileSync(promptPath, content, "utf-8"); + + /* + Self-check against the REAL predicate the release gate uses, not a restatement of it. If this + fires, the fixture is no longer modelling a planned card and every release assertion downstream + of it is testing the gate instead of the sweep. + + `title`/`description` are optional because the check is about SHAPE: both seed forms the + classifier recognises are "\n\n" with no section headings, so the spec + above cannot equal either one for ANY description. Callers that have the real values may pass + them to keep the comparison exact; omitting them cannot mask a positive. + */ + if (isUnplannedSeedPrompt(content, taskId, opts.title, opts.description ?? "")) { + throw new Error( + `seedPlannedSpec(${taskId}): the spec written by this fixture is still classified as an ` + + `unplanned bootstrap seed by isUnplannedSeedPrompt, so isUnplannedForExecution will hold ` + + `the card and the release sweep will report "move-rejected-or-no-slot" while releasing ` + + `nothing. This is a FIXTURE defect, not a scheduler defect — update the prompt shape in ` + + `_planned-spec-fixture.ts to match the current seed-detection heuristic.`, + ); + } + + // The store caches tasks; release-gate reads must see the on-disk spec. + store.taskCache?.delete(taskId); +} diff --git a/packages/engine/src/__tests__/workflow-lifecycle-live-e2e.pg.test.ts b/packages/engine/src/__tests__/workflow-lifecycle-live-e2e.pg.test.ts index b6192ec5cf..857cd50ab4 100644 --- a/packages/engine/src/__tests__/workflow-lifecycle-live-e2e.pg.test.ts +++ b/packages/engine/src/__tests__/workflow-lifecycle-live-e2e.pg.test.ts @@ -53,6 +53,7 @@ import { WorkflowGraphTaskRunner, type WorkflowColumnBoundaryHooks } from "../wo import { createExecutorColumnBoundaryHooks } from "../workflow-column-boundary-hooks.js"; import { runHoldReleaseSweep } from "../hold-release.js"; import { DEFAULT_VOCAB, RENAMED_VOCAB, MERGED_VOCAB, MERGED_RENAMED_VOCAB, HOLD_STALENESS_MS, lifecycleIr, type Vocabulary } from "./_workflow-vocabulary-fixture.js"; +import { seedPlannedSpec } from "./_planned-spec-fixture.js"; import { SelfHealingManager } from "../self-healing.js"; import { reconcileRecovery } from "../recovery-reconciler.js"; @@ -139,36 +140,13 @@ pgDescribe("live lifecycle E2E: real graph + real PostgreSQL store", () => { ); await store.writeTaskWorkflowSelection(taskId, workflowId, []); /* - FNXC:WorkflowLifecycleColumns 2026-07-30-04:10 (release-leg fixture fix): - The card needs a PLANNED PROMPT.md before the sweep will release it. Task creation - leaves a bootstrap seed ("# \n\n"), and FN-7648's - `isUnplannedForExecution` reads that file for any card resting in an intake- OR - hold-trait column and refuses to move an unplanned card into a processing column — - so the sweep reported `held: [{ reason: "move-rejected-or-no-slot" }]` and released - nothing, which is the gate WORKING, not a scheduler defect. - - Diagnosed rather than guessed, and two hypotheses died on the way: adding - `maxConcurrent`/`maxWorktrees` to the E2E settings changed nothing (so the - in-transaction capacity gate from #2488/#2499 was NOT the cause), and a direct - `moveTask(id, wip)` succeeded (so the move itself was never the blocker). Probing the - two release gates showed `isTaskBlockedOnApproval=false`, - `isUnplannedForExecution=true`, and dumping the file showed the stub. - - This is the fixture modelling a card that cleared specification, which - `docs/solutions/architecture-patterns/workflow-node-column-placement-and-graph-entry-contract.md` - states outright: "Scheduler/release test fixtures must model a card that cleared the - gate ... A held unreviewed card is the gate working." + FNXC:WorkflowLifecycleColumns 2026-07-30-12:05 (release-leg fixture): + The card needs a spec FN-7648 accepts as PLANNED before the sweep will release it. The + reasoning, the two dead-end hypotheses, and the self-check now live in + `_planned-spec-fixture.ts` — this defect was diagnosed three times independently because it + presents as a scheduler bug, so it is documented once at the seam that causes it. */ - const { writeFileSync, mkdirSync } = await import("node:fs"); - const { join } = await import("node:path"); - const dir = join(store.getTasksDir(), taskId); - mkdirSync(dir, { recursive: true }); - writeFileSync( - join(dir, "PROMPT.md"), - `# ${taskId}\n\n## Context\nA planned spec, so the release sweep does not classify this card as an unplanned seed.\n\n## Steps\n### Step 1\n- [ ] do the planned work\n`, - "utf-8", - ); - store.taskCache.delete(taskId); + seedPlannedSpec(store, taskId); return task as Task; } diff --git a/packages/engine/src/__tests__/workflow-merged-board-live-e2e.pg.test.ts b/packages/engine/src/__tests__/workflow-merged-board-live-e2e.pg.test.ts index 4e030d4a4e..3dea9e3ade 100644 --- a/packages/engine/src/__tests__/workflow-merged-board-live-e2e.pg.test.ts +++ b/packages/engine/src/__tests__/workflow-merged-board-live-e2e.pg.test.ts @@ -34,6 +34,7 @@ import { } from "../../../core/src/__test-utils__/pg-test-harness.js"; import { runHoldReleaseSweep } from "../hold-release.js"; import { MERGED_VOCAB, RENAMED_VOCAB, lifecycleIr } from "./_workflow-vocabulary-fixture.js"; +import { seedPlannedSpec } from "./_planned-spec-fixture.js"; pgDescribe("live MERGED-board E2E: one column carrying both intake and hold", () => { const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({ @@ -63,30 +64,12 @@ pgDescribe("live MERGED-board E2E: one column carrying both intake and hold", () ); await store.writeTaskWorkflowSelection(taskId, workflowId, []); /* - FNXC:WorkflowLifecycleColumns 2026-07-29-14:40 (fixture fix — and a retracted escalation): - The card needs a PLANNED PROMPT.md before the release sweep will move it. FN-7648's - `isUnplannedForExecution` reads that file for any card resting in an intake- OR hold-trait - column and refuses to move an unplanned card into a processing column. On a MERGED board the - hold lane IS the intake lane, so every card seeded here is subject to that gate — a bootstrap - seed is held, and being held is the GATE WORKING, not a scheduler defect. - - This corrects a wrong escalation of mine. I bisected the failure to #2613's task-creation.ts - (reverting that one file re-greened everything) and reported it as a regression. The bisect was - sound as ATTRIBUTION and wrong as a verdict: #2613 made a card created in `todo` classify as - INTAKE, which post-U11 it genuinely is, so it correctly receives a bootstrap seed instead of a - specified prompt. My fixture had been relying on the pre-U11 semantics where `todo` was not - intake and a card created there was treated as already specified. - - The lesson, recorded because it cost another worker time: a revert that changes an outcome - proves WHICH change moved it, never that the OLD behaviour was the correct one. Diagnosis of - #2613's behaviour belongs to #2634, which probed the actual release gates rather than - bisecting. + FNXC:WorkflowLifecycleColumns 2026-07-30-12:05 (release-leg fixture): + On a MERGED board the hold lane IS the intake lane, so EVERY card seeded here is subject to + FN-7648's planned-spec gate. Rationale, the retracted #2613 escalation, and the self-check + live in `_planned-spec-fixture.ts`. */ - const { writeFileSync, mkdirSync } = await import("node:fs"); - const { join } = await import("node:path"); - const dir = join((store as never as { getTasksDir(): string }).getTasksDir(), taskId); - mkdirSync(dir, { recursive: true }); - writeFileSync(join(dir, "PROMPT.md"), `# ${taskId}\n\n## Context\nA planned spec.\n\n## Steps\n### Step 1\n- [ ] work\n`, "utf-8"); + seedPlannedSpec(store as never as { getTasksDir(): string }, taskId); store.taskCache.delete(taskId); } diff --git a/packages/engine/src/__tests__/workflow-planning-lane-live-e2e.pg.test.ts b/packages/engine/src/__tests__/workflow-planning-lane-live-e2e.pg.test.ts index 82d66ec30a..e2d0c4ab6b 100644 --- a/packages/engine/src/__tests__/workflow-planning-lane-live-e2e.pg.test.ts +++ b/packages/engine/src/__tests__/workflow-planning-lane-live-e2e.pg.test.ts @@ -53,6 +53,7 @@ import { import { runHoldReleaseSweep } from "../hold-release.js"; import { DEFAULT_VOCAB, RENAMED_VOCAB, lifecycleIr, type Vocabulary } from "./_workflow-vocabulary-fixture.js"; +import { seedPlannedSpec } from "./_planned-spec-fixture.js"; pgDescribe("live planning-lane E2E: real hold-release sweep + real PostgreSQL store", () => { const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({ @@ -86,27 +87,12 @@ pgDescribe("live planning-lane E2E: real hold-release sweep + real PostgreSQL st ); await store.writeTaskWorkflowSelection(taskId, workflowId, []); /* - FNXC:WorkflowLifecycleColumns 2026-07-30-09:30 (release-leg fixture fix, same defect as #2634): - The card needs a PLANNED PROMPT.md before the sweep will release it. Task creation leaves a - bootstrap seed ("# \n\n"), and FN-7648's `isUnplannedForExecution` reads that - file for any card resting in an intake- or hold-trait column and refuses to move an unplanned - card into a processing column. So the sweep released NOTHING and even this file's own control - case ("releases an ordinary held card on a default board") went red — the gate working, not a - scheduler defect. - - Identical to the defect #2634 repaired in workflow-lifecycle-live-e2e, and the reason it is - worth stating twice: a release/scheduler fixture that does not model a card which cleared - specification is testing the gate, not the sweep. + FNXC:WorkflowLifecycleColumns 2026-07-30-12:05 (release-leg fixture): + Needs a spec FN-7648 accepts as PLANNED, or even this file's own control case ("releases an + ordinary held card on a default board") goes red. Rationale and self-check live in + `_planned-spec-fixture.ts`. */ - const { writeFileSync, mkdirSync } = await import("node:fs"); - const { join } = await import("node:path"); - const dir = join((store as never as { getTasksDir(): string }).getTasksDir(), taskId); - mkdirSync(dir, { recursive: true }); - writeFileSync( - join(dir, "PROMPT.md"), - `# ${taskId}\n\n## Context\nA planned spec, so the release sweep does not classify this card as an unplanned seed.\n\n## Steps\n### Step 1\n- [ ] do the planned work\n`, - "utf-8", - ); + seedPlannedSpec(store as never as { getTasksDir(): string }, taskId); if (Object.keys(fields).length > 0) await store.updateTask(taskId, fields as never); store.taskCache.delete(taskId); }