diff --git a/src/config/config-watcher.ts b/src/config/config-watcher.ts new file mode 100644 index 0000000..82b4d14 --- /dev/null +++ b/src/config/config-watcher.ts @@ -0,0 +1,111 @@ +/** + * Reliable watcher for the user-level config file (~/.champ/config.yaml). + * + * Issue #129: the previous implementation relied on + * vscode.workspace.createFileSystemWatcher with a RelativePattern rooted at + * os.homedir(). Watching a path OUTSIDE the active workspace via + * FileSystemWatcher is unreliable on Linux (microsoft/vscode out-of-workspace + * watching), so edits made from a terminal/external tool were frequently + * missed — the provider kept a stale config until a full window restart. + * + * This combines two signals, both debounced: + * - An injected native watcher (the VS Code FileSystemWatcher fast path), + * which fires immediately for in-VS-Code edits. + * - A content signal poller: it stats the file's mtime every pollIntervalMs + * and schedules a reload only when the mtime actually changes. This is + * platform-independent and catches edits made outside the workspace. + * + * The poller is self-contained and dependency-injected so it is unit-testable + * with a fake clock and a fake stat() — no VS Code API required. + */ + +export interface ConfigWatcherDeps { + /** Absolute path of the config file to watch. */ + path: string; + /** Debounced callback invoked when the config file changes. */ + onChange: () => void; + /** Debounce window in ms before onChange fires. Default 300. */ + debounceMs?: number; + /** Poll interval in ms. Default 2000. */ + pollIntervalMs?: number; + /** Injectable clock for tests. */ + now: () => number; + setIntervalFn: (cb: () => void, ms: number) => ReturnType; + clearIntervalFn: (id: ReturnType) => void; + setTimeoutFn: (cb: () => void, ms: number) => ReturnType; + clearTimeoutFn: (id: ReturnType) => void; + /** Stat the file's mtime. Return null when the file is missing. */ + stat: (path: string) => Promise<{ mtimeMs: number } | null>; +} + +export class ConfigWatcher { + private readonly debounceMs: number; + private readonly pollIntervalMs: number; + private intervalId: ReturnType | undefined; + private debounceTimer: ReturnType | undefined; + /** Last observed mtime (null until baseline established). */ + private lastMtimeMs: number | null = null; + private disposed = false; + + constructor(private readonly deps: ConfigWatcherDeps) { + this.debounceMs = deps.debounceMs ?? 300; + this.pollIntervalMs = deps.pollIntervalMs ?? 2000; + } + + start(): void { + if (this.intervalId !== undefined) return; + this.intervalId = this.deps.setIntervalFn(() => { + void this.poll(); + }, this.pollIntervalMs); + } + + dispose(): void { + this.disposed = true; + if (this.intervalId !== undefined) { + this.deps.clearIntervalFn(this.intervalId); + this.intervalId = undefined; + } + if (this.debounceTimer !== undefined) { + this.deps.clearTimeoutFn(this.debounceTimer); + this.debounceTimer = undefined; + } + } + + /** Called when the native watcher (or any caller) reports a change. */ + signalChanged(): void { + this.scheduleReload(); + } + + private async poll(): Promise { + let mtime: number | null; + try { + const stat = await this.deps.stat(this.deps.path); + mtime = stat ? stat.mtimeMs : null; + } catch { + // stat failure is treated as "missing" — surface a reload once. + mtime = null; + } + + // First observation establishes the baseline without firing. + if (this.lastMtimeMs === null) { + this.lastMtimeMs = mtime; + return; + } + + if (mtime !== this.lastMtimeMs) { + this.lastMtimeMs = mtime; + this.scheduleReload(); + } + } + + private scheduleReload(): void { + if (this.disposed) return; + if (this.debounceTimer !== undefined) { + this.deps.clearTimeoutFn(this.debounceTimer); + } + this.debounceTimer = this.deps.setTimeoutFn(() => { + this.debounceTimer = undefined; + this.deps.onChange(); + }, this.debounceMs); + } +} diff --git a/src/extension.ts b/src/extension.ts index 5c628dd..60d11d0 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -47,6 +47,7 @@ import { resolveLayered, type ChampConfig, } from "./config/config-loader"; +import { ConfigWatcher } from "./config/config-watcher"; import { SkillRegistry } from "./skills/skill-registry"; import { SkillLoader } from "./skills/skill-loader"; import { VariableResolver } from "./skills/variable-resolver"; @@ -4182,30 +4183,54 @@ export async function activate( }), ); - // Watch ~/.champ/config.yaml for live reload. Created, changed, or - // deleted — any triggers a provider reload. Since #126, config comes only - // from the user-level file, so no workspace folder is watched. + // Watch ~/.champ/config.yaml for live reload. Since #126 config comes only + // from the user-level file. Issue #129: FileSystemWatcher rooted at the + // home dir is unreliable for out-of-workspace files on Linux, so the + // native watcher is the fast path and a mtime poller is the reliable + // fallback — either signal triggers the same debounced loadProvider(). { - const yamlWatchers: vscode.FileSystemWatcher[] = []; - const watchedRoots = [os.homedir()]; - let configReloadTimer: ReturnType | undefined; - const debouncedReload = () => { - if (configReloadTimer) clearTimeout(configReloadTimer); - configReloadTimer = setTimeout(() => { - configReloadTimer = undefined; - void loadProvider(); - }, 300); - }; - for (const root of watchedRoots) { - const w = vscode.workspace.createFileSystemWatcher( - new vscode.RelativePattern(root, ".champ/config.yaml"), - ); - w.onDidChange(debouncedReload); - w.onDidCreate(debouncedReload); - w.onDidDelete(debouncedReload); - yamlWatchers.push(w); - } - context.subscriptions.push(...yamlWatchers); + const configPath = path.join(os.homedir(), ".champ", "config.yaml"); + const watcher = new ConfigWatcher({ + path: configPath, + onChange: () => void loadProvider(), + now: () => Date.now(), + setIntervalFn: (cb, ms) => { + const id = setInterval(cb, ms); + context.subscriptions.push({ + dispose: () => clearInterval(id), + }); + return id; + }, + clearIntervalFn: (id) => clearInterval(id), + setTimeoutFn: (cb, ms) => { + const id = setTimeout(cb, ms); + context.subscriptions.push({ + dispose: () => clearTimeout(id), + }); + return id; + }, + clearTimeoutFn: (id) => clearTimeout(id), + stat: async (p) => { + try { + const st = await vscode.workspace.fs.stat(vscode.Uri.file(p)); + return { mtimeMs: st.mtime }; + } catch { + return null; + } + }, + }); + context.subscriptions.push({ dispose: () => watcher.dispose() }); + + // Fast path: native watcher for in-VS-Code / same-machine edits. + const native = vscode.workspace.createFileSystemWatcher( + new vscode.RelativePattern(os.homedir(), ".champ/config.yaml"), + ); + native.onDidChange(() => watcher.signalChanged()); + native.onDidCreate(() => watcher.signalChanged()); + native.onDidDelete(() => watcher.signalChanged()); + context.subscriptions.push(native); + + watcher.start(); } // ---- Team Builder and Rules Editor commands ------------------------- diff --git a/test/unit/config/config-watcher.test.ts b/test/unit/config/config-watcher.test.ts new file mode 100644 index 0000000..31e5410 --- /dev/null +++ b/test/unit/config/config-watcher.test.ts @@ -0,0 +1,151 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { ConfigWatcher } from "@/config/config-watcher"; + +interface FakeClock { + now: number; + intervals: Array<() => void>; + timeouts: Map void>; + nextId: number; + step(ms: number): void; + setInterval(cb: () => void): ReturnType; + clearInterval(id: ReturnType): void; + setTimeout(cb: () => void, ms: number): ReturnType; + clearTimeout(id: ReturnType): void; +} + +function makeClock(): FakeClock { + const clock: FakeClock = { + now: 0, + intervals: [], + timeouts: new Map(), + nextId: 1, + step(ms) { + clock.now += ms; + // run due timeouts that have elapsed (simple: fire all in order) + for (const [, cb] of [...clock.timeouts]) cb(); + clock.timeouts.clear(); + }, + setInterval(cb) { + clock.intervals.push(cb); + return clock.nextId++; + }, + clearInterval(id) { + // not needed for correctness of these tests + }, + setTimeout(cb) { + const id = clock.nextId++; + clock.timeouts.set(id, cb); + return id; + }, + clearTimeout(id) { + clock.timeouts.delete(id); + }, + }; + return clock; +} + +function build( + onChange: () => void, + opts: { mtimeMs?: () => number; pollIntervalMs?: number } = {}, +) { + const clock = makeClock(); + let curMtime = opts.mtimeMs ? opts.mtimeMs() : 1000; + const setMtime = (ms: number) => (curMtime = ms); + const w = new ConfigWatcher({ + path: "/home/user/.champ/config.yaml", + onChange, + debounceMs: 300, + pollIntervalMs: opts.pollIntervalMs ?? 1000, + now: () => clock.now, + setIntervalFn: clock.setInterval, + clearIntervalFn: clock.clearInterval, + setTimeoutFn: clock.setTimeout, + clearTimeoutFn: clock.clearTimeout, + stat: async () => (curMtime === -1 ? null : { mtimeMs: curMtime }), + }); + return { w, clock, setMtime }; +} + +describe("ConfigWatcher (#129 out-of-workspace config reload)", () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => vi.useRealTimers()); + + it("fires a baseline without onChange on first poll", async () => { + const onChange = vi.fn(); + const { w, clock } = build(onChange); + w.start(); + clock.intervals[0](); // first poll -> baseline only + await Promise.resolve(); + expect(onChange).not.toHaveBeenCalled(); + w.dispose(); + }); + + it("calls onChange (debounced) exactly once when mtime changes", async () => { + const onChange = vi.fn(); + const { w, clock, setMtime } = build(onChange); + w.start(); + clock.intervals[0](); // baseline + await Promise.resolve(); + expect(onChange).not.toHaveBeenCalled(); + + setMtime(2000); + clock.intervals[0](); // poll sees new mtime -> schedule debounce + await Promise.resolve(); + expect(onChange).not.toHaveBeenCalled(); // debounce not elapsed yet + clock.step(400); // debounce elapses + expect(onChange).toHaveBeenCalledTimes(1); + w.dispose(); + }); + + it("collapses rapid mtime changes into a single reload", async () => { + const onChange = vi.fn(); + const { w, clock, setMtime } = build(onChange); + w.start(); + clock.intervals[0](); // baseline + await Promise.resolve(); + + // several changes arrive inside the debounce window + setMtime(3000); + clock.intervals[0](); + await Promise.resolve(); + setMtime(4000); + clock.intervals[0](); + await Promise.resolve(); + clock.step(400); + expect(onChange).toHaveBeenCalledTimes(1); + w.dispose(); + }); + + it("does not reload again when mtime is unchanged on subsequent polls", async () => { + const onChange = vi.fn(); + const { w, clock, setMtime } = build(onChange); + w.start(); + clock.intervals[0](); // baseline + setMtime(2000); + clock.intervals[0](); // change detected + await Promise.resolve(); + clock.step(400); + expect(onChange).toHaveBeenCalledTimes(1); + + clock.intervals[0](); // unchanged + await Promise.resolve(); + clock.step(400); + expect(onChange).toHaveBeenCalledTimes(1); // still one + w.dispose(); + }); + + it("treats a deleted file as a signal (missing -> reload once)", async () => { + const onChange = vi.fn(); + const { w, clock, setMtime } = build(onChange); + w.start(); + clock.intervals[0](); // baseline + await Promise.resolve(); + + setMtime(-1); // file deleted + clock.intervals[0](); + await Promise.resolve(); + clock.step(400); + expect(onChange).toHaveBeenCalledTimes(1); + w.dispose(); + }); +});