Merge pull request #4928 from gitbutlerapp/fix-use-correct-branch-name

apps/desktop/src/lib/branch/BranchHeader.svelte

@@ -121,8 +121,11 @@
 
 			if (branch.commits.some((c) => !c.isRemote)) {
 				const firstPush = !branch.upstream;
-				const remoteBranchRef = await branchController.pushBranch(branch.id, branch.requiresForce);
-				upstreamBranchName = getBranchNameFromRef(remoteBranchRef);
+				const { refname, remote } = await branchController.pushBranch(
+					branch.id,
+					branch.requiresForce
+				);
+				upstreamBranchName = getBranchNameFromRef(refname, remote);
 
 				if (firstPush) {
 					// TODO: fix this hack for reactively available prService.

apps/desktop/src/lib/utils/branch.test.ts

@@ -4,20 +4,36 @@ import { expect, test, describe } from 'vitest';
 describe.concurrent('getBranchNameFromRef', () => {
 	test('When provided a ref with a remote prefix, it returns the branch name', () => {
 		const ref = 'refs/remotes/origin/main';
+		const remote = 'origin';
 
-		expect(BranchUtils.getBranchNameFromRef(ref)).toBe('main');
+		expect(BranchUtils.getBranchNameFromRef(ref, remote)).toBe('main');
 	});
 
-	test('When provided a ref without a remote prefix, it returns the branch name', () => {
+	test("When provided a ref with a remote prefix that can't be found, it throws an error", () => {
 		const ref = 'main';
+		const remote = 'origin';
 
-		expect(BranchUtils.getBranchNameFromRef(ref)).toBe('main');
+		expect(() => BranchUtils.getBranchNameFromRef(ref, remote)).toThrowError();
 	});
 
-	test('When provided a ref with a remote prefix and multiple separators, it returns the branch name', () => {
+	test('When provided a ref with a remote prefix and multiple separators, it returns the correct branch name', () => {
 		const ref = 'refs/remotes/origin/feature/cool-thing';
+		const remote = 'origin';
 
-		expect(BranchUtils.getBranchNameFromRef(ref)).toBe('feature/cool-thing');
+		expect(BranchUtils.getBranchNameFromRef(ref, remote)).toBe('feature/cool-thing');
+	});
+
+	test('When provided a ref with a remote that has multiple separators in it, it returns the correct branch name', () => {
+		const ref = 'refs/remotes/origin/feature/cool-thing';
+		const remote = 'origin/feature';
+
+		expect(BranchUtils.getBranchNameFromRef(ref, remote)).toBe('cool-thing');
+	});
+
+	test('When provided a ref but no explicit remote, it returns the branch name with the remote prefix', () => {
+		const ref = 'refs/remotes/origin/main';
+
+		expect(BranchUtils.getBranchNameFromRef(ref)).toBe('origin/main');
 	});
 });
 

apps/desktop/src/lib/utils/branch.ts

@@ -2,13 +2,25 @@ const PREFERRED_REMOTE = 'origin';
 const BRANCH_SEPARATOR = '/';
 const REF_REMOTES_PREFIX = 'refs/remotes/';
 
-export function getBranchNameFromRef(ref: string): string | undefined {
+/**
+ * Get the branch name from a refname.
+ *
+ * If a remote is provided, the remote prefix will be removed.
+ */
+export function getBranchNameFromRef(ref: string, remote?: string): string | undefined {
 	if (ref.startsWith(REF_REMOTES_PREFIX)) {
-		ref = ref.slice(REF_REMOTES_PREFIX.length);
+		ref = ref.replace(REF_REMOTES_PREFIX, '');
 	}
 
-	const parts = ref.split(BRANCH_SEPARATOR);
-	return parts.length > 1 ? parts.slice(1).join(BRANCH_SEPARATOR) : ref;
+	if (remote !== undefined) {
+		const originPrefix = `${remote}${BRANCH_SEPARATOR}`;
+		if (!ref.startsWith(originPrefix)) {
+			throw new Error('Failed to parse branch name as reference');
+		}
+		ref = ref.replace(originPrefix, '');
+	}
+
+	return ref;
 }
 
 export function getBranchRemoteFromRef(ref: string): string | undefined {

apps/desktop/src/lib/vbranches/branchController.ts

@@ -4,7 +4,7 @@ import * as toasts from '$lib/utils/toasts';
 import posthog from 'posthog-js';
 import type { BaseBranchService } from '$lib/baseBranch/baseBranchService';
 import type { RemoteBranchService } from '$lib/stores/remoteBranches';
-import type { Hunk, LocalFile } from './types';
+import type { BranchPushResult, Hunk, LocalFile } from './types';
 import type { VirtualBranchService } from './virtualBranch';
 
 export class BranchController {
@@ -261,16 +261,16 @@ export class BranchController {
 		}
 	}
 
-	async pushBranch(branchId: string, withForce: boolean): Promise<string> {
+	async pushBranch(branchId: string, withForce: boolean): Promise<BranchPushResult> {
 		try {
-			const upstreamRef = await invoke<string>('push_virtual_branch', {
+			const pushResult = await invoke<BranchPushResult>('push_virtual_branch', {
 				projectId: this.projectId,
 				branchId,
 				withForce
 			});
 			posthog.capture('Push Successful');
 			await this.vbranchService.refresh();
-			return upstreamRef;
+			return pushResult;
 		} catch (err: any) {
 			console.error(err);
 			const { code, message } = err;

apps/desktop/src/lib/vbranches/types.ts

@@ -376,3 +376,8 @@ export class BranchData {
 		return this.name.replace('refs/remotes/', '').replace('origin/', '').replace('refs/heads/', '');
 	}
 }
+
+export interface BranchPushResult {
+	refname: string;
+	remote: string;
+}

crates/gitbutler-branch-actions/src/actions.rs

@@ -418,7 +418,7 @@ pub fn push_virtual_branch(
     branch_id: BranchId,
     with_force: bool,
     askpass: Option<Option<BranchId>>,
-) -> Result<Refname> {
+) -> Result<vbranch::PushResult> {
     let ctx = open_with_verify(project)?;
     assure_open_workspace_mode(&ctx).context("Pushing a branch requires open workspace mode")?;
     vbranch::push(&ctx, branch_id, with_force, askpass)

crates/gitbutler-branch-actions/src/virtual.rs

@@ -82,6 +82,13 @@ pub struct VirtualBranches {
     pub skipped_files: Vec<gitbutler_diff::FileDiff>,
 }
 
+#[derive(Debug, PartialEq, Clone, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct PushResult {
+    pub remote: String,
+    pub refname: Refname,
+}
+
 pub fn unapply_ownership(
     ctx: &CommandContext,
     ownership: &BranchOwnershipClaims,
@@ -1054,19 +1061,19 @@ pub(crate) fn push(
     branch_id: BranchId,
     with_force: bool,
     askpass: Option<Option<BranchId>>,
-) -> Result<Refname> {
+) -> Result<PushResult> {
     let vb_state = ctx.project().virtual_branches();
 
+    let default_target = vb_state.get_default_target()?;
+    let upstream_remote = match default_target.push_remote_name {
+        Some(remote) => remote.clone(),
+        None => default_target.branch.remote().to_owned(),
+    };
+
     let mut vbranch = vb_state.get_branch_in_workspace(branch_id)?;
     let remote_branch = if let Some(upstream_branch) = &vbranch.upstream {
         upstream_branch.clone()
     } else {
-        let default_target = vb_state.get_default_target()?;
-        let upstream_remote = match default_target.push_remote_name {
-            Some(remote) => remote.clone(),
-            None => default_target.branch.remote().to_owned(),
-        };
-
         let remote_branch = format!(
             "refs/remotes/{}/{}",
             upstream_remote,
@@ -1101,7 +1108,10 @@ pub(crate) fn push(
         .context("failed to write target branch after push")?;
     ctx.fetch(remote_branch.remote(), askpass.map(|_| "modal".to_string()))?;
 
-    Ok(gitbutler_reference::Refname::Remote(remote_branch))
+    Ok(PushResult {
+        remote: upstream_remote,
+        refname: gitbutler_reference::Refname::Remote(remote_branch),
+    })
 }
 
 struct IsCommitIntegrated<'repo> {

crates/gitbutler-tauri/src/virtual_branches.rs

@@ -3,6 +3,7 @@ pub mod commands {
     use gitbutler_branch::{
         BranchCreateRequest, BranchId, BranchOwnershipClaims, BranchUpdateRequest,
     };
+    use gitbutler_branch_actions::internal::PushResult;
     use gitbutler_branch_actions::upstream_integration::{BranchStatuses, Resolution};
     use gitbutler_branch_actions::{
         BaseBranch, BranchListing, BranchListingDetails, BranchListingFilter, RemoteBranch,
@@ -267,7 +268,7 @@ pub mod commands {
         project_id: ProjectId,
         branch_id: BranchId,
         with_force: bool,
-    ) -> Result<Refname, Error> {
+    ) -> Result<PushResult, Error> {
         let project = projects.get(project_id)?;
         let upstream_refname = gitbutler_branch_actions::push_virtual_branch(
             &project,