From f45f90aa47e3aaab0ef63961549be352cab64311 Mon Sep 17 00:00:00 2001 From: John Doe Date: Mon, 31 Aug 2026 12:13:57 +0300 Subject: [PATCH 1/2] fix(desktop): rc-noise-proof node resolution + spawn error dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Finder launch crashed with 'Uncaught Exception: spawn node ENOENT' on machines where node is installed only via nvm. The login-shell fallback took the LAST line of stdout from `$SHELL -lic 'command -v node'`, but rc files freely print to stdout — iTerm2 shell integration emits OSC 1337 escape sequences without a trailing newline, so the node path came back glued to escape garbage, existsSync() rejected it, and the app fell through to bare 'node', which the stripped GUI PATH cannot find. - Extract resolution into desktop/node-resolve.js (unit-testable; main.js requires electron) and probe the login shell with the same \x01-sentinel technique src/shell-path.js already uses, immune to rc noise. - Scan ~/.nvm/versions/node/*/bin/node (highest first) before spawning a shell at all — fixes nvm-only machines even when the probe fails, and skips the ~1s shell spawn on them. - Handle spawn 'error' in startServer: a clear dialog (install Node 18+ or set CODBASH_NODE) instead of an uncaught exception. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F3q8EdVFkKPTtSGhAU1Wxf --- desktop/main.js | 68 +++++--------- desktop/node-resolve.js | 98 ++++++++++++++++++++ desktop/package.json | 1 + test/desktop-node-resolve.test.js | 147 ++++++++++++++++++++++++++++++ 4 files changed, 270 insertions(+), 44 deletions(-) create mode 100644 desktop/node-resolve.js create mode 100644 test/desktop-node-resolve.test.js diff --git a/desktop/main.js b/desktop/main.js index 87b9b40..9b8ab03 100644 --- a/desktop/main.js +++ b/desktop/main.js @@ -12,9 +12,7 @@ const { spawn } = require('child_process'); const http = require('http'); const net = require('net'); const path = require('path'); -const fs = require('fs'); -const os = require('os'); -const { execFileSync } = require('child_process'); +const { resolveNodeBin } = require('./node-resolve.js'); let serverProc = null; let win = null; @@ -66,49 +64,17 @@ function resolveServerEntry() { return app.isPackaged ? packaged : dev; } -// The Node binary used to run the server. We deliberately avoid Electron's own -// Node (ELECTRON_RUN_AS_NODE) because its ABI differs from the prebuilt -// node-pty. -// -// A Finder/`open`-launched macOS app inherits only a minimal PATH -// (/usr/bin:/bin:/usr/sbin:/sbin), so bare "node" (installed via nvm, Homebrew, -// conda, etc.) usually isn't found. We therefore resolve an absolute path: -// explicit override → bundled node → common install locations → the user's -// login shell → bare "node" as a last resort. -function resolveNodeBin() { - if (process.env.CODBASH_NODE) return process.env.CODBASH_NODE; - - const bundled = path.join(process.resourcesPath || '', process.platform === 'win32' ? 'node.exe' : 'node'); - try { if (app.isPackaged && fs.existsSync(bundled)) return bundled; } catch (_e) {} - - if (process.platform === 'win32') return 'node.exe'; - - const home = os.homedir(); - const candidates = [ - '/opt/homebrew/bin/node', - '/usr/local/bin/node', - '/usr/bin/node', - path.join(home, '.local/bin/node'), - path.join(home, '.volta/bin/node'), - ]; - for (const c of candidates) { - try { if (fs.existsSync(c)) return c; } catch (_e) {} - } - - // Ask the user's login shell (picks up nvm/conda/asdf shims a plain env misses). - try { - const shell = process.env.SHELL || '/bin/zsh'; - const out = execFileSync(shell, ['-lic', 'command -v node'], { encoding: 'utf8', timeout: 6000 }); - const p = out.split('\n').map(function (s) { return s.trim(); }).filter(Boolean).pop(); - if (p && fs.existsSync(p)) return p; - } catch (_e) {} - - return 'node'; -} - function startServer(port) { const entry = resolveServerEntry(); - const nodeBin = resolveNodeBin(); + // We deliberately avoid Electron's own Node (ELECTRON_RUN_AS_NODE) because + // its ABI differs from the prebuilt node-pty. See node-resolve.js for the + // resolution order and the rc-noise-proof login-shell probe. + const nodeBin = resolveNodeBin({ + env: process.env, + platform: process.platform, + resourcesPath: process.resourcesPath, + isPackaged: app.isPackaged, + }); serverProc = spawn(nodeBin, [entry, 'run', '--port=' + port, '--host=127.0.0.1', '--no-browser'], { // CODBASH_DESKTOP=1 tells the server it runs inside the Electron shell, so the // web self-update route (`POST /api/update` → `npm i -g`) refuses: it would @@ -119,6 +85,20 @@ function startServer(port) { }); serverProc.stdout.on('data', function (d) { process.stdout.write('[codbash] ' + d); }); serverProc.stderr.on('data', function (d) { process.stderr.write('[codbash] ' + d); }); + // spawn failures (ENOENT when no node binary was found) emit 'error', not + // 'exit' — without this handler they surface as an Uncaught Exception dialog. + serverProc.on('error', function (err) { + serverProc = null; + if (!app.isQuitting && !SMOKE) { + dialog.showErrorBox( + 'codbash could not start', + 'Failed to launch the codbash server with "' + nodeBin + '": ' + err.message + + '\n\nInstall Node.js 18+ (https://nodejs.org), or point the CODBASH_NODE ' + + 'environment variable at your node binary, then relaunch codbash.' + ); + } + app.quit(); + }); serverProc.on('exit', function (code) { serverProc = null; if (!app.isQuitting && !SMOKE) { diff --git a/desktop/node-resolve.js b/desktop/node-resolve.js new file mode 100644 index 0000000..bb3e074 --- /dev/null +++ b/desktop/node-resolve.js @@ -0,0 +1,98 @@ +// Locate the Node binary that runs the bundled codbash server. +// +// Extracted from main.js (which requires electron and can't be unit-tested +// under `node --test`). The resolution order is: explicit override → bundled +// node → common install locations → nvm-installed versions → the user's login +// shell → bare "node" as a last resort. +// +// The login-shell probe deliberately mirrors src/shell-path.js: rc files +// freely print to stdout (oh-my-zsh warnings, iTerm2 OSC 1337 escape +// sequences without a trailing newline), so "take the last line of stdout" +// returned the node path glued to escape garbage and the fallback silently +// failed. Wrapping the answer in \x01 sentinels and extracting by regex makes +// the probe immune to any rc noise. +'use strict'; + +const { execFileSync } = require('child_process'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const PROBE_SCRIPT = 'printf "\\1CBN\\1%s\\1CBE\\1" "$(command -v node 2>/dev/null)"'; + +function extractProbedNodePath(raw) { + const m = /\x01CBN\x01([\s\S]*?)\x01CBE\x01/.exec(raw || ''); + if (!m) return ''; + const p = m[1].trim(); + return p && path.isAbsolute(p) ? p : ''; +} + +// Installed nvm nodes (~/.nvm/versions/node/vX.Y.Z/bin/node), highest version +// first. Numeric compare — lexicographic would rank v8 above v20. +function listNvmNodes(home) { + const root = path.join(home, '.nvm', 'versions', 'node'); + let entries; + try { entries = fs.readdirSync(root); } catch (_e) { return []; } + return entries + .map((name) => ({ name, parts: /^v(\d+)\.(\d+)\.(\d+)$/.exec(name) })) + .filter((e) => e.parts) + .sort((a, b) => + (b.parts[1] - a.parts[1]) || (b.parts[2] - a.parts[2]) || (b.parts[3] - a.parts[3])) + .map((e) => path.join(root, e.name, 'bin', 'node')) + .filter((p) => { try { return fs.existsSync(p); } catch (_e) { return false; } }); +} + +// Ask the user's login shell where node lives (picks up nvm/conda/asdf shims a +// plain env misses). Flags and limits follow src/shell-path.js: `-i -l -c` so +// PATH matches a real terminal, SIGKILL because an rc that traps SIGTERM could +// outlive the timeout, stderr ignored so rc noise doesn't leak into our logs. +function probeLoginShellForNode(env) { + const shell = (env.SHELL && path.isAbsolute(env.SHELL)) ? env.SHELL : '/bin/zsh'; + try { + const raw = execFileSync(shell, ['-i', '-l', '-c', PROBE_SCRIPT], { + encoding: 'utf8', + timeout: 6000, + killSignal: 'SIGKILL', + stdio: ['ignore', 'pipe', 'ignore'], + }); + return extractProbedNodePath(raw); + } catch (_e) { + return ''; + } +} + +// Options exist for tests only; production callers pass the real process/app +// values (see main.js). `existsSync` and `probe` default to the real thing. +function resolveNodeBin(o) { + o = o || {}; + const env = o.env || process.env; + const platform = o.platform || process.platform; + const exists = o.existsSync || fs.existsSync; + + if (env.CODBASH_NODE) return env.CODBASH_NODE; + + const bundled = path.join(o.resourcesPath || '', platform === 'win32' ? 'node.exe' : 'node'); + try { if (o.isPackaged && exists(bundled)) return bundled; } catch (_e) {} + + if (platform === 'win32') return 'node.exe'; + + const home = o.home || os.homedir(); + const candidates = [ + '/opt/homebrew/bin/node', + '/usr/local/bin/node', + '/usr/bin/node', + path.join(home, '.local/bin/node'), + path.join(home, '.volta/bin/node'), + ...listNvmNodes(home), + ]; + for (const c of candidates) { + try { if (exists(c)) return c; } catch (_e) {} + } + + const probed = (o.probe || probeLoginShellForNode)(env); + try { if (probed && exists(probed)) return probed; } catch (_e) {} + + return 'node'; +} + +module.exports = { PROBE_SCRIPT, extractProbedNodePath, listNvmNodes, resolveNodeBin }; diff --git a/desktop/package.json b/desktop/package.json index fa19b94..1d29b6a 100644 --- a/desktop/package.json +++ b/desktop/package.json @@ -32,6 +32,7 @@ "afterSign": "scripts/notarize.js", "files": [ "main.js", + "node-resolve.js", "preload.js" ], "extraResources": [ diff --git a/test/desktop-node-resolve.test.js b/test/desktop-node-resolve.test.js new file mode 100644 index 0000000..9722301 --- /dev/null +++ b/test/desktop-node-resolve.test.js @@ -0,0 +1,147 @@ +// Unit tests for desktop/node-resolve.js — locating the Node binary that runs +// the bundled server (extracted from desktop/main.js, which requires electron +// and therefore cannot be loaded under `node --test`). +// +// The bug this pins down: the login-shell fallback used to run +// `$SHELL -lic 'command -v node'` and take the LAST line of stdout. Shell rc +// files freely print to stdout — iTerm2 shell integration emits OSC 1337 +// escape sequences WITHOUT a trailing newline, so the node path came back +// glued to escape garbage, existsSync() rejected it, and the app fell through +// to bare `node`, which a Finder launch (PATH=/usr/bin:/bin:/usr/sbin:/sbin) +// cannot find → "Uncaught Exception: spawn node ENOENT". The fix mirrors +// src/shell-path.js: wrap the answer in \x01 sentinels and extract by regex, +// so rc noise can never contaminate it. + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const { extractProbedNodePath, listNvmNodes, resolveNodeBin } = require('../desktop/node-resolve.js'); + +const NVM_NODE = '/Users/u/.nvm/versions/node/v20.20.0/bin/node'; +const wrap = (s) => '\x01CBN\x01' + s + '\x01CBE\x01'; + +// --------------------------------------------------------------------------- +// extractProbedNodePath +// --------------------------------------------------------------------------- + +test('extractProbedNodePath survives iTerm2 OSC escape noise glued around the sentinel (the original crash)', () => { + // Real stdout captured on the machine where the app crashed: oh-my-zsh + // warning, then three OSC 1337 sequences with no trailing newline, then the + // probe answer on the same "line". + const raw = + "[oh-my-zsh] plugin 'fig' not found\n" + + '\x1b]1337;RemoteHost=u@mac.local\x07' + + '\x1b]1337;CurrentDir=/Users/u\x07' + + '\x1b]1337;ShellIntegrationVersion=14;shell=zsh\x07' + + wrap(NVM_NODE) + + '\x1b]1337;After=1\x07'; + assert.equal(extractProbedNodePath(raw), NVM_NODE); +}); + +test('extractProbedNodePath trims whitespace inside the sentinels', () => { + assert.equal(extractProbedNodePath(wrap('\n ' + NVM_NODE + ' \n')), NVM_NODE); +}); + +test('extractProbedNodePath returns empty for empty/missing/relative answers', () => { + assert.equal(extractProbedNodePath(''), ''); + assert.equal(extractProbedNodePath(undefined), ''); + assert.equal(extractProbedNodePath('no sentinels here'), ''); + assert.equal(extractProbedNodePath(wrap('')), ''); // node not installed + assert.equal(extractProbedNodePath(wrap('node')), ''); // not absolute +}); + +// --------------------------------------------------------------------------- +// listNvmNodes +// --------------------------------------------------------------------------- + +function makeFakeHome(versions) { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'codbash-nvm-')); + for (const v of versions) { + const bin = path.join(home, '.nvm', 'versions', 'node', v, 'bin'); + fs.mkdirSync(bin, { recursive: true }); + fs.writeFileSync(path.join(bin, 'node'), ''); + } + return home; +} + +test('listNvmNodes returns installed nvm nodes, highest version first (numeric, not lexicographic)', () => { + // Lexicographic order would put v8 above v20 — the sort must be numeric. + const home = makeFakeHome(['v8.17.0', 'v20.20.0', 'v18.19.1']); + const got = listNvmNodes(home); + assert.deepEqual(got, [ + path.join(home, '.nvm', 'versions', 'node', 'v20.20.0', 'bin', 'node'), + path.join(home, '.nvm', 'versions', 'node', 'v18.19.1', 'bin', 'node'), + path.join(home, '.nvm', 'versions', 'node', 'v8.17.0', 'bin', 'node'), + ]); +}); + +test('listNvmNodes returns [] when nvm is absent', () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'codbash-nonvm-')); + assert.deepEqual(listNvmNodes(home), []); +}); + +// --------------------------------------------------------------------------- +// resolveNodeBin (all I/O injected) +// --------------------------------------------------------------------------- + +const noneExist = () => false; + +test('resolveNodeBin: CODBASH_NODE override wins over everything', () => { + const got = resolveNodeBin({ + env: { CODBASH_NODE: '/custom/node' }, + platform: 'darwin', + home: '/nonexistent', + existsSync: noneExist, + probe: () => { throw new Error('probe must not run'); }, + }); + assert.equal(got, '/custom/node'); +}); + +test('resolveNodeBin: finds an nvm-installed node when no standard location has one', () => { + const home = makeFakeHome(['v20.20.0']); + const expected = path.join(home, '.nvm', 'versions', 'node', 'v20.20.0', 'bin', 'node'); + const got = resolveNodeBin({ + env: {}, + platform: 'darwin', + home, + existsSync: fs.existsSync, // standard candidates don't exist under this fake home + probe: () => { throw new Error('probe must not run when nvm node exists'); }, + }); + assert.equal(got, expected); +}); + +test('resolveNodeBin: falls back to the sentinel login-shell probe', () => { + const got = resolveNodeBin({ + env: {}, + platform: 'darwin', + home: '/nonexistent', + existsSync: (p) => p === '/opt/weird/bin/node', + probe: () => '/opt/weird/bin/node', + }); + assert.equal(got, '/opt/weird/bin/node'); +}); + +test('resolveNodeBin: probe answers pointing at nonexistent files are rejected', () => { + const got = resolveNodeBin({ + env: {}, + platform: 'darwin', + home: '/nonexistent', + existsSync: noneExist, + probe: () => '/gone/node', + }); + assert.equal(got, 'node'); +}); + +test('resolveNodeBin: last resort is bare "node"', () => { + const got = resolveNodeBin({ + env: {}, + platform: 'darwin', + home: '/nonexistent', + existsSync: noneExist, + probe: () => '', + }); + assert.equal(got, 'node'); +}); From 98dab17d6b545eb6a786e4f9b8bed6911dc2f08a Mon Sep 17 00:00:00 2001 From: John Doe Date: Mon, 31 Aug 2026 12:25:39 +0300 Subject: [PATCH 2/2] =?UTF-8?q?fix(desktop):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20reuse=20shell-path=20probe,=20honor=20nvm=20default?= =?UTF-8?q?,=20guard=20dialogs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Drop the duplicated sentinel probe: reuse src/shell-path.js's cached, logged captureLoginShellPath() (dev: ../src, packaged: extraResources app/src) and scan its PATH for node. - listNvmNodes: put ~/.nvm/alias/default (exact or prefix form) first so the scan never silently overrides the user's default with a newer install (prebuilt node-pty ABI may not match); drop the double stat — the candidate loop owns the existence check via the injected seam. - startServer 'error' handler: return early during a quit in progress; in SMOKE mode fail loudly with a nonzero exit instead of a silent green run. - whenReady catch: skip the generic 'failed to start' dialog when the spawn error dialog already told the user what's wrong. - Tests: scope injected existsSync fakes to the fake home so the suite doesn't depend on the host machine's /usr/local/bin/node (CI runners all have one). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F3q8EdVFkKPTtSGhAU1Wxf --- desktop/main.js | 29 +++++-- desktop/node-resolve.js | 107 ++++++++++++++--------- test/desktop-node-resolve.test.js | 136 +++++++++++++++++++----------- 3 files changed, 174 insertions(+), 98 deletions(-) diff --git a/desktop/main.js b/desktop/main.js index 9b8ab03..c1ae836 100644 --- a/desktop/main.js +++ b/desktop/main.js @@ -15,6 +15,7 @@ const path = require('path'); const { resolveNodeBin } = require('./node-resolve.js'); let serverProc = null; +let serverSpawnFailed = false; // set by the spawn 'error' handler; suppresses the later waitForServer dialog let win = null; let serverPort = 0; const SMOKE = !!process.env.CODBASH_SMOKE; // launch, verify, auto-quit (CI/local test) @@ -89,14 +90,21 @@ function startServer(port) { // 'exit' — without this handler they surface as an Uncaught Exception dialog. serverProc.on('error', function (err) { serverProc = null; - if (!app.isQuitting && !SMOKE) { - dialog.showErrorBox( - 'codbash could not start', - 'Failed to launch the codbash server with "' + nodeBin + '": ' + err.message + - '\n\nInstall Node.js 18+ (https://nodejs.org), or point the CODBASH_NODE ' + - 'environment variable at your node binary, then relaunch codbash.' - ); + serverSpawnFailed = true; + if (app.isQuitting) return; + if (SMOKE) { + // A silent green smoke run for a broken launch would be worse than the + // crash — fail loudly and nonzero. + process.stderr.write('[desktop] SMOKE FAIL — server spawn error: ' + err.message + '\n'); + app.exit(1); + return; } + dialog.showErrorBox( + 'codbash could not start', + 'Failed to launch the codbash server with "' + nodeBin + '": ' + err.message + + '\n\nInstall Node.js 18+ (https://nodejs.org), or point the CODBASH_NODE ' + + 'environment variable at your node binary, then relaunch codbash.' + ); app.quit(); }); serverProc.on('exit', function (code) { @@ -347,7 +355,12 @@ app.whenReady().then(async function () { await createWindow(); initAutoUpdater(); } catch (e) { - dialog.showErrorBox('codbash failed to start', String((e && e.message) || e)); + // The spawn 'error' handler already showed a specific dialog (missing node + // binary) — a second "did not become ready" dialog would point the user at + // a nonexistent server problem. Same for a quit already in progress. + if (!app.isQuitting && !serverSpawnFailed) { + dialog.showErrorBox('codbash failed to start', String((e && e.message) || e)); + } app.isQuitting = true; app.quit(); return; diff --git a/desktop/node-resolve.js b/desktop/node-resolve.js index bb3e074..cdcb924 100644 --- a/desktop/node-resolve.js +++ b/desktop/node-resolve.js @@ -2,67 +2,85 @@ // // Extracted from main.js (which requires electron and can't be unit-tested // under `node --test`). The resolution order is: explicit override → bundled -// node → common install locations → nvm-installed versions → the user's login -// shell → bare "node" as a last resort. +// node → common install locations → nvm-installed versions (the user's +// default alias first) → the user's login-shell PATH → bare "node" as a last +// resort. // -// The login-shell probe deliberately mirrors src/shell-path.js: rc files -// freely print to stdout (oh-my-zsh warnings, iTerm2 OSC 1337 escape -// sequences without a trailing newline), so "take the last line of stdout" -// returned the node path glued to escape garbage and the fallback silently -// failed. Wrapping the answer in \x01 sentinels and extracting by regex makes -// the probe immune to any rc noise. +// The login-shell step deliberately reuses src/shell-path.js rather than +// spawning its own probe: rc files freely print to stdout (oh-my-zsh +// warnings, iTerm2 OSC 1337 escape sequences without a trailing newline), so +// the old `$SHELL -lic 'command -v node'` + "take the last line" approach +// returned the node path glued to escape garbage and silently failed. +// shell-path.js already solves this with an \x01-sentinel probe, logs probe +// failures, and caches the ~1s interactive shell spawn on disk for a day — +// duplicating that machinery here would just drift. 'use strict'; -const { execFileSync } = require('child_process'); const fs = require('fs'); const os = require('os'); const path = require('path'); -const PROBE_SCRIPT = 'printf "\\1CBN\\1%s\\1CBE\\1" "$(command -v node 2>/dev/null)"'; - -function extractProbedNodePath(raw) { - const m = /\x01CBN\x01([\s\S]*?)\x01CBE\x01/.exec(raw || ''); - if (!m) return ''; - const p = m[1].trim(); - return p && path.isAbsolute(p) ? p : ''; +// First PATH entry that holds a node binary ('' when none). Relative entries +// are skipped — a poisoned PATH must not make us spawn ./node. +function findNodeInPathString(pathString, exists) { + for (const dir of String(pathString || '').split(path.delimiter)) { + if (!dir || !path.isAbsolute(dir)) continue; + const p = path.join(dir, 'node'); + try { if (exists(p)) return p; } catch (_e) {} + } + return ''; } -// Installed nvm nodes (~/.nvm/versions/node/vX.Y.Z/bin/node), highest version -// first. Numeric compare — lexicographic would rank v8 above v20. +// Installed nvm nodes (~/.nvm/versions/node/vX.Y.Z/bin/node). The version the +// user actually runs — ~/.nvm/alias/default, exact ("v20.20.0") or prefix +// ("20", "20.3") form — comes first so we never silently override their +// default with a newer install (the prebuilt node-pty ABI may not match it); +// the rest follow highest-first (numeric compare — lexicographic would rank +// v8 above v20). Existence of bin/node is left to the caller's candidate +// loop, which owns the injected existsSync seam. function listNvmNodes(home) { const root = path.join(home, '.nvm', 'versions', 'node'); let entries; try { entries = fs.readdirSync(root); } catch (_e) { return []; } - return entries + + const versions = entries .map((name) => ({ name, parts: /^v(\d+)\.(\d+)\.(\d+)$/.exec(name) })) .filter((e) => e.parts) .sort((a, b) => - (b.parts[1] - a.parts[1]) || (b.parts[2] - a.parts[2]) || (b.parts[3] - a.parts[3])) - .map((e) => path.join(root, e.name, 'bin', 'node')) - .filter((p) => { try { return fs.existsSync(p); } catch (_e) { return false; } }); -} + (b.parts[1] - a.parts[1]) || (b.parts[2] - a.parts[2]) || (b.parts[3] - a.parts[3])); -// Ask the user's login shell where node lives (picks up nvm/conda/asdf shims a -// plain env misses). Flags and limits follow src/shell-path.js: `-i -l -c` so -// PATH matches a real terminal, SIGKILL because an rc that traps SIGTERM could -// outlive the timeout, stderr ignored so rc noise doesn't leak into our logs. -function probeLoginShellForNode(env) { - const shell = (env.SHELL && path.isAbsolute(env.SHELL)) ? env.SHELL : '/bin/zsh'; - try { - const raw = execFileSync(shell, ['-i', '-l', '-c', PROBE_SCRIPT], { - encoding: 'utf8', - timeout: 6000, - killSignal: 'SIGKILL', - stdio: ['ignore', 'pipe', 'ignore'], + let alias = ''; + try { alias = fs.readFileSync(path.join(home, '.nvm', 'alias', 'default'), 'utf8').trim(); } catch (_e) {} + if (alias) { + const want = alias.replace(/^v/, ''); + // Highest install matching the alias exactly or by prefix ("20" → v20.20.0). + const i = versions.findIndex((e) => { + const have = e.name.slice(1); + return have === want || have.startsWith(want + '.'); }); - return extractProbedNodePath(raw); - } catch (_e) { - return ''; + if (i > 0) versions.unshift(versions.splice(i, 1)[0]); } + + return versions.map((e) => path.join(root, e.name, 'bin', 'node')); +} + +// src/shell-path.js ships beside the server: repo layout in dev, +// extraResources (app/src) when packaged — mirroring resolveServerEntry in +// main.js. Electron can require() it from outside the asar. +function loadShellPathModule(o) { + const candidates = [ + o.isPackaged && o.resourcesPath ? path.join(o.resourcesPath, 'app', 'src', 'shell-path.js') : null, + path.join(__dirname, '..', 'src', 'shell-path.js'), + ].filter(Boolean); + for (const c of candidates) { + try { return require(c); } catch (_e) {} + } + return null; } // Options exist for tests only; production callers pass the real process/app -// values (see main.js). `existsSync` and `probe` default to the real thing. +// values (see main.js). `existsSync` and `shellPathModule` default to the +// real thing. function resolveNodeBin(o) { o = o || {}; const env = o.env || process.env; @@ -89,10 +107,15 @@ function resolveNodeBin(o) { try { if (exists(c)) return c; } catch (_e) {} } - const probed = (o.probe || probeLoginShellForNode)(env); - try { if (probed && exists(probed)) return probed; } catch (_e) {} + const shellPath = 'shellPathModule' in o ? o.shellPathModule : loadShellPathModule(o); + if (shellPath) { + try { + const found = findNodeInPathString(shellPath.captureLoginShellPath(), exists); + if (found) return found; + } catch (_e) {} // probe failures are logged inside shell-path.js + } return 'node'; } -module.exports = { PROBE_SCRIPT, extractProbedNodePath, listNvmNodes, resolveNodeBin }; +module.exports = { findNodeInPathString, listNvmNodes, resolveNodeBin }; diff --git a/test/desktop-node-resolve.test.js b/test/desktop-node-resolve.test.js index 9722301..f8a2387 100644 --- a/test/desktop-node-resolve.test.js +++ b/test/desktop-node-resolve.test.js @@ -8,9 +8,14 @@ // escape sequences WITHOUT a trailing newline, so the node path came back // glued to escape garbage, existsSync() rejected it, and the app fell through // to bare `node`, which a Finder launch (PATH=/usr/bin:/bin:/usr/sbin:/sbin) -// cannot find → "Uncaught Exception: spawn node ENOENT". The fix mirrors -// src/shell-path.js: wrap the answer in \x01 sentinels and extract by regex, -// so rc noise can never contaminate it. +// cannot find → "Uncaught Exception: spawn node ENOENT". The fix reuses +// src/shell-path.js's rc-noise-proof sentinel probe (see its own tests) and +// scans the captured PATH for a node binary. +// +// NOTE: injected existsSync fakes are scoped to the fake home on purpose — +// resolveNodeBin checks absolute standard paths (/usr/local/bin/node, +// /opt/homebrew/bin/node, ...) first, and a bare fs.existsSync would make +// these tests depend on what the host machine has installed. const test = require('node:test'); const assert = require('node:assert/strict'); @@ -18,63 +23,82 @@ const fs = require('fs'); const os = require('os'); const path = require('path'); -const { extractProbedNodePath, listNvmNodes, resolveNodeBin } = require('../desktop/node-resolve.js'); - -const NVM_NODE = '/Users/u/.nvm/versions/node/v20.20.0/bin/node'; -const wrap = (s) => '\x01CBN\x01' + s + '\x01CBE\x01'; +const { findNodeInPathString, listNvmNodes, resolveNodeBin } = require('../desktop/node-resolve.js'); // --------------------------------------------------------------------------- -// extractProbedNodePath +// findNodeInPathString // --------------------------------------------------------------------------- -test('extractProbedNodePath survives iTerm2 OSC escape noise glued around the sentinel (the original crash)', () => { - // Real stdout captured on the machine where the app crashed: oh-my-zsh - // warning, then three OSC 1337 sequences with no trailing newline, then the - // probe answer on the same "line". - const raw = - "[oh-my-zsh] plugin 'fig' not found\n" + - '\x1b]1337;RemoteHost=u@mac.local\x07' + - '\x1b]1337;CurrentDir=/Users/u\x07' + - '\x1b]1337;ShellIntegrationVersion=14;shell=zsh\x07' + - wrap(NVM_NODE) + - '\x1b]1337;After=1\x07'; - assert.equal(extractProbedNodePath(raw), NVM_NODE); +test('findNodeInPathString returns the first PATH dir that holds a node binary', () => { + const exists = (p) => p === path.join('/x/bin', 'node') || p === path.join('/y/bin', 'node'); + assert.equal(findNodeInPathString('/usr/bin:/x/bin:/y/bin', exists), path.join('/x/bin', 'node')); }); -test('extractProbedNodePath trims whitespace inside the sentinels', () => { - assert.equal(extractProbedNodePath(wrap('\n ' + NVM_NODE + ' \n')), NVM_NODE); +test('findNodeInPathString skips empty and relative entries', () => { + const exists = (p) => p === path.join('/abs/bin', 'node'); + assert.equal(findNodeInPathString('::rel/bin:./also/rel:/abs/bin', exists), path.join('/abs/bin', 'node')); }); -test('extractProbedNodePath returns empty for empty/missing/relative answers', () => { - assert.equal(extractProbedNodePath(''), ''); - assert.equal(extractProbedNodePath(undefined), ''); - assert.equal(extractProbedNodePath('no sentinels here'), ''); - assert.equal(extractProbedNodePath(wrap('')), ''); // node not installed - assert.equal(extractProbedNodePath(wrap('node')), ''); // not absolute +test('findNodeInPathString returns empty when no dir has node', () => { + assert.equal(findNodeInPathString('/a:/b', () => false), ''); + assert.equal(findNodeInPathString('', () => true), ''); }); // --------------------------------------------------------------------------- // listNvmNodes // --------------------------------------------------------------------------- -function makeFakeHome(versions) { +function makeFakeHome(versions, defaultAlias) { const home = fs.mkdtempSync(path.join(os.tmpdir(), 'codbash-nvm-')); for (const v of versions) { const bin = path.join(home, '.nvm', 'versions', 'node', v, 'bin'); fs.mkdirSync(bin, { recursive: true }); fs.writeFileSync(path.join(bin, 'node'), ''); } + if (defaultAlias != null) { + const aliasDir = path.join(home, '.nvm', 'alias'); + fs.mkdirSync(aliasDir, { recursive: true }); + fs.writeFileSync(path.join(aliasDir, 'default'), defaultAlias + '\n'); + } return home; } +const nvmBin = (home, v) => path.join(home, '.nvm', 'versions', 'node', v, 'bin', 'node'); + test('listNvmNodes returns installed nvm nodes, highest version first (numeric, not lexicographic)', () => { // Lexicographic order would put v8 above v20 — the sort must be numeric. const home = makeFakeHome(['v8.17.0', 'v20.20.0', 'v18.19.1']); - const got = listNvmNodes(home); - assert.deepEqual(got, [ - path.join(home, '.nvm', 'versions', 'node', 'v20.20.0', 'bin', 'node'), - path.join(home, '.nvm', 'versions', 'node', 'v18.19.1', 'bin', 'node'), - path.join(home, '.nvm', 'versions', 'node', 'v8.17.0', 'bin', 'node'), + assert.deepEqual(listNvmNodes(home), [ + nvmBin(home, 'v20.20.0'), + nvmBin(home, 'v18.19.1'), + nvmBin(home, 'v8.17.0'), + ]); +}); + +test('listNvmNodes puts the ~/.nvm/alias/default version first, not the highest', () => { + // The user runs v20 by default (matching the prebuilt node-pty ABI); v24 is + // merely installed. The scan must not silently override their default. + const home = makeFakeHome(['v24.1.0', 'v20.20.0'], 'v20.20.0'); + assert.deepEqual(listNvmNodes(home), [ + nvmBin(home, 'v20.20.0'), + nvmBin(home, 'v24.1.0'), + ]); +}); + +test('listNvmNodes resolves a major-only default alias ("20") to the highest matching install', () => { + const home = makeFakeHome(['v24.1.0', 'v20.20.0', 'v20.3.0'], '20'); + assert.deepEqual(listNvmNodes(home), [ + nvmBin(home, 'v20.20.0'), + nvmBin(home, 'v24.1.0'), + nvmBin(home, 'v20.3.0'), + ]); +}); + +test('listNvmNodes falls back to highest-first when the default alias is unresolvable', () => { + const home = makeFakeHome(['v20.20.0', 'v18.19.1'], 'lts/hydrogen'); + assert.deepEqual(listNvmNodes(home), [ + nvmBin(home, 'v20.20.0'), + nvmBin(home, 'v18.19.1'), ]); }); @@ -84,10 +108,15 @@ test('listNvmNodes returns [] when nvm is absent', () => { }); // --------------------------------------------------------------------------- -// resolveNodeBin (all I/O injected) +// resolveNodeBin (all I/O injected; existsSync scoped to the fake home — see +// note at the top of the file) // --------------------------------------------------------------------------- const noneExist = () => false; +const underHome = (home) => (p) => p.startsWith(home) && fs.existsSync(p); +const probelessShellPath = { + captureLoginShellPath: () => { throw new Error('login-shell probe must not run'); }, +}; test('resolveNodeBin: CODBASH_NODE override wins over everything', () => { const got = resolveNodeBin({ @@ -95,53 +124,64 @@ test('resolveNodeBin: CODBASH_NODE override wins over everything', () => { platform: 'darwin', home: '/nonexistent', existsSync: noneExist, - probe: () => { throw new Error('probe must not run'); }, + shellPathModule: probelessShellPath, }); assert.equal(got, '/custom/node'); }); test('resolveNodeBin: finds an nvm-installed node when no standard location has one', () => { const home = makeFakeHome(['v20.20.0']); - const expected = path.join(home, '.nvm', 'versions', 'node', 'v20.20.0', 'bin', 'node'); const got = resolveNodeBin({ env: {}, platform: 'darwin', home, - existsSync: fs.existsSync, // standard candidates don't exist under this fake home - probe: () => { throw new Error('probe must not run when nvm node exists'); }, + existsSync: underHome(home), + shellPathModule: probelessShellPath, + }); + assert.equal(got, nvmBin(home, 'v20.20.0')); +}); + +test('resolveNodeBin: honors the nvm default alias over a higher installed version', () => { + const home = makeFakeHome(['v24.1.0', 'v20.20.0'], 'v20.20.0'); + const got = resolveNodeBin({ + env: {}, + platform: 'darwin', + home, + existsSync: underHome(home), + shellPathModule: probelessShellPath, }); - assert.equal(got, expected); + assert.equal(got, nvmBin(home, 'v20.20.0')); }); -test('resolveNodeBin: falls back to the sentinel login-shell probe', () => { +test('resolveNodeBin: falls back to scanning the login-shell PATH for node', () => { const got = resolveNodeBin({ env: {}, platform: 'darwin', home: '/nonexistent', - existsSync: (p) => p === '/opt/weird/bin/node', - probe: () => '/opt/weird/bin/node', + existsSync: (p) => p === path.join('/opt/weird/bin', 'node'), + shellPathModule: { captureLoginShellPath: () => '/usr/bin:/opt/weird/bin' }, }); - assert.equal(got, '/opt/weird/bin/node'); + assert.equal(got, path.join('/opt/weird/bin', 'node')); }); -test('resolveNodeBin: probe answers pointing at nonexistent files are rejected', () => { +test('resolveNodeBin: last resort is bare "node" (probe throws, nothing exists)', () => { const got = resolveNodeBin({ env: {}, platform: 'darwin', home: '/nonexistent', existsSync: noneExist, - probe: () => '/gone/node', + shellPathModule: { captureLoginShellPath: () => { throw new Error('rc exploded'); } }, }); assert.equal(got, 'node'); }); -test('resolveNodeBin: last resort is bare "node"', () => { +test('resolveNodeBin: last resort is bare "node" when the shell-path module is unavailable', () => { const got = resolveNodeBin({ env: {}, platform: 'darwin', home: '/nonexistent', existsSync: noneExist, - probe: () => '', + shellPathModule: null, }); assert.equal(got, 'node'); });