From eecac800b6135b7ddcadca7f95fb8009bc401633 Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Mon, 2 Nov 2026 09:54:18 +0000 Subject: [PATCH] fix(gitlab): identify fork checkouts by their source project Merge request authors do not necessarily own the source fork. Resolve its namespace and repository URLs before naming the checkout and configuring its push remote, retaining the target MR ref when the source returns 404. Co-authored-by: quickwind --- .../src/server/worktree-core.posix.test.ts | 36 +++++++++ .../src/services/gitlab-service.test.ts | 78 ++++++++++++++----- .../server/src/services/gitlab-service.ts | 44 +++++++---- 3 files changed, 122 insertions(+), 36 deletions(-) diff --git a/packages/server/src/server/worktree-core.posix.test.ts b/packages/server/src/server/worktree-core.posix.test.ts index 7a7a342732..4e91932815 100644 --- a/packages/server/src/server/worktree-core.posix.test.ts +++ b/packages/server/src/server/worktree-core.posix.test.ts @@ -806,6 +806,25 @@ describe.skipIf(isPlatform("win32"))("worktree-core POSIX-only", () => { test("checks out the authoritative GitLab MR ref before a same-named origin branch", async () => { const { tempDir, repoDir, paseoHome } = createGitLabMrWithConflictingOriginBranchRepo(); cleanupPaths.push(tempDir); + const sourceRemote = path.join(tempDir, "fork.git"); + execFileSync("git", ["clone", "--bare", path.join(tempDir, "origin.git"), sourceRemote], { + stdio: "pipe", + }); + const mrHead = execFileSync( + "git", + [ + `--git-dir=${path.join(tempDir, "origin.git")}`, + "rev-parse", + "refs/merge-requests/14/head", + ], + { encoding: "utf8" }, + ).trim(); + execFileSync("git", [ + `--git-dir=${sourceRemote}`, + "update-ref", + "refs/heads/feature/gitlab-mr", + mrHead, + ]); const gitlab = createGitLabService({ resolveGlabPath: async () => "/usr/bin/glab", resolveRemoteUrl: async () => "git@gitlab.example.com:example-group/example-project.git", @@ -820,12 +839,23 @@ describe.skipIf(isPlatform("win32"))("worktree-core POSIX-only", () => { state: "opened", source_branch: "feature/gitlab-mr", target_branch: "main", + author: { username: "mr-author" }, source_project_id: 202, target_project_id: 101, }), stderr: "", }; } + if (args[0] === "api" && args[1] === "projects/202") { + return { + stdout: JSON.stringify({ + namespace: { full_path: "team" }, + ssh_url_to_repo: sourceRemote, + http_url_to_repo: "https://gitlab.example.com/team/fork.git", + }), + stderr: "", + }; + } throw new Error(`unexpected glab args: ${args.join(" ")}`); }, }); @@ -843,11 +873,17 @@ describe.skipIf(isPlatform("win32"))("worktree-core POSIX-only", () => { expect(result.intent).toMatchObject({ kind: "checkout-change-request", forge: "gitlab", + headRepository: "team/fork", + localBranchName: "team/feature/gitlab-mr", checkoutRefs: [ { remoteName: "origin", remoteRef: "refs/merge-requests/14/head" }, { remoteName: "origin", remoteRef: "refs/heads/feature/gitlab-mr" }, ], }); + expect(result.worktree.branchName).toBe("team/feature/gitlab-mr"); + expect(getGitConfigValue(result.worktree.worktreePath, "remote.paseo-pr-14.url")).toBe( + sourceRemote, + ); expect(readFileSync(path.join(result.worktree.worktreePath, "README.md"), "utf8")).toBe( "authoritative merge request head\n", ); diff --git a/packages/server/src/services/gitlab-service.test.ts b/packages/server/src/services/gitlab-service.test.ts index af1c9b9db7..14877d9708 100644 --- a/packages/server/src/services/gitlab-service.test.ts +++ b/packages/server/src/services/gitlab-service.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it } from "vitest"; +import { resolveWorktreeCreationIntent } from "../server/resolve-worktree-creation-intent.js"; + import type { PullRequestCommandStatus } from "./forge-service.js"; import { GITLAB_ACTIVE_PIPELINE_STATUS_SET } from "@getpaseo/protocol/gitlab-pipeline"; import type { GitLabStatusFacts } from "./gitlab-facts.js"; @@ -484,36 +486,70 @@ describe("createGitLabService", () => { checkoutTarget: target!, }), ).toBe("release/v0.4.0"); - expect(calls[0]).toEqual(["mr", "view", "14", "-F", "json"]); + expect(calls).toEqual([["mr", "view", "14", "-F", "json"]]); }); - it("marks fork merge request checkout targets as cross-repository", async () => { - const { service } = makeService(() => - ok( - JSON.stringify({ - ...OPEN_MR, - source_project_id: 202, - target_project_id: 101, - author: { username: "contributor" }, - }), - ), - ); + it.each(["contributor", "team"])( + "resolves the source project rather than the MR author for %s forks", + async (owner) => { + const { service, calls } = makeService((args) => { + if (args[0] === "api") + return ok( + JSON.stringify({ + namespace: { full_path: owner }, + ssh_url_to_repo: `git@gitlab.example.com:${owner}/fork.git`, + http_url_to_repo: `https://gitlab.example.com/${owner}/fork.git`, + }), + ); + return ok( + JSON.stringify({ ...OPEN_MR, source_project_id: 202, author: { username: "mr-author" } }), + ); + }); + const intent = await resolveWorktreeCreationIntent( + { + action: "checkout", + checkoutSource: { kind: "change_request", forge: "gitlab", number: 14 }, + }, + "/repo", + { forge: "gitlab", forgeService: service, resolveDefaultBranch: async () => "main" }, + ); + expect(intent).toMatchObject({ + kind: "checkout-change-request", + headRepositoryOwner: owner, + headRepository: `${owner}/fork`, + pushRemoteUrl: `git@gitlab.example.com:${owner}/fork.git`, + localBranchName: `${owner}/release/v0.4.0`, + checkoutRefs: [ + { remoteName: "origin", remoteRef: "refs/merge-requests/14/head" }, + { remoteName: "origin", remoteRef: "refs/heads/release/v0.4.0" }, + ], + }); + expect(calls).toEqual([ + ["mr", "view", "14", "-F", "json"], + ["api", "projects/202"], + ]); + }, + ); - const target = await service.getPullRequestCheckoutTarget?.({ cwd: "/repo", number: 14 }); + it("keeps the target MR ref usable when its source project is inaccessible", async () => { + const { service } = makeService((args) => { + if (args[0] === "api") throw { code: 1, stderr: "404 Project Not Found" }; + return ok(JSON.stringify({ ...OPEN_MR, source_project_id: 202 })); + }); + const target = await service.getPullRequestCheckoutTarget!({ cwd: "/repo", number: 14 }); expect(target).toMatchObject({ - number: 14, - headRefName: "release/v0.4.0", isCrossRepository: true, - headOwnerLogin: "contributor", + headOwnerLogin: null, headRepositorySshUrl: null, headRepositoryUrl: null, + checkoutRefs: [ + { remoteName: "origin", remoteRef: "refs/merge-requests/14/head" }, + { remoteName: "origin", remoteRef: "refs/heads/release/v0.4.0" }, + ], }); expect( - service.buildPrLocalBranchName?.({ - headRef: target!.headRefName, - checkoutTarget: target!, - }), - ).toBe("contributor/release/v0.4.0"); + service.buildPrLocalBranchName!({ headRef: target.headRefName, checkoutTarget: target }), + ).toBe("pr-14/release/v0.4.0"); }); it("creates a merge request and parses the URL and iid from glab output", async () => { diff --git a/packages/server/src/services/gitlab-service.ts b/packages/server/src/services/gitlab-service.ts index da55e60b48..c7df5e239e 100644 --- a/packages/server/src/services/gitlab-service.ts +++ b/packages/server/src/services/gitlab-service.ts @@ -145,14 +145,11 @@ const GitLabPipelineDetailsSchema = z }) .passthrough(); -const GitLabAuthorSchema = z - .object({ - username: z.string().optional(), - name: z.string().optional(), - web_url: z.string().nullable().optional(), - avatar_url: z.string().nullable().optional(), - }) - .passthrough(); +const GitLabSourceProjectSchema = z.object({ + namespace: z.object({ full_path: z.string() }), + ssh_url_to_repo: z.string(), + http_url_to_repo: z.string(), +}); const GitLabMergeRequestSchema = z .object({ @@ -165,7 +162,6 @@ const GitLabMergeRequestSchema = z sha: z.string().optional(), source_project_id: z.number().nullable().optional(), target_project_id: z.number().nullable().optional(), - author: GitLabAuthorSchema.nullable().optional(), draft: z.boolean().optional(), work_in_progress: z.boolean().optional(), has_conflicts: z.boolean().optional(), @@ -197,7 +193,14 @@ const GitLabIssueSchema = z }) .passthrough(); -const GitLabNoteAuthorSchema = GitLabAuthorSchema; +const GitLabNoteAuthorSchema = z + .object({ + username: z.string().optional(), + name: z.string().optional(), + web_url: z.string().nullable().optional(), + avatar_url: z.string().nullable().optional(), + }) + .passthrough(); const GitLabNoteLineRangeEndpointSchema = z .object({ @@ -953,6 +956,16 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F return runJson(["mr", "view", ref, "-F", "json"], { cwd }, GitLabMergeRequestSchema); } + async function loadSourceProject(cwd: string, projectId: number) { + try { + return await runJson(["api", `projects/${projectId}`], { cwd }, GitLabSourceProjectSchema); + } catch (error) { + // The target's MR ref remains fetchable even if the source project is private. + if (error instanceof GlabCommandError && /\b404\b/.test(error.stderr)) return null; + throw error; + } + } + async function listMergeRequestsBySourceBranch( cwd: string, sourceBranch: string, @@ -1201,8 +1214,9 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F mr.target_project_id !== undefined && mr.target_project_id !== null && mr.source_project_id !== mr.target_project_id; - const headOwnerLogin = - isCrossRepository && mr.author?.username?.trim() ? mr.author.username.trim() : null; + const sourceProject = isCrossRepository + ? await loadSourceProject(input.cwd, mr.source_project_id!) + : null; return { number: mr.iid, baseRefName: mr.target_branch, @@ -1211,9 +1225,9 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F { remoteName: "origin", remoteRef: `refs/merge-requests/${mr.iid}/head` }, { remoteName: "origin", remoteRef: `refs/heads/${mr.source_branch}` }, ], - headOwnerLogin, - headRepositorySshUrl: null, - headRepositoryUrl: null, + headOwnerLogin: sourceProject?.namespace.full_path ?? null, + headRepositorySshUrl: sourceProject?.ssh_url_to_repo ?? null, + headRepositoryUrl: sourceProject?.http_url_to_repo ?? null, isCrossRepository, headRefKind: "branch", };