mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-23 17:33:51 +02:00
fix(ui): stop coercing imported agents to the destination CEO adapter (#11192)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Company import lets an operator bring a package of agents into an instance, and each agent declares which adapter runs it (Claude Code, Codex, and so on) > - The export and the server importer preserve each agent's adapter faithfully, but the Import page seeds an adapter override for every agent with the destination CEO's adapter before the user touches anything > - Every imported agent therefore arrives as the CEO's adapter (usually Claude Code) even when the source package holds a mix, and the picker shows the coerced value as if it were the source's, so nothing looks wrong > - This pull request makes the manifest adapter the default, sends overrides only for agents the user actually changed, and replaces the silent coercion with an explicit per-agent fallback warning when the destination truly lacks the source adapter > - The benefit is that a mixed Claude/Codex team imports as a mixed Claude/Codex team, and any real adapter gap is visible instead of silent ## Linked Issues or Issue Description **What happened?** A user imported a company package whose agents were a mix of Claude Code and Codex on the source instance. After the import, every agent was configured as Claude Code. The import preview showed no sign that anything had been changed. Cause: the Import page initializes its adapter-override map by assigning every agent the destination CEO's adapter type and sends that override for every agent, overriding the manifest's per-agent adapter server-side. For imports into a new company, the "CEO adapter" is read from whichever unrelated company is currently selected. **Expected behavior** Imported agents keep the adapter declared in the package. An override is sent only when the operator explicitly picks a different adapter, or when the source adapter is not installed on the destination — and in that case the page must say so per agent, not silently substitute. **Steps to reproduce** 1. On a source instance, create a company with one Claude Code agent and one Codex agent, and export it. 2. Import the package on another instance whose CEO uses Claude Code, changing nothing in the import dialog. 3. Both agents arrive configured as Claude Code; the Codex identity is gone. ## What Changed - The preview no longer seeds adapter overrides; the override map starts empty, and the picker displays each agent's manifest adapter (`ui/src/pages/CompanyImport.tsx`). - `buildFinalAdapterOverrides` sends an entry only when the effective adapter differs from the manifest or the agent's adapter config was edited — untouched agents flow through with no override. - The page fetches the destination's installed adapters (existing `adaptersApi.list()` client). When a manifest adapter is missing or disabled on the destination, only that agent defaults to the CEO's adapter, with a visible amber warning naming both adapters. If the adapters request fails, the page fails open: manifest adapters are kept and no coercion happens. - Tests: untouched mixed-adapter import sends no overrides; a user-changed agent sends exactly one; a missing destination adapter produces the fallback plus rendered warning for that agent only; an adapters-endpoint failure produces no coercion. ## Verification - `npx vitest run ui/src/pages/CompanyImport.test.tsx` — 19 passed (15 pre-existing + 4 new). - `pnpm --filter ./ui typecheck` (`tsc -b`) — clean. ## Risks - Behavior change: users who previously relied on the silent conversion (importing packages that reference adapters they don't have) now get an explicit per-agent fallback with a warning — same outcome, visible. The server's hard rejection of unknown adapter types remains the backstop for API callers. - UI-only change; no server or schema impact. ## Model Used - Claude Fable 5 (`claude-fable-5`) via Claude Code CLI, with extended thinking and tool use (multi-agent implementation with independent 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
Devin Foley authored and GitHub committed
1 parent
d816eb8095
commit
4c062a0eb2
2 files changed
+222
-13
No files matched your search
@@ -22,6 +22,9 @@ const mockAgentsApi = vi.hoisted(() => ({
|
||||
list: vi.fn(),
|
||||
resume: vi.fn(),
|
||||
}));
|
||||
const mockAdaptersApi = vi.hoisted(() => ({
|
||||
list: vi.fn(),
|
||||
}));
|
||||
const mockRoutinesApi = vi.hoisted(() => ({
|
||||
update: vi.fn(),
|
||||
}));
|
||||
@@ -54,6 +57,10 @@ vi.mock("../api/agents", () => ({
|
||||
agentsApi: mockAgentsApi,
|
||||
}));
|
||||
|
||||
vi.mock("../api/adapters", () => ({
|
||||
adaptersApi: mockAdaptersApi,
|
||||
}));
|
||||
|
||||
vi.mock("../api/routines", () => ({
|
||||
routinesApi: mockRoutinesApi,
|
||||
}));
|
||||
@@ -156,6 +163,44 @@ function buildPreviewResult(): CompanyPortabilityPreviewResult {
|
||||
} as unknown as CompanyPortabilityPreviewResult;
|
||||
}
|
||||
|
||||
/** Preview whose manifest mixes adapters: one claude_local agent and one codex_local agent. */
|
||||
function buildMixedAdapterPreviewResult(): CompanyPortabilityPreviewResult {
|
||||
return {
|
||||
include: { company: true, agents: true, projects: true, issues: true },
|
||||
targetCompanyId: null,
|
||||
targetCompanyName: null,
|
||||
collisionStrategy: "rename",
|
||||
selectedAgentSlugs: ["coder", "researcher"],
|
||||
plan: {
|
||||
companyAction: "create",
|
||||
agentPlans: [
|
||||
{ slug: "coder", action: "create", plannedName: "Coder", existingAgentId: null, reason: null },
|
||||
{ slug: "researcher", action: "create", plannedName: "Researcher", existingAgentId: null, reason: null },
|
||||
],
|
||||
projectPlans: [],
|
||||
issuePlans: [],
|
||||
},
|
||||
manifest: {
|
||||
agents: [
|
||||
{ slug: "coder", name: "Coder", path: "agents/coder/AGENTS.md", adapterType: "claude_local" },
|
||||
{ slug: "researcher", name: "Researcher", path: "agents/researcher/AGENTS.md", adapterType: "codex_local" },
|
||||
],
|
||||
projects: [],
|
||||
issues: [],
|
||||
skills: [],
|
||||
company: null,
|
||||
},
|
||||
files: {
|
||||
".paperclip.yaml": 'schema: "paperclip/v1"\n',
|
||||
"agents/coder/AGENTS.md": "---\nname: Coder\n---\n\nYou write code.\n",
|
||||
"agents/researcher/AGENTS.md": "---\nname: Researcher\n---\n\nYou research.\n",
|
||||
},
|
||||
envInputs: [],
|
||||
warnings: [],
|
||||
errors: [],
|
||||
} as unknown as CompanyPortabilityPreviewResult;
|
||||
}
|
||||
|
||||
function buildImportResult(): CompanyPortabilityImportResult {
|
||||
return {
|
||||
company: { id: "company-2", name: "Imported Test", action: "created" },
|
||||
@@ -186,6 +231,10 @@ describe("CompanyImport", () => {
|
||||
document.body.appendChild(container);
|
||||
mockAuthApi.getSession.mockResolvedValue({ user: { id: "user-1" } });
|
||||
mockAgentsApi.list.mockResolvedValue([]);
|
||||
mockAdaptersApi.list.mockResolvedValue([
|
||||
{ type: "claude_local", disabled: false },
|
||||
{ type: "codex_local", disabled: false },
|
||||
]);
|
||||
mockAgentsApi.resume.mockResolvedValue({ id: "agent-1", status: "idle" });
|
||||
mockRoutinesApi.update.mockResolvedValue({ id: "routine-1", status: "active" });
|
||||
mockCompaniesApi.importPreview.mockResolvedValue(buildPreviewResult());
|
||||
@@ -775,4 +824,111 @@ describe("CompanyImport", () => {
|
||||
// The stored entry is cleared once the job settles.
|
||||
expect(sessionStorage.getItem("paperclip:company-import-job:company-1:acme/starter")).toBeNull();
|
||||
});
|
||||
|
||||
/** Adapter selects in the picker list, in manifest order (excludes the target/collision selects). */
|
||||
function findAdapterSelects() {
|
||||
return Array.from(container.querySelectorAll("select"))
|
||||
.filter((select) => select.value !== "new" && select.value !== "existing" && select.value !== "rename");
|
||||
}
|
||||
|
||||
async function previewMixedAdapterPackage() {
|
||||
mockCompaniesApi.importPreview.mockResolvedValue(buildMixedAdapterPreviewResult());
|
||||
await renderPage();
|
||||
await enterGithubUrl();
|
||||
await clickButton((text) => text === "Preview import");
|
||||
}
|
||||
|
||||
function lastImportMeta() {
|
||||
const call = mockCompaniesApi.importBundleAsync.mock.calls.at(-1);
|
||||
expect(call).toBeTruthy();
|
||||
return call![0] as { adapterOverrides?: Record<string, { adapterType: string }> };
|
||||
}
|
||||
|
||||
it("keeps manifest adapters and sends no overrides when the user touches nothing", async () => {
|
||||
await previewMixedAdapterPackage();
|
||||
|
||||
// The picker shows each agent's source adapter, not a coerced CEO default.
|
||||
expect(findAdapterSelects().map((select) => select.value)).toEqual(["claude_local", "codex_local"]);
|
||||
|
||||
await clickButton((text) => text.startsWith("Import 3 file"));
|
||||
await settle();
|
||||
|
||||
// Untouched agents flow through with no override at all, so the server
|
||||
// applies each agent's manifest adapterType.
|
||||
expect(lastImportMeta().adapterOverrides).toBeUndefined();
|
||||
});
|
||||
|
||||
it("sends an override only for the agent whose adapter the user changed", async () => {
|
||||
await previewMixedAdapterPackage();
|
||||
|
||||
const coderSelect = findAdapterSelects()[0];
|
||||
expect(coderSelect?.value).toBe("claude_local");
|
||||
await act(async () => {
|
||||
const setter = Object.getOwnPropertyDescriptor(HTMLSelectElement.prototype, "value")!.set!;
|
||||
setter.call(coderSelect!, "codex_local");
|
||||
coderSelect!.dispatchEvent(new Event("change", { bubbles: true }));
|
||||
});
|
||||
await flushReact();
|
||||
|
||||
await clickButton((text) => text.startsWith("Import 3 file"));
|
||||
await settle();
|
||||
|
||||
expect(lastImportMeta().adapterOverrides).toEqual({
|
||||
coder: { adapterType: "codex_local" },
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to the CEO adapter with a visible warning when a manifest adapter is not installed", async () => {
|
||||
// The destination has no codex_local adapter; the CEO fallback (an empty
|
||||
// agent list defaults to claude_local) takes over — never silently.
|
||||
mockAdaptersApi.list.mockResolvedValue([{ type: "claude_local", disabled: false }]);
|
||||
await previewMixedAdapterPackage();
|
||||
|
||||
expect(container.textContent).toContain("source adapter codex_local is not installed here");
|
||||
expect(container.textContent).toContain("will use Claude Code");
|
||||
expect(findAdapterSelects().map((select) => select.value)).toEqual(["claude_local", "claude_local"]);
|
||||
|
||||
await clickButton((text) => text.startsWith("Import 3 file"));
|
||||
await settle();
|
||||
|
||||
// Only the unavailable agent is overridden; the claude_local agent still
|
||||
// carries no override and keeps its manifest adapter.
|
||||
expect(lastImportMeta().adapterOverrides).toEqual({
|
||||
researcher: { adapterType: "claude_local" },
|
||||
});
|
||||
});
|
||||
|
||||
it("picks an installed adapter as the fallback when the CEO adapter is itself unavailable", async () => {
|
||||
// Neither codex_local nor the CEO fallback (claude_local) is installed;
|
||||
// the fallback must be an adapter that actually exists on the
|
||||
// destination, never an unavailable type the server would reject.
|
||||
mockAdaptersApi.list.mockResolvedValue([{ type: "gemini_local", disabled: false }]);
|
||||
await previewMixedAdapterPackage();
|
||||
|
||||
expect(container.textContent).toContain("source adapter codex_local is not installed here");
|
||||
expect(findAdapterSelects().map((select) => select.value)).toEqual(["gemini_local", "gemini_local"]);
|
||||
|
||||
await clickButton((text) => text.startsWith("Import 3 file"));
|
||||
await settle();
|
||||
|
||||
expect(lastImportMeta().adapterOverrides).toEqual({
|
||||
researcher: { adapterType: "gemini_local" },
|
||||
coder: { adapterType: "gemini_local" },
|
||||
});
|
||||
});
|
||||
|
||||
it("fails open to manifest adapters when the adapter list cannot be read", async () => {
|
||||
// Unknown availability must never coerce: with no adapter list, every
|
||||
// agent keeps its manifest adapter and no fallback warning renders.
|
||||
mockAdaptersApi.list.mockRejectedValue(new ApiError("adapters unavailable", 500, null));
|
||||
await previewMixedAdapterPackage();
|
||||
|
||||
expect(container.textContent).not.toContain("is not installed here");
|
||||
expect(findAdapterSelects().map((select) => select.value)).toEqual(["claude_local", "codex_local"]);
|
||||
|
||||
await clickButton((text) => text.startsWith("Import 3 file"));
|
||||
await settle();
|
||||
|
||||
expect(lastImportMeta().adapterOverrides).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -14,6 +14,7 @@ import { useToastActions } from "../context/ToastContext";
|
||||
import { authApi } from "../api/auth";
|
||||
import { ApiError } from "../api/client";
|
||||
import { companiesApi, type CompanyImportJobAccepted } from "../api/companies";
|
||||
import { adaptersApi } from "../api/adapters";
|
||||
import { agentsApi } from "../api/agents";
|
||||
import { routinesApi } from "../api/routines";
|
||||
import { sidebarPreferencesApi } from "../api/sidebarPreferences";
|
||||
@@ -559,7 +560,15 @@ const IMPORT_ADAPTER_OPTIONS: { value: string; label: string }[] = listUIAdapter
|
||||
interface AdapterPickerItem {
|
||||
slug: string;
|
||||
name: string;
|
||||
/** Adapter type from the package manifest (the source's adapter). */
|
||||
adapterType: string;
|
||||
/**
|
||||
* Set when the manifest adapter is not installed on the destination: the
|
||||
* adapter type the agent falls back to unless the user picks another one.
|
||||
* Null when the manifest adapter is usable here (or availability is unknown,
|
||||
* which fails open to the manifest adapter).
|
||||
*/
|
||||
fallbackAdapterType: string | null;
|
||||
}
|
||||
|
||||
function AdapterPickerList({
|
||||
@@ -592,7 +601,8 @@ function AdapterPickerList({
|
||||
</div>
|
||||
<div className="divide-y divide-border">
|
||||
{agents.map((agent) => {
|
||||
const selectedType = adapterOverrides[agent.slug] ?? agent.adapterType;
|
||||
const selectedType =
|
||||
adapterOverrides[agent.slug] ?? agent.fallbackAdapterType ?? agent.adapterType;
|
||||
const isExpanded = expandedSlugs.has(agent.slug);
|
||||
const vals = configValues[agent.slug] ?? { ...defaultCreateValues, adapterType: selectedType };
|
||||
|
||||
@@ -634,6 +644,14 @@ function AdapterPickerList({
|
||||
configure adapter
|
||||
</button>
|
||||
</div>
|
||||
{agent.fallbackAdapterType && (
|
||||
<div className="mx-4 mb-2.5 rounded-md border border-amber-500/30 bg-amber-500/5 px-3 py-2">
|
||||
<p className="text-xs text-amber-500">
|
||||
source adapter {agent.adapterType} is not installed here — this agent
|
||||
will use {adapterLabels[selectedType] ?? getAdapterLabel(selectedType)}
|
||||
</p>
|
||||
</div>
|
||||
)}
|
||||
{isExpanded && (
|
||||
<div className="border-t border-border bg-accent/10 px-4 py-3 space-y-3">
|
||||
<AgentConfigForm
|
||||
@@ -873,6 +891,22 @@ export function CompanyImport() {
|
||||
return ceo?.adapterType ?? "claude_local";
|
||||
}, [companyAgents]);
|
||||
|
||||
// Fetch the destination's installed adapters so imported agents keep their
|
||||
// manifest adapter whenever it is usable here. Only agents whose manifest
|
||||
// adapter is missing (or disabled) fall back to the CEO's adapter — with a
|
||||
// visible per-agent warning, never silently.
|
||||
const { data: installedAdapters } = useQuery({
|
||||
queryKey: queryKeys.adapters.all,
|
||||
queryFn: () => adaptersApi.list(),
|
||||
staleTime: 5 * 60 * 1000,
|
||||
});
|
||||
// Null while the list is loading or unreadable: availability is unknown, so
|
||||
// fail open and trust the manifest rather than coercing every agent.
|
||||
const availableAdapterTypes = useMemo(() => {
|
||||
if (!installedAdapters) return null;
|
||||
return new Set(installedAdapters.filter((a) => !a.disabled).map((a) => a.type));
|
||||
}, [installedAdapters]);
|
||||
|
||||
const localZipHelpText =
|
||||
"Upload a .zip exported directly from Paperclip. Re-zipped archives created by Finder, Explorer, or other zip tools may not import correctly.";
|
||||
|
||||
@@ -952,12 +986,10 @@ export function CompanyImport() {
|
||||
setSkippedSlugs(new Set());
|
||||
setConfirmedSlugs(new Set());
|
||||
|
||||
// Initialize adapter overrides — default all agents to the CEO's adapter type
|
||||
const defaultAdapters: Record<string, string> = {};
|
||||
for (const agent of result.manifest.agents) {
|
||||
defaultAdapters[agent.slug] = ceoAdapterType;
|
||||
}
|
||||
setAdapterOverrides(defaultAdapters);
|
||||
// Adapter overrides start empty: each agent keeps its manifest adapter
|
||||
// unless the user changes it, or the manifest adapter is not installed
|
||||
// here (handled per-agent via a warned fallback, never seeded silently).
|
||||
setAdapterOverrides({});
|
||||
setAdapterExpandedSlugs(new Set());
|
||||
setAdapterConfigValues({});
|
||||
|
||||
@@ -1346,9 +1378,11 @@ export function CompanyImport() {
|
||||
|
||||
function handleAdapterConfigChange(slug: string, patch: Partial<CreateConfigValues>) {
|
||||
resetMutationState();
|
||||
const agent = adapterAgents.find((a) => a.slug === slug);
|
||||
const currentType = agent ? effectiveAdapterType(agent) : adapterOverrides[slug] ?? "claude_local";
|
||||
setAdapterConfigValues((prev) => ({
|
||||
...prev,
|
||||
[slug]: { ...(prev[slug] ?? { ...defaultCreateValues, adapterType: adapterOverrides[slug] ?? "claude_local" }), ...patch },
|
||||
[slug]: { ...(prev[slug] ?? { ...defaultCreateValues, adapterType: currentType }), ...patch },
|
||||
}));
|
||||
}
|
||||
|
||||
@@ -1392,23 +1426,42 @@ export function CompanyImport() {
|
||||
}
|
||||
}
|
||||
|
||||
// Build the list of agents for adapter picking
|
||||
// Build the list of agents for adapter picking. An agent whose manifest
|
||||
// adapter is not installed on the destination gets a warned fallback to the
|
||||
// CEO's adapter; while availability is unknown the manifest adapter stands.
|
||||
const adapterAgents = useMemo<AdapterPickerItem[]>(() => {
|
||||
if (!importPreview) return [];
|
||||
return importPreview.manifest.agents.map((a) => ({
|
||||
slug: a.slug,
|
||||
name: a.name,
|
||||
adapterType: a.adapterType,
|
||||
// The fallback must itself be installed: the CEO's adapter when it is,
|
||||
// else any installed adapter, else null so the manifest adapter stands
|
||||
// and the server's unknown-adapter rejection is the backstop.
|
||||
fallbackAdapterType:
|
||||
availableAdapterTypes && !availableAdapterTypes.has(a.adapterType)
|
||||
? availableAdapterTypes.has(ceoAdapterType)
|
||||
? ceoAdapterType
|
||||
: [...availableAdapterTypes][0] ?? null
|
||||
: null,
|
||||
}));
|
||||
}, [importPreview]);
|
||||
}, [importPreview, availableAdapterTypes, ceoAdapterType]);
|
||||
|
||||
// Build final adapterOverrides for import request
|
||||
/** The adapter type an imported agent will actually use: an explicit user pick, else the availability fallback, else the manifest adapter. */
|
||||
function effectiveAdapterType(agent: AdapterPickerItem): string {
|
||||
return adapterOverrides[agent.slug] ?? agent.fallbackAdapterType ?? agent.adapterType;
|
||||
}
|
||||
|
||||
// Build final adapterOverrides for import request. Only agents that diverge
|
||||
// from the manifest adapter (a user pick or an availability fallback) or
|
||||
// carry edited adapter config send an override — untouched agents flow
|
||||
// through with none, so the manifest adapter survives the import.
|
||||
function buildFinalAdapterOverrides(): Record<string, CompanyPortabilityAdapterOverride> | undefined {
|
||||
if (adapterAgents.length === 0) return undefined;
|
||||
const overrides: Record<string, CompanyPortabilityAdapterOverride> = {};
|
||||
for (const agent of adapterAgents) {
|
||||
const selectedType = adapterOverrides[agent.slug] ?? agent.adapterType;
|
||||
const selectedType = effectiveAdapterType(agent);
|
||||
const configVals = adapterConfigValues[agent.slug];
|
||||
if (selectedType === agent.adapterType && !configVals) continue;
|
||||
const override: CompanyPortabilityAdapterOverride = { adapterType: selectedType };
|
||||
if (configVals) {
|
||||
const uiAdapter = getUIAdapter(selectedType);
|
||||
|
||||
Reference in new issue
Block a user