forked from farhoodlabs/paperclip
24232078fd
## 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
458 lines
14 KiB
TypeScript
458 lines
14 KiB
TypeScript
import { describe, expect, it, beforeEach, afterEach, vi } from "vitest";
|
|
import type { ServerAdapterModule } from "../adapters/index.js";
|
|
|
|
const hermesExecuteMock = vi.hoisted(() =>
|
|
vi.fn(async () => ({
|
|
exitCode: 0,
|
|
signal: null,
|
|
timedOut: false,
|
|
})),
|
|
);
|
|
|
|
vi.mock("hermes-paperclip-adapter/server", () => ({
|
|
execute: hermesExecuteMock,
|
|
testEnvironment: async () => ({
|
|
adapterType: "hermes_local",
|
|
status: "pass",
|
|
checks: [],
|
|
testedAt: new Date(0).toISOString(),
|
|
}),
|
|
sessionCodec: null,
|
|
listSkills: async () => [],
|
|
syncSkills: async () => ({ entries: [] }),
|
|
detectModel: async () => null,
|
|
}));
|
|
|
|
import {
|
|
detectAdapterModel,
|
|
findActiveServerAdapter,
|
|
findServerAdapter,
|
|
listAdapterModels,
|
|
registerServerAdapter,
|
|
requireServerAdapter,
|
|
unregisterServerAdapter,
|
|
} from "../adapters/index.js";
|
|
import {
|
|
resolveExternalAdapterRegistration,
|
|
setOverridePaused,
|
|
} from "../adapters/registry.js";
|
|
|
|
const externalAdapter: ServerAdapterModule = {
|
|
type: "external_test",
|
|
execute: async () => ({
|
|
exitCode: 0,
|
|
signal: null,
|
|
timedOut: false,
|
|
}),
|
|
testEnvironment: async () => ({
|
|
adapterType: "external_test",
|
|
status: "pass",
|
|
checks: [],
|
|
testedAt: new Date(0).toISOString(),
|
|
}),
|
|
models: [{ id: "external-model", label: "External Model" }],
|
|
supportsLocalAgentJwt: false,
|
|
};
|
|
|
|
describe("server adapter registry", () => {
|
|
beforeEach(() => {
|
|
unregisterServerAdapter("external_test");
|
|
unregisterServerAdapter("claude_local");
|
|
setOverridePaused("claude_local", false);
|
|
});
|
|
|
|
afterEach(() => {
|
|
unregisterServerAdapter("external_test");
|
|
unregisterServerAdapter("claude_local");
|
|
setOverridePaused("claude_local", false);
|
|
hermesExecuteMock.mockClear();
|
|
});
|
|
|
|
it("registers external adapters and exposes them through lookup helpers", async () => {
|
|
expect(findServerAdapter("external_test")).toBeNull();
|
|
|
|
registerServerAdapter(externalAdapter);
|
|
|
|
expect(requireServerAdapter("external_test")).toBe(externalAdapter);
|
|
expect(await listAdapterModels("external_test")).toEqual([
|
|
{ id: "external-model", label: "External Model" },
|
|
]);
|
|
});
|
|
|
|
it("removes external adapters when unregistered", () => {
|
|
registerServerAdapter(externalAdapter);
|
|
|
|
unregisterServerAdapter("external_test");
|
|
|
|
expect(findServerAdapter("external_test")).toBeNull();
|
|
expect(() => requireServerAdapter("external_test")).toThrow(
|
|
"Unknown adapter type: external_test",
|
|
);
|
|
});
|
|
|
|
it("allows external plugin to override a built-in adapter type", () => {
|
|
// claude_local is always built-in
|
|
const builtIn = findServerAdapter("claude_local");
|
|
expect(builtIn).not.toBeNull();
|
|
|
|
const plugin: 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(),
|
|
}),
|
|
models: [{ id: "plugin-model", label: "Plugin Override" }],
|
|
supportsLocalAgentJwt: false,
|
|
};
|
|
|
|
registerServerAdapter(plugin);
|
|
|
|
// Plugin wins
|
|
const resolved = requireServerAdapter("claude_local");
|
|
expect(resolved).toBe(plugin);
|
|
expect(resolved.models).toEqual([
|
|
{ id: "plugin-model", label: "Plugin Override" },
|
|
]);
|
|
});
|
|
|
|
it("exposes capability flags from registered adapters", () => {
|
|
const adapterWithCaps: ServerAdapterModule = {
|
|
type: "external_test",
|
|
execute: async () => ({ exitCode: 0, signal: null, timedOut: false }),
|
|
testEnvironment: async () => ({
|
|
adapterType: "external_test",
|
|
status: "pass" as const,
|
|
checks: [],
|
|
testedAt: new Date(0).toISOString(),
|
|
}),
|
|
supportsLocalAgentJwt: true,
|
|
supportsInstructionsBundle: true,
|
|
instructionsPathKey: "customPathKey",
|
|
requiresMaterializedRuntimeSkills: true,
|
|
};
|
|
|
|
registerServerAdapter(adapterWithCaps);
|
|
|
|
const resolved = findActiveServerAdapter("external_test");
|
|
expect(resolved).not.toBeNull();
|
|
expect(resolved!.supportsInstructionsBundle).toBe(true);
|
|
expect(resolved!.instructionsPathKey).toBe("customPathKey");
|
|
expect(resolved!.requiresMaterializedRuntimeSkills).toBe(true);
|
|
expect(resolved!.supportsLocalAgentJwt).toBe(true);
|
|
});
|
|
|
|
it("returns undefined for capability flags on adapters that do not set them", () => {
|
|
registerServerAdapter(externalAdapter);
|
|
|
|
const resolved = findActiveServerAdapter("external_test");
|
|
expect(resolved).not.toBeNull();
|
|
expect(resolved!.supportsInstructionsBundle).toBeUndefined();
|
|
expect(resolved!.instructionsPathKey).toBeUndefined();
|
|
expect(resolved!.requiresMaterializedRuntimeSkills).toBeUndefined();
|
|
});
|
|
|
|
it("built-in claude_local adapter declares capability flags", () => {
|
|
const adapter = findActiveServerAdapter("claude_local");
|
|
expect(adapter).not.toBeNull();
|
|
expect(adapter!.supportsInstructionsBundle).toBe(true);
|
|
expect(adapter!.instructionsPathKey).toBe("instructionsFilePath");
|
|
expect(adapter!.requiresMaterializedRuntimeSkills).toBe(false);
|
|
expect(adapter!.supportsLocalAgentJwt).toBe(true);
|
|
});
|
|
|
|
it("switches active adapter behavior back to the builtin when an override is paused", async () => {
|
|
const builtIn = findServerAdapter("claude_local");
|
|
expect(builtIn).not.toBeNull();
|
|
|
|
const detectModel = vi.fn(async () => ({
|
|
model: "plugin-model",
|
|
provider: "plugin-provider",
|
|
source: "plugin-source",
|
|
}));
|
|
const plugin: 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(),
|
|
}),
|
|
models: [{ id: "plugin-model", label: "Plugin Override" }],
|
|
detectModel,
|
|
supportsLocalAgentJwt: false,
|
|
};
|
|
|
|
registerServerAdapter(plugin);
|
|
|
|
expect(findActiveServerAdapter("claude_local")).toBe(plugin);
|
|
expect(await listAdapterModels("claude_local")).toEqual([
|
|
{ id: "plugin-model", label: "Plugin Override" },
|
|
]);
|
|
expect(await detectAdapterModel("claude_local")).toMatchObject({
|
|
model: "plugin-model",
|
|
provider: "plugin-provider",
|
|
});
|
|
|
|
expect(setOverridePaused("claude_local", true)).toBe(true);
|
|
|
|
expect(findActiveServerAdapter("claude_local")).not.toBe(plugin);
|
|
expect(await listAdapterModels("claude_local")).toEqual(builtIn?.models ?? []);
|
|
expect(await detectAdapterModel("claude_local")).toBeNull();
|
|
expect(detectModel).toHaveBeenCalledTimes(1);
|
|
});
|
|
|
|
it("injects the local agent JWT and Paperclip API auth guidance into Hermes", async () => {
|
|
const adapter = requireServerAdapter("hermes_local");
|
|
|
|
await adapter.execute({
|
|
runId: "run-123",
|
|
agent: {
|
|
id: "agent-123",
|
|
companyId: "company-123",
|
|
name: "Hermes Agent",
|
|
role: "engineer",
|
|
adapterType: "hermes_local",
|
|
adapterConfig: {
|
|
env: {
|
|
OPENAI_API_KEY: "llm-token",
|
|
},
|
|
promptTemplate: "Existing prompt",
|
|
},
|
|
},
|
|
runtime: {},
|
|
config: {},
|
|
context: {},
|
|
onLog: async () => {},
|
|
onMeta: async () => {},
|
|
onSpawn: async () => {},
|
|
authToken: "agent-run-jwt",
|
|
});
|
|
|
|
expect(hermesExecuteMock).toHaveBeenCalledTimes(1);
|
|
const [patchedCtx] = hermesExecuteMock.mock.calls[0];
|
|
expect(patchedCtx.agent.adapterConfig).toMatchObject({
|
|
env: {
|
|
OPENAI_API_KEY: "llm-token",
|
|
PAPERCLIP_API_KEY: "agent-run-jwt",
|
|
PAPERCLIP_RUN_ID: "run-123",
|
|
},
|
|
});
|
|
expect(patchedCtx.agent.adapterConfig.promptTemplate).toContain(
|
|
"Authorization: Bearer $PAPERCLIP_API_KEY",
|
|
);
|
|
expect(patchedCtx.agent.adapterConfig.promptTemplate).toContain(
|
|
"X-Paperclip-Run-Id: $PAPERCLIP_RUN_ID",
|
|
);
|
|
expect(patchedCtx.agent.adapterConfig.promptTemplate).toContain("Existing prompt");
|
|
});
|
|
|
|
it("preserves Hermes command normalization while injecting auth", async () => {
|
|
const adapter = requireServerAdapter("hermes_local");
|
|
|
|
await adapter.execute({
|
|
runId: "run-123",
|
|
agent: {
|
|
id: "agent-123",
|
|
companyId: "company-123",
|
|
name: "Hermes Agent",
|
|
role: "engineer",
|
|
adapterType: "hermes_local",
|
|
adapterConfig: {
|
|
command: "agent-hermes",
|
|
},
|
|
},
|
|
runtime: {},
|
|
config: {
|
|
command: "runtime-hermes",
|
|
},
|
|
context: {},
|
|
onLog: async () => {},
|
|
onMeta: async () => {},
|
|
onSpawn: async () => {},
|
|
authToken: "agent-run-jwt",
|
|
});
|
|
|
|
expect(hermesExecuteMock).toHaveBeenCalledTimes(1);
|
|
const [patchedCtx] = hermesExecuteMock.mock.calls[0];
|
|
expect(patchedCtx.config.hermesCommand).toBe("runtime-hermes");
|
|
expect(patchedCtx.agent.adapterConfig.hermesCommand).toBe("agent-hermes");
|
|
expect(patchedCtx.agent.adapterConfig.env.PAPERCLIP_API_KEY).toBe("agent-run-jwt");
|
|
});
|
|
|
|
it("passes the original Hermes context through when authToken is absent", async () => {
|
|
const adapter = requireServerAdapter("hermes_local");
|
|
const ctx = {
|
|
runId: "run-123",
|
|
agent: {
|
|
id: "agent-123",
|
|
companyId: "company-123",
|
|
name: "Hermes Agent",
|
|
role: "engineer",
|
|
adapterType: "hermes_local",
|
|
adapterConfig: {
|
|
env: {
|
|
PAPERCLIP_API_KEY: "server-level-key",
|
|
},
|
|
promptTemplate: "Existing prompt",
|
|
},
|
|
},
|
|
runtime: {},
|
|
config: {},
|
|
context: {},
|
|
onLog: async () => {},
|
|
onMeta: async () => {},
|
|
onSpawn: async () => {},
|
|
};
|
|
|
|
await adapter.execute(ctx);
|
|
|
|
expect(hermesExecuteMock).toHaveBeenCalledTimes(1);
|
|
expect(hermesExecuteMock).toHaveBeenCalledWith(ctx);
|
|
});
|
|
|
|
it("preserves an explicit Hermes Paperclip API key and does not set promptTemplate when none was configured", async () => {
|
|
const adapter = requireServerAdapter("hermes_local");
|
|
|
|
await adapter.execute({
|
|
runId: "run-123",
|
|
agent: {
|
|
id: "agent-123",
|
|
companyId: "company-123",
|
|
name: "Hermes Agent",
|
|
role: "engineer",
|
|
adapterType: "hermes_local",
|
|
adapterConfig: {
|
|
env: {
|
|
PAPERCLIP_API_KEY: "explicit-agent-key",
|
|
PAPERCLIP_RUN_ID: "stale-run-id",
|
|
},
|
|
},
|
|
},
|
|
runtime: {},
|
|
config: {},
|
|
context: {},
|
|
onLog: async () => {},
|
|
onMeta: async () => {},
|
|
onSpawn: async () => {},
|
|
authToken: "agent-run-jwt",
|
|
});
|
|
|
|
const [patchedCtx] = hermesExecuteMock.mock.calls[0];
|
|
expect(patchedCtx.agent.adapterConfig.env.PAPERCLIP_API_KEY).toBe("explicit-agent-key");
|
|
expect(patchedCtx.agent.adapterConfig.env.PAPERCLIP_RUN_ID).toBe("run-123");
|
|
// No custom promptTemplate was set — Hermes must use its built-in default.
|
|
// Setting promptTemplate here would replace the full default with just the auth guard text,
|
|
// stripping assigned issue / workflow instructions.
|
|
expect(patchedCtx.agent.adapterConfig.promptTemplate).toBeUndefined();
|
|
});
|
|
|
|
it("does not set promptTemplate when no custom template is configured, preserving Hermes default", async () => {
|
|
const adapter = requireServerAdapter("hermes_local");
|
|
|
|
await adapter.execute({
|
|
runId: "run-123",
|
|
agent: {
|
|
id: "agent-123",
|
|
companyId: "company-123",
|
|
name: "Hermes Agent",
|
|
role: "engineer",
|
|
adapterType: "hermes_local",
|
|
adapterConfig: {},
|
|
},
|
|
runtime: {},
|
|
config: {},
|
|
context: {},
|
|
onLog: async () => {},
|
|
onMeta: async () => {},
|
|
onSpawn: async () => {},
|
|
authToken: "agent-run-jwt",
|
|
});
|
|
|
|
const [patchedCtx] = hermesExecuteMock.mock.calls[0];
|
|
// promptTemplate must remain unset so Hermes uses its built-in heartbeat/task prompt.
|
|
expect(patchedCtx.agent.adapterConfig.promptTemplate).toBeUndefined();
|
|
// Auth token is still injected.
|
|
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();
|
|
});
|
|
});
|