Fix duplicate entries in .git/config for GitHub PRs (#8876)

* Initial plan

* Prevent duplicate PR config entries

Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>

* Serialize PR config updates

Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>

* Attestation commit

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>

Author: Copilot

src/github/pullRequestGitHelper.ts

@@ -42,6 +42,8 @@ export type BranchInfo = {
 
 export class PullRequestGitHelper {
 	static ID = 'PullRequestGitHelper';
+	private static readonly configUpdates = new WeakMap<Repository, Map<string, Promise<void>>>();
+
 	static async checkoutFromFork(
 		repository: Repository,
 		pullRequest: PullRequestModel & IResolvedPullRequestModel,
@@ -307,6 +309,38 @@ export class PullRequestGitHelper {
 		return `${owner}#${repository}#${baseBranch}`;
 	}
 
+	private static async setConfig(repository: Repository, key: string, value: string): Promise<void> {
+		let repositoryUpdates = PullRequestGitHelper.configUpdates.get(repository);
+		if (!repositoryUpdates) {
+			repositoryUpdates = new Map();
+			PullRequestGitHelper.configUpdates.set(repository, repositoryUpdates);
+		}
+
+		const previousUpdate = repositoryUpdates.get(key);
+		const update = (previousUpdate ? previousUpdate.catch(() => undefined) : Promise.resolve()).then(async () => {
+			const existingConfigs = (await repository.getConfigs()).filter(config => config.key === key);
+			if (existingConfigs.some(config => config.value === value)) {
+				return;
+			}
+			if (existingConfigs.length === 1 && repository.unsetConfig) {
+				await repository.unsetConfig(key);
+			}
+			await repository.setConfig(key, value);
+		});
+		repositoryUpdates.set(key, update);
+
+		try {
+			await update;
+		} finally {
+			if (repositoryUpdates.get(key) === update) {
+				repositoryUpdates.delete(key);
+				if (!repositoryUpdates.size) {
+					PullRequestGitHelper.configUpdates.delete(repository);
+				}
+			}
+		}
+	}
+
 	static parsePullRequestMetadata(value: string): PullRequestMetadata | undefined {
 		if (value) {
 			const matches = /(.*)#(.*)#(.*)/g.exec(value);
@@ -434,7 +468,7 @@ export class PullRequestGitHelper {
 			}
 			const prConfigKey = `branch.${branchName}.${PullRequestMetadataKey}`;
 			if (pullRequest) {
-				await repository.setConfig(prConfigKey, PullRequestGitHelper.buildPullRequestMetadata(pullRequest));
+				await PullRequestGitHelper.setConfig(repository, prConfigKey, PullRequestGitHelper.buildPullRequestMetadata(pullRequest));
 			} else if (repository.unsetConfig) {
 				await repository.unsetConfig(prConfigKey);
 			}
@@ -458,7 +492,7 @@ export class PullRequestGitHelper {
 			const prConfigKey = `branch.${branch}.${BaseBranchMetadataKey}`;
 			if (base) {
 				Logger.appendLine(`associate ${branch} with base branch ${base.owner}/${base.repo}#${base.branch}`, PullRequestGitHelper.ID);
-				await repository.setConfig(prConfigKey, PullRequestGitHelper.buildBaseBranchMetadata(base.owner, base.repo, base.branch));
+				await PullRequestGitHelper.setConfig(repository, prConfigKey, PullRequestGitHelper.buildBaseBranchMetadata(base.owner, base.repo, base.branch));
 			} else if (repository.unsetConfig) {
 				await repository.unsetConfig(prConfigKey);
 				const vscodeBaseBranchConfigKey = `branch.${branch}.${VscodeBaseBranchMetadataKey}`;

src/test/github/pullRequestGitHelper.test.ts

@@ -189,6 +189,64 @@ describe('PullRequestGitHelper', function () {
 		});
 	});
 
+	describe('associateBranchWithPullRequest', function () {
+		const pullRequest = (number: number) => ({
+			number,
+			base: {
+				repositoryCloneUrl: {
+					owner: 'owner',
+					repositoryName: 'name',
+				},
+			},
+		}) as PullRequestModel;
+
+		it('replaces pull request metadata instead of appending values', async function () {
+			await PullRequestGitHelper.associateBranchWithPullRequest(repository, pullRequest(100), 'feature');
+			await PullRequestGitHelper.associateBranchWithPullRequest(repository, pullRequest(100), 'feature');
+			await PullRequestGitHelper.associateBranchWithPullRequest(repository, pullRequest(101), 'feature');
+
+			const key = 'branch.feature.github-pr-owner-number';
+			assert.deepStrictEqual((await repository.getConfigs()).filter(config => config.key === key), [
+				{ key, value: 'owner#name#101' },
+			]);
+		});
+
+		it('does not append metadata during concurrent associations', async function () {
+			await Promise.all([
+				PullRequestGitHelper.associateBranchWithPullRequest(repository, pullRequest(100), 'feature'),
+				PullRequestGitHelper.associateBranchWithPullRequest(repository, pullRequest(100), 'feature'),
+			]);
+
+			const key = 'branch.feature.github-pr-owner-number';
+			assert.deepStrictEqual((await repository.getConfigs()).filter(config => config.key === key), [
+				{ key, value: 'owner#name#100' },
+			]);
+		});
+
+		it('does not append to existing duplicate metadata', async function () {
+			const key = 'branch.feature.github-pr-owner-number';
+			await repository.setConfig(key, 'owner#name#100');
+			await repository.setConfig(key, 'owner#name#100');
+
+			await PullRequestGitHelper.associateBranchWithPullRequest(repository, pullRequest(100), 'feature');
+
+			assert.strictEqual((await repository.getConfigs()).filter(config => config.key === key).length, 2);
+		});
+	});
+
+	describe('associateBaseBranchWithBranch', function () {
+		it('replaces base branch metadata instead of appending values', async function () {
+			await PullRequestGitHelper.associateBaseBranchWithBranch(repository, 'feature', { owner: 'owner', repo: 'name', branch: 'main' });
+			await PullRequestGitHelper.associateBaseBranchWithBranch(repository, 'feature', { owner: 'owner', repo: 'name', branch: 'main' });
+			await PullRequestGitHelper.associateBaseBranchWithBranch(repository, 'feature', { owner: 'owner', repo: 'name', branch: 'next' });
+
+			const key = 'branch.feature.github-pr-base-branch';
+			assert.deepStrictEqual((await repository.getConfigs()).filter(config => config.key === key), [
+				{ key, value: 'owner#name#next' },
+			]);
+		});
+	});
+
 	describe('getMatchingPullRequestMetadataForBranch', function () {
 		it('returns the highest-numbered PR when duplicate config entries exist for the branch', async function () {
 			// Simulate the case where a branch name has been associated with multiple