authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-09-22 20:50:50-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-22 20:50:50-07:00
logeafb4a9340bd2c2271b7c8fac7eddd55beea3972
tree018bd6a7573dbb9f39077a250d8f40681f605c29
parent18de6bb328059703b5d1b0661610076e3c0fa2d4
signature Signed by PGP key B5690EEEBB952194

fix(server): GitHub PR lookups stop probing owner-qualified heads (#13200)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

2 files changed, 51 insertions(+), 104 deletions(-)

apps/server/src/git/GitManager.test.ts+21-98
......@@ -1309,7 +1309,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
13091309 prListByHeadSelector: {
13101310 // Fake gh returns raw JSON stdout, matching the CLI boundary under test.
13111311 // @effect-diagnostics-next-line preferSchemaOverJson:off
1312 "contributor:feature/deleted-fork-branch": JSON.stringify([
1312 "feature/deleted-fork-branch": JSON.stringify([
13131313 {
13141314 number: 218,
13151315 title: "Deleted fork branch PR",
......@@ -1339,8 +1339,9 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
13391339 updatedAt: "2026-04-05T15:00:00.000Z",
13401340 });
13411341 expect(
1342 ghCalls.some((call) => call.includes("--head contributor:feature/deleted-fork-branch")),
1342 ghCalls.some((call) => call.includes("--head feature/deleted-fork-branch --state all")),
13431343 ).toBe(true);
1344 expect(ghCalls.some((call) => call.includes("--head contributor:"))).toBe(false);
13441345 }),
13451346 );
13461347
......@@ -1776,8 +1777,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
17761777 const { manager } = yield* makeManager({
17771778 ghScenario: {
17781779 prListByHeadSelector: {
1779 "alice:feature": output,
1780 "fork:feature": output,
17811780 feature: output,
17821781 },
17831782 },
......@@ -1944,10 +1943,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
19441943 const { manager, ghCalls } = yield* makeManager({
19451944 ghScenario: {
19461945 prListSequence: [
1947 // @effect-diagnostics-next-line preferSchemaOverJson:off
1948 JSON.stringify([]),
1949 // @effect-diagnostics-next-line preferSchemaOverJson:off
1950 JSON.stringify([]),
19511946 // @effect-diagnostics-next-line preferSchemaOverJson:off
19521947 JSON.stringify([
19531948 {
......@@ -1983,7 +1978,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
19831978 updatedAt: "2026-03-10T07:00:00.000Z",
19841979 });
19851980 expect(ghCalls).toContain(
1986 "pr list --head jasonLaster:statemachine --state all --limit 20 --json number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
1981 "pr list --head statemachine --state all --limit 100 --json number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
19871982 );
19881983 }),
19891984 20_000,
......@@ -2015,7 +2010,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
20152010 ghScenario: {
20162011 prListByHeadSelector: {
20172012 // @effect-diagnostics-next-line preferSchemaOverJson:off
2018 "contributor:main": JSON.stringify([
2013 main: JSON.stringify([
20192014 {
20202015 number: 777,
20212016 title: "Fork PR from main",
......@@ -2049,7 +2044,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
20492044 updatedAt: "2026-03-10T07:00:00.000Z",
20502045 });
20512046 expect(ghCalls).toContain(
2052 "pr list --head contributor:main --state all --limit 20 --json number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
2047 "pr list --head main --state all --limit 100 --json number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
20532048 );
20542049 }),
20552050 20_000,
......@@ -2114,34 +2109,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
21142109 updatedAt: "2026-04-01T10:00:00Z",
21152110 },
21162111 ]),
2117 // @effect-diagnostics-next-line preferSchemaOverJson:off
2118 "pingdotgg:effect-atom": JSON.stringify([]),
2119 // @effect-diagnostics-next-line preferSchemaOverJson:off
2120 "my-org/upstream:effect-atom": JSON.stringify([]),
2121 // @effect-diagnostics-next-line preferSchemaOverJson:off
2122 "pingdotgg:upstream/effect-atom": JSON.stringify([
2123 {
2124 number: 1518,
2125 title: "Wrong PR",
2126 url: "https://github.com/pingdotgg/t3code/pull/1518",
2127 baseRefName: "main",
2128 headRefName: "upstream/effect-atom",
2129 state: "OPEN",
2130 updatedAt: "2026-04-01T10:00:00Z",
2131 },
2132 ]),
2133 // @effect-diagnostics-next-line preferSchemaOverJson:off
2134 "my-org/upstream:upstream/effect-atom": JSON.stringify([
2135 {
2136 number: 1518,
2137 title: "Wrong PR",
2138 url: "https://github.com/pingdotgg/t3code/pull/1518",
2139 baseRefName: "main",
2140 headRefName: "upstream/effect-atom",
2141 state: "OPEN",
2142 updatedAt: "2026-04-01T10:00:00Z",
2143 },
2144 ]),
21452112 },
21462113 },
21472114 });
......@@ -2352,7 +2319,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
23522319 prListByHeadSelector: {
23532320 // Fake gh returns raw JSON stdout, matching the CLI boundary under test.
23542321 // @effect-diagnostics-next-line preferSchemaOverJson:off
2355 "contributor:feature/fork-plain": JSON.stringify([
2322 "feature/fork-plain": JSON.stringify([
23562323 {
23572324 number: 89,
23582325 title: "Fork PR pushed without -u",
......@@ -2372,9 +2339,8 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
23722339
23732340 const status = yield* manager.status({ cwd: repoDir });
23742341 expect(status.pr?.number).toBe(89);
2375 expect(ghCalls.some((call) => call.includes("--head contributor:feature/fork-plain"))).toBe(
2376 true,
2377 );
2342 expect(ghCalls.some((call) => call.includes("--head feature/fork-plain"))).toBe(true);
2343 expect(ghCalls.some((call) => call.includes("--head contributor:"))).toBe(false);
23782344 expect(ghCalls.some((call) => call.includes("--head main"))).toBe(false);
23792345 }),
23802346 );
......@@ -2409,7 +2375,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
24092375 prListByHeadSelector: {
24102376 // Fake gh returns raw JSON stdout, matching the CLI boundary under test.
24112377 // @effect-diagnostics-next-line preferSchemaOverJson:off
2412 "contributor:feature/fork-settle": JSON.stringify([
2378 "feature/fork-settle": JSON.stringify([
24132379 {
24142380 number: 91,
24152381 title: "Fork PR to settle",
......@@ -3570,7 +3536,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
35703536 );
35713537
35723538 it.effect(
3573 "returns existing cross-repo PR metadata using the fork owner selector",
3539 "returns existing cross-repo PR metadata found under the bare branch name",
35743540 () =>
35753541 Effect.gen(function* () {
35763542 const repoDir = yield* makeTempDir("t3code-git-manager-");
......@@ -3589,8 +3555,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
35893555 const { manager, ghCalls } = yield* makeManager({
35903556 ghScenario: {
35913557 prListSequence: [
3592 // @effect-diagnostics-next-line preferSchemaOverJson:off
3593 JSON.stringify([]),
35943558 // @effect-diagnostics-next-line preferSchemaOverJson:off
35953559 JSON.stringify([
35963560 {
......@@ -3622,7 +3586,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
36223586 expect(result.pr.number).toBe(142);
36233587 expect(
36243588 ghCalls.some((call) =>
3625 call.includes("pr list --head octocat:statemachine --state open --limit 1"),
3589 call.includes("pr list --head statemachine --state open --limit 100"),
36263590 ),
36273591 ).toBe(true);
36283592 expect(ghCalls.some((call) => call.startsWith("pr create "))).toBe(false);
......@@ -3688,30 +3652,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
36883652 headRefName: "upstream/effect-atom",
36893653 },
36903654 ]),
3691 // @effect-diagnostics-next-line preferSchemaOverJson:off
3692 "pingdotgg:effect-atom": JSON.stringify([]),
3693 // @effect-diagnostics-next-line preferSchemaOverJson:off
3694 "my-org/upstream:effect-atom": JSON.stringify([]),
3695 // @effect-diagnostics-next-line preferSchemaOverJson:off
3696 "pingdotgg:upstream/effect-atom": JSON.stringify([
3697 {
3698 number: 1518,
3699 title: "Wrong PR",
3700 url: "https://github.com/pingdotgg/t3code/pull/1518",
3701 baseRefName: "main",
3702 headRefName: "upstream/effect-atom",
3703 },
3704 ]),
3705 // @effect-diagnostics-next-line preferSchemaOverJson:off
3706 "my-org/upstream:upstream/effect-atom": JSON.stringify([
3707 {
3708 number: 1518,
3709 title: "Wrong PR",
3710 url: "https://github.com/pingdotgg/t3code/pull/1518",
3711 baseRefName: "main",
3712 headRefName: "upstream/effect-atom",
3713 },
3714 ]),
37153655 },
37163656 },
37173657 });
......@@ -3731,7 +3671,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
37313671 );
37323672
37333673 it.effect(
3734 "prefers owner-qualified selectors before bare branch names for cross-repo PRs",
3674 "picks the fork PR among same-named branches from other repositories",
37353675 () =>
37363676 Effect.gen(function* () {
37373677 const repoDir = yield* makeTempDir("t3code-git-manager-");
......@@ -3763,9 +3703,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
37633703 baseRefName: "main",
37643704 headRefName: "statemachine",
37653705 },
3766 ]),
3767 // @effect-diagnostics-next-line preferSchemaOverJson:off
3768 "octocat:statemachine": JSON.stringify([
37693706 {
37703707 number: 142,
37713708 title: "Existing fork PR",
......@@ -3782,8 +3719,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
37823719 },
37833720 },
37843721 ]),
3785 // @effect-diagnostics-next-line preferSchemaOverJson:off
3786 "fork-seed:statemachine": JSON.stringify([]),
37873722 },
37883723 },
37893724 });
......@@ -3795,11 +3730,9 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
37953730
37963731 expect(result.pr.status).toBe("opened_existing");
37973732 expect(result.pr.number).toBe(142);
3798
3799 const ownerSelectorCallIndex = ghCalls.findIndex((call) =>
3800 call.includes("pr list --head octocat:statemachine --state open --limit 1"),
3801 );
3802 expect(ownerSelectorCallIndex).toBeGreaterThanOrEqual(0);
3733 expect(
3734 ghCalls.some((call) => /--head [^ ]*:/u.test(call) && call.startsWith("pr list")),
3735 ).toBe(false);
38033736 expect(ghCalls.some((call) => call.startsWith("pr create "))).toBe(false);
38043737 }),
38053738 12_000,
......@@ -3828,7 +3761,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
38283761 ghScenario: {
38293762 prListByHeadSelector: {
38303763 // @effect-diagnostics-next-line preferSchemaOverJson:off
3831 "octocat:statemachine": JSON.stringify([
3764 statemachine: JSON.stringify([
38323765 {
38333766 number: 142,
38343767 title: "Existing fork PR",
......@@ -3846,11 +3779,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
38463779 },
38473780 ]),
38483781 // @effect-diagnostics-next-line preferSchemaOverJson:off
3849 "fork-seed:statemachine": JSON.stringify([]),
3850 // @effect-diagnostics-next-line preferSchemaOverJson:off
38513782 "t3code/pr-142/statemachine": JSON.stringify([]),
3852 // @effect-diagnostics-next-line preferSchemaOverJson:off
3853 statemachine: JSON.stringify([]),
38543783 },
38553784 },
38563785 });
......@@ -3863,10 +3792,10 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
38633792 expect(result.pr.status).toBe("opened_existing");
38643793 expect(result.pr.number).toBe(142);
38653794
3866 const openLookupCalls = ghCalls.filter((call) => call.includes("--state open --limit 1"));
3795 const openLookupCalls = ghCalls.filter((call) => call.includes("--state open --limit 100"));
38673796 expect(openLookupCalls).toHaveLength(1);
38683797 expect(openLookupCalls[0]).toContain(
3869 "pr list --head octocat:statemachine --state open --limit 1",
3798 "pr list --head statemachine --state open --limit 100",
38703799 );
38713800 }),
38723801 12_000,
......@@ -3891,12 +3820,10 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
38913820 const { manager, ghCalls } = yield* makeManager({
38923821 ghScenario: {
38933822 prListSequenceByHeadSelector: {
3894 "octocat:statemachine": [
3823 statemachine: [
38953824 `[{"number":41,"title":"Ambiguous fork PR","url":"https://github.com/pingdotgg/codething-mvp/pull/41","baseRefName":"main","headRefName":"statemachine","state":"OPEN"}]`,
38963825 `[{"number":142,"title":"Add stacked git actions","url":"https://github.com/pingdotgg/codething-mvp/pull/142","baseRefName":"main","headRefName":"statemachine","state":"OPEN","isCrossRepository":true,"headRepository":{"nameWithOwner":"octocat/codething-mvp"},"headRepositoryOwner":{"login":"octocat"}}]`,
38973826 ],
3898 "fork-seed:statemachine": ["[]"],
3899 statemachine: ["[]"],
39003827 },
39013828 },
39023829 });
......@@ -4295,7 +4222,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
42954222 const { manager, ghCalls } = yield* makeManager({
42964223 ghScenario: {
42974224 prListSequenceByHeadSelector: {
4298 "octocat:statemachine": [
4225 statemachine: [
42994226 // @effect-diagnostics-next-line preferSchemaOverJson:off
43004227 JSON.stringify([]),
43014228 // @effect-diagnostics-next-line preferSchemaOverJson:off
......@@ -4317,10 +4244,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
43174244 },
43184245 ]),
43194246 ],
4320 // @effect-diagnostics-next-line preferSchemaOverJson:off
4321 "fork-seed:statemachine": [JSON.stringify([])],
4322 // @effect-diagnostics-next-line preferSchemaOverJson:off
4323 statemachine: [JSON.stringify([])],
43244247 },
43254248 },
43264249 });
apps/server/src/git/GitManager.ts+30-6
......@@ -32,6 +32,7 @@ import {
3232 ModelSelection,
3333 type ProjectId,
3434 SourceControlProviderError,
35 type SourceControlProviderKind,
3536 type SourceControlWritingStyleSettings,
3637 type ThreadId,
3738} from "@t3tools/contracts";
......@@ -594,6 +595,26 @@ function parseCustomCommitMessage(raw: string): { subject: string; body: string
594595 };
595596}
596597
598// Without the owner selector, a bare branch name also lists same-named
599// branches on other forks (`main`, `patch-1`), so GitHub probes ask for a full
600// page and let matchesBranchHeadContext pick the right head. gh fetches up to
601// 100 in one request, and GitHub prices a first:100 connection like first:1.
602const GITHUB_HEAD_BRANCH_PROBE_LIMIT = 100;
603
604// `gh pr list --head` filters on the head ref name alone and accepts anything, so an
605// `owner:branch` or `remote:branch` selector silently lists zero pull requests
606// while spending a GraphQL call. Git branch names cannot contain ":", and the
607// bare head branch is always among the selectors, so GitHub probes skip them and
608// leave the owner check to matchesBranchHeadContext.
609function probeableHeadSelectors(
610 providerKind: SourceControlProviderKind,
611 headSelectors: ReadonlyArray<string>,
612): ReadonlyArray<string> {
613 return providerKind === "github"
614 ? headSelectors.filter((selector) => !selector.includes(":"))
615 : headSelectors;
616}
617
597618function appendUnique(values: string[], next: string | null | undefined): void {
598619 const trimmed = next?.trim() ?? "";
599620 if (trimmed.length === 0 || values.includes(trimmed)) {
......@@ -1604,12 +1625,14 @@ export const make = Effect.gen(function* () {
16041625 | "isCrossRepository"
16051626 >,
16061627 ) {
1607 for (const headSelector of headContext.headSelectors) {
1608 const pullRequests = yield* (yield* sourceControlProvider(cwd)).listChangeRequests({
1628 const provider = yield* sourceControlProvider(cwd);
1629 const headSelectors = probeableHeadSelectors(provider.kind, headContext.headSelectors);
1630 for (const headSelector of headSelectors) {
1631 const pullRequests = yield* provider.listChangeRequests({
16091632 cwd,
16101633 headSelector,
16111634 state: "open",
1612 limit: 1,
1635 limit: provider.kind === "github" ? GITHUB_HEAD_BRANCH_PROBE_LIMIT : 1,
16131636 });
16141637 const normalizedPullRequests = pullRequests.map(toPullRequestInfo);
16151638
......@@ -1634,12 +1657,13 @@ export const make = Effect.gen(function* () {
16341657 ) {
16351658 const parsedByNumber = new Map<number, PullRequestInfo>();
16361659
1637 for (const headSelector of headContext.headSelectors) {
1638 const pullRequests = yield* (yield* sourceControlProvider(cwd)).listChangeRequests({
1660 const provider = yield* sourceControlProvider(cwd);
1661 for (const headSelector of probeableHeadSelectors(provider.kind, headContext.headSelectors)) {
1662 const pullRequests = yield* provider.listChangeRequests({
16391663 cwd,
16401664 headSelector,
16411665 state: "all",
1642 limit: 20,
1666 limit: provider.kind === "github" ? GITHUB_HEAD_BRANCH_PROBE_LIMIT : 20,
16431667 });
16441668
16451669 for (const pr of pullRequests.map(toPullRequestInfo)) {