From f91b8a41783fabeaceeed6b0353192d57d582f99 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 29 Jul 2026 23:06:55 -0700 Subject: [PATCH] TAKING mission-feature-sync.ts: roadmap reconciliation resolves lifecycle roles (unowned drift site) (#2602) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > **Taking `packages/engine/src/mission-feature-sync.ts`** from the shared backlog — announced in the title per the collision protocol. Based on `main`, no dependencies. It is in **no unit's file list**: absent from the plan's per-file census *and* from the drift review's ownership split (self-healing, dashboard, triage/replan-target, core, executor). It is a planning-lane reader. ## What was broken `reconcileMissionFeatureState` maps a task's lifecycle **position** onto its mission feature's roadmap status, and read five column literals: `done`, `archived`, `in-progress`, `in-review`, `triage`/`todo`. On a renamed workflow **every branch answers "no"**, so the function collapses to a permanent `noop`. **What an operator sees:** a mission roadmap frozen at whatever status it last held, while the tasks underneath it run to completion. Nothing errors, nothing retries. Worse than a wrong status, because a stale roadmap reads as a stable one. ## Guard counts (per the reporting requirement) | Metric | Before | After | |---|---:|---:| | `column === / !== "triage"` in this file | **1** | **1** | | role comparisons converted | — | **5** | **The metric does not move here, and I am not claiming it does.** The five role comparisons are converted; the one literal that remains is the deliberate scoped migration acceptance this change *adds*. That is the third time on this program the real fix has been invisible to the convergence count — the count finds the site, it does not define done. Worth knowing while the shared backlog is being tracked by that number: repo-wide it currently reads **29** triage comparisons (including 4 in `plugins/`, which are also unowned). ## Fallback direction matters **Unresolvable workflow falls back to the legacy ids, not to `noop`.** A mission whose workflow cannot be read should keep tracking on the default vocabulary rather than go silent — going silent *is* the failure being fixed, so the fallback must not reproduce it. **The planner-lane branch also accepts an orphaned legacy id.** A pre-existing test asserted a card in `triage` returns its feature to `triaged`; that stopped holding for the default lineage after #2515 — the migration-window population again. Accepting `triage`/`todo` additively keeps those rows tracked, **scoped to ids the workflow does not declare**, for the reason greptile gave on #2593: a custom workflow may legitimately name its **review** lane `triage`, and mapping a card there to `triaged` would walk the roadmap backwards while the task is awaiting merge. ## A test of mine that proved nothing until fixed The scoping case first used a `triaged` feature. The planner-lane branch only fires for an **in-progress** feature, so the fixture fell through to the review branch and **passed under both implementations**. It discriminates only once the feature status lets the wrong branch win — verified by reverting the scoping and watching exactly that case fail. ## Revert proofs | Reverted | Result | |---|---| | all five literals restored | **5 of 15 fail** — every renamed case; every default case passes | | legacy acceptance unscoped | **1 of 15 fails** — the custom-`triage`-as-review case | ## Verification | Check | Result | |---|---| | new suite | 15/15 | | pre-existing mission-feature-sync + mission-autopilot + scheduler-trait-dispatch | 94/94, **no expectation edits** | | `tsc --noEmit` (engine) | clean | | `pnpm lint` | clean | | `pnpm test:gate` | green (482 + 10 + 71) | | `pnpm check:changesets` | clean | ## Next from the shared backlog Taking `plugins/fusion-plugin-even-realities-glasses/src/agent-actions.ts` (3) and `plugins/fusion-plugin-dependency-graph/src/GraphTaskNode.tsx` (1) next — 4 sites in `plugins/`, which no unit owns and which the #2587 ratchet now scans. Shout if anyone is already there. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) --- .changeset/mission-feature-sync-lanes.md | 7 + .../mission-feature-sync-lanes.test.ts | 261 ++++++++++++++++++ packages/engine/src/mission-feature-sync.ts | 111 +++++++- 3 files changed, 370 insertions(+), 9 deletions(-) create mode 100644 .changeset/mission-feature-sync-lanes.md create mode 100644 packages/engine/src/__tests__/mission-feature-sync-lanes.test.ts diff --git a/.changeset/mission-feature-sync-lanes.md b/.changeset/mission-feature-sync-lanes.md new file mode 100644 index 0000000000..703f8409b0 --- /dev/null +++ b/.changeset/mission-feature-sync-lanes.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: A mission roadmap keeps tracking its tasks on workflows with renamed columns instead of silently freezing. +category: fix +dev: U7 / R3 — unowned drift site (mission-feature-sync.ts is in no unit's file list). `reconcileMissionFeatureState` read five column literals (done, archived, in-progress, in-review, triage/todo); on a renamed workflow every branch answered "no" and the function collapsed to a permanent noop, so the roadmap froze while the tasks underneath ran to completion. Now resolves complete/archived/wip/review/intake/hold from the task's own workflow. Unresolvable workflow falls back to the legacy ids, NOT to noop — going silent is the failure being fixed. The planner-lane branch also accepts an orphaned legacy `triage`/`todo` id (pre-U11 rows awaiting re-homing), scoped to ids the workflow does not declare so a custom workflow naming its review lane `triage` is not walked backwards. diff --git a/packages/engine/src/__tests__/mission-feature-sync-lanes.test.ts b/packages/engine/src/__tests__/mission-feature-sync-lanes.test.ts new file mode 100644 index 0000000000..b8a7eb459e --- /dev/null +++ b/packages/engine/src/__tests__/mission-feature-sync-lanes.test.ts @@ -0,0 +1,261 @@ +/* +FNXC:MissionFeatureSyncLanes 2026-07-30-02:10 (U7 / R3, R12 — unowned drift site): + +`reconcileMissionFeatureState` maps a task's lifecycle POSITION onto its mission +feature's roadmap status. It read five column literals — `done`, `archived`, +`in-progress`, `in-review`, `triage`/`todo` — and on a renamed workflow every one of +them silently answers "no", so the function collapses to a permanent `noop`. + +WHAT THAT LOOKS LIKE TO AN OPERATOR: a mission roadmap frozen at whatever status it +last held while the tasks underneath it run to completion. Nothing errors, nothing +retries, and the mission view simply stops tracking reality. That is worse than a +wrong status, because a stale roadmap reads as a stable one. + +`mission-feature-sync.ts` is in no unit's file list — not in the plan's per-file +census, and not in the drift review's ownership split (self-healing, dashboard, +triage/replan-target, core, executor). Picked up because it is a planning-lane +reader and nobody else has it. + +The three mappings, in role terms: + complete -> feature done + archived -> noop (retention, never progress) + wip or review -> feature in-progress + intake or hold -> feature triaged ("returned to triage") + +The function is already async and already takes the store, so this resolves for +real rather than needing the injected-lane pattern the synchronous predicates +required. An unresolvable workflow falls back to the legacy ids — NOT to `noop`: a +mission whose workflow cannot be read should keep tracking on the default +vocabulary rather than go silent, which is the failure being fixed. +*/ +import { describe, expect, it, vi } from "vitest"; +import type { MissionFeature, Task, TaskStore, WorkflowIr } from "@fusion/core"; + +import { reconcileMissionFeatureState } from "../mission-feature-sync.js"; + +const DEFAULT_NAMES = { + intake: "triage", hold: "todo", wip: "in-progress", + review: "in-review", complete: "done", archived: "archived", +}; +/* Every role renamed, and no id collides with a legacy literal, so a surviving + comparison cannot match by luck. */ +const RENAMED = { + intake: "backlog", hold: "drafting", wip: "building", + review: "checking", complete: "shipped", archived: "attic", +}; + +function ir(n: typeof DEFAULT_NAMES): WorkflowIr { + return { + version: "v2", id: "wf", name: "wf", nodes: [], edges: [], + columns: [ + { id: n.intake, name: "Intake", traits: [{ trait: "intake" }] }, + { id: n.hold, name: "Hold", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: n.wip, name: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { + id: n.review, name: "Review", + traits: [{ trait: "merge-blocker" }, { trait: "human-review" }, { trait: "merge" }], + }, + { id: n.complete, name: "Complete", traits: [{ trait: "complete" }] }, + { id: n.archived, name: "Archived", traits: [{ trait: "archived" }] }, + ], + } as unknown as WorkflowIr; +} + +const task = (column: string): Task => ({ + id: "FN-1", title: "t", description: "", column, status: null, error: null, + dependencies: [], steps: [], currentStep: 0, log: [], + createdAt: "2026-01-01T00:00:00.000Z", updatedAt: "2026-01-01T00:00:00.000Z", +} as unknown as Task); + +const feature = (status: string): Pick => + ({ id: "feat-1", status, lastValidatorStatus: undefined } as never); + +function storeWith(workflowIr: WorkflowIr | null): TaskStore { + const selection = { workflowId: "wf", stepIds: [] }; + return { + getTask: vi.fn(async () => undefined), + getTaskWorkflowSelection: vi.fn(() => selection), + getTaskWorkflowSelectionAsync: vi.fn(async () => selection), + getWorkflowDefinition: vi.fn(async () => { + if (!workflowIr) throw new Error("unresolvable"); + return { ir: workflowIr }; + }), + } as unknown as TaskStore; +} + +const decide = (n: typeof DEFAULT_NAMES, column: string, featureStatus: string, workflowIr: WorkflowIr | null = ir(n)) => + reconcileMissionFeatureState(storeWith(workflowIr), task(column), feature(featureStatus)); + +describe("mission feature reconciliation resolves lifecycle roles", () => { + for (const [label, n] of [["default", DEFAULT_NAMES], ["renamed", RENAMED]] as const) { + it(`marks a feature done when its task reaches the ${label} complete column`, async () => { + const d = await decide(n, n.complete, "in-progress"); + expect(d.kind).toBe("update"); + expect(d.kind === "update" && d.status).toBe("done"); + }); + + it(`treats the ${label} archived column as retention, never progress`, async () => { + // Archiving must not fabricate roadmap progress. + expect((await decide(n, n.archived, "triaged")).kind).toBe("noop"); + }); + + it(`marks a feature in-progress when its task reaches the ${label} wip column`, async () => { + const d = await decide(n, n.wip, "triaged"); + expect(d.kind === "update" && d.status).toBe("in-progress"); + }); + + it(`marks a feature in-progress when its task reaches the ${label} review column`, async () => { + const d = await decide(n, n.review, "defined"); + expect(d.kind === "update" && d.status).toBe("in-progress"); + }); + + it(`returns a feature to triaged when its task bounces to the ${label} intake column`, async () => { + const d = await decide(n, n.intake, "in-progress"); + expect(d.kind === "update" && d.status).toBe("triaged"); + }); + + it(`returns a feature to triaged when its task bounces to the ${label} hold column`, async () => { + const d = await decide(n, n.hold, "in-progress"); + expect(d.kind === "update" && d.status).toBe("triaged"); + }); + } + + it("falls back to the legacy vocabulary when the workflow cannot be resolved", async () => { + /* + NOT `noop`. A mission whose workflow cannot be read should keep tracking on the + default vocabulary rather than go silent — going silent is the exact failure this + conversion fixes, so the unresolvable path must not reproduce it. + */ + const d = await decide(DEFAULT_NAMES, "in-progress", "triaged", null); + expect(d.kind === "update" && d.status).toBe("in-progress"); + }); +}); + +/* +FNXC:MissionFeatureSyncLanes 2026-07-30-02:40: +The legacy-id acceptance for the planner-lane branch, and its scoping. Pre-#2515 rows +still resting in `triage`/`todo` must keep returning their feature to `triaged` — but +only when the workflow does not declare that id as some OTHER role, which is the +over-reach greptile caught on #2593. +*/ +describe("legacy planner ids are accepted only when orphaned", () => { + /** A workflow that names its REVIEW lane `triage` — legal, and not a planner lane. */ + const TRIAGE_IS_REVIEW = { ...RENAMED, review: "triage" }; + + it("returns the feature to triaged for an ORPHANED legacy `triage` row", async () => { + // The migration window: the workflow declares no `triage`, so the row is a + // pre-U11 leftover awaiting re-homing and still means "back to planning". + const d = await decide(RENAMED, "triage", "in-progress"); + expect(d.kind === "update" && d.status).toBe("triaged"); + }); + + it("does NOT return the feature to triaged when `triage` is the workflow's REVIEW lane", async () => { + /* + Mapping a review-lane card to `triaged` would walk the roadmap BACKWARDS while the + task is actually awaiting merge. The review role must win over the legacy id. + + `feature.status: "in-progress"` is load-bearing: the planner-lane branch only + fires for an in-progress feature, so a `triaged` fixture would fall through to the + review branch and pass whether or not the acceptance is scoped. My first version + made exactly that mistake and passed under both implementations — it proved + nothing until the feature status let the wrong branch win. + */ + const d = await decide(TRIAGE_IS_REVIEW, "triage", "in-progress"); + + // Stays in-progress via the REVIEW role, rather than being walked back to triaged. + expect(d.kind).toBe("noop"); + }); +}); + +/* +FNXC:MissionFeatureSyncLanes 2026-07-30-05:40 (PR #2602 review — greptile P1): +A per-role legacy fallback must never claim a column the workflow assigned to a +DIFFERENT role. Unguarded, a workflow that omits `hold` but names its REVIEW lane +`todo` got `lane.hold = "todo"`, so a card awaiting merge matched the planner-lane +branch and its feature was walked BACKWARDS from in-progress to triaged. +*/ +describe("legacy per-role fallbacks never alias a declared role", () => { + /** No hold role, and the REVIEW lane is named with the legacy hold id. */ + function holdlessTodoIsReviewIr(): WorkflowIr { + return { + version: "v2", id: "wf", name: "wf", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Intake", traits: [{ trait: "intake" }] }, + { id: "building", name: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + // `merge` is what makes this resolve as the REVIEW role — see the note above + // `todoIsReviewIr` in the sibling suite for why that detail matters. + { id: "todo", name: "Review", traits: [{ trait: "merge-blocker" }, { trait: "human-review" }, { trait: "merge" }] }, + { id: "shipped", name: "Complete", traits: [{ trait: "complete" }] }, + ], + } as unknown as WorkflowIr; + } + + it("does NOT walk an in-progress feature back to triaged for a card in the REVIEW lane", async () => { + const d = await reconcileMissionFeatureState( + storeWith(holdlessTodoIsReviewIr()), + task("todo"), + feature("in-progress"), + ); + + // The review role governs: the feature stays in-progress rather than regressing. + expect(d.kind).toBe("noop"); + }); + + it("still advances a feature to in-progress from that same REVIEW lane", async () => { + // The other side, so "always noop" cannot pass for "correctly not a planner lane". + const d = await reconcileMissionFeatureState( + storeWith(holdlessTodoIsReviewIr()), + task("todo"), + feature("triaged"), + ); + + expect(d.kind === "update" && d.status).toBe("in-progress"); + }); +}); + +/* +FNXC:MissionFeatureSyncLanes 2026-07-30-06:40 (PR #2602 review, second P1 — greptile): +"DECLARED" means the workflow declares a column with that id — NOT that some role +resolved to it. A non-lifecycle column named `todo` (traits mapping to no role) was +invisible to the previous role-only check, so the fallback claimed it as `lane.hold` +and a task resting there had its feature regressed to `triaged`. + +I had recorded that as a residual limitation. It was not a limitation, it was an unread +input: the IR is in reach here. +*/ +describe("a non-lifecycle column named with a legacy id is not claimed", () => { + /** No hold role, plus a column literally named `todo` that carries NO lifecycle trait. */ + function holdlessWithInertTodoIr(): WorkflowIr { + return { + version: "v2", id: "wf", name: "wf", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Intake", traits: [{ trait: "intake" }] }, + { id: "building", name: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + // Declared, but maps to no role — the case the role-only check could not see. + { id: "todo", name: "Parking", traits: [] }, + { id: "shipped", name: "Complete", traits: [{ trait: "complete" }] }, + ], + } as unknown as WorkflowIr; + } + + it("does NOT regress an in-progress feature for a task in that inert column", async () => { + const d = await reconcileMissionFeatureState( + storeWith(holdlessWithInertTodoIr()), + task("todo"), + feature("in-progress"), + ); + + expect(d.kind).toBe("noop"); + }); + + it("still treats the workflow's REAL intake column as a planner lane", async () => { + // The counter-case, so "never a planner lane" cannot pass for "reads the IR". + const d = await reconcileMissionFeatureState( + storeWith(holdlessWithInertTodoIr()), + task("backlog"), + feature("in-progress"), + ); + + expect(d.kind === "update" && d.status).toBe("triaged"); + }); +}); diff --git a/packages/engine/src/mission-feature-sync.ts b/packages/engine/src/mission-feature-sync.ts index 5e20aca8cc..56d8e7f783 100644 --- a/packages/engine/src/mission-feature-sync.ts +++ b/packages/engine/src/mission-feature-sync.ts @@ -1,4 +1,5 @@ import type { MissionFeature, Task, TaskStore } from "@fusion/core"; +import { resolveTaskLifecycleColumns, resolveWorkflowIrForTask } from "@fusion/core"; import { getTaskCompletionBlockerForStore } from "./task-completion.js"; export type MissionFeatureSyncTargetStatus = "done" | "in-progress" | "triaged"; @@ -40,7 +41,7 @@ export type MissionFeatureSyncDecision = | { kind: "noop" }; export async function reconcileMissionFeatureState( - taskStore: Pick, + taskStore: Pick & Parameters[0], task: Task, feature: Pick, context: MissionFeatureSyncContext = {}, @@ -62,7 +63,66 @@ export async function reconcileMissionFeatureState( const hasUnvalidatedAssertions = context.hasLinkedAssertions === true && feature.lastValidatorStatus !== "passed"; - if (task.column === "done") { + /* + FNXC:MissionFeatureSyncLanes 2026-07-30-02:10 (U7 / R3): + Map the task's lifecycle POSITION onto the feature's roadmap status by ROLE. Keyed + on the five literals, EVERY branch below silently answered "no" on a renamed + workflow, so this collapsed to a permanent `noop`: the mission's roadmap froze at + whatever status it last held while the tasks underneath it ran to completion. + + Worse than a wrong status — a stale roadmap reads as a stable one. Nothing errors, + nothing retries, and the mission view stops tracking reality. + + Unresolvable workflow falls back to the LEGACY ids rather than to `noop`: a mission + whose workflow cannot be read should keep tracking on the default vocabulary, not go + silent, which is the exact failure being fixed here. + */ + const roles = await resolveTaskLifecycleColumns(taskStore, task.id); + /* + FNXC:MissionFeatureSyncLanes 2026-07-30-05:40 (PR #2602 review — greptile P1): + A per-role legacy fallback must NEVER claim a column the workflow assigned to a + DIFFERENT role. Unguarded, a workflow that omits `hold` but names its REVIEW lane + `todo` got `lane.hold = "todo"` — so a card awaiting merge matched the planner-lane + branch and its feature was walked BACKWARDS from in-progress to triaged. + + The fallback exists for a workflow that declares no such role at all; it is not a + licence to alias one that does. Same over-reach greptile caught in the plugin gates + (#2607) and in the recovery acceptance (#2593) — three variations of "a legacy id is + not a role". + */ + /* + FNXC:MissionFeatureSyncLanes 2026-07-30-06:40 (PR #2602 review, second P1 — greptile): + DECLARED means "the workflow declares a column with this id", not "some ROLE resolved + to this id". My previous revision built `declared` from the six resolved roles, so a + custom workflow with a non-lifecycle column named `todo` — one carrying traits that + map to no role — left the id invisible, the fallback claimed it as `lane.hold`, and a + task resting there had its feature regressed to `triaged`. + + I had written that gap down as a residual limitation. Documenting it was not handling + it: the IR is in reach here, so read the columns and the limitation disappears. + */ + const ir = await resolveWorkflowIrForTask(taskStore, task.id).catch(() => undefined); + const declaredColumnIds = new Set( + ((ir as { columns?: Array<{ id?: unknown }> } | undefined)?.columns ?? []) + .map((c) => c?.id) + .filter((id): id is string => typeof id === "string"), + ); + const declared = new Set([ + ...Object.values(roles ?? {}).filter((v): v is string => typeof v === "string"), + ...declaredColumnIds, + ]); + const laneOr = (resolved: string | undefined, legacy: string): string | undefined => + resolved ?? (declared.has(legacy) ? undefined : legacy); + const lane = { + intake: laneOr(roles?.intake, "triage"), + hold: laneOr(roles?.hold, "todo"), + wip: laneOr(roles?.wip, "in-progress"), + review: laneOr(roles?.review, "in-review"), + complete: laneOr(roles?.complete, "done"), + archived: laneOr(roles?.archived, "archived"), + }; + + if ((lane.complete !== undefined && task.column === lane.complete)) { const blocker = await getTaskCompletionBlockerForStore(taskStore, task); if (blocker) { return { kind: "blocked", reason: blocker }; @@ -96,25 +156,58 @@ export async function reconcileMissionFeatureState( status untouched so a terminal/duplicate archive cannot fabricate roadmap progress; callers may still recompute hierarchy idempotently. */ - if (task.column === "archived") return { kind: "noop" }; + if ((lane.archived !== undefined && task.column === lane.archived)) return { kind: "noop" }; if ( - (task.column === "in-progress" || task.column === "in-review") + ((lane.wip !== undefined && task.column === lane.wip) || (lane.review !== undefined && task.column === lane.review)) && (feature.status === "triaged" || feature.status === "defined") ) { return { kind: "update", status: "in-progress", - reason: task.column === "in-review" + reason: (lane.review !== undefined && task.column === lane.review) ? `task ${task.id} is in review` : `task ${task.id} started`, }; } - if ( - (context.plannerColumns ?? LEGACY_PLANNER_COLUMNS).includes(task.column) - && feature.status === "in-progress" - ) { + /* + FNXC:MissionFeatureSyncLanes 2026-07-30-23:55 (rebase onto main's independent conversion): + MAIN converted this branch to `context.plannerColumns` while this PR converted it to the + task's own resolved lanes. Kept BOTH, because they answer different halves: a caller that + knows the board's planner columns should win, and a caller that does not should still get + the task's resolved lanes rather than the legacy pair. + + The ORPHANED-legacy-id acceptance is this PR's remaining contribution: a card resting in + `triage`/`todo` on a workflow that does NOT declare that id is a pre-#2515 row U11's + re-homing has not reached, and its feature must still return to `triaged`. Resolving lanes + alone would silently stop tracking those rows — the same going-silent failure this whole + conversion exists to fix. + + SCOPED to ids the workflow does not declare, per greptile on #2593: a custom workflow may + legitimately name a NON-planner lane `triage` (its review column), and mapping a card there + to `triaged` would misreport the roadmap. + */ + /* + FNXC:MissionFeatureSyncLanes 2026-07-31-00:10 (rebase onto main's independent conversion): + Three sources of truth, in priority order, and each is here for a reason main's version and + this PR's version each covered only half of: + 1. `context.plannerColumns` — a caller that KNOWS the board's planner columns (main's + conversion). Most specific, so it wins. + 2. the task's own resolved intake / hold roles (this PR's conversion), for callers that + pass no planner columns. + 3. an ORPHANED legacy id — a card resting in `triage`/`todo` on a workflow that does NOT + declare it. Those are pre-#2515 rows U11's re-homing has not reached; without this their + feature silently stops being tracked, which is the going-silent failure being fixed. + Scoped per greptile on #2593: a custom workflow may legitimately name a NON-planner lane + `triage`, and mapping a card there to `triaged` would misreport the roadmap. + */ + const declaresColumn = (id: string): boolean => Object.values(lane).includes(id) || declared.has(id); + const inPlannerLane = (context.plannerColumns ?? []).includes(task.column) + || (lane.intake !== undefined && task.column === lane.intake) + || (lane.hold !== undefined && task.column === lane.hold) + || (LEGACY_PLANNER_COLUMNS.includes(task.column) && !declaresColumn(task.column)); + if (inPlannerLane && feature.status === "in-progress") { return { kind: "update", status: "triaged",