diff --git a/server/src/__tests__/plugin-lifecycle-restart.test.ts b/server/src/__tests__/plugin-lifecycle-restart.test.ts new file mode 100644 index 00000000000..d7731fde1fa --- /dev/null +++ b/server/src/__tests__/plugin-lifecycle-restart.test.ts @@ -0,0 +1,115 @@ +/** + * Regression test for PAP-9585. + * + * `restartWorker` is called by the dev file-watcher whenever a local-path + * plugin's source files change. Before PAP-9585 it only bounced the worker + * subprocess, which left newly added `migrations/*.sql` files unapplied — the + * plugin schema would silently drift out of sync with worker code. + * + * The fix is for `restartWorker` to do a full deactivate + reactivate cycle + * via the plugin loader, which re-reads the manifest from disk and runs + * `applyMigrations` (idempotently) before starting the new worker. + */ +import { describe, expect, it, vi } from "vitest"; + +const pluginRecord = { + id: "plugin-1", + pluginKey: "example.plugin", + status: "ready", + manifestJson: { id: "example.plugin", capabilities: [] }, + packageName: "@example/plugin", + version: "1.0.0", + packagePath: "/tmp/example-plugin", +}; + +const mockRegistry = vi.hoisted(() => ({ + getById: vi.fn(), + getByKey: vi.fn(), + update: vi.fn(), + updateStatus: vi.fn(), + upsertConfig: vi.fn(), + getConfig: vi.fn(), + list: vi.fn(), + delete: vi.fn(), +})); + +vi.mock("../services/plugin-registry.js", () => ({ + pluginRegistryService: () => mockRegistry, +})); + +import { pluginLifecycleManager } from "../services/plugin-lifecycle.js"; +import type { PluginLoader } from "../services/plugin-loader.js"; +import type { PluginWorkerManager } from "../services/plugin-worker-manager.js"; + +function makeWorkerManagerStub() { + const handle = { + restart: vi.fn().mockResolvedValue(undefined), + stop: vi.fn().mockResolvedValue(undefined), + }; + return { + handle, + workerManager: { + getWorker: vi.fn().mockReturnValue(handle), + isRunning: vi.fn().mockReturnValue(true), + startWorker: vi.fn().mockResolvedValue(undefined), + stopWorker: vi.fn().mockResolvedValue(undefined), + restartWorker: vi.fn().mockResolvedValue(undefined), + } as unknown as PluginWorkerManager, + }; +} + +describe("pluginLifecycleManager.restartWorker", () => { + it("does a full deactivate+reactivate cycle when the loader has runtime services", async () => { + mockRegistry.getById.mockResolvedValue(pluginRecord); + mockRegistry.updateStatus.mockResolvedValue(pluginRecord); + + const { handle, workerManager } = makeWorkerManagerStub(); + + const loader: Partial = { + hasRuntimeServices: vi.fn().mockReturnValue(true) as PluginLoader["hasRuntimeServices"], + loadSingle: vi.fn().mockResolvedValue({ + success: true, + plugin: pluginRecord, + registered: { worker: true, eventSubscriptions: 0, jobs: 0, webhooks: 0, tools: 0 }, + }) as PluginLoader["loadSingle"], + unloadSingle: vi.fn().mockResolvedValue(undefined) as PluginLoader["unloadSingle"], + }; + + const lifecycle = pluginLifecycleManager( + {} as never, + { loader: loader as PluginLoader, workerManager }, + ); + + await lifecycle.restartWorker("plugin-1"); + + expect(loader.unloadSingle).toHaveBeenCalledWith("plugin-1", "example.plugin"); + expect(loader.loadSingle).toHaveBeenCalledWith("plugin-1"); + // The bare worker handle should NOT be bounced — the loader handles + // worker (re)start as part of activate. + expect(handle.restart).not.toHaveBeenCalled(); + }); + + it("falls back to bouncing the worker handle when the loader has no runtime services", async () => { + mockRegistry.getById.mockResolvedValue(pluginRecord); + mockRegistry.updateStatus.mockResolvedValue(pluginRecord); + + const { handle, workerManager } = makeWorkerManagerStub(); + + const loader: Partial = { + hasRuntimeServices: vi.fn().mockReturnValue(false) as PluginLoader["hasRuntimeServices"], + loadSingle: vi.fn() as PluginLoader["loadSingle"], + unloadSingle: vi.fn() as PluginLoader["unloadSingle"], + }; + + const lifecycle = pluginLifecycleManager( + {} as never, + { loader: loader as PluginLoader, workerManager }, + ); + + await lifecycle.restartWorker("plugin-1"); + + expect(loader.unloadSingle).not.toHaveBeenCalled(); + expect(loader.loadSingle).not.toHaveBeenCalled(); + expect(handle.restart).toHaveBeenCalledTimes(1); + }); +}); diff --git a/server/src/services/plugin-lifecycle.ts b/server/src/services/plugin-lifecycle.ts index e9f9b02b125..daee94173c2 100644 --- a/server/src/services/plugin-lifecycle.ts +++ b/server/src/services/plugin-lifecycle.ts @@ -776,19 +776,48 @@ export function pluginLifecycleManager( ); } - log.info( - { pluginId, pluginKey: plugin.pluginKey }, - "plugin lifecycle: restarting worker", - ); + const supportsRuntimeActivation = + typeof pluginLoaderInstance.hasRuntimeServices === "function" + && typeof pluginLoaderInstance.loadSingle === "function" + && typeof pluginLoaderInstance.unloadSingle === "function" + && pluginLoaderInstance.hasRuntimeServices(); - await handle.restart(); + if (supportsRuntimeActivation) { + log.info( + { pluginId, pluginKey: plugin.pluginKey }, + "plugin lifecycle: reloading plugin (re-reading manifest, re-applying pending migrations, restarting worker)", + ); + + // Full deactivate+reactivate cycle (not just `handle.restart()`) so that: + // - the manifest is re-read from disk, picking up newly declared + // `migrations/*.sql` files and any other manifest changes, + // - `applyMigrations` runs idempotently against the up-to-date + // migrations directory — pending migrations get applied, already- + // applied ones are skipped via the `pluginMigrations` table, + // - the worker subprocess is replaced with one loading the freshly + // built bundle. + // + // Bouncing the worker process alone (`handle.restart()`) leaves plugin + // schema out of sync with worker code whenever a hot reload adds a new + // migration, which makes downstream queries fail against missing tables. + await deactivatePluginRuntime(pluginId, plugin.pluginKey); + await activateReadyPlugin(pluginId); + } else { + // No runtime activation services wired in (e.g. state-only test harness) + // — fall back to a bare worker subprocess bounce. + log.info( + { pluginId, pluginKey: plugin.pluginKey }, + "plugin lifecycle: restarting worker (runtime services unavailable; skipping migration re-apply)", + ); + await handle.restart(); + } emitDomain("plugin.worker_stopped", { pluginId, pluginKey: plugin.pluginKey }); emitDomain("plugin.worker_started", { pluginId, pluginKey: plugin.pluginKey }); log.info( { pluginId, pluginKey: plugin.pluginKey }, - "plugin lifecycle: worker restarted", + "plugin lifecycle: plugin reloaded", ); },