From 3e376b98bad6b35eb94237ec14b76aaa9675c46d Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 19 Jul 2026 16:07:34 -0700 Subject: [PATCH] FN-8369: centralize GitHub issue import deduplication Use provenance-first deduplication consistently across dashboard, CLI, and extension GitHub imports. - Prioritize persisted sourceIssue and legacy metadata over editable descriptions. - Reuse the dashboard deduplication helper for CLI and extension imports. - Prevent duplicate issue creation within a dashboard batch import. Files changed: .changeset/fn-8369-github-import-dedup.md | 7 ++++++ packages/cli/src/__tests__/extension.test.ts | 27 ++++++++++++++++++++++ packages/cli/src/commands/__tests__/task.test.ts | 20 ++++++++++------ packages/cli/src/extension.ts | 25 +++++++++++--------- packages/dashboard/src/__tests__/github.test.ts | 26 ++++++++++++++++----- packages/dashboard/src/__tests__/routes-github.test.ts | 22 ++++++++++++++++++ packages/dashboard/src/github.ts | 27 +++++++++++++--------- 7 files changed, 119 insertions(+), 35 deletions(-) Fusion-Task-Id: FN-8369 Fusion-Task-Lineage: 8eb6d19d-bd0e-487a-9f11-8945d744b7df Co-authored-by: Fusion (runfusion.ai) --- .changeset/fn-8369-github-import-dedup.md | 7 +++++ packages/cli/src/__tests__/extension.test.ts | 27 +++++++++++++++++++ .../cli/src/commands/__tests__/task.test.ts | 20 +++++++++----- packages/cli/src/extension.ts | 25 +++++++++-------- .../dashboard/src/__tests__/github.test.ts | 26 +++++++++++++----- .../src/__tests__/routes-github.test.ts | 22 +++++++++++++++ packages/dashboard/src/github.ts | 27 +++++++++++-------- 7 files changed, 119 insertions(+), 35 deletions(-) create mode 100644 .changeset/fn-8369-github-import-dedup.md diff --git a/.changeset/fn-8369-github-import-dedup.md b/.changeset/fn-8369-github-import-dedup.md new file mode 100644 index 0000000000..63a49cf7cc --- /dev/null +++ b/.changeset/fn-8369-github-import-dedup.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix GitHub issue imports so edited descriptions cannot hide or falsely match prior imports. +category: fix +dev: Centralizes provenance-first deduplication for dashboard, CLI, and extension issue imports. diff --git a/packages/cli/src/__tests__/extension.test.ts b/packages/cli/src/__tests__/extension.test.ts index 1389fad9c3..6dbea90dbe 100644 --- a/packages/cli/src/__tests__/extension.test.ts +++ b/packages/cli/src/__tests__/extension.test.ts @@ -2781,6 +2781,33 @@ legacyDescribe("fn pi extension (legacy exhaustive suite)", () => { expect(result.content[0].text).toContain(existing.id); }); + it("fn_task_browse_github_issues marks sourceIssue-only imports as imported", async () => { + await h.store().createTask({ + title: "Imported issue", + description: "Edited description without source URL", + sourceIssue: { + provider: "github", + repository: "Acme/Demo", + externalIssueId: "10", + issueNumber: 10, + url: "https://github.com/other/repo/issues/99", + }, + }); + const tool = api.tools.get("fn_task_browse_github_issues")!; + vi.mocked(runGhJsonAsync).mockResolvedValueOnce([{ + number: 10, + title: "Investigate latency", + body: null, + html_url: "https://github.com/acme/demo/issues/10", + labels: [], + }] as never); + + const result = await tool.execute("gh-3a", { owner: "acme", repo: "demo" }, undefined, undefined, makeCtx(tmpDir)); + + expect(result.content[0].text).toContain("✓ Imported"); + expect(result.details.issues[0]).toMatchObject({ number: 10, imported: true }); + }); + it("fn_task_browse_github_issues lists issues via gh api", async () => { const tool = api.tools.get("fn_task_browse_github_issues")!; vi.mocked(runGhJsonAsync).mockResolvedValueOnce([ diff --git a/packages/cli/src/commands/__tests__/task.test.ts b/packages/cli/src/commands/__tests__/task.test.ts index 315a5e829c..53eaa13f5a 100644 --- a/packages/cli/src/commands/__tests__/task.test.ts +++ b/packages/cli/src/commands/__tests__/task.test.ts @@ -123,11 +123,7 @@ vi.mock("@fusion/dashboard", () => ({ sourceIssue: { provider: "github", repository: `${owner}/${repo}`, externalIssueId: String(issue.number), issueNumber: issue.number, url: issue.html_url }, sourceMetadata: { issueUrl: issue.html_url, issueNumber: issue.number }, })), - isGitHubIssueAlreadyImported: vi.fn((task: any, input: any) => - task.description?.toLowerCase().includes(input.sourceUrl.toLowerCase()) - || (task.sourceIssue?.provider === "github" - && task.sourceIssue.repository?.toLowerCase() === `${input.owner}/${input.repo}`.toLowerCase() - && (task.sourceIssue.issueNumber === input.issueNumber || task.sourceIssue.externalIssueId === String(input.issueNumber)))), + isGitHubIssueAlreadyImported: vi.fn(), isGitLabAlreadyImported: vi.fn(), buildGitLabTaskDescription: vi.fn(), })); @@ -208,7 +204,7 @@ import { isGhAvailable, runGhJsonAsync, } from "@fusion/core/gh-cli"; -import { GitHubClient, generatePrMetadata } from "@fusion/dashboard"; +import { GitHubClient, generatePrMetadata, isGitHubIssueAlreadyImported } from "@fusion/dashboard"; import { createSession, submitResponse } from "@fusion/dashboard/planning"; import { resolveProject, createLocalStore } from "../../project-context.js"; import { aiMergeTask, runAiMerge, landWorkspaceTask } from "@fusion/engine"; @@ -1769,6 +1765,7 @@ describe("runTaskImportGitHubInteractive", () => { }, }, ]); + vi.mocked(isGitHubIssueAlreadyImported).mockImplementation((_task, input) => input.issueNumber === 1); vi.mocked(runGhJsonAsync).mockResolvedValueOnce([ mockIssue(1, "First Issue", "Description 1"), @@ -2083,7 +2080,7 @@ describe("runTaskImportFromGitHub", () => { })); }); - it("skips already imported issues", async () => { + it("delegates provenance-first deduplication to the dashboard helper", async () => { // Setup existing task with source URL mockListTasks.mockResolvedValueOnce([ { @@ -2100,6 +2097,7 @@ describe("runTaskImportFromGitHub", () => { }, }, ]); + vi.mocked(isGitHubIssueAlreadyImported).mockImplementation((_task, input) => input.issueNumber === 1); vi.mocked(runGhJsonAsync).mockResolvedValueOnce([ mockIssue(1, "First Issue", "Description 1"), @@ -2109,6 +2107,14 @@ describe("runTaskImportFromGitHub", () => { await runTaskImportFromGitHub("owner/repo"); expect(mockCreateTask).toHaveBeenCalledTimes(1); + expect(isGitHubIssueAlreadyImported).toHaveBeenCalledWith(expect.objectContaining({ + sourceIssue: expect.objectContaining({ provider: "github", issueNumber: 1 }), + }), expect.objectContaining({ + owner: "owner", + repo: "repo", + issueNumber: 1, + sourceUrl: "https://github.com/owner/repo/issues/1", + })); const skipLine = logSpy.mock.calls.find( (call) => typeof call[0] === "string" && call[0].includes("Skipping #1"), ); diff --git a/packages/cli/src/extension.ts b/packages/cli/src/extension.ts index 0bc84ca7b8..2c58084a61 100644 --- a/packages/cli/src/extension.ts +++ b/packages/cli/src/extension.ts @@ -2411,21 +2411,24 @@ export default function kbExtension(pi: ExtensionAPI) { // Check which issues are already imported const store = await getStore(ctx.cwd); - const existingTasks = await store.listTasks({ slim: true }); - const importedUrls = new Set(); - - for (const task of existingTasks) { - const match = task.description.match(/Source: (https:\/\/github\.com\/[^/]+\/[^/]+\/issues\/\d+)/); - if (match) { - importedUrls.add(match[1]); - } - } + // FNXC:GithubImport 2026-07-17-00:00: Browse must load full provenance and use the shared helper so its imported marker agrees with all issue-import surfaces even after descriptions are edited. + const existingTasks = await store.listTasks({ slim: false }); + const importedIssueNumbers = new Set( + issues + .filter((issue) => existingTasks.some((task) => dashboard.isGitHubIssueAlreadyImported(task, { + owner, + repo, + issueNumber: issue.number, + sourceUrl: issue.html_url, + }))) + .map((issue) => issue.number), + ); const lines: string[] = []; lines.push(`Found ${issues.length} open issues in ${owner}/${repo}:\n`); for (const issue of issues) { - const isImported = importedUrls.has(issue.html_url); + const isImported = importedIssueNumbers.has(issue.number); const issueLabels = issue.labels ?? []; const labelStr = issueLabels.length > 0 ? ` [${issueLabels.map((label) => label.name).join(", ")}]` : ""; const importedStr = isImported ? " ✓ Imported" : ""; @@ -2444,7 +2447,7 @@ export default function kbExtension(pi: ExtensionAPI) { title: issue.title, url: issue.html_url, labels: (issue.labels ?? []).map((label) => label.name), - imported: importedUrls.has(issue.html_url), + imported: importedIssueNumbers.has(issue.number), })), }, }; diff --git a/packages/dashboard/src/__tests__/github.test.ts b/packages/dashboard/src/__tests__/github.test.ts index 1add14232c..13be256bc9 100644 --- a/packages/dashboard/src/__tests__/github.test.ts +++ b/packages/dashboard/src/__tests__/github.test.ts @@ -2435,19 +2435,33 @@ describe("GitHubClient", () => { describe("isGitHubIssueAlreadyImported", () => { const input = { owner: "owner", repo: "repo", issueNumber: 1, sourceUrl: "https://github.com/owner/repo/issues/1" }; - it("matches edited sourceIssue metadata, legacy source metadata, and description URLs", () => { - expect(isGitHubIssueAlreadyImported({ description: "Source: https://github.com/OWNER/REPO/issues/1" }, input)).toBe(true); + it("prefers persisted sourceIssue provenance over an edited description", () => { expect(isGitHubIssueAlreadyImported({ - description: "Edited description", - sourceIssue: { provider: "github", repository: "Owner/Repo", issueNumber: 1, externalIssueId: "1", url: "https://github.com/other/repo/issues/2" }, + description: "Edited description without source URL", + sourceIssue: { provider: "github", repository: "Owner/Repo", externalIssueId: "1", url: "https://github.com/other/repo/issues/99" }, }, input)).toBe(true); + }); + + it("matches normalized legacy metadata after sourceIssue", () => { expect(isGitHubIssueAlreadyImported({ - description: "Edited description", + description: "Edited description without source URL", source: { sourceType: "github_import", sourceMetadata: { issueUrl: "https://github.com/Owner/Repo/issues/2", issueNumber: 1 } }, }, input)).toBe(true); }); - it("does not match a fresh issue", () => { + it("does not let a description URL override nonmatching structured provenance", () => { + expect(isGitHubIssueAlreadyImported({ + description: `Quoted target URL: ${input.sourceUrl}`, + sourceIssue: { provider: "github", repository: "other/repo", issueNumber: 2, url: "https://github.com/other/repo/issues/2" }, + }, input)).toBe(false); + expect(isGitHubIssueAlreadyImported({ + description: `Quoted target URL: ${input.sourceUrl}`, + source: { sourceType: "github_import", sourceMetadata: { issueUrl: "https://github.com/other/repo/issues/2", issueNumber: 2 } }, + }, input)).toBe(false); + }); + + it("uses description URLs only as the final legacy fallback", () => { + expect(isGitHubIssueAlreadyImported({ description: "Source: https://github.com/OWNER/REPO/issues/1" }, input)).toBe(true); expect(isGitHubIssueAlreadyImported({ description: "Unrelated" }, input)).toBe(false); }); }); diff --git a/packages/dashboard/src/__tests__/routes-github.test.ts b/packages/dashboard/src/__tests__/routes-github.test.ts index 5242eaeb99..49d51276ee 100644 --- a/packages/dashboard/src/__tests__/routes-github.test.ts +++ b/packages/dashboard/src/__tests__/routes-github.test.ts @@ -1419,6 +1419,28 @@ describe("POST /github/issues/batch-import", () => { expect(throttledSpy).toHaveBeenCalledTimes(2); }); + it("deduplicates repeated issue numbers using tasks accumulated within the batch", async () => { + const throttledSpy = vi.spyOn(GitHubClient.prototype, "fetchThrottled") + .mockResolvedValueOnce({ success: true, data: mockGitHubIssue(1, "Duplicate issue") } as Awaited>) + .mockResolvedValueOnce({ success: true, data: mockGitHubIssue(1, "Duplicate issue") } as Awaited>); + + const res = await REQUEST( + buildApp(), + "POST", + "/api/github/issues/batch-import", + JSON.stringify({ owner: "owner", repo: "repo", issueNumbers: [1, 1], delayMs: 1 }), + { "Content-Type": "application/json" }, + ); + + expect(res.status).toBe(200); + expect(res.body.results).toEqual([ + { issueNumber: 1, success: true, taskId: expect.any(String) }, + { issueNumber: 1, success: true, skipped: true, taskId: expect.any(String) }, + ]); + expect(store.createTask).toHaveBeenCalledTimes(1); + expect(throttledSpy).toHaveBeenCalledTimes(2); + }); + it("returns 400 for empty issueNumbers array", async () => { const res = await REQUEST( buildApp(), diff --git a/packages/dashboard/src/github.ts b/packages/dashboard/src/github.ts index 410ba7cb69..2fa6ca477c 100644 --- a/packages/dashboard/src/github.ts +++ b/packages/dashboard/src/github.ts @@ -113,8 +113,11 @@ function parseIssueUrl(stdout: string): { owner: string; repo: string; number: n } /* -FNXC:GithubImport 2026-07-15-00:00: -GitHub import deduplication must survive edited task descriptions and owner/repo casing changes. Both CLI import paths, both extension tools, and dashboard single/batch routes share this sourceIssue-first helper, mirroring GitLab provenance while retaining legacy description URL matching. +FNXC:GithubImport 2026-07-17-00:00: +GitHub issue import deduplication treats persisted provenance as authoritative so edited descriptions and owner/repo casing changes cannot misidentify an import. Every dashboard, CLI, and extension issue-import surface shares this helper, which checks sourceIssue first, legacy github_import metadata second, and legacy description URLs last. + +FNXC:GithubImport 2026-07-17-00:00: +The description compatibility fallback is eligible only when neither GitHub sourceIssue nor object-shaped github_import metadata exists. A nonmatching structured record must return false rather than letting quoted or stale URL text override its provenance. */ export function buildGitHubIssueSource(owner: string, repo: string, issue: { number: number; html_url: string }): { sourceIssue: TaskSourceIssue; @@ -148,12 +151,9 @@ export function isGitHubIssueAlreadyImported( ): boolean { const { owner, repo, issueNumber, sourceUrl } = input; const repository = `${owner}/${repo}`; - const normalizedSourceUrl = sourceUrl.toLocaleLowerCase(); - - if (task.description?.toLocaleLowerCase().includes(normalizedSourceUrl)) return true; - const sourceIssue = task.sourceIssue; - if (sourceIssue?.provider === "github") { + const hasGitHubSourceIssue = sourceIssue?.provider === "github"; + if (hasGitHubSourceIssue) { if (equalsIgnoreCase(sourceIssue.url, sourceUrl)) return true; if (equalsIgnoreCase(sourceIssue.repository, repository) && (sourceIssue.issueNumber === issueNumber || sourceIssue.externalIssueId === String(issueNumber))) { @@ -162,14 +162,19 @@ export function isGitHubIssueAlreadyImported( } const metadata = task.source?.sourceMetadata; - if (task.source?.sourceType === "github_import" && metadata && typeof metadata === "object") { + const hasGitHubSourceMetadata = task.source?.sourceType === "github_import" && metadata && typeof metadata === "object"; + if (hasGitHubSourceMetadata) { const sourceMetadata = metadata as Record; if (equalsIgnoreCase(typeof sourceMetadata.issueUrl === "string" ? sourceMetadata.issueUrl : undefined, sourceUrl)) return true; - return sourceMetadata.issueNumber === issueNumber - && equalsIgnoreCase(repositoryFromGitHubIssueUrl(sourceMetadata.issueUrl), repository); + if (sourceMetadata.issueNumber === issueNumber + && equalsIgnoreCase(repositoryFromGitHubIssueUrl(sourceMetadata.issueUrl), repository)) { + return true; + } } - return false; + if (hasGitHubSourceIssue || hasGitHubSourceMetadata) return false; + + return task.description?.toLocaleLowerCase().includes(sourceUrl.toLocaleLowerCase()) ?? false; } /**