diff --git a/.changeset/github-import-and-issue-close-fixes.md b/.changeset/github-import-and-issue-close-fixes.md new file mode 100644 index 0000000000..e05d07caf7 --- /dev/null +++ b/.changeset/github-import-and-issue-close-fixes.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": minor +--- + +summary: GitHub issue import now pages through all open issues with Previous/Next controls, and linked issues reliably close when their task reaches Done. +category: feature +dev: The import picker (GitHubImportModal) fetches up to 300 open issues in one request and pages the result client-side at 30/page with Prev/Next controls and a page indicator; a truncation notice appears past the cap. NewTaskModal's reference picker limit rose 30→100. GitHubClient.listIssues now pages the REST path (per_page loop until limit/exhaustion, PR-filtering no longer stops paging early) and lifts the gh path's 100 cap (gh --limit paginates internally); gh-CLI label filtering fetches the full cap before client-side OR filtering. Separately, the GitHub-tracking reconcile sweep now isolates its three passes in runSweep so a throw in one pass no longer silently starves the others — previously a failure in the first pass disabled the entire close-on-Done backstop, leaving linked/imported issues open; failures are now logged instead of swallowed. diff --git a/packages/dashboard/app/components/GitHubImportModal.css b/packages/dashboard/app/components/GitHubImportModal.css index 1e5a5bdd19..f2148c7612 100644 --- a/packages/dashboard/app/components/GitHubImportModal.css +++ b/packages/dashboard/app/components/GitHubImportModal.css @@ -1640,3 +1640,30 @@ Import Tasks embedded header now adopts the canonical ViewHeader chrome — edge font-size: 13px; } } + +/* +FNXC:GitHubImport 2026-07-16-16:20: +Page controls for the issues list. Reuses the shared `.btn`/`.btn-secondary`/`.btn-sm` primitives (only layout +lives here) and design tokens for spacing/color so the bar matches the modal's other footers in light and dark. +*/ +.github-import-pagination { + display: flex; + align-items: center; + justify-content: space-between; + gap: var(--space-sm); + padding: var(--space-sm) var(--space-xs) 0; +} + +.github-import-pagination__status { + color: var(--text-muted); + font-size: var(--font-size-sm, 13px); + text-align: center; + flex: 1 1 auto; +} + +.github-import-pagination__truncation { + color: var(--text-muted); + font-size: var(--font-size-sm, 13px); + padding: var(--space-xs) var(--space-xs) 0; + text-align: center; +} diff --git a/packages/dashboard/app/components/GitHubImportModal.tsx b/packages/dashboard/app/components/GitHubImportModal.tsx index 45c20673b3..a22275273e 100644 --- a/packages/dashboard/app/components/GitHubImportModal.tsx +++ b/packages/dashboard/app/components/GitHubImportModal.tsx @@ -290,6 +290,18 @@ The list endpoint already returns the complete (untruncated) body, so no per-ite The full body renders as GitHub-flavored markdown via the shared MailboxMessageContent component; the preview pane is already scrollable (prior fix), so the body takes full height with no line clamping. */ +/* +FNXC:GitHubImport 2026-07-16-16:20: +Repos with >100 open issues need page controls, not a silently-truncated single fetch. Design: fetch up +to ISSUES_FETCH_CAP issues in ONE request (the server + gh paginate internally to reach it), then page the +result client-side at ISSUES_PAGE_SIZE per page. Client-side paging is used deliberately because the label +filter (OR semantics) and the "hide imported" toggle both already filter the fetched set in-memory — real +server-side paging would fight both and the gh CLI has no offset. When a repo exceeds the cap, a truncation +notice tells the operator to refine by label rather than pretending the list is complete. +*/ +const ISSUES_FETCH_CAP = 300; +const ISSUES_PAGE_SIZE = 30; + export function GitHubImportModal({ isOpen, onClose, onImport, tasks, projectId, presentation = "modal" }: GitHubImportModalProps) { const { isEmbedded, scrollLockEnabled, resizePersistEnabled, escapeEnabled } = useEmbeddedPresentation(presentation); useMobileScrollLock(isOpen && scrollLockEnabled); @@ -385,6 +397,8 @@ export function GitHubImportModal({ isOpen, onClose, onImport, tasks, projectId, // Issues state const [issues, setIssues] = useState([]); const [selectedIssueNumber, setSelectedIssueNumber] = useState(null); + // FNXC:GitHubImport 2026-07-16-16:20: 0-based client-side page index for the issues list (reset on every reload/filter change). + const [issuePage, setIssuePage] = useState(0); // Pulls state const [pulls, setPulls] = useState([]); @@ -478,6 +492,11 @@ export function GitHubImportModal({ isOpen, onClose, onImport, tasks, projectId, */ const [optimisticImportedUrls, setOptimisticImportedUrls] = useState>(new Set()); + // FNXC:GitHubImport 2026-07-16-16:20: Reset the issues page whenever the filtered set changes shape (tab switch or hide-imported toggle) so the operator always lands on page 1 of the new view. + useEffect(() => { + setIssuePage(0); + }, [hideImported, activeTab]); + // Build set of already imported URLs from existing tasks const importedUrls = new Set(); for (const task of tasks) { @@ -675,13 +694,19 @@ export function GitHubImportModal({ isOpen, onClose, onImport, tasks, projectId, setIsIssuesEmptyState(false); setIssues([]); setSelectedIssueNumber(null); + setIssuePage(0); try { const labelArray = labels .split(",") .map((l) => l.trim()) .filter(Boolean); - const fetchedIssues = await apiFetchGitHubIssues(owner.trim(), repo.trim(), 30, labelArray.length > 0 ? labelArray : undefined); + /* + FNXC:GitHubImport 2026-07-16-16:20: + Fetch up to ISSUES_FETCH_CAP in one request (server + gh paginate internally to reach it), then page + the result client-side. Replaces the earlier hardcoded 30/100 caps that silently hid issues past the limit. + */ + const fetchedIssues = await apiFetchGitHubIssues(owner.trim(), repo.trim(), ISSUES_FETCH_CAP, labelArray.length > 0 ? labelArray : undefined); setIssues(fetchedIssues); if (fetchedIssues.length === 0) { setIsIssuesEmptyState(true); @@ -1212,6 +1237,17 @@ export function GitHubImportModal({ isOpen, onClose, onImport, tasks, projectId, imported counts always use the full fetched sets. Use `isUrlImported` so optimistic imports also disappear immediately. */ const visibleIssues = hideImported ? issues.filter((issue) => !isUrlImported(issue.html_url)) : issues; + /* + FNXC:GitHubImport 2026-07-16-16:20: + Client-side page window over the (already label/hide-imported filtered) visible issues. issuePage is + clamped here so shrinking the filtered set (e.g. toggling "hide imported") can never strand the view on + an out-of-range page. isIssuesTruncated flags that the repo hit ISSUES_FETCH_CAP so the operator knows + the list is bounded, not complete. + */ + const issuePageCount = Math.max(1, Math.ceil(visibleIssues.length / ISSUES_PAGE_SIZE)); + const clampedIssuePage = Math.min(issuePage, issuePageCount - 1); + const pagedIssues = visibleIssues.slice(clampedIssuePage * ISSUES_PAGE_SIZE, (clampedIssuePage + 1) * ISSUES_PAGE_SIZE); + const isIssuesTruncated = issues.length >= ISSUES_FETCH_CAP; const visiblePulls = hideImported ? pulls.filter((pull) => !isUrlImported(pull.html_url)) : pulls; const visibleGitlabItems = hideImported ? gitlabItems.filter((item) => !isUrlImported(item.webUrl)) : gitlabItems; const allIssuesHidden = hideImported && issues.length > 0 && visibleIssues.length === 0; @@ -1532,7 +1568,7 @@ export function GitHubImportModal({ isOpen, onClose, onImport, tasks, projectId, {/* Issues list */} {activeTab === "issues" && visibleIssues.length > 0 && (
- {visibleIssues.map((issue) => { + {pagedIssues.map((issue) => { const isImported = isUrlImported(issue.html_url); return (
)} + {/* + FNXC:GitHubImport 2026-07-16-16:20: + Page controls appear only when the filtered set exceeds one page. Prev/Next are gated on + clampedIssuePage so they can never navigate out of range. The truncation notice renders when the + fetch hit ISSUES_FETCH_CAP, telling the operator the list is bounded and to refine by label. + */} + {activeTab === "issues" && visibleIssues.length > ISSUES_PAGE_SIZE && ( +
+ + + {t("git.pageStatus", "Page {{page}} of {{total}}", { page: clampedIssuePage + 1, total: issuePageCount })} + {" · "} + {t("git.issuesCount", "{{count}} issues", { count: visibleIssues.length })} + + +
+ )} + + {activeTab === "issues" && isIssuesTruncated && ( +
+ {t("git.issuesTruncated", "Showing the first {{cap}} open issues. Refine with a label filter to narrow the list.", { cap: ISSUES_FETCH_CAP })} +
+ )} + {activeTab === "pulls" && allPullsHidden && (
{t("git.allImportedHidden", "All loaded items are already imported")}
diff --git a/packages/dashboard/app/components/NewTaskModal.tsx b/packages/dashboard/app/components/NewTaskModal.tsx index 63d54f115a..f2f2943a3a 100644 --- a/packages/dashboard/app/components/NewTaskModal.tsx +++ b/packages/dashboard/app/components/NewTaskModal.tsx @@ -143,7 +143,13 @@ function writeFloatPosition(position: FloatPosition, size: FloatSize): FloatPosi type FloatResizeDirection = "n" | "s" | "e" | "w" | "ne" | "nw" | "se" | "sw"; const NEW_TASK_RESIZE_DIRECTIONS: FloatResizeDirection[] = ["n", "s", "e", "w", "ne", "nw", "se", "sw"]; -const NEW_TASK_GITHUB_REFERENCE_LIMIT = 30; +/* +FNXC:GitHubImport 2026-07-16-15:20: +Reference picker must surface all of a normal repo's open issues/PRs, not just the first 30. +The server + GitHubClient clamp a single fetch at 100 (one GitHub page), so 100 is the effective ceiling here. +Was 30, which silently hid every issue past the 30th for any repo with more open issues. +*/ +const NEW_TASK_GITHUB_REFERENCE_LIMIT = 100; type GitHubReferenceOption = | { type: "issue"; number: number; title: string; url: string } diff --git a/packages/dashboard/app/components/__tests__/GitHubImportModal.test.tsx b/packages/dashboard/app/components/__tests__/GitHubImportModal.test.tsx index 585b9b3a3e..4bd68bd5f0 100644 --- a/packages/dashboard/app/components/__tests__/GitHubImportModal.test.tsx +++ b/packages/dashboard/app/components/__tests__/GitHubImportModal.test.tsx @@ -705,7 +705,7 @@ describe("GitHubImportModal", () => { render(); await waitFor(() => { - expect(apiFetchGitHubIssues).toHaveBeenCalledWith("dustinbyrne", "kb", 30, undefined); + expect(apiFetchGitHubIssues).toHaveBeenCalledWith("dustinbyrne", "kb", 300, undefined); }); expect(screen.getByText("Auto-loaded Issue")).toBeTruthy(); @@ -746,7 +746,7 @@ describe("GitHubImportModal", () => { await waitFor(() => { const select = screen.getByRole("combobox") as HTMLSelectElement; expect(select.value).toBe("origin"); - expect(apiFetchGitHubIssues).toHaveBeenCalledWith("dustinbyrne", "kb", 30, undefined); + expect(apiFetchGitHubIssues).toHaveBeenCalledWith("dustinbyrne", "kb", 300, undefined); expect(screen.getByText("Auto-loaded from origin")).toBeTruthy(); }); @@ -779,14 +779,14 @@ describe("GitHubImportModal", () => { await waitFor(() => { const select = screen.getByRole("combobox") as HTMLSelectElement; expect(select.value).toBe("origin"); - expect(apiFetchGitHubIssues).toHaveBeenCalledWith("dustinbyrne", "kb", 30, undefined); + expect(apiFetchGitHubIssues).toHaveBeenCalledWith("dustinbyrne", "kb", 300, undefined); expect(screen.getByText("Issue from origin")).toBeTruthy(); }); fireEvent.change(screen.getByRole("combobox"), { target: { value: "upstream" } }); await waitFor(() => { - expect(apiFetchGitHubIssues).toHaveBeenLastCalledWith("upstream", "kb", 30, undefined); + expect(apiFetchGitHubIssues).toHaveBeenLastCalledWith("upstream", "kb", 300, undefined); expect(screen.getByText("Issue from upstream")).toBeTruthy(); }); }); @@ -809,6 +809,52 @@ describe("GitHubImportModal", () => { }); }); + /* + FNXC:GitHubImport 2026-07-16-16:20: + Page controls for repos with >1 page (30/page). Asserts page 1 shows only the first 30, the pager reports + the right page/total, and Next reveals the next page — the behavior missing when the list was capped at 30. + */ + it("paginates the issue list with Previous/Next controls (30 per page)", async () => { + const manyIssues = Array.from({ length: 65 }, (_, i) => ({ + number: i + 1, title: `Issue ${i + 1}`, body: `Body ${i + 1}`, + html_url: `https://github.com/owner/repo/issues/${i + 1}`, labels: [], + })); + vi.mocked(fetchGitRemotes).mockResolvedValueOnce(singleRemote); + vi.mocked(apiFetchGitHubIssues).mockResolvedValueOnce(manyIssues); + + render(); + + // Page 1 shows issues 1–30 only. + await waitFor(() => expect(screen.getByText("Issue 1")).toBeTruthy()); + expect(screen.getByText("Issue 30")).toBeTruthy(); + expect(screen.queryByText("Issue 31")).toBeNull(); + expect(screen.getByText(/Page 1 of 3/)).toBeTruthy(); + + // Previous is disabled on the first page; Next advances to page 2 (issues 31–60). + const prev = screen.getByRole("button", { name: /Previous/i }); + const next = screen.getByRole("button", { name: /Next/i }); + expect((prev as HTMLButtonElement).disabled).toBe(true); + + fireEvent.click(next); + await waitFor(() => expect(screen.getByText("Issue 31")).toBeTruthy()); + expect(screen.queryByText("Issue 1")).toBeNull(); + expect(screen.getByText("Issue 60")).toBeTruthy(); + expect(screen.getByText(/Page 2 of 3/)).toBeTruthy(); + }); + + it("shows no page controls when the issue list fits on one page", async () => { + const issues = [ + { number: 1, title: "Only Issue", body: "Body", html_url: "https://github.com/owner/repo/issues/1", labels: [] }, + ]; + vi.mocked(fetchGitRemotes).mockResolvedValueOnce(singleRemote); + vi.mocked(apiFetchGitHubIssues).mockResolvedValueOnce(issues); + + render(); + + await waitFor(() => expect(screen.getByText("Only Issue")).toBeTruthy()); + expect(screen.queryByRole("button", { name: /Next/i })).toBeNull(); + }); + it("calls apiImportGitHubIssue and onImport when Import is clicked", async () => { @@ -983,7 +1029,7 @@ describe("GitHubImportModal", () => { // Wait for initial auto-load without labels await waitFor(() => { - expect(apiFetchGitHubIssues).toHaveBeenCalledWith("dustinbyrne", "kb", 30, undefined); + expect(apiFetchGitHubIssues).toHaveBeenCalledWith("dustinbyrne", "kb", 300, undefined); expect(screen.getByText("Issue without labels")).toBeTruthy(); }); @@ -998,7 +1044,7 @@ describe("GitHubImportModal", () => { // Verify re-fetch with labels await waitFor(() => { - expect(apiFetchGitHubIssues).toHaveBeenLastCalledWith("dustinbyrne", "kb", 30, ["bug"]); + expect(apiFetchGitHubIssues).toHaveBeenLastCalledWith("dustinbyrne", "kb", 300, ["bug"]); expect(screen.getByText("Bug issue")).toBeTruthy(); }); }); @@ -1845,7 +1891,7 @@ describe("GitHubImportModal", () => { // No persisted state exists for this project: the single detected remote is still auto-selected and its issues load. await waitFor(() => { expect(screen.getByTestId("github-import-single-remote")).toBeTruthy(); - expect(apiFetchGitHubIssues).toHaveBeenCalledWith("dustinbyrne", "kb", 30, undefined); + expect(apiFetchGitHubIssues).toHaveBeenCalledWith("dustinbyrne", "kb", 300, undefined); expect(screen.getByText("Fresh Issue")).toBeTruthy(); }); }); diff --git a/packages/dashboard/src/__tests__/github-tracking-periodic-reconcile-sweep.test.ts b/packages/dashboard/src/__tests__/github-tracking-periodic-reconcile-sweep.test.ts index 02da030d7d..85b05bdb0a 100644 --- a/packages/dashboard/src/__tests__/github-tracking-periodic-reconcile-sweep.test.ts +++ b/packages/dashboard/src/__tests__/github-tracking-periodic-reconcile-sweep.test.ts @@ -3,19 +3,18 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { TaskStore } from "@fusion/core"; import { registerGitGitHubRoutes, GITHUB_TRACKING_RECONCILE_INTERVAL_MS } from "../routes/register-git-github.js"; +import { GitHubTrackingReconciler } from "../github-tracking-reconciler.js"; -const reconcile = vi.fn().mockResolvedValue({ scanned: 0, closed: 0, skipped: 0, errors: 0 }); -const reconcileDeletedAndArchived = vi.fn(); -const reconcileSourceIssues = vi.fn().mockResolvedValue({ scanned: 0, closed: 0, skipped: 0, errors: 0 }); - -vi.mock("../github-tracking-reconciler.js", () => ({ - RECONCILE_SCAN_LIMIT: 200, - GitHubTrackingReconciler: vi.fn().mockImplementation(function () { return { - reconcile, - reconcileDeletedAndArchived, - reconcileSourceIssues, - }; }), -})); +/* +FNXC:GithubTrackingReconcile 2026-07-16-15:40: +Spy the three individual reconcile passes on the prototype but keep the REAL runSweep, so this test +exercises production's actual pass-isolation + offset-paging orchestration (not a re-implemented mock). +This is what proves the sweep still pages reconcileDeletedAndArchived by offset after runSweep took +ownership of that logic. +*/ +let reconcile: ReturnType; +let reconcileDeletedAndArchived: ReturnType; +let reconcileSourceIssues: ReturnType; vi.mock("../github-issue-comment.js", () => ({ GitHubIssueCommentService: vi.fn().mockImplementation(function () { return { start: vi.fn(), stop: vi.fn() }; }), @@ -49,10 +48,15 @@ describe("GitHub tracking periodic reconcile sweep", () => { beforeEach(() => { vi.useFakeTimers(); vi.clearAllMocks(); + // Keep the real runSweep; stub only the three passes so we can assert paged offsets. + reconcile = vi.spyOn(GitHubTrackingReconciler.prototype, "reconcile").mockResolvedValue({ scanned: 0, closed: 0, skipped: 0, errors: 0 }); + reconcileDeletedAndArchived = vi.spyOn(GitHubTrackingReconciler.prototype, "reconcileDeletedAndArchived"); + reconcileSourceIssues = vi.spyOn(GitHubTrackingReconciler.prototype, "reconcileSourceIssues").mockResolvedValue({ scanned: 0, closed: 0, skipped: 0, errors: 0 }); }); afterEach(() => { vi.useRealTimers(); + vi.restoreAllMocks(); }); it("runs startup and periodic sweeps with paged offsets and clears interval on dispose", async () => { @@ -74,6 +78,9 @@ describe("GitHub tracking periodic reconcile sweep", () => { await vi.advanceTimersByTimeAsync(0); expect(reconcileDeletedAndArchived).toHaveBeenNthCalledWith(1, store, { offset: 0, limit: 200 }); + // All three passes run per sweep (regression: a throwing pass must not starve the others). + expect(reconcile).toHaveBeenCalledTimes(1); + expect(reconcileSourceIssues).toHaveBeenCalledTimes(1); await vi.advanceTimersByTimeAsync(GITHUB_TRACKING_RECONCILE_INTERVAL_MS); expect(reconcileDeletedAndArchived).toHaveBeenNthCalledWith(2, store, { offset: 200, limit: 200 }); diff --git a/packages/dashboard/src/__tests__/github-tracking-reconciler.test.ts b/packages/dashboard/src/__tests__/github-tracking-reconciler.test.ts index 14bcd80da2..25f3dd972d 100644 --- a/packages/dashboard/src/__tests__/github-tracking-reconciler.test.ts +++ b/packages/dashboard/src/__tests__/github-tracking-reconciler.test.ts @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { TaskStore } from "@fusion/core"; -import { GitHubTrackingReconciler, RECONCILE_CONCURRENCY_LIMIT } from "../github-tracking-reconciler.js"; +import { GitHubTrackingReconciler, RECONCILE_CONCURRENCY_LIMIT, RECONCILE_SCAN_LIMIT } from "../github-tracking-reconciler.js"; const { mockGetIssue, mockSetIssueState } = vi.hoisted(() => ({ mockGetIssue: vi.fn(), @@ -282,4 +282,63 @@ describe("GitHubTrackingReconciler", () => { expect(result.skipped).toBe(1); }); }); + + /* + FNXC:GithubTrackingReconcile 2026-07-16-15:40: + Regression coverage for the reconcile backstop going fully dark. The bug: a throw in the + deleted/archived pass aborted the whole sweep (shared try/catch, silent swallow), so the + done-task tracking pass — which closes linked GitHub issues on Done — never ran on any sweep, + and imported/linked issues stayed open forever. Invariant asserted below: each pass is isolated, + so a throw in ANY one pass never prevents the others from running. + */ + describe("runSweep pass isolation", () => { + let warnSpy: ReturnType; + beforeEach(() => { + warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + }); + afterEach(() => { + warnSpy.mockRestore(); + }); + + it("runs done-task + source-issue passes even when the deleted/archived pass throws", async () => { + const reconciler = new GitHubTrackingReconciler(); + const deletedArchived = vi + .spyOn(reconciler, "reconcileDeletedAndArchived") + .mockRejectedValue(new Error("listTasksForGithubTrackingReconcile exploded")); + const reconcile = vi + .spyOn(reconciler, "reconcile") + .mockResolvedValue({ scanned: 1, closed: 1, skipped: 0, errors: 0 }); + const reconcileSource = vi + .spyOn(reconciler, "reconcileSourceIssues") + .mockResolvedValue({ scanned: 0, closed: 0, skipped: 0, errors: 0 }); + + const store = createStore({}); + const { nextOffset } = await reconciler.runSweep(store, { offset: 0 }); + + // The critical invariant: the two closing passes still ran despite pass 1 throwing. + expect(deletedArchived).toHaveBeenCalledTimes(1); + expect(reconcile).toHaveBeenCalledTimes(1); + expect(reconcileSource).toHaveBeenCalledTimes(1); + // A failed deleted/archived pass resets the paging offset (retry from 0 next sweep). + expect(nextOffset).toBe(0); + expect(warnSpy).toHaveBeenCalled(); + }); + + it("runs the source-issue pass even when the done-task pass throws, and advances paging", async () => { + const reconciler = new GitHubTrackingReconciler(); + vi.spyOn(reconciler, "reconcileDeletedAndArchived").mockResolvedValue({ scanned: 0, closed: 0, skipped: 0, errors: 0, hasMore: true }); + const reconcile = vi.spyOn(reconciler, "reconcile").mockRejectedValue(new Error("done-task pass boom")); + const reconcileSource = vi + .spyOn(reconciler, "reconcileSourceIssues") + .mockResolvedValue({ scanned: 0, closed: 0, skipped: 0, errors: 0 }); + + const store = createStore({}); + const { nextOffset } = await reconciler.runSweep(store, { offset: 200 }); + + expect(reconcile).toHaveBeenCalledTimes(1); + expect(reconcileSource).toHaveBeenCalledTimes(1); + // deleted/archived reported hasMore, so paging advances by the scan limit. + expect(nextOffset).toBe(200 + RECONCILE_SCAN_LIMIT); + }); + }); }); diff --git a/packages/dashboard/src/__tests__/github.test.ts b/packages/dashboard/src/__tests__/github.test.ts index a281dbeaeb..830e008dd8 100644 --- a/packages/dashboard/src/__tests__/github.test.ts +++ b/packages/dashboard/src/__tests__/github.test.ts @@ -1177,6 +1177,72 @@ describe("GitHubClient", () => { vi.restoreAllMocks(); }); + + /* + FNXC:GitHubImport 2026-07-16-16:20: + Regression: the REST path used to fetch a single page (per_page capped at 100), so repos with >100 open + issues silently lost everything past the first page. It now pages `page` until the limit or a short page. + */ + it("pages the REST API across multiple pages up to the requested limit", async () => { + mockRunGhJsonAsync.mockRejectedValue(new Error("gh failed")); + const clientWithToken = new GitHubClient("ghp_token"); + + const makeIssue = (n: number) => ({ + number: n, title: `Issue ${n}`, body: null, + html_url: `https://github.com/owner/repo/issues/${n}`, + labels: [], state: "open", updated_at: "2026-01-01T00:00:00Z", + }); + const page1 = Array.from({ length: 100 }, (_, i) => makeIssue(i + 1)); + const page2 = Array.from({ length: 50 }, (_, i) => makeIssue(101 + i)); + + const mockFetch = vi.fn().mockImplementation((url: string) => { + const page = new URL(url).searchParams.get("page"); + return Promise.resolve({ ok: true, json: () => Promise.resolve(page === "1" ? page1 : page2) }); + }); + global.fetch = mockFetch as any; + + const result = await clientWithToken.listIssues("owner", "repo", { limit: 150 }); + + expect(mockFetch).toHaveBeenCalledTimes(2); + expect(new URL(mockFetch.mock.calls[0][0]).searchParams.get("page")).toBe("1"); + expect(new URL(mockFetch.mock.calls[1][0]).searchParams.get("page")).toBe("2"); + expect(result).toHaveLength(150); + expect(result[0].number).toBe(1); + expect(result[149].number).toBe(150); + + vi.restoreAllMocks(); + }); + + it("keeps paging when a full page is entirely pull requests (does not stop early)", async () => { + mockRunGhJsonAsync.mockRejectedValue(new Error("gh failed")); + const clientWithToken = new GitHubClient("ghp_token"); + + // Page 1 is 100 PRs (all filtered out) — a naive "stop when this page yields no issues" would return []. + const prPage = Array.from({ length: 100 }, (_, i) => ({ + number: i + 1, title: `PR ${i + 1}`, body: null, + html_url: `https://github.com/owner/repo/issues/${i + 1}`, + labels: [], state: "open", updated_at: "2026-01-01T00:00:00Z", pull_request: {}, + })); + const issuePage = Array.from({ length: 30 }, (_, i) => ({ + number: 200 + i, title: `Issue ${200 + i}`, body: null, + html_url: `https://github.com/owner/repo/issues/${200 + i}`, + labels: [], state: "open", updated_at: "2026-01-01T00:00:00Z", + })); + + const mockFetch = vi.fn().mockImplementation((url: string) => { + const page = new URL(url).searchParams.get("page"); + return Promise.resolve({ ok: true, json: () => Promise.resolve(page === "1" ? prPage : issuePage) }); + }); + global.fetch = mockFetch as any; + + const result = await clientWithToken.listIssues("owner", "repo", { limit: 150 }); + + expect(mockFetch).toHaveBeenCalledTimes(2); + expect(result).toHaveLength(30); + expect(result.every((r) => r.number >= 200)).toBe(true); + + vi.restoreAllMocks(); + }); }); describe("getIssue", () => { diff --git a/packages/dashboard/src/github-tracking-reconciler.ts b/packages/dashboard/src/github-tracking-reconciler.ts index d365a4672f..c0f5e68677 100644 --- a/packages/dashboard/src/github-tracking-reconciler.ts +++ b/packages/dashboard/src/github-tracking-reconciler.ts @@ -6,6 +6,44 @@ const RECONCILE_SCAN_LIMIT = 200; const RECONCILE_CONCURRENCY_LIMIT = 4; export class GitHubTrackingReconciler { + /* + FNXC:GithubTrackingReconcile 2026-07-16-15:40: + The three reconcile passes are INDEPENDENT and each MUST run even when another throws. + Regression that motivated this: the caller ran all three inside one try/catch with a silent + swallow, and the fragile PG-backend `reconcileDeletedAndArchived` pass ran first. When it threw + (e.g. an async-layer/row-hydration failure), the done-task `reconcile()` and source-issue + `reconcileSourceIssues()` passes never executed — on every sweep, startup and periodic. Net effect: + the reconcile safety-net closed ZERO GitHub issues while only the live move-handler worked, so any + task the live path missed (moved to Done before tracking adoption was reflected in the move event, + or a transient close failure like FN-8066's) kept its linked issue OPEN indefinitely. + runSweep isolates each pass and surfaces failures via console.warn instead of hiding them, so one + broken pass can never starve the others and a future breakage is observable rather than silent. + */ + async runSweep(store: TaskStore, options: { offset: number }): Promise<{ nextOffset: number }> { + let nextOffset = 0; + await this.runPass("deleted/archived", async () => { + const result = await this.reconcileDeletedAndArchived(store, { + offset: options.offset, + limit: RECONCILE_SCAN_LIMIT, + }); + nextOffset = result.hasMore ? options.offset + RECONCILE_SCAN_LIMIT : 0; + }); + // Done-task tracking + source-issue passes run regardless of the deleted/archived pass outcome. + await this.runPass("done-task tracking", () => this.reconcile(store)); + await this.runPass("source-issue", () => this.reconcileSourceIssues(store)); + return { nextOffset }; + } + + private async runPass(label: string, fn: () => Promise): Promise { + try { + await fn(); + } catch (err) { + console.warn( + `[github-tracking-reconcile] ${label} pass failed (other passes still run): ${err instanceof Error ? err.message : String(err)}`, + ); + } + } + 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 : []) diff --git a/packages/dashboard/src/github.ts b/packages/dashboard/src/github.ts index ae08a0e0b1..1078560891 100644 --- a/packages/dashboard/src/github.ts +++ b/packages/dashboard/src/github.ts @@ -241,6 +241,16 @@ export interface PrComment { const PR_REVIEW_PAGE_SIZE = 100; const MAX_PR_REVIEW_PAGES = 10; +/* +FNXC:GitHubImport 2026-07-16-16:20: +Upper bound on issues returned by listIssues for the import picker. The picker pages this set client-side, +so this cap bounds one fetch: gh's `--limit` paginates internally to reach it, and the REST path loops +`page` at ISSUE_LIST_PAGE_SIZE (100, GitHub's per_page max) until the cap or exhaustion. Keeps a huge repo +from returning an unbounded body while still surfacing far more than the old single 30/100-issue page. +*/ +const MAX_LIST_ISSUES = 300; +const ISSUE_LIST_PAGE_SIZE = 100; + export type ReviewDecision = "APPROVED" | "CHANGES_REQUESTED" | "REVIEW_REQUIRED" | null; export type PrCheckState = | "success" @@ -3204,10 +3214,19 @@ export class GitHubClient { updatedAt?: string; author?: string | null; }>> { - const limit = options?.limit ?? 30; + const limit = Math.min(options?.limit ?? 30, MAX_LIST_ISSUES); const state = options?.state ?? "open"; - // gh issue list doesn't support label filtering directly, so we fetch and filter client-side + /* + FNXC:GitHubImport 2026-07-16-16:20: + Label filtering is client-side (OR across labels, matching the historical `.some()` semantics that `gh --label`'s AND cannot express). + Because filtering happens AFTER the fetch, the fetch must pull the full cap when labels are set — otherwise `gh` returns the first `limit` UNFILTERED issues and the post-filter `.slice(0, limit)` starves, hiding labeled issues that sort past the first `limit` rows. + Without labels there is nothing to filter, so fetch exactly `limit`. `gh --limit` paginates internally past 100 to reach the requested count. + */ + const hasLabelFilter = Boolean(options?.labels && options.labels.length > 0); + const fetchCount = hasLabelFilter ? MAX_LIST_ISSUES : limit; + + // gh issue list doesn't support OR label filtering directly, so we fetch and filter client-side const issues = await runGhJsonAsync> { - const limit = options?.limit ?? 30; + const limit = Math.min(options?.limit ?? 30, MAX_LIST_ISSUES); const state = options?.state ?? "open"; - - const params = new URLSearchParams(); - params.append("state", state); - params.append("per_page", String(Math.min(limit, 100))); - if (options?.labels && options.labels.length > 0) { - params.append("labels", options.labels.join(",")); - } - - const url = `${this.baseUrl}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/issues?${params}`; const headers = this.buildHeaders(); - const response = await fetch(url, { headers }); - - if (!response.ok) { - if (response.status === 404) { - throw new Error(`Repository not found: ${owner}/${repo}`); - } - throw new Error(`GitHub API error: ${response.status} ${response.statusText}`); - } - - const data = (await response.json()) as Array<{ + /* + FNXC:GitHubImport 2026-07-16-16:20: + REST `/issues` caps per_page at 100, so loop `page` until we collect `limit` real issues or a short page + signals exhaustion. Pull requests share the `/issues` feed and are dropped here, which can shrink a page + below per_page — so keep paging on a full 100-item page even after PR filtering, and stop only on a genuinely + short page. Bounded by MAX_LIST_ISSUES pages-worth so a huge repo can't loop unbounded. + */ + const perPage = Math.min(limit, ISSUE_LIST_PAGE_SIZE); + const collected: Array<{ number: number; title: string; body: string | null; html_url: string; labels: Array<{ name: string }>; - state: string; - updated_at: string; - user?: { login?: string } | null; - pull_request?: unknown; - }>; + state?: "open" | "closed"; + updatedAt?: string; + author?: string | null; + }> = []; - // Filter out pull requests (they have a pull_request property) - return data - .filter((issue) => !issue.pull_request) - .map((issue) => ({ - number: issue.number, - title: issue.title, - body: issue.body, - html_url: issue.html_url, - labels: issue.labels, - state: this.mapIssueState(issue.state), - updatedAt: issue.updated_at, - author: issue.user?.login ?? null, - })) - .slice(0, limit); + for (let page = 1; collected.length < limit; page += 1) { + const params = new URLSearchParams(); + params.append("state", state); + params.append("per_page", String(perPage)); + params.append("page", String(page)); + if (options?.labels && options.labels.length > 0) { + params.append("labels", options.labels.join(",")); + } + + const url = `${this.baseUrl}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/issues?${params}`; + const response = await fetch(url, { headers }); + + if (!response.ok) { + if (response.status === 404) { + throw new Error(`Repository not found: ${owner}/${repo}`); + } + throw new Error(`GitHub API error: ${response.status} ${response.statusText}`); + } + + const data = (await response.json()) as Array<{ + number: number; + title: string; + body: string | null; + html_url: string; + labels: Array<{ name: string }>; + state: string; + updated_at: string; + user?: { login?: string } | null; + pull_request?: unknown; + }>; + + for (const issue of data) { + if (issue.pull_request) continue; // PRs share the /issues feed; exclude them + collected.push({ + number: issue.number, + title: issue.title, + body: issue.body, + html_url: issue.html_url, + labels: issue.labels, + state: this.mapIssueState(issue.state), + updatedAt: issue.updated_at, + author: issue.user?.login ?? null, + }); + } + + // A page shorter than per_page means GitHub has no further issues to return. + if (data.length < perPage) break; + } + + return collected.slice(0, limit); } async searchIssues( diff --git a/packages/dashboard/src/routes/register-git-github.ts b/packages/dashboard/src/routes/register-git-github.ts index f1a0f8dc73..7e07d43829 100644 --- a/packages/dashboard/src/routes/register-git-github.ts +++ b/packages/dashboard/src/routes/register-git-github.ts @@ -2624,22 +2624,22 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { reconcileSweepInFlightByStore.set(projectStore, true); try { + /* + FNXC:GithubTrackingReconcile 2026-07-16-15:40: + Delegate to reconciler.runSweep so the three reconcile passes are isolated — a throw in the + deleted/archived pass must not starve the done-task tracking + source-issue passes (the ones + that actually close linked issues on Done). Previously they shared this try/catch and the + silent swallow below meant a single early throw disabled the entire reconcile backstop, every + sweep, with no diagnostic. + */ const offset = options?.startup ? 0 : reconcileSweepOffsetByStore.get(projectStore) ?? 0; - const deletedArchivedResult = await githubTrackingReconciler.reconcileDeletedAndArchived(projectStore, { - offset, - limit: RECONCILE_SCAN_LIMIT, - }); - - if (deletedArchivedResult.hasMore) { - reconcileSweepOffsetByStore.set(projectStore, offset + RECONCILE_SCAN_LIMIT); - } else { - reconcileSweepOffsetByStore.set(projectStore, 0); - } - - await githubTrackingReconciler.reconcile(projectStore); - await githubTrackingReconciler.reconcileSourceIssues(projectStore); - } catch { - // best-effort sweep + const { nextOffset } = await githubTrackingReconciler.runSweep(projectStore, { offset }); + reconcileSweepOffsetByStore.set(projectStore, nextOffset); + } catch (err) { + // runSweep isolates per-pass failures internally; this guards only unexpected orchestration errors. + console.warn( + `[github-tracking-reconcile] sweep orchestration error: ${err instanceof Error ? err.message : String(err)}`, + ); } finally { reconcileSweepInFlightByStore.set(projectStore, false); }