From 447801cbcb33c3b8dff2bc9dcbf704f2e7abe05c Mon Sep 17 00:00:00 2001 From: claude-ceo-assistant Date: Fri, 8 May 2026 03:33:40 +0000 Subject: [PATCH 01/10] test(acp): update commands list snapshot for steer + queue acp_adapter/server.py:_ADVERTISED_COMMANDS now includes "steer" (inject guidance into active turn) and "queue" (run prompt after current turn finishes) between "compact" and "version". Production code is the source of truth; this test was the last reader still on the pre-feature snapshot. The substantive features were added in PRs that introduced steer/queue themselves; this is purely test-snapshot follow-through. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/acp/test_server.py | 2 ++ 1 file changed, 2 insertions(+) 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( -- 2.45.2 From 7672bd9b4f96be99f6616bae479779ff3948dfb1 Mon Sep 17 00:00:00 2001 From: claude-ceo-assistant Date: Fri, 8 May 2026 03:34:17 +0000 Subject: [PATCH 02/10] test(credential-pool): invert obsolete os.environ-wins test for #18254 fix The stale invariant "os.environ wins over .env" was deliberately inverted in 2ef1ad2 ("fix: prefer ~/.hermes/.env over os.environ when seeding credential pool"). The fix targets the case where a parent shell (Codex CLI, harness scripts) exports a stale OPENROUTER_API_KEY, the user updates ~/.hermes/.env with a fresh value, and Hermes silently 401s because auth.json cached the stale env-var. Rename + invert this test to assert the new invariant (.env wins). The positive load_pool coverage already exists in tests/agent/test_credential_pool.py::test_load_pool_prefers_dotenv_over_stale_os_environ (added in 0a6865b alongside the fix); this case still serves a purpose because it exercises _seed_from_env directly, which is a separate code path from load_pool. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../test_credential_pool_env_fallback.py | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) 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: -- 2.45.2 From ed0957256bc92b98940f4c319181b2b5104d73be Mon Sep 17 00:00:00 2001 From: claude-ceo-assistant Date: Fri, 8 May 2026 03:35:28 +0000 Subject: [PATCH 03/10] test(gateway-service): compute expected TimeoutStopSec from SSOT, not literal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The literal "TimeoutStopSec=90" assertion was correct when default restart_drain_timeout was 60s (max(60,60)+30=90). The default was bumped to 180s in hermes_cli/config.py, making the unit emit TimeoutStopSec=210 — still satisfying issue #8202 ("timeout must exceed drain so systemd doesn't SIGKILL pre-cleanup") but breaking the literal assertion. Refactor: compute the expected minimum from gateway.restart.DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT and assert >=. The test now expresses the invariant directly and survives future drain bumps. Mutation-tested mentally: deleting the +30 headroom in hermes_cli/gateway.py:1636 still fails this test. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/hermes_cli/test_gateway_service.py | 36 +++++++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/tests/hermes_cli/test_gateway_service.py b/tests/hermes_cli/test_gateway_service.py index f2bfa8b8..7011b1c0 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 @@ -107,6 +109,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 +129,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 +155,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 -- 2.45.2 From 74d5e5a89935fcf18ce76e581211f50f3dd97e29 Mon Sep 17 00:00:00 2001 From: claude-ceo-assistant Date: Fri, 8 May 2026 03:38:13 +0000 Subject: [PATCH 04/10] test(dockerfile): rewrite tui-resolution invariants for post-a49f4c6 design Two contract tests guarded properties of the now-retired @hermes/ink materialization dance (--prefix node_modules/@hermes/ink + omit=dev + nested react cleanup + await-import smoke). The Dockerfile was restructured in a49f4c6 ("fix: prevent tui rebuilding assets") to instead: 1. Copy the full ui-tui/packages/hermes-ink/ tree (not just manifests) 2. Set ENV npm_config_install_links=false to force symlink resolution despite Debian's bundled npm 9.x defaulting to install-links=true The new mechanism solves the same workspace-resolution problem with fewer build steps (no rebuild on container start). Update the tests to guard the new invariants, both designed to catch a regression that would break @hermes/ink at runtime: - test_dockerfile_installs_tui_dependencies now asserts the full packages/hermes-ink/ COPY (catches manifest-only revert) - new test_dockerfile_forces_npm_install_links_false_for_workspace_resolution asserts the env var (catches accidental removal that re-introduces the npm 9 install-as-copy bug) Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/tools/test_dockerfile_pid1_reaping.py | 39 ++++++++++++++------- 1 file changed, 27 insertions(+), 12 deletions(-) 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." ) -- 2.45.2 From 5f179d6d352dd17b71f7681644fd6cc0cf46eb7c Mon Sep 17 00:00:00 2001 From: claude-ceo-assistant Date: Fri, 8 May 2026 03:41:31 +0000 Subject: [PATCH 05/10] test(run_agent): keep concurrent-interrupt stub in sync with AIAgent surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stub in tests/run_agent/test_concurrent_interrupt.py mirrors a subset of AIAgent attributes/methods deliberately ("Avoid full AIAgent init — just import the class and build a stub"). Two recent additions to the real AIAgent broke it: 1. Tool-call guardrails: AIAgent gained _tool_guardrails (line 1160) + _append_guardrail_observation / _guardrail_block_result / _set_tool_guardrail_halt that the concurrent-execution path now invokes during result collection. Add a MagicMock guardrail controller whose decision objects mirror the ToolGuardrailDecision shape (action="allow", allows_execution=True, should_halt=False) — the bound methods read sensible defaults instead of truthy MagicMock children. Bind the three methods the same way as other AIAgent methods. 2. _invoke_tool gained kwargs: messages= and pre_tool_block_checked= are now forwarded by the concurrent path. The two test fakes (slow_tool, polling_tool) didn't accept them and raised TypeError on every call. Add **kwargs so future kwargs additions don't break these fakes. These are pure stub-drift fixes — production behaviour is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/run_agent/test_concurrent_interrupt.py | 46 +++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) 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 -- 2.45.2 From d70d6b1ab30195ce9fc65e193c923972f8d6709e Mon Sep 17 00:00:00 2001 From: claude-ceo-assistant Date: Fri, 8 May 2026 03:43:43 +0000 Subject: [PATCH 06/10] test(update-restart): model successful primary-kill in find_gateway_pids mocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cmd_update gained a post-restart survivor sweep in hermes_cli/main.py (around line 7568, for issue #17648): after the primary kill attempts, sleep 3s, re-poll find_gateway_pids, and SIGKILL any PIDs from killed_pids that are still alive. This catches gateways that ignore SIGTERM (stuck drain, blocked I/O, zombies). Three tests patched find_gateway_pids with a static return_value (or a fake that ignored call count), so the sweep always saw the killed PIDs as still alive and fired an extra SIGKILL — contaminating the kill-call assertions: - test_update_restarts_profile_manual_gateways saw kill called 1× (SIGKILL from sweep) when expecting 0 - test_update_profile_manual_gateway_falls_back_to_sigterm saw kill called 2× (SIGTERM + SIGKILL from sweep) when expecting 1 - test_update_kills_manual_pid_but_not_service_pid saw 2 kills on the manual PID instead of 1 Real behaviour: after SIGTERM lands, the process exits and the next find_gateway_pids() returns []. The static mocks didn't model that. Make each find_gateway_pids mock call-count-aware: first call returns the live PID(s), subsequent calls return empty / service-only — modelling a successful primary kill. The survivor sweep then sees nothing to escalate, so the original assertions hold. Tests that want to exercise the sweep path can opt in by keeping the PID alive across calls; none of the existing tests need that. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../hermes_cli/test_update_gateway_restart.py | 37 ++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) 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} -- 2.45.2 From f88e0222297025f02f938cc010d1801d935bbcbc Mon Sep 17 00:00:00 2001 From: claude-ceo-assistant Date: Fri, 8 May 2026 03:46:57 +0000 Subject: [PATCH 07/10] test(teams): patch adapter's TypingActivityInput binding for test_send_typing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The teams adapter imports TypingActivityInput at module load time: try: from microsoft_teams.api.activities.typing import TypingActivityInput except ImportError: TypingActivityInput = None When the real microsoft_teams package isn't installed (CI runner image doesn't bundle Microsoft Teams SDK), the import fails and the local binding stays None — even though the test file's _ensure_teams_mock fixture registers a MockTypingActivityInput in sys.modules. The test-time mock-in-sys.modules trick only fixes future imports; a binding captured before the mock was registered remains stale. send_typing() calls TypingActivityInput() and the resulting TypeError ('NoneType' object is not callable) is swallowed by `except Exception: pass`, so self._app.send is never reached and the test's assert_awaited_once fails with "Awaited 0 times" — invisibly, because the swallowed error hid the real cause. Fix: monkey-patch the adapter module's local TypingActivityInput binding in test_send_typing only — narrowest possible patch since no other test exercises send_typing. Document the import-time-vs-mock-time gap inline so a future reader doesn't fall into the same trap. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/gateway/test_teams.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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() -- 2.45.2 From c04e05f05c9c0f0ab31d1ebc89f6c587755df0dd Mon Sep 17 00:00:00 2001 From: claude-ceo-assistant Date: Fri, 8 May 2026 03:50:39 +0000 Subject: [PATCH 08/10] fix(tui_gateway): drop pending_title on ValueError, retain on transient errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production bug + missing test coverage. c5b4c48 ("fix: lazy session creation — defer DB row until first message (#18370)") moved pending_title application from the eager _start_agent_build path to a post-message-complete handler. The original block had: except ValueError as e: current["pending_title"] = None logger.info("Dropping pending title for session %s: %s", sid, e) except Exception: logger.warning("Failed to apply pending title ...", exc_info=True) …differentiating "title is invalid / duplicate, retrying won't help" (ValueError, drop) from "transient DB failure, retry on next message" (other Exception, keep + log). The replacement block collapsed both into: except Exception: pass # Best effort — auto-title will handle it below …so a duplicate-title session keeps the same dud pending_title forever, hitting set_session_title with the same losing argument on every message-complete. Auto-title can't kick in because pending_title still shadows it. Fix: extract a documented _apply_pending_session_title helper that restores the three-branch semantics (success → clear, ValueError → drop, other Exception → retain). Call it from the message-complete handler instead of the inline try/except. Test rewrite: the previous test_session_create_drops_pending_title_on_valueerror exercised an obsolete code path (eager apply during session.create) that no longer existed after c5b4c48. Replace with four focused tests against the helper: - drops_on_valueerror — invariant from the original test name - clears_on_success — happy path - retains_on_transient_exception — guards the new "don't lose title on a flaky DB" behaviour - no_op_without_pending — most calls hit this path Mutation-tested mentally: deleting the `session["pending_title"] = None` in the ValueError branch fails drops_on_valueerror; deleting the same in the success branch fails clears_on_success; widening except ValueError to except Exception fails retains_on_transient_exception. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/test_tui_gateway_server.py | 96 +++++++++++++++++--------------- tui_gateway/server.py | 55 +++++++++++++++--- 2 files changed, 97 insertions(+), 54 deletions(-) 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/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" -- 2.45.2 From c7ec523a213daa530e7a3a47a24520eb8b0efaf7 Mon Sep 17 00:00:00 2001 From: claude-ceo-assistant Date: Fri, 8 May 2026 04:00:44 +0000 Subject: [PATCH 09/10] test(env-isolation): mock is_container + _preflight_user_systemd in tests asserting non-container behaviour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production code grew two environment-detection chains that test mocks didn't anticipate, causing 10 tests to fail on container CI runners without testing what their assertions claimed: 1. supports_systemd_services() routes container hosts through _container_systemd_operational(), which returns False inside the act_runner Docker. Two tests (test_native_linux, test_supports_systemd_services_returns_true_when_systemctl_present) mocked is_linux/is_termux/is_wsl but not is_container, so they hit the container branch and got False instead of True. Add the missing monkeypatch. 2. systemd_start / systemd_restart now call _preflight_user_systemd() which probes the user D-Bus socket and raises UserSystemdUnavailableError on its absence (common in containers and fresh SSH sessions without linger). Four tests (test_systemd_start_refreshes_outdated_unit, test_systemd_restart_refreshes_outdated_unit, test_systemd_restart_self_requests_graceful_restart_and_waits, test_systemd_restart_recovers_failed_planned_restart) exercise the unit-refresh / self-restart routing logic and shouldn't care about D-Bus availability. Mock _preflight_user_systemd as a no-op. 3. detect_audio_environment() routes container hosts through is_container() and emits a hard-fail "no audio devices" warning. Four tests in TestDetectAudioEnvironment assert per-environment detection (clean Linux, WSL with PulseAudio, Termux) and expect `available is True`; the container check overrode them. Add a class-level autouse fixture that mocks is_container=False so the per-environment logic runs unobstructed. These are deliberate "isolate the unit under test from environmental concerns" patches — production code is not changed. Tests that want to exercise the container/D-Bus branches can opt in by overriding the mocks. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/hermes_cli/test_gateway_service.py | 20 ++++++++++++++++++++ tests/hermes_cli/test_gateway_wsl.py | 6 +++++- tests/tools/test_voice_mode.py | 11 +++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/tests/hermes_cli/test_gateway_service.py b/tests/hermes_cli/test_gateway_service.py index 7011b1c0..75dacff1 100644 --- a/tests/hermes_cli/test_gateway_service.py +++ b/tests/hermes_cli/test_gateway_service.py @@ -66,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 = [] @@ -89,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 = [] @@ -465,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 @@ -515,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", @@ -568,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/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) -- 2.45.2 From 4d71bcb47ddf084e96c9693ba67ff9cb0a4d7302 Mon Sep 17 00:00:00 2001 From: claude-ceo-assistant Date: Fri, 8 May 2026 04:05:51 +0000 Subject: [PATCH 10/10] chore(release): map claude-ceo-assistant email for AUTHOR_MAP The contributor-check.yml workflow requires every commit author email to have an entry in scripts/release.py:AUTHOR_MAP. claude-ceo-assistant is the Gitea-only bot identity used by Claude-Code-driven PRs in the molecule-ai fork (introduced post-2026-05-06 GitHub suspension; no upstream/GitHub equivalent). Register it so PRs from that identity pass the attribution check. Pattern matches recent same-shape commits: 73bcd83, 50f9f38, 9c626ef. Co-Authored-By: Claude Opus 4.7 (1M context) --- scripts/release.py | 3 +++ 1 file changed, 3 insertions(+) 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", -- 2.45.2