fix(review): fail loudly when the context script is missing

[zfarrell]

Two follow-ups to #29.

The test -f precondition asked for in review, now that main has the script so it no longer deadlocks. It sits in a step without continue-on-error, matching the protection the prompt document already gets from cat.

Also fixes a regression from #29: the guard was ${PR_NUMBER:?}, which fires on empty as well as unset. tests.yml calls the workflow on push: branches: [main] where there is no pull request, so every main-push smoke run exited on line 26 and covered nothing past it — green, because the step is continue-on-error. Caught it in the merge run's log.

[claude]

Both changes are correct. The ? vs :? distinction is the right fix — env: PR_NUMBER: ${{ github.event.pull_request.number }} always sets the variable, so only the empty case was ever reachable, and the main-push smoke run was exiting on line 26 exactly as described. The new test catches a regression to :? on both the exit code and the stderr message.

The precondition step sits in a step without continue-on-error, which is what makes it worth anything. Three non-blocking notes inline, the main one being that the lint assertion for it doesn't actually verify that placement.

[claude]

nit: the "tests for the script" half still isn't what passes here (not blocking).

Against the current step, the first alternative never matches — the -f test is written [ ! -f "$script" ], so no line contains both -f and gather-review-context.sh. The only alternative that fires is script=.*gather-review-context\.sh, which matches the assignment on its own. So deleting the whole if [ ! -f "$script" ] ... fi block, while keeping script=.github-workflows/scripts/gather-review-context.sh and the bash "$script" that reads it, leaves this green — the same shape as the file-wide grep, one scope smaller.

Same reason the variable-name coupling isn't quite gone either: SCRIPT= or ctx_path= breaks the assertion even with the check intact. (context_script= survives only because the pattern is unanchored.)

Requiring both halves independently rather than either-of-two would match the claim:

elif ! printf '%s\n' "$precondition_step" | grep -q 'gather-review-context\.sh' \
  || ! printf '%s\n' "$precondition_step" | grep -q -- '-f'; then

[claude]

super nit: the extraction runs past the step into the next step's comment block (not blocking).

found && /^ - / exits on the next step, but the comments that precede it are indented six spaces too, so they land inside $precondition_step — right now that's the blank line plus the ten-line "Frontloads what a week of tool-usage artifacts showed…" block above Gather review context.

That block happens not to contain the string continue-on-error, which is the only reason the third branch stays green. Given how much of this file's commentary is about continue-on-error (the context step's own comment says it five times, forty lines down), a future comment written in that gap fails the lint with a message that names the wrong cause: "the precondition step is continue-on-error". Exiting on a comment as well as a step boundary keeps the block to the step:

                         found && /^      (- |#)/ { exit }