Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/skills/agentic-workflows/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ Load these files from `github/gh-aw` (they are not available locally).
- `.github/aw/evals.md`
- `.github/aw/experiments.md`
- `.github/aw/github-agentic-workflows.md`
- `.github/aw/github-mcp-server-pagination.md`
- `.github/aw/github-mcp-server.md`
- `.github/aw/instructions.md`
- `.github/aw/linter-workflows.md`
Expand Down Expand Up @@ -64,10 +65,12 @@ Load these files from `github/gh-aw` (they are not available locally).
- `.github/aw/subagents.md`
- `.github/aw/syntax-agentic.md`
- `.github/aw/syntax-core.md`
- `.github/aw/syntax-engine.md`
- `.github/aw/syntax-tools-imports.md`
- `.github/aw/syntax.md`
- `.github/aw/test-coverage.md`
- `.github/aw/test-expression.md`
- `.github/aw/token-optimization-caching-budgets.md`
- `.github/aw/token-optimization.md`
- `.github/aw/triggers.md`
- `.github/aw/update-agentic-workflow.md`
Expand Down
53 changes: 46 additions & 7 deletions actions/setup/js/create_pull_request.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const { isStagedMode } = require("./safe_output_helpers.cjs");
const { normalizeCommitSHA } = require("./commit_sha_helpers.cjs");
const { withRetry, RATE_LIMIT_RETRY_CONFIG } = require("./error_recovery.cjs");
const { findAgent, getIssueDetails, assignAgentToIssue } = require("./assign_agent_helpers.cjs");
const { ensureFullHistoryForBundle, extractBundlePrerequisiteCommits, isShallowOrSparseCheckout, linearizeRangeAsCommit } = require("./git_helpers.cjs");
const { ensureFullHistoryForBundle, extractBundlePrerequisiteCommits, getBundlePrerequisites, isShallowOrSparseCheckout, linearizeRangeAsCommit } = require("./git_helpers.cjs");
const { parseDiffGitHeader: parseDiffGitHeaderPaths, extractDiffGitHeaderEntries } = require("./patch_path_helpers.cjs");
const { resolveTransportPaths } = require("./resolve_transport_paths.cjs");
const { resolveAllowedMentionsFromPayload } = require("./resolve_mentions_from_payload.cjs");
Expand Down Expand Up @@ -322,15 +322,54 @@ async function applyBundleToBranch(bundleFilePath, branchName, originalAgentBran
}

/**
* Rewrites the current branch to a single non-merge commit relative to origin/<baseBranch>.
* This is used as a recovery path when signed commit replay rejects merge commit topology.
* Rewrites the current branch to a single non-merge commit relative to the bundle's
* actual base commit (the prerequisite SHA recorded in the bundle). Falls back to
* origin/<baseBranch> when the bundle prerequisite cannot be determined.
*
* Using the bundle's prerequisite SHA instead of the current origin/<baseBranch> tip
* ensures the linearized commit only contains the agent's actual changes and does not
* revert or absorb commits that were added to the base branch after the agent's checkout.
*
* @param {string} baseBranch
* @param {{ exec: Function, getExecOutput: Function }} execApi
* @param {string} [bundleFilePath] - Optional path to the bundle file; used to extract the
* precise base commit the agent worked from.
* @returns {Promise<void>}
*/
async function rewriteBundleBranchAsSingleCommit(baseBranch, execApi) {
const baseRef = `origin/${baseBranch}`;
async function rewriteBundleBranchAsSingleCommit(baseBranch, execApi, bundleFilePath) {
const fallbackBaseRef = `origin/${baseBranch}`;
let baseRef = fallbackBaseRef;

if (bundleFilePath) {
try {
const prereqs = await getBundlePrerequisites(execApi, bundleFilePath);
if (prereqs.length === 1) {
// Guard: verify the prerequisite SHA is accessible in the local repository
// before using it as a linearization base. In a shallow clone the commit
// may not have been fetched, causing `git reset --soft <sha>` to fail.
// We check reachability here — before synthesizing any commit — so we can
// fall back cleanly rather than letting linearizeRangeAsCommit abort mid-run.
const prereqSha = prereqs[0];
try {
const { exitCode } = await execApi.getExecOutput("git", ["cat-file", "-e", `${prereqSha}^{commit}`], { ignoreReturnCode: true, silent: true });
if (exitCode === 0) {
baseRef = prereqSha;
core.info(`Using bundle prerequisite commit ${baseRef} as linearization base (avoids including base-branch drift)`);
} else {
core.info(`Bundle prerequisite ${prereqSha} not accessible locally; falling back to ${fallbackBaseRef} as linearization base`);
}
} catch {
core.info(`Could not verify bundle prerequisite accessibility; falling back to ${fallbackBaseRef} as linearization base`);
}
} else if (prereqs.length > 1) {
core.info(`Bundle has ${prereqs.length} prerequisite commits; falling back to ${fallbackBaseRef} as linearization base`);
} else {
core.info(`Bundle declares no prerequisites; falling back to ${fallbackBaseRef} as linearization base`);
}
} catch (prereqError) {
core.warning(`Could not extract bundle prerequisites: ${getErrorMessage(prereqError)}; falling back to ${fallbackBaseRef}`);
}
}

let commitHeadline = "Apply bundled create_pull_request changes";
try {
Expand Down Expand Up @@ -1678,7 +1717,7 @@ async function main(config = {}) {
if (isSignedMergeReplayRefusal) {
core.warning("Signed push rejected merge commit topology from bundle; rewriting branch and retrying signed push");
try {
await rewriteBundleBranchAsSingleCommit(baseBranch, exec);
await rewriteBundleBranchAsSingleCommit(baseBranch, exec, bundleFilePath);
const runRetryPush = async () =>
pushSignedCommits({
githubClient: pushGithubClient,
Expand Down Expand Up @@ -2744,4 +2783,4 @@ ${patchPreview}`;
}; // End of handleCreatePullRequest
} // End of main

module.exports = { main, enforcePullRequestLimits, countUniquePatchFiles, parseDiffGitHeader, applyBundleToBranch };
module.exports = { main, enforcePullRequestLimits, countUniquePatchFiles, parseDiffGitHeader, applyBundleToBranch, rewriteBundleBranchAsSingleCommit };
122 changes: 122 additions & 0 deletions actions/setup/js/create_pull_request_bundle_integration.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -435,4 +435,126 @@ describe("create_pull_request bundle integration", () => {
expect(rawPushError).toContain("@org/team");
core.info("[reconcile-spark] push error captured — ready for sanitization and fallback issue embedding");
});

it("rewriteBundleBranchAsSingleCommit uses bundle prerequisite SHA to avoid including base-branch drift", async () => {
// ─── Why this test exists ────────────────────────────────────────────────
//
// Production race: the agent creates its bundle while main is at commit A.
// Between bundle creation and safe-outputs applying it, a collaborator
// advances main to commit B (adding "base-drift.txt"). The safe-outputs
// runner clones origin/main at B, applies the bundle, then calls
// rewriteBundleBranchAsSingleCommit.
//
// With the naive fix (soft-reset to origin/main = B), the staged diff is
// {agent-file.txt added, base-drift.txt deleted}, so the synthesized commit
// incorrectly deletes base-drift.txt.
//
// The correct fix: read the prerequisite SHA from the bundle (= commit A,
// the exact base the agent started from) and soft-reset to that SHA.
// Staged diff becomes {agent-file.txt added} only.
//
// This test models the production race faithfully:
// 1. Bare remote with initial commit A on main.
// 2. Agent clones at A, creates feature branch, adds "agent-file.txt".
// 3. Agent bundles while main is still at A → prereq = A.
// 4. Collaborator advances main to B ("base-drift.txt").
// 5. Safe-outputs clones updated origin (B on main), applies bundle.
// 6. rewriteBundleBranchAsSingleCommit is called.
// 7. Synthesized commit MUST contain only "agent-file.txt" (from agent).
// 8. "base-drift.txt" MUST NOT appear anywhere in the synthesized diff
// (neither as added nor as deleted — it was never the agent's change).
// ─────────────────────────────────────────────────────────────────────────

const bareRemote = fs.mkdtempSync(path.join(os.tmpdir(), "create-pr-moving-base-bare-"));
const agentRepo = fs.mkdtempSync(path.join(os.tmpdir(), "create-pr-moving-base-agent-"));
const safeOutputsRepo = fs.mkdtempSync(path.join(os.tmpdir(), "create-pr-moving-base-so-"));
tempDirs.push(bareRemote, agentRepo, safeOutputsRepo);

// 1. Initialize bare remote and seed with initial commit A.
execGit(["init", "--bare", "-b", "main"], { cwd: bareRemote });
execGit(["clone", bareRemote, "."], { cwd: agentRepo });
execGit(["config", "user.name", "Agent"], { cwd: agentRepo });
execGit(["config", "user.email", "agent@example.com"], { cwd: agentRepo });
fs.writeFileSync(path.join(agentRepo, "README.md"), "# Project\n");
execGit(["add", "README.md"], { cwd: agentRepo });
execGit(["commit", "-m", "Initial commit"], { cwd: agentRepo });
execGit(["branch", "-M", "main"], { cwd: agentRepo });
execGit(["push", "-u", "origin", "main"], { cwd: agentRepo });

// Capture commit A — the exact base the agent starts from.
const agentBaseCommit = execGit(["rev-parse", "HEAD"], { cwd: agentRepo }).stdout.trim();

// 2. Agent creates feature branch and adds a file (main is still at A here).
const branchName = "feature/moving-base-test";
execGit(["checkout", "-b", branchName], { cwd: agentRepo });
fs.writeFileSync(path.join(agentRepo, "agent-file.txt"), "agent change\n");
execGit(["add", "agent-file.txt"], { cwd: agentRepo });
execGit(["commit", "-m", "feat: agent adds a file"], { cwd: agentRepo });

// 3. Agent bundles while main is STILL at A (before any drift).
// The bundle prerequisite is therefore A — the agent's actual base.
const bundlePath = path.join(agentRepo, "feature.bundle");
execGit(["bundle", "create", bundlePath, `${agentBaseCommit}..refs/heads/${branchName}`], { cwd: agentRepo });

// Confirm the bundle's prerequisite is recorded as agentBaseCommit (A).
const verifyOutput = execGit(["bundle", "verify", bundlePath], { cwd: agentRepo, allowFailure: true });
const combinedVerify = `${verifyOutput.stdout || ""}\n${verifyOutput.stderr || ""}`;
expect(combinedVerify).toMatch(new RegExp(agentBaseCommit.slice(0, 8), "i"));

// 4. AFTER bundling: collaborator advances main to B (base-branch drift).
execGit(["checkout", "main"], { cwd: agentRepo });
fs.writeFileSync(path.join(agentRepo, "base-drift.txt"), "base drift added after agent checkout\n");
execGit(["add", "base-drift.txt"], { cwd: agentRepo });
execGit(["commit", "-m", "chore: add base-drift file"], { cwd: agentRepo });
execGit(["push", "origin", "main"], { cwd: agentRepo });

// 5. Safe-outputs runner clones the *updated* origin/main (B — includes base-drift.txt).
execGit(["clone", bareRemote, "."], { cwd: safeOutputsRepo });
execGit(["config", "user.name", "Runner"], { cwd: safeOutputsRepo });
execGit(["config", "user.email", "runner@example.com"], { cwd: safeOutputsRepo });
const newOriginMainSha = execGit(["rev-parse", "origin/main"], { cwd: safeOutputsRepo }).stdout.trim();
expect(newOriginMainSha).not.toBe(agentBaseCommit); // Confirms base has moved.

// Confirm base-drift.txt is on origin/main (the drift file is present on the remote).
const driftOnOriginMain = execGit(["show", "origin/main:base-drift.txt"], { cwd: safeOutputsRepo, allowFailure: true });
expect(driftOnOriginMain.status).toBe(0);

// 6. Apply the bundle to the safe-outputs repo (bundle prereq = A, not B).
const { applyBundleToBranch, rewriteBundleBranchAsSingleCommit } = require("./create_pull_request.cjs");
await applyBundleToBranch(bundlePath, branchName, `refs/heads/${branchName}`, createExecApi(safeOutputsRepo), "main");

// 7. Call rewriteBundleBranchAsSingleCommit with the bundle path so it reads prereq A.
await rewriteBundleBranchAsSingleCommit("main", createExecApi(safeOutputsRepo), bundlePath);

// 8. Verify the synthesized commit.
//
// a) "agent-file.txt" must be in the working tree (agent's actual change was preserved).
// b) "base-drift.txt" must NOT be in the working tree — the rewrite was anchored at A
// (which predates the drift), so the feature branch tree never included it.
// c) The diff HEAD^..HEAD must show "agent-file.txt" as added.
// d) "base-drift.txt" must NOT appear in the diff at all — not deleted, not added.
// (With the naive soft-reset to origin/main=B, it would appear as "D base-drift.txt".)
// e) HEAD must have exactly one parent (linear, not a merge commit).
const agentFilePresent = fs.existsSync(path.join(safeOutputsRepo, "agent-file.txt"));
const baseDriftInWorkingTree = fs.existsSync(path.join(safeOutputsRepo, "base-drift.txt"));

expect(agentFilePresent).toBe(true);
// base-drift.txt belongs to origin/main (B), not to the feature branch anchored at A.
expect(baseDriftInWorkingTree).toBe(false);

// The HEAD commit (linearized) should show "agent-file.txt" as added.
const headStat = execGit(["show", "--stat", "HEAD"], { cwd: safeOutputsRepo }).stdout;
expect(headStat).toContain("agent-file.txt");

// The diff introduced by HEAD must NOT reference base-drift.txt at all.
// If the prereq-SHA path were bypassed and origin/main (B) were used instead,
// the diff would contain "D base-drift.txt" and this assertion would fail.
const diffStat = execGit(["diff", "--name-status", "HEAD^", "HEAD"], { cwd: safeOutputsRepo }).stdout;
expect(diffStat).not.toMatch(/base-drift\.txt/);

// Verify the commit has exactly one parent (linearized, not a merge commit).
const parentLine = execGit(["log", "-1", "--format=%P", "HEAD"], { cwd: safeOutputsRepo }).stdout.trim();
const parentShas = parentLine.split(/\s+/).filter(Boolean);
expect(parentShas).toHaveLength(1);
});
});
1 change: 1 addition & 0 deletions actions/setup/js/git_helpers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,7 @@ module.exports = {
ensureFullHistoryForBundle,
ensureOriginRemoteTrackingRef,
extractBundlePrerequisiteCommits,
getBundlePrerequisites,
getGitAuthEnv,
hasMergeCommitsInRange,
isShallowOrSparseCheckout,
Expand Down
Loading
Loading