Skip to content

Commit fd50a2a

Browse files
committed
Improve vscode extension CodeQL pack install
1 parent 6e9f718 commit fd50a2a

7 files changed

Lines changed: 325 additions & 14 deletions

File tree

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

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ const KNOWN_LOCATIONS = [
3232
export class CliResolver extends DisposableObject {
3333
private cachedPath: string | undefined | null = null; // null = not yet resolved
3434
private cachedVersion: string | undefined;
35+
private resolvePromise: Promise<string | undefined> | null = null;
3536

3637
constructor(
3738
private readonly logger: Logger,
@@ -57,6 +58,21 @@ export class CliResolver extends DisposableObject {
5758
return this.cachedPath;
5859
}
5960

61+
// Return the in-flight promise if a resolution is already in progress
62+
if (this.resolvePromise) {
63+
return this.resolvePromise;
64+
}
65+
66+
this.resolvePromise = this.doResolve();
67+
try {
68+
return await this.resolvePromise;
69+
} finally {
70+
this.resolvePromise = null;
71+
}
72+
}
73+
74+
/** Internal resolution logic. Called at most once per cache cycle. */
75+
private async doResolve(): Promise<string | undefined> {
6076
this.logger.debug('Resolving CodeQL CLI path...');
6177

6278
// Strategy 1: CODEQL_PATH env var
@@ -74,9 +90,13 @@ export class CliResolver extends DisposableObject {
7490
// Strategy 2: which/command -v
7591
const whichPath = await this.resolveFromPath();
7692
if (whichPath) {
77-
this.logger.info(`CodeQL CLI found on PATH: ${whichPath}`);
78-
this.cachedPath = whichPath;
79-
return whichPath;
93+
const validated = await this.validateBinary(whichPath);
94+
if (validated) {
95+
this.logger.info(`CodeQL CLI found on PATH: ${whichPath}`);
96+
this.cachedPath = whichPath;
97+
return whichPath;
98+
}
99+
this.logger.warn(`Found 'codeql' on PATH at '${whichPath}' but it failed validation.`);
80100
}
81101

82102
// Strategy 3: vscode-codeql managed distribution
@@ -106,6 +126,7 @@ export class CliResolver extends DisposableObject {
106126
invalidateCache(): void {
107127
this.cachedPath = null;
108128
this.cachedVersion = undefined;
129+
this.resolvePromise = null;
109130
}
110131

111132
/** Check if a path exists and responds to `--version`. */

extensions/vscode/src/server/pack-installer.ts

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,21 @@ export class PackInstaller extends DisposableObject {
153153
const actualCliVersion = this.cliResolver.getCliVersion();
154154
const targetCliVersion = this.getTargetCliVersion();
155155

156+
if (actualCliVersion) {
157+
this.logger.info(
158+
`Detected CodeQL CLI version: ${actualCliVersion}, target: ${targetCliVersion}.`,
159+
);
160+
} else {
161+
this.logger.info(
162+
`CodeQL CLI version could not be determined. Target: ${targetCliVersion}. ` +
163+
'Using bundled pack install.',
164+
);
165+
}
166+
156167
if (downloadEnabled && actualCliVersion && actualCliVersion !== targetCliVersion) {
157168
this.logger.info(
158169
`CodeQL CLI version ${actualCliVersion} differs from VSIX target ${targetCliVersion}. ` +
159-
'Attempting to download compatible tool query packs...',
170+
'Attempting to install compatible tool query packs...',
160171
);
161172
const downloaded = await this.downloadPacksForCliVersion(
162173
codeqlPath, actualCliVersion, languages,
@@ -165,7 +176,11 @@ export class PackInstaller extends DisposableObject {
165176
return;
166177
}
167178
this.logger.info(
168-
'Pack download did not succeed for all languages — falling back to bundled pack install.',
179+
'Pack install did not succeed for all languages — falling back to bundled pack install.',
180+
);
181+
} else if (actualCliVersion && actualCliVersion === targetCliVersion) {
182+
this.logger.info(
183+
`CLI and target versions match (${actualCliVersion}). Using bundled pack install.`,
169184
);
170185
}
171186

@@ -195,24 +210,29 @@ export class PackInstaller extends DisposableObject {
195210
}
196211

197212
this.logger.info(
198-
`Downloading ql-mcp tool query packs v${packVersion} for CodeQL CLI ${cliVersion}...`,
213+
`Installing ql-mcp tool query packs v${packVersion} for CodeQL CLI ${cliVersion}...`,
199214
);
200215

201216
let allSucceeded = true;
217+
let successCount = 0;
202218
for (const lang of languages) {
203219
const packRef =
204220
`${PackInstaller.PACK_SCOPE}/ql-mcp-${lang}-tools-src@${packVersion}`;
205-
this.logger.info(`Downloading ${packRef}...`);
221+
this.logger.info(`Installing ${packRef}...`);
206222
try {
207223
await this.runCodeqlPackDownload(codeqlPath, packRef);
208-
this.logger.info(`Downloaded ${packRef}.`);
224+
this.logger.info(`Installed ${packRef}.`);
225+
successCount++;
209226
} catch (err) {
210227
this.logger.error(
211-
`Failed to download ${packRef}: ${err instanceof Error ? err.message : String(err)}`,
228+
`Failed to install ${packRef}: ${err instanceof Error ? err.message : String(err)}`,
212229
);
213230
allSucceeded = false;
214231
}
215232
}
233+
this.logger.info(
234+
`Pack install complete: ${successCount}/${languages.length} languages succeeded.`,
235+
);
216236
return allSucceeded;
217237
}
218238

@@ -226,9 +246,11 @@ export class PackInstaller extends DisposableObject {
226246
languages: string[],
227247
): Promise<void> {
228248
const qlRoot = this.getQlpackRoot();
249+
let successCount = 0;
229250

230251
for (const lang of languages) {
231252
const packDir = join(qlRoot, 'ql', lang, 'tools', 'src');
253+
const packName = `${PackInstaller.PACK_SCOPE}/ql-mcp-${lang}-tools-src`;
232254

233255
// Check if the pack directory exists
234256
try {
@@ -238,16 +260,20 @@ export class PackInstaller extends DisposableObject {
238260
continue;
239261
}
240262

241-
this.logger.info(`Installing CodeQL pack dependencies for ${lang}...`);
263+
this.logger.info(`Installing CodeQL pack dependencies for ${packName} (${lang})...`);
242264
try {
243265
await this.runCodeqlPackInstall(codeqlPath, packDir);
244-
this.logger.info(`Pack dependencies installed for ${lang}.`);
266+
this.logger.info(`Pack dependencies installed for ${packName} (${lang}).`);
267+
successCount++;
245268
} catch (err) {
246269
this.logger.error(
247270
`Failed to install pack dependencies for ${lang}: ${err instanceof Error ? err.message : String(err)}`,
248271
);
249272
}
250273
}
274+
this.logger.info(
275+
`Bundled pack install complete: ${successCount}/${languages.length} languages succeeded.`,
276+
);
251277
}
252278

253279
/** Run `codeql pack install` for a single pack directory. */

extensions/vscode/src/server/server-manager.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,7 @@ export class ServerManager extends DisposableObject {
124124
// VSIX bundle or monorepo server is present — no npm install required.
125125
if (this.getBundledQlRoot()) {
126126
this.logger.info(
127-
`Using bundled server (v${this.getExtensionVersion()}). ` +
128-
'No npm install required.',
127+
`Bundled server ready (v${this.getExtensionVersion()}).`,
129128
);
130129
return false;
131130
}

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

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,4 +434,121 @@ describe('CliResolver', () => {
434434
expect(result).toBe(expectedPath);
435435
});
436436
});
437+
438+
describe('PATH resolution version detection', () => {
439+
let originalEnv: string | undefined;
440+
441+
beforeEach(() => {
442+
originalEnv = process.env.CODEQL_PATH;
443+
delete process.env.CODEQL_PATH;
444+
});
445+
446+
afterEach(() => {
447+
if (originalEnv === undefined) {
448+
delete process.env.CODEQL_PATH;
449+
} else {
450+
process.env.CODEQL_PATH = originalEnv;
451+
}
452+
});
453+
454+
it('should detect CLI version when resolved via PATH', async () => {
455+
// `which codeql` succeeds, then `codeql --version` returns version
456+
vi.mocked(execFile).mockImplementation(
457+
(_cmd: any, _args: any, callback: any) => {
458+
const cmd = String(_cmd);
459+
const args = Array.isArray(_args) ? _args : [];
460+
if (cmd === 'which' || cmd === 'where') {
461+
callback(null, '/usr/local/bin/codeql\n', '');
462+
} else if (args.includes('--version')) {
463+
callback(null, 'CodeQL CLI 2.24.1\n', '');
464+
}
465+
return {} as any;
466+
},
467+
);
468+
469+
vi.mocked(access).mockResolvedValue(undefined as any);
470+
471+
const result = await resolver.resolve();
472+
expect(result).toBe('/usr/local/bin/codeql');
473+
expect(resolver.getCliVersion()).toBe('2.24.1');
474+
});
475+
476+
it('should fall through when PATH binary fails validation', async () => {
477+
// `which codeql` returns a path, but --version fails
478+
vi.mocked(execFile).mockImplementation(
479+
(_cmd: any, _args: any, callback: any) => {
480+
const cmd = String(_cmd);
481+
if (cmd === 'which' || cmd === 'where') {
482+
callback(null, '/broken/codeql\n', '');
483+
} else {
484+
callback(new Error('not a valid binary'), '', '');
485+
}
486+
return {} as any;
487+
},
488+
);
489+
490+
// access fails for everything (including known locations)
491+
vi.mocked(access).mockRejectedValue(new Error('ENOENT'));
492+
493+
const result = await resolver.resolve();
494+
expect(result).toBeUndefined();
495+
expect(resolver.getCliVersion()).toBeUndefined();
496+
});
497+
});
498+
499+
describe('concurrent resolution', () => {
500+
let originalEnv: string | undefined;
501+
502+
beforeEach(() => {
503+
originalEnv = process.env.CODEQL_PATH;
504+
process.env.CODEQL_PATH = '/usr/local/bin/codeql';
505+
});
506+
507+
afterEach(() => {
508+
if (originalEnv === undefined) {
509+
delete process.env.CODEQL_PATH;
510+
} else {
511+
process.env.CODEQL_PATH = originalEnv;
512+
}
513+
});
514+
515+
it('should not duplicate resolution work for concurrent calls', async () => {
516+
vi.mocked(access).mockResolvedValue(undefined as any);
517+
vi.mocked(execFile).mockImplementation(
518+
(_cmd: any, _args: any, callback: any) => {
519+
callback(null, 'CodeQL CLI 2.25.1\n', '');
520+
return {} as any;
521+
},
522+
);
523+
524+
// Fire two concurrent resolve() calls
525+
const [result1, result2] = await Promise.all([
526+
resolver.resolve(),
527+
resolver.resolve(),
528+
]);
529+
530+
expect(result1).toBe('/usr/local/bin/codeql');
531+
expect(result2).toBe('/usr/local/bin/codeql');
532+
// Should only validate once, not twice
533+
expect(access).toHaveBeenCalledTimes(1);
534+
});
535+
536+
it('should allow re-resolution after invalidateCache', async () => {
537+
vi.mocked(access).mockResolvedValue(undefined as any);
538+
vi.mocked(execFile).mockImplementation(
539+
(_cmd: any, _args: any, callback: any) => {
540+
callback(null, 'CodeQL CLI 2.25.1\n', '');
541+
return {} as any;
542+
},
543+
);
544+
545+
await resolver.resolve();
546+
resolver.invalidateCache();
547+
548+
const result = await resolver.resolve();
549+
expect(result).toBe('/usr/local/bin/codeql');
550+
// access called twice: once before invalidation, once after
551+
expect(access).toHaveBeenCalledTimes(2);
552+
});
553+
});
437554
});

0 commit comments

Comments
 (0)