safe-outputs: make create-pull-request bundling resilient to diverged-history replay conflicts

.github/workflows/agentic-auto-upgrade.yml

@@ -23,10 +23,10 @@
 #
 # Alternative regeneration methods:
 # make recompile
-# 
+#
 # Or use the gh-aw CLI directly:
 # ./gh-aw compile --validate --verbose
-# 
+#
 # The workflow is generated when auto_upgrade is set to true in aw.json.
 # The weekly schedule is deterministically scattered based on the repository slug.
 #

.github/workflows/agentics-maintenance.yml

@@ -23,13 +23,13 @@
 #
 # This file defines the generated agentic maintenance workflow for this repository.
 # It runs scheduled cleanup for expiring safe outputs and supports manual maintenance operations.
-# 
+#
 # This workflow is generated automatically when workflows use expiring safe outputs
 # or when repository maintenance features are enabled in .github/workflows/aw.json.
-# 
+#
 # To disable maintenance workflow generation, set in .github/workflows/aw.json:
 # {"maintenance": false}
-# 
+#
 # Agentic maintenance docs:
 # https://github.github.com/gh-aw/reference/ephemerals/#manual-maintenance-operations
 #

actions/setup/js/create_pull_request.cjs

@@ -1625,13 +1625,18 @@ async function main(config = {}) {
                 const artifactFileName = bundleFilePath ? bundleFilePath.replace("/tmp/gh-aw/", "") : "aw-unknown.bundle";
                 const fallbackBundleSourceRef = `refs/heads/${originalAgentBranch || branchName}`;
                 const fallbackBundleTempRef = createBundleTempRef(branchName);
+                const pushFailureMessage = sanitizeContent(neutralizeClosingKeywordsForIssueBody(pushError instanceof Error ? pushError.message : String(pushError)), { allowedAliases: allowedMentionAliases })
+                  .replace(/\s+/g, " ")
+                  .trim();
                 const fallbackBody = `${issueSafeBody}
 
 ---
 
 > [!NOTE]
 > This was originally intended as a pull request, but the git push operation failed.
 >
+> **Original error:** ${pushFailureMessage}
+>
 > **Workflow Run:** [View run details and download bundle artifact](${runUrl})
 >
 > The bundle file is available in the \`agent\` artifact in the workflow run linked above.
@@ -1966,13 +1971,18 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${branchName} --repo
                 }
 
                 const patchFileName = patchFilePath ? patchFilePath.replace("/tmp/gh-aw/", "") : "aw-unknown.patch";
+                const pushFailureMessage = sanitizeContent(neutralizeClosingKeywordsForIssueBody(pushError instanceof Error ? pushError.message : String(pushError)), { allowedAliases: allowedMentionAliases })
+                  .replace(/\s+/g, " ")
+                  .trim();
                 const fallbackBody = `${issueSafeBody}
 
 ---
 
 > [!NOTE]
 > This was originally intended as a pull request, but the git push operation failed.
 >
+> **Original error:** ${pushFailureMessage}
+>
 > **Workflow Run:** [View run details and download patch artifact](${runUrl})
 >
 > The patch file is available in the \`agent\` artifact in the workflow run linked above.

actions/setup/js/create_pull_request.test.cjs

@@ -1018,6 +1018,48 @@ index 0000000..abc1234
     expect(fallbackIssueBody).toContain("git reset --hard");
     expect(fallbackIssueBody).toContain(`git update-ref -d ${fallbackBundleTempRef}`);
     expect(fallbackIssueBody).not.toContain("refs/heads/autoloop/perf-comparison:refs/heads/autoloop/perf-comparison");
+    expect(fallbackIssueBody).toContain("**Original error:** push rejected");
+    expect(fallbackIssueBody).toContain("Test body");
+    expect(fallbackIssueBody).toContain("Closes \\#57");
+    expect(fallbackIssueBody).toContain("Resolves test-owner/test-repo\\#58");
+    expect(fallbackIssueBody).not.toContain("Closes #57");
+    expect(fallbackIssueBody).not.toContain("Resolves test-owner/test-repo#58");
+  });
+
+  it("should include original error in fallback issue patch instructions when push fails (no bundle)", async () => {
+    const patchPath = canonicalPatchPath("autoloop/perf-comparison");
+    fs.writeFileSync(
+      patchPath,
+      `From abc123 Mon Sep 17 00:00:00 2001
+From: Test Author <test@example.com>
+Date: Mon, 1 Jan 2024 00:00:00 +0000
+Subject: [PATCH] Test commit
+
+diff --git a/test.txt b/test.txt
+new file mode 100644
+index 0000000..abc1234
+--- /dev/null
++++ b/test.txt
+@@ -0,0 +1 @@
++Hello World
+--
+2.34.1
+`
+    );
+    // No bundle file - forces the patch transport fallback path
+    pushSignedSpy.mockRejectedValueOnce(new Error("push rejected"));
+
+    const { main } = require("./create_pull_request.cjs");
+    const handler = await main({ base_branch: "main", preserve_branch_name: true });
+    const result = await handler({ title: "Test PR", body: "Test body\n\nCloses #57\nResolves test-owner/test-repo#58", branch: "autoloop/perf-comparison" }, {});
+
+    expect(result.success).toBe(true);
+    expect(result.fallback_used).toBe(true);
+
+    const fallbackIssueBody = global.github.rest.issues.create.mock.calls[0][0].body;
+    expect(fallbackIssueBody).toContain("**Original error:** push rejected");
+    expect(fallbackIssueBody).toContain("git am --3way");
+    expect(fallbackIssueBody).not.toContain("git update-ref");
     expect(fallbackIssueBody).toContain("Test body");
     expect(fallbackIssueBody).toContain("Closes \\#57");
     expect(fallbackIssueBody).toContain("Resolves test-owner/test-repo\\#58");

actions/setup/js/create_pull_request_bundle_integration.test.cjs

@@ -262,4 +262,177 @@ describe("create_pull_request bundle integration", () => {
     expect(fs.readFileSync(path.join(targetRepo, "feature.txt"), "utf8")).toBe("feature branch commit\n");
     expect(fs.readFileSync(path.join(targetRepo, "main.txt"), "utf8")).toBe("main branch commit\n");
   });
+
+  it("captures a push rejection error after applying a reconcile-spark diverged-history bundle", async () => {
+    // ─── Why this test exists ────────────────────────────────────────────────
+    //
+    // The "reconcile-spark" chaos scenario exposed a gap in the
+    // create_pull_request bundle path:
+    //
+    //   1. An agent works on a feature branch and makes commits.
+    //   2. The main branch receives new commits while the agent is working
+    //      (history diverges).
+    //   3. The agent reconciles by merging main into their branch, producing
+    //      a non-linear (merge-commit) history — this is the "reconcile-spark"
+    //      topology.
+    //   4. A git bundle is created from that non-linear history.
+    //   5. The bundle is applied to the safe-outputs runner via applyBundleToBranch.
+    //   6. The subsequent push to origin fails because the remote branch has
+    //      also diverged (or a policy hook rejects the push).
+    //
+    // Previously, pushSignedCommits attempted a linear cherry-pick replay of
+    // the commit range onto the current GraphQL parent. That path choked on
+    // merge commits and produced a CONFLICT error, dropping the flow into the
+    // fallback-issue path with no useful context.
+    //
+    // The fix adds a sanitized `pushFailureMessage` to the fallback issue body
+    // so that manual recovery is deterministic. This integration test verifies:
+    //
+    //   • applyBundleToBranch correctly imports a reconcile-spark merge topology
+    //     (merge commits are preserved, not flattened).
+    //   • A real git push to a diverged bare remote fails with an error — the
+    //     kind of raw error string that create_pull_request captures and sanitizes
+    //     before embedding in the fallback issue body.
+    //   • The raw error produced by git contains content that sanitization must
+    //     handle (the test injects an @-mention into the hook rejection message
+    //     to document the attack surface; sanitizeContent strips it in prod).
+    //
+    // ─────────────────────────────────────────────────────────────────────────
+
+    const branchName = "scratchpad/chaos/reconcile-spark";
+
+    // 1. Set up a bare "origin" repo and a working clone — this mimics the
+    //    relationship between GitHub and the safe-outputs runner checkout.
+    const bareRemote = fs.mkdtempSync(path.join(os.tmpdir(), "create-pr-reconcile-spark-bare-"));
+    const agentRepo = fs.mkdtempSync(path.join(os.tmpdir(), "create-pr-reconcile-spark-agent-"));
+    const safeOutputsRepo = fs.mkdtempSync(path.join(os.tmpdir(), "create-pr-reconcile-spark-so-"));
+    tempDirs.push(bareRemote, agentRepo, safeOutputsRepo);
+
+    // Initialize the bare remote and push a first commit onto main.
+    // Use -b main so we never need to run git symbolic-ref inside the bare repo
+    // (git 2.36+ restricts bare-repo commands unless safe.bareRepository=all).
+    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"), "# Chaos scenario\n");
+    execGit(["add", "README.md"], { cwd: agentRepo });
+    execGit(["commit", "-m", "Initial commit on main"], { cwd: agentRepo });
+    execGit(["branch", "-M", "main"], { cwd: agentRepo });
+    execGit(["push", "-u", "origin", "main"], { cwd: agentRepo });
+    core.info("[reconcile-spark] bare remote initialized with main");
+
+    // 2. Agent creates the feature branch and makes a first content commit.
+    execGit(["checkout", "-b", branchName], { cwd: agentRepo });
+    fs.writeFileSync(path.join(agentRepo, "notes.md"), "# Agent notes\n");
+    execGit(["add", "notes.md"], { cwd: agentRepo });
+    execGit(["commit", "-m", "feat: add initial notes"], { cwd: agentRepo });
+    core.info("[reconcile-spark] agent made first commit on feature branch");
+
+    // 3. While the agent is working, a collaborator pushes a commit directly
+    //    to main on the remote. The agent's local main diverges from origin/main.
+    //    We simulate this by making a commit on main in the agent repo and then
+    //    force-pushing it so the bare remote has a commit the agent branch doesn't.
+    execGit(["checkout", "main"], { cwd: agentRepo });
+    fs.writeFileSync(path.join(agentRepo, "collab.md"), "# Collaborator change\n");
+    execGit(["add", "collab.md"], { cwd: agentRepo });
+    execGit(["commit", "-m", "collab: landing from main"], { cwd: agentRepo });
+    execGit(["push", "origin", "main"], { cwd: agentRepo });
+    core.info("[reconcile-spark] collaborator commit pushed to origin/main — histories now diverged");
+
+    // 4. Agent reconciles: merges the updated main back into the feature branch.
+    //    This creates the "reconcile-spark" non-linear merge commit.
+    execGit(["checkout", branchName], { cwd: agentRepo });
+    execGit(["merge", "--no-ff", "main", "-m", "reconcile: merge main into feature"], { cwd: agentRepo });
+    core.info("[reconcile-spark] merge commit created — non-linear history established");
+
+    // Add one more commit after the reconcile merge to ensure the bundle tip is
+    // beyond the merge (the pathological shape that the original linear-replay
+    // path could not handle: a non-empty range starting with a merge parent).
+    fs.writeFileSync(path.join(agentRepo, "notes.md"), "# Agent notes\n\nPost-reconcile edit\n");
+    execGit(["commit", "-am", "feat: post-reconcile update"], { cwd: agentRepo });
+    const expectedBundleTip = execGit(["rev-parse", "HEAD"], { cwd: agentRepo }).stdout.trim();
+    const mergeCommitCount = Number(execGit(["rev-list", "--count", "--merges", `main..${branchName}`], { cwd: agentRepo }).stdout.trim());
+    core.info(`[reconcile-spark] feature branch tip: ${expectedBundleTip.slice(0, 8)}, merge commits in range: ${mergeCommitCount}`);
+    // Confirm the branch contains at least one merge commit — the test topology
+    // is only valid when the reconcile merge is present.
+    expect(mergeCommitCount).toBeGreaterThanOrEqual(1);
+
+    // 5. Create a git bundle from the reconcile-spark branch.  The bundle
+    //    includes the full history so that the safe-outputs runner can apply it
+    //    without access to origin.
+    const bundlePath = path.join(agentRepo, "reconcile-spark.bundle");
+    execGit(["bundle", "create", bundlePath, `refs/heads/${branchName}`], { cwd: agentRepo });
+    core.info(`[reconcile-spark] bundle created: ${bundlePath}`);
+
+    // 6. Set up the safe-outputs runner checkout — a fresh clone of origin/main.
+    //    This is the state the runner is in before it applies the agent's bundle.
+    execGit(["clone", bareRemote, "."], { cwd: safeOutputsRepo });
+    execGit(["config", "user.name", "Runner"], { cwd: safeOutputsRepo });
+    execGit(["config", "user.email", "runner@example.com"], { cwd: safeOutputsRepo });
+    execGit(["checkout", "-b", branchName], { cwd: safeOutputsRepo });
+    core.info("[reconcile-spark] safe-outputs runner checkout ready");
+
+    // 7. Apply the bundle via applyBundleToBranch — the function under test.
+    const { applyBundleToBranch } = require("./create_pull_request.cjs");
+    await applyBundleToBranch(bundlePath, branchName, `refs/heads/${branchName}`, createExecApi(safeOutputsRepo));
+
+    // Verify that the merge-commit topology survived the bundle round-trip.
+    const appliedTip = execGit(["rev-parse", "HEAD"], { cwd: safeOutputsRepo }).stdout.trim();
+    const appliedMergeCount = Number(execGit(["rev-list", "--count", "--merges", "HEAD"], { cwd: safeOutputsRepo }).stdout.trim());
+    core.info(`[reconcile-spark] bundle applied; tip: ${appliedTip.slice(0, 8)}, merges: ${appliedMergeCount}`);
+    expect(appliedTip).toBe(expectedBundleTip);
+    expect(appliedMergeCount).toBeGreaterThanOrEqual(1);
+    expect(fs.readFileSync(path.join(safeOutputsRepo, "notes.md"), "utf8")).toContain("Post-reconcile edit");
+    expect(fs.readFileSync(path.join(safeOutputsRepo, "collab.md"), "utf8")).toBe("# Collaborator change\n");
+
+    // 8. Simulate a push rejection.
+    //
+    //    In the reconcile-spark scenario the push fails because:
+    //    (a) The remote branch may not accept non-fast-forward pushes, or
+    //    (b) A policy hook (e.g. "require signed commits") rejects the push.
+    //
+    //    We reproduce (b) by installing a pre-receive hook that emits a message
+    //    containing an @-mention — deliberately chosen to document the attack
+    //    surface that sanitizeContent must neutralise before the error is
+    //    embedded in the fallback issue body.
+    const hooksDir = path.join(bareRemote, "hooks");
+    fs.mkdirSync(hooksDir, { recursive: true });
+    const hookPath = path.join(hooksDir, "pre-receive");
+    // The @org/team mention in the hook message is intentional: it demonstrates
+    // the class of content (@ mentions, URLs, closing keywords) that can appear
+    // in raw git push errors and must be stripped by sanitizeContent before the
+    // message is interpolated into the fallback issue markdown body.
+    fs.writeFileSync(
+      hookPath,
+      [
+        "#!/bin/sh",
+        "echo 'remote: error: pushSignedCommits: failed to rebase commit range onto current GraphQL parent (merge commit detected)' >&2",
+        "echo 'remote: - CONFLICT (content): Merge conflict in scratchpad/chaos/reconcile-spark.md' >&2",
+        "echo 'remote: - See @org/team for recovery steps.' >&2",
+        "exit 1",
+      ].join("\n") + "\n"
+    );
+    fs.chmodSync(hookPath, "0755");
+
+    // Attempt the real git push — this MUST fail so we can capture the error.
+    const pushResult = execGit(["push", "origin", branchName], { cwd: safeOutputsRepo, allowFailure: true });
+    core.info(`[reconcile-spark] push exit code: ${pushResult.status}`);
+    core.info(`[reconcile-spark] push stderr: ${pushResult.stderr.trim()}`);
+    expect(pushResult.status).not.toBe(0);
+
+    // 9. Verify the raw push error contains the content that create_pull_request
+    //    must sanitize and embed in the fallback issue body.
+    //    This is the value that will be passed through:
+    //      sanitizeContent(neutralizeClosingKeywordsForIssueBody(pushError.message), ...)
+    //    before being written into the fallback issue markdown.
+    const rawPushError = pushResult.stderr.trim();
+    expect(rawPushError).toContain("merge commit detected");
+    expect(rawPushError).toContain("CONFLICT");
+    // The @-mention in the hook output confirms that unsanitized error text can
+    // contain @ tokens — sanitizeContent replaces them with safe equivalents.
+    expect(rawPushError).toContain("@org/team");
+    core.info("[reconcile-spark] push error captured — ready for sanitization and fallback issue embedding");
+  });
 });

pkg/workflow/header.go

@@ -70,7 +70,11 @@ func GenerateWorkflowHeader(sourceFile string, generatedBy string, customInstruc
 		instructionLines := strings.Split(strings.TrimSpace(customInstructions), "\n")
 		headerLog.Printf("Adding %d lines of custom instructions", len(instructionLines))
 		for _, line := range instructionLines {
-			fmt.Fprintf(&header, "# %s\n", strings.TrimSpace(line))
+			if trimmedLine := strings.TrimSpace(line); trimmedLine == "" {
+				header.WriteString("#\n")
+			} else {
+				fmt.Fprintf(&header, "# %s\n", trimmedLine)
+			}
 		}
 	}