fix(agents): refuse to hire onto an adapter this instance cannot run (#10256)
## 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 "<type>" is not available on this instance.
Available adapters: <list>`. 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) <noreply@anthropic.com>
This commit is contained in:
Jannes Stubbemann
GitHub
parent
68c699687e
commit
a0bdf388af
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<string, unknown>;
|
||||
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<string, unknown>;
|
||||
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<string, unknown> | null = null;
|
||||
if (hasOwn(patchData, "runtimeConfig")) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue