From 5aff3c450aea0ae15085d0bdb59bb0e11217a524 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:50:43 +0000 Subject: [PATCH 1/7] Initial plan From 3b49216e876c582ec01e460eb7a6fea2f7d0a602 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:10:49 +0000 Subject: [PATCH 2/7] Exclude trusted base merges from unsigned PR bundle validation Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../setup/js/push_to_pull_request_branch.cjs | 199 +++++++++++++----- ...o_pull_request_branch.integration.test.cjs | 121 ++++++++++- .../js/push_to_pull_request_branch.test.cjs | 82 ++++++++ .../md/safe_outputs_push_to_pr_branch.md | 4 +- .../reference/safe-outputs-pull-requests.md | 2 + 5 files changed, 347 insertions(+), 61 deletions(-) diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index d49dcd73e47..472a089293e 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -113,11 +113,47 @@ function parsePositiveInteger(value) { * @param {Record} gitOptions * @param {string} rangeBaseRef * @param {string} bundleRef + * @param {string[]} [conflictedFiles] * @returns {Promise} */ -async function getBundlePreApplyFiles(exec, gitOptions, rangeBaseRef, bundleRef) { +async function getBundlePreApplyFiles(exec, gitOptions, rangeBaseRef, bundleRef, conflictedFiles = []) { const bundleDiffResult = await exec.getExecOutput("git", ["diff", "--name-only", "--no-renames", "-z", `${rangeBaseRef}..${bundleRef}`], gitOptions); - return bundleDiffResult.stdout.split("\0").filter(Boolean); + return [...new Set([...bundleDiffResult.stdout.split("\0").filter(Boolean), ...conflictedFiles])]; +} + +/** + * Build a validation-only tree including trusted base-branch changes. Conflict + * markers remain in the tree. Track conflicted paths too: binary and modify/delete + * conflicts may keep one side unchanged without inserting conflict markers. + * + * @param {{ getExecOutput: Function }} exec + * @param {Record} gitOptions + * @param {string} oldHead + * @param {string} candidate + * @param {string} trustedBase + * @returns {Promise<{ ref: string, conflictedFiles: string[] }>} + */ +async function getBundleValidationBase(exec, gitOptions, oldHead, candidate, trustedBase) { + const fallback = { ref: oldHead, conflictedFiles: [] }; + if (!trustedBase) return fallback; + try { + const opts = { ...gitOptions, ignoreReturnCode: true }; + const oldAncestor = await exec.getExecOutput("git", ["merge-base", "--is-ancestor", oldHead, candidate], opts); + const baseAncestor = await exec.getExecOutput("git", ["merge-base", "--is-ancestor", trustedBase, candidate], opts); + const alreadyMerged = await exec.getExecOutput("git", ["merge-base", "--is-ancestor", trustedBase, oldHead], opts); + if (oldAncestor.exitCode !== 0 || baseAncestor.exitCode !== 0 || alreadyMerged.exitCode !== 1) return fallback; + + const merge = await exec.getExecOutput("git", ["merge-tree", "--write-tree", "--name-only", "-z", oldHead, trustedBase], opts); + const [tree, ...fields] = merge.stdout.split("\0"); + if ((merge.exitCode === 0 || merge.exitCode === 1) && /^[0-9a-f]{40}$/.test(tree)) { + return { ref: tree, conflictedFiles: merge.exitCode === 1 ? fields.slice(0, fields.indexOf("")) : [] }; + } + core.warning("Unable to compute trusted base merge tree; using PR-head validation"); + } catch { + // Missing history or unsupported merge-tree: retain the conservative PR-head baseline. + core.warning("Unable to compute trusted base merge tree; using PR-head validation"); + } + return fallback; } async function fetchBundlePrerequisites(exec, core, gitAuthEnv, baseGitOpts, prerequisiteCommits, logPrefix = "") { @@ -432,6 +468,7 @@ async function main(config = {}) { const hasBundleFile = !!(bundleFilePath && fs.existsSync(bundleFilePath)); const hasPatchFile = !!(patchFilePath && fs.existsSync(patchFilePath)); const applyTransport = hasBundleFile ? "bundle" : "patch"; + const deferBundleValidation = hasBundleFile && !signedCommits && !isStaged; core.info(`Apply transport mode: ${applyTransport} (patch file present: ${hasPatchFile}, bundle file present: ${hasBundleFile})`); if (bundleFilePath && !hasBundleFile) { core.warning(`Bundle file path was provided but file is not present on disk: ${bundleFilePath}; falling back to patch transport`); @@ -474,64 +511,75 @@ async function main(config = {}) { return { success: false, error: msg }; } const isEmpty = !patchContent || !patchContent.trim(); - // Validate patch/bundle size against `max_patch_size`. - // - // Size-check source of truth, in order of preference: - // Use the uncompressed patch representation for both transport modes. - // Bundle size is compressed and can undercount highly compressible changes. - if (!isEmpty) { - const patchSizeBytes = Buffer.byteLength(patchContent, "utf8"); - const patchSizeKb = Math.ceil(patchSizeBytes / 1024); - - let bundleSizeBytes = 0; - if (hasBundleFile) { - try { - bundleSizeBytes = fs.statSync(bundleFilePath).size; - } catch (statErr) { - core.warning(`Failed to stat bundle file for size check: ${getErrorMessage(statErr)}`); + /** @type {{ files: string[] | null }} Protected files found in the patch */ + const protectedFilesForFallback = { files: null }; + /** @param {string} patchContent */ + function validatePatch(patchContent) { + // Validate patch/bundle size against `max_patch_size`. + // + // Size-check source of truth, in order of preference: + // Use the uncompressed patch representation for both transport modes. + // Bundle size is compressed and can undercount highly compressible changes. + if (!isEmpty) { + const patchSizeBytes = Buffer.byteLength(patchContent, "utf8"); + const patchSizeKb = Math.ceil(patchSizeBytes / 1024); + + let bundleSizeBytes = 0; + if (hasBundleFile) { + try { + bundleSizeBytes = fs.statSync(bundleFilePath).size; + } catch (statErr) { + core.warning(`Failed to stat bundle file for size check: ${getErrorMessage(statErr)}`); + } } - } - const bundleSizeKb = Math.ceil(bundleSizeBytes / 1024); + const bundleSizeKb = Math.ceil(bundleSizeBytes / 1024); - const sizeForCheckBytes = patchSizeBytes; - const sizeLabel = "Patch size"; - const sizeForCheckKb = Math.ceil(sizeForCheckBytes / 1024); + const sizeForCheckBytes = patchSizeBytes; + const sizeLabel = "Patch size"; + const sizeForCheckKb = Math.ceil(sizeForCheckBytes / 1024); - if (hasBundleFile) { - core.info(`Bundle file size: ${bundleSizeKb} KB`); - } else { - core.info(`Patch file size: ${patchSizeKb} KB`); - } - core.info(`${sizeLabel}: ${sizeForCheckKb} KB (maximum allowed: ${maxSizeKb} KB)`); + if (hasBundleFile) { + core.info(`Bundle file size: ${bundleSizeKb} KB`); + } else { + core.info(`Patch file size: ${patchSizeKb} KB`); + } + core.info(`${sizeLabel}: ${sizeForCheckKb} KB (maximum allowed: ${maxSizeKb} KB)`); + + if (sizeForCheckKb > maxSizeKb) { + let msg; + msg = `Patch size (${sizeForCheckKb} KB) exceeds maximum allowed size (${maxSizeKb} KB)`; + return { success: false, error: msg }; + } - if (sizeForCheckKb > maxSizeKb) { - let msg; - msg = `Patch size (${sizeForCheckKb} KB) exceeds maximum allowed size (${maxSizeKb} KB)`; - return { success: false, error: msg }; + core.info("Patch size validation passed"); } - core.info("Patch size validation passed"); + // Check file protection: allowlist (strict) or protected-files policy. + // Fallback-to-issue detection is deferred until after PR metadata is resolved below. + if (!isEmpty) { + const protection = checkFileProtection(patchContent, config); + if (protection.action === "deny") { + const filesStr = protection.files.join(", "); + const msg = + protection.source === "allowlist" + ? `Cannot push to pull request branch: patch modifies files outside the allowed-files list (${filesStr}). Add the files to the allowed-files configuration field or remove them from the patch.` + : `Cannot push to pull request branch: patch modifies protected files (${filesStr}). Add them to the allowed-files configuration field or set protected-files: fallback-to-issue to create a review issue instead.`; + core.warning(msg); + return { success: false, skipped: true, reasonCode: POLICY_FILE_PROTECTION_DENIED_REASON_CODE, error: msg }; + } + if (protection.action === "fallback") { + protectedFilesForFallback.files = protection.files; + core.warning(`Protected file protection triggered (fallback-to-issue): ${protection.files.join(", ")}. Will create review issue instead of pushing.`); + } + } + return null; } - // Check file protection: allowlist (strict) or protected-files policy. - // Fallback-to-issue detection is deferred until after PR metadata is resolved below. - /** @type {string[] | null} Protected files found in the patch (manifest basenames + path-prefix matches) */ - let protectedFilesForFallback = null; - if (!isEmpty) { - const protection = checkFileProtection(patchContent, config); - if (protection.action === "deny") { - const filesStr = protection.files.join(", "); - const msg = - protection.source === "allowlist" - ? `Cannot push to pull request branch: patch modifies files outside the allowed-files list (${filesStr}). Add the files to the allowed-files configuration field or remove them from the patch.` - : `Cannot push to pull request branch: patch modifies protected files (${filesStr}). Add them to the allowed-files configuration field or set protected-files: fallback-to-issue to create a review issue instead.`; - core.warning(msg); - return { success: false, skipped: true, reasonCode: POLICY_FILE_PROTECTION_DENIED_REASON_CODE, error: msg }; - } - if (protection.action === "fallback") { - protectedFilesForFallback = protection.files; - core.warning(`Protected file protection triggered (fallback-to-issue): ${protection.files.join(", ")}. Will create review issue instead of pushing.`); - } + // Bundle previews include merged base commits. Defer validation until the + // fetched bundle can be compared with a trusted, merge-aware baseline. + if (!deferBundleValidation) { + const validationError = validatePatch(patchContent); + if (validationError) return validationError; } if (isEmpty) { @@ -879,8 +927,8 @@ async function main(config = {}) { // Deferred protected file protection – fallback-to-issue path. // Create a review issue now that we have repoParts, pullNumber, and prTitle available. - if (protectedFilesForFallback && protectedFilesForFallback.length > 0) { - return await createProtectedFilesFallbackIssue(protectedFilesForFallback); + if (protectedFilesForFallback.files && protectedFilesForFallback.files.length > 0) { + return await createProtectedFilesFallbackIssue(protectedFilesForFallback.files); } const hasChanges = !isEmpty; @@ -979,6 +1027,8 @@ async function main(config = {}) { let remoteHeadBeforePatch = ""; let pushedCommitSha = ""; let rangeBaseRef = branchRemoteRef; + let validationBaseRef = branchRemoteRef; + let validationConflictFiles = []; if (hasChanges) { // Capture HEAD before applying changes to compute new-commit count later try { @@ -1031,6 +1081,18 @@ async function main(config = {}) { core.info(`Applying changes from bundle: ${bundleFilePath}`); const bundleRef = `refs/bundles/push-${branchName.replace(/[^a-zA-Z0-9-]/g, "-")}`; try { + // Resolve from GitHub metadata and fetch from the target checkout's + // origin, never from agent-supplied refs or patch headers. + let trustedBase = ""; + const baseSha = pullRequest.base?.sha; + if (!signedCommits && typeof baseSha === "string" && /^[0-9a-f]{40}$/.test(baseSha)) { + try { + await exec.exec("git", ["fetch", "origin", baseSha], { env: { ...process.env, ...gitAuthEnv }, ...baseGitOpts }); + trustedBase = baseSha; + } catch (baseFetchError) { + core.warning(`Unable to fetch trusted PR base; using PR-head validation: ${getErrorMessage(baseFetchError)}`); + } + } await ensureFullHistoryForBundle( exec, { @@ -1107,6 +1169,20 @@ async function main(config = {}) { } } core.info(`Fetched bundle to ${bundleRef}`); + const validationBase = await getBundleValidationBase(exec, baseGitOpts, rangeBaseRef, bundleRef, trustedBase); + validationBaseRef = validationBase.ref; + validationConflictFiles = validationBase.conflictedFiles; + let validationPatch = patchContent; + if (validationBaseRef !== rangeBaseRef) { + core.info(`Bundle validation baseline includes trusted base ${trustedBase}: ${validationBaseRef}`); + const preview = await exec.getExecOutput("git", ["diff", "--binary", "--no-ext-diff", "--no-textconv", validationBaseRef, bundleRef], baseGitOpts); + validationPatch = preview.stdout; + } + if (deferBundleValidation) { + const validationError = validatePatch(validationPatch); + if (validationError) return validationError; + if (protectedFilesForFallback.files?.length) return await createProtectedFilesFallbackIssue(protectedFilesForFallback.files); + } // SECURITY: Use git's own diff against the fetched bundle ref as the // authoritative pre-apply file set for bundle transport. This keeps @@ -1114,7 +1190,13 @@ async function main(config = {}) { // patch artifact under-detects files (for example, merge-resolution // content preserved only by the bundle transport). { - const bundleFiles = await getBundlePreApplyFiles(exec, baseGitOpts, rangeBaseRef, bundleRef); + let bundleFiles = await getBundlePreApplyFiles(exec, baseGitOpts, validationBaseRef, bundleRef, validationConflictFiles); + if (validationBaseRef !== rangeBaseRef) { + // Preserve protection for agent commits that change then revert a + // file, without attributing trusted base-branch commits to the agent. + const history = await exec.getExecOutput("git", ["log", "--format=", "--name-only", "--no-renames", "-z", bundleRef, "--not", rangeBaseRef, trustedBase], baseGitOpts); + bundleFiles = [...new Set([...bundleFiles, ...history.stdout.split("\0").filter(Boolean)])]; + } if (bundleFiles.length > 0) { core.info(`Pre-apply bundle verification: ${bundleFiles.length} file(s) detected from bundle transport`); const bundleProtection = checkFileProtectionPostApply(bundleFiles, config); @@ -1269,6 +1351,7 @@ async function main(config = {}) { } } } // end else (patch path) + if (!hasBundleFile) validationBaseRef = rangeBaseRef; core.info(`Apply transport completed; signed-push base ref: ${rangeBaseRef}`); // POST-APPLY FILE PROTECTION: Verify actual files written match policy. @@ -1277,8 +1360,8 @@ async function main(config = {}) { // (see github/agentic-workflows#539) let agentChangedFiles = []; { - const diffResult = await exec.getExecOutput("git", ["diff", "--name-only", "--no-renames", "-z", `${rangeBaseRef}..HEAD`], baseGitOpts); - const actualFiles = diffResult.stdout.split("\0").filter(Boolean); + const diffResult = await exec.getExecOutput("git", ["diff", "--name-only", "--no-renames", "-z", `${validationBaseRef}..HEAD`], baseGitOpts); + const actualFiles = [...new Set([...diffResult.stdout.split("\0").filter(Boolean), ...validationConflictFiles])]; agentChangedFiles = actualFiles; if (actualFiles.length > 0) { core.info(`Post-apply verification: ${actualFiles.length} file(s) actually modified`); @@ -1752,4 +1835,4 @@ async function main(config = {}) { }; } -module.exports = { main, HANDLER_TYPE, getBundlePreApplyFiles }; +module.exports = { main, HANDLER_TYPE, getBundlePreApplyFiles, getBundleValidationBase }; diff --git a/actions/setup/js/push_to_pull_request_branch.integration.test.cjs b/actions/setup/js/push_to_pull_request_branch.integration.test.cjs index 49bdddabf3c..cf1149c1f65 100644 --- a/actions/setup/js/push_to_pull_request_branch.integration.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.integration.test.cjs @@ -6,7 +6,7 @@ import path from "path"; import { spawnSync } from "child_process"; const require = createRequire(import.meta.url); -const { getBundlePreApplyFiles } = require("./push_to_pull_request_branch.cjs"); +const { getBundlePreApplyFiles, getBundleValidationBase } = require("./push_to_pull_request_branch.cjs"); const { checkFileProtectionPostApply } = require("./manifest_file_helpers.cjs"); global.core = { @@ -75,6 +75,125 @@ describe("push_to_pull_request_branch bundle integration", () => { vi.clearAllMocks(); }); + it.each([ + [false, false], + [true, false], + [false, true], + ])("excludes trusted base changes while counting agent changes (conflict: %s, reverted change: %s)", async (conflict, revertedChange) => { + const repo = createRepo("push-pr-validation-base-"); + tempDirs.push(repo); + const git = args => execGit(args, { cwd: repo }); + writeRepoFile(repo, "conflict.txt", "original\n"); + git(["add", "."]); + git(["commit", "-m", "common base"]); + git(["branch", "-M", "main"]); + git(["checkout", "-b", "feature"]); + writeRepoFile(repo, "existing-pr.txt", "existing PR work\n"); + if (conflict) writeRepoFile(repo, "conflict.txt", "PR version\n"); + git(["add", "."]); + git(["commit", "-m", "existing PR"]); + const oldHead = git(["rev-parse", "HEAD"]).stdout.trim(); + git(["checkout", "main"]); + writeRepoFile(repo, "upstream.txt", "upstream\n".repeat(10000)); + writeRepoFile(repo, "package.json", "{}\n"); + if (conflict) writeRepoFile(repo, "conflict.txt", "base version\n"); + git(["add", "."]); + git(["commit", "-m", "trusted upstream changes"]); + const trustedBase = git(["rev-parse", "HEAD"]).stdout.trim(); + git(["checkout", "feature"]); + const merge = execGit(["merge", "--no-ff", "main", "-m", "merge base"], { cwd: repo, allowFailure: true }); + expect(merge.status).toBe(conflict ? 1 : 0); + if (conflict) { + writeRepoFile(repo, "conflict.txt", "agent resolution\n"); + git(["add", "."]); + git(["commit", "-m", "resolve conflict"]); + } + if (revertedChange) { + writeRepoFile(repo, "package.json", '{"agent":true}\n'); + git(["commit", "-am", "agent modifies protected file"]); + writeRepoFile(repo, "package.json", "{}\n"); + git(["commit", "-am", "agent reverts protected file"]); + } + writeRepoFile(repo, "agent.txt", "agent change\n"); + git(["add", "."]); + git(["commit", "-m", "agent work"]); + + const exec = createExecApi(repo); + const { ref: baseline, conflictedFiles } = await getBundleValidationBase(exec, {}, oldHead, "HEAD", trustedBase); + expect(baseline).not.toBe(oldHead); + const files = await getBundlePreApplyFiles(exec, {}, baseline, "HEAD", conflictedFiles); + expect(files.sort()).toEqual(conflict ? ["agent.txt", "conflict.txt"] : ["agent.txt"]); + expect(checkFileProtectionPostApply(files, { protected_files: ["package.json"] }).action).toBe("allow"); + const originalPatch = git(["format-patch", "--stdout", `${oldHead}..HEAD`]).stdout; + const agentPatch = git(["diff", "--binary", baseline, "HEAD"]).stdout; + expect(Buffer.byteLength(originalPatch)).toBeGreaterThan(10240); + expect(Buffer.byteLength(agentPatch)).toBeLessThan(1024); + expect(files.reduce((size, file) => size + fs.statSync(path.join(repo, file)).size, 0)).toBeLessThan(1024); + expect(git(["rev-list", "--count", "--merges", `${oldHead}..HEAD`]).stdout.trim()).toBe("1"); + const historyFiles = git(["log", "--format=", "--name-only", "--no-renames", "-z", "HEAD", "--not", oldHead, trustedBase]).stdout.split("\0").filter(Boolean); + expect([...new Set(historyFiles)].sort()).toEqual(revertedChange ? ["agent.txt", "package.json"] : ["agent.txt"]); + expect(checkFileProtectionPostApply([...files, ...historyFiles], { protected_files: ["package.json"] }).action).toBe(revertedChange ? "deny" : "allow"); + }); + + it.each(["text", "binary", "modify/delete"])("keeps protected %s conflict resolutions in the validation file set", async kind => { + const repo = createRepo("push-pr-protected-resolution-"); + tempDirs.push(repo); + const git = args => execGit(args, { cwd: repo }); + writeRepoFile(repo, "package.json", kind === "binary" ? Buffer.from([0, 1]) : '{"version":"1"}\n'); + git(["add", "."]); + git(["commit", "-m", "base"]); + git(["branch", "-M", "main"]); + git(["checkout", "-b", "feature"]); + const prContent = kind === "binary" ? Buffer.from([0, 2]) : '{"version":"2"}\n'; + writeRepoFile(repo, "package.json", prContent); + git(["commit", "-am", "PR version"]); + const oldHead = git(["rev-parse", "HEAD"]).stdout.trim(); + git(["checkout", "main"]); + if (kind === "modify/delete") { + git(["rm", "package.json"]); + } else { + writeRepoFile(repo, "package.json", kind === "binary" ? Buffer.from([0, 3]) : '{"version":"3"}\n'); + } + git(["commit", "-am", "base version"]); + const trustedBase = git(["rev-parse", "HEAD"]).stdout.trim(); + git(["checkout", "feature"]); + expect(execGit(["merge", "main"], { cwd: repo, allowFailure: true }).status).toBe(1); + writeRepoFile(repo, "package.json", kind === "text" ? '{"version":"4"}\n' : prContent); + git(["add", "package.json"]); + git(["commit", "-m", "agent resolution"]); + const exec = createExecApi(repo); + const { ref: baseline, conflictedFiles } = await getBundleValidationBase(exec, {}, oldHead, "HEAD", trustedBase); + const files = await getBundlePreApplyFiles(exec, {}, baseline, "HEAD", conflictedFiles); + expect(conflictedFiles).toEqual(["package.json"]); + if (kind !== "text") expect(git(["diff", "--name-only", baseline, "HEAD"]).stdout).toBe(""); + expect(files).toEqual(["package.json"]); + expect(checkFileProtectionPostApply(files, { protected_files: ["package.json"] }).action).toBe("deny"); + }); + + it("does not adjust the baseline for absent, unmerged, already merged, or unrelated base commits", async () => { + const repo = createRepo("push-pr-baseline-fallback-"); + tempDirs.push(repo); + const git = args => execGit(args, { cwd: repo }); + writeRepoFile(repo, "base.txt", "base\n"); + git(["add", "."]); + git(["commit", "-m", "base"]); + const oldHead = git(["rev-parse", "HEAD"]).stdout.trim(); + git(["checkout", "-b", "other"]); + writeRepoFile(repo, "other.txt", "unmerged\n"); + git(["add", "."]); + git(["commit", "-m", "unmerged"]); + const unmerged = git(["rev-parse", "HEAD"]).stdout.trim(); + git(["checkout", "-b", "candidate", oldHead]); + writeRepoFile(repo, "agent.txt", "agent\n"); + git(["add", "."]); + git(["commit", "-m", "agent"]); + const exec = createExecApi(repo); + for (const trustedBase of ["", "f".repeat(40), unmerged, oldHead]) { + expect(await getBundleValidationBase(exec, {}, oldHead, "HEAD", trustedBase)).toEqual({ ref: oldHead, conflictedFiles: [] }); + } + expect(await getBundleValidationBase(exec, {}, unmerged, "HEAD", oldHead)).toEqual({ ref: unmerged, conflictedFiles: [] }); + }); + it("lists files from a fetched bundle before applying it", async () => { const branchName = "autoloop/simple-bundle"; const sourceRepo = createRepo("push-pr-bundle-source-"); diff --git a/actions/setup/js/push_to_pull_request_branch.test.cjs b/actions/setup/js/push_to_pull_request_branch.test.cjs index 132405a8586..2f144bcae86 100644 --- a/actions/setup/js/push_to_pull_request_branch.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.test.cjs @@ -2225,6 +2225,88 @@ index 0000000..abc1234 // ────────────────────────────────────────────────────── describe("patch size validation", () => { + const protectedPatch = "diff --git a/package.json b/package.json\n--- a/package.json\n+++ b/package.json\n@@ -1 +1 @@\n-old\n+new\n"; + it.each([ + ["small agent change", "small patch\n", 16, "merge-agent.txt", null, false], + ["oversized agent patch", "x".repeat(2048), 16, "merge-agent.txt", "Patch size", false], + ["oversized expanded agent content", "small patch\n", 2048, "merge-agent.txt", "Changed content size", false], + ["agent modification of a protected file", protectedPatch, 16, "package.json", "protected files", false], + ["oversized unchanged-side binary resolution", "", 2048, "", "Changed content size", true], + ["reverted protected-file change", "small patch\n", 16, "merge-agent.txt", "bundle modifies protected files", false, "package.json"], + ])("validates %s independently of merged base changes", async (_name, agentPatch, contentSize, changedFile, expectedError, binaryConflict, historyFile = "") => { + const branch = "merge-size-validation"; + const trustedBase = "b".repeat(40); + const validationTree = "a".repeat(40); + createPatchFile(branch, protectedPatch + "x".repeat(4096)); + fs.writeFileSync(canonicalBundlePath(branch), "bundle content"); + const changedPath = path.join(process.cwd(), "merge-agent.txt"); + fs.writeFileSync(changedPath, Buffer.alloc(contentSize)); + const pr = (await mockGithub.rest.pulls.get()).data; + pr.base.sha = trustedBase; + mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + if (args[0] === "merge-base") { + return { exitCode: args[3] === "remote-head" ? 1 : 0, stdout: "", stderr: "" }; + } + if (args[0] === "merge-tree") return { exitCode: binaryConflict ? 1 : 0, stdout: validationTree + (binaryConflict ? "\0merge-agent.txt\0\0" : "\0"), stderr: "" }; + if (args[0] === "diff" && args[1] === "--binary") return { exitCode: 0, stdout: agentPatch, stderr: "" }; + if (args[0] === "diff" && args[1] === "--name-only") return { exitCode: 0, stdout: changedFile ? changedFile + "\0" : "", stderr: "" }; + if (args[0] === "log" && args[1] === "--format=") return { exitCode: 0, stdout: historyFile ? historyFile + "\0" : "", stderr: "" }; + return { exitCode: 0, stdout: "remote-head\n", stderr: "" }; + }); + const pushModule = require("./push_signed_commits.cjs"); + const pushSpy = vi.spyOn(pushModule, "pushSignedCommits").mockResolvedValue("candidate-head"); + try { + const module = await loadModule(); + const handler = await module.main({ max_patch_size: 1, signed_commits: false, protected_files: ["package.json"] }); + const result = await handler({ branch, diff_size: 0 }, {}); + if (expectedError) { + expect(result.success).toBe(false); + expect(result.error).toContain(expectedError); + expect(pushSpy).not.toHaveBeenCalled(); + } else { + expect(result.success).toBe(true); + expect(pushSpy).toHaveBeenCalledWith(expect.objectContaining({ baseRef: "remote-head", signedCommits: false })); + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["diff", "--name-only", "--no-renames", "-z", `${validationTree}..HEAD`], expect.any(Object)); + } + expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", trustedBase], expect.any(Object)); + if (expectedError === "Changed content size") { + expect(mockExec.exec).toHaveBeenCalledWith("git", ["reset", "--hard", "remote-head"], expect.any(Object)); + } + } finally { + pushSpy.mockRestore(); + fs.rmSync(changedPath, { force: true }); + } + }); + + it.each([128, "throw"])("retains the PR-head baseline when merge-tree fails (%s)", async failure => { + mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + if (args[0] === "merge-base") return { exitCode: args[3] === "old-head" ? 1 : 0, stdout: "", stderr: "" }; + if (failure === "throw") throw new Error("missing history"); + return { exitCode: failure, stdout: "", stderr: "unsupported merge-tree" }; + }); + const module = await loadModule(); + expect(await module.getBundleValidationBase(mockExec, {}, "old-head", "candidate", "trusted-base")).toEqual({ ref: "old-head", conflictedFiles: [] }); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("using PR-head validation")); + }); + + it("retains the preview size limit when the trusted base fetch fails", async () => { + const branch = "merge-base-fetch-fails"; + createPatchFile(branch, "x".repeat(4096)); + fs.writeFileSync(canonicalBundlePath(branch), "bundle content"); + const pr = (await mockGithub.rest.pulls.get()).data; + pr.base.sha = "b".repeat(40); + mockExec.exec.mockImplementation(async (_cmd, args) => { + if (args[0] === "fetch" && args[2] === pr.base.sha) throw new Error("base unavailable"); + return 0; + }); + const module = await loadModule(); + const handler = await module.main({ max_patch_size: 1, signed_commits: false }); + const result = await handler({ branch, diff_size: 0 }, {}); + expect(result.success).toBe(false); + expect(result.error).toContain("Patch size"); + expect(mockExec.exec.mock.calls.some(([, args]) => args[0] === "update-ref" && args[1] === "refs/heads/feature-branch")).toBe(false); + }); + it("should reject patches exceeding max size", async () => { // Create a patch larger than 1KB const largePatch = "x".repeat(2 * 1024 * 1024); // 2MB diff --git a/actions/setup/md/safe_outputs_push_to_pr_branch.md b/actions/setup/md/safe_outputs_push_to_pr_branch.md index ba1d9550c97..c43821b4c3a 100644 --- a/actions/setup/md/safe_outputs_push_to_pr_branch.md +++ b/actions/setup/md/safe_outputs_push_to_pr_branch.md @@ -12,6 +12,6 @@ To push changes to the branch of a pull request: - If needed, check out the PR branch locally from `origin/` first. **Important constraints:** - This tool is **append-only**: it adds new commits on top of the existing PR branch. Force-push is NOT supported. -- Do NOT use `git merge` to bring another branch (e.g., `main`) into the PR branch — merge commits cannot be signed; the action will attempt to squash them into a single linear commit before pushing, but this rewrites history. Use `git rebase` instead (e.g., `git rebase origin/main`) to avoid the rewrite. -- **No git credentials are available**: Git credentials are intentionally removed after checkout for security. Do NOT attempt `git fetch`, `git pull`, or any other network git operation that requires authentication (e.g., private repos) — it will fail. Remote-tracking refs already fetched during checkout (e.g., `origin/main`) are available for local operations like `git rebase origin/main`. If you need a branch or ref that is not already present locally, stop and report this rather than attempting to fetch it. +- Do NOT rebase or otherwise rewrite existing PR commits: the resulting branch cannot be published by this append-only tool. To bring the base branch into the PR branch, merge an already-fetched base ref (e.g., `git merge origin/main`) and commit any conflict resolutions. With `patch-format: bundle` and `signed-commits: false`, merge commits are preserved. When signed commits are required, the action squashes new merge-containing history into a regular commit on top of the existing PR head. +- **No git credentials are available**: Git credentials are intentionally removed after checkout for security. Do NOT attempt `git fetch`, `git pull`, or any other network git operation that requires authentication (e.g., private repos) — it will fail. Remote-tracking refs already fetched during checkout (e.g., `origin/main`) are available for local operations like `git merge origin/main`. If you need a branch or ref that is not already present locally, stop and report this rather than attempting to fetch it. - Contributor fork branches are not writable unless the workflow explicitly configures that exact `head-repo` and matching credentials. If `push_to_pull_request_branch` reports this limitation, do not retry. Use `add_comment` when available to post the proposed code or patch to the pull request; otherwise call `report_incomplete` with the proposed change and the error so the maintainer receives it. 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 c5f3028114c..d8d7096c205 100644 --- a/docs/src/content/docs/reference/safe-outputs-pull-requests.md +++ b/docs/src/content/docs/reference/safe-outputs-pull-requests.md @@ -433,6 +433,8 @@ branch name (see [issue #37835](https://github.com/github/gh-aw/issues/37835)). By default, pushes are replayed through GitHub's signed commit API because `signed-commits: true` means signed commits are required. Set `signed-commits: false` only for repositories that do not require signed commits; this uses direct `git push` and can preserve merge commits that the signed commit API cannot represent. This field is supported by both `create-pull-request` and `push-to-pull-request-branch`. +For unsigned bundle pushes that newly incorporate the PR base commit fetched by the handler, patch-size, changed-content-size, and protected-file checks compare against a validation-only merge of the old PR head and that trusted base. Unmodified base-branch changes are excluded; agent changes and conflict resolutions remain subject to the checks. If the trusted baseline cannot be established, checks use the old PR head. Staged mode retains offline patch-preview checks. The push remains append-only: merge an already-fetched base ref rather than rebasing existing PR commits. + ### Cross-repo usage `push-to-pull-request-branch` supports pushing to pull requests in a different repository via `target-repo` (and optionally `allowed-repos`). For fork-backed pull requests, set `head-repo` to the exact repository that owns the PR head branch; follow-up pushes are permitted only when the PR head repository matches that value exactly. When `target-repo` is set, **the target repository must be checked out into the workflow workspace** using either the `checkout:` frontmatter field with a `path:` specified or a custom `steps:` entry that clones it under `$GITHUB_WORKSPACE`. Use `target-repo: "*"` to let the agent choose the target repository at runtime (the safe_outputs job will check out all `checkout:` repositories into subdirectories automatically). From 252bbd9d403017e7ac5f73fa8b53d9eb8e020b3e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:14:20 +0000 Subject: [PATCH 3/7] Preserve protected-file checks for agent merge history Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../setup/js/push_to_pull_request_branch.cjs | 2 +- ...o_pull_request_branch.integration.test.cjs | 45 ++++++++++++++++++- .../js/push_to_pull_request_branch.test.cjs | 7 +++ 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index 472a089293e..0cedae98aac 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -1194,7 +1194,7 @@ async function main(config = {}) { if (validationBaseRef !== rangeBaseRef) { // Preserve protection for agent commits that change then revert a // file, without attributing trusted base-branch commits to the agent. - const history = await exec.getExecOutput("git", ["log", "--format=", "--name-only", "--no-renames", "-z", bundleRef, "--not", rangeBaseRef, trustedBase], baseGitOpts); + const history = await exec.getExecOutput("git", ["log", "--format=", "--name-only", "--no-renames", "--diff-merges=remerge", "-z", bundleRef, "--not", rangeBaseRef, trustedBase], baseGitOpts); bundleFiles = [...new Set([...bundleFiles, ...history.stdout.split("\0").filter(Boolean)])]; } if (bundleFiles.length > 0) { diff --git a/actions/setup/js/push_to_pull_request_branch.integration.test.cjs b/actions/setup/js/push_to_pull_request_branch.integration.test.cjs index cf1149c1f65..a267119ed25 100644 --- a/actions/setup/js/push_to_pull_request_branch.integration.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.integration.test.cjs @@ -130,11 +130,52 @@ describe("push_to_pull_request_branch bundle integration", () => { expect(Buffer.byteLength(agentPatch)).toBeLessThan(1024); expect(files.reduce((size, file) => size + fs.statSync(path.join(repo, file)).size, 0)).toBeLessThan(1024); expect(git(["rev-list", "--count", "--merges", `${oldHead}..HEAD`]).stdout.trim()).toBe("1"); - const historyFiles = git(["log", "--format=", "--name-only", "--no-renames", "-z", "HEAD", "--not", oldHead, trustedBase]).stdout.split("\0").filter(Boolean); - expect([...new Set(historyFiles)].sort()).toEqual(revertedChange ? ["agent.txt", "package.json"] : ["agent.txt"]); + const historyFiles = git(["log", "--format=", "--name-only", "--no-renames", "--diff-merges=remerge", "-z", "HEAD", "--not", oldHead, trustedBase]).stdout.split("\0").filter(Boolean); + expect([...new Set(historyFiles)].sort()).toEqual(revertedChange ? ["agent.txt", "package.json"] : conflict ? ["agent.txt", "conflict.txt"] : ["agent.txt"]); expect(checkFileProtectionPostApply([...files, ...historyFiles], { protected_files: ["package.json"] }).action).toBe(revertedChange ? "deny" : "allow"); }); + it("detects protected changes made and restored only in agent merge commits", async () => { + const repo = createRepo("push-pr-merge-history-protection-"); + tempDirs.push(repo); + const git = args => execGit(args, { cwd: repo }); + writeRepoFile(repo, "package.json", "{}\n"); + git(["add", "."]); + git(["commit", "-m", "base"]); + git(["branch", "-M", "main"]); + git(["checkout", "-b", "feature"]); + writeRepoFile(repo, "existing.txt", "PR work\n"); + git(["add", "."]); + git(["commit", "-m", "existing PR"]); + const oldHead = git(["rev-parse", "HEAD"]).stdout.trim(); + git(["checkout", "main"]); + writeRepoFile(repo, "upstream.txt", "base work\n"); + git(["add", "."]); + git(["commit", "-m", "trusted base work"]); + const trustedBase = git(["rev-parse", "HEAD"]).stdout.trim(); + git(["checkout", "feature"]); + git(["merge", "--no-ff", "--no-commit", "main"]); + writeRepoFile(repo, "package.json", '{"agent":true}\n'); + git(["commit", "-am", "merge-only protected change"]); + git(["checkout", "-b", "side"]); + writeRepoFile(repo, "side.txt", "side work\n"); + git(["add", "."]); + git(["commit", "-m", "side work"]); + git(["checkout", "feature"]); + git(["merge", "--no-ff", "--no-commit", "side"]); + writeRepoFile(repo, "package.json", "{}\n"); + git(["commit", "-am", "merge-only protected restoration"]); + + const exec = createExecApi(repo); + const { ref, conflictedFiles } = await getBundleValidationBase(exec, {}, oldHead, "HEAD", trustedBase); + const files = await getBundlePreApplyFiles(exec, {}, ref, "HEAD", conflictedFiles); + expect(files).toEqual(["side.txt"]); + const historyFiles = git(["log", "--format=", "--name-only", "--no-renames", "--diff-merges=remerge", "-z", "HEAD", "--not", oldHead, trustedBase]).stdout.split("\0").filter(Boolean); + expect(historyFiles).toContain("package.json"); + expect(historyFiles).not.toContain("upstream.txt"); + expect(checkFileProtectionPostApply([...files, ...historyFiles], { protected_files: ["package.json"] }).action).toBe("deny"); + }); + it.each(["text", "binary", "modify/delete"])("keeps protected %s conflict resolutions in the validation file set", async kind => { const repo = createRepo("push-pr-protected-resolution-"); tempDirs.push(repo); diff --git a/actions/setup/js/push_to_pull_request_branch.test.cjs b/actions/setup/js/push_to_pull_request_branch.test.cjs index 2f144bcae86..29e104963eb 100644 --- a/actions/setup/js/push_to_pull_request_branch.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.test.cjs @@ -2269,6 +2269,13 @@ index 0000000..abc1234 expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["diff", "--name-only", "--no-renames", "-z", `${validationTree}..HEAD`], expect.any(Object)); } expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", trustedBase], expect.any(Object)); + if (expectedError === "bundle modifies protected files") { + expect(mockExec.getExecOutput).toHaveBeenCalledWith( + "git", + ["log", "--format=", "--name-only", "--no-renames", "--diff-merges=remerge", "-z", "refs/bundles/push-feature-branch", "--not", "remote-head", trustedBase], + expect.any(Object) + ); + } if (expectedError === "Changed content size") { expect(mockExec.exec).toHaveBeenCalledWith("git", ["reset", "--hard", "remote-head"], expect.any(Object)); } From 221750d5d569d16c66ab106513a9bc7e52854359 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:17:06 +0000 Subject: [PATCH 4/7] Reject unvalidated agent octopus merges conservatively Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../setup/js/push_to_pull_request_branch.cjs | 4 +++ ...o_pull_request_branch.integration.test.cjs | 26 ++++++++++++++++--- .../js/push_to_pull_request_branch.test.cjs | 4 ++- .../reference/safe-outputs-pull-requests.md | 2 +- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index 0cedae98aac..d076eca4a42 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -1192,6 +1192,10 @@ async function main(config = {}) { { let bundleFiles = await getBundlePreApplyFiles(exec, baseGitOpts, validationBaseRef, bundleRef, validationConflictFiles); if (validationBaseRef !== rangeBaseRef) { + const octopus = await exec.getExecOutput("git", ["rev-list", "--min-parents=3", bundleRef, "--not", rangeBaseRef, trustedBase], baseGitOpts); + if (octopus.stdout.trim()) { + return { success: false, error: "Cannot validate agent-created octopus merge commits in the bundle. Use two-parent merges instead." }; + } // Preserve protection for agent commits that change then revert a // file, without attributing trusted base-branch commits to the agent. const history = await exec.getExecOutput("git", ["log", "--format=", "--name-only", "--no-renames", "--diff-merges=remerge", "-z", bundleRef, "--not", rangeBaseRef, trustedBase], baseGitOpts); diff --git a/actions/setup/js/push_to_pull_request_branch.integration.test.cjs b/actions/setup/js/push_to_pull_request_branch.integration.test.cjs index a267119ed25..8529dbe467f 100644 --- a/actions/setup/js/push_to_pull_request_branch.integration.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.integration.test.cjs @@ -135,7 +135,7 @@ describe("push_to_pull_request_branch bundle integration", () => { expect(checkFileProtectionPostApply([...files, ...historyFiles], { protected_files: ["package.json"] }).action).toBe(revertedChange ? "deny" : "allow"); }); - it("detects protected changes made and restored only in agent merge commits", async () => { + it.each([false, true])("detects protected changes made and restored only in agent merge commits (octopus: %s)", async octopus => { const repo = createRepo("push-pr-merge-history-protection-"); tempDirs.push(repo); const git = args => execGit(args, { cwd: repo }); @@ -153,23 +153,41 @@ describe("push_to_pull_request_branch bundle integration", () => { git(["add", "."]); git(["commit", "-m", "trusted base work"]); const trustedBase = git(["rev-parse", "HEAD"]).stdout.trim(); + if (octopus) { + git(["checkout", "-b", "first-side", oldHead]); + writeRepoFile(repo, "first-side.txt", "first side\n"); + git(["add", "."]); + git(["commit", "-m", "first side"]); + } git(["checkout", "feature"]); - git(["merge", "--no-ff", "--no-commit", "main"]); + git(["merge", "--no-ff", "--no-commit", "main", ...(octopus ? ["first-side"] : [])]); writeRepoFile(repo, "package.json", '{"agent":true}\n'); git(["commit", "-am", "merge-only protected change"]); git(["checkout", "-b", "side"]); writeRepoFile(repo, "side.txt", "side work\n"); git(["add", "."]); git(["commit", "-m", "side work"]); + if (octopus) { + git(["checkout", "-b", "second-side", "feature"]); + writeRepoFile(repo, "second-side.txt", "second side\n"); + git(["add", "."]); + git(["commit", "-m", "second side"]); + } git(["checkout", "feature"]); - git(["merge", "--no-ff", "--no-commit", "side"]); + git(["merge", "--no-ff", "--no-commit", "side", ...(octopus ? ["second-side"] : [])]); writeRepoFile(repo, "package.json", "{}\n"); git(["commit", "-am", "merge-only protected restoration"]); const exec = createExecApi(repo); const { ref, conflictedFiles } = await getBundleValidationBase(exec, {}, oldHead, "HEAD", trustedBase); const files = await getBundlePreApplyFiles(exec, {}, ref, "HEAD", conflictedFiles); - expect(files).toEqual(["side.txt"]); + expect(files.sort()).toEqual(octopus ? ["first-side.txt", "second-side.txt", "side.txt"] : ["side.txt"]); + const octopusCommits = git(["rev-list", "--min-parents=3", "HEAD", "--not", oldHead, trustedBase]).stdout.trim(); + if (octopus) { + expect(octopusCommits.split("\n")).toHaveLength(2); + return; + } + expect(octopusCommits).toBe(""); const historyFiles = git(["log", "--format=", "--name-only", "--no-renames", "--diff-merges=remerge", "-z", "HEAD", "--not", oldHead, trustedBase]).stdout.split("\0").filter(Boolean); expect(historyFiles).toContain("package.json"); expect(historyFiles).not.toContain("upstream.txt"); diff --git a/actions/setup/js/push_to_pull_request_branch.test.cjs b/actions/setup/js/push_to_pull_request_branch.test.cjs index 29e104963eb..3dd69a44bc5 100644 --- a/actions/setup/js/push_to_pull_request_branch.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.test.cjs @@ -2233,7 +2233,8 @@ index 0000000..abc1234 ["agent modification of a protected file", protectedPatch, 16, "package.json", "protected files", false], ["oversized unchanged-side binary resolution", "", 2048, "", "Changed content size", true], ["reverted protected-file change", "small patch\n", 16, "merge-agent.txt", "bundle modifies protected files", false, "package.json"], - ])("validates %s independently of merged base changes", async (_name, agentPatch, contentSize, changedFile, expectedError, binaryConflict, historyFile = "") => { + ["agent-created octopus merge", "small patch\n", 16, "merge-agent.txt", "octopus merge", false, "", true], + ])("validates %s independently of merged base changes", async (_name, agentPatch, contentSize, changedFile, expectedError, binaryConflict, historyFile = "", octopusMerge = false) => { const branch = "merge-size-validation"; const trustedBase = "b".repeat(40); const validationTree = "a".repeat(40); @@ -2251,6 +2252,7 @@ index 0000000..abc1234 if (args[0] === "diff" && args[1] === "--binary") return { exitCode: 0, stdout: agentPatch, stderr: "" }; if (args[0] === "diff" && args[1] === "--name-only") return { exitCode: 0, stdout: changedFile ? changedFile + "\0" : "", stderr: "" }; if (args[0] === "log" && args[1] === "--format=") return { exitCode: 0, stdout: historyFile ? historyFile + "\0" : "", stderr: "" }; + if (args[0] === "rev-list" && args[1] === "--min-parents=3") return { exitCode: 0, stdout: octopusMerge ? "c".repeat(40) + "\n" : "", stderr: "" }; return { exitCode: 0, stdout: "remote-head\n", stderr: "" }; }); const pushModule = require("./push_signed_commits.cjs"); 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 d8d7096c205..73e837f93d0 100644 --- a/docs/src/content/docs/reference/safe-outputs-pull-requests.md +++ b/docs/src/content/docs/reference/safe-outputs-pull-requests.md @@ -433,7 +433,7 @@ branch name (see [issue #37835](https://github.com/github/gh-aw/issues/37835)). By default, pushes are replayed through GitHub's signed commit API because `signed-commits: true` means signed commits are required. Set `signed-commits: false` only for repositories that do not require signed commits; this uses direct `git push` and can preserve merge commits that the signed commit API cannot represent. This field is supported by both `create-pull-request` and `push-to-pull-request-branch`. -For unsigned bundle pushes that newly incorporate the PR base commit fetched by the handler, patch-size, changed-content-size, and protected-file checks compare against a validation-only merge of the old PR head and that trusted base. Unmodified base-branch changes are excluded; agent changes and conflict resolutions remain subject to the checks. If the trusted baseline cannot be established, checks use the old PR head. Staged mode retains offline patch-preview checks. The push remains append-only: merge an already-fetched base ref rather than rebasing existing PR commits. +For unsigned bundle pushes that newly incorporate the PR base commit fetched by the handler, patch-size, changed-content-size, and protected-file checks compare against a validation-only merge of the old PR head and that trusted base. Unmodified base-branch changes are excluded; agent changes and conflict resolutions remain subject to the checks. Agent-created octopus merges (three or more parents) are rejected in this mode because their resolution changes cannot be validated with Git's remerge diffs. If the trusted baseline cannot be established, checks use the old PR head. Staged mode retains offline patch-preview checks. The push remains append-only: merge an already-fetched base ref rather than rebasing existing PR commits. ### Cross-repo usage From 21443a304e457ecdbeaeb1d0be17a3d4c6bd88c1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:18:47 +0000 Subject: [PATCH 5/7] Apply remaining changes Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- go.sum | 2 -- 1 file changed, 2 deletions(-) diff --git a/go.sum b/go.sum index 1ebe678cf6c..846be0b42e4 100644 --- a/go.sum +++ b/go.sum @@ -44,8 +44,6 @@ github.com/charmbracelet/harmonica v0.2.0 h1:8NxJWRWg/bzKqqEaaeFNipOu77YR5t8aSwG github.com/charmbracelet/harmonica v0.2.0/go.mod h1:KSri/1RMQOZLbw7AHqgcBycp8pgJnQMYYT8QZRqZ1Ao= github.com/charmbracelet/ultraviolet v0.0.0-20260811164956-006e29f97886 h1:rdnVWKgJpTVXKuKuJyxDJ+NFJdUaUqGvyGy61OcvlbA= github.com/charmbracelet/ultraviolet v0.0.0-20260811164956-006e29f97886/go.mod h1:nAw0d9PhFp1qdzi2xhQU5YOu5sVpDIHWlaW2Uz/bCro= -github.com/charmbracelet/x/ansi v0.11.8 h1:JMFwp0CgDC2+jcOB162HH5k7I3FVbgFSMMYg7dSPBQQ= -github.com/charmbracelet/x/ansi v0.11.8/go.mod h1:ZNN+3mXny/516oTQPLMPIBeSINvNJJQ8uQXDgbeJxY0= github.com/charmbracelet/x/ansi v0.11.9 h1:kIrqLBxtTVbLyyHO2Evoi93EjQo1mylWmpNEsfPxXAM= github.com/charmbracelet/x/ansi v0.11.9/go.mod h1:DG/hpOnaI2QJ/KMMqLL2h0KVhluuJLazf8973P+Vy5w= github.com/charmbracelet/x/conpty v0.1.1 h1:s1bUxjoi7EpqiXysVtC+a8RrvPPNcNvAjfi4jxsAuEs= From 0f62c2a8ec7b398fcd2837012a5cace6a2f90ca0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:25:20 +0000 Subject: [PATCH 6/7] Validate agent-only unsigned bundle history Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- .github/aw/create-agentic-workflow.md | 1 + .../setup/js/push_to_pull_request_branch.cjs | 68 ++++++++++--------- .../js/push_to_pull_request_branch.test.cjs | 40 +++++++++-- actions/setup/js/safe_outputs_tools.json | 2 +- pkg/cli/sessions_download_test.go | 7 +- pkg/cli/work_command_test.go | 10 ++- pkg/workflow/js/safe_outputs_tools.json | 2 +- 7 files changed, 87 insertions(+), 43 deletions(-) diff --git a/.github/aw/create-agentic-workflow.md b/.github/aw/create-agentic-workflow.md index 8747e863a9d..18487b8b5d3 100644 --- a/.github/aw/create-agentic-workflow.md +++ b/.github/aw/create-agentic-workflow.md @@ -65,6 +65,7 @@ When triggered from a workflow-creation issue form, read the form fields and gen ## Conversation Rules - Keep the conversation short and iterative. +- When the request already states a goal, proceed with the relevant mode and ask only about missing details that would change the workflow design. - Translate user intent into workflow structure. - When the user asks for exploration, evaluation, or scenario design rather than file creation, stay in ad hoc evaluation mode. - In ad hoc evaluation mode, do not create `.github/workflows/*.md`. diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index f3155bfda99..b1df498000a 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -150,7 +150,11 @@ async function getBundleValidationBase(exec, gitOptions, oldHead, candidate, tru const merge = await exec.getExecOutput("git", ["merge-tree", "--write-tree", "--name-only", "-z", oldHead, trustedBase], opts); const [tree, ...fields] = merge.stdout.split("\0"); if ((merge.exitCode === 0 || merge.exitCode === 1) && /^[0-9a-f]{40}$/.test(tree)) { - return { ref: tree, conflictedFiles: merge.exitCode === 1 ? fields.slice(0, fields.indexOf("")) : [] }; + const separatorIndex = fields.indexOf(""); + return { + ref: tree, + conflictedFiles: merge.exitCode === 1 ? fields.slice(0, separatorIndex === -1 ? fields.length : separatorIndex) : [], + }; } core.warning("Unable to compute trusted base merge tree; using PR-head validation"); } catch { @@ -516,16 +520,20 @@ async function main(config = {}) { return { success: false, error: msg }; } const isEmpty = !patchContent || !patchContent.trim(); - /** @type {{ files: string[] | null }} Protected files found in the patch */ - const protectedFilesForFallback = { files: null }; - /** @param {string} patchContent */ + let protectedFilesForFallback = []; + /** + * @param {string} patchContent + * @returns {{ error: { success: boolean, error: string, skipped?: boolean, reasonCode?: string } | null, fallbackFiles: string[] }} + */ function validatePatch(patchContent) { + const fallbackFiles = []; + const isEmptyPatch = !patchContent || !patchContent.trim(); // Validate patch/bundle size against `max_patch_size`. // // Size-check source of truth, in order of preference: // Use the uncompressed patch representation for both transport modes. // Bundle size is compressed and can undercount highly compressible changes. - if (!isEmpty) { + if (!isEmptyPatch) { const patchSizeBytes = Buffer.byteLength(patchContent, "utf8"); const patchSizeKb = Math.ceil(patchSizeBytes / 1024); @@ -553,7 +561,7 @@ async function main(config = {}) { if (sizeForCheckKb > maxSizeKb) { let msg; msg = `Patch size (${sizeForCheckKb} KB) exceeds maximum allowed size (${maxSizeKb} KB)`; - return { success: false, error: msg }; + return { error: { success: false, error: msg }, fallbackFiles }; } core.info("Patch size validation passed"); @@ -561,7 +569,7 @@ async function main(config = {}) { // Check file protection: allowlist (strict) or protected-files policy. // Fallback-to-issue detection is deferred until after PR metadata is resolved below. - if (!isEmpty) { + if (!isEmptyPatch) { const protection = checkFileProtection(patchContent, config); if (protection.action === "deny") { const filesStr = protection.files.join(", "); @@ -570,21 +578,22 @@ async function main(config = {}) { ? `Cannot push to pull request branch: patch modifies files outside the allowed-files list (${filesStr}). Add the files to the allowed-files configuration field or remove them from the patch.` : `Cannot push to pull request branch: patch modifies protected files (${filesStr}). Add them to the allowed-files configuration field or set protected-files: fallback-to-issue to create a review issue instead.`; core.warning(msg); - return { success: false, skipped: true, reasonCode: POLICY_FILE_PROTECTION_DENIED_REASON_CODE, error: msg }; + return { error: { success: false, skipped: true, reasonCode: POLICY_FILE_PROTECTION_DENIED_REASON_CODE, error: msg }, fallbackFiles }; } if (protection.action === "fallback") { - protectedFilesForFallback.files = protection.files; + fallbackFiles.push(...protection.files); core.warning(`Protected file protection triggered (fallback-to-issue): ${protection.files.join(", ")}. Will create review issue instead of pushing.`); } } - return null; + return { error: null, fallbackFiles }; } // Bundle previews include merged base commits. Defer validation until the // fetched bundle can be compared with a trusted, merge-aware baseline. if (!deferBundleValidation) { - const validationError = validatePatch(patchContent); - if (validationError) return validationError; + const validation = validatePatch(patchContent); + if (validation.error) return validation.error; + protectedFilesForFallback = validation.fallbackFiles; } if (isEmpty) { @@ -934,8 +943,8 @@ async function main(config = {}) { // Deferred protected file protection – fallback-to-issue path. // Create a review issue now that we have repoParts, pullNumber, and prTitle available. - if (protectedFilesForFallback.files && protectedFilesForFallback.files.length > 0) { - return await createProtectedFilesFallbackIssue(protectedFilesForFallback.files); + if (protectedFilesForFallback.length > 0) { + return await createProtectedFilesFallbackIssue(protectedFilesForFallback); } const hasChanges = !isEmpty; @@ -1179,16 +1188,17 @@ async function main(config = {}) { const validationBase = await getBundleValidationBase(exec, baseGitOpts, rangeBaseRef, bundleRef, trustedBase); validationBaseRef = validationBase.ref; validationConflictFiles = validationBase.conflictedFiles; - let validationPatch = patchContent; - if (validationBaseRef !== rangeBaseRef) { + const trustedBaseInValidationBase = validationBaseRef !== rangeBaseRef; + if (trustedBaseInValidationBase) { core.info(`Bundle validation baseline includes trusted base ${trustedBase}: ${validationBaseRef}`); - const preview = await exec.getExecOutput("git", ["diff", "--binary", "--no-ext-diff", "--no-textconv", validationBaseRef, bundleRef], baseGitOpts); - validationPatch = preview.stdout; } + const agentHistoryExclusions = [rangeBaseRef]; + if (trustedBaseInValidationBase) agentHistoryExclusions.push(trustedBase); if (deferBundleValidation) { - const validationError = validatePatch(validationPatch); - if (validationError) return validationError; - if (protectedFilesForFallback.files?.length) return await createProtectedFilesFallbackIssue(protectedFilesForFallback.files); + const agentPatchStream = await exec.getExecOutput("git", ["log", "--format=", "--patch", "--binary", "--no-ext-diff", "--no-textconv", "--diff-merges=remerge", bundleRef, "--not", ...agentHistoryExclusions], baseGitOpts); + const validation = validatePatch(agentPatchStream.stdout); + if (validation.error) return validation.error; + if (validation.fallbackFiles.length) return await createProtectedFilesFallbackIssue(validation.fallbackFiles); } // SECURITY: Use git's own diff against the fetched bundle ref as the @@ -1198,14 +1208,14 @@ async function main(config = {}) { // content preserved only by the bundle transport). { let bundleFiles = await getBundlePreApplyFiles(exec, baseGitOpts, validationBaseRef, bundleRef, validationConflictFiles); - if (validationBaseRef !== rangeBaseRef) { - const octopus = await exec.getExecOutput("git", ["rev-list", "--min-parents=3", bundleRef, "--not", rangeBaseRef, trustedBase], baseGitOpts); + if (deferBundleValidation) { + const octopus = await exec.getExecOutput("git", ["rev-list", "--min-parents=3", bundleRef, "--not", ...agentHistoryExclusions], baseGitOpts); if (octopus.stdout.trim()) { return { success: false, error: "Cannot validate agent-created octopus merge commits in the bundle. Use two-parent merges instead." }; } // Preserve protection for agent commits that change then revert a // file, without attributing trusted base-branch commits to the agent. - const history = await exec.getExecOutput("git", ["log", "--format=", "--name-only", "--no-renames", "--diff-merges=remerge", "-z", bundleRef, "--not", rangeBaseRef, trustedBase], baseGitOpts); + const history = await exec.getExecOutput("git", ["log", "--format=", "--name-only", "--no-renames", "--diff-merges=remerge", "-z", bundleRef, "--not", ...agentHistoryExclusions], baseGitOpts); bundleFiles = [...new Set([...bundleFiles, ...history.stdout.split("\0").filter(Boolean)])]; } if (bundleFiles.length > 0) { @@ -1521,12 +1531,8 @@ async function main(config = {}) { } // When signed commits are required, the GitHub GraphQL createCommitOnBranch mutation - // cannot represent merge commits. If the range to be pushed contains merge commits - // (e.g. the agent ran `git merge origin/main` instead of `git rebase origin/main`), - // squash the entire range into a single regular commit that carries the same tree. - // This preserves the file-level outcome while producing a linear history that can - // be signed. A warning is emitted so workflow authors and agents know that rebase - // should be preferred over merge in future runs. + // cannot represent merge commits. Squash merge-containing history into one regular + // commit that preserves the tree; unsigned bundle pushes keep their merge history. if (signedCommits && hasChanges) { const squashBase = rangeBaseRef; try { @@ -1536,7 +1542,7 @@ async function main(config = {}) { core.warning( `push_to_pull_request_branch: detected ${mergeCount} merge commit(s) in range ${squashBase}..HEAD. ` + `Merge commits cannot be pushed as signed commits. Squashing the range into a single regular commit to preserve content. ` + - `To avoid this, use 'git rebase' instead of 'git merge' when updating the PR branch.` + `Merge commits are squashed because signed commits cannot include merges. Set signed-commits: false to preserve merge history.` ); // Prefer the message from the first non-merge commit in the range — it carries // the most meaningful description of the actual work. Fall back to the last diff --git a/actions/setup/js/push_to_pull_request_branch.test.cjs b/actions/setup/js/push_to_pull_request_branch.test.cjs index 8cc668547bc..926834013e4 100644 --- a/actions/setup/js/push_to_pull_request_branch.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.test.cjs @@ -2327,12 +2327,15 @@ index 0000000..abc1234 it.each([ ["small agent change", "small patch\n", 16, "merge-agent.txt", null, false], ["oversized agent patch", "x".repeat(2048), 16, "merge-agent.txt", "Patch size", false], + ["oversized transient agent commit patch", "small patch\n", 16, "merge-agent.txt", "Patch size", false, "", false, false, "x".repeat(2048)], ["oversized expanded agent content", "small patch\n", 2048, "merge-agent.txt", "Changed content size", false], ["agent modification of a protected file", protectedPatch, 16, "package.json", "protected files", false], ["oversized unchanged-side binary resolution", "", 2048, "", "Changed content size", true], ["reverted protected-file change", "small patch\n", 16, "merge-agent.txt", "bundle modifies protected files", false, "package.json"], + ["reverted protected-file change after trusted-base fallback", "small patch\n", 16, "merge-agent.txt", "bundle modifies protected files", false, "package.json", false, true], ["agent-created octopus merge", "small patch\n", 16, "merge-agent.txt", "octopus merge", false, "", true], - ])("validates %s independently of merged base changes", async (_name, agentPatch, contentSize, changedFile, expectedError, binaryConflict, historyFile = "", octopusMerge = false) => { + ["agent-created octopus merge after trusted-base fallback", "small patch\n", 16, "merge-agent.txt", "octopus merge", false, "", true, true], + ])("validates %s independently of merged base changes", async (_name, agentPatch, contentSize, changedFile, expectedError, binaryConflict, historyFile = "", octopusMerge = false, mergeTreeFailure = false, historyPatch = agentPatch) => { const branch = "merge-size-validation"; const trustedBase = "b".repeat(40); const validationTree = "a".repeat(40); @@ -2346,9 +2349,13 @@ index 0000000..abc1234 if (args[0] === "merge-base") { return { exitCode: args[3] === "remote-head" ? 1 : 0, stdout: "", stderr: "" }; } - if (args[0] === "merge-tree") return { exitCode: binaryConflict ? 1 : 0, stdout: validationTree + (binaryConflict ? "\0merge-agent.txt\0\0" : "\0"), stderr: "" }; + if (args[0] === "merge-tree") { + if (mergeTreeFailure) return { exitCode: 128, stdout: "", stderr: "unsupported merge-tree" }; + return { exitCode: binaryConflict ? 1 : 0, stdout: validationTree + (binaryConflict ? "\0merge-agent.txt\0\0" : "\0"), stderr: "" }; + } if (args[0] === "diff" && args[1] === "--binary") return { exitCode: 0, stdout: agentPatch, stderr: "" }; if (args[0] === "diff" && args[1] === "--name-only") return { exitCode: 0, stdout: changedFile ? changedFile + "\0" : "", stderr: "" }; + if (args[0] === "log" && args.includes("--patch")) return { exitCode: 0, stdout: historyPatch, stderr: "" }; if (args[0] === "log" && args[1] === "--format=") return { exitCode: 0, stdout: historyFile ? historyFile + "\0" : "", stderr: "" }; if (args[0] === "rev-list" && args[1] === "--min-parents=3") return { exitCode: 0, stdout: octopusMerge ? "c".repeat(40) + "\n" : "", stderr: "" }; return { exitCode: 0, stdout: "remote-head\n", stderr: "" }; @@ -2370,11 +2377,12 @@ index 0000000..abc1234 } expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", trustedBase], expect.any(Object)); if (expectedError === "bundle modifies protected files") { - expect(mockExec.getExecOutput).toHaveBeenCalledWith( - "git", - ["log", "--format=", "--name-only", "--no-renames", "--diff-merges=remerge", "-z", "refs/bundles/push-feature-branch", "--not", "remote-head", trustedBase], - expect.any(Object) - ); + const historyExclusions = mergeTreeFailure ? ["remote-head"] : ["remote-head", trustedBase]; + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["log", "--format=", "--name-only", "--no-renames", "--diff-merges=remerge", "-z", "refs/bundles/push-feature-branch", "--not", ...historyExclusions], expect.any(Object)); + } + if (expectedError === "octopus merge") { + const historyExclusions = mergeTreeFailure ? ["remote-head"] : ["remote-head", trustedBase]; + expect(mockExec.getExecOutput).toHaveBeenCalledWith("git", ["rev-list", "--min-parents=3", "refs/bundles/push-feature-branch", "--not", ...historyExclusions], expect.any(Object)); } if (expectedError === "Changed content size") { expect(mockExec.exec).toHaveBeenCalledWith("git", ["reset", "--hard", "remote-head"], expect.any(Object)); @@ -2396,6 +2404,20 @@ index 0000000..abc1234 expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("using PR-head validation")); }); + it("keeps all conflicted paths when merge-tree omits its empty separator", async () => { + const validationTree = "a".repeat(40); + mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + if (args[0] === "merge-base") return { exitCode: args[3] === "old-head" ? 1 : 0, stdout: "", stderr: "" }; + if (args[0] === "merge-tree") return { exitCode: 1, stdout: `${validationTree}\0only-conflict.txt`, stderr: "" }; + return { exitCode: 0, stdout: "", stderr: "" }; + }); + const module = await loadModule(); + expect(await module.getBundleValidationBase(mockExec, {}, "old-head", "candidate", "trusted-base")).toEqual({ + ref: validationTree, + conflictedFiles: ["only-conflict.txt"], + }); + }); + it("retains the preview size limit when the trusted base fetch fails", async () => { const branch = "merge-base-fetch-fails"; createPatchFile(branch, "x".repeat(4096)); @@ -2406,6 +2428,10 @@ index 0000000..abc1234 if (args[0] === "fetch" && args[2] === pr.base.sha) throw new Error("base unavailable"); return 0; }); + mockExec.getExecOutput.mockImplementation(async (_cmd, args) => { + if (args[0] === "log" && args.includes("--patch")) return { exitCode: 0, stdout: "x".repeat(4096), stderr: "" }; + return { exitCode: 0, stdout: "", stderr: "" }; + }); const module = await loadModule(); const handler = await module.main({ max_patch_size: 1, signed_commits: false }); const result = await handler({ branch, diff_size: 0 }, {}); diff --git a/actions/setup/js/safe_outputs_tools.json b/actions/setup/js/safe_outputs_tools.json index 4d1edf8b5e2..92077d154d8 100644 --- a/actions/setup/js/safe_outputs_tools.json +++ b/actions/setup/js/safe_outputs_tools.json @@ -1478,7 +1478,7 @@ }, { "name": "push_to_pull_request_branch", - "description": "Push committed changes to a pull request's branch. APPEND-ONLY: this tool adds new commits on top of the existing PR branch \u2014 force-push is NOT supported and will be rejected. Use this to add follow-up commits to an existing PR, such as addressing review feedback or fixing issues. This is a write-once declaration for a real intended PR branch update, not a sandbox or probe: do not call it with probe branches, placeholder commit messages, or auth experiments. If you are not ready to push the real update, use noop or report_incomplete instead. Changes must be committed locally before calling this tool. IMPORTANT: always supply the 'branch' argument with the local branch name you committed to. In batch workflows that process multiple PRs, this is required \u2014 if omitted, the branch is inferred from the current git HEAD, which will produce wrong results if the workspace has been checked out to a different branch between commit and tool-call time. IMPORTANT: do NOT use 'git merge' to update the branch against another branch \u2014 merge commits cannot be signed; the action will attempt to squash them into a single linear commit before pushing, but this rewrites history. Use 'git rebase' instead to avoid the rewrite. This tool auto-pins the PR branch HEAD before pushing, so it takes no concurrency-control / compare-and-swap parameter \u2014 do NOT pass expected_head_sha, head_sha, or base_sha; the only accepted fields are message, branch, pull_request_number, and repo.", + "description": "Push committed changes to a pull request's branch. APPEND-ONLY: this tool adds new commits on top of the existing PR branch \u2014 force-push is NOT supported and will be rejected. Use this to add follow-up commits to an existing PR, such as addressing review feedback or fixing issues. This is a write-once declaration for a real intended PR branch update, not a sandbox or probe: do not call it with probe branches, placeholder commit messages, or auth experiments. If you are not ready to push the real update, use noop or report_incomplete instead. Changes must be committed locally before calling this tool. IMPORTANT: always supply the 'branch' argument with the local branch name you committed to. In batch workflows that process multiple PRs, this is required \u2014 if omitted, the branch is inferred from the current git HEAD, which will produce wrong results if the workspace has been checked out to a different branch between commit and tool-call time. IMPORTANT: do NOT rebase or otherwise rewrite existing PR commits: this append-only tool cannot publish rewritten history. To bring a base branch into the PR branch, merge an already-fetched ref (e.g., 'git merge origin/main') and commit conflict resolutions. Unsigned bundle pushes preserve merge commits; when signed commits are required, merge-containing history is squashed into a regular commit on top of the existing PR head. Only use refs already present locally; do not fetch. This tool auto-pins the PR branch HEAD before pushing, so it takes no concurrency-control / compare-and-swap parameter \u2014 do NOT pass expected_head_sha, head_sha, or base_sha; the only accepted fields are message, branch, pull_request_number, and repo.", "inputSchema": { "type": "object", "required": ["message"], diff --git a/pkg/cli/sessions_download_test.go b/pkg/cli/sessions_download_test.go index c955c6abd60..d32aa5b7666 100644 --- a/pkg/cli/sessions_download_test.go +++ b/pkg/cli/sessions_download_test.go @@ -148,7 +148,12 @@ func TestSessionsDownloadNormalizesPublishedPiObservations(t *testing.T) { require.Contains(t, output, `"numTurns":1`) } else { require.Contains(t, output, "Chronological trace") - require.Equal(t, 1, strings.Count(output, "Session download works.")) + conversationStart := strings.Index(output, "### Conversation") + timelineStart := strings.Index(output, "Full timeline") + require.GreaterOrEqual(t, conversationStart, 0) + require.Greater(t, timelineStart, conversationStart) + require.Equal(t, 1, strings.Count(output[conversationStart:timelineStart], "Session download works.")) + require.Equal(t, 1, strings.Count(output[timelineStart:], "Session download works.")) } commands, err := os.ReadFile(calls) require.NoError(t, err) diff --git a/pkg/cli/work_command_test.go b/pkg/cli/work_command_test.go index 45c80db1426..8e5ef9d6a52 100644 --- a/pkg/cli/work_command_test.go +++ b/pkg/cli/work_command_test.go @@ -185,7 +185,7 @@ func TestWorkCommandProtectedNativeDeliveryEntryPoint(t *testing.T) { "display_title": "native worker " + assignment.DispatchID, } case r.Method == http.MethodGet && path == "git/ref/heads/"+workqueue.DefaultBranch: - response = map[string]any{"ref": "refs/heads/" + workqueue.DefaultBranch, "object": map[string]string{"sha": strconv.Itoa(head)}} + response = map[string]any{"ref": "refs/heads/" + workqueue.DefaultBranch, "object": map[string]string{"sha": fmt.Sprintf("%040x", head)}} case r.Method == http.MethodGet && strings.HasPrefix(path, "git/commits/"): response = map[string]any{"tree": map[string]string{"sha": "tree"}} case r.Method == http.MethodGet && path == "git/trees/tree": @@ -556,7 +556,13 @@ func TestWorkCommandCurrentProtocolWithoutCheckout(t *testing.T) { } admin = true invalidGitSHA = true - reject("", "checkpoint_invalid", "compact") + invalidRefCommand := NewWorkCommand() + invalidRefCommand.SetOut(io.Discard) + invalidRefCommand.SetErr(io.Discard) + invalidRefCommand.SetArgs([]string{"--repo", remote, "--json", "compact"}) + if err := invalidRefCommand.Execute(); err == nil || !strings.Contains(err.Error(), "invalid queue branch reference") { + t.Fatalf("invalid queue branch reference was accepted: %v", err) + } if apiWrites != writes { t.Fatal("invalid prior Git SHA attempted a remote mutation") } diff --git a/pkg/workflow/js/safe_outputs_tools.json b/pkg/workflow/js/safe_outputs_tools.json index 4d1edf8b5e2..92077d154d8 100644 --- a/pkg/workflow/js/safe_outputs_tools.json +++ b/pkg/workflow/js/safe_outputs_tools.json @@ -1478,7 +1478,7 @@ }, { "name": "push_to_pull_request_branch", - "description": "Push committed changes to a pull request's branch. APPEND-ONLY: this tool adds new commits on top of the existing PR branch \u2014 force-push is NOT supported and will be rejected. Use this to add follow-up commits to an existing PR, such as addressing review feedback or fixing issues. This is a write-once declaration for a real intended PR branch update, not a sandbox or probe: do not call it with probe branches, placeholder commit messages, or auth experiments. If you are not ready to push the real update, use noop or report_incomplete instead. Changes must be committed locally before calling this tool. IMPORTANT: always supply the 'branch' argument with the local branch name you committed to. In batch workflows that process multiple PRs, this is required \u2014 if omitted, the branch is inferred from the current git HEAD, which will produce wrong results if the workspace has been checked out to a different branch between commit and tool-call time. IMPORTANT: do NOT use 'git merge' to update the branch against another branch \u2014 merge commits cannot be signed; the action will attempt to squash them into a single linear commit before pushing, but this rewrites history. Use 'git rebase' instead to avoid the rewrite. This tool auto-pins the PR branch HEAD before pushing, so it takes no concurrency-control / compare-and-swap parameter \u2014 do NOT pass expected_head_sha, head_sha, or base_sha; the only accepted fields are message, branch, pull_request_number, and repo.", + "description": "Push committed changes to a pull request's branch. APPEND-ONLY: this tool adds new commits on top of the existing PR branch \u2014 force-push is NOT supported and will be rejected. Use this to add follow-up commits to an existing PR, such as addressing review feedback or fixing issues. This is a write-once declaration for a real intended PR branch update, not a sandbox or probe: do not call it with probe branches, placeholder commit messages, or auth experiments. If you are not ready to push the real update, use noop or report_incomplete instead. Changes must be committed locally before calling this tool. IMPORTANT: always supply the 'branch' argument with the local branch name you committed to. In batch workflows that process multiple PRs, this is required \u2014 if omitted, the branch is inferred from the current git HEAD, which will produce wrong results if the workspace has been checked out to a different branch between commit and tool-call time. IMPORTANT: do NOT rebase or otherwise rewrite existing PR commits: this append-only tool cannot publish rewritten history. To bring a base branch into the PR branch, merge an already-fetched ref (e.g., 'git merge origin/main') and commit conflict resolutions. Unsigned bundle pushes preserve merge commits; when signed commits are required, merge-containing history is squashed into a regular commit on top of the existing PR head. Only use refs already present locally; do not fetch. This tool auto-pins the PR branch HEAD before pushing, so it takes no concurrency-control / compare-and-swap parameter \u2014 do NOT pass expected_head_sha, head_sha, or base_sha; the only accepted fields are message, branch, pull_request_number, and repo.", "inputSchema": { "type": "object", "required": ["message"], From b0782c852540f95b76b1323fbc625302665dc9bf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:17:12 +0000 Subject: [PATCH 7/7] Retry workflow timing API errors Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- .github/workflows/CWI.yml | 5 +++-- .github/workflows/cgo.yml | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/CWI.yml b/.github/workflows/CWI.yml index a8481862342..828ca374f81 100644 --- a/.github/workflows/CWI.yml +++ b/.github/workflows/CWI.yml @@ -189,13 +189,14 @@ jobs: GH_TOKEN: ${{ github.token }} CACHE_KEY: checkout-cwi-${{ github.run_id }}-${{ github.sha }} run: | - cache_id=$(gh cache list --key "$CACHE_KEY" --ref "$GITHUB_REF" --limit 1 --json id,key --jq '.[] | select(.key == env.CACHE_KEY) | .id') + cache_id=$(gh cache list --repo "$GITHUB_REPOSITORY" --key "$CACHE_KEY" --ref "$GITHUB_REF" --limit 1 --json id,key --jq '.[] | select(.key == env.CACHE_KEY) | .id') if [ -n "$cache_id" ]; then - gh cache delete "$cache_id" + gh cache delete "$cache_id" --repo "$GITHUB_REPOSITORY" fi - name: Summarize job and step durations uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9 with: + retries: 3 script: | const fs = require('fs'); diff --git a/.github/workflows/cgo.yml b/.github/workflows/cgo.yml index 7e277721ce4..3c7ad46ddb2 100644 --- a/.github/workflows/cgo.yml +++ b/.github/workflows/cgo.yml @@ -2880,13 +2880,14 @@ jobs: GH_TOKEN: ${{ github.token }} CACHE_KEY: checkout-cgo-${{ github.run_id }}-${{ github.sha }} run: | - cache_id=$(gh cache list --key "$CACHE_KEY" --ref "$GITHUB_REF" --limit 1 --json id,key --jq '.[] | select(.key == env.CACHE_KEY) | .id') + cache_id=$(gh cache list --repo "$GITHUB_REPOSITORY" --key "$CACHE_KEY" --ref "$GITHUB_REF" --limit 1 --json id,key --jq '.[] | select(.key == env.CACHE_KEY) | .id') if [ -n "$cache_id" ]; then - gh cache delete "$cache_id" + gh cache delete "$cache_id" --repo "$GITHUB_REPOSITORY" fi - name: Summarize job and step durations uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9 with: + retries: 3 script: | const fs = require('fs');