fix: ql-mcp server must handle vscode workspace folder changes (#196)

* fix: restart MCP server on vscode workspace folder changes

When workspace folders were added or removed, fireDidChange() notified
VS Code that the MCP server definitions changed, causing it to stop the
running server. However, because the returned McpStdioServerDefinition
had the same version as before, VS Code did not restart the server.

Add a monotonically increasing revision counter to McpProvider that
increments on each fireDidChange() call and is appended to the version
string (e.g. 2.25.1.1, 2.25.1.2). This signals VS Code that the
definition has genuinely changed and triggers a server restart with
the updated environment instead of only stopping the server.

- Add _revision counter and getEffectiveVersion() to McpProvider
- Add 4 unit tests for version-bumping behaviour
- Add integration test suite for workspace folder change scenarios

* SqliteStore backend + annotation, audit, and query result cache tools (#169)

* fix: handle workspace folder changes in ql-mcp extension

- Invalidate cached environment when workspace folders change so the
  next server start picks up updated folder list
- getEffectiveVersion() always returns a concrete string (never undefined)
  so VS Code has a reliable baseline for version comparison
- Add multi-root workspace integration tests verifying environment
  correctness (CODEQL_MCP_WORKSPACE_FOLDERS includes/excludes folders)
- Add unit test for workspace folder change handler behavior
- Add unit test for always-defined version string
- Add multiRoot test profile with 4 workspace folders

* fix: remove machine-specific temp folder from workspace fixture

Agent-Logs-Url: https://github.com/advanced-security/codeql-development-mcp-server/sessions/bc6e5960-6b48-4be3-addb-49a4f14b6efa

Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>

* fix: apply PR review feedback - env invalidation, +rN version format, vi.spyOn cleanup

Agent-Logs-Url: https://github.com/advanced-security/codeql-development-mcp-server/sessions/b3b84dce-0b52-4c7d-9391-014500df1b9a

Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>

* Update extensions/vscode/test/suite/workspace-folder-change.integration.test.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Nathan Randall <70299490+data-douser@users.noreply.github.com>

* Update extensions/vscode/test/suite/workspace-folder-change.integration.test.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Nathan Randall <70299490+data-douser@users.noreply.github.com>

* fix: remove duplicate assertion, add try/finally for spy, cache extension version

Agent-Logs-Url: https://github.com/advanced-security/codeql-development-mcp-server/sessions/28160745-507f-46aa-b350-6fd6d19315ba

Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>

* fix: populate env cache before folder add in test; re-instantiate provider after mock change

Agent-Logs-Url: https://github.com/advanced-security/codeql-development-mcp-server/sessions/a42c54f5-5445-42e7-bad8-f86e1aa717f7

Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>

---------

Signed-off-by: Nathan Randall <70299490+data-douser@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: 3 people

extensions/vscode/esbuild.config.js

@@ -41,6 +41,7 @@ const testSuiteConfig = {
     'test/suite/mcp-resource-e2e.integration.test.ts',
     'test/suite/mcp-server.integration.test.ts',
     'test/suite/mcp-tool-e2e.integration.test.ts',
+    'test/suite/workspace-folder-change.integration.test.ts',
     'test/suite/workspace-scenario.integration.test.ts',
   ],
   outdir: 'dist/test/suite',

extensions/vscode/src/extension.ts

@@ -81,19 +81,20 @@ export async function activate(
   context.subscriptions.push(
     vscode.workspace.onDidChangeConfiguration((e) => {
       if (e.affectsConfiguration('codeql-mcp')) {
-        logger.info('Configuration changed — rebuilding environment');
-        envBuilder.invalidate();
-        mcpProvider.fireDidChange();
+        logger.info('Configuration changed — requesting MCP server restart');
+        mcpProvider.requestRestart();
       }
     }),
   );
 
-  // Re-scan when workspace folders change
+  // Invalidate cached environment when workspace folders change.
+  // VS Code itself manages MCP server lifecycle when roots change
+  // (stopping and restarting the server as needed).  We just clear
+  // the cached env so the next server start picks up updated folders.
   context.subscriptions.push(
     vscode.workspace.onDidChangeWorkspaceFolders(() => {
-      logger.info('Workspace folders changed — refreshing watchers');
+      logger.info('Workspace folders changed — invalidating environment cache');
       envBuilder.invalidate();
-      mcpProvider.fireDidChange();
     }),
   );
 

extensions/vscode/src/server/mcp-provider.ts

@@ -20,27 +20,69 @@ export class McpProvider
   private readonly _onDidChange = new vscode.EventEmitter<void>();
   readonly onDidChangeMcpServerDefinitions = this._onDidChange.event;
 
+  /**
+   * Monotonically increasing counter, bumped by `requestRestart()`.
+   * Appended to the version string so that VS Code sees a genuinely new
+   * server definition and triggers a stop → restart cycle.
+   */
+  private _revision = 0;
+
+  /**
+   * Cached extension version to avoid repeated synchronous `readFileSync`
+   * calls on every `getEffectiveVersion()` invocation.
+   */
+  private readonly _extensionVersion: string;
+
   constructor(
     private readonly serverManager: ServerManager,
     private readonly envBuilder: EnvironmentBuilder,
     private readonly logger: Logger,
   ) {
     super();
+    this._extensionVersion = serverManager.getExtensionVersion();
     this.push(this._onDidChange);
   }
 
-  /** Notify VS Code that the MCP server definitions have changed. */
+  /**
+   * Soft notification: tell VS Code that definitions may have changed.
+   *
+   * Does NOT bump the version, so VS Code will re-query
+   * `provideMcpServerDefinitions()` / `resolveMcpServerDefinition()` but
+   * will NOT restart the server. Use for lightweight updates (file watcher
+   * events, extension changes, background install completion) where the
+   * running server can continue with its current environment.
+   */
   fireDidChange(): void {
     this._onDidChange.fire();
   }
 
+  /**
+   * Request that VS Code restart the MCP server with a fresh environment.
+   *
+   * Invalidates the cached environment and bumps the internal revision counter
+   * so that the next call to `provideMcpServerDefinitions()` returns a
+   * definition with a different `version` string. VS Code compares the new
+   * version to the running server's version and, seeing a change, triggers a
+   * stop → start cycle.
+   *
+   * Use for changes that require a server restart (configuration changes).
+   */
+  requestRestart(): void {
+    this.envBuilder.invalidate();
+    this._revision++;
+    this.logger.info(
+      `Requesting ql-mcp restart (revision ${this._revision})...`,
+    );
+    this._onDidChange.fire();
+  }
+
   async provideMcpServerDefinitions(
     _token: vscode.CancellationToken,
   ): Promise<vscode.McpStdioServerDefinition[]> {
     const command = this.serverManager.getCommand();
     const args = this.serverManager.getArgs();
     const env = await this.envBuilder.build();
-    const version = this.serverManager.getVersion();
+    const version = this.getEffectiveVersion();
 
     this.logger.info(
       `Providing MCP server definition: ${command} ${args.join(' ')}`,
@@ -66,4 +108,26 @@ export class McpProvider
     server.env = env;
     return server;
   }
+
+  /**
+   * Computes the version string for the MCP server definition.
+   *
+   * Always returns a concrete string so that VS Code has a reliable
+   * baseline for version comparison.  When `serverManager.getVersion()`
+   * returns `undefined` (the "latest" / unpinned case), the extension
+   * version is used as the base instead.
+   *
+   * After one or more `requestRestart()` calls, a `+rN` revision suffix
+   * is appended so that the version is always different from the
+   * previous one.  VS Code uses the version to decide whether to
+   * restart a running server: a changed version triggers a stop → start
+   * cycle.
+   */
+  private getEffectiveVersion(): string {
+    const base = this.serverManager.getVersion() ?? this._extensionVersion;
+    if (this._revision === 0) {
+      return base;
+    }
+    return `${base}+r${this._revision}`;
+  }
 }

extensions/vscode/test/extension.test.ts

@@ -50,6 +50,7 @@ vi.mock('../src/server/mcp-provider', () => ({
       resolveMcpServerDefinition: vi.fn().mockResolvedValue(undefined),
       onDidChangeMcpServerDefinitions: vi.fn(),
       fireDidChange: vi.fn(),
+      requestRestart: vi.fn(),
       dispose: vi.fn(),
     };
   }),
@@ -102,6 +103,8 @@ vi.mock('../src/bridge/environment-builder', () => ({
 
 import { activate, deactivate } from '../src/extension';
 import * as vscode from 'vscode';
+import { McpProvider } from '../src/server/mcp-provider';
+import { EnvironmentBuilder } from '../src/bridge/environment-builder';
 
 function createMockContext(): vscode.ExtensionContext {
   return {
@@ -168,4 +171,39 @@ describe('Extension', () => {
   it('should deactivate even if activate was never called', () => {
     expect(() => deactivate()).not.toThrow();
   });
+
+  it('should invalidate env cache on workspace folder changes', async () => {
+    // Capture the workspace folder change callback
+    let workspaceFolderChangeCallback: Function | undefined;
+    const spy = vi.spyOn(vscode.workspace, 'onDidChangeWorkspaceFolders').mockImplementation(
+      (cb: Function) => { workspaceFolderChangeCallback = cb; return { dispose: vi.fn() }; },
+    );
+
+    await activate(ctx);
+
+    // The extension should have registered a workspace folder change handler
+    expect(workspaceFolderChangeCallback).toBeDefined();
+
+    // Get the mock instances created during activation
+    const mcpProviderInstance = vi.mocked(McpProvider).mock.results[0]?.value;
+    const envBuilderInstance = vi.mocked(EnvironmentBuilder).mock.results[0]?.value;
+
+    // Reset call counts from activation
+    mcpProviderInstance.fireDidChange.mockClear();
+    mcpProviderInstance.requestRestart.mockClear();
+    envBuilderInstance.invalidate.mockClear();
+
+    try {
+      // Simulate workspace folder change
+      workspaceFolderChangeCallback!();
+
+      // Cache invalidated immediately; no VS Code notification.
+      // VS Code manages MCP server lifecycle (stop/restart) when roots change.
+      expect(envBuilderInstance.invalidate).toHaveBeenCalledTimes(1);
+      expect(mcpProviderInstance.fireDidChange).not.toHaveBeenCalled();
+      expect(mcpProviderInstance.requestRestart).not.toHaveBeenCalled();
+    } finally {
+      spy.mockRestore();
+    }
+  });
 });

extensions/vscode/test/fixtures/multi-root-workspace/folder-c/.gitkeep

@@ -1,6 +1,16 @@
 {
 	"folders": [
-		{ "path": "folder-a" },
-		{ "path": "folder-b" }
+		{
+			"path": "folder-a"
+		},
+		{
+			"path": "folder-b"
+		},
+		{
+			"path": "folder-c"
+		},
+		{
+			"path": "folder-d"
+		}
 	]
 }

extensions/vscode/test/fixtures/multi-root-workspace/folder-d/.gitkeep

@@ -7,6 +7,7 @@ function createMockServerManager() {
   return {
     getCommand: vi.fn().mockReturnValue('npx'),
     getArgs: vi.fn().mockReturnValue(['-y', 'codeql-development-mcp-server']),
+    getExtensionVersion: vi.fn().mockReturnValue('2.25.1'),
     getVersion: vi.fn().mockReturnValue(undefined),
     getDescription: vi.fn().mockReturnValue('npx -y codeql-development-mcp-server'),
     getInstallDir: vi.fn().mockReturnValue('/mock/install'),
@@ -111,6 +112,26 @@ describe('McpProvider', () => {
     expect(definitions).toHaveLength(1);
   });
 
+  it('should always provide a defined version string even when serverVersion is latest', async () => {
+    // When serverManager.getVersion() returns undefined ("latest" mode),
+    // the definition must still carry a concrete version string so that
+    // VS Code has a baseline for version comparison. An undefined initial
+    // version prevents VS Code from detecting changes after requestRestart().
+    //
+    // NOTE: McpProvider caches getExtensionVersion() in its constructor, so the
+    // mock must be configured before constructing the provider.
+    serverManager.getVersion.mockReturnValue(undefined);
+    serverManager.getExtensionVersion.mockReturnValue('2.25.1');
+    provider = new McpProvider(serverManager, envBuilder, logger);
+
+    const token = { isCancellationRequested: false, onCancellationRequested: vi.fn() };
+    const definitions = await provider.provideMcpServerDefinitions(token as any);
+
+    expect(definitions![0].version).toBeDefined();
+    expect(typeof definitions![0].version).toBe('string');
+    expect(definitions![0].version!.length).toBeGreaterThan(0);
+  });
+
   it('should pass version from serverManager when pinned', async () => {
     serverManager.getVersion.mockReturnValue('2.20.0');
 
@@ -120,6 +141,78 @@ describe('McpProvider', () => {
     expect(definitions![0].version).toBe('2.20.0');
   });
 
+  it('should not change version on fireDidChange (soft signal)', async () => {
+    const token = { isCancellationRequested: false, onCancellationRequested: vi.fn() };
+    const before = await provider.provideMcpServerDefinitions(token as any);
+    const versionBefore = before![0].version;
+
+    provider.fireDidChange();
+    const after = await provider.provideMcpServerDefinitions(token as any);
+
+    expect(after![0].version).toBe(versionBefore);
+  });
+
+  it('should produce a different version after requestRestart', async () => {
+    const token = { isCancellationRequested: false, onCancellationRequested: vi.fn() };
+    const before = await provider.provideMcpServerDefinitions(token as any);
+    const versionBefore = before![0].version;
+
+    provider.requestRestart();
+    const after = await provider.provideMcpServerDefinitions(token as any);
+    const versionAfter = after![0].version;
+
+    expect(versionAfter).toBeDefined();
+    expect(versionAfter).not.toBe(versionBefore);
+  });
+
+  it('should produce distinct versions after successive requestRestart calls', async () => {
+    const token = { isCancellationRequested: false, onCancellationRequested: vi.fn() };
+
+    provider.requestRestart();
+    const defs1 = await provider.provideMcpServerDefinitions(token as any);
+
+    provider.requestRestart();
+    const defs2 = await provider.provideMcpServerDefinitions(token as any);
+
+    expect(defs1![0].version).not.toBe(defs2![0].version);
+  });
+
+  it('should append revision to pinned version after requestRestart', async () => {
+    serverManager.getVersion.mockReturnValue('2.20.0');
+    const token = { isCancellationRequested: false, onCancellationRequested: vi.fn() };
+
+    provider.requestRestart();
+    const definitions = await provider.provideMcpServerDefinitions(token as any);
+
+    expect(definitions![0].version).toMatch(/^2\.20\.0\+r\d+$/);
+  });
+
+  it('should append revision to extension version after requestRestart when version is latest', async () => {
+    serverManager.getVersion.mockReturnValue(undefined);
+    serverManager.getExtensionVersion.mockReturnValue('2.25.1');
+    const token = { isCancellationRequested: false, onCancellationRequested: vi.fn() };
+
+    provider.requestRestart();
+    const definitions = await provider.provideMcpServerDefinitions(token as any);
+
+    expect(definitions![0].version).toMatch(/^2\.25\.1\+r\d+$/);
+  });
+
+  it('should fire change event when requestRestart is called', () => {
+    const listener = vi.fn();
+    provider.onDidChangeMcpServerDefinitions(listener);
+
+    provider.requestRestart();
+
+    expect(listener).toHaveBeenCalledTimes(1);
+  });
+
+  it('should invalidate env cache when requestRestart is called', () => {
+    provider.requestRestart();
+
+    expect(envBuilder.invalidate).toHaveBeenCalledTimes(1);
+  });
+
   it('should resolve definition by refreshing environment', async () => {
     const updatedEnv = { CODEQL_PATH: '/new/path', TRANSPORT_MODE: 'stdio' };
     envBuilder.build.mockResolvedValue(updatedEnv);