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>

Author: Copilot

extensions/vscode/src/extension.ts

@@ -82,7 +82,6 @@ export async function activate(
     vscode.workspace.onDidChangeConfiguration((e) => {
       if (e.affectsConfiguration('codeql-mcp')) {
         logger.info('Configuration changed — requesting MCP server restart');
-        envBuilder.invalidate();
         mcpProvider.requestRestart();
       }
     }),

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

@@ -52,14 +52,16 @@ export class McpProvider
   /**
    * Request that VS Code restart the MCP server with a fresh environment.
    *
-   * 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.
+   * 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})...`,
@@ -108,7 +110,7 @@ export class McpProvider
    * returns `undefined` (the "latest" / unpinned case), the extension
    * version is used as the base instead.
    *
-   * After one or more `requestRestart()` calls, a `.N` revision suffix
+   * 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
@@ -121,6 +123,6 @@ export class McpProvider
     if (this._revision === 0) {
       return base;
     }
-    return `${base}.${this._revision}`;
+    return `${base}+r${this._revision}`;
   }
 }

extensions/vscode/test/extension.test.ts

@@ -175,8 +175,7 @@ describe('Extension', () => {
   it('should invalidate env cache on workspace folder changes', async () => {
     // Capture the workspace folder change callback
     let workspaceFolderChangeCallback: Function | undefined;
-    const vscodeMock = vi.mocked(vscode);
-    vscodeMock.workspace.onDidChangeWorkspaceFolders = vi.fn().mockImplementation(
+    const spy = vi.spyOn(vscode.workspace, 'onDidChangeWorkspaceFolders').mockImplementation(
       (cb: Function) => { workspaceFolderChangeCallback = cb; return { dispose: vi.fn() }; },
     );
 
@@ -202,5 +201,7 @@ describe('Extension', () => {
     expect(envBuilderInstance.invalidate).toHaveBeenCalledTimes(1);
     expect(mcpProviderInstance.fireDidChange).not.toHaveBeenCalled();
     expect(mcpProviderInstance.requestRestart).not.toHaveBeenCalled();
+
+    spy.mockRestore();
   });
 });

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

@@ -180,7 +180,7 @@ describe('McpProvider', () => {
     provider.requestRestart();
     const definitions = await provider.provideMcpServerDefinitions(token as any);
 
-    expect(definitions![0].version).toMatch(/^2\.20\.0\.\d+$/);
+    expect(definitions![0].version).toMatch(/^2\.20\.0\+r\d+$/);
   });
 
   it('should append revision to extension version after requestRestart when version is latest', async () => {
@@ -191,7 +191,7 @@ describe('McpProvider', () => {
     provider.requestRestart();
     const definitions = await provider.provideMcpServerDefinitions(token as any);
 
-    expect(definitions![0].version).toMatch(/^2\.25\.1\.\d+$/);
+    expect(definitions![0].version).toMatch(/^2\.25\.1\+r\d+$/);
   });
 
   it('should fire change event when requestRestart is called', () => {
@@ -203,6 +203,12 @@ describe('McpProvider', () => {
     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);