fix(workflows): resolve PR number from aw_context in /review reviewers - #1779
Merged
Conversation
The `/review` fan-out dispatched all five reviewers correctly, but every one
of them silently reviewed nothing.
Under `slash_command.strategy: centralized`, the generated
`agentic_commands.yml` router dispatches each reviewer via `workflow_dispatch`,
so the payload carries neither `github.event.pull_request.number` nor
`github.event.issue.number` — the PR identity arrives only in the `aw_context`
input as `{item_type: "pull_request", item_number: "<N>"}`.
As a result, in `shared/pr-diff-data-fetch.md`:
* `PR_NUMBER` resolved empty, so the step wrote empty pre-fetch files and
exited 0. The agent then reported a successful run with "No review
performed: pre-fetched PR data files were empty".
* The cache key collapsed to the constant `pr-prefetch-`, so all five
reviewers restored the same 349-byte poisoned entry.
Fixes:
* `PR_NUMBER` gains an `aw_context` fallback, guarded by
`item_type == 'pull_request'` as required by gh-aw ADR-31820 —
`item_number` is shared across entity kinds, so an issue-routed run must
not populate the PR number slot. The raw `fromJSON(...)` form is used
because `github.aw.context.*` is a prompt-only virtual namespace and is
not transformed inside step `env:`.
* Cache keys are re-scoped to `pr-prefetch-<pr-number>-<sha>` so the
`restore-keys` prefix still matches when a dispatch run knows the PR
number but not the head SHA. `pr-data-head-sha.txt` continues to reject
stale entries.
* The unresolved-PR path now fails loudly instead of reporting a successful
empty review. That fail-open behaviour is what hid this.
* Reviewer prompt bodies now defer to `pull-request-number` in gh-aw's
builtin `<github-context>` block, which already carries the `aw_context`
fallback, rather than hand-rolling the expression.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9cc16936-10fe-4069-9a37-0d2d1335b6e5
|
Azure Pipelines: 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/reviewwas routing correctly — all five reviewers dispatched on #1670. The bug was downstream: they ran, reported success, and reviewed nothing.Under
slash_command.strategy: centralized, the generatedagentic_commands.ymlrouter dispatches each reviewer viaworkflow_dispatch, so the payload carries neithergithub.event.pull_request.numbernorgithub.event.issue.number. The PR identity arrives only in theaw_contextinput as{item_type: "pull_request", item_number: "<N>"}.In
shared/pr-diff-data-fetch.mdthat meant:PR_NUMBERresolved empty → the step wrote empty pre-fetch files and exited 0. The agent then posted a successful run: "No review performed: pre-fetched PR data files were empty".pr-prefetch-, so all five reviewers restored the same poisoned 349-byte entry.Evidence from run 30698000006 (and the four sibling runs):
Changes
PR_NUMBERgains anaw_contextfallback, guarded byitem_type == 'pull_request'as required by gh-aw ADR-31820 —item_numberis shared across entity kinds, so an issue-routed run must not leak its number into the PR slot.pr-prefetch-<pr-number>-<sha>so therestore-keysprefix still matches when a dispatch run knows the PR number but not the head SHA.pr-data-head-sha.txtcontinues to reject stale entries, which is what makes a prefix-matched restore safe.<github-context>block. gh-aw already injectspull-request-numberwith theaw_contextfallback baked in, so hand-rolling the expression was redundant.Note on
github.aw.context.*Worth recording, since it shaped the fix:
github.aw.context.item_numberis a prompt-only virtual namespace. Verified by compiling all three variants:env:3 unauthorized expressions found<github-context>GH_AW_EXPR_463A214Awith the full fallbackHence the raw
fromJSON(...)form in stepenv:, and deferring to the builtin block in the body.Test plan
gh aw compile— 27 workflows, 0 errors; the 4 warnings are pre-existing and in unrelated workflows (bash-lint-auditor,clippy-fixer,test-reducer,frontmatter-aligner).gh aw validate— all five reviewers pass.agentic_commands.ymlis unchanged — routing was never the problem.pull-request-numberstill reaches the prompt (GH_AW_EXPR_463A214A).Still needs a live check: comment
/reviewon this PR once merged (or on a scratch PR) to confirm the reviewers pick up a real diff end to end. The empty-diff path now hard-fails, so a regression will be loud rather than silent.