From 5d0f1ef631bc926f70599b63b3e08a0f98fa976e Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Mon, 27 Jul 2026 14:19:32 -0700 Subject: [PATCH] Phase B slice B1: lifecycle column roles in the U6 policy modules (4 guards, red-green) (#2479) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **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. ## 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. --- .../core/src/__tests__/blocker-fanout.test.ts | 67 ++++++ ...locked-todo-report-renamed-columns.test.ts | 138 +++++++++++ ...store-stale-paused-renamed-hold.pg.test.ts | 125 ++++++++++ .../src/__tests__/stale-paused-todo.test.ts | 57 +++++ .../core/src/__tests__/task-priority.test.ts | 44 ++++ packages/core/src/blocker-fanout.ts | 64 ++++- .../src/dependency-blocked-todo-report.ts | 70 +++++- packages/core/src/stale-paused-todo.ts | 16 +- packages/core/src/task-priority.ts | 32 ++- packages/core/src/task-store/reads.ts | 67 +++++- ...ocked-todo-reporter-per-task-roles.test.ts | 223 ++++++++++++++++++ ...cked-todo-reporter-renamed-columns.test.ts | 202 ++++++++++++++++ ...-healing-stale-paused-renamed-hold.test.ts | 192 +++++++++++++++ .../src/dependency-blocked-todo-reporter.ts | 54 +++++ packages/engine/src/self-healing.ts | 33 ++- 15 files changed, 1367 insertions(+), 17 deletions(-) create mode 100644 packages/core/src/__tests__/dependency-blocked-todo-report-renamed-columns.test.ts create mode 100644 packages/core/src/__tests__/postgres/store-stale-paused-renamed-hold.pg.test.ts create mode 100644 packages/engine/src/__tests__/dependency-blocked-todo-reporter-per-task-roles.test.ts create mode 100644 packages/engine/src/__tests__/dependency-blocked-todo-reporter-renamed-columns.test.ts create mode 100644 packages/engine/src/__tests__/self-healing-stale-paused-renamed-hold.test.ts diff --git a/packages/core/src/__tests__/blocker-fanout.test.ts b/packages/core/src/__tests__/blocker-fanout.test.ts index 8cf1c78999..6e9af33c4b 100644 --- a/packages/core/src/__tests__/blocker-fanout.test.ts +++ b/packages/core/src/__tests__/blocker-fanout.test.ts @@ -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); + }); +}); diff --git a/packages/core/src/__tests__/dependency-blocked-todo-report-renamed-columns.test.ts b/packages/core/src/__tests__/dependency-blocked-todo-report-renamed-columns.test.ts new file mode 100644 index 0000000000..f83ec38fa5 --- /dev/null +++ b/packages/core/src/__tests__/dependency-blocked-todo-report-renamed-columns.test.ts @@ -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 { + 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"]); + }); +}); diff --git a/packages/core/src/__tests__/postgres/store-stale-paused-renamed-hold.pg.test.ts b/packages/core/src/__tests__/postgres/store-stale-paused-renamed-hold.pg.test.ts new file mode 100644 index 0000000000..92119d153d --- /dev/null +++ b/packages/core/src/__tests__/postgres/store-stale-paused-renamed-hold.pg.test.ts @@ -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 { + 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"); + }); +}); diff --git a/packages/core/src/__tests__/stale-paused-todo.test.ts b/packages/core/src/__tests__/stale-paused-todo.test.ts index 02833281d1..d65e36ca4f 100644 --- a/packages/core/src/__tests__/stale-paused-todo.test.ts +++ b/packages/core/src/__tests__/stale-paused-todo.test.ts @@ -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(); + }); +}); diff --git a/packages/core/src/__tests__/task-priority.test.ts b/packages/core/src/__tests__/task-priority.test.ts index 8cf7f3c25f..e8a1f560f1 100644 --- a/packages/core/src/__tests__/task-priority.test.ts +++ b/packages/core/src/__tests__/task-priority.test.ts @@ -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); + }); +}); diff --git a/packages/core/src/blocker-fanout.ts b/packages/core/src/blocker-fanout.ts index 5472a7d820..2d590eadf8 100644 --- a/packages/core/src/blocker-fanout.ts +++ b/packages/core/src/blocker-fanout.ts @@ -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; + /** 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; + /* + 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(["in-progress", "in-review"]); -const ACTIVE_COLUMNS = new Set(["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 = 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 = + options.holdColumns ?? new Set([options.holdColumn ?? "todo"]); + const taskById = new Map(tasks.map((task) => [task.id, task])); const fanout = new Map(); @@ -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; diff --git a/packages/core/src/dependency-blocked-todo-report.ts b/packages/core/src/dependency-blocked-todo-report.ts index 154ca5e29d..f8b3073d57 100644 --- a/packages/core/src/dependency-blocked-todo-report.ts +++ b/packages/core/src/dependency-blocked-todo-report.ts @@ -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; } diff --git a/packages/core/src/stale-paused-todo.ts b/packages/core/src/stale-paused-todo.ts index 9f1832d7ca..35bb7419f6 100644 --- a/packages/core/src/stale-paused-todo.ts +++ b/packages/core/src/stale-paused-todo.ts @@ -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, 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; diff --git a/packages/core/src/task-priority.ts b/packages/core/src/task-priority.ts index 5c4f455b5e..7177b5f3ec 100644 --- a/packages/core/src/task-priority.ts +++ b/packages/core/src/task-priority.ts @@ -87,14 +87,30 @@ export function sortTasksByPriorityThenAgeAndId( } const FANOUT_SECONDARY_WEIGHT_MULTIPLIER = 1_000_000; -const UNBLOCK_ACTIVE_COLUMNS = new Set(["triage", "todo", "in-progress", "in-review"]); -const DONE_COLUMNS = new Set(["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 = 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; } -function countUnmetDependencies(task: Task, taskById: Map): number { +function countUnmetDependencies( + task: Task, + taskById: Map, + terminalColumns: ReadonlySet, +): 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): 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 { 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(); @@ -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; } } diff --git a/packages/core/src/task-store/reads.ts b/packages/core/src/task-store/reads.ts index 1ab64b7ec5..a7ef25a27c 100644 --- a/packages/core/src/task-store/reads.ts +++ b/packages/core/src/task-store/reads.ts @@ -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, +): Promise { + 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 { 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(); /* * 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(); + { + const irCache = new Map(); + 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, }); diff --git a/packages/engine/src/__tests__/dependency-blocked-todo-reporter-per-task-roles.test.ts b/packages/engine/src/__tests__/dependency-blocked-todo-reporter-per-task-roles.test.ts new file mode 100644 index 0000000000..7cf31477c1 --- /dev/null +++ b/packages/engine/src/__tests__/dependency-blocked-todo-reporter-per-task-roles.test.ts @@ -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 { + 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, + irByWorkflow: Record, +) { + 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 = { + 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 = { + 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); + }); +}); diff --git a/packages/engine/src/__tests__/dependency-blocked-todo-reporter-renamed-columns.test.ts b/packages/engine/src/__tests__/dependency-blocked-todo-reporter-renamed-columns.test.ts new file mode 100644 index 0000000000..1fbd3a9b70 --- /dev/null +++ b/packages/engine/src/__tests__/dependency-blocked-todo-reporter-renamed-columns.test.ts @@ -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 { + 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, + irByWorkflow: Record, +): { store: TaskStore; upsertInsight: ReturnType } { + 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 = {}; + 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); + }); +}); diff --git a/packages/engine/src/__tests__/self-healing-stale-paused-renamed-hold.test.ts b/packages/engine/src/__tests__/self-healing-stale-paused-renamed-hold.test.ts new file mode 100644 index 0000000000..c746d20f54 --- /dev/null +++ b/packages/engine/src/__tests__/self-healing-stale-paused-renamed-hold.test.ts @@ -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 { + 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).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 = { + "wf-renamed": ir("drafting"), + "wf-legacy": ir("todo"), + }; + const byTask: Record = { "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"]); + }); +}); diff --git a/packages/engine/src/dependency-blocked-todo-reporter.ts b/packages/engine/src/dependency-blocked-todo-reporter.ts index 46bf8a5810..4d67ae8183 100644 --- a/packages/engine/src/dependency-blocked-todo-reporter.ts +++ b/packages/engine/src/dependency-blocked-todo-reporter.ts @@ -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(); + const rolesByTaskId = new Map(); + + 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) { diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index b2276efdcc..10e201dc8e 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -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>>(); 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, });