Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions crates/but-api/src/legacy/forge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,31 @@ pub async fn publish_review(
)
.await
}

/// Merge a review on the forge.
#[but_api]
#[instrument(err(Debug))]
pub async fn merge_review(ctx: ThreadSafeContext, review_id: usize) -> Result<()> {
let (storage, base_branch, preferred_forge_user) = {
let ctx = ctx.into_thread_local();
let base_branch = gitbutler_branch_actions::base::get_base_branch_data(&ctx)?;
(
but_forge_storage::Controller::from_path(but_path::app_data_dir()?),
base_branch,
ctx.legacy_project.preferred_forge_user.clone(),
)
};
but_forge::merge_review(
&preferred_forge_user,
&base_branch
.forge_repo_info
.context("No forge could be determined for this repository branch")?,
review_id,
&storage,
)
.await
}

/// Update the stacked review descriptions to have the correct footers.
#[but_api]
#[instrument(err(Debug))]
Expand Down
2 changes: 1 addition & 1 deletion crates/but-forge/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub use review::{
CacheConfig, CreateForgeReviewParams, ForgeAccountValidity, ForgeReview, ForgeReviewDescriptionUpdate,
ForgeReviewFilter, ReviewTemplateFunctions, available_review_templates, check_forge_account_is_valid,
create_forge_review, get_forge_review, get_review_template_functions, list_forge_reviews_for_branch,
list_forge_reviews_with_cache, update_review_description_tables,
list_forge_reviews_with_cache, merge_review, update_review_description_tables,
};

fn determine_forge_from_host(host: &str) -> Option<ForgeName> {
Expand Down
46 changes: 45 additions & 1 deletion crates/but-forge/src/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{
path::{self},
};

use anyhow::{Error, Result};
use anyhow::{Context as _, Error, Result};
use but_fs::list_files;
use but_github::CredentialCheckResult;
use but_gitlab::GitLabProjectId;
Expand Down Expand Up @@ -733,6 +733,50 @@ pub async fn get_forge_review(
}
}

/// Merge a review to it's target branch
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar error in comment: "it's" should be "its" (possessive form, not contraction).

Suggested change
/// Merge a review to it's target branch
/// Merge a review to its target branch

Copilot uses AI. Check for mistakes.
pub async fn merge_review(
preferred_forge_user: &Option<crate::ForgeUser>,
forge_repo_info: &crate::forge::ForgeRepoInfo,
review_number: usize,
storage: &but_forge_storage::Controller,
) -> Result<()> {
let crate::forge::ForgeRepoInfo { forge, owner, repo, .. } = forge_repo_info;
match forge {
ForgeName::GitHub => {
let preferred_account = preferred_forge_user.as_ref().and_then(|user| user.github());
let pr_number = review_number
.try_into()
.context("PR: Failed to cast usize to i64, somehow")?;
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message should be more user-friendly and consistent with the pattern used in other functions. Change "PR: Failed to cast usize to i64, somehow" to "PR number is too large" to match the error message in but_github::pr::get (line 54 of crates/but-github/src/pr.rs).

Suggested change
.context("PR: Failed to cast usize to i64, somehow")?;
.context("PR number is too large")?;

Copilot uses AI. Check for mistakes.
let params = but_github::MergePullRequestParams {
owner,
repo,
pr_number,
commit_message: None,
commit_title: None,
merge_method: None,
};
but_github::pr::merge(preferred_account, params, storage).await
}
ForgeName::GitLab => {
let preferred_account = preferred_forge_user.as_ref().and_then(|user| user.gitlab());
let project_id = GitLabProjectId::new(owner, repo);
let mr_iid = review_number
.try_into()
.context("MR: Failed to cast usize to i64, somehow")?;
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message should be more user-friendly and consistent with the pattern used in other functions. Change "MR: Failed to cast usize to i64, somehow" to "MR number is too large" to match the error message in but_gitlab::mr::get (line 52 of crates/but-gitlab/src/mr.rs).

Suggested change
.context("MR: Failed to cast usize to i64, somehow")?;
.context("MR number is too large")?;

Copilot uses AI. Check for mistakes.
let params = but_gitlab::MergeMergeRequestParams {
project_id,
mr_iid,
squash: None,
};

but_gitlab::mr::merge(preferred_account, params, storage).await
}
_ => Err(Error::msg(format!(
"Merging reviews for forge {forge:?} is not implemented yet.",
))),
}
}

#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct CreateForgeReviewParams {
Expand Down
62 changes: 62 additions & 0 deletions crates/but-github/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,39 @@ impl GitHubClient {
let pr: GitHubPullRequest = response.json().await?;
Ok(pr.into())
}

pub async fn merge_pull_request(&self, params: &MergePullRequestParams<'_>) -> Result<()> {
#[derive(Serialize)]
struct MergePullRequestBody<'a> {
#[serde(skip_serializing_if = "Option::is_none")]
commit_title: Option<&'a str>,
#[serde(skip_serializing_if = "Option::is_none")]
commit_message: Option<&'a str>,
#[serde(skip_serializing_if = "Option::is_none")]
merge_method: Option<&'a str>,
}

let url = format!(
"{}/repos/{}/{}/pulls/{}/merge",
self.base_url, params.owner, params.repo, params.pr_number
);

let merge_method = params.merge_method.as_ref().map(Into::into);

let body = MergePullRequestBody {
commit_title: params.commit_title,
commit_message: params.commit_message,
merge_method,
};

let response = self.client.put(&url).json(&body).send().await?;

if !response.status().is_success() {
bail!("Failed to merge pull request: {}", response.status());
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider including the response body in the error message for better debugging. Similar to the pattern used in GitLab's create_merge_request (crates/but-gitlab/src/client.rs:166), the error could be:

let status = response.status();
let error_text = response.text().await.unwrap_or_default();
bail!("Failed to merge pull request: {status} - {error_text}")

This provides more context about why the merge failed (e.g., conflicts, permissions, required checks).

Suggested change
bail!("Failed to merge pull request: {}", response.status());
let status = response.status();
let error_text = response.text().await.unwrap_or_default();
bail!("Failed to merge pull request: {status} - {error_text}");

Copilot uses AI. Check for mistakes.
}

Ok(())
}
}

pub struct CreatePullRequestParams<'a> {
Expand All @@ -254,6 +287,35 @@ pub struct UpdatePullRequestParams<'a> {
pub state: Option<&'a str>,
}

#[derive(Debug, Clone, Serialize, Deserialize, Default)]
#[serde(rename_all = "lowercase")]
pub enum MergeMethod {
#[default]
Merge,
Squash,
Rebase,
}

impl From<&MergeMethod> for &str {
fn from(val: &MergeMethod) -> Self {
match val {
MergeMethod::Merge => "merge",
MergeMethod::Rebase => "rebase",
MergeMethod::Squash => "squash",
}
}
}

#[derive(Debug, Clone)]
pub struct MergePullRequestParams<'a> {
pub owner: &'a str,
pub repo: &'a str,
pub pr_number: i64,
pub commit_title: Option<&'a str>,
pub commit_message: Option<&'a str>,
pub merge_method: Option<MergeMethod>,
}

#[derive(Debug, Serialize)]
pub struct AuthenticatedUser {
pub login: String,
Expand Down
3 changes: 2 additions & 1 deletion crates/but-github/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use serde::{Deserialize, Serialize};
mod client;
pub mod pr;
pub use client::{
CheckRun, CreatePullRequestParams, GitHubClient, GitHubPrLabel, GitHubUser, PullRequest, UpdatePullRequestParams,
CheckRun, CreatePullRequestParams, GitHubClient, GitHubPrLabel, GitHubUser, MergeMethod, MergePullRequestParams,
PullRequest, UpdatePullRequestParams,
};
mod token;
pub use token::GithubAccountIdentifier;
Expand Down
11 changes: 11 additions & 0 deletions crates/but-github/src/pr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,14 @@ pub async fn update(
.context("Failed to update pull request")?;
Ok(pr)
}

pub async fn merge(
preferred_account: Option<&crate::GithubAccountIdentifier>,
params: crate::client::MergePullRequestParams<'_>,
storage: &but_forge_storage::Controller,
) -> Result<()> {
GitHubClient::from_storage(storage, preferred_account)?
.merge_pull_request(&params)
.await
.context("Failed to merge PR")
}
28 changes: 28 additions & 0 deletions crates/but-gitlab/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,29 @@ impl GitLabClient {
let mr: GitLabMergeRequest = response.json().await?;
Ok(mr.into())
}

pub async fn merge_merge_request(&self, params: &MergeMergeRequestParams) -> Result<()> {
#[derive(Serialize)]
struct MergeMergeRequestBody {
#[serde(skip_serializing_if = "Option::is_none")]
squash: Option<bool>,
}

let url = format!(
"{}/projects/{}/merge_requests/{}/merge",
self.base_url, params.project_id, params.mr_iid
);

let body = MergeMergeRequestBody { squash: params.squash };

let response = self.client.put(&url).json(&body).send().await?;

if !response.status().is_success() {
bail!("Failed to merge merge request: {}", response.status());
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider including the response body in the error message for better debugging. Similar to the pattern used in create_merge_request at line 166 of this file, the error could be:

let status = response.status();
let error_text = response.text().await.unwrap_or_default();
bail!("Failed to merge merge request: {status} - {error_text}")

This provides more context about why the merge failed (e.g., conflicts, permissions, CI checks).

Suggested change
bail!("Failed to merge merge request: {}", response.status());
let status = response.status();
let error_text = response.text().await.unwrap_or_default();
bail!("Failed to merge merge request: {status} - {error_text}");

Copilot uses AI. Check for mistakes.
}

Ok(())
}
}

pub struct CreateMergeRequestParams<'a> {
Expand All @@ -192,6 +215,11 @@ pub struct CreateMergeRequestParams<'a> {
pub target_branch: &'a str,
pub project_id: GitLabProjectId,
}
pub struct MergeMergeRequestParams {
pub project_id: GitLabProjectId,
pub mr_iid: i64,
pub squash: Option<bool>,
}

#[derive(Debug, Serialize)]
pub struct AuthenticatedUser {
Expand Down
4 changes: 3 additions & 1 deletion crates/but-gitlab/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ use but_secret::Sensitive;
mod client;
pub mod mr;
mod project;
pub use client::{CreateMergeRequestParams, GitLabClient, GitLabLabel, GitLabUser, MergeRequest};
pub use client::{
CreateMergeRequestParams, GitLabClient, GitLabLabel, GitLabUser, MergeMergeRequestParams, MergeRequest,
};
pub use project::GitLabProjectId;
mod token;
use serde::Serialize;
Expand Down
11 changes: 11 additions & 0 deletions crates/but-gitlab/src/mr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,14 @@ pub async fn get(
.context("Failed to get merge request")?;
Ok(mr)
}

pub async fn merge(
preferred_account: Option<&crate::GitlabAccountIdentifier>,
params: crate::client::MergeMergeRequestParams,
storage: &but_forge_storage::Controller,
) -> Result<()> {
GitLabClient::from_storage(storage, preferred_account)?
.merge_merge_request(&params)
.await
.context("Faile to merge MR")
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spelling error in error message: "Faile" should be "Failed".

Suggested change
.context("Faile to merge MR")
.context("Failed to merge MR")

Copilot uses AI. Check for mistakes.
}
10 changes: 10 additions & 0 deletions crates/but-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,16 @@ async fn handle_command(
Err(e) => Err(e),
}
}
"merge_review" => {
let params = deserialize_json(request.params);
match params {
Ok(params) => {
let result = legacy::forge::merge_review_cmd(params).await;
result.map(|_| json!({"result": "success"}))
}
Err(e) => Err(e),
}
}
"update_review_footers" => {
let params = deserialize_json(request.params);
match params {
Expand Down
1 change: 1 addition & 0 deletions crates/gitbutler-tauri/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ fn main() -> anyhow::Result<()> {
legacy::forge::tauri_pr_template::pr_template,
legacy::forge::tauri_list_reviews::list_reviews,
legacy::forge::tauri_publish_review::publish_review,
legacy::forge::tauri_merge_review::merge_review,
legacy::forge::tauri_update_review_footers::update_review_footers,
legacy::cli::tauri_install_cli::install_cli,
legacy::cli::tauri_cli_path::cli_path,
Expand Down
Loading