feat(KB-066): add batch GitHub import with throttled request handling
- Add fetchThrottled utility to GitHubClient with 429 retry handling and exponential backoff - Implement POST /api/github/batch-import endpoint for bulk GitHub resource imports - Add frontend API client function for batch import operations - Add comprehensive tests for throttled requests and batch import endpoint - Add retryAfter field to batch import result types for rate limit handling - Clean up unfinished UsageIndicator component and worktree retry cleanup code - Add changeset documenting the new batch GitHub import feature
This commit is contained in:
gsxdsm
11
.changeset/batch-github-import.md
Normal file
11
.changeset/batch-github-import.md
Normal file
@@ -0,0 +1,11 @@
|
||||
---
|
||||
"@dustinbyrne/kb": minor
|
||||
---
|
||||
|
||||
Add batch GitHub issue import with intelligent throttling
|
||||
|
||||
- New `POST /api/github/issues/batch-import` endpoint for importing multiple issues sequentially
|
||||
- Throttled request utility with exponential backoff and Retry-After header support
|
||||
- Rate limit protection: 1 batch request per 10 seconds per IP
|
||||
- Frontend API client function `apiBatchImportGitHubIssues()` with full type support
|
||||
- Comprehensive test coverage for retry logic, validation, and error handling
|
||||
@@ -276,6 +276,29 @@ export function apiImportGitHubIssue(owner: string, repo: string, issueNumber: n
|
||||
});
|
||||
}
|
||||
|
||||
/** Result of a batch import operation for a single issue */
|
||||
export interface BatchImportResult {
|
||||
issueNumber: number;
|
||||
success: boolean;
|
||||
taskId?: string;
|
||||
error?: string;
|
||||
skipped?: boolean;
|
||||
retryAfter?: number;
|
||||
}
|
||||
|
||||
/** Batch import multiple GitHub issues as kb tasks with throttling */
|
||||
export function apiBatchImportGitHubIssues(
|
||||
owner: string,
|
||||
repo: string,
|
||||
issueNumbers: number[],
|
||||
delayMs?: number
|
||||
): Promise<{ results: BatchImportResult[] }> {
|
||||
return api<{ results: BatchImportResult[] }>("/github/issues/batch-import", {
|
||||
method: "POST",
|
||||
body: JSON.stringify({ owner, repo, issueNumbers, delayMs }),
|
||||
});
|
||||
}
|
||||
|
||||
// --- Git Remote Detection API ---
|
||||
|
||||
/** Git remote info returned by the remotes endpoint */
|
||||
|
||||
@@ -809,4 +809,222 @@ describe("GitHubClient", () => {
|
||||
})).rejects.toThrow("GitHub CLI (gh) is not available or not authenticated");
|
||||
});
|
||||
});
|
||||
|
||||
describe("fetchThrottled", () => {
|
||||
let fetchSpy: ReturnType<typeof vi.fn>;
|
||||
const originalFetch = globalThis.fetch;
|
||||
|
||||
beforeEach(() => {
|
||||
fetchSpy = vi.fn();
|
||||
globalThis.fetch = fetchSpy as any;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
globalThis.fetch = originalFetch;
|
||||
});
|
||||
|
||||
it("returns success with data on successful request", async () => {
|
||||
const mockData = { id: 1, title: "Test Issue" };
|
||||
fetchSpy.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: () => Promise.resolve(mockData),
|
||||
} as Response);
|
||||
|
||||
const result = await client.fetchThrottled("https://api.github.com/repos/owner/repo/issues/1");
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.data).toEqual(mockData);
|
||||
expect(result.error).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns error on non-429 HTTP error without retry", async () => {
|
||||
fetchSpy.mockResolvedValueOnce({
|
||||
ok: false,
|
||||
status: 404,
|
||||
statusText: "Not Found",
|
||||
json: () => Promise.resolve({ message: "Not Found" }),
|
||||
} as Response);
|
||||
|
||||
const result = await client.fetchThrottled("https://api.github.com/repos/owner/repo/issues/1");
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.error).toContain("404");
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(1); // No retries for non-429 errors
|
||||
});
|
||||
|
||||
it("retries on 429 with exponential backoff", async () => {
|
||||
fetchSpy
|
||||
.mockResolvedValueOnce({
|
||||
ok: false,
|
||||
status: 429,
|
||||
statusText: "Too Many Requests",
|
||||
headers: new Headers(),
|
||||
json: () => Promise.resolve({ message: "Rate limited" }),
|
||||
} as Response)
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: () => Promise.resolve({ id: 1 }),
|
||||
} as Response);
|
||||
|
||||
const result = await client.fetchThrottled(
|
||||
"https://api.github.com/repos/owner/repo/issues/1",
|
||||
{},
|
||||
{ delayMs: 10, maxRetries: 3 }
|
||||
);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.data).toEqual({ id: 1 });
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("respects Retry-After header on 429", async () => {
|
||||
const headers = new Headers();
|
||||
headers.set("Retry-After", "1"); // Use 1 second for test speed
|
||||
|
||||
fetchSpy
|
||||
.mockResolvedValueOnce({
|
||||
ok: false,
|
||||
status: 429,
|
||||
statusText: "Too Many Requests",
|
||||
headers,
|
||||
json: () => Promise.resolve({ message: "Rate limited" }),
|
||||
} as Response)
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: () => Promise.resolve({ id: 1 }),
|
||||
} as Response);
|
||||
|
||||
const startTime = Date.now();
|
||||
const result = await client.fetchThrottled(
|
||||
"https://api.github.com/repos/owner/repo/issues/1",
|
||||
{},
|
||||
{ delayMs: 100, maxRetries: 3 }
|
||||
);
|
||||
const elapsed = Date.now() - startTime;
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
// Should wait at least 1 second (Retry-After value), not just the exponential backoff
|
||||
expect(elapsed).toBeGreaterThanOrEqual(900); // Allow some tolerance
|
||||
}, 10000); // Increase timeout for this test
|
||||
|
||||
it("returns error with retryAfter after max retries exceeded", async () => {
|
||||
const headers = new Headers();
|
||||
headers.set("Retry-After", "1"); // Use 1 second for test speed
|
||||
|
||||
// All attempts return 429
|
||||
fetchSpy.mockResolvedValue({
|
||||
ok: false,
|
||||
status: 429,
|
||||
statusText: "Too Many Requests",
|
||||
headers,
|
||||
json: () => Promise.resolve({ message: "Rate limited" }),
|
||||
} as Response);
|
||||
|
||||
const result = await client.fetchThrottled(
|
||||
"https://api.github.com/repos/owner/repo/issues/1",
|
||||
{},
|
||||
{ delayMs: 1, maxRetries: 2 }
|
||||
);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.error).toContain("rate limit exceeded");
|
||||
expect(result.retryAfter).toBe(1);
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(3); // initial + 2 retries
|
||||
}, 10000); // Increase timeout for this test
|
||||
|
||||
it("retries on network errors with exponential backoff", async () => {
|
||||
fetchSpy
|
||||
.mockRejectedValueOnce(new Error("Network error"))
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: () => Promise.resolve({ id: 1 }),
|
||||
} as Response);
|
||||
|
||||
const result = await client.fetchThrottled(
|
||||
"https://api.github.com/repos/owner/repo/issues/1",
|
||||
{},
|
||||
{ delayMs: 10, maxRetries: 3 }
|
||||
);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.data).toEqual({ id: 1 });
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("returns error after max retries on persistent network errors", async () => {
|
||||
fetchSpy.mockRejectedValue(new Error("Network error"));
|
||||
|
||||
const result = await client.fetchThrottled(
|
||||
"https://api.github.com/repos/owner/repo/issues/1",
|
||||
{},
|
||||
{ delayMs: 1, maxRetries: 2 }
|
||||
);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.error).toBe("Network error");
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(3); // initial + 2 retries
|
||||
});
|
||||
|
||||
it("enforces delay between sequential requests", async () => {
|
||||
fetchSpy.mockResolvedValue({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: () => Promise.resolve({ id: 1 }),
|
||||
} as Response);
|
||||
|
||||
// First request
|
||||
await client.fetchThrottled(
|
||||
"https://api.github.com/repos/owner/repo/issues/1",
|
||||
{},
|
||||
{ delayMs: 100 }
|
||||
);
|
||||
|
||||
const startTime = Date.now();
|
||||
// Second request should be delayed
|
||||
await client.fetchThrottled(
|
||||
"https://api.github.com/repos/owner/repo/issues/2",
|
||||
{},
|
||||
{ delayMs: 100 }
|
||||
);
|
||||
const elapsed = Date.now() - startTime;
|
||||
|
||||
// Should have waited at least 100ms between requests
|
||||
expect(elapsed).toBeGreaterThanOrEqual(90); // Allow some tolerance
|
||||
});
|
||||
|
||||
it("uses custom delayMs option", async () => {
|
||||
fetchSpy.mockResolvedValue({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: () => Promise.resolve({ id: 1 }),
|
||||
} as Response);
|
||||
|
||||
const startTime = Date.now();
|
||||
await client.fetchThrottled(
|
||||
"https://api.github.com/repos/owner/repo/issues/1",
|
||||
{},
|
||||
{ delayMs: 200 }
|
||||
);
|
||||
const elapsed = Date.now() - startTime;
|
||||
|
||||
// Should be relatively quick since no previous request
|
||||
expect(elapsed).toBeLessThan(100);
|
||||
});
|
||||
|
||||
it("uses custom maxRetries option", async () => {
|
||||
fetchSpy.mockRejectedValue(new Error("Network error"));
|
||||
|
||||
await client.fetchThrottled(
|
||||
"https://api.github.com/repos/owner/repo/issues/1",
|
||||
{},
|
||||
{ delayMs: 1, maxRetries: 1 }
|
||||
);
|
||||
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(2); // initial + 1 retry
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -9,6 +9,33 @@ import {
|
||||
runGh,
|
||||
} from "@kb/core";
|
||||
|
||||
/**
|
||||
* Sleep for a specified number of milliseconds.
|
||||
*/
|
||||
function delay(ms: number): Promise<void> {
|
||||
return new Promise((resolve) => setTimeout(resolve, ms));
|
||||
}
|
||||
|
||||
/**
|
||||
* Result of a throttled fetch operation.
|
||||
*/
|
||||
export interface ThrottledFetchResult<T> {
|
||||
success: boolean;
|
||||
data?: T;
|
||||
error?: string;
|
||||
retryAfter?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Options for throttled fetch operations.
|
||||
*/
|
||||
export interface ThrottledFetchOptions {
|
||||
/** Delay between requests in milliseconds (default: 1000ms) */
|
||||
delayMs?: number;
|
||||
/** Maximum number of retries on 429 responses (default: 3) */
|
||||
maxRetries?: number;
|
||||
}
|
||||
|
||||
export interface CreatePrParams {
|
||||
owner?: string;
|
||||
repo?: string;
|
||||
@@ -244,6 +271,7 @@ export function isPrMergeReady(input: {
|
||||
export class GitHubClient {
|
||||
private token: string | undefined;
|
||||
private baseUrl = "https://api.github.com";
|
||||
private lastRequestTime = 0;
|
||||
|
||||
/**
|
||||
* Create a GitHub client.
|
||||
@@ -1047,6 +1075,105 @@ export class GitHubClient {
|
||||
return normalizeBadgeBatchPayload(payload.data?.repository, requests);
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch a URL with throttling and automatic retry on rate limit (429) responses.
|
||||
* Implements exponential backoff and respects Retry-After header when present.
|
||||
* Ensures minimum delay between sequential requests.
|
||||
*/
|
||||
async fetchThrottled<T>(
|
||||
url: string,
|
||||
options: RequestInit = {},
|
||||
throttleOptions: ThrottledFetchOptions = {},
|
||||
): Promise<ThrottledFetchResult<T>> {
|
||||
const { delayMs = 1000, maxRetries = 3 } = throttleOptions;
|
||||
|
||||
// Enforce delay between sequential requests
|
||||
const now = Date.now();
|
||||
const timeSinceLastRequest = now - this.lastRequestTime;
|
||||
if (this.lastRequestTime > 0 && timeSinceLastRequest < delayMs) {
|
||||
await delay(delayMs - timeSinceLastRequest);
|
||||
}
|
||||
|
||||
let didBackoffDelay = false;
|
||||
|
||||
for (let attempt = 0; attempt <= maxRetries; attempt++) {
|
||||
try {
|
||||
// On retry attempts (after first failure), apply delay
|
||||
// Skip if we already applied backoff delay in previous iteration
|
||||
if (attempt > 0 && !didBackoffDelay) {
|
||||
await delay(delayMs);
|
||||
}
|
||||
didBackoffDelay = false; // Reset for this iteration
|
||||
|
||||
this.lastRequestTime = Date.now();
|
||||
|
||||
const response = await fetch(url, {
|
||||
...options,
|
||||
headers: {
|
||||
...this.buildHeaders(),
|
||||
...(options.headers || {}),
|
||||
},
|
||||
});
|
||||
|
||||
// Handle rate limit (429) with retry logic
|
||||
if (response.status === 429) {
|
||||
const retryAfter = response.headers.get("Retry-After");
|
||||
const retryAfterSeconds = retryAfter ? parseInt(retryAfter, 10) : undefined;
|
||||
|
||||
// If this is the last retry, return the error
|
||||
if (attempt >= maxRetries) {
|
||||
return {
|
||||
success: false,
|
||||
error: `GitHub API rate limit exceeded. Retry after ${retryAfterSeconds ?? "unknown"} seconds.`,
|
||||
retryAfter: retryAfterSeconds,
|
||||
};
|
||||
}
|
||||
|
||||
// Calculate exponential backoff delay
|
||||
// Use Retry-After header if present, otherwise use exponential backoff
|
||||
const backoffDelay = retryAfterSeconds
|
||||
? retryAfterSeconds * 1000
|
||||
: delayMs * Math.pow(2, attempt);
|
||||
|
||||
await delay(backoffDelay);
|
||||
didBackoffDelay = true;
|
||||
// Continue to next iteration - the backoff delay was already applied
|
||||
// so we skip the standard inter-request delay logic
|
||||
continue;
|
||||
}
|
||||
|
||||
// Handle other non-OK responses (don't retry)
|
||||
if (!response.ok) {
|
||||
const error = await response.json().catch(() => ({ message: response.statusText }));
|
||||
return {
|
||||
success: false,
|
||||
error: `GitHub API error: ${response.status} ${error.message || response.statusText}`,
|
||||
};
|
||||
}
|
||||
|
||||
// Success - parse and return data
|
||||
const data = await response.json() as T;
|
||||
return { success: true, data };
|
||||
} catch (err) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
|
||||
// On last attempt, return the error
|
||||
if (attempt >= maxRetries) {
|
||||
return { success: false, error: errorMessage };
|
||||
}
|
||||
|
||||
// For network errors, wait and retry with exponential backoff
|
||||
// Skip standard inter-request delay since we're applying backoff
|
||||
const backoffDelay = delayMs * Math.pow(2, attempt);
|
||||
await delay(backoffDelay);
|
||||
didBackoffDelay = true;
|
||||
}
|
||||
}
|
||||
|
||||
// Should never reach here, but TypeScript needs it
|
||||
return { success: false, error: "Max retries exceeded" };
|
||||
}
|
||||
|
||||
private buildHeaders(): Record<string, string> {
|
||||
const headers: Record<string, string> = {
|
||||
Accept: "application/vnd.github+json",
|
||||
|
||||
@@ -2100,6 +2100,360 @@ describe("POST /github/issues/import", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("POST /github/issues/batch-import", () => {
|
||||
let store: TaskStore;
|
||||
let fetchSpy: ReturnType<typeof vi.fn>;
|
||||
const originalFetch = globalThis.fetch;
|
||||
|
||||
beforeEach(() => {
|
||||
fetchSpy = vi.fn();
|
||||
globalThis.fetch = fetchSpy as any;
|
||||
|
||||
store = createMockStore({
|
||||
listTasks: vi.fn().mockResolvedValue([]),
|
||||
createTask: vi.fn().mockImplementation((input) =>
|
||||
Promise.resolve({
|
||||
id: `KB-${String(Math.floor(Math.random() * 999)).padStart(3, "0")}`,
|
||||
title: input.title,
|
||||
description: input.description,
|
||||
column: "triage",
|
||||
})
|
||||
),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
globalThis.fetch = originalFetch;
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
function buildApp() {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(store));
|
||||
return app;
|
||||
}
|
||||
|
||||
const mockGitHubIssue = (number: number, title = `Issue ${number}`) => ({
|
||||
number,
|
||||
title,
|
||||
body: `Body for issue ${number}`,
|
||||
html_url: `https://github.com/owner/repo/issues/${number}`,
|
||||
labels: [{ name: "bug" }],
|
||||
});
|
||||
|
||||
it("imports multiple issues successfully", async () => {
|
||||
fetchSpy
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: () => Promise.resolve(mockGitHubIssue(1, "First Issue")),
|
||||
} as Response)
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: () => Promise.resolve(mockGitHubIssue(2, "Second Issue")),
|
||||
} as Response)
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: () => Promise.resolve(mockGitHubIssue(3, "Third Issue")),
|
||||
} as Response);
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/github/issues/batch-import",
|
||||
JSON.stringify({ owner: "owner", repo: "repo", issueNumbers: [1, 2, 3], delayMs: 10 }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.results).toHaveLength(3);
|
||||
expect(res.body.results.every((r: { success: boolean }) => r.success)).toBe(true);
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(3);
|
||||
expect(store.createTask).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it("skips already-imported issues", async () => {
|
||||
// Mock issue 1 fetch
|
||||
fetchSpy.mockResolvedValue({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: () => Promise.resolve(mockGitHubIssue(1, "Already Imported Issue")),
|
||||
} as Response);
|
||||
|
||||
// First import - should create a new task
|
||||
const res1 = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/github/issues/batch-import",
|
||||
JSON.stringify({ owner: "owner", repo: "repo", issueNumbers: [1], delayMs: 10 }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res1.status).toBe(200);
|
||||
expect(res1.body.results).toHaveLength(1);
|
||||
expect(res1.body.results[0].success).toBe(true);
|
||||
expect(res1.body.results[0].skipped).toBeUndefined();
|
||||
const createdTaskId = res1.body.results[0].taskId;
|
||||
expect(createdTaskId).toBeDefined();
|
||||
|
||||
// Now verify that if we import again with the task in the list, it gets skipped
|
||||
// Update the listTasks mock to return the created task
|
||||
const createdTaskDescription = `Already Imported Issue\n\nSource: https://github.com/owner/repo/issues/1`;
|
||||
store.listTasks = vi.fn().mockResolvedValue([
|
||||
{
|
||||
id: createdTaskId,
|
||||
description: createdTaskDescription,
|
||||
column: "triage",
|
||||
},
|
||||
]);
|
||||
|
||||
// Second import - should skip
|
||||
const res2 = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/github/issues/batch-import",
|
||||
JSON.stringify({ owner: "owner", repo: "repo", issueNumbers: [1], delayMs: 10 }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res2.status).toBe(200);
|
||||
expect(res2.body.results).toHaveLength(1);
|
||||
expect(res2.body.results[0].success).toBe(true);
|
||||
expect(res2.body.results[0].skipped).toBe(true);
|
||||
expect(res2.body.results[0].taskId).toBe(createdTaskId);
|
||||
});
|
||||
|
||||
it("returns 400 for empty issueNumbers array", async () => {
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/github/issues/batch-import",
|
||||
JSON.stringify({ owner: "owner", repo: "repo", issueNumbers: [] }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("at least 1");
|
||||
});
|
||||
|
||||
it("returns 400 for more than 50 issue numbers", async () => {
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/github/issues/batch-import",
|
||||
JSON.stringify({ owner: "owner", repo: "repo", issueNumbers: Array.from({ length: 51 }, (_, i) => i + 1) }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("more than 50");
|
||||
});
|
||||
|
||||
it("returns 400 for invalid issueNumbers (non-integers)", async () => {
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/github/issues/batch-import",
|
||||
JSON.stringify({ owner: "owner", repo: "repo", issueNumbers: [1, "two", 3] }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("positive integers");
|
||||
});
|
||||
|
||||
it("handles partial failures (some succeed, some fail)", async () => {
|
||||
fetchSpy
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: () => Promise.resolve(mockGitHubIssue(1)),
|
||||
} as Response)
|
||||
.mockResolvedValueOnce({
|
||||
ok: false,
|
||||
status: 404,
|
||||
statusText: "Not Found",
|
||||
json: () => Promise.resolve({ message: "Not Found" }),
|
||||
} as Response)
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: () => Promise.resolve(mockGitHubIssue(3)),
|
||||
} as Response);
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/github/issues/batch-import",
|
||||
JSON.stringify({ owner: "owner", repo: "repo", issueNumbers: [1, 2, 3], delayMs: 10 }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.results).toHaveLength(3);
|
||||
expect(res.body.results[0].success).toBe(true);
|
||||
expect(res.body.results[1].success).toBe(false);
|
||||
expect(res.body.results[1].error).toContain("404");
|
||||
expect(res.body.results[2].success).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects pull requests with appropriate error", async () => {
|
||||
fetchSpy.mockResolvedValue({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: () => Promise.resolve({ ...mockGitHubIssue(1), pull_request: {} }),
|
||||
} as Response);
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/github/issues/batch-import",
|
||||
JSON.stringify({ owner: "owner", repo: "repo", issueNumbers: [1], delayMs: 10 }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.results[0].success).toBe(false);
|
||||
expect(res.body.results[0].error).toContain("pull request");
|
||||
});
|
||||
|
||||
it("handles rate limit (429) with retry and eventual success", async () => {
|
||||
fetchSpy
|
||||
.mockResolvedValueOnce({
|
||||
ok: false,
|
||||
status: 429,
|
||||
statusText: "Too Many Requests",
|
||||
headers: new Headers({ "Retry-After": "1" }),
|
||||
json: () => Promise.resolve({ message: "Rate limited" }),
|
||||
} as Response)
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: () => Promise.resolve(mockGitHubIssue(1, "Issue After Rate Limit")),
|
||||
} as Response);
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/github/issues/batch-import",
|
||||
JSON.stringify({ owner: "owner", repo: "repo", issueNumbers: [1], delayMs: 10 }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.results).toHaveLength(1);
|
||||
expect(res.body.results[0].success).toBe(true);
|
||||
expect(res.body.results[0].taskId).toBeDefined();
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(2); // Initial 429 + 1 retry
|
||||
}, 10000); // Increase timeout for retry delay
|
||||
|
||||
it("returns error after max retries exceeded on 429", async () => {
|
||||
// Always return 429
|
||||
fetchSpy.mockResolvedValue({
|
||||
ok: false,
|
||||
status: 429,
|
||||
statusText: "Too Many Requests",
|
||||
headers: new Headers({ "Retry-After": "1" }), // 1 second for test speed
|
||||
json: () => Promise.resolve({ message: "Rate limited" }),
|
||||
} as Response);
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/github/issues/batch-import",
|
||||
JSON.stringify({ owner: "owner", repo: "repo", issueNumbers: [1], delayMs: 1 }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.results).toHaveLength(1);
|
||||
expect(res.body.results[0].success).toBe(false);
|
||||
expect(res.body.results[0].error).toContain("rate limit");
|
||||
expect(res.body.results[0].retryAfter).toBe(1);
|
||||
// Initial attempt + 3 retries = 4 calls
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(4);
|
||||
}, 15000); // Increase timeout for multiple retries
|
||||
|
||||
it("processes issues sequentially (not parallel)", async () => {
|
||||
const callTimes: number[] = [];
|
||||
fetchSpy.mockImplementation(() => {
|
||||
callTimes.push(Date.now());
|
||||
return Promise.resolve({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: () => Promise.resolve(mockGitHubIssue(callTimes.length)),
|
||||
} as Response);
|
||||
});
|
||||
|
||||
await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/github/issues/batch-import",
|
||||
JSON.stringify({ owner: "owner", repo: "repo", issueNumbers: [1, 2, 3], delayMs: 50 }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
// Verify sequential processing by checking call timing
|
||||
expect(callTimes).toHaveLength(3);
|
||||
// Each call should be at least 40ms after the previous (allowing for small timing variations)
|
||||
for (let i = 1; i < callTimes.length; i++) {
|
||||
expect(callTimes[i] - callTimes[i - 1]).toBeGreaterThanOrEqual(40);
|
||||
}
|
||||
});
|
||||
|
||||
it("requires owner parameter", async () => {
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/github/issues/batch-import",
|
||||
JSON.stringify({ repo: "repo", issueNumbers: [1] }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("owner");
|
||||
});
|
||||
|
||||
it("requires repo parameter", async () => {
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/github/issues/batch-import",
|
||||
JSON.stringify({ owner: "owner", issueNumbers: [1] }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("repo");
|
||||
});
|
||||
|
||||
it("logs import actions for created tasks", async () => {
|
||||
fetchSpy.mockResolvedValue({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: () => Promise.resolve(mockGitHubIssue(1)),
|
||||
} as Response);
|
||||
|
||||
await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/github/issues/batch-import",
|
||||
JSON.stringify({ owner: "owner", repo: "repo", issueNumbers: [1], delayMs: 10 }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
expect.any(String),
|
||||
"Imported from GitHub",
|
||||
"https://github.com/owner/repo/issues/1"
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// --- Spec Revision route tests ---
|
||||
|
||||
describe("POST /tasks/:id/spec/revise", () => {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { Router } from "express";
|
||||
import { Router, type Request, type Response, type NextFunction } from "express";
|
||||
import multer from "multer";
|
||||
import { createReadStream } from "node:fs";
|
||||
import { execSync } from "node:child_process";
|
||||
@@ -1491,6 +1491,182 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* POST /api/github/issues/batch-import
|
||||
* Import multiple GitHub issues as kb tasks with throttling.
|
||||
* Body: { owner: string, repo: string, issueNumbers: number[], delayMs?: number }
|
||||
* Returns: { results: BatchImportResult[] }
|
||||
*/
|
||||
// Batch import rate limiter: max 1 request per 10 seconds per IP
|
||||
const batchImportRateLimiter = (() => {
|
||||
const clients = new Map<string, number>();
|
||||
const windowMs = 10_000; // 10 seconds
|
||||
|
||||
setInterval(() => {
|
||||
const now = Date.now();
|
||||
for (const [ip, resetTime] of clients) {
|
||||
if (now >= resetTime) {
|
||||
clients.delete(ip);
|
||||
}
|
||||
}
|
||||
}, windowMs);
|
||||
|
||||
return (req: Request, res: Response, next: NextFunction): void => {
|
||||
const ip = req.ip ?? req.socket.remoteAddress ?? "unknown";
|
||||
const now = Date.now();
|
||||
|
||||
const resetTime = clients.get(ip);
|
||||
if (resetTime && now < resetTime) {
|
||||
const retryAfter = Math.ceil((resetTime - now) / 1000);
|
||||
res.setHeader("Retry-After", String(retryAfter));
|
||||
res.status(429).json({ error: "Batch import rate limit exceeded. Try again in a few seconds." });
|
||||
return;
|
||||
}
|
||||
|
||||
clients.set(ip, now + windowMs);
|
||||
next();
|
||||
};
|
||||
})();
|
||||
|
||||
router.post("/github/issues/batch-import", batchImportRateLimiter, async (req, res) => {
|
||||
try {
|
||||
const { owner, repo, issueNumbers, delayMs } = req.body;
|
||||
|
||||
// Validate owner
|
||||
if (!owner || typeof owner !== "string") {
|
||||
res.status(400).json({ error: "owner is required" });
|
||||
return;
|
||||
}
|
||||
|
||||
// Validate repo
|
||||
if (!repo || typeof repo !== "string") {
|
||||
res.status(400).json({ error: "repo is required" });
|
||||
return;
|
||||
}
|
||||
|
||||
// Validate issueNumbers
|
||||
if (!Array.isArray(issueNumbers)) {
|
||||
res.status(400).json({ error: "issueNumbers is required and must be an array" });
|
||||
return;
|
||||
}
|
||||
|
||||
if (issueNumbers.length === 0) {
|
||||
res.status(400).json({ error: "issueNumbers must contain at least 1 issue number" });
|
||||
return;
|
||||
}
|
||||
|
||||
if (issueNumbers.length > 50) {
|
||||
res.status(400).json({ error: "issueNumbers cannot contain more than 50 issue numbers" });
|
||||
return;
|
||||
}
|
||||
|
||||
if (!issueNumbers.every((n) => typeof n === "number" && n > 0 && Number.isInteger(n))) {
|
||||
res.status(400).json({ error: "issueNumbers must contain only positive integers" });
|
||||
return;
|
||||
}
|
||||
|
||||
const token = process.env.GITHUB_TOKEN;
|
||||
const githubClient = new GitHubClient(token);
|
||||
|
||||
// Get existing tasks to check for duplicates
|
||||
const existingTasks = await store.listTasks();
|
||||
|
||||
// Process issues sequentially with throttling
|
||||
const results: Array<{
|
||||
issueNumber: number;
|
||||
success: boolean;
|
||||
taskId?: string;
|
||||
error?: string;
|
||||
skipped?: boolean;
|
||||
retryAfter?: number;
|
||||
}> = [];
|
||||
|
||||
for (const issueNumber of issueNumbers) {
|
||||
const url = `https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/issues/${issueNumber}`;
|
||||
|
||||
// Use throttled fetch to avoid rate limits
|
||||
const fetchResult = await githubClient.fetchThrottled<{
|
||||
number: number;
|
||||
title: string;
|
||||
body: string | null;
|
||||
html_url: string;
|
||||
pull_request?: unknown;
|
||||
}>(url, {}, { delayMs: delayMs ?? 1000, maxRetries: 3 });
|
||||
|
||||
if (!fetchResult.success) {
|
||||
results.push({
|
||||
issueNumber,
|
||||
success: false,
|
||||
error: fetchResult.error ?? "Failed to fetch issue",
|
||||
retryAfter: fetchResult.retryAfter,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
const issue = fetchResult.data!;
|
||||
|
||||
// Check if it's a pull request
|
||||
if (issue.pull_request) {
|
||||
results.push({
|
||||
issueNumber,
|
||||
success: false,
|
||||
error: "This is a pull request, not an issue",
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
// Check if already imported
|
||||
const sourceUrl = issue.html_url;
|
||||
const existingTask = existingTasks.find((t) => t.description.includes(sourceUrl));
|
||||
if (existingTask) {
|
||||
results.push({
|
||||
issueNumber,
|
||||
success: true,
|
||||
skipped: true,
|
||||
taskId: existingTask.id,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
// Create the task
|
||||
const title = issue.title.slice(0, 200);
|
||||
const body = issue.body?.trim() || "(no description)";
|
||||
const description = `${body}\n\nSource: ${sourceUrl}`;
|
||||
|
||||
try {
|
||||
const task = await store.createTask({
|
||||
title: title || undefined,
|
||||
description,
|
||||
column: "triage",
|
||||
dependencies: [],
|
||||
});
|
||||
|
||||
// Log the import action
|
||||
await store.logEntry(task.id, "Imported from GitHub", sourceUrl);
|
||||
|
||||
results.push({
|
||||
issueNumber,
|
||||
success: true,
|
||||
taskId: task.id,
|
||||
});
|
||||
|
||||
// Add to existingTasks to avoid duplicate imports within the same batch
|
||||
existingTasks.push({ ...task, description });
|
||||
} catch (err: any) {
|
||||
results.push({
|
||||
issueNumber,
|
||||
success: false,
|
||||
error: err.message ?? "Failed to create task",
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
res.json({ results });
|
||||
} catch (err: any) {
|
||||
res.status(500).json({ error: err.message });
|
||||
}
|
||||
});
|
||||
|
||||
// ---------- Auth routes ----------
|
||||
registerAuthRoutes(router, options?.authStorage);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user