consolidate/u12 — U12 consolidation: 4 live defects, the AST ratchet fail-closed, and the moves.ts flag scoped (#2647)
One branch, one PR, per the consolidation directive. Contents file-by-file below. **Supersedes #2625** (its overlapping conversions landed via U11's #2624/#2626/#2636; only the parts nobody else did are folded here). **#2630 and #2639 stay open** — both green with zero threads, per rule 3. ## Four live defects, each measured **1. Every planning card renders an actions menu.** `TaskContextMenu.tsx` still had `shouldShowActionsMenu: task.column !== "triage"` on main *after* the rest of that file was converted. Since #2515 removed the id, the condition is TRUE for every card, so the suppression stopped applying anywhere — including on cards whose menu is empty, the orphaned click target the Surface Enumeration rule exists to catch. Found **twice independently**: by reading the guard, and again by the invariance test below, which failed on main with `shouldShowActionsMenu` true on one lineage and false on another. That is the argument for an invariance property over per-site conversion — the file had already been converted "2 → 1" and the survivor was the live one. **2. Worktree upcoming-work list empty on renamed boards.** `groupByWorktree` filtered `t.column === "todo"`. On the default board the id and the role coincide so every existing test passed; renamed, it matched nothing and a whole panel read as idle. **3. Hold-lane FIFO ordering lost on renamed boards.** `sortTasksForDisplayColumn` gated priority-then-FIFO on `column === "todo"`, degrading to the generic id-ordered sort elsewhere. Cards simply appear in the wrong order, silently. **4. The AST ratchet still failed open** — fourth time in that file, third found by review. `receiverName` understood only one-level property access and bare identifiers, so `task["column"]`, `metadataColumn(entry, "to")`, ternaries, `(task!.column)` and backtick literals were dropped. **Measured on main: `in-progress` 196 → 197, `in-review` 211 → 213** — three real guards nobody counted, including `metadataColumn(entry, "to") === "in-review"` in `reliability-metrics.ts`. Now walks wrappers, resolves calls to the callee name, and emits a `<SyntaxKind>` **sentinel** for anything unnameable: counted *and* trips the classification guard, so a human judges it instead of it vanishing. ## Per-file guard counts | file | before | after | |---|---:|---:| | `app/components/TaskContextMenu.tsx` | 1 | **0** | | `app/utils/worktreeGrouping.ts` | 1 | **0** | | `app/components/taskSorting.ts` | 1 | **0** | The other dashboard files I had converted reached 0 via U11's PRs; where our work overlapped I took theirs during the rebase, including two places where theirs was **stronger** than mine — they deleted Column's unreachable quick-create arm outright (with fixtures migrated) where I had converted it, and they verified the same `isPreExecutionHoldColumn` degraded-set asymmetry I did, independently. ## Flip precondition: the moves.ts flag is scoped, not flipped `move-target-declared-census.test.ts` answers precondition 2 with measurement. 41 engine `moveTask` calls have literal targets — `todo` 27, `in-progress` 7, `done` 6, `archived` 1 — and **all four are declared by the default lineage**, so the default board is not the exposure. `triage` appears only in a comment noting `replan-target.ts` used to hardcode it. My own grep had said `todo=29`; the AST says 27, because grep counts comments. The exposure is **custom** lineages: 20 of the 41 carry no `recoveryRehome` and would reject with unknown-column post-flip; 21 are exempt via the #1411 carve-out, which makes that carve-out load-bearing. I did not flip the flag. It is six seams, not the `789`/`837` pair every summary including mine described, and seam 2 turns on *new refusals* rather than swapping equivalent implementations — a green suite says nothing about that. #2639 pins the blast radius. ## Tests - `column-role-id-invariance.test.tsx` — hold traits fixed, vary only the column id across MERGED / LEGACY / RENAMED; every decision must agree. Drives the real consumers, so a component keeping an inline comparison fails it. Includes a unanimous-and-**false** case so it can't be satisfied by a predicate hardwired to true. **This is the test that caught defect 1 on main.** - `worktreeGrouping.test.ts` — includes two cards both in a column named `staging`, one hold and one not, asserting opposite answers. That assertion is impossible under a board-wide column-id set, which is why hold resolution is keyed per task via `getEffectiveTaskWorkflowId` (#2625 review). - `taskSorting.test.ts` — discriminates on the **tiebreak**, not priority: both branches sort by priority, so my first version passed for the wrong reason. Equal-priority cards whose `createdAt` order disagrees with their id order. - `no-hardcoded-lifecycle-columns.test.ts` — 16 detector cases: 11 shapes counted, 4 legitimate ignored, one asserting the sentinel path. Revert checks, all run: menu suppression → diff names the field; worktree → `expected [] to include 'FN-50'`; sort → `FN-2, FN-9` instead of `FN-9, FN-2`; ratchet → the 3 recovered guards disappear. ## One site that should never be converted `MissionControlPanel.tsx:46` — `{ id: "triage", match: (c) => c === "triage" || c === "signal" || c === "backlog" }` is a deliberate name-similarity heuristic for the SDLC funnel; it matches synonyms and folds unknown columns into an "other" bucket so custom columns still contribute. Converting it changes what the funnel displays. Like the `live-agent-count` fallbacks, it belongs in a documented floor — **the ratchet's target is that floor, not zero.** `DocumentsView.tsx:73` is convertible but the file has no column flags at all, so a real fix means plumbing board-workflow metadata into a view that doesn't fetch it — its own unit of work. ## Verification `pnpm lint` clean. `pnpm test:gate` green (10 / 482 / 71). `tsc -p packages/dashboard/tsconfig.app.json` and `packages/core/tsconfig.json` clean. Core ratchet + seam suites 24/24. Dashboard target suites 37/38 — the one failure is the pre-existing `"Back to In Progress"` label casing, confirmed identical on the base. --- ## Added after the initial push **5. `TaskCard` lost inline editing on renamed boards; `TaskDetailModal` kept it.** Still live on main: the modal resolved field editability from traits in U10/R8, the card used a hardcoded `{triage, todo}` set with **no trait path at all** — even though `taskColumnFlags` was already in scope. On a renamed board the title was editable in the modal and the pencil was missing from the card. Body moved unchanged into `isFieldEditableColumnRole` so the two surfaces cannot drift again. The veto traits are the substance: a column can legally carry `hold` **and** a WIP or review trait, and a plain `intake || hold` check would let an operator rewrite a description while a session executes against it. Coverage gap **measured, not assumed**: mutating `canEdit` back to the hardcoded set left `TaskCard*` at the same failure count as the unmutated run — nothing caught it. The four render cases assert the real `aria-label`; that mutation now fails with `Unable to find an accessible element ... name 'Edit task'`. **6. The ratchet's target is a documented FLOOR, not zero** — and this changes the completion bar. Zero is not reachable, and chasing it means breaking working code. Two categories are permanent, now protected as positive assertions so a future sweep cannot "finish the job" by deleting them: - `MissionControlPanel.tsx`'s `FUNNEL_STAGES` is a deliberate **name-similarity** heuristic — it matches `signal`, `backlog`, `to-do`, `ready`, `shipped` and folds unrecognised columns into an "other" bucket so a custom board still contributes counts. It is not asking whether a column has the intake trait; it buckets arbitrary column *names* for display. Asserted on the **synonym list**, because the synonyms are what prove it is name matching — if they disappear the site has changed character and the exemption stops applying. - `live-agent-count.ts`'s no-flags arm is reachable (a remote store is deliberately given an empty flag map; a card in an undeclared column has no flags at all) and deleting the literal makes such a card match **no** arm, so the queued total silently under-reports a stranded card. A count with an undocumented floor invites someone to drive it to zero. **Not done, and why:** `DocumentsView.tsx:73` is convertible but that file has no column flags anywhere, so a real fix means plumbing board-workflow metadata into a view that does not fetch it — its own unit of work, not something to smuggle into a conversion. **Re-verified after these commits:** `pnpm lint` clean, `pnpm test:gate` green (10 / 482 / 71), `tsc` clean on core and `tsconfig.app.json`, core ratchet suite 26/26, `columnRoles` 10/10, `TaskCard.test.tsx` 384/386 (the 2 are pre-existing CSS assertions). `TaskDetail*` is 130 failed / 551 passed **both with and without** this change — verified by stashing, so pre-existing and unrelated. --- ## Flag resolution: preconditions 1 and 2 are now DISCHARGED. Precondition 3 is blocked, and by evidence. **Precondition 1 — the side-effect equivalence proof — done.** `moves-flag-equivalence.test.ts` runs the same journey under both flag states against live PG and diffs the persisted row. **Result: identical** — whole-row equality across 128 fields plus an equal timing shape, over `todo → in-progress → in-review → todo → in-progress`. That test was **wrong twice** before it meant anything, and both times it was passing: 1. **It proved nothing.** `experimentalFeatures` is **global-only**, and `moves.ts` reads `getSettingsFast()`, which filters global-only keys out of the project layer. My `updateSettings` write was silently discarded, `useWorkflow` was false in *both* runs, and the "proof" compared the legacy path against itself. Found by stamping the flag-ON branch and observing the test still passed. Now written via `updateGlobalSettings`, and the helper **asserts the flag took effect** before the journey runs. 2. **The journey was forward-only**, so it never reached the reopen hook's field resets (`status`, `error`, `blockedBy`, pause clearing) — a mutation there passed. Extended with a backward move and a re-entry. Mutation-verified after both fixes: stamping seam 3, and diverging the reopen hook, each fail the comparison. **Precondition 2 — done, and its answer is a blocker.** The census says the default board is safe: all 41 literal engine move targets are declared by the default lineage. But **20 of those 41 carry no `recoveryRehome`**, so on a custom lineage that does not declare `todo` / `in-progress` / `done`, seam 2 would start rejecting them with unknown-column. That is a user-facing break on custom boards, not a theoretical one, and it is not fixed by the equivalence proof — seam 2 adds *new refusals* rather than swapping implementations. **So the flip is one step away, and the step is not mine to take alone:** those 20 call sites need to resolve their target from the task's workflow (or justify `recoveryRehome`), and they live across engine lanes in `moves.ts` caller territory — U2b/MAIN. Flipping before that trades a dormant flag for broken custom boards. What remains for precondition 3 once those land: flip both readers **atomically** (`moves.ts` + `workflow-task-create-ops.ts`, since the latter computes the preflight the former consumes), delete the flag-OFF branch with its guards, and drop the settings key. --- ## CORRECTION: seam 2 is not a blocker. My earlier claim was wrong. I stated in #2639 and above that "with the flag off there is **no** target-column validation on the move path", so flipping would introduce new refusals. **That is not what happens.** Reproduced against live PG: the identical custom-lineage move rejects with the flag **OFF** as well — ``` Error: Invalid transition: 'backlog' -> 'todo'. Valid targets: building ``` Transition validation is already in force on the flag-OFF path. So for the shape in question — an engine move to a column the task's own workflow does not declare — **the move already fails today**, and seam 2 introduces no new break for it. The 20 census sites lacking `recoveryRehome` are broken on a custom lineage *now*, not broken by the flip. I found this because the discriminator I added to prove "the flag is the cause" failed. Had I written the test to my assumption it would have passed and the false claim would have shipped — the same way the equivalence test passed while proving nothing until I tried to make it fail. **Revised precondition status:** | precondition | status | |---|---| | 1 — side-effect equivalence | **discharged** — identical rows, mutation-verified both directions | | 2 — seam-2 exposure census | **discharged, and it is not a blocker** — the rejection predates the flag | | 3 — flip both readers atomically, delete the flag-OFF branch, drop the settings key | **the remaining work** | So the flip is no longer gated on fixing 20 engine call sites. What it is still gated on is precondition 3 being done atomically across `moves.ts` and `workflow-task-create-ops.ts` (the latter computes the preflight the former consumes), which is `moves.ts` caller territory. Three cases now cover seam 2: the flag-ON rejection, the flag-OFF rejection (asserting the error *message*, so a change in which guard rejects stays visible rather than reading as agreement), and the #1411 `recoveryRehome` carve-out succeeding — pinning why that carve-out is load-bearing and must not be tidied away. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
7
.changeset/u12-dashboard-column-roles.md
Normal file
7
.changeset/u12-dashboard-column-roles.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Fix board affordances that broke on renamed or merged column lineages.
|
||||
category: fix
|
||||
dev: Column-role helpers move to @fusion/core (column-roles.ts); dashboard resolves intake/hold/planner roles from traits instead of the literal `triage`/`todo` ids. Fixes empty actions menus on planning cards, missing first-paint quick-create, lost hold-lane FIFO ordering, and an empty worktree upcoming-work list on renamed boards.
|
||||
@@ -49,7 +49,7 @@
|
||||
"test": "vitest run --silent=passed-only --reporter=dot",
|
||||
"test:embedded-postgres": "vitest run src/__tests__/postgres/embedded-lifecycle.test.ts --silent=passed-only --reporter=dot",
|
||||
"test:pg-gate": "vitest run --config vitest.pg.config.ts src/__tests__/postgres/handoff-to-review-atomicity.pg.test.ts src/__tests__/postgres/task-lifecycle-e2e.pg.test.ts --silent=passed-only --reporter=dot",
|
||||
"test:unit-gate": "vitest run src/__tests__/task-merge.test.ts src/__tests__/legacy-adoption.test.ts --silent=passed-only --reporter=dot"
|
||||
"test:unit-gate": "vitest run src/__tests__/task-merge.test.ts src/__tests__/legacy-adoption.test.ts src/__tests__/no-hardcoded-lifecycle-columns.test.ts --silent=passed-only --reporter=dot"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@earendil-works/pi-coding-agent": "0.82.1",
|
||||
|
||||
312
packages/core/src/__tests__/moves-flag-equivalence.test.ts
Normal file
312
packages/core/src/__tests__/moves-flag-equivalence.test.ts
Normal file
@@ -0,0 +1,312 @@
|
||||
/*
|
||||
FNXC:WorkflowColumns 2026-07-31-02:00 (U12 — precondition 1 for flipping the move-path flag):
|
||||
DO THE TWO COLUMN-SIDE-EFFECT IMPLEMENTATIONS AGREE?
|
||||
|
||||
`moves.ts` gates six behaviours on the raw compatibility flag (#2639 pins all six). Seam 3 is the one
|
||||
that is genuinely an EQUIVALENCE question: flag-OFF runs an inline legacy block, flag-ON routes the
|
||||
same column side effects through the default-workflow trait hooks — timing accumulation,
|
||||
reset-on-entry, abort-on-exit, `merge.onEnter`. Neither is the observed baseline, because they have
|
||||
never both run in production, so "the suite is green after the flip" says nothing.
|
||||
|
||||
WHY THIS IS BUILDABLE NOW, which I had assumed it was not. The flag reads
|
||||
`settings.experimentalFeatures.workflowColumns`, and `updateSettings` is public — so a test can run
|
||||
the SAME move under both flag states against a live store and diff the persisted row. No production
|
||||
change, no mock of the thing under test.
|
||||
|
||||
WHAT IT COMPARES. The full persisted task, minus fields whose difference carries no meaning
|
||||
(identity, and wall-clock stamps that advance between two runs). Comparing whole rows rather than a
|
||||
curated field list is deliberate: a curated list only proves the fields I already suspected, and the
|
||||
entire risk here is a side effect nobody enumerated. `moves.ts` mutates ~15 fields in that branch.
|
||||
|
||||
WHAT IT DOES NOT COVER, stated so this is not mistaken for a full clearance:
|
||||
- `resetPromptCheckboxes` writes to the task DIRECTORY, not the row, so a row diff cannot see it.
|
||||
- Plugin hooks (seam 5) and the transition-pending marker (seam 4) are separate seams.
|
||||
- Seam 2 turns on NEW REJECTIONS rather than swapping implementations, so it is not an equivalence
|
||||
question at all; #2647's `move-target-declared-census.test.ts` measures that exposure instead.
|
||||
This test discharges seam 3 for the row state, which is the part that was pure assertion before.
|
||||
*/
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, expect, it } from "vitest";
|
||||
import { pgDescribe, createSharedPgTaskStoreTestHarness } from "../__test-utils__/pg-test-harness.js";
|
||||
import type { TaskDetail } from "../types.js";
|
||||
import type { TaskStore } from "../store.js";
|
||||
|
||||
/**
|
||||
* Fields whose difference between two runs is meaningless: identity, and stamps that advance with
|
||||
* wall clock. Everything else must match, including the timing ACCUMULATORS (`cumulativeActiveMs`),
|
||||
* which are the interesting part — they are computed from deltas, so a divergence in how the two
|
||||
* implementations anchor a segment shows up there rather than in a raw timestamp.
|
||||
*/
|
||||
const VOLATILE_FIELDS = new Set([
|
||||
"id",
|
||||
// Per-task UUID; carries no behavioural meaning.
|
||||
"lineageId",
|
||||
"createdAt",
|
||||
"updatedAt",
|
||||
"columnMovedAt",
|
||||
"executionStartedAt",
|
||||
"executionCompletedAt",
|
||||
"firstExecutionAt",
|
||||
"cumulativeActiveMs",
|
||||
"log",
|
||||
]);
|
||||
|
||||
/*
|
||||
Wall-clock noise is normalised RECURSIVELY rather than by a flat key list, because it is nested:
|
||||
`columnDwellMs` is a column -> milliseconds map and run-audit-ish entries carry their own `observedAt`.
|
||||
A flat list missed both, and the first run of this test reported them as divergences.
|
||||
|
||||
Durations become BOOLEANS (`>0`) rather than being dropped: whether time was attributed to a column at
|
||||
all is exactly the behaviour under test, while the millisecond value differs between any two runs.
|
||||
Dropping them would have hidden a real divergence; comparing them would have been permanently flaky.
|
||||
*/
|
||||
function normalize(value: unknown, key?: string, taskId?: string): unknown {
|
||||
/*
|
||||
IDENTITY is substituted inside strings rather than the field being dropped. `prompt` embeds the task
|
||||
id (`# KB-001` vs `# KB-002`), so a raw comparison always fails and dropping it would stop comparing
|
||||
the spec content entirely — which is one of the things the reset-on-entry side effect can touch.
|
||||
Replacing the id keeps the content under test.
|
||||
*/
|
||||
if (typeof value === "string" && taskId) return value.split(taskId).join("<TASK_ID>");
|
||||
if (typeof value === "number" && key !== undefined && /Ms$/.test(key)) return value > 0;
|
||||
if (Array.isArray(value)) return value.map((entry) => normalize(entry, undefined, taskId));
|
||||
if (value && typeof value === "object") {
|
||||
const out: Record<string, unknown> = {};
|
||||
for (const [k, v] of Object.entries(value as Record<string, unknown>)) {
|
||||
if (VOLATILE_FIELDS.has(k) || /At$/.test(k)) continue;
|
||||
// A duration MAP: keep the keys, reduce each value to "time was attributed here".
|
||||
out[k] = /Ms$/.test(k) && v && typeof v === "object" && !Array.isArray(v)
|
||||
? Object.fromEntries(Object.entries(v as Record<string, unknown>).map(([ck, cv]) => [ck, typeof cv === "number" ? cv > 0 : cv]))
|
||||
: normalize(v, k, taskId);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
function comparableSnapshot(task: TaskDetail): Record<string, unknown> {
|
||||
return normalize(task, undefined, task.id) as Record<string, unknown>;
|
||||
}
|
||||
|
||||
/** Which timing fields were SET (not their values), so anchoring behaviour is still compared. */
|
||||
function timingShape(task: TaskDetail): Record<string, boolean> {
|
||||
const t = task as unknown as Record<string, unknown>;
|
||||
return {
|
||||
hasExecutionStartedAt: t.executionStartedAt != null,
|
||||
hasExecutionCompletedAt: t.executionCompletedAt != null,
|
||||
hasFirstExecutionAt: t.firstExecutionAt != null,
|
||||
accumulatedActiveTime: typeof t.cumulativeActiveMs === "number" && (t.cumulativeActiveMs as number) > 0,
|
||||
};
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:WorkflowColumns 2026-07-31-02:30 (U12 — the trap this test fell into first):
|
||||
THE FLAG IS GLOBAL-ONLY, so it must be written through `updateGlobalSettings`.
|
||||
|
||||
My first version used `updateSettings`, and the test PASSED — while proving nothing. `moves.ts` reads
|
||||
`getSettingsFast()`, which filters `isGlobalOnlySettingsKey` out of the project layer, and
|
||||
`experimentalFeatures` is exactly such a key (`isGlobalSettingsKey("experimentalFeatures") === true`).
|
||||
So the project-scoped write was discarded, `useWorkflow` was false in BOTH runs, and the "equivalence
|
||||
proof" was comparing the legacy path against itself.
|
||||
|
||||
Caught by stamping the flag-ON branch of `moves.ts` and observing that the test still passed — i.e. by
|
||||
checking that the mutation could be detected, not by trusting the green. Exactly the failure this
|
||||
program keeps finding, produced by me this time.
|
||||
*/
|
||||
async function setFlag(store: TaskStore, enabled: boolean): Promise<void> {
|
||||
const current = await store.globalSettingsStore.getSettings();
|
||||
await store.updateGlobalSettings({
|
||||
...current,
|
||||
experimentalFeatures: { ...(current.experimentalFeatures ?? {}), workflowColumns: enabled },
|
||||
} as never);
|
||||
|
||||
// Prove the write took effect before relying on it — the whole point of this note.
|
||||
const effective = await store.getSettingsFast();
|
||||
if ((effective.experimentalFeatures?.workflowColumns === true) !== enabled) {
|
||||
throw new Error(`flag write did not take effect: wanted ${enabled}, moves.ts would read ${effective.experimentalFeatures?.workflowColumns === true}`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Drive one task through a column journey under a fixed flag state and return what persisted.
|
||||
* The journey covers the transitions the legacy block special-cases: entering execution, leaving it
|
||||
* (segment accumulation + abort-on-exit), and reaching review.
|
||||
*/
|
||||
async function runJourney(store: TaskStore, flagEnabled: boolean): Promise<{ snapshot: Record<string, unknown>; timing: Record<string, boolean> }> {
|
||||
await setFlag(store, flagEnabled);
|
||||
/*
|
||||
IDENTICAL description in both runs. My first version interpolated the flag state and the whole-row
|
||||
diff dutifully reported it — a self-inflicted failure that would have read as a real divergence.
|
||||
*/
|
||||
const created = await store.createTask({ description: "equivalence journey" });
|
||||
|
||||
/*
|
||||
THE JOURNEY MUST GO BACKWARD TOO. My first version was forward-only (todo -> in-progress ->
|
||||
in-review) and a mutation to the REOPEN hook did not fail it: those field resets (`status`, `error`,
|
||||
`blockedBy`, pause clearing) only run when a card moves back out of a later column, so a
|
||||
forward-only journey never reached them. The test passed while covering roughly half the branch.
|
||||
|
||||
Now: enter execution, reach review, reopen to the hold column (reset-on-entry + abort-on-exit), and
|
||||
re-enter execution so the second segment's timing accumulation is exercised on top of the first.
|
||||
*/
|
||||
await store.moveTask(created.id, "in-progress");
|
||||
await store.moveTask(created.id, "in-review");
|
||||
await store.moveTask(created.id, "todo");
|
||||
await store.moveTask(created.id, "in-progress");
|
||||
|
||||
const final = await store.getTask(created.id);
|
||||
if (!final) throw new Error("task vanished mid-journey");
|
||||
return { snapshot: comparableSnapshot(final), timing: timingShape(final) };
|
||||
}
|
||||
|
||||
pgDescribe("move-path side effects are equivalent with the compatibility flag OFF and ON (U12 seam 3)", () => {
|
||||
const harness = createSharedPgTaskStoreTestHarness({ prefix: "fusion_moves_flag_equiv" });
|
||||
beforeAll(harness.beforeAll);
|
||||
beforeEach(harness.beforeEach);
|
||||
afterEach(harness.afterEach);
|
||||
afterAll(harness.afterAll);
|
||||
|
||||
/*
|
||||
One test rather than two, because the assertion IS the comparison: neither run has meaning alone.
|
||||
Ordered flag-OFF first so the legacy path — the one actually running in production today — is the
|
||||
expected value, and any divergence reads as "the trait hooks differ from shipped behaviour".
|
||||
*/
|
||||
it("the persisted row is identical either way, and so is the timing shape", async () => {
|
||||
const store = harness.store();
|
||||
|
||||
const legacy = await runJourney(store, false);
|
||||
const traitHooks = await runJourney(store, true);
|
||||
|
||||
/*
|
||||
Whole-row equality. If this fails, the flip changes persisted state on every task move and the
|
||||
diff names the field — which is the evidence precondition 1 asks for, in either direction.
|
||||
*/
|
||||
expect(traitHooks.snapshot).toEqual(legacy.snapshot);
|
||||
|
||||
/*
|
||||
Timing is compared as a SHAPE, not by value: the two runs happen at different wall-clock instants,
|
||||
so equal millisecond counts would be coincidence and a mismatch would be noise. What must agree is
|
||||
which anchors got set and whether active time accumulated at all — a divergence there means the
|
||||
two implementations disagree about when execution starts or ends, which would silently corrupt
|
||||
every task's duration.
|
||||
*/
|
||||
expect(traitHooks.timing).toEqual(legacy.timing);
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowColumns 2026-07-31-03:00 (U12 — seam 2, demonstrated rather than inferred):
|
||||
WHAT THE FLIP WOULD BREAK ON A CUSTOM BOARD.
|
||||
|
||||
Precondition 2's census established that 20 of 41 literal engine `moveTask` targets carry no
|
||||
`recoveryRehome`, and inferred that those would start rejecting on a lineage that does not declare
|
||||
the legacy ids. Inference is not enough to hand someone as a work order, so this reproduces it.
|
||||
|
||||
Seam 2 is NOT an equivalence question: with the flag off there is no target validation at all, so
|
||||
flipping introduces refusals for moves that succeed today. This case is the refusal, and the second
|
||||
case is the #1411 carve-out that the other 21 sites rely on — which is what makes that carve-out
|
||||
load-bearing for the flip rather than incidental.
|
||||
|
||||
If the first expectation ever starts passing, seam 2 stopped rejecting undeclared targets and the
|
||||
20 call sites are no longer a blocker — delete this and flip.
|
||||
*/
|
||||
it("REJECTS an engine move to a column the task's own workflow does not declare", async () => {
|
||||
const store = harness.store();
|
||||
await setFlag(store, true);
|
||||
|
||||
// A lineage with none of the legacy ids: no todo, no in-progress, no done.
|
||||
const definition = await store.createWorkflowDefinition({
|
||||
name: "no-legacy-ids",
|
||||
ir: {
|
||||
version: "v2",
|
||||
name: "no-legacy-ids",
|
||||
columns: [
|
||||
{ id: "backlog", name: "Backlog", traits: [{ trait: "intake" }, { trait: "hold" }] },
|
||||
{ id: "building", name: "Building", traits: [{ trait: "wip" }] },
|
||||
{ id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] },
|
||||
],
|
||||
nodes: [{ id: "start", kind: "start", column: "backlog" }, { id: "end", kind: "end", column: "shipped" }],
|
||||
edges: [{ from: "start", to: "end" }],
|
||||
},
|
||||
} as never);
|
||||
|
||||
const task = await store.createTask({ description: "custom lineage card", workflowId: definition.id } as never);
|
||||
|
||||
/*
|
||||
A plain engine-shaped move to `todo` — the shape 20 census sites use. It must reject, because
|
||||
`todo` is not a column this workflow declares. This is the break the flip would ship.
|
||||
*/
|
||||
await expect(store.moveTask(task.id, "todo")).rejects.toThrow();
|
||||
});
|
||||
|
||||
it("REJECTS that same move with the flag OFF TOO — correcting the blast-radius claim", () => {
|
||||
/*
|
||||
FNXC:WorkflowColumns 2026-07-31-03:15 (U12 — I had this wrong, and it matters):
|
||||
I wrote in #2639 and in the census that "with the flag off there is NO target-column validation on
|
||||
the move path", so flipping would introduce new refusals. THAT IS NOT WHAT HAPPENS. Running the
|
||||
identical custom-lineage move with the flag OFF also rejects:
|
||||
|
||||
Error: Invalid transition: 'backlog' -> 'todo'. Valid targets: building
|
||||
|
||||
Transition validation is already in force on the flag-OFF path. So for this shape — an engine move
|
||||
to a column the task's workflow does not declare — the move is ALREADY failing today, and seam 2
|
||||
does not introduce a new break for it. The 20 census sites without `recoveryRehome` are therefore a
|
||||
smaller risk than I reported: on a custom lineage they are broken now, not broken by the flip.
|
||||
|
||||
I am asserting the CURRENT behaviour rather than the behaviour I expected, because a test written to
|
||||
my assumption would have failed and I would have "fixed" the fixture until it agreed with a claim
|
||||
that was false. The error message is asserted so a future change in WHICH guard rejects is visible
|
||||
rather than silently reinterpreted as agreement.
|
||||
*/
|
||||
return (async () => {
|
||||
const store = harness.store();
|
||||
await setFlag(store, false);
|
||||
|
||||
const definition = await store.createWorkflowDefinition({
|
||||
name: "no-legacy-ids-flagoff",
|
||||
ir: {
|
||||
version: "v2",
|
||||
name: "no-legacy-ids-flagoff",
|
||||
columns: [
|
||||
{ id: "backlog", name: "Backlog", traits: [{ trait: "intake" }, { trait: "hold" }] },
|
||||
{ id: "building", name: "Building", traits: [{ trait: "wip" }] },
|
||||
{ id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] },
|
||||
],
|
||||
nodes: [{ id: "start", kind: "start", column: "backlog" }, { id: "end", kind: "end", column: "shipped" }],
|
||||
edges: [{ from: "start", to: "end" }],
|
||||
},
|
||||
} as never);
|
||||
|
||||
const task = await store.createTask({ description: "custom lineage card, flag off", workflowId: definition.id } as never);
|
||||
await expect(store.moveTask(task.id, "todo")).rejects.toThrow(/Invalid transition/);
|
||||
})();
|
||||
});
|
||||
|
||||
it("ACCEPTS the same move when it carries the #1411 recoveryRehome carve-out", async () => {
|
||||
const store = harness.store();
|
||||
await setFlag(store, true);
|
||||
|
||||
const definition = await store.createWorkflowDefinition({
|
||||
name: "no-legacy-ids-rescue",
|
||||
ir: {
|
||||
version: "v2",
|
||||
name: "no-legacy-ids-rescue",
|
||||
columns: [
|
||||
{ id: "backlog", name: "Backlog", traits: [{ trait: "intake" }, { trait: "hold" }] },
|
||||
{ id: "building", name: "Building", traits: [{ trait: "wip" }] },
|
||||
{ id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] },
|
||||
],
|
||||
nodes: [{ id: "start", kind: "start", column: "backlog" }, { id: "end", kind: "end", column: "shipped" }],
|
||||
edges: [{ from: "start", to: "end" }],
|
||||
},
|
||||
} as never);
|
||||
|
||||
const task = await store.createTask({ description: "rescued card", workflowId: definition.id } as never);
|
||||
|
||||
/*
|
||||
The carve-out exists so a custom-workflow card can still be rescued to a guaranteed-safe landing
|
||||
column. The 21 census sites that pass it are already flip-safe; this pins that, so nobody "cleans
|
||||
up" the carve-out and breaks recovery on custom boards.
|
||||
*/
|
||||
const moved = await store.moveTask(task.id, "todo", { recoveryRehome: true, bypassGuards: true } as never);
|
||||
expect(moved.column).toBe("todo");
|
||||
});
|
||||
});
|
||||
@@ -92,15 +92,55 @@ function sourceFiles(): string[] {
|
||||
return out;
|
||||
}
|
||||
|
||||
/** The receiver's name: `task.column` -> "column"; a bare `toColumn` -> "toColumn". */
|
||||
/*
|
||||
FNXC:LifecycleColumnRatchet 2026-07-30-23:00 (U12 — the fourth fail-open in this file):
|
||||
AN UNNAMEABLE RECEIVER IS COUNTED, NOT DROPPED.
|
||||
|
||||
The AST rewrite fixed quoting and line-orientation, but `receiverName` still understood only a
|
||||
ONE-LEVEL property access or a bare identifier, and the caller `continue`d on `undefined`. So these
|
||||
were silently uncounted:
|
||||
|
||||
task["column"] === "triage" // ElementAccess
|
||||
metadataColumn(entry, "to") === "in-review" // CallExpression
|
||||
(flag ? from : to) === "triage" // ConditionalExpression
|
||||
(task!.column) === "triage" // Parenthesized / NonNull wrapper
|
||||
task.column === `triage` // NoSubstitutionTemplateLiteral
|
||||
|
||||
Live, not theoretical: `metadataColumn(entry, "to") === "in-review"` in
|
||||
`dashboard/src/reliability-metrics.ts` is THREE real lifecycle guards this ratchet was not counting.
|
||||
Found by printing what the scanner reported as unnamed instead of trusting its total.
|
||||
|
||||
That is the fourth time this file has failed OPEN — grep quoting, then the `/\w*[Cc]olumn/` name
|
||||
pattern, then one-level property access. Same mechanism every time: a shape the scanner did not
|
||||
understand became "not a column" and left the count. So the fix is the PROPERTY, not the case: walk
|
||||
through wrappers, resolve a call to its callee name, and emit a `<SyntaxKind>` SENTINEL for anything
|
||||
still unnameable. A sentinel is counted AND trips the classification test, so a human judges it
|
||||
instead of it vanishing. Fail closed — which is what makes this number safe to gate on at all.
|
||||
*/
|
||||
function receiverName(node: ts.Expression): string | undefined {
|
||||
if (ts.isPropertyAccessExpression(node)) return node.name.text;
|
||||
if (ts.isIdentifier(node)) return node.text;
|
||||
return undefined;
|
||||
let cur: ts.Node = node;
|
||||
for (;;) {
|
||||
if (ts.isPropertyAccessExpression(cur)) { cur = cur.name; continue; }
|
||||
if (ts.isElementAccessExpression(cur)) {
|
||||
const arg = cur.argumentExpression;
|
||||
if (arg && (ts.isStringLiteral(arg) || ts.isNoSubstitutionTemplateLiteral(arg))) return arg.text;
|
||||
cur = cur.expression;
|
||||
continue;
|
||||
}
|
||||
// A helper that RETURNS a column is classified by the function's name.
|
||||
if (ts.isCallExpression(cur)) { cur = cur.expression; continue; }
|
||||
if (ts.isParenthesizedExpression(cur) || ts.isNonNullExpression(cur) || ts.isAsExpression(cur)) {
|
||||
cur = cur.expression;
|
||||
continue;
|
||||
}
|
||||
break;
|
||||
}
|
||||
if (ts.isIdentifier(cur)) return cur.text;
|
||||
return `<${ts.SyntaxKind[cur.kind]}>`;
|
||||
}
|
||||
|
||||
/** Walk one parsed file for `X === "<id>"` / `X !== "<id>"` where X names something column-like. */
|
||||
function collect(sf: ts.SourceFile, columnId: string, file: string, sites: Site[]): void {
|
||||
export function collect(sf: ts.SourceFile, columnId: string, file: string, sites: Site[]): void {
|
||||
const visit = (node: ts.Node): void => {
|
||||
if (
|
||||
ts.isBinaryExpression(node)
|
||||
@@ -110,8 +150,16 @@ function collect(sf: ts.SourceFile, columnId: string, file: string, sites: Site[
|
||||
const pairs = [[node.right, node.left], [node.left, node.right]] as const;
|
||||
for (const [lit, other] of pairs) {
|
||||
// `.text` is the DECODED value, so single and double quotes are indistinguishable here.
|
||||
if (!ts.isStringLiteral(lit) || lit.text !== columnId) continue;
|
||||
// Backtick literals are string literals to a reader and to the runtime; excluding them left
|
||||
// `task.column === \`triage\`` uncounted for no reason anyone would defend.
|
||||
const isLiteral = ts.isStringLiteral(lit) || ts.isNoSubstitutionTemplateLiteral(lit);
|
||||
if (!isLiteral || lit.text !== columnId) continue;
|
||||
const receiver = receiverName(other);
|
||||
/*
|
||||
`receiverName` no longer returns undefined for an expression it cannot name — it returns a
|
||||
`<SyntaxKind>` sentinel that is COUNTED and must be classified. Dropping on undefined is what
|
||||
made this fail open; the undefined arm is kept only as a defensive no-op.
|
||||
*/
|
||||
if (receiver === undefined || NON_COLUMN_RECEIVERS.has(receiver)) continue;
|
||||
sites.push({
|
||||
file,
|
||||
@@ -148,11 +196,30 @@ function comparisonSites(columnId: string): Site[] {
|
||||
* A raise means a guard came back — convert it, or if the receiver genuinely is not a column, add it
|
||||
* to NON_COLUMN_RECEIVERS with a reason.
|
||||
*/
|
||||
/*
|
||||
FNXC:LifecycleColumnRatchet 2026-07-31-01:30 (PR #2647 review — greptile, and worse than reported):
|
||||
TIGHTENED TO THE MEASURED COUNTS, because slack made this a ratchet in name only.
|
||||
|
||||
The review's finding was that this test is not in the blocking gate, so a regression can merge. True,
|
||||
and fixed (`packages/core/package.json` -> `test:unit-gate`). But admitting it to the gate proved the
|
||||
bigger problem: I reintroduced `task.column === 'triage'` and the gate STAYED GREEN at 158 passed.
|
||||
|
||||
The ceilings carried 26 free `triage` slots (11 measured vs 37), plus 18 for `todo`, 4 for
|
||||
`in-progress` and 4 for `in-review`. A ceiling with slack does not ratchet — it permits exactly as
|
||||
many regressions as the gap, silently, which is the same "guard that cannot fire" this whole program
|
||||
keeps finding. Being in the gate was necessary and not sufficient.
|
||||
|
||||
Every number here is now the measured count, so ANY reintroduction fails. Proven, not assumed: with
|
||||
these values the same single-guard probe takes the gate red.
|
||||
|
||||
LOWER these as conversions land; a raise means a literal came back OR detection improved — and if it
|
||||
is the latter, say which sites in the commit, as previous revisions of this file did.
|
||||
*/
|
||||
const CEILINGS: Record<string, number> = {
|
||||
triage: 37,
|
||||
todo: 82,
|
||||
"in-progress": 201,
|
||||
"in-review": 217,
|
||||
triage: 11,
|
||||
todo: 64,
|
||||
"in-progress": 197,
|
||||
"in-review": 213,
|
||||
};
|
||||
|
||||
describe("lifecycle-column literal ratchet (AST)", () => {
|
||||
@@ -241,3 +308,125 @@ describe("lifecycle-column literal ratchet (AST)", () => {
|
||||
expect(KNOWN_COLUMN_RECEIVERS.size).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:LifecycleColumnRatchet 2026-07-30-23:00 (the fourth fail-open, pinned so it is the last):
|
||||
PROOF THAT EVERY REINTRODUCTION SHAPE IS COUNTED.
|
||||
|
||||
Each shape below was silently dropped by at least one earlier revision of this file: quoting by the
|
||||
grep, short bindings by the name pattern, and wrappers/calls/brackets by the one-level
|
||||
`receiverName`. Three of the four were found by review rather than by the ratchet itself, which is
|
||||
the argument for testing the detector directly instead of trusting its total.
|
||||
|
||||
MEASURED IMPACT of the wrapper/call fix, on current main: `in-progress` 196 -> 197 and `in-review`
|
||||
211 -> 213. Those three were real guards nobody was counting, including
|
||||
`metadataColumn(entry, "to") === "in-review"` in dashboard/src/reliability-metrics.ts.
|
||||
*/
|
||||
describe("the detector counts every reintroduction shape (fail closed)", () => {
|
||||
function sitesFor(source: string, columnId: string) {
|
||||
const sf = ts.createSourceFile("probe.ts", source, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS);
|
||||
const sites: Site[] = [];
|
||||
collect(sf, columnId, "probe.ts", sites);
|
||||
return sites;
|
||||
}
|
||||
|
||||
const RED: Array<[string, string]> = [
|
||||
["double-quoted", `if (task.column === "triage") return;`],
|
||||
["single-quoted", `if (task.column === 'triage') return;`],
|
||||
["backtick literal", "if (task.column === `triage`) return;"],
|
||||
["multiline", `if (task.column ===\n "triage") return;`],
|
||||
["negated", `if (task.column !== "triage") return;`],
|
||||
["reversed operands", `if ("triage" === task.column) return;`],
|
||||
["bracket access", `if (task["column"] === "triage") return;`],
|
||||
["deep qualified", `if (state.board.tasks[i].column === "triage") return;`],
|
||||
["parenthesised non-null", `if ((task!.column) === "triage") return;`],
|
||||
["helper call returning a column", `if (metadataColumn(entry, "to") === "triage") return;`],
|
||||
["bare binding", `if (toColumn === "triage") return;`],
|
||||
];
|
||||
|
||||
for (const [label, source] of RED) {
|
||||
it(`counts ${label}`, () => {
|
||||
expect(sitesFor(source, "triage")).toHaveLength(1);
|
||||
});
|
||||
}
|
||||
|
||||
it("counts an unnameable receiver under a sentinel rather than dropping it", () => {
|
||||
/*
|
||||
THE FAIL-CLOSED PROPERTY ITSELF. Every earlier revision answered "not a column" for a shape it
|
||||
did not understand, so the site left the count in silence. A ternary receiver must be COUNTED
|
||||
with a `<...>` sentinel, which then trips the classification guard and forces a human judgement.
|
||||
|
||||
If this returns 0, the ratchet has gone back to failing open and its number stops being
|
||||
trustworthy — which is the only reason it is allowed to gate.
|
||||
*/
|
||||
const sites = sitesFor(`if ((flag ? from : to) === "triage") return;`, "triage");
|
||||
expect(sites).toHaveLength(1);
|
||||
expect(sites[0]!.receiver).toMatch(/^<.*>$/);
|
||||
});
|
||||
|
||||
const GREEN: Array<[string, string, string]> = [
|
||||
["agent role", `if (role === "triage") return;`, "triage"],
|
||||
["agent display name", `if (entry.agent === "triage") return;`, "triage"],
|
||||
["a line comment", `// if (task.column === "triage") return;`, "triage"],
|
||||
["a block comment", `/* task.column === "triage" */`, "triage"],
|
||||
];
|
||||
|
||||
for (const [label, source, id] of GREEN) {
|
||||
it(`ignores ${label}`, () => {
|
||||
/*
|
||||
The other half: a detector that counted everything would "catch" reintroductions and be
|
||||
useless, because converting `role === "triage"` breaks prompt-template resolution and the count
|
||||
could never reach its floor. Comments are excluded structurally — the AST has none.
|
||||
*/
|
||||
expect(sitesFor(source, id)).toHaveLength(0);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:LifecycleColumnRatchet 2026-07-31-00:45 (U12 — the bar is a FLOOR, not zero):
|
||||
SITES THAT MUST NEVER BE CONVERTED, asserted as a positive.
|
||||
|
||||
The program has been tracking these counts toward zero. Zero is not reachable, and pursuing it means
|
||||
breaking working code — the same trap as demanding conversion of `role === "triage"`, one level up.
|
||||
Two categories are permanent, each for a stated reason, and both are protected here so a future sweep
|
||||
cannot "finish the job" by removing them:
|
||||
|
||||
1. `dashboard/app/components/command-center/MissionControlPanel.tsx` — `FUNNEL_STAGES` is a deliberate
|
||||
NAME-SIMILARITY heuristic for the SDLC funnel. It matches synonyms (`signal`, `backlog`, `to-do`,
|
||||
`ready`, `shipped`) and folds anything unrecognised into an "other" bucket precisely so a custom
|
||||
board still contributes counts. It is not asking "does this column have the intake trait" — it is
|
||||
bucketing arbitrary column NAMES for display. Resolving it to traits would change what the funnel
|
||||
shows and would drop the synonym coverage that makes it work on boards Fusion has never seen.
|
||||
|
||||
2. `core/live-agent-count.ts` — the no-flags arm. Documented in that file and in earlier revisions of
|
||||
this one: it is REACHABLE (a remote store is deliberately given an empty flag map, and a card in a
|
||||
column its workflow no longer declares has no flags at all), and deleting the literal would make
|
||||
such a card match NO arm, so the footer's queued total would silently under-report a stranded card.
|
||||
|
||||
This test exists because a count with an undocumented floor invites someone to drive it to zero. The
|
||||
honest target is: these sites, and nothing else.
|
||||
*/
|
||||
describe("the reachable floor: sites that must stay", () => {
|
||||
it("keeps the Mission Control funnel's name-similarity heuristic", () => {
|
||||
const funnel = readFileSync(
|
||||
join(REPO_ROOT, "packages/dashboard/app/components/command-center/MissionControlPanel.tsx"),
|
||||
"utf-8",
|
||||
);
|
||||
/*
|
||||
Asserted on the SYNONYM list rather than on the `triage` comparison alone: the synonyms are the
|
||||
evidence that this is name matching and not a lifecycle guard, so if they disappear the site has
|
||||
changed character and the exemption below no longer applies.
|
||||
*/
|
||||
expect(funnel).toContain("FUNNEL_STAGES");
|
||||
expect(funnel).toContain('"signal"');
|
||||
expect(funnel).toContain('"backlog"');
|
||||
});
|
||||
|
||||
it("keeps live-agent-count's no-flags arm", () => {
|
||||
const liveAgentCount = readFileSync(join(REPO_ROOT, "packages/core/src/live-agent-count.ts"), "utf-8");
|
||||
// Reachable via an empty remote flag map and via a card in an undeclared column; removing it
|
||||
// makes such a card match no arm at all and the queued total under-reports it.
|
||||
expect(liveAgentCount).toContain("columnIsIntakeOrHold");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -20,7 +20,7 @@ LEGACY_….has(columnId)`) fails the "traits win" cases; making it ignore the id
|
||||
(`return Boolean(flags?.intake)`) fails the degraded cases.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { isIntakeColumnRole, isPreImplementationColumnRole } from "../utils/columnRoles";
|
||||
import { isFieldEditableColumnRole, isIntakeColumnRole, isPreImplementationColumnRole } from "../utils/columnRoles";
|
||||
|
||||
describe("isIntakeColumnRole", () => {
|
||||
it("uses the intake TRAIT when the column resolved", () => {
|
||||
@@ -71,3 +71,40 @@ describe("isPreImplementationColumnRole", () => {
|
||||
expect(isPreImplementationColumnRole(undefined, "in-review")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-00:15 (U12 — one affordance, two components):
|
||||
FIELD EDITABILITY. TaskDetailModal resolved this from traits in U10/R8; TaskCard kept a hardcoded
|
||||
`{triage, todo}` set with no trait path, so a renamed board lost the pencil on the card while the
|
||||
modal kept it — the same one-surface-missed shape as the FN-6115 chevron chain.
|
||||
|
||||
The VETO cases are the ones worth pinning: a column can legally carry `hold` AND a WIP or review
|
||||
trait, and a plain `intake || hold` check would let an operator rewrite a description while a session
|
||||
executes against it.
|
||||
*/
|
||||
describe("isFieldEditableColumnRole", () => {
|
||||
it("allows editing in a renamed pre-implementation column", () => {
|
||||
expect(isFieldEditableColumnRole({ intake: true }, "backlog")).toBe(true);
|
||||
expect(isFieldEditableColumnRole({ hold: true }, "parked")).toBe(true);
|
||||
});
|
||||
|
||||
it("VETOES editing when a terminal, executing or review trait is also present", () => {
|
||||
expect(isFieldEditableColumnRole({ hold: true, countsTowardWip: true }, "parked")).toBe(false);
|
||||
expect(isFieldEditableColumnRole({ intake: true, mergeBlocker: true }, "backlog")).toBe(false);
|
||||
expect(isFieldEditableColumnRole({ intake: true, humanReview: true }, "backlog")).toBe(false);
|
||||
expect(isFieldEditableColumnRole({ intake: true, complete: true }, "backlog")).toBe(false);
|
||||
expect(isFieldEditableColumnRole({ intake: true, archived: true }, "backlog")).toBe(false);
|
||||
});
|
||||
|
||||
it("refuses a resolved column with no pre-implementation trait", () => {
|
||||
expect(isFieldEditableColumnRole({ countsTowardWip: true }, "building")).toBe(false);
|
||||
});
|
||||
|
||||
it("falls back to the legacy id pair when traits are absent", () => {
|
||||
// First paint, and what every caller got before the conversion.
|
||||
expect(isFieldEditableColumnRole(undefined, "todo")).toBe(true);
|
||||
expect(isFieldEditableColumnRole(undefined, "triage")).toBe(true);
|
||||
expect(isFieldEditableColumnRole(undefined, "backlog")).toBe(false);
|
||||
expect(isFieldEditableColumnRole(undefined, "in-progress")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -549,6 +549,37 @@ export function Board({ tasks, projectId, maxConcurrent, showWorktreeGrouping, o
|
||||
return map;
|
||||
}, [boardWorkflows]);
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8 drift conversion):
|
||||
The TASK IDS whose own column is a hold lane IN THEIR OWN WORKFLOW.
|
||||
|
||||
Resolved per task rather than as a board-wide set of hold column IDS (PR #2625 review —
|
||||
greptile). Column ids are namespaced per workflow, so two workflows can both declare
|
||||
`staging` with one marking it `hold` and the other `countsTowardWip`. A unioned id set
|
||||
cannot tell those apart, and every executing card in the second workflow would have shown
|
||||
up under Up Next as waiting work — a wrong answer presented confidently, which is worse
|
||||
than the renamed-board emptiness this change set out to fix.
|
||||
|
||||
Resolving through `getEffectiveTaskWorkflowId` removes the ambiguity instead of narrowing
|
||||
it: the question "is this card waiting?" is answered by the card's own workflow, which is
|
||||
the only workflow that can answer it.
|
||||
*/
|
||||
const holdTaskIds = useMemo(() => {
|
||||
const holdColumnsByWorkflowId = new Map<string, Set<string>>();
|
||||
for (const workflow of boardWorkflows?.workflows ?? []) {
|
||||
holdColumnsByWorkflowId.set(
|
||||
workflow.id,
|
||||
new Set(workflow.columns.filter((col) => col.flags.hold === true).map((col) => col.id)),
|
||||
);
|
||||
}
|
||||
const ids = new Set<string>();
|
||||
for (const task of tasks) {
|
||||
const workflowId = getEffectiveTaskWorkflowId(task);
|
||||
if (workflowId && holdColumnsByWorkflowId.get(workflowId)?.has(task.column)) ids.add(task.id);
|
||||
}
|
||||
return ids;
|
||||
}, [boardWorkflows, tasks, getEffectiveTaskWorkflowId]);
|
||||
|
||||
const selectedWorkflowContextMenuColumns = useMemo(() => (
|
||||
selectedWorkflow ? workflowContextMenuColumnsByWorkflowId.get(selectedWorkflow.id) : undefined
|
||||
), [selectedWorkflow, workflowContextMenuColumnsByWorkflowId]);
|
||||
@@ -586,7 +617,7 @@ export function Board({ tasks, projectId, maxConcurrent, showWorktreeGrouping, o
|
||||
const isWorkflowDoneLikeColumn = column.flags.complete === true && column.flags.archived !== true;
|
||||
grouped[column.id] = isWorkflowDoneLikeColumn
|
||||
? sortTasksForDisplayColumn(grouped[column.id] ?? [], "done", doneSortMode)
|
||||
: sortTasksForDisplayColumn(grouped[column.id] ?? [], column.id as ColumnType, doneSortMode, column.flags.archived === true);
|
||||
: sortTasksForDisplayColumn(grouped[column.id] ?? [], column.id as ColumnType, doneSortMode, column.flags.archived === true, column.flags.hold === true);
|
||||
}
|
||||
return grouped;
|
||||
}, [doneSortMode, selectedWorkflow, selectedWorkflowCreateColumnId, selectedWorkflowTasks]);
|
||||
@@ -770,7 +801,7 @@ export function Board({ tasks, projectId, maxConcurrent, showWorktreeGrouping, o
|
||||
const isDoneLikeColumn = column.flags.complete === true && column.flags.archived !== true;
|
||||
grouped[column.id] = isDoneLikeColumn
|
||||
? sortTasksForDisplayColumn(grouped[column.id] ?? [], "done", doneSortMode)
|
||||
: sortTasksForDisplayColumn(grouped[column.id] ?? [], column.id as ColumnType, doneSortMode, column.flags.archived === true);
|
||||
: sortTasksForDisplayColumn(grouped[column.id] ?? [], column.id as ColumnType, doneSortMode, column.flags.archived === true, column.flags.hold === true);
|
||||
}
|
||||
return grouped;
|
||||
}, [aggregateBoardColumns, aggregateQuickCreateTarget, boardWorkflows, doneSortMode, getEffectiveTaskWorkflowId, tasks, workflowColumnsByWorkflowId]);
|
||||
@@ -890,6 +921,7 @@ export function Board({ tasks, projectId, maxConcurrent, showWorktreeGrouping, o
|
||||
columnDisplayName={columnDef.name}
|
||||
columnDescription={columnDef.description}
|
||||
columnFlags={columnDef.flags}
|
||||
holdTaskIds={holdTaskIds}
|
||||
taskContextMenuColumnsByTaskId={taskContextMenuColumnsByTaskId}
|
||||
tasks={aggregateTasksByColumn[columnDef.id] ?? []}
|
||||
projectId={projectId}
|
||||
@@ -972,6 +1004,7 @@ export function Board({ tasks, projectId, maxConcurrent, showWorktreeGrouping, o
|
||||
columnDisplayName={columnDef.name}
|
||||
columnDescription={columnDef.description}
|
||||
columnFlags={columnDef.flags}
|
||||
holdTaskIds={holdTaskIds}
|
||||
workflowContextMenuColumns={selectedWorkflowContextMenuColumns}
|
||||
tasks={selectedWorkflowTasksByColumn[columnDef.id] ?? []}
|
||||
allTasks={selectedWorkflowTasks}
|
||||
@@ -1032,6 +1065,7 @@ export function Board({ tasks, projectId, maxConcurrent, showWorktreeGrouping, o
|
||||
columnDisplayName={selectedWorkflowArchivedColumn.name}
|
||||
columnDescription={selectedWorkflowArchivedColumn.description}
|
||||
columnFlags={selectedWorkflowArchivedColumn.flags}
|
||||
holdTaskIds={holdTaskIds}
|
||||
workflowContextMenuColumns={selectedWorkflowContextMenuColumns}
|
||||
tasks={selectedWorkflowTasksByColumn[selectedWorkflowArchivedColumn.id] ?? []}
|
||||
allTasks={selectedWorkflowTasks}
|
||||
|
||||
@@ -197,6 +197,14 @@ interface ColumnProps {
|
||||
/** True when the board is in multi-lane workflow mode (flag ON). Switches
|
||||
* column behavior (label, bulk actions, archived detection) from legacy
|
||||
* literals to trait-flag predicates. Flag OFF leaves all behavior legacy. */
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8 drift conversion):
|
||||
The ids of tasks whose own column is a hold lane in THEIR OWN workflow, for the worktree
|
||||
grouping's upcoming-work list. Task ids rather than column ids because column ids are
|
||||
namespaced per workflow and two workflows can disagree about the same name (PR #2625
|
||||
review). Board resolves it; Lane does not pass it and keeps the legacy-id fallback.
|
||||
*/
|
||||
holdTaskIds?: ReadonlySet<string>;
|
||||
workflowMode?: boolean;
|
||||
/** Workflow id for column-aware task creation in workflow mode. */
|
||||
workflowId?: string;
|
||||
@@ -230,7 +238,7 @@ interface ColumnProps {
|
||||
getDraggingTaskId?: () => string | null;
|
||||
}
|
||||
|
||||
function ColumnComponent({ column, tasks, projectId, maxConcurrent, showWorktreeGrouping, onMoveTask, onPauseTask, onUnpauseTask, onResetTask, onDuplicateTask, onMergeTask, onOpenDetail, onOpenRefine, onOpenGroupModal, addToast, onQuickCreate, onNewTask, autoMerge, mergeStrategy = "direct", onToggleAutoMerge, planAutoApproveEnabled, onTogglePlanAutoApprove, globalPaused, onUpdateTask, onRetryTask, onArchiveTask, onUnarchiveTask, onRevertTask, onDeleteTask, onArchiveAllDone, doneSortMode, onDoneSortModeChange, collapsed, onToggleCollapse, archivedHasMore, archivedLoadingMore, onLoadMoreArchived, allTasks, availableModels, onPlanningMode, onSubtaskBreakdown, onOpenDetailWithTab, favoriteProviders, favoriteModels, onToggleFavorite, onToggleModelFavorite, isSearchActive, taskStuckTimeoutMs, onOpenMission, lastFetchTimeMs, taskCardFieldDefs, taskWorkflowBadges, blockerFanoutMap, prAuthAvailable, workflowMode, workflowId, workflowOptions, defaultWorkflowId, columnDisplayName, columnDescription, columnFlags, workflowContextMenuColumns, taskContextMenuColumnsByTaskId, onPromote, canDropTask, getDraggingTaskId }: ColumnProps) {
|
||||
function ColumnComponent({ column, tasks, projectId, maxConcurrent, showWorktreeGrouping, onMoveTask, onPauseTask, onUnpauseTask, onResetTask, onDuplicateTask, onMergeTask, onOpenDetail, onOpenRefine, onOpenGroupModal, addToast, onQuickCreate, onNewTask, autoMerge, mergeStrategy = "direct", onToggleAutoMerge, planAutoApproveEnabled, onTogglePlanAutoApprove, globalPaused, onUpdateTask, onRetryTask, onArchiveTask, onUnarchiveTask, onRevertTask, onDeleteTask, onArchiveAllDone, doneSortMode, onDoneSortModeChange, collapsed, onToggleCollapse, archivedHasMore, archivedLoadingMore, onLoadMoreArchived, allTasks, availableModels, onPlanningMode, onSubtaskBreakdown, onOpenDetailWithTab, favoriteProviders, favoriteModels, onToggleFavorite, onToggleModelFavorite, isSearchActive, taskStuckTimeoutMs, onOpenMission, lastFetchTimeMs, taskCardFieldDefs, taskWorkflowBadges, blockerFanoutMap, prAuthAvailable, holdTaskIds, workflowMode, workflowId, workflowOptions, defaultWorkflowId, columnDisplayName, columnDescription, columnFlags, workflowContextMenuColumns, taskContextMenuColumnsByTaskId, onPromote, canDropTask, getDraggingTaskId }: ColumnProps) {
|
||||
const { t } = useTranslation("app");
|
||||
// Anchor the board.rejection.* catalog keys for the i18next extractor (it
|
||||
// scopes `t` to the useTranslation binding, so the shared translateRejection
|
||||
@@ -555,8 +563,12 @@ function ColumnComponent({ column, tasks, projectId, maxConcurrent, showWorktree
|
||||
|
||||
const worktreeGroups = useMemo(() => {
|
||||
if (!showWorktreeGroups) return [];
|
||||
return groupByWorktree(tasks, allTasks ?? tasks, maxConcurrent);
|
||||
}, [showWorktreeGroups, tasks, allTasks, maxConcurrent]);
|
||||
return groupByWorktree(tasks, allTasks ?? tasks, maxConcurrent, holdTaskIds);
|
||||
// `holdTaskIds` IS a dependency: the board resolves it after the workflows fetch, so
|
||||
// omitting it would pin the first-paint value and the upcoming-work list would keep
|
||||
// using the legacy-id fallback for the rest of the session. This repo has no
|
||||
// react-hooks/exhaustive-deps rule, so nothing catches that but reading it.
|
||||
}, [showWorktreeGroups, tasks, allTasks, maxConcurrent, holdTaskIds]);
|
||||
|
||||
const visibleTasks = useMemo(() => {
|
||||
if (!shouldPaginate) return tasks;
|
||||
|
||||
@@ -34,6 +34,7 @@ import { useTaskDiffStats } from "../hooks/useTaskDiffStats";
|
||||
import { useAgentsMapCache } from "../hooks/useAgentsMapCache";
|
||||
import { useLiveTimeTicker } from "../hooks/useLiveTimeTicker";
|
||||
import { isTaskStuck } from "../utils/taskStuck";
|
||||
import { isFieldEditableColumnRole } from "../utils/columnRoles";
|
||||
import { hasPendingAutomaticRecovery, isTaskManuallyRetryable } from "../utils/taskRecovery";
|
||||
import { getRevertOfId, isTaskReverted } from "../utils/taskRevert";
|
||||
import { getStalledReviewSignal } from "../utils/taskStalledReview";
|
||||
@@ -309,7 +310,6 @@ function isSameAgentIdentity(
|
||||
|
||||
// Issue 1403: widened to ColumnId so `.has(task.column)` accepts custom column ids
|
||||
// (which are not members and correctly resolve to false).
|
||||
const EDITABLE_COLUMNS: Set<ColumnId> = new Set<ColumnId>(["triage", "todo"]);
|
||||
|
||||
const ACTIVE_MERGE_STATUSES = new Set(
|
||||
[...ACTIVE_STATUSES].filter((status) => ["merging", "merging-pr", "merging-fix", "reviewing", "landing"].includes(status)),
|
||||
@@ -1498,7 +1498,15 @@ function TaskCardComponent({
|
||||
const isDraggable = !disableDrag && !queued && !isPaused && !isEditing && !isArchived && !isCoarsePointer; // Disable drag during edit/archived, host embedding, or touch
|
||||
|
||||
// Check if this card can be edited inline
|
||||
const canEdit = EDITABLE_COLUMNS.has(task.column) && !isAgentActive && !isPaused && !queued && onUpdateTask;
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-00:15 (U12 — R8 drift conversion):
|
||||
THE ASYMMETRY: this read a hardcoded `{triage, todo}` set with no trait path, while
|
||||
TaskDetailModal resolved the SAME affordance from column traits (U10/R8). So on a renamed board an
|
||||
operator could edit a task's title in the detail modal but the pencil was missing from its card,
|
||||
and after #2515 the `triage` half was dead weight. `taskColumnFlags` was already in scope here —
|
||||
the card simply never asked.
|
||||
*/
|
||||
const canEdit = isFieldEditableColumnRole(taskColumnFlags, task.column) && !isAgentActive && !isPaused && !queued && onUpdateTask;
|
||||
const githubTrackedIssue = task.githubTracking?.issue;
|
||||
const hasGithubTrackingLink = Boolean(githubTrackedIssue);
|
||||
const isGitHubImportedTask = task.sourceType === "github_import";
|
||||
|
||||
@@ -24,6 +24,7 @@ import { resolveEffectivePlannerOversightLevel } from "../../../core/src/workflo
|
||||
import { resolveTaskSessionAdvisorEnabled } from "../../../core/src/session-advisor";
|
||||
import { isNearDuplicateCanonicalInactive } from "../../../core/src/near-duplicate-canonical";
|
||||
import { getRevertOfId, findOpenUndoTaskForSource } from "../utils/taskRevert";
|
||||
import { isFieldEditableColumnRole } from "../utils/columnRoles";
|
||||
import { resolveEffectiveAutoMerge } from "../../../core/src/task-merge";
|
||||
import { uploadAttachment, deleteAttachment, updateTask, repairOverlapBlocker, pauseTask, unpauseTask, fetchTaskDetail, fetchTaskVerificationRequest, fetchSettings, fetchTaskEffectiveSettings, fetchGlobalSettings, requestSpecRevision, rebuildTaskSpec, approvePlan, rejectPlan, refineTask, fetchWorkflowResults, assignTask, fetchAgents, fetchAgent, refreshPrStatus, fetchBoardWorkflows, updateTaskCustomFields, summarizeTitle, fetchWorkflowSettingValues, nudgeOverseer, stopOverseer, explainOverseer, fetchModels, fetchNodes, api } from "../api";
|
||||
import type { RevertTaskOptions, RevertTaskResult, ModelInfo, NodeInfo } from "../api";
|
||||
@@ -662,7 +663,6 @@ function getProvenanceLabel(task: Task | TaskDetail, options: ProvenanceLabelOpt
|
||||
|
||||
// #1403: widened to ColumnId so `.has(task.column)` accepts custom column ids
|
||||
// (non-members correctly resolve to false → not editable).
|
||||
const EDITABLE_COLUMNS: Set<ColumnId> = new Set<ColumnId>(["triage", "todo"]);
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-27-15:30 (U10 / R8):
|
||||
@@ -675,11 +675,13 @@ before the board-workflows payload resolves and for a column the workflow does n
|
||||
where the traits are unknown rather than known-false.
|
||||
*/
|
||||
function isTaskFieldEditableColumn(column: ColumnId, flags?: TaskContextMenuColumnFlags): boolean {
|
||||
if (!flags) return EDITABLE_COLUMNS.has(column);
|
||||
if (flags.complete || flags.archived || flags.countsTowardWip || flags.mergeBlocker || flags.humanReview) {
|
||||
return false;
|
||||
}
|
||||
return flags.intake === true || flags.hold === true;
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-00:15 (U12): body moved UNCHANGED to
|
||||
`isFieldEditableColumnRole`, so this and TaskCard cannot drift apart again. TaskCard implemented the
|
||||
same affordance with the raw id set and no trait path, which is how a renamed board lost inline
|
||||
editing on the card while this surface kept it.
|
||||
*/
|
||||
return isFieldEditableColumnRole(flags, column);
|
||||
}
|
||||
const GITHUB_TRACKING_EDITABLE_COLUMNS: Set<ColumnId> = new Set<ColumnId>(["triage", "todo", "in-progress", "in-review", "ideas"]);
|
||||
const CODING_IDEAS_WORKFLOW_ID = "builtin:coding-ideas";
|
||||
|
||||
@@ -8177,3 +8177,70 @@ describe("TaskCard Start affordance (FN-7596)", () => {
|
||||
await waitFor(() => expect(addToast).toHaveBeenCalledWith("move blocked", "error"));
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-00:15 (U12 — the affordance this file never covered):
|
||||
THE EDIT BUTTON ON A RENAMED BOARD.
|
||||
|
||||
TaskDetailModal resolved field editability from column traits in U10/R8. TaskCard implemented the
|
||||
same affordance with a hardcoded `{triage, todo}` id set and NO trait path, even though
|
||||
`taskColumnFlags` was already in scope — so on a board whose pre-implementation column is renamed,
|
||||
the title was editable in the detail modal and the pencil was absent from the card.
|
||||
|
||||
VERIFIED UNCOVERED rather than assumed: mutating `canEdit` back to the hardcoded set left
|
||||
`app/components/__tests__/TaskCard*` at exactly the same failure count as the unmutated run, so
|
||||
nothing caught it. These four assert the real `aria-label`, and that mutation now fails with
|
||||
"Unable to find an accessible element ... name 'Edit task'".
|
||||
*/
|
||||
describe("TaskCard field editability resolves column traits (U12 — R8)", () => {
|
||||
const EDIT_LABEL = { name: "Edit task" };
|
||||
|
||||
it("renders the edit button for a RENAMED pre-implementation column", () => {
|
||||
render(
|
||||
<TaskCard
|
||||
task={makeTask({ column: "backlog" as any })}
|
||||
taskColumnFlags={{ intake: true, hold: true }}
|
||||
onUpdateTask={noop}
|
||||
onOpenDetail={noop}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
// Fails with the hardcoded id set: `backlog` is not in it.
|
||||
expect(screen.getByRole("button", EDIT_LABEL)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("does NOT render it for a resolved mid-flight column", () => {
|
||||
// The narrowing guard: without it the case above passes for a card that always shows the pencil,
|
||||
// letting an operator rewrite a description while a session executes against it.
|
||||
render(
|
||||
<TaskCard
|
||||
task={makeTask({ column: "building" as any })}
|
||||
taskColumnFlags={{ countsTowardWip: true }}
|
||||
onUpdateTask={noop}
|
||||
onOpenDetail={noop}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
expect(screen.queryByRole("button", EDIT_LABEL)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("vetoes editing when a hold column ALSO carries a review trait", () => {
|
||||
// A legal shape a plain `intake || hold` check gets wrong.
|
||||
render(
|
||||
<TaskCard
|
||||
task={makeTask({ column: "backlog" as any })}
|
||||
taskColumnFlags={{ hold: true, mergeBlocker: true }}
|
||||
onUpdateTask={noop}
|
||||
onOpenDetail={noop}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
expect(screen.queryByRole("button", EDIT_LABEL)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("still renders it for a legacy `todo` card with no flags resolved", () => {
|
||||
// The pre-load window, and what every board did before the conversion.
|
||||
render(<TaskCard task={makeTask({ column: "todo" as any })} onUpdateTask={noop} onOpenDetail={noop} addToast={noop} />);
|
||||
expect(screen.getByRole("button", EDIT_LABEL)).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,154 @@
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8, completion criterion 3):
|
||||
EVIDENCE that the converted dashboard surfaces behave on a RENAMED board and on the
|
||||
MERGED board — stated as an invariance property rather than a pile of per-site cases.
|
||||
|
||||
THE CLAIM THE CONVERSION MAKES. After resolving roles from traits, a surface's behaviour
|
||||
is a function of the column's TRAITS, not of its id. So the test is: hold the traits
|
||||
fixed, change only the id, and every decision must be identical. That is one assertion
|
||||
covering every site at once, and it fails for any site that still consults an id — which
|
||||
per-site cases cannot promise, because a per-site case only proves the site it names.
|
||||
|
||||
WHY THIS IS NOT A RESTATEMENT OF THE HELPER TESTS. `columnRoles.test.ts` proves the
|
||||
helpers answer correctly. It cannot prove the CONSUMERS ask them: a component that kept an
|
||||
inline id comparison passes every helper test. These cases drive the real consumers —
|
||||
`buildTaskActionMenuModel` and `isTaskAgentActive`, the predicates behind the actions menu,
|
||||
the pulsing badge, the row border and the column header's executing count — and compare
|
||||
their outputs across three lineages that differ only in naming.
|
||||
|
||||
THE THREE LINEAGES.
|
||||
MERGED — post-#2515 default: one pre-implementation column, id `todo`, "Planning".
|
||||
LEGACY — the pre-merge shape, id `triage`, same traits.
|
||||
RENAMED — a custom board: `backlog` / `building` / `shipped`, same traits.
|
||||
An id-sensitive site behaves differently on at least one of the three; a trait-driven one
|
||||
cannot tell them apart.
|
||||
|
||||
REVERT CHECK, measured. Restoring `task.column !== "triage"` in `shouldShowActionsMenu`
|
||||
fails the actions-menu invariance case (MERGED and RENAMED start showing a menu that
|
||||
LEGACY suppresses). Restoring the intake-id comparison in `taskActivity`'s planner-lane
|
||||
check fails the agent-active case the same way. Both were run.
|
||||
*/
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { Task } from "@fusion/core";
|
||||
import { buildTaskActionMenuModel } from "../TaskContextMenu";
|
||||
import { isTaskAgentActive } from "../../utils/taskActivity";
|
||||
|
||||
const t = ((_key: string, fallback?: string) => fallback ?? _key) as never;
|
||||
const columnLabel = ((column: string) => column) as never;
|
||||
|
||||
/**
|
||||
* The same PRE-IMPLEMENTATION column expressed three ways. Traits are byte-identical;
|
||||
* only `id` differs, which is the whole point.
|
||||
*/
|
||||
const PRE_IMPLEMENTATION_TRAITS = { intake: true, hold: true } as const;
|
||||
const LINEAGES = [
|
||||
{ label: "MERGED (post-#2515 default)", columnId: "todo" },
|
||||
{ label: "LEGACY (pre-merge)", columnId: "triage" },
|
||||
{ label: "RENAMED (custom board)", columnId: "backlog" },
|
||||
] as const;
|
||||
|
||||
/** A mid-flight column, likewise expressed under three names. */
|
||||
const WIP_TRAITS = { countsTowardWip: true } as const;
|
||||
const WIP_LINEAGES = [
|
||||
{ label: "MERGED", columnId: "in-progress" },
|
||||
{ label: "RENAMED", columnId: "building" },
|
||||
] as const;
|
||||
|
||||
function mkTask(overrides: Partial<Task> & { id: string; column: string }): Task {
|
||||
return {
|
||||
title: overrides.id,
|
||||
description: "Task",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
status: undefined,
|
||||
paused: false,
|
||||
log: [],
|
||||
createdAt: "2026-07-27T00:00:00.000Z",
|
||||
updatedAt: "2026-07-27T00:00:00.000Z",
|
||||
...overrides,
|
||||
} as unknown as Task;
|
||||
}
|
||||
|
||||
/** Every decision the actions menu exposes, as a comparable shape. */
|
||||
function menuDecisions(columnId: string, flags: Record<string, boolean>, task: Partial<Task> = {}) {
|
||||
const model = buildTaskActionMenuModel({
|
||||
task: mkTask({ id: "FN-1", column: columnId, ...task }),
|
||||
t,
|
||||
columnLabel,
|
||||
currentColumnFlags: flags as never,
|
||||
onPlan: vi.fn(),
|
||||
} as never);
|
||||
return {
|
||||
shouldShowActionsMenu: model.shouldShowActionsMenu,
|
||||
actionIds: model.actions.map((action: { id: string }) => action.id),
|
||||
};
|
||||
}
|
||||
|
||||
describe("column-role decisions are invariant under column RENAMING (U12 evidence)", () => {
|
||||
it("the actions menu decides identically on merged, legacy and renamed lineages", () => {
|
||||
const decisions = LINEAGES.map((lineage) => ({
|
||||
label: lineage.label,
|
||||
...menuDecisions(lineage.columnId, PRE_IMPLEMENTATION_TRAITS as never),
|
||||
}));
|
||||
|
||||
/*
|
||||
Compared against the FIRST lineage rather than a hardcoded expectation: the property
|
||||
under test is agreement, and hardcoding would quietly bake in whichever shape happened
|
||||
to be current. If a site consults an id, the entries diverge and the diff names it.
|
||||
*/
|
||||
const [first, ...rest] = decisions;
|
||||
for (const other of rest) {
|
||||
expect({ ...other, label: first!.label }).toEqual(first);
|
||||
}
|
||||
|
||||
// And the shape is not vacuous: a pre-implementation card really does offer Plan.
|
||||
expect(first!.actionIds).toContain("plan");
|
||||
});
|
||||
|
||||
it("a mid-flight card decides identically whether its column is `in-progress` or `building`", () => {
|
||||
const decisions = WIP_LINEAGES.map((lineage) => ({
|
||||
label: WIP_LINEAGES[0]!.label,
|
||||
...menuDecisions(lineage.columnId, WIP_TRAITS as never),
|
||||
}));
|
||||
expect(decisions[1]).toEqual(decisions[0]);
|
||||
// The inversion guard, stated positively: executing cards are never planning targets.
|
||||
expect(decisions[0]!.actionIds).not.toContain("plan");
|
||||
});
|
||||
|
||||
it("planner activity reads as agent-active identically across all three lineages", () => {
|
||||
/*
|
||||
`isTaskAgentActive` drives three separate surfaces from one predicate, so an id-sensitive
|
||||
fallback here reports planning work as idle board-wide — the failure that motivated the
|
||||
`taskActivity` conversion, and one that throws nothing.
|
||||
*/
|
||||
const recent = new Date(Date.now() - 5_000).toISOString();
|
||||
const verdicts = LINEAGES.map((lineage) =>
|
||||
isTaskAgentActive(
|
||||
mkTask({ id: "FN-2", column: lineage.columnId, recentAgentActivityAt: recent } as never),
|
||||
{ columnFlags: PRE_IMPLEMENTATION_TRAITS as never },
|
||||
),
|
||||
);
|
||||
|
||||
expect(new Set(verdicts).size).toBe(1);
|
||||
// Non-vacuous: fresh planner activity on a pre-implementation card IS active.
|
||||
expect(verdicts[0]).toBe(true);
|
||||
});
|
||||
|
||||
it("a stale planner card is inactive on all three lineages, so the invariance is not just `always true`", () => {
|
||||
/*
|
||||
Without this, the case above would pass for a predicate hardwired to `true`. Both
|
||||
verdicts must be unanimous AND opposite each other for the invariance to mean anything.
|
||||
*/
|
||||
const stale = new Date(Date.now() - 60 * 60 * 1000).toISOString();
|
||||
const verdicts = LINEAGES.map((lineage) =>
|
||||
isTaskAgentActive(
|
||||
mkTask({ id: "FN-3", column: lineage.columnId, recentAgentActivityAt: stale } as never),
|
||||
{ columnFlags: PRE_IMPLEMENTATION_TRAITS as never },
|
||||
),
|
||||
);
|
||||
|
||||
expect(new Set(verdicts).size).toBe(1);
|
||||
expect(verdicts[0]).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -139,3 +139,63 @@ describe("sortTasksForDisplayColumn", () => {
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8 drift conversion):
|
||||
The hold lane's QUEUE ORDER must follow the hold trait, not the id `todo`.
|
||||
|
||||
Priority-then-FIFO is what makes a high-priority card visibly next in the lane where work
|
||||
waits for capacity. Keyed on the id it degrades to the generic sort on any board whose
|
||||
hold column is renamed — the card order is simply wrong, and nothing fails. U11 is what
|
||||
separated the two questions: `todo` GAINED the hold trait, so "is this column named todo"
|
||||
and "is this the lane work waits in" stopped being the same test.
|
||||
|
||||
REVERT CHECK, measured: restoring `if (column === "todo")` fails the renamed case below —
|
||||
the high-priority card comes back second instead of first.
|
||||
*/
|
||||
describe("hold-lane queue order follows the hold TRAIT", () => {
|
||||
const mk = (id: string, priority: string, createdAt: string, column = "backlog") =>
|
||||
({ id, title: id, priority, createdAt, column, steps: [], dependencies: [], log: [] } as never);
|
||||
|
||||
/*
|
||||
THE DISCRIMINATOR IS THE TIEBREAK, not the priority. Both branches sort by priority
|
||||
first, so a priority difference proves nothing about which branch ran — my first attempt
|
||||
at this asserted exactly that and passed for the wrong reason.
|
||||
|
||||
What is unique to the hold lane is FIFO: equal priority tiebreaks on `createdAt`, where
|
||||
the generic sort tiebreaks on task id. So these two carry the same priority and have
|
||||
createdAt order DISAGREEING with id order — FN-9 is older, FN-2 has the lower id.
|
||||
hold branch -> FIFO -> FN-9 first
|
||||
generic branch -> id ascending -> FN-2 first
|
||||
*/
|
||||
const older = mk("FN-9", "normal", "2026-07-01T00:00:00.000Z");
|
||||
const newer = mk("FN-2", "normal", "2026-07-05T00:00:00.000Z");
|
||||
|
||||
it("applies FIFO order on a RENAMED hold column when the trait is supplied", () => {
|
||||
const sorted = sortTasksForDisplayColumn([newer, older], "backlog" as never, undefined, false, true);
|
||||
expect(sorted.map((task) => task.id)).toEqual(["FN-9", "FN-2"]);
|
||||
});
|
||||
|
||||
it("still applies it to the legacy `todo` id by default, for callers that resolve no flags", () => {
|
||||
// Lane and ListView do not pass flags; the default keeps their behaviour unchanged.
|
||||
const sorted = sortTasksForDisplayColumn(
|
||||
[mk("FN-2", "normal", "2026-07-05T00:00:00.000Z", "todo"), mk("FN-9", "normal", "2026-07-01T00:00:00.000Z", "todo")],
|
||||
"todo" as never,
|
||||
);
|
||||
expect(sorted.map((task) => task.id)).toEqual(["FN-9", "FN-2"]);
|
||||
});
|
||||
|
||||
it("does NOT apply FIFO to a non-hold column, which sorts by id", () => {
|
||||
// The narrowing guard: without it, a function that FIFO-sorted everything would pass
|
||||
// the cases above while silently reordering the in-progress and review lanes.
|
||||
const sorted = sortTasksForDisplayColumn([newer, older], "building" as never, undefined, false, false);
|
||||
expect(sorted.map((task) => task.id)).toEqual(["FN-2", "FN-9"]);
|
||||
});
|
||||
|
||||
it("keeps priority ahead of FIFO in the hold lane", () => {
|
||||
// FIFO is the tiebreak, not the primary key: a newer high-priority card still leads.
|
||||
const high = mk("FN-3", "high", "2026-07-09T00:00:00.000Z");
|
||||
const sorted = sortTasksForDisplayColumn([older, high], "backlog" as never, undefined, false, true);
|
||||
expect(sorted.map((task) => task.id)).toEqual(["FN-3", "FN-9"]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -54,6 +54,20 @@ export function sortTasksForDisplayColumn(
|
||||
column: Column,
|
||||
doneSortMode: DoneColumnSortMode = "completion-date-desc",
|
||||
isArchivedColumn: boolean = column === "archived",
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8 drift conversion):
|
||||
Does this column HOLD planned work waiting for capacity? Follows the same
|
||||
caller-supplies-the-trait shape `isArchivedColumn` already established, and defaults to
|
||||
the legacy id so the callers that do not resolve flags (Lane, ListView) keep today's
|
||||
behaviour exactly.
|
||||
|
||||
The priority-then-FIFO order below is the hold lane's queue order — it is what makes a
|
||||
high-priority card visibly next. Keyed on the id, it silently degrades to the generic
|
||||
sort on any board whose hold column is not named `todo`, so a renamed lineage loses
|
||||
priority ordering with nothing failing. Not a rename: `todo` gained the hold trait in
|
||||
U11, so the id and the role stopped being the same question.
|
||||
*/
|
||||
isHoldColumn: boolean = column === "todo",
|
||||
): Task[] {
|
||||
/*
|
||||
FNXC:ArchivePagination 2026-07-08-00:00:
|
||||
@@ -70,7 +84,7 @@ export function sortTasksForDisplayColumn(
|
||||
return [...tasks];
|
||||
}
|
||||
|
||||
if (column === "todo") {
|
||||
if (isHoldColumn) {
|
||||
return [...tasks].sort((a, b) => {
|
||||
const priorityCmp = compareTaskPriority(a.priority, b.priority);
|
||||
if (priorityCmp !== 0) return priorityCmp;
|
||||
|
||||
@@ -147,3 +147,100 @@ describe("groupByWorktree", () => {
|
||||
expect(upNext!.queuedTasks).toEqual([queued]);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8 drift conversion):
|
||||
The upcoming-work list must find the HOLD lane by trait, not by the id `todo`.
|
||||
|
||||
WHY THIS ONE HID. On the default board the id and the role coincide — U11 gave `todo` the
|
||||
hold trait — so every existing case here passed and the site looked healthy. Rename the
|
||||
hold column and the filter matched nothing: the worktree view showed no upcoming work at
|
||||
all and read as idle. A whole panel silently empty, nothing thrown.
|
||||
|
||||
Board resolves the hold ids across ALL workflows on the board (a card in another
|
||||
workflow's hold lane is still upcoming work); Lane passes nothing and keeps the legacy
|
||||
fallback, which is why the default case below omits the argument entirely.
|
||||
|
||||
REVERT CHECK, measured: dropping the parameter back to `t.column === "todo"` fails the
|
||||
renamed case with an empty queue.
|
||||
*/
|
||||
describe("upcoming-work queue resolves the hold lane by trait", () => {
|
||||
const mkTask = (id: string, column: string, extra: Record<string, unknown> = {}) =>
|
||||
({
|
||||
id, title: id, description: "", column, dependencies: [], steps: [], currentStep: 0,
|
||||
log: [], createdAt: "2026-07-01T00:00:00.000Z", updatedAt: "2026-07-01T00:00:00.000Z",
|
||||
...extra,
|
||||
} as never);
|
||||
|
||||
it("finds waiting cards in a RENAMED hold column when the ids are supplied", () => {
|
||||
const waiting = mkTask("FN-50", "backlog");
|
||||
const groups = groupByWorktree([], [waiting], 3, new Set(["FN-50"]));
|
||||
const queued = groups.flatMap((group) => group.queuedTasks ?? []);
|
||||
expect(queued.map((task: { id: string }) => task.id)).toContain("FN-50");
|
||||
});
|
||||
|
||||
it("finds nothing in a renamed hold column when the ids are NOT supplied", () => {
|
||||
/*
|
||||
Pins the fallback's real limit rather than pretending it covers custom boards: with no
|
||||
resolved ids the legacy guess is all there is, and it cannot know about `backlog`. This
|
||||
is the case that used to be the ONLY behaviour, on every board.
|
||||
*/
|
||||
const groups = groupByWorktree([], [mkTask("FN-51", "backlog")], 3);
|
||||
const queued = groups.flatMap((group) => group.queuedTasks ?? []);
|
||||
expect(queued.map((task: { id: string }) => task.id)).not.toContain("FN-51");
|
||||
});
|
||||
|
||||
it("still finds legacy `todo` cards with no ids supplied, so Lane is unaffected", () => {
|
||||
const groups = groupByWorktree([], [mkTask("FN-52", "todo")], 3);
|
||||
const queued = groups.flatMap((group) => group.queuedTasks ?? []);
|
||||
expect(queued.map((task: { id: string }) => task.id)).toContain("FN-52");
|
||||
});
|
||||
|
||||
it("does not treat a task outside the resolved set as waiting", () => {
|
||||
/*
|
||||
The narrowing guard: a lookup that ignored its set would pass the first case. It is keyed
|
||||
on TASK id, so this also pins the per-workflow scoping — a card whose own workflow does
|
||||
not mark its column `hold` is absent from the set even if another workflow reuses the id
|
||||
(PR #2625 review).
|
||||
*/
|
||||
const groups = groupByWorktree([], [mkTask("FN-53", "building")], 3, new Set(["FN-99"]));
|
||||
const queued = groups.flatMap((group) => group.queuedTasks ?? []);
|
||||
expect(queued.map((task: { id: string }) => task.id)).not.toContain("FN-53");
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (PR #2625 review — greptile):
|
||||
TWO WORKFLOWS, ONE COLUMN NAME, DIFFERENT TRAITS — the case that killed the first design.
|
||||
|
||||
My first version passed a board-wide set of hold COLUMN ids. Column ids are namespaced per
|
||||
workflow, so workflow A can declare `staging` as its hold lane while workflow B declares
|
||||
`staging` as executing. A unioned id set answers "is `staging` a hold column?" — a question
|
||||
with no single answer — and every executing card in workflow B would have been listed under
|
||||
Up Next as waiting work. Confidently wrong, which is worse than the renamed-board emptiness
|
||||
the change set out to fix.
|
||||
|
||||
Keying on TASK id moves the decision to the only place that can make it: the caller, which
|
||||
knows each task's own workflow. This case is the regression test for that, expressed the way
|
||||
the helper now sees it — one card in the set, one not, both in a column called `staging`.
|
||||
*/
|
||||
describe("hold resolution is scoped per workflow, not per column name", () => {
|
||||
const mkTask = (id: string, column: string) =>
|
||||
({
|
||||
id, title: id, description: "", column, dependencies: [], steps: [], currentStep: 0,
|
||||
log: [], createdAt: "2026-07-01T00:00:00.000Z", updatedAt: "2026-07-01T00:00:00.000Z",
|
||||
} as never);
|
||||
|
||||
it("lists only the card whose OWN workflow marks `staging` as hold", () => {
|
||||
const waitingInA = mkTask("FN-60", "staging");
|
||||
const executingInB = mkTask("FN-61", "staging");
|
||||
|
||||
// What Board computes: FN-60's workflow declares `staging` hold; FN-61's does not.
|
||||
const groups = groupByWorktree([], [waitingInA, executingInB], 3, new Set(["FN-60"]));
|
||||
const queued = groups.flatMap((group) => group.queuedTasks ?? []).map((task: { id: string }) => task.id);
|
||||
|
||||
expect(queued).toContain("FN-60");
|
||||
// The assertion the column-id design could not satisfy: same column name, opposite answer.
|
||||
expect(queued).not.toContain("FN-61");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -63,3 +63,55 @@ export function isPreImplementationColumnRole(flags: ColumnRoleFlags | undefined
|
||||
? Boolean(flags.intake || flags.hold)
|
||||
: LEGACY_PRE_IMPLEMENTATION_COLUMN_IDS.has(columnId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Does this column play the HOLD role — a lane a card WAITS in rather than works in?
|
||||
*
|
||||
* FNXC:WorkflowResolvedColumns 2026-07-30-23:30 (U12 — worktree upcoming-work list):
|
||||
* Distinct from {@link isPreImplementationColumnRole}, whose degraded id set is `{todo, triage}`.
|
||||
* This one degrades to `todo` ALONE, because `triage` was the intake lane and never the
|
||||
* wait-for-capacity lane — listing an intake card as "upcoming work" in the worktree view would
|
||||
* report a card that has no plan yet as ready to run.
|
||||
*
|
||||
* Same shape, different degraded answer — the asymmetry U11 verified for
|
||||
* `isPreExecutionHoldColumn` and this file already documents elsewhere.
|
||||
*/
|
||||
export function isHoldColumnRole(flags: ColumnRoleFlags | undefined, columnId: string): boolean {
|
||||
return flags ? flags.hold === true : columnId === "todo";
|
||||
}
|
||||
|
||||
/** Legacy pre-implementation id pair, used only when a column has no resolved traits. */
|
||||
const LEGACY_FIELD_EDITABLE_COLUMN_IDS: ReadonlySet<string> = new Set(["triage", "todo"]);
|
||||
|
||||
/**
|
||||
* May a card's title/description be edited in this column?
|
||||
*
|
||||
* FNXC:WorkflowResolvedColumns 2026-07-31-00:15 (U12 — one affordance, two components):
|
||||
* Editing belongs to PRE-IMPLEMENTATION lanes: the card has no session, no worktree, and no plan
|
||||
* being executed against the text. Any terminal, executing or review trait VETOES it even when
|
||||
* `intake`/`hold` is also present — a column carrying both is still one where work is underway, and
|
||||
* rewriting a description out from under a running plan is the failure being prevented.
|
||||
*
|
||||
* Extracted because `TaskDetailModal` resolved this from traits (U10/R8) while `TaskCard` kept a
|
||||
* hardcoded `{triage, todo}` set with NO trait path at all, even though `taskColumnFlags` was already
|
||||
* in scope there. On a renamed board the operator could edit a task's title in the detail modal but
|
||||
* the pencil button was absent from its card. One affordance living in two components with one of
|
||||
* them converted is the FN-6115 -> FN-6118 -> FN-6123 shape; a shared definition is what stops it
|
||||
* recurring, not a second conversion.
|
||||
*/
|
||||
export function isFieldEditableColumnRole(
|
||||
flags: (ColumnRoleFlags & {
|
||||
readonly complete?: boolean;
|
||||
readonly archived?: boolean;
|
||||
readonly countsTowardWip?: boolean;
|
||||
readonly mergeBlocker?: boolean;
|
||||
readonly humanReview?: boolean;
|
||||
}) | undefined,
|
||||
columnId: string,
|
||||
): boolean {
|
||||
if (!flags) return LEGACY_FIELD_EDITABLE_COLUMN_IDS.has(columnId);
|
||||
if (flags.complete || flags.archived || flags.countsTowardWip || flags.mergeBlocker || flags.humanReview) {
|
||||
return false;
|
||||
}
|
||||
return flags.intake === true || flags.hold === true;
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { Task } from "@fusion/core";
|
||||
import { getPathBasename } from "./pathDisplay";
|
||||
import { isHoldColumnRole } from "./columnRoles";
|
||||
|
||||
export interface WorktreeGroupData {
|
||||
label: string;
|
||||
@@ -58,6 +59,21 @@ export function groupByWorktree(
|
||||
inProgressTasks: Task[],
|
||||
allTasks: Task[],
|
||||
maxConcurrent: number,
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8 drift conversion):
|
||||
The ids of TASKS whose own column is a hold lane in their own workflow, when the caller
|
||||
resolved them.
|
||||
|
||||
Task ids, not column ids (PR #2625 review — greptile). This helper scans `allTasks`, which
|
||||
can span workflows, and column ids are namespaced per workflow — two workflows may both
|
||||
declare `staging` with only one of them marking it `hold`. A unioned column-id set cannot
|
||||
tell those apart and would list every executing card of the second workflow as waiting.
|
||||
Only a card's own workflow can answer "is this waiting?", so the caller answers it per task
|
||||
and passes the result.
|
||||
|
||||
Board resolves this; Lane does not pass it and keeps the legacy-id fallback.
|
||||
*/
|
||||
holdTaskIds?: ReadonlySet<string>,
|
||||
): WorktreeGroupData[] {
|
||||
// Separate assigned vs unassigned in-progress tasks
|
||||
const assigned = inProgressTasks.filter((t) => t.worktree);
|
||||
@@ -72,9 +88,25 @@ export function groupByWorktree(
|
||||
worktreeMap.set(key, list);
|
||||
}
|
||||
|
||||
// Find queued todo tasks: "todo" tasks with all deps satisfied (done or in-review)
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8 drift conversion):
|
||||
The filter wants "cards waiting for capacity" — the HOLD role, resolved from the board's
|
||||
columns rather than the id `todo`.
|
||||
|
||||
THE BUG THIS CLOSES, measured rather than assumed: on the default board the id and the
|
||||
role coincide (U11 gave `todo` the hold trait), so this looked healthy. On a board whose
|
||||
hold column is renamed the filter matched NOTHING, so the worktree view showed no upcoming
|
||||
work at all and read as idle — a whole panel silently empty, with nothing failing.
|
||||
|
||||
Dependency satisfaction below still names terminal ids. That is a separate question from
|
||||
the hold role and is left alone deliberately: it needs `complete`/`mergeBlocker`/`archived`
|
||||
traits for the DEPENDENCY's column, which is another lookup and another unit of work.
|
||||
*/
|
||||
// Find queued hold-lane tasks: cards in the hold column with all deps satisfied.
|
||||
const taskById = new Map(allTasks.map((t) => [t.id, t]));
|
||||
const todoTasks = allTasks.filter((t) => t.column === "todo");
|
||||
const isWaitingTask = (task: Task): boolean =>
|
||||
holdTaskIds ? holdTaskIds.has(task.id) : isHoldColumnRole(undefined, task.column);
|
||||
const todoTasks = allTasks.filter(isWaitingTask);
|
||||
const eligible = todoTasks.filter((t) =>
|
||||
!t.paused &&
|
||||
(t.dependencies || []).every((depId) => {
|
||||
|
||||
@@ -0,0 +1,192 @@
|
||||
/*
|
||||
FNXC:WorkflowColumns 2026-07-30-21:00 (U12 — precondition 2 for flipping the move-path flag):
|
||||
WHICH ENGINE MOVES WOULD SEAM 2 NEWLY REJECT?
|
||||
|
||||
`moves.ts` gates target-column validation on the retired compatibility flag (see
|
||||
`packages/core/src/__tests__/moves-workflow-flag-seams.test.ts` for all six seams). With the flag
|
||||
off there is NO validation: any `moveTask(id, column)` is accepted. Flipping it turns on
|
||||
unknown-column and adjacency rejections, so a move whose target the task's workflow does not declare
|
||||
starts FAILING on the path every engine lane uses. That is new refusals, not an equivalence
|
||||
question, and it is the precondition that has to be measured rather than asserted.
|
||||
|
||||
WHAT THIS MEASURES, and the finding. Every engine `moveTask` call whose target is a string LITERAL,
|
||||
against the columns the default workflow declares:
|
||||
|
||||
todo 27, in-progress 7, done 6, archived 1 (41 calls) -> all four ARE declared by the default lineage.
|
||||
|
||||
Of those 41, twenty carry no `recoveryRehome` (todo 7, in-progress 7, done 6) and twenty-one do. The
|
||||
numbers are the AST census's, not a grep's: a line-oriented grep reported todo=29 because it cannot
|
||||
tell a call from a comment mentioning one, which is exactly the class of error this file exists to
|
||||
stop repeating.
|
||||
|
||||
So the default board is not the exposure. `triage` appears only inside a comment recording that
|
||||
`replan-target.ts` "used to hardcode moveTask(id, 'triage')" — a live call would have been the first
|
||||
thing to break, and there isn't one.
|
||||
|
||||
THE EXPOSURE IS CUSTOM WORKFLOWS. Those same hardcoded legacy ids are not guaranteed to exist in a
|
||||
custom lineage, so post-flip an engine recovery moving a custom-workflow card to `todo` rejects with
|
||||
unknown-column unless it carries `recoveryRehome` — the #1411 carve-out that exempts legacy landing
|
||||
columns precisely so a custom-workflow card can still be rescued. That carve-out is therefore
|
||||
load-bearing for the flip, and this test pins which call sites depend on it.
|
||||
|
||||
A rejection here is not a crash — it is a recovery that silently stops recovering, which is the
|
||||
failure class this whole program has been finding. Hence a census with names, not a count.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { existsSync, readFileSync, readdirSync, statSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import ts from "typescript";
|
||||
import { getBuiltinWorkflow, parseWorkflowIr } from "@fusion/core";
|
||||
|
||||
const ENGINE_SRC = join(import.meta.dirname, "..");
|
||||
|
||||
interface MoveCall {
|
||||
file: string;
|
||||
line: number;
|
||||
target: string;
|
||||
/** Options-object keys we can see statically, e.g. `recoveryRehome`, `bypassGuards`. */
|
||||
optionKeys: string[];
|
||||
}
|
||||
|
||||
function collectFiles(dir: string, out: string[]): void {
|
||||
if (!existsSync(dir)) return;
|
||||
for (const entry of readdirSync(dir)) {
|
||||
if (entry === "__tests__" || entry === "node_modules" || entry === "dist") continue;
|
||||
const full = join(dir, entry);
|
||||
if (statSync(full).isDirectory()) {
|
||||
collectFiles(full, out);
|
||||
continue;
|
||||
}
|
||||
if (entry.endsWith(".ts") && !entry.includes(".test.")) out.push(full);
|
||||
}
|
||||
}
|
||||
|
||||
/** Rightmost name of a possibly-qualified callee, so `store.moveTask` and `this.store.moveTask` both match. */
|
||||
function calleeName(node: ts.Expression): string | undefined {
|
||||
let cur: ts.Node = node;
|
||||
while (ts.isPropertyAccessExpression(cur)) cur = cur.name;
|
||||
return ts.isIdentifier(cur) ? cur.text : undefined;
|
||||
}
|
||||
|
||||
function literalText(node: ts.Node | undefined): string | undefined {
|
||||
if (!node) return undefined;
|
||||
if (ts.isStringLiteral(node) || ts.isNoSubstitutionTemplateLiteral(node)) return node.text;
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function collectMoveCalls(): MoveCall[] {
|
||||
const files: string[] = [];
|
||||
collectFiles(ENGINE_SRC, files);
|
||||
// Guards the guard: a broken path would make every assertion below vacuously true.
|
||||
if (files.length < 20) throw new Error(`census scanned only ${files.length} engine files; path resolution is broken`);
|
||||
|
||||
const calls: MoveCall[] = [];
|
||||
for (const file of files) {
|
||||
const source = readFileSync(file, "utf-8");
|
||||
const sf = ts.createSourceFile(file, source, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS);
|
||||
/*
|
||||
`createSourceFile` is error-tolerant, so a syntax error yields a PARTIAL tree whose calls are
|
||||
simply absent. Reporting that as "no undeclared targets" would report not-inspected as inspected.
|
||||
*/
|
||||
const parseErrors = (sf as unknown as { parseDiagnostics?: readonly ts.Diagnostic[] }).parseDiagnostics ?? [];
|
||||
if (parseErrors.length > 0) {
|
||||
throw new Error(`census could not parse ${file}: ${ts.flattenDiagnosticMessageText(parseErrors[0]!.messageText, " ")}`);
|
||||
}
|
||||
|
||||
const visit = (node: ts.Node): void => {
|
||||
if (ts.isCallExpression(node) && calleeName(node.expression) === "moveTask") {
|
||||
const target = literalText(node.arguments[1]);
|
||||
if (target !== undefined) {
|
||||
const optionsArg = node.arguments[2];
|
||||
const optionKeys =
|
||||
optionsArg && ts.isObjectLiteralExpression(optionsArg)
|
||||
? optionsArg.properties
|
||||
.map((prop) => (prop.name && ts.isIdentifier(prop.name) ? prop.name.text : undefined))
|
||||
.filter((name): name is string => name !== undefined)
|
||||
: [];
|
||||
calls.push({
|
||||
file: file.slice(ENGINE_SRC.length + 1),
|
||||
line: sf.getLineAndCharacterOfPosition(node.getStart(sf)).line + 1,
|
||||
target,
|
||||
optionKeys,
|
||||
});
|
||||
}
|
||||
}
|
||||
ts.forEachChild(node, visit);
|
||||
};
|
||||
visit(sf);
|
||||
}
|
||||
return calls;
|
||||
}
|
||||
|
||||
function defaultColumnIds(): Set<string> {
|
||||
/*
|
||||
Read through the builtin registry rather than a private default helper: this is the same lookup
|
||||
`resolveWorkflowIrById` uses for a builtin id, so the census is judged against the IR the product
|
||||
actually resolves.
|
||||
*/
|
||||
const builtin = getBuiltinWorkflow("builtin:stepwise-coding");
|
||||
if (!builtin) throw new Error("census could not resolve the default builtin workflow");
|
||||
const resolved = typeof builtin.ir === "string" ? parseWorkflowIr(builtin.ir) : builtin.ir;
|
||||
return new Set(resolved.columns.map((column: { id: string }) => column.id));
|
||||
}
|
||||
|
||||
describe("engine move targets vs the columns a workflow declares (U12 flip precondition)", () => {
|
||||
it("finds engine moveTask calls with literal targets, so the census is not vacuous", () => {
|
||||
const calls = collectMoveCalls();
|
||||
expect(calls.length).toBeGreaterThan(10);
|
||||
});
|
||||
|
||||
it("every literal engine move target IS declared by the DEFAULT workflow", () => {
|
||||
/*
|
||||
The reassuring half, and the reason the flip is not immediately fatal: nothing in the engine moves
|
||||
a card to a column the default lineage lacks. If this fails, a call site was added whose target
|
||||
the default workflow does not declare, and it will reject the moment the flag flips — on the
|
||||
default board, for every user.
|
||||
*/
|
||||
const declared = defaultColumnIds();
|
||||
const undeclared = collectMoveCalls().filter((call) => !declared.has(call.target));
|
||||
|
||||
expect(
|
||||
undeclared.map((call) => `${call.file}:${call.line} -> "${call.target}"`),
|
||||
undeclared.length
|
||||
? `These engine moves target a column the DEFAULT workflow does not declare, so seam 2 will\n` +
|
||||
`reject them once the move-path flag flips:\n` +
|
||||
undeclared.map((c) => ` ${c.file}:${c.line} "${c.target}" options: ${c.optionKeys.join(", ") || "(none)"}`).join("\n")
|
||||
: undefined,
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("records which call sites depend on the #1411 recoveryRehome carve-out", () => {
|
||||
/*
|
||||
THE ACTUAL EXPOSURE. The default board is safe; a CUSTOM lineage need not declare `todo`,
|
||||
`in-progress`, `done` or `archived` at all. Post-flip, an engine move sending a custom-workflow
|
||||
card to one of those hardcoded ids rejects with unknown-column UNLESS it carries
|
||||
`recoveryRehome`, which exempts legacy landing columns so a custom-workflow card can still be
|
||||
rescued (#1411).
|
||||
|
||||
So this is not "are we safe" — it is the list of moves whose safety RESTS ENTIRELY on that
|
||||
carve-out, which is the thing the flip PR has to argue about. Asserted as a non-empty inventory
|
||||
rather than a pass/fail, because the answer is a list a human must read, and a green tick would
|
||||
hide it.
|
||||
*/
|
||||
const calls = collectMoveCalls();
|
||||
const rescueMoves = calls.filter((call) => call.optionKeys.includes("recoveryRehome"));
|
||||
const plainMoves = calls.filter((call) => !call.optionKeys.includes("recoveryRehome"));
|
||||
|
||||
// Both groups exist; if either were empty the distinction below would be meaningless.
|
||||
expect(rescueMoves.length).toBeGreaterThan(0);
|
||||
expect(plainMoves.length).toBeGreaterThan(0);
|
||||
const all = new Map<string, number>();
|
||||
for (const call of calls) all.set(call.target, (all.get(call.target) ?? 0) + 1);
|
||||
console.info(`ALL literal engine move targets: ${[...all.entries()].sort((a,b)=>b[1]-a[1]).map(([t,n])=>`${t}=${n}`).join(", ")} (total ${calls.length})`);
|
||||
|
||||
const byTarget = new Map<string, number>();
|
||||
for (const call of plainMoves) byTarget.set(call.target, (byTarget.get(call.target) ?? 0) + 1);
|
||||
console.info(
|
||||
`engine moves with a literal target, NO recoveryRehome (reject on a custom lineage post-flip):\n` +
|
||||
[...byTarget.entries()].sort((a, b) => b[1] - a[1]).map(([t, n]) => ` ${String(n).padStart(3)} -> "${t}"`).join("\n") +
|
||||
`\nwith recoveryRehome (exempt via #1411): ${rescueMoves.length}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user