diff --git a/.changeset/fn-8227-tracked-issue-commit-link.md b/.changeset/fn-8227-tracked-issue-commit-link.md new file mode 100644 index 0000000000..1457edaab6 --- /dev/null +++ b/.changeset/fn-8227-tracked-issue-commit-link.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Closed GitHub tracked issues now reliably link the landing commit. +category: fix +dev: GitHubTrackingCommentService re-reads the authoritative task via store.getTask before building the Done comment, so mergeDetails.commitSha present at closure time is linked even when the task:moved snapshot omitted it (autoMerge:false PR merges, no-op landings, recovery finalization). Falls back to the event snapshot on refetch failure. diff --git a/packages/dashboard/src/__tests__/github-tracking-comments.test.ts b/packages/dashboard/src/__tests__/github-tracking-comments.test.ts index 76cc1a7d43..cc3831e670 100644 --- a/packages/dashboard/src/__tests__/github-tracking-comments.test.ts +++ b/packages/dashboard/src/__tests__/github-tracking-comments.test.ts @@ -41,12 +41,15 @@ vi.mock("../cli-package-version.js", async (importOriginal) => ({ class MockStore extends EventEmitter { logEntry: Mock; + getTask: Mock; getSettings: Mock; getGlobalSettingsStore: Mock; constructor() { super(); this.logEntry = vi.fn().mockResolvedValue(undefined); + // Null preserves the event snapshot unless a test supplies a newer authoritative row. + this.getTask = vi.fn().mockResolvedValue(null); this.getSettings = vi.fn().mockResolvedValue({ githubAuthMode: "token", githubAuthToken: "ghp_test" }); this.getGlobalSettingsStore = vi.fn(() => ({ getSettings: vi.fn().mockResolvedValue({}) })); } @@ -503,6 +506,84 @@ describe("GitHubTrackingCommentService", () => { expect(body).not.toContain("Current version"); }); + it("recovers a landing commit from the authoritative row when the done event is stale", async () => { + service.start(); + const snapshot = createTask({ mergeDetails: { prNumber: 7 } }); + store.getTask.mockResolvedValueOnce(createTask({ + mergeDetails: { commitSha: "abcdef1234567890", prNumber: 7 }, + })); + + store.emit("task:moved", { task: snapshot, from: "in-progress", to: "done" }); + await flushAsync(); + + expect(store.getTask).toHaveBeenCalledWith("FN-1"); + expect(mockCommentOnIssue).toHaveBeenCalledTimes(1); + expect(mockCommentOnIssue.mock.calls[0]?.[3]).toContain( + "Commit: [abcdef1](https://github.com/owner/repo/commit/abcdef1234567890)", + ); + }); + + it("keeps an event-snapshot commit link when the authoritative row is unavailable", async () => { + service.start(); + store.getTask.mockRejectedValueOnce(new Error("store unavailable")); + + store.emit("task:moved", { + task: createTask({ mergeDetails: { commitSha: "abcdef1234567890" } }), + from: "in-progress", + to: "done", + }); + await flushAsync(); + + expect(mockCommentOnIssue).toHaveBeenCalledTimes(1); + expect(mockCommentOnIssue.mock.calls[0]?.[3]).toContain( + "Commit: [abcdef1](https://github.com/owner/repo/commit/abcdef1234567890)", + ); + }); + + it("keeps an event-snapshot commit link when the authoritative row is missing", async () => { + service.start(); + store.getTask.mockResolvedValueOnce(null); + + store.emit("task:moved", { + task: createTask({ mergeDetails: { commitSha: "abcdef1234567890" } }), + from: "in-progress", + to: "done", + }); + await flushAsync(); + + expect(mockCommentOnIssue).toHaveBeenCalledTimes(1); + expect(mockCommentOnIssue.mock.calls[0]?.[3]).toContain( + "Commit: [abcdef1](https://github.com/owner/repo/commit/abcdef1234567890)", + ); + }); + + it("posts a no-op landing without a Commit line", async () => { + service.start(); + store.getTask.mockResolvedValueOnce(createTask({ mergeDetails: { noOpMerge: true } })); + + store.emit("task:moved", { + task: createTask({ mergeDetails: { noOpMerge: true } }), + from: "in-progress", + to: "done", + }); + await flushAsync(); + + expect(mockCommentOnIssue).toHaveBeenCalledTimes(1); + expect(mockCommentOnIssue.mock.calls[0]?.[3]).not.toContain("Commit:"); + }); + + it("does not refetch or duplicate a comment for in-progress and same-column transitions", async () => { + service.start(); + + store.emit("task:moved", { task: createTask(), from: "todo", to: "in-progress" }); + store.emit("task:moved", { task: createTask(), from: "done", to: "done" }); + await flushAsync(); + + expect(store.getTask).not.toHaveBeenCalled(); + expect(mockCommentOnIssue).toHaveBeenCalledTimes(1); + expect(mockCommentOnIssue.mock.calls[0]?.[3]).toContain("🚧 In progress"); + }); + it("writes success logs", async () => { service.start(); diff --git a/packages/dashboard/src/github-tracking-comments.ts b/packages/dashboard/src/github-tracking-comments.ts index 14a7bdf951..e9522087ba 100644 --- a/packages/dashboard/src/github-tracking-comments.ts +++ b/packages/dashboard/src/github-tracking-comments.ts @@ -248,9 +248,19 @@ export class GitHubTrackingCommentService { return; } + /* + * FNXC:GitHubTrackingComments 2026-07-16-12:40: + * A closed tracked issue must link its landing commit when one exists. The task:moved snapshot + * can predate mergeDetails persistence on human PR, no-op, and recovery done paths, so re-read + * the authoritative row before building the Done comment. Fall back to the snapshot when the + * read fails so the comment is never dropped. + */ + const taskForComment = event.to === "done" + ? await this.store.getTask(event.task.id).catch(() => null) ?? event.task + : event.task; const body = event.to === "done" - ? formatTrackingComment(event.task, event.to, { owner, repo }) - : formatTrackingComment(event.task, event.to); + ? formatTrackingComment(taskForComment, event.to, { owner, repo }) + : formatTrackingComment(taskForComment, event.to); try { const projectSettings = await this.store.getSettings() as Pick;