Skip to content

[Refactor] Collapse dual terminal PR status writes#270

Draft
roomote-roomote[bot] wants to merge 1 commit into
developfrom
feature/collapse-terminal-pr-status-1e0ujjb73p4gj
Draft

[Refactor] Collapse dual terminal PR status writes#270
roomote-roomote[bot] wants to merge 1 commit into
developfrom
feature/collapse-terminal-pr-status-1e0ujjb73p4gj

Conversation

@roomote-roomote

Copy link
Copy Markdown
Contributor

Created by Roomote. Follow up by mentioning @roomote-roomote or in the web UI.

What changed

Merged and closed pull-request webhooks no longer run two copy-pasted side effects in every source-control provider. They call one SDK helper that updates the linked task_pull_requests row and then records task-history best-effort with a single place for actor, title, URL, and error logging. GitHub, GitLab, Gitea, Bitbucket, and Azure DevOps terminal handlers were migrated, and provider tests now assert the unified call.

Why this change was made

Provider closed/merged paths all paired updateTaskPrStatus with recordPrStatusChangeInTaskHistory behind nearly identical non-throwing wrappers. That dual write forked as providers grew and made it easy for one side effect to drift without the other.

Impact

No intentional product behavior change for users. Terminal PR status still updates the tracked PR row and still lands in task history when history writing succeeds; history failures remain best-effort and non-throwing, while awaited providers still surface status-update failures. New providers only need to pass event facts into the shared helper.

@roomote-roomote

roomote-roomote Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

1 minor item to verify. See task

The refactor is clean and behavior-preserving for the awaited providers (GitLab, Gitea, Bitbucket, Azure DevOps). Provider tests and the new SDK helper test pass.

  • apps/api/src/handlers/github/index.ts:429 — GitHub pull_request.closed couples the two writes that were previously independent. Because markTaskPullRequestTerminal skips history recording when updateTaskPrStatus rejects, a transient failure updating the tracked PR row now also suppresses the task-history notification, whereas the old code recorded history regardless. Low impact (both are DB writes), but confirm this is intended given the "no behavior change" note.

Reviewed 6724694

@github-actions

Copy link
Copy Markdown
Contributor

Thank you for your contribution! Before we can merge this pull request, we need you to sign our Contributor License Agreement. You can sign it by posting the comment below.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

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