From c77b046337663b829b332d84e795879ea236fb64 Mon Sep 17 00:00:00 2001 From: Alexander Adam Date: Thu, 24 Sep 2026 22:14:51 +0200 Subject: [PATCH] fix(gitea): handle AGit PRs and deleted head branches neither has a branch on origin. forgejo reports refs/pull/N/head as head.ref for both, so skip the refs/heads fallback and dont track origin. also try refs/pull/N/head on upstream for clones where origin is your fork, like github does. that list lives in the shared util now. --- .../resolve-worktree-creation-intent.test.ts | 22 ++++++ .../resolve-worktree-creation-intent.ts | 3 +- packages/server/src/services/forge-service.ts | 2 + .../server/src/services/gitea-service.test.ts | 73 ++++++++++++++++++- packages/server/src/services/gitea-service.ts | 50 +++++++++---- .../server/src/services/github-service.ts | 10 +-- .../src/utils/change-request-checkout.ts | 10 +++ 7 files changed, 146 insertions(+), 24 deletions(-) diff --git a/packages/server/src/server/resolve-worktree-creation-intent.test.ts b/packages/server/src/server/resolve-worktree-creation-intent.test.ts index 82895e302b..e7509d7006 100644 --- a/packages/server/src/server/resolve-worktree-creation-intent.test.ts +++ b/packages/server/src/server/resolve-worktree-creation-intent.test.ts @@ -182,6 +182,28 @@ describe("resolveWorktreeCreationIntent", () => { expect(deps.headRefLookups).toEqual([]); }); + test("does not track origin for a same-repo head with no origin branch", async () => { + const deps = createResolverHarness(); + deps.forgeService.getPullRequestCheckoutTarget = async () => ({ + number: 42, + baseRefName: "main", + headRefName: "ford/towel", + headOwnerLogin: null, + headRepositorySshUrl: null, + headRepositoryUrl: null, + isCrossRepository: false, + hasOriginHeadBranch: false, + }); + + const intent = await resolveWorktreeCreationIntent( + { action: "checkout", githubPrNumber: 42 }, + repoRoot, + deps, + ); + + expect(intent).not.toHaveProperty("trackOriginHead"); + }); + test("configures the contributor remote for fork PR targets", async () => { const deps = createResolverHarness(); deps.forgeService.getPullRequestCheckoutTarget = async () => ({ diff --git a/packages/server/src/server/resolve-worktree-creation-intent.ts b/packages/server/src/server/resolve-worktree-creation-intent.ts index 73403f5908..15aba9479a 100644 --- a/packages/server/src/server/resolve-worktree-creation-intent.ts +++ b/packages/server/src/server/resolve-worktree-creation-intent.ts @@ -191,7 +191,8 @@ async function resolvePrCheckoutIntent( }) ?? [{ remoteName: "origin", remoteRef: `refs/heads/${headRef}` }]; const localBranchName = service.buildPrLocalBranchName?.({ headRef, checkoutTarget }); const crossRepository = resolveCrossRepositoryFields(checkoutTarget); - const trackOriginHead = !checkoutTarget.isCrossRepository; + const trackOriginHead = + !checkoutTarget.isCrossRepository && checkoutTarget.hasOriginHeadBranch !== false; return { kind: "checkout-change-request", diff --git a/packages/server/src/services/forge-service.ts b/packages/server/src/services/forge-service.ts index a3e70cd5d1..6f48a7b767 100644 --- a/packages/server/src/services/forge-service.ts +++ b/packages/server/src/services/forge-service.ts @@ -39,6 +39,8 @@ export interface PullRequestCheckoutTarget { headRepositorySshUrl: string | null; headRepositoryUrl: string | null; isCrossRepository: boolean; + /** false when origin has no head branch to track */ + hasOriginHeadBranch?: boolean; } export interface PullRequestCheckoutRef { diff --git a/packages/server/src/services/gitea-service.test.ts b/packages/server/src/services/gitea-service.test.ts index 1f00e780c9..3d5a8aa632 100644 --- a/packages/server/src/services/gitea-service.test.ts +++ b/packages/server/src/services/gitea-service.test.ts @@ -2297,11 +2297,13 @@ describe("createGiteaService", () => { checkoutRefs: [ { remoteName: "origin", remoteRef: "refs/pull/5/head" }, { remoteName: "origin", remoteRef: "refs/heads/feat/sample-change" }, + { remoteName: "upstream", remoteRef: "refs/pull/5/head" }, ], headOwnerLogin: null, headRepositorySshUrl: null, headRepositoryUrl: null, isCrossRepository: false, + hasOriginHeadBranch: true, }); }); @@ -2330,16 +2332,62 @@ describe("createGiteaService", () => { number: 5, baseRefName: "main", headRefName: "feat/sample-change", - checkoutRefs: [{ remoteName: "origin", remoteRef: "refs/pull/5/head" }], + checkoutRefs: [ + { remoteName: "origin", remoteRef: "refs/pull/5/head" }, + { remoteName: "upstream", remoteRef: "refs/pull/5/head" }, + ], headOwnerLogin: "contributor", headRepositorySshUrl: "git@gitea.com:contributor/sample-repo.git", headRepositoryUrl: "https://gitea.com/contributor/sample-repo", isCrossRepository: true, + hasOriginHeadBranch: false, }); expect(calls).toContainEqual(["api", "repos/example-user/sample-repo/pulls/5"]); }); + it("skips the origin branch fallback and tracking for an AGit pull request", async () => { + const { service } = makeService((args) => + args[0] === "api" + ? ok( + JSON.stringify({ + flow: 1, + head: { ref: "refs/pull/5/head", repo: { id: 1 } }, + base: { repo: { id: 1 } }, + }), + ) + : ok(JSON.stringify(STATUS_PR_VIEW)), + ); + + const target = await service.getPullRequestCheckoutTarget({ cwd: "/repo", number: 5 }); + + expect(target).toMatchObject({ + checkoutRefs: [ + { remoteName: "origin", remoteRef: "refs/pull/5/head" }, + { remoteName: "upstream", remoteRef: "refs/pull/5/head" }, + ], + isCrossRepository: false, + hasOriginHeadBranch: false, + }); + }); + + it("skips the origin branch fallback when the head branch was deleted", async () => { + const { service } = makeService((args) => + args[0] === "api" + ? ok( + JSON.stringify({ + head: { ref: "refs/pull/5/head", repo: { id: 1 } }, + base: { repo: { id: 1 } }, + }), + ) + : ok(JSON.stringify(STATUS_PR_VIEW)), + ); + + const target = await service.getPullRequestCheckoutTarget({ cwd: "/repo", number: 5 }); + + expect(target.hasOriginHeadBranch).toBe(false); + }); + it("keeps a pull request from a deleted fork cross-repository", async () => { const { service } = makeService((args) => args[0] === "api" @@ -2350,12 +2398,32 @@ describe("createGiteaService", () => { const target = await service.getPullRequestCheckoutTarget({ cwd: "/repo", number: 5 }); expect(target).toMatchObject({ - checkoutRefs: [{ remoteName: "origin", remoteRef: "refs/pull/5/head" }], + checkoutRefs: [ + { remoteName: "origin", remoteRef: "refs/pull/5/head" }, + { remoteName: "upstream", remoteRef: "refs/pull/5/head" }, + ], headOwnerLogin: null, isCrossRepository: true, + hasOriginHeadBranch: false, }); }); + it("keeps the origin branch fallback when the server sends no head ref", async () => { + const { service } = makeService((args) => + args[0] === "api" + ? ok(JSON.stringify({ head: { repo: { id: 1 } }, base: { repo: { id: 1 } } })) + : ok(JSON.stringify(STATUS_PR_VIEW)), + ); + + const target = await service.getPullRequestCheckoutTarget({ cwd: "/repo", number: 5 }); + + expect(target.checkoutRefs).toContainEqual({ + remoteName: "origin", + remoteRef: "refs/heads/feat/sample-change", + }); + expect(target.hasOriginHeadBranch).toBe(true); + }); + it("prefixes the local branch name with the fork owner for a cross-repository checkout", () => { const { service } = makeService(() => ok("")); @@ -2380,6 +2448,7 @@ describe("createGiteaService", () => { expect(service.defaultCheckoutRefs?.({ changeRequestNumber: 5, headRef: "patch-1" })).toEqual([ { remoteName: "origin", remoteRef: "refs/pull/5/head" }, + { remoteName: "upstream", remoteRef: "refs/pull/5/head" }, ]); }); diff --git a/packages/server/src/services/gitea-service.ts b/packages/server/src/services/gitea-service.ts index b6d2088c5e..5c1a431603 100644 --- a/packages/server/src/services/gitea-service.ts +++ b/packages/server/src/services/gitea-service.ts @@ -5,7 +5,10 @@ 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 { + buildForkLocalBranchName, + buildPullHeadCheckoutRefs, +} from "../utils/change-request-checkout.js"; import { execCommand } from "../utils/spawn.js"; import { createCachedCliPathResolver, @@ -76,6 +79,8 @@ const CURRENT_PR_HEAD_SHA_CACHE_TTL_MS = 30_000; // the sweep to the 250 most recent Actions tasks. const GITEA_ACTION_PAGE_LIMIT = 50; const GITEA_ACTION_MAX_PAGES = 5; +// PullRequestFlowAGit in forgejo models/issues/pull.go +const GITEA_PULL_REQUEST_FLOW_AGIT = 1; /** * Fields requested from `tea pr list -o json`. tea's default field set omits the @@ -226,8 +231,12 @@ const GiteaPullRequestRepoSchema = z const GiteaPullRequestApiSchema = z .object({ + flow: z.number().optional(), head: z - .object({ repo: GiteaPullRequestRepoSchema.nullable().optional() }) + .object({ + ref: z.string().optional(), + repo: GiteaPullRequestRepoSchema.nullable().optional(), + }) .passthrough() .optional(), base: z @@ -1441,12 +1450,14 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For summary: PullRequestSummary, ): Promise<{ isCrossRepository: boolean; + hasHeadBranch: boolean; headOwnerLogin: string | null; headRepositorySshUrl: string | null; headRepositoryUrl: string | null; }> { const sameRepo = { isCrossRepository: false, + hasHeadBranch: true, headOwnerLogin: null, headRepositorySshUrl: null, headRepositoryUrl: null, @@ -1465,18 +1476,21 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For ); const headRepo = pr.head?.repo ?? null; const baseRepo = pr.base?.repo ?? null; + // head.ref only becomes the branch name when the branch still exists, + // agit PRs and deleted branches keep the pull ref + const hasHeadBranch = + pr.flow !== GITEA_PULL_REQUEST_FLOW_AGIT && + pr.head?.ref !== `refs/pull/${summary.number}/head`; if (baseRepo?.id == null || headRepo?.id === baseRepo.id) { - return sameRepo; - } - if (headRepo?.id == null) { - // fork was deleted - return { ...sameRepo, isCrossRepository: true }; + return { ...sameRepo, hasHeadBranch }; } + // headRepo is null when the fork was deleted return { isCrossRepository: true, - headOwnerLogin: headRepo.owner?.login ?? null, - headRepositorySshUrl: headRepo.ssh_url ?? null, - headRepositoryUrl: headRepo.html_url ?? null, + hasHeadBranch, + headOwnerLogin: headRepo?.owner?.login ?? null, + headRepositorySshUrl: headRepo?.ssh_url ?? null, + headRepositoryUrl: headRepo?.html_url ?? null, }; } @@ -2003,7 +2017,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For }, defaultCheckoutRefs({ changeRequestNumber }) { - return [{ remoteName: "origin", remoteRef: `refs/pull/${changeRequestNumber}/head` }]; + return buildPullHeadCheckoutRefs(changeRequestNumber); }, buildPrLocalBranchName({ headRef, checkoutTarget }) { @@ -2015,13 +2029,16 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For ): Promise { const summary = await this.getPullRequest(input); const fork = await resolveForkCheckoutFacts(input.cwd, summary); - const checkoutRefs = [ - { remoteName: "origin", remoteRef: `refs/pull/${summary.number}/head` }, - ]; - // on a fork PR thats the base repos branch - if (summary.headRefName && !fork.isCrossRepository) { + const [originPullRef, upstreamPullRef] = buildPullHeadCheckoutRefs(summary.number); + const checkoutRefs = [originPullRef]; + // for a fork PR a branch with that name on origin belongs to the base repo + const headBranchOnOrigin = + !fork.isCrossRepository && fork.hasHeadBranch && Boolean(summary.headRefName); + if (headBranchOnOrigin) { checkoutRefs.push({ remoteName: "origin", remoteRef: `refs/heads/${summary.headRefName}` }); } + // upstream can hold a different PR with the same number, so it goes last + checkoutRefs.push(upstreamPullRef); return { number: summary.number, baseRefName: summary.baseRefName, @@ -2031,6 +2048,7 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For headRepositorySshUrl: fork.headRepositorySshUrl, headRepositoryUrl: fork.headRepositoryUrl, isCrossRepository: fork.isCrossRepository, + hasOriginHeadBranch: headBranchOnOrigin, }; }, diff --git a/packages/server/src/services/github-service.ts b/packages/server/src/services/github-service.ts index 28dc7c58f9..0ea4a8bf14 100644 --- a/packages/server/src/services/github-service.ts +++ b/packages/server/src/services/github-service.ts @@ -6,7 +6,10 @@ 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 { + buildForkLocalBranchName, + buildPullHeadCheckoutRefs, +} from "../utils/change-request-checkout.js"; import { execCommand } from "../utils/spawn.js"; import { resolveSshHostname } from "../utils/ssh-hostname.js"; import { @@ -2105,10 +2108,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G }, defaultCheckoutRefs({ changeRequestNumber }) { - return [ - { remoteName: "origin", remoteRef: `refs/pull/${changeRequestNumber}/head` }, - { remoteName: "upstream", remoteRef: `refs/pull/${changeRequestNumber}/head` }, - ]; + return buildPullHeadCheckoutRefs(changeRequestNumber); }, buildPrLocalBranchName({ headRef, checkoutTarget }) { diff --git a/packages/server/src/utils/change-request-checkout.ts b/packages/server/src/utils/change-request-checkout.ts index cf5dcaa8fa..f3f0b3feeb 100644 --- a/packages/server/src/utils/change-request-checkout.ts +++ b/packages/server/src/utils/change-request-checkout.ts @@ -1,3 +1,13 @@ +import type { PullRequestCheckoutRef } from "../services/forge-service.js"; + +// upstream for clones where origin is your fork +export function buildPullHeadCheckoutRefs(number: number): PullRequestCheckoutRef[] { + return [ + { remoteName: "origin", remoteRef: `refs/pull/${number}/head` }, + { remoteName: "upstream", remoteRef: `refs/pull/${number}/head` }, + ]; +} + // forgejo username pattern, covers github logins too export function normalizeForgeOwnerForBranch(owner: string | null): string | null { const normalized = owner?.trim().toLowerCase() ?? "";