mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-18 09:51:59 +00:00
* fix(cli): omit --workspace when subpackage has its own package-lock.json When ui-tui/ (or web/) contains its own package-lock.json, _workspace_root() returns the subpackage directory itself. Passing --workspace ui-tui in that case fails because npm cannot find a workspace named 'ui-tui' inside ui-tui/. Fix: skip the --workspace flag when npm_cwd equals the target directory, running a plain 'npm install' from the standalone project root instead. Applies the same fix to both _make_tui_argv (TUI) and _build_web_ui (web). Fixes #42973 * test(cli): fix web workspace-scope fixture + cover own-lockfile fallback (#42973) The web half of the #42977 fix broke test_npm_install_uses_workspace_web_scope, which built its fixture with no lockfile anywhere. Without a root lockfile, _workspace_root(web_dir) already returns web_dir, so the new "() if npm_cwd == web_dir" branch correctly drops --workspace and the assertion failed. Model a real workspace checkout instead: the single package-lock.json lives at the root, so --workspace web scopes the install. Also add the symmetric web regression test (web/ carrying its own lockfile => --workspace must be dropped and the install runs plainly from web_dir via npm ci), matching the TUI coverage already in test_tui_npm_install.py. --------- Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
This commit is contained in:
parent
3e74f75e41
commit
975edd4140
4 changed files with 86 additions and 6 deletions
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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():
|
||||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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).
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue