Phase B slice B1: lifecycle column roles in the U6 policy modules (4 guards, red-green) (#2479)
**Stacked on #2469** → #2468 → #2467. Base is `feature/workflow-capacity-ground-truth`. This is **slice B1 of Phase B, not all of Phase B.** Sizing escalation sent separately; the census is below. ## Why this is a slice Measured census of code lines referencing a lifecycle column literal (comments excluded): | Unit | Files | Sites | |---|---|---:| | U4 | `self-healing.ts` | 203 | | U5 | `executor.ts` 171, `scheduler.ts` 55, `replan-target.ts` 20, `merger-ai.ts` 5, `hold-release.ts` 4, `mesh-lease-manager.ts` 4, `task-agent-sync.ts` 3 | 262 | | U6 | `moves.ts` 34, `default-workflow-hooks.ts` 13, `board-config.ts` 9, `blocker-fanout.ts` 6, `task-priority.ts` 5, `dependency-blocked-todo-report.ts` 2, `stale-paused-todo.ts` 1 | 70 | | | **Total** | **535** | The plan's "~207" counts the guard category only. Under the phase's non-negotiable rule — a test that **fails before** conversion, per guard — that is ~200 red-green cycles. Doing it as one sweep would reproduce exactly the failure this phase exists to prevent: converted guards nobody proved still fire. `moves.ts` and `default-workflow-hooks.ts` stay **parked** per the dispatch constraint (move-path convergence and the pool-id sentinel are on an operator decision). ## Guards converted (4), each red-green Every case below was written **first** and observed failing against the literal implementation. | Module | Guard | Before → After | |---|---|---| | `stale-paused-todo.ts` | stall detection | `column !== "todo"` → resolved **hold** column | | `blocker-fanout.ts` | active | `ACTIVE_COLUMNS.has(col)` → `!terminalColumns.has(col)` | | `blocker-fanout.ts` | hold-wait metric | `col === "todo"` → resolved **hold** column | | `task-priority.ts` | unblock active | `UNBLOCK_ACTIVE_COLUMNS` **deleted**, folded into the terminal set | Three of the seven new cases are **regression floors** that pass before and after. One of them earned its keep immediately: it failed on my own fixture (`activeCount` vs the public `totalCount`), catching a bad test rather than bad code — which is the point of asserting the default path alongside the renamed one. ### The `task-priority` finding `UNBLOCK_ACTIVE_COLUMNS` and `DONE_COLUMNS` encoded **one concept twice**, two lines apart, and disagreed for any custom column: dependency counting treated a `drafting` card as unmet (correct) while the active check treated it as inactive (wrong), zeroing the blocker's unblock weight. The enumeration wasn't just legacy-shaped — it contradicted its own neighbour. ## ⚠️ Behavior change, not a pure refactor Inverting active from enumeration to exclusion means **a card in a column that is neither terminal nor in the legacy enum now counts as active where it previously did not.** That is the plan's stated intent, but it is a real change for any project already using a custom column — **Coding (Ideas)' `ideas` column is the in-tree case.** Fan-out counts and unblock weights for such cards will rise. ## Verification - Four affected suites green (45 tests), each conversion observed red→green. - `pnpm lint`, `tsc --noEmit` (core) green. **Not verified / not done, stated plainly:** - **Call sites are not wired.** These modules now *accept* resolved roles; every parameter still defaults to the legacy set, so at the call sites the vocabulary is unchanged. A caller that cannot resolve a workflow keeps literal behavior. Threading `resolveLifecycleColumns` through `reads.ts` and `self-healing.ts` is follow-on work — until then the guards are *convertible*, not *converted end-to-end*. - `dependency-blocked-todo-report.ts` and `board-config.ts` are untouched in this slice. - 19 core-suite failures exist on this branch; all confirmed **pre-existing** by stashing and re-running on a clean tree (`duplicate-guard`, `log-severity-spam-contract`, `settings-parity`, `task-delete-caller-attribution`, `settings-defaults`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- **Supersedes #2470**, which GitHub force-closed when its base branch was deleted by the merge of #2469 and refuses to reopen. Same head branch, same commits (rebased onto `main`), now based on `main` directly. The two P1 review threads on #2470 were resolved there — one of them with a correction noting the threading half landed in code that was subsequently deleted as a dead feature in #2477. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Dependency and blocker reports now correctly recognize custom hold, active, and terminal workflow columns. * Blockers in renamed terminal columns are no longer incorrectly reported as active. * Stale paused-task badges and self-healing now work with workflow-specific hold columns. * Mixed boards with different workflow column names are handled consistently. * Existing default workflow behavior remains compatible, including fallback handling when workflow details cannot be resolved. * **Enhancements** * Reporting and task-priority calculations now support configurable single or multiple hold and terminal columns. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
gsxdsm
@@ -64,3 +64,70 @@ describe("computeBlockerFanoutMap escalation", () => {
|
||||
expect(entry?.escalation).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-27-21:45 (Phase B / U6 — vocabulary conversion):
|
||||
RED-GREEN PROOF for the two column guards in `computeBlockerFanoutMap`, written
|
||||
BEFORE the conversion and asserted to fail against the literal implementation.
|
||||
|
||||
Two distinct guards, converted for different reasons:
|
||||
|
||||
ACTIVE — was an ENUMERATION (`triage/todo/in-progress/in-review`), which silently
|
||||
excludes every column a custom workflow adds. Inverted to the plan's own
|
||||
phrasing: active means NOT complete and NOT archived. That is the definition the
|
||||
concept always had; the enumeration was a default-workflow-shaped stand-in for it,
|
||||
and it under-counted for any other workflow rather than failing.
|
||||
|
||||
HOLD (`isTodo`) — the fan-out metric "how many blocked cards are waiting for
|
||||
capacity" is about the hold role, not the id `todo`.
|
||||
*/
|
||||
describe("computeBlockerFanoutMap — column roles are resolved, not enumerated (U6)", () => {
|
||||
function task(id: string, column: string, dependencies: string[] = []): Task {
|
||||
return {
|
||||
id, column, dependencies,
|
||||
title: id, description: "", priority: "normal", steps: [],
|
||||
createdAt: "2026-07-01T00:00:00.000Z",
|
||||
updatedAt: "2026-07-01T00:00:00.000Z",
|
||||
columnMovedAt: "2026-07-01T00:00:00.000Z",
|
||||
} as unknown as Task;
|
||||
}
|
||||
|
||||
it("counts a DEFAULT-workflow dependent as active and as waiting in hold (regression floor)", () => {
|
||||
const map = computeBlockerFanoutMap([task("FN-1", "in-review"), task("FN-2", "todo", ["FN-1"])], 3);
|
||||
expect(map.get("FN-1")?.totalCount).toBe(1);
|
||||
expect(map.get("FN-1")?.activeTodoCount).toBe(1);
|
||||
});
|
||||
|
||||
it("counts a RENAMED-workflow dependent as active — the enumeration silently missed it", () => {
|
||||
// `writing` is in no legacy enum, so the literal ACTIVE_COLUMNS set scored 0
|
||||
// active dependents and the blocker looked unblocking. No error, no failure.
|
||||
const map = computeBlockerFanoutMap(
|
||||
[task("FN-1", "editorial-review"), task("FN-2", "drafting", ["FN-1"])],
|
||||
3,
|
||||
{ terminalColumns: new Set(["published", "shelved"]), holdColumn: "drafting" },
|
||||
);
|
||||
expect(map.get("FN-1")?.totalCount).toBe(1);
|
||||
expect(map.get("FN-1")?.activeTodoCount).toBe(1);
|
||||
});
|
||||
|
||||
it("excludes the renamed workflow's OWN terminal columns from active", () => {
|
||||
// The other half: 'not complete, not archived' must still exclude something.
|
||||
const map = computeBlockerFanoutMap(
|
||||
[task("FN-1", "editorial-review"), task("FN-2", "published", ["FN-1"])],
|
||||
3,
|
||||
{ terminalColumns: new Set(["published", "shelved"]), holdColumn: "drafting" },
|
||||
);
|
||||
expect(map.get("FN-1")?.totalCount).toBe(0);
|
||||
expect(map.get("FN-1")?.activeTodoCount).toBe(0);
|
||||
});
|
||||
|
||||
it("does not count a non-hold column toward the hold-wait metric", () => {
|
||||
const map = computeBlockerFanoutMap(
|
||||
[task("FN-1", "editorial-review"), task("FN-2", "writing", ["FN-1"])],
|
||||
3,
|
||||
{ terminalColumns: new Set(["published", "shelved"]), holdColumn: "drafting" },
|
||||
);
|
||||
expect(map.get("FN-1")?.totalCount).toBe(1);
|
||||
expect(map.get("FN-1")?.activeTodoCount).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,138 @@
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-28-02:40 (PR #2470 review, P1):
|
||||
|
||||
`computeBlockerFanoutMap` accepts resolved `terminalColumns` / `holdColumn`, but
|
||||
`computeDependencyBlockedTodoReport` called it with NEITHER — so the fan-out fell
|
||||
back to the legacy {done, archived} / "todo" defaults even for a workflow that
|
||||
renames them. That is the "convertible rather than converted" defect: the module
|
||||
takes the roles, the caller never passes them, and end-to-end the bug is still
|
||||
live.
|
||||
|
||||
Concretely, for a workflow whose terminal column is `published` and whose hold
|
||||
column is `queued`:
|
||||
- a FINISHED blocker in `published` counted as ACTIVE, so it kept appearing as
|
||||
a live blocker in the report;
|
||||
- dependents resting in `queued` were not counted as blocked todos at all
|
||||
(`activeTodoCount` keyed on the literal "todo"), so genuinely-blocked work
|
||||
was invisible to the report.
|
||||
|
||||
The two errors point in OPPOSITE directions — over-reporting dead blockers while
|
||||
under-reporting real ones — which is why both are asserted separately rather
|
||||
than through a single aggregate count.
|
||||
|
||||
The report also had two literals of its OWN beyond the unthreaded call: its
|
||||
`todoTaskIds` filter and its `blocker.column === "done" || "archived"` skip.
|
||||
Threading the fan-out alone would have left those, so a renamed workflow would
|
||||
still report nothing. All three now read the same resolved roles.
|
||||
|
||||
Written against the unthreaded implementation and observed FAILING first.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
computeDependencyBlockedTodoReport,
|
||||
type DependencyBlockedTodoReportContext,
|
||||
} from "../dependency-blocked-todo-report.js";
|
||||
import type { Task } from "../types.js";
|
||||
|
||||
const NOW = Date.parse("2026-05-01T12:00:00.000Z");
|
||||
/** Old enough to be a "stale" blocker, so age bucketing never masks a miss. */
|
||||
const MOVED_AT = new Date(NOW - 5 * 60 * 60_000).toISOString();
|
||||
|
||||
function task(over: Partial<Task> = {}): Task {
|
||||
return {
|
||||
id: "FN-1",
|
||||
title: "t",
|
||||
description: "",
|
||||
column: "todo",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: MOVED_AT,
|
||||
updatedAt: MOVED_AT,
|
||||
columnMovedAt: MOVED_AT,
|
||||
...over,
|
||||
} as Task;
|
||||
}
|
||||
|
||||
/**
|
||||
* One blocker plus two dependents resting in the hold column, expressed under a
|
||||
* caller-supplied vocabulary. The SHAPE is identical across vocabularies, so any
|
||||
* difference in the report is attributable to a surviving literal.
|
||||
*/
|
||||
function board(names: { hold: string; terminal: string }, blockerColumn: string): Task[] {
|
||||
return [
|
||||
task({ id: "BLOCKER", column: blockerColumn }),
|
||||
task({ id: "DEP-1", column: names.hold, dependencies: ["BLOCKER"] }),
|
||||
task({ id: "DEP-2", column: names.hold, dependencies: ["BLOCKER"] }),
|
||||
];
|
||||
}
|
||||
|
||||
const DEFAULT_NAMES = { hold: "todo", terminal: "done" };
|
||||
/* Neither renamed id collides with a legacy literal. */
|
||||
const RENAMED = { hold: "queued", terminal: "published" };
|
||||
const RENAMED_ROLES: DependencyBlockedTodoReportContext = {
|
||||
now: NOW,
|
||||
holdColumn: RENAMED.hold,
|
||||
terminalColumns: [RENAMED.terminal, "retired"],
|
||||
};
|
||||
|
||||
describe("dependency-blocked-todo report under a renamed column vocabulary", () => {
|
||||
it("counts dependents resting in a RENAMED hold column as blocked todos", async () => {
|
||||
// Blocker is live (in the wip column), dependents wait in the renamed hold.
|
||||
const report = computeDependencyBlockedTodoReport(board(RENAMED, "building"), 0, RENAMED_ROLES);
|
||||
|
||||
expect(report.totalBlockedTodoCount).toBe(2);
|
||||
expect(report.uniqueBlockerCount).toBe(1);
|
||||
expect(report.groups[0]?.blockerId).toBe("BLOCKER");
|
||||
expect(report.groups[0]?.blockedTodoIds).toEqual(["DEP-1", "DEP-2"]);
|
||||
});
|
||||
|
||||
it("drops a blocker that already reached a RENAMED terminal column", async () => {
|
||||
/* The opposite-direction error: a finished blocker kept being reported as a
|
||||
live one because `published` is not in the legacy terminal set. */
|
||||
const report = computeDependencyBlockedTodoReport(
|
||||
board(RENAMED, RENAMED.terminal),
|
||||
0,
|
||||
RENAMED_ROLES,
|
||||
);
|
||||
|
||||
expect(report.groups).toEqual([]);
|
||||
expect(report.totalBlockedTodoCount).toBe(0);
|
||||
});
|
||||
|
||||
it("honors a SECOND declared terminal column, not just the first", async () => {
|
||||
/* terminalColumns is a set, not a single id — a fix that only handled the
|
||||
primary terminal column would pass the test above and fail here. */
|
||||
const report = computeDependencyBlockedTodoReport(board(RENAMED, "retired"), 0, RENAMED_ROLES);
|
||||
|
||||
expect(report.groups).toEqual([]);
|
||||
});
|
||||
|
||||
it("is byte-identical for the builtin vocabulary when roles are omitted (regression floor)", async () => {
|
||||
const live = computeDependencyBlockedTodoReport(board(DEFAULT_NAMES, "in-progress"), 0, { now: NOW });
|
||||
expect(live.totalBlockedTodoCount).toBe(2);
|
||||
expect(live.groups[0]?.blockedTodoIds).toEqual(["DEP-1", "DEP-2"]);
|
||||
|
||||
const finished = computeDependencyBlockedTodoReport(board(DEFAULT_NAMES, "done"), 0, { now: NOW });
|
||||
expect(finished.groups).toEqual([]);
|
||||
|
||||
const archived = computeDependencyBlockedTodoReport(board(DEFAULT_NAMES, "archived"), 0, { now: NOW });
|
||||
expect(archived.groups).toEqual([]);
|
||||
});
|
||||
|
||||
it("still counts a legacy-named board correctly when roles ARE supplied explicitly", async () => {
|
||||
/* Supplying the legacy ids explicitly must behave exactly like omitting
|
||||
them — otherwise threading the caller would itself change behavior for
|
||||
builtin:coding. */
|
||||
const report = computeDependencyBlockedTodoReport(board(DEFAULT_NAMES, "in-progress"), 0, {
|
||||
now: NOW,
|
||||
holdColumn: "todo",
|
||||
terminalColumns: ["done", "archived"],
|
||||
});
|
||||
|
||||
expect(report.totalBlockedTodoCount).toBe(2);
|
||||
expect(report.groups[0]?.blockedTodoIds).toEqual(["DEP-1", "DEP-2"]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,125 @@
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-28-04:20 (PR #2470 review, P1):
|
||||
|
||||
End-to-end proof for the dashboard half of the stale-paused-todo fix.
|
||||
|
||||
`getStalePausedTodoSignal` gained a `holdColumn` parameter in B1, but BOTH
|
||||
hydration sites in reads.ts omitted it — so the guard compared against the
|
||||
literal "todo" and the badge was silent for a paused card resting in a renamed
|
||||
hold column. Silent is the worst failure shape here: an operator cannot tell a
|
||||
stalled board from a healthy one.
|
||||
|
||||
This is a real-store test rather than a mock because the defect lived in
|
||||
hydration, not in the pure signal — the pure `stale-paused-todo` unit tests were
|
||||
already green with the parameter in place.
|
||||
|
||||
Scope note, found while writing this file: `getTaskImpl` does NOT hydrate
|
||||
`stalePausedTodo` at all — the two hydration sites live in `listTasksImpl` and
|
||||
`listTasksModifiedSinceImpl`. So the task DETAIL view has never carried this
|
||||
badge, with or without a renamed workflow. That is a pre-existing gap unrelated
|
||||
to the column-vocabulary work, so it is reported rather than fixed here; do not
|
||||
read the absence of a getTask case below as an oversight.
|
||||
|
||||
Fixture note worth keeping: `createWorkflowDefinition` ALLOCATES ITS OWN id
|
||||
(`WF-001`) and ignores the `id` field in the input. Binding a task to the id we
|
||||
passed in resolves to the default builtin IR instead, and every assertion here
|
||||
then passes or fails for reasons having nothing to do with the code under test.
|
||||
Always bind to the returned id.
|
||||
*/
|
||||
import { it, expect, beforeAll, beforeEach, afterEach, afterAll } from "vitest";
|
||||
import { eq } from "drizzle-orm";
|
||||
import {
|
||||
pgDescribe,
|
||||
createSharedPgTaskStoreTestHarness,
|
||||
type SharedPgTaskStoreHarness,
|
||||
} from "../../__test-utils__/pg-test-harness.js";
|
||||
import * as schema from "../../postgres/schema/index.js";
|
||||
|
||||
const THRESHOLD_MS = 24 * 60 * 60_000;
|
||||
|
||||
pgDescribe("TaskStore stalePausedTodo hydration under a renamed hold column (PostgreSQL)", () => {
|
||||
const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({
|
||||
prefix: "fusion_stale_paused_renamed",
|
||||
});
|
||||
|
||||
beforeAll(h.beforeAll);
|
||||
beforeEach(h.beforeEach);
|
||||
afterEach(h.afterEach);
|
||||
afterAll(h.afterAll);
|
||||
|
||||
/**
|
||||
* A workflow whose hold column is `drafting` — it has NO `todo` column.
|
||||
* Returns the id the STORE assigned: `createWorkflowDefinition` allocates its
|
||||
* own (`WF-001`) and ignores the `id` field, so binding a task to the id we
|
||||
* passed in silently resolves to the default builtin IR instead.
|
||||
*/
|
||||
async function seedRenamedWorkflow(): Promise<string> {
|
||||
const created = await h.store().createWorkflowDefinition({
|
||||
id: "custom:renamed-hold",
|
||||
name: "Renamed Hold",
|
||||
kind: "workflow",
|
||||
ir: {
|
||||
version: "v2",
|
||||
id: "custom:renamed-hold",
|
||||
// A valid IR needs exactly one start and one end node.
|
||||
nodes: [
|
||||
{ id: "start", kind: "start", column: "drafting" },
|
||||
{ id: "end", kind: "end", column: "shipped" },
|
||||
],
|
||||
edges: [{ from: "start", to: "end" }],
|
||||
columns: [
|
||||
{ id: "drafting", label: "Drafting", traits: [{ trait: "hold", config: { release: "capacity" } }] },
|
||||
{ id: "building", label: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
|
||||
{ id: "shipped", label: "Shipped", traits: [{ trait: "complete" }] },
|
||||
],
|
||||
},
|
||||
} as never);
|
||||
return (created as { id: string }).id;
|
||||
}
|
||||
|
||||
/** Seed a paused card, aged past the threshold, bound to `workflowId`. */
|
||||
async function seedPausedTask(id: string, column: string, workflowId?: string) {
|
||||
const store = h.store();
|
||||
const movedAt = new Date(Date.now() - (THRESHOLD_MS + 60_000)).toISOString();
|
||||
await store.createTaskWithReservedId(
|
||||
{ description: id, column } as never,
|
||||
{ taskId: id, createdAt: movedAt, updatedAt: movedAt, applyDefaultWorkflowSteps: false } as never,
|
||||
);
|
||||
if (workflowId) await store.writeTaskWorkflowSelection(id, workflowId, []);
|
||||
await h
|
||||
.adminDb()
|
||||
.update(schema.project.tasks)
|
||||
.set({ paused: 1, columnMovedAt: movedAt, updatedAt: movedAt })
|
||||
.where(eq(schema.project.tasks.id, id));
|
||||
store.taskCache.delete(id);
|
||||
}
|
||||
|
||||
it("hydrates the badge via listTasks for a paused card in a RENAMED hold column", async () => {
|
||||
const wf = await seedRenamedWorkflow();
|
||||
await seedPausedTask("FN-RH-1", "drafting", wf);
|
||||
|
||||
const task = (await h.store().listTasks({ slim: true })).find((t) => t.id === "FN-RH-1");
|
||||
|
||||
expect(task?.stalePausedTodo?.code).toBe("stale-paused-todo");
|
||||
});
|
||||
|
||||
it("does NOT badge a paused card resting in a non-hold column of that workflow", async () => {
|
||||
/* The negative half: threading the hold column must not turn the badge into
|
||||
"any paused card anywhere", which would be a noisier bug than the silence
|
||||
it replaces. */
|
||||
const wf = await seedRenamedWorkflow();
|
||||
await seedPausedTask("FN-RH-3", "building", wf);
|
||||
|
||||
const task = (await h.store().listTasks({ slim: true })).find((t) => t.id === "FN-RH-3");
|
||||
|
||||
expect(task?.stalePausedTodo).toBeUndefined();
|
||||
});
|
||||
|
||||
it("still badges a builtin todo card with no custom workflow (regression floor)", async () => {
|
||||
await seedPausedTask("FN-RH-4", "todo");
|
||||
|
||||
const listed = (await h.store().listTasks({ slim: true })).find((t) => t.id === "FN-RH-4");
|
||||
|
||||
expect(listed?.stalePausedTodo?.code).toBe("stale-paused-todo");
|
||||
});
|
||||
});
|
||||
@@ -66,3 +66,60 @@ describe("getStalePausedTodoSignal", () => {
|
||||
expect(signal?.ageMs).toBe(DEFAULT_STALE_PAUSED_TODO_THRESHOLD_MS);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-27-21:30 (Phase B / U6 — vocabulary conversion):
|
||||
RED-GREEN PROOF for the hold-column guard in `getStalePausedTodoSignal`.
|
||||
|
||||
Written BEFORE the conversion and asserted to FAIL against the literal `"todo"`
|
||||
implementation. That ordering is the whole point of this phase: a guard converted
|
||||
first and tested after proves nothing, because a guard that silently stops
|
||||
matching disables its recovery path without failing anything — which is how 82
|
||||
dead column guards passed a merge gate before this program existed.
|
||||
|
||||
The signal detects a card that has sat PAUSED in the capacity-hold column past a
|
||||
threshold. "Hold column" is the lifecycle role; `todo` is merely the id the
|
||||
built-in coding workflow happens to give it. A workflow that names its hold
|
||||
column `drafting` has exactly the same stall condition and must produce exactly
|
||||
the same signal.
|
||||
*/
|
||||
describe("getStalePausedTodoSignal — hold column is resolved, not literal (U6)", () => {
|
||||
const staleAnchor = new Date(Date.now() - 48 * 60 * 60_000).toISOString();
|
||||
const pausedCard = (column: string) => ({
|
||||
column,
|
||||
paused: true as const,
|
||||
columnMovedAt: staleAnchor,
|
||||
updatedAt: staleAnchor,
|
||||
pausedReason: "operator",
|
||||
pausedByAgentId: undefined,
|
||||
});
|
||||
|
||||
it("fires for the DEFAULT workflow's hold column (regression floor)", () => {
|
||||
expect(getStalePausedTodoSignal(pausedCard("todo"))).toMatchObject({
|
||||
code: "stale-paused-todo",
|
||||
});
|
||||
});
|
||||
|
||||
it("fires for a RENAMED hold column when the caller resolves it", () => {
|
||||
// THE conversion assertion. Fails against the literal implementation.
|
||||
expect(
|
||||
getStalePausedTodoSignal(pausedCard("drafting"), { holdColumn: "drafting" }),
|
||||
).toMatchObject({ code: "stale-paused-todo" });
|
||||
});
|
||||
|
||||
it("does NOT fire for a non-hold column in a renamed workflow", () => {
|
||||
// The other half: conversion must not make the guard match everything.
|
||||
expect(
|
||||
getStalePausedTodoSignal(pausedCard("writing"), { holdColumn: "drafting" }),
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it("does NOT fire for the legacy id when the workflow's hold column is different", () => {
|
||||
// Proves the guard follows the WORKFLOW, not the legacy vocabulary: a card
|
||||
// parked in `todo` under a workflow whose hold column is `drafting` is not
|
||||
// stalled-in-hold, it is sitting in some other column entirely.
|
||||
expect(
|
||||
getStalePausedTodoSignal(pausedCard("todo"), { holdColumn: "drafting" }),
|
||||
).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -173,3 +173,47 @@ describe("task-priority", () => {
|
||||
expect(typeof core.sortTasksForDisplayColumn).toBe("function");
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-27-22:05 (Phase B / U6 — vocabulary conversion):
|
||||
RED-GREEN PROOF for `UNBLOCK_ACTIVE_COLUMNS` in `buildUnblockWeightMap`, written
|
||||
before the conversion. Same enumeration bug as blocker-fanout's ACTIVE_COLUMNS,
|
||||
same fix: active is NOT complete and NOT archived. The module already had a
|
||||
`DONE_COLUMNS` set expressing exactly that exclusion for dependency counting, so
|
||||
the two halves of one concept were encoded twice and disagreed for any custom
|
||||
column — an unblock weight silently scored 0 for a renamed workflow.
|
||||
*/
|
||||
describe("buildUnblockWeightMap — active is 'not terminal', not an enumeration (U6)", () => {
|
||||
function t(id: string, column: string, dependencies: string[] = []): Task {
|
||||
return {
|
||||
id, column, dependencies,
|
||||
title: id, description: "", priority: "normal", steps: [],
|
||||
createdAt: "2026-07-01T00:00:00.000Z",
|
||||
updatedAt: "2026-07-01T00:00:00.000Z",
|
||||
columnMovedAt: "2026-07-01T00:00:00.000Z",
|
||||
} as unknown as Task;
|
||||
}
|
||||
|
||||
it("weights a blocker with a DEFAULT-workflow active dependent (regression floor)", () => {
|
||||
const weights = buildUnblockWeightMap([t("FN-1", "in-review"), t("FN-2", "todo", ["FN-1"])]);
|
||||
expect(weights.get("FN-1")).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("weights a blocker whose dependent sits in a RENAMED active column", () => {
|
||||
// Against the enumeration this scored 0: `drafting` is in no legacy set, so
|
||||
// the blocker looked like it was unblocking nobody.
|
||||
const weights = buildUnblockWeightMap(
|
||||
[t("FN-1", "editorial-review"), t("FN-2", "drafting", ["FN-1"])],
|
||||
{ terminalColumns: new Set(["published", "shelved"]) },
|
||||
);
|
||||
expect(weights.get("FN-1")).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("does NOT weight a dependent sitting in the renamed workflow's terminal column", () => {
|
||||
const weights = buildUnblockWeightMap(
|
||||
[t("FN-1", "editorial-review"), t("FN-2", "published", ["FN-1"])],
|
||||
{ terminalColumns: new Set(["published", "shelved"]) },
|
||||
);
|
||||
expect(weights.get("FN-1") ?? 0).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -28,11 +28,57 @@ export interface ComputeBlockerFanoutOptions {
|
||||
nowMs?: number;
|
||||
highFanoutTodoThreshold?: number;
|
||||
staleHighFanoutAgeThresholdMs?: number;
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-27-21:50 (Phase B / U6):
|
||||
The workflow's TERMINAL columns (complete + archived). "Active" is defined by
|
||||
exclusion — not complete, not archived — which is what the concept always
|
||||
meant; the old `ACTIVE_COLUMNS` enumeration was a default-workflow-shaped
|
||||
stand-in that silently scored 0 active dependents for every column a custom
|
||||
workflow adds. Under-counting, not erroring: a blocker with real blocked
|
||||
dependents looked unblocking, and no test failed.
|
||||
Defaults to the legacy `{done, archived}` so existing callers are unchanged.
|
||||
Callers resolving the IR pass `[complete, archived]` from
|
||||
`resolveLifecycleColumns`.
|
||||
*/
|
||||
terminalColumns?: ReadonlySet<string>;
|
||||
/** The workflow's HOLD (capacity-wait) column. The fan-out metric counts cards
|
||||
* waiting for capacity, which is the hold role — `todo` is only the id the
|
||||
* built-in coding workflow gives it. Defaults to `"todo"`. */
|
||||
holdColumn?: string;
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-28-03:05 (PR #2470 review, P1):
|
||||
PLURAL form, for callers computing over a board that spans MORE THAN ONE
|
||||
workflow. `holdColumn` assumes a single vocabulary, which is wrong for the
|
||||
board-wide backlog-health reporter: a project running two workflows has two
|
||||
hold columns, and collapsing them to one silently drops every card held by the
|
||||
other. Takes precedence over `holdColumn` when supplied; when neither is given
|
||||
the legacy `"todo"` applies, so existing callers are byte-identical.
|
||||
*/
|
||||
holdColumns?: ReadonlySet<string>;
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-28-17:50 (PR #2479 review, P1):
|
||||
PER-TASK classification, and the only correct option on a multi-workflow board.
|
||||
|
||||
The set-shaped options above are board-wide, which silently assumes a column id
|
||||
means the same thing everywhere. It does not: an id is meaningful only RELATIVE
|
||||
TO ITS OWN WORKFLOW. When two workflows reuse an id for different roles — one
|
||||
calling `done` its hold column, another calling `done` terminal — any union of
|
||||
those sets marks that column BOTH held and terminal, and every card in it is
|
||||
misclassified regardless of which workflow it belongs to.
|
||||
|
||||
Supplying `classify` resolves each task against its own workflow, which makes
|
||||
that misclassification impossible by construction. It takes precedence over
|
||||
`terminalColumns`/`holdColumn(s)`; those remain for single-vocabulary callers
|
||||
(task-priority's unblock weighting) and as the legacy default.
|
||||
*/
|
||||
classify?: (task: Task) => { isHold: boolean; isTerminal: boolean };
|
||||
}
|
||||
|
||||
export const BLOCKER_ESCALATION_COLUMNS = new Set<Task["column"]>(["in-progress", "in-review"]);
|
||||
|
||||
const ACTIVE_COLUMNS = new Set<Task["column"]>(["triage", "todo", "in-progress", "in-review"]);
|
||||
/** Legacy default: the built-in coding workflow's terminal columns. Retained as
|
||||
* the fallback so an un-resolved caller keeps byte-identical behavior (R11). */
|
||||
const DEFAULT_TERMINAL_COLUMNS: ReadonlySet<string> = new Set(["done", "archived"]);
|
||||
|
||||
interface MutableEntry {
|
||||
dependentIds: string[];
|
||||
@@ -71,6 +117,12 @@ export function computeBlockerFanoutMap(
|
||||
const staleHighFanoutAgeThresholdMs =
|
||||
options.staleHighFanoutAgeThresholdMs ?? STALE_HIGH_FANOUT_BLOCKER_AGE_THRESHOLD_MS;
|
||||
|
||||
const terminalColumns = options.terminalColumns ?? DEFAULT_TERMINAL_COLUMNS;
|
||||
/* Plural wins; else the singular; else the legacy id. One resolved set so the
|
||||
two spellings cannot disagree downstream. */
|
||||
const holdColumns: ReadonlySet<string> =
|
||||
options.holdColumns ?? new Set([options.holdColumn ?? "todo"]);
|
||||
|
||||
const taskById = new Map(tasks.map((task) => [task.id, task]));
|
||||
const fanout = new Map<string, MutableEntry>();
|
||||
|
||||
@@ -92,8 +144,14 @@ export function computeBlockerFanoutMap(
|
||||
};
|
||||
|
||||
for (const task of tasks) {
|
||||
const active = ACTIVE_COLUMNS.has(task.column);
|
||||
const isTodo = task.column === "todo";
|
||||
/*
|
||||
Per-task classification wins when supplied (PR #2479 P1); otherwise fall back
|
||||
to the board-wide sets. Active is by EXCLUSION — not terminal — never by
|
||||
enumeration.
|
||||
*/
|
||||
const roles = options.classify?.(task);
|
||||
const active = roles ? !roles.isTerminal : !terminalColumns.has(task.column);
|
||||
const isTodo = roles ? roles.isHold : holdColumns.has(task.column);
|
||||
|
||||
for (const depId of task.dependencies ?? []) {
|
||||
if (!depId) continue;
|
||||
|
||||
@@ -33,8 +33,46 @@ export interface DependencyBlockedTodoReportContext {
|
||||
staleAgeMs?: number;
|
||||
minBlockedTodoCount?: number;
|
||||
maxGroups?: number;
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-28-02:40 (PR #2470 review, P1):
|
||||
The task's resolved lifecycle roles. `computeBlockerFanoutMap` already accepted
|
||||
these, but this report called it with NEITHER — so a renamed workflow silently
|
||||
fell back to the legacy sets. The failures pointed in opposite directions: a
|
||||
FINISHED blocker in a renamed terminal column counted as ACTIVE (over-reporting
|
||||
dead blockers), while dependents in a renamed hold column were not counted as
|
||||
blocked todos at all (under-reporting real ones).
|
||||
|
||||
Both default to the legacy values, so a caller that cannot resolve a workflow
|
||||
is byte-identical.
|
||||
*/
|
||||
/** Columns that end a task's life (`complete` + `archived` roles). */
|
||||
terminalColumns?: readonly string[];
|
||||
/** The capacity-wait column whose residents are the report's "blocked todos". */
|
||||
holdColumn?: string;
|
||||
/*
|
||||
PLURAL form. This report runs BOARD-WIDE, and a board may span more than one
|
||||
workflow — so there can be more than one hold column and collapsing them to one
|
||||
silently drops every card held by the other workflows. The engine reporter
|
||||
passes the union across the workflows actually present on the board.
|
||||
*/
|
||||
holdColumns?: readonly string[];
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-28-17:50 (PR #2479 review, P1):
|
||||
PER-TASK classification against each task's OWN workflow — the only correct
|
||||
option for this report, which is board-wide and therefore multi-workflow.
|
||||
Board-wide sets assume a column id means the same thing in every workflow; when
|
||||
two workflows reuse an id for different roles, a union marks that column both
|
||||
held and terminal and misclassifies every card in it. Takes precedence over the
|
||||
set-shaped options, which remain as the single-vocabulary/legacy fallback.
|
||||
*/
|
||||
classifyTask?: (task: Task) => { isHold: boolean; isTerminal: boolean };
|
||||
}
|
||||
|
||||
/* Legacy role ids — the builtin coding workflow's names, used when a caller
|
||||
cannot resolve the task's workflow. */
|
||||
const DEFAULT_REPORT_TERMINAL_COLUMNS: readonly string[] = ["done", "archived"];
|
||||
const DEFAULT_REPORT_HOLD_COLUMN = "todo";
|
||||
|
||||
export const DEFAULT_DEPENDENCY_BLOCKED_TODO_FRESH_MS = 30 * 60_000;
|
||||
export const DEFAULT_DEPENDENCY_BLOCKED_TODO_STALE_MS = 4 * 60 * 60_000;
|
||||
export const DEFAULT_DEPENDENCY_BLOCKED_TODO_MIN_COUNT = 1;
|
||||
@@ -71,8 +109,33 @@ export function computeDependencyBlockedTodoReport(
|
||||
context: DependencyBlockedTodoReportContext = {},
|
||||
): DependencyBlockedTodoReport {
|
||||
const { now, freshMs, staleMs, minBlockedTodoCount, maxGroups } = sanitizeContext(context);
|
||||
const blockerFanout = computeBlockerFanoutMap(tasks, maxAutoMergeRetries, { nowMs: now });
|
||||
const todoTaskIds = new Set(tasks.filter((task) => task.column === "todo").map((task) => task.id));
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-28-02:40 (PR #2470 review, P1):
|
||||
Thread the resolved roles into the fan-out AND into this function's own two
|
||||
literals below. Threading only the fan-out would leave the `todoTaskIds` filter
|
||||
and the terminal-blocker skip on legacy ids, so a renamed workflow would still
|
||||
report nothing — a fix that looks complete and changes no outcome.
|
||||
*/
|
||||
const terminalColumns = context.terminalColumns ?? DEFAULT_REPORT_TERMINAL_COLUMNS;
|
||||
const holdColumns = new Set(
|
||||
context.holdColumns ?? [context.holdColumn ?? DEFAULT_REPORT_HOLD_COLUMN],
|
||||
);
|
||||
const terminalColumnSet = new Set(terminalColumns);
|
||||
|
||||
const classify = context.classifyTask;
|
||||
const blockerFanout = computeBlockerFanoutMap(tasks, maxAutoMergeRetries, {
|
||||
nowMs: now,
|
||||
terminalColumns: terminalColumnSet,
|
||||
holdColumns,
|
||||
classify,
|
||||
});
|
||||
/* Held-ness is per task when a classifier is supplied; the set is the legacy
|
||||
single-vocabulary fallback. */
|
||||
const isHold = (task: Task): boolean =>
|
||||
classify ? classify(task).isHold : holdColumns.has(task.column);
|
||||
const isTerminal = (task: Task): boolean =>
|
||||
classify ? classify(task).isTerminal : terminalColumnSet.has(task.column);
|
||||
const todoTaskIds = new Set(tasks.filter(isHold).map((task) => task.id));
|
||||
const taskById = new Map(tasks.map((task) => [task.id, task]));
|
||||
|
||||
const groups: DependencyBlockedTodoGroup[] = [];
|
||||
@@ -83,7 +146,8 @@ export function computeDependencyBlockedTodoReport(
|
||||
}
|
||||
|
||||
const blocker = taskById.get(blockerId);
|
||||
if (!blocker || blocker.column === "done" || blocker.column === "archived") {
|
||||
// Terminal by the blocker's OWN workflow, never by a board-wide union.
|
||||
if (!blocker || isTerminal(blocker)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
@@ -13,6 +13,19 @@ export interface StalePausedTodoSignal {
|
||||
}
|
||||
|
||||
export interface StalePausedTodoContext {
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-27-21:35 (Phase B / U6):
|
||||
The lifecycle role this signal is about is HOLD (the capacity-wait column), not
|
||||
the id `todo` — that is merely what the built-in coding workflow calls it. A
|
||||
workflow naming its hold column `drafting` has the identical stall condition,
|
||||
and before this parameter existed the guard silently stopped matching for it:
|
||||
no error, no failing test, just a recovery signal that never fired.
|
||||
|
||||
Defaults to `"todo"` so every existing caller is byte-identical (R11 keeps
|
||||
`todo` a legal column id). Callers that can resolve the task's workflow pass
|
||||
`resolveLifecycleColumns(ir).hold` instead.
|
||||
*/
|
||||
holdColumn?: string;
|
||||
now?: number;
|
||||
thresholdMs?: number;
|
||||
engineActiveSinceMs?: number;
|
||||
@@ -25,7 +38,8 @@ export function getStalePausedTodoSignal(
|
||||
task: Pick<Task, "column" | "paused" | "columnMovedAt" | "updatedAt" | "pausedReason" | "pausedByAgentId">,
|
||||
context: StalePausedTodoContext = {},
|
||||
): StalePausedTodoSignal | undefined {
|
||||
if (task.column !== "todo" || task.paused !== true) return undefined;
|
||||
const holdColumn = context.holdColumn ?? "todo";
|
||||
if (task.column !== holdColumn || task.paused !== true) return undefined;
|
||||
|
||||
const thresholdMs = context.thresholdMs ?? DEFAULT_STALE_PAUSED_TODO_THRESHOLD_MS;
|
||||
if (!Number.isFinite(thresholdMs) || thresholdMs <= 0) return undefined;
|
||||
|
||||
@@ -87,14 +87,30 @@ export function sortTasksByPriorityThenAgeAndId<T extends TaskPrioritySortable>(
|
||||
}
|
||||
|
||||
const FANOUT_SECONDARY_WEIGHT_MULTIPLIER = 1_000_000;
|
||||
const UNBLOCK_ACTIVE_COLUMNS = new Set<Task["column"]>(["triage", "todo", "in-progress", "in-review"]);
|
||||
const DONE_COLUMNS = new Set<Task["column"]>(["done", "archived"]);
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-27-22:10 (Phase B / U6):
|
||||
`UNBLOCK_ACTIVE_COLUMNS` is DELETED. It enumerated the default workflow's
|
||||
non-terminal columns, which is the same concept `DONE_COLUMNS` already expressed
|
||||
by exclusion two lines below — one idea encoded twice, and the two halves
|
||||
disagreed for any column a custom workflow adds: dependency counting treated a
|
||||
`drafting` card as unmet (correct) while the active check treated it as inactive
|
||||
(wrong), so the blocker's unblock weight silently scored 0. Both halves now read
|
||||
the single terminal set.
|
||||
*/
|
||||
const DEFAULT_TERMINAL_COLUMNS: ReadonlySet<string> = new Set(["done", "archived"]);
|
||||
|
||||
export interface BuildUnblockWeightMapOptions {
|
||||
maxAutoMergeRetries?: ProjectSettings["maxAutoMergeRetries"];
|
||||
/** The workflow's terminal columns (complete + archived). Defaults to the
|
||||
* built-in `{done, archived}` so existing callers are unchanged (R11). */
|
||||
terminalColumns?: ReadonlySet<string>;
|
||||
}
|
||||
|
||||
function countUnmetDependencies(task: Task, taskById: Map<string, Task>): number {
|
||||
function countUnmetDependencies(
|
||||
task: Task,
|
||||
taskById: Map<string, Task>,
|
||||
terminalColumns: ReadonlySet<string>,
|
||||
): number {
|
||||
let unmet = 0;
|
||||
for (const dependencyId of task.dependencies ?? []) {
|
||||
const dependency = taskById.get(dependencyId);
|
||||
@@ -102,7 +118,7 @@ function countUnmetDependencies(task: Task, taskById: Map<string, Task>): number
|
||||
unmet += 1;
|
||||
continue;
|
||||
}
|
||||
if (DONE_COLUMNS.has(dependency.column)) {
|
||||
if (terminalColumns.has(dependency.column)) {
|
||||
continue;
|
||||
}
|
||||
unmet += 1;
|
||||
@@ -115,7 +131,8 @@ export function buildUnblockWeightMap(
|
||||
options: BuildUnblockWeightMapOptions = {},
|
||||
): Map<string, number> {
|
||||
const taskList = [...tasks];
|
||||
const fanout = computeBlockerFanoutMap(taskList, options.maxAutoMergeRetries ?? 0);
|
||||
const terminalColumns = options.terminalColumns ?? DEFAULT_TERMINAL_COLUMNS;
|
||||
const fanout = computeBlockerFanoutMap(taskList, options.maxAutoMergeRetries ?? 0, { terminalColumns });
|
||||
const taskById = new Map(taskList.map((task) => [task.id, task]));
|
||||
const weights = new Map<string, number>();
|
||||
|
||||
@@ -125,11 +142,12 @@ export function buildUnblockWeightMap(
|
||||
|
||||
for (const dependentId of entry.dependencyDependentIds) {
|
||||
const dependent = taskById.get(dependentId);
|
||||
if (!dependent || !UNBLOCK_ACTIVE_COLUMNS.has(dependent.column)) {
|
||||
// Active by exclusion — the same terminal set the dependency count uses.
|
||||
if (!dependent || terminalColumns.has(dependent.column)) {
|
||||
continue;
|
||||
}
|
||||
secondaryActiveDependentCount += 1;
|
||||
if (countUnmetDependencies(dependent, taskById) === 1) {
|
||||
if (countUnmetDependencies(dependent, taskById, terminalColumns) === 1) {
|
||||
primaryOnlyUnmetCount += 1;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -20,6 +20,10 @@ import {getAgentLogFilePath} from "../agent-log-file-store.js";
|
||||
import {getInReviewStalledSignal} from "../in-review-stalled.js";
|
||||
import {getStalePausedReviewSignal} from "../stale-paused-review.js";
|
||||
import {getStalePausedTodoSignal} from "../stale-paused-todo.js";
|
||||
import {resolveLifecycleColumns} from "../workflow-lifecycle-traits.js";
|
||||
import {resolveWorkflowIrForTask} from "../workflow-ir-resolver.js";
|
||||
import type {WorkflowIr} from "../workflow-ir-types.js";
|
||||
|
||||
import {getTaskAgeStalenessSignal, type TaskAgeStalenessThresholds} from "../task-age-staleness.js";
|
||||
import {detectStalledReview} from "../stalled-review-detector.js";
|
||||
import {computeRetrySummary} from "../retry-summary.js";
|
||||
@@ -106,6 +110,30 @@ import {
|
||||
searchArchivedTasks,
|
||||
} from "../async-archive-db.js";
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-28-04:00 (PR #2470 review, P1):
|
||||
Resolve a task's HOLD column for the stalePausedTodo badge. B1 gave
|
||||
`getStalePausedTodoSignal` a `holdColumn` parameter, but both hydration sites
|
||||
here omitted it — so the guard still compared against the literal "todo" and the
|
||||
dashboard badge was silent for a paused card in a renamed hold column.
|
||||
|
||||
Fail-soft to "todo": this is read-path badge hydration, so a workflow lookup
|
||||
failure must degrade to today's behavior, never break a board list. The cache is
|
||||
caller-owned so a list hydration reads one IR per workflow rather than per card.
|
||||
*/
|
||||
async function resolveHoldColumnForTask(
|
||||
store: TaskStore,
|
||||
taskId: string,
|
||||
cache?: Map<string, WorkflowIr>,
|
||||
): Promise<string> {
|
||||
try {
|
||||
const lifecycle = resolveLifecycleColumns(await resolveWorkflowIrForTask(store, taskId, cache));
|
||||
return lifecycle?.hold ?? "todo";
|
||||
} catch {
|
||||
return "todo";
|
||||
}
|
||||
}
|
||||
|
||||
export async function getTaskImpl(store: TaskStore, id: string, options?: { activityLogLimit?: number; includeDeleted?: boolean }): Promise<TaskDetail> {
|
||||
return store.withTaskLock(id, async () => {
|
||||
// FNXC:RuntimePersistenceAsync 2026-06-24-10:50:
|
||||
@@ -279,6 +307,15 @@ export async function listTasksImpl(store: TaskStore, options?: { limit?: number
|
||||
const now = Date.now();
|
||||
const settings = await store.getSettingsFast();
|
||||
const mergeQueuedTaskIds = await store.getMergeQueuedTaskIdsAsync();
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-28-18:05 (PR #2479 review, P2):
|
||||
ONE IR cache for the whole list pass. Without it, every paused row resolved
|
||||
its workflow independently, repeating workflow-definition and prompt-override
|
||||
reads for a board with many paused cards on the same workflow. Caller-owned by
|
||||
design (U1's `resolveTaskLifecycleColumns` takes the cache for exactly this),
|
||||
so reads scale with the number of WORKFLOWS, not the number of cards.
|
||||
*/
|
||||
const listPassIrCache = new Map<string, WorkflowIr>();
|
||||
/*
|
||||
* FNXC:SqliteFinalRemoval 2026-06-26-10:30:
|
||||
* Compute staleness thresholds once for the whole list pass, mirroring
|
||||
@@ -331,6 +368,10 @@ export async function listTasksImpl(store: TaskStore, options?: { limit?: number
|
||||
task.stalePausedTodo = getStalePausedTodoSignal(task, {
|
||||
now,
|
||||
thresholdMs: settings.stalePausedTodoThresholdMs,
|
||||
// Paused-only (the signal is a no-op otherwise), sharing the list-pass
|
||||
// IR cache so one workflow is read once per pass, not once per card.
|
||||
holdColumn:
|
||||
task.paused === true ? await resolveHoldColumnForTask(store, task.id, listPassIrCache) : undefined,
|
||||
engineActiveSinceMs: settings.engineActiveSinceMs,
|
||||
engineActivationGraceMs: settings.engineActivationGraceMs,
|
||||
});
|
||||
@@ -450,7 +491,30 @@ export async function listTasksModifiedSinceImpl(store: TaskStore, since: string
|
||||
.limit(resolvedLimit + 1);
|
||||
const hasMore = pgRows.length > resolvedLimit;
|
||||
const mergeQueuedTaskIds = await store.getMergeQueuedTaskIdsAsync();
|
||||
const tasks = pgRows.slice(0, resolvedLimit).map((pgRow) => {
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-28-04:00 (PR #2470 review, P1):
|
||||
Pre-resolve hold columns for the PAUSED rows only, before the synchronous
|
||||
hydration map below.
|
||||
|
||||
Two constraints shape this. The map is sync, so an await cannot go inside it
|
||||
without converting a hot board-list path to Promise.all — a restructure this
|
||||
fix does not need. And `getStalePausedTodoSignal` is a no-op for a card that
|
||||
is not paused, so resolving for every row would buy nothing at real cost:
|
||||
paused cards are a small minority of a board, and the shared `irCache` means
|
||||
those few resolve one IR per workflow. A board with no paused cards does zero
|
||||
extra work.
|
||||
*/
|
||||
const pageRows = pgRows.slice(0, resolvedLimit);
|
||||
const holdColumnByTaskId = new Map<string, string>();
|
||||
{
|
||||
const irCache = new Map<string, WorkflowIr>();
|
||||
for (const pgRow of pageRows) {
|
||||
const row = store.pgRowToTaskRow(pgRow);
|
||||
if (store.rowToTask(row).paused !== true) continue;
|
||||
holdColumnByTaskId.set(row.id, await resolveHoldColumnForTask(store, row.id, irCache));
|
||||
}
|
||||
}
|
||||
const tasks = pageRows.map((pgRow) => {
|
||||
const task = store.rowToTask(store.pgRowToTaskRow(pgRow));
|
||||
const isMergeQueued = mergeQueuedTaskIds.has(task.id);
|
||||
/*
|
||||
@@ -488,6 +552,7 @@ export async function listTasksModifiedSinceImpl(store: TaskStore, since: string
|
||||
task.stalePausedTodo = getStalePausedTodoSignal(task, {
|
||||
now,
|
||||
thresholdMs: settings.stalePausedTodoThresholdMs,
|
||||
holdColumn: holdColumnByTaskId.get(task.id),
|
||||
engineActiveSinceMs: settings.engineActiveSinceMs,
|
||||
engineActivationGraceMs: settings.engineActivationGraceMs,
|
||||
});
|
||||
|
||||
@@ -0,0 +1,223 @@
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-28-17:40 (PR #2479 review, P1):
|
||||
|
||||
BOARD-WIDE ROLE UNIONS MISCLASSIFY TASKS.
|
||||
|
||||
The previous fix resolved lifecycle roles across every workflow on the board and
|
||||
UNIONED them into one `holdColumns` / `terminalColumns` pair. That is wrong the
|
||||
moment two workflows reuse a column ID for DIFFERENT roles — and it is wrong for
|
||||
the reason this whole program exists: **a column id only means something relative
|
||||
to its own workflow.** A board-wide union quietly re-assumes ids are globally
|
||||
meaningful, which is the precise assumption being removed.
|
||||
|
||||
The concrete break, and the fixture below: workflow A calls its HOLD column
|
||||
`done`; workflow B calls its TERMINAL column `done`. Under a union,
|
||||
`holdColumns` and `terminalColumns` BOTH contain `done`, so every card in a
|
||||
column named `done` is simultaneously "held" and "terminal" regardless of which
|
||||
workflow it belongs to. Dependents get counted as blocked while the blocker
|
||||
sitting beside them is discarded as finished — from one ambiguous id.
|
||||
|
||||
The fix is per-task classification: each task is classified against ITS OWN
|
||||
workflow, which makes the misclassification impossible by construction rather
|
||||
than detected after the fact. It also removes the repeated workflow-definition
|
||||
reads (the sibling P2), because one shared IR cache serves the whole pass.
|
||||
|
||||
These tests were written FIRST and observed FAILING against the union.
|
||||
*/
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { Task, TaskStore, WorkflowIr } from "@fusion/core";
|
||||
|
||||
import { DependencyBlockedTodoReporter } from "../dependency-blocked-todo-reporter.js";
|
||||
|
||||
const NOW = Date.parse("2026-05-18T12:00:00.000Z");
|
||||
/** Old enough to bucket "stale", so significance gating never masks a miss. */
|
||||
const MOVED_AT = new Date(NOW - 5 * 60 * 60_000).toISOString();
|
||||
|
||||
function task(over: Partial<Task> = {}): Task {
|
||||
return {
|
||||
id: "FN-1",
|
||||
title: "t",
|
||||
description: "test",
|
||||
column: "todo",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
paused: false,
|
||||
blockedBy: "",
|
||||
overlapBlockedBy: "",
|
||||
log: [],
|
||||
createdAt: MOVED_AT,
|
||||
updatedAt: MOVED_AT,
|
||||
columnMovedAt: MOVED_AT,
|
||||
...over,
|
||||
} as Task;
|
||||
}
|
||||
|
||||
/**
|
||||
* `done` is the HOLD column here — a workflow that finished naming its columns
|
||||
* differently. Nothing about this is exotic; ids are workflow-local by design.
|
||||
*/
|
||||
function holdIsDoneIr(): WorkflowIr {
|
||||
return {
|
||||
version: "v2",
|
||||
id: "wf-hold-done",
|
||||
nodes: [],
|
||||
edges: [],
|
||||
columns: [
|
||||
{ id: "done", name: "done", traits: [{ trait: "hold", config: { release: "capacity" } }] },
|
||||
{ id: "building", name: "building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
|
||||
{ id: "shipped", name: "shipped", traits: [{ trait: "complete" }] },
|
||||
],
|
||||
} as unknown as WorkflowIr;
|
||||
}
|
||||
|
||||
/** The builtin shape, where `done` is TERMINAL. */
|
||||
function doneIsTerminalIr(): WorkflowIr {
|
||||
return {
|
||||
version: "v2",
|
||||
id: "wf-done-terminal",
|
||||
nodes: [],
|
||||
edges: [],
|
||||
columns: [
|
||||
{ id: "todo", name: "todo", traits: [{ trait: "hold", config: { release: "capacity" } }] },
|
||||
{ id: "in-progress", name: "in-progress", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
|
||||
{ id: "done", name: "done", traits: [{ trait: "complete" }] },
|
||||
],
|
||||
} as unknown as WorkflowIr;
|
||||
}
|
||||
|
||||
function createStore(
|
||||
tasks: Task[],
|
||||
workflowByTask: Record<string, string>,
|
||||
irByWorkflow: Record<string, WorkflowIr>,
|
||||
) {
|
||||
const upsertInsight = vi.fn().mockResolvedValue(undefined);
|
||||
const getWorkflowDefinition = vi.fn(async (id: string) =>
|
||||
irByWorkflow[id] ? { ir: irByWorkflow[id] } : null,
|
||||
);
|
||||
const store = {
|
||||
getSettings: vi.fn().mockResolvedValue({ maxAutoMergeRetries: 3 }),
|
||||
listTasks: vi.fn().mockResolvedValue(tasks),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
getInsightStore: vi.fn(() => ({ upsertInsight, listInsights: vi.fn().mockResolvedValue([]) })),
|
||||
getTaskWorkflowSelectionAsync: vi.fn(async (id: string) => ({ workflowId: workflowByTask[id], stepIds: [] })),
|
||||
getTaskWorkflowSelection: vi.fn((id: string) => ({ workflowId: workflowByTask[id], stepIds: [] })),
|
||||
getWorkflowDefinition,
|
||||
} as unknown as TaskStore;
|
||||
return { store, upsertInsight, getWorkflowDefinition };
|
||||
}
|
||||
|
||||
function reporter(store: TaskStore) {
|
||||
return new DependencyBlockedTodoReporter({
|
||||
store,
|
||||
projectId: "p1",
|
||||
logger: { warn: vi.fn(), error: vi.fn() },
|
||||
now: () => NOW,
|
||||
});
|
||||
}
|
||||
|
||||
describe("dependency-blocked report classifies each task by ITS OWN workflow", () => {
|
||||
it("does not treat a blocker as finished because ANOTHER workflow calls that column terminal", async () => {
|
||||
/*
|
||||
The P1, minimally. Every card belongs to `wf-hold-done`, where `done` is the
|
||||
HOLD column. A second workflow on the board calls `done` terminal.
|
||||
|
||||
Under the union, `terminalColumns` contains `done`, so the blocker resting in
|
||||
`done` is discarded as finished and the report goes silent — even though for
|
||||
ITS workflow that column means "waiting for capacity".
|
||||
*/
|
||||
const tasks = [
|
||||
task({ id: "BLOCKER", column: "done" }),
|
||||
task({ id: "DEP-1", column: "done", dependencies: ["BLOCKER"] }),
|
||||
task({ id: "DEP-2", column: "done", dependencies: ["BLOCKER"] }),
|
||||
task({ id: "DEP-3", column: "done", dependencies: ["BLOCKER"] }),
|
||||
// A card from the OTHER workflow, whose `done` genuinely is terminal.
|
||||
task({ id: "OTHER", column: "done" }),
|
||||
];
|
||||
const byTask: Record<string, string> = {
|
||||
BLOCKER: "wf-hold-done",
|
||||
"DEP-1": "wf-hold-done",
|
||||
"DEP-2": "wf-hold-done",
|
||||
"DEP-3": "wf-hold-done",
|
||||
OTHER: "wf-done-terminal",
|
||||
};
|
||||
const { store, upsertInsight } = createStore(tasks, byTask, {
|
||||
"wf-hold-done": holdIsDoneIr(),
|
||||
"wf-done-terminal": doneIsTerminalIr(),
|
||||
});
|
||||
|
||||
const result = await reporter(store).report();
|
||||
|
||||
expect(result.alerted).toBe(true);
|
||||
const payload = JSON.parse(upsertInsight.mock.calls[0][1].content);
|
||||
expect(payload.groups.map((g: { blockerId: string }) => g.blockerId)).toEqual(["BLOCKER"]);
|
||||
expect(payload.groups[0].blockedTodoIds).toEqual(["DEP-1", "DEP-2", "DEP-3"]);
|
||||
});
|
||||
|
||||
it("does not treat a finished card as held because ANOTHER workflow calls that column hold", async () => {
|
||||
/*
|
||||
The mirror image, and the half a one-directional fix would miss. Every card
|
||||
belongs to `wf-done-terminal`, where `done` is TERMINAL. Under the union
|
||||
`holdColumns` contains `done`, so finished cards are counted as blocked
|
||||
todos and the report invents blockage that does not exist.
|
||||
*/
|
||||
const tasks = [
|
||||
task({ id: "BLOCKER", column: "in-progress" }),
|
||||
task({ id: "DEP-1", column: "done", dependencies: ["BLOCKER"] }),
|
||||
task({ id: "DEP-2", column: "done", dependencies: ["BLOCKER"] }),
|
||||
task({ id: "DEP-3", column: "done", dependencies: ["BLOCKER"] }),
|
||||
task({ id: "OTHER", column: "done" }),
|
||||
];
|
||||
const byTask: Record<string, string> = {
|
||||
BLOCKER: "wf-done-terminal",
|
||||
"DEP-1": "wf-done-terminal",
|
||||
"DEP-2": "wf-done-terminal",
|
||||
"DEP-3": "wf-done-terminal",
|
||||
OTHER: "wf-hold-done",
|
||||
};
|
||||
const { store } = createStore(tasks, byTask, {
|
||||
"wf-hold-done": holdIsDoneIr(),
|
||||
"wf-done-terminal": doneIsTerminalIr(),
|
||||
});
|
||||
|
||||
const result = await reporter(store).report();
|
||||
|
||||
// Those dependents are DONE in their own workflow — nothing is blocked.
|
||||
expect(result.alerted).toBe(false);
|
||||
expect(result.reason).toBe("no-blocked-groups");
|
||||
});
|
||||
|
||||
it("reads one workflow definition per WORKFLOW, not per task (P2)", async () => {
|
||||
/*
|
||||
The sibling P2, which the per-task fix resolves as a side effect: a shared IR
|
||||
cache across the pass means workflow-definition reads scale with the number of
|
||||
WORKFLOWS, not the number of cards.
|
||||
*/
|
||||
const tasks = Array.from({ length: 12 }, (_, i) =>
|
||||
task({ id: `FN-${i}`, column: i === 0 ? "in-progress" : "todo", dependencies: i === 0 ? [] : ["FN-0"] }),
|
||||
);
|
||||
const byTask = Object.fromEntries(tasks.map((t) => [t.id, "wf-done-terminal"]));
|
||||
const { store, getWorkflowDefinition } = createStore(tasks, byTask, {
|
||||
"wf-done-terminal": doneIsTerminalIr(),
|
||||
});
|
||||
|
||||
await reporter(store).report();
|
||||
|
||||
expect(getWorkflowDefinition.mock.calls.length).toBeLessThanOrEqual(1);
|
||||
});
|
||||
|
||||
it("still degrades to legacy behavior when no workflow resolves", async () => {
|
||||
const tasks = [
|
||||
task({ id: "BLOCKER", column: "in-progress" }),
|
||||
task({ id: "DEP-1", column: "todo", dependencies: ["BLOCKER"] }),
|
||||
task({ id: "DEP-2", column: "todo", dependencies: ["BLOCKER"] }),
|
||||
task({ id: "DEP-3", column: "todo", dependencies: ["BLOCKER"] }),
|
||||
];
|
||||
const { store } = createStore(tasks, {}, {});
|
||||
|
||||
const result = await reporter(store).report();
|
||||
|
||||
expect(result.alerted).toBe(true);
|
||||
expect(result.groupCount).toBe(1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,202 @@
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-28-03:20 (PR #2470 review, P1):
|
||||
|
||||
End-to-end half of the "convertible rather than converted" fix. B1 gave
|
||||
`computeBlockerFanoutMap` resolved `terminalColumns`/`holdColumn` parameters, but
|
||||
this reporter — the only production caller of the report — passed NEITHER, so a
|
||||
renamed workflow still fell through to the legacy {done,archived}/"todo" sets.
|
||||
Fixing the module without the caller changes no observable behavior, which is
|
||||
precisely the defect Greptile flagged.
|
||||
|
||||
Asserted here at the REPORTER level rather than the pure-function level, because
|
||||
that is where the bug actually lived: the pure-function tests were already green
|
||||
before this fix.
|
||||
|
||||
Also covers the board-wide multi-workflow case. This report runs over the WHOLE
|
||||
board, so a project running two workflows has two hold columns; the reporter
|
||||
therefore passes a UNION of roles, not one vocabulary. A single-vocabulary fix
|
||||
would pass the renamed test below and silently drop every card belonging to the
|
||||
other workflow.
|
||||
*/
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { Task, TaskStore, WorkflowIr } from "@fusion/core";
|
||||
|
||||
import { DependencyBlockedTodoReporter } from "../dependency-blocked-todo-reporter.js";
|
||||
|
||||
const NOW = Date.parse("2026-05-18T12:00:00.000Z");
|
||||
/** Old enough to bucket as "stale", so significance gating never masks a miss. */
|
||||
const MOVED_AT = new Date(NOW - 5 * 60 * 60_000).toISOString();
|
||||
|
||||
function task(over: Partial<Task> = {}): Task {
|
||||
return {
|
||||
id: "FN-1",
|
||||
title: "t",
|
||||
description: "test",
|
||||
column: "todo",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
paused: false,
|
||||
blockedBy: "",
|
||||
overlapBlockedBy: "",
|
||||
log: [],
|
||||
createdAt: MOVED_AT,
|
||||
updatedAt: MOVED_AT,
|
||||
columnMovedAt: MOVED_AT,
|
||||
...over,
|
||||
} as Task;
|
||||
}
|
||||
|
||||
function ir(id: string, names: { hold: string; wip: string; complete: string }): WorkflowIr {
|
||||
return {
|
||||
version: "v2",
|
||||
id,
|
||||
nodes: [],
|
||||
edges: [],
|
||||
columns: [
|
||||
{ id: names.hold, label: "Hold", traits: [{ trait: "hold", config: { release: "capacity" } }] },
|
||||
{ id: names.wip, label: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
|
||||
{ id: names.complete, label: "Complete", traits: [{ trait: "complete" }] },
|
||||
],
|
||||
} as unknown as WorkflowIr;
|
||||
}
|
||||
|
||||
const RENAMED = { hold: "queued", wip: "building", complete: "published" };
|
||||
const DEFAULTS = { hold: "todo", wip: "in-progress", complete: "done" };
|
||||
|
||||
/**
|
||||
* @param workflowByTask taskId → workflow id, so a board can span workflows.
|
||||
* @param irByWorkflow workflow id → its IR.
|
||||
*/
|
||||
function createStore(
|
||||
tasks: Task[],
|
||||
workflowByTask: Record<string, string>,
|
||||
irByWorkflow: Record<string, WorkflowIr>,
|
||||
): { store: TaskStore; upsertInsight: ReturnType<typeof vi.fn> } {
|
||||
const upsertInsight = vi.fn().mockResolvedValue(undefined);
|
||||
const store = {
|
||||
getSettings: vi.fn().mockResolvedValue({ maxAutoMergeRetries: 3 }),
|
||||
listTasks: vi.fn().mockResolvedValue(tasks),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
getInsightStore: vi.fn(() => ({ upsertInsight, listInsights: vi.fn().mockResolvedValue([]) })),
|
||||
getTaskWorkflowSelectionAsync: vi.fn(async (id: string) => ({
|
||||
workflowId: workflowByTask[id] ?? "wf-default",
|
||||
stepIds: [],
|
||||
})),
|
||||
getTaskWorkflowSelection: vi.fn((id: string) => ({
|
||||
workflowId: workflowByTask[id] ?? "wf-default",
|
||||
stepIds: [],
|
||||
})),
|
||||
getWorkflowDefinition: vi.fn(async (id: string) =>
|
||||
irByWorkflow[id] ? { ir: irByWorkflow[id] } : null,
|
||||
),
|
||||
} as unknown as TaskStore;
|
||||
return { store, upsertInsight };
|
||||
}
|
||||
|
||||
function reporter(store: TaskStore) {
|
||||
return new DependencyBlockedTodoReporter({
|
||||
store,
|
||||
projectId: "p1",
|
||||
logger: { warn: vi.fn(), error: vi.fn() },
|
||||
now: () => NOW,
|
||||
});
|
||||
}
|
||||
|
||||
/** Blocker + 3 dependents held in `hold`, enough to clear the significance gate. */
|
||||
function blockedBoard(hold: string, blockerColumn: string): Task[] {
|
||||
return [
|
||||
task({ id: "BLOCKER", column: blockerColumn }),
|
||||
task({ id: "DEP-1", column: hold, dependencies: ["BLOCKER"] }),
|
||||
task({ id: "DEP-2", column: hold, dependencies: ["BLOCKER"] }),
|
||||
task({ id: "DEP-3", column: hold, dependencies: ["BLOCKER"] }),
|
||||
];
|
||||
}
|
||||
|
||||
describe("DependencyBlockedTodoReporter under a renamed column vocabulary", () => {
|
||||
it("reports cards blocked in a RENAMED hold column", async () => {
|
||||
/* The under-reporting half: before the fix, `queued` residents were not
|
||||
counted as blocked todos at all, so the reporter alerted on nothing. */
|
||||
const tasks = blockedBoard(RENAMED.hold, RENAMED.wip);
|
||||
const workflows = Object.fromEntries(tasks.map((t) => [t.id, "wf-renamed"]));
|
||||
const { store, upsertInsight } = createStore(tasks, workflows, {
|
||||
"wf-renamed": ir("wf-renamed", RENAMED),
|
||||
});
|
||||
|
||||
const result = await reporter(store).report();
|
||||
|
||||
expect(result.alerted).toBe(true);
|
||||
expect(result.groupCount).toBe(1);
|
||||
const payload = JSON.parse(upsertInsight.mock.calls[0][1].content);
|
||||
expect(payload.totalBlockedTodoCount).toBe(3);
|
||||
expect(payload.groups[0].blockedTodoIds).toEqual(["DEP-1", "DEP-2", "DEP-3"]);
|
||||
});
|
||||
|
||||
it("does NOT report a blocker that already reached a RENAMED terminal column", async () => {
|
||||
/* The over-reporting half, in the opposite direction: `published` is not in
|
||||
the legacy terminal set, so a finished blocker looked live. */
|
||||
const tasks = blockedBoard(RENAMED.hold, RENAMED.complete);
|
||||
const workflows = Object.fromEntries(tasks.map((t) => [t.id, "wf-renamed"]));
|
||||
const { store } = createStore(tasks, workflows, { "wf-renamed": ir("wf-renamed", RENAMED) });
|
||||
|
||||
const result = await reporter(store).report();
|
||||
|
||||
expect(result.alerted).toBe(false);
|
||||
expect(result.reason).toBe("no-blocked-groups");
|
||||
});
|
||||
|
||||
it("covers BOTH workflows on a board that mixes a renamed and a builtin one", async () => {
|
||||
/* The union case. A single-vocabulary fix reports one group and silently
|
||||
drops the other workflow's blocked cards entirely. */
|
||||
const tasks = [
|
||||
...blockedBoard(RENAMED.hold, RENAMED.wip),
|
||||
...blockedBoard(DEFAULTS.hold, DEFAULTS.wip).map((t) =>
|
||||
task({ ...t, id: `L-${t.id}`, dependencies: t.dependencies?.length ? ["L-BLOCKER"] : [] }),
|
||||
),
|
||||
];
|
||||
const workflows: Record<string, string> = {};
|
||||
for (const t of tasks) workflows[t.id] = t.id.startsWith("L-") ? "wf-default" : "wf-renamed";
|
||||
|
||||
const { store, upsertInsight } = createStore(tasks, workflows, {
|
||||
"wf-renamed": ir("wf-renamed", RENAMED),
|
||||
"wf-default": ir("wf-default", DEFAULTS),
|
||||
});
|
||||
|
||||
const result = await reporter(store).report();
|
||||
|
||||
expect(result.alerted).toBe(true);
|
||||
expect(result.groupCount).toBe(2);
|
||||
const payload = JSON.parse(upsertInsight.mock.calls[0][1].content);
|
||||
expect(payload.groups.map((g: { blockerId: string }) => g.blockerId).sort()).toEqual([
|
||||
"BLOCKER",
|
||||
"L-BLOCKER",
|
||||
]);
|
||||
expect(payload.totalBlockedTodoCount).toBe(6);
|
||||
});
|
||||
|
||||
it("still reports a builtin-only board identically (regression floor)", async () => {
|
||||
const tasks = blockedBoard(DEFAULTS.hold, DEFAULTS.wip);
|
||||
const workflows = Object.fromEntries(tasks.map((t) => [t.id, "wf-default"]));
|
||||
const { store, upsertInsight } = createStore(tasks, workflows, {
|
||||
"wf-default": ir("wf-default", DEFAULTS),
|
||||
});
|
||||
|
||||
const result = await reporter(store).report();
|
||||
|
||||
expect(result.alerted).toBe(true);
|
||||
const payload = JSON.parse(upsertInsight.mock.calls[0][1].content);
|
||||
expect(payload.groups[0].blockedTodoIds).toEqual(["DEP-1", "DEP-2", "DEP-3"]);
|
||||
});
|
||||
|
||||
it("degrades to the legacy sets when no workflow resolves", async () => {
|
||||
/* Conservative fallback: an unresolvable board must behave exactly as it did
|
||||
before this threading rather than dropping columns from the union. */
|
||||
const tasks = blockedBoard(DEFAULTS.hold, DEFAULTS.wip);
|
||||
const { store } = createStore(tasks, {}, {});
|
||||
|
||||
const result = await reporter(store).report();
|
||||
|
||||
expect(result.alerted).toBe(true);
|
||||
expect(result.groupCount).toBe(1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,192 @@
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-28-03:45 (PR #2470 review, P1):
|
||||
|
||||
`getStalePausedTodoSignal` gained a `holdColumn` parameter in B1, but EVERY
|
||||
production caller omitted it — so the guard still compared against the literal
|
||||
"todo" and a paused card in a renamed hold column produced no signal at all. The
|
||||
operator-visible consequence: the dashboard badge and the self-healing log are
|
||||
both silent for a stalled card, which is indistinguishable from a healthy board.
|
||||
|
||||
This sweep needed TWO fixes, and the second is the one a careless patch misses:
|
||||
|
||||
1. the signal call omitted `holdColumn`;
|
||||
2. the QUERY itself was `listTasks({ column: "todo" })` — so the sweep never
|
||||
even SAW a card in a renamed hold column. Threading the signal alone would
|
||||
have been a dead fix: the guard would be correct and still never run.
|
||||
|
||||
The store mock below therefore HONORS the column filter. The pre-existing sweep
|
||||
test mocks `listTasks` to return its fixture regardless of arguments, which means
|
||||
a renamed-column test written against that harness would pass while the query
|
||||
stayed broken — exactly the "dead guard passes tests" failure mode. Do not
|
||||
"simplify" this mock to ignore the filter.
|
||||
*/
|
||||
import { beforeEach, afterEach, describe, expect, it, vi } from "vitest";
|
||||
import type { Task, TaskStore, WorkflowIr } from "@fusion/core";
|
||||
|
||||
import { SelfHealingManager } from "../self-healing.js";
|
||||
|
||||
const WF = "custom:wf";
|
||||
const THRESHOLD_MS = 24 * 60 * 60_000;
|
||||
|
||||
function pausedTask(over: Partial<Task> = {}): Task {
|
||||
return {
|
||||
id: "FN-1",
|
||||
title: "t",
|
||||
description: "",
|
||||
column: "todo",
|
||||
paused: true,
|
||||
pausedReason: "manual-hold",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
columnMovedAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||
...over,
|
||||
} as Task;
|
||||
}
|
||||
|
||||
function ir(holdId: string): WorkflowIr {
|
||||
return {
|
||||
version: "v2",
|
||||
id: WF,
|
||||
nodes: [],
|
||||
edges: [],
|
||||
columns: [
|
||||
{ id: holdId, label: "Hold", traits: [{ trait: "hold", config: { release: "capacity" } }] },
|
||||
{ id: "building", label: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
|
||||
{ id: "shipped", label: "Shipped", traits: [{ trait: "complete" }] },
|
||||
],
|
||||
} as unknown as WorkflowIr;
|
||||
}
|
||||
|
||||
function createStore(tasks: Task[], workflowIr: WorkflowIr | undefined) {
|
||||
const logEntry = vi.fn().mockResolvedValue(undefined);
|
||||
const selection = { workflowId: WF, stepIds: [] };
|
||||
const store = {
|
||||
getSettings: vi.fn().mockResolvedValue({ stalePausedTodoThresholdMs: THRESHOLD_MS }),
|
||||
/*
|
||||
HONORS the column filter — see the file header. A mock that ignores it makes
|
||||
the renamed-hold assertions pass against the unfixed query.
|
||||
*/
|
||||
listTasks: vi.fn(async (opts?: { column?: string }) =>
|
||||
opts?.column ? tasks.filter((t) => t.column === opts.column) : tasks,
|
||||
),
|
||||
getTask: vi.fn(async (id: string) => tasks.find((t) => t.id === id) ?? null),
|
||||
logEntry,
|
||||
recordRunAuditEvent: vi.fn().mockResolvedValue(undefined),
|
||||
getTaskWorkflowSelection: vi.fn(() => selection),
|
||||
getTaskWorkflowSelectionAsync: vi.fn(async () => selection),
|
||||
getWorkflowDefinition: vi.fn(async () => (workflowIr ? { ir: workflowIr } : null)),
|
||||
} as unknown as TaskStore;
|
||||
return { store, logEntry };
|
||||
}
|
||||
|
||||
function manager(store: TaskStore) {
|
||||
return new SelfHealingManager(store, { rootDir: "/tmp/test-project" });
|
||||
}
|
||||
|
||||
describe("surfaceStalePausedTodos under a renamed hold column", () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
// Well past the threshold from the fixture's columnMovedAt.
|
||||
vi.setSystemTime(new Date("2026-01-03T00:00:00.000Z"));
|
||||
});
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("surfaces a stale paused card resting in a RENAMED hold column", async () => {
|
||||
const task = pausedTask({ id: "FN-R", column: "drafting" });
|
||||
const { store, logEntry } = createStore([task], ir("drafting"));
|
||||
|
||||
const surfaced = await manager(store).surfaceStalePausedTodos();
|
||||
|
||||
expect(surfaced).toBe(1);
|
||||
expect(logEntry).toHaveBeenCalledWith(
|
||||
"FN-R",
|
||||
expect.stringContaining("Stale paused todo surfaced [stale-paused-todo]"),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not scope its query to the literal todo column", async () => {
|
||||
/*
|
||||
Pins fix #2 directly. Even with a correct signal, a `column: "todo"` query
|
||||
hands the loop an empty list for a renamed workflow — the sweep would report
|
||||
0 while looking entirely healthy.
|
||||
*/
|
||||
const task = pausedTask({ id: "FN-R", column: "drafting" });
|
||||
const { store } = createStore([task], ir("drafting"));
|
||||
|
||||
await manager(store).surfaceStalePausedTodos();
|
||||
|
||||
const columnArgs = (store.listTasks as ReturnType<typeof vi.fn>).mock.calls.map(
|
||||
(call) => (call[0] as { column?: string } | undefined)?.column,
|
||||
);
|
||||
expect(columnArgs).not.toContain("todo");
|
||||
});
|
||||
|
||||
it("does NOT surface a paused card resting in a non-hold column", async () => {
|
||||
/* The negative half — otherwise dropping the column filter would surface
|
||||
paused cards from every column, which is a louder bug than the silent one
|
||||
it replaces. */
|
||||
const task = pausedTask({ id: "FN-W", column: "building" });
|
||||
const { store, logEntry } = createStore([task], ir("drafting"));
|
||||
|
||||
const surfaced = await manager(store).surfaceStalePausedTodos();
|
||||
|
||||
expect(surfaced).toBe(0);
|
||||
expect(logEntry).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("still surfaces a builtin todo card (regression floor)", async () => {
|
||||
const task = pausedTask({ id: "FN-D", column: "todo" });
|
||||
const { store, logEntry } = createStore([task], ir("todo"));
|
||||
|
||||
const surfaced = await manager(store).surfaceStalePausedTodos();
|
||||
|
||||
expect(surfaced).toBe(1);
|
||||
expect(logEntry).toHaveBeenCalledWith("FN-D", expect.stringContaining("Stale paused todo surfaced"));
|
||||
});
|
||||
|
||||
it("falls back to the legacy todo column when the workflow cannot be resolved", async () => {
|
||||
const task = pausedTask({ id: "FN-U", column: "todo" });
|
||||
const { store } = createStore([task], undefined);
|
||||
|
||||
expect(await manager(store).surfaceStalePausedTodos()).toBe(1);
|
||||
});
|
||||
|
||||
it("surfaces the right cards on a board mixing a renamed and a builtin workflow", async () => {
|
||||
/* Per-task resolution, not one board-wide vocabulary: each card's hold
|
||||
column comes from ITS OWN workflow. */
|
||||
const renamed = pausedTask({ id: "FN-R", column: "drafting" });
|
||||
const legacy = pausedTask({ id: "FN-D", column: "todo" });
|
||||
const irByWorkflow: Record<string, WorkflowIr> = {
|
||||
"wf-renamed": ir("drafting"),
|
||||
"wf-legacy": ir("todo"),
|
||||
};
|
||||
const byTask: Record<string, string> = { "FN-R": "wf-renamed", "FN-D": "wf-legacy" };
|
||||
const tasks = [renamed, legacy];
|
||||
const logEntry = vi.fn().mockResolvedValue(undefined);
|
||||
const store = {
|
||||
getSettings: vi.fn().mockResolvedValue({ stalePausedTodoThresholdMs: THRESHOLD_MS }),
|
||||
listTasks: vi.fn(async (opts?: { column?: string }) =>
|
||||
opts?.column ? tasks.filter((t) => t.column === opts.column) : tasks,
|
||||
),
|
||||
getTask: vi.fn(async (id: string) => tasks.find((t) => t.id === id) ?? null),
|
||||
logEntry,
|
||||
recordRunAuditEvent: vi.fn().mockResolvedValue(undefined),
|
||||
getTaskWorkflowSelection: vi.fn((id: string) => ({ workflowId: byTask[id], stepIds: [] })),
|
||||
getTaskWorkflowSelectionAsync: vi.fn(async (id: string) => ({ workflowId: byTask[id], stepIds: [] })),
|
||||
getWorkflowDefinition: vi.fn(async (id: string) =>
|
||||
irByWorkflow[id] ? { ir: irByWorkflow[id] } : null,
|
||||
),
|
||||
} as unknown as TaskStore;
|
||||
|
||||
const surfaced = await manager(store).surfaceStalePausedTodos();
|
||||
|
||||
expect(surfaced).toBe(2);
|
||||
expect(logEntry.mock.calls.map((c) => c[0]).sort()).toEqual(["FN-D", "FN-R"]);
|
||||
});
|
||||
});
|
||||
@@ -1,8 +1,12 @@
|
||||
import {
|
||||
computeDependencyBlockedTodoReport,
|
||||
computeInsightFingerprint,
|
||||
resolveLifecycleColumns,
|
||||
resolveWorkflowIrForTask,
|
||||
DEFAULT_DEPENDENCY_BLOCKED_TODO_MAX_GROUPS,
|
||||
type Task,
|
||||
type TaskStore,
|
||||
type WorkflowIr,
|
||||
} from "@fusion/core";
|
||||
import { createLogger } from "./logger.js";
|
||||
|
||||
@@ -34,6 +38,54 @@ export class DependencyBlockedTodoReporter {
|
||||
this.now = options.now ?? (() => Date.now());
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-28-17:50 (PR #2479 review, P1 + P2):
|
||||
Classify every task against ITS OWN workflow.
|
||||
|
||||
This replaces a board-wide UNION of roles, which was wrong in the way this whole
|
||||
program is about: a column id means something only RELATIVE TO ITS WORKFLOW. If
|
||||
one workflow calls `done` its hold column and another calls `done` terminal, a
|
||||
union marks that column BOTH, so dependents count as held while the blocker
|
||||
beside them is discarded as finished — from a single ambiguous id. Resolving per
|
||||
task makes that impossible by construction instead of detectable afterwards.
|
||||
|
||||
It also fixes the sibling P2 as a side effect rather than needing its own memo
|
||||
layer: ONE caller-owned `irCache` is shared across the whole pass, so
|
||||
workflow-definition and prompt-override reads scale with the number of
|
||||
WORKFLOWS, not the number of cards.
|
||||
|
||||
Fail-soft per task: a card whose workflow will not resolve falls back to the
|
||||
legacy roles, so one bad workflow degrades that card to today's behavior instead
|
||||
of breaking the report.
|
||||
*/
|
||||
private async buildTaskLifecycleClassifier(
|
||||
tasks: readonly Task[],
|
||||
): Promise<(task: Task) => { isHold: boolean; isTerminal: boolean }> {
|
||||
const irCache = new Map<string, WorkflowIr>();
|
||||
const rolesByTaskId = new Map<string, { isHold: boolean; isTerminal: boolean }>();
|
||||
|
||||
for (const task of tasks) {
|
||||
try {
|
||||
const lifecycle = resolveLifecycleColumns(await resolveWorkflowIrForTask(this.store, task.id, irCache));
|
||||
if (!lifecycle) continue;
|
||||
rolesByTaskId.set(task.id, {
|
||||
isHold: lifecycle.hold !== undefined && task.column === lifecycle.hold,
|
||||
isTerminal:
|
||||
(lifecycle.complete !== undefined && task.column === lifecycle.complete) ||
|
||||
(lifecycle.archived !== undefined && task.column === lifecycle.archived),
|
||||
});
|
||||
} catch {
|
||||
// Leave unmapped: the legacy fallback below applies to this card only.
|
||||
}
|
||||
}
|
||||
|
||||
return (task: Task) =>
|
||||
rolesByTaskId.get(task.id) ?? {
|
||||
isHold: task.column === "todo",
|
||||
isTerminal: task.column === "done" || task.column === "archived",
|
||||
};
|
||||
}
|
||||
|
||||
async report(): Promise<{ alerted: boolean; reason?: string; groupCount?: number }> {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
@@ -66,12 +118,14 @@ export class DependencyBlockedTodoReporter {
|
||||
const tasks = await this.store.listTasks({ slim: true, includeArchived: false });
|
||||
const taskById = new Map(tasks.map((task) => [task.id, task]));
|
||||
const nowMs = this.now();
|
||||
const classifyTask = await this.buildTaskLifecycleClassifier(tasks);
|
||||
const report = computeDependencyBlockedTodoReport(tasks, maxAutoMergeRetries, {
|
||||
now: nowMs,
|
||||
freshAgeMs,
|
||||
staleAgeMs,
|
||||
minBlockedTodoCount,
|
||||
maxGroups: DEFAULT_DEPENDENCY_BLOCKED_TODO_MAX_GROUPS,
|
||||
classifyTask,
|
||||
});
|
||||
|
||||
if (report.uniqueBlockerCount === 0) {
|
||||
|
||||
@@ -31,7 +31,7 @@ import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, rmSync,
|
||||
import { readFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { isAbsolute, join, relative, resolve } from "node:path";
|
||||
import { resolveColumnFlags, IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, resolveWorkflowIrForTask, resolveReboundTarget, workflowHasColumn, planLegacyAdoption, resolveOrphanedPendingStepResults, classifyReviewLease, PLAN_REVIEW_LEASE_STALENESS_MS, DEFAULT_MAX_POST_REVIEW_FIXES, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AWAITING_APPROVAL_PAUSE_REASON, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult } from "@fusion/core";
|
||||
import { resolveColumnFlags, IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, resolveWorkflowIrForTask, resolveReboundTarget, resolveLifecycleColumns, workflowHasColumn, planLegacyAdoption, resolveOrphanedPendingStepResults, classifyReviewLease, PLAN_REVIEW_LEASE_STALENESS_MS, DEFAULT_MAX_POST_REVIEW_FIXES, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AWAITING_APPROVAL_PAUSE_REASON, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult } from "@fusion/core";
|
||||
import { finalizePlanningSegment } from "@fusion/core";
|
||||
import type { MeshLeaseManager } from "./mesh-lease-manager.js";
|
||||
import { createLogger, schedulerLog } from "./logger.js";
|
||||
@@ -8210,14 +8210,43 @@ export class SelfHealingManager {
|
||||
const thresholdMs = settings.stalePausedTodoThresholdMs;
|
||||
if (!thresholdMs || thresholdMs <= 0) return 0;
|
||||
|
||||
const tasks = await this.store.listTasks({ column: "todo", slim: false });
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-28-03:45 (PR #2470 review, P1):
|
||||
This sweep needed TWO fixes, not one. `getStalePausedTodoSignal` gained a
|
||||
`holdColumn` parameter in B1 but every caller omitted it, so the guard still
|
||||
compared against the literal "todo" — and the QUERY was `{ column: "todo" }`,
|
||||
so the sweep never even saw a card resting in a renamed hold column.
|
||||
Threading only the signal would have left a correct guard that never runs.
|
||||
|
||||
The column filter is therefore dropped and the hold column resolved PER TASK
|
||||
(a board can span workflows, each with its own hold column). Cost is
|
||||
contained by ordering: `paused !== true` rejects almost every card before any
|
||||
IR resolution, and the survivors share an `irCache`, so a board of 400 cards
|
||||
with three paused ones resolves at most three times.
|
||||
*/
|
||||
const tasks = await this.store.listTasks({ slim: false });
|
||||
const irCache = new Map<string, Awaited<ReturnType<typeof resolveWorkflowIrForTask>>>();
|
||||
let surfaced = 0;
|
||||
|
||||
for (const task of tasks) {
|
||||
if (task.paused !== true) continue;
|
||||
|
||||
let holdColumn = "todo";
|
||||
try {
|
||||
const lifecycle = resolveLifecycleColumns(
|
||||
await resolveWorkflowIrForTask(this.store, task.id, irCache),
|
||||
);
|
||||
// A workflow declaring no hold column keeps the legacy id rather than
|
||||
// matching nothing (conservative: preserves today's behavior).
|
||||
if (lifecycle?.hold) holdColumn = lifecycle.hold;
|
||||
} catch {
|
||||
holdColumn = "todo";
|
||||
}
|
||||
|
||||
const signal = getStalePausedTodoSignal(task, {
|
||||
now: cycleStartMs,
|
||||
thresholdMs,
|
||||
holdColumn,
|
||||
engineActiveSinceMs: settings.engineActiveSinceMs,
|
||||
engineActivationGraceMs: settings.engineActivationGraceMs,
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user