Centralize git command execution and error handling

src/renderer/actions/gitActions.ts

@@ -1,16 +1,22 @@
 import { buildWorktreeLocation } from "@/shared/worktree";
 import { readBridge } from "@/renderer/bridge";
-import { captureRendererException } from "@/renderer/diagnostics/sentry";
 import { useAppStore } from "@/renderer/state/appStore";
 import { usePanelStore } from "@/renderer/state/panelStore";
 import { usePullFromSourceDialogStore } from "@/renderer/state/pullFromSourceDialogStore";
 import { useSharedSettings } from "@/renderer/state/sharedSettingsStore";
 import { resolveWorktreeBranch } from "@/renderer/utils/gitHelpers";
 import { closeThreads } from "@/renderer/utils/shellUtils";
+import {
+  runGitMergeToSource,
+  runGitPullFromSource,
+  runGitSyncCommand,
+  showGitActionError,
+  showGitOperationFailure,
+} from "./gitCommandRunner";
 import { performWorktreeRemoval } from "./worktreeActions";
 
 function captureGitActionError(error: unknown): void {
-  captureRendererException(error, { featureArea: "git" });
+  showGitActionError(error, { capture: true });
 }
 
 export function openGitReviewForWorktree(projectId: string, worktreePath: string): void {
@@ -30,7 +36,9 @@ export function gitSync(projectId: string, worktreePath?: string): void {
   const location = worktreePath
     ? buildWorktreeLocation(project.location, worktreePath)
     : project.location;
-  void readBridge().gitSync({ projectLocation: location }).catch(captureGitActionError);
+  void runGitSyncCommand({ command: "sync", projectLocation: location }).catch(
+    captureGitActionError,
+  );
 }
 
 export function gitSyncRebase(projectId: string, worktreePath?: string): void {
@@ -39,7 +47,9 @@ export function gitSyncRebase(projectId: string, worktreePath?: string): void {
   const location = worktreePath
     ? buildWorktreeLocation(project.location, worktreePath)
     : project.location;
-  void readBridge().gitSyncRebase({ projectLocation: location }).catch(captureGitActionError);
+  void runGitSyncCommand({ command: "syncRebase", projectLocation: location }).catch(
+    captureGitActionError,
+  );
 }
 
 export function gitPush(projectId: string, worktreePath: string): void {
@@ -48,32 +58,35 @@ export function gitPush(projectId: string, worktreePath: string): void {
   const worktreeBranch = resolveWorktreeBranch(projectId, worktreePath);
   if (!worktreeBranch) return;
   const worktreeLocation = buildWorktreeLocation(project.location, worktreePath);
-  void readBridge()
-    .gitPush({
-      projectLocation: worktreeLocation,
-      remote: "origin",
-      branch: worktreeBranch,
-      setUpstream: true,
-    })
-    .catch(captureGitActionError);
+  void runGitSyncCommand({
+    command: "push",
+    projectLocation: worktreeLocation,
+    remote: "origin",
+    branch: worktreeBranch,
+    setUpstream: true,
+  }).catch(captureGitActionError);
 }
 
 export function gitPull(projectId: string, worktreePath: string): void {
   const project = useAppStore.getState().projects.find((p) => p.id === projectId);
   if (!project) return;
   const worktreeLocation = buildWorktreeLocation(project.location, worktreePath);
-  void readBridge()
-    .gitPull({ projectLocation: worktreeLocation, remote: "origin" })
-    .catch(captureGitActionError);
+  void runGitSyncCommand({
+    command: "pull",
+    projectLocation: worktreeLocation,
+    remote: "origin",
+  }).catch(captureGitActionError);
 }
 
 export function gitPullRebase(projectId: string, worktreePath: string): void {
   const project = useAppStore.getState().projects.find((p) => p.id === projectId);
   if (!project) return;
   const worktreeLocation = buildWorktreeLocation(project.location, worktreePath);
-  void readBridge()
-    .gitPullRebase({ projectLocation: worktreeLocation, remote: "origin" })
-    .catch(captureGitActionError);
+  void runGitSyncCommand({
+    command: "pullRebase",
+    projectLocation: worktreeLocation,
+    remote: "origin",
+  }).catch(captureGitActionError);
 }
 
 export function gitMergeToSource(projectId: string, worktreePath: string): void {
@@ -89,7 +102,7 @@ export function gitMergeToSource(projectId: string, worktreePath: string): void
       });
       if (!sourceBranch) return;
       const worktreeLocation = buildWorktreeLocation(project.location, worktreePath);
-      await readBridge().gitMergeToSource({
+      await runGitMergeToSource({
         projectLocation: project.location,
         worktreeLocation,
         worktreeBranch,
@@ -115,7 +128,7 @@ export function gitMergeAndRemove(projectId: string, worktreePath: string): void
       });
       if (!sourceBranch) return;
       const worktreeLocation = buildWorktreeLocation(project.location, worktreePath);
-      const result = await readBridge().gitMergeToSource({
+      const result = await runGitMergeToSource({
         projectLocation: project.location,
         worktreeLocation,
         worktreeBranch,
@@ -150,7 +163,7 @@ export function gitPullFromSource(projectId: string, worktreePath: string): void
       });
       if (!sourceBranch) return;
       const worktreeLocation = buildWorktreeLocation(project.location, worktreePath);
-      const result = await readBridge().gitPullFromSource({
+      const result = await runGitPullFromSource({
         worktreeLocation,
         sourceBranch,
         preserveLocalChanges: false,
@@ -165,10 +178,13 @@ export function gitPullFromSource(projectId: string, worktreePath: string): void
       }
       if (result.conflicting) {
         openGitReviewForWorktree(projectId, worktreePath);
+        return;
+      }
+      if (!result.merged) {
+        showGitOperationFailure(result);
       }
     } catch (error) {
       captureGitActionError(error);
-      // ignored — user can open git review for details
     }
   })();
 }

src/renderer/actions/gitCommandRunner.test.ts

@@ -0,0 +1,66 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { runGitSyncCommand, showGitActionError } from "./gitCommandRunner";
+
+const bridgeMock = vi.hoisted(() => ({
+  gitPull: vi.fn<() => Promise<void>>(),
+  gitPullRebase: vi.fn<() => Promise<void>>(),
+  gitPush: vi.fn<() => Promise<void>>(),
+  gitSync: vi.fn<() => Promise<void>>(),
+  gitSyncRebase: vi.fn<() => Promise<void>>(),
+}));
+
+const toastMock = vi.hoisted(() => ({
+  danger: vi.fn<(message: string) => void>(),
+}));
+
+const captureRendererExceptionMock = vi.hoisted(() => vi.fn<() => void>());
+
+vi.mock("@heroui/react", () => ({
+  toast: toastMock,
+}));
+
+vi.mock("@/renderer/bridge", () => ({
+  readBridge: () => bridgeMock,
+}));
+
+vi.mock("@/renderer/diagnostics/sentry", () => ({
+  captureRendererException: captureRendererExceptionMock,
+}));
+
+const projectLocation = { kind: "posix" as const, path: "/repo" };
+
+describe("gitCommandRunner", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("routes push commands through the shared bridge payload builder", async () => {
+    bridgeMock.gitPush.mockResolvedValueOnce(undefined);
+
+    await runGitSyncCommand({
+      command: "push",
+      projectLocation,
+      remote: "origin",
+      branch: "feature/a",
+      setUpstream: true,
+    });
+
+    expect(bridgeMock.gitPush).toHaveBeenCalledWith({
+      projectLocation,
+      remote: "origin",
+      branch: "feature/a",
+      setUpstream: true,
+    });
+  });
+
+  it("shows shared git action errors and captures only when requested", () => {
+    const error = new Error("pull failed");
+
+    showGitActionError(error, { capture: true });
+
+    expect(toastMock.danger).toHaveBeenCalledWith("pull failed");
+    expect(captureRendererExceptionMock).toHaveBeenCalledWith(error, {
+      featureArea: "git",
+    });
+  });
+});

src/renderer/actions/gitCommandRunner.ts

@@ -0,0 +1,100 @@
+import { toast } from "@heroui/react";
+import type {
+  GitMergeToSourcePayload,
+  GitMergeToSourceResult,
+  GitPullFromSourcePayload,
+  GitPullFromSourceResult,
+  GitPushPayload,
+  GitSyncPayload,
+  ProjectLocation,
+} from "@/shared/contracts";
+import { friendlyError, msg } from "@/shared/messages";
+import { readBridge } from "@/renderer/bridge";
+import { captureRendererException } from "@/renderer/diagnostics/sentry";
+
+export type GitSyncCommand = "pull" | "pullRebase" | "push" | "sync" | "syncRebase";
+export type SyncAction = "push" | "pull" | "sync";
+
+export function deriveSyncAction(hasTracking: boolean, ahead: number, behind: number): SyncAction {
+  if (!hasTracking) return "push";
+  if (ahead > 0 && behind === 0) return "push";
+  if (behind > 0 && ahead === 0) return "pull";
+  return "sync";
+}
+
+export function showGitActionError(
+  error: unknown,
+  options?: { logPrefix?: string; capture?: boolean },
+): void {
+  if (options?.logPrefix) {
+    console.error(options.logPrefix, error);
+  }
+  if (options?.capture) {
+    captureRendererException(error, { featureArea: "git" });
+  }
+  toast.danger(friendlyError(error));
+}
+
+export function showGitOperationFailure(result: { error?: string }): void {
+  toast.danger(result.error ?? msg("git.merge.failed"));
+}
+
+export async function runGitSyncCommand(input: {
+  command: GitSyncCommand;
+  projectLocation: ProjectLocation;
+  remote?: string;
+  branch?: string;
+  setUpstream?: boolean;
+}): Promise<void> {
+  const { command, projectLocation } = input;
+  switch (command) {
+    case "pull":
+      await readBridge().gitPull(buildGitSyncPayload(projectLocation, input.remote));
+      return;
+    case "pullRebase":
+      await readBridge().gitPullRebase(buildGitSyncPayload(projectLocation, input.remote));
+      return;
+    case "push":
+      await readBridge().gitPush(buildGitPushPayload(input));
+      return;
+    case "sync":
+      await readBridge().gitSync(buildGitSyncPayload(projectLocation, input.remote));
+      return;
+    case "syncRebase":
+      await readBridge().gitSyncRebase(buildGitSyncPayload(projectLocation, input.remote));
+      return;
+  }
+}
+
+export async function runGitMergeToSource(
+  payload: GitMergeToSourcePayload,
+): Promise<GitMergeToSourceResult> {
+  return readBridge().gitMergeToSource(payload);
+}
+
+export async function runGitPullFromSource(
+  payload: GitPullFromSourcePayload,
+): Promise<GitPullFromSourceResult> {
+  return readBridge().gitPullFromSource(payload);
+}
+
+function buildGitSyncPayload(projectLocation: ProjectLocation, remote?: string): GitSyncPayload {
+  return {
+    projectLocation,
+    ...(remote ? { remote } : {}),
+  };
+}
+
+function buildGitPushPayload(input: {
+  projectLocation: ProjectLocation;
+  remote?: string;
+  branch?: string;
+  setUpstream?: boolean;
+}): GitPushPayload {
+  return {
+    projectLocation: input.projectLocation,
+    ...(input.remote ? { remote: input.remote } : {}),
+    ...(input.branch ? { branch: input.branch } : {}),
+    ...(input.setUpstream !== undefined ? { setUpstream: input.setUpstream } : {}),
+  };
+}

src/renderer/views/GitReviewOverlay/parts/GitReviewSidebar/parts/CommitSyncPanel.tsx

@@ -1,6 +1,7 @@
 import { ArrowDown, ArrowUp, ArrowUpDown, ChevronDown, Lock, Sparkles } from "lucide-react";
 import { Button, ButtonGroup, Dropdown, Label, Tooltip } from "@heroui/react";
 import { PixelLoader, TextArea } from "@/renderer/components/common";
+import type { GitSyncCommand } from "@/renderer/actions/gitCommandRunner";
 import { GitReviewSection } from "./GitReviewSection";
 
 export function CommitSyncPanel(props: {
@@ -25,7 +26,7 @@ export function CommitSyncPanel(props: {
   handleCommit: (addAll: boolean, pushAfter?: boolean) => Promise<void>;
   handleGenerateMessage: () => Promise<void>;
   handleSyncOrPush: () => Promise<void>;
-  handleSyncAction: (key: "pull" | "pullRebase" | "push" | "sync" | "syncRebase") => Promise<void>;
+  handleSyncAction: (key: GitSyncCommand) => Promise<void>;
   handlePullFromSource: () => Promise<void>;
 }) {
   const {
@@ -257,9 +258,7 @@ export function CommitSyncPanel(props: {
                         void handlePullFromSource();
                         return;
                       }
-                      void handleSyncAction(
-                        key as "pull" | "pullRebase" | "push" | "sync" | "syncRebase",
-                      );
+                      void handleSyncAction(key as GitSyncCommand);
                     }}
                   >
                     {showPull ? (