fix(walkthrough): use remote default branch
This commit is contained in:
@@ -0,0 +1,30 @@
|
|||||||
|
import { describe, expect, test } from 'bun:test';
|
||||||
|
import { deriveBaseBranch, hasResolvableBaseBranch } from './baseBranch';
|
||||||
|
|
||||||
|
describe('deriveBaseBranch', () => {
|
||||||
|
test('prefers the remote default branch hint over conventional fallbacks', () => {
|
||||||
|
expect(deriveBaseBranch({
|
||||||
|
remoteNames: new Set(['origin']),
|
||||||
|
localBranches: ['next'],
|
||||||
|
rootBranchHint: 'origin/react',
|
||||||
|
})).toBe('react');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('hasResolvableBaseBranch', () => {
|
||||||
|
test('rejects the main fallback when it does not exist', () => {
|
||||||
|
expect(hasResolvableBaseBranch({
|
||||||
|
baseBranch: 'main',
|
||||||
|
localBranches: ['next', 'react'],
|
||||||
|
remoteBranches: ['origin/next', 'origin/react'],
|
||||||
|
})).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('accepts a base branch available through a remote-tracking ref', () => {
|
||||||
|
expect(hasResolvableBaseBranch({
|
||||||
|
baseBranch: 'main',
|
||||||
|
localBranches: ['next'],
|
||||||
|
remoteBranches: ['origin/main', 'origin/next'],
|
||||||
|
})).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -62,3 +62,18 @@ export const deriveBaseBranch = (options: {
|
|||||||
if (localBranches.includes('develop')) return 'develop';
|
if (localBranches.includes('develop')) return 'develop';
|
||||||
return 'main';
|
return 'main';
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether a base branch can be resolved locally or through one of the active
|
||||||
|
* remote-tracking refs. Callers must not offer comparisons against the `main`
|
||||||
|
* fallback when that ref does not actually exist in the repository.
|
||||||
|
*/
|
||||||
|
export const hasResolvableBaseBranch = (options: {
|
||||||
|
baseBranch: string;
|
||||||
|
localBranches: readonly string[];
|
||||||
|
remoteBranches: readonly string[];
|
||||||
|
}): boolean => {
|
||||||
|
const { baseBranch, localBranches, remoteBranches } = options;
|
||||||
|
return localBranches.includes(baseBranch)
|
||||||
|
|| remoteBranches.some((branch) => branch.endsWith(`/${baseBranch}`));
|
||||||
|
};
|
||||||
|
|||||||
@@ -14,10 +14,10 @@ import { useI18n, type Locale } from '@/lib/i18n';
|
|||||||
import { buildWalkthroughView } from '@/lib/walkthrough/model';
|
import { buildWalkthroughView } from '@/lib/walkthrough/model';
|
||||||
import type { WalkthroughSource, WalkthroughWorkingTreeScope } from '@/lib/walkthrough/types';
|
import type { WalkthroughSource, WalkthroughWorkingTreeScope } from '@/lib/walkthrough/types';
|
||||||
import { ModelSelector } from '@/components/sections/agents/ModelSelector';
|
import { ModelSelector } from '@/components/sections/agents/ModelSelector';
|
||||||
import { deriveBaseBranch } from '@/components/views/git/baseBranch';
|
import { deriveBaseBranch, hasResolvableBaseBranch } from '@/components/views/git/baseBranch';
|
||||||
import { runtimeFetch } from '@/lib/runtime-fetch';
|
import { runtimeFetch } from '@/lib/runtime-fetch';
|
||||||
import { useConfigStore } from '@/stores/useConfigStore';
|
import { useConfigStore } from '@/stores/useConfigStore';
|
||||||
import { useGitBranches, useGitStatus } from '@/stores/useGitStore';
|
import { useGitBranches, useGitStatus, useGitStore } from '@/stores/useGitStore';
|
||||||
import { useGitHubAuthStore } from '@/stores/useGitHubAuthStore';
|
import { useGitHubAuthStore } from '@/stores/useGitHubAuthStore';
|
||||||
import {
|
import {
|
||||||
getFreshestPrStatusForBranch,
|
getFreshestPrStatusForBranch,
|
||||||
@@ -152,6 +152,12 @@ export const WalkthroughView = ({ directory }: WalkthroughViewProps) => {
|
|||||||
|
|
||||||
const status = useGitStatus(directory || null);
|
const status = useGitStatus(directory || null);
|
||||||
const branches = useGitBranches(directory || null);
|
const branches = useGitBranches(directory || null);
|
||||||
|
const ensureAll = useGitStore((state) => state.ensureAll);
|
||||||
|
const { github, git } = useRuntimeAPIs();
|
||||||
|
|
||||||
|
useEffect(() => {
|
||||||
|
if (directory) void ensureAll(directory, git);
|
||||||
|
}, [directory, ensureAll, git]);
|
||||||
|
|
||||||
// The branch source reviews everything on this branch that is not on its
|
// The branch source reviews everything on this branch that is not on its
|
||||||
// base. Three-dot semantics server-side mean merges from the base are
|
// base. Three-dot semantics server-side mean merges from the base are
|
||||||
@@ -162,22 +168,28 @@ export const WalkthroughView = ({ directory }: WalkthroughViewProps) => {
|
|||||||
if (!headRef) return null;
|
if (!headRef) return null;
|
||||||
const all = branches?.all ?? [];
|
const all = branches?.all ?? [];
|
||||||
const localBranches = all.filter((name) => !name.startsWith('remotes/'));
|
const localBranches = all.filter((name) => !name.startsWith('remotes/'));
|
||||||
|
const remoteBranches = all
|
||||||
|
.filter((name) => name.startsWith('remotes/'))
|
||||||
|
.map((name) => name.slice('remotes/'.length));
|
||||||
const remoteNames = new Set(
|
const remoteNames = new Set(
|
||||||
all
|
remoteBranches
|
||||||
.filter((name) => name.startsWith('remotes/'))
|
.map((name) => name.split('/')[0])
|
||||||
.map((name) => name.slice('remotes/'.length).split('/')[0])
|
|
||||||
.filter(Boolean)
|
.filter(Boolean)
|
||||||
);
|
);
|
||||||
const baseRef = deriveBaseBranch({ remoteNames, localBranches });
|
const trackingRemote = status?.tracking?.split('/')[0];
|
||||||
if (!baseRef || baseRef === headRef) return null;
|
const rootBranchHint = (trackingRemote && branches?.defaultBranches?.[trackingRemote])
|
||||||
|
?? branches?.defaultBranches?.origin;
|
||||||
|
const baseRef = deriveBaseBranch({ remoteNames, localBranches, rootBranchHint });
|
||||||
|
if (!baseRef || baseRef === headRef || !hasResolvableBaseBranch({ baseBranch: baseRef, localBranches, remoteBranches })) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
return { kind: 'branch', baseRef, headRef };
|
return { kind: 'branch', baseRef, headRef };
|
||||||
}, [branches, currentBranch]);
|
}, [branches, currentBranch, status?.tracking]);
|
||||||
|
|
||||||
// The pull request for this branch used to appear only after visiting the PR
|
// The pull request for this branch used to appear only after visiting the PR
|
||||||
// panel, because nothing else asked GitHub about it. Ask here too: the status
|
// panel, because nothing else asked GitHub about it. Ask here too: the status
|
||||||
// store already dedupes by signature and throttles by TTL, so several panels
|
// store already dedupes by signature and throttles by TTL, so several panels
|
||||||
// wanting the same answer produce one request.
|
// wanting the same answer produce one request.
|
||||||
const { github } = useRuntimeAPIs();
|
|
||||||
const githubConnected = useGitHubAuthStore((state) => state.status?.connected ?? false);
|
const githubConnected = useGitHubAuthStore((state) => state.status?.connected ?? false);
|
||||||
const githubAuthChecked = useGitHubAuthStore((state) => state.hasChecked);
|
const githubAuthChecked = useGitHubAuthStore((state) => state.hasChecked);
|
||||||
const ensurePrStatusEntry = useGitHubPrStatusStore((state) => state.ensureEntry);
|
const ensurePrStatusEntry = useGitHubPrStatusStore((state) => state.ensureEntry);
|
||||||
|
|||||||
@@ -183,6 +183,7 @@ export interface GitBranch {
|
|||||||
all: string[];
|
all: string[];
|
||||||
current: string;
|
current: string;
|
||||||
branches: Record<string, GitBranchDetails>;
|
branches: Record<string, GitBranchDetails>;
|
||||||
|
defaultBranches?: Record<string, string>;
|
||||||
}
|
}
|
||||||
|
|
||||||
interface GitCommitSummary {
|
interface GitCommitSummary {
|
||||||
|
|||||||
@@ -105,6 +105,7 @@ The following functions are internal helpers used by exported functions:
|
|||||||
- `ahead`: Number of commits ahead of upstream.
|
- `ahead`: Number of commits ahead of upstream.
|
||||||
- `behind`: Number of commits behind upstream.
|
- `behind`: Number of commits behind upstream.
|
||||||
- `upstreamComparison`: Optional comparison against `upstream/<current-branch>`, with `{ remote, branch, ahead, behind }`.
|
- `upstreamComparison`: Optional comparison against `upstream/<current-branch>`, with `{ remote, branch, ahead, behind }`.
|
||||||
|
- `defaultBranches`: Remote default branches derived from local symbolic refs such as `remotes/origin/HEAD -> origin/main`, keyed by remote name. Omitted by runtimes that do not provide this Git metadata.
|
||||||
- `files`: Array of file objects with `path`, `index`, `working_dir` status codes.
|
- `files`: Array of file objects with `path`, `index`, `working_dir` status codes.
|
||||||
- `isClean`: Boolean indicating if working tree is clean.
|
- `isClean`: Boolean indicating if working tree is clean.
|
||||||
- `diffStats`: Object mapping file paths to `{ insertions, deletions }`.
|
- `diffStats`: Object mapping file paths to `{ insertions, deletions }`.
|
||||||
|
|||||||
@@ -3367,6 +3367,7 @@ export async function getBranches(directory) {
|
|||||||
const allBranches = result.all;
|
const allBranches = result.all;
|
||||||
const remoteBranches = allBranches.filter(branch => branch.startsWith('remotes/'));
|
const remoteBranches = allBranches.filter(branch => branch.startsWith('remotes/'));
|
||||||
const activeRemoteBranches = await filterActiveRemoteBranches(git, remoteBranches);
|
const activeRemoteBranches = await filterActiveRemoteBranches(git, remoteBranches);
|
||||||
|
const defaultBranches = await getRemoteDefaultBranches(git);
|
||||||
|
|
||||||
const filteredAll = [
|
const filteredAll = [
|
||||||
...allBranches.filter(branch => !branch.startsWith('remotes/')),
|
...allBranches.filter(branch => !branch.startsWith('remotes/')),
|
||||||
@@ -3376,7 +3377,8 @@ export async function getBranches(directory) {
|
|||||||
return {
|
return {
|
||||||
all: filteredAll,
|
all: filteredAll,
|
||||||
current: result.current,
|
current: result.current,
|
||||||
branches: result.branches
|
branches: result.branches,
|
||||||
|
defaultBranches,
|
||||||
};
|
};
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
console.error('Failed to get branches:', error);
|
console.error('Failed to get branches:', error);
|
||||||
@@ -3384,6 +3386,28 @@ export async function getBranches(directory) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function getRemoteDefaultBranches(git) {
|
||||||
|
try {
|
||||||
|
const refs = await git.raw([
|
||||||
|
'for-each-ref',
|
||||||
|
'--format=%(refname) %(symref)',
|
||||||
|
'refs/remotes',
|
||||||
|
]);
|
||||||
|
return Object.fromEntries(
|
||||||
|
refs.trim().split('\n').flatMap((line) => {
|
||||||
|
const [ref, symbolicRef] = line.split(' ');
|
||||||
|
const match = ref.match(/^refs\/remotes\/([^/]+)\/HEAD$/);
|
||||||
|
const prefix = match ? `refs/remotes/${match[1]}/` : '';
|
||||||
|
return match && typeof symbolicRef === 'string' && symbolicRef.startsWith(prefix)
|
||||||
|
? [[match[1], symbolicRef.slice(prefix.length)]]
|
||||||
|
: [];
|
||||||
|
})
|
||||||
|
);
|
||||||
|
} catch {
|
||||||
|
return {};
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async function filterActiveRemoteBranches(git, remoteBranches) {
|
async function filterActiveRemoteBranches(git, remoteBranches) {
|
||||||
try {
|
try {
|
||||||
const remotes = await git.getRemotes();
|
const remotes = await git.getRemotes();
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import {
|
|||||||
cherryPick,
|
cherryPick,
|
||||||
createWorktree,
|
createWorktree,
|
||||||
getWorktreeBootstrapStatus,
|
getWorktreeBootstrapStatus,
|
||||||
|
getBranches,
|
||||||
getStatus,
|
getStatus,
|
||||||
isGitRepository,
|
isGitRepository,
|
||||||
populateWorktreeWithLockRecovery,
|
populateWorktreeWithLockRecovery,
|
||||||
@@ -988,3 +989,25 @@ describe('hash validation', () => {
|
|||||||
).rejects.not.toThrow('Invalid commit hash');
|
).rejects.not.toThrow('Invalid commit hash');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe.runIf(canRunGit())('getBranches', () => {
|
||||||
|
it('returns a remote default branch whose name is not a conventional fallback', async () => {
|
||||||
|
const remote = createTempDir();
|
||||||
|
const repository = createTempDir();
|
||||||
|
runGit(remote, ['init', '--bare', '--initial-branch=react']);
|
||||||
|
runGit(repository, ['init', '-b', 'next']);
|
||||||
|
runGit(repository, ['config', 'user.email', 'test@example.com']);
|
||||||
|
runGit(repository, ['config', 'user.name', 'Test']);
|
||||||
|
fs.writeFileSync(path.join(repository, 'README.md'), '# Test\n');
|
||||||
|
runGit(repository, ['add', 'README.md']);
|
||||||
|
runGit(repository, ['commit', '-m', 'init']);
|
||||||
|
runGit(repository, ['remote', 'add', 'origin', remote]);
|
||||||
|
runGit(repository, ['push', 'origin', 'HEAD:react']);
|
||||||
|
runGit(repository, ['fetch', 'origin']);
|
||||||
|
runGit(repository, ['remote', 'set-head', 'origin', '--auto']);
|
||||||
|
|
||||||
|
await expect(getBranches(repository)).resolves.toMatchObject({
|
||||||
|
defaultBranches: { origin: 'react' },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -55,6 +55,11 @@ written against staged code never silently re-anchors onto an unstaged edit.
|
|||||||
| `branch` | `branch` | `getRangeDiff` uses three-dot `base...head`, so work merged in from the base branch is excluded |
|
| `branch` | `branch` | `getRangeDiff` uses three-dot `base...head`, so work merged in from the base branch is excluded |
|
||||||
| `pr` | `pr:<number>` | GitHub returns the merge-base diff, matching the branch semantics |
|
| `pr` | `pr:<number>` | GitHub returns the merge-base diff, matching the branch semantics |
|
||||||
|
|
||||||
|
For the current-branch source, the UI prefers the default branch of the current
|
||||||
|
branch's tracking remote (from its local `remote/HEAD` symbolic ref), then uses
|
||||||
|
the existing conventional-branch fallback. It does not offer the source when the
|
||||||
|
chosen base cannot be resolved locally or through a remote-tracking ref.
|
||||||
|
|
||||||
The panel offers the current branch's pull request on its own: it registers with
|
The panel offers the current branch's pull request on its own: it registers with
|
||||||
the shared GitHub PR status store (`useGitHubPrStatusStore`) rather than waiting
|
the shared GitHub PR status store (`useGitHubPrStatusStore`) rather than waiting
|
||||||
for the pull request panel to have been visited. That store already dedupes
|
for the pull request panel to have been visited. That store already dedupes
|
||||||
|
|||||||
Reference in New Issue
Block a user