Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
21 changes: 21 additions & 0 deletions actions/setup/js/create_pull_request.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -983,6 +983,13 @@ async function main(config = {}) {
}

if (remoteBranchExists) {
if (preserveBranchName) {
throw new Error(
`Remote branch "${branchName}" already exists and preserve-branch-name is enabled. ` +
`Refusing to silently rename the branch. Either delete the remote branch, choose a different ` +
`branch name, or disable preserve-branch-name to allow a random suffix to be appended.`
);

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actionable collision error message is duplicated verbatim in three separate remote-branch collision blocks (bundle push, patch push, and allow-empty push). To reduce drift risk and make future edits safer, consider extracting this message into a small helper (e.g., makePreserveBranchNameCollisionError(branchName)) or a shared constant.

Copilot uses AI. Check for mistakes.
}
core.warning(`Remote branch ${branchName} already exists - appending random suffix`);
const extraHex = crypto.randomBytes(4).toString("hex");
const oldBranch = branchName;
Expand Down Expand Up @@ -1211,6 +1218,13 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${branchName} --repo
}

if (remoteBranchExists) {
if (preserveBranchName) {
throw new Error(
`Remote branch "${branchName}" already exists and preserve-branch-name is enabled. ` +
`Refusing to silently rename the branch. Either delete the remote branch, choose a different ` +
`branch name, or disable preserve-branch-name to allow a random suffix to be appended.`
);
}
core.warning(`Remote branch ${branchName} already exists - appending random suffix`);
const extraHex = crypto.randomBytes(4).toString("hex");
const oldBranch = branchName;
Expand Down Expand Up @@ -1374,6 +1388,13 @@ ${patchPreview}`;
}

if (remoteBranchExists) {
if (preserveBranchName) {
throw new Error(
`Remote branch "${branchName}" already exists and preserve-branch-name is enabled. ` +
`Refusing to silently rename the branch. Either delete the remote branch, choose a different ` +
`branch name, or disable preserve-branch-name to allow a random suffix to be appended.`

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the allow-empty (empty patch) push path, this new throw ends up in the catch (pushError) that returns { success: false, error } without error_type: "push_failed" and without honoring fallback-as-issue (unlike the bundle and patch paths). If callers rely on error_type/fallback consistency (and per the PR description), consider mapping this failure to the same push_failed structure and/or using the same fallback-as-issue behavior here.

Copilot uses AI. Check for mistakes.
);
}
core.warning(`Remote branch ${branchName} already exists - appending random suffix`);
const extraHex = crypto.randomBytes(4).toString("hex");
const oldBranch = branchName;
Expand Down
57 changes: 57 additions & 0 deletions actions/setup/js/create_pull_request.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1574,6 +1574,63 @@ describe("create_pull_request - patch apply fallback to original base commit", (
expect(result.error).toBe("Failed to apply patch");
expect(global.core.warning).toHaveBeenCalledWith("No base_commit recorded in safe output entry - fallback not possible");
});

it("should fail loudly when preserve-branch-name is true and remote branch already exists", async () => {
// Simulate the remote branch existing (ls-remote returns content)
global.exec = {
exec: vi.fn().mockResolvedValue(0),
getExecOutput: vi.fn().mockImplementation((cmd, args) => {
const cmdStr = typeof cmd === "string" ? cmd : `${cmd} ${(args || []).join(" ")}`;
if (cmdStr.includes("ls-remote --heads origin")) {
return Promise.resolve({ exitCode: 0, stdout: "abc123\trefs/heads/preserve-me\n", stderr: "" });
}
return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" });
}),
};

const { main } = require("./create_pull_request.cjs");
const handler = await main({ preserve_branch_name: true, fallback_as_issue: false });

const result = await handler({ title: "Test PR", body: "Test body", patch_path: patchFilePath, branch: "preserve-me", base_commit: MOCK_BASE_COMMIT_SHA }, {});

expect(result.success).toBe(false);
expect(result.error_type).toBe("push_failed");
expect(result.error).toContain('Remote branch "preserve-me" already exists');
expect(result.error).toContain("preserve-branch-name is enabled");
// Critical: should NOT have warned about appending random suffix (silent bypass)
const warningCalls = global.core.warning.mock.calls.map(call => String(call[0]));
expect(warningCalls.some(msg => msg.includes("appending random suffix"))).toBe(false);
});

it("should append random suffix when preserve-branch-name is false and remote branch already exists", async () => {
let renameCalled = false;
global.exec = {
exec: vi.fn().mockImplementation((cmd, args) => {
const cmdStr = typeof cmd === "string" ? cmd : `${cmd} ${(args || []).join(" ")}`;
if (cmdStr.includes("git branch -m")) {
renameCalled = true;
}
return Promise.resolve(0);
}),
getExecOutput: vi.fn().mockImplementation((cmd, args) => {
const cmdStr = typeof cmd === "string" ? cmd : `${cmd} ${(args || []).join(" ")}`;
if (cmdStr.includes("ls-remote --heads origin")) {
return Promise.resolve({ exitCode: 0, stdout: "abc123\trefs/heads/some-branch\n", stderr: "" });
}
return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" });
}),
};

const { main } = require("./create_pull_request.cjs");
const handler = await main({});

const result = await handler({ title: "Test PR", body: "Test body", patch_path: patchFilePath, branch: "some-branch", base_commit: MOCK_BASE_COMMIT_SHA }, {});

expect(result.success).toBe(true);
expect(renameCalled).toBe(true);
const warningCalls = global.core.warning.mock.calls.map(call => String(call[0]));
expect(warningCalls.some(msg => msg.includes("appending random suffix"))).toBe(true);
});
});

describe("create_pull_request - copilot assignee on fallback issues", () => {
Expand Down
2 changes: 2 additions & 0 deletions docs/src/content/docs/reference/safe-outputs-pull-requests.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ The `excluded-files` field accepts a list of glob patterns. Each matching file i

The `preserve-branch-name` field, when set to `true`, omits the random hex salt suffix that is normally appended to the agent-specified branch name. This is useful when the target repository enforces branch naming conventions such as Jira keys in uppercase (e.g., `bugfix/BR-329-red` instead of `bugfix/br-329-red-cde2a954`). Invalid characters are always replaced for security, and casing is always preserved regardless of this setting. Defaults to `false`.

When `preserve-branch-name: true` and the agent-supplied branch name already exists on the remote, the workflow fails with an explicit error rather than silently appending a random suffix. To resolve, delete the existing remote branch, choose a different branch name, or disable `preserve-branch-name` to allow collision-avoidance via a random suffix.

The `draft` field is a **configuration policy**, not a default. Whatever value is set in the workflow frontmatter is always used — the agent cannot override it at runtime.

By default, when a workflow is triggered from an issue, the `create-pull-request` handler automatically appends `- Fixes #N` to the PR description if no closing keyword is already present. This causes GitHub to auto-close the triggering issue when the PR is merged. Set `auto-close-issue: false` to opt out of this behavior — useful for partial-work PRs, multi-PR workflows, or any case where the PR should reference but not close the issue.
Expand Down
Loading