mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-16 19:00:38 +09:00
fix(plugin-tool-dispatcher): propagate pluginDbId so worker.isRunning resolves (#5671)
Some checks failed
Docker / build-and-push (push) Failing after 3m41s
Refresh Lockfile / refresh (push) Failing after 5m12s
Release / verify_canary (push) Failing after 10m53s
Release / verify_stable (push) Has been skipped
Release / publish_canary (push) Has been skipped
Release / preview_stable (push) Has been skipped
Release / publish_stable (push) Has been skipped
Some checks failed
Docker / build-and-push (push) Failing after 3m41s
Refresh Lockfile / refresh (push) Failing after 5m12s
Release / verify_canary (push) Failing after 10m53s
Release / verify_stable (push) Has been skipped
Release / publish_canary (push) Has been skipped
Release / preview_stable (push) Has been skipped
Release / publish_stable (push) Has been skipped
Fixes #2391 Fixes #3394 Fixes #4094 Fixes #5501 Fixes #5916 Fixes #6215 Fixes #6514 ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - Plugins extend the platform by registering agent-callable tools backed by long-running worker processes > - `PluginToolDispatcher` is the boundary between the HTTP `/api/plugins/tools/execute` route and `PluginWorkerManager`, which owns those worker processes > - `PluginWorkerManager` keys live workers by the plugin's **database UUID**, but `plugin-loader` was registering tools using only `pluginKey` — so every tool call did `workerManager.isRunning(pluginKey)` and always got `false` > - As a result, every `POST /api/plugins/tools/execute` against a tool-exposing plugin returned 502 `worker for plugin X is not running`, even though the worker process was alive (hit in production by `vexion.council-chat`; `mem0-sync` would be next) > - This pull request threads the DB UUID through the dispatcher → registry hop and hardens the contract so omitting the UUID is a compile-time error, not a silent fallback > - The benefit is plugin tool execution actually works for any plugin declaring `manifest.tools[]`, and the type system prevents the same bug from recurring ## What Changed - `server/src/services/plugin-loader.ts` — pass in-scope `pluginId` (DB UUID) as the third argument to `toolDispatcher.registerPluginTools`. Single-line root fix. - `server/src/services/plugin-tool-dispatcher.ts` — `registerPluginTools` now takes `pluginDbId: string` (required, was optional). JSDoc updated to document the worker-routing contract and why the optional signature masked the bug. - `server/src/services/plugin-tool-registry.ts` — `registerPlugin` throws on missing/empty `pluginDbId` so any new call site that forgets the UUID fails immediately rather than silently falling back to `pluginKey`. - `server/src/__tests__/plugin-tool-dispatcher-pluginDbId.test.ts` — new focused regression suite covering the activation path, disable→enable lifecycle, worker re-spawn, and the empty-UUID guard. ## Verification - `pnpm vitest run server/src/__tests__/plugin-tool-dispatcher-pluginDbId.test.ts` — 6/6 passing. - `pnpm vitest run server/src/__tests__/plugin-database.test.ts server/src/__tests__/plugin-routes-authz.test.ts server/src/__tests__/plugin-lifecycle-restart.test.ts` — 48/48 passing on the merge commit. - `pnpm --filter @paperclipai/server typecheck` — no new errors introduced by these files. - Manual repro path: 1. Install a plugin that declares `manifest.tools[]` and uses `runWorker`. 2. Confirm status `ready` and a live worker (`paperclipai plugin diagnostics <key>`). 3. `POST /api/plugins/tools/execute` with `{ tool: "<pluginKey>:<toolName>", parameters, runContext }`. 4. Pre-fix: HTTP 502, `worker for plugin <key> is not running`. Post-fix: tool dispatches normally. ## Risks - Low risk. The signature tightening (`pluginDbId?` → `pluginDbId`) is a back-compatible behavioral fix at the only production call site (`plugin-loader`), which already had the UUID in scope. - Test/recovery paths that previously omitted the UUID must now supply it; the new error message identifies the missing arg explicitly. - No database migration, no API/schema change, no plugin-author-facing change. - The merge commit pulls master into the PR branch additively (no rebase); reviewers can read the fix commits independently of the merge. ## Model Used - Provider/model: Anthropic Claude (Opus 4.7, `claude-opus-4-7`) for the additive merge-conflict resolution, PR description rewrite, and Greptile follow-up; original fix authored by [@Ramon-nassa](https://github.com/Ramon-nassa). - Capabilities used: tool use (file edit, shell, GitHub CLI), extended thinking off, no code execution by the model. ## 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 - [ ] If this change affects the UI, I have included before/after screenshots (N/A — server-only change) - [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 --- ## Original Summary (preserved from contributor) `plugin-loader` activates plugins and calls ```ts toolDispatcher.registerPluginTools(pluginKey, manifest) ``` with only two args. `PluginToolDispatcher.registerPluginTools` forwards them to `registry.registerPlugin(pluginKey, manifest)`. The registry falls back `pluginDbId ?? pluginKey`, but `PluginWorkerManager` keys live workers by the DB UUID — so the downstream ```ts workerManager.isRunning(pluginKey) // always false ``` causes every `POST /api/plugins/tools/execute` to fail with `worker for plugin X is not running`, even when the worker process is alive and healthy. **This hits every plugin that exposes tools** (we hit it in `vexion.council-chat`; `mem0-sync` would too). Reported-by: Vexion / Ramon Nassar (vexion.council-chat plugin, MO-068). --------- Co-authored-by: ramon nassar <ramon@tabs.co> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Devin Foley <devin@devinfoley.com> Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
c369d3d357
commit
62863126a3
3 changed files with 343 additions and 14 deletions
|
|
@ -150,14 +150,18 @@ export interface PluginToolDispatcher {
|
|||
* This is called automatically when a plugin transitions to `ready`.
|
||||
* Can also be called manually for testing or recovery scenarios.
|
||||
*
|
||||
* @param pluginId - The plugin's stable manifest/plugin key used for tool namespacing
|
||||
* @param manifest - The plugin manifest containing tool declarations
|
||||
* @param pluginDbId - The plugin database ID used for worker lookup
|
||||
* @param pluginKey - The plugin's namespaced key (e.g. `acme.linear`).
|
||||
* Used as the lookup key for tool registration.
|
||||
* @param manifest - The plugin manifest containing tool declarations.
|
||||
* @param pluginDbId - The plugin's database UUID. Required:
|
||||
* `workerManager` keys running workers by DB UUID, not by pluginKey, so
|
||||
* without this `workerManager.isRunning(...)` always returns false and
|
||||
* every tool dispatch fails with `worker for plugin X is not running`.
|
||||
*/
|
||||
registerPluginTools(
|
||||
pluginId: string,
|
||||
pluginKey: string,
|
||||
manifest: PaperclipPluginManifestV1,
|
||||
pluginDbId?: string,
|
||||
pluginDbId: string,
|
||||
): void;
|
||||
|
||||
/**
|
||||
|
|
@ -429,11 +433,11 @@ export function createPluginToolDispatcher(
|
|||
},
|
||||
|
||||
registerPluginTools(
|
||||
pluginId: string,
|
||||
pluginKey: string,
|
||||
manifest: PaperclipPluginManifestV1,
|
||||
pluginDbId?: string,
|
||||
pluginDbId: string,
|
||||
): void {
|
||||
registry.registerPlugin(pluginId, manifest, pluginDbId);
|
||||
registry.registerPlugin(pluginKey, manifest, pluginDbId);
|
||||
},
|
||||
|
||||
unregisterPluginTools(pluginId: string): void {
|
||||
|
|
|
|||
|
|
@ -107,12 +107,15 @@ export interface PluginToolRegistry {
|
|||
* Called when a plugin worker starts and its manifest is loaded. Any
|
||||
* previously registered tools for the same plugin are replaced (idempotent).
|
||||
*
|
||||
* @param pluginId - The plugin's unique identifier (e.g. `"acme.linear"`)
|
||||
* @param manifest - The plugin manifest containing the `tools` array
|
||||
* @param pluginId - The plugin's unique identifier (e.g. `"acme.linear"`).
|
||||
* @param manifest - The plugin manifest containing the `tools` array.
|
||||
* @param pluginDbId - The plugin's database UUID, used for worker routing
|
||||
* and availability checks. If omitted, `pluginId` is used (backwards-compat).
|
||||
* and availability checks. Required — `workerManager` keys live workers
|
||||
* by the DB UUID, so omitting this guarantees that every subsequent
|
||||
* `workerManager.isRunning(pluginDbId)` call returns false and every tool
|
||||
* dispatch fails with `worker for plugin X is not running`.
|
||||
*/
|
||||
registerPlugin(pluginId: string, manifest: PaperclipPluginManifestV1, pluginDbId?: string): void;
|
||||
registerPlugin(pluginId: string, manifest: PaperclipPluginManifestV1, pluginDbId: string): void;
|
||||
|
||||
/**
|
||||
* Remove all tool registrations for a plugin.
|
||||
|
|
@ -295,8 +298,17 @@ export function createPluginToolRegistry(
|
|||
// -----------------------------------------------------------------------
|
||||
|
||||
return {
|
||||
registerPlugin(pluginId: string, manifest: PaperclipPluginManifestV1, pluginDbId?: string): void {
|
||||
const dbId = pluginDbId ?? pluginId;
|
||||
registerPlugin(pluginId: string, manifest: PaperclipPluginManifestV1, pluginDbId: string): void {
|
||||
// Guard at the registry boundary so a missing UUID surfaces as an
|
||||
// explicit contract error instead of a downstream
|
||||
// `worker for plugin X is not running`.
|
||||
if (!pluginDbId) {
|
||||
throw new Error(
|
||||
`plugin-tool-registry.registerPlugin: pluginDbId is required (pluginId="${pluginId}"). ` +
|
||||
`Workers are keyed by DB UUID; omitting this guarantees worker-lookup failure.`,
|
||||
);
|
||||
}
|
||||
const dbId = pluginDbId;
|
||||
|
||||
// Remove any previously registered tools for this plugin (idempotent)
|
||||
const previousCount = removePluginTools(pluginId);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue