fix(engine): TAKING spec-staleness.ts + mission-feature-sync.ts — planner lanes (2 triage guards → 0) (#2616)
**Claiming `packages/engine/src/spec-staleness.ts` and `packages/engine/src/mission-feature-sync.ts`.** Deliberately *not* `self-healing.ts` (contended) or `task-creation.ts` (#2589 in flight). ## Guard counts | scope | before | after | |---|---|---| | `spec-staleness.ts` | 1 | **0** | | `mission-feature-sync.ts` | 1 | **0** | | repo-wide `column === / !== "triage"` in `packages/*/src` (excl. tests) | 26 | **24** | **21** once #2612 (comments-ops, 3 guards) also lands. ## What was silently broken **mission-feature-sync** — *"has this task returned to a planner lane?"* decided whether a mission feature drops from `in-progress` back to `triaged`. Keyed on the legacy pair, a card sent back for re-planning on a renamed board left its feature stuck at `in-progress` **forever**: the mission board showed work in flight that nobody was doing, and nothing said so. **spec-staleness** — the preserved-progress skip refuses to fire for an **intake** card, since a card being specified has no progress to protect. Keyed on `triage`, a renamed-board intake card looked like started work and its stale spec was skipped instead of re-planned. ## The union is a deliberate call, and the existing suite forced it My first cut *replaced* the legacy pair with the resolved lanes. That broke a real case: **post-U11 the default lineage has no `triage`**, so a legacy row still resting there stopped counting as a planner lane. `usage-limit-detector` already made this call for the same situation and wrote down why — **over-inclusion is the safe direction**. Marking a feature `triaged` for a card in a legacy planner column is recoverable; a mission board permanently showing phantom work is the bug. So the legacy pair stands and resolved lanes are *added* to it. Worth noting the existing test is what caught this, not review — which is the argument for converting against a real suite rather than in isolation. ## A parameter, and why that needs the ratchet `spec-staleness`'s predicate is **pure** (a task, no store), so the role arrives as a parameter and both callers resolve it. That optionality is exactly the caller-omission hazard this program has already shipped twice, so the function is also registered in core's `role-parameter-caller-audit` (#2588). **This PR's tests prove the parameter is honoured; the audit proves it is passed. Neither alone is enough** — that split is the whole lesson of #2586. ## Mutation-verified | mutation | result | |---|---| | mission-feature-sync → legacy pair only | the two renamed cases fail | | spec-staleness → restore the `triage` literal | its renamed case fails | Negatives included in both: demoting a **WIP** card's feature would report running work as un-started, and never-skipping would discard every card with real progress. ## Verification - new suites 4/4 and 3/3; `mission-feature-sync` + `spec-staleness` 27/27 - engine `tsc --noEmit` clean; `pnpm test:gate` green (482 + 132 + 10) - `executor-prompt` reports 3 failures **both with and without** this change — pre-existing, baselined by stashing rather than assumed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,81 @@
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-11:10 (Phase B conversion — mission-feature-sync):
|
||||
|
||||
"Has this task returned to a PLANNER LANE?" decided whether a mission feature drops from
|
||||
`in-progress` back to `triaged`. It asked with the legacy `triage`/`todo` pair, so on a
|
||||
renamed board a card sent back for re-planning left its feature stuck at `in-progress`
|
||||
FOREVER — the mission board showed work in flight that nobody was doing, and nothing said so.
|
||||
|
||||
The conversion UNIONS the resolved lanes with the legacy pair rather than replacing it. That
|
||||
is not caution for its own sake: replacing broke a real case, because post-U11 the default
|
||||
lineage has no `triage` and a legacy row still sitting there stopped counting. The existing
|
||||
suite caught that, which is why the union is here and why this file records the reason.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
import "@fusion/core";
|
||||
import { reconcileMissionFeatureState } from "../mission-feature-sync.js";
|
||||
|
||||
/** A store whose IR resolution yields a workflow with renamed planner lanes. */
|
||||
function renamedStore() {
|
||||
return {
|
||||
getTask: async () => undefined,
|
||||
getTaskWorkflowSelectionAsync: async () => ({ workflowId: "custom:renamed", stepIds: [] }),
|
||||
getWorkflowDefinition: async () => ({
|
||||
id: "custom:renamed",
|
||||
ir: {
|
||||
version: "v2",
|
||||
id: "custom:renamed",
|
||||
nodes: [{ id: "start", kind: "start", column: "drafting" }, { id: "end", kind: "end", column: "shipped" }],
|
||||
edges: [{ from: "start", to: "end" }],
|
||||
columns: [
|
||||
{ id: "drafting", label: "Drafting", traits: [{ trait: "intake" }] },
|
||||
{ id: "queued", label: "Queued", traits: [{ trait: "hold", config: { release: "capacity" } }] },
|
||||
{ id: "building", label: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
|
||||
{ id: "shipped", label: "Shipped", traits: [{ trait: "complete" }] },
|
||||
],
|
||||
},
|
||||
}),
|
||||
} as never;
|
||||
}
|
||||
|
||||
describe("mission feature sync under a renamed planner vocabulary", () => {
|
||||
it("drops the feature back to `triaged` for a card in the RENAMED intake column", async () => {
|
||||
const decision = await reconcileMissionFeatureState(
|
||||
renamedStore(),
|
||||
{ id: "FN-MR-1", column: "drafting" } as never,
|
||||
{ id: "F-1", status: "in-progress" } as never,
|
||||
);
|
||||
expect(decision).toMatchObject({ kind: "update", status: "triaged" });
|
||||
});
|
||||
|
||||
it("drops the feature back to `triaged` for a card in the RENAMED hold column", async () => {
|
||||
const decision = await reconcileMissionFeatureState(
|
||||
renamedStore(),
|
||||
{ id: "FN-MR-2", column: "queued" } as never,
|
||||
{ id: "F-1", status: "in-progress" } as never,
|
||||
);
|
||||
expect(decision).toMatchObject({ kind: "update", status: "triaged" });
|
||||
});
|
||||
|
||||
it("does NOT drop the feature for a card in the renamed WIP column", async () => {
|
||||
/* The negative half: `building` is active work, and demoting its feature to `triaged`
|
||||
would report a running task as un-started. */
|
||||
const decision = await reconcileMissionFeatureState(
|
||||
renamedStore(),
|
||||
{ id: "FN-MR-3", column: "building", status: "in-progress" } as never,
|
||||
{ id: "F-1", status: "in-progress" } as never,
|
||||
);
|
||||
expect(decision).not.toMatchObject({ status: "triaged" });
|
||||
});
|
||||
|
||||
it("still honours the LEGACY pair for a card left in `triage` (the union, not a replacement)", async () => {
|
||||
/* Post-U11 the default lineage has no `triage`, so a resolved-lanes-only check would
|
||||
leave a legacy row's feature stuck at `in-progress` forever. */
|
||||
const decision = await reconcileMissionFeatureState(
|
||||
renamedStore(),
|
||||
{ id: "FN-MR-4", column: "triage" } as never,
|
||||
{ id: "F-1", status: "in-progress" } as never,
|
||||
);
|
||||
expect(decision).toMatchObject({ kind: "update", status: "triaged" });
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,37 @@
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-11:30 (Phase B conversion — spec-staleness):
|
||||
|
||||
`shouldSkipSpecStalenessForPreservedProgress` refuses to skip staleness for a card in the
|
||||
INTAKE column — a card being specified has no preserved progress to protect. It compared
|
||||
against the literal `triage`, so on a renamed board an intake card looked like started work
|
||||
and its stale spec was skipped rather than re-planned.
|
||||
|
||||
The predicate is PURE (a task, no store), so the role arrives as a parameter and the two
|
||||
callers resolve it. That optionality is the caller-omission hazard, which is why the function
|
||||
is also registered in core's role-parameter-caller-audit — this file proves the parameter is
|
||||
HONOURED, the audit proves it is PASSED. Neither alone is enough.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { shouldSkipSpecStalenessForPreservedProgress } from "../spec-staleness.js";
|
||||
|
||||
const started = { column: "drafting", currentStep: 2, status: undefined } as never;
|
||||
|
||||
describe("spec staleness preserved-progress skip under a renamed intake column", () => {
|
||||
it("does NOT skip for a card in the RENAMED intake column", () => {
|
||||
/* The conversion: with `drafting` named as intake this must refuse to skip, exactly as
|
||||
it refuses for `triage` on the default board. */
|
||||
expect(shouldSkipSpecStalenessForPreservedProgress(started, "drafting")).toBe(false);
|
||||
});
|
||||
|
||||
it("DOES skip started work in a non-intake column of the same board", () => {
|
||||
/* The negative half — otherwise "never skip" would send every card with real progress
|
||||
back through re-planning and discard it. */
|
||||
expect(shouldSkipSpecStalenessForPreservedProgress({ ...started, column: "building" } as never, "drafting")).toBe(true);
|
||||
});
|
||||
|
||||
it("still refuses to skip for `triage` when no role is supplied (regression floor)", () => {
|
||||
expect(
|
||||
shouldSkipSpecStalenessForPreservedProgress({ ...started, column: "triage" } as never),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user