Skip to content

Commit cd3efc5

Browse files
authored
fix(buds): stop unrelated PRs leaking onto PROD / UAT tabs (#193)
* fix(buds): stop unrelated PRs leaking onto PROD / UAT tabs The release-stage tabs (PROD, UAT) were showing every open PR raised to the configured branch on any impacted repo — including PRs linked to a different BUD entirely. Root cause was the OR-of-equalities predicate in ``list_open_for_bud_with_repo`` which let ``repo_id IN impacted`` bypass the BUD link. Two coordinated fixes: 1. **Broaden the BUD-NNN resolver.** ``extract_bud_number`` now scans PR titles in addition to head branches, with a negative-lookbehind regex that rejects mid-word and run-on matches (``auth-bud-7``, ``abcbud-5``) while accepting the punctuation real titles use (``Closes #BUD-4``, ``fix,BUD-3``, ``revert: BUD-21 cleanup``). The webhook handler now also processes ``pull_request.edited`` events gated on ``changes.title``, so retitling an orphan PR to ``[BUD-008] X`` relinks it on the next webhook. Old + new BUD both get audit timeline events. The duplicate regex in ``mcp.handlers_hooks`` is removed in favour of the single seam. 2. **Tighten the stage-tab query.** ``list_open_for_bud_with_repo`` now uses ``bud_id == X OR (bud_id IS NULL AND repo_id IN impacted)``. The IS-NULL arm preserves aggregate release PRs (``develop → main``) that legitimately carry no single owning BUD; PRs linked to a different BUD on the same impacted repo no longer leak. Adds ``scripts.backfill_pr_bud_links`` to re-link existing orphan PRs using the broadened resolver. Defaults to open PRs only; an explicit ``--include-merged`` opts in to merged-PR backfill behind a CONFIRM prompt because the next release-detection walk can fire new ``merged_to_{stage}`` timeline events and auto-close BUDs that were previously orphaned in the release chain. Tests: 5 parametrized-accept and 8 parametrized-reject cases pin the matcher; predicate-shape tests pin the three-way SQL form. Signed-off-by: Arun Rajkumar <mickyarunr@gmail.com> * fix(buds): release-stage tabs filter open PRs by BUD-number reference The previous over-match fix preserved aggregate release PRs by keeping ``bud_id IS NULL`` rows on impacted repos visible — but that pulled in unrelated PRs to ``main`` / ``release/*`` that simply happened to share a repo with the BUD (e.g. an ATOA-9396 PR appearing on BUD-004's PROD tab even though it has no BUD reference at all). Real semantics: a PR belongs on a BUD's release-stage tab only when its head ref or title carries that BUD's number. Multi-BUD release branches like ``release/bud-001-bud-004-bud-007`` should surface on EACH referenced BUD's tab, not on every impacted-repo BUD. Adds two helpers in ``pr_auto_transition``: - ``extract_all_bud_numbers(text)`` returns every BUD number found. - ``pr_references_bud(num, head, title)`` is the call site. Loosens the matcher's left lookbehind so digit-then-hyphen prefixes are allowed (``-bud-`` after a digit is the multi-BUD chain pattern), while keeping letter-then-hyphen prefixes rejected (``auth-bud-7`` mid-word case is still rejected). The release-stage filter in ``bud_prs.py`` now applies the content guard to any PR that isn't directly linked to this BUD — directly linked PRs short-circuit so a user-edited link wins over text matching. Tests pin the multi-BUD branch parser and the user's exact reported case (``release/bud-001-bud-004`` matches BUD-1 and BUD-4 but not BUD-7; unrelated ``ATOA-9396`` PR no longer leaks onto BUD-4's tab). Signed-off-by: Arun Rajkumar <mickyarunr@gmail.com> * fix(buds): sync base_branch / head_branch on pull_request.edited The edited webhook handler only synced ``title`` — base-branch and head-branch changes from GitHub were silently dropped. When a PR was retargeted from ``main`` to ``develop`` (the BUD-004 / PR-1997 case), the local row kept ``base_branch = main`` and the release-stage filter kept matching the PR onto the PROD tab because ``branch_matches`` runs against the stale value. Two coordinated changes: * Dispatcher widens the ``edited`` action gate. We now act when the payload's ``changes`` carries ``title`` OR ``base`` — the two fields that affect either the BUD link or the release-stage filter. Body / label edits stay silent. * Handler also assigns ``pr.base_branch`` and ``pr.head_branch`` from the latest pr_data on every invocation. Cheap, and prevents the same drift on future force-pushes or base-branch edits. Does NOT retroactively fix already-stale rows: an operator-triggered sync script or a webhook replay is needed for those. The unit test also pins that the loose space-form title ``Bud 004/processing loader`` does NOT satisfy ``pr_references_bud`` on its own; the head branch ``bud-004/...`` is the reliable link source. Signed-off-by: Arun Rajkumar <mickyarunr@gmail.com> * fix(buds): tighten BUD-NNN matcher + guard relink to title-only edits Code review of the post-merge fixes flagged two real regressions and one auto-transition risk: * The relaxed regex ``(?<![A-Za-z])(?<![A-Za-z]-)`` accepted ``1bud-3``, ``sha7bud-5``, ``v2bud-5``, and ``auth1-bud-7`` — digit-glued mid-word and prefix-with-trailing-digit shapes that are not real BUD references. Revert to the strict ``(?<![A-Za-z0-9-])`` prefix that uniformly rejects letters, digits, and hyphens on the left. Multi-BUD release branches are now handled by a SEPARATE chain matcher: ``extract_all_bud_numbers`` finds each strict ``bud-NNN`` match and then walks forward through any ``-bud-NNN-bud-NNN`` tail. Six new parametrize-reject cases pin the digit-glued and trailing-digit-prefix rejections. * ``_handle_pr_edited`` previously called ``resolve_bud_from_pr`` on every invocation. Now that the dispatcher also fires on base edits, unconditional re-resolution risked unlinking a PR whose title carried a once-accepted-now-rejected reference (the regex tightening is exactly that kind of contract change). Gate the relink to actual title changes; base / head edits still sync ``pr.base_branch`` and ``pr.head_branch`` but do not touch ``pr.bud_id``. Auto-transition audit confirmed clean: * ``check_all_repos_have_prs`` / ``check_all_prs_merged`` key off ``pr.bud_id``, not the text content — the matcher tightening only affects NEW PR-open / title-edit links, not existing rows. * The development → code_review threshold is unaffected by base edits because the gate above blocks the only path that could have unlinked an existing link on a base-only edit. Full backend suite green: 1887 passed. Signed-off-by: Arun Rajkumar <mickyarunr@gmail.com> --------- Signed-off-by: Arun Rajkumar <mickyarunr@gmail.com>
1 parent 49aec62 commit cd3efc5

8 files changed

Lines changed: 812 additions & 40 deletions

File tree

backend/app/api/v1/bud_prs.py

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
ReleaseTimelineEvent,
3636
)
3737
from app.schemas.pull_request import PRChecklistItem, PullRequestRead
38+
from app.services.pr_auto_transition import pr_references_bud
3839
from app.utils.branch_matching import branch_matches
3940

4041
router = APIRouter()
@@ -232,18 +233,31 @@ async def get_bud_release_stage(
232233
target_branch = (
233234
(repo.uat_branch if typed_stage == "uat" else repo.main_branch) if repo else None
234235
)
235-
if target_branch and branch_matches(pr.base_branch, target_branch):
236-
seen_pr_ids.add(pr.github_pr_id)
237-
open_prs.append(
238-
ReleasePR(
239-
pr_number=pr.github_pr_number,
240-
repo_name=repo.name if repo else "",
241-
html_url=pr.html_url,
242-
title=pr.title,
243-
author_login=pr.author_github_login,
244-
merged_at=None,
245-
),
246-
)
236+
if not (target_branch and branch_matches(pr.base_branch, target_branch)):
237+
continue
238+
# Content guard for unlinked PRs (``bud_id IS NULL``): the
239+
# release-PR fallback in the repository surfaces every PR
240+
# targeting the stage branch on an impacted repo, but the
241+
# release-stage tab must only show PRs that reference THIS BUD.
242+
# ``pr_references_bud`` looks for ``bud-NNN`` in the head ref
243+
# or title, so a release branch like
244+
# ``release/bud-001-bud-004`` correctly appears on both BUD-001
245+
# and BUD-004 tabs, while an unrelated PR to ``main`` does not
246+
# leak through. Directly-linked PRs (``bud_id == bud_id``)
247+
# short-circuit so the user-edited link wins over text matching.
248+
if pr.bud_id != bud_id and not pr_references_bud(bud.bud_number, pr.head_branch, pr.title):
249+
continue
250+
seen_pr_ids.add(pr.github_pr_id)
251+
open_prs.append(
252+
ReleasePR(
253+
pr_number=pr.github_pr_number,
254+
repo_name=repo.name if repo else "",
255+
html_url=pr.html_url,
256+
title=pr.title,
257+
author_login=pr.author_github_login,
258+
merged_at=None,
259+
),
260+
)
247261

248262
return BUDReleaseStage(
249263
bud_id=str(bud_id),

backend/app/mcp/handlers_hooks.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121

2222
import asyncio
2323
import os
24-
import re
2524
import uuid
2625

2726
import structlog
@@ -35,12 +34,15 @@
3534
from app.schemas.dev_activity import DevActivityHookRequest, DevActivityHookResponse
3635
from app.services.colyseus_bridge import publish_to_colyseus
3736
from app.services.event_bus import publish
37+
from app.services.pr_auto_transition import extract_bud_number
3838
from app.services.user_resolution import resolve_user_by_email
3939
from app.services.xp_service import check_and_award_streak
4040

4141
logger = structlog.get_logger(__name__)
4242

43-
_BUD_BRANCH_RE = re.compile(r"^bud-(\d+)/")
43+
# BUD number extraction is owned by ``pr_auto_transition.extract_bud_number``
44+
# so a single matcher governs PR linking, the dev-activity hook below, and any
45+
# future reuse — keeping branch heuristics from drifting between call sites.
4446

4547
# Strong references to in-flight Colyseus publish tasks. ``asyncio.create_task``
4648
# only holds a weak reference to the task it returns, so without an external
@@ -297,13 +299,11 @@ async def _resolve_bud(
297299
return bud.id, bud.bud_number
298300

299301
# Auto-detect from branch name: bud-001/feature → 1
300-
if branch:
301-
match = _BUD_BRANCH_RE.match(branch)
302-
if match:
303-
detected_num = int(match.group(1))
304-
bud = await bud_repo.get_by_number(detected_num)
305-
if bud:
306-
return bud.id, bud.bud_number
302+
detected_num = extract_bud_number(branch)
303+
if detected_num is not None:
304+
bud = await bud_repo.get_by_number(detected_num)
305+
if bud:
306+
return bud.id, bud.bud_number
307307

308308
return None, None
309309

backend/app/repositories/pull_request.py

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
import uuid
1818
from datetime import datetime
1919

20-
from sqlalchemy import func, or_, select
20+
from sqlalchemy import and_, func, or_, select
2121
from sqlalchemy.ext.asyncio import AsyncSession
2222

2323
from app.models.pull_request import PRState, PullRequest
@@ -172,22 +172,40 @@ async def list_open_for_bud_with_repo(
172172
) -> list[tuple[PullRequest, TrackedRepository | None]]:
173173
"""List open PRs for a BUD joined with their tracked repository.
174174
175-
When ``impacted_repo_ids`` is provided, the result includes any open
176-
PR that is either linked to ``bud_id`` directly OR targets one of
177-
the impacted repos. Used by release-stage views that need to surface
178-
open PRs in repos affected by the BUD even if the PR forgot to set
179-
a ``bud_id``.
175+
Three-way predicate so the release-stage views (UAT / PROD tabs)
176+
only surface PRs that genuinely relate to ``bud_id``:
177+
178+
* ``bud_id == X`` — the PR is directly linked to this BUD.
179+
* ``bud_id IS NULL AND repo_id IN impacted_repo_ids`` — aggregate
180+
release PRs like ``develop → main`` legitimately carry no single
181+
owning BUD; we keep them visible on the impacted repo's stage tab
182+
because the SHA-walk in the release detector uses them to attribute
183+
merges back to multiple BUDs.
184+
185+
A plain ``OR(bud_id == X, repo_id IN impacted)`` would also let
186+
through PRs linked to a **different** BUD that happens to touch the
187+
same impacted repo — which is the over-matching bug this method now
188+
prevents.
180189
181190
Args:
182191
bud_id: The BUD UUID to filter on.
183-
impacted_repo_ids: Additional repo UUIDs to include open PRs for.
192+
impacted_repo_ids: Repo UUIDs whose unlinked release PRs should
193+
stay visible. When ``None`` / empty, only directly-linked
194+
PRs are returned.
184195
185196
Returns:
186197
List of ``(PullRequest, TrackedRepository | None)`` tuples.
187198
"""
188-
filters = [PullRequest.bud_id == bud_id]
189199
if impacted_repo_ids:
190-
filters.append(PullRequest.repo_id.in_(impacted_repo_ids))
200+
bud_predicate = or_(
201+
PullRequest.bud_id == bud_id,
202+
and_(
203+
PullRequest.bud_id.is_(None),
204+
PullRequest.repo_id.in_(impacted_repo_ids),
205+
),
206+
)
207+
else:
208+
bud_predicate = PullRequest.bud_id == bud_id
191209

192210
stmt = self._scoped(
193211
select(PullRequest, TrackedRepository)
@@ -198,7 +216,7 @@ async def list_open_for_bud_with_repo(
198216
)
199217
.where(
200218
PullRequest.state == PRState.OPEN,
201-
or_(*filters),
219+
bud_predicate,
202220
)
203221
)
204222
result = await self._db.execute(stmt)

backend/app/services/github_webhook_handler.py

Lines changed: 119 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
Triggers auto-transitions when all impacted repos have PRs or all merge.
2020
"""
2121

22-
import re
2322
import uuid
2423
from typing import Any
2524

@@ -43,7 +42,7 @@
4342
from app.services.pr_auto_transition import (
4443
check_all_prs_merged,
4544
check_all_repos_have_prs,
46-
resolve_bud_from_branch,
45+
resolve_bud_from_pr,
4746
)
4847
from app.services.release_detection import detect_release_promotion
4948
from app.services.stage_award import award_stage_xp_to_contributors
@@ -52,8 +51,6 @@
5251

5352
logger = structlog.get_logger(__name__)
5453

55-
_BUD_BRANCH_RE = re.compile(r"bud-0*(\d+)/", re.IGNORECASE)
56-
5754

5855
async def handle_github_event(
5956
org_id: uuid.UUID,
@@ -79,6 +76,16 @@ async def handle_github_event(
7976
await _handle_pr_closed(org_id, repo, pr_data, db)
8077
elif action == "synchronize":
8178
await _handle_pr_synchronize(org_id, pr_data, db)
79+
elif action == "edited":
80+
# GitHub's ``edited`` action fires on every body / label / title /
81+
# base / head change. We act on the subset that affects either
82+
# the BUD link (title edits relink to a different BUD-NNN) or
83+
# the release-stage filter (base change moves the PR off the
84+
# tab that watches ``main`` / ``release/*``). Body and label
85+
# edits stay silent to keep the log quiet.
86+
changes = payload.get("changes") or {}
87+
if changes.keys() & {"title", "base"}:
88+
await _handle_pr_edited(org_id, pr_data, db)
8289
elif event_type == "pull_request_review" and action == "submitted":
8390
review = GitHubReview.model_validate(payload["review"])
8491
pr_data = GitHubPullRequest.model_validate(payload["pull_request"])
@@ -104,7 +111,7 @@ async def _handle_pr_opened(
104111
db: AsyncSession,
105112
) -> None:
106113
"""Create PR record and check if all impacted repos have PRs."""
107-
bud_id, bud = await resolve_bud_from_branch(db, org_id, pr_data.head.ref)
114+
bud_id, bud = await resolve_bud_from_pr(db, org_id, pr_data.head.ref, pr_data.title)
108115

109116
pr_repo = PullRequestRepository(db, org_id=org_id)
110117
existing = await pr_repo.get_by_github_pr_id(pr_data.id)
@@ -408,6 +415,113 @@ async def _handle_pr_synchronize(
408415
await db.commit()
409416

410417

418+
async def _handle_pr_edited(
419+
org_id: uuid.UUID,
420+
pr_data: GitHubPullRequest,
421+
db: AsyncSession,
422+
) -> None:
423+
"""Pick up the title + base/head changes GitHub reports on edit.
424+
425+
Two concerns share this handler because both ride the same webhook:
426+
427+
* **Title edits** can relink the PR to a different BUD. We re-run
428+
``resolve_bud_from_pr`` and emit ``pr_linked`` / ``pr_unlinked``
429+
timeline events so the audit trail stays symmetric.
430+
* **Base-branch edits** move the PR between release-stage tabs.
431+
Without syncing ``pr.base_branch``, a PR retargeted from ``main``
432+
to ``develop`` keeps showing on the PROD tab indefinitely because
433+
``branch_matches`` runs against the stale value. ``head_branch``
434+
is synced for the same reason (force-pushes that rename the
435+
branch land here too).
436+
437+
Stale state is what makes ``BUD-004`` keep ``#1997`` on its PROD
438+
tab even after the PR was retargeted to ``develop`` — fixing the
439+
sync side closes that loop for future edits.
440+
"""
441+
pr_repo = PullRequestRepository(db, org_id=org_id)
442+
pr = await pr_repo.get_by_github_pr_id(pr_data.id)
443+
if pr is None:
444+
return
445+
446+
# Title / base / head are always synced — the dispatcher only fires
447+
# this handler when at least one of them changed, so the writes are
448+
# never wasted.
449+
title_changed = pr.title != pr_data.title
450+
pr.title = pr_data.title
451+
pr.base_branch = pr_data.base.ref
452+
pr.head_branch = pr_data.head.ref
453+
454+
# Only re-resolve the BUD link when the title actually changed. A
455+
# base-only edit (the BUD-004 / PR-1997 case) must NOT trigger a
456+
# re-resolution that could unlink a PR whose old title once carried
457+
# a now-rejected reference shape (``auth1-bud-7``-style strings the
458+
# previous, looser regex used to accept). The auto-transition checks
459+
# downstream key off ``pr.bud_id``; leaving the link intact keeps
460+
# the development → code_review threshold stable across base edits.
461+
if not title_changed:
462+
await db.commit()
463+
return
464+
465+
new_bud_id, new_bud = await resolve_bud_from_pr(db, org_id, pr_data.head.ref, pr_data.title)
466+
if new_bud_id == pr.bud_id:
467+
await db.commit()
468+
return
469+
470+
old_bud_id = pr.bud_id
471+
pr.bud_id = new_bud_id
472+
473+
# Audit symmetry: both BUDs record the rebind. Skipping the unlink
474+
# event would leave the previous BUD's timeline showing a PR that
475+
# quietly stopped belonging to it.
476+
if old_bud_id is not None and old_bud_id != new_bud_id:
477+
await record_event(
478+
db,
479+
org_id,
480+
old_bud_id,
481+
"pr_unlinked",
482+
detail={
483+
"pr_number": pr_data.number,
484+
"reason": "title_edited",
485+
"new_bud_id": str(new_bud_id) if new_bud_id else None,
486+
},
487+
)
488+
489+
if new_bud is not None:
490+
await record_event(
491+
db,
492+
org_id,
493+
new_bud.id,
494+
"pr_linked",
495+
detail={
496+
"pr_number": pr_data.number,
497+
"reason": "title_edited",
498+
"previous_bud_id": str(old_bud_id) if old_bud_id else None,
499+
},
500+
)
501+
502+
await db.commit()
503+
504+
# Re-run the auto-advance check on both sides. ``check_all_repos_have_prs``
505+
# only fires forward transitions (development → code_review) and no-ops
506+
# outside that stage, so the old-BUD call is defensive: it covers the
507+
# case where stripping a PR drops the old BUD back below the "all
508+
# impacted repos have PRs" threshold while it's still in development.
509+
if old_bud_id is not None and old_bud_id != new_bud_id:
510+
bud_repo = BUDRepository(db, org_id=org_id)
511+
old_bud = await bud_repo.get_by_id(old_bud_id)
512+
if old_bud is not None:
513+
await check_all_repos_have_prs(db, org_id, old_bud)
514+
if new_bud is not None:
515+
await check_all_repos_have_prs(db, org_id, new_bud)
516+
517+
logger.info(
518+
"pr_relinked_on_title_edit",
519+
pr_number=pr_data.number,
520+
from_bud=str(old_bud_id) if old_bud_id else None,
521+
to_bud=str(new_bud_id) if new_bud_id else None,
522+
)
523+
524+
411525
async def _handle_review_submitted(
412526
org_id: uuid.UUID,
413527
pr_data: GitHubPullRequest,

0 commit comments

Comments
 (0)