From 747bbf51f12c7db36f628bc4e5225c2308952564 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 10 Jun 2026 10:34:36 -0700 Subject: [PATCH] FN-6194: fix archived GitHub issue reconciliation Handle archived task transitions consistently for linked GitHub issues. - close tracked and source issues when archived tasks should land closed, including not_planned vs completed reasons - reopen linked issues when archived tasks move back to done or active work resumes - extend dashboard reconciler and state coverage to include archived tasks and source issue handling Files changed: .../__tests__/github-source-issue-close.test.ts | 70 +++++++++++++++++++++- .../__tests__/github-tracking-reconciler.test.ts | 60 ++++++++++++++++++- .../src/__tests__/github-tracking-state.test.ts | 10 +++- .../dashboard/src/github-source-issue-close.ts | 32 ++++++---- .../dashboard/src/github-tracking-reconciler.ts | 14 +++-- packages/dashboard/src/github-tracking-state.ts | 4 ++ 6 files changed, 167 insertions(+), 23 deletions(-) Fusion-Task-Id: FN-6194 Fusion-Task-Lineage: 867527a0-6eb9-416e-b1eb-d690fa029d60 --- .../github-source-issue-close.test.ts | 70 ++++++++++++++++++- .../github-tracking-reconciler.test.ts | 60 +++++++++++++++- .../__tests__/github-tracking-state.test.ts | 10 ++- .../src/github-source-issue-close.ts | 34 +++++---- .../src/github-tracking-reconciler.ts | 14 ++-- .../dashboard/src/github-tracking-state.ts | 4 ++ 6 files changed, 168 insertions(+), 24 deletions(-) diff --git a/packages/dashboard/src/__tests__/github-source-issue-close.test.ts b/packages/dashboard/src/__tests__/github-source-issue-close.test.ts index dbaa772b78..1284458f1b 100644 --- a/packages/dashboard/src/__tests__/github-source-issue-close.test.ts +++ b/packages/dashboard/src/__tests__/github-source-issue-close.test.ts @@ -121,6 +121,65 @@ describe("GitHubSourceIssueCloseService", () => { expect(store.logEntry).toHaveBeenCalledWith("FN-1", "Closed linked GitHub source issue", "owner/repo#42"); }); + it.each([ + ["todo", "not_planned"], + ["in-progress", "not_planned"], + ["done", "completed"], + ])("closes source issue when archived from %s", async (from, stateReason) => { + service.start(); + store.emit("task:moved", { ...createEvent(), from, to: "archived" }); + await flushAsync(); + expect(mockSetIssueState).toHaveBeenCalledWith("owner", "repo", 42, "closed", stateReason); + expect(store.logEntry).toHaveBeenCalledWith("FN-1", "Closed linked GitHub source issue", "owner/repo#42"); + }); + + it("reopens source issue when unarchived to done", async () => { + mockGetIssue.mockResolvedValueOnce({ state: "closed" }); + service.start(); + store.emit("task:moved", { ...createEvent(), from: "archived", to: "done" }); + await flushAsync(); + expect(mockSetIssueState).toHaveBeenCalledWith("owner", "repo", 42, "open", "reopened"); + expect(store.logEntry).toHaveBeenCalledWith("FN-1", "Reopened linked GitHub source issue", "owner/repo#42"); + }); + + it("reopens source issue when moved out of done", async () => { + mockGetIssue.mockResolvedValueOnce({ state: "closed" }); + service.start(); + store.emit("task:moved", { ...createEvent(), from: "done", to: "todo" }); + await flushAsync(); + expect(mockSetIssueState).toHaveBeenCalledWith("owner", "repo", 42, "open", "reopened"); + expect(store.logEntry).toHaveBeenCalledWith("FN-1", "Reopened linked GitHub source issue", "owner/repo#42"); + }); + + it("does nothing for archived source issue transition when setting is disabled", async () => { + store.getSettings.mockResolvedValueOnce({ githubCloseSourceIssueOnDone: false }); + service.start(); + store.emit("task:moved", { ...createEvent(), to: "archived" }); + await flushAsync(); + expect(mockGetIssue).not.toHaveBeenCalled(); + expect(mockSetIssueState).not.toHaveBeenCalled(); + expect(store.logEntry).not.toHaveBeenCalled(); + }); + + it("ignores archived transitions without sourceIssue", async () => { + service.start(); + store.emit("task:moved", { ...createEvent({ sourceIssue: undefined }), to: "archived" }); + await flushAsync(); + expect(mockGetIssue).not.toHaveBeenCalled(); + expect(mockSetIssueState).not.toHaveBeenCalled(); + }); + + it("ignores archived transitions for non-github provider", async () => { + service.start(); + store.emit("task:moved", { + ...createEvent({ sourceIssue: { provider: "jira", repository: "owner/repo", issueNumber: 42 } }), + to: "archived", + }); + await flushAsync(); + expect(mockGetIssue).not.toHaveBeenCalled(); + expect(mockSetIssueState).not.toHaveBeenCalled(); + }); + it("retries transient close failures once", async () => { service.start(); mockSetIssueState.mockRejectedValueOnce(new Error("ECONNRESET")); @@ -138,20 +197,27 @@ describe("GitHubSourceIssueCloseService", () => { expect(store.logEntry).toHaveBeenCalledWith("FN-1", "Failed to close linked GitHub source issue", "bad request"); }); - it("no-ops when to is not done", async () => { + it("no-ops for non-actionable transitions", async () => { service.start(); store.emit("task:moved", { ...createEvent(), to: "in-review" }); await flushAsync(); expect(mockSetIssueState).not.toHaveBeenCalled(); }); - it("no-ops when from is done", async () => { + it("no-ops when remaining done", async () => { service.start(); store.emit("task:moved", { ...createEvent(), from: "done", to: "done" }); await flushAsync(); expect(mockSetIssueState).not.toHaveBeenCalled(); }); + it("no-ops when remaining archived", async () => { + service.start(); + store.emit("task:moved", { ...createEvent(), from: "archived", to: "archived" }); + await flushAsync(); + expect(mockSetIssueState).not.toHaveBeenCalled(); + }); + it("skips and logs when auth resolution fails", async () => { mockResolveGithubTrackingAuth.mockReturnValueOnce({ ok: false, message: "no auth" }); service.start(); diff --git a/packages/dashboard/src/__tests__/github-tracking-reconciler.test.ts b/packages/dashboard/src/__tests__/github-tracking-reconciler.test.ts index 2d2b5edf08..bca46d01e1 100644 --- a/packages/dashboard/src/__tests__/github-tracking-reconciler.test.ts +++ b/packages/dashboard/src/__tests__/github-tracking-reconciler.test.ts @@ -26,6 +26,7 @@ function createStore(options: { listTasks?: Array>; reconcileCandidates?: Array>; reconcileHasMore?: boolean; + settings?: Record; }): TaskStore { return { listTasks: vi.fn().mockResolvedValue(options.listTasks ?? []), @@ -33,7 +34,7 @@ function createStore(options: { .fn() .mockResolvedValue({ tasks: options.reconcileCandidates ?? [], hasMore: options.reconcileHasMore ?? false }), logEntry: vi.fn().mockResolvedValue(undefined), - getSettings: vi.fn().mockResolvedValue({ githubAuthMode: "token", githubAuthToken: "ghp_test" }), + getSettings: vi.fn().mockResolvedValue(options.settings ?? { githubAuthMode: "token", githubAuthToken: "ghp_test" }), getGlobalSettingsStore: vi.fn(() => ({ getSettings: vi.fn().mockResolvedValue({}) })), } as unknown as TaskStore; } @@ -50,10 +51,28 @@ describe("GitHubTrackingReconciler", () => { const result = await new GitHubTrackingReconciler().reconcile(store); + expect((store.listTasks as any)).toHaveBeenCalledWith({ slim: true, includeArchived: true }); expect(mockSetIssueState).toHaveBeenCalledWith("o", "r", 1, "closed", "completed"); expect(result.closed).toBe(1); }); + it("closes open issues for archived tracked tasks using completion heuristic", async () => { + mockResolveGithubTrackingAuth.mockReturnValue({ ok: true, auth: { mode: "token", token: "ghp_test" } }); + mockGetIssue.mockResolvedValue({ state: "open" }); + const store = createStore({ + listTasks: [ + { id: "FN-1", column: "archived", executionCompletedAt: "2026-01-01T00:00:00.000Z", githubTracking: { enabled: true, issue: { owner: "o", repo: "r", number: 1 } } }, + { id: "FN-2", column: "archived", githubTracking: { enabled: true, issue: { owner: "o", repo: "r", number: 2 } } }, + ], + }); + + const result = await new GitHubTrackingReconciler().reconcile(store); + + expect(mockSetIssueState).toHaveBeenCalledWith("o", "r", 1, "closed", "completed"); + expect(mockSetIssueState).toHaveBeenCalledWith("o", "r", 2, "closed", "not_planned"); + expect(result).toMatchObject({ scanned: 2, closed: 2, skipped: 0, errors: 0 }); + }); + it("skips closed issues and invalid tracking tasks", async () => { mockResolveGithubTrackingAuth.mockReturnValue({ ok: true, auth: { mode: "token", token: "ghp_test" } }); mockGetIssue.mockResolvedValue({ state: "closed" }); @@ -119,6 +138,45 @@ describe("GitHubTrackingReconciler", () => { expect(maxInFlight).toBeLessThanOrEqual(RECONCILE_CONCURRENCY_LIMIT); }); + describe("reconcileSourceIssues", () => { + const sourceSettings = { githubCloseSourceIssueOnDone: true, githubAuthMode: "token", githubAuthToken: "ghp_test" }; + + it("scans done and archived GitHub source issues including archived tasks", async () => { + mockResolveGithubTrackingAuth.mockReturnValue({ ok: true, auth: { mode: "token", token: "ghp_test" } }); + mockGetIssue.mockResolvedValue({ state: "open" }); + const store = createStore({ + settings: sourceSettings, + listTasks: [ + { id: "FN-1", column: "done", sourceIssue: { provider: "github", repository: "o/r", issueNumber: 1 } }, + { id: "FN-2", column: "archived", executionCompletedAt: "2026-01-01T00:00:00.000Z", sourceIssue: { provider: "github", repository: "o/r", issueNumber: 2 } }, + { id: "FN-3", column: "archived", sourceIssue: { provider: "github", repository: "o/r", issueNumber: 3 } }, + { id: "FN-4", column: "todo", sourceIssue: { provider: "github", repository: "o/r", issueNumber: 4 } }, + { id: "FN-5", column: "archived", sourceIssue: { provider: "jira", repository: "o/r", issueNumber: 5 } }, + ], + }); + + const result = await new GitHubTrackingReconciler().reconcileSourceIssues(store); + + expect((store.listTasks as any)).toHaveBeenCalledWith({ slim: false, includeArchived: true }); + expect(mockSetIssueState).toHaveBeenCalledWith("o", "r", 1, "closed", "completed"); + expect(mockSetIssueState).toHaveBeenCalledWith("o", "r", 2, "closed", "completed"); + expect(mockSetIssueState).toHaveBeenCalledWith("o", "r", 3, "closed", "not_planned"); + expect(result).toMatchObject({ scanned: 3, closed: 3, skipped: 0, errors: 0 }); + }); + + it("skips source issue reconciliation when close-on-done is disabled", async () => { + const store = createStore({ + settings: { githubCloseSourceIssueOnDone: false, githubAuthMode: "token", githubAuthToken: "ghp_test" }, + listTasks: [{ id: "FN-1", column: "archived", sourceIssue: { provider: "github", repository: "o/r", issueNumber: 1 } }], + }); + + const result = await new GitHubTrackingReconciler().reconcileSourceIssues(store); + + expect(mockSetIssueState).not.toHaveBeenCalled(); + expect(result).toEqual({ scanned: 1, closed: 0, skipped: 1, errors: 0 }); + }); + }); + describe("reconcileDeletedAndArchived", () => { it("closes with not_planned for soft-deleted tasks", async () => { mockResolveGithubTrackingAuth.mockReturnValue({ ok: true, auth: { mode: "token", token: "ghp_test" } }); diff --git a/packages/dashboard/src/__tests__/github-tracking-state.test.ts b/packages/dashboard/src/__tests__/github-tracking-state.test.ts index 431d70290d..eb961e9781 100644 --- a/packages/dashboard/src/__tests__/github-tracking-state.test.ts +++ b/packages/dashboard/src/__tests__/github-tracking-state.test.ts @@ -76,10 +76,14 @@ describe("decideIssueAction", () => { const columns = ["triage", "todo", "in-progress", "in-review", "done", "archived"] as const; const activeColumns = ["triage", "todo", "in-progress", "in-review"] as const; - it.each(columns.filter((from) => from !== "done"))("returns close for %s -> done", (from) => { + it.each(columns.filter((from) => from !== "done" && from !== "archived"))("returns close for %s -> done", (from) => { expect(decideIssueAction(from, "done")).toEqual({ action: "close", stateReason: "completed" }); }); + it("returns reopen for archived -> done", () => { + expect(decideIssueAction("archived", "done")).toEqual({ action: "reopen", stateReason: "reopened" }); + }); + it.each(activeColumns)("returns reopen for done -> %s", (to) => { expect(decideIssueAction("done", to)).toEqual({ action: "reopen", stateReason: "reopened" }); }); @@ -146,13 +150,13 @@ describe("GitHubTrackingStateService", () => { expect(store.logEntry).toHaveBeenCalledWith("FN-1", "Closed linked GitHub tracking issue", "owner/repo#42"); }); - it("closes on archived -> done", async () => { + it("reopens on archived -> done", async () => { service.start(); store.emit("task:moved", { task: createTask(), from: "archived", to: "done" }); await flushAsync(); - expect(mockSetIssueState).toHaveBeenCalledWith("owner", "repo", 42, "closed", "completed"); + expect(mockSetIssueState).toHaveBeenCalledWith("owner", "repo", 42, "open", "reopened"); }); it.each(["todo", "triage", "in-progress", "in-review"] as const)("reopens on done -> %s", async (to) => { diff --git a/packages/dashboard/src/github-source-issue-close.ts b/packages/dashboard/src/github-source-issue-close.ts index 77dd284ba2..379caa4298 100644 --- a/packages/dashboard/src/github-source-issue-close.ts +++ b/packages/dashboard/src/github-source-issue-close.ts @@ -1,7 +1,7 @@ import type { GlobalSettings, ProjectSettings, TaskStore } from "@fusion/core"; import { resolveGithubTrackingAuth } from "./github-auth.js"; import { GitHubClient } from "./github.js"; -import { delay, isTransientGitHubError } from "./github-tracking-state.js"; +import { decideIssueAction, delay, isTransientGitHubError } from "./github-tracking-state.js"; interface TaskMovedEvent { task: { @@ -66,15 +66,17 @@ export class GitHubSourceIssueCloseService { } private async handleTaskMoved(store: TaskStore, event: TaskMovedEvent): Promise { - if (event.to !== "done" || event.from === "done") { - return; - } - const settings = ((await store.getSettings()) ?? {}) as Pick; if (settings.githubCloseSourceIssueOnDone !== true) { return; } + const action = decideIssueAction(event.from, event.to); + if (!action) { + return; + } + const state = action.action === "close" ? "closed" : "open"; + const sourceIssue = event.task.sourceIssue; if (!sourceIssue || sourceIssue.provider !== "github") { return; @@ -107,26 +109,34 @@ export class GitHubSourceIssueCloseService { : new GitHubClient({ forceMode: "gh-cli" }); const existing = await client.getIssue(owner, repo, issueNumberValue); - if (!existing || existing.state === "closed") { - await store.logEntry(event.task.id, "Skipped closing GitHub source issue - issue not found or already closed", `${owner}/${repo}#${issueNumberValue}`); + if (!existing || existing.state === state) { + await store.logEntry( + event.task.id, + `Skipped ${action.action === "close" ? "closing" : "reopening"} GitHub source issue - issue not found or already ${state}`, + `${owner}/${repo}#${issueNumberValue}`, + ); return; } - const closeIssue = async () => { - await client.setIssueState(owner, repo, issueNumberValue, "closed", "completed"); + const applyIssueAction = async () => { + await client.setIssueState(owner, repo, issueNumberValue, state, action.stateReason); }; try { - await closeIssue(); + await applyIssueAction(); } catch (error) { if (!isTransientGitHubError(error)) { throw error; } await delay(25); - await closeIssue(); + await applyIssueAction(); } - await store.logEntry(event.task.id, "Closed linked GitHub source issue", `${owner}/${repo}#${issueNumberValue}`); + await store.logEntry( + event.task.id, + `${action.action === "close" ? "Closed" : "Reopened"} linked GitHub source issue`, + `${owner}/${repo}#${issueNumberValue}`, + ); } catch (error) { await store.logEntry( event.task.id, diff --git a/packages/dashboard/src/github-tracking-reconciler.ts b/packages/dashboard/src/github-tracking-reconciler.ts index a8e2d720eb..2d46243516 100644 --- a/packages/dashboard/src/github-tracking-reconciler.ts +++ b/packages/dashboard/src/github-tracking-reconciler.ts @@ -7,9 +7,9 @@ const RECONCILE_CONCURRENCY_LIMIT = 4; export class GitHubTrackingReconciler { async reconcile(store: TaskStore): Promise<{ scanned: number; closed: number; skipped: number; errors: number }> { - const listedTasks = await store.listTasks({ slim: true, includeArchived: false }); + const listedTasks = await store.listTasks({ slim: true, includeArchived: true }); const tasks = (Array.isArray(listedTasks) ? listedTasks : []) - .filter((task) => task.column === "done") + .filter((task) => task.column === "done" || task.column === "archived") .slice(0, RECONCILE_SCAN_LIMIT); const projectSettings = ((await store.getSettings()) ?? {}) as Pick; @@ -44,7 +44,8 @@ export class GitHubTrackingReconciler { return; } - await client.setIssueState(issue.owner, issue.repo, issue.number, "closed", "completed"); + const stateReason = task.column === "archived" && !task.executionCompletedAt ? "not_planned" : "completed"; + await client.setIssueState(issue.owner, issue.repo, issue.number, "closed", stateReason); closed += 1; } catch (error) { errors += 1; @@ -60,9 +61,9 @@ export class GitHubTrackingReconciler { } async reconcileSourceIssues(store: TaskStore): Promise<{ scanned: number; closed: number; skipped: number; errors: number }> { - const listedTasks = await store.listTasks({ slim: false, includeArchived: false }); + const listedTasks = await store.listTasks({ slim: false, includeArchived: true }); const tasks = (Array.isArray(listedTasks) ? listedTasks : []) - .filter((task) => task.column === "done" && task.sourceIssue?.provider === "github") + .filter((task) => (task.column === "done" || task.column === "archived") && task.sourceIssue?.provider === "github") .slice(0, RECONCILE_SCAN_LIMIT); const projectSettings = ((await store.getSettings()) ?? {}) as Pick; @@ -105,7 +106,8 @@ export class GitHubTrackingReconciler { return; } - await client.setIssueState(owner, repo, issueNumberValue, "closed", "completed"); + const stateReason = task.column === "archived" && !task.executionCompletedAt ? "not_planned" : "completed"; + await client.setIssueState(owner, repo, issueNumberValue, "closed", stateReason); closed += 1; } catch (error) { errors += 1; diff --git a/packages/dashboard/src/github-tracking-state.ts b/packages/dashboard/src/github-tracking-state.ts index 02112e3d30..c746b2343e 100644 --- a/packages/dashboard/src/github-tracking-state.ts +++ b/packages/dashboard/src/github-tracking-state.ts @@ -30,6 +30,10 @@ export function decideIssueAction( from: string, to: string, ): { action: "close" | "reopen"; stateReason: "completed" | "not_planned" | "reopened" } | null { + if (from === "archived" && to === "done") { + return { action: "reopen", stateReason: "reopened" }; + } + if (to === "done" && from !== "done") { return { action: "close", stateReason: "completed" }; }