forked from farhoodlabs/paperclip
[codex] Improve local plugin development workflow (#5821)
## Thinking Path > - Paperclip is the control plane for autonomous AI-agent companies. > - Plugins are the extension point for adding capabilities without expanding the core product surface. > - Local plugin development needed a tighter CLI-first loop so plugin authors can scaffold, run, install, inspect, and reload plugins without reaching into internal package paths. > - The server plugin install path also needed local-path handling that keeps plugin identity, dashboard routes, and development watchers coherent. > - This pull request adds the CLI scaffold/install workflow, fixes the server and SDK edge cases that blocked that loop, and updates the agent-facing plugin creation skill and docs. > - The benefit is that contributors can develop plugins from local folders with a documented, repeatable happy path. ## What Changed - Added `paperclipai plugin init` coverage and CLI wiring for local plugin scaffolding. - Improved local plugin install handling, plugin key route resolution, dashboard capability behavior, and dev watcher startup/reload behavior. - Fixed plugin SDK worker entrypoint validation for symlinked package layouts. - Added targeted tests for plugin init, server plugin authz/watcher behavior, SDK worker host validation, and the authoring smoke example. - Added a short local plugin development guide and refreshed the plugin authoring guide plus `paperclip-create-plugin` skill instructions. ## Verification - `pnpm run preflight:workspace-links && pnpm --filter @paperclipai/plugin-sdk build && pnpm --filter @paperclipai/create-paperclip-plugin typecheck && pnpm --filter paperclipai typecheck && pnpm --filter @paperclipai/plugin-sdk typecheck && pnpm --filter @paperclipai/server typecheck` - `pnpm exec vitest run --project paperclipai cli/src/__tests__/plugin-init.test.ts` - `pnpm exec vitest run --project @paperclipai/plugin-sdk packages/plugins/sdk/tests/worker-rpc-host.test.ts` - `pnpm exec vitest run --project @paperclipai/server server/src/__tests__/plugin-dev-watcher.test.ts --pool=forks --poolOptions.forks.isolate=true` - `pnpm exec vitest run --project @paperclipai/server server/src/__tests__/plugin-routes-authz.test.ts --pool=forks --poolOptions.forks.isolate=true` - `pnpm --dir packages/plugins/examples/plugin-authoring-smoke-example test` - Confirmed `pnpm-lock.yaml` is not included in the PR diff. ## Risks - Medium risk: this touches plugin install routing, CLI command behavior, and the local development watcher. - Local path plugin installs execute trusted local code by design; the new docs call out that trust boundary. - No database migrations are included. > 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 coding agent, tool-enabled local shell and git workflow, medium reasoning effort. Context window details were not exposed in this runtime. ## 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 UI screenshots: not applicable; this PR changes CLI/server/plugin docs and tests, not board UI rendering. --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
@@ -1,12 +1,28 @@
|
||||
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { EventEmitter } from "node:events";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { resolvePluginWatchTargets } from "../services/plugin-dev-watcher.js";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const chokidarMock = vi.hoisted(() => ({
|
||||
watch: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("chokidar", () => ({
|
||||
default: chokidarMock,
|
||||
}));
|
||||
|
||||
import { createPluginDevWatcher, resolvePluginWatchTargets } from "../services/plugin-dev-watcher.js";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useRealTimers();
|
||||
chokidarMock.watch.mockReset();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
while (tempDirs.length > 0) {
|
||||
const dir = tempDirs.pop();
|
||||
if (dir) rmSync(dir, { recursive: true, force: true });
|
||||
@@ -19,25 +35,49 @@ function makeTempPluginDir(): string {
|
||||
return dir;
|
||||
}
|
||||
|
||||
function writePluginPackage(pluginDir: string): void {
|
||||
mkdirSync(path.join(pluginDir, "dist", "ui"), { recursive: true });
|
||||
writeFileSync(
|
||||
path.join(pluginDir, "package.json"),
|
||||
JSON.stringify({
|
||||
name: "@acme/example",
|
||||
paperclipPlugin: {
|
||||
manifest: "./dist/manifest.js",
|
||||
worker: "./dist/worker.js",
|
||||
ui: "./dist/ui",
|
||||
},
|
||||
}),
|
||||
);
|
||||
writeFileSync(path.join(pluginDir, "dist", "manifest.js"), "export default {};\n");
|
||||
writeFileSync(path.join(pluginDir, "dist", "worker.js"), "export default {};\n");
|
||||
writeFileSync(path.join(pluginDir, "dist", "ui", "index.js"), "export default {};\n");
|
||||
writeFileSync(path.join(pluginDir, "dist", "ui", "index.css"), "body {}\n");
|
||||
}
|
||||
|
||||
function createLifecycle() {
|
||||
const emitter = new EventEmitter();
|
||||
return Object.assign(emitter, {
|
||||
restartWorker: vi.fn().mockResolvedValue(undefined),
|
||||
});
|
||||
}
|
||||
|
||||
function installMockFsWatcher() {
|
||||
const handlers: Record<string, (...args: unknown[]) => void> = {};
|
||||
const fakeWatcher = {
|
||||
close: vi.fn(),
|
||||
on: vi.fn((event: string, listener: (...args: unknown[]) => void) => {
|
||||
handlers[event] = listener;
|
||||
return fakeWatcher;
|
||||
}),
|
||||
};
|
||||
chokidarMock.watch.mockReturnValue(fakeWatcher);
|
||||
return { fakeWatcher, handlers };
|
||||
}
|
||||
|
||||
describe("resolvePluginWatchTargets", () => {
|
||||
it("watches package metadata plus concrete declared runtime files", () => {
|
||||
const pluginDir = makeTempPluginDir();
|
||||
mkdirSync(path.join(pluginDir, "dist", "ui"), { recursive: true });
|
||||
writeFileSync(
|
||||
path.join(pluginDir, "package.json"),
|
||||
JSON.stringify({
|
||||
name: "@acme/example",
|
||||
paperclipPlugin: {
|
||||
manifest: "./dist/manifest.js",
|
||||
worker: "./dist/worker.js",
|
||||
ui: "./dist/ui",
|
||||
},
|
||||
}),
|
||||
);
|
||||
writeFileSync(path.join(pluginDir, "dist", "manifest.js"), "export default {};\n");
|
||||
writeFileSync(path.join(pluginDir, "dist", "worker.js"), "export default {};\n");
|
||||
writeFileSync(path.join(pluginDir, "dist", "ui", "index.js"), "export default {};\n");
|
||||
writeFileSync(path.join(pluginDir, "dist", "ui", "index.css"), "body {}\n");
|
||||
writePluginPackage(pluginDir);
|
||||
|
||||
const targets = resolvePluginWatchTargets(pluginDir);
|
||||
|
||||
@@ -66,3 +106,43 @@ describe("resolvePluginWatchTargets", () => {
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("createPluginDevWatcher", () => {
|
||||
it("starts watching local plugins announced by lifecycle events", async () => {
|
||||
const pluginDir = makeTempPluginDir();
|
||||
writePluginPackage(pluginDir);
|
||||
installMockFsWatcher();
|
||||
const lifecycle = createLifecycle();
|
||||
|
||||
const devWatcher = createPluginDevWatcher(
|
||||
lifecycle as never,
|
||||
async (pluginId) => (pluginId === "plugin-1" ? pluginDir : null),
|
||||
);
|
||||
|
||||
lifecycle.emit("plugin.loaded", { pluginId: "plugin-1", pluginKey: "example" });
|
||||
|
||||
await vi.waitFor(() => expect(chokidarMock.watch).toHaveBeenCalledTimes(1));
|
||||
const [watchedPaths] = chokidarMock.watch.mock.calls[0] ?? [];
|
||||
expect(watchedPaths).toContain(path.join(pluginDir, "dist", "worker.js"));
|
||||
|
||||
devWatcher.close();
|
||||
});
|
||||
|
||||
it("debounces watched file changes and restarts the plugin worker", async () => {
|
||||
vi.useFakeTimers();
|
||||
const pluginDir = makeTempPluginDir();
|
||||
writePluginPackage(pluginDir);
|
||||
const { handlers } = installMockFsWatcher();
|
||||
const lifecycle = createLifecycle();
|
||||
|
||||
const devWatcher = createPluginDevWatcher(lifecycle as never);
|
||||
devWatcher.watch("plugin-1", pluginDir);
|
||||
|
||||
handlers.all?.("change", path.join(pluginDir, "dist", "worker.js"));
|
||||
await vi.advanceTimersByTimeAsync(500);
|
||||
|
||||
expect(lifecycle.restartWorker).toHaveBeenCalledWith("plugin-1");
|
||||
|
||||
devWatcher.close();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -224,6 +224,46 @@ describe.sequential("plugin install and upgrade authz", () => {
|
||||
expect(mockLifecycle.disable).not.toHaveBeenCalled();
|
||||
}, 20_000);
|
||||
|
||||
it("resolves plugin keys without probing the UUID id column for core plugin actions", async () => {
|
||||
const pluginKey = "paperclipqa.hello-plugin";
|
||||
const plugin = {
|
||||
id: pluginId,
|
||||
pluginKey,
|
||||
version: "1.0.0",
|
||||
status: "ready",
|
||||
};
|
||||
mockRegistry.getById.mockImplementation(() => {
|
||||
throw new Error("getById should not be called for plugin keys");
|
||||
});
|
||||
mockRegistry.getByKey.mockResolvedValue(plugin);
|
||||
mockLifecycle.unload.mockResolvedValue(plugin);
|
||||
mockLifecycle.enable.mockResolvedValue(plugin);
|
||||
mockLifecycle.disable.mockResolvedValue(plugin);
|
||||
|
||||
const { app } = await createApp({
|
||||
type: "board",
|
||||
userId: "admin-1",
|
||||
source: "session",
|
||||
isInstanceAdmin: true,
|
||||
companyIds: [companyA],
|
||||
});
|
||||
|
||||
const inspectRes = await request(app).get(`/api/plugins/${pluginKey}`);
|
||||
const disableRes = await request(app).post(`/api/plugins/${pluginKey}/disable`).send({});
|
||||
const enableRes = await request(app).post(`/api/plugins/${pluginKey}/enable`).send({});
|
||||
const uninstallRes = await request(app).delete(`/api/plugins/${pluginKey}?purge=true`);
|
||||
|
||||
expect(inspectRes.status).toBe(200);
|
||||
expect(disableRes.status).toBe(200);
|
||||
expect(enableRes.status).toBe(200);
|
||||
expect(uninstallRes.status).toBe(200);
|
||||
expect(mockRegistry.getById).not.toHaveBeenCalled();
|
||||
expect(mockRegistry.getByKey).toHaveBeenCalledWith(pluginKey);
|
||||
expect(mockLifecycle.disable).toHaveBeenCalledWith(pluginId, undefined);
|
||||
expect(mockLifecycle.enable).toHaveBeenCalledWith(pluginId);
|
||||
expect(mockLifecycle.unload).toHaveBeenCalledWith(pluginId, true);
|
||||
}, 20_000);
|
||||
|
||||
it("rejects plugin config saves that contain secret refs even for instance admins", async () => {
|
||||
readyPlugin();
|
||||
|
||||
|
||||
+4
-6
@@ -420,12 +420,10 @@ export async function createApp(
|
||||
void toolDispatcher.initialize().catch((err) => {
|
||||
logger.error({ err }, "Failed to initialize plugin tool dispatcher");
|
||||
});
|
||||
const devWatcher = opts.uiMode === "vite-dev"
|
||||
? createPluginDevWatcher(
|
||||
lifecycle,
|
||||
async (pluginId) => (await pluginRegistry.getById(pluginId))?.packagePath ?? null,
|
||||
)
|
||||
: null;
|
||||
const devWatcher = createPluginDevWatcher(
|
||||
lifecycle,
|
||||
async (pluginId) => (await pluginRegistry.getById(pluginId))?.packagePath ?? null,
|
||||
);
|
||||
void loader.loadAll().then((result) => {
|
||||
if (!result) return;
|
||||
for (const loaded of result.results) {
|
||||
|
||||
@@ -199,9 +199,9 @@ function listBundledPluginExamples(): AvailablePluginExample[] {
|
||||
*
|
||||
* Lookup order:
|
||||
* - UUID-like IDs: getById first, then getByKey.
|
||||
* - Scoped package keys (e.g. "@scope/name"): getByKey only, never getById.
|
||||
* - Other non-UUID IDs: try getById first (test/memory registries may allow this),
|
||||
* then fallback to getByKey. Any UUID parse error from getById is ignored.
|
||||
* - All non-UUID values: getByKey only, never getById. The persisted plugin
|
||||
* ID column is a PostgreSQL UUID, so probing it with keys such as
|
||||
* "acme.plugin" raises a database cast error before a key lookup can happen.
|
||||
*
|
||||
* @param registry - The plugin registry service instance
|
||||
* @param pluginId - Either a database UUID or plugin key (manifest id)
|
||||
@@ -212,27 +212,13 @@ async function resolvePlugin(
|
||||
pluginId: string,
|
||||
) {
|
||||
const isUuid = UUID_REGEX.test(pluginId);
|
||||
const isScopedPackageKey = pluginId.startsWith("@") || pluginId.includes("/");
|
||||
|
||||
// Scoped package IDs are valid plugin keys but invalid UUIDs.
|
||||
// Skip getById() entirely to avoid Postgres uuid parse errors.
|
||||
if (isScopedPackageKey && !isUuid) {
|
||||
if (!isUuid) {
|
||||
return registry.getByKey(pluginId);
|
||||
}
|
||||
|
||||
try {
|
||||
const byId = await registry.getById(pluginId);
|
||||
if (byId) return byId;
|
||||
} catch (error) {
|
||||
const maybeCode =
|
||||
typeof error === "object" && error !== null && "code" in error
|
||||
? (error as { code?: unknown }).code
|
||||
: undefined;
|
||||
// Ignore invalid UUID cast errors and continue with key lookup.
|
||||
if (maybeCode !== "22P02") {
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
const byId = await registry.getById(pluginId);
|
||||
if (byId) return byId;
|
||||
|
||||
return registry.getByKey(pluginId);
|
||||
}
|
||||
|
||||
@@ -164,6 +164,10 @@ export function createPluginDevWatcher(
|
||||
const watchers = new Map<string, FSWatcher>();
|
||||
const debounceTimers = new Map<string, ReturnType<typeof setTimeout>>();
|
||||
const fileExists = fsDeps?.existsSync ?? existsSync;
|
||||
log.info(
|
||||
{ resolvesInstalledPlugins: Boolean(resolvePluginPackagePath) },
|
||||
"plugin-dev-watcher: initialized",
|
||||
);
|
||||
|
||||
function watchPlugin(pluginId: string, packagePath: string): void {
|
||||
// Don't double-watch
|
||||
@@ -293,11 +297,23 @@ export function createPluginDevWatcher(
|
||||
}
|
||||
|
||||
async function watchLocalPluginById(pluginId: string): Promise<void> {
|
||||
if (!resolvePluginPackagePath) return;
|
||||
if (!resolvePluginPackagePath) {
|
||||
log.debug(
|
||||
{ pluginId },
|
||||
"plugin-dev-watcher: no package path resolver configured, skipping lifecycle event",
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
const packagePath = await resolvePluginPackagePath(pluginId);
|
||||
if (!packagePath) return;
|
||||
if (!packagePath) {
|
||||
log.debug(
|
||||
{ pluginId },
|
||||
"plugin-dev-watcher: plugin is not a local-path install, skipping watch",
|
||||
);
|
||||
return;
|
||||
}
|
||||
watchPlugin(pluginId, packagePath);
|
||||
} catch (err) {
|
||||
log.warn(
|
||||
|
||||
Reference in New Issue
Block a user