diff --git a/.github/aw/create-agentic-workflow.md b/.github/aw/create-agentic-workflow.md index ec28dacf2a0..5c97247e8ba 100644 --- a/.github/aw/create-agentic-workflow.md +++ b/.github/aw/create-agentic-workflow.md @@ -67,6 +67,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/.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'); diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index 4653bf7bab7..b1df498000a 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -117,11 +117,51 @@ 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)) { + 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 { + // 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 = "") { @@ -437,6 +477,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`); @@ -479,64 +520,80 @@ 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)}`); + 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 (!isEmptyPatch) { + 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 { error: { success: false, error: msg }, fallbackFiles }; + } - 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 (!isEmptyPatch) { + 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 { error: { success: false, skipped: true, reasonCode: POLICY_FILE_PROTECTION_DENIED_REASON_CODE, error: msg }, fallbackFiles }; + } + if (protection.action === "fallback") { + fallbackFiles.push(...protection.files); + core.warning(`Protected file protection triggered (fallback-to-issue): ${protection.files.join(", ")}. Will create review issue instead of pushing.`); + } + } + return { error: null, fallbackFiles }; } - // 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 validation = validatePatch(patchContent); + if (validation.error) return validation.error; + protectedFilesForFallback = validation.fallbackFiles; } if (isEmpty) { @@ -886,7 +943,7 @@ 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) { + if (protectedFilesForFallback.length > 0) { return await createProtectedFilesFallbackIssue(protectedFilesForFallback); } @@ -986,6 +1043,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 { @@ -1038,6 +1097,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, { @@ -1114,6 +1185,21 @@ 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; + const trustedBaseInValidationBase = validationBaseRef !== rangeBaseRef; + if (trustedBaseInValidationBase) { + core.info(`Bundle validation baseline includes trusted base ${trustedBase}: ${validationBaseRef}`); + } + const agentHistoryExclusions = [rangeBaseRef]; + if (trustedBaseInValidationBase) agentHistoryExclusions.push(trustedBase); + if (deferBundleValidation) { + 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 // authoritative pre-apply file set for bundle transport. This keeps @@ -1121,7 +1207,17 @@ 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 (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", ...agentHistoryExclusions], 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); @@ -1276,6 +1372,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. @@ -1284,8 +1381,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`); @@ -1434,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 { @@ -1449,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 @@ -1760,4 +1853,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..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 @@ -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,184 @@ 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", "--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.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 }); + 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(); + 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", ...(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", ...(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.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"); + 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); + 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 21a2f8e586d..926834013e4 100644 --- a/actions/setup/js/push_to_pull_request_branch.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.test.cjs @@ -2323,6 +2323,123 @@ 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 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], + ["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); + 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") { + 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: "" }; + }); + 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 === "bundle modifies protected files") { + 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)); + } + } 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("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)); + 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; + }); + 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 }, {}); + 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/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/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 939763e300a..e64e63cf1d2 100644 --- a/docs/src/content/docs/reference/safe-outputs-pull-requests.md +++ b/docs/src/content/docs/reference/safe-outputs-pull-requests.md @@ -435,6 +435,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. 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 `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). diff --git a/pkg/cli/sessions_download_test.go b/pkg/cli/sessions_download_test.go index db219a0d99d..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, 2, 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 29a10bfe292..7a5228caec2 100644 --- a/pkg/cli/work_command_test.go +++ b/pkg/cli/work_command_test.go @@ -556,13 +556,12 @@ func TestWorkCommandCurrentProtocolWithoutCheckout(t *testing.T) { } admin = true invalidGitSHA = true - cmd := NewWorkCommand() - cmd.SetOut(io.Discard) - cmd.SetErr(io.Discard) - cmd.SetArgs([]string{"--repo", remote, "--json", "compact"}) - err = cmd.Execute() - if err == nil || !strings.Contains(err.Error(), "invalid queue branch reference") { - t.Fatalf("invalid queue branch SHA was not rejected: %v", err) + 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 log != before { t.Fatal("invalid queue branch SHA changed queue authority") 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"],