diff --git a/doc/plans/2026-10-12-claude-runner-default.md b/doc/plans/2026-10-12-claude-runner-default.md index 49e1afaf8c5..fc95b3a075f 100644 --- a/doc/plans/2026-10-12-claude-runner-default.md +++ b/doc/plans/2026-10-12-claude-runner-default.md @@ -525,3 +525,33 @@ fixture now has its own exception alongside immutable prior identities; other fixtures remain excluded. Cloud job 115053600515 retained the exact ENOENT failure. No local Docker invocation was used. The next composed heads must pass this check before candidate package qualification. + +## Completed-discovery onboarding guard — 2026-10-12 + +Fresh behavior review reproduced a separate onboarding race: saved-subscription +verification could start while runner discovery was pending, building an +unresolved Auto configuration with Legacy's 1000-turn cap. Both delayed native +and Legacy cases fail before treatment. Onboarding now enables discovery after +its environment/settings/policy queries succeed and gates manual, keyboard, +saved-account and completed-login advancement on the matching completed result. +The builder receives that resolved value. Target or runner changes invalidate +an in-flight attempt; metadata-only environment refreshes preserve it. Discovery +failure or a missing harness result stops setup and offers an explicit availability +retry, without testing or hiring. No connection/authentication mechanism changed. + +All 173 mounted onboarding/guided-setup/builder controls pass, including keyboard +submission while discovery is pending, both native and Legacy results, failed +availability/retry and obsolete-target verification rejection. UI noemit, token +and map checks pass. Earlier subscription/account/login controls remain intact; +the prior environment-refresh fixture now changes metadata rather than changing +its target ID, and a separate two-target control proves actual invalidation. + +Protocol source `3aaf1f8b1e1e525378f0f2330fd5453a3ac1c252` has 53 successful +current-head cloud checks, including Rust, static checks, both runner unit shards +and Docker-context integrity, with fresh 5/5 review. Earlier local varying lease +failures remain recorded. Behavior/platform need fresh gates after this UI guard; +no final candidate package campaign has been dispatched and no paid turn added. +The retained installed preview has been inspected in light/dark mode with the +original theme restored and agent Save disabled. The browser's requested mobile +override did not affect its actual 1150-pixel viewport, so that attempt is not +mobile proof and its override was reset. Final responsive qualification remains. diff --git a/ui/src/components/OnboardingWizard.test.tsx b/ui/src/components/OnboardingWizard.test.tsx index c6294d78ac4..7871c7a5470 100644 --- a/ui/src/components/OnboardingWizard.test.tsx +++ b/ui/src/components/OnboardingWizard.test.tsx @@ -1,6 +1,8 @@ // @vitest-environment jsdom import { act } from "react"; +import { buildClaudeLocalConfig } from "../../../packages/adapters/claude-local/src/ui/build-config"; +import type { CreateConfigValues } from "@paperclipai/adapter-utils"; import type { LocalAiLoginStatus } from "@paperclipai/shared"; import { createRoot } from "react-dom/client"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; @@ -124,7 +126,7 @@ const mockAgentsApi = vi.hoisted(() => ({ // can shape the built adapter config (e.g. a configured ANTHROPIC_API_KEY) // without a real adapter package. const mockAdapterBuild = vi.hoisted(() => ({ - buildAdapterConfig: vi.fn(() => ({}) as Record), + buildAdapterConfig: vi.fn((_values?: CreateConfigValues) => ({}) as Record), })); // The Connect path loads environment settings before probing; without these // the probe dies on "Could not load environment settings" and the hire never @@ -178,6 +180,8 @@ vi.mock("../context/DialogContext", () => ({ vi.mock("../context/CompanyContext", () => ({ useCompany: () => mockCompany, })); +const mockAdaptersApi = vi.hoisted(() => ({ list: vi.fn() })); +vi.mock("../api/adapters", () => ({ adaptersApi: mockAdaptersApi })); vi.mock("../api/companies", () => ({ companiesApi: mockCompaniesApi })); vi.mock("../api/goals", () => ({ goalsApi: mockGoalsApi })); vi.mock("../api/agents", () => ({ agentsApi: mockAgentsApi })); @@ -349,6 +353,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( mockCompaniesApi.list.mockResolvedValue([]); mockAdapterRegistry.list = []; mockAdapterRegistry.disabled = new Set(); + mockAdaptersApi.list.mockReset().mockResolvedValue(["claude_local", "codex_local"].map(type => ({ type, loaded: true, disabled: false, supportedRunners: ["paperclip", "legacy"], defaultRunner: "legacy" }))); mockAdapterBuild.buildAdapterConfig.mockReset(); mockAdapterBuild.buildAdapterConfig.mockReturnValue({}); // Default: no stored Claude login for the owner. The route returns a @@ -3136,6 +3141,82 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( expect(document.body.textContent).not.toContain("Start sign-in again"); } + it.each(["paperclip", "legacy"] as const)("waits for Claude runner discovery before saved subscription verification (%s)", async defaultRunner => { + let finish!: (value: object[]) => void; + mockAdaptersApi.list.mockReturnValue(new Promise(resolve => { finish = resolve; })); + mockAdapterBuild.buildAdapterConfig.mockImplementation(values => buildClaudeLocalConfig(values!)); + const { root, queryClient } = await openStep4({ adapterType: "claude_local" }); + try { + await act(async () => queryClient.setQueryData(["ai-connections", "company-new"], { + currentUserId: "user-1", connections: [{ id: "saved", grantId: "grant", companyId: "company-new", provider: "anthropic", method: "subscription", ownership: "personal", ownerUserId: "user-1", status: "connected", isDefault: true }], + })); + await pickSource(/Claude/); + await settle(); + expect(button("Connect").disabled).toBe(true); + await act(async () => button("Connect").dispatchEvent(new KeyboardEvent("keydown", { key: "Enter", metaKey: true, bubbles: true }))); + await settle(); + expect(mockAgentsApi.testEnvironment).not.toHaveBeenCalled(); + expect(mockAgentsApi.hire).not.toHaveBeenCalled(); + await act(async () => finish([{ type: "claude_local", defaultRunner, supportedRunners: ["paperclip", "legacy"] }])); + await settle(); + expect(mockAgentsApi.testEnvironment).toHaveBeenCalledTimes(1); + expect(mockAgentsApi.hire).toHaveBeenCalledTimes(1); + const tested = mockAgentsApi.testEnvironment.mock.calls[0] as unknown as [string, string, { adapterConfig: Record }]; + const hired = mockAgentsApi.hire.mock.calls[0] as unknown as [string, { adapterConfig: Record }]; + expect(hired[1].adapterConfig).toEqual(tested[2].adapterConfig); + if (defaultRunner === "paperclip") expect(hired[1].adapterConfig).not.toHaveProperty("maxTurnsPerRun"); + else expect(hired[1].adapterConfig.maxTurnsPerRun).toBe(1000); + expect(hired[1].adapterConfig).not.toHaveProperty("resolvedRunner"); + } finally { await act(async () => root.unmount()); } + }); + + it("does not hire from obsolete Claude discovery after the target changes during verification", async () => { + mockAdapterBuild.buildAdapterConfig.mockImplementation(values => buildClaudeLocalConfig(values!)); + mockAdaptersApi.list.mockImplementation(async ({ environmentId }: { environmentId: string | null }) => [{ type: "claude_local", defaultRunner: environmentId === "new-local-default" ? "paperclip" : "legacy" }]); + const finishes: Array<(value: typeof passed) => void> = []; + mockAgentsApi.testEnvironment.mockImplementation(() => new Promise(resolve => finishes.push(resolve))); + const { root, queryClient } = await openStep4({ adapterType: "claude_local" }); + try { + await pickSource(/Claude/); + await detected(); + expect(mockAgentsApi.testEnvironment).toHaveBeenCalledTimes(1); + await act(async () => queryClient.setQueryData(queryKeys.environments.list("company-new"), [{ ...LOCAL_ENVIRONMENT, id: "new-local-default" }])); + await settle(); + expect(mockAgentsApi.testEnvironment).toHaveBeenCalledTimes(2); + await act(async () => finishes[0](passed)); + await settle(); + expect(mockAgentsApi.hire).not.toHaveBeenCalled(); + await act(async () => finishes[1](passed)); + await settle(); + expect(mockAgentsApi.hire).toHaveBeenCalledTimes(1); + const tested = mockAgentsApi.testEnvironment.mock.calls as unknown as Array<[string, string, { environmentId: string; adapterConfig: Record }]>; + expect(tested[0][2].adapterConfig.maxTurnsPerRun).toBe(1000); + expect(tested[1][2].environmentId).toBe("new-local-default"); + expect(tested[1][2].adapterConfig).not.toHaveProperty("maxTurnsPerRun"); + const hired = mockAgentsApi.hire.mock.calls[0] as unknown as [string, { adapterConfig: Record }]; + expect(hired[1].adapterConfig).toEqual(tested[1][2].adapterConfig); + } finally { await act(async () => root.unmount()); } + }); + + it("fails closed on Claude discovery rejection and retries availability before verification", async () => { + mockAdaptersApi.list.mockRejectedValue(new Error("Discovery unavailable")); + const { root, queryClient } = await openStep4({ adapterType: "claude_local" }); + try { + await act(async () => queryClient.setQueryData(["ai-connections", "company-new"], { + currentUserId: "user-1", connections: [{ id: "saved", grantId: "grant", companyId: "company-new", provider: "anthropic", method: "subscription", ownership: "personal", ownerUserId: "user-1", status: "connected", isDefault: true }], + })); + await pickSource(/Claude/); + await settle(); + expect(mockAgentsApi.testEnvironment).not.toHaveBeenCalled(); + expect(mockAgentsApi.hire).not.toHaveBeenCalled(); + expect(document.body.textContent).toContain("Could not load runner availability"); + mockAdaptersApi.list.mockResolvedValue([{ type: "claude_local", defaultRunner: "paperclip" }]); + await click("Retry setup"); + expect(mockAgentsApi.testEnvironment).toHaveBeenCalledTimes(1); + expect(mockAgentsApi.hire).toHaveBeenCalledTimes(1); + } finally { await act(async () => root.unmount()); } + }); + it.each(providers)("automatically verifies detected %s once through connection refreshes and advances", async (adapterType, provider, label) => { let finish!: (result: typeof passed) => void; mockAgentsApi.testEnvironment.mockReturnValue(new Promise(resolve => { finish = resolve; })); @@ -3229,7 +3310,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( await detected(); expectTesting(); await act(async () => queryClient.setQueryData(queryKeys.environments.list("company-new"), [ - { ...LOCAL_ENVIRONMENT, id: "new-local-default" }, + { ...LOCAL_ENVIRONMENT, name: "Refreshed local environment" }, ])); await settle(); expectTesting(); diff --git a/ui/src/components/OnboardingWizard.tsx b/ui/src/components/OnboardingWizard.tsx index 3c17c6752c7..1400f20fe34 100644 --- a/ui/src/components/OnboardingWizard.tsx +++ b/ui/src/components/OnboardingWizard.tsx @@ -947,14 +947,14 @@ function OnboardingWizardInner({ // managed-sandbox-only redirect (see AgentConfigForm.tsx:618-640). A render // must not throw, so a resolver error yields no login environment rather // than an error boundary. - const { data: loginEnvironmentList = [] } = useQuery({ + const { data: loginEnvironmentList = [], isSuccess: loginEnvironmentsReady } = useQuery({ queryKey: createdCompanyId ? queryKeys.environments.list(createdCompanyId) : ["environments", "none"], queryFn: () => environmentsApi.list(createdCompanyId!), enabled: Boolean(createdCompanyId) && effectiveOnboardingOpen && step === 4, }); - const { data: instanceSettingsForLogin } = useQuery({ + const { data: instanceSettingsForLogin, isSuccess: loginSettingsReady } = useQuery({ queryKey: queryKeys.instance.settings, queryFn: () => instanceSettingsApi.get(), enabled: effectiveOnboardingOpen && step === 4, @@ -962,7 +962,7 @@ function OnboardingWizardInner({ // Wanted across the whole arc, not just the connect step. The progress strip // reads it too — see `enteredFromCloud` — and a value fetched only on step 4 // would let the strip change length as the customer walked through it. - const { data: experimentalSettingsForLogin } = useQuery({ + const { data: experimentalSettingsForLogin, isSuccess: loginPolicyReady } = useQuery({ queryKey: queryKeys.instance.experimentalSettings, queryFn: () => instanceSettingsApi.getExperimental(), enabled: effectiveOnboardingOpen && step >= 3 && step <= 5, @@ -988,8 +988,24 @@ function OnboardingWizardInner({ const runnerDiscovery = useQuery({ queryKey: queryKeys.adapters.availability(createdCompanyId, resolvedLoginEnvironmentId), queryFn: () => adaptersApi.list({ companyId: createdCompanyId!, environmentId: resolvedLoginEnvironmentId }), - enabled: Boolean(createdCompanyId) && effectiveOnboardingOpen && step === 4, + enabled: Boolean(createdCompanyId) && effectiveOnboardingOpen && step === 4 + && loginEnvironmentsReady && loginSettingsReady && loginPolicyReady, }); + const needsRunnerDiscovery = ["codex_local", "claude_local"].includes(adapterType) && runner === "auto"; + const resolvedRunner = runner === "auto" + ? runnerDiscovery.data?.find(item => item.type === adapterType)?.defaultRunner + : runner; + const runnerDiscoveryReady = !needsRunnerDiscovery + || (runnerDiscovery.isSuccess && !runnerDiscovery.isFetching && resolvedRunner !== undefined); + const runnerDiscoveryError = needsRunnerDiscovery && runnerDiscovery.isError + ? "Could not load runner availability. Retry setup." + : needsRunnerDiscovery && runnerDiscovery.isSuccess && resolvedRunner === undefined + ? "Runner availability is missing for this harness. Retry setup." + : null; + const visibleAdapterEnvError = adapterEnvError ?? runnerDiscoveryError; + const runnerSelectionKey = JSON.stringify([createdCompanyId, resolvedLoginEnvironmentId, adapterType, runner, resolvedRunner]); + const runnerSelectionRef = useRef(runnerSelectionKey); + runnerSelectionRef.current = runnerSelectionKey; const resolvedLoginEnvironment = useMemo( () => loginEnvironmentList.find((environment) => environment.id === resolvedLoginEnvironmentId) ?? @@ -1033,8 +1049,8 @@ function OnboardingWizardInner({ Boolean(managedProvider) && !savedSubscription && !savedKeys.storedLogin.data && !managedBindingForStep(), ); // A result from a previous selection must not hire or advance this wizard. - // Environment query updates are not user navigation: the test resolves its - // own environment, and those updates must not interrupt the pending attempt. + // Discovery starts after environment settings resolve. A later target or + // runner change invalidates the configuration an in-flight test was built from. useEffect(() => { autoConnectStartedRef.current = false; hiringAgentRef.current = null; @@ -1043,7 +1059,7 @@ function OnboardingWizardInner({ setAdapterEnvLoading(false); } return () => { connectAttemptRef.current++; }; - }, [effectiveOnboardingOpen, createdCompanyId, adapterType, credentialMode, step]); + }, [effectiveOnboardingOpen, createdCompanyId, adapterType, credentialMode, step, resolvedLoginEnvironmentId, runner, resolvedRunner]); const canShowAdapterLogin = Boolean( adapterCaps.login != null && @@ -1183,7 +1199,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 && !savedKeys.loading; + const connectStepReady = sourceSelected && !adapterEnvLoading && !savedKeys.loading && runnerDiscoveryReady; /** * Whether this step has a sign-in to do before it can hire. @@ -1326,7 +1342,7 @@ function OnboardingWizardInner({ // Not before the login is stored. "Connecting" starts at the paste now, // ahead of the server confirming anything, so a hire from here would go // out against a source with no credential to run on. - if (!connectCredentialStored) return; + if (!connectCredentialStored || !runnerDiscoveryReady) return; // No success state: the step advances. The hold is so "Connecting" is // legible as a state rather than a flicker on the way out — a step that // left the instant a paste landed would read as the paste having gone @@ -1365,7 +1381,7 @@ function OnboardingWizardInner({ return () => clearTimeout(t); } return; - }, [step, connectPhase, credentialMode, connectStepNeedsLogin, connectCredentialStored]); + }, [step, connectPhase, credentialMode, connectStepNeedsLogin, connectCredentialStored, runnerDiscoveryReady]); /** * The button's four faces, and which of them can be pressed. @@ -1832,7 +1848,7 @@ function OnboardingWizardInner({ ...defaultCreateValues, adapterType, runner, - resolvedRunner: runner === "auto" ? runnerDiscovery.data?.find(item => item.type === adapterType)?.defaultRunner : runner, + resolvedRunner, model: adapterType === "gemini_local" ? model || DEFAULT_GEMINI_LOCAL_MODEL @@ -1899,6 +1915,7 @@ function OnboardingWizardInner({ appliedStoredClaudeLoginBinding = false, isCurrent = () => true, ): Promise { + if (!runnerDiscoveryReady) return null; if (!createdCompanyId) { setAdapterEnvError( "Create or select an organization before testing adapter environment." @@ -2042,9 +2059,10 @@ function OnboardingWizardInner({ setStep(5); return; } - if (hiringAgentRef.current !== null) return; + if (!runnerDiscoveryReady || hiringAgentRef.current !== null) return; const attempt = ++connectAttemptRef.current; - const isCurrent = () => connectAttemptRef.current === attempt && stillTheSameCompany(createdCompanyId); + const isCurrent = () => connectAttemptRef.current === attempt && stillTheSameCompany(createdCompanyId) + && runnerSelectionRef.current === runnerSelectionKey; hiringAgentRef.current = attempt; autoConnectStartedRef.current = true; setLoading(true); @@ -2939,14 +2957,20 @@ function OnboardingWizardInner({ the hire, because `handleGiveHeartbeat` clears it in its `finally` after the early return — so a genuine block still shows its checks, which is the whole reason this is here. */} - {isLocalAdapter && !loading && (adapterEnvError || (adapterEnvResult && adapterEnvResult.status !== "pass")) && ( + {isLocalAdapter && !loading && (visibleAdapterEnvError || (adapterEnvResult && adapterEnvResult.status !== "pass")) && (
- {adapterEnvError && ( + {visibleAdapterEnvError && (
- {adapterEnvError} + {visibleAdapterEnvError}
)} + {runnerDiscoveryError && ( + + )} + {adapterEnvResult && adapterEnvResult.status === "pass" ? (