chore: add orphan exports cleanup

Author: himanshusinghs

src/common/exportsManager.ts

@@ -58,27 +58,23 @@ type AvailableExport = Pick<StoredExport, "exportName" | "exportURI" | "exportPa
 export type ExportsManagerConfig = Pick<UserConfig, "exportsPath" | "exportTimeoutMs" | "exportCleanupIntervalMs">;
 
 type ExportsManagerEvents = {
+    initialized: [];
     "export-expired": [string];
     "export-available": [string];
 };
 
 export class ExportsManager extends EventEmitter<ExportsManagerEvents> {
+    private wasInitialized: boolean = false;
     private storedExports: Record<StoredExport["exportName"], StoredExport> = {};
     private exportsCleanupInProgress: boolean = false;
-    private exportsCleanupInterval: NodeJS.Timeout;
-    private exportsDirectoryPath: string;
+    private exportsCleanupInterval?: NodeJS.Timeout;
 
-    constructor(
-        sessionId: string,
+    private constructor(
+        private readonly exportsDirectoryPath: string,
         private readonly config: ExportsManagerConfig,
         private readonly logger: LoggerBase
     ) {
         super();
-        this.exportsDirectoryPath = path.join(this.config.exportsPath, sessionId);
-        this.exportsCleanupInterval = setInterval(
-            () => void this.cleanupExpiredExports(),
-            this.config.exportCleanupIntervalMs
-        );
     }
 
     public get availableExports(): AvailableExport[] {
@@ -96,6 +92,24 @@ export class ExportsManager extends EventEmitter<ExportsManagerEvents> {
             }));
     }
 
+    protected async init(): Promise<void> {
+        if (!this.wasInitialized) {
+            // If a server is killed, the exports from the active session on
+            // that server won't be cleaned up. We tackle that by attempting to
+            // clean any directory on the exportsPath when the server starts and
+            // initializes the ExportsManager.
+            await this.cleanupOrphanExportsDirectories();
+
+            this.exportsCleanupInterval = setInterval(
+                () => void this.cleanupExpiredExports(),
+                this.config.exportCleanupIntervalMs
+            );
+
+            this.wasInitialized = true;
+            this.emit("initialized");
+        }
+    }
+
     public async close(): Promise<void> {
         try {
             clearInterval(this.exportsCleanupInterval);
@@ -302,6 +316,25 @@ export class ExportsManager extends EventEmitter<ExportsManagerEvents> {
             }
         }
     }
+
+    private async cleanupOrphanExportsDirectories(): Promise<void> {
+        try {
+            await fs.rm(this.exportsDirectoryPath, { force: true, recursive: true });
+        } catch (error) {
+            this.logger.error({
+                id: LogId.exportInitError,
+                context: "Error while cleaning orphan exports",
+                message: error instanceof Error ? error.message : String(error),
+            });
+        }
+    }
+
+    static init(sessionId: string, config: ExportsManagerConfig, logger: LoggerBase): ExportsManager {
+        const exportsDirectoryPath = path.join(config.exportsPath, sessionId);
+        const exportsManager = new ExportsManager(exportsDirectoryPath, config, logger);
+        void exportsManager.init();
+        return exportsManager;
+    }
 }
 
 /**

src/common/logger.ts

@@ -54,9 +54,10 @@ export const LogId = {
     exportCreationError: mongoLogId(1_007_002),
     exportCreationCleanupError: mongoLogId(1_007_003),
     exportReadError: mongoLogId(1_007_004),
-    exportCloseError: mongoLogId(1_007_005),
-    exportedDataListError: mongoLogId(1_007_006),
-    exportedDataAutoCompleteError: mongoLogId(1_007_007),
+    exportInitError: mongoLogId(1_007_005),
+    exportCloseError: mongoLogId(1_007_006),
+    exportedDataListError: mongoLogId(1_007_007),
+    exportedDataAutoCompleteError: mongoLogId(1_007_008),
 } as const;
 
 interface LogPayload {

src/transports/base.ts

@@ -44,7 +44,7 @@ export abstract class TransportRunnerBase {
 
         const logger = new CompositeLogger(...loggers);
         const sessionId = new ObjectId().toString();
-        const exportsManager = new ExportsManager(sessionId, userConfig, logger);
+        const exportsManager = ExportsManager.init(sessionId, userConfig, logger);
         const connectionManager = new ConnectionManager();
 
         const session = new Session({

tests/integration/helpers.ts

@@ -10,8 +10,8 @@ import { Telemetry } from "../../src/telemetry/telemetry.js";
 import { config } from "../../src/common/config.js";
 import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
 import { ConnectionManager } from "../../src/common/connectionManager.js";
-import { CompositeLogger } from "../../src/common/logger.js";
-import { ExportsManager } from "../../src/common/exportsManager.js";
+import { CompositeLogger, LoggerBase } from "../../src/common/logger.js";
+import { ExportsManager, ExportsManagerConfig } from "../../src/common/exportsManager.js";
 
 interface ParameterInfo {
     name: string;
@@ -59,7 +59,7 @@ export function setupIntegrationTest(getUserConfig: () => UserConfig): Integrati
 
         const logger = new CompositeLogger();
         const sessionId = new ObjectId().toString();
-        const exportsManager = new ExportsManager(sessionId, userConfig, logger);
+        const exportsManager = await exportsManagerForTests(sessionId, userConfig, logger);
         const connectionManager = new ConnectionManager();
 
         const session = new Session({
@@ -294,3 +294,14 @@ export function resourceChangedNotification(client: Client, uri: string): Promis
         });
     });
 }
+
+export async function exportsManagerForTests(
+    sessionId: string,
+    exportsConfig: ExportsManagerConfig,
+    logger: LoggerBase
+): Promise<ExportsManager> {
+    return new Promise((resolve) => {
+        const manager = ExportsManager.init(sessionId, exportsConfig, logger);
+        manager.once("initialized", () => resolve(manager));
+    });
+}

tests/integration/resources/exportedData.test.ts

@@ -1,9 +1,19 @@
+import path from "path";
+import fs from "fs/promises";
 import { Long } from "bson";
-import { describe, expect, it, beforeEach } from "vitest";
+import { describe, expect, it, beforeEach, afterAll } from "vitest";
 import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
 import { defaultTestConfig, resourceChangedNotification, timeout } from "../helpers.js";
 import { describeWithMongoDB } from "../tools/mongodb/mongodbHelpers.js";
 import { contentWithResourceURILink } from "../tools/mongodb/read/export.test.js";
+import { UserConfig } from "../../../src/lib.js";
+
+const userConfig: UserConfig = {
+    ...defaultTestConfig,
+    exportsPath: path.join(path.dirname(defaultTestConfig.exportsPath), `exports-${Date.now()}`),
+    exportTimeoutMs: 200,
+    exportCleanupIntervalMs: 300,
+};
 
 describeWithMongoDB(
     "exported-data resource",
@@ -19,6 +29,10 @@ describeWithMongoDB(
                 ]);
         });
 
+        afterAll(async () => {
+            await fs.rm(userConfig.exportsPath, { recursive: true, force: true });
+        });
+
         it("should be able to list resource template", async () => {
             await integration.connectMcpClient();
             const response = await integration.mcpClient().listResourceTemplates();
@@ -123,11 +137,5 @@ describeWithMongoDB(
             });
         });
     },
-    () => {
-        return {
-            ...defaultTestConfig,
-            exportTimeoutMs: 200,
-            exportCleanupIntervalMs: 300,
-        };
-    }
+    () => userConfig
 );

tests/integration/telemetry.test.ts

@@ -6,7 +6,7 @@ import nodeMachineId from "node-machine-id";
 import { describe, expect, it } from "vitest";
 import { CompositeLogger } from "../../src/common/logger.js";
 import { ConnectionManager } from "../../src/common/connectionManager.js";
-import { ExportsManager } from "../../src/common/exportsManager.js";
+import { exportsManagerForTests } from "./helpers.js";
 
 describe("Telemetry", () => {
     it("should resolve the actual machine ID", async () => {
@@ -20,7 +20,7 @@ describe("Telemetry", () => {
                 apiBaseUrl: "",
                 logger: new CompositeLogger(),
                 sessionId: "1FOO",
-                exportsManager: new ExportsManager("1FOO", config, logger),
+                exportsManager: await exportsManagerForTests("1FOO", config, logger),
                 connectionManager: new ConnectionManager(),
             }),
             config