mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 08:22:20 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Local plugin development uses file changes to restart the plugin worker. > - A plugin can move to a different local package directory. > - The watcher rejected every later registration for the same plugin ID. > - This pull request replaces the watcher when the resolved package path changes. > - The benefit is that current plugin files trigger reloads and previous files stop doing so. ## Linked Issues or Issue Description Refs: #14955. This change addresses its second observation: the watcher keeps the previous local directory after its package path changes. The issue also tracks stored configuration replay as a separate problem. Related PRs: #11336 shares the lifecycle manager across routes and the dispatcher. #14381 controls when production file watching is enabled. #10100 orders configuration delivery. Those changes address other parts of plugin reload behavior. ## What Changed - Store the resolved package path with each watcher. Keep repeated registrations of the same path as a no-op. Replace the watcher for a different path. - Cancel pending changes from the previous directory. Ignore late change, error, and timer callbacks from a replaced watcher. - Apply only the latest pending directory lookup. Manual registration, unwatching, and shutdown invalidate earlier lookups. - Stop a failed watcher without cancelling its pending replacement lookup. Explicit unwatching, disable, unload, and shutdown still cancel pending registrations. - Stop the old watcher when the new directory is unavailable or the plugin no longer has a local package path. - Document the path-change behavior and add twelve regression cases. ## Verification - `pnpm exec vitest run server/src/__tests__/plugin-dev-watcher.test.ts server/src/__tests__/plugin-lifecycle-restart.test.ts`: 18 tests passed. Regressions cover path changes, stale callbacks, lookup order, explicit cancellation, and watcher errors during a pending replacement. The current-watcher error case fails before the stop-versus-cancel fix; the stale-watcher error control passes. - A local smoke check used real Chokidar watchers and two real temporary plugin directories. It confirmed that a pending old change is cancelled, later old-directory changes are ignored, and a new-directory change triggers one restart. - `pnpm -r typecheck` passed on the path and lookup guards. The final watcher-error fix passed `pnpm --filter @paperclipai/server exec tsc --noEmit`. - `pnpm build` passed on the initial path-change fix. The final source passed `pnpm --filter @paperclipai/server build`. - `git diff --check`: passed. - Greptile reviewed `bef3182` at 5/5 with no new findings. The watcher-error review thread is resolved. CI is still running. - The full `pnpm test:run` suite was not repeated for this watcher-only change. The affected suites ran locally. The broad test shards will run in CI. ## Risks - Low risk. The watcher follows the current registered path. An unavailable replacement stops watching until another valid registration arrives. - Closing a filesystem watcher is asynchronous. Callback identity checks prevent an old watcher from acting on its replacement. - No database migration, API shape, or dependency change is required. ## Model Used - OpenAI Codex based on GPT-6, with reasoning, tool use, and code execution. The runtime does not expose the exact serving model ID or context window. ## 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 - [ ] 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
381 lines
12 KiB
TypeScript
381 lines
12 KiB
TypeScript
/**
|
|
* PluginDevWatcher — watches local-path plugin directories for file changes
|
|
* and triggers worker restarts so plugin authors get a fast rebuild-and-reload
|
|
* cycle without manually restarting the server.
|
|
*
|
|
* Only plugins installed from a local path (i.e. those with a non-null
|
|
* `packagePath` in the DB) are watched. File changes in the plugin's package
|
|
* directory trigger a debounced worker restart via the lifecycle manager.
|
|
*
|
|
* Uses chokidar rather than raw fs.watch so we get a production-grade watcher
|
|
* backend across platforms and avoid exhausting file descriptors as quickly in
|
|
* large dev workspaces.
|
|
*
|
|
* @see PLUGIN_SPEC.md §27.2 — Local Development Workflow
|
|
*/
|
|
import chokidar, { type FSWatcher } from "chokidar";
|
|
import { existsSync, readFileSync, readdirSync, statSync } from "node:fs";
|
|
import path from "node:path";
|
|
import { logger } from "../middleware/logger.js";
|
|
import type { PluginLifecycleManager } from "./plugin-lifecycle.js";
|
|
|
|
const log = logger.child({ service: "plugin-dev-watcher" });
|
|
|
|
/** Debounce interval for file changes (ms). */
|
|
const DEBOUNCE_MS = 500;
|
|
|
|
export interface PluginDevWatcher {
|
|
/** Start watching a local-path plugin directory. */
|
|
watch(pluginId: string, packagePath: string): void;
|
|
/** Stop watching a specific plugin. */
|
|
unwatch(pluginId: string): void;
|
|
/** Stop all watchers and clean up. */
|
|
close(): void;
|
|
}
|
|
|
|
export type ResolvePluginPackagePath = (
|
|
pluginId: string,
|
|
) => Promise<string | null | undefined>;
|
|
|
|
export interface PluginDevWatcherFsDeps {
|
|
existsSync?: typeof existsSync;
|
|
readFileSync?: typeof readFileSync;
|
|
readdirSync?: typeof readdirSync;
|
|
statSync?: typeof statSync;
|
|
}
|
|
|
|
type PluginWatchTarget = {
|
|
path: string;
|
|
recursive: boolean;
|
|
kind: "file" | "dir";
|
|
};
|
|
|
|
type PluginPackageJson = {
|
|
paperclipPlugin?: {
|
|
manifest?: string;
|
|
worker?: string;
|
|
ui?: string;
|
|
};
|
|
};
|
|
|
|
function shouldIgnorePath(filename: string | null | undefined): boolean {
|
|
if (!filename) return false;
|
|
const normalized = filename.replace(/\\/g, "/");
|
|
const segments = normalized.split("/").filter(Boolean);
|
|
return segments.some(
|
|
(segment) =>
|
|
segment === "node_modules" ||
|
|
segment === ".git" ||
|
|
segment === ".vite" ||
|
|
segment === ".paperclip-sdk" ||
|
|
segment.startsWith("."),
|
|
);
|
|
}
|
|
|
|
export function resolvePluginWatchTargets(
|
|
packagePath: string,
|
|
fsDeps?: Pick<PluginDevWatcherFsDeps, "existsSync" | "readFileSync" | "readdirSync" | "statSync">,
|
|
): PluginWatchTarget[] {
|
|
const fileExists = fsDeps?.existsSync ?? existsSync;
|
|
const readFile = fsDeps?.readFileSync ?? readFileSync;
|
|
const readDir = fsDeps?.readdirSync ?? readdirSync;
|
|
const statFile = fsDeps?.statSync ?? statSync;
|
|
const absPath = path.resolve(packagePath);
|
|
const targets = new Map<string, PluginWatchTarget>();
|
|
|
|
function addWatchTarget(targetPath: string, recursive: boolean, kind?: "file" | "dir"): void {
|
|
const resolved = path.resolve(targetPath);
|
|
if (!fileExists(resolved)) return;
|
|
const inferredKind = kind ?? (statFile(resolved).isDirectory() ? "dir" : "file");
|
|
|
|
const existing = targets.get(resolved);
|
|
if (existing) {
|
|
existing.recursive = existing.recursive || recursive;
|
|
return;
|
|
}
|
|
|
|
targets.set(resolved, { path: resolved, recursive, kind: inferredKind });
|
|
}
|
|
|
|
function addRuntimeFilesFromDir(dirPath: string): void {
|
|
if (!fileExists(dirPath)) return;
|
|
|
|
for (const entry of readDir(dirPath, { withFileTypes: true })) {
|
|
const entryPath = path.join(dirPath, entry.name);
|
|
if (entry.isDirectory()) {
|
|
addRuntimeFilesFromDir(entryPath);
|
|
continue;
|
|
}
|
|
|
|
if (!entry.isFile()) continue;
|
|
if (!entry.name.endsWith(".js") && !entry.name.endsWith(".css")) continue;
|
|
addWatchTarget(entryPath, false, "file");
|
|
}
|
|
}
|
|
|
|
const packageJsonPath = path.join(absPath, "package.json");
|
|
addWatchTarget(packageJsonPath, false, "file");
|
|
if (!fileExists(packageJsonPath)) {
|
|
return [...targets.values()];
|
|
}
|
|
|
|
let packageJson: PluginPackageJson | null = null;
|
|
try {
|
|
packageJson = JSON.parse(readFile(packageJsonPath, "utf8")) as PluginPackageJson;
|
|
} catch {
|
|
packageJson = null;
|
|
}
|
|
|
|
const entrypointPaths = [
|
|
packageJson?.paperclipPlugin?.manifest,
|
|
packageJson?.paperclipPlugin?.worker,
|
|
packageJson?.paperclipPlugin?.ui,
|
|
].filter((value): value is string => typeof value === "string" && value.length > 0);
|
|
|
|
if (entrypointPaths.length === 0) {
|
|
addRuntimeFilesFromDir(path.join(absPath, "dist"));
|
|
return [...targets.values()];
|
|
}
|
|
|
|
for (const relativeEntrypoint of entrypointPaths) {
|
|
const resolvedEntrypoint = path.resolve(absPath, relativeEntrypoint);
|
|
if (!fileExists(resolvedEntrypoint)) continue;
|
|
|
|
const stat = statFile(resolvedEntrypoint);
|
|
if (stat.isDirectory()) {
|
|
addRuntimeFilesFromDir(resolvedEntrypoint);
|
|
} else {
|
|
addWatchTarget(resolvedEntrypoint, false, "file");
|
|
}
|
|
}
|
|
|
|
return [...targets.values()].sort((a, b) => a.path.localeCompare(b.path));
|
|
}
|
|
|
|
/**
|
|
* Create a PluginDevWatcher that monitors local plugin directories and
|
|
* restarts workers on file changes.
|
|
*/
|
|
export function createPluginDevWatcher(
|
|
lifecycle: PluginLifecycleManager,
|
|
resolvePluginPackagePath?: ResolvePluginPackagePath,
|
|
fsDeps?: PluginDevWatcherFsDeps,
|
|
): PluginDevWatcher {
|
|
const watchers = new Map<string, { watcher: FSWatcher; packagePath: string }>();
|
|
const debounceTimers = new Map<string, ReturnType<typeof setTimeout>>();
|
|
const pendingRegistrations = new Map<string, number>();
|
|
// Never reuse an identity after unwatching: an older lookup may still finish.
|
|
let registrationGeneration = 0;
|
|
const fileExists = fsDeps?.existsSync ?? existsSync;
|
|
log.info(
|
|
{ resolvesInstalledPlugins: Boolean(resolvePluginPackagePath) },
|
|
"plugin-dev-watcher: initialized",
|
|
);
|
|
|
|
function watchPlugin(pluginId: string, packagePath: string): void {
|
|
pendingRegistrations.delete(pluginId);
|
|
const absPath = path.resolve(packagePath);
|
|
// A reinstall can move a plugin to another directory. Keep an existing
|
|
// watcher only while it still targets the installed package path.
|
|
if (watchers.get(pluginId)?.packagePath === absPath) return;
|
|
unwatchPlugin(pluginId);
|
|
|
|
if (!fileExists(absPath)) {
|
|
log.warn(
|
|
{ pluginId, packagePath: absPath },
|
|
"plugin-dev-watcher: package path does not exist, skipping watch",
|
|
);
|
|
return;
|
|
}
|
|
|
|
try {
|
|
const watcherTargets = resolvePluginWatchTargets(absPath, fsDeps);
|
|
if (watcherTargets.length === 0) {
|
|
log.warn(
|
|
{ pluginId, packagePath: absPath },
|
|
"plugin-dev-watcher: no valid watch targets found, skipping watch",
|
|
);
|
|
return;
|
|
}
|
|
|
|
const watcher = chokidar.watch(
|
|
watcherTargets.map((target) => target.path),
|
|
{
|
|
ignoreInitial: true,
|
|
awaitWriteFinish: {
|
|
stabilityThreshold: 200,
|
|
pollInterval: 100,
|
|
},
|
|
ignored: (watchedPath) => {
|
|
const relativePath = path.relative(absPath, watchedPath);
|
|
return shouldIgnorePath(relativePath);
|
|
},
|
|
},
|
|
);
|
|
|
|
watcher.on("all", (_eventName, changedPath) => {
|
|
if (watchers.get(pluginId)?.watcher !== watcher) return;
|
|
const relativePath = path.relative(absPath, changedPath);
|
|
if (shouldIgnorePath(relativePath)) return;
|
|
|
|
const existing = debounceTimers.get(pluginId);
|
|
if (existing) clearTimeout(existing);
|
|
|
|
debounceTimers.set(
|
|
pluginId,
|
|
setTimeout(() => {
|
|
if (watchers.get(pluginId)?.watcher !== watcher) return;
|
|
debounceTimers.delete(pluginId);
|
|
log.info(
|
|
{ pluginId, changedFile: relativePath || path.basename(changedPath) },
|
|
"plugin-dev-watcher: file change detected, restarting worker",
|
|
);
|
|
|
|
lifecycle.restartWorker(pluginId).catch((err) => {
|
|
log.warn(
|
|
{
|
|
pluginId,
|
|
err: err instanceof Error ? err.message : String(err),
|
|
},
|
|
"plugin-dev-watcher: failed to restart worker after file change",
|
|
);
|
|
});
|
|
}, DEBOUNCE_MS),
|
|
);
|
|
});
|
|
|
|
watcher.on("error", (err) => {
|
|
if (watchers.get(pluginId)?.watcher !== watcher) return;
|
|
log.warn(
|
|
{
|
|
pluginId,
|
|
packagePath: absPath,
|
|
err: err instanceof Error ? err.message : String(err),
|
|
},
|
|
"plugin-dev-watcher: watcher error, stopping watch for this plugin",
|
|
);
|
|
// Stop this failed watcher without cancelling a pending replacement.
|
|
stopWatchingPlugin(pluginId);
|
|
});
|
|
|
|
watchers.set(pluginId, { watcher, packagePath: absPath });
|
|
log.info(
|
|
{
|
|
pluginId,
|
|
packagePath: absPath,
|
|
watchTargets: watcherTargets.map((target) => ({
|
|
path: target.path,
|
|
kind: target.kind,
|
|
})),
|
|
},
|
|
"plugin-dev-watcher: watching local plugin for changes",
|
|
);
|
|
} catch (err) {
|
|
log.warn(
|
|
{
|
|
pluginId,
|
|
packagePath: absPath,
|
|
err: err instanceof Error ? err.message : String(err),
|
|
},
|
|
"plugin-dev-watcher: failed to start file watcher",
|
|
);
|
|
}
|
|
}
|
|
|
|
function stopWatchingPlugin(pluginId: string): void {
|
|
const pluginWatcher = watchers.get(pluginId);
|
|
if (pluginWatcher) {
|
|
void pluginWatcher.watcher.close();
|
|
watchers.delete(pluginId);
|
|
}
|
|
const timer = debounceTimers.get(pluginId);
|
|
if (timer) {
|
|
clearTimeout(timer);
|
|
debounceTimers.delete(pluginId);
|
|
}
|
|
}
|
|
|
|
function unwatchPlugin(pluginId: string): void {
|
|
pendingRegistrations.delete(pluginId);
|
|
stopWatchingPlugin(pluginId);
|
|
}
|
|
|
|
function close(): void {
|
|
pendingRegistrations.clear();
|
|
lifecycle.off("plugin.loaded", handlePluginLoaded);
|
|
lifecycle.off("plugin.enabled", handlePluginEnabled);
|
|
lifecycle.off("plugin.disabled", handlePluginDisabled);
|
|
lifecycle.off("plugin.unloaded", handlePluginUnloaded);
|
|
|
|
for (const [pluginId] of watchers) {
|
|
unwatchPlugin(pluginId);
|
|
}
|
|
}
|
|
|
|
async function watchLocalPluginById(pluginId: string): Promise<void> {
|
|
if (!resolvePluginPackagePath) {
|
|
log.debug(
|
|
{ pluginId },
|
|
"plugin-dev-watcher: no package path resolver configured, skipping lifecycle event",
|
|
);
|
|
return;
|
|
}
|
|
|
|
const generation = ++registrationGeneration;
|
|
pendingRegistrations.set(pluginId, generation);
|
|
try {
|
|
const packagePath = await resolvePluginPackagePath(pluginId);
|
|
if (pendingRegistrations.get(pluginId) !== generation) return;
|
|
if (!packagePath) {
|
|
unwatchPlugin(pluginId);
|
|
log.debug(
|
|
{ pluginId },
|
|
"plugin-dev-watcher: plugin is not a local-path install, skipping watch",
|
|
);
|
|
return;
|
|
}
|
|
watchPlugin(pluginId, packagePath);
|
|
} catch (err) {
|
|
if (pendingRegistrations.get(pluginId) !== generation) return;
|
|
log.warn(
|
|
{
|
|
pluginId,
|
|
err: err instanceof Error ? err.message : String(err),
|
|
},
|
|
"plugin-dev-watcher: failed to resolve plugin package path",
|
|
);
|
|
} finally {
|
|
if (pendingRegistrations.get(pluginId) === generation) {
|
|
pendingRegistrations.delete(pluginId);
|
|
}
|
|
}
|
|
}
|
|
|
|
function handlePluginLoaded(payload: { pluginId: string }): void {
|
|
void watchLocalPluginById(payload.pluginId);
|
|
}
|
|
|
|
function handlePluginEnabled(payload: { pluginId: string }): void {
|
|
void watchLocalPluginById(payload.pluginId);
|
|
}
|
|
|
|
function handlePluginDisabled(payload: { pluginId: string }): void {
|
|
unwatchPlugin(payload.pluginId);
|
|
}
|
|
|
|
function handlePluginUnloaded(payload: { pluginId: string }): void {
|
|
unwatchPlugin(payload.pluginId);
|
|
}
|
|
|
|
lifecycle.on("plugin.loaded", handlePluginLoaded);
|
|
lifecycle.on("plugin.enabled", handlePluginEnabled);
|
|
lifecycle.on("plugin.disabled", handlePluginDisabled);
|
|
lifecycle.on("plugin.unloaded", handlePluginUnloaded);
|
|
|
|
return {
|
|
watch: watchPlugin,
|
|
unwatch: unwatchPlugin,
|
|
close,
|
|
};
|
|
}
|