From b2be7f4e4f0aed3c4b03e3986dd3507c501a4abb Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 2 Mar 2026 19:40:55 -0800 Subject: [PATCH] Address PR feedback on git workflows Co-authored-by: codex --- apps/server/src/git/Layers/GitManager.test.ts | 18 +++++++++- apps/server/src/git/Layers/GitManager.ts | 36 +++++++++++++++---- .../GitActionsControl.logic.test.ts | 5 +++ apps/web/src/components/GitActionsControl.tsx | 7 ++-- apps/web/src/lib/gitReactQuery.ts | 20 ----------- packages/shared/src/git.ts | 2 +- 6 files changed, 56 insertions(+), 32 deletions(-) diff --git a/apps/server/src/git/Layers/GitManager.test.ts b/apps/server/src/git/Layers/GitManager.test.ts index 6f0917a0e6ad8a3015f2b4c692f9274c948f0f10..d8a3753bb0653c9e178dd07dbccd6a9c6c6d48ee 100644 --- a/apps/server/src/git/Layers/GitManager.test.ts +++ b/apps/server/src/git/Layers/GitManager.test.ts @@ -546,8 +546,23 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); yield* runGit(repoDir, ["push", "-u", "origin", "main"]); fs.writeFileSync(path.join(repoDir, "README.md"), "hello\nfeature-branch\n"); + let generatedCount = 0; - const { manager } = yield* makeManager(); + const { manager } = yield* makeManager({ + textGeneration: { + generateCommitMessage: (input) => + Effect.sync(() => { + generatedCount += 1; + return { + subject: "Implement stacked git actions", + body: "", + ...(input.includeBranch + ? { branch: "feature/implement-stacked-git-actions" } + : {}), + }; + }), + }, + }); const result = yield* runStackedAction(manager, { cwd: repoDir, action: "commit_push", @@ -571,6 +586,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { Effect.map((r) => r.stdout.trim()), ); expect(mergeBase).toBe(mainSha); + expect(generatedCount).toBe(1); }), ); diff --git a/apps/server/src/git/Layers/GitManager.ts b/apps/server/src/git/Layers/GitManager.ts index 3cd812afe0195698579ab355751aa269819ef32f..c4a29e15ddb3637012af6df925cd8892759510cf 100644 --- a/apps/server/src/git/Layers/GitManager.ts +++ b/apps/server/src/git/Layers/GitManager.ts @@ -105,6 +105,13 @@ function sanitizeCommitMessage(generated: { }; } +interface CommitAndBranchSuggestion { + subject: string; + body: string; + branch?: string | undefined; + commitMessage: string; +} + function formatCommitMessage(subject: string, body: string): string { const trimmedBody = body.trim(); if (trimmedBody.length === 0) { @@ -311,13 +318,20 @@ export const makeGitManager = Effect.gen(function* () { }; }); - const runCommitStep = (cwd: string, branch: string | null, commitMessage?: string) => + const runCommitStep = ( + cwd: string, + branch: string | null, + commitMessage?: string, + preResolvedSuggestion?: CommitAndBranchSuggestion, + ) => Effect.gen(function* () { - const suggestion = yield* resolveCommitAndBranchSuggestion({ - cwd, - branch, - ...(commitMessage ? { commitMessage } : {}), - }); + const suggestion = + preResolvedSuggestion ?? + (yield* resolveCommitAndBranchSuggestion({ + cwd, + branch, + ...(commitMessage ? { commitMessage } : {}), + })); if (!suggestion) { return { status: "skipped_no_changes" as const }; } @@ -456,6 +470,7 @@ export const makeGitManager = Effect.gen(function* () { return { branchStep: { status: "created" as const, name: resolvedBranch }, resolvedCommitMessage: suggestion.commitMessage, + resolvedCommitSuggestion: suggestion, }; }); @@ -477,6 +492,7 @@ export const makeGitManager = Effect.gen(function* () { let branchStep: { status: "created" | "skipped_not_requested"; name?: string }; let commitMessageForStep = input.commitMessage; + let preResolvedCommitSuggestion: CommitAndBranchSuggestion | undefined = undefined; if (input.featureBranch) { const result = yield* runFeatureBranchStep( @@ -486,13 +502,19 @@ export const makeGitManager = Effect.gen(function* () { ); branchStep = result.branchStep; commitMessageForStep = result.resolvedCommitMessage; + preResolvedCommitSuggestion = result.resolvedCommitSuggestion; } else { branchStep = { status: "skipped_not_requested" as const }; } const currentBranch = branchStep.name ?? initialStatus.branch; - const commit = yield* runCommitStep(input.cwd, currentBranch, commitMessageForStep); + const commit = yield* runCommitStep( + input.cwd, + currentBranch, + commitMessageForStep, + preResolvedCommitSuggestion, + ); const push = wantsPush ? yield* gitCore.pushCurrentBranch(input.cwd, currentBranch) diff --git a/apps/web/src/components/GitActionsControl.logic.test.ts b/apps/web/src/components/GitActionsControl.logic.test.ts index 73d5e9e77ca0f25f61c2765ae8242ae6721864b2..451f496fba283b934cb863b0c5daf001a7e971e6 100644 --- a/apps/web/src/components/GitActionsControl.logic.test.ts +++ b/apps/web/src/components/GitActionsControl.logic.test.ts @@ -943,6 +943,11 @@ describe("resolveAutoFeatureBranchName", () => { assert.equal(branch, "feature/fix-toast-copy-3"); }); + it("treats existing branch names as case-insensitive for collision checks", () => { + const branch = resolveAutoFeatureBranchName(["Feature/Ticket-1"], "feature/ticket-1"); + assert.equal(branch, "feature/ticket-1-2"); + }); + it("falls back to feature/update when no preferred name is provided", () => { const branch = resolveAutoFeatureBranchName(["main"]); assert.equal(branch, "feature/update"); diff --git a/apps/web/src/components/GitActionsControl.tsx b/apps/web/src/components/GitActionsControl.tsx index dd299092b2af199fd00db3848eac6750246f151e..216e123c8c644f11cf5a0c8b961537f0a3505d81 100644 --- a/apps/web/src/components/GitActionsControl.tsx +++ b/apps/web/src/components/GitActionsControl.tsx @@ -254,8 +254,7 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions !skipDefaultBranchPrompt && !featureBranch && requiresDefaultBranchConfirmation(action, actionIsDefaultBranch) && - actionBranch && - !!actionStatus?.hasWorkingTreeChanges + actionBranch ) { if (action !== "commit_push" && action !== "commit_push_pr") { return; @@ -416,6 +415,7 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions (actionParams: { action: GitStackedAction; commitMessage?: string; + forcePushOnlyProgress?: boolean; onConfirmed?: () => void; }) => { void runGitActionWithToast({ @@ -429,11 +429,12 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions const checkoutFeatureBranchAndContinuePendingAction = useCallback(() => { if (!pendingDefaultBranchAction) return; - const { action, commitMessage, onConfirmed } = pendingDefaultBranchAction; + const { action, commitMessage, forcePushOnlyProgress, onConfirmed } = pendingDefaultBranchAction; setPendingDefaultBranchAction(null); checkoutNewBranchAndRunAction({ action, ...(commitMessage ? { commitMessage } : {}), + forcePushOnlyProgress, ...(onConfirmed ? { onConfirmed } : {}), }); }, [pendingDefaultBranchAction, checkoutNewBranchAndRunAction]); diff --git a/apps/web/src/lib/gitReactQuery.ts b/apps/web/src/lib/gitReactQuery.ts index 16cbe8a6cada9e0039fc320199fdbed0a4902624..dbd7ad1c3ae278c1371a9fc43d7d9a6a9c63effd 100644 --- a/apps/web/src/lib/gitReactQuery.ts +++ b/apps/web/src/lib/gitReactQuery.ts @@ -16,8 +16,6 @@ export const gitQueryKeys = { export const gitMutationKeys = { init: (cwd: string | null) => ["git", "mutation", "init", cwd] as const, checkout: (cwd: string | null) => ["git", "mutation", "checkout", cwd] as const, - createBranchAndCheckout: (cwd: string | null) => - ["git", "mutation", "create-branch-and-checkout", cwd] as const, runStackedAction: (cwd: string | null) => ["git", "mutation", "run-stacked-action", cwd] as const, pull: (cwd: string | null) => ["git", "mutation", "pull", cwd] as const, }; @@ -89,24 +87,6 @@ export function gitCheckoutMutationOptions(input: { }); } -export function gitCreateBranchAndCheckoutMutationOptions(input: { - cwd: string | null; - queryClient: QueryClient; -}) { - return mutationOptions({ - mutationKey: gitMutationKeys.createBranchAndCheckout(input.cwd), - mutationFn: async (branch: string) => { - const api = ensureNativeApi(); - if (!input.cwd) throw new Error("Git branch creation is unavailable."); - await api.git.createBranch({ cwd: input.cwd, branch }); - return api.git.checkout({ cwd: input.cwd, branch }); - }, - onSuccess: async () => { - await invalidateGitQueries(input.queryClient); - }, - }); -} - export function gitRunStackedActionMutationOptions(input: { cwd: string | null; queryClient: QueryClient; diff --git a/packages/shared/src/git.ts b/packages/shared/src/git.ts index 113112cd5d5ee95de84c51f2ae904379875a0f34..bbd290393fd5679725dfca8029591f21e5b7462d 100644 --- a/packages/shared/src/git.ts +++ b/packages/shared/src/git.ts @@ -46,7 +46,7 @@ export function resolveAutoFeatureBranchName( const resolvedBase = sanitizeFeatureBranchName( preferred && preferred.length > 0 ? preferred : AUTO_FEATURE_BRANCH_FALLBACK, ); - const existingNames = new Set(existingBranchNames); + const existingNames = new Set(existingBranchNames.map((branch) => branch.toLowerCase())); if (!existingNames.has(resolvedBase)) { return resolvedBase; -- 2.54.0