From a0bdf388af69456eb6fa846d879944442c46f26e Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Thu, 13 Aug 2026 01:43:40 +0200 Subject: [PATCH] fix(agents): refuse to hire onto an adapter this instance cannot run (#10256) 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 > - Hiring an agent means choosing a harness (adapter) for it, and an instance can declare which harnesses it actually runs through `PAPERCLIP_ADAPTERS`, which `reconcileAdapterAvailability` turns into a disabled set at boot > - The hire and create routes validate the adapter type with `assertKnownAdapterType`, which only asks whether the adapter is REGISTERED — a disabled adapter passes > - So an agent can be created on a harness the instance cannot run, and the failure only appears later, per run, at lease time: `Adapter "..." is not in the configured adapter registry` > - By then the error is in a run log, minutes after the choice, with nothing tying it back to the harness the user picked; the agent also keeps accepting work it can never do > - This pull request validates the hire and create paths against the ENABLED set and refuses with a message that names the adapters that are available > - The benefit is that an impossible choice fails at the moment it is made, in the words of the choice itself, instead of as a run failure the user cannot act on ## Linked Issues or Issue Description No existing issue; describing it here per the bug report template. **What happened** On an instance with a curated registry, a company's Chief of Staff was hired on `cursor_cloud`, which that instance had disabled. The API accepted the hire. Its first assignment run then failed: ``` Failed to acquire lease for environment "Kubernetes Sandbox" (sandbox): Adapter "cursor_cloud" is not in the configured adapter registry ``` and its automation run sat in `queued` for hours afterwards. Nothing in the hire response, the agent detail view, or the agent's status explained that this harness could never run. **Expected behavior** hiring on an adapter the instance has disabled is refused at hire time, with a message naming the adapters that can be chosen. **Steps to reproduce** 1. Start the server with a registry that omits an otherwise-registered adapter, e.g. `PAPERCLIP_ADAPTERS` listing `claude_local` but not `cursor_cloud`. 2. `POST /api/companies/:companyId/agents` with `{"name":"CoS","adapterType":"cursor_cloud"}`. 3. The agent is created (201). Every run it attempts fails at lease time with the message above. **Paperclip version or commit** master (`4c55f0d8d`). ## What Changed - `server/src/routes/agents.ts`: adds `assertSelectableAdapterType`, which extends `assertKnownAdapterType` with an enabled-set check and throws `422 Adapter "" is not available on this instance. Available adapters: `. The hire (`POST .../agent-hires`) and create (`POST .../agents`) paths now use it. - Routes that operate on an EXISTING agent keep `assertKnownAdapterType`, so an agent already running on a since-disabled adapter is unaffected — the same rule `listEnabledServerAdapters` already documents ("hidden from selection, still functional for agents that already use them"). - `server/src/__tests__/agent-adapter-validation-routes.test.ts`: mocks the adapter-plugin store's disabled set (so the test never writes to a real `~/.paperclip/adapter-settings.json`), and covers refuse-when-disabled (including that the message names the alternatives and that no agent is created) plus create-still-works-when-enabled. ## Verification ``` pnpm vitest run server/src/__tests__/agent-adapter-validation-routes.test.ts ``` 13 tests pass, including the two new cases and the existing unknown-adapter-type test. Manual: disable an adapter (`PATCH /api/adapters/:type {"disabled": true}` as an instance admin, or omit it from `PAPERCLIP_ADAPTERS` and restart), then POST an agent with that `adapterType` — 422 naming the available adapters, and no agent row is created. ## Risks Low, and scoped to new selections: - Automation that creates agents on a disabled adapter now gets a 422 where it previously got a 201 followed by runs that always failed. That is the intended behavior change, and the message names the valid choices. - Existing agents, and every route that acts on an existing agent, are untouched. - The enabled set comes from the same store `GET /api/adapters` already reports, so the API and the picker cannot disagree. ## Model Used Claude Opus 5 (Anthropic), model id `claude-opus-5`, 1M context window, extended thinking, with tool use and code execution via Claude Code. ## 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 (`upstream/adapter-selection-guard`) and contains no internal ticket id - [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 (the new helper documents the selection-vs-existing-agent rule) - [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 Related: #10254 makes the adapter inventory readable during onboarding, which is what lets the picker hide these adapters in the first place. This PR is the server-side backstop for the same failure. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) --- .../agent-adapter-validation-routes.test.ts | 90 +++++++++++++++++++ server/src/routes/agents.ts | 46 +++++++++- 2 files changed, 133 insertions(+), 3 deletions(-) diff --git a/server/src/__tests__/agent-adapter-validation-routes.test.ts b/server/src/__tests__/agent-adapter-validation-routes.test.ts index f17158ce37..ff1a00e9d8 100644 --- a/server/src/__tests__/agent-adapter-validation-routes.test.ts +++ b/server/src/__tests__/agent-adapter-validation-routes.test.ts @@ -11,6 +11,10 @@ const mockAgentService = vi.hoisted(() => ({ update: vi.fn(), })); +const mockAdapterPluginStore = vi.hoisted(() => ({ + getDisabledAdapterTypes: vi.fn<() => string[]>(() => []), +})); + const mockAccessService = vi.hoisted(() => ({ canUser: vi.fn(), decide: vi.fn(), @@ -114,6 +118,18 @@ function registerModuleMocks() { vi.doMock("../services/secrets.js", () => ({ secretService: () => mockSecretService, })); + + // The adapter registry reads the disabled set from this store. Mock it so a + // test can declare an adapter disabled without writing to the real + // ~/.paperclip/adapter-settings.json. + vi.doMock("../services/adapter-plugin-store.js", () => ({ + getDisabledAdapterTypes: mockAdapterPluginStore.getDisabledAdapterTypes, + isAdapterDisabled: (type: string) => + mockAdapterPluginStore.getDisabledAdapterTypes().includes(type), + listAdapterPlugins: () => [], + getAdapterPluginByType: () => undefined, + setAdapterDisabled: vi.fn(), + })); } const externalAdapter: ServerAdapterModule = { @@ -204,6 +220,7 @@ describe("agent routes adapter validation", () => { vi.doUnmock("../routes/agents.js"); registerModuleMocks(); vi.clearAllMocks(); + mockAdapterPluginStore.getDisabledAdapterTypes.mockReturnValue([]); mockCompanySkillService.listRuntimeSkillEntries.mockResolvedValue([]); mockCompanySkillService.resolveRequestedSkillKeys.mockResolvedValue([]); mockAccessService.canUser.mockResolvedValue(true); @@ -423,4 +440,77 @@ describe("agent routes adapter validation", () => { expect(res.status, JSON.stringify(res.body)).toBe(422); expect(String(res.body.error ?? res.body.message ?? "")).toContain(`Unknown adapter type: ${missingAdapterType}`); }); + + it("refuses to create an agent on an adapter the instance has disabled", async () => { + // A disabled adapter is one the instance cannot run (e.g. curated out of + // PAPERCLIP_ADAPTERS). Creating an agent on it "succeeds" and then every + // run of that agent dies at lease time with "not in the configured adapter + // registry", so the refusal belongs here, where it can name the choices. + const { registerServerAdapter } = await import("../adapters/index.js"); + registerServerAdapter(externalAdapter); + mockAdapterPluginStore.getDisabledAdapterTypes.mockReturnValue(["external_test"]); + + const app = await createApp(); + const res = await requestApp(app, (baseUrl) => + request(baseUrl) + .post("/api/companies/company-1/agents") + .send({ name: "Disabled Harness", adapterType: "external_test" }), + ); + + expect(res.status, JSON.stringify(res.body)).toBe(422); + const message = String(res.body.error ?? res.body.message ?? ""); + expect(message).toContain('Adapter "external_test" is not available on this instance'); + // The message must be actionable: it names what CAN be chosen. + expect(message).toMatch(/Available adapters?: .+/); + expect(mockAgentService.create).not.toHaveBeenCalled(); + }); + + it("refuses to switch an existing agent onto a disabled adapter", async () => { + const { registerServerAdapter } = await import("../adapters/index.js"); + registerServerAdapter(externalAdapter); + mockAdapterPluginStore.getDisabledAdapterTypes.mockReturnValue(["external_test"]); + + const app = await createApp(); + const res = await requestApp(app, (baseUrl) => + request(baseUrl) + .patch("/api/agents/11111111-1111-4111-8111-111111111111") + .send({ adapterType: "external_test" }), + ); + + expect(res.status, JSON.stringify(res.body)).toBe(422); + expect(String(res.body.error ?? res.body.message ?? "")).toContain( + 'Adapter "external_test" is not available on this instance', + ); + expect(mockAgentService.update).not.toHaveBeenCalled(); + }); + + it("still lets an agent already on a disabled adapter be edited", async () => { + // Disabling an adapter must not make the agents that already use it + // uneditable — only NEW selections of it are refused. + mockAdapterPluginStore.getDisabledAdapterTypes.mockReturnValue(["codex_local"]); + + const app = await createApp(); + const res = await requestApp(app, (baseUrl) => + request(baseUrl) + .patch("/api/agents/11111111-1111-4111-8111-111111111111") + .send({ adapterType: "codex_local", adapterConfig: { model: "gpt-5.4" } }), + ); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + }); + + it("still creates an agent on an adapter that is registered and enabled", async () => { + const { registerServerAdapter } = await import("../adapters/index.js"); + registerServerAdapter(externalAdapter); + mockAdapterPluginStore.getDisabledAdapterTypes.mockReturnValue(["some_other_adapter"]); + + const app = await createApp(); + const res = await requestApp(app, (baseUrl) => + request(baseUrl) + .post("/api/companies/company-1/agents") + .send({ name: "Enabled Harness", adapterType: "external_test" }), + ); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + }); }); diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 3dbdc4a7ed..7ff546cb9f 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -73,6 +73,7 @@ import type { AdapterEnvironmentTestResult, AdapterModelProfileDefinition, } from "@paperclipai/adapter-utils"; +import { getDisabledAdapterTypes } from "../services/adapter-plugin-store.js"; import { skillVersionSelectionMap } from "../services/runtime-skill-selections.js"; import { secretService } from "../services/secrets.js"; import { authorizationDeniedDetails } from "../services/authorization.js"; @@ -80,6 +81,7 @@ import { detectAdapterModel, findActiveServerAdapter, findServerAdapter, + listServerAdapters, listAdapterModels, listAdapterModelProfiles, refreshAdapterModels, @@ -1288,6 +1290,37 @@ export function agentRoutes( return adapterType; } + /** + * Adapter validation for the paths that CHOOSE a harness for a new agent + * (hire + create), as opposed to the paths that operate on an existing one. + * + * A disabled adapter is one this instance cannot run — most often because a + * declarative registry (PAPERCLIP_ADAPTERS) curated it out, which + * reconcileAdapterAvailability turns into a disabled type at boot. Registered + * but disabled still passes assertKnownAdapterType, so an agent could be + * created on it and then fail EVERY run at lease time with + * `Adapter "..." is not in the configured adapter registry` — an error that + * arrives minutes later, in a run log, with no way back to the choice that + * caused it. Refuse at selection time instead, and name what can be chosen. + * + * Existing agents on a now-disabled adapter are deliberately untouched + * (listEnabledServerAdapters documents the same rule: hidden from selection, + * still functional for agents that already use them). + */ + function assertSelectableAdapterType(type: string | null | undefined): string { + const adapterType = assertKnownAdapterType(type); + const disabled = new Set(getDisabledAdapterTypes()); + if (!disabled.has(adapterType)) return adapterType; + const available = listServerAdapters() + .map((a) => a.type) + .filter((t) => !disabled.has(t)) + .sort(); + throw unprocessable( + `Adapter "${adapterType}" is not available on this instance. ` + + `Available adapters: ${available.length > 0 ? available.join(", ") : "(none configured)"}`, + ); + } + async function assertAgentDefaultEnvironmentSelection( companyId: string, environmentId: string | null | undefined, @@ -2945,7 +2978,7 @@ export function agentRoutes( sourceIssueIds: _sourceIssueIds, ...hireInput } = req.body; - hireInput.adapterType = assertKnownAdapterType(hireInput.adapterType); + hireInput.adapterType = assertSelectableAdapterType(hireInput.adapterType); const rawHireAdapterConfig = (hireInput.adapterConfig ?? {}) as Record; assertNoNewAgentLegacyPromptTemplate( hireInput.adapterType, @@ -3141,7 +3174,7 @@ export function agentRoutes( instructionsBundle, ...createInput } = req.body; - createInput.adapterType = assertKnownAdapterType(createInput.adapterType); + createInput.adapterType = assertSelectableAdapterType(createInput.adapterType); const rawCreateAdapterConfig = (createInput.adapterConfig ?? {}) as Record; assertNoNewAgentLegacyPromptTemplate( createInput.adapterType, @@ -3553,8 +3586,15 @@ export function agentRoutes( patchData.adapterConfig = adapterConfig; } + // Switching an existing agent ONTO another adapter is a new selection, so + // it gets the selectable check; keeping the agent's current adapter (even + // one since disabled) stays allowed, so a disabled harness does not make an + // existing agent uneditable. const requestedAdapterType = hasOwn(patchData, "adapterType") - ? assertKnownAdapterType(patchData.adapterType as string | null | undefined) + ? (() => { + const next = assertKnownAdapterType(patchData.adapterType as string | null | undefined); + return next === existing.adapterType ? next : assertSelectableAdapterType(next); + })() : existing.adapterType; let requestedRuntimeConfig: Record | null = null; if (hasOwn(patchData, "runtimeConfig")) {