From 24232078fd64575a31713428c3df13e57dd66f38 Mon Sep 17 00:00:00 2001 From: Michel Tomas Date: Thu, 23 Apr 2026 14:39:43 +0200 Subject: [PATCH] fix(adapters/registry): honor module-provided sessionManagement for external adapters (#4296) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - Adapters are how paperclip hands work off to specific agent runtimes; since #2218, external adapter packages can ship as npm modules loaded via `server/src/adapters/plugin-loader.ts` > - Each `ServerAdapterModule` can declare `sessionManagement` (`supportsSessionResume`, `nativeContextManagement`, `defaultSessionCompaction`) — but the init-time load at `registry.ts:363-369` hard-overwrote it with a hardcoded-registry lookup that has no entries for external types, so modules could not actually set these fields > - The hot-install path at `routes/adapters.ts:179` → `registerServerAdapter` preserves module-provided `sessionManagement`, so externals worked after `POST /api/adapters/install` — *until the next server restart*, when the init-time IIFE wiped it back to `undefined` > - #2218 explicitly deferred this: *"Adapter execution model, heartbeat protocol, and session management are untouched."* This PR is the natural follow-up for session management on the plugin-loader path > - This PR aligns init-time registration with the hot-install path: honor module-provided `sessionManagement` first, fall back to the hardcoded registry when absent (so externals overriding a built-in type still inherit its policy). Extracted as a testable helper with three unit tests > - The benefit is external adapters can declare session-resume capabilities consistently across cold-start and hot-install, without requiring upstream additions to the hardcoded registry for each new plugin ## What Changed - `server/src/adapters/registry.ts`: extracted the merge logic into a new exported helper `resolveExternalAdapterRegistration()` — honors module-provided `sessionManagement` first, falls back to `getAdapterSessionManagement(type)`, else `undefined`. The init-time IIFE calls the helper instead of inlining an overwrite. - `server/src/adapters/registry.ts`: updated the section comment (lines 331–340) to reflect the new semantics and cross-reference the hot-install path's behavior. - `server/src/__tests__/adapter-registry.test.ts`: new `describe("resolveExternalAdapterRegistration")` block with three tests — module-provided value preserved, registry fallback when module omits, `undefined` when neither provides. ## Verification Targeted test run from a clean tree on `fix/external-session-management`: ``` cd server && pnpm exec vitest run src/__tests__/adapter-registry.test.ts # 1 test file, 15 tests passed, 0 failed (12 pre-existing + 3 new) ``` Full server suite via the independent review pass noted under Model Used: **1,156 tests passed, 0 failed**. Typecheck note: `pnpm --filter @paperclipai/server exec tsc --noEmit` surfaces two errors in `src/services/plugin-host-services.ts:1510` (`createInteraction` + implicit-any). Verified by `git stash` + re-run on clean `upstream/master` — they reproduce without this PR's changes. Pre-existing, out of scope. ## Risks - **Low behavioral risk.** Strictly additive: externals that do NOT provide `sessionManagement` continue to receive exactly the same value as before (registry lookup → `undefined` for pure externals, or the builtin's entry for externals overriding a built-in type). Only a new capability is unlocked; no existing behavior changes for existing adapters. - **No breaking change.** `ServerAdapterModule.sessionManagement` was already optional at the type level. Externals that never set it see no difference on either path. - **Consistency verified.** Init-time IIFE now matches the post-`POST /api/adapters/install` behavior — a server restart no longer regresses the field. ## Note This is part of a broader effort to close the parity gap between external and built-in adapters. Once externals reach 1:1 capability coverage with internals, new-adapter contributions can increasingly be steered toward the external-plugin path instead of the core product — a trajectory CONTRIBUTING.md already encourages ("*If the idea fits as an extension, prefer building it with the plugin system*"). ## Model Used - **Provider**: Anthropic - **Model**: Claude Opus 4.7 - **Exact model ID**: `claude-opus-4-7` (1M-context variant: `claude-opus-4-7[1m]`) - **Context window**: 1,000,000 tokens - **Harness**: Claude Code (Anthropic's official CLI), orchestrated by @superbiche as human-in-the-loop. Full file-editing, shell, and `gh` tool use, plus parallel research subagents for fact-finding against paperclip internals (plugin-loader contract, sessionCodec reachability, UI parser surface, Cline CLI JSON schema). - **Independent local review**: Gemini 3.1 Pro (Google) performed a separate verification pass on the committed branch — confirmed the approach & necessity, ran the full workspace build, and executed the complete server test suite (1,156 tests, all passing). Not used for authoring; second-opinion pass only. - **Authoring split**: @superbiche identified the gap (while mapping the external-adapter surface for a downstream adapter build) and shaped the plan — categorising the surface into `works / acceptable / needs-upstream` buckets, directing the surgical-diff approach on a fresh branch from `upstream/master`, and calling the framing ("alignment bug between init-time IIFE and hot-install path" rather than "missing capability"). Opus 4.7 executed the fact-finding, the diff, the tests, and drafted this PR body — all under direct review. ## 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 (convention-aligned bug fix on the external-adapter plugin path introduced by #2218) - [x] I have run tests locally and they pass (15/15 in the touched file; 1,156/1,156 full server suite via the independent Gemini 3.1 Pro review) - [x] I have added tests where applicable (3 new for the extracted helper) - [x] If this change affects the UI, I have included before/after screenshots (no UI touched) - [x] I have updated relevant documentation to reflect my changes (in-file comment reflects new semantics) - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --- server/src/__tests__/adapter-registry.test.ts | 73 ++++++++++++++++++- server/src/adapters/registry.ts | 36 +++++++-- 2 files changed, 103 insertions(+), 6 deletions(-) diff --git a/server/src/__tests__/adapter-registry.test.ts b/server/src/__tests__/adapter-registry.test.ts index ec5c1988..8567d028 100644 --- a/server/src/__tests__/adapter-registry.test.ts +++ b/server/src/__tests__/adapter-registry.test.ts @@ -32,7 +32,10 @@ import { requireServerAdapter, unregisterServerAdapter, } from "../adapters/index.js"; -import { setOverridePaused } from "../adapters/registry.js"; +import { + resolveExternalAdapterRegistration, + setOverridePaused, +} from "../adapters/registry.js"; const externalAdapter: ServerAdapterModule = { type: "external_test", @@ -384,3 +387,71 @@ describe("server adapter registry", () => { expect(patchedCtx.agent.adapterConfig.env.PAPERCLIP_API_KEY).toBe("agent-run-jwt"); }); }); + +describe("resolveExternalAdapterRegistration", () => { + it("preserves module-provided sessionManagement", () => { + const sessionManagement = { + supportsSessionResume: true, + nativeContextManagement: "unknown" as const, + defaultSessionCompaction: { + enabled: true, + maxSessionRuns: 200, + maxRawInputTokens: 2_000_000, + maxSessionAgeHours: 72, + }, + }; + const adapter: ServerAdapterModule = { + type: "external_session_test", + execute: async () => ({ exitCode: 0, signal: null, timedOut: false }), + testEnvironment: async () => ({ + adapterType: "external_session_test", + status: "pass", + checks: [], + testedAt: new Date(0).toISOString(), + }), + sessionManagement, + }; + + const resolved = resolveExternalAdapterRegistration(adapter); + + expect(resolved.sessionManagement).toBe(sessionManagement); + }); + + it("falls back to the hardcoded registry when the module omits sessionManagement", () => { + // An external that overrides a built-in type should inherit the built-in's + // sessionManagement when it does not provide its own. + const adapter: ServerAdapterModule = { + type: "claude_local", + execute: async () => ({ exitCode: 0, signal: null, timedOut: false }), + testEnvironment: async () => ({ + adapterType: "claude_local", + status: "pass", + checks: [], + testedAt: new Date(0).toISOString(), + }), + }; + + const resolved = resolveExternalAdapterRegistration(adapter); + + expect(resolved.sessionManagement).toBeDefined(); + expect(resolved.sessionManagement?.supportsSessionResume).toBe(true); + expect(resolved.sessionManagement?.nativeContextManagement).toBe("confirmed"); + }); + + it("leaves sessionManagement undefined when neither module nor registry provides one", () => { + const adapter: ServerAdapterModule = { + type: "external_unknown_test", + execute: async () => ({ exitCode: 0, signal: null, timedOut: false }), + testEnvironment: async () => ({ + adapterType: "external_unknown_test", + status: "pass", + checks: [], + testedAt: new Date(0).toISOString(), + }), + }; + + const resolved = resolveExternalAdapterRegistration(adapter); + + expect(resolved.sessionManagement).toBeUndefined(); + }); +}); diff --git a/server/src/adapters/registry.ts b/server/src/adapters/registry.ts index 9ee10d13..40202edc 100644 --- a/server/src/adapters/registry.ts +++ b/server/src/adapters/registry.ts @@ -331,7 +331,13 @@ registerBuiltInAdapters(); // Load external adapter plugins (e.g. droid_local) // // External adapter packages export createServerAdapter() which returns a -// ServerAdapterModule. The host fills in sessionManagement. +// ServerAdapterModule. When the module provides its own sessionManagement +// it is preserved; otherwise the host falls back to the built-in registry +// lookup (so externals that override a built-in type inherit the builtin's +// policy). This brings init-time registration to at-least-as-good behavior +// as the hot-install path (routes/adapters.ts:179 -> registerServerAdapter): +// both preserve module-provided sessionManagement, and init-time additionally +// applies the registry fallback for externals overriding a built-in type. // --------------------------------------------------------------------------- /** Cached sync wrapper — the store is a simple JSON file read, safe to call frequently. */ @@ -339,6 +345,29 @@ function getDisabledAdapterTypesFromStore(): string[] { return getDisabledAdapterTypes(); } +/** + * Merge an external adapter module with host-provided session management. + * + * Module-provided `sessionManagement` takes precedence. When absent, fall + * back to the hardcoded registry keyed by adapter type (so externals that + * override a built-in — same `type` — inherit the builtin's policy). If + * neither is available, `sessionManagement` remains `undefined`. + * + * Exported for unit tests; runtime callers use the IIFE below, which + * applies this transformation during the external-adapter load pass. + */ +export function resolveExternalAdapterRegistration( + externalAdapter: ServerAdapterModule, +): ServerAdapterModule { + return { + ...externalAdapter, + sessionManagement: + externalAdapter.sessionManagement + ?? getAdapterSessionManagement(externalAdapter.type) + ?? undefined, + }; +} + /** * Load external adapters from the plugin store and hardcoded sources. * Called once at module initialization. The promise is exported so that @@ -362,10 +391,7 @@ const externalAdaptersReady: Promise = (async () => { } adaptersByType.set( externalAdapter.type, - { - ...externalAdapter, - sessionManagement: getAdapterSessionManagement(externalAdapter.type) ?? undefined, - }, + resolveExternalAdapterRegistration(externalAdapter), ); } } catch (err) {