golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix(install): tokenize before ALL_RUNTIMES_OPTION check + isolate HERMES_HOME in test

Browse Source 
Two CodeRabbit findings on PR #2920:

1. parseRuntimeInput previously only matched the bare "16" exactly for
   the all-runtimes shortcut. Inputs the prompt explicitly encourages —
   "16,", "16 1", "1,16" — fell through to per-token parsing and
   silently installed only Claude or a partial subset. Move the
   ALL_RUNTIMES_OPTION check after tokenization so any token equal to
   "16" expands. Added regression coverage in
   tests/multi-runtime-select.test.cjs for the four mixed-input forms.

2. The "maps Hermes to ~/.hermes for global installs" test invoked
   getGlobalDir('hermes') without isolating HERMES_HOME. On a developer
   machine that exports HERMES_HOME the assertion would fail even
   though getGlobalDir was behaving correctly. Save/clear/restore the
   env var around the assertion, mirroring the pattern the later
   describe block already uses.

Full suite: 6128/6128 pass.
This commit is contained in:
Tom Boucher
2026-04-30 22:48:08 -04:00
parent c9d6306981
commit 372d3453f5
3 changed files with 25 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
bin/install.js
View File

@@ -8268,11 +8268,13 @@ function buildRuntimePromptText() {
function parseRuntimeInput(answer) {
const input = (answer == null ? '' : String(answer)).trim() || '1';
if (input === ALL_RUNTIMES_OPTION) {
// Tokenize first so the all-runtimes shortcut also fires for inputs the
// prompt encourages — "16,", "16 1", etc. — not just the bare "16".
const choices = input.split(/[\s,]+/).filter(Boolean);
if (choices.includes(ALL_RUNTIMES_OPTION)) {
return allRuntimes.slice();
}
const choices = input.split(/[\s,]+/).filter(Boolean);
const selected = [];
for (const c of choices) {
const runtime = runtimeMap[c];

11
tests/hermes-install.test.cjs
View File

@@ -23,7 +23,16 @@ describe('Hermes Agent runtime directory mapping', () => {
});
test('maps Hermes to ~/.hermes for global installs', () => {
assert.strictEqual(getGlobalDir('hermes'), path.join(os.homedir(), '.hermes'));
// Isolate from any HERMES_HOME exported on the developer's machine —
// otherwise this test asserts the env-derived path, not the default.
const originalHermesHome = process.env.HERMES_HOME;
delete process.env.HERMES_HOME;
try {
assert.strictEqual(getGlobalDir('hermes'), path.join(os.homedir(), '.hermes'));
} finally {
if (originalHermesHome === undefined) delete process.env.HERMES_HOME;
else process.env.HERMES_HOME = originalHermesHome;
}
});
test('returns .hermes config fragments for local and global installs', () => {

11
tests/multi-runtime-select.test.cjs
View File

@@ -84,6 +84,17 @@ describe('multi-runtime selection parsing', () => {
assert.deepStrictEqual(parseRuntimeInput('16'), allRuntimes);
});
test('choice 16 returns all runtimes when mixed with separators or other tokens', () => {
// CR feedback: tokenized inputs that include 16 (e.g. trailing comma, or
// alongside other choices) must still expand to all-runtimes — previously
// only the bare "16" matched, so "16," or "16 1" silently installed a
// subset.
assert.deepStrictEqual(parseRuntimeInput('16,'), allRuntimes);
assert.deepStrictEqual(parseRuntimeInput('16 1'), allRuntimes);
assert.deepStrictEqual(parseRuntimeInput('1,16'), allRuntimes);
assert.deepStrictEqual(parseRuntimeInput(' 16 '), allRuntimes);
});
test('empty input defaults to claude', () => {
assert.deepStrictEqual(parseRuntimeInput(''), ['claude']);
assert.deepStrictEqual(parseRuntimeInput(' '), ['claude']);