feat(KB-093): add PR-first merge mode with configurable merge strategies
- Add mergeStrategy setting (fast-forward, squash, merge-commit) to config - Implement PR-first auto-completion flow that monitors PR merge status - Wire PR monitoring service to detect merge completion and trigger auto-close - Add PR status UI to dashboard with merge progress indicator - Update settings modal with merge strategy selector - Add changeset for PR-first merge mode feature
This commit is contained in:
@@ -49,7 +49,16 @@ An executor agent works through the spec step by step in the worktree. At each s
|
||||
|
||||
### Merge
|
||||
|
||||
When execution finishes and the reviewer signs off, the task moves to "in review." By default, the completed work is automatically squash-merged into your current branch with a clean commit. Worktrees can be cleaned up after merge or reused by the next task to keep build caches warm. You can disable auto-merge if you prefer to review and merge manually.
|
||||
When execution finishes and the reviewer signs off, the task moves to "in review." kb supports two completion modes:
|
||||
|
||||
- **Direct merge** *(default)* — automatically squash-merges the completed task branch into your current branch with a clean commit.
|
||||
- **Pull request** — automatically creates or links a GitHub PR for the task branch, waits for GitHub reviews/checks, then merges the PR once policy conditions are satisfied.
|
||||
|
||||
`autoMerge` still controls whether kb performs completion automatically at all. If `autoMerge` is disabled, tasks stay in **In Review** until you finish the merge yourself.
|
||||
|
||||
For PR-first mode, authenticate GitHub with `gh auth login` or `GITHUB_TOKEN`, and make sure the task branch already exists on GitHub as `kb/<task-id-lower>`. kb does **not** push branches for you before PR creation.
|
||||
|
||||
Worktrees can be cleaned up after merge or reused by the next task to keep build caches warm.
|
||||
|
||||
Tasks flow through: **Triage → Todo → In Progress → In Review → Done**.
|
||||
|
||||
|
||||
@@ -52,8 +52,16 @@ const mockListen = vi.fn((port: number) => {
|
||||
return server;
|
||||
});
|
||||
|
||||
const MockGitHubClient = vi.fn().mockImplementation(() => ({
|
||||
findPrForBranch: vi.fn(),
|
||||
createPr: vi.fn(),
|
||||
getPrMergeStatus: vi.fn(),
|
||||
mergePr: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@kb/dashboard", () => ({
|
||||
createServer: vi.fn(() => ({ listen: mockListen })),
|
||||
GitHubClient: MockGitHubClient,
|
||||
}));
|
||||
|
||||
// ── Mock @kb/engine ────────────────────────────────────────────────
|
||||
|
||||
@@ -17,10 +17,14 @@ function makeMockStore() {
|
||||
maxConcurrent: 1,
|
||||
maxWorktrees: 2,
|
||||
autoMerge: false,
|
||||
mergeStrategy: "direct",
|
||||
pollIntervalMs: 60_000,
|
||||
}),
|
||||
listTasks: vi.fn().mockResolvedValue([]),
|
||||
getTask: vi.fn().mockResolvedValue({ column: "in-review", paused: false }),
|
||||
getTask: vi.fn().mockResolvedValue({ id: "KB-TEST", column: "in-review", paused: false, description: "Test task", log: [] }),
|
||||
moveTask: vi.fn().mockResolvedValue({}),
|
||||
updatePrInfo: vi.fn().mockResolvedValue({}),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
updateTask: vi.fn().mockResolvedValue({}),
|
||||
on: vi.fn((event: string, handler: (...args: unknown[]) => void) => {
|
||||
emitter.on(event, handler);
|
||||
@@ -35,6 +39,35 @@ vi.mock("@kb/core", () => ({
|
||||
TaskStore: vi.fn().mockImplementation(() => makeMockStore()),
|
||||
}));
|
||||
|
||||
// ── Hoisted shared mocks ───────────────────────────────────────────
|
||||
|
||||
const {
|
||||
mockExec,
|
||||
mockExecSync,
|
||||
mockFindPrForBranch,
|
||||
mockCreatePr,
|
||||
mockGetPrMergeStatus,
|
||||
mockMergePr,
|
||||
} = vi.hoisted(() => ({
|
||||
mockExec: vi.fn((_command: string, callback?: () => void) => callback?.()),
|
||||
mockExecSync: vi.fn(() => ""),
|
||||
mockFindPrForBranch: vi.fn(),
|
||||
mockCreatePr: vi.fn(),
|
||||
mockGetPrMergeStatus: vi.fn(),
|
||||
mockMergePr: vi.fn(),
|
||||
}));
|
||||
|
||||
// ── Mock node:child_process ────────────────────────────────────────
|
||||
|
||||
vi.mock("node:child_process", async (importOriginal) => {
|
||||
const original = await importOriginal<typeof import("node:child_process")>();
|
||||
return {
|
||||
...original,
|
||||
exec: mockExec,
|
||||
execSync: mockExecSync,
|
||||
};
|
||||
});
|
||||
|
||||
// ── Mock @kb/dashboard ─────────────────────────────────────────────
|
||||
|
||||
/** Create a mock server (EventEmitter) that simulates net.Server behavior. */
|
||||
@@ -59,6 +92,12 @@ const mockListen = vi.fn((port: number) => {
|
||||
|
||||
vi.mock("@kb/dashboard", () => ({
|
||||
createServer: vi.fn(() => ({ listen: mockListen })),
|
||||
GitHubClient: vi.fn().mockImplementation(() => ({
|
||||
findPrForBranch: mockFindPrForBranch,
|
||||
createPr: mockCreatePr,
|
||||
getPrMergeStatus: mockGetPrMergeStatus,
|
||||
mergePr: mockMergePr,
|
||||
})),
|
||||
}));
|
||||
|
||||
// ── Mock node:readline ──────────────────────────────────────────────
|
||||
@@ -94,6 +133,17 @@ vi.mock("@kb/engine", async (importOriginal) => {
|
||||
start: vi.fn(),
|
||||
stop: vi.fn(),
|
||||
})),
|
||||
PrMonitor: vi.fn().mockImplementation(() => ({
|
||||
onNewComments: vi.fn(),
|
||||
startMonitoring: vi.fn(),
|
||||
stopMonitoring: vi.fn(),
|
||||
stopAll: vi.fn(),
|
||||
getTrackedPrs: vi.fn().mockReturnValue(new Map()),
|
||||
updatePrInfo: vi.fn(),
|
||||
})),
|
||||
PrCommentHandler: vi.fn().mockImplementation(() => ({
|
||||
handleNewComments: vi.fn().mockResolvedValue(undefined),
|
||||
})),
|
||||
aiMergeTask: vi.fn().mockImplementation(() => Promise.resolve({ merged: true })),
|
||||
scanIdleWorktrees: vi.fn().mockResolvedValue([]),
|
||||
cleanupOrphanedWorktrees: vi.fn().mockResolvedValue(0),
|
||||
@@ -102,14 +152,294 @@ vi.mock("@kb/engine", async (importOriginal) => {
|
||||
|
||||
// ── Import module under test (after mocks) ──────────────────────────
|
||||
|
||||
const { runDashboard } = await import("./dashboard.js");
|
||||
const { runDashboard, processPullRequestMergeTask, getMergeStrategy, getTaskBranchName } = await import("./dashboard.js");
|
||||
|
||||
// ── Tests ───────────────────────────────────────────────────────────
|
||||
|
||||
function resetGitHubMocks() {
|
||||
mockFindPrForBranch.mockReset();
|
||||
mockCreatePr.mockReset();
|
||||
mockGetPrMergeStatus.mockReset();
|
||||
mockMergePr.mockReset();
|
||||
|
||||
mockFindPrForBranch.mockResolvedValue(null);
|
||||
mockCreatePr.mockResolvedValue({
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open",
|
||||
title: "KB-TEST",
|
||||
headBranch: "kb/kb-test",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
});
|
||||
mockGetPrMergeStatus.mockResolvedValue({
|
||||
prInfo: {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open",
|
||||
title: "KB-TEST",
|
||||
headBranch: "kb/kb-test",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
},
|
||||
reviewDecision: null,
|
||||
checks: [],
|
||||
mergeReady: false,
|
||||
blockingReasons: ["required checks not successful: ci (pending)"],
|
||||
});
|
||||
mockMergePr.mockResolvedValue({
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "merged",
|
||||
title: "KB-TEST",
|
||||
headBranch: "kb/kb-test",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
});
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
resetGitHubMocks();
|
||||
mockExecSync.mockReset();
|
||||
mockExecSync.mockReturnValue("");
|
||||
mockExec.mockClear();
|
||||
});
|
||||
|
||||
describe("PR merge helpers", () => {
|
||||
it("defaults mergeStrategy to direct when unset", () => {
|
||||
expect(getMergeStrategy({ mergeStrategy: undefined })).toBe("direct");
|
||||
});
|
||||
|
||||
it("uses pull-request mergeStrategy when configured", () => {
|
||||
expect(getMergeStrategy({ mergeStrategy: "pull-request" })).toBe("pull-request");
|
||||
});
|
||||
|
||||
it("uses kb/{task-id-lower} branch naming for pull requests", () => {
|
||||
expect(getTaskBranchName("KB-093")).toBe("kb/kb-093");
|
||||
});
|
||||
});
|
||||
|
||||
describe("processPullRequestMergeTask", () => {
|
||||
it("creates and links a PR when task.prInfo is missing", async () => {
|
||||
const store = makeMockStore();
|
||||
store.getTask.mockResolvedValue({
|
||||
id: "KB-093",
|
||||
title: "Add support for creating pull requests",
|
||||
description: "Implement PR automation",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
worktree: "/tmp/kb-093",
|
||||
log: [],
|
||||
});
|
||||
|
||||
const result = await processPullRequestMergeTask(store as any, "/repo", "KB-093", {
|
||||
findPrForBranch: mockFindPrForBranch,
|
||||
createPr: mockCreatePr,
|
||||
getPrMergeStatus: mockGetPrMergeStatus,
|
||||
mergePr: mockMergePr,
|
||||
} as any);
|
||||
|
||||
expect(result).toBe("waiting");
|
||||
expect(mockFindPrForBranch).toHaveBeenCalledWith({ head: "kb/kb-093", state: "all" });
|
||||
expect(mockCreatePr).toHaveBeenCalledWith({
|
||||
title: "KB-093: Add support for creating pull requests",
|
||||
body: "Automated PR for KB-093.\n\nImplement PR automation",
|
||||
head: "kb/kb-093",
|
||||
});
|
||||
expect(store.updatePrInfo).toHaveBeenCalledWith(
|
||||
"KB-093",
|
||||
expect.objectContaining({ number: 42, status: "open" }),
|
||||
);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-093", { status: "awaiting-pr-checks" });
|
||||
});
|
||||
|
||||
it("links an existing PR instead of creating a duplicate", async () => {
|
||||
const store = makeMockStore();
|
||||
const existingPr = {
|
||||
url: "https://github.com/owner/repo/pull/7",
|
||||
number: 7,
|
||||
status: "open" as const,
|
||||
title: "Existing PR",
|
||||
headBranch: "kb/kb-093",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
};
|
||||
mockFindPrForBranch.mockResolvedValue(existingPr);
|
||||
store.getTask.mockResolvedValue({
|
||||
id: "KB-093",
|
||||
title: "Task",
|
||||
description: "Description",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
log: [],
|
||||
});
|
||||
|
||||
await processPullRequestMergeTask(store as any, "/repo", "KB-093", {
|
||||
findPrForBranch: mockFindPrForBranch,
|
||||
createPr: mockCreatePr,
|
||||
getPrMergeStatus: mockGetPrMergeStatus,
|
||||
mergePr: mockMergePr,
|
||||
} as any);
|
||||
|
||||
expect(mockCreatePr).not.toHaveBeenCalled();
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"KB-093",
|
||||
"Linked existing PR",
|
||||
"PR #7: https://github.com/owner/repo/pull/7",
|
||||
);
|
||||
});
|
||||
|
||||
it("merges a ready PR and finalizes task cleanup", async () => {
|
||||
const store = makeMockStore();
|
||||
store.getTask.mockResolvedValue({
|
||||
id: "KB-093",
|
||||
title: "Task",
|
||||
description: "Description",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
worktree: "/tmp/kb-093",
|
||||
prInfo: {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open",
|
||||
title: "Task",
|
||||
headBranch: "kb/kb-093",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
},
|
||||
log: [],
|
||||
});
|
||||
mockGetPrMergeStatus.mockResolvedValue({
|
||||
prInfo: {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open",
|
||||
title: "Task",
|
||||
headBranch: "kb/kb-093",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
},
|
||||
reviewDecision: "APPROVED",
|
||||
checks: [{ name: "ci", required: true, state: "success" }],
|
||||
mergeReady: true,
|
||||
blockingReasons: [],
|
||||
});
|
||||
|
||||
const result = await processPullRequestMergeTask(store as any, "/repo", "KB-093", {
|
||||
findPrForBranch: mockFindPrForBranch,
|
||||
createPr: mockCreatePr,
|
||||
getPrMergeStatus: mockGetPrMergeStatus,
|
||||
mergePr: mockMergePr,
|
||||
} as any);
|
||||
|
||||
expect(result).toBe("merged");
|
||||
expect(mockMergePr).toHaveBeenCalledWith({ number: 42, method: "squash" });
|
||||
expect(store.moveTask).toHaveBeenCalledWith("KB-093", "done");
|
||||
expect(mockExecSync).toHaveBeenCalledWith('git worktree remove "/tmp/kb-093" --force', expect.any(Object));
|
||||
expect(mockExecSync).toHaveBeenCalledWith('git branch -d "kb/kb-093"', expect.any(Object));
|
||||
});
|
||||
|
||||
it("does not merge when required checks or reviews are blocking", async () => {
|
||||
const store = makeMockStore();
|
||||
store.getTask.mockResolvedValue({
|
||||
id: "KB-093",
|
||||
title: "Task",
|
||||
description: "Description",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
prInfo: {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open",
|
||||
title: "Task",
|
||||
headBranch: "kb/kb-093",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
},
|
||||
log: [],
|
||||
});
|
||||
mockGetPrMergeStatus.mockResolvedValue({
|
||||
prInfo: {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open",
|
||||
title: "Task",
|
||||
headBranch: "kb/kb-093",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
},
|
||||
reviewDecision: "CHANGES_REQUESTED",
|
||||
checks: [{ name: "ci", required: true, state: "pending" }],
|
||||
mergeReady: false,
|
||||
blockingReasons: ["changes requested review is active", "required checks not successful: ci (pending)"],
|
||||
});
|
||||
|
||||
const result = await processPullRequestMergeTask(store as any, "/repo", "KB-093", {
|
||||
findPrForBranch: mockFindPrForBranch,
|
||||
createPr: mockCreatePr,
|
||||
getPrMergeStatus: mockGetPrMergeStatus,
|
||||
mergePr: mockMergePr,
|
||||
} as any);
|
||||
|
||||
expect(result).toBe("waiting");
|
||||
expect(mockMergePr).not.toHaveBeenCalled();
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-093", { status: "awaiting-pr-checks" });
|
||||
});
|
||||
});
|
||||
|
||||
describe("runDashboard — PR-first auto-merge queue", () => {
|
||||
let mockStore: ReturnType<typeof makeMockStore>;
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.clearAllMocks();
|
||||
resetGitHubMocks();
|
||||
mockStore = makeMockStore();
|
||||
mockStore.getSettings.mockResolvedValue({
|
||||
maxConcurrent: 1,
|
||||
maxWorktrees: 2,
|
||||
autoMerge: true,
|
||||
mergeStrategy: "pull-request",
|
||||
pollIntervalMs: 60_000,
|
||||
enginePaused: false,
|
||||
globalPause: false,
|
||||
});
|
||||
mockStore.listTasks.mockResolvedValue([
|
||||
{ id: "KB-093", column: "in-review", paused: false },
|
||||
]);
|
||||
mockStore.getTask.mockResolvedValue({
|
||||
id: "KB-093",
|
||||
title: "Task",
|
||||
description: "Description",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
log: [],
|
||||
});
|
||||
|
||||
const { TaskStore } = await import("@kb/core");
|
||||
(TaskStore as ReturnType<typeof vi.fn>).mockImplementation(() => mockStore);
|
||||
});
|
||||
|
||||
it("uses PR lifecycle instead of aiMergeTask when mergeStrategy is pull-request", async () => {
|
||||
const { aiMergeTask } = await import("@kb/engine");
|
||||
|
||||
await runDashboard(0, { open: false });
|
||||
await new Promise((r) => setTimeout(r, 100));
|
||||
|
||||
expect(mockCreatePr).toHaveBeenCalledWith({
|
||||
title: "KB-093: Task",
|
||||
body: "Automated PR for KB-093.\n\nDescription",
|
||||
head: "kb/kb-093",
|
||||
});
|
||||
expect(aiMergeTask).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("runDashboard — WorktreePool wiring", () => {
|
||||
beforeEach(async () => {
|
||||
capturedExecutorOpts = undefined;
|
||||
vi.clearAllMocks();
|
||||
resetGitHubMocks();
|
||||
// Re-set TaskStore mock (clearAllMocks wipes implementations)
|
||||
const { TaskStore } = await import("@kb/core");
|
||||
(TaskStore as ReturnType<typeof vi.fn>).mockImplementation(() => makeMockStore());
|
||||
@@ -177,6 +507,7 @@ describe("runDashboard — auto-merge pause exclusion", () => {
|
||||
beforeEach(async () => {
|
||||
capturedExecutorOpts = undefined;
|
||||
vi.clearAllMocks();
|
||||
resetGitHubMocks();
|
||||
mockStore = makeMockStore();
|
||||
const { TaskStore } = await import("@kb/core");
|
||||
(TaskStore as ReturnType<typeof vi.fn>).mockImplementation(() => mockStore);
|
||||
@@ -254,6 +585,7 @@ describe("runDashboard — immediate resume on unpause", () => {
|
||||
beforeEach(async () => {
|
||||
capturedExecutorOpts = undefined;
|
||||
vi.clearAllMocks();
|
||||
resetGitHubMocks();
|
||||
mockStore = makeMockStore();
|
||||
const { TaskStore } = await import("@kb/core");
|
||||
(TaskStore as ReturnType<typeof vi.fn>).mockImplementation(() => mockStore);
|
||||
@@ -359,6 +691,7 @@ describe("runDashboard — engine pause/unpause cycle", () => {
|
||||
beforeEach(async () => {
|
||||
capturedExecutorOpts = undefined;
|
||||
vi.clearAllMocks();
|
||||
resetGitHubMocks();
|
||||
mockStore = makeMockStore();
|
||||
const { TaskStore } = await import("@kb/core");
|
||||
(TaskStore as ReturnType<typeof vi.fn>).mockImplementation(() => mockStore);
|
||||
@@ -400,6 +733,7 @@ describe("runDashboard — port fallback on EADDRINUSE", () => {
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.clearAllMocks();
|
||||
resetGitHubMocks();
|
||||
const { TaskStore } = await import("@kb/core");
|
||||
(TaskStore as ReturnType<typeof vi.fn>).mockImplementation(() => makeMockStore());
|
||||
const engine = await import("@kb/engine");
|
||||
@@ -516,6 +850,7 @@ describe("runDashboard — enginePaused (soft pause)", () => {
|
||||
beforeEach(async () => {
|
||||
capturedExecutorOpts = undefined;
|
||||
vi.clearAllMocks();
|
||||
resetGitHubMocks();
|
||||
mockStore = makeMockStore();
|
||||
const { TaskStore } = await import("@kb/core");
|
||||
(TaskStore as ReturnType<typeof vi.fn>).mockImplementation(() => mockStore);
|
||||
@@ -630,6 +965,7 @@ describe("runDashboard — --paused flag", () => {
|
||||
beforeEach(async () => {
|
||||
capturedExecutorOpts = undefined;
|
||||
vi.clearAllMocks();
|
||||
resetGitHubMocks();
|
||||
mockStore = makeMockStore();
|
||||
const { TaskStore } = await import("@kb/core");
|
||||
(TaskStore as ReturnType<typeof vi.fn>).mockImplementation(() => mockStore);
|
||||
@@ -690,6 +1026,7 @@ describe("runDashboard — --paused flag", () => {
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.clearAllMocks();
|
||||
resetGitHubMocks();
|
||||
mockStore = makeMockStore();
|
||||
const { TaskStore } = await import("@kb/core");
|
||||
(TaskStore as ReturnType<typeof vi.fn>).mockImplementation(() => mockStore);
|
||||
@@ -742,6 +1079,7 @@ describe("runDashboard — --dev mode", () => {
|
||||
beforeEach(async () => {
|
||||
capturedExecutorOpts = undefined;
|
||||
vi.clearAllMocks();
|
||||
resetGitHubMocks();
|
||||
mockStore = makeMockStore();
|
||||
const { TaskStore } = await import("@kb/core");
|
||||
(TaskStore as ReturnType<typeof vi.fn>).mockImplementation(() => mockStore);
|
||||
@@ -855,6 +1193,7 @@ describe("runDashboard — merge conflict retry logic", () => {
|
||||
beforeEach(async () => {
|
||||
capturedExecutorOpts = undefined;
|
||||
vi.clearAllMocks();
|
||||
resetGitHubMocks();
|
||||
mockStore = makeMockStore();
|
||||
const { TaskStore } = await import("@kb/core");
|
||||
(TaskStore as ReturnType<typeof vi.fn>).mockImplementation(() => mockStore);
|
||||
|
||||
@@ -1,9 +1,10 @@
|
||||
import { exec } from "node:child_process";
|
||||
import { exec, execSync } from "node:child_process";
|
||||
import type { AddressInfo } from "node:net";
|
||||
import { createInterface } from "node:readline";
|
||||
import { TaskStore } from "@kb/core";
|
||||
import { createServer } from "@kb/dashboard";
|
||||
import { TriageProcessor, TaskExecutor, Scheduler, AgentSemaphore, WorktreePool, aiMergeTask, UsageLimitPauser, PRIORITY_MERGE, scanIdleWorktrees, cleanupOrphanedWorktrees, NtfyNotifier } from "@kb/engine";
|
||||
import type { Settings, TaskDetail, PrInfo } from "@kb/core";
|
||||
import { createServer, GitHubClient } from "@kb/dashboard";
|
||||
import { TriageProcessor, TaskExecutor, Scheduler, AgentSemaphore, WorktreePool, aiMergeTask, UsageLimitPauser, PRIORITY_MERGE, scanIdleWorktrees, cleanupOrphanedWorktrees, NtfyNotifier, PrMonitor, PrCommentHandler } from "@kb/engine";
|
||||
import { AuthStorage, ModelRegistry } from "@mariozechner/pi-coding-agent";
|
||||
|
||||
function openBrowser(url: string): void {
|
||||
@@ -73,6 +74,124 @@ export function promptForPort(defaultPort: number = 4040, input: NodeJS.Readable
|
||||
});
|
||||
}
|
||||
|
||||
export function getMergeStrategy(settings: Pick<Settings, "mergeStrategy">): NonNullable<Settings["mergeStrategy"]> {
|
||||
return settings.mergeStrategy ?? "direct";
|
||||
}
|
||||
|
||||
export function getTaskBranchName(taskId: string): string {
|
||||
return `kb/${taskId.toLowerCase()}`;
|
||||
}
|
||||
|
||||
function buildPullRequestTitle(task: Pick<TaskDetail, "id" | "title">): string {
|
||||
return task.title ? `${task.id}: ${task.title}` : task.id;
|
||||
}
|
||||
|
||||
function buildPullRequestBody(task: Pick<TaskDetail, "id" | "description">): string {
|
||||
return [`Automated PR for ${task.id}.`, "", task.description].join("\n");
|
||||
}
|
||||
|
||||
function cleanupMergedTaskArtifacts(cwd: string, task: Pick<TaskDetail, "id" | "worktree">): void {
|
||||
const branch = getTaskBranchName(task.id);
|
||||
|
||||
if (task.worktree) {
|
||||
try {
|
||||
execSync(`git worktree remove \"${task.worktree}\" --force`, {
|
||||
cwd,
|
||||
stdio: "pipe",
|
||||
});
|
||||
} catch {
|
||||
// Best-effort cleanup — worktree may already be gone.
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
execSync(`git branch -d \"${branch}\"`, {
|
||||
cwd,
|
||||
stdio: "pipe",
|
||||
});
|
||||
} catch {
|
||||
try {
|
||||
execSync(`git branch -D \"${branch}\"`, {
|
||||
cwd,
|
||||
stdio: "pipe",
|
||||
});
|
||||
} catch {
|
||||
// Best-effort cleanup — branch may already be gone.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export async function processPullRequestMergeTask(
|
||||
store: TaskStore,
|
||||
cwd: string,
|
||||
taskId: string,
|
||||
github: Pick<GitHubClient, "findPrForBranch" | "createPr" | "getPrMergeStatus" | "mergePr">,
|
||||
): Promise<"waiting" | "merged" | "skipped"> {
|
||||
const task = await store.getTask(taskId);
|
||||
if (task.column !== "in-review" || task.paused) {
|
||||
return "skipped";
|
||||
}
|
||||
|
||||
const branch = getTaskBranchName(task.id);
|
||||
let prInfo: PrInfo | undefined = task.prInfo;
|
||||
|
||||
if (!prInfo) {
|
||||
await store.updateTask(task.id, { status: "creating-pr" });
|
||||
|
||||
const existingPr = await github.findPrForBranch({ head: branch, state: "all" });
|
||||
prInfo = existingPr ?? await github.createPr({
|
||||
title: buildPullRequestTitle(task),
|
||||
body: buildPullRequestBody(task),
|
||||
head: branch,
|
||||
});
|
||||
|
||||
await store.updatePrInfo(task.id, prInfo);
|
||||
await store.logEntry(
|
||||
task.id,
|
||||
existingPr ? "Linked existing PR" : "Created PR",
|
||||
`PR #${prInfo.number}: ${prInfo.url}`,
|
||||
);
|
||||
}
|
||||
|
||||
if (!prInfo) {
|
||||
throw new Error(`Failed to create or resolve pull request for ${task.id}`);
|
||||
}
|
||||
|
||||
const mergeStatus = await github.getPrMergeStatus(undefined, undefined, prInfo.number);
|
||||
const refreshedPrInfo: PrInfo = {
|
||||
...prInfo,
|
||||
...mergeStatus.prInfo,
|
||||
lastCheckedAt: new Date().toISOString(),
|
||||
};
|
||||
await store.updatePrInfo(task.id, refreshedPrInfo);
|
||||
|
||||
if (mergeStatus.prInfo.status === "merged") {
|
||||
cleanupMergedTaskArtifacts(cwd, task);
|
||||
await store.moveTask(task.id, "done");
|
||||
await store.updateTask(task.id, { status: null, mergeRetries: 0 });
|
||||
await store.logEntry(task.id, "Pull request merged", `PR #${prInfo.number}: ${prInfo.url}`);
|
||||
return "merged";
|
||||
}
|
||||
|
||||
if (!mergeStatus.mergeReady) {
|
||||
if (mergeStatus.prInfo.status === "open") {
|
||||
await store.updateTask(task.id, { status: "awaiting-pr-checks" });
|
||||
} else {
|
||||
await store.updateTask(task.id, { status: null });
|
||||
}
|
||||
return "waiting";
|
||||
}
|
||||
|
||||
await store.updateTask(task.id, { status: "merging-pr" });
|
||||
const mergedPr = await github.mergePr({ number: prInfo.number, method: "squash" });
|
||||
await store.updatePrInfo(task.id, { ...mergedPr, lastCheckedAt: new Date().toISOString() });
|
||||
cleanupMergedTaskArtifacts(cwd, task);
|
||||
await store.moveTask(task.id, "done");
|
||||
await store.updateTask(task.id, { status: null, mergeRetries: 0 });
|
||||
await store.logEntry(task.id, "Pull request merged", `PR #${mergedPr.number}: ${mergedPr.url}`);
|
||||
return "merged";
|
||||
}
|
||||
|
||||
export async function runDashboard(port: number, opts: { open?: boolean; paused?: boolean; dev?: boolean; interactive?: boolean } = {}) {
|
||||
// Handle interactive port selection
|
||||
let selectedPort = port;
|
||||
@@ -160,6 +279,7 @@ export async function runDashboard(port: number, opts: { open?: boolean; paused?
|
||||
// pause is deduplicated across concurrent agents.
|
||||
//
|
||||
const usageLimitPauser = new UsageLimitPauser(store);
|
||||
const githubClient = new GitHubClient(process.env.GITHUB_TOKEN);
|
||||
|
||||
// AI-powered merge handler (used by the web UI for manual merges).
|
||||
// Wrapped with the shared semaphore so merges count toward the global
|
||||
@@ -238,52 +358,71 @@ export async function runDashboard(port: number, opts: { open?: boolean; paused?
|
||||
if (task.column !== "in-review" || task.paused) {
|
||||
continue;
|
||||
}
|
||||
console.log(`[auto-merge] Merging ${taskId}...`);
|
||||
await onMerge(taskId);
|
||||
console.log(`[auto-merge] ✓ ${taskId} merged`);
|
||||
// Clear mergeRetries on success
|
||||
if (task.mergeRetries && task.mergeRetries > 0) {
|
||||
await store.updateTask(taskId, { mergeRetries: 0 });
|
||||
const mergeStrategy = getMergeStrategy(settings);
|
||||
if (mergeStrategy === "pull-request") {
|
||||
console.log(`[auto-merge] Processing PR flow for ${taskId}...`);
|
||||
const result = await processPullRequestMergeTask(store, cwd, taskId, githubClient);
|
||||
if (result === "merged") {
|
||||
console.log(`[auto-merge] ✓ ${taskId} merged via pull request`);
|
||||
} else if (result === "waiting") {
|
||||
console.log(`[auto-merge] … ${taskId} waiting on PR checks or reviews`);
|
||||
}
|
||||
} else {
|
||||
console.log(`[auto-merge] Merging ${taskId}...`);
|
||||
await onMerge(taskId);
|
||||
console.log(`[auto-merge] ✓ ${taskId} merged`);
|
||||
// Clear mergeRetries on success
|
||||
if (task.mergeRetries && task.mergeRetries > 0) {
|
||||
await store.updateTask(taskId, { mergeRetries: 0 });
|
||||
}
|
||||
}
|
||||
} catch (err: any) {
|
||||
const errorMsg = err.message ?? String(err);
|
||||
console.log(`[auto-merge] ✗ ${taskId}: ${errorMsg}`);
|
||||
|
||||
// Check if this is a conflict error and if we should retry
|
||||
const isConflictError = errorMsg.includes("conflict") || errorMsg.includes("Conflict");
|
||||
const settings = await store.getSettings().catch(() => ({ autoResolveConflicts: true, mergeStrategy: "direct" as const }));
|
||||
const task = await store.getTask(taskId).catch(() => null);
|
||||
const mergeStrategy = getMergeStrategy(settings);
|
||||
|
||||
if (task && isConflictError) {
|
||||
const settings = await store.getSettings().catch(() => ({ autoResolveConflicts: true }));
|
||||
const currentRetries = task.mergeRetries ?? 0;
|
||||
const maxRetries = 3;
|
||||
if (mergeStrategy === "direct") {
|
||||
// Check if this is a conflict error and if we should retry
|
||||
const isConflictError = errorMsg.includes("conflict") || errorMsg.includes("Conflict");
|
||||
|
||||
if (settings.autoResolveConflicts !== false && currentRetries < maxRetries) {
|
||||
// Increment retry counter and re-enqueue with delay
|
||||
const newRetryCount = currentRetries + 1;
|
||||
await store.updateTask(taskId, { mergeRetries: newRetryCount, status: null });
|
||||
if (task && isConflictError) {
|
||||
const currentRetries = task.mergeRetries ?? 0;
|
||||
const maxRetries = 3;
|
||||
|
||||
// Calculate exponential backoff delay: 5s, 10s, 20s
|
||||
const delayMs = 5000 * Math.pow(2, currentRetries);
|
||||
console.log(`[auto-merge] ↻ ${taskId}: retry ${newRetryCount}/${maxRetries} in ${delayMs / 1000}s`);
|
||||
if (settings.autoResolveConflicts !== false && currentRetries < maxRetries) {
|
||||
// Increment retry counter and re-enqueue with delay
|
||||
const newRetryCount = currentRetries + 1;
|
||||
await store.updateTask(taskId, { mergeRetries: newRetryCount, status: null });
|
||||
|
||||
setTimeout(() => {
|
||||
enqueueMerge(taskId);
|
||||
}, delayMs);
|
||||
} else {
|
||||
// Max retries exceeded or auto-resolve disabled - keep in in-review
|
||||
if (currentRetries >= maxRetries) {
|
||||
console.log(`[auto-merge] ⊘ ${taskId}: max retries (${maxRetries}) exceeded — manual resolution required`);
|
||||
// Calculate exponential backoff delay: 5s, 10s, 20s
|
||||
const delayMs = 5000 * Math.pow(2, currentRetries);
|
||||
console.log(`[auto-merge] ↻ ${taskId}: retry ${newRetryCount}/${maxRetries} in ${delayMs / 1000}s`);
|
||||
|
||||
setTimeout(() => {
|
||||
enqueueMerge(taskId);
|
||||
}, delayMs);
|
||||
} else {
|
||||
console.log(`[auto-merge] ⊘ ${taskId}: autoResolveConflicts disabled — manual resolution required`);
|
||||
// Max retries exceeded or auto-resolve disabled - keep in in-review
|
||||
if (currentRetries >= maxRetries) {
|
||||
console.log(`[auto-merge] ⊘ ${taskId}: max retries (${maxRetries}) exceeded — manual resolution required`);
|
||||
} else {
|
||||
console.log(`[auto-merge] ⊘ ${taskId}: autoResolveConflicts disabled — manual resolution required`);
|
||||
}
|
||||
// Reset task status so it doesn't appear stuck as "merging" in the UI
|
||||
try {
|
||||
await store.updateTask(taskId, { status: null });
|
||||
} catch { /* best-effort */ }
|
||||
}
|
||||
// Reset task status so it doesn't appear stuck as "merging" in the UI
|
||||
} else {
|
||||
// Non-conflict error - reset task status
|
||||
try {
|
||||
await store.updateTask(taskId, { status: null });
|
||||
} catch { /* best-effort */ }
|
||||
}
|
||||
} else {
|
||||
// Non-conflict error - reset task status
|
||||
try {
|
||||
await store.updateTask(taskId, { status: null });
|
||||
} catch { /* best-effort */ }
|
||||
@@ -341,9 +480,15 @@ export async function runDashboard(port: number, opts: { open?: boolean; paused?
|
||||
});
|
||||
|
||||
const settings = await store.getSettings();
|
||||
const prMonitor = new PrMonitor();
|
||||
const prCommentHandler = new PrCommentHandler(store);
|
||||
prMonitor.onNewComments((taskId, prInfo, comments) =>
|
||||
prCommentHandler.handleNewComments(taskId, prInfo, comments),
|
||||
);
|
||||
|
||||
const scheduler = new Scheduler(store, {
|
||||
semaphore,
|
||||
prMonitor,
|
||||
onSchedule: (t) => console.log(`[engine] Scheduled ${t.id}`),
|
||||
onBlocked: (t, deps) => console.log(`[engine] ${t.id} blocked by ${deps.join(", ")}`),
|
||||
});
|
||||
|
||||
@@ -384,6 +384,19 @@ describe("TaskStore", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("mergeStrategy setting", () => {
|
||||
it("defaults mergeStrategy to direct for backward compatibility", async () => {
|
||||
const settings = await store.getSettings();
|
||||
expect(settings.mergeStrategy).toBe("direct");
|
||||
});
|
||||
|
||||
it("persists mergeStrategy and returns it via getSettings", async () => {
|
||||
await store.updateSettings({ mergeStrategy: "pull-request" });
|
||||
const settings = await store.getSettings();
|
||||
expect(settings.mergeStrategy).toBe("pull-request");
|
||||
});
|
||||
});
|
||||
|
||||
// ── Concurrent stress test ───────────────────────────────────────
|
||||
|
||||
describe("concurrent stress", () => {
|
||||
|
||||
@@ -23,6 +23,7 @@ export const COLOR_THEMES = [
|
||||
export type ColorTheme = (typeof COLOR_THEMES)[number];
|
||||
|
||||
export type PrStatus = "open" | "closed" | "merged";
|
||||
export type MergeStrategy = "direct" | "pull-request";
|
||||
|
||||
export interface PrInfo {
|
||||
url: string;
|
||||
@@ -195,6 +196,12 @@ export interface Settings {
|
||||
pollIntervalMs: number;
|
||||
groupOverlappingFiles: boolean;
|
||||
autoMerge: boolean;
|
||||
/** How completed in-review tasks should be finalized when autoMerge is enabled.
|
||||
* - "direct": preserve the existing local squash-merge flow into the current branch
|
||||
* - "pull-request": create or reuse a GitHub PR and wait for GitHub-side checks/reviews
|
||||
* before merging through GitHub
|
||||
* Default: "direct" for backward compatibility. */
|
||||
mergeStrategy?: MergeStrategy;
|
||||
/** Shell command to run inside each new worktree immediately after creation.
|
||||
* Useful for project-specific setup (e.g. `pnpm install`, `cp .env.local .env`). */
|
||||
worktreeInitCommand?: string;
|
||||
@@ -271,6 +278,7 @@ export const DEFAULT_SETTINGS: Settings = {
|
||||
pollIntervalMs: 15000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: true,
|
||||
mergeStrategy: "direct",
|
||||
worktreeInitCommand: undefined,
|
||||
recycleWorktrees: false,
|
||||
worktreeNaming: "random",
|
||||
|
||||
@@ -43,7 +43,7 @@ AI-guided interactive planning for creating well-specified tasks from high-level
|
||||
- **List View**: Alternative tabular view for tasks with sorting and filtering. The "Hide Done" toggle hides both Done and Archived tasks for an active-work-only view.
|
||||
- **Task Details**: View full task specifications, agent logs, and attachments
|
||||
- **GitHub Import**: Import issues directly from GitHub repositories
|
||||
- **PR Management**: Create and track pull requests for in-review tasks
|
||||
- **PR Management**: Create, monitor, and merge pull requests for in-review tasks
|
||||
|
||||
### Interactive Terminal
|
||||
Access a fully functional shell terminal directly from the dashboard. Click the terminal icon in the header to open the interactive terminal modal.
|
||||
@@ -142,6 +142,28 @@ Browse and edit task worktree files directly from the task detail modal:
|
||||
- **Pause Controls**: Soft pause (stop new work) and hard stop (kill all agents)
|
||||
- **Theming**: Light/dark/system mode toggle and 8 color themes (see Theming section below)
|
||||
|
||||
### Merge strategies
|
||||
|
||||
The dashboard exposes two automated completion strategies in Settings:
|
||||
|
||||
- **Direct merge** *(default)* — preserves existing behavior. When `autoMerge` is enabled, kb merges in-review tasks locally.
|
||||
- **Pull request** — when `autoMerge` is enabled, kb creates or links a PR for the task branch, keeps the task in **In Review** while waiting on GitHub policy, and merges the PR when it is ready.
|
||||
|
||||
`autoMerge` is still the master switch for automation. Turning it off disables both direct merge and PR-first auto-completion.
|
||||
|
||||
### PR-first workflow notes
|
||||
|
||||
When the merge strategy is **Pull request**:
|
||||
|
||||
- The task's PR section shows whether kb is waiting on checks/reviews or has merged successfully
|
||||
- Required checks must pass before kb merges the PR; optional checks do not block auto-merge
|
||||
- A blocking review state (for example, active changes requested) prevents auto-merge until cleared
|
||||
- Closed PRs do not auto-merge
|
||||
- GitHub access must be available via `gh auth login` or `GITHUB_TOKEN`
|
||||
- kb expects the task branch to already be pushed using the standard branch name `kb/<task-id-lower>`
|
||||
|
||||
**Non-goal:** the dashboard does not implicitly push branches before PR creation. Use your normal git workflow or automation to publish task branches first.
|
||||
|
||||
## Theming
|
||||
|
||||
The dashboard supports a comprehensive theming system with both light/dark mode and color theme options.
|
||||
|
||||
@@ -291,6 +291,27 @@ export interface PrInfo {
|
||||
lastCheckedAt?: string;
|
||||
}
|
||||
|
||||
export interface PrCheckStatus {
|
||||
name: string;
|
||||
required: boolean;
|
||||
state: string;
|
||||
}
|
||||
|
||||
export interface PrStatusResponse {
|
||||
prInfo: PrInfo;
|
||||
stale: boolean;
|
||||
automationStatus?: string | null;
|
||||
}
|
||||
|
||||
export interface PrRefreshResponse {
|
||||
prInfo: PrInfo;
|
||||
mergeReady: boolean;
|
||||
blockingReasons: string[];
|
||||
reviewDecision: "APPROVED" | "CHANGES_REQUESTED" | "REVIEW_REQUIRED" | null;
|
||||
checks: PrCheckStatus[];
|
||||
automationStatus?: string | null;
|
||||
}
|
||||
|
||||
/** Create a GitHub PR for a task */
|
||||
export function createPr(
|
||||
id: string,
|
||||
@@ -303,13 +324,13 @@ export function createPr(
|
||||
}
|
||||
|
||||
/** 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`);
|
||||
export function fetchPrStatus(id: string): Promise<PrStatusResponse> {
|
||||
return api<PrStatusResponse>(`/tasks/${id}/pr/status`);
|
||||
}
|
||||
|
||||
/** Force refresh PR status from GitHub */
|
||||
export function refreshPrStatus(id: string): Promise<PrInfo> {
|
||||
return api<PrInfo>(`/tasks/${id}/pr/refresh`, {
|
||||
export function refreshPrStatus(id: string): Promise<PrRefreshResponse> {
|
||||
return api<PrRefreshResponse>(`/tasks/${id}/pr/refresh`, {
|
||||
method: "POST",
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1,12 +1,13 @@
|
||||
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 { createPr, refreshPrStatus, type PrRefreshResponse } from "../api";
|
||||
import type { ToastType } from "../hooks/useToast";
|
||||
|
||||
interface PrSectionProps {
|
||||
taskId: string;
|
||||
prInfo?: PrInfo;
|
||||
automationStatus?: string | null;
|
||||
hasGitHubToken: boolean;
|
||||
onPrCreated: (prInfo: PrInfo) => void;
|
||||
onPrUpdated: (prInfo: PrInfo) => void;
|
||||
@@ -22,6 +23,7 @@ const STATUS_COLORS = {
|
||||
export function PrSection({
|
||||
taskId,
|
||||
prInfo,
|
||||
automationStatus,
|
||||
hasGitHubToken,
|
||||
onPrCreated,
|
||||
onPrUpdated,
|
||||
@@ -32,6 +34,7 @@ export function PrSection({
|
||||
const [prBody, setPrBody] = useState("");
|
||||
const [isCreating, setIsCreating] = useState(false);
|
||||
const [isRefreshing, setIsRefreshing] = useState(false);
|
||||
const [refreshState, setRefreshState] = useState<PrRefreshResponse | null>(null);
|
||||
|
||||
const handleCreate = useCallback(async () => {
|
||||
if (!prTitle.trim()) return;
|
||||
@@ -60,7 +63,8 @@ export function PrSection({
|
||||
setIsRefreshing(true);
|
||||
try {
|
||||
const updated = await refreshPrStatus(taskId);
|
||||
onPrUpdated(updated);
|
||||
setRefreshState(updated);
|
||||
onPrUpdated(updated.prInfo);
|
||||
addToast("PR status refreshed", "success");
|
||||
} catch (err: any) {
|
||||
addToast(err.message || "Failed to refresh PR", "error");
|
||||
@@ -69,8 +73,22 @@ export function PrSection({
|
||||
}
|
||||
}, [taskId, prInfo, onPrUpdated, addToast]);
|
||||
|
||||
// No PR yet - show create button
|
||||
// No PR yet - show create button or automation state
|
||||
if (!prInfo) {
|
||||
if (automationStatus === "creating-pr") {
|
||||
return (
|
||||
<div className="pr-section">
|
||||
<h4>
|
||||
<GitPullRequest size={16} style={{ verticalAlign: "middle", marginRight: 8 }} />
|
||||
Pull Request
|
||||
</h4>
|
||||
<div className="pr-hint" style={{ opacity: 0.8, fontSize: 13 }}>
|
||||
kb is creating a pull request automatically for this task.
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
if (showCreateForm) {
|
||||
return (
|
||||
<div className="pr-section">
|
||||
@@ -142,6 +160,7 @@ export function PrSection({
|
||||
|
||||
// PR exists - show PR card
|
||||
const statusStyle = STATUS_COLORS[prInfo.status];
|
||||
const blockingReasons = refreshState?.blockingReasons ?? [];
|
||||
|
||||
return (
|
||||
<div className="pr-section">
|
||||
@@ -196,6 +215,23 @@ export function PrSection({
|
||||
<span style={{ margin: "0 8px" }}>→</span>
|
||||
<span>{prInfo.baseBranch}</span>
|
||||
</div>
|
||||
{automationStatus === "merging-pr" && (
|
||||
<div className="pr-hint" style={{ marginBottom: 8, fontSize: 12 }}>
|
||||
kb is merging this pull request automatically.
|
||||
</div>
|
||||
)}
|
||||
{automationStatus === "awaiting-pr-checks" && (
|
||||
<div className="pr-hint" style={{ marginBottom: 8, fontSize: 12 }}>
|
||||
{blockingReasons.length > 0
|
||||
? `Waiting for: ${blockingReasons.join("; ")}`
|
||||
: "Waiting for required checks or review feedback before auto-merge."}
|
||||
</div>
|
||||
)}
|
||||
{prInfo.status === "merged" && (
|
||||
<div className="pr-hint" style={{ marginBottom: 8, fontSize: 12 }}>
|
||||
This PR is merged. kb will finish local cleanup and move the task to Done.
|
||||
</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 }}>
|
||||
|
||||
@@ -64,7 +64,7 @@ export function SettingsModal({
|
||||
onThemeModeChange,
|
||||
onColorThemeChange,
|
||||
}: SettingsModalProps) {
|
||||
const [form, setForm] = useState<Settings & { worktreeInitCommand?: string }>({ maxConcurrent: 2, maxWorktrees: 4, pollIntervalMs: 15000, groupOverlappingFiles: false, autoMerge: true, recycleWorktrees: false, worktreeNaming: "random", includeTaskIdInCommit: true, worktreeInitCommand: "" });
|
||||
const [form, setForm] = useState<Settings & { worktreeInitCommand?: string }>({ maxConcurrent: 2, maxWorktrees: 4, pollIntervalMs: 15000, groupOverlappingFiles: false, autoMerge: true, mergeStrategy: "direct", recycleWorktrees: false, worktreeNaming: "random", includeTaskIdInCommit: true, worktreeInitCommand: "" });
|
||||
const [loading, setLoading] = useState(true);
|
||||
const [activeSection, setActiveSection] = useState<SectionId>(initialSection ?? SETTINGS_SECTIONS[0].id);
|
||||
const [prefixError, setPrefixError] = useState<string | null>(null);
|
||||
@@ -528,6 +528,22 @@ export function SettingsModal({
|
||||
</label>
|
||||
<small>When enabled, tasks that pass review are automatically merged into the main branch</small>
|
||||
</div>
|
||||
<div className="form-group">
|
||||
<label htmlFor="mergeStrategy">Auto-completion mode</label>
|
||||
<select
|
||||
id="mergeStrategy"
|
||||
value={form.mergeStrategy || "direct"}
|
||||
onChange={(e) =>
|
||||
setForm((f) => ({ ...f, mergeStrategy: e.target.value as Settings["mergeStrategy"] }))
|
||||
}
|
||||
>
|
||||
<option value="direct">Direct merge into the current branch</option>
|
||||
<option value="pull-request">Create, monitor, and merge a GitHub pull request</option>
|
||||
</select>
|
||||
<small>
|
||||
Controls what happens after a task reaches In Review. Direct mode preserves kb's current local squash-merge behavior. Pull request mode keeps the task in In Review while kb waits for GitHub reviews and required checks before merging the PR.
|
||||
</small>
|
||||
</div>
|
||||
<div className="form-group">
|
||||
<label htmlFor="includeTaskIdInCommit" className="checkbox-label">
|
||||
<input
|
||||
|
||||
@@ -386,6 +386,12 @@ export function TaskDetailModal({
|
||||
});
|
||||
|
||||
const transitions = VALID_TRANSITIONS[task.column] || [];
|
||||
const prAutomationStatusLabels: Record<string, string> = {
|
||||
"creating-pr": "Creating PR…",
|
||||
"awaiting-pr-checks": "Awaiting PR checks",
|
||||
"merging-pr": "Merging PR…",
|
||||
};
|
||||
const prAutomationLabel = task.status ? prAutomationStatusLabels[task.status] : undefined;
|
||||
|
||||
return (
|
||||
<div className="modal-overlay open" onClick={handleOverlayClick}>
|
||||
@@ -716,6 +722,7 @@ export function TaskDetailModal({
|
||||
<PrSection
|
||||
taskId={task.id}
|
||||
prInfo={task.prInfo}
|
||||
automationStatus={task.status ?? null}
|
||||
hasGitHubToken={githubTokenConfigured ?? false}
|
||||
onPrCreated={(prInfo) => {
|
||||
// Update task locally to show new PR
|
||||
@@ -772,9 +779,15 @@ export function TaskDetailModal({
|
||||
<button className="btn btn-sm" onClick={() => handleMove("in-progress")}>
|
||||
Back to In Progress
|
||||
</button>
|
||||
<button className="btn btn-primary btn-sm" onClick={handleMerge}>
|
||||
Merge & Close
|
||||
</button>
|
||||
{prAutomationLabel ? (
|
||||
<button className="btn btn-primary btn-sm" disabled>
|
||||
{prAutomationLabel}
|
||||
</button>
|
||||
) : (
|
||||
<button className="btn btn-primary btn-sm" onClick={handleMerge}>
|
||||
Merge & Close
|
||||
</button>
|
||||
)}
|
||||
</>
|
||||
) : (
|
||||
transitions.map((col) => (
|
||||
|
||||
@@ -183,6 +183,7 @@ describe("PrSection", () => {
|
||||
);
|
||||
|
||||
expect(screen.getByText("merged")).toBeDefined();
|
||||
expect(screen.getByText(/finish local cleanup and move the task to Done/i)).toBeDefined();
|
||||
});
|
||||
|
||||
it("shows correct status badge for closed PR", () => {
|
||||
@@ -218,7 +219,14 @@ describe("PrSection", () => {
|
||||
|
||||
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);
|
||||
(refreshPrStatus as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
prInfo: updatedPr,
|
||||
mergeReady: true,
|
||||
blockingReasons: [],
|
||||
reviewDecision: "APPROVED",
|
||||
checks: [{ name: "ci", required: true, state: "success" }],
|
||||
automationStatus: null,
|
||||
});
|
||||
|
||||
render(
|
||||
<PrSection
|
||||
@@ -263,5 +271,50 @@ describe("PrSection", () => {
|
||||
expect(mockAddToast).toHaveBeenCalledWith("Network error", "error");
|
||||
});
|
||||
});
|
||||
|
||||
it("shows automatic PR creation message while PR-first automation is creating a PR", () => {
|
||||
render(
|
||||
<PrSection
|
||||
taskId="KB-001"
|
||||
automationStatus="creating-pr"
|
||||
hasGitHubToken={true}
|
||||
onPrCreated={mockOnPrCreated}
|
||||
onPrUpdated={mockOnPrUpdated}
|
||||
addToast={mockAddToast}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(screen.getByText(/creating a pull request automatically/i)).toBeDefined();
|
||||
expect(screen.queryByText("Create PR")).toBeNull();
|
||||
});
|
||||
|
||||
it("shows awaiting-checks messaging from refreshed merge blockers", async () => {
|
||||
(refreshPrStatus as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
prInfo: mockPrInfo,
|
||||
mergeReady: false,
|
||||
blockingReasons: ["required checks not successful: ci (pending)"],
|
||||
reviewDecision: null,
|
||||
checks: [{ name: "ci", required: true, state: "pending" }],
|
||||
automationStatus: "awaiting-pr-checks",
|
||||
});
|
||||
|
||||
render(
|
||||
<PrSection
|
||||
taskId="KB-001"
|
||||
prInfo={mockPrInfo}
|
||||
automationStatus="awaiting-pr-checks"
|
||||
hasGitHubToken={true}
|
||||
onPrCreated={mockOnPrCreated}
|
||||
onPrUpdated={mockOnPrUpdated}
|
||||
addToast={mockAddToast}
|
||||
/>
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByTitle("Refresh PR status"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText(/Waiting for: required checks not successful: ci \(pending\)/)).toBeDefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -10,6 +10,7 @@ const defaultSettings: Settings = {
|
||||
pollIntervalMs: 15000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: true,
|
||||
mergeStrategy: "direct",
|
||||
recycleWorktrees: false,
|
||||
worktreeInitCommand: "",
|
||||
testCommand: "",
|
||||
@@ -107,6 +108,7 @@ describe("SettingsModal", () => {
|
||||
// Merge
|
||||
fireEvent.click(screen.getByText("Merge"));
|
||||
expect(screen.getByText("Auto-merge completed tasks")).toBeTruthy();
|
||||
expect(screen.getByLabelText("Auto-completion mode")).toBeTruthy();
|
||||
expect(screen.getByText("Include task ID in commit scope")).toBeTruthy();
|
||||
expect(screen.getByText("Auto-resolve conflicts in lock files and generated files")).toBeTruthy();
|
||||
expect(screen.getByText("Smart conflict resolution")).toBeTruthy();
|
||||
@@ -188,6 +190,31 @@ describe("SettingsModal", () => {
|
||||
expect(updateSettings).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("shows Auto-completion mode select in Merge section", async () => {
|
||||
render(<SettingsModal onClose={onClose} addToast={addToast} />);
|
||||
await waitFor(() => expect(fetchSettings).toHaveBeenCalled());
|
||||
|
||||
fireEvent.click(screen.getByText("Merge"));
|
||||
const select = screen.getByLabelText("Auto-completion mode") as HTMLSelectElement;
|
||||
expect(select).toBeTruthy();
|
||||
expect(select.value).toBe("direct");
|
||||
});
|
||||
|
||||
it("saves pull-request mergeStrategy when selected", async () => {
|
||||
render(<SettingsModal onClose={onClose} addToast={addToast} />);
|
||||
await waitFor(() => expect(fetchSettings).toHaveBeenCalled());
|
||||
|
||||
fireEvent.click(screen.getByText("Merge"));
|
||||
const select = screen.getByLabelText("Auto-completion mode") as HTMLSelectElement;
|
||||
fireEvent.change(select, { target: { value: "pull-request" } });
|
||||
|
||||
fireEvent.click(screen.getByText("Save"));
|
||||
await waitFor(() => expect(updateSettings).toHaveBeenCalledTimes(1));
|
||||
|
||||
const payload = (updateSettings as ReturnType<typeof vi.fn>).mock.calls[0][0];
|
||||
expect(payload.mergeStrategy).toBe("pull-request");
|
||||
});
|
||||
|
||||
it("shows Include task ID in commit scope checkbox in Merge section", async () => {
|
||||
render(<SettingsModal onClose={onClose} addToast={addToast} />);
|
||||
await waitFor(() => expect(fetchSettings).toHaveBeenCalled());
|
||||
|
||||
@@ -1233,6 +1233,50 @@ describe("TaskDetailModal", () => {
|
||||
expect(screen.getByText("Merge & Close")).toBeTruthy();
|
||||
expect(screen.getByText("Back to In Progress")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("shows PR automation waiting label instead of Merge & Close when awaiting PR checks", () => {
|
||||
render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({ column: "in-review" as Column, status: "awaiting-pr-checks", prInfo: {
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
number: 42,
|
||||
status: "open",
|
||||
title: "Task",
|
||||
headBranch: "kb/kb-099",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
} })}
|
||||
onClose={noop}
|
||||
onMoveTask={noopMove}
|
||||
onDeleteTask={noopDelete}
|
||||
onMergeTask={noopMerge}
|
||||
onOpenDetail={noopOpenDetail}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
const button = screen.getByText("Awaiting PR checks") as HTMLButtonElement;
|
||||
expect(button.disabled).toBe(true);
|
||||
expect(screen.queryByText("Merge & Close")).toBeNull();
|
||||
});
|
||||
|
||||
it("shows Creating PR label while PR-first automation is creating a PR", () => {
|
||||
render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({ column: "in-review" as Column, status: "creating-pr" })}
|
||||
onClose={noop}
|
||||
onMoveTask={noopMove}
|
||||
onDeleteTask={noopDelete}
|
||||
onMergeTask={noopMerge}
|
||||
onOpenDetail={noopOpenDetail}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
const button = screen.getByText("Creating PR…") as HTMLButtonElement;
|
||||
expect(button.disabled).toBe(true);
|
||||
expect(screen.queryByText("Merge & Close")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("dependency dropdown search", () => {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { GitHubClient, CreatePrParams, PrComment } from "./github.js";
|
||||
import { GitHubClient, CreatePrParams, PrComment, isPrMergeReady } from "./github.js";
|
||||
|
||||
// Mock the gh-cli module from @kb/core
|
||||
vi.mock("@kb/core", async () => {
|
||||
@@ -554,6 +554,242 @@ describe("GitHubClient", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("findPrForBranch", () => {
|
||||
it("finds an existing PR for a head branch via gh CLI", async () => {
|
||||
mockRunGhJsonAsync.mockResolvedValue([
|
||||
{
|
||||
number: 42,
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
title: "Existing PR",
|
||||
state: "OPEN",
|
||||
baseRefName: "main",
|
||||
headRefName: "kb/kb-093",
|
||||
mergedAt: null,
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await client.findPrForBranch({ owner: "owner", repo: "repo", head: "kb/kb-093", state: "all" });
|
||||
|
||||
expect(mockRunGhJsonAsync).toHaveBeenCalledWith([
|
||||
"pr", "list",
|
||||
"--repo", "owner/repo",
|
||||
"--head", "kb/kb-093",
|
||||
"--state", "all",
|
||||
"--json", "number,url,title,state,baseRefName,headRefName,mergedAt",
|
||||
]);
|
||||
expect(result).toEqual(expect.objectContaining({ number: 42, status: "open" }));
|
||||
});
|
||||
|
||||
it("falls back to REST API for branch lookup when gh CLI fails and token is available", async () => {
|
||||
mockRunGhJsonAsync.mockRejectedValue(new Error("gh failed"));
|
||||
const clientWithToken = new GitHubClient("ghp_token");
|
||||
const mockFetch = vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: () => Promise.resolve([
|
||||
{
|
||||
number: 5,
|
||||
html_url: "https://github.com/owner/repo/pull/5",
|
||||
title: "API PR",
|
||||
state: "open",
|
||||
merged_at: null,
|
||||
head: { ref: "kb/kb-093" },
|
||||
base: { ref: "main" },
|
||||
comments: 2,
|
||||
},
|
||||
]),
|
||||
});
|
||||
global.fetch = mockFetch as any;
|
||||
|
||||
const result = await clientWithToken.findPrForBranch({ owner: "owner", repo: "repo", head: "kb/kb-093" });
|
||||
|
||||
expect(mockFetch).toHaveBeenCalled();
|
||||
expect(result).toEqual(expect.objectContaining({ number: 5, commentCount: 2 }));
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
});
|
||||
|
||||
describe("getPrMergeStatus", () => {
|
||||
it("returns merge-ready status only when required checks pass and review is non-blocking", async () => {
|
||||
mockRunGhJsonAsync
|
||||
.mockResolvedValueOnce({
|
||||
number: 42,
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
title: "Ready PR",
|
||||
state: "OPEN",
|
||||
reviewDecision: "APPROVED",
|
||||
baseRefName: "main",
|
||||
headRefName: "kb/kb-093",
|
||||
})
|
||||
.mockResolvedValueOnce([
|
||||
{ name: "ci", state: "SUCCESS" },
|
||||
{ name: "lint", state: "SUCCESS" },
|
||||
]);
|
||||
|
||||
const result = await client.getPrMergeStatus("owner", "repo", 42);
|
||||
|
||||
expect(result.mergeReady).toBe(true);
|
||||
expect(result.blockingReasons).toEqual([]);
|
||||
expect(result.checks).toEqual([
|
||||
{ name: "ci", required: true, state: "success" },
|
||||
{ name: "lint", required: true, state: "success" },
|
||||
]);
|
||||
});
|
||||
|
||||
it("falls back to GraphQL API when gh CLI merge-status lookup fails and token is available", async () => {
|
||||
mockRunGhJsonAsync.mockRejectedValue(new Error("gh failed"));
|
||||
const clientWithToken = new GitHubClient("ghp_token");
|
||||
const mockFetch = vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: () => Promise.resolve({
|
||||
data: {
|
||||
repository: {
|
||||
pullRequest: {
|
||||
number: 42,
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
title: "Fallback PR",
|
||||
state: "OPEN",
|
||||
reviewDecision: null,
|
||||
baseRefName: "main",
|
||||
headRefName: "kb/kb-093",
|
||||
comments: { totalCount: 0 },
|
||||
commits: {
|
||||
nodes: [
|
||||
{
|
||||
commit: {
|
||||
statusCheckRollup: {
|
||||
contexts: {
|
||||
nodes: [
|
||||
{
|
||||
__typename: "CheckRun",
|
||||
name: "ci",
|
||||
status: "COMPLETED",
|
||||
conclusion: "SUCCESS",
|
||||
isRequired: true,
|
||||
},
|
||||
{
|
||||
__typename: "CheckRun",
|
||||
name: "optional-preview",
|
||||
status: "COMPLETED",
|
||||
conclusion: "FAILURE",
|
||||
isRequired: false,
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}),
|
||||
});
|
||||
global.fetch = mockFetch as any;
|
||||
|
||||
const result = await clientWithToken.getPrMergeStatus("owner", "repo", 42);
|
||||
|
||||
expect(result.mergeReady).toBe(true);
|
||||
expect(result.checks).toEqual([{ name: "ci", required: true, state: "success" }]);
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
});
|
||||
|
||||
describe("mergePr", () => {
|
||||
it("merges a PR with gh CLI and refetches merged status", async () => {
|
||||
mockRunGh.mockReturnValue("Merged pull request");
|
||||
mockRunGhJsonAsync.mockResolvedValue({
|
||||
number: 42,
|
||||
url: "https://github.com/owner/repo/pull/42",
|
||||
title: "Merged PR",
|
||||
state: "MERGED",
|
||||
baseRefName: "main",
|
||||
headRefName: "kb/kb-093",
|
||||
});
|
||||
|
||||
const result = await client.mergePr({ owner: "owner", repo: "repo", number: 42, method: "squash" });
|
||||
|
||||
expect(mockRunGh).toHaveBeenCalledWith([
|
||||
"pr", "merge", "42",
|
||||
"--repo", "owner/repo",
|
||||
"--squash",
|
||||
"--delete-branch",
|
||||
]);
|
||||
expect(result.status).toBe("merged");
|
||||
});
|
||||
|
||||
it("falls back to REST API merge when gh CLI fails and token is available", async () => {
|
||||
mockRunGh.mockImplementation(() => {
|
||||
throw new Error("gh failed");
|
||||
});
|
||||
const clientWithToken = new GitHubClient("ghp_token");
|
||||
const mockFetch = vi.fn()
|
||||
.mockResolvedValueOnce({ ok: true, json: () => Promise.resolve({ merged: true }) })
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () => Promise.resolve({
|
||||
number: 42,
|
||||
html_url: "https://github.com/owner/repo/pull/42",
|
||||
title: "Merged PR",
|
||||
state: "closed",
|
||||
merged: true,
|
||||
head: { ref: "kb/kb-093" },
|
||||
base: { ref: "main" },
|
||||
comments: 0,
|
||||
updated_at: "2024-01-01T00:00:00Z",
|
||||
}),
|
||||
});
|
||||
global.fetch = mockFetch as any;
|
||||
|
||||
const result = await clientWithToken.mergePr({ owner: "owner", repo: "repo", number: 42 });
|
||||
|
||||
expect(result.status).toBe("merged");
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
});
|
||||
|
||||
describe("isPrMergeReady", () => {
|
||||
it("blocks closed PRs", () => {
|
||||
expect(isPrMergeReady({ status: "closed", reviewDecision: null, checks: [] })).toEqual({
|
||||
ready: false,
|
||||
blockingReasons: ["PR is closed"],
|
||||
});
|
||||
});
|
||||
|
||||
it("blocks changes requested review even when checks pass", () => {
|
||||
expect(isPrMergeReady({
|
||||
status: "open",
|
||||
reviewDecision: "CHANGES_REQUESTED",
|
||||
checks: [{ name: "ci", required: true, state: "success" }],
|
||||
})).toEqual({
|
||||
ready: false,
|
||||
blockingReasons: ["changes requested review is active"],
|
||||
});
|
||||
});
|
||||
|
||||
it("blocks pending required checks", () => {
|
||||
expect(isPrMergeReady({
|
||||
status: "open",
|
||||
reviewDecision: null,
|
||||
checks: [{ name: "ci", required: true, state: "pending" }],
|
||||
})).toEqual({
|
||||
ready: false,
|
||||
blockingReasons: ["required checks not successful: ci (pending)"],
|
||||
});
|
||||
});
|
||||
|
||||
it("ignores optional checks when determining readiness", () => {
|
||||
expect(isPrMergeReady({
|
||||
status: "open",
|
||||
reviewDecision: "REVIEW_REQUIRED",
|
||||
checks: [
|
||||
{ name: "required-ci", required: true, state: "success" },
|
||||
{ name: "optional-preview", required: false, state: "failure" },
|
||||
],
|
||||
})).toEqual({ ready: true, blockingReasons: [] });
|
||||
});
|
||||
});
|
||||
|
||||
describe("error handling when gh CLI not available", () => {
|
||||
it("throws error when gh CLI not available and no token", async () => {
|
||||
mockIsGhAvailable.mockReturnValue(false);
|
||||
|
||||
@@ -2,7 +2,6 @@ import type { PrInfo } from "@kb/core";
|
||||
import {
|
||||
isGhAvailable,
|
||||
isGhAuthenticated,
|
||||
runGhJson,
|
||||
runGhJsonAsync,
|
||||
getGhErrorMessage,
|
||||
getCurrentRepo,
|
||||
@@ -27,12 +26,55 @@ export interface PrComment {
|
||||
html_url: string;
|
||||
}
|
||||
|
||||
export type ReviewDecision = "APPROVED" | "CHANGES_REQUESTED" | "REVIEW_REQUIRED" | null;
|
||||
export type PrCheckState =
|
||||
| "success"
|
||||
| "pending"
|
||||
| "failure"
|
||||
| "cancelled"
|
||||
| "timed_out"
|
||||
| "action_required"
|
||||
| "neutral"
|
||||
| "skipped"
|
||||
| "stale"
|
||||
| "startup_failure";
|
||||
|
||||
export interface PrCheckStatus {
|
||||
name: string;
|
||||
required: boolean;
|
||||
state: PrCheckState;
|
||||
}
|
||||
|
||||
export interface PrMergeStatus {
|
||||
prInfo: PrInfo;
|
||||
reviewDecision: ReviewDecision;
|
||||
checks: PrCheckStatus[];
|
||||
mergeReady: boolean;
|
||||
blockingReasons: string[];
|
||||
}
|
||||
|
||||
export interface FindPrParams {
|
||||
owner?: string;
|
||||
repo?: string;
|
||||
head: string;
|
||||
state?: "open" | "closed" | "all";
|
||||
}
|
||||
|
||||
export interface MergePrParams {
|
||||
owner?: string;
|
||||
repo?: string;
|
||||
number: number;
|
||||
method?: "merge" | "squash" | "rebase";
|
||||
}
|
||||
|
||||
// gh CLI JSON output types
|
||||
interface GhPrViewJson {
|
||||
id?: string;
|
||||
number: number;
|
||||
url: string;
|
||||
title: string;
|
||||
state: "OPEN" | "CLOSED" | "MERGED";
|
||||
reviewDecision?: ReviewDecision;
|
||||
baseRefName: string;
|
||||
headRefName: string;
|
||||
comments: Array<{
|
||||
@@ -45,6 +87,22 @@ interface GhPrViewJson {
|
||||
}>;
|
||||
}
|
||||
|
||||
interface GhPrListJson {
|
||||
number: number;
|
||||
url: string;
|
||||
title: string;
|
||||
state: "OPEN" | "CLOSED" | "MERGED";
|
||||
baseRefName: string;
|
||||
headRefName: string;
|
||||
isCrossRepository?: boolean;
|
||||
mergedAt?: string | null;
|
||||
}
|
||||
|
||||
interface GhPrCheckJson {
|
||||
name: string;
|
||||
state: string;
|
||||
}
|
||||
|
||||
interface GhIssueViewJson {
|
||||
number: number;
|
||||
url: string;
|
||||
@@ -53,6 +111,94 @@ interface GhIssueViewJson {
|
||||
stateReason?: "completed" | "not_planned" | "reopened";
|
||||
}
|
||||
|
||||
function normalizeCheckState(state: string | null | undefined): PrCheckState {
|
||||
switch ((state ?? "").toLowerCase()) {
|
||||
case "success":
|
||||
return "success";
|
||||
case "pending":
|
||||
case "queued":
|
||||
case "in_progress":
|
||||
case "expected":
|
||||
return "pending";
|
||||
case "failure":
|
||||
case "failed":
|
||||
case "error":
|
||||
return "failure";
|
||||
case "cancelled":
|
||||
return "cancelled";
|
||||
case "timed_out":
|
||||
return "timed_out";
|
||||
case "action_required":
|
||||
return "action_required";
|
||||
case "neutral":
|
||||
return "neutral";
|
||||
case "skipped":
|
||||
return "skipped";
|
||||
case "stale":
|
||||
return "stale";
|
||||
case "startup_failure":
|
||||
return "startup_failure";
|
||||
default:
|
||||
return "failure";
|
||||
}
|
||||
}
|
||||
|
||||
function toPrInfo(input: {
|
||||
url: string;
|
||||
number: number;
|
||||
title: string;
|
||||
status: PrInfo["status"];
|
||||
headBranch: string;
|
||||
baseBranch: string;
|
||||
commentCount?: number;
|
||||
lastCommentAt?: string;
|
||||
lastCheckedAt?: string;
|
||||
}): PrInfo {
|
||||
return {
|
||||
url: input.url,
|
||||
number: input.number,
|
||||
status: input.status,
|
||||
title: input.title,
|
||||
headBranch: input.headBranch,
|
||||
baseBranch: input.baseBranch,
|
||||
commentCount: input.commentCount ?? 0,
|
||||
lastCommentAt: input.lastCommentAt,
|
||||
lastCheckedAt: input.lastCheckedAt,
|
||||
};
|
||||
}
|
||||
|
||||
export function isPrMergeReady(input: {
|
||||
status: PrInfo["status"];
|
||||
reviewDecision: ReviewDecision;
|
||||
checks: PrCheckStatus[];
|
||||
}): { ready: boolean; blockingReasons: string[] } {
|
||||
const blockingReasons: string[] = [];
|
||||
|
||||
if (input.status !== "open") {
|
||||
blockingReasons.push(`PR is ${input.status}`);
|
||||
}
|
||||
|
||||
if (input.reviewDecision === "CHANGES_REQUESTED") {
|
||||
blockingReasons.push("changes requested review is active");
|
||||
}
|
||||
|
||||
const blockingChecks = input.checks.filter(
|
||||
(check) => check.required && check.state !== "success",
|
||||
);
|
||||
if (blockingChecks.length > 0) {
|
||||
blockingReasons.push(
|
||||
`required checks not successful: ${blockingChecks
|
||||
.map((check) => `${check.name} (${check.state})`)
|
||||
.join(", ")}`,
|
||||
);
|
||||
}
|
||||
|
||||
return {
|
||||
ready: blockingReasons.length === 0,
|
||||
blockingReasons,
|
||||
};
|
||||
}
|
||||
|
||||
export class GitHubClient {
|
||||
private token: string | undefined;
|
||||
private baseUrl = "https://api.github.com";
|
||||
@@ -65,13 +211,32 @@ export class GitHubClient {
|
||||
this.token = token;
|
||||
}
|
||||
|
||||
private hasGhAuth(): boolean {
|
||||
return isGhAvailable() && isGhAuthenticated();
|
||||
}
|
||||
|
||||
private resolveRepo(owner?: string, repo?: string): { owner: string; repo: string } {
|
||||
if (owner && repo) {
|
||||
return { owner, repo };
|
||||
}
|
||||
|
||||
const currentRepo = getCurrentRepo();
|
||||
if (!currentRepo) {
|
||||
throw new Error(
|
||||
"Could not determine repository. Specify owner/repo in params or run from a git repository with a GitHub remote.",
|
||||
);
|
||||
}
|
||||
|
||||
return currentRepo;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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)
|
||||
if (isGhAvailable() && isGhAuthenticated()) {
|
||||
if (this.hasGhAuth()) {
|
||||
try {
|
||||
return this.createPrWithGh(params);
|
||||
} catch (err) {
|
||||
@@ -92,24 +257,7 @@ export class GitHubClient {
|
||||
|
||||
private createPrWithGh(params: CreatePrParams): PrInfo {
|
||||
const { owner: paramOwner, repo: paramRepo, title, body, head, base } = params;
|
||||
|
||||
// Get owner/repo from params or current repo context
|
||||
let owner = paramOwner;
|
||||
let repo = paramRepo;
|
||||
|
||||
if (!owner || !repo) {
|
||||
const currentRepo = getCurrentRepo();
|
||||
if (!currentRepo) {
|
||||
throw new Error("Could not determine repository. Specify owner/repo in params or run from a git repository with a GitHub remote.");
|
||||
}
|
||||
owner = currentRepo.owner;
|
||||
repo = currentRepo.repo;
|
||||
}
|
||||
|
||||
// Type guard: owner and repo are now guaranteed to be strings
|
||||
if (!owner || !repo) {
|
||||
throw new Error("Could not determine repository.");
|
||||
}
|
||||
const { owner, repo } = this.resolveRepo(paramOwner, paramRepo);
|
||||
|
||||
// Build gh pr create command arguments (as array for safety)
|
||||
const args = [
|
||||
@@ -138,7 +286,7 @@ export class GitHubClient {
|
||||
|
||||
const number = parseInt(match[1], 10);
|
||||
|
||||
return {
|
||||
return toPrInfo({
|
||||
url: prUrl,
|
||||
number,
|
||||
status: "open",
|
||||
@@ -146,29 +294,12 @@ export class GitHubClient {
|
||||
headBranch: head,
|
||||
baseBranch: base || "main",
|
||||
commentCount: 0,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
private async createPrWithApi(params: CreatePrParams): Promise<PrInfo> {
|
||||
const { owner: paramOwner, repo: paramRepo, title, body, head, base = "main" } = params;
|
||||
|
||||
// Get owner/repo from params or current repo context
|
||||
let owner = paramOwner;
|
||||
let repo = paramRepo;
|
||||
|
||||
if (!owner || !repo) {
|
||||
const currentRepo = getCurrentRepo();
|
||||
if (!currentRepo) {
|
||||
throw new Error("Could not determine repository. Specify owner/repo in params or run from a git repository with a GitHub remote.");
|
||||
}
|
||||
owner = currentRepo.owner;
|
||||
repo = currentRepo.repo;
|
||||
}
|
||||
|
||||
// Type guard: owner and repo are now guaranteed to be strings
|
||||
if (!owner || !repo) {
|
||||
throw new Error("Could not determine repository.");
|
||||
}
|
||||
const { owner, repo } = this.resolveRepo(paramOwner, paramRepo);
|
||||
|
||||
const url = `${this.baseUrl}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/pulls`;
|
||||
|
||||
@@ -200,7 +331,7 @@ export class GitHubClient {
|
||||
comments: number;
|
||||
};
|
||||
|
||||
return {
|
||||
return toPrInfo({
|
||||
url: data.html_url,
|
||||
number: data.number,
|
||||
status: this.mapPrState(data.state),
|
||||
@@ -208,14 +339,339 @@ export class GitHubClient {
|
||||
headBranch: data.head.ref,
|
||||
baseBranch: data.base.ref,
|
||||
commentCount: data.comments,
|
||||
});
|
||||
}
|
||||
|
||||
async findPrForBranch(params: FindPrParams): Promise<PrInfo | null> {
|
||||
if (this.hasGhAuth()) {
|
||||
try {
|
||||
return await this.findPrForBranchWithGh(params);
|
||||
} catch (err) {
|
||||
if (this.token) {
|
||||
return this.findPrForBranchWithApi(params);
|
||||
}
|
||||
throw new Error(getGhErrorMessage(err));
|
||||
}
|
||||
}
|
||||
|
||||
if (this.token) {
|
||||
return this.findPrForBranchWithApi(params);
|
||||
}
|
||||
throw new Error("GitHub CLI (gh) is not available or not authenticated, and no GITHUB_TOKEN provided.");
|
||||
}
|
||||
|
||||
private async findPrForBranchWithGh(params: FindPrParams): Promise<PrInfo | null> {
|
||||
const { owner, repo } = this.resolveRepo(params.owner, params.repo);
|
||||
const prs = await runGhJsonAsync<GhPrListJson[]>([
|
||||
"pr", "list",
|
||||
"--repo", `${owner}/${repo}`,
|
||||
"--head", params.head,
|
||||
"--state", params.state ?? "all",
|
||||
"--json", "number,url,title,state,baseRefName,headRefName,mergedAt",
|
||||
]);
|
||||
|
||||
const pr = prs[0];
|
||||
if (!pr) return null;
|
||||
|
||||
return toPrInfo({
|
||||
url: pr.url,
|
||||
number: pr.number,
|
||||
status: pr.mergedAt ? "merged" : this.mapGhPrState(pr.state),
|
||||
title: pr.title,
|
||||
headBranch: pr.headRefName,
|
||||
baseBranch: pr.baseRefName,
|
||||
commentCount: 0,
|
||||
});
|
||||
}
|
||||
|
||||
private async findPrForBranchWithApi(params: FindPrParams): Promise<PrInfo | null> {
|
||||
const { owner, repo } = this.resolveRepo(params.owner, params.repo);
|
||||
const searchParams = new URLSearchParams();
|
||||
searchParams.set("head", `${owner}:${params.head}`);
|
||||
searchParams.set("state", params.state ?? "all");
|
||||
searchParams.set("per_page", "1");
|
||||
|
||||
const response = await fetch(
|
||||
`${this.baseUrl}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/pulls?${searchParams}`,
|
||||
{ headers: this.buildHeaders() },
|
||||
);
|
||||
|
||||
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 pulls = (await response.json()) as Array<{
|
||||
number: number;
|
||||
html_url: string;
|
||||
title: string;
|
||||
state: string;
|
||||
merged_at: string | null;
|
||||
head: { ref: string };
|
||||
base: { ref: string };
|
||||
comments: number;
|
||||
}>;
|
||||
|
||||
const pr = pulls[0];
|
||||
if (!pr) return null;
|
||||
|
||||
return toPrInfo({
|
||||
url: pr.html_url,
|
||||
number: pr.number,
|
||||
status: pr.merged_at ? "merged" : this.mapPrState(pr.state),
|
||||
title: pr.title,
|
||||
headBranch: pr.head.ref,
|
||||
baseBranch: pr.base.ref,
|
||||
commentCount: pr.comments,
|
||||
});
|
||||
}
|
||||
|
||||
async getPrMergeStatus(owner: string | undefined, repo: string | undefined, number: number): Promise<PrMergeStatus> {
|
||||
if (this.hasGhAuth()) {
|
||||
try {
|
||||
return await this.getPrMergeStatusWithGh(owner, repo, number);
|
||||
} catch (err) {
|
||||
if (this.token) {
|
||||
return this.getPrMergeStatusWithApi(owner, repo, number);
|
||||
}
|
||||
throw new Error(getGhErrorMessage(err));
|
||||
}
|
||||
}
|
||||
|
||||
if (this.token) {
|
||||
return this.getPrMergeStatusWithApi(owner, repo, number);
|
||||
}
|
||||
throw new Error("GitHub CLI (gh) is not available or not authenticated, and no GITHUB_TOKEN provided.");
|
||||
}
|
||||
|
||||
private async getPrMergeStatusWithGh(owner: string | undefined, repo: string | undefined, number: number): Promise<PrMergeStatus> {
|
||||
const resolved = this.resolveRepo(owner, repo);
|
||||
const pr = await runGhJsonAsync<GhPrViewJson>([
|
||||
"pr", "view", String(number),
|
||||
"--repo", `${resolved.owner}/${resolved.repo}`,
|
||||
"--json", "number,url,title,state,baseRefName,headRefName,reviewDecision",
|
||||
]);
|
||||
const checks = await runGhJsonAsync<GhPrCheckJson[]>([
|
||||
"pr", "checks", String(number),
|
||||
"--repo", `${resolved.owner}/${resolved.repo}`,
|
||||
"--required",
|
||||
"--json", "name,state",
|
||||
]).catch(() => []);
|
||||
|
||||
const prInfo = toPrInfo({
|
||||
url: pr.url,
|
||||
number: pr.number,
|
||||
status: this.mapGhPrState(pr.state),
|
||||
title: pr.title,
|
||||
headBranch: pr.headRefName,
|
||||
baseBranch: pr.baseRefName,
|
||||
commentCount: 0,
|
||||
});
|
||||
const normalizedChecks = checks.map((check) => ({
|
||||
name: check.name,
|
||||
required: true,
|
||||
state: normalizeCheckState(check.state),
|
||||
} satisfies PrCheckStatus));
|
||||
const readiness = isPrMergeReady({
|
||||
status: prInfo.status,
|
||||
reviewDecision: pr.reviewDecision ?? null,
|
||||
checks: normalizedChecks,
|
||||
});
|
||||
|
||||
return {
|
||||
prInfo,
|
||||
reviewDecision: pr.reviewDecision ?? null,
|
||||
checks: normalizedChecks,
|
||||
mergeReady: readiness.ready,
|
||||
blockingReasons: readiness.blockingReasons,
|
||||
};
|
||||
}
|
||||
|
||||
private async getPrMergeStatusWithApi(owner: string | undefined, repo: string | undefined, number: number): Promise<PrMergeStatus> {
|
||||
const resolved = this.resolveRepo(owner, repo);
|
||||
const response = await fetch(`${this.baseUrl}/graphql`, {
|
||||
method: "POST",
|
||||
headers: this.buildHeaders(),
|
||||
body: JSON.stringify({
|
||||
query: `query PullRequestMergeStatus($owner: String!, $repo: String!, $number: Int!) {
|
||||
repository(owner: $owner, name: $repo) {
|
||||
pullRequest(number: $number) {
|
||||
number
|
||||
url
|
||||
title
|
||||
state
|
||||
reviewDecision
|
||||
baseRefName
|
||||
headRefName
|
||||
comments { totalCount }
|
||||
commits(last: 1) {
|
||||
nodes {
|
||||
commit {
|
||||
statusCheckRollup {
|
||||
contexts(first: 100) {
|
||||
nodes {
|
||||
__typename
|
||||
... on CheckRun {
|
||||
name
|
||||
status
|
||||
conclusion
|
||||
isRequired(pullRequestNumber: $number)
|
||||
}
|
||||
... on StatusContext {
|
||||
context
|
||||
state
|
||||
isRequired(pullRequestNumber: $number)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}`,
|
||||
variables: { owner: resolved.owner, repo: resolved.repo, number },
|
||||
}),
|
||||
});
|
||||
|
||||
const payload = await response.json() as {
|
||||
data?: {
|
||||
repository?: {
|
||||
pullRequest?: {
|
||||
number: number;
|
||||
url: string;
|
||||
title: string;
|
||||
state: "OPEN" | "CLOSED" | "MERGED";
|
||||
reviewDecision: ReviewDecision;
|
||||
baseRefName: string;
|
||||
headRefName: string;
|
||||
comments: { totalCount: number };
|
||||
commits: {
|
||||
nodes: Array<{
|
||||
commit: {
|
||||
statusCheckRollup?: {
|
||||
contexts?: {
|
||||
nodes?: Array<
|
||||
| { __typename: "CheckRun"; name: string; status: string; conclusion: string | null; isRequired?: boolean }
|
||||
| { __typename: "StatusContext"; context: string; state: string; isRequired?: boolean }
|
||||
| null
|
||||
>;
|
||||
};
|
||||
} | null;
|
||||
};
|
||||
}>;
|
||||
};
|
||||
};
|
||||
};
|
||||
};
|
||||
errors?: Array<{ message: string }>;
|
||||
};
|
||||
|
||||
if (!response.ok || payload.errors?.length) {
|
||||
const message = payload.errors?.[0]?.message || response.statusText;
|
||||
throw new Error(`GitHub API error: ${response.status} ${message}`);
|
||||
}
|
||||
|
||||
const pr = payload.data?.repository?.pullRequest;
|
||||
if (!pr) {
|
||||
throw new Error(`PR #${number} not found in ${resolved.owner}/${resolved.repo}`);
|
||||
}
|
||||
|
||||
const nodes = pr.commits.nodes[0]?.commit.statusCheckRollup?.contexts?.nodes ?? [];
|
||||
const checks = nodes.flatMap((node) => {
|
||||
if (!node || !node.isRequired) return [];
|
||||
if (node.__typename === "CheckRun") {
|
||||
return [{
|
||||
name: node.name,
|
||||
required: true,
|
||||
state: normalizeCheckState(node.conclusion ?? node.status),
|
||||
} satisfies PrCheckStatus];
|
||||
}
|
||||
return [{
|
||||
name: node.context,
|
||||
required: true,
|
||||
state: normalizeCheckState(node.state),
|
||||
} satisfies PrCheckStatus];
|
||||
});
|
||||
|
||||
const prInfo = toPrInfo({
|
||||
url: pr.url,
|
||||
number: pr.number,
|
||||
status: this.mapGhPrState(pr.state),
|
||||
title: pr.title,
|
||||
headBranch: pr.headRefName,
|
||||
baseBranch: pr.baseRefName,
|
||||
commentCount: pr.comments.totalCount,
|
||||
});
|
||||
const readiness = isPrMergeReady({
|
||||
status: prInfo.status,
|
||||
reviewDecision: pr.reviewDecision,
|
||||
checks,
|
||||
});
|
||||
|
||||
return {
|
||||
prInfo,
|
||||
reviewDecision: pr.reviewDecision,
|
||||
checks,
|
||||
mergeReady: readiness.ready,
|
||||
blockingReasons: readiness.blockingReasons,
|
||||
};
|
||||
}
|
||||
|
||||
async mergePr(params: MergePrParams): Promise<PrInfo> {
|
||||
if (this.hasGhAuth()) {
|
||||
try {
|
||||
return await this.mergePrWithGh(params);
|
||||
} catch (err) {
|
||||
if (this.token) {
|
||||
return this.mergePrWithApi(params);
|
||||
}
|
||||
throw new Error(getGhErrorMessage(err));
|
||||
}
|
||||
}
|
||||
|
||||
if (this.token) {
|
||||
return this.mergePrWithApi(params);
|
||||
}
|
||||
throw new Error("GitHub CLI (gh) is not available or not authenticated, and no GITHUB_TOKEN provided.");
|
||||
}
|
||||
|
||||
private async mergePrWithGh(params: MergePrParams): Promise<PrInfo> {
|
||||
const resolved = this.resolveRepo(params.owner, params.repo);
|
||||
runGh([
|
||||
"pr", "merge", String(params.number),
|
||||
"--repo", `${resolved.owner}/${resolved.repo}`,
|
||||
`--${params.method ?? "squash"}`,
|
||||
"--delete-branch",
|
||||
]);
|
||||
return this.getPrStatus(resolved.owner, resolved.repo, params.number);
|
||||
}
|
||||
|
||||
private async mergePrWithApi(params: MergePrParams): Promise<PrInfo> {
|
||||
const resolved = this.resolveRepo(params.owner, params.repo);
|
||||
const response = await fetch(
|
||||
`${this.baseUrl}/repos/${encodeURIComponent(resolved.owner)}/${encodeURIComponent(resolved.repo)}/pulls/${params.number}/merge`,
|
||||
{
|
||||
method: "PUT",
|
||||
headers: this.buildHeaders(),
|
||||
body: JSON.stringify({ merge_method: params.method ?? "squash" }),
|
||||
},
|
||||
);
|
||||
|
||||
if (!response.ok) {
|
||||
const error = await response.json().catch(() => ({ message: response.statusText }));
|
||||
throw new Error(`GitHub API error: ${response.status} ${error.message || response.statusText}`);
|
||||
}
|
||||
|
||||
return this.getPrStatus(resolved.owner, resolved.repo, params.number);
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch current PR status using gh CLI if available, otherwise REST API.
|
||||
*/
|
||||
async getPrStatus(owner: string, repo: string, number: number): Promise<PrInfo> {
|
||||
if (isGhAvailable() && isGhAuthenticated()) {
|
||||
if (this.hasGhAuth()) {
|
||||
try {
|
||||
return await this.getPrStatusWithGh(owner, repo, number);
|
||||
} catch (err) {
|
||||
@@ -298,7 +754,7 @@ export class GitHubClient {
|
||||
number: number,
|
||||
since?: string,
|
||||
): Promise<PrComment[]> {
|
||||
if (isGhAvailable() && isGhAuthenticated()) {
|
||||
if (this.hasGhAuth()) {
|
||||
try {
|
||||
return await this.listPrCommentsWithGh(owner, repo, number, since);
|
||||
} catch (err) {
|
||||
@@ -383,7 +839,7 @@ export class GitHubClient {
|
||||
repo: string,
|
||||
number: number,
|
||||
): Promise<Omit<import("@kb/core").IssueInfo, "lastCheckedAt"> | null> {
|
||||
if (isGhAvailable() && isGhAuthenticated()) {
|
||||
if (this.hasGhAuth()) {
|
||||
try {
|
||||
return await this.getIssueStatusWithGh(owner, repo, number);
|
||||
} catch (err) {
|
||||
@@ -525,7 +981,7 @@ export class GitHubClient {
|
||||
html_url: string;
|
||||
labels: Array<{ name: string }>;
|
||||
}>> {
|
||||
if (isGhAvailable() && isGhAuthenticated()) {
|
||||
if (this.hasGhAuth()) {
|
||||
try {
|
||||
return await this.listIssuesWithGh(owner, repo, options);
|
||||
} catch (err) {
|
||||
@@ -652,7 +1108,7 @@ export class GitHubClient {
|
||||
state: "open" | "closed";
|
||||
stateReason?: "completed" | "not_planned" | "reopened";
|
||||
} | null> {
|
||||
if (isGhAvailable() && isGhAuthenticated()) {
|
||||
if (this.hasGhAuth()) {
|
||||
try {
|
||||
return await this.getIssueWithGh(owner, repo, number);
|
||||
} catch (err) {
|
||||
|
||||
@@ -1,2 +1,3 @@
|
||||
export { createServer, type ServerOptions } from "./server.js";
|
||||
export { GitHubClient, isPrMergeReady, type PrMergeStatus, type PrCheckStatus, type ReviewDecision, type MergePrParams, type FindPrParams } from "./github.js";
|
||||
export { rateLimit, RATE_LIMITS, type RateLimitOptions } from "./rate-limit.js";
|
||||
|
||||
@@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import express from "express";
|
||||
import http from "node:http";
|
||||
import { createApiRoutes } from "./routes.js";
|
||||
import { GitHubClient } from "./github.js";
|
||||
import type { TaskStore, TaskAttachment } from "@kb/core";
|
||||
import type { TaskDetail } from "@kb/core";
|
||||
import type { AuthStorageLike, ModelRegistryLike } from "./routes.js";
|
||||
@@ -1409,6 +1410,7 @@ describe("Pause/Unpause endpoints", () => {
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.prInfo).toEqual(mockPrInfo);
|
||||
expect(res.body.stale).toBe(false);
|
||||
expect(res.body.automationStatus).toBeNull();
|
||||
});
|
||||
|
||||
it("returns 404 when task has no PR", async () => {
|
||||
@@ -1460,6 +1462,20 @@ describe("Pause/Unpause endpoints", () => {
|
||||
expect(res.body.stale).toBe(true);
|
||||
});
|
||||
|
||||
it("returns automationStatus so the UI can reflect PR-first waiting states", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...FAKE_TASK_DETAIL,
|
||||
status: "awaiting-pr-checks",
|
||||
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.automationStatus).toBe("awaiting-pr-checks");
|
||||
});
|
||||
|
||||
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({
|
||||
@@ -1504,6 +1520,44 @@ describe("Pause/Unpause endpoints", () => {
|
||||
commentCount: 3,
|
||||
};
|
||||
|
||||
it("returns merge readiness details for PR-first UI refreshes", async () => {
|
||||
const originalRepo = process.env.GITHUB_REPOSITORY;
|
||||
process.env.GITHUB_REPOSITORY = "owner/repo";
|
||||
vi.spyOn(GitHubClient.prototype, "getPrMergeStatus").mockResolvedValue({
|
||||
prInfo: mockPrInfo,
|
||||
mergeReady: false,
|
||||
blockingReasons: ["required checks not successful: ci (pending)"],
|
||||
reviewDecision: "CHANGES_REQUESTED",
|
||||
checks: [{ name: "ci", required: true, state: "pending" }],
|
||||
});
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...FAKE_TASK_DETAIL,
|
||||
status: "awaiting-pr-checks",
|
||||
prInfo: mockPrInfo,
|
||||
});
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/tasks/KB-001/pr/refresh",
|
||||
JSON.stringify({}),
|
||||
{ "Content-Type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.prInfo.number).toBe(42);
|
||||
expect(res.body.mergeReady).toBe(false);
|
||||
expect(res.body.blockingReasons).toEqual(["required checks not successful: ci (pending)"]);
|
||||
expect(res.body.reviewDecision).toBe("CHANGES_REQUESTED");
|
||||
expect(res.body.automationStatus).toBe("awaiting-pr-checks");
|
||||
|
||||
if (originalRepo) {
|
||||
process.env.GITHUB_REPOSITORY = originalRepo;
|
||||
} else {
|
||||
delete process.env.GITHUB_REPOSITORY;
|
||||
}
|
||||
});
|
||||
|
||||
it("returns 404 when task has no PR", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(FAKE_TASK_DETAIL);
|
||||
|
||||
|
||||
@@ -693,6 +693,31 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
}
|
||||
});
|
||||
|
||||
// Create refinement task from a completed or in-review task
|
||||
router.post("/tasks/:id/refine", async (req, res) => {
|
||||
try {
|
||||
const { feedback } = req.body;
|
||||
if (!feedback || typeof feedback !== "string") {
|
||||
res.status(400).json({ error: "feedback is required and must be a string" });
|
||||
return;
|
||||
}
|
||||
if (feedback.length === 0 || feedback.length > 2000) {
|
||||
res.status(400).json({ error: "feedback must be between 1 and 2000 characters" });
|
||||
return;
|
||||
}
|
||||
|
||||
const refinedTask = await store.refineTask(req.params.id, feedback);
|
||||
await store.logEntry(req.params.id, "Refinement requested", feedback);
|
||||
res.status(201).json(refinedTask);
|
||||
} catch (err: any) {
|
||||
const status = err.code === "ENOENT" ? 404
|
||||
: err.message?.includes("must be in 'done' or 'in-review'") ? 400
|
||||
: err.message?.includes("Feedback is required") ? 400
|
||||
: 500;
|
||||
res.status(status).json({ error: err.message });
|
||||
}
|
||||
});
|
||||
|
||||
// Archive task (done → archived)
|
||||
router.post("/tasks/:id/archive", async (req, res) => {
|
||||
try {
|
||||
@@ -1576,6 +1601,7 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
res.json({
|
||||
prInfo: task.prInfo,
|
||||
stale: isStale,
|
||||
automationStatus: task.status ?? null,
|
||||
});
|
||||
|
||||
// Trigger background refresh if stale (don't await, let it run)
|
||||
@@ -1635,18 +1661,26 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
return;
|
||||
}
|
||||
|
||||
// Fetch fresh PR status
|
||||
// Fetch fresh PR status + merge readiness
|
||||
const client = new GitHubClient(githubToken);
|
||||
const mergeStatus = await client.getPrMergeStatus(owner, repo, task.prInfo.number);
|
||||
|
||||
const prInfo = await client.getPrStatus(owner, repo, task.prInfo.number);
|
||||
|
||||
// Add lastCheckedAt timestamp
|
||||
prInfo.lastCheckedAt = new Date().toISOString();
|
||||
const prInfo = {
|
||||
...mergeStatus.prInfo,
|
||||
lastCheckedAt: new Date().toISOString(),
|
||||
};
|
||||
|
||||
// Update stored PR info
|
||||
await store.updatePrInfo(task.id, prInfo);
|
||||
|
||||
res.json(prInfo);
|
||||
res.json({
|
||||
prInfo,
|
||||
mergeReady: mergeStatus.mergeReady,
|
||||
blockingReasons: mergeStatus.blockingReasons,
|
||||
reviewDecision: mergeStatus.reviewDecision,
|
||||
checks: mergeStatus.checks,
|
||||
automationStatus: task.status ?? null,
|
||||
});
|
||||
} catch (err: any) {
|
||||
if (err.code === "ENOENT") {
|
||||
res.status(404).json({ error: `Task ${req.params.id} not found` });
|
||||
|
||||
@@ -10,4 +10,5 @@ export { WorktreePool, scanIdleWorktrees, cleanupOrphanedWorktrees } from "./wor
|
||||
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";
|
||||
export { NtfyNotifier, type NtfyNotifierOptions } from "./notifier.js";
|
||||
|
||||
@@ -4,7 +4,7 @@ import type { TaskStore } from "@kb/core";
|
||||
|
||||
const mockStore = {
|
||||
addSteeringComment: vi.fn(),
|
||||
createTask: vi.fn(),
|
||||
createTask: vi.fn().mockResolvedValue({ id: "KB-123" }),
|
||||
} as unknown as TaskStore;
|
||||
|
||||
describe("PrCommentHandler", () => {
|
||||
@@ -176,6 +176,23 @@ describe("PrCommentHandler", () => {
|
||||
"agent"
|
||||
);
|
||||
});
|
||||
|
||||
it("calls out follow-up context when feedback arrives after PR is merged", async () => {
|
||||
await handler.handleNewComments("KB-001", { ...mockPrInfo, status: "merged" }, [
|
||||
{
|
||||
id: 1,
|
||||
body: "Please add one more regression test",
|
||||
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 text = mockStore.addSteeringComment.mock.calls[0][1] as string;
|
||||
expect(text).toContain("This PR is already merged");
|
||||
expect(text).toContain("follow-up work");
|
||||
});
|
||||
});
|
||||
|
||||
describe("createFollowUpTask", () => {
|
||||
|
||||
@@ -128,6 +128,9 @@ export class PrCommentHandler {
|
||||
|
||||
lines.push(`**PR Review Feedback** from @${comment.user.login}`);
|
||||
lines.push(`**PR:** #${prInfo.number} (${prInfo.status})`);
|
||||
if (prInfo.status !== "open") {
|
||||
lines.push(`**Note:** This PR is already ${prInfo.status}. Treat the feedback as follow-up work.`);
|
||||
}
|
||||
lines.push("");
|
||||
|
||||
// Truncate comment body if too long
|
||||
|
||||
@@ -44,6 +44,19 @@ describe("PrMonitor", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("updatePrInfo", () => {
|
||||
it("updates tracked PR metadata without restarting monitoring", () => {
|
||||
monitor.startMonitoring("KB-001", "owner", "repo", mockPrInfo);
|
||||
const updatedPrInfo = { ...mockPrInfo, status: "merged" as const };
|
||||
|
||||
monitor.updatePrInfo("KB-001", updatedPrInfo);
|
||||
|
||||
const tracked = monitor.getTrackedPrs();
|
||||
expect(tracked.get("KB-001")?.prInfo.status).toBe("merged");
|
||||
expect(tracked.get("KB-001")?.owner).toBe("owner");
|
||||
});
|
||||
});
|
||||
|
||||
describe("stopMonitoring", () => {
|
||||
it("stops monitoring a task", () => {
|
||||
monitor.startMonitoring("KB-001", "owner", "repo", mockPrInfo);
|
||||
|
||||
@@ -154,6 +154,16 @@ export class PrMonitor {
|
||||
prMonitorLog.log(`Started monitoring PR #${prInfo.number} for task ${taskId}`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Update the cached PR metadata for an already tracked task.
|
||||
* Keeps comment polling state intact while refreshing fields like PR status.
|
||||
*/
|
||||
updatePrInfo(taskId: string, prInfo: PrInfo): void {
|
||||
const tracked = this.trackedPrs.get(taskId);
|
||||
if (!tracked) return;
|
||||
tracked.prInfo = prInfo;
|
||||
}
|
||||
|
||||
/**
|
||||
* Stop monitoring a PR.
|
||||
*/
|
||||
|
||||
@@ -150,11 +150,14 @@ export class Scheduler {
|
||||
|
||||
// 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);
|
||||
}
|
||||
if (tracked.has(task.id)) {
|
||||
this.options.prMonitor.updatePrInfo(task.id, task.prInfo);
|
||||
return;
|
||||
}
|
||||
|
||||
const repo = getCurrentGitHubRepo(this.store.getRootDir());
|
||||
if (repo) {
|
||||
this.options.prMonitor.startMonitoring(task.id, repo.owner, repo.repo, task.prInfo);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user