mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 13:24:55 +01:00
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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
8549ecaf9a
commit
0c3b5ce273
4 files changed
+107
-6
No files matched your search
@@ -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
|
||||
|
||||
@@ -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<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]);
|
||||
});
|
||||
});
|
||||
@@ -169,7 +169,7 @@ export async function resolvePluginSandboxProviderDriverByKey(input: {
|
||||
requireRunning?: boolean;
|
||||
}): Promise<{ plugin: Awaited<ReturnType<ReturnType<typeof pluginRegistryService>["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",
|
||||
|
||||
@@ -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).
|
||||
|
||||
Reference in new issue
Block a user