fix(heartbeat): GitHub-verify PR-review evidence before flagging pr_review_output_missing (BLO-10448) - #427
Merged
Conversation
…eview_output_missing (BLO-10448) The evidence guard (evaluatePrReviewCompletionEvidence) is a text heuristic over the agent's free-text completion summary; it flags pr_review_output_missing whenever the summary lacks a recognized posted-review/skip marker. In production that is mostly false positives: over 7d, of 95 flagged runs, 0 claim a fresh post, ~34% say 'already reviewed' (idempotency skips the regex misses — a backtick before the SHA, or a repo#n ref instead of a bare SHA), and only ~3 are genuinely empty. The PRs DID get Ally reviews. These false failed/error flags create sweep + agent-health noise (BLO-3202) and distort metrics. The BLO-8195/9293/3202 trail shows the guard has been regex-patched repeatedly. Fix: before keeping a 'missing' verdict, authoritatively check GitHub for a reviewer-bot review/comment at THIS head. The server has no ambient GitHub token, so mint short-lived App installation tokens in-process (node:crypto RS256, no new dep) from the existing paperclip-github-app-creds secret. The check only runs on the 'missing' branch and only RESCUES a false missing — never downgrades a heuristic success; any error / unconfigured creds / not-found falls back to today's behavior, so it cannot make things worse. Head-SHA precision (review commit_id===headSha, or comment body referencing the SHA) avoids masking a genuine miss on a newer head. - server/src/services/github-app-auth.ts (new): App JWT mint, cached installation token, githubHasReviewerEvidenceForPr (reviews + comments, head-SHA precise). - server/src/services/heartbeat.ts: async override at the evidence seam in executeRun; fallback-on-error. - server/src/config.ts: githubApp* + prReviewerBotLogin (all empty-default -> inert). - deploy/helm: mount paperclip-github-app-creds into api + workers (the workers tier runs executeRun) behind githubApp.enabled (default false). - Tests: github-app-auth.test.ts (JWT verify, token cache, evidence branches, error fallback). Full heartbeat guard/recovery/retry suites green (190). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merged
12 tasks
kkroo
added a commit
that referenced
this pull request
Jun 15, 2026
…ockcast (BLO-10448) (#428) Activates the GitHub-verified pr_review_output_missing fix (#427) on the Blockcast deploy by setting githubApp.enabled=true in values.blockcast.yaml. The docker.yml release runs `helm upgrade ... -f values.blockcast.yaml`, so this mounts the existing paperclip-github-app-creds Secret (app_id / installation_id / private_key.pem) into the api + workers tiers. The workers StatefulSet runs the heartbeat executeRun finalization where the verification fires. Chart plumbing + the default-off flag landed in #427; this is the deploy-side enablement. helm template -f values.blockcast.yaml renders the three GITHUB_APP_* env on both tiers. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merged
1 task
kkroo
added a commit
that referenced
this pull request
Jun 19, 2026
…og non-rescue (BLO-10878) (#454) The GitHub-verify rescue for false pr_review_output_missing (#427) left two gaps, both observed in the post-deploy residual: 1. When the reviewer-wake carried no head SHA, derivePaperclipPrReview returned headSha=null, so githubHasReviewerEvidenceForPr's `headPrefix` was null and the comment-mode check was skipped entirely — only the formal-review loop ran. Ally frequently reviews in comment mode (no formal review), so those PRs (NOP#395, tc#777, …) flagged a false missing. Fix: when input.headSha is null, fetch the PR's current head from the API and run the precise review+comment match with it. Falls back to the existing lenient any-bot-review path only if the fetch fails. 2. The non-rescue branch logged nothing, making residual false-missings unclassifiable. Add a lifecycle event recording the outcome (the {error} code or no_evidence_found) so the remaining residual is diagnosable from events. Tests: github-app-auth.test.ts +3 (null-head fallback→comment match, lenient fallback when head unresolved, not-found after resolve). 14/14 pass; tsc clean; heartbeat-retry-scheduling + process-recovery 121/121 green. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 20, 2026
13 tasks
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the BLO-10448 reviewer-reliability work (#421/#423/#425). Those fixed the retry classes; this fixes the #1 remaining reviewer failure: false
pr_review_output_missing.Related (searched, no duplicates): #421/#423/#425 (merged BLO-10448 retry fixes). No open PR touches the evidence guard.
Thinking Path
evaluatePrReviewCompletionEvidence(server/src/services/heartbeat.ts) decides whether a finished PR-review run actually left a review by text-matching the agent's free-text summary. It flagspr_review_output_missingwhenever the summary lacks a recognized posted-review/skip marker. Production data shows this is mostly false positives: over 7 days, of 95 flagged runs, 0 claim a fresh post, ~34% say "already reviewed" (legitimate idempotency skips the regex misses — e.g. a backtick before the SHA, or arepo#nref instead of a bare SHA), and only ~3 are genuinely empty. The flagged PRs did receive Ally reviews (NOP#409: 6 reviews, penstock#156: 1). Each false flag flips a correct run tofailed/error, creating sweep + agent-health noise (BLO-3202) and making metrics show dropped reviews that weren't dropped. The BLO-8195/9293/3202 comment trail is a history of regex-patching this guard — whack-a-mole. The authoritative signal lives on GitHub, not in the summary.What Changed
server/src/services/github-app-auth.ts(new): mint an RS256 GitHub App JWT (node:crypto, no new dep) → cached short-lived installation token →githubHasReviewerEvidenceForPr()which queries/pulls/{n}/reviews(bot review withcommit_id === headSha) and/issues/{n}/comments(bot comment referencing the head SHA, for comment-mode reviews on bot-authored PRs). ReusesgitHubApiBase/ghFetch.server/src/services/heartbeat.ts: at the evidence seam inexecuteRun, when the heuristic returnsmissing,awaitthe GitHub check; onfoundoverride toposted_review(+ a lifecycle event); on error/not-found keepmissing.server/src/config.ts:githubAppId/githubAppInstallationId/githubAppPrivateKey/prReviewerBotLogin(all empty-default → feature inert when unset).deploy/helm/...: mount the existingpaperclip-github-app-credssecret into the api + workers tiers (the workers statefulset runsexecuteRun) behindgithubApp.enabled(default false).Verification
server/src/__tests__/github-app-auth.test.ts(11): JWT verifies against its public key; installation-token caches; evidence found-via-review / found-via-comment / not-found / non-OK→error / no-creds→error.heartbeat-context-summary+heartbeat-process-recovery+heartbeat-retry-scheduling— 190 pass.tsc --noEmitclean (0 errors).helm templaterenders the threeGITHUB_APP_*env on the workers tier whengithubApp.enabled=true, nothing when false.githubApp.enabled=truein the Blockcast values + redeploy so the workers pod gets the creds; then watch for "suppressing false pr_review_output_missing" lifecycle events and the flagged-rate drop.Risks
Low / strictly additive. The GitHub check runs only on the
missingbranch, only rescues a false missing, and never downgrades a heuristic success. Any unconfigured creds / token failure / API error / not-found → unchangedmissing(today's behavior), so it cannot regress. Head-SHA precision avoids masking a genuine miss on a newer head. The App key lives in pod env (standard for GitHub Apps; tokens are 1h-TTL, minted from the existing secret — no new long-lived credential). Default-off via Helm flag.Model Used
claude-opus-4-8 (1M context)
Checklist
Fixes: #/Closes #/Refs #OR (b) described the issue in-PR — Refs: BLO-10448🤖 Generated with Claude Code