Link worktrees to PRs from differently named tracked branches (#1718)
* fix(server): link PRs for differently named tracked branches PR lookup now uses the configured tracked head ref when it differs from the local branch, while keeping fork owner scoping only for cross-repo remotes. * fix(server): avoid extra PR lookup git reads Use the shared lookup-target builder for an early local-branch result before reading origin and base metadata on on-demand PR status refreshes.
This commit is contained in:
Mohamed Boudra
@@ -149,6 +149,44 @@ function createPullRequestStatus(overrides?: Partial<GitHubCurrentPullRequestSta
|
||||
};
|
||||
}
|
||||
|
||||
interface RequestedPullRequestTarget {
|
||||
headRef: string;
|
||||
headRepositoryOwner?: string;
|
||||
}
|
||||
|
||||
interface RecordingPullRequestTargetsOptions {
|
||||
requestedTargets: RequestedPullRequestTarget[];
|
||||
statusOverrides?: Partial<GitHubCurrentPullRequestStatus>;
|
||||
}
|
||||
|
||||
function createGitHubServiceRecordingPullRequestTargets(
|
||||
options: RecordingPullRequestTargetsOptions,
|
||||
): GitHubService {
|
||||
const github = createGitHubServiceForStatus(null);
|
||||
github.getCurrentPullRequestStatus = async (request) => {
|
||||
options.requestedTargets.push({
|
||||
headRef: request.headRef,
|
||||
...(request.headRepositoryOwner ? { headRepositoryOwner: request.headRepositoryOwner } : {}),
|
||||
});
|
||||
return createPullRequestStatus({
|
||||
...options.statusOverrides,
|
||||
headRefName: request.headRef,
|
||||
});
|
||||
};
|
||||
return github;
|
||||
}
|
||||
|
||||
async function readPullRequestLookupTargetFromFacts(
|
||||
repoDir: string,
|
||||
paseoHome: string,
|
||||
): Promise<RequestedPullRequestTarget | null> {
|
||||
const facts = await getCheckoutSnapshotFacts(repoDir, { paseoHome });
|
||||
if (!facts.isGit) {
|
||||
throw new Error("Expected git checkout facts");
|
||||
}
|
||||
return facts.pullRequestLookupTarget;
|
||||
}
|
||||
|
||||
function setupRemoteTrackingMain(
|
||||
repoDir: string,
|
||||
tempDir: string,
|
||||
@@ -2000,6 +2038,118 @@ const x = 1;
|
||||
});
|
||||
});
|
||||
|
||||
it("uses an origin tracked head when the local branch name differs", async () => {
|
||||
execFileSync("git", ["checkout", "-b", "tender-parrot"], { cwd: repoDir });
|
||||
execFileSync("git", ["remote", "add", "origin", "https://github.com/getpaseo/paseo.git"], {
|
||||
cwd: repoDir,
|
||||
});
|
||||
execFileSync("git", ["config", "branch.tender-parrot.remote", "origin"], { cwd: repoDir });
|
||||
execFileSync(
|
||||
"git",
|
||||
["config", "branch.tender-parrot.merge", "refs/heads/refactor/workspace-scripts"],
|
||||
{ cwd: repoDir },
|
||||
);
|
||||
|
||||
const lookupTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome);
|
||||
|
||||
expect(lookupTarget).toEqual({ headRef: "refactor/workspace-scripts" });
|
||||
});
|
||||
|
||||
it("keeps the local branch lookup when origin tracking uses the same head name", async () => {
|
||||
execFileSync("git", ["checkout", "-b", "feature"], { cwd: repoDir });
|
||||
execFileSync("git", ["remote", "add", "origin", "https://github.com/getpaseo/paseo.git"], {
|
||||
cwd: repoDir,
|
||||
});
|
||||
execFileSync("git", ["config", "branch.feature.remote", "origin"], { cwd: repoDir });
|
||||
execFileSync("git", ["config", "branch.feature.merge", "refs/heads/feature"], {
|
||||
cwd: repoDir,
|
||||
});
|
||||
|
||||
const lookupTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome);
|
||||
|
||||
expect(lookupTarget).toEqual({ headRef: "feature" });
|
||||
});
|
||||
|
||||
it("does not attach an owner when the tracked remote is the same GitHub repository", async () => {
|
||||
execFileSync("git", ["checkout", "-b", "local-feature"], { cwd: repoDir });
|
||||
execFileSync("git", ["remote", "add", "origin", "git@github.com:getpaseo/paseo.git"], {
|
||||
cwd: repoDir,
|
||||
});
|
||||
execFileSync("git", ["remote", "add", "upstream", "https://github.com/getpaseo/paseo.git"], {
|
||||
cwd: repoDir,
|
||||
});
|
||||
execFileSync("git", ["config", "branch.local-feature.remote", "upstream"], {
|
||||
cwd: repoDir,
|
||||
});
|
||||
execFileSync(
|
||||
"git",
|
||||
["config", "branch.local-feature.merge", "refs/heads/refactor/workspace-scripts"],
|
||||
{ cwd: repoDir },
|
||||
);
|
||||
|
||||
const lookupTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome);
|
||||
|
||||
expect(lookupTarget).toEqual({ headRef: "refactor/workspace-scripts" });
|
||||
});
|
||||
|
||||
it("keeps the fork owner when same-repo comparison is indeterminate", async () => {
|
||||
execFileSync("git", ["checkout", "-b", "chethanuk/main"], { cwd: repoDir });
|
||||
execFileSync("git", ["remote", "add", "origin", "not-a-github-remote"], { cwd: repoDir });
|
||||
execFileSync("git", ["remote", "add", "paseo-pr-345", "git@github.com:chethanuk/paseo.git"], {
|
||||
cwd: repoDir,
|
||||
});
|
||||
execFileSync("git", ["config", "branch.chethanuk/main.remote", "paseo-pr-345"], {
|
||||
cwd: repoDir,
|
||||
});
|
||||
execFileSync("git", ["config", "branch.chethanuk/main.merge", "refs/heads/main"], {
|
||||
cwd: repoDir,
|
||||
});
|
||||
|
||||
const lookupTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome);
|
||||
|
||||
expect(lookupTarget).toEqual({ headRef: "main", headRepositoryOwner: "chethanuk" });
|
||||
});
|
||||
|
||||
it("keeps the local branch lookup when same-repo tracking points at the base branch", async () => {
|
||||
execFileSync("git", ["checkout", "-b", "tender-parrot"], { cwd: repoDir });
|
||||
execFileSync("git", ["remote", "add", "origin", "https://github.com/getpaseo/paseo.git"], {
|
||||
cwd: repoDir,
|
||||
});
|
||||
execFileSync("git", ["config", "branch.tender-parrot.remote", "origin"], { cwd: repoDir });
|
||||
execFileSync("git", ["config", "branch.tender-parrot.merge", "refs/heads/main"], {
|
||||
cwd: repoDir,
|
||||
});
|
||||
|
||||
const lookupTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome);
|
||||
|
||||
expect(lookupTarget).toEqual({ headRef: "tender-parrot" });
|
||||
});
|
||||
|
||||
it("derives the same origin tracked head for on-demand PR status reads", async () => {
|
||||
execFileSync("git", ["checkout", "-b", "tender-parrot"], { cwd: repoDir });
|
||||
execFileSync("git", ["remote", "add", "origin", "https://github.com/getpaseo/paseo.git"], {
|
||||
cwd: repoDir,
|
||||
});
|
||||
execFileSync("git", ["config", "branch.tender-parrot.remote", "origin"], { cwd: repoDir });
|
||||
execFileSync(
|
||||
"git",
|
||||
["config", "branch.tender-parrot.merge", "refs/heads/refactor/workspace-scripts"],
|
||||
{ cwd: repoDir },
|
||||
);
|
||||
const factsTarget = await readPullRequestLookupTargetFromFacts(repoDir, paseoHome);
|
||||
const requestedTargets: RequestedPullRequestTarget[] = [];
|
||||
const github = createGitHubServiceRecordingPullRequestTargets({ requestedTargets });
|
||||
|
||||
await getPullRequestStatus(
|
||||
repoDir,
|
||||
github,
|
||||
{ force: true, reason: "tracked-head-parity" },
|
||||
{ paseoHome },
|
||||
);
|
||||
|
||||
expect(requestedTargets).toEqual([factsTarget]);
|
||||
});
|
||||
|
||||
it("uses the tracked fork branch for PR worktree status lookup", async () => {
|
||||
execFileSync("git", ["checkout", "-b", "chethanuk/main"], { cwd: repoDir });
|
||||
execFileSync("git", ["remote", "add", "origin", "https://github.com/getpaseo/paseo.git"], {
|
||||
@@ -2015,30 +2165,14 @@ const x = 1;
|
||||
cwd: repoDir,
|
||||
});
|
||||
|
||||
const requestedTargets: Array<{ headRef: string; headRepositoryOwner?: string }> = [];
|
||||
const github = createGitHubServiceForStatus(
|
||||
createPullRequestStatus({
|
||||
const requestedTargets: RequestedPullRequestTarget[] = [];
|
||||
const github = createGitHubServiceRecordingPullRequestTargets({
|
||||
requestedTargets,
|
||||
statusOverrides: {
|
||||
number: 345,
|
||||
url: "https://github.com/getpaseo/paseo/pull/345",
|
||||
headRefName: "main",
|
||||
}),
|
||||
{
|
||||
onStatus: () => {},
|
||||
},
|
||||
);
|
||||
github.getCurrentPullRequestStatus = async (options) => {
|
||||
requestedTargets.push({
|
||||
headRef: options.headRef,
|
||||
...(options.headRepositoryOwner
|
||||
? { headRepositoryOwner: options.headRepositoryOwner }
|
||||
: {}),
|
||||
});
|
||||
return createPullRequestStatus({
|
||||
number: 345,
|
||||
url: "https://github.com/getpaseo/paseo/pull/345",
|
||||
headRefName: options.headRef,
|
||||
});
|
||||
};
|
||||
});
|
||||
|
||||
const status = await getPullRequestStatus(repoDir, github);
|
||||
|
||||
|
||||
@@ -70,6 +70,15 @@ interface PullRequestStatusLookupTarget {
|
||||
headRepositoryOwner?: string;
|
||||
}
|
||||
|
||||
interface PullRequestLookupTargetBranchConfig {
|
||||
currentBranch: string;
|
||||
branchRemoteName: string | null;
|
||||
branchMergeRef: string | null;
|
||||
branchRemoteUrl: string | null;
|
||||
originRemoteUrl: string | null;
|
||||
resolvedBaseRef: string | null;
|
||||
}
|
||||
|
||||
function getErrorStderr(error: Error): string {
|
||||
return "stderr" in error && typeof error.stderr === "string" ? error.stderr : "";
|
||||
}
|
||||
@@ -1132,24 +1141,37 @@ async function resolvePullRequestStatusLookupTarget(
|
||||
if (context?.facts?.isGit && context.facts.pullRequestLookupTarget) {
|
||||
return context.facts.pullRequestLookupTarget;
|
||||
}
|
||||
const remoteName = await getGitConfigValue(cwd, `branch.${currentBranch}.remote`);
|
||||
if (!remoteName?.startsWith("paseo-pr-")) {
|
||||
return { headRef: currentBranch };
|
||||
const branchRemoteName = await getGitConfigValue(cwd, `branch.${currentBranch}.remote`, context);
|
||||
let branchMergeRef: string | null = null;
|
||||
if (branchRemoteName) {
|
||||
branchMergeRef = await getGitConfigValue(cwd, `branch.${currentBranch}.merge`, context);
|
||||
}
|
||||
|
||||
const mergeRef = await getGitConfigValue(cwd, `branch.${currentBranch}.merge`);
|
||||
const trackedHeadRef = parseBranchMergeHeadRef(mergeRef);
|
||||
if (!trackedHeadRef) {
|
||||
return { headRef: currentBranch };
|
||||
const localBranchTarget = buildPullRequestLookupTargetFromBranchConfig({
|
||||
currentBranch,
|
||||
branchRemoteName,
|
||||
branchMergeRef,
|
||||
branchRemoteUrl: null,
|
||||
originRemoteUrl: null,
|
||||
resolvedBaseRef: null,
|
||||
});
|
||||
if (localBranchTarget.headRef === currentBranch) {
|
||||
return localBranchTarget;
|
||||
}
|
||||
|
||||
const remoteUrl = await getGitConfigValue(cwd, `remote.${remoteName}.url`);
|
||||
const remoteRepo = remoteUrl ? parseGitHubRepoFromRemote(remoteUrl) : null;
|
||||
const headRepositoryOwner = remoteRepo?.split("/")[0];
|
||||
return {
|
||||
headRef: trackedHeadRef,
|
||||
...(headRepositoryOwner ? { headRepositoryOwner } : {}),
|
||||
};
|
||||
const [branchRemoteUrl, originRemoteUrl, resolvedBaseRef] = await Promise.all([
|
||||
branchRemoteName ? getGitConfigValue(cwd, `remote.${branchRemoteName}.url`, context) : null,
|
||||
getGitConfigValue(cwd, "remote.origin.url", context),
|
||||
getResolvedBaseRefForCwd(cwd, context),
|
||||
]);
|
||||
return buildPullRequestLookupTargetFromBranchConfig({
|
||||
currentBranch,
|
||||
branchRemoteName,
|
||||
branchMergeRef,
|
||||
branchRemoteUrl,
|
||||
originRemoteUrl,
|
||||
resolvedBaseRef,
|
||||
});
|
||||
}
|
||||
|
||||
export async function resolveAbsoluteGitDir(cwd: string): Promise<string | null> {
|
||||
@@ -1498,25 +1520,33 @@ async function inspectCheckoutContext(
|
||||
}
|
||||
}
|
||||
|
||||
function buildPullRequestLookupTargetFromBranchConfig(input: {
|
||||
currentBranch: string;
|
||||
branchRemoteName: string | null;
|
||||
branchMergeRef: string | null;
|
||||
branchRemoteUrl: string | null;
|
||||
}): PullRequestStatusLookupTarget {
|
||||
if (!input.branchRemoteName?.startsWith("paseo-pr-")) {
|
||||
return { headRef: input.currentBranch };
|
||||
}
|
||||
|
||||
function buildPullRequestLookupTargetFromBranchConfig(
|
||||
input: PullRequestLookupTargetBranchConfig,
|
||||
): PullRequestStatusLookupTarget {
|
||||
const trackedHeadRef = parseBranchMergeHeadRef(input.branchMergeRef);
|
||||
if (!trackedHeadRef) {
|
||||
if (!input.branchRemoteName || !trackedHeadRef || trackedHeadRef === input.currentBranch) {
|
||||
return { headRef: input.currentBranch };
|
||||
}
|
||||
|
||||
const remoteRepo = input.branchRemoteUrl
|
||||
? parseGitHubRepoFromRemote(input.branchRemoteUrl)
|
||||
: null;
|
||||
const headRepositoryOwner = remoteRepo?.split("/")[0];
|
||||
const originRepo = input.originRemoteUrl
|
||||
? parseGitHubRepoFromRemote(input.originRemoteUrl)
|
||||
: null;
|
||||
const isSameRepo = Boolean(remoteRepo && originRepo && remoteRepo === originRepo);
|
||||
const headRepositoryOwner = remoteRepo && !isSameRepo ? remoteRepo.split("/")[0] : null;
|
||||
const normalizedBaseRef = input.resolvedBaseRef
|
||||
? normalizeLocalBranchRefName(input.resolvedBaseRef)
|
||||
: null;
|
||||
if (trackedHeadRef === normalizedBaseRef && !headRepositoryOwner) {
|
||||
return { headRef: input.currentBranch };
|
||||
}
|
||||
|
||||
if (isSameRepo) {
|
||||
return { headRef: trackedHeadRef };
|
||||
}
|
||||
|
||||
return {
|
||||
headRef: trackedHeadRef,
|
||||
...(headRepositoryOwner ? { headRepositoryOwner } : {}),
|
||||
@@ -1559,21 +1589,17 @@ export async function getCheckoutSnapshotFacts(
|
||||
let branchRemoteName: string | null = null;
|
||||
let branchMergeRef: string | null = null;
|
||||
let branchRemoteUrl: string | null = null;
|
||||
if (inspected.remoteUrl && inspected.currentBranch) {
|
||||
if (inspected.currentBranch) {
|
||||
branchRemoteName = await getGitConfigValue(
|
||||
cwd,
|
||||
`branch.${inspected.currentBranch}.remote`,
|
||||
context,
|
||||
);
|
||||
if (branchRemoteName) {
|
||||
branchMergeRef = await getGitConfigValue(
|
||||
cwd,
|
||||
`branch.${inspected.currentBranch}.merge`,
|
||||
context,
|
||||
);
|
||||
if (branchRemoteName.startsWith("paseo-pr-")) {
|
||||
branchRemoteUrl = await getGitConfigValue(cwd, `remote.${branchRemoteName}.url`, context);
|
||||
}
|
||||
[branchMergeRef, branchRemoteUrl] = await Promise.all([
|
||||
getGitConfigValue(cwd, `branch.${inspected.currentBranch}.merge`, context),
|
||||
getGitConfigValue(cwd, `remote.${branchRemoteName}.url`, context),
|
||||
]);
|
||||
}
|
||||
}
|
||||
const pullRequestLookupTarget = inspected.currentBranch
|
||||
@@ -1582,6 +1608,8 @@ export async function getCheckoutSnapshotFacts(
|
||||
branchRemoteName,
|
||||
branchMergeRef,
|
||||
branchRemoteUrl,
|
||||
originRemoteUrl: inspected.remoteUrl,
|
||||
resolvedBaseRef,
|
||||
})
|
||||
: null;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user