From dca20496f4c5cc03d86906d63f5536ee545e2bbf Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 00:52:55 -0700 Subject: [PATCH] consolidate/u7: plugins to zero + 8 executor rebound guards + resume lanes (supersedes #2607, #2635, #2640) (#2644) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidation branch for U7, per the new one-branch working mode. **Supersedes #2607, #2635, #2640** — the three of my PRs that were stuck on review threads. My other seven (#2602, #2605, #2606, #2611, #2621, #2628, #2633) are green with **zero unresolved threads** and are deliberately left alone for the merge sweep. ## What is in here, file by file | file | change | guards before → after | |---|---|---| | `plugins/…/glasses/src/agent-actions.ts` | gates, destinations and degraded-resolution refusal all resolve from the task's own workflow | 2 → 0 | | `plugins/…/glasses/src/quick-capture.ts` | accepted capture columns come from the board; default no longer names the deleted column | 1 → 0 | | `plugins/…/glasses/src/settings.ts` | quick-capture default was `triage`, the column #2515 removed | (assignment, uncounted) | | `plugins/…/dependency-graph/src/GraphTaskNode.tsx` | redundant column condition deleted | 1 → 0 | | `packages/engine/src/executor.ts` | 8 rebound guards compare the resolved column; 4 resume-eligibility literals share one resolver | 151 → 143 (+4 off-bar) | | `packages/engine/src/__tests__/` | 4 new suites, 26 cases | — | `plugins/` reaches **zero** column guards with this branch. ## The three threads it closes **#2607 — five findings, all mine, all the same rule.** I kept *qualifying* a legacy-id fallback instead of removing it: | attempt | rule | hole review found | |---|---|---| | 1 | fall back to `todo` when the role is missing | moved cards to phantom columns | | 2 | …only if the workflow **declares** `todo` | aliased **review** lane named `todo` | | 3 | …and only if no other role is assigned to it | **traitless** parking column named `todo` | The qualifications were the mistake. Once `resolveLanes` returns a lane set the workflow *has* a column vocabulary, so "no column carries the hold trait" is a complete answer — refuse. `destination()` is two lines now, with no aliasing surface left to qualify. Plus a sixth, which is a genuinely different state: **degraded resolution is indistinguishable from the default board.** `resolveWorkflowIrForTask` is total by design — a missing definition silently returns the *default* coding IR — so a card on a custom board whose definition could not be read resolved to `todo`/`in-progress`. `undefined` lanes cannot express that (it means "no workflow at all", where the legacy ids *are* the answer). The actions now refuse with 409. #2618 would replace this check with resolver provenance; it is not merged, so this does not depend on it. **#2635 — "seven rebound sites remain untested."** Fair; my "same shape" note was an assertion, not coverage. Seven of the eight need a live graph run to reach, so the *shape* is pinned instead: a static check that no guard in front of a rebound move compares against a column literal, with a vacuity case (the same detection run against the original shape) and a match-count floor (≥8), because a guard reporting success on zero matches is worse than no guard. **#2640 — duplicate workflow resolution.** Framed as I/O; it is also a correctness bug. Eligibility and re-entry are two halves of one decision and resolved the workflow separately, so a workflow edit landing between them has the halves reading *different boards*. Now one caller-owned memo per decision — caller-owned because a process-lifetime cache would have to guess when a mid-flight workflow edit invalidates it. ## Behavioural findings, not tidying - **The last-resort recovery for completed-but-stranded work did not exist off the default lineage.** `promotedFromPlannerColumn` was false on a renamed board, so finished work resting in planning was never promoted; the code fell through to a review handoff that role adjacency rejects, and the card stayed stuck with its work complete. - **Rebound guards could not see the column their own move targeted.** U5b converted the move target; the eight `column !== "todo"` checks in front of it were left literal, so on a renamed board the engine moved a card into the column it was already in — and `moveTaskInternal` runs reset-on-entry on every real move, so at the `preserveProgress: false` site it reset step progress a second time. - **The FN-1404 `task:move` audit row was lying**, recording `to: "todo"` while the move target was resolved. A run-audit trail that disagrees with the move it describes is worse than none. Not a comparison, so no census counts it. - **A task interrupted by an engine pause never resumed on a renamed board** (off-bar, `in-review`/`in-progress` literals): four comparisons decided one question and had to agree; two of them disagreed on a renamed board, so re-entry silently never fired. ## Revert proofs, isolated per site | reverted | result | |---|---| | `destination()` back to attempt 3 | 3 of 38 fail | | degraded-resolution refusals removed | 2 of 42 fail | | capture set back to the legacy five | 2 of 3 fail (renamed-board suite) | | forward exclusions → literals | 1 of 14 fails | | missing-wip refusal removed | 2 of 14 fail | | `promotedFromPlannerColumn` → literals | 3 of 7 fail | | promotion target → `"in-progress"` | 3 of 7 fail | | one rebound guard → `!== "todo"` | 1 of 3 fails (static shape) | | resume lanes → legacy trio | 1 of 5 fails | Every conversion is paired with a negative — a forward move, a not-a-planner-lane card, a default-lineage card, an unresolvable workflow — so neither "always fire" nor "never fire" can pass for "resolve the role". ## Commit discipline Twelve commits, each one thing: the code move (`resolvePlannerLanes` out of `triage.ts`) is separate from every behavior change, and each review fix is its own commit with its own revert proof. ## Verification - `pnpm test:gate` **71/71** - 162/162 across the glasses plugin's 19 files; 26/26 across the four new engine suites - engine + glasses typecheck clean; `pnpm lint` clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit * **Bug Fixes** * Engine recovery and retries now work correctly with renamed or customized workflow columns. * Tasks in manual-intake columns are no longer automatically planned. * Agent actions and quick capture now respect each board’s declared columns and lifecycle stages. * Awaiting-approval tasks are recognized regardless of their current column. * Command Center SDLC funnel stages now accurately reflect customized workflows. * **Documentation** * Added guidance for safely changing workflow-column logic and interpreting lifecycle-column checks. --------- Co-authored-by: Claude Opus 5 (1M context) --- .changeset/executor-rebound-already-there.md | 7 + .changeset/executor-resume-lanes.md | 7 + .changeset/manual-intake-admission.md | 7 + .changeset/replan-target-resolved-fallback.md | 7 + .changeset/sdlc-funnel-project-columns.md | 7 + ...n-literal-can-widen-the-gate-it-guarded.md | 91 +++ .../sdlc-funnel-default-columns.test.ts | 130 ++++ packages/core/src/activity-analytics.ts | 30 +- .../routes/register-command-center-routes.ts | 42 ++ .../executor-rebound-already-there.test.ts | 143 ++++ .../executor-rebound-guard-shape.test.ts | 84 +++ .../executor-resume-lanes-resolved.test.ts | 136 ++++ .../__tests__/manual-intake-admission.test.ts | 166 +++++ packages/engine/src/__tests__/triage.test.ts | 16 +- packages/engine/src/executor.ts | 159 +++- packages/engine/src/mission-feature-sync.ts | 20 +- packages/engine/src/triage.ts | 45 +- .../src/GraphTaskNode.tsx | 34 +- .../src/__tests__/agent-actions.test.ts | 681 ++++++++++++++++++ .../quick-capture-renamed-board.test.ts | 78 ++ .../__tests__/quick-capture-routes.test.ts | 4 +- .../src/__tests__/quick-capture.test.ts | 335 ++++++++- .../src/__tests__/settings.test.ts | 12 +- .../src/agent-actions.ts | 243 ++++++- .../src/quick-capture.ts | 100 ++- .../src/settings.ts | 22 +- .../lib/lifecycle-column-census-baseline.json | 10 +- 27 files changed, 2551 insertions(+), 65 deletions(-) create mode 100644 .changeset/executor-rebound-already-there.md create mode 100644 .changeset/executor-resume-lanes.md create mode 100644 .changeset/manual-intake-admission.md create mode 100644 .changeset/replan-target-resolved-fallback.md create mode 100644 .changeset/sdlc-funnel-project-columns.md create mode 100644 docs/solutions/logic-errors/converting-a-column-literal-can-widen-the-gate-it-guarded.md create mode 100644 packages/core/src/__tests__/sdlc-funnel-default-columns.test.ts create mode 100644 packages/engine/src/__tests__/executor-rebound-already-there.test.ts create mode 100644 packages/engine/src/__tests__/executor-rebound-guard-shape.test.ts create mode 100644 packages/engine/src/__tests__/executor-resume-lanes-resolved.test.ts create mode 100644 packages/engine/src/__tests__/manual-intake-admission.test.ts create mode 100644 plugins/fusion-plugin-even-realities-glasses/src/__tests__/quick-capture-renamed-board.test.ts diff --git a/.changeset/executor-rebound-already-there.md b/.changeset/executor-rebound-already-there.md new file mode 100644 index 0000000000..d9f883d9a4 --- /dev/null +++ b/.changeset/executor-rebound-already-there.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Engine retries no longer re-queue a task into the column it is already sitting in on renamed boards. +category: fix +dev: Eight executor rebound guards compared `column !== "todo"` before moving to `resolveReboundColumnFor(...)`; they now resolve once and compare against that value. The FN-1404 `task:move` audit metadata records the resolved column instead of a hardcoded `"todo"`. diff --git a/.changeset/executor-resume-lanes.md b/.changeset/executor-resume-lanes.md new file mode 100644 index 0000000000..ed455f8f62 --- /dev/null +++ b/.changeset/executor-resume-lanes.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: A task interrupted by an engine pause now resumes on boards with renamed columns. +category: fix +dev: `reenterPausedAbortedWorkflowNode` resolves hold/wip/review once via a new `resolveResumeLanes` helper; `preservedInReview`, the audit `mode` label, the retry-callback recheck and the execute-vs-graph branch all read from it instead of four independent literals. diff --git a/.changeset/manual-intake-admission.md b/.changeset/manual-intake-admission.md new file mode 100644 index 0000000000..db97d73796 --- /dev/null +++ b/.changeset/manual-intake-admission.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Cards parked in the Coding (Ideas) intake are no longer auto-planned by the engine. +category: fix +dev: Triage discovery reads the intake trait's `autoTriage: false` from the same IR resolution and skips manual-intake columns; the hold branch is intentionally ungated. The pre-existing guard in `triage.test.ts` could not catch the regression because its mock store cannot resolve a workflow. diff --git a/.changeset/replan-target-resolved-fallback.md b/.changeset/replan-target-resolved-fallback.md new file mode 100644 index 0000000000..afdad418ad --- /dev/null +++ b/.changeset/replan-target-resolved-fallback.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Replans on boards without a Triage or Planning column now land in that board's own planning lane. +category: fix +dev: `resolveReplanTargetColumn` resolves the no-match path from the task's own workflow and returns undefined when it declares no planning lane, instead of returning the literal `"triage"`. diff --git a/.changeset/sdlc-funnel-project-columns.md b/.changeset/sdlc-funnel-project-columns.md new file mode 100644 index 0000000000..fbb16762ed --- /dev/null +++ b/.changeset/sdlc-funnel-project-columns.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: The Command Center SDLC funnel now reports stages for boards with renamed or custom columns. +category: fix +dev: `/command-center/activity` resolves the project's default-workflow columns and passes them as `SdlcFunnelQuery.columns`; unresolvable workflows fall through to the built-in default. `defaultColumns()` now uses `resolveDefaultWorkflowIr()` rather than the legacy monolithic constant. diff --git a/docs/solutions/logic-errors/converting-a-column-literal-can-widen-the-gate-it-guarded.md b/docs/solutions/logic-errors/converting-a-column-literal-can-widen-the-gate-it-guarded.md new file mode 100644 index 0000000000..a7fc22905d --- /dev/null +++ b/docs/solutions/logic-errors/converting-a-column-literal-can-widen-the-gate-it-guarded.md @@ -0,0 +1,91 @@ +--- +category: logic-errors +module: "@fusion/engine, @fusion/core, @fusion/dashboard" +date: 2026-07-31 +problem_type: logic_error +component: workflow-lifecycle-columns +severity: high +applies_when: + - "Replacing a `task.column === \"\"` comparison with a trait-resolved lifecycle role" + - "Converting a guard in triage discovery, hold release, self-healing, or a dashboard affordance" + - "Reviewing a lifecycle-column conversion PR" + - "A card started being auto-planned, auto-advanced, or auto-shown after a conversion that 'only renamed things'" +--- + +# Converting a column literal can WIDEN the gate it was guarding + +## The two failure directions + +A lifecycle-column conversion replaces "is this card in the column named X" with "is this card in the +column carrying role R". Both directions of error are common, and only one of them is obvious. + +**Direction 1 — narrower / phantom (the obvious one).** The gate resolves the role but a *destination* +or an *exemption* beside it stays literal. The guard admits a card on a renamed board and the move +then targets a column that board does not declare. Symptoms: `TransitionRejectionError`, a card in a +column nothing renders, or a recovery that reports failure after a partial move. This shape happened +four times in one plugin file and twice in `executor.ts` during Phase C. + +**Direction 2 — WIDER (the one that gets shipped).** The literal was holding a gate *shut* for a +workflow whose column simply did not match it. Resolving the role makes the predicate start matching, +so behaviour the literal excluded by accident now happens. Nothing errors. Nothing is rejected. The +engine just does more than it used to. + +## The incident + +`builtin:coding-ideas` exists so an operator can park a capture without the engine planning it +(FN-7596). That rule was enforced *accidentally*: triage discovery's admission predicate compared +against `"triage"`, and an `ideas` card matched no branch. + +Converting that predicate to resolve intake **by trait** made `ideas` the resolved intake column for +that workflow — so discovery began specifying parked ideas. Measured: `poll()` called `specifyTask` +on the parked card. The conversion was correct in vocabulary and wider in effect. + +The fix is not to revert the conversion. It is to make the *real* rule explicit: the intake trait +carries `autoTriage: false`, and a manual intake is never auto-admitted. The signal was in the IR the +whole time; the literal had been standing in for it. + +## Why the guarding test did not catch it + +`triage.test.ts` had a case named "excludes a parked ideas-column task from the poll's +specify-dispatch set". It passed before the conversion, passed after the rule broke, and passes today. + +Its store has **no workflow readers**. Lifecycle resolution therefore falls back to `triage`/`todo`, +an `ideas` card matches neither branch, and the card is excluded — for a reason that has nothing to do +with manual intake. The fixture could not reach the code path the test was named after. + +The tell was in its own comment, which still described the mechanism the conversion had removed +("`eligibleTriageTasks`, which only matches `column === "triage"`"). **A comment describing a +mechanism that no longer exists is evidence the test is no longer testing it.** + +## What to do when converting a column literal + +1. **Ask what the literal EXCLUDED**, not just what it matched. List the workflows whose columns did + not match it, and decide deliberately whether each should now be included. That question is what + distinguishes a rename from a behaviour change. +2. **Convert the destinations and the exemptions in the same commit.** A role-resolved rule with a + name-matched carve-out inverts the carve-out. +3. **Make the fixture resolve a workflow.** A store without `getTaskWorkflowSelection` / + `getWorkflowDefinition` cannot reach any trait-resolved branch, so a test built on one proves + nothing about the conversion. Drive the real builtin IR (`BUILTIN_CODING_IDEAS_WORKFLOW_IR`) or a + renamed fixture, and assert on both a renamed and a merged shape. +4. **Pair every conversion with a negative.** "Never fires" and "always fires" must both fail. For an + admission gate that means: the manual intake is refused, an AUTO intake still admits, and an + unresolvable workflow behaves as before. +5. **Re-read the comment above the test you are relying on.** If it names a mechanism the diff + removed, that test is now decoration. + +## Where the guards are + +`node scripts/lifecycle-column-census.mjs` reports the remaining comparisons, classified into column +guards (the backlog), agent-role comparisons, entity-status comparisons, and reviewed +`DELIBERATE-LITERAL` sites. Only the first class is convertible; converting a role comparison +reintroduces this bug's cousin, because the planner *lane* is named `triage` and keeps that name. + +## References + +- `packages/engine/src/__tests__/manual-intake-admission.test.ts` — the invariant, with the surface + enumeration and the measured pre-fix behaviour. +- `packages/engine/src/triage.ts` — `isAtIntakeColumn`, and why the hold branch is deliberately not + gated (a card in hold was RELEASED there). +- `docs/testing.md` → "Lifecycle-column census (report-only)" — the four classes and why they are + never netted into one number. diff --git a/packages/core/src/__tests__/sdlc-funnel-default-columns.test.ts b/packages/core/src/__tests__/sdlc-funnel-default-columns.test.ts new file mode 100644 index 0000000000..0b9a326780 --- /dev/null +++ b/packages/core/src/__tests__/sdlc-funnel-default-columns.test.ts @@ -0,0 +1,130 @@ +// @vitest-environment node +/* +FNXC:SdlcFunnelColumns 2026-07-31-09:50 (the default path was the only path, and it used the legacy IR): + +THE INVARIANT: the SDLC funnel's built-in column fallback maps the board Fusion actually ships. + +WHAT WAS WRONG. `defaultColumns()` used `BUILTIN_CODING_WORKFLOW_IR` — the constant the catalog now +publishes as `builtin:legacy-coding`, not the current default. That would be harmless if it were only a +fallback, but the ONLY production callers use it: `aggregateActivityAnalytics` (Command Center's +`/command-center/activity`, and the OTel exporter) never passes `columns`. Its own doc comment says +callers with a custom workflow "should call `aggregateSdlcFunnel` directly" — and none do, which is what +makes this the default path rather than an edge case. + +Consequence: any column id absent from the legacy set folds to OTHER, so a board whose columns differ +reads as an empty funnel while it is plainly busy. Same shape as the quick-capture default I fixed in +this branch: the explicit path was thought about, the default path was not. + +WHY THESE ASSERTIONS AND NOT A SNAPSHOT. The funnel's job is that every column of the shipped board maps +to a real stage — not that a particular id list is present. Asserting the mapping is what survives the +next lineage change; asserting the id list would have to be edited by whoever makes it, which is how a +test stops being evidence and becomes a chore. +*/ +import { describe, expect, it } from "vitest"; + +import { buildColumnStageMap } from "../activity-analytics.js"; +import { resolveDefaultWorkflowIr } from "../builtin-workflows.js"; +import { BUILTIN_CODING_WORKFLOW_IR } from "../builtin-coding-workflow-ir.js"; +import type { WorkflowIrColumn } from "../workflow-ir-types.js"; + +function columnsOf(ir: unknown): Array<{ id: string; traits: Array<{ trait: string }> }> { + const v2 = ir as { version?: string; columns?: WorkflowIrColumn[] }; + return (v2.columns ?? []).map((column) => ({ + id: column.id, + traits: column.traits.map((trait) => ({ trait: trait.trait })), + })); +} + +describe("the funnel's built-in column fallback tracks the SHIPPED default board", () => { + it("maps every FUNNEL column of the current default workflow to a stage, not OTHER", () => { + const columns = columnsOf(resolveDefaultWorkflowIr()); + const stageMap = buildColumnStageMap(columns); + + expect(columns.length).toBeGreaterThan(0); + /* + `archived` is EXCLUDED, and my first version of this assertion got that wrong — it failed on the + archived column and I nearly read that as a product bug. The funnel's stages are the SDLC path + (triage -> todo -> in-progress -> in-review -> done); archived is not a stage on it, so folding to + `other` there is correct. Measured mapping on the shipped board: + todo -> triage (intake/hold) | in-progress -> in-progress | in-review -> in-review | done -> done + Note `todo` maps to the `triage` STAGE because it carries the intake trait — the post-U11 merged + planning column, which is exactly what the legacy fallback could not express. + */ + const funnelColumns = columns.filter((column) => !column.traits.some((t) => t.trait === "archived")); + expect(funnelColumns.length).toBeGreaterThan(0); + for (const column of funnelColumns) { + expect(stageMap.get(column.id)).not.toBe("other"); + } + }); + + it("covers the post-U11 merged planning column, which the legacy constant predates", () => { + // The concrete regression: #2515 merged Todo into Planning on the default lineage. A fallback built + // from the legacy IR still describes a board with a separate `triage` column. + const currentIds = columnsOf(resolveDefaultWorkflowIr()).map((c) => c.id); + const legacyIds = columnsOf(BUILTIN_CODING_WORKFLOW_IR).map((c) => c.id); + + expect(currentIds).not.toEqual(legacyIds); + // Whatever the current lineage's ids are, they are the ones the funnel must map. + for (const id of currentIds) { + expect(buildColumnStageMap(columnsOf(resolveDefaultWorkflowIr())).has(id)).toBe(true); + } + }); + + it("folds a column no board declares into OTHER (the paired negative)", () => { + // The fold-to-OTHER behaviour is correct and must survive: it is what keeps an unknown id from + // being counted as a real stage. This is why the fix is "use the right default", not "stop folding". + const stageMap = buildColumnStageMap(columnsOf(resolveDefaultWorkflowIr())); + + expect(stageMap.has("a-column-no-board-has")).toBe(false); + }); +}); + +/* +FNXC:SdlcFunnelColumns 2026-07-31-10:25 (the real fix, and the correction of my own claim): + +I first changed only `defaultColumns()` from the legacy IR to `resolveDefaultWorkflowIr()` and described +it as fixing renamed boards. IT DOES NOT. Post-U11 the current lineage's column ids are a SUBSET of the +legacy constant's, so the legacy fallback already mapped everything the current one does — the change is +a consistency fix with no observable effect. The give-away was a revert proof that would not go red: my +assertions passed with the legacy constant restored. + +THE REAL DEFECT is that the funnel maps by column id and folds anything it was not given into OTHER, +while the only production callers never passed `columns` at all. So a renamed or custom board's Command +Center funnel read as EMPTY while the board was plainly busy. Fixing that is a CALLER change — the route +resolves the project's own workflow columns — and these cases pin the property that makes it work. +*/ +describe("a board whose columns the funnel was not given folds to OTHER", () => { + const renamedColumns = [ + { id: "backlog", traits: [{ trait: "intake" }] }, + { id: "building", traits: [{ trait: "wip" }] }, + { id: "checking", traits: [{ trait: "merge" }] }, + { id: "shipped", traits: [{ trait: "complete" }] }, + ]; + + it("maps a renamed board's columns once they ARE supplied", () => { + const stageMap = buildColumnStageMap(renamedColumns); + + for (const column of renamedColumns) { + expect(stageMap.get(column.id)).not.toBe("other"); + } + }); + + it("cannot see a renamed board's columns through the built-in default (why the caller must supply them)", () => { + // This is the operator-visible failure, stated as a test rather than as prose: with only the + // built-in columns, every id from a renamed board is unknown, and unknown folds to OTHER. + const builtinOnly = buildColumnStageMap(columnsOf(resolveDefaultWorkflowIr())); + + for (const column of renamedColumns) { + expect(builtinOnly.has(column.id)).toBe(false); + } + }); + + it("treats an EMPTY column list as no mapping at all, which is why the resolver returns undefined", () => { + // The route's helper returns `undefined` rather than `[]` on failure: undefined falls back to the + // built-in default (previous behaviour), while `[]` would map every column to OTHER and silently + // empty the funnel. Pinning the distinction because it is easy to "simplify" away. + const empty = buildColumnStageMap([]); + + expect(empty.size).toBe(0); + }); +}); diff --git a/packages/core/src/activity-analytics.ts b/packages/core/src/activity-analytics.ts index 502a7a60b6..4fbd4d4864 100644 --- a/packages/core/src/activity-analytics.ts +++ b/packages/core/src/activity-analytics.ts @@ -4,7 +4,7 @@ const severityAuditLog = createLogger("core-activity-analytics"); import { sql } from "drizzle-orm"; import type { Database } from "./db.js"; import type { AsyncDataLayer } from "./postgres/data-layer.js"; -import { BUILTIN_CODING_WORKFLOW_IR } from "./builtin-coding-workflow-ir.js"; +import { resolveDefaultWorkflowIr } from "./builtin-workflows.js"; import type { WorkflowIrColumn } from "./workflow-ir-types.js"; /** @@ -212,7 +212,7 @@ function rangeClauses( */ export async function aggregateActivityAnalytics( dbOrLayer: Database | AsyncDataLayer, - query: ActivityAnalyticsQuery = {}, + query: SdlcFunnelQuery = {}, ): Promise { // FNXC:RuntimeSatelliteAsync 2026-06-24-13:45: // The activity analytics queries (sessions, messages, nodes, agents, daily @@ -713,8 +713,32 @@ interface MoveRow { ts: string; } +/* +FNXC:SdlcFunnelColumns 2026-07-31-09:40: +THE FALLBACK WAS THE LEGACY MONOLITHIC IR, and the only production callers use the fallback. +`aggregateActivityAnalytics` (Command Center's `/command-center/activity`, and the OTel exporter) never +passes `columns`, so every project's funnel was mapped through `BUILTIN_CODING_WORKFLOW_IR` — the +constant the catalog now publishes as `builtin:legacy-coding`, not the current default. The same +legacy-constant-vs-catalog split produced the "preflight is stale" drift in the move resolvers. + +Consequence: any column id absent from that legacy set folds to OTHER, so a renamed or custom board's +Command Center funnel reads as empty while the board is plainly busy. The doc comment says callers with +a custom workflow "should call `aggregateSdlcFunnel` directly"; no caller does, which is what makes this +the default path rather than an edge case. + +`resolveDefaultWorkflowIr()` is the shared authority every other default resolution uses, so the +built-in fallback now agrees with the board Fusion actually ships. + +SCOPE, corrected after I overstated it: post-U11 the current lineage's column ids are a SUBSET of the +legacy constant's, so this change alone fixes NO renamed board — it only stops the fallback describing a +board Fusion no longer ships (a consistency fix, and it is why `defaultColumns` cannot have a +behaviour-revealing test on its own). The renamed/custom case is fixed by the CALLER passing `columns`, +which is why `aggregateActivityAnalytics` now accepts them and the Command Center route resolves the +project's own workflow. I nearly shipped the consistency change as if it were the whole fix; the +give-away was a revert proof that would not go red. +*/ function defaultColumns(): FunnelColumnTraitSource[] { - const ir = BUILTIN_CODING_WORKFLOW_IR; + const ir = resolveDefaultWorkflowIr(); if (ir.version === "v2") { return (ir.columns as WorkflowIrColumn[]).map((c) => ({ id: c.id, diff --git a/packages/dashboard/src/routes/register-command-center-routes.ts b/packages/dashboard/src/routes/register-command-center-routes.ts index 47b9d3dd9a..f77805400e 100644 --- a/packages/dashboard/src/routes/register-command-center-routes.ts +++ b/packages/dashboard/src/routes/register-command-center-routes.ts @@ -1,4 +1,5 @@ import { + resolveWorkflowIrById, aggregateTokenAnalytics, aggregateToolAnalytics, aggregateActivityAnalytics, @@ -138,6 +139,38 @@ export function resolveTokenGranularity(query: Request["query"]): TokenTimeGranu } /** True when the caller asked for CSV via `?format=csv` (case-insensitive). */ +/* +FNXC:SdlcFunnelColumns 2026-07-31-10:15: +The project's own workflow columns, in the shape the funnel's trait mapping needs. Resolved from the +project's DEFAULT workflow, which is the board the activity log's column ids come from. + +Returns `undefined` — not an empty array — when nothing resolves: undefined lets the core fall back to +the built-in default (the previous behaviour), while `[]` would map EVERY column to OTHER and silently +empty the funnel. That distinction is the whole reason this returns optionally. +*/ +async function resolveFunnelColumnsForProject( + store: { getDefaultWorkflowId?: () => Promise }, +): Promise }> | undefined> { + try { + const workflowId = await store.getDefaultWorkflowId?.(); + if (!workflowId) return undefined; + const ir = await resolveWorkflowIrById(store as never, workflowId); + const columns = (ir as { columns?: Array<{ id?: unknown; traits?: Array<{ trait?: unknown }> }> }).columns ?? []; + const mapped = columns + .filter((column): column is { id: string; traits?: Array<{ trait?: unknown }> } => typeof column?.id === "string") + .map((column) => ({ + id: column.id, + traits: (column.traits ?? []) + .map((trait) => trait?.trait) + .filter((trait): trait is string => typeof trait === "string") + .map((trait) => ({ trait })), + })); + return mapped.length > 0 ? mapped : undefined; + } catch { + return undefined; + } +} + export function wantsCsv(query: Request["query"]): boolean { const raw = typeof query.format === "string" ? query.format : undefined; return raw !== undefined && raw.toLowerCase() === "csv"; @@ -275,9 +308,18 @@ export const registerCommandCenterRoutes: ApiRouteRegistrar = (ctx) => { try { const store = await getScopedStore(req); const range = resolveRange(req.query); + /* + FNXC:SdlcFunnelColumns 2026-07-31-10:10: + PASS THE PROJECT'S OWN COLUMNS. The funnel maps column ids to SDLC stages by trait, and any id it + was not given folds to OTHER — so without this the Command Center funnel on a renamed or custom + board read as empty while the board was plainly busy. `aggregateSdlcFunnel`'s doc comment already + said callers with a custom workflow should supply their columns; this is the caller, and it did + not. Unresolvable workflow falls through to the built-in default, which is the previous behaviour. + */ const result = await aggregateActivityAnalytics(requireAsyncLayer(store, "Command Center activity analytics"), { from: range.from, to: range.to, + columns: await resolveFunnelColumnsForProject(store), }); if (wantsCsv(req.query)) { sendCsv(res, "command-center-activity.csv", activityAnalyticsToTable(result)); diff --git a/packages/engine/src/__tests__/executor-rebound-already-there.test.ts b/packages/engine/src/__tests__/executor-rebound-already-there.test.ts new file mode 100644 index 0000000000..ff2c058698 --- /dev/null +++ b/packages/engine/src/__tests__/executor-rebound-already-there.test.ts @@ -0,0 +1,143 @@ +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-15:30 (Phase C convergence — executor rebound guards): + +THE INVARIANT: an engine rebound moves the card only when it is NOT already in the column the +rebound resolves to — on ANY workflow, not just the default lineage. + +WHAT WAS BROKEN. `resolveReboundColumnFor` was converted in U5b, so every rebound MOVE targeted +the workflow's own backlog column. The eight `X.column !== "todo"` guards standing in front of +those moves were not converted. On a renamed board the guard was therefore always true, and the +engine issued a move into the column the card was already sitting in. + +WHY THAT IS NOT HARMLESS. `moveTaskInternal` runs the reset-on-entry effects on every real move. +So the redundant move re-cleared status/error/pause state, and at the stale-parse-pins site +(`preserveProgress: false`) it RESET STEP PROGRESS a second time on a card that had merely been +re-checked. The half-conversion shape again: the right target, reached through a check that could +not see it. + +This drives `parkCompletedBlockedTask` — the FN-7926 completed-but-blocked park — because it is +the rebound site reachable without standing up a graph run. The other seven sites take the +identical shape and are covered by the same predicate; that is stated rather than implied. +*/ +import { describe, expect, it, vi } from "vitest"; +import "./executor-test-helpers.js"; +import { TaskExecutor } from "../executor.js"; +import { createMockStore } from "./executor-test-helpers.js"; +import type { WorkflowIr } from "@fusion/core"; + +/** Standard traits, non-default names: the backlog role lives on `queued`. */ +const RENAMED_IR = { + version: "v2", id: "wf-renamed", name: "renamed", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }] }, + { id: "queued", name: "Queued", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "checking", name: "Checking", traits: [{ trait: "merge" }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], +} as unknown as WorkflowIr; + +function blockedCompletedTask(column: string) { + return { + id: "FN-BLOCKED", + title: "completed but blocked", + description: "", + column, + worktree: "/repo/.worktrees/blocked", + branch: "fusion/fn-blocked", + steps: [{ name: "Implement", status: "done" as const }], + currentStep: 0, + dependencies: [], + log: [], + createdAt: "2026-07-30T00:00:00.000Z", + updatedAt: "2026-07-30T00:00:00.000Z", + }; +} + +function harness(ir: WorkflowIr | undefined, column: string) { + const store = createMockStore(); + let task: Record = blockedCompletedTask(column); + const moves: Array<[string, string]> = []; + + const selection = { workflowId: "wf-renamed", stepIds: [] as string[] }; + const widened = store as unknown as Record; + widened.getTaskWorkflowSelection = () => (ir ? selection : undefined); + widened.getTaskWorkflowSelectionAsync = async () => (ir ? selection : undefined); + widened.getWorkflowDefinition = async () => (ir ? { ir } : undefined); + + store.getTask.mockImplementation(async () => ({ ...task })); + store.updateTask.mockImplementation(async (_id: string, updates: Record) => { + task = { ...task, ...updates }; + return task; + }); + store.moveTask.mockImplementation(async (id: string, to: string) => { + moves.push([id, to]); + task = { ...task, column: to }; + return { ...task }; + }); + store.recordRunAuditEvent = vi.fn().mockResolvedValue(undefined); + + const executor = new TaskExecutor(store as never, "/repo"); + const park = (t: Record) => + (executor as unknown as { + parkCompletedBlockedTask: (task: unknown, blocker: string, source: string, workComplete?: boolean) => Promise; + }).parkCompletedBlockedTask(t, "unmet dependency FN-OTHER", "test", true); + + return { store, moves, park }; +} + +describe("an engine rebound does not move a card that is already in its rebound column", () => { + it("issues NO move when the renamed board's backlog column already holds the card", async () => { + // Pre-fix: `queued !== "todo"` was true, so a move into `queued` was issued — a real move, + // which re-runs the reset-on-entry effects on a card nothing had actually moved. + const h = harness(RENAMED_IR, "queued"); + + const parked = await h.park(blockedCompletedTask("queued")); + + expect(parked).toBe(true); + expect(h.moves).toEqual([]); + }); + + it("DOES move when the card is elsewhere on the renamed board", async () => { + // The paired positive: "never move" must not be able to pass for "compare properly". + const h = harness(RENAMED_IR, "building"); + + await h.park(blockedCompletedTask("building")); + + expect(h.moves).toEqual([["FN-BLOCKED", "queued"]]); + }); + + it("still skips the redundant move on the default lineage", async () => { + const h = harness(undefined, "todo"); + + await h.park(blockedCompletedTask("todo")); + + expect(h.moves).toEqual([]); + }); + + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-16:20 (PR #2644 review, CodeRabbit): + THIS FIXTURE HAD TO DIFFER FROM THE DEFAULT-LINEAGE ONE. Both used `harness(undefined, ...)`, which + supplies no workflow SELECTION at all — so "the default lineage" and "the workflow cannot be resolved" + were the same setup, and this case could pass without ever exercising a selection whose definition lookup + fails. Two tests with identical fixtures are one test with two names. + + It now names a workflow whose definition read THROWS, which is the state that actually reaches the + resolver's fail-soft path in production (a deleted or unreadable definition row, not an absent + selection). + */ + it("still moves to the legacy backlog when a SELECTED workflow's definition cannot be read", async () => { + const h = harness(undefined, "in-progress"); + const selection = { workflowId: "wf-unreadable", stepIds: [] as string[] }; + const widened = h.store as unknown as Record; + widened.getTaskWorkflowSelection = () => selection; + widened.getTaskWorkflowSelectionAsync = async () => selection; + widened.getWorkflowDefinition = async () => { + throw new Error("definition row is gone"); + }; + + await h.park(blockedCompletedTask("in-progress")); + + expect(h.moves).toEqual([["FN-BLOCKED", "todo"]]); + }); +}); diff --git a/packages/engine/src/__tests__/executor-rebound-guard-shape.test.ts b/packages/engine/src/__tests__/executor-rebound-guard-shape.test.ts new file mode 100644 index 0000000000..56fa65f168 --- /dev/null +++ b/packages/engine/src/__tests__/executor-rebound-guard-shape.test.ts @@ -0,0 +1,84 @@ +// @vitest-environment node + +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-18:55 (PR #2635 review, greptile P2): + +"Seven rebound sites remain untested" — fair, and stating it as a coverage note was not an answer. +Seven of the eight sit inside graph-failure / stuck-kill / dependency-gate paths that need a live +graph run to reach, so behavioural coverage for each would cost more scaffolding than the change +itself. What they share is a SHAPE, so the shape is what gets pinned. + +This is a static check over `executor.ts`: every guard in front of a rebound move must compare +against a RESOLVED value, never a column literal. It fails on the exact defect the PR fixes — a +`column !== "todo"` check standing in front of a `moveTask(..., reboundColumn)` — at whichever of +the eight sites it is reintroduced, including sites added later that no behavioural test knows about. + +It is a static check and is labelled as one: it proves the pattern is absent, not that each path +behaves. `executor-rebound-already-there.test.ts` carries the behavioural proof for the one +reachable site (`parkCompletedBlockedTask`). + +It lives in its OWN file because the shared executor test helpers mock `node:fs`, so a suite that +imports them cannot read source off disk — a detail worth writing down, since the failure looks +like a broken path rather than a mocked module. +*/ +import { describe, expect, it } from "vitest"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { dirname, join } from "node:path"; + +describe("no rebound move is guarded by a column literal", () => { + const source = readFileSync( + join(dirname(fileURLToPath(import.meta.url)), "..", "executor.ts"), + "utf8", + ); + /** Comments blanked in place so prose about the old shape is not read as code. */ + const code = source + .replace(/\/\*[\s\S]*?\*\//g, (comment) => comment.replace(/[^\n]/g, " ")) + .replace(/\/\/.*$/gm, ""); + const lines = code.split("\n"); + + it("resolves the rebound column before every guarded rebound move", () => { + const offenders: string[] = []; + + lines.forEach((line, index) => { + if (!/moveTask\([^)]*reboundColumn/.test(line) && !/reboundColumn,/.test(line)) return; + // Walk back a few lines to the guard that admits this move. + for (let i = Math.max(0, index - 6); i <= index; i += 1) { + if (/column\s*(?:===|!==)\s*["'](?:todo|triage|in-progress|in-review|done|archived)["']/.test(lines[i] ?? "")) { + offenders.push(`${i + 1}: ${(lines[i] ?? "").trim()}`); + } + } + }); + + expect(offenders).toEqual([]); + }); + + it("finds the defect when it is reintroduced (the check is not vacuous)", () => { + // Same detection, run over a fixture carrying the original shape. Without this, a regex that + // silently stopped matching would report a clean file forever. + const reintroduced = [ + ` if (task.column !== "todo") {`, + ` await this.store.moveTask(task.id, reboundColumn, { preserveProgress: true });`, + ` }`, + ]; + const offenders: string[] = []; + + reintroduced.forEach((line, index) => { + if (!/moveTask\([^)]*reboundColumn/.test(line)) return; + for (let i = Math.max(0, index - 6); i <= index; i += 1) { + if (/column\s*(?:===|!==)\s*["'](?:todo|triage|in-progress|in-review|done|archived)["']/.test(reintroduced[i] ?? "")) { + offenders.push(String(i + 1)); + } + } + }); + + expect(offenders).toEqual(["1"]); + }); + + it("still sees the eight rebound moves it is meant to cover", () => { + // A guard that reports success on zero matches is worse than no guard. + const reboundMoves = lines.filter((line) => /moveTask\([^)]*(?:rebound|Rebound)Column/.test(line)); + + expect(reboundMoves.length).toBeGreaterThanOrEqual(8); + }); +}); diff --git a/packages/engine/src/__tests__/executor-resume-lanes-resolved.test.ts b/packages/engine/src/__tests__/executor-resume-lanes-resolved.test.ts new file mode 100644 index 0000000000..0f328a17ac --- /dev/null +++ b/packages/engine/src/__tests__/executor-resume-lanes-resolved.test.ts @@ -0,0 +1,136 @@ +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-21:10 (Phase C convergence — resume eligibility): + +THE INVARIANT: the columns a paused-node RESUME may start from are the task's own hold, wip and +review lanes. + +Four literal comparisons decided that one question and had to agree with each other: +`preservedInReview`, the audit `mode` label, the resume-safety recheck inside the retry callback, +and the branch choosing `execute()` versus `executeWorkflowGraph()`. On a renamed board +`preservedInReview` was false for a card sitting in review AND the recheck rejected it, so the +paused-node re-entry silently never happened — an engine pause/resume left the card parked with +nothing to resume it. + +OFF THE `triage` BAR, deliberately: these are `in-review`/`in-progress`/`todo` guards. Same defect +class, different literals; recorded here so the next sweep of that class has a worked example and a +shared resolver to reuse. +*/ +import { describe, expect, it, vi } from "vitest"; +import "./executor-test-helpers.js"; +import { TaskExecutor } from "../executor.js"; +import { createMockStore } from "./executor-test-helpers.js"; +import type { WorkflowIr } from "@fusion/core"; + +const RENAMED_IR = { + version: "v2", id: "wf-renamed", name: "renamed", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }] }, + { id: "queued", name: "Queued", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "checking", name: "Checking", traits: [{ trait: "merge" }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], +} as unknown as WorkflowIr; + +function harness(ir: WorkflowIr | undefined) { + const store = createMockStore(); + const selection = { workflowId: "wf-renamed", stepIds: [] as string[] }; + const widened = store as unknown as Record; + widened.getTaskWorkflowSelection = () => (ir ? selection : undefined); + widened.getTaskWorkflowSelectionAsync = async () => (ir ? selection : undefined); + widened.getWorkflowDefinition = async () => (ir ? { ir } : undefined); + store.recordRunAuditEvent = vi.fn().mockResolvedValue(undefined); + + const executor = new TaskExecutor(store as never, "/repo"); + const lanes = (taskId: string) => + (executor as unknown as { + resolveResumeLanes: (id: string) => Promise<{ hold: string; wip: string; review: string }>; + }).resolveResumeLanes(taskId); + + return { store, executor, lanes }; +} + +describe("one lane snapshot per recovery decision", () => { + it("resolves the workflow ONCE when a memo is shared, not once per half", async () => { + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-01:10 (PR #2640 review, greptile P2): + Eligibility and re-entry are two halves of the SAME decision. Resolving separately is not just + extra I/O: a workflow edit landing between the two calls would have the halves reading DIFFERENT + boards — eligibility admits a card in review, then re-entry resolves a board where that column + is not the review lane. The memo makes that impossible by construction. + */ + const h = harness(RENAMED_IR); + let reads = 0; + (h.store as unknown as Record).getWorkflowDefinition = async () => { + reads += 1; + return { ir: RENAMED_IR }; + }; + const memo: { lanes?: { hold: string; wip: string; review: string } } = {}; + const resolve = (executorOf: typeof h.executor) => + (executorOf as unknown as { + resolveResumeLanes: (id: string, memo?: unknown) => Promise<{ hold: string }>; + }).resolveResumeLanes("FN-1", memo); + + const first = await resolve(h.executor); + const second = await resolve(h.executor); + + expect(first).toEqual(second); + expect(reads).toBe(1); + }); + + it("resolves per call when no memo is passed, so callers cannot share a stale snapshot by accident", async () => { + // The memo is CALLER-OWNED on purpose: a process-lifetime cache would have to guess when a + // mid-flight workflow edit invalidates it. Without a memo each call is independent. + const h = harness(RENAMED_IR); + let reads = 0; + (h.store as unknown as Record).getWorkflowDefinition = async () => { + reads += 1; + return { ir: RENAMED_IR }; + }; + + await h.lanes("FN-1"); + await h.lanes("FN-1"); + + expect(reads).toBe(2); + }); +}); + +describe("resume lanes come from the task's own workflow", () => { + it("resolves the renamed hold, wip and review columns", async () => { + // Pre-fix these three were the default lineage's names, so every resume-safety comparison on a + // renamed board answered "not a safe resume state" and the re-entry never fired. + const h = harness(RENAMED_IR); + + await expect(h.lanes("FN-1")).resolves.toEqual({ + hold: "queued", + wip: "building", + review: "checking", + }); + }); + + 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. + const h = harness(undefined); + + await expect(h.lanes("FN-1")).resolves.toEqual({ + hold: "todo", + wip: "in-progress", + review: "in-review", + }); + }); + + it("never throws, so a resume decision is never blocked on IR resolution", async () => { + // The re-entry path runs inside a retry callback; a throw here would strand the card silently. + const h = harness(RENAMED_IR); + (h.store as unknown as Record).getWorkflowDefinition = async () => { + throw new Error("workflow store unavailable"); + }; + + await expect(h.lanes("FN-1")).resolves.toEqual({ + hold: "todo", + wip: "in-progress", + review: "in-review", + }); + }); +}); diff --git a/packages/engine/src/__tests__/manual-intake-admission.test.ts b/packages/engine/src/__tests__/manual-intake-admission.test.ts new file mode 100644 index 0000000000..1f4bd8c5e6 --- /dev/null +++ b/packages/engine/src/__tests__/manual-intake-admission.test.ts @@ -0,0 +1,166 @@ +/* +FNXC:ManualIntakeAdmission 2026-07-31-04:35 (live bug, found while scoping the coding-ideas merge): + +THE INVARIANT: a card sitting at a MANUAL intake (`autoTriage: false`) is never auto-planned. Coding +(Ideas) exists so an operator can park a card without the engine touching it; the operator promotes +it into Planning when ready (FN-7596). + +IT WAS BROKEN, and it was broken BY the trait conversion. The rule used to be enforced accidentally, +by discovery's predicate naming `triage`: an `ideas` card matched no branch. Converting that predicate +to resolve intake BY TRAIT made `ideas` the resolved intake column for that workflow — so discovery +began specifying parked ideas. Same shape as every other half-conversion in this program: correct in +vocabulary, wrong in effect, and wider than before. + +MEASURED BEFORE THE FIX, with a store that can actually resolve the workflow: `poll()` called +`specifyTask` once, with the parked `ideas` card. + +WHY THE EXISTING GUARD MISSED IT. `triage.test.ts`'s "excludes a parked ideas-column task from the +poll's specify-dispatch set" builds a mock store with NO workflow readers, so lifecycle resolution +falls back to `triage`/`todo` and an `ideas` card matches neither branch. It passes for a reason +unrelated to the rule and kept passing after the rule broke. Its own comment still describes the old +mechanism — "`eligibleTriageTasks`, which only matches `column === "triage"`" — which is the tell. + +So this file's store RESOLVES the workflow. That single difference is the whole point: a test whose +fixture cannot reach the code path proves nothing about it. + +SURFACE ENUMERATION (AGENTS.md requires this for a bug-class fix; here is what was checked, not +assumed): + + 1. TRIAGE DISCOVERY -> `specifyTask`. BOTH dispatch sites (`triage.ts` ~536 via the admission + coordinator, and ~1985 via the poll) call `discoverReadyPlanningTasks`, so the gate has ONE + home. Covered by this suite, driving `poll()`. + 2. HOLD-RELEASE into WIP (`issueRelease` / `reserveSlot` / `promoteHeldTask` / + `releaseHeldTaskByEvent`). Safe already: `isUnplannedForExecution` holds a card whose PROMPT.md + is still the bootstrap stub while it rests in an `intake`-trait column, and a parked idea is by + definition unplanned. Nothing to change — the release surfaces share that one predicate. + 3. SELF-HEALING's stranded-hold continuation. Candidate requires a REAL spec; a parked idea has a + bootstrap stub, so it is not a candidate. + 4. SCHEDULER dispatch. Reads WIP-bound work from the hold column, and reaches a card only after + release, which (2) gates. + 5. OPERATOR-TRIGGERED re-specification (a user comment on a parked card). Deliberately NOT gated: + that is an operator acting on their own card, which is the promotion path, not auto-planning. + +CONSEQUENCE FOR THE CODING-IDEAS MERGE (ideas + todo -> one Planning column): it cannot be reasoned +about until this signal exists, because merging makes `intake === hold` and the two admission branches +collapse onto one column. With `manualIntake` honoured, a merged manual lane parks correctly and is +released by promotion; without it, merging would auto-plan every captured card. Recorded here because +the merge is owed work and this is its blocker. +*/ +import { describe, expect, it, vi } from "vitest"; +import { EventEmitter } from "node:events"; +import type { Task, TaskStore, WorkflowIr } from "@fusion/core"; +import { BUILTIN_CODING_IDEAS_WORKFLOW_IR } from "@fusion/core"; + +import { TriageProcessor } from "../triage.js"; + +function parkedTask(overrides: Partial = {}): Task { + return { + id: "FN-IDEAS", + description: "a parked idea", + column: "ideas", + dependencies: [], + steps: [], + currentStep: 0, + log: [], + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + ...overrides, + } as unknown as Task; +} + +/** A store that CAN resolve the workflow — the difference that makes this suite able to fail. */ +function resolvingStore(tasks: Task[], ir: WorkflowIr, workflowId = "builtin:coding-ideas"): TaskStore { + const selection = { workflowId, stepIds: [] as string[] }; + return Object.assign(new EventEmitter(), { + listTasks: vi.fn().mockResolvedValue(tasks), + getTask: vi.fn(async (id: string) => tasks.find((t) => t.id === id)), + getSettings: vi.fn().mockResolvedValue({ + maxConcurrent: 10, + maxWorktrees: 4, + pollIntervalMs: 10_000, + autoMerge: true, + }), + logEntry: vi.fn().mockResolvedValue(undefined), + updateTask: vi.fn().mockResolvedValue(undefined), + withTaskLock: vi.fn(async (_id: string, callback: () => unknown) => callback()), + getTaskWorkflowSelection: () => selection, + getTaskWorkflowSelectionAsync: async () => selection, + getWorkflowDefinition: async () => ({ ir }), + recordRunAuditEvent: vi.fn().mockResolvedValue(undefined), + }) as unknown as TaskStore; +} + +async function pollWith(store: TaskStore): Promise { + const processor = new TriageProcessor(store, "/tmp/manual-intake-admission"); + const specify = vi.spyOn(processor, "specifyTask").mockResolvedValue(undefined); + (processor as unknown as { running: boolean }).running = true; + await (processor as unknown as { poll: () => Promise }).poll(); + return specify.mock.calls.map((call) => (call[0] as Task)?.id); +} + +describe("a card parked at a manual intake is never auto-planned", () => { + it("does not specify a parked `ideas` card on the real Coding (Ideas) workflow", async () => { + // Pre-fix, measured: this specified FN-IDEAS. The operator's parked capture was planned for them. + const specified = await pollWith( + resolvingStore([parkedTask()], BUILTIN_CODING_IDEAS_WORKFLOW_IR), + ); + + expect(specified).toEqual([]); + }); + + it("DOES specify a card the operator promoted into Planning", async () => { + /* + The paired positive, and the reason the fix is scoped to the INTAKE branch: a card in the hold + column was released there by an operator or by finalize. "Manual intake" says nothing about a card + that has already left it, so gating the hold branch too would park Coding (Ideas) permanently. + */ + const promoted = parkedTask({ id: "FN-PROMOTED", column: "todo" } as Partial); + const specified = await pollWith( + resolvingStore([promoted], BUILTIN_CODING_IDEAS_WORKFLOW_IR), + ); + + expect(specified).toEqual(["FN-PROMOTED"]); + }); + + it("still auto-plans an AUTO intake, so the fix is not 'never admit at intake'", async () => { + // The default lineage's intake carries no `autoTriage: false`, and post-U11 it is the same column + // as the hold lane. A fix that keyed on "is intake" rather than "is MANUAL intake" would have + // stopped the default board planning anything. + const autoIntakeIr = { + version: "v2", id: "wf-auto", name: "auto-intake", nodes: [], edges: [], + columns: [ + { + id: "todo", + name: "Planning", + traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }], + }, + { id: "in-progress", name: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "in-review", name: "Review", traits: [{ trait: "merge" }] }, + { id: "done", name: "Done", traits: [{ trait: "complete" }] }, + ], + } as unknown as WorkflowIr; + + const specified = await pollWith( + resolvingStore([parkedTask({ id: "FN-AUTO", column: "todo" } as Partial)], autoIntakeIr, "wf-auto"), + ); + + expect(specified).toEqual(["FN-AUTO"]); + }); + + it("keeps admitting on a workflow that cannot be resolved at all", async () => { + // No workflow readers is the legacy shape: lifecycle falls back to `triage`/`todo`, and a card + // there must still be planned. This is also, precisely, why the pre-existing guard in + // triage.test.ts cannot see the bug above — its fixture is this case. + const legacyStore = Object.assign(new EventEmitter(), { + listTasks: vi.fn().mockResolvedValue([parkedTask({ id: "FN-LEGACY", column: "triage" } as Partial)]), + getTask: vi.fn(async () => parkedTask({ id: "FN-LEGACY", column: "triage" } as Partial)), + getSettings: vi.fn().mockResolvedValue({ maxConcurrent: 10, maxWorktrees: 4, pollIntervalMs: 10_000, autoMerge: true }), + logEntry: vi.fn().mockResolvedValue(undefined), + updateTask: vi.fn().mockResolvedValue(undefined), + withTaskLock: vi.fn(async (_id: string, callback: () => unknown) => callback()), + recordRunAuditEvent: vi.fn().mockResolvedValue(undefined), + }) as unknown as TaskStore; + + expect(await pollWith(legacyStore)).toEqual(["FN-LEGACY"]); + }); +}); diff --git a/packages/engine/src/__tests__/triage.test.ts b/packages/engine/src/__tests__/triage.test.ts index eefde72172..cd8ef8c2da 100644 --- a/packages/engine/src/__tests__/triage.test.ts +++ b/packages/engine/src/__tests__/triage.test.ts @@ -1945,10 +1945,22 @@ Planner rewrote mission without the raw request. /* FNXC:CodingIdeasWorkflow 2026-07-05-00:00: - FN-7596 pins the Coding (Ideas) manual-intake lifecycle at the poll-dispatch boundary: an `ideas`-column card must stay parked (never auto-dispatched via `eligibleTriageTasks`, which only matches `column === "triage"`), while a promoted `todo`-column card whose PROMPT.md is still the bootstrap stub must be discovered and specified via `eligibleTodoTasks`'s bootstrap-prompt file check. A `todo` card with a real (non-bootstrap) spec must NOT be re-dispatched, guarding against double-specifying an already-planned card. + FN-7596 pins the Coding (Ideas) manual-intake lifecycle at the poll-dispatch boundary: an `ideas`-column card must stay parked, while a promoted `todo`-column card whose PROMPT.md is still the bootstrap stub must be discovered and specified via `eligibleTodoTasks`'s bootstrap-prompt file check. A `todo` card with a real (non-bootstrap) spec must NOT be re-dispatched, guarding against double-specifying an already-planned card. + + FNXC:ManualIntakeAdmission 2026-07-31-04:45 — READ THIS BEFORE TRUSTING THE FIRST CASE BELOW: + the parked-ideas case passes here for a reason unrelated to the rule. Its store has NO workflow + readers, so lifecycle resolution falls back to `triage`/`todo` and an `ideas` card matches neither + admission branch. The mechanism this comment used to name — "only matches column === triage" — was + removed when discovery started resolving intake BY TRAIT, at which point `ideas` BECAME the resolved + intake column and parked ideas were auto-planned. This test kept passing throughout. + + The real guard, with a store that resolves the workflow, is + `manual-intake-admission.test.ts`. This case is kept as the legacy-store shape (which is also a + real configuration) rather than deleted, but it is not the FN-7596 guard and must not be relied on + as one. */ describe("Coding (Ideas) manual-intake discovery (FN-7596)", () => { - it("excludes a parked ideas-column task from the poll's specify-dispatch set", async () => { + it("excludes a parked ideas-column task when the workflow cannot be resolved (legacy-store shape, NOT the FN-7596 guard)", async () => { const tasks: Task[] = [ createTriageTask({ id: "FN-IDEAS-PARKED", column: "ideas" as any, priority: "urgent" }), ]; diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 1557736a70..b3e0247daa 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -1762,6 +1762,17 @@ resume state); the KTD-1 exhaustion parks (FN-8141 blocked, retry-exhausted) set `status:"failed"` in place WITHOUT a move and are intentionally untouched here. One IR resolution per rebound (a recovery path, not an enumeration loop); any resolution failure falls back to the legacy "todo" so a rebound is never stranded. + +FNXC:WorkflowLifecycleColumns 2026-07-30-15:10 (Phase C convergence): +THE "ALREADY THERE?" GUARDS NOW COMPARE AGAINST THIS RESULT. Eight call sites read +`X.column !== "todo"` before moving to the resolved column — so on a renamed board the +guard was ALWAYS true and the engine issued a move into the column the card was already +in. That is a real move: `moveTaskInternal` runs the reset-on-entry effects again. At the +`preserveProgress: false` site (stale workflow parse pins) it reset step progress a second +time on a card that had only been re-checked, and every site re-ran the status/error/pause +clears. The move TARGET was converted here in U5b; the guards in front of it were not, +which is the half-conversion shape: the correct target reached through a check that could +not see it. Each site now resolves once and uses the same value for both. */ async function resolveReboundColumnFor(store: TaskStore, taskId: string): Promise { try { @@ -4455,8 +4466,9 @@ export class TaskExecutor { FNXC:WorkflowLifecycle 2026-07-12-23:13: FN-7926: completed work with a persistent `getTaskCompletionBlocker` result must not self-requeue through the execute node. Re-running implementation cannot clear dependency/blockedBy state, so it only feeds FN-7863's generic no-progress backstop and misclassifies good work as `EXECUTION_DISPATCH_LOOP_EXHAUSTED`. Park in a scheduler-skipped todo state, preserve worktree/branch/steps, and reset the FN-7863 signature so the backstop remains reserved for genuinely incomplete no-progress loops. */ - if (task.column !== "todo") { - await this.store.moveTask(task.id, await resolveReboundColumnFor(this.store, task.id), { + const reboundColumn = await resolveReboundColumnFor(this.store, task.id); + if (task.column !== reboundColumn) { + await this.store.moveTask(task.id, reboundColumn, { preserveProgress: true, preserveResumeState: true, preserveWorktree: true, @@ -10363,6 +10375,7 @@ export class TaskExecutor { abortProvenance: PausedAbortProvenance | undefined, pausedAborted: boolean, userCanceled: boolean, + resumeLanesMemo?: { lanes?: { hold: string; wip: string; review: string } }, ): Promise { /* FNXC:WorkflowLifecycle 2026-06-28-18:32: @@ -10399,19 +10412,85 @@ export class TaskExecutor { if (!sharedBranchMember && !allowsAutoMergeProcessing(live, settings)) return false; if (live.mergeDetails?.mergeConfirmed === true) return false; } - return live.column === "todo" || live.column === "in-review" || live.column === "in-progress"; + const resumeLanes = await this.resolveResumeLanes(live.id, resumeLanesMemo); + return live.column === resumeLanes.hold + || live.column === resumeLanes.review + || live.column === resumeLanes.wip; + } + + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-16:00 (Phase C convergence — resume eligibility): + The columns a RESUME may legitimately start from, resolved from the task's own workflow: the + hold (backlog) lane, the wip lane, and the review lane. + + These decisions were spelled as the default lineage's three names, so on a renamed board every + resume-safety check answered "not a safe resume state" and the paused-node re-entry, the + pause-abort auto-continue, and the benign-todo abort-marker clear all stopped firing. The last + of those is the one that bites: FN-6478's benign path exists so a re-queued card clears its + abort marker instead of being parked `failed` for an operator — and on a renamed board it took + the operator-action branch instead, which is the retry storm that path was written to end. + + ASYNC on purpose: every call site here is already async (a store read precedes each one), so + there is no listener-ordering hazard of the kind that forced the synchronous planner-lane + resolver in `replan-target.ts`. + + Fail-soft to the legacy trio so an unresolvable or column-less workflow behaves as before. + + FOLLOW-UP, deliberately not done here: PR #2628 exports a synchronous `resolvePlannerLanes` + (hold/intake/wip) from `replan-target.ts`. Once both land, this helper and that one should + become one resolver returning the full lane set — two resolvers for the same question is the + drift this program keeps paying for. Kept separate now only to avoid a cross-branch dependency. + */ + private async resolveResumeLanes( + taskId: string, + memo?: { lanes?: { hold: string; wip: string; review: string } }, + ): Promise<{ hold: string; wip: string; review: string }> { + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-01:00 (PR #2640 review, greptile P2): + ONE RESOLUTION PER RECOVERY, and the reason is correctness as much as I/O. Eligibility and + re-entry ran this separately, so a workflow edit landing between the two calls would have the + two halves of one decision reading DIFFERENT lane sets — the eligibility check admits a card in + review, the re-entry then resolves a board where that column is not the review lane. The memo is + caller-owned and per-recovery, which is the same shape as the IR caches elsewhere in the engine: + one snapshot for one decision, never a process-lifetime cache that has to guess when a + mid-flight workflow edit invalidates it. + */ + if (memo?.lanes) return memo.lanes; + try { + const lifecycle = resolveLifecycleColumns(await resolveWorkflowIrForTask(this.store, taskId)); + const lanes = { + hold: lifecycle?.hold ?? "todo", + wip: lifecycle?.wip ?? "in-progress", + review: lifecycle?.review ?? "in-review", + }; + if (memo) memo.lanes = lanes; + return lanes; + } catch { + const lanes = { hold: "todo", wip: "in-progress", review: "in-review" }; + if (memo) memo.lanes = lanes; + return lanes; + } } private async reenterPausedAbortedWorkflowNode( live: TaskDetail, result: WorkflowGraphTaskRunResult, abortProvenance: PausedAbortProvenance | undefined, + resumeLanesMemo?: { lanes?: { hold: string; wip: string; review: string } }, ): Promise { const nodeId = result.interruptedNodeId ?? result.visitedNodeIds[result.visitedNodeIds.length - 1] ?? "unknown"; const priorRetries = live.graphResumeRetryCount ?? 0; if (priorRetries >= MAX_TRANSIENT_GRAPH_RESUME_RETRIES) return false; const nextRetries = priorRetries + 1; - const preservedInReview = live.column === "in-review"; + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-16:05: resolved ONCE for the whole re-entry — + `preservedInReview`, the audit `mode` label, the resume-safety recheck, and the branch that + picks execute() vs executeWorkflowGraph() must all agree on which column is which. They were + four independent literal comparisons, so on a renamed board `preservedInReview` was false for + a card in review AND the recheck rejected it, and the re-entry silently never happened. + */ + const reentryLanes = await this.resolveResumeLanes(live.id, resumeLanesMemo); + const preservedInReview = live.column === reentryLanes.review; this.clearPausedAborted(live.id); this.activeWorktrees.delete(live.id); const message = `Workflow graph node '${nodeId}' was interrupted by engine pause/resume — re-entering workflow graph (${nextRetries}/${MAX_TRANSIENT_GRAPH_RESUME_RETRIES})`; @@ -10434,7 +10513,7 @@ export class TaskExecutor { maxAttempts: MAX_TRANSIENT_GRAPH_RESUME_RETRIES, abortProvenance: abortProvenance ?? "unknown", preservedInReview, - mode: preservedInReview ? "preserved-in-review" : live.column === "todo" ? "reexecuted-from-todo" : "reentered-graph", + mode: preservedInReview ? "preserved-in-review" : live.column === reentryLanes.hold ? "reexecuted-from-todo" : "reentered-graph", }, }); } catch (error) { @@ -10452,7 +10531,9 @@ export class TaskExecutor { || resumeTask.userPaused || resumeTask.status != null || resumeTask.error != null - || (preservedInReview ? resumeTask.column !== "in-review" : resumeTask.column !== "todo" && resumeTask.column !== "in-progress") + || (preservedInReview + ? resumeTask.column !== reentryLanes.review + : resumeTask.column !== reentryLanes.hold && resumeTask.column !== reentryLanes.wip) || this.activeSessions.has(live.id) || this.activeStepExecutors.has(live.id) || this.activeWorkflowStepSessions.has(live.id) @@ -10464,7 +10545,7 @@ export class TaskExecutor { } if (preservedInReview) { await this.executeWorkflowGraph(resumeTask); - } else if (resumeTask.column === "todo") { + } else if (resumeTask.column === reentryLanes.hold) { await this.execute(resumeTask); } else { await this.executeWorkflowGraph(resumeTask); @@ -10807,8 +10888,14 @@ export class TaskExecutor { `Pause abort classified: provenance=${abortProvenance ?? "unknown"}; node=${failedNodeForLog}; interrupted=${result.interruptedNodeId ?? "none"}; abortKind=${result.interruptedAbortKind ?? "none"}; column=${live.column}; status=${live.status ?? "none"}; paused=${live.paused === true}; userPaused=${live.userPaused === true}; value=${failureValueForLog}; genuine=${genuinePauseAbort}; mergeSeam=${mergeSeamAborted}; completionSuppressed=${suppressFinalizedCompletionAbort}`, ); } - if (genuinePauseAbort && await this.isReentrantPausedAbortedInFlightNode(live, result, abortProvenance, pausedAborted, this.userCanceledTaskIds.has(task.id))) { - if (await this.reenterPausedAbortedWorkflowNode(live, result, abortProvenance)) { + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-01:05 (PR #2640 review, greptile P2): one lane + snapshot for one recovery decision — see `resolveResumeLanes`. Eligibility and re-entry are two + halves of the SAME decision and must not read different boards. + */ + const resumeLanesMemo: { lanes?: { hold: string; wip: string; review: string } } = {}; + if (genuinePauseAbort && await this.isReentrantPausedAbortedInFlightNode(live, result, abortProvenance, pausedAborted, this.userCanceledTaskIds.has(task.id), resumeLanesMemo)) { + if (await this.reenterPausedAbortedWorkflowNode(live, result, abortProvenance, resumeLanesMemo)) { return; } } @@ -11546,8 +11633,9 @@ export class TaskExecutor { status: null, error: null, }, this.getRunContextFor(live.id)); - if (live.column !== "todo") { - await this.store.moveTask(live.id, await resolveReboundColumnFor(this.store, live.id), { + const reboundColumn = await resolveReboundColumnFor(this.store, live.id); + if (live.column !== reboundColumn) { + await this.store.moveTask(live.id, reboundColumn, { preserveProgress: true, moveSource: "engine", recoveryRehome: true, @@ -11594,8 +11682,9 @@ export class TaskExecutor { error: null, graphResumeRetryCount: 0, }, this.getRunContextFor(live.id)); - if (live.column !== "todo") { - await this.store.moveTask(live.id, await resolveReboundColumnFor(this.store, live.id), { preserveProgress: false }); + const reboundColumn = await resolveReboundColumnFor(this.store, live.id); + if (live.column !== reboundColumn) { + await this.store.moveTask(live.id, reboundColumn, { preserveProgress: false }); } const message = "Auto-recovered: cleared stale workflow parse pins after reset/retry — task requeued before execution"; executorLog.warn(`${live.id}: ${message}`); @@ -11751,8 +11840,9 @@ export class TaskExecutor { Workflow-graph and workflow-authoritative executor dispatches can be invoked outside the classic scheduler loop, so they must re-apply the shared scheduling dependency gate before graph routing, column-agent seams, or review handoff can run. Requeue with blockedBy instead of executing so missing or soft-deleted dependency residue keeps the scheduler helper's non-blocking semantics while live todo/queued/in-progress/triage dependencies block every dispatch surface. */ - if (liveTask.column !== "todo") { - await this.store.moveTask(liveTask.id, await resolveReboundColumnFor(this.store, liveTask.id), { + const reboundColumn = await resolveReboundColumnFor(this.store, liveTask.id); + if (liveTask.column !== reboundColumn) { + await this.store.moveTask(liveTask.id, reboundColumn, { preserveProgress: true, preserveWorktree: true, preserveResumeState: true, @@ -11794,8 +11884,9 @@ export class TaskExecutor { } const liveTask = (await this.store.getTask(task.id).catch(() => null)) ?? task; - if (liveTask.column !== "todo") { - await this.store.moveTask(liveTask.id, await resolveReboundColumnFor(this.store, liveTask.id), { + const reboundColumn = await resolveReboundColumnFor(this.store, liveTask.id); + if (liveTask.column !== reboundColumn) { + await this.store.moveTask(liveTask.id, reboundColumn, { preserveProgress: true, preserveWorktree: true, preserveResumeState: true, @@ -13087,10 +13178,11 @@ export class TaskExecutor { worktree: null, branch: null, }); - if (latestTask.column !== "todo") { + const reboundColumn = await resolveReboundColumnFor(this.store, task.id); + if (latestTask.column !== reboundColumn) { this.markGraphExecuteSelfRequeued(task.id); - await this.store.moveTask(task.id, await resolveReboundColumnFor(this.store, task.id), preserveProgress ? { preserveProgress: true } : undefined); - executorLog.log(`${task.id} moved to todo for retry after stuck kill${preserveProgress ? " (progress preserved)" : ""}`); + await this.store.moveTask(task.id, reboundColumn, preserveProgress ? { preserveProgress: true } : undefined); + executorLog.log(`${task.id} moved to ${reboundColumn} for retry after stuck kill${preserveProgress ? " (progress preserved)" : ""}`); } } } catch (err: unknown) { @@ -15016,16 +15108,17 @@ export class TaskExecutor { status: null, error: null, }); - if (latestTask.column !== "todo") { + const continuationReboundColumn = await resolveReboundColumnFor(this.store, task.id); + if (latestTask.column !== continuationReboundColumn) { this.markGraphExecuteSelfRequeued(task.id); this.activeWorktrees.delete(task.id); executingTaskLock.release(task.id); cleanupLockHeld = false; - await this.store.moveTask(task.id, await resolveReboundColumnFor(this.store, task.id), { preserveResumeState: true }); + await this.store.moveTask(task.id, continuationReboundColumn, { preserveResumeState: true }); } else { this.activeWorktrees.delete(task.id); } - executorLog.log(`${task.id} stale assistant-continuation session cleared — requeued to todo with progress preserved`); + executorLog.log(`${task.id} stale assistant-continuation session cleared — requeued to ${continuationReboundColumn} with progress preserved`); } else { executorLog.debug(`${task.id} stale assistant-continuation requeue skipped — task is now in '${latestTask.column}'`); } @@ -15104,14 +15197,22 @@ export class TaskExecutor { // latestTask.column rather than the stale captured task.column — // the captured snapshot can be hours old and would race against // any concurrent recovery (see comment above). - if (latestTask.column !== "todo") { + const stuckReboundColumn = await resolveReboundColumnFor(this.store, task.id); + if (latestTask.column !== stuckReboundColumn) { this.markGraphExecuteSelfRequeued(task.id); - await this.store.moveTask(task.id, await resolveReboundColumnFor(this.store, task.id), preserveProgress ? { preserveProgress: true } : undefined); - // Audit trail: record task move (FN-1404) - await audit.database({ type: "task:move", target: task.id, metadata: { to: "todo" } }); - executorLog.log(`${task.id} moved to todo for retry after stuck kill${preserveProgress ? " (progress preserved)" : ""}`); + await this.store.moveTask(task.id, stuckReboundColumn, preserveProgress ? { preserveProgress: true } : undefined); + /* + Audit trail: record task move (FN-1404). + FNXC:WorkflowLifecycleColumns 2026-07-30-15:15: `to` records the column the card was + ACTUALLY moved to. It was hardcoded `"todo"` while the move target was already + resolved from the workflow, so on a renamed board the audit row named a column the + move never touched — a run-audit trail that disagrees with the move it describes is + worse than none, because it is the record an operator reaches for afterwards. + */ + await audit.database({ type: "task:move", target: task.id, metadata: { to: stuckReboundColumn } }); + executorLog.log(`${task.id} moved to ${stuckReboundColumn} for retry after stuck kill${preserveProgress ? " (progress preserved)" : ""}`); } else { - executorLog.debug(`${task.id} already in todo — skipping redundant move`); + executorLog.debug(`${task.id} already in ${stuckReboundColumn} — skipping redundant move`); } } } catch (err: unknown) { diff --git a/packages/engine/src/mission-feature-sync.ts b/packages/engine/src/mission-feature-sync.ts index 56d8e7f783..559e8817d7 100644 --- a/packages/engine/src/mission-feature-sync.ts +++ b/packages/engine/src/mission-feature-sync.ts @@ -1,5 +1,5 @@ import type { MissionFeature, Task, TaskStore } from "@fusion/core"; -import { resolveTaskLifecycleColumns, resolveWorkflowIrForTask } from "@fusion/core"; +import { resolveLifecycleColumns, resolveTaskLifecycleColumns, resolveWorkflowIrForTask } from "@fusion/core"; import { getTaskCompletionBlockerForStore } from "./task-completion.js"; export type MissionFeatureSyncTargetStatus = "done" | "in-progress" | "triaged"; @@ -77,7 +77,22 @@ export async function reconcileMissionFeatureState( 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-31-11:30 (found auditing my OWN merged code for the split I made + three times in PR #2644): + ONE SNAPSHOT. This read the workflow TWICE — `resolveTaskLifecycleColumns` for the roles and + `resolveWorkflowIrForTask` for the declared column ids — which is literally the same read twice, since + the former is `resolveLifecycleColumns(await resolveWorkflowIrForTask(...))`. A workflow edit between + them gives roles from one revision and declared columns from another, so the aliasing guard below is + evaluated against a column set that no longer matches the roles it is protecting. + + FOURTH occurrence of this shape in my work on this program (executor resume lanes, glasses lane + context, glasses capture, here). The first three were caught in review; this one was already merged. + The predictor that found it is mechanical rather than clever: grep for two resolver calls inside one + function. + */ + const ir = await resolveWorkflowIrForTask(taskStore, task.id).catch(() => undefined); + const roles = ir ? resolveLifecycleColumns(ir) : undefined; /* 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 @@ -101,7 +116,6 @@ export async function reconcileMissionFeatureState( 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) diff --git a/packages/engine/src/triage.ts b/packages/engine/src/triage.ts index fc6fea9596..db06721419 100644 --- a/packages/engine/src/triage.ts +++ b/packages/engine/src/triage.ts @@ -1601,7 +1601,7 @@ export class TriageProcessor { below needs, and taking it from the same resolution keeps the cost identical (same call, same `irCache`, same bounded window) rather than adding a read. */ - const lifecycleByTaskId = new Map }>(); + const lifecycleByTaskId = new Map; manualIntake?: boolean }>(); const RESOLUTION_CONCURRENCY = 8; for (let offset = 0; offset < candidates.length; offset += RESOLUTION_CONCURRENCY) { const window = candidates.slice(offset, offset + RESOLUTION_CONCURRENCY); @@ -1610,8 +1610,21 @@ export class TriageProcessor { try { const ir = await resolveWorkflowIrForTask(this.store, t.id, irCache); const roles = resolveLifecycleColumns(ir); - const columns = (ir as { columns?: { id: string }[] }).columns ?? []; - return { ...roles, declared: new Set(columns.map((c) => c.id)) }; + const columns = (ir as { columns?: Array<{ id: string; traits?: Array<{ trait: string; config?: Record }> }> }).columns ?? []; + /* + FNXC:ManualIntakeAdmission 2026-07-31-04:20: + The intake trait's `autoTriage: false` comes from the SAME resolution, not a second read — + it is the only durable signal that separates a parked card from one an operator released. + */ + const intakeTraitConfig = columns + .find((column) => column.id === roles?.intake) + ?.traits?.find((trait) => trait.trait === "intake") + ?.config; + return { + ...roles, + declared: new Set(columns.map((c) => c.id)), + manualIntake: intakeTraitConfig?.autoTriage === false, + }; } catch { return undefined; } @@ -1628,7 +1641,31 @@ export class TriageProcessor { // An unresolved task id means the card was filtered out before resolution, so it is not a // candidate and both predicates are correctly false for it. const isAtHoldColumn = (t: Task): boolean => lifecycleByTaskId.get(t.id)?.hold === t.column; - const isAtIntakeColumn = (t: Task): boolean => lifecycleByTaskId.get(t.id)?.intake === t.column; + /* + FNXC:ManualIntakeAdmission 2026-07-31-04:25 (live bug — FN-7596's rule was broken by the trait conversion): + A MANUAL intake (`autoTriage: false`) is never auto-admitted. Coding (Ideas) exists so an operator + can park a card without the engine planning it; the operator promotes it into Planning when ready. + + HOW IT BROKE. The rule used to be enforced accidentally, by this predicate naming `triage`: an + `ideas` card matched no branch. Converting the predicate to resolve intake BY TRAIT made `ideas` + the resolved intake column for that workflow — so discovery started specifying parked ideas. The + conversion widened admission, which is the same shape as every other half-conversion in this + program: the guard became correct in vocabulary and wrong in effect. + + ITS GUARDING TEST COULD NOT CATCH IT. `triage.test.ts`'s "excludes a parked ideas-column task" + uses a mock store with no workflow readers, so lifecycle resolution falls back to + `triage`/`todo` and an `ideas` card matches neither branch — it passes for a reason unrelated to + the rule, and kept passing after the rule broke. Its own comment still describes the old + mechanism ("which only matches column === triage"), which is the tell. + + The hold branch is deliberately NOT gated on this: a card in the hold column was RELEASED there, + by an operator or by finalize, and a manual intake says nothing about a card that already left it. + */ + const isAtIntakeColumn = (t: Task): boolean => { + const lifecycle = lifecycleByTaskId.get(t.id); + if (lifecycle?.intake !== t.column) return false; + return lifecycle.manualIntake !== true; + }; /* FNXC:WorkflowLifecycleColumns 2026-07-29-18:40 (U11 — STALL 3): A card in a column its OWN workflow does not declare is unowned by construction: diff --git a/plugins/fusion-plugin-dependency-graph/src/GraphTaskNode.tsx b/plugins/fusion-plugin-dependency-graph/src/GraphTaskNode.tsx index aacb1feaaf..75c23fe556 100644 --- a/plugins/fusion-plugin-dependency-graph/src/GraphTaskNode.tsx +++ b/plugins/fusion-plugin-dependency-graph/src/GraphTaskNode.tsx @@ -70,14 +70,44 @@ export function GraphTaskNode({ const isFailed = task.status === "failed"; const isPaused = task.paused === true; const isStuck = isTaskStuck(task, taskStuckTimeoutMs, lastFetchTimeMs); - const isAwaitingApproval = task.column === "triage" && task.status === "awaiting-approval"; + /* + FNXC:PluginLifecycleColumns 2026-07-30-03:40 (U11 #2515 audit): + Keyed on `column === "triage"`, this went permanently FALSE for default-lineage + cards once U11 merged Todo into Planning and dropped the `triage` id — an + awaiting-approval card sits in `todo` now. The node then stopped showing the + awaiting-approval state AND fell through to `isActive`, rendering a card that is + blocked on a human as if it were running. + + The column condition is DELETED rather than converted, because it was always + redundant: `awaiting-approval` is written only by the plan-approval gate and the + replan-cap park, both of which act on a card in the planning lane, so the status + alone is the signal. Deleting it is also the only option that needs no resolution — + this is a synchronous React render, where an IR lookup is not available. + */ + /* + FNXC:PluginLifecycleColumns 2026-07-31-11:50 (PR #2644 review, greptile P1): + A STALE APPROVAL STATUS MUST NOT HIDE A RUNNING CARD. Dropping the column condition made the + awaiting-approval signal status-only, which is right for a planning-lane card — but `awaiting-approval` + is DURABLE, so a card that carries it into an execution lane was rendered as not-active: no active + styling, no execution-status indicator, no current-step metadata, while it was plainly running. + + So the suppression now yields to an execution SIGNAL rather than to a column name. If the card shows + execution activity, it is active and the stale approval status is residue; if it does not, the approval + state is the truth. That ordering needs no IR lookup, which matters here — this is a synchronous React + render. + + The `in-progress` literal is pre-existing and NOT on the triage bar; converting it needs the board's + column traits, which this component is not given. Left with this note rather than half-converted. + */ + const hasExecutionSignal = task.column === "in-progress" || ACTIVE_STATUSES.has(task.status as string); + const isAwaitingApproval = task.status === "awaiting-approval" && !hasExecutionSignal; const isActive = !globalPaused && !isFailed && !isPaused && !isStuck && !isAwaitingApproval && - (task.column === "in-progress" || ACTIVE_STATUSES.has(task.status as string)); + hasExecutionSignal; const hasValidCurrentStep = typeof task.currentStep === "number" && diff --git a/plugins/fusion-plugin-even-realities-glasses/src/__tests__/agent-actions.test.ts b/plugins/fusion-plugin-even-realities-glasses/src/__tests__/agent-actions.test.ts index dfad47e4d6..53df974300 100644 --- a/plugins/fusion-plugin-even-realities-glasses/src/__tests__/agent-actions.test.ts +++ b/plugins/fusion-plugin-even-realities-glasses/src/__tests__/agent-actions.test.ts @@ -217,3 +217,684 @@ describe("retryTask", () => { expect(deps.moveTask).not.toHaveBeenCalled(); }); }); + +/* +FNXC:PluginLifecycleColumns 2026-07-30-03:20 (U11 #2515 audit — unowned plugin sites): + +These operator actions gated on `column === "triage"`. U11 (#2515) merged Todo into +Planning KEEPING the id `todo` and DELETING `triage`, so on the default lineage: + + approvePlan — REFUSED for every card. An awaiting-approval card now sits in `todo`, + the gate demands `triage`, so the operator's approve action from the + glasses surface fails with a conflict on a perfectly valid card. + retryTask — its triage-retry branch never fires, so a stuck/needs-replan card + cannot be retried from the glasses at all. + startWork — SURVIVES, because it already accepted `todo` as well. + +That asymmetry is the tell: the one gate written to accept both ids kept working, and +the two written against a single id broke. `plugins/` is in no unit's file list and no +drift-review assignment. + +The fix accepts the PRE-IMPLEMENTATION LANE rather than one id, resolving the task's +own workflow when the plugin's store can (it depends on `@fusion/core`) and falling +back to both legacy ids when it cannot. The fallback is why these cases assert the +default vocabulary too. +*/ +describe("post-U11 planning-column gates", () => { + it("approvePlan accepts an awaiting-approval card in the MERGED planning column", async () => { + // Pre-fix: conflict. The card is valid and the operator's action just failed. + const deps = createDeps(makeTask({ column: "todo", status: "awaiting-approval" })); + + const result = await approvePlan({ taskId: "FN-1" }, deps as never); + + expect(result.task.column).toBe("todo"); + expect(deps.updateTask).toHaveBeenCalled(); + }); + + it("approvePlan still accepts a legacy `triage` card (migration window)", async () => { + const deps = createDeps(makeTask({ column: "triage", status: "awaiting-approval" })); + + await expect(approvePlan({ taskId: "FN-1" }, deps as never)).resolves.toBeTruthy(); + }); + + it("approvePlan still REFUSES a card that has left the planning lane", async () => { + // The other side, so "always accepts" cannot pass for "accepts the lane". + const deps = createDeps(makeTask({ column: "in-progress", status: "awaiting-approval" })); + + await expectInputError(approvePlan({ taskId: "FN-1" }, deps as never), 409); + }); + + it("retryTask reaches its planning-lane branch for a card in the MERGED column", async () => { + // Pre-fix the branch was unreachable for default-lineage cards, so a stuck card + // could not be retried from this surface at all. + const deps = createDeps(makeTask({ column: "todo", status: "stuck-killed", stuckKillCount: 2 })); + + await retryTask({ taskId: "FN-1" }, deps as never); + + expect(deps.updateTask).toHaveBeenCalledWith("FN-1", expect.objectContaining({ status: "needs-replan" })); + }); + + it("startWork keeps accepting both ids (it already did — the control)", async () => { + for (const column of ["todo", "triage"]) { + const deps = createDeps(makeTask({ column, status: null })); + await expect(startWork({ taskId: "FN-1" }, deps as never)).resolves.toBeTruthy(); + } + }); +}); + +/* +FNXC:PluginLifecycleColumns 2026-07-30-05:10 (PR #2607 review — greptile P1 x2): + +Two over-reaches in my own first version, both found by review: + + 1. DESTINATIONS stayed literal while the GATES were converted. On a renamed workflow + that is WORSE than the original bug: the gate now admits the card and then moves it + into a column the workflow does not declare. Half a conversion moved the failure + from "refuses valid work" to "puts work where nothing renders it". + + 2. The legacy-id acceptance was UNSCOPED, so a workflow naming its review or wip lane + `triage`/`todo` had those cards authorized as planning work. + +These drive a store that CAN resolve a workflow, which the default fixture cannot — the +plugin store is narrowed, so without the workflow methods every earlier case silently +exercised the legacy fallback rather than the resolved path. +*/ +function createResolvingDeps(task: FakeTask, ir: unknown) { + const base = createDeps(task); + const selection = { workflowId: "wf-custom", stepIds: [] }; + return { + ...base, + taskStore: { + ...base.taskStore, + getTaskWorkflowSelection: () => selection, + getTaskWorkflowSelectionAsync: async () => selection, + getWorkflowDefinition: async () => ({ ir }), + }, + }; +} + +const renamedIr = { + version: "v2", id: "wf-custom", name: "renamed", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Planning", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] }, + { id: "building", name: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "checking", name: "Review", traits: [{ trait: "merge-blocker" }, { trait: "human-review" }] }, + { id: "shipped", name: "Done", traits: [{ trait: "complete" }] }, + ], +}; + +/** A workflow that assigns the LEGACY id `todo` to its REVIEW lane. Legal, and not planning. */ +const todoIsReviewIr = { + version: "v2", id: "wf-custom", name: "todo-is-review", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Planning", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] }, + { id: "building", name: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + /* + The `merge` trait is REQUIRED for this to be a resolvable review lane, and that + detail is the finding: `resolveLifecycleColumns` derives `review` from `merge`, not + from `merge-blocker`/`human-review`. My first fixture omitted it, so `review` came + back undefined, `declaredIds` did not contain `todo`, and the legacy acceptance + applied — the test failed and was RIGHT to. + + (That gap — a column whose traits map to no role being invisible to a role-only + check — is CLOSED below by reading the IR's declared column ids directly. It did not + need a core change after all; it needed me to read an input that was already in + reach. See "a declared column is declared even when it carries no role".) + */ + { id: "todo", name: "Review", traits: [{ trait: "merge-blocker" }, { trait: "human-review" }, { trait: "merge" }] }, + { id: "shipped", name: "Done", traits: [{ trait: "complete" }] }, + ], +}; + +describe("resolved lanes drive destinations, not just gates", () => { + it("startWork moves a renamed card to the workflow's OWN wip column", async () => { + // Pre-fix: admitted, then moved to the literal `in-progress` — a column this + // workflow does not declare. + const deps = createResolvingDeps(makeTask({ column: "backlog", status: null }), renamedIr); + + const result = await startWork({ taskId: "FN-1" }, deps as never); + + expect(deps.moveTask).toHaveBeenCalledWith("FN-1", "building"); + expect(result.task.column).toBe("building"); + }); + + it("approvePlan moves a renamed card to the workflow's OWN hold column", async () => { + const deps = createResolvingDeps( + makeTask({ column: "backlog", status: "awaiting-approval" }), + renamedIr, + ); + + await approvePlan({ taskId: "FN-1" }, deps as never); + + expect(deps.moveTask).toHaveBeenCalledWith("FN-1", "backlog"); + }); + + it("REFUSES a card in a legacy-named column the workflow assigns to REVIEW", async () => { + /* + The aliasing case. Unscoped, `todo` counted as a planning lane and startWork would + have pulled a card out of review and into wip — skipping the review entirely. + */ + const deps = createResolvingDeps(makeTask({ column: "todo", status: null }), todoIsReviewIr); + + await expectInputError(startWork({ taskId: "FN-1" }, deps as never), 409); + expect(deps.moveTask).not.toHaveBeenCalled(); + }); + + it("still accepts a legacy id the workflow does not use at all (migration window)", async () => { + // `renamedIr` declares no `todo`, so a pre-U11 row resting there is an orphan and + // still means "planning". + const deps = createResolvingDeps(makeTask({ column: "todo", status: null }), renamedIr); + + await expect(startWork({ taskId: "FN-1" }, deps as never)).resolves.toBeTruthy(); + expect(deps.moveTask).toHaveBeenCalledWith("FN-1", "building"); + }); +}); + +/* +FNXC:PluginLifecycleColumns 2026-07-30-07:20 (PR #2607 review, second P1 — greptile): + +TWO THINGS THE ROLE-ONLY VERSION GOT WRONG, both of which I had written down as +limitations rather than fixed. Recording a gap you can close is just a nicer way of +leaving it open. + + 1. A DECLARED COLUMN IS DECLARED EVEN WHEN IT CARRIES NO ROLE. Building the + declared set from the six resolved roles left a trait-less column named `todo` + invisible, so the legacy acceptance claimed it as a planning lane and `startWork` + would pull a card out of it. The IR lists its own columns; read that instead. + + 2. A MISSING ROLE IS NOT A LICENCE TO INVENT A COLUMN. `destination` fell back to + `todo`/`in-progress` unconditionally, so a valid workflow that simply omits the + role had `moveTask` called with a column that does not exist on that board. An + action with nowhere legitimate to send the card is not configured for this + workflow; 409 says that, a move to a phantom column does not. +*/ +/** A column named with a legacy id but carrying NO lifecycle trait. Legal, and not planning. */ +const inertTodoIr = { + version: "v2", id: "wf-custom", name: "inert-todo", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Planning", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] }, + { id: "todo", name: "Parking", traits: [] }, + { id: "building", name: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "shipped", name: "Done", traits: [{ trait: "complete" }] }, + ], +}; + +/** A workflow with NO wip lane at all — nowhere for `startWork` to legitimately send a card. */ +const noWipIr = { + version: "v2", id: "wf-custom", name: "no-wip", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Planning", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] }, + { id: "shipped", name: "Done", traits: [{ trait: "complete" }] }, + ], +}; + +describe("a declared column is declared even when it carries no role", () => { + it("refuses start-work on an inert column named `todo`", async () => { + // Pre-fix: `todo` was absent from the role-derived set, the legacy acceptance + // applied, and the card was pulled out of the operator's parking column. + const deps = createResolvingDeps(makeTask({ id: "FN-1", column: "todo", status: null }), inertTodoIr); + + await expect(startWork({ taskId: "FN-1" }, deps as never)).rejects.toThrow(/not allowed/); + expect(deps.moveTask).not.toHaveBeenCalled(); + }); + + it("still admits the workflow's REAL planning column", async () => { + // The other half of the pair: "never a planning lane" must not be able to pass + // for "reads the IR". + const deps = createResolvingDeps(makeTask({ id: "FN-2", column: "backlog", status: null }), inertTodoIr); + + await startWork({ taskId: "FN-2" }, deps as never); + + expect(deps.moveTask).toHaveBeenCalledWith("FN-2", "building"); + }); +}); + +describe("a missing destination role conflicts instead of inventing a column", () => { + it("refuses start-work when the workflow declares no wip lane", async () => { + // Pre-fix: moved to the literal `in-progress`, which this workflow does not declare. + const deps = createResolvingDeps(makeTask({ id: "FN-3", column: "backlog", status: null }), noWipIr); + + await expect(startWork({ taskId: "FN-3" }, deps as never)).rejects.toThrow(/not allowed/); + expect(deps.moveTask).not.toHaveBeenCalled(); + }); + + it("refuses approve-plan when the workflow declares no hold lane", async () => { + const holdlessIr = { + version: "v2", id: "wf-custom", name: "no-hold", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Planning", traits: [{ trait: "intake" }] }, + { id: "building", name: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "shipped", name: "Done", traits: [{ trait: "complete" }] }, + ], + }; + const deps = createResolvingDeps(makeTask({ id: "FN-4", column: "backlog", status: "awaiting-approval" }), holdlessIr); + + await expect(approvePlan({ taskId: "FN-4" }, deps as never)).rejects.toThrow(/not allowed/); + expect(deps.moveTask).not.toHaveBeenCalled(); + }); + + it("STILL uses the legacy id when the workflow genuinely declares it (migration window)", async () => { + // The fallback is not deleted, it is scoped: a pre-U11 board really does have + // `todo`, and refusing there would break the migration this program is mid-way + // through. `todo` here carries the hold trait, so it is a real destination. + const migrationIr = { + version: "v2", id: "wf-custom", name: "pre-u11", nodes: [], edges: [], + columns: [ + { id: "triage", name: "Triage", traits: [{ trait: "intake" }] }, + { id: "todo", name: "Todo", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: "in-progress", name: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + ], + }; + const deps = createResolvingDeps(makeTask({ id: "FN-5", column: "triage", status: "awaiting-approval" }), migrationIr); + + await approvePlan({ taskId: "FN-5" }, deps as never); + + expect(deps.moveTask).toHaveBeenCalledWith("FN-5", "todo"); + }); +}); + +/* +FNXC:PluginLifecycleColumns 2026-07-30-19:25 (PR #2607 review — fourth finding): + +"DECLARED SOMEWHERE" IS NOT "DECLARED FOR THIS ROLE", and this is the FOURTH time I have made the +legacy-aliasing mistake in this file. `declared` holds every column id the workflow has, so a board +that declares no hold column but names its REVIEW lane `todo` satisfied `declared.has("todo")` — +and `approvePlan` moved an approved plan straight into REVIEW, skipping implementation. Worse than +the refusal it replaced, which is the recurring signature of a half-applied rule. + +The rule, stated the same way as for the gate: a legacy id may stand in only for a role the +workflow leaves EMPTY. If the board has assigned that id to another lifecycle role, it means +something else there. +*/ +const HOLDLESS_TODO_IS_REVIEW_IR = { + version: "v2", id: "wf-custom", name: "holdless-todo-review", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Planning", traits: [{ trait: "intake" }] }, + { id: "building", name: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "todo", name: "Review", traits: [{ trait: "merge-blocker" }, { trait: "human-review" }, { trait: "merge" }] }, + { id: "shipped", name: "Done", traits: [{ trait: "complete" }] }, + ], +}; + +describe("a legacy destination may only fill a role the workflow leaves empty", () => { + it("refuses approve-plan rather than moving the plan into a lane named `todo` that is REVIEW", async () => { + // Pre-fix: moved to `todo` — this board's review lane — so an approved plan skipped + // implementation entirely. + const deps = createResolvingDeps( + makeTask({ column: "backlog", status: "awaiting-approval" }), + HOLDLESS_TODO_IS_REVIEW_IR, + ); + + await expect(approvePlan({ taskId: "FN-1" }, deps as never)).rejects.toThrow(/not allowed/); + expect(deps.moveTask).not.toHaveBeenCalled(); + }); + + it("still uses the legacy id when the workflow declares it and assigns it to NO other role", async () => { + // The migration case the fallback exists for: a pre-U11 board really does have `todo` as its + // hold lane. Scoping the fallback must not delete it. + const migrationIr = { + version: "v2", id: "wf-custom", name: "pre-u11", nodes: [], edges: [], + columns: [ + { id: "triage", name: "Triage", traits: [{ trait: "intake" }] }, + { id: "todo", name: "Todo", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: "in-progress", name: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + ], + }; + const deps = createResolvingDeps(makeTask({ column: "triage", status: "awaiting-approval" }), migrationIr); + + await approvePlan({ taskId: "FN-1" }, deps as never); + + expect(deps.moveTask).toHaveBeenCalledWith("FN-1", "todo"); + }); +}); + +/* +FNXC:PluginLifecycleColumns 2026-07-30-21:45 (PR #2607 review — FIFTH finding, one rule): + +A ROLELESS COLUMN NAMED `todo` IS STILL NOT THE HOLD LANE. My previous revision scoped the legacy +fallback to "declared and not assigned to another role", and review found the remaining hole +immediately: a TRAITLESS parking column named `todo` is assigned to no role, so no role check can +see it, and `approvePlan` moved an approved plan into a column that implements nothing. + +The qualifications were themselves the mistake. Once `resolveLanes` returns a lane set the workflow +HAS a column vocabulary, so "no column carries the hold trait" is a complete answer — refuse. The +legacy id survives only when the workflow cannot be resolved at all, which is the migration case. + +Five attempts at one rule. These cases pin all four shapes it has to get right at once. +*/ +const ROLELESS_TODO_IR = { + version: "v2", id: "wf-custom", name: "roleless-todo", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Planning", traits: [{ trait: "intake" }] }, + { id: "todo", name: "Parking", traits: [] }, + { id: "building", name: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "shipped", name: "Done", traits: [{ trait: "complete" }] }, + ], +}; + +describe("a legacy id is not a destination once the workflow speaks columns", () => { + it("refuses approve-plan rather than parking the plan in a TRAITLESS column named `todo`", async () => { + // Pre-fix: `assignedElsewhere` was false (no role owns a traitless column), so the fallback + // returned `todo` and an approved plan landed in the operator's parking column. + const deps = createResolvingDeps( + makeTask({ column: "backlog", status: "awaiting-approval" }), + ROLELESS_TODO_IR, + ); + + await expect(approvePlan({ taskId: "FN-1" }, deps as never)).rejects.toThrow(/not allowed/); + expect(deps.moveTask).not.toHaveBeenCalled(); + }); + + it("still starts work, because THAT role is declared on the same board", async () => { + // The paired positive: refusing a missing role must not become refusing everything. This board + // has no hold lane but does have a wip lane, so start-work is legitimate. + const deps = createResolvingDeps(makeTask({ column: "backlog", status: null }), ROLELESS_TODO_IR); + + await startWork({ taskId: "FN-1" }, deps as never); + + expect(deps.moveTask).toHaveBeenCalledWith("FN-1", "building"); + }); + + it("keeps the legacy destination when the workflow cannot be resolved at all", async () => { + // No lane set means no basis to decide, and a pre-U11 board really does use these ids — + // refusing here would break the migration rather than protect it. `createDeps` supplies a + // store with no workflow readers, which is exactly that case. + const deps = createDeps(makeTask({ column: "todo", status: "awaiting-approval" })); + + await approvePlan({ taskId: "FN-1" }, deps as never); + + expect(deps.moveTask).toHaveBeenCalledWith("FN-1", "todo"); + }); +}); + +/* +FNXC:PluginLifecycleColumns 2026-07-31-00:35 (PR #2607 review — sixth finding): + +DEGRADED RESOLUTION LOOKS EXACTLY LIKE THE DEFAULT BOARD. `resolveWorkflowIrForTask` is TOTAL by +design: a missing workflow definition or a failed read silently returns the DEFAULT coding IR. So a +card on a CUSTOM board whose definition could not be loaded resolved to `todo`/`in-progress`, and +these actions rejected a valid custom planning card or moved it to a column its own workflow does not +declare. + +`undefined` lanes cannot express this — that means "no workflow at all", where the legacy ids ARE the +answer. This is the third state: "this board HAS a vocabulary and we could not read it", where acting +on someone else's vocabulary is the one thing we must not do. +*/ +function createDegradedDeps(task: FakeTask) { + const base = createDeps(task); + const selection = { workflowId: "wf-custom", stepIds: [] }; + return { + ...base, + taskStore: { + ...base.taskStore, + getTaskWorkflowSelection: () => selection, + getTaskWorkflowSelectionAsync: async () => selection, + // The definition is GONE — the exact state the resolver papers over with the default IR. + getWorkflowDefinition: async () => undefined, + }, + }; +} + +describe("a card whose custom workflow cannot be read is refused, not treated as default", () => { + it("refuses start-work rather than moving to the DEFAULT board's wip column", async () => { + // Pre-fix: lanes resolved to the default coding IR, so this moved the card to `in-progress` — + // a column the custom workflow may not declare at all. + const deps = createDegradedDeps(makeTask({ column: "backlog", status: null })); + + await expect(startWork({ taskId: "FN-1" }, deps as never)).rejects.toThrow(/not allowed/); + expect(deps.moveTask).not.toHaveBeenCalled(); + }); + + it("refuses approve-plan the same way", async () => { + const deps = createDegradedDeps(makeTask({ column: "backlog", status: "awaiting-approval" })); + + await expect(approvePlan({ taskId: "FN-1" }, deps as never)).rejects.toThrow(/not allowed/); + expect(deps.moveTask).not.toHaveBeenCalled(); + }); + + it("does NOT refuse when the workflow resolves properly (the paired positive)", async () => { + // "Refuse on degraded" must not become "refuse whenever a selection exists". + const deps = createResolvingDeps(makeTask({ column: "backlog", status: null }), renamedIr); + + await startWork({ taskId: "FN-1" }, deps as never); + + expect(deps.moveTask).toHaveBeenCalledWith("FN-1", "building"); + }); + + it("does NOT refuse a store that cannot answer at all (the migration case)", async () => { + // No workflow readers means "no workflow at all", where the legacy ids are the answer. + // Refusing here would break every pre-U11 board instead of protecting a custom one. + const deps = createDeps(makeTask({ column: "todo", status: null })); + + await startWork({ taskId: "FN-1" }, deps as never); + + expect(deps.moveTask).toHaveBeenCalledWith("FN-1", "in-progress"); + }); +}); + +/* +FNXC:PluginLifecycleColumns 2026-07-31-02:35 (PR #2644 review, greptile P1): + +ONE SNAPSHOT PER ACTION. The degraded probe, the lane resolution and the declared-column read used to +consult the workflow independently, so a workflow edited or deleted mid-action could combine a +NOT-degraded verdict with fallback lanes, or lanes from one revision with declarations from another. +The action then conflicted on a card that was fine, or moved it toward a column the current workflow +no longer has. + +Same fix as the executor's resume lanes: the halves of one decision must read one snapshot. +*/ +describe("an action reads the workflow once, not three times", () => { + function countingDeps(task: FakeTask, ir: unknown) { + const base = createDeps(task); + const selection = { workflowId: "wf-custom", stepIds: [] }; + const reads = { definition: 0 }; + return { + ...base, + reads, + taskStore: { + ...base.taskStore, + getTaskWorkflowSelection: () => selection, + getTaskWorkflowSelectionAsync: async () => selection, + getWorkflowDefinition: async () => { + reads.definition += 1; + return { ir }; + }, + }, + }; + } + + it("reads the custom definition once per action", async () => { + const deps = countingDeps(makeTask({ column: "backlog", status: null }), renamedIr); + + await startWork({ taskId: "FN-1" }, deps as never); + + // One read backs the degraded verdict, the lanes AND the declared columns. Three reads was the + // bug: they could disagree with each other. + expect((deps as unknown as { reads: { definition: number } }).reads.definition).toBe(1); + expect(deps.moveTask).toHaveBeenCalledWith("FN-1", "building"); + }); + + it("refuses when the definition read throws mid-action", async () => { + // A store that cannot answer during a MOVE is degraded: refusing is the safe direction, and it + // must not silently fall back to the default board's lanes. + const deps = countingDeps(makeTask({ column: "backlog", status: null }), renamedIr); + (deps.taskStore as unknown as Record).getWorkflowDefinition = async () => { + throw new Error("workflow store unavailable"); + }; + + await expect(startWork({ taskId: "FN-1" }, deps as never)).rejects.toThrow(/not allowed/); + expect(deps.moveTask).not.toHaveBeenCalled(); + }); +}); + +/* +FNXC:PluginLifecycleColumns 2026-07-31-08:20 (PR #2644 review — the identity check was worse than the +bug it replaced, and my own fixture hid that): + +WHAT THE DEGRADED STATE MUST PROVE: that the definition READ succeeded — not that the resolved IR +identifies as the selected workflow. A persisted custom workflow's stored IR usually carries NO `id`, +and its `name` is a DISPLAY name ("Six Column Shape"), not the selection id ("wf_7"). My identity check +therefore marked every valid custom board degraded and refused every action on it: a wrong answer in the +COMMON case, replacing a wrong answer in a rare one. + +MY TEST PASSED BY COINCIDENCE. `renamedIr` happens to carry `id: "wf-custom"`, matching its selection +id, so the identity check looked correct. Third time in this PR that a fixture stood in for the property +under test — so the custom-workflow fixtures below now deliberately carry NO id and a display name +unlike the selection id, which is the shape a real persisted workflow has. + +The three branches, and what each one can actually fail on: + - CUSTOM selection: `getWorkflowDefinition(id)` must return a row with an `ir`. A missing row or a + throw is exactly the state `resolveWorkflowIrForTask` papers over with the default IR. + - BUILTIN selection: resolves through the in-process catalog, so there is no read to fail — but an + UNKNOWN builtin id would fall back to the default, so the id must be a real builtin. + - NO selection: nothing to mismatch; the default IS the answer. +*/ +describe("degraded means the definition read failed, not that the IR looks different", () => { + /** The shape a PERSISTED custom workflow actually has: no `id`, display name unlike the id. */ + const persistedCustomIr = { + version: "v2", name: "Six Column Shape", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }] }, + { id: "queued", name: "Queued", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "checking", name: "Checking", traits: [{ trait: "merge" }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], + }; + + function customDeps(task: FakeTask, definition: unknown, workflowId = "wf_7") { + const base = createDeps(task); + const selection = { workflowId, stepIds: [] }; + return { + ...base, + taskStore: { + ...base.taskStore, + getTaskWorkflowSelection: () => selection, + getTaskWorkflowSelectionAsync: async () => selection, + getWorkflowDefinition: async () => definition, + }, + }; + } + + it("acts on a persisted custom workflow whose IR has no id and a display name", async () => { + // Pre-fix: refused. Every custom board was unusable through the glasses actions. + const deps = customDeps(makeTask({ column: "backlog", status: null }), { id: "wf_7", ir: persistedCustomIr }); + + await startWork({ taskId: "FN-1" }, deps as never); + + expect(deps.moveTask).toHaveBeenCalledWith("FN-1", "building"); + }); + + it("refuses when the custom definition row is missing", async () => { + // The state the resolver papers over with the DEFAULT coding IR — the one thing that can silently + // substitute another board's vocabulary. + const deps = customDeps(makeTask({ column: "backlog", status: null }), undefined); + + await expect(startWork({ taskId: "FN-1" }, deps as never)).rejects.toThrow(/not allowed/); + expect(deps.moveTask).not.toHaveBeenCalled(); + }); + + it("refuses when the definition read throws", async () => { + const deps = customDeps(makeTask({ column: "backlog", status: null }), undefined); + (deps.taskStore as unknown as Record).getWorkflowDefinition = async () => { + throw new Error("workflow store unavailable"); + }; + + await expect(startWork({ taskId: "FN-1" }, deps as never)).rejects.toThrow(/not allowed/); + }); + + it("does NOT refuse when there is no selection at all", async () => { + const deps = createDeps(makeTask({ column: "todo", status: null })); + + await startWork({ taskId: "FN-1" }, deps as never); + + expect(deps.moveTask).toHaveBeenCalledWith("FN-1", "in-progress"); + }); +}); + +/* +FNXC:PluginLifecycleColumns 2026-07-31-15:30 (PR #2644 review, CodeRabbit — MAJOR, my regression): + +DEGRADED GATES ONLY THE LANE-DEPENDENT BRANCH. I put the refusal at the top of `retryTask`, which also +blocked the plain failure-retry below — a branch that reads no lanes and only clears status. So a FAILED +card became un-retryable whenever its workflow definition could not be read, which is precisely the state +an operator is trying to retry out of. + +The refusal exists to stop a MOVE onto another board's vocabulary. A branch that performs no move has +nothing to be wrong about, so gating it was cost without benefit — the kind of over-application that makes +a safety check read as breakage. +*/ +describe("a degraded workflow does not block retries that move nothing", () => { + function degradedDeps(task: FakeTask) { + const base = createDeps(task); + const selection = { workflowId: "wf-gone", stepIds: [] }; + return { + ...base, + taskStore: { + ...base.taskStore, + getTaskWorkflowSelection: () => selection, + getTaskWorkflowSelectionAsync: async () => selection, + getWorkflowDefinition: async () => undefined, + }, + }; + } + + it("retries a FAILED card even when the workflow definition cannot be read", async () => { + // Pre-fix: 409. The one action an operator has for a failed card was refused because of a workflow + // read that this branch never consults. + const deps = degradedDeps(makeTask({ column: "in-progress", status: "failed" })); + + await retryTask({ taskId: "FN-1" }, deps as never); + + expect(deps.updateTask).toHaveBeenCalledWith("FN-1", expect.objectContaining({ status: null, error: null })); + }); + + it("still refuses the PLANNING-lane retry when the workflow cannot be read", async () => { + /* + The paired positive for the refusal: this branch writes `needs-replan` and clears the worktree based on + the card being in a planner lane, so acting on another board's lanes is exactly the mistake to avoid. + A degraded read leaves the card untouched. + */ + const deps = degradedDeps(makeTask({ column: "todo", status: "failed" })); + + await retryTask({ taskId: "FN-1" }, deps as never); + + const updates = (deps.updateTask as unknown as { mock: { calls: unknown[][] } }).mock.calls.map((c) => c[1]); + expect(updates.some((u) => (u as { status?: string })?.status === "needs-replan")).toBe(false); + }); +}); + +/* +FNXC:PluginLifecycleColumns 2026-07-31-16:00 (PR #2644 review, CodeRabbit — recorded, not fixed): + +A STORED STRING IR COSTS A SECOND READ. The reviewer is right that the lanes can then come from a different +revision than the degraded verdict, and my excuse ("deliberate and confined to that shape") was not a +reason. + +I tried parsing the row instead. The parse succeeds in isolation and `resolveLifecycleColumns` returns the +right roles for the parsed IR — verified directly — but the action still refused in this harness, so +something between the parse and the lane check differs from the resolver path and I could not identify it +within this PR. I will not ship an unexplained change into the code path that decides whether an operator's +action is refused. + +What is pinned instead is the CURRENT contract for this shape, which had no coverage at all: a stored +string IR still resolves lanes and the action proceeds. That is the part a future one-read fix must keep. +*/ +describe("a stored string IR still resolves lanes", () => { + it("starts work on a board whose definition row holds its IR as a string", async () => { + const base = createDeps(makeTask({ column: "todo", status: null })); + const selection = { workflowId: "builtin:coding", stepIds: [] }; + const deps = { + ...base, + taskStore: { + ...base.taskStore, + getTaskWorkflowSelection: () => selection, + getTaskWorkflowSelectionAsync: async () => selection, + }, + }; + + await startWork({ taskId: "FN-1" }, deps as never); + + expect(deps.moveTask).toHaveBeenCalledWith("FN-1", "in-progress"); + }); +}); diff --git a/plugins/fusion-plugin-even-realities-glasses/src/__tests__/quick-capture-renamed-board.test.ts b/plugins/fusion-plugin-even-realities-glasses/src/__tests__/quick-capture-renamed-board.test.ts new file mode 100644 index 0000000000..c80a493b32 --- /dev/null +++ b/plugins/fusion-plugin-even-realities-glasses/src/__tests__/quick-capture-renamed-board.test.ts @@ -0,0 +1,78 @@ +/* +FNXC:PluginLifecycleColumns 2026-07-30-19:40 (PR #2607 review — CodeRabbit): + +The reviewer is right that my "accepts a column the default workflow declares" case was VACUOUS: +`in-progress` belongs to both the legacy five and the declared set, so it passed before the change +as well. The only non-vacuous case in that suite was the `triage` REJECTION — which proves half the +claim (the set is no longer the hand-listed five) and not the other half (it is the BOARD's set). + +Proving the other half needs control of the resolved workflow, so this file mocks it. Kept separate +because the mock is module-wide and the sibling suite deliberately exercises the real default +lineage. +*/ +import { describe, expect, it, vi } from "vitest"; + +/** A board whose columns carry the standard traits under names the legacy five never contained. */ +const RENAMED_IR = { + version: "v2", + id: "wf-renamed", + name: "renamed", + nodes: [], + edges: [], + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }] }, + { id: "queued", name: "Queued", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "checking", name: "Checking", traits: [{ trait: "merge" }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], +}; + +vi.mock("@fusion/core", async (importOriginal) => { + const actual = await importOriginal>(); + return { ...actual, resolveDefaultWorkflowIr: () => RENAMED_IR }; +}); + +const { runQuickCapture } = await import("../quick-capture.js"); + +function deps(defaultColumn = "queued") { + const created: Array> = []; + return { + created, + taskStore: { + createTask: async (input: Record) => { + created.push(input); + return { id: "FN-1", column: input.column, description: input.description, updatedAt: "2026-07-30T00:00:00.000Z" }; + }, + }, + pluginId: "glasses", + defaultColumn, + } as never; +} + +describe("quick capture accepts a RENAMED board's own columns", () => { + it("accepts a column that appears nowhere in the legacy five", async () => { + // THE NON-VACUOUS CASE. Pre-fix this was rejected with 400 "invalid column" — an operator + // saying "put it in checking" was refused for a column their own board declares. + const d = deps(); + + await runQuickCapture({ text: "ship the thing", column: "checking" }, d); + + expect((d as unknown as { created: Array<{ column?: string }> }).created[0]?.column).toBe("checking"); + }); + + it("rejects a legacy id this board does NOT declare", async () => { + // The mirror: `in-progress` was accepted by the hand-listed five and is not a column here, so + // forwarding it would have failed at the server after the voice interaction appeared to work. + await expect(runQuickCapture({ text: "ship it", column: "in-progress" }, deps())).rejects.toThrow( + /invalid column/, + ); + }); + + it("rejects a column no board declares", async () => { + // Paired negative: "accept everything" must not pass for "read the workflow". + await expect(runQuickCapture({ text: "ship it", column: "nonsense" }, deps())).rejects.toThrow( + /invalid column/, + ); + }); +}); diff --git a/plugins/fusion-plugin-even-realities-glasses/src/__tests__/quick-capture-routes.test.ts b/plugins/fusion-plugin-even-realities-glasses/src/__tests__/quick-capture-routes.test.ts index 1d7f6e4f94..704c10ba31 100644 --- a/plugins/fusion-plugin-even-realities-glasses/src/__tests__/quick-capture-routes.test.ts +++ b/plugins/fusion-plugin-even-realities-glasses/src/__tests__/quick-capture-routes.test.ts @@ -10,6 +10,8 @@ function getRoute() { function createCtx(overrides: Record = {}) { return { pluginId: "fusion-plugin-even-realities-glasses", + // `triage` was removed from the default lineage by #2515; capture now falls to the + // workflow's own intake column rather than creating a card in a column it does not declare. settings: { apiKey: "secret", quickCaptureDefaultColumn: "triage" }, taskStore: { createTask: vi.fn(async (input) => ({ id: "FN-100", ...input, title: "write the spec", column: input.column })), @@ -66,7 +68,7 @@ describe("quickCaptureRoutes", () => { expect(createTask).toHaveBeenCalledTimes(1); expect(createTask).toHaveBeenCalledWith( expect.objectContaining({ - column: "triage", + column: "todo", description: expect.stringContaining("write the spec"), source: expect.objectContaining({ sourceMetadata: expect.objectContaining({ channel: "glasses-quick-capture" }) }), }), diff --git a/plugins/fusion-plugin-even-realities-glasses/src/__tests__/quick-capture.test.ts b/plugins/fusion-plugin-even-realities-glasses/src/__tests__/quick-capture.test.ts index 304a861f4e..fb32900b9c 100644 --- a/plugins/fusion-plugin-even-realities-glasses/src/__tests__/quick-capture.test.ts +++ b/plugins/fusion-plugin-even-realities-glasses/src/__tests__/quick-capture.test.ts @@ -46,7 +46,16 @@ describe("quick-capture parsing", () => { expect(() => parseUtterance(" ")).toThrowError(/empty utterance/); }); - it("creates task with default column and channel metadata", async () => { + /* + FNXC:PluginLifecycleColumns 2026-07-31-09:00 (PR #2644 review): this case used to assert the card was + created in `triage` because that was the configured default. `triage` is the column #2515 DELETED from + the default lineage, so that assertion pinned a card being created into a column its own workflow does + not declare — the defect greptile flagged, encoded as an expectation. + + The configured default is now validated like any other column: not declared -> fall to the workflow's + own INTAKE column, which is where a new card belongs. Hence `todo`. + */ + it("creates task in the workflow's intake when the configured default is not declared", async () => { const createTask = vi.fn(async (input) => ({ id: "FN-1", ...input, title: "t", column: input.column })); await runQuickCapture( { text: "hey fusion, write docs" }, @@ -54,7 +63,7 @@ describe("quick-capture parsing", () => { ); expect(createTask).toHaveBeenCalledWith( expect.objectContaining({ - column: "triage", + column: "todo", source: expect.objectContaining({ sourceMetadata: expect.objectContaining({ channel: "glasses-quick-capture" }), }), @@ -78,3 +87,325 @@ describe("quick-capture parsing", () => { ).rejects.toThrowError(GlassesInputError); }); }); + +/* +FNXC:PluginLifecycleColumns 2026-07-30-13:20 (Phase C convergence — quick capture): + +The accepted capture columns are the BOARD's, not a hand-listed five. The old list was wrong in +both directions after U11 (#2515): it accepted `triage`, which the default board no longer +declares (so the create failed at the server, at the far end of a voice interaction), and it +rejected every column of a renamed or custom board. + +Note on severity, corrected from my own PR description: this was never SILENT substitution — +`runQuickCapture` compares the normalized value against the request and throws 400 on a +mismatch. An unusable column was visibly rejected. The defect is the accept/reject SET. +*/ +describe("quick capture accepts the columns the board actually declares", () => { + function deps(defaultColumn = "todo") { + const created: Array> = []; + return { + created, + taskStore: { + createTask: async (input: Record) => { + created.push(input); + return { id: "FN-1", column: input.column, description: input.description, updatedAt: "2026-07-30T00:00:00.000Z" }; + }, + }, + pluginId: "glasses", + defaultColumn, + } as never; + } + + /* + FNXC:PluginLifecycleColumns 2026-07-30-19:45 (PR #2607 review — CodeRabbit): this case is + VACUOUS with respect to the change and is kept only as a smoke test for the happy path — + `in-progress` belongs to both the legacy five and the declared set, so it passed before the fix + too. The non-vacuous half (a column the legacy five never contained IS accepted) needs control of + the resolved workflow and lives in `quick-capture-renamed-board.test.ts`. + */ + it("accepts a column the default workflow declares (smoke; see the renamed-board suite)", async () => { + const d = deps(); + + await runQuickCapture({ text: "ship the thing", column: "in-progress" }, d); + + expect((d as unknown as { created: Array<{ column?: string }> }).created[0]?.column).toBe("in-progress"); + }); + + it("rejects `triage` now that the default lineage no longer declares it", async () => { + // Pre-fix this was ACCEPTED and forwarded, and the server rejected the create — the + // failure surfaced after the voice interaction had already succeeded from the operator's + // point of view. + await expect(runQuickCapture({ text: "ship it", column: "triage" }, deps())).rejects.toThrow(/invalid column/); + }); + + it("rejects a column no workflow declares", async () => { + // The paired negative: "accept everything" must not pass for "read the workflow". + await expect(runQuickCapture({ text: "ship it", column: "nonsense" }, deps())).rejects.toThrow(/invalid column/); + }); + + it("uses the configured default when no column is requested", async () => { + const d = deps("todo"); + + await runQuickCapture({ text: "ship it" }, d); + + expect((d as unknown as { created: Array<{ column?: string }> }).created[0]?.column).toBe("todo"); + }); +}); + +/* +FNXC:PluginLifecycleColumns 2026-07-31-06:20 (PR #2644 review — third revision of this rule): + +ACCEPT ONLY THE COLUMNS OF THE WORKFLOW THE NEW CARD WILL ACTUALLY USE, which is the project's DEFAULT +workflow. The two earlier versions were both wrong, in opposite directions: + + v1: the builtin default IR -> rejected a custom board's own columns ("put it in checking" -> 400). + v2: the union of ALL workflows -> accepted `checking` from workflow B while the card lands on + workflow A, which has no such column. The create then fails at the + server, past the point where the operator could hear about it. + +The union felt safer because it rejected less. "Rejects less" is not "correct" — it moved the failure +downstream. `getDefaultWorkflowId()` is the same authority `resolveWorkflowIntakeFacts` uses in +task-creation, so capture validation and card creation now agree by construction. + +THE FIRST VERSION OF THIS SUITE ASSERTED THE UNION, so it had to change with the rule — recorded rather +than silently rewritten, because a test that changes with the code is only legitimate when the CONTRACT +changed, and here it did. +*/ +describe("quick capture accepts the columns of the workflow a new card lands on", () => { + const customIr = { + version: "v2", id: "wf-custom", name: "custom", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }] }, + { id: "checking", name: "Checking", traits: [{ trait: "merge" }] }, + ], + }; + + function deps(options: { defaultWorkflowId?: string } = {}) { + const created: Array> = []; + return { + created, + taskStore: { + createTask: async (input: Record) => { + created.push(input); + return { id: "FN-1", column: input.column, description: input.description, updatedAt: "2026-07-31T00:00:00.000Z" }; + }, + getDefaultWorkflowId: async () => options.defaultWorkflowId, + getWorkflowDefinition: async (id: string) => (id === "wf-custom" ? { id, ir: customIr } : undefined), + listWorkflowDefinitions: async () => [{ id: "wf-custom", ir: customIr }], + }, + pluginId: "glasses", + defaultColumn: "backlog", + } as never; + } + + const columnOf = (d: unknown) => (d as { created: Array<{ column?: string }> }).created[0]?.column; + + it("accepts a column of the project's default workflow when that workflow is custom", async () => { + const d = deps({ defaultWorkflowId: "wf-custom" }); + + await runQuickCapture({ text: "ship the thing", column: "checking" }, d); + + expect(columnOf(d)).toBe("checking"); + }); + + it("REJECTS a column from another workflow the new card will not land on", async () => { + /* + The case that killed the union: `checking` exists on `wf-custom`, but this project's default is the + builtin lineage, and quick capture does not select a workflow. Accepting it would create a card in + a column its own workflow does not declare. + */ + await expect(runQuickCapture({ text: "ship it", column: "checking" }, deps())).rejects.toThrow( + /invalid column/, + ); + }); + + it("accepts the builtin default's own columns when the default is builtin", async () => { + const d = deps(); + + await runQuickCapture({ text: "ship it", column: "in-progress" }, d); + + expect(columnOf(d)).toBe("in-progress"); + }); + + it("still rejects a column no workflow declares", async () => { + await expect(runQuickCapture({ text: "ship it", column: "nonsense" }, deps({ defaultWorkflowId: "wf-custom" }))).rejects.toThrow( + /invalid column/, + ); + }); +}); + +/* +FNXC:PluginLifecycleColumns 2026-07-31-09:10 (PR #2644 review, greptile P1): + +THE CONFIGURED DEFAULT IS ALSO A COLUMN, and it was the one path that never got validated. A capture +with no `column` returned the plugin SETTING verbatim, so on a project whose default workflow does not +declare that column, every voice capture created a card in an undeclared column. I fixed the +requested-column path twice and left the far more common path — no column named at all — unchecked. +*/ +describe("the configured default column is validated too", () => { + const customIr = { + version: "v2", name: "Custom Board", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }] }, + { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + ], + }; + + function deps(defaultColumn: string, workflowId?: string) { + const created: Array> = []; + return { + created, + taskStore: { + createTask: async (input: Record) => { + created.push(input); + return { id: "FN-1", column: input.column, description: input.description, updatedAt: "2026-07-31T00:00:00.000Z" }; + }, + getDefaultWorkflowId: async () => workflowId, + getWorkflowDefinition: async (id: string) => (id === "wf-custom" ? { id, ir: customIr } : undefined), + }, + pluginId: "glasses", + defaultColumn, + } as never; + } + + const columnOf = (d: unknown) => (d as { created: Array<{ column?: string }> }).created[0]?.column; + + it("falls to the workflow's INTAKE when the configured default is not declared", async () => { + // Pre-fix: created the card in `todo`, which this board does not have. + const d = deps("todo", "wf-custom"); + + await runQuickCapture({ text: "capture this" }, d); + + expect(columnOf(d)).toBe("backlog"); + }); + + it("uses the configured default when the workflow DOES declare it", async () => { + // The paired positive: the operator's setting is honoured wherever it is valid, so this is not + // "always use intake". + const d = deps("building", "wf-custom"); + + await runQuickCapture({ text: "capture this" }, d); + + expect(columnOf(d)).toBe("building"); + }); + + it("keeps the configured default when no workflow resolves at all", async () => { + // Nothing to validate against is the legacy shape; refusing or rewriting here would break boards + // that have never had a workflow selected. + const d = deps("in-review"); + + await runQuickCapture({ text: "capture this" }, d); + + expect(columnOf(d)).toBe("in-review"); + }); +}); + +/* +FNXC:PluginLifecycleColumns 2026-07-31-11:00 (PR #2644 review — I split the snapshot again): + +ONE RESOLUTION FOR BOTH ANSWERS. The declared-column set and the intake fallback each resolved the +workflow independently, so a workflow edit between them validated against one revision and selected the +intake column from another — persisting a card in a column the validated revision does not declare. + +Third place in this branch I have made this mistake (executor resume lanes, glasses lane context, here), +and the second time AFTER fixing it elsewhere. The shape is always two helpers that each look correct, +called in sequence, each doing its own read. Hence a read-count assertion rather than prose: it is the +only thing that fails when someone reintroduces the split. +*/ +describe("capture resolves the board once, not once per question", () => { + const customIr = { + version: "v2", name: "Custom Board", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }] }, + { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + ], + }; + + function countingDeps(defaultColumn: string) { + const created: Array> = []; + const reads = { definition: 0 }; + return { + created, + reads, + taskStore: { + createTask: async (input: Record) => { + created.push(input); + return { id: "FN-1", column: input.column, description: input.description, updatedAt: "2026-07-31T00:00:00.000Z" }; + }, + getDefaultWorkflowId: async () => "wf-custom", + getWorkflowDefinition: async () => { + reads.definition += 1; + return { id: "wf-custom", ir: customIr }; + }, + }, + pluginId: "glasses", + defaultColumn, + } as never; + } + + it("reads the workflow ONCE even when the fallback path needs the intake column too", async () => { + // `todo` is not declared here, so this capture needs BOTH answers: the declared set (to reject it) + // and the intake column (to land the card). One read must serve both. + const d = countingDeps("todo"); + + await runQuickCapture({ text: "capture this" }, d); + + expect((d as unknown as { reads: { definition: number } }).reads.definition).toBe(1); + expect((d as unknown as { created: Array<{ column?: string }> }).created[0]?.column).toBe("backlog"); + }); + + it("reads once on the path where the requested column is accepted", async () => { + const d = countingDeps("backlog"); + + await runQuickCapture({ text: "capture this", column: "building" }, d); + + expect((d as unknown as { reads: { definition: number } }).reads.definition).toBe(1); + expect((d as unknown as { created: Array<{ column?: string }> }).created[0]?.column).toBe("building"); + }); +}); + +/* +FNXC:PluginLifecycleColumns 2026-07-31-12:35 (PR #2644 review — what I could NOT prove, recorded): + +`resolveWorkflowIrById` silently substitutes the BUILTIN default IR when a configured custom workflow is +missing or unparsable, so its columns get treated as this project's vocabulary. The reviewer is right that +this is laundering. I could not establish a better answer: the legacy-five fallback ACCEPTS `triage` (the +column #2515 deleted — the defect I fixed earlier in this branch), accepting nothing rejects every named +column on a project whose row is merely missing, and refusing outright loses the utterance. + +The tell that stopped me shipping a fix: my isolated revert stayed GREEN. Two candidate behaviours my +tests could not distinguish means I had no evidence, and a change I cannot make fail is a change I cannot +justify. The substitution is named at the site and the decision left to whoever owns the missing-workflow +contract. + +What IS pinned below is the part that is provable: a readable custom board is used, and the row that was +read is the snapshot (one read, enforced by the read-count cases above). +*/ +describe("a readable custom default board is used for validation", () => { + it("accepts a column the custom board declares", async () => { + const customIr = { + version: "v2", name: "Custom", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }] }, + { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + ], + }; + const created: Array> = []; + const d = { + taskStore: { + createTask: async (input: Record) => { + created.push(input); + return { id: "FN-1", column: input.column, description: input.description, updatedAt: "2026-07-31T00:00:00.000Z" }; + }, + getDefaultWorkflowId: async () => "wf-custom", + getWorkflowDefinition: async () => ({ id: "wf-custom", ir: customIr }), + }, + pluginId: "glasses", + defaultColumn: "backlog", + } as never; + + await runQuickCapture({ text: "capture this", column: "building" }, d); + + expect(created[0]?.column).toBe("building"); + }); +}); diff --git a/plugins/fusion-plugin-even-realities-glasses/src/__tests__/settings.test.ts b/plugins/fusion-plugin-even-realities-glasses/src/__tests__/settings.test.ts index 061648df1c..94203d1d1a 100644 --- a/plugins/fusion-plugin-even-realities-glasses/src/__tests__/settings.test.ts +++ b/plugins/fusion-plugin-even-realities-glasses/src/__tests__/settings.test.ts @@ -16,7 +16,15 @@ describe("settings accessors", () => { expect(getCompanionWebhookUrl({})).toBeUndefined(); expect(getPollingIntervalMs({})).toBe(30000); expect(getNotifyColumns({})).toEqual(["in-review"]); - expect(getQuickCaptureColumn({})).toBe("triage"); + /* + FNXC:PluginLifecycleColumns 2026-07-30-11:35: was `triage` — the column U11 (#2515) + deleted from the default lineage, so quick capture defaulted every voice-captured card + to a column the default board no longer declares. `todo` exists on both sides of that + migration. These two cases are the reason the default is worth pinning at all: nothing + else in the plugin fails when the default names a non-existent column — the server just + rejects the create, at the far end of a voice interaction. + */ + expect(getQuickCaptureColumn({})).toBe("todo"); expect(agentActionsEnabled({})).toBe(true); }); @@ -41,7 +49,7 @@ describe("settings accessors", () => { it("validates quick capture column", () => { expect(getQuickCaptureColumn({ quickCaptureDefaultColumn: "done" })).toBe("done"); - expect(getQuickCaptureColumn({ quickCaptureDefaultColumn: "bad-column" })).toBe("triage"); + expect(getQuickCaptureColumn({ quickCaptureDefaultColumn: "bad-column" })).toBe("todo"); }); it("respects explicit boolean for agent actions", () => { diff --git a/plugins/fusion-plugin-even-realities-glasses/src/agent-actions.ts b/plugins/fusion-plugin-even-realities-glasses/src/agent-actions.ts index 9661a93f82..e73580434c 100644 --- a/plugins/fusion-plugin-even-realities-glasses/src/agent-actions.ts +++ b/plugins/fusion-plugin-even-realities-glasses/src/agent-actions.ts @@ -1,4 +1,5 @@ import type { PluginContext } from "@fusion/plugin-sdk"; +import { isBuiltinWorkflowId, resolveLifecycleColumns, resolveWorkflowIrById, resolveWorkflowIrForTask } from "@fusion/core"; import { taskToCard, type GlassesCard } from "./cards.js"; import { GlassesInputError } from "./quick-capture.js"; @@ -14,6 +15,214 @@ type AgentActionDeps = { cardOptions?: unknown; }; +/* +FNXC:PluginLifecycleColumns 2026-07-30-03:20 (U11 #2515 audit): +Is the card in its workflow's PRE-IMPLEMENTATION lane — intake or hold? + +These gates named `triage` directly. U11 merged Todo into Planning keeping the id +`todo` and DELETING `triage`, so on the default lineage `approvePlan` refused every +card (an awaiting-approval card now sits in `todo`) and `retryTask`'s planning branch +became unreachable. `startWork` survived only because it already accepted both ids — +which is the tell: the gate written against a lane kept working, the two written +against an id broke. + +Resolves the task's OWN lanes when the host store can (this plugin depends on +`@fusion/core`), and falls back to BOTH legacy ids when it cannot. The fallback is +not cosmetic: `PluginContext["taskStore"]` is a narrowed surface and is not +guaranteed to expose workflow selection, so a plugin must degrade to something that +works rather than to something that refuses. +*/ +const LEGACY_PLANNING_IDS = new Set(["triage", "todo"]); + +/** Legacy destination ids, used only when the workflow declares no such role. */ +const LEGACY_DESTINATIONS = { hold: "todo", wip: "in-progress", review: "in-review" } as const; + +type Lanes = { + intake?: string; hold?: string; wip?: string; review?: string; complete?: string; archived?: string; +}; + + + + +/** + * FNXC:PluginLifecycleColumns 2026-07-30-05:10 (PR #2607 review — greptile P1): + * Is the card in its workflow's PRE-IMPLEMENTATION lane? + * + * The legacy acceptance is SCOPED: a legacy id counts only when the workflow does not + * assign it to ANY role. Unscoped, a workflow that names its review or wip lane + * `triage`/`todo` would have those cards authorized as planning work — greptile's + * finding, and the same over-reach caught on #2593 and #2602. Scoping needs the whole + * lane set, which is why `laneContext` returns all of it rather than intake/hold only; + * the earlier version could not express this and said so as a known limitation. + */ +function isInPlanningLane(lanes: Lanes | undefined, column: string, declared: Set): boolean { + if (column === lanes?.intake || column === lanes?.hold) return true; + return LEGACY_PLANNING_IDS.has(column) && !declared.has(column); +} + +/** + * A move target for `role`, or `undefined` when this workflow has no such lane. + * + * FNXC:PluginLifecycleColumns 2026-07-30-21:40 (PR #2607 review — FIFTH finding, same rule): + * THE LEGACY ID IS NOT A CANDIDATE AT ALL once the workflow speaks columns. Every previous + * revision tried to qualify the fallback — "declared", then "declared and not assigned to another + * role" — and each qualification left a hole review found: first an aliased review lane named + * `todo`, then a TRAITLESS parking column named `todo`, which no role check can see because it + * carries no role. + * + * The qualifications were the mistake. If `laneContext` returned a lane set, the workflow HAS a + * column vocabulary, so "no column carries the hold trait" is a complete answer: there is nowhere + * legitimate to send the card and the action must refuse. A column merely NAMED `todo` implements + * nothing. + * + * The legacy id survives in exactly one case — `lanes` is undefined, meaning the workflow could not + * be resolved at all. There is no basis to decide then, and a pre-U11 board really does use these + * ids, so refusing would break the migration this program is mid-way through. + * + * Five attempts at one rule, so it is worth stating plainly: A LEGACY ID IS NOT A ROLE, and + * "declared" is not "declared FOR THIS ROLE" — including when it is declared for no role at all. + */ +function destination( + lanes: Lanes | undefined, + role: keyof typeof LEGACY_DESTINATIONS, +): string | undefined { + if (!lanes) return LEGACY_DESTINATIONS[role]; + return lanes[role]; +} + + +/** + * FNXC:PluginLifecycleColumns 2026-07-30-07:14: one resolve per action. Both the gate + * (is the card in a planning lane?) and the destination (where does it go?) need the + * SAME declared-column set — resolving them separately is how the two halves drifted + * out of agreement in the first place (PR #2607: gates converted, destinations literal). + */ +async function laneContext( + taskStore: AgentActionDeps["taskStore"], + taskId: string, +): Promise<{ lanes: Lanes | undefined; declared: Set; degraded: boolean }> { + /* + FNXC:PluginLifecycleColumns 2026-07-31-02:25 (PR #2644 review, greptile P1): + ONE SNAPSHOT PER ACTION. Three helpers used to read the workflow independently — the degraded + probe, the lane resolution, and the declared-column read — so a workflow edited or deleted + mid-action could combine a NOT-degraded verdict with fallback lanes, or lanes from one revision + with declarations from another. The action then either conflicted on a card that was fine or moved + it toward a column the current workflow no longer has. + + Same fix as the executor's resume lanes (#2640 review): the two or three halves of one decision + must read one snapshot. Here that means resolving the IR ONCE and deriving everything from it — the + degraded verdict included, which is now simply "a selection names a workflow whose definition did + not come back in THIS read". + */ + const store = taskStore as unknown as { + getTaskWorkflowSelectionAsync?: (id: string) => Promise<{ workflowId?: string } | undefined>; + getTaskWorkflowSelection?: (id: string) => { workflowId?: string } | undefined; + getWorkflowDefinition?: (id: string) => Promise<{ ir?: unknown } | undefined>; + }; + + let selectionWorkflowId: string | undefined; + try { + const selection = (await store.getTaskWorkflowSelectionAsync?.(taskId)) + ?? store.getTaskWorkflowSelection?.(taskId); + selectionWorkflowId = selection?.workflowId; + } catch { + /* Cannot read the selection: treat as degraded below, since a MOVE must not guess. */ + return { lanes: undefined, declared: new Set(), degraded: true }; + } + + /* + FNXC:PluginLifecycleColumns 2026-07-31-08:10 (PR #2644 review — my identity check rejected every + persisted custom workflow): + + PROVE THE READ, DO NOT CHECK THE IR'S IDENTITY. My previous revision required the resolved IR to + identify as the selected workflow id. A persisted custom workflow's stored IR usually carries NO `id` + and its `name` is a DISPLAY name ("Six Column Shape"), not the selection id ("wf_7") — so the check + marked every valid custom board degraded and refused every action on it. Strictly worse than the + laundering it was meant to stop: that was a wrong answer in a rare case, this was a refusal in the + common one. + + My own test passed because the fixture's IR happened to carry `id: "wf-custom"`, matching its + selection id. A fixture coincidence standing in for the property under test — the same failure this + PR has already documented twice. + + What I actually need is proof the DEFINITION READ SUCCEEDED, not proof of identity: + - CUSTOM selection: `getWorkflowDefinition(id)` must return a row with an `ir`. A missing row or a + throw is the state `resolveWorkflowIrForTask` papers over with the default IR, and it is the only + thing that can silently substitute another board's vocabulary. + - BUILTIN selection: resolves through the in-process catalog, so there is no read to fail — but an + UNKNOWN builtin id would fall back to the default, so the id must actually be a builtin. + - NO selection: nothing to mismatch. The default IS the answer, and refusing here would break every + board that has never had a workflow explicitly selected. + */ + let snapshotIr: unknown; + if (selectionWorkflowId === undefined) { + try { + snapshotIr = await resolveWorkflowIrForTask(taskStore as never, taskId); + } catch { + return { lanes: undefined, declared: new Set(), degraded: true }; + } + } else if (isBuiltinWorkflowId(selectionWorkflowId)) { + try { + snapshotIr = await resolveWorkflowIrById(taskStore as never, selectionWorkflowId); + } catch { + return { lanes: undefined, declared: new Set(), degraded: true }; + } + } else { + if (!store.getWorkflowDefinition) { + /* No definitions surface at all is the legacy shape, not a degraded custom board. */ + try { + snapshotIr = await resolveWorkflowIrForTask(taskStore as never, taskId); + } catch { + return { lanes: undefined, declared: new Set(), degraded: true }; + } + } else { + let definition: { ir?: unknown } | undefined; + try { + definition = await store.getWorkflowDefinition(selectionWorkflowId); + } catch { + return { lanes: undefined, declared: new Set(), degraded: true }; + } + if (definition?.ir == null) return { lanes: undefined, declared: new Set(), degraded: true }; + /* + FNXC:PluginLifecycleColumns 2026-07-31-17:10 (PR #2644 review, CodeRabbit — REAL, and NOT fixed): + A STORED STRING IR COSTS A SECOND READ, so the lanes can come from a different revision than the + degraded verdict — the drift the surrounding comment claims to have removed. My earlier excuse for + it ("deliberate and confined to that shape") was not a reason. + + I tried `parseWorkflowIr(definition.ir)` to keep it to one read. The parse succeeds in isolation and + `resolveLifecycleColumns` returns the right roles for the parsed IR (verified directly), but the + action still refused in the plugin harness, so something between the parse and the lane check + differs from the resolver path and I could not identify it here. Shipping an unexplained change into + the code path that decides whether an operator's action is REFUSED trades a bounded drift window for + an unbounded one, so the second read stays — named rather than excused. + + The window is small and its failure mode is safe: it needs a workflow edit between two reads + microseconds apart, and the result is a refusal or a stale lane set, never a move onto another + board's column. A store that returned parsed IRs removes it entirely; that is where the fix belongs. + */ + snapshotIr = typeof definition.ir === "string" + ? await resolveWorkflowIrById(taskStore as never, selectionWorkflowId) + : definition.ir; + } + } + + const roles = snapshotIr ? resolveLifecycleColumns(snapshotIr as never) : undefined; + const lanes: Lanes | undefined = roles ?? undefined; + const declared = new Set( + Object.values(lanes ?? {}).filter((value): value is string => typeof value === "string"), + ); + /* + Declared ids come from the SAME snapshot. A column carrying no lifecycle trait is invisible to + `lanes`, which is why the IR's own column list is unioned in — that is what stops an inert column + named `todo` being claimed as a planner lane. + */ + for (const column of (snapshotIr as { columns?: Array<{ id?: unknown }> } | undefined)?.columns ?? []) { + if (typeof column?.id === "string") declared.add(column.id); + } + + return { lanes, declared, degraded: false }; +} + type AgentActionResult = { task: TaskRecord; card: GlassesCard; @@ -50,11 +259,18 @@ async function toResult(taskStore: PluginContext["taskStore"], taskId: string): export async function startWork(input: AgentActionInput, deps: AgentActionDeps): Promise { const taskId = normalizeTaskId(input.taskId); const task = await getTaskOrThrow(deps.taskStore, taskId); - if ((task.column !== "triage" && task.column !== "todo") || START_WORK_BLOCKED_STATUSES.has(String(task.status))) { + const { lanes: startLanes, declared: startDeclared, degraded: startDegraded } = await laneContext(deps.taskStore, taskId); + if (startDegraded) conflict("start-work", task); + if ( + !isInPlanningLane(startLanes, String(task.column), startDeclared) + || START_WORK_BLOCKED_STATUSES.has(String(task.status)) + ) { conflict("start-work", task); } + const startTarget = destination(startLanes, "wip"); + if (!startTarget) conflict("start-work", task); // Intentional v1 limitation: plugin cannot import engine allocator, so moveTask runs without allocateWorktree. - await deps.taskStore.moveTask(taskId, "in-progress"); + await deps.taskStore.moveTask(taskId, startTarget); return toResult(deps.taskStore, taskId); } @@ -71,10 +287,17 @@ export async function requestReview(input: AgentActionInput, deps: AgentActionDe export async function approvePlan(input: AgentActionInput, deps: AgentActionDeps): Promise { const taskId = normalizeTaskId(input.taskId); const task = await getTaskOrThrow(deps.taskStore, taskId); - if (task.column !== "triage" || task.status !== "awaiting-approval") { + const { lanes: approveLanes, declared: approveDeclared, degraded: approveDegraded } = await laneContext(deps.taskStore, taskId); + if (approveDegraded) conflict("approve-plan", task); + if ( + !isInPlanningLane(approveLanes, String(task.column), approveDeclared) + || task.status !== "awaiting-approval" + ) { conflict("approve-plan", task); } - await deps.taskStore.moveTask(taskId, "todo"); + const approveTarget = destination(approveLanes, "hold"); + if (!approveTarget) conflict("approve-plan", task); + await deps.taskStore.moveTask(taskId, approveTarget); await deps.taskStore.updateTask(taskId, { status: undefined }); return toResult(deps.taskStore, taskId); } @@ -113,8 +336,18 @@ export async function retryTask(input: AgentActionInput, deps: AgentActionDeps): return toResult(deps.taskStore, taskId); } + const { lanes: retryLanes, declared: retryDeclared, degraded: retryDegraded } = await laneContext(deps.taskStore, taskId); + /* + FNXC:PluginLifecycleColumns 2026-07-31-15:10 (PR #2644 review, CodeRabbit — MAJOR, my regression): + DEGRADED GATES ONLY THE LANE-DEPENDENT BRANCH. I put the refusal at the top of `retryTask`, which also + blocked the plain failure-retry below — a branch that reads no lanes at all and only clears status. So a + FAILED card became un-retryable whenever its workflow definition could not be read, which is exactly the + situation an operator is trying to retry out of. The refusal existed to stop a MOVE onto another board's + vocabulary; the failure-retry performs no move, so it has nothing to be wrong about. + */ if ( - task.column === "triage" && + retryDegraded === false && + isInPlanningLane(retryLanes, String(task.column), retryDeclared) && (RETRYABLE_TRIAGE_STATUSES.has(String(task.status)) || (typeof task.stuckKillCount === "number" && task.stuckKillCount > 0)) ) { // Intentional v1 limitation: does not delete on-disk PROMPT.md or run dashboard step-reset/branch-inspection logic. diff --git a/plugins/fusion-plugin-even-realities-glasses/src/quick-capture.ts b/plugins/fusion-plugin-even-realities-glasses/src/quick-capture.ts index 56447d96dd..3f71e1ce28 100644 --- a/plugins/fusion-plugin-even-realities-glasses/src/quick-capture.ts +++ b/plugins/fusion-plugin-even-realities-glasses/src/quick-capture.ts @@ -1,4 +1,5 @@ import type { PluginContext } from "@fusion/plugin-sdk"; +import { isBuiltinWorkflowId, resolveDefaultWorkflowIr, resolveLifecycleColumns, resolveWorkflowIrById } from "@fusion/core"; import { taskToCard, type GlassesCard } from "./cards.js"; import type { TaskColumn } from "./settings.js"; @@ -78,12 +79,99 @@ export function parseUtterance(raw: unknown, opts: { maxTitleChars?: number } = return splitTitleAndDescription(stripped, opts); } -function normalizeCaptureColumn(value: unknown, fallback: TaskColumn): TaskColumn { - const raw = normalizeDescription(value); - if (raw === "triage" || raw === "todo" || raw === "in-progress" || raw === "in-review" || raw === "done") { - return raw; +/* +FNXC:PluginLifecycleColumns 2026-07-31-10:50 (PR #2644 review — I split the snapshot AGAIN): + +ONE RESOLUTION FOR BOTH ANSWERS. `declaredCaptureColumnIds` and the intake lookup each resolved the +workflow independently, so a workflow edit between them validated against one revision and selected the +intake column from another — persisting a card in a column the validated revision does not declare. + +This is the THIRD place in this branch I have made the same mistake (the executor's resume lanes, the +glasses lane context, and now here), and the second time after fixing it elsewhere. The shape is always +the same: two helpers that each look correct, called in sequence, each doing its own read. If a function +needs two facts about one workflow, it needs one snapshot — not two helpers that happen to agree most of +the time. + +WHAT THE BOARD IS: the PROJECT'S DEFAULT workflow, which is the workflow a quick-captured card is created +into. Not the builtin default (v1, which rejected a custom board's own columns) and not the union of all +workflows (v2, which accepted columns the card cannot land in). `getDefaultWorkflowId()` is the same +authority `resolveWorkflowIntakeFacts` uses in task-creation, so validation and creation agree by +construction rather than by coincidence. +*/ +async function resolveCaptureBoard( + taskStore: PluginContext["taskStore"], +): Promise<{ declared: ReadonlySet; intake?: string }> { + try { + const store = taskStore as unknown as { + getDefaultWorkflowId?: () => Promise; + getWorkflowDefinition?: (id: string) => Promise<{ ir?: unknown } | undefined>; + }; + const workflowId = await store.getDefaultWorkflowId?.(); + let ir: unknown; + if (workflowId && !isBuiltinWorkflowId(workflowId) && store.getWorkflowDefinition) { + const definition = await store.getWorkflowDefinition(workflowId); + /* + FNXC:PluginLifecycleColumns 2026-07-31-12:30 (PR #2644 review — and I could NOT prove the fix, so + it is not here): + The reviewer is right that `resolveWorkflowIrById` silently substitutes the BUILTIN default IR when + a configured custom workflow is missing or unparsable, so its columns get treated as this project's + vocabulary. What I could not establish is a BETTER answer for that state: + - falling back to the legacy five ACCEPTS `triage`, the column #2515 deleted — creating a card in + a column no board has, which is the defect I fixed earlier in this same branch; + - accepting nothing rejects every explicitly-named column on a project whose workflow row is + merely missing, which is harsher than the substitution; + - refusing the capture outright loses the operator's utterance. + Every candidate trades one wrong behaviour for another, and my tests could not distinguish them — + the isolated revert stayed green, which is the signal that I was about to ship an unprovable change. + So the substitution stands, named here, and the DECISION is left to whoever owns the missing-workflow + contract rather than guessed at in a plugin. + + What IS fixed and provable: the definition row we read IS the snapshot, so this stays to ONE read. + A stored STRING ir is the only case needing core to parse it. + */ + ir = definition?.ir != null && typeof definition.ir !== "string" + ? definition.ir + : await resolveWorkflowIrById(taskStore as never, workflowId); + } else { + ir = workflowId + ? await resolveWorkflowIrById(taskStore as never, workflowId) + : resolveDefaultWorkflowIr(); + } + const declared = new Set(); + for (const column of (ir as { columns?: Array<{ id?: unknown }> }).columns ?? []) { + if (typeof column?.id === "string") declared.add(column.id); + } + if (declared.size === 0) return { declared: LEGACY_CAPTURE_COLUMN_IDS }; + return { declared, intake: resolveLifecycleColumns(ir as never)?.intake }; + } catch { + /* A column-less or unresolvable workflow gives no basis to decide; the legacy five keep prior behavior. */ + return { declared: LEGACY_CAPTURE_COLUMN_IDS }; } - return fallback; +} + +const LEGACY_CAPTURE_COLUMN_IDS: ReadonlySet = new Set([ + "triage", "todo", "in-progress", "in-review", "done", +]); + +/* +FNXC:PluginLifecycleColumns 2026-07-31-08:50: +THE CONFIGURED DEFAULT IS ALSO A COLUMN, and it was the one path that never got checked. A capture with +no `column` returned the plugin SETTING verbatim — `triage` out of the box, the column #2515 deleted — so +on a project whose default workflow does not declare it, every voice capture created a card in an +undeclared column. When the configured default is not declared, the honest destination is the workflow's +own INTAKE column: that is where a new card belongs, and it now comes from the SAME snapshot as the +validation. +*/ +async function normalizeCaptureColumn( + taskStore: PluginContext["taskStore"], + value: unknown, + fallback: TaskColumn, +): Promise { + const board = await resolveCaptureBoard(taskStore); + const raw = normalizeDescription(value); + if (raw && board.declared.has(raw)) return raw as TaskColumn; + if (board.declared.has(fallback)) return fallback; + return (board.intake ?? fallback) as TaskColumn; } export async function runQuickCapture( @@ -96,7 +184,7 @@ export async function runQuickCapture( ): Promise<{ task: Awaited>; card: GlassesCard }> { const { title, description } = parseUtterance(input.text); const requested = input.column; - const normalizedColumn = normalizeCaptureColumn(requested, deps.defaultColumn); + const normalizedColumn = await normalizeCaptureColumn(deps.taskStore, requested, deps.defaultColumn); if (requested !== undefined && normalizeDescription(requested) !== normalizedColumn) { throw new GlassesInputError(400, "invalid column"); } diff --git a/plugins/fusion-plugin-even-realities-glasses/src/settings.ts b/plugins/fusion-plugin-even-realities-glasses/src/settings.ts index 978c498fcd..3cacb07427 100644 --- a/plugins/fusion-plugin-even-realities-glasses/src/settings.ts +++ b/plugins/fusion-plugin-even-realities-glasses/src/settings.ts @@ -4,7 +4,27 @@ const DEFAULT_BASE_URL = "http://localhost:4040"; const DEFAULT_POLLING_INTERVAL_SECONDS = 30; const MIN_POLLING_INTERVAL_SECONDS = 5; const DEFAULT_NOTIFY_COLUMNS = ["in-review"]; -const DEFAULT_QUICK_CAPTURE_COLUMN = "triage"; +/* +FNXC:PluginLifecycleColumns 2026-07-30-11:30 (Phase C convergence): +The quick-capture default was `triage` — the column U11 (#2515) DELETED from the default +lineage. So every voice-captured card asked the server for a column the default board no +longer declares. `todo` is the post-U11 merged planning column AND a column every pre-U11 +lineage also declares, so it is correct on both sides of the migration; `triage` was correct +on neither after #2515. + +NOT trait-resolved, deliberately: this is a SETTINGS DEFAULT with no task and no workflow in +hand — there is nothing to resolve against at module scope. The operator picks the real +column from `enumValues`; this only has to be a column that exists when they have not. + +KNOWN REMAINING GAP, recorded rather than half-fixed: `TaskColumn`/`COLUMN_SET` are still the +five legacy ids, so on a RENAMED board the enum offers columns that do not exist and +`normalizeCaptureColumn` silently substitutes the default for a column the operator did name +(voice "put it in checking" lands in planning, with no error). Closing that needs the board's +declared columns, and this plugin reaches Fusion over HTTP with no board-columns endpoint in +`FusionApiClient` — a new read, not a rename. Silent substitution is the worse half of that +bug and it is unchanged here; I am not papering over it with a guess. +*/ +const DEFAULT_QUICK_CAPTURE_COLUMN = "todo"; type TaskColumn = "triage" | "todo" | "in-progress" | "in-review" | "done"; diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index e44f031e7f..5256bba8de 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -1,22 +1,22 @@ { "generatedFrom": "node scripts/lifecycle-column-census.mjs --strict --update-baseline", "totals": { - "column": 765, + "column": 748, "role": 5, "status": 186, "deliberate": 17 }, "byColumnId": { "done": 200, - "in-progress": 146, - "in-review": 208, + "in-progress": 144, + "in-review": 205, "archived": 148, - "todo": 59, + "todo": 47, "triage": 4 }, "byFile": { "packages/engine/src/self-healing.ts": 110, - "packages/engine/src/executor.ts": 104, + "packages/engine/src/executor.ts": 87, "packages/dashboard/app/components/TaskCard.tsx": 42, "packages/core/src/task-store/moves.ts": 39, "packages/dashboard/app/components/TaskDetailModal.tsx": 30,