From 1ebf5254b6849ad04d1cce8ccf9cf8373f8229c4 Mon Sep 17 00:00:00 2001 From: Michael Nguyen Date: Thu, 23 Jul 2026 09:34:42 -0700 Subject: [PATCH] fix(plugin-loader): deliver stored config to freshly-started plugin workers (#10092) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - One capability is first-party **plugins** that run as isolated workers spawned by the host `plugin-loader`, reading company-scoped config through a governed `ctx.config.get(companyId)` channel. > - A **proactive** plugin (e.g. a chat gateway that opens a Slack Socket Mode connection at startup) does its company work from `setup()`, where there is **no company-scoped invocation** — so `ctx.config.get()` is rejected with `company context is required`. > - The worker swallows that error and falls back to its default (feature-off) config, so the plugin comes up **inert** even though correct config exists in the database. > - This is a regression from #9557 ("governed access contracts"), which changed `plugin-loader.ts` `activatePlugin` from loading stored config into the worker bootstrap to `const config = {}`. > - This pull request replays each configured company's stored config to the freshly-started worker over the **same `configChanged` host→worker path an operator config-save already uses**. > - The benefit is that proactive plugins receive their config on worker start (both server boot and operator enable) without weakening the governed-access surface. ## Linked Issues or Issue Description No public GitHub issue — describing in-PR (bug): **Bug.** After a proactive plugin's worker spawns, it never receives its stored config. Governed access (`packages/plugins/sdk/src/host-client-factory.ts`) only resolves `config.get` inside a company-scoped invocation (event/action/tool, or explicit `params.companyId`). Proactive plugins operate from `setup()` where no such scope exists, so `config.get()` fails with `company context is required`, the worker falls back to defaults, and the feature stays disabled despite valid DB config. - Regression introduced by #9557. - Related follow-up (latent multi-company hardening): #10096. ## What Changed - `plugin-registry.ts`: add read-only `listConfigs(pluginId)` returning all stored company config rows for a plugin (scoped `where eq(pluginConfig.pluginId, pluginId)`). - `plugin-loader.ts`: after the worker starts in `activatePlugin`, replay each company's stored config through the existing `configChanged` host→worker RPC — one `{ config, companyId }` per row, the same payload shape as the operator config-save path in `routes/plugins.ts`. Best-effort and idempotent; covers both server-boot `loadAll` and operator enable. - test: DB-backed `plugin-config-startup-delivery.test.ts` covering `registry.listConfigs` completeness and cross-plugin isolation. ## Verification - `tsc --noEmit` on `@paperclipai/server` — clean. - New `plugin-config-startup-delivery.test.ts` (embedded-postgres, 3 cases) — pass. - Full PR CI green: typecheck, all server/e2e/serialized test shards, build, canary dry-run, verify, and the security scanners (Snyk, Socket, Superagent, Greptile). ## Risks - **Low functional risk.** Adds an outbound host→worker push that mirrors the already-shipped operator-save path. A worker without an `onConfigChanged` handler (or momentarily unavailable) simply keeps the runtime `ctx.config.get(companyId)` model. - **Startup fan-out.** One `configChanged` per configured company at activation (sequential, default RPC timeout). `plugin_config` rows are writable only by instance-admins, so fan-out size is operator-controlled — not a remote surface. - **No secret-handling change.** `configJson` is delivered as-is, exactly as `config.get`/operator-save already deliver it. No new secret sink; catch-blocks log only ids + `err.message` at debug, never `configJson`. - **Latent multi-company behavior (pre-existing, not introduced here).** The worker-side `configChanged` dispatch forwards only `config` (drops `companyId`), and `listConfigs` has no `ORDER BY`, so a plugin configured for **more than one** company would apply a nondeterministic last-write-wins global config. This is existing SDK behavior — operator-save already pushes into the same handler — and is **not reachable by the single-company consumer this fix targets**. Greptile flagged this shape (4/5). It is tracked and fixed as a separate, non-blocking hardening PR (#10096): thread `companyId` through `onConfigChanged`, deterministic ordering, bounded fan-out. ## Model Used Claude — Anthropic `claude-opus-4-8` (Opus 4.8), extended thinking, with tool use / code execution via Claude Code. ## 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) - [ ] My branch name describes the change and contains no internal Paperclip ticket id — branch predates this rule; not renaming an open PR mid-review - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] I have updated relevant documentation to reflect my changes — no doc surface; internal SDK/host behavior only - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups — 4/5; two latent multi-company items triaged as non-blocking and fixed in follow-up #10096 (see Risks) - [x] I will address all Greptile and reviewer comments before requesting merge — addressed: triaged as non-blocking follow-up in #10096 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Paperclip --- .../plugin-config-startup-delivery.test.ts | 136 ++++++++++++++++++ server/src/services/plugin-loader.ts | 48 +++++++ server/src/services/plugin-registry.ts | 16 +++ 3 files changed, 200 insertions(+) create mode 100644 server/src/__tests__/plugin-config-startup-delivery.test.ts diff --git a/server/src/__tests__/plugin-config-startup-delivery.test.ts b/server/src/__tests__/plugin-config-startup-delivery.test.ts new file mode 100644 index 0000000000..58b02fbb25 --- /dev/null +++ b/server/src/__tests__/plugin-config-startup-delivery.test.ts @@ -0,0 +1,136 @@ +import { randomUUID } from "node:crypto"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { companies, createDb, pluginConfig, plugins } from "@paperclipai/db"; +import { pluginRegistryService } from "../services/plugin-registry.js"; +import { + getEmbeddedPostgresTestSupport, + startEmbeddedPostgresTestDatabase, +} from "./helpers/embedded-postgres.js"; + +/** + * LOOA-629: a plugin worker is spawned once per plugin (not per company) with + * an empty bootstrap config, and can only read company-scoped config from + * inside a company-scoped invocation. A proactive plugin (e.g. the chat + * gateway) has no such invocation at setup(), so the loader must replay every + * configured company's config to the freshly-started worker. That replay reads + * the config rows via `registry.listConfigs(pluginId)`, which this exercises. + */ + +const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; + +if (!embeddedPostgresSupport.supported) { + console.warn( + `Skipping plugin config startup-delivery tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`, + ); +} + +function issuePrefix(id: string) { + return `T${id.replace(/-/g, "").slice(0, 6).toUpperCase()}`; +} + +describeEmbeddedPostgres("registry.listConfigs (startup config delivery)", () => { + let db!: ReturnType; + let tempDb: Awaited> | null = null; + + beforeAll(async () => { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-plugin-config-delivery-"); + db = createDb(tempDb.connectionString); + }, 20_000); + + afterEach(async () => { + await db.delete(pluginConfig); + await db.delete(plugins); + await db.delete(companies); + }); + + afterAll(async () => { + await tempDb?.cleanup(); + }); + + async function seedPlugin(pluginKey: string, installOrder: number) { + const pluginId = randomUUID(); + await db.insert(plugins).values({ + id: pluginId, + pluginKey, + packageName: `@paperclipai/${pluginKey}`, + version: "0.0.1", + apiVersion: 1, + categories: ["automation"], + manifestJson: { + id: pluginKey, + apiVersion: 1, + version: "0.0.1", + displayName: pluginKey, + description: "Test plugin", + author: "Paperclip", + categories: ["automation"], + capabilities: [], + entrypoints: { worker: "./dist/worker.js" }, + }, + status: "ready", + installOrder, + }); + return pluginId; + } + + async function seedCompany() { + const companyId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: `Co ${companyId.slice(0, 6)}`, + issuePrefix: issuePrefix(companyId), + }); + return companyId; + } + + it("returns every company-scoped config row for a plugin", async () => { + const registry = pluginRegistryService(db); + const pluginId = await seedPlugin("paperclip.gateway-test", 1); + const companyA = await seedCompany(); + const companyB = await seedCompany(); + + await registry.upsertConfig(pluginId, companyA, { + companyId: companyA, + configJson: { slackBotToken: "xoxb-a", slackAppToken: "xapp-a" }, + }); + await registry.upsertConfig(pluginId, companyB, { + companyId: companyB, + configJson: { slackBotToken: "xoxb-b", slackAppToken: "xapp-b" }, + }); + + const rows = await registry.listConfigs(pluginId); + expect(rows).toHaveLength(2); + + const byCompany = new Map(rows.map((r) => [r.companyId, r])); + expect(byCompany.get(companyA)?.configJson).toMatchObject({ slackBotToken: "xoxb-a" }); + expect(byCompany.get(companyB)?.configJson).toMatchObject({ slackBotToken: "xoxb-b" }); + }); + + it("only returns rows for the requested plugin (no cross-plugin bleed)", async () => { + const registry = pluginRegistryService(db); + const pluginId = await seedPlugin("paperclip.gateway-test", 1); + const otherPluginId = await seedPlugin("paperclip.other-test", 2); + const companyA = await seedCompany(); + + await registry.upsertConfig(pluginId, companyA, { + companyId: companyA, + configJson: { marker: "mine" }, + }); + await registry.upsertConfig(otherPluginId, companyA, { + companyId: companyA, + configJson: { marker: "theirs" }, + }); + + const rows = await registry.listConfigs(pluginId); + expect(rows).toHaveLength(1); + expect(rows[0]?.configJson).toMatchObject({ marker: "mine" }); + }); + + it("returns an empty list when the plugin has no configured companies", async () => { + const registry = pluginRegistryService(db); + const pluginId = await seedPlugin("paperclip.gateway-test", 1); + const rows = await registry.listConfigs(pluginId); + expect(rows).toEqual([]); + }); +}); diff --git a/server/src/services/plugin-loader.ts b/server/src/services/plugin-loader.ts index faa0517fa0..aded537056 100644 --- a/server/src/services/plugin-loader.ts +++ b/server/src/services/plugin-loader.ts @@ -2137,6 +2137,8 @@ export function pluginLoader( // ------------------------------------------------------------------ // Plugin configuration is company-scoped. Workers receive an empty // bootstrap config and must use ctx.config.get(companyId) at runtime. + // Stored config is delivered right after the worker starts (step 5b) via + // the same configChanged path an operator config-save uses. const config: Record = {}; // ------------------------------------------------------------------ @@ -2169,6 +2171,52 @@ export function pluginLoader( "plugin-loader: worker started", ); + // ------------------------------------------------------------------ + // 5b. Deliver stored configuration to the freshly-started worker + // ------------------------------------------------------------------ + // The worker is spawned with an empty bootstrap config and is expected to + // read company-scoped config via ctx.config.get(companyId). That call + // only resolves inside a company-scoped invocation (event/action/tool), + // so a proactive plugin that does company work from setup() — e.g. the + // chat gateway opening a Slack Socket Mode connection — can never read + // its own config and comes up inert. Replay each configured company's + // config through the same configChanged path an operator config-save + // uses (routes/plugins.ts), so the worker receives it at startup. + // Best-effort: a worker that doesn't implement onConfigChanged + // (METHOD_NOT_IMPLEMENTED) or is momentarily unavailable simply keeps the + // runtime ctx.config.get(companyId) model. onConfigChanged is idempotent + // for well-behaved plugins, so replaying an unchanged config is safe. + try { + const configRows = await registry.listConfigs(pluginId); + for (const row of configRows) { + try { + await workerManager.call(pluginId, "configChanged", { + config: (row.configJson ?? {}) as Record, + companyId: row.companyId, + }); + } catch (configErr) { + log.debug( + { + pluginId, + pluginKey, + companyId: row.companyId, + err: configErr instanceof Error ? configErr.message : String(configErr), + }, + "plugin-loader: startup config delivery skipped for company", + ); + } + } + } catch (listErr) { + log.debug( + { + pluginId, + pluginKey, + err: listErr instanceof Error ? listErr.message : String(listErr), + }, + "plugin-loader: could not list stored configs for startup delivery", + ); + } + // ------------------------------------------------------------------ // 6. Sync job declarations and register with scheduler // ------------------------------------------------------------------ diff --git a/server/src/services/plugin-registry.ts b/server/src/services/plugin-registry.ts index 1ee05092fb..344cc26c9c 100644 --- a/server/src/services/plugin-registry.ts +++ b/server/src/services/plugin-registry.ts @@ -288,6 +288,22 @@ export function pluginRegistryService(db: Db) { .where(and(eq(pluginConfig.pluginId, pluginId), eq(pluginConfig.companyId, companyId))) .then((rows) => rows[0] ?? null), + /** + * List every company-scoped configuration row for a plugin. + * + * Plugin config is company-scoped, but a worker is spawned once per plugin + * (not per company). Callers such as the plugin loader use this to replay + * each configured company's config to a freshly-started worker, so a + * proactive plugin that never runs inside a company-scoped invocation (and + * therefore cannot resolve `ctx.config.get(companyId)`) still receives its + * configuration at startup. + */ + listConfigs: (pluginId: string) => + db + .select() + .from(pluginConfig) + .where(eq(pluginConfig.pluginId, pluginId)), + /** * Create or fully replace a plugin's company-scoped configuration. * If a config row already exists for the plugin/company pair it is replaced;