mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-14 01:50:39 +09:00
fix(adapters): honor paused overrides and isolate UI parser state
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
c6d2dc8b56
commit
d9476abecb
17 changed files with 297 additions and 53 deletions
|
|
@ -1,12 +1,15 @@
|
|||
import { describe, expect, it, beforeEach, afterEach } from "vitest";
|
||||
import { describe, expect, it, beforeEach, afterEach, vi } from "vitest";
|
||||
import type { ServerAdapterModule } from "../adapters/index.js";
|
||||
import {
|
||||
detectAdapterModel,
|
||||
findActiveServerAdapter,
|
||||
findServerAdapter,
|
||||
listAdapterModels,
|
||||
registerServerAdapter,
|
||||
requireServerAdapter,
|
||||
unregisterServerAdapter,
|
||||
} from "../adapters/index.js";
|
||||
import { setOverridePaused } from "../adapters/registry.js";
|
||||
|
||||
const externalAdapter: ServerAdapterModule = {
|
||||
type: "external_test",
|
||||
|
|
@ -28,10 +31,14 @@ const externalAdapter: ServerAdapterModule = {
|
|||
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);
|
||||
});
|
||||
|
||||
it("registers external adapters and exposes them through lookup helpers", async () => {
|
||||
|
|
@ -87,4 +94,50 @@ describe("server adapter registry", () => {
|
|||
{ id: "plugin-model", label: "Plugin Override" },
|
||||
]);
|
||||
});
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
78
server/src/__tests__/adapter-routes.test.ts
Normal file
78
server/src/__tests__/adapter-routes.test.ts
Normal file
|
|
@ -0,0 +1,78 @@
|
|||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import type { ServerAdapterModule } from "../adapters/index.js";
|
||||
import { registerServerAdapter, unregisterServerAdapter } from "../adapters/index.js";
|
||||
import { setOverridePaused } from "../adapters/registry.js";
|
||||
import { adapterRoutes } from "../routes/adapters.js";
|
||||
import { errorHandler } from "../middleware/index.js";
|
||||
|
||||
const overridingConfigSchemaAdapter: 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(),
|
||||
}),
|
||||
getConfigSchema: async () => ({
|
||||
version: 1,
|
||||
fields: [
|
||||
{
|
||||
key: "mode",
|
||||
type: "text",
|
||||
label: "Mode",
|
||||
},
|
||||
],
|
||||
}),
|
||||
};
|
||||
|
||||
function createApp() {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use((req, _res, next) => {
|
||||
(req as any).actor = {
|
||||
type: "board",
|
||||
userId: "local-board",
|
||||
companyIds: [],
|
||||
source: "local_implicit",
|
||||
isInstanceAdmin: false,
|
||||
};
|
||||
next();
|
||||
});
|
||||
app.use("/api", adapterRoutes());
|
||||
app.use(errorHandler);
|
||||
return app;
|
||||
}
|
||||
|
||||
describe("adapter routes", () => {
|
||||
beforeEach(() => {
|
||||
setOverridePaused("claude_local", false);
|
||||
registerServerAdapter(overridingConfigSchemaAdapter);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
setOverridePaused("claude_local", false);
|
||||
unregisterServerAdapter("claude_local");
|
||||
});
|
||||
|
||||
it("uses the active adapter when resolving config schema for a paused builtin override", async () => {
|
||||
const app = createApp();
|
||||
|
||||
const active = await request(app).get("/api/adapters/claude_local/config-schema");
|
||||
expect(active.status, JSON.stringify(active.body)).toBe(200);
|
||||
expect(active.body).toMatchObject({
|
||||
fields: [{ key: "mode" }],
|
||||
});
|
||||
|
||||
const paused = await request(app)
|
||||
.patch("/api/adapters/claude_local/override")
|
||||
.send({ paused: true });
|
||||
expect(paused.status, JSON.stringify(paused.body)).toBe(200);
|
||||
|
||||
const builtin = await request(app).get("/api/adapters/claude_local/config-schema");
|
||||
expect(builtin.status, JSON.stringify(builtin.body)).toBe(404);
|
||||
expect(String(builtin.body.error ?? "")).toContain("does not provide a config schema");
|
||||
});
|
||||
});
|
||||
|
|
@ -61,6 +61,7 @@ const mockAdapter = vi.hoisted(() => ({
|
|||
|
||||
vi.mock("@paperclipai/shared/telemetry", () => ({
|
||||
trackAgentCreated: mockTrackAgentCreated,
|
||||
trackErrorHandlerCrash: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../telemetry.js", () => ({
|
||||
|
|
@ -85,6 +86,7 @@ vi.mock("../services/index.js", () => ({
|
|||
|
||||
vi.mock("../adapters/index.js", () => ({
|
||||
findServerAdapter: vi.fn(() => mockAdapter),
|
||||
findActiveServerAdapter: vi.fn(() => mockAdapter),
|
||||
listAdapterModels: vi.fn(),
|
||||
detectAdapterModel: vi.fn(),
|
||||
}));
|
||||
|
|
|
|||
|
|
@ -4,14 +4,14 @@ import { notifyHireApproved } from "../services/hire-hook.js";
|
|||
|
||||
// Mock the registry so we control whether the adapter has onHireApproved and what it does.
|
||||
vi.mock("../adapters/registry.js", () => ({
|
||||
findServerAdapter: vi.fn(),
|
||||
findActiveServerAdapter: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../services/activity-log.js", () => ({
|
||||
logActivity: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
const { findServerAdapter } = await import("../adapters/registry.js");
|
||||
const { findActiveServerAdapter } = await import("../adapters/registry.js");
|
||||
const { logActivity } = await import("../services/activity-log.js");
|
||||
|
||||
function mockDbWithAgent(agent: { id: string; companyId: string; name: string; adapterType: string; adapterConfig?: Record<string, unknown> }): Db {
|
||||
|
|
@ -39,7 +39,7 @@ afterEach(() => {
|
|||
|
||||
describe("notifyHireApproved", () => {
|
||||
it("writes success activity when adapter hook returns ok", async () => {
|
||||
vi.mocked(findServerAdapter).mockReturnValue({
|
||||
vi.mocked(findActiveServerAdapter).mockReturnValue({
|
||||
type: "openclaw_gateway",
|
||||
onHireApproved: vi.fn().mockResolvedValue({ ok: true }),
|
||||
} as any);
|
||||
|
|
@ -88,11 +88,11 @@ describe("notifyHireApproved", () => {
|
|||
}),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
expect(findServerAdapter).not.toHaveBeenCalled();
|
||||
expect(findActiveServerAdapter).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does nothing when adapter has no onHireApproved", async () => {
|
||||
vi.mocked(findServerAdapter).mockReturnValue({ type: "process" } as any);
|
||||
vi.mocked(findActiveServerAdapter).mockReturnValue({ type: "process" } as any);
|
||||
|
||||
const db = mockDbWithAgent({
|
||||
id: "a1",
|
||||
|
|
@ -110,12 +110,12 @@ describe("notifyHireApproved", () => {
|
|||
}),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
expect(findServerAdapter).toHaveBeenCalledWith("process");
|
||||
expect(findActiveServerAdapter).toHaveBeenCalledWith("process");
|
||||
expect(logActivity).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("logs failed result when adapter onHireApproved returns ok=false", async () => {
|
||||
vi.mocked(findServerAdapter).mockReturnValue({
|
||||
vi.mocked(findActiveServerAdapter).mockReturnValue({
|
||||
type: "openclaw_gateway",
|
||||
onHireApproved: vi.fn().mockResolvedValue({ ok: false, error: "HTTP 500", detail: { status: 500 } }),
|
||||
} as any);
|
||||
|
|
@ -147,7 +147,7 @@ describe("notifyHireApproved", () => {
|
|||
});
|
||||
|
||||
it("does not throw when adapter onHireApproved throws (non-fatal)", async () => {
|
||||
vi.mocked(findServerAdapter).mockReturnValue({
|
||||
vi.mocked(findActiveServerAdapter).mockReturnValue({
|
||||
type: "openclaw_gateway",
|
||||
onHireApproved: vi.fn().mockRejectedValue(new Error("Network error")),
|
||||
} as any);
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ export {
|
|||
listAdapterModels,
|
||||
listServerAdapters,
|
||||
findServerAdapter,
|
||||
findActiveServerAdapter,
|
||||
detectAdapterModel,
|
||||
registerServerAdapter,
|
||||
unregisterServerAdapter,
|
||||
|
|
|
|||
|
|
@ -278,16 +278,33 @@ export function waitForExternalAdapters(): Promise<void> {
|
|||
}
|
||||
|
||||
export function registerServerAdapter(adapter: ServerAdapterModule): void {
|
||||
if (BUILTIN_ADAPTER_TYPES.has(adapter.type) && !builtinFallbacks.has(adapter.type)) {
|
||||
const existing = adaptersByType.get(adapter.type);
|
||||
if (existing) {
|
||||
builtinFallbacks.set(adapter.type, existing);
|
||||
}
|
||||
}
|
||||
adaptersByType.set(adapter.type, adapter);
|
||||
}
|
||||
|
||||
export function unregisterServerAdapter(type: string): void {
|
||||
if (type === processAdapter.type || type === httpAdapter.type) return;
|
||||
if (builtinFallbacks.has(type)) {
|
||||
pausedOverrides.delete(type);
|
||||
const fallback = builtinFallbacks.get(type);
|
||||
if (fallback) {
|
||||
adaptersByType.set(type, fallback);
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (BUILTIN_ADAPTER_TYPES.has(type)) {
|
||||
return;
|
||||
}
|
||||
adaptersByType.delete(type);
|
||||
}
|
||||
|
||||
export function requireServerAdapter(type: string): ServerAdapterModule {
|
||||
const adapter = adaptersByType.get(type);
|
||||
const adapter = findActiveServerAdapter(type);
|
||||
if (!adapter) {
|
||||
throw new Error(`Unknown adapter type: ${type}`);
|
||||
}
|
||||
|
|
@ -295,15 +312,11 @@ export function requireServerAdapter(type: string): ServerAdapterModule {
|
|||
}
|
||||
|
||||
export function getServerAdapter(type: string): ServerAdapterModule {
|
||||
if (pausedOverrides.has(type)) {
|
||||
const fallback = builtinFallbacks.get(type);
|
||||
if (fallback) return fallback;
|
||||
}
|
||||
return adaptersByType.get(type) ?? processAdapter;
|
||||
return findActiveServerAdapter(type) ?? processAdapter;
|
||||
}
|
||||
|
||||
export async function listAdapterModels(type: string): Promise<{ id: string; label: string }[]> {
|
||||
const adapter = adaptersByType.get(type);
|
||||
const adapter = findActiveServerAdapter(type);
|
||||
if (!adapter) return [];
|
||||
if (adapter.listModels) {
|
||||
const discovered = await adapter.listModels();
|
||||
|
|
@ -332,7 +345,7 @@ export function listEnabledServerAdapters(): ServerAdapterModule[] {
|
|||
export async function detectAdapterModel(
|
||||
type: string,
|
||||
): Promise<{ model: string; provider: string; source: string; candidates?: string[] } | null> {
|
||||
const adapter = adaptersByType.get(type);
|
||||
const adapter = findActiveServerAdapter(type);
|
||||
if (!adapter?.detectModel) return null;
|
||||
const detected = await adapter.detectModel();
|
||||
if (!detected) return null;
|
||||
|
|
@ -390,3 +403,11 @@ export function getPausedOverrides(): Set<string> {
|
|||
export function findServerAdapter(type: string): ServerAdapterModule | null {
|
||||
return adaptersByType.get(type) ?? null;
|
||||
}
|
||||
|
||||
export function findActiveServerAdapter(type: string): ServerAdapterModule | null {
|
||||
if (pausedOverrides.has(type)) {
|
||||
const fallback = builtinFallbacks.get(type);
|
||||
if (fallback) return fallback;
|
||||
}
|
||||
return adaptersByType.get(type) ?? null;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ import { Router } from "express";
|
|||
import {
|
||||
listServerAdapters,
|
||||
findServerAdapter,
|
||||
findActiveServerAdapter,
|
||||
listEnabledServerAdapters,
|
||||
registerServerAdapter,
|
||||
unregisterServerAdapter,
|
||||
|
|
@ -593,7 +594,7 @@ export function adapterRoutes() {
|
|||
assertBoard(req);
|
||||
const { type } = req.params;
|
||||
|
||||
const adapter = findServerAdapter(type);
|
||||
const adapter = findActiveServerAdapter(type);
|
||||
if (!adapter) {
|
||||
res.status(404).json({ error: `Adapter "${type}" is not registered.` });
|
||||
return;
|
||||
|
|
|
|||
|
|
@ -48,6 +48,7 @@ import { conflict, forbidden, notFound, unprocessable } from "../errors.js";
|
|||
import { assertBoard, assertCompanyAccess, assertInstanceAdmin, getActorInfo } from "./authz.js";
|
||||
import {
|
||||
detectAdapterModel,
|
||||
findActiveServerAdapter,
|
||||
findServerAdapter,
|
||||
listAdapterModels,
|
||||
requireServerAdapter,
|
||||
|
|
@ -820,7 +821,7 @@ export function agentRoutes(db: Db) {
|
|||
}
|
||||
await assertCanReadConfigurations(req, agent.companyId);
|
||||
|
||||
const adapter = findServerAdapter(agent.adapterType);
|
||||
const adapter = findActiveServerAdapter(agent.adapterType);
|
||||
if (!adapter?.listSkills) {
|
||||
const preference = readPaperclipSkillSyncPreference(
|
||||
agent.adapterConfig as Record<string, unknown>,
|
||||
|
|
@ -898,7 +899,7 @@ export function agentRoutes(db: Db) {
|
|||
return;
|
||||
}
|
||||
|
||||
const adapter = findServerAdapter(updated.adapterType);
|
||||
const adapter = findActiveServerAdapter(updated.adapterType);
|
||||
const { config: runtimeConfig } = await secretsSvc.resolveAdapterConfigForRuntime(
|
||||
updated.companyId,
|
||||
updated.adapterConfig,
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ import type {
|
|||
CompanySkillUsageAgent,
|
||||
} from "@paperclipai/shared";
|
||||
import { normalizeAgentUrlKey } from "@paperclipai/shared";
|
||||
import { findServerAdapter } from "../adapters/index.js";
|
||||
import { findActiveServerAdapter } from "../adapters/index.js";
|
||||
import { resolvePaperclipInstanceRoot } from "../home-paths.js";
|
||||
import { notFound, unprocessable } from "../errors.js";
|
||||
import { ghFetch, gitHubApiBase, resolveRawGitHubUrl } from "./github-fetch.js";
|
||||
|
|
@ -1575,7 +1575,7 @@ export function companySkillService(db: Db) {
|
|||
|
||||
return Promise.all(
|
||||
desiredAgents.map(async (agent) => {
|
||||
const adapter = findServerAdapter(agent.adapterType);
|
||||
const adapter = findActiveServerAdapter(agent.adapterType);
|
||||
let actualState: string | null = null;
|
||||
|
||||
if (!adapter?.listSkills) {
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ import { and, eq } from "drizzle-orm";
|
|||
import type { Db } from "@paperclipai/db";
|
||||
import { agents } from "@paperclipai/db";
|
||||
import type { HireApprovedPayload } from "@paperclipai/adapter-utils";
|
||||
import { findServerAdapter } from "../adapters/registry.js";
|
||||
import { findActiveServerAdapter } from "../adapters/registry.js";
|
||||
import { logger } from "../middleware/logger.js";
|
||||
import { logActivity } from "./activity-log.js";
|
||||
|
||||
|
|
@ -40,7 +40,7 @@ export async function notifyHireApproved(
|
|||
}
|
||||
|
||||
const adapterType = row.adapterType ?? "process";
|
||||
const adapter = findServerAdapter(adapterType);
|
||||
const adapter = findActiveServerAdapter(adapterType);
|
||||
const onHireApproved = adapter?.onHireApproved;
|
||||
if (!onHireApproved) {
|
||||
return;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue