fleet(dashboard): taskRevert 2 → 0 — the recorded blocker named the wrong variable (#3129)
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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<TabId | undefined>(initialTab);
|
||||
const taskColumnRef = useRef(task.column);
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string, ColumnRoleTraitFlags>,
|
||||
): 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) {
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user