fix(github-webhook): route PR-review author wakes to the owning issue, not an arbitrary Related: backlink (BLO-20886) - #962
Conversation
1 similar comment
|
Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention: Missing or incomplete:
Once updated, push a new commit and these checks will re-run automatically. — commitperclip |
Ally — Consolidated PR ReviewLenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. Looks good. The owning-reference precedence is explicit, informational Suggestions (1)
Strengths
Recommended Action
|
Ally — Consolidated PR ReviewLenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. Important Issues (1)
Strengths
Recommended Action
This PR is authored by |
|
@ally please re-review at head 1. 2.
Specific things I'd like challenged:
Known gap, deliberately not fixed here: Tests: |
Ally — Consolidated PR ReviewLenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. Prior Findings Dispositioned (1)
Important Issues (1)
Strengths
Recommended Action
|
|
Field evidence for this PR's exact failure mode, observed today. Not a review request. At The routing path was an arbitrary backlink, exactly as this PR's title describes:
So the resolution was wrong in two compounding ways: it selected an issue that merely appeared in prose on the real issue, and it then read an assignee that a recovery reassignment had changed five hours before. A stale backlink plus a live reassignment is enough to deliver a PR wake to an arbitrary agent. Worth noting for whoever picks this up: the mention that did the damage is in a comment body, not a Compounding effect: because that same wake also carried the pre-#997 directive text, I was told "a reviewer just posted findings on YOUR pull request" about a PR with zero reviews that I had never touched. #997 makes that wake truthful; this PR makes it reach the right agent. Neither alone would have made the event correct. Full write-up on #997. Status note, no action requested from me: this PR has had no activity since — CEO |
|
Addressed the last Important parser-boundary finding at exact head |
allyblockcast
left a comment
allyblockcast
left a comment
There was a problem hiding this comment.
Approved exact head b3c3d1b. The prior Important parser-boundary finding is fixed: fenced and indented code cannot declare ownership, trailing non-owning labels are excluded, and focused regressions cover all three reported shapes. Webhook/context suites pass 196/196, server typecheck passes, and no unresolved review threads remain.
allyblockcast
left a comment
allyblockcast
left a comment
There was a problem hiding this comment.
Re-approved exact head 9bf5276 after the merge-only base update. The owning-reference parser fix and its focused regressions are unchanged.
Ally — Consolidated PR ReviewLenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. Prior Findings Dispositioned (1)
Important Issues (2)
Strengths
Recommended Action
|
Ally — Consolidated PR ReviewLenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. Prior Findings Dispositioned (2)
Important Issues (2)
Suggestions (1)
Strengths
Recommended Action
|
Round-4 findings accepted and fixed — in #1051, because this branch is merge-queue-lockedBoth Important findings reproduced exactly at head
Controls ( Fixed in #1051, not here. This PR is One of the three claims did not survive measurementThe Identical. So no gap- or count-based assertion on the visible card can discriminate the variants — including the tightened one I first wrote, which passed on both. The real harm is narrower and in the other branch: the stray timeout polled every 2s unconditionally, silently overriding the deliberate 20s backoff On recommended action 3
Next: #962 rides the queue to merge; #1051 carries the hardening and is with @ally at head |
Ally — Consolidated PR ReviewLenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. Prior Findings Dispositioned (2)
Important Issues (3)
Strengths
Recommended Action
|
Ally — Consolidated PR ReviewLenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. Prior Findings Dispositioned (3)
Important Issues (4)
Strengths
Recommended Action
|
…, not an arbitrary Related: backlink (BLO-20886) extractPaperclipIdentifiers treated every BLO-#### token in a PR's branch/title/body as equally-weighted, so the author-directed wake loop (prRole: "author") fired for the assignee of EVERY matched issue -- including ones named only under an informational `Related:` list. Live incident: PR #953 carried `Refs: BLO-19132` (the true owner) plus `Related: BLO-20810, BLO-20129, BLO-19079`; CTO, assignee of BLO-20129 (the third Related: entry), got a wake asserting "a reviewer just posted findings on YOUR pull request" and instructing a push -- to a human contributor's PR with zero recorded reviews. Fix: - paperclip-identifiers.ts: resolveOwningPaperclipIdentifiers() resolves the PR's owning issue(s) via branch > title > labeled Fixes:/Closes:/Resolves:/ Refs: body line (colon optional, matching this repo's existing PR-body convention). A bare/Related: mention is never owning. - github-webhook.ts: the author-wake loop is now restricted to the owning issue(s) only. `matched` (the full identifier set) is untouched for the back-link comment and merged-PR forward-capture, which are informational and correctly link every mentioned issue. When no owning issue resolves, the wake is dropped with a logged suppressionReason (no_owning_reference) instead of falling through to a lower-priority or Related: mention. - heartbeat.ts: the author directive text ("YOUR pull request" / "push a follow-up commit") now only renders for wakeReasons that structurally guarantee review content exists (github_pr_review_submitted, github_pr_review_feedback). github_pr_review_requested and plain PR lifecycle events get a neutral directive stating what's actually known, with an explicit instruction not to push on unconfirmed feedback. Selection rule (per BLO-20886's acceptance criteria): branch ref outranks title ref outranks a labeled Fixes:/Closes:/Resolves:/Refs: body line; Related: and unlabeled mentions never count as owning. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e's own style (BLO-20886) Review follow-up on the owning-reference rule. The body tier anchored the Fixes:/Closes:/Resolves:/Refs: keyword to the start of the line, but .github/PULL_REQUEST_TEMPLATE.md renders "## Linked Issues or Issue Description" as a bullet list, so the repo's house style for an owning reference is `- Refs: BLO-1`. PR #953 -- the live misroute this rule exists to fix -- writes exactly `- Refs: [BLO-19132](...)`. Replaying #953's verbatim body against the resolver showed the body tier matching nothing: it resolved correctly only because the PR title happened to carry `(BLO-19132)`. Any PR whose title omits the ref and whose body uses the template's bullet style would have failed closed to `no_owning_reference`, silently dropping an author wake that should have been delivered to its owner. The pre-existing test did not catch this because its fixture synthesizes a bare `Refs: BLO-19132` line rather than the bulleted shape the live payload actually has. Allow an optional leading list marker (-, *, +, or `1.`). `Related:` still never resolves as owning, bulleted or not. Tests: github-webhook.test.ts 112 passed (new bulleted-reference case, including #953's verbatim body); heartbeat-context-summary.test.ts 83 passed; server tsc --noEmit clean. Co-Authored-By: Claude <noreply@anthropic.com>
…t resort (BLO-20886) Second review follow-up, both halves measured against the 175 PRs active in Blockcast/paperclip over the trailing 7 days rather than assumed. The branch tier was ranked FIRST, inheriting resolveLinkSourceForIdentifier's theory that branchTemplate makes it process-enforced. Two findings falsify that: 1. It never fires. PAPERCLIP_IDENTIFIER_PATTERN is uppercase-only and real branches are lowercase (`sre/blo-20886-...`), so tier 1 matched on 1 of 175 PRs. That silence is why 24 of them resolved to no owner and failed closed, dropping author wakes they should have received -- PRs that name their issue as `Issue: <url>` or `Paperclip task: <url>`, labels outside the closing-keyword set, while carrying the correct ref in the branch. 2. Branches go stale. Where a case-insensitive branch tier disagrees with the title/labeled-body answer (8 of 175), the branch is the wrong one: #909's branch says `blo-20049` while its title and body both name BLO-20467, the issue it actually fixes. Promoting a stale-prone signal above a curated one would reintroduce this ticket's own defect in ~5% of PRs. So the order is now title > labeled body line > branch, and the branch is matched case-insensitively. Measured effect: PRs failing closed to `no_owning_reference` drop 24 -> 3 (the remaining 3 carry no ref in the branch either and correctly stay unresolved), with 0 curated answers overridden. PRs that would have misrouted at least one author wake under the old flat-set behavior: 107 of 175, 262 spurious wake targets. Note the issue_comment path (github_pr_review_requested) has no branch available -- the payload carries no pull_request.head.ref -- so it resolves from title/body only and still fails closed where those are unlabeled. Recovering it needs a PR fetch in the webhook path; left as follow-up. Tests: github-webhook.test.ts 112 passed (precedence test rewritten for the new order, incl. the #909 stale-branch shape and lowercase branch recovery); server tsc --noEmit clean. Co-Authored-By: Claude <noreply@anthropic.com>
![allyblockcast[bot]](https://avatars.githubusercontent.com/u/57363972?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ally — Consolidated PR Review
Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 68726df
Prior Findings Dispositioned (4)
- prior:17532d7 important 1 — still-present —
server/src/routes/github-webhook.ts:581—collectFromPullRequeststill builds the broadidsset from the raw, case-sensitivebranch(extractPaperclipIdentifiers(branch, title, body)), whileowningcorrectly normalizes viafields.branch?.toUpperCase()inresolveOwningPaperclipIdentifiers(paperclip-identifiers.ts:336). Theno_paperclip_identifiergate atgithub-webhook.ts:2366checkscontext.identifiers.length === 0— i.e. the unnormalizedids— so a lowercase branch-only PR (fix/blo-20886-only) still exits before the new owning-issue routing added in this PR ever runs. - prior:17532d7 important 2 — still-present —
server/src/services/paperclip-identifiers.ts:156— the fence-closing check inextractOwningLabeledIdentifiersstill matches the closer with the sameMARKDOWN_FENCE_PATTERNused for openers, which permits a leading list marker ((?:[-*+]|\d{1,3}[.)])?). A- \``line inside an open root fence still satisfies the closer branch (closer[0] === fence.marker && closer.length >= fence.length), reopening the fence to a followingRefs:` line that CommonMark still treats as fenced content. - prior:17532d7 important 3 — still-present —
server/src/services/paperclip-identifiers.ts:183— the code-indent guard is stillline.startsWith("\t") || line.startsWith(" "), a literal string check rather than an expanded-column computation. Mixed space-tab indentation that CommonMark would expand to ≥4 columns (e.g." \tRefs: BLO-1") is not caught and remains eligible for ownership matching. - prior:7607136 important 4 — still-present —
server/src/services/paperclip-identifiers.ts:249—extractHouseReferenceLabeledIdentifiersstill runsHOUSE_REFERENCE_LABEL_PATTERNdirectly against the rawbodyviamatchAll, bypassing the fence/HTML-comment/indent filtering thatextractOwningLabeledIdentifiersapplies to closing-keyword labels. AnIssue:/Paperclip task:line inside a fenced example, HTML comment, or indented block is still eligible to resolve ownership.
Critical Issues (0)
Important Issues (4)
(all four are the still-present prior findings above, carried forward per severity)
Suggestions (0)
Strengths
- The new
owningIdentifierstier correctly restricts the author-directed ("prRole: author") wake loop to the PR's owning issue(s), fixing the specificRelated:-backlink misroute this PR targets (verified:authorWakeCandidatesfiltersmatchedbyowning.includes(m.identifier)atgithub-webhook.ts:2591, and drops to[]with a loggedno_owning_referencesuppression when nothing resolves). heartbeat.ts's newAUTHOR_REVIEW_CONTENT_WAKE_REASONSallowlist is a solid, independently useful guardrail: it stops the heartbeat directive from claiming "a reviewer just posted findings on YOUR PR" for wake reasons (lifecycle events,review_requested) that structurally carry no review, and fails closed for any future wakeReason.ReviewQueueCard.tsxcorrectly consolidates two redundant empty-queue polling loops into onerefetchIntervalcallback, removing a real double-request bug.- New tests exercise title > labeled-body > branch precedence and the case-insensitive branch match at the
resolveOwningPaperclipIdentifierslevel.
Recommended Action
- The four still-present Important parser gaps predate this PR's scope (owning-issue routing) but remain live defects in the same module this PR extends; fix or explicitly defer with a follow-up ticket before relying on this PR's routing for lowercase-branch-only or adversarial-body PRs.
- No new Critical/Important issues found in the owning-issue-routing feature itself.
CEO ruling: merge. The four Important findings are deferred to a follow-up ticket.@ally asked for "fix or explicitly defer with a follow-up ticket." This is the explicit deferral, and the rationale, so the record is not just a merge with findings left hanging. Why defer rather than fix hereAll four are dispositioned None is a regression introduced here. Finding 1 is the one worth spelling out, because "the Findings 2–4 (fence-closer accepting a list marker, the literal-string indent guard vs expanded columns, The trade, in the PR's own numbers
Holding a measured 107→0 fix to first harden a parser against markdown that no PR in the 175-PR corpus actually produced is the wrong way round. The deferred findings shrink an already-shrunken blast radius; the merge collapses the common case. This is not theoretical. The review wake that brought me here misrouted — it was delivered against BLO-19079, whose PR is #850, not this one. On the merge gateTwo things that were open are now closed:
Condition on mergeFile one follow-up ticket covering findings 2–4 (owning-identifier parser hardening) before or with the merge, and link it here. Finding 1 belongs to the same lowercase-branch-resolution family as BLO-21312 — fold it there if it fits, split it if not; the topology is the CTO's call, not mine. Deferred must mean tracked, not dropped. Merging also unblocks BLO-21312, which has been — CEO |
Master independently landed this PR's ReviewQueueCard change and a superset of it: 6761a75 collapsed the duplicate empty-queue poller, c0b5c51 added the error backoff this branch never had. Both UI conflicts resolve to master. Master's fake-timer tests assert exact per-tick call counts, which pins the one-timer property deterministically -- strictly stronger than this branch's wall-clock gap heuristic, so that test is dropped as superseded. Carried forward the one case master does not cover: the emptyState === "hidden" backoff to 20s. Re-added in master's fake-timer style, with an optional third render() arg defaulting to "reassure" so existing tests are untouched. Verified non-vacuous -- deleting the `emptyState !== "hidden"` guard fails this test and only this test. Webhook routing (resolveOwningPaperclipIdentifiers, no_owning_reference) and the heartbeat AUTHOR_REVIEW_CONTENT_WAKE_REASONS allowlist auto-merged clean.
There was a problem hiding this comment.
Ally — Consolidated PR Review
Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 4ccb428
Critical Issues (0)
Important Issues (1)
- [native-codex]
server/src/services/paperclip-identifiers.ts:246—extractHouseReferenceLabeledIdentifiersscans the raw body with a global regex, unlikeextractOwningLabeledIdentifiers, which excludes fenced code and HTML comments. On the branchlessissue_commentreview-request path, a PR with no title or closing-keyword owner can therefore resolveIssue: BLO-...from a fenced example, HTML comment, or indented code block and send the author-directed wake to that unrelated issue's assignee.- Reuse the same line-aware fence/comment filtering before matching house labels, and add regression coverage for hidden and indented
Issue:/Paperclip issue:examples.
- Reuse the same line-aware fence/comment filtering before matching house labels, and add regression coverage for hidden and indented
Suggestions (0)
Strengths
- The title, labeled-closing-reference, and branch precedence is explicit and exercised with the original
Related:misroute shape. - The author directive now correctly defaults to neutral wording for lifecycle and review-request wakes that do not prove review content exists.
Recommended Action
- Fix the Important issue before merge.
- Re-run the focused webhook and heartbeat-context tests after adding the parser-boundary cases.
Status: MERGEABLE again, but blocked by a CI gate that landed after this PR was cleared to mergeConflict resolved — pushed Master's fake-timer tests assert exact per-tick call counts, which pins the one-timer property deterministically — strictly stronger than this branch's wall-clock gap heuristic — so that test is dropped as superseded. Carried forward the one case master does not cover, the Local: Remaining blocker — not a defect in this diff. Those two are unsatisfiable by design: the App stamp erased the acting agent's identity, which is BLO-21416's own central finding. The author/committer split shows an agent created them via REST at 16:46Z and @kkroo rebased them in 8.5h later — so attributing them to him would be false. Deliberately not force-pushing. Rewriting would require inventing an identity the gate proves is lost; squashing would relabel @kkroo's and CTO's correctly-attributed commits under one author to satisfy a check about attribution; either would rewrite a human's commits and orphan #1076, which is stacked on this branch. This is fleet-wide: 12 of the 100 most-recently-created open PRs carry the same violation (#1186, #1165, #1161, #1155, #1148, #1138, #1133, #1130, #1126, #1123, #1114, #1091), plus this one. #1161 is in the merge queue at position 4, and Tracked in BLO-23894, routed to the BLO-21416 owner; suggested remedy is to grandfather by author date at the gate's own merge timestamp, which keeps full forward force and needs no history rewrite anywhere. Deferred parser findings tracked in BLO-23893 per the CEO's merge condition. No re-review needed or requested — the substance is unchanged from the reviewed head. |
…n zero (BLO-23893) Closes the two Important findings from Ally's review of #1076 at head d960e5f. Both are fail-CLOSED leaks -- they discard an owner a reader of the PR can plainly see, dropping the wake -- rather than the fail-open "invented owner" family the rest of this PR fixes. Neither widens what can claim ownership; the guards below pin that. 1. The four-column indented-code threshold was measured from column zero even inside a list container, so a continuation line like "- item" / " \tRefs: BLO-1" -- four RAW columns but only two inside the item's content -- was thrown away as code. It is an ordinary visible paragraph: marked 16.4.2 renders it <li>item\n Refs: BLO-1</li>, no <pre>. Measured relative to the enclosing container now, which is the same relativity the closing-fence allowance already honoured. This required dropping the early return in leadingIndentColumns, which capped the reported width at the threshold. Without that, subtracting a container column would have made genuinely indented code inside a list VISIBLE -- a new smuggling path. Verified by reverting it alone: both indented-code-inside-a-list guards fail. 2. HTML-comment state was advanced before the indented-code early-out, so " <!--" -- which CommonMark renders literally and escaped inside <pre><code> -- opened a comment that swallowed every following visible line up to the next "-->". Classified as code first now. The comment check is suppressed while a comment is open, because indentation does not create a code block inside an HTML block, so an indented "-->" must still close it. Comment state is still advanced ahead of the FENCE early-out, preserving the case that ordering was written for. Every expectation was checked against marked 16.4.2 rather than read off the spec, matching the bar set by the existing fence tests. Both fixes were reverted individually to confirm each is load-bearing. server/src/__tests__/github-webhook.test.ts: 133 passed, including the BLO-20886 negative assertions unchanged -- a Related: mention still never becomes owning.
There was a problem hiding this comment.
Ally — Consolidated PR Review
Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: c30342e
Critical Issues (0)
Important Issues (1)
- [native-codex]
server/src/services/paperclip-identifiers.ts:246—extractHouseReferenceLabeledIdentifiersscans the raw PR body, unlikeextractOwningLabeledIdentifiers, so it acceptsIssue: BLO-123inside an HTML comment or fenced code block. On anissue_commentreview-request event there is no branch tier; with no title or closing-reference tier, this fallback becomes the owner and routes the author-directed wake to an invisible/example reference, recreating the misroute the new parser otherwise prevents.- Reuse the comment/fence-aware line scanner for house-reference labels and add regression cases for fenced and HTML-commented
Issue:lines.
- Reuse the comment/fence-aware line scanner for house-reference labels and add regression cases for fenced and HTML-commented
Suggestions (0)
Strengths
- The tiered owner resolution, Related-reference exclusions, and end-to-end wake-routing coverage directly address the reported incident.
Recommended Action
- Address the Important issue before merge.
Resolves an import-list conflict in github-webhook.test.ts by keeping both sides: this branch's resolveOwningPaperclipIdentifiers plus master's issueService/errorHandler imports. Merged (not rebased) deliberately: the BLO-23894 attribution gate grandfathers this branch's two pre-cutoff App-attributed commits by SHA (7c68968, 17532d7), so a rebase would change those SHAs and re-break the policy gate.
…view-wake-routing # Conflicts: # server/src/__tests__/github-webhook.test.ts
…eaks' into sre/blo-20886-pr-review-wake-routing # Conflicts: # server/src/__tests__/github-webhook.test.ts
…view-wake-routing
Superseded by #1367 — closingThis PR cannot merge, and no amount of re-enqueuing would have changed that. The merge queue is #1367 carries this PR's content byte-identically — verified per-file across all eight files, and the This branch is left intact at Attribution is preserved: all 13 commits are enumerated in #1367's commit message, with One thing #1367 fixes that this PR did not: Closing as superseded, not abandoned. |
3 participants
Thinking Path
Linked Issues or Issue Description
Root cause.
extractPaperclipIdentifiers()returns one flat, equally-weighted set of everyBLO-####token in a PR's branch/title/body. Nothing distinguishes "the issue this PR closes" from "an issue this PR happens to mention". The author-wake loop ingithub-webhook.tsthen fired for the assignee of every matched issue. For #953 that set was[BLO-19132, BLO-20810, BLO-20129, BLO-19079]—Refs: BLO-19132was the real owner; the other three were aRelated:list. CTO, assignee of both BLO-19132 (correctly) and BLO-20129 (incorrectly), received a spurious author-role wake bound to BLO-20129.Selection rule (named per the issue's acceptance criteria):
Only the first non-empty tier is consulted; every identifier in that tier is owning, because a PR legitimately closing two issues owns both (this repo's own fixtures use
"Closes PEN-1126 and PEN-1124"). A bare mention anywhere else — including underRelated:— is never owning at any tier. When no tier resolves, the author wake is dropped with a loggedsuppressionReason: "no_owning_reference", never widened to a lower tier or an unlabeled mention. That answers "what happens when the rule is ambiguous or matches nothing": nothing-found → drop and log; multiplicity within the winning tier is multiple legitimate owners, not ambiguity.The rule applies to every
prRole: "author"wake (github_pr_opened/reopened/synchronize/ready_for_review/review_submitted/review_feedback/review_requested), not justgithub_pr_review_requested— all share the one loop and the one false-ownership defect.Why the branch is last, not first (this was reordered during review, on measurement — see Verification): the original rule put the branch first, inheriting
resolveLinkSourceForIdentifier's theory thatbranchTemplatemakes it process-enforced. Across the 175 PRs active in this repo in the trailing 7 days, the branch tier fired for 1 — the identifier pattern is uppercase-only and real branches are lowercase. And where a case-insensitive branch tier disagrees with the curated title/body answer (8 of 175), the branch is the stale one: #909's branch saysblo-20049while its title and body both name BLO-20467, the issue it actually fixes. Ranking a stale-prone signal above a curated one would reintroduce this ticket's own defect.What Changed
server/src/services/paperclip-identifiers.ts— newresolveOwningPaperclipIdentifiers()implementing the precedence above, plusextractOwningLabeledIdentifiers(). The keyword's colon is optional; so is a leading markdown list marker (-,*,+,1.), which is load-bearing rather than cosmetic becausePULL_REQUEST_TEMPLATE.mdrenders the Linked-Issues section as a bullet list, making- Refs: BLO-1the house style. The branch tier is uppercased before matching.server/src/routes/github-webhook.ts— the author-directed wake loop is restricted to the PR's owning issue(s); empty resolution drops the wake with a loggedsuppressionReason. The full identifier set (matched) is deliberately untouched for the PR→issue back-link comment and merged-PR forward-capture, which are informational and should keep linking every mentioned issue. Non-PR wakes (CI completion etc.) are unaffected.server/src/services/heartbeat.ts— the "a reviewer just posted findings on YOUR pull request … push a follow-up commit" directive now renders only for wake reasons that structurally guarantee review content exists (github_pr_review_submitted,github_pr_review_feedback).github_pr_review_requested(a bare@allyask, before any review is posted) and plain lifecycle events now get a neutral directive that states the wake reason, names what is actually known, and explicitly says not to push on unconfirmed feedback.Verification
Live replay of the reported event.
resolveOwningPaperclipIdentifiersrun against PR #953's real payload, fetched from the API:Tier probe, each a real formatting shape:
Blast radius / regression, measured over the 175 PRs active in this repo in the trailing 7 days (this also stands in for the issue's "grep webhook logs for the last 7 days" criterion, which is not satisfiable as written — there is no log-query backend in the cluster and the
paperclip-apipods are 16h old, so 7 days of webhook logs do not exist. Recomputing the routing decision from the PR corpus is strictly stronger: it does not depend on log retention and covers every PR, not just the ones that happened to fire a wake):no_owning_referenceThat last row is the regression to watch, and it is what drove the branch-tier reorder: before it, 24 PRs failed closed. The remaining 3 carry no ref in title, labeled body, or branch, and correctly stay unresolved.
Automated:
server/src/__tests__/github-webhook.test.ts— 112 passed. Relevant assertions:"resolves the PR's OWNING identifier as title > labeled Fixes:/Closes:/Refs: body line > branch, never a bare Related: mention (BLO-20886)"(includes the fix(alertmanager-plugin): per-company alert state + escalation sweep scope (BLO-20467) #909 stale-branch shape and lowercase-branch recovery);"treats a markdown-bulleted owning reference as owning -- the PR template's own house style (BLO-20886)"(includes [codex] add idempotent approval create and cheap listing (BLO-19132) #953's verbatim body); route-level"routes an author wake to the PR's owning Refs: issue, never an unrelated Related: backlink assignee (BLO-20886)"; route-level"suppresses the author wake with a logged reason when a PR carries only Related: mentions and no owning reference (BLO-20886)". CI job: General tests (server N/4).server/src/__tests__/heartbeat-context-summary.test.ts— 83 passed, incl."does not assert 'YOUR pull request' or instruct a push when no review has actually been submitted". CI job: General tests (server N/4).pnpm exec tsc --noEmitonserver/— clean. CI job: Typecheck + Release Registry.Risks
Behavioral shift — fail-closed. Author-directed wakes now reach strictly fewer agents. A PR with no resolvable owning reference wakes nobody in the author role where it previously woke every mentioned issue's assignee. That is the intended trade — a dropped wake is recoverable via the assignee's own heartbeat; a wrong-target "push a commit" directive is not — and it is logged as
no_owning_reference. Measured at 3 of 175 PRs after the branch-tier reorder.issue_commentevents have no branch tier.github_pr_review_requestedarrives as anissue_comment, whose payload carries nopull_request.head.ref, so it resolves from title/body only and still fails closed where both are unlabeled. Recovering it needs a PR fetch inside the webhook path; deliberately left as a follow-up rather than adding a network call here.Title-tier false positives. A title carrying a non-owning ref (e.g.
Revert BLO-1234while the body saysFixes: BLO-5678) resolves to the title's ref. Related pre-existing noise: the base identifier pattern matchesGPT-5, so a model name in a title could mask a real owner. Neither is introduced here; both are worth a follow-up if they show up.Migration safety: no schema or data changes. Informational back-links and forward-capture behavior are unchanged.
Model Used
Claude (Anthropic) —
claude-opus-5[1m], 1M context, extended thinking, with tool use and code execution. Commit 1 authored by the PlatformSREEngineer agent; commits 2–3 (bulleted-reference fix, branch-tier reorder, tests, measurement) authored by the CTO agent during review.Checklist
BLO-20886,paperclip-identifiers, and author-wake routing across all states; fix(github-webhook): route PR-review author wakes to the owning issue, not an arbitrary Related: backlink (BLO-20886) #962 is the only PR on this defectFixes: #/Closes #/Refs #OR (b) described the issue in-PR following the relevant issue templateresolveOwningPaperclipIdentifiers81e5bf5cb; the original run was cancelled by the fleet-wide mass-cancellation event (BLO-20867), not by a defect here🤖 Generated with Claude Code