Skip to content

Signals inbox: merged PRs stay in the Pull requests tab when the PR webhook binds to the wrong task run #87114

Description

@andrewm4894

Problem

A signal report whose implementation PR has already merged can stay in the Signals inbox Pull requests tab indefinitely, as if the work were still in flight. The same report also renders as unmerged, because implementation_pr_merged stays false.

The Pull requests tab is has_implementation_pr=true AND status=ready (products/signals/frontend/inbox/logics/reportListLogic.ts:48), so a report that never leaves ready never leaves the tab.

This is intermittent. Most reports resolve correctly on merge; a minority get stuck, and the stuck ones accumulate because nothing ever retries them.

Root cause

find_task_run() picks the wrong TaskRun for the PR.

products/tasks/backend/webhooks.py:105-133 — both branch legs resolve the run with an unordered .first():

task_run = (
    candidates.filter(
        _run_repository_filter(repository),
        branch=branch,
        state__wizard_head_branch__isnull=True,
    )
    .select_related(*TASK_RUN_SELECT_RELATED)
    .first()          # no order_by
)

Every self-driving PR has at least two runs on the same head branch: the implementation run that opened it, and the ReviewHog run (Task.OriginProduct.REVIEW_HOG) created to review it. ReviewHog's run is created within seconds of the PR opening, so both rows are present when the pull_request opened delivery arrives, both match branch + repository, and Postgres returns whichever it likes.

Once the wrong run wins, the mis-binding is permanent: _record_run_pr_url() writes output.pr_url and state.verified_pr_urls onto it, so leg 1 (state__verified_pr_urls__contains) resolves to that same wrong run for every later delivery on that PR.

At merge time (webhooks.py:294-305) two things then go wrong:

  1. _record_run_pr_merged() stamps output.pr_merged on the review run. implementation_pr_merged is read from the report's own task (products/signals/backend/implementation_pr.py:77, via tasks_facade.get_merged_pr_task_ids), so it stays false.
  2. _transition_signal_reports_for_task() is keyed on task_run.task_id, and SignalReport.reports_for_task_filter() finds no reports for the review task — so the ready → resolved transition never fires.

Note that the merge webhook itself is fine: it is received, verified, matched to a run, and a pr_merged analytics event is captured. Only the run→task→report hop is wrong, which is why this is invisible without cross-checking the event's task_id against the report's task_run artefact.

Evidence

Confirmed on two independent stuck reports in the PostHog project:

  • The report's task_run artefact names the implementation task; the pr_merged event for the same PR carries a different task_id, whose task_created / task_run_created events have origin_product: review_hog.
  • In one case the review task was created ~5 seconds before the pull_request opened delivery was processed — the exact race window.
  • The implementation run's output holds the correct pr_url but has no pr_merged and no state.verified_pr_urls; the review run holds both.

Every entry currently stuck in the tab points at a PR that is already merged or closed on GitHub, while the large majority of reports historically do resolve on merge — consistent with a race rather than a systematic failure.

Proposed fixes

Three independent changes, smallest blast radius first.

PR 1 — make find_task_run() deterministic (the actual fix)

products/tasks/backend/webhooks.py

  • Order both branch legs explicitly and prefer the run that created the branch: .order_by("created_at", "id"), oldest first. A review run is always created after the run it reviews.
  • Deprioritise (or exclude) runs that cannot have opened the PR — task__origin_product=REVIEW_HOG is the concrete case today. Prefer a positive rule over a denylist if one is available: a run whose output.pr_url already equals the webhook's pr_url should always win outright, before any branch matching.
  • Suggested leg order: exact output.pr_url match → state.verified_pr_urls → branch+repo (ordered, PR-bearing runs only) → output.head_branches (same) → wizard head branch.
  • Tests: two runs on one branch (implementation + ReviewHog) in both insertion orders resolve to the implementation run; a review-only branch still resolves to the review run; existing wizard and fork cases unchanged.

PR 2 — stop a mis-bound run from stranding a report

products/tasks/backend/webhooks.py

_transition_signal_reports_for_task() is keyed only on task_id. Add a PR-URL fallback: when the task-keyed lookup returns nothing, resolve reports whose surfaced implementation PR URL equals the webhook's pr_url and transition those. Same for the closed-unmerged (suppress) branch.

This makes the pipeline self-correcting — a future binding bug degrades to "resolved slightly late" instead of "stuck forever". Worth landing even after PR 1.

PR 3 — reconcile the existing stuck reports

A management command (or Celery beat job) that walks reports in ready with an implementation PR, asks GitHub for each PR's state, and:

  • merged → transition the report to resolved
  • closed-unmerged → transition to suppressed
  • also backfill output.pr_merged / output.pr_state on the report's own implementation run, so implementation_pr_merged reads correctly

Run it once as a backfill, then keep it on a low-frequency schedule as a safety net against dropped or mis-bound deliveries. Bound the GitHub calls per run and log what it skipped.

Out of scope / related


Created with PostHog from a Slack thread

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions