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")) {