From 0c3b5ce273912958240702d5fba29011cf50bde1 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Fri, 23 Oct 2026 18:31:36 -0700 Subject: [PATCH] perf: filter sandbox provider lookups in Postgres (#17395) ## 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 --- doc/DEVELOPING.md | 4 + ...lugin-environment-driver-selection.test.ts | 95 +++++++++++++++++++ .../src/services/plugin-environment-driver.ts | 2 +- server/src/services/plugin-registry.ts | 12 ++- 4 files changed, 107 insertions(+), 6 deletions(-) create mode 100644 server/src/__tests__/plugin-environment-driver-selection.test.ts diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index f21ffd3348d..31196954cb9 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -2881,6 +2881,10 @@ Native event and retry-exhaustion ingestion lock the run row to verify its company and agent bindings. That lock reads only those two fields. Sequence allocation, replay checks, and event insertion remain in the same transaction. +Sandbox-provider driver lookup filters plugin manifests by driver key and kind +in Postgres before returning rows. Install order, status and worker checks still +select the provider. Each lookup reads current registry state without a cache. + Run the deterministic native event replay with the normal server test runner: ```sh diff --git a/server/src/__tests__/plugin-environment-driver-selection.test.ts b/server/src/__tests__/plugin-environment-driver-selection.test.ts new file mode 100644 index 00000000000..f04739a91a1 --- /dev/null +++ b/server/src/__tests__/plugin-environment-driver-selection.test.ts @@ -0,0 +1,95 @@ +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>; + let db: ReturnType; + 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]); + }); +}); diff --git a/server/src/services/plugin-environment-driver.ts b/server/src/services/plugin-environment-driver.ts index e9c4751985a..d7f56f2b197 100644 --- a/server/src/services/plugin-environment-driver.ts +++ b/server/src/services/plugin-environment-driver.ts @@ -169,7 +169,7 @@ export async function resolvePluginSandboxProviderDriverByKey(input: { requireRunning?: boolean; }): Promise<{ plugin: Awaited["list"]>>[number]; driver: PluginEnvironmentDriverDeclaration } | null> { const pluginRegistry = pluginRegistryService(input.db); - const plugins = await pluginRegistry.list(); + const plugins = await pluginRegistry.list({ sandboxProviderDriverKey: input.driverKey }); for (const plugin of plugins) { const driver = plugin.manifestJson.environmentDrivers?.find( (candidate) => candidate.driverKey === input.driverKey && candidate.kind === "sandbox_provider", diff --git a/server/src/services/plugin-registry.ts b/server/src/services/plugin-registry.ts index 2ff529cc2d8..c47cf4f6f5d 100644 --- a/server/src/services/plugin-registry.ts +++ b/server/src/services/plugin-registry.ts @@ -97,11 +97,13 @@ export function pluginRegistryService(db: Db) { // ----- Read ----------------------------------------------------------- /** List all registered plugins ordered by install order. */ - list: () => - db - .select() - .from(plugins) - .orderBy(asc(plugins.installOrder)), + list: (filter?: { sandboxProviderDriverKey: string }) => { + const query = db.select().from(plugins); + const driverKey = filter?.sandboxProviderDriverKey; + return (driverKey === undefined ? query : query.where(sql`${plugins.manifestJson} @> ${JSON.stringify({ + environmentDrivers: [{ driverKey, kind: "sandbox_provider" }], + })}::jsonb`)).orderBy(asc(plugins.installOrder)); + }, /** * List installed plugins (excludes soft-deleted/uninstalled).