refactor: agent provider to use explicit type states (#8879)

ui/goose2/src/features/settings/ui/AgentProviderCard.tsx

@@ -15,6 +15,8 @@ import {
 import type { ProviderDisplayInfo } from "@/shared/types/providers";
 
 type SetupPhase = "idle" | "checking" | "installing" | "authenticating";
+type InstallStatus = "checking" | "installed" | "missing";
+type AuthStatus = "checking" | "authenticated" | "unauthenticated" | "unknown";
 
 interface OutputLine {
   id: number;
@@ -29,11 +31,21 @@ interface AgentProviderCardProps {
 
 export function AgentProviderCard({ provider }: AgentProviderCardProps) {
   const { t } = useTranslation(["settings", "common"]);
+  const isBuiltIn = provider.status === "built_in";
+  const hasInstallCommand = !!provider.installCommand;
+  const hasAuthCommand = !!provider.authCommand;
+  const hasBinary = !!provider.binaryName;
   const [setupPhase, setSetupPhase] = useState<SetupPhase>("idle");
   const [setupOutput, setSetupOutput] = useState<OutputLine[]>([]);
   const [setupError, setSetupError] = useState<string | null>(null);
-  const [isInstalled, setIsInstalled] = useState<boolean | null>(null);
-  const [isAuthenticated, setIsAuthenticated] = useState<boolean | null>(null);
+  const [installStatus, setInstallStatus] = useState<InstallStatus>(
+    hasBinary && !isBuiltIn ? "checking" : "installed",
+  );
+  const [authStatus, setAuthStatus] = useState<AuthStatus>(
+    provider.authStatusCommand && hasBinary && !isBuiltIn
+      ? "checking"
+      : "unknown",
+  );
   const outputRef = useRef<HTMLDivElement>(null);
   const outputLengthRef = useRef(0);
   const lineCounterRef = useRef(0);
@@ -41,12 +53,7 @@ export function AgentProviderCard({ provider }: AgentProviderCardProps) {
   const unlistenRef = useRef<(() => void) | null>(null);
 
   const icon = getProviderIcon(provider.id, "size-6");
-  const isBuiltIn = provider.status === "built_in";
   const isActive = setupPhase !== "idle";
-  const hasInstallCommand = !!provider.installCommand;
-  const hasAuthCommand = !!provider.authCommand;
-  const hasAuthStatusCheck = !!provider.authStatusCommand;
-  const hasBinary = !!provider.binaryName;
   const authStorageKey = `agent-provider-auth:${provider.id}`;
 
   const setAuthHint = useCallback(
@@ -83,24 +90,25 @@ export function AgentProviderCard({ provider }: AgentProviderCardProps) {
     checkAgentInstalled(provider.id)
       .then((installed) => {
         if (!isMountedRef.current) return;
-        setIsInstalled(installed);
+        setInstallStatus(installed ? "installed" : "missing");
         if (installed && provider.authStatusCommand) {
           return checkAgentAuth(provider.id).then((authenticated) => {
             if (!isMountedRef.current) return;
-            setIsAuthenticated(authenticated);
+            setAuthStatus(authenticated ? "authenticated" : "unauthenticated");
           });
         }
         if (installed && !provider.authStatusCommand) {
-          setIsAuthenticated(getAuthHint() ? true : null);
+          setAuthStatus(getAuthHint() ? "authenticated" : "unknown");
         }
         if (!installed) {
-          setIsAuthenticated(null);
+          setAuthStatus("unknown");
           setAuthHint(false);
         }
       })
       .catch(() => {
         if (!isMountedRef.current) return;
-        setIsInstalled(false);
+        setInstallStatus("missing");
+        setAuthStatus("unknown");
       });
   }, [
     getAuthHint,
@@ -135,7 +143,7 @@ export function AgentProviderCard({ provider }: AgentProviderCardProps) {
     setSetupOutput([]);
     lineCounterRef.current = 0;
 
-    if (hasInstallCommand && isInstalled === false) {
+    if (hasInstallCommand && installStatus === "missing") {
       await runInstall();
     } else if (hasAuthCommand) {
       await runAuth();
@@ -161,14 +169,13 @@ export function AgentProviderCard({ provider }: AgentProviderCardProps) {
 
       if (hasBinary && provider.binaryName) {
         setSetupPhase("checking");
-        const installed = await checkAgentInstalled(provider.binaryName);
+        const installed = await checkAgentInstalled(provider.id);
         if (!isMountedRef.current) return;
-        setIsInstalled(installed);
+        setInstallStatus(installed ? "installed" : "missing");
         if (!installed) {
+          setAuthStatus("unknown");
           setAuthHint(false);
-          setSetupError(
-            "Install finished but the CLI was not found on PATH. You may need to restart your terminal.",
-          );
+          setSetupError(t("providers.agents.errors.installVerificationFailed"));
           setSetupPhase("idle");
           return;
         }
@@ -206,7 +213,7 @@ export function AgentProviderCard({ provider }: AgentProviderCardProps) {
       clearListener();
       if (!isMountedRef.current) return;
       setAuthHint(true);
-      setIsAuthenticated(true);
+      setAuthStatus("authenticated");
       setSetupPhase("idle");
     } catch (err) {
       clearListener();
@@ -221,15 +228,19 @@ export function AgentProviderCard({ provider }: AgentProviderCardProps) {
     void handleConnect();
   }
 
-  if (provider.showOnlyWhenInstalled && isInstalled !== true) return null;
+  if (provider.showOnlyWhenInstalled && installStatus !== "installed")
+    return null;
 
   const isReady =
     isBuiltIn ||
-    (isInstalled === true && !hasAuthCommand) ||
-    (isInstalled === true && isAuthenticated === true);
+    (installStatus === "installed" && !hasAuthCommand) ||
+    (installStatus === "installed" && authStatus === "authenticated");
   const needsAuth =
-    isInstalled === true && hasAuthCommand && isAuthenticated !== true;
-  const needsInstall = isInstalled === false && hasInstallCommand;
+    installStatus === "installed" &&
+    hasAuthCommand &&
+    authStatus !== "checking" &&
+    authStatus !== "authenticated";
+  const needsInstall = installStatus === "missing" && hasInstallCommand;
 
   function renderStatusIndicator() {
     if (isBuiltIn || isReady) {
@@ -272,7 +283,6 @@ export function AgentProviderCard({ provider }: AgentProviderCardProps) {
           variant="ghost"
           size="icon-xs"
           onClick={() => void handleConnect()}
-          disabled={isInstalled === null && hasBinary}
           className="flex-shrink-0 text-muted-foreground"
           aria-label={t("providers.agents.installLabel", {
             name: provider.displayName,
@@ -288,11 +298,12 @@ export function AgentProviderCard({ provider }: AgentProviderCardProps) {
 
   function renderStatusText() {
     if (isBuiltIn || isReady) return null;
-    if (setupError) return "Setup failed";
+    if (setupError) return t("providers.agents.status.setupFailed");
 
-    if (isInstalled === null && hasBinary) return "Checking...";
-    if (isInstalled === true && hasAuthStatusCheck && isAuthenticated === null)
-      return "Checking...";
+    if (installStatus === "checking" && hasBinary)
+      return t("providers.agents.status.checking");
+    if (installStatus === "installed" && authStatus === "checking")
+      return t("providers.agents.status.checking");
 
     return null;
   }
@@ -313,21 +324,38 @@ export function AgentProviderCard({ provider }: AgentProviderCardProps) {
     return null;
   }
 
+  function renderSetupOutput(scrollToEnd = false) {
+    if (setupOutput.length === 0) return null;
+
+    return (
+      <div
+        ref={scrollToEnd ? outputRef : undefined}
+        className="max-h-24 overflow-y-auto rounded-md bg-muted px-2 py-1.5 font-mono text-xxs leading-relaxed text-muted-foreground"
+      >
+        {setupOutput.map((entry) => (
+          <div key={entry.id}>{entry.text || "\u00A0"}</div>
+        ))}
+      </div>
+    );
+  }
+
   function renderSetupProgress() {
     if (!isActive) return null;
 
     const phaseLabel =
       setupPhase === "installing"
-        ? `Installing ${provider.displayName}...`
+        ? t("providers.agents.progress.installing", {
+            name: provider.displayName,
+          })
         : setupPhase === "authenticating"
-          ? "Waiting for sign-in..."
-          : "Verifying installation...";
+          ? t("providers.waitingForSignIn")
+          : t("providers.agents.progress.verifyingInstallation");
 
     const stepInfo =
       setupPhase === "installing" && hasAuthCommand
-        ? "Step 1 of 2"
+        ? t("providers.agents.progress.step", { step: 1, total: 2 })
         : setupPhase === "authenticating" && hasInstallCommand
-          ? "Step 2 of 2"
+          ? t("providers.agents.progress.step", { step: 2, total: 2 })
           : null;
 
     return (
@@ -344,20 +372,14 @@ export function AgentProviderCard({ provider }: AgentProviderCardProps) {
           </div>
         </div>
 
-        {setupOutput.length > 0 && (
-          <div
-            ref={outputRef}
-            className="max-h-24 overflow-y-auto rounded-md bg-muted px-2 py-1.5 font-mono text-xxs leading-relaxed text-muted-foreground"
-          >
-            {setupOutput.map((entry) => (
-              <div key={entry.id}>{entry.text || "\u00A0"}</div>
-            ))}
-          </div>
-        )}
+        {renderSetupOutput(true)}
       </div>
     );
   }
 
+  const statusText = renderStatusText();
+  const action = renderAction();
+
   return (
     <div
       className={cn(
@@ -378,48 +400,37 @@ export function AgentProviderCard({ provider }: AgentProviderCardProps) {
         {renderStatusIndicator()}
       </div>
 
-      {(() => {
-        const statusText = renderStatusText();
-        const action = renderAction();
-        if (!statusText && !action) return null;
-        return (
-          <div className="mt-3 flex items-center justify-between">
-            <div className="flex items-center gap-1.5">
-              {statusText && (
-                <>
-                  <span
-                    className={cn(
-                      "size-1.5 rounded-full",
-                      setupError
-                        ? "bg-danger"
-                        : isActive
-                          ? "bg-accent animate-pulse"
-                          : "bg-muted-foreground/40",
-                    )}
-                  />
-                  <span className="text-xs text-muted-foreground">
-                    {statusText}
-                  </span>
-                </>
-              )}
-            </div>
-            {action}
+      {(statusText || action) && (
+        <div className="mt-3 flex items-center justify-between">
+          <div className="flex items-center gap-1.5">
+            {statusText && (
+              <>
+                <span
+                  className={cn(
+                    "size-1.5 rounded-full",
+                    setupError
+                      ? "bg-danger"
+                      : isActive
+                        ? "bg-accent animate-pulse"
+                        : "bg-muted-foreground/40",
+                  )}
+                />
+                <span className="text-xs text-muted-foreground">
+                  {statusText}
+                </span>
+              </>
+            )}
           </div>
-        );
-      })()}
+          {action}
+        </div>
+      )}
 
       {renderSetupProgress()}
 
       {setupError && !isActive && (
         <div className="mt-3 space-y-2 border-t pt-3">
           <p className="text-xs text-danger">{setupError}</p>
-          {setupOutput.length > 0 && (
-            <div className="max-h-24 overflow-y-auto rounded-md bg-muted px-2 py-1.5 font-mono text-xxs leading-relaxed text-muted-foreground">
-              {setupOutput.map((entry) => (
-                <div key={entry.id}>{entry.text || "\u00A0"}</div>
-              ))}
-            </div>
-          )}
+          {renderSetupOutput()}
         </div>
       )}
     </div>

ui/goose2/src/features/settings/ui/__tests__/AgentProviderCard.test.tsx

@@ -0,0 +1,94 @@
+import { act, screen, waitFor } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { renderWithProviders } from "@/test/render";
+import { AgentProviderCard } from "../AgentProviderCard";
+import type { ProviderDisplayInfo } from "@/shared/types/providers";
+
+const checkAgentInstalled = vi.fn();
+const checkAgentAuth = vi.fn();
+const installAgent = vi.fn();
+
+vi.mock("@/features/providers/api/agentSetup", () => ({
+  checkAgentInstalled: (...args: unknown[]) => checkAgentInstalled(...args),
+  checkAgentAuth: (...args: unknown[]) => checkAgentAuth(...args),
+  installAgent: (...args: unknown[]) => installAgent(...args),
+  authenticateAgent: vi.fn(),
+  onAgentSetupOutput: vi.fn(async () => vi.fn()),
+}));
+
+function createProvider(): ProviderDisplayInfo {
+  return {
+    id: "claude-acp",
+    displayName: "Claude",
+    category: "agent",
+    description: "Claude provider",
+    setupMethod: "cli_auth",
+    binaryName: "claude",
+    authCommand: "claude auth login",
+    authStatusCommand: "claude auth status",
+    tier: "standard",
+    status: "not_installed",
+  };
+}
+
+describe("AgentProviderCard", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("does not show sign in while auth status is checking", async () => {
+    let resolveAuth!: (authenticated: boolean) => void;
+    const authPromise = new Promise<boolean>((resolve) => {
+      resolveAuth = resolve;
+    });
+
+    checkAgentInstalled.mockResolvedValue(true);
+    checkAgentAuth.mockReturnValue(authPromise);
+
+    renderWithProviders(<AgentProviderCard provider={createProvider()} />);
+
+    expect(await screen.findByText("Checking...")).toBeInTheDocument();
+    expect(
+      screen.queryByRole("button", { name: /sign in/i }),
+    ).not.toBeInTheDocument();
+
+    await act(async () => {
+      resolveAuth(false);
+      await authPromise;
+    });
+
+    await waitFor(() => {
+      expect(
+        screen.getByRole("button", { name: /sign in/i }),
+      ).toBeInTheDocument();
+    });
+  });
+
+  it("checks installation by provider id after installing", async () => {
+    const user = userEvent.setup();
+    checkAgentInstalled
+      .mockResolvedValueOnce(false)
+      .mockResolvedValueOnce(true);
+    installAgent.mockResolvedValue(undefined);
+
+    renderWithProviders(
+      <AgentProviderCard
+        provider={{
+          ...createProvider(),
+          authCommand: undefined,
+          authStatusCommand: undefined,
+          installCommand: "npm install -g claude-agent-acp",
+        }}
+      />,
+    );
+
+    await user.click(
+      await screen.findByRole("button", { name: /install claude/i }),
+    );
+
+    await waitFor(() => {
+      expect(checkAgentInstalled).toHaveBeenNthCalledWith(2, "claude-acp");
+    });
+  });
+});

ui/goose2/src/shared/i18n/locales/en/settings.json

@@ -207,9 +207,21 @@
   "providers": {
     "agents": {
       "description": "Agents handle your requests using their own tools and models",
+      "errors": {
+        "installVerificationFailed": "Install finished but the CLI was not found on PATH. You may need to restart your terminal."
+      },
       "installLabel": "Install {{name}}",
+      "progress": {
+        "installing": "Installing {{name}}...",
+        "step": "Step {{step}} of {{total}}",
+        "verifyingInstallation": "Verifying installation..."
+      },
       "signIn": "Sign in",
       "signInLabel": "Sign in to {{name}}",
+      "status": {
+        "checking": "Checking...",
+        "setupFailed": "Setup failed"
+      },
       "title": "Agents"
     },
     "description": "Connect agents and AI models to use with Goose",

ui/goose2/src/shared/i18n/locales/es/settings.json

@@ -207,9 +207,21 @@
   "providers": {
     "agents": {
       "description": "Los agentes manejan tus solicitudes usando sus propias herramientas y modelos",
+      "errors": {
+        "installVerificationFailed": "La instalación terminó, pero no se encontró la CLI en PATH. Puede que tengas que reiniciar tu terminal."
+      },
       "installLabel": "Instalar {{name}}",
+      "progress": {
+        "installing": "Instalando {{name}}...",
+        "step": "Paso {{step}} de {{total}}",
+        "verifyingInstallation": "Verificando instalación..."
+      },
       "signIn": "Iniciar sesión",
       "signInLabel": "Iniciar sesión en {{name}}",
+      "status": {
+        "checking": "Comprobando...",
+        "setupFailed": "La configuración falló"
+      },
       "title": "Agentes"
     },
     "description": "Conecta agentes y modelos de IA para usar con Goose",