fix(github-webhook): resolve PR owner via house-reference label when the branch tier is unreachable (BLO-21312)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• The GitHub webhook receiver maps a PR event to the Paperclip issue(s) it concerns and, for an author-directed wake, restricts that to the PR's owning issue (BLO-20886 / fix(github-webhook): route PR-review author wakes to the owning issue, not an arbitrary Related: backlink (BLO-20886) #962): title > labeled Fixes:/Closes:/Resolves:/Refs: body line > branch ref (case-insensitive, last resort)
• github_pr_review_requested — the exact wake reason BLO-20886 was filed over — arrives via issue_comment (an @ally review mention), not pull_request, and that payload carries no pull_request.head.ref
• So the branch tier is structurally unreachable on this path, not merely unmeasured: a PR naming its owner only via a non-closing house label (Issue:, Paperclip task:, Paperclip issue:, Paperclip QA task: — real shapes on test(ci): run all configured adapter projects #931, [codex] bound serialized dispatcher critical scans #963, test(ci): verify merged adapter artifact on ARC #976, fix(pipelines): retire exited stage automation issues #916) still failed closed to no_owning_reference, dropping the author wake
• This pull request adds a fourth, lowest-priority tier to resolveOwningPaperclipIdentifiers for these house labels, so the issue_comment path has a fallback below the closing-keyword tier
• The benefit is fewer silently-dropped author wakes on the review-request path, without touching how pull_request events (which do carry a branch) resolve today

Linked Issues or Issue Description

• Fixes: BLO-21312
• Follow-up from: BLO-20886 / fix(github-webhook): route PR-review author wakes to the owning issue, not an arbitrary Related: backlink (BLO-20886) #962 (this PR is stacked on that branch — see Verification)

Option chosen: #2 from the issue (widen the labeled-reference keyword set), not #1 (fetch the PR via API on issue_comment to recover head.ref). Fetching would turn resolveEventContext — a pure, synchronously-tested function — into an async network call inside the webhook request path, exactly the cost the issue calls out as the reason #962 didn't do it. Ranking mitigates the "weaker claim than a closing keyword" concern the issue raises: this tier sits below both the closing-keyword tier and the branch tier, so it only ever activates when title, closing keyword, and branch (when available) are all empty.

What Changed

• resolveOwningPaperclipIdentifiers (server/src/services/paperclip-identifiers.ts) gains a 4th tier, extractHouseReferenceLabeledIdentifiers, matching body lines labeled Issue:/Paperclip task:/Paperclip issue:/Paperclip QA task: (optional bullet marker, colon, case-insensitive) — ranked last, below the case-insensitive branch tier.
• Updated the docblock and the stale issue_comment comment in github-webhook.ts that said "no branch tier here either" without naming the new fallback.
• Added a unit test (resolveOwningPaperclipIdentifiers) covering all four observed house-label shapes, the ranking below both closing-keyword and branch, and that a bare Related: mention still never becomes owning even alongside a house label in the same body.
• Added a route-level test firing an issue_comment @ally review event for a PR (modeled on the real fix(pipelines): retire exited stage automation issues #916 shape) whose title/body carry no closing-keyword reference, only a Paperclip issue: house label plus an unrelated Related: mention — asserting the author wake resolves to the house-label owner and never the Related: issue.

Verification

• npx vitest run src/__tests__/github-webhook.test.ts (from server/) — 114/114 passed, including the 2 new tests and all pre-existing BLO-20886 assertions unchanged.
• npx tsc --noEmit — clean.
• Manual corpus measurement (BLO-21312 AC), 200 PRs updated in the trailing 7 days (repo:Blockcast/paperclip is:pr updated:>=2026-07-28):
  • issue_comment path (title + body only, branch never available — the actual gap this PR closes): no_owning_reference 49/200 → 38/200 (11 PRs recovered: feat(alertmanager): harden issue intake creation #984, test(ci): verify merged adapter artifact on ARC #976, [codex] bound serialized dispatcher critical scans #963, fix(heartbeat): honor Ally concurrency 15 #936, [codex] land deterministic queued-run dispatch #933, test(ci): run all configured adapter projects #931, fix(heartbeat): classify external OOM kills #927, feat(alertmanager): make issue intake aggregate-safe #923, fix(pipelines): retire exited stage automation issues #916, test(ci): serialize destructive heartbeat cleanup #914, Treat penstock 400 allocation_missing as a retryable transient upstream fault #889).
  • pull_request path (title + body + branch): no_owning_reference 25/200 → 25/200, 0 changed — confirms the house-label tier is inert wherever the branch tier already resolves, i.e. this is additive only on the branch-less path, not a behavior change for fix(github-webhook): route PR-review author wakes to the owning issue, not an arbitrary Related: backlink (BLO-20886) #962's measured path.
• This PR is stacked on the still-open sre/blo-20886-pr-review-wake-routing (fix(github-webhook): route PR-review author wakes to the owning issue, not an arbitrary Related: backlink (BLO-20886) #962), since resolveOwningPaperclipIdentifiers doesn't exist on master yet. Base should be repointed to master (or this PR merged after fix(github-webhook): route PR-review author wakes to the owning issue, not an arbitrary Related: backlink (BLO-20886) #962) once fix(github-webhook): route PR-review author wakes to the owning issue, not an arbitrary Related: backlink (BLO-20886) #962 lands. fix(github-webhook): route PR-review author wakes to the owning issue, not an arbitrary Related: backlink (BLO-20886) #962's own CI has one unrelated flaky failure (heartbeat-queued-backlog-convergence.test.ts, BLO-20396 lane) — confirmed unrelated to the BLO-20886/BLO-21312 diff.

Risks

• Low behavioral risk: the new house-label tier is ranked below title, closing-keyword body references, and branch references, so it only fills the branch-less issue_comment gap.
• Main risk is accidental ownership from a supported house label in a PR body; mitigated by not treating Related: as owning and by tests covering ranking.

Model Used

• Claude Code generated the original change; Codex CLI (GPT-5 coding agent) refreshed this PR metadata while triaging the review gate.

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
• I have updated relevant documentation to reflect my changes
• 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

🤖 Generated with Claude Code

[allyblockcast]

🔗 Paperclip issue: BLO-20396
🔗 Paperclip issue: BLO-21312
🔗 Paperclip issue: BLO-20886

[allyblockcast]

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

Missing or incomplete:

• 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 — Consolidated PR Review

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

Important Issues (2)

• [pr-review-toolkit / gstack / native-codex] server/src/services/paperclip-identifiers.ts:126 — The new house-label regex makes the colon optional, so ordinary prose beginning with Issue is treated as an ownership label. For example, Issue filed a related bug, see BLO-1 and Issue description for BLO-2 both match and route a branchless review-request wake to those issues. This directly contradicts the adjacent comment's own non-owning example and can wake the wrong assignee.
  • Require the colon for these weaker house-reference labels, and add negative tests for ordinary Issue ... BLO-N prose.
• [gstack / native-codex] server/src/services/paperclip-identifiers.ts:136 — Every identifier in the captured remainder becomes owning, including explicitly unrelated references on the same line. Issue: BLO-1; Related: BLO-2 resolves both identifiers and wakes both assignees, bypassing the stated guarantee that a Related: mention never becomes owning.
  • Parse only the house label's direct reference value, or fail closed when the weak fallback line contains additional labeled references; add this same-line counterexample to the tests.

Strengths

• The fallback is correctly ranked below title, closing-keyword, and branch evidence.
• The route-level test verifies the intended branchless issue_comment path and protects the separate-line Related: case.

Recommended Action

1. Address both Important routing issues before merge.
2. Add the negative parser cases above so this weak fallback remains fail-closed.

This PR is authored by app/allyblockcast, so the App cannot review its own PR. The exact head ed9120aad1c20c01719fdce7ea0167197b4775b9 must be reopened under an independent author before an App approval is possible; the shared merge-token user is not a substitute for the App gate.

[allyblockcast]

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

Missing or incomplete:

• 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

[kkroo]

Reviewed BLO-21312 house-reference fallback on top of current #962 stack. Restacked onto sre/blo-20886-pr-review-wake-routing, fixed PR template gate, and verified github-webhook/context tests plus server typecheck.

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (2)

• prior:ed9120a important 1 — fixed — server/src/services/paperclip-identifiers.ts:155 — the house-label pattern now requires a colon, and the new negative tests cover ordinary Issue ... BLO-N prose.
• prior:ed9120a important 2 — still-present — server/src/services/paperclip-identifiers.ts:165 — the new truncation guard only recognizes a secondary label after ;, ,, or |; Issue: BLO-1 Related: BLO-2 therefore still passes both identifiers to the owning extractor.

Important Issues (2)

• [prior:ed9120a important 2 / pr-review-toolkit / gstack / native-codex] server/src/services/paperclip-identifiers.ts:165 — The same-line non-owning-reference fix remains delimiter-dependent. A valid Markdown line such as Issue: BLO-1 Related: BLO-2 has no comma/semicolon/pipe, so stripTrailingLabelReference does not truncate it and both issues become owners, preserving the wrong-assignee wake this guard is intended to prevent.
  • Match the whitespace boundary already supported by TRAILING_NON_OWNING_LABEL_PATTERN, or parse only the direct house-label value; add a no-punctuation same-line regression test.
• [pr-review-toolkit / gstack / native-codex] server/src/services/paperclip-identifiers.ts:177 — The house-reference extractor scans the raw body with a multiline regex and does not apply the fenced-code or four-space/tab-indented-code exclusions used by extractOwningLabeledIdentifiers. A PR body containing a documentation example such as a fenced Issue: BLO-1 can therefore become ownership evidence on the branchless review-request path and wake the example issue assignee.
  • Reuse the existing line/fence-aware parsing path (or equivalent preprocessing) for house labels, and add fenced plus indented-code negative tests.

Strengths

• The fallback remains correctly ranked below title, closing-keyword body references, and branch references.
• The mandatory-colon change closes the ordinary-prose ambiguity, and the route-level test exercises the intended branchless issue_comment path.
• Current GitHub checks report both review and security-review passing.

Recommended Action

1. Keep the prior same-line finding open until secondary labels are rejected regardless of punctuation.
2. Exclude Markdown code examples before treating a house label as ownership evidence.

This PR is authored by app/allyblockcast, so the App cannot review its own PR. The exact head 950689262abea01d2c18a100763e2a41917f44a4 must be reopened under an independent author before an App approval is possible; the shared merge-token user is not a substitute for the App gate.

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (2)

• prior:ed9120a important 2 — still-present — server/src/services/paperclip-identifiers.ts:238 — the trailing-label guard still requires comma, semicolon, or pipe punctuation, so Issue: BLO-1 Related: BLO-2 leaves both identifiers in the owning value; the same vocabulary also omits the already-recognized non-owning Supersedes: label.
• prior:9506892 important 1 — still-present — server/src/services/paperclip-identifiers.ts:250 — the house-label extractor still scans the raw body with matchAll, bypassing the fence, indented-code, and HTML-comment filtering now used by the closing-keyword extractor.

Important Issues (2)

• [prior:ed9120a important 2 / pr-review-toolkit / gstack / native-codex] server/src/services/paperclip-identifiers.ts:238 — Same-line non-owning labels can still be promoted to owners. The current pattern does not truncate Issue: BLO-1 Related: BLO-2 because there is no punctuation, and it does not truncate Issue: BLO-1; Supersedes: BLO-2 because Supersedes: is absent from its label vocabulary. Both forms resolve both identifiers and can wake the wrong assignee.
  • Reuse the existing whitespace-aware TRAILING_NON_OWNING_LABEL_PATTERN vocabulary, or parse only the direct house-label value, then add no-punctuation and Supersedes: regressions.
• [prior:9506892 important 1 / pr-review-toolkit / gstack / native-codex] server/src/services/paperclip-identifiers.ts:250 — House references inside rendered-nonclaim content still become ownership evidence. Fenced examples, four-space/tab-indented code, and multiline HTML comments are scanned as raw lines, so an invisible or illustrative Issue: BLO-N can route a real review-request wake.
  • Share the existing line/fence/comment-aware scanner with the house-label tier and add fenced, indented, inline-comment, multiline-comment, and unterminated-comment negative tests.

Strengths

• The fallback remains correctly ranked below title, closing-keyword, and branch evidence.
• The mandatory-colon guard and current route-level test protect the intended branchless issue_comment path.
• Current review and security-review checks pass.

Recommended Action

1. Keep both Important findings blocking until the parser rejects secondary labels without punctuation and ignores non-rendered Markdown content.
2. Add the missing negative cases to the house-label tests before merge.

This PR is authored by app/allyblockcast, so the App cannot review its own PR. The exact head 801f24ba1d81eefcf951104ca87d6466165413a5 must be reopened under an independent author before an App approval is possible; the shared merge-token user is not a substitute for the App gate.