forked from farhoodlabs/paperclip
c91a062326
## Thinking Path > - Paperclip orchestrates AI agents through a server-side control plane > - That control plane depends on reliable issue state transitions, plugin lifecycle behavior, import limits, and startup/shutdown handling > - Several small runtime fixes had accumulated on the working branch and were mixed with larger feature work > - Keeping them separate makes the correctness fixes reviewable and mergeable without waiting for cloud-sync UI work > - This pull request groups the server/runtime control-plane fixes into one standalone branch > - The benefit is a tighter, safer runtime baseline for retries, imports, plugin migrations, feedback flushing, and trusted cloud import handling ## What Changed - Fixed updated issue list pagination sorting and scheduled retry comment handling. - Re-applied pending plugin migrations during hot reload and fixed plugin-schema worktree seed restore. - Hardened public tenant DB startup, portable import body limits, trusted cloud import errors, and trusted cloud tenant import mutation access. - Expired stale request confirmations after user comments. - Added feedback export shutdown hardening so database-unavailable flush loops stop cleanly. - Guarded plugin worker `error` event emission when no listener is registered. ## Verification - `pnpm install --frozen-lockfile --ignore-scripts` - `pnpm --filter @paperclipai/plugin-sdk build` - `npm run install --prefix node_modules/.pnpm/sqlite3@5.1.7/node_modules/sqlite3` - `pnpm exec vitest run server/src/__tests__/issues-service.test.ts server/src/__tests__/plugin-lifecycle-restart.test.ts server/src/__tests__/server-startup-feedback-export.test.ts server/src/__tests__/issue-comment-reopen-routes.test.ts server/src/__tests__/issue-thread-interactions-service.test.ts server/src/__tests__/issue-thread-interaction-routes.test.ts server/src/__tests__/body-limits.test.ts server/src/__tests__/feedback-flush-controller.test.ts server/src/__tests__/error-handler.test.ts server/src/__tests__/board-mutation-guard.test.ts packages/db/src/backup-lib.test.ts` initially exposed local setup issues and two 5s test timeouts. - Rerun after local prereq build: `pnpm exec vitest run --testTimeout 15000 server/src/__tests__/issue-comment-reopen-routes.test.ts server/src/__tests__/issue-thread-interaction-routes.test.ts server/src/__tests__/feedback-flush-controller.test.ts server/src/__tests__/server-startup-feedback-export.test.ts` passed. - Some embedded Postgres-backed tests skipped on this host because local Postgres init was unavailable. ## Risks - Runtime-touching branch: startup/shutdown and issue interaction behavior should be reviewed carefully. - The feedback export change disables repeated flush attempts only for database connection-refused failures; other upload failures still log normally. - The plugin worker error guard avoids process crashes from unhandled EventEmitter errors but may hide errors from code paths that expected an emitted listener. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex, GPT-5-based coding agent with local shell/git/tool use. Exact hosted model ID and context-window size are not exposed by the local Paperclip adapter runtime. ## 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 - [x] If this change affects the UI, I have included before/after screenshots - [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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
130 lines
4.7 KiB
TypeScript
130 lines
4.7 KiB
TypeScript
/**
|
|
* 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<PluginLoader> = {
|
|
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 },
|
|
);
|
|
const stopped = vi.fn();
|
|
const started = vi.fn();
|
|
lifecycle.on("plugin.worker_stopped", stopped);
|
|
lifecycle.on("plugin.worker_started", started);
|
|
|
|
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();
|
|
expect(stopped).not.toHaveBeenCalled();
|
|
expect(started).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<PluginLoader> = {
|
|
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 },
|
|
);
|
|
const stopped = vi.fn();
|
|
const started = vi.fn();
|
|
lifecycle.on("plugin.worker_stopped", stopped);
|
|
lifecycle.on("plugin.worker_started", started);
|
|
|
|
await lifecycle.restartWorker("plugin-1");
|
|
|
|
expect(loader.unloadSingle).not.toHaveBeenCalled();
|
|
expect(loader.loadSingle).not.toHaveBeenCalled();
|
|
expect(handle.restart).toHaveBeenCalledTimes(1);
|
|
expect(stopped).toHaveBeenCalledTimes(1);
|
|
expect(stopped).toHaveBeenCalledWith({ pluginId: "plugin-1", pluginKey: "example.plugin" });
|
|
expect(started).toHaveBeenCalledTimes(1);
|
|
expect(started).toHaveBeenCalledWith({ pluginId: "plugin-1", pluginKey: "example.plugin" });
|
|
});
|
|
});
|