workspace lockfile conflict with sub-package lockfiles

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #43986 fix(cli): omit --workspace when subpackage has its own package-lock.json (#42973)

  merged to main

  files: hermes_cli/main.py, tests/hermes_cli/test_cron_parser_builder.py, tests/hermes_cli/test_tui_npm_install.py, tests/hermes_cli/test_web_ui_build.py

  Show the diff

  • hermes_cli/main.py

    @@ -1623,7 +1623,11 @@ def _node_bin(bin: str) -> str:
             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 _relay(result: "subprocess.CompletedProcess") -> None:
         # 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(

  • 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():

  • tests/hermes_cli/test_tui_npm_install.py

    @@ -425,3 +425,43 @@ def fake_run(cmd, **kwargs):
         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)

  • tests/hermes_cli/test_web_ui_build.py

    @@ -142,6 +142,11 @@ def test_npm_install_uses_utf8_replace_output_decoding(self, tmp_path):
     
         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 @@ def test_npm_install_uses_workspace_web_scope(self, tmp_path):
             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).