From 42210133b811390a0e25a717ad6d8be898d66a0d Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 8 Jun 2026 17:34:53 +0100 Subject: [PATCH 01/12] fix(safe-outputs): always derive push_to_pull_request_branch from PR head ref (#37835) The push_to_pull_request_branch MCP tool previously accepted a 'branch' argument from the agent, and apply_samples invented synthetic branch names like 'gh-aw-sample-N' that did not exist on the remote, causing 'fatal: couldn't find remote ref' failures during sample replay. Apply two coupled fixes: 1. Remove 'branch' from the MCP tool schema and payload: - pkg/workflow/js/safe_outputs_tools.json and the actions/setup mirror no longer expose 'branch' for push_to_pull_request_branch. - pkg/workflow/safe_outputs_validation_config.go drops the field. - safe_outputs_handlers.cjs strips any 'branch' the agent sends and always uses the current working-tree branch (via getCurrentBranch), erroring clearly if the tree is on the base branch. 2. Derive the PR head ref from the triggering pull request in apply_samples: - Read GITHUB_EVENT_PATH first (pull_request/pull_request_target/review), fall back to issue.pull_request via the REST API for issue_comment, and finally honour an explicit sample.pull_request_number. - Wire GITHUB_TOKEN into the apply_samples step in samples_replay.go so the REST fallback can authenticate. - For push_to_pull_request_branch samples, fail fast with an actionable message instead of inventing a synthetic branch name. Update docs, tests, and regenerate affected lock files. --- .github/workflows/changeset.lock.yml | 6 - .github/workflows/craft.lock.yml | 6 - .../workflows/design-decision-gate.lock.yml | 6 - .github/workflows/mergefest.lock.yml | 6 - .github/workflows/necromancer.lock.yml | 6 - .github/workflows/poem-bot.lock.yml | 6 - .github/workflows/pr-sous-chef.lock.yml | 6 - .github/workflows/smoke-claude.lock.yml | 6 - .../smoke-update-cross-repo-pr.lock.yml | 6 - .github/workflows/tidy.lock.yml | 6 - actions/setup/js/apply_samples.cjs | 138 +++++++++++++++++- actions/setup/js/apply_samples.test.cjs | 110 +++++++++++--- .../setup/js/handle_agent_failure.test.cjs | 6 +- actions/setup/js/safe_outputs_handlers.cjs | 53 ++++--- .../setup/js/safe_outputs_handlers.test.cjs | 53 ++++--- .../safe_outputs_mcp_server_defaults.test.cjs | 9 +- actions/setup/js/safe_outputs_tools.json | 6 +- .../reference/safe-outputs-pull-requests.md | 14 ++ pkg/workflow/js/safe_outputs_tools.json | 6 +- .../safe_outputs_validation_config.go | 1 - pkg/workflow/samples_replay.go | 4 + pkg/workflow/samples_replay_test.go | 5 + 22 files changed, 328 insertions(+), 137 deletions(-) diff --git a/.github/workflows/changeset.lock.yml b/.github/workflows/changeset.lock.yml index 790e2bcf0fb..e6536093f07 100644 --- a/.github/workflows/changeset.lock.yml +++ b/.github/workflows/changeset.lock.yml @@ -637,12 +637,6 @@ jobs: "push_to_pull_request_branch": { "defaultMax": 1, "fields": { - "branch": { - "required": true, - "type": "string", - "sanitize": true, - "maxLength": 256 - }, "message": { "required": true, "type": "string", diff --git a/.github/workflows/craft.lock.yml b/.github/workflows/craft.lock.yml index bd29370ba7a..a611b669ea7 100644 --- a/.github/workflows/craft.lock.yml +++ b/.github/workflows/craft.lock.yml @@ -667,12 +667,6 @@ jobs: "push_to_pull_request_branch": { "defaultMax": 1, "fields": { - "branch": { - "required": true, - "type": "string", - "sanitize": true, - "maxLength": 256 - }, "message": { "required": true, "type": "string", diff --git a/.github/workflows/design-decision-gate.lock.yml b/.github/workflows/design-decision-gate.lock.yml index 6f192d7cbd3..4bb4baca6b3 100644 --- a/.github/workflows/design-decision-gate.lock.yml +++ b/.github/workflows/design-decision-gate.lock.yml @@ -702,12 +702,6 @@ jobs: "push_to_pull_request_branch": { "defaultMax": 1, "fields": { - "branch": { - "required": true, - "type": "string", - "sanitize": true, - "maxLength": 256 - }, "message": { "required": true, "type": "string", diff --git a/.github/workflows/mergefest.lock.yml b/.github/workflows/mergefest.lock.yml index e3c5271c7ea..b0d8f7ee32f 100644 --- a/.github/workflows/mergefest.lock.yml +++ b/.github/workflows/mergefest.lock.yml @@ -644,12 +644,6 @@ jobs: "push_to_pull_request_branch": { "defaultMax": 1, "fields": { - "branch": { - "required": true, - "type": "string", - "sanitize": true, - "maxLength": 256 - }, "message": { "required": true, "type": "string", diff --git a/.github/workflows/necromancer.lock.yml b/.github/workflows/necromancer.lock.yml index dba8d949879..e80277043c6 100644 --- a/.github/workflows/necromancer.lock.yml +++ b/.github/workflows/necromancer.lock.yml @@ -676,12 +676,6 @@ jobs: "push_to_pull_request_branch": { "defaultMax": 1, "fields": { - "branch": { - "required": true, - "type": "string", - "sanitize": true, - "maxLength": 256 - }, "message": { "required": true, "type": "string", diff --git a/.github/workflows/poem-bot.lock.yml b/.github/workflows/poem-bot.lock.yml index 7443abd4503..95eb4e417bd 100644 --- a/.github/workflows/poem-bot.lock.yml +++ b/.github/workflows/poem-bot.lock.yml @@ -922,12 +922,6 @@ jobs: "push_to_pull_request_branch": { "defaultMax": 1, "fields": { - "branch": { - "required": true, - "type": "string", - "sanitize": true, - "maxLength": 256 - }, "message": { "required": true, "type": "string", diff --git a/.github/workflows/pr-sous-chef.lock.yml b/.github/workflows/pr-sous-chef.lock.yml index c33b91c031d..e8063465e95 100644 --- a/.github/workflows/pr-sous-chef.lock.yml +++ b/.github/workflows/pr-sous-chef.lock.yml @@ -663,12 +663,6 @@ jobs: "push_to_pull_request_branch": { "defaultMax": 1, "fields": { - "branch": { - "required": true, - "type": "string", - "sanitize": true, - "maxLength": 256 - }, "message": { "required": true, "type": "string", diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index 17f29851e8b..0a93e267265 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -1158,12 +1158,6 @@ jobs: "push_to_pull_request_branch": { "defaultMax": 1, "fields": { - "branch": { - "required": true, - "type": "string", - "sanitize": true, - "maxLength": 256 - }, "message": { "required": true, "type": "string", diff --git a/.github/workflows/smoke-update-cross-repo-pr.lock.yml b/.github/workflows/smoke-update-cross-repo-pr.lock.yml index ad2c38da80c..3f198d16d03 100644 --- a/.github/workflows/smoke-update-cross-repo-pr.lock.yml +++ b/.github/workflows/smoke-update-cross-repo-pr.lock.yml @@ -780,12 +780,6 @@ jobs: "push_to_pull_request_branch": { "defaultMax": 1, "fields": { - "branch": { - "required": true, - "type": "string", - "sanitize": true, - "maxLength": 256 - }, "message": { "required": true, "type": "string", diff --git a/.github/workflows/tidy.lock.yml b/.github/workflows/tidy.lock.yml index 141c9efab61..5dc04c408d9 100644 --- a/.github/workflows/tidy.lock.yml +++ b/.github/workflows/tidy.lock.yml @@ -703,12 +703,6 @@ jobs: "push_to_pull_request_branch": { "defaultMax": 1, "fields": { - "branch": { - "required": true, - "type": "string", - "sanitize": true, - "maxLength": 256 - }, "message": { "required": true, "type": "string", diff --git a/actions/setup/js/apply_samples.cjs b/actions/setup/js/apply_samples.cjs index 6c214dd30e9..dba954dc472 100644 --- a/actions/setup/js/apply_samples.cjs +++ b/actions/setup/js/apply_samples.cjs @@ -113,20 +113,143 @@ function ensureGitIdentity(cwd) { } } +/** + * Read and parse the GitHub Actions event payload from GITHUB_EVENT_PATH. + * Returns null when the file is unset, missing, or not parseable. + * @returns {Record|null} + */ +function readEventPayload() { + const p = process.env.GITHUB_EVENT_PATH; + if (!p) return null; + try { + return JSON.parse(fs.readFileSync(p, "utf8")); + } catch { + return null; + } +} + +/** + * Fetch a pull request via the REST API and return its head ref. Uses the + * runner's GITHUB_TOKEN when present; falls back to anonymous (works for + * public repositories). Returns null on any failure so the caller can decide + * how to recover. + * @param {{owner: string, repo: string, pullNumber: number}} args + * @returns {Promise} + */ +async function fetchPullRequestHeadRef({ owner, repo, pullNumber }) { + const apiUrl = process.env.GITHUB_API_URL || "https://api.github.com"; + const url = `${apiUrl}/repos/${owner}/${repo}/pulls/${pullNumber}`; + /** @type {Record} */ + const headers = { + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "gh-aw-apply-samples", + }; + const token = process.env.GITHUB_TOKEN || process.env.GH_TOKEN; + if (token) headers["Authorization"] = `Bearer ${token}`; + try { + const resp = await fetch(url, { headers }); + if (!resp.ok) { + core.warning(`apply_samples: GET ${url} returned HTTP ${resp.status}`); + return null; + } + const body = await resp.json(); + const ref = body && body.head && typeof body.head.ref === "string" ? body.head.ref : null; + return ref || null; + } catch (err) { + core.warning(`apply_samples: failed to fetch PR ${owner}/${repo}#${pullNumber}: ${getErrorMessage(err)}`); + return null; + } +} + +/** + * Derive the pull request head ref for the triggering event. + * + * Resolution order: + * 1. `pull_request.head.ref` from the event payload (pull_request and + * pull_request_target events). + * 2. PR API lookup using `issue.number` when the payload describes an + * issue_comment on a pull request. + * 3. PR API lookup using an explicit `pull_request_number` argument on the + * sample (covers workflow_dispatch driven by the agent). + * + * @param {SampleEntry} entry + * @returns {Promise} + */ +async function derivePrHeadRef(entry) { + const payload = readEventPayload(); + + // 1. pull_request* events expose the head ref directly. + const directRef = payload?.pull_request?.head?.ref; + if (typeof directRef === "string" && directRef.trim()) { + return directRef.trim(); + } + + // Determine the target repo for any API lookups. Prefer the entry's repo if + // the sample sets one (cross-repo workflows), otherwise fall back to + // GITHUB_REPOSITORY. + const repoSlug = (typeof entry.arguments.repo === "string" && entry.arguments.repo.trim()) || process.env.GITHUB_REPOSITORY || ""; + const [owner, repo] = repoSlug.split("/"); + if (!owner || !repo) return null; + + // 2. issue_comment / pull_request_review_comment with a PR-linked issue. + if (payload?.issue?.pull_request && typeof payload.issue.number === "number") { + const ref = await fetchPullRequestHeadRef({ owner, repo, pullNumber: payload.issue.number }); + if (ref) return ref; + } + + // 3. Explicit pull_request_number on the sample arguments. + const argNumber = Number(entry.arguments.pull_request_number); + if (Number.isFinite(argNumber) && argNumber > 0) { + const ref = await fetchPullRequestHeadRef({ owner, repo, pullNumber: argNumber }); + if (ref) return ref; + } + + return null; +} + /** * Pre-stage a branch + patch for samples whose tool reads the workspace diff. - * Mutates `entry.arguments.branch` to the actual checked-out branch. + * + * - For `create_pull_request`, the sample creates a brand-new branch, so we + * accept the sample's declared `branch` or synthesize `gh-aw-sample-`. + * - For `push_to_pull_request_branch`, the destination is the triggering PR's + * head branch — we derive it from event/PR context and refuse to invent a + * synthetic branch (which would never exist on origin and would break the + * MCP server's incremental-patch generation, per issue #37835). + * * @param {SampleEntry} entry * @param {number} index * @param {string} workspace + * @returns {Promise} */ -function preStagePatch(entry, index, workspace) { +async function preStagePatch(entry, index, workspace) { const patch = entry.sidecars && entry.sidecars.patch; if (typeof patch !== "string" || !patch.trim()) { return; } - const branch = typeof entry.arguments.branch === "string" && entry.arguments.branch.trim() ? entry.arguments.branch.trim() : `gh-aw-sample-${index + 1}`; - entry.arguments.branch = branch; + + let branch; + if (entry.tool === "push_to_pull_request_branch") { + // Source ref MUST match the PR's head ref so that + // `git diff origin/..` in the MCP server resolves the + // correct baseline. Synthetic `gh-aw-sample-N` names produce + // `fatal: couldn't find remote ref` failures (issue #37835). + branch = await derivePrHeadRef(entry); + if (!branch) { + throw new Error( + `apply_samples: cannot derive pull-request head branch for sample[${index}] (tool=${entry.tool}). ` + + `Trigger the workflow from a pull_request event, or set sample.pull_request_number, ` + + `or provide GITHUB_TOKEN so the PR can be fetched.` + ); + } + // The agent input no longer carries `branch`; ensure stray sample-supplied + // values do not leak through to the MCP tools/call payload. + delete entry.arguments.branch; + } else { + branch = typeof entry.arguments.branch === "string" && entry.arguments.branch.trim() ? entry.arguments.branch.trim() : `gh-aw-sample-${index + 1}`; + entry.arguments.branch = branch; + } ensureGitIdentity(workspace); @@ -268,11 +391,12 @@ async function main() { const logPath = process.env.GH_AW_AGENT_STDIO_LOG || ""; // Pre-stage branches/patches. - samples.forEach((sample, i) => { + for (let i = 0; i < samples.length; i++) { + const sample = samples[i]; if (PATCH_SIDECAR_TOOLS.has(sample.tool)) { - preStagePatch(sample, i, workspace); + await preStagePatch(sample, i, workspace); } - }); + } if (samples.length === 0) { core.info("apply_samples: nothing to replay; exiting cleanly."); diff --git a/actions/setup/js/apply_samples.test.cjs b/actions/setup/js/apply_samples.test.cjs index 5c291eea83e..a5ab24bc0ea 100644 --- a/actions/setup/js/apply_samples.test.cjs +++ b/actions/setup/js/apply_samples.test.cjs @@ -205,7 +205,7 @@ describe("apply_samples.cjs preStagePatch (create_pull_request / push_to_pull_re return `diff --git a/${filePath} b/${filePath}\n` + `new file mode 100644\n` + `index 0000000..1111111\n` + `--- /dev/null\n` + `+++ b/${filePath}\n` + `@@ -0,0 +1,${lines.length} @@\n` + body + "\n"; } - it("checks out the requested branch and commits the patch on it (create_pull_request)", () => { + it("checks out the requested branch and commits the patch on it (create_pull_request)", async () => { const workspace = makeTempDir("gh-aw-prestage-cpr-"); initRepo(workspace, "main"); @@ -227,7 +227,7 @@ describe("apply_samples.cjs preStagePatch (create_pull_request / push_to_pull_re const prev = process.env.GH_AW_CUSTOM_BASE_BRANCH; process.env.GH_AW_CUSTOM_BASE_BRANCH = "main"; try { - preStagePatch(entry, 0, workspace); + await preStagePatch(entry, 0, workspace); } finally { if (prev === undefined) delete process.env.GH_AW_CUSTOM_BASE_BRANCH; else process.env.GH_AW_CUSTOM_BASE_BRANCH = prev; @@ -260,36 +260,110 @@ describe("apply_samples.cjs preStagePatch (create_pull_request / push_to_pull_re expect(diff).toContain("+hello from a deterministic sample"); }); - it("defaults the branch name to gh-aw-sample- when none is supplied", () => { - const workspace = makeTempDir("gh-aw-prestage-default-"); + it("derives push_to_pull_request_branch branch from pull_request event payload", async () => { + const workspace = makeTempDir("gh-aw-prestage-push-pr-"); initRepo(workspace, "main"); + const headRef = "feat/copilot-push-branch"; + const eventPath = path.join(workspace, "event.json"); + fs.writeFileSync( + eventPath, + JSON.stringify({ + pull_request: { number: 654, head: { ref: headRef } }, + }) + ); + const entry = { tool: "push_to_pull_request_branch", - arguments: { - body: "Sample push body", - // branch intentionally omitted — driver should synthesize one. - }, + // No `branch` in arguments — the agent never supplies one now. + arguments: { message: "Push update" }, sidecars: { patch: newFileDiff("push-feature.txt", "from push sample\n") }, }; - const prev = process.env.GH_AW_CUSTOM_BASE_BRANCH; + const prevBase = process.env.GH_AW_CUSTOM_BASE_BRANCH; + const prevEvent = process.env.GITHUB_EVENT_PATH; process.env.GH_AW_CUSTOM_BASE_BRANCH = "main"; + process.env.GITHUB_EVENT_PATH = eventPath; try { - preStagePatch(entry, 2, workspace); + await preStagePatch(entry, 0, workspace); } finally { - if (prev === undefined) delete process.env.GH_AW_CUSTOM_BASE_BRANCH; - else process.env.GH_AW_CUSTOM_BASE_BRANCH = prev; + if (prevBase === undefined) delete process.env.GH_AW_CUSTOM_BASE_BRANCH; + else process.env.GH_AW_CUSTOM_BASE_BRANCH = prevBase; + if (prevEvent === undefined) delete process.env.GITHUB_EVENT_PATH; + else process.env.GITHUB_EVENT_PATH = prevEvent; } - // Index in preStagePatch is zero-based; the default uses i+1 → "gh-aw-sample-3". - expect(entry.arguments.branch).toBe("gh-aw-sample-3"); - const head = git(["rev-parse", "--abbrev-ref", "HEAD"], workspace).trim(); - expect(head).toBe("gh-aw-sample-3"); + // The staged branch is the PR's head ref (not a synthetic "gh-aw-sample-N"). + expect(git(["rev-parse", "--abbrev-ref", "HEAD"], workspace).trim()).toBe(headRef); expect(fs.existsSync(path.join(workspace, "push-feature.txt"))).toBe(true); + // The agent input never carries `branch` for this tool. + expect(entry.arguments.branch).toBeUndefined(); + }); + + it("derives push_to_pull_request_branch branch via PR API for issue_comment events", async () => { + const workspace = makeTempDir("gh-aw-prestage-push-issue-"); + initRepo(workspace, "main"); + + const headRef = "feat/issue-comment-branch"; + const eventPath = path.join(workspace, "event.json"); + fs.writeFileSync( + eventPath, + JSON.stringify({ + issue: { number: 42, pull_request: { url: "https://api.github.com/repos/owner/repo/pulls/42" } }, + }) + ); + + // Stub global fetch to simulate the GitHub REST API response. + const realFetch = global.fetch; + global.fetch = async url => { + expect(String(url)).toContain("/repos/owner/repo/pulls/42"); + return { ok: true, status: 200, json: async () => ({ head: { ref: headRef } }) }; + }; + + const prevBase = process.env.GH_AW_CUSTOM_BASE_BRANCH; + const prevEvent = process.env.GITHUB_EVENT_PATH; + const prevRepo = process.env.GITHUB_REPOSITORY; + process.env.GH_AW_CUSTOM_BASE_BRANCH = "main"; + process.env.GITHUB_EVENT_PATH = eventPath; + process.env.GITHUB_REPOSITORY = "owner/repo"; + try { + const entry = { + tool: "push_to_pull_request_branch", + arguments: { message: "Push update" }, + sidecars: { patch: newFileDiff("issue-comment.txt", "from issue comment\n") }, + }; + await preStagePatch(entry, 0, workspace); + expect(git(["rev-parse", "--abbrev-ref", "HEAD"], workspace).trim()).toBe(headRef); + } finally { + global.fetch = realFetch; + if (prevBase === undefined) delete process.env.GH_AW_CUSTOM_BASE_BRANCH; + else process.env.GH_AW_CUSTOM_BASE_BRANCH = prevBase; + if (prevEvent === undefined) delete process.env.GITHUB_EVENT_PATH; + else process.env.GITHUB_EVENT_PATH = prevEvent; + if (prevRepo === undefined) delete process.env.GITHUB_REPOSITORY; + else process.env.GITHUB_REPOSITORY = prevRepo; + } + }); + + it("fails fast for push_to_pull_request_branch when no PR context is available", async () => { + const workspace = makeTempDir("gh-aw-prestage-push-nopr-"); + initRepo(workspace, "main"); + + const prevEvent = process.env.GITHUB_EVENT_PATH; + delete process.env.GITHUB_EVENT_PATH; + try { + const entry = { + tool: "push_to_pull_request_branch", + arguments: { message: "Push update" }, + sidecars: { patch: newFileDiff("orphan.txt", "no PR context\n") }, + }; + await expect(preStagePatch(entry, 0, workspace)).rejects.toThrow(/cannot derive pull-request head branch/); + } finally { + if (prevEvent !== undefined) process.env.GITHUB_EVENT_PATH = prevEvent; + } }); - it("is a no-op when the sample tool isn't in the patch-sidecar set", () => { + it("is a no-op when the sample tool isn't in the patch-sidecar set", async () => { // We assert this at the driver level (PATCH_SIDECAR_TOOLS gate in main()), // but preStagePatch itself should also be a no-op when called with an // entry that has no patch sidecar — protecting against misuse. @@ -300,7 +374,7 @@ describe("apply_samples.cjs preStagePatch (create_pull_request / push_to_pull_re tool: "create_issue", arguments: { title: "x", body: "y" }, }; - preStagePatch(entry, 0, workspace); + await preStagePatch(entry, 0, workspace); // Still on main, no extra commits, no new files. expect(git(["rev-parse", "--abbrev-ref", "HEAD"], workspace).trim()).toBe("main"); diff --git a/actions/setup/js/handle_agent_failure.test.cjs b/actions/setup/js/handle_agent_failure.test.cjs index 7f4a4533be9..017844c5f79 100644 --- a/actions/setup/js/handle_agent_failure.test.cjs +++ b/actions/setup/js/handle_agent_failure.test.cjs @@ -2392,7 +2392,7 @@ describe("handle_agent_failure", () => { fs.mkdirSync(promptsDir, { recursive: true }); fs.copyFileSync(path.join(__dirname, "../md/tool_denials_exceeded_context.md"), path.join(promptsDir, "tool_denials_exceeded_context.md")); - const python3Reason = "permission denied: shell(python3 << 'EOF'\nimport re\n\nfiles = [(\"foo.go\", \"/path/foo.go\")]\nfor f, p in files:\n print(f)\nEOF)"; + const python3Reason = 'permission denied: shell(python3 << \'EOF\'\nimport re\n\nfiles = [("foo.go", "/path/foo.go")]\nfor f, p in files:\n print(f)\nEOF)'; const result = buildToolDenialsExceededContext([{ denialCount: 5, threshold: 5, reason: python3Reason }], "daily-compiler-quality"); expect(result).toContain("shell(python3 ...)"); // The full multi-line program body should not appear in the output @@ -2434,8 +2434,8 @@ describe("handle_agent_failure", () => { expect(normalizeDeniedPermissionCommand(cmd)).toBe("shell(python3 ...)"); }); - it("collapses shell(python3 << \"EOF\" ...) heredoc with double-quoted marker", () => { - const cmd = "shell(python3 << \"EOF\"\nimport sys\nsys.exit(0)\nEOF)"; + it('collapses shell(python3 << "EOF" ...) heredoc with double-quoted marker', () => { + const cmd = 'shell(python3 << "EOF"\nimport sys\nsys.exit(0)\nEOF)'; expect(normalizeDeniedPermissionCommand(cmd)).toBe("shell(python3 ...)"); }); diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index 0f397b8a051..7b0490a2874 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -797,14 +797,20 @@ function createHandlers(server, appendSafeOutput, config = {}) { /** * Handler for push_to_pull_request_branch tool * Spec cross-reference: Safe Output Outcome Evaluation §17 (`push_to_pull_request_branch`). - * Resolves the current branch if branch is not provided or is the base branch - * Generates git patch for the changes + * The agent does NOT supply a branch. The source branch is derived from the + * current working checkout (the agent must already be on the PR head ref to + * have committed onto it). The destination branch is independently derived + * by the apply-time push handler from pulls.get(pull_number).head.ref. * * Note: Fork PR detection is handled by push_to_pull_request_branch.cjs handler * which fetches the PR and calls detectForkPR() with full PR data. */ const pushToPullRequestBranchHandler = async args => { - const entry = { ...args, type: "push_to_pull_request_branch" }; + // Defensive strip: the input schema no longer declares a `branch` property, + // but an older or non-conforming client could still attempt to pass one. + // Drop it so the agent cannot override the derived source branch. + const { branch: _agentBranch, ...sanitizedArgs } = args || {}; + const entry = { ...sanitizedArgs, type: "push_to_pull_request_branch" }; // Resolve target repo configuration and validate the target repo early // This is needed before getBaseBranch to ensure we resolve the base branch @@ -915,23 +921,36 @@ function createHandlers(server, appendSafeOutput, config = {}) { // If branch is not provided, is empty, or equals the base branch, use the current branch from git // This handles cases where the agent incorrectly passes the base branch instead of the working branch - if (!entry.branch || entry.branch.trim() === "" || entry.branch === baseBranch) { + // The agent never supplies a branch. Always derive it from the current + // checkout: the working tree must already be on the PR head ref because + // that's what the agent committed onto. The apply-time push job + // independently re-derives the destination from pulls.get(pull_number), + // so this branch name is used only as the source ref for the incremental + // diff against origin/. + try { const detectedBranch = getCurrentBranch(repoCwd); - - if (entry.branch === baseBranch) { - server.debug(`Branch equals base branch (${baseBranch}), detecting actual working branch: ${detectedBranch}`); - } else { - server.debug(`Using current branch for push_to_pull_request_branch: ${detectedBranch}`); - } - + server.debug(`Using current branch for push_to_pull_request_branch: ${detectedBranch}`); entry.branch = detectedBranch; + } catch (branchErr) { + return { + content: [ + { + type: "text", + text: JSON.stringify({ + result: "error", + error: `Failed to determine source branch for push_to_pull_request_branch: ${getErrorMessage(branchErr)}. The working tree must be on the pull request's head ref before this tool is called.`, + }), + }, + ], + isError: true, + }; } - // Reject if branch still equals base_branch after detection. - // This means the base branch was incorrectly resolved (e.g., resolved to the - // feature branch itself due to a confused event context). Writing a safe output - // in this state would cause a cryptic git exit-1 in the safe_outputs job when - // it tries to fetch a non-existent remote ref. + // Reject if the detected branch equals base_branch. This means the workspace + // is checked out on the PR's base (e.g. main) rather than the PR's head ref, + // so there is nothing to push. Writing a safe output in this state would + // cause a cryptic git exit-1 in the safe_outputs job when it tries to fetch + // a non-existent remote ref. if (entry.branch === entry.base_branch) { return { content: [ @@ -939,7 +958,7 @@ function createHandlers(server, appendSafeOutput, config = {}) { type: "text", text: JSON.stringify({ result: "error", - error: `Branch '${entry.branch}' equals base_branch '${entry.base_branch}'. Cannot push to a pull request branch that targets itself. Ensure 'branch' is your feature branch and that the base branch resolves to the target (e.g., 'main' or 'master').`, + error: `Detected branch '${entry.branch}' equals base_branch '${entry.base_branch}'. The workspace is checked out on the base branch, not the pull request's head branch — there is nothing to push. Check out the PR's head ref and commit your changes there before calling push_to_pull_request_branch.`, }), }, ], diff --git a/actions/setup/js/safe_outputs_handlers.test.cjs b/actions/setup/js/safe_outputs_handlers.test.cjs index 5ded2243e7e..4eef2bdfbbf 100644 --- a/actions/setup/js/safe_outputs_handlers.test.cjs +++ b/actions/setup/js/safe_outputs_handlers.test.cjs @@ -1053,6 +1053,19 @@ describe("safe_outputs_handlers", () => { }); describe("pushToPullRequestBranchHandler", () => { + // The agent never supplies a branch — the handler always derives the source + // branch from getCurrentBranch(). In production this resolves the working + // tree's HEAD; in unit tests the workspace is an empty temp dir with no git + // repo, so seed the env-var fallback path so tests can focus on the + // downstream patch-generation behavior they actually want to exercise. + beforeEach(() => { + process.env.GITHUB_REF_NAME = "feature-branch"; + }); + + afterEach(() => { + delete process.env.GITHUB_REF_NAME; + }); + function createSideRepoWithTrackedAndLocalCommits() { const targetRepoDir = path.join(testWorkspaceDir, "target-repo"); fs.mkdirSync(targetRepoDir, { recursive: true }); @@ -1114,17 +1127,26 @@ describe("safe_outputs_handlers", () => { }); it("should reject obvious exploratory test payloads before recording a PR branch update intent", async () => { - const result = await handlers.pushToPullRequestBranchHandler({ - branch: "docs/pr-17198-test-from-main-1853f10f924372d4", - message: "test", - }); + // The agent can no longer supply `branch`; the handler derives it from + // the current working checkout. Model the failure mode where the + // working tree itself is sitting on an obviously exploratory branch. + const prevRefName = process.env.GITHUB_REF_NAME; + process.env.GITHUB_REF_NAME = "docs/pr-17198-test-from-main-1853f10f924372d4"; + try { + const result = await handlers.pushToPullRequestBranchHandler({ + message: "test", + }); - expect(result.isError).toBe(true); - const responseData = JSON.parse(result.content[0].text); - expect(responseData.result).toBe("error"); - expect(responseData.error).toContain("Refusing to record an exploratory pull request branch update"); - expect(responseData.error).toContain("noop or report_incomplete"); - expect(mockAppendSafeOutput).not.toHaveBeenCalled(); + expect(result.isError).toBe(true); + const responseData = JSON.parse(result.content[0].text); + expect(responseData.result).toBe("error"); + expect(responseData.error).toContain("Refusing to record an exploratory pull request branch update"); + expect(responseData.error).toContain("noop or report_incomplete"); + expect(mockAppendSafeOutput).not.toHaveBeenCalled(); + } finally { + if (prevRefName === undefined) delete process.env.GITHUB_REF_NAME; + else process.env.GITHUB_REF_NAME = prevRefName; + } }); it("should include helpful details in error response", async () => { @@ -1215,7 +1237,7 @@ describe("safe_outputs_handlers", () => { const responseData = JSON.parse(result.content[0].text); expect(responseData.result).toBe("error"); expect(responseData.error).toContain("equals base_branch"); - expect(responseData.error).toContain("Cannot push to a pull request branch that targets itself"); + expect(responseData.error).toContain("checked out on the base branch"); expect(mockAppendSafeOutput).not.toHaveBeenCalled(); } finally { delete process.env.GITHUB_BASE_REF; @@ -1234,14 +1256,12 @@ describe("safe_outputs_handlers", () => { process.env.GITHUB_BASE_REF = "main"; try { - const result = await handlersWithTarget.pushToPullRequestBranchHandler({ - branch: "main", - }); + const result = await handlersWithTarget.pushToPullRequestBranchHandler({}); expect(result.isError).toBeFalsy(); expect(mockServer.debug).toHaveBeenCalledWith(expect.stringContaining("Looking for checkout of target repo: test-owner/test-repo")); expect(mockServer.debug).toHaveBeenCalledWith(expect.stringContaining(`Selected checkout folder for test-owner/test-repo: ${targetRepoDir}`)); - expect(mockServer.debug).toHaveBeenCalledWith(expect.stringContaining("detecting actual working branch: feature/test-change")); + expect(mockServer.debug).toHaveBeenCalledWith(expect.stringContaining("Using current branch for push_to_pull_request_branch: feature/test-change")); expect(mockAppendSafeOutput).toHaveBeenCalledWith( expect.objectContaining({ type: "push_to_pull_request_branch", @@ -1260,13 +1280,12 @@ describe("safe_outputs_handlers", () => { process.env.GITHUB_BASE_REF = "main"; try { const result = await handlers.pushToPullRequestBranchHandler({ - branch: "main", repo: "test-owner/test-repo", }); expect(result.isError).toBeFalsy(); expect(mockServer.debug).toHaveBeenCalledWith(expect.stringContaining(`Selected checkout folder for test-owner/test-repo: ${targetRepoDir}`)); - expect(mockServer.debug).toHaveBeenCalledWith(expect.stringContaining("detecting actual working branch: feature/test-change")); + expect(mockServer.debug).toHaveBeenCalledWith(expect.stringContaining("Using current branch for push_to_pull_request_branch: feature/test-change")); expect(mockAppendSafeOutput).toHaveBeenCalledWith( expect.objectContaining({ type: "push_to_pull_request_branch", diff --git a/actions/setup/js/safe_outputs_mcp_server_defaults.test.cjs b/actions/setup/js/safe_outputs_mcp_server_defaults.test.cjs index e3e61579a94..ecc1c08c6f5 100644 --- a/actions/setup/js/safe_outputs_mcp_server_defaults.test.cjs +++ b/actions/setup/js/safe_outputs_mcp_server_defaults.test.cjs @@ -222,7 +222,7 @@ const canWriteDefault = canWriteToDefaultPath(); }, 500)); }); }), - it("should have optional branch parameter for push_to_pull_request_branch", async () => { + it("should not expose a branch parameter for push_to_pull_request_branch", async () => { const tempConfigPath = path.join("/tmp", `test-config-${Date.now()}-${Math.random().toString(36).substring(7)}.json`); fs.writeFileSync(tempConfigPath, JSON.stringify({ push_to_pull_request_branch: {} })); const serverPath = path.join(__dirname, "safe_outputs_mcp_server.cjs"); @@ -266,9 +266,10 @@ const canWriteDefault = canWriteToDefaultPath(); (expect(pushTool).toBeDefined(), expect(pushTool.inputSchema.required).toEqual(["message"]), expect(pushTool.inputSchema.required).not.toContain("branch"), - expect(pushTool.inputSchema.properties.branch).toBeDefined(), - expect(pushTool.inputSchema.properties.branch.description).toContain("If omitted"), - expect(pushTool.inputSchema.properties.branch.description).toContain("current"), + // The agent must not be able to specify a branch — it's always + // derived from the pull request's head ref. See issue #37835. + expect(pushTool.inputSchema.properties.branch).toBeUndefined(), + expect(pushTool.description).toMatch(/derived from the pull request/i), resolve()); }, 500)); }); diff --git a/actions/setup/js/safe_outputs_tools.json b/actions/setup/js/safe_outputs_tools.json index 6031c4f1824..4a5e1420e01 100644 --- a/actions/setup/js/safe_outputs_tools.json +++ b/actions/setup/js/safe_outputs_tools.json @@ -951,15 +951,11 @@ }, { "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: 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.", + "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. The destination branch is always derived from the pull request's head ref \u2014 you do not specify it. 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.", "inputSchema": { "type": "object", "required": ["message"], "properties": { - "branch": { - "type": "string", - "description": "Branch name to push changes from. If omitted, uses the current working branch. Only specify if you need to push from a different branch." - }, "message": { "type": "string", "description": "Commit message describing the changes. Follow repository commit message conventions (e.g., conventional commits). This field is named message, NOT commit_message.", 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 45a3c64d301..b895cb5ed85 100644 --- a/docs/src/content/docs/reference/safe-outputs-pull-requests.md +++ b/docs/src/content/docs/reference/safe-outputs-pull-requests.md @@ -277,6 +277,20 @@ safe-outputs: When `push-to-pull-request-branch` is configured, git commands (`checkout`, `branch`, `switch`, `add`, `rm`, `commit`, `merge`) are automatically enabled. +### Destination branch + +The agent **does not specify the destination branch**. Both the source and +destination branches are derived from the triggering pull request: + +- The **source branch** is the branch currently checked out in the agent's + workspace. The agent must commit its changes onto the PR's head ref before + calling the tool. +- The **destination branch** is always the triggering pull request's head ref, + resolved by the apply-time push job via `pulls.get(pull_number).head.ref`. + +This eliminates a class of failures where the agent passed a wrong or synthetic +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`. ### Cross-repo usage diff --git a/pkg/workflow/js/safe_outputs_tools.json b/pkg/workflow/js/safe_outputs_tools.json index 6031c4f1824..4a5e1420e01 100644 --- a/pkg/workflow/js/safe_outputs_tools.json +++ b/pkg/workflow/js/safe_outputs_tools.json @@ -951,15 +951,11 @@ }, { "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: 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.", + "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. The destination branch is always derived from the pull request's head ref \u2014 you do not specify it. 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.", "inputSchema": { "type": "object", "required": ["message"], "properties": { - "branch": { - "type": "string", - "description": "Branch name to push changes from. If omitted, uses the current working branch. Only specify if you need to push from a different branch." - }, "message": { "type": "string", "description": "Commit message describing the changes. Follow repository commit message conventions (e.g., conventional commits). This field is named message, NOT commit_message.", diff --git a/pkg/workflow/safe_outputs_validation_config.go b/pkg/workflow/safe_outputs_validation_config.go index 4e41891fcc8..ff9bf9e1a13 100644 --- a/pkg/workflow/safe_outputs_validation_config.go +++ b/pkg/workflow/safe_outputs_validation_config.go @@ -207,7 +207,6 @@ var ValidationConfig = map[string]TypeValidationConfig{ "push_to_pull_request_branch": { DefaultMax: 1, Fields: map[string]FieldValidation{ - "branch": {Required: true, Type: "string", Sanitize: true, MaxLength: 256}, "message": {Required: true, Type: "string", Sanitize: true, MaxLength: MaxBodyLength}, "pull_request_number": {IssueOrPRNumber: true}, }, diff --git a/pkg/workflow/samples_replay.go b/pkg/workflow/samples_replay.go index 28f16afe1c4..b72568a9b13 100644 --- a/pkg/workflow/samples_replay.go +++ b/pkg/workflow/samples_replay.go @@ -105,6 +105,10 @@ func (c *Compiler) generateSamplesReplayStep(yaml *strings.Builder, data *Workfl fmt.Fprintf(yaml, " GH_AW_AGENT_STDIO_LOG: %s\n", logFile) yaml.WriteString(" GH_AW_SAFE_OUTPUTS_CONFIG_PATH: ${{ runner.temp }}/gh-aw/safeoutputs/config.json\n") yaml.WriteString(" GH_AW_SAFE_OUTPUTS: ${{ runner.temp }}/gh-aw/safeoutputs/outputs.jsonl\n") + // GITHUB_TOKEN lets apply_samples.cjs resolve pull-request head refs via + // the REST API for issue_comment / slash_command events, where the head + // ref is not present in the event payload. + yaml.WriteString(" GITHUB_TOKEN: ${{ github.token }}\n") yaml.WriteString(" run: |\n") yaml.WriteString(" set -euo pipefail\n") yaml.WriteString(" mkdir -p \"$(dirname \"$GH_AW_AGENT_STDIO_LOG\")\"\n") diff --git a/pkg/workflow/samples_replay_test.go b/pkg/workflow/samples_replay_test.go index fbaec041129..e4804bba2e6 100644 --- a/pkg/workflow/samples_replay_test.go +++ b/pkg/workflow/samples_replay_test.go @@ -92,6 +92,11 @@ Trivial workflow whose only job is to be compiled with --use-samples. if !strings.Contains(lockContent, "GH_AW_SAMPLES:") { t.Error("Expected GH_AW_SAMPLES env var in lock file") } + // GITHUB_TOKEN is required so apply_samples.cjs can resolve the PR head + // ref via the REST API for issue_comment / slash-command triggers. + if !strings.Contains(lockContent, "GITHUB_TOKEN: ${{ github.token }}") { + t.Error("Expected GITHUB_TOKEN env var in samples replay step") + } if !strings.Contains(lockContent, `"tool":"create_issue"`) { t.Error("Expected JSON-encoded create_issue tool entry in lock file") } From fff68b7dc7828bda6915c41d4e766da5be1aa581 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 8 Jun 2026 19:09:19 +0100 Subject: [PATCH 02/12] CI fixes --- actions/setup/js/apply_samples.cjs | 2 +- cmd/gh-aw/version_test.go | 12 ++++++++++-- pkg/cli/commands_compile_workflow_test.go | 2 +- pkg/cli/compile_command_test.go | 12 +++++++++++- pkg/cli/compile_integration_test.go | 3 +++ 5 files changed, 26 insertions(+), 5 deletions(-) diff --git a/actions/setup/js/apply_samples.cjs b/actions/setup/js/apply_samples.cjs index dba954dc472..6e8acb1b2df 100644 --- a/actions/setup/js/apply_samples.cjs +++ b/actions/setup/js/apply_samples.cjs @@ -153,7 +153,7 @@ async function fetchPullRequestHeadRef({ owner, repo, pullNumber }) { core.warning(`apply_samples: GET ${url} returned HTTP ${resp.status}`); return null; } - const body = await resp.json(); + const body = /** @type {{head?: {ref?: string}}} */ (await resp.json()); const ref = body && body.head && typeof body.head.ref === "string" ? body.head.ref : null; return ref || null; } catch (err) { diff --git a/cmd/gh-aw/version_test.go b/cmd/gh-aw/version_test.go index 5f3f285dc03..8455064b909 100644 --- a/cmd/gh-aw/version_test.go +++ b/cmd/gh-aw/version_test.go @@ -4,6 +4,8 @@ package main import ( "os/exec" + "path/filepath" + "runtime" "strings" "testing" ) @@ -16,10 +18,16 @@ func TestVersionIsSetDuringBuild(t *testing.T) { // Build a test binary with a specific version testVersion := "v0.0.0-test" + binaryName := "gh-aw-test-version" + if runtime.GOOS == "windows" { + binaryName += ".exe" + } + binaryPath := filepath.Join(t.TempDir(), binaryName) + // Build the binary with version set via ldflags cmd := exec.Command("go", "build", "-ldflags", "-X main.version="+testVersion, - "-o", "/tmp/gh-aw-test-version", + "-o", binaryPath, ".") cmd.Dir = "." @@ -29,7 +37,7 @@ func TestVersionIsSetDuringBuild(t *testing.T) { } // Run the test binary to check its version - versionCmd := exec.Command("/tmp/gh-aw-test-version", "version") + versionCmd := exec.Command(binaryPath, "version") versionOutput, err := versionCmd.CombinedOutput() if err != nil { t.Fatalf("Failed to run version command: %v", err) diff --git a/pkg/cli/commands_compile_workflow_test.go b/pkg/cli/commands_compile_workflow_test.go index 0726bd7cbe7..71782c666c5 100644 --- a/pkg/cli/commands_compile_workflow_test.go +++ b/pkg/cli/commands_compile_workflow_test.go @@ -156,7 +156,7 @@ This workflow has invalid frontmatter. verbose: false, engineOverride: "", expectError: true, - errorContains: "no such file", + errorContains: "failed to read file", }, { name: "compilation with invalid engine override", diff --git a/pkg/cli/compile_command_test.go b/pkg/cli/compile_command_test.go index 18e63461324..3b854c818a1 100644 --- a/pkg/cli/compile_command_test.go +++ b/pkg/cli/compile_command_test.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strings" "testing" @@ -18,6 +19,15 @@ import ( "github.com/github/gh-aw/pkg/workflow" ) +// absoluteTestPath returns a path that filepath.IsAbs reports as absolute on +// the current OS. +func absoluteTestPath() string { + if runtime.GOOS == "windows" { + return `C:\absolute\path` + } + return "/absolute/path" +} + // TestCompileConfig tests the CompileConfig structure func TestCompileConfig(t *testing.T) { config := CompileConfig{ @@ -319,7 +329,7 @@ func TestCompileWorkflows_WorkflowDirValidation(t *testing.T) { }{ { name: "absolute path not allowed", - workflowDir: "/absolute/path", + workflowDir: absoluteTestPath(), expectError: true, errorMsg: "must be a relative path", }, diff --git a/pkg/cli/compile_integration_test.go b/pkg/cli/compile_integration_test.go index cf2b1f5b76f..5138b086743 100644 --- a/pkg/cli/compile_integration_test.go +++ b/pkg/cli/compile_integration_test.go @@ -230,6 +230,9 @@ Please check the repository for any open issues and create a summary. } func TestCompileWithIncludeWithEmptyFrontmatterUnderPty(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("PTY-based test is not supported on Windows") + } setup := setupIntegrationTest(t) defer setup.cleanup() From 034582ee3c28b16a12740bcd2edfd4fada8529b5 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 8 Jun 2026 19:09:54 +0100 Subject: [PATCH 03/12] CI fixes --- actions/setup/js/apply_samples.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actions/setup/js/apply_samples.cjs b/actions/setup/js/apply_samples.cjs index 6e8acb1b2df..004b9a00103 100644 --- a/actions/setup/js/apply_samples.cjs +++ b/actions/setup/js/apply_samples.cjs @@ -153,7 +153,7 @@ async function fetchPullRequestHeadRef({ owner, repo, pullNumber }) { core.warning(`apply_samples: GET ${url} returned HTTP ${resp.status}`); return null; } - const body = /** @type {{head?: {ref?: string}}} */ (await resp.json()); + const body = /** @type {{head?: {ref?: string}}} */ await resp.json(); const ref = body && body.head && typeof body.head.ref === "string" ? body.head.ref : null; return ref || null; } catch (err) { From f433551e909698011210d86b252771a6f8f73ace Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 8 Jun 2026 19:11:04 +0100 Subject: [PATCH 04/12] CI fixes --- pkg/actionpins/data/action_pins.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/actionpins/data/action_pins.json b/pkg/actionpins/data/action_pins.json index 5d5c29c9c83..ff6ab1d737c 100644 --- a/pkg/actionpins/data/action_pins.json +++ b/pkg/actionpins/data/action_pins.json @@ -385,6 +385,11 @@ "digest": "sha256:0dd1bd91a41e24a3ccc31b1ec6cb61d36608997fabf91f2d643b64e3fc33180a", "pinned_image": "ghcr.io/github/gh-aw-mcpg:v0.3.23@sha256:0dd1bd91a41e24a3ccc31b1ec6cb61d36608997fabf91f2d643b64e3fc33180a" }, + "ghcr.io/github/gh-aw-mcpg:v0.3.24": { + "image": "ghcr.io/github/gh-aw-mcpg:v0.3.24", + "digest": "sha256:72cc921901afa1d67b6fee568f110c6b84436624c437fb2996d14c83e90a2b54", + "pinned_image": "ghcr.io/github/gh-aw-mcpg:v0.3.24@sha256:72cc921901afa1d67b6fee568f110c6b84436624c437fb2996d14c83e90a2b54" + }, "ghcr.io/github/gh-aw-mcpg:v0.3.6": { "image": "ghcr.io/github/gh-aw-mcpg:v0.3.6", "digest": "sha256:2bb8eef86006a4c5963c55616a9c51c32f27bfdecb023b8aa6f91f6718d9171c", From 40babd125eea12207066ed8a58e20a052f1393ba Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 8 Jun 2026 19:15:20 +0100 Subject: [PATCH 05/12] Fix js-typecheck by using @ts-ignore that prettier doesn't strip --- actions/setup/js/apply_samples.cjs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/actions/setup/js/apply_samples.cjs b/actions/setup/js/apply_samples.cjs index 004b9a00103..806a0b39dfc 100644 --- a/actions/setup/js/apply_samples.cjs +++ b/actions/setup/js/apply_samples.cjs @@ -153,7 +153,8 @@ async function fetchPullRequestHeadRef({ owner, repo, pullNumber }) { core.warning(`apply_samples: GET ${url} returned HTTP ${resp.status}`); return null; } - const body = /** @type {{head?: {ref?: string}}} */ await resp.json(); + const body = await resp.json(); + // @ts-ignore - REST API response shape is well-defined; trust GitHub PR endpoint contract const ref = body && body.head && typeof body.head.ref === "string" ? body.head.ref : null; return ref || null; } catch (err) { From 28e526a633a521be5efd5a63a1ba98f1e6d1e1b6 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 10 Jun 2026 15:37:27 +0100 Subject: [PATCH 06/12] Treat detached-HEAD ('HEAD') as invalid in getCurrentBranch Address PR review on #37863: git rev-parse --abbrev-ref HEAD returns the literal string 'HEAD' when the working tree is in a detached-HEAD state (common with the default actions/checkout behaviour). Returning that value as a branch name caused later incremental patch generation to look for origin/HEAD and fail with a misleading 'remote ref not present' error. Now treat 'HEAD' as not-a-branch and fall through to the existing GITHUB_HEAD_REF / GITHUB_REF_NAME env-var fallbacks. If none of those yield a real ref the error message explicitly names detached-HEAD and tells the operator to check out the PR head ref before calling the step. Added unit tests that initialise a real git repo, detach HEAD, and assert both fallbacks plus the actionable error message. --- actions/setup/js/get_current_branch.cjs | 11 +++- actions/setup/js/get_current_branch.test.cjs | 62 ++++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/get_current_branch.cjs b/actions/setup/js/get_current_branch.cjs index f77f9d0a799..e048d304e7e 100644 --- a/actions/setup/js/get_current_branch.cjs +++ b/actions/setup/js/get_current_branch.cjs @@ -20,7 +20,14 @@ function getCurrentBranch(customCwd) { cwd: cwd, stdio: ["pipe", "pipe", "pipe"], }).trim(); - return branch; + // "HEAD" means the repo is in a detached-HEAD state (common with the + // default actions/checkout behaviour). It is not a valid branch name; + // fall through to the GITHUB_HEAD_REF / GITHUB_REF_NAME env-var + // fallbacks so callers get a real ref rather than the literal "HEAD", + // which would later produce a misleading "remote ref not present" error. + if (branch && branch !== "HEAD") { + return branch; + } } catch (error) { // Ignore error and try fallback } @@ -39,7 +46,7 @@ function getCurrentBranch(customCwd) { return ghRefName; } - throw new Error(`${ERR_CONFIG}: Failed to determine current branch: git command failed and no GitHub environment variables available`); + throw new Error(`${ERR_CONFIG}: Failed to determine current branch: git command returned a detached-HEAD state and no GitHub environment variables (GITHUB_HEAD_REF / GITHUB_REF_NAME) are available. Ensure the workflow checks out the pull request's head ref before calling this step.`); } module.exports = { diff --git a/actions/setup/js/get_current_branch.test.cjs b/actions/setup/js/get_current_branch.test.cjs index 19126360e39..b5cb34aa9b9 100644 --- a/actions/setup/js/get_current_branch.test.cjs +++ b/actions/setup/js/get_current_branch.test.cjs @@ -1,4 +1,8 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { execSync } from "child_process"; +import fs from "fs"; +import os from "os"; +import path from "path"; describe("getCurrentBranch", () => { let originalEnv; @@ -94,3 +98,61 @@ describe("getCurrentBranch", () => { } }); }); + +describe("getCurrentBranch detached-HEAD handling", () => { + let tmpDir; + let originalEnv; + + beforeEach(() => { + originalEnv = { + GITHUB_HEAD_REF: process.env.GITHUB_HEAD_REF, + GITHUB_REF_NAME: process.env.GITHUB_REF_NAME, + GITHUB_WORKSPACE: process.env.GITHUB_WORKSPACE, + }; + delete process.env.GITHUB_HEAD_REF; + delete process.env.GITHUB_REF_NAME; + delete process.env.GITHUB_WORKSPACE; + + // Create a real git repo and leave it in detached-HEAD state so that + // `git rev-parse --abbrev-ref HEAD` returns the literal string "HEAD". + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-aw-detached-")); + execSync("git init -b main", { cwd: tmpDir, stdio: "pipe" }); + execSync("git config user.email 'test@example.com'", { cwd: tmpDir, stdio: "pipe" }); + execSync("git config user.name 'Test User'", { cwd: tmpDir, stdio: "pipe" }); + fs.writeFileSync(path.join(tmpDir, "file.txt"), "content"); + execSync("git add file.txt", { cwd: tmpDir, stdio: "pipe" }); + execSync("git commit -m 'init'", { cwd: tmpDir, stdio: "pipe" }); + const sha = execSync("git rev-parse HEAD", { cwd: tmpDir, stdio: "pipe" }).toString().trim(); + // Detach HEAD by checking out the commit SHA directly. + execSync(`git checkout ${sha}`, { cwd: tmpDir, stdio: "pipe" }); + }); + + afterEach(() => { + if (originalEnv.GITHUB_HEAD_REF !== undefined) process.env.GITHUB_HEAD_REF = originalEnv.GITHUB_HEAD_REF; + else delete process.env.GITHUB_HEAD_REF; + if (originalEnv.GITHUB_REF_NAME !== undefined) process.env.GITHUB_REF_NAME = originalEnv.GITHUB_REF_NAME; + else delete process.env.GITHUB_REF_NAME; + if (originalEnv.GITHUB_WORKSPACE !== undefined) process.env.GITHUB_WORKSPACE = originalEnv.GITHUB_WORKSPACE; + else delete process.env.GITHUB_WORKSPACE; + try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch (_) {} + }); + + it("falls back to GITHUB_HEAD_REF when git returns HEAD (detached-HEAD state)", async () => { + process.env.GITHUB_HEAD_REF = "feature/pr-head-ref"; + const { getCurrentBranch } = await import("./get_current_branch.cjs"); + const result = getCurrentBranch(tmpDir); + expect(result).toBe("feature/pr-head-ref"); + }); + + it("falls back to GITHUB_REF_NAME when git returns HEAD and GITHUB_HEAD_REF is absent", async () => { + process.env.GITHUB_REF_NAME = "feature/pr-ref-name"; + const { getCurrentBranch } = await import("./get_current_branch.cjs"); + const result = getCurrentBranch(tmpDir); + expect(result).toBe("feature/pr-ref-name"); + }); + + it("throws an actionable error when git returns HEAD and no env vars are set", async () => { + const { getCurrentBranch } = await import("./get_current_branch.cjs"); + expect(() => getCurrentBranch(tmpDir)).toThrow("detached-HEAD"); + }); +}); From c5c83f04e0be1a1f8bc53eab3688d4a1a8aa47f9 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 10 Jun 2026 16:37:02 +0100 Subject: [PATCH 07/12] update tokens for sample replay --- actions/setup/js/apply_samples.cjs | 40 +++++- actions/setup/js/apply_samples.test.cjs | 66 +++++++++ actions/setup/js/get_current_branch.cjs | 4 +- actions/setup/js/get_current_branch.test.cjs | 4 +- pkg/workflow/samples_replay.go | 92 +++++++++++- pkg/workflow/samples_replay_test.go | 142 +++++++++++++++++++ 6 files changed, 339 insertions(+), 9 deletions(-) diff --git a/actions/setup/js/apply_samples.cjs b/actions/setup/js/apply_samples.cjs index c75925c09ff..a13567eb140 100644 --- a/actions/setup/js/apply_samples.cjs +++ b/actions/setup/js/apply_samples.cjs @@ -129,10 +129,42 @@ function readEventPayload() { } } +/** + * Resolve the best token for a `owner/repo` API call. + * + * Multi-checkout workflows often need different tokens for different + * repositories (e.g. a workflow runs in `owner/automation` but reaches into + * `owner/product` via a separate `checkout:` entry that supplies its own + * `github-token:` or `github-app:`). The compiler emits that mapping as + * `GH_AW_REPO_TOKENS` (a JSON object keyed by `owner/repo`); this helper + * looks the requested slug up there and falls back to GITHUB_TOKEN / + * GH_TOKEN. + * + * @param {string} owner + * @param {string} repo + * @returns {string|undefined} + */ +function selectTokenForRepo(owner, repo) { + const slug = `${owner}/${repo}`; + const raw = process.env.GH_AW_REPO_TOKENS; + if (raw && raw.trim()) { + try { + const map = JSON.parse(raw); + if (map && typeof map === "object" && typeof map[slug] === "string" && map[slug].trim()) { + return map[slug].trim(); + } + } catch (err) { + core.warning(`apply_samples: GH_AW_REPO_TOKENS is not valid JSON, ignoring: ${getErrorMessage(err)}`); + } + } + return process.env.GITHUB_TOKEN || process.env.GH_TOKEN || undefined; +} + /** * Fetch a pull request via the REST API and return its head ref. Uses the - * runner's GITHUB_TOKEN when present; falls back to anonymous (works for - * public repositories). Returns null on any failure so the caller can decide + * per-repo token from GH_AW_REPO_TOKENS when present, falling back to + * GITHUB_TOKEN; falls back to anonymous (works for public repositories) when + * no token is available. Returns null on any failure so the caller can decide * how to recover. * @param {{owner: string, repo: string, pullNumber: number}} args * @returns {Promise} @@ -146,7 +178,7 @@ async function fetchPullRequestHeadRef({ owner, repo, pullNumber }) { "X-GitHub-Api-Version": "2022-11-28", "User-Agent": "gh-aw-apply-samples", }; - const token = process.env.GITHUB_TOKEN || process.env.GH_TOKEN; + const token = selectTokenForRepo(owner, repo); if (token) headers["Authorization"] = `Bearer ${token}`; try { const resp = await fetch(url, { headers }); @@ -500,4 +532,4 @@ if (require.main === module) { }); } -module.exports = { main, loadSamples, preStagePatch, resolveMcpServerPath, sendJsonRpc }; +module.exports = { main, loadSamples, preStagePatch, resolveMcpServerPath, selectTokenForRepo, sendJsonRpc }; diff --git a/actions/setup/js/apply_samples.test.cjs b/actions/setup/js/apply_samples.test.cjs index a5ab24bc0ea..c07ce9a2066 100644 --- a/actions/setup/js/apply_samples.test.cjs +++ b/actions/setup/js/apply_samples.test.cjs @@ -185,6 +185,72 @@ describe("apply_samples.cjs sendJsonRpc", () => { }); }); +describe("apply_samples.cjs selectTokenForRepo", () => { + const { selectTokenForRepo } = require("./apply_samples.cjs"); + + function withEnv(vars, fn) { + const saved = {}; + for (const k of Object.keys(vars)) { + saved[k] = process.env[k]; + if (vars[k] === undefined) delete process.env[k]; + else process.env[k] = vars[k]; + } + try { + return fn(); + } finally { + for (const k of Object.keys(saved)) { + if (saved[k] === undefined) delete process.env[k]; + else process.env[k] = saved[k]; + } + } + } + + it("prefers the per-repo token from GH_AW_REPO_TOKENS over GITHUB_TOKEN", () => { + withEnv( + { + GH_AW_REPO_TOKENS: JSON.stringify({ "owner/cross": "cross-token", "owner/auto": "auto-token" }), + GITHUB_TOKEN: "default-token", + GH_TOKEN: undefined, + }, + () => { + expect(selectTokenForRepo("owner", "cross")).toBe("cross-token"); + expect(selectTokenForRepo("owner", "auto")).toBe("auto-token"); + } + ); + }); + + it("falls back to GITHUB_TOKEN when the slug is not present in GH_AW_REPO_TOKENS", () => { + withEnv( + { + GH_AW_REPO_TOKENS: JSON.stringify({ "owner/cross": "cross-token" }), + GITHUB_TOKEN: "default-token", + GH_TOKEN: undefined, + }, + () => { + expect(selectTokenForRepo("owner", "other")).toBe("default-token"); + } + ); + }); + + it("falls back to GITHUB_TOKEN when GH_AW_REPO_TOKENS is unset", () => { + withEnv({ GH_AW_REPO_TOKENS: undefined, GITHUB_TOKEN: "default-token", GH_TOKEN: undefined }, () => { + expect(selectTokenForRepo("owner", "auto")).toBe("default-token"); + }); + }); + + it("falls back to GITHUB_TOKEN when GH_AW_REPO_TOKENS is malformed JSON", () => { + withEnv({ GH_AW_REPO_TOKENS: "{not json", GITHUB_TOKEN: "default-token", GH_TOKEN: undefined }, () => { + expect(selectTokenForRepo("owner", "auto")).toBe("default-token"); + }); + }); + + it("returns undefined when no env-var token is available and slug is missing", () => { + withEnv({ GH_AW_REPO_TOKENS: undefined, GITHUB_TOKEN: undefined, GH_TOKEN: undefined }, () => { + expect(selectTokenForRepo("owner", "auto")).toBeUndefined(); + }); + }); +}); + describe("apply_samples.cjs preStagePatch (create_pull_request / push_to_pull_request_branch)", () => { // Load the module under test directly so we can drive preStagePatch in // isolation against a real, throwaway git working tree. This is the diff --git a/actions/setup/js/get_current_branch.cjs b/actions/setup/js/get_current_branch.cjs index e048d304e7e..dc643590062 100644 --- a/actions/setup/js/get_current_branch.cjs +++ b/actions/setup/js/get_current_branch.cjs @@ -46,7 +46,9 @@ function getCurrentBranch(customCwd) { return ghRefName; } - throw new Error(`${ERR_CONFIG}: Failed to determine current branch: git command returned a detached-HEAD state and no GitHub environment variables (GITHUB_HEAD_REF / GITHUB_REF_NAME) are available. Ensure the workflow checks out the pull request's head ref before calling this step.`); + throw new Error( + `${ERR_CONFIG}: Failed to determine current branch: git command returned a detached-HEAD state and no GitHub environment variables (GITHUB_HEAD_REF / GITHUB_REF_NAME) are available. Ensure the workflow checks out the pull request's head ref before calling this step.` + ); } module.exports = { diff --git a/actions/setup/js/get_current_branch.test.cjs b/actions/setup/js/get_current_branch.test.cjs index b5cb34aa9b9..1348ec265f6 100644 --- a/actions/setup/js/get_current_branch.test.cjs +++ b/actions/setup/js/get_current_branch.test.cjs @@ -134,7 +134,9 @@ describe("getCurrentBranch detached-HEAD handling", () => { else delete process.env.GITHUB_REF_NAME; if (originalEnv.GITHUB_WORKSPACE !== undefined) process.env.GITHUB_WORKSPACE = originalEnv.GITHUB_WORKSPACE; else delete process.env.GITHUB_WORKSPACE; - try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch (_) {} + try { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } catch (_) {} }); it("falls back to GITHUB_HEAD_REF when git returns HEAD (detached-HEAD state)", async () => { diff --git a/pkg/workflow/samples_replay.go b/pkg/workflow/samples_replay.go index b72568a9b13..a271c3925d8 100644 --- a/pkg/workflow/samples_replay.go +++ b/pkg/workflow/samples_replay.go @@ -66,6 +66,56 @@ func collectSampleEntries(config *SafeOutputsConfig) []SampleEntry { return entries } +// collectSampleRepoTokens walks the workflow's checkout configs and returns +// a map of "owner/repo" -> token expression so that apply_samples.cjs can +// pick the right token when calling the GitHub REST API to resolve a PR head +// ref for a cross-repo sample. +// +// The keys are repository slugs as written in the workflow frontmatter; the +// value is the GitHub Actions expression that resolves to the token at +// runtime — for plain `github-token: ${{ secrets.X }}` checkouts this is the +// expression itself, and for `github-app:` checkouts it is the +// `${{ steps.checkout-app-token-N.outputs.token }}` reference for the same +// app-token step that the agent job's checkout uses. Entries whose checkout +// declares no auth are omitted; the driver falls back to GITHUB_TOKEN for +// those. +func collectSampleRepoTokens(configs []*CheckoutConfig) map[string]string { + if len(configs) == 0 { + return nil + } + cm := NewCheckoutManager(configs) + tokens := make(map[string]string) + for i, entry := range cm.ordered { + repo := entry.key.repository + if repo == "" { + // Empty repository targets `github.repository` at runtime. Emit a + // substituted key so the runtime JSON has the actual slug. + repo = "${{ github.repository }}" + } + var token string + switch { + case entry.githubApp != nil: + //nolint:gosec // G101: GitHub Actions expression template, not a hardcoded credential + token = fmt.Sprintf("${{ steps.checkout-app-token-%d.outputs.token }}", i) + case entry.token != "": + token = entry.token + default: + continue + } + // First-seen wins so the per-repo entry from the user's frontmatter + // takes precedence over later imported configs (CheckoutManager + // already enforces this for merged entries; this guards against + // distinct entries that share a repo but differ in path). + if _, exists := tokens[repo]; !exists { + tokens[repo] = token + } + } + if len(tokens) == 0 { + return nil + } + return tokens +} + // generateSamplesReplayStep emits the YAML that replaces the agentic // `Execute coding agent` step when the hidden `gh aw compile --use-samples` // flag is used. It spawns the safe-outputs MCP server over stdio and feeds it @@ -95,6 +145,34 @@ func (c *Compiler) generateSamplesReplayStep(yaml *strings.Builder, data *Workfl payload = []byte("[]") } + // Build the per-repo token map so apply_samples.cjs can reach repos that + // require a non-default token (cross-repo `checkout:` entries with their + // own `github-token:` or `github-app:`). + repoTokens := collectSampleRepoTokens(data.CheckoutConfigs) + var repoTokensPayload []byte + if len(repoTokens) > 0 { + // Sort keys for deterministic output. + keys := make([]string, 0, len(repoTokens)) + for k := range repoTokens { + keys = append(keys, k) + } + sort.Strings(keys) + var buf strings.Builder + buf.WriteString("{") + for i, k := range keys { + if i > 0 { + buf.WriteString(",") + } + kEnc, _ := json.Marshal(k) + vEnc, _ := json.Marshal(repoTokens[k]) + buf.Write(kEnc) + buf.WriteString(":") + buf.Write(vEnc) + } + buf.WriteString("}") + repoTokensPayload = []byte(buf.String()) + } + yaml.WriteString(" - name: Replay safe-outputs samples (deterministic)\n") yaml.WriteString(" id: agentic_execution\n") yaml.WriteString(" env:\n") @@ -105,10 +183,18 @@ func (c *Compiler) generateSamplesReplayStep(yaml *strings.Builder, data *Workfl fmt.Fprintf(yaml, " GH_AW_AGENT_STDIO_LOG: %s\n", logFile) yaml.WriteString(" GH_AW_SAFE_OUTPUTS_CONFIG_PATH: ${{ runner.temp }}/gh-aw/safeoutputs/config.json\n") yaml.WriteString(" GH_AW_SAFE_OUTPUTS: ${{ runner.temp }}/gh-aw/safeoutputs/outputs.jsonl\n") - // GITHUB_TOKEN lets apply_samples.cjs resolve pull-request head refs via - // the REST API for issue_comment / slash_command events, where the head - // ref is not present in the event payload. + // GITHUB_TOKEN is the fallback used by apply_samples.cjs when resolving a + // pull-request head ref via the REST API for issue_comment / slash_command + // events. For cross-repo samples whose target repository has its own + // `checkout:` entry with `github-token:` or `github-app:`, the driver + // prefers the matching token from GH_AW_REPO_TOKENS below. yaml.WriteString(" GITHUB_TOKEN: ${{ github.token }}\n") + if repoTokensPayload != nil { + yaml.WriteString(" GH_AW_REPO_TOKENS: |\n") + for line := range strings.SplitSeq(string(repoTokensPayload), "\n") { + fmt.Fprintf(yaml, " %s\n", line) + } + } yaml.WriteString(" run: |\n") yaml.WriteString(" set -euo pipefail\n") yaml.WriteString(" mkdir -p \"$(dirname \"$GH_AW_AGENT_STDIO_LOG\")\"\n") diff --git a/pkg/workflow/samples_replay_test.go b/pkg/workflow/samples_replay_test.go index e4804bba2e6..bc119125db2 100644 --- a/pkg/workflow/samples_replay_test.go +++ b/pkg/workflow/samples_replay_test.go @@ -409,3 +409,145 @@ Runtime-templated sample for workflow_dispatch-driven testing. require.NoError(t, err, "GH_AW_SAMPLES should remain valid JSON at compile time") require.NotEmpty(t, parsed, "GH_AW_SAMPLES should include at least one sample entry") } + +// TestUseSamplesEmitsPerRepoTokenMap verifies that a workflow with multiple +// `checkout:` entries — each carrying its own `github-token:` — produces a +// `GH_AW_REPO_TOKENS` env var whose JSON map binds every repo slug to the +// matching token expression. apply_samples.cjs uses that map to pick the +// right token when it calls the GitHub REST API to resolve a PR head ref +// for a cross-repo sample (issue: a single hard-coded `github.token` +// would fail for repos that require a PAT or App-token). +func TestUseSamplesEmitsPerRepoTokenMap(t *testing.T) { + const md = `--- +on: + workflow_dispatch: +permissions: read-all +engine: + id: claude +checkout: + - fetch-depth: 0 + path: ./automation + - repository: github/github + token: ${{ secrets.CRITICAL_PATH_GITHUB_GITHUB_PAT }} + path: ./github +safe-outputs: + create-issue: + samples: + - title: "Cross-repo sample" + body: "Body for cross-repo issue." +--- + +Multi-checkout workflow exercising the per-repo token map. +` + tmpFile, err := os.CreateTemp("", "use-samples-tokens-*.md") + if err != nil { + t.Fatal(err) + } + defer os.Remove(tmpFile.Name()) + if _, err := tmpFile.WriteString(md); err != nil { + t.Fatal(err) + } + tmpFile.Close() + + compiler := NewCompiler() + compiler.SetUseSamples(true) + if err := compiler.CompileWorkflow(tmpFile.Name()); err != nil { + t.Fatalf("compile failed: %v", err) + } + lockPath := strings.TrimSuffix(tmpFile.Name(), ".md") + ".lock.yml" + defer os.Remove(lockPath) + b, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("read lock: %v", err) + } + lock := string(b) + + require.Contains(t, lock, "Replay safe-outputs samples (deterministic)") + require.Contains(t, lock, "GH_AW_REPO_TOKENS: |", "expected GH_AW_REPO_TOKENS env var in lock file") + + tokensJSON := extractGHAWRepoTokensJSON(t, lock) + var tokens map[string]string + require.NoError(t, json.Unmarshal([]byte(tokensJSON), &tokens), "GH_AW_REPO_TOKENS must be valid JSON") + assert.Equal(t, "${{ secrets.CRITICAL_PATH_GITHUB_GITHUB_PAT }}", tokens["github/github"], "cross-repo entry must map to its declared token") + // The default-checkout entry has no explicit token and must NOT appear in + // the map (GITHUB_TOKEN handles that case in apply_samples.cjs). + _, hasDefaultRepo := tokens["${{ github.repository }}"] + assert.False(t, hasDefaultRepo, "default checkout without a github-token must not appear in the map; got: %v", tokens) + // GITHUB_TOKEN fallback must still be emitted alongside the map. + assert.Contains(t, lock, "GITHUB_TOKEN: ${{ github.token }}") +} + +// TestUseSamplesOmitsRepoTokenMapWhenNoCustomAuth verifies that workflows +// whose `checkout:` entries declare no explicit token (i.e. rely on the +// runner's default GITHUB_TOKEN) do NOT emit a GH_AW_REPO_TOKENS env var. +// This keeps the generated YAML noise-free for the common case. +func TestUseSamplesOmitsRepoTokenMapWhenNoCustomAuth(t *testing.T) { + const md = `--- +on: + workflow_dispatch: +permissions: read-all +engine: + id: claude +safe-outputs: + create-issue: + samples: + - title: "No custom auth" + body: "Body for the default-auth sample issue, long enough to satisfy schema." +--- + +Default-auth workflow — should not need the per-repo token map. +` + tmpFile, err := os.CreateTemp("", "use-samples-no-tokens-*.md") + if err != nil { + t.Fatal(err) + } + defer os.Remove(tmpFile.Name()) + if _, err := tmpFile.WriteString(md); err != nil { + t.Fatal(err) + } + tmpFile.Close() + + compiler := NewCompiler() + compiler.SetUseSamples(true) + require.NoError(t, compiler.CompileWorkflow(tmpFile.Name())) + lockPath := strings.TrimSuffix(tmpFile.Name(), ".md") + ".lock.yml" + defer os.Remove(lockPath) + b, err := os.ReadFile(lockPath) + require.NoError(t, err) + lock := string(b) + + assert.Contains(t, lock, "Replay safe-outputs samples (deterministic)") + assert.NotContains(t, lock, "GH_AW_REPO_TOKENS", "GH_AW_REPO_TOKENS must be omitted when no checkout supplies a custom token") +} + +// extractGHAWRepoTokensJSON pulls the literal block scalar value of +// GH_AW_REPO_TOKENS out of the compiled YAML and returns the unindented JSON +// text. Mirrors extractGHAWSamplesJSON. +func extractGHAWRepoTokensJSON(t *testing.T, lock string) string { + t.Helper() + const marker = "GH_AW_REPO_TOKENS: |\n" + start := strings.Index(lock, marker) + if start < 0 { + t.Fatalf("could not find %q in lock file", marker) + } + start += len(marker) + rest := lock[start:] + firstNL := strings.Index(rest, "\n") + if firstNL < 0 { + t.Fatal("malformed GH_AW_REPO_TOKENS block: no newline after first line") + } + firstLine := rest[:firstNL] + indent := firstLine[:len(firstLine)-len(strings.TrimLeft(firstLine, " "))] + if indent == "" { + t.Fatal("malformed GH_AW_REPO_TOKENS block: expected indented content") + } + var out strings.Builder + for _, line := range strings.Split(rest, "\n") { + if !strings.HasPrefix(line, indent) { + break + } + out.WriteString(strings.TrimPrefix(line, indent)) + out.WriteString("\n") + } + return strings.TrimSpace(out.String()) +} From 7678ec49951b6fe3f2addf75f92242ae28660e90 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 10 Jun 2026 16:44:10 +0100 Subject: [PATCH 08/12] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- actions/setup/js/apply_samples.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actions/setup/js/apply_samples.cjs b/actions/setup/js/apply_samples.cjs index a13567eb140..6ca9fa4bef4 100644 --- a/actions/setup/js/apply_samples.cjs +++ b/actions/setup/js/apply_samples.cjs @@ -273,7 +273,7 @@ async function preStagePatch(entry, index, workspace) { if (!branch) { throw new Error( `apply_samples: cannot derive pull-request head branch for sample[${index}] (tool=${entry.tool}). ` + - `Trigger the workflow from a pull_request event, or set sample.pull_request_number, ` + + `Trigger the workflow from a pull_request event, or set arguments.pull_request_number on the sample, ` + `or provide GITHUB_TOKEN so the PR can be fetched.` ); } From 8b6c4455d1599d721f3fb359f3b202e43117e3dd Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 10 Jun 2026 16:44:21 +0100 Subject: [PATCH 09/12] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- actions/setup/js/safe_outputs_handlers.cjs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index 7b0490a2874..a297f900472 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -919,8 +919,7 @@ function createHandlers(server, appendSafeOutput, config = {}) { // like issue_comment on PRs targeting non-default branches. entry.base_branch = baseBranch; - // If branch is not provided, is empty, or equals the base branch, use the current branch from git - // This handles cases where the agent incorrectly passes the base branch instead of the working branch + // Always derive the source branch from the current checkout (the agent does not supply one). // The agent never supplies a branch. Always derive it from the current // checkout: the working tree must already be on the PR head ref because // that's what the agent committed onto. The apply-time push job From 09e27ab8153df7a1abdb60312151be383939b6af Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 10 Jun 2026 17:00:37 +0100 Subject: [PATCH 10/12] CI fixes --- actions/setup/js/send_otlp_span.cjs | 2 +- pkg/workflow/samples_replay.go | 52 ++++++++++++++++------------- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/actions/setup/js/send_otlp_span.cjs b/actions/setup/js/send_otlp_span.cjs index 736a1773599..1a5e3788e78 100644 --- a/actions/setup/js/send_otlp_span.cjs +++ b/actions/setup/js/send_otlp_span.cjs @@ -2070,7 +2070,7 @@ async function sendJobConclusionSpan(spanName, options = {}) { // (zero != no-data) and lets the Sentry EAP schema infer the attribute as numeric // so sum()/avg()/percentile() aggregations work without manual schema configuration. const aiCreditsFromEnv = normalizeNonNegativeNumber(process.env.GH_AW_AIC); - const aiCreditsFromFile = agentUsage.ai_credits; + const aiCreditsFromFile = agentUsage.ai_credits ?? 0; const aiCreditsFromMetrics = runtimeMetrics.tokenUsage?.ai_credits; const aiCredits = jobEmitsOwnTokenUsage ? (aiCreditsFromEnv ?? (aiCreditsFromFile > 0 ? aiCreditsFromFile : (aiCreditsFromMetrics ?? aiCreditsFromFile))) : undefined; if (typeof aiCredits === "number") { diff --git a/pkg/workflow/samples_replay.go b/pkg/workflow/samples_replay.go index a271c3925d8..2e025126e1f 100644 --- a/pkg/workflow/samples_replay.go +++ b/pkg/workflow/samples_replay.go @@ -116,6 +116,34 @@ func collectSampleRepoTokens(configs []*CheckoutConfig) map[string]string { return tokens } +// marshalStringMapDeterministic returns a compact JSON object encoding of m +// with keys sorted lexicographically so the emitted YAML is byte-stable +// across runs. Returns nil for empty/nil maps so callers can skip emission. +func marshalStringMapDeterministic(m map[string]string) []byte { + if len(m) == 0 { + return nil + } + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + var buf strings.Builder + buf.WriteString("{") + for i, k := range keys { + if i > 0 { + buf.WriteString(",") + } + kEnc, _ := json.Marshal(k) + vEnc, _ := json.Marshal(m[k]) + buf.Write(kEnc) + buf.WriteString(":") + buf.Write(vEnc) + } + buf.WriteString("}") + return []byte(buf.String()) +} + // generateSamplesReplayStep emits the YAML that replaces the agentic // `Execute coding agent` step when the hidden `gh aw compile --use-samples` // flag is used. It spawns the safe-outputs MCP server over stdio and feeds it @@ -149,29 +177,7 @@ func (c *Compiler) generateSamplesReplayStep(yaml *strings.Builder, data *Workfl // require a non-default token (cross-repo `checkout:` entries with their // own `github-token:` or `github-app:`). repoTokens := collectSampleRepoTokens(data.CheckoutConfigs) - var repoTokensPayload []byte - if len(repoTokens) > 0 { - // Sort keys for deterministic output. - keys := make([]string, 0, len(repoTokens)) - for k := range repoTokens { - keys = append(keys, k) - } - sort.Strings(keys) - var buf strings.Builder - buf.WriteString("{") - for i, k := range keys { - if i > 0 { - buf.WriteString(",") - } - kEnc, _ := json.Marshal(k) - vEnc, _ := json.Marshal(repoTokens[k]) - buf.Write(kEnc) - buf.WriteString(":") - buf.Write(vEnc) - } - buf.WriteString("}") - repoTokensPayload = []byte(buf.String()) - } + repoTokensPayload := marshalStringMapDeterministic(repoTokens) yaml.WriteString(" - name: Replay safe-outputs samples (deterministic)\n") yaml.WriteString(" id: agentic_execution\n") From 5eb0c6521ae4be5b0f4023fa321d61caf349cdab Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 10 Jun 2026 17:02:00 +0100 Subject: [PATCH 11/12] CI fixes --- pkg/workflow/samples_replay.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/workflow/samples_replay.go b/pkg/workflow/samples_replay.go index 2e025126e1f..fa41f999967 100644 --- a/pkg/workflow/samples_replay.go +++ b/pkg/workflow/samples_replay.go @@ -134,8 +134,8 @@ func marshalStringMapDeterministic(m map[string]string) []byte { if i > 0 { buf.WriteString(",") } - kEnc, _ := json.Marshal(k) - vEnc, _ := json.Marshal(m[k]) + kEnc, _ := json.Marshal(k) //nolint:jsonmarshalignoredeerror // marshaling a string cannot fail + vEnc, _ := json.Marshal(m[k]) //nolint:jsonmarshalignoredeerror // marshaling a string cannot fail buf.Write(kEnc) buf.WriteString(":") buf.Write(vEnc) From 7876fe557eca0375a48ae1ee1ed5d30f0346d00e Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 10 Jun 2026 22:33:33 +0100 Subject: [PATCH 12/12] update --- actions/setup/js/apply_samples.cjs | 14 ++++- actions/setup/js/apply_samples.test.cjs | 57 ++++++++++++++++--- actions/setup/js/safe_outputs_handlers.cjs | 13 ++--- .../setup/js/safe_outputs_handlers.test.cjs | 17 ++++++ actions/setup/js/send_otlp_span.cjs | 2 +- .../safe_outputs_validation_config.go | 1 + pkg/workflow/samples_replay.go | 38 ++++--------- 7 files changed, 98 insertions(+), 44 deletions(-) diff --git a/actions/setup/js/apply_samples.cjs b/actions/setup/js/apply_samples.cjs index 6ca9fa4bef4..68274141f90 100644 --- a/actions/setup/js/apply_samples.cjs +++ b/actions/setup/js/apply_samples.cjs @@ -273,7 +273,7 @@ async function preStagePatch(entry, index, workspace) { if (!branch) { throw new Error( `apply_samples: cannot derive pull-request head branch for sample[${index}] (tool=${entry.tool}). ` + - `Trigger the workflow from a pull_request event, or set arguments.pull_request_number on the sample, ` + + `Trigger the workflow from a pull_request event, or set arguments.pull_request_number on the sample entry, ` + `or provide GITHUB_TOKEN so the PR can be fetched.` ); } @@ -532,4 +532,14 @@ if (require.main === module) { }); } -module.exports = { main, loadSamples, preStagePatch, resolveMcpServerPath, selectTokenForRepo, sendJsonRpc }; +module.exports = { + main, + loadSamples, + preStagePatch, + resolveMcpServerPath, + selectTokenForRepo, + sendJsonRpc, + // Exported for unit testing of the 3-tier PR head ref resolution logic. + derivePrHeadRef, + fetchPullRequestHeadRef, +}; diff --git a/actions/setup/js/apply_samples.test.cjs b/actions/setup/js/apply_samples.test.cjs index c07ce9a2066..5a0a6ac45a3 100644 --- a/actions/setup/js/apply_samples.test.cjs +++ b/actions/setup/js/apply_samples.test.cjs @@ -12,7 +12,7 @@ // Tests intentionally use the simplest safe-output tool (`create_issue`) so we // do not need to set up a git working tree for patch sidecars. -import { describe, it, expect, beforeAll } from "vitest"; +import { describe, it, expect, beforeAll, vi } from "vitest"; import { spawnSync } from "child_process"; import { createRequire } from "module"; import fs from "fs"; @@ -379,12 +379,13 @@ describe("apply_samples.cjs preStagePatch (create_pull_request / push_to_pull_re }) ); - // Stub global fetch to simulate the GitHub REST API response. - const realFetch = global.fetch; - global.fetch = async url => { - expect(String(url)).toContain("/repos/owner/repo/pulls/42"); - return { ok: true, status: 200, json: async () => ({ head: { ref: headRef } }) }; - }; + // Spy on global fetch so the call-URL assertion happens on the test side + // (an assertion failure inside a global-mutating stub is hard to attribute). + const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ head: { ref: headRef } }), + }); const prevBase = process.env.GH_AW_CUSTOM_BASE_BRANCH; const prevEvent = process.env.GITHUB_EVENT_PATH; @@ -400,8 +401,9 @@ describe("apply_samples.cjs preStagePatch (create_pull_request / push_to_pull_re }; await preStagePatch(entry, 0, workspace); expect(git(["rev-parse", "--abbrev-ref", "HEAD"], workspace).trim()).toBe(headRef); + expect(fetchSpy).toHaveBeenCalledWith(expect.stringContaining("/repos/owner/repo/pulls/42"), expect.anything()); } finally { - global.fetch = realFetch; + fetchSpy.mockRestore(); if (prevBase === undefined) delete process.env.GH_AW_CUSTOM_BASE_BRANCH; else process.env.GH_AW_CUSTOM_BASE_BRANCH = prevBase; if (prevEvent === undefined) delete process.env.GITHUB_EVENT_PATH; @@ -429,6 +431,45 @@ describe("apply_samples.cjs preStagePatch (create_pull_request / push_to_pull_re } }); + it("derives push_to_pull_request_branch branch from explicit arguments.pull_request_number when no event payload exists", async () => { + // Resolution path 3: no GITHUB_EVENT_PATH, but the sample entry carries an + // explicit `arguments.pull_request_number`. The driver should hit the PR + // API directly rather than fail fast. + const workspace = makeTempDir("gh-aw-prestage-push-argpr-"); + initRepo(workspace, "main"); + + const headRef = "feat/explicit-pr-number"; + const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ head: { ref: headRef } }), + }); + + const prevBase = process.env.GH_AW_CUSTOM_BASE_BRANCH; + const prevEvent = process.env.GITHUB_EVENT_PATH; + const prevRepo = process.env.GITHUB_REPOSITORY; + delete process.env.GITHUB_EVENT_PATH; // force path 3 + process.env.GH_AW_CUSTOM_BASE_BRANCH = "main"; + process.env.GITHUB_REPOSITORY = "owner/repo"; + try { + const entry = { + tool: "push_to_pull_request_branch", + arguments: { message: "Push update", pull_request_number: 99 }, + sidecars: { patch: newFileDiff("arg-pr.txt", "via arg pr\n") }, + }; + await preStagePatch(entry, 0, workspace); + expect(git(["rev-parse", "--abbrev-ref", "HEAD"], workspace).trim()).toBe(headRef); + expect(fetchSpy).toHaveBeenCalledWith(expect.stringContaining("/repos/owner/repo/pulls/99"), expect.anything()); + } finally { + fetchSpy.mockRestore(); + if (prevBase === undefined) delete process.env.GH_AW_CUSTOM_BASE_BRANCH; + else process.env.GH_AW_CUSTOM_BASE_BRANCH = prevBase; + if (prevEvent !== undefined) process.env.GITHUB_EVENT_PATH = prevEvent; + if (prevRepo === undefined) delete process.env.GITHUB_REPOSITORY; + else process.env.GITHUB_REPOSITORY = prevRepo; + } + }); + it("is a no-op when the sample tool isn't in the patch-sidecar set", async () => { // We assert this at the driver level (PATCH_SIDECAR_TOOLS gate in main()), // but preStagePatch itself should also be a no-op when called with an diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index a297f900472..bafa4f25cb7 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -919,13 +919,12 @@ function createHandlers(server, appendSafeOutput, config = {}) { // like issue_comment on PRs targeting non-default branches. entry.base_branch = baseBranch; - // Always derive the source branch from the current checkout (the agent does not supply one). - // The agent never supplies a branch. Always derive it from the current - // checkout: the working tree must already be on the PR head ref because - // that's what the agent committed onto. The apply-time push job - // independently re-derives the destination from pulls.get(pull_number), - // so this branch name is used only as the source ref for the incremental - // diff against origin/. + // The agent never supplies a branch; the validator already strips it from + // args. Derive it from the current checkout: the working tree must be on + // the PR head ref because that's what the agent committed onto. The + // apply-time push job independently re-derives the destination from + // pulls.get(pull_number), so this branch name is used only as the source + // ref for the incremental diff against origin/. try { const detectedBranch = getCurrentBranch(repoCwd); server.debug(`Using current branch for push_to_pull_request_branch: ${detectedBranch}`); diff --git a/actions/setup/js/safe_outputs_handlers.test.cjs b/actions/setup/js/safe_outputs_handlers.test.cjs index 4eef2bdfbbf..b2824000a15 100644 --- a/actions/setup/js/safe_outputs_handlers.test.cjs +++ b/actions/setup/js/safe_outputs_handlers.test.cjs @@ -1528,6 +1528,23 @@ describe("safe_outputs_handlers", () => { delete process.env.GITHUB_BASE_REF; } }); + + it("returns error when getCurrentBranch cannot resolve a branch", async () => { + // Override the beforeEach seed so every getCurrentBranch resolution path + // fails (no GITHUB_REF_NAME, no GITHUB_HEAD_REF, no git repo in the empty + // test workspace) and the handler hits its try/catch. + delete process.env.GITHUB_REF_NAME; + delete process.env.GITHUB_HEAD_REF; + try { + const result = await handlers.pushToPullRequestBranchHandler({ message: "test" }); + expect(result.isError).toBe(true); + const data = JSON.parse(result.content[0].text); + expect(data.result).toBe("error"); + expect(data.error).toContain("Failed to determine source branch"); + } finally { + process.env.GITHUB_REF_NAME = "feature-branch"; // restore for sibling tests + } + }); }); describe("handler structure", () => { diff --git a/actions/setup/js/send_otlp_span.cjs b/actions/setup/js/send_otlp_span.cjs index 491bede4234..04ec3f1a5e0 100644 --- a/actions/setup/js/send_otlp_span.cjs +++ b/actions/setup/js/send_otlp_span.cjs @@ -2075,7 +2075,7 @@ async function sendJobConclusionSpan(spanName, options = {}) { // sum()/avg()/percentile() aggregations work without manual schema configuration, // and Tempo indexes it so { span."gh-aw.aic" > 0 } is queryable immediately. const aiCreditsFromEnv = normalizeNonNegativeNumber(process.env.GH_AW_AIC); - const aiCreditsFromFile = agentUsage.ai_credits ?? 0; + const aiCreditsFromFile = agentUsage.ai_credits; const aiCreditsFromMetrics = runtimeMetrics.tokenUsage?.ai_credits; const aiCredits = jobEmitsOwnTokenUsage ? (aiCreditsFromEnv ?? ((aiCreditsFromFile ?? 0) > 0 ? aiCreditsFromFile : (aiCreditsFromMetrics ?? aiCreditsFromFile ?? 0))) : undefined; if (typeof aiCredits === "number") { diff --git a/pkg/workflow/safe_outputs_validation_config.go b/pkg/workflow/safe_outputs_validation_config.go index 6107e0bf5e0..4479f4bc65f 100644 --- a/pkg/workflow/safe_outputs_validation_config.go +++ b/pkg/workflow/safe_outputs_validation_config.go @@ -210,6 +210,7 @@ var ValidationConfig = map[string]TypeValidationConfig{ Fields: map[string]FieldValidation{ "message": {Required: true, Type: "string", Sanitize: true, MaxLength: MaxBodyLength}, "pull_request_number": {IssueOrPRNumber: true}, + "branch": {Type: "string", Sanitize: true, MaxLength: 256}, // Optional: stripped before MCP call; validated for type/length when present. }, }, "create_pull_request_review_comment": { diff --git a/pkg/workflow/samples_replay.go b/pkg/workflow/samples_replay.go index fa41f999967..ecdd93404c8 100644 --- a/pkg/workflow/samples_replay.go +++ b/pkg/workflow/samples_replay.go @@ -88,8 +88,10 @@ func collectSampleRepoTokens(configs []*CheckoutConfig) map[string]string { for i, entry := range cm.ordered { repo := entry.key.repository if repo == "" { - // Empty repository targets `github.repository` at runtime. Emit a - // substituted key so the runtime JSON has the actual slug. + // The repo slug is unknown at compile time; emit a GitHub Actions + // expression. The Actions runner expands ${{ github.repository }} to + // "owner/repo" before apply_samples.cjs reads GH_AW_REPO_TOKENS, so + // the runtime key matches the slug the driver looks up. repo = "${{ github.repository }}" } var token string @@ -116,32 +118,16 @@ func collectSampleRepoTokens(configs []*CheckoutConfig) map[string]string { return tokens } -// marshalStringMapDeterministic returns a compact JSON object encoding of m -// with keys sorted lexicographically so the emitted YAML is byte-stable -// across runs. Returns nil for empty/nil maps so callers can skip emission. -func marshalStringMapDeterministic(m map[string]string) []byte { +// marshalRepoTokens returns a compact JSON object encoding of m, or nil for +// empty/nil maps so callers can skip emission. encoding/json sorts string- +// keyed map entries lexicographically, so the output is byte-stable across +// runs without an explicit sort step. +func marshalRepoTokens(m map[string]string) []byte { if len(m) == 0 { return nil } - keys := make([]string, 0, len(m)) - for k := range m { - keys = append(keys, k) - } - sort.Strings(keys) - var buf strings.Builder - buf.WriteString("{") - for i, k := range keys { - if i > 0 { - buf.WriteString(",") - } - kEnc, _ := json.Marshal(k) //nolint:jsonmarshalignoredeerror // marshaling a string cannot fail - vEnc, _ := json.Marshal(m[k]) //nolint:jsonmarshalignoredeerror // marshaling a string cannot fail - buf.Write(kEnc) - buf.WriteString(":") - buf.Write(vEnc) - } - buf.WriteString("}") - return []byte(buf.String()) + out, _ := json.Marshal(m) //nolint:jsonmarshalignoredeerror // marshaling a string map cannot fail + return out } // generateSamplesReplayStep emits the YAML that replaces the agentic @@ -177,7 +163,7 @@ func (c *Compiler) generateSamplesReplayStep(yaml *strings.Builder, data *Workfl // require a non-default token (cross-repo `checkout:` entries with their // own `github-token:` or `github-app:`). repoTokens := collectSampleRepoTokens(data.CheckoutConfigs) - repoTokensPayload := marshalStringMapDeterministic(repoTokens) + repoTokensPayload := marshalRepoTokens(repoTokens) yaml.WriteString(" - name: Replay safe-outputs samples (deterministic)\n") yaml.WriteString(" id: agentic_execution\n")