Skip to content

Commit 8ab17d7

Browse files
zaknafeynTest
andauthored
fix: resolve configured remote instead of hardcoding origin (#219)
* fix: resolve configured remote instead of hardcoding origin getDefaultBranch and pushSetUpstream always assumed a remote named "origin", breaking fork workflows where the only remote is e.g. "upstream". Both now take a remote parameter (still defaulting to "origin" for source compatibility), and every call site resolves the actual remote via the existing resolveRemote/remoteSelection helpers (respecting the defaultRemote setting) instead of assuming origin. getDefaultBranch also falls back to `git remote show <remote>` when the local origin/HEAD symbolic ref was never set, before giving up and guessing a local main/master branch. Closes #210 * chore: retrigger CI * test: fix e2e ConfigurationManager stub broken by remote-resolution change TestableCheckoutToCommand in checkoutInlineActions.test.ts used '{} as ConfigurationManager', which throws on .get() now that publishBranchAction/deleteLocalBranchAction call resolveRemoteInteractive(git, { defaultRemote: configManager.get()... }). Give it a working get() stub, matching the unit-test fix already applied in checkoutToInlineActions.test.ts. --------- Co-authored-by: Test <t@t.local>
1 parent 35194fb commit 8ab17d7

6 files changed

Lines changed: 130 additions & 10 deletions

File tree

src/commands/checkoutToCommand/index.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { LoggingService } from '../../logging/loggingService';
1010
import { AutoStashService } from '../../services/autoStashService';
1111
import { RefDetailsCache } from '../../services/refDetailsCache';
1212
import { getRepoId } from '../../utils/getRepoId';
13+
import { resolveRemoteInteractive } from '../../utils/remoteSelection';
1314
import { UserCancelledError } from '../../utils/userCancelledError';
1415
import { BaseCommand } from '../command';
1516
import { validateBranchName } from '../createBranchFromTemplateCommand/validateBranchName';
@@ -386,12 +387,21 @@ export class CheckoutToCommand extends BaseCommand {
386387

387388
protected async publishBranchAction(git: GitExecutor, ref: IGitRef): Promise<boolean> {
388389
try {
390+
const remote = await resolveRemoteInteractive(git, {
391+
branch: ref.name,
392+
defaultRemote: this.configManager.get().defaultRemote,
393+
purpose: 'push',
394+
});
389395
await vscode.window.withProgress(
390396
{ location: vscode.ProgressLocation.Notification, title: `Publishing ${ref.name}` },
391-
() => git.pushSetUpstream(ref.name)
397+
() => git.pushSetUpstream(ref.name, remote)
392398
);
393399
return true;
394400
} catch (e) {
401+
if (e instanceof UserCancelledError) {
402+
// User dismissed the remote picker — not an error.
403+
return false;
404+
}
395405
captureException(e);
396406
const msg = e instanceof Error ? e.message : String(e);
397407
await this.showErrorMessage(`Failed to publish branch ${ref.name}: ${msg}`, 'OK');
@@ -470,7 +480,12 @@ export class CheckoutToCommand extends BaseCommand {
470480
let force = false;
471481
let defaultBranch: string | undefined;
472482
try {
473-
defaultBranch = await git.getDefaultBranch();
483+
const remote = await resolveRemoteInteractive(git, {
484+
branch: ref.name,
485+
defaultRemote: this.configManager.get().defaultRemote,
486+
purpose: 'fetch',
487+
});
488+
defaultBranch = await git.getDefaultBranch(remote);
474489
const merged = await git.getMergedBranches(defaultBranch);
475490
force = !merged.includes(ref.name);
476491
} catch (e) {

src/commands/cleanupBranchesCommand/index.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import { GitExecutor } from '../../common/git/gitExecutor';
44
import { VscodeGitProvider } from '../../common/git/vscodeGitProvider';
55
import { ConfigurationManager } from '../../configuration/configurationManager';
66
import { LoggingService } from '../../logging/loggingService';
7+
import { resolveRemoteInteractive } from '../../utils/remoteSelection';
8+
import { UserCancelledError } from '../../utils/userCancelledError';
79
import { BaseCommand } from '../command';
810
import {
911
buildCleanupQuickPickItems,
@@ -33,8 +35,17 @@ export class CleanupBranchesCommand extends BaseCommand {
3335

3436
let base: string;
3537
try {
36-
base = await git.getDefaultBranch();
38+
const remote = await resolveRemoteInteractive(git, {
39+
branch: current,
40+
defaultRemote: this.configManager.get().defaultRemote,
41+
purpose: 'fetch',
42+
});
43+
base = await git.getDefaultBranch(remote);
3744
} catch (error) {
45+
if (error instanceof UserCancelledError) {
46+
// User dismissed the remote picker — not an error.
47+
return;
48+
}
3849
await vscode.window.showErrorMessage(
3950
`Could not determine the default branch: ${error instanceof Error ? error.message : String(error)}`
4051
);

src/common/git/gitExecutor.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,8 +1108,8 @@ export class GitExecutor {
11081108
await this.#execGitCommand(['tag', '-d', name]);
11091109
}
11101110

1111-
async pushSetUpstream(branch: string): Promise<void> {
1112-
await this.#execGitCommand(['push', '-u', 'origin', branch]);
1111+
async pushSetUpstream(branch: string, remote = 'origin'): Promise<void> {
1112+
await this.#execGitCommand(['push', '-u', remote, branch]);
11131113
}
11141114

11151115
async getMergedBranches(base: string): Promise<string[]> {
@@ -1150,11 +1150,24 @@ export class GitExecutor {
11501150
return !!line && line.trimStart().startsWith('-');
11511151
}
11521152

1153-
async getDefaultBranch(): Promise<string> {
1153+
async getDefaultBranch(remote = 'origin'): Promise<string> {
11541154
try {
1155-
const { stdout } = await this.#execGitCommand(['symbolic-ref', '--short', 'refs/remotes/origin/HEAD']);
1156-
return stdout.trim().replace(/^origin\//, '');
1155+
const { stdout } = await this.#execGitCommand(['symbolic-ref', '--short', `refs/remotes/${remote}/HEAD`]);
1156+
return stdout.trim().replace(new RegExp(`^${remote}/`), '');
11571157
} catch {
1158+
// The symbolic ref is only populated after an explicit `git remote set-head`
1159+
// (or a clone that set it up); many repos never have it. `git remote show`
1160+
// asks the remote directly for its HEAD branch instead.
1161+
try {
1162+
const { stdout } = await this.#execGitCommand(['remote', 'show', remote]);
1163+
const match = stdout.match(/HEAD branch:\s*(\S+)/);
1164+
if (match && match[1] && match[1] !== '(unknown)') {
1165+
return match[1];
1166+
}
1167+
} catch {
1168+
// Remote unreachable or doesn't exist — fall through to local guesses.
1169+
}
1170+
11581171
const refs = await this.getAllRefListExtended();
11591172
if (refs.some((ref) => !ref.remote && !ref.isTag && ref.name === 'main')) return 'main';
11601173
if (refs.some((ref) => !ref.remote && !ref.isTag && ref.name === 'master')) return 'master';

src/test/e2e/checkoutInlineActions.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ class TestableCheckoutToCommand extends CheckoutToCommand {
2727
errorMessages: string[] = [];
2828

2929
constructor() {
30-
super({} as ConfigurationManager, mockLogService, {} as AutoStashService);
30+
const configManager = { get: () => ({ defaultRemote: undefined }) } as unknown as ConfigurationManager;
31+
super(configManager, mockLogService, {} as AutoStashService);
3132
}
3233

3334
protected async showInformationMessage(message: string): Promise<string | undefined> {

src/test/unit/checkoutToInlineActions.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ class TestableCheckoutToCommand extends CheckoutToCommand {
3333
infoMessages: string[] = [];
3434

3535
constructor() {
36-
super({} as ConfigurationManager, mockLogService, {} as AutoStashService);
36+
const configManager = { get: () => ({ defaultRemote: undefined }) } as unknown as ConfigurationManager;
37+
super(configManager, mockLogService, {} as AutoStashService);
3738
}
3839

3940
// Avoid popping a real (blocking) notification during tests — just record it.

src/test/unit/cleanupBranches.test.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,85 @@ describe('GitExecutor.getDefaultBranch', () => {
414414
await assert.rejects(() => git.getDefaultBranch(), /Could not determine the default branch/);
415415
fs.rmSync(dir, { recursive: true, force: true });
416416
});
417+
418+
it('resolves the HEAD branch of a non-"origin" remote (e.g. a fork\'s "upstream")', async () => {
419+
const remoteDir = initRepo('gsc-default-upstream-remote-');
420+
execSync('git checkout -q -b develop', { cwd: remoteDir });
421+
commit(remoteDir, 'init');
422+
423+
const dir = initRepo('gsc-default-upstream-local-');
424+
execSync('git checkout -q -b unrelated', { cwd: dir });
425+
commit(dir, 'init');
426+
execSync(`git remote add upstream "${remoteDir}"`, { cwd: dir });
427+
execSync('git fetch -q upstream', { cwd: dir });
428+
execSync('git remote set-head upstream develop', { cwd: dir });
429+
430+
const git = new GitExecutor(dir, mockLogService as unknown as LoggingService);
431+
assert.strictEqual(await git.getDefaultBranch('upstream'), 'develop');
432+
fs.rmSync(remoteDir, { recursive: true, force: true });
433+
fs.rmSync(dir, { recursive: true, force: true });
434+
});
435+
436+
it('falls back to "git remote show <remote>" when the symbolic ref is absent, then to local main/master', async () => {
437+
// A repo with only an "upstream" remote and no `refs/remotes/upstream/HEAD`
438+
// symbolic ref set locally (e.g. never explicitly `git remote set-head`d)
439+
// must still resolve via `git remote show upstream`, not throw or assume "origin".
440+
const remoteDir = initRepo('gsc-default-show-remote-');
441+
execSync('git checkout -q -b develop', { cwd: remoteDir });
442+
commit(remoteDir, 'init');
443+
444+
const dir = initRepo('gsc-default-show-local-');
445+
execSync('git checkout -q -b unrelated', { cwd: dir });
446+
commit(dir, 'init');
447+
execSync(`git remote add upstream "${remoteDir}"`, { cwd: dir });
448+
execSync('git fetch -q upstream', { cwd: dir });
449+
// Intentionally no `git remote set-head` — refs/remotes/upstream/HEAD is absent.
450+
451+
const git = new GitExecutor(dir, mockLogService as unknown as LoggingService);
452+
assert.strictEqual(await git.getDefaultBranch('upstream'), 'develop');
453+
fs.rmSync(remoteDir, { recursive: true, force: true });
454+
fs.rmSync(dir, { recursive: true, force: true });
455+
});
456+
});
457+
458+
describe('GitExecutor.pushSetUpstream', () => {
459+
function initRepo(prefix: string): string {
460+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix));
461+
execSync('git init -q', { cwd: dir });
462+
execSync('git config user.email "test@test.local"', { cwd: dir });
463+
execSync('git config user.name "Test"', { cwd: dir });
464+
return dir;
465+
}
466+
467+
function commit(dir: string, message: string) {
468+
fs.writeFileSync(path.join(dir, 'file.txt'), `${message}\n`);
469+
execSync('git add file.txt', { cwd: dir });
470+
execSync(`git commit -q -m "${message}"`, { cwd: dir });
471+
}
472+
473+
it('pushes and sets upstream tracking against the given remote, not "origin"', async () => {
474+
const remoteDir = initRepo('gsc-push-remote-');
475+
execSync('git checkout -q -b main', { cwd: remoteDir });
476+
commit(remoteDir, 'init');
477+
478+
const dir = initRepo('gsc-push-local-');
479+
execSync(`git remote add upstream "${remoteDir}"`, { cwd: dir });
480+
execSync('git fetch -q upstream', { cwd: dir });
481+
execSync('git checkout -q -b feat upstream/main', { cwd: dir });
482+
commit(dir, 'feat change');
483+
484+
const git = new GitExecutor(dir, mockLogService as unknown as LoggingService);
485+
await git.pushSetUpstream('feat', 'upstream');
486+
487+
const remoteBranches = execSync('git branch', { cwd: remoteDir }).toString();
488+
assert.ok(remoteBranches.includes('feat'), `expected "feat" to be pushed to remote, got: ${remoteBranches}`);
489+
490+
const upstreamRef = execSync('git rev-parse --abbrev-ref feat@{upstream}', { cwd: dir }).toString().trim();
491+
assert.strictEqual(upstreamRef, 'upstream/feat');
492+
493+
fs.rmSync(remoteDir, { recursive: true, force: true });
494+
fs.rmSync(dir, { recursive: true, force: true });
495+
});
417496
});
418497

419498
describe('GitExecutor.getMergedBranches', () => {

0 commit comments

Comments
 (0)