pr-review process defects found driving the ADR-0007 pilot (false hard-stop, empty fix-requested, silent no-op, gate counts maintainer comments, unwired bypass, edited-comment deadlock, self-dismissed approvals) #2173

Summary

Driving the ADR-0007 markets pilot (combsmith/markets#513, plus #2010, #2039, #2051, .github#1238/#1239) exposed a cluster of pr-review process defects. The review's substance was largely sound (on #513 the final audit tier cleared its own hard-stop and reached the same assessment a human review did). The defects are in the heuristics and gate machinery around it, and together they cost days of wall-clock and a stream of bot churn. Each item below was observed, with evidence. Unlabelled on purpose: items 1, 4, 5 and 6 are design decisions that need a ruling before dev-lead takes them.

1. The PROMPT_INJECTION_DETECTED hard-stop fires on comment text

scripts/lib/safety-checks.sh:105 applies c ~ /pull_request_target/ to every added line of a workflow file, including comment lines, with no comment stripping and no check that it is an actual on: trigger. prompts/deep-review-*.md then say a true flag means risk: HIGH and "do not approve". markets#513's header comment lists the pull_request_target carve-outs, so it was guaranteed to trip it (tier-1 reported escalate=true risk=HIGH). The later tiers cleared it, but the deterministic flag can never be overridden.
AC: the scan ignores # comments and matches only a real pull_request_target key under on:; add a fixture where the string appears only in a comment (must NOT flag) and one with the real trigger (must flag).

2. Escalations are posted as decision=fix-requested with an EMPTY findings list

In a sample of 8 PRs, 32 of 32 decision=fix-requested reviews have an empty ### Findings to fix list immediately followed by ## Automated review — NEEDS HUMAN REVIEW. The review says "address each one" and lists none, which consumes a 3-cycle budget and invites dev-lead to push (and so dismiss approvals). .github#1238 accumulated 15 such reviews across five 1-to-3 cycle runs; .github-private#2010 and #2051 each hit cycle 3/3 on empty lists.
AC: an escalation is posted as an escalation (no "fix requested", no cycle consumed, no dev-lead trigger); fix-requested is used only when there is at least one actionable finding, and the list is non-empty.

3. A gated review is a SILENT no-op, after an acknowledgement that promises a result

The @donpetry-bot mention posts "I'm on it — starting a fresh review now. Results will appear in a few minutes" (templates/mention-listener.yml:77). When the maintainer-comment gate then skips, the only record is {"decision":"skip","reason":"undispositioned-pr-comment"} and "0 reviews posted" in the Actions log. Nothing is posted on the PR. On markets#513 this cost hours across three re-requests.
AC: a no-op posts one short PR comment naming the reason and the blocking comment ids (idempotent per head SHA).

4. The maintainer gate counts the human owner's own comments as findings

Dev-lead, the maintainer and the agent all post as don-petry. The gate counts any don-petry comment without an automation marker as an undispositioned finding. I ran the gate's own jq verdict over #513: it counted my steering comment, both of my @donpetry-bot mention comments, and the automation's own auto-rebase-conflict sentinel (whose marker is not in the gate's marker regex). Each re-review mention creates its own blocker.
AC: comments authored by the maintainer account that are pure steering/mentions (or carry a registered automation marker, including auto-rebase-conflict) are not findings; test with a PR whose only open comments are an @donpetry-bot mention and a sentinel.

5. The documented human bypass (@mention → FORCE_REVIEW) is not delivered on the channel consumers use

Already tracked: #2082. Evidence from the markets pilot has been added there as a comment (the v2-stable mention reusable forwards only pr_url; the mention path logs FORCE_REVIEW: false; a direct repository_dispatch with force_review=true works). Not duplicated here.

6. Resolved-then-edited bot summaries deadlock, and the fix is not live

Already tracked: #2166. The markets-pilot manifestation, including the bot-comment-already-resolved skip and why a human minimize makes it permanent, has been added there as a comment. Not duplicated here.

7. The bot approves, then dismisses its own approval

On .github-private#2039 donpetry-bot posted a full decision=approved review on the head twice (20:02Z and 22:24Z), then dismissed each about a minute later (review_dismissed by donpetry-bot) because three answered finding-bot review threads were still unresolved, while the review text itself said "all answered, none blocking". Cleared only by a human resolving the threads and minimizing their root comments.
AC: the approval is not posted when the gate would dismiss it; or the dismissal states its reason on the PR.

Related

• #1894 AC #4: give maintainers a dev-lead-independent way to disposition a blocking PR comment #1910, pr-review can't approve ANY open PR: maintainer-comment gate needs pushedDate (always null) and treats codeant/qodo/auto-rebase comments as maintainer findings — 18/18 PRs stranded #1813, CodeRabbit's Security Architecture Review is throttled separately from its code review — findings in both sections must be addressed, and an in-place edit after disposition must re-open the comment #2008 (the gate), and Follow-ups from #2004 / PR #2051 and the markets ingress pilot (verifier region check, phantom Fixed replies, dispatch cancellation, dry-run slot) #2068 (other follow-ups from A fixed disposition citing the WRONG sha can never verify — comment is never minimized, dev-lead refuses to re-answer, PR blocks permanently (#1977) #2004/feat: implement issue #2004 — A fixed disposition citing the WRONG sha can never verify — comment is never minimized, dev-lead refuses to re-answer, PR blocks permanently (#1977) #2051). Item 6's fix (unminimize on edit, from feat: implement issue #2004 — A fixed disposition citing the WRONG sha can never verify — comment is never minimized, dev-lead refuses to re-answer, PR blocks permanently (#1977) #2051) not being live in consumers is an instance of the channel-promotion gap: merged fixes don't run until the dev-lead channel tag is promoted.
• The comment-record deadlocks (items 4 and 6) were worked around by hand on feat: implement issue #2038 — Markets collapse package §3/§4: re-render pr-auto-review and pr-review concurrency to ADR-0010 and mark the ruling ratified #2039, feat: implement issue #2004 — A fixed disposition citing the WRONG sha can never verify — comment is never minimized, dev-lead refuses to re-answer, PR blocks permanently (#1977) #2051 and feat(pr-review): agent_ref input for pinned self-checkout (#497/#506) #513; each needed several rounds.