From eb2cb916be3271e3e7ab5f643ad3ca3eb7c34d01 Mon Sep 17 00:00:00 2001
From: fengqing <139055027+fengwuqingchen@users.noreply.github.com>
Date: Sun, 5 Jul 2026 06:25:01 +0800
Subject: [PATCH] fix(agents): preserve skill selection when switching adapter
 type (#8975)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Each agent runs on an adapter (`claude_local`, `codex_local`, …) and
can be assigned company skills that are synced into its runtime
> - An agent's desired-skill selection is persisted inside its single
`adapterConfig` JSON blob under `paperclipSkillSync`, even though the
selection is a company-level, adapter-agnostic choice
> - When a user changes an agent's adapter type, both the server PATCH
handler and the UI patch builder rebuild `adapterConfig` and carry over
only a hardcoded allow-list of adapter-agnostic keys (`env`, `cwd`,
instructions bundle, …)
> - `paperclipSkillSync` was missing from both allow-lists, so switching
adapters (e.g. claude_local → codex_local) silently wiped every assigned
skill
> - This pull request adds `paperclipSkillSync` to the adapter-agnostic
preservation list on both layers and covers it with regression tests
> - The benefit is that switching an agent's adapter no longer destroys
its skill configuration — skills are preserved exactly like
env/cwd/instructions already are

## Linked Issues or Issue Description

Fixes #8974

## What Changed

- **Server (authoritative fix)** — `server/src/routes/agents.ts`: added
`"paperclipSkillSync"` to the `ADAPTER_AGNOSTIC_KEYS` list in the
`changingAdapterType` branch of `PATCH /agents/:id`. On an adapter-type
change the handler now restores the skill-sync selection from the
existing persisted config when the incoming config omits it — the same
mechanism already used for `env`, `cwd`, and the instructions bundle.
This protects every API/CLI client, not just the UI.
- **UI (defense in depth)** — `ui/src/lib/agent-config-patch.ts`: added
`"paperclipSkillSync"` to the client-side `ADAPTER_AGNOSTIC_KEYS` in
`buildAgentUpdatePatch`, so the optimistic patch the client builds on an
adapter switch stops stripping the key before it reaches the server.
- **Tests** — added regression tests on both layers:
- `server/src/__tests__/agent-instructions-routes.test.ts`: `PATCH`ing
`adapterType` (claude_local → codex_local) with `replaceAdapterConfig:
true` keeps `adapterConfig.paperclipSkillSync`.
- `ui/src/lib/agent-config-patch.test.ts`: `buildAgentUpdatePatch`
preserves `paperclipSkillSync` when the overlay changes the adapter
type.

## Verification

```
# server (run from repo root)
cd server && ../node_modules/.bin/vitest run \
  src/__tests__/agent-instructions-routes.test.ts \
  src/__tests__/agent-skills-routes.test.ts \
  src/__tests__/agent-adapter-validation-routes.test.ts \
  src/__tests__/agent-permissions-routes.test.ts
# 83 passed
../node_modules/.bin/tsc --noEmit -p tsconfig.json   # clean

# ui
cd ui && ./node_modules/.bin/vitest run src/lib/agent-config-patch.test.ts   # 7 passed
pnpm --filter @paperclipai/ui typecheck   # clean
```

Both new tests fail without the corresponding source change (verified
red → green).

Manual: create an agent on `claude_local`, assign skills, switch it to
`codex_local`, and confirm `GET /api/agents/:id/skills` still returns
the desired skills.

## Risks

Low risk.

- The change only *adds* one key to an existing preservation allow-list;
it does not alter how any other key is handled. Behavior for agents
without a `paperclipSkillSync` block is unchanged (the key is simply
absent and nothing is copied).
- `paperclipSkillSync` is adapter-agnostic (company skill keys, not
adapter-specific), so carrying it across an adapter switch is always
safe — a target adapter that does not support skill sync just ignores
it, and switching back restores the selection.
- Same-adapter config edits already merged and preserved the key; this
only closes the adapter-type-change gap, matching the existing
env/cwd/instructions behavior.
- Follow-up (not in this PR to keep it minimal): the server and client
`ADAPTER_AGNOSTIC_KEYS` lists are maintained separately and already
diverge (`instructionsFilePath` is client-only); a shared constant could
prevent future drift.

## Model Used

Claude Opus 4.8 (`claude-opus-4-8`, 1M-token context), extended thinking
enabled, with tool use (file edit, shell, GitHub CLI) via Claude Code. A
read-only sub-agent was used to trace the root cause across the server
and UI layers.

## 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 (bug fix, not a feature)
- [x] I have searched GitHub for duplicate or related PRs and linked
them above (none found)
- [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
(`fix/preserve-skills-on-adapter-type-switch`) 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 (N/A —
internal config-preservation fix, no user-facing docs or API contract
change)
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green (pending CI on this PR)
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
(pending review)
- [x] I will address all Greptile and reviewer comments before
requesting merge
---
 .../agent-instructions-routes.test.ts         | 39 +++++++++++++++++++
 server/src/routes/agents.ts                   |  4 ++
 ui/src/lib/agent-config-patch.test.ts         | 24 ++++++++++++
 ui/src/lib/agent-config-patch.ts              |  4 ++
 4 files changed, 71 insertions(+)

diff --git a/server/src/__tests__/agent-instructions-routes.test.ts b/server/src/__tests__/agent-instructions-routes.test.ts
index f35ef61060..6d36a820d3 100644
--- a/server/src/__tests__/agent-instructions-routes.test.ts
+++ b/server/src/__tests__/agent-instructions-routes.test.ts
@@ -328,6 +328,45 @@ describe("agent instructions bundle routes", () => {
     );
   });
 
+  it("preserves paperclip skill-sync selections when switching adapters", async () => {
+    // Desired skills live inside the per-adapter config under
+    // `paperclipSkillSync`, yet they are adapter-agnostic company-level
+    // selections. Switching adapter type must not silently wipe them — the
+    // server carries them over from the existing config the same way it
+    // preserves env/cwd and the instructions bundle.
+    mockAgentService.getById.mockResolvedValue({
+      ...makeAgent(),
+      adapterType: "claude_local",
+      adapterConfig: {
+        model: "claude-sonnet-4",
+        paperclipSkillSync: { desiredSkills: ["research", "code-review"] },
+      },
+    });
+
+    const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl)
+      .patch("/api/agents/11111111-1111-4111-8111-111111111111?companyId=company-1")
+      .send({
+        adapterType: "codex_local",
+        replaceAdapterConfig: true,
+        adapterConfig: {
+          model: "gpt-5.4",
+        },
+      }));
+
+    expect(res.status, JSON.stringify(res.body)).toBe(200);
+    expect(mockAgentService.update).toHaveBeenCalledWith(
+      "11111111-1111-4111-8111-111111111111",
+      expect.objectContaining({
+        adapterType: "codex_local",
+        adapterConfig: expect.objectContaining({
+          model: "gpt-5.4",
+          paperclipSkillSync: { desiredSkills: ["research", "code-review"] },
+        }),
+      }),
+      expect.any(Object),
+    );
+  });
+
   it("merges same-adapter config patches so instructions metadata is not dropped", async () => {
     mockAgentService.getById.mockResolvedValue({
       ...makeAgent(),
diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts
index e86851d871..1ac42fe9fa 100644
--- a/server/src/routes/agents.ts
+++ b/server/src/routes/agents.ts
@@ -2870,9 +2870,13 @@ export function agentRoutes(
         // Preserve adapter-agnostic keys (env, cwd, etc.) from the existing config
         // when the adapter type changes. Without this, a PATCH that includes
         // adapterConfig but omits these keys would silently drop them.
+        // `paperclipSkillSync` holds the agent's desired-skill selection, which is
+        // a company-level (adapter-agnostic) choice even though it is persisted
+        // inside the per-adapter config; switching adapters must not wipe it.
         const ADAPTER_AGNOSTIC_KEYS = [
           "env", "cwd", "timeoutSec", "graceSec",
           "promptTemplate", "bootstrapPromptTemplate",
+          "paperclipSkillSync",
         ] as const;
         for (const key of ADAPTER_AGNOSTIC_KEYS) {
           if (rawEffectiveAdapterConfig[key] === undefined && existingAdapterConfig[key] !== undefined) {
diff --git a/ui/src/lib/agent-config-patch.test.ts b/ui/src/lib/agent-config-patch.test.ts
index 7cdc79e2a2..46e6e85829 100644
--- a/ui/src/lib/agent-config-patch.test.ts
+++ b/ui/src/lib/agent-config-patch.test.ts
@@ -220,4 +220,28 @@ describe("buildAgentUpdatePatch", () => {
       replaceAdapterConfig: true,
     });
   });
+
+  it("preserves paperclip skill-sync selections when changing adapter types", () => {
+    // Desired skills are adapter-agnostic (company-level selections) but are
+    // persisted inside the per-adapter config under `paperclipSkillSync`. A
+    // patch that switches adapters must carry them over instead of wiping the
+    // agent's skills.
+    const agent = makeAgent();
+    agent.adapterConfig = {
+      ...agent.adapterConfig,
+      paperclipSkillSync: { desiredSkills: ["research", "code-review"] },
+    };
+
+    const patch = buildAgentUpdatePatch(
+      agent,
+      makeOverlay({
+        adapterType: "codex_local",
+        adapterConfig: { model: "gpt-5.4" },
+      }),
+    );
+
+    expect((patch.adapterConfig as Record<string, unknown>).paperclipSkillSync).toEqual({
+      desiredSkills: ["research", "code-review"],
+    });
+  });
 });
diff --git a/ui/src/lib/agent-config-patch.ts b/ui/src/lib/agent-config-patch.ts
index e57e0c05ab..325d8d8833 100644
--- a/ui/src/lib/agent-config-patch.ts
+++ b/ui/src/lib/agent-config-patch.ts
@@ -27,6 +27,10 @@ const ADAPTER_AGNOSTIC_KEYS = [
   "timeoutSec",
   "graceSec",
   "bootstrapPromptTemplate",
+  // Desired-skill selection is a company-level, adapter-agnostic choice even
+  // though it is persisted inside the per-adapter config; keep it when the
+  // adapter type changes so switching adapters does not wipe the agent's skills.
+  "paperclipSkillSync",
 ] as const;
 
 function omitUndefinedEntries(value: Record<string, unknown>) {