diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 20728c4f336..53441055958 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -1623,7 +1623,11 @@ def _make_tui_argv(tui_dir: Path, tui_dev: bool) -> tuple[list[str], Path]: npm_cwd = _workspace_root(tui_dir) # --workspace ui-tui avoids resolving apps/desktop (Electron + node-pty). # See #38772. - npm_workspace_args: tuple[str, ...] = ("--workspace", "ui-tui") + # When ui-tui/ has its own package-lock.json (e.g. curl install), + # _workspace_root() returns tui_dir itself. Passing --workspace in + # that case fails because npm cannot find a workspace named "ui-tui" + # inside ui-tui/. See #42973. + npm_workspace_args: tuple[str, ...] = () if npm_cwd == tui_dir else ("--workspace", "ui-tui") if termux_startup: npm_cwd, npm_workspace_args = _termux_workspace_install_context( tui_dir, @@ -4642,7 +4646,9 @@ def _build_web_ui(web_dir: Path, *, fatal: bool = False) -> bool: # graph (including apps/desktop with its Electron + node-pty deps) is never # resolved here. Without --workspace the root package.json's apps/* glob # would pull in desktop on every web build. See #38772. - npm_workspace_args: tuple[str, ...] = ("--workspace", "web") + # When web/ has its own package-lock.json, _workspace_root() returns + # web_dir itself and --workspace would fail. See #42973. + npm_workspace_args: tuple[str, ...] = () if npm_cwd == web_dir else ("--workspace", "web") if _is_termux_startup_environment(): npm_cwd, npm_workspace_args = _termux_workspace_install_context(web_dir) r1 = _run_npm_install_deterministic( diff --git a/tests/hermes_cli/test_subcommands_cron.py b/tests/hermes_cli/test_cron_parser_builder.py similarity index 94% rename from tests/hermes_cli/test_subcommands_cron.py rename to tests/hermes_cli/test_cron_parser_builder.py index e51a0bb6409..16be898b1a9 100644 --- a/tests/hermes_cli/test_subcommands_cron.py +++ b/tests/hermes_cli/test_cron_parser_builder.py @@ -47,20 +47,19 @@ def test_cron_aliases(): def test_cron_create_options(): parser = _build() ns = parser.parse_args([ - "cron", "create", "0 9 * * *", "do the thing", + "cron", "create", "0 9 * * *", "daily task prompt", "--name", "daily", "--deliver", "origin", "--repeat", "3", "--skill", "a", "--skill", "b", "--no-agent", - "--workdir", "/tmp/x", "--profile", "work", + "--workdir", "/tmp/x", ]) assert ns.schedule == "0 9 * * *" - assert ns.prompt == "do the thing" + assert ns.prompt == "daily task prompt" assert ns.name == "daily" assert ns.deliver == "origin" assert ns.repeat == 3 assert ns.skills == ["a", "b"] assert ns.no_agent is True assert ns.workdir == "/tmp/x" - assert ns.profile == "work" def test_cron_edit_no_agent_tristate(): diff --git a/tests/hermes_cli/test_tui_npm_install.py b/tests/hermes_cli/test_tui_npm_install.py index 5d52e4276c4..f9b3cf108fc 100644 --- a/tests/hermes_cli/test_tui_npm_install.py +++ b/tests/hermes_cli/test_tui_npm_install.py @@ -425,3 +425,43 @@ def test_tui_launch_install_uses_workspace_scope( install_cmd = npm_calls[0] assert "--workspace" in install_cmd assert "ui-tui" in install_cmd + +def test_make_tui_argv_omits_workspace_when_tui_has_own_lockfile( + tmp_path: Path, main_mod, monkeypatch +) -> None: + """When ui-tui/ has its own package-lock.json, _workspace_root returns + tui_dir itself. npm install --workspace ui-tui would fail in that case + because npm cannot find a workspace named "ui-tui" inside ui-tui/. + The fix omits --workspace and runs plain npm install from tui_dir. + See #42973. + """ + tui_dir = tmp_path / "ui-tui" + tui_dir.mkdir() + (tui_dir / "package.json").write_text("{}") + # Simulate curl-install layout: tui_dir has its own lockfile + (tui_dir / "package-lock.json").write_text("{}") + # Parent also has lockfile (but _workspace_root prefers tui_dir's own) + (tmp_path / "package-lock.json").write_text("{}") + + monkeypatch.delenv("TERMUX_VERSION", raising=False) + monkeypatch.setenv("PREFIX", "/usr") + monkeypatch.setattr(main_mod, "_tui_need_npm_install", lambda _root: True) + monkeypatch.setattr(main_mod.shutil, "which", lambda name: f"/bin/{name}") + calls = [] + + def fake_run(*args, **kwargs): + calls.append((args, kwargs)) + return types.SimpleNamespace(returncode=0, stdout="", stderr="") + + monkeypatch.setattr(main_mod.subprocess, "run", fake_run) + + main_mod._make_tui_argv(tui_dir, tui_dev=False) + + install_cmd = calls[0][0][0] + # Must NOT contain --workspace when npm_cwd == tui_dir + assert "--workspace" not in install_cmd, ( + f"npm install should omit --workspace when tui_dir has its own lockfile, got: {install_cmd}" + ) + assert install_cmd[:2] == ["/bin/npm", "install"] + # cwd must be tui_dir (standalone), not parent + assert calls[0][1]["cwd"] == str(tui_dir) diff --git a/tests/hermes_cli/test_web_ui_build.py b/tests/hermes_cli/test_web_ui_build.py index 0783af22a13..cc20fa61cf3 100644 --- a/tests/hermes_cli/test_web_ui_build.py +++ b/tests/hermes_cli/test_web_ui_build.py @@ -142,6 +142,11 @@ class TestBuildWebUISkipsWhenFresh: def test_npm_install_uses_workspace_web_scope(self, tmp_path): web_dir, _ = _make_web_dir(tmp_path) + # Real workspace checkout: the single lockfile lives at the root, so + # _workspace_root(web_dir) resolves to the parent and --workspace web + # scopes the install. (Without a root lockfile, web_dir IS the root and + # --workspace would be dropped — see test below and #42973.) + (tmp_path / "package-lock.json").write_text("{}", encoding="utf-8") mock_cp = __import__("subprocess").CompletedProcess([], 0, stdout="", stderr="") build_ok = __import__("subprocess").CompletedProcess([], 0, stdout="", stderr="") with patch("hermes_cli.main.shutil.which", return_value="/usr/bin/npm"), \ @@ -153,6 +158,36 @@ class TestBuildWebUISkipsWhenFresh: assert "--workspace" in install_cmd assert install_cmd[install_cmd.index("--workspace") + 1] == "web" + def test_web_install_omits_workspace_when_web_has_own_lockfile( + self, tmp_path, monkeypatch + ): + """web/ with its own lockfile => _workspace_root returns web_dir, so + --workspace web would fail (npm can't find that workspace from inside + web/). The flag must be dropped and the install run plainly from web_dir. + Symmetric to the TUI fix in test_tui_npm_install.py. See #42973. + + With web's own lockfile present at cwd, _run_npm_install_deterministic + uses ``npm ci`` (not ``npm install``). + """ + web_dir, _ = _make_web_dir(tmp_path) + (web_dir / "package-lock.json").write_text("{}", encoding="utf-8") + (tmp_path / "package-lock.json").write_text("{}", encoding="utf-8") + monkeypatch.delenv("TERMUX_VERSION", raising=False) + monkeypatch.setenv("PREFIX", "/usr") + + install_cp = __import__("subprocess").CompletedProcess([], 0, stdout="", stderr="") + build_cp = __import__("subprocess").CompletedProcess([], 0, stdout="", stderr="") + with patch("hermes_cli.main.shutil.which", return_value="/usr/bin/npm"), \ + patch("hermes_cli.main.subprocess.run", return_value=install_cp) as mock_run, \ + patch("hermes_cli.main._run_with_idle_timeout", return_value=build_cp): + result = _build_web_ui(web_dir) + + assert result is True + args, kwargs = mock_run.call_args + assert "--workspace" not in args[0] + assert args[0] == ["/usr/bin/npm", "ci", "--silent"] + assert kwargs["cwd"] == web_dir + def test_web_build_uses_idle_timeout_helper(self, tmp_path): """npm run build now goes through _run_with_idle_timeout (issue #33788).