diff --git a/.changeset/watcher-signature-dedupe.md b/.changeset/watcher-signature-dedupe.md new file mode 100644 index 000000000..febd7ecd1 --- /dev/null +++ b/.changeset/watcher-signature-dedupe.md @@ -0,0 +1,5 @@ +--- +"agent-bundle": patch +--- + +Prevent delayed duplicate filesystem events from triggering redundant development rebuilds when the watched path has not changed. diff --git a/packages/agent-bundle/src/dev/watcher.ts b/packages/agent-bundle/src/dev/watcher.ts index b04f02b44..17a1fcfe0 100644 --- a/packages/agent-bundle/src/dev/watcher.ts +++ b/packages/agent-bundle/src/dev/watcher.ts @@ -1,4 +1,5 @@ import chokidar from 'chokidar'; +import { stat } from 'node:fs/promises'; import { relative, resolve } from 'node:path'; import { freezeInvalidation, type Invalidation } from './types.ts'; @@ -19,17 +20,29 @@ export interface ProjectWatcherOptions { readonly onError?: (error: unknown) => void; readonly onInvalidation: (invalidation: Invalidation) => Promise; readonly outputPaths?: readonly string[]; + readonly readPathSignature?: (path: string) => Promise; readonly root: string; } const sourceEvents: readonly SourceWatchEvent[] = ['add', 'addDir', 'change', 'unlink', 'unlinkDir']; const excludedDirectoryNames = new Set(['.agent-bundle', '.git', 'node_modules']); +const deletedPathSignature = 'deleted'; const relativePath = (root: string, path: string): string | undefined => { const value = relative(root, resolve(root, path)).replaceAll('\\', '/'); return value === '..' || value.startsWith('../') ? undefined : value; }; +const defaultPathSignature = async (path: string): Promise => { + try { + const source = await stat(path, { bigint: true }); + return `${source.dev}:${source.ino}:${source.size}:${source.mtimeNs}`; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return undefined; + throw error; + } +}; + const defaultWatcher = ( root: string, options: Readonly<{ readonly ignored: (path: string) => boolean }>, @@ -62,12 +75,15 @@ export class ProjectWatcher { readonly #onInvalidation: (invalidation: Invalidation) => Promise; readonly #outputPaths = new Set(); readonly #paths = new Set(); + readonly #readPathSignature: (path: string) => Promise; readonly #ready: Promise; readonly #root: string; + readonly #signatures = new Map(); readonly #watcher: SourceWatcher; #closePromise: Promise | undefined; #closed = false; #delivery: Promise = Promise.resolve(); + #flushTail: Promise = Promise.resolve(); #timer: ReturnType | undefined; constructor(options: ProjectWatcherOptions) { @@ -79,6 +95,7 @@ export class ProjectWatcher { this.#now = options.now ?? (() => new Date()); this.#onError = options.onError; this.#onInvalidation = options.onInvalidation; + this.#readPathSignature = options.readPathSignature ?? defaultPathSignature; this.addOutputPaths([...(options.ignoredPaths ?? []), ...(options.outputPaths ?? ['dist'])]); this.#ignored = (path) => { const source = relativePath(this.#root, path); @@ -111,16 +128,35 @@ export class ProjectWatcher { } } - async flush(): Promise { - if (this.#closed) return; + flush(): Promise { + if (this.#closed) return Promise.resolve(); this.#clearTimer(); - if (this.#paths.size === 0) return; + if (this.#paths.size === 0) return this.#flushTail; + const paths = [...this.#paths].sort((left, right) => left.localeCompare(right)); + this.#paths.clear(); + const flush = this.#flushTail.then(async () => this.#flushPaths(paths)); + this.#flushTail = flush.catch(() => undefined); + return flush; + } + + async #flushPaths(paths: readonly string[]): Promise { + const signatures = await Promise.all(paths.map(async (path) => Object.freeze({ + path, + signature: await this.#readPathSignature(resolve(this.#root, path)), + }))); + const changedPaths: string[] = []; + for (const { path, signature } of signatures) { + const normalizedSignature = signature ?? deletedPathSignature; + if (this.#signatures.has(path) && this.#signatures.get(path) === normalizedSignature) continue; + changedPaths.push(path); + this.#signatures.set(path, normalizedSignature); + } + if (changedPaths.length === 0) return; const invalidation = freezeInvalidation({ occurredAt: this.#now().toISOString(), - paths: Object.freeze([...this.#paths].sort((left, right) => left.localeCompare(right))), + paths: Object.freeze(changedPaths), reason: 'source-change', }); - this.#paths.clear(); this.#delivery = Promise.resolve(this.#onInvalidation(invalidation)); await this.#delivery; } @@ -152,6 +188,7 @@ export class ProjectWatcher { } async #close(): Promise { + await this.#flushTail; await this.#delivery; await this.#watcher.close(); } diff --git a/packages/agent-bundle/tests/dev-watcher.test.ts b/packages/agent-bundle/tests/dev-watcher.test.ts index aa00de412..0dead382c 100644 --- a/packages/agent-bundle/tests/dev-watcher.test.ts +++ b/packages/agent-bundle/tests/dev-watcher.test.ts @@ -85,8 +85,54 @@ it('debounces only relevant source paths into one ordered invalidation and close expect(invalidations).toHaveLength(1); }); -// Flaky under full-pool load (chokidar create-event coalescing, #122); retry while the root cause stays open. -it('waits for the real watcher root before reporting create, change, and delete source inputs', { retry: 2 }, async () => { +it('drops delayed source events until the path signature changes', async () => { + const fake = new FakeWatcher(); + const invalidations: Invalidation[] = []; + const source = '/project/src/input.ts'; + let signature: string | undefined = 'revision-1'; + const watcher = new ProjectWatcher({ + createWatcher: () => fake, + debounceMs: 60_000, + onInvalidation: async (invalidation) => { + invalidations.push(invalidation); + }, + readPathSignature: async () => signature, + root: '/project', + }); + + fake.emit('add', source); + await watcher.flush(); + fake.emit('change', source); + await watcher.flush(); + + expect(invalidations).toHaveLength(1); + + signature = 'revision-2'; + fake.emit('change', source); + await watcher.flush(); + + expect(invalidations).toEqual([ + expect.objectContaining({ paths: ['src/input.ts'] }), + expect.objectContaining({ paths: ['src/input.ts'] }), + ]); + + signature = undefined; + fake.emit('unlink', source); + await watcher.flush(); + fake.emit('unlink', source); + await watcher.flush(); + + expect(invalidations).toHaveLength(3); + + signature = 'revision-1'; + fake.emit('add', source); + await watcher.flush(); + + expect(invalidations).toHaveLength(4); + await watcher.close(); +}); + +it('waits for the real watcher root before reporting create, change, and delete source inputs', async () => { const root = await mkdtemp(join(tmpdir(), 'agent-bundle-real-watcher-')); await mkdir(join(root, 'src'), { recursive: true }); await writeFile(join(root, 'src', 'existing.ts'), 'export const value = 1;\n'); @@ -125,8 +171,15 @@ it('waits for the real watcher root before reporting create, change, and delete mkdir(join(root, 'node_modules', 'dependency'), { recursive: true }).then(async () => writeFile(join(root, 'node_modules', 'dependency', 'index.js'), 'ignored\n')), ]); - await new Promise((resolvePromise) => setTimeout(resolvePromise, 100)); - expect(received).toHaveLength(3); + const sentinel = nextInvalidation((listener) => { listen = listener; }); + await writeFile(join(root, 'src', 'sentinel.ts'), 'export const sentinel = true;\n'); + expect((await sentinel).paths).toContain('src/sentinel.ts'); + + const reportedPaths = received.flatMap((invalidation) => invalidation.paths); + expect(reportedPaths).not.toContain('.agent-bundle/output/generated.ts'); + expect(reportedPaths).not.toContain('.git/HEAD'); + expect(reportedPaths).not.toContain('node_modules/dependency/index.js'); + expect(received).toHaveLength(4); } finally { await watcher.close(); await rm(root, { force: true, recursive: true }); diff --git a/packages/workbench/tests/examples-real.e2e.test.ts b/packages/workbench/tests/examples-real.e2e.test.ts index c87d17343..4a9d87273 100644 --- a/packages/workbench/tests/examples-real.e2e.test.ts +++ b/packages/workbench/tests/examples-real.e2e.test.ts @@ -108,6 +108,8 @@ e2e('drives the populated Skills Starter in real Chrome', { timeout: 90_000 }, a } }); +// #122's delayed duplicate event is signature-gated; this retry still covers the first +// Chokidar event racing the immediate manual rebuild after each source write. e2e('reveals, retains, repairs, and removes capabilities without reloading Chrome', { retry: 2, timeout: 120_000 }, async ({ page }) => { await buildWorkbench(); const project = await copyExample('skills-starter'); @@ -177,6 +179,8 @@ e2e('reveals, retains, repairs, and removes capabilities without reloading Chrom } }); +// #122's delayed duplicate event is signature-gated; this retry still covers the first +// Chokidar event racing the immediate manual rebuild after each source write. e2e('drives Hooks, scripts, logs, diagnostics, and repair in real Chrome', { retry: 2, timeout: 150_000 }, async ({ page }) => { await buildWorkbench(); const project = await copyExample('hooks-and-scripts'); @@ -571,6 +575,8 @@ e2e('drives every populated MCP App workflow surface in real Chrome', { timeout: } }); +// #122's delayed duplicate event is signature-gated; this retry still covers the first +// Chokidar event racing the immediate manual rebuild after each staged source replacement. e2e('renders the flagship compiled route catalog by server and kind in real Chrome', { retry: 2, timeout: 150_000 }, async ({ page }) => { await buildWorkbench(); const project = await copyExample('audiobook-curator');