diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index e7921748d..dced14e4e 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -127,7 +127,7 @@ import { MIN_WIDTH as WINDOW_MIN_WIDTH } from './window-state' import { hiddenWindowsChildOptions } from './windows-child-options' -import { buildPathExtCandidates, chooseUpdaterArgs, resolveVenvHermesCommand } from './windows-hermes-path' +import { buildPathExtCandidates, chooseUpdaterArgs, getVenvSitePackagesEntries, resolveVenvHermesCommand } from './windows-hermes-path' import { readWindowsUserEnvVar } from './windows-user-env' import { isPackagedInstallPath as isPackagedInstallPathUnderRoots } from './workspace-cwd' import { readWslWindowsClipboardImage } from './wsl-clipboard-image' @@ -1808,45 +1808,6 @@ function getVenvPython(venvRoot) { // normal HERMES_DASHBOARD_READY stdout line and no ready-file side channel is // needed. -function getVenvSitePackagesEntries(venvRoot) { - const entries = [] - - if (!venvRoot) { - return entries - } - - if (IS_WINDOWS) { - const sitePackages = path.join(venvRoot, 'Lib', 'site-packages') - - if (directoryExists(sitePackages)) { - entries.push(sitePackages) - } - - return entries - } - - const version = (() => { - try { - const cfg = fs.readFileSync(path.join(venvRoot, 'pyvenv.cfg'), 'utf8') - const match = cfg.match(/^version_info\s*=\s*(\d+\.\d+)/im) - - return match ? match[1].trim() : null - } catch { - return null - } - })() - - if (version) { - const sitePackages = path.join(venvRoot, 'lib', `python${version}`, 'site-packages') - - if (directoryExists(sitePackages)) { - entries.push(sitePackages) - } - } - - return entries -} - function makeDashboardReadyFile() { const dir = path.join(app.getPath('userData'), 'backend-ready') fs.mkdirSync(dir, { recursive: true }) diff --git a/apps/desktop/electron/windows-child-process.test.ts b/apps/desktop/electron/windows-child-process.test.ts deleted file mode 100644 index f81c60ce9..000000000 --- a/apps/desktop/electron/windows-child-process.test.ts +++ /dev/null @@ -1,118 +0,0 @@ -import assert from 'node:assert/strict' -import fs from 'node:fs' -import path from 'node:path' -import { test } from 'vitest' -import { fileURLToPath } from 'node:url' - -const ELECTRON_DIR = path.dirname(fileURLToPath(import.meta.url)) - -// TODO FIXME these tests all grep source code for specific things. This is an antipattern. -// Tests should NEVER read src, only assert behavior. - -function readElectronFile(name) { - return fs.readFileSync(path.join(ELECTRON_DIR, name), 'utf8').replace(/\r\n/g, '\n') -} - -function requireHiddenChildOptions(source, needle) { - const match = needle instanceof RegExp ? needle.exec(source) : null - const index = needle instanceof RegExp ? (match?.index ?? -1) : source.indexOf(needle) - assert.notEqual(index, -1, `missing call site: ${needle}`) - const snippet = source.slice(index, index + 700) - assert.match( - snippet, - /hiddenWindowsChildOptions\(/, - `expected ${needle} to wrap child-process options with hiddenWindowsChildOptions` - ) -} - -test('desktop background child processes opt into hidden Windows consoles', () => { - const source = readElectronFile('main.ts') - - assert.match(source, /function hiddenWindowsChildOptions\(options: any = \{\}\)/) - - requireHiddenChildOptions(source, "execFileSync(\n 'reg'") - requireHiddenChildOptions(source, /execFileSync\(\s*pyExe/) - requireHiddenChildOptions(source, /spawn\(\s*resolveGitBinary\(\)/) - requireHiddenChildOptions(source, "execFileSync('taskkill'") - requireHiddenChildOptions(source, /spawn\(\s*command,\s*args/) - requireHiddenChildOptions(source, "spawn('curl'") - requireHiddenChildOptions(source, /spawn\(\s*backend\.command,\s*backend\.args/) - requireHiddenChildOptions(source, /hermesProcess = spawn\(\s*backend\.command,\s*backend\.args/) - requireHiddenChildOptions(source, /spawn\(\s*py,\s*\['-m', 'hermes_cli\.main', 'uninstall', '--gui-summary'\]/) - - assert.match(source, /function unwrapWindowsVenvHermesCommand\(command, backendArgs\)/) - assert.match(source, /function getVenvSitePackagesEntries\(venvRoot\)/) - assert.match(source, /path\.join\(venvRoot, 'Lib', 'site-packages'\)/) - assert.match(source, /args: \['-m', 'hermes_cli\.main', \.\.\.backendArgs\]/) -}) - -test('desktop backend launches console python so child consoles are inherited, not pythonw', () => { - const source = readElectronFile('main.ts') - - // The flash fix is structural: the backend runs as a console-subsystem - // python.exe under hiddenWindowsChildOptions() (-> CREATE_NO_WINDOW), so it - // owns ONE windowless console that every descendant spawn inherits. Launching - // it as GUI-subsystem pythonw.exe is what made each child allocate (and flash) - // its own console, so the backend command must never be pythonw. - assert.doesNotMatch(source, /pythonw\.exe'\)/, 'backend must not be launched via pythonw.exe') - assert.doesNotMatch( - source, - /function getNoConsoleVenvPython\b/, - 'pythonw-conversion helper should be gone; console python is launched directly' - ) - assert.doesNotMatch( - source, - /function applyWindowsNoConsoleSpawnHints\b/, - 'pythonw spawn-hint rewriter should be gone' - ) - - // Console python restores stdout, so the port is announced on the normal - // HERMES_DASHBOARD_READY stdout line — no ready-file side channel is set. - assert.doesNotMatch(source, /readyFile: true/, 'no backend should opt into the pythonw ready-file path') - - // Both desktop backend launches must still go through hiddenWindowsChildOptions - // so the single backend console is created windowless. - requireHiddenChildOptions(source, /spawn\(\s*backend\.command,\s*backend\.args/) - requireHiddenChildOptions(source, /hermesProcess = spawn\(\s*backend\.command,\s*backend\.args/) -}) - -test('desktop backend teardown tree-kills Windows backend descendants', () => { - const source = readElectronFile('main.ts') - - const helperIndex = source.indexOf('function stopBackendChild(child)') - assert.notEqual(helperIndex, -1, 'missing backend teardown helper') - const helperSnippet = source.slice(helperIndex, helperIndex + 500) - assert.match(helperSnippet, /IS_WINDOWS && Number\.isInteger\(child\.pid\)/) - assert.match(helperSnippet, /forceKillProcessTree\(child\.pid\)/) - assert.match(helperSnippet, /child\.kill\('SIGTERM'\)/) - - const resetIndex = source.indexOf('function resetHermesConnection()') - assert.notEqual(resetIndex, -1, 'missing resetHermesConnection') - const resetSnippet = source.slice(resetIndex, resetIndex + 300) - assert.match(resetSnippet, /stopBackendChild\(hermesProcess\)/) - assert.doesNotMatch(resetSnippet, /hermesProcess\.kill\('SIGTERM'\)/) - - const quitIndex = source.indexOf("app.on('before-quit'") - assert.notEqual(quitIndex, -1, 'missing before-quit handler') - const quitSnippet = source.slice(quitIndex, quitIndex + 900) - assert.match(quitSnippet, /stopBackendChild\(hermesProcess\)/) - assert.doesNotMatch(quitSnippet, /hermesProcess\.kill\('SIGTERM'\)/) -}) - -test('intentional or interactive desktop child processes stay documented', () => { - const source = readElectronFile('main.ts') - - assert.match(source, /windowsHide: false/) - assert.match(source, /handOffWindowsBootstrapRecovery/) - assert.match(source, /'--repair', '--branch'/) - assert.match(source, /'--update', '--branch'/) - assert.match(source, /nodePty\.spawn\(command, args/) - assert.match(source, /spawn\('cmd\.exe', \['\/c', 'start'/) -}) - -test('bootstrap PowerShell runner hides Windows console children', () => { - const source = readElectronFile('bootstrap-runner.ts') - - assert.match(source, /function hiddenWindowsChildOptions\(options = \{\}\)/) - requireHiddenChildOptions(source, /spawn\(\s*ps,\s*fullArgs/) -}) diff --git a/apps/desktop/electron/windows-hermes-path.test.ts b/apps/desktop/electron/windows-hermes-path.test.ts index 71a673151..8600c3501 100644 --- a/apps/desktop/electron/windows-hermes-path.test.ts +++ b/apps/desktop/electron/windows-hermes-path.test.ts @@ -13,9 +13,11 @@ // re-selected forever instead of falling through to bootstrap. import assert from 'node:assert/strict' +import path from 'node:path' + import { test } from 'vitest' -import { buildPathExtCandidates, chooseUpdaterArgs, resolveVenvHermesCommand } from './windows-hermes-path' +import { buildPathExtCandidates, chooseUpdaterArgs, getVenvSitePackagesEntries, resolveVenvHermesCommand } from './windows-hermes-path' test('buildPathExtCandidates: Windows tries PATHEXT extensions before the empty extension', () => { const extensions = buildPathExtCandidates('.COM;.EXE;.BAT;.CMD', true) @@ -137,3 +139,71 @@ test('resolveVenvHermesCommand: is case-insensitive on hermes.exe and the Script assert.ok(resolveVenvHermesCommand('/root/venv/Scripts/HERMES.EXE', [], deps)) assert.ok(resolveVenvHermesCommand('/root/venv/SCRIPTS/hermes.exe', [], deps)) }) + +// ── getVenvSitePackagesEntries ───────────────────────────────────────────── + +test('getVenvSitePackagesEntries: returns Lib/site-packages on Windows when it exists', () => { + const expected = path.join('C:\\venv', 'Lib', 'site-packages') + + const result = getVenvSitePackagesEntries('C:\\venv', { + isWindows: true, + directoryExists: p => p === expected + }) + + assert.deepEqual(result, [expected]) +}) + +test('getVenvSitePackagesEntries: returns empty on Windows when site-packages does not exist', () => { + const result = getVenvSitePackagesEntries('C:\\venv', { + isWindows: true, + directoryExists: () => false + }) + + assert.deepEqual(result, []) +}) + +test('getVenvSitePackagesEntries: reads pyvenv.cfg version on POSIX and resolves lib/pythonX.Y/site-packages', () => { + const result = getVenvSitePackagesEntries('/venv', { + isWindows: false, + directoryExists: p => p === '/venv/lib/python3.12/site-packages', + readFile: () => 'version_info = 3.12.1\n' + }) + + assert.deepEqual(result, ['/venv/lib/python3.12/site-packages']) +}) + +test('getVenvSitePackagesEntries: returns empty on POSIX when pyvenv.cfg is missing', () => { + const result = getVenvSitePackagesEntries('/venv', { + isWindows: false, + directoryExists: () => true, + readFile: () => undefined + }) + + assert.deepEqual(result, []) +}) + +test('getVenvSitePackagesEntries: returns empty on POSIX when pyvenv.cfg has no version_info', () => { + const result = getVenvSitePackagesEntries('/venv', { + isWindows: false, + directoryExists: () => true, + readFile: () => 'home = /usr/bin\n' + }) + + assert.deepEqual(result, []) +}) + +test('getVenvSitePackagesEntries: returns empty on POSIX when version is present but site-packages dir is absent', () => { + const result = getVenvSitePackagesEntries('/venv', { + isWindows: false, + directoryExists: () => false, + readFile: () => 'version_info = 3.11\n' + }) + + assert.deepEqual(result, []) +}) + +test('getVenvSitePackagesEntries: returns empty for a falsy venvRoot', () => { + assert.deepEqual(getVenvSitePackagesEntries('', { isWindows: true, directoryExists: () => true }), []) + assert.deepEqual(getVenvSitePackagesEntries(null, { isWindows: true, directoryExists: () => true }), []) + assert.deepEqual(getVenvSitePackagesEntries(undefined, { isWindows: true, directoryExists: () => true }), []) +}) diff --git a/apps/desktop/electron/windows-hermes-path.ts b/apps/desktop/electron/windows-hermes-path.ts index c78ec614e..a40524910 100644 --- a/apps/desktop/electron/windows-hermes-path.ts +++ b/apps/desktop/electron/windows-hermes-path.ts @@ -30,6 +30,7 @@ * backend-command.ts. */ +import fs from 'node:fs' import path from 'node:path' /** @@ -79,6 +80,86 @@ export function chooseUpdaterArgs(haveRealInstall: boolean, branch: string): str return haveRealInstall ? ['--update', '--branch', branch] : ['--repair', '--branch', branch] } +/** + * Resolve the site-packages directory entries for a Python venv. + * + * On Windows, venv layout is `/Lib/site-packages`. + * On POSIX, it's `/lib/python/site-packages` where + * `` (e.g. `3.12`) is read from the venv's `pyvenv.cfg` + * `version_info` field. + * + * Returns only directories that actually exist on disk. Returns an empty + * array when `venvRoot` is falsy or no matching site-packages dir is found. + * + * Extracted from main.ts so the platform branching can be tested without + * reading source text. `isWindows` and `directoryExists` are injectable; + * `readFile` defaults to `fs.readFileSync` but can be overridden for tests. + */ +export function getVenvSitePackagesEntries( + venvRoot: string | undefined | null, + opts: { + isWindows?: boolean + directoryExists?: (p: string) => boolean + readFile?: (p: string) => string | undefined + } = {} +): string[] { + const entries: string[] = [] + + if (!venvRoot) { + return entries + } + + const isWindows = opts.isWindows ?? process.platform === 'win32' + + const directoryExists = opts.directoryExists ?? ((p: string) => { + try { + return fs.statSync(p).isDirectory() + } catch { + return false + } + }) + + const readFile = opts.readFile ?? ((p: string) => { + try { + return fs.readFileSync(p, 'utf8') + } catch { + return undefined + } + }) + + if (isWindows) { + const sitePackages = path.join(venvRoot, 'Lib', 'site-packages') + + if (directoryExists(sitePackages)) { + entries.push(sitePackages) + } + + return entries + } + + const cfg = readFile(path.join(venvRoot, 'pyvenv.cfg')) + + const version = (() => { + if (!cfg) { + return null + } + + const match = cfg.match(/^version_info\s*=\s*(\d+\.\d+)/im) + + return match ? match[1].trim() : null + })() + + if (version) { + const sitePackages = path.join(venvRoot, 'lib', `python${version}`, 'site-packages') + + if (directoryExists(sitePackages)) { + entries.push(sitePackages) + } + } + + return entries +} + export interface ResolveVenvHermesCommandDeps { isWindows: boolean isCommandScript: (command: string) => boolean