mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-14 01:50:39 +09:00
Improve ACPX adapter configuration (#5290)
## Thinking Path > - Paperclip orchestrates AI agents across several adapter implementations. > - ACPX is a local adapter path that can proxy Claude and Codex-style execution. > - Its configuration needed stronger schema defaults, provider-aware model handling, and better UI support. > - Plugin authors also need clear docs for managed resources. > - This pull request improves ACPX adapter configuration and documents plugin-managed resources. > - The benefit is a more predictable adapter setup path without changing unrelated control-plane behavior. ## What Changed - Improved ACPX config schema, execution config handling, UI build config, and route coverage. - Added ACPX model filtering support and tests. - Updated the agent config form and storybook coverage for ACPX model/provider behavior. - Expanded plugin authoring documentation for managed resources. ## Verification - `pnpm install --frozen-lockfile` - `pnpm exec vitest run server/src/__tests__/acpx-local-execute.test.ts server/src/__tests__/adapter-routes.test.ts ui/src/lib/acpx-model-filter.test.ts` ## Risks - Low-to-medium risk: adapter configuration behavior changes can affect ACPX users, but the change is isolated to ACPX/plugin-doc surfaces and covered by targeted adapter tests. ## Model Used - OpenAI GPT-5 Codex via Paperclip `codex_local` adapter, with shell/git/GitHub CLI 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 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] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
454edfe81e
commit
11ffd6f2c5
15 changed files with 949 additions and 211 deletions
|
|
@ -1,4 +1,4 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
|
@ -215,6 +215,103 @@ describe("acpx_local execute", () => {
|
|||
}
|
||||
});
|
||||
|
||||
it("closes successful persistent runs by default while retaining session state", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-acpx-close-success-"));
|
||||
try {
|
||||
const runtime = new FakeRuntime({} as AcpRuntimeOptions);
|
||||
const execute = createAcpxLocalExecutor({
|
||||
createRuntime: () => runtime,
|
||||
});
|
||||
const result = await execute(buildContext(root));
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.sessionParams).toMatchObject({
|
||||
mode: "persistent",
|
||||
acpSessionId: "acp-1",
|
||||
});
|
||||
expect(runtime.closeInputs).toEqual([
|
||||
expect.objectContaining({
|
||||
reason: "paperclip completed turn cleanup",
|
||||
discardPersistentState: false,
|
||||
}),
|
||||
]);
|
||||
} finally {
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("applies requested Codex model, reasoning effort, and fast mode before starting the turn", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-acpx-codex-config-"));
|
||||
try {
|
||||
const runtime = new FakeRuntime({} as AcpRuntimeOptions);
|
||||
const execute = createAcpxLocalExecutor({
|
||||
createRuntime: () => runtime,
|
||||
});
|
||||
const result = await execute(buildContext(root, {
|
||||
config: {
|
||||
agent: "codex",
|
||||
cwd: root,
|
||||
stateDir: path.join(root, "state"),
|
||||
promptTemplate: "Do the assigned work.",
|
||||
model: "gpt-5.4",
|
||||
modelReasoningEffort: "xhigh",
|
||||
fastMode: true,
|
||||
},
|
||||
}));
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.model).toBe("gpt-5.4");
|
||||
expect(runtime.setConfigInputs).toEqual([
|
||||
expect.objectContaining({ key: "model", value: "gpt-5.4" }),
|
||||
expect.objectContaining({ key: "reasoning_effort", value: "xhigh" }),
|
||||
expect.objectContaining({ key: "service_tier", value: "fast" }),
|
||||
expect.objectContaining({ key: "features.fast_mode", value: "true" }),
|
||||
]);
|
||||
expect(runtime.startInputs).toHaveLength(1);
|
||||
} finally {
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("logs a clear error when configured session options need unsupported runtime controls", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-acpx-missing-config-controls-"));
|
||||
try {
|
||||
const runtime = new FakeRuntime({} as AcpRuntimeOptions);
|
||||
Object.defineProperty(runtime, "setConfigOption", { value: undefined });
|
||||
const logs: LogEntry[] = [];
|
||||
const execute = createAcpxLocalExecutor({
|
||||
createRuntime: () => runtime,
|
||||
});
|
||||
const result = await execute(buildContext(root, {
|
||||
config: {
|
||||
agent: "codex",
|
||||
cwd: root,
|
||||
stateDir: path.join(root, "state"),
|
||||
promptTemplate: "Do the assigned work.",
|
||||
model: "gpt-5.4",
|
||||
},
|
||||
onLog: async (stream, chunk) => logs.push({ stream, chunk }),
|
||||
}));
|
||||
|
||||
expect(result.exitCode).toBe(1);
|
||||
expect(result.errorMessage).toContain("does not expose session config controls");
|
||||
expect(logs).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
stream: "stderr",
|
||||
chunk: expect.stringContaining("upgrade ACPX or remove configured model"),
|
||||
}),
|
||||
]));
|
||||
expect(runtime.closeInputs).toEqual([
|
||||
expect.objectContaining({
|
||||
reason: "paperclip config cleanup",
|
||||
discardPersistentState: false,
|
||||
}),
|
||||
]);
|
||||
} finally {
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("reuses a compatible warm session and starts fresh when cwd changes", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-acpx-reuse-"));
|
||||
const other = path.join(root, "other");
|
||||
|
|
@ -228,8 +325,15 @@ describe("acpx_local execute", () => {
|
|||
return runtime;
|
||||
},
|
||||
});
|
||||
const warmConfig = {
|
||||
agent: "claude",
|
||||
cwd: root,
|
||||
stateDir: path.join(root, "state"),
|
||||
promptTemplate: "Do the assigned work.",
|
||||
warmHandleIdleMs: 60_000,
|
||||
};
|
||||
|
||||
const first = await execute(buildContext(root));
|
||||
const first = await execute(buildContext(root, { config: warmConfig }));
|
||||
const second = await execute(buildContext(root, {
|
||||
runtime: {
|
||||
sessionId: first.sessionId ?? null,
|
||||
|
|
@ -237,6 +341,7 @@ describe("acpx_local execute", () => {
|
|||
sessionDisplayId: first.sessionDisplayId ?? null,
|
||||
taskKey: "PAP-1",
|
||||
},
|
||||
config: warmConfig,
|
||||
}));
|
||||
const third = await execute(buildContext(root, {
|
||||
runtime: {
|
||||
|
|
@ -250,6 +355,7 @@ describe("acpx_local execute", () => {
|
|||
cwd: other,
|
||||
stateDir: path.join(root, "state"),
|
||||
promptTemplate: "Do the assigned work.",
|
||||
warmHandleIdleMs: 60_000,
|
||||
},
|
||||
}));
|
||||
|
||||
|
|
@ -279,8 +385,26 @@ describe("acpx_local execute", () => {
|
|||
});
|
||||
|
||||
const [first, second] = await Promise.all([
|
||||
execute(buildContext(root, { runId: "run-1" })),
|
||||
execute(buildContext(root, { runId: "run-2" })),
|
||||
execute(buildContext(root, {
|
||||
runId: "run-1",
|
||||
config: {
|
||||
agent: "claude",
|
||||
cwd: root,
|
||||
stateDir: path.join(root, "state"),
|
||||
promptTemplate: "Do the assigned work.",
|
||||
warmHandleIdleMs: 60_000,
|
||||
},
|
||||
})),
|
||||
execute(buildContext(root, {
|
||||
runId: "run-2",
|
||||
config: {
|
||||
agent: "claude",
|
||||
cwd: root,
|
||||
stateDir: path.join(root, "state"),
|
||||
promptTemplate: "Do the assigned work.",
|
||||
warmHandleIdleMs: 60_000,
|
||||
},
|
||||
})),
|
||||
]);
|
||||
|
||||
expect(first.exitCode).toBe(0);
|
||||
|
|
@ -295,6 +419,47 @@ describe("acpx_local execute", () => {
|
|||
}
|
||||
});
|
||||
|
||||
it("cleans configured warm handles after their idle window", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-acpx-warm-idle-"));
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
let clock = 0;
|
||||
const runtime = new FakeRuntime({} as AcpRuntimeOptions);
|
||||
const warmHandles = new Map();
|
||||
const execute = createAcpxLocalExecutor({
|
||||
warmHandles,
|
||||
now: () => clock,
|
||||
createRuntime: () => runtime,
|
||||
});
|
||||
|
||||
const result = await execute(buildContext(root, {
|
||||
config: {
|
||||
agent: "claude",
|
||||
cwd: root,
|
||||
stateDir: path.join(root, "state"),
|
||||
promptTemplate: "Do the assigned work.",
|
||||
warmHandleIdleMs: 1_000,
|
||||
},
|
||||
}));
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(warmHandles.size).toBe(1);
|
||||
clock = 1_000;
|
||||
await vi.advanceTimersByTimeAsync(1_000);
|
||||
|
||||
expect(warmHandles.size).toBe(0);
|
||||
expect(runtime.closeInputs).toEqual([
|
||||
expect.objectContaining({
|
||||
reason: "paperclip idle cleanup",
|
||||
discardPersistentState: false,
|
||||
}),
|
||||
]);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("retries with a fresh session when ACPX cannot resume the saved backend session", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-acpx-resume-"));
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ import { models as codexFallbackModels } from "@paperclipai/adapter-codex-local"
|
|||
import { models as cursorFallbackModels } from "@paperclipai/adapter-cursor-local";
|
||||
import { models as opencodeFallbackModels } from "@paperclipai/adapter-opencode-local";
|
||||
import { resetOpenCodeModelsCacheForTests } from "@paperclipai/adapter-opencode-local/server";
|
||||
import { listAdapterModels, refreshAdapterModels } from "../adapters/index.js";
|
||||
import { listAdapterModels, listServerAdapters, refreshAdapterModels } from "../adapters/index.js";
|
||||
import { resetCodexModelsCacheForTests } from "../adapters/codex-models.js";
|
||||
import { resetCursorModelsCacheForTests, setCursorModelsRunnerForTests } from "../adapters/cursor-models.js";
|
||||
|
||||
|
|
@ -30,6 +30,13 @@ describe("adapter model listing", () => {
|
|||
expect(models).toEqual([]);
|
||||
});
|
||||
|
||||
it("uses provider-prefixed ACPX fallback model labels", () => {
|
||||
const adapter = listServerAdapters().find((candidate) => candidate.type === "acpx_local");
|
||||
|
||||
expect(adapter?.models?.some((model) => model.label.startsWith("Claude: "))).toBe(true);
|
||||
expect(adapter?.models?.some((model) => model.label.startsWith("Codex: "))).toBe(true);
|
||||
});
|
||||
|
||||
it("returns codex fallback models when no OpenAI key is available", async () => {
|
||||
const fetchSpy = vi.spyOn(globalThis, "fetch");
|
||||
const models = await listAdapterModels("codex_local");
|
||||
|
|
|
|||
|
|
@ -248,11 +248,33 @@ describe("adapter routes", () => {
|
|||
]),
|
||||
}),
|
||||
expect.objectContaining({
|
||||
key: "permissionMode",
|
||||
default: "approve-all",
|
||||
key: "fastMode",
|
||||
default: false,
|
||||
meta: { visibleWhen: { key: "agent", values: ["codex"] } },
|
||||
}),
|
||||
expect.objectContaining({
|
||||
key: "warmHandleIdleMs",
|
||||
default: 0,
|
||||
}),
|
||||
]),
|
||||
);
|
||||
const keys = res.body.fields.map((field: { key: string }) => field.key);
|
||||
expect(keys).not.toContain("mode");
|
||||
expect(keys).not.toContain("permissionMode");
|
||||
expect(keys).not.toContain("instructionsFilePath");
|
||||
expect(keys).not.toContain("promptTemplate");
|
||||
expect(keys).not.toContain("bootstrapPromptTemplate");
|
||||
});
|
||||
|
||||
it("GET /api/adapters includes ACPX model availability", async () => {
|
||||
const app = createApp();
|
||||
|
||||
const res = await request(app).get("/api/adapters");
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||
const acpxLocal = res.body.find((a: any) => a.type === "acpx_local");
|
||||
expect(acpxLocal).toBeDefined();
|
||||
expect(acpxLocal.modelsCount).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("rejects signed-in users without org access", async () => {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,9 @@
|
|||
import type { AdapterModelProfileDefinition, AdapterRuntimeCommandSpec, ServerAdapterModule } from "./types.js";
|
||||
import type {
|
||||
AdapterModel,
|
||||
AdapterModelProfileDefinition,
|
||||
AdapterRuntimeCommandSpec,
|
||||
ServerAdapterModule,
|
||||
} from "./types.js";
|
||||
import { getAdapterSessionManagement } from "@paperclipai/adapter-utils";
|
||||
import {
|
||||
execute as acpxExecute,
|
||||
|
|
@ -8,7 +13,10 @@ import {
|
|||
listAcpxSkills,
|
||||
syncAcpxSkills,
|
||||
} from "@paperclipai/adapter-acpx-local/server";
|
||||
import { agentConfigurationDoc as acpxAgentConfigurationDoc } from "@paperclipai/adapter-acpx-local";
|
||||
import {
|
||||
agentConfigurationDoc as acpxAgentConfigurationDoc,
|
||||
models as acpxModels,
|
||||
} from "@paperclipai/adapter-acpx-local";
|
||||
import {
|
||||
execute as claudeExecute,
|
||||
listClaudeSkills,
|
||||
|
|
@ -182,6 +190,38 @@ function normalizeHermesConfig<T extends { config?: unknown; agent?: unknown }>(
|
|||
return ctx;
|
||||
}
|
||||
|
||||
function dedupeAdapterModels(models: AdapterModel[]): AdapterModel[] {
|
||||
const seen = new Set<string>();
|
||||
const result: AdapterModel[] = [];
|
||||
for (const model of models) {
|
||||
const id = model.id.trim();
|
||||
if (!id || seen.has(id)) continue;
|
||||
seen.add(id);
|
||||
result.push({ ...model, id });
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
function prefixAdapterModelLabels(models: AdapterModel[], provider: "Claude" | "Codex"): AdapterModel[] {
|
||||
const prefix = `${provider}: `;
|
||||
return models.map((model) => ({
|
||||
...model,
|
||||
label: model.label.startsWith(prefix) ? model.label : `${prefix}${model.label}`,
|
||||
}));
|
||||
}
|
||||
|
||||
async function listAcpxModels(): Promise<AdapterModel[]> {
|
||||
const [claude, codex] = await Promise.all([
|
||||
listClaudeModels().catch(() => claudeModels),
|
||||
listCodexModels().catch(() => codexModels),
|
||||
]);
|
||||
return dedupeAdapterModels([
|
||||
...acpxModels,
|
||||
...prefixAdapterModelLabels(claude, "Claude"),
|
||||
...prefixAdapterModelLabels(codex, "Codex"),
|
||||
]);
|
||||
}
|
||||
|
||||
const claudeLocalAdapter: ServerAdapterModule = {
|
||||
type: "claude_local",
|
||||
execute: claudeExecute,
|
||||
|
|
@ -211,6 +251,11 @@ const acpxLocalAdapter: ServerAdapterModule = {
|
|||
syncSkills: syncAcpxSkills,
|
||||
sessionCodec: acpxSessionCodec,
|
||||
sessionManagement: getAdapterSessionManagement("acpx_local") ?? undefined,
|
||||
models: dedupeAdapterModels([
|
||||
...prefixAdapterModelLabels(claudeModels, "Claude"),
|
||||
...prefixAdapterModelLabels(codexModels, "Codex"),
|
||||
]),
|
||||
listModels: listAcpxModels,
|
||||
supportsLocalAgentJwt: true,
|
||||
supportsInstructionsBundle: true,
|
||||
instructionsPathKey: "instructionsFilePath",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue