diff --git a/.changeset/patch-preserve-branch-name-recreate-ref.md b/.changeset/patch-preserve-branch-name-recreate-ref.md new file mode 100644 index 00000000000..8ae42550808 --- /dev/null +++ b/.changeset/patch-preserve-branch-name-recreate-ref.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Add `recreate-ref` option to `create-pull-request` safe outputs. When set to `true` together with `preserve-branch-name: true`, the handler force-deletes an existing remote branch ref and recreates it from the agent's local HEAD (force-push semantics), enabling workflows that intentionally maintain long-lived reusable branches across iterations (e.g. autoloop programs whose previous PR was merged but whose branch still exists). When `recreate-ref` is omitted or `false` (the default), an existing remote branch under `preserve-branch-name: true` causes the handler to fall back (e.g. open an issue) rather than overwrite the remote ref. diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index 84cf74d8ebe..286acece4ea 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -427,15 +427,36 @@ function generatePatchPreview(patchContent) { } /** - * Check whether the remote branch already exists and, if so, either fail loudly - * (when preserve-branch-name is enabled) or rename the local branch by appending - * a random hex suffix. + * Check whether the remote branch already exists and, if so, either reuse it + * (when preserve-branch-name and recreate-ref are enabled, by force-deleting + * the remote ref so the subsequent push recreates it from the local HEAD) or rename + * the local branch by appending a random hex suffix. + * + * The "force-delete then recreate" semantic is gated behind `recreate-ref` + * because the existing remote branch may have diverged from the local HEAD + * (e.g. a long-lived branch whose previous PR was merged and is now behind + * the base branch). Deleting the ref first lets `pushSignedCommits` recreate + * the branch at the local commit's parent OID and replay only the local + * commits via the GraphQL `createCommitOnBranch` mutation, which is what + * users intend by enabling `recreate-ref` on a reusable branch. + * + * When `preserve-branch-name: true` but `recreate-ref: false` (default), + * an existing remote branch results in an error so the caller falls back to + * the configured fallback (e.g. opening an issue) rather than silently + * destroying the remote ref. + * * @param {string} branchName - Current local branch name. * @param {boolean} preserveBranchName - Whether preserve-branch-name is enabled. + * @param {object} [options] - Additional options. + * @param {boolean} [options.recreateRef] - Whether recreate-ref is enabled. + * Only meaningful when preserveBranchName is true. + * @param {object} [options.githubClient] - Authenticated Octokit client used to delete the + * existing remote ref when recreate-ref is enabled. + * @param {string} [options.owner] - Repository owner for the deleteRef call. + * @param {string} [options.repo] - Repository name for the deleteRef call. * @returns {Promise} The (possibly renamed) branch name to use going forward. - * @throws {Error} If the remote branch exists and preserve-branch-name is true. */ -async function handleRemoteBranchCollision(branchName, preserveBranchName) { +async function handleRemoteBranchCollision(branchName, preserveBranchName, options = {}) { let remoteBranchExists = false; try { const { stdout } = await exec.getExecOutput(`git ls-remote --heads origin ${branchName}`); @@ -451,11 +472,45 @@ async function handleRemoteBranchCollision(branchName, preserveBranchName) { } 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.` - ); + const { recreateRef, githubClient, owner, repo } = options; + if (!recreateRef) { + // preserve-branch-name asked us to keep the exact branch name, but + // recreate-ref is not enabled, so we cannot silently destroy the + // existing remote ref. Surface an error so the caller falls back to the + // configured fallback (e.g. opening an issue). + throw new Error( + `Remote branch "${branchName}" already exists and preserve-branch-name is enabled. ` + `Set recreate-ref: true to force-delete and recreate the remote ref, or disable ` + `preserve-branch-name to allow renaming the branch.` + ); + } + // Reuse the existing branch by deleting the remote ref so the subsequent + // push recreates it from the local HEAD (force-push semantics). This is the + // intended behavior when recreate-ref is enabled for long-lived + // reusable branches whose previous PR was merged. + if (!githubClient || !owner || !repo) { + throw new Error( + `Remote branch "${branchName}" already exists and recreate-ref is enabled, ` + + `but no GitHub client was provided to delete the existing remote ref. This is an ` + + `internal error: the caller must pass githubClient, owner, and repo to reuse the branch.` + ); + } + core.warning(`Remote branch ${branchName} already exists - reusing it (recreate-ref enabled, force-deleting remote ref)`); + try { + await githubClient.rest.git.deleteRef({ owner, repo, ref: `heads/${branchName}` }); + core.info(`Deleted remote branch ${branchName} to reuse it`); + } catch (deleteError) { + /** @type {any} */ + const err = deleteError; + const status = err && typeof err === "object" ? err.status : undefined; + const message = err && typeof err === "object" ? String(err.message || "") : ""; + // 422 "Reference does not exist" can happen if the branch was deleted concurrently; + // treat that as success and continue. + if (status === 422 && /Reference does not exist/i.test(message)) { + core.info(`Remote branch ${branchName} was already deleted concurrently; continuing`); + } else { + throw new Error(`Failed to delete existing remote branch "${branchName}" for reuse with recreate-ref: ${message || String(err)}`); + } + } + return branchName; } core.warning(`Remote branch ${branchName} already exists - appending random suffix`); @@ -488,6 +543,7 @@ async function main(config = {}) { const allowEmpty = parseBoolTemplatable(config.allow_empty, false); const autoMerge = parseBoolTemplatable(config.auto_merge, false); const preserveBranchName = config.preserve_branch_name === true; + const recreateRef = config.recreate_ref === true; const expiresHours = config.expires ? parseInt(String(config.expires), 10) : 0; const maxCount = config.max || 1; // PRs are typically limited to 1 const maxSizeKb = config.max_patch_size ? parseInt(String(config.max_patch_size), 10) : 1024; @@ -1201,7 +1257,7 @@ async function main(config = {}) { // Push the commits from the bundle to the remote branch try { - branchName = await handleRemoteBranchCollision(branchName, preserveBranchName); + branchName = await handleRemoteBranchCollision(branchName, preserveBranchName, { recreateRef, githubClient, owner: repoParts.owner, repo: repoParts.repo }); await pushSignedCommits({ githubClient, @@ -1410,7 +1466,7 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${branchName} --repo // Push the applied commits to the branch (with fallback to issue creation on failure) try { - branchName = await handleRemoteBranchCollision(branchName, preserveBranchName); + branchName = await handleRemoteBranchCollision(branchName, preserveBranchName, { recreateRef, githubClient, owner: repoParts.owner, repo: repoParts.repo }); await pushSignedCommits({ githubClient, @@ -1554,7 +1610,7 @@ ${patchPreview}`; await exec.exec(`git commit --allow-empty -m "Initialize"`); core.info("Created empty commit"); - branchName = await handleRemoteBranchCollision(branchName, preserveBranchName); + branchName = await handleRemoteBranchCollision(branchName, preserveBranchName, { recreateRef, githubClient, owner: repoParts.owner, repo: repoParts.repo }); await pushSignedCommits({ githubClient, diff --git a/actions/setup/js/create_pull_request.test.cjs b/actions/setup/js/create_pull_request.test.cjs index 5d73d8abe84..423c45953a3 100644 --- a/actions/setup/js/create_pull_request.test.cjs +++ b/actions/setup/js/create_pull_request.test.cjs @@ -1548,6 +1548,9 @@ describe("create_pull_request - patch apply fallback to original base commit", ( issues: { addLabels: vi.fn().mockResolvedValue({}), }, + git: { + deleteRef: vi.fn().mockResolvedValue({}), + }, }, graphql: vi.fn(), }; @@ -1702,8 +1705,48 @@ describe("create_pull_request - patch apply fallback to original base commit", ( 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 () => { + it("should reuse existing remote branch when preserve-branch-name and recreate-ref are true (force-delete then recreate)", async () => { // Simulate the remote branch existing (ls-remote returns content) + 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/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, recreate_ref: true }); + + 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(true); + // Should have called deleteRef to force-delete the existing remote branch + expect(global.github.rest.git.deleteRef).toHaveBeenCalledWith({ + owner: "test-owner", + repo: "test-repo", + ref: "heads/preserve-me", + }); + // Should NOT have renamed the local branch (preserve-branch-name keeps the name) + expect(renameCalled).toBe(false); + // Should NOT have warned about appending random suffix + const warningCalls = global.core.warning.mock.calls.map(call => String(call[0])); + expect(warningCalls.some(msg => msg.includes("appending random suffix"))).toBe(false); + // Should have warned about reusing the branch + expect(warningCalls.some(msg => msg.includes("reusing it") && msg.includes("recreate-ref"))).toBe(true); + }); + + it("should fall back when preserve-branch-name is true but recreate-ref is false and remote branch exists", async () => { global.exec = { exec: vi.fn().mockResolvedValue(0), getExecOutput: vi.fn().mockImplementation((cmd, args) => { @@ -1722,11 +1765,34 @@ describe("create_pull_request - patch apply fallback to original base commit", ( 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); + expect(result.error).toContain("already exists and preserve-branch-name is enabled"); + expect(result.error).toContain("recreate-ref"); + // Should NOT have called deleteRef when recreate-ref is not enabled + expect(global.github.rest.git.deleteRef).not.toHaveBeenCalled(); + }); + + it("should fall back to issue when deleteRef fails for recreate-ref reuse", async () => { + 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: "" }); + }), + }; + // Simulate deleteRef failing with a non-recoverable error + global.github.rest.git.deleteRef = vi.fn().mockRejectedValue(Object.assign(new Error("Forbidden"), { status: 403 })); + + const { main } = require("./create_pull_request.cjs"); + const handler = await main({ preserve_branch_name: true, recreate_ref: 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('Failed to delete existing remote branch "preserve-me"'); }); it("should append random suffix when preserve-branch-name is false and remote branch already exists", async () => { diff --git a/docs/src/content/docs/reference/safe-outputs-pull-requests.md b/docs/src/content/docs/reference/safe-outputs-pull-requests.md index 52ceb65393a..263e6affd8a 100644 --- a/docs/src/content/docs/reference/safe-outputs-pull-requests.md +++ b/docs/src/content/docs/reference/safe-outputs-pull-requests.md @@ -48,6 +48,7 @@ safe-outputs: fallback-as-issue: false # disable issue fallback (default: true) auto-close-issue: false # don't auto-add "Fixes #N" to PR description (default: true) preserve-branch-name: true # omit random salt suffix from branch name (default: false) + recreate-ref: true # force-delete and recreate the remote branch when it already exists (requires preserve-branch-name; default: false) excluded-files: # files to omit from the patch entirely - "**/*.lock" - "dist/**" @@ -84,7 +85,7 @@ 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. +When `preserve-branch-name: true` and the agent-supplied branch name already exists on the remote, the default behavior is to fall back (e.g. open an issue when `fallback-as-issue: true`) rather than rename the branch or overwrite the remote ref. To enable reuse of the existing remote branch, set `recreate-ref: true`: the handler will force-delete the stale remote ref and recreate it from the agent's local HEAD (force-push semantics). This is the intended behavior for long-lived reusable branches whose previous PR was merged. `recreate-ref` requires `preserve-branch-name: true` to take effect; the handler does not silently rename the branch in this case. 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. diff --git a/docs/src/content/docs/reference/safe-outputs-specification.md b/docs/src/content/docs/reference/safe-outputs-specification.md index d5c66d63dae..fda43cd8dcb 100644 --- a/docs/src/content/docs/reference/safe-outputs-specification.md +++ b/docs/src/content/docs/reference/safe-outputs-specification.md @@ -1579,6 +1579,8 @@ create-pull-request: commit-changes: true # Auto-commit workspace changes reviewers: [user1, copilot] # Auto-request reviewers labels: [automated] # Auto-apply labels + preserve-branch-name: false # Keep agent branch name verbatim (no random salt suffix) + recreate-ref: false # When preserve-branch-name and remote branch exists, force-delete and recreate the remote ref ``` **Asset Upload Extensions**: @@ -2179,6 +2181,14 @@ safe-outputs: 3. **Draft Status**: Creates as draft by default for safety. 4. **Auto-Commit**: When `commit-changes: true`, commits workspace changes before PR creation. 5. **Reviewer Assignment**: Auto-requests reviewers if configured. +6. **Branch Name Normalization**: The agent-supplied branch name is sanitized (invalid characters replaced; casing preserved). When `preserve-branch-name: false` (default), a random hex salt suffix is appended to ensure uniqueness across runs. When `preserve-branch-name: true`, the salt suffix is omitted so the branch name appears verbatim (useful for repository naming conventions, e.g. `bugfix/BR-329-red`). +7. **Remote Branch Collision Handling**: When the resolved branch name already exists on the remote, behavior depends on the configuration: + + | `preserve-branch-name` | `recreate-ref` | Behavior on collision | + |---|---|---| + | `false` (default) | n/a | Append random hex suffix to local branch name and continue | + | `true` | `false` (default) | Surface `push_failed`; caller falls back (e.g. opens an issue when `fallback-as-issue: true`) | + | `true` | `true` | Force-delete the existing remote ref via `DELETE /repos/{owner}/{repo}/git/refs/heads/{branch}` and let the subsequent push recreate it from the agent's local HEAD (force-push semantics). Concurrent-deletion 422 responses with "Reference does not exist" are treated as success. | **Configuration Parameters**: @@ -2191,6 +2201,8 @@ safe-outputs: - `labels`: Auto-apply labels - `title-prefix`: Prepend to titles - `footer`: Footer override +- `preserve-branch-name`: When `true`, use the agent-supplied branch name verbatim without appending a random salt suffix (default: `false`) +- `recreate-ref`: When `true` (and `preserve-branch-name: true`), allows the handler to force-delete an existing remote branch ref and recreate it from the agent's local HEAD on collision. When `false` (default), an existing remote branch under `preserve-branch-name: true` causes a fallback rather than overwriting the remote ref. Has no effect when `preserve-branch-name: false`. (default: `false`) **Security Requirements**: diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 793cf730117..5e05504750d 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -5950,6 +5950,11 @@ "description": "When true, the random salt suffix is not appended to the agent-specified branch name. Invalid characters are still replaced for security, and casing is always preserved regardless of this setting. Useful when the target repository enforces branch naming conventions (e.g. Jira keys in uppercase such as 'bugfix/BR-329-red'). Defaults to false.", "default": false }, + "recreate-ref": { + "type": "boolean", + "description": "When true (and preserve-branch-name is true), allows the handler to force-delete an existing remote branch ref and recreate it from the agent's local HEAD. When false (default), if the agent-specified branch already exists on the remote with preserve-branch-name enabled, the handler falls back (e.g. opens an issue) rather than overwriting the remote ref. Useful for long-lived reusable branches whose previous PR was merged.", + "default": false + }, "excluded-files": { "type": "array", "items": { diff --git a/pkg/workflow/compiler_safe_outputs_handlers.go b/pkg/workflow/compiler_safe_outputs_handlers.go index 1041cfc3ba2..65c39172bbd 100644 --- a/pkg/workflow/compiler_safe_outputs_handlers.go +++ b/pkg/workflow/compiler_safe_outputs_handlers.go @@ -416,6 +416,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddStringSlice("allowed_files", c.AllowedFiles). AddStringSlice("excluded_files", c.ExcludedFiles). AddIfTrue("preserve_branch_name", c.PreserveBranchName). + AddIfTrue("recreate_ref", c.RecreateRef). AddIfNotEmpty("patch_format", c.PatchFormat). AddIfTrue("staged", c.Staged) return builder.Build() diff --git a/pkg/workflow/create_pull_request.go b/pkg/workflow/create_pull_request.go index 99ebfae0093..4823fe04cc0 100644 --- a/pkg/workflow/create_pull_request.go +++ b/pkg/workflow/create_pull_request.go @@ -42,6 +42,7 @@ type CreatePullRequestsConfig struct { AllowedFiles []string `yaml:"allowed-files,omitempty"` // Strict allowlist of glob patterns for files eligible for create. Checked independently of protected-files; both checks must pass. ExcludedFiles []string `yaml:"excluded-files,omitempty"` // List of glob patterns for files to exclude from the patch using git :(exclude) pathspecs. Matching files are stripped by git at generation time and will not appear in the commit or be subject to allowed-files or protected-files checks. PreserveBranchName bool `yaml:"preserve-branch-name,omitempty"` // When true, skips the random salt suffix on agent-specified branch names. Invalid characters are still replaced for security; casing is always preserved. Useful when CI enforces branch naming conventions (e.g. Jira keys in uppercase). + RecreateRef bool `yaml:"recreate-ref,omitempty"` // When true (and preserve-branch-name is true), allows the handler to force-delete an existing remote branch ref and recreate it from the agent's local HEAD. When false (default), an existing remote branch causes a fallback to issue (or push_failed). Useful for long-lived reusable branches whose previous PR was merged. PatchFormat string `yaml:"patch-format,omitempty"` // Transport format for packaging changes: "am" (default, uses git format-patch) or "bundle" (uses git bundle, preserves merge topology and per-commit metadata). AllowWorkflows bool `yaml:"allow-workflows,omitempty"` // When true, adds workflows: write to the GitHub App token. Requires safe-outputs.github-app to be configured. }