diff --git a/apps/server/src/git/Layers/GitManager.test.ts b/apps/server/src/git/Layers/GitManager.test.ts index ef9221b4f..36587167b 100644 --- a/apps/server/src/git/Layers/GitManager.test.ts +++ b/apps/server/src/git/Layers/GitManager.test.ts @@ -491,6 +491,7 @@ function runStackedAction( actionId?: string; commitMessage?: string; featureBranch?: boolean; + rebaseBeforeCommit?: boolean; filePaths?: readonly string[]; }, options?: Parameters[1], @@ -780,6 +781,47 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { }), ); + it.effect("rebases onto the latest default branch before commit when enabled", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("okcode-git-manager-"); + yield* initRepo(repoDir); + const remoteDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "main"]); + yield* runGit(repoDir, ["checkout", "-b", "feature/rebase-before-commit"]); + + const updaterDir = yield* makeTempDir("okcode-git-manager-updater-"); + yield* runGit(updaterDir, ["clone", remoteDir, "."]); + yield* runGit(updaterDir, ["config", "user.email", "test@example.com"]); + yield* runGit(updaterDir, ["config", "user.name", "Test User"]); + fs.writeFileSync(path.join(updaterDir, "base.txt"), "remote main update\n"); + yield* runGit(updaterDir, ["add", "base.txt"]); + yield* runGit(updaterDir, ["commit", "-m", "Remote main update"]); + yield* runGit(updaterDir, ["push", "origin", "main"]); + + fs.writeFileSync(path.join(repoDir, "README.md"), "hello\nrebased feature work\n"); + + const { manager } = yield* makeManager(); + const result = yield* runStackedAction(manager, { + cwd: repoDir, + action: "commit", + commitMessage: "feat: rebase before commit", + rebaseBeforeCommit: true, + }); + + expect(result.commit.status).toBe("created"); + + const remoteMainSha = yield* runGit(repoDir, ["rev-parse", "origin/main"]).pipe( + Effect.map((gitResult) => gitResult.stdout.trim()), + ); + const mergeBase = yield* runGit(repoDir, ["merge-base", "HEAD", "origin/main"]).pipe( + Effect.map((gitResult) => gitResult.stdout.trim()), + ); + + expect(mergeBase).toBe(remoteMainSha); + }), + ); + it.effect("uses custom commit message when provided", () => Effect.gen(function* () { const repoDir = yield* makeTempDir("okcode-git-manager-"); diff --git a/apps/server/src/git/Layers/GitManager.ts b/apps/server/src/git/Layers/GitManager.ts index 5c217c485..59ad1e3f0 100644 --- a/apps/server/src/git/Layers/GitManager.ts +++ b/apps/server/src/git/Layers/GitManager.ts @@ -283,6 +283,13 @@ function appendUnique(values: string[], next: string | null | undefined): void { values.push(trimmed); } +function parseRemoteNames(stdout: string): string[] { + return stdout + .split("\n") + .map((line) => line.trim()) + .filter((line) => line.length > 0); +} + function toStatusPr(pr: PullRequestInfo): { number: number; title: string; @@ -678,6 +685,141 @@ export const makeGitManager = Effect.gen(function* () { return "main"; }); + const resolvePreferredRemoteName = (cwd: string, branch: string) => + Effect.gen(function* () { + const configuredRemote = yield* gitCore + .readConfigValue(cwd, `branch.${branch}.remote`) + .pipe(Effect.catch(() => Effect.succeed(null))); + if (configuredRemote && configuredRemote !== ".") { + return configuredRemote; + } + + const remoteResult = yield* gitCore + .execute({ + operation: "GitManager.resolvePreferredRemoteName", + cwd, + args: ["remote"], + allowNonZeroExit: true, + }) + .pipe(Effect.catch(() => Effect.succeed(null))); + const remoteNames = remoteResult ? parseRemoteNames(remoteResult.stdout) : []; + if (remoteNames.includes("origin")) { + return "origin"; + } + + return remoteNames[0] ?? null; + }); + + const resolvePreCommitRebaseTarget = (cwd: string, branch: string) => + Effect.gen(function* () { + const branchList = yield* gitCore.listBranches({ cwd }); + const localDefaultBranch = + branchList.branches.find((candidate) => !candidate.isRemote && candidate.isDefault)?.name ?? + null; + const defaultFromGh = yield* gitHubCli + .getDefaultBranch({ cwd }) + .pipe(Effect.catch(() => Effect.succeed(null))); + const preferredRemote = yield* resolvePreferredRemoteName(cwd, branch); + const baseBranchCandidates = [ + ...new Set( + [localDefaultBranch, defaultFromGh, "main"].filter( + (candidate): candidate is string => + typeof candidate === "string" && candidate.length > 0, + ), + ), + ]; + + for (const baseBranch of baseBranchCandidates) { + if (preferredRemote) { + const fetchResult = yield* gitCore + .execute({ + operation: "GitManager.resolvePreCommitRebaseTarget.fetch", + cwd, + args: [ + "fetch", + "--quiet", + "--no-tags", + preferredRemote, + `+refs/heads/${baseBranch}:refs/remotes/${preferredRemote}/${baseBranch}`, + ], + allowNonZeroExit: true, + timeoutMs: 30_000, + }) + .pipe(Effect.catch(() => Effect.succeed(null))); + if (fetchResult?.code === 0) { + return { + baseBranch, + targetRef: `${preferredRemote}/${baseBranch}`, + }; + } + } + + const localBranchExists = yield* gitCore + .execute({ + operation: "GitManager.resolvePreCommitRebaseTarget.localBranchExists", + cwd, + args: ["show-ref", "--verify", "--quiet", `refs/heads/${baseBranch}`], + allowNonZeroExit: true, + timeoutMs: 5_000, + }) + .pipe( + Effect.map((result) => result.code === 0), + Effect.catch(() => Effect.succeed(false)), + ); + if (localBranchExists) { + return { + baseBranch, + targetRef: baseBranch, + }; + } + } + + return null; + }); + + const runRebaseBeforeCommitStep = (cwd: string, branch: string) => + Effect.gen(function* () { + const target = yield* resolvePreCommitRebaseTarget(cwd, branch); + if (!target) { + return yield* gitManagerError( + "runRebaseBeforeCommitStep", + `Could not resolve a repository default branch to rebase '${branch}' onto before committing.`, + ); + } + + const rebaseResult = yield* gitCore.execute({ + operation: "GitManager.runRebaseBeforeCommitStep", + cwd, + args: ["rebase", "--autostash", target.targetRef], + allowNonZeroExit: true, + timeoutMs: 60_000, + }); + if (rebaseResult.code === 0) { + return target; + } + + const refreshed = yield* gitCore + .statusDetails(cwd) + .pipe(Effect.catch(() => Effect.succeed(null))); + const commandDetail = + rebaseResult.stderr.trim().length > 0 + ? rebaseResult.stderr.trim() + : rebaseResult.stdout.trim().length > 0 + ? rebaseResult.stdout.trim() + : `git rebase --autostash ${target.targetRef} failed`; + if (refreshed?.hasConflicts) { + return yield* gitManagerError( + "runRebaseBeforeCommitStep", + `Rebase onto ${target.baseBranch} stopped with conflicts. Resolve the conflicts, continue or abort the rebase, then retry the git action.\n\n${commandDetail}`, + ); + } + + return yield* gitManagerError( + "runRebaseBeforeCommitStep", + `Could not rebase '${branch}' onto ${target.baseBranch} before committing.\n\n${commandDetail}`, + ); + }); + const resolveCommitAndBranchSuggestion = (input: { cwd: string; branch: string | null; @@ -1162,6 +1304,14 @@ export const makeGitManager = Effect.gen(function* () { const wantsPr = input.action === "commit_push_pr"; const initialStatus = yield* gitCore.statusDetails(input.cwd); + const shouldRebaseBeforeCommit = + input.rebaseBeforeCommit === true && initialStatus.hasWorkingTreeChanges; + if (shouldRebaseBeforeCommit && !input.featureBranch && !initialStatus.branch) { + return yield* gitManagerError( + "runStackedAction", + "Cannot rebase before commit from detached HEAD.", + ); + } if (!input.featureBranch && wantsPush && !initialStatus.branch) { return yield* gitManagerError("runStackedAction", "Cannot push from detached HEAD."); } @@ -1176,6 +1326,23 @@ export const makeGitManager = Effect.gen(function* () { let commitMessageForStep = input.commitMessage; let preResolvedCommitSuggestion: CommitAndBranchSuggestion | undefined = undefined; + if (shouldRebaseBeforeCommit && initialStatus.branch) { + currentPhase = "commit"; + const target = yield* resolvePreCommitRebaseTarget(input.cwd, initialStatus.branch); + if (!target) { + return yield* gitManagerError( + "runStackedAction", + `Could not resolve a repository default branch to rebase '${initialStatus.branch}' onto before committing.`, + ); + } + yield* progress.emit({ + kind: "phase_started", + phase: "commit", + label: `Rebasing onto ${target.baseBranch}...`, + }); + yield* runRebaseBeforeCommitStep(input.cwd, initialStatus.branch); + } + if (input.featureBranch) { currentPhase = "branch"; yield* progress.emit({ @@ -1199,6 +1366,33 @@ export const makeGitManager = Effect.gen(function* () { const currentBranch = branchStep.name ?? initialStatus.branch; + if (shouldRebaseBeforeCommit && input.featureBranch && !initialStatus.branch) { + if (!currentBranch) { + return yield* gitManagerError( + "runStackedAction", + "Cannot rebase before commit from detached HEAD.", + ); + } + currentPhase = "commit"; + const target = yield* resolvePreCommitRebaseTarget(input.cwd, currentBranch); + if (!target) { + return yield* gitManagerError( + "runStackedAction", + `Could not resolve a repository default branch to rebase '${currentBranch}' onto before committing.`, + ); + } + yield* progress.emit({ + kind: "phase_started", + phase: "commit", + label: `Rebasing onto ${target.baseBranch}...`, + }); + yield* runRebaseBeforeCommitStep(input.cwd, currentBranch); + if (!input.commitMessage?.trim()) { + commitMessageForStep = undefined; + preResolvedCommitSuggestion = undefined; + } + } + currentPhase = "commit"; const commit = yield* runCommitStep( input.cwd, diff --git a/apps/web/src/appSettings.test.ts b/apps/web/src/appSettings.test.ts index 00165f3d7..de1a4c86a 100644 --- a/apps/web/src/appSettings.test.ts +++ b/apps/web/src/appSettings.test.ts @@ -26,6 +26,12 @@ describe("AppSettingsSchema", () => { expect(settings.showAuthFailuresAsErrors).toBe(true); }); + it("defaults rebase-before-commit to disabled", () => { + const settings = Schema.decodeUnknownSync(AppSettingsSchema)({}); + + expect(settings.rebaseBeforeCommit).toBe(false); + }); + it("drops deprecated window opacity values from persisted settings", () => { const decode = Schema.decodeSync(Schema.fromJsonString(AppSettingsSchema)); diff --git a/apps/web/src/appSettings.ts b/apps/web/src/appSettings.ts index 4dbd29e66..8528e7e25 100644 --- a/apps/web/src/appSettings.ts +++ b/apps/web/src/appSettings.ts @@ -72,6 +72,7 @@ export const AppSettingsSchema = Schema.Struct({ confirmThreadDelete: Schema.Boolean.pipe(withDefaults(() => true)), autoDeleteMergedThreads: Schema.Boolean.pipe(withDefaults(() => false)), autoDeleteMergedThreadsDelayMinutes: Schema.Number.pipe(withDefaults(() => 5)), + rebaseBeforeCommit: Schema.Boolean.pipe(withDefaults(() => false)), enableAssistantStreaming: Schema.Boolean.pipe(withDefaults(() => false)), showAuthFailuresAsErrors: Schema.Boolean.pipe(withDefaults(() => true)), locale: AppLocale.pipe(withDefaults(() => DEFAULT_APP_LOCALE)), diff --git a/apps/web/src/components/GitActionsControl.logic.test.ts b/apps/web/src/components/GitActionsControl.logic.test.ts index e56d3b97d..97833ba5a 100644 --- a/apps/web/src/components/GitActionsControl.logic.test.ts +++ b/apps/web/src/components/GitActionsControl.logic.test.ts @@ -1034,6 +1034,20 @@ describe("buildGitActionProgressStages", () => { "Pushing to origin/feature/test...", ]); }); + + it("prepends a rebase stage when rebase-before-commit is enabled", () => { + const stages = buildGitActionProgressStages({ + action: "commit", + hasCustomCommitMessage: false, + hasWorkingTreeChanges: true, + rebaseBeforeCommit: true, + }); + assert.deepEqual(stages, [ + "Rebasing before commit...", + "Generating commit message...", + "Committing...", + ]); + }); }); describe("summarizeGitResult", () => { diff --git a/apps/web/src/components/GitActionsControl.logic.ts b/apps/web/src/components/GitActionsControl.logic.ts index 73ec01e35..5d24af069 100644 --- a/apps/web/src/components/GitActionsControl.logic.ts +++ b/apps/web/src/components/GitActionsControl.logic.ts @@ -78,8 +78,13 @@ export function buildGitActionProgressStages(input: { forcePushOnly?: boolean; pushTarget?: string; featureBranch?: boolean; + rebaseBeforeCommit?: boolean; }): string[] { const branchStages = input.featureBranch ? ["Preparing feature branch..."] : []; + const rebaseStages = + input.rebaseBeforeCommit && !input.forcePushOnly && input.hasWorkingTreeChanges + ? ["Rebasing before commit..."] + : []; const shouldIncludeCommitStages = !input.forcePushOnly && (input.action === "commit" || input.hasWorkingTreeChanges); const commitStages = !shouldIncludeCommitStages @@ -89,12 +94,12 @@ export function buildGitActionProgressStages(input: { : ["Generating commit message...", "Committing..."]; const pushStage = input.pushTarget ? `Pushing to ${input.pushTarget}...` : "Pushing..."; if (input.action === "commit") { - return [...branchStages, ...commitStages]; + return [...branchStages, ...rebaseStages, ...commitStages]; } if (input.action === "commit_push") { - return [...branchStages, ...commitStages, pushStage]; + return [...branchStages, ...rebaseStages, ...commitStages, pushStage]; } - return [...branchStages, ...commitStages, pushStage, "Creating PR..."]; + return [...branchStages, ...rebaseStages, ...commitStages, pushStage, "Creating PR..."]; } const withDescription = (title: string, description: string | undefined) => diff --git a/apps/web/src/components/GitActionsControl.tsx b/apps/web/src/components/GitActionsControl.tsx index 9ed685b71..70744cf55 100644 --- a/apps/web/src/components/GitActionsControl.tsx +++ b/apps/web/src/components/GitActionsControl.tsx @@ -413,7 +413,8 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions gitRunStackedActionMutationOptions({ cwd: gitCwd, queryClient, - model: settings.textGenerationModel ?? null, + textGenerationModel: settings.textGenerationModel ?? null, + rebaseBeforeCommit: settings.rebaseBeforeCommit, }), ); const pullMutation = useMutation(gitPullMutationOptions({ cwd: gitCwd, queryClient })); @@ -659,6 +660,7 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions hasWorkingTreeChanges: !!actionStatus?.hasWorkingTreeChanges, forcePushOnly: forcePushOnlyProgress, featureBranch, + rebaseBeforeCommit: settings.rebaseBeforeCommit, }); const actionId = randomUUID(); const resolvedProgressToastId = @@ -697,6 +699,7 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions action, ...(commitMessage ? { commitMessage } : {}), ...(featureBranch ? { featureBranch } : {}), + ...(settings.rebaseBeforeCommit ? { rebaseBeforeCommit: true } : {}), ...(filePaths ? { filePaths } : {}), }); diff --git a/apps/web/src/lib/gitReactQuery.ts b/apps/web/src/lib/gitReactQuery.ts index f06ddac73..e8a6069f6 100644 --- a/apps/web/src/lib/gitReactQuery.ts +++ b/apps/web/src/lib/gitReactQuery.ts @@ -144,7 +144,8 @@ export function gitCheckoutMutationOptions(input: { export function gitRunStackedActionMutationOptions(input: { cwd: string | null; queryClient: QueryClient; - model?: string | null; + textGenerationModel?: string | null; + rebaseBeforeCommit?: boolean; }) { return mutationOptions({ mutationKey: gitMutationKeys.runStackedAction(input.cwd), @@ -153,12 +154,14 @@ export function gitRunStackedActionMutationOptions(input: { action, commitMessage, featureBranch, + rebaseBeforeCommit, filePaths, }: { actionId: string; action: GitStackedAction; commitMessage?: string; featureBranch?: boolean; + rebaseBeforeCommit?: boolean; filePaths?: string[]; }) => { const api = ensureNativeApi(); @@ -169,8 +172,11 @@ export function gitRunStackedActionMutationOptions(input: { action, ...(commitMessage ? { commitMessage } : {}), ...(featureBranch ? { featureBranch } : {}), + ...(rebaseBeforeCommit ? { rebaseBeforeCommit } : {}), ...(filePaths ? { filePaths } : {}), - ...(input.model ? { model: input.model } : {}), + ...(input.textGenerationModel + ? { textGenerationModel: input.textGenerationModel } + : {}), }); }, onSettled: async () => { diff --git a/apps/web/src/routes/_chat.settings.tsx b/apps/web/src/routes/_chat.settings.tsx index 8662b5e44..851824c6f 100644 --- a/apps/web/src/routes/_chat.settings.tsx +++ b/apps/web/src/routes/_chat.settings.tsx @@ -439,6 +439,9 @@ function SettingsRouteView() { defaults.autoDeleteMergedThreadsDelayMinutes ? ["Auto-delete delay"] : []), + ...(settings.rebaseBeforeCommit !== defaults.rebaseBeforeCommit + ? ["Rebase before commit"] + : []), ...(isGitTextGenerationModelDirty ? ["Git writing model"] : []), ...(settings.customCodexModels.length > 0 || settings.customClaudeModels.length > 0 ? ["Custom models"] @@ -1453,6 +1456,36 @@ function SettingsRouteView() { + + + updateSettings({ + rebaseBeforeCommit: defaults.rebaseBeforeCommit, + }) + } + /> + ) : null + } + control={ + + updateSettings({ + rebaseBeforeCommit: Boolean(checked), + }) + } + aria-label="Rebase onto the default branch before committing" + /> + } + /> + + { actionId: "action-1", cwd: "/repo", action: "commit", + rebaseBeforeCommit: true, }); expect(parsed.actionId).toBe("action-1"); expect(parsed.action).toBe("commit"); + expect(parsed.rebaseBeforeCommit).toBe(true); }); }); diff --git a/packages/contracts/src/git.ts b/packages/contracts/src/git.ts index 6ae704c95..89261336c 100644 --- a/packages/contracts/src/git.ts +++ b/packages/contracts/src/git.ts @@ -125,6 +125,7 @@ export const GitRunStackedActionInput = Schema.Struct({ action: GitStackedAction, commitMessage: Schema.optional(TrimmedNonEmptyStringSchema.check(Schema.isMaxLength(10_000))), featureBranch: Schema.optional(Schema.Boolean), + rebaseBeforeCommit: Schema.optional(Schema.Boolean), filePaths: Schema.optional( Schema.Array(TrimmedNonEmptyStringSchema).check(Schema.isMinLength(1)), ),