agentHost: only offer Sync Changes for a remote-tracking upstream

src/vs/platform/agentHost/common/state/sessionState.ts

@@ -1596,6 +1596,13 @@ export interface ISessionGitState {
 	readonly baseBranchName?: string;
 	/** Upstream tracking branch (e.g. `origin/feature`). */
 	readonly upstreamBranchName?: string;
+	/**
+	 * Remote of the upstream branch when it can be synced through its remote-tracking ref
+	 * (`origin`, `my/fork`), or `.` when it cannot: a local upstream, or a fetch refspec
+	 * that keeps the tracking ref outside `refs/remotes/<remote>/`. Absent when there is
+	 * no upstream, and in git state persisted before this field existed.
+	 */
+	readonly upstreamRemote?: string;
 	/** Number of commits the upstream branch has ahead of the local branch. */
 	readonly incomingChanges?: number;
 	/** Number of commits the local branch has ahead of the upstream branch. */
@@ -1808,6 +1815,7 @@ export function readSessionGitState(meta: SessionMeta | undefined): ISessionGitS
 		isDetachedHead?: boolean;
 		baseBranchName?: string;
 		upstreamBranchName?: string;
+		upstreamRemote?: string;
 		incomingChanges?: number;
 		outgoingChanges?: number;
 		uncommittedChanges?: number;
@@ -1821,6 +1829,7 @@ export function readSessionGitState(meta: SessionMeta | undefined): ISessionGitS
 	if (typeof raw['isDetachedHead'] === 'boolean') { result.isDetachedHead = raw['isDetachedHead']; }
 	if (typeof raw['baseBranchName'] === 'string') { result.baseBranchName = raw['baseBranchName']; }
 	if (typeof raw['upstreamBranchName'] === 'string') { result.upstreamBranchName = raw['upstreamBranchName']; }
+	if (typeof raw['upstreamRemote'] === 'string') { result.upstreamRemote = raw['upstreamRemote']; }
 	if (typeof raw['incomingChanges'] === 'number') { result.incomingChanges = raw['incomingChanges']; }
 	if (typeof raw['outgoingChanges'] === 'number') { result.outgoingChanges = raw['outgoingChanges']; }
 	if (typeof raw['uncommittedChanges'] === 'number') { result.uncommittedChanges = raw['uncommittedChanges']; }
@@ -1843,9 +1852,15 @@ export function readSessionGitState(meta: SessionMeta | undefined): ISessionGitS
  * `HEAD` is a legitimate branch-less checkout and must not be mistaken for it,
  * or every caller would refresh in a loop against a repository that will never
  * report a branch.
+ *
+ * A state that names an upstream but not its remote was persisted before
+ * {@link ISessionGitState.upstreamRemote} existed; recompute it once so the
+ * consumers that need the remote (Sync Changes) do not stay stranded.
  */
 export function needsSessionGitStateRefresh(gitState: ISessionGitState | undefined): boolean {
-	return gitState === undefined || (gitState.branchName === undefined && !gitState.isDetachedHead);
+	return gitState === undefined
+		|| (gitState.branchName === undefined && !gitState.isDetachedHead)
+		|| (gitState.upstreamBranchName !== undefined && gitState.upstreamRemote === undefined);
 }
 
 /**

src/vs/platform/agentHost/node/agentHostGitService.ts

@@ -87,7 +87,7 @@ export class AgentHostGitService implements IAgentHostGitService {
 	}
 
 	async getRefs(workingDirectory: URI, query?: IRefQuery): Promise<GitRef[]> {
-		const args = ['for-each-ref', '--format=%(refname)%00%(upstream)'];
+		const args = ['for-each-ref', '--format=%(refname)%00%(upstream)%00%(upstream:remotename)'];
 
 		if (query?.sort && query.sort !== 'alphabetically') {
 			args.push('--sort', `-${query.sort}`);
@@ -1017,20 +1017,28 @@ export class AgentHostGitService implements IAgentHostGitService {
 		const hasGitHubRemote = parseHasGitHubRemote(remotesOutput);
 		const baseBranchName = configuredBaseBranch ?? parseDefaultBranchRef(defaultBranchRef);
 		const githubRepo = parseGitHubRepoFromRemote(remotesOutput);
-		const upstreamRemote = status.upstreamBranchName?.split('/')[0];
+		// Guessed from the name; only feeds the GitHub head-owner lookup below.
+		const upstreamRemoteGuess = status.upstreamBranchName?.split('/')[0];
 		// `gh pr checkout` can create a local branch whose head lives on a fork but
 		// has no upstream tracking ref; Git still reports the branch's push remote,
 		// which can be a remote name or the literal fork URL.
-		const [pushRemote, baseBranchDivergence] = await Promise.all([
-			!upstreamRemote && status.branchName
+		// The persisted remote is asked from git and only kept when the sync handler can use it.
+		const [pushRemote, baseBranchDivergence, upstreamTrackingRemote] = await Promise.all([
+			!upstreamRemoteGuess && status.branchName
 				? this._getPushRemote(repositoryRoot, status.branchName)
 				: undefined,
 			baseBranchName && status.branchName && status.branchName !== baseBranchName
 				? this._computeBaseBranchDivergence(repositoryRoot, baseBranchName, status.outgoingChanges === undefined)
 				: undefined,
+			status.upstreamBranchName && status.branchName
+				? this._getUpstreamRemote(repositoryRoot, status.branchName)
+				: undefined,
 		]);
-		const githubHeadRepo = upstreamRemote
-			? parseGitHubRepoFromRemote(remotesOutput, upstreamRemote)
+		if (status.upstreamBranchName && !upstreamTrackingRemote) {
+			this._logService.warn(`[agentHostGitService] Could not resolve the upstream remote of ${status.branchName}; the state will be recomputed on the next refresh: ${repositoryRoot.fsPath}`);
+		}
+		const githubHeadRepo = upstreamRemoteGuess
+			? parseGitHubRepoFromRemote(remotesOutput, upstreamRemoteGuess)
 			: parseGitHubHeadRepoFromRemoteSelection(remotesOutput, pushRemote);
 
 		// `git status -b --porcelain=v2` only emits ahead/behind counts when the
@@ -1051,6 +1059,7 @@ export class AgentHostGitService implements IAgentHostGitService {
 			isDetachedHead: status.isDetachedHead,
 			baseBranchName,
 			upstreamBranchName: status.upstreamBranchName,
+			upstreamRemote: upstreamTrackingRemote,
 			incomingChanges: status.incomingChanges,
 			outgoingChanges,
 			uncommittedChanges: status.uncommittedChanges,
@@ -1064,6 +1073,10 @@ export class AgentHostGitService implements IAgentHostGitService {
 		return stripUndefined(result);
 	}
 
+	private async _getUpstreamRemote(repositoryRoot: URI, branchName: string): Promise<string | undefined> {
+		return resolveUpstreamRemote(await this._runGit(repositoryRoot, ['for-each-ref', '--format=%(upstream)%00%(upstream:remotename)', `refs/heads/${branchName}`]));
+	}
+
 	private async _getPushRemote(repositoryRoot: URI, branchName: string): Promise<string | undefined> {
 		return (await this._runGit(repositoryRoot, ['for-each-ref', '--format=%(push:remotename)', `refs/heads/${branchName}`]))?.trim() || undefined;
 	}
@@ -1742,21 +1755,50 @@ export function parseRemoteBranchRef(ref: string): { ref: string; name: string;
 	return { ref, name, remote };
 }
 
+/**
+ * Resolves a branch's upstream from `%(upstream)` and `%(upstream:remotename)`. The remote
+ * name comes from git, so a remote containing `/` (`my/fork`) is not split at the first
+ * segment; a local upstream (remote `.`) has no remote-tracking ref and yields `undefined`.
+ * Without the remote name (older callers), falls back to splitting the ref.
+ */
+export function parseUpstreamRef(upstream: string, upstreamRemote: string | undefined): { ref: string; name: string; remote: string } | undefined {
+	if (!upstreamRemote) {
+		return parseRemoteBranchRef(upstream);
+	}
+	if (upstreamRemote === '.' || !upstream.startsWith(`refs/remotes/${upstreamRemote}/`)) {
+		return undefined;
+	}
+	return { ref: upstream, name: upstream.substring('refs/remotes/'.length), remote: upstreamRemote };
+}
+
+/**
+ * The {@link ISessionGitState.upstreamRemote} value for a `%(upstream)%00%(upstream:remotename)`
+ * line: the remote when {@link parseUpstreamRef} accepts the upstream, which is exactly what
+ * the sync handler needs; `.` for an upstream it cannot sync; `undefined` without an upstream.
+ */
+export function resolveUpstreamRemote(output: string | undefined): string | undefined {
+	const [upstream, remote] = (output ?? '').trim().split(/\r?\n/)[0].split('\0');
+	if (!remote) {
+		return undefined;
+	}
+	return parseUpstreamRef(upstream, remote) ? remote : '.';
+}
+
 export function parseGitRefs(output: string | undefined): GitRef[] {
 	if (!output) {
 		return [];
 	}
 
 	const refs: GitRef[] = [];
 	for (const line of output.split(/\r?\n/g)) {
-		const [ref, upstream] = line.trim().split('\0');
+		const [ref, upstream, upstreamRemote] = line.trim().split('\0');
 
 		if (ref.startsWith('refs/heads/')) {
 			refs.push({
 				ref,
 				name: ref.substring(11),
 				upstream: upstream
-					? parseRemoteBranchRef(upstream)
+					? parseUpstreamRef(upstream, upstreamRemote)
 					: undefined,
 				kind: GitRefType.Head
 			} satisfies IBranch);

src/vs/platform/agentHost/node/agentHostSyncOperationProvider.ts

@@ -39,8 +39,8 @@ export class AgentHostSyncOperationContribution extends Disposable implements IC
 			return undefined;
 		}
 
-		// No upstream branch
-		if (!gitState?.upstreamBranchName) {
+		// No syncable upstream: none, unknown (legacy state), or one the handler cannot sync (`.`)
+		if (!gitState?.upstreamBranchName || !gitState.upstreamRemote || gitState.upstreamRemote === '.') {
 			return undefined;
 		}
 

src/vs/platform/agentHost/test/node/agentHostGitService.integrationTest.ts

@@ -27,7 +27,7 @@ import { FileService } from '../../../files/common/fileService.js';
 import { Schemas } from '../../../../base/common/network.js';
 import { DiskFileSystemProvider } from '../../../files/node/diskFileSystemProvider.js';
 import { DisposableStore } from '../../../../base/common/lifecycle.js';
-import { CheckoutBlockedByLocalChangesError } from '../../common/agentHostGitService.js';
+import { CheckoutBlockedByLocalChangesError, GitRefType } from '../../common/agentHostGitService.js';
 import { AgentHostGitService } from '../../node/agentHostGitService.js';
 
 class TestLogService extends NullLogService {
@@ -120,11 +120,78 @@ suite('AgentHostGitService - getSessionGitState (real git)', () => {
 			githubHeadOwner: result?.githubHeadOwner,
 			githubRepo: result?.githubRepo,
 			upstreamBranchName: result?.upstreamBranchName,
+			upstreamRemote: result?.upstreamRemote,
 		}, {
 			githubOwner: 'base-owner',
 			githubHeadOwner: 'fork-owner',
 			githubRepo: 'repo',
 			upstreamBranchName: 'fork/feature',
+			upstreamRemote: 'fork',
+		});
+	});
+
+	(hasGit ? test : test.skip)('reports an unsyncable upstream remote when the upstream is a local branch', async () => {
+		const dir = initRepo({ remote: 'https://github.com/owner/repo.git' });
+		cp.execFileSync('git', ['branch', 'feature/base'], { cwd: dir, stdio: 'pipe' });
+		cp.execFileSync('git', ['checkout', '-q', '-b', 'topic'], { cwd: dir, stdio: 'pipe' });
+		cp.execFileSync('git', ['branch', '--set-upstream-to', 'feature/base'], { cwd: dir, stdio: 'pipe' });
+
+		const result = await svc!.getSessionGitState(URI.file(dir));
+		const branch = await svc!.getBranch(URI.file(dir), 'topic');
+
+		assert.deepStrictEqual({
+			upstreamBranchName: result?.upstreamBranchName,
+			upstreamRemote: result?.upstreamRemote,
+			githubHeadOwner: result?.githubHeadOwner,
+			branchUpstream: branch?.kind === GitRefType.Head ? branch.upstream : 'no branch',
+		}, {
+			upstreamBranchName: 'feature/base',
+			upstreamRemote: '.',
+			githubHeadOwner: undefined,
+			branchUpstream: undefined,
+		});
+	});
+
+	(hasGit ? test : test.skip)('reports no syncable remote when a fetch refspec keeps the tracking ref outside refs/remotes', async () => {
+		const dir = initRepo({ remote: 'https://github.com/owner/repo.git' });
+		cp.execFileSync('git', ['config', 'remote.origin.fetch', '+refs/heads/*:refs/custom/origin/*'], { cwd: dir, stdio: 'pipe' });
+		cp.execFileSync('git', ['checkout', '-q', '-b', 'feature'], { cwd: dir, stdio: 'pipe' });
+		cp.execFileSync('git', ['update-ref', 'refs/custom/origin/feature', 'HEAD'], { cwd: dir, stdio: 'pipe' });
+		cp.execFileSync('git', ['config', 'branch.feature.remote', 'origin'], { cwd: dir, stdio: 'pipe' });
+		cp.execFileSync('git', ['config', 'branch.feature.merge', 'refs/heads/feature'], { cwd: dir, stdio: 'pipe' });
+
+		const result = await svc!.getSessionGitState(URI.file(dir));
+		const branch = await svc!.getBranch(URI.file(dir), 'feature');
+
+		assert.deepStrictEqual({
+			hasUpstream: result?.upstreamBranchName !== undefined,
+			upstreamRemote: result?.upstreamRemote,
+			branchUpstream: branch?.kind === GitRefType.Head ? branch.upstream : 'no branch',
+		}, {
+			hasUpstream: true,
+			upstreamRemote: '.',
+			branchUpstream: undefined,
+		});
+	});
+
+	(hasGit ? test : test.skip)('reports the full upstream remote name when it contains a slash', async () => {
+		const dir = initRepo({ remote: 'https://github.com/base-owner/repo.git' });
+		cp.execFileSync('git', ['remote', 'add', 'my/fork', 'https://github.com/fork-owner/repo.git'], { cwd: dir, stdio: 'pipe' });
+		cp.execFileSync('git', ['checkout', '-q', '-b', 'feature'], { cwd: dir, stdio: 'pipe' });
+		cp.execFileSync('git', ['update-ref', 'refs/remotes/my/fork/feature', 'HEAD'], { cwd: dir, stdio: 'pipe' });
+		cp.execFileSync('git', ['branch', '--set-upstream-to', 'my/fork/feature'], { cwd: dir, stdio: 'pipe' });
+
+		const result = await svc!.getSessionGitState(URI.file(dir));
+		const branch = await svc!.getBranch(URI.file(dir), 'feature');
+
+		assert.deepStrictEqual({
+			upstreamBranchName: result?.upstreamBranchName,
+			upstreamRemote: result?.upstreamRemote,
+			branchUpstream: branch?.kind === GitRefType.Head ? branch.upstream : undefined,
+		}, {
+			upstreamBranchName: 'my/fork/feature',
+			upstreamRemote: 'my/fork',
+			branchUpstream: { ref: 'refs/remotes/my/fork/feature', name: 'my/fork/feature', remote: 'my/fork' },
 		});
 	});
 

src/vs/platform/agentHost/test/node/agentHostGitService.test.ts

@@ -5,10 +5,10 @@
 
 import assert from 'assert';
 import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js';
-import { formatGitError, getRemoteTrackingRef, GitCheckoutProgressParser, isRetryableWorktreeRemovalError, parseChangedPaths, parseDefaultBranchRef, parseFetchRemoteUrls, parseGitDiffRawNumstat, parseGitHubRepoFromRemote, parseGitStatusV2, parseHasGitHubRemote, parseSingleLsTreeEntry, parseUntrackedPaths, summarizeStderrForError } from '../../node/agentHostGitService.js';
+import { formatGitError, getRemoteTrackingRef, GitCheckoutProgressParser, isRetryableWorktreeRemovalError, parseChangedPaths, parseDefaultBranchRef, parseFetchRemoteUrls, parseGitDiffRawNumstat, parseGitHubRepoFromRemote, parseGitRefs, parseGitStatusV2, parseHasGitHubRemote, parseSingleLsTreeEntry, parseUntrackedPaths, parseUpstreamRef, resolveUpstreamRemote, summarizeStderrForError } from '../../node/agentHostGitService.js';
 import { buildGitBlobUri } from '../../node/gitDiffContent.js';
 import { URI } from '../../../../base/common/uri.js';
-import { EMPTY_TREE_OBJECT, getBranchCompletions, resolveDiffBaseBranchName } from '../../common/agentHostGitService.js';
+import { EMPTY_TREE_OBJECT, getBranchCompletions, GitRefType, resolveDiffBaseBranchName } from '../../common/agentHostGitService.js';
 import { needsSessionGitStateRefresh } from '../../common/state/sessionState.js';
 
 suite('AgentHostGitService', () => {
@@ -162,15 +162,81 @@ suite('AgentHostGitService', () => {
 				probeFailureRemnant: needsSessionGitStateRefresh({ baseBranchName: 'main' }),
 				detachedHead: needsSessionGitStateRefresh({ isDetachedHead: true, baseBranchName: 'main' }),
 				onABranch: needsSessionGitStateRefresh({ branchName: 'feature', baseBranchName: 'main' }),
+				// Persisted before upstreamRemote existed: the remote is needed to offer Sync Changes.
+				upstreamWithoutRemote: needsSessionGitStateRefresh({ branchName: 'feature', upstreamBranchName: 'origin/feature' }),
+				remoteUpstream: needsSessionGitStateRefresh({ branchName: 'feature', upstreamBranchName: 'origin/feature', upstreamRemote: 'origin' }),
+				localUpstream: needsSessionGitStateRefresh({ branchName: 'feature', upstreamBranchName: 'main', upstreamRemote: '.' }),
 			}, {
 				neverComputed: true,
 				probeFailureRemnant: true,
 				detachedHead: false,
 				onABranch: false,
+				upstreamWithoutRemote: true,
+				remoteUpstream: false,
+				localUpstream: false,
 			});
 		});
 	});
 
+	suite('parseUpstreamRef', () => {
+		test('takes the remote name from git instead of splitting the ref', () => {
+			assert.deepStrictEqual({
+				origin: parseUpstreamRef('refs/remotes/origin/feature', 'origin'),
+				slashRemote: parseUpstreamRef('refs/remotes/my/fork/feature', 'my/fork'),
+				local: parseUpstreamRef('refs/heads/main', '.'),
+				mismatch: parseUpstreamRef('refs/remotes/origin/feature', 'other'),
+				legacy: parseUpstreamRef('refs/remotes/origin/feature', undefined),
+				empty: parseUpstreamRef('refs/remotes/origin/feature', ''),
+			}, {
+				origin: { ref: 'refs/remotes/origin/feature', name: 'origin/feature', remote: 'origin' },
+				slashRemote: { ref: 'refs/remotes/my/fork/feature', name: 'my/fork/feature', remote: 'my/fork' },
+				local: undefined,
+				mismatch: undefined,
+				legacy: { ref: 'refs/remotes/origin/feature', name: 'origin/feature', remote: 'origin' },
+				empty: { ref: 'refs/remotes/origin/feature', name: 'origin/feature', remote: 'origin' },
+			});
+		});
+	});
+
+	suite('resolveUpstreamRemote', () => {
+		test('keeps the remote only when the sync handler can use the upstream', () => {
+			assert.deepStrictEqual({
+				noUpstream: resolveUpstreamRemote('\0'),
+				probeFailed: resolveUpstreamRemote(undefined),
+				origin: resolveUpstreamRemote('refs/remotes/origin/feature\0origin'),
+				slashRemote: resolveUpstreamRemote('refs/remotes/my/fork/feature\0my/fork'),
+				local: resolveUpstreamRemote('refs/heads/main\0.'),
+				customRefspec: resolveUpstreamRemote('refs/custom/origin/feature\0origin'),
+			}, {
+				noUpstream: undefined,
+				probeFailed: undefined,
+				origin: 'origin',
+				slashRemote: 'my/fork',
+				local: '.',
+				customRefspec: '.',
+			});
+		});
+	});
+
+	suite('parseGitRefs', () => {
+		test('reads the upstream remote column', () => {
+			const out = [
+				'refs/heads/main\0\0',
+				'refs/heads/topic\0refs/heads/main\0.',
+				'refs/heads/feature\0refs/remotes/my/fork/feature\0my/fork',
+				'refs/remotes/my/fork/feature\0\0',
+				'refs/heads/legacy\0refs/remotes/origin/legacy',
+			].join('\n');
+			assert.deepStrictEqual(parseGitRefs(out).map(ref => ref.kind === GitRefType.Head ? { name: ref.name, upstream: ref.upstream } : { name: ref.name, kind: ref.kind }), [
+				{ name: 'main', upstream: undefined },
+				{ name: 'topic', upstream: undefined },
+				{ name: 'feature', upstream: { ref: 'refs/remotes/my/fork/feature', name: 'my/fork/feature', remote: 'my/fork' } },
+				{ name: 'my/fork/feature', kind: GitRefType.RemoteHead },
+				{ name: 'legacy', upstream: { ref: 'refs/remotes/origin/legacy', name: 'origin/legacy', remote: 'origin' } },
+			]);
+		});
+	});
+
 	suite('parseHasGitHubRemote', () => {
 		test('detects ssh github remote', () => {
 			assert.strictEqual(parseHasGitHubRemote('origin\tgit@github.com:owner/repo.git (fetch)\n'), true);