Design 3 TDD: add cache lookup tests for cacheKey/queryName/language/limit; fix env-builder to not overwrite pre-existing vars

TDD tests for Design 3 (query_results_cache_lookup):
- Exact cacheKey lookup returns metadata including resultCount
- Non-existent cacheKey returns cached:false
- Language-only filter correctly returns matching entries
- databasePath filter returns matching entries
- limit parameter restricts returned entries

Environment builder fix (addresses PR review):
- Only set ENABLE_ANNOTATION_TOOLS when not already defined in env
- Only set MONITORING_STORAGE_LOCATION when not already defined in env
- Added test: additionalEnv MONITORING_STORAGE_LOCATION override is preserved

Agent-Logs-Url: https://github.com/advanced-security/codeql-development-mcp-server/sessions/97593c7f-7fd2-48f4-9f4c-0e97dfa47902

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

Author: Copilot

extensions/vscode/src/bridge/environment-builder.ts

@@ -151,9 +151,17 @@ export class EnvironmentBuilder extends DisposableObject {
     // The setting controls ENABLE_ANNOTATION_TOOLS and defaults
     // MONITORING_STORAGE_LOCATION to the scratch directory so tools work
     // out-of-the-box without manual env var configuration.
+    // Only set these when not already defined, to avoid overriding values
+    // inherited from a parent process or earlier env setup.
     const enableAnnotations = config.get<boolean>('enableAnnotationTools', true);
-    env.ENABLE_ANNOTATION_TOOLS = enableAnnotations ? 'true' : 'false';
-    if (enableAnnotations && env.CODEQL_MCP_SCRATCH_DIR) {
+    if (!('ENABLE_ANNOTATION_TOOLS' in env)) {
+      env.ENABLE_ANNOTATION_TOOLS = enableAnnotations ? 'true' : 'false';
+    }
+    if (
+      enableAnnotations &&
+      env.CODEQL_MCP_SCRATCH_DIR &&
+      !('MONITORING_STORAGE_LOCATION' in env)
+    ) {
       env.MONITORING_STORAGE_LOCATION = env.CODEQL_MCP_SCRATCH_DIR;
     }
 

extensions/vscode/test/bridge/environment-builder.test.ts

@@ -278,6 +278,39 @@ describe('EnvironmentBuilder', () => {
     expect(env.ENABLE_ANNOTATION_TOOLS).toBe('true');
   });
 
+  it('should not overwrite MONITORING_STORAGE_LOCATION if already set in parent env', async () => {
+    const vscode = await import('vscode');
+    const origFolders = vscode.workspace.workspaceFolders;
+    const originalGetConfig = vscode.workspace.getConfiguration;
+
+    try {
+      (vscode.workspace.workspaceFolders as any) = [
+        { uri: { fsPath: '/mock/workspace' }, name: 'ws', index: 0 },
+      ];
+      // Simulate parent process env with MONITORING_STORAGE_LOCATION already set
+      vscode.workspace.getConfiguration = () => ({
+        get: (_key: string, defaultVal?: any) => {
+          if (_key === 'additionalEnv') return { MONITORING_STORAGE_LOCATION: '/custom/storage/path' };
+          if (_key === 'additionalDatabaseDirs') return [];
+          if (_key === 'additionalQueryRunResultsDirs') return [];
+          if (_key === 'additionalMrvaRunResultsDirs') return [];
+          return defaultVal;
+        },
+        has: () => false,
+        inspect: () => undefined as any,
+        update: () => Promise.resolve(),
+      }) as any;
+
+      builder.invalidate();
+      const env = await builder.build();
+      // additionalEnv should override the default MONITORING_STORAGE_LOCATION
+      expect(env.MONITORING_STORAGE_LOCATION).toBe('/custom/storage/path');
+    } finally {
+      (vscode.workspace.workspaceFolders as any) = origFolders;
+      vscode.workspace.getConfiguration = originalGetConfig;
+    }
+  });
+
   it('should set ENABLE_ANNOTATION_TOOLS=false when setting is disabled', async () => {
     const vscode = await import('vscode');
     const originalGetConfig = vscode.workspace.getConfiguration;

server/test/src/tools/cache-tools.test.ts

@@ -350,6 +350,162 @@ describe('Cache Tools', () => {
         expect(result.content[0].text).toContain('At least one filter');
       });
 
+      it('should lookup cache entry by exact cacheKey', async () => {
+        const store = sessionDataManager.getStore();
+        store.putCacheEntry({
+          cacheKey: 'exact-key-abc',
+          queryName: 'PrintAST',
+          queryPath: '/print-ast.ql',
+          databasePath: '/db1',
+          language: 'javascript',
+          codeqlVersion: '2.25.0',
+          outputFormat: 'graphtext',
+          resultContent: 'AST output',
+          resultCount: 5,
+        });
+
+        registerCacheTools(mockServer);
+
+        const lookupHandler = (mockServer.tool as any).mock.calls.find(
+          (call: any) => call[0] === 'query_results_cache_lookup',
+        )[3];
+
+        const result = await lookupHandler({ cacheKey: 'exact-key-abc' });
+        const parsed = JSON.parse(result.content[0].text);
+        expect(parsed.cached).toBe(true);
+        expect(parsed.cacheKey).toBe('exact-key-abc');
+        expect(parsed.queryName).toBe('PrintAST');
+        expect(parsed.language).toBe('javascript');
+        expect(parsed.resultCount).toBe(5);
+      });
+
+      it('should return cached:false for non-existent cacheKey lookup', async () => {
+        registerCacheTools(mockServer);
+
+        const lookupHandler = (mockServer.tool as any).mock.calls.find(
+          (call: any) => call[0] === 'query_results_cache_lookup',
+        )[3];
+
+        const result = await lookupHandler({ cacheKey: 'nonexistent-key' });
+        const parsed = JSON.parse(result.content[0].text);
+        expect(parsed.cached).toBe(false);
+        expect(parsed.cacheKey).toBe('nonexistent-key');
+      });
+
+      it('should lookup cache entries by language filter', async () => {
+        const store = sessionDataManager.getStore();
+        store.putCacheEntry({
+          cacheKey: 'js-entry-1',
+          queryName: 'PrintAST',
+          queryPath: '/print-ast.ql',
+          databasePath: '/db1',
+          language: 'javascript',
+          codeqlVersion: '2.25.0',
+          outputFormat: 'graphtext',
+          resultContent: 'content1',
+        });
+        store.putCacheEntry({
+          cacheKey: 'js-entry-2',
+          queryName: 'CallGraphTo',
+          queryPath: '/cg.ql',
+          databasePath: '/db2',
+          language: 'javascript',
+          codeqlVersion: '2.25.0',
+          outputFormat: 'graphtext',
+          resultContent: 'content2',
+        });
+        store.putCacheEntry({
+          cacheKey: 'cpp-entry-1',
+          queryName: 'PrintAST',
+          queryPath: '/print-ast.ql',
+          databasePath: '/db3',
+          language: 'cpp',
+          codeqlVersion: '2.25.0',
+          outputFormat: 'graphtext',
+          resultContent: 'content3',
+        });
+
+        registerCacheTools(mockServer);
+
+        const lookupHandler = (mockServer.tool as any).mock.calls.find(
+          (call: any) => call[0] === 'query_results_cache_lookup',
+        )[3];
+
+        const result = await lookupHandler({ language: 'javascript' });
+        const parsed = JSON.parse(result.content[0].text);
+        expect(parsed.cached).toBe(true);
+        expect(parsed.count).toBe(2);
+        expect(parsed.entries).toHaveLength(2);
+        // All entries should be JavaScript
+        for (const entry of parsed.entries) {
+          expect(entry.language).toBe('javascript');
+        }
+      });
+
+      it('should lookup cache entries by databasePath filter', async () => {
+        const store = sessionDataManager.getStore();
+        store.putCacheEntry({
+          cacheKey: 'db1-entry',
+          queryName: 'PrintAST',
+          queryPath: '/print-ast.ql',
+          databasePath: '/db1',
+          language: 'javascript',
+          codeqlVersion: '2.25.0',
+          outputFormat: 'graphtext',
+          resultContent: 'content1',
+        });
+        store.putCacheEntry({
+          cacheKey: 'db2-entry',
+          queryName: 'PrintAST',
+          queryPath: '/print-ast.ql',
+          databasePath: '/db2',
+          language: 'javascript',
+          codeqlVersion: '2.25.0',
+          outputFormat: 'graphtext',
+          resultContent: 'content2',
+        });
+
+        registerCacheTools(mockServer);
+
+        const lookupHandler = (mockServer.tool as any).mock.calls.find(
+          (call: any) => call[0] === 'query_results_cache_lookup',
+        )[3];
+
+        const result = await lookupHandler({ databasePath: '/db1' });
+        const parsed = JSON.parse(result.content[0].text);
+        expect(parsed.cached).toBe(true);
+        expect(parsed.count).toBe(1);
+        expect(parsed.entries[0].databasePath).toBe('/db1');
+      });
+
+      it('should respect limit parameter in cache lookup', async () => {
+        const store = sessionDataManager.getStore();
+        for (let i = 0; i < 5; i++) {
+          store.putCacheEntry({
+            cacheKey: `limit-entry-${i}`,
+            queryName: 'Q',
+            queryPath: '/q.ql',
+            databasePath: `/db${i}`,
+            language: 'javascript',
+            codeqlVersion: '2.25.0',
+            outputFormat: 'graphtext',
+            resultContent: `content${i}`,
+          });
+        }
+
+        registerCacheTools(mockServer);
+
+        const lookupHandler = (mockServer.tool as any).mock.calls.find(
+          (call: any) => call[0] === 'query_results_cache_lookup',
+        )[3];
+
+        const result = await lookupHandler({ language: 'javascript', limit: 2 });
+        const parsed = JSON.parse(result.content[0].text);
+        expect(parsed.cached).toBe(true);
+        expect(parsed.count).toBe(2);
+        expect(parsed.entries).toHaveLength(2);
+      });
+
       it('should derive totalResultCount from SARIF content when resultCount is null', async () => {
         const sarif = {
           version: '2.1.0',