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
63 changes: 62 additions & 1 deletion packages/pi/extensions/forges.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { existsSync } from "node:fs";
import { fileURLToPath } from "node:url";
import { stripVTControlCharacters } from "node:util";

import type { ExtensionAPI } from "@earendil-works/pi-coding-agent";

Expand Down Expand Up @@ -37,6 +38,49 @@ function loadToolOperations(): Promise<typeof ForgesTools> {
return toolOperationsPromise;
}

const platformLabels: Record<ForgesTools.ForgesPlatform, string> = {
github: "GitHub",
gitlab: "GitLab",
gitea: "Gitea",
};
// oxlint-disable-next-line eslint/no-control-regex -- Removing terminal control bytes is intentional.
const controlCharacter = /[\u0000-\u0009\u000B-\u001F\u007F-\u009F]/g;
const formatOrLineSeparator = /[\p{Cf}\p{Zl}\p{Zp}]/gu;
const loneSurrogate = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(?<![\uD800-\uDBFF])[\uDC00-\uDFFF]/g;

function sanitizeApprovalText(value: string): string {
const separated = value
.replaceAll("\r\n", "\n")
.replaceAll("\r", "\n")
.replaceAll("\u001B", " \u001B")
.replaceAll("\u009B", " \u009B")
.replaceAll("\u009D", " \u009D");
return stripVTControlCharacters(separated)
.replace(controlCharacter, " ")
.replace(formatOrLineSeparator, " ")
.replace(loneSurrogate, " ");
}

function approvalField(value: string): string {
return sanitizeApprovalText(value).replaceAll("\n", " ");
}

function pullRequestApprovalMessage(params: ForgesTools.CreatePullRequestParams): string {
const body = sanitizeApprovalText(params.body);
return [
`Repository ${approvalField(params.owner)}/${approvalField(params.repo)} on ${platformLabels[params.platform]}`,
`Branches ${approvalField(params.sourceBranch)} → ${approvalField(params.targetBranch)}`,
`Status ${params.draft === true ? "Draft" : "Ready for review"}`,
`Assignees ${params.assignees?.map(approvalField).join(", ") || "None"}`,
"",
"Title",
approvalField(params.title),
"",
"Description",
body || "(none)",
].join("\n");
}

export default function forgesExtension(pi: ExtensionAPI): void {
pi.registerTool({
name: "forges_repos_list",
Expand Down Expand Up @@ -298,7 +342,24 @@ export default function forgesExtension(pi: ExtensionAPI): void {
"Use forges_pull_requests_create only when the user explicitly asks to create a pull request.",
],
parameters: createPullRequestParameters,
async execute(_toolCallId, params) {
async execute(_toolCallId, params, signal, _onUpdate, ctx) {
if (!ctx.hasUI) {
throw new Error(
"Pull request creation requires interactive approval in Pi TUI or RPC mode",
);
}

const approved = await ctx.ui.confirm(
"Create pull request?",
pullRequestApprovalMessage(params),
{ signal },
);
if (!approved) {
throw new Error(
"Pull request creation was cancelled by the user. Do not retry unless the user asks again.",
);
}

return (await loadToolOperations()).createPullRequest(params);
},
});
Expand Down
164 changes: 164 additions & 0 deletions test/extensions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,13 @@ function ompAccepts(tool: OmpToolDefinition, value: unknown): boolean {
const unusedPiContext = {} as PiExtensionContext;
const unusedOmpContext = {} as OmpExtensionContext;

function approvalPiContext(
confirm: PiExtensionContext["ui"]["confirm"],
hasUI = true,
): PiExtensionContext {
return { hasUI, ui: { confirm } } as unknown as PiExtensionContext;
}

beforeEach(() => {
vi.clearAllMocks();
resetPinnedProviders();
Expand Down Expand Up @@ -172,6 +179,22 @@ beforeEach(() => {
createdAt: "2026-08-18T00:00:00Z",
updatedAt: "2026-08-18T00:00:00Z",
});
mocks.pullRequests.create.mockResolvedValue({
id: "43",
number: 43,
title: "Add approval",
body: "Require confirmation before creation.",
state: "open",
labels: [],
author: { login: "oritwoen" },
assignees: [{ login: "reviewer" }],
createdAt: "2026-09-01T00:00:00Z",
updatedAt: "2026-09-01T00:00:00Z",
sourceBranch: "feat/pr-approval",
targetBranch: "main",
merged: false,
draft: true,
});
});

afterEach(() => {
Expand Down Expand Up @@ -406,6 +429,147 @@ describe("Forges Pi extension", () => {
expect(result.details.result).toEqual({ items: [], hasNextPage: false });
});

it("fails closed when pull-request creation has no approval UI", async () => {
const confirm = vi.fn();
const tool = requirePiTool(registerPiTools(), "forges_pull_requests_create");

await expect(
tool.execute(
"test",
{
platform: "github",
owner: "agntn",
repo: "forges",
title: "Add approval",
body: "Require confirmation before creation.",
sourceBranch: "feat/pr-approval",
targetBranch: "main",
},
undefined,
undefined,
approvalPiContext(confirm, false),
),
).rejects.toThrow("requires interactive approval");

expect(confirm).not.toHaveBeenCalled();
expect(mocks.pullRequests.create).not.toHaveBeenCalled();
});

it("does not create a pull request when Pi approval is declined", async () => {
const confirm = vi.fn().mockResolvedValue(false);
const tool = requirePiTool(registerPiTools(), "forges_pull_requests_create");

await expect(
tool.execute(
"test",
{
platform: "github",
owner: "agntn",
repo: "forges",
title: "Add approval",
body: "Require confirmation before creation.",
sourceBranch: "feat/pr-approval",
targetBranch: "main",
draft: true,
assignees: ["reviewer"],
},
undefined,
undefined,
approvalPiContext(confirm),
),
).rejects.toThrow("cancelled by the user");

expect(confirm).toHaveBeenCalledOnce();
expect(confirm).toHaveBeenCalledWith(
"Create pull request?",
expect.stringContaining("Branches feat/pr-approval → main"),
{ signal: undefined },
);
expect(mocks.pullRequests.create).not.toHaveBeenCalled();
});

it("creates a pull request after Pi approval", async () => {
const confirm = vi.fn().mockResolvedValue(true);
const tool = requirePiTool(registerPiTools(), "forges_pull_requests_create");
const params = {
platform: "github" as const,
owner: "agntn",
repo: "forges",
title: "Add approval",
body: "Require confirmation before creation.",
sourceBranch: "feat/pr-approval",
targetBranch: "main",
draft: true,
assignees: ["reviewer"],
};

const result = await tool.execute(
"test",
params,
undefined,
undefined,
approvalPiContext(confirm),
);

expect(confirm).toHaveBeenCalledWith(
"Create pull request?",
[
"Repository agntn/forges on GitHub",
"Branches feat/pr-approval → main",
"Status Draft",
"Assignees reviewer",
"",
"Title",
"Add approval",
"",
"Description",
"Require confirmation before creation.",
].join("\n"),
{ signal: undefined },
);
expect(mocks.pullRequests.create).toHaveBeenCalledWith("agntn", "forges", {
title: "Add approval",
body: "Require confirmation before creation.",
sourceBranch: "feat/pr-approval",
targetBranch: "main",
draft: true,
assignees: ["reviewer"],
});
expect(result.details.result).toMatchObject({ number: 43 });
});

it("sanitizes pull-request approval fields before rendering them", async () => {
const confirm = vi.fn().mockResolvedValue(false);
const tool = requirePiTool(registerPiTools(), "forges_pull_requests_create");

await expect(
tool.execute(
"test",
{
platform: "github",
owner: "agntn",
repo: "forges",
title: "Title\u001B]0;owned\u0007",
body: `Line one\u009B31mred\nLine two${String.fromCodePoint(0xd800)}end`,
sourceBranch: "feat/pr-approval",
targetBranch: "main",
},
undefined,
undefined,
approvalPiContext(confirm),
),
).rejects.toThrow("cancelled by the user");

const message = confirm.mock.calls[0]?.[1];
expect(message).toContain("Title\nTitle");
expect(message).toContain("Description\nLine one red\nLine two end");
expect(message).not.toMatch(
// oxlint-disable-next-line eslint/no-control-regex -- The assertion detects unsafe terminal bytes.
/[\u0000-\u0009\u000B-\u001F\u007F-\u009F\uD800-\uDFFF\u2028\u2029]/u,
);
expect(mocks.pullRequests.create).not.toHaveBeenCalled();
});

it("reloads authentication through the shared operation", async () => {
const user = { id: "1", login: "aeitwoen" };
mocks.users.authenticated.mockResolvedValue(user);
Expand Down
Loading