Compare commits

..

1 Commits

Author SHA1 Message Date
dev-lead
53a01a8400 fix(systemd): align tests with production contract — D-Bus stubs + TimeoutStopSec (partial close hermes-agent#9)
Some checks failed
Nix / nix (macos-latest) (pull_request) Waiting to run
Contributor Attribution Check / check-attribution (pull_request) Failing after 43s
Supply Chain Audit / Scan PR for critical supply chain risks (pull_request) Successful in 38s
Tests / e2e (pull_request) Failing after 1m12s
Tests / test (pull_request) Failing after 1m31s
Nix / nix (ubuntu-latest) (pull_request) Failing after 31m1s
Two distinct sub-shapes converged in the seven systemd test failures
tracked under hermes-agent#9.  Both are addressed here without changing
production code; the production contract is correct on both axes.

## Sub-shape 1 — TimeoutStopSec=210 vs 90 (2 tests)

`TestGeneratedSystemdUnits::test_user_unit_avoids_recursive_execstop_and_uses_extended_stop_timeout`
and its system-scope sibling pin `TimeoutStopSec=90`.  The unit
generator computes `restart_timeout = max(60, drain_timeout) + 30s`
of post-interrupt cleanup headroom (gateway.py L1635).

PR #18761 (2026-05-02) intentionally raised the default
`agent.restart_drain_timeout` from 60s to 180s after a /restart on
2026-05-02 force-interrupted three mid-API-call agents inside the old
60s budget.  The new arithmetic is therefore `max(60, 180) + 30 = 210s`,
and both unit generators produce `TimeoutStopSec=210`.

The tests are updated to derive the expected value from
`DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT` (already imported at the top of
the file) so future drain-timeout changes flag the contract drift in
one place.  An explicit `expected_timeout == 210` assert + commit
reference keeps the rationale visible at the failure site.

## Sub-shape 2 — D-Bus / loginctl mocks (5 tests)

`systemd_start()` and `systemd_restart()` (user scope) gained a
`_preflight_user_systemd()` call that probes `/run/user/$UID/{bus,
systemd/private}` and, if both are missing, runs `loginctl
enable-linger $USER` to bring the user@.service socket up.  None of
that is reachable from a unit-test fixture: the test runner has no
real user D-Bus session, the linger probe touches the live filesystem,
and `loginctl enable-linger runner` falls outside the
`fake_subprocess_run` whitelist these tests pin.

Five tests were therefore failing not on production behavior but on
their fixtures missing a setup that the production code path can't
have in a test:

  - TestSystemdServiceRefresh::test_systemd_start_refreshes_outdated_unit
  - TestSystemdServiceRefresh::test_systemd_restart_refreshes_outdated_unit
  - TestGatewaySystemServiceRouting::test_systemd_restart_self_requests_graceful_restart_and_waits
  - TestGatewaySystemServiceRouting::test_systemd_restart_recovers_failed_planned_restart
  - TestGatewayServiceDetection::test_supports_systemd_services_returns_true_when_systemctl_present
    (different shape — see below)
  - TestSupportsSystemdServicesWSL::test_native_linux (same shape as
    the detection test)

Each of the four restart/start tests now stubs `_preflight_user_systemd`
with a no-op.  The preflight contract itself is exercised end-to-end
by the existing `TestPreflightUserSystemd` suite (six tests), so this
stub doesn't paper over any real bug — it only narrows the assertion
to what the test was actually meant to check (refresh + systemctl call
shape).

For the two `supports_systemd_services()` tests, the missing fixture
was `is_container()`.  CI runners frequently expose `/.dockerenv`,
which makes `is_container()` return True at the gateway layer and
sends the call into the `_container_systemd_operational()` branch
(real systemctl probe).  Both tests model a native-Linux host, so
they pin `is_container=False`; the WSL one also pins
`shutil.which("systemctl")` to a non-None value so the early
binary-presence check doesn't short-circuit.

## Verification

  - 7/7 named tests in the brief now pass on a clean macOS Python 3.13
    venv (no real systemd present).
  - Set-diff of `pytest tests/hermes_cli/ -q` failure list before vs
    after the change: 7 fewer failures, 0 added (residual non-systemd
    failures match between baseline and branch — same set, just
    reordered by xdist).
  - `pytest tests/gateway/ -q`: same 14 pre-existing failures as on
    main; no new regressions.

The post-fix contract:

  - User and system units write `TimeoutStopSec=210` while
    `restart_drain_timeout=180s` (default).  The formula is pinned
    in test as `max(60, drain_timeout) + 30` so future drain bumps
    fail loudly in one place.
  - `_preflight_user_systemd()` probe sequence (still owned by
    `TestPreflightUserSystemd`): D-Bus / private-socket exists →
    short-circuit; else linger-on → wait 3s; else `loginctl
    enable-linger $USER` → wait 5s; else raise
    `UserSystemdUnavailableError` with remediation hint.
2026-05-08 14:11:19 -07:00
12 changed files with 119 additions and 265 deletions

View File

@ -200,8 +200,6 @@ class TestSessionOps:
"context",
"reset",
"compact",
"steer",
"queue",
"version",
]
model_cmd = next(

View File

@ -397,22 +397,13 @@ class TestTeamsSend:
assert "Network error" in result.error
@pytest.mark.asyncio
async def test_send_typing(self, monkeypatch):
async def test_send_typing(self):
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()

View File

@ -64,11 +64,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")
# Production now preflights user-systemd availability (loginctl
# enable-linger + D-Bus socket wait, #14531) before start/restart.
# These unit tests assert the systemctl call sequence, not the
# preflight — stub the preflight as a no-op so the fake subprocess
# runner doesn't have to reproduce the loginctl/D-Bus dance.
# Test fixtures don't have a real user D-Bus session — stub the preflight
# so the test exercises the unit-refresh + systemctl call sequence rather
# than the linger / loginctl probe (covered by TestPreflightUserSystemd).
monkeypatch.setattr(gateway_cli, "_preflight_user_systemd", lambda *a, **kw: None)
calls = []
@ -93,8 +91,8 @@ 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 note on test_systemd_start_refreshes_outdated_unit — preflight
# is a separate concern and has its own dedicated coverage.
# Same rationale as test_systemd_start_refreshes_outdated_unit: this test
# validates the unit-refresh + systemctl call shape, not the D-Bus probe.
monkeypatch.setattr(gateway_cli, "_preflight_user_systemd", lambda *a, **kw: None)
calls = []
@ -117,15 +115,6 @@ class TestSystemdServiceRefresh:
class TestGeneratedSystemdUnits:
@staticmethod
def _expected_timeout_stop_sec() -> int:
# Mirror the formula in gateway.generate_systemd_unit:
# restart_timeout = max(60, drain_timeout) + 30
# so that bumping the default drain_timeout in config doesn't silently
# break this test — we want to pin the relationship, not a magic number.
drain_timeout = int(gateway_cli._get_restart_drain_timeout() or 0)
return max(60, drain_timeout) + 30
def test_user_unit_avoids_recursive_execstop_and_uses_extended_stop_timeout(self):
unit = gateway_cli.generate_systemd_unit(system=False)
@ -133,13 +122,20 @@ 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 configured drain_timeout so
# systemd doesn't SIGKILL the cgroup before post-interrupt cleanup
# (tool subprocess kill, adapter disconnect) runs — issue #8202.
# Formula is max(60, drain_timeout) + 30; pin the relationship to
# _get_restart_drain_timeout() rather than a literal so a config
# default bump (default jumped 60→180s) doesn't silently regress us.
assert f"TimeoutStopSec={self._expected_timeout_stop_sec()}" in unit
# TimeoutStopSec must exceed the default drain_timeout so systemd
# doesn't SIGKILL the cgroup before post-interrupt cleanup (tool
# subprocess kill, adapter disconnect) runs — issue #8202.
# gateway.py: restart_timeout = max(60, drain_timeout) + 30s headroom.
# PR #18761 (2026-05-02) raised the default drain_timeout from 60s
# to 180s after a /restart force-interrupted three mid-API-call
# agents inside the old 60s budget, so the expected TimeoutStopSec
# is now 180 + 30 = 210s.
expected_timeout = max(60, int(DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT)) + 30
assert f"TimeoutStopSec={expected_timeout}" in unit
assert expected_timeout == 210, (
"drain_timeout default changed; update DEFAULT_CONFIG and this "
"test in lockstep — see PR #18761 commit message for rationale."
)
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)
@ -155,13 +151,16 @@ 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 configured drain_timeout so
# systemd doesn't SIGKILL the cgroup before post-interrupt cleanup
# (tool subprocess kill, adapter disconnect) runs — issue #8202.
# Formula is max(60, drain_timeout) + 30; pin the relationship to
# _get_restart_drain_timeout() rather than a literal so a config
# default bump (default jumped 60→180s) doesn't silently regress us.
assert f"TimeoutStopSec={self._expected_timeout_stop_sec()}" in unit
# See test_user_unit_avoids_recursive_execstop_and_uses_extended_stop_timeout
# for the TimeoutStopSec rationale; both unit generators share the same
# max(60, drain_timeout) + 30s formula so the user and system units stay
# in lockstep on this contract.
expected_timeout = max(60, int(DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT)) + 30
assert f"TimeoutStopSec={expected_timeout}" in unit
assert expected_timeout == 210, (
"drain_timeout default changed; update DEFAULT_CONFIG and this "
"test in lockstep — see PR #18761 commit message for rationale."
)
assert "WantedBy=multi-user.target" in unit
@ -461,9 +460,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)
# Native-Linux assertion: explicitly opt out of the container path
# (added after this test was written) so a containerized CI runner
# doesn't inherit a probe of the real systemd in the runner image.
# supports_systemd_services() also branches on is_container() — without
# this stub the test runs the container fallback which calls real
# systemctl, returning False on a CI runner that has /.dockerenv but
# no working systemd. The test models a native-Linux host, so pin it.
monkeypatch.setattr(gateway_cli, "is_container", lambda: False)
monkeypatch.setattr(gateway_cli.shutil, "which", lambda name: "/usr/bin/systemctl")
@ -515,10 +515,9 @@ class TestGatewaySystemServiceRouting:
calls = []
monkeypatch.setattr(gateway_cli, "_select_systemd_scope", lambda system=False: False)
# Production now preflights user-systemd availability (loginctl
# enable-linger + D-Bus socket wait, #14531) before restart. This
# test exercises the restart routing path; preflight has its own
# dedicated coverage in TestUserSystemdPrivateSocketPreflight.
# systemd_restart() calls _preflight_user_systemd() before refreshing
# the unit; the test fixture has no real D-Bus session so stub it. The
# preflight contract itself is covered by TestPreflightUserSystemd below.
monkeypatch.setattr(gateway_cli, "_preflight_user_systemd", lambda *a, **kw: None)
monkeypatch.setattr(gateway_cli, "refresh_systemd_unit_if_needed", lambda system=False: calls.append(("refresh", system)))
monkeypatch.setattr(
@ -574,8 +573,7 @@ class TestGatewaySystemServiceRouting:
def test_systemd_restart_recovers_failed_planned_restart(self, monkeypatch, capsys):
monkeypatch.setattr(gateway_cli, "_select_systemd_scope", lambda system=False: False)
# See note on test_systemd_restart_self_requests_graceful_restart_and_waits
# — preflight is a separate concern with dedicated coverage.
# See sibling test for preflight stub rationale.
monkeypatch.setattr(gateway_cli, "_preflight_user_systemd", lambda *a, **kw: None)
monkeypatch.setattr(gateway_cli, "refresh_systemd_unit_if_needed", lambda system=False: None)
monkeypatch.setattr(

View File

@ -141,18 +141,16 @@ class TestSupportsSystemdServicesWSL:
assert gateway.supports_systemd_services() is False
def test_native_linux(self, monkeypatch):
"""Native Linux (not WSL, not container) → True without further probing."""
"""Native Linux (not WSL) → True without checking systemd."""
monkeypatch.setattr(gateway, "is_linux", lambda: True)
monkeypatch.setattr(gateway, "is_termux", lambda: False)
monkeypatch.setattr(gateway, "is_wsl", lambda: False)
# supports_systemd_services() now also branches on is_container() to
# decide whether to probe `systemctl is-system-running` — explicitly
# opt this case out of the container path so a containerized CI
# runner doesn't inherit the probe of the runner image's systemd.
# Pin is_container() too: CI runners often have /.dockerenv which would
# send supports_systemd_services() down the _container_systemd_operational()
# branch (calls real systemctl) and return False here.
monkeypatch.setattr(gateway, "is_container", lambda: False)
# On macOS dev boxes shutil.which("systemctl") returns None; stub it
# so the test exercises the native-Linux branch independently of the
# host's $PATH.
# Make sure a systemctl binary appears present so we don't trip the
# which() short-circuit in supports_systemd_services().
monkeypatch.setattr(gateway.shutil, "which", lambda name: "/usr/bin/systemctl")
assert gateway.supports_systemd_services() is True

View File

@ -478,19 +478,9 @@ def test_ws_events_rejects_when_token_required(tmp_path, monkeypatch):
kb.init_db()
# Stub web_server so _check_ws_token has a token to compare against.
# NOTE: monkeypatch.setitem(sys.modules, ...) alone is not enough.
# If another test in the same xdist worker has already imported
# hermes_cli.web_server, the parent package `hermes_cli` has the real
# module bound as an attribute. `from hermes_cli import web_server`
# then resolves via the attribute, NOT sys.modules — so the stub is
# bypassed and _check_ws_token compares against the real (random)
# _SESSION_TOKEN, rejecting our "secret-xyz" branch with 1008.
# Patching the parent package attribute keeps both lookup paths in sync.
import types
import hermes_cli
stub = types.SimpleNamespace(_SESSION_TOKEN="secret-xyz")
monkeypatch.setitem(sys.modules, "hermes_cli.web_server", stub)
monkeypatch.setattr(hermes_cli, "web_server", stub, raising=False)
app = FastAPI()
app.include_router(_load_plugin_router(), prefix="/api/plugins/kanban")

View File

@ -20,7 +20,6 @@ def _make_agent(monkeypatch):
monkeypatch.setenv("HERMES_INFERENCE_PROVIDER", "")
# Avoid full AIAgent init — just import the class and build a stub
import run_agent as _ra
from agent.tool_guardrails import ToolCallGuardrailController
class _Stub:
_interrupt_requested = False
@ -54,12 +53,6 @@ def _make_agent(monkeypatch):
self._tool_worker_threads: set = set()
self._tool_worker_threads_lock = threading.Lock()
self._active_children_lock = threading.Lock()
# Mirror AIAgent.__init__ (run_agent.py:1160 — added in 58b89965
# "fix(agent): add tool-call loop guardrails", 2026-04-27).
# _execute_tool_calls_concurrent calls self._tool_guardrails
# .before_call(...) on every tool, so the stub needs a real
# controller instance with default (warning-only) config.
self._tool_guardrails = ToolCallGuardrailController()
def _touch_activity(self, desc):
self._last_activity = time.time()
@ -84,14 +77,6 @@ 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-loop guardrails (added in 58b89965, 2026-04-27) are invoked
# before/after every concurrent tool. Bind the real helpers — the
# default ToolCallGuardrailController() above is warning-only so
# they never block a tool, just observe.
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 = lambda *a, **kw: None
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.
@ -122,9 +107,7 @@ def test_concurrent_interrupt_cancels_pending(monkeypatch):
original_invoke = agent._invoke_tool
def slow_tool(name, args, task_id, call_id=None, **kwargs):
# **kwargs swallows production-only kwargs (messages,
# pre_tool_block_checked) added to _invoke_tool over time.
def slow_tool(name, args, task_id, call_id=None):
if name == "slow_one":
# Block until the test sets the interrupt
barrier.wait(timeout=10)
@ -201,9 +184,7 @@ 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, **kwargs):
# **kwargs swallows production-only kwargs (pre_tool_block_checked)
# added to _invoke_tool over time.
def polling_tool(name, args, task_id, call_id=None, messages=None):
observed["worker_tid"] = threading.current_thread().ident
worker_started.set()
deadline = time.monotonic() + 5.0

View File

@ -753,63 +753,57 @@ def test_session_title_set_errors_when_row_lookup_fails_after_noop(monkeypatch):
server._sessions.pop("sid", 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.
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
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")
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())
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)
assert session["pending_title"] is None
server._sessions.pop(sid, None)
def test_config_set_yolo_toggles_session_scope():

View File

@ -106,20 +106,10 @@ class TestCredentialPoolSeedsFromDotEnv:
assert active_sources == set()
assert entries == []
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")
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")
from agent.credential_pool import _seed_from_env
entries = []
@ -128,7 +118,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-dotenv-fresh"
assert seeded[0].access_token == "sk-env-fresh-xyz"
class TestAuthResolvesFromDotEnv:

View File

@ -106,18 +106,8 @@ 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 @hermes/ink workspace tree (not just its manifests)
so npm can resolve the ``file:`` workspace dep without falling back to
the bare manifest. See PR #16690 + a49f4c6 for the design.
"""
assert "ui-tui/package.json" in dockerfile_text
# ui-tui/packages/hermes-ink/ is referenced as a `file:` workspace dep
# from ui-tui/package.json. Copying the FULL tree (rather than just
# package.json + package-lock.json as in earlier revisions) is what lets
# npm resolve the workspace to real content. This assertion catches a
# regression that reverts to manifest-only copies.
assert "COPY ui-tui/packages/hermes-ink/ ui-tui/packages/hermes-ink/" in dockerfile_text
assert "ui-tui/packages/hermes-ink/package-lock.json" 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)
@ -131,33 +121,17 @@ def test_dockerfile_builds_tui_assets(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). The host-side ``ui-tui/package-lock.json`` is
generated by npm 10+ which uses symlinks, so an install-as-copy in the
image produces a hidden ``node_modules/.package-lock.json`` that
permanently disagrees with the root lockfile on the @hermes/ink entry.
That disagreement trips the TUI launcher's ``_tui_need_npm_install()``
check on every startup and triggers a runtime ``npm install`` that
fails with EACCES (node_modules/ is root-owned from build time).
This assertion replaces the older ``--prefix node_modules/@hermes/ink``
materialization smoke test (PR #16690), which was retired in a49f4c6
in favour of ``install-links=false`` because the materialization step
rebuilt TUI assets unnecessarily on every container start.
"""
instructions = _dockerfile_instructions(dockerfile_text)
has_env_directive = any(
instr.startswith("ENV ") and "npm_config_install_links=false" in instr
for instr in instructions
)
assert has_env_directive, (
"ENV npm_config_install_links=false missing — without it, Debian npm 9.x "
"installs `file:` deps as copies, breaking @hermes/ink workspace "
"resolution at runtime. See PR #16690 + a49f4c6."
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)
)

View File

@ -61,16 +61,6 @@ def mock_sd(monkeypatch):
# ============================================================================
class TestDetectAudioEnvironment:
@pytest.fixture(autouse=True)
def _isolate_container_detection(self, monkeypatch):
"""Default `is_container` to False so tests don't inherit the host
runner's container state (e.g. CI itself runs inside Docker, where
the production `is_container()` returns True via /.dockerenv or
/proc/1/cgroup and silently appended a 'Running inside Docker'
warning to every scenario). Individual tests opt in via setattr.
"""
monkeypatch.setattr("tools.voice_mode.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)
@ -95,20 +85,6 @@ class TestDetectAudioEnvironment:
assert result["available"] is False
assert any("SSH" in w for w in result["warnings"])
def test_docker_container_blocks_voice(self, monkeypatch):
"""Running inside a Docker/Podman container should block voice mode."""
monkeypatch.delenv("SSH_CLIENT", raising=False)
monkeypatch.delenv("SSH_TTY", raising=False)
monkeypatch.delenv("SSH_CONNECTION", raising=False)
monkeypatch.setattr("tools.voice_mode.is_container", lambda: True)
monkeypatch.setattr("tools.voice_mode._import_audio",
lambda: (MagicMock(), MagicMock()))
from tools.voice_mode import detect_audio_environment
result = detect_audio_environment()
assert result["available"] is False
assert any("Docker container" in w for w in result["warnings"])
def test_wsl_without_pulse_blocks_voice(self, monkeypatch, tmp_path):
"""WSL without PULSE_SERVER should block voice mode."""
monkeypatch.delenv("SSH_CLIENT", raising=False)

View File

@ -49,7 +49,7 @@ def _audio_available() -> bool:
return False
from hermes_constants import is_container, is_termux as _is_termux_environment
from hermes_constants import is_termux as _is_termux_environment
def _voice_capture_install_hint() -> str:
@ -103,6 +103,7 @@ def detect_audio_environment() -> dict:
warnings.append("Running over SSH -- no audio devices available")
# Docker/Podman container detection
from hermes_constants import is_container
if is_container():
warnings.append("Running inside Docker container -- no audio devices")

View File

@ -515,50 +515,6 @@ 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.
@ -3026,8 +2982,15 @@ 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.
if status == "complete":
_apply_pending_session_title(session, sid, _get_db())
_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"