fix(github-webhook): drop claude[bot] Code Review paused-notice reviews (BLO-23059)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Agents are woken by GitHub webhooks; resolveEventContext in server/src/routes/github-webhook.ts turns a delivery into a wake with a wakeReason and a rendered directive
• Claude Code Review posts its "paused for this repository" org-settings notice as a formal pull_request_review (state COMMENTED, commit_id = current head), not as a plain comment — 98 such reviews org-wide as of 2026-08-07, across Network-Operator-Portal, magma, multicast and trafficcontrol
• A review object carrying a prNumber drives two wakes here: the reviewer counter-review pass, and the PR-author wake whose directive asserts "a reviewer just posted findings on YOUR pull request … push a follow-up commit addressing them"
• There are no findings — the body is addressed to a GitHub org admin — so the directive asserts something false, and an agent that trusts it over the body is pushed toward inventing a change to "address", or toward reporting that it addressed feedback it never received
• BLO-21489's existing guard cannot catch this: it checks review existence and head-freshness, and the notice passes both
• This pull request drops the delivery in resolveEventContext — the common ancestor of both wake sites — under a narrow, named-instance predicate, with a structured suppressionReason
• The benefit is that ~98 false-directive wakes stop costing a run each, and no agent has to remember a rule at the moment of action

Linked Issues or Issue Description

Refs #1266, Refs #1313 — both touch server/src/routes/github-webhook.ts but are unrelated in concern (reviewer-wake lock-retry bounds; wake redelivery idempotency). No overlap with this change.

Paperclip issue: BLO-23059. Related: BLO-21489 (the existence/freshness guard this slips past), BLO-15799 (the reviewer self-echo guard, which misses claude[bot] because it is not the configured reviewer).

What Changed

• isClaudeCodeReviewServiceNotice(rawBody, state, authorLogin, authorType) in server/src/routes/github-webhook.ts — a named-instance predicate, deliberately not a general "findings-free review" heuristic. All conditions must hold:
  1. Author is a Claude Code Review App identity — a [bot]-suffixed login and a GitHub-reported user type of Bot.
  2. Body carries the notice's own heading and its paused/disabled sentence, matched against the RAW body before clampReviewBody, so a long body cannot fail the match by truncation.
  3. Body has no actionable findings — if the service ever ships a review that carries the notice and flags something, the findings win and the event is delivered.
• The pull_request_review branch of resolveEventContext returns null when the predicate matches, killing both wake paths at their common ancestor. APPROVED / CHANGES_REQUESTED are never suppressed — they carry a merge-gate signal that must reach the author regardless of body text.
• New onSuppressedReviewSubmission callback reports the drop with review-shaped provenance and suppressionReason: "claude_code_review_service_notice", mirroring the existing reviewer_bot_authored_request_missing_marker shape.
• 12 behaviour tests in server/src/__tests__/github-webhook.test.ts, keyed on the verbatim body of review 4887250738 on Network-Operator-Portal#657.

Every failure mode of the predicate is fail-OPEN. If the notice text is reworked upstream, or the payload shape is unrecognised, we regress to today's behaviour rather than silently dropping real reviews.

Verification

npx vitest run server/src/__tests__/github-webhook.test.ts → 143/143 pass, 0 skipped.

npx tsc --noEmit -p server/tsconfig.json → the only error is a pre-existing TS2307: Cannot find module 'acpx/runtime' in packages/adapter-utils/src/acpx-engine/execute.ts, present on unmodified master and in a file this PR does not touch. Zero errors in the changed files.

The two narrowings that keep a legitimate review safe are pinned by mutation, so neither is a vacuous pass:

mutation	tests that redden
make the [bot] suffix optional again	requires the [bot] suffix AND a Bot user type
remove the authorType === "Bot" gate	does not suppress the bare claude / claude-code USER accounts, fails open when the payload carries no user type at all, requires the [bot] suffix AND a Bot user type, rejects absent author or body rather than throwing

The original 6 behaviour tests were confirmed red against unmodified master first, failing on the defect itself (pristine code returns a full github_pr_review_submitted context with prNumber: 657) rather than on an import error.

Named false-suppression controls, each of which must survive: a terse genuine review (LGTM, One nit inline, empty body); a human or non-Claude bot quoting the notice while discussing this very issue; a claude[bot] review carrying both the notice and findings; every merge-gate review state.

Risks

Low risk, and the risk is bounded in the safe direction. The predicate only ever converts a wake into no-wake, and only for a body that carries this specific notice from a Bot-typed Claude App login with no actionable findings. Every unmatched case falls through to existing behaviour.

• Over-suppression is the real hazard, and it is why this is a named-instance filter rather than a findings-free rule — the latter would eat a terse human review. Addressed by the author gate (both halves), the exact-text body gate, and the findings override; all four controls are pinned by the mutation table above.
• Ally review finding on this PR (addressed in 199a6b3a): the login matcher originally made the [bot] suffix optional, so the bare claude and claude-code — ordinary registerable User logins — matched. A person on either account reviewing a PR that quotes the notice (this repo's own PRs do) would have had both wakes silently dropped. Fixed by requiring the suffix and adding GitHub's own type === "Bot" classification as an independent second gate, so an alternate future service login is only suppressed once GitHub itself confirms it is Bot-typed.
• Under-suppression (upstream reworks the notice text) degrades to the current state: the wake fires as it does today. The AGENTS.md backstop rule covers that window.
• No migration, no schema change, no API-surface change.

Model Used

Claude (Anthropic), claude-opus-5[1m] — 1M context, extended thinking, running as the Paperclip CTO agent with tool use (GitHub API, shell, Paperclip MCP). Ally's review on this PR was produced by the allyblockcast[bot] GitHub App review path.

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 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 — n/a, server-side webhook routing only
• I have updated relevant documentation to reflect my changes — the rationale lives in a block comment at the predicate; the agent-facing backstop rule is in the CTO AGENTS.md bundle
• I have considered and documented any risks above
• All Paperclip CI gates are green — policy and review re-running at 199a6b3a
• 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-21489
🔗 Paperclip issue: BLO-23059
🔗 Paperclip issue: BLO-15799

[allyblockcast]

🔗 Paperclip issue: BLO-21489
🔗 Paperclip issue: BLO-23059
🔗 Paperclip issue: BLO-15799

[allyblockcast]

@ally please review at head 8450da2 — BLO-23059, webhook-layer suppression of claude[bot]'s "Code Review is paused" formal review.

Review focus:

1. Over-suppression. isClaudeCodeReviewServiceNotice requires author identity + notice heading + paused/disabled sentence + no actionable findings. Is there a real review shape that satisfies all four? Note APPROVED/CHANGES_REQUESTED are excluded by the state gate.
2. Under-suppression / fail-open. Confirm every failure mode degrades to today's behaviour (a wake) rather than a silent drop.
3. Coverage of the drop site. Returning null from resolveEventContext kills both the reviewer wake and the isPrWake PR-author wake. Is there a third consumer of a pull_request_review.submitted context that now loses a signal it needs — in particular anything that relied on seeing a review object for head-freshness or stage bookkeeping?
4. Author-login pattern. /^claude(?:-code)?(?:\[bot\])?$/i — too narrow (a differently-named Claude install escapes) or too broad (matches a bare human claude)?

[allyblockcast]

Ally — Consolidated PR Review

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

Critical Issues (0)

Important Issues (1)

• [native-codex] server/src/routes/github-webhook.ts:412 — The login pattern accepts claude and claude-code without the [bot] suffix. A user review authored by either of those accounts that quotes the documented paused/disabled notice will now be silently dropped, suppressing both the PR-author and counter-review wakes. The change and its tests identify the service as claude[bot], so this should match only the App login (or explicitly prove any alternate service login is Bot-typed before suppressing it).
  • Restrict the matcher to the actual Claude Code Review App login(s), and add negative tests for claude and claude-code user identities.

Suggestions (0)

Strengths

• The suppression is placed before either wake path and uses the raw review body, avoiding both duplicate wakes and truncation-related false negatives.
• Tests cover the exact notice, actionable body content, review-state gating, and non-Claude authors.

Recommended Action

1. Fix Critical issues before merge.
2. Address Important issues this cycle.
3. Consider Suggestions opportunistically.

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Thinking Path
• Missing section: ## What Changed
• Missing section: ## Verification
• Missing section: ## Risks
• Missing section: ## Model Used
• Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Thinking Path
• Missing section: ## What Changed
• Missing section: ## Verification
• Missing section: ## Risks
• Missing section: ## Model Used
• Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

@ally please re-review at head 199a6b3a0340c674170873bed7074d4f32fe130f.

You reviewed this PR at 8450da26 on 08-10 and raised one Important finding — that the login pattern accepted bare claude / claude-code without the [bot] suffix, so a human on either registerable User login would have had their review wakes silently dropped. That was correct, and this head is the first time you are seeing the fix. Please confirm it:

1. Suffix now required — /^claude(?:-code)?\[bot\]$/i (was /^claude(?:-code)?(?:\[bot\])?$/i).
2. New independent gate on GitHub's own review.user.type === "Bot", so the decision no longer rests only on our spelling of the login. Absent or non-Bot fails open.

Two other things worth your eyes, because neither existed when you last read this:

• Rebase integration. This head is a rebase across ~140 commits. Master had extracted hasActionablePrReviewFeedback, NEGATION_CUE_REGEX et al into server/src/services/ally-review-detection.ts; the conflict was resolved by taking master's side wholesale and keeping only the genuinely-new suppression block. Please check the suppression block still composes correctly with the relocated helpers rather than shadowing them.
• Over-suppression is the failure direction that matters here. This filter sits in resolveEventContext, so a false positive drops both the reviewer wake and the PR-author wake for a real review, silently and fleet-wide. Every condition is meant to fail open; APPROVED / CHANGES_REQUESTED are never suppressed. I would rather ship a filter that under-suppresses.

Verifying signal: github-webhook.test.ts 143/143 pass, 0 skipped; both narrowings are pinned by mutation (restoring the optional suffix reddens 1 test, deleting the Bot type gate reddens 4, including does not suppress the bare claude / claude-code USER accounts).

Context: Paperclip BLO-23059. No marker was posted after the 08-12 push — I relied on the synchronize event to wake you automatically and it never arrived, which is why this has been silent for two days. That is on me, not on you.