From ec15bfb3ecc623be893dc56fb7cf33ae5392df4e Mon Sep 17 00:00:00 2001 From: Florent Benoit Date: Fri, 17 Jul 2026 18:12:34 +0200 Subject: [PATCH] fix(openshell): use extension assets folder for bundled binaries in dev mode (#2485) In development mode, process.resourcesPath is not set, so bundled OpenShell binaries were never found and the system PATH was used instead. Now checks the extension's assets/-/ folder when not in production, matching the layout created by download scripts. Co-Authored-By: Claude Signed-off-by: Florent Benoit --- .../src/manager/openshell-cli-manager.spec.ts | 106 ++++++++++++++---- .../src/manager/openshell-cli-manager.ts | 22 +++- extensions/openshell/tsconfig.json | 2 +- 3 files changed, 104 insertions(+), 26 deletions(-) diff --git a/extensions/openshell/src/manager/openshell-cli-manager.spec.ts b/extensions/openshell/src/manager/openshell-cli-manager.spec.ts index 5f80936f5d..2334f246c0 100644 --- a/extensions/openshell/src/manager/openshell-cli-manager.spec.ts +++ b/extensions/openshell/src/manager/openshell-cli-manager.spec.ts @@ -21,7 +21,7 @@ import { existsSync, readFileSync } from 'node:fs'; import { join } from 'node:path'; import { cli, configuration, process as extensionProcess } from '@openkaiden/api'; -import { beforeEach, describe, expect, test, vi } from 'vitest'; +import { assert, beforeEach, describe, expect, test, vi } from 'vitest'; import { OpenshellCliManager } from './openshell-cli-manager'; @@ -82,17 +82,16 @@ describe('OpenshellCliManager', () => { expect(registeredNames).toContain('openshell-gateway'); const gwCall = createCalls.find(call => call[0].name === 'openshell-gateway'); - expect(gwCall).toBeDefined(); - expect(gwCall![0].installationSource).toBe('extension'); - expect(gwCall![0].path).toBeUndefined(); - expect(gwCall![0].version).toBeUndefined(); + assert(gwCall); + expect(gwCall[0].installationSource).toBe('extension'); + expect(gwCall[0].path).toBeUndefined(); + expect(gwCall[0].version).toBeUndefined(); }); describe('binary discovery priority', () => { test('prefers bundled resource over system PATH', async () => { - const bundledPath = join('/resources', 'openshell', 'openshell'); - - Object.defineProperty(process, 'resourcesPath', { value: '/resources', configurable: true }); + const platformArch = `${process.platform}-${process.arch}`; + const bundledPath = join(EXTENSION_URI, 'assets', platformArch, 'openshell'); vi.mocked(existsSync).mockImplementation((p: PathLike) => { return String(p) === bundledPath; @@ -118,13 +117,9 @@ describe('OpenshellCliManager', () => { expect(extensionProcess.exec).not.toHaveBeenCalledWith('openshell', expect.anything()); // only the bundled binary should have been version-checked expect(extensionProcess.exec).toHaveBeenCalledWith(bundledPath, ['--version']); - - Object.defineProperty(process, 'resourcesPath', { value: undefined, configurable: true }); }); test('falls back to system PATH when no bundled resource exists', async () => { - Object.defineProperty(process, 'resourcesPath', { value: '/resources', configurable: true }); - // no binary exists on disk vi.mocked(existsSync).mockReturnValue(false); @@ -143,15 +138,12 @@ describe('OpenshellCliManager', () => { await manager.init(); expect(manager.getRegisteredPath()).toBe('/usr/local/bin/openshell'); - - Object.defineProperty(process, 'resourcesPath', { value: undefined, configurable: true }); }); test('prefers extension storage over bundled resource when resolution is storage,bundled,system', async () => { const storageBinPath = join(STORAGE_PATH, 'bin', 'openshell'); - const bundledPath = join('/resources', 'openshell', 'openshell'); - - Object.defineProperty(process, 'resourcesPath', { value: '/resources', configurable: true }); + const platformArch = `${process.platform}-${process.arch}`; + const bundledPath = join(EXTENSION_URI, 'assets', platformArch, 'openshell'); vi.mocked(configuration.getConfiguration).mockReturnValue({ get: vi.fn().mockImplementation((key: string) => { @@ -183,14 +175,11 @@ describe('OpenshellCliManager', () => { expect(manager.getRegisteredPath()).toBe(storageBinPath); // bundled binary should not have been checked expect(extensionProcess.exec).not.toHaveBeenCalledWith(bundledPath, expect.anything()); - - Object.defineProperty(process, 'resourcesPath', { value: undefined, configurable: true }); }); test('prefers system PATH over bundled resource when resolution is system,bundled,storage', async () => { - const bundledPath = join('/resources', 'openshell', 'openshell'); - - Object.defineProperty(process, 'resourcesPath', { value: '/resources', configurable: true }); + const platformArch = `${process.platform}-${process.arch}`; + const bundledPath = join(EXTENSION_URI, 'assets', platformArch, 'openshell'); vi.mocked(configuration.getConfiguration).mockReturnValue({ get: vi.fn().mockImplementation((key: string) => { @@ -221,8 +210,81 @@ describe('OpenshellCliManager', () => { expect(manager.getRegisteredPath()).toBe('/usr/local/bin/openshell'); // bundled binary should NOT have been version-checked because system PATH was found first expect(extensionProcess.exec).not.toHaveBeenCalledWith(bundledPath, expect.anything()); + }); + + test('uses process.resourcesPath with original subdir in production mode', async () => { + vi.stubEnv('PROD', true); + const bundledPath = join('/resources', 'openshell', 'openshell'); + + Object.defineProperty(process, 'resourcesPath', { value: '/resources', configurable: true }); + + vi.mocked(existsSync).mockImplementation((p: PathLike) => { + return String(p) === bundledPath; + }); + + vi.mocked(extensionProcess.exec).mockImplementation(async (cmd: string) => { + if (cmd === bundledPath) { + return { stdout: 'openshell 0.2.0', stderr: '', command: cmd }; + } + throw new Error(`unexpected exec: ${cmd}`); + }); + + const manager = createManager(); + await manager.init(); + + expect(manager.getRegisteredPath()).toBe(bundledPath); Object.defineProperty(process, 'resourcesPath', { value: undefined, configurable: true }); + vi.unstubAllEnvs(); + }); + + test('uses assets folder with platform-arch subdir in development mode', async () => { + const platformArch = `${process.platform}-${process.arch}`; + const bundledPath = join(EXTENSION_URI, 'assets', platformArch, 'openshell'); + + vi.mocked(existsSync).mockImplementation((p: PathLike) => { + return String(p) === bundledPath; + }); + + vi.mocked(extensionProcess.exec).mockImplementation(async (cmd: string) => { + if (cmd === bundledPath) { + return { stdout: 'openshell 0.2.0', stderr: '', command: cmd }; + } + throw new Error(`unexpected exec: ${cmd}`); + }); + + const manager = createManager(); + await manager.init(); + + expect(manager.getRegisteredPath()).toBe(bundledPath); + expect(extensionProcess.exec).toHaveBeenCalledWith(bundledPath, ['--version']); + }); + + test('uses assets/image-builder subdir for image builder in development mode', async () => { + const platformArch = `${process.platform}-${process.arch}`; + const ibBundledPath = join(EXTENSION_URI, 'assets', 'image-builder', platformArch, 'openshell-image-builder'); + + vi.mocked(existsSync).mockImplementation((p: PathLike) => { + return String(p) === ibBundledPath; + }); + + vi.mocked(extensionProcess.exec).mockImplementation(async (cmd: string) => { + if (cmd === ibBundledPath) { + return { stdout: 'openshell-image-builder 0.9.0', stderr: '', command: cmd }; + } + throw new Error(`unexpected exec: ${cmd}`); + }); + + const manager = createManager(); + await manager.init(); + + expect(extensionProcess.exec).toHaveBeenCalledWith(ibBundledPath, ['--version']); + + const createCalls = vi.mocked(cli.createCliTool).mock.calls; + const ibCall = createCalls.find(call => call[0].name === 'openshell-image-builder'); + assert(ibCall); + expect(ibCall[0].path).toBe(ibBundledPath); + expect(ibCall[0].version).toBe('0.9.0'); }); test('prefers custom config path over all others', async () => { diff --git a/extensions/openshell/src/manager/openshell-cli-manager.ts b/extensions/openshell/src/manager/openshell-cli-manager.ts index 4a266d904b..056c5dcc5f 100644 --- a/extensions/openshell/src/manager/openshell-cli-manager.ts +++ b/extensions/openshell/src/manager/openshell-cli-manager.ts @@ -73,6 +73,7 @@ export class OpenshellCliManager implements Disposable { 'openshell-image-builder', 'imageBuilder.binary.path', 'openshell-image-builder', + 'image-builder', ); const ibRegistration: BinaryDiscoveryResult = ibResult ?? { installationSource: 'extension', @@ -139,6 +140,7 @@ export class OpenshellCliManager implements Disposable { binaryBaseName: string, configKey: string, resourceSubdir: string, + assetsSubdir?: string, ): Promise { const binDir = join(this.extensionContext.storagePath, 'bin'); const binaryName = extensionApi.env.isWindows ? `${binaryBaseName}.exe` : binaryBaseName; @@ -167,7 +169,7 @@ export class OpenshellCliManager implements Disposable { result = await this.discoverFromExtensionStorage(binaryBaseName, localBinaryPath); break; case 'bundled': - result = await this.discoverFromBundledResources(binaryBaseName, binaryName, resourceSubdir); + result = await this.discoverFromBundledResources(binaryBaseName, binaryName, resourceSubdir, assetsSubdir); break; case 'system': result = await this.discoverFromSystemPath(binaryBaseName); @@ -198,10 +200,24 @@ export class OpenshellCliManager implements Disposable { binaryBaseName: string, binaryName: string, resourceSubdir: string, + assetsSubdir?: string, ): Promise { - const resourcesPath = (process as NodeJS.Process & { resourcesPath?: string }).resourcesPath; + let resourcesPath: string | undefined; + let bundledResourceSubdir: string; + if (import.meta.env.PROD) { + resourcesPath = (process as NodeJS.Process & { resourcesPath?: string }).resourcesPath; + bundledResourceSubdir = resourceSubdir; + } else { + // In development mode, use the path used by download scripts in assets folder + const parts: string[] = [this.extensionContext.extensionUri.fsPath, 'assets']; + if (assetsSubdir) { + parts.push(assetsSubdir); + } + resourcesPath = join(...parts); + bundledResourceSubdir = `${process.platform}-${process.arch}`; + } if (resourcesPath) { - const bundledBinaryPath = join(resourcesPath, resourceSubdir, binaryName); + const bundledBinaryPath = join(resourcesPath, bundledResourceSubdir, binaryName); console.log(`[${binaryBaseName}] checking bundled resources at ${bundledBinaryPath}`); if (existsSync(bundledBinaryPath)) { const version = await this.getVersion(bundledBinaryPath); diff --git a/extensions/openshell/tsconfig.json b/extensions/openshell/tsconfig.json index 5685172135..b472012ee9 100644 --- a/extensions/openshell/tsconfig.json +++ b/extensions/openshell/tsconfig.json @@ -18,5 +18,5 @@ "/@/*": ["./src/*"] } }, - "include": ["src"] + "include": ["src", "../../types/*.d.ts"] }