diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 634898612b077..72b3504807c93 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -3503,6 +3503,125 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { }), ); + it.effect.each([ + { localBranch: "develop" }, + // Pushes to origin's develop under a name that differs from the base. + { localBranch: "origin/develop" }, + ] as const)( + "commit_push_pr refuses to commit or push on the default branch when origin/HEAD is missing ($localBranch)", + ({ localBranch }) => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + yield* runGit(repoDir, ["branch", "-m", "main", "develop"]); + const remoteDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "develop"]); + if (localBranch !== "develop") yield* runGit(repoDir, ["checkout", "-b", localBranch]); + const readRemoteDevelop = runGit(repoDir, [ + "ls-remote", + "origin", + "refs/heads/develop", + ]).pipe(Effect.map((output) => output.stdout)); + const remoteBefore = yield* readRemoteDevelop; + const localBefore = (yield* runGit(repoDir, ["rev-parse", "HEAD"])).stdout; + NodeFS.writeFileSync(NodePath.join(repoDir, "change.txt"), "change\n"); + + const { manager } = yield* makeManager({ + providerScenario: { defaultBranch: "develop", prListSequence: [[], []] }, + }); + + const error = yield* runStackedAction(manager, { + cwd: repoDir, + action: "commit_push_pr", + }).pipe(Effect.flip); + + expect(yield* readRemoteDevelop).toBe(remoteBefore); + expect((yield* runGit(repoDir, ["rev-parse", "HEAD"])).stdout).toBe(localBefore); + expect(error.message).toContain("develop is the base branch"); + }), + ); + + it.effect("commit_push_pr on the default branch returns its open PR into another base", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const remoteDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "main"]); + const releasePr = { + number: 43, + title: "Release", + url: "https://github.com/pingdotgg/codething-mvp/pull/43", + baseRefName: "release", + headRefName: "main", + }; + + const { manager } = yield* makeManager({ + providerScenario: { defaultBranch: "main", prListSequence: [[releasePr], [releasePr]] }, + }); + const result = yield* runStackedAction(manager, { + cwd: repoDir, + action: "commit_push_pr", + }); + + expect(result.pr.status).toBe("opened_existing"); + expect(result.pr.number).toBe(43); + }), + ); + + it.effect( + "commit_push_pr publishes a branch that tracks the default branch under its own name", + () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + yield* runGit(repoDir, ["branch", "-m", "main", "develop"]); + const remoteDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "develop"]); + yield* runGit(repoDir, ["checkout", "-b", "t3code/feature", "--track", "origin/develop"]); + const readRemoteDevelop = runGit(repoDir, [ + "ls-remote", + "origin", + "refs/heads/develop", + ]).pipe(Effect.map((output) => output.stdout)); + const remoteBefore = yield* readRemoteDevelop; + NodeFS.writeFileSync(NodePath.join(repoDir, "change.txt"), "change\n"); + + const { manager, providerCalls } = yield* makeManager({ + providerScenario: { + defaultBranch: "develop", + prListSequence: [ + [], + [ + { + number: 506, + title: "Feature", + url: "https://github.com/pingdotgg/codething-mvp/pull/506", + baseRefName: "develop", + headRefName: "t3code/feature", + }, + ], + ], + }, + }); + + const result = yield* runStackedAction(manager, { + cwd: repoDir, + action: "commit_push_pr", + }); + + expect(result.pr.status).toBe("created"); + expect(yield* readRemoteDevelop).toBe(remoteBefore); + expect( + created(providerCalls).some( + (call) => call.baseRefName === "develop" && call.headSelector === "t3code/feature", + ), + ).toBe(true); + }), + ); + it.effect("create_pr reports a created PR when the lookup after creation fails", () => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); @@ -4355,6 +4474,74 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { }), ); + it.effect("commit_push_pr publishes the default branch to the push remote's fork", () => + 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", "origin", "main"]); + yield* runGit(repoDir, ["remote", "add", "fork", forkDir]); + yield* runGit(repoDir, ["push", "fork", "main"]); + yield* runGit(repoDir, ["config", "remote.pushDefault", "fork"]); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "origin", + "git@github.com:pingdotgg/codething-mvp.git", + originDir, + ); + yield* configureVisibleRemoteUrlWithLocalRewrite( + repoDir, + "fork", + "git@github.com:octocat/codething-mvp.git", + forkDir, + ); + const readOriginMain = runGit(repoDir, ["ls-remote", "origin", "refs/heads/main"]).pipe( + Effect.map((output) => output.stdout), + ); + const originBefore = yield* readOriginMain; + NodeFS.writeFileSync(NodePath.join(repoDir, "change.txt"), "change\n"); + + const { manager, providerCalls } = yield* makeManager({ + providerScenario: { + defaultBranch: "main", + prListSequenceByHeadSelector: { + "octocat:main": [ + [], + [ + { + number: 189, + title: "Contribute from main", + url: "https://github.com/pingdotgg/codething-mvp/pull/189", + baseRefName: "main", + headRefName: "main", + state: "open", + isCrossRepository: true, + headRepositoryNameWithOwner: "octocat/codething-mvp", + headRepositoryOwnerLogin: "octocat", + }, + ], + ], + }, + }, + }); + + const result = yield* runStackedAction(manager, { + cwd: repoDir, + action: "commit_push_pr", + }); + + expect(result.pr.status).toBe("created"); + expect(yield* readOriginMain).toBe(originBefore); + expect( + created(providerCalls).some( + (call) => call.baseRefName === "main" && call.headSelector === "octocat:main", + ), + ).toBe(true); + }), + ); + 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 e6b4bdcfe4230..d9e1d7d4c09e1 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -2282,6 +2282,34 @@ export const make = Effect.gen(function* () { }; }); + const ensurePrHeadIsNotBase = Effect.fn("ensurePrHeadIsNotBase")(function* ( + cwd: string, + status: { branch: string; upstreamRef: string | null; hasUpstream: boolean }, + changeRequestLabel: string, + ) { + const target = yield* gitCore.resolvePushTarget(cwd, status); + if (!target) return; + const headContext = yield* resolveBranchHeadContext(cwd, { + branch: status.branch, + upstreamRef: `${target.remoteName}/${target.branchName}`, + remoteName: target.remoteName, + }); + if (headContext.isCrossRepository) return; + const { baseBranch } = yield* resolvePrBase( + cwd, + status.branch, + status.upstreamRef, + headContext, + ); + if (baseBranch !== target.branchName) return; + if (yield* findOpenPr(cwd, headContext)) return; + return yield* new GitManagerError({ + operation: "runStackedAction", + cwd, + detail: `${baseBranch} is the base branch. Check out a feature branch to create ${changeRequestLabel}.`, + }); + }); + const runPrStep = Effect.fn("runPrStep")(function* ( settings: SourceControlTextGenerationSettings, cwd: string, @@ -3251,6 +3279,15 @@ export const make = Effect.gen(function* () { const currentBranch = branchStep.name ?? initialStatus.branch; const commitAction = isCommitAction(input.action) ? input.action : null; const changeRequestTerms = wantsPr ? yield* changeRequestTermsFor(input.cwd) : null; + // A stale or missing origin/HEAD hides the default branch from status, so check here + // before anything is committed or pushed to it. + if (changeRequestTerms && !input.featureBranch && initialStatus.branch) { + yield* ensurePrHeadIsNotBase( + input.cwd, + { ...initialStatus, branch: initialStatus.branch }, + changeRequestTerms.shortLabelWithArticle, + ); + } const commit = commitAction ? yield* Ref.set(currentPhase, Option.some("commit")).pipe( diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index 59db351198182..78f428901a48b 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -181,6 +181,11 @@ export interface GitPushResult { setUpstream?: boolean | undefined; } +export interface GitPushTarget { + remoteName: string; + branchName: string; +} + export interface GitRangeContext { commitSummary: string; diffSummary: string; @@ -339,6 +344,11 @@ export class GitVcsDriver extends Context.Service< fallbackBranch: string | null, options?: { readonly remoteName?: string | null }, ) => Effect.Effect; + /** Where `pushCurrentBranch` would push the checked-out branch; null with no remote or when Git picks. */ + readonly resolvePushTarget: ( + cwd: string, + current: { readonly branch: string; readonly hasUpstream: boolean }, + ) => Effect.Effect; readonly readRangeContext: ( cwd: string, baseRef: string, diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 4fcf1b05fe6f5..ef91660a0e9a7 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -2663,6 +2663,54 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ), ); + // `publishes` is false only when HEAD goes to the head of its current `upstream`. + const resolvePushDestination = Effect.fn("resolvePushDestination")(function* ( + cwd: string, + branch: string, + hasUpstream: boolean, + ) { + if (!hasUpstream) { + const remoteName = yield* resolvePushRemoteName(cwd, branch); + if (!remoteName) return null; + const branchName = yield* resolvePublishBranchName(cwd, branch); + return { remoteName, branchName, publishes: true, upstream: null }; + } + + const upstream = yield* resolveCurrentUpstream(cwd).pipe(Effect.orElseSucceed(() => null)); + if (!upstream) return null; + // Differently named upstreams are bases, not publish targets; pushing HEAD there would write features to shared branches. + const isAliasOfUpstreamHead = + branch === upstream.branchName || + (branch.endsWith(`/${upstream.branchName}`) && upstream.upstreamRef.endsWith(`/${branch}`)); + if (isAliasOfUpstreamHead) { + return { + remoteName: upstream.remoteName, + branchName: upstream.branchName, + publishes: false, + upstream, + }; + } + const publishRemoteName = yield* resolvePushRemoteName(cwd, branch).pipe( + Effect.orElseSucceed(() => null), + ); + const branchName = yield* resolvePublishBranchName(cwd, branch); + return { + remoteName: publishRemoteName ?? upstream.remoteName, + branchName, + publishes: true, + upstream, + }; + }); + + const resolvePushTarget: GitVcsDriver.GitVcsDriver["Service"]["resolvePushTarget"] = Effect.fn( + "resolvePushTarget", + )(function* (cwd, current) { + const destination = yield* resolvePushDestination(cwd, current.branch, current.hasUpstream); + return destination + ? { remoteName: destination.remoteName, branchName: destination.branchName } + : null; + }); + const pushCurrentBranch: GitVcsDriver.GitVcsDriver["Service"]["pushCurrentBranch"] = Effect.fn( "pushCurrentBranch", )(function* (cwd, fallbackBranch, options) { @@ -2697,30 +2745,27 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* } const hasNoLocalDelta = details.aheadCount === 0 && details.behindCount === 0; - if (hasNoLocalDelta) { - if (details.hasUpstream) { - return { - status: "skipped_up_to_date" as const, - branch, - ...(details.upstreamRef ? { upstreamBranch: details.upstreamRef } : {}), - }; - } + 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) { const comparableBaseBranch = yield* resolveBaseBranchForNoUpstream(cwd, branch).pipe( Effect.orElseSucceed(() => null), ); if (comparableBaseBranch) { - const publishRemoteName = yield* resolvePushRemoteName(cwd, branch).pipe( - Effect.orElseSucceed(() => null), - ); - if (!publishRemoteName) { + if (!destination) { return { status: "skipped_up_to_date" as const, branch, }; } - const publishBranch = yield* resolvePublishBranchName(cwd, branch); // The configured diff base can contain HEAD while the publish branch is still behind. const publishContainsHead = yield* executeGit( "GitVcsDriver.pushCurrentBranch.publishContainsHead", @@ -2729,7 +2774,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* "merge-base", "--is-ancestor", "HEAD", - `refs/remotes/${publishRemoteName}/${publishBranch}`, + `refs/remotes/${destination.remoteName}/${destination.branchName}`, ], { allowNonZeroExit: true }, ).pipe( @@ -2745,87 +2790,61 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* } } - if (!details.hasUpstream) { - const publishRemoteName = yield* resolvePushRemoteName(cwd, branch); - if (!publishRemoteName) { - return yield* new GitCommandError({ - ...gitCommandContext({ - operation: "GitVcsDriver.pushCurrentBranch", - cwd, - args: ["push"], - }), - detail: "Cannot push because no git remote is configured for this repository.", + if (!details.hasUpstream && !destination) { + return yield* new GitCommandError({ + ...gitCommandContext({ + operation: "GitVcsDriver.pushCurrentBranch", + cwd, + args: ["push"], + }), + detail: "Cannot push because no git remote is configured for this repository.", + }); + } + + if (destination?.publishes) { + const { remoteName, branchName: publishBranch, upstream } = destination; + // Publishing retargets the upstream, so preserve its comparison base first. + if (upstream && (yield* readBranchBase(cwd, branch)) === null) { + yield* writeConfigValue({ + cwd, + key: branchBaseConfigKey(branch), + value: upstream.branchName, }); + if (!(yield* readConfigValue(cwd, `branch.${branch}.t3-merge-base-ref`))) { + yield* writeConfigValue({ + cwd, + key: `branch.${branch}.t3-merge-base-ref`, + value: `refs/remotes/${upstream.upstreamRef}`, + }); + } } - const publishBranch = yield* resolvePublishBranchName(cwd, branch); yield* runGit( - "GitVcsDriver.pushCurrentBranch.pushWithUpstream", + upstream + ? "GitVcsDriver.pushCurrentBranch.pushOwnBranch" + : "GitVcsDriver.pushCurrentBranch.pushWithUpstream", cwd, - ["push", "--", publishRemoteName, `HEAD:refs/heads/${publishBranch}`], + ["push", "--", remoteName, `HEAD:refs/heads/${publishBranch}`], { timeoutMs: null }, ); return { status: "pushed" as const, branch, - upstreamBranch: `${publishRemoteName}/${publishBranch}`, - setUpstream: yield* recordPushedUpstream(cwd, branch, publishRemoteName, publishBranch), + upstreamBranch: `${remoteName}/${publishBranch}`, + setUpstream: yield* recordPushedUpstream(cwd, branch, remoteName, publishBranch), }; } - const currentUpstream = yield* resolveCurrentUpstream(cwd).pipe( - Effect.orElseSucceed(() => null), - ); - if (currentUpstream) { - // Differently named upstreams are bases, not publish targets; pushing HEAD there would write features to shared branches. - const isAliasOfUpstreamHead = - branch === currentUpstream.branchName || - (branch.endsWith(`/${currentUpstream.branchName}`) && - currentUpstream.upstreamRef.endsWith(`/${branch}`)); - if (!isAliasOfUpstreamHead) { - const publishRemoteName = yield* resolvePushRemoteName(cwd, branch).pipe( - Effect.orElseSucceed(() => null), - ); - const remoteName = publishRemoteName ?? currentUpstream.remoteName; - const publishBranch = yield* resolvePublishBranchName(cwd, branch); - // Publishing retargets the upstream, so preserve its comparison base first. - if ((yield* readBranchBase(cwd, branch)) === null) { - yield* writeConfigValue({ - cwd, - key: branchBaseConfigKey(branch), - value: currentUpstream.branchName, - }); - if (!(yield* readConfigValue(cwd, `branch.${branch}.t3-merge-base-ref`))) { - yield* writeConfigValue({ - cwd, - key: `branch.${branch}.t3-merge-base-ref`, - value: `refs/remotes/${currentUpstream.upstreamRef}`, - }); - } - } - yield* runGit( - "GitVcsDriver.pushCurrentBranch.pushOwnBranch", - cwd, - ["push", "--", remoteName, `HEAD:refs/heads/${publishBranch}`], - { timeoutMs: null }, - ); - return { - status: "pushed" as const, - branch, - upstreamBranch: `${remoteName}/${publishBranch}`, - setUpstream: yield* recordPushedUpstream(cwd, branch, remoteName, publishBranch), - }; - } - + if (destination?.upstream) { yield* runGit( "GitVcsDriver.pushCurrentBranch.pushUpstream", cwd, - ["push", "--", currentUpstream.remoteName, `HEAD:refs/heads/${currentUpstream.branchName}`], + ["push", "--", destination.remoteName, `HEAD:refs/heads/${destination.branchName}`], { timeoutMs: null }, ); return { status: "pushed" as const, branch, - upstreamBranch: currentUpstream.upstreamRef, + upstreamBranch: destination.upstream.upstreamRef, setUpstream: false, }; } @@ -5104,6 +5123,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* commit: Effect.fnUntraced(function* (cwd, subject, body, options) { return yield* withListRefsInvalidation(cwd, commit(cwd, subject, body, options)); }), + resolvePushTarget, pushCurrentBranch: Effect.fnUntraced(function* (cwd, fallbackBranch, options) { return yield* withListRefsInvalidation(cwd, pushCurrentBranch(cwd, fallbackBranch, options)); }),