feat(KB-004): add GitHub PR creation and comment monitoring
- Add PR fields (prInfo, prNumber, prUrl) to Task type and TaskStore with updatePrInfo method - Add PR Management API endpoints: create, status, refresh with per-repo rate limiting - Add GitHubClient for PR creation and status fetching with comment support - Add PrSection component for PR status display and actions in dashboard - Add PR comment monitoring engine with PrMonitor and PrCommentHandler - Integrate PR monitoring into scheduler for automatic PR tracking - Add comprehensive tests for PR features in all packages
This commit is contained in:
gsxdsm
25
README.md
25
README.md
@@ -210,6 +210,31 @@ KB_CLIENT_DIR=/path/to/client ./kb dashboard
|
||||
|
||||
**Prerequisites:** Bun ≥ 1.0 (`bun --version`)
|
||||
|
||||
## GitHub Integration
|
||||
|
||||
### PR Creation from Dashboard
|
||||
|
||||
kb can create GitHub Pull Requests directly from the dashboard for tasks in the **In Review** column:
|
||||
|
||||
1. Set the `GITHUB_TOKEN` environment variable with a personal access token (requires `repo` scope)
|
||||
2. Open a task in the **In Review** column
|
||||
3. Click **"Create PR"** in the Pull Request section
|
||||
4. Enter a title and optional description
|
||||
5. The PR is created and linked to the task automatically
|
||||
|
||||
The dashboard shows real-time PR status (open, closed, merged) with a refresh button to fetch the latest state from GitHub.
|
||||
|
||||
### PR Comment Monitoring
|
||||
|
||||
When a task has a linked PR, kb automatically monitors it for new review comments:
|
||||
|
||||
- **Adaptive polling**: Checks every 30 seconds when active, 5 minutes when idle
|
||||
- **Actionable feedback detection**: Filters out "LGTM" and "Thanks" comments, detects requests like "fix", "change", "update"
|
||||
- **Steering comments**: Automatically adds actionable review feedback as steering comments on the task
|
||||
- **Follow-up tasks**: When a PR is closed with unaddressed feedback, a follow-up task is created
|
||||
|
||||
Configure via the same `GITHUB_TOKEN` environment variable.
|
||||
|
||||
## Releases
|
||||
|
||||
Packages are published to npm automatically via GitHub Actions and [changesets](https://github.com/changesets/changesets).
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
export { COLUMNS, COLUMN_LABELS, COLUMN_DESCRIPTIONS, VALID_TRANSITIONS, DEFAULT_SETTINGS, THINKING_LEVELS } from "./types.js";
|
||||
export type { Column, Task, TaskAttachment, TaskCreateInput, TaskDetail, AgentLogEntry, AgentLogType, AgentRole, BoardConfig, MergeResult, Settings, TaskStep, StepStatus, TaskLogEntry, ThinkingLevel, SteeringComment } from "./types.js";
|
||||
export type { Column, PrInfo, PrStatus, Task, TaskAttachment, TaskCreateInput, TaskDetail, AgentLogEntry, AgentLogType, AgentRole, BoardConfig, MergeResult, Settings, TaskStep, StepStatus, TaskLogEntry, ThinkingLevel, SteeringComment } from "./types.js";
|
||||
export { TaskStore } from "./store.js";
|
||||
export { canTransition, getValidTransitions, resolveDependencyOrder } from "./board.js";
|
||||
|
||||
@@ -810,6 +810,189 @@ describe("TaskStore", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("updatePrInfo", () => {
|
||||
it("adds PR info to a task without existing PR", async () => {
|
||||
const task = await createTestTask();
|
||||
const prInfo = {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open" as const,
|
||||
title: "Fix the bug",
|
||||
headBranch: "kb-001-fix-bug",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
};
|
||||
|
||||
const updated = await store.updatePrInfo(task.id, prInfo);
|
||||
|
||||
expect(updated.prInfo).toEqual(prInfo);
|
||||
expect(updated.log.some((l) => l.action === "PR linked" && l.outcome?.includes("#42"))).toBe(true);
|
||||
});
|
||||
|
||||
it("updates existing PR info with new values", async () => {
|
||||
const task = await createTestTask();
|
||||
const prInfo1 = {
|
||||
url: "https://github.com/owner/repo/pull/1",
|
||||
number: 1,
|
||||
status: "open" as const,
|
||||
title: "Initial PR",
|
||||
headBranch: "branch-1",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
};
|
||||
await store.updatePrInfo(task.id, prInfo1);
|
||||
|
||||
const prInfo2 = {
|
||||
url: "https://github.com/owner/repo/pull/1",
|
||||
number: 1,
|
||||
status: "merged" as const,
|
||||
title: "Initial PR (updated)",
|
||||
headBranch: "branch-1",
|
||||
baseBranch: "main",
|
||||
commentCount: 3,
|
||||
lastCommentAt: "2026-01-01T00:00:00.000Z",
|
||||
};
|
||||
const updated = await store.updatePrInfo(task.id, prInfo2);
|
||||
|
||||
expect(updated.prInfo?.status).toBe("merged");
|
||||
expect(updated.prInfo?.commentCount).toBe(3);
|
||||
expect(updated.prInfo?.lastCommentAt).toBe("2026-01-01T00:00:00.000Z");
|
||||
});
|
||||
|
||||
it("clears PR info when passed null", async () => {
|
||||
const task = await createTestTask();
|
||||
const prInfo = {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open" as const,
|
||||
title: "Fix the bug",
|
||||
headBranch: "kb-001-fix-bug",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
};
|
||||
await store.updatePrInfo(task.id, prInfo);
|
||||
|
||||
const updated = await store.updatePrInfo(task.id, null);
|
||||
|
||||
expect(updated.prInfo).toBeUndefined();
|
||||
expect(updated.log.some((l) => l.action === "PR unlinked")).toBe(true);
|
||||
});
|
||||
|
||||
it("emits task:updated event when PR info changes", async () => {
|
||||
const task = await createTestTask();
|
||||
const events: any[] = [];
|
||||
store.on("task:updated", (t) => events.push(t));
|
||||
|
||||
const prInfo = {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open" as const,
|
||||
title: "Fix the bug",
|
||||
headBranch: "kb-001-fix-bug",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
};
|
||||
await store.updatePrInfo(task.id, prInfo);
|
||||
|
||||
expect(events).toHaveLength(1);
|
||||
expect(events[0].prInfo?.number).toBe(42);
|
||||
});
|
||||
|
||||
it("does NOT emit task:updated when PR info is unchanged", async () => {
|
||||
const task = await createTestTask();
|
||||
const prInfo = {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open" as const,
|
||||
title: "Fix the bug",
|
||||
headBranch: "kb-001-fix-bug",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
};
|
||||
await store.updatePrInfo(task.id, prInfo);
|
||||
|
||||
const events: any[] = [];
|
||||
store.on("task:updated", (t) => events.push(t));
|
||||
|
||||
// Update with same values (status and number unchanged)
|
||||
await store.updatePrInfo(task.id, { ...prInfo });
|
||||
|
||||
// Should not emit because number and status are the same
|
||||
expect(events).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("persists to disk and round-trips correctly", async () => {
|
||||
const task = await createTestTask();
|
||||
const prInfo = {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open" as const,
|
||||
title: "Fix the bug",
|
||||
headBranch: "kb-001-fix-bug",
|
||||
baseBranch: "main",
|
||||
commentCount: 5,
|
||||
lastCommentAt: "2026-03-30T12:00:00.000Z",
|
||||
};
|
||||
|
||||
await store.updatePrInfo(task.id, prInfo);
|
||||
const fetched = await store.getTask(task.id);
|
||||
|
||||
expect(fetched.prInfo).toEqual(prInfo);
|
||||
});
|
||||
|
||||
it("updates updatedAt timestamp", async () => {
|
||||
const task = await createTestTask();
|
||||
const before = task.updatedAt;
|
||||
await new Promise((r) => setTimeout(r, 10)); // Ensure time passes
|
||||
|
||||
const prInfo = {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open" as const,
|
||||
title: "Fix the bug",
|
||||
headBranch: "kb-001-fix-bug",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
};
|
||||
const updated = await store.updatePrInfo(task.id, prInfo);
|
||||
|
||||
expect(updated.updatedAt).not.toBe(before);
|
||||
});
|
||||
|
||||
it("serializes concurrent updates correctly", async () => {
|
||||
const task = await createTestTask();
|
||||
|
||||
// Fire 5 concurrent updates
|
||||
const promises = Array.from({ length: 5 }, (_, i) =>
|
||||
store.updatePrInfo(task.id, {
|
||||
url: `https://github.com/owner/repo/pull/${i + 1}`,
|
||||
number: i + 1,
|
||||
status: "open" as const,
|
||||
title: `PR ${i + 1}`,
|
||||
headBranch: `branch-${i + 1}`,
|
||||
baseBranch: "main",
|
||||
commentCount: i,
|
||||
}),
|
||||
);
|
||||
|
||||
await Promise.all(promises);
|
||||
|
||||
// Read back and verify valid JSON
|
||||
const taskJsonPath = join(rootDir, ".kb", "tasks", task.id, "task.json");
|
||||
const raw = await readFile(taskJsonPath, "utf-8");
|
||||
const result = JSON.parse(raw) as Task;
|
||||
|
||||
// Should have exactly one of the PRs set (last one wins)
|
||||
expect(result.prInfo).toBeDefined();
|
||||
expect(result.prInfo!.number).toBeGreaterThanOrEqual(1);
|
||||
expect(result.prInfo!.number).toBeLessThanOrEqual(5);
|
||||
|
||||
// Should have all the PR linked log entries
|
||||
const prLogs = result.log.filter((l) => l.action === "PR linked");
|
||||
expect(prLogs).toHaveLength(5);
|
||||
});
|
||||
});
|
||||
|
||||
describe("parseDependenciesFromPrompt", () => {
|
||||
it("returns single dependency from PROMPT.md", async () => {
|
||||
const task = await store.createTask({ description: "Task with dep" });
|
||||
|
||||
@@ -1023,6 +1023,57 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Update or clear PR information for a task.
|
||||
* Updates task.json atomically and emits `task:updated` event.
|
||||
*
|
||||
* @param id - The task ID
|
||||
* @param prInfo - The PR info to set, or null to clear
|
||||
* @returns The updated task
|
||||
*/
|
||||
async updatePrInfo(
|
||||
id: string,
|
||||
prInfo: import("./types.js").PrInfo | null,
|
||||
): Promise<Task> {
|
||||
return this.withTaskLock(id, async () => {
|
||||
const dir = this.taskDir(id);
|
||||
const task = await this.readTaskJson(dir);
|
||||
|
||||
const prevPrNumber = task.prInfo?.number;
|
||||
const prevPrStatus = task.prInfo?.status;
|
||||
|
||||
if (prInfo) {
|
||||
task.prInfo = prInfo;
|
||||
task.log.push({
|
||||
timestamp: new Date().toISOString(),
|
||||
action: "PR linked",
|
||||
outcome: `PR #${prInfo.number}: ${prInfo.url}`,
|
||||
});
|
||||
} else {
|
||||
task.prInfo = undefined;
|
||||
if (prevPrNumber) {
|
||||
task.log.push({
|
||||
timestamp: new Date().toISOString(),
|
||||
action: "PR unlinked",
|
||||
outcome: `PR #${prevPrNumber} removed`,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
task.updatedAt = new Date().toISOString();
|
||||
|
||||
await this.atomicWriteTaskJson(dir, task);
|
||||
if (this.watcher) this.taskCache.set(id, { ...task });
|
||||
|
||||
// Only emit if PR info actually changed
|
||||
if (prevPrNumber !== prInfo?.number || prevPrStatus !== prInfo?.status) {
|
||||
this.emit("task:updated", task);
|
||||
}
|
||||
|
||||
return task;
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Read all historical agent log entries for a task from its agent log file.
|
||||
* Returns entries in chronological order (oldest first).
|
||||
|
||||
@@ -5,6 +5,20 @@ export type ThinkingLevel = (typeof THINKING_LEVELS)[number];
|
||||
export const COLUMNS = ["triage", "todo", "in-progress", "in-review", "done"] as const;
|
||||
export type Column = (typeof COLUMNS)[number];
|
||||
|
||||
export type PrStatus = "open" | "closed" | "merged";
|
||||
|
||||
export interface PrInfo {
|
||||
url: string;
|
||||
number: number;
|
||||
status: PrStatus;
|
||||
title: string;
|
||||
headBranch: string;
|
||||
baseBranch: string;
|
||||
commentCount: number;
|
||||
lastCommentAt?: string;
|
||||
lastCheckedAt?: string;
|
||||
}
|
||||
|
||||
export type StepStatus = "pending" | "in-progress" | "done" | "skipped";
|
||||
|
||||
export interface TaskStep {
|
||||
@@ -81,6 +95,8 @@ export interface Task {
|
||||
baseBranch?: string;
|
||||
attachments?: TaskAttachment[];
|
||||
steeringComments?: SteeringComment[];
|
||||
/** PR information for tasks linked to GitHub pull requests */
|
||||
prInfo?: PrInfo;
|
||||
log: TaskLogEntry[];
|
||||
size?: "S" | "M" | "L";
|
||||
reviewLevel?: number;
|
||||
@@ -141,6 +157,9 @@ export interface Settings {
|
||||
* Defaults to `"KB"`. Only affects new tasks — existing tasks retain
|
||||
* their original IDs. */
|
||||
taskPrefix?: string;
|
||||
/** Whether GitHub token is configured for PR operations (read-only, set by server).
|
||||
* When false, PR creation features are disabled in the UI. */
|
||||
githubTokenConfigured?: boolean;
|
||||
/** When true, merge commit messages include the task ID as the conventional
|
||||
* commit scope (e.g. `feat(KB-001): ...`). When false, the scope is
|
||||
* omitted (e.g. `feat: ...`). Default: true. */
|
||||
|
||||
@@ -32,6 +32,7 @@ function AppInner() {
|
||||
}
|
||||
return "board";
|
||||
});
|
||||
const [githubTokenConfigured, setGithubTokenConfigured] = useState(false);
|
||||
const { tasks, createTask, moveTask, deleteTask, mergeTask, retryTask } = useTasks();
|
||||
|
||||
useEffect(() => {
|
||||
@@ -43,6 +44,7 @@ function AppInner() {
|
||||
setAutoMerge(!!s.autoMerge);
|
||||
setGlobalPaused(!!s.globalPause);
|
||||
setEnginePaused(!!s.enginePaused);
|
||||
setGithubTokenConfigured(!!s.githubTokenConfigured);
|
||||
})
|
||||
.catch(() => {/* keep default */});
|
||||
fetchAuthStatus()
|
||||
@@ -167,6 +169,7 @@ function AppInner() {
|
||||
onMergeTask={mergeTask}
|
||||
onRetryTask={retryTask}
|
||||
addToast={addToast}
|
||||
githubTokenConfigured={githubTokenConfigured}
|
||||
/>
|
||||
)}
|
||||
{settingsOpen && (
|
||||
|
||||
@@ -205,3 +205,41 @@ export interface GitRemote {
|
||||
export function fetchGitRemotes(): Promise<GitRemote[]> {
|
||||
return api<GitRemote[]>("/git/remotes");
|
||||
}
|
||||
|
||||
// --- PR Management API ---
|
||||
|
||||
/** PR info returned by PR endpoints */
|
||||
export interface PrInfo {
|
||||
url: string;
|
||||
number: number;
|
||||
status: "open" | "closed" | "merged";
|
||||
title: string;
|
||||
headBranch: string;
|
||||
baseBranch: string;
|
||||
commentCount: number;
|
||||
lastCommentAt?: string;
|
||||
lastCheckedAt?: string;
|
||||
}
|
||||
|
||||
/** Create a GitHub PR for a task */
|
||||
export function createPr(
|
||||
id: string,
|
||||
params: { title: string; body?: string; base?: string }
|
||||
): Promise<PrInfo> {
|
||||
return api<PrInfo>(`/tasks/${id}/pr/create`, {
|
||||
method: "POST",
|
||||
body: JSON.stringify(params),
|
||||
});
|
||||
}
|
||||
|
||||
/** Fetch cached PR status for a task */
|
||||
export function fetchPrStatus(id: string): Promise<{ prInfo: PrInfo; stale: boolean }> {
|
||||
return api<{ prInfo: PrInfo; stale: boolean }>(`/tasks/${id}/pr/status`);
|
||||
}
|
||||
|
||||
/** Force refresh PR status from GitHub */
|
||||
export function refreshPrStatus(id: string): Promise<PrInfo> {
|
||||
return api<PrInfo>(`/tasks/${id}/pr/refresh`, {
|
||||
method: "POST",
|
||||
});
|
||||
}
|
||||
|
||||
227
packages/dashboard/app/components/PrSection.tsx
Normal file
227
packages/dashboard/app/components/PrSection.tsx
Normal file
@@ -0,0 +1,227 @@
|
||||
import { useState, useCallback } from "react";
|
||||
import { GitPullRequest, ExternalLink, RefreshCw, Plus, MessageSquare } from "lucide-react";
|
||||
import type { PrInfo } from "@kb/core";
|
||||
import { createPr, refreshPrStatus } from "../api";
|
||||
import type { ToastType } from "../hooks/useToast";
|
||||
|
||||
interface PrSectionProps {
|
||||
taskId: string;
|
||||
prInfo?: PrInfo;
|
||||
hasGitHubToken: boolean;
|
||||
onPrCreated: (prInfo: PrInfo) => void;
|
||||
onPrUpdated: (prInfo: PrInfo) => void;
|
||||
addToast: (message: string, type?: ToastType) => void;
|
||||
}
|
||||
|
||||
const STATUS_COLORS = {
|
||||
open: { bg: "rgba(63,185,80,0.15)", text: "#3fb950", icon: "🔵" },
|
||||
closed: { bg: "rgba(218,54,51,0.15)", text: "#da3633", icon: "⚪" },
|
||||
merged: { bg: "rgba(188,140,255,0.15)", text: "#bc8cff", icon: "🟣" },
|
||||
};
|
||||
|
||||
export function PrSection({
|
||||
taskId,
|
||||
prInfo,
|
||||
hasGitHubToken,
|
||||
onPrCreated,
|
||||
onPrUpdated,
|
||||
addToast,
|
||||
}: PrSectionProps) {
|
||||
const [showCreateForm, setShowCreateForm] = useState(false);
|
||||
const [prTitle, setPrTitle] = useState("");
|
||||
const [prBody, setPrBody] = useState("");
|
||||
const [isCreating, setIsCreating] = useState(false);
|
||||
const [isRefreshing, setIsRefreshing] = useState(false);
|
||||
|
||||
const handleCreate = useCallback(async () => {
|
||||
if (!prTitle.trim()) return;
|
||||
|
||||
setIsCreating(true);
|
||||
try {
|
||||
const newPr = await createPr(taskId, {
|
||||
title: prTitle.trim(),
|
||||
body: prBody.trim() || undefined,
|
||||
});
|
||||
onPrCreated(newPr);
|
||||
setShowCreateForm(false);
|
||||
setPrTitle("");
|
||||
setPrBody("");
|
||||
addToast(`Created PR #${newPr.number}`, "success");
|
||||
} catch (err: any) {
|
||||
addToast(err.message || "Failed to create PR", "error");
|
||||
} finally {
|
||||
setIsCreating(false);
|
||||
}
|
||||
}, [taskId, prTitle, prBody, onPrCreated, addToast]);
|
||||
|
||||
const handleRefresh = useCallback(async () => {
|
||||
if (!prInfo) return;
|
||||
|
||||
setIsRefreshing(true);
|
||||
try {
|
||||
const updated = await refreshPrStatus(taskId);
|
||||
onPrUpdated(updated);
|
||||
addToast("PR status refreshed", "success");
|
||||
} catch (err: any) {
|
||||
addToast(err.message || "Failed to refresh PR", "error");
|
||||
} finally {
|
||||
setIsRefreshing(false);
|
||||
}
|
||||
}, [taskId, prInfo, onPrUpdated, addToast]);
|
||||
|
||||
// No PR yet - show create button
|
||||
if (!prInfo) {
|
||||
if (showCreateForm) {
|
||||
return (
|
||||
<div className="pr-section">
|
||||
<h4>
|
||||
<GitPullRequest size={16} style={{ verticalAlign: "middle", marginRight: 8 }} />
|
||||
Create Pull Request
|
||||
</h4>
|
||||
<div className="pr-form">
|
||||
<input
|
||||
type="text"
|
||||
placeholder="PR title"
|
||||
value={prTitle}
|
||||
onChange={(e) => setPrTitle(e.target.value)}
|
||||
disabled={isCreating}
|
||||
className="pr-input"
|
||||
/>
|
||||
<textarea
|
||||
placeholder="PR description (optional)"
|
||||
value={prBody}
|
||||
onChange={(e) => setPrBody(e.target.value)}
|
||||
disabled={isCreating}
|
||||
className="pr-textarea"
|
||||
rows={3}
|
||||
/>
|
||||
<div className="pr-actions">
|
||||
<button
|
||||
className="btn btn-sm"
|
||||
onClick={() => setShowCreateForm(false)}
|
||||
disabled={isCreating}
|
||||
>
|
||||
Cancel
|
||||
</button>
|
||||
<button
|
||||
className="btn btn-primary btn-sm"
|
||||
onClick={handleCreate}
|
||||
disabled={!prTitle.trim() || isCreating}
|
||||
>
|
||||
{isCreating ? "Creating…" : "Create PR"}
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="pr-section">
|
||||
<h4>
|
||||
<GitPullRequest size={16} style={{ verticalAlign: "middle", marginRight: 8 }} />
|
||||
Pull Request
|
||||
</h4>
|
||||
<button
|
||||
className="btn btn-primary btn-sm"
|
||||
onClick={() => setShowCreateForm(true)}
|
||||
disabled={!hasGitHubToken}
|
||||
title={hasGitHubToken ? "Create a PR for this task" : "GitHub token not configured"}
|
||||
>
|
||||
<Plus size={14} style={{ verticalAlign: "middle", marginRight: 4 }} />
|
||||
Create PR
|
||||
</button>
|
||||
{!hasGitHubToken && (
|
||||
<div className="pr-hint" style={{ marginTop: 8, opacity: 0.7, fontSize: 12 }}>
|
||||
Set GITHUB_TOKEN env var to enable PR creation
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// PR exists - show PR card
|
||||
const statusStyle = STATUS_COLORS[prInfo.status];
|
||||
|
||||
return (
|
||||
<div className="pr-section">
|
||||
<h4>
|
||||
<GitPullRequest size={16} style={{ verticalAlign: "middle", marginRight: 8 }} />
|
||||
Pull Request
|
||||
</h4>
|
||||
<div
|
||||
className="pr-card"
|
||||
style={{
|
||||
border: "1px solid var(--border, #333)",
|
||||
borderRadius: 8,
|
||||
padding: 12,
|
||||
background: statusStyle.bg,
|
||||
}}
|
||||
>
|
||||
<div className="pr-header" style={{ display: "flex", alignItems: "center", gap: 8, marginBottom: 8 }}>
|
||||
<span style={{ fontSize: 16 }}>{statusStyle.icon}</span>
|
||||
<span
|
||||
className="pr-status-badge"
|
||||
style={{
|
||||
background: statusStyle.bg,
|
||||
color: statusStyle.text,
|
||||
padding: "2px 8px",
|
||||
borderRadius: 12,
|
||||
fontSize: 12,
|
||||
fontWeight: 500,
|
||||
textTransform: "capitalize",
|
||||
}}
|
||||
>
|
||||
{prInfo.status}
|
||||
</span>
|
||||
<span className="pr-number" style={{ fontSize: 14, opacity: 0.8 }}>
|
||||
#{prInfo.number}
|
||||
</span>
|
||||
<div style={{ flex: 1 }} />
|
||||
<button
|
||||
className="btn btn-sm"
|
||||
onClick={handleRefresh}
|
||||
disabled={isRefreshing}
|
||||
title="Refresh PR status"
|
||||
style={{ padding: "4px 8px" }}
|
||||
>
|
||||
<RefreshCw size={14} style={{ verticalAlign: "middle", opacity: isRefreshing ? 0.5 : 1 }} />
|
||||
</button>
|
||||
</div>
|
||||
<div className="pr-title" style={{ fontWeight: 500, marginBottom: 8 }}>
|
||||
{prInfo.title}
|
||||
</div>
|
||||
<div className="pr-meta" style={{ fontSize: 12, opacity: 0.7, marginBottom: 8 }}>
|
||||
<span>{prInfo.headBranch}</span>
|
||||
<span style={{ margin: "0 8px" }}>→</span>
|
||||
<span>{prInfo.baseBranch}</span>
|
||||
</div>
|
||||
<div className="pr-footer" style={{ display: "flex", alignItems: "center", gap: 12 }}>
|
||||
{prInfo.commentCount > 0 && (
|
||||
<span className="pr-comments" style={{ display: "flex", alignItems: "center", gap: 4 }}>
|
||||
<MessageSquare size={14} />
|
||||
{prInfo.commentCount}
|
||||
</span>
|
||||
)}
|
||||
<a
|
||||
href={prInfo.url}
|
||||
target="_blank"
|
||||
rel="noopener noreferrer"
|
||||
className="pr-link"
|
||||
style={{
|
||||
display: "flex",
|
||||
alignItems: "center",
|
||||
gap: 4,
|
||||
color: "var(--link, #58a6ff)",
|
||||
textDecoration: "none",
|
||||
fontSize: 12,
|
||||
}}
|
||||
>
|
||||
<ExternalLink size={14} />
|
||||
View on GitHub
|
||||
</a>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -1,5 +1,5 @@
|
||||
import { useCallback, useState } from "react";
|
||||
import { Link, Clock, Layers } from "lucide-react";
|
||||
import { Link, Clock, Layers, GitPullRequest } from "lucide-react";
|
||||
import type { Task, TaskDetail, Column } from "@kb/core";
|
||||
import { fetchTaskDetail, uploadAttachment } from "../api";
|
||||
import type { ToastType } from "../hooks/useToast";
|
||||
@@ -128,6 +128,34 @@ export function TaskCard({ task, queued, onOpenDetail, addToast, globalPaused }:
|
||||
{task.status}
|
||||
</span>
|
||||
)}
|
||||
{/* PR Status Indicator for in-review tasks */}
|
||||
{task.column === "in-review" && task.prInfo && (
|
||||
<span
|
||||
className="card-pr-badge"
|
||||
title={`PR #${task.prInfo.number}: ${task.prInfo.status}`}
|
||||
style={{
|
||||
background: task.prInfo.status === "merged"
|
||||
? "rgba(188,140,255,0.2)"
|
||||
: task.prInfo.status === "closed"
|
||||
? "rgba(139,148,158,0.2)"
|
||||
: "rgba(63,185,80,0.2)",
|
||||
color: task.prInfo.status === "merged"
|
||||
? "#bc8cff"
|
||||
: task.prInfo.status === "closed"
|
||||
? "#8b949e"
|
||||
: "#3fb950",
|
||||
fontSize: "11px",
|
||||
padding: "2px 6px",
|
||||
borderRadius: "10px",
|
||||
display: "flex",
|
||||
alignItems: "center",
|
||||
gap: "4px",
|
||||
}}
|
||||
>
|
||||
<GitPullRequest size={12} />
|
||||
#{task.prInfo.number}
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
<div className="card-title">
|
||||
{task.title || (task.description ? task.description.slice(0, 60) + (task.description.length > 60 ? "…" : "") : task.id)}
|
||||
|
||||
@@ -1,13 +1,14 @@
|
||||
import { useCallback, useEffect, useRef, useState } from "react";
|
||||
import ReactMarkdown from "react-markdown";
|
||||
import remarkGfm from "remark-gfm";
|
||||
import type { Task, TaskDetail, TaskAttachment, Column, MergeResult } from "@kb/core";
|
||||
import type { Task, TaskDetail, TaskAttachment, Column, MergeResult, PrInfo } from "@kb/core";
|
||||
import { COLUMN_LABELS, VALID_TRANSITIONS } from "@kb/core";
|
||||
import { uploadAttachment, deleteAttachment, updateTask, pauseTask, unpauseTask } from "../api";
|
||||
import type { ToastType } from "../hooks/useToast";
|
||||
import { useAgentLogs } from "../hooks/useAgentLogs";
|
||||
import { AgentLogViewer } from "./AgentLogViewer";
|
||||
import { SteeringTab } from "./SteeringTab";
|
||||
import { PrSection } from "./PrSection";
|
||||
|
||||
function getStepStatusColor(status: string): string {
|
||||
switch (status) {
|
||||
@@ -53,6 +54,7 @@ interface TaskDetailModalProps {
|
||||
onMergeTask: (id: string) => Promise<MergeResult>;
|
||||
onRetryTask?: (id: string) => Promise<Task>;
|
||||
addToast: (message: string, type?: ToastType) => void;
|
||||
githubTokenConfigured?: boolean;
|
||||
}
|
||||
|
||||
function truncate(s: string, max: number): string {
|
||||
@@ -68,6 +70,7 @@ export function TaskDetailModal({
|
||||
onMergeTask,
|
||||
onRetryTask,
|
||||
addToast,
|
||||
githubTokenConfigured,
|
||||
}: TaskDetailModalProps) {
|
||||
const [activeTab, setActiveTab] = useState<"definition" | "agent-log" | "steering">("definition");
|
||||
const [attachments, setAttachments] = useState<TaskAttachment[]>(task.attachments || []);
|
||||
@@ -517,6 +520,23 @@ export function TaskDetailModal({
|
||||
<div className="detail-log-empty">(no activity)</div>
|
||||
)}
|
||||
</div>
|
||||
{/* PR Section - only for in-review tasks */}
|
||||
{task.column === "in-review" && (
|
||||
<PrSection
|
||||
taskId={task.id}
|
||||
prInfo={task.prInfo}
|
||||
hasGitHubToken={githubTokenConfigured}
|
||||
onPrCreated={(prInfo) => {
|
||||
// Update task locally to show new PR
|
||||
(task as TaskDetail).prInfo = prInfo;
|
||||
addToast(`PR #${prInfo.number} created`, "success");
|
||||
}}
|
||||
onPrUpdated={(prInfo) => {
|
||||
(task as TaskDetail).prInfo = prInfo;
|
||||
}}
|
||||
addToast={addToast}
|
||||
/>
|
||||
)}
|
||||
</>
|
||||
)}
|
||||
</div>
|
||||
|
||||
267
packages/dashboard/app/components/__tests__/PrSection.test.tsx
Normal file
267
packages/dashboard/app/components/__tests__/PrSection.test.tsx
Normal file
@@ -0,0 +1,267 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { render, screen, fireEvent, waitFor } from "@testing-library/react";
|
||||
import { PrSection } from "../PrSection";
|
||||
|
||||
// Mock the API module
|
||||
vi.mock("../../api", () => ({
|
||||
createPr: vi.fn(),
|
||||
refreshPrStatus: vi.fn(),
|
||||
}));
|
||||
|
||||
import { createPr, refreshPrStatus } from "../../api";
|
||||
|
||||
const mockAddToast = vi.fn();
|
||||
const mockOnPrCreated = vi.fn();
|
||||
const mockOnPrUpdated = vi.fn();
|
||||
|
||||
const mockPrInfo = {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open" as const,
|
||||
title: "Fix the bug",
|
||||
headBranch: "kb/kb-001",
|
||||
baseBranch: "main",
|
||||
commentCount: 3,
|
||||
lastCommentAt: "2026-01-01T00:00:00.000Z",
|
||||
};
|
||||
|
||||
describe("PrSection", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe("when task has no PR", () => {
|
||||
it("shows create PR button when GitHub token is available", () => {
|
||||
render(
|
||||
<PrSection
|
||||
taskId="KB-001"
|
||||
hasGitHubToken={true}
|
||||
onPrCreated={mockOnPrCreated}
|
||||
onPrUpdated={mockOnPrUpdated}
|
||||
addToast={mockAddToast}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(screen.getByText("Create PR")).toBeDefined();
|
||||
});
|
||||
|
||||
it("shows disabled button and hint when GitHub token is missing", () => {
|
||||
render(
|
||||
<PrSection
|
||||
taskId="KB-001"
|
||||
hasGitHubToken={false}
|
||||
onPrCreated={mockOnPrCreated}
|
||||
onPrUpdated={mockOnPrUpdated}
|
||||
addToast={mockAddToast}
|
||||
/>
|
||||
);
|
||||
|
||||
const button = screen.getByText("Create PR") as HTMLButtonElement;
|
||||
expect(button.disabled).toBe(true);
|
||||
expect(screen.getByText(/GITHUB_TOKEN env var/i)).toBeDefined();
|
||||
});
|
||||
|
||||
it("shows create form when clicking create button", () => {
|
||||
render(
|
||||
<PrSection
|
||||
taskId="KB-001"
|
||||
hasGitHubToken={true}
|
||||
onPrCreated={mockOnPrCreated}
|
||||
onPrUpdated={mockOnPrUpdated}
|
||||
addToast={mockAddToast}
|
||||
/>
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByText("Create PR"));
|
||||
|
||||
expect(screen.getByPlaceholderText("PR title")).toBeDefined();
|
||||
expect(screen.getByPlaceholderText("PR description (optional)")).toBeDefined();
|
||||
expect(screen.getByText("Cancel")).toBeDefined();
|
||||
});
|
||||
|
||||
it("hides form when clicking cancel", () => {
|
||||
render(
|
||||
<PrSection
|
||||
taskId="KB-001"
|
||||
hasGitHubToken={true}
|
||||
onPrCreated={mockOnPrCreated}
|
||||
onPrUpdated={mockOnPrUpdated}
|
||||
addToast={mockAddToast}
|
||||
/>
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByText("Create PR"));
|
||||
fireEvent.click(screen.getByText("Cancel"));
|
||||
|
||||
expect(screen.queryByPlaceholderText("PR title")).toBeNull();
|
||||
});
|
||||
|
||||
it("creates PR when form is submitted", async () => {
|
||||
(createPr as ReturnType<typeof vi.fn>).mockResolvedValue(mockPrInfo);
|
||||
|
||||
render(
|
||||
<PrSection
|
||||
taskId="KB-001"
|
||||
hasGitHubToken={true}
|
||||
onPrCreated={mockOnPrCreated}
|
||||
onPrUpdated={mockOnPrUpdated}
|
||||
addToast={mockAddToast}
|
||||
/>
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByText("Create PR"));
|
||||
fireEvent.change(screen.getByPlaceholderText("PR title"), {
|
||||
target: { value: "My PR Title" },
|
||||
});
|
||||
fireEvent.click(screen.getByText("Create PR"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(createPr).toHaveBeenCalledWith("KB-001", {
|
||||
title: "My PR Title",
|
||||
body: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
expect(mockOnPrCreated).toHaveBeenCalledWith(mockPrInfo);
|
||||
expect(mockAddToast).toHaveBeenCalledWith("Created PR #42", "success");
|
||||
});
|
||||
|
||||
it("shows error when PR creation fails", async () => {
|
||||
(createPr as ReturnType<typeof vi.fn>).mockRejectedValue(new Error("API error"));
|
||||
|
||||
render(
|
||||
<PrSection
|
||||
taskId="KB-001"
|
||||
hasGitHubToken={true}
|
||||
onPrCreated={mockOnPrCreated}
|
||||
onPrUpdated={mockOnPrUpdated}
|
||||
addToast={mockAddToast}
|
||||
/>
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByText("Create PR"));
|
||||
fireEvent.change(screen.getByPlaceholderText("PR title"), {
|
||||
target: { value: "My PR Title" },
|
||||
});
|
||||
fireEvent.click(screen.getByText("Create PR"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockAddToast).toHaveBeenCalledWith("API error", "error");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("when task has a PR", () => {
|
||||
it("displays PR info for open PR", () => {
|
||||
render(
|
||||
<PrSection
|
||||
taskId="KB-001"
|
||||
prInfo={mockPrInfo}
|
||||
hasGitHubToken={true}
|
||||
onPrCreated={mockOnPrCreated}
|
||||
onPrUpdated={mockOnPrUpdated}
|
||||
addToast={mockAddToast}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(screen.getByText("#42")).toBeDefined();
|
||||
expect(screen.getByText("Fix the bug")).toBeDefined();
|
||||
expect(screen.getByText("open")).toBeDefined();
|
||||
expect(screen.getByText("View on GitHub")).toBeDefined();
|
||||
});
|
||||
|
||||
it("shows correct status badge for merged PR", () => {
|
||||
render(
|
||||
<PrSection
|
||||
taskId="KB-001"
|
||||
prInfo={{ ...mockPrInfo, status: "merged" }}
|
||||
hasGitHubToken={true}
|
||||
onPrCreated={mockOnPrCreated}
|
||||
onPrUpdated={mockOnPrUpdated}
|
||||
addToast={mockAddToast}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(screen.getByText("merged")).toBeDefined();
|
||||
});
|
||||
|
||||
it("shows correct status badge for closed PR", () => {
|
||||
render(
|
||||
<PrSection
|
||||
taskId="KB-001"
|
||||
prInfo={{ ...mockPrInfo, status: "closed" }}
|
||||
hasGitHubToken={true}
|
||||
onPrCreated={mockOnPrCreated}
|
||||
onPrUpdated={mockOnPrUpdated}
|
||||
addToast={mockAddToast}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(screen.getByText("closed")).toBeDefined();
|
||||
});
|
||||
|
||||
it("displays comment count when PR has comments", () => {
|
||||
render(
|
||||
<PrSection
|
||||
taskId="KB-001"
|
||||
prInfo={{ ...mockPrInfo, commentCount: 5 }}
|
||||
hasGitHubToken={true}
|
||||
onPrCreated={mockOnPrCreated}
|
||||
onPrUpdated={mockOnPrUpdated}
|
||||
addToast={mockAddToast}
|
||||
/>
|
||||
);
|
||||
|
||||
// The comment count should be rendered
|
||||
expect(screen.getByText("5")).toBeDefined();
|
||||
});
|
||||
|
||||
it("refreshes PR status when refresh button is clicked", async () => {
|
||||
const updatedPr = { ...mockPrInfo, status: "merged" as const };
|
||||
(refreshPrStatus as ReturnType<typeof vi.fn>).mockResolvedValue(updatedPr);
|
||||
|
||||
render(
|
||||
<PrSection
|
||||
taskId="KB-001"
|
||||
prInfo={mockPrInfo}
|
||||
hasGitHubToken={true}
|
||||
onPrCreated={mockOnPrCreated}
|
||||
onPrUpdated={mockOnPrUpdated}
|
||||
addToast={mockAddToast}
|
||||
/>
|
||||
);
|
||||
|
||||
const refreshButton = screen.getByTitle("Refresh PR status");
|
||||
fireEvent.click(refreshButton);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(refreshPrStatus).toHaveBeenCalledWith("KB-001");
|
||||
});
|
||||
|
||||
expect(mockOnPrUpdated).toHaveBeenCalledWith(updatedPr);
|
||||
expect(mockAddToast).toHaveBeenCalledWith("PR status refreshed", "success");
|
||||
});
|
||||
|
||||
it("shows error when refresh fails", async () => {
|
||||
(refreshPrStatus as ReturnType<typeof vi.fn>).mockRejectedValue(new Error("Network error"));
|
||||
|
||||
render(
|
||||
<PrSection
|
||||
taskId="KB-001"
|
||||
prInfo={mockPrInfo}
|
||||
hasGitHubToken={true}
|
||||
onPrCreated={mockOnPrCreated}
|
||||
onPrUpdated={mockOnPrUpdated}
|
||||
addToast={mockAddToast}
|
||||
/>
|
||||
);
|
||||
|
||||
const refreshButton = screen.getByTitle("Refresh PR status");
|
||||
fireEvent.click(refreshButton);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockAddToast).toHaveBeenCalledWith("Network error", "error");
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
259
packages/dashboard/src/github.ts
Normal file
259
packages/dashboard/src/github.ts
Normal file
@@ -0,0 +1,259 @@
|
||||
import { execFileSync } from "node:child_process";
|
||||
import type { PrInfo } from "@kb/core";
|
||||
|
||||
export interface CreatePrParams {
|
||||
owner: string;
|
||||
repo: string;
|
||||
title: string;
|
||||
body?: string;
|
||||
head: string;
|
||||
base?: string;
|
||||
}
|
||||
|
||||
export interface PrComment {
|
||||
id: number;
|
||||
body: string;
|
||||
user: { login: string };
|
||||
created_at: string;
|
||||
updated_at: string;
|
||||
html_url: string;
|
||||
}
|
||||
|
||||
export class GitHubClient {
|
||||
private token: string | undefined;
|
||||
private baseUrl = "https://api.github.com";
|
||||
|
||||
constructor(token?: string) {
|
||||
this.token = token;
|
||||
}
|
||||
|
||||
/**
|
||||
* Try to create a PR using the `gh` CLI if available, otherwise fall back
|
||||
* to the REST API. Returns the created PR info.
|
||||
*/
|
||||
async createPr(params: CreatePrParams): Promise<PrInfo> {
|
||||
// Try gh CLI first (preferred for auth handling)
|
||||
try {
|
||||
return this.createPrWithGh(params);
|
||||
} catch {
|
||||
// Fall back to REST API
|
||||
return this.createPrWithApi(params);
|
||||
}
|
||||
}
|
||||
|
||||
private createPrWithGh(params: CreatePrParams): PrInfo {
|
||||
const { owner, repo, title, body, head, base } = params;
|
||||
|
||||
// Build gh pr create command arguments (as array for safety)
|
||||
const args = [
|
||||
"pr", "create",
|
||||
"--repo", `${owner}/${repo}`,
|
||||
"--title", title,
|
||||
"--head", head,
|
||||
];
|
||||
|
||||
if (body) {
|
||||
args.push("--body", body);
|
||||
}
|
||||
if (base) {
|
||||
args.push("--base", base);
|
||||
}
|
||||
|
||||
// Execute gh command using execFileSync for proper argument handling
|
||||
const result = execFileSync("gh", args, {
|
||||
encoding: "utf-8",
|
||||
stdio: ["pipe", "pipe", "ignore"],
|
||||
});
|
||||
|
||||
// Extract PR URL from output (gh outputs the PR URL on success)
|
||||
const prUrl = result.trim();
|
||||
const match = prUrl.match(/\/pull\/(\d+)$/);
|
||||
if (!match) {
|
||||
throw new Error(`Failed to parse PR URL from gh output: ${prUrl}`);
|
||||
}
|
||||
|
||||
const number = parseInt(match[1], 10);
|
||||
|
||||
return {
|
||||
url: prUrl,
|
||||
number,
|
||||
status: "open",
|
||||
title,
|
||||
headBranch: head,
|
||||
baseBranch: base || "main",
|
||||
commentCount: 0,
|
||||
};
|
||||
}
|
||||
|
||||
private async createPrWithApi(params: CreatePrParams): Promise<PrInfo> {
|
||||
const { owner, repo, title, body, head, base = "main" } = params;
|
||||
|
||||
const url = `${this.baseUrl}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/pulls`;
|
||||
|
||||
const headers = this.buildHeaders();
|
||||
|
||||
const response = await fetch(url, {
|
||||
method: "POST",
|
||||
headers,
|
||||
body: JSON.stringify({
|
||||
title,
|
||||
body: body || "",
|
||||
head,
|
||||
base,
|
||||
}),
|
||||
});
|
||||
|
||||
if (!response.ok) {
|
||||
const error = await response.json().catch(() => ({ message: response.statusText }));
|
||||
throw new Error(`GitHub API error: ${response.status} ${error.message || response.statusText}`);
|
||||
}
|
||||
|
||||
const data = await response.json() as {
|
||||
number: number;
|
||||
html_url: string;
|
||||
title: string;
|
||||
state: string;
|
||||
head: { ref: string };
|
||||
base: { ref: string };
|
||||
comments: number;
|
||||
};
|
||||
|
||||
return {
|
||||
url: data.html_url,
|
||||
number: data.number,
|
||||
status: this.mapPrState(data.state),
|
||||
title: data.title,
|
||||
headBranch: data.head.ref,
|
||||
baseBranch: data.base.ref,
|
||||
commentCount: data.comments,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch current PR status from GitHub API.
|
||||
*/
|
||||
async getPrStatus(owner: string, repo: string, number: number): Promise<PrInfo> {
|
||||
const url = `${this.baseUrl}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/pulls/${number}`;
|
||||
|
||||
const headers = this.buildHeaders();
|
||||
|
||||
const response = await fetch(url, { headers });
|
||||
|
||||
if (!response.ok) {
|
||||
if (response.status === 404) {
|
||||
throw new Error(`PR #${number} not found in ${owner}/${repo}`);
|
||||
}
|
||||
const error = await response.json().catch(() => ({ message: response.statusText }));
|
||||
throw new Error(`GitHub API error: ${response.status} ${error.message || response.statusText}`);
|
||||
}
|
||||
|
||||
const data = await response.json() as {
|
||||
number: number;
|
||||
html_url: string;
|
||||
title: string;
|
||||
state: string;
|
||||
merged: boolean;
|
||||
head: { ref: string };
|
||||
base: { ref: string };
|
||||
comments: number;
|
||||
updated_at: string;
|
||||
};
|
||||
|
||||
return {
|
||||
url: data.html_url,
|
||||
number: data.number,
|
||||
status: data.merged ? "merged" : this.mapPrState(data.state),
|
||||
title: data.title,
|
||||
headBranch: data.head.ref,
|
||||
baseBranch: data.base.ref,
|
||||
commentCount: data.comments,
|
||||
lastCommentAt: data.updated_at,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* List PR comments since a specific timestamp.
|
||||
*/
|
||||
async listPrComments(
|
||||
owner: string,
|
||||
repo: string,
|
||||
number: number,
|
||||
since?: string,
|
||||
): Promise<PrComment[]> {
|
||||
const params = new URLSearchParams();
|
||||
params.append("per_page", "100");
|
||||
if (since) {
|
||||
params.append("since", since);
|
||||
}
|
||||
|
||||
const url = `${this.baseUrl}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/issues/${number}/comments?${params}`;
|
||||
|
||||
const headers = this.buildHeaders();
|
||||
|
||||
const response = await fetch(url, { headers });
|
||||
|
||||
if (!response.ok) {
|
||||
if (response.status === 404) {
|
||||
return []; // PR might not exist or have no comments
|
||||
}
|
||||
const error = await response.json().catch(() => ({ message: response.statusText }));
|
||||
throw new Error(`GitHub API error: ${response.status} ${error.message || response.statusText}`);
|
||||
}
|
||||
|
||||
return response.json() as Promise<PrComment[]>;
|
||||
}
|
||||
|
||||
private buildHeaders(): Record<string, string> {
|
||||
const headers: Record<string, string> = {
|
||||
Accept: "application/vnd.github+json",
|
||||
"X-GitHub-Api-Version": "2022-11-28",
|
||||
"User-Agent": "kb-dashboard/1.0",
|
||||
};
|
||||
|
||||
if (this.token) {
|
||||
headers.Authorization = `Bearer ${this.token}`;
|
||||
}
|
||||
|
||||
return headers;
|
||||
}
|
||||
|
||||
private mapPrState(state: string): "open" | "closed" {
|
||||
return state === "open" ? "open" : "closed";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract owner/repo from a GitHub remote URL or return null if not a GitHub remote.
|
||||
*/
|
||||
export function parseGitHubRemote(remoteUrl: string): { owner: string; repo: string } | null {
|
||||
// Handle HTTPS: https://github.com/owner/repo.git or https://github.com/owner/repo
|
||||
const httpsMatch = remoteUrl.match(/github\.com\/([^\/]+)\/([^\/\.]+)(?:\.git)?$/);
|
||||
if (httpsMatch) {
|
||||
return { owner: httpsMatch[1], repo: httpsMatch[2] };
|
||||
}
|
||||
|
||||
// Handle SSH: git@github.com:owner/repo.git or git@github.com:owner/repo
|
||||
const sshMatch = remoteUrl.match(/github\.com:([^\/]+)\/([^\/\.]+)(?:\.git)?$/);
|
||||
if (sshMatch) {
|
||||
return { owner: sshMatch[1], repo: sshMatch[2] };
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the current GitHub remote owner/repo from the git config.
|
||||
*/
|
||||
export function getCurrentGitHubRepo(cwd: string): { owner: string; repo: string } | null {
|
||||
try {
|
||||
const remoteUrl = execFileSync("git", ["remote", "get-url", "origin"], {
|
||||
cwd,
|
||||
encoding: "utf-8",
|
||||
stdio: ["pipe", "pipe", "ignore"],
|
||||
}).trim();
|
||||
|
||||
return parseGitHubRemote(remoteUrl);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
@@ -763,6 +763,343 @@ describe("Pause/Unpause endpoints", () => {
|
||||
expect(res.body.error).toBe("Database error");
|
||||
});
|
||||
});
|
||||
|
||||
// --- PR Management route tests ---
|
||||
|
||||
describe("POST /tasks/:id/pr/create", () => {
|
||||
let store: TaskStore;
|
||||
|
||||
beforeEach(() => {
|
||||
store = createMockStore({
|
||||
getTask: vi.fn(),
|
||||
updatePrInfo: vi.fn(),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
getRootDir: vi.fn().mockReturnValue("/fake/root"),
|
||||
});
|
||||
});
|
||||
|
||||
function buildApp() {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(store));
|
||||
return app;
|
||||
}
|
||||
|
||||
const mockPrInfo = {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open" as const,
|
||||
title: "Test PR",
|
||||
headBranch: "kb/kb-001",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
};
|
||||
|
||||
const mockInReviewTask = {
|
||||
...FAKE_TASK_DETAIL,
|
||||
column: "in-review" as const,
|
||||
prInfo: undefined,
|
||||
};
|
||||
|
||||
it("returns 400 if task is not in in-review column", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...FAKE_TASK_DETAIL,
|
||||
column: "in-progress",
|
||||
});
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/tasks/KB-001/pr/create",
|
||||
JSON.stringify({ title: "Test PR" }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("in-review");
|
||||
});
|
||||
|
||||
it("returns 409 if task already has a PR", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...FAKE_TASK_DETAIL,
|
||||
column: "in-review",
|
||||
prInfo: mockPrInfo,
|
||||
});
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/tasks/KB-001/pr/create",
|
||||
JSON.stringify({ title: "Test PR" }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(409);
|
||||
expect(res.body.error).toContain("already has PR");
|
||||
});
|
||||
|
||||
it("returns 400 if title is missing", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(mockInReviewTask);
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/tasks/KB-001/pr/create",
|
||||
JSON.stringify({}),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("title is required");
|
||||
});
|
||||
|
||||
it("returns 429 when rate limit exceeded", { timeout: 15000 }, async () => {
|
||||
// Set up GITHUB_REPOSITORY env to bypass git lookup
|
||||
const originalEnv = process.env.GITHUB_REPOSITORY;
|
||||
process.env.GITHUB_REPOSITORY = "owner/rate-test";
|
||||
|
||||
// Create a fresh store mock for this test to isolate rate limit state
|
||||
const freshStore = createMockStore({
|
||||
getTask: vi.fn(),
|
||||
updatePrInfo: vi.fn(),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
getRootDir: vi.fn().mockReturnValue("/fake/root"),
|
||||
});
|
||||
|
||||
function buildFreshApp() {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(freshStore));
|
||||
return app;
|
||||
}
|
||||
|
||||
// Make 60 requests to hit the rate limit
|
||||
const app = buildFreshApp();
|
||||
for (let i = 0; i < 60; i++) {
|
||||
(freshStore.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...mockInReviewTask,
|
||||
id: `KB-RATE-${i}`,
|
||||
});
|
||||
await REQUEST(
|
||||
app,
|
||||
"POST",
|
||||
`/api/tasks/KB-RATE-${i}/pr/create`,
|
||||
JSON.stringify({ title: `Test PR ${i}` }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
}
|
||||
|
||||
// 61st request should be rate limited
|
||||
(freshStore.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...mockInReviewTask,
|
||||
id: "KB-RATE-61",
|
||||
});
|
||||
|
||||
const res = await REQUEST(
|
||||
app,
|
||||
"POST",
|
||||
"/api/tasks/KB-RATE-61/pr/create",
|
||||
JSON.stringify({ title: "Test PR 61" }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(429);
|
||||
expect(res.body.error).toContain("rate limit exceeded");
|
||||
expect(res.body.resetAt).toBeDefined();
|
||||
|
||||
// Restore env
|
||||
if (originalEnv) {
|
||||
process.env.GITHUB_REPOSITORY = originalEnv;
|
||||
} else {
|
||||
delete process.env.GITHUB_REPOSITORY;
|
||||
}
|
||||
});
|
||||
|
||||
it("returns 404 for non-existent task", async () => {
|
||||
// Create error with proper ENOENT code
|
||||
const error = new Error("ENOENT: task not found") as NodeJS.ErrnoException;
|
||||
error.code = "ENOENT";
|
||||
error.errno = -2;
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockRejectedValue(error);
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/tasks/KB-999/pr/create",
|
||||
JSON.stringify({ title: "Test PR" }),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body.error).toContain("not found");
|
||||
});
|
||||
});
|
||||
|
||||
describe("GET /tasks/:id/pr/status", () => {
|
||||
let store: TaskStore;
|
||||
|
||||
beforeEach(() => {
|
||||
store = createMockStore({
|
||||
getTask: vi.fn(),
|
||||
getRootDir: vi.fn().mockReturnValue("/fake/root"),
|
||||
});
|
||||
});
|
||||
|
||||
function buildApp() {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(store));
|
||||
return app;
|
||||
}
|
||||
|
||||
const mockPrInfo = {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open" as const,
|
||||
title: "Test PR",
|
||||
headBranch: "kb/kb-001",
|
||||
baseBranch: "main",
|
||||
commentCount: 3,
|
||||
};
|
||||
|
||||
it("returns cached PR info when available", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...FAKE_TASK_DETAIL,
|
||||
prInfo: mockPrInfo,
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
const res = await GET(buildApp(), "/api/tasks/KB-001/pr/status");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.prInfo).toEqual(mockPrInfo);
|
||||
expect(res.body.stale).toBe(false);
|
||||
});
|
||||
|
||||
it("returns 404 when task has no PR", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(FAKE_TASK_DETAIL);
|
||||
|
||||
const res = await GET(buildApp(), "/api/tasks/KB-001/pr/status");
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body.error).toContain("no associated PR");
|
||||
});
|
||||
|
||||
it("returns 404 for non-existent task", async () => {
|
||||
const error = new Error("Task not found") as Error & { code?: string };
|
||||
error.code = "ENOENT";
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockRejectedValue(error);
|
||||
|
||||
const res = await GET(buildApp(), "/api/tasks/KB-999/pr/status");
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
|
||||
it("marks data as stale when older than 5 minutes", async () => {
|
||||
const oldDate = new Date(Date.now() - 6 * 60 * 1000).toISOString(); // 6 minutes ago
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...FAKE_TASK_DETAIL,
|
||||
prInfo: mockPrInfo,
|
||||
updatedAt: oldDate,
|
||||
});
|
||||
|
||||
const res = await GET(buildApp(), "/api/tasks/KB-001/pr/status");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.stale).toBe(true);
|
||||
});
|
||||
|
||||
it("uses lastCheckedAt for staleness check when available", async () => {
|
||||
const recentUpdate = new Date().toISOString();
|
||||
const oldCheck = new Date(Date.now() - 6 * 60 * 1000).toISOString(); // 6 minutes ago
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...FAKE_TASK_DETAIL,
|
||||
prInfo: { ...mockPrInfo, lastCheckedAt: oldCheck },
|
||||
updatedAt: recentUpdate,
|
||||
});
|
||||
|
||||
const res = await GET(buildApp(), "/api/tasks/KB-001/pr/status");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
// Should be stale because lastCheckedAt is old, even though updatedAt is recent
|
||||
expect(res.body.stale).toBe(true);
|
||||
});
|
||||
|
||||
it("marks data as fresh when lastCheckedAt is recent", async () => {
|
||||
const recentCheck = new Date(Date.now() - 2 * 60 * 1000).toISOString(); // 2 minutes ago
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...FAKE_TASK_DETAIL,
|
||||
prInfo: { ...mockPrInfo, lastCheckedAt: recentCheck },
|
||||
updatedAt: new Date(Date.now() - 10 * 60 * 1000).toISOString(), // 10 minutes ago
|
||||
});
|
||||
|
||||
const res = await GET(buildApp(), "/api/tasks/KB-001/pr/status");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
// Should be fresh because lastCheckedAt is recent, even though updatedAt is old
|
||||
expect(res.body.stale).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("POST /tasks/:id/pr/refresh", () => {
|
||||
let store: TaskStore;
|
||||
|
||||
beforeEach(() => {
|
||||
store = createMockStore({
|
||||
getTask: vi.fn(),
|
||||
updatePrInfo: vi.fn(),
|
||||
getRootDir: vi.fn().mockReturnValue("/fake/root"),
|
||||
});
|
||||
});
|
||||
|
||||
function buildApp() {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(store));
|
||||
return app;
|
||||
}
|
||||
|
||||
const mockPrInfo = {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open" as const,
|
||||
title: "Test PR",
|
||||
headBranch: "kb/kb-001",
|
||||
baseBranch: "main",
|
||||
commentCount: 3,
|
||||
};
|
||||
|
||||
it("returns 404 when task has no PR", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(FAKE_TASK_DETAIL);
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/tasks/KB-001/pr/refresh",
|
||||
JSON.stringify({}),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body.error).toContain("no associated PR");
|
||||
});
|
||||
|
||||
it("returns 404 for non-existent task", async () => {
|
||||
const error = new Error("Task not found") as Error & { code?: string };
|
||||
error.code = "ENOENT";
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockRejectedValue(error);
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/tasks/KB-999/pr/refresh",
|
||||
JSON.stringify({}),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// --- GitHub Import route tests ---
|
||||
|
||||
@@ -3,8 +3,9 @@ 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 } from "@kb/core";
|
||||
import { COLUMNS, type PrInfo } from "@kb/core";
|
||||
import type { ServerOptions } from "./server.js";
|
||||
import { GitHubClient, getCurrentGitHubRepo } from "./github.js";
|
||||
|
||||
/**
|
||||
* Minimal interface matching pi-coding-agent's ModelRegistry API surface
|
||||
@@ -115,8 +116,46 @@ function getGitHubRemotes(): GitRemote[] {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Per-repo GitHub API rate limiter.
|
||||
* Tracks requests per repo and enforces 60 requests per hour per repo.
|
||||
*/
|
||||
class GitHubRateLimiter {
|
||||
private requests = new Map<string, number[]>();
|
||||
private readonly maxRequests = 60;
|
||||
private readonly windowMs = 60 * 60 * 1000; // 1 hour
|
||||
|
||||
canMakeRequest(repo: string): boolean {
|
||||
const now = Date.now();
|
||||
const timestamps = this.requests.get(repo) || [];
|
||||
|
||||
// Remove timestamps outside the window
|
||||
const validTimestamps = timestamps.filter((ts) => now - ts < this.windowMs);
|
||||
|
||||
if (validTimestamps.length >= this.maxRequests) {
|
||||
return false;
|
||||
}
|
||||
|
||||
validTimestamps.push(now);
|
||||
this.requests.set(repo, validTimestamps);
|
||||
return true;
|
||||
}
|
||||
|
||||
getResetTime(repo: string): Date | null {
|
||||
const timestamps = this.requests.get(repo);
|
||||
if (!timestamps || timestamps.length === 0) return null;
|
||||
|
||||
const oldest = Math.min(...timestamps);
|
||||
return new Date(oldest + this.windowMs);
|
||||
}
|
||||
}
|
||||
|
||||
export function createApiRoutes(store: TaskStore, options?: ServerOptions): Router {
|
||||
const router = Router();
|
||||
const ghRateLimiter = new GitHubRateLimiter();
|
||||
|
||||
// Get GitHub token from options or env
|
||||
const githubToken = options?.githubToken ?? process.env.GITHUB_TOKEN;
|
||||
|
||||
// Scheduler config (includes persisted settings)
|
||||
router.get("/config", async (_req, res) => {
|
||||
@@ -135,7 +174,11 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
router.get("/settings", async (_req, res) => {
|
||||
try {
|
||||
const settings = await store.getSettings();
|
||||
res.json(settings);
|
||||
// Inject server-side configuration flags
|
||||
res.json({
|
||||
...settings,
|
||||
githubTokenConfigured: Boolean(githubToken),
|
||||
});
|
||||
} catch (err: any) {
|
||||
res.status(500).json({ error: err.message });
|
||||
}
|
||||
@@ -592,9 +635,238 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
// ---------- Auth routes ----------
|
||||
registerAuthRoutes(router, options?.authStorage);
|
||||
|
||||
// ── PR Management Routes ─────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* POST /api/tasks/:id/pr/create
|
||||
* Create a GitHub PR for an in-review task.
|
||||
* Body: { title: string, body?: string, base?: string }
|
||||
* Returns: Created PrInfo
|
||||
*/
|
||||
router.post("/tasks/:id/pr/create", async (req, res) => {
|
||||
try {
|
||||
const { title, body, base } = req.body;
|
||||
|
||||
if (!title || typeof title !== "string") {
|
||||
res.status(400).json({ error: "title is required and must be a string" });
|
||||
return;
|
||||
}
|
||||
|
||||
// Get task and validate
|
||||
const task = await store.getTask(req.params.id);
|
||||
if (task.column !== "in-review") {
|
||||
res.status(400).json({ error: "Task must be in 'in-review' column to create a PR" });
|
||||
return;
|
||||
}
|
||||
|
||||
if (task.prInfo) {
|
||||
res.status(409).json({ error: `Task already has PR #${task.prInfo.number}: ${task.prInfo.url}` });
|
||||
return;
|
||||
}
|
||||
|
||||
// Determine branch name from task
|
||||
const branchName = `kb/${task.id.toLowerCase()}`;
|
||||
|
||||
// Get owner/repo from git remote or GITHUB_REPOSITORY env
|
||||
let owner: string;
|
||||
let repo: string;
|
||||
|
||||
const envRepo = process.env.GITHUB_REPOSITORY;
|
||||
if (envRepo) {
|
||||
const [o, r] = envRepo.split("/");
|
||||
owner = o;
|
||||
repo = r;
|
||||
} else {
|
||||
const gitRepo = getCurrentGitHubRepo(store.getRootDir());
|
||||
if (!gitRepo) {
|
||||
res.status(400).json({ error: "Could not determine GitHub repository. Set GITHUB_REPOSITORY env var or configure git remote." });
|
||||
return;
|
||||
}
|
||||
owner = gitRepo.owner;
|
||||
repo = gitRepo.repo;
|
||||
}
|
||||
|
||||
// Check rate limit
|
||||
const repoKey = `${owner}/${repo}`;
|
||||
if (!ghRateLimiter.canMakeRequest(repoKey)) {
|
||||
const resetTime = ghRateLimiter.getResetTime(repoKey);
|
||||
res.status(429).json({
|
||||
error: "GitHub API rate limit exceeded for this repository",
|
||||
resetAt: resetTime?.toISOString(),
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
// Create the PR
|
||||
const client = new GitHubClient(githubToken);
|
||||
|
||||
const prInfo = await client.createPr({
|
||||
owner,
|
||||
repo,
|
||||
title,
|
||||
body,
|
||||
head: branchName,
|
||||
base,
|
||||
});
|
||||
|
||||
// Store PR info
|
||||
await store.updatePrInfo(task.id, prInfo);
|
||||
await store.logEntry(task.id, "Created PR", `PR #${prInfo.number}: ${prInfo.url}`);
|
||||
|
||||
res.status(201).json(prInfo);
|
||||
} catch (err: any) {
|
||||
if (err.code === "ENOENT") {
|
||||
res.status(404).json({ error: `Task ${req.params.id} not found` });
|
||||
} else if (err.message?.includes("already exists")) {
|
||||
res.status(409).json({ error: err.message });
|
||||
} else if (err.message?.includes("No commits between")) {
|
||||
res.status(400).json({ error: "Branch has no commits. Push changes before creating PR." });
|
||||
} else {
|
||||
res.status(500).json({ error: err.message || "Failed to create PR" });
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* GET /api/tasks/:id/pr/status
|
||||
* Get cached PR status for a task. Triggers background refresh if stale (>5 min).
|
||||
*/
|
||||
router.get("/tasks/:id/pr/status", async (req, res) => {
|
||||
try {
|
||||
const task = await store.getTask(req.params.id);
|
||||
|
||||
if (!task.prInfo) {
|
||||
res.status(404).json({ error: "Task has no associated PR" });
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if data is stale (>5 minutes since last check)
|
||||
const fiveMinutesMs = 5 * 60 * 1000;
|
||||
const lastChecked = task.prInfo.lastCheckedAt || task.updatedAt;
|
||||
const lastCheckedTime = new Date(lastChecked).getTime();
|
||||
const isStale = Date.now() - lastCheckedTime > fiveMinutesMs;
|
||||
|
||||
// Return cached data immediately
|
||||
res.json({
|
||||
prInfo: task.prInfo,
|
||||
stale: isStale,
|
||||
});
|
||||
|
||||
// Trigger background refresh if stale (don't await, let it run)
|
||||
if (isStale) {
|
||||
refreshPrInBackground(store, task.id, task.prInfo, githubToken);
|
||||
}
|
||||
} catch (err: any) {
|
||||
if (err.code === "ENOENT") {
|
||||
res.status(404).json({ error: `Task ${req.params.id} not found` });
|
||||
} else {
|
||||
res.status(500).json({ error: err.message });
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* POST /api/tasks/:id/pr/refresh
|
||||
* Force refresh PR status from GitHub API.
|
||||
* Returns: Updated PrInfo
|
||||
*/
|
||||
router.post("/tasks/:id/pr/refresh", async (req, res) => {
|
||||
try {
|
||||
const task = await store.getTask(req.params.id);
|
||||
|
||||
if (!task.prInfo) {
|
||||
res.status(404).json({ error: "Task has no associated PR" });
|
||||
return;
|
||||
}
|
||||
|
||||
// Get owner/repo from git remote or GITHUB_REPOSITORY env
|
||||
let owner: string;
|
||||
let repo: string;
|
||||
|
||||
const envRepo = process.env.GITHUB_REPOSITORY;
|
||||
if (envRepo) {
|
||||
const [o, r] = envRepo.split("/");
|
||||
owner = o;
|
||||
repo = r;
|
||||
} else {
|
||||
const gitRepo = getCurrentGitHubRepo(store.getRootDir());
|
||||
if (!gitRepo) {
|
||||
res.status(400).json({ error: "Could not determine GitHub repository" });
|
||||
return;
|
||||
}
|
||||
owner = gitRepo.owner;
|
||||
repo = gitRepo.repo;
|
||||
}
|
||||
|
||||
// Check rate limit
|
||||
const repoKey = `${owner}/${repo}`;
|
||||
if (!ghRateLimiter.canMakeRequest(repoKey)) {
|
||||
const resetTime = ghRateLimiter.getResetTime(repoKey);
|
||||
res.status(429).json({
|
||||
error: "GitHub API rate limit exceeded for this repository",
|
||||
resetAt: resetTime?.toISOString(),
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
// Fetch fresh PR status
|
||||
const client = new GitHubClient(githubToken);
|
||||
|
||||
const prInfo = await client.getPrStatus(owner, repo, task.prInfo.number);
|
||||
|
||||
// Add lastCheckedAt timestamp
|
||||
prInfo.lastCheckedAt = new Date().toISOString();
|
||||
|
||||
// Update stored PR info
|
||||
await store.updatePrInfo(task.id, prInfo);
|
||||
|
||||
res.json(prInfo);
|
||||
} catch (err: any) {
|
||||
if (err.code === "ENOENT") {
|
||||
res.status(404).json({ error: `Task ${req.params.id} not found` });
|
||||
} else if (err.message?.includes("not found")) {
|
||||
res.status(404).json({ error: err.message });
|
||||
} else {
|
||||
res.status(500).json({ error: err.message });
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
return router;
|
||||
}
|
||||
|
||||
/**
|
||||
* Background PR refresh - updates PR status without blocking the response.
|
||||
* Silently logs errors without affecting the user experience.
|
||||
*/
|
||||
async function refreshPrInBackground(store: TaskStore, taskId: string, currentPrInfo: PrInfo, token?: string): Promise<void> {
|
||||
try {
|
||||
// Get owner/repo from git remote or GITHUB_REPOSITORY env
|
||||
let owner: string;
|
||||
let repo: string;
|
||||
|
||||
const envRepo = process.env.GITHUB_REPOSITORY;
|
||||
if (envRepo) {
|
||||
const [o, r] = envRepo.split("/");
|
||||
owner = o;
|
||||
repo = r;
|
||||
} else {
|
||||
const gitRepo = getCurrentGitHubRepo(store.getRootDir());
|
||||
if (!gitRepo) return; // Silent fail - can't determine repo
|
||||
owner = gitRepo.owner;
|
||||
repo = gitRepo.repo;
|
||||
}
|
||||
|
||||
const client = new GitHubClient(token);
|
||||
|
||||
const prInfo = await client.getPrStatus(owner, repo, currentPrInfo.number);
|
||||
prInfo.lastCheckedAt = new Date().toISOString();
|
||||
await store.updatePrInfo(taskId, prInfo);
|
||||
} catch {
|
||||
// Silent fail - background refresh is best-effort
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Register the GET /api/models route.
|
||||
* Returns available AI models from the ModelRegistry for the UI model selector.
|
||||
|
||||
@@ -15,6 +15,8 @@ export interface ServerOptions {
|
||||
onMerge?: (taskId: string) => Promise<MergeResult>;
|
||||
/** Maximum concurrent worktrees / execution slots (default 2) */
|
||||
maxConcurrent?: number;
|
||||
/** Optional GitHub token for PR operations — falls back to GITHUB_TOKEN env var */
|
||||
githubToken?: string;
|
||||
/** Optional AuthStorage instance for auth routes — if not provided, one is created internally */
|
||||
authStorage?: AuthStorageLike;
|
||||
/** Optional ModelRegistry instance for the models API — if not provided, the endpoint returns an empty list */
|
||||
|
||||
37
packages/engine/src/github.ts
Normal file
37
packages/engine/src/github.ts
Normal file
@@ -0,0 +1,37 @@
|
||||
import { execFileSync } from "node:child_process";
|
||||
|
||||
/**
|
||||
* Extract owner/repo from a GitHub remote URL or return null if not a GitHub remote.
|
||||
*/
|
||||
export function parseGitHubRemote(remoteUrl: string): { owner: string; repo: string } | null {
|
||||
// Handle HTTPS: https://github.com/owner/repo.git or https://github.com/owner/repo
|
||||
const httpsMatch = remoteUrl.match(/github\.com\/([^\/]+)\/([^\/\.]+)(?:\.git)?$/);
|
||||
if (httpsMatch) {
|
||||
return { owner: httpsMatch[1], repo: httpsMatch[2] };
|
||||
}
|
||||
|
||||
// Handle SSH: git@github.com:owner/repo.git or git@github.com:owner/repo
|
||||
const sshMatch = remoteUrl.match(/github\.com:([^\/]+)\/([^\/\.]+)(?:\.git)?$/);
|
||||
if (sshMatch) {
|
||||
return { owner: sshMatch[1], repo: sshMatch[2] };
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the current GitHub remote owner/repo from the git config.
|
||||
*/
|
||||
export function getCurrentGitHubRepo(cwd: string): { owner: string; repo: string } | null {
|
||||
try {
|
||||
const remoteUrl = execFileSync("git", ["remote", "get-url", "origin"], {
|
||||
cwd,
|
||||
encoding: "utf-8",
|
||||
stdio: ["pipe", "pipe", "ignore"],
|
||||
}).trim();
|
||||
|
||||
return parseGitHubRemote(remoteUrl);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
@@ -9,3 +9,5 @@ export { createKbAgent, type AgentOptions, type AgentResult } from "./pi.js";
|
||||
export { WorktreePool, scanIdleWorktrees, cleanupOrphanedWorktrees } from "./worktree-pool.js";
|
||||
export { createLogger, type Logger } from "./logger.js";
|
||||
export { isUsageLimitError, UsageLimitPauser } from "./usage-limit-detector.js";
|
||||
export { PrMonitor, type PrComment, type TrackedPr, type OnNewCommentsCallback } from "./pr-monitor.js";
|
||||
export { PrCommentHandler } from "./pr-comment-handler.js";
|
||||
|
||||
@@ -61,3 +61,6 @@ export const worktreePoolLog = createLogger("worktree-pool");
|
||||
|
||||
/** Logger for the review subsystem. */
|
||||
export const reviewerLog = createLogger("reviewer");
|
||||
|
||||
/** Logger for the PR monitor subsystem. */
|
||||
export const prMonitorLog = createLogger("pr-monitor");
|
||||
|
||||
237
packages/engine/src/pr-comment-handler.test.ts
Normal file
237
packages/engine/src/pr-comment-handler.test.ts
Normal file
@@ -0,0 +1,237 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { PrCommentHandler } from "./pr-comment-handler.js";
|
||||
import type { TaskStore } from "@kb/core";
|
||||
|
||||
const mockStore = {
|
||||
addSteeringComment: vi.fn(),
|
||||
createTask: vi.fn(),
|
||||
} as unknown as TaskStore;
|
||||
|
||||
describe("PrCommentHandler", () => {
|
||||
let handler: PrCommentHandler;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
handler = new PrCommentHandler(mockStore);
|
||||
});
|
||||
|
||||
const mockPrInfo = {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open" as const,
|
||||
title: "Test PR",
|
||||
headBranch: "kb/kb-001",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
};
|
||||
|
||||
describe("isNonActionable", () => {
|
||||
it.each([
|
||||
"LGTM",
|
||||
"lgtm",
|
||||
"Looks good",
|
||||
"Looks good to me",
|
||||
"Thanks",
|
||||
"Thank you",
|
||||
"Nice",
|
||||
"Great work",
|
||||
"👍",
|
||||
"✅",
|
||||
])("filters out non-actionable comment: %s", async (body) => {
|
||||
await handler.handleNewComments("KB-001", mockPrInfo, [
|
||||
{
|
||||
id: 1,
|
||||
body,
|
||||
user: { login: "reviewer" },
|
||||
created_at: new Date().toISOString(),
|
||||
updated_at: new Date().toISOString(),
|
||||
html_url: "https://github.com/owner/repo/pull/42#issuecomment-1",
|
||||
},
|
||||
]);
|
||||
|
||||
expect(mockStore.addSteeringComment).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("isActionable", () => {
|
||||
it.each([
|
||||
{ body: "Please fix the indentation", keyword: "fix" },
|
||||
{ body: "Should change the variable name", keyword: "change" },
|
||||
{ body: "Update the documentation", keyword: "update" },
|
||||
{ body: "Remove the unused import", keyword: "remove" },
|
||||
{ body: "Add error handling", keyword: "add" },
|
||||
{ body: "You should refactor this", keyword: "should" },
|
||||
{ body: "Needs to handle edge cases", keyword: "needs to" },
|
||||
{ body: "Consider using a different approach", keyword: "consider" },
|
||||
{ body: "I suggest renaming this", keyword: "suggest" },
|
||||
{ body: "Recommend adding tests", keyword: "recommend" },
|
||||
])("creates steering comment for actionable feedback containing '$keyword': $body", async ({ body }) => {
|
||||
await handler.handleNewComments("KB-001", mockPrInfo, [
|
||||
{
|
||||
id: 1,
|
||||
body,
|
||||
user: { login: "reviewer" },
|
||||
created_at: new Date().toISOString(),
|
||||
updated_at: new Date().toISOString(),
|
||||
html_url: "https://github.com/owner/repo/pull/42#issuecomment-1",
|
||||
},
|
||||
]);
|
||||
|
||||
expect(mockStore.addSteeringComment).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("code suggestions", () => {
|
||||
it("creates steering comment for comments with code blocks", async () => {
|
||||
await handler.handleNewComments("KB-001", mockPrInfo, [
|
||||
{
|
||||
id: 1,
|
||||
body: "```typescript\nconst x = 1;\n```",
|
||||
user: { login: "reviewer" },
|
||||
created_at: new Date().toISOString(),
|
||||
updated_at: new Date().toISOString(),
|
||||
html_url: "https://github.com/owner/repo/pull/42#issuecomment-1",
|
||||
},
|
||||
]);
|
||||
|
||||
expect(mockStore.addSteeringComment).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("creates steering comment for inline code suggestions", async () => {
|
||||
await handler.handleNewComments("KB-001", mockPrInfo, [
|
||||
{
|
||||
id: 1,
|
||||
body: "Use `const` instead of `let`",
|
||||
user: { login: "reviewer" },
|
||||
created_at: new Date().toISOString(),
|
||||
updated_at: new Date().toISOString(),
|
||||
html_url: "https://github.com/owner/repo/pull/42#issuecomment-1",
|
||||
},
|
||||
]);
|
||||
|
||||
expect(mockStore.addSteeringComment).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("steering comment content", () => {
|
||||
it("includes PR info and comment details", async () => {
|
||||
await handler.handleNewComments("KB-001", mockPrInfo, [
|
||||
{
|
||||
id: 1,
|
||||
body: "Please fix the bug",
|
||||
user: { login: "reviewer" },
|
||||
created_at: new Date().toISOString(),
|
||||
updated_at: new Date().toISOString(),
|
||||
html_url: "https://github.com/owner/repo/pull/42#issuecomment-1",
|
||||
},
|
||||
]);
|
||||
|
||||
const call = mockStore.addSteeringComment.mock.calls[0];
|
||||
const text = call[1] as string;
|
||||
|
||||
expect(text).toContain("PR Review Feedback");
|
||||
expect(text).toContain("@reviewer");
|
||||
expect(text).toContain("#42");
|
||||
expect(text).toContain("open");
|
||||
expect(text).toContain("Please fix the bug");
|
||||
expect(text).toContain("View on GitHub");
|
||||
});
|
||||
|
||||
it("truncates long comments", async () => {
|
||||
const longBody = "Please fix this issue: " + "a".repeat(1000);
|
||||
|
||||
await handler.handleNewComments("KB-001", mockPrInfo, [
|
||||
{
|
||||
id: 1,
|
||||
body: longBody,
|
||||
user: { login: "reviewer" },
|
||||
created_at: new Date().toISOString(),
|
||||
updated_at: new Date().toISOString(),
|
||||
html_url: "https://github.com/owner/repo/pull/42#issuecomment-1",
|
||||
},
|
||||
]);
|
||||
|
||||
const call = mockStore.addSteeringComment.mock.calls[0];
|
||||
const text = call[1] as string;
|
||||
|
||||
expect(text.length).toBeLessThan(longBody.length);
|
||||
expect(text).toContain("...");
|
||||
});
|
||||
|
||||
it("marks as agent-authored", async () => {
|
||||
await handler.handleNewComments("KB-001", mockPrInfo, [
|
||||
{
|
||||
id: 1,
|
||||
body: "Please fix this",
|
||||
user: { login: "reviewer" },
|
||||
created_at: new Date().toISOString(),
|
||||
updated_at: new Date().toISOString(),
|
||||
html_url: "https://github.com/owner/repo/pull/42#issuecomment-1",
|
||||
},
|
||||
]);
|
||||
|
||||
expect(mockStore.addSteeringComment).toHaveBeenCalledWith(
|
||||
"KB-001",
|
||||
expect.any(String),
|
||||
"agent"
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("createFollowUpTask", () => {
|
||||
it("creates follow-up task for unaddressed feedback", async () => {
|
||||
await handler.createFollowUpTask("KB-001", mockPrInfo, [
|
||||
{
|
||||
id: 1,
|
||||
body: "This needs fixing",
|
||||
user: { login: "reviewer" },
|
||||
created_at: new Date().toISOString(),
|
||||
updated_at: new Date().toISOString(),
|
||||
html_url: "https://github.com/owner/repo/pull/42#issuecomment-1",
|
||||
},
|
||||
]);
|
||||
|
||||
expect(mockStore.createTask).toHaveBeenCalledWith({
|
||||
title: "Follow-up: Address PR #42 feedback",
|
||||
description: expect.stringContaining("KB-001"),
|
||||
column: "triage",
|
||||
dependencies: ["KB-001"],
|
||||
});
|
||||
});
|
||||
|
||||
it("does nothing when no unaddressed comments", async () => {
|
||||
await handler.createFollowUpTask("KB-001", mockPrInfo, []);
|
||||
|
||||
expect(mockStore.createTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("summarizes multiple comments", async () => {
|
||||
await handler.createFollowUpTask("KB-001", mockPrInfo, [
|
||||
{
|
||||
id: 1,
|
||||
body: "First issue to fix",
|
||||
user: { login: "reviewer1" },
|
||||
created_at: new Date().toISOString(),
|
||||
updated_at: new Date().toISOString(),
|
||||
html_url: "https://github.com/owner/repo/pull/42#issuecomment-1",
|
||||
},
|
||||
{
|
||||
id: 2,
|
||||
body: "Second issue",
|
||||
user: { login: "reviewer2" },
|
||||
created_at: new Date().toISOString(),
|
||||
updated_at: new Date().toISOString(),
|
||||
html_url: "https://github.com/owner/repo/pull/42#issuecomment-2",
|
||||
},
|
||||
]);
|
||||
|
||||
const call = mockStore.createTask.mock.calls[0];
|
||||
const description = call[0].description as string;
|
||||
|
||||
expect(description).toContain("@reviewer1");
|
||||
expect(description).toContain("@reviewer2");
|
||||
expect(description).toContain("First issue");
|
||||
expect(description).toContain("Second issue");
|
||||
});
|
||||
});
|
||||
});
|
||||
187
packages/engine/src/pr-comment-handler.ts
Normal file
187
packages/engine/src/pr-comment-handler.ts
Normal file
@@ -0,0 +1,187 @@
|
||||
import type { TaskStore } from "@kb/core";
|
||||
import type { PrInfo } from "@kb/core";
|
||||
import { prMonitorLog } from "./logger.js";
|
||||
|
||||
interface PrComment {
|
||||
id: number;
|
||||
body: string;
|
||||
user: { login: string };
|
||||
created_at: string;
|
||||
updated_at: string;
|
||||
html_url: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Analyzes PR comments for actionable feedback and creates
|
||||
* steering comments or follow-up tasks.
|
||||
*/
|
||||
export class PrCommentHandler {
|
||||
// Keywords that suggest actionable feedback
|
||||
private readonly ACTION_KEYWORDS = [
|
||||
"fix",
|
||||
"change",
|
||||
"update",
|
||||
"remove",
|
||||
"add",
|
||||
"should",
|
||||
"need to",
|
||||
"needs to",
|
||||
"please",
|
||||
"consider",
|
||||
"suggest",
|
||||
"recommend",
|
||||
];
|
||||
|
||||
// Non-actionable patterns to filter out
|
||||
private readonly NON_ACTIONABLE_PATTERNS = [
|
||||
/^\s*lgtm\s*$/i,
|
||||
/^\s*looks? good\s*$/i,
|
||||
/^\s*thanks?\s*$/i,
|
||||
/^\s*thank you\s*$/i,
|
||||
/^\s*nice\s*$/i,
|
||||
/^\s*great\s*$/i,
|
||||
/^\s*awesome\s*$/i,
|
||||
/^\s*👍\s*$/,
|
||||
/^\s*✅\s*$/,
|
||||
];
|
||||
|
||||
constructor(private store: TaskStore) {}
|
||||
|
||||
/**
|
||||
* Process new PR comments for a task.
|
||||
* Called by PrMonitor when new comments are detected.
|
||||
*/
|
||||
async handleNewComments(
|
||||
taskId: string,
|
||||
prInfo: PrInfo,
|
||||
comments: PrComment[]
|
||||
): Promise<void> {
|
||||
for (const comment of comments) {
|
||||
await this.processComment(taskId, prInfo, comment);
|
||||
}
|
||||
}
|
||||
|
||||
private async processComment(
|
||||
taskId: string,
|
||||
prInfo: PrInfo,
|
||||
comment: PrComment
|
||||
): Promise<void> {
|
||||
// Skip non-actionable comments
|
||||
if (this.isNonActionable(comment.body)) {
|
||||
prMonitorLog.log(`Skipping non-actionable comment #${comment.id}`);
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if comment contains actionable feedback
|
||||
const isActionable = this.isActionable(comment.body);
|
||||
const hasCodeSuggestions = this.hasCodeBlock(comment.body);
|
||||
|
||||
if (!isActionable && !hasCodeSuggestions) {
|
||||
prMonitorLog.log(`Comment #${comment.id} does not contain actionable feedback`);
|
||||
return;
|
||||
}
|
||||
|
||||
// Build steering comment text
|
||||
const text = this.buildSteeringText(prInfo, comment, hasCodeSuggestions);
|
||||
|
||||
try {
|
||||
await this.store.addSteeringComment(taskId, text, "agent");
|
||||
prMonitorLog.log(`Added steering comment for PR review #${comment.id}`);
|
||||
} catch (err) {
|
||||
prMonitorLog.error(`Failed to add steering comment for ${taskId}:`, err);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a comment is non-actionable (LGTM, thanks, etc.)
|
||||
*/
|
||||
private isNonActionable(body: string): boolean {
|
||||
const trimmed = body.trim();
|
||||
return this.NON_ACTIONABLE_PATTERNS.some((pattern) => pattern.test(trimmed));
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a comment contains actionable feedback keywords.
|
||||
*/
|
||||
private isActionable(body: string): boolean {
|
||||
const lowerBody = body.toLowerCase();
|
||||
return this.ACTION_KEYWORDS.some((keyword) => lowerBody.includes(keyword));
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a comment contains code blocks suggesting changes.
|
||||
*/
|
||||
private hasCodeBlock(body: string): boolean {
|
||||
// Look for code blocks (``` or `code`)
|
||||
return /```[\s\S]*?```/.test(body) || /`[^`]+`/.test(body);
|
||||
}
|
||||
|
||||
/**
|
||||
* Build steering comment text from PR review comment.
|
||||
*/
|
||||
private buildSteeringText(
|
||||
prInfo: PrInfo,
|
||||
comment: PrComment,
|
||||
hasCodeSuggestions: boolean
|
||||
): string {
|
||||
const lines: string[] = [];
|
||||
|
||||
lines.push(`**PR Review Feedback** from @${comment.user.login}`);
|
||||
lines.push(`**PR:** #${prInfo.number} (${prInfo.status})`);
|
||||
lines.push("");
|
||||
|
||||
// Truncate comment body if too long
|
||||
const maxBodyLength = 500;
|
||||
let body = comment.body.trim();
|
||||
if (body.length > maxBodyLength) {
|
||||
body = body.slice(0, maxBodyLength) + "...";
|
||||
}
|
||||
lines.push(body);
|
||||
lines.push("");
|
||||
|
||||
if (hasCodeSuggestions) {
|
||||
lines.push("💡 This comment contains code suggestions. Please review and apply if appropriate.");
|
||||
}
|
||||
|
||||
lines.push(`[View on GitHub](${comment.html_url})`);
|
||||
|
||||
return lines.join("\n");
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a follow-up task when a PR is closed with unaddressed feedback.
|
||||
* This is called when a PR is merged or closed.
|
||||
*/
|
||||
async createFollowUpTask(
|
||||
originalTaskId: string,
|
||||
prInfo: PrInfo,
|
||||
unaddressedComments: PrComment[]
|
||||
): Promise<void> {
|
||||
if (unaddressedComments.length === 0) return;
|
||||
|
||||
const summary = unaddressedComments
|
||||
.map((c) => `- @${c.user.login}: ${c.body.slice(0, 100).trim()}${c.body.length > 100 ? "..." : ""}`)
|
||||
.join("\n");
|
||||
|
||||
const description = `Follow-up for ${originalTaskId}
|
||||
|
||||
PR #${prInfo.number} was ${prInfo.status} with unaddressed feedback:
|
||||
|
||||
${summary}
|
||||
|
||||
Please review the PR comments and address any remaining issues.`;
|
||||
|
||||
try {
|
||||
const task = await this.store.createTask({
|
||||
title: `Follow-up: Address PR #${prInfo.number} feedback`,
|
||||
description,
|
||||
column: "triage",
|
||||
dependencies: [originalTaskId],
|
||||
});
|
||||
|
||||
prMonitorLog.log(`Created follow-up task ${task.id} for PR #${prInfo.number}`);
|
||||
} catch (err) {
|
||||
prMonitorLog.error(`Failed to create follow-up task:`, err);
|
||||
}
|
||||
}
|
||||
}
|
||||
143
packages/engine/src/pr-monitor.test.ts
Normal file
143
packages/engine/src/pr-monitor.test.ts
Normal file
@@ -0,0 +1,143 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { PrMonitor, type PrComment } from "./pr-monitor.js";
|
||||
|
||||
describe("PrMonitor", () => {
|
||||
let monitor: PrMonitor;
|
||||
const mockFetch = vi.fn();
|
||||
const originalFetch = globalThis.fetch;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
monitor = new PrMonitor({ getGitHubToken: () => "test-token" });
|
||||
globalThis.fetch = mockFetch;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
monitor.stopAll();
|
||||
globalThis.fetch = originalFetch;
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
const mockPrInfo = {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open" as const,
|
||||
title: "Test PR",
|
||||
headBranch: "kb/kb-001",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
};
|
||||
|
||||
const mockComment: PrComment = {
|
||||
id: 123,
|
||||
body: "Test comment",
|
||||
user: { login: "reviewer" },
|
||||
created_at: new Date().toISOString(),
|
||||
updated_at: new Date().toISOString(),
|
||||
html_url: "https://github.com/owner/repo/pull/42#issuecomment-123",
|
||||
};
|
||||
|
||||
describe("startMonitoring", () => {
|
||||
it("starts monitoring a PR", () => {
|
||||
monitor.startMonitoring("KB-001", "owner", "repo", mockPrInfo);
|
||||
|
||||
const tracked = monitor.getTrackedPrs();
|
||||
expect(tracked.has("KB-001")).toBe(true);
|
||||
expect(tracked.get("KB-001")?.prInfo.number).toBe(42);
|
||||
});
|
||||
|
||||
it("replaces existing monitoring for same task", () => {
|
||||
monitor.startMonitoring("KB-001", "owner", "repo", mockPrInfo);
|
||||
const newPrInfo = { ...mockPrInfo, number: 43 };
|
||||
monitor.startMonitoring("KB-001", "owner", "repo", newPrInfo);
|
||||
|
||||
const tracked = monitor.getTrackedPrs();
|
||||
expect(tracked.get("KB-001")?.prInfo.number).toBe(43);
|
||||
});
|
||||
});
|
||||
|
||||
describe("stopMonitoring", () => {
|
||||
it("stops monitoring a task", () => {
|
||||
monitor.startMonitoring("KB-001", "owner", "repo", mockPrInfo);
|
||||
monitor.stopMonitoring("KB-001");
|
||||
|
||||
const tracked = monitor.getTrackedPrs();
|
||||
expect(tracked.has("KB-001")).toBe(false);
|
||||
});
|
||||
|
||||
it("does nothing for untracked task", () => {
|
||||
expect(() => monitor.stopMonitoring("KB-999")).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe("stopAll", () => {
|
||||
it("stops all monitoring", () => {
|
||||
monitor.startMonitoring("KB-001", "owner", "repo", mockPrInfo);
|
||||
monitor.startMonitoring("KB-002", "owner", "repo", mockPrInfo);
|
||||
|
||||
monitor.stopAll();
|
||||
|
||||
const tracked = monitor.getTrackedPrs();
|
||||
expect(tracked.size).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("polling", () => {
|
||||
it("polls for comments on interval", async () => {
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () => Promise.resolve([]),
|
||||
});
|
||||
|
||||
monitor.startMonitoring("KB-001", "owner", "repo", mockPrInfo);
|
||||
|
||||
// Wait for initial check
|
||||
await vi.advanceTimersByTimeAsync(1);
|
||||
|
||||
expect(mockFetch).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("calls onNewComments when new comments found", async () => {
|
||||
const callback = vi.fn();
|
||||
monitor.onNewComments(callback);
|
||||
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () => Promise.resolve([mockComment]),
|
||||
});
|
||||
|
||||
monitor.startMonitoring("KB-001", "owner", "repo", mockPrInfo);
|
||||
await vi.advanceTimersByTimeAsync(1);
|
||||
|
||||
expect(callback).toHaveBeenCalledWith("KB-001", mockPrInfo, [mockComment]);
|
||||
});
|
||||
|
||||
it("tracks lastCommentId to avoid duplicate notifications", async () => {
|
||||
const callback = vi.fn();
|
||||
monitor.onNewComments(callback);
|
||||
|
||||
mockFetch
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () => Promise.resolve([mockComment]),
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () => Promise.resolve([mockComment]), // Same comment again
|
||||
});
|
||||
|
||||
monitor.startMonitoring("KB-001", "owner", "repo", mockPrInfo);
|
||||
|
||||
// First check
|
||||
await vi.advanceTimersByTimeAsync(1);
|
||||
expect(callback).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Second scheduled check after 30s
|
||||
await vi.advanceTimersByTimeAsync(30 * 1000);
|
||||
|
||||
// Second poll should not trigger callback for same comment
|
||||
expect(callback).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
261
packages/engine/src/pr-monitor.ts
Normal file
261
packages/engine/src/pr-monitor.ts
Normal file
@@ -0,0 +1,261 @@
|
||||
import { prMonitorLog } from "./logger.js";
|
||||
import type { PrInfo } from "@kb/core";
|
||||
|
||||
export interface TrackedPr {
|
||||
owner: string;
|
||||
repo: string;
|
||||
prInfo: PrInfo;
|
||||
lastCheckedAt: Date;
|
||||
lastCommentId?: number;
|
||||
consecutiveErrors: number;
|
||||
isActive: boolean; // true if we've seen recent activity
|
||||
}
|
||||
|
||||
export interface PrComment {
|
||||
id: number;
|
||||
body: string;
|
||||
user: { login: string };
|
||||
created_at: string;
|
||||
updated_at: string;
|
||||
html_url: string;
|
||||
}
|
||||
|
||||
export type OnNewCommentsCallback = (
|
||||
taskId: string,
|
||||
prInfo: PrInfo,
|
||||
comments: PrComment[]
|
||||
) => void | Promise<void>;
|
||||
|
||||
/**
|
||||
* Monitors GitHub PRs for new comments.
|
||||
* Uses adaptive polling: 30s when active, 5min when idle.
|
||||
* Implements exponential backoff on errors.
|
||||
*/
|
||||
export class PrMonitor {
|
||||
private trackedPrs = new Map<string, TrackedPr>();
|
||||
private intervals = new Map<string, ReturnType<typeof setInterval>>();
|
||||
private newCommentsCallback?: OnNewCommentsCallback;
|
||||
private getGitHubToken: () => string | undefined;
|
||||
|
||||
// Polling intervals in ms
|
||||
private readonly ACTIVE_INTERVAL = 30 * 1000; // 30 seconds
|
||||
private readonly IDLE_INTERVAL = 5 * 60 * 1000; // 5 minutes
|
||||
private readonly MIN_INTERVAL = 30 * 1000;
|
||||
private readonly MAX_INTERVAL = 15 * 60 * 1000; // 15 minutes max backoff
|
||||
|
||||
constructor(options: { getGitHubToken?: () => string | undefined } = {}) {
|
||||
this.getGitHubToken = options.getGitHubToken ?? (() => process.env.GITHUB_TOKEN);
|
||||
}
|
||||
|
||||
/**
|
||||
* Register a callback to be called when new comments are found.
|
||||
*/
|
||||
onNewComments(callback: OnNewCommentsCallback): void {
|
||||
this.newCommentsCallback = callback;
|
||||
}
|
||||
|
||||
/**
|
||||
* Start monitoring a PR for comments.
|
||||
*/
|
||||
startMonitoring(
|
||||
taskId: string,
|
||||
owner: string,
|
||||
repo: string,
|
||||
prInfo: PrInfo
|
||||
): void {
|
||||
// Stop any existing monitoring for this task
|
||||
this.stopMonitoring(taskId);
|
||||
|
||||
const tracked: TrackedPr = {
|
||||
owner,
|
||||
repo,
|
||||
prInfo,
|
||||
lastCheckedAt: new Date(),
|
||||
lastCommentId: undefined,
|
||||
consecutiveErrors: 0,
|
||||
isActive: true, // Start as active
|
||||
};
|
||||
|
||||
this.trackedPrs.set(taskId, tracked);
|
||||
|
||||
// Do an initial check immediately
|
||||
this.checkForComments(taskId, tracked);
|
||||
|
||||
// Set up polling interval
|
||||
this.scheduleNextCheck(taskId, tracked);
|
||||
|
||||
prMonitorLog.log(`Started monitoring PR #${prInfo.number} for task ${taskId}`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Stop monitoring a PR.
|
||||
*/
|
||||
stopMonitoring(taskId: string): void {
|
||||
const interval = this.intervals.get(taskId);
|
||||
if (interval) {
|
||||
clearTimeout(interval);
|
||||
this.intervals.delete(taskId);
|
||||
}
|
||||
|
||||
if (this.trackedPrs.has(taskId)) {
|
||||
this.trackedPrs.delete(taskId);
|
||||
prMonitorLog.log(`Stopped monitoring task ${taskId}`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Stop monitoring all PRs. Called on scheduler shutdown.
|
||||
*/
|
||||
stopAll(): void {
|
||||
for (const [taskId] of this.trackedPrs) {
|
||||
this.stopMonitoring(taskId);
|
||||
}
|
||||
prMonitorLog.log("Stopped all PR monitoring");
|
||||
}
|
||||
|
||||
/**
|
||||
* Get currently tracked PRs (for testing/debugging).
|
||||
*/
|
||||
getTrackedPrs(): Map<string, TrackedPr> {
|
||||
return new Map(this.trackedPrs);
|
||||
}
|
||||
|
||||
private scheduleNextCheck(taskId: string, tracked: TrackedPr): void {
|
||||
// Calculate interval based on activity and error count
|
||||
let interval = tracked.isActive ? this.ACTIVE_INTERVAL : this.IDLE_INTERVAL;
|
||||
|
||||
// Exponential backoff on errors: 30s * 2^errors, capped at 15min
|
||||
if (tracked.consecutiveErrors > 0) {
|
||||
const backoffMultiplier = Math.pow(2, Math.min(tracked.consecutiveErrors, 5));
|
||||
interval = Math.min(interval * backoffMultiplier, this.MAX_INTERVAL);
|
||||
}
|
||||
|
||||
const timeoutId = setTimeout(() => {
|
||||
this.checkForComments(taskId, tracked).then(() => {
|
||||
// Reschedule if still tracked
|
||||
if (this.trackedPrs.has(taskId)) {
|
||||
this.scheduleNextCheck(taskId, tracked);
|
||||
}
|
||||
});
|
||||
}, interval);
|
||||
|
||||
this.intervals.set(taskId, timeoutId);
|
||||
}
|
||||
|
||||
private async checkForComments(
|
||||
taskId: string,
|
||||
tracked: TrackedPr
|
||||
): Promise<boolean> {
|
||||
const token = this.getGitHubToken();
|
||||
if (!token) {
|
||||
prMonitorLog.warn(`No GitHub token available for task ${taskId}`);
|
||||
tracked.consecutiveErrors++;
|
||||
return false; // Don't reschedule - wait for next scheduled check
|
||||
}
|
||||
|
||||
try {
|
||||
const since = tracked.lastCheckedAt.toISOString();
|
||||
const comments = await this.fetchComments(
|
||||
tracked.owner,
|
||||
tracked.repo,
|
||||
tracked.prInfo.number,
|
||||
since,
|
||||
token
|
||||
);
|
||||
|
||||
// Filter to only new comments (by ID)
|
||||
const newComments = tracked.lastCommentId
|
||||
? comments.filter((c) => c.id > tracked.lastCommentId!)
|
||||
: comments;
|
||||
|
||||
if (newComments.length > 0) {
|
||||
prMonitorLog.log(
|
||||
`Found ${newComments.length} new comment(s) on PR #${tracked.prInfo.number}`
|
||||
);
|
||||
|
||||
// Update lastCommentId
|
||||
const maxId = Math.max(...newComments.map((c) => c.id));
|
||||
tracked.lastCommentId = maxId;
|
||||
|
||||
// Mark as active since we found new comments
|
||||
tracked.isActive = true;
|
||||
|
||||
// Notify handler
|
||||
if (this.newCommentsCallback) {
|
||||
try {
|
||||
await this.newCommentsCallback(taskId, tracked.prInfo, newComments);
|
||||
} catch (err) {
|
||||
prMonitorLog.error(`Error handling new comments for ${taskId}:`, err);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// No new comments - mark as idle after 5 minutes of no activity
|
||||
const timeSinceLastComment = Date.now() - tracked.lastCheckedAt.getTime();
|
||||
if (timeSinceLastComment > 5 * 60 * 1000) {
|
||||
tracked.isActive = false;
|
||||
}
|
||||
}
|
||||
|
||||
// Reset error count on success
|
||||
tracked.consecutiveErrors = 0;
|
||||
tracked.lastCheckedAt = new Date();
|
||||
return true;
|
||||
} catch (err: any) {
|
||||
tracked.consecutiveErrors++;
|
||||
prMonitorLog.error(
|
||||
`Error checking PR #${tracked.prInfo.number} for task ${taskId} ` +
|
||||
`(attempt ${tracked.consecutiveErrors}):`,
|
||||
err.message
|
||||
);
|
||||
|
||||
// Disable monitoring after 5 consecutive failures
|
||||
if (tracked.consecutiveErrors >= 5) {
|
||||
prMonitorLog.warn(
|
||||
`Disabling PR monitoring for task ${taskId} after 5 consecutive failures`
|
||||
);
|
||||
this.stopMonitoring(taskId);
|
||||
return false;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
private async fetchComments(
|
||||
owner: string,
|
||||
repo: string,
|
||||
prNumber: number,
|
||||
since: string,
|
||||
token: string
|
||||
): Promise<PrComment[]> {
|
||||
const params = new URLSearchParams();
|
||||
params.append("per_page", "100");
|
||||
if (since) {
|
||||
params.append("since", since);
|
||||
}
|
||||
|
||||
const url = `https://api.github.com/repos/${encodeURIComponent(
|
||||
owner
|
||||
)}/${encodeURIComponent(repo)}/issues/${prNumber}/comments?${params}`;
|
||||
|
||||
const headers: Record<string, string> = {
|
||||
Accept: "application/vnd.github+json",
|
||||
"X-GitHub-Api-Version": "2022-11-28",
|
||||
"User-Agent": "kb-engine/1.0",
|
||||
Authorization: `Bearer ${token}`,
|
||||
};
|
||||
|
||||
const response = await fetch(url, { headers });
|
||||
|
||||
if (!response.ok) {
|
||||
if (response.status === 404) {
|
||||
throw new Error(`PR #${prNumber} not found in ${owner}/${repo}`);
|
||||
}
|
||||
if (response.status === 401 || response.status === 403) {
|
||||
throw new Error("Authentication failed or rate limited");
|
||||
}
|
||||
throw new Error(`GitHub API error: ${response.status} ${response.statusText}`);
|
||||
}
|
||||
|
||||
return response.json() as Promise<PrComment[]>;
|
||||
}
|
||||
}
|
||||
@@ -1,6 +1,8 @@
|
||||
import { resolveDependencyOrder, type TaskStore, type Task } from "@kb/core";
|
||||
import type { AgentSemaphore } from "./concurrency.js";
|
||||
import { schedulerLog } from "./logger.js";
|
||||
import type { PrMonitor } from "./pr-monitor.js";
|
||||
import { getCurrentGitHubRepo } from "./github.js";
|
||||
|
||||
/**
|
||||
* Check whether two sets of file scope paths overlap.
|
||||
@@ -53,6 +55,8 @@ export interface SchedulerOptions {
|
||||
onSchedule?: (task: Task) => void;
|
||||
/** Called when a task is blocked by deps */
|
||||
onBlocked?: (task: Task, blockedBy: string[]) => void;
|
||||
/** Optional PR monitor for tracking in-review PRs */
|
||||
prMonitor?: PrMonitor;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -111,6 +115,48 @@ export class Scheduler {
|
||||
this.schedule();
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* PR Monitoring: Start monitoring when a task moves to "in-review",
|
||||
* stop monitoring when it moves out.
|
||||
*/
|
||||
this.store.on("task:moved", ({ task, to }) => {
|
||||
if (!this.options.prMonitor) return;
|
||||
|
||||
if (to === "in-review" && task.prInfo) {
|
||||
// Start monitoring existing PR
|
||||
const repo = getCurrentGitHubRepo(this.store.getRootDir());
|
||||
if (repo) {
|
||||
this.options.prMonitor.startMonitoring(task.id, repo.owner, repo.repo, task.prInfo);
|
||||
}
|
||||
} else if (task.column === "in-review" && to !== "in-review") {
|
||||
// Task moved out of in-review, stop monitoring
|
||||
this.options.prMonitor.stopMonitoring(task.id);
|
||||
|
||||
// If task has a closed/merged PR, check for unaddressed feedback
|
||||
if (task.prInfo && (task.prInfo.status === "closed" || task.prInfo.status === "merged")) {
|
||||
// This would need the tracked PR data - handled by PrMonitor/PrCommentHandler
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* PR Monitoring: Start monitoring when PR is linked to an in-review task.
|
||||
*/
|
||||
this.store.on("task:updated", (task) => {
|
||||
if (!this.options.prMonitor) return;
|
||||
if (task.column !== "in-review") return;
|
||||
if (!task.prInfo) return;
|
||||
|
||||
// Check if we're already monitoring this task
|
||||
const tracked = this.options.prMonitor.getTrackedPrs();
|
||||
if (!tracked.has(task.id)) {
|
||||
const repo = getCurrentGitHubRepo(this.store.getRootDir());
|
||||
if (repo) {
|
||||
this.options.prMonitor.startMonitoring(task.id, repo.owner, repo.repo, task.prInfo);
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
start(): void {
|
||||
@@ -131,6 +177,10 @@ export class Scheduler {
|
||||
this.pollInterval = null;
|
||||
this.activePollMs = null;
|
||||
}
|
||||
// Stop all PR monitoring when scheduler shuts down
|
||||
if (this.options.prMonitor) {
|
||||
this.options.prMonitor.stopAll();
|
||||
}
|
||||
schedulerLog.log("Stopped");
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user