Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
juliusmarminge merged 2 commits into from
Sep 23, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f3c140
fix(server): GitHub PR lookups stop probing owner-qualified heads
juliusmarminge Sep 23, 2026
36211c8
fix(server): GitHub head lookups read a full page of same-named branches
juliusmarminge Sep 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 21 additions & 98 deletions apps/server/src/git/GitManager.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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);
}),
);

Expand Down Expand Up @@ -1776,8 +1777,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
const { manager } = yield* makeManager({
ghScenario: {
prListByHeadSelector: {
"alice:feature": output,
"fork:feature": output,
feature: output,
},
},
Expand Down Expand Up @@ -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([
{
Expand Down Expand Up @@ -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 100 --json number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
);
}),
20_000,
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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 100 --json number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner",
);
}),
20_000,
Expand Down Expand Up @@ -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",
},
]),
},
},
});
Expand Down Expand Up @@ -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",
Expand All @@ -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);
}),
);
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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-");
Expand All @@ -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([
{
Expand Down Expand Up @@ -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 100"),
),
).toBe(true);
expect(ghCalls.some((call) => call.startsWith("pr create "))).toBe(false);
Expand Down Expand Up @@ -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",
},
]),
},
},
});
Expand All @@ -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-");
Expand Down Expand Up @@ -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",
Expand All @@ -3782,8 +3719,6 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
},
},
]),
// @effect-diagnostics-next-line preferSchemaOverJson:off
"fork-seed:statemachine": JSON.stringify([]),
},
},
});
Expand All @@ -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,
Expand Down Expand Up @@ -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",
Expand All @@ -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([]),
},
},
});
Expand All @@ -3863,10 +3792,10 @@ 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 100"));
expect(openLookupCalls).toHaveLength(1);
expect(openLookupCalls[0]).toContain(
"pr list --head octocat:statemachine --state open --limit 1",
"pr list --head statemachine --state open --limit 100",
);
}),
12_000,
Expand All @@ -3891,12 +3820,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: ["[]"],
},
},
});
Expand Down Expand Up @@ -4295,7 +4222,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
Expand All @@ -4317,10 +4244,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([])],
},
},
});
Expand Down
36 changes: 30 additions & 6 deletions apps/server/src/git/GitManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
ModelSelection,
type ProjectId,
SourceControlProviderError,
type SourceControlProviderKind,
type SourceControlWritingStyleSettings,
type ThreadId,
} from "@t3tools/contracts";
Expand Down Expand Up @@ -594,6 +595,26 @@ function parseCustomCommitMessage(raw: string): { subject: string; body: string
};
}

// 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
// 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<string>,
): ReadonlyArray<string> {
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)) {
Expand Down Expand Up @@ -1604,12 +1625,14 @@ 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,
limit: provider.kind === "github" ? GITHUB_HEAD_BRANCH_PROBE_LIMIT : 1,
});
const normalizedPullRequests = pullRequests.map(toPullRequestInfo);

Expand All @@ -1634,12 +1657,13 @@ export const make = Effect.gen(function* () {
) {
const parsedByNumber = new Map<number, PullRequestInfo>();

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",
limit: 20,
limit: provider.kind === "github" ? GITHUB_HEAD_BRANCH_PROBE_LIMIT : 20,
});

for (const pr of pullRequests.map(toPullRequestInfo)) {
Expand Down
Loading