Skip to content

refactor: agent provider to use explicit type states #8879

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kalvinnchau merged 2 commits into from
Apr 28, 2026
Merged

refactor: agent provider to use explicit type states #8879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dfcd8c7
refactor: use explicit type states instead of null
kalvinnchau Apr 28, 2026
c534233
fix: prevent agent sign-in during auth check
kalvinnchau Apr 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 90 additions & 79 deletions ui/goose2/src/features/settings/ui/AgentProviderCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,24 +31,29 @@ 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);
const isMountedRef = useRef(true);
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(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand All @@ -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;
}
Expand Down Expand Up @@ -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();
Expand All @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -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;
}
Expand All @@ -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 (
Expand All @@ -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(
Expand All @@ -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>
Expand Down
Loading
Loading