fix: improve MR detection with branch normalization (ee15d634) · Commits · GitLab.org / gitlab-vscode-extension

src/desktop/gitlab/get_pipeline_and_mr_for_branch.ts

+34 −15

Original line number	Diff line number	Diff line
		import { log } from '../../common/log';
		import { GitLabProject } from '../../common/platform/gitlab_project';
		import { getMostRelevantMergeRequest } from '../utils/get_most_relevant_merge_request';
		import { sort } from '../utils/sort';
		import { getMergeRequestsForBranch } from './api/get_merge_requests_for_branch';
		import { findOpenMrForCurrentBranch } from './utils/mr_lookup_helpers';
		import { getPipelinesForMr } from './api/get_pipelines_for_mr';
		import { getPipelinesForRef } from './api/get_pipelines_for_ref';
		import { GitLabService } from './gitlab_service';

		// TODO: implement more granular approach to errors (deciding between expected and critical)
		const turnErrorToUndefined: <T>(p: Promise<T>) => Promise<T \| undefined> = p =>
		p.catch(e => {
		log.error(e);
		// Error handler that distinguishes between real errors and not-found cases
		const handleApiError = <T>(error: unknown, context: string): T \| undefined => {
		// Log with context for debugging
		if (error instanceof Error) {
		log.error(`${context}: ${error.message}`);
		} else {
		log.error(context, error instanceof Error ? error : new Error(String(error)));
		}
		return undefined;
		});
		};

		export const getPipelineAndMrForBranch = async (
		gitlabService: GitLabService,
		@@ -22,20 +25,36 @@ export const getPipelineAndMrForBranch = async (
		pipeline?: RestPipeline;
		mr?: RestMr;
		}> => {
		const mr = getMostRelevantMergeRequest(
		await turnErrorToUndefined(
		gitlabService.fetchFromApi(getMergeRequestsForBranch(project, trackingBranchName)),
		),
		);
		// Use centralized MR lookup helper (handles normalization, selection, error handling)
		let mr: RestMr \| undefined;
		try {
		mr =
		(await findOpenMrForCurrentBranch(gitlabService, project, trackingBranchName)) \|\| undefined;
		} catch (e) {
		handleApiError(e, 'findOpenMrForCurrentBranch failed');
		}

		// Only fetch MR-specific pipeline if MR exists
		if (mr) {
		const pipelines = await turnErrorToUndefined(gitlabService.fetchFromApi(getPipelinesForMr(mr)));
		try {
		const pipelines = await gitlabService.fetchFromApi(getPipelinesForMr(mr));
		if (pipelines && pipelines.length > 0) {
		const pipeline = sort(pipelines, (p1, p2) => p2.iid - p1.iid)[0];
		return { mr, pipeline };
		}
		} catch (e) {
		handleApiError(e, `Failed to fetch pipelines for MR !${mr.iid}`);
		}
		}
		const pipelines = await turnErrorToUndefined(
		gitlabService.fetchFromApi(getPipelinesForRef(project, trackingBranchName)),

		// Fallback: fetch pipeline by ref (works for both MR and non-MR branches)
		try {
		const pipelines = await gitlabService.fetchFromApi(
		getPipelinesForRef(project, trackingBranchName),
		);
		return { mr, pipeline: pipelines?.[0] };
		} catch (e) {
		handleApiError(e, `Failed to fetch pipelines for ref "${trackingBranchName}"`);
		return { mr };
		}
		};

src/desktop/gitlab/utils/mr_lookup_helpers.test.ts

0 → 100644

+232 −0

Original line number	Diff line number	Diff line
		import assert from 'assert';
		import * as sinon from 'sinon';
		import * as logModule from '../../../common/log';
		import { GitLabProject } from '../../../common/platform/gitlab_project';
		import { GitLabService } from '../gitlab_service';
		import {
		normalizeBranchName,
		selectMostRecentMr,
		findOpenMrForCurrentBranch,
		} from './mr_lookup_helpers';

		describe('mr_lookup_helpers', () => {
		describe('normalizeBranchName', () => {
		it('removes refs/heads/ prefix from branch name', () => {
		const result = normalizeBranchName('refs/heads/feature/my-branch');
		assert.strictEqual(result, 'feature/my-branch');
		});

		it('returns the branch name as-is if no prefix', () => {
		const result = normalizeBranchName('feature/my-branch');
		assert.strictEqual(result, 'feature/my-branch');
		});

		it('trims whitespace from branch name', () => {
		const result = normalizeBranchName(' feature/my-branch ');
		assert.strictEqual(result, 'feature/my-branch');
		});

		it('returns null for empty string', () => {
		const result = normalizeBranchName('');
		assert.strictEqual(result, null);
		});

		it('returns null for undefined input', () => {
		const result = normalizeBranchName(undefined);
		assert.strictEqual(result, null);
		});

		it('returns null for whitespace-only input', () => {
		const result = normalizeBranchName(' ');
		assert.strictEqual(result, null);
		});

		it('handles refs/heads/ with complex branch names', () => {
		const result = normalizeBranchName('refs/heads/feature/JIRA-123/fix-bug');
		assert.strictEqual(result, 'feature/JIRA-123/fix-bug');
		});
		});

		describe('selectMostRecentMr', () => {
		const createMr = (iid: number, draft: boolean, updatedAt: string): RestMr => ({
		id: iid,
		iid,
		title: `MR ${iid}`,
		project_id: 1,
		author: { name: 'User', avatar_url: null },
		web_url: `https://example.com/mr/${iid}`,
		references: { full: 'project#123' },
		severity: 'none',
		name: `MR ${iid}`,
		sha: 'abc123',
		source_project_id: 1,
		target_project_id: 1,
		source_branch: 'feature',
		state: 'opened',
		updated_at: updatedAt,
		draft,
		});

		it('returns null for undefined input', () => {
		const result = selectMostRecentMr(undefined);
		assert.strictEqual(result, null);
		});

		it('returns null for empty array', () => {
		const result = selectMostRecentMr([]);
		assert.strictEqual(result, null);
		});

		it('selects single non-draft MR', () => {
		const mr = createMr(1, false, '2026-01-27T10:00:00Z');
		const result = selectMostRecentMr([mr]);
		assert.strictEqual(result?.iid, 1);
		});

		it('includes draft MRs when selecting the most recent MR', () => {
		const draft = createMr(1, true, '2026-01-27T10:00:00Z');
		const nonDraft = createMr(2, false, '2026-01-27T09:00:00Z');
		const result = selectMostRecentMr([draft, nonDraft]);
		assert.strictEqual(result?.iid, 1);
		});

		it('selects most recent MR when all MRs are drafts', () => {
		const draft1 = createMr(1, true, '2026-01-27T09:00:00Z');
		const draft2 = createMr(2, true, '2026-01-27T10:00:00Z');
		const result = selectMostRecentMr([draft1, draft2]);
		assert.strictEqual(result?.iid, 2);
		});

		it('selects most recently updated MR', () => {
		const mr1 = createMr(1, false, '2026-01-27T10:00:00Z');
		const mr2 = createMr(2, false, '2026-01-27T15:00:00Z');
		const mr3 = createMr(3, false, '2026-01-27T12:00:00Z');
		const result = selectMostRecentMr([mr1, mr2, mr3]);
		assert.strictEqual(result?.iid, 2);
		});

		it('handles MRs with identical updated_at timestamps', () => {
		const mr1 = createMr(1, false, '2026-01-27T10:00:00Z');
		const mr2 = createMr(2, false, '2026-01-27T10:00:00Z');
		const result = selectMostRecentMr([mr1, mr2]);
		// Should return first one in the sorted list
		assert(result?.iid === 1 \|\| result?.iid === 2);
		});

		it('handles MRs with missing updated_at (treats as 0)', () => {
		const mrNoDate = createMr(1, false, '');
		const mrWithDate = createMr(2, false, '2026-01-27T10:00:00Z');
		const result = selectMostRecentMr([mrNoDate, mrWithDate]);
		assert.strictEqual(result?.iid, 2);
		});

		it('correctly sorts MRs in descending order by date', () => {
		const mr1 = createMr(1, false, '2026-01-26T10:00:00Z');
		const mr2 = createMr(2, false, '2026-01-28T10:00:00Z');
		const mr3 = createMr(3, false, '2026-01-27T10:00:00Z');
		const result = selectMostRecentMr([mr1, mr2, mr3]);
		assert.strictEqual(result?.iid, 2);
		});
		});

		describe('findOpenMrForCurrentBranch', () => {
		let logDebugStub: sinon.SinonStub;
		let logWarnStub: sinon.SinonStub;
		let gitlabServiceStub: sinon.SinonStubbedInstance<GitLabService>;
		let projectStub: GitLabProject;

		beforeEach(() => {
		logDebugStub = sinon.stub(logModule.log, 'debug');
		logWarnStub = sinon.stub(logModule.log, 'warn');
		gitlabServiceStub = sinon.createStubInstance(GitLabService);
		projectStub = {
		gqlId: 'gid://gitlab/Project/1',
		restId: 1,
		description: 'Test Project',
		namespaceWithPath: 'test/project',
		webUrl: 'https://example.com/test/project',
		} as GitLabProject;
		});

		afterEach(() => {
		sinon.restore();
		});

		it('returns null for undefined branch name', async () => {
		const result = await findOpenMrForCurrentBranch(
		gitlabServiceStub as unknown as GitLabService,
		projectStub,
		undefined,
		);
		assert.strictEqual(result, null);
		sinon.assert.called(logDebugStub);
		});

		it('returns null for empty branch name', async () => {
		const result = await findOpenMrForCurrentBranch(
		gitlabServiceStub as unknown as GitLabService,
		projectStub,
		'',
		);
		assert.strictEqual(result, null);
		sinon.assert.called(logDebugStub);
		});

		it('normalizes branch name before API call', async () => {
		const createMr = (iid: number): RestMr => ({
		id: iid,
		iid,
		title: `MR ${iid}`,
		project_id: 1,
		author: { name: 'User', avatar_url: null },
		web_url: `https://example.com/mr/${iid}`,
		references: { full: 'project#123' },
		severity: 'none',
		name: `MR ${iid}`,
		sha: 'abc123',
		source_project_id: 1,
		target_project_id: 1,
		source_branch: 'feature',
		state: 'opened',
		updated_at: '2026-01-27T10:00:00Z',
		draft: false,
		});

		gitlabServiceStub.fetchFromApi.resolves([createMr(1)]);

		const result = await findOpenMrForCurrentBranch(
		gitlabServiceStub as unknown as GitLabService,
		projectStub,
		'refs/heads/feature/my-branch',
		);

		assert.strictEqual(result?.iid, 1);
		sinon.assert.called(logDebugStub);
		});

		it('handles API errors gracefully', async () => {
		gitlabServiceStub.fetchFromApi.rejects(new Error('API error'));

		const result = await findOpenMrForCurrentBranch(
		gitlabServiceStub as unknown as GitLabService,
		projectStub,
		'feature/my-branch',
		);

		assert.strictEqual(result, null);
		sinon.assert.called(logWarnStub);
		});

		it('returns null when no MRs found for branch', async () => {
		gitlabServiceStub.fetchFromApi.resolves([]);

		const result = await findOpenMrForCurrentBranch(
		gitlabServiceStub as unknown as GitLabService,
		projectStub,
		'feature/my-branch',
		);

		assert.strictEqual(result, null);
		});
		});
		});

src/desktop/gitlab/utils/mr_lookup_helpers.ts

0 → 100644

+97 −0

Original line number	Diff line number	Diff line
		import { log } from '../../../common/log';
		import { GitLabProject } from '../../../common/platform/gitlab_project';
		import { GitLabService } from '../gitlab_service';
		import { getMergeRequestsForBranch } from '../api/get_merge_requests_for_branch';

		/**
		* Normalizes branch names by removing refs/heads/ prefix from git config output.
		* - refs/heads/branch → branch
		* - Returns null for invalid/empty input
		*/
		export const normalizeBranchName = (branch: string \| undefined): string \| null => {
		if (!branch?.trim()) {
		return null;
		}

		const normalized = branch.trim();

		// Remove refs/heads/ prefix (can appear in git config output)
		if (normalized.startsWith('refs/heads/')) {
		return normalized.replace('refs/heads/', '');
		}

		return normalized;
		};

		/**
		* Selects the most recently updated MR from a list deterministically:
		* 1. Sorts by most recently updated
		* 2. Returns the first (most recent)
		* 3. Returns null if no valid MR found
		*/
		export const selectMostRecentMr = (mrs: RestMr[] \| undefined): RestMr \| null => {
		if (!mrs \|\| mrs.length === 0) {
		return null;
		}

		const sortedMrs = [...mrs];

		// Sort by updated_at descending (most recent first)
		sortedMrs.sort((a, b) => {
		const aTime = a.updated_at ? new Date(a.updated_at).getTime() : 0;
		const bTime = b.updated_at ? new Date(b.updated_at).getTime() : 0;
		return bTime - aTime;
		});

		return sortedMrs[0] ?? null;
		};

		/**
		* Centralized helper to find open MR for current branch.
		* Handles:
		* - Branch normalization
		* - Deterministic MR selection
		* - Graceful error handling
		* Returns null if not found or error occurs (caller decides fallback)
		*/
		export const findOpenMrForCurrentBranch = async (
		gitlabService: GitLabService,
		project: GitLabProject,
		branchName: string \| undefined,
		): Promise<RestMr \| null> => {
		try {
		// Normalize branch name
		const normalizedBranch = normalizeBranchName(branchName);
		if (!normalizedBranch) {
		log.debug('MR lookup: invalid branch name');
		return null;
		}

		// Fetch MRs from API
		try {
		const mrs = await gitlabService.fetchFromApi(
		getMergeRequestsForBranch(project, normalizedBranch),
		);

		// Select most recent MR deterministically
		const selectedMr = selectMostRecentMr(mrs);
		if (selectedMr) {
		log.debug(`MR lookup: found MR !${selectedMr.iid} for branch "${normalizedBranch}"`);
		}
		return selectedMr;
		} catch (apiError) {
		// Graceful API failure handling
		if (apiError instanceof Error) {
		log.warn(`MR lookup API failed: ${apiError.message}`);
		} else {
		log.warn('MR lookup API failed with unknown error');
		}
		// Return null to let caller fallback to "Create MR"
		return null;
		}
		} catch (e) {
		// Catch any unexpected errors
		log.error('MR lookup: unexpected error', e instanceof Error ? e : new Error(String(e)));
		return null;
		}
		};

src/desktop/status_bar.ts

+8 −1

Original line number	Diff line number	Diff line
		import assert = require('assert');
		import assert from 'assert';
		import * as vscode from 'vscode';
		import { createStatusBarItem } from '../common/utils/status_bar_item';
		import * as openers from './commands/openers';
		@@ -124,6 +124,13 @@ export class StatusBar {

		updateMrItem(mr: RestMr \| undefined, rootFsPath: string): void {
		if (!this.mrStatusBarItem) return;

		// Handle edge case: mr exists but is missing critical fields
		if (mr && (!mr.iid \|\| typeof mr.iid !== 'number')) {
		this.mrStatusBarItem.hide();
		return;
		}

		this.mrStatusBarItem.show();

		if (mr) {

src/desktop/test_utils/entities.ts

+1 −0

Original line number	Diff line number	Diff line
		@@ -44,6 +44,7 @@ export const mr: RestMr = {
		source_project_id: 9999,
		target_project_id: 9999,
		source_branch: 'feature-a',
		updated_at: '2021-02-12T12:06:17Z',
		};

		export const diffFile: RestDiffFile = {