mirror of
https://github.com/getpaseo/paseo.git
synced 2026-10-14 13:22:37 +00:00
fix(gitea): prefix fork PR branches with the owner
Two forks with the same branch name (say patch-1) ended up on the same local branch. The gitea service had no buildPrLocalBranchName or defaultCheckoutRefs, github has both. getPullRequestCheckoutTarget also fell back to refs/heads/<name> on origin for fork PRs. Thats the base repo's branch, not the fork's, so a failed refs/pull fetch checked out the wrong branch without an error. fixes #3515
This commit is contained in:
committed by
paseo-bot[bot]
parent
e135e9c101
commit
3534c50da4
@@ -2330,10 +2330,7 @@ describe("createGiteaService", () => {
|
||||
number: 5,
|
||||
baseRefName: "main",
|
||||
headRefName: "feat/sample-change",
|
||||
checkoutRefs: [
|
||||
{ remoteName: "origin", remoteRef: "refs/pull/5/head" },
|
||||
{ remoteName: "origin", remoteRef: "refs/heads/feat/sample-change" },
|
||||
],
|
||||
checkoutRefs: [{ remoteName: "origin", remoteRef: "refs/pull/5/head" }],
|
||||
headOwnerLogin: "contributor",
|
||||
headRepositorySshUrl: "git@gitea.com:contributor/sample-repo.git",
|
||||
headRepositoryUrl: "https://gitea.com/contributor/sample-repo",
|
||||
@@ -2343,6 +2340,128 @@ describe("createGiteaService", () => {
|
||||
expect(calls).toContainEqual(["api", "repos/example-user/sample-repo/pulls/5"]);
|
||||
});
|
||||
|
||||
it("prefixes the local branch name with the fork owner for a cross-repository checkout", () => {
|
||||
const { service } = makeService(() => ok(""));
|
||||
|
||||
const localBranchName = service.buildPrLocalBranchName?.({
|
||||
headRef: "patch-1",
|
||||
checkoutTarget: {
|
||||
number: 5,
|
||||
baseRefName: "main",
|
||||
headRefName: "patch-1",
|
||||
headOwnerLogin: "contributor",
|
||||
headRepositorySshUrl: null,
|
||||
headRepositoryUrl: null,
|
||||
isCrossRepository: true,
|
||||
},
|
||||
});
|
||||
|
||||
expect(localBranchName).toBe("contributor/patch-1");
|
||||
});
|
||||
|
||||
it("keeps the head ref name as-is for a same-repository checkout", () => {
|
||||
const { service } = makeService(() => ok(""));
|
||||
|
||||
const localBranchName = service.buildPrLocalBranchName?.({
|
||||
headRef: "patch-1",
|
||||
checkoutTarget: {
|
||||
number: 5,
|
||||
baseRefName: "main",
|
||||
headRefName: "patch-1",
|
||||
headOwnerLogin: null,
|
||||
headRepositorySshUrl: null,
|
||||
headRepositoryUrl: null,
|
||||
isCrossRepository: false,
|
||||
},
|
||||
});
|
||||
|
||||
expect(localBranchName).toBe("patch-1");
|
||||
});
|
||||
|
||||
it("falls back to a PR-number prefix when the fork owner login is not a safe branch path segment", () => {
|
||||
const { service } = makeService(() => ok(""));
|
||||
|
||||
const localBranchName = service.buildPrLocalBranchName?.({
|
||||
headRef: "patch-1",
|
||||
checkoutTarget: {
|
||||
number: 5,
|
||||
baseRefName: "main",
|
||||
headRefName: "patch-1",
|
||||
headOwnerLogin: "../evil",
|
||||
headRepositorySshUrl: null,
|
||||
headRepositoryUrl: null,
|
||||
isCrossRepository: true,
|
||||
},
|
||||
});
|
||||
|
||||
expect(localBranchName).toBe("pr-5/patch-1");
|
||||
});
|
||||
|
||||
it("falls back to a PR-number prefix when the fork owner login would produce an invalid ref", () => {
|
||||
const { service } = makeService(() => ok(""));
|
||||
|
||||
const localBranchName = service.buildPrLocalBranchName?.({
|
||||
headRef: "patch-1",
|
||||
checkoutTarget: {
|
||||
number: 5,
|
||||
baseRefName: "main",
|
||||
headRefName: "patch-1",
|
||||
headOwnerLogin: "contributor.lock",
|
||||
headRepositorySshUrl: null,
|
||||
headRepositoryUrl: null,
|
||||
isCrossRepository: true,
|
||||
},
|
||||
});
|
||||
|
||||
expect(localBranchName).toBe("pr-5/patch-1");
|
||||
});
|
||||
|
||||
it("falls back to a PR-number prefix when the fork owner login is unknown", () => {
|
||||
const { service } = makeService(() => ok(""));
|
||||
|
||||
const localBranchName = service.buildPrLocalBranchName?.({
|
||||
headRef: "patch-1",
|
||||
checkoutTarget: {
|
||||
number: 5,
|
||||
baseRefName: "main",
|
||||
headRefName: "patch-1",
|
||||
headOwnerLogin: null,
|
||||
headRepositorySshUrl: null,
|
||||
headRepositoryUrl: null,
|
||||
isCrossRepository: true,
|
||||
},
|
||||
});
|
||||
|
||||
expect(localBranchName).toBe("pr-5/patch-1");
|
||||
});
|
||||
|
||||
it("lowercases the fork owner login for the branch prefix", () => {
|
||||
const { service } = makeService(() => ok(""));
|
||||
|
||||
const localBranchName = service.buildPrLocalBranchName?.({
|
||||
headRef: "patch-1",
|
||||
checkoutTarget: {
|
||||
number: 5,
|
||||
baseRefName: "main",
|
||||
headRefName: "patch-1",
|
||||
headOwnerLogin: "Contributor",
|
||||
headRepositorySshUrl: null,
|
||||
headRepositoryUrl: null,
|
||||
isCrossRepository: true,
|
||||
},
|
||||
});
|
||||
|
||||
expect(localBranchName).toBe("contributor/patch-1");
|
||||
});
|
||||
|
||||
it("returns the pull head ref for default checkout refs", () => {
|
||||
const { service } = makeService(() => ok(""));
|
||||
|
||||
expect(service.defaultCheckoutRefs?.({ changeRequestNumber: 5, headRef: "patch-1" })).toEqual([
|
||||
{ remoteName: "origin", remoteRef: "refs/pull/5/head" },
|
||||
]);
|
||||
});
|
||||
|
||||
it("creates a pull request and parses the resulting URL and index", async () => {
|
||||
const { service, calls } = makeService((args) => {
|
||||
if (args[0] === "pr" && args[1] === "create") {
|
||||
|
||||
@@ -1377,6 +1377,19 @@ function throwFirstNonTeaAuthSearchRejection(results: PromiseSettledResult<unkno
|
||||
}
|
||||
}
|
||||
|
||||
// forgejo username pattern
|
||||
function normalizeGiteaOwnerForBranch(owner: string | null): string | null {
|
||||
const normalized = owner?.trim().toLowerCase() ?? "";
|
||||
if (!/^[a-z0-9][a-z0-9_.-]*$/.test(normalized)) {
|
||||
return null;
|
||||
}
|
||||
// git refuses .. and .lock, trailing dot too
|
||||
if (normalized.includes("..") || normalized.endsWith(".") || normalized.endsWith(".lock")) {
|
||||
return null;
|
||||
}
|
||||
return normalized;
|
||||
}
|
||||
|
||||
export function createGiteaService(options: CreateGiteaServiceOptions = {}): ForgeService {
|
||||
const runner = options.runner ?? runTeaCommand;
|
||||
const resolveTea = createCachedCliPathResolver(options.resolveTeaPath ?? resolveTeaPath);
|
||||
@@ -1997,17 +2010,31 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For
|
||||
return summary.headRefName;
|
||||
},
|
||||
|
||||
defaultCheckoutRefs({ changeRequestNumber }) {
|
||||
return [{ remoteName: "origin", remoteRef: `refs/pull/${changeRequestNumber}/head` }];
|
||||
},
|
||||
|
||||
buildPrLocalBranchName({ headRef, checkoutTarget }) {
|
||||
if (!checkoutTarget.isCrossRepository) {
|
||||
return headRef;
|
||||
}
|
||||
const owner = normalizeGiteaOwnerForBranch(checkoutTarget.headOwnerLogin);
|
||||
// odd login, use pr-N instead
|
||||
return `${owner ?? `pr-${checkoutTarget.number}`}/${headRef}`;
|
||||
},
|
||||
|
||||
async getPullRequestCheckoutTarget(
|
||||
input: GetPullRequestOptions,
|
||||
): Promise<PullRequestCheckoutTarget> {
|
||||
const summary = await this.getPullRequest(input);
|
||||
const fork = await resolveForkCheckoutFacts(input.cwd, summary);
|
||||
const checkoutRefs = [
|
||||
{ remoteName: "origin", remoteRef: `refs/pull/${summary.number}/head` },
|
||||
];
|
||||
if (summary.headRefName) {
|
||||
// on a fork PR thats the base repos branch
|
||||
if (summary.headRefName && !fork.isCrossRepository) {
|
||||
checkoutRefs.push({ remoteName: "origin", remoteRef: `refs/heads/${summary.headRefName}` });
|
||||
}
|
||||
const fork = await resolveForkCheckoutFacts(input.cwd, summary);
|
||||
return {
|
||||
number: summary.number,
|
||||
baseRefName: summary.baseRefName,
|
||||
|
||||
Reference in New Issue
Block a user