| author |  Gianmarco <gianmarcosimone89@gmail.com> 2026-08-18 23:07:58+02:00 |
| committer |  GitHub <noreply@github.com> 2026-08-18 17:07:58-04:00 |
| log | 636caf4c70345d74c5391cbf9a607a91426a6e39 |
| tree | a0747cece953e6ba982bb93d38c6b35a8ffb9e69 |
| parent | 26af903b9bec7f8da56bb5f545d9980d08d418e1 |
| signature | Signed by PGP key B5690EEEBB952194 |
fix(server): avoid PRs inherited from default upstreams (#7317)
4 files changed, 146 insertions(+), 6 deletions(-)
apps
apps/server/src/git/GitManager.test.ts+102
| ... | @@ -1129,6 +1129,71 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { | ... | @@ -1129,6 +1129,71 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { |
| 1129 | 20_000, | 1129 | 20_000, |
| 1130 | ); | 1130 | ); |
| 1131 | 1131 | ||
| 1132 | it.effect( | ||
| 1133 | "status preserves a fork PR whose head is named after the default branch", | ||
| 1134 | () => | ||
| 1135 | Effect.gen(function* () { | ||
| 1136 | const repoDir = yield* makeTempDir("t3code-git-manager-"); | ||
| 1137 | yield* initRepo(repoDir); | ||
| 1138 | const originDir = yield* createBareRemote(); | ||
| 1139 | const forkDir = yield* createBareRemote(); | ||
| 1140 | yield* runGit(repoDir, ["remote", "add", "origin", originDir]); | ||
| 1141 | yield* runGit(repoDir, ["push", "-u", "origin", "main"]); | ||
| 1142 | yield* runGit(repoDir, ["remote", "set-head", "origin", "main"]); | ||
| 1143 | yield* runGit(repoDir, ["remote", "add", "fork-seed", forkDir]); | ||
| 1144 | yield* runGit(repoDir, ["push", "fork-seed", "main"]); | ||
| 1145 | yield* runGit(repoDir, ["checkout", "-b", "t3code/pr-777/main"]); | ||
| 1146 | yield* runGit(repoDir, ["branch", "--set-upstream-to", "fork-seed/main"]); | ||
| 1147 | yield* configureVisibleRemoteUrlWithLocalRewrite( | ||
| 1148 | repoDir, | ||
| 1149 | "fork-seed", | ||
| 1150 | "git@github.com:contributor/codething-mvp.git", | ||
| 1151 | forkDir, | ||
| 1152 | ); | ||
| 1153 | |||
| 1154 | const { manager, ghCalls } = yield* makeManager({ | ||
| 1155 | ghScenario: { | ||
| 1156 | prListByHeadSelector: { | ||
| 1157 | // @effect-diagnostics-next-line preferSchemaOverJson:off | ||
| 1158 | "contributor:main": JSON.stringify([ | ||
| 1159 | { | ||
| 1160 | number: 777, | ||
| 1161 | title: "Fork PR from main", | ||
| 1162 | url: "https://github.com/pingdotgg/codething-mvp/pull/777", | ||
| 1163 | baseRefName: "main", | ||
| 1164 | headRefName: "main", | ||
| 1165 | state: "OPEN", | ||
| 1166 | updatedAt: "2026-03-10T07:00:00Z", | ||
| 1167 | isCrossRepository: true, | ||
| 1168 | headRepository: { | ||
| 1169 | nameWithOwner: "contributor/codething-mvp", | ||
| 1170 | }, | ||
| 1171 | headRepositoryOwner: { | ||
| 1172 | login: "contributor", | ||
| 1173 | }, | ||
| 1174 | }, | ||
| 1175 | ]), | ||
| 1176 | }, | ||
| 1177 | }, | ||
| 1178 | }); | ||
| 1179 | |||
| 1180 | const status = yield* manager.status({ cwd: repoDir }); | ||
| 1181 | expect(status.refName).toBe("t3code/pr-777/main"); | ||
| 1182 | expect(status.pr).toEqual({ | ||
| 1183 | number: 777, | ||
| 1184 | title: "Fork PR from main", | ||
| 1185 | url: "https://github.com/pingdotgg/codething-mvp/pull/777", | ||
| 1186 | baseRef: "main", | ||
| 1187 | headRef: "main", | ||
| 1188 | state: "open", | ||
| 1189 | }); | ||
| 1190 | expect(ghCalls).toContain( | ||
| 1191 | "pr list --head contributor:main --state all --limit 20 --json number,title,url,baseRefName,headRefName,state,mergedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", | ||
| 1192 | ); | ||
| 1193 | }), | ||
| 1194 | 20_000, | ||
| 1195 | ); | ||
| 1196 | |||
| 1132 | it.effect( | 1197 | it.effect( |
| 1133 | "status ignores synthetic local branch aliases when the upstream remote name contains slashes", | 1198 | "status ignores synthetic local branch aliases when the upstream remote name contains slashes", |
| 1134 | () => | 1199 | () => |
| ... | @@ -1315,6 +1380,43 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { | ... | @@ -1315,6 +1380,43 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { |
| 1315 | }), | 1380 | }), |
| 1316 | ); | 1381 | ); |
| 1317 | 1382 | ||
| 1383 | it.effect("status does not inherit a merged PR from a feature branch's default upstream", () => | ||
| 1384 | Effect.gen(function* () { | ||
| 1385 | const repoDir = yield* makeTempDir("t3code-git-manager-"); | ||
| 1386 | yield* initRepo(repoDir); | ||
| 1387 | const remoteDir = yield* createBareRemote(); | ||
| 1388 | yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); | ||
| 1389 | yield* runGit(repoDir, ["push", "-u", "origin", "main"]); | ||
| 1390 | yield* runGit(repoDir, ["remote", "set-head", "origin", "main"]); | ||
| 1391 | yield* runGit(repoDir, ["checkout", "-b", "feature/from-main", "origin/main"]); | ||
| 1392 | |||
| 1393 | const { manager, ghCalls } = yield* makeManager({ | ||
| 1394 | ghScenario: { | ||
| 1395 | prListSequence: [ | ||
| 1396 | // @effect-diagnostics-next-line preferSchemaOverJson:off | ||
| 1397 | JSON.stringify([ | ||
| 1398 | { | ||
| 1399 | number: 54, | ||
| 1400 | title: "Reverse merge from main", | ||
| 1401 | url: "https://github.com/pingdotgg/codething-mvp/pull/54", | ||
| 1402 | baseRefName: "je-filter-list", | ||
| 1403 | headRefName: "main", | ||
| 1404 | state: "MERGED", | ||
| 1405 | mergedAt: "2023-09-28T03:21:10Z", | ||
| 1406 | updatedAt: "2023-09-28T03:21:10Z", | ||
| 1407 | }, | ||
| 1408 | ]), | ||
| 1409 | ], | ||
| 1410 | }, | ||
| 1411 | }); | ||
| 1412 | |||
| 1413 | const status = yield* manager.status({ cwd: repoDir }); | ||
| 1414 | expect(status.refName).toBe("feature/from-main"); | ||
| 1415 | expect(status.pr).toBeNull(); | ||
| 1416 | expect(ghCalls.some((call) => call.includes("pr list"))).toBe(false); | ||
| 1417 | }), | ||
| 1418 | ); | ||
| 1419 | |||
| 1318 | it.effect("status prefers open PR when merged PR has newer updatedAt", () => | 1420 | it.effect("status prefers open PR when merged PR has newer updatedAt", () => |
| 1319 | Effect.gen(function* () { | 1421 | Effect.gen(function* () { |
| 1320 | const repoDir = yield* makeTempDir("t3code-git-manager-"); | 1422 | const repoDir = yield* makeTempDir("t3code-git-manager-"); |
apps/server/src/git/GitManager.ts+41-6
| ... | @@ -904,11 +904,24 @@ export const make = Effect.gen(function* () { | ... | @@ -904,11 +904,24 @@ export const make = Effect.gen(function* () { |
| 904 | prLookupEpochByCwd.set(cacheKey, prLookupEpoch(cacheKey) + 1); | 904 | prLookupEpochByCwd.set(cacheKey, prLookupEpoch(cacheKey) + 1); |
| 905 | }), | 905 | }), |
| 906 | ); | 906 | ); |
| 907 | // Cache keys are NUL-joined [cwd, branch, upstreamRef, epoch] — none of the | 907 | // Cache keys are NUL-joined [cwd, branch, upstreamRef, defaultBranch, epoch] — none of the |
| 908 | // segments can contain a NUL byte, and refs are never empty, so "" decodes | 908 | // segments can contain a NUL byte, and refs are never empty, so "" decodes |
| 909 | // back to a null upstreamRef. | 909 | // back to a null ref. |
| 910 | const prLookupCacheKey = (cwd: string, details: { branch: string; upstreamRef: string | null }) => | 910 | const prLookupCacheKey = ( |
| 911 | [cwd, details.branch, details.upstreamRef ?? "", String(prLookupEpoch(cwd))].join("\u0000"); | 911 | cwd: string, |
| 912 | details: { | ||
| 913 | branch: string; | ||
| 914 | upstreamRef: string | null; | ||
| 915 | defaultBranch: string | null; | ||
| 916 | }, | ||
| 917 | ) => | ||
| 918 | [ | ||
| 919 | cwd, | ||
| 920 | details.branch, | ||
| 921 | details.upstreamRef ?? "", | ||
| 922 | details.defaultBranch ?? "", | ||
| 923 | String(prLookupEpoch(cwd)), | ||
| 924 | ].join("\u0000"); | ||
| 912 | // Consecutive failures per cache key, so a branch that keeps failing waits | 925 | // Consecutive failures per cache key, so a branch that keeps failing waits |
| 913 | // longer before the next attempt. Cleared as soon as a lookup succeeds. | 926 | // longer before the next attempt. Cleared as soon as a lookup succeeds. |
| 914 | const prLookupFailureStreakByKey = new Map<string, number>(); | 927 | const prLookupFailureStreakByKey = new Map<string, number>(); |
| ... | @@ -928,13 +941,29 @@ export const make = Effect.gen(function* () { | ... | @@ -928,13 +941,29 @@ export const make = Effect.gen(function* () { |
| 928 | }; | 941 | }; |
| 929 | const prLookupCache = yield* Cache.makeWith( | 942 | const prLookupCache = yield* Cache.makeWith( |
| 930 | (key: string) => { | 943 | (key: string) => { |
| 931 | const [cwd = "", branch = "", upstreamRef = ""] = key.split("\u0000"); | 944 | const [cwd = "", branch = "", upstreamRef = "", defaultBranch = ""] = key.split("\u0000"); |
| 932 | const details = { | 945 | const details = { |
| 933 | branch, | 946 | branch, |
| 934 | upstreamRef: upstreamRef.length > 0 ? upstreamRef : null, | 947 | upstreamRef: upstreamRef.length > 0 ? upstreamRef : null, |
| 948 | defaultBranch: defaultBranch.length > 0 ? defaultBranch : null, | ||
| 935 | }; | 949 | }; |
| 936 | return Effect.gen(function* () { | 950 | return Effect.gen(function* () { |
| 937 | const headContext = yield* resolveBranchHeadContext(cwd, details); | 951 | const headContext = yield* resolveBranchHeadContext(cwd, details); |
| 952 | const upstreamHeadIsDefault = | ||
| 953 | headContext.headBranch === details.defaultBranch || | ||
| 954 | (details.defaultBranch === null && | ||
| 955 | (headContext.headBranch === "main" || headContext.headBranch === "master")); | ||
| 956 | // `git worktree add -b feature origin/main` makes the new local branch | ||
| 957 | // track origin/main. That upstream is the branch's base, not its | ||
| 958 | // published PR head. Looking up PRs for it can attach an old reverse | ||
| 959 | // merge from main and auto-settle an unrelated feature thread. | ||
| 960 | if ( | ||
| 961 | headContext.headBranch !== details.branch && | ||
| 962 | upstreamHeadIsDefault && | ||
| 963 | !headContext.isCrossRepository | ||
| 964 | ) { | ||
| 965 | return { latest: null, headContext }; | ||
| 966 | } | ||
| 938 | // Only skip when the branch is untracked as well: anything carrying an | 967 | // Only skip when the branch is untracked as well: anything carrying an |
| 939 | // upstream keeps the old behaviour. | 968 | // upstream keeps the old behaviour. |
| 940 | if (details.upstreamRef === null && (yield* isUnpublishedBranch(cwd, headContext))) { | 969 | if (details.upstreamRef === null && (yield* isUnpublishedBranch(cwd, headContext))) { |
| ... | @@ -1015,7 +1044,12 @@ export const make = Effect.gen(function* () { | ... | @@ -1015,7 +1044,12 @@ export const make = Effect.gen(function* () { |
| 1015 | }; | 1044 | }; |
| 1016 | const lookupStatusPr = Effect.fn("lookupStatusPr")(function* ( | 1045 | const lookupStatusPr = Effect.fn("lookupStatusPr")(function* ( |
| 1017 | cwd: string, | 1046 | cwd: string, |
| 1018 | details: { branch: string; upstreamRef: string | null; isDefaultBranch: boolean }, | 1047 | details: { |
| 1048 | branch: string; | ||
| 1049 | upstreamRef: string | null; | ||
| 1050 | defaultBranch: string | null; | ||
| 1051 | isDefaultBranch: boolean; | ||
| 1052 | }, | ||
| 1019 | ) { | 1053 | ) { |
| 1020 | // Keyed by (cwd, branch) only: the upstream ref changing (e.g. a first | 1054 | // Keyed by (cwd, branch) only: the upstream ref changing (e.g. a first |
| 1021 | // `push -u`) must not orphan the fallback value for the same branch. | 1055 | // `push -u`) must not orphan the fallback value for the same branch. |
| ... | @@ -1089,6 +1123,7 @@ export const make = Effect.gen(function* () { | ... | @@ -1089,6 +1123,7 @@ export const make = Effect.gen(function* () { |
| 1089 | ? yield* lookupStatusPr(cwd, { | 1123 | ? yield* lookupStatusPr(cwd, { |
| 1090 | branch: details.branch, | 1124 | branch: details.branch, |
| 1091 | upstreamRef: details.upstreamRef, | 1125 | upstreamRef: details.upstreamRef, |
| 1126 | defaultBranch: details.defaultBranch, | ||
| 1092 | isDefaultBranch: details.isDefaultBranch, | 1127 | isDefaultBranch: details.isDefaultBranch, |
| 1093 | }) | 1128 | }) |
| 1094 | : null; | 1129 | : null; |
apps/server/src/vcs/GitVcsDriver.ts+1
| ... | @@ -72,6 +72,7 @@ export interface GitStatusDetails { | ... | @@ -72,6 +72,7 @@ export interface GitStatusDetails { |
| 72 | 72 | ||
| 73 | export interface GitRemoteStatusDetails { | 73 | export interface GitRemoteStatusDetails { |
| 74 | isRepo: boolean; | 74 | isRepo: boolean; |
| 75 | defaultBranch: string | null; | ||
| 75 | isDefaultBranch: boolean; | 76 | isDefaultBranch: boolean; |
| 76 | branch: string | null; | 77 | branch: string | null; |
| 77 | upstreamRef: string | null; | 78 | upstreamRef: string | null; |
apps/server/src/vcs/GitVcsDriverCore.ts+2
| ... | @@ -89,6 +89,7 @@ const NON_REPOSITORY_STATUS_DETAILS = Object.freeze<GitVcsDriver.GitStatusDetail | ... | @@ -89,6 +89,7 @@ const NON_REPOSITORY_STATUS_DETAILS = Object.freeze<GitVcsDriver.GitStatusDetail |
| 89 | }); | 89 | }); |
| 90 | const NON_REPOSITORY_REMOTE_STATUS_DETAILS = Object.freeze<GitVcsDriver.GitRemoteStatusDetails>({ | 90 | const NON_REPOSITORY_REMOTE_STATUS_DETAILS = Object.freeze<GitVcsDriver.GitRemoteStatusDetails>({ |
| 91 | isRepo: false, | 91 | isRepo: false, |
| 92 | defaultBranch: null, | ||
| 92 | isDefaultBranch: false, | 93 | isDefaultBranch: false, |
| 93 | branch: null, | 94 | branch: null, |
| 94 | upstreamRef: null, | 95 | upstreamRef: null, |
| ... | @@ -1550,6 +1551,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* | ... | @@ -1550,6 +1551,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* |
| 1550 | 1551 | ||
| 1551 | return { | 1552 | return { |
| 1552 | isRepo: true, | 1553 | isRepo: true, |
| 1554 | defaultBranch, | ||
| 1553 | isDefaultBranch, | 1555 | isDefaultBranch, |
| 1554 | branch, | 1556 | branch, |
| 1555 | upstreamRef, | 1557 | upstreamRef, |