From e9828f8bf4b9fe24ee2bcac781080aaa98626f6c Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 10 Sep 2026 12:57:53 -0500 Subject: [PATCH] fix: reuse saved model connections during agent setup (#13161) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agent setup connects each agent to a model provider. > - The organization can already hold subscription logins and API keys. > - The simplified setup flow did not consistently offer those saved credentials. > - This pull request restores reuse and selects a saved connection by default. > - Agents keep secret references, so reuse does not copy or rotate credentials. ## Linked Issues or Issue Description Related change: #13011. Searched public issues and PRs; no duplicate fix found. **What happened?** Onboarding and new-agent setup could ask for a new API key or sign-in despite an existing saved connection. A general environment auth signal could also be mistaken for the owner's saved Claude subscription. **Expected behavior** Offer saved credentials from the selected organization. Default to a saved subscription when one exists. Otherwise select a saved API key. Keep the option to enter a new key or sign in to another account. **Steps to reproduce** 1. Save a Claude or OpenAI API key, or complete a supported subscription login. 2. Add another agent with the same provider. 3. Open the provider connection step. 4. Check whether the saved credential is available and selected. **Paperclip version or commit** Reproduced on 5cb4f061d after #13011. This branch is rebased onto current master. **Deployment mode** Built from source. Tested in an isolated local test drive with embedded storage and board access. ## What Changed - Add a shared saved-credential lookup and picker for active personal and organization keys. - Reuse saved Claude subscriptions and saved Codex account homes. Select an existing connection by default. - Preserve secret references through connection tests and agent creation, including the native Claude and Codex runner setup paths. - Store newly entered onboarding keys separately. Do not rotate another agent's key. - Keep explicit choices during metadata refresh. Prevent refreshes from remounting an active login panel. - Add integration tests and production-component Storybook stories. Document connection reuse. ## Verification - All 5,628 UI tests passed before rebase. - Twenty targeted server credential tests passed. - UI typecheck, UI build, token gates, and diff whitespace checks passed. - Browser walkthroughs covered onboarding and new-agent setup, saved keys, saved subscription fixtures, and new sign-in screens. - Live Claude and Codex API-key probes succeeded. Created both agents and confirmed that each retained its saved-secret reference. Both secret versions remained unchanged. Codex passed after one retry. - Live subscription authentication was not repeated. Subscription flows use fixture browser tests and integration tests. - After rebase and the cache fix, all 109 focused onboarding and agent-creation tests passed. - Full repository `pnpm build` and `pnpm -r typecheck` passed. - The full local test attempt encountered timeouts and embedded PostgreSQL startup failures under parallel load. All four affected suites passed in isolation: 20 tests, with no code changes. The complete CI matrix passed, including all workspace, general server, serialized server, browser end-to-end, build, typecheck, and canary dry-run checks. - Greptile reviewed commit d53ddf6b82c101d35894587afc9b0d135a5abc55: 5/5, successful check, no review threads. ## Risks - The default connection mode changes when saved credentials exist. A saved subscription takes priority over saved API keys; personal keys appear before organization keys. - A listed credential can be expired or unavailable in the selected environment. The existing connection test still checks it. - No database migration or API contract change is required. ## Model Used OpenAI Codex, GPT-6. The exact runtime model identifier and context-window size are not exposed in this session. Used reasoning, code execution, repository tools, and browser automation. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- docs/guides/board-operator/managing-agents.md | 20 ++ ui/src/components/OnboardingWizard.test.tsx | 81 +++--- ui/src/components/OnboardingWizard.tsx | 126 +++++---- .../AgentProviderConnection.test.tsx | 244 ++++++++++++++++++ .../new-agent/AgentProviderConnection.tsx | 151 ++++++++--- .../onboarding/SavedProviderKeySelect.tsx | 122 +++++++++ ui/src/lib/saved-provider-credentials.test.ts | 95 +++++++ ui/src/lib/saved-provider-credentials.ts | 79 ++++++ ui/src/pages/DesignGuide.tsx | 7 + ui/src/pages/NewAgent.test.tsx | 30 ++- ui/storybook/.storybook/preview.tsx | 24 +- .../fixtures/onboardingEnvironment.ts | 9 + .../agent-connection-reuse.stories.tsx | 140 ++++++++++ .../stories/onboarding-agent-arc.stories.tsx | 46 +++- 14 files changed, 1052 insertions(+), 122 deletions(-) create mode 100644 ui/src/components/new-agent/AgentProviderConnection.test.tsx create mode 100644 ui/src/components/onboarding/SavedProviderKeySelect.tsx create mode 100644 ui/src/lib/saved-provider-credentials.test.ts create mode 100644 ui/src/lib/saved-provider-credentials.ts create mode 100644 ui/storybook/stories/agent-connection-reuse.stories.tsx diff --git a/docs/guides/board-operator/managing-agents.md b/docs/guides/board-operator/managing-agents.md index 1ada4ec87e..663767048a 100644 --- a/docs/guides/board-operator/managing-agents.md +++ b/docs/guides/board-operator/managing-agents.md @@ -40,6 +40,26 @@ should call that server. Both are built-in adapter types from the unified For `opencode_local`, configure an explicit `adapterConfig.model` (`provider/model`). Paperclip validates the selected model against live `opencode models` output. +### Reusing model connections + +Both onboarding and the new-agent connection step can reuse saved credentials +in the selected organization. A saved subscription is the default when available; +otherwise a saved API key is selected automatically. Personal keys appear before +organization keys. You can still choose a new key or another account: + +- Claude can use your saved subscription login without another sign-in. +- OpenAI lists ChatGPT accounts saved by Paperclip's Codex sign-in flow. Choose + an account or select **Sign in to another account**. +- In API-key mode, choose a saved personal or organization provider key, or + enter a new key. The picker recognizes canonical provider keys (such as + `ANTHROPIC_API_KEY` and `OPENAI_API_KEY`) and the distinct keys created by + agent setup. + +Reusing a connection binds its secret reference to the agent. It does not copy +or rotate the saved value. The connection is tested before the agent is created; +being listed does not guarantee that a provider still accepts the credential. +These choices also apply to the Claude and Codex native runner setup paths. + ## Agent Hiring via Governance Agents can request to hire subordinates. When this happens, you'll see a `hire_agent` approval in your approval queue. Review the proposed agent config and approve or reject. diff --git a/ui/src/components/OnboardingWizard.test.tsx b/ui/src/components/OnboardingWizard.test.tsx index 6b63e02480..15d35e13d5 100644 --- a/ui/src/components/OnboardingWizard.test.tsx +++ b/ui/src/components/OnboardingWizard.test.tsx @@ -131,6 +131,8 @@ const mockApprovalsApi = vi.hoisted(() => ({ create: vi.fn(), })); const mockSecretsApi = vi.hoisted(() => ({ + list: vi.fn(), + removeUserSecretDefinition: vi.fn(), listMyUserSecrets: vi.fn(), createUserSecretDefinition: vi.fn(), createMyUserSecret: vi.fn(), @@ -314,6 +316,9 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( session: { id: "session-b", userId: SESSION_USER_ID }, user: { id: SESSION_USER_ID, name: "B", email: "b@example.com", image: null }, }); + mockSecretsApi.list.mockResolvedValue([]); + mockSecretsApi.listMyUserSecrets.mockResolvedValue([]); + mockSecretsApi.removeUserSecretDefinition.mockResolvedValue({ ok: true }); window.localStorage.clear(); mockDialog.onboardingOpen = true; mockDialog.onboardingOptions = {}; @@ -855,6 +860,32 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( * what this step did before, and what the Claude token path has always * avoided by holding a `user_secret_ref` instead. */ + it.each(["personal", "organization"])("defaults to a saved %s API key and uses the same reference for probe and hire", async (scope) => { + const key = "ANTHROPIC_API_KEY"; + const binding = scope === "personal" + ? { type: "user_secret_ref", key, version: "latest" } + : { type: "secret_ref", secretId: "saved-org-key", version: "latest" }; + if (scope === "personal") { + mockSecretsApi.listMyUserSecrets.mockResolvedValue([{ + definition: { id: "saved-key", companyId: "company-new", key, name: "Saved key", status: "active" }, + secret: { companyId: "company-new", status: "active" }, + }]); + } else { + mockSecretsApi.list.mockResolvedValue([{ + id: "saved-org-key", companyId: "company-new", key, name: "Saved key", scope: "company", status: "active", + }]); + } + const { root, clickByText } = await openConnectStep(); + const picker = document.body.querySelector('select[aria-label="Saved API key"]') as HTMLSelectElement; + expect(picker.value).toBe(scope === "personal" ? "user:saved-key" : "company:saved-org-key"); + await clickByText((t) => isArcPrimary(t)); + expect((mockAgentsApi.testEnvironment.mock.calls.at(-1) as unknown[])[2]).toMatchObject({ adapterConfig: { env: { [key]: binding } } }); + expect((mockAgentsApi.hire.mock.calls.at(-1) as unknown[])[1]).toMatchObject({ adapterConfig: { env: { [key]: binding } } }); + expect(mockSecretsApi.createMyUserSecret).not.toHaveBeenCalled(); + expect(mockSecretsApi.rotateMyUserSecret).not.toHaveBeenCalled(); + await act(async () => root.unmount()); + }); + describe("an API key typed on the step", () => { const KEY = "sk-ant-typed-by-the-customer"; @@ -892,7 +923,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( string, { definitionKey: string; value: string }, ]; - expect(createBody.definitionKey).toBe("ANTHROPIC_API_KEY"); + expect(createBody.definitionKey).toMatch(/^ANTHROPIC_API_KEY\.setup\./); expect(createBody.value).toBe(KEY); const hireBody = (mockAgentsApi.hire.mock.calls.at(-1) as unknown[])[1] as { @@ -901,7 +932,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( // The same binding kind the subscription half of this step produces. expect(hireBody.adapterConfig.env?.ANTHROPIC_API_KEY).toEqual({ type: "user_secret_ref", - key: "ANTHROPIC_API_KEY", + key: createBody.definitionKey, version: "latest", }); // The whole payload, not just that one field: the point is that the key @@ -911,39 +942,16 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( await act(async () => root.unmount()); }); - // Onboarding is the first thing to need this definition, so it creates it. - it("creates the definition once, then reuses it", async () => { - await connectWithApiKey(); - expect(mockSecretsApi.createUserSecretDefinition).toHaveBeenCalledTimes(1); - + it("creates a distinct definition instead of rotating an existing key", async () => { mockSecretsApi.listMyUserSecrets.mockResolvedValue([ - { definition: { id: "def-1", key: "ANTHROPIC_API_KEY" }, secret: null }, + { definition: { id: "old-def", key: "ANTHROPIC_API_KEY" }, secret: { id: "secret-existing" } }, ]); const { root } = await connectWithApiKey(); - - expect(mockSecretsApi.createUserSecretDefinition).toHaveBeenCalledTimes(1); - - await act(async () => root.unmount()); - }); - - // A second value against one definition is what the server refuses, so a - // customer who already has a key stored must rotate rather than add. - it("rotates an existing value instead of storing a second one", async () => { - mockSecretsApi.listMyUserSecrets.mockResolvedValue([ - { - definition: { id: "def-1", key: "ANTHROPIC_API_KEY" }, - secret: { id: "secret-existing" }, - }, - ]); - const { root } = await connectWithApiKey(); - - expect(mockSecretsApi.rotateMyUserSecret).toHaveBeenCalledWith( - expect.any(String), - "secret-existing", - { value: KEY }, + expect(mockSecretsApi.createUserSecretDefinition).toHaveBeenCalledWith( + expect.any(String), expect.objectContaining({ key: expect.stringMatching(/^ANTHROPIC_API_KEY\.setup\./) }), ); - expect(mockSecretsApi.createMyUserSecret).not.toHaveBeenCalled(); - + expect(mockSecretsApi.rotateMyUserSecret).not.toHaveBeenCalled(); + expect(mockSecretsApi.createMyUserSecret).toHaveBeenCalledTimes(1); await act(async () => root.unmount()); }); @@ -1099,9 +1107,9 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( await clickByText((t) => isArcPrimary(t)); expect(mockAgentsApi.hire).toHaveBeenCalled(); - // The status route must not even be asked — the conflict is decided - // from the adapter configuration alone, before any network round trip. - expect(mockAgentsApi.getClaudeOAuthTokenStatus).not.toHaveBeenCalled(); + // Discovery reads saved-login metadata once; the hire does not re-read + // or apply it when the configuration already has an API key. + expect(mockAgentsApi.getClaudeOAuthTokenStatus).toHaveBeenCalledTimes(1); const hireArgs = mockAgentsApi.hire.mock.calls.at(-1) as unknown[]; const hireBody = hireArgs[1] as { adapterConfig: { env?: Record }; @@ -1263,11 +1271,12 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( mockAgentsApi.hire.mockRejectedValue(new Error("hire failed")); const { root, clickByText } = await openConnectStep(); + const discoveryReads = mockAgentsApi.getClaudeOAuthTokenStatus.mock.calls.length; await clickByText((t) => isArcPrimary(t)); - expect(mockAgentsApi.getClaudeOAuthTokenStatus).toHaveBeenCalledTimes(1); + expect(mockAgentsApi.getClaudeOAuthTokenStatus).toHaveBeenCalledTimes(discoveryReads + 1); await clickByText((t) => isArcPrimary(t)); - expect(mockAgentsApi.getClaudeOAuthTokenStatus).toHaveBeenCalledTimes(2); + expect(mockAgentsApi.getClaudeOAuthTokenStatus).toHaveBeenCalledTimes(discoveryReads + 2); await act(async () => root.unmount()); }); diff --git a/ui/src/components/OnboardingWizard.tsx b/ui/src/components/OnboardingWizard.tsx index 8c1dea01c2..2b7af85e04 100644 --- a/ui/src/components/OnboardingWizard.tsx +++ b/ui/src/components/OnboardingWizard.tsx @@ -1,3 +1,5 @@ +import { storeProviderApiKey } from "../lib/provider-credential"; +import { SavedProviderKeySelect, useSavedProviderKeys } from "./onboarding/SavedProviderKeySelect"; import { useEffect, useState, useMemo, useRef } from "react"; import type { ComponentType, CSSProperties } from "react"; import { useQuery, useQueryClient } from "@tanstack/react-query"; @@ -620,8 +622,10 @@ function OnboardingWizardInner({ * picked keys, left, and came back should not be handed a sign-in panel they * already said no to. */ - const [credentialMode, setCredentialMode] = useState( - (saved?.credentialMode as CredentialMode) ?? "subscription", + const [credentialModeChoice, setCredentialMode] = useState( + (saved?.credentialModeChoice !== undefined + ? saved.credentialModeChoice as CredentialMode | null + : saved?.credentialMode as CredentialMode | undefined) ?? null, ); /** * Where the connect step's sign-in sequence is. @@ -661,6 +665,28 @@ function OnboardingWizardInner({ const [createdCompanyId, setCreatedCompanyId] = useState( existingCompanyId ?? (saved?.createdCompanyId as string) ?? null ); + const savedKeys = useSavedProviderKeys( + createdCompanyId, + apiKeyEnvKeyFor(adapterType), + effectiveOnboardingOpen && step === 4, + ); + const [subscriptionId, setSubscriptionId] = useState<{ companyId: string; id: string } | null>(null); + const savedSubscription = adapterType === "codex_local" + ? savedKeys.subscriptions.find((option) => option.id === ( + subscriptionId?.companyId === createdCompanyId + ? subscriptionId.id + : savedKeys.subscriptions[0]?.id + )) + : undefined; + const [selectedSavedKey, setSelectedSavedKey] = useState<{ companyId: string; envKey: string; id: string } | null>(null); + const selectedApiKeyId = selectedSavedKey?.companyId === createdCompanyId && selectedSavedKey?.envKey === apiKeyEnvKeyFor(adapterType) + ? selectedSavedKey.id + : savedKeys.options[0]?.id; + const selectedApiKey = savedKeys.options.find((option) => option.id === selectedApiKeyId); + const credentialMode = credentialModeChoice ?? ( + (adapterType === "claude_local" ? savedKeys.storedLogin.data : adapterType === "codex_local" && savedKeys.subscriptions.length) + ? "subscription" : savedKeys.options.length ? "api" : "subscription" + ); const [createdCompanyPrefix, setCreatedCompanyPrefix] = useState< string | null >((saved?.createdCompanyPrefix as string) ?? null); @@ -706,7 +732,7 @@ function OnboardingWizardInner({ * customer on the step to try again — and without this each press would store * another copy of the same credential. */ - const apiKeySecretRef = useRef<{ key: string } | null>(null); + const apiKeySecretRef = useRef<{ key: string; companyId: string; envKey: string; binding: Awaited>["binding"] } | null>(null); createdCompanyIdRef.current = createdCompanyId; // The step the request wants, mirrored for the same reason. `initialStep` is @@ -843,7 +869,7 @@ function OnboardingWizardInner({ step, companyName, agentName, agentRole, adapterType, cwd, model, command, args, url, // The mode, never the key: this blob is localStorage. - credentialMode, + credentialMode, credentialModeChoice, createdCompanyId, createdCompanyPrefix, createdAgentId, createdCompanyGoalId, createdProjectId, createdIssueRef, }; @@ -851,7 +877,7 @@ function OnboardingWizardInner({ }, [ effectiveOnboardingOpen, step, companyName, agentName, agentRole, adapterType, cwd, model, command, args, url, - credentialMode, + credentialMode, credentialModeChoice, createdCompanyId, createdCompanyPrefix, createdAgentId, createdCompanyGoalId, createdProjectId, createdIssueRef, ]); @@ -1009,7 +1035,7 @@ function OnboardingWizardInner({ Boolean(createdCompanyId) && effectiveOnboardingOpen && step === 4 && canShowAdapterLogin, }); useEffect(() => { - if (!activeLoginSessionQuery.data) return; + if (!activeLoginSessionQuery.data || credentialMode === "api" || savedSubscription || savedKeys.storedLogin.data) return; // Re-derive the row's answer along with the sequence: a resumed session // implies a source was already picked, and the row stays a question // otherwise (see `sourcePicked` above). @@ -1020,7 +1046,7 @@ function OnboardingWizardInner({ // prompt through `onPromptReady`, below, which is what moves this beat // from `loading` to `ready`, exactly as a fresh press would. setConnectPhase((phase) => (phase === "idle" ? "loading" : phase)); - }, [activeLoginSessionQuery.data]); + }, [activeLoginSessionQuery.data, credentialMode, savedSubscription, savedKeys.storedLogin.data]); /** * The signal is being fetched and has not answered yet. * @@ -1089,7 +1115,7 @@ function OnboardingWizardInner({ * Anything that gates this step belongs in here, so the next one is added * once rather than twice. */ - const connectStepReady = sourceSelected && !adapterEnvLoading; + const connectStepReady = sourceSelected && !adapterEnvLoading && !savedKeys.loading; /** * Whether this step has a sign-in to do before it can hire. @@ -1102,7 +1128,10 @@ function OnboardingWizardInner({ */ const connectStepNeedsLogin = Boolean( credentialMode !== "api" && - showAdapterLoginPanel && + (showAdapterLoginPanel || (canShowAdapterLogin && adapterType === "codex_local" && subscriptionId?.companyId === createdCompanyId && subscriptionId.id === "")) && + !savedSubscription && + !(adapterType === "claude_local" && savedKeys.storedLogin.data) && + !savedKeys.loading && createdCompanyId && resolvedLoginEnvironmentId, ); @@ -1258,7 +1287,7 @@ function OnboardingWizardInner({ label: "Connect", icon: "arrow", disabled: - !connectStepReady || (credentialMode === "api" && !apiKey.trim()), + !connectStepReady || (credentialMode === "api" && !apiKey.trim() && !selectedApiKey), } : // Nothing is chosen on arrival, and the row is what chooses. Until // it has been answered the button has nothing to do. @@ -1404,7 +1433,7 @@ function OnboardingWizardInner({ setAdapterEnvResult(null); adapterEnvResultAppliedStoredLoginRef.current = false; setAdapterEnvError(null); - }, [step, adapterType, model, command, args, url, credentialMode, apiKey]); + }, [step, adapterType, model, command, args, url, credentialMode, apiKey, selectedSavedKey, selectedApiKey?.id, subscriptionId, savedSubscription?.id]); /** * Leaving the step puts the row back to a question. @@ -1646,7 +1675,7 @@ function OnboardingWizardInner({ * * A user secret needs a definition to hang off. The Claude token's is fixed * and server-owned; there is no such definition for API keys, so onboarding - * creates one on first use. That needs company owner or admin rights, which + * creates a distinct definition for each new key, preserving existing keys. That needs company owner or admin rights, which * whoever just created this company in onboarding has. * * Returns false on failure, having set the error. Callers must treat false as @@ -1656,31 +1685,10 @@ function OnboardingWizardInner({ async function storeApiKeyUserSecret(companyId: string): Promise { const key = apiKey.trim(); const envKey = apiKeyEnvKeyFor(adapterType); - if (apiKeySecretRef.current?.key === key) return true; + if (apiKeySecretRef.current?.key === key && apiKeySecretRef.current.companyId === companyId && apiKeySecretRef.current.envKey === envKey) return true; try { - const entries = await secretsApi.listMyUserSecrets(companyId); - const existing = entries.find((entry) => entry.definition.key === envKey); - const definitionId = - existing?.definition.id ?? - ( - await secretsApi.createUserSecretDefinition(companyId, { - key: envKey, - name: `${envKey} for onboarding`, - description: "Created while connecting a model during onboarding.", - }) - ).id; - // Rotate rather than create when a value is already stored, because - // creating a second value for one definition is what the server refuses. - if (existing?.secret) { - await secretsApi.rotateMyUserSecret(companyId, existing.secret.id, { value: key }); - } else { - await secretsApi.createMyUserSecret(companyId, { - definitionId, - definitionKey: envKey, - value: key, - }); - } - apiKeySecretRef.current = { key }; + const stored = await storeProviderApiKey(companyId, envKey, key); + apiKeySecretRef.current = { key, companyId, envKey, binding: stored.binding }; return true; } catch (err) { setError( @@ -1744,18 +1752,17 @@ function OnboardingWizardInner({ // present. If storing failed this stays false, and the right outcome is a // configuration with no credential — which the hire then blocks on — rather // than one that quietly falls back to embedding the value. - if (credentialMode === "api" && bindApiKey) { + if (credentialMode === "api" && (bindApiKey || selectedApiKey)) { const env = typeof config.env === "object" && config.env !== null && !Array.isArray(config.env) ? { ...(config.env as Record) } : {}; - env[apiKeyEnvKeyFor(adapterType)] = { - type: "user_secret_ref", - key: apiKeyEnvKeyFor(adapterType), - version: "latest", - }; + env[apiKeyEnvKeyFor(adapterType)] = selectedApiKey?.binding ?? apiKeySecretRef.current?.binding; config.env = env; } + if (credentialMode === "subscription" && savedSubscription) { + config.env = { ...((config.env as object) ?? {}), CODEX_HOME: savedSubscription.binding }; + } return config; } @@ -1958,7 +1965,7 @@ function OnboardingWizardInner({ // hire describe it the same way — as a reference. A failure here stops the // hire rather than falling through to a configuration with no credential. let apiKeyStored = false; - if (credentialMode === "api" && apiKey.trim()) { + if (credentialMode === "api" && !selectedApiKey && apiKey.trim()) { apiKeyStored = await storeApiKeyUserSecret(createdCompanyId); if (!apiKeyStored) return; } @@ -2514,6 +2521,20 @@ function OnboardingWizardInner({ }} /> + {credentialMode === "subscription" && adapterType === "codex_local" && savedKeys.subscriptions.length > 0 && ( +
+ setSubscriptionId(createdCompanyId ? { companyId: createdCompanyId, id } : null)} + loading={false} + error={false} + kind="subscription" + disabled={loading || adapterEnvLoading || connectPhase === "connecting"} + /> +
+ )} + {/* Fades on the first beat but keeps its space until the second, so pressing a tile moves nothing vertically. Once a sign-in is running there is no switching to keys @@ -2538,6 +2559,8 @@ function OnboardingWizardInner({ >
+ {savedKeys.options.length > 0 &&

{savedKeys.options.length} saved API {savedKeys.options.length === 1 ? "key available" : "keys available"}.

} + {credentialMode === "subscription" && authSignalStatus === "present" &&

An existing provider connection is available.

}
@@ -2580,11 +2603,15 @@ function OnboardingWizardInner({ */} {!connectCardMounted ? null : credentialMode === "api" ? ( - { + setSelectedSavedKey(createdCompanyId ? { companyId: createdCompanyId, envKey: apiKeyEnvKeyFor(adapterType), id } : null); + setApiKey(""); + }} /> + {!selectedApiKey && { + setSelectedSavedKey(createdCompanyId ? { companyId: createdCompanyId, envKey: apiKeyEnvKeyFor(adapterType), id: "" } : null); + setApiKey(value); + }} onSubmit={() => handleConnectStepPrimary()} - /> + />} ) : connectStepNeedsLogin && createdCompanyId && @@ -2640,6 +2670,8 @@ function OnboardingWizardInner({ }); }} /> + ) : adapterType === "claude_local" && savedKeys.storedLogin.data ? ( +

Use your saved Claude subscription for this agent.

) : connectStepHasNoSandbox ? ( /* The one thing that can be wrong here before anything is pressed, and the one worth saying out loud: without a diff --git a/ui/src/components/new-agent/AgentProviderConnection.test.tsx b/ui/src/components/new-agent/AgentProviderConnection.test.tsx new file mode 100644 index 0000000000..2116e0e6cc --- /dev/null +++ b/ui/src/components/new-agent/AgentProviderConnection.test.tsx @@ -0,0 +1,244 @@ +// @vitest-environment jsdom +import { flushSync } from "react-dom"; +import { createRoot, type Root } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { AgentProviderConnection } from "./AgentProviderConnection"; +import { ApiError } from "@/api/client"; +const mocks = vi.hoisted(() => ({ + auth: vi.fn(), + login: vi.fn(), + personal: vi.fn(), + organization: vi.fn(), +})); +vi.mock("@/api/agents", () => ({ + agentsApi: { + getAdapterAuthSignal: mocks.auth, + getClaudeOAuthTokenStatus: mocks.login, + }, +})); +vi.mock("@/api/secrets", () => ({ + secretsApi: { listMyUserSecrets: mocks.personal, list: mocks.organization }, +})); +vi.mock("../AgentConfigForm", () => ({ + AdapterLoginPanel: () =>
New subscription login
, +})); +let root: Root; +let host: HTMLDivElement; +let client: QueryClient; +afterEach(() => { + flushSync(() => root?.unmount()); + host?.remove(); + client?.clear(); + vi.resetAllMocks(); +}); +async function mount( + adapterType: "claude_local" | "codex_local" = "claude_local", + savedLogin = false, + canLogin = true, + codexSubscriptions = false, + savedApiKeys = true, + cachedClaudeLogin = false, +) { + const key = + adapterType === "claude_local" ? "ANTHROPIC_API_KEY" : "OPENAI_API_KEY"; + mocks.auth.mockResolvedValue({ + status: codexSubscriptions ? "unknown" : "present", + }); + mocks.login.mockImplementation(async () => { + if (savedLogin) return { secretId: "oauth", latestVersion: 1 }; + throw new ApiError("Not found", 404, null); + }); + mocks.personal.mockResolvedValue([ + { + definition: { + id: "d1", + companyId: "c1", + key: `${key}.setup.1`, + name: "Personal key", + status: "active", + }, + secret: { companyId: "c1", status: "active" }, + }, + ]); + mocks.organization.mockResolvedValue([ + { + id: "s1", + companyId: "c1", + key, + name: "Company key", + scope: "company", + status: "active", + }, + ...(codexSubscriptions + ? [ + { + id: "codex-home", + companyId: "c1", + name: "CODEX_HOME_team", + scope: "company", + status: "active", + }, + ] + : []), + ]); + if (!savedApiKeys) { + mocks.personal.mockResolvedValue([]); + mocks.organization.mockResolvedValue([]); + } + client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + if (cachedClaudeLogin) { + client.setQueryData(["claude-oauth-token-status", "c1"], { secretId: "cached-claude", latestVersion: 1 }); + mocks.auth.mockResolvedValue({ status: "absent" }); + } + host = document.createElement("div"); + document.body.appendChild(host); + root = createRoot(host); + const test = vi.fn().mockResolvedValue(true); + const connected = vi.fn(); + flushSync(() => + root.render( + + {}} + testConnection={test} + onConnected={connected} + /> + , + ), + ); + await vi.waitFor(() => expect(mocks.personal).toHaveBeenCalled()); + await vi.waitFor(() => expect(client.isFetching()).toBe(0)); + if (savedApiKeys) await vi.waitFor(() => expect(host.textContent).toContain("2 saved API keys")); + return { test, connected, key }; +} +function click(text: string) { + const button = [...host.querySelectorAll("button")].find((b) => + b.textContent?.includes(text), + )!; + expect(button).toBeTruthy(); + flushSync(() => button.click()); +} +function openProvider() { + flushSync(() => + (host.querySelector('[role="radio"]') as HTMLElement).click(), + ); +} +describe("AgentProviderConnection reuse", () => { + it("defaults to subscription when no saved credentials exist", async () => { + await mount("claude_local", false, true, false, false); + expect(host.textContent).toContain("Use API key instead"); + click("Use API key instead"); + openProvider(); + expect(host.querySelector('input[type="password"]')).not.toBeNull(); + }); + + it("does not use a cached Claude login for Codex", async () => { + await mount("codex_local", false, true, false, false, true); + openProvider(); + expect(host.textContent).toContain("New subscription login"); + expect(host.textContent).not.toContain("saved Claude subscription"); + expect(host.textContent).not.toContain("Use saved subscription"); + expect(mocks.login).not.toHaveBeenCalled(); + }); + it("reuses a saved ChatGPT account when the sandbox auth signal is unknown", async () => { + const { test, connected } = await mount("codex_local", false, true, true); + openProvider(); + expect(host.textContent).not.toContain("New subscription login"); + click("Use saved subscription"); + await vi.waitFor(() => + expect(connected).toHaveBeenCalledWith({ + env: { + CODEX_HOME: { + type: "secret_ref", + secretId: "codex-home", + version: "latest", + }, + }, + }), + ); + expect(test).toHaveBeenCalledWith(connected.mock.calls[0][0]); + flushSync(() => { + const select = host.querySelector("select")!; + select.value = ""; + select.dispatchEvent(new Event("change", { bubbles: true })); + }); + expect(host.textContent).toContain("New subscription login"); + }); + it.each(["claude_local", "codex_local"] as const)( + "passes a personal reference without credentials for %s", + async (adapter) => { + const { test, connected, key } = await mount(adapter); + openProvider(); + const select = host.querySelector("select")!; + expect(select.value).toBe("user:d1"); + click("Use saved API key"); + await vi.waitFor(() => + expect(connected).toHaveBeenCalledWith({ + env: { + [key]: { + type: "user_secret_ref", + key: `${key}.setup.1`, + version: "latest", + }, + }, + }), + ); + expect(test).toHaveBeenCalledWith(connected.mock.calls[0][0]); + }, + ); + it("uses an organization reference and requires a new key after switching away", async () => { + const { connected, key } = await mount(); + openProvider(); + const select = host.querySelector("select")!; + flushSync(() => { + select.value = "company:s1"; + select.dispatchEvent(new Event("change", { bubbles: true })); + }); + click("Use saved API key"); + await vi.waitFor(() => + expect(connected).toHaveBeenCalledWith({ + env: { + [key]: { type: "secret_ref", secretId: "s1", version: "latest" }, + }, + }), + ); + flushSync(() => { + select.value = ""; + select.dispatchEvent(new Event("change", { bubbles: true })); + }); + expect(host.querySelector('input[type="password"]')).not.toBeNull(); + const button = [...host.querySelectorAll("button")].find((b) => + b.textContent?.includes("Connect"), + )!; + expect(button.disabled).toBe(true); + await client.invalidateQueries(); + await vi.waitFor(() => expect(client.isFetching()).toBe(0)); + expect(host.querySelector("select")!.value).toBe(""); + expect(host.querySelector('input[type="password"]')).not.toBeNull(); + }); + it("reuses saved Claude login even without a login-capable environment", async () => { + const { test } = await mount("claude_local", true, false); + openProvider(); + await vi.waitFor(() => + expect(host.textContent).toContain("Use saved subscription"), + ); + click("Use saved subscription"); + await vi.waitFor(() => + expect(test).toHaveBeenCalledWith( + expect.objectContaining({ applyStoredClaudeLogin: true }), + ), + ); + }); + it("does not treat an environment credential as a stored Claude login", async () => { + const { test } = await mount(); + click("Use subscription instead"); + openProvider(); + click("Connect"); + await vi.waitFor(() => expect(test).toHaveBeenCalledWith({ env: {} })); + }); +}); diff --git a/ui/src/components/new-agent/AgentProviderConnection.tsx b/ui/src/components/new-agent/AgentProviderConnection.tsx index 43b9ae20f4..6faa67a3e7 100644 --- a/ui/src/components/new-agent/AgentProviderConnection.tsx +++ b/ui/src/components/new-agent/AgentProviderConnection.tsx @@ -1,6 +1,10 @@ import { useEffect, useRef, useState } from "react"; import { useQuery } from "@tanstack/react-query"; import { motion } from "motion/react"; +import { + SavedProviderKeySelect, + useSavedProviderKeys, +} from "../onboarding/SavedProviderKeySelect"; import { agentsApi } from "@/api/agents"; import { queryKeys } from "@/lib/queryKeys"; import { AdapterLoginPanel } from "../AgentConfigForm"; @@ -53,7 +57,7 @@ export function AgentProviderConnection({ setBusy(false); setOpened(false); }; - const [method, setMethod] = useState<"subscription" | "api">("subscription"); + const [methodChoice, setMethod] = useState<"subscription" | "api" | null>(null); const [opened, setOpened] = useState(false); const [apiKey, setApiKey] = useState(""); const [busy, setBusy] = useState(false); @@ -63,6 +67,24 @@ export function AgentProviderConnection({ const provider = adapterType === "claude_local" ? "Claude" : "OpenAI"; const envKey = adapterType === "claude_local" ? "ANTHROPIC_API_KEY" : "OPENAI_API_KEY"; + const savedKeys = useSavedProviderKeys(companyId, envKey); + const [subscriptionId, setSubscriptionId] = useState(null); + const savedSubscription = + adapterType === "codex_local" + ? savedKeys.subscriptions.find( + (option) => + option.id === (subscriptionId ?? savedKeys.subscriptions[0]?.id), + ) + : undefined; + const [selectedKeyId, setSelectedKeyId] = useState(null); + const selectedKey = savedKeys.options.find( + (option) => option.id === (selectedKeyId ?? savedKeys.options[0]?.id), + ); + const storedLogin = savedKeys.storedLogin; + const method = methodChoice ?? ( + (adapterType === "claude_local" ? storedLogin.data : savedKeys.subscriptions.length) + ? "subscription" : savedKeys.options.length ? "api" : "subscription" + ); const auth = useQuery({ queryKey: queryKeys.agents.authSignal( companyId, @@ -85,15 +107,17 @@ export function AgentProviderConnection({ try { const connection = method === "api" - ? (storedConnection ?? { - env: {}, - credentials: { [envKey]: apiKey.trim() }, - }) + ? selectedKey + ? { env: { [envKey]: selectedKey.binding } } + : (storedConnection ?? { + env: {}, + credentials: { [envKey]: apiKey.trim() }, + }) : { - env: {}, - ...(adapterType === "claude_local" && - canLogin && - auth.data?.status === "present" + env: savedSubscription + ? { CODEX_HOME: savedSubscription.binding } + : {}, + ...(adapterType === "claude_local" && storedLogin.data ? { env: buildFixedClaudeOAuthBinding(), applyStoredClaudeLogin: true, @@ -103,7 +127,7 @@ export function AgentProviderConnection({ if (run !== epoch.current) return; if (method === "api") { setApiKey(""); - setStoredConnection(connection); + if (!selectedKey) setStoredConnection(connection); } const connected = await testConnection(connection); if (run !== epoch.current) return; @@ -127,7 +151,10 @@ export function AgentProviderConnection({ method === "subscription" && canLogin && environmentId && - auth.data?.status !== "present"; + !savedSubscription && + !savedKeys.loading && + !storedLogin.data && + (auth.data?.status !== "present" || subscriptionId === ""); return (
)} + {!opened && savedKeys.options.length > 0 && ( +

+ {savedKeys.options.length} saved API{" "} + {savedKeys.options.length === 1 ? "key available" : "keys available"}. +

+ )} + {method === "subscription" && + adapterType === "codex_local" && + savedKeys.subscriptions.length > 0 && ( + + )} {method === "api" ? ( - { - setApiKey(value); - setStoredConnection(null); - }} - onSubmit={() => void connect()} + { + setSelectedKeyId(id); + setApiKey(""); + setStoredConnection(null); + setError(null); + }} /> + {!selectedKey && ( + { + setSelectedKeyId(""); + setApiKey(value); + setStoredConnection(null); + }} + onSubmit={() => void connect()} + disabled={busy} + /> + )} ) : needsLogin ? ( - ) : ( + ) : savedSubscription ? null : (

- {canLogin - ? "Use the subscription already connected to this environment." - : `Use the ${provider} login on this machine. If you haven’t signed in yet, run ${adapterType === "claude_local" ? "claude auth login" : "codex login"} in your terminal, then connect.`} + {storedLogin.data + ? "Use your saved Claude subscription for this agent." + : canLogin + ? "Use the existing provider connection for this environment." + : `Use the ${provider} login on this machine. If you haven’t signed in yet, run ${adapterType === "claude_local" ? "claude auth login" : "codex login"} in your terminal, then connect.`}

)} )} + {method === "subscription" && storedLogin.isError && ( +

+ Could not check your saved Claude subscription. Try again. +

+ )} {error && (

{testError ?? error} @@ -230,12 +301,26 @@ export function AgentProviderConnection({ if (opened) cancel(); else onBack(); }} - primaryLabel={busy ? "Connecting" : "Connect"} + primaryLabel={ + busy + ? "Connecting" + : method === "subscription" && + (storedLogin.data || savedSubscription) + ? "Use saved subscription" + : method === "api" && selectedKey + ? "Use saved API key" + : "Connect" + } primaryDisabled={ auth.isPending || + savedKeys.loading || + (adapterType === "claude_local" && storedLogin.isPending) || !opened || Boolean(needsLogin) || - (method === "api" && !apiKey.trim() && !storedConnection) + (method === "api" && + !apiKey.trim() && + !storedConnection && + !selectedKey) } loading={busy} onPrimary={() => void connect()} diff --git a/ui/src/components/onboarding/SavedProviderKeySelect.tsx b/ui/src/components/onboarding/SavedProviderKeySelect.tsx new file mode 100644 index 0000000000..4ee6fa265f --- /dev/null +++ b/ui/src/components/onboarding/SavedProviderKeySelect.tsx @@ -0,0 +1,122 @@ +import { useQuery } from "@tanstack/react-query"; +import { agentsApi } from "@/api/agents"; +import { ApiError } from "@/api/client"; +import { secretsApi } from "@/api/secrets"; +import { queryKeys } from "@/lib/queryKeys"; +import { + savedProviderKeys, + savedCodexSubscriptions, + type SavedProviderKey, +} from "@/lib/saved-provider-credentials"; + +export function useSavedProviderKeys( + companyId: string | null, + envKey: string, + enabled = true, +) { + const personal = useQuery({ + queryKey: queryKeys.secrets.myUserSecrets(companyId ?? ""), + queryFn: () => secretsApi.listMyUserSecrets(companyId!), + enabled: Boolean(companyId) && enabled, + retry: false, + }); + const organization = useQuery({ + queryKey: queryKeys.secrets.list(companyId ?? ""), + queryFn: () => secretsApi.list(companyId!), + enabled: Boolean(companyId) && enabled, + retry: false, + }); + const storedLogin = useQuery({ + // Disabled queries still return cached data. Keep other providers away + // from the shared Claude login cache. + queryKey: ["claude-oauth-token-status", envKey === "ANTHROPIC_API_KEY" ? companyId : null], + queryFn: async () => { + try { + return await agentsApi.getClaudeOAuthTokenStatus(companyId!); + } catch (error) { + if (error instanceof ApiError && error.status === 404) return null; + throw error; + } + }, + enabled: Boolean(companyId) && enabled && envKey === "ANTHROPIC_API_KEY", + retry: false, + }); + return { + storedLogin, + options: savedProviderKeys( + companyId ?? "", + envKey, + personal.data ?? [], + organization.data ?? [], + ), + subscriptions: savedCodexSubscriptions( + companyId ?? "", + organization.data ?? [], + ), + // Background refreshes must not unmount an active login panel sharing this query. + loading: personal.isLoading || organization.isLoading || storedLogin.isLoading, + error: personal.isError || organization.isError, + }; +} + +export function SavedProviderKeySelect({ + options, + value, + onChange, + loading, + error, + disabled, + kind = "api", +}: { + options: SavedProviderKey[]; + value: string; + onChange: (id: string) => void; + loading: boolean; + error: boolean; + disabled?: boolean; + kind?: "api" | "subscription"; +}) { + return ( +

+ {options.length > 0 && ( + + )} + {loading && ( +

+ Checking saved API keys… +

+ )} + {error && ( +

+ Some saved keys could not be loaded. You can still enter a new key. +

+ )} + {value && ( +

+ Reuse this saved {kind === "api" ? "key" : "subscription"} for this + agent. +

+ )} +
+ ); +} diff --git a/ui/src/lib/saved-provider-credentials.test.ts b/ui/src/lib/saved-provider-credentials.test.ts new file mode 100644 index 0000000000..fa1834a9d8 --- /dev/null +++ b/ui/src/lib/saved-provider-credentials.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from "vitest"; +import type { CompanySecret } from "@paperclipai/shared"; +import type { MyUserSecretEntry } from "../api/secrets"; +import { + savedProviderKeys, + savedCodexSubscriptions, +} from "./saved-provider-credentials"; +const secret = (overrides = {}) => + ({ + id: "s1", + companyId: "c1", + key: "ANTHROPIC_API_KEY", + name: "Claude", + scope: "company", + status: "active", + ...overrides, + }) as CompanySecret; +const personal = (key = "ANTHROPIC_API_KEY.setup.abc", overrides = {}) => + ({ + definition: { + id: "d1", + companyId: "c1", + key, + name: "My Claude", + status: "active", + ...overrides, + }, + secret: secret({ scope: "user" }), + }) as MyUserSecretEntry; +describe("saved provider keys", () => { + it("reuses canonical and setup keys with references, including normalized organization keys", () => { + expect( + savedProviderKeys( + "c1", + "ANTHROPIC_API_KEY", + [personal()], + [secret({ key: "anthropic_api_key" })], + ), + ).toEqual([ + { + id: "user:d1", + label: "My Claude (Your key)", + binding: { + type: "user_secret_ref", + key: "ANTHROPIC_API_KEY.setup.abc", + version: "latest", + }, + }, + { + id: "company:s1", + label: "Claude (Organization key)", + binding: { type: "secret_ref", secretId: "s1", version: "latest" }, + }, + ]); + }); + it("excludes unavailable, wrong-provider, and foreign-company credentials", () => { + expect( + savedProviderKeys( + "c1", + "ANTHROPIC_API_KEY", + [ + personal("OPENAI_API_KEY"), + personal(undefined, { status: "disabled" }), + { ...personal(), secret: null }, + { ...personal(), secret: secret({ status: "archived" }) }, + personal(undefined, { companyId: "c2" }), + ], + [ + secret({ companyId: "c2" }), + secret({ status: "disabled" }), + secret({ scope: "user" }), + secret({ key: "ANTHROPIC_API_KEY_OTHER" }), + ], + ), + ).toEqual([]); + }); +}); + +it("lists only active company Codex account connections", () => { + const account = secret({ name: "CODEX_HOME_team" }); + expect( + savedCodexSubscriptions("c1", [ + account, + { ...account, status: "disabled" }, + { ...account, companyId: "c2" }, + secret(), + ]), + ).toEqual([ + { + id: "company:s1", + label: "ChatGPT account · team", + binding: { type: "secret_ref", secretId: "s1", version: "latest" }, + }, + ]); +}); diff --git a/ui/src/lib/saved-provider-credentials.ts b/ui/src/lib/saved-provider-credentials.ts new file mode 100644 index 0000000000..35abf6ee55 --- /dev/null +++ b/ui/src/lib/saved-provider-credentials.ts @@ -0,0 +1,79 @@ +import type { CompanySecret, EnvBinding } from "@paperclipai/shared"; +import type { MyUserSecretEntry } from "../api/secrets"; + +export interface SavedProviderKey { + id: string; + label: string; + binding: EnvBinding; +} + +/** Match the canonical onboarding key and distinct keys created by agent setup. */ +export function savedProviderKeys( + companyId: string, + envKey: string, + personal: MyUserSecretEntry[], + organization: CompanySecret[], +): SavedProviderKey[] { + const matches = (key: string | null) => + key?.toUpperCase() === envKey || + key?.toUpperCase().startsWith(`${envKey}.SETUP.`); + return [ + ...personal.flatMap(({ definition, secret }) => + definition.companyId === companyId && + definition.status === "active" && + secret?.companyId === companyId && + secret.status === "active" && + matches(definition.key) + ? [ + { + id: `user:${definition.id}`, + label: `${definition.name} (Your key)`, + binding: { + type: "user_secret_ref" as const, + key: definition.key, + version: "latest" as const, + }, + }, + ] + : [], + ), + ...organization.flatMap((secret) => + secret.companyId === companyId && + secret.scope === "company" && + secret.status === "active" && + matches(secret.key) + ? [ + { + id: `company:${secret.id}`, + label: `${secret.name} (Organization key)`, + binding: { + type: "secret_ref" as const, + secretId: secret.id, + version: "latest" as const, + }, + }, + ] + : [], + ), + ]; +} + +/** Codex device login creates one reusable company secret per account home. */ +export function savedCodexSubscriptions( + companyId: string, + organization: CompanySecret[], +): SavedProviderKey[] { + return organization + .filter( + (secret) => + secret.companyId === companyId && + secret.scope === "company" && + secret.status === "active" && + secret.name.startsWith("CODEX_HOME_"), + ) + .map((secret) => ({ + id: `company:${secret.id}`, + label: secret.name.replace("CODEX_HOME_", "ChatGPT account · "), + binding: { type: "secret_ref", secretId: secret.id, version: "latest" }, + })); +} diff --git a/ui/src/pages/DesignGuide.tsx b/ui/src/pages/DesignGuide.tsx index cf4f63a54d..544287f4f7 100644 --- a/ui/src/pages/DesignGuide.tsx +++ b/ui/src/pages/DesignGuide.tsx @@ -1,3 +1,4 @@ +import { SavedProviderKeySelect } from "../components/onboarding/SavedProviderKeySelect"; import { RepositoryEditor } from "@/components/RepositoryEditor"; import { TaskChatMarker } from "@/components/task-chat/TaskChatMarker"; import { TaskChatComposer } from "@/components/task-chat/TaskChatComposer"; @@ -2179,6 +2180,12 @@ export function DesignGuide() {

+
+ {}} loading={false} error={false} /> + {}} loading error={false} /> + {}} loading={false} error /> +
+

The task card is the dialog host for the shared connection setup flow. Provider forms, diff --git a/ui/src/pages/NewAgent.test.tsx b/ui/src/pages/NewAgent.test.tsx index 0be59dea4b..8f1de89d01 100644 --- a/ui/src/pages/NewAgent.test.tsx +++ b/ui/src/pages/NewAgent.test.tsx @@ -6,6 +6,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { TooltipProvider } from "@/components/ui/tooltip"; import { queryKeys } from "@/lib/queryKeys"; import { NewAgent } from "./NewAgent"; +import { ApiError } from "@/api/client"; const api = vi.hoisted(() => ({ get: vi.fn(), @@ -14,6 +15,7 @@ const api = vi.hoisted(() => ({ hire: vi.fn(), testEnvironment: vi.fn(), getAdapterAuthSignal: vi.fn(), + getClaudeOAuthTokenStatus: vi.fn(), })); const envApi = vi.hoisted(() => ({ list: vi.fn(), capabilities: vi.fn() })); const settings = vi.hoisted(() => ({ @@ -165,6 +167,7 @@ beforeEach(() => { api.adapterModels.mockResolvedValue([]); api.list.mockResolvedValue([{ id: "ceo", role: "ceo", status: "idle" }]); api.getAdapterAuthSignal.mockResolvedValue({ status: "present" }); + api.getClaudeOAuthTokenStatus.mockRejectedValue(new ApiError("Not found", 404, null)); api.testEnvironment.mockResolvedValue(pass); api.hire.mockImplementation(async (_company, input) => ({ agent: { ...input, id: "new-agent", status: "idle", urlKey: "atlas" }, @@ -383,6 +386,29 @@ describe("New agent setup", () => { expect(api.hire.mock.calls[0][1].adapterConfig.env[key].type).toBe("user_secret_ref"); expect(JSON.stringify(api.hire.mock.calls)).not.toContain("connection-key"); }); + it.each([ + ["claude_local", "claude", "Claude", "ANTHROPIC_API_KEY"], + ["codex_local", "codex", "OpenAI", "OPENAI_API_KEY"], + ["paperclip_runner", "claude", "Claude", "ANTHROPIC_API_KEY"], + ["paperclip_runner", "codex", "OpenAI", "OPENAI_API_KEY"], + ])("defaults %s %s to a saved key and preserves its reference through hire", async (adapter, runner, provider, key) => { + secrets.listMyUserSecrets.mockResolvedValue([{ + definition: { id: "existing-key", companyId: "company-1", key, name: "Existing key", status: "active" }, + secret: { companyId: "company-1", status: "active" }, + }]); + await render(adapter, runner); + await click(provider + "API"); + expect((container.querySelector("select[aria-label='Saved API key']") as HTMLSelectElement).value).toBe("user:existing-key"); + await click("Use saved API key"); + const binding = { type: "user_secret_ref", key, version: "latest" }; + expect(api.testEnvironment.mock.calls[0][2].adapterConfig.env[key]).toEqual(binding); + expect(api.testEnvironment.mock.calls[0][2].testCredentials).toEqual({}); + await click("Finish setup"); + expect(api.hire.mock.calls[0][1].adapterConfig.env[key]).toEqual(binding); + expect(secrets.createUserSecretDefinition).not.toHaveBeenCalled(); + expect(secrets.createMyUserSecret).not.toHaveBeenCalled(); + expect(secrets.rotateMyUserSecret).not.toHaveBeenCalled(); + }); it.each(["opencode_local", "pi_local"])( "persists %s OpenRouter credentials only as a secret reference", async (adapter) => { @@ -514,8 +540,10 @@ describe("New agent setup", () => { settings.getExperimental.mockResolvedValue({ enableManagedSandboxOnly: true, }); + api.getClaudeOAuthTokenStatus.mockResolvedValue({ secretId: "saved-oauth", latestVersion: 1 }); await render("claude_local"); - await connect("Claude"); + await click("ClaudeSubscription"); + await click("Use saved subscription"); await click("Finish setup"); expect(api.testEnvironment.mock.calls[0][2].environmentId).toBe( "sandbox-1", diff --git a/ui/storybook/.storybook/preview.tsx b/ui/storybook/.storybook/preview.tsx index 5e5fe31bbf..51592afd83 100644 --- a/ui/storybook/.storybook/preview.tsx +++ b/ui/storybook/.storybook/preview.tsx @@ -11,6 +11,7 @@ import { ONBOARDING_STORAGE_KEY } from "@/components/OnboardingWizard"; import { STORYBOOK_COMPANY_ID } from "../fixtures/onboardingDraft"; import { STORYBOOK_SANDBOX_ENVIRONMENT_ID, + onboardingFixtureState, storybookAuthSignal, storybookEnvironmentCapabilities, storybookEnvironmentTest, @@ -204,6 +205,7 @@ function installStorybookApiFixtures() { /^\/api\/companies\/[^/]+\/adapters\/([^/]+)\/login-sessions(?:\/([^/]+))?(\/cancel)?$/, ); if (adapterLoginMatch) { + if (adapterLoginMatch[2] === "active") return new Response(null, { status: 404 }); const session = { sessionId: "adapter-login-storybook", environmentId: STORYBOOK_SANDBOX_ENVIRONMENT_ID, @@ -249,6 +251,9 @@ function installStorybookApiFixtures() { failure: null, }); } + if (/^\/api\/companies\/[^/]+\/setup-token-login-sessions\/active$/.test(url.pathname)) { + return new Response(null, { status: 404 }); + } if ( /^\/api\/companies\/[^/]+\/setup-token-login-sessions\/[^/]+$/.test( url.pathname, @@ -298,7 +303,15 @@ function installStorybookApiFixtures() { if ( /^\/api\/companies\/[^/]+\/claude-oauth-token-status$/.test(url.pathname) ) { - return new Response(null, { status: 404 }); + return onboardingFixtureState.savedClaudeLogin + ? Response.json({ secretId: "saved-claude-subscription", latestVersion: 1 }) + : new Response(null, { status: 404 }); + } + if (/^\/api\/companies\/[^/]+\/me\/user-secrets$/.test(url.pathname)) { + return Response.json(onboardingFixtureState.savedApiKeys ? ["ANTHROPIC_API_KEY", "OPENAI_API_KEY"].map((key) => ({ + definition: { id: key, companyId: "company-storybook", key: `${key}.setup.storybook`, name: key === "ANTHROPIC_API_KEY" ? "My Claude key" : "My OpenAI key", status: "active" }, + secret: { id: `secret-${key}`, companyId: "company-storybook", scope: "user", status: "active" }, + })) : []); } // The hire, and the three calls either side of it. @@ -315,7 +328,12 @@ function installStorybookApiFixtures() { /^\/api\/companies\/[^/]+\/adapters\/([^/]+)\/test-environment$/, ); if (testEnvMatch) { - return Response.json(storybookEnvironmentTest(testEnvMatch[1])); + const body = typeof init?.body === "string" ? JSON.parse(init.body) : {}; + const env = body.adapterConfig?.env ?? {}; + const usesSavedCodex = onboardingFixtureState.savedCodexLogin && env.CODEX_HOME?.secretId === "saved-codex-home"; + const usesSavedKey = onboardingFixtureState.savedApiKeys && ["ANTHROPIC_API_KEY", "OPENAI_API_KEY"].some((key) => env[key]?.type === "user_secret_ref" && env[key]?.key === `${key}.setup.storybook`); + const usesSavedClaude = onboardingFixtureState.savedClaudeLogin && env.CLAUDE_CODE_OAUTH_TOKEN?.type === "user_secret_ref" && env.CLAUDE_CODE_OAUTH_TOKEN?.key === "CLAUDE_CODE_OAUTH_TOKEN"; + return Response.json(usesSavedCodex || usesSavedKey || usesSavedClaude ? { adapterType: testEnvMatch[1], status: "pass", checks: [], testedAt: new Date(0).toISOString() } : storybookEnvironmentTest(testEnvMatch[1])); } if (/^\/api\/companies\/[^/]+\/agent-hires$/.test(url.pathname)) { // `approval: null` on purpose. A hire that returns one sends the wizard @@ -501,7 +519,7 @@ function installStorybookApiFixtures() { if (secretsListMatch) { const [, companyId] = secretsListMatch; return Response.json( - companyId === "company-storybook" ? storybookSecrets : [], + companyId === "company-storybook" ? [...storybookSecrets, ...(onboardingFixtureState.savedCodexLogin ? [{ ...storybookSecrets[0], id: "saved-codex-home", key: "codex_home_saved", name: "CODEX_HOME_team-account" }] : [])] : [], ); } diff --git a/ui/storybook/fixtures/onboardingEnvironment.ts b/ui/storybook/fixtures/onboardingEnvironment.ts index a7240d0ec9..daea0747d4 100644 --- a/ui/storybook/fixtures/onboardingEnvironment.ts +++ b/ui/storybook/fixtures/onboardingEnvironment.ts @@ -29,6 +29,9 @@ export const STORYBOOK_SANDBOX_ENVIRONMENT_ID = "environment-storybook-sandbox"; interface FixtureState { environments: OnboardingEnvironmentState; authSignal: AdapterAuthSignal; + savedApiKeys: boolean; + savedClaudeLogin: boolean; + savedCodexLogin: boolean; } /** @@ -38,6 +41,9 @@ interface FixtureState { export const onboardingFixtureState: FixtureState = { environments: "managed-sandbox", authSignal: "absent", + savedApiKeys: false, + savedClaudeLogin: false, + savedCodexLogin: false, }; export function setOnboardingFixtureState(next: Partial): void { @@ -47,6 +53,9 @@ export function setOnboardingFixtureState(next: Partial): void { export function resetOnboardingFixtureState(): void { onboardingFixtureState.environments = "managed-sandbox"; onboardingFixtureState.authSignal = "absent"; + onboardingFixtureState.savedApiKeys = false; + onboardingFixtureState.savedClaudeLogin = false; + onboardingFixtureState.savedCodexLogin = false; } /** diff --git a/ui/storybook/stories/agent-connection-reuse.stories.tsx b/ui/storybook/stories/agent-connection-reuse.stories.tsx new file mode 100644 index 0000000000..d3f98a949d --- /dev/null +++ b/ui/storybook/stories/agent-connection-reuse.stories.tsx @@ -0,0 +1,140 @@ +import { useState } from "react"; +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { expect, userEvent, within } from "storybook/test"; +import { + AgentProviderConnection, + type ProviderConnection, +} from "@/components/new-agent/AgentProviderConnection"; +import { + resetOnboardingFixtureState, + setOnboardingFixtureState, + STORYBOOK_SANDBOX_ENVIRONMENT_ID, +} from "../fixtures/onboardingEnvironment"; + +function ConnectionStory({ + adapterType = "claude_local", +}: { + adapterType?: "claude_local" | "codex_local"; +}) { + const [connection, setConnection] = useState(null); + return ( +

+

Connect a model

+ {connection ? ( +
+ Connection selected. Continue to agent configuration. +
+ ) : ( + {}} + onConnected={setConnection} + testConnection={async (value) => { + if ( + Object.values(value.env).some( + (binding) => + typeof binding === "string" || binding.type === "plain", + ) + ) + throw new Error("Expected a stored reference"); + return true; + }} + /> + )} +
+ ); +} +const meta = { + title: "Onboarding/Saved connections", + component: ConnectionStory, + parameters: { + layout: "centered", + docs: { + description: { + component: + "Production agent connection component with fixture API responses. Provider tests and subsequent hiring are simulated. Choose subscription or API key, reuse a saved connection, or enter a new key.", + }, + }, + }, + beforeEach: () => { + resetOnboardingFixtureState(); + return resetOnboardingFixtureState; + }, +} satisfies Meta; +export default meta; +type Story = StoryObj; +export const ClaudeSubscription: Story = { + beforeEach: () => { + setOnboardingFixtureState({ + savedClaudeLogin: true, + authSignal: "present", + }); + return resetOnboardingFixtureState; + }, + play: async ({ canvasElement }) => { + await userEvent.click(await within(canvasElement).findByRole("radio")); + }, +}; +export const NewClaudeSubscription: Story = { + play: async ({ canvasElement }) => { + await userEvent.click(await within(canvasElement).findByRole("radio")); + }, +}; +export const NewCodexSubscription: Story = { + args: { adapterType: "codex_local" }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.click(await canvas.findByRole("button", { name: "Use subscription instead" })); + await userEvent.click(canvas.getByRole("radio")); + }, +}; +export const ClaudeApiKeys: Story = { + beforeEach: () => { + setOnboardingFixtureState({ savedApiKeys: true }); + return resetOnboardingFixtureState; + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("radio")); + await expect( + await canvas.findByRole("combobox", { name: "Saved API key" }), + ).not.toHaveValue(""); + }, +}; +export const CodexApiKeys: Story = { + ...ClaudeApiKeys, + args: { adapterType: "codex_local" }, +}; +export const ReuseClaudeApiKey: Story = { + ...ClaudeApiKeys, + play: async (context) => { + await ClaudeApiKeys.play!(context); + const canvas = within(context.canvasElement); + await userEvent.selectOptions( + canvas.getByRole("combobox"), + "user:ANTHROPIC_API_KEY", + ); + await userEvent.click( + canvas.getByRole("button", { name: "Use saved API key" }), + ); + await expect(await canvas.findByRole("status")).toHaveTextContent( + "Connection selected", + ); + }, +}; + +export const CodexSubscription: Story = { + args: { adapterType: "codex_local" }, + beforeEach: () => { + setOnboardingFixtureState({ savedCodexLogin: true, authSignal: "unknown" }); + return resetOnboardingFixtureState; + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await canvas.findByRole("combobox", { name: "Saved subscription" }); + await userEvent.click(canvas.getByRole("radio")); + }, +}; diff --git a/ui/storybook/stories/onboarding-agent-arc.stories.tsx b/ui/storybook/stories/onboarding-agent-arc.stories.tsx index 4c6275c80c..8e05032562 100644 --- a/ui/storybook/stories/onboarding-agent-arc.stories.tsx +++ b/ui/storybook/stories/onboarding-agent-arc.stories.tsx @@ -348,8 +348,9 @@ function signedOutConnectionFixture() { async function openProviderConnection(provider: "Claude" | "OpenAI", mode: "subscription" | "api") { await advance("Connect a model"); - if (mode === "api") { - await userEvent.click(await screen.findByRole("button", { name: "Use API key instead" }, { timeout: STEP_TIMEOUT_MS })); + await userEvent.click(await screen.findByRole("button", { name: "Use API key instead" }, { timeout: STEP_TIMEOUT_MS })); + if (mode === "subscription") { + await userEvent.click(await screen.findByRole("button", { name: "Use subscription instead" }, { timeout: STEP_TIMEOUT_MS })); } await userEvent.click(await screen.findByRole("radio", { name: new RegExp(`^${provider} `) }, { timeout: STEP_TIMEOUT_MS })); if (mode === "api") { @@ -385,3 +386,44 @@ export const CodexApiKey: StoryObj = { render: () => , play: () => openProviderConnection("OpenAI", "api"), }; + +/** Production onboarding, with the current user's previously saved provider key. */ +export const ConnectWithSavedApiKey: StoryObj = { + beforeEach: () => { + setOnboardingFixtureState({ savedApiKeys: true, authSignal: "absent" }); + return resetOnboardingFixtureState; + }, + render: () => , + play: async () => { + await advance("Connect a model"); + await pickFirstSource(); + await screen.findByRole("combobox", { name: "Saved API key" }, { timeout: STEP_TIMEOUT_MS }); + await expect(screen.getByRole("combobox", { name: "Saved API key" })).toHaveValue("user:ANTHROPIC_API_KEY"); + }, +}; + +export const ConnectWithSavedChatGptSubscription: StoryObj = { + beforeEach: () => { + setOnboardingFixtureState({ savedCodexLogin: true, authSignal: "unknown" }); + return resetOnboardingFixtureState; + }, + render: () => , + play: async () => { + await advance("Connect a model"); + await userEvent.click(screen.getByRole("radio", {name: /OpenAI/})); + await screen.findByRole("combobox", {name: "Saved subscription"}, {timeout: STEP_TIMEOUT_MS}); + }, +}; + +export const ConnectWithSavedClaudeSubscription: StoryObj = { + beforeEach: () => { + setOnboardingFixtureState({ savedClaudeLogin: true, savedApiKeys: true, authSignal: "absent" }); + return resetOnboardingFixtureState; + }, + render: () => , + play: async () => { + await advance("Connect a model"); + await pickFirstSource(); + await expect(screen.queryByRole("combobox", { name: "Saved API key" })).not.toBeInTheDocument(); + }, +};