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.

[azure-pipelines]

Azure Pipelines:
Successfully started running 2 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR updates the automatic build-failure-analysis GitHub Actions workflow (gh-aw based) so the build-failure analyst can append a mechanical fix commit to the PR branch in the specific case where GitHub inline suggestion blocks are structurally impossible (because the fix is in files outside the PR diff, e.g. dependency-flow PRs). It also adds checkout/prompt-injection guardrails so the agent authors commits against the PR head while still loading agent configuration from the base branch.

Changes:

• Enable push-to-pull-request-branch as a safe-output tool in the automatic workflow, with src/** + test/** allowlisting and loop-guard guidance.
• Add PR-head checkout resolution (pr-checkout-ref) so any authored fix commit applies to the PR’s actual tree (branch-attached for same-repo PRs, detached for forks).
• Restore .github/ + .agents/ from base branch before the agent runs to mitigate PR-controlled instruction/prompt injection.

Reviewed changes

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

Show a summary per file

File	Description
.github/workflows/shared/build-failure-analysis-shared.md	Updates shared analyst instructions to describe the push escape hatch and when it is allowed.
.github/workflows/shared/build-failure-analysis-fetch.md	Adds pr-checkout-ref output and logic to select a safe checkout ref for PR-head analysis.
.github/workflows/build-failure-analysis.md	Enables push safe-output and introduces PR-head checkout + base-branch config restore guardrail.
.github/workflows/build-failure-analysis.lock.yml	Regenerated lock workflow reflecting new safe-output tool, permissions, checkouts, and handler wiring.
.github/workflows/build-failure-analysis-command.lock.yml	Regenerated lock workflow to align with updated shared fetch outputs (still comment-only).
.github/agents/build-failure-analyst.agent.md	Extends the analyst playbook with Step 6b for pushing a mechanical fix commit under strict conditions.

[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.

[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.

[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.