feat(dashboard): page GitHub issue import + fix close-on-Done backstop
Import picker now surfaces all open issues instead of a silent first-30 slice, and reliably closes linked/imported issues when their task reaches Done. Import: - GitHubImportModal fetches up to 300 open issues in one request and pages the result client-side at 30/page with Previous/Next controls, a page indicator, and a truncation notice past the cap. - NewTaskModal reference picker limit 30 -> 100. - GitHubClient.listIssues 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. Close-on-Done: - GitHubTrackingReconciler.runSweep isolates its three reconcile passes so a throw in one (e.g. the PG-backend deleted/archived pass) no longer starves the done-task + source-issue passes. Previously all three shared one try/catch with a silent swallow, so a single early throw disabled the whole close-on-Done reconcile backstop on every sweep, leaving linked issues open. Failures are now logged instead of hidden. Tests: REST multi-page paging, PR-filter-doesn't-stop-paging, Prev/Next UI, one-page (no controls), and per-pass isolation invariants. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
7
.changeset/github-import-and-issue-close-fixes.md
Normal file
7
.changeset/github-import-and-issue-close-fixes.md
Normal file
@@ -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.
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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<GitHubIssue[]>([]);
|
||||
const [selectedIssueNumber, setSelectedIssueNumber] = useState<number | null>(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<GitHubPull[]>([]);
|
||||
@@ -478,6 +492,11 @@ export function GitHubImportModal({ isOpen, onClose, onImport, tasks, projectId,
|
||||
*/
|
||||
const [optimisticImportedUrls, setOptimisticImportedUrls] = useState<Set<string>>(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<string>();
|
||||
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 && (
|
||||
<div className="issues-list" aria-live="polite">
|
||||
{visibleIssues.map((issue) => {
|
||||
{pagedIssues.map((issue) => {
|
||||
const isImported = isUrlImported(issue.html_url);
|
||||
return (
|
||||
<div
|
||||
@@ -1580,6 +1616,44 @@ export function GitHubImportModal({ isOpen, onClose, onImport, tasks, projectId,
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/*
|
||||
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 && (
|
||||
<div className="github-import-pagination" role="navigation" aria-label={t("git.issuesPaginationAria", "Issues pages")}>
|
||||
<button
|
||||
type="button"
|
||||
className="btn btn-secondary btn-sm"
|
||||
onClick={() => setIssuePage((p) => Math.max(0, p - 1))}
|
||||
disabled={clampedIssuePage <= 0}
|
||||
>
|
||||
{t("git.prevPage", "Previous")}
|
||||
</button>
|
||||
<span className="github-import-pagination__status" aria-live="polite">
|
||||
{t("git.pageStatus", "Page {{page}} of {{total}}", { page: clampedIssuePage + 1, total: issuePageCount })}
|
||||
{" · "}
|
||||
{t("git.issuesCount", "{{count}} issues", { count: visibleIssues.length })}
|
||||
</span>
|
||||
<button
|
||||
type="button"
|
||||
className="btn btn-secondary btn-sm"
|
||||
onClick={() => setIssuePage((p) => Math.min(issuePageCount - 1, p + 1))}
|
||||
disabled={clampedIssuePage >= issuePageCount - 1}
|
||||
>
|
||||
{t("git.nextPage", "Next")}
|
||||
</button>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{activeTab === "issues" && isIssuesTruncated && (
|
||||
<div className="github-import-pagination__truncation" role="status">
|
||||
{t("git.issuesTruncated", "Showing the first {{cap}} open issues. Refine with a label filter to narrow the list.", { cap: ISSUES_FETCH_CAP })}
|
||||
</div>
|
||||
)}
|
||||
|
||||
{activeTab === "pulls" && allPullsHidden && (
|
||||
<div className="github-import-state github-import-state--empty" data-testid="github-import-all-imported-empty" role="status">
|
||||
<div><strong>{t("git.allImportedHidden", "All loaded items are already imported")}</strong></div>
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -705,7 +705,7 @@ describe("GitHubImportModal", () => {
|
||||
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
|
||||
|
||||
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(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
|
||||
|
||||
// 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(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<typeof vi.spyOn>;
|
||||
let reconcileDeletedAndArchived: ReturnType<typeof vi.spyOn>;
|
||||
let reconcileSourceIssues: ReturnType<typeof vi.spyOn>;
|
||||
|
||||
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 });
|
||||
|
||||
@@ -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<typeof vi.spyOn>;
|
||||
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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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<unknown>): Promise<void> {
|
||||
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 : [])
|
||||
|
||||
@@ -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<Array<{
|
||||
number: number;
|
||||
title: string;
|
||||
@@ -3221,7 +3240,7 @@ export class GitHubClient {
|
||||
"issue", "list",
|
||||
"--repo", `${owner}/${repo}`,
|
||||
"--state", state,
|
||||
"--limit", String(Math.min(limit, 100)),
|
||||
"--limit", String(fetchCount),
|
||||
// FNXC:GitHubImport 2026-06-22-18:30: Request `author` so the import preview pane can show full issue metadata (author/state alongside the already-present full body) without a per-item detail fetch.
|
||||
"--json", "number,title,body,url,labels,state,updatedAt,author",
|
||||
]);
|
||||
@@ -3263,54 +3282,79 @@ 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";
|
||||
|
||||
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(
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user