authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-07-22 17:49:46+02:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-07-22 17:49:46+02:00
log376c149eac201fd1492c7b7512a1de96c3b345b7
tree5f633841dc0ec7374a1fd3ecdb12cd1f5eeed7da
parent7e2bb475042ccfd196779831e69f5b19a38a2423
signature Signed by PGP key B5690EEEBB952194

Stabilize PR status lookups and provider session lifecycle (#4281)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

9 files changed, 465 insertions(+), 36 deletions(-)

apps/server/src/git/GitManager.test.ts+179-1
......@@ -50,6 +50,8 @@ interface FakeGhScenario {
5050 };
5151 repositoryCloneUrls?: Record<string, { url: string; sshUrl: string }>;
5252 failWith?: GitHubCli.GitHubCliError;
53 /** Let this many gh calls succeed before failWith kicks in (default 0 = fail immediately). */
54 failAfterCalls?: number;
5355}
5456
5557function fakeGhOutput(stdout: string): VcsProcess.VcsProcessOutput {
......@@ -382,7 +384,7 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): {
382384 const args = [...input.args];
383385 ghCalls.push(args.join(" "));
384386
385 if (scenario.failWith) {
387 if (scenario.failWith && ghCalls.length > (scenario.failAfterCalls ?? 0)) {
386388 return Effect.fail(scenario.failWith);
387389 }
388390
......@@ -1336,6 +1338,182 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
13361338 }),
13371339 );
13381340
1341 it.effect("status keeps the last known PR when a later lookup fails", () =>
1342 Effect.gen(function* () {
1343 const repoDir = yield* makeTempDir("t3code-git-manager-");
1344 yield* initRepo(repoDir);
1345 yield* runGit(repoDir, ["checkout", "-b", "feature/pr-sticky"]);
1346 const remoteDir = yield* createBareRemote();
1347 yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]);
1348 yield* runGit(repoDir, ["push", "-u", "origin", "feature/pr-sticky"]);
1349
1350 const existingPr = {
1351 number: 214,
1352 title: "Sticky PR",
1353 url: "https://github.com/pingdotgg/codething-mvp/pull/214",
1354 baseRefName: "main",
1355 headRefName: "feature/pr-sticky",
1356 };
1357 const { manager } = yield* makeManager({
1358 ghScenario: {
1359 // @effect-diagnostics-next-line preferSchemaOverJson:off
1360 prListSequence: [JSON.stringify([existingPr])],
1361 failWith: new GitHubCli.GitHubCliUnavailableError({
1362 command: "gh",
1363 cwd: repoDir,
1364 cause: new Error("rate limited"),
1365 }),
1366 failAfterCalls: 1,
1367 },
1368 });
1369
1370 const first = yield* manager.status({ cwd: repoDir });
1371 expect(first.pr?.number).toBe(214);
1372
1373 // An explicit invalidation (user refresh, git action) bypasses the PR
1374 // cache and forces a live lookup — which now fails. The badge must keep
1375 // the last known PR instead of blanking out.
1376 yield* manager.invalidateStatus(repoDir);
1377 const second = yield* manager.status({ cwd: repoDir });
1378 expect(second.pr?.number).toBe(214);
1379 }),
1380 );
1381
1382 it.effect(
1383 "status does not reuse a stale PR after the branch is retargeted to a different upstream",
1384 () =>
1385 Effect.gen(function* () {
1386 const repoDir = yield* makeTempDir("t3code-git-manager-");
1387 yield* initRepo(repoDir);
1388 yield* runGit(repoDir, ["checkout", "-b", "feature/pr-retarget"]);
1389
1390 const originRemote = yield* createBareRemote();
1391 yield* runGit(repoDir, ["remote", "add", "origin", originRemote]);
1392 yield* runGit(repoDir, ["push", "-u", "origin", "feature/pr-retarget"]);
1393
1394 const existingPr = {
1395 number: 214,
1396 title: "Sticky PR",
1397 url: "https://github.com/pingdotgg/codething-mvp/pull/214",
1398 baseRefName: "main",
1399 headRefName: "feature/pr-retarget",
1400 };
1401 const { manager } = yield* makeManager({
1402 ghScenario: {
1403 // @effect-diagnostics-next-line preferSchemaOverJson:off
1404 prListSequence: [JSON.stringify([existingPr])],
1405 failWith: new GitHubCli.GitHubCliUnavailableError({
1406 command: "gh",
1407 cwd: repoDir,
1408 cause: new Error("rate limited"),
1409 }),
1410 failAfterCalls: 1,
1411 },
1412 });
1413
1414 const first = yield* manager.status({ cwd: repoDir });
1415 expect(first.pr?.number).toBe(214);
1416
1417 // Retarget the branch to a different remote/upstream (e.g. the PR was
1418 // reopened against a fork). The previously cached PR belonged to the
1419 // old upstream and must not be shown against the new one.
1420 const forkRemote = yield* createBareRemote();
1421 yield* runGit(repoDir, ["remote", "add", "fork", forkRemote]);
1422 yield* runGit(repoDir, ["push", "fork", "feature/pr-retarget"]);
1423 yield* runGit(repoDir, [
1424 "branch",
1425 "--set-upstream-to=fork/feature/pr-retarget",
1426 "feature/pr-retarget",
1427 ]);
1428
1429 yield* manager.invalidateStatus(repoDir);
1430 const second = yield* manager.status({ cwd: repoDir });
1431 expect(second.pr).toBeNull();
1432 }),
1433 );
1434
1435 it.effect("status keeps the last known PR when the branch gains its first upstream", () =>
1436 Effect.gen(function* () {
1437 const repoDir = yield* makeTempDir("t3code-git-manager-");
1438 yield* initRepo(repoDir);
1439 yield* runGit(repoDir, ["checkout", "-b", "feature/pr-sticky-first-push"]);
1440 const remoteDir = yield* createBareRemote();
1441 yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]);
1442
1443 const existingPr = {
1444 number: 215,
1445 title: "Sticky first-push PR",
1446 url: "https://github.com/pingdotgg/codething-mvp/pull/215",
1447 baseRefName: "main",
1448 headRefName: "feature/pr-sticky-first-push",
1449 };
1450 const { manager } = yield* makeManager({
1451 ghScenario: {
1452 // @effect-diagnostics-next-line preferSchemaOverJson:off
1453 prListSequence: [JSON.stringify([existingPr])],
1454 failWith: new GitHubCli.GitHubCliUnavailableError({
1455 command: "gh",
1456 cwd: repoDir,
1457 cause: new Error("rate limited"),
1458 }),
1459 failAfterCalls: 1,
1460 },
1461 });
1462
1463 const first = yield* manager.status({ cwd: repoDir });
1464 expect(first.pr?.number).toBe(215);
1465
1466 yield* runGit(repoDir, ["push", "-u", "origin", "feature/pr-sticky-first-push"]);
1467 yield* manager.invalidateStatus(repoDir);
1468
1469 const second = yield* manager.status({ cwd: repoDir });
1470 expect(second.pr?.number).toBe(215);
1471 }),
1472 );
1473
1474 it.effect("status drops the last known PR when the tracked remote is repointed", () =>
1475 Effect.gen(function* () {
1476 const repoDir = yield* makeTempDir("t3code-git-manager-");
1477 yield* initRepo(repoDir);
1478 yield* runGit(repoDir, ["checkout", "-b", "feature/pr-repointed"]);
1479 const originalRemoteDir = yield* createBareRemote();
1480 yield* runGit(repoDir, ["remote", "add", "origin", originalRemoteDir]);
1481 yield* runGit(repoDir, ["push", "-u", "origin", "feature/pr-repointed"]);
1482
1483 const existingPr = {
1484 number: 216,
1485 title: "Old remote PR",
1486 url: "https://github.com/pingdotgg/codething-mvp/pull/216",
1487 baseRefName: "main",
1488 headRefName: "feature/pr-repointed",
1489 };
1490 const { manager } = yield* makeManager({
1491 ghScenario: {
1492 // @effect-diagnostics-next-line preferSchemaOverJson:off
1493 prListSequence: [JSON.stringify([existingPr])],
1494 failWith: new GitHubCli.GitHubCliUnavailableError({
1495 command: "gh",
1496 cwd: repoDir,
1497 cause: new Error("rate limited"),
1498 }),
1499 failAfterCalls: 1,
1500 },
1501 });
1502
1503 const first = yield* manager.status({ cwd: repoDir });
1504 expect(first.pr?.number).toBe(216);
1505
1506 const replacementRemoteDir = yield* createBareRemote();
1507 yield* runGit(repoDir, ["remote", "add", "replacement", replacementRemoteDir]);
1508 yield* runGit(repoDir, ["push", "replacement", "feature/pr-repointed"]);
1509 yield* runGit(repoDir, ["remote", "set-url", "origin", replacementRemoteDir]);
1510 yield* manager.invalidateStatus(repoDir);
1511
1512 const second = yield* manager.status({ cwd: repoDir });
1513 expect(second.pr).toBeNull();
1514 }),
1515 );
1516
13391517 it.effect("creates a commit when working tree is dirty", () =>
13401518 Effect.gen(function* () {
13411519 const repoDir = yield* makeTempDir("t3code-git-manager-");
apps/server/src/git/GitManager.ts+158-15
......@@ -32,6 +32,7 @@ import {
3232import {
3333 detectSourceControlProviderFromGitRemoteUrl,
3434 mergeGitStatusParts,
35 normalizeGitRemoteUrl,
3536 resolveAutoFeatureBranchName,
3637 sanitizeBranchFragment,
3738 sanitizeFeatureBranchName,
......@@ -95,6 +96,9 @@ const SHORT_SHA_LENGTH = 7;
9596const TOAST_DESCRIPTION_MAX = 72;
9697const STATUS_RESULT_CACHE_TTL = Duration.seconds(1);
9798const STATUS_RESULT_CACHE_CAPACITY = 2_048;
99const PR_LOOKUP_CACHE_TTL = Duration.minutes(2);
100const PR_LOOKUP_FAILURE_TTL = Duration.seconds(20);
101const PR_LOOKUP_CACHE_CAPACITY = 2_048;
98102type StripProgressContext<T> = T extends any ? Omit<T, "actionId" | "cwd" | "action"> : never;
99103type GitActionProgressPayload = StripProgressContext<GitActionProgressEvent>;
100104type GitActionProgressEmitter = (event: GitActionProgressPayload) => Effect.Effect<void, never>;
......@@ -142,6 +146,7 @@ interface BranchHeadContext {
142146 headSelectors: ReadonlyArray<string>;
143147 preferredHeadSelector: string;
144148 remoteName: string | null;
149 headRemoteUrlKey: string | null;
145150 headRepositoryNameWithOwner: string | null;
146151 headRepositoryOwnerLogin: string | null;
147152 isCrossRepository: boolean;
......@@ -768,6 +773,145 @@ export const make = Effect.gen(function* () {
768773 normalizeStatusCacheKey(cwd).pipe(
769774 Effect.flatMap((cacheKey) => Cache.invalidate(localStatusResultCache, cacheKey)),
770775 );
776 // PR lookups hit the hosting provider's API (gh/glab/...), so they refresh
777 // on their own, slower cadence: ahead/behind counts stay fresh on every
778 // status poll while the PR association is re-fetched at most once per
779 // PR_LOOKUP_CACHE_TTL per branch. Git actions and user-driven refreshes bump
780 // the epoch (invalidateStatus) to bypass the cache immediately.
781 const prLookupEpochByCwd = new Map<string, number>();
782 const prLookupEpoch = (cwd: string) => prLookupEpochByCwd.get(cwd) ?? 0;
783 const bumpPrLookupEpoch = (cwd: string) =>
784 normalizeStatusCacheKey(cwd).pipe(
785 Effect.map((cacheKey) => {
786 prLookupEpochByCwd.set(cacheKey, prLookupEpoch(cacheKey) + 1);
787 }),
788 );
789 // Cache keys are NUL-joined [cwd, branch, upstreamRef, epoch] — none of the
790 // segments can contain a NUL byte, and refs are never empty, so "" decodes
791 // back to a null upstreamRef.
792 const prLookupCacheKey = (cwd: string, details: { branch: string; upstreamRef: string | null }) =>
793 [cwd, details.branch, details.upstreamRef ?? "", String(prLookupEpoch(cwd))].join("\u0000");
794 const prLookupCache = yield* Cache.makeWith(
795 (key: string) => {
796 const [cwd = "", branch = "", upstreamRef = ""] = key.split("\u0000");
797 const details = {
798 branch,
799 upstreamRef: upstreamRef.length > 0 ? upstreamRef : null,
800 };
801 return resolveBranchHeadContext(cwd, details).pipe(
802 Effect.flatMap((headContext) =>
803 findLatestPrForHeadContext(cwd, headContext).pipe(
804 Effect.map((latest) => ({ latest, headContext })),
805 ),
806 ),
807 );
808 },
809 {
810 capacity: PR_LOOKUP_CACHE_CAPACITY,
811 timeToLive: (exit) => (Exit.isSuccess(exit) ? PR_LOOKUP_CACHE_TTL : PR_LOOKUP_FAILURE_TTL),
812 },
813 );
814 // A transient lookup failure (rate limit, network blip) must not clear an
815 // already-known PR badge, so the last successful answer per branch sticks
816 // around as the fallback. Keep the resolved head context with it so a
817 // branch retargeted to another remote/fork cannot inherit the old badge.
818 interface LastKnownPr {
819 readonly pr: ReturnType<typeof toStatusPr> | null;
820 readonly upstreamRef: string | null;
821 readonly headBranch: string;
822 readonly remoteName: string | null;
823 readonly headRemoteUrlKey: string | null;
824 }
825 const lastKnownPrByBranchKey = new Map<string, LastKnownPr>();
826 const rememberLastKnownPr = (branchKey: string, entry: LastKnownPr) => {
827 if (
828 !lastKnownPrByBranchKey.has(branchKey) &&
829 lastKnownPrByBranchKey.size >= PR_LOOKUP_CACHE_CAPACITY
830 ) {
831 const oldestKey = lastKnownPrByBranchKey.keys().next().value;
832 if (oldestKey !== undefined) {
833 lastKnownPrByBranchKey.delete(oldestKey);
834 }
835 }
836 lastKnownPrByBranchKey.set(branchKey, entry);
837 };
838 const resolveLastKnownPr = (
839 branchKey: string,
840 current: Pick<LastKnownPr, "upstreamRef" | "headBranch" | "remoteName" | "headRemoteUrlKey">,
841 ): ReturnType<typeof toStatusPr> | null => {
842 const lastKnown = lastKnownPrByBranchKey.get(branchKey);
843 if (!lastKnown) return null;
844 if (lastKnown.headBranch !== current.headBranch) {
845 return null;
846 }
847
848 // The normalized URL catches both remote-alias changes and an existing
849 // alias being repointed. It also lets an upstream appear after `push -u`
850 // without invalidating the fallback when it still targets the same repo.
851 if (lastKnown.headRemoteUrlKey !== null || current.headRemoteUrlKey !== null) {
852 return lastKnown.headRemoteUrlKey === current.headRemoteUrlKey ? lastKnown.pr : null;
853 }
854
855 // If neither remote URL is available, fall back to the remote identity
856 // encoded by tracked branches. A null-to-non-null transition is allowed
857 // because that is the expected first-push case.
858 if (lastKnown.upstreamRef !== null && current.upstreamRef !== null) {
859 return lastKnown.remoteName === current.remoteName ? lastKnown.pr : null;
860 }
861 return lastKnown.pr;
862 };
863 const lookupStatusPr = Effect.fn("lookupStatusPr")(function* (
864 cwd: string,
865 details: { branch: string; upstreamRef: string | null; isDefaultBranch: boolean },
866 ) {
867 // Keyed by (cwd, branch) only: the upstream ref changing (e.g. a first
868 // `push -u`) must not orphan the fallback value for the same branch.
869 const branchKey = `${cwd}\u0000${details.branch}`;
870 return yield* Cache.get(prLookupCache, prLookupCacheKey(cwd, details)).pipe(
871 Effect.map(({ latest, headContext }) => {
872 if (!latest) return { pr: null, headContext };
873 // On the default branch, only surface open PRs.
874 // Merged/closed matches are usually reverse-merge history, not the thread's PR context.
875 if (details.isDefaultBranch && latest.state !== "open") {
876 return { pr: null, headContext };
877 }
878 return { pr: toStatusPr(latest), headContext };
879 }),
880 Effect.tap(({ pr, headContext }) =>
881 Effect.sync(() =>
882 rememberLastKnownPr(branchKey, {
883 pr,
884 upstreamRef: details.upstreamRef,
885 headBranch: headContext.headBranch,
886 remoteName: headContext.remoteName,
887 headRemoteUrlKey: headContext.headRemoteUrlKey,
888 }),
889 ),
890 ),
891 Effect.map(({ pr }) => pr),
892 Effect.catch((error) =>
893 Effect.logWarning("PR lookup failed; keeping last known PR state.").pipe(
894 Effect.annotateLogs({
895 operation: "lookupStatusPr",
896 branch: details.branch,
897 errorTag:
898 typeof error === "object" && error !== null && "_tag" in error
899 ? String(error._tag)
900 : typeof error,
901 }),
902 Effect.andThen(resolveBranchHeadContext(cwd, details)),
903 Effect.map((headContext) =>
904 resolveLastKnownPr(branchKey, {
905 upstreamRef: details.upstreamRef,
906 headBranch: headContext.headBranch,
907 remoteName: headContext.remoteName,
908 headRemoteUrlKey: headContext.headRemoteUrlKey,
909 }),
910 ),
911 ),
912 ),
913 );
914 });
771915 const readRemoteStatus = Effect.fn("readRemoteStatus")(function* (
772916 cwd: string,
773917 options?: GitVcsDriver.GitRemoteStatusOptions,
......@@ -781,19 +925,11 @@ export const make = Effect.gen(function* () {
781925
782926 const pr =
783927 details.branch !== null
784 ? yield* findLatestPr(cwd, {
928 ? yield* lookupStatusPr(cwd, {
785929 branch: details.branch,
786930 upstreamRef: details.upstreamRef,
787 }).pipe(
788 Effect.map((latest) => {
789 if (!latest) return null;
790 // On the default branch, only surface open PRs.
791 // Merged/closed matches are usually reverse-merge history, not the thread's PR context.
792 if (details.isDefaultBranch && latest.state !== "open") return null;
793 return toStatusPr(latest);
794 }),
795 Effect.orElseSucceed(() => null),
796 )
931 isDefaultBranch: details.isDefaultBranch,
932 })
797933 : null;
798934
799935 return {
......@@ -837,6 +973,7 @@ export const make = Effect.gen(function* () {
837973 ) {
838974 if (!remoteName) {
839975 return {
976 remoteUrlKey: null,
840977 repositoryNameWithOwner: null,
841978 ownerLogin: null,
842979 };
......@@ -845,6 +982,7 @@ export const make = Effect.gen(function* () {
845982 const remoteUrl = yield* readConfigValueNullable(cwd, `remote.${remoteName}.url`);
846983 const repositoryNameWithOwner = parseGitHubRepositoryNameWithOwnerFromRemoteUrl(remoteUrl);
847984 return {
985 remoteUrlKey: remoteUrl ? normalizeGitRemoteUrl(remoteUrl) : null,
848986 repositoryNameWithOwner,
849987 ownerLogin: parseRepositoryOwnerLogin(repositoryNameWithOwner),
850988 };
......@@ -915,6 +1053,9 @@ export const make = Effect.gen(function* () {
9151053 preferredHeadSelector:
9161054 ownerHeadSelector && isCrossRepository ? ownerHeadSelector : headBranch,
9171055 remoteName,
1056 headRemoteUrlKey:
1057 remoteRepository.remoteUrlKey ??
1058 (remoteName === null ? originRepository.remoteUrlKey : null),
9181059 headRepositoryNameWithOwner: remoteRepository.repositoryNameWithOwner,
9191060 headRepositoryOwnerLogin: remoteRepository.ownerLogin,
9201061 isCrossRepository,
......@@ -960,11 +1101,10 @@ export const make = Effect.gen(function* () {
9601101 return null;
9611102 });
9621103
963 const findLatestPr = Effect.fn("findLatestPr")(function* (
1104 const findLatestPrForHeadContext = Effect.fn("findLatestPrForHeadContext")(function* (
9641105 cwd: string,
965 details: { branch: string; upstreamRef: string | null },
1106 headContext: BranchHeadContext,
9661107 ) {
967 const headContext = yield* resolveBranchHeadContext(cwd, details);
9681108 const parsedByNumber = new Map<number, PullRequestInfo>();
9691109
9701110 for (const headSelector of headContext.headSelectors) {
......@@ -991,7 +1131,6 @@ export const make = Effect.gen(function* () {
9911131 }
9921132 return parsed[0] ?? null;
9931133 });
994
9951134 const buildCompletionToast = Effect.fn("buildCompletionToast")(function* (
9961135 cwd: string,
9971136 result: Pick<GitRunStackedActionResult, "action" | "branch" | "commit" | "push" | "pr">,
......@@ -1442,6 +1581,10 @@ export const make = Effect.gen(function* () {
14421581 function* (cwd) {
14431582 yield* invalidateLocalStatusResultCache(cwd);
14441583 yield* invalidateRemoteStatusResultCache(cwd);
1584 // Full invalidation is the explicit-freshness path (git actions, user
1585 // refresh); it also bypasses the slow PR-lookup cache. The periodic
1586 // status poll only invalidates local/remote and keeps the PR cache warm.
1587 yield* bumpPrLookupEpoch(cwd);
14451588 },
14461589 );
14471590
apps/server/src/server.test.ts+3
......@@ -5285,6 +5285,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => {
52855285 gitManager: {
52865286 invalidateLocalStatus: () => Effect.void,
52875287 invalidateRemoteStatus: () => Effect.void,
5288 invalidateStatus: () => Effect.void,
52885289 localStatus: () =>
52895290 Effect.succeed({
52905291 isRepo: true,
......@@ -5331,6 +5332,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => {
53315332 gitManager: {
53325333 invalidateLocalStatus: () => Effect.void,
53335334 invalidateRemoteStatus: () => Effect.void,
5335 invalidateStatus: () => Effect.void,
53345336 localStatus: () =>
53355337 Effect.succeed({
53365338 isRepo: true,
......@@ -5407,6 +5409,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => {
54075409 gitManager: {
54085410 invalidateLocalStatus: () => Effect.void,
54095411 invalidateRemoteStatus: () => Effect.void,
5412 invalidateStatus: () => Effect.void,
54105413 localStatus: () =>
54115414 Deferred.succeed(localRefreshStarted, undefined).pipe(
54125415 Effect.ignore,
apps/server/src/sourceControl/GitHubCli.test.ts+55
......@@ -208,6 +208,61 @@ describe("GitHubCli.layer", () => {
208208 }).pipe(Effect.provide(layer)),
209209 );
210210
211 it.effect("keeps pull requests from gh versions without headRepository.nameWithOwner", () =>
212 // gh < 2.47 (e.g. Ubuntu-packaged 2.46) exports headRepository as
213 // {id, name} only. These entries must decode instead of being dropped,
214 // with nameWithOwner rebuilt from the owner login.
215 Effect.gen(function* () {
216 mockRun.mockReturnValueOnce(
217 Effect.succeed(
218 processOutput(
219 // @effect-diagnostics-next-line preferSchemaOverJson:off
220 JSON.stringify([
221 {
222 number: 2829,
223 title: "Codex turn mapping",
224 url: "https://github.com/pingdotgg/codething-mvp/pull/2829",
225 baseRefName: "main",
226 headRefName: "t3code/codex-turn-mapping",
227 state: "OPEN",
228 mergedAt: null,
229 isCrossRepository: false,
230 headRepository: {
231 id: "R_kgDORLtfbQ",
232 name: "codething-mvp",
233 },
234 headRepositoryOwner: {
235 id: "MDEyOk9yZ2FuaXphdGlvbjg5MTkxNzI3",
236 login: "pingdotgg",
237 },
238 },
239 ]),
240 ),
241 ),
242 );
243
244 const gh = yield* GitHubCli.GitHubCli;
245 const result = yield* gh.listOpenPullRequests({
246 cwd: "/repo",
247 headSelector: "t3code/codex-turn-mapping",
248 });
249
250 assert.deepStrictEqual(result, [
251 {
252 number: 2829,
253 title: "Codex turn mapping",
254 url: "https://github.com/pingdotgg/codething-mvp/pull/2829",
255 baseRefName: "main",
256 headRefName: "t3code/codex-turn-mapping",
257 state: "open",
258 isCrossRepository: false,
259 headRepositoryNameWithOwner: "pingdotgg/codething-mvp",
260 headRepositoryOwnerLogin: "pingdotgg",
261 },
262 ]);
263 }).pipe(Effect.provide(layer)),
264 );
265
211266 it.effect("reads repository clone URLs", () =>
212267 Effect.gen(function* () {
213268 mockRun.mockReturnValueOnce(
apps/server/src/sourceControl/gitHubPullRequests.ts+13-5
......@@ -30,17 +30,21 @@ const GitHubPullRequestSchema = Schema.Struct({
3030 mergedAt: Schema.optional(Schema.NullOr(Schema.String)),
3131 updatedAt: Schema.optional(Schema.OptionFromNullOr(Schema.DateTimeUtcFromString)),
3232 isCrossRepository: Schema.optional(Schema.Boolean),
33 // gh < 2.47 exports headRepository as {id, name} only; nameWithOwner was
34 // added later. Both fields stay optional so a version-drifted gh CLI can
35 // never fail the decode and silently drop the PR from the list.
3336 headRepository: Schema.optional(
3437 Schema.NullOr(
3538 Schema.Struct({
36 nameWithOwner: Schema.String,
39 nameWithOwner: Schema.optional(Schema.NullOr(Schema.String)),
40 name: Schema.optional(Schema.NullOr(Schema.String)),
3741 }),
3842 ),
3943 ),
4044 headRepositoryOwner: Schema.optional(
4145 Schema.NullOr(
4246 Schema.Struct({
43 login: Schema.String,
47 login: Schema.optional(Schema.NullOr(Schema.String)),
4448 }),
4549 ),
4650 ),
......@@ -71,11 +75,15 @@ function normalizeGitHubPullRequestState(input: {
7175function normalizeGitHubPullRequestRecord(
7276 raw: Schema.Schema.Type<typeof GitHubPullRequestSchema>,
7377): NormalizedGitHubPullRequestRecord {
74 const headRepositoryNameWithOwner = trimOptionalString(raw.headRepository?.nameWithOwner);
78 const explicitNameWithOwner = trimOptionalString(raw.headRepository?.nameWithOwner);
79 const headRepositoryName = trimOptionalString(raw.headRepository?.name);
7580 const headRepositoryOwnerLogin =
7681 trimOptionalString(raw.headRepositoryOwner?.login) ??
77 (typeof headRepositoryNameWithOwner === "string" && headRepositoryNameWithOwner.includes("/")
78 ? (headRepositoryNameWithOwner.split("/")[0] ?? null)
82 (explicitNameWithOwner?.includes("/") ? (explicitNameWithOwner.split("/")[0] ?? null) : null);
83 const headRepositoryNameWithOwner =
84 explicitNameWithOwner ??
85 (headRepositoryOwnerLogin && headRepositoryName
86 ? `${headRepositoryOwnerLogin}/${headRepositoryName}`
7987 : null);
8088
8189 return {
apps/server/src/vcs/GitVcsDriverCore.test.ts+41
......@@ -690,6 +690,47 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
690690 );
691691 });
692692
693 describe("remote operations", () => {
694 it.effect("ensureRemote reuses an existing remote across ssh/https transport variants", () =>
695 Effect.gen(function* () {
696 const cwd = yield* makeTmpDir();
697 yield* initRepoWithCommit(cwd);
698 const driver = yield* GitVcsDriver.GitVcsDriver;
699
700 yield* git(cwd, ["remote", "add", "origin", "https://github.com/pingdotgg/t3code.git"]);
701
702 const reusedForSsh = yield* driver.ensureRemote({
703 cwd,
704 preferredName: "pingdotgg",
705 url: "git@github.com:pingdotgg/t3code.git",
706 });
707 assert.equal(reusedForSsh, "origin");
708
709 const reusedForSshScheme = yield* driver.ensureRemote({
710 cwd,
711 preferredName: "pingdotgg",
712 url: "ssh://git@github.com/pingdotgg/t3code",
713 });
714 assert.equal(reusedForSshScheme, "origin");
715
716 const reusedForSshWithPort = yield* driver.ensureRemote({
717 cwd,
718 preferredName: "pingdotgg",
719 url: "ssh://git@github.com:22/pingdotgg/t3code.git",
720 });
721 assert.equal(reusedForSshWithPort, "origin");
722
723 const addedForFork = yield* driver.ensureRemote({
724 cwd,
725 preferredName: "octocat",
726 url: "git@github.com:octocat/t3code.git",
727 });
728 assert.equal(addedForFork, "octocat");
729 assert.equal(yield* git(cwd, ["remote"]), "octocat\norigin");
730 }),
731 );
732 });
733
693734 describe("commit context", () => {
694735 it.effect("stages selected files and commits only those files", () =>
695736 Effect.gen(function* () {
apps/server/src/vcs/GitVcsDriverCore.ts+3-11
......@@ -25,7 +25,7 @@ import {
2525 type ReviewDiffPreviewSource,
2626 type VcsRef,
2727} from "@t3tools/contracts";
28import { dedupeRemoteBranchesWithLocalMatches } from "@t3tools/shared/git";
28import { dedupeRemoteBranchesWithLocalMatches, normalizeGitRemoteUrl } from "@t3tools/shared/git";
2929import { compactTraceAttributes } from "@t3tools/shared/observability";
3030import { decodeJsonResult } from "@t3tools/shared/schemaJson";
3131import { gitCommandDuration, gitCommandsTotal, withMetrics } from "../observability/Metrics.ts";
......@@ -235,14 +235,6 @@ function sanitizeRemoteName(value: string): string {
235235 return sanitized.length > 0 ? sanitized : "fork";
236236}
237237
238function normalizeRemoteUrl(value: string): string {
239 return value
240 .trim()
241 .replace(/\/+$/g, "")
242 .replace(/\.git$/i, "")
243 .toLowerCase();
244}
245
246238function parseRemoteFetchUrls(stdout: string): Map<string, string> {
247239 const remotes = new Map<string, string>();
248240 for (const line of stdout.split("\n")) {
......@@ -1092,7 +1084,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
10921084 "ensureRemote",
10931085 )(function* (input) {
10941086 const preferredName = sanitizeRemoteName(input.preferredName);
1095 const normalizedTargetUrl = normalizeRemoteUrl(input.url);
1087 const normalizedTargetUrl = normalizeGitRemoteUrl(input.url);
10961088 const remoteFetchUrls = yield* runGitStdout(
10971089 "GitVcsDriver.ensureRemote.listRemoteUrls",
10981090 input.cwd,
......@@ -1100,7 +1092,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
11001092 ).pipe(Effect.map((stdout) => parseRemoteFetchUrls(stdout)));
11011093
11021094 for (const [remoteName, remoteUrl] of remoteFetchUrls.entries()) {
1103 if (normalizeRemoteUrl(remoteUrl) === normalizedTargetUrl) {
1095 if (normalizeGitRemoteUrl(remoteUrl) === normalizedTargetUrl) {
11041096 return remoteName;
11051097 }
11061098 }
apps/server/src/vcs/VcsStatusBroadcaster.test.ts+10
......@@ -94,6 +94,11 @@ function makeTestLayer(state: {
9494 Effect.sync(() => {
9595 state.remoteInvalidationCalls += 1;
9696 }),
97 invalidateStatus: () =>
98 Effect.sync(() => {
99 state.localInvalidationCalls += 1;
100 state.remoteInvalidationCalls += 1;
101 }),
97102 }),
98103 ),
99104 );
......@@ -206,6 +211,11 @@ describe("VcsStatusBroadcaster", () => {
206211 Effect.sync(() => {
207212 state.remoteInvalidationCalls += 1;
208213 }),
214 invalidateStatus: () =>
215 Effect.sync(() => {
216 state.localInvalidationCalls += 1;
217 state.remoteInvalidationCalls += 1;
218 }),
209219 }),
210220 ),
211221 );
apps/server/src/vcs/VcsStatusBroadcaster.ts+3-4
......@@ -366,10 +366,9 @@ export const make = Effect.gen(function* () {
366366 "VcsStatusBroadcaster.refreshStatus",
367367 )(function* (rawCwd) {
368368 const cwd = yield* withFileSystem(normalizeCwd(rawCwd));
369 yield* Effect.all([workflow.invalidateLocalStatus(cwd), workflow.invalidateRemoteStatus(cwd)], {
370 concurrency: "unbounded",
371 discard: true,
372 });
369 // invalidateStatus (not the two partial invalidations) so an explicit
370 // refresh also bypasses GitManager's slow PR-lookup cache.
371 yield* workflow.invalidateStatus(cwd);
373372 const [local, remote] = yield* Effect.all(
374373 [workflow.localStatus({ cwd }), workflow.remoteStatus({ cwd })],
375374 { concurrency: "unbounded" },