Skip to content

Commit 6396b86

Browse files
Copilotdata-douser
andauthored
Fix getExtensionVersion() and getBundledQlRoot() per review comments (#82)
* Initial plan * Fix getExtensionVersion() and getBundledQlRoot() per review comments Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
1 parent 68c5bcf commit 6396b86

File tree

2 files changed

+28
-12
lines changed

2 files changed

+28
-12
lines changed

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

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as vscode from 'vscode';
22
import { execFile } from 'child_process';
33
import { access, readFile, mkdir } from 'fs/promises';
4-
import { accessSync, constants } from 'fs';
4+
import { accessSync, constants, readFileSync } from 'fs';
55
import { join } from 'path';
66
import { DisposableObject } from '../common/disposable';
77
import type { Logger } from '../common/logger';
@@ -92,11 +92,21 @@ export class ServerManager extends DisposableObject {
9292
/**
9393
* Get the extension's own version from its packageJSON.
9494
*
95+
* Reads `package.json` from the extension root (`context.extensionUri`)
96+
* so that this works in all environments (VSIX, Extension Development
97+
* Host, and tests) without relying on `vscode.extensions.getExtension`.
98+
*
9599
* This is the version baked into the VSIX and is used to determine
96100
* whether the locally installed npm package is still up-to-date.
97101
*/
98102
getExtensionVersion(): string {
99-
return this.context.extension.packageJSON.version as string;
103+
try {
104+
const pkgPath = join(this.context.extensionUri.fsPath, 'package.json');
105+
const pkg = JSON.parse(readFileSync(pkgPath, 'utf-8')) as { version?: string };
106+
return pkg.version ?? 'unknown';
107+
} catch {
108+
return 'unknown';
109+
}
100110
}
101111

102112
/**
@@ -111,10 +121,10 @@ export class ServerManager extends DisposableObject {
111121
* `globalStorage`. Returns `true` if a fresh install was performed.
112122
*/
113123
async ensureInstalled(): Promise<boolean> {
114-
// VSIX bundle is self-contained — no npm install required.
124+
// VSIX bundle or monorepo server is present — no npm install required.
115125
if (this.getBundledQlRoot()) {
116126
this.logger.info(
117-
`Using VSIX-bundled server (v${this.getExtensionVersion()}). ` +
127+
`Using bundled server (v${this.getExtensionVersion()}). ` +
118128
'No npm install required.',
119129
);
120130
return false;
@@ -164,13 +174,15 @@ export class ServerManager extends DisposableObject {
164174
// ---------------------------------------------------------------------------
165175

166176
/**
167-
* Root of the bundled `server/` directory inside the VSIX.
177+
* Root of the `server/` directory, checked in two locations:
178+
*
179+
* 1. **VSIX layout**: `<extensionRoot>/server/` (created by `vscode:prepublish`)
180+
* — the extension is self-contained, no npm install required.
181+
* 2. **Monorepo dev layout**: `<extensionRoot>/../../server/` — used when
182+
* running from the Extension Development Host without a prepublish build.
168183
*
169-
* In VSIX layout the `vscode:prepublish` step copies `server/dist/`,
170-
* `server/ql/`, and `server/package.json` into the extension so the VSIX
171-
* is self-contained. Returns the path to that `server/` directory, or
172-
* `undefined` if the bundle is missing (local dev without a prepublish
173-
* build).
184+
* Returns the first location whose `server/package.json` is readable, or
185+
* `undefined` if neither location exists.
174186
*/
175187
getBundledQlRoot(): string | undefined {
176188
const extensionRoot = this.context.extensionUri.fsPath;

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,13 @@ vi.mock('fs/promises', () => ({
1313

1414
vi.mock('fs', () => ({
1515
accessSync: vi.fn(),
16+
readFileSync: vi.fn(),
1617
constants: { R_OK: 4 },
1718
}));
1819

1920
import { execFile } from 'child_process';
2021
import { access, readFile } from 'fs/promises';
21-
import { accessSync } from 'fs';
22+
import { accessSync, readFileSync } from 'fs';
2223

2324
function createMockContext(extensionVersion = '2.24.2') {
2425
return {
@@ -176,6 +177,7 @@ describe('ServerManager', () => {
176177
});
177178

178179
it('should return extension version from context', () => {
180+
vi.mocked(readFileSync).mockReturnValue(JSON.stringify({ version: '2.24.2' }));
179181
expect(manager.getExtensionVersion()).toBe('2.24.2');
180182
});
181183

@@ -212,7 +214,7 @@ describe('ServerManager', () => {
212214
expect(installed).toBe(false);
213215
expect(execFile).not.toHaveBeenCalled();
214216
expect(logger.info).toHaveBeenCalledWith(
215-
expect.stringContaining('VSIX-bundled server'),
217+
expect.stringContaining('bundled server'),
216218
);
217219
});
218220

@@ -239,6 +241,8 @@ describe('ServerManager', () => {
239241
it('should skip npm install when bundle is missing but matching version installed', async () => {
240242
// No bundle
241243
vi.mocked(accessSync).mockImplementation(() => { throw new Error('ENOENT'); });
244+
// Extension version from package.json
245+
vi.mocked(readFileSync).mockReturnValue(JSON.stringify({ version: '2.24.2' }));
242246
// Already installed with matching version
243247
vi.mocked(access).mockResolvedValue(undefined);
244248
vi.mocked(readFile).mockResolvedValue(JSON.stringify({ version: '2.24.2' }));

0 commit comments

Comments
 (0)