fix(dashboard): archived tasks stayed in the research picker on a renamed board (#3215)
## The defect The enrich-mode task picker filtered with `task.column !== "archived"`. On a board whose archive lane is renamed, that matched nothing — so filed-away tasks stayed in the picker and an operator could attach research findings to work they had deliberately archived. ## Census before / after | | before | after | |---|---|---| | COLUMN guards (backlog) | 10 | **9** | | `ResearchTaskActionModal.tsx` | 1 | **0 — converted** | Baseline re-recorded in the same commit; `--strict` green. ## This site was declined twice, and I wrote the second wrong estimate #3213 left it counted, correctly, on the note that was here — which was mine. Both prior cost estimates were wrong, so this corrects my own work: 1. **"Needs a data-fetch change"** — reasoned about `columnFlagsByTaskId`, a per-**task** map built from board-resident rows. Right that such a map can't help (archived rows are exactly what a board map omits), but this guard asks a per-**column** question, so it never needed one. 2. **"Needs prop threading, MainContent → ResearchView → here"** — right that the answer is column-keyed, wrong about where it lives. `ListView` builds `columnFlagsById` *inline*, which made it look like the owner. The data is `useBoardWorkflows`, a hook already called from `App`, `Board`, and `HeaderWorkflowSwitcherSlot`. **Measured cost: one file.** The modal already takes `projectId`, and `ResearchView` renders it only when a finding is open (`open` hardcoded beside `if (!finding) return null`) — so the hook cannot fetch for a closed modal, which was the one real objection to calling it here. Union across workflows keyed by column id, first declaration wins — the same convention `ListView` uses, so the two cannot disagree about a shared id. `isArchivedColumnRole` fail-softs to the legacy id when a column has no flags, so an unresolved workflow behaves exactly as the literal did. ## Tests — the invariant, not the repro Per the surface-enumeration rule, four cases: renamed archive lane, legacy id, unresolved workflow (fail-soft), and a second workflow's archive lane through the cross-workflow union. A repro-only test would pass on the legacy board and prove nothing about the case the guard exists for. **Anti-vacuity control:** | | renamed lane | union | legacy id | fail-soft | |---|---|---|---|---| | pre-fix literal | **FAIL** | **FAIL** | pass | pass | | converted | pass | pass | pass | pass | The legacy and fail-soft cases hold in both directions **on purpose** — they pin that this conversion did not change the pre-resolution answer. Flagging that so 4/4 isn't read as four independent proofs. ## Measured | check | result | |---|---| | `census --strict` / `check-fnxc-future-dates` | exit 0 / exit 0 | | `eslint` | clean | | `tsc -p tsconfig.app.json` (the config that actually covers `app/`) | exit 0 | | new tests | 4/4 | | `pnpm test:gate` | exit 0 (744 tests) | ## Note on process My first attempt at the control silently did nothing — the revert script threw a `SyntaxError`, so the "pre-fix" run was the fixed code and reported 4/4. Caught it because the error printed. The table above is from the re-run.
This commit is contained in:
@@ -0,0 +1,109 @@
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-11:55 (u12 — the last convertible census guard in this file):
|
||||
The enrich-mode task picker filtered archived rows with `task.column !== "archived"`. On a board whose
|
||||
archive lane is renamed, that matched nothing, so filed-away tasks stayed in the picker and an operator
|
||||
could attach research findings to work they had deliberately archived.
|
||||
|
||||
Asserted as the INVARIANT rather than the single repro, per the surface-enumeration rule: a renamed
|
||||
archive lane, the legacy id, an unresolved workflow (fail-soft), and a second workflow's archive lane
|
||||
in the cross-workflow union. A repro-only test here would pass on the legacy board and prove nothing
|
||||
about the case the guard exists for.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { render, waitFor } from "@testing-library/react";
|
||||
import type { Task } from "@fusion/core";
|
||||
|
||||
import { ResearchTaskActionModal } from "../components/ResearchTaskActionModal";
|
||||
import { fetchTasks } from "../api";
|
||||
import { useBoardWorkflows } from "../hooks/useBoardWorkflows";
|
||||
|
||||
vi.mock("../api", () => ({ fetchTasks: vi.fn() }));
|
||||
vi.mock("../hooks/useBoardWorkflows", () => ({ useBoardWorkflows: vi.fn() }));
|
||||
vi.mock("../hooks/useMobileScrollLock", () => ({ useMobileScrollLock: vi.fn() }));
|
||||
vi.mock("react-i18next", () => ({
|
||||
useTranslation: () => ({ t: (_key: string, fallback?: string) => fallback ?? _key }),
|
||||
}));
|
||||
|
||||
const task = (id: string, column: string): Task => ({
|
||||
id, title: `task ${id}`, description: "", column,
|
||||
dependencies: [], steps: [], currentStep: 0, log: [],
|
||||
createdAt: "2026-01-01T00:00:00Z", updatedAt: "2026-01-01T00:00:00Z",
|
||||
} as unknown as Task);
|
||||
|
||||
function workflows(columns: { id: string; flags: Record<string, boolean> }[][]) {
|
||||
return {
|
||||
boardWorkflows: {
|
||||
defaultWorkflowId: "wf0",
|
||||
workflows: columns.map((cols, i) => ({ id: `wf${i}`, name: `wf${i}`, columns: cols })),
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
/*
|
||||
The kept tasks render into a <datalist>, whose <option>s jsdom does not expose as role="option" —
|
||||
so this reads the datalist's DOM directly. That list IS the filter's output.
|
||||
*/
|
||||
async function renderPickerTaskIds(): Promise<string[]> {
|
||||
const { container } = render(
|
||||
<ResearchTaskActionModal
|
||||
open
|
||||
mode="enrich"
|
||||
run={{ title: "run" } as never}
|
||||
finding={{ id: "f1", heading: "h", content: "c" }}
|
||||
projectId="p1"
|
||||
onClose={() => {}}
|
||||
onConfirm={async () => {}}
|
||||
/>,
|
||||
);
|
||||
await waitFor(() => expect(fetchTasks).toHaveBeenCalled());
|
||||
await waitFor(() => {
|
||||
expect(container.querySelector("#research-task-action-task-list")).not.toBeNull();
|
||||
});
|
||||
const list = container.querySelector("#research-task-action-task-list");
|
||||
await waitFor(() => expect(list!.querySelectorAll("option").length).toBeGreaterThan(0));
|
||||
return [...list!.querySelectorAll("option")].map((o) => (o as HTMLOptionElement).value);
|
||||
}
|
||||
|
||||
describe("the research picker hides archived tasks on ANY board, not just the legacy one", () => {
|
||||
beforeEach(() => vi.clearAllMocks());
|
||||
|
||||
it("hides a task resting in a RENAMED archive lane", async () => {
|
||||
vi.mocked(fetchTasks).mockResolvedValue([task("FN-1", "building"), task("FN-2", "cold")] as never);
|
||||
vi.mocked(useBoardWorkflows).mockReturnValue(workflows([[
|
||||
{ id: "building", flags: { archived: false } },
|
||||
{ id: "cold", flags: { archived: true } },
|
||||
]]) as never);
|
||||
|
||||
// Before the fix this returned both: "cold" !== "archived", so the archived row survived.
|
||||
expect(await renderPickerTaskIds()).toEqual(["FN-1"]);
|
||||
});
|
||||
|
||||
it("still hides the LEGACY archived id when the workflow declares it", async () => {
|
||||
vi.mocked(fetchTasks).mockResolvedValue([task("FN-1", "building"), task("FN-2", "archived")] as never);
|
||||
vi.mocked(useBoardWorkflows).mockReturnValue(workflows([[
|
||||
{ id: "building", flags: { archived: false } },
|
||||
{ id: "archived", flags: { archived: true } },
|
||||
]]) as never);
|
||||
|
||||
expect(await renderPickerTaskIds()).toEqual(["FN-1"]);
|
||||
});
|
||||
|
||||
it("FAILS SOFT to the legacy id when no workflow resolved", async () => {
|
||||
// The pre-resolution answer. An unresolved board must behave exactly as the old literal did,
|
||||
// rather than showing archived rows because the flags map is empty.
|
||||
vi.mocked(fetchTasks).mockResolvedValue([task("FN-1", "building"), task("FN-2", "archived")] as never);
|
||||
vi.mocked(useBoardWorkflows).mockReturnValue({ boardWorkflows: null } as never);
|
||||
|
||||
expect(await renderPickerTaskIds()).toEqual(["FN-1"]);
|
||||
});
|
||||
|
||||
it("honours a SECOND workflow's archive lane through the cross-workflow union", async () => {
|
||||
vi.mocked(fetchTasks).mockResolvedValue([task("FN-1", "building"), task("FN-2", "retired")] as never);
|
||||
vi.mocked(useBoardWorkflows).mockReturnValue(workflows([
|
||||
[{ id: "building", flags: { archived: false } }],
|
||||
[{ id: "retired", flags: { archived: true } }],
|
||||
]) as never);
|
||||
|
||||
expect(await renderPickerTaskIds()).toEqual(["FN-1"]);
|
||||
});
|
||||
});
|
||||
@@ -2,7 +2,9 @@ import { useEffect, useMemo, useState } from "react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import type { Task, TaskPriority } from "@fusion/core";
|
||||
import { fetchTasks } from "../api";
|
||||
import { useBoardWorkflows } from "../hooks/useBoardWorkflows";
|
||||
import { useMobileScrollLock } from "../hooks/useMobileScrollLock";
|
||||
import { isArchivedColumnRole } from "../utils/columnRoles";
|
||||
import type { ResearchRunDetail } from "../research-types";
|
||||
import "./ResearchTaskActionModal.css";
|
||||
|
||||
@@ -30,6 +32,34 @@ export function ResearchTaskActionModal({ open, mode, run, finding, projectId, o
|
||||
const [loadingTasks, setLoadingTasks] = useState(false);
|
||||
const [saving, setSaving] = useState(false);
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-11:40 (u12 — CONVERTED, and the earlier cost estimate was wrong):
|
||||
The note this replaces said converting the filter below needed prop threading through three
|
||||
components (MainContent -> ResearchView -> here) because `ListView.tsx:756` builds `columnFlagsById`
|
||||
locally. That was wrong about WHERE the data lives: `listColumns` derives from `useBoardWorkflows`,
|
||||
a hook already called from App, Board and HeaderWorkflowSwitcherSlot. ListView only looked like the
|
||||
owner because it happens to build the map inline.
|
||||
|
||||
So the real cost is this file, and nothing else. The modal already takes `projectId`, and
|
||||
ResearchView renders it ONLY when a finding is open (`open` is hardcoded true beside a
|
||||
`if (!finding) return null`), so the hook cannot fetch for a closed modal.
|
||||
|
||||
Union across workflows keyed by column id, first declaration wins — the same convention
|
||||
`ListView.tsx` uses for its cross-workflow map, so the two cannot disagree about a shared id.
|
||||
*/
|
||||
const { boardWorkflows } = useBoardWorkflows({ projectId });
|
||||
const isArchivedColumn = useMemo(() => {
|
||||
const flagsById = new Map<string, { archived?: boolean }>();
|
||||
for (const workflow of boardWorkflows?.workflows ?? []) {
|
||||
for (const column of workflow.columns) {
|
||||
if (!flagsById.has(column.id)) flagsById.set(column.id, column.flags);
|
||||
}
|
||||
}
|
||||
/* `isArchivedColumnRole` fail-softs to the legacy id when a column has no flags, which is the
|
||||
pre-resolution answer — so an unresolved workflow behaves exactly as this filter did before. */
|
||||
return (column: string): boolean => isArchivedColumnRole(flagsById.get(column), column);
|
||||
}, [boardWorkflows]);
|
||||
|
||||
const preview = useMemo(() => {
|
||||
const firstSentence = (finding.content ?? "").split(/(?<=[.!?])\s+/)[0] ?? "";
|
||||
return `${finding.heading || t("research.defaultFindingHeading", "Research finding")} — ${firstSentence}`.trim();
|
||||
@@ -47,49 +77,24 @@ export function ResearchTaskActionModal({ open, mode, run, finding, projectId, o
|
||||
setLoadingTasks(true);
|
||||
void fetchTasks(50, 0, projectId)
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-20:10 (batch-dashboard-app — SIZED, NOT CONVERTED):
|
||||
STILL A LITERAL, and threading the board's flags map here would be the WRONG fix.
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-11:45 (u12 — the history, kept short because it is CONVERTED now):
|
||||
This guard was sized twice and declined twice, each time on a cost that turned out to be wrong.
|
||||
Recorded because both wrong answers are instructive, not to relitigate them:
|
||||
|
||||
The guard is real: on a renamed board `archived` matches nothing, so filed-away tasks stay in
|
||||
this picker and an operator can attach research findings to work they deliberately archived.
|
||||
1. "Needs a data-fetch change" — reasoning about `columnFlagsByTaskId`, a per-TASK map built
|
||||
from board-resident rows. Correct that such a map cannot help (archived rows are exactly
|
||||
what a board map omits), but this guard asks a per-COLUMN question, so it never needed one.
|
||||
2. "Needs prop threading, MainContent -> ResearchView -> here" — correct that the answer is
|
||||
column-keyed, wrong about where it lives. `ListView` builds `columnFlagsById` inline, which
|
||||
made it look like the owner; the data is `useBoardWorkflows`, callable from here directly.
|
||||
|
||||
But this modal fetches its OWN page (`fetchTasks(50, 0, projectId)`), which is not the board's
|
||||
task set. The obvious move — thread `columnFlagsByTaskId` down from MainContent through
|
||||
ResearchView — resolves only rows that happen to be board-resident, and the rows THIS filter
|
||||
cares about are archived ones, which are exactly the rows a board-built map does not contain.
|
||||
It would look converted, drop the guard count, and leave the case it exists for unresolved.
|
||||
|
||||
The honest fix is for this modal to resolve lanes for the page it fetched — either a
|
||||
`fetchTasks` variant that returns resolved flags, or a per-task resolution over the 50 rows.
|
||||
That is a data-fetch change, not a prop-threading one, so it is sized here rather than faked.
|
||||
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-23:50 (CORRECTING THE SHAPE — it is not a data-fetch
|
||||
change, and the objection above applies to a map this guard does not need):
|
||||
Everything above is about a per-TASK map (`columnFlagsByTaskId`), and it is right that one
|
||||
cannot help here: it is built from board-resident rows, and the rows this filter cares about
|
||||
are archived ones, which are exactly what a board map omits.
|
||||
|
||||
But this guard does not ask a per-task question. "Is `task.column` an archive lane" is a
|
||||
question about a COLUMN, and the answer lives in the workflow definition — a lane exists there
|
||||
whether or not any board row currently sits in it. The board already derives exactly that map:
|
||||
`ListView.tsx:756` builds `columnFlagsById` (ColumnId -> flags) from its workflow columns, and
|
||||
`useExecutorStats` takes the same shape. Archived rows being absent from the board is
|
||||
irrelevant to a column-keyed answer.
|
||||
|
||||
So the cost is prop threading, not a fetch: MainContent -> ResearchView -> this modal, plus
|
||||
sourcing the column map where MainContent renders ResearchView (it does not hold one today).
|
||||
Three layers for one guard is a real cost and a fair thing to decline — but it is a different
|
||||
decision from "needs new data", which is what the note above would have the next reader
|
||||
believe, and the two have very different prices.
|
||||
|
||||
Left counted and unconverted. I am recording the corrected shape rather than threading it,
|
||||
because a three-component prop chain wants to be someone's deliberate change rather than a
|
||||
drive-by on the last census entry in this file.
|
||||
The guard was real either way: on a renamed board `archived` matched nothing, so filed-away
|
||||
tasks stayed in this picker and an operator could attach findings to work they had archived.
|
||||
*/
|
||||
.then((rows) => setTasks(rows.filter((task) => task.column !== "archived")))
|
||||
.then((rows) => setTasks(rows.filter((task) => !isArchivedColumn(task.column))))
|
||||
.finally(() => setLoadingTasks(false));
|
||||
}
|
||||
}, [open, mode, projectId, finding.heading, preview, run.title]);
|
||||
}, [open, mode, projectId, finding.heading, preview, run.title, isArchivedColumn]);
|
||||
|
||||
if (!open) return null;
|
||||
|
||||
|
||||
@@ -1,8 +1,6 @@
|
||||
{
|
||||
"generatedFrom": "node scripts/lifecycle-column-census.mjs --strict --update-baseline",
|
||||
"byFile": {
|
||||
"packages/dashboard/app/components/ResearchTaskActionModal.tsx": 1
|
||||
},
|
||||
"byFile": {},
|
||||
"deliberateByFile": {
|
||||
"packages/core/src/task-store/async-comments-attachments.ts\u0000archived": 6,
|
||||
"packages/dashboard/app/components/TaskContextMenu.tsx\u0000in-review": 3,
|
||||
|
||||
Reference in New Issue
Block a user