From 28d50aa0170b700fbc8333947aef0b9048cd0956 Mon Sep 17 00:00:00 2001
From: Shirone <kacperlachowiczwp.pl@wp.pl>
Date: Wed, 21 Jan 2026 22:28:22 +0100
Subject: [PATCH] refactor: Consolidate validation and improve error logging

---
 .../src/routes/worktree/routes/add-remote.ts  | 45 +++++-----
 .../dialogs/push-to-remote-dialog.tsx         | 85 +++++++++----------
 apps/ui/src/lib/utils.ts                      |  4 +
 3 files changed, 62 insertions(+), 72 deletions(-)

diff --git a/apps/server/src/routes/worktree/routes/add-remote.ts b/apps/server/src/routes/worktree/routes/add-remote.ts
index bb68ed4d..29389203 100644
--- a/apps/server/src/routes/worktree/routes/add-remote.ts
+++ b/apps/server/src/routes/worktree/routes/add-remote.ts
@@ -69,28 +69,13 @@ export function createAddRemoteHandler() {
         remoteUrl: string;
       };
 
-      if (!worktreePath) {
-        res.status(400).json({
-          success: false,
-          error: 'worktreePath required',
-        });
-        return;
-      }
-
-      if (!remoteName) {
-        res.status(400).json({
-          success: false,
-          error: 'remoteName required',
-        });
-        return;
-      }
-
-      if (!remoteUrl) {
-        res.status(400).json({
-          success: false,
-          error: 'remoteUrl required',
-        });
-        return;
+      // Validate required fields
+      const requiredFields = { worktreePath, remoteName, remoteUrl };
+      for (const [key, value] of Object.entries(requiredFields)) {
+        if (!value) {
+          res.status(400).json({ success: false, error: `${key} required` });
+          return;
+        }
       }
 
       // Validate remote name
@@ -129,8 +114,13 @@ export function createAddRemoteHandler() {
           });
           return;
         }
-      } catch {
-        // If git remote fails, continue with adding the remote
+      } catch (error) {
+        // If git remote fails, continue with adding the remote. Log for debugging.
+        logWorktreeError(
+          error,
+          'Checking for existing remotes failed, proceeding to add.',
+          worktreePath
+        );
       }
 
       // Add the remote using execFile with array arguments to prevent command injection
@@ -146,8 +136,13 @@ export function createAddRemoteHandler() {
           timeout: FETCH_TIMEOUT_MS,
         });
         fetchSucceeded = true;
-      } catch {
+      } catch (fetchError) {
         // Fetch failed (maybe offline or invalid URL), but remote was added successfully
+        logWorktreeError(
+          fetchError,
+          `Fetch from new remote '${remoteName}' failed (remote added successfully)`,
+          worktreePath
+        );
         fetchSucceeded = false;
       }
 
diff --git a/apps/ui/src/components/views/board-view/dialogs/push-to-remote-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/push-to-remote-dialog.tsx
index 02367fb4..0871d267 100644
--- a/apps/ui/src/components/views/board-view/dialogs/push-to-remote-dialog.tsx
+++ b/apps/ui/src/components/views/board-view/dialogs/push-to-remote-dialog.tsx
@@ -19,6 +19,7 @@ import {
   SelectValue,
 } from '@/components/ui/select';
 import { getHttpApiClient } from '@/lib/http-api-client';
+import { getErrorMessage } from '@/lib/utils';
 import { toast } from 'sonner';
 import { Upload, RefreshCw, AlertTriangle, Sparkles, Plus, Link } from 'lucide-react';
 import { Spinner } from '@/components/ui/spinner';
@@ -31,16 +32,6 @@ interface RemoteInfo {
 
 const logger = createLogger('PushToRemoteDialog');
 
-/**
- * Extracts error message from unknown error type
- */
-function getErrorMessage(error: unknown): string {
-  if (error instanceof Error) {
-    return error.message;
-  }
-  return String(error);
-}
-
 interface PushToRemoteDialogProps {
   open: boolean;
   onOpenChange: (open: boolean) => void;
@@ -67,41 +58,6 @@ export function PushToRemoteDialog({
   const [isAddingRemote, setIsAddingRemote] = useState(false);
   const [addRemoteError, setAddRemoteError] = useState<string | null>(null);
 
-  // Fetch remotes when dialog opens
-  useEffect(() => {
-    if (open && worktree) {
-      fetchRemotes();
-    }
-  }, [open, worktree]);
-
-  // Reset state when dialog closes
-  useEffect(() => {
-    if (!open) {
-      setSelectedRemote('');
-      setError(null);
-      setShowAddRemoteForm(false);
-      setNewRemoteName('origin');
-      setNewRemoteUrl('');
-      setAddRemoteError(null);
-    }
-  }, [open]);
-
-  // Auto-select default remote when remotes are loaded
-  useEffect(() => {
-    if (remotes.length > 0 && !selectedRemote) {
-      // Default to 'origin' if available, otherwise first remote
-      const defaultRemote = remotes.find((r) => r.name === 'origin') || remotes[0];
-      setSelectedRemote(defaultRemote.name);
-    }
-  }, [remotes, selectedRemote]);
-
-  // Show add remote form when no remotes
-  useEffect(() => {
-    if (!isLoading && remotes.length === 0) {
-      setShowAddRemoteForm(true);
-    }
-  }, [isLoading, remotes.length]);
-
   /**
    * Transforms API remote data to RemoteInfo format
    */
@@ -125,7 +81,7 @@ export function PushToRemoteDialog({
     }
   }, []);
 
-  const fetchRemotes = async () => {
+  const fetchRemotes = useCallback(async () => {
     if (!worktree) return;
 
     setIsLoading(true);
@@ -147,7 +103,42 @@ export function PushToRemoteDialog({
     } finally {
       setIsLoading(false);
     }
-  };
+  }, [worktree, transformRemoteData, updateRemotesState]);
+
+  // Fetch remotes when dialog opens
+  useEffect(() => {
+    if (open && worktree) {
+      fetchRemotes();
+    }
+  }, [open, worktree, fetchRemotes]);
+
+  // Reset state when dialog closes
+  useEffect(() => {
+    if (!open) {
+      setSelectedRemote('');
+      setError(null);
+      setShowAddRemoteForm(false);
+      setNewRemoteName('origin');
+      setNewRemoteUrl('');
+      setAddRemoteError(null);
+    }
+  }, [open]);
+
+  // Auto-select default remote when remotes are loaded
+  useEffect(() => {
+    if (remotes.length > 0 && !selectedRemote) {
+      // Default to 'origin' if available, otherwise first remote
+      const defaultRemote = remotes.find((r) => r.name === 'origin') || remotes[0];
+      setSelectedRemote(defaultRemote.name);
+    }
+  }, [remotes, selectedRemote]);
+
+  // Show add remote form when no remotes (but not when there's an error)
+  useEffect(() => {
+    if (!isLoading && remotes.length === 0 && !error) {
+      setShowAddRemoteForm(true);
+    }
+  }, [isLoading, remotes.length, error]);
 
   const handleRefresh = async () => {
     if (!worktree) return;
diff --git a/apps/ui/src/lib/utils.ts b/apps/ui/src/lib/utils.ts
index bdaaa9cf..6e291a2e 100644
--- a/apps/ui/src/lib/utils.ts
+++ b/apps/ui/src/lib/utils.ts
@@ -7,6 +7,10 @@ export function cn(...inputs: ClassValue[]) {
   return twMerge(clsx(inputs));
 }
 
+// Re-export getErrorMessage from @automaker/utils to maintain backward compatibility
+// for components that already import it from here
+export { getErrorMessage } from '@automaker/utils';
+
 /**
  * Determine if the current model supports extended thinking controls
  * Note: This is for Claude's "thinking levels" only, not Codex's "reasoning effort"