From faf4245c7babb14a99504b22485c93d63479d012 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 23:54:35 -0700 Subject: [PATCH] fix(dashboard): the duplicate chip kept pointing at work that had already landed (#2997) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second live defect from the same sweep as #2996 — memoized hooks reading a lane value absent from their dependency list. **13 hits, triaged by hand, 2 real.** ## The defect `resolveNearDuplicateCanonicalInactive` decides whether a card's *"duplicate of X"* chip is hidden because the canonical is finished. It calls `getTaskColumnFlags`, whose identity changes when the board's workflow traits arrive — while its own dependency list was `[allTasks]` alone. So it kept the closure created during the **pre-load** render, over an empty trait map. With no traits the role helpers fall back to legacy ids, so a canonical sitting in a renamed complete lane reads as still **active**, and the chip stays up advertising a duplicate of work that has shipped. ## Severity, stated honestly The closure is rebuilt whenever `allTasks` changes identity, which any task-list refresh does. So this is a **bounded window**, not a permanent wrong answer — unlike #2996, where the dependency that would have refreshed it (`task.column`) never changes. On a quiet board the window is the gap until the next update. I'd rather say that plainly than let it read as equally severe because it's in the same family. ## The hoist is required by the fix, not tidying `getTaskColumnFlags` sat *after* this callback, with a note explaining that the body only runs during render so the const is initialised by then. That's true of the **body** and false of the **dependency array**, which evaluates eagerly — so the reference could not be listed at all until the declaration moved. The existing note reasoned carefully about declaration order and said nothing about staleness, which is exactly how it read as considered. Both hoisted callbacks close over props only, so the move carries no behaviour. ## Measured | check | result | |---|---| | test written first | red for the right reason — arrival case `expected false to be true`, negative passed | | after the fix | 2 passed | | **dropping the dep while keeping the hoist** | arrival case fails again | | `Column.test` + new suite | **87 tests green** | | gates | census + FNXC green; lint and `tsc` clean | That third row is the one that matters: it isolates the test as load-bearing on the **dependency**, not on the code move that had to accompany it. The observable is the prop `Column` computes, not the chip markup — `Column` is the producer here, and asserting on `TaskCard`'s rendering would test the consumer of a value this component gets wrong. ## The negative case Re-resolving must not degrade into "every canonical is inactive". A canonical still in a live lane keeps its chip, or the fix silently hides **real** duplicate warnings — worse than a stale one, because then nothing points at the collision at all. Co-authored-by: Claude Opus 5 (1M context) --- .changeset/column-neardup-flags-arrival.md | 7 ++ packages/dashboard/app/components/Column.tsx | 32 +++-- .../Column.neardup-flags-arrival.test.tsx | 115 ++++++++++++++++++ 3 files changed, 144 insertions(+), 10 deletions(-) create mode 100644 .changeset/column-neardup-flags-arrival.md create mode 100644 packages/dashboard/app/components/__tests__/Column.neardup-flags-arrival.test.tsx diff --git a/.changeset/column-neardup-flags-arrival.md b/.changeset/column-neardup-flags-arrival.md new file mode 100644 index 0000000000..0c51bdc4e8 --- /dev/null +++ b/.changeset/column-neardup-flags-arrival.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: The duplicate chip now clears once its canonical task lands on a board with renamed lanes. +category: fix +dev: `resolveNearDuplicateCanonicalInactive` kept the pre-load `getTaskColumnFlags` closure; the callback is hoisted so the dependency can be listed. diff --git a/packages/dashboard/app/components/Column.tsx b/packages/dashboard/app/components/Column.tsx index a44dbac19d..08408e2f4d 100644 --- a/packages/dashboard/app/components/Column.tsx +++ b/packages/dashboard/app/components/Column.tsx @@ -277,17 +277,35 @@ function ColumnComponent({ column, tasks, projectId, maxConcurrent, showWorktree const menuRef = useRef(null); const countFlashing = useFlashOnIncrease(tasks.length); const { confirm } = useConfirm(); + const getTaskContextMenuColumns = useCallback((task: Task) => ( + taskContextMenuColumnsByTaskId?.get(task.id) ?? workflowContextMenuColumns + ), [taskContextMenuColumnsByTaskId, workflowContextMenuColumns]); + const getTaskColumnFlags = useCallback((task: Task) => ( + getTaskContextMenuColumns(task)?.find((candidate) => candidate.id === task.column)?.flags ?? (task.column === column ? columnFlags : undefined) + ), [column, columnFlags, getTaskContextMenuColumns]); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-23:55: + HOISTED so `getTaskColumnFlags` can be a DEPENDENCY below, not merely a closed-over value. + + It previously sat after this callback, with a note observing that the body only runs during render + so the const is initialised by then. That is true of the BODY and false of the dependency array, + which evaluates eagerly — so the reference could not be listed, and the callback silently kept the + closure built during the PRE-LOAD render, over an empty trait map. The note reasoned about + declaration order and nothing about staleness, which is how it read as considered. + + Both callbacks close over props only, so the move is mechanical: no behaviour rides on it beyond + making the dependency expressible. + */ const resolveNearDuplicateCanonicalInactive = useCallback((task: Task): boolean | undefined => { const nearDuplicateOf = task.sourceMetadata?.nearDuplicateOf; if (typeof nearDuplicateOf !== "string" || !allTasks) { return undefined; } const canonical = allTasks.find((candidate) => candidate.id === nearDuplicateOf); - /* FNXC:WorkflowResolvedColumns 2026-07-30-01:10: the canonical's own flags. Declared above - `getTaskColumnFlags` in source order, but this body only runs during render, so the const is - initialised by then — tsc and the suite both confirm it. */ + /* The canonical's OWN flags — a different task from the card being rendered, so this must not + reuse the row's flags. */ return isNearDuplicateCanonicalInactive(canonical, canonical ? getTaskColumnFlags(canonical) : undefined); - }, [allTasks]); + }, [allTasks, getTaskColumnFlags]); // Clear the inline capacity-exhausted banner once the column's task list // changes via SSE (e.g. an occupant moves out and capacity frees up). The @@ -337,12 +355,6 @@ function ColumnComponent({ column, tasks, projectId, maxConcurrent, showWorktree const isHoldColumn = isHoldColumnRole(columnFlags, column); const isCollapsed = isArchived && collapsed; const isWipProcessingColumn = isWipColumnRole(columnFlags, column); - const getTaskContextMenuColumns = useCallback((task: Task) => ( - taskContextMenuColumnsByTaskId?.get(task.id) ?? workflowContextMenuColumns - ), [taskContextMenuColumnsByTaskId, workflowContextMenuColumns]); - const getTaskColumnFlags = useCallback((task: Task) => ( - getTaskContextMenuColumns(task)?.find((candidate) => candidate.id === task.column)?.flags ?? (task.column === column ? columnFlags : undefined) - ), [column, columnFlags, getTaskContextMenuColumns]); /* FNXC:WorktreeGroupingSetting 2026-06-27-22:30: The project setting is an explicit show/hide control: worktree grouping and labels render only when enabled and only for the board's WIP/processing column. Turning it off must leave plain task cards with no legacy group shell in either legacy or workflow-mode columns. diff --git a/packages/dashboard/app/components/__tests__/Column.neardup-flags-arrival.test.tsx b/packages/dashboard/app/components/__tests__/Column.neardup-flags-arrival.test.tsx new file mode 100644 index 0000000000..5ee6fbf093 --- /dev/null +++ b/packages/dashboard/app/components/__tests__/Column.neardup-flags-arrival.test.tsx @@ -0,0 +1,115 @@ +/* +FNXC:WorkflowResolvedColumns 2026-07-30-23:50: +THE NEAR-DUPLICATE CHIP KEPT POINTING AT WORK THAT HAD ALREADY LANDED. + +`resolveNearDuplicateCanonicalInactive` decides whether a card's "duplicate of X" chip is hidden +because the canonical is finished. It calls `getTaskColumnFlags`, whose identity changes when the +board's workflow traits arrive — but its own dependency list was `[allTasks]` alone, so it kept the +closure created during the PRE-LOAD render, over an empty trait map. + +With no traits the role helpers fall back to the legacy ids, so a canonical sitting in a renamed +complete lane reads as still ACTIVE and the chip stays up, advertising a duplicate of work that has +shipped. + +SEVERITY, STATED HONESTLY: the stale closure is rebuilt whenever `allTasks` changes identity, which +any task-list refresh does — so this is a bounded window rather than a permanent wrong answer, unlike +the TaskCard ticker defect (#2996) where the dependency that would have refreshed it never changes. +On a quiet board the window is the gap until the next update. + +THE OBSERVABLE IS THE PROP COLUMN COMPUTES, not the chip markup: `Column` is the producer here, and +asserting on TaskCard's rendering would test the consumer of a value this component gets wrong. + +Found by the same sweep as #2996 — memoized hooks reading a lane value absent from their deps. That +sweep's 13 hits were triaged by hand; this is the second of two live defects, and the comment already +at this call site reasons carefully about declaration ORDER while saying nothing about staleness, +which is how it read as considered. +*/ + +import React from "react"; +import { describe, it, expect, vi } from "vitest"; +import { render } from "@testing-library/react"; +import type { Task, Column as ColumnType } from "@fusion/core"; +import { Column } from "../Column"; + +/* Capture the computed prop rather than the chip: this is the value Column is responsible for. */ +const seen: (boolean | undefined)[] = []; +vi.mock("../TaskCard", () => ({ + TaskCard: ({ nearDuplicateCanonicalInactive }: { nearDuplicateCanonicalInactive?: boolean }) => { + seen.push(nearDuplicateCanonicalInactive); + return
; + }, +})); +vi.mock("../WorktreeGroup", () => ({ WorktreeGroup: () =>
})); +vi.mock("../QuickEntryBox", () => ({ QuickEntryBox: () =>
})); + +const BASE = { description: "t", createdAt: "2026-06-01T00:00:00.000Z", updatedAt: "2026-06-01T00:00:00.000Z", steps: [] }; + +const duplicate = { + id: "KB-DUP", title: "the duplicate", column: "drafting", + sourceMetadata: { nearDuplicateOf: "KB-CANON" }, ...BASE, +} as unknown as Task; + +/* The canonical has LANDED, in a lane called `shipped` rather than `done`. */ +const canonical = { id: "KB-CANON", title: "the canonical", column: "shipped", ...BASE } as unknown as Task; + +/* One stable array identity across both renders: if this changed, the callback would be rebuilt for + an unrelated reason and the test would pass without the fix. */ +const allTasks = [duplicate, canonical]; + +const props = { + column: "drafting" as ColumnType, + maxConcurrent: 2, + showWorktreeGrouping: false, + onMoveTask: vi.fn().mockResolvedValue({} as Task), + onOpenDetail: vi.fn(), + addToast: vi.fn(), + tasks: [duplicate], + allTasks, +}; + +/** The traits the board resolves once its workflow fetch lands. */ +const arrivedTraits = new Map([ + ["KB-CANON", [{ id: "shipped", label: "Shipped", flags: { complete: true } }]], + ["KB-DUP", [{ id: "drafting", label: "Drafting", flags: { hold: true, intake: true } }]], +]); + +describe("the near-duplicate canonical check when column traits arrive after first paint", () => { + it("re-resolves the canonical once traits arrive, without a task-list change", () => { + seen.length = 0; + const { rerender } = render(); + + /* Pre-load: no traits, so the legacy fallback cannot see `shipped` as terminal. Correct for + what it knows — the canonical genuinely has not been proven inactive yet. */ + expect(seen[seen.length - 1]).not.toBe(true); + + rerender(); + + /* The traits now prove the canonical landed, so the chip must be suppressed. */ + expect(seen[seen.length - 1]).toBe(true); + }); + + /* + The paired negative: re-resolving must not degrade into "every canonical is inactive". A canonical + still in a live lane must keep the chip up, or the fix silently hides real duplicate warnings — + which is worse than showing a stale one, because nothing then points at the collision at all. + */ + it("a canonical still in a live lane keeps the chip", () => { + seen.length = 0; + const liveTraits = new Map([ + ["KB-CANON", [{ id: "building", label: "Building", flags: { countsTowardWip: true } }]], + ["KB-DUP", [{ id: "drafting", label: "Drafting", flags: { hold: true, intake: true } }]], + ]); + const liveCanonical = { ...canonical, column: "building" } as Task; + const liveAll = [duplicate, liveCanonical]; + + render( + , + ); + + expect(seen[seen.length - 1]).not.toBe(true); + }); +});