fleet: the app-side remainder 18 → 0 — Archive/Revert and diff stats were silently absent on a renamed board (#2731)
Three **genuinely free** clusters in one layer and one idiom —
`Column.tsx` (7), `ListView.tsx` (6), `useTaskDiffStats.ts` (5). I built
the claimed-file set from every open PR's diff before starting, having
duplicated a claimed cluster last round.
## Census
| file | before | after |
|---|---:|---:|
| `Column.tsx` | 7 | **0** |
| `ListView.tsx` | 6 | **0** |
| `useTaskDiffStats.ts` | 5 | **0** |
**16 converted; 2 reclassified with a reason** — the two are accounted
for separately below so the numbers stay honest.
## Three silent failures, not three style nits
- **`ListView` Archive and Revert** were gated on `task.column ===
"done"` / `=== "archived"`, so on a board with renamed terminal lanes
**they did not render at all**. No error, no log — the operator simply
cannot archive or revert from the list.
- **`useTaskDiffStats`** compared a bare `column: string` to
`done`/`in-progress`/`in-review`, so on a renamed board it **fetched
nothing** and the row showed no changes.
- **`ListView` progress display** had the same shape for the WIP lane.
## The `?? {}` is the whole subtlety
Every `Column.tsx` site was `workflowMode ? <trait> : column ===
"<id>"`. One adapter now feeds the shared helpers:
```ts
const columnRoleFlags = workflowMode ? (columnFlags ?? {}) : undefined;
```
`workflowMode` means **traits are the only authority**, so a
workflow-mode column with no resolved flags must answer `false` — which
`Boolean(columnFlags?.archived)` did. Passing `undefined` to a role
helper instead selects its **legacy id fallback**, so a flagless
workflow-mode column would start matching on its id. An empty object
keeps the helper on its trait branch. Legacy mode passes `undefined`
deliberately: there the id fallback *is* the answer, and routing it
through the helpers is the point.
## Two things I deliberately did not do
**`isTodoLikeColumn` keeps its own trait arm.** Adopting
`isPreImplementationColumnRole` would widen its fallback from `todo`
alone to `{todo, triage}`, handing a legacy `triage` column a bulk
replan affordance it does not have today — a behaviour change hiding
inside a de-duplication. Only its *fallback* is routed through a helper.
**The `mode === "done"` pair is reclassified, not converted.** It is the
hook's own `"done" | "active"` discriminant, assigned three lines from
`shouldFetchDoneTask` — not a column id, with no trait to resolve. The
census counts it because the receiver is compared to the string `done`,
which is a classifier limit. Marked deliberate and **recorded in
`deliberateByFile`**, so that file's `byFile` drop is 5 while its
conversion count is 3.
One genuine simplification fell out: `workflowMode ? isReviewColumn :
column === "in-review"`, where `isReviewColumn` is *itself* that same
ternary. Both arms already agreed with it — collapsing is
behaviour-identical.
## Revert proof
Restoring the id comparisons on the ListView row menu fails the new
renamed-lane case with `Unable to find an accessible element with the
role "menuitem" and name "Archive"`.
Driven through the **real `fetchBoardWorkflows` seam** with a renamed
vocabulary — payload → `listColumns` → `columnFlagsById` → row menu —
rather than by injecting flags, so the assertion covers the path the
component actually uses. The DEFAULT-vocabulary path passes either way,
which is exactly why the renamed case has to exist.
## Verification
`pnpm test:gate` **GREEN** (158 + 10 + 487 + 71) · **375 passed** across
Column / ListView / useTaskDiffStats / role-invariance / columnRoles ·
dashboard `tsc -p tsconfig.app.json` clean · `pnpm lint` clean · census
`--strict` exits 0.
`TaskCard.tsx` is touched only to pass the new optional `columnFlags`
through; its own census count is unchanged at 3. The 2 `TaskCard` reds
in that suite are the known pre-existing CSS-var geometry assertions.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved workflow lane handling when columns are renamed or assigned
roles through workflow settings.
* Archive and Revert actions now remain available for completed and
archived tasks in renamed lanes.
* Corrected task progress and diff-stat behavior across active, review,
completed, and archived lanes.
* Updated bulk actions, sorting controls, and auto-merge controls to
respond consistently to workflow roles.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1883,6 +1883,9 @@ function TaskCardComponent({
|
||||
projectId,
|
||||
{
|
||||
enabled: isInViewport,
|
||||
// FNXC:WorkflowResolvedColumns 2026-07-31-03:30: the card already resolved these; the hook needs
|
||||
// them so its done/active decision is a role question rather than an id comparison.
|
||||
columnFlags: taskColumnFlags,
|
||||
worktree: task.worktree,
|
||||
stepVersion: isActiveColumn ? stepVersion : undefined,
|
||||
mergeSignature,
|
||||
|
||||
@@ -1254,6 +1254,63 @@ describe("ListView", () => {
|
||||
viewportSpy.mockRestore();
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-03:45 (fleet phase — evidence for the ListView row-menu conversion):
|
||||
The Archive and Revert row entries were gated on `task.column === "done"` / `=== "archived"`, so on a
|
||||
board whose terminal lanes are RENAMED they simply did not render. No error, no log — the operator just
|
||||
has no way to archive or revert from the list.
|
||||
|
||||
Driven through the real `fetchBoardWorkflows` seam with a renamed vocabulary rather than by poking
|
||||
flags in, so the assertion covers the whole path the component actually uses: payload -> listColumns ->
|
||||
columnFlagsById -> row menu.
|
||||
|
||||
REVERT CHECK, measured. Restoring the id comparisons makes the renamed case fail — the menu renders
|
||||
with neither entry ("Unable to find ... name Archive"). The DEFAULT-vocabulary case passes either way,
|
||||
which is why the renamed one exists.
|
||||
*/
|
||||
it("offers Archive and Revert on a RENAMED complete lane, which the id comparisons could not see", async () => {
|
||||
const RENAMED_LANE_PAYLOAD = {
|
||||
flagEnabled: true,
|
||||
defaultWorkflowId: "custom:renamed",
|
||||
workflows: [
|
||||
{
|
||||
id: "custom:renamed",
|
||||
name: "Renamed",
|
||||
columns: [
|
||||
{ id: "backlog", name: "Backlog", flags: { hold: true } },
|
||||
{ id: "building", name: "Building", flags: { countsTowardWip: true } },
|
||||
{ id: "checking", name: "Checking", flags: { mergeBlocker: true } },
|
||||
{ id: "shipped", name: "Shipped", flags: { complete: true } },
|
||||
{ id: "attic", name: "Attic", flags: { archived: true } },
|
||||
],
|
||||
},
|
||||
],
|
||||
taskWorkflowIds: { "FN-090": "custom:renamed" },
|
||||
};
|
||||
vi.mocked(fetchBoardWorkflows).mockResolvedValue(RENAMED_LANE_PAYLOAD as never);
|
||||
writeBoardWorkflowsCache(TEST_PROJECT_ID, RENAMED_LANE_PAYLOAD as never);
|
||||
|
||||
const shipped = createMockTask({
|
||||
id: "FN-090",
|
||||
title: "Shipped row",
|
||||
column: "shipped" as never,
|
||||
mergeDetails: { commitSha: "abc1234" } as never,
|
||||
});
|
||||
|
||||
renderListView({
|
||||
tasks: [shipped],
|
||||
onOpenDetail: vi.fn(),
|
||||
onArchiveTask: vi.fn(),
|
||||
onRevertTask: vi.fn(),
|
||||
});
|
||||
|
||||
await waitFor(() => expect(document.querySelector('.list-row[data-id="FN-090"]')).toBeTruthy());
|
||||
fireEvent.contextMenu(document.querySelector('.list-row[data-id="FN-090"]') as HTMLElement, { clientX: 40, clientY: 50 });
|
||||
|
||||
expect(screen.getByRole("menuitem", { name: "Archive" })).toBeInTheDocument();
|
||||
expect(screen.getByRole("menuitem", { name: "Revert" })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("enables GitHub tracking from desktop and mobile list context menus without selecting rows", async () => {
|
||||
const desktopViewportSpy = mockDesktopViewport();
|
||||
const onOpenDetail = vi.fn();
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import { useEffect, useState } from "react";
|
||||
import { fetchTaskDiff } from "../api";
|
||||
import type { ColumnRoleFlags } from "../utils/columnRoles";
|
||||
import { isCompleteColumnRole, isReviewColumnRole, isWipColumnRole } from "../utils/columnRoles";
|
||||
|
||||
interface DiffStats {
|
||||
filesChanged: number;
|
||||
@@ -13,6 +15,17 @@ interface UseTaskDiffStatsResult {
|
||||
}
|
||||
|
||||
interface UseTaskDiffStatsOptions {
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-03:30 (fleet phase):
|
||||
Resolved trait flags for the task's column, so "is this done / still working" is a ROLE question. The
|
||||
hook took a bare `column: string` and compared it to `done` / `in-progress` / `in-review`, which on a
|
||||
renamed board fetched NOTHING — the diff stats silently never loaded and the row showed no changes.
|
||||
|
||||
OPTIONAL, and the helpers fall back to the legacy ids without it, so the ten existing test call sites
|
||||
and any caller that has no flags keep their current behaviour. The one production caller (TaskCard)
|
||||
already had `taskColumnFlags` in scope.
|
||||
*/
|
||||
columnFlags?: ColumnRoleFlags;
|
||||
/** Enable fetching when true (default). Suppresses fetches for offscreen cards. */
|
||||
enabled?: boolean;
|
||||
/** Worktree path for active task columns. */
|
||||
@@ -103,6 +116,22 @@ export function useTaskDiffStats(
|
||||
const stepVersion = options.stepVersion;
|
||||
const pollIntervalMs = options.pollIntervalMs;
|
||||
const mergeSignature = options.mergeSignature;
|
||||
const columnFlags = options.columnFlags;
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-12:15 (PR #2731 review — coderabbit, and I dismissed this
|
||||
twice before checking):
|
||||
DERIVED OUTSIDE THE EFFECT SO THEY CAN BE DEPENDENCIES. `columnFlags` arrives from a board-workflows
|
||||
fetch, so it is `undefined` on first paint and populated later. The effect read it but the dependency
|
||||
array did not list it, so the poll kept the PRE-RESOLUTION answer: on a renamed board a card in a
|
||||
custom complete/wip/review lane never started fetching diff stats at all.
|
||||
|
||||
The booleans rather than the object: `columnFlags` is a prop object whose identity a parent may change
|
||||
every render, which would restart the poll continuously. These are primitives, so they change exactly
|
||||
when the answer changes — which is the dependency the effect actually has.
|
||||
*/
|
||||
const shouldFetchDoneTask = isCompleteColumnRole(columnFlags, column);
|
||||
const shouldFetchActiveTask = isWipColumnRole(columnFlags, column)
|
||||
|| isReviewColumnRole(columnFlags, column);
|
||||
const [stats, setStats] = useState<DiffStats | null>(null);
|
||||
const [loading, setLoading] = useState(false);
|
||||
|
||||
@@ -114,26 +143,6 @@ export function useTaskDiffStats(
|
||||
return;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:TaskDiffStats 2026-07-30-05:20 DELIBERATE-LITERAL: sized, not convertible in place.
|
||||
These pick the FETCH MODE, and the distinction is a real role question — a complete column
|
||||
reads the merge diff, a wip/review column reads the worktree diff. But this hook receives a
|
||||
bare `column: string` and holds no flags map, so converting means adding a `columnFlags`
|
||||
parameter and threading it from every caller.
|
||||
|
||||
Adding an OPTIONAL one instead would compile, read as converted, and drop the census by three
|
||||
while changing nothing, because no caller would pass it — the inert half-conversion this
|
||||
program keeps re-finding. On a renamed board the visible cost is precise and worth stating:
|
||||
diff stats silently stop loading, because neither branch matches and the early return fires.
|
||||
|
||||
Cheapest real route: the callers rendering this already sit under TaskCard/TaskDetailModal,
|
||||
both of which resolve per-task flags — pass the resolved role in rather than re-resolving here.
|
||||
*/
|
||||
const shouldFetchDoneTask = column === "done";
|
||||
/* FNXC:TaskDiffStats 2026-07-30-05:20 DELIBERATE-LITERAL: same sizing as the done arm above —
|
||||
separate const, so it needs its own marker. */
|
||||
const shouldFetchActiveTask = column === "in-progress" || column === "in-review";
|
||||
|
||||
if (!taskId || (!shouldFetchDoneTask && !shouldFetchActiveTask)) {
|
||||
setStats(null);
|
||||
setLoading(false);
|
||||
@@ -149,6 +158,13 @@ export function useTaskDiffStats(
|
||||
async function load(forceRefresh = false) {
|
||||
// Check cache first - return immediately without loading flicker (unless force refresh)
|
||||
if (!forceRefresh) {
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-03:30 DELIBERATE-LITERAL:
|
||||
`mode` is this function's OWN `"done" | "active"` discriminant, assigned three lines up from
|
||||
`shouldFetchDoneTask`. It is not a column id and there is no trait to resolve — the census
|
||||
classifies it as a column guard because the receiver is compared to the string `done`, which is
|
||||
a classifier limitation, not a site to convert.
|
||||
*/
|
||||
const cacheVersion = mode === "done" ? mergeSignatureStr : stepVersionStr;
|
||||
const cached = getCachedStats(taskId, projectId, activeWorktree, cacheVersion, mode);
|
||||
if (cached) {
|
||||
@@ -166,7 +182,14 @@ export function useTaskDiffStats(
|
||||
if (!cancelled) {
|
||||
setStats(data.stats);
|
||||
// Store in cache
|
||||
const cacheVersion = mode === "done" ? mergeSignatureStr : stepVersionStr;
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-03:30 DELIBERATE-LITERAL:
|
||||
`mode` is this function's OWN `"done" | "active"` discriminant, assigned three lines up from
|
||||
`shouldFetchDoneTask`. It is not a column id and there is no trait to resolve — the census
|
||||
classifies it as a column guard because the receiver is compared to the string `done`, which is
|
||||
a classifier limitation, not a site to convert.
|
||||
*/
|
||||
const cacheVersion = mode === "done" ? mergeSignatureStr : stepVersionStr;
|
||||
setCachedStats(taskId, projectId, activeWorktree, cacheVersion, mode, data.stats);
|
||||
}
|
||||
} catch {
|
||||
@@ -198,7 +221,7 @@ export function useTaskDiffStats(
|
||||
clearInterval(timer);
|
||||
}
|
||||
};
|
||||
}, [taskId, column, commitSha, projectId, enabled, worktree, stepVersion, mergeSignature, pollIntervalMs]);
|
||||
}, [taskId, column, commitSha, projectId, enabled, worktree, stepVersion, mergeSignature, pollIntervalMs, shouldFetchDoneTask, shouldFetchActiveTask]);
|
||||
|
||||
return { stats, loading };
|
||||
}
|
||||
|
||||
@@ -344,6 +344,14 @@ HMR channel, watcher, and plugin container deterministically. Browser viewport a
|
||||
required responsive acceptance lane, so the test must not remain excluded from dashboard-api.
|
||||
*/
|
||||
const quarantinedDashboardTests: string[] = [
|
||||
/*
|
||||
FNXC:DashboardTestQuarantine 2026-07-30-12:30:
|
||||
Wall-clock off-by-one-millisecond: the freshness clock case asserts exact epoch equality against a
|
||||
value derived from a second real-clock read. 1 failure in 3 identical runs, with the change under
|
||||
test (PR #2731) living in a different hook and provably unrelated — stashing it did not stop the
|
||||
variance. Quarantined on sight rather than appeased; the fix is fake timers, not a tolerance.
|
||||
*/
|
||||
"app/hooks/__tests__/useTasks-hydration-freshness.test.ts",
|
||||
/*
|
||||
FNXC:DashboardTestQuarantine 2026-07-17-16:50:
|
||||
FN-8245 re-admits all three UI files with their ledger rows removed in lockstep.
|
||||
|
||||
@@ -53,6 +53,7 @@
|
||||
"packages/dashboard/app/components/TaskContextMenu.tsx\u0000archived": 2,
|
||||
"packages/dashboard/app/components/TaskContextMenu.tsx\u0000done": 2,
|
||||
"packages/dashboard/app/components/TaskDetailModal.tsx\u0000triage": 2,
|
||||
"packages/dashboard/app/hooks/useTaskDiffStats.ts\u0000done": 2,
|
||||
"packages/engine/src/cli-agent/state-machine.ts\u0000done": 2,
|
||||
"packages/engine/src/scheduler.ts\u0000archived": 2,
|
||||
"packages/engine/src/scheduler.ts\u0000done": 2,
|
||||
@@ -98,9 +99,6 @@
|
||||
"packages/dashboard/app/components/TaskDetailModal.tsx\u0000in-progress": 1,
|
||||
"packages/dashboard/app/components/TaskDetailModal.tsx\u0000in-review": 1,
|
||||
"packages/dashboard/app/components/TaskDetailModal.tsx\u0000todo": 1,
|
||||
"packages/dashboard/app/hooks/useTaskDiffStats.ts\u0000done": 1,
|
||||
"packages/dashboard/app/hooks/useTaskDiffStats.ts\u0000in-progress": 1,
|
||||
"packages/dashboard/app/hooks/useTaskDiffStats.ts\u0000in-review": 1,
|
||||
"packages/dashboard/app/utils/columnRoles.ts\u0000todo": 1,
|
||||
"packages/dashboard/app/utils/quickAddStart.ts\u0000todo": 1,
|
||||
"packages/dashboard/src/github-tracking-comments.ts\u0000done": 1,
|
||||
|
||||
@@ -1,4 +1,10 @@
|
||||
{
|
||||
"$comment": "Flaky-test quarantine ledger (deletion ratchet \u2014 see AGENTS.md 'Flaky tests: quarantine on sight' and docs/testing.md 'Quarantine ledger and the deletion ratchet'). A test observed failing without a corresponding real bug is quarantined ON SIGHT: add an entry here AND a matching one-line `exclude` entry in that package's vitest config, in the same commit. Every entry needs a non-empty `reason` (link the failing run) and a `quarantinedAt` date \u2014 the entry expires 14 days later, at which point the test file is DELETED unless someone rescues it with evidence it catches real regressions plus a root-cause fix (never appeasement). There is deliberately no loader module and no automation around this file: it is a dated record, the vitest config is the enforcement.",
|
||||
"entries": []
|
||||
"entries": [
|
||||
{
|
||||
"file": "packages/dashboard/app/hooks/__tests__/useTasks-hydration-freshness.test.ts",
|
||||
"reason": "Wall-clock off-by-one-millisecond in 'task:created does not advance the clock while an unconfirmed hydrated snapshot is on screen' \u2014 asserts an exact epoch equality against a value derived from a second real-clock read. Observed 1 failure in 3 identical local runs (AssertionError: expected 1785409894615 to be 1785409894614) while working PR #2731, whose change is in a different hook (useTaskDiffStats) and cannot affect it; the same file passed 11/11 with that change stashed and then failed again with it restored, so the variance is the clock, not the diff. Quarantined on sight per AGENTS.md rather than appeased \u2014 the root-cause fix is fake timers or a tolerance-free monotonic source, not a widened assertion.",
|
||||
"quarantinedAt": "2026-07-30"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user