diff --git a/packages/client/src/stores/hermes/app.ts b/packages/client/src/stores/hermes/app.ts index 5df84a2e..15253d1c 100644 --- a/packages/client/src/stores/hermes/app.ts +++ b/packages/client/src/stores/hermes/app.ts @@ -37,6 +37,9 @@ export const useAppStore = defineStore('app', () => { await checkConnection() } return res.success + } catch (err) { + console.error('Failed to update Hermes Web UI:', err) + return false } finally { updating.value = false } diff --git a/packages/server/src/controllers/update.ts b/packages/server/src/controllers/update.ts index 782d6fdf..c61a626d 100644 --- a/packages/server/src/controllers/update.ts +++ b/packages/server/src/controllers/update.ts @@ -1,42 +1,75 @@ import { execFileSync, spawn } from 'child_process' -import { join } from 'path' +import { existsSync } from 'fs' +import { delimiter, dirname, join } from 'path' -function getNpmBin() { - return process.platform === 'win32' ? 'npm.cmd' : 'npm' +function getNodeBinDir() { + return dirname(process.execPath) } -function getGlobalPrefix() { - return execFileSync(getNpmBin(), ['prefix', '-g'], { - encoding: 'utf-8', - stdio: ['pipe', 'pipe', 'pipe'], - }).trim() +function getNodePrefix() { + return process.platform === 'win32' ? getNodeBinDir() : dirname(getNodeBinDir()) } -function getGlobalCliBin() { - const prefix = getGlobalPrefix() +function getNpmCliPath() { + const prefix = getNodePrefix() + const candidates = process.platform === 'win32' + ? [ + join(prefix, 'node_modules', 'npm', 'bin', 'npm-cli.js'), + join(getNodeBinDir(), 'node_modules', 'npm', 'bin', 'npm-cli.js'), + ] + : [join(prefix, 'lib', 'node_modules', 'npm', 'bin', 'npm-cli.js')] + const npmCli = candidates.find(existsSync) - if (process.platform === 'win32') { - return join(prefix, 'hermes-web-ui.cmd') + if (!npmCli) { + throw new Error(`Unable to locate npm CLI for ${process.execPath}; checked ${candidates.join(', ')}`) } - return join(prefix, 'bin', 'hermes-web-ui') + return npmCli } -function runUpdateInstall() { - return execFileSync(getNpmBin(), ['install', '-g', 'hermes-web-ui@latest'], { +function getGlobalPackageBin(prefix: string) { + return process.platform === 'win32' + ? join(prefix, 'node_modules', 'hermes-web-ui', 'bin', 'hermes-web-ui.mjs') + : join(prefix, 'lib', 'node_modules', 'hermes-web-ui', 'bin', 'hermes-web-ui.mjs') +} + +function getCurrentNodeEnv() { + return { + ...process.env, + PATH: [getNodeBinDir(), process.env.PATH].filter(Boolean).join(delimiter), + npm_node_execpath: process.execPath, + } +} + +function runNpm(args: string[], options: { timeout?: number } = {}) { + return execFileSync(process.execPath, [getNpmCliPath(), ...args], { encoding: 'utf-8', - timeout: 10 * 60 * 1000, + timeout: options.timeout, stdio: ['pipe', 'pipe', 'pipe'], - }) + env: getCurrentNodeEnv(), + }).trim() +} + +function getGlobalPrefix() { + return runNpm(['prefix', '-g']) +} + +function getGlobalCliScript() { + return getGlobalPackageBin(getGlobalPrefix()) +} + +function runUpdateInstall() { + return runNpm(['install', '-g', 'hermes-web-ui@latest'], { timeout: 10 * 60 * 1000 }) } function spawnRestart(port: string) { - const cli = getGlobalCliBin() + const cli = getGlobalCliScript() - return spawn(cli, ['restart', '--port', port], { + return spawn(process.execPath, [cli, 'restart', '--port', port], { detached: true, stdio: 'ignore', windowsHide: true, + env: getCurrentNodeEnv(), }) } diff --git a/tests/client/app-store.test.ts b/tests/client/app-store.test.ts index 02856c57..d2ac26be 100644 --- a/tests/client/app-store.test.ts +++ b/tests/client/app-store.test.ts @@ -33,4 +33,17 @@ describe('App Store', () => { expect(store.sidebarCollapsed).toBe(false) expect(window.localStorage.getItem('hermes_sidebar_collapsed')).toBe('0') }) + + it('clears the updating state and reports failure when self-update request fails', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + mockSystemApi.triggerUpdate.mockRejectedValue(new Error('install failed')) + const store = useAppStore() + + const ok = await store.doUpdate() + + expect(ok).toBe(false) + expect(store.updating).toBe(false) + expect(consoleError).toHaveBeenCalledWith('Failed to update Hermes Web UI:', expect.any(Error)) + consoleError.mockRestore() + }) }) diff --git a/tests/server/update-controller.test.ts b/tests/server/update-controller.test.ts index 379c2e70..83d96940 100644 --- a/tests/server/update-controller.test.ts +++ b/tests/server/update-controller.test.ts @@ -1,24 +1,27 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { dirname, join } from 'path' +import { delimiter, dirname, join } from 'path' -type ChildProcessMocks = { +type UpdateControllerMocks = { execFileSync: ReturnType spawn: ReturnType unref: ReturnType + existsSync: ReturnType } -async function loadUpdateController(overrides: Partial = {}) { +async function loadUpdateController(overrides: Partial = {}) { const execFileSync = overrides.execFileSync ?? vi.fn().mockReturnValue('updated') const unref = overrides.unref ?? vi.fn() const spawn = overrides.spawn ?? vi.fn(() => ({ unref })) + const existsSync = overrides.existsSync ?? vi.fn(() => true) vi.resetModules() vi.doMock('child_process', () => ({ execFileSync, spawn })) + vi.doMock('fs', () => ({ existsSync })) const mod = await import('../../packages/server/src/controllers/update') return { ...mod, - mocks: { execFileSync, spawn, unref }, + mocks: { execFileSync, spawn, unref, existsSync }, } } @@ -29,6 +32,27 @@ function createMockCtx() { } } +function getNodeBinDir() { + return dirname(process.execPath) +} + +function getNodePrefix() { + return process.platform === 'win32' ? getNodeBinDir() : dirname(getNodeBinDir()) +} + +function getNpmCliPath() { + const prefix = getNodePrefix() + return process.platform === 'win32' + ? join(prefix, 'node_modules', 'npm', 'bin', 'npm-cli.js') + : join(prefix, 'lib', 'node_modules', 'npm', 'bin', 'npm-cli.js') +} + +function getGlobalCliScript(prefix: string) { + return process.platform === 'win32' + ? join(prefix, 'node_modules', 'hermes-web-ui', 'bin', 'hermes-web-ui.mjs') + : join(prefix, 'lib', 'node_modules', 'hermes-web-ui', 'bin', 'hermes-web-ui.mjs') +} + describe('update controller', () => { const originalPort = process.env.PORT const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => undefined) as never) @@ -40,6 +64,8 @@ describe('update controller', () => { afterEach(() => { vi.useRealTimers() + vi.doUnmock('child_process') + vi.doUnmock('fs') if (originalPort === undefined) { delete process.env.PORT } else { @@ -47,35 +73,56 @@ describe('update controller', () => { } }) - it('updates using npm from PATH and restarts via global prefix', async () => { + it('updates and restarts through the running Node executable, not PATH shims', async () => { process.env.PORT = '9129' - const { handleUpdate, mocks } = await loadUpdateController() + const nodeBinDir = getNodeBinDir() + const npmCli = getNpmCliPath() + const globalPrefix = getNodePrefix() + const cliScript = getGlobalCliScript(globalPrefix) + const execFileSync = vi.fn((_command: string, args: string[]) => { + if (args[1] === 'prefix') return globalPrefix + return 'updated' + }) + const { handleUpdate, mocks } = await loadUpdateController({ execFileSync }) const ctx = createMockCtx() await handleUpdate(ctx) expect(mocks.execFileSync).toHaveBeenCalledWith( - process.platform === 'win32' ? 'npm.cmd' : 'npm', - ['install', '-g', 'hermes-web-ui@latest'], - { + process.execPath, + [npmCli, 'install', '-g', 'hermes-web-ui@latest'], + expect.objectContaining({ encoding: 'utf-8', timeout: 10 * 60 * 1000, stdio: ['pipe', 'pipe', 'pipe'], - }, + env: expect.objectContaining({ + npm_node_execpath: process.execPath, + PATH: expect.stringContaining(`${nodeBinDir}${delimiter}`), + }), + }), ) expect(ctx.body).toEqual({ success: true, message: 'updated' }) vi.runAllTimers() - // Note: spawn is called with getGlobalCliBin() result + expect(mocks.execFileSync).toHaveBeenCalledWith( + process.execPath, + [npmCli, 'prefix', '-g'], + expect.objectContaining({ + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + env: expect.objectContaining({ npm_node_execpath: process.execPath }), + }), + ) expect(mocks.spawn).toHaveBeenCalledWith( - expect.any(String), // Dynamic path based on npm prefix -g - ['restart', '--port', '9129'], - { + process.execPath, + [cliScript, 'restart', '--port', '9129'], + expect.objectContaining({ detached: true, stdio: 'ignore', windowsHide: true, - }, + env: expect.objectContaining({ npm_node_execpath: process.execPath }), + }), ) expect(mocks.unref).toHaveBeenCalledOnce() expect(exitSpy).toHaveBeenCalledWith(0) @@ -90,8 +137,8 @@ describe('update controller', () => { vi.runAllTimers() expect(mocks.spawn).toHaveBeenCalledWith( - expect.any(String), - ['restart', '--port', '8648'], + process.execPath, + [expect.any(String), 'restart', '--port', '8648'], expect.objectContaining({ detached: true, stdio: 'ignore', windowsHide: true }), ) }) @@ -112,4 +159,20 @@ describe('update controller', () => { expect(mocks.spawn).not.toHaveBeenCalled() expect(exitSpy).not.toHaveBeenCalled() }) + + it('fails closed instead of falling back to PATH npm when the current Node install has no npm CLI', async () => { + const { handleUpdate, mocks } = await loadUpdateController({ existsSync: vi.fn(() => false) }) + const ctx = createMockCtx() + + await handleUpdate(ctx) + + expect(ctx.status).toBe(500) + expect(ctx.body).toEqual({ + success: false, + message: expect.stringContaining(`Unable to locate npm CLI for ${process.execPath}`), + }) + expect(mocks.execFileSync).not.toHaveBeenCalled() + expect(mocks.spawn).not.toHaveBeenCalled() + expect(exitSpy).not.toHaveBeenCalled() + }) })