Skip to content

[Fix] Fast conversations receive duplicate canned PR status updates - #1739

Draft
roomote-roomote[bot] wants to merge 8 commits into
developfrom
fix/duplicate-pr-merge-notifications-27y3lsscwh5ns
Draft

[Fix] Fast conversations receive duplicate canned PR status updates#1739
roomote-roomote[bot] wants to merge 8 commits into
developfrom
fix/duplicate-pr-merge-notifications-27y3lsscwh5ns

Conversation

@roomote-roomote

@roomote-roomote roomote-roomote Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

​Opened on behalf of @mrubens. Follow up by mentioning @roomote-roomote, in the web UI, or in Slack.

What changed

  • Stop the terminal-status webhook fan-out from treating a Fast parent's Slack, Teams, Telegram, or Discord coordinates as an ordinary direct notification target.
  • Match exact destination coordinates so non-Fast conversations and distinct child-task threads keep their canned status notifications.
  • Deliver pull_request_status_changed to Fast before the independent Redis/task-history claim, then suppress the matching direct canned target only after that Fast path completes.
  • Restore direct canned delivery when Fast handling fails before completion; task-history-only failures retry history without duplicating the already-delivered Fast event.
  • Continue Fast delivery across every linked task before returning an accumulated task-history error, so one failed history write cannot starve later Fast parents.
  • Carry failed Fast task IDs through webhook handling and restore direct fallback only for those targets, preventing a later relay failure from duplicating earlier successful Fast notifications.
  • Deduplicate failed-task fallback by Fast conversation coordinates, so sibling tasks sharing one parent cannot re-post after any sibling already delivered there.
  • Derive successful Fast destinations from each task's latest run only, preventing stale historical Fast payloads from suppressing a current relay fallback.
  • Keep the Slack white_check_mark reaction owned by Fast parent-turn setup on success and by the direct notifier only on fallback or non-Fast paths.
  • Keep source-control event policy explicit: opened and terminal events use normal Fast interpretation, while feedback and conflict events retain their visibility-required presentation-only behavior.

Why this change was made

Fast child payloads inherit their parent conversation coordinates. The terminal webhook fan-out read those fields as an ordinary direct destination and posted its fixed was merged/closed by text into the Fast conversation, independently of the structured platform event that Fast also received.

Impact

Fast conversations receive the structured event and report it conversationally in session-owned wording without a preceding canned duplicate. Ordinary task conversations and dedicated child threads retain automated terminal updates. Pre-Fast failures restore the direct message and reaction only for affected tasks; post-Fast history failures do not duplicate either or prevent later linked tasks from being notified.

@roomote-community

roomote-community Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

No new code issues found. See task

  • Terminal delivery is dropped when the Fast-parent relay fails.
  • Terminal notification is still lost when status-history recording fails before Fast delivery.
  • A history failure can leave later linked Fast tasks without a terminal notification.
  • A failed sibling can duplicate a terminal notification after a successful sibling already delivered to the same Fast conversation.
  • A stale Fast payload from an older run can suppress the direct fallback for a current relay failure.

Reviewed 91d8030

Comment thread apps/api/src/handlers/github/notifyPullRequestTerminalStatus.ts Outdated
@roomote-roomote

roomote-roomote Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the stale-run fallback finding in 91d80305. The terminal notifier now orders runs by descending creation/id and selects one authoritative latest run per task, matching recordPrStatusChangeInTaskHistory. Successful destination suppression and failed-task fallback eligibility use only those latest runs; historical payloads remain available only for ordinary route compatibility.

The resumed-run regression covers an older Fast payload, a newer non-Fast run, and a current failed sibling relay, verifying the required direct fallback is not suppressed. Focused API tests pass (30), focused SDK tests pass (11), full lint/typechecking pass, pre-push checks pass, and the review thread is resolved.

@roomote-roomote roomote-roomote Bot changed the title [Fix] Fast sessions post duplicate PR merge notifications [Fix] Fast sessions repeat automated PR status notifications Aug 27, 2026
@roomote-roomote roomote-roomote Bot changed the title [Fix] Fast sessions repeat automated PR status notifications [Fix] Fast conversations receive duplicate canned PR status updates Aug 27, 2026
Comment thread packages/sdk/src/server/lib/task-runs/record-pr-status-change.ts Outdated
Comment thread packages/sdk/src/server/lib/task-runs/record-pr-status-change.ts
Comment thread apps/api/src/handlers/github/notifyPullRequestTerminalStatus.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant