Let the build-failure analyst push fixes it cannot suggest inline

[YuliiaKovalova]

Why

GitHub only accepts inline suggestion blocks on lines that are part of a PR's diff. Dependency-flow PRs break exactly that assumption: their diff is nothing but version files, so when a flowed package changes an API and previously-untouched call sites stop compiling, the build-failure analyst can only describe the fix and ask a human to commit it.

Concrete example that motivated this: dotnet/arcade#17348 (comment) — the analysis correctly identified two CS1503s in PublishBuildToMaestro.cs (a flowed Microsoft.DotNet.ProductConstructionService.Client inserted a parameter before CancellationToken) but had to end with "the fix needs to be committed separately", because that file is not in the dependency-bump diff.

What

Enable the push-to-pull-request-branch safe output on the automatic build-failure-analysis workflow (the /analyze-build-failure command workflow is deliberately left comment-only), so the agent can append the fix commit when — and only when — a suggestion structurally cannot carry it.

Guardrails

• The push target is not the model's to choose. With target: "*" gh-aw takes the pull request number out of the agent's own tool call and only then checks whether that pull request is a fork, so an injected agent could redirect the push at an unrelated same-repo PR. The target is instead bound to check_run.pull_requests[0].number, which GitHub fills in itself and which never passes through the model.
• Fork PRs can never be pushed to. GitHub leaves check_run.pull_requests empty for fork-originated check runs, so a fork PR has no push target at all (verified against live check runs in all three repos: same-repo PRs report exactly one entry, fork ones report none) — and gh-aw's handler refuses fork branches regardless. That bounds roles: all to same-repo branches: dependency flow plus people who already have write access. Pushes are append-only; force-push is impossible. The comment-only analysis still runs on fork PRs.
• allowed-files is an exclusive allowlist, so build infrastructure (eng/, global.json, NuGet.config, .github/) is out of reach no matter what the agent produces; protected-files keeps its default blocked policy on top.
• max: 1 plus a loop guard enforced entirely in trusted code. commit-title-suffix makes gh-aw's push handler stamp [build-failure-analysis] onto the commit title as it applies the patch, so the marker is written by the handler and never by the model. This requires patch-format: am: the default bundle transport never rewrites commit titles, which would have made the marker — and therefore the guard — a silent no-op. Before anything else runs, the fetch job reads the PR's head commit; if that tip is already such a commit and the build still fails, it publishes push-blocked, and the workflow's job-level if: skips the activation and agent jobs — gh-aw's safe_outputs job is itself conditioned on the agent not being skipped, so no push code path remains. The check fails closed (an unreadable commit blocks) and is scoped to the branch tip, so any later commit by anyone restores full analysis rather than abandoning the PR after one attempt. The push uses GITHUB_TOKEN, which does not re-trigger GitHub Actions — but the Azure DevOps GitHub app does rebuild, so this guard is the real brake on a fail → push → rebuild → fail loop.
• Step 6b of the analyst playbook additionally requires the fix to be outside the diff, mechanical, and provable from the compiler error itself. Anything needing a design decision, changing behavior, or suppressing an analyzer stays a comment.
• fallback-as-pull-request: false so a diverged branch cannot silently become a surprise PR.

Supporting changes

• PR-head checkout. Authoring the commit needs the PR's tree — push-to-pull-request-branch ships file contents, so a fix authored against main would silently revert whatever else changed in that file. The fetch job now resolves pr-checkout-ref: the head branch name for same-repo PRs (gh-aw derives the push target from git rev-parse --abbrev-ref HEAD, so a detached SHA checkout would break bundle generation), falling back to refs/pull/<n>/head for forks.
• Prompt-injection mitigation. gh-aw's own base-branch config restore is gated on its built-in PR-checkout step, which never fires for check_run (that event carries no pull_request payload). Checking out the PR head therefore puts PR-controlled agent-config content in the workspace — where the analyst reads its own playbook. A second sparse checkout of the base branch plus a pre-agent-steps step restores it before the agent starts, replaying gh-aw's inline sub-agent/skill restores afterwards. That checkout uses the PR's own base ref (resolved from the GitHub API by the fetch job), not the repository default branch, so a release-branch PR is analysed with the playbook that branch actually carries. The restore covers gh-aw's full protected set — folders .agents .antigravity .claude .codex .crush .gemini .github .opencode .pi and root files .crush.json .mcp.json AGENTS.md ANTIGRAVITY.md CLAUDE.md GEMINI.md PI.md opencode.jsonc — including paths that do not exist on the base branch, which are deleted rather than left as PR-authored content.
• No new secret is required. GH_AW_CI_TRIGGER_TOKEN appears in the lock because gh-aw wires that magic secret into every workflow using this safe output; it only exists to add an empty commit under a PAT so GitHub Actions CI re-runs on the pushed commit. It is deliberately left unset — our CI is Azure DevOps, which rebuilds on its own — and when unset the value is empty and gh-aw simply skips that step.
• No PR code is built or executed. The workflow's bash allowlist gains only scoped git status/diff/log/rev-parse/add/commit — no interpreters, package managers or build tools. Note that gh-aw itself appends git branch/checkout/merge/rm/switch to the generated --allow-tool list whenever push-to-pull-request-branch is enabled; that cannot be narrowed from the workflow config, so the analyst playbook forbids those commands explicitly. git push is not injected — the agent can never write to the remote, and the push is performed by the safe_outputs job from a bundle of the agent's local commits.

Validation

• Compiled with the pinned gh-aw compiler version, --strict, clean.
• Lock diff reviewed: no action pin changes; contents: write is added only to the safe_outputs and conclusion jobs, the agent job stays least-privilege.
• Verified git bundle of the incremental commit range works from the shallow agent checkout.

Residual risks

• Azure DevOps rebuilds on the pushed commit; the marker guard bounds it to one automated attempt.
• Fix commits land on the PR branch as verified bot commits and still require human review — the agent is required to say so in its summary comment.

[Copilot]

Pull request overview

Enables automatic build-failure analysis to commit mechanical fixes when affected files are outside the PR diff.

Changes:

• Adds guarded PR-branch push support.
• Checks out the PR head and restores trusted agent configuration.
• Extends the analyst playbook and regenerates workflow locks.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
.github/workflows/shared/build-failure-analysis-shared.md	Documents conditional fix pushing.
.github/workflows/shared/build-failure-analysis-fetch.md	Resolves the appropriate PR checkout ref.
.github/workflows/build-failure-analysis.md	Configures checkout, restoration, Git tools, and push output.
.github/workflows/build-failure-analysis.lock.yml	Regenerates the automatic workflow.
.github/workflows/build-failure-analysis-command.lock.yml	Regenerates shared fetch behavior.
.github/agents/build-failure-analyst.agent.md	Adds mechanical-fix commit instructions and guardrails.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

.github/agents/build-failure-analyst.agent.md:205

• The loop guard must explicitly paginate the PR commits. The get_commits operation is paginated, so checking only its default first page can miss the newest [build-failure-analysis] commit on a longer PR and allow another fail → push cycle. Require fetching every page (using the maximum page size and continuing until exhausted) before concluding that the marker is absent.

6. **Loop guard.** List the PR's commits **with the GitHub tools** (the `pull_requests` toolset) — not `git log`: the workspace is a shallow, depth-1 checkout and does not contain the branch's history. If any commit on the branch already carries the marker `[build-failure-analysis]` in its message, do **not** push again: a previous run already attempted a fix and the build still failed, which means the automated fix is not converging and a human must take over. Say exactly that in the summary comment instead.

.github/workflows/build-failure-analysis.md:119

• The generated safe_outputs job reuses this checkout expression, but that job only declares activation, agent, and detection in needs; it does not directly need fetch-binlog. GitHub Actions therefore cannot resolve needs.fetch-binlog.outputs.pr-checkout-ref there (see the generated lock around lines 2233 and 2312), so the write-capable checkout falls back to the event/default ref instead of the PR branch and the new push path cannot reliably apply its bundle. Ensure fetch-binlog is a direct dependency of safe_outputs (or relay this output through one of its direct dependencies), then regenerate the lock file.

  - ref: ${{ needs.fetch-binlog.outputs.pr-checkout-ref }}

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (4)

.github/agents/build-failure-analyst.agent.md:223

• Step 5 already tells the agent to post exactly one summary before reaching Step 6b, but this instruction asks it to post the summary again and claim the commit was pushed. That either creates duplicate summaries or leaves the first one unable to mention the push; moreover, the push safe output runs in a later job and may still fail. Defer the single add_comment request until after deciding whether to queue a push, and describe the push as requested/pending rather than completed.

5. Post the Step 5 summary comment **as well**, stating near the top that a fix commit was pushed to the branch and still requires human review.

.github/workflows/shared/build-failure-analysis-fetch.md:313

• This loop check fails open. The fetch script has disabled pipefail and suppresses gh errors, so a transient API or pagination failure makes grep return no marker and leaves PUSH_BLOCKED=false; the next failed rebuild can then push again. Default to blocked and clear it only after the complete commit list was fetched successfully.

          if gh api "repos/${GH_AW_REPO}/pulls/${PR_NUMBER}/commits" --paginate \
               --jq '.[].commit.message' 2>/dev/null | grep -qF '[build-failure-analysis]'; then

.github/workflows/build-failure-analysis.md:213

• The refusing hook is not visible where git commit runs. The compiled AWF command mounts only ${RUNNER_TEMP}/gh-aw plus the workspace (build-failure-analysis.lock.yml:990-991), while this hook lives in the sibling ${RUNNER_TEMP}/gh-aw-refuse-commits; Git therefore sees a nonexistent hooks directory and proceeds. Even if mounted, the allowed git commit:* permits --no-verify. Enforce the block in the trusted safe_outputs path before processing a push rather than with an agent-side pre-commit hook.

        HOOKS_DIR="${RUNNER_TEMP}/gh-aw-refuse-commits"

.github/agents/build-failure-analyst.agent.md:219

• The marker that activates the next-run guard is itself only an agent instruction. If the model omits or misspells it, the fetch job never recognizes the automated commit, so every Azure rebuild can make another attempt despite the claimed deterministic limit. Trusted workflow/handler code must append or validate the marker before allowing the push.

This issue also appears on line 223 of the same file.

   [build-failure-analysis]

[YuliiaKovalova]

Addressed the latest review round (including the suppressed comments, which contained the most important findings).

The loop guard did not actually work, so it was rebuilt in trusted code.
The pre-commit hook lived in ${RUNNER_TEMP}/gh-aw-refuse-commits, but the agent firewall only mounts ${RUNNER_TEMP}/gh-aw and the workspace — git saw a nonexistent core.hooksPath and would have committed anyway, and even mounted, the allowed git commit:* permits --no-verify. Anything installed inside the agent sandbox is a suggestion, not a guarantee. The hook is gone. Instead:

• The fetch job publishes push-blocked and the workflow's job-level if: consumes it. When it is true, the activation and agent jobs never run, and gh-aw's safe_outputs job — conditioned on the agent not being skipped — is skipped with them, so no push code path remains. The /analyze-build-failure command workflow is comment-only and ignores the output.
• The [build-failure-analysis] marker is now stamped by commit-title-suffix, i.e. by gh-aw's push handler while it applies the patch. The guard no longer depends on the model writing (or correctly spelling) its own marker, and the playbook explicitly tells the agent not to add one.

The check fails closed now. It previously ran gh api … 2>/dev/null | grep -q, so a transient API error silently produced "not blocked". It defaults to blocked and clears only after the tip commit was read successfully. It also resolves the tip from the PR's head.sha instead of the ambient HEAD_SHA, which can hold the check run's merge commit — that alone would have made the guard never fire. Fork PRs are exempt, since gh-aw refuses to push to them and the guard must not suppress their comment-only analysis.

Scoped to the branch tip, not the whole history, so one automated attempt does not disable analysis on that PR forever: any later commit by anyone restores it.

Real bug fixed. Two echos had been joined onto one line in the fetch job's outputs block, which would have written a malformed $GITHUB_OUTPUT entry and lost ado-build-id entirely. Good catch.

No duplicate summary comment. Step 5 posts exactly one; it is now posted after the push is requested and describes the commit as requested and pending rather than completed, since the push runs in a later job and can still be rejected.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

[YuliiaKovalova]

Round 4 of review feedback, applied to all three repos (arcade #17353, testfx #10607, sdk #55807).

1. The push target was model-controlled. With target: "*", gh-aw's handler resolves the pull request number supplied in the agent's own tool call and only then checks whether that pull request is a fork — so an injection could have aimed the push at an unrelated same-repo PR, escaping both the fork boundary and the loop guard. The target is now bound to check_run.pull_requests[0].number, straight from GitHub's webhook payload.

Because GitHub leaves that field empty for fork-originated check runs, this removes the push target entirely on fork PRs while leaving the comment-only analysis untouched. Verified against live check runs in all three repos: same-repo PRs report exactly one entry, fork PRs report none.

2. commit-title-suffix was a silent no-op. patch-format defaults to bundle, and the handler only rewrites commit titles on the git am path. The [build-failure-analysis] marker would never have been stamped, so the loop guard added in round 3 would never have fired. Pinned to patch-format: am, verified as "patch_format":"am" in the regenerated locks.

3. The agent-config restore used the wrong branch. It checked out github.event.repository.default_branch, so a release-branch PR was analysed with main's playbook and agent instructions. It now uses the PR's own base ref, which the fetch job already resolves from the GitHub API, falling back to the default branch if that lookup returns nothing.

4. Target/guard consistency. The fetch job prefers the ADO build's source branch when resolving the PR number. If that ever disagreed with the check payload, the guard and the push would be scoped to different branches, so the run is now refused outright in that case.

5. Reworded a comment that claimed a deliberately non-fatal diagnostic listing would "fail loudly".

All locks recompiled --strict with 0 errors and 0 warnings — including the target: "*" warning gh-aw used to emit, which the bound target removes.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Note

This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.

[YuliiaKovalova]

Round 5. One finding this time (arcade, suppressed): the loop guard matched [build-failure-analysis] anywhere in the tip commit subject, which can false-positive on an unrelated commit that quotes the marker. The suggested fix was to anchor the match to the end of the subject.

Tightened, but deliberately not anchored — anchoring would have introduced a fail-open bug.

gh-aw appends the suffix by rewriting the first Subject: line of a git format-patch mbox, and git folds subjects longer than ~72 characters onto continuation lines. git am then reassembles the title with the marker in the middle. Verified end to end, using real git format-patch output rewritten with the handler's own regex:

SUBJECT: Fix CS1503 after [build-failure-analysis] Microsoft.DotNet.ProductConstructionService.Client bump changed GetBuildAsync signature at both call sites
ends with marker? False
contains " [marker]"? True

So an end-of-subject anchor would silently miss exactly the commits the guard exists to catch, and the fail → push → rebuild → fail loop would run unbounded. The asymmetry matters: a false positive skips one analysis (and any later commit on the branch restores it), while a false negative removes the brake entirely.

What did change:

• The match now requires the leading space the handler always inserts, so a subject that merely opens with the marker is no longer treated as an automated fix. That removes the plausible false positive without giving up the substring semantics.
• The reasoning is recorded next to the check, so it is not "simplified" into an anchored match later.
• The analyst playbook now asks for commit titles of 60 characters or fewer, so the marker lands at the end in practice and the title stays readable.

Also confirmed there is nothing outstanding elsewhere: the two sdk threads (git checkout/git merge in the generated lock, and GH_AW_CI_TRIGGER_TOKEN) were addressed in earlier rounds and are resolved, and testfx has no comments newer than my round-4 replies.

All locks recompiled --strict: 0 errors, 0 warnings. testfx pin audit passes (2181 references across 47 files).

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Note

This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Note

This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Note

This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (3)

.github/workflows/shared/build-failure-analysis-shared.md:76

• This condition contradicts the tip-scoped loop guard described in Step 6b and the PR: after any human commit, an earlier automated commit remains in history, so this wording can make the analyst refuse all future pushes even though the trusted guard intentionally re-enables them. Refer to the trusted guard allowing the current run, not to absence of a marker anywhere on the branch.

     to the PR branch. Every one of its conditions must hold (same-repo PR,
     `src/` or `test/` only, provable from the compiler error, no earlier
     `[build-failure-analysis]` commit on the branch); otherwise just describe

.github/agents/build-failure-analyst.agent.md:237

• The summarized condition “no previous automated attempt” conflicts with Step 6b item 6, which permits another attempt once a later human commit becomes the branch tip. Since this file is the analyst's instruction set, the contradiction can permanently disable the escape hatch after its first use. Summarize the actual tip-scoped trusted guard instead.

- Do not propose fixes to files outside the PR diff in scan mode unless you are extremely confident — those changes are usually load-bearing across other projects. Prefer to explain the root cause in the comment and let a human apply the fix. The single exception is Step 6b, whose conditions (mechanical fix, provable from the compiler error, `src/` or `test/` only, same-repo PR, no previous automated attempt) exist precisely to keep that confidence bar high.

.github/workflows/build-failure-analysis.md:117

• This describes a full commit-list check, but the trusted guard reads only the current tip subject (PR_TIP_SHA) and Step 6b explicitly tells the agent not to inspect history. Because this workflow body is runtime-imported into the prompt, the inaccurate comment can reinforce the conflicting “no earlier commit” instruction and prevent the intended re-enable after a human commit. Describe the tip-only API check instead.

# The PR-head checkout is intentionally shallow (`actions/checkout`'s default
# depth of 1): gh-aw bundles only the commits the agent creates on top of it, so
# no history is needed. Step 6b's loop guard therefore reads the PR's commit
# list through the GitHub tools rather than `git log`, which cannot see the
# branch's history here.

[YuliiaKovalova]

Round 6 — all six suppressed findings triaged; five applied, one respectfully disputed with evidence.

Applied

1. Tip-scoped guard vs. "no earlier commit on the branch" (3 findings on Let the build-failure analyst push fixes it cannot suggest inline #10607, same root cause). Correct and the most serious of the batch. The guard reads only PR_JSON.head.sha's subject, so a human commit on top legitimately re-enables the escape hatch — but the shared playbook, the analyst instructions and a workflow comment all still said "no earlier [build-failure-analysis] commit on the branch" / "no previous automated attempt". Since those files are injected into the prompt, that phrasing could have taught the analyst to refuse every push after the first, permanently disabling the feature. All three now describe the tip-only check and tell the agent it does not need to check history at all.

2. Token comment (#55807). Correct. The push runs with secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN — verified in the generated lock — so a repo that sets GH_AW_GITHUB_TOKEN to a PAT/App token does get Actions re-runs. The comment now states both cases and notes this is a further reason the guard lives in a trusted job.

3. git log:* allowlist entry (#17353). Correct — dropped. The checkout is depth-1 so it could only ever show the tip, and the playbook already forbade using it to reconstruct the guard.

Disputed — the 60-character commit-title guidance

The suggestion was to lower the limit to ~47 because the ~25-char suffix is appended. That would be right if the suffix were part of the commit before git format-patch ran, but it is not. From actions/setup/js/push_to_pull_request_branch.cjs at the pinned v0.77.5, the suffix is applied to the already-generated patch text:

patchContent.replace(/^Subject: (?:\[PATCH\] )?(.*)$/gm,
  (match, title) => `Subject: [PATCH] ${title}${commitTitleSuffix}`);

So folding is decided by format-patch on the bare title; the suffix never competes for that budget. Measured threshold, not estimated:

title length	Subject: line	folded?
60	77	no
61	78	no
62	—	yes

61 is the exact limit (Subject: [PATCH] = 17 chars of overhead against a 78-column header), so 60 is correct with one character of margin; 47 would have cost usable title space for no benefit.

That said, the original sentence was ambiguous enough to invite exactly this reading, so I rewrote it to say explicitly that the budget is for the title alone, that the marker is appended afterwards to the generated patch, and that the real constraint is the Subject: [PATCH] prefix folding at 62.

Note this is also why the round-5 guard match is deliberately unanchored: if a title does fold, git am reassembles it with the marker mid-string, so anchoring to end-of-subject would fail open.

All three repos recompiled with their pinned compilers — 0 errors, 0 warnings. testfx pin audit passes (2274 refs / 49 files).

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

.github/workflows/build-failure-analysis.md:433

• This allowlist makes the escape hatch reject the dependency-flow PRs it is intended to fix. In the pinned gh-aw v0.84.3 implementation, configuring allowed-files triggers a preflight over every file in origin/<base>..<local tip>, not just the analyst's incremental commit. A dependency-flow branch already changes files such as eng/Version.Details.xml outside src//test/, so push_to_pull_request_branch returns an error before generating the patch. Please use a gh-aw version/configuration that applies this policy only to the incremental commit (while retaining apply-time enforcement), then regenerate the locks.

    allowed-files:
      - "src/**"
      - "test/**"

[Jan Krivanek (JanKrivanek)]

Looks good as is.

If there are any future concerns with agent pushing directly into PRs - lets flip it to create PR targetting the underlying PR