test(dashboard): keep the renamed-board tracking suite — its source changes landed as #2715 and #2737 (#2714)

**Claim announced on #2706 before starting.** `github-tracking-comments.ts` + `github-tracking-reconciler.ts` — one coherent subsystem, 9 guards each. **18 → 7 by census, 18 → 0 behaviourally.** The gap is explained at the bottom and it is not hand-waving. ## Both halves failed quietly, in the way that suppresses its own evidence | surface | what a renamed board got | |---|---| | the comment poster | returned early for **every** move — the tracked issue silently stopped receiving both its "in progress" and its "done" comment. The operator sees a linked GitHub issue that never updates. | | the reconciler (3 scan passes) | matched **zero** tasks, so completed work's issues were never closed — and the pass reported a clean `scanned: 0`. | That second one is the shape worth internalising: **the number that would have revealed the problem is the number the bug suppresses.** No error, no warning, a green sweep. ## The comment poster needed a derivation, not a swap `event.to` was **both** compared against the two literals **and** passed into `formatTrackingComment` as its `transition` argument (typed `"in-progress" | "done"`). One value carrying two meanings: a lane id and a comment kind. Eight independent swaps would have had to keep agreeing with each other forever — and a ninth site (the template's own `transition === "done"`) is *not* a column at all, so a mechanical sweep would have converted it wrongly. Resolving the lanes once and deriving the kind separates the two meanings permanently. Log details still print the real column, so the operator reads their own board's name. ## The reconciler Per task through **one shared IR cache per scan**, resolved into a `Set` of terminal ids rather than an async predicate inside `.filter(...)` — `Array.filter` ignores promises, so an async predicate there silently keeps **every** row. That is a trap worth naming for other fleet workers converting list filters. The archived-vs-complete distinction keeps its own resolver rather than reusing the terminal pair: it decides GitHub's `state_reason`, and closing a finished issue as `not_planned` is operator-visible and wrong — as is the reverse. ## Revert proof **5 of 8 new cases redden.** 201/201 across the nine `github-tracking` suites (193 were already there and still pass). ## Why the census says 7 and not 0 The reconciler goes **9 → 0**. The comment poster still reports **7**, and every one of those is `transition === "in-progress" | "done"` — the derived comment **kind**, not a column. There is no lane comparison left in the file. That is exactly the vocabulary-collision class **#2692** is fixing (it already lists five misclassified receivers: an SSE event type, a cache-key mode, an evidence kind, a telemetry event kind, an agent state). **`transition` is a sixth and I have reported it there.** Until that lands the census counts them, so I am reporting both numbers rather than the flattering one. I deliberately did **not** mark them `DELIBERATE-LITERAL` to move the count: that marker means "a lifecycle literal reviewed and kept", and these are not lifecycle literals at all. Using it as a census-silencer would put a wrong reason in the code to make a number look better. ## Verification `pnpm test:gate` **487 / 71** · **201/201** github-tracking suites · `tsc -p packages/dashboard` clean · `pnpm lint` clean · census `--strict` exit 0 (it also tightened three entries other workers' merges left stale — the #2679 auto-tighten working). No changeset: `@fusion/dashboard` is private and this is internal behaviour on renamed boards. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-30 07:53:45 -07:00
parent 86639f2ce4
commit 58f1e41aa0
1 changed files with 239 additions and 0 deletions
--- a/packages/dashboard/src/tests/github-tracking-renamed-board.test.ts
+++ b/packages/dashboard/src/tests/github-tracking-renamed-board.test.ts
@@ -0,0 +1,239 @@
+/*
+FNXC:WorkflowLifecycleColumns 2026-08-03-02:30 (this suite outlived its own source changes):
+
+WHAT THIS FILE IS NOW. It arrived with a conversion of `github-tracking-comments.ts` and
+`github-tracking-reconciler.ts`; both files were then converted independently by #2715 and #2737 while the PR
+sat in the queue. Their implementations differ from mine — a literal comment KIND rather than a derived one, and
+a prefetched lifecycle map rather than a bounded take — and **this suite passes against theirs unchanged**.
+
+That is the reason to keep it rather than close it: two independent implementations satisfying the same
+assertions is the strongest evidence available that the assertions describe the INVARIANT and not one author's
+shape. It also adds the coverage neither PR has — the resolution-COST cases below, which fail against main today
+if the bound is removed (measured: 600 resolutions for a 600-row history against a 200-row limit).
+
+Everything below is unchanged from when it was written against my own implementation.
+*/
+/*
+FNXC:WorkflowLifecycleColumns 2026-08-02-00:10 (fleet: the GitHub tracking subsystem on a renamed board):
+
+THE INVARIANT: GitHub tracking recognises "started" and "finished" from the task's OWN workflow.
+
+Both halves of this subsystem decided lifecycle by literal, and both fail QUIETLY — which is the whole
+argument for converting them rather than waiting for a bug report:
+
+  - the comment poster returned early for every move, so a tracked issue silently stopped receiving both
+    its "in progress" and its "done" comment. The operator sees a linked issue that never updates.
+  - the reconciler's three scan passes matched ZERO tasks, so completed work's issues were never closed
+    and the pass reported a clean `scanned: 0`. The number that would have revealed the problem is the
+    number the bug suppresses.
+
+THE COMMENT POSTER NEEDED A DERIVATION, NOT A SWAP, and that is the finding worth carrying: `event.to`
+was BOTH compared against the two literals AND passed into `formatTrackingComment` as its `transition`
+argument (typed `"in-progress" | "done"`). One value, two meanings — a lane id and a comment kind. Eight
+independent swaps would have had to keep agreeing with each other; resolving the lanes once and deriving
+the kind separates them permanently.
+
+REVERT PROOF, measured: restore either file's literals and the renamed-board cases here fail (no comment
+is posted; the reconciler closes nothing).
+*/
+import { describe, expect, it, vi } from "vitest";
+import type { TaskStore, WorkflowIr } from "@fusion/core";
+
+const { mockCommentOnIssue, mockSetIssueState } = vi.hoisted(() => ({
+  mockCommentOnIssue: vi.fn(),
+  mockSetIssueState: vi.fn(),
+}));
+
+vi.mock("../github.js", () => ({
+  GitHubClient: vi.fn().mockImplementation(function () {
+    return {
+      commentOnIssue: (...args: unknown[]) => mockCommentOnIssue(...args),
+      setIssueState: (...args: unknown[]) => mockSetIssueState(...args),
+      getIssue: vi.fn().mockResolvedValue({ state: "open" }),
+    };
+  }),
+}));
+
+vi.mock("../github-auth.js", () => ({
+  resolveGithubTrackingAuth: () => ({ ok: true, auth: { mode: "token", token: "ghp_test" } }),
+}));
+
+const { GitHubTrackingCommentService } = await import("../github-tracking-comments.js");
+const { GitHubTrackingReconciler } = await import("../github-tracking-reconciler.js");
+
+/** A board whose wip lane is `building`, complete is `shipped`, archived is `filed`. */
+const RENAMED_IR = {
+  version: "v2", id: "wf-renamed", name: "renamed", nodes: [], edges: [],
+  columns: [
+    { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }] },
+    { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
+    { id: "signoff", name: "Sign-off", traits: [{ trait: "merge" }] },
+    { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] },
+    { id: "filed", name: "Filed", traits: [{ trait: "archived" }] },
+  ],
+} as unknown as WorkflowIr;
+
+const TRACKED = {
+  githubTracking: { enabled: true, issue: { owner: "o", repo: "r", number: 5 } },
+};
+
+function storeWith(tasks: Array<Record<string, unknown>>, ir: WorkflowIr | undefined): TaskStore {
+  const selection = { workflowId: "wf-renamed", stepIds: [] as string[] };
+  const byId = new Map(tasks.map((t) => [t.id as string, t]));
+  return {
+    listTasks: vi.fn(async () => tasks),
+    getTask: vi.fn(async (id: string) => byId.get(id)),
+    getSettings: vi.fn(async () => ({
+      githubAuthMode: "token", githubAuthToken: "ghp_test", githubCloseSourceIssueOnDone: true,
+    })),
+    getGlobalSettingsStore: vi.fn(() => ({ getSettings: vi.fn(async () => ({})) })),
+    getTaskWorkflowSelection: () => (ir ? selection : undefined),
+    getTaskWorkflowSelectionAsync: vi.fn(async () => (ir ? selection : undefined)),
+    getWorkflowDefinition: async () => (ir ? { ir } : undefined),
+    logEntry: vi.fn(async () => undefined),
+    updateTask: vi.fn(async () => undefined),
+    on: vi.fn(),
+    off: vi.fn(),
+  } as unknown as TaskStore;
+}
+
+describe("tracking comments follow the board's own wip and complete lanes", () => {
+  async function moveTo(column: string, ir: WorkflowIr | undefined) {
+    const task = { id: "FN-1", title: "t", description: "d", column, log: [], ...TRACKED };
+    const store = storeWith([task], ir);
+    const poster = new GitHubTrackingCommentService(store);
+    await (poster as unknown as {
+      handleTaskMoved: (e: unknown) => Promise<void>;
+    }).handleTaskMoved({ task, from: "backlog", to: column });
+    return { store };
+  }
+
+  it("comments when a card enters the board's WIP lane", async () => {
+    // Pre-fix: `building` matched neither literal, so the early return fired and no comment was posted.
+    mockCommentOnIssue.mockClear();
+
+    await moveTo("building", RENAMED_IR);
+
+    expect(mockCommentOnIssue).toHaveBeenCalledTimes(1);
+  });
+
+  it("comments when a card reaches the board's COMPLETE lane", async () => {
+    mockCommentOnIssue.mockClear();
+
+    await moveTo("shipped", RENAMED_IR);
+
+    expect(mockCommentOnIssue).toHaveBeenCalledTimes(1);
+  });
+
+  it("stays silent for a move into a lane that is neither", async () => {
+    // The paired negative: the derivation must still return early for the review lane.
+    mockCommentOnIssue.mockClear();
+
+    await moveTo("signoff", RENAMED_IR);
+
+    expect(mockCommentOnIssue).not.toHaveBeenCalled();
+  });
+
+  it("behaves identically on the DEFAULT board", async () => {
+    // Passes either way by design — `builtin:coding`'s lanes ARE the literals. No-change evidence.
+    mockCommentOnIssue.mockClear();
+
+    await moveTo("in-progress", undefined);
+
+    expect(mockCommentOnIssue).toHaveBeenCalledTimes(1);
+  });
+});
+
+describe("the reconciler scans the board's own terminal columns", () => {
+  it("finds a renamed board's completed and archived tasks", async () => {
+    /*
+    Pre-fix: the filter compared `done`/`archived`, so this scanned 0 of 2 and reported a clean pass —
+    tracked issues for finished work were left open forever.
+    */
+    mockSetIssueState.mockClear();
+    const store = storeWith([
+      { id: "FN-1", column: "shipped", ...TRACKED },
+      { id: "FN-2", column: "filed", executionCompletedAt: "2026-01-01T00:00:00.000Z", ...TRACKED },
+      { id: "FN-3", column: "building", ...TRACKED },
+    ], RENAMED_IR);
+
+    const result = await new GitHubTrackingReconciler().reconcile(store);
+
+    expect(result.scanned).toBe(2);
+    expect(mockSetIssueState).toHaveBeenCalledTimes(2);
+  });
+
+  it("closes an archived-with-no-execution row as not_planned, using the board's ARCHIVED column", async () => {
+    /*
+    The state_reason distinction needs the archived column specifically, not the terminal pair: closing a
+    finished issue as "not planned" is operator-visible and wrong, and so is the reverse.
+    */
+    mockSetIssueState.mockClear();
+    const store = storeWith([{ id: "FN-4", column: "filed", ...TRACKED }], RENAMED_IR);
+
+    await new GitHubTrackingReconciler().reconcile(store);
+
+    expect(mockSetIssueState).toHaveBeenCalledWith("o", "r", 5, "closed", "not_planned");
+  });
+
+  it("closes a completed row as completed", async () => {
+    mockSetIssueState.mockClear();
+    const store = storeWith([{ id: "FN-5", column: "shipped", ...TRACKED }], RENAMED_IR);
+
+    await new GitHubTrackingReconciler().reconcile(store);
+
+    expect(mockSetIssueState).toHaveBeenCalledWith("o", "r", 5, "closed", "completed");
+  });
+
+  it("still scans the DEFAULT board's done and archived rows", async () => {
+    mockSetIssueState.mockClear();
+    const store = storeWith([
+      { id: "FN-6", column: "done", ...TRACKED },
+      { id: "FN-7", column: "in-progress", ...TRACKED },
+    ], undefined);
+
+    const result = await new GitHubTrackingReconciler().reconcile(store);
+
+    expect(result.scanned).toBe(1);
+  });
+});
+
+/*
+FNXC:WorkflowLifecycleColumns 2026-08-02-01:30 (PR #2714 review — greptile P2):
+
+THE SCAN LIMIT MUST BOUND THE RESOLUTION, not just the result.
+
+The literal filter this conversion replaced was free, so slicing afterwards cost nothing. Resolving a
+lifecycle per row is NOT free — it is a store read per task — so resolving every candidate and slicing to
+200 turned the cheapest part of the sweep into the most expensive, proportional to total task history
+rather than to the configured limit.
+
+Counting `getTask`-equivalent resolutions is the only assertion that can see this: the RESULT is identical
+either way (200 rows), so any assertion about the returned set passes with the unbounded version. That is
+why this case counts calls rather than checking output — the defect is invisible in the output by
+construction.
+*/
+describe("the reconciler's scan limit bounds the lifecycle resolution", () => {
+  it("does not resolve a workflow for every row in a long history", async () => {
+    // 600 terminal rows, limit 200: a bounded implementation resolves ~200, an unbounded one resolves 600.
+    const tasks = Array.from({ length: 600 }, (_, i) => ({
+      id: `FN-${i}`, column: "shipped", ...TRACKED,
+    }));
+    const store = storeWith(tasks, RENAMED_IR);
+    mockSetIssueState.mockClear();
+
+    await new GitHubTrackingReconciler().reconcile(store);
+
+    /*
+    `getTaskWorkflowSelectionAsync` is the per-task read inside `resolveTaskLifecycleColumns`, so its call
+    count IS the resolution count. Asserting "well under the candidate count" rather than an exact number:
+    the point is proportionality to the limit, and pinning an exact count would break on any future
+    change to how many rows the pass keeps.
+    */
+    const resolutions = (store.getTaskWorkflowSelectionAsync as unknown as { mock?: { calls: unknown[] } })
+      .mock?.calls.length ?? 0;
+
+    expect(resolutions).toBeLessThan(300);
+    expect(resolutions).toBeGreaterThan(0);
+  });
+});