From 824b21db384d59c8c615188c6d4223a92d0b217b Mon Sep 17 00:00:00 2001 From: 7Sageer <7sageer@djwcb.cn> Date: Tue, 4 Aug 2026 21:53:27 +0800 Subject: [PATCH 1/4] fix(agent-core-v2): bound the project skill-root watch fd footprint The workspace skill-root source recursively watches the skill-root candidates with chokidar, which holds one fs.watch fd per file and per directory on macOS. A skill bundling a large runtime tree can exhaust the process fd budget and break every subsequent spawn (EBADF). Mirror the scanner's own pruning (node_modules / dot entries, scan depth cap) in the watch filter, and add a signal mode to hostFsWatch: rescan-style consumers get ONE native recursive fs.watch on darwin/win32, whose fd footprint is constant in the subtree size. --- .changeset/skill-watch-fd-exhaustion.md | 5 ++ .../agent-core-v2/src/_base/utils/paths.ts | 30 ++++++- .../app/skillCatalog/fileSkillDiscovery.ts | 15 +++- .../backends/node-local/hostFsWatchService.ts | 89 ++++++++++++++++++- .../src/os/interface/hostFsWatch.ts | 8 +- .../rootFileSkillSource.ts | 18 +++- .../test/_base/utils/paths.test.ts | 43 +++++++++ .../node-local/hostFsWatchService.test.ts | 65 +++++++++++++- .../skillCatalog.test.ts | 69 +++++++++++++- 9 files changed, 328 insertions(+), 14 deletions(-) create mode 100644 .changeset/skill-watch-fd-exhaustion.md create mode 100644 packages/agent-core-v2/test/_base/utils/paths.test.ts diff --git a/.changeset/skill-watch-fd-exhaustion.md b/.changeset/skill-watch-fd-exhaustion.md new file mode 100644 index 0000000000..dd684030f7 --- /dev/null +++ b/.changeset/skill-watch-fd-exhaustion.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix all tool calls failing with spawn EBADF on macOS when a skill folder contains a very large file tree (e.g. a bundled Python runtime); the skill watcher no longer opens every file it watches. diff --git a/packages/agent-core-v2/src/_base/utils/paths.ts b/packages/agent-core-v2/src/_base/utils/paths.ts index 2bb530ba69..d3d84b9238 100644 --- a/packages/agent-core-v2/src/_base/utils/paths.ts +++ b/packages/agent-core-v2/src/_base/utils/paths.ts @@ -1,14 +1,29 @@ /** * Path-filter helpers — pure string predicates, no IO. + * + * `subtreeWatchFilter` builds an `ignored` predicate that confines a recursive + * fs watch to the candidate subtrees under `root` plus their ancestor chain + * (so candidates that do not exist yet are still detected once created). The + * optional knobs prune paths BELOW a candidate only: `skipEntry` rejects + * entries by basename (e.g. a scanner's `node_modules` / dot-entry rule) and + * `maxDepth` rejects anything deeper than that many segments below the + * candidate — letting a watch mirror a scanner's own pruning instead of + * watching subtrees the consumer would never read. */ function normalizeSlashes(p: string): string { return p.replaceAll('\\', '/'); } +export interface SubtreeWatchFilterOptions { + readonly maxDepth?: number; + readonly skipEntry?: (entryName: string) => boolean; +} + export function subtreeWatchFilter( root: string, candidates: readonly string[], + options?: SubtreeWatchFilterOptions, ): (path: string) => boolean { const normRoot = normalizeSlashes(root); const normCandidates = candidates.map(normalizeSlashes); @@ -16,9 +31,22 @@ export function subtreeWatchFilter( const norm = normalizeSlashes(p); if (norm === normRoot) return false; for (const candidate of normCandidates) { - if (norm === candidate || norm.startsWith(`${candidate}/`)) return false; + if (norm === candidate) return false; + if (norm.startsWith(`${candidate}/`)) { + return isPrunedBelowCandidate(norm.slice(candidate.length + 1), options); + } if (candidate.startsWith(`${norm}/`)) return false; } return true; }; } + +function isPrunedBelowCandidate( + rel: string, + options: SubtreeWatchFilterOptions | undefined, +): boolean { + if (options === undefined) return false; + const segments = rel.split('/'); + if (options.skipEntry !== undefined && segments.some(options.skipEntry)) return true; + return options.maxDepth !== undefined && segments.length > options.maxDepth; +} diff --git a/packages/agent-core-v2/src/app/skillCatalog/fileSkillDiscovery.ts b/packages/agent-core-v2/src/app/skillCatalog/fileSkillDiscovery.ts index 366084cd74..ceb2f9ad66 100644 --- a/packages/agent-core-v2/src/app/skillCatalog/fileSkillDiscovery.ts +++ b/packages/agent-core-v2/src/app/skillCatalog/fileSkillDiscovery.ts @@ -2,8 +2,11 @@ * `skillCatalog` domain — filesystem `ISkillDiscovery` backend. * * Discovers skill bundles by walking caller-supplied roots and parsing each - * SKILL.md. Exposes both the App-scoped `ISkillDiscovery` service and a - * stateless standalone function. + * SKILL.md, pruning `node_modules` / dot entries and capping the walk depth + * (`isSkillScanExcludedEntry`, `MAX_SKILL_SCAN_DEPTH` — exported so fs + * watches can mirror the scan's own pruning instead of watching subtrees it + * would never read). Exposes both the App-scoped `ISkillDiscovery` service + * and a stateless standalone function. */ import { promises as fs } from 'node:fs'; @@ -16,7 +19,11 @@ import type { SkillDiscoveryResult, ISkillDiscovery } from './skillDiscovery'; import type { SkillDefinition, SkillRoot, SkippedSkill } from './types'; import { normalizeSkillName } from './types'; -const MAX_SKILL_SCAN_DEPTH = 8; +export const MAX_SKILL_SCAN_DEPTH = 8; + +export function isSkillScanExcludedEntry(entryName: string): boolean { + return entryName === 'node_modules' || entryName.startsWith('.'); +} export class FileSkillDiscovery implements ISkillDiscovery { declare readonly _serviceBrand: undefined; @@ -60,7 +67,7 @@ export async function discoverFileSkills( if (await isFile(path.join(entryPath, 'SKILL.md'))) { directorySkills.add(entry); } - if (entry === 'node_modules' || entry.startsWith('.')) continue; + if (isSkillScanExcludedEntry(entry)) continue; if (await isDir(entryPath)) subdirs.push(entry); } diff --git a/packages/agent-core-v2/src/os/backends/node-local/hostFsWatchService.ts b/packages/agent-core-v2/src/os/backends/node-local/hostFsWatchService.ts index 8e8794643a..e1840623b0 100644 --- a/packages/agent-core-v2/src/os/backends/node-local/hostFsWatchService.ts +++ b/packages/agent-core-v2/src/os/backends/node-local/hostFsWatchService.ts @@ -2,10 +2,18 @@ * `hostFsWatch` domain — `IHostFsWatchService` implementation. * * Wraps `chokidar` to report raw create/modify/delete events under an absolute - * path. Each `watch()` call owns an independent `FSWatcher`; disposing the - * handle closes it. Bound at App scope. + * path. Each `watch()` call owns an independent watcher; disposing the handle + * closes it. A `signal`-mode recursive watch on darwin/win32 instead uses ONE + * native recursive `fs.watch` (FSEvents / ReadDirectoryChangesW), whose fd + * footprint stays constant in the subtree size — chokidar holds one + * `fs.watch` fd per file and per directory on macOS, so per-node watching of + * a fat subtree can exhaust the process fd budget and break every subsequent + * spawn. Bound at App scope. */ +import { watch as fsWatch, lstatSync, type FSWatcher as NodeFSWatcher } from 'node:fs'; +import { join } from 'node:path'; + import { FSWatcher } from 'chokidar'; import { Emitter, type Event } from '#/_base/event'; @@ -58,14 +66,91 @@ class HostFsWatchHandle implements IHostFsWatchHandle { } } +class NativeRecursiveWatchHandle implements IHostFsWatchHandle { + readonly onDidChange: Event; + + private readonly emitter: Emitter; + private readonly watcher: NodeFSWatcher; + private readonly ignored: (path: string) => boolean; + private readonly knownKinds = new Map(); + private disposed = false; + + constructor(path: string, options: HostFsWatchOptions | undefined) { + this.emitter = new Emitter(); + this.onDidChange = this.emitter.event; + this.ignored = options?.ignored ?? DEFAULT_IGNORED; + this.watcher = fsWatch(path, { persistent: false, recursive: true }, (eventType, filename) => { + this.onNativeEvent(path, eventType, filename); + }); + this.watcher.on('error', (error: unknown) => { + onUnexpectedError(error); + }); + } + + private onNativeEvent(root: string, eventType: string, filename: string | null): void { + if (this.disposed) return; + const absPath = filename === null || filename === '' ? root : join(root, filename); + if (absPath !== root && this.ignored(absPath)) return; + const mapped = this.mapNativeEvent(absPath, eventType); + if (mapped !== undefined) this.emitter.fire(mapped); + } + + private mapNativeEvent(absPath: string, eventType: string): HostFsChange | undefined { + if (eventType === 'change') { + return { path: absPath, action: 'modified', kind: this.knownKinds.get(absPath) ?? 'file' }; + } + if (eventType !== 'rename') return undefined; + try { + const kind: HostFsChangeKind = lstatSync(absPath).isDirectory() ? 'directory' : 'file'; + const action: HostFsChangeAction = this.knownKinds.has(absPath) ? 'modified' : 'created'; + this.knownKinds.set(absPath, kind); + return { path: absPath, action, kind }; + } catch { + const kind: HostFsChangeKind = this.knownKinds.get(absPath) ?? 'file'; + this.knownKinds.delete(absPath); + return { path: absPath, action: 'deleted', kind }; + } + } + + dispose(): void { + if (this.disposed) return; + this.disposed = true; + this.watcher.close(); + this.emitter.dispose(); + } +} + export class HostFsWatchService implements IHostFsWatchService { declare readonly _serviceBrand: undefined; watch(path: string, options?: HostFsWatchOptions): IHostFsWatchHandle { + if (useNativeRecursive(options)) { + const native = tryNativeRecursiveWatch(path, options); + if (native !== undefined) return native; + } return new HostFsWatchHandle(path, options); } } +function useNativeRecursive(options: HostFsWatchOptions | undefined): boolean { + return ( + options?.signal === true && + options.recursive !== false && + (process.platform === 'darwin' || process.platform === 'win32') + ); +} + +function tryNativeRecursiveWatch( + path: string, + options: HostFsWatchOptions | undefined, +): IHostFsWatchHandle | undefined { + try { + return new NativeRecursiveWatchHandle(path, options); + } catch { + return undefined; + } +} + function mapChokidarEvent(eventName: string, absPath: string): HostFsChange | undefined { const mapped = mapActionAndKind(eventName); if (mapped === undefined) return undefined; diff --git a/packages/agent-core-v2/src/os/interface/hostFsWatch.ts b/packages/agent-core-v2/src/os/interface/hostFsWatch.ts index c1668e85f2..44601d8bdf 100644 --- a/packages/agent-core-v2/src/os/interface/hostFsWatch.ts +++ b/packages/agent-core-v2/src/os/interface/hostFsWatch.ts @@ -4,7 +4,12 @@ * Defines the `IHostFsWatchService`, a thin primitive over the host OS file * watcher. It reports raw create/modify/delete events under an absolute path * and knows nothing about sessions, connections, workspaces or wire frames. - * App-scoped — one shared instance. + * `HostFsWatchOptions.signal` marks callers that consume events as a mere + * "something changed" signal (ignoring action/kind); the backend may then + * pick a cheaper implementation (one native recursive watch instead of + * per-node watchers), and action/kind may be coarse — a deleted entry whose + * kind was never observed is reported as 'file'. App-scoped — one shared + * instance. */ import { createDecorator, type ServiceIdentifier } from '#/_base/di/instantiation'; @@ -23,6 +28,7 @@ export interface HostFsChange { export interface HostFsWatchOptions { readonly recursive?: boolean; readonly ignored?: (path: string) => boolean; + readonly signal?: boolean; } export interface IHostFsWatchHandle extends IDisposable { diff --git a/packages/agent-core-v2/src/workspace/workspaceSkillCatalog/rootFileSkillSource.ts b/packages/agent-core-v2/src/workspace/workspaceSkillCatalog/rootFileSkillSource.ts index 05d51b47ec..8d66eb2732 100644 --- a/packages/agent-core-v2/src/workspace/workspaceSkillCatalog/rootFileSkillSource.ts +++ b/packages/agent-core-v2/src/workspace/workspaceSkillCatalog/rootFileSkillSource.ts @@ -8,8 +8,12 @@ * skill-root candidates (`.kimi-code/skills`, `.agents/skills` under the * project root, watched whether or not they exist yet) through * `hostFsWatch` and re-fires `onDidChange` debounced, so the catalog - * re-scans THIS source only when project skill files change. Bound at - * Workspace scope so every session of the handler shares one scan. + * re-scans THIS source only when project skill files change. The watch + * mirrors the scanner's own pruning — `node_modules` / dot entries, and the + * scan depth cap plus two segments (the skill directory and its SKILL.md) + * — and runs in `signal` mode, so a fat skill subtree (e.g. a skill + * bundling a runtime environment) does not cost one fs-watch fd per file. + * Bound at Workspace scope so every session of the handler shares one scan. */ import { createDecorator, type ServiceIdentifier } from '#/_base/di/instantiation'; @@ -24,6 +28,10 @@ import { MERGE_ALL_AVAILABLE_SKILLS_SECTION, type MergeAllAvailableSkillsConfig, } from '#/app/skillCatalog/configSection'; +import { + MAX_SKILL_SCAN_DEPTH, + isSkillScanExcludedEntry, +} from '#/app/skillCatalog/fileSkillDiscovery'; import { ISkillDiscovery } from '#/app/skillCatalog/skillDiscovery'; import { projectRoots, projectSkillRootCandidates } from '#/app/skillCatalog/skillRoots'; import { @@ -87,7 +95,11 @@ export class WorkspaceRootSkillSource extends Disposable implements IWorkspaceRo private async watchProjectSkillRoots(): Promise { const { projectRoot, candidates } = await projectSkillRootCandidates(this.workspace.cwd); const handle = this.fsWatch.watch(projectRoot, { - ignored: subtreeWatchFilter(projectRoot, candidates), + ignored: subtreeWatchFilter(projectRoot, candidates, { + maxDepth: MAX_SKILL_SCAN_DEPTH + 2, + skipEntry: isSkillScanExcludedEntry, + }), + signal: true, }); this._register(handle); this._register( diff --git a/packages/agent-core-v2/test/_base/utils/paths.test.ts b/packages/agent-core-v2/test/_base/utils/paths.test.ts new file mode 100644 index 0000000000..2e4d0f4372 --- /dev/null +++ b/packages/agent-core-v2/test/_base/utils/paths.test.ts @@ -0,0 +1,43 @@ +/** + * `_base/utils/paths` — unit tests for `subtreeWatchFilter`. + */ + +import { describe, expect, it } from 'vitest'; + +import { subtreeWatchFilter } from '#/_base/utils/paths'; + +describe('subtreeWatchFilter', () => { + const root = '/repo'; + const candidates = ['/repo/.kimi-code/skills', '/repo/.agents/skills']; + + it('keeps the root, candidate ancestors and candidate subtrees watched', () => { + const ignored = subtreeWatchFilter(root, candidates); + expect(ignored('/repo')).toBe(false); + expect(ignored('/repo/.agents')).toBe(false); + expect(ignored('/repo/.agents/skills')).toBe(false); + expect(ignored('/repo/.agents/skills/demo/SKILL.md')).toBe(false); + expect(ignored('/repo/src')).toBe(true); + expect(ignored('/repo/src/index.ts')).toBe(true); + }); + + it('prunes skipped entries and over-depth paths below candidates', () => { + const ignored = subtreeWatchFilter(root, candidates, { + maxDepth: 3, + skipEntry: (name) => name === 'node_modules' || name.startsWith('.'), + }); + expect(ignored('/repo/.agents/skills/demo/node_modules')).toBe(true); + expect(ignored('/repo/.agents/skills/demo/node_modules/pkg/x.js')).toBe(true); + expect(ignored('/repo/.agents/skills/demo/.venv/bin/python')).toBe(true); + expect(ignored('/repo/.agents/skills/demo/scripts/run.sh')).toBe(false); + expect(ignored('/repo/.agents/skills/a/b/c/d/e/f/SKILL.md')).toBe(true); + }); + + it('never prunes the candidate ancestor chain itself', () => { + const ignored = subtreeWatchFilter(root, candidates, { + skipEntry: (name) => name.startsWith('.'), + }); + expect(ignored('/repo/.agents')).toBe(false); + expect(ignored('/repo/.agents/skills')).toBe(false); + expect(ignored('/repo/.agents/skills/demo')).toBe(false); + }); +}); diff --git a/packages/agent-core-v2/test/os/backends/node-local/hostFsWatchService.test.ts b/packages/agent-core-v2/test/os/backends/node-local/hostFsWatchService.test.ts index b815c9bcf3..7c079314cc 100644 --- a/packages/agent-core-v2/test/os/backends/node-local/hostFsWatchService.test.ts +++ b/packages/agent-core-v2/test/os/backends/node-local/hostFsWatchService.test.ts @@ -1,8 +1,10 @@ /** - * `hostFsWatch` domain — integration test against the real `chokidar` - * watcher on a temporary directory. + * `hostFsWatch` domain — integration tests against the real watcher + * backends (chokidar, plus the native recursive watch used by `signal` + * mode on darwin/win32) on a temporary directory. */ +import { readdirSync } from 'node:fs'; import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -33,6 +35,15 @@ describe('HostFsWatchService', () => { return events; } + async function startSignal(ignored?: (path: string) => boolean): Promise { + const events: HostFsChange[] = []; + const svc = new HostFsWatchService(); + handle = svc.watch(root, { recursive: true, signal: true, ignored }); + handle.onDidChange((e) => events.push(e)); + await wait(200); + return events; + } + it('reports create / modify / delete for a file', async () => { root = await mkdtemp(join(tmpdir(), 'hostfswatch-')); const events = await start(); @@ -86,4 +97,54 @@ describe('HostFsWatchService', () => { expect(events).toHaveLength(0); }); + + it('signal mode reports create / modify / delete for a file', async () => { + root = await mkdtemp(join(tmpdir(), 'hostfswatch-signal-')); + const events = await startSignal(); + + const file = join(root, 'a.txt'); + await writeFile(file, 'v1'); + await wait(500); + await writeFile(file, 'v2'); + await wait(500); + await rm(file); + await wait(500); + + const actions = events.filter((e) => e.path === file).map((e) => e.action); + expect(actions).toContain('created'); + expect(actions).toContain('modified'); + expect(actions).toContain('deleted'); + }); + + it('signal mode honors the ignored predicate', async () => { + root = await mkdtemp(join(tmpdir(), 'hostfswatch-signal-')); + const events = await startSignal((p) => p.includes('node_modules')); + + await mkdir(join(root, 'node_modules', 'pkg'), { recursive: true }); + await writeFile(join(root, 'node_modules', 'pkg', 'index.js'), 'x'); + await writeFile(join(root, 'visible.txt'), 'x'); + await wait(500); + + expect(events.some((e) => e.path.includes('node_modules'))).toBe(false); + expect(events.some((e) => e.path === join(root, 'visible.txt'))).toBe(true); + }); + + it.skipIf(process.platform !== 'darwin')( + 'signal mode keeps the fd footprint bounded on a fat subtree', + async () => { + root = await mkdtemp(join(tmpdir(), 'hostfswatch-fat-')); + const fat = join(root, 'fat'); + await mkdir(fat, { recursive: true }); + for (let i = 0; i < 1200; i++) { + await writeFile(join(fat, `f${i}.txt`), 'x'); + } + + const fdsBefore = readdirSync('/dev/fd').length; + await startSignal(); + const fdsAfter = readdirSync('/dev/fd').length; + + expect(fdsAfter - fdsBefore).toBeLessThan(50); + }, + 30000, + ); }); diff --git a/packages/agent-core-v2/test/workspace/workspaceSkillCatalog/skillCatalog.test.ts b/packages/agent-core-v2/test/workspace/workspaceSkillCatalog/skillCatalog.test.ts index 8a372a8bd1..1fb78ebfa3 100644 --- a/packages/agent-core-v2/test/workspace/workspaceSkillCatalog/skillCatalog.test.ts +++ b/packages/agent-core-v2/test/workspace/workspaceSkillCatalog/skillCatalog.test.ts @@ -3,7 +3,10 @@ * * Exercises the real Workspace-scoped catalog and source services with * filesystem or in-memory discovery boundaries, including controlled - * concurrent refreshes and the fs-watch-driven single-source rescan. + * concurrent refreshes and the fs-watch-driven single-source rescan. On + * darwin the skill-source watch runs in native `signal` mode, which may + * surface coalesced ancestor-directory events, so the pruned-write case + * asserts unchanged catalog content there and zero rescans elsewhere. * Run: `pnpm --filter @moonshot-ai/agent-core-v2 exec vitest run * test/workspace/workspaceSkillCatalog/skillCatalog.test.ts`. */ @@ -835,4 +838,68 @@ describe('WorkspaceSkillCatalogService', () => { await rm(workDir, { recursive: true, force: true }); } }, 15000); + + it('ignores changes under scanner-pruned entries but still rescans on real skill changes', async () => { + const workDir = await mkdtemp(join(tmpdir(), 'skill-watch-pruned-')); + await mkdir(join(workDir, '.agents', 'skills', 'demo'), { recursive: true }); + await writeFile( + join(workDir, '.agents', 'skills', 'demo', 'SKILL.md'), + '---\nname: demo\ndescription: v1\n---\nbody', + 'utf8', + ); + const host = createScopedTestHost([ + stubPair(IBootstrapService, bootstrapStub), + stubPair(IConfigService, configStub()), + stubPair(IPluginService, pluginStub()), + stubPair(ILogService, stubLog()), + stubPair(ISkillDiscovery, new FileSkillDiscovery(stubLog())), + stubPair(IHostFsWatchService, new HostFsWatchService()), + ]); + const workspace = host.child(LifecycleScope.Workspace, 'w1', [ + stubPair(IWorkspaceContext, workspaceContextStub(workDir)), + ]); + + try { + const catalog = workspace.accessor.get(IWorkspaceSkillCatalog); + await catalog.load(); + expect(catalog.catalog.getSkill('demo')?.description).toBe('v1'); + + const sourceIds: string[] = []; + const subscription = catalog.onDidChange((sourceId) => sourceIds.push(sourceId)); + + await mkdir(join(workDir, '.agents', 'skills', 'demo', 'node_modules', 'pkg'), { + recursive: true, + }); + await writeFile( + join(workDir, '.agents', 'skills', 'demo', 'node_modules', 'pkg', 'index.js'), + 'x', + 'utf8', + ); + await mkdir(join(workDir, '.agents', 'skills', 'demo', '.hidden'), { recursive: true }); + await writeFile(join(workDir, '.agents', 'skills', 'demo', '.hidden', 'notes.md'), 'x', 'utf8'); + await new Promise((resolve) => setTimeout(resolve, 1500)); + if (process.platform === 'darwin') { + expect(catalog.catalog.getSkill('demo')?.description).toBe('v1'); + expect(catalog.catalog.getSkill('pkg')).toBeUndefined(); + } else { + expect(sourceIds).toEqual([]); + } + + const refreshed = waitForEvents(catalog.onDidChange, 1); + const timedOut = new Promise((_resolve, reject) => { + setTimeout(() => reject(new Error('watch-driven refresh timed out')), 10000); + }); + await writeFile( + join(workDir, '.agents', 'skills', 'demo', 'SKILL.md'), + '---\nname: demo\ndescription: v2\n---\nbody', + 'utf8', + ); + await Promise.race([refreshed, timedOut]); + expect(catalog.catalog.getSkill('demo')?.description).toBe('v2'); + subscription.dispose(); + } finally { + host.dispose(); + await rm(workDir, { recursive: true, force: true }); + } + }, 15000); }); From b7af1d12889b7e9180ae5a5173a0f0455ded15ba Mon Sep 17 00:00:00 2001 From: 7Sageer <7sageer@djwcb.cn> Date: Wed, 5 Aug 2026 00:12:08 +0800 Subject: [PATCH 2/4] fix(agent-core-v2): align the skill watch with scanner semantics and harden signal mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up: - The scanner probes every entry's direct SKILL.md before gating recursion, so the watch filter now keeps an excluded entry itself and its direct SKILL.md (keepEntryFile) instead of pruning them — skills under node_modules / dot directories keep their hot reload. - The signal-mode native leg now owns its recovery: a native watch error fires one root invalidation and re-arms with capped exponential backoff; chokidar is used only where recursive fs.watch is unavailable, so a transient failure can neither silently end hot reload nor downgrade to the per-node watcher. - Native event path resolution handles absolute filenames and the root-basename case, clamping out-of-root events to a root invalidation instead of dropping them. - The event mapping is extracted into NativeSignalMapper with the stat call injected, so the native-branch decisions are unit-tested on any platform. --- .../agent-core-v2/src/_base/utils/paths.ts | 26 ++- .../backends/node-local/hostFsWatchService.ts | 168 +++++++++++++----- .../rootFileSkillSource.ts | 6 +- .../test/_base/utils/paths.test.ts | 19 +- .../node-local/hostFsWatchService.test.ts | 96 +++++++++- .../skillCatalog.test.ts | 38 ++++ 6 files changed, 294 insertions(+), 59 deletions(-) diff --git a/packages/agent-core-v2/src/_base/utils/paths.ts b/packages/agent-core-v2/src/_base/utils/paths.ts index d3d84b9238..8b2b321519 100644 --- a/packages/agent-core-v2/src/_base/utils/paths.ts +++ b/packages/agent-core-v2/src/_base/utils/paths.ts @@ -4,11 +4,14 @@ * `subtreeWatchFilter` builds an `ignored` predicate that confines a recursive * fs watch to the candidate subtrees under `root` plus their ancestor chain * (so candidates that do not exist yet are still detected once created). The - * optional knobs prune paths BELOW a candidate only: `skipEntry` rejects - * entries by basename (e.g. a scanner's `node_modules` / dot-entry rule) and - * `maxDepth` rejects anything deeper than that many segments below the - * candidate — letting a watch mirror a scanner's own pruning instead of - * watching subtrees the consumer would never read. + * optional knobs prune paths BELOW a candidate only, mirroring a + * recursion-gated scanner: `maxDepth` rejects anything deeper than that many + * segments below the candidate, and an entry matching `skipEntry` (e.g. the + * skill scanner's `node_modules` / dot-entry rule) stops further watching — + * but the entry itself, and with `keepEntryFile` also its direct child file + * of that name, stay watched, because a scanner still probes those without + * recursing deeper. Everything below that point is what the consumer never + * reads, and only that is pruned. */ function normalizeSlashes(p: string): string { @@ -18,6 +21,7 @@ function normalizeSlashes(p: string): string { export interface SubtreeWatchFilterOptions { readonly maxDepth?: number; readonly skipEntry?: (entryName: string) => boolean; + readonly keepEntryFile?: string; } export function subtreeWatchFilter( @@ -47,6 +51,16 @@ function isPrunedBelowCandidate( ): boolean { if (options === undefined) return false; const segments = rel.split('/'); - if (options.skipEntry !== undefined && segments.some(options.skipEntry)) return true; + if (options.skipEntry !== undefined) { + const excludedAt = segments.findIndex(options.skipEntry); + if (excludedAt !== -1) { + if (segments.length <= excludedAt + 1) return false; + return !( + options.keepEntryFile !== undefined && + segments.length === excludedAt + 2 && + segments.at(-1) === options.keepEntryFile + ); + } + } return options.maxDepth !== undefined && segments.length > options.maxDepth; } diff --git a/packages/agent-core-v2/src/os/backends/node-local/hostFsWatchService.ts b/packages/agent-core-v2/src/os/backends/node-local/hostFsWatchService.ts index e1840623b0..bd169da69d 100644 --- a/packages/agent-core-v2/src/os/backends/node-local/hostFsWatchService.ts +++ b/packages/agent-core-v2/src/os/backends/node-local/hostFsWatchService.ts @@ -8,11 +8,18 @@ * footprint stays constant in the subtree size — chokidar holds one * `fs.watch` fd per file and per directory on macOS, so per-node watching of * a fat subtree can exhaust the process fd budget and break every subsequent - * spawn. Bound at App scope. + * spawn. The signal leg owns its recovery: a native error (including sync + * creation failure) fires one root-level invalidation and re-arms the native + * watch with capped exponential backoff, so a transient failure neither + * downgrades the consumer to the per-node watcher nor silently ends hot + * reload; chokidar serves only when the platform has no recursive + * `fs.watch`. Event mapping lives in `NativeSignalMapper` as pure logic + * with the stat call injected, so it is testable off darwin/win32. + * Bound at App scope. */ import { watch as fsWatch, lstatSync, type FSWatcher as NodeFSWatcher } from 'node:fs'; -import { join } from 'node:path'; +import { basename, isAbsolute, join, relative } from 'node:path'; import { FSWatcher } from 'chokidar'; @@ -31,6 +38,57 @@ import { const DEFAULT_IGNORED = (p: string): boolean => /(?:^|[/\\])\.git(?:$|[/\\])/.test(p); +const NATIVE_RETRY_BASE_MS = 1000; +const NATIVE_RETRY_MAX_MS = 30000; + +export interface NativeSignalStat { + kindOf(absPath: string): HostFsChangeKind | undefined; +} + +export class NativeSignalMapper { + private readonly knownKinds = new Map(); + + constructor(private readonly stat: NativeSignalStat) {} + + map( + root: string, + eventType: string, + filename: string | null, + ignored: (path: string) => boolean, + ): HostFsChange | undefined { + const absPath = this.resolveEventPath(root, filename); + if (absPath !== root && ignored(absPath)) return undefined; + if (eventType === 'change') { + return { path: absPath, action: 'modified', kind: this.knownKinds.get(absPath) ?? 'file' }; + } + if (eventType !== 'rename') return undefined; + const statKind = this.stat.kindOf(absPath); + if (statKind !== undefined) { + const action: HostFsChangeAction = this.knownKinds.has(absPath) ? 'modified' : 'created'; + this.knownKinds.set(absPath, statKind); + return { path: absPath, action, kind: statKind }; + } + const kind = this.knownKinds.get(absPath) ?? 'file'; + this.knownKinds.delete(absPath); + return { path: absPath, action: 'deleted', kind }; + } + + private resolveEventPath(root: string, filename: string | null): string { + if (filename === null || filename === '') return root; + if (isAbsolute(filename)) return clampToRoot(root, filename); + if (filename === basename(root) && this.stat.kindOf(join(root, filename)) === undefined) { + return root; + } + return join(root, filename); + } +} + +function clampToRoot(root: string, absPath: string): string { + const rel = relative(root, absPath); + if (rel === '' || (!rel.startsWith('..') && !isAbsolute(rel))) return absPath; + return root; +} + class HostFsWatchHandle implements IHostFsWatchHandle { readonly onDidChange: Event; @@ -66,56 +124,88 @@ class HostFsWatchHandle implements IHostFsWatchHandle { } } -class NativeRecursiveWatchHandle implements IHostFsWatchHandle { +class SignalWatchHandle implements IHostFsWatchHandle { readonly onDidChange: Event; private readonly emitter: Emitter; - private readonly watcher: NodeFSWatcher; private readonly ignored: (path: string) => boolean; - private readonly knownKinds = new Map(); + private readonly mapper = new NativeSignalMapper({ + kindOf: (absPath) => { + try { + return lstatSync(absPath).isDirectory() ? 'directory' : 'file'; + } catch { + return undefined; + } + }, + }); + private nativeWatcher: NodeFSWatcher | undefined; + private chokidarLeg: HostFsWatchHandle | undefined; + private retryTimer: ReturnType | undefined; + private retryAttempts = 0; private disposed = false; - constructor(path: string, options: HostFsWatchOptions | undefined) { + constructor(private readonly root: string, options: HostFsWatchOptions | undefined) { this.emitter = new Emitter(); this.onDidChange = this.emitter.event; this.ignored = options?.ignored ?? DEFAULT_IGNORED; - this.watcher = fsWatch(path, { persistent: false, recursive: true }, (eventType, filename) => { - this.onNativeEvent(path, eventType, filename); - }); - this.watcher.on('error', (error: unknown) => { - onUnexpectedError(error); - }); + this.startNativeLeg(); } - private onNativeEvent(root: string, eventType: string, filename: string | null): void { + private startNativeLeg(): void { if (this.disposed) return; - const absPath = filename === null || filename === '' ? root : join(root, filename); - if (absPath !== root && this.ignored(absPath)) return; - const mapped = this.mapNativeEvent(absPath, eventType); - if (mapped !== undefined) this.emitter.fire(mapped); + try { + const watcher = fsWatch( + this.root, + { persistent: false, recursive: true }, + (eventType, filename) => { + if (this.disposed) return; + const mapped = this.mapper.map(this.root, eventType, filename, this.ignored); + if (mapped !== undefined) this.emitter.fire(mapped); + }, + ); + watcher.on('error', (error: NodeJS.ErrnoException) => { + this.onNativeError(error); + }); + this.nativeWatcher = watcher; + this.retryAttempts = 0; + } catch (error) { + this.onNativeError(error as NodeJS.ErrnoException); + } } - private mapNativeEvent(absPath: string, eventType: string): HostFsChange | undefined { - if (eventType === 'change') { - return { path: absPath, action: 'modified', kind: this.knownKinds.get(absPath) ?? 'file' }; - } - if (eventType !== 'rename') return undefined; - try { - const kind: HostFsChangeKind = lstatSync(absPath).isDirectory() ? 'directory' : 'file'; - const action: HostFsChangeAction = this.knownKinds.has(absPath) ? 'modified' : 'created'; - this.knownKinds.set(absPath, kind); - return { path: absPath, action, kind }; - } catch { - const kind: HostFsChangeKind = this.knownKinds.get(absPath) ?? 'file'; - this.knownKinds.delete(absPath); - return { path: absPath, action: 'deleted', kind }; + private onNativeError(error: NodeJS.ErrnoException): void { + if (this.disposed) return; + this.nativeWatcher?.close(); + this.nativeWatcher = undefined; + if (error.code === 'ERR_FEATURE_UNAVAILABLE_ON_PLATFORM') { + this.startChokidarLeg(); + return; } + onUnexpectedError(error); + this.emitter.fire({ path: this.root, action: 'modified', kind: 'directory' }); + const delay = Math.min(NATIVE_RETRY_BASE_MS * 2 ** this.retryAttempts, NATIVE_RETRY_MAX_MS); + this.retryAttempts += 1; + this.retryTimer = setTimeout(() => { + this.startNativeLeg(); + }, delay); + this.retryTimer.unref?.(); + } + + private startChokidarLeg(): void { + if (this.chokidarLeg !== undefined) return; + const leg = new HostFsWatchHandle(this.root, { recursive: true, ignored: this.ignored }); + leg.onDidChange((event) => { + if (!this.disposed) this.emitter.fire(event); + }); + this.chokidarLeg = leg; } dispose(): void { if (this.disposed) return; this.disposed = true; - this.watcher.close(); + if (this.retryTimer !== undefined) clearTimeout(this.retryTimer); + this.nativeWatcher?.close(); + this.chokidarLeg?.dispose(); this.emitter.dispose(); } } @@ -125,8 +215,7 @@ export class HostFsWatchService implements IHostFsWatchService { watch(path: string, options?: HostFsWatchOptions): IHostFsWatchHandle { if (useNativeRecursive(options)) { - const native = tryNativeRecursiveWatch(path, options); - if (native !== undefined) return native; + return new SignalWatchHandle(path, options); } return new HostFsWatchHandle(path, options); } @@ -140,17 +229,6 @@ function useNativeRecursive(options: HostFsWatchOptions | undefined): boolean { ); } -function tryNativeRecursiveWatch( - path: string, - options: HostFsWatchOptions | undefined, -): IHostFsWatchHandle | undefined { - try { - return new NativeRecursiveWatchHandle(path, options); - } catch { - return undefined; - } -} - function mapChokidarEvent(eventName: string, absPath: string): HostFsChange | undefined { const mapped = mapActionAndKind(eventName); if (mapped === undefined) return undefined; diff --git a/packages/agent-core-v2/src/workspace/workspaceSkillCatalog/rootFileSkillSource.ts b/packages/agent-core-v2/src/workspace/workspaceSkillCatalog/rootFileSkillSource.ts index 8d66eb2732..54c4954985 100644 --- a/packages/agent-core-v2/src/workspace/workspaceSkillCatalog/rootFileSkillSource.ts +++ b/packages/agent-core-v2/src/workspace/workspaceSkillCatalog/rootFileSkillSource.ts @@ -9,8 +9,9 @@ * project root, watched whether or not they exist yet) through * `hostFsWatch` and re-fires `onDidChange` debounced, so the catalog * re-scans THIS source only when project skill files change. The watch - * mirrors the scanner's own pruning — `node_modules` / dot entries, and the - * scan depth cap plus two segments (the skill directory and its SKILL.md) + * mirrors the scanner's own pruning — `node_modules` / dot entries gate + * recursion but their direct SKILL.md stays watched, and the depth cap is + * the scanner's plus two segments (the skill directory and its SKILL.md) * — and runs in `signal` mode, so a fat skill subtree (e.g. a skill * bundling a runtime environment) does not cost one fs-watch fd per file. * Bound at Workspace scope so every session of the handler shares one scan. @@ -98,6 +99,7 @@ export class WorkspaceRootSkillSource extends Disposable implements IWorkspaceRo ignored: subtreeWatchFilter(projectRoot, candidates, { maxDepth: MAX_SKILL_SCAN_DEPTH + 2, skipEntry: isSkillScanExcludedEntry, + keepEntryFile: 'SKILL.md', }), signal: true, }); diff --git a/packages/agent-core-v2/test/_base/utils/paths.test.ts b/packages/agent-core-v2/test/_base/utils/paths.test.ts index 2e4d0f4372..e90a6f4c85 100644 --- a/packages/agent-core-v2/test/_base/utils/paths.test.ts +++ b/packages/agent-core-v2/test/_base/utils/paths.test.ts @@ -20,18 +20,33 @@ describe('subtreeWatchFilter', () => { expect(ignored('/repo/src/index.ts')).toBe(true); }); - it('prunes skipped entries and over-depth paths below candidates', () => { + it('prunes what an excluded entry hides from the scanner, keeps what it still probes', () => { const ignored = subtreeWatchFilter(root, candidates, { maxDepth: 3, skipEntry: (name) => name === 'node_modules' || name.startsWith('.'), + keepEntryFile: 'SKILL.md', }); - expect(ignored('/repo/.agents/skills/demo/node_modules')).toBe(true); + expect(ignored('/repo/.agents/skills/demo/node_modules')).toBe(false); + expect(ignored('/repo/.agents/skills/demo/node_modules/SKILL.md')).toBe(false); + expect(ignored('/repo/.agents/skills/demo/.hidden')).toBe(false); + expect(ignored('/repo/.agents/skills/demo/.hidden/SKILL.md')).toBe(false); + expect(ignored('/repo/.agents/skills/.flat.md')).toBe(false); + expect(ignored('/repo/.agents/skills/demo/node_modules/pkg')).toBe(true); expect(ignored('/repo/.agents/skills/demo/node_modules/pkg/x.js')).toBe(true); expect(ignored('/repo/.agents/skills/demo/.venv/bin/python')).toBe(true); expect(ignored('/repo/.agents/skills/demo/scripts/run.sh')).toBe(false); expect(ignored('/repo/.agents/skills/a/b/c/d/e/f/SKILL.md')).toBe(true); }); + it('prunes an excluded entry beyond itself when no keepEntryFile is set', () => { + const ignored = subtreeWatchFilter(root, candidates, { + skipEntry: (name) => name === 'node_modules', + }); + expect(ignored('/repo/.agents/skills/demo/node_modules')).toBe(false); + expect(ignored('/repo/.agents/skills/demo/node_modules/SKILL.md')).toBe(true); + expect(ignored('/repo/.agents/skills/demo/node_modules/pkg')).toBe(true); + }); + it('never prunes the candidate ancestor chain itself', () => { const ignored = subtreeWatchFilter(root, candidates, { skipEntry: (name) => name.startsWith('.'), diff --git a/packages/agent-core-v2/test/os/backends/node-local/hostFsWatchService.test.ts b/packages/agent-core-v2/test/os/backends/node-local/hostFsWatchService.test.ts index 7c079314cc..f880927c86 100644 --- a/packages/agent-core-v2/test/os/backends/node-local/hostFsWatchService.test.ts +++ b/packages/agent-core-v2/test/os/backends/node-local/hostFsWatchService.test.ts @@ -1,21 +1,109 @@ /** * `hostFsWatch` domain — integration tests against the real watcher * backends (chokidar, plus the native recursive watch used by `signal` - * mode on darwin/win32) on a temporary directory. + * mode on darwin/win32) on a temporary directory, and platform-independent + * unit tests for the native event-mapping logic (`NativeSignalMapper`, + * stat injected). */ import { readdirSync } from 'node:fs'; import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { basename, join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; -import { HostFsWatchService } from '#/os/backends/node-local/hostFsWatchService'; -import type { HostFsChange, IHostFsWatchHandle } from '#/os/interface/hostFsWatch'; +import { + HostFsWatchService, + NativeSignalMapper, +} from '#/os/backends/node-local/hostFsWatchService'; +import type { + HostFsChange, + HostFsChangeKind, + IHostFsWatchHandle, +} from '#/os/interface/hostFsWatch'; const wait = (ms: number): Promise => new Promise((r) => setTimeout(r, ms)); +describe('NativeSignalMapper', () => { + const root = join(tmpdir(), 'mapper-root'); + const noIgnore = (): boolean => false; + + function mapperWith(kinds: Record): NativeSignalMapper { + return new NativeSignalMapper({ kindOf: (p) => kinds[p] }); + } + + it('maps a create → modify → delete sequence', () => { + const file = join(root, 'a.txt'); + const kinds: Record = { [file]: 'file' }; + const mapper = mapperWith(kinds); + expect(mapper.map(root, 'rename', 'a.txt', noIgnore)).toEqual({ + path: file, + action: 'created', + kind: 'file', + }); + expect(mapper.map(root, 'rename', 'a.txt', noIgnore)).toEqual({ + path: file, + action: 'modified', + kind: 'file', + }); + delete kinds[file]; + expect(mapper.map(root, 'rename', 'a.txt', noIgnore)).toEqual({ + path: file, + action: 'deleted', + kind: 'file', + }); + }); + + it('reports the directory kind for renamed directories', () => { + const dir = join(root, 'd'); + const mapper = mapperWith({ [dir]: 'directory' }); + expect(mapper.map(root, 'rename', 'd', noIgnore)).toEqual({ + path: dir, + action: 'created', + kind: 'directory', + }); + }); + + it('maps change events to modified without consulting stat', () => { + const file = join(root, 'a.txt'); + const mapper = mapperWith({}); + expect(mapper.map(root, 'change', 'a.txt', noIgnore)).toEqual({ + path: file, + action: 'modified', + kind: 'file', + }); + }); + + it('treats null, empty and root-basename filenames as root events', () => { + const mapper = mapperWith({ [root]: 'directory' }); + expect(mapper.map(root, 'rename', null, noIgnore)?.path).toBe(root); + expect(mapper.map(root, 'rename', '', noIgnore)?.path).toBe(root); + expect(mapper.map(root, 'rename', basename(root), noIgnore)?.path).toBe(root); + }); + + it('keeps a root-basename filename as a child when that child exists', () => { + const child = join(root, basename(root)); + const mapper = mapperWith({ [child]: 'file' }); + expect(mapper.map(root, 'rename', basename(root), noIgnore)?.path).toBe(child); + }); + + it('keeps absolute filenames inside the root and clamps outside ones to a root event', () => { + const file = join(root, 'a.txt'); + const mapper = mapperWith({ [file]: 'file' }); + expect(mapper.map(root, 'rename', file, noIgnore)?.path).toBe(file); + expect(mapper.map(root, 'rename', join(tmpdir(), 'elsewhere'), noIgnore)?.path).toBe(root); + }); + + it('drops events whose path is ignored', () => { + const file = join(root, 'node_modules', 'x.js'); + const mapper = mapperWith({ [file]: 'file' }); + expect( + mapper.map(root, 'rename', join('node_modules', 'x.js'), (p) => p.includes('node_modules')), + ).toBeUndefined(); + }); +}); + describe('HostFsWatchService', () => { let root: string; let handle: IHostFsWatchHandle | undefined; diff --git a/packages/agent-core-v2/test/workspace/workspaceSkillCatalog/skillCatalog.test.ts b/packages/agent-core-v2/test/workspace/workspaceSkillCatalog/skillCatalog.test.ts index 1fb78ebfa3..0504fc3050 100644 --- a/packages/agent-core-v2/test/workspace/workspaceSkillCatalog/skillCatalog.test.ts +++ b/packages/agent-core-v2/test/workspace/workspaceSkillCatalog/skillCatalog.test.ts @@ -902,4 +902,42 @@ describe('WorkspaceSkillCatalogService', () => { await rm(workDir, { recursive: true, force: true }); } }, 15000); + + it('rescans when a skill under a dot directory appears on disk', async () => { + const workDir = await mkdtemp(join(tmpdir(), 'skill-watch-dot-')); + const host = createScopedTestHost([ + stubPair(IBootstrapService, bootstrapStub), + stubPair(IConfigService, configStub()), + stubPair(IPluginService, pluginStub()), + stubPair(ILogService, stubLog()), + stubPair(ISkillDiscovery, new FileSkillDiscovery(stubLog())), + stubPair(IHostFsWatchService, new HostFsWatchService()), + ]); + const workspace = host.child(LifecycleScope.Workspace, 'w1', [ + stubPair(IWorkspaceContext, workspaceContextStub(workDir)), + ]); + + try { + const catalog = workspace.accessor.get(IWorkspaceSkillCatalog); + await catalog.load(); + expect(catalog.catalog.getSkill('dot-skill')).toBeUndefined(); + + await mkdir(join(workDir, '.agents', 'skills', '.dot-skill'), { recursive: true }); + await writeFile( + join(workDir, '.agents', 'skills', '.dot-skill', 'SKILL.md'), + '---\nname: dot-skill\ndescription: under dot dir\n---\nbody', + 'utf8', + ); + + const deadline = Date.now() + 10000; + while (catalog.catalog.getSkill('dot-skill') === undefined) { + if (Date.now() > deadline) throw new Error('watch-driven refresh timed out'); + await new Promise((resolve) => setTimeout(resolve, 100)); + } + expect(catalog.catalog.getSkill('dot-skill')?.description).toBe('under dot dir'); + } finally { + host.dispose(); + await rm(workDir, { recursive: true, force: true }); + } + }, 15000); }); From 320faf82d17e4eebc05e3e580e1046b0dbd22b0a Mon Sep 17 00:00:00 2001 From: 7Sageer Date: Wed, 5 Aug 2026 10:42:14 +0800 Subject: [PATCH 3/4] Fix spawn EBADF issue on macOS for large file trees The skill watcher no longer opens every file it watches, improving performance. Signed-off-by: 7Sageer --- .changeset/skill-watch-fd-exhaustion.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/skill-watch-fd-exhaustion.md b/.changeset/skill-watch-fd-exhaustion.md index dd684030f7..89585e6889 100644 --- a/.changeset/skill-watch-fd-exhaustion.md +++ b/.changeset/skill-watch-fd-exhaustion.md @@ -2,4 +2,4 @@ "@moonshot-ai/kimi-code": patch --- -Fix all tool calls failing with spawn EBADF on macOS when a skill folder contains a very large file tree (e.g. a bundled Python runtime); the skill watcher no longer opens every file it watches. +Fix all tool calls failing with spawn EBADF on macOS when a skill folder contains a very large file tree. From 4582900719896d9a9ea52cfca63a92fd564aa540 Mon Sep 17 00:00:00 2001 From: 7Sageer <7sageer@djwcb.cn> Date: Wed, 5 Aug 2026 11:29:17 +0800 Subject: [PATCH 4/4] fix(agent-core-v2): harden native signal watch recovery --- .../agent-core-v2/src/_base/utils/paths.ts | 18 +- .../app/skillCatalog/fileSkillDiscovery.ts | 7 +- .../backends/node-local/hostFsWatchService.ts | 175 +++++----- .../src/os/interface/hostFsWatch.ts | 5 +- .../rootFileSkillSource.ts | 14 +- .../test/_base/utils/paths.test.ts | 18 +- .../node-local/hostFsWatchService.test.ts | 317 +++++++++++------- .../skillCatalog.test.ts | 76 +---- 8 files changed, 313 insertions(+), 317 deletions(-) diff --git a/packages/agent-core-v2/src/_base/utils/paths.ts b/packages/agent-core-v2/src/_base/utils/paths.ts index 8b2b321519..968ae8bb0b 100644 --- a/packages/agent-core-v2/src/_base/utils/paths.ts +++ b/packages/agent-core-v2/src/_base/utils/paths.ts @@ -1,17 +1,8 @@ /** - * Path-filter helpers — pure string predicates, no IO. + * `_base/utils/paths` (cross-cutting) — pure path-filter predicates. * - * `subtreeWatchFilter` builds an `ignored` predicate that confines a recursive - * fs watch to the candidate subtrees under `root` plus their ancestor chain - * (so candidates that do not exist yet are still detected once created). The - * optional knobs prune paths BELOW a candidate only, mirroring a - * recursion-gated scanner: `maxDepth` rejects anything deeper than that many - * segments below the candidate, and an entry matching `skipEntry` (e.g. the - * skill scanner's `node_modules` / dot-entry rule) stops further watching — - * but the entry itself, and with `keepEntryFile` also its direct child file - * of that name, stay watched, because a scanner still probes those without - * recursing deeper. Everything below that point is what the consumer never - * reads, and only that is pruned. + * Constrains filesystem watches to selected subtrees and scanner-visible + * entries. */ function normalizeSlashes(p: string): string { @@ -51,6 +42,7 @@ function isPrunedBelowCandidate( ): boolean { if (options === undefined) return false; const segments = rel.split('/'); + if (options.maxDepth !== undefined && segments.length > options.maxDepth) return true; if (options.skipEntry !== undefined) { const excludedAt = segments.findIndex(options.skipEntry); if (excludedAt !== -1) { @@ -62,5 +54,5 @@ function isPrunedBelowCandidate( ); } } - return options.maxDepth !== undefined && segments.length > options.maxDepth; + return false; } diff --git a/packages/agent-core-v2/src/app/skillCatalog/fileSkillDiscovery.ts b/packages/agent-core-v2/src/app/skillCatalog/fileSkillDiscovery.ts index ceb2f9ad66..539f341cad 100644 --- a/packages/agent-core-v2/src/app/skillCatalog/fileSkillDiscovery.ts +++ b/packages/agent-core-v2/src/app/skillCatalog/fileSkillDiscovery.ts @@ -2,11 +2,8 @@ * `skillCatalog` domain — filesystem `ISkillDiscovery` backend. * * Discovers skill bundles by walking caller-supplied roots and parsing each - * SKILL.md, pruning `node_modules` / dot entries and capping the walk depth - * (`isSkillScanExcludedEntry`, `MAX_SKILL_SCAN_DEPTH` — exported so fs - * watches can mirror the scan's own pruning instead of watching subtrees it - * would never read). Exposes both the App-scoped `ISkillDiscovery` service - * and a stateless standalone function. + * SKILL.md. Exposes discovery through the App-scoped service and a stateless + * filesystem entry point. */ import { promises as fs } from 'node:fs'; diff --git a/packages/agent-core-v2/src/os/backends/node-local/hostFsWatchService.ts b/packages/agent-core-v2/src/os/backends/node-local/hostFsWatchService.ts index bd169da69d..631db3176b 100644 --- a/packages/agent-core-v2/src/os/backends/node-local/hostFsWatchService.ts +++ b/packages/agent-core-v2/src/os/backends/node-local/hostFsWatchService.ts @@ -1,28 +1,16 @@ /** * `hostFsWatch` domain — `IHostFsWatchService` implementation. * - * Wraps `chokidar` to report raw create/modify/delete events under an absolute - * path. Each `watch()` call owns an independent watcher; disposing the handle - * closes it. A `signal`-mode recursive watch on darwin/win32 instead uses ONE - * native recursive `fs.watch` (FSEvents / ReadDirectoryChangesW), whose fd - * footprint stays constant in the subtree size — chokidar holds one - * `fs.watch` fd per file and per directory on macOS, so per-node watching of - * a fat subtree can exhaust the process fd budget and break every subsequent - * spawn. The signal leg owns its recovery: a native error (including sync - * creation failure) fires one root-level invalidation and re-arms the native - * watch with capped exponential backoff, so a transient failure neither - * downgrades the consumer to the per-node watcher nor silently ends hot - * reload; chokidar serves only when the platform has no recursive - * `fs.watch`. Event mapping lives in `NativeSignalMapper` as pure logic - * with the stat call injected, so it is testable off darwin/win32. - * Bound at App scope. + * Reports precise or coarse host filesystem changes through platform + * watchers. Each handle owns and disposes its watcher. Bound at App scope. */ -import { watch as fsWatch, lstatSync, type FSWatcher as NodeFSWatcher } from 'node:fs'; +import { watch as fsWatch } from 'node:fs'; import { basename, isAbsolute, join, relative } from 'node:path'; import { FSWatcher } from 'chokidar'; +import type { IDisposable } from '#/_base/di/lifecycle'; import { Emitter, type Event } from '#/_base/event'; import { LifecycleScope, ScopeActivation, registerScopedService } from '#/_base/di/scope'; import { onUnexpectedError } from '#/_base/errors/unexpectedError'; @@ -41,53 +29,34 @@ const DEFAULT_IGNORED = (p: string): boolean => /(?:^|[/\\])\.git(?:$|[/\\])/.te const NATIVE_RETRY_BASE_MS = 1000; const NATIVE_RETRY_MAX_MS = 30000; -export interface NativeSignalStat { - kindOf(absPath: string): HostFsChangeKind | undefined; +interface NativeFsWatcher { + close(): void; + on(event: 'error', listener: (error: NodeJS.ErrnoException) => void): this; } -export class NativeSignalMapper { - private readonly knownKinds = new Map(); - - constructor(private readonly stat: NativeSignalStat) {} - - map( +interface HostFsWatchRuntime { + readonly platform: NodeJS.Platform; + watchNative( root: string, - eventType: string, - filename: string | null, - ignored: (path: string) => boolean, - ): HostFsChange | undefined { - const absPath = this.resolveEventPath(root, filename); - if (absPath !== root && ignored(absPath)) return undefined; - if (eventType === 'change') { - return { path: absPath, action: 'modified', kind: this.knownKinds.get(absPath) ?? 'file' }; - } - if (eventType !== 'rename') return undefined; - const statKind = this.stat.kindOf(absPath); - if (statKind !== undefined) { - const action: HostFsChangeAction = this.knownKinds.has(absPath) ? 'modified' : 'created'; - this.knownKinds.set(absPath, statKind); - return { path: absPath, action, kind: statKind }; - } - const kind = this.knownKinds.get(absPath) ?? 'file'; - this.knownKinds.delete(absPath); - return { path: absPath, action: 'deleted', kind }; - } - - private resolveEventPath(root: string, filename: string | null): string { - if (filename === null || filename === '') return root; - if (isAbsolute(filename)) return clampToRoot(root, filename); - if (filename === basename(root) && this.stat.kindOf(join(root, filename)) === undefined) { - return root; - } - return join(root, filename); - } + listener: (eventType: string, filename: string | null) => void, + ): NativeFsWatcher; + scheduleRetry(callback: () => void, delayMs: number): IDisposable; } -function clampToRoot(root: string, absPath: string): string { - const rel = relative(root, absPath); - if (rel === '' || (!rel.startsWith('..') && !isAbsolute(rel))) return absPath; - return root; -} +const NODE_HOST_FS_WATCH_RUNTIME: HostFsWatchRuntime = { + platform: process.platform, + watchNative: (root, listener) => + fsWatch(root, { persistent: false, recursive: true }, listener), + scheduleRetry: (callback, delayMs) => { + const timer = setTimeout(callback, delayMs); + timer.unref?.(); + return { + dispose: () => { + clearTimeout(timer); + }, + }; + }, +}; class HostFsWatchHandle implements IHostFsWatchHandle { readonly onDidChange: Event; @@ -129,22 +98,18 @@ class SignalWatchHandle implements IHostFsWatchHandle { private readonly emitter: Emitter; private readonly ignored: (path: string) => boolean; - private readonly mapper = new NativeSignalMapper({ - kindOf: (absPath) => { - try { - return lstatSync(absPath).isDirectory() ? 'directory' : 'file'; - } catch { - return undefined; - } - }, - }); - private nativeWatcher: NodeFSWatcher | undefined; + private nativeWatcher: NativeFsWatcher | undefined; private chokidarLeg: HostFsWatchHandle | undefined; - private retryTimer: ReturnType | undefined; + private retry: IDisposable | undefined; private retryAttempts = 0; + private recovering = false; private disposed = false; - constructor(private readonly root: string, options: HostFsWatchOptions | undefined) { + constructor( + private readonly root: string, + options: HostFsWatchOptions | undefined, + private readonly runtime: HostFsWatchRuntime, + ) { this.emitter = new Emitter(); this.onDidChange = this.emitter.event; this.ignored = options?.ignored ?? DEFAULT_IGNORED; @@ -154,41 +119,47 @@ class SignalWatchHandle implements IHostFsWatchHandle { private startNativeLeg(): void { if (this.disposed) return; try { - const watcher = fsWatch( - this.root, - { persistent: false, recursive: true }, - (eventType, filename) => { - if (this.disposed) return; - const mapped = this.mapper.map(this.root, eventType, filename, this.ignored); - if (mapped !== undefined) this.emitter.fire(mapped); - }, - ); + const watcher = this.runtime.watchNative(this.root, (_eventType, filename) => { + if (this.disposed) return; + this.retryAttempts = 0; + const absPath = resolveNativeSignalPath(this.root, filename); + if (absPath !== this.root && this.ignored(absPath)) return; + this.fireInvalidation(); + }); watcher.on('error', (error: NodeJS.ErrnoException) => { - this.onNativeError(error); + this.onNativeError(watcher, error); }); this.nativeWatcher = watcher; - this.retryAttempts = 0; + if (this.recovering) { + this.recovering = false; + this.fireInvalidation(); + } } catch (error) { - this.onNativeError(error as NodeJS.ErrnoException); + this.onNativeError(undefined, error as NodeJS.ErrnoException); } } - private onNativeError(error: NodeJS.ErrnoException): void { + private onNativeError(watcher: NativeFsWatcher | undefined, error: NodeJS.ErrnoException): void { if (this.disposed) return; - this.nativeWatcher?.close(); + if (watcher !== undefined && watcher !== this.nativeWatcher) return; + watcher?.close(); this.nativeWatcher = undefined; if (error.code === 'ERR_FEATURE_UNAVAILABLE_ON_PLATFORM') { + this.recovering = false; this.startChokidarLeg(); + this.fireInvalidation(); return; } onUnexpectedError(error); - this.emitter.fire({ path: this.root, action: 'modified', kind: 'directory' }); + this.recovering = true; + this.fireInvalidation(); const delay = Math.min(NATIVE_RETRY_BASE_MS * 2 ** this.retryAttempts, NATIVE_RETRY_MAX_MS); this.retryAttempts += 1; - this.retryTimer = setTimeout(() => { + this.retry?.dispose(); + this.retry = this.runtime.scheduleRetry(() => { + this.retry = undefined; this.startNativeLeg(); }, delay); - this.retryTimer.unref?.(); } private startChokidarLeg(): void { @@ -200,10 +171,14 @@ class SignalWatchHandle implements IHostFsWatchHandle { this.chokidarLeg = leg; } + private fireInvalidation(): void { + this.emitter.fire({ path: this.root, action: 'modified', kind: 'directory' }); + } + dispose(): void { if (this.disposed) return; this.disposed = true; - if (this.retryTimer !== undefined) clearTimeout(this.retryTimer); + this.retry?.dispose(); this.nativeWatcher?.close(); this.chokidarLeg?.dispose(); this.emitter.dispose(); @@ -213,22 +188,38 @@ class SignalWatchHandle implements IHostFsWatchHandle { export class HostFsWatchService implements IHostFsWatchService { declare readonly _serviceBrand: undefined; + constructor(private readonly runtime: HostFsWatchRuntime = NODE_HOST_FS_WATCH_RUNTIME) {} + watch(path: string, options?: HostFsWatchOptions): IHostFsWatchHandle { - if (useNativeRecursive(options)) { - return new SignalWatchHandle(path, options); + if (useNativeRecursive(options, this.runtime.platform)) { + return new SignalWatchHandle(path, options, this.runtime); } return new HostFsWatchHandle(path, options); } } -function useNativeRecursive(options: HostFsWatchOptions | undefined): boolean { +function useNativeRecursive( + options: HostFsWatchOptions | undefined, + platform: NodeJS.Platform, +): boolean { return ( options?.signal === true && options.recursive !== false && - (process.platform === 'darwin' || process.platform === 'win32') + (platform === 'darwin' || platform === 'win32') ); } +function resolveNativeSignalPath(root: string, filename: string | null): string { + if (filename === null || filename === '' || filename === basename(root)) return root; + return clampToRoot(root, isAbsolute(filename) ? filename : join(root, filename)); +} + +function clampToRoot(root: string, absPath: string): string { + const rel = relative(root, absPath); + if (rel === '' || (!rel.startsWith('..') && !isAbsolute(rel))) return absPath; + return root; +} + function mapChokidarEvent(eventName: string, absPath: string): HostFsChange | undefined { const mapped = mapActionAndKind(eventName); if (mapped === undefined) return undefined; diff --git a/packages/agent-core-v2/src/os/interface/hostFsWatch.ts b/packages/agent-core-v2/src/os/interface/hostFsWatch.ts index 44601d8bdf..6da00ebfa5 100644 --- a/packages/agent-core-v2/src/os/interface/hostFsWatch.ts +++ b/packages/agent-core-v2/src/os/interface/hostFsWatch.ts @@ -7,9 +7,8 @@ * `HostFsWatchOptions.signal` marks callers that consume events as a mere * "something changed" signal (ignoring action/kind); the backend may then * pick a cheaper implementation (one native recursive watch instead of - * per-node watchers), and action/kind may be coarse — a deleted entry whose - * kind was never observed is reported as 'file'. App-scoped — one shared - * instance. + * per-node watchers). Signal events may use the watched root as their path + * and report coarse action/kind values. App-scoped — one shared instance. */ import { createDecorator, type ServiceIdentifier } from '#/_base/di/instantiation'; diff --git a/packages/agent-core-v2/src/workspace/workspaceSkillCatalog/rootFileSkillSource.ts b/packages/agent-core-v2/src/workspace/workspaceSkillCatalog/rootFileSkillSource.ts index 54c4954985..120eb837f6 100644 --- a/packages/agent-core-v2/src/workspace/workspaceSkillCatalog/rootFileSkillSource.ts +++ b/packages/agent-core-v2/src/workspace/workspaceSkillCatalog/rootFileSkillSource.ts @@ -4,17 +4,9 @@ * * Discovers project skills from the handler's workspace root * (`workspaceContext.cwd`) through `ISkillDiscovery`, contributing them at - * priority 30 (above user / extra / plugin / builtin). Watches the project - * skill-root candidates (`.kimi-code/skills`, `.agents/skills` under the - * project root, watched whether or not they exist yet) through - * `hostFsWatch` and re-fires `onDidChange` debounced, so the catalog - * re-scans THIS source only when project skill files change. The watch - * mirrors the scanner's own pruning — `node_modules` / dot entries gate - * recursion but their direct SKILL.md stays watched, and the depth cap is - * the scanner's plus two segments (the skill directory and its SKILL.md) - * — and runs in `signal` mode, so a fat skill subtree (e.g. a skill - * bundling a runtime environment) does not cost one fs-watch fd per file. - * Bound at Workspace scope so every session of the handler shares one scan. + * priority 30. Watches project skill-root candidates through `hostFsWatch` + * and emits debounced invalidations for source reloads. Bound at Workspace + * scope so every session of the handler shares one scan. */ import { createDecorator, type ServiceIdentifier } from '#/_base/di/instantiation'; diff --git a/packages/agent-core-v2/test/_base/utils/paths.test.ts b/packages/agent-core-v2/test/_base/utils/paths.test.ts index e90a6f4c85..8fe83c9f23 100644 --- a/packages/agent-core-v2/test/_base/utils/paths.test.ts +++ b/packages/agent-core-v2/test/_base/utils/paths.test.ts @@ -1,12 +1,16 @@ /** - * `_base/utils/paths` — unit tests for `subtreeWatchFilter`. + * Scenario: recursive watches constrained to selected candidate subtrees. + * Responsibilities: candidate ancestry, scan-depth bounds, and excluded-entry + * probing. Wiring: pure path predicates with no external collaborators. + * Run: `pnpm --filter @moonshot-ai/agent-core-v2 exec vitest run + * test/_base/utils/paths.test.ts`. */ import { describe, expect, it } from 'vitest'; import { subtreeWatchFilter } from '#/_base/utils/paths'; -describe('subtreeWatchFilter', () => { +describe('subtree watch filtering', () => { const root = '/repo'; const candidates = ['/repo/.kimi-code/skills', '/repo/.agents/skills']; @@ -47,6 +51,16 @@ describe('subtreeWatchFilter', () => { expect(ignored('/repo/.agents/skills/demo/node_modules/pkg')).toBe(true); }); + it('applies max depth before excluded-entry exceptions', () => { + const ignored = subtreeWatchFilter(root, candidates, { + maxDepth: 3, + skipEntry: (name) => name === 'node_modules', + keepEntryFile: 'SKILL.md', + }); + expect(ignored('/repo/.agents/skills/demo/a/b/node_modules')).toBe(true); + expect(ignored('/repo/.agents/skills/demo/a/b/node_modules/SKILL.md')).toBe(true); + }); + it('never prunes the candidate ancestor chain itself', () => { const ignored = subtreeWatchFilter(root, candidates, { skipEntry: (name) => name.startsWith('.'), diff --git a/packages/agent-core-v2/test/os/backends/node-local/hostFsWatchService.test.ts b/packages/agent-core-v2/test/os/backends/node-local/hostFsWatchService.test.ts index f880927c86..b5c167c2e8 100644 --- a/packages/agent-core-v2/test/os/backends/node-local/hostFsWatchService.test.ts +++ b/packages/agent-core-v2/test/os/backends/node-local/hostFsWatchService.test.ts @@ -1,117 +1,138 @@ /** - * `hostFsWatch` domain — integration tests against the real watcher - * backends (chokidar, plus the native recursive watch used by `signal` - * mode on darwin/win32) on a temporary directory, and platform-independent - * unit tests for the native event-mapping logic (`NativeSignalMapper`, - * stat injected). + * Scenario: precise host watches and coarse native signal watches. + * Responsibilities: event delivery, filtering, recovery, disposal, and the + * macOS descriptor bound. Wiring: real temporary files for integration and an + * injected native-watch boundary with a manual retry scheduler for recovery. + * Run: `pnpm --filter @moonshot-ai/agent-core-v2 exec vitest run + * test/os/backends/node-local/hostFsWatchService.test.ts`. */ import { readdirSync } from 'node:fs'; import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; -import { basename, join } from 'node:path'; +import { join } from 'node:path'; -import { afterEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { - HostFsWatchService, - NativeSignalMapper, -} from '#/os/backends/node-local/hostFsWatchService'; + resetUnexpectedErrorHandler, + setUnexpectedErrorHandler, +} from '#/_base/errors/unexpectedError'; +import { HostFsWatchService } from '#/os/backends/node-local/hostFsWatchService'; import type { HostFsChange, - HostFsChangeKind, IHostFsWatchHandle, + IHostFsWatchService, } from '#/os/interface/hostFsWatch'; const wait = (ms: number): Promise => new Promise((r) => setTimeout(r, ms)); -describe('NativeSignalMapper', () => { - const root = join(tmpdir(), 'mapper-root'); - const noIgnore = (): boolean => false; +type HostFsWatchRuntime = NonNullable[0]>; - function mapperWith(kinds: Record): NativeSignalMapper { - return new NativeSignalMapper({ kindOf: (p) => kinds[p] }); - } - - it('maps a create → modify → delete sequence', () => { - const file = join(root, 'a.txt'); - const kinds: Record = { [file]: 'file' }; - const mapper = mapperWith(kinds); - expect(mapper.map(root, 'rename', 'a.txt', noIgnore)).toEqual({ - path: file, - action: 'created', - kind: 'file', - }); - expect(mapper.map(root, 'rename', 'a.txt', noIgnore)).toEqual({ - path: file, - action: 'modified', - kind: 'file', - }); - delete kinds[file]; - expect(mapper.map(root, 'rename', 'a.txt', noIgnore)).toEqual({ - path: file, - action: 'deleted', - kind: 'file', - }); - }); - - it('reports the directory kind for renamed directories', () => { - const dir = join(root, 'd'); - const mapper = mapperWith({ [dir]: 'directory' }); - expect(mapper.map(root, 'rename', 'd', noIgnore)).toEqual({ - path: dir, - action: 'created', - kind: 'directory', - }); - }); +class TestNativeWatcher { + private errorListener: ((error: NodeJS.ErrnoException) => void) | undefined; + closed = false; - it('maps change events to modified without consulting stat', () => { - const file = join(root, 'a.txt'); - const mapper = mapperWith({}); - expect(mapper.map(root, 'change', 'a.txt', noIgnore)).toEqual({ - path: file, - action: 'modified', - kind: 'file', - }); - }); - - it('treats null, empty and root-basename filenames as root events', () => { - const mapper = mapperWith({ [root]: 'directory' }); - expect(mapper.map(root, 'rename', null, noIgnore)?.path).toBe(root); - expect(mapper.map(root, 'rename', '', noIgnore)?.path).toBe(root); - expect(mapper.map(root, 'rename', basename(root), noIgnore)?.path).toBe(root); - }); - - it('keeps a root-basename filename as a child when that child exists', () => { - const child = join(root, basename(root)); - const mapper = mapperWith({ [child]: 'file' }); - expect(mapper.map(root, 'rename', basename(root), noIgnore)?.path).toBe(child); - }); - - it('keeps absolute filenames inside the root and clamps outside ones to a root event', () => { - const file = join(root, 'a.txt'); - const mapper = mapperWith({ [file]: 'file' }); - expect(mapper.map(root, 'rename', file, noIgnore)?.path).toBe(file); - expect(mapper.map(root, 'rename', join(tmpdir(), 'elsewhere'), noIgnore)?.path).toBe(root); - }); + on(_event: 'error', listener: (error: NodeJS.ErrnoException) => void): this { + this.errorListener = listener; + return this; + } - it('drops events whose path is ignored', () => { - const file = join(root, 'node_modules', 'x.js'); - const mapper = mapperWith({ [file]: 'file' }); - expect( - mapper.map(root, 'rename', join('node_modules', 'x.js'), (p) => p.includes('node_modules')), - ).toBeUndefined(); - }); -}); + close(): void { + this.closed = true; + } -describe('HostFsWatchService', () => { + fail(code = 'EIO'): void { + this.errorListener?.(Object.assign(new Error('native watch failed'), { code })); + } +} + +interface TestNativeAttempt { + readonly watcher: TestNativeWatcher; + emit(filename: string | null): void; +} + +interface TestRetry { + readonly delayMs: number; + readonly active: boolean; + run(): void; +} + +function signalRig(options?: { readonly synchronousFailures?: number }): { + readonly service: IHostFsWatchService; + readonly attempts: TestNativeAttempt[]; + readonly retries: TestRetry[]; + attempt(index: number): TestNativeAttempt; + retry(index: number): TestRetry; +} { + const attempts: TestNativeAttempt[] = []; + const retries: TestRetry[] = []; + let synchronousFailures = options?.synchronousFailures ?? 0; + const runtime: HostFsWatchRuntime = { + platform: 'darwin', + watchNative: (_root, listener) => { + if (synchronousFailures > 0) { + synchronousFailures -= 1; + throw Object.assign(new Error('native watch creation failed'), { code: 'EIO' }); + } + const watcher = new TestNativeWatcher(); + attempts.push({ + watcher, + emit: (filename) => { + listener('rename', filename); + }, + }); + return watcher; + }, + scheduleRetry: (callback, delayMs) => { + let active = true; + retries.push({ + delayMs, + get active() { + return active; + }, + run: () => { + if (!active) return; + active = false; + callback(); + }, + }); + return { + dispose: () => { + active = false; + }, + }; + }, + }; + return { + service: new HostFsWatchService(runtime), + attempts, + retries, + attempt: (index) => requiredAt(attempts, index), + retry: (index) => requiredAt(retries, index), + }; +} + +function requiredAt(values: readonly T[], index: number): T { + const value = values[index]; + if (value === undefined) throw new Error(`missing test value at index ${index}`); + return value; +} + +describe('host filesystem change notifications', () => { let root: string; let handle: IHostFsWatchHandle | undefined; + beforeEach(() => { + setUnexpectedErrorHandler(() => undefined); + }); + afterEach(async () => { handle?.dispose(); handle = undefined; if (root) await rm(root, { recursive: true, force: true }); + root = ''; + resetUnexpectedErrorHandler(); }); async function start(recursive = true): Promise { @@ -132,6 +153,97 @@ describe('HostFsWatchService', () => { return events; } + it('emits a coarse root invalidation when a native signal path changes', () => { + const rig = signalRig(); + const events: HostFsChange[] = []; + handle = rig.service.watch('/repo', { signal: true }); + handle.onDidChange((event) => events.push(event)); + + rig.attempt(0).emit('skills/demo/SKILL.md'); + + expect(events).toEqual([{ path: '/repo', action: 'modified', kind: 'directory' }]); + }); + + it('does not invalidate when a native signal path is ignored', () => { + const rig = signalRig(); + const events: HostFsChange[] = []; + handle = rig.service.watch('/repo', { + signal: true, + ignored: (path) => path.includes('node_modules'), + }); + handle.onDidChange((event) => events.push(event)); + + rig.attempt(0).emit('node_modules/pkg/index.js'); + + expect(events).toEqual([]); + }); + + it('increases the retry delay after consecutive native failures', () => { + const rig = signalRig(); + handle = rig.service.watch('/repo', { signal: true }); + + rig.attempt(0).watcher.fail(); + rig.retry(0).run(); + rig.attempt(1).watcher.fail(); + rig.retry(1).run(); + rig.attempt(2).watcher.fail(); + + expect(rig.retries.map((retry) => retry.delayMs)).toEqual([1000, 2000, 4000]); + }); + + it('invalidates again after a native watch is rearmed', () => { + const rig = signalRig(); + const events: HostFsChange[] = []; + handle = rig.service.watch('/repo', { signal: true }); + handle.onDidChange((event) => events.push(event)); + + rig.attempt(0).watcher.fail(); + rig.retry(0).run(); + + expect(events).toEqual([ + { path: '/repo', action: 'modified', kind: 'directory' }, + { path: '/repo', action: 'modified', kind: 'directory' }, + ]); + }); + + it('invalidates after recovering from a synchronous native-watch creation failure', () => { + const rig = signalRig({ synchronousFailures: 1 }); + const events: HostFsChange[] = []; + handle = rig.service.watch('/repo', { signal: true }); + handle.onDidChange((event) => events.push(event)); + + rig.retry(0).run(); + + expect(rig.attempts).toHaveLength(1); + expect(events).toEqual([{ path: '/repo', action: 'modified', kind: 'directory' }]); + }); + + it('resets the retry delay after the recovered native watch emits an event', () => { + const rig = signalRig(); + handle = rig.service.watch('/repo', { signal: true }); + + rig.attempt(0).watcher.fail(); + rig.retry(0).run(); + rig.attempt(1).emit('skills/demo/SKILL.md'); + rig.attempt(1).watcher.fail(); + + expect(rig.retries.map((retry) => retry.delayMs)).toEqual([1000, 1000]); + }); + + it('cancels a pending native retry when the watch handle is disposed', () => { + const rig = signalRig(); + handle = rig.service.watch('/repo', { signal: true }); + rig.attempt(0).watcher.fail(); + + handle.dispose(); + handle = undefined; + rig.retry(0).run(); + + expect(rig.retry(0).active).toBe(false); + expect(rig.attempt(0).watcher.closed).toBe(true); + expect(rig.attempts).toHaveLength(1); + }); + it('reports create / modify / delete for a file', async () => { root = await mkdtemp(join(tmpdir(), 'hostfswatch-')); const events = await start(); @@ -186,37 +298,6 @@ describe('HostFsWatchService', () => { expect(events).toHaveLength(0); }); - it('signal mode reports create / modify / delete for a file', async () => { - root = await mkdtemp(join(tmpdir(), 'hostfswatch-signal-')); - const events = await startSignal(); - - const file = join(root, 'a.txt'); - await writeFile(file, 'v1'); - await wait(500); - await writeFile(file, 'v2'); - await wait(500); - await rm(file); - await wait(500); - - const actions = events.filter((e) => e.path === file).map((e) => e.action); - expect(actions).toContain('created'); - expect(actions).toContain('modified'); - expect(actions).toContain('deleted'); - }); - - it('signal mode honors the ignored predicate', async () => { - root = await mkdtemp(join(tmpdir(), 'hostfswatch-signal-')); - const events = await startSignal((p) => p.includes('node_modules')); - - await mkdir(join(root, 'node_modules', 'pkg'), { recursive: true }); - await writeFile(join(root, 'node_modules', 'pkg', 'index.js'), 'x'); - await writeFile(join(root, 'visible.txt'), 'x'); - await wait(500); - - expect(events.some((e) => e.path.includes('node_modules'))).toBe(false); - expect(events.some((e) => e.path === join(root, 'visible.txt'))).toBe(true); - }); - it.skipIf(process.platform !== 'darwin')( 'signal mode keeps the fd footprint bounded on a fat subtree', async () => { diff --git a/packages/agent-core-v2/test/workspace/workspaceSkillCatalog/skillCatalog.test.ts b/packages/agent-core-v2/test/workspace/workspaceSkillCatalog/skillCatalog.test.ts index 0504fc3050..56533d70a5 100644 --- a/packages/agent-core-v2/test/workspace/workspaceSkillCatalog/skillCatalog.test.ts +++ b/packages/agent-core-v2/test/workspace/workspaceSkillCatalog/skillCatalog.test.ts @@ -3,10 +3,7 @@ * * Exercises the real Workspace-scoped catalog and source services with * filesystem or in-memory discovery boundaries, including controlled - * concurrent refreshes and the fs-watch-driven single-source rescan. On - * darwin the skill-source watch runs in native `signal` mode, which may - * surface coalesced ancestor-directory events, so the pruned-write case - * asserts unchanged catalog content there and zero rescans elsewhere. + * concurrent refreshes and fs-watch-driven single-source rescans. * Run: `pnpm --filter @moonshot-ai/agent-core-v2 exec vitest run * test/workspace/workspaceSkillCatalog/skillCatalog.test.ts`. */ @@ -839,70 +836,6 @@ describe('WorkspaceSkillCatalogService', () => { } }, 15000); - it('ignores changes under scanner-pruned entries but still rescans on real skill changes', async () => { - const workDir = await mkdtemp(join(tmpdir(), 'skill-watch-pruned-')); - await mkdir(join(workDir, '.agents', 'skills', 'demo'), { recursive: true }); - await writeFile( - join(workDir, '.agents', 'skills', 'demo', 'SKILL.md'), - '---\nname: demo\ndescription: v1\n---\nbody', - 'utf8', - ); - const host = createScopedTestHost([ - stubPair(IBootstrapService, bootstrapStub), - stubPair(IConfigService, configStub()), - stubPair(IPluginService, pluginStub()), - stubPair(ILogService, stubLog()), - stubPair(ISkillDiscovery, new FileSkillDiscovery(stubLog())), - stubPair(IHostFsWatchService, new HostFsWatchService()), - ]); - const workspace = host.child(LifecycleScope.Workspace, 'w1', [ - stubPair(IWorkspaceContext, workspaceContextStub(workDir)), - ]); - - try { - const catalog = workspace.accessor.get(IWorkspaceSkillCatalog); - await catalog.load(); - expect(catalog.catalog.getSkill('demo')?.description).toBe('v1'); - - const sourceIds: string[] = []; - const subscription = catalog.onDidChange((sourceId) => sourceIds.push(sourceId)); - - await mkdir(join(workDir, '.agents', 'skills', 'demo', 'node_modules', 'pkg'), { - recursive: true, - }); - await writeFile( - join(workDir, '.agents', 'skills', 'demo', 'node_modules', 'pkg', 'index.js'), - 'x', - 'utf8', - ); - await mkdir(join(workDir, '.agents', 'skills', 'demo', '.hidden'), { recursive: true }); - await writeFile(join(workDir, '.agents', 'skills', 'demo', '.hidden', 'notes.md'), 'x', 'utf8'); - await new Promise((resolve) => setTimeout(resolve, 1500)); - if (process.platform === 'darwin') { - expect(catalog.catalog.getSkill('demo')?.description).toBe('v1'); - expect(catalog.catalog.getSkill('pkg')).toBeUndefined(); - } else { - expect(sourceIds).toEqual([]); - } - - const refreshed = waitForEvents(catalog.onDidChange, 1); - const timedOut = new Promise((_resolve, reject) => { - setTimeout(() => reject(new Error('watch-driven refresh timed out')), 10000); - }); - await writeFile( - join(workDir, '.agents', 'skills', 'demo', 'SKILL.md'), - '---\nname: demo\ndescription: v2\n---\nbody', - 'utf8', - ); - await Promise.race([refreshed, timedOut]); - expect(catalog.catalog.getSkill('demo')?.description).toBe('v2'); - subscription.dispose(); - } finally { - host.dispose(); - await rm(workDir, { recursive: true, force: true }); - } - }, 15000); - it('rescans when a skill under a dot directory appears on disk', async () => { const workDir = await mkdtemp(join(tmpdir(), 'skill-watch-dot-')); const host = createScopedTestHost([ @@ -923,17 +856,14 @@ describe('WorkspaceSkillCatalogService', () => { expect(catalog.catalog.getSkill('dot-skill')).toBeUndefined(); await mkdir(join(workDir, '.agents', 'skills', '.dot-skill'), { recursive: true }); + const refreshed = waitForEvents(catalog.onDidChange, 1); await writeFile( join(workDir, '.agents', 'skills', '.dot-skill', 'SKILL.md'), '---\nname: dot-skill\ndescription: under dot dir\n---\nbody', 'utf8', ); - const deadline = Date.now() + 10000; - while (catalog.catalog.getSkill('dot-skill') === undefined) { - if (Date.now() > deadline) throw new Error('watch-driven refresh timed out'); - await new Promise((resolve) => setTimeout(resolve, 100)); - } + await refreshed; expect(catalog.catalog.getSkill('dot-skill')?.description).toBe('under dot dir'); } finally { host.dispose();