diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e424da2b86..af137d95977 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Docs: https://docs.openclaw.ai - Plugins/CLI: make plugin install and uninstall config writes conflict-aware, clear stale denylist entries on explicit reinstall/removal, and delete managed plugin files only after config/index commit succeeds. Thanks @codex. - Plugins: fail `plugins update` when tracked plugin or hook updates error, keep bundled runtime-dependency repair behind restrictive allowlists, and reject package installs with unloadable extension entries. Thanks @codex. - WebChat/Control UI: support non-video file attachments in chat uploads while preserving the existing image attachment path and MIME-sniff fallback for generic image uploads. (#70947) Thanks @IAMSamuelRodda. +- Skills/memory: restore Chokidar v5 hot reloads by watching concrete skill and memory roots with filters, including SKILL.md removals and deleted skill folders without broad workspace recursion. Fixes #27404, #33585, and #41606. Thanks @shelvenzhou, @08820048, and @rocke2020. - Gateway/chat: keep duplicate attachment-backed `chat.send` retries with the same idempotency key on the documented in-flight path so aborts still target the real active run. Fixes #70139. Thanks @Feelw00. - Plugins: share package entrypoint resolution between install and discovery, reject mismatched `runtimeExtensions`, and cache bundled runtime-dependency manifest reads during scans. Thanks @codex. - WhatsApp/Web: keep quiet but healthy linked-device sessions connected by basing the watchdog on WhatsApp Web transport activity, while retaining a longer app-silence cap so frame activity cannot mask a stuck session forever. Fixes #70678; carries forward the focused #71466 approach and keeps #63939 as related configurable-timeout follow-up. Thanks @vincentkoc and @oromeis. diff --git a/extensions/memory-core/src/memory/manager-sync-ops.ts b/extensions/memory-core/src/memory/manager-sync-ops.ts index 22ef640044e..53d3cceb284 100644 --- a/extensions/memory-core/src/memory/manager-sync-ops.ts +++ b/extensions/memory-core/src/memory/manager-sync-ops.ts @@ -5,11 +5,7 @@ import path from "node:path"; import type { DatabaseSync } from "node:sqlite"; import chokidar, { FSWatcher } from "chokidar"; import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; -import { - buildCaseInsensitiveExtensionGlob, - classifyMemoryMultimodalPath, - getMemoryMultimodalExtensions, -} from "openclaw/plugin-sdk/memory-core-host-engine-embeddings"; +import { classifyMemoryMultimodalPath } from "openclaw/plugin-sdk/memory-core-host-engine-embeddings"; import { createSubsystemLogger, onSessionTranscriptUpdate, @@ -105,6 +101,9 @@ function shouldIgnoreMemoryWatchPath( if (stats?.isDirectory?.()) { return false; } + if (!stats) { + return false; + } const extension = normalizeLowercaseStringOrEmpty(path.extname(normalized)); if (extension.length === 0 || extension === ".md") { return false; @@ -383,16 +382,7 @@ export abstract class MemoryManagerSyncOps { continue; } if (stat.isDirectory()) { - watchPaths.add(path.join(entry, "**", "*.md")); - if (this.settings.multimodal.enabled) { - for (const modality of this.settings.multimodal.modalities) { - for (const extension of getMemoryMultimodalExtensions(modality)) { - watchPaths.add( - path.join(entry, "**", buildCaseInsensitiveExtensionGlob(extension)), - ); - } - } - } + watchPaths.add(entry); continue; } if ( @@ -422,6 +412,7 @@ export abstract class MemoryManagerSyncOps { this.watcher.on("add", markDirty); this.watcher.on("change", markDirty); this.watcher.on("unlink", markDirty); + this.watcher.on("unlinkDir", markDirty); } protected ensureSessionListener() { diff --git a/extensions/memory-core/src/memory/manager.watcher-config.test.ts b/extensions/memory-core/src/memory/manager.watcher-config.test.ts index 31fa7b1135f..64da449505a 100644 --- a/extensions/memory-core/src/memory/manager.watcher-config.test.ts +++ b/extensions/memory-core/src/memory/manager.watcher-config.test.ts @@ -11,12 +11,35 @@ import { registerBuiltInMemoryEmbeddingProviders } from "./provider-adapters.js" type WatchIgnoredFn = (watchPath: string, stats?: { isDirectory?: () => boolean }) => boolean; -const { watchMock } = vi.hoisted(() => ({ - watchMock: vi.fn(() => ({ - on: vi.fn(), - close: vi.fn(async () => undefined), - })), -})); +const { createdWatchers, watchMock } = vi.hoisted(() => { + type WatchEvent = "add" | "change" | "unlink" | "unlinkDir"; + type WatchCallback = () => void; + function createMockWatcher() { + const handlers = new Map(); + const watcher = { + on: vi.fn((event: WatchEvent, callback: WatchCallback) => { + handlers.set(event, [...(handlers.get(event) ?? []), callback]); + return watcher; + }), + close: vi.fn(async () => undefined), + emit: (event: WatchEvent) => { + for (const callback of handlers.get(event) ?? []) { + callback(); + } + }, + }; + return watcher; + } + const watchers: Array> = []; + return { + createdWatchers: watchers, + watchMock: vi.fn(() => { + const watcher = createMockWatcher(); + watchers.push(watcher); + return watcher; + }), + }; +}); vi.mock("chokidar", () => ({ default: { watch: watchMock }, @@ -69,7 +92,9 @@ describe("memory watcher config", () => { }); afterEach(async () => { + vi.useRealTimers(); watchMock.mockClear(); + createdWatchers.length = 0; if (manager) { await manager.close(); manager = null; @@ -140,9 +165,10 @@ describe("memory watcher config", () => { expect.arrayContaining([ path.join(workspaceDir, "MEMORY.md"), path.join(workspaceDir, "memory"), - path.join(extraDir, "**", "*.md"), + extraDir, ]), ); + expect(watchedPaths.every((watchPath) => !watchPath.includes("*"))).toBe(true); expect(options.ignoreInitial).toBe(true); expect(options.awaitWriteFinish).toEqual({ stabilityThreshold: 25, pollInterval: 100 }); @@ -152,15 +178,19 @@ describe("memory watcher config", () => { true, ); expect(ignored?.(path.join(workspaceDir, "memory", ".venv", "lib", "python.md"))).toBe(true); - expect(ignored?.(path.join(workspaceDir, "memory", "project", "notes.tmp"))).toBe(true); - expect(ignored?.(path.join(workspaceDir, "memory", "project", "notes.json"))).toBe(true); + expect(ignored?.(path.join(workspaceDir, "memory", "project", "notes.tmp"), {})).toBe(true); + expect(ignored?.(path.join(workspaceDir, "memory", "project", "notes.json"), {})).toBe(true); + expect(ignored?.(path.join(workspaceDir, "memory", "project", "notes.json"), undefined)).toBe( + false, + ); expect(ignored?.(path.join(workspaceDir, "memory", "project", "notes.md"))).toBe(false); + expect(ignored?.(path.join(workspaceDir, "memory", "project", "notes.md"), {})).toBe(false); expect( ignored?.(path.join(workspaceDir, "memory", "project"), { isDirectory: () => true }), ).toBe(false); }); - it("watches multimodal extensions with case-insensitive globs", async () => { + it("watches multimodal extra directories with filtered extensions", async () => { await setupWatcherWorkspace({ name: "PHOTO.PNG", contents: "png" }); const cfg = createWatcherConfig({ provider: "gemini", @@ -177,16 +207,40 @@ describe("memory watcher config", () => { Record, ]; expect(watchedPaths).toEqual( - expect.arrayContaining([ - path.join(extraDir, "**", "*.[pP][nN][gG]"), - path.join(extraDir, "**", "*.[wW][aA][vV]"), - ]), + expect.arrayContaining([path.join(workspaceDir, "MEMORY.md"), path.join(extraDir)]), ); + expect(watchedPaths.every((watchPath) => !watchPath.includes("*"))).toBe(true); const ignored = options.ignored as WatchIgnoredFn | undefined; expect(ignored).toBeTypeOf("function"); expect(ignored?.(path.join(extraDir, "nested", "PHOTO.PNG"))).toBe(false); + expect(ignored?.(path.join(extraDir, "nested", "PHOTO.PNG"), {})).toBe(false); expect(ignored?.(path.join(extraDir, "nested", "voice.WAV"))).toBe(false); - expect(ignored?.(path.join(extraDir, "nested", "metadata.json"))).toBe(true); + expect(ignored?.(path.join(extraDir, "nested", "voice.WAV"), {})).toBe(false); + expect(ignored?.(path.join(extraDir, "nested", "metadata.json"), {})).toBe(true); }); + + it.each(["add", "change", "unlink", "unlinkDir"] as const)( + "schedules watch sync on %s", + async (event) => { + await setupWatcherWorkspace({ name: "notes.md", contents: "hello" }); + const cfg = createWatcherConfig(); + + await expectWatcherManager(cfg); + vi.useFakeTimers(); + const syncSpy = vi + .spyOn( + manager as unknown as { + sync: (params?: { reason?: string }) => Promise; + }, + "sync", + ) + .mockResolvedValue(undefined); + + createdWatchers[0]?.emit(event); + await vi.advanceTimersByTimeAsync(25); + + expect(syncSpy).toHaveBeenCalledWith({ reason: "watch" }); + }, + ); }); diff --git a/src/agents/skills/refresh.test.ts b/src/agents/skills/refresh.test.ts index 087f61221e3..590b30fae74 100644 --- a/src/agents/skills/refresh.test.ts +++ b/src/agents/skills/refresh.test.ts @@ -1,11 +1,34 @@ import os from "node:os"; import path from "node:path"; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import type { SkillsChangeEvent } from "./refresh.js"; -const watchMock = vi.fn(() => ({ - on: vi.fn(), - close: vi.fn(async () => undefined), -})); +type WatchEvent = "add" | "change" | "unlink" | "unlinkDir" | "error"; +type WatchCallback = (watchPath: string) => void; + +function createMockWatcher() { + const handlers = new Map(); + const watcher = { + on: vi.fn((event: WatchEvent, callback: WatchCallback) => { + handlers.set(event, [...(handlers.get(event) ?? []), callback]); + return watcher; + }), + close: vi.fn(async () => undefined), + emit: (event: WatchEvent, watchPath: string) => { + for (const callback of handlers.get(event) ?? []) { + callback(watchPath); + } + }, + }; + return watcher; +} + +const createdWatchers: Array> = []; +const watchMock = vi.fn(() => { + const watcher = createMockWatcher(); + createdWatchers.push(watcher); + return watcher; +}); let refreshModule: typeof import("./refresh.js"); @@ -24,13 +47,15 @@ describe("ensureSkillsWatcher", () => { beforeEach(() => { watchMock.mockClear(); + createdWatchers.length = 0; }); afterEach(async () => { + vi.useRealTimers(); await refreshModule.resetSkillsRefreshForTest(); }); - it("ignores node_modules, dist, .git, and Python venvs by default", async () => { + it("watches skill roots and filters non-skill churn", async () => { refreshModule.ensureSkillsWatcher({ workspaceDir: "/tmp/workspace" }); expect(watchMock).toHaveBeenCalledTimes(1); @@ -40,49 +65,64 @@ describe("ensureSkillsWatcher", () => { const targets = firstCall?.[0] ?? []; const opts = firstCall?.[1] ?? {}; - expect(opts.ignored).toBe(refreshModule.DEFAULT_SKILLS_WATCH_IGNORED); + expect(opts.ignored).toBe(refreshModule.shouldIgnoreSkillsWatchPath); const posix = (p: string) => p.replaceAll("\\", "/"); expect(targets).toEqual( expect.arrayContaining([ - posix(path.join("/tmp/workspace", "skills", "SKILL.md")), - posix(path.join("/tmp/workspace", "skills", "*", "SKILL.md")), - posix(path.join("/tmp/workspace", ".agents", "skills", "SKILL.md")), - posix(path.join("/tmp/workspace", ".agents", "skills", "*", "SKILL.md")), - posix(path.join(os.homedir(), ".agents", "skills", "SKILL.md")), - posix(path.join(os.homedir(), ".agents", "skills", "*", "SKILL.md")), + posix(path.join("/tmp/workspace", "skills")), + posix(path.join("/tmp/workspace", ".agents", "skills")), + posix(path.join(os.homedir(), ".agents", "skills")), ]), ); - expect(targets.every((target) => target.includes("SKILL.md"))).toBe(true); - const ignored = refreshModule.DEFAULT_SKILLS_WATCH_IGNORED; + expect(targets.every((target) => !target.includes("*"))).toBe(true); + const ignored = refreshModule.shouldIgnoreSkillsWatchPath; // Node/JS paths - expect(ignored.some((re) => re.test("/tmp/workspace/skills/node_modules/pkg/index.js"))).toBe( - true, - ); - expect(ignored.some((re) => re.test("/tmp/workspace/skills/dist/index.js"))).toBe(true); - expect(ignored.some((re) => re.test("/tmp/workspace/skills/.git/config"))).toBe(true); + expect(ignored("/tmp/workspace/skills/node_modules/pkg/index.js")).toBe(true); + expect(ignored("/tmp/workspace/skills/dist/index.js")).toBe(true); + expect(ignored("/tmp/workspace/skills/.git/config")).toBe(true); // Python virtual environments and caches - expect(ignored.some((re) => re.test("/tmp/workspace/skills/scripts/.venv/bin/python"))).toBe( - true, - ); - expect(ignored.some((re) => re.test("/tmp/workspace/skills/venv/lib/python3.10/site.py"))).toBe( - true, - ); - expect(ignored.some((re) => re.test("/tmp/workspace/skills/__pycache__/module.pyc"))).toBe( - true, - ); - expect(ignored.some((re) => re.test("/tmp/workspace/skills/.mypy_cache/3.10/foo.json"))).toBe( - true, - ); - expect(ignored.some((re) => re.test("/tmp/workspace/skills/.pytest_cache/v/cache"))).toBe(true); + expect(ignored("/tmp/workspace/skills/scripts/.venv/bin/python")).toBe(true); + expect(ignored("/tmp/workspace/skills/venv/lib/python3.10/site.py")).toBe(true); + expect(ignored("/tmp/workspace/skills/__pycache__/module.pyc")).toBe(true); + expect(ignored("/tmp/workspace/skills/.mypy_cache/3.10/foo.json")).toBe(true); + expect(ignored("/tmp/workspace/skills/.pytest_cache/v/cache")).toBe(true); // Build artifacts and caches - expect(ignored.some((re) => re.test("/tmp/workspace/skills/build/output.js"))).toBe(true); - expect(ignored.some((re) => re.test("/tmp/workspace/skills/.cache/data.json"))).toBe(true); + expect(ignored("/tmp/workspace/skills/build/output.js")).toBe(true); + expect(ignored("/tmp/workspace/skills/.cache/data.json")).toBe(true); // Should NOT ignore normal skill files - expect(ignored.some((re) => re.test("/tmp/.hidden/skills/index.md"))).toBe(false); - expect(ignored.some((re) => re.test("/tmp/workspace/skills/my-skill/SKILL.md"))).toBe(false); + expect(ignored("/tmp/.hidden/skills/index.md")).toBe(false); + expect(ignored("/tmp/workspace/skills/my-skill", { isDirectory: () => true })).toBe(false); + expect(ignored("/tmp/workspace/skills/my-skill/README.md", {})).toBe(true); + expect(ignored("/tmp/workspace/skills/my-skill/SKILL.md", {})).toBe(false); }); + + it.each(["add", "change", "unlink", "unlinkDir"] as const)( + "refreshes skills snapshots on %s", + async (event) => { + vi.useFakeTimers(); + const seen: SkillsChangeEvent[] = []; + refreshModule.registerSkillsChangeListener((change) => { + seen.push(change); + }); + refreshModule.ensureSkillsWatcher({ + workspaceDir: "/tmp/workspace", + config: { skills: { load: { watchDebounceMs: 10 } } }, + }); + + createdWatchers[0]?.emit(event, "/tmp/workspace/skills/demo/SKILL.md"); + await vi.advanceTimersByTimeAsync(10); + + expect(seen).toEqual([ + { + workspaceDir: "/tmp/workspace", + reason: "watch", + changedPath: "/tmp/workspace/skills/demo/SKILL.md", + }, + ]); + }, + ); }); diff --git a/src/agents/skills/refresh.ts b/src/agents/skills/refresh.ts index ab16db8e9ac..b7e2d0363e9 100644 --- a/src/agents/skills/refresh.ts +++ b/src/agents/skills/refresh.ts @@ -72,26 +72,36 @@ function resolveWatchPaths(workspaceDir: string, config?: OpenClawConfig): strin return paths; } -function toWatchGlobRoot(raw: string): string { - // Chokidar treats globs as POSIX-ish patterns. Normalize Windows separators - // so `*` works consistently across platforms. - return raw.replaceAll("\\", "/").replace(/\/+$/, ""); +function toWatchRoot(raw: string): string { + const normalized = raw.replaceAll("\\", "/"); + return normalized.replace(/\/+$/, "") || normalized; } function resolveWatchTargets(workspaceDir: string, config?: OpenClawConfig): string[] { - // Skills are defined by SKILL.md; watch only those files to avoid traversing - // or watching unrelated large trees (e.g. datasets) that can exhaust FDs. const targets = new Set(); for (const root of resolveWatchPaths(workspaceDir, config)) { - const globRoot = toWatchGlobRoot(root); - // Some configs point directly at a skill folder. - targets.add(`${globRoot}/SKILL.md`); - // Standard layout: //SKILL.md - targets.add(`${globRoot}/*/SKILL.md`); + targets.add(toWatchRoot(root)); } return Array.from(targets).toSorted(); } +export function shouldIgnoreSkillsWatchPath( + watchPath: string, + stats?: { isDirectory?: () => boolean }, +): boolean { + if (DEFAULT_SKILLS_WATCH_IGNORED.some((re) => re.test(watchPath))) { + return true; + } + if (stats?.isDirectory?.()) { + return false; + } + if (!stats) { + return false; + } + const normalized = watchPath.replaceAll("\\", "/"); + return path.posix.basename(normalized) !== "SKILL.md"; +} + export function ensureSkillsWatcher(params: { workspaceDir: string; config?: OpenClawConfig }) { const workspaceDir = params.workspaceDir.trim(); if (!workspaceDir) { @@ -135,9 +145,7 @@ export function ensureSkillsWatcher(params: { workspaceDir: string; config?: Ope stabilityThreshold: debounceMs, pollInterval: 100, }, - // Avoid FD exhaustion on macOS when a workspace contains huge trees. - // This watcher only needs to react to SKILL.md changes. - ignored: DEFAULT_SKILLS_WATCH_IGNORED, + ignored: shouldIgnoreSkillsWatchPath, }); const state: SkillsWatchState = { watcher, pathsKey, debounceMs }; @@ -162,6 +170,7 @@ export function ensureSkillsWatcher(params: { workspaceDir: string; config?: Ope watcher.on("add", (p) => schedule(p)); watcher.on("change", (p) => schedule(p)); watcher.on("unlink", (p) => schedule(p)); + watcher.on("unlinkDir", (p) => schedule(p)); watcher.on("error", (err) => { log.warn(`skills watcher error (${workspaceDir}): ${String(err)}`); });