mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-02 16:58:52 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - First onboarding helps a new user connect the first agent. > - Its provider picker currently includes GitHub Copilot. > - The first setup should offer only Claude and Codex. > - This pull request limits that picker to those two providers. > - Users can still choose other providers when they add more agents. ## Linked Issues or Issue Description **What existing behavior does this improve?** The model-source picker in first-agent onboarding. **Subsystem affected** `ui/` — the onboarding wizard and its tests. **Current behavior** First onboarding offers GitHub Copilot. It can also expose other providers when their display metadata marks them as recommended. **Proposed behavior** Show only enabled Claude and Codex choices in first onboarding. Keep their automatic native-runner routing on qualified execution environments and the silent legacy fallback on native Windows, with the same provider UI. Keep all supported providers in the later Add Agent flow. Require a visible provider choice when an old draft names a provider that is no longer offered. **Reason and benefit** Keep the first setup focused while preserving provider choice for an existing company. **Breaking changes** Old Copilot or other provider drafts require a new choice in first onboarding. Existing agents, provider APIs, and the Add Agent flow keep their current behavior. **Additional context** Related provider setup work: #15562. This change only narrows the first-onboarding picker. Searches for onboarding/Copilot and first-onboarding PRs found no duplicate change. This is a small improvement to the existing setup flow, consistent with ROADMAP.md. ## What Changed - Limit first-onboarding adapters to Claude and Codex, with existing availability checks. - Remove the Copilot tile, login card, connection state, model discovery, and creation guards from first onboarding. - Retain only migration handling for old Copilot drafts: clear the retired selection/model/flag, restore automatic runner choice, and avoid reusing GitHub bindings when the user selects Claude or Codex. This also works when both onboarding providers are disabled. - Update coverage for provider choices, disabled adapters, and old Copilot and Gemini drafts. - Clear the model when an unavailable saved provider changes to Codex. - Explain how to proceed when both first-onboarding providers are unavailable. - Update the onboarding feature and access maps. - Document the Windows fallback beside automatic runner discovery, with an explicit note to default Windows to native when its runner support is qualified. This inline migration note is an explicit requirement of the change. - Extend both providers' setup coverage for native and legacy availability without forcing a runtime from the browser. ## Verification - 282 tests passed across all onboarding component/route tests and `NewAgentDialog.test.tsx`. This includes the separate adapter-draft suite that CI identified as still expecting Copilot in first onboarding. - The existing server runner-selection suite passed: 231 tests, including native Claude/Codex runtime translation and Windows legacy fallback. This is automated routing coverage; no native Windows end-to-end run is claimed. - UI TypeScript checks and the production UI build passed. - Token gates, feature/access map validation, module-boundary checks, and `git diff --check` passed. - Reviewer command from `ui/`: `pnpm exec vitest run src/components/OnboardingWizard.test.tsx src/components/OnboardingWizard.adapters.test.tsx src/components/OnboardingWizard.step.test.tsx src/components/OnboardingWizardVariant.test.tsx src/components/NewAgentDialog.test.tsx src/lib/onboarding-route.test.ts`. - A separate browser walkthrough on staging completed the Add Agent flow with Codex and confirmed Copilot remained available. That walkthrough used an existing deployment; it does not prove this branch's first-onboarding change. - Ponytail Paperclip Review and Greptile both scored the latest commit `b470f7023ecb6657bd77edefbcaf2ab12fd89414` 5/5. All review threads are resolved. - All latest-head PR checks are complete: 55 passed and 3 neutral/skipped, with no failures or pending checks. Two server shards passed on a single retry after an unchanged timing-sensitive test exceeded its limit and a self-hosted runner lost its connection. No code or test changes were made for the retry. ## Risks - Low scope: the provider filter belongs only to the onboarding wizard. Native Windows runner support is still unmerged (#18250 is only its build/CI baseline), so automatic selection must retain the existing legacy fallback. Platform selection is based on the execution target, not the browser OS. - Users with an old draft for another provider must choose Claude or Codex. Existing agent configuration is not removed. - Instances that disable both providers have no first-onboarding choice. The wizard explains how to enable a provider or use New Agent from the Agents page. ## Model Used OpenAI GPT-6 through Codex. The exact runtime model ID and context-window size are not exposed in this session. Capabilities used: reasoning, repository edits, shell commands, and browser UI verification. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
307 lines
10 KiB
TypeScript
307 lines
10 KiB
TypeScript
// @vitest-environment jsdom
|
|
|
|
import { act } from "react";
|
|
import { createRoot } from "react-dom/client";
|
|
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
|
|
// --- Mocks (hoisted so vi.mock factories can close over them) ----------------
|
|
|
|
const ONBOARDING_STORAGE_KEY = "paperclip-onboarding-state";
|
|
|
|
const mockDialog = vi.hoisted(() => ({
|
|
onboardingOpen: true,
|
|
onboardingOptions: {} as { initialStep?: number; companyId?: string },
|
|
closeOnboarding: vi.fn(),
|
|
onboardingRouteDismissed: false,
|
|
setOnboardingRouteDismissed: vi.fn(),
|
|
}));
|
|
|
|
const mockCompany = vi.hoisted(() => ({
|
|
companies: [] as Array<{ id: string; name: string; issuePrefix: string }>,
|
|
setSelectedCompanyId: vi.fn(),
|
|
loading: false,
|
|
}));
|
|
|
|
// The real adapter registry eagerly imports every adapter package. The
|
|
// model/harness picker internals are out of scope here, so stub the adapter
|
|
// layer entirely and drive the grid through these two knobs.
|
|
const mockAdapterRegistry = vi.hoisted(() => ({
|
|
list: [] as Array<{ type: string }>,
|
|
disabled: new Set<string>(),
|
|
loaded: true,
|
|
}));
|
|
|
|
vi.mock("@/lib/router", () => ({
|
|
useLocation: () => ({ pathname: "/", search: "", hash: "", state: null }),
|
|
useNavigate: () => vi.fn(),
|
|
useParams: () => ({}),
|
|
}));
|
|
vi.mock("../context/DialogContext", () => ({
|
|
useDialog: () => mockDialog,
|
|
}));
|
|
vi.mock("../context/CompanyContext", () => ({
|
|
useCompany: () => mockCompany,
|
|
}));
|
|
// The restore gate fetches the company list itself to verify draft ownership,
|
|
// rather than trusting the shared cache, so this module now needs stubbing
|
|
// here. An empty list is fine: the drafts in this file carry no
|
|
// `createdCompanyId`, so there is no ownership question — but the fetch has to
|
|
// succeed for the gate to treat the draft as decidable at all.
|
|
// The company list is keyed by account, so it holds until the session query
|
|
// *succeeds*. A seeded entry is stale under the test client and refetches, so
|
|
// the refetch has to answer too — otherwise the identity errors and the list
|
|
// never runs.
|
|
const mockAuthApi = vi.hoisted(() => ({ getSession: vi.fn() }));
|
|
vi.mock("../api/auth", async (importOriginal) => {
|
|
const actual = await importOriginal<typeof import("../api/auth")>();
|
|
return { ...actual, authApi: { ...actual.authApi, getSession: mockAuthApi.getSession } };
|
|
});
|
|
|
|
vi.mock("../api/companies", () => ({
|
|
companiesApi: {
|
|
create: vi.fn(),
|
|
list: vi.fn().mockResolvedValue([]),
|
|
detachInflightList: vi.fn(),
|
|
},
|
|
}));
|
|
vi.mock("../adapters", () => ({
|
|
listUIAdapters: () => mockAdapterRegistry.list,
|
|
getUIAdapter: () => ({ buildAdapterConfig: () => ({}) }),
|
|
}));
|
|
vi.mock("../adapters/metadata", () => ({ isVisualAdapterChoice: () => true }));
|
|
vi.mock("../adapters/adapter-display-registry", () => ({
|
|
getAdapterDisplay: (type: string) => ({
|
|
type,
|
|
recommended: false,
|
|
label: type,
|
|
description: "",
|
|
icon: () => null,
|
|
}),
|
|
getAdapterLabel: (type: string) => type,
|
|
getAdapterLabels: () => ({}) as Record<string, string>,
|
|
isKnownAdapterType: () => true,
|
|
}));
|
|
vi.mock("../adapters/use-disabled-adapters", () => ({
|
|
useDisabledAdaptersSync: () => mockAdapterRegistry.disabled,
|
|
useAdapterRegistryLoaded: () => mockAdapterRegistry.loaded,
|
|
}));
|
|
vi.mock("../adapters/use-adapter-capabilities", () => ({
|
|
useAdapterCapabilities: () => () => ({
|
|
supportsInstructionsBundle: false,
|
|
supportsSkills: false,
|
|
supportsLocalAgentJwt: false,
|
|
requiresMaterializedRuntimeSkills: false,
|
|
}),
|
|
}));
|
|
// Animation / canvas-ish children that add nothing to the logic under test.
|
|
vi.mock("./AsciiArtAnimation", () => ({ AsciiArtAnimation: () => null }));
|
|
vi.mock("./AgentCapsule", () => ({ AgentCapsule: () => null }));
|
|
|
|
import { queryKeys } from "../lib/queryKeys";
|
|
import { OnboardingWizard } from "./OnboardingWizard";
|
|
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
|
|
|
|
async function flushReact() {
|
|
await act(async () => {
|
|
await Promise.resolve();
|
|
await new Promise((resolve) => window.setTimeout(resolve, 0));
|
|
});
|
|
}
|
|
|
|
async function mount(nativeRunnerEnabled?: boolean) {
|
|
const container = document.createElement("div");
|
|
document.body.appendChild(container);
|
|
const root = createRoot(container);
|
|
const queryClient = new QueryClient({
|
|
defaultOptions: { queries: { retry: false } },
|
|
});
|
|
// The company list is keyed by account, so it holds until the session is
|
|
// known. Seeding it is how this test says "signed in".
|
|
queryClient.setQueryData(queryKeys.auth.session, {
|
|
session: { id: "session-1", userId: "user-1" },
|
|
user: { id: "user-1", name: "Example", email: "user-1@example.com", image: null },
|
|
});
|
|
if (nativeRunnerEnabled !== undefined) queryClient.setQueryData(queryKeys.instance.experimentalSettings, { enableNativeRunner: nativeRunnerEnabled, enableOpenAiDot: true });
|
|
await act(async () => {
|
|
root.render(
|
|
<QueryClientProvider client={queryClient}>
|
|
<OnboardingWizard />
|
|
</QueryClientProvider>,
|
|
);
|
|
});
|
|
await flushReact();
|
|
return { container, root };
|
|
}
|
|
|
|
describe("OnboardingWizard adapter selection", () => {
|
|
beforeEach(() => {
|
|
mockAuthApi.getSession.mockResolvedValue({
|
|
session: { id: "session-1", userId: "user-1" },
|
|
user: { id: "user-1", name: "Example", email: "user-1@example.com", image: null },
|
|
});
|
|
window.localStorage.clear();
|
|
mockDialog.onboardingOpen = true;
|
|
mockDialog.onboardingOptions = {};
|
|
mockCompany.companies = [];
|
|
mockAdapterRegistry.list = [];
|
|
mockAdapterRegistry.disabled = new Set<string>();
|
|
mockAdapterRegistry.loaded = true;
|
|
});
|
|
|
|
afterEach(() => {
|
|
document.body.innerHTML = "";
|
|
vi.clearAllMocks();
|
|
});
|
|
|
|
it("snaps a disabled default adapterType to the first enabled adapter", async () => {
|
|
// A deployment whose adapter registry omits claude_local disables it, so
|
|
// the wizard's claude_local default must not survive as an invisible
|
|
// selection (the created agent could never acquire a lease).
|
|
mockAdapterRegistry.list = [
|
|
{ type: "claude_local" },
|
|
{ type: "codex_local" },
|
|
{ type: "opencode_local" },
|
|
];
|
|
mockAdapterRegistry.disabled = new Set(["claude_local"]);
|
|
|
|
const { root } = await mount();
|
|
|
|
const saved = JSON.parse(
|
|
window.localStorage.getItem(ONBOARDING_STORAGE_KEY) ?? "{}",
|
|
);
|
|
expect(saved.adapterType).toBe("codex_local");
|
|
|
|
await act(async () => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("keeps an enabled saved adapterType untouched", async () => {
|
|
mockAdapterRegistry.list = [
|
|
{ type: "claude_local" },
|
|
{ type: "codex_local" },
|
|
];
|
|
window.localStorage.setItem(
|
|
ONBOARDING_STORAGE_KEY,
|
|
JSON.stringify({ step: 0, adapterType: "claude_local" }),
|
|
);
|
|
|
|
const { root } = await mount();
|
|
|
|
const saved = JSON.parse(
|
|
window.localStorage.getItem(ONBOARDING_STORAGE_KEY) ?? "{}",
|
|
);
|
|
expect(saved.adapterType).toBe("claude_local");
|
|
|
|
await act(async () => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("recovers a saved Codex runner draft without changing its model or runner", async () => {
|
|
mockAdapterRegistry.list = [
|
|
{ type: "paperclip_runner" },
|
|
{ type: "codex_local" },
|
|
];
|
|
window.localStorage.setItem(
|
|
ONBOARDING_STORAGE_KEY,
|
|
JSON.stringify({
|
|
step: 0,
|
|
adapterType: "paperclip_runner",
|
|
model: "gpt-runner-only",
|
|
command: "runnerd",
|
|
args: "--native",
|
|
url: "ws://runner",
|
|
}),
|
|
);
|
|
|
|
const { root } = await mount();
|
|
|
|
const saved = JSON.parse(
|
|
window.localStorage.getItem(ONBOARDING_STORAGE_KEY) ?? "{}",
|
|
);
|
|
expect(saved.adapterType).toBe("codex_local");
|
|
expect(saved.runner).toBe("paperclip");
|
|
expect(saved.model).toBe("gpt-runner-only");
|
|
expect(saved.command).toBe("");
|
|
expect(saved.args).toBe("");
|
|
expect(saved.url).toBe("");
|
|
|
|
await act(async () => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("normalizes a saved Paperclip Runner draft before adapter discovery resolves", async () => {
|
|
mockAdapterRegistry.loaded = false;
|
|
mockAdapterRegistry.list = [{ type: "codex_local" }];
|
|
window.localStorage.setItem(
|
|
ONBOARDING_STORAGE_KEY,
|
|
JSON.stringify({
|
|
step: 0,
|
|
adapterType: "paperclip_runner",
|
|
model: "gpt-runner-only",
|
|
command: "runnerd",
|
|
args: "--native",
|
|
url: "ws://runner",
|
|
}),
|
|
);
|
|
|
|
const { root } = await mount();
|
|
|
|
const saved = JSON.parse(
|
|
window.localStorage.getItem(ONBOARDING_STORAGE_KEY) ?? "{}",
|
|
);
|
|
expect(saved.adapterType).toBe("codex_local");
|
|
expect(saved.runner).toBe("paperclip");
|
|
expect(saved.model).toBe("gpt-runner-only");
|
|
expect(saved.command).toBe("");
|
|
expect(saved.args).toBe("");
|
|
expect(saved.url).toBe("");
|
|
|
|
await act(async () => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("does not replace a saved adapter before the registry has loaded", async () => {
|
|
// External adapter types are only registered once the adapters query
|
|
// resolves. Until then `listUIAdapters()` returns the built-ins alone, so
|
|
// a saved external adapter looks exactly like a disabled one — and
|
|
// snapping would swap the customer's choice for a built-in and persist it.
|
|
window.localStorage.setItem(
|
|
ONBOARDING_STORAGE_KEY,
|
|
JSON.stringify({ step: 0, adapterType: "acme_external" }),
|
|
);
|
|
mockAdapterRegistry.loaded = false;
|
|
mockAdapterRegistry.list = [{ type: "codex_local" }];
|
|
|
|
const { root } = await mount();
|
|
|
|
const saved = JSON.parse(
|
|
window.localStorage.getItem(ONBOARDING_STORAGE_KEY) ?? "{}",
|
|
);
|
|
expect(saved.adapterType).toBe("acme_external");
|
|
|
|
await act(async () => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it.each([false, true])("normalizes a saved Copilot draft with the native runner experiment set to %s", async (nativeRunnerEnabled) => {
|
|
mockAdapterRegistry.list = [{ type: "paperclip_runner" }, { type: "codex_local" }];
|
|
window.localStorage.setItem(ONBOARDING_STORAGE_KEY, JSON.stringify({ step: 0, adapterType: "paperclip_runner", onboardingCopilot: true, model: "gpt-5.6-luna" }));
|
|
const { root } = await mount(nativeRunnerEnabled);
|
|
try {
|
|
const saved = JSON.parse(window.localStorage.getItem(ONBOARDING_STORAGE_KEY)!);
|
|
expect(saved).toMatchObject({ adapterType: "codex_local", runner: "auto", model: "" });
|
|
expect(saved).not.toHaveProperty("onboardingCopilot");
|
|
} finally {
|
|
await act(async () => root.unmount());
|
|
}
|
|
});
|
|
});
|