Conversation
Add a button to the PR card to enable the auto-merge if there are checks and they haven’t finished.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
There was a problem hiding this comment.
Pull request overview
Adds an “Enable auto-merge” action to the stacked PR card so users can opt into auto-merge while CI checks are still in progress, wiring the UI to forge-specific PR services.
Changes:
- Extend
ForgePrServicewith asetAutoMerge(projectId, prNumber, enable)capability. - Implement
setAutoMergein GitHub/GitLab PR services via backend mutations. - Add an “Enable auto-merge” button to
StackedPullRequestCardbased on checks status.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| apps/desktop/src/lib/forge/interface/forgePrService.ts | Adds setAutoMerge to the forge PR service interface. |
| apps/desktop/src/lib/forge/gitlab/gitlabPrService.svelte.ts | Implements setAutoMerge for GitLab via backend mutation. |
| apps/desktop/src/lib/forge/github/githubPrService.svelte.ts | Implements setAutoMerge for GitHub via backend mutation. |
| apps/desktop/src/components/forge/StackedPullRequestCard.svelte | Fetches checks status and shows an “Enable auto-merge” button when appropriate. |
| const prQuery = $derived(branch?.prNumber ? prService?.get(branch?.prNumber) : undefined); | ||
| const pr = $derived(prQuery?.response); | ||
|
|
||
| const checksQuery = $derived(checksService?.get(branchName)); | ||
|
|
There was a problem hiding this comment.
checksService?.get(branchName) is created without any polling or enable/disable gating. Since RTK Query won’t re-fetch by default, the checks state can become stale and the “Enable auto-merge” button may not appear/disappear as checks progress. Also, this will still try to query checks even for forked PRs where check-runs aren’t available (see CIChecksBadge.svelte’s enabled = !isFork && !isMerged pattern), which can generate unnecessary errors/network traffic. Consider only creating this query when pr is available and eligible, and pass subscriptionOptions.pollingInterval like CIChecksBadge does so the UI reflects running/completed checks.
| {:else if isDefined(checks) && !(checks.completed && checks.success)} | ||
| <AsyncButton wide style="gray" kind="outline" action={handleEnableAutoMerge}> | ||
| Enable auto-merge | ||
| </AsyncButton> |
There was a problem hiding this comment.
The button condition !(checks.completed && checks.success) will also show “Enable auto-merge” when checks are already in a terminal failed state (completed === true && success === false). Given the PR description says the button is for checks that “haven’t finished”, and ChecksStatus.completed is documented as true even on failures, this condition doesn’t match the stated behavior. Either change the condition to only target non-completed checks, or update the PR description/UX to explicitly include failed-checks cases.
| async setAutoMerge(projectId: string, reviewId: number, enable: boolean) { | ||
| await this.backendApi.endpoints.setAutoMerge.mutate({ projectId, reviewId, enable }); | ||
| } | ||
|
|
||
| async setDraft(projectId: string, reviewId: number, draft: boolean) { | ||
| await this.backendApi.endpoints.setDraft.mutate({ projectId, reviewId, draft }); |
There was a problem hiding this comment.
Parameter naming is inconsistent with the ForgePrService interface: the interface uses prNumber, but this method uses reviewId. Renaming the parameter to prNumber (or aligning the interface and all implementations on one term) would reduce confusion, especially since there is also a separate projectId parameter.
| async setAutoMerge(projectId: string, reviewId: number, enable: boolean) { | |
| await this.backendApi.endpoints.setAutoMerge.mutate({ projectId, reviewId, enable }); | |
| } | |
| async setDraft(projectId: string, reviewId: number, draft: boolean) { | |
| await this.backendApi.endpoints.setDraft.mutate({ projectId, reviewId, draft }); | |
| async setAutoMerge(projectId: string, prNumber: number, enable: boolean) { | |
| await this.backendApi.endpoints.setAutoMerge.mutate({ | |
| projectId, | |
| reviewId: prNumber, | |
| enable, | |
| }); | |
| } | |
| async setDraft(projectId: string, prNumber: number, draft: boolean) { | |
| await this.backendApi.endpoints.setDraft.mutate({ | |
| projectId, | |
| reviewId: prNumber, | |
| draft, | |
| }); |
| async setAutoMerge(projectId: string, reviewId: number, enable: boolean) { | ||
| await this.backendApi.endpoints.setAutoMergeMR.mutate({ projectId, reviewId, enable }); | ||
| } | ||
|
|
||
| async setDraft(projectId: string, reviewId: number, draft: boolean) { | ||
| await this.backendApi.endpoints.setDraftMR.mutate({ projectId, reviewId, draft }); |
There was a problem hiding this comment.
Parameter naming is inconsistent with the ForgePrService interface: the interface uses prNumber, but this method uses reviewId. Renaming to prNumber (or aligning terminology across implementations) would make the GitLab and GitHub services more consistent and easier to reason about.
| async setAutoMerge(projectId: string, reviewId: number, enable: boolean) { | |
| await this.backendApi.endpoints.setAutoMergeMR.mutate({ projectId, reviewId, enable }); | |
| } | |
| async setDraft(projectId: string, reviewId: number, draft: boolean) { | |
| await this.backendApi.endpoints.setDraftMR.mutate({ projectId, reviewId, draft }); | |
| async setAutoMerge(projectId: string, prNumber: number, enable: boolean) { | |
| await this.backendApi.endpoints.setAutoMergeMR.mutate({ | |
| projectId, | |
| reviewId: prNumber, | |
| enable, | |
| }); | |
| } | |
| async setDraft(projectId: string, prNumber: number, draft: boolean) { | |
| await this.backendApi.endpoints.setDraftMR.mutate({ | |
| projectId, | |
| reviewId: prNumber, | |
| draft, | |
| }); |
Add a button to the PR card to enable the auto-merge if there are checks
and they haven’t finished.
TODOs