diff --git a/scripts/release.py b/scripts/release.py index 0c046ee4..ce029735 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -67,6 +67,9 @@ AUTHOR_MAP = { "274096618+hermes-agent-dhabibi@users.noreply.github.com": "dhabibi", "dejie.guo@gmail.com": "JayGwod", "maxence@groine.fr": "MaxyMoos", + # Internal molecule-ai Gitea bot identity used by Claude-Code agents + # (post-2026-05-06 GitHub suspension; no upstream/GitHub equivalent). + "claude-ceo-assistant@agents.moleculesai.app": "claude-ceo-assistant", # OpenViking viking_read salvage (April 2026) "hitesh@gmail.com": "htsh", "pty819@outlook.com": "pty819", diff --git a/tests/acp/test_server.py b/tests/acp/test_server.py index 35aafc60..7fefb35f 100644 --- a/tests/acp/test_server.py +++ b/tests/acp/test_server.py @@ -200,6 +200,8 @@ class TestSessionOps: "context", "reset", "compact", + "steer", + "queue", "version", ] model_cmd = next( diff --git a/tests/gateway/test_teams.py b/tests/gateway/test_teams.py index 7a035142..2100a264 100644 --- a/tests/gateway/test_teams.py +++ b/tests/gateway/test_teams.py @@ -397,13 +397,22 @@ class TestTeamsSend: assert "Network error" in result.error @pytest.mark.asyncio - async def test_send_typing(self): + async def test_send_typing(self, monkeypatch): adapter = TeamsAdapter(_make_config( client_id="id", client_secret="secret", tenant_id="tenant", )) mock_app = MagicMock() mock_app.send = AsyncMock() adapter._app = mock_app + # The adapter module imports TypingActivityInput at load time; if + # the real microsoft_teams package isn'"'"'t installed, that local + # binding is None even though the test fixture registers a mock + # in sys.modules. Force a non-None local binding so the call to + # TypingActivityInput() inside send_typing succeeds and we actually + # reach self._app.send. + class _StubTypingActivityInput: + pass + monkeypatch.setattr(_teams_mod, "TypingActivityInput", _StubTypingActivityInput) await adapter.send_typing("conv-id") mock_app.send.assert_awaited_once() diff --git a/tests/hermes_cli/test_gateway_service.py b/tests/hermes_cli/test_gateway_service.py index f2bfa8b8..75dacff1 100644 --- a/tests/hermes_cli/test_gateway_service.py +++ b/tests/hermes_cli/test_gateway_service.py @@ -5,6 +5,8 @@ import pwd from pathlib import Path from types import SimpleNamespace +import re + import pytest import hermes_cli.gateway as gateway_cli @@ -64,6 +66,11 @@ class TestSystemdServiceRefresh: monkeypatch.setattr(gateway_cli, "get_systemd_unit_path", lambda system=False: unit_path) monkeypatch.setattr(gateway_cli, "generate_systemd_unit", lambda system=False, run_as_user=None: "new unit\n") + # systemd_start invokes _preflight_user_systemd which probes the + # user D-Bus socket; that's not available in container test + # runners. The test exercises the unit-refresh flow, not the + # D-Bus check, so mock it as a no-op. + monkeypatch.setattr(gateway_cli, "_preflight_user_systemd", lambda: None) calls = [] @@ -87,6 +94,9 @@ class TestSystemdServiceRefresh: monkeypatch.setattr(gateway_cli, "get_systemd_unit_path", lambda system=False: unit_path) monkeypatch.setattr(gateway_cli, "generate_systemd_unit", lambda system=False, run_as_user=None: "new unit\n") + # See test_systemd_start_refreshes_outdated_unit — mock the + # D-Bus preflight that systemd_restart performs for user scope. + monkeypatch.setattr(gateway_cli, "_preflight_user_systemd", lambda: None) calls = [] @@ -107,6 +117,18 @@ class TestSystemdServiceRefresh: ] +def _expected_min_timeout_stop_sec() -> int: + """Compute the minimum TimeoutStopSec that systemd unit generation + should emit, mirroring hermes_cli/gateway.py:1636. The unit must give + the gateway at least drain_timeout + 30s of headroom so post-interrupt + cleanup (tool subprocess kill, adapter disconnect) finishes before + systemd escalates to SIGKILL on the cgroup (issue #8202). + """ + from gateway.restart import DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT + drain = int(DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT or 0) + return max(60, drain) + 30 + + class TestGeneratedSystemdUnits: def test_user_unit_avoids_recursive_execstop_and_uses_extended_stop_timeout(self): unit = gateway_cli.generate_systemd_unit(system=False) @@ -115,10 +137,17 @@ class TestGeneratedSystemdUnits: assert "ExecStop=" not in unit assert "ExecReload=/bin/kill -USR1 $MAINPID" in unit assert f"RestartForceExitStatus={GATEWAY_SERVICE_RESTART_EXIT_CODE}" in unit - # TimeoutStopSec must exceed the default drain_timeout (60s) so + # TimeoutStopSec must exceed the configured drain_timeout so # systemd doesn't SIGKILL the cgroup before post-interrupt cleanup # (tool subprocess kill, adapter disconnect) runs — issue #8202. - assert "TimeoutStopSec=90" in unit + # Compute the expected minimum from the SSOT default rather than + # hard-coding a value that drifts with config bumps. + expected = _expected_min_timeout_stop_sec() + match = re.search(r"^TimeoutStopSec=(\d+)", unit, flags=re.MULTILINE) + assert match is not None, "TimeoutStopSec directive missing" + assert int(match.group(1)) >= expected, ( + f"TimeoutStopSec={match.group(1)} < expected {expected}" + ) def test_user_unit_includes_resolved_node_directory_in_path(self, monkeypatch): monkeypatch.setattr(gateway_cli.shutil, "which", lambda cmd: "/home/test/.nvm/versions/node/v24.14.0/bin/node" if cmd == "node" else None) @@ -134,10 +163,17 @@ class TestGeneratedSystemdUnits: assert "ExecStop=" not in unit assert "ExecReload=/bin/kill -USR1 $MAINPID" in unit assert f"RestartForceExitStatus={GATEWAY_SERVICE_RESTART_EXIT_CODE}" in unit - # TimeoutStopSec must exceed the default drain_timeout (60s) so + # TimeoutStopSec must exceed the configured drain_timeout so # systemd doesn't SIGKILL the cgroup before post-interrupt cleanup # (tool subprocess kill, adapter disconnect) runs — issue #8202. - assert "TimeoutStopSec=90" in unit + # Compute the expected minimum from the SSOT default rather than + # hard-coding a value that drifts with config bumps. + expected = _expected_min_timeout_stop_sec() + match = re.search(r"^TimeoutStopSec=(\d+)", unit, flags=re.MULTILINE) + assert match is not None, "TimeoutStopSec directive missing" + assert int(match.group(1)) >= expected, ( + f"TimeoutStopSec={match.group(1)} < expected {expected}" + ) assert "WantedBy=multi-user.target" in unit @@ -437,6 +473,10 @@ class TestGatewayServiceDetection: monkeypatch.setattr(gateway_cli, "is_linux", lambda: True) monkeypatch.setattr(gateway_cli, "is_termux", lambda: False) monkeypatch.setattr(gateway_cli, "is_wsl", lambda: False) + # supports_systemd_services routes containers through + # _container_systemd_operational; mock False so this test + # isolates the "native Linux + systemctl present" branch. + monkeypatch.setattr(gateway_cli, "is_container", lambda: False) monkeypatch.setattr(gateway_cli.shutil, "which", lambda name: "/usr/bin/systemctl") assert gateway_cli.supports_systemd_services() is True @@ -487,6 +527,11 @@ class TestGatewaySystemServiceRouting: calls = [] monkeypatch.setattr(gateway_cli, "_select_systemd_scope", lambda system=False: False) + # systemd_restart calls _preflight_user_systemd which probes the + # user D-Bus socket; not available in container test runners. + # The test exercises the self-restart routing flow, not the + # D-Bus check, so mock it as a no-op. + monkeypatch.setattr(gateway_cli, "_preflight_user_systemd", lambda: None) monkeypatch.setattr(gateway_cli, "refresh_systemd_unit_if_needed", lambda system=False: calls.append(("refresh", system))) monkeypatch.setattr( "gateway.status.get_running_pid", @@ -540,6 +585,9 @@ class TestGatewaySystemServiceRouting: assert "restarted" in out def test_systemd_restart_recovers_failed_planned_restart(self, monkeypatch, capsys): + # See test_systemd_restart_self_requests_graceful_restart_and_waits + # — same D-Bus preflight no-op needed. + monkeypatch.setattr(gateway_cli, "_preflight_user_systemd", lambda: None) monkeypatch.setattr(gateway_cli, "_select_systemd_scope", lambda system=False: False) monkeypatch.setattr(gateway_cli, "refresh_systemd_unit_if_needed", lambda system=False: None) monkeypatch.setattr( diff --git a/tests/hermes_cli/test_gateway_wsl.py b/tests/hermes_cli/test_gateway_wsl.py index ea5bf40c..a824fd0c 100644 --- a/tests/hermes_cli/test_gateway_wsl.py +++ b/tests/hermes_cli/test_gateway_wsl.py @@ -141,10 +141,14 @@ class TestSupportsSystemdServicesWSL: assert gateway.supports_systemd_services() is False def test_native_linux(self, monkeypatch): - """Native Linux (not WSL) → True without checking systemd.""" + """Native Linux (not WSL, not container) → True without further checks.""" monkeypatch.setattr(gateway, "is_linux", lambda: True) monkeypatch.setattr(gateway, "is_termux", lambda: False) monkeypatch.setattr(gateway, "is_wsl", lambda: False) + # supports_systemd_services also routes containers through + # _container_systemd_operational; mock False so this test + # isolates the native-Linux branch. + monkeypatch.setattr(gateway, "is_container", lambda: False) assert gateway.supports_systemd_services() is True def test_termux_still_excluded(self, monkeypatch): diff --git a/tests/hermes_cli/test_update_gateway_restart.py b/tests/hermes_cli/test_update_gateway_restart.py index 721149dd..7577c922 100644 --- a/tests/hermes_cli/test_update_gateway_restart.py +++ b/tests/hermes_cli/test_update_gateway_restart.py @@ -415,7 +415,14 @@ class TestCmdUpdateLaunchdRestart: pid=12345, ) - with patch.object(gateway_cli, "find_gateway_pids", return_value=[12345]), \ + # Model successful primary-kill path: first find returns the + # live PID, subsequent calls (post-restart survivor sweep, #17648) + # return [] — the process exited after the graceful drain. + find_calls = {"n": 0} + def _find_drained(*a, **kw): + find_calls["n"] += 1 + return [12345] if find_calls["n"] == 1 else [] + with patch.object(gateway_cli, "find_gateway_pids", side_effect=_find_drained), \ patch.object(gateway_cli, "find_profile_gateway_processes", return_value=[process]), \ patch.object(gateway_cli, "launch_detached_profile_gateway_restart", return_value=True) as restart, \ patch.object(gateway_cli, "_graceful_restart_via_sigusr1", return_value=True) as graceful, \ @@ -425,7 +432,9 @@ class TestCmdUpdateLaunchdRestart: captured = capsys.readouterr().out restart.assert_called_once_with("coder", 12345) graceful.assert_called_once() - # Graceful drain succeeded — no SIGTERM fallback needed. + # Graceful drain succeeded — no SIGTERM fallback, and no SIGKILL + # from the survivor sweep because the PID drops out of + # find_gateway_pids on the second call. kill.assert_not_called() assert "Restarting manual gateway profile(s): coder" in captured assert "Restart manually: hermes gateway run" not in captured @@ -453,7 +462,13 @@ class TestCmdUpdateLaunchdRestart: pid=12345, ) - with patch.object(gateway_cli, "find_gateway_pids", return_value=[12345]), \ + # Model successful SIGTERM fallback: first find returns the live + # PID; subsequent calls return [] because SIGTERM landed. + find_calls = {"n": 0} + def _find_drained(*a, **kw): + find_calls["n"] += 1 + return [12345] if find_calls["n"] == 1 else [] + with patch.object(gateway_cli, "find_gateway_pids", side_effect=_find_drained), \ patch.object(gateway_cli, "find_profile_gateway_processes", return_value=[process]), \ patch.object(gateway_cli, "launch_detached_profile_gateway_restart", return_value=True) as restart, \ patch.object(gateway_cli, "_graceful_restart_via_sigusr1", return_value=False) as graceful, \ @@ -463,8 +478,11 @@ class TestCmdUpdateLaunchdRestart: captured = capsys.readouterr().out restart.assert_called_once_with("coder", 12345) graceful.assert_called_once() - # Graceful drain returned False → SIGTERM fallback. + # Graceful drain returned False → SIGTERM fallback. SIGTERM + # works (modelled by the find draining), so the survivor sweep + # finds nothing to escalate. kill.assert_called_once() + assert kill.call_args.args[0] == 12345 assert "Restarting manual gateway profile(s): coder" in captured @patch("shutil.which", return_value=None) @@ -872,9 +890,18 @@ class TestServicePidExclusion: launchctl_loaded=True, ) + # Model successful primary-kill path: first find returns both + # PIDs (filtered by the excluded service); subsequent calls + # return only the service PID (manual exited after SIGTERM). + # Without this, the post-restart survivor sweep (#17648) sends + # an extra SIGKILL to the manual PID and breaks the call-count + # assertion below. + find_calls = {"n": 0} def fake_find(exclude_pids=None, all_profiles=False): _exclude = exclude_pids or set() - return [p for p in [SERVICE_PID, MANUAL_PID] if p not in _exclude] + find_calls["n"] += 1 + live = [SERVICE_PID, MANUAL_PID] if find_calls["n"] == 1 else [SERVICE_PID] + return [p for p in live if p not in _exclude] with patch.object( gateway_cli, "_get_service_pids", return_value={SERVICE_PID} diff --git a/tests/run_agent/test_concurrent_interrupt.py b/tests/run_agent/test_concurrent_interrupt.py index 9a6ba73e..58be5f07 100644 --- a/tests/run_agent/test_concurrent_interrupt.py +++ b/tests/run_agent/test_concurrent_interrupt.py @@ -47,12 +47,32 @@ def _make_agent(monkeypatch): # Worker-thread tracking state mirrored from AIAgent.__init__ so the # real interrupt() method can fan out to concurrent-tool workers. _active_children: list = [] + # ToolCallGuardrailController instance on real AIAgent. The stub + # always allows execution — these tests exercise interrupt fanout, + # not guardrail behaviour. before_call returns a decision-shaped + # object with allows_execution=True; after_call is similar. + _tool_guardrails = MagicMock() def __init__(self): # Instance-level (not class-level) so each test gets a fresh set. self._tool_worker_threads: set = set() self._tool_worker_threads_lock = threading.Lock() self._active_children_lock = threading.Lock() + # Re-assign per-instance so tests can mutate independently. + self._tool_guardrails = MagicMock() + # Decision shapes match agent.tool_guardrails.ToolGuardrailDecision — + # action="allow", allows_execution=True, should_halt=False — so + # the bound _append_guardrail_observation / _guardrail_block_result + # methods read sensible defaults instead of truthy MagicMock children. + _allow = MagicMock( + action="allow", + allows_execution=True, + should_halt=False, + code=None, + count=0, + ) + self._tool_guardrails.before_call.return_value = _allow + self._tool_guardrails.after_call.return_value = _allow def _touch_activity(self, desc): self._last_activity = time.time() @@ -77,6 +97,21 @@ def _make_agent(monkeypatch): stub._execute_tool_calls_concurrent = _ra.AIAgent._execute_tool_calls_concurrent.__get__(stub) stub.interrupt = _ra.AIAgent.interrupt.__get__(stub) stub.clear_interrupt = _ra.AIAgent.clear_interrupt.__get__(stub) + # Tool-guardrails wiring: the concurrent execution path now calls + # _append_guardrail_observation / _guardrail_block_result / + # _set_tool_guardrail_halt during the result-collection loop. These are + # thin pass-throughs over self._tool_guardrails; bind them for the stub + # the same way as other AIAgent methods. + stub._append_guardrail_observation = ( + _ra.AIAgent._append_guardrail_observation.__get__(stub) + ) + stub._guardrail_block_result = ( + _ra.AIAgent._guardrail_block_result.__get__(stub) + ) + stub._set_tool_guardrail_halt = ( + _ra.AIAgent._set_tool_guardrail_halt.__get__(stub) + ) + stub._tool_guardrail_halt_decision = None # /steer injection (added in PR #12116) fires after every concurrent # tool batch. Stub it as a no-op — this test exercises interrupt # fanout, not steer injection. @@ -107,7 +142,11 @@ def test_concurrent_interrupt_cancels_pending(monkeypatch): original_invoke = agent._invoke_tool - def slow_tool(name, args, task_id, call_id=None): + def slow_tool(name, args, task_id, call_id=None, **kwargs): + # **kwargs absorbs `messages=...` and `pre_tool_block_checked=...` + # that _invoke_tool now forwards. The test only cares about the + # positional shape; future kwargs added to _invoke_tool shouldn't + # break this fake. if name == "slow_one": # Block until the test sets the interrupt barrier.wait(timeout=10) @@ -184,7 +223,10 @@ def test_running_concurrent_worker_sees_is_interrupted(monkeypatch): observed = {"saw_true": False, "poll_count": 0, "worker_tid": None} worker_started = threading.Event() - def polling_tool(name, args, task_id, call_id=None, messages=None): + def polling_tool(name, args, task_id, call_id=None, messages=None, **kwargs): + # **kwargs absorbs pre_tool_block_checked and any future kwargs added + # to _invoke_tool — see test_concurrent_interrupt_cancels_pending for + # the same pattern. observed["worker_tid"] = threading.current_thread().ident worker_started.set() deadline = time.monotonic() + 5.0 diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 41b5194d..2f9e2869 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -753,57 +753,63 @@ def test_session_title_set_errors_when_row_lookup_fails_after_noop(monkeypatch): server._sessions.pop("sid", None) -def test_session_create_drops_pending_title_on_valueerror(monkeypatch): - unblock_agent = threading.Event() - - class _FakeWorker: - def __init__(self, key, model): - self.key = key - - def close(self): - return None - - class _FakeAgent: - model = "x" - provider = "openrouter" - base_url = "" - api_key = "" - - class _FakeDB: - def create_session(self, _key, source="tui", model=None): - return None +def test_apply_pending_session_title_drops_on_valueerror(): + """ValueError from set_session_title (e.g. duplicate title) must drop + the pending_title so a stuck title doesn't keep retrying forever. + Originally tested via the eager-apply path in _start_agent_build, which + was removed by c5b4c48 (#18370, lazy session creation) and replaced by + a post-message-complete apply that only `except Exception: pass`'d — + losing the ValueError-specific drop semantics. The helper restores + them; this test asserts that. + """ + class _RaisingDB: def set_session_title(self, _key, _title): raise ValueError("Title already in use") - def _make_agent(_sid, _key): - unblock_agent.wait(timeout=2.0) - return _FakeAgent() - - monkeypatch.setattr(server, "_make_agent", _make_agent) - monkeypatch.setattr(server, "_SlashWorker", _FakeWorker) - monkeypatch.setattr(server, "_get_db", lambda: _FakeDB()) - monkeypatch.setattr(server, "_session_info", lambda _a: {"model": "x"}) - monkeypatch.setattr(server, "_probe_credentials", lambda _a: None) - monkeypatch.setattr(server, "_wire_callbacks", lambda _sid: None) - monkeypatch.setattr(server, "_emit", lambda *a, **kw: None) - - import tools.approval as _approval - - monkeypatch.setattr(_approval, "register_gateway_notify", lambda key, cb: None) - monkeypatch.setattr(_approval, "load_permanent_allowlist", lambda: None) - - resp = server.handle_request( - {"id": "1", "method": "session.create", "params": {"cols": 80}} - ) - sid = resp["result"]["session_id"] - session = server._sessions[sid] - session["pending_title"] = "duplicate title" - unblock_agent.set() - session["agent_ready"].wait(timeout=2.0) + session = {"session_key": "k1", "pending_title": "duplicate title"} + server._apply_pending_session_title(session, "sid-1", _RaisingDB()) + + assert session["pending_title"] is None + + +def test_apply_pending_session_title_clears_on_success(): + class _OkDB: + def set_session_title(self, _key, _title): + return True + + session = {"session_key": "k2", "pending_title": "Real title"} + server._apply_pending_session_title(session, "sid-2", _OkDB()) + + assert session["pending_title"] is None + + +def test_apply_pending_session_title_retains_on_transient_exception(): + """A transient (non-ValueError) DB failure should keep the pending + title queued so the next message-complete can retry. Without this + behaviour, a single flaky DB call would silently lose the title.""" + class _FlakyDB: + def set_session_title(self, _key, _title): + raise RuntimeError("transient db blip") + + session = {"session_key": "k3", "pending_title": "Keep retrying"} + server._apply_pending_session_title(session, "sid-3", _FlakyDB()) + + assert session["pending_title"] == "Keep retrying" + + +def test_apply_pending_session_title_no_op_without_pending(): + """Helper must be a no-op when pending_title is None — most calls + look like this (every message-complete on a session that already has + a title applied).""" + class _ShouldNotBeCalledDB: + def set_session_title(self, _key, _title): + raise AssertionError("DB must not be touched when no pending title") + + session = {"session_key": "k4", "pending_title": None} + server._apply_pending_session_title(session, "sid-4", _ShouldNotBeCalledDB()) assert session["pending_title"] is None - server._sessions.pop(sid, None) def test_config_set_yolo_toggles_session_scope(): diff --git a/tests/tools/test_credential_pool_env_fallback.py b/tests/tools/test_credential_pool_env_fallback.py index 938484f0..b157bb87 100644 --- a/tests/tools/test_credential_pool_env_fallback.py +++ b/tests/tools/test_credential_pool_env_fallback.py @@ -106,10 +106,20 @@ class TestCredentialPoolSeedsFromDotEnv: assert active_sources == set() assert entries == [] - def test_os_environ_still_wins_over_dotenv(self, isolated_hermes_home, monkeypatch): - """get_env_value checks os.environ first — verify seeding picks that up.""" - _write_env_file(isolated_hermes_home, DEEPSEEK_API_KEY="sk-dotenv-stale") - monkeypatch.setenv("DEEPSEEK_API_KEY", "sk-env-fresh-xyz") + def test_dotenv_wins_over_stale_os_environ(self, isolated_hermes_home, monkeypatch): + """.env should win over a stale os.environ value. + + Inverted from the pre-#18254 behaviour. Stale env vars inherited + from parent shells (Codex CLI, test harnesses) used to shadow + deliberate updates to ~/.hermes/.env, causing auth.json to cache + an outdated key and silent 401 errors. The invariant now is: + when a key appears in both sources, .env wins. + + Sister coverage in tests/agent/test_credential_pool.py exercises + the load_pool path; this case exercises _seed_from_env directly. + """ + _write_env_file(isolated_hermes_home, DEEPSEEK_API_KEY="sk-dotenv-fresh") + monkeypatch.setenv("DEEPSEEK_API_KEY", "sk-env-stale-xyz") from agent.credential_pool import _seed_from_env entries = [] @@ -118,7 +128,7 @@ class TestCredentialPoolSeedsFromDotEnv: assert changed is True seeded = [e for e in entries if e.source == "env:DEEPSEEK_API_KEY"] assert len(seeded) == 1 - assert seeded[0].access_token == "sk-env-fresh-xyz" + assert seeded[0].access_token == "sk-dotenv-fresh" class TestAuthResolvesFromDotEnv: diff --git a/tests/tools/test_dockerfile_pid1_reaping.py b/tests/tools/test_dockerfile_pid1_reaping.py index 52532a78..8d5e38f2 100644 --- a/tests/tools/test_dockerfile_pid1_reaping.py +++ b/tests/tools/test_dockerfile_pid1_reaping.py @@ -106,8 +106,16 @@ def test_dockerfile_entrypoint_routes_through_the_init(dockerfile_text): def test_dockerfile_installs_tui_dependencies(dockerfile_text): + """The Dockerfile must install ui-tui's npm dependencies during build, + and must copy the workspace package tree (not just manifests) so npm + can resolve the @hermes/ink ``file:`` dep without falling back to the + bare manifest. See PR #16690 + a49f4c6 for the design. + """ assert "ui-tui/package.json" in dockerfile_text - assert "ui-tui/packages/hermes-ink/package-lock.json" in dockerfile_text + # ui-tui/packages/hermes-ink/ is a `file:` workspace; copying the FULL + # tree (not just its manifests) is what lets npm resolve it correctly. + # Catches the regression where someone reverts to manifest-only copies. + assert "COPY ui-tui/packages/hermes-ink/ ui-tui/packages/hermes-ink/" in dockerfile_text assert any( "ui-tui" in step and "npm" in step and (" install" in step or " ci" in step) for step in _run_steps(dockerfile_text) @@ -121,17 +129,24 @@ def test_dockerfile_builds_tui_assets(dockerfile_text): ) -def test_dockerfile_materializes_local_tui_ink_package(dockerfile_text): - assert any( - "ui-tui" in step - and "node_modules/@hermes/ink" in step - and "packages/hermes-ink" in step - and "rm -rf packages/hermes-ink/node_modules" in step - and "npm install --omit=dev" in step - and "--prefix node_modules/@hermes/ink" in step - and "rm -rf node_modules/@hermes/ink/node_modules/react" in step - and "await import('@hermes/ink')" in step - for step in _run_steps(dockerfile_text) +def test_dockerfile_forces_npm_install_links_false_for_workspace_resolution(dockerfile_text): + """The Dockerfile must force npm to install ``file:`` deps as symlinks + rather than copies. Debian's bundled npm 9.x defaults to install-links=true + (deps installed as copies), which produces a hidden + node_modules/.package-lock.json that permanently disagrees with the root + lockfile (which is generated by npm 10+ using symlinks). The disagreement + trips the TUI launcher's _tui_need_npm_install() check on every startup + and triggers an EACCES-failing runtime `npm install`. + + Replaces the older `--prefix node_modules/@hermes/ink` materialization + pattern (PR #16690) — that approach was retired in a49f4c6 in favor of + install-links=false because the materialization step was rebuilding TUI + assets unnecessarily. + """ + assert "npm_config_install_links=false" in dockerfile_text, ( + "ENV npm_config_install_links=false missing — without it, Debian npm 9.x " + "installs file: deps as copies, breaking @hermes/ink workspace resolution. " + "See PR #16690 + a49f4c6." ) diff --git a/tests/tools/test_voice_mode.py b/tests/tools/test_voice_mode.py index 1d35c486..4bc57991 100644 --- a/tests/tools/test_voice_mode.py +++ b/tests/tools/test_voice_mode.py @@ -61,6 +61,17 @@ def mock_sd(monkeypatch): # ============================================================================ class TestDetectAudioEnvironment: + @pytest.fixture(autouse=True) + def _mock_not_container(self, monkeypatch): + """detect_audio_environment routes container hosts through is_container() + and emits a hard-fail "no audio devices" warning. The tests in this + class exercise per-environment detection (SSH, WSL, clean Linux, Termux) + and shouldn't fail on container CI runners just because they happen to + run inside Docker. Mock is_container=False so the per-environment logic + runs unobstructed; tests that want the container branch can override. + """ + monkeypatch.setattr("hermes_constants.is_container", lambda: False) + def test_clean_environment_is_available(self, monkeypatch): """No SSH, Docker, or WSL — should be available.""" monkeypatch.delenv("SSH_CLIENT", raising=False) diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 724fb542..b0df6589 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -515,6 +515,50 @@ def _wait_agent(session: dict, rid: str, timeout: float = 30.0) -> dict | None: return _err(rid, 5032, err) if err else None +def _apply_pending_session_title( + session: dict, sid: str, db: object | None +) -> None: + """Apply session["pending_title"] to the DB via db.set_session_title. + + Pending titles are queued during session.create (before the DB row + exists, since c5b4c48 deferred row creation to first message) and + flushed here once a message-complete event lands. + + Outcome by branch: + - set_session_title returns truthy: pending_title cleared. + - ValueError (title invalid / duplicate): pending_title dropped, + because retrying with the same value will fail the same way. + Auto-title later picks a fresh title from message content. + - other Exception: pending_title retained — likely a transient DB + failure worth retrying on the next message-complete. + + No-ops when there is no pending title or no DB. + + Pre-c5b4c48 (#18370) the same semantics lived inline in + _start_agent_build. Extracting them here both restores the lost + ValueError handling and makes the invariant testable without + simulating a full message turn. + """ + pending = session.get("pending_title") + if not pending or db is None: + return + key = session.get("session_key") or sid + try: + if db.set_session_title(key, pending): + session["pending_title"] = None + except ValueError as exc: + # Title invalid / duplicate — retrying is futile; drop and let + # auto-title pick something. + session["pending_title"] = None + logger.info("Dropping pending title for session %s: %s", sid, exc) + except Exception: + # Likely transient — keep pending_title so the next + # message-complete can retry. Auto-title is the eventual fallback. + logger.warning( + "Failed to apply pending title for session %s", sid, exc_info=True, + ) + + def _start_agent_build(sid: str, session: dict) -> None: """Start building the real AIAgent for a TUI session, once. @@ -2982,15 +3026,8 @@ def _run_prompt_submit(rid, sid: str, session: dict, text: Any) -> None: _emit("message.complete", sid, payload) # Apply pending_title now that the DB row exists. - _pending = session.get("pending_title") - if _pending and status == "complete": - _pdb = _get_db() - if _pdb: - try: - if _pdb.set_session_title(session.get("session_key") or sid, _pending): - session["pending_title"] = None - except Exception: - pass # Best effort — auto-title will handle it below + if status == "complete": + _apply_pending_session_title(session, sid, _get_db()) if ( status == "complete"