| author |  Julius Marminge <julius0216@outlook.com> 2026-03-01 19:50:05-08:00 |
| committer |  Julius Marminge <jmarminge@gmail.com> 2026-03-01 20:06:01-08:00 |
| log | ba03241df35b56a43c8f676a0b12c70b04ce004c |
| tree | cdd8f5fb550c94f3c5959759d513e79dcc2962c4 |
| parent | a8059dc2c0341e72efab496c88b15b8dadac71e2 |
fix: address latest git action review feedback
Co-authored-by: codex <codex@users.noreply.github.com>4 files changed, 76 insertions(+), 15 deletions(-)
apps
server
src
git
web
src
apps/server/src/git/Layers/GitCore.test.ts+36
| ... | @@ -1234,6 +1234,42 @@ it.layer(TestLayer)("git integration", (it) => { | ... | @@ -1234,6 +1234,42 @@ it.layer(TestLayer)("git integration", (it) => { |
| 1234 | }), | 1234 | }), |
| 1235 | ); | 1235 | ); |
| 1236 | 1236 | ||
| 1237 | it.effect( | ||
| 1238 | "pushes with upstream setup when comparable base exists but remote branch is missing", | ||
| 1239 | () => | ||
| 1240 | Effect.gen(function* () { | ||
| 1241 | const tmp = yield* makeTmpDir(); | ||
| 1242 | const remote = yield* makeTmpDir(); | ||
| 1243 | yield* git(remote, ["init", "--bare"]); | ||
| 1244 | |||
| 1245 | yield* initRepoWithCommit(tmp); | ||
| 1246 | const initialBranch = (yield* listGitBranches({ cwd: tmp })).branches.find( | ||
| 1247 | (branch) => branch.current, | ||
| 1248 | )!.name; | ||
| 1249 | yield* git(tmp, ["remote", "add", "origin", remote]); | ||
| 1250 | yield* git(tmp, ["push", "-u", "origin", initialBranch]); | ||
| 1251 | |||
| 1252 | yield* writeTextFile(path.join(tmp, "default-ahead.txt"), "ahead on default\n"); | ||
| 1253 | yield* git(tmp, ["add", "default-ahead.txt"]); | ||
| 1254 | yield* git(tmp, ["commit", "-m", "default ahead"]); | ||
| 1255 | |||
| 1256 | const featureBranch = "feature/publish-no-upstream"; | ||
| 1257 | yield* git(tmp, ["checkout", "-b", featureBranch]); | ||
| 1258 | |||
| 1259 | const core = yield* GitCore; | ||
| 1260 | const pushed = yield* core.pushCurrentBranch(tmp, null); | ||
| 1261 | expect(pushed.status).toBe("pushed"); | ||
| 1262 | expect(pushed.setUpstream).toBe(true); | ||
| 1263 | expect(pushed.upstreamBranch).toBe(`origin/${featureBranch}`); | ||
| 1264 | expect(yield* git(tmp, ["rev-parse", "--abbrev-ref", "@{upstream}"])).toBe( | ||
| 1265 | `origin/${featureBranch}`, | ||
| 1266 | ); | ||
| 1267 | expect(yield* git(tmp, ["ls-remote", "--heads", "origin", featureBranch])).toContain( | ||
| 1268 | featureBranch, | ||
| 1269 | ); | ||
| 1270 | }), | ||
| 1271 | ); | ||
| 1272 | |||
| 1237 | it.effect("includes command context when worktree removal fails", () => | 1273 | it.effect("includes command context when worktree removal fails", () => |
| 1238 | Effect.gen(function* () { | 1274 | Effect.gen(function* () { |
| 1239 | const tmp = yield* makeTmpDir(); | 1275 | const tmp = yield* makeTmpDir(); |
apps/server/src/git/Layers/GitCore.ts+24-4
| ... | @@ -381,6 +381,11 @@ const makeGitCore = Effect.gen(function* () { | ... | @@ -381,6 +381,11 @@ const makeGitCore = Effect.gen(function* () { |
| 381 | }, | 381 | }, |
| 382 | ).pipe(Effect.map((result) => result.code === 0)); | 382 | ).pipe(Effect.map((result) => result.code === 0)); |
| 383 | 383 | ||
| 384 | const originRemoteExists = (cwd: string): Effect.Effect<boolean, GitCommandError> => | ||
| 385 | executeGit("GitCore.originRemoteExists", cwd, ["remote", "get-url", "origin"], { | ||
| 386 | allowNonZeroExit: true, | ||
| 387 | }).pipe(Effect.map((result) => result.code === 0)); | ||
| 388 | |||
| 384 | const resolveBaseBranchForNoUpstream = ( | 389 | const resolveBaseBranchForNoUpstream = ( |
| 385 | cwd: string, | 390 | cwd: string, |
| 386 | branch: string, | 391 | branch: string, |
| ... | @@ -668,10 +673,25 @@ const makeGitCore = Effect.gen(function* () { | ... | @@ -668,10 +673,25 @@ const makeGitCore = Effect.gen(function* () { |
| 668 | Effect.catch(() => Effect.succeed(null)), | 673 | Effect.catch(() => Effect.succeed(null)), |
| 669 | ); | 674 | ); |
| 670 | if (comparableBaseBranch) { | 675 | if (comparableBaseBranch) { |
| 671 | return { | 676 | const hasOriginRemote = yield* originRemoteExists(cwd).pipe( |
| 672 | status: "skipped_up_to_date" as const, | 677 | Effect.catch(() => Effect.succeed(false)), |
| 673 | branch, | 678 | ); |
| 674 | }; | 679 | if (!hasOriginRemote) { |
| 680 | return { | ||
| 681 | status: "skipped_up_to_date" as const, | ||
| 682 | branch, | ||
| 683 | }; | ||
| 684 | } | ||
| 685 | |||
| 686 | const hasRemoteBranch = yield* remoteBranchExists(cwd, branch).pipe( | ||
| 687 | Effect.catch(() => Effect.succeed(false)), | ||
| 688 | ); | ||
| 689 | if (hasRemoteBranch) { | ||
| 690 | return { | ||
| 691 | status: "skipped_up_to_date" as const, | ||
| 692 | branch, | ||
| 693 | }; | ||
| 694 | } | ||
| 675 | } | 695 | } |
| 676 | } | 696 | } |
| 677 | 697 |
apps/web/src/components/GitActionsControl.logic.ts+3-9
| ... | @@ -31,6 +31,8 @@ export interface DefaultBranchActionDialogCopy { | ... | @@ -31,6 +31,8 @@ export interface DefaultBranchActionDialogCopy { |
| 31 | continueLabel: string; | 31 | continueLabel: string; |
| 32 | } | 32 | } |
| 33 | 33 | ||
| 34 | export type DefaultBranchConfirmableAction = "commit_push" | "commit_push_pr"; | ||
| 35 | |||
| 34 | const SHORT_SHA_LENGTH = 7; | 36 | const SHORT_SHA_LENGTH = 7; |
| 35 | const TOAST_DESCRIPTION_MAX = 72; | 37 | const TOAST_DESCRIPTION_MAX = 72; |
| 36 | const AUTO_FEATURE_BRANCH_PREFIX = "feature/stacked"; | 38 | const AUTO_FEATURE_BRANCH_PREFIX = "feature/stacked"; |
| ... | @@ -283,21 +285,13 @@ export function requiresDefaultBranchConfirmation( | ... | @@ -283,21 +285,13 @@ export function requiresDefaultBranchConfirmation( |
| 283 | } | 285 | } |
| 284 | 286 | ||
| 285 | export function resolveDefaultBranchActionDialogCopy(input: { | 287 | export function resolveDefaultBranchActionDialogCopy(input: { |
| 286 | action: GitStackedAction; | 288 | action: DefaultBranchConfirmableAction; |
| 287 | branchName: string; | 289 | branchName: string; |
| 288 | includesCommit: boolean; | 290 | includesCommit: boolean; |
| 289 | }): DefaultBranchActionDialogCopy { | 291 | }): DefaultBranchActionDialogCopy { |
| 290 | const branchLabel = input.branchName; | 292 | const branchLabel = input.branchName; |
| 291 | const suffix = ` on "${branchLabel}". You can continue on this branch or create a feature branch and run the same action there.`; | 293 | const suffix = ` on "${branchLabel}". You can continue on this branch or create a feature branch and run the same action there.`; |
| 292 | 294 | ||
| 293 | if (input.action === "commit") { | ||
| 294 | return { | ||
| 295 | title: "Commit to default branch?", | ||
| 296 | description: `This action will commit changes${suffix}`, | ||
| 297 | continueLabel: `Commit to ${branchLabel}`, | ||
| 298 | }; | ||
| 299 | } | ||
| 300 | |||
| 301 | if (input.action === "commit_push") { | 295 | if (input.action === "commit_push") { |
| 302 | if (input.includesCommit) { | 296 | if (input.includesCommit) { |
| 303 | return { | 297 | return { |
apps/web/src/components/GitActionsControl.tsx+13-2
| ... | @@ -9,6 +9,7 @@ import { | ... | @@ -9,6 +9,7 @@ import { |
| 9 | type GitActionIconName, | 9 | type GitActionIconName, |
| 10 | type GitActionMenuItem, | 10 | type GitActionMenuItem, |
| 11 | type GitQuickAction, | 11 | type GitQuickAction, |
| 12 | type DefaultBranchConfirmableAction, | ||
| 12 | requiresDefaultBranchConfirmation, | 13 | requiresDefaultBranchConfirmation, |
| 13 | resolveAutoFeatureBranchName, | 14 | resolveAutoFeatureBranchName, |
| 14 | resolveDefaultBranchActionDialogCopy, | 15 | resolveDefaultBranchActionDialogCopy, |
| ... | @@ -50,7 +51,7 @@ interface GitActionsControlProps { | ... | @@ -50,7 +51,7 @@ interface GitActionsControlProps { |
| 50 | } | 51 | } |
| 51 | 52 | ||
| 52 | interface PendingDefaultBranchAction { | 53 | interface PendingDefaultBranchAction { |
| 53 | action: GitStackedAction; | 54 | action: DefaultBranchConfirmableAction; |
| 54 | branchName: string; | 55 | branchName: string; |
| 55 | includesCommit: boolean; | 56 | includesCommit: boolean; |
| 56 | commitMessage?: string; | 57 | commitMessage?: string; |
| ... | @@ -268,6 +269,9 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions | ... | @@ -268,6 +269,9 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions |
| 268 | requiresDefaultBranchConfirmation(action, actionIsDefaultBranch) && | 269 | requiresDefaultBranchConfirmation(action, actionIsDefaultBranch) && |
| 269 | actionBranch | 270 | actionBranch |
| 270 | ) { | 271 | ) { |
| 272 | if (action !== "commit_push" && action !== "commit_push_pr") { | ||
| 273 | return; | ||
| 274 | } | ||
| 271 | setPendingDefaultBranchAction({ | 275 | setPendingDefaultBranchAction({ |
| 272 | action, | 276 | action, |
| 273 | branchName: actionBranch, | 277 | branchName: actionBranch, |
| ... | @@ -578,11 +582,18 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions | ... | @@ -578,11 +582,18 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions |
| 578 | if (!isCommitDialogOpen) return; | 582 | if (!isCommitDialogOpen) return; |
| 579 | const commitMessage = dialogCommitMessage.trim(); | 583 | const commitMessage = dialogCommitMessage.trim(); |
| 580 | setIsCommitDialogOpen(false); | 584 | setIsCommitDialogOpen(false); |
| 585 | setDialogCommitMessage(""); | ||
| 581 | void runGitActionWithToast({ | 586 | void runGitActionWithToast({ |
| 582 | action: "commit", | 587 | action: "commit", |
| 583 | ...(commitMessage ? { commitMessage } : {}), | 588 | ...(commitMessage ? { commitMessage } : {}), |
| 584 | }); | 589 | }); |
| 585 | }, [dialogCommitMessage, isCommitDialogOpen, runGitActionWithToast, setIsCommitDialogOpen]); | 590 | }, [ |
| 591 | dialogCommitMessage, | ||
| 592 | isCommitDialogOpen, | ||
| 593 | runGitActionWithToast, | ||
| 594 | setDialogCommitMessage, | ||
| 595 | setIsCommitDialogOpen, | ||
| 596 | ]); | ||
| 586 | 597 | ||
| 587 | const openChangedFileInEditor = useCallback( | 598 | const openChangedFileInEditor = useCallback( |
| 588 | (filePath: string) => { | 599 | (filePath: string) => { |