feat(github): gate comment-shaped Ally review findings (BLO-21907)

[kkroo]

Thinking Path

• Paperclip is the open source control plane people use to manage AI agents for work.
• Its GitHub integration coordinates reviewer wakes and feeds protected-merge evidence back to pull requests.
• GitHub’s native reviewDecision sees formal pull_request_review objects, but Ally must publish a plain consolidated comment when its App cannot formally review its own PR.
• Master recognizes those comments for wake handling but does not publish a branch-protection status for their Critical or Important findings.
• As a result, an unaddressed comment-shaped finding can be invisible to merge eligibility even though it is real review feedback.
• App-authored feat(github): gate comment-shaped Ally review findings (BLO-21907) #1049 contains the needed behavior but cannot be the independently authored protected-merge path.
• This draft carries the behavior forward on current master, preserving its opt-in, exact-head, trusted-App semantics.
• The benefit is that a comment-shaped blocking Ally finding can enforce a required GitHub status without changing the existing formal-review path.

Linked Issues or Issue Description

Refs #1049 (the App-authored source PR; this independent successor does not mutate or close it).

The underlying issue is BLO-21907: comment-shaped Ally review findings are not represented in GitHub’s formal review decision, so an unresolved Critical or Important finding can otherwise have no merge-time enforcement.

What Changed

• Extract the hardened consolidated-review heading, exact-head attestation, and actionable-finding detection into a shared ally-review-detection service.
• Add an exact-head comment-review evaluator: only the configured Ally App identity’s latest consolidated comment for the current head can produce a failure; a later clean review clears an earlier finding.
• Add complete paginated GitHub issue-comment reads, current-head lookup, and a bounded retrying status publisher.
• Add an opt-in prCommentReviewGateStatusContext configuration and Helm wiring; the default empty value performs no API calls or status writes.
• Trigger the detached status evaluation from signed issue_comment.created and pull_request opened/reopened/synchronize payloads, independently of Paperclip identifier matching, so clean re-reviews can clear a prior failure.
• Add focused evaluator, publisher, auth-helper, and webhook-trigger coverage.

Verification

• pnpm --dir server exec vitest run src/__tests__/pr-comment-review-gate.test.ts src/__tests__/pr-comment-review-gate-check.test.ts --reporter=verbose — 11 passed.
• pnpm --dir server exec vitest run src/__tests__/github-app-auth.test.ts --reporter=verbose — 46 passed.
• pnpm --dir server exec tsc --noEmit — passed.
• Helm renders pass for default, configured StatefulSet, and configured HA Deployment paths; the new environment variable is absent by default.
• node --test deploy/helm/paperclip/tests/*.test.mjs had 32 passes and 12 failures in the pre-existing approval-plan-marker tests at kubectl create --dry-run=client; the identical 12 failures reproduce on clean origin/master.
• The full github-webhook.test.ts embedded-Postgres harness did not complete in this runner after its worker was orphaned; the temporary task-only processes were stopped. CI should run that full integration suite.

Risks

Low-to-moderate operational risk. The feature is inert until an operator configures a status context and makes it required in GitHub branch protection. Webhook execution remains fire-and-forget, so a GitHub read/write failure cannot delay or fail the webhook acknowledgement; a subsequent relevant delivery retries evaluation. Exact full-head attestations prevent stale comments from blocking a replacement head, while an unambiguous trusted App identity prevents same-shaped contributor comments from controlling the gate.

Model Used

OpenAI GPT-5 (Codex agent), tool-assisted repository analysis, conflict resolution, and focused validation. The exact context-window size is not exposed to this session.

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• I have run focused tests locally and they pass
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots (not applicable)
• I have updated relevant documentation to reflect my changes (Helm values comments)
• I have considered and documented any risks above
• All Paperclip CI gates are green
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-21907

[allyblockcast]

🔗 Paperclip issue: BLO-21907

[allyblockcast]

Checked in on this per BLO-21907 (this PR carries forward the fix from #1049). Falsifying before acting, per the reliability lane's triage protocol:

No reviewer findings currently exist to address. pulls/1262/reviews is empty (no formal review), pulls/1262/comments has zero review comments, and the only issue comments are the two duplicate Paperclip backlink notices — no ## Ally — Consolidated PR Review comment has landed on this head. There's nothing comment-shaped or otherwise for this gate itself to react to yet.

The one failing required check (verify) is a CI-infra artifact, not a diff defect. Its upstream dependency policy shows conclusion: cancelled (The operation was canceled. mid-run at 13:05:36Z on 2026-08-10), which cascaded to verify reporting five lanes as "did not run." That's the mid-run-interruption class this repo already tracks (see the verify step's own annotation referencing BLO-20867), not a test failure — policy's partial output before cancellation showed all its own checks passing (13 unit tests + shard-folding + release-package-map + IP-security override, all green) with no failure line anywhere in the log.

Re-ran the failed jobs (POST /actions/runs/31390602298/rerun-failed-jobs) to get a clean signal — attempt 2 is queued now. Repo-wide there are currently 44 queued / 1 in-progress workflow runs, so it may sit a bit; I'll keep an eye on it. Not touching the branch or approving — leaving that to your review pass once CI reports clean.

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[kkroo]

@ally please review exact head 1d3174b188d786302a2610005398b5cb364a1bc0.

Rebased onto current master while preserving masters formal App-approval evidence path unchanged; this head adds only the separate opt-in exact-head comment-status gate. Verification: 124 runnable focused tests passed (54 host DB skips), server typecheck passed, configured StatefulSet and HA Deployment Helm renders passed, diff check passed, and commit attribution passed.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 1d3174b

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The gate is opt-in, exact-head scoped, and fails closed on incomplete comment history.
• The serial evaluation chain prevents an older webhook snapshot from overwriting a newer status verdict.

Recommended Action

1. Merge when the normal CI and required checks are green.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 1d3174b

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The gate is opt-in, exact-head scoped, and fails closed on incomplete comment history.
• The serial evaluation chain prevents an older webhook snapshot from overwriting a newer status verdict.

Recommended Action

1. Merge when the normal CI and required checks are green.