Constrain what may follow review, and fix two runner-portability gaps

[dem-extra1]

Closes #346. Post-merge follow-ups to #341.

1. The tail was left open

#341 constrained only what may precede the keyword. The other end stayed
open, so these matched:

@claude can you review this and also fix the failing test?
@claude please review my reasoning in the issue description above
@claude, can you review why the coverage job is flaky and patch it?
@claude could you review the docs and update them if wrong

All four were false under the old @claude[[:space:]]+review pattern.
A match suppresses claude.yml's "Post Claude's response if no code was
committed" step (claude.yml:844), so each of these went to a read-only
reviewer and nobody answered the question -- the exact harm the closed-set
lead-in was chosen to prevent, arriving through the other end of the pattern.

review may now take an object, but only from a second closed set of deictic
references to the PR under discussion (this, the latest changes, again,
trailing politeness), and the request has to end its line. Same idea as
POLITE, applied to both sides of the keyword.

That end-of-line requirement also subsumes the old [^[:alnum:]]|$ whole-word
guard, so @claude reviewer still fails without a separate clause.

The trade is deliberate and in the cheap direction. An unlisted-but-genuine
object -- @claude review the test-coverage changes -- now self-reviews
instead of dispatching. By #341's own asymmetry a false negative costs a
self-review; a false positive costs an unanswered question.

2. CRLF normalization was dead code, and is now load-bearing

I verified the sed 's/\r$//' in #341 changed no outcome on any case: \r was
already covered by [[:space:]] in the separator class and [^[:alnum:]] in
the word-boundary class.

Anchoring the tail on a bare newline makes it matter, because GitHub delivers
comment bodies with CRLF. So rather than deleting it as #346 first suggested,
it becomes tr -d '\r' -- \r in a sed BRE is a GNU extension, and BSD/macOS
sed reads it as a literal r and strips trailing rs instead.

3. base64 -d is not portable either

GNU coreutils spells decode -d; BSD/macOS base64 spells it -D. Unlike
item 2 this is on the live path for the late-comment rescan, and per the
composite's deliberate no-|| true policy a decode failure reddens the calling
job. It now probes once and picks the right flag.

Items 2 and 3 are the same standard #341 already applied when it declined
jq --raw-output0 because runs-on is a consumer-settable input.

4. Two halves of the late-dispatch path disagreed

"Collect comments posted after the trigger" ends || : + touch so a
transient gh api error degrades to "no late review". The step right after it
did the opposite by design, under set -euo pipefail, so an unreadable
bodies-file could redden the whole job over what the workflow's own comment
calls an optional nicety. It is now continue-on-error: true -- still red in
the log, and an empty match leaves the dispatch skipped.

Both steps also log their result now. A false outcome was previously
invisible: the old step's No late @claude review requests. line went away
with the refactor, and gated steps just render as skipped.

Verification

• 40 cases pass, up from 27.
• The four new negatives were confirmed meaningful by reverting the tail
  constraint: exactly those four fail, nothing else.
• The CRLF case was confirmed meaningful by reverting the normalizer to a
  no-op: it fails, nothing else does.
• The composite's shell logic (probe, decode loop, unreadable-file exit) was
  simulated locally against the same payloads _selftest.yml builds, and
  reproduces both of its existing e2e assertions (match=true on the
  bodies-file, match=false on the negative comment), so those steps still
  pass unchanged.
• check-new-line-breaks clean against origin/main; all three touched YAML
  files parse; added lines are ASCII-only.

Not covered

Same bootstrapping gap as #341: claude.yml reaches the composite through
Morrison-Lab/gha/...@v2, which does not resolve until the tag advances past
this merge. _selftest.yml exercises the composite through a local ./ ref,
so nothing goes red in the meantime.

Left for a follow-up

#346's last item -- fenced and inline code are not stripped, though
blockquotes now are, so comment `@claude review` on the PR still matches.
Not a regression (the pre-#341 pattern matched it too), but it needs its own
decision about what counts as a code span and how fenced blocks nest, rather
than riding along here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BWWei35DwEzyAgzfrH1BfK

---

Generated by Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[dem-extra1]

review / claude-review is red here, but the failure predates this branch and reproduces on unrelated PRs. Not fixing it in this diff.

The SDK errored at startup before the model produced anything:

{"type":"result","subtype":"success","is_error":true,"duration_ms":643,
 "num_turns":1,"total_cost_usd":0,"permission_denials_count":0}

643 ms, one turn, $0 spent, zero permission denials. check-review-execution.sh classified it as a hard SDK error rather than the gha#185 stub signature, so no retry was eligible, and the quota-exhaustion skip path did not match either. review / require-review is purely downstream of this.

Every pull_request-triggered run of this workflow has failed since 09:41 UTC today, across two unrelated branches:

run	time (UTC)	branch	result
30347230035	09:35	claude/github-issue-277-4116tm	success
30347632180	09:41	claude/markdown-semantic-line-breaks-0aqhlh	failure
30348361180	09:51	claude/github-issue-277-4116tm	failure
30365233869	13:48	claude/github-issue-277-4116tm	failure
30365565116	13:52	claude/markdown-semantic-line-breaks-0aqhlh	failure
30379509790	16:42	this PR	failure

This diff cannot reach the review path: it touches detect-review-request, claude.yml (the agent workflow, not the reviewer), and docs. claude-code-review.yml, run-claude-review-attempt, and check-review-execution.sh are all unchanged, and the review job restores CLAUDE.md and .claude/ from origin/main before running anyway.

The shape -- clean install, immediate is_error at zero cost, starting mid-morning after a success -- looks like credentials rather than anything in a diff. CLAUDE_CODE_OAUTH_TOKEN is worth checking first; as with WORKFLOW_TOKEN, only someone with repo admin access can rotate it.

Since no automated verdict will be produced while this holds, the substance of the change is covered in the PR description, and the pattern changes are pinned by 38 offline cases with both new guards confirmed to bite when reverted. I will re-check when the workflow recovers.

---

Generated by Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[dem-extra1]

Self-review

The automated reviewer is down (see the comment above), so this is a self-review, per the fallback in d-morrison/ai-config's CLAUDE.md. No external verdict was produced on this PR.

Finding: the prose understated the cost, in all three places it appeared

Fixed in f3c06e6.

The script comment, CLAUDE.md, and the changelog all illustrated the closed tail set's cost with @claude review the test-coverage changes -- a phrasing I invented, and one that reads as marginal enough to dismiss. Measuring the actual regression surface against #341's pattern turns up two natural cases:

phrasing	#341	this PR
@claude review the changes I just pushed	dispatch	self-review
@claude please review when you get a chance	dispatch	self-review

Both are pure review requests carrying no instruction to the agent, which is exactly the population the tail constraint was not meant to touch. the changes I just pushed is the one that stings.

Citing a weaker example than the ones I had measured is the failure mode shared/writing/fact-check-prose.md exists to catch, and I did it three times over. All three now cite the real case, and both phrasings are pinned in the test table (40 cases, up from 38) so recovering them by widening TAIL_WORD stays a deliberate decision rather than a silent drift.

This is the one open judgment call in the PR. when you get a chance is trailing politeness, which the set already admits in principle (please, thanks) -- adding it is principled and safe. the changes I just pushed needs i|just|pushed, which is defensible under the "points back at the PR" rule but starts the slide toward "any few words". I left both out rather than widen unilaterally. Happy to add either on request; it is a one-line change with the tests already in place.

Checks that came back clean

• No ReDoS. TAIL nests + inside *, the classic catastrophic-backtracking shape, and comment bodies are attacker-influenced. Measured at 200 / 2,000 / 20,000 separator characters in both positions: 0.00s, 0.00s, 0.01s. Linear, because each * iteration requires a TAIL_WORD to follow, so the inner + cannot be split combinatorially.
• Both new guards bite. Reverting the tail constraint fails exactly the four intended negatives and nothing else; reverting the CRLF normalizer fails exactly the CRLF case. My first CRLF test case did not bite -- it passed for the wrong reason, via a thanks tail match -- and was replaced before the first push.
• No stale references to the renamed strip_quotes, and no leftover mention of the removed [^[:alnum:]]|$ guard.
• No injection regression. Comment bodies still reach the script only through env: into $VAR, never interpolated into run: text. The with: hop added in Recognize punctuated and polite @claude review requests #341 preserves that.
• fixed is a valid changelog category per changelog.d/README.md; check-new-line-breaks clean; YAML parses.

Considered and deliberately not done

• No new e2e selftest step for the tail constraint. The two uses: steps in review-fail-check exist to prove github.action_path resolution and the base64 round-trip, not pattern coverage -- that is the offline table's job. Adding a third would duplicate the table without proving anything new.
• TAIL_WORD built by three appends rather than an array joined on IFS='|'. The append is dull and obvious; the IFS subshell join is the cleverer idiom, and CLAUDE.md's review priorities prefer the standard, well-known form over the clever one.
• Fenced and inline code still are not stripped (#346's last item), so comment `@claude review` on the PR still matches. Not a regression -- the pre-Recognize punctuated and polite @claude review requests #341 pattern matched it too -- and it needs its own decision about code-span and fence-nesting semantics.

---

Generated by Claude Code

[github-actions]

👀 Picked up by workflow run #30381851377. Setup runs first; Claude itself responds after that.

[dem-extra1]

Correction to my earlier comment on this failure: the cause is known and is not a credentials problem. The bot's account is out until Thursday. Disregard the suggestion to check or rotate CLAUDE_CODE_OAUTH_TOKEN -- rotating it would not help, and I should not have floated a remedy before the cause was established.

Everything else in that comment stands: the immediate is_error:true at ~0.6s and $0, and the failures beginning mid-morning across unrelated branches, are all downstream of the same outage. review / require-review stays red until the account is back, on this PR and on every other open one.

Practical consequence for this PR: no automated verdict is obtainable before Thursday, and re-triggering will not produce one. The documented fallback applies -- the self-review above, plus the rest of CI. I have stopped polling hourly and will re-check on Thursday.

---

Generated by Claude Code

[dem-extra1]

@claude review

[dem-extra1]

@claude review

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[dem-extra1]

Rebased onto #351 (58f287f), which unblocks CI here.

One conflict, in claude.yml: #351 retargeted the late-detect step's uses: while this branch added continue-on-error: to the same step. Resolved by keeping both.

Verified after the rebase: no uses: d-morrison/gha refs remain anywhere in the repo, both touched YAML files parse, and all 40 matcher cases still pass.

Two corrections to the PR description above, both now stale:

• The "Not covered" section names the composite's ref as d-morrison/gha/...@v2; it is Morrison-Lab/gha/...@v2 after Retarget every uses: reference to Morrison-Lab/gha #351. The bootstrapping gap itself still stands -- v2 points at fee46d9, which predates Recognize punctuated and polite @claude review requests #341, so the detect-review-request action does not exist at that tag yet regardless of which org owns it.
• "Verification: 38 cases pass, up from 27" is now 40, after the two known false negatives were pinned.

The failures earlier in this thread were two unrelated outages back to back -- account quota, then the org rename -- neither caused by this diff. #350 tracks the first one, which is a real bug: the quota-exhaustion graceful-skip path cannot be reached, because the step that detects it is gated on the step that quota exhaustion always fails.

---

Generated by Claude Code

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.

_{2026-07-28 19:25 UTC}

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[dem-extra1]

Working on this — paws off until I'm done.

Self-review. Posted because review / claude-review cannot run on this branch: its second attempt came back startup_failure (the uses: d-morrison/gha/...@v2 breakage #351 fixes), so no bot verdict is obtainable here until that merges. Copilot has been requested in parallel.

What I verified rather than read

The full test table passes: 40 cases, up from 27 on main. I ran both.

All eight phrasings the docs promise behave as documented. website/reference/claude.qmd makes specific claims, so I ran each against the script rather than trusting the prose:

Body	Result
@claude review this PR	✅ dispatch
@claude please review the latest changes	✅ dispatch
@claude can you review this?	✅ dispatch
@claude, review / @claude re-review / @claude review again	✅ dispatch
@claude the review workflow is broken, can you fix it?	✅ no dispatch
@claude can you review this and fix the failing test?	✅ no dispatch

Eleven adversarial cases beyond the committed table, aimed at the new end-of-line anchor, since that is the part most likely to have collateral effects: bare @claude review with no trailing newline, trailing ./!!, leading indentation, a mid-line prefix (Thanks! @claude review), review this please, and a newline followed by a parenthetical. All behave correctly. The anchor's interaction with [[:space:]] inside TAIL — which can cross a line boundary — is saved by backtracking to the bare newline, which I confirmed rather than assumed.

shell: bash is declared on the step using B64_DECODE=(...), so the array is safe; a composite defaulting to sh would have broken it.

Added lines are free of em-dashes, en-dashes, curly quotes, and ×.

Findings

1. The PR body's test count is wrong (minor, factual). It says "38 cases pass, up from 27." The harness reports 40. The up from 27 half is right — I ran main to confirm. Presumably two cases were added after the body was written. Worth fixing since the body is the record of what was verified.

2. @claude review when you get a chance is a false negative (non-blocking). Already pinned in your test table as deliberate, and I agree with the asymmetry argument — a false positive swallows a real question, a false negative costs a self-review. Raising it only because, of the two regressions against #341, this one reads as the more likely phrasing in practice: it is pure courtesy with no object and no instruction, so nothing about it signals "this is a mixed request." If you ever revisit TAIL_WORD, a trailing-courtesy clause (when you (get|have) a (chance|moment|sec), whenever, no rush) sitting alongside POLITE would recover it without loosening the object set at all. Not asking for it in this PR.

Things I checked and found correct

• The continue-on-error: true reasoning holds: with the step failed, steps.late_review_request.outputs.match is empty, so the dispatch stays gated off — the comment's claim matches the mechanism.
• base64 -d vs -D probing with printf '' | base64 -d is the right shape; it distinguishes the two without depending on either succeeding on real input.
• tr -d '\r' over sed 's/\r$//' is correct and the stated reason (GNU-only \r in a sed BRE) is accurate. Making CRLF normalization load-bearing rather than deleting it as detect-review-request: post-merge findings from reviewing #341 #346 proposed is the better call now that the tail anchors on a bare newline.
• The changelog fragment is in the right category (fixed) and its description matches what the code now does.

Verdict

Ready for merge once finding 1 is corrected — one number in the PR body, no code change.

Sequencing note: this PR cannot be validated end-to-end until #351 lands, because claude.yml still reaches the composite through d-morrison/gha/.github/actions/detect-review-request@v2. That is not this PR's problem to fix, but it does mean merging #351 first and then merging main into this branch is what will produce a real review run here.

---

Generated by Claude Code

[github-actions]

👀 Picked up by workflow run #30387037425. Setup runs first; Claude itself responds after that.

[github-actions]

Claude finished review — View run

---

Based on my review, I independently traced the regex logic (TAIL_WORD/TAIL construction, the CRLF/quote normalization, and the end-of-line anchor) against a dozen edge cases including the newly pinned false negatives and the gha#346 regression cases, verified the base64 -d/-D portability probe logic, confirmed the continue-on-error: true addition in claude.yml is correctly scoped and its downstream if: gate degrades safely, confirmed the referenced issue/PR numbers (#341 merged, #346 open) are real, verified the changelog fragment's category and format comply with changelog.d/README.md, and hand-counted the test table (37 array cases + 3 special-case assertions = 40 total), which matches the PR body's "40 cases pass, up from 27" claim exactly.

I found no new issues beyond what the PR author's own self-review already surfaced.

Review summary

This PR is already covered by a thorough self-review posted on the thread: comment #issuecomment-5108082215 (2026-07-28 18:21 UTC), posted because the automated claude-review check couldn't produce a verdict at the time (blocked first by an account outage, then by the d-morrison/gha → Morrison-Lab/gha org-rename bootstrapping gap on @v2, per CLAUDE.md's "can't self-verify" precedent).

That self-review:

• Ran the full offline test table (40 cases, up from 27 on main).
• Verified all 8 documented phrasings in website/reference/claude.qmd against the actual script.
• Ran 11 additional adversarial cases targeting the new end-of-line anchor.
• Confirmed shell: bash is declared where the B64_DECODE array is used.
• Confirmed no em-dashes/curly quotes/× in added lines.
• Raised finding 1 (PR body understated the test count: said 38, harness reports 40) as the sole blocking-before-merge item, and finding 2 (the when you get a chance false negative) as non-blocking, already deliberately accepted.
• Concluded: "Ready for merge once finding 1 is corrected — one number in the PR body, no code change."

I confirmed finding 1 is already resolved: the current PR body's Verification section reads "40 cases pass, up from 27," matching the harness output, and this matches commit 58f287f ("Pin the two known false negatives and stop understating the trade"), which is the current HEAD.

I found no new issues in the diff beyond what that self-review already covered and resolved. My own regex trace confirms the TAIL/TAIL_WORD construction correctly distinguishes the pinned true/false cases (e.g., @claude review this PR → dispatch; @claude can you review this and fix the failing test? → no dispatch; @claude reviewer assignments are wrong → no dispatch via the end-of-line anchor subsuming the old word-boundary guard), and the CRLF/base64 -d vs -D portability fixes are logically sound and correctly reasoned (BSD/macOS base64 only accepts -D, not -d).

Verdict

Ready for merge — the prior self-review's verdict stands; finding 1 (the only blocking item) has already been corrected, and no new issues surfaced in this pass.

[github-actions]

💰 Cost: $1.6067 (review) — run