diff --git a/actions/setup/js/push_signed_commits.cjs b/actions/setup/js/push_signed_commits.cjs index 2b89cef3011..dbf3e0f8e8a 100644 --- a/actions/setup/js/push_signed_commits.cjs +++ b/actions/setup/js/push_signed_commits.cjs @@ -9,6 +9,7 @@ const { ERR_API } = require("./error_codes.cjs"); const { loadTemporaryIdMapFromResolved, replaceTemporaryIdReferencesInPatch, TEMPORARY_ID_CANDIDATE_REFERENCE_PATTERN } = require("./temporary_id.cjs"); +const OID_PATTERN = /^[0-9a-f]{40}$/i; /** Sentinel error class used to signal that the commit range contains a shape * that the GitHub GraphQL `createCommitOnBranch` mutation cannot represent @@ -258,13 +259,45 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c } } + /** @type {string | undefined} */ + let baseRefOid; + try { + const { stdout: baseRefOut } = await exec.getExecOutput("git", ["rev-parse", `${baseRef}^{commit}`], { cwd }); + const trimmedBaseRefOid = baseRefOut.trim(); + if (OID_PATTERN.test(trimmedBaseRefOid)) { + baseRefOid = trimmedBaseRefOid; + } else if (trimmedBaseRefOid) { + core.warning( + `pushSignedCommits: git rev-parse returned an unexpected baseRef OID value for '${baseRef}'; ` + + `boundary-commit filter is disabled for this run. Check that '${baseRef}' resolves to a valid commit in this checkout. ` + + `Observed value: ${JSON.stringify(trimmedBaseRefOid)}` + ); + } + } catch (baseRefResolveError) { + core.warning( + `pushSignedCommits: could not resolve baseRef '${baseRef}' to OID; boundary-commit filter is disabled for this run and parent OID resolution may fall back to per-commit rev-parse: ${baseRefResolveError instanceof Error ? baseRefResolveError.message : String(baseRefResolveError)}` + ); + } // Collect the commits introduced (oldest-first) using topological order to ensure // correct sequencing even when commit dates are out of sync (e.g. after rebase --committer-date-is-author-date). // Using --parents emits each line as " [ ...]", which lets us detect merge commits // (more than one parent) in a single subprocess call without iterating each SHA individually. - const { stdout: revListOut } = await exec.getExecOutput("git", ["rev-list", "--parents", "--topo-order", "--reverse", `${baseRef}..HEAD`], { cwd }); - const revListLines = revListOut.trim().split("\n").filter(Boolean); - const shas = revListLines.map(line => line.split(" ")[0]); + const revListBase = baseRefOid ?? baseRef; + const { stdout: revListOut } = await exec.getExecOutput("git", ["rev-list", "--parents", "--topo-order", "--reverse", `${revListBase}..HEAD`], { cwd }); + const revListEntriesRaw = revListOut + .trim() + .split("\n") + .filter(Boolean) + .map(line => { + const fields = line.split(" "); + return { line, fields, sha: fields[0] }; + }); + const revListEntries = baseRefOid !== undefined ? revListEntriesRaw.filter(entry => entry.sha !== baseRefOid) : revListEntriesRaw; + const droppedBoundaryCount = revListEntriesRaw.length - revListEntries.length; + if (baseRefOid !== undefined && droppedBoundaryCount > 0) { + core.info(`pushSignedCommits: dropped ${droppedBoundaryCount} baseRef boundary commit(s) from replay set`); + } + const shas = revListEntries.map(entry => entry.sha); if (shas.length === 0) { core.info("pushSignedCommits: no new commits to push via GraphQL"); @@ -278,8 +311,7 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c // A line with 3+ space-separated fields means the commit has 2+ parents (i.e. a merge commit). // The GitHub GraphQL createCommitOnBranch mutation does not support multiple parents, so refuse // the unsigned push fallback if any merge commit is found. - for (const line of revListLines) { - const fields = line.split(" "); + for (const { fields } of revListEntries) { if (fields.length > 2) { const sha = fields[0]; core.warning(`pushSignedCommits: merge commit ${sha} detected, refusing unsigned push fallback`); @@ -426,8 +458,13 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c // Resolve the parent OID, create the branch on the remote via the REST API, // then proceed with the signed-commit mutation as normal. core.info(`pushSignedCommits: branch ${branch} not yet on the remote, resolving parent OID for first commit`); - const { stdout: parentOut } = await exec.getExecOutput("git", ["rev-parse", `${sha}^`], { cwd }); - expectedHeadOid = parentOut.trim(); + if (baseRefOid !== undefined) { + expectedHeadOid = baseRefOid; + core.info(`pushSignedCommits: using baseRef OID for initial branch creation: ${expectedHeadOid}`); + } else { + const { stdout: parentOut } = await exec.getExecOutput("git", ["rev-parse", `${sha}^`], { cwd }); + expectedHeadOid = parentOut.trim(); + } if (!expectedHeadOid) { throw new Error(`${ERR_API}: Could not resolve OID for new branch ${branch}`); } @@ -449,6 +486,15 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c // GitHub returns 422 "Reference refs/heads/ already exists". Treat that as success and continue. if (status === 422 && /reference.*already exists/i.test(message)) { core.info(`pushSignedCommits: remote branch ${branch} was created concurrently (422 Reference already exists); continuing with signed commits`); + const { stdout: refreshedOidOut } = await exec.getExecOutput("git", ["ls-remote", "origin", `refs/heads/${branch}`], { cwd, env: { ...process.env, ...(gitAuthEnv || {}) } }); + const refreshedHeadOid = refreshedOidOut.trim().split(/\s+/)[0]; + if (!refreshedHeadOid) { + throw new Error(`${ERR_API}: Could not resolve remote branch OID for ${branch} after concurrent creation; ls-remote output was ${JSON.stringify(refreshedOidOut)}`); + } + if (!OID_PATTERN.test(refreshedHeadOid)) { + throw new Error(`${ERR_API}: Invalid remote branch OID for ${branch} after concurrent creation; ls-remote output was ${JSON.stringify(refreshedOidOut)}`); + } + expectedHeadOid = refreshedHeadOid; } else { throw createRefError; } diff --git a/actions/setup/js/push_signed_commits.test.cjs b/actions/setup/js/push_signed_commits.test.cjs index 2367e6cccb2..fc92767aff9 100644 --- a/actions/setup/js/push_signed_commits.test.cjs +++ b/actions/setup/js/push_signed_commits.test.cjs @@ -599,6 +599,158 @@ describe("push_signed_commits integration tests", () => { expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("not yet on the remote")); }); + it("should ignore an injected baseRef boundary commit in rev-list output", async () => { + execGit(["checkout", "-b", "new-boundary-branch"], { cwd: workDir }); + fs.writeFileSync(path.join(workDir, "boundary-file.txt"), "Boundary file content\n"); + execGit(["add", "boundary-file.txt"], { cwd: workDir }); + execGit(["commit", "-m", "Add boundary-file.txt"], { cwd: workDir }); + + const baseRefOid = execGit(["rev-parse", "origin/main"], { cwd: workDir }).stdout.trim(); + const newCommitOid = execGit(["rev-parse", "HEAD"], { cwd: workDir }).stdout.trim(); + + const realExec = makeRealExec(workDir); + global.exec = { + ...realExec, + getExecOutput: vi.fn(async (program, args, opts = {}) => { + if (program === "git" && args[0] === "rev-list" && args[1] === "--parents") { + return { + exitCode: 0, + stdout: `${baseRefOid}\n${newCommitOid} ${baseRefOid}\n`, + stderr: "", + }; + } + return realExec.getExecOutput(program, args, opts); + }), + }; + const githubClient = makeMockGithubClient(); + + await pushSignedCommits({ + githubClient, + owner: "test-owner", + repo: "test-repo", + branch: "new-boundary-branch", + baseRef: "origin/main", + cwd: workDir, + }); + + expect(githubClient.rest.git.createRef).toHaveBeenCalledTimes(1); + expect(githubClient.rest.git.createRef).toHaveBeenCalledWith({ + owner: "test-owner", + repo: "test-repo", + ref: "refs/heads/new-boundary-branch", + sha: baseRefOid, + }); + expect(githubClient.graphql).toHaveBeenCalledTimes(1); + const callArg = githubClient.graphql.mock.calls[0][1].input; + expect(callArg.message.headline).toBe("Add boundary-file.txt"); + expect(callArg.expectedHeadOid).toBe(baseRefOid); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("baseRef boundary commit(s)")); + }); + + it("should drop injected baseRef boundary entries and replay only new commits in order", async () => { + execGit(["checkout", "-b", "new-boundary-multi-branch"], { cwd: workDir }); + + fs.writeFileSync(path.join(workDir, "boundary-alpha.txt"), "Alpha boundary content\n"); + execGit(["add", "boundary-alpha.txt"], { cwd: workDir }); + execGit(["commit", "-m", "Add boundary-alpha.txt"], { cwd: workDir }); + + fs.writeFileSync(path.join(workDir, "boundary-beta.txt"), "Beta boundary content\n"); + execGit(["add", "boundary-beta.txt"], { cwd: workDir }); + execGit(["commit", "-m", "Add boundary-beta.txt"], { cwd: workDir }); + + const baseRefOid = execGit(["rev-parse", "origin/main"], { cwd: workDir }).stdout.trim(); + const alphaCommitOid = execGit(["rev-parse", "HEAD~1"], { cwd: workDir }).stdout.trim(); + const betaCommitOid = execGit(["rev-parse", "HEAD"], { cwd: workDir }).stdout.trim(); + + const realExec = makeRealExec(workDir); + global.exec = { + ...realExec, + getExecOutput: vi.fn(async (program, args, opts = {}) => { + if (program === "git" && args[0] === "rev-list" && args[1] === "--parents") { + return { + exitCode: 0, + stdout: `${baseRefOid}\n${alphaCommitOid} ${baseRefOid}\n${betaCommitOid} ${alphaCommitOid}\n`, + stderr: "", + }; + } + return realExec.getExecOutput(program, args, opts); + }), + }; + const githubClient = makeMockGithubClient({ oid: "signed-oid-first" }); + + await pushSignedCommits({ + githubClient, + owner: "test-owner", + repo: "test-repo", + branch: "new-boundary-multi-branch", + baseRef: "origin/main", + cwd: workDir, + }); + + expect(githubClient.rest.git.createRef).toHaveBeenCalledTimes(1); + expect(githubClient.rest.git.createRef).toHaveBeenCalledWith({ + owner: "test-owner", + repo: "test-repo", + ref: "refs/heads/new-boundary-multi-branch", + sha: baseRefOid, + }); + expect(githubClient.graphql).toHaveBeenCalledTimes(2); + expect(githubClient.graphql.mock.calls[0][1].input.expectedHeadOid).toBe(baseRefOid); + expect(githubClient.graphql.mock.calls[1][1].input.expectedHeadOid).toBe("signed-oid-first"); + const committedHeadlines = githubClient.graphql.mock.calls.map(call => call[1].input.message.headline); + expect(committedHeadlines).toEqual(["Add boundary-alpha.txt", "Add boundary-beta.txt"]); + + const attemptedBoundaryParentResolution = global.exec.getExecOutput.mock.calls.some(([program, args]) => program === "git" && args[0] === "rev-parse" && args[1] === `${baseRefOid}^`); + expect(attemptedBoundaryParentResolution).toBe(false); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("dropped 1 baseRef boundary commit")); + }); + + it("should fall back to per-commit parent resolution when baseRef OID resolution fails", async () => { + execGit(["checkout", "-b", "new-base-ref-failure-branch"], { cwd: workDir }); + fs.writeFileSync(path.join(workDir, "fallback-file.txt"), "Fallback content\n"); + execGit(["add", "fallback-file.txt"], { cwd: workDir }); + execGit(["commit", "-m", "Add fallback-file.txt"], { cwd: workDir }); + + const newCommitOid = execGit(["rev-parse", "HEAD"], { cwd: workDir }).stdout.trim(); + const expectedParentOid = execGit(["rev-parse", "HEAD^"], { cwd: workDir }).stdout.trim(); + + const realExec = makeRealExec(workDir); + global.exec = { + ...realExec, + getExecOutput: vi.fn(async (program, args, opts = {}) => { + if (program === "git" && args[0] === "rev-parse" && args[1] === "origin/main^{commit}") { + throw new Error("simulated rev-parse failure"); + } + return realExec.getExecOutput(program, args, opts); + }), + }; + const githubClient = makeMockGithubClient(); + + await pushSignedCommits({ + githubClient, + owner: "test-owner", + repo: "test-repo", + branch: "new-base-ref-failure-branch", + baseRef: "origin/main", + cwd: workDir, + }); + + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("boundary-commit filter is disabled for this run")); + expect(githubClient.rest.git.createRef).toHaveBeenCalledTimes(1); + expect(githubClient.rest.git.createRef).toHaveBeenCalledWith({ + owner: "test-owner", + repo: "test-repo", + ref: "refs/heads/new-base-ref-failure-branch", + sha: expectedParentOid, + }); + expect(githubClient.graphql).toHaveBeenCalledTimes(1); + expect(githubClient.graphql.mock.calls[0][1].input.message.headline).toBe("Add fallback-file.txt"); + expect(githubClient.graphql.mock.calls[0][1].input.expectedHeadOid).toBe(expectedParentOid); + + const hasPerCommitParentCall = global.exec.getExecOutput.mock.calls.some(([program, args]) => program === "git" && args[0] === "rev-parse" && args[1] === `${newCommitOid}^`); + expect(hasPerCommitParentCall).toBe(true); + }); + it("should create remote branch once then chain GraphQL OIDs for multiple commits on a new branch", async () => { // Create a local branch with two commits but do NOT push it execGit(["checkout", "-b", "new-multi-commit-branch"], { cwd: workDir }); @@ -657,7 +809,21 @@ describe("push_signed_commits integration tests", () => { const expectedParentOid = execGit(["rev-parse", "HEAD^"], { cwd: workDir }).stdout.trim(); - global.exec = makeRealExec(workDir); + const realExec = makeRealExec(workDir); + let lsRemoteCallCount = 0; + global.exec = { + ...realExec, + getExecOutput: vi.fn(async (program, args, opts = {}) => { + if (program === "git" && args[0] === "ls-remote" && args[2] === "refs/heads/race-condition-branch") { + lsRemoteCallCount += 1; + if (lsRemoteCallCount === 1) { + return { exitCode: 0, stdout: "", stderr: "" }; + } + return { exitCode: 0, stdout: `${expectedParentOid}\trefs/heads/race-condition-branch\n`, stderr: "" }; + } + return realExec.getExecOutput(program, args, opts); + }), + }; // Simulate concurrent branch creation: createRef throws 422 (GitHub API exact format) const concurrentError = Object.assign(new Error("Reference refs/heads/race-condition-branch already exists"), { status: 422 });