| author |  Julius Marminge <julius0216@outlook.com> 2026-07-22 18:11:07+02:00 |
| committer |  GitHub <noreply@github.com> 2026-07-22 18:11:07+02:00 |
| log | 9a0a07167f0623c3a7db0ffeff2e3939760309df |
| tree | c83e0a220db925cbab95720abcd3c417e586e3bb |
| parent | 9fe4832a3f87e48152f915cfac9f85ec78e14e24 |
| signature | Signed by PGP key B5690EEEBB952194 |
fix(server): don't drop sticky PR fallback when remote URL can't be resolved (#4289)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>2 files changed, 64 insertions(+), 7 deletions(-)
apps
server
apps/server/src/git/GitManager.test.ts+48
| ... | @@ -1514,6 +1514,54 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { | ... | @@ -1514,6 +1514,54 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { |
| 1514 | }), | 1514 | }), |
| 1515 | ); | 1515 | ); |
| 1516 | 1516 | ||
| 1517 | it.effect("status keeps the last known PR when the current remote URL can't be resolved", () => | ||
| 1518 | Effect.gen(function* () { | ||
| 1519 | const repoDir = yield* makeTempDir("t3code-git-manager-"); | ||
| 1520 | yield* initRepo(repoDir); | ||
| 1521 | yield* runGit(repoDir, ["checkout", "-b", "feature/pr-config-hiccup"]); | ||
| 1522 | const remoteDir = yield* createBareRemote(); | ||
| 1523 | yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); | ||
| 1524 | yield* runGit(repoDir, ["push", "-u", "origin", "feature/pr-config-hiccup"]); | ||
| 1525 | |||
| 1526 | const existingPr = { | ||
| 1527 | number: 217, | ||
| 1528 | title: "Config hiccup PR", | ||
| 1529 | url: "https://github.com/pingdotgg/codething-mvp/pull/217", | ||
| 1530 | baseRefName: "main", | ||
| 1531 | headRefName: "feature/pr-config-hiccup", | ||
| 1532 | }; | ||
| 1533 | const { manager } = yield* makeManager({ | ||
| 1534 | ghScenario: { | ||
| 1535 | // @effect-diagnostics-next-line preferSchemaOverJson:off | ||
| 1536 | prListSequence: [JSON.stringify([existingPr])], | ||
| 1537 | failWith: new GitHubCli.GitHubCliUnavailableError({ | ||
| 1538 | command: "gh", | ||
| 1539 | cwd: repoDir, | ||
| 1540 | cause: new Error("rate limited"), | ||
| 1541 | }), | ||
| 1542 | failAfterCalls: 1, | ||
| 1543 | }, | ||
| 1544 | }); | ||
| 1545 | |||
| 1546 | const first = yield* manager.status({ cwd: repoDir }); | ||
| 1547 | expect(first.pr?.number).toBe(217); | ||
| 1548 | |||
| 1549 | // `remote.origin.url` reads go through readConfigValueNullable, which | ||
| 1550 | // maps ANY failed read (a real "no remote configured" state or a | ||
| 1551 | // transient git-config hiccup) to null the same way. Unsetting the | ||
| 1552 | // key here reproduces that ambiguity without touching branch | ||
| 1553 | // tracking (refs/remotes/origin/* and branch.<b>.remote are | ||
| 1554 | // untouched) — the remote identity has not actually changed, so the | ||
| 1555 | // sticky PR must survive even though the current lookup can no | ||
| 1556 | // longer resolve a remote URL to compare against. | ||
| 1557 | yield* runGit(repoDir, ["config", "--unset", "remote.origin.url"]); | ||
| 1558 | yield* manager.invalidateStatus(repoDir); | ||
| 1559 | |||
| 1560 | const second = yield* manager.status({ cwd: repoDir }); | ||
| 1561 | expect(second.pr?.number).toBe(217); | ||
| 1562 | }), | ||
| 1563 | ); | ||
| 1564 | |||
| 1517 | it.effect("creates a commit when working tree is dirty", () => | 1565 | it.effect("creates a commit when working tree is dirty", () => |
| 1518 | Effect.gen(function* () { | 1566 | Effect.gen(function* () { |
| 1519 | const repoDir = yield* makeTempDir("t3code-git-manager-"); | 1567 | const repoDir = yield* makeTempDir("t3code-git-manager-"); |
apps/server/src/git/GitManager.ts+16-7
| ... | @@ -846,16 +846,25 @@ export const make = Effect.gen(function* () { | ... | @@ -846,16 +846,25 @@ export const make = Effect.gen(function* () { |
| 846 | } | 846 | } |
| 847 | 847 | ||
| 848 | // The normalized URL catches both remote-alias changes and an existing | 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` | 849 | // alias being repointed. Both sides must be resolved before treating a |
| 850 | // without invalidating the fallback when it still targets the same repo. | 850 | // mismatch as real: `readConfigValueNullable` swallows any git-config |
| 851 | if (lastKnown.headRemoteUrlKey !== null || current.headRemoteUrlKey !== null) { | 851 | // read failure into `null`, so a transient failure to resolve the |
| 852 | // *current* remote URL must read as "unknown", not as "no remote" — the | ||
| 853 | // latter would otherwise drop an already-known PR badge on every hiccup. | ||
| 854 | if (lastKnown.headRemoteUrlKey !== null && current.headRemoteUrlKey !== null) { | ||
| 852 | return lastKnown.headRemoteUrlKey === current.headRemoteUrlKey ? lastKnown.pr : null; | 855 | return lastKnown.headRemoteUrlKey === current.headRemoteUrlKey ? lastKnown.pr : null; |
| 853 | } | 856 | } |
| 854 | 857 | ||
| 855 | // If neither remote URL is available, fall back to the remote identity | 858 | // If the remote URL can't be compared, fall back to the remote identity |
| 856 | // encoded by tracked branches. A null-to-non-null transition is allowed | 859 | // encoded by tracked branches — same "both sides known" requirement, for |
| 857 | // because that is the expected first-push case. | 860 | // the same reason. A null-to-non-null transition (upstream/remoteName) |
| 858 | if (lastKnown.upstreamRef !== null && current.upstreamRef !== null) { | 861 | // is allowed because that is the expected first-push case. |
| 862 | if ( | ||
| 863 | lastKnown.upstreamRef !== null && | ||
| 864 | current.upstreamRef !== null && | ||
| 865 | lastKnown.remoteName !== null && | ||
| 866 | current.remoteName !== null | ||
| 867 | ) { | ||
| 859 | return lastKnown.remoteName === current.remoteName ? lastKnown.pr : null; | 868 | return lastKnown.remoteName === current.remoteName ? lastKnown.pr : null; |
| 860 | } | 869 | } |
| 861 | return lastKnown.pr; | 870 | return lastKnown.pr; |