mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 12:59:13 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Sandbox execution selects a provider from installed plugin manifests. > - Driver-key lookup reads every plugin row before it finds a matching declaration. > - Repeated lookups transfer manifests for unrelated plugins. > - The registry owns this database read and can filter candidates before returning rows. > - This pull request adds that filter and keeps the existing provider selection checks. > - The returned data now grows with matching providers instead of the full catalog. ## Linked Issues or Issue Description **Problem or motivation** `resolvePluginSandboxProviderDriverByKey` calls the unfiltered registry catalog on each lookup. The caller only needs plugins with a sandbox-provider declaration for one driver key. Unrelated plugin manifests still cross the database connection and are decoded in the server. **Proposed solution** Add an optional sandbox-provider driver-key filter to `pluginRegistryService().list`. Match the key and kind on the same JSON declaration in Postgres. Pass the key from the existing resolver. Keep full matching rows, install order, status checks and worker checks. **Alternatives considered** A catalog cache would need an invalidation contract for plugin updates and independent writers. This change keeps fresh database reads. Returning only one SQL row could skip a later eligible provider when an earlier worker is unavailable. The filter therefore keeps all matching candidates. **Roadmap alignment** This is a small performance fix to existing plugin execution. It adds no product feature or provider. Searches for sandbox/driver lookup and plugin catalog/filter work found no duplicate PR or issue. ## What Changed - Add an optional JSONB containment filter to the existing registry list read. - Use it in sandbox-provider lookup by driver key. The unfiltered catalog call keeps its existing behavior. - Add real-Postgres regression coverage and a short developer note. ## Verification - Four focused provider-selection/recovery/seam suites passed: 27 tests. - `pnpm build`: passed. - `pnpm -r typecheck`: passed. - The new Postgres tests seed large unrelated manifests and verify the actual resolver SQL filters rows in the database. They cover key/kind matching within one declaration, unfiltered reads, install order, disabled and unavailable workers, live status/declaration changes, unknown keys and bound unusual input. - The filter keeps the existing SQL call count. It does not add a network call, cache, timer, or background worker. - `git diff --check`: passed. Reviewed the staged diff for secrets and private account data before push. - Full PR CI passed on `6f97c984a931416b6347e9885d840557c54d8aab`: 54 successful checks and two expected skips. [Workflow evidence](https://github.com/paperclipai/paperclip/actions/runs/40041294925). - Apex review: 5/5 on that exact head, completed cleanly, with no comments or unresolved threads. Merge state is clean. - The prior local full run on parent work is documented in #17384. It did not pass because of test setup, host Git-default assumptions and transient failures. Those results were preserved and triaged against unchanged code. This change uses focused local checks and full current-head CI; no new local full-suite pass is claimed. - No hosted user journey, live provider throughput, production deployment or billed savings is claimed. ## Risks - JSONB containment checks the stored manifest before returning rows. It still scans candidate metadata in the database; no index or SQL-call reduction is claimed. - Both key and kind must match the same declaration. The regression fixture includes a plugin with those values on different declarations. - Selection still uses the existing install-order and live readiness/worker checks. Matching disabled records remain visible to callers that do not require a running worker. - No schema, API response, provider protocol, file-copy behavior or scheduling changes. ## Model Used OpenAI Codex, GPT-6 family. The exact deployment model ID and context window are not exposed in this session. Used reasoning, repository tools and code execution. ## 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 searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
96 lines
5.6 KiB
TypeScript
96 lines
5.6 KiB
TypeScript
import { randomBytes, randomUUID } from "node:crypto";
|
|
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { eq } from "drizzle-orm";
|
|
import { drizzle } from "drizzle-orm/postgres-js";
|
|
import { createDb, plugins } from "@paperclipai/db";
|
|
import type { PaperclipPluginManifestV1, PluginEnvironmentDriverDeclaration } from "@paperclipai/shared";
|
|
import { pluginRegistryService } from "../services/plugin-registry.js";
|
|
import { resolvePluginSandboxProviderDriverByKey } from "../services/plugin-environment-driver.js";
|
|
import type { PluginWorkerManager } from "../services/plugin-worker-manager.js";
|
|
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
|
|
|
|
const support = await getEmbeddedPostgresTestSupport();
|
|
const describePostgres = support.supported ? describe : describe.skip;
|
|
|
|
describePostgres("sandbox-provider plugin selection", () => {
|
|
let temporary: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
|
let db: ReturnType<typeof createDb>;
|
|
const key = "fixture-provider";
|
|
const ids = Array.from({ length: 6 }, () => randomUUID());
|
|
const driver = (driverKey: string, kind: PluginEnvironmentDriverDeclaration["kind"] = "sandbox_provider") => ({
|
|
driverKey, kind, displayName: driverKey, configSchema: { type: "object" },
|
|
});
|
|
const manifest = (id: string, environmentDrivers: PluginEnvironmentDriverDeclaration[]): PaperclipPluginManifestV1 => ({
|
|
id: `test.selection.${id}`, apiVersion: 1, version: "1.0.0", displayName: "Selection fixture",
|
|
description: "Fixture", author: "Test", categories: ["automation"],
|
|
capabilities: ["environment.drivers.register"], entrypoints: { worker: "worker.js" }, environmentDrivers,
|
|
});
|
|
const largeDriver = { ...driver("other-provider"), configSchema: { type: "object", description: randomBytes(64 * 1024).toString("hex") } };
|
|
|
|
beforeAll(async () => {
|
|
temporary = await startEmbeddedPostgresTestDatabase("paperclip-provider-selection-");
|
|
db = createDb(temporary.connectionString);
|
|
}, 60_000);
|
|
beforeEach(async () => {
|
|
await db.delete(plugins);
|
|
await db.insert(plugins).values(ids.map((id, index) => ({
|
|
id, pluginKey: `test.selection.${id}`, packageName: "@test/selection", version: "1.0.0",
|
|
installOrder: index, status: index === 1 ? "disabled" as const : "ready" as const,
|
|
manifestJson: manifest(id, index === 0 ? [driver(key, "environment_driver"), largeDriver]
|
|
: index < 4 ? [driver(key)] : index === 4 ? [largeDriver] : []),
|
|
})));
|
|
});
|
|
afterAll(async () => { await temporary?.cleanup(); });
|
|
|
|
it("keeps the unfiltered catalog and filters matching declarations in install order", async () => {
|
|
const registry = pluginRegistryService(db);
|
|
expect((await registry.list()).map(plugin => plugin.id)).toEqual(ids);
|
|
const selected = await registry.list({ sandboxProviderDriverKey: key });
|
|
expect(selected.map(plugin => plugin.id)).toEqual(ids.slice(1, 4));
|
|
expect(selected[0].status).toBe("disabled");
|
|
expect(selected.every(plugin => plugin.manifestJson.environmentDrivers?.some(
|
|
entry => entry.driverKey === key && entry.kind === "sandbox_provider",
|
|
))).toBe(true);
|
|
expect(await registry.list({ sandboxProviderDriverKey: "unknown-provider" })).toEqual([]);
|
|
});
|
|
|
|
it("filters the actual resolver query before large unrelated manifests cross the DB boundary", async () => {
|
|
const statements: string[] = [];
|
|
const recorded = drizzle(db.$client, {
|
|
schema: db._.fullSchema,
|
|
logger: { logQuery: query => statements.push(query) },
|
|
}) as typeof db;
|
|
const selected = await resolvePluginSandboxProviderDriverByKey({ db: recorded, driverKey: key });
|
|
expect(selected?.plugin.id).toBe(ids[1]);
|
|
expect(statements).toHaveLength(1);
|
|
expect(statements[0]).toContain('where "plugins"."manifest_json" @>');
|
|
expect(statements[0]).toContain('order by "plugins"."install_order" asc');
|
|
});
|
|
|
|
it("keeps readiness and worker selection after the database filter", async () => {
|
|
const isRunning = vi.fn((id: string) => id === ids[3]);
|
|
const workerManager = { isRunning } as unknown as PluginWorkerManager;
|
|
const selected = await resolvePluginSandboxProviderDriverByKey({ db, driverKey: key, workerManager, requireRunning: true });
|
|
expect(selected?.plugin.id).toBe(ids[3]);
|
|
expect(isRunning.mock.calls.map(([id]) => id)).toEqual([ids[2], ids[3]]);
|
|
expect(await resolvePluginSandboxProviderDriverByKey({ db, driverKey: key, requireRunning: true })).toBeNull();
|
|
});
|
|
|
|
it("reads changed status and declarations on the next lookup", async () => {
|
|
const workerManager = { isRunning: () => true } as unknown as PluginWorkerManager;
|
|
const resolve = () => resolvePluginSandboxProviderDriverByKey({ db, driverKey: key, workerManager, requireRunning: true });
|
|
expect((await resolve())?.plugin.id).toBe(ids[2]);
|
|
await db.update(plugins).set({ status: "disabled" }).where(eq(plugins.id, ids[2]));
|
|
expect((await resolve())?.plugin.id).toBe(ids[3]);
|
|
await db.update(plugins).set({ manifestJson: manifest(ids[3], [driver("replacement-provider")]) }).where(eq(plugins.id, ids[3]));
|
|
expect(await resolve()).toBeNull();
|
|
});
|
|
|
|
it("binds unusual driver keys as JSON values", async () => {
|
|
const unusual = "provider-\"quoted' OR true --";
|
|
await db.update(plugins).set({ manifestJson: manifest(ids[4], [driver(unusual)]) }).where(eq(plugins.id, ids[4]));
|
|
expect((await pluginRegistryService(db).list({ sandboxProviderDriverKey: unusual })).map(plugin => plugin.id)).toEqual([ids[4]]);
|
|
expect((await resolvePluginSandboxProviderDriverByKey({ db, driverKey: unusual }))?.plugin.id).toBe(ids[4]);
|
|
});
|
|
});
|