diff --git a/dev/src/utils/agent_loader.ts b/dev/src/utils/agent_loader.ts index 05687a199..e2d553841 100644 --- a/dev/src/utils/agent_loader.ts +++ b/dev/src/utils/agent_loader.ts @@ -358,7 +358,6 @@ export class AgentFile { * app/rootApp as instance of App. */ export class AgentLoader { - private agentsAlreadyPreloaded = false; private preloadInFlight?: Promise; private readonly preloadedAgents: Record = {}; private watcher?: fs.FSWatcher; @@ -434,7 +433,6 @@ export class AgentLoader { delete this.preloadedAgents[key]; } - this.agentsAlreadyPreloaded = false; // Detach any running scan so the invalidation is not swallowed by a // caller joining results that were gathered before the change. this.preloadInFlight = undefined; @@ -485,23 +483,25 @@ export class AgentLoader { /** * Discovers, compiles and imports every agent in the agents directory. * - * Callers that arrive while a scan is running join it. A rejected scan is - * discarded, so a later call retries from scratch. + * Callers that arrive while a scan is running join it, and the settled + * promise serves every later caller. A rejected scan is discarded, so a + * later call retries from scratch. */ async preloadAgents(): Promise { - if (this.agentsAlreadyPreloaded) { - return; - } + // Dedupe concurrent scans; a second scan re-bundles every entrypoint. + const scan: Promise = (this.preloadInFlight ??= + this.scanAgents().catch((e: unknown) => { + // Only the scan that is still current may drop the memo. A scan + // superseded by invalidateAll() would otherwise discard the results of + // the replacement that a later caller already started. + if (this.preloadInFlight === scan) { + this.preloadInFlight = undefined; + } - // A second concurrent scan re-bundles and re-imports every entrypoint, and - // its AgentFile instances overwrite the first scan's in `preloadedAgents`, - // so the displaced ones are never disposed and their temp directories leak. - this.preloadInFlight ??= this.scanAgents().catch((e: unknown) => { - this.preloadInFlight = undefined; - throw e; - }); + throw e; + })); - return this.preloadInFlight; + return scan; } private async scanAgents(): Promise { @@ -521,8 +521,6 @@ export class AgentLoader { }), ); - this.agentsAlreadyPreloaded = true; - if (this.watchForChanges && !this.watcher) { this.startWatching(); } diff --git a/dev/test/utils/agent_loader_test.ts b/dev/test/utils/agent_loader_test.ts index f1cbcebd2..7bcdec32e 100644 --- a/dev/test/utils/agent_loader_test.ts +++ b/dev/test/utils/agent_loader_test.ts @@ -768,29 +768,6 @@ describe('AgentLoader', () => { await loader.disposeAll(); }); - it('resets preload cache when invalidateAll is called (simulates file-change reload)', async () => { - const loader = new AgentLoader(tempAgentsDir); - - // Initial load should populate the cache and mark as preloaded - await loader.listAgents(); - expect( - (loader as unknown as {agentsAlreadyPreloaded: boolean}) - .agentsAlreadyPreloaded, - ).toBe(true); - - // Simulate what the fs.watch callback does when a file changes - (loader as unknown as {invalidateAll: () => void}).invalidateAll(); - - // After invalidation the preloaded flag is reset so that the next - // request triggers a full re-scan from disk - expect( - (loader as unknown as {agentsAlreadyPreloaded: boolean}) - .agentsAlreadyPreloaded, - ).toBe(false); - - await loader.disposeAll(); - }); - /** * The entrypoint each esbuild invocation compiled, in call order. * Comparing the list against its distinct entries counts duplicated @@ -802,6 +779,35 @@ describe('AgentLoader', () => { ); } + /** + * Invalidates the loader the way the fs.watch callback does. The loader + * keeps that entry point private and exposes no public trigger, so the + * cast lives here once instead of in every test that needs it. + */ + function invalidateAll(loader: AgentLoader): void { + (loader as unknown as {invalidateAll: () => void}).invalidateAll(); + } + + it('resets preload cache when invalidateAll is called (simulates file-change reload)', async () => { + const loader = new AgentLoader(tempAgentsDir); + + // Initial load should populate the cache + await loader.listAgents(); + const compiledAfterFirstLoad = compiledEntryPoints().length; + expect(compiledAfterFirstLoad).toBeGreaterThan(0); + + // Simulate what the fs.watch callback does when a file changes + invalidateAll(loader); + + // After invalidation the next request triggers a full re-scan from disk + await loader.listAgents(); + expect(compiledEntryPoints().length).toBeGreaterThan( + compiledAfterFirstLoad, + ); + + await loader.disposeAll(); + }); + it('runs a single discovery pass for concurrent preloadAgents() calls', async () => { const loader = new AgentLoader(tempAgentsDir); await Promise.all([loader.preloadAgents(), loader.preloadAgents()]); @@ -841,7 +847,7 @@ describe('AgentLoader', () => { const loader = new AgentLoader(tempAgentsDir); const invalidatedScan = loader.preloadAgents(); - (loader as unknown as {invalidateAll: () => void}).invalidateAll(); + invalidateAll(loader); await Promise.all([invalidatedScan, loader.preloadAgents()]); @@ -857,5 +863,60 @@ describe('AgentLoader', () => { await loader.disposeAll(); }); + + it('does not reuse a scan that was invalidated while it was in flight', async () => { + const loader = new AgentLoader(tempAgentsDir); + + const invalidatedScan = loader.preloadAgents(); + invalidateAll(loader); + await invalidatedScan; + + const compiledBeforeReload = compiledEntryPoints().length; + await loader.preloadAgents(); + + expect(compiledEntryPoints().length).toBeGreaterThan( + compiledBeforeReload, + ); + expect(await loader.listAgents()).toEqual(['agent1', 'agent2', 'agent3']); + + await loader.disposeAll(); + }); + + it('keeps the replacement memo when a superseded scan fails', async () => { + // The first compile is held open so the test decides when the scan that + // owns it fails, instead of racing the replacement scan's compiles. + let compileStarted!: () => void; + const compiling = new Promise((resolve) => { + compileStarted = resolve; + }); + let failCompile!: (error: Error) => void; + (esbuild.build as Mock).mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + failCompile = reject; + compileStarted(); + }), + ); + + const loader = new AgentLoader(tempAgentsDir); + const superseded = loader.preloadAgents(); + await compiling; + + invalidateAll(loader); + const replacement = loader.preloadAgents(); + await replacement; + + // The superseded scan fails while the replacement owns the memo. + failCompile(new Error('compile failed')); + await expect(superseded).rejects.toThrow('compile failed'); + + // The replacement is still memoized, so a later call does not scan again. + const compiledAfterReplacement = compiledEntryPoints().length; + await loader.preloadAgents(); + + expect(compiledEntryPoints().length).toBe(compiledAfterReplacement); + + await loader.disposeAll(); + }); }); });