root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(KB-067): migrate GitHub issue routes from fetch to gh CLI

Browse Source 
- Replace fetch-based GitHub API calls with gh CLI in issues/fetch and issues/import routes
- Update tests to mock gh CLI instead of fetch with proper error handling
- Reduce code complexity by leveraging gh CLI's built-in JSON output
- Add changeset documenting the refactoring work
This commit is contained in:
gsxdsm
2026-03-30 11:19:02 -07:00
parent 904a95f7c0
commit 753bec19df
3 changed files with 131 additions and 172 deletions
Download Patch File Download Diff File Expand all files Collapse all files

144
packages/dashboard/src/routes.test.ts
View File

@@ -8,6 +8,19 @@ import type { TaskDetail } from "@kb/core";
import type { AuthStorageLike, ModelRegistryLike } from "./routes.js";
import { __resetPlanningState } from "./planning.js";
// Mock @kb/core for gh CLI auth checks
vi.mock("@kb/core", async () => {
const actual = await vi.importActual<typeof import("@kb/core")>("@kb/core");
return {
...actual,
isGhAuthenticated: vi.fn(),
};
});
import { isGhAuthenticated } from "@kb/core";
const mockIsGhAuthenticated = vi.mocked(isGhAuthenticated);
function createMockStore(overrides: Partial<TaskStore> = {}): TaskStore {
return {
getTask: vi.fn(),
@@ -1809,17 +1822,16 @@ describe("Pause/Unpause endpoints", () => {
describe("POST /github/issues/fetch", () => {
let store: TaskStore;
let fetchSpy: ReturnType<typeof vi.fn>;
const originalFetch = globalThis.fetch;
let listIssuesSpy: ReturnType<typeof vi.fn>;
beforeEach(() => {
store = createMockStore();
fetchSpy = vi.fn();
globalThis.fetch = fetchSpy as any;
mockIsGhAuthenticated.mockReturnValue(true);
listIssuesSpy = vi.fn();
vi.spyOn(GitHubClient.prototype, "listIssues").mockImplementation(listIssuesSpy);
});
afterEach(() => {
globalThis.fetch = originalFetch;
vi.restoreAllMocks();
});
@@ -1839,11 +1851,7 @@ describe("POST /github/issues/fetch", () => {
};
it("fetches issues successfully", async () => {
fetchSpy.mockResolvedValueOnce({
ok: true,
status: 200,
json: () => Promise.resolve([mockGitHubIssue]),
} as Response);
listIssuesSpy.mockResolvedValueOnce([mockGitHubIssue]);
const res = await REQUEST(buildApp(), "POST", "/api/github/issues/fetch", JSON.stringify({ owner: "owner", repo: "repo" }), {
"Content-Type": "application/json",
@@ -1874,11 +1882,7 @@ describe("POST /github/issues/fetch", () => {
});
it("returns 404 when repository not found", async () => {
fetchSpy.mockResolvedValueOnce({
ok: false,
status: 404,
statusText: "Not Found",
} as Response);
listIssuesSpy.mockRejectedValueOnce(new Error("Repository not found: owner/repo"));
const res = await REQUEST(buildApp(), "POST", "/api/github/issues/fetch", JSON.stringify({ owner: "owner", repo: "repo" }), {
"Content-Type": "application/json",
@@ -1888,33 +1892,37 @@ describe("POST /github/issues/fetch", () => {
expect(res.body.error).toContain("Repository not found");
});
it("returns 401/403 when authentication fails", async () => {
fetchSpy.mockResolvedValueOnce({
ok: false,
status: 403,
statusText: "Forbidden",
} as Response);
it("returns 401 when gh not authenticated", async () => {
mockIsGhAuthenticated.mockReturnValueOnce(false);
const res = await REQUEST(buildApp(), "POST", "/api/github/issues/fetch", JSON.stringify({ owner: "owner", repo: "repo" }), {
"Content-Type": "application/json",
});
expect(res.status).toBe(401);
expect(res.body.error).toContain("Authentication failed");
expect(res.body.error).toContain("Not authenticated with GitHub");
expect(res.body.error).toContain("gh auth login");
});
it("filters out pull requests", async () => {
const pr = { ...mockGitHubIssue, pull_request: {} };
fetchSpy.mockResolvedValueOnce({
ok: true,
status: 200,
json: () => Promise.resolve([mockGitHubIssue, pr]),
} as Response);
it("returns 502 when gh CLI fails", async () => {
listIssuesSpy.mockRejectedValueOnce(new Error("Some gh CLI error"));
const res = await REQUEST(buildApp(), "POST", "/api/github/issues/fetch", JSON.stringify({ owner: "owner", repo: "repo" }), {
"Content-Type": "application/json",
});
expect(res.status).toBe(502);
expect(res.body.error).toContain("GitHub CLI error");
});
it("filters out pull requests (gh CLI already filters them)", async () => {
// gh issue list already filters out PRs, so we just verify the response
listIssuesSpy.mockResolvedValueOnce([mockGitHubIssue]);
const res = await REQUEST(buildApp(), "POST", "/api/github/issues/fetch", JSON.stringify({ owner: "owner", repo: "repo", limit: 10 }), {
"Content-Type": "application/json",
});
expect(res.status).toBe(200);
expect(res.body).toHaveLength(1);
expect(res.body[0].number).toBe(1);
@@ -1922,11 +1930,7 @@ describe("POST /github/issues/fetch", () => {
it("respects limit parameter", async () => {
const manyIssues = Array.from({ length: 50 }, (_, i) => ({ ...mockGitHubIssue, number: i + 1 }));
fetchSpy.mockResolvedValueOnce({
ok: true,
status: 200,
json: () => Promise.resolve(manyIssues),
} as Response);
listIssuesSpy.mockResolvedValueOnce(manyIssues.slice(0, 10));
const res = await REQUEST(buildApp(), "POST", "/api/github/issues/fetch", JSON.stringify({ owner: "owner", repo: "repo", limit: 10 }), {
"Content-Type": "application/json",
@@ -1939,12 +1943,12 @@ describe("POST /github/issues/fetch", () => {
describe("POST /github/issues/import", () => {
let store: TaskStore;
let fetchSpy: ReturnType<typeof vi.fn>;
const originalFetch = globalThis.fetch;
let getIssueSpy: ReturnType<typeof vi.fn>;
beforeEach(() => {
fetchSpy = vi.fn();
globalThis.fetch = fetchSpy as any;
mockIsGhAuthenticated.mockReturnValue(true);
getIssueSpy = vi.fn();
vi.spyOn(GitHubClient.prototype, "getIssue").mockImplementation(getIssueSpy);
store = createMockStore({
createTask: vi.fn().mockResolvedValue({
@@ -1958,7 +1962,6 @@ describe("POST /github/issues/import", () => {
});
afterEach(() => {
globalThis.fetch = originalFetch;
vi.restoreAllMocks();
});
@@ -1974,15 +1977,11 @@ describe("POST /github/issues/import", () => {
title: "Test Issue",
body: "Test body",
html_url: "https://github.com/owner/repo/issues/1",
labels: [{ name: "bug" }],
state: "open",
};
it("imports a single issue successfully", async () => {
fetchSpy.mockResolvedValueOnce({
ok: true,
status: 200,
json: () => Promise.resolve(mockGitHubIssue),
} as Response);
getIssueSpy.mockResolvedValueOnce(mockGitHubIssue);
const res = await REQUEST(buildApp(), "POST", "/api/github/issues/import", JSON.stringify({ owner: "owner", repo: "repo", issueNumber: 1 }), {
"Content-Type": "application/json",
@@ -1999,11 +1998,7 @@ describe("POST /github/issues/import", () => {
});
it("logs the import action", async () => {
fetchSpy.mockResolvedValueOnce({
ok: true,
status: 200,
json: () => Promise.resolve(mockGitHubIssue),
} as Response);
getIssueSpy.mockResolvedValueOnce(mockGitHubIssue);
await REQUEST(buildApp(), "POST", "/api/github/issues/import", JSON.stringify({ owner: "owner", repo: "repo", issueNumber: 1 }), {
"Content-Type": "application/json",
@@ -2021,34 +2016,39 @@ describe("POST /github/issues/import", () => {
expect(res.body.error).toContain("issueNumber is required");
});
it("returns 404 when issue not found", async () => {
fetchSpy.mockResolvedValueOnce({
ok: false,
status: 404,
statusText: "Not Found",
} as Response);
it("returns 400 when issue not found or is a pull request", async () => {
// getIssue returns null for both "not found" and "PR" cases
getIssueSpy.mockResolvedValueOnce(null);
const res = await REQUEST(buildApp(), "POST", "/api/github/issues/import", JSON.stringify({ owner: "owner", repo: "repo", issueNumber: 999 }), {
"Content-Type": "application/json",
});
expect(res.status).toBe(404);
expect(res.body.error).toContain("not found");
expect(res.status).toBe(400);
expect(res.body.error).toContain("is a pull request");
});
it("returns 400 when importing a pull request", async () => {
fetchSpy.mockResolvedValueOnce({
ok: true,
status: 200,
json: () => Promise.resolve({ ...mockGitHubIssue, pull_request: {} }),
} as Response);
it("returns 401 when gh not authenticated", async () => {
mockIsGhAuthenticated.mockReturnValueOnce(false);
const res = await REQUEST(buildApp(), "POST", "/api/github/issues/import", JSON.stringify({ owner: "owner", repo: "repo", issueNumber: 1 }), {
"Content-Type": "application/json",
});
expect(res.status).toBe(400);
expect(res.body.error).toContain("pull request");
expect(res.status).toBe(401);
expect(res.body.error).toContain("Not authenticated with GitHub");
expect(res.body.error).toContain("gh auth login");
});
it("returns 502 when gh CLI fails", async () => {
getIssueSpy.mockRejectedValueOnce(new Error("Some gh CLI error"));
const res = await REQUEST(buildApp(), "POST", "/api/github/issues/import", JSON.stringify({ owner: "owner", repo: "repo", issueNumber: 1 }), {
"Content-Type": "application/json",
});
expect(res.status).toBe(502);
expect(res.body.error).toContain("GitHub CLI error");
});
it("returns 409 when issue already imported", async () => {
@@ -2060,11 +2060,7 @@ describe("POST /github/issues/import", () => {
},
]);
fetchSpy.mockResolvedValueOnce({
ok: true,
status: 200,
json: () => Promise.resolve(mockGitHubIssue),
} as Response);
getIssueSpy.mockResolvedValueOnce(mockGitHubIssue);
const res = await REQUEST(buildApp(), "POST", "/api/github/issues/import", JSON.stringify({ owner: "owner", repo: "repo", issueNumber: 1 }), {
"Content-Type": "application/json",
@@ -2081,11 +2077,7 @@ describe("POST /github/issues/import", () => {
...mockGitHubIssue,
title: "A".repeat(250),
};
fetchSpy.mockResolvedValueOnce({
ok: true,
status: 200,
json: () => Promise.resolve(longTitleIssue),
} as Response);
getIssueSpy.mockResolvedValueOnce(longTitleIssue);
await REQUEST(buildApp(), "POST", "/api/github/issues/import", JSON.stringify({ owner: "owner", repo: "repo", issueNumber: 1 }), {
"Content-Type": "application/json",

154
packages/dashboard/src/routes.ts
View File

@@ -3,7 +3,7 @@ import multer from "multer";
import { createReadStream } from "node:fs";
import { execSync } from "node:child_process";
import type { TaskStore, Column, MergeResult } from "@kb/core";
import { COLUMNS, VALID_TRANSITIONS, type PrInfo } from "@kb/core";
import { COLUMNS, VALID_TRANSITIONS, type PrInfo, isGhAuthenticated } from "@kb/core";
import type { ServerOptions } from "./server.js";
import { GitHubClient, getCurrentGitHubRepo } from "./github.js";
import { githubPoller, githubRateLimiter } from "./github-poll.js";
@@ -1316,66 +1316,35 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
return;
}
const token = process.env.GITHUB_TOKEN;
// Build query parameters - only open issues, no PRs
const params = new URLSearchParams();
params.append("state", "open");
params.append("per_page", String(Math.min(limit, 100)));
if (labels && labels.length > 0) {
params.append("labels", labels.join(","));
// Check gh authentication
if (!isGhAuthenticated()) {
res.status(401).json({
error: "Not authenticated with GitHub. Run `gh auth login`.",
});
return;
}
const url = `https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/issues?${params}`;
const headers: Record<string, string> = {
Accept: "application/vnd.github+json",
"X-GitHub-Api-Version": "2022-11-28",
"User-Agent": "kb-dashboard/1.0",
};
if (token) {
headers.Authorization = `Bearer ${token}`;
}
const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 30000);
const client = new GitHubClient();
try {
const response = await fetch(url, {
headers,
signal: controller.signal,
});
if (!response.ok) {
if (response.status === 404) {
res.status(404).json({ error: `Repository not found: ${owner}/${repo}` });
return;
}
if (response.status === 401 || response.status === 403) {
res.status(token ? 403 : 401).json({
error: `Authentication failed. ${token ? "Check your GITHUB_TOKEN." : "Set GITHUB_TOKEN env var."}`,
});
return;
}
res.status(502).json({ error: `GitHub API error: ${response.status} ${response.statusText}` });
const issues = await client.listIssues(owner, repo, { limit, labels });
res.json(issues);
} catch (err: any) {
// Handle specific error cases from gh CLI
const errorMessage = err instanceof Error ? err.message : String(err);
if (errorMessage.includes("not found") || errorMessage.includes("404")) {
res.status(404).json({ error: `Repository not found: ${owner}/${repo}` });
return;
}
// Filter out pull requests (they have a pull_request property)
const issues = (await response.json()) as Array<{
number: number;
title: string;
body: string | null;
html_url: string;
labels: Array<{ name: string }>;
pull_request?: unknown;
}>;
const filteredIssues = issues.filter((issue) => !issue.pull_request).slice(0, limit);
res.json(filteredIssues);
} finally {
clearTimeout(timeoutId);
if (errorMessage.includes("authentication") || errorMessage.includes("401") || errorMessage.includes("403")) {
res.status(401).json({
error: "Not authenticated with GitHub. Run `gh auth login`.",
});
return;
}
res.status(502).json({ error: `GitHub CLI error: ${errorMessage}` });
}
} catch (err: any) {
res.status(500).json({ error: err.message });
@@ -1405,56 +1374,49 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
return;
}
const token = process.env.GITHUB_TOKEN;
// Fetch the specific issue
const url = `https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/issues/${issueNumber}`;
const headers: Record<string, string> = {
Accept: "application/vnd.github+json",
"X-GitHub-Api-Version": "2022-11-28",
"User-Agent": "kb-dashboard/1.0",
};
if (token) {
headers.Authorization = `Bearer ${token}`;
// Check gh authentication
if (!isGhAuthenticated()) {
res.status(401).json({
error: "Not authenticated with GitHub. Run `gh auth login`.",
});
return;
}
const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 30000);
const client = new GitHubClient();
let issue: { number: number; title: string; body: string | null; html_url: string; pull_request?: unknown };
let issue: {
number: number;
title: string;
body: string | null;
html_url: string;
state: "open" | "closed";
} | null;
try {
const response = await fetch(url, {
headers,
signal: controller.signal,
});
issue = await client.getIssue(owner, repo, issueNumber);
if (!response.ok) {
if (response.status === 404) {
res.status(404).json({ error: `Issue #${issueNumber} not found in ${owner}/${repo}` });
return;
}
if (response.status === 401 || response.status === 403) {
res.status(token ? 403 : 401).json({
error: `Authentication failed. ${token ? "Check your GITHUB_TOKEN." : "Set GITHUB_TOKEN env var."}`,
});
return;
}
res.status(502).json({ error: `GitHub API error: ${response.status} ${response.statusText}` });
return;
}
issue = await response.json() as typeof issue;
// Check if it's a pull request
if (issue.pull_request) {
// getIssue returns null when the issue doesn't exist OR when it's a PR
// We return a 400 error indicating it might be a PR (consistent with old behavior)
if (issue === null) {
res.status(400).json({ error: `#${issueNumber} is a pull request, not an issue` });
return;
}
} finally {
clearTimeout(timeoutId);
} catch (err: any) {
const errorMessage = err instanceof Error ? err.message : String(err);
if (errorMessage.includes("not found") || errorMessage.includes("404")) {
res.status(404).json({ error: `Issue #${issueNumber} not found in ${owner}/${repo}` });
return;
}
if (errorMessage.includes("authentication") || errorMessage.includes("401") || errorMessage.includes("403")) {
res.status(401).json({
error: "Not authenticated with GitHub. Run `gh auth login`.",
});
return;
}
res.status(502).json({ error: `GitHub CLI error: ${errorMessage}` });
return;
}
// Check if already imported