fix(review): skip the review when context gathering fails

Author: zfarrell

.github/workflows/claude-pr-review.yml

@@ -168,13 +168,24 @@ jobs:
       #
       #   Reduce execution log  -- needs steps.review.outputs.execution_file, empty when skipped
       #   Upload tool usage     -- needs steps.tool-usage.outcome == 'success', which is 'skipped'
-      #   Notify on failure     -- needs outcome 'failure' or 'cancelled', and this is 'skipped'
+      #   Notify on failure     -- carries its own !inputs.dry_run, because it can now also fire
+      #                            on a context failure rather than only on a review failure
       #
       # So a smoke run posts no review, no comment and no artifact. Everything before this step
       # still runs against the live API: the app token, the cross-repo prompt checkout, and the
       # nine context reads with the job's real permissions.
+      #
+      # steps.context.outcome, because the context step is continue-on-error and therefore fails
+      # green. Anything that stops the script running -- a checkout that does not deliver it, a
+      # renamed path, a sparse pattern that stops matching -- leaves pr_context, threads and
+      # review_cycle unset, and an empty context is not a neutral one: a blank REVIEW CYCLE and an
+      # empty prior-comments block read as cycle 1 with nothing raised before. That is a false
+      # statement to the model, of exactly the kind the script's own guards exist to prevent, and
+      # it would reach every consumer repo at once. No context, no review.
       - uses: anthropics/claude-code-action@v1
-        if: github.event.pull_request.user.login != 'dependabot[bot]' && !inputs.dry_run
+        if: >-
+          github.event.pull_request.user.login != 'dependabot[bot]' && !inputs.dry_run
+          && steps.context.outcome == 'success'
         id: review
         continue-on-error: true
         with:
@@ -267,8 +278,15 @@ jobs:
           # (~100 review runs/week org-wide); the question is days-old, not quarters.
           retention-days: 14
 
+      # Also on a context failure, which now skips the review rather than feeding it an empty
+      # context. Without this clause that path is the silent one: no review, no comment, green
+      # check -- the shape of failure this workflow keeps being bitten by. !inputs.dry_run because
+      # a smoke run reaches this step with the review skipped and must never write.
       - name: Notify on review failure
-        if: github.event.pull_request.user.login != 'dependabot[bot]' && (steps.review.outcome == 'failure' || steps.review.outcome == 'cancelled')
-        run: gh pr comment ${{ github.event.pull_request.number }} --body "Automated review unavailable (Claude step failed). Please review manually."
+        if: >-
+          github.event.pull_request.user.login != 'dependabot[bot]' && !inputs.dry_run
+          && (steps.review.outcome == 'failure' || steps.review.outcome == 'cancelled'
+              || steps.context.outcome == 'failure')
+        run: gh pr comment ${{ github.event.pull_request.number }} --body "Automated review unavailable (the review step failed, or the context it needs could not be gathered). Please review manually."
         env:
           GH_TOKEN: ${{ github.token }}

tests/workflow-lint-test.sh

@@ -162,6 +162,31 @@ check_permission "statusCheckRollup" statuses "the StatusContext half of the CI
 check_permission "/compare/" contents "the since-last-review comparison"
 check_permission "/pulls/" pull-requests "the PR reads"
 
+# The context step is continue-on-error, so everything it can fail at -- a checkout that does not
+# deliver the script, a bad path, a rename that stops matching the sparse pattern -- leaves the run
+# green with pr_context, threads and review_cycle all unset. Ungated, the review step then runs on
+# that: a blank REVIEW CYCLE and an empty prior-comments block read as cycle 1 with nothing raised
+# before, which is a false statement rather than a missing one, and it reaches every consumer repo
+# at once. The script's own guards exist to stop exactly that claim, and they cannot help if the
+# script never ran. So the review must be gated on the context step having succeeded.
+# The condition is a folded block, so collect its continuation lines too: everything indented
+# past the `if:` key, up to the next key of the step.
+review_gate=$(awk '/^      - uses: anthropics\/claude-code-action/ { found = 1; next }
+                   found && /^        if:/ { print; in_if = 1; next }
+                   in_if && /^          / { print; next }
+                   in_if { exit }' "$WORKFLOW_FILE")
+if [ -z "$review_gate" ]; then
+  echo "FAIL could not find the review step's if: in $WORKFLOW_FILE; this check proves nothing"
+  failures=$((failures + 1))
+elif ! printf '%s\n' "$review_gate" | grep -q "steps\.context\.outcome == 'success'"; then
+  echo "FAIL the review step does not require the context step to have succeeded, so a failed"
+  echo "     context read sends the model an empty context that reads as a clean cycle 1:"
+  printf '%s\n' "$review_gate" | sed 's/^/       /'
+  failures=$((failures + 1))
+else
+  echo "ok   the review step runs only when the context step succeeded"
+fi
+
 # The table above forces a new API call in the context step to declare its permission on the
 # review job. That does nothing for the smoke job in tests.yml, which calls the review workflow
 # and has to grant the same set by hand: a caller cannot give a reusable workflow more than it