From e3570280bde775a153ee04388393506da7ac4cc1 Mon Sep 17 00:00:00 2001 From: 7Sageer Date: Wed, 5 Aug 2026 13:47:26 +0800 Subject: [PATCH] fix(agent-core-v2): bound the project skill-root watch fd footprint (#2612) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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. * fix(agent-core-v2): align the skill watch with scanner semantics and harden signal mode 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. * 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 * fix(agent-core-v2): harden native signal watch recovery * fix(agent-core-v2): align skill watch with scanner traversal * fix(agent-core-v2): make skill watch handoff converge --------- Signed-off-by: 7Sageer --- .changeset/skill-watch-fd-exhaustion.md | 5 + .../agent-core-v2/src/_base/utils/paths.ts | 74 +++++- .../app/skillCatalog/fileSkillDiscovery.ts | 15 +- .../skillCatalog/inMemorySkillDiscovery.ts | 2 +- .../src/app/skillCatalog/skillDiscovery.ts | 1 + .../src/app/skillCatalog/skillRoots.ts | 10 +- .../backends/node-local/hostFsWatchService.ts | 205 ++++++++++++++- .../src/os/interface/hostFsWatch.ts | 10 +- .../rootFileSkillSource.ts | 57 +++-- .../test/_base/utils/paths.test.ts | 107 ++++++++ .../app/plugin/manager-consumption.test.ts | 1 + .../test/app/plugin/pluginService.test.ts | 7 +- .../skillCatalog/fileSkillDiscovery.test.ts | 17 ++ .../node-local/hostFsWatchService.test.ts | 242 +++++++++++++++++- .../agentProfileLoader.test.ts | 1 + .../workspaceFs/fsWatchService.test.ts | 1 + .../instructions.test.ts | 2 +- .../workspaceMcp/initialization.test.ts | 1 + .../workspaceMcpConfig.test.ts | 2 +- .../skillCatalog.test.ts | 202 ++++++++++++++- 20 files changed, 917 insertions(+), 45 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 000000000..89585e688 --- /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. diff --git a/packages/agent-core-v2/src/_base/utils/paths.ts b/packages/agent-core-v2/src/_base/utils/paths.ts index 2bb530ba6..e6b230df7 100644 --- a/packages/agent-core-v2/src/_base/utils/paths.ts +++ b/packages/agent-core-v2/src/_base/utils/paths.ts @@ -1,24 +1,94 @@ /** - * Path-filter helpers — pure string predicates, no IO. + * `_base/utils/paths` (cross-cutting) — pure path-filter predicates. + * + * Constrains filesystem watches to selected subtrees and scanner-visible + * entries. */ function normalizeSlashes(p: string): string { return p.replaceAll('\\', '/'); } +export interface SubtreeWatchFilterOptions { + readonly maxDepth?: number; + readonly skipEntry?: (entryName: string) => boolean; + readonly keepEntryFile?: string; + readonly scannedDirectories?: readonly string[]; +} + export function subtreeWatchFilter( root: string, candidates: readonly string[], + options?: SubtreeWatchFilterOptions, ): (path: string) => boolean { const normRoot = normalizeSlashes(root); const normCandidates = candidates.map(normalizeSlashes); + const normScannedDirectories = + options?.scannedDirectories === undefined + ? undefined + : new Set([...normCandidates, ...options.scannedDirectories.map(normalizeSlashes)]); return (p: string): boolean => { 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, + norm.slice(candidate.length + 1), + options, + normScannedDirectories, + ); + } if (candidate.startsWith(`${norm}/`)) return false; } return true; }; } + +function isPrunedBelowCandidate( + normPath: string, + rel: string, + options: SubtreeWatchFilterOptions | undefined, + scannedDirectories: ReadonlySet | undefined, +): 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) { + if (segments.length <= excludedAt + 1) return false; + return !( + options.keepEntryFile !== undefined && + segments.length === excludedAt + 2 && + segments.at(-1) === options.keepEntryFile + ); + } + } + if (scannedDirectories !== undefined) { + return !isScannerVisiblePath(normPath, scannedDirectories, options.keepEntryFile); + } + return false; +} + +function isScannerVisiblePath( + normPath: string, + scannedDirectories: ReadonlySet, + keepEntryFile: string | undefined, +): boolean { + if (scannedDirectories.has(normPath)) return true; + const separatorAt = normPath.lastIndexOf('/'); + if (separatorAt === -1) return false; + const parent = normPath.slice(0, separatorAt); + if (scannedDirectories.has(parent)) return true; + if ( + keepEntryFile === undefined || + normPath.slice(separatorAt + 1) !== keepEntryFile + ) { + return false; + } + const parentSeparatorAt = parent.lastIndexOf('/'); + if (parentSeparatorAt === -1) return false; + return scannedDirectories.has(parent.slice(0, parentSeparatorAt)); +} diff --git a/packages/agent-core-v2/src/app/skillCatalog/fileSkillDiscovery.ts b/packages/agent-core-v2/src/app/skillCatalog/fileSkillDiscovery.ts index 366084cd7..a73077ba8 100644 --- a/packages/agent-core-v2/src/app/skillCatalog/fileSkillDiscovery.ts +++ b/packages/agent-core-v2/src/app/skillCatalog/fileSkillDiscovery.ts @@ -2,8 +2,8 @@ * `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. Exposes discovery through the App-scoped service and a stateless + * filesystem entry point. */ import { promises as fs } from 'node:fs'; @@ -16,7 +16,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; @@ -36,6 +40,7 @@ export async function discoverFileSkills( ): Promise { const byDiscoveryKey = new Map(); const skipped: SkippedSkill[] = []; + const scannedDirectories: string[] = []; async function walkSkillDir( dirPath: string, @@ -52,6 +57,7 @@ export async function discoverFileSkills( } catch { return; } + scannedDirectories.push(dirPath); const directorySkills = new Set(); const subdirs: string[] = []; @@ -60,7 +66,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); } @@ -134,6 +140,7 @@ export async function discoverFileSkills( skills: sortSkills([...byDiscoveryKey.values()]), skipped, scannedRoots: roots.map((root) => root.path), + scannedDirectories, }; } diff --git a/packages/agent-core-v2/src/app/skillCatalog/inMemorySkillDiscovery.ts b/packages/agent-core-v2/src/app/skillCatalog/inMemorySkillDiscovery.ts index 5e41d7d5a..175a94abf 100644 --- a/packages/agent-core-v2/src/app/skillCatalog/inMemorySkillDiscovery.ts +++ b/packages/agent-core-v2/src/app/skillCatalog/inMemorySkillDiscovery.ts @@ -52,7 +52,7 @@ export class InMemorySkillDiscovery implements ISkillDiscovery { if (roots.some((root) => root.source === 'user')) skills.push(...this.userSkills); if (roots.some((root) => root.source === 'project')) skills.push(...this.projectSkills); } - return { skills, skipped: [], scannedRoots: [] }; + return { skills, skipped: [], scannedRoots: [], scannedDirectories: [] }; } } diff --git a/packages/agent-core-v2/src/app/skillCatalog/skillDiscovery.ts b/packages/agent-core-v2/src/app/skillCatalog/skillDiscovery.ts index 544e62c0e..7a30a5cfd 100644 --- a/packages/agent-core-v2/src/app/skillCatalog/skillDiscovery.ts +++ b/packages/agent-core-v2/src/app/skillCatalog/skillDiscovery.ts @@ -19,6 +19,7 @@ export interface SkillDiscoveryResult { readonly skills: readonly SkillDefinition[]; readonly skipped: readonly SkippedSkill[]; readonly scannedRoots: readonly string[]; + readonly scannedDirectories: readonly string[]; } export interface ISkillDiscovery { diff --git a/packages/agent-core-v2/src/app/skillCatalog/skillRoots.ts b/packages/agent-core-v2/src/app/skillCatalog/skillRoots.ts index 2322032e0..8c9d4a5f7 100644 --- a/packages/agent-core-v2/src/app/skillCatalog/skillRoots.ts +++ b/packages/agent-core-v2/src/app/skillCatalog/skillRoots.ts @@ -54,7 +54,7 @@ export interface ProjectSkillRootCandidates { export async function projectSkillRootCandidates( workDir: string, ): Promise { - const projectRoot = await findProjectRoot(workDir); + const projectRoot = await realpathOrSelf(await findProjectRoot(workDir)); return { projectRoot, candidates: [...PROJECT_BRAND_DIRS, ...PROJECT_GENERIC_DIRS].map((dir) => @@ -145,6 +145,14 @@ async function realpath(p: string): Promise { return (await fs.realpath(p)).replaceAll('\\', '/'); } +async function realpathOrSelf(p: string): Promise { + try { + return await realpath(p); + } catch { + return p.replaceAll('\\', '/'); + } +} + async function exists(p: string): Promise { try { await fs.stat(p); 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 8e8794643..cdb945bf1 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,13 +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 `FSWatcher`; disposing the - * handle closes it. 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 } 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'; @@ -23,14 +26,79 @@ import { const DEFAULT_IGNORED = (p: string): boolean => /(?:^|[/\\])\.git(?:$|[/\\])/.test(p); +const NATIVE_RETRY_BASE_MS = 1000; +const NATIVE_RETRY_MAX_MS = 30000; + +interface NativeFsWatcher { + close(): void; + on(event: 'error', listener: (error: NodeJS.ErrnoException) => void): this; +} + +interface HostFsWatchRuntime { + readonly platform: NodeJS.Platform; + watchNative( + root: string, + listener: (eventType: string, filename: string | null) => void, + ): NativeFsWatcher; + scheduleRetry(callback: () => void, delayMs: number): IDisposable; +} + +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); + }, + }; + }, +}; + +interface WatchReadiness { + readonly promise: Promise; + resolve(): void; + reject(error: unknown): void; +} + +function createWatchReadiness(): WatchReadiness { + let resolvePromise!: () => void; + let rejectPromise!: (error: unknown) => void; + let settled = false; + const promise = new Promise((resolve, reject) => { + resolvePromise = resolve; + rejectPromise = reject; + }); + void promise.catch(() => undefined); + return { + promise, + resolve: () => { + if (settled) return; + settled = true; + resolvePromise(); + }, + reject: (error) => { + if (settled) return; + settled = true; + rejectPromise(error); + }, + }; +} + class HostFsWatchHandle implements IHostFsWatchHandle { + readonly ready: Promise; readonly onDidChange: Event; + private readonly readiness = createWatchReadiness(); private readonly emitter: Emitter; private readonly watcher: FSWatcher; private disposed = false; constructor(path: string, options: HostFsWatchOptions | undefined) { + this.ready = this.readiness.promise; this.emitter = new Emitter(); this.onDidChange = this.emitter.event; this.watcher = new FSWatcher({ @@ -45,27 +113,158 @@ class HostFsWatchHandle implements IHostFsWatchHandle { if (mapped !== undefined) this.emitter.fire(mapped); }); this.watcher.on('error', (error: unknown) => { + this.readiness.reject(error); onUnexpectedError(error); }); + this.watcher.once('ready', () => this.readiness.resolve()); this.watcher.add(path); } dispose(): void { if (this.disposed) return; this.disposed = true; + this.readiness.resolve(); void this.watcher.close().catch(() => undefined); this.emitter.dispose(); } } +class SignalWatchHandle implements IHostFsWatchHandle { + readonly ready: Promise; + readonly onDidChange: Event; + + private readonly readiness = createWatchReadiness(); + private readonly emitter: Emitter; + private readonly ignored: (path: string) => boolean; + private nativeWatcher: NativeFsWatcher | undefined; + private chokidarLeg: HostFsWatchHandle | undefined; + private retry: IDisposable | undefined; + private retryAttempts = 0; + private recovering = false; + private disposed = false; + + constructor( + private readonly root: string, + options: HostFsWatchOptions | undefined, + private readonly runtime: HostFsWatchRuntime, + ) { + this.ready = this.readiness.promise; + this.emitter = new Emitter(); + this.onDidChange = this.emitter.event; + this.ignored = options?.ignored ?? DEFAULT_IGNORED; + this.startNativeLeg(); + } + + private startNativeLeg(): void { + if (this.disposed) return; + try { + 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(watcher, error); + }); + this.nativeWatcher = watcher; + this.readiness.resolve(); + if (this.recovering) { + this.recovering = false; + this.fireInvalidation(); + } + } catch (error) { + this.onNativeError(undefined, error as NodeJS.ErrnoException); + } + } + + private onNativeError(watcher: NativeFsWatcher | undefined, error: NodeJS.ErrnoException): void { + if (this.disposed) return; + 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.recovering = true; + this.fireInvalidation(); + const delay = Math.min(NATIVE_RETRY_BASE_MS * 2 ** this.retryAttempts, NATIVE_RETRY_MAX_MS); + this.retryAttempts += 1; + this.retry?.dispose(); + this.retry = this.runtime.scheduleRetry(() => { + this.retry = undefined; + this.startNativeLeg(); + }, delay); + } + + 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); + }); + void leg.ready.then( + () => this.readiness.resolve(), + (error: unknown) => this.readiness.reject(error), + ); + 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; + this.readiness.resolve(); + this.retry?.dispose(); + this.nativeWatcher?.close(); + this.chokidarLeg?.dispose(); + this.emitter.dispose(); + } +} + 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, this.runtime.platform)) { + return new SignalWatchHandle(path, options, this.runtime); + } return new HostFsWatchHandle(path, options); } } +function useNativeRecursive( + options: HostFsWatchOptions | undefined, + platform: NodeJS.Platform, +): boolean { + return ( + options?.signal === true && + options.recursive !== false && + (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 c1668e85f..162f78657 100644 --- a/packages/agent-core-v2/src/os/interface/hostFsWatch.ts +++ b/packages/agent-core-v2/src/os/interface/hostFsWatch.ts @@ -4,7 +4,13 @@ * 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). Signal events may use the watched root as their path + * and report coarse action/kind values. A handle's `ready` promise resolves + * after its backend has installed the initial subscription and rejects when + * initialization fails. App-scoped — one shared instance. */ import { createDecorator, type ServiceIdentifier } from '#/_base/di/instantiation'; @@ -23,9 +29,11 @@ export interface HostFsChange { export interface HostFsWatchOptions { readonly recursive?: boolean; readonly ignored?: (path: string) => boolean; + readonly signal?: boolean; } export interface IHostFsWatchHandle extends IDisposable { + readonly ready: Promise; readonly onDidChange: Event; } diff --git a/packages/agent-core-v2/src/workspace/workspaceSkillCatalog/rootFileSkillSource.ts b/packages/agent-core-v2/src/workspace/workspaceSkillCatalog/rootFileSkillSource.ts index 05d51b47e..17c400b72 100644 --- a/packages/agent-core-v2/src/workspace/workspaceSkillCatalog/rootFileSkillSource.ts +++ b/packages/agent-core-v2/src/workspace/workspaceSkillCatalog/rootFileSkillSource.ts @@ -4,16 +4,13 @@ * * 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. 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'; -import { Disposable } from '#/_base/di/lifecycle'; +import { Disposable, DisposableStore } from '#/_base/di/lifecycle'; import { Emitter, type Event } from '#/_base/event'; import { LifecycleScope, ScopeActivation, registerScopedService } from '#/_base/di/scope'; import { TimeoutTimer } from '#/_base/utils/timer'; @@ -53,7 +50,10 @@ export class WorkspaceRootSkillSource extends Disposable implements IWorkspaceRo private readonly onDidChangeEmitter = this._register(new Emitter()); readonly onDidChange: Event = this.onDidChangeEmitter.event; private readonly watchDebounce = this._register(new TimeoutTimer()); + private readonly watchResources = this._register(new DisposableStore()); private readonly watchReady: Promise; + private activeWatchResources: DisposableStore | undefined; + private watchSignature: string | undefined; constructor( @ISkillDiscovery private readonly discovery: ISkillDiscovery, @@ -68,7 +68,7 @@ export class WorkspaceRootSkillSource extends Disposable implements IWorkspaceRo if (event.domain === MERGE_ALL_AVAILABLE_SKILLS_SECTION) this.onDidChangeEmitter.fire(); }), ); - this.watchReady = this.watchProjectSkillRoots(); + this.watchReady = this.updateProjectSkillRootWatch([]).then(() => undefined); } async load(): Promise { @@ -79,22 +79,49 @@ export class WorkspaceRootSkillSource extends Disposable implements IWorkspaceRo await this.config.ready; const mergeAllAvailableSkills = this.config.get(MERGE_ALL_AVAILABLE_SKILLS_SECTION) ?? true; - return this.discovery.discover( - await projectRoots(this.workspace.cwd, { mergeAllAvailableSkills }), - ); + const discover = async () => + this.discovery.discover( + await projectRoots(this.workspace.cwd, { mergeAllAvailableSkills }), + ); + let contribution = await discover(); + while (await this.updateProjectSkillRootWatch(contribution.scannedDirectories)) { + contribution = await discover(); + } + return contribution; } - private async watchProjectSkillRoots(): Promise { + private async updateProjectSkillRootWatch( + scannedDirectories: readonly string[], + ): Promise { const { projectRoot, candidates } = await projectSkillRootCandidates(this.workspace.cwd); + const signature = [...scannedDirectories].toSorted().join('\0'); + if (signature === this.watchSignature) return false; + const resources = this.watchResources.add(new DisposableStore()); const handle = this.fsWatch.watch(projectRoot, { - ignored: subtreeWatchFilter(projectRoot, candidates), + ignored: subtreeWatchFilter(projectRoot, candidates, { + scannedDirectories, + keepEntryFile: 'SKILL.md', + }), + signal: true, }); - this._register(handle); - this._register( + resources.add(handle); + resources.add( handle.onDidChange(() => { this.watchDebounce.cancelAndSet(() => this.onDidChangeEmitter.fire(), WATCH_DEBOUNCE_MS); }), ); + try { + await handle.ready; + } catch (error) { + this.watchResources.delete(resources); + throw error; + } + if (this.watchResources.isDisposed) return false; + const previous = this.activeWatchResources; + this.activeWatchResources = resources; + this.watchSignature = signature; + if (previous !== undefined) this.watchResources.delete(previous); + return 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 new file mode 100644 index 000000000..1f73ebc59 --- /dev/null +++ b/packages/agent-core-v2/test/_base/utils/paths.test.ts @@ -0,0 +1,107 @@ +/** + * 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('subtree watch filtering', () => { + 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 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(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('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('.'), + }); + expect(ignored('/repo/.agents')).toBe(false); + expect(ignored('/repo/.agents/skills')).toBe(false); + expect(ignored('/repo/.agents/skills/demo')).toBe(false); + }); + + it('keeps direct probes but prunes payload below a terminal bundle', () => { + const ignored = subtreeWatchFilter(root, candidates, { + scannedDirectories: ['/repo/.agents/skills'], + keepEntryFile: 'SKILL.md', + }); + expect(ignored('/repo/.agents/skills/demo')).toBe(false); + expect(ignored('/repo/.agents/skills/demo/SKILL.md')).toBe(false); + expect(ignored('/repo/.agents/skills/demo/runtime')).toBe(true); + expect(ignored('/repo/.agents/skills/demo/runtime/0.py')).toBe(true); + expect(ignored('/repo/.agents/skills/.flat.md')).toBe(false); + }); + + it('keeps direct bundle probes when the candidate root has not been scanned yet', () => { + const ignored = subtreeWatchFilter(root, candidates, { + scannedDirectories: [], + keepEntryFile: 'SKILL.md', + }); + expect(ignored('/repo/.agents/skills/new-skill')).toBe(false); + expect(ignored('/repo/.agents/skills/new-skill/SKILL.md')).toBe(false); + expect(ignored('/repo/.agents/skills/new-skill/runtime')).toBe(true); + }); + + it('keeps direct sub-skill probes below a directory the scanner traversed', () => { + const ignored = subtreeWatchFilter(root, candidates, { + scannedDirectories: [ + '/repo/.agents/skills', + '/repo/.agents/skills/parent', + ], + keepEntryFile: 'SKILL.md', + }); + expect(ignored('/repo/.agents/skills/parent/child')).toBe(false); + expect(ignored('/repo/.agents/skills/parent/child/SKILL.md')).toBe(false); + expect(ignored('/repo/.agents/skills/parent/child/runtime')).toBe(true); + }); +}); diff --git a/packages/agent-core-v2/test/app/plugin/manager-consumption.test.ts b/packages/agent-core-v2/test/app/plugin/manager-consumption.test.ts index 67b4591cd..fc1ec6691 100644 --- a/packages/agent-core-v2/test/app/plugin/manager-consumption.test.ts +++ b/packages/agent-core-v2/test/app/plugin/manager-consumption.test.ts @@ -251,6 +251,7 @@ describe('PluginManager consumption plane', () => { skills: [stubSkill('provided')], skipped: [], scannedRoots: [], + scannedDirectories: [], }), }); await manager.load(); diff --git a/packages/agent-core-v2/test/app/plugin/pluginService.test.ts b/packages/agent-core-v2/test/app/plugin/pluginService.test.ts index 173c8dae1..59eac597c 100644 --- a/packages/agent-core-v2/test/app/plugin/pluginService.test.ts +++ b/packages/agent-core-v2/test/app/plugin/pluginService.test.ts @@ -58,7 +58,12 @@ function makeHost( stubPair(IProviderService, providers), stubPair(ISkillDiscovery, { _serviceBrand: undefined, - discover: async () => ({ skills: [], skipped: [], scannedRoots: [] }), + discover: async () => ({ + skills: [], + skipped: [], + scannedRoots: [], + scannedDirectories: [], + }), } satisfies ISkillDiscovery), ]); } diff --git a/packages/agent-core-v2/test/app/skillCatalog/fileSkillDiscovery.test.ts b/packages/agent-core-v2/test/app/skillCatalog/fileSkillDiscovery.test.ts index e82d2be20..b768139fc 100644 --- a/packages/agent-core-v2/test/app/skillCatalog/fileSkillDiscovery.test.ts +++ b/packages/agent-core-v2/test/app/skillCatalog/fileSkillDiscovery.test.ts @@ -198,6 +198,23 @@ describe('FileSkillDiscovery', () => { expect(result.skills.find((s) => s.name === 'outer.child')?.metadata.isSubSkill).toBe(true); }); + it('reports only directories the scanner actually traverses', async () => { + await writeSkill('skills/terminal/SKILL.md', 'name: terminal\ndescription: terminal'); + await mkdir(join(root, 'skills', 'terminal', 'runtime', 'nested'), { recursive: true }); + await writeSkill( + 'skills/parent/SKILL.md', + 'name: parent\ndescription: parent\nhas-sub-skill: true', + ); + await writeSkill('skills/parent/child/SKILL.md', 'name: child\ndescription: child'); + + const result = await discover([skillRoot('skills')]); + + expect(result.scannedDirectories).toEqual([ + join(root, 'skills'), + join(root, 'skills', 'parent'), + ]); + }); + it('ignores node_modules and dot directories while walking', async () => { await writeSkill( 'skills/node_modules/hidden/SKILL.md', 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 b815c9bcf..95e9bb9d4 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,27 +1,138 @@ /** - * `hostFsWatch` domain — integration test against the real `chokidar` - * watcher on a temporary directory. + * 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 { join } from 'node:path'; -import { afterEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { + resetUnexpectedErrorHandler, + setUnexpectedErrorHandler, +} from '#/_base/errors/unexpectedError'; import { HostFsWatchService } from '#/os/backends/node-local/hostFsWatchService'; -import type { HostFsChange, IHostFsWatchHandle } from '#/os/interface/hostFsWatch'; +import type { + HostFsChange, + IHostFsWatchHandle, + IHostFsWatchService, +} from '#/os/interface/hostFsWatch'; const wait = (ms: number): Promise => new Promise((r) => setTimeout(r, ms)); -describe('HostFsWatchService', () => { +type HostFsWatchRuntime = NonNullable[0]>; + +class TestNativeWatcher { + private errorListener: ((error: NodeJS.ErrnoException) => void) | undefined; + closed = false; + + on(_event: 'error', listener: (error: NodeJS.ErrnoException) => void): this { + this.errorListener = listener; + return this; + } + + close(): void { + this.closed = true; + } + + 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 { @@ -29,10 +140,110 @@ describe('HostFsWatchService', () => { const svc = new HostFsWatchService(); handle = svc.watch(root, { recursive }); handle.onDidChange((e) => events.push(e)); - await wait(200); + await handle.ready; 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 handle.ready; + 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(); @@ -86,4 +297,23 @@ describe('HostFsWatchService', () => { expect(events).toHaveLength(0); }); + + 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/workspaceAgentProfileLoader/agentProfileLoader.test.ts b/packages/agent-core-v2/test/workspace/workspaceAgentProfileLoader/agentProfileLoader.test.ts index 0176a1130..4218b77de 100644 --- a/packages/agent-core-v2/test/workspace/workspaceAgentProfileLoader/agentProfileLoader.test.ts +++ b/packages/agent-core-v2/test/workspace/workspaceAgentProfileLoader/agentProfileLoader.test.ts @@ -113,6 +113,7 @@ function fsWatchStub(): IHostFsWatchService { return { _serviceBrand: undefined, watch: (): IHostFsWatchHandle => ({ + ready: Promise.resolve(), onDidChange: Event.None as Event, dispose: () => {}, }), diff --git a/packages/agent-core-v2/test/workspace/workspaceFs/fsWatchService.test.ts b/packages/agent-core-v2/test/workspace/workspaceFs/fsWatchService.test.ts index db461b278..0553515fc 100644 --- a/packages/agent-core-v2/test/workspace/workspaceFs/fsWatchService.test.ts +++ b/packages/agent-core-v2/test/workspace/workspaceFs/fsWatchService.test.ts @@ -68,6 +68,7 @@ function fakeHostFsWatch(): FakeWatch { let listener: ((e: HostFsChange) => void) | undefined; let disposedCount = 0; const handle: IHostFsWatchHandle = { + ready: Promise.resolve(), onDidChange: (l) => { listener = l; return { dispose: () => (listener = undefined) }; diff --git a/packages/agent-core-v2/test/workspace/workspaceInstructions/instructions.test.ts b/packages/agent-core-v2/test/workspace/workspaceInstructions/instructions.test.ts index e814cabcc..9627b96a7 100644 --- a/packages/agent-core-v2/test/workspace/workspaceInstructions/instructions.test.ts +++ b/packages/agent-core-v2/test/workspace/workspaceInstructions/instructions.test.ts @@ -73,7 +73,7 @@ describe('WorkspaceInstructionsService', () => { emitter = new Emitter(); watchFires.set(path, emitter); } - return { onDidChange: emitter.event, dispose: () => {} }; + return { ready: Promise.resolve(), onDidChange: emitter.event, dispose: () => {} }; }, }; } diff --git a/packages/agent-core-v2/test/workspace/workspaceMcp/initialization.test.ts b/packages/agent-core-v2/test/workspace/workspaceMcp/initialization.test.ts index 0e9cfa5e1..2ef798c38 100644 --- a/packages/agent-core-v2/test/workspace/workspaceMcp/initialization.test.ts +++ b/packages/agent-core-v2/test/workspace/workspaceMcp/initialization.test.ts @@ -92,6 +92,7 @@ describe('Workspace MCP initialization', () => { }); reg.definePartialInstance(IHostFsWatchService, { watch: (): IHostFsWatchHandle => ({ + ready: Promise.resolve(), onDidChange: Event.None as Event, dispose: () => {}, }), diff --git a/packages/agent-core-v2/test/workspace/workspaceMcpConfig/workspaceMcpConfig.test.ts b/packages/agent-core-v2/test/workspace/workspaceMcpConfig/workspaceMcpConfig.test.ts index 7ede8521c..303f1958a 100644 --- a/packages/agent-core-v2/test/workspace/workspaceMcpConfig/workspaceMcpConfig.test.ts +++ b/packages/agent-core-v2/test/workspace/workspaceMcpConfig/workspaceMcpConfig.test.ts @@ -91,7 +91,7 @@ describe('WorkspaceMcpConfigService', () => { emitter = new Emitter(); watchFires.set(path, emitter); } - return { onDidChange: emitter.event, dispose: () => {} }; + return { ready: Promise.resolve(), onDidChange: emitter.event, dispose: () => {} }; }, }; } 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 8a372a8bd..822a37e55 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,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. + * 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`. */ @@ -26,7 +26,12 @@ import { IPluginService } from '#/app/plugin/plugin'; import { PluginService } from '#/app/plugin/pluginService'; import type { ReloadSummary } from '#/app/plugin/types'; import { IProviderService } from '#/kosong/provider/provider'; -import { IHostFsWatchService, type HostFsChange, type IHostFsWatchHandle } from '#/os/interface/hostFsWatch'; +import { + IHostFsWatchService, + type HostFsChange, + type HostFsWatchOptions, + type IHostFsWatchHandle, +} from '#/os/interface/hostFsWatch'; import { IAppStateService } from '#/app/state/appState'; import { AppStateService } from '#/app/state/appStateService'; import { IWorkspaceStateService } from '#/workspace/state/workspaceState'; @@ -149,13 +154,19 @@ function workspaceContextStub(workDir: string): IWorkspaceContext { }; } -function fsWatchStub(): IHostFsWatchService { +function fsWatchStub( + onWatch?: (options: HostFsWatchOptions | undefined) => void, +): IHostFsWatchService { return { _serviceBrand: undefined, - watch: (): IHostFsWatchHandle => ({ - onDidChange: Event.None as Event, - dispose: () => {}, - }), + watch: (_path, options): IHostFsWatchHandle => { + onWatch?.(options); + return { + ready: Promise.resolve(), + onDidChange: Event.None as Event, + dispose: () => {}, + }; + }, }; } @@ -408,7 +419,7 @@ describe('WorkspaceSkillCatalogService', () => { calls = 0; async discover() { this.calls++; - return { skills: [], skipped: [], scannedRoots: [] }; + return { skills: [], skipped: [], scannedRoots: [], scannedDirectories: [] }; } } const store = new CountingDiscovery(); @@ -492,7 +503,7 @@ describe('WorkspaceSkillCatalogService', () => { const pluginSkills = roots .filter((root) => root.plugin !== undefined) .map((root) => stubSkill('demo-skill', { source: 'extra', plugin: root.plugin })); - return { skills: pluginSkills, skipped: [], scannedRoots: [] }; + return { skills: pluginSkills, skipped: [], scannedRoots: [], scannedDirectories: [] }; } } const store = new ExtraRootStore(); @@ -519,6 +530,7 @@ describe('WorkspaceSkillCatalogService', () => { scannedRoots: roots .filter((root) => root.source === 'project') .map((root) => root.path), + scannedDirectories: [], }; } } @@ -551,6 +563,7 @@ describe('WorkspaceSkillCatalogService', () => { skills: [], skipped: isProject ? [skippedEntry] : [], scannedRoots: [], + scannedDirectories: [], }; } } @@ -793,6 +806,99 @@ describe('WorkspaceSkillCatalogService', () => { await rm(homeDir, { recursive: true, force: true }); } }); + + it('keeps the old watch active until a ready replacement converges on post-scan changes', async () => { + const workDir = await mkdtemp(join(tmpdir(), 'skill-watch-handoff-')); + const skillRoot = join(workDir, '.agents', 'skills'); + await mkdir(join(skillRoot, 'demo'), { recursive: true }); + await writeFile( + join(skillRoot, 'demo', 'SKILL.md'), + '---\nname: demo\ndescription: initial\n---\nbody', + 'utf8', + ); + const scannedDirectory = await realpath(skillRoot); + const replacementReady = deferred(); + const replacementStarted = deferred(); + + class TestWatchHandle implements IHostFsWatchHandle { + readonly changes = new Emitter(); + readonly onDidChange = this.changes.event; + disposed = false; + + constructor(readonly ready: Promise) {} + + dispose(): void { + this.disposed = true; + this.changes.dispose(); + } + } + + const handles: TestWatchHandle[] = []; + const watchService: IHostFsWatchService = { + _serviceBrand: undefined, + watch: () => { + const handle = new TestWatchHandle( + handles.length === 0 ? Promise.resolve() : replacementReady.promise, + ); + handles.push(handle); + if (handles.length === 2) replacementStarted.resolve(undefined); + return handle; + }, + }; + let scans = 0; + const discovery: ISkillDiscovery = { + _serviceBrand: undefined, + discover: async () => { + scans += 1; + return { + skills: [stubSkill('demo', { description: scans === 1 ? 'stale' : 'fresh' })], + skipped: [], + scannedRoots: [scannedDirectory], + scannedDirectories: [scannedDirectory], + }; + }, + }; + + _clearScopedRegistryForTests(); + registerScopedService( + LifecycleScope.Workspace, + IWorkspaceRootSkillSource, + WorkspaceRootSkillSource, + ); + const host = createScopedTestHost([ + stubPair(ISkillDiscovery, discovery), + stubPair(IBootstrapService, bootstrapStub), + stubPair(IConfigService, configStub()), + stubPair(IHostFsWatchService, watchService), + ]); + const workspace = host.child(LifecycleScope.Workspace, 'w1', [ + stubPair(IWorkspaceContext, workspaceContextStub(workDir)), + ]); + + try { + const loading = workspace.accessor.get(IWorkspaceRootSkillSource).load(); + await replacementStarted.promise; + const initialHandle = handles[0]; + const replacementHandle = handles[1]; + if (initialHandle === undefined || replacementHandle === undefined) { + throw new Error('expected initial and replacement watch handles'); + } + + expect(initialHandle.disposed).toBe(false); + + replacementReady.resolve(undefined); + const contribution = await loading; + + expect(initialHandle.disposed).toBe(true); + expect(replacementHandle.disposed).toBe(false); + expect(scans).toBe(2); + expect(contribution.skills.map((skill) => skill.description)).toEqual(['fresh']); + } finally { + host.dispose(); + await rm(workDir, { recursive: true, force: true }); + } + }); + it('rescans the workspace-root source when a project skill file changes on disk', async () => { const workDir = await mkdtemp(join(tmpdir(), 'skill-watch-')); const host = createScopedTestHost([ @@ -835,4 +941,82 @@ describe('WorkspaceSkillCatalogService', () => { await rm(workDir, { recursive: true, force: true }); } }, 15000); + + it('prunes terminal skill payloads from the workspace watch after discovery', async () => { + const workDir = await mkdtemp(join(tmpdir(), 'skill-watch-plan-')); + const skillDir = join(workDir, '.agents', 'skills', 'demo'); + const runtimeFile = join(skillDir, 'runtime', '0.py'); + await mkdir(join(skillDir, 'runtime'), { recursive: true }); + await writeFile( + join(skillDir, 'SKILL.md'), + '---\nname: demo\ndescription: demo\n---\nbody', + 'utf8', + ); + await writeFile(runtimeFile, 'x', 'utf8'); + const watchedSkillDir = await realpath(skillDir); + const watchedRuntimeFile = await realpath(runtimeFile); + let ignored: ((path: string) => boolean) | undefined; + const host = createScopedTestHost([ + stubPair(IBootstrapService, bootstrapStub), + stubPair(IConfigService, configStub()), + stubPair(IPluginService, pluginStub()), + stubPair(ILogService, stubLog()), + stubPair(ISkillDiscovery, new FileSkillDiscovery(stubLog())), + stubPair( + IHostFsWatchService, + fsWatchStub((options) => { + ignored = options?.ignored; + }), + ), + ]); + const workspace = host.child(LifecycleScope.Workspace, 'w1', [ + stubPair(IWorkspaceContext, workspaceContextStub(workDir)), + ]); + + try { + const catalog = workspace.accessor.get(IWorkspaceSkillCatalog); + await catalog.load(); + + expect(ignored?.(join(watchedSkillDir, 'SKILL.md'))).toBe(false); + expect(ignored?.(watchedRuntimeFile)).toBe(true); + } finally { + host.dispose(); + await rm(workDir, { recursive: true, force: true }); + } + }); + + 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 }); + 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', + ); + + await refreshed; + expect(catalog.catalog.getSkill('dot-skill')?.description).toBe('under dot dir'); + } finally { + host.dispose(); + await rm(workDir, { recursive: true, force: true }); + } + }, 15000); });