From 4a67ea29de284449a36c63f350d8b5498bcf07f4 Mon Sep 17 00:00:00 2001 From: Yuruzuu Date: Wed, 11 Nov 2026 00:21:52 +0800 Subject: [PATCH] perf(server): a fork default branch finds its pull request 6.5x faster (median 5,450 to 841 ms) Co-Authored-By: Claude Opus 5.5 --- apps/server/src/git/GitManager.test.ts | 15 +-- apps/server/src/git/GitManager.ts | 15 ++- .../src/server/SourceControlProvider.ts | 2 + .../GitHubSourceControlProvider.test.ts | 63 ++++++++--- .../src/server/GitHubSourceControlProvider.ts | 100 ++++++++++++++++-- 5 files changed, 160 insertions(+), 35 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 72b3504807c93..5ded2027a9909 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -94,8 +94,9 @@ interface FakeProviderScenario { /** Check out the pull request head from its pull ref: fast-forward only unless forced. */ syncCheckoutWithPullHead?: boolean; /** - * The host's `headBranchProbe`. By default the fake probes only bare branch names, a hundred - * at a time, so tests see GitManager defer to the hook; null leaves the probe to GitManager. + * The host's `headBranchProbe`. By default the fake probes bare branch names, a hundred at a + * time, or only the owner-qualified default branch, as GitHub does, so tests see GitManager + * defer to the hook; null leaves the probe to GitManager. */ headBranchProbe?: SourceControlProvider["Service"]["headBranchProbe"] | null; } @@ -438,8 +439,10 @@ function createFakeSourceControlProvider(scenario: FakeProviderScenario = {}): { : { headBranchProbe: scenario.headBranchProbe ?? - (({ headSelectors }: { readonly headSelectors: ReadonlyArray }) => ({ - headSelectors: headSelectors.filter((selector) => !selector.includes(":")), + (({ headSelectors, defaultBranchHeadSelector }) => ({ + headSelectors: defaultBranchHeadSelector + ? [defaultBranchHeadSelector] + : headSelectors.filter((selector) => !selector.includes(":")), limit: 100, })), }; @@ -2388,7 +2391,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { const { manager, providerCalls } = yield* makeManager({ providerScenario: { prListByHeadSelector: { - main: [ + "contributor:main": [ { number: 777, title: "Fork PR from main", @@ -2419,7 +2422,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { }); expect(providerCalls).toContainEqual({ operation: "listChangeRequests", - headSelector: "main", + headSelector: "contributor:main", state: "all", limit: 100, }); diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index d9e1d7d4c09e1..bbf093c45c8c7 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -567,9 +567,10 @@ function headBranchProbe( provider: SourceControlProvider.SourceControlProvider["Service"], headSelectors: ReadonlyArray, state: "open" | "all", + defaultBranchHeadSelector?: string, ) { return ( - provider.headBranchProbe?.({ headSelectors, state }) ?? { + provider.headBranchProbe?.({ headSelectors, state, defaultBranchHeadSelector }) ?? { headSelectors, limit: state === "open" ? 1 : 20, } @@ -1258,7 +1259,7 @@ export const make = Effect.gen(function* () { ) { return { latest: null, headContext }; } - const latest = yield* findLatestPrForHeadContext(cwd, headContext); + const latest = yield* findLatestPrForHeadContext(cwd, headContext, details.defaultBranch); if (latest !== null) yield* noteLookupState(latest); return { latest, headContext }; }); @@ -1912,11 +1913,19 @@ export const make = Effect.gen(function* () { const findLatestPrForHeadContext = Effect.fn("findLatestPrForHeadContext")(function* ( cwd: string, headContext: BranchHeadContext, + defaultBranch: string | null, ) { const parsedByNumber = new Map(); const provider = yield* sourceControlProvider(cwd); - const probe = headBranchProbe(provider, headContext.headSelectors, "all"); + const probe = headBranchProbe( + provider, + headContext.headSelectors, + "all", + headContext.headBranch === defaultBranch && headContext.headRepositoryOwnerLogin + ? `${headContext.headRepositoryOwnerLogin}:${defaultBranch}` + : undefined, + ); for (const headSelector of probe.headSelectors) { const pullRequests = yield* provider.listChangeRequests({ cwd, diff --git a/packages/source-control-core/src/server/SourceControlProvider.ts b/packages/source-control-core/src/server/SourceControlProvider.ts index a54e47921065c..3bd72f8eb2ef8 100644 --- a/packages/source-control-core/src/server/SourceControlProvider.ts +++ b/packages/source-control-core/src/server/SourceControlProvider.ts @@ -121,6 +121,8 @@ export class SourceControlProvider extends Context.Service< readonly headBranchProbe?: (input: { readonly headSelectors: ReadonlyArray; readonly state: "open" | "all"; + /** `owner:branch` when the head is the default branch, a name most forks share. */ + readonly defaultBranchHeadSelector?: string | undefined; }) => { readonly headSelectors: ReadonlyArray; readonly limit: number }; readonly readChangeRequestTemplate?: (input: { readonly cwd: string; diff --git a/packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts b/packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts index a5a08fcfc2cc4..6bcdf28429609 100644 --- a/packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts +++ b/packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts @@ -421,28 +421,59 @@ describe("GitHubSourceControlProvider repository resolution", () => { describe("GitHubSourceControlProvider change request lookups by head", () => { const remotes = remotesOutput(["origin", "git@github.com:acme/web.git"]); - it.effect("matches an owner:branch selector on the head owner", () => { - const { layer } = harness({ + it.effect("filters an owner:branch selector by owner through REST", () => { + const paths: string[] = []; + const restNode = (number: number, mergedAt: string | null) => ({ + number, + title: `PR ${number}`, + html_url: `https://github.com/acme/web/pull/${number}`, + state: mergedAt === null ? "open" : "closed", + merged_at: mergedAt, + closed_at: mergedAt, + updated_at: "2026-01-02T00:00:00Z", + base: { ref: "main", repo: { full_name: "acme/web" } }, + head: { + ref: "main", + sha: "f".repeat(40), + repo: { name: "web", full_name: "me/web", owner: { login: "me", type: "User" } }, + }, + }); + const { layer, graphqlCalls } = harness({ remotes, - graphql: (lookup) => ({ - pullRequests: { - nodes: - lookup.head === "main" - ? [node(9, "main", "someone"), node(8, "main", "me"), node(7, "main", "me")] - : [], - }, - }), + api: { + rest: (input) => + Effect.sync(() => { + paths.push(input.path); + return restResponse([restNode(8, null), restNode(7, "2026-01-01T00:00:00Z")]); + }), + }, }); return Effect.gen(function* () { const gh = yield* GitHubSourceControlProvider.make; - const open = yield* gh - .listChangeRequests({ cwd: "/repo", headSelector: "me:main", state: "open", limit: 1 }) - .pipe(Effect.forkChild); - yield* TestClock.adjust("50 millis"); + const probe = (defaultBranchHeadSelector?: string) => + gh.headBranchProbe?.({ + headSelectors: ["me:main", "main"], + state: "all", + defaultBranchHeadSelector, + }).headSelectors; + assert.deepStrictEqual([probe(), probe("me:main")], [["main"], ["me:main"]]); + const read = (state: "open" | "merged") => + gh.listChangeRequests({ cwd: "/repo", headSelector: "me:main", state, limit: 1 }); + const [open] = yield* read("open"); + const [merged] = yield* read("merged"); + assert.deepStrictEqual( + [open, merged].map((pr) => [pr?.number, pr?.state, pr?.headRepositoryOwnerLogin]), + [ + [8, "open", "me"], + [7, "merged", "me"], + ], + ); assert.deepStrictEqual( - (yield* Fiber.join(open)).map((pr) => pr.number), - [8], + paths.map((path) => new URL(path, "https://api.test/").searchParams.get("state")), + ["open", "closed"], ); + assert.include(paths[0], "repos/acme/web/pulls?head=me%3Amain"); + assert.lengthOf(graphqlCalls, 0); }).pipe(Effect.provide(layer)); }); diff --git a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts index 4abb3d8d6669f..9dde6f1f24a24 100644 --- a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts +++ b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts @@ -393,6 +393,60 @@ function repositoryCloneUrls( const decodeViewerLogin = decodeJsonResult(Schema.Struct({ login: TrimmedNonEmptyString })); +const decodeRestPullRequests = decodeJsonResult(Schema.Array(Schema.Unknown)); +const decodeRestPullRequest = Schema.decodeUnknownOption( + Schema.Struct({ + number: Schema.Number, + title: Schema.String, + html_url: Schema.String, + state: Schema.String, + draft: Schema.optional(Schema.Boolean), + merged_at: Schema.NullOr(Schema.String), + closed_at: Schema.NullOr(Schema.String), + updated_at: Schema.String, + base: Schema.Struct({ ref: Schema.String, repo: Schema.Struct({ full_name: Schema.String }) }), + head: Schema.Struct({ + ref: Schema.String, + sha: Schema.String, + repo: Schema.NullOr( + Schema.Struct({ + name: Schema.String, + full_name: Schema.String, + owner: Schema.Struct({ login: Schema.String, type: Schema.String }), + }), + ), + }), + }), +); + +/** Reshapes a REST pull request into the GraphQL node so both decode the same way. */ +function restPullRequestNode(entry: unknown) { + const pr = Option.getOrNull(decodeRestPullRequest(entry)); + if (pr === null) return null; + return { + number: pr.number, + title: pr.title, + url: pr.html_url, + baseRefName: pr.base.ref, + headRefName: pr.head.ref, + headRefOid: pr.head.sha, + state: pr.merged_at === null ? pr.state.toUpperCase() : "MERGED", + isDraft: pr.draft === true, + mergedAt: pr.merged_at, + closedAt: pr.closed_at, + updatedAt: pr.updated_at, + isCrossRepository: pr.head.repo?.full_name !== pr.base.repo.full_name, + headRepository: pr.head.repo && { + name: pr.head.repo.name, + nameWithOwner: pr.head.repo.full_name, + }, + headRepositoryOwner: pr.head.repo && { + __typename: pr.head.repo.owner.type, + login: pr.head.repo.owner.login, + }, + }; +} + type PullRequestListState = "open" | "closed" | "merged" | "all"; const GRAPHQL_STATES: Record< @@ -559,6 +613,34 @@ export const make = Effect.gen(function* () { const limit = Math.min(Math.max(Math.trunc(input.limit), 1), 100); const ownerMatch = /^([^:/\s]+):(.+)$/u.exec(input.headSelector.trim()); const headRefName = ownerMatch?.[2] ?? input.headSelector.trim(); + if (ownerMatch) { + // GraphQL cannot filter heads by owner, and names like a fork's `main` match thousands. + const query = new URLSearchParams({ + head: `${ownerMatch[1]}:${headRefName}`, + state: input.state === "all" || input.state === "open" ? input.state : "closed", + sort: "created", + direction: "desc", + per_page: String(OWNER_HEAD_SCAN_LIMIT), + }); + const response = yield* rest({ + host: locator.host, + operation: "listPullRequestsByHead", + allowReserve: input.allowReserve, + path: `repos/${encodeURIComponent(locator.owner)}/${encodeURIComponent(locator.name)}/pulls?${query}`, + }); + const decoded = decodeRestPullRequests(response.body); + if (Result.isFailure(decoded)) { + return yield* failure("GitHub returned an invalid change request list.", decoded.failure); + } + return decoded.success + .flatMap((entry) => { + const node = Option.getOrNull(decodePullRequestNode(restPullRequestNode(entry))); + const record = node === null ? null : toPullRequestRecord(node); + return record === null ? [] : [record]; + }) + .filter((row) => input.state === "all" || row.state === input.state) + .slice(0, limit); + } const result = yield* graphqlRead( { host: locator.host, @@ -574,22 +656,17 @@ export const make = Effect.gen(function* () { name: locator.name, head: headRefName, states: GRAPHQL_STATES[input.state], - first: ownerMatch ? OWNER_HEAD_SCAN_LIMIT : limit, + first: limit, }, input.allowReserve ? interactiveHeadResolver : backgroundHeadResolver, ), ); if (result.repository === null) return yield* notFound("The repository could not be read."); - const rows = (result.repository.pullRequests.nodes ?? []).flatMap((entry) => { + return (result.repository.pullRequests.nodes ?? []).flatMap((entry) => { const node = Option.getOrNull(decodePullRequestNode(entry)); const record = node === null ? null : toPullRequestRecord(node); return record === null ? [] : [record]; }); - if (!ownerMatch) return rows; - const headOwner = ownerMatch[1]!.toLowerCase(); - return rows - .filter((row) => row.headRepositoryOwnerLogin?.toLowerCase() === headOwner) - .slice(0, limit); }); const readPullRequest = Effect.fn("GitHubSourceControlProvider.readPullRequest")( @@ -876,9 +953,12 @@ export const make = Effect.gen(function* () { resolveRemoteWebHost: Effect.fn("GitHubSourceControlProvider.resolveRemoteWebHost")( ({ context }) => Effect.succeed(gitHubApiHostForRemote(context.remoteUrl)), ), - // GitHub matches bare heads; read a full page to disambiguate forks by owner. - headBranchProbe: ({ headSelectors }) => ({ - headSelectors: headSelectors.filter((selector) => !selector.includes(":")), + // GitHub matches bare heads; read a full page to disambiguate forks by owner. A default branch + // collides with most forks, so its owner-qualified lookup has GitHub filter by owner instead. + headBranchProbe: ({ headSelectors, defaultBranchHeadSelector }) => ({ + headSelectors: defaultBranchHeadSelector + ? [defaultBranchHeadSelector] + : headSelectors.filter((selector) => !selector.includes(":")), limit: 100, }), readChangeRequestTemplate: Effect.fn("GitHubSourceControlProvider.readChangeRequestTemplate")(