From bd29afeaffdc7924a7bce21f9344bf0ff8738cf1 Mon Sep 17 00:00:00 2001 From: William Valentin Date: Thu, 12 Feb 2026 17:40:41 -0800 Subject: [PATCH] chore(skills): improve watcher event observability --- docs/plans/state.json | 11 ++++++++- src/daemon/services.test.ts | 5 +++++ src/daemon/services.ts | 45 ++++++++++++++++++++++++++++--------- 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/docs/plans/state.json b/docs/plans/state.json index 0471966..4c36721 100644 --- a/docs/plans/state.json +++ b/docs/plans/state.json @@ -1319,6 +1319,15 @@ "src/daemon/services.test.ts" ], "test_status": "pnpm typecheck + pnpm test:run src/daemon/services.test.ts + pnpm test:run + pnpm lint (warnings only, 0 errors) + pnpm build passing" + }, + "watcher_observability_polish": { + "status": "completed", + "description": "Improved watcher event logs with explicit mode/reason and per-event counters for upsert/remove/shadowed updates versus fallback reloads", + "files_modified": [ + "src/daemon/services.ts", + "src/daemon/services.test.ts" + ], + "test_status": "pnpm typecheck + pnpm test:run src/daemon/services.test.ts + pnpm test:run + pnpm lint (warnings only, 0 errors) + pnpm build passing" } } }, @@ -1373,7 +1382,7 @@ "gmail_auth_cli": "flynn gmail-auth command implemented with OAuth2 flow, doctor check, config routed to Telegram", "native_audio_support": "completed — smart routing for native audio (Gemini/OpenAI/GitHub) vs Whisper transcription fallback", "remaining_phases_completion": "Phase 1: 3/3 (100%) — context levels, command registry, memory structure. Phase 2: 2/2 (100%) — component registry, confidence routing. Phase 3: 2/2 (100%) — adaptive memory/compaction, truthfulness/autonomy hardening", - "next_up": "Skills infrastructure Phase 2: watcher observability polish (clear event reasoning and targeted/full-reload counts in logs)" + "next_up": "Skills infrastructure Phase 3: installer specs kickoff (manifest installers type support with validation and tests)" }, "soul_md_and_cron_create": { "date": "2026-02-11", diff --git a/src/daemon/services.test.ts b/src/daemon/services.test.ts index d3fd263..b9aded8 100644 --- a/src/daemon/services.test.ts +++ b/src/daemon/services.test.ts @@ -11,6 +11,7 @@ describe('initSkills watcher wiring', () => { afterEach(() => { vi.useRealTimers(); + vi.restoreAllMocks(); for (const root of roots.splice(0)) { rmSync(root, { recursive: true, force: true }); } @@ -64,6 +65,7 @@ describe('initSkills watcher wiring', () => { it('applies targeted add/update changes for a mapped skill path', () => { vi.useFakeTimers(); + const consoleLog = vi.spyOn(console, 'log').mockImplementation(() => {}); const root = mkdtempSync(join(tmpdir(), 'flynn-services-')); roots.push(root); const managedDir = join(root, 'skills'); @@ -91,6 +93,7 @@ describe('initSkills watcher wiring', () => { result.skillsWatcher?.notifyPathChanged(join(managedDir, 'beta', 'SKILL.md')); vi.advanceTimersByTime(20); expect(result.skillRegistry.get('beta')?.instructions).toContain('Updated instructions.'); + expect(consoleLog.mock.calls.some((call) => call[0]?.includes('mode=targeted'))).toBe(true); result.skillsWatcher?.stop(); }); @@ -128,6 +131,7 @@ describe('initSkills watcher wiring', () => { it('falls back to full reload for ambiguous paths', () => { vi.useFakeTimers(); + const consoleLog = vi.spyOn(console, 'log').mockImplementation(() => {}); const root = mkdtempSync(join(tmpdir(), 'flynn-services-')); roots.push(root); const managedDir = join(root, 'skills'); @@ -151,6 +155,7 @@ describe('initSkills watcher wiring', () => { vi.advanceTimersByTime(20); expect(result.skillRegistry.get('beta')).toBeDefined(); + expect(consoleLog.mock.calls.some((call) => call[0]?.includes('fallback triggered (reason=unmapped_path'))).toBe(true); result.skillsWatcher?.stop(); }); }); diff --git a/src/daemon/services.ts b/src/daemon/services.ts index d057995..8f79db4 100644 --- a/src/daemon/services.ts +++ b/src/daemon/services.ts @@ -93,25 +93,31 @@ export function initSkills(config: Config, lifecycle?: Lifecycle): SkillsResult return selected; }; - const applyTargetedSkillChange = (changedPath: string): boolean => { + type TargetedSkillChangeResult = + | { kind: 'upsert' } + | { kind: 'removed' } + | { kind: 'shadowed' } + | { kind: 'ambiguous'; reason: 'unmapped_path' | 'unmapped_removal' }; + + const applyTargetedSkillChange = (changedPath: string): TargetedSkillChangeResult => { const resolved = resolveChangedSkillDir(changedPath); if (!resolved) { - return false; + return { kind: 'ambiguous', reason: 'unmapped_path' }; } const loaded = loadSkill(resolved.dir, resolved.tier); if (loaded) { const existing = skillRegistry.get(loaded.manifest.name); if (existing && tierPriority[existing.manifest.tier] > tierPriority[loaded.manifest.tier]) { - return true; + return { kind: 'shadowed' }; } skillRegistry.register(loaded); - return true; + return { kind: 'upsert' }; } const existingAtDir = skillRegistry.list().find((skill) => resolve(skill.directory) === resolve(resolved.dir)); if (!existingAtDir) { - return false; + return { kind: 'ambiguous', reason: 'unmapped_removal' }; } skillRegistry.unregister(existingAtDir.manifest.name); @@ -119,7 +125,7 @@ export function initSkills(config: Config, lifecycle?: Lifecycle): SkillsResult if (replacement) { skillRegistry.register(replacement); } - return true; + return { kind: 'removed' }; }; const skills = loadAllSkills(skillLoadConfig); @@ -148,15 +154,32 @@ export function initSkills(config: Config, lifecycle?: Lifecycle): SkillsResult skillDirs, debounceMs: config.skills.load.watch_debounce_ms, onSkillsChanged: ({ changedPaths }) => { - let targetedCount = 0; + let upsertCount = 0; + let removedCount = 0; + let shadowedCount = 0; + for (const changedPath of changedPaths) { - if (!applyTargetedSkillChange(changedPath)) { - reloadAllSkills(`ambiguous change path: ${changedPath}`); + const result = applyTargetedSkillChange(changedPath); + if (result.kind === 'ambiguous') { + console.log( + `Skills watcher fallback triggered (reason=${result.reason}, path=${changedPath}, upsert=${upsertCount}, removed=${removedCount}, shadowed=${shadowedCount})`, + ); + reloadAllSkills(`ambiguous ${result.reason}: ${changedPath}`); return; } - targetedCount += 1; + + if (result.kind === 'upsert') { + upsertCount += 1; + } else if (result.kind === 'removed') { + removedCount += 1; + } else { + shadowedCount += 1; + } } - console.log(`Skills watcher applied targeted updates for ${targetedCount} change(s)`); + + console.log( + `Skills watcher event (mode=targeted, paths=${changedPaths.length}, upsert=${upsertCount}, removed=${removedCount}, shadowed=${shadowedCount})`, + ); }, }); skillsWatcher.start();