diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index d3e0d49bdbe53..cdc7bde90fa72 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -4741,6 +4741,57 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { }), ); + it.effect( + "create_pr publishes to the push remote when the tracked upstream already has HEAD", + () => + 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, ["remote", "add", "fork", forkDir]); + yield* runGit(repoDir, ["checkout", "-b", "topic"]); + NodeFS.writeFileSync(NodePath.join(repoDir, "changes.txt"), "change\n"); + yield* runGit(repoDir, ["add", "changes.txt"]); + yield* runGit(repoDir, ["commit", "-m", "Topic change"]); + yield* runGit(repoDir, ["push", "-u", "origin", "topic"]); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "origin", + "git@github.com:pingdotgg/codething-mvp.git", + originDir, + ); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "fork", + "git@github.com:octocat/codething-mvp.git", + forkDir, + ); + yield* runGit(repoDir, ["config", "remote.pushDefault", "fork"]); + + const { manager, providerCalls } = yield* makeManager(); + const result = yield* runStackedAction(manager, { + cwd: repoDir, + action: "create_pr", + }); + + expect(result.push).toMatchObject({ status: "pushed", upstreamBranch: "fork/topic" }); + expect(result.pr.status).toBe("created"); + expect( + yield* runGit(forkDir, ["--git-dir=.", "log", "-1", "--pretty=%s", "topic"]), + ).toMatchObject({ + stdout: "Topic change\n", + }); + expect(created(providerCalls)).toEqual([ + { baseRefName: "main", headSelector: "octocat:topic" }, + ]); + expect(yield* runGit(repoDir, ["rev-parse", "--abbrev-ref", "@{upstream}"])).toMatchObject({ + stdout: "origin/topic\n", + }); + }), + ); + it.effect("rejects push/pr actions from detached HEAD", () => 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 af56b1b65b511..c3f573a9391e6 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -2376,6 +2376,7 @@ export const make = Effect.gen(function* () { fallbackBranch: string | null, threadId: ThreadId | undefined, emit: GitActionProgressEmitter, + pushRemoteRef?: { readonly remoteName: string; readonly upstreamRef: string }, ) { const provider = yield* sourceControlProvider(cwd); const terms = sourceControlDefinitions.get(provider.kind).changeRequest; @@ -2396,10 +2397,12 @@ export const make = Effect.gen(function* () { }); } - const headContext = yield* resolveBranchHeadContext(cwd, { - branch, - upstreamRef: details.upstreamRef, - }); + // A configured push remote receives the commits while the branch keeps + // tracking its upstream, so the PR head is the push remote's branch. + const headContext = yield* resolveBranchHeadContext( + cwd, + pushRemoteRef ? { branch, ...pushRemoteRef } : { branch, upstreamRef: details.upstreamRef }, + ); const existing = yield* findOpenPr(cwd, headContext); if (existing) { @@ -3402,12 +3405,26 @@ export const make = Effect.gen(function* () { > { const initialStatus = yield* gitCore.statusDetails(input.cwd); const wantsCommit = isCommitAction(input.action); + const upToDateBranch = + input.action === "create_pr" && + initialStatus.hasUpstream && + initialStatus.aheadCount === 0 + ? initialStatus.branch + : null; + // A current upstream says nothing about a separate push remote, which may lack HEAD. + const pushTarget = upToDateBranch + ? yield* gitCore + .resolvePushTarget(input.cwd, { branch: upToDateBranch, hasUpstream: true }) + .pipe(Effect.orElseSucceed(() => null)) + : null; const wantsPush = input.action === "push" || input.action === "commit_push" || input.action === "commit_push_pr" || (input.action === "create_pr" && - (!initialStatus.hasUpstream || initialStatus.aheadCount > 0)); + (!initialStatus.hasUpstream || + initialStatus.aheadCount > 0 || + pushTarget?.viaPushRemote === true)); const wantsPr = input.action === "create_pr" || input.action === "commit_push_pr"; if (input.featureBranch && !wantsCommit) { @@ -3560,6 +3577,9 @@ export const make = Effect.gen(function* () { currentBranch, input.threadId, progress.emit, + "pushRemote" in push && push.pushRemote && push.upstreamBranch + ? { remoteName: push.pushRemote, upstreamRef: push.upstreamBranch } + : undefined, ), ), ) diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index 4084e531f2302..319f1ffb86651 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -177,12 +177,16 @@ export interface GitPushResult { status: "pushed" | "skipped_up_to_date"; branch: string; upstreamBranch?: string | undefined; + /** Set when `upstreamBranch` is on a configured push remote instead of the tracked upstream. */ + pushRemote?: string | undefined; setUpstream?: boolean | undefined; } export interface GitPushTarget { remoteName: string; branchName: string; + /** The tracked upstream is current, yet pushes go to this configured push remote. */ + viaPushRemote: boolean; } export interface GitRangeContext { diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 060beadad39f7..2378fab9124d4 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -7951,6 +7951,152 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { assert.notEqual(originMain.exitCode, 0); }), ); + + it.effect.each(["branch.topic.pushRemote", "remote.pushDefault"] as const)( + "pushes a branch tracking its upstream head to the %s remote", + (configKey) => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const upstreamRemote = yield* makeTmpDir("git-upstream-remote-"); + const forkRemote = yield* makeTmpDir("git-fork-remote-"); + yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* git(upstreamRemote, ["init", "--bare"]); + yield* git(forkRemote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "upstream", upstreamRemote]); + yield* git(cwd, ["remote", "add", "fork", forkRemote]); + yield* git(cwd, ["checkout", "-b", "topic"]); + yield* git(cwd, ["push", "upstream", "topic"]); + yield* git(cwd, ["push", "fork", "topic"]); + yield* git(cwd, ["branch", "--set-upstream-to=upstream/topic"]); + yield* git(cwd, ["config", configKey, "fork"]); + const upstreamTopicSha = yield* git(cwd, ["rev-parse", "HEAD"]); + yield* writeTextFile(cwd, "fork.txt", "fork\n"); + yield* git(cwd, ["add", "-A"]); + yield* driver.commit(cwd, "Add fork update", ""); + + const pushed = yield* driver.pushCurrentBranch(cwd, null); + + assert.deepInclude(pushed, { + status: "pushed", + branch: "topic", + upstreamBranch: "fork/topic", + pushRemote: "fork", + setUpstream: false, + }); + assert.equal( + yield* git(forkRemote, ["--git-dir=.", "log", "-1", "--pretty=%s", "topic"]), + "Add fork update", + ); + assert.equal( + yield* git(upstreamRemote, ["--git-dir=.", "rev-parse", "topic"]), + upstreamTopicSha, + ); + assert.equal( + yield* git(cwd, ["rev-parse", "--abbrev-ref", "@{upstream}"]), + "upstream/topic", + ); + }), + ); + + it.effect.each(["deleted", "rewound"] as const)( + "pushes an up-to-date tracked branch to the push remote when its branch there is %s", + (forkState) => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const upstreamRemote = yield* makeTmpDir("git-upstream-remote-"); + const forkRemote = yield* makeTmpDir("git-fork-remote-"); + yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* git(upstreamRemote, ["init", "--bare"]); + yield* git(forkRemote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "upstream", upstreamRemote]); + yield* git(cwd, ["remote", "add", "fork", forkRemote]); + yield* git(cwd, ["checkout", "-b", "topic"]); + yield* writeTextFile(cwd, "upstream.txt", "upstream\n"); + yield* git(cwd, ["add", "-A"]); + yield* driver.commit(cwd, "Add upstream update", ""); + yield* git(cwd, ["push", "-u", "upstream", "topic"]); + yield* git(cwd, ["config", "remote.pushDefault", "fork"]); + // Another checkout changes the fork after this one last fetched it, + // leaving a local fork/topic that still contains HEAD. + yield* git(cwd, ["push", "fork", "topic"]); + yield* git( + forkRemote, + forkState === "deleted" + ? ["--git-dir=.", "update-ref", "-d", "refs/heads/topic"] + : ["--git-dir=.", "update-ref", "refs/heads/topic", "topic~1"], + ); + + const pushed = yield* driver.pushCurrentBranch(cwd, null); + + assert.deepInclude(pushed, { + status: "pushed", + upstreamBranch: "fork/topic", + pushRemote: "fork", + }); + assert.equal( + yield* git(forkRemote, ["--git-dir=.", "log", "-1", "--pretty=%s", "topic"]), + "Add upstream update", + ); + assert.equal( + yield* git(cwd, ["rev-parse", "--abbrev-ref", "@{upstream}"]), + "upstream/topic", + ); + }), + ); + + it.effect("skips an up-to-date tracked branch the push remote already has", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const upstreamRemote = yield* makeTmpDir("git-upstream-remote-"); + const forkRemote = yield* makeTmpDir("git-fork-remote-"); + yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* git(upstreamRemote, ["init", "--bare"]); + yield* git(forkRemote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "upstream", upstreamRemote]); + yield* git(cwd, ["remote", "add", "fork", forkRemote]); + yield* git(cwd, ["checkout", "-b", "topic"]); + yield* git(cwd, ["push", "-u", "upstream", "topic"]); + yield* git(cwd, ["push", "fork", "topic"]); + yield* git(cwd, ["config", "remote.pushDefault", "fork"]); + + const pushed = yield* driver.pushCurrentBranch(cwd, null); + + assert.deepInclude(pushed, { + status: "skipped_up_to_date", + upstreamBranch: "fork/topic", + pushRemote: "fork", + }); + }), + ); + + it.effect("skips an up-to-date branch whose differently named upstream is its base", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const upstreamRemote = yield* makeTmpDir("git-upstream-remote-"); + const forkRemote = yield* makeTmpDir("git-fork-remote-"); + yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* git(upstreamRemote, ["init", "--bare"]); + yield* git(forkRemote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "upstream", upstreamRemote]); + yield* git(cwd, ["remote", "add", "fork", forkRemote]); + yield* git(cwd, ["push", "upstream", "HEAD:refs/heads/release"]); + yield* git(cwd, ["fetch", "upstream"]); + yield* git(cwd, ["checkout", "-b", "topic", "--track", "upstream/release"]); + yield* git(cwd, ["config", "remote.pushDefault", "fork"]); + + const pushed = yield* driver.pushCurrentBranch(cwd, null); + + assert.deepInclude(pushed, { + status: "skipped_up_to_date", + upstreamBranch: "upstream/release", + }); + assert.equal(yield* git(forkRemote, ["--git-dir=.", "branch", "--list", "topic"]), ""); + }), + ); }); }); diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 2a0c6772d8d28..abdd21614f683 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -1816,7 +1816,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ), ); - const resolvePushRemoteName = Effect.fn("resolvePushRemoteName")(function* ( + const resolveConfiguredPushRemoteName = Effect.fn("resolveConfiguredPushRemoteName")(function* ( cwd: string, refName: string, ) { @@ -1836,13 +1836,32 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ["config", "--get", "remote.pushDefault"], true, ).pipe(Effect.map((stdout) => stdout.trim())); - if (pushDefaultRemote.length > 0) { - return pushDefaultRemote; + return pushDefaultRemote.length > 0 ? pushDefaultRemote : null; + }); + + const resolvePushRemoteName = Effect.fn("resolvePushRemoteName")(function* ( + cwd: string, + refName: string, + ) { + const configuredRemote = yield* resolveConfiguredPushRemoteName(cwd, refName); + if (configuredRemote) { + return configuredRemote; } return yield* resolvePrimaryRemoteName(cwd).pipe(Effect.orElseSucceed(() => null)); }); + const remoteRefContainsHead = (cwd: string, remoteRef: string) => + executeGit( + "GitVcsDriver.pushCurrentBranch.publishContainsHead", + cwd, + ["merge-base", "--is-ancestor", "HEAD", `refs/remotes/${remoteRef}`], + { allowNonZeroExit: true }, + ).pipe( + Effect.map((result) => result.exitCode === 0), + Effect.orElseSucceed(() => false), + ); + const ensureRemote: GitVcsDriver.GitVcsDriver["Service"]["ensureRemote"] = Effect.fn( "ensureRemote", )(function* (input) { @@ -2843,7 +2862,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const remoteName = yield* resolvePushRemoteName(cwd, branch); if (!remoteName) return null; const branchName = yield* resolvePublishBranchName(cwd, branch); - return { remoteName, branchName, publishes: true, upstream: null }; + return { remoteName, branchName, publishes: true, upstream: null, pushRemote: null }; } if ((yield* readConfigValue(cwd, `branch.${branch}.remote`)) === ".") { @@ -2861,6 +2880,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* branchName: branch, publishes: true, upstream: { remoteName: ".", branchName: localUpstream, upstreamRef: localUpstream }, + pushRemote: null, }; } @@ -2871,11 +2891,20 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* branch === upstream.branchName || (branch.endsWith(`/${upstream.branchName}`) && upstream.upstreamRef.endsWith(`/${branch}`)); if (isAliasOfUpstreamHead) { + // A fork workflow pushes to the configured push remote and keeps the upstream for pulls, as plain `git push` does. + const configuredPushRemote = yield* resolveConfiguredPushRemoteName(cwd, branch).pipe( + Effect.orElseSucceed(() => null), + ); + const pushRemote = + configuredPushRemote && configuredPushRemote !== upstream.remoteName + ? configuredPushRemote + : null; return { - remoteName: upstream.remoteName, + remoteName: pushRemote ?? upstream.remoteName, branchName: upstream.branchName, publishes: false, upstream, + pushRemote, }; } const publishRemoteName = yield* resolvePushRemoteName(cwd, branch).pipe( @@ -2887,6 +2916,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* branchName, publishes: true, upstream, + pushRemote: null, }; }); @@ -2895,7 +2925,11 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* )(function* (cwd, current) { const destination = yield* resolvePushDestination(cwd, current.branch, current.hasUpstream); return destination - ? { remoteName: destination.remoteName, branchName: destination.branchName } + ? { + remoteName: destination.remoteName, + branchName: destination.branchName, + viaPushRemote: destination.pushRemote !== null, + } : null; }); @@ -2933,16 +2967,35 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* } const hasNoLocalDelta = details.aheadCount === 0 && details.behindCount === 0; - if (hasNoLocalDelta && details.hasUpstream) { - return { - status: "skipped_up_to_date" as const, - branch, - ...(details.upstreamRef ? { upstreamBranch: details.upstreamRef } : {}), - }; - } - const destination = yield* resolvePushDestination(cwd, branch, details.hasUpstream); - if (hasNoLocalDelta) { + if (hasNoLocalDelta && details.hasUpstream) { + if (!destination?.pushRemote) { + return { + status: "skipped_up_to_date" as const, + branch, + ...(details.upstreamRef ? { upstreamBranch: details.upstreamRef } : {}), + }; + } + const { pushRemote, branchName } = destination; + const pushRemoteRef = `${pushRemote}/${branchName}`; + // The local ref can be stale when the branch was deleted or rewound elsewhere. + const pushRemoteHasHead = yield* fetchRemoteTrackingBranch({ + cwd, + remoteName: pushRemote, + remoteBranch: branchName, + }).pipe( + Effect.andThen(remoteRefContainsHead(cwd, pushRemoteRef)), + Effect.orElseSucceed(() => false), + ); + if (pushRemoteHasHead) { + return { + status: "skipped_up_to_date" as const, + branch, + upstreamBranch: pushRemoteRef, + pushRemote, + }; + } + } else if (hasNoLocalDelta) { const comparableBaseBranch = yield* resolveBaseBranchForNoUpstream(cwd, branch).pipe( Effect.orElseSucceed(() => null), ); @@ -2955,21 +3008,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* } // The configured diff base can contain HEAD while the publish branch is still behind. - const publishContainsHead = yield* executeGit( - "GitVcsDriver.pushCurrentBranch.publishContainsHead", - cwd, - [ - "merge-base", - "--is-ancestor", - "HEAD", - `refs/remotes/${destination.remoteName}/${destination.branchName}`, - ], - { allowNonZeroExit: true }, - ).pipe( - Effect.map((result) => result.exitCode === 0), - Effect.orElseSucceed(() => false), - ); - if (publishContainsHead) { + if ( + yield* remoteRefContainsHead(cwd, `${destination.remoteName}/${destination.branchName}`) + ) { return { status: "skipped_up_to_date" as const, branch, @@ -3038,7 +3079,12 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* return { status: "pushed" as const, branch, - upstreamBranch: destination.upstream.upstreamRef, + ...(destination.pushRemote + ? { + upstreamBranch: `${destination.remoteName}/${destination.branchName}`, + pushRemote: destination.pushRemote, + } + : { upstreamBranch: destination.upstream.upstreamRef }), setUpstream: false, }; }