feat(adapters): allow external overrides of built-ins (#7394)
## Thinking Path > - Paperclip orchestrates AI agents through server-side adapters. > - Some adapters are bundled as built-ins, while external adapter plugins can provide newer or organization-specific implementations. > - The adapter registry already supports external plugins overriding a built-in type while keeping the built-in available as fallback. > - The hot-install API still rejected built-in adapter types before registration, so plugin installation did not match registry behavior. > - That blocked users from installing an external adapter update for a built-in adapter type such as `hermes_local`. > - This pull request removes the hot-install conflict guard and keeps the existing fallback lifecycle intact. > - The benefit is consistent adapter override behavior across startup registration, hot install, pause/resume, and removal. Fixes #7395 ## What Changed - Allows `POST /api/adapters/install` to register an external adapter whose type matches a built-in adapter. - Keeps built-in adapters protected from deletion unless there is an external plugin record for that adapter type. - Tightens the install route so `requiresRestart` is only reported on a true reinstall (existing external plugin record), not on a first-time override of a built-in adapter type. - Adds route coverage for installing a built-in type override, pausing back to the built-in implementation, deleting the override, and restoring the built-in adapter. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/adapter-routes.test.ts src/__tests__/adapter-registry.test.ts` - `pnpm --filter @paperclipai/server typecheck` - GitHub Actions passed for server tests, typecheck, build, serialized server suites, e2e, canary dry run, Socket, Snyk, Greptile, and policy checks on the prior pushed commit before the follow-up review fix. ## Risks - Low risk: this only changes the hot-install/removal lifecycle for external plugins targeting a built-in adapter type. - Built-in adapters remain protected when no external plugin record exists. - The existing registry fallback behavior restores the built-in adapter when an override is paused or removed. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex GPT-5.5 via Hermes Agent for the initial implementation and verification (terminal/file/GitHub tool use). - Anthropic Claude Opus 4.7 (claude-opus-4-7) via Paperclip Claude adapter for the Greptile-feedback follow-up commit (extended-thinking reasoning, terminal/file/GitHub tool use). ## 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 confirmed none exist for this hot-install override fix - [x] I have linked the existing issue with `Fixes #7395` - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [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 --------- Co-authored-by: HenkDz <henkdz@users.noreply.github.com> Co-authored-by: Devin Foley <devin@devinfoley.com> Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
committed by
GitHub
parent
9802636be3
commit
823c2b115a
@@ -46,6 +46,7 @@ const overridingConfigSchemaAdapter: ServerAdapterModule = {
|
||||
let registerServerAdapter: typeof import("../adapters/registry.js").registerServerAdapter;
|
||||
let unregisterServerAdapter: typeof import("../adapters/registry.js").unregisterServerAdapter;
|
||||
let findServerAdapter: typeof import("../adapters/registry.js").findServerAdapter;
|
||||
let findActiveServerAdapter: typeof import("../adapters/registry.js").findActiveServerAdapter;
|
||||
let setOverridePaused: typeof import("../adapters/registry.js").setOverridePaused;
|
||||
let adapterRoutes: typeof import("../routes/adapters.js").adapterRoutes;
|
||||
let errorHandler: typeof import("../middleware/index.js").errorHandler;
|
||||
@@ -109,6 +110,7 @@ describe("adapter routes", () => {
|
||||
registerServerAdapter = registry.registerServerAdapter;
|
||||
unregisterServerAdapter = registry.unregisterServerAdapter;
|
||||
findServerAdapter = registry.findServerAdapter;
|
||||
findActiveServerAdapter = registry.findActiveServerAdapter;
|
||||
setOverridePaused = registry.setOverridePaused;
|
||||
adapterRoutes = routes.adapterRoutes;
|
||||
errorHandler = middleware.errorHandler;
|
||||
@@ -339,4 +341,54 @@ describe("adapter routes", () => {
|
||||
|
||||
unregisterServerAdapter(HOT_INSTALL_TYPE);
|
||||
});
|
||||
|
||||
it("POST /api/adapters/install allows an external adapter to override a builtin type", async () => {
|
||||
const builtin = findServerAdapter("codex_local");
|
||||
expect(builtin).not.toBeNull();
|
||||
|
||||
const externalModule: ServerAdapterModule = {
|
||||
type: "codex_local",
|
||||
execute: async () => ({ exitCode: 0, signal: null, timedOut: false }),
|
||||
testEnvironment: async () => ({
|
||||
adapterType: "codex_local",
|
||||
status: "pass",
|
||||
checks: [],
|
||||
testedAt: new Date(0).toISOString(),
|
||||
}),
|
||||
models: [{ id: "plugin-codex", label: "Plugin Codex" }],
|
||||
};
|
||||
mockPluginLoader.loadExternalAdapterPackage.mockResolvedValue(externalModule);
|
||||
|
||||
const app = createApp({ isInstanceAdmin: true });
|
||||
const res = await request(app)
|
||||
.post("/api/adapters/install")
|
||||
.send({ packageName: "/tmp/fake-codex-override", isLocalPath: true });
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(201);
|
||||
expect(res.body.type).toBe("codex_local");
|
||||
const registeredOverride = findServerAdapter("codex_local");
|
||||
expect(registeredOverride).toMatchObject({
|
||||
type: "codex_local",
|
||||
models: [{ id: "plugin-codex", label: "Plugin Codex" }],
|
||||
});
|
||||
|
||||
setOverridePaused("codex_local", true);
|
||||
expect(findActiveServerAdapter("codex_local")).toBe(builtin);
|
||||
|
||||
mockAdapterPluginStore.getAdapterPluginByType.mockReturnValue({
|
||||
type: "codex_local",
|
||||
packageName: undefined,
|
||||
localPath: "/tmp/fake-codex-override",
|
||||
installedAt: new Date(0).toISOString(),
|
||||
});
|
||||
mockAdapterPluginStore.removeAdapterPlugin.mockReturnValue(true);
|
||||
|
||||
const removed = await request(app).delete("/api/adapters/codex_local");
|
||||
expect(removed.status, JSON.stringify(removed.body)).toBe(200);
|
||||
expect(removed.body).toMatchObject({ type: "codex_local", removed: true });
|
||||
|
||||
unregisterServerAdapter("codex_local");
|
||||
expect(findServerAdapter("codex_local")).toBe(builtin);
|
||||
setOverridePaused("codex_local", false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -296,17 +296,16 @@ export function adapterRoutes() {
|
||||
// Load and register the adapter (use canonicalName for path resolution)
|
||||
const adapterModule = await loadExternalAdapterPackage(canonicalName, moduleLocalPath);
|
||||
|
||||
// Check if this type conflicts with a built-in adapter
|
||||
if (BUILTIN_ADAPTER_TYPES.has(adapterModule.type)) {
|
||||
res.status(409).json({
|
||||
error: `Adapter type "${adapterModule.type}" is a built-in adapter and cannot be overwritten.`,
|
||||
});
|
||||
return;
|
||||
}
|
||||
// External adapters may intentionally override built-in adapter types.
|
||||
// registerServerAdapter preserves the built-in as a fallback so pausing or
|
||||
// removing the override restores the original implementation.
|
||||
|
||||
// Check if already registered (indicates a reinstall/update)
|
||||
// Check if already registered (indicates a reinstall/update).
|
||||
// For built-in types the registry always returns the built-in, so we
|
||||
// additionally require an existing external plugin record to
|
||||
// distinguish a true reinstall from a first-time override.
|
||||
const existing = findServerAdapter(adapterModule.type);
|
||||
const isReinstall = existing !== null;
|
||||
const isReinstall = existing !== null && !!getAdapterPluginByType(adapterModule.type);
|
||||
if (existing) {
|
||||
unregisterServerAdapter(adapterModule.type);
|
||||
logger.info({ type: adapterModule.type }, "Unregistered existing adapter for replacement");
|
||||
@@ -446,8 +445,9 @@ export function adapterRoutes() {
|
||||
return;
|
||||
}
|
||||
|
||||
// Prevent removal of built-in adapters
|
||||
if (BUILTIN_ADAPTER_TYPES.has(adapterType)) {
|
||||
// Prevent removal of built-in adapters, unless this built-in type is
|
||||
// currently backed by an external adapter plugin override.
|
||||
if (BUILTIN_ADAPTER_TYPES.has(adapterType) && !getAdapterPluginByType(adapterType)) {
|
||||
res.status(403).json({
|
||||
error: `Cannot remove built-in adapter "${adapterType}".`,
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user