Skip to content

Commit 2bd5d17

Browse files
committed
fix(watchers): revive dead skill and memory file watchers
chokidar v4+ dropped glob support, so the literal glob paths handed to chokidar (skills/watcher.ts, memory/index.ts) matched nothing: the watchers registered but never fired. Skill hot-reload was fully dead, and root-level .md edits never triggered a memory reindex. Watch the containing directories instead and filter events in the handler: - skills: watch the skill roots, react only to SKILL.md basenames - memory: watch the workspace root + sessions dir, filter to indexed files - replace the unanchored substring ignore regexes (/build/, /dist/, ...) with exact path-segment matching scoped below the watch roots, so a skill named 'prompt-builder' or a memory file like 'node_modules-notes.md' is no longer silently pruned Add integration tests that write real files and assert the watcher fires, and that vendored dirs / non-target files stay ignored.
1 parent e765e93 commit 2bd5d17

5 files changed

Lines changed: 331 additions & 35 deletions

File tree

src/memory/index.ts

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
import { Database } from "bun:sqlite";
1515
import { existsSync, readFileSync, statSync, mkdirSync, readdirSync } from "node:fs";
16-
import { join, dirname } from "node:path";
16+
import { join, dirname, sep } from "node:path";
1717
import { createSubsystemLogger } from "../logging/index.js";
1818
import { getConfigDir } from "../storage/config.js";
1919
import { createSchema, setMeta, getMeta } from "./schema.js";
@@ -792,22 +792,38 @@ export class MemoryIndex {
792792

793793
private startWatcher(): void {
794794
const wsPath = this.workspacePath;
795-
const watchPaths: string[] = [];
795+
const sessionsPath = this.sessionsPath;
796796

797+
// chokidar v4+ dropped glob support, so `wsPath/*.md` no longer matches
798+
// anything. Watch the containing directories and filter changed paths down
799+
// to the files listWorkspaceFiles() would actually index.
800+
const watchDirs: string[] = [];
797801
if (existsSync(wsPath)) {
798-
watchPaths.push(join(wsPath, "*.md"), join(wsPath, "memory"));
802+
watchDirs.push(wsPath); // covers root *.md and the memory/ subtree
799803
}
800-
801-
// Watch sessions directory for changes (even if workspace is absent)
802-
if (this.sessionsPath && existsSync(this.sessionsPath)) {
803-
watchPaths.push(this.sessionsPath);
804+
if (sessionsPath && existsSync(sessionsPath)) {
805+
watchDirs.push(sessionsPath);
804806
}
805807

806-
if (watchPaths.length === 0) return;
808+
if (watchDirs.length === 0) return;
809+
810+
const memDir = join(wsPath, "memory");
811+
const isInside = (dir: string, p: string): boolean =>
812+
p === dir || p.startsWith(dir.endsWith(sep) ? dir : dir + sep);
813+
814+
const shouldReindex = (p: string): boolean => {
815+
// Session JSONL files (recursive)
816+
if (sessionsPath && isInside(sessionsPath, p)) return p.endsWith(".jsonl");
817+
// memory/ subtree: indexed .md and .jsonl (recursive)
818+
if (isInside(memDir, p)) return p.endsWith(".md") || p.endsWith(".jsonl");
819+
// Workspace root: top-level .md files only (not nested)
820+
if (dirname(p) === wsPath) return p.endsWith(".md");
821+
return false;
822+
};
807823

808-
this.watcher = createMemoryWatcher(watchPaths, () => {
824+
this.watcher = createMemoryWatcher(watchDirs, () => {
809825
this.dirty = true;
810-
});
826+
}, shouldReindex);
811827
}
812828
}
813829

src/memory/watcher.ts

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,29 +7,58 @@
77
// =============================================================================
88

99
import { watch } from "chokidar";
10+
import { sep } from "node:path";
1011

1112
export interface MemoryWatcher {
1213
close(): void;
1314
}
1415

16+
// Vendored/generated directories we never descend into. Matched per path
17+
// *segment* (not as substrings), so a memory file like "node_modules-notes.md"
18+
// or a path under a project that merely contains ".git" is not mistaken for
19+
// one of these directories.
20+
const IGNORED_DIRS = new Set([".git", "node_modules"]);
21+
22+
/**
23+
* Build a chokidar `ignored` predicate that skips IGNORED_DIRS by exact path
24+
* segment, only for segments below one of the watch roots — never the roots or
25+
* their ancestors (a root may live under e.g. a `node_modules/` directory).
26+
*/
27+
function makeIgnored(roots: string[]): (path: string) => boolean {
28+
const normRoots = roots.map((r) => (r.endsWith(sep) ? r.slice(0, -1) : r));
29+
return (path: string): boolean => {
30+
const root = normRoots.find((r) => path === r || path.startsWith(r + sep));
31+
if (!root) return false;
32+
const rel = path.slice(root.length + 1);
33+
if (!rel) return false; // the root itself
34+
return rel.split(sep).some((segment) => IGNORED_DIRS.has(segment));
35+
};
36+
}
37+
1538
/**
1639
* Create a file watcher for workspace memory files.
1740
* Calls onDirty() when files change (debounced).
41+
*
42+
* `watchPaths` must be plain directories — chokidar v4+ dropped glob support,
43+
* so callers watch containing directories and pass `filter` to decide which
44+
* changed paths are relevant (e.g. only top-level `.md`, or `.jsonl` sessions).
1845
*/
1946
export function createMemoryWatcher(
2047
watchPaths: string[],
2148
onDirty: () => void,
49+
filter?: (path: string) => boolean,
2250
debounceMs = 1500,
2351
): MemoryWatcher {
2452
let timer: ReturnType<typeof setTimeout> | null = null;
2553

2654
const watcher = watch(watchPaths, {
2755
ignoreInitial: true,
2856
depth: 2,
29-
ignored: [/\.git/, /node_modules/],
57+
ignored: makeIgnored(watchPaths),
3058
});
3159

32-
watcher.on("all", () => {
60+
watcher.on("all", (_event, path) => {
61+
if (filter && (!path || !filter(path))) return;
3362
if (timer) clearTimeout(timer);
3463
timer = setTimeout(onDirty, debounceMs);
3564
});

src/skills/watcher.ts

Lines changed: 47 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
// =============================================================================
77

88
import { watch } from "chokidar";
9-
import { join } from "node:path";
9+
import { join, basename, sep } from "node:path";
1010
import { getConfigDir } from "../storage/config.js";
1111

1212
// -----------------------------------------------------------------------------
@@ -16,17 +16,38 @@ import { getConfigDir } from "../storage/config.js";
1616
let dirty = false;
1717
let watcherInstance: { close: () => void } | null = null;
1818

19-
// Ignored directories (same as memory watcher)
20-
const IGNORED = [
21-
/\.git/,
22-
/node_modules/,
23-
/dist/,
24-
/\.venv/,
25-
/venv/,
26-
/__pycache__/,
27-
/build/,
28-
/\.cache/,
29-
];
19+
// Vendored/generated directories we never descend into. Matched per path
20+
// *segment* (not as substrings), and only below the watch roots, so a skill
21+
// named "prompt-builder" or even "build" is not mistaken for a "build" dir.
22+
const IGNORED_DIRS = new Set([
23+
".git",
24+
"node_modules",
25+
"dist",
26+
".venv",
27+
"venv",
28+
"__pycache__",
29+
"build",
30+
".cache",
31+
]);
32+
33+
/**
34+
* Build a chokidar `ignored` predicate that skips IGNORED_DIRS by exact path
35+
* segment, scoped to *inside* a skill folder. Never treats a watch root, its
36+
* ancestors (a root may live under e.g. a `build/` directory), or a skill
37+
* folder's own name (a skill may be named "build") as junk — only vendored
38+
* dirs nested within a skill.
39+
*/
40+
function makeIgnored(roots: string[]): (path: string) => boolean {
41+
const normRoots = roots.map((r) => (r.endsWith(sep) ? r.slice(0, -1) : r));
42+
return (path: string): boolean => {
43+
const root = normRoots.find((r) => path === r || path.startsWith(r + sep));
44+
if (!root) return false;
45+
const rel = path.slice(root.length + 1);
46+
if (!rel) return false; // the root itself
47+
// rel[0] is the skill folder name; only segments *within* it can be junk.
48+
return rel.split(sep).slice(1).some((segment) => IGNORED_DIRS.has(segment));
49+
};
50+
}
3051

3152
// -----------------------------------------------------------------------------
3253
// Public API
@@ -54,28 +75,32 @@ export function markSkillsDirty(): void {
5475
export function startSkillsWatcher(workspacePath?: string): void {
5576
if (watcherInstance) return; // Already watching
5677

57-
const watchPaths: string[] = [];
78+
// chokidar v4+ dropped glob support, so a path containing "*" is treated as a
79+
// literal filename and matches nothing. Watch the containing skill directories
80+
// directly and filter events down to SKILL.md files in the handler.
81+
const watchDirs: string[] = [];
5882

59-
// User skills
60-
const userSkillsDir = join(getConfigDir(), "skills");
61-
watchPaths.push(join(userSkillsDir, "*/SKILL.md"));
83+
// User skills: <config root>/skills/<name>/SKILL.md
84+
watchDirs.push(join(getConfigDir(), "skills"));
6285

63-
// Workspace skills
86+
// Workspace skills: <workspace>/skills/<name>/SKILL.md
6487
if (workspacePath) {
65-
watchPaths.push(join(workspacePath, "skills", "*/SKILL.md"));
88+
watchDirs.push(join(workspacePath, "skills"));
6689
}
6790

68-
if (watchPaths.length === 0) return;
91+
if (watchDirs.length === 0) return;
6992

7093
let timer: ReturnType<typeof setTimeout> | null = null;
7194

72-
const watcher = watch(watchPaths, {
95+
const watcher = watch(watchDirs, {
7396
ignoreInitial: true,
7497
depth: 2,
75-
ignored: IGNORED,
98+
ignored: makeIgnored(watchDirs),
7699
});
77100

78-
watcher.on("all", () => {
101+
watcher.on("all", (_event, path) => {
102+
// The directory watch also sees sibling files; only SKILL.md matters.
103+
if (!path || basename(path) !== "SKILL.md") return;
79104
if (timer) clearTimeout(timer);
80105
timer = setTimeout(() => {
81106
dirty = true;

tests/test-memory-watcher.ts

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
// =============================================================================
2+
// Memory Watcher Tests
3+
//
4+
// Regression guard for #9 — createMemoryWatcher previously received glob paths
5+
// (e.g. "<ws>/*.md") from startWatcher(). chokidar v4+ dropped glob support, so
6+
// those paths matched nothing and the watcher never fired. These tests write
7+
// real files under a watched directory and assert the debounced onDirty runs,
8+
// and that the path filter suppresses irrelevant files.
9+
// =============================================================================
10+
11+
import { describe, expect, test, beforeEach, afterEach } from "bun:test";
12+
import { mkdirSync, writeFileSync, rmSync, existsSync } from "node:fs";
13+
import { join } from "node:path";
14+
import { tmpdir } from "node:os";
15+
import { createMemoryWatcher, type MemoryWatcher } from "../src/memory/watcher.js";
16+
import { MemoryIndex } from "../src/memory/index.js";
17+
18+
let tempDir: string;
19+
let watcher: MemoryWatcher | null = null;
20+
21+
async function pollFor(fn: () => Promise<boolean>, timeoutMs = 5000, stepMs = 150): Promise<boolean> {
22+
const start = Date.now();
23+
while (Date.now() - start < timeoutMs) {
24+
if (await fn()) return true;
25+
await new Promise((r) => setTimeout(r, stepMs));
26+
}
27+
return fn();
28+
}
29+
30+
async function waitFor(cond: () => boolean, timeoutMs = 5000, stepMs = 50): Promise<boolean> {
31+
const start = Date.now();
32+
while (Date.now() - start < timeoutMs) {
33+
if (cond()) return true;
34+
await new Promise((r) => setTimeout(r, stepMs));
35+
}
36+
return cond();
37+
}
38+
39+
beforeEach(() => {
40+
tempDir = join(tmpdir(), `hawky-mem-watch-${Date.now()}-${Math.random().toString(36).slice(2)}`);
41+
mkdirSync(tempDir, { recursive: true });
42+
});
43+
44+
afterEach(() => {
45+
watcher?.close();
46+
watcher = null;
47+
if (existsSync(tempDir)) rmSync(tempDir, { recursive: true, force: true });
48+
});
49+
50+
describe("createMemoryWatcher fires on real file changes", () => {
51+
test("writing a top-level .md triggers onDirty", async () => {
52+
let dirty = false;
53+
// Short debounce so the test stays fast.
54+
watcher = createMemoryWatcher([tempDir], () => { dirty = true; }, undefined, 50);
55+
56+
// Let chokidar finish its initial scan before writing (ignoreInitial: true).
57+
await new Promise((r) => setTimeout(r, 400));
58+
writeFileSync(join(tempDir, "HAWKY.md"), "# root memory\n");
59+
60+
expect(await waitFor(() => dirty)).toBe(true);
61+
});
62+
63+
test("filter suppresses irrelevant files but allows .md", async () => {
64+
let dirty = false;
65+
const filter = (p: string) => p.endsWith(".md");
66+
watcher = createMemoryWatcher([tempDir], () => { dirty = true; }, filter, 50);
67+
68+
await new Promise((r) => setTimeout(r, 400));
69+
// A non-.md write must NOT flip dirty.
70+
writeFileSync(join(tempDir, "scratch.txt"), "ignore me\n");
71+
await new Promise((r) => setTimeout(r, 600));
72+
expect(dirty).toBe(false);
73+
74+
// A .md write in the same directory must flip it.
75+
writeFileSync(join(tempDir, "notes.md"), "index me\n");
76+
expect(await waitFor(() => dirty)).toBe(true);
77+
});
78+
79+
// Regression: the old ignore was /node_modules/ (unanchored substring), which
80+
// pruned a file whose name merely contains "node_modules".
81+
test("a .md whose name contains an ignored word still fires", async () => {
82+
let dirty = false;
83+
const filter = (p: string) => p.endsWith(".md");
84+
watcher = createMemoryWatcher([tempDir], () => { dirty = true; }, filter, 50);
85+
86+
await new Promise((r) => setTimeout(r, 400));
87+
writeFileSync(join(tempDir, "node_modules-notes.md"), "still index me\n");
88+
expect(await waitFor(() => dirty)).toBe(true);
89+
});
90+
91+
// A real vendored directory nested under the root must still be ignored.
92+
test("a file inside a real node_modules/ dir is ignored", async () => {
93+
let dirty = false;
94+
const filter = (p: string) => p.endsWith(".md");
95+
watcher = createMemoryWatcher([tempDir], () => { dirty = true; }, filter, 50);
96+
97+
await new Promise((r) => setTimeout(r, 400));
98+
const nm = join(tempDir, "node_modules");
99+
mkdirSync(nm, { recursive: true });
100+
writeFileSync(join(nm, "dep.md"), "vendored\n");
101+
102+
// Past the debounce window — must NOT have fired.
103+
await new Promise((r) => setTimeout(r, 600));
104+
expect(dirty).toBe(false);
105+
});
106+
});
107+
108+
// End-to-end through the real MemoryIndex: exercises the startWatcher()
109+
// `dirname(p) === wsPath` branch (root .md) that the unit tests above don't
110+
// cover, and proves a live edit becomes searchable — the exact behavior #9
111+
// silently lost.
112+
describe("MemoryIndex reindexes a live-edited root .md", () => {
113+
test("a top-level .md written after startup becomes searchable", async () => {
114+
const dbPath = join(tempDir, "index.db");
115+
const wsPath = join(tempDir, "workspace");
116+
mkdirSync(wsPath, { recursive: true });
117+
118+
const idx = new MemoryIndex({
119+
workspacePath: wsPath,
120+
dbPath,
121+
sessionsPath: null,
122+
enableWatcher: true,
123+
});
124+
try {
125+
// First search clears the startup dirty flag (initial sync).
126+
await idx.search("warmup");
127+
// Let chokidar finish its initial scan (ignoreInitial: true).
128+
await new Promise((r) => setTimeout(r, 500));
129+
130+
const marker = `zqxmarker${Math.random().toString(36).slice(2)}`;
131+
writeFileSync(join(wsPath, "PROBE.md"), `# probe\n${marker} lives here\n`);
132+
133+
// The watcher (1500ms debounce) must flip dirty so the next search syncs
134+
// and indexes the new file. Poll until the marker is retrievable.
135+
const found = await pollFor(async () => {
136+
const results = await idx.search(marker);
137+
return results.some((r) => JSON.stringify(r).includes(marker));
138+
}, 6000);
139+
expect(found).toBe(true);
140+
} finally {
141+
idx.close();
142+
}
143+
});
144+
});

0 commit comments

Comments
 (0)