From c8a9ae64b694d74ebae74f3ae42b6e67f20333a8 Mon Sep 17 00:00:00 2001
From: Kacper <kacperlachowiczwp.pl@wp.pl>
Date: Thu, 11 Dec 2025 21:07:15 +0100
Subject: [PATCH] fix: address Gemini code review suggestions

Fixed two critical code quality issues identified in code review:

1. Auth Method Selector - Tailwind CSS Dynamic Classes:
   - Replaced dynamic class name concatenation with static class mapping
   - Tailwind JIT compiler requires complete class names at build time
   - Added getBadgeClasses() helper function with predefined color mappings
   - Prevents broken styling from unparseable dynamic classes

2. Claude CLI Detector - Async Terminal Detection:
   - Moved execSync loop to async/await pattern to prevent UI blocking
   - Refactored Linux terminal detection to run before Promise constructor
   - Prevents main process freezing during terminal emulator detection
   - Improves responsiveness on Linux systems

Both fixes improve code reliability and user experience.
---
 app/electron/services/claude-cli-detector.js  | 59 +++++++++------
 .../components/auth-method-selector.tsx       | 75 ++++++++++++++-----
 2 files changed, 92 insertions(+), 42 deletions(-)

diff --git a/app/electron/services/claude-cli-detector.js b/app/electron/services/claude-cli-detector.js
index ba18e915..2a8f193c 100644
--- a/app/electron/services/claude-cli-detector.js
+++ b/app/electron/services/claude-cli-detector.js
@@ -611,7 +611,39 @@ class ClaudeCliDetector {
     }
     send("Opening system terminal for authentication...\n");
 
-    return await new Promise((resolve, reject) => {
+    // Helper function to check if a command exists asynchronously
+    const commandExists = (cmd) => {
+      return new Promise((resolve) => {
+        require("child_process").exec(
+          `which ${cmd}`,
+          { timeout: 1000 },
+          (error) => {
+            resolve(!error);
+          }
+        );
+      });
+    };
+
+    // For Linux, find available terminal first (async)
+    let linuxTerminal = null;
+    if (platform !== "win32" && platform !== "darwin") {
+      const terminals = [
+        ["gnome-terminal", ["--", claudePath, "setup-token"]],
+        ["konsole", ["-e", claudePath, "setup-token"]],
+        ["xterm", ["-e", claudePath, "setup-token"]],
+        ["x-terminal-emulator", ["-e", `${claudePath} setup-token`]],
+      ];
+
+      for (const [term, termArgs] of terminals) {
+        const exists = await commandExists(term);
+        if (exists) {
+          linuxTerminal = { command: term, args: termArgs };
+          break;
+        }
+      }
+    }
+
+    return new Promise((resolve, reject) => {
       // Open command in external terminal since Claude CLI requires TTY
       let command, args;
 
@@ -629,27 +661,8 @@ class ClaudeCliDetector {
           'tell application "Terminal" to activate',
         ];
       } else {
-        // Linux: Try common terminal emulators
-        const terminals = [
-          ["gnome-terminal", ["--", claudePath, "setup-token"]],
-          ["konsole", ["-e", claudePath, "setup-token"]],
-          ["xterm", ["-e", claudePath, "setup-token"]],
-          ["x-terminal-emulator", ["-e", `${claudePath} setup-token`]],
-        ];
-
-        // Try to find an available terminal
-        for (const [term, termArgs] of terminals) {
-          try {
-            execSync(`which ${term}`, { stdio: "ignore" });
-            command = term;
-            args = termArgs;
-            break;
-          } catch {
-            // Terminal not found, try next
-          }
-        }
-
-        if (!command) {
+        // Linux: Use the terminal we found earlier
+        if (!linuxTerminal) {
           reject({
             success: false,
             error:
@@ -658,6 +671,8 @@ class ClaudeCliDetector {
           });
           return;
         }
+        command = linuxTerminal.command;
+        args = linuxTerminal.args;
       }
 
       console.log(
diff --git a/app/src/components/views/setup-view/components/auth-method-selector.tsx b/app/src/components/views/setup-view/components/auth-method-selector.tsx
index 283bba10..75d06e52 100644
--- a/app/src/components/views/setup-view/components/auth-method-selector.tsx
+++ b/app/src/components/views/setup-view/components/auth-method-selector.tsx
@@ -14,33 +14,68 @@ interface AuthMethodSelectorProps {
   onSelect: (methodId: string) => void;
 }
 
+// Map badge colors to complete Tailwind class names
+const getBadgeClasses = (badgeColor: string) => {
+  const colorMap: Record<string, { border: string; bg: string; text: string }> = {
+    "brand-500": {
+      border: "hover:border-brand-500/50",
+      bg: "hover:bg-brand-500/5",
+      text: "text-brand-500",
+    },
+    "green-500": {
+      border: "hover:border-green-500/50",
+      bg: "hover:bg-green-500/5",
+      text: "text-green-500",
+    },
+    "blue-500": {
+      border: "hover:border-blue-500/50",
+      bg: "hover:bg-blue-500/5",
+      text: "text-blue-500",
+    },
+    "purple-500": {
+      border: "hover:border-purple-500/50",
+      bg: "hover:bg-purple-500/5",
+      text: "text-purple-500",
+    },
+  };
+
+  return colorMap[badgeColor] || {
+    border: "hover:border-brand-500/50",
+    bg: "hover:bg-brand-500/5",
+    text: "text-brand-500",
+  };
+};
+
 export function AuthMethodSelector({
   options,
   onSelect,
 }: AuthMethodSelectorProps) {
   return (
     <div className="grid grid-cols-1 md:grid-cols-2 gap-4">
-      {options.map((option) => (
-        <button
-          key={option.id}
-          onClick={() => onSelect(option.id)}
-          className={`p-4 rounded-lg border border-border hover:border-${option.badgeColor}/50 bg-card hover:bg-${option.badgeColor}/5 transition-all text-left`}
-          data-testid={`select-${option.id}-auth`}
-        >
-          <div className="flex items-start gap-3">
-            {option.icon}
-            <div>
-              <p className="font-medium text-foreground">{option.title}</p>
-              <p className="text-sm text-muted-foreground mt-1">
-                {option.description}
-              </p>
-              <p className={`text-xs text-${option.badgeColor} mt-2`}>
-                {option.badge}
-              </p>
+      {options.map((option) => {
+        const badgeClasses = getBadgeClasses(option.badgeColor);
+        return (
+          <button
+            key={option.id}
+            onClick={() => onSelect(option.id)}
+            className={`p-4 rounded-lg border border-border ${badgeClasses.border} bg-card ${badgeClasses.bg} transition-all text-left`}
+            data-testid={`select-${option.id}-auth`}
+          >
+            <div className="flex items-start gap-3">
+              {option.icon}
+              <div>
+                <p className="font-medium text-foreground">{option.title}</p>
+                <p className="text-sm text-muted-foreground mt-1">
+                  {option.description}
+                </p>
+                <p className={`text-xs ${badgeClasses.text} mt-2`}>
+                  {option.badge}
+                </p>
+              </div>
             </div>
-          </div>
-        </button>
-      ))}
+          </button>
+        );
+      })}
     </div>
   );
 }