feat(onboarding): auto-detect ACP subscription/login via /api/acp/auth-status

__tests__/components/onboarding/onboarding-modal.test.tsx

@@ -86,6 +86,17 @@ vi.mock("#/hooks/mutation/use-create-conversation", () => ({
   }),
 }));
 
+// The ACP credentials slide runs a login-detection probe (calls
+// GET /api/acp/auth-status). Stub it here so the modal routing tests don't hit
+// the network; the probe itself is covered in use-acp-auth-status.test.tsx.
+vi.mock("#/hooks/query/use-acp-auth-status", () => ({
+  useAcpAuthStatus: () => ({
+    status: "unknown",
+    isChecking: false,
+    isSupported: false,
+  }),
+}));
+
 function renderModal(onClose = vi.fn()) {
   const queryClient = new QueryClient({
     defaultOptions: { queries: { retry: false } },
@@ -283,26 +294,15 @@ describe("OnboardingModal", () => {
     expect(settings.contains(next)).toBe(false);
   });
 
-  it("skips the step-2 slide for an ACP agent with no credentials to collect", async () => {
+  it("shows slide 2 as a login screen (no key fields) for a credential-less ACP agent", async () => {
     renderModal();
     const user = userEvent.setup();
 
-    // Pick Gemini CLI: it authenticates via an interactive OAuth login
-    // (no env-var API key), so it has no credentials step and slide 2 is
-    // skipped — unlike Claude Code / Codex, which now render one there.
+    // Pick Gemini CLI: it authenticates via an interactive OAuth login (no
+    // env-var API key), but the slide still shows so the "you're already
+    // signed in" banner can render — the flow no longer skips it.
     await user.click(screen.getByTestId("onboarding-agent-option-gemini-cli"));
     await user.click(screen.getByTestId("onboarding-agent-next"));
-    await waitFor(
-      () =>
-        expect(screen.getByTestId("onboarding-modal")).toHaveAttribute(
-          "data-current-step",
-          "1",
-        ),
-      { timeout: 3000 },
-    );
-
-    // Advancing again should jump straight to Say Hello (index 3) and
-    // bypass slide 2 — Gemini owns its own auth via the OAuth login.
     await waitFor(
       () =>
         expect(
@@ -312,43 +312,35 @@ describe("OnboardingModal", () => {
     );
     await user.click(screen.getByTestId("onboarding-backend-next"));
 
+    // Lands on slide 2 (the ACP step) — not jumped past to Say Hello.
     await waitFor(
       () =>
         expect(screen.getByTestId("onboarding-modal")).toHaveAttribute(
           "data-current-step",
-          "3",
+          "2",
         ),
       { timeout: 3000 },
     );
-    // All four slides remain mounted (the rail just translates them);
-    // the assertion that the LLM step was skipped is that slide 3 (Say
-    // Hello) is the active one immediately after the backend step,
-    // *not* slide 2 (LLM).
     expect(screen.getByTestId("onboarding-slide-2")).toHaveAttribute(
-      "data-active",
-      "false",
-    );
-    expect(screen.getByTestId("onboarding-slide-3")).toHaveAttribute(
       "data-active",
       "true",
     );
-
-    // Progress bar reflects the *visited* step count, not the slide
-    // index — 3 segments total (not 4), and segment 2 is current (not
-    // segment 3, which would imply LLM was completed). Without this
-    // mapping, picking an ACP agent makes the bar show segment 2 as
-    // "completed" despite the user never visiting it.
     expect(
-      screen.queryByTestId("onboarding-progress-step-3"),
+      screen.getByTestId("onboarding-step-setup-acp-secrets"),
+    ).toBeInTheDocument();
+    // No API-key inputs for Gemini — it's a login-status-only screen.
+    expect(
+      screen.queryByTestId("onboarding-acp-secret-GEMINI_API_KEY"),
     ).not.toBeInTheDocument();
+
+    // The flow keeps all four progress segments (nothing is skipped).
+    expect(
+      screen.getByTestId("onboarding-progress-step-3"),
+    ).toBeInTheDocument();
     expect(screen.getByTestId("onboarding-progress-step-2")).toHaveAttribute(
       "data-state",
       "current",
     );
-    expect(screen.getByTestId("onboarding-progress-step-1")).toHaveAttribute(
-      "data-state",
-      "completed",
-    );
   });
 
   it("shows the ACP credentials step on slide 2 for Claude Code and saves entered keys as secrets", async () => {

__tests__/components/onboarding/setup-acp-secrets-step.test.tsx

@@ -10,7 +10,18 @@ import { SetupAcpSecretsStep } from "#/components/features/onboarding/steps/setu
 import { type OnboardingAgentId } from "#/components/features/onboarding/steps/choose-agent-step";
 import { SecretsService } from "#/api/secrets-service";
 
-function renderStep(providerKey: OnboardingAgentId = "claude-code") {
+// The login-detection probe is exercised in its own hook test; here we stub it
+// so rendering the step doesn't spin a conversation, and so we can drive the
+// banner states directly.
+const acpAuthStatusMock = vi.hoisted(() => vi.fn());
+vi.mock("#/hooks/query/use-acp-auth-status", () => ({
+  useAcpAuthStatus: (...args: unknown[]) => acpAuthStatusMock(...args),
+}));
+
+function renderStep(
+  providerKey: OnboardingAgentId = "claude-code",
+  isActive = true,
+) {
   const onBack = vi.fn();
   const onNext = vi.fn();
   const user = userEvent.setup();
@@ -23,6 +34,7 @@ function renderStep(providerKey: OnboardingAgentId = "claude-code") {
       <ActiveBackendProvider>
         <SetupAcpSecretsStep
           providerKey={providerKey}
+          isActive={isActive}
           onBack={onBack}
           onNext={onNext}
         />
@@ -53,6 +65,11 @@ async function renderWithSavedApiKey() {
 beforeEach(() => {
   vi.restoreAllMocks();
   __resetActiveStoreForTests();
+  acpAuthStatusMock.mockReturnValue({
+    status: "unknown",
+    isChecking: false,
+    isSupported: false,
+  });
   vi.spyOn(SecretsService, "getSecrets").mockResolvedValue([]);
   vi.spyOn(SecretsService, "createSecret").mockResolvedValue();
 });
@@ -166,4 +183,85 @@ describe("SetupAcpSecretsStep", () => {
     );
     expect(onNext).not.toHaveBeenCalled();
   });
+
+  it("runs the login probe scoped to the active step and provider", () => {
+    renderStep("claude-code", true);
+    expect(acpAuthStatusMock).toHaveBeenCalledWith("claude-code", {
+      enabled: true,
+    });
+  });
+
+  it("disables the login probe when the step is not active", () => {
+    renderStep("claude-code", false);
+    expect(acpAuthStatusMock).toHaveBeenCalledWith("claude-code", {
+      enabled: false,
+    });
+  });
+
+  it("shows the 'checking' banner while the login probe is in flight", () => {
+    acpAuthStatusMock.mockReturnValue({
+      status: "unknown",
+      isChecking: true,
+      isSupported: true,
+    });
+    renderStep("claude-code");
+
+    expect(
+      screen.getByTestId("onboarding-acp-auth-checking"),
+    ).toBeInTheDocument();
+    expect(
+      screen.queryByTestId("onboarding-acp-auth-detected"),
+    ).not.toBeInTheDocument();
+  });
+
+  it("shows the 'already signed in' banner when authenticated, keeping the key fields", () => {
+    acpAuthStatusMock.mockReturnValue({
+      status: "authenticated",
+      isChecking: false,
+      isSupported: true,
+    });
+    renderStep("claude-code");
+
+    expect(
+      screen.getByTestId("onboarding-acp-auth-detected"),
+    ).toBeInTheDocument();
+    // The fields stay visible (now optional) even when already logged in.
+    expect(
+      screen.getByTestId("onboarding-acp-secret-ANTHROPIC_API_KEY"),
+    ).toBeInTheDocument();
+  });
+
+  it("renders a login-only screen (banner, no key fields) for a credential-less provider", () => {
+    // Gemini authenticates via browser OAuth — no API-key fields. The step is
+    // purely a login-status screen and still shows the "signed in" banner.
+    acpAuthStatusMock.mockReturnValue({
+      status: "authenticated",
+      isChecking: false,
+      isSupported: true,
+    });
+    renderStep("gemini-cli");
+
+    expect(
+      screen.getByTestId("onboarding-acp-auth-detected"),
+    ).toBeInTheDocument();
+    expect(
+      screen.queryByTestId("onboarding-acp-secret-GEMINI_API_KEY"),
+    ).not.toBeInTheDocument();
+  });
+
+  it("shows no banner when the provider is not authenticated", () => {
+    acpAuthStatusMock.mockReturnValue({
+      status: "unauthenticated",
+      isChecking: false,
+      isSupported: true,
+    });
+    renderStep("claude-code");
+
+    expect(
+      screen.queryByTestId("onboarding-acp-auth-detected"),
+    ).not.toBeInTheDocument();
+    expect(
+      screen.queryByTestId("onboarding-acp-auth-checking"),
+    ).not.toBeInTheDocument();
+  });
 });

__tests__/hooks/query/use-acp-auth-status.test.tsx

@@ -0,0 +1,144 @@
+import React from "react";
+import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
+import { renderHook, waitFor } from "@testing-library/react";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+import { useAcpAuthStatus } from "#/hooks/query/use-acp-auth-status";
+import type { ACPAuthStatusResponse } from "@openhands/typescript-client";
+
+// Active backend is swapped per-test (local vs cloud) via this mutable holder.
+const backendMock = vi.hoisted(() => ({
+  current: {
+    backend: { id: "local-1", kind: "local" as "local" | "cloud" },
+    orgId: null as string | null,
+  },
+}));
+vi.mock("#/contexts/active-backend-context", () => ({
+  useActiveBackend: () => backendMock.current,
+}));
+
+const getAuthStatus = vi.hoisted(() => vi.fn());
+vi.mock("#/api/acp-service/acp-service.api", () => ({
+  default: {
+    getAuthStatus: (...args: unknown[]) => getAuthStatus(...args),
+  },
+}));
+
+// Build a full ACPAuthStatusResponse with the given status; the hook only
+// reads ``.status`` but the service returns the whole record.
+function authResponse(
+  status: ACPAuthStatusResponse["status"],
+): ACPAuthStatusResponse {
+  return {
+    server: "claude-code",
+    status,
+    auth_methods: [],
+    agent_name: "",
+    agent_version: "",
+    detail: null,
+  };
+}
+
+function wrapper({ children }: { children: React.ReactNode }) {
+  const client = new QueryClient({
+    defaultOptions: { queries: { retry: false } },
+  });
+  return <QueryClientProvider client={client}>{children}</QueryClientProvider>;
+}
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  backendMock.current = {
+    backend: { id: "local-1", kind: "local" },
+    orgId: null,
+  };
+});
+
+describe("useAcpAuthStatus", () => {
+  it("reports authenticated when the probe endpoint says so", async () => {
+    getAuthStatus.mockResolvedValue(authResponse("authenticated"));
+
+    const { result } = renderHook(() => useAcpAuthStatus("claude-code"), {
+      wrapper,
+    });
+
+    await waitFor(() => expect(result.current.status).toBe("authenticated"));
+    // The provider key parameterizes the probe.
+    expect(getAuthStatus).toHaveBeenCalledTimes(1);
+    expect(getAuthStatus).toHaveBeenCalledWith("claude-code");
+  });
+
+  it("reports unauthenticated when the endpoint reports auth_required", async () => {
+    getAuthStatus.mockResolvedValue(authResponse("unauthenticated"));
+
+    const { result } = renderHook(() => useAcpAuthStatus("codex"), { wrapper });
+
+    await waitFor(() => expect(result.current.status).toBe("unauthenticated"));
+    expect(getAuthStatus).toHaveBeenCalledWith("codex");
+  });
+
+  it("falls back to unknown when the probe call rejects", async () => {
+    getAuthStatus.mockRejectedValue(new Error("network down"));
+
+    const { result } = renderHook(() => useAcpAuthStatus("claude-code"), {
+      wrapper,
+    });
+
+    // A rejected probe must not be read as "not logged in" — it stays unknown
+    // so the caller keeps showing the API-key fields.
+    await waitFor(() => expect(result.current.isChecking).toBe(false));
+    expect(result.current.status).toBe("unknown");
+  });
+
+  it("surfaces an unknown status from the endpoint verbatim", async () => {
+    getAuthStatus.mockResolvedValue(authResponse("unknown"));
+
+    const { result } = renderHook(() => useAcpAuthStatus("claude-code"), {
+      wrapper,
+    });
+
+    await waitFor(() => expect(result.current.isChecking).toBe(false));
+    expect(result.current.status).toBe("unknown");
+  });
+
+  it("does not probe on a cloud backend", async () => {
+    backendMock.current = {
+      backend: { id: "cloud-1", kind: "cloud" },
+      orgId: null,
+    };
+
+    const { result } = renderHook(() => useAcpAuthStatus("claude-code"), {
+      wrapper,
+    });
+
+    await Promise.resolve();
+    expect(result.current.status).toBe("unknown");
+    expect(result.current.isSupported).toBe(false);
+    expect(getAuthStatus).not.toHaveBeenCalled();
+  });
+
+  it("probes credential-less providers too (e.g. gemini-cli, OAuth login)", async () => {
+    // Eligibility is not tied to having API-key fields — the server can detect
+    // subscription/OAuth providers like Gemini, so the hook must still probe.
+    getAuthStatus.mockResolvedValue(authResponse("authenticated"));
+
+    const { result } = renderHook(() => useAcpAuthStatus("gemini-cli"), {
+      wrapper,
+    });
+
+    await waitFor(() => expect(result.current.status).toBe("authenticated"));
+    expect(result.current.isSupported).toBe(true);
+    expect(getAuthStatus).toHaveBeenCalledWith("gemini-cli");
+  });
+
+  it("does not probe when disabled (e.g. the step is not the active slide)", async () => {
+    const { result } = renderHook(
+      () => useAcpAuthStatus("claude-code", { enabled: false }),
+      { wrapper },
+    );
+
+    await Promise.resolve();
+    expect(result.current.status).toBe("unknown");
+    expect(getAuthStatus).not.toHaveBeenCalled();
+  });
+});