From a8dae03fdbe99b2b59650bcd3992277620cb203a Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 05:58:44 -0700 Subject: [PATCH] =?UTF-8?q?fleet(dashboard):=20taskRevert=202=20=E2=86=92?= =?UTF-8?q?=200=20=E2=80=94=20the=20recorded=20blocker=20named=20the=20wro?= =?UTF-8?q?ng=20variable=20(#3129)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The largest remaining census cluster. Deferred twice, with a blocker that turns out to be false **in the same component where its counter-example already lives**. ## What the earlier notes got right `detailColumnFlags` describes the **modal's own task**, and the column classified here belongs to a **neighbour**. Supplying it would answer *"is this neighbour finished?"* with a different row's traits — wrong on data, not merely stale on vocabulary. That reasoning stands and I kept it. An earlier pass also converted this, left the parameter unsupplied, and **reverted it** — correctly. An unsupplied optional parameter is strictly worse than the literal: the guard is gone, the census counts a conversion, and the behaviour is the legacy fallback forever. That rule is why the wiring ships in this same commit. ## What the conclusion got wrong > "A correct conversion needs per-**neighbour** flags — which the modal does not have and should not fetch mid-render." `columnFlagsByTaskId` is a per-task map. It is **already a prop** of `TaskDetailModal` (declared :367, destructured :727), and the call site at :992 sits **below** that destructure. And `TaskDetailModal` already uses it exactly this way, for the near-duplicate canonical: ```ts columnFlagsByTaskId?.get(nearDuplicateCanonical.id) ``` …under a note observing that *its* blocker had been *"asserted from the shape of the problem rather than tested against what was in scope."* Same assertion, one function over. So the supplier the earlier note went looking for exists, is per-neighbour, and needs no fetch. ## What it fixes This lookup skips **finished** candidates so a done/archived prior undo attempt never renders as an active "Undo task" link. On a board that renames those lanes it matched neither — a finished undo task kept rendering as open, which is precisely the stale affordance the function's own header says it exists to prevent. ## Census | | before | after | |---|---|---| | `taskRevert.ts` | 2 | **0** | | repo backlog | 17 | **15** | ## Measured - 4 new cases; `taskRevert.test.ts` **11/11 pass**. - **MUTATION**: restoring the literal pair fails the renamed case. - **The negative is load-bearing.** The map is fail-soft, so a candidate it does not cover must still be treated as **open**, not skipped. A conversion that skipped unknown candidates would *hide live undo links* — failing in the direction nobody reports. - A **control** pins that an unwired caller (no flags at all) still skips the legacy ids, so the optional parameter cannot regress default boards. - `TaskDetailModal` suites — **31 files / 664 tests pass**. - `tsc --noEmit -p tsconfig.app.json` clean; census `--strict`, `check-lane-wiring`, `check-fnxc-future-dates` clean. ## Pattern worth noting This is the fourth deferral this session whose stated blocker had dissolved or misidentified itself, and the second where the counter-example was already in the same file. The common shape: a note records *why* something is blocked, is accurate when written, and is never re-checked — so the block outlives its cause. Re-reading them cost minutes each and returned two real conversions. Co-authored-by: Claude Opus 5 (1M context) --- .../app/components/TaskDetailModal.tsx | 4 +- .../app/utils/__tests__/taskRevert.test.ts | 54 ++++++++++++++++++- packages/dashboard/app/utils/taskRevert.ts | 53 ++++++++++-------- .../lib/lifecycle-column-census-baseline.json | 3 -- 4 files changed, 88 insertions(+), 26 deletions(-) diff --git a/packages/dashboard/app/components/TaskDetailModal.tsx b/packages/dashboard/app/components/TaskDetailModal.tsx index d953c94c4c..6b95e0e77e 100644 --- a/packages/dashboard/app/components/TaskDetailModal.tsx +++ b/packages/dashboard/app/components/TaskDetailModal.tsx @@ -989,7 +989,9 @@ export function TaskDetailContent({ * (open board columns only) so a done/archived/soft-deleted prior undo attempt never renders as * an active "Undo task" link — that would be a stale/leftover affordance. */ - const openUndoTask = findOpenUndoTaskForSource(tasks, workingTask.id); + /* FNXC:WorkflowResolvedColumns 2026-07-31-23:20: the CANDIDATES' own flags, keyed by id — the same + per-neighbour supply this component already uses for the near-duplicate canonical above. */ + const openUndoTask = findOpenUndoTaskForSource(tasks, workingTask.id, columnFlagsByTaskId); const previousInitialTabRef = useRef(initialTab); const taskColumnRef = useRef(task.column); diff --git a/packages/dashboard/app/utils/__tests__/taskRevert.test.ts b/packages/dashboard/app/utils/__tests__/taskRevert.test.ts index b38cc7dad0..88d623f64f 100644 --- a/packages/dashboard/app/utils/__tests__/taskRevert.test.ts +++ b/packages/dashboard/app/utils/__tests__/taskRevert.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import type { Task } from "@fusion/core"; -import { isTaskReverted } from "../taskRevert"; +import { isTaskReverted, findOpenUndoTaskForSource } from "../taskRevert"; describe("isTaskReverted", () => { it.each([ @@ -15,3 +15,55 @@ describe("isTaskReverted", () => { expect(isTaskReverted(sourceMetadata as Task["sourceMetadata"] | undefined)).toBe(expected); }); }); + +/* +FNXC:WorkflowResolvedColumns 2026-07-31-23:30: +THE UNDO-TASK LOOKUP CLASSIFIED A NEIGHBOUR'S COLUMN BY ID. + +`findOpenUndoTaskForSource` skips candidates that are finished, so a done/archived prior undo attempt +never renders as an active "Undo task" link. Keyed on `done`/`archived`, a board that renames those +lanes matched neither: a FINISHED undo task kept rendering as an open one, which is the stale +affordance the function's own header says it exists to prevent. + +The flags are PER-CANDIDATE, keyed by task id. That is what makes this correct and what two earlier +notes said was unavailable — `TaskDetailModal` has had `columnFlagsByTaskId` as a prop all along and +already uses it this way for the near-duplicate canonical. + +Both directions are asserted, and the negative is the load-bearing one: the map is fail-soft, so a +candidate it does not cover must still be treated as OPEN rather than silently skipped. A conversion +that skipped unknown candidates would hide live undo links. +*/ +describe("findOpenUndoTaskForSource resolves each candidate's own lanes", () => { + const candidate = (id: string, column: string, createdAt: string): Task => ({ + id, column, title: id, description: "", createdAt, updatedAt: createdAt, + dependencies: [], steps: [], sourceMetadata: { revertOf: "KB-SRC" }, + } as unknown as Task); + + it("skips an undo task resting in a RENAMED complete lane", () => { + const tasks = [candidate("KB-UNDO", "shipped", "2026-06-01T00:00:00.000Z")]; + const flags = new Map([["KB-UNDO", { complete: true }]]); + + expect(findOpenUndoTaskForSource(tasks, "KB-SRC", flags as never)).toBeUndefined(); + }); + + it("still returns an undo task resting in a live lane on that same board", () => { + const tasks = [candidate("KB-UNDO", "building", "2026-06-01T00:00:00.000Z")]; + const flags = new Map([["KB-UNDO", { countsTowardWip: true }]]); + + expect(findOpenUndoTaskForSource(tasks, "KB-SRC", flags as never)?.id).toBe("KB-UNDO"); + }); + + it("treats a candidate the map does not cover as OPEN, not skipped", () => { + /* Fail-soft in the safe direction: an unknown candidate keeps its link rather than losing it. */ + const tasks = [candidate("KB-UNDO", "building", "2026-06-01T00:00:00.000Z")]; + + expect(findOpenUndoTaskForSource(tasks, "KB-SRC", new Map() as never)?.id).toBe("KB-UNDO"); + }); + + it("still skips the legacy ids when no flags are supplied at all", () => { + /* CONTROL: the parameter is optional, so an unwired caller behaves exactly as before. */ + const tasks = [candidate("KB-UNDO", "done", "2026-06-01T00:00:00.000Z")]; + + expect(findOpenUndoTaskForSource(tasks, "KB-SRC")).toBeUndefined(); + }); +}); diff --git a/packages/dashboard/app/utils/taskRevert.ts b/packages/dashboard/app/utils/taskRevert.ts index 957eba9ce2..360f8da670 100644 --- a/packages/dashboard/app/utils/taskRevert.ts +++ b/packages/dashboard/app/utils/taskRevert.ts @@ -1,4 +1,5 @@ import type { Task } from "@fusion/core"; +import { isTerminalColumnRole, type ColumnRoleTraitFlags } from "@fusion/core"; /** * FNXC:TaskRevert 2026-07-04-00:00: @@ -72,7 +73,17 @@ behaviour. This searches for an OPEN undo task, so a finished one must be skippe literals, a renamed board never skipped anything: a completed undo task counted as still open, and the UI offered to resume work that had already landed. */ -export function findOpenUndoTaskForSource(tasks: readonly Task[], sourceTaskId: string): Task | undefined { +export function findOpenUndoTaskForSource( + tasks: readonly Task[], + sourceTaskId: string, + /* + FNXC:WorkflowResolvedColumns 2026-07-31-23:20: + PER-NEIGHBOUR flags, keyed by task id — the thing the note below said did not exist. Optional and + fail-soft: a candidate the map does not cover yields `undefined` and the role helper falls back to + the legacy ids, which is the documented degraded answer rather than a fabricated one. + */ + flagsByTaskId?: ReadonlyMap, +): Task | undefined { const trimmedSourceId = sourceTaskId.trim(); if (trimmedSourceId.length === 0) { return undefined; @@ -88,35 +99,35 @@ export function findOpenUndoTaskForSource(tasks: readonly Task[], sourceTaskId: STILL A LITERAL, deliberately, and left counted. I converted this and added a `columnFlags` parameter — SINCE REMOVED, so this function takes only - `(tasks, sourceTaskId)` today. Its only caller is TaskDetailModal ~line - 926, which sits ~60 lines ABOVE where `detailColumnFlags` is derived, so it could not supply one. - The parameter was therefore never passed: the guard was gone, the census counted a conversion, - and the behaviour was the legacy fallback forever. + `(tasks, sourceTaskId)` today. Its only caller is TaskDetailModal ~line 926, which sits ~60 lines + ABOVE where `detailColumnFlags` is derived, so it could not supply one. The parameter was therefore + never passed: the guard was gone, the census counted a conversion, and the behaviour was the legacy + fallback forever. Reverted rather than left as a dead seam. An unsupplied optional parameter is strictly worse than the literal — the literal is at least honest, and the census keeps pointing here. FNXC:WorkflowResolvedColumns 2026-07-30-20:50 (correcting the unblock recorded above): - HOISTING THE FLAGS WOULD NOT UNBLOCK THIS — IT WOULD INTRODUCE A WORSE DEFECT. + HOISTING THE FLAGS WOULD NOT UNBLOCK THIS — the column classified here belongs to a NEIGHBOUR, and + `detailColumnFlags` describes the MODAL'S OWN task. Supplying it would answer "is this neighbour + finished?" with a different row's traits — wrong on data, not merely stale on vocabulary. - The note above says the blocker is hook ordering, i.e. a cost. It is not: it is a correctness - boundary. This function scans the `tasks` list for OTHER tasks pointing back at the source, so the - column it classifies belongs to a NEIGHBOUR. `detailColumnFlags` in TaskDetailModal describes the - MODAL'S OWN task, and its own FNXC note says so explicitly — it is guarded by - `detailFlagsAreForThisTask` precisely because using it for anything else is wrong. + FNXC:WorkflowResolvedColumns 2026-07-31-23:20 (CONVERTED — the blocker named the wrong variable): + Both notes above are right that `detailColumnFlags` is the wrong supplier. The conclusion drawn + from that — "the modal does not have per-neighbour flags and should not fetch mid-render" — is + false, and the counter-example is in the same component. - So supplying it here would answer "is this neighbour finished?" with the modal task's traits: on a - project where two workflows reuse a column id, an open undo task would be classified by a workflow - it does not belong to and the affordance would vanish or persist wrongly. That is the flags-for- - the-wrong-row shape, and it is worse than the literal because it is wrong on data rather than - merely stale on vocabulary. + `columnFlagsByTaskId` is a per-task map, already a prop of TaskDetailModal (declared :367, + destructured :727), and the call site at :992 is BELOW that destructure. TaskDetailModal itself + already uses it exactly this way for the near-duplicate canonical + (`columnFlagsByTaskId?.get(nearDuplicateCanonical.id)`), under a note making the same point: the + blocker there had been "asserted from the shape of the problem rather than tested against what was + in scope". This is the same assertion, one function over. - A CORRECT conversion needs per-NEIGHBOUR flags — the caller would have to resolve each candidate's - own workflow, which the modal does not have and should not fetch mid-render. Until a per-task lane - map is available at that call site, the literal is the right answer and the census entry is - accurate debt rather than a missed conversion. + So the supplier the 22:40 note went looking for exists, it is per-neighbour, and it needs no fetch. + The parameter is supplied at the only call site in the same commit, so this is not a dead seam. */ - if (candidate.column === "done" || candidate.column === "archived") { + if (isTerminalColumnRole(flagsByTaskId?.get(candidate.id), candidate.column)) { continue; } if (getRevertOfId(candidate.sourceMetadata) !== trimmedSourceId) { diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index 6e4430919a..86cc10b14c 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -1,7 +1,6 @@ { "generatedFrom": "node scripts/lifecycle-column-census.mjs --strict --update-baseline", "byFile": { - "packages/dashboard/app/utils/taskRevert.ts": 2, "packages/engine/src/scheduler.ts": 2, "packages/core/src/mission-store.ts": 1, "packages/core/src/task-store/audit-ops.ts": 1, @@ -11,7 +10,6 @@ "packages/core/src/task-store/task-id-integrity.ts": 1, "packages/dashboard/app/components/ResearchTaskActionModal.tsx": 1, "packages/dashboard/app/components/TaskCard.tsx": 1, - "packages/engine/src/auto-merge-finalization.ts": 1, "packages/engine/src/notification/notification-service.ts": 1, "packages/engine/src/self-healing.ts": 1, "packages/engine/src/triage.ts": 1 @@ -110,7 +108,6 @@ "packages/dashboard/src/test/mockCoreEngine.ts\u0000in-review": 1, "packages/engine/src/agent-heartbeat.ts\u0000archived": 1, "packages/engine/src/agent-heartbeat.ts\u0000done": 1, - "packages/engine/src/auto-merge-finalization.ts\u0000done": 1, "packages/engine/src/cli-agent/task-session.ts\u0000done": 1, "packages/engine/src/executor.ts\u0000in-progress": 1, "packages/engine/src/hold-release.ts\u0000archived": 1,