From 372d3453f5879d2ab10ba81f684a82997131c9f5 Mon Sep 17 00:00:00 2001
From: Tom Boucher <trekkie@nomorestars.com>
Date: Thu, 30 Apr 2026 22:48:08 -0400
Subject: [PATCH] fix(install): tokenize before ALL_RUNTIMES_OPTION check +
 isolate HERMES_HOME in test
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 bin/install.js                      |  6 ++++--
 tests/hermes-install.test.cjs       | 11 ++++++++++-
 tests/multi-runtime-select.test.cjs | 11 +++++++++++
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/bin/install.js b/bin/install.js
index e1020afbc..779a1ae55 100755
--- a/bin/install.js
+++ b/bin/install.js
@@ -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];
diff --git a/tests/hermes-install.test.cjs b/tests/hermes-install.test.cjs
index 3724e4d0c..13d2395a5 100644
--- a/tests/hermes-install.test.cjs
+++ b/tests/hermes-install.test.cjs
@@ -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', () => {
diff --git a/tests/multi-runtime-select.test.cjs b/tests/multi-runtime-select.test.cjs
index e7f760dc1..1e15d046b 100644
--- a/tests/multi-runtime-select.test.cjs
+++ b/tests/multi-runtime-select.test.cjs
@@ -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']);