mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 19:28:46 +01:00
fix(claude): await matching runner discovery during onboarding
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
79779c5a96
commit
d663c7f98f
3 files changed
+153
-18
No files matched your search
@@ -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.
|
||||
@@ -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<string, unknown>),
|
||||
buildAdapterConfig: vi.fn((_values?: CreateConfigValues) => ({}) as Record<string, unknown>),
|
||||
}));
|
||||
// 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<string>();
|
||||
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<object[]>(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<string, unknown> }];
|
||||
const hired = mockAgentsApi.hire.mock.calls[0] as unknown as [string, { adapterConfig: Record<string, unknown> }];
|
||||
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<string, unknown> }]>;
|
||||
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<string, unknown> }];
|
||||
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();
|
||||
|
||||
@@ -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<AdapterEnvironmentTestResult | null> {
|
||||
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")) && (
|
||||
<div className="space-y-2 rounded-md border border-border p-3">
|
||||
{adapterEnvError && (
|
||||
{visibleAdapterEnvError && (
|
||||
<div className="rounded-md border border-destructive/30 bg-destructive/10 px-2.5 py-2 text-(length:--text-micro) text-destructive">
|
||||
{adapterEnvError}
|
||||
{visibleAdapterEnvError}
|
||||
</div>
|
||||
)}
|
||||
|
||||
{runnerDiscoveryError && (
|
||||
<Button type="button" variant="outline" disabled={runnerDiscovery.isFetching} onClick={() => void runnerDiscovery.refetch()}>
|
||||
Retry setup
|
||||
</Button>
|
||||
)}
|
||||
|
||||
{adapterEnvResult &&
|
||||
adapterEnvResult.status === "pass" ? (
|
||||
<div className="space-y-2 animate-in fade-in slide-in-from-bottom-1 duration-300">
|
||||
|
||||
Reference in new issue
Block a user