diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index cd176d8f938a46812321c36b368307a729a6c98c..53bb49c346e361a26f29bff6d88927128a32cbce 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -272,6 +272,11 @@ function initRepo( yield* runGit(cwd, ["init", "--initial-branch=main"]); yield* runGit(cwd, ["config", "user.email", "test@example.com"]); yield* runGit(cwd, ["config", "user.name", "Test User"]); + // Hermetic line endings: machine-global `core.autocrlf=true` on Windows + // makes Git check out CRLF, which breaks exact-content assertions that + // write LF with Node. Local config overrides the global so the fixture + // (and its linked worktrees, which share config) always checks out LF. + yield* runGit(cwd, ["config", "core.autocrlf", "false"]); yield* fs.writeFileString(NodePath.join(cwd, "README.md"), "hello\n"); yield* runGit(cwd, ["add", "README.md"]); yield* runGit(cwd, ["commit", "-m", "Initial commit"]); @@ -1630,6 +1635,316 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { }), ); + it.effect("finds a fork PR for a branch pushed without an upstream", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const originDir = yield* createBareRemote(); + const forkDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", originDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "main"]); + yield* runGit(repoDir, ["remote", "add", "fork", forkDir]); + yield* runGit(repoDir, ["checkout", "-b", "feature/no-upstream-fork"]); + yield* runGit(repoDir, ["push", "fork", "feature/no-upstream-fork"]); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "origin", + "git@github.com:pingdotgg/codething-mvp.git", + originDir, + ); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "fork", + "git@github.com:contributor/codething-mvp.git", + forkDir, + ); + + const ghScenario = { + prListByHeadSelector: { + "feature/no-upstream-fork": JSON.stringify([ + { + number: 40, + title: "Another fork's PR", + url: "https://github.com/pingdotgg/codething-mvp/pull/40", + baseRefName: "main", + headRefName: "feature/no-upstream-fork", + state: "OPEN", + isCrossRepository: true, + headRepository: { nameWithOwner: "someone-else/codething-mvp" }, + headRepositoryOwner: { login: "someone-else" }, + }, + { + number: 41, + title: "Our fork's PR", + url: "https://github.com/pingdotgg/codething-mvp/pull/41", + baseRefName: "main", + headRefName: "feature/no-upstream-fork", + state: "OPEN", + isCrossRepository: true, + headRepository: { nameWithOwner: "contributor/codething-mvp" }, + headRepositoryOwner: { login: "contributor" }, + }, + ]), + }, + }; + const { manager } = yield* makeManager({ ghScenario }); + + const pullRequest = yield* manager.branchPullRequest({ + cwd: repoDir, + branch: "feature/no-upstream-fork", + }); + expect(pullRequest?.number).toBe(41); + expect((yield* manager.status({ cwd: repoDir })).pr?.number).toBe(41); + + yield* runGit(repoDir, ["commit", "--allow-empty", "-m", "Continue feature"]); + const localHead = yield* runGit(repoDir, ["rev-parse", "HEAD"]); + const pushedHead = yield* runGit(repoDir, [ + "rev-parse", + "refs/remotes/fork/feature/no-upstream-fork", + ]); + expect(localHead.stdout.trim()).not.toBe(pushedHead.stdout.trim()); + const { manager: aheadManager } = yield* makeManager({ ghScenario }); + expect( + (yield* aheadManager.branchPullRequest({ + cwd: repoDir, + branch: "feature/no-upstream-fork", + }))?.number, + ).toBe(41); + expect((yield* aheadManager.status({ cwd: repoDir })).pr?.number).toBe(41); + + yield* runGit(repoDir, ["config", "branch.feature/no-upstream-fork.remote", "origin"]); + const upstream = yield* runGit(repoDir, [ + "for-each-ref", + "--format=%(upstream:short)", + "refs/heads/feature/no-upstream-fork", + ]); + expect(upstream.stdout.trim()).toBe(""); + const { manager: staleRemoteManager } = yield* makeManager({ ghScenario }); + expect( + (yield* staleRemoteManager.branchPullRequest({ + cwd: repoDir, + branch: "feature/no-upstream-fork", + }))?.number, + ).toBe(41); + + yield* runGit(repoDir, ["config", "--unset", "branch.feature/no-upstream-fork.remote"]); + yield* runGit(repoDir, ["checkout", "-b", "diverged-push", "main"]); + yield* runGit(repoDir, ["commit", "--allow-empty", "-m", "Rewrite fork head"]); + yield* runGit(repoDir, [ + "push", + "--force", + "fork", + "HEAD:refs/heads/feature/no-upstream-fork", + ]); + yield* runGit(repoDir, ["checkout", "feature/no-upstream-fork"]); + const { manager: divergedManager } = yield* makeManager({ ghScenario }); + expect( + yield* divergedManager.branchPullRequest({ + cwd: repoDir, + branch: "feature/no-upstream-fork", + }), + ).toBeNull(); + }), + ); + + it.effect("prefers an exact fork ref but does not guess between equal refs", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const originDir = yield* createBareRemote(); + const firstForkDir = yield* createBareRemote(); + const secondForkDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", originDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "main"]); + yield* runGit(repoDir, ["remote", "add", "first-fork", firstForkDir]); + yield* runGit(repoDir, ["remote", "add", "second-fork", secondForkDir]); + yield* runGit(repoDir, ["checkout", "-b", "feature/ambiguous-fork"]); + yield* runGit(repoDir, ["push", "first-fork", "feature/ambiguous-fork"]); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "origin", + "git@github.com:pingdotgg/codething-mvp.git", + originDir, + ); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "first-fork", + "git@github.com:contributor/codething-mvp.git", + firstForkDir, + ); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "second-fork", + "git@github.com:someone-else/codething-mvp.git", + secondForkDir, + ); + + const ghScenario = { + prListByHeadSelector: { + "feature/ambiguous-fork": JSON.stringify([ + { + number: 41, + title: "One fork's PR", + url: "https://github.com/pingdotgg/codething-mvp/pull/41", + baseRefName: "main", + headRefName: "feature/ambiguous-fork", + state: "OPEN", + isCrossRepository: true, + headRepository: { nameWithOwner: "contributor/codething-mvp" }, + headRepositoryOwner: { login: "contributor" }, + }, + ]), + }, + }; + const { manager: singleRemoteManager } = yield* makeManager({ ghScenario }); + expect( + (yield* singleRemoteManager.branchPullRequest({ + cwd: repoDir, + branch: "feature/ambiguous-fork", + }))?.number, + ).toBe(41); + + yield* runGit(repoDir, ["push", "second-fork", "feature/ambiguous-fork"]); + const { manager } = yield* makeManager({ ghScenario }); + expect( + yield* manager.branchPullRequest({ cwd: repoDir, branch: "feature/ambiguous-fork" }), + ).toBeNull(); + + yield* runGit(repoDir, ["commit", "--allow-empty", "-m", "Continue feature"]); + yield* runGit(repoDir, ["push", "first-fork", "feature/ambiguous-fork"]); + const { manager: exactManager } = yield* makeManager({ ghScenario }); + expect( + (yield* exactManager.branchPullRequest({ + cwd: repoDir, + branch: "feature/ambiguous-fork", + }))?.number, + ).toBe(41); + }), + ); + + it.effect("does not guess a fork when an ancestry probe fails", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const originDir = yield* createBareRemote(); + const forkDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", originDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "main"]); + yield* runGit(repoDir, ["remote", "add", "fork", forkDir]); + yield* runGit(repoDir, ["checkout", "-b", "feature/failed-probe"]); + yield* runGit(repoDir, ["push", "fork", "feature/failed-probe"]); + yield* runGit(repoDir, ["push", "origin", "feature/failed-probe"]); + // Local work past both tips, so no exact ref matches and the ancestor + // tier decides. + yield* runGit(repoDir, ["commit", "--allow-empty", "-m", "Local work"]); + // Simulate a failed ancestry probe: point origin's tracking ref at a + // non-commit object so `merge-base --is-ancestor` exits 128 — neither + // a match (0) nor a negative result (1). + const treeOid = (yield* runGit(repoDir, ["rev-parse", "HEAD^{tree}"])).stdout.trim(); + yield* runGit(repoDir, ["update-ref", "refs/remotes/origin/feature/failed-probe", treeOid]); + const forkProbe = yield* runGit( + repoDir, + ["merge-base", "--is-ancestor", "refs/remotes/fork/feature/failed-probe", "HEAD"], + true, + ); + expect(forkProbe.exitCode).toBe(0); + const originProbe = yield* runGit( + repoDir, + ["merge-base", "--is-ancestor", "refs/remotes/origin/feature/failed-probe", "HEAD"], + true, + ); + expect(originProbe.exitCode).not.toBe(0); + expect(originProbe.exitCode).not.toBe(1); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "origin", + "git@github.com:pingdotgg/codething-mvp.git", + originDir, + ); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "fork", + "git@github.com:contributor/codething-mvp.git", + forkDir, + ); + + const ghScenario = { + prListByHeadSelector: { + "feature/failed-probe": JSON.stringify([ + { + number: 43, + title: "Failed probe PR", + url: "https://github.com/pingdotgg/codething-mvp/pull/43", + baseRefName: "main", + headRefName: "feature/failed-probe", + state: "OPEN", + isCrossRepository: true, + headRepository: { nameWithOwner: "contributor/codething-mvp" }, + headRepositoryOwner: { login: "contributor" }, + }, + ]), + }, + }; + const { manager } = yield* makeManager({ ghScenario }); + // The fork probe matches, but the failed origin probe rejects the + // whole inference: unknown, invisible rather than wrong. + expect( + yield* manager.branchPullRequest({ cwd: repoDir, branch: "feature/failed-probe" }), + ).toBeNull(); + }), + ); + + it.effect("finds a fork PR when the remote name contains a slash", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const originDir = yield* createBareRemote(); + const forkDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", originDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "main"]); + yield* runGit(repoDir, ["remote", "add", "team/fork", forkDir]); + yield* runGit(repoDir, ["checkout", "-b", "feature/slash-remote"]); + yield* runGit(repoDir, ["push", "team/fork", "feature/slash-remote"]); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "origin", + "git@github.com:pingdotgg/codething-mvp.git", + originDir, + ); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "team/fork", + "git@github.com:contributor/codething-mvp.git", + forkDir, + ); + + const { manager } = yield* makeManager({ + ghScenario: { + prListByHeadSelector: { + "feature/slash-remote": JSON.stringify([ + { + number: 42, + title: "Slash remote PR", + url: "https://github.com/pingdotgg/codething-mvp/pull/42", + baseRefName: "main", + headRefName: "feature/slash-remote", + state: "OPEN", + isCrossRepository: true, + headRepository: { nameWithOwner: "contributor/codething-mvp" }, + headRepositoryOwner: { login: "contributor" }, + }, + ]), + }, + }, + }); + + expect( + (yield* manager.branchPullRequest({ cwd: repoDir, branch: "feature/slash-remote" })) + ?.number, + ).toBe(42); + }), + ); + it.effect("branch PR lookup does not reuse a cached PR after the remote is repointed", () => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); @@ -2096,9 +2411,11 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { ); it.effect.each([ - "git@gitlab.com:Group/Subgroup/Fork.git", - "https://gitlab.com/Group/Subgroup/Fork.git", - ])("matches nested GitLab forks through the adapter for %s", (remoteUrl) => + { remoteUrl: "git@gitlab.com:Group/Subgroup/Fork.git", setUpstream: true }, + { remoteUrl: "git@gitlab.com:Group/Subgroup/Fork.git", setUpstream: false }, + { remoteUrl: "https://gitlab.com/Group/Subgroup/Fork.git", setUpstream: true }, + { remoteUrl: "https://gitlab.com/Group/Subgroup/Fork.git", setUpstream: false }, + ] as const)("matches GitLab forks for $remoteUrl/$setUpstream", ({ remoteUrl, setUpstream }) => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); yield* initRepo(repoDir); @@ -2109,7 +2426,13 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { yield* runGit(repoDir, ["push", "-u", "origin", "main"]); yield* runGit(repoDir, ["remote", "add", "fork", forkDir]); yield* runGit(repoDir, ["checkout", "-b", branch]); - yield* runGit(repoDir, ["push", "-u", "fork", branch]); + yield* runGit(repoDir, ["push", ...(setUpstream ? ["-u"] : []), "fork", branch]); + const upstream = yield* runGit(repoDir, [ + "for-each-ref", + "--format=%(upstream:short)", + `refs/heads/${branch}`, + ]); + expect(upstream.stdout.trim()).toBe(setUpstream ? `fork/${branch}` : ""); yield* configureVisibleRemoteUrlWithLocalRewrite( repoDir, "origin", @@ -5639,9 +5962,12 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { expect((yield* runGit(worktreePath, ["rev-parse", "HEAD"])).stdout.trim()).toBe( rewrittenHead, ); - expect(NodeFS.readFileSync(NodePath.join(worktreePath, "force-pushed.txt"), "utf8")).toBe( - "rewritten\n", - ); + expect( + NodeFS.readFileSync(NodePath.join(worktreePath, "force-pushed.txt"), "utf8").replace( + /\r\n/g, + "\n", + ), + ).toBe("rewritten\n"); }), ); diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index e2ae3109b2fe84da94296fed389b5aa67c4ef6fa..6079ee714f69575a9af0cea4c6446930a2296ea2 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -1337,8 +1337,21 @@ export const make = Effect.gen(function* () { ), ), Effect.map(({ pr }) => pr), - Effect.catch((error) => - Effect.logWarning("PR lookup failed; keeping last known PR state.").pipe( + Effect.catch((error) => { + // No prior PR means there is no badge to preserve or remote to verify. + const fallback = lastKnownPrByBranchKey.get(branchKey)?.pr + ? resolveLookupHeadContext(cwd, details).pipe( + Effect.map(({ headContext }) => + resolveLastKnownPr(branchKey, { + upstreamRef: details.upstreamRef, + headBranch: headContext.headBranch, + remoteName: headContext.remoteName, + headRemoteUrlKey: headContext.headRemoteUrlKey, + }), + ), + ) + : Effect.succeed(null); + return Effect.logWarning("PR lookup failed.").pipe( Effect.annotateLogs({ operation: "lookupStatusPr", branch: details.branch, @@ -1355,17 +1368,9 @@ export const make = Effect.gen(function* () { } : {}), }), - Effect.andThen(resolveLookupHeadContext(cwd, details)), - Effect.map(({ headContext }) => - resolveLastKnownPr(branchKey, { - upstreamRef: details.upstreamRef, - headBranch: headContext.headBranch, - remoteName: headContext.remoteName, - headRemoteUrlKey: headContext.headRemoteUrlKey, - }), - ), - ), - ), + Effect.andThen(fallback), + ); + }), ); }); const readRemoteStatus = Effect.fn("readRemoteStatus")(function* ( @@ -1615,7 +1620,94 @@ export const make = Effect.gen(function* () { } if (matching.includes("origin")) return "origin"; return matching[0] ?? null; - }).pipe(Effect.orElseSucceed(() => null)); + }); + }); + + // A no-upstream branch may have local commits after its last push. Prefer a + // unique exact ref, then a unique remote tip in its history; never break a + // fork tie by remote order. + const findPublishedBranchRemote = Effect.fn("findPublishedBranchRemote")(function* ( + cwd: string, + branch: string, + ) { + const localRef = `refs/heads/${branch}`; + const remoteNames = (yield* gitCore.execute({ + operation: "GitManager.findPublishedBranchRemote.remotes", + cwd, + args: ["remote"], + timeoutMs: 5_000, + })).stdout + .split("\n") + .map((name) => name.trim()) + .filter((name) => name.length > 0); + if (remoteNames.length === 0) return null; + + const refs = yield* gitCore.execute({ + operation: "GitManager.findPublishedBranchRemote", + cwd, + args: [ + "for-each-ref", + "--format=%(refname)%00%(objectname)", + localRef, + ...remoteNames.map((name) => `refs/remotes/${name}/${branch}`), + ], + timeoutMs: 5_000, + }); + const oidByRef = new Map( + refs.stdout + .split("\n") + .map((line) => line.trim().split("\u0000")) + .filter((entry): entry is [string, string] => entry.length === 2), + ); + const localOid = oidByRef.get(localRef); + if (!localOid) return null; + + const remoteRefs = remoteNames.flatMap((name) => { + const oid = oidByRef.get(`refs/remotes/${name}/${branch}`); + return oid ? [{ name, oid }] : []; + }); + const exactRemotes = remoteRefs.filter((remote) => remote.oid === localOid); + if (exactRemotes.length > 1) return null; + let remoteName = exactRemotes[0]?.name; + if (remoteName === undefined) { + const ancestorRemotes = yield* Effect.forEach( + remoteRefs, + (remote) => + gitCore + .execute({ + operation: "GitManager.findPublishedBranchRemote.ancestor", + cwd, + args: ["merge-base", "--is-ancestor", remote.oid, localOid], + allowNonZeroExit: true, + timeoutMs: 5_000, + }) + .pipe( + Effect.flatMap((result) => { + // `merge-base --is-ancestor` reports 0 for ancestor and 1 + // for not-ancestor; any other non-zero exit is a failed + // probe, not a negative result. A failed probe rejects the + // whole inference instead of letting a surviving probe win + // by default and attach the wrong fork's PR. + if (result.exitCode === 0) return Effect.succeed(remote.name); + if (result.exitCode === 1) return Effect.succeed(null); + return new GitCommandError({ + operation: "GitManager.findPublishedBranchRemote.ancestor", + command: "git merge-base --is-ancestor", + cwd, + exitCode: result.exitCode, + detail: "Ancestry probe failed; rejecting ancestry inference.", + }); + }), + ), + { concurrency: "unbounded" }, + ); + const matches = ancestorRemotes.filter((name) => name !== null); + if (matches.length !== 1) return null; + remoteName = matches[0]; + } + if (remoteName === undefined) return null; + const remoteUrl = yield* readConfigValueNullable(cwd, `remote.${remoteName}.url`); + return remoteUrl === null ? null : remoteName; }); // `git worktree add -b feature origin/main` makes the new local branch track @@ -1640,6 +1732,17 @@ export const make = Effect.gen(function* () { }, ) { const headContext = yield* resolveBranchHeadContext(cwd, details); + if (details.upstreamRef === null) { + const remoteName = yield* findPublishedBranchRemote(cwd, details.branch).pipe( + Effect.orElseSucceed(() => null), + ); + if (remoteName !== null && remoteName !== headContext.remoteName) { + return { + headContext: yield* resolveBranchHeadContext(cwd, { ...details, remoteName }), + lookup: true, + }; + } + } const upstreamHeadIsDefault = headContext.headBranch === details.defaultBranch || (details.defaultBranch === null && @@ -1651,7 +1754,11 @@ export const make = Effect.gen(function* () { ) { return { headContext, lookup: true }; } - const remoteName = yield* findRemoteTrackingRemote(cwd, details.branch, headContext.remoteName); + const remoteName = yield* findRemoteTrackingRemote( + cwd, + details.branch, + headContext.remoteName, + ).pipe(Effect.orElseSucceed(() => null)); if (remoteName === null) { return { headContext, lookup: false }; } @@ -1671,7 +1778,7 @@ export const make = Effect.gen(function* () { * terminal and agent pushes land), and configured upstream metadata survives * when a merged change request's remote branch is deleted. Together they * distinguish branches known to have reached a host from genuinely local - * branches. The ref glob spans every remote so a fork branch still counts. A + * branches. The remote ref check includes names containing slashes. A * repository that tracks no remotes at all cannot answer the question, * because then every branch looks unpublished; it, and any failed probe, * keeps the lookup. @@ -1705,11 +1812,11 @@ export const make = Effect.gen(function* () { return false; } - const [tracksAnyRemote, tracksThisBranch] = yield* Effect.all( - [matchesRef("refs/remotes"), matchesRef(`refs/remotes/*/${headContext.headBranch}`)], + const [tracksAnyRemote, trackedRemote] = yield* Effect.all( + [matchesRef("refs/remotes"), findRemoteTrackingRemote(cwd, headContext.headBranch, null)], { concurrency: "unbounded" }, ); - return tracksAnyRemote && !tracksThisBranch; + return tracksAnyRemote && trackedRemote === null; }).pipe(Effect.orElseSucceed(() => false)); }); diff --git a/docs/user/source-control.md b/docs/user/source-control.md index b906779021b1fd2bb0b1a4a60c3e0f9df001e811..4b15fe6e713ec367b14d1d3d1c367537056a048a 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -140,6 +140,10 @@ make your first commit before pushing. Use a thread's Git actions to commit, push, and create a pull request. T3 Code can generate commit messages, review titles, and descriptions from your changes. +An existing fork PR can appear beside the branch even if you pushed without setting an upstream. +If its badge is missing because several remotes hold that branch, set the branch's upstream to +the intended remote with `git branch --set-upstream-to=/`. + Choose the writing style and model in **Settings → Source Control**. **Repository conventions** uses the project's instructions and recent commit subjects.