Deferred from #567 (which closes #543), and named there as deferred rather than dropped.
What is inline today
#567 adds a "Save the PR diff for the reviewer" step to claude-code-review.yml: about a dozen lines of shell that run gh pr diff into the workspace, emit a path output, and on any failure remove the partial file, leave path empty, and warn rather than failing the job.
Its behaviour was verified against a stub gh across four shapes — success, empty output, failure, and the important one, writes bytes then fails (the partial file must be removed, or a truncated diff gets read as a complete one). But that verification was a one-off local run. Nothing in _selftest.yml guards it, so a later edit that drops the rm -f, or turns the warning into a hard failure, goes unnoticed.
That is the gap this repo normally closes by extracting the logic to a script with a stub-driven test — the trigger-bugbot-review, classify-push-failure, and check-review-execution pattern.
Why #567 did not do it
A reusable workflow cannot reference this repo's own scripts/ by path. actions/checkout inside claude-code-review.yml produces the consumer's checkout, not gha's, so the script would not be there — the same failure gha#191 hit and gha#196 recorded. The established fix is a composite action, whose files are reachable through github.action_path regardless of checkout state.
But a new composite action cannot gain its first @v2 caller in the PR that introduces it: a uses: ref resolves at job-preparation time, so Morrison-Lab/gha/.github/actions/<new>@v2 fails the whole job until the tag advances past the merge. claude-code-review.yml is dogfooded on every PR here through claude-review.yml, so that would be a red check on every PR with no way to fix it inside the PR — exactly the split gha#326/#327 records.
So: land the step, then extract it once the tag has moved.
What this issue is
- Extract the save logic to
.github/workflows/scripts/save-pr-diff.sh.
- Wrap it in a composite action so
claude-code-review.yml can reach it.
- Add
run-save-pr-diff-tests.sh with a stub gh covering the four shapes above, plus a real uses: call in _selftest.yml for the github.action_path-resolution proof the other composites get.
- Mutation-check it: dropping the
rm -f must turn the partial-file case red, and removing the emptiness guard must turn the empty-output case red. A suite that passes under both is not testing anything.
Not urgent — the step's failure mode is "the reviewer falls back to the behaviour it has today", so a regression here degrades rather than breaks. Worth doing before anyone edits that step for a second reason.
Deferred from #567 (which closes #543), and named there as deferred rather than dropped.
What is inline today
#567 adds a "Save the PR diff for the reviewer" step to
claude-code-review.yml: about a dozen lines of shell that rungh pr diffinto the workspace, emit apathoutput, and on any failure remove the partial file, leavepathempty, and warn rather than failing the job.Its behaviour was verified against a stub
ghacross four shapes — success, empty output, failure, and the important one, writes bytes then fails (the partial file must be removed, or a truncated diff gets read as a complete one). But that verification was a one-off local run. Nothing in_selftest.ymlguards it, so a later edit that drops therm -f, or turns the warning into a hard failure, goes unnoticed.That is the gap this repo normally closes by extracting the logic to a script with a stub-driven test — the
trigger-bugbot-review,classify-push-failure, andcheck-review-executionpattern.Why #567 did not do it
A reusable workflow cannot reference this repo's own
scripts/by path.actions/checkoutinsideclaude-code-review.ymlproduces the consumer's checkout, not gha's, so the script would not be there — the same failure gha#191 hit and gha#196 recorded. The established fix is a composite action, whose files are reachable throughgithub.action_pathregardless of checkout state.But a new composite action cannot gain its first
@v2caller in the PR that introduces it: auses:ref resolves at job-preparation time, soMorrison-Lab/gha/.github/actions/<new>@v2fails the whole job until the tag advances past the merge.claude-code-review.ymlis dogfooded on every PR here throughclaude-review.yml, so that would be a red check on every PR with no way to fix it inside the PR — exactly the split gha#326/#327 records.So: land the step, then extract it once the tag has moved.
What this issue is
.github/workflows/scripts/save-pr-diff.sh.claude-code-review.ymlcan reach it.run-save-pr-diff-tests.shwith a stubghcovering the four shapes above, plus a realuses:call in_selftest.ymlfor thegithub.action_path-resolution proof the other composites get.rm -fmust turn the partial-file case red, and removing the emptiness guard must turn the empty-output case red. A suite that passes under both is not testing anything.Not urgent — the step's failure mode is "the reviewer falls back to the behaviour it has today", so a regression here degrades rather than breaks. Worth doing before anyone edits that step for a second reason.