diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 464c3afad64..000a9d99c6b 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -1514,6 +1514,54 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { }), ); + it.effect("status keeps the last known PR when the current remote URL can't be resolved", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + yield* runGit(repoDir, ["checkout", "-b", "feature/pr-config-hiccup"]); + const remoteDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "feature/pr-config-hiccup"]); + + const existingPr = { + number: 217, + title: "Config hiccup PR", + url: "https://github.com/pingdotgg/codething-mvp/pull/217", + baseRefName: "main", + headRefName: "feature/pr-config-hiccup", + }; + const { manager } = yield* makeManager({ + ghScenario: { + // @effect-diagnostics-next-line preferSchemaOverJson:off + prListSequence: [JSON.stringify([existingPr])], + failWith: new GitHubCli.GitHubCliUnavailableError({ + command: "gh", + cwd: repoDir, + cause: new Error("rate limited"), + }), + failAfterCalls: 1, + }, + }); + + const first = yield* manager.status({ cwd: repoDir }); + expect(first.pr?.number).toBe(217); + + // `remote.origin.url` reads go through readConfigValueNullable, which + // maps ANY failed read (a real "no remote configured" state or a + // transient git-config hiccup) to null the same way. Unsetting the + // key here reproduces that ambiguity without touching branch + // tracking (refs/remotes/origin/* and branch..remote are + // untouched) — the remote identity has not actually changed, so the + // sticky PR must survive even though the current lookup can no + // longer resolve a remote URL to compare against. + yield* runGit(repoDir, ["config", "--unset", "remote.origin.url"]); + yield* manager.invalidateStatus(repoDir); + + const second = yield* manager.status({ cwd: repoDir }); + expect(second.pr?.number).toBe(217); + }), + ); + it.effect("creates a commit when working tree is dirty", () => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index 58bd8602df1..e59613f7cc8 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -846,16 +846,25 @@ export const make = Effect.gen(function* () { } // The normalized URL catches both remote-alias changes and an existing - // alias being repointed. It also lets an upstream appear after `push -u` - // without invalidating the fallback when it still targets the same repo. - if (lastKnown.headRemoteUrlKey !== null || current.headRemoteUrlKey !== null) { + // alias being repointed. Both sides must be resolved before treating a + // mismatch as real: `readConfigValueNullable` swallows any git-config + // read failure into `null`, so a transient failure to resolve the + // *current* remote URL must read as "unknown", not as "no remote" — the + // latter would otherwise drop an already-known PR badge on every hiccup. + if (lastKnown.headRemoteUrlKey !== null && current.headRemoteUrlKey !== null) { return lastKnown.headRemoteUrlKey === current.headRemoteUrlKey ? lastKnown.pr : null; } - // If neither remote URL is available, fall back to the remote identity - // encoded by tracked branches. A null-to-non-null transition is allowed - // because that is the expected first-push case. - if (lastKnown.upstreamRef !== null && current.upstreamRef !== null) { + // If the remote URL can't be compared, fall back to the remote identity + // encoded by tracked branches — same "both sides known" requirement, for + // the same reason. A null-to-non-null transition (upstream/remoteName) + // is allowed because that is the expected first-push case. + if ( + lastKnown.upstreamRef !== null && + current.upstreamRef !== null && + lastKnown.remoteName !== null && + current.remoteName !== null + ) { return lastKnown.remoteName === current.remoteName ? lastKnown.pr : null; } return lastKnown.pr;