diff --git a/scripts/install.sh b/scripts/install.sh index 1f2635673c118..a0f308523f78c 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -329,7 +329,11 @@ emit_manifest() { if [ "$INCLUDE_DESKTOP" = true ]; then desktop_stage='{"name":"desktop","title":"Build desktop app","category":"runtime","needs_user_input":false},' fi - printf '%s' '{"protocol_version":1,"stages":[{"name":"prerequisites","title":"System prerequisites","category":"runtime","needs_user_input":false},{"name":"repository","title":"Download Hermes Agent","category":"runtime","needs_user_input":false},{"name":"venv","title":"Create Python virtual environment","category":"runtime","needs_user_input":false},{"name":"python-deps","title":"Install Python dependencies","category":"runtime","needs_user_input":false},{"name":"node-deps","title":"Install browser-tool dependencies","category":"runtime","needs_user_input":false},{"name":"path","title":"Install hermes command","category":"runtime","needs_user_input":false},{"name":"config","title":"Prepare config and skills","category":"configuration","needs_user_input":false},{"name":"setup","title":"Configure API keys and settings","category":"configuration","needs_user_input":true},{"name":"gateway","title":"Configure gateway service","category":"configuration","needs_user_input":true},'"$desktop_stage"'{"name":"complete","title":"Finish install","category":"runtime","needs_user_input":false}]}' + local interactive_stages="" + if ! has_existing_hermes_config; then + interactive_stages='{"name":"setup","title":"Configure API keys and settings","category":"configuration","needs_user_input":true},{"name":"gateway","title":"Configure gateway service","category":"configuration","needs_user_input":true},' + fi + printf '%s' '{"protocol_version":1,"stages":[{"name":"prerequisites","title":"System prerequisites","category":"runtime","needs_user_input":false},{"name":"repository","title":"Download Hermes Agent","category":"runtime","needs_user_input":false},{"name":"venv","title":"Create Python virtual environment","category":"runtime","needs_user_input":false},{"name":"python-deps","title":"Install Python dependencies","category":"runtime","needs_user_input":false},{"name":"node-deps","title":"Install browser-tool dependencies","category":"runtime","needs_user_input":false},{"name":"path","title":"Install hermes command","category":"runtime","needs_user_input":false},{"name":"config","title":"Prepare config and skills","category":"configuration","needs_user_input":false},'"$interactive_stages$desktop_stage"'{"name":"complete","title":"Finish install","category":"runtime","needs_user_input":false}]}' printf '\n' } @@ -340,6 +344,17 @@ stage_needs_user_input() { esac } +has_existing_hermes_config() { + local env_file="$HERMES_HOME/.env" + local cfg_file="$HERMES_HOME/config.yaml" + + [ -s "$cfg_file" ] || return 1 + [ -s "$env_file" ] || return 1 + + grep -Eq '^[[:space:]]*[A-Z0-9_]*(API_KEY|TOKEN)[[:space:]]*=[[:space:]]*[^[:space:]#]+' "$env_file" 2>/dev/null || return 1 + ! grep -Eq '^[[:space:]]*[A-Z0-9_]*(API_KEY|TOKEN)[[:space:]]*=[[:space:]]*(your-|changeme|placeholder|xxx)' "$env_file" 2>/dev/null +} + emit_stage_json() { local stage="$1" local ok="$2" @@ -3829,9 +3844,13 @@ run_stage_protocol() { fi if [ "$NON_INTERACTIVE" = true ] && stage_needs_user_input "$stage"; then - log_info "Skipping $stage (non-interactive bootstrap)" + local skip_reason="non-interactive bootstrap" + if has_existing_hermes_config; then + skip_reason="existing Hermes config detected" + fi + log_info "Skipping $stage ($skip_reason)" if [ "$JSON_OUTPUT" = true ]; then - emit_stage_json "$stage" true true + emit_stage_json "$stage" true true "$skip_reason" fi return 0 fi diff --git a/tests/hermes_cli/test_cmd_update.py b/tests/hermes_cli/test_cmd_update.py index d91a8482ccf46..ecb64f8b95824 100644 --- a/tests/hermes_cli/test_cmd_update.py +++ b/tests/hermes_cli/test_cmd_update.py @@ -95,12 +95,12 @@ def test_record_npm_lockfile_hash(self, tmp_path, monkeypatch): from hermes_cli import main as hm monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) - (tmp_path / "package-lock.json").write_text('{"lockfileVersion": 3}') + (tmp_path / "package-lock.json").write_text('{"lockfileVersion": 3}', encoding="utf-8") update_cmd._record_npm_lockfile_hash(tmp_path) assert ( - self._cache_file(tmp_path, tmp_path).read_text() + self._cache_file(tmp_path, tmp_path).read_text(encoding="utf-8") == update_cmd._npm_manifests_digest() ) @@ -111,8 +111,8 @@ def test_package_json_only_edit_defeats_skip(self, tmp_path, monkeypatch): from hermes_cli import main as hm monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) - (tmp_path / "package-lock.json").write_text('{"lockfileVersion": 3}') - (tmp_path / "package.json").write_text('{"dependencies": {}}') + (tmp_path / "package-lock.json").write_text('{"lockfileVersion": 3}', encoding="utf-8") + (tmp_path / "package.json").write_text('{"dependencies": {}}', encoding="utf-8") (tmp_path / "node_modules").mkdir() update_cmd._record_npm_lockfile_hash(tmp_path) assert hm._npm_lockfile_changed(tmp_path) is False @@ -137,7 +137,7 @@ def test_update_uses_one_shared_npm_cache_across_profiles( checkout = tmp_path / "checkout" checkout.mkdir() - (checkout / "package.json").write_text("{}") + (checkout / "package.json").write_text("{}", encoding="utf-8") shared_root = tmp_path / ".hermes" named_profile = shared_root / "profiles" / "work" named_profile.mkdir(parents=True) @@ -515,6 +515,27 @@ def test_fork_upstream_sync_that_moves_head_runs_post_update_steps( captured = capsys.readouterr() assert "Already up to date!" not in captured.out + @patch("shutil.which") + @patch("subprocess.run") + def test_update_refreshes_node_and_web_ui_when_browser_toolset_disabled( + self, mock_run, mock_which, mock_args + ): + from hermes_cli import main as hm + + mock_which.side_effect = {"uv": "/usr/bin/uv", "npm": "/usr/bin/npm"}.get + mock_run.side_effect = _make_run_side_effect( + branch="main", verify_ok=True, commit_count="1" + ) + + disabled_browser_config = {"platform_toolsets": {"cli": ["file", "terminal", "web"]}} + with patch("hermes_cli.config.load_config", return_value=disabled_browser_config), \ + patch("hermes_cli.update_cmd._update_node_dependencies", return_value=[]) as update_node, \ + patch.object(hm, "_build_web_ui") as build_web: + cmd_update(mock_args) + + update_node.assert_called_once_with() + build_web.assert_called_once_with(hm.PROJECT_ROOT / "web") + def test_update_non_interactive_runs_safe_config_migrations(self, mock_args, capsys): """Dashboard/web updates apply non-interactive migrations before restart.""" with patch("shutil.which", return_value=None), patch( @@ -1058,7 +1079,7 @@ def test_node_failure_returns_failed_labels_and_warns( ): from hermes_cli import main as hm - (tmp_path / "package.json").write_text("{}") + (tmp_path / "package.json").write_text("{}", encoding="utf-8") monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) monkeypatch.setattr(hm, "_resolve_node_runtime_npm", lambda: "/usr/bin/npm") monkeypatch.setattr( @@ -1316,8 +1337,8 @@ def test_install_names_ui_tui_and_web_workspaces(self, _which, mock_popen, tmp_p """ from hermes_cli import main as hm - (tmp_path / "package.json").write_text("{}") - (tmp_path / "package-lock.json").write_text("{}") + (tmp_path / "package.json").write_text("{}", encoding="utf-8") + (tmp_path / "package-lock.json").write_text("{}", encoding="utf-8") monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) monkeypatch.setattr(hm, "_npm_lockfile_changed", lambda root: True) popen_calls = [] @@ -1353,8 +1374,8 @@ def test_install_includes_workspace_root_to_protect_root_devdependencies( review).""" from hermes_cli import main as hm - (tmp_path / "package.json").write_text("{}") - (tmp_path / "package-lock.json").write_text("{}") + (tmp_path / "package.json").write_text("{}", encoding="utf-8") + (tmp_path / "package-lock.json").write_text("{}", encoding="utf-8") monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) monkeypatch.setattr(hm, "_npm_lockfile_changed", lambda root: True) popen_calls = [] @@ -1374,8 +1395,8 @@ def test_install_preserves_standard_flags(self, _which, mock_popen, tmp_path, mo """--no-fund, --no-audit, --progress=false must survive.""" from hermes_cli import main as hm - (tmp_path / "package.json").write_text("{}") - (tmp_path / "package-lock.json").write_text("{}") + (tmp_path / "package.json").write_text("{}", encoding="utf-8") + (tmp_path / "package-lock.json").write_text("{}", encoding="utf-8") monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) monkeypatch.setattr(hm, "_npm_lockfile_changed", lambda root: True) popen_calls = [] @@ -1395,8 +1416,8 @@ def test_skips_install_when_deps_up_to_date(self, _which, mock_run, tmp_path, mo """When _npm_lockfile_changed reports no change, npm must not be called.""" from hermes_cli import main as hm - (tmp_path / "package.json").write_text("{}") - (tmp_path / "package-lock.json").write_text("{}") + (tmp_path / "package.json").write_text("{}", encoding="utf-8") + (tmp_path / "package-lock.json").write_text("{}", encoding="utf-8") monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) monkeypatch.setattr(hm, "_npm_lockfile_changed", lambda root: False) @@ -1412,8 +1433,8 @@ def test_runs_install_when_lockfile_changed(self, _which, mock_popen, tmp_path, """When _npm_lockfile_changed reports a change, npm must run.""" from hermes_cli import main as hm - (tmp_path / "package.json").write_text("{}") - (tmp_path / "package-lock.json").write_text("{}") + (tmp_path / "package.json").write_text("{}", encoding="utf-8") + (tmp_path / "package-lock.json").write_text("{}", encoding="utf-8") monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) monkeypatch.setattr(hm, "_npm_lockfile_changed", lambda root: True) popen_calls = [] @@ -1431,8 +1452,8 @@ def test_records_lockfile_hash_only_on_success(self, _which, mock_popen, tmp_pat run retries instead of wrongly believing deps are up to date).""" from hermes_cli import main as hm - (tmp_path / "package.json").write_text("{}") - (tmp_path / "package-lock.json").write_text("{}") + (tmp_path / "package.json").write_text("{}", encoding="utf-8") + (tmp_path / "package-lock.json").write_text("{}", encoding="utf-8") monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) monkeypatch.setattr(hm, "_npm_lockfile_changed", lambda root: True) recorded = [] @@ -1455,8 +1476,8 @@ def test_warms_npx_agent_browser_cache_regardless_of_install_result( it's independent of ui-tui/web dependency state (#43564).""" from hermes_cli import main as hm - (tmp_path / "package.json").write_text("{}") - (tmp_path / "package-lock.json").write_text("{}") + (tmp_path / "package.json").write_text("{}", encoding="utf-8") + (tmp_path / "package-lock.json").write_text("{}", encoding="utf-8") monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) monkeypatch.setattr(hm, "_npm_lockfile_changed", lambda root: True) mock_popen.side_effect = self._make_popen([], returncode=1, stderr_lines=["npm ERR!\n"]) @@ -1474,7 +1495,7 @@ def test_returns_silently_when_npm_not_found(self, _which, mock_run, tmp_path, m """No npm on PATH → return without calling subprocess.""" from hermes_cli import main as hm - (tmp_path / "package.json").write_text("{}") + (tmp_path / "package.json").write_text("{}", encoding="utf-8") monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) update_cmd._update_node_dependencies() @@ -1499,8 +1520,8 @@ def test_install_runs_from_project_root(self, _which, mock_popen, tmp_path, monk """npm install must execute from PROJECT_ROOT, not a workspace subdir.""" from hermes_cli import main as hm - (tmp_path / "package.json").write_text("{}") - (tmp_path / "package-lock.json").write_text("{}") + (tmp_path / "package.json").write_text("{}", encoding="utf-8") + (tmp_path / "package-lock.json").write_text("{}", encoding="utf-8") monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path) popen_calls = [] diff --git a/tests/hermes_cli/test_dep_ensure.py b/tests/hermes_cli/test_dep_ensure.py index 3487c845d364a..2e9d8015a8f9b 100644 --- a/tests/hermes_cli/test_dep_ensure.py +++ b/tests/hermes_cli/test_dep_ensure.py @@ -1,9 +1,27 @@ -from unittest.mock import patch +from unittest.mock import Mock, patch import pytest from tools import browser_tool_install as bt_install +def test_acp_browser_setup_ignores_disabled_cli_browser_toolset(tmp_path): + """An explicit ACP setup request must install browser support.""" + from acp_adapter.entry import _run_setup_browser + + script = tmp_path / "install.sh" + script.touch() + config = {"platform_toolsets": {"cli": ["file", "terminal", "web"]}} + browser_check = Mock(side_effect=[False, True]) + with patch("hermes_cli.dep_ensure._DEP_CHECKS", {"node": lambda: True, "browser": browser_check}), \ + patch("hermes_cli.config.load_config", return_value=config), \ + patch("hermes_cli.dep_ensure._find_install_script", return_value=(script, "bash")), \ + patch("hermes_cli.dep_ensure.subprocess.run", return_value=Mock(returncode=0)) as run: + assert _run_setup_browser(assume_yes=True) == 0 + + assert run.call_args.args[0] == ["bash", str(script), "--ensure", "browser"] + assert browser_check.call_count == 2 + + @pytest.mark.linux_only def test_find_install_script_from_checkout(tmp_path): """_find_install_script finds scripts/install.sh in a git checkout. @@ -104,7 +122,7 @@ def test_ensure_dependency_uses_powershell_on_windows(tmp_path): from hermes_cli.dep_ensure import ensure_dependency scripts_dir = tmp_path / "scripts" scripts_dir.mkdir(parents=True) - (scripts_dir / "install.ps1").write_text("# fake") + (scripts_dir / "install.ps1").write_text("# fake", encoding="utf-8") with patch("hermes_cli.dep_ensure._DEP_CHECKS", {"node": lambda: False}), \ patch("hermes_cli.dep_ensure._find_install_script", return_value=(scripts_dir / "install.ps1", "powershell")), \ patch("hermes_cli.dep_ensure.shutil") as mock_shutil, \ diff --git a/tests/scripts/install/test_install_sh_browser_install.py b/tests/scripts/install/test_install_sh_browser_install.py index e2f5f0323ced8..aeae6e667c35c 100644 --- a/tests/scripts/install/test_install_sh_browser_install.py +++ b/tests/scripts/install/test_install_sh_browser_install.py @@ -5,6 +5,9 @@ unsupported distribution. """ +import json +import os +import subprocess from pathlib import Path @@ -16,7 +19,7 @@ def test_install_script_honors_explicit_browser_override_only() -> None: """find_system_browser consults only an explicit AGENT_BROWSER_EXECUTABLE_PATH.""" - text = INSTALL_SH.read_text() + text = INSTALL_SH.read_text(encoding="utf-8") assert 'override="${AGENT_BROWSER_EXECUTABLE_PATH:-}"' in text # An explicit override still skips the bundled download (override, not fallback). @@ -26,7 +29,7 @@ def test_install_script_honors_explicit_browser_override_only() -> None: def test_playwright_installs_are_timeout_guarded() -> None: - text = INSTALL_SH.read_text() + text = INSTALL_SH.read_text(encoding="utf-8") # The timeout wrapper still exists and is used internally by the install # wrapper, so every Playwright download remains bounded. @@ -47,7 +50,7 @@ def test_playwright_installs_are_timeout_guarded() -> None: def test_install_script_supports_skip_browser_flag() -> None: """--skip-browser (and --no-playwright alias) skips the Playwright install.""" - text = INSTALL_SH.read_text() + text = INSTALL_SH.read_text(encoding="utf-8") assert "--skip-browser|--no-playwright)" in text assert "SKIP_BROWSER=true" in text @@ -55,6 +58,27 @@ def test_install_script_supports_skip_browser_flag() -> None: assert "--skip-browser Skip Playwright/Chromium install" in text +def test_manifest_omits_interactive_stages_for_existing_config(tmp_path) -> None: + """Desktop/bootstrap staged installs should be incremental when configured.""" + hermes_home = tmp_path / "home" + hermes_home.mkdir() + (hermes_home / ".env").write_text("OPENAI_API_KEY=sk-real\n", encoding="utf-8") + (hermes_home / "config.yaml").write_text("model:\n provider: openai\n", encoding="utf-8") + + env = {**os.environ, "HERMES_HOME": str(hermes_home)} + result = subprocess.run( + ["bash", str(INSTALL_SH), "--manifest", "--include-desktop"], + env=env, + capture_output=True, + text=True, + check=True, + ) + manifest = json.loads(result.stdout) + stage_names = [stage["name"] for stage in manifest["stages"]] + + assert "desktop" in stage_names + assert "setup" not in stage_names + assert "gateway" not in stage_names @@ -68,7 +92,7 @@ def test_browser_install_timeout_stays_interruptible() -> None: `-k 10` guarantees a SIGKILL after the deadline. Both are GNU-only, so the installer probes support once and falls back to plain `timeout`. """ - text = INSTALL_SH.read_text() + text = INSTALL_SH.read_text(encoding="utf-8") # GNU-flag probe + the guarded invocation must both be present. The timeout # binary is parameterized ($timeout_bin) so macOS gtimeout works too (#39219). @@ -84,9 +108,6 @@ def test_browser_install_timeout_stays_interruptible() -> None: # host Playwright already supports. # --------------------------------------------------------------------------- -import subprocess - - def _run_install_fn(distro: str, version: str, *, native_fails: bool, arch: str = "x86_64", operator_override: str = "") -> dict: """Source the relevant functions from install.sh and drive run_playwright_install. @@ -105,7 +126,7 @@ def _run_install_fn(distro: str, version: str, *, native_fails: bool, "playwright_fallback_platform", "run_playwright_install", ] - src = INSTALL_SH.read_text() + src = INSTALL_SH.read_text(encoding="utf-8") import re extracted = [] @@ -158,7 +179,7 @@ def _run_install_fn(distro: str, version: str, *, native_fails: bool, env = dict(os.environ, RUNLOG=runlog) proc = subprocess.run(["bash", "-c", harness], capture_output=True, text=True, env=env) - runs = Path(runlog).read_text().strip().splitlines() + runs = Path(runlog).read_text(encoding="utf-8").strip().splitlines() final_rc = None for line in proc.stdout.splitlines(): if line.startswith("FINAL_RC="): @@ -213,7 +234,7 @@ def test_ensure_browser_no_longer_npm_installs_agent_browser() -> None: copy of it. Removed: agent-browser acquisition now happens only via `hermes update`'s npx cache warm or an actual browser-tool call's lazy npx resolution (PR #44772 review).""" - body = _extract_function_body(INSTALL_SH.read_text(), "ensure_browser") + body = _extract_function_body(INSTALL_SH.read_text(encoding="utf-8"), "ensure_browser") assert "agent-browser@" not in body assert "Installing Chromium via agent-browser install" not in body @@ -228,7 +249,7 @@ def test_ensure_browser_still_ignore_scripts_and_timeout_guarded() -> None: """The removal of agent-browser must not have also dropped the supply-chain and hang-protection hardening that still applies to the remaining camofox install.""" - body = _extract_function_body(INSTALL_SH.read_text(), "ensure_browser") + body = _extract_function_body(INSTALL_SH.read_text(encoding="utf-8"), "ensure_browser") assert "--ignore-scripts" in body assert "run_with_timeout" in body @@ -239,11 +260,9 @@ def test_ensure_browser_no_longer_references_agent_browser_binary_path() -> None remain now that this function never installs it — a leftover reference would be dead code pointing at a binary that no longer gets placed there by this function.""" - body = _extract_function_body(INSTALL_SH.read_text(), "ensure_browser") + body = _extract_function_body(INSTALL_SH.read_text(encoding="utf-8"), "ensure_browser") assert "$HERMES_HOME/node/bin/agent-browser" not in body - -