refactor(review): extract the context step script

[zfarrell]

The Gather review context run: block was 20,545 characters against Actions' 21,000-character expression limit — about six comment lines of headroom, and crossing it stops every review in the org. It now lives in scripts/gather-review-context.sh, checked out alongside the prompt document, where Actions never parses it. Largest remaining block is 3,272.

PR_NUMBER and REPO move to env: with the other values, so nothing is interpolated into the script. The tests run the real file instead of scraping it back out of the workflow.

Note: the cross-repo checkout reads ref: main, so on this PR the script is not on main yet and the smoke job's context step will fail into its guarded fallback. It resolves on merge.

[claude]

Review

Blocking Issues

• .github/workflows/claude-pr-review.yml:151 — the newly extracted script is consumed by a continue-on-error: true step, so a checkout that does not deliver it exits 127 and the run stays green. The review step (line 177) gates only on the author login and dry_run, never on steps.context.outcome, so the review proceeds with pr_context, threads and review_cycle all unset. A blank REVIEW CYCLE: plus an empty prior-comments block reads as cycle 1 with nothing raised before — the false claim the script's own guards (lines 92-96, 137-139) exist to prevent — and it lands in every consumer repo at once.

• The prompt document, checked out by the same sparse-checkout, does not have this problem: Load review prompt (line 105) has no continue-on-error, so cat on a missing file fails the job loudly. That asymmetry is what tests.yml:91-93 rests on when it says a broken sparse-checkout "surfaces here rather than in the org." After this change that holds for only one of the two paths.

• The smoke job reporting SUCCESS on this PR while its context step could not have found the script is the demonstration, not only the merge-order artifact the description describes: the same green run is what a typo in the new sparse-checkout pattern, a rename, or a path that stops matching in non-cone mode would produce.

Action Required

Add a loud precondition for the script after the cross-repo checkout. Folding test -f .github-workflows/scripts/gather-review-context.sh into the existing Verify jq is available step (line 114) is the smallest version and restores parity with the prompt file. If failing the review job on a bad checkout is deliberately unwanted, the alternative is to keep the step green and render a "the context script could not be found" sentence into the context, so the model is told rather than handed an empty block.

One super nit is inline on scripts/gather-review-context.sh and is not blocking.

The extraction itself reads as a faithful move: the script body is unchanged, set -eo pipefail correctly replaces what shell: bash supplied, PR_NUMBER and REPO via env: removes the last interpolation, and the test changes — running the real file, extract_jq over both sources, the permission table reading the script — are improvements over scraping the run block. I did not run the suite; Tests / test had not reported when this review started.

[zfarrell]

Fixed the dangerous half in fe02bbf. The review step now requires steps.context.outcome == 'success', so an empty context can no longer be presented to the model as a clean cycle 1, and the notify step gained a steps.context.outcome == 'failure' clause so that path is not the silent one. workflow-lint-test.sh asserts the gate; it fails on the pre-fix condition.

Not taking the test -f precondition in this PR, only because it deadlocks here: the cross-repo checkout reads ref: main, main does not have the script until this merges, so the precondition would fail this PR's own smoke job and block the merge that fixes it. It lands in the follow-up, where main has the file.

On the third point — you are right that the green smoke run is the demonstration and not just a merge-order artifact. I verified the sparse pattern separately rather than trusting that run: a --no-cone checkout of this branch with those two patterns delivers exactly docs/claude-pr-review-prompt.md and scripts/gather-review-context.sh. So the pattern is correct; what CI cannot yet see is a future break of it, which is what the precondition is for.

[claude]

Prior blocking finding is resolved: a context failure no longer feeds the model an empty context that reads as a clean cycle 1, and the notify clause keeps that path from being the silent one. steps.context.outcome (not conclusion) is the correct property under continue-on-error, and the !inputs.dry_run on the notify step is necessary — without it a smoke run with a failed context would comment on the real PR.

Two nits inline, both non-blocking: comments in the context step and in tests.yml that this commit inverted, and the smoke job still not seeing a sparse-checkout pattern that stops matching (which cannot be fixed in this PR, since ref: main has no script yet).