refactor(forge): share fork branch naming with github

moved the owner normalizer and the branch name builder to
utils/change-request-checkout.ts, gitea and github both use it.

github changes a bit: a fork PR with an unknown or unusable owner (eg
deleted fork) now gets pr-N/<branch> instead of the bare branch name,
which could collide with a local branch the same way #3515 did.

packages/server/src/services/forge-service.ts

@@ -441,9 +441,9 @@ export interface ForgeService {
   getPullRequestCheckoutTarget(options: GetPullRequestOptions): Promise<PullRequestCheckoutTarget>;
   /**
    * Refs to fetch for a change-request checkout when the resolved checkout
-   * target carries none. Adapters that expose a universal change-request head
-   * ref (GitHub's refs/pull/N/head) return it here; others let the shell fall
-   * back to the head branch.
+   * target carries none. Adapters with a universal change-request head ref
+   * (refs/pull/N/head) return it here; others let the shell fall back to the
+   * head branch.
    */
   defaultCheckoutRefs?(params: {
     changeRequestNumber: number;
@@ -451,7 +451,7 @@ export interface ForgeService {
   }): PullRequestCheckoutRef[];
   /**
    * Local branch name for a checked-out change request when the adapter
-   * disambiguates cross-repository heads (GitHub prefixes the fork owner).
+   * disambiguates cross-repository heads (prefixes the fork owner).
    * Returns undefined to keep the head ref name as-is.
    */
   buildPrLocalBranchName?(params: {

packages/server/src/services/gitea-service.test.ts

@@ -2359,101 +2359,6 @@ describe("createGiteaService", () => {
     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(""));
 

packages/server/src/services/gitea-service.ts

@@ -5,6 +5,7 @@ import pLimit from "p-limit";
 import { parseGitHubRemoteIdentity, parseGitRemoteLocation } from "@getpaseo/protocol/git-remote";
 import { findExecutable } from "../executable-resolution/executable-resolution.js";
 import { runGitCommand } from "../utils/run-git-command.js";
+import { buildForkLocalBranchName } from "../utils/change-request-checkout.js";
 import { execCommand } from "../utils/spawn.js";
 import {
   createCachedCliPathResolver,
@@ -1377,19 +1378,6 @@ 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);
@@ -2015,12 +2003,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For
     },
 
     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}`;
+      return buildForkLocalBranchName({ headRef, ...checkoutTarget });
     },
 
     async getPullRequestCheckoutTarget(

packages/server/src/services/github-service.ts

@@ -6,6 +6,7 @@ import {
 } from "@getpaseo/protocol/git-remote";
 import { findExecutable } from "../executable-resolution/executable-resolution.js";
 import { runGitCommand } from "../utils/run-git-command.js";
+import { buildForkLocalBranchName } from "../utils/change-request-checkout.js";
 import { execCommand } from "../utils/spawn.js";
 import { resolveSshHostname } from "../utils/ssh-hostname.js";
 import {
@@ -2111,10 +2112,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G
     },
 
     buildPrLocalBranchName({ headRef, checkoutTarget }) {
-      const owner = checkoutTarget.isCrossRepository
-        ? normalizeGitHubOwnerForBranch(checkoutTarget.headOwnerLogin)
-        : null;
-      return owner ? `${owner}/${headRef}` : headRef;
+      return buildForkLocalBranchName({ headRef, ...checkoutTarget });
     },
 
     supportsCrossRepoCheckoutWithoutRefs: true,
@@ -2663,11 +2661,6 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G
   return api;
 }
 
-function normalizeGitHubOwnerForBranch(owner: string | null): string | null {
-  const normalized = owner?.trim().toLowerCase() ?? "";
-  return /^[a-z0-9-]+$/.test(normalized) ? normalized : null;
-}
-
 function getGithubStatusFacts(
   status: PullRequestCommandStatus | null | undefined,
 ): GitHubPullRequestStatusFacts | null {

packages/server/src/utils/change-request-checkout.test.ts

@@ -0,0 +1,64 @@
+import { describe, expect, it } from "vitest";
+
+import {
+  buildForkLocalBranchName,
+  normalizeForgeOwnerForBranch,
+} from "./change-request-checkout.js";
+
+describe("normalizeForgeOwnerForBranch", () => {
+  it("lowercases a valid login", () => {
+    expect(normalizeForgeOwnerForBranch("Contributor")).toBe("contributor");
+  });
+
+  it("allows dots and underscores", () => {
+    expect(normalizeForgeOwnerForBranch("a.b_c")).toBe("a.b_c");
+  });
+
+  it("rejects a login starting with a dash", () => {
+    expect(normalizeForgeOwnerForBranch("-abc")).toBeNull();
+  });
+
+  it("rejects a login starting with a dot", () => {
+    expect(normalizeForgeOwnerForBranch(".hidden")).toBeNull();
+  });
+
+  it("rejects a login containing '..'", () => {
+    expect(normalizeForgeOwnerForBranch("a..b")).toBeNull();
+  });
+
+  it("rejects a login ending in .lock", () => {
+    expect(normalizeForgeOwnerForBranch("contributor.lock")).toBeNull();
+  });
+
+  it("rejects a login with a path traversal segment", () => {
+    expect(normalizeForgeOwnerForBranch("../evil")).toBeNull();
+  });
+
+  it("rejects an empty or null login", () => {
+    expect(normalizeForgeOwnerForBranch(null)).toBeNull();
+    expect(normalizeForgeOwnerForBranch("")).toBeNull();
+    expect(normalizeForgeOwnerForBranch("   ")).toBeNull();
+  });
+});
+
+describe("buildForkLocalBranchName", () => {
+  const fork = { headRef: "patch-1", number: 42, isCrossRepository: true };
+
+  it("keeps the head ref for a same-repository head", () => {
+    expect(
+      buildForkLocalBranchName({ ...fork, isCrossRepository: false, headOwnerLogin: "arthur" }),
+    ).toBe("patch-1");
+  });
+
+  it("prefixes a fork head with the lowercased owner", () => {
+    expect(buildForkLocalBranchName({ ...fork, headOwnerLogin: "Arthur" })).toBe("arthur/patch-1");
+  });
+
+  it("falls back to the pr number when the owner is unknown", () => {
+    expect(buildForkLocalBranchName({ ...fork, headOwnerLogin: null })).toBe("pr-42/patch-1");
+  });
+
+  it("falls back to the pr number when the owner is not a safe ref segment", () => {
+    expect(buildForkLocalBranchName({ ...fork, headOwnerLogin: "-zaphod" })).toBe("pr-42/patch-1");
+  });
+});

packages/server/src/utils/change-request-checkout.ts

@@ -0,0 +1,27 @@
+// forgejo username pattern, covers github logins too
+export function normalizeForgeOwnerForBranch(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 interface ForkLocalBranchNameInput {
+  headRef: string;
+  number: number;
+  isCrossRepository: boolean;
+  headOwnerLogin: string | null;
+}
+
+export function buildForkLocalBranchName(params: ForkLocalBranchNameInput): string {
+  if (!params.isCrossRepository) {
+    return params.headRef;
+  }
+  const owner = normalizeForgeOwnerForBranch(params.headOwnerLogin);
+  return `${owner ?? `pr-${params.number}`}/${params.headRef}`;
+}