Skip to content

Commit 58d3ec1

Browse files
authored
Fix deterministic vscode-codeql discovery tests and dual-casing path probe in CliResolver (#92)
1 parent bdb5f92 commit 58d3ec1

File tree

6 files changed

+132
-87
lines changed

6 files changed

+132
-87
lines changed

extensions/vscode/src/codeql/cli-resolver.ts

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { execFile } from 'child_process';
22
import { access, readdir, readFile } from 'fs/promises';
33
import { constants } from 'fs';
4-
import { join } from 'path';
4+
import { dirname, join } from 'path';
55
import { DisposableObject } from '../common/disposable';
66
import type { Logger } from '../common/logger';
77

@@ -122,33 +122,48 @@ export class CliResolver extends DisposableObject {
122122
private async resolveFromVsCodeDistribution(): Promise<string | undefined> {
123123
if (!this.vsCodeCodeqlStoragePath) return undefined;
124124

125-
try {
126-
// Fast path: read distribution.json for the exact folder index
127-
const hintPath = await this.resolveFromDistributionJson();
128-
if (hintPath) return hintPath;
129-
} catch {
130-
this.logger.debug('distribution.json hint unavailable, falling back to directory scan');
131-
}
125+
const parent = dirname(this.vsCodeCodeqlStoragePath);
126+
// VS Code stores the extension directory as either 'GitHub.vscode-codeql'
127+
// (original publisher casing) or 'github.vscode-codeql' (lowercased by VS Code
128+
// on some platforms/versions). Probe both to ensure discovery works on
129+
// case-sensitive filesystems.
130+
const candidatePaths = [
131+
...new Set([
132+
this.vsCodeCodeqlStoragePath,
133+
join(parent, 'github.vscode-codeql'),
134+
join(parent, 'GitHub.vscode-codeql'),
135+
]),
136+
];
137+
138+
for (const storagePath of candidatePaths) {
139+
try {
140+
// Fast path: read distribution.json for the exact folder index
141+
const hintPath = await this.resolveFromDistributionJson(storagePath);
142+
if (hintPath) return hintPath;
143+
} catch {
144+
this.logger.debug('distribution.json hint unavailable, falling back to directory scan');
145+
}
132146

133-
// Fallback: scan for distribution* directories
134-
return this.resolveFromDistributionScan();
147+
// Fallback: scan for distribution* directories
148+
const scanPath = await this.resolveFromDistributionScan(storagePath);
149+
if (scanPath) return scanPath;
150+
}
151+
return undefined;
135152
}
136153

137154
/**
138155
* Read `distribution.json` to get the current `folderIndex` and validate
139156
* the binary at the corresponding path.
140157
*/
141-
private async resolveFromDistributionJson(): Promise<string | undefined> {
142-
if (!this.vsCodeCodeqlStoragePath) return undefined;
143-
144-
const jsonPath = join(this.vsCodeCodeqlStoragePath, 'distribution.json');
158+
private async resolveFromDistributionJson(storagePath: string): Promise<string | undefined> {
159+
const jsonPath = join(storagePath, 'distribution.json');
145160
const content = await readFile(jsonPath, 'utf-8');
146161
const data = JSON.parse(content) as { folderIndex?: number };
147162

148163
if (typeof data.folderIndex !== 'number') return undefined;
149164

150165
const binaryPath = join(
151-
this.vsCodeCodeqlStoragePath,
166+
storagePath,
152167
`distribution${data.folderIndex}`,
153168
'codeql',
154169
CODEQL_BINARY_NAME,
@@ -165,11 +180,9 @@ export class CliResolver extends DisposableObject {
165180
* Scan for `distribution*` directories sorted by numeric suffix (highest
166181
* first) and return the first one containing a valid `codeql` binary.
167182
*/
168-
private async resolveFromDistributionScan(): Promise<string | undefined> {
169-
if (!this.vsCodeCodeqlStoragePath) return undefined;
170-
183+
private async resolveFromDistributionScan(storagePath: string): Promise<string | undefined> {
171184
try {
172-
const entries = await readdir(this.vsCodeCodeqlStoragePath, { withFileTypes: true });
185+
const entries = await readdir(storagePath, { withFileTypes: true });
173186

174187
const distDirs = entries
175188
.filter(e => e.isDirectory() && /^distribution\d*$/.test(e.name))
@@ -181,7 +194,7 @@ export class CliResolver extends DisposableObject {
181194

182195
for (const dir of distDirs) {
183196
const binaryPath = join(
184-
this.vsCodeCodeqlStoragePath,
197+
storagePath,
185198
dir.name,
186199
'codeql',
187200
CODEQL_BINARY_NAME,
@@ -194,7 +207,7 @@ export class CliResolver extends DisposableObject {
194207
}
195208
} catch {
196209
this.logger.debug(
197-
`Could not scan vscode-codeql distribution directory: ${this.vsCodeCodeqlStoragePath}`,
210+
`Could not scan vscode-codeql distribution directory: ${storagePath}`,
198211
);
199212
}
200213
return undefined;

extensions/vscode/test/codeql/cli-resolver.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,31 @@ describe('CliResolver', () => {
324324
expect(result).toBe(expectedPath);
325325
});
326326

327+
it('should discover CLI from alternate casing of storage path', async () => {
328+
// Simulate the caller providing 'GitHub.vscode-codeql' (StoragePaths publisher casing)
329+
// but the actual directory on disk is 'github.vscode-codeql' (lowercased by VS Code).
330+
const lowercaseExtStorage = '/mock/globalStorage/github.vscode-codeql';
331+
const publisherCaseExtStorage = '/mock/globalStorage/GitHub.vscode-codeql';
332+
resolver = new CliResolver(logger, publisherCaseExtStorage);
333+
334+
// distribution.json at the publisher-cased path throws; the lowercase path returns valid JSON
335+
vi.mocked(readFile).mockImplementation((path: any) => {
336+
if (String(path).startsWith(publisherCaseExtStorage)) {
337+
return Promise.reject(new Error('ENOENT'));
338+
}
339+
return Promise.resolve(JSON.stringify({ folderIndex: 1 }) as any);
340+
});
341+
342+
const expectedPath = `${lowercaseExtStorage}/distribution1/codeql/${binaryName}`;
343+
vi.mocked(access).mockImplementation((path: any) => {
344+
if (String(path) === expectedPath) return Promise.resolve(undefined as any);
345+
return Promise.reject(new Error('ENOENT'));
346+
});
347+
348+
const result = await resolver.resolve();
349+
expect(result).toBe(expectedPath);
350+
});
351+
327352
it('should handle distribution.json without folderIndex property', async () => {
328353
resolver = new CliResolver(logger, storagePath);
329354

server/dist/codeql-development-mcp-server.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38209,8 +38209,8 @@ function getVsCodeGlobalStorageCandidates() {
3820938209
}
3821038210
return candidates;
3821138211
}
38212-
function discoverVsCodeCodeQLDistribution() {
38213-
const globalStorageCandidates = getVsCodeGlobalStorageCandidates();
38212+
function discoverVsCodeCodeQLDistribution(candidateStorageRoots) {
38213+
const globalStorageCandidates = candidateStorageRoots ?? getVsCodeGlobalStorageCandidates();
3821438214
for (const gsRoot of globalStorageCandidates) {
3821538215
for (const dirName of VSCODE_CODEQL_STORAGE_DIR_NAMES) {
3821638216
const codeqlStorage = join4(gsRoot, dirName);

server/dist/codeql-development-mcp-server.js.map

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

server/src/lib/cli-executor.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,8 @@ export function getVsCodeGlobalStorageCandidates(): string[] {
176176
*
177177
* @returns Absolute path to the `codeql` binary, or `undefined` if not found.
178178
*/
179-
export function discoverVsCodeCodeQLDistribution(): string | undefined {
180-
const globalStorageCandidates = getVsCodeGlobalStorageCandidates();
179+
export function discoverVsCodeCodeQLDistribution(candidateStorageRoots?: string[]): string | undefined {
180+
const globalStorageCandidates = candidateStorageRoots ?? getVsCodeGlobalStorageCandidates();
181181

182182
for (const gsRoot of globalStorageCandidates) {
183183
// Check both casings: VS Code may lowercase the extension ID on disk,

server/test/src/lib/cli-executor.test.ts

Lines changed: 67 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44

55
import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest';
6-
import { writeFileSync, rmSync, chmodSync, mkdirSync, existsSync } from 'fs';
6+
import { writeFileSync, rmSync, chmodSync, mkdirSync } from 'fs';
77
import { execFileSync } from 'child_process';
88
import { isAbsolute, join } from 'path';
99
import { createProjectTempDir } from '../../../src/utils/temp-dir';
@@ -587,20 +587,12 @@ describe('resolveCodeQLBinary', () => {
587587
delete process.env.CODEQL_PATH;
588588
const result = resolveCodeQLBinary();
589589
expect(result).toBe('codeql');
590-
// getResolvedCodeQLDir() may be non-null if the vscode-codeql
591-
// distribution is installed on this machine (auto-discovery)
592-
const dir = getResolvedCodeQLDir();
593-
expect(dir === null || typeof dir === 'string').toBe(true);
594590
});
595591

596592
it('should default to "codeql" when CODEQL_PATH is empty', () => {
597593
process.env.CODEQL_PATH = '';
598594
const result = resolveCodeQLBinary();
599595
expect(result).toBe('codeql');
600-
// getResolvedCodeQLDir() may be non-null if the vscode-codeql
601-
// distribution is installed on this machine (auto-discovery)
602-
const dir = getResolvedCodeQLDir();
603-
expect(dir === null || typeof dir === 'string').toBe(true);
604596
});
605597

606598
it('should return bare codeql command and set dir to parent directory', () => {
@@ -690,10 +682,6 @@ describe('CODEQL_PATH - PATH prepend integration', () => {
690682
process.env.CODEQL_PATH = '/some/changed/path/codeql';
691683
const second = resolveCodeQLBinary();
692684
expect(second).toBe('codeql');
693-
// getResolvedCodeQLDir() may be non-null if vscode-codeql
694-
// distribution was auto-discovered on the first resolve()
695-
const dir = getResolvedCodeQLDir();
696-
expect(dir === null || typeof dir === 'string').toBe(true);
697685
});
698686

699687
it.skipIf(process.platform === 'win32')('should prepend CODEQL_PATH directory to child process PATH', async () => {
@@ -823,66 +811,89 @@ describe('getVsCodeGlobalStorageCandidates', () => {
823811
});
824812

825813
describe('discoverVsCodeCodeQLDistribution', () => {
814+
const binaryName = process.platform === 'win32' ? 'codeql.exe' : 'codeql';
815+
826816
it('should return undefined when no vscode-codeql storage exists', () => {
827-
// In the test environment, the vscode-codeql storage is unlikely to exist
828-
// at the standard location, so discovery should return undefined.
829-
// This is effectively a no-op test that ensures the function handles
830-
// missing directories gracefully.
831-
const result = discoverVsCodeCodeQLDistribution();
832-
// Result depends on whether vscode-codeql is installed — just verify it doesn't throw
833-
expect(result === undefined || typeof result === 'string').toBe(true);
817+
const tmpDir = createProjectTempDir('vscode-codeql-no-storage-');
818+
try {
819+
// No extension storage dirs created inside tmpDir
820+
const result = discoverVsCodeCodeQLDistribution([tmpDir]);
821+
expect(result).toBeUndefined();
822+
} finally {
823+
rmSync(tmpDir, { recursive: true, force: true });
824+
}
834825
});
835826

836-
it('should discover CLI from a simulated distribution directory', () => {
837-
const tmpDir = createProjectTempDir('vscode-codeql-discovery-test-');
827+
it('should discover CLI from distribution.json fast path', () => {
828+
const tmpDir = createProjectTempDir('vscode-codeql-json-fastpath-');
838829
const codeqlStorage = join(tmpDir, 'github.vscode-codeql');
839830
const distDir = join(codeqlStorage, 'distribution3', 'codeql');
840-
841-
// Create the distribution directory structure with a fake binary
842831
mkdirSync(distDir, { recursive: true });
843-
const binaryPath = join(distDir, process.platform === 'win32' ? 'codeql.exe' : 'codeql');
832+
const binaryPath = join(distDir, binaryName);
844833
writeFileSync(binaryPath, '#!/bin/sh\necho test', { mode: 0o755 });
845-
846-
// Also create distribution.json
847-
writeFileSync(
848-
join(codeqlStorage, 'distribution.json'),
849-
JSON.stringify({ folderIndex: 3 }),
850-
);
851-
834+
writeFileSync(join(codeqlStorage, 'distribution.json'), JSON.stringify({ folderIndex: 3 }));
852835
try {
853-
// We can't easily test discoverVsCodeCodeQLDistribution directly because
854-
// it uses hardcoded platform-specific paths. Instead, test the behavior
855-
// by creating the structure and verifying the file was created correctly.
856-
expect(existsSync(binaryPath)).toBe(true);
836+
const result = discoverVsCodeCodeQLDistribution([tmpDir]);
837+
expect(result).toBe(binaryPath);
857838
} finally {
858839
rmSync(tmpDir, { recursive: true, force: true });
859840
}
860841
});
861842

862-
it('should prefer distribution.json folderIndex when available', () => {
863-
const tmpDir = createProjectTempDir('vscode-codeql-json-test-');
843+
it('should fall back to directory scan when distribution.json is missing', () => {
844+
const tmpDir = createProjectTempDir('vscode-codeql-scan-fallback-');
864845
const codeqlStorage = join(tmpDir, 'github.vscode-codeql');
846+
const dist1 = join(codeqlStorage, 'distribution1', 'codeql');
847+
const dist3 = join(codeqlStorage, 'distribution3', 'codeql');
848+
mkdirSync(dist1, { recursive: true });
849+
mkdirSync(dist3, { recursive: true });
850+
writeFileSync(join(dist1, binaryName), '#!/bin/sh\necho v1', { mode: 0o755 });
851+
writeFileSync(join(dist3, binaryName), '#!/bin/sh\necho v3', { mode: 0o755 });
852+
// No distribution.json — scan should pick the highest-numbered directory
853+
try {
854+
const result = discoverVsCodeCodeQLDistribution([tmpDir]);
855+
expect(result).toBe(join(dist3, binaryName));
856+
} finally {
857+
rmSync(tmpDir, { recursive: true, force: true });
858+
}
859+
});
865860

866-
// Create two distribution directories
867-
const dist2Dir = join(codeqlStorage, 'distribution2', 'codeql');
868-
const dist3Dir = join(codeqlStorage, 'distribution3', 'codeql');
869-
mkdirSync(dist2Dir, { recursive: true });
870-
mkdirSync(dist3Dir, { recursive: true });
871-
872-
const binaryName = process.platform === 'win32' ? 'codeql.exe' : 'codeql';
873-
writeFileSync(join(dist2Dir, binaryName), '#!/bin/sh\necho v2', { mode: 0o755 });
874-
writeFileSync(join(dist3Dir, binaryName), '#!/bin/sh\necho v3', { mode: 0o755 });
875-
876-
// distribution.json points to distribution3
877-
writeFileSync(
878-
join(codeqlStorage, 'distribution.json'),
879-
JSON.stringify({ folderIndex: 3 }),
880-
);
861+
it('should prefer distribution.json folderIndex over highest-numbered directory', () => {
862+
const tmpDir = createProjectTempDir('vscode-codeql-json-precedence-');
863+
const codeqlStorage = join(tmpDir, 'github.vscode-codeql');
864+
const dist2 = join(codeqlStorage, 'distribution2', 'codeql');
865+
const dist3 = join(codeqlStorage, 'distribution3', 'codeql');
866+
mkdirSync(dist2, { recursive: true });
867+
mkdirSync(dist3, { recursive: true });
868+
writeFileSync(join(dist2, binaryName), '#!/bin/sh\necho v2', { mode: 0o755 });
869+
writeFileSync(join(dist3, binaryName), '#!/bin/sh\necho v3', { mode: 0o755 });
870+
// distribution.json points to distribution2 (lower index than distribution3)
871+
writeFileSync(join(codeqlStorage, 'distribution.json'), JSON.stringify({ folderIndex: 2 }));
872+
try {
873+
const result = discoverVsCodeCodeQLDistribution([tmpDir]);
874+
// Should pick distribution2 per distribution.json, not distribution3
875+
expect(result).toBe(join(dist2, binaryName));
876+
} finally {
877+
rmSync(tmpDir, { recursive: true, force: true });
878+
}
879+
});
881880

881+
it('should scan directories sorted by descending numeric suffix', () => {
882+
const tmpDir = createProjectTempDir('vscode-codeql-scan-order-');
883+
const codeqlStorage = join(tmpDir, 'github.vscode-codeql');
884+
const dist1 = join(codeqlStorage, 'distribution1', 'codeql');
885+
const dist2 = join(codeqlStorage, 'distribution2', 'codeql');
886+
const dist10 = join(codeqlStorage, 'distribution10', 'codeql');
887+
mkdirSync(dist1, { recursive: true });
888+
mkdirSync(dist2, { recursive: true });
889+
mkdirSync(dist10, { recursive: true });
890+
writeFileSync(join(dist1, binaryName), '#!/bin/sh\necho v1', { mode: 0o755 });
891+
writeFileSync(join(dist2, binaryName), '#!/bin/sh\necho v2', { mode: 0o755 });
892+
writeFileSync(join(dist10, binaryName), '#!/bin/sh\necho v10', { mode: 0o755 });
882893
try {
883-
// Verify both files exist
884-
expect(existsSync(join(dist3Dir, binaryName))).toBe(true);
885-
expect(existsSync(join(dist2Dir, binaryName))).toBe(true);
894+
const result = discoverVsCodeCodeQLDistribution([tmpDir]);
895+
// distribution10 > distribution2 > distribution1 by numeric sort
896+
expect(result).toBe(join(dist10, binaryName));
886897
} finally {
887898
rmSync(tmpDir, { recursive: true, force: true });
888899
}
@@ -906,10 +917,6 @@ describe('resolveCodeQLBinary - vscode-codeql auto-discovery', () => {
906917
// This will either find a vscode-codeql distribution or fall back to bare 'codeql'
907918
const result = resolveCodeQLBinary();
908919
expect(result).toBe('codeql');
909-
// If a distribution was found, resolvedCodeQLDir will be set
910-
// If not, it will be null — either is acceptable
911-
const dir = getResolvedCodeQLDir();
912-
expect(dir === null || typeof dir === 'string').toBe(true);
913920
});
914921
});
915922

0 commit comments

Comments
 (0)