Fix: make the AgentLoader preload memo the only record of a completed scan (stacked on #664)

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<void>;
   private readonly preloadedAgents: Record<string, AgentFile> = {};
   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<void> {
-    if (this.agentsAlreadyPreloaded) {
-      return;
-    }
+    // Dedupe concurrent scans; a second scan re-bundles every entrypoint.
+    const scan: Promise<void> = (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<void> {
@@ -521,8 +521,6 @@ export class AgentLoader {
       }),
     );
 
-    this.agentsAlreadyPreloaded = true;
-
     if (this.watchForChanges && !this.watcher) {
       this.startWatching();
     }

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<void>((resolve) => {
+        compileStarted = resolve;
+      });
+      let failCompile!: (error: Error) => void;
+      (esbuild.build as Mock).mockImplementationOnce(
+        () =>
+          new Promise<void>((_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();
+    });
   });
 });