diff --git a/packages/dashboard/src/__tests__/github-tracking-reconciler.test.ts b/packages/dashboard/src/__tests__/github-tracking-reconciler.test.ts index 25f3dd972d..084e63510a 100644 --- a/packages/dashboard/src/__tests__/github-tracking-reconciler.test.ts +++ b/packages/dashboard/src/__tests__/github-tracking-reconciler.test.ts @@ -22,13 +22,29 @@ vi.mock("../github-auth.js", () => ({ resolveGithubTrackingAuth: (...args: unknown[]) => mockResolveGithubTrackingAuth(...args), })); +/* +FNXC:WorkflowResolvedColumns 2026-07-31-05:30 (fleet phase — why the existing cases could not catch this): +`workflowIr` is OPTIONAL and every pre-existing case omits it. Without a workflow reader, +`resolveTaskLifecycleColumns` catches and returns undefined, so the reconciler falls back to the legacy +ids and those cases assert exactly what they always asserted — which is why all 33 stayed green through +the conversion, and why none of them could have caught it being wrong. + +Supplying an IR is what makes the renamed-lane case below a real test rather than a restatement. +*/ function createStore(options: { listTasks?: Array>; reconcileCandidates?: Array>; reconcileHasMore?: boolean; settings?: Record; + workflowIr?: Record; }): TaskStore { return { + ...(options.workflowIr + ? { + getTaskWorkflowSelection: () => ({ workflowId: "custom:renamed", stepIds: [] }), + getWorkflowDefinition: async () => ({ ir: options.workflowIr }), + } + : {}), listTasks: vi.fn().mockResolvedValue(options.listTasks ?? []), listTasksForGithubTrackingReconcile: vi .fn() @@ -341,4 +357,64 @@ describe("GitHubTrackingReconciler", () => { expect(nextOffset).toBe(200 + RECONCILE_SCAN_LIMIT); }); }); + + /* + FNXC:WorkflowResolvedColumns 2026-07-31-05:30 (fleet phase — the SYNC-FILTER class, converted): + Before this, every `.filter((task) => task.column === "done" || task.column === "archived")` in the + reconciler matched NOTHING on a board whose terminal lanes are renamed. The pass then reported + `scanned: 0, closed: 0` — a clean-looking run that closed no GitHub issues at all. + + REVERT CHECK, measured (both run): restoring the id comparisons makes "closes issues on a RENAMED + complete lane" fail with `expected 0 to be 1` and no `setIssueState` call, and makes the archived + heuristic case fail the same way. The default-vocabulary cases above pass either way. + */ + const RENAMED_IR = { + version: "v2", + id: "custom:renamed", + name: "Renamed", + nodes: [], + edges: [], + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }, { trait: "hold" }] }, + { id: "building", name: "Building", traits: [{ trait: "wip" }] }, + { id: "checking", name: "Checking", traits: [{ trait: "merge-blocker" }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + { id: "attic", name: "Attic", traits: [{ trait: "archived" }] }, + ], + }; + + it("closes issues on a RENAMED complete lane, which the id comparisons could not see", async () => { + mockResolveGithubTrackingAuth.mockReturnValue({ ok: true, auth: { mode: "token", token: "ghp_test" } }); + mockGetIssue.mockResolvedValue({ state: "open" }); + const store = createStore({ + workflowIr: RENAMED_IR, + listTasks: [ + { id: "FN-9", column: "shipped", githubTracking: { enabled: true, issue: { owner: "o", repo: "r", number: 9 } } }, + ], + }); + + const result = await new GitHubTrackingReconciler().reconcile(store); + + expect(mockSetIssueState).toHaveBeenCalledWith("o", "r", 9, "closed", "completed"); + expect(result.closed).toBe(1); + }); + + it("applies the archived done-heuristic on a RENAMED archived lane", async () => { + mockResolveGithubTrackingAuth.mockReturnValue({ ok: true, auth: { mode: "token", token: "ghp_test" } }); + mockGetIssue.mockResolvedValue({ state: "open" }); + const store = createStore({ + workflowIr: RENAMED_IR, + listTasks: [ + { id: "FN-10", column: "attic", executionCompletedAt: "2026-01-01T00:00:00.000Z", githubTracking: { enabled: true, issue: { owner: "o", repo: "r", number: 10 } } }, + { id: "FN-11", column: "attic", githubTracking: { enabled: true, issue: { owner: "o", repo: "r", number: 11 } } }, + ], + }); + + const result = await new GitHubTrackingReconciler().reconcile(store); + + // Completed-before-archive closes as `completed`; never-executed closes as `not_planned`. + expect(mockSetIssueState).toHaveBeenCalledWith("o", "r", 10, "closed", "completed"); + expect(mockSetIssueState).toHaveBeenCalledWith("o", "r", 11, "closed", "not_planned"); + expect(result.closed).toBe(2); + }); }); diff --git a/packages/dashboard/src/github-tracking-reconciler.ts b/packages/dashboard/src/github-tracking-reconciler.ts index 7b6915a5b1..2b757323ac 100644 --- a/packages/dashboard/src/github-tracking-reconciler.ts +++ b/packages/dashboard/src/github-tracking-reconciler.ts @@ -1,13 +1,88 @@ import { createLogger } from "@fusion/core"; const severityAuditLog = createLogger("dashboard-github-tracking-reconciler"); -import type { GlobalSettings, ProjectSettings, TaskSourceIssue, TaskStore } from "@fusion/core"; +import type { GlobalSettings, LifecycleColumns, ProjectSettings, Task, TaskSourceIssue, TaskStore, WorkflowIr } from "@fusion/core"; +import { resolveTaskLifecycleColumns } from "@fusion/core"; import { resolveGithubTrackingAuth } from "./github-auth.js"; import { GitHubClient } from "./github.js"; const RECONCILE_SCAN_LIMIT = 200; const RECONCILE_CONCURRENCY_LIMIT = 4; + +/* +FNXC:WorkflowResolvedColumns 2026-07-31-05:10 (fleet phase — the SYNC-FILTER class, decided): +PREFETCH A RESOLVED MAP, then filter synchronously. This is the pattern for every +`.filter((task) => task.column === "")` over a list of OTHER tasks — a shape I flagged across four +files and left unconverted while waiting for a decision that had to be mine. + +THE TWO OPTIONS AND WHY THIS ONE. The alternative is making the predicates async, which forces every +caller into `for await` and turns one list comprehension into a sequential walk. Prefetching keeps the +filters synchronous and puts the awaits in one bounded place; it also lets the IR cache do its job, which +is the whole reason `resolveTaskLifecycleColumns` takes a caller-owned one: + + "A self-healing pass over 400 cards spanning three workflows must read three IRs, not 400." + +So the cache is shared across the WHOLE reconcile run, not per pass. The three passes in this file each +list the board independently; one cache means the IR is read once per distinct workflow for all of them. +`resolveLifecycleColumns` itself is pure and is not memoized by that cache, so this still costs one cheap +struct build per task — acceptable in a background reconcile, and stated rather than hidden. + +WHY CONVERTING `archived` HERE IS NOT THE SPLIT BRAIN #2724 DESCRIBES. That guard covers the archived +gate in `packages/core`, where the same question is answered in TypeScript AND in SQL, so converting one +encoding alone diverges them. This file contains ZERO SQL (measured: no drizzle, no `sql` template, no +eq/ne) and calls `listTasks({ includeArchived: true })` — the SQL half has already been told to include +archived rows, so this filter SELECTS among rows it was handed rather than deciding liveness a second +time. Gate versus consumer is the distinction; a consumer can be converted alone. + +WHAT IT COST BEFORE. On a board whose terminal lanes are renamed, every filter here matched nothing, so +the reconciler closed NO GitHub issues and reported `scanned: 0` — a clean-looking pass that did nothing. +*/ +type LifecycleByTaskId = ReadonlyMap; + +async function resolveLifecycleByTaskId( + store: TaskStore, + tasks: readonly Task[], + irCache: Map, + /* + FNXC:WorkflowResolvedColumns 2026-07-31-12:10 (#2737 review — greptile P2): + STOP once `limit` tasks have matched. The first version resolved for every row `listTasks` returned — + an unbounded board — before slicing to RECONCILE_SCAN_LIMIT, so the prefetch did unbounded work to feed + a bounded scan. `match` is applied here rather than by the caller precisely so the loop can stop. + + Rows past the cut are left unresolved and absent from the map. That is safe because the only consumers + are the terminal predicates, which fall back to the legacy ids for an absent entry — the same degraded + answer they would give on a store with no workflow reader — and those rows are dropped by the slice + anyway. + */ + options?: { match?: (task: Task, lifecycle: LifecycleColumns | undefined) => boolean; limit?: number }, +): Promise { + const byTaskId = new Map(); + let matched = 0; + for (const task of tasks) { + if (byTaskId.has(task.id)) continue; + const lifecycle = await resolveTaskLifecycleColumns(store, task.id, irCache); + byTaskId.set(task.id, lifecycle); + if (options?.match && options.match(task, lifecycle)) { + matched += 1; + if (options.limit !== undefined && matched >= options.limit) break; + } + } + return byTaskId; +} + +/** Is this task in a terminal lane — complete or archived — by its OWN workflow's roles? */ +function isTerminalTask(task: Task, lifecycleByTaskId: LifecycleByTaskId): boolean { + const lifecycle = lifecycleByTaskId.get(task.id); + return task.column === (lifecycle?.complete ?? "done") + || task.column === (lifecycle?.archived ?? "archived"); +} + +/** Is this task in the ARCHIVED lane specifically (used for the FN-5577 done-heuristic)? */ +function isArchivedTask(task: Task, lifecycleByTaskId: LifecycleByTaskId): boolean { + return task.column === (lifecycleByTaskId.get(task.id)?.archived ?? "archived"); +} + export class GitHubTrackingReconciler { /* FNXC:GithubTrackingReconcile 2026-07-16-15:40: @@ -49,8 +124,14 @@ export class GitHubTrackingReconciler { async reconcile(store: TaskStore): Promise<{ scanned: number; closed: number; skipped: number; errors: number }> { const listedTasks = await store.listTasks({ slim: true, includeArchived: true }); - const tasks = (Array.isArray(listedTasks) ? listedTasks : []) - .filter((task) => task.column === "done" || task.column === "archived") + const allTasks = Array.isArray(listedTasks) ? listedTasks : []; + const lifecycleByTaskId = await resolveLifecycleByTaskId(store, allTasks, new Map(), { + match: (task, lifecycle) => task.column === (lifecycle?.complete ?? "done") + || task.column === (lifecycle?.archived ?? "archived"), + limit: RECONCILE_SCAN_LIMIT, + }); + const tasks = allTasks + .filter((task) => isTerminalTask(task, lifecycleByTaskId)) .slice(0, RECONCILE_SCAN_LIMIT); const projectSettings = ((await store.getSettings()) ?? {}) as Pick; @@ -85,7 +166,7 @@ export class GitHubTrackingReconciler { return; } - const stateReason = task.column === "archived" && !task.executionCompletedAt ? "not_planned" : "completed"; + const stateReason = isArchivedTask(task, lifecycleByTaskId) && !task.executionCompletedAt ? "not_planned" : "completed"; await client.setIssueState(issue.owner, issue.repo, issue.number, "closed", stateReason); closed += 1; } catch (error) { @@ -103,8 +184,14 @@ export class GitHubTrackingReconciler { async reconcileSourceIssues(store: TaskStore): Promise<{ scanned: number; closed: number; skipped: number; errors: number }> { const listedTasks = await store.listTasks({ slim: false, includeArchived: true }); - const tasks = (Array.isArray(listedTasks) ? listedTasks : []) - .filter((task) => (task.column === "done" || task.column === "archived") && task.sourceIssue?.provider === "github") + const allTasks = Array.isArray(listedTasks) ? listedTasks : []; + const lifecycleByTaskId = await resolveLifecycleByTaskId(store, allTasks, new Map(), { + match: (task, lifecycle) => task.sourceIssue?.provider === "github" + && (task.column === (lifecycle?.complete ?? "done") || task.column === (lifecycle?.archived ?? "archived")), + limit: RECONCILE_SCAN_LIMIT, + }); + const tasks = allTasks + .filter((task) => isTerminalTask(task, lifecycleByTaskId) && task.sourceIssue?.provider === "github") .slice(0, RECONCILE_SCAN_LIMIT); const projectSettings = ((await store.getSettings()) ?? {}) as Pick; @@ -154,7 +241,7 @@ export class GitHubTrackingReconciler { return; } - const stateReason = task.column === "archived" && !task.executionCompletedAt ? "not_planned" : "completed"; + const stateReason = isArchivedTask(task, lifecycleByTaskId) && !task.executionCompletedAt ? "not_planned" : "completed"; await client.setIssueState(owner, repo, issueNumberValue, "closed", stateReason); if (!sourceIssue.closedAt) { await persistSourceIssueClosedAt(store, task.id, sourceIssue, new Date().toISOString()); @@ -186,8 +273,10 @@ export class GitHubTrackingReconciler { const limit = Number.isInteger(options?.limit) && (options?.limit ?? RECONCILE_SCAN_LIMIT) >= 0 ? Math.min(options?.limit ?? RECONCILE_SCAN_LIMIT, RECONCILE_SCAN_LIMIT) : RECONCILE_SCAN_LIMIT; - const matchingTasks = (Array.isArray(listedTasks) ? listedTasks : []) - .filter((task) => (task.column === "done" || task.column === "archived") + const allTasks = Array.isArray(listedTasks) ? listedTasks : []; + const lifecycleByTaskId = await resolveLifecycleByTaskId(store, allTasks, new Map()); + const matchingTasks = allTasks + .filter((task) => isTerminalTask(task, lifecycleByTaskId) && task.sourceIssue?.provider === "github" && !task.sourceIssue?.closedAt); const tasks = matchingTasks.slice(offset, offset + limit); @@ -252,6 +341,23 @@ export class GitHubTrackingReconciler { const listedTasks = await store.listTasksForGithubTrackingReconcile(options); const tasks = Array.isArray(listedTasks?.tasks) ? listedTasks.tasks : []; const hasMore = listedTasks?.hasMore === true; + /* + FNXC:WorkflowResolvedColumns 2026-07-31-05:20: + Resolved for the PAGE, not the board — this pass's list comes from + `listTasksForGithubTrackingReconcile`, which is already offset/limit bounded (<= RECONCILE_SCAN_LIMIT). + + NOT the split brain #2724 documents, and I checked before converting: that store impl filters on + `deletedAt IS NOT NULL` AND `githubTracking IS NOT NULL` — it does NOT compare the column to + 'archived', so there is no SQL-side encoding of this question to diverge from. + + REACHABILITY, worth recording: in backend mode this pass returns only SOFT-DELETED rows (its own + comment says the archived-tasks fallback is a separate AsyncArchiveLineage subsystem, skipped here), + and `task.deletedAt` is tested FIRST in the stateReason chain below. So the archived arm is + effectively unreachable in backend mode today. Converted anyway rather than deleted: it is the + documented FN-5577 done-heuristic, and whether that fallback should be wired here is a separate + question from what vocabulary it speaks. + */ + const lifecycleByTaskId = await resolveLifecycleByTaskId(store, tasks, new Map()); const projectSettings = ((await store.getSettings()) ?? {}) as Pick; const globalSettings = (await store.getGlobalSettingsStore?.()?.getSettings?.() ?? {}) as Pick; @@ -289,7 +395,7 @@ export class GitHubTrackingReconciler { // executionCompletedAt as the done-heuristic for archived rows. const stateReason = task.deletedAt ? "not_planned" - : task.column === "archived" && task.executionCompletedAt + : isArchivedTask(task, lifecycleByTaskId) && task.executionCompletedAt ? "completed" : "not_planned"; diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index 39433dc77a..89497c6e07 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -7,7 +7,6 @@ "packages/engine/src/scheduler.ts": 12, "packages/core/src/task-store/async-comments-attachments.ts": 9, "packages/dashboard/app/components/TaskContextMenu.tsx": 9, - "packages/dashboard/src/github-tracking-reconciler.ts": 9, "packages/engine/src/notification/notification-service.ts": 9, "packages/core/src/default-workflow-hooks.ts": 7, "packages/dashboard/app/components/Column.tsx": 7,