From 8621bcf53ee2e4bbefb7d193add42dd544ca06e8 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Fri, 6 Nov 2026 14:31:11 -0800 Subject: [PATCH] Cancel subscription sign-in requests when leaving agent setup (#19610) Back during pending subscription sign-in could accept a late response and reopen agent setup. Abort pending reads, release abandoned sessions, and ignore stale responses. Wait for cleanup before starting a replacement, and preserve sessions belonging to another account or environment. Cover deferred resume/start/prompt responses for both providers, local login cancellation, quick reselection, and browser request abort. Stabilize Runner fixtures and normalization assertions exposed by integration CI. Verified: 483 focused UI tests, six onboarding browser cases, UI typecheck and token gates, Runner fixture checks, and the complete CI run on 221f2e3eb7b116a0a84c9706d79a20a3b53b6b84. Greptile scored 5/5; all review threads are resolved. Earlier failures and recovery evidence remain in the PR description. Co-Authored-By: Paperclip --- doc/connections/AI-CONNECTIONS.md | 8 + .../agents-paperclip-runner-skills.test.ts | 5 +- tests/e2e/onboarding.spec.ts | 46 ++++- tests/runner-e2e/report.test.ts | 1 + ui/src/api/agents.ts | 18 +- .../AgentConfigForm.render.test.tsx | 2 + ui/src/components/AgentConfigForm.tsx | 83 ++++---- ui/src/components/OnboardingWizard.test.tsx | 179 ++++++++++++++++-- ui/src/components/OnboardingWizard.tsx | 56 +++--- .../ai-connections/useLocalAiLogin.test.tsx | 20 +- .../ai-connections/useLocalAiLogin.ts | 16 +- .../components/useAdapterLoginCancellation.ts | 94 +++++++++ 12 files changed, 439 insertions(+), 89 deletions(-) create mode 100644 ui/src/components/useAdapterLoginCancellation.ts diff --git a/doc/connections/AI-CONNECTIONS.md b/doc/connections/AI-CONNECTIONS.md index e758bbf89c5..0372f939330 100644 --- a/doc/connections/AI-CONNECTIONS.md +++ b/doc/connections/AI-CONNECTIONS.md @@ -41,6 +41,14 @@ the provider's API key, and Advanced. All three stay visible while the selected mode's form is shown below. Advanced opens the existing compatible-connection picker and provider setup. Execution environment selection lives in Configure. +Back from subscription sign-in cancels that attempt and pending prompt or status +reads. A session returned after Back is released instead of reopening sign-in. If an +aborted read hides the session ID, a separate owner lookup releases the attempt +before a replacement can start. Cleanup checks the abandoned environment and +account intent, so a conflicting settings sign-in stays active. +Selecting a subscription again starts a fresh attempt. Reloading the page can +still resume a sign-in that was not cancelled. + Storybook's simulated controllers and page annotations do not run in the app. ## Compatibility and selection diff --git a/server/src/__tests__/agents-paperclip-runner-skills.test.ts b/server/src/__tests__/agents-paperclip-runner-skills.test.ts index eb7bb633ac2..1e5b291ff83 100644 --- a/server/src/__tests__/agents-paperclip-runner-skills.test.ts +++ b/server/src/__tests__/agents-paperclip-runner-skills.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "vitest"; +import { PAPERCLIP_RUNNER_DEFAULT_MODELS } from "@paperclipai/adapter-utils/paperclip-runner-permissions"; import { normalizePaperclipOperationalSkillPreference, normalizePaperclipRunnerAdapterConfig, @@ -16,7 +17,7 @@ describe("paperclip_runner operational skill normalization", () => { it("applies full-auto native runner defaults at persistence boundaries", () => { expect(normalizePaperclipRunnerAdapterConfig("paperclip_runner", {})).toEqual({ provider: "codex", - model: "gpt-5.6-sol", + model: PAPERCLIP_RUNNER_DEFAULT_MODELS.codex, codexPermissionMode: "never", lifecycleMode: "per_turn", }); @@ -24,7 +25,7 @@ describe("paperclip_runner operational skill normalization", () => { it("repairs an existing blank model without replacing an explicit model", () => { expect(normalizePaperclipRunnerAdapterConfig("paperclip_runner", { model: "" })) - .toMatchObject({ model: "gpt-5.6-sol" }); + .toMatchObject({ model: PAPERCLIP_RUNNER_DEFAULT_MODELS.codex }); expect(normalizePaperclipRunnerAdapterConfig("paperclip_runner", { model: "gpt-5.5" })) .toMatchObject({ model: "gpt-5.5" }); }); diff --git a/tests/e2e/onboarding.spec.ts b/tests/e2e/onboarding.spec.ts index bf0f86d916b..08432d16cf2 100644 --- a/tests/e2e/onboarding.spec.ts +++ b/tests/e2e/onboarding.spec.ts @@ -234,11 +234,12 @@ test.describe("Onboarding wizard", () => { // The sign-in Connect now starts. Stubbed so the card is deterministic: // the session start answers, and the guarded prompt read hands back an // authorization URL for the card's link row. + let loginStarts = 0; await page.route("**/setup-token-login-sessions", (route) => route.fulfill({ contentType: "application/json", body: JSON.stringify({ - sessionId: "e2e-setup-token-session", + sessionId: `e2e-setup-token-session-${++loginStarts}`, status: "pending", expiresAt: new Date(Date.now() + 600_000).toISOString(), }), @@ -253,11 +254,11 @@ test.describe("Onboarding wizard", () => { }), }), ); - await page.route("**/setup-token-login-sessions/e2e-setup-token-session", (route) => + await page.route(/\/setup-token-login-sessions\/e2e-setup-token-session-\d+$/, (route) => route.fulfill({ contentType: "application/json", body: JSON.stringify({ - sessionId: "e2e-setup-token-session", + sessionId: route.request().url().split("/").at(-1), status: "pending", expiresAt: new Date(Date.now() + 600_000).toISOString(), }), @@ -343,6 +344,45 @@ test.describe("Onboarding wizard", () => { await expect(page.getByRole("button", { name: "Use saved login" })).toHaveCount(0); expect(hireCalled).toBe(false); + let cancellations = 0; + await page.route("**/setup-token-login-sessions/*/cancel", async route => { + cancellations++; + await route.fulfill({ status: 204 }); + }); + const back = page.getByRole("button", { name: "Back", exact: true }); + await back.click(); + await expect(source).toHaveAttribute("aria-checked", "false"); + await expect.poll(() => cancellations).toBe(1); + await page.evaluate(() => { + const state = window as typeof window & { cancelledLoginPrompts: number }; + state.cancelledLoginPrompts = 0; + const fetch = window.fetch.bind(window); + window.fetch = (input, init) => { + const url = input instanceof Request ? input.url : String(input); + if (url.endsWith("/prompt")) init?.signal?.addEventListener("abort", () => { state.cancelledLoginPrompts++; }, { once: true }); + return fetch(input, init); + }; + }); + let finishPrompt!: () => void; + let promptStarted!: () => void; + const pendingPrompt = new Promise(resolve => { finishPrompt = resolve; }); + const promptRequest = new Promise(resolve => { promptStarted = resolve; }); + await page.route("**/setup-token-login-sessions/*/prompt", async route => { + promptStarted(); + await pendingPrompt; + await route.fulfill({ json: { authorizationUrl: "https://claude.ai/oauth/authorize?late=true", transportAdvisory: null } }).catch(() => {}); + }); + await source.click(); + await promptRequest; + await back.click(); + await expect.poll(() => page.evaluate(() => (window as typeof window & { cancelledLoginPrompts: number }).cancelledLoginPrompts)).toBe(1); + finishPrompt(); + await expect(source).toHaveAttribute("aria-checked", "false"); + await expect.poll(() => cancellations).toBe(2); + await expect(cardInstruction).toHaveCount(0); + await expect(page.getByRole("button", { name: "Next", exact: true })).toBeDisabled(); + expect(hireCalled).toBe(false); + expect(pageErrors, pageErrors.join("\n")).toHaveLength(0); }); test("a reload during a login resumes the same session", async ({ page }) => { diff --git a/tests/runner-e2e/report.test.ts b/tests/runner-e2e/report.test.ts index 97dc40f9d1f..c65610db9d3 100644 --- a/tests/runner-e2e/report.test.ts +++ b/tests/runner-e2e/report.test.ts @@ -391,6 +391,7 @@ describe("runner E2E report aggregation", () => { GITHUB_SERVER_URL: "https://github.com", GITHUB_REPOSITORY: "paperclipai/paperclip", GITHUB_RUN_ID: "123456", + GITHUB_RUN_ATTEMPT: "1", PAPERCLIP_RUNNER_E2E_HISTORY_PUBLIC_BASE_URL: "https://reports.example.test/", PAPERCLIP_RUNNER_E2E_HISTORY_PREFIX: "/runner-e2e/", diff --git a/ui/src/api/agents.ts b/ui/src/api/agents.ts index 0c729e0dbfb..f192d65bc4c 100644 --- a/ui/src/api/agents.ts +++ b/ui/src/api/agents.ts @@ -338,18 +338,18 @@ export const agentsApi = { // The owner response repeats the live prompt on every read while the // session holds an active public status, so this polling call carries the // same no-store request option as the active-session read below. - getAdapterAuthLoginStatus: (companyId: string, type: string, sessionId: string) => + getAdapterAuthLoginStatus: (companyId: string, type: string, sessionId: string, options?: { signal?: AbortSignal }) => api.get( `/companies/${encodeURIComponent(companyId)}/adapters/${encodeURIComponent(type)}/login-sessions/${encodeURIComponent(sessionId)}`, - { cache: "no-store" }, + { cache: "no-store", ...options }, ), // Reads the caller's active login session for one company and adapter, with // no session id, so the browser rediscovers its own session after a reload // with no local state. A 404 means no active session for the caller. - getActiveAdapterAuthLoginSession: (companyId: string, type: string) => + getActiveAdapterAuthLoginSession: (companyId: string, type: string, options?: { signal?: AbortSignal; fresh?: boolean }) => api.get( `/companies/${encodeURIComponent(companyId)}/adapters/${encodeURIComponent(type)}/login-sessions/active`, - { cache: "no-store" }, + { cache: "no-store", ...options }, ), cancelAdapterAuthLogin: (companyId: string, type: string, sessionId: string) => api.post( @@ -378,21 +378,23 @@ export const agentsApi = { `/companies/${encodeURIComponent(companyId)}/setup-token-login-sessions`, { adapterType: "claude_local", ...data }, ), - getClaudeSetupTokenLoginStatus: (companyId: string, sessionId: string) => + getClaudeSetupTokenLoginStatus: (companyId: string, sessionId: string, options?: { signal?: AbortSignal }) => api.get( `/companies/${encodeURIComponent(companyId)}/setup-token-login-sessions/${encodeURIComponent(sessionId)}`, + options, ), // Reads the caller's active Claude setup-token login session, with no // session id, so the browser rediscovers its own session after a reload with // no local state. A 404 means no active session for the caller. - getActiveClaudeSetupTokenLoginSession: (companyId: string) => + getActiveClaudeSetupTokenLoginSession: (companyId: string, options?: { signal?: AbortSignal; fresh?: boolean }) => api.get( `/companies/${encodeURIComponent(companyId)}/setup-token-login-sessions/active`, - { cache: "no-store" }, + { cache: "no-store", ...options }, ), - getClaudeSetupTokenLoginPrompt: (companyId: string, sessionId: string) => + getClaudeSetupTokenLoginPrompt: (companyId: string, sessionId: string, options?: { signal?: AbortSignal }) => api.get( `/companies/${encodeURIComponent(companyId)}/setup-token-login-sessions/${encodeURIComponent(sessionId)}/prompt`, + options, ), submitClaudeSetupTokenBrowserCode: (companyId: string, sessionId: string, browserCode: string) => api.post( diff --git a/ui/src/components/AgentConfigForm.render.test.tsx b/ui/src/components/AgentConfigForm.render.test.tsx index 08e11823def..9dd03313a6a 100644 --- a/ui/src/components/AgentConfigForm.render.test.tsx +++ b/ui/src/components/AgentConfigForm.render.test.tsx @@ -2998,6 +2998,7 @@ describe("AgentConfigForm environment selector", () => { "company-1", "codex_local", "resumed-session-1", + expect.objectContaining({ signal: expect.any(AbortSignal) }), ); expect(findButton(result.container, "Cancel")).toBeTruthy(); }); @@ -3769,6 +3770,7 @@ describe("AgentConfigForm environment selector", () => { expect(mockAgentsApi.getClaudeSetupTokenLoginStatus).toHaveBeenCalledWith( "company-1", "resumed-claude-session-1", + expect.objectContaining({ signal: expect.any(AbortSignal) }), ); expect(findButton(result.container, "Cancel")).toBeTruthy(); }); diff --git a/ui/src/components/AgentConfigForm.tsx b/ui/src/components/AgentConfigForm.tsx index 5f8eb70185c..97dd7c52241 100644 --- a/ui/src/components/AgentConfigForm.tsx +++ b/ui/src/components/AgentConfigForm.tsx @@ -1,3 +1,4 @@ +import { useAdapterLoginCancellation } from "./useAdapterLoginCancellation"; import { ACCOUNT_BOUND_MODEL_DISCOVERY_QUERY_OPTIONS, isAccountBoundAcpxAgent } from "@/lib/native-model-catalog"; import { useConnectionModels } from "./ai-connections/useConnectionModels"; import { agentHarnessType, agentRunner, paperclipRunnerProfileForHarness, normalizeClaudeRunnerOptions, type AgentRunnerChoice } from "@paperclipai/shared"; @@ -2519,6 +2520,7 @@ export type AdapterLoginDescriptor = { // correctly, and the first thing to rot would have been the timeout and // cleanup paths, which are the ones nobody exercises by hand. export type AdapterLoginPanelProps = AdapterLoginDescriptor & { + active?: boolean; aiConnection?: import("@paperclipai/shared").AiConnectionLoginIntent; onStored?: (storedSessionId: string) => void; onApplyStored?: () => void; @@ -2597,6 +2599,7 @@ class AdapterLoginConflictError extends Error { } function DisplayedCodeLoginPanel({ + active: loginActive = true, companyId, adapterType, environmentId, @@ -2610,6 +2613,7 @@ function DisplayedCodeLoginPanel({ onRetryReady, }: AdapterLoginPanelProps) { const [sessionId, setSessionId] = useState(null); + const cancellation = useAdapterLoginCancellation(loginActive, companyId, adapterType, false, sessionId, { environmentId, aiConnection }); const [retrying, setRetrying] = useState(false); const [startError, setStartError] = useState(null); // The server delivers the one-time prompt on the first owner read only. Latch @@ -2632,8 +2636,9 @@ function DisplayedCodeLoginPanel({ const resumedRef = useRef(false); const startLogin = useMutation({ - mutationFn: () => agentsApi.startAdapterAuthLogin(companyId, adapterType, { environmentId, aiConnection }), + mutationFn: () => cancellation.accept(() => agentsApi.startAdapterAuthLogin(companyId, adapterType, { environmentId, aiConnection })), onSuccess: (session) => { + if (!session || !cancellation.isActive()) return; resumedRef.current = false; setStartError(null); setLatchedPrompt(null); @@ -2643,6 +2648,7 @@ function DisplayedCodeLoginPanel({ setSessionId(session.sessionId); }, onError: (error) => { + if (!cancellation.isActive()) return; setStartError(describeError(error).body); }, }); @@ -2660,6 +2666,7 @@ function DisplayedCodeLoginPanel({ mutationFn: () => agentsApi.cancelAdapterAuthLogin(companyId, adapterType, sessionId!), onSuccess: clearActiveSession, onError: (error) => { + if (!cancellation.isActive()) return; setStartError(describeError(error).body); }, }); @@ -2669,9 +2676,9 @@ function DisplayedCodeLoginPanel({ // 404 means no active session for the caller. const activeSessionQuery = useQuery({ queryKey: ["adapter-login-active-session", companyId, adapterType], - queryFn: async () => { + queryFn: async ({ signal }) => { try { - const active = await agentsApi.getActiveAdapterAuthLoginSession(companyId, adapterType); + const active = await cancellation.resume(() => agentsApi.getActiveAdapterAuthLoginSession(companyId, adapterType, { signal })); if (!active) return null; if ((aiConnection && active.environmentId !== environmentId) || Boolean(active.aiConnection) !== Boolean(aiConnection) || (aiConnection && (active.aiConnection?.provider !== aiConnection.provider || active.aiConnection?.method !== aiConnection.method || active.aiConnection?.connectionId !== aiConnection.connectionId || active.aiConnection?.ownership !== aiConnection.ownership || active.aiConnection?.allAgents !== aiConnection.allAgents || JSON.stringify(active.aiConnection?.agentIds) !== JSON.stringify(aiConnection.agentIds)))) throw new AdapterLoginConflictError(active.sessionId); return active; @@ -2687,6 +2694,7 @@ function DisplayedCodeLoginPanel({ // in flight — which the per-owner cap then rejects. gcTime: 0, staleTime: 0, + enabled: loginActive, }); const activeSessionView = useQueryView(activeSessionQuery); @@ -2697,8 +2705,8 @@ function DisplayedCodeLoginPanel({ const statusQuery = useQuery({ queryKey: ["adapter-login-status", companyId, adapterType, sessionId], - queryFn: () => agentsApi.getAdapterAuthLoginStatus(companyId, adapterType, sessionId!), - enabled: Boolean(sessionId) && !releasingResumedSession && !retrying, + queryFn: ({ signal }) => agentsApi.getAdapterAuthLoginStatus(companyId, adapterType, sessionId!, { signal }), + enabled: loginActive && Boolean(sessionId) && !releasingResumedSession && !retrying, // A status 404 is unrecoverable: the server removed the row, so a retry // cannot bring it back. The shared retry policy never retries a 4xx, so // it stops at once and fails loudly; only transient errors are retried. @@ -2721,14 +2729,14 @@ function DisplayedCodeLoginPanel({ const status = session?.status ?? null; const prompt = latchedPrompt; const isTerminal = status ? ADAPTER_LOGIN_TERMINAL_STATUSES.has(status) : false; - const isActive = Boolean(sessionId) && !isTerminal && !retrying; + const isActive = loginActive && Boolean(sessionId) && !isTerminal && !retrying; // A saving bind also blocks a new Sign in: the bind is an agent-update save, // and a second login started while it is in flight could finish its own // save first — the older save would then land last and silently revert the // agent to the previous account while the panel reports the newer bind. // Serializing at the only entry point is the whole fix; the panel has no // other way to start a login mid-save. - const startDisabled = startLogin.isPending || isActive || accountBindState === "saving"; + const startDisabled = !loginActive || startLogin.isPending || isActive || accountBindState === "saving"; // Adopt the caller's active session once, on mount. This is what makes a // page reload keep the session: with no local state at all, the panel would @@ -2736,7 +2744,7 @@ function DisplayedCodeLoginPanel({ // active login for this owner. const resumeAttemptedRef = useRef(false); useEffect(() => { - if (resumeAttemptedRef.current || !activeSessionQuery.isFetched) return; + if (!loginActive || resumeAttemptedRef.current || !activeSessionQuery.isFetched) return; resumeAttemptedRef.current = true; const active = activeSessionQuery.data; if (!active) return; @@ -2744,7 +2752,7 @@ function DisplayedCodeLoginPanel({ setStartError(null); setLatchedPrompt(active.prompt ?? null); setSessionId(active.sessionId); - }, [activeSessionQuery.isFetched, activeSessionQuery.data]); + }, [loginActive, activeSessionQuery.isFetched, activeSessionQuery.data]); // A resumed session's status poll found the session already gone: the read // that discovered it and the poll that tried to use it raced, and the @@ -2793,7 +2801,7 @@ function DisplayedCodeLoginPanel({ const startLoginRef = useRef(startLogin.mutate); startLoginRef.current = startLogin.mutate; useEffect(() => { - if (!autoStart || autoStartedRef.current) return; + if (!loginActive || !autoStart || autoStartedRef.current) return; // A failed lookup is not proof that no session exists: only a successful // lookup is. Show the failure to the user instead of starting a second // login the server would reject against the per-owner cap. @@ -2808,6 +2816,7 @@ function DisplayedCodeLoginPanel({ startLoginRef.current(); }, [ autoStart, + loginActive, activeSessionQuery.isSuccess, activeSessionView.kind, activeSessionQuery.data, @@ -2821,10 +2830,10 @@ function DisplayedCodeLoginPanel({ const onConnectedRef = useRef(onConnected); onConnectedRef.current = onConnected; useEffect(() => { - if (status !== "authenticated" || connectedRef.current) return; + if (!loginActive || status !== "authenticated" || connectedRef.current) return; connectedRef.current = true; onConnectedRef.current?.(sessionId ?? undefined); - }, [status]); + }, [loginActive, status]); // Drive the account-binding hand-off as a visible state machine, not a // fire-and-forget latch. The bind saves the agent, and the status poll @@ -2869,8 +2878,8 @@ function DisplayedCodeLoginPanel({ const onPromptReadyRef = useRef(onPromptReady); onPromptReadyRef.current = onPromptReady; useEffect(() => { - onPromptReadyRef.current?.(prompt?.url ?? null); - }, [prompt]); + onPromptReadyRef.current?.(loginActive ? prompt?.url ?? null : null); + }, [loginActive, prompt]); const failed = Boolean(isTerminal && status !== "authenticated") || statusQuery.isError; const retryingRef = useRef(false); @@ -3130,6 +3139,7 @@ const CLAUDE_LOGIN_TIMED_OUT_MESSAGE = "The login timed out. Start the login aga // and submits it. The panel clears the code right after submit. The panel treats // only the server `stored` state as success, and it never shows the OAuth token. function SubmittedBrowserCodeLoginPanel({ + active: loginActive = true, companyId, adapterType, environmentId, @@ -3147,6 +3157,7 @@ function SubmittedBrowserCodeLoginPanel({ onRetryReady, }: AdapterLoginPanelProps) { const [sessionId, setSessionId] = useState(null); + const cancellation = useAdapterLoginCancellation(loginActive, companyId, adapterType, true, sessionId, { environmentId, aiConnection }); const [retrying, setRetrying] = useState(false); const [startError, setStartError] = useState(null); // The server delivers the authorization URL on the guarded prompt read only. @@ -3227,7 +3238,7 @@ function SubmittedBrowserCodeLoginPanel({ const storedToken = storedTokenQuery.data ?? null; const startLogin = useMutation({ - mutationFn: () => + mutationFn: () => cancellation.accept(() => agentsApi.startClaudeSetupTokenLogin(companyId, { environmentId, aiConnection, @@ -3242,13 +3253,15 @@ function SubmittedBrowserCodeLoginPanel({ }, } : {}), - }), + })), onSuccess: (session) => { + if (!session || !cancellation.isActive()) return; resumedRef.current = false; resetLocalState(); setSessionId(session.sessionId); }, onError: (error) => { + if (!cancellation.isActive()) return; setStartError(describeError(error).body); }, }); @@ -3306,9 +3319,9 @@ function SubmittedBrowserCodeLoginPanel({ // with no local state. A 404 means no active session for the caller. const activeSessionQuery = useQuery({ queryKey: ["claude-setup-token-active-session", companyId], - queryFn: async () => { + queryFn: async ({ signal }) => { try { - const active = await agentsApi.getActiveClaudeSetupTokenLoginSession(companyId); + const active = await cancellation.resume(() => agentsApi.getActiveClaudeSetupTokenLoginSession(companyId, { signal })); if (!active) return null; if ((aiConnection && active.environmentId !== environmentId) || Boolean(active.aiConnection) !== Boolean(aiConnection) || (aiConnection && (active.aiConnection?.provider !== aiConnection.provider || active.aiConnection?.method !== aiConnection.method || active.aiConnection?.connectionId !== aiConnection.connectionId || active.aiConnection?.ownership !== aiConnection.ownership || active.aiConnection?.allAgents !== aiConnection.allAgents || JSON.stringify(active.aiConnection?.agentIds) !== JSON.stringify(aiConnection.agentIds)))) throw new Error("Another sign-in attempt is active. Finish or cancel it in its original account setup before starting this one."); return active; @@ -3324,6 +3337,7 @@ function SubmittedBrowserCodeLoginPanel({ // in flight — which the per-owner cap then rejects. gcTime: 0, staleTime: 0, + enabled: loginActive, }); const activeSessionView = useQueryView(activeSessionQuery); @@ -3337,11 +3351,11 @@ function SubmittedBrowserCodeLoginPanel({ // status 404 also stops the polls: the server cleaned up the session, so the // panel enters a terminal failure state instead. const pollingEnabled = - Boolean(sessionId) && !timedOut && !statusGone && !releasingResumedSession && !retrying; + loginActive && Boolean(sessionId) && !timedOut && !statusGone && !releasingResumedSession && !retrying; const statusQuery = useQuery({ queryKey: ["claude-setup-token-status", companyId, sessionId], - queryFn: () => agentsApi.getClaudeSetupTokenLoginStatus(companyId, sessionId!), + queryFn: ({ signal }) => agentsApi.getClaudeSetupTokenLoginStatus(companyId, sessionId!, { signal }), enabled: pollingEnabled, // A status 404 is terminal. The server removes a cleaned-up session at once, // so a retry cannot recover it. The shared retry policy never retries a @@ -3394,7 +3408,7 @@ function SubmittedBrowserCodeLoginPanel({ // active login for this owner. const resumeAttemptedRef = useRef(false); useEffect(() => { - if (resumeAttemptedRef.current || !activeSessionQuery.isFetched) return; + if (!loginActive || resumeAttemptedRef.current || !activeSessionQuery.isFetched) return; resumeAttemptedRef.current = true; const active = activeSessionQuery.data; if (!active) return; @@ -3405,7 +3419,7 @@ function SubmittedBrowserCodeLoginPanel({ setAuthorizationUrl(active.prompt.authorizationUrl); if (active.prompt.transportAdvisory) setTransportInsecure(true); } - }, [activeSessionQuery.isFetched, activeSessionQuery.data]); + }, [loginActive, activeSessionQuery.isFetched, activeSessionQuery.data]); // Poll the guarded prompt route until it returns the authorization URL. The // route returns 404 until the URL is ready, so the panel treats a 404 as @@ -3413,9 +3427,9 @@ function SubmittedBrowserCodeLoginPanel({ const promptQuery = useQuery({ queryKey: ["claude-setup-token-prompt", companyId, sessionId], enabled: pollingEnabled && !authorizationUrl, - queryFn: async () => { + queryFn: async ({ signal }) => { try { - return await agentsApi.getClaudeSetupTokenLoginPrompt(companyId, sessionId!); + return await agentsApi.getClaudeSetupTokenLoginPrompt(companyId, sessionId!, { signal }); } catch (error) { if (error instanceof ApiError && error.status === 404) return null; throw error; @@ -3437,6 +3451,7 @@ function SubmittedBrowserCodeLoginPanel({ mutationFn: (code: string) => agentsApi.submitClaudeSetupTokenBrowserCode(companyId, sessionId!, code), onError: (error) => { + if (!cancellation.isActive()) return; setStartError(describeError(error).body); }, }); @@ -3444,6 +3459,7 @@ function SubmittedBrowserCodeLoginPanel({ const completeLogin = useMutation({ mutationFn: () => agentsApi.completeClaudeSetupTokenLogin(companyId, sessionId!), onSuccess: (result) => { + if (!cancellation.isActive()) return; // The server reached the `stored` state. Hold the non-secret claim and // report it to the parent. The response carries no token. setStoredSessionId(result.storedSessionId); @@ -3465,11 +3481,11 @@ function SubmittedBrowserCodeLoginPanel({ const completeLoginRef = useRef(completeLogin.mutate); completeLoginRef.current = completeLogin.mutate; useEffect(() => { - if (status === "authenticated" && !completionStartedRef.current) { + if (loginActive && status === "authenticated" && !completionStartedRef.current) { completionStartedRef.current = true; completeLoginRef.current(); } - }, [status]); + }, [loginActive, status]); const isStored = storedSessionId !== null; const isFailure = @@ -3477,8 +3493,8 @@ function SubmittedBrowserCodeLoginPanel({ statusGone || Boolean(status && CLAUDE_LOGIN_FAILURE_STATUSES.has(status)); const isCompleting = status === "authenticated" && !isStored && !completionFailed; - const isActive = Boolean(sessionId) && !isStored && !isFailure && !timedOut && !retrying; - const startDisabled = startLogin.isPending || isActive; + const isActive = loginActive && Boolean(sessionId) && !isStored && !isFailure && !timedOut && !retrying; + const startDisabled = !loginActive || startLogin.isPending || isActive; // Cap the active login at the server deadline. The timer arms when the login // becomes active and clears when the login leaves the active state (a terminal @@ -3562,7 +3578,7 @@ function SubmittedBrowserCodeLoginPanel({ const startLoginRef = useRef(startLogin.mutate); startLoginRef.current = startLogin.mutate; useEffect(() => { - if (!autoStart || autoStartedRef.current) return; + if (!loginActive || !autoStart || autoStartedRef.current) return; // A failed lookup is not proof that no session exists: only a successful // lookup is. Show the failure to the user instead of starting a second // login the server would reject against the per-owner cap. @@ -3577,6 +3593,7 @@ function SubmittedBrowserCodeLoginPanel({ startLoginRef.current(); }, [ autoStart, + loginActive, activeSessionQuery.isSuccess, activeSessionView.kind, activeSessionQuery.data, @@ -3630,10 +3647,10 @@ function SubmittedBrowserCodeLoginPanel({ const onConnectedRef = useRef(onConnected); onConnectedRef.current = onConnected; useEffect(() => { - if (!isStored || connectedRef.current) return; + if (!loginActive || !isStored || connectedRef.current) return; connectedRef.current = true; onConnectedRef.current?.(sessionId ?? undefined); - }, [isStored]); + }, [loginActive, isStored]); // The other end of `onCodeSubmitted`. Any of these after a submit means the // code is not going to become a stored login, and a caller still showing @@ -3651,8 +3668,8 @@ function SubmittedBrowserCodeLoginPanel({ const onPromptReadyRef = useRef(onPromptReady); onPromptReadyRef.current = onPromptReady; useEffect(() => { - onPromptReadyRef.current?.(authorizationUrl); - }, [authorizationUrl]); + onPromptReadyRef.current?.(loginActive ? authorizationUrl : null); + }, [loginActive, authorizationUrl]); const retryingRef = useRef(false); const retryLogin = useMutation({ diff --git a/ui/src/components/OnboardingWizard.test.tsx b/ui/src/components/OnboardingWizard.test.tsx index b28ff4784cb..2e6e9993e94 100644 --- a/ui/src/components/OnboardingWizard.test.tsx +++ b/ui/src/components/OnboardingWizard.test.tsx @@ -3968,19 +3968,168 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( await act(async () => root.unmount()); }); - it("resumes the same claude_local session after Back, rather than starting a second", async () => { - // This is the behaviour that makes the card's Cancel removable. Back only - // hides the card — it deliberately does not release the session — so - // coming back has to adopt the one already running. If it started a - // fresh one instead, the removed Cancel would have been the only way out - // of a login the customer could no longer reach, and the per-owner cap - // would reject the second start. + it.each(([ + ["claude_local", "resume"], ["codex_local", "resume"], + ["claude_local", "start"], ["codex_local", "start"], + ["claude_local", "prompt"], ["codex_local", "prompt"], + ] as const).flatMap(([adapterType, stage]) => [false, true].map(afterExit => [adapterType, stage, afterExit] as const)))("keeps %s cancelled when its pending %s response arrives after Back (exit finished: %s)", async (adapterType, stage, afterExit) => { + const claude = adapterType === "claude_local"; + const activeApi = claude ? mockAgentsApi.getActiveClaudeSetupTokenLoginSession : mockAgentsApi.getActiveAdapterAuthLoginSession; + const startApi = claude ? mockAgentsApi.startClaudeSetupTokenLogin : mockAgentsApi.startAdapterAuthLogin; + const promptApi = claude ? mockAgentsApi.getClaudeSetupTokenLoginPrompt : mockAgentsApi.getAdapterAuthLoginStatus; + const cancelApi = claude ? mockAgentsApi.cancelClaudeSetupTokenLogin : mockAgentsApi.cancelAdapterAuthLogin; + const session = { + sessionId: claude ? "claude-session-1" : "codex-session-1", environmentId: "env-sandbox-1", + status: "waiting_for_user", expiresAt: null, failure: null, + aiConnection: { provider: claude ? "anthropic" : "openai", method: "subscription", ownership: "personal", agentIds: [], allAgents: true }, + panelMode: claude ? "submitted_browser_code" : "displayed_code", + prompt: claude ? { authorizationUrl: "https://claude.ai/oauth/authorize?cancelled=true" } + : { url: "https://auth.openai.com/codex/device", code: "CANCELLED-CODE" }, + }; + let finish!: (value: unknown) => void; + const pending = new Promise(resolve => { finish = resolve; }); + const pendingApi = stage === "resume" ? activeApi : stage === "start" ? startApi : promptApi; + pendingApi.mockReturnValue(pending as never); + mockAgentsApi.getAdapterAuthSignal.mockResolvedValue({ status: "absent" }); + const { root } = await openStep4({ adapterType }); + try { + await pickSource(claude ? /Claude/ : /OpenAI/); + expect(pendingApi).toHaveBeenCalled(); + await act(async () => [...document.body.querySelectorAll("button")].find(button => button.textContent?.trim().startsWith("Back"))!.click()); + if (afterExit) for (let i = 0; i < 12; i++) await flushReact(); + if (stage !== "start") { + const signals = (pendingApi.mock.calls as unknown[][]).map(call => (call.at(-1) as { signal?: AbortSignal })?.signal).filter(signal => signal !== undefined); + expect(signals.length).toBeGreaterThan(0); + expect(signals.every(signal => signal.aborted)).toBe(true); + } + await act(async () => finish(stage === "prompt" && claude ? session.prompt : session)); + for (let i = 0; i < 12; i++) await flushReact(); + expect(document.querySelectorAll('[role="radio"][aria-checked="true"]')).toHaveLength(0); + expect(document.body.textContent).not.toContain("CANCELLED-CODE"); + expect(document.querySelector('input[aria-label="Authorization code"]')).toBeNull(); + expect(document.body.textContent).not.toContain("Let's get started..."); + expect(mockAgentsApi.hire).not.toHaveBeenCalled(); + if (claude) expect(cancelApi).toHaveBeenCalledWith("company-new", session.sessionId); + else expect(cancelApi).toHaveBeenCalledWith("company-new", "codex_local", session.sessionId); + } finally { await act(async () => root.unmount()); } + }); + + it.each([ + ["claude_local", false], ["codex_local", false], ["claude_local", true], ["codex_local", true], + ] as const)("discovers and releases %s after its resume read rejects on abort (panel mounted: %s)", async (adapterType, panelMounted) => { + const claude = adapterType === "claude_local"; + const activeApi = claude ? mockAgentsApi.getActiveClaudeSetupTokenLoginSession : mockAgentsApi.getActiveAdapterAuthLoginSession; + const cancelApi = claude ? mockAgentsApi.cancelClaudeSetupTokenLogin : mockAgentsApi.cancelAdapterAuthLogin; + const startApi = claude ? mockAgentsApi.startClaudeSetupTokenLogin : mockAgentsApi.startAdapterAuthLogin; + const session = { sessionId: "abandoned-session", environmentId: "env-sandbox-1", status: "waiting_for_user", expiresAt: null, + panelMode: claude ? "submitted_browser_code" : "displayed_code", + aiConnection: { provider: claude ? "anthropic" : "openai", method: "subscription", ownership: "personal", agentIds: [], allAgents: true } }; + let serverActive = true; + let aborts = 0; + let finishDiscovery!: () => void; + let finishCancellation = () => {}; + const discovery = new Promise(resolve => { finishDiscovery = resolve; }); + activeApi.mockImplementation((...args: unknown[]) => { + const options = args.at(-1) as { signal?: AbortSignal; fresh?: boolean }; + if (options?.fresh) return discovery.then(() => serverActive ? session : null); + if (!serverActive) return Promise.resolve(null); + return new Promise((_resolve, reject) => options?.signal?.addEventListener("abort", () => { + aborts++; + reject(new DOMException("The request was aborted", "AbortError")); + }, { once: true })); + }); + cancelApi.mockImplementation(() => new Promise(resolve => { finishCancellation = () => { serverActive = false; resolve({}); }; })); + mockAgentsApi.getAdapterAuthSignal.mockResolvedValue({ status: "absent" }); + const { root } = await openStep4({ adapterType }); + const matchMedia = window.matchMedia; + const restoreMotion = () => Object.defineProperty(window, "matchMedia", { configurable: true, writable: true, value: matchMedia }); + try { + if (panelMounted) await pickSource(claude ? /Claude/ : /OpenAI/); + else { + Object.defineProperty(window, "matchMedia", { configurable: true, writable: true, + value: (media: string) => ({ matches: false, media, onchange: null, addListener() {}, removeListener() {}, addEventListener() {}, removeEventListener() {}, dispatchEvent: () => false }) }); + const tile = [...document.body.querySelectorAll('[role="radio"]')].find(tile => (claude ? /Claude/ : /OpenAI/).test(tile.textContent ?? ""))!; + await act(async () => (tile as HTMLElement).click()); + expect(activeApi).toHaveBeenCalledTimes(1); + restoreMotion(); + } + await act(async () => [...document.body.querySelectorAll("button")].find(button => button.textContent?.trim().startsWith("Back"))!.click()); + for (let i = 0; i < 12; i++) await flushReact(); + expect(aborts).toBeGreaterThan(0); + await vi.waitFor(() => expect((activeApi.mock.calls as unknown[][]).some(call => (call.at(-1) as { fresh?: boolean })?.fresh)).toBe(true)); + expect(document.querySelectorAll('[role="radio"][aria-checked="true"]')).toHaveLength(0); + await pickSource(claude ? /Claude/ : /OpenAI/); + expect(startApi).not.toHaveBeenCalled(); + await act(async () => finishDiscovery()); + await vi.waitFor(() => expect(cancelApi).toHaveBeenCalled()); + expect(startApi).not.toHaveBeenCalled(); + await act(async () => finishCancellation()); + for (let i = 0; i < 12; i++) await flushReact(); + expect(serverActive).toBe(false); + expect(startApi).toHaveBeenCalledTimes(1); + expect(mockAgentsApi.hire).not.toHaveBeenCalled(); + } finally { + restoreMotion(); + finishDiscovery(); + finishCancellation(); + await act(async () => root.unmount()); + } + }); + + it.each((["claude_local", "codex_local"] as const).flatMap(adapterType => + (["environment", "legacy", "connection", "ownership", "agents", "allAgents"] as const).map(difference => [adapterType, difference] as const)))("preserves another %s sign-in after Back when %s differs", async (adapterType, difference) => { + const claude = adapterType === "claude_local"; + const activeApi = claude ? mockAgentsApi.getActiveClaudeSetupTokenLoginSession : mockAgentsApi.getActiveAdapterAuthLoginSession; + const cancelApi = claude ? mockAgentsApi.cancelClaudeSetupTokenLogin : mockAgentsApi.cancelAdapterAuthLogin; + const startApi = claude ? mockAgentsApi.startClaudeSetupTokenLogin : mockAgentsApi.startAdapterAuthLogin; + const intent = { provider: claude ? "anthropic" : "openai", method: "subscription", ownership: "personal", agentIds: [] as string[], allAgents: true }; + const session = { sessionId: "settings-sign-in", environmentId: difference === "environment" ? "other-environment" : "env-sandbox-1", + status: "waiting_for_user", expiresAt: null, aiConnection: difference === "legacy" ? undefined : { ...intent, + ...(difference === "connection" ? { connectionId: "other-connection" } : {}), + ...(difference === "ownership" ? { ownership: "organization" } : {}), + ...(difference === "agents" ? { agentIds: ["other-agent"] } : {}), + ...(difference === "allAgents" ? { allAgents: false } : {}) } }; + activeApi.mockResolvedValue(session); + mockAgentsApi.getAdapterAuthSignal.mockResolvedValue({ status: "absent" }); + const { root } = await openStep4({ adapterType }); + try { + for (let i = 0; i < 12; i++) await flushReact(); + expect(document.body.textContent).toContain("Sign-in didn’t finish"); + await act(async () => [...document.body.querySelectorAll("button")].find(button => button.textContent?.trim().startsWith("Back"))!.click()); + for (let i = 0; i < 12; i++) await flushReact(); + expect(cancelApi).not.toHaveBeenCalled(); + expect(startApi).not.toHaveBeenCalled(); + expect(document.querySelectorAll('[role="radio"][aria-checked="true"]')).toHaveLength(0); + } finally { await act(async () => root.unmount()); } + }); + + it.each(["claude_local", "codex_local"] as const)("preserves a conflicting %s resume response received after Back", async adapterType => { + const claude = adapterType === "claude_local"; + const activeApi = claude ? mockAgentsApi.getActiveClaudeSetupTokenLoginSession : mockAgentsApi.getActiveAdapterAuthLoginSession; + const cancelApi = claude ? mockAgentsApi.cancelClaudeSetupTokenLogin : mockAgentsApi.cancelAdapterAuthLogin; + let finish!: (session: unknown) => void; + activeApi.mockReturnValue(new Promise(resolve => { finish = resolve; })); + mockAgentsApi.getAdapterAuthSignal.mockResolvedValue({ status: "absent" }); + const { root } = await openStep4({ adapterType }); + try { + await pickSource(claude ? /Claude/ : /OpenAI/); + await act(async () => [...document.body.querySelectorAll("button")].find(button => button.textContent?.trim().startsWith("Back"))!.click()); + await act(async () => finish({ sessionId: "settings-sign-in", environmentId: "another-environment", status: "waiting_for_user" })); + for (let i = 0; i < 12; i++) await flushReact(); + expect(cancelApi).not.toHaveBeenCalled(); + expect(document.querySelectorAll('[role="radio"][aria-checked="true"]')).toHaveLength(0); + } finally { await act(async () => root.unmount()); } + }); + + it("cancels claude_local on Back and starts a fresh sign-in when selected again", async () => { const session = { sessionId: "claude-session-1", status: "pending", expiresAt: new Date(Date.now() + 600_000).toISOString(), }; let started = false; + let finishCancellation!: () => void; + mockAgentsApi.cancelClaudeSetupTokenLogin.mockImplementation(() => new Promise(resolve => { finishCancellation = () => { started = false; resolve({}); }; })); mockAgentsApi.startClaudeSetupTokenLogin.mockImplementation(async () => { started = true; return session; @@ -3993,8 +4142,6 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( const { root } = await openStep4({ adapterType: "claude_local" }); await pickSource(/Claude/); - // The login is genuinely running: without this the assertion below holds - // for the wrong reason. expect(mockAgentsApi.startClaudeSetupTokenLogin).toHaveBeenCalledTimes(1); const back = [...document.body.querySelectorAll("button")].find((b) => @@ -4005,18 +4152,23 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( }); for (let i = 0; i < 12; i++) await flushReact(); + expect(mockAgentsApi.cancelClaudeSetupTokenLogin).toHaveBeenCalled(); await pickSource(/Claude/); + expect(mockAgentsApi.startClaudeSetupTokenLogin).toHaveBeenCalledTimes(1); + await act(async () => finishCancellation()); for (let i = 0; i < 8; i++) await flushReact(); - expect(mockAgentsApi.startClaudeSetupTokenLogin).toHaveBeenCalledTimes(1); + expect(mockAgentsApi.startClaudeSetupTokenLogin).toHaveBeenCalledTimes(2); expect(mockAgentsApi.getActiveClaudeSetupTokenLoginSession).toHaveBeenCalled(); await act(async () => root.unmount()); }); - it("resumes the same codex_local session after Back, rather than starting a second", async () => { + it("cancels codex_local on Back and starts a fresh sign-in when selected again", async () => { const session = { sessionId: "codex-session-1", status: "pending" }; let started = false; + let finishCancellation!: () => void; + mockAgentsApi.cancelAdapterAuthLogin.mockImplementation(() => new Promise(resolve => { finishCancellation = () => { started = false; resolve({}); }; })); mockAgentsApi.startAdapterAuthLogin.mockImplementation(async () => { started = true; return session; @@ -4039,10 +4191,13 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( }); for (let i = 0; i < 12; i++) await flushReact(); + expect(mockAgentsApi.cancelAdapterAuthLogin).toHaveBeenCalled(); await pickSource(/OpenAI/); + expect(mockAgentsApi.startAdapterAuthLogin).toHaveBeenCalledTimes(1); + await act(async () => finishCancellation()); for (let i = 0; i < 8; i++) await flushReact(); - expect(mockAgentsApi.startAdapterAuthLogin).toHaveBeenCalledTimes(1); + expect(mockAgentsApi.startAdapterAuthLogin).toHaveBeenCalledTimes(2); expect(mockAgentsApi.getActiveAdapterAuthLoginSession).toHaveBeenCalled(); await act(async () => root.unmount()); diff --git a/ui/src/components/OnboardingWizard.tsx b/ui/src/components/OnboardingWizard.tsx index 8ffa177b135..c2ba778a69e 100644 --- a/ui/src/components/OnboardingWizard.tsx +++ b/ui/src/components/OnboardingWizard.tsx @@ -1,3 +1,5 @@ +import type { AiConnectionLoginIntent } from "@paperclipai/shared"; +import { cancelAdapterLoginSession, cancelPendingAdapterLogin, matchesAdapterLoginTarget } from "./useAdapterLoginCancellation"; import { useQueryView } from "@/components/QueryView"; import type { AgentRunner, AgentRunnerChoice } from "@paperclipai/shared"; import { needsClaudeSetupQualification, qualifyClaudeSetup } from "../lib/qualify-claude-setup"; @@ -1149,25 +1151,37 @@ function OnboardingWizardInner({ * panel unmounted and the resumed session unreachable from this step. A 404 * means no active session for the caller. */ + const managedSubscriptionIntent: AiConnectionLoginIntent | undefined = managedSubscriptionProvider + ? { provider: managedSubscriptionProvider, method: "subscription", name: `My ${CONNECT_SOURCE_NAMES[adapterType] ?? managedProvider} subscription`, ownership: "personal", agentIds: [], allAgents: true } + : undefined; + const loginTarget = { environmentId: resolvedLoginEnvironmentId ?? "", aiConnection: managedSubscriptionIntent }; + const dismissedLoginResumes = useRef(new Set()); + const loginResumeKey = JSON.stringify([createdCompanyId, adapterType]); const activeLoginSessionQuery = useQuery({ queryKey: createdCompanyId ? queryKeys.agents.activeLoginSession(createdCompanyId, adapterType) : ["agents", "none", "active-login-session", adapterType], - queryFn: async () => { + queryFn: async ({ signal }) => { try { - return adapterCaps.login?.panelMode === "submitted_browser_code" - ? await agentsApi.getActiveClaudeSetupTokenLoginSession(createdCompanyId!) - : await agentsApi.getActiveAdapterAuthLoginSession(createdCompanyId!, adapterType); + const session = adapterCaps.login?.panelMode === "submitted_browser_code" + ? await agentsApi.getActiveClaudeSetupTokenLoginSession(createdCompanyId!, { signal }) + : await agentsApi.getActiveAdapterAuthLoginSession(createdCompanyId!, adapterType, { signal }); + if (session && dismissedLoginResumes.current.has(loginResumeKey)) { + if (matchesAdapterLoginTarget(session, loginTarget)) await cancelAdapterLoginSession(createdCompanyId!, adapterType, + adapterCaps.login?.panelMode === "submitted_browser_code", session.sessionId); + return null; + } + return session; } catch (error) { if (error instanceof ApiError && error.status === 404) return null; throw error; } }, enabled: - Boolean(createdCompanyId) && effectiveOnboardingOpen && step === 4 && canShowAdapterLogin, + Boolean(createdCompanyId) && effectiveOnboardingOpen && step === 4 && canShowAdapterLogin && !dismissedLoginResumes.current.has(loginResumeKey), }); useEffect(() => { - if (!activeLoginSessionQuery.data || credentialMode === "api" || savedSubscription || savedKeys.storedLogin.data) return; + if (dismissedLoginResumes.current.has(loginResumeKey) || !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). @@ -1554,21 +1568,14 @@ function OnboardingWizardInner({ // it has been answered the button has nothing to do. { label: "Next", icon: "arrow", disabled: true }; - /** - * Back, on the connect step, unwinds the sign-in before it leaves the step. - * - * This hides the card; it does not cancel the login. Unmounting the panel - * does not release the server session — the session stays reachable for a - * later resume, the same read that restores it after a reload — so backing - * out and returning shows the sign-in still running, not a fresh one. - * - * Nothing releases it explicitly any more. The card carried a Cancel that - * did, sitting beside an instruction and directly above this step's own - * Back, and two ways out of one screen is one too many — the button went and - * the release went with it. What is left is the server deadline, which is - * the same thing that collects a session abandoned by closing the tab. - */ function unwindConnectStep() { + localLogin.cancel(); + dismissedLoginResumes.current.add(loginResumeKey); + if (createdCompanyId) { + void queryClient.cancelQueries({ queryKey: queryKeys.agents.activeLoginSession(createdCompanyId, adapterType) }); + if (canShowAdapterLogin && credentialMode !== "api") void cancelPendingAdapterLogin(createdCompanyId, adapterType, + adapterCaps.login?.panelMode === "submitted_browser_code", loginTarget); + } connectAttemptRef.current++; hiringAgentRef.current = null; setLoading(false); @@ -3242,11 +3249,6 @@ function OnboardingWizardInner({ in the connect step's chrome. It owns the session; the step owns the sequence around it. - Unmounting it is not the cancel: the session stays - reachable for a later resume, so Back and a source - switch only hide the card, and nothing here releases - the session early — see `unwindConnectStep`. - No "Use saved login" control: the hire step already applies a stored login on its own. */ { + if (!connectCardLive) return; setConnectAuthUrl(url); // The prompt arriving is what ends the waiting beat. if (url) setConnectPhase((p) => (p === "loading" ? "ready" : p)); diff --git a/ui/src/components/ai-connections/useLocalAiLogin.test.tsx b/ui/src/components/ai-connections/useLocalAiLogin.test.tsx index 1bb18c5d49c..d97385d3e1b 100644 --- a/ui/src/components/ai-connections/useLocalAiLogin.test.tsx +++ b/ui/src/components/ai-connections/useLocalAiLogin.test.tsx @@ -22,7 +22,7 @@ beforeEach(() => { afterEach(() => { flushSync(() => root.unmount()); host.remove(); }); function Harness({ name = "Account", provider = "openai", enabled = true }: { name?: string; provider?: "anthropic" | "openai"; enabled?: boolean }) { const login = useLocalAiLogin("company", { provider, method: "subscription", name, ownership: "personal", agentIds: [], allAgents: true }, enabled); - return <>; + return <>; } it("checks once under StrictMode, preserves renaming and navigation, and cancels only on explicit retry", async () => { flushSync(() => root.render()); @@ -106,3 +106,21 @@ it("hides a failed device prompt and lets the user restart sign-in", async () => await vi.waitFor(() => expect(host.textContent).toContain("NEW-CODE")); expect(host.querySelector('[role="alert"]')).toBeNull(); }); + +it.each(["start", "check"])("cancels a local subscription with a pending %s and ignores its late result", async stage => { + let finish!: (result: unknown) => void; + const pending = new Promise(resolve => { finish = resolve; }); + const held = stage === "start" ? api.startLocalLogin : api.checkLocalLogin; + held.mockReturnValueOnce(pending); + flushSync(() => root.render()); + await vi.waitFor(() => expect(held).toHaveBeenCalledTimes(1)); + flushSync(() => Array.from(host.querySelectorAll("button")).find(button => button.textContent === "Back")!.click()); + finish(stage === "start" ? { sessionId: "attempt-1", expiresAt: "2099-01-01T00:00:00Z" } + : { status: "sign_in_required", authorizationUrl: "https://auth.openai.com/codex/device", code: "CANCELLED-CODE" }); + await vi.waitFor(() => expect(api.cancelLocalLogin).toHaveBeenCalledWith("company", "attempt-1")); + window.dispatchEvent(new Event("focus")); + await new Promise(resolve => setTimeout(resolve, 10)); + expect(host.textContent).not.toContain("CANCELLED-CODE"); + expect(api.checkLocalLogin).toHaveBeenCalledTimes(stage === "start" ? 0 : 1); + expect(api.connectLocal).not.toHaveBeenCalled(); +}); diff --git a/ui/src/components/ai-connections/useLocalAiLogin.ts b/ui/src/components/ai-connections/useLocalAiLogin.ts index 16f5abfcebf..1dfec4c866f 100644 --- a/ui/src/components/ai-connections/useLocalAiLogin.ts +++ b/ui/src/components/ai-connections/useLocalAiLogin.ts @@ -49,17 +49,17 @@ export function useLocalAiLogin(companyId: string | null, intent: AiConnectionLo } const request = isolated ? current.current!.request : Promise.resolve(null); async function check() { - if (checking || cancelled) return; + if (checking || cancelled || (isolated && current.current?.request !== request)) return; checking = true; clearTimeout(timer); try { const result = await request; - if (cancelled) return; + if (cancelled || (isolated && current.current?.request !== request)) return; setAttempt(result); const next = await aiConnectionsApi.checkLocalLogin(companyId!, { ...latestIntent.current, ...(result ? { localSessionId: result.sessionId } : {}), }); - if (cancelled) return; + if (cancelled || (isolated && current.current?.request !== request)) return; setStatus(next.status); setAuthorizationUrl(next.authorizationUrl ?? null); setCode(next.code ?? null); @@ -69,7 +69,7 @@ export function useLocalAiLogin(companyId: string | null, intent: AiConnectionLo // browser visit; sign-in never requires repeated Connect clicks. if (next.status === "sign_in_required") timer = setTimeout(() => void check(), 5000); } catch (cause) { - if (!cancelled) setError(describeError(cause).body); + if (!cancelled && (!isolated || current.current?.request === request)) setError(describeError(cause).body); } finally { checking = false; } } const onFocus = () => { if (!document.hidden) void check(); }; @@ -94,6 +94,14 @@ export function useLocalAiLogin(companyId: string | null, intent: AiConnectionLo status, preparing: active && !status && !error, error, + cancel: () => { + cancelCurrent(); + setAttempt(null); + setStatus(null); + setAuthorizationUrl(null); + setCode(null); + setError(null); + }, retry: () => { restartRequested.current = true; cancelCurrent(); setGeneration((value) => value + 1); }, submitCode: async (browserCode: string) => { if (!companyId || !attempt) throw new Error("Start sign-in before submitting a code."); diff --git a/ui/src/components/useAdapterLoginCancellation.ts b/ui/src/components/useAdapterLoginCancellation.ts new file mode 100644 index 00000000000..ea077cc662a --- /dev/null +++ b/ui/src/components/useAdapterLoginCancellation.ts @@ -0,0 +1,94 @@ +import { useCallback, useEffect, useMemo, useRef } from "react"; +import { useQueryClient } from "@tanstack/react-query"; +import type { AiConnectionLoginIntent } from "@paperclipai/shared"; +import { agentsApi } from "../api/agents"; + +type AdapterLoginTarget = { environmentId: string; aiConnection?: AiConnectionLoginIntent }; +type AdapterLoginSession = { sessionId: string; environmentId?: string; aiConnection?: AiConnectionLoginIntent }; + +function connectionTargetKey(intent?: AiConnectionLoginIntent) { + return intent ? JSON.stringify([intent.provider, intent.method, intent.connectionId, intent.ownership, intent.allAgents, intent.agentIds]) : null; +} + +export function matchesAdapterLoginTarget(session: AdapterLoginSession, target: AdapterLoginTarget) { + return Boolean(target.environmentId) && session.environmentId === target.environmentId + && connectionTargetKey(session.aiConnection) === connectionTargetKey(target.aiConnection); +} + +const pendingCancellations = new Map>(); +const sessionCancellations = new Map>(); + +export function cancelAdapterLoginSession(companyId: string, adapterType: string, submittedCode: boolean, sessionId: string) { + const key = JSON.stringify([companyId, adapterType, sessionId]); + const previous = sessionCancellations.get(key); + if (previous) return previous; + const request = (submittedCode + ? agentsApi.cancelClaudeSetupTokenLogin(companyId, sessionId) + : agentsApi.cancelAdapterAuthLogin(companyId, adapterType, sessionId)).catch(() => {}); + sessionCancellations.set(key, request); + void request.then(() => { if (sessionCancellations.get(key) === request) sessionCancellations.delete(key); }); + return request; +} + +export function cancelPendingAdapterLogin(companyId: string, adapterType: string, submittedCode: boolean, target: AdapterLoginTarget, pending: Promise[] = [], discover = true) { + const key = JSON.stringify([companyId, adapterType]); + const previous = pendingCancellations.get(key); + const cleanup = Promise.allSettled([...(previous ? [previous] : []), ...pending]).then(async () => { + if (!discover) return; + const session = submittedCode + ? await agentsApi.getActiveClaudeSetupTokenLoginSession(companyId, { fresh: true }) + : await agentsApi.getActiveAdapterAuthLoginSession(companyId, adapterType, { fresh: true }); + if (session && matchesAdapterLoginTarget(session, target)) await cancelAdapterLoginSession(companyId, adapterType, submittedCode, session.sessionId); + }).catch(() => {}); + pendingCancellations.set(key, cleanup); + void cleanup.then(() => { if (pendingCancellations.get(key) === cleanup) pendingCancellations.delete(key); }); + return cleanup; +} + +export function useAdapterLoginCancellation(active: boolean, companyId: string, adapterType: string, submittedCode: boolean, sessionId: string | null, target: AdapterLoginTarget) { + const queryClient = useQueryClient(); + const ownerKey = JSON.stringify([companyId, adapterType]); + const targetKey = JSON.stringify([companyId, adapterType, target.environmentId, connectionTargetKey(target.aiConnection)]); + const attemptTarget = useMemo(() => target, [targetKey]); + const scope = useRef({ active, generation: 0, targetKey }); + if (scope.current.active !== active || scope.current.targetKey !== targetKey) scope.current = { active, generation: scope.current.generation + 1, targetKey }; + const requests = useRef(new Set>()); + const cancelled = useRef(new Map>()); + const cancel = useCallback((id: string) => { + const previous = cancelled.current.get(id); + if (previous) return previous; + const request = cancelAdapterLoginSession(companyId, adapterType, submittedCode, id); + cancelled.current.set(id, request); + return request; + }, [companyId, adapterType, submittedCode]); + + const accept = useCallback((request: () => Promise, resumed = false): Promise => { + const generation = scope.current.generation; + const result = (async () => { + await pendingCancellations.get(ownerKey); + if (!scope.current.active || scope.current.generation !== generation) return null; + const session = await request(); + if (session && (!scope.current.active || scope.current.generation !== generation)) { + if (!resumed || matchesAdapterLoginTarget(session, attemptTarget)) await cancel(session.sessionId); + return null; + } + return session; + })(); + requests.current.add(result); + const forget = () => { requests.current.delete(result); }; + void result.then(forget, forget); + return result; + }, [cancel, ownerKey, attemptTarget]); + + useEffect(() => { + if (active) return; + const keys = submittedCode + ? [["claude-setup-token-active-session", companyId], ["claude-setup-token-status", companyId], ["claude-setup-token-prompt", companyId]] + : [["adapter-login-active-session", companyId, adapterType], ["adapter-login-status", companyId, adapterType]]; + for (const queryKey of keys) void queryClient.cancelQueries({ queryKey }); + void cancelPendingAdapterLogin(companyId, adapterType, submittedCode, attemptTarget, + [...requests.current, ...(sessionId ? [cancel(sessionId)] : [])], !sessionId); + }, [active, companyId, adapterType, submittedCode, sessionId, queryClient, cancel, ownerKey, attemptTarget]); + + return { accept, resume: (request: () => Promise) => accept(request, true), isActive: () => scope.current.active }; +}