From 1f3c140899a1193c7a13285ed3c448c1544f49da Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 22 Sep 2026 20:20:41 -0700 Subject: [PATCH 1/2] fix(server): GitHub PR lookups stop probing owner-qualified heads `gh pr list --head` filters on the head ref name alone, so `owner:branch` and `remote:branch` selectors always list nothing while spending a GraphQL call. Fork lookups on GitHub now query the bare branch and let matchesBranchHeadContext pick the right fork. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/git/GitManager.test.ts | 121 +++++-------------------- apps/server/src/git/GitManager.ts | 32 ++++++- 2 files changed, 48 insertions(+), 105 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index a502506ef9f3..91c88f3bf69b 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -1309,7 +1309,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { prListByHeadSelector: { // Fake gh returns raw JSON stdout, matching the CLI boundary under test. // @effect-diagnostics-next-line preferSchemaOverJson:off - "contributor:feature/deleted-fork-branch": JSON.stringify([ + "feature/deleted-fork-branch": JSON.stringify([ { number: 218, title: "Deleted fork branch PR", @@ -1339,8 +1339,9 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { updatedAt: "2026-04-05T15:00:00.000Z", }); expect( - ghCalls.some((call) => call.includes("--head contributor:feature/deleted-fork-branch")), + ghCalls.some((call) => call.includes("--head feature/deleted-fork-branch --state all")), ).toBe(true); + expect(ghCalls.some((call) => call.includes("--head contributor:"))).toBe(false); }), ); @@ -1776,8 +1777,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { const { manager } = yield* makeManager({ ghScenario: { prListByHeadSelector: { - "alice:feature": output, - "fork:feature": output, feature: output, }, }, @@ -1944,10 +1943,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { const { manager, ghCalls } = yield* makeManager({ ghScenario: { prListSequence: [ - // @effect-diagnostics-next-line preferSchemaOverJson:off - JSON.stringify([]), - // @effect-diagnostics-next-line preferSchemaOverJson:off - JSON.stringify([]), // @effect-diagnostics-next-line preferSchemaOverJson:off JSON.stringify([ { @@ -1983,7 +1978,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { updatedAt: "2026-03-10T07:00:00.000Z", }); expect(ghCalls).toContain( - "pr list --head jasonLaster:statemachine --state all --limit 20 --json number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", + "pr list --head statemachine --state all --limit 20 --json number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", ); }), 20_000, @@ -2015,7 +2010,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { ghScenario: { prListByHeadSelector: { // @effect-diagnostics-next-line preferSchemaOverJson:off - "contributor:main": JSON.stringify([ + main: JSON.stringify([ { number: 777, title: "Fork PR from main", @@ -2049,7 +2044,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { updatedAt: "2026-03-10T07:00:00.000Z", }); expect(ghCalls).toContain( - "pr list --head contributor:main --state all --limit 20 --json number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", + "pr list --head main --state all --limit 20 --json number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", ); }), 20_000, @@ -2114,34 +2109,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { updatedAt: "2026-04-01T10:00:00Z", }, ]), - // @effect-diagnostics-next-line preferSchemaOverJson:off - "pingdotgg:effect-atom": JSON.stringify([]), - // @effect-diagnostics-next-line preferSchemaOverJson:off - "my-org/upstream:effect-atom": JSON.stringify([]), - // @effect-diagnostics-next-line preferSchemaOverJson:off - "pingdotgg:upstream/effect-atom": JSON.stringify([ - { - number: 1518, - title: "Wrong PR", - url: "https://github.com/pingdotgg/t3code/pull/1518", - baseRefName: "main", - headRefName: "upstream/effect-atom", - state: "OPEN", - updatedAt: "2026-04-01T10:00:00Z", - }, - ]), - // @effect-diagnostics-next-line preferSchemaOverJson:off - "my-org/upstream:upstream/effect-atom": JSON.stringify([ - { - number: 1518, - title: "Wrong PR", - url: "https://github.com/pingdotgg/t3code/pull/1518", - baseRefName: "main", - headRefName: "upstream/effect-atom", - state: "OPEN", - updatedAt: "2026-04-01T10:00:00Z", - }, - ]), }, }, }); @@ -2352,7 +2319,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { prListByHeadSelector: { // Fake gh returns raw JSON stdout, matching the CLI boundary under test. // @effect-diagnostics-next-line preferSchemaOverJson:off - "contributor:feature/fork-plain": JSON.stringify([ + "feature/fork-plain": JSON.stringify([ { number: 89, title: "Fork PR pushed without -u", @@ -2372,9 +2339,8 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { const status = yield* manager.status({ cwd: repoDir }); expect(status.pr?.number).toBe(89); - expect(ghCalls.some((call) => call.includes("--head contributor:feature/fork-plain"))).toBe( - true, - ); + expect(ghCalls.some((call) => call.includes("--head feature/fork-plain"))).toBe(true); + expect(ghCalls.some((call) => call.includes("--head contributor:"))).toBe(false); expect(ghCalls.some((call) => call.includes("--head main"))).toBe(false); }), ); @@ -2409,7 +2375,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { prListByHeadSelector: { // Fake gh returns raw JSON stdout, matching the CLI boundary under test. // @effect-diagnostics-next-line preferSchemaOverJson:off - "contributor:feature/fork-settle": JSON.stringify([ + "feature/fork-settle": JSON.stringify([ { number: 91, title: "Fork PR to settle", @@ -3570,7 +3536,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { ); it.effect( - "returns existing cross-repo PR metadata using the fork owner selector", + "returns existing cross-repo PR metadata found under the bare branch name", () => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); @@ -3589,8 +3555,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { const { manager, ghCalls } = yield* makeManager({ ghScenario: { prListSequence: [ - // @effect-diagnostics-next-line preferSchemaOverJson:off - JSON.stringify([]), // @effect-diagnostics-next-line preferSchemaOverJson:off JSON.stringify([ { @@ -3622,7 +3586,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { expect(result.pr.number).toBe(142); expect( ghCalls.some((call) => - call.includes("pr list --head octocat:statemachine --state open --limit 1"), + call.includes("pr list --head statemachine --state open --limit 10"), ), ).toBe(true); expect(ghCalls.some((call) => call.startsWith("pr create "))).toBe(false); @@ -3688,30 +3652,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { headRefName: "upstream/effect-atom", }, ]), - // @effect-diagnostics-next-line preferSchemaOverJson:off - "pingdotgg:effect-atom": JSON.stringify([]), - // @effect-diagnostics-next-line preferSchemaOverJson:off - "my-org/upstream:effect-atom": JSON.stringify([]), - // @effect-diagnostics-next-line preferSchemaOverJson:off - "pingdotgg:upstream/effect-atom": JSON.stringify([ - { - number: 1518, - title: "Wrong PR", - url: "https://github.com/pingdotgg/t3code/pull/1518", - baseRefName: "main", - headRefName: "upstream/effect-atom", - }, - ]), - // @effect-diagnostics-next-line preferSchemaOverJson:off - "my-org/upstream:upstream/effect-atom": JSON.stringify([ - { - number: 1518, - title: "Wrong PR", - url: "https://github.com/pingdotgg/t3code/pull/1518", - baseRefName: "main", - headRefName: "upstream/effect-atom", - }, - ]), }, }, }); @@ -3731,7 +3671,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { ); it.effect( - "prefers owner-qualified selectors before bare branch names for cross-repo PRs", + "picks the fork PR among same-named branches from other repositories", () => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); @@ -3763,9 +3703,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { baseRefName: "main", headRefName: "statemachine", }, - ]), - // @effect-diagnostics-next-line preferSchemaOverJson:off - "octocat:statemachine": JSON.stringify([ { number: 142, title: "Existing fork PR", @@ -3782,8 +3719,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { }, }, ]), - // @effect-diagnostics-next-line preferSchemaOverJson:off - "fork-seed:statemachine": JSON.stringify([]), }, }, }); @@ -3795,11 +3730,9 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { expect(result.pr.status).toBe("opened_existing"); expect(result.pr.number).toBe(142); - - const ownerSelectorCallIndex = ghCalls.findIndex((call) => - call.includes("pr list --head octocat:statemachine --state open --limit 1"), - ); - expect(ownerSelectorCallIndex).toBeGreaterThanOrEqual(0); + expect( + ghCalls.some((call) => /--head [^ ]*:/u.test(call) && call.startsWith("pr list")), + ).toBe(false); expect(ghCalls.some((call) => call.startsWith("pr create "))).toBe(false); }), 12_000, @@ -3828,7 +3761,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { ghScenario: { prListByHeadSelector: { // @effect-diagnostics-next-line preferSchemaOverJson:off - "octocat:statemachine": JSON.stringify([ + statemachine: JSON.stringify([ { number: 142, title: "Existing fork PR", @@ -3846,11 +3779,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { }, ]), // @effect-diagnostics-next-line preferSchemaOverJson:off - "fork-seed:statemachine": JSON.stringify([]), - // @effect-diagnostics-next-line preferSchemaOverJson:off "t3code/pr-142/statemachine": JSON.stringify([]), - // @effect-diagnostics-next-line preferSchemaOverJson:off - statemachine: JSON.stringify([]), }, }, }); @@ -3863,11 +3792,9 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { expect(result.pr.status).toBe("opened_existing"); expect(result.pr.number).toBe(142); - const openLookupCalls = ghCalls.filter((call) => call.includes("--state open --limit 1")); + const openLookupCalls = ghCalls.filter((call) => call.includes("--state open --limit 10")); expect(openLookupCalls).toHaveLength(1); - expect(openLookupCalls[0]).toContain( - "pr list --head octocat:statemachine --state open --limit 1", - ); + expect(openLookupCalls[0]).toContain("pr list --head statemachine --state open --limit 10"); }), 12_000, ); @@ -3891,12 +3818,10 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { const { manager, ghCalls } = yield* makeManager({ ghScenario: { prListSequenceByHeadSelector: { - "octocat:statemachine": [ + statemachine: [ `[{"number":41,"title":"Ambiguous fork PR","url":"https://github.com/pingdotgg/codething-mvp/pull/41","baseRefName":"main","headRefName":"statemachine","state":"OPEN"}]`, `[{"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"}}]`, ], - "fork-seed:statemachine": ["[]"], - statemachine: ["[]"], }, }, }); @@ -4295,7 +4220,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { const { manager, ghCalls } = yield* makeManager({ ghScenario: { prListSequenceByHeadSelector: { - "octocat:statemachine": [ + statemachine: [ // @effect-diagnostics-next-line preferSchemaOverJson:off JSON.stringify([]), // @effect-diagnostics-next-line preferSchemaOverJson:off @@ -4317,10 +4242,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { }, ]), ], - // @effect-diagnostics-next-line preferSchemaOverJson:off - "fork-seed:statemachine": [JSON.stringify([])], - // @effect-diagnostics-next-line preferSchemaOverJson:off - statemachine: [JSON.stringify([])], }, }, }); diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index dc2b8bcdec35..719a867b1c0f 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -32,6 +32,7 @@ import { ModelSelection, type ProjectId, SourceControlProviderError, + type SourceControlProviderKind, type SourceControlWritingStyleSettings, type ThreadId, } from "@t3tools/contracts"; @@ -594,6 +595,22 @@ function parseCustomCommitMessage(raw: string): { subject: string; body: string }; } +const OPEN_PR_PROBE_LIMIT = 10; + +// `gh pr list --head` filters on the head ref name alone and accepts anything, so an +// `owner:branch` or `remote:branch` selector silently lists zero pull requests +// while spending a GraphQL call. Git branch names cannot contain ":", and the +// bare head branch is always among the selectors, so GitHub probes skip them and +// leave the owner check to matchesBranchHeadContext. +function probeableHeadSelectors( + providerKind: SourceControlProviderKind, + headSelectors: ReadonlyArray, +): ReadonlyArray { + return providerKind === "github" + ? headSelectors.filter((selector) => !selector.includes(":")) + : headSelectors; +} + function appendUnique(values: string[], next: string | null | undefined): void { const trimmed = next?.trim() ?? ""; if (trimmed.length === 0 || values.includes(trimmed)) { @@ -1604,12 +1621,16 @@ export const make = Effect.gen(function* () { | "isCrossRepository" >, ) { - for (const headSelector of headContext.headSelectors) { - const pullRequests = yield* (yield* sourceControlProvider(cwd)).listChangeRequests({ + const provider = yield* sourceControlProvider(cwd); + const headSelectors = probeableHeadSelectors(provider.kind, headContext.headSelectors); + for (const headSelector of headSelectors) { + const pullRequests = yield* provider.listChangeRequests({ cwd, headSelector, state: "open", - limit: 1, + // A bare branch name also lists same-named branches on other forks, + // so leave room for the owner match below. + limit: OPEN_PR_PROBE_LIMIT, }); const normalizedPullRequests = pullRequests.map(toPullRequestInfo); @@ -1634,8 +1655,9 @@ export const make = Effect.gen(function* () { ) { const parsedByNumber = new Map(); - for (const headSelector of headContext.headSelectors) { - const pullRequests = yield* (yield* sourceControlProvider(cwd)).listChangeRequests({ + const provider = yield* sourceControlProvider(cwd); + for (const headSelector of probeableHeadSelectors(provider.kind, headContext.headSelectors)) { + const pullRequests = yield* provider.listChangeRequests({ cwd, headSelector, state: "all", From 36211c8ffc9416f36a42c7dca4d29157136d2d5c Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 22 Sep 2026 20:47:26 -0700 Subject: [PATCH 2/2] fix(server): GitHub head lookups read a full page of same-named branches A bare branch name also matches same-named branches on other forks, so a limit of 10 could miss the tracked fork's PR. GitHub probes now ask for 100 (one request, same GraphQL cost); other providers keep their limits. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/git/GitManager.test.ts | 12 +++++++----- apps/server/src/git/GitManager.ts | 12 +++++++----- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 91c88f3bf69b..6a1a8c1510fa 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -1978,7 +1978,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { updatedAt: "2026-03-10T07:00:00.000Z", }); expect(ghCalls).toContain( - "pr list --head statemachine --state all --limit 20 --json number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", + "pr list --head statemachine --state all --limit 100 --json number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", ); }), 20_000, @@ -2044,7 +2044,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { updatedAt: "2026-03-10T07:00:00.000Z", }); expect(ghCalls).toContain( - "pr list --head main --state all --limit 20 --json number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", + "pr list --head main --state all --limit 100 --json number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", ); }), 20_000, @@ -3586,7 +3586,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { expect(result.pr.number).toBe(142); expect( ghCalls.some((call) => - call.includes("pr list --head statemachine --state open --limit 10"), + call.includes("pr list --head statemachine --state open --limit 100"), ), ).toBe(true); expect(ghCalls.some((call) => call.startsWith("pr create "))).toBe(false); @@ -3792,9 +3792,11 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { expect(result.pr.status).toBe("opened_existing"); expect(result.pr.number).toBe(142); - const openLookupCalls = ghCalls.filter((call) => call.includes("--state open --limit 10")); + const openLookupCalls = ghCalls.filter((call) => call.includes("--state open --limit 100")); expect(openLookupCalls).toHaveLength(1); - expect(openLookupCalls[0]).toContain("pr list --head statemachine --state open --limit 10"); + expect(openLookupCalls[0]).toContain( + "pr list --head statemachine --state open --limit 100", + ); }), 12_000, ); diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index 719a867b1c0f..43d674662d61 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -595,7 +595,11 @@ function parseCustomCommitMessage(raw: string): { subject: string; body: string }; } -const OPEN_PR_PROBE_LIMIT = 10; +// Without the owner selector, a bare branch name also lists same-named +// branches on other forks (`main`, `patch-1`), so GitHub probes ask for a full +// page and let matchesBranchHeadContext pick the right head. gh fetches up to +// 100 in one request, and GitHub prices a first:100 connection like first:1. +const GITHUB_HEAD_BRANCH_PROBE_LIMIT = 100; // `gh pr list --head` filters on the head ref name alone and accepts anything, so an // `owner:branch` or `remote:branch` selector silently lists zero pull requests @@ -1628,9 +1632,7 @@ export const make = Effect.gen(function* () { cwd, headSelector, state: "open", - // A bare branch name also lists same-named branches on other forks, - // so leave room for the owner match below. - limit: OPEN_PR_PROBE_LIMIT, + limit: provider.kind === "github" ? GITHUB_HEAD_BRANCH_PROBE_LIMIT : 1, }); const normalizedPullRequests = pullRequests.map(toPullRequestInfo); @@ -1661,7 +1663,7 @@ export const make = Effect.gen(function* () { cwd, headSelector, state: "all", - limit: 20, + limit: provider.kind === "github" ? GITHUB_HEAD_BRANCH_PROBE_LIMIT : 20, }); for (const pr of pullRequests.map(toPullRequestInfo)) {