diff --git a/__tests__/components/onboarding/setup-acp-secrets-step.test.tsx b/__tests__/components/onboarding/setup-acp-secrets-step.test.tsx index d1c0cd4c5d..29eea47676 100644 --- a/__tests__/components/onboarding/setup-acp-secrets-step.test.tsx +++ b/__tests__/components/onboarding/setup-acp-secrets-step.test.tsx @@ -277,6 +277,25 @@ describe("SetupAcpSecretsStep", () => { ).not.toBeInTheDocument(); }); + it("shows the 'credentials configured' banner when a credential is stored but the probe can't confirm a login", async () => { + acpAuthStatusMock.mockReturnValue({ + status: "unknown", + isChecking: false, + isSupported: true, + }); + vi.spyOn(SecretsService, "getSecrets").mockResolvedValue([ + { name: "ANTHROPIC_API_KEY" }, + ]); + renderStep("claude-code"); + + expect( + await screen.findByTestId("onboarding-acp-auth-configured"), + ).toBeInTheDocument(); + expect( + screen.queryByTestId("onboarding-acp-auth-detected"), + ).not.toBeInTheDocument(); + }); + it("renders the Codex subscription blob as a multiline textarea", () => { renderStep("codex"); diff --git a/__tests__/components/settings/acp-auth-status-banner.test.tsx b/__tests__/components/settings/acp-auth-status-banner.test.tsx new file mode 100644 index 0000000000..81a0ab40aa --- /dev/null +++ b/__tests__/components/settings/acp-auth-status-banner.test.tsx @@ -0,0 +1,77 @@ +import { describe, it, expect } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { AcpAuthStatusBanner } from "#/components/features/settings/acp-auth-status-banner"; + +const PREFIX = "settings-acp-auth"; + +describe("AcpAuthStatusBanner", () => { + it("shows the 'signed in' banner when the host-login probe authenticated", () => { + render( + , + ); + expect(screen.getByTestId(`${PREFIX}-detected`)).toBeInTheDocument(); + expect( + screen.queryByTestId(`${PREFIX}-configured`), + ).not.toBeInTheDocument(); + }); + + it("shows the checking spinner while the first probe is in flight", () => { + render( + , + ); + expect(screen.getByTestId(`${PREFIX}-checking`)).toBeInTheDocument(); + }); + + it("shows the 'credentials configured' banner when the probe can't confirm a login but a credential is stored (Docker/cloud)", () => { + render( + , + ); + expect(screen.getByTestId(`${PREFIX}-configured`)).toBeInTheDocument(); + // Honesty guard: a stored credential must NOT render as the "signed in" banner. + expect(screen.queryByTestId(`${PREFIX}-detected`)).not.toBeInTheDocument(); + }); + + it("renders nothing when there is no login signal and no stored credential", () => { + const { container } = render( + , + ); + expect(container).toBeEmptyDOMElement(); + }); + + it("prefers the checking spinner over 'configured' while the probe is still in flight", () => { + render( + , + ); + expect(screen.getByTestId(`${PREFIX}-checking`)).toBeInTheDocument(); + expect( + screen.queryByTestId(`${PREFIX}-configured`), + ).not.toBeInTheDocument(); + }); +}); diff --git a/__tests__/components/settings/acp-credentials-section-cloud.test.tsx b/__tests__/components/settings/acp-credentials-section-cloud.test.tsx new file mode 100644 index 0000000000..e8906b94b1 --- /dev/null +++ b/__tests__/components/settings/acp-credentials-section-cloud.test.tsx @@ -0,0 +1,116 @@ +import React from "react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render, screen } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { + __resetActiveStoreForTests, + setRegisteredBackends, + setActiveSelection, +} from "#/api/backend-registry/active-store"; +import { ActiveBackendProvider } from "#/contexts/active-backend-context"; +import { AcpCredentialsSection } from "#/components/features/settings/acp-credentials-section"; +import { useAcpCredentialForm } from "#/hooks/use-acp-credential-form"; +import { fetchCloudSecrets } from "#/api/cloud/secrets-service.api"; + +// The login-detection probe is gated off on cloud backends (it shells the +// host CLI, which doesn't exist there), so it always reports "unknown" — stub +// it to that so this test isolates the credentials-configured path. +const acpAuthStatusMock = vi.hoisted(() => vi.fn()); +vi.mock("#/hooks/query/use-acp-auth-status", () => ({ + useAcpAuthStatus: (...args: unknown[]) => acpAuthStatusMock(...args), +})); + +// Mock the cloud secrets boundary, NOT SecretsService — so the real +// SecretsService.getSecrets runs and exercises its cloud branch +// (kind === "cloud" → fetchCloudSecrets), which is the path the host-probe +// can never reach. This is the gap the Docker/cloud-only mocked-getSecrets +// test cannot cover. +vi.mock("#/api/cloud/secrets-service.api", () => ({ + fetchCloudSecrets: vi.fn(), + createCloudSecret: vi.fn(), + updateCloudSecret: vi.fn(), + deleteCloudSecret: vi.fn(), +})); + +function Harness({ providerKey }: { providerKey: string }) { + const form = useAcpCredentialForm(providerKey); + return ; +} + +function renderSection(providerKey: string) { + render( + + + + + , + ); +} + +function activateCloudBackend() { + setRegisteredBackends([ + { + id: "cloud-1", + name: "Cloud", + host: "https://app.example.com", + apiKey: "", + kind: "cloud", + }, + ]); + setActiveSelection({ backendId: "cloud-1", orgId: "org-1" }); +} + +beforeEach(() => { + vi.restoreAllMocks(); + __resetActiveStoreForTests(); + acpAuthStatusMock.mockReturnValue({ + status: "unknown", + isChecking: false, + isSupported: false, + }); + vi.mocked(fetchCloudSecrets).mockResolvedValue([]); +}); +afterEach(() => { + __resetActiveStoreForTests(); +}); + +describe("AcpCredentialsSection on a cloud backend (#1244)", () => { + it("surfaces 'credentials configured' from the cloud secret store, never 'signed in'", async () => { + activateCloudBackend(); + // A credential resolved through the cloud secrets API (not the host probe, + // which is gated off on cloud). + vi.mocked(fetchCloudSecrets).mockResolvedValue([ + { name: "CLAUDE_CODE_OAUTH_TOKEN" }, + ]); + + renderSection("claude-code"); + + expect( + await screen.findByTestId("settings-acp-auth-configured"), + ).toBeInTheDocument(); + // The real SecretsService.getSecrets routed through the cloud branch. + expect(fetchCloudSecrets).toHaveBeenCalled(); + // A stored credential must never overstate itself as a verified login. + expect( + screen.queryByTestId("settings-acp-auth-detected"), + ).not.toBeInTheDocument(); + }); + + it("does not show 'configured' when the cloud store has no provider credential", async () => { + activateCloudBackend(); + vi.mocked(fetchCloudSecrets).mockResolvedValue([]); + + renderSection("claude-code"); + + // Wait for the secrets query to settle on a stable element, then assert. + await screen.findByTestId("settings-acp-secret-CLAUDE_CODE_OAUTH_TOKEN"); + expect( + screen.queryByTestId("settings-acp-auth-configured"), + ).not.toBeInTheDocument(); + }); +}); diff --git a/__tests__/components/settings/acp-credentials-section.test.tsx b/__tests__/components/settings/acp-credentials-section.test.tsx index d4b524aab6..89b9edba98 100644 --- a/__tests__/components/settings/acp-credentials-section.test.tsx +++ b/__tests__/components/settings/acp-credentials-section.test.tsx @@ -141,4 +141,61 @@ describe("AcpCredentialsSection", () => { screen.queryByTestId("settings-acp-auth-checking"), ).not.toBeInTheDocument(); }); + + it("shows the 'credentials configured' banner when a credential is stored but the probe can't confirm a login (Docker/cloud)", async () => { + acpAuthStatusMock.mockReturnValue({ + status: "unknown", + isChecking: false, + isSupported: true, + }); + vi.spyOn(SecretsService, "getSecrets").mockResolvedValue([ + { name: "ANTHROPIC_API_KEY" }, + ]); + renderSection("claude-code"); + expect( + await screen.findByTestId("settings-acp-auth-configured"), + ).toBeInTheDocument(); + // A stored credential must never overstate itself as a verified login. + expect( + screen.queryByTestId("settings-acp-auth-detected"), + ).not.toBeInTheDocument(); + }); + + it("does not treat a stored non-credential field (base URL) as configured", async () => { + acpAuthStatusMock.mockReturnValue({ + status: "unknown", + isChecking: false, + isSupported: true, + }); + vi.spyOn(SecretsService, "getSecrets").mockResolvedValue([ + { name: "ANTHROPIC_BASE_URL" }, + ]); + renderSection("claude-code"); + // Wait for the secrets query to settle on a stable element, then assert. + await screen.findByTestId("settings-acp-secret-ANTHROPIC_API_KEY"); + expect( + screen.queryByTestId("settings-acp-auth-configured"), + ).not.toBeInTheDocument(); + }); + + it("treats a stored file-blob credential as configured for a non-Claude provider (Codex)", async () => { + // The configured signal is provider-generic, not Claude-specific, and a + // multiline file-content blob (Codex auth.json) is a credential the same as + // an API key or OAuth token. + acpAuthStatusMock.mockReturnValue({ + status: "unknown", + isChecking: false, + isSupported: true, + }); + vi.spyOn(SecretsService, "getSecrets").mockResolvedValue([ + { name: "CODEX_AUTH_JSON" }, + ]); + renderSection("codex"); + expect( + await screen.findByTestId("settings-acp-auth-configured"), + ).toBeInTheDocument(); + expect( + screen.queryByTestId("settings-acp-auth-detected"), + ).not.toBeInTheDocument(); + }); }); diff --git a/__tests__/hooks/query/use-acp-auth-status.test.tsx b/__tests__/hooks/query/use-acp-auth-status.test.tsx index 6ffca8e08e..a9642c48f2 100644 --- a/__tests__/hooks/query/use-acp-auth-status.test.tsx +++ b/__tests__/hooks/query/use-acp-auth-status.test.tsx @@ -101,6 +101,11 @@ describe("useAcpAuthStatus", () => { await Promise.resolve(); expect(result.current.status).toBe("unknown"); expect(result.current.isSupported).toBe(false); + // The probe is gated off, so it never enters a "checking" state — the + // banner consumer (resolveAcpAuthDisplay) can fall straight through to the + // credentials-configured signal instead of spinning on a probe that will + // never run. This is the behaviour the Docker/cloud banner state relies on. + expect(result.current.isChecking).toBe(false); expect(getAuthStatus).not.toHaveBeenCalled(); }); diff --git a/__tests__/utils/acp-auth-display.test.ts b/__tests__/utils/acp-auth-display.test.ts new file mode 100644 index 0000000000..4d92d8c5af --- /dev/null +++ b/__tests__/utils/acp-auth-display.test.ts @@ -0,0 +1,92 @@ +import { describe, it, expect } from "vitest"; +import { + resolveAcpAuthDisplay, + type AcpAuthDisplay, +} from "#/utils/acp-auth-display"; + +// Pure decision matrix for the ACP auth banner. The host-login probe +// (`status`) is only trustworthy on a native backend; inside a container it +// can't run and returns "unknown". `credentialsConfigured` is the separate, +// backend-truthful signal (a secret exists in the active backend's store) that +// works on Docker/cloud too. See issue #1244. +describe("resolveAcpAuthDisplay", () => { + it("reports a probe-confirmed login ('signed-in') whenever the probe authenticated, regardless of other signals", () => { + expect( + resolveAcpAuthDisplay({ + status: "authenticated", + isChecking: false, + credentialsConfigured: false, + }), + ).toBe("signed-in"); + expect( + resolveAcpAuthDisplay({ + status: "authenticated", + isChecking: true, + credentialsConfigured: true, + }), + ).toBe("signed-in"); + }); + + it("reports 'checking' while the first probe is in flight and no login is confirmed yet", () => { + expect( + resolveAcpAuthDisplay({ + status: "unknown", + isChecking: true, + credentialsConfigured: false, + }), + ).toBe("checking"); + expect( + resolveAcpAuthDisplay({ + status: "unauthenticated", + isChecking: true, + credentialsConfigured: true, + }), + ).toBe("checking"); + }); + + it("reports 'configured' when the probe can't confirm a login but a credential exists in the secret store (the Docker/cloud case)", () => { + // "unknown" == couldn't tell (e.g. a container with no interactive CLI); + // a stored secret is still a real signal the agent will authenticate. + expect( + resolveAcpAuthDisplay({ + status: "unknown", + isChecking: false, + credentialsConfigured: true, + }), + ).toBe("configured"); + expect( + resolveAcpAuthDisplay({ + status: "unauthenticated", + isChecking: false, + credentialsConfigured: true, + }), + ).toBe("configured"); + }); + + it("reports 'none' when there is no confirmed login and no stored credential", () => { + expect( + resolveAcpAuthDisplay({ + status: "unknown", + isChecking: false, + credentialsConfigured: false, + }), + ).toBe("none"); + expect( + resolveAcpAuthDisplay({ + status: "unauthenticated", + isChecking: false, + credentialsConfigured: false, + }), + ).toBe("none"); + }); + + it("never reports 'signed-in' from a stored credential alone — only the probe confirms a host login (honesty guard)", () => { + const display: AcpAuthDisplay = resolveAcpAuthDisplay({ + status: "unknown", + isChecking: false, + credentialsConfigured: true, + }); + expect(display).not.toBe("signed-in"); + expect(display).toBe("configured"); + }); +}); diff --git a/src/components/features/onboarding/steps/setup-acp-secrets-step.tsx b/src/components/features/onboarding/steps/setup-acp-secrets-step.tsx index fdbef5cda7..270b21eb51 100644 --- a/src/components/features/onboarding/steps/setup-acp-secrets-step.tsx +++ b/src/components/features/onboarding/steps/setup-acp-secrets-step.tsx @@ -77,6 +77,7 @@ export function SetupAcpSecretsStep({ secretExists, hasValueFor, conflicts, + credentialsConfigured, consumesFileCredentials, save, isSaving, @@ -170,6 +171,7 @@ export function SetupAcpSecretsStep({ diff --git a/src/components/features/settings/acp-auth-status-banner.tsx b/src/components/features/settings/acp-auth-status-banner.tsx index 1d808b47cf..cf7cda2e61 100644 --- a/src/components/features/settings/acp-auth-status-banner.tsx +++ b/src/components/features/settings/acp-auth-status-banner.tsx @@ -1,35 +1,60 @@ -import { Check, Loader2 } from "lucide-react"; +import { Check, KeyRound, Loader2 } from "lucide-react"; import { useTranslation } from "react-i18next"; import { I18nKey } from "#/i18n/declaration"; import type { AcpAuthStatus } from "#/hooks/query/use-acp-auth-status"; +import { resolveAcpAuthDisplay } from "#/utils/acp-auth-display"; interface AcpAuthStatusBannerProps { status: AcpAuthStatus; isChecking: boolean; providerName: string; + /** + * Whether a credential for the provider exists in the active backend's secret + * store. On Docker/cloud backends the host-login probe can't run (it shells + * the interactive CLI, which isn't installed there), so this is the only + * accurate "the agent will authenticate" signal. Optional; defaults to + * ``false`` so existing callers behave exactly as before. + */ + credentialsConfigured?: boolean; /** * Prefix for the banner test ids, e.g. ``"onboarding-acp-auth"`` → - * ``onboarding-acp-auth-detected`` / ``onboarding-acp-auth-checking``. + * ``onboarding-acp-auth-detected`` / ``onboarding-acp-auth-checking`` / + * ``onboarding-acp-auth-configured``. */ testIdPrefix: string; } /** * Auth-status banner shared by the ACP credential forms (the onboarding step - * and Settings → Agent): a green "already signed in" banner when the local - * login probe detects a session, or a spinner while it's checking. Renders - * nothing otherwise (unauthenticated / unknown / non-local backend), so the - * caller falls back to the API-key fields. + * and Settings → Agent): + * + * - a green "already signed in" banner when the host-login probe detects a + * session (native backend), + * - a spinner while the probe is checking, + * - a neutral "credentials configured" banner when the probe can't confirm a + * login but a credential for the provider is stored — the accurate signal on + * Docker/cloud backends, where the probe goes silent, + * - nothing otherwise, so the caller falls back to the API-key fields. + * + * A stored credential never renders as "signed in": only the probe confirms an + * actual host login. See issue #1244. */ export function AcpAuthStatusBanner({ status, isChecking, providerName, + credentialsConfigured = false, testIdPrefix, }: AcpAuthStatusBannerProps) { const { t } = useTranslation("openhands"); - if (status === "authenticated") { + const display = resolveAcpAuthDisplay({ + status, + isChecking, + credentialsConfigured, + }); + + if (display === "signed-in") { return (
+ + + {t(I18nKey.ONBOARDING$ACP_CREDENTIALS_CONFIGURED, { + provider: providerName, + })} + +
+ ); + } + return null; } diff --git a/src/components/features/settings/acp-credentials-section.tsx b/src/components/features/settings/acp-credentials-section.tsx index 028f1f6494..c11075471a 100644 --- a/src/components/features/settings/acp-credentials-section.tsx +++ b/src/components/features/settings/acp-credentials-section.tsx @@ -24,7 +24,14 @@ export function AcpCredentialsSection({ providerKey: string; }) { const { t } = useTranslation("openhands"); - const { fields, values, setValue, secretExists, conflicts } = form; + const { + fields, + values, + setValue, + secretExists, + conflicts, + credentialsConfigured, + } = form; const { status: authStatus, isChecking } = useAcpAuthStatus(providerKey); const providerName = getAcpProviderDisplayName(providerKey) ?? providerKey; @@ -44,6 +51,7 @@ export function AcpCredentialsSection({ diff --git a/src/hooks/use-acp-credential-form.ts b/src/hooks/use-acp-credential-form.ts index f78a888a5b..92589462dc 100644 --- a/src/hooks/use-acp-credential-form.ts +++ b/src/hooks/use-acp-credential-form.ts @@ -19,6 +19,12 @@ export interface AcpCredentialForm { hasValueFor: (name: string) => boolean; /** ``[credential, conflicting]`` pairs currently both set (typed or saved). */ conflicts: Array<[string, string]>; + /** Whether a credential (a ``secret`` field — API key, OAuth token, or + * file-content blob) is already saved on the backend for this provider. This + * is the backend-truthful auth signal that works on Docker/cloud, where the + * host-login probe can't run (agent-canvas#1244). A non-credential field (a + * base URL or GCP scalar) being set does not count. */ + credentialsConfigured: boolean; /** Whether the active backend can materialise file-content (``multiline``) * credentials to disk. False on cloud (agent-canvas#1016), where such a * credential would be orphaned. */ @@ -92,6 +98,13 @@ export function useAcpCredentialForm( secretExists, hasValueFor, conflicts: getAcpCredentialConflicts(providerKey, hasValueFor), + // `=== true` is the intended strict check: only a credential-bearing field + // (API key, OAuth token, file blob) counts — never a base URL or GCP scalar, + // which omit `secret` entirely. A field must never set `secret: false` to + // opt out; omit the flag instead. + credentialsConfigured: fields.some( + (field) => field.secret === true && secretExists(field.name), + ), consumesFileCredentials, isDirty: fields.some((field) => Boolean(values[field.name]?.trim())), save: (options) => saveFilled(values, options), diff --git a/src/i18n/translation.json b/src/i18n/translation.json index c9cd066cb7..caaaeeff43 100644 --- a/src/i18n/translation.json +++ b/src/i18n/translation.json @@ -271,6 +271,23 @@ "zh-CN": "正在检查现有的 {{provider}} 登录…", "zh-TW": "正在檢查現有的 {{provider}} 登入…" }, + "ONBOARDING$ACP_CREDENTIALS_CONFIGURED": { + "ar": "بيانات اعتماد {{provider}} مُهيّأة — يمكنك ترك الحقول أدناه فارغة.", + "ca": "Les credencials de {{provider}} estan configurades — pots deixar els camps de sota en blanc.", + "de": "Die Anmeldedaten für {{provider}} sind konfiguriert — Sie können die folgenden Felder leer lassen.", + "en": "Credentials for {{provider}} are configured — you can leave the fields below blank.", + "es": "Las credenciales de {{provider}} están configuradas — puedes dejar los campos de abajo en blanco.", + "fr": "Les identifiants pour {{provider}} sont configurés — vous pouvez laisser les champs ci-dessous vides.", + "it": "Le credenziali per {{provider}} sono configurate — puoi lasciare vuoti i campi qui sotto.", + "ja": "{{provider}}の認証情報が設定されています — 以下のフィールドは空欄のままで構いません。", + "ko-KR": "{{provider}}의 자격 증명이 구성되어 있습니다 — 아래 항목은 비워 두셔도 됩니다.", + "no": "Legitimasjonen for {{provider}} er konfigurert — du kan la feltene nedenfor stå tomme.", + "pt": "As credenciais de {{provider}} estão configuradas — pode deixar os campos abaixo em branco.", + "tr": "{{provider}} kimlik bilgileri yapılandırıldı — aşağıdaki alanları boş bırakabilirsiniz.", + "uk": "Облікові дані для {{provider}} налаштовано — поля нижче можна залишити порожніми.", + "zh-CN": "{{provider}} 的凭据已配置 — 下面的字段可以留空。", + "zh-TW": "{{provider}} 的憑證已設定 — 下方欄位可以留空。" + }, "MAINTENANCE$SCHEDULED_MESSAGE": { "en": "Scheduled maintenance will begin at {{time}}", "ja": "予定されたメンテナンスは{{time}}に開始されます", diff --git a/src/utils/acp-auth-display.ts b/src/utils/acp-auth-display.ts new file mode 100644 index 0000000000..cce5f89ded --- /dev/null +++ b/src/utils/acp-auth-display.ts @@ -0,0 +1,46 @@ +import type { AcpAuthStatus } from "#/hooks/query/use-acp-auth-status"; + +/** + * Resolved display state for the ACP auth banner. + * + * - ``"signed-in"`` — the host-login probe confirmed a session (native backend). + * - ``"checking"`` — the first probe is still in flight. + * - ``"configured"`` — the probe can't confirm a login (e.g. a container with no + * interactive CLI, or a cloud backend where the probe doesn't run), but a + * credential for the provider exists in the active backend's secret store. + * - ``"none"`` — no confirmed login and no stored credential; show nothing. + */ +export type AcpAuthDisplay = "signed-in" | "checking" | "configured" | "none"; + +interface AcpAuthDisplayInput { + /** Result of the host-login probe (only meaningful on a native backend). */ + status: AcpAuthStatus; + /** True while the first probe for this provider is in flight. */ + isChecking: boolean; + /** + * Whether a credential for the provider exists in the active backend's secret + * store. Unlike {@link status}, this signal is available on Docker and cloud + * backends, so it can convey an accurate state where the probe goes silent. + */ + credentialsConfigured: boolean; +} + +/** + * Decide what the ACP auth banner should show, given the (host-only) login + * probe and the backend-truthful "a credential is stored" signal. + * + * Precedence preserves the existing banner behavior — a probe-confirmed login + * wins, then the in-flight spinner — and only adds ``"configured"`` in the gap + * that previously rendered nothing. A stored credential never reports as + * ``"signed-in"``: only the probe can confirm an actual host login. See #1244. + */ +export function resolveAcpAuthDisplay({ + status, + isChecking, + credentialsConfigured, +}: AcpAuthDisplayInput): AcpAuthDisplay { + if (status === "authenticated") return "signed-in"; + if (isChecking) return "checking"; + if (credentialsConfigured) return "configured"; + return "none"; +} diff --git a/tests/e2e/mock-llm/settings/mock-llm-acp-auth-banner.spec.ts b/tests/e2e/mock-llm/settings/mock-llm-acp-auth-banner.spec.ts new file mode 100644 index 0000000000..9059c0783f --- /dev/null +++ b/tests/e2e/mock-llm/settings/mock-llm-acp-auth-banner.spec.ts @@ -0,0 +1,185 @@ +/** + * Mock-LLM E2E test: ACP "credentials configured" auth banner (issue #1244). + * + * Regression coverage for the banner state added in #1244. On a backend where + * the host-login probe can't confirm a session — a Docker/cloud agent-server, + * which ships the ACP wrappers but not the interactive `claude` CLI — the probe + * classifies as `unknown`. Before the fix the banner rendered nothing, so a + * provider credential saved to the backend store looked identical to "no + * credentials at all". The fix surfaces a neutral "credentials configured" + * banner from the secret store (the one signal that works on Docker/cloud) + * WITHOUT ever claiming a verified host login. + * + * Flow (Settings → Agent profiles — the #1571 profile library — the same way a + * user configures a built-in provider): + * 1. Seed a Claude credential (CLAUDE_CODE_OAUTH_TOKEN) directly in the + * backend secret store via the secrets API. The value is never read or + * validated by the banner — the store only needs the name present — so a + * placeholder is sufficient and no real/PAYG credential is required. + * 2. Open the "default" agent profile and switch it to ACP → Claude Code so + * the credentials section + auth banner mount. + * 3. With the host-login probe inconclusive, the stored credential surfaces + * the neutral "configured" banner. + * 4. The green "signed in" banner does NOT appear (the honesty guard — a + * stored credential is not a verified login). + * + * Determinism note: the assertion requires the host-login probe to be + * inconclusive (`unknown`). That is always true on the containerized + * agent-server this suite runs against (no interactive `claude` CLI) and on a + * clean CI runner. If the suite is ever run against a backend whose host HAS a + * real Claude login, the probe legitimately reports "signed in" and the + * "configured" precondition can't be reached — the test skips with a reason + * rather than false-failing. + */ + +import type { Page } from "@playwright/test"; +import { test, expect } from "@playwright/test"; +import { + selectDropdownOption, + resetToOpenHandsAgentViaUI, + openAgentProfileEditor, + BACKEND_URL, + SESSION_API_KEY, +} from "../utils/mock-llm-helpers"; + +const CLAUDE_PROVIDER_NAME = "Claude Code"; +const OAUTH_TOKEN_SECRET = "CLAUDE_CODE_OAUTH_TOKEN"; +// A placeholder value — the banner reads only whether the secret NAME exists in +// the backend store; it never authenticates, so no real credential is needed. +const PLACEHOLDER_TOKEN = "e2e-placeholder-not-a-real-token"; + +/** + * Seed onboarding flags + a single local backend, like the shared + * {@link seedLocalStorage}, but point the backend at ``BACKEND_URL`` rather than + * ``window.location.origin``. In CI those are the same value (the config serves + * the browser and the backend from one ingress origin), so behaviour is + * unchanged there; pointing at ``BACKEND_URL`` additionally lets the suite run + * against a standalone backend (e.g. ``examples/acp-docker`` on a separate port + * during local development). + */ +async function seedBackend(page: Page) { + await page.addInitScript( + ({ host, apiKey }) => { + localStorage.setItem("analytics-consent", "false"); + localStorage.setItem("openhands-telemetry-consent", "denied"); + localStorage.setItem("openhands-telemetry-first-use", "true"); + localStorage.setItem("openhands-onboarded", "1"); + localStorage.setItem( + "openhands-backends", + JSON.stringify([ + { id: "default-local", name: "Local", host, apiKey, kind: "local" }, + ]), + ); + }, + { host: BACKEND_URL, apiKey: SESSION_API_KEY }, + ); +} + +test.describe.configure({ mode: "serial" }); + +test.describe("mock-LLM ACP credentials-configured banner (#1244)", () => { + test.beforeAll(async ({ request }) => { + // Seed a Claude credential so the banner resolver sees a stored secret when + // the credentials section mounts. PUT /api/settings/secrets is the + // upsert-by-name endpoint the app itself uses (secrets-service.ts). The + // value is never authenticated by the banner, so a placeholder suffices. + // No catch — a seed failure should surface here, not as a mystery at the + // banner assertion. + await request.put(`${BACKEND_URL}/api/settings/secrets`, { + headers: { "X-Session-API-Key": SESSION_API_KEY }, + data: { name: OAUTH_TOKEN_SECRET, value: PLACEHOLDER_TOKEN }, + }); + }); + + test.beforeEach(async ({ page }) => { + await seedBackend(page); + }); + + test.afterAll(async ({ request, browser }) => { + // Remove the saved credential so other suites start from a clean store. + try { + await request.delete( + `${BACKEND_URL}/api/settings/secrets/${OAUTH_TOKEN_SECRET}`, + { headers: { "X-Session-API-Key": SESSION_API_KEY } }, + ); + } catch { + // best-effort + } + // Reset agent_kind back to OpenHands so suites expecting the default agent + // aren't affected by our ACP selection. + const page = await browser.newPage(); + try { + await seedBackend(page); + await resetToOpenHandsAgentViaUI(page); + } catch { + // best-effort + } finally { + await page.close(); + } + }); + + test("Claude credential in the store surfaces a 'configured' banner, never 'signed in'", async ({ + page, + }) => { + // #1571 turned Settings → Agent into a profile library: the form is reached + // via /settings/agents (plural) by opening a profile editor, not the old + // standalone /settings/agent route. + await openAgentProfileEditor(page, "default"); + + const configuredBanner = page.getByTestId("settings-acp-auth-configured"); + const signedInBanner = page.getByTestId("settings-acp-auth-detected"); + const checkingBanner = page.getByTestId("settings-acp-auth-checking"); + + // Arm the probe-settled wait BEFORE selecting the provider (selecting the + // Claude Code preset is what fires the host-login probe). The probe POSTs + // the provider's status command to the agent-server bash endpoint, so + // settling on that response is deterministic. Waiting for the "checking" + // banner to hide is not: `waitFor({ state: "hidden" })` is satisfied by + // "not in the DOM", so it resolves immediately if the spinner hasn't + // painted yet, and the skip-precondition below would then be read too + // early — on a host with a real login the probe later reports "signed in" + // and the test false-fails instead of skipping (see #1244). + const probeSettled = page + .waitForResponse( + (res) => + res.url().includes("/api/bash/execute_bash_command") && + res.request().method() === "POST", + { timeout: 10_000 }, + ) + .catch(() => null); + + // ── Select ACP → Claude Code ───────────────────────────────────────── + await selectDropdownOption(page, /Agent/, /ACP/); + await selectDropdownOption( + page, + /Preset/, + new RegExp(CLAUDE_PROVIDER_NAME), + ); + + // The Claude credential field renders for the built-in provider. + const tokenField = page.getByTestId( + `settings-acp-secret-${OAUTH_TOKEN_SECRET}`, + ); + await expect(tokenField).toBeVisible({ timeout: 10_000 }); + + // Let the host-login probe settle: wait for its bash response, then for + // React Query to render the terminal banner state (the spinner clears). + // The probe is gated to local backends and runs once per provider. If it + // reports a real login, the "configured" precondition is unreachable on + // this host — skip rather than false-fail. + await probeSettled; + await expect(checkingBanner).toHaveCount(0, { timeout: 10_000 }); + test.skip( + (await signedInBanner.count()) > 0, + "host-login probe reported a verified login; the credentials-configured " + + "state only applies when the probe is inconclusive (Docker/cloud/CI)", + ); + + // ── The seeded credential surfaces the neutral "configured" banner … ─ + await expect(configuredBanner).toBeVisible({ timeout: 10_000 }); + await expect(configuredBanner).toContainText(/configured/i); + + // ── … and the green "signed in" banner never does (honesty guard). ─── + await expect(signedInBanner).toHaveCount(0); + }); +});