Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions lib/coding-agent/__tests__/handlePRCreated.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ vi.mock("../prState", () => ({
}));

describe("handlePRCreated", () => {
it("posts a card with PR links and merge button", async () => {
it("posts a card with PR links and individual merge buttons", async () => {
const { handlePRCreated } = await import("../handlePRCreated");

await handlePRCreated("slack:C123:ts", {
Expand All @@ -47,7 +47,10 @@ describe("handlePRCreated", () => {

const { Button } = await import("chat");
expect(Button).toHaveBeenCalledWith(
expect.objectContaining({ id: "merge_all_prs", label: "Merge All PRs" }),
expect.objectContaining({
id: "merge_pr:recoupable/api#42",
label: "Merge recoupable/api#42",
}),
);

expect(mockThread.setState).toHaveBeenCalledWith(
Expand Down
39 changes: 39 additions & 0 deletions lib/coding-agent/__tests__/mergeGithubPR.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { mergeGithubPR } from "../mergeGithubPR";

global.fetch = vi.fn();

beforeEach(() => {
vi.clearAllMocks();
});

describe("mergeGithubPR", () => {
it("squash-merges a PR and returns ok", async () => {
vi.mocked(fetch).mockResolvedValue({ ok: true } as Response);

const result = await mergeGithubPR("recoupable/api", 42, "ghp_test");

expect(result).toEqual({ ok: true });
expect(fetch).toHaveBeenCalledWith(
"https://api.github.com/repos/recoupable/api/pulls/42/merge",
expect.objectContaining({
method: "PUT",
body: JSON.stringify({ merge_method: "squash" }),
}),
);
});

it("returns error message on failure", async () => {
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
vi.mocked(fetch).mockResolvedValue({
ok: false,
status: 405,
text: () => Promise.resolve(JSON.stringify({ message: "Not allowed" })),
} as any);

const result = await mergeGithubPR("recoupable/api", 42, "ghp_test");

expect(result).toEqual({ ok: false, message: "Not allowed" });
consoleSpy.mockRestore();
});
});
155 changes: 122 additions & 33 deletions lib/coding-agent/__tests__/onMergeAction.test.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,32 @@
import { describe, it, expect, vi, beforeEach } from "vitest";

global.fetch = vi.fn();
const mockMergeGithubPR = vi.fn();
vi.mock("../mergeGithubPR", () => ({
mergeGithubPR: (...args: unknown[]) => mockMergeGithubPR(...args),
}));

const mockHandleMergeSuccess = vi.fn();
vi.mock("../handleMergeSuccess", () => ({
handleMergeSuccess: (...args: unknown[]) => mockHandleMergeSuccess(...args),
}));

const mockDeletePRState = vi.fn();
vi.mock("../prState", () => ({
deleteCodingAgentPRState: (...args: unknown[]) => mockDeletePRState(...args),
}));

const mockUpsertAccountSnapshot = vi.fn();
vi.mock("@/lib/supabase/account_snapshots/upsertAccountSnapshot", () => ({
upsertAccountSnapshot: (...args: unknown[]) => mockUpsertAccountSnapshot(...args),
}));

const { registerOnMergeAction } = await import("../handlers/onMergeAction");

beforeEach(() => {
vi.clearAllMocks();
process.env.GITHUB_TOKEN = "ghp_test";
mockHandleMergeSuccess.mockResolvedValue(undefined);
mockUpsertAccountSnapshot.mockResolvedValue({ data: {}, error: null });
});

/**
Expand All @@ -25,18 +39,34 @@ function createMockBot() {
}

describe("registerOnMergeAction", () => {
it("registers merge_all_prs action handler", () => {
it("registers catch-all action handler", () => {
const bot = createMockBot();
registerOnMergeAction(bot);
expect(bot.onAction).toHaveBeenCalledWith("merge_all_prs", expect.any(Function));
expect(bot.onAction).toHaveBeenCalledWith(expect.any(Function));
});

it("squash-merges PRs, calls handleMergeSuccess, and posts results", async () => {
vi.mocked(fetch).mockResolvedValue({ ok: true } as Response);
it("ignores non-merge actions", async () => {
const bot = createMockBot();
registerOnMergeAction(bot);
const handler = bot.onAction.mock.calls[0][0];

const mockThread = {
state: Promise.resolve({ status: "pr_created", prompt: "fix bug", prs: [] }),
post: vi.fn(),
setState: vi.fn(),
};

await handler({ thread: mockThread, actionId: "some_other_action" });

expect(mockThread.post).not.toHaveBeenCalled();
});

it("squash-merges a single PR, calls handleMergeSuccess, and posts result", async () => {
mockMergeGithubPR.mockResolvedValue({ ok: true });

const bot = createMockBot();
registerOnMergeAction(bot);
const handler = bot.onAction.mock.calls[0][1];
const handler = bot.onAction.mock.calls[0][0];

const mockThread = {
state: Promise.resolve({
Expand All @@ -50,66 +80,125 @@ describe("registerOnMergeAction", () => {
setState: vi.fn(),
};

await handler({ thread: mockThread });
await handler({ thread: mockThread, actionId: "merge_pr:recoupable/api#42" });

expect(fetch).toHaveBeenCalledWith(
"https://api.github.com/repos/recoupable/api/pulls/42/merge",
expect.objectContaining({ method: "PUT" }),
);
expect(mockThread.setState).toHaveBeenCalledWith({ status: "merged" });
expect(mockMergeGithubPR).toHaveBeenCalledWith("recoupable/api", 42, "ghp_test");
expect(mockThread.setState).toHaveBeenCalledWith({ status: "merged", prs: [] });
expect(mockHandleMergeSuccess).toHaveBeenCalledWith(
expect.objectContaining({ branch: "agent/fix-bug", snapshotId: "snap_abc123" }),
);
expect(mockThread.post).toHaveBeenCalledWith(expect.stringContaining("merged"));
expect(mockThread.post).toHaveBeenCalledWith("✅ recoupable/api#42 merged.");
});

it("does not call handleMergeSuccess when a merge fails", async () => {
vi.mocked(fetch).mockResolvedValue({
ok: false,
status: 409,
text: () => Promise.resolve(JSON.stringify({ message: "merge conflict" })),
} as unknown as Response);
it("merges one PR and keeps remaining PRs in state without calling handleMergeSuccess", async () => {
mockMergeGithubPR.mockResolvedValue({ ok: true });

const bot = createMockBot();
registerOnMergeAction(bot);
const handler = bot.onAction.mock.calls[0][1];
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
const handler = bot.onAction.mock.calls[0][0];

const mockThread = {
state: Promise.resolve({
status: "pr_created",
prompt: "fix bug",
branch: "agent/fix-bug",
snapshotId: "snap_abc123",
prs: [{ repo: "recoupable/api", number: 42, url: "url", baseBranch: "test" }],
prs: [
{ repo: "recoupable/api", number: 42, url: "url1", baseBranch: "test" },
{ repo: "recoupable/chat", number: 10, url: "url2", baseBranch: "test" },
],
}),
post: vi.fn(),
setState: vi.fn(),
};

await handler({ thread: mockThread });
await handler({ thread: mockThread, actionId: "merge_pr:recoupable/api#42" });

expect(mockThread.setState).toHaveBeenCalledWith({
status: "pr_created",
prs: [{ repo: "recoupable/chat", number: 10, url: "url2", baseBranch: "test" }],
});
expect(mockHandleMergeSuccess).not.toHaveBeenCalled();
expect(mockThread.setState).toHaveBeenCalledWith({ status: "pr_created" });
expect(mockThread.post).toHaveBeenCalledWith(expect.stringContaining("failed"));
consoleSpy.mockRestore();
expect(mockThread.post).toHaveBeenCalledWith("✅ recoupable/api#42 merged.");
});

it("posts no PRs message when state has no PRs", async () => {
it("posts not found message when PR is not in thread state", async () => {
const bot = createMockBot();
registerOnMergeAction(bot);
const handler = bot.onAction.mock.calls[0][1];
const handler = bot.onAction.mock.calls[0][0];

const mockThread = {
state: Promise.resolve({ status: "pr_created", prompt: "fix bug" }),
state: Promise.resolve({ status: "pr_created", prompt: "fix bug", prs: [] }),
post: vi.fn(),
setState: vi.fn(),
};

await handler({ thread: mockThread });
await handler({ thread: mockThread, actionId: "merge_pr:recoupable/api#99" });

expect(mockThread.post).toHaveBeenCalledWith("No PRs to merge.");
expect(fetch).not.toHaveBeenCalled();
expect(mockThread.post).toHaveBeenCalledWith("PR recoupable/api#99 not found in this thread.");
expect(mockMergeGithubPR).not.toHaveBeenCalled();
expect(mockHandleMergeSuccess).not.toHaveBeenCalled();
});

it("posts error message when merge fails", async () => {
mockMergeGithubPR.mockResolvedValue({ ok: false, message: "Not allowed" });

const bot = createMockBot();
registerOnMergeAction(bot);
const handler = bot.onAction.mock.calls[0][0];

const mockThread = {
state: Promise.resolve({
status: "pr_created",
prompt: "fix bug",
branch: "agent/fix-bug",
prs: [{ repo: "recoupable/api", number: 42, url: "url", baseBranch: "test" }],
}),
post: vi.fn(),
setState: vi.fn(),
};

await handler({ thread: mockThread, actionId: "merge_pr:recoupable/api#42" });

expect(mockHandleMergeSuccess).not.toHaveBeenCalled();
expect(mockThread.post).toHaveBeenCalledWith("❌ recoupable/api#42 failed to merge: Not allowed");
});

it("merge button click triggers snapshot upsert in Supabase via handleMergeSuccess", async () => {
// Use the real handleMergeSuccess instead of the mock to verify the full chain
const { handleMergeSuccess: realHandleMergeSuccess } = await vi.importActual<
typeof import("../handleMergeSuccess")
>("../handleMergeSuccess");
mockHandleMergeSuccess.mockImplementation(realHandleMergeSuccess);
mockMergeGithubPR.mockResolvedValue({ ok: true });

const bot = createMockBot();
registerOnMergeAction(bot);
const handler = bot.onAction.mock.calls[0][0];

const state = {
status: "pr_created" as const,
prompt: "fix bug",
branch: "agent/fix-bug",
snapshotId: "snap_abc123",
prs: [{ repo: "recoupable/api", number: 42, url: "url", baseBranch: "test" }],
};

const mockThread = {
state: Promise.resolve(state),
post: vi.fn(),
setState: vi.fn(),
};

await handler({ thread: mockThread, actionId: "merge_pr:recoupable/api#42" });

// Verify the full chain: merge → handleMergeSuccess → upsertAccountSnapshot
expect(mockMergeGithubPR).toHaveBeenCalledWith("recoupable/api", 42, "ghp_test");
expect(mockDeletePRState).toHaveBeenCalledWith("recoupable/api", "agent/fix-bug");
expect(mockUpsertAccountSnapshot).toHaveBeenCalledWith(
expect.objectContaining({
snapshot_id: "snap_abc123",
expires_at: expect.any(String),
}),
);
});
});
25 changes: 25 additions & 0 deletions lib/coding-agent/__tests__/parseMergeActionId.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { describe, it, expect } from "vitest";
import { parseMergeActionId } from "../parseMergeActionId";

describe("parseMergeActionId", () => {
it("parses a valid merge action ID", () => {
expect(parseMergeActionId("merge_pr:recoupable/api#42")).toEqual({
repo: "recoupable/api",
number: 42,
});
});

it("parses action ID with nested org/repo", () => {
expect(parseMergeActionId("merge_pr:org/sub-repo#7")).toEqual({
repo: "org/sub-repo",
number: 7,
});
});

it("returns null for invalid format", () => {
expect(parseMergeActionId("merge_all_prs")).toBeNull();
expect(parseMergeActionId("merge_pr:")).toBeNull();
expect(parseMergeActionId("merge_pr:repo")).toBeNull();
expect(parseMergeActionId("other_action:repo#1")).toBeNull();
});
});
12 changes: 8 additions & 4 deletions lib/coding-agent/buildPRCard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Card, CardText, Actions, Button, LinkButton } from "chat";
import type { CodingAgentPR } from "./types";

/**
* Builds a Card with PR review links and a Merge All PRs button.
* Builds a Card with PR review links and individual Merge buttons per PR.
*
* @param title - Card title (e.g. "PRs Created", "PRs Updated")
* @param prs - Array of PRs to build review links for
Expand All @@ -13,10 +13,14 @@ export function buildPRCard(title: string, prs: CodingAgentPR[]) {
children: [
CardText(`${prs.map(pr => `- ${pr.repo}#${pr.number} → \`${pr.baseBranch}\``).join("\n")}\n\nReply in this thread to give feedback.`),
Actions([
...prs.map(pr =>
...prs.flatMap(pr => [
LinkButton({ url: pr.url, label: `Review ${pr.repo}#${pr.number}` }),
),
Button({ id: "merge_all_prs", label: "Merge All PRs", style: "primary" }),
Button({
id: `merge_pr:${pr.repo}#${pr.number}`,
label: `Merge ${pr.repo}#${pr.number}`,
style: "primary",
}),
]),
]),
],
});
Expand Down
Loading