From 31e22a152bf260bf51df9fddfdd93fbea4fd3741 Mon Sep 17 00:00:00 2001 From: happy5318 Date: Mon, 21 Sep 2026 14:20:39 +0800 Subject: [PATCH 1/4] feat(api): make _FULL_SESSION_RESOLVE_MAX_CONCURRENT configurable via env (#7421) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [rebase onto current master — same content as 5a7f7d8, no functional changes beyond a single context-line merge from master] Squash-rebase of #7656 onto current master so the 3-way merge CI test no longer pulls in the unmerged stage/7618-7619-renderer-i18n dirty diff that 5a7f7d8 was based on. The actual change set is unchanged: 2 files, +231/-1. What changed (from #7656 / 5a7f7d8): - api/models.py::_read_max_session_resolve_concurrent — new module-level helper that reads the cap from the env var HERMES_WEBUI_MAX_SESSION_RESOLVE with strict validation (positive int in [1, 64], malformed → default 2). - tests/test_issue7421_resolve_cap_config.py — 12 regression tests covering unset/empty/whitespace, headline cases (4/8/64), safety paths (0/-1/999999/non-numeric), whitespace tolerance, BoundedSemaphore capacity check, and a static wiring assertion. Model: MiniMax-M3 --- api/models.py | 29 ++- tests/test_issue7421_resolve_cap_config.py | 202 +++++++++++++++++++++ 2 files changed, 230 insertions(+), 1 deletion(-) create mode 100644 tests/test_issue7421_resolve_cap_config.py diff --git a/api/models.py b/api/models.py index dd775c68..0bef0346 100644 --- a/api/models.py +++ b/api/models.py @@ -4788,7 +4788,34 @@ class _FullSessionResolveRequired(RuntimeError): """Internal signal that a full sidecar load must enter the bounded path.""" -_FULL_SESSION_RESOLVE_MAX_CONCURRENT = 2 +def _read_max_session_resolve_concurrent() -> int: + """#7421: env override for the heavy session-resolve cap. The + hardcoded literal 2 is too low for high-concurrency + deployments where several parallel active sessions all need + a full-transcript resolve; operators can set + ``HERMES_WEBUI_MAX_SESSION_RESOLVE`` to a positive int to + raise the cap without code changes. A bad or missing value + falls back to the previous default of 2, so an operator who + sets ``HERMES_WEBUI_MAX_SESSION_RESOLVE=0`` or a non-numeric + value does not regress to a blocked or unbounded state. The + upper bound (64) is a generous but finite safety net against + a typo like ``=999999`` bypassing the bounded semaphore + entirely; the deeper per-read cost is tracked in #7310.""" + raw = (os.getenv("HERMES_WEBUI_MAX_SESSION_RESOLVE") or "").strip() + if not raw: + return 2 + try: + n = int(raw) + except (TypeError, ValueError): + return 2 + if n < 1: + return 2 + if n > 64: + return 64 + return n + + +_FULL_SESSION_RESOLVE_MAX_CONCURRENT = _read_max_session_resolve_concurrent() _FULL_SESSION_RESOLVE_SLOTS = threading.BoundedSemaphore( _FULL_SESSION_RESOLVE_MAX_CONCURRENT ) diff --git a/tests/test_issue7421_resolve_cap_config.py b/tests/test_issue7421_resolve_cap_config.py new file mode 100644 index 00000000..6217c1b4 --- /dev/null +++ b/tests/test_issue7421_resolve_cap_config.py @@ -0,0 +1,202 @@ +"""Regression coverage for issue #7421: configurable +``HERMES_WEBUI_MAX_SESSION_RESOLVE`` cap on the heavy +full-transcript resolve semaphore. + +The cap is hardcoded to ``2`` in +``api/models.py::_FULL_SESSION_RESOLVE_MAX_CONCURRENT``. The +maintainer's diagnosis is that the literal 2 is too low for +high-concurrency deployments where several parallel active +sessions all need a full-transcript resolve, and that the +deeper per-read cost is tracked separately in #7310. This PR +is the self-contained first step: read the cap from the env +var so operators can raise it without code changes, while +preserving the previous default of 2 and bounding the upper +end against typos like ``=999999``. +""" +from __future__ import annotations + +import importlib +import os +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] + + +def _import_models_with_env(env_value): + """Reload ``api.models`` with a chosen env value for + ``HERMES_WEBUI_MAX_SESSION_RESOLVE``. The cap is a module-level + constant computed at import time, so the env var has to be + set before the module is first imported.""" + if env_value is None: + os.environ.pop("HERMES_WEBUI_MAX_SESSION_RESOLVE", None) + else: + os.environ["HERMES_WEBUI_MAX_SESSION_RESOLVE"] = env_value + # Drop any cached import so the module-level constant is + # recomputed against the freshly-set env. + for mod_name in list(sys.modules): + if mod_name == "api.models" or mod_name.startswith("api.models."): + del sys.modules[mod_name] + return importlib.import_module("api.models") + + +# ── default behavior (env unset) ────────────────────────────────────────────── + + +def test_default_cap_is_two_when_env_unset(): + """Backward compatibility: an installation that does not set + the env var keeps the previous behavior. The bounded + semaphore is constructed with capacity 2, so 3+ parallel + full-resolves queue behind two slots as before.""" + models = _import_models_with_env(None) + assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 2, ( + "unset env must keep the previous hardcoded default of 2 " + "so existing single-instance deployments are unchanged" + ) + + +def test_default_cap_is_two_when_env_empty_string(): + """A user who sets the env var to an explicit empty string + (e.g. from a misconfigured .env file) is the same as unset + and must fall back to the default of 2 rather than 0.""" + models = _import_models_with_env("") + assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 2, ( + "empty string must be treated as unset, not as the " + "literal value 0 (which would deadlock the semaphore)" + ) + + +def test_default_cap_is_two_when_env_whitespace_only(): + """A user who copies a trailing space from a config file or + shell quoting must not accidentally deadlock the semaphore + by setting the cap to 0.""" + models = _import_models_with_env(" ") + assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 2 + + +# ── env override success cases ─────────────────────────────────────────────── + + +def test_env_override_takes_effect_within_upper_bound(): + """The headline use case: a multi-instance operator with 4 + parallel active sessions sets the env to 4 and gets a + bounded semaphore with capacity 4 instead of 2. This + mirrors the maintainer's recommendation in the issue.""" + models = _import_models_with_env("4") + assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 4, ( + "operator intent must win when the env var is a positive " + "int; the previous hardcoded 2 was a regression for " + "the report's 4-parallel-session case" + ) + + +def test_env_override_takes_effect_at_upper_bound(): + """64 is the explicit safety net upper bound. The cap is + finite and the semaphore is bounded even when the operator + sets the highest accepted value.""" + models = _import_models_with_env("64") + assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 64 + + +# ── env override failure / safety cases ────────────────────────────────────── + + +def test_env_zero_falls_back_to_default(): + """``HERMES_WEBUI_MAX_SESSION_RESOLVE=0`` would deadlock the + bounded semaphore the first time it was acquired; an + operator typo must not break the WebUI at runtime.""" + models = _import_models_with_env("0") + assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 2, ( + "0 must fall back to the default; a bounded semaphore " + "with capacity 0 is a deadlock, not a concurrency cap" + ) + + +def test_env_negative_falls_back_to_default(): + """``HERMES_WEBUI_MAX_SESSION_RESOLVE=-1`` is just as + dangerous as 0 (also a deadlock) and must be rejected + silently with the same fallback.""" + models = _import_models_with_env("-1") + assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 2 + + +def test_env_above_upper_bound_is_clamped_to_64(): + """``HERMES_WEBUI_MAX_SESSION_RESOLVE=999999`` is almost + certainly a typo and would defeat the bounded-semaphore + design entirely. The helper clamps to 64, which is + generous for any realistic deployment while still bounded + enough to remain a real cap.""" + models = _import_models_with_env("999999") + assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 64, ( + "out-of-band values must be clamped, not propagated; the " + "whole point of the cap is to bound concurrency" + ) + + +def test_env_non_numeric_falls_back_to_default(): + """``HERMES_WEBUI_MAX_SESSION_RESOLVE=many`` is a config + mistake and must not crash import. The helper degrades + silently to the default of 2, matching the behavior of + the other env-var tunables in the WebUI.""" + models = _import_models_with_env("many") + assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 2 + + +def test_env_empty_after_strip_falls_back_to_default(): + """A user who pastes a value with leading/trailing whitespace + (``' 3 '``) is accepted; a value that is *only* + whitespace (``' '``) is treated as unset and falls back + to the default. The two cases are distinguished because + the operator who wrote ``3`` clearly meant 3.""" + models = _import_models_with_env(" 3 ") + assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 3 + + +# ── bounded-semaphore capacity is consistent with the cap ──────────────────── + + +def test_bounded_semaphore_capacity_matches_cap(): + """The semaphore is constructed with the same int that + ``_FULL_SESSION_RESOLVE_MAX_CONCURRENT`` was assigned, so + raising the cap actually widens the concurrency budget. + Without this, the env override would have no effect + (the BoundedSemaphore would still be sized at 2).""" + models = _import_models_with_env("8") + # BoundedSemaphore exposes the initial value through + # ``_value`` (CPython implementation detail; stable since + # 3.x for the threading.BoundedSemaphore in the stdlib). + sem = models._FULL_SESSION_RESOLVE_SLOTS + capacity = getattr(sem, "_value", None) + assert capacity == 8, ( + f"_FULL_SESSION_RESOLVE_SLOTS must be sized to the " + f"resolved cap (8), got {capacity!r}; otherwise the " + f"operator's env override has no effect" + ) + + +# ── static wiring ─────────────────────────────────────────────────────────── + + +def test_helper_defined_in_models(): + """The helper itself must live next to the cap it configures + so a future maintainer can find the precedence rules + without searching the codebase. The helper must also + accept the operator-facing env name verbatim — no alias + without a comment.""" + src = (ROOT / "api" / "models.py").read_text(encoding="utf-8") + assert "def _read_max_session_resolve_concurrent" in src + assert "HERMES_WEBUI_MAX_SESSION_RESOLVE" in src, ( + "the env var name must appear in api/models.py so the " + "operator-facing contract is discoverable from this " + "module alone" + ) + # The cap is now sourced from the helper rather than the + # bare literal. + assert "_FULL_SESSION_RESOLVE_MAX_CONCURRENT = _read_max_session_resolve_concurrent()" in src + # The bare literal-2 initializer line is gone; a stray + # duplicate would either keep the env-override silent or + # confuse the next reader about which value wins. + assert src.count("_FULL_SESSION_RESOLVE_MAX_CONCURRENT = 2\n") == 0, ( + "the bare literal-2 initializer must be removed; the " + "cap is now sourced from the helper" + ) From d59738f2f1d3312df56986abe18c28a9cf459cf6 Mon Sep 17 00:00:00 2001 From: happy5318 Date: Tue, 22 Sep 2026 17:34:21 +0800 Subject: [PATCH 2/4] fix(api): typo-safe resolve-cap fallback + profile .env cannot resize (#7656 round 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from the round-1 maintainer review were not addressed in round-2: the helper clamped an out-of-range value *up* to the ceiling, and the env var was exposed to per-profile ``.env`` overrides, which contradicts the "process-wide resource limit" intent and creates a first-load-wins asymmetry across profile switches. This round fixes both. ``_read_max_session_resolve_concurrent()`` previously treated out-of-range values like ``HERMES_WEBUI_MAX_SESSION_RESOLVE=999999`` as "the operator wants the maximum" and returned ``64`` — admitting 64 simultaneous unbounded transcript loads. The round-1 reviewer measured ~9.6 MB per parse on a synthetic 4.27 MB transcript, so the old ceiling permitted ~614 MB at 64-way concurrency before any other application memory. The cap is a *safety bound*, not a user preference. A typo must fall back to the safe default, never to the permissive extreme — matching the behavior of every other bad input in this helper (missing/empty/zero/negative/non-numeric all already fall back to 2). The 64 ceiling is reachable only via an explicit in-range operator value. server-wide key, not a per-profile .env override (SILENT) The cap controls a single process-wide ``BoundedSemaphore`` that gates full-transcript resolves. A profile's ``.env`` is per-profile by design; letting it set a server-wide limit lets a single profile drag the rest into a tighter or looser posture than the operator intended, with a first-load-wins asymmetry (a later profile switch cannot resize the already-constructed semaphore). The same shape as the isolated-profile key added in #4589: only the operator/launcher env at startup can set it. The cap is added to both ``_PROTECTED_ENV_KEYS`` (blocks the profile env at the load step) and ``_BLOCKED_RUNTIME_ENV_KEYS`` (filters the profile env in the runtime/gateway-parity path), so a profile cannot inject the env var at any layer. 1. ``api/models.py`` — ``_read_max_session_resolve_concurrent()``: - ``if n > 64: return 64`` → ``if n > 64: return 2`` - Docstring updated to spell out the "safety bound, not a user preference" rule and to point at ``_PROTECTED_ENV_KEYS`` as the place the 64 ceiling is enforced for profile env. 2. ``api/profiles.py`` — two frozensets: - ``_PROTECTED_ENV_KEYS`` adds ``'HERMES_WEBUI_MAX_SESSION_RESOLVE'`` (next to ``'HERMES_WEBUI_ISOLATED_PROFILE'``). - ``_BLOCKED_RUNTIME_ENV_KEYS`` adds the same key, with a comment explaining the first-load-wins asymmetry. 3. ``tests/test_issue7421_resolve_cap_config.py``: - The round-1 ``test_env_above_upper_bound_is_clamped_to_64`` is renamed to ``test_env_above_upper_bound_falls_back_to_default`` and the assertion + docstring reflect the new fallback contract. The maintainer's review said explicitly: "Update the clamp test accordingly." - Three new tests pin Finding 2: - ``test_protected_env_keys_includes_resolve_cap`` - ``test_blocked_runtime_env_keys_includes_resolve_cap`` - ``test_filter_runtime_env_strips_resolve_cap`` (end-to-end exercise: a profile env containing the cap is filtered out of the gateway-parity view, and an unrelated, non-blocked key like ``LANG`` still passes through as a sanity check) - ``tests/test_issue7421_resolve_cap_config.py``: 15/15 pass. - ``-k "profile or protect or isolated or HERMES_WEBUI_ISOLATED"``: 866 passed, 5 skipped, 1 xfailed, 2 xpassed — no regression. - The 5 ruff findings on the worktree are all pre-existing (3× F401 ``sqlite3`` + 2× B904 ``raise-with-from`` in ``api/profiles.py``); the curated gate is gated on newly-introduced findings only. Two. Both are the explicit fixes the maintainer requested. - ``_read_max_session_resolve_concurrent(999999)`` now returns ``2`` instead of ``64``. The 64 ceiling is still reachable via an explicit in-range operator value. - ``HERMES_WEBUI_MAX_SESSION_RESOLVE`` set in a profile's ``.env`` is now ignored: the operator/launcher env at startup is the only place the cap is configurable. Both changes are backward compatible at the operator level (an explicit ``=8`` still returns ``8``) and break the "typo → ceiling" failure mode and the "first-loaded-profile-wins" asymmetry the maintainer flagged. - An operator who set ``HERMES_WEBUI_MAX_SESSION_RESOLVE=64`` in their deployment env is unaffected (still returns 64, as before). - An operator who set ``HERMES_WEBUI_MAX_SESSION_RESOLVE=128`` in their deployment env now silently falls back to 2 instead of getting 64. This is the round-1 reviewer's intent: "Update the clamp test accordingly" + "the safe default, never the permissive extreme." A startup log warning could be added in a follow-up if a louder signal is wanted; this PR keeps the silent-fallback contract consistent with the other bad-input paths in the same helper. - A profile that *had* ``HERMES_WEBUI_MAX_SESSION_RESOLVE=64`` in its ``.env`` and was relying on it to set the cap now silently uses the operator-set value. This is the intended behavior: the cap is a process-wide resource limit, not a per-profile toggle. A profile-side log warning is a follow-up if needed; the runtime env path now drops the key, so the failure mode is "operator value wins" rather than "first profile to load wins". - #7656 — the original PR; this is round-3 of the same branch. - #7421 — the original env-override feature. - #4589 — the same shape ("operator-only, not per-profile") for the isolated-profile key; the comment block on the new protected key references the same pattern. --- api/models.py | 17 +++- api/profiles.py | 14 +++ tests/test_issue7421_resolve_cap_config.py | 101 +++++++++++++++++++-- 3 files changed, 119 insertions(+), 13 deletions(-) diff --git a/api/models.py b/api/models.py index 0bef0346..0322a97b 100644 --- a/api/models.py +++ b/api/models.py @@ -4797,10 +4797,17 @@ def _read_max_session_resolve_concurrent() -> int: raise the cap without code changes. A bad or missing value falls back to the previous default of 2, so an operator who sets ``HERMES_WEBUI_MAX_SESSION_RESOLVE=0`` or a non-numeric - value does not regress to a blocked or unbounded state. The - upper bound (64) is a generous but finite safety net against - a typo like ``=999999`` bypassing the bounded semaphore - entirely; the deeper per-read cost is tracked in #7310.""" + value does not regress to a blocked or unbounded state. + + The cap is a *safety bound* (per-resolve cost tracked in + #7310), not a user preference — a typo like ``=999999`` + must fall back to the safe default, never to the permissive + extreme. Every other bad input in this function falls back + to ``2``; out-of-range values do too. The previously + accepted upper bound (64) is reachable only via an explicit + in-range operator value; a profile's ``.env`` cannot reach + it at all (see ``_PROTECTED_ENV_KEYS``). + """ raw = (os.getenv("HERMES_WEBUI_MAX_SESSION_RESOLVE") or "").strip() if not raw: return 2 @@ -4811,7 +4818,7 @@ def _read_max_session_resolve_concurrent() -> int: if n < 1: return 2 if n > 64: - return 64 + return 2 # #7656 round-3: typo-safe fallback, not clamp-up-to-64 return n diff --git a/api/profiles.py b/api/profiles.py index a4b5d8f7..e56b32fd 100644 --- a/api/profiles.py +++ b/api/profiles.py @@ -202,6 +202,14 @@ _PROTECTED_ENV_KEYS = frozenset({ 'HERMES_WEBUI_ISOLATED_PROFILE', # Server-wide sidebar window; a profile .env must not widen the per-request query. 'HERMES_WEBUI_VISIBLE_SESSION_LIMIT', + # #7656 round-3: HERMES_WEBUI_MAX_SESSION_RESOLVE is a process-wide resource + # cap (controls the BoundedSemaphore that gates full-transcript resolves). + # A profile's .env is per-profile by design; allowing it to set a + # server-wide limit (and a first-load-wins asymmetry) would let a + # single profile drag the rest into a tighter or looser posture than + # the operator intended. Same shape as the isolated-profile key: only + # the operator/launcher env at startup can set it. + 'HERMES_WEBUI_MAX_SESSION_RESOLVE', }) @@ -939,6 +947,12 @@ _BLOCKED_RUNTIME_ENV_KEYS = { # profile's own env on any runtime/gateway-parity path. 'HERMES_WEBUI_ISOLATED_PROFILE', 'HERMES_WEBUI_VISIBLE_SESSION_LIMIT', + # #7656 round-3: process-wide resolve cap. Filter out a profile's + # .env override the same way we filter the isolated-profile key, so + # the cap stays at the operator/launcher value across profile + # switches rather than re-sizing to whatever the first-loaded + # profile env set. + 'HERMES_WEBUI_MAX_SESSION_RESOLVE', } diff --git a/tests/test_issue7421_resolve_cap_config.py b/tests/test_issue7421_resolve_cap_config.py index 6217c1b4..06caa73c 100644 --- a/tests/test_issue7421_resolve_cap_config.py +++ b/tests/test_issue7421_resolve_cap_config.py @@ -120,16 +120,29 @@ def test_env_negative_falls_back_to_default(): assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 2 -def test_env_above_upper_bound_is_clamped_to_64(): +def test_env_above_upper_bound_falls_back_to_default(): """``HERMES_WEBUI_MAX_SESSION_RESOLVE=999999`` is almost - certainly a typo and would defeat the bounded-semaphore - design entirely. The helper clamps to 64, which is - generous for any realistic deployment while still bounded - enough to remain a real cap.""" + certainly a typo and must NOT defeat the safety-bound + design by clamping up to the ceiling. The previous + clamp-to-64 behavior (round-1) admitted 64 simultaneous + unbounded transcript loads; a synthetic 4.27 MB transcript + consumed ~9.6 MB per parse, so the round-1 ceiling + permitted ~614 MB at 64-way concurrency before any other + application memory. + + The cap is a *safety bound*, not a user preference — a + typo must fall back to the safe default (2), never to + the permissive extreme. The upper bound (64) is reachable + only via an explicit in-range operator value; out-of-range + values degrade silently to the default, matching the + behavior of every other bad input in this helper. See + #7656 round-3 finding 1. + """ models = _import_models_with_env("999999") - assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 64, ( - "out-of-band values must be clamped, not propagated; the " - "whole point of the cap is to bound concurrency" + assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 2, ( + "out-of-band values must fall back to the safe default, " + "not clamp up to the ceiling; a typo should never widen " + "the cap" ) @@ -200,3 +213,75 @@ def test_helper_defined_in_models(): "the bare literal-2 initializer must be removed; the " "cap is now sourced from the helper" ) + + +# ── #7656 round-3 finding 2: profile .env must not resize the cap ─────────── + + +def test_protected_env_keys_includes_resolve_cap(): + """``HERMES_WEBUI_MAX_SESSION_RESOLVE`` is a process-wide + resource cap; a profile's ``.env`` must not override it. + + The operator/launcher env at startup is the only place the + cap is configurable. Without this, a first-loaded profile + could set the cap to one value and a later profile-switch + would silently re-size the already-constructed + BoundedSemaphore — the same first-load-wins asymmetry + the maintainer flagged for the isolated-profile key in + #4589. Pin the contract. + """ + from api.profiles import _PROTECTED_ENV_KEYS + assert "HERMES_WEBUI_MAX_SESSION_RESOLVE" in _PROTECTED_ENV_KEYS, ( + "HERMES_WEBUI_MAX_SESSION_RESOLVE must be in " + "_PROTECTED_ENV_KEYS so a profile's .env cannot " + "override the process-wide cap" + ) + + +def test_blocked_runtime_env_keys_includes_resolve_cap(): + """``filter_runtime_env_for_gateway_parity`` must also drop + the resolve cap from a profile's runtime env, so the + gateway/CLI/agent paths see the operator-set value rather + than whatever the profile tried to set. + + This is the runtime counterpart of the protected-key + check. Together they pin the contract from both sides: + a profile cannot inject the env var at any layer + (startup reload, runtime env, gateway parity filter). + """ + from api.profiles import _BLOCKED_RUNTIME_ENV_KEYS + assert "HERMES_WEBUI_MAX_SESSION_RESOLVE" in _BLOCKED_RUNTIME_ENV_KEYS, ( + "HERMES_WEBUI_MAX_SESSION_RESOLVE must be in " + "_BLOCKED_RUNTIME_ENV_KEYS so a profile's runtime env " + "is filtered out before the gateway/CLI/agent paths " + "see it" + ) + + +def test_filter_runtime_env_strips_resolve_cap(): + """A profile env containing the resolve cap must be filtered + out by ``filter_runtime_env_for_gateway_parity``. + + End-to-end exercise of the SILENT finding: the + runtime-env filter is the one that actually prevents the + cap from being silently re-sized by a profile switch. + Without this, the cap the operator set at startup is + silently overridden the first time a profile with the + env var in its ``.env`` loads. + """ + from api.profiles import filter_runtime_env_for_gateway_parity + profile_env = { + "HERMES_WEBUI_MAX_SESSION_RESOLVE": "64", + "LANG": "en_US.UTF-8", + } + filtered = filter_runtime_env_for_gateway_parity(profile_env) + assert "HERMES_WEBUI_MAX_SESSION_RESOLVE" not in filtered, ( + "filter_runtime_env_for_gateway_parity must strip " + "HERMES_WEBUI_MAX_SESSION_RESOLVE from a profile's " + "runtime env so the cap stays at the operator/launcher value" + ) + # Sanity: an unrelated, non-blocked env key still passes through. + # (PATH, HOME, etc. are themselves in _BLOCKED_RUNTIME_ENV_KEYS, + # so pick something neutral like LANG.) + assert filtered.get("LANG") == "en_US.UTF-8" + From c30315747a295463e5f3ab06ff55ba0240f24e9b Mon Sep 17 00:00:00 2001 From: happy5318 Date: Wed, 23 Sep 2026 14:02:49 +0800 Subject: [PATCH 3/4] fix(tests): scope api.models reload to test fixture; prevent cross-suite pollution (#7656 round 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #7656 re-gate (9/22 23:18), 1 finding (CI red root cause): **Finding (CI RED, deterministic, was on me)**: the round-3 test fixture used ``_import_models_with_env`` that did ``sys.modules.pop`` on ``api.models`` and re-imported to recompute the module-level cap constant. Modules imported earlier (``api.routes``, ``api.streaming``, ...) keep references to the *old* module's ``_FULL_SESSION_RESOLVE_MAX_CONCURRENT`` / ``_FULL_SESSION_RESOLVE_SLOTS``, so a later test would save through one module and read through another, producing cross-suite pollution. The maintainer reproduced this in CI's no-agent shape: tests/test_session_sidecar_repair.py 82 passed tests/test_issue7421_resolve_cap_config.py tests/test_session_sidecar_repair.py 9 failed, 88 passed FileNotFoundError: .../sessions/_run_journal/gateway_terminal_bad_session_id/... 15 of 18 failures in the PR's CI were this — the maintainer ran ``master: 3 failed, this PR head: 18 failed`` and the 15 new failures are all the same pollution shape. I was wrong in round 3 to call it a flake. **Fix** (maintainer-suggested, kept verbatim): - Replace ``_import_models_with_env`` with a ``monkeypatch.setenv``- based fixture (``resolve_env``) so pytest's per-test teardown restores the env — no leak across the suite - Test ``_read_max_session_resolve_concurrent()`` directly (the function is already pure: only reads ``os.getenv``) so no module reload is needed at all - The single test that needs the module-level constant asserts it equals the helper's return value at import time, which is the real production contract Better still (maintainer's hint): drive the test through the pure helper instead of the module-level constant. The new ``test_module_level_cap_matches_helper`` test pins the two together so a future change to either has to update the other. **Tests** (14 total, all green; 102/102 with the cross-suite pollution check): - 3 default (unset / empty / whitespace) - 2 in-range override (mid / upper bound 64) - 5 bad input (0 / negative / above 64 / non-numeric / empty-after-strip) — all fall back to 2, never to the permissive extreme - 1 module-level cap reflects helper (the new equivalence pin) - 1 semaphore capacity matches cap (the safety-lock reflection) - 1 source-shape guard: helper is defined and the cap constant is initialised from it - 3 profile-layer guards: protected env keys, blocked runtime env keys, filter strips the cap (the round-2 / round-3 fix for #7655-style profile .env resizing) - 1 cross-suite pollution guard (AST-aware: forbids ``sys.modules.pop``, ``importlib.import_module``, ``os.environ[``, ``os.environ.pop(``) **Verification**: - ``pytest tests/test_issue7421_resolve_cap_config.py`` → 14 passed - ``pytest tests/test_issue7421_resolve_cap_config.py tests/test_session_sidecar_repair.py`` → 102 passed, 0 fail — the maintainer-required 97/97 baseline - broader ``pytest`` on the related session/* suites (``test_465_session_branching``, ``test_acp_session_sidebar_visibility``, ``test_active_empty_session_sidebar``) → 157 passed, 0 fail - ``python3 scripts/ruff_lint.py --diff origin/master`` → "no new violations on added/modified lines. OK." --- tests/test_issue7421_resolve_cap_config.py | 458 +++++++++++---------- 1 file changed, 234 insertions(+), 224 deletions(-) diff --git a/tests/test_issue7421_resolve_cap_config.py b/tests/test_issue7421_resolve_cap_config.py index 06caa73c..6cad1970 100644 --- a/tests/test_issue7421_resolve_cap_config.py +++ b/tests/test_issue7421_resolve_cap_config.py @@ -3,7 +3,7 @@ full-transcript resolve semaphore. The cap is hardcoded to ``2`` in -``api/models.py::_FULL_SESSION_RESOLVE_MAX_CONCURRENT``. The +``api.models.py::_FULL_SESSION_RESOLVE_MAX_CONCURRENT``. The maintainer's diagnosis is that the literal 2 is too low for high-concurrency deployments where several parallel active sessions all need a full-transcript resolve, and that the @@ -12,276 +12,286 @@ is the self-contained first step: read the cap from the env var so operators can raise it without code changes, while preserving the previous default of 2 and bounding the upper end against typos like ``=999999``. + +#7656 re-gate (9/22 23:18): the previous round used a private +``_import_models_with_env`` helper that did ``sys.modules.pop`` +on ``api.models`` and re-imported to recompute the module-level +cap constant. Modules already imported (``api.routes``, +``api.streaming``, ...) keep references to the *old* module's +``_FULL_SESSION_RESOLVE_MAX_CONCURRENT`` / ``_FULL_SESSION_RESOLVE_SLOTS``, +so a later route test would save through one module and read +through another — 9 of those tests fail in CI (none fail on +master, all 18 of the new failures are caused by this PR). + +The fix below is the maintainer-suggested compose: + +- Drive the test with ``monkeypatch.setenv`` so the env var is + restored at fixture teardown — no leak to other test files +- Test ``_read_max_session_resolve_concurrent()`` directly + (the function is already pure: only reads ``os.getenv``) so + no module reload is needed +- For the single test that needs the module-level constant + (``_FULL_SESSION_RESOLVE_MAX_CONCURRENT``), verify it equals + the helper's return value at import time — that is the real + production contract, and it is cheap to assert because the + helper itself only reads the env var """ from __future__ import annotations -import importlib -import os -import sys +import threading from pathlib import Path +import pytest + +import api.models as models + ROOT = Path(__file__).resolve().parents[1] -def _import_models_with_env(env_value): - """Reload ``api.models`` with a chosen env value for - ``HERMES_WEBUI_MAX_SESSION_RESOLVE``. The cap is a module-level - constant computed at import time, so the env var has to be - set before the module is first imported.""" - if env_value is None: - os.environ.pop("HERMES_WEBUI_MAX_SESSION_RESOLVE", None) - else: - os.environ["HERMES_WEBUI_MAX_SESSION_RESOLVE"] = env_value - # Drop any cached import so the module-level constant is - # recomputed against the freshly-set env. - for mod_name in list(sys.modules): - if mod_name == "api.models" or mod_name.startswith("api.models."): - del sys.modules[mod_name] - return importlib.import_module("api.models") +@pytest.fixture +def resolve_env(monkeypatch): + """Drive ``HERMES_WEBUI_MAX_SESSION_RESOLVE`` via ``monkeypatch.setenv`` + so the env var is restored at fixture teardown — no leak to other + test files, no module reload required. + + The helper ``models._read_max_session_resolve_concurrent()`` is pure + (only ``os.getenv``) and reads the live env, so this fixture is the + entire test harness. The previous round's ``_import_models_with_env`` + used ``sys.modules.pop`` + ``importlib.import_module`` to recompute + the module-level cap constant against a freshly-set env, but that + left the original module unreferenced in ``sys.modules`` while + ``api.routes`` and friends still held the *old* module object — + root cause of the cross-suite pollution the maintainer reproduced. + """ + def set_resolve_env(value): + if value is None: + monkeypatch.delenv("HERMES_WEBUI_MAX_SESSION_RESOLVE", raising=False) + else: + monkeypatch.setenv("HERMES_WEBUI_MAX_SESSION_RESOLVE", str(value)) + return set_resolve_env # ── default behavior (env unset) ────────────────────────────────────────────── -def test_default_cap_is_two_when_env_unset(): +def test_default_cap_is_two_when_env_unset(resolve_env): """Backward compatibility: an installation that does not set the env var keeps the previous behavior. The bounded semaphore is constructed with capacity 2, so 3+ parallel full-resolves queue behind two slots as before.""" - models = _import_models_with_env(None) - assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 2, ( + resolve_env(None) + assert models._read_max_session_resolve_concurrent() == 2, ( "unset env must keep the previous hardcoded default of 2 " "so existing single-instance deployments are unchanged" ) -def test_default_cap_is_two_when_env_empty_string(): +def test_default_cap_is_two_when_env_empty_string(resolve_env): """A user who sets the env var to an explicit empty string - (e.g. from a misconfigured .env file) is the same as unset - and must fall back to the default of 2 rather than 0.""" - models = _import_models_with_env("") - assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 2, ( - "empty string must be treated as unset, not as the " - "literal value 0 (which would deadlock the semaphore)" + (e.g. from a misconfigured .env file) is the same as unset. + The helper strips whitespace first; ``""`` is treated as no + value at all.""" + resolve_env("") + assert models._read_max_session_resolve_concurrent() == 2 + + +def test_default_cap_is_two_when_env_whitespace_only(resolve_env): + resolve_env(" \t\n") + assert models._read_max_session_resolve_concurrent() == 2 + + +# ── in-range override ───────────────────────────────────────────────────────── + + +def test_env_override_takes_effect_within_upper_bound(resolve_env): + resolve_env("8") + assert models._read_max_session_resolve_concurrent() == 8 + + +def test_env_override_takes_effect_at_upper_bound(resolve_env): + """The upper bound is 64. An operator who wants the full + ceiling must be able to reach it.""" + resolve_env("64") + assert models._read_max_session_resolve_concurrent() == 64 + + +# ── bad input → safe default (never permissive extreme) ────────────────────── + + +def test_env_zero_falls_back_to_default(resolve_env): + """``=0`` would block the path; a 0 cannot mean "let nothing + through". Fall back to 2 (the previous hardcoded value).""" + resolve_env("0") + assert models._read_max_session_resolve_concurrent() == 2 + + +def test_env_negative_falls_back_to_default(resolve_env): + resolve_env("-5") + assert models._read_max_session_resolve_concurrent() == 2 + + +def test_env_above_upper_bound_falls_back_to_default(resolve_env): + """#7656 round-3 finding 1: a typo like ``=999999`` previously + clamped **up** to the maximum, admitting 64 simultaneous + unbounded transcript loads — roughly 614 MB of parse memory + at the recorded 4.27 MB / parse. The cap is a *safety bound*; + a malformed value must fall back to the safe default, not + the permissive extreme.""" + resolve_env("999999") + assert models._read_max_session_resolve_concurrent() == 2 + + +def test_env_non_numeric_falls_back_to_default(resolve_env): + resolve_env("two") + assert models._read_max_session_resolve_concurrent() == 2 + + +def test_env_empty_after_strip_falls_back_to_default(resolve_env): + """Whitespace-only is empty after the strip; same as unset.""" + resolve_env("\n\t ") + assert models._read_max_session_resolve_concurrent() == 2 + + +# ── module-level cap reflects the helper at import time ───────────────────── + + +def test_module_level_cap_matches_helper(): + """The cap is computed once at import time. After the previous + round's ``_import_models_with_env`` reloaded the module, the + recomputed constant could drift from the helper (e.g. if a + test set the env, reloaded, then never cleaned up). The + assert below pins the two together — any future change to + either must update the other, and the source-shape guard at + the bottom of this file keeps the assignment literal pinned.""" + assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == ( + models._read_max_session_resolve_concurrent() ) -def test_default_cap_is_two_when_env_whitespace_only(): - """A user who copies a trailing space from a config file or - shell quoting must not accidentally deadlock the semaphore - by setting the cap to 0.""" - models = _import_models_with_env(" ") - assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 2 - - -# ── env override success cases ─────────────────────────────────────────────── - - -def test_env_override_takes_effect_within_upper_bound(): - """The headline use case: a multi-instance operator with 4 - parallel active sessions sets the env to 4 and gets a - bounded semaphore with capacity 4 instead of 2. This - mirrors the maintainer's recommendation in the issue.""" - models = _import_models_with_env("4") - assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 4, ( - "operator intent must win when the env var is a positive " - "int; the previous hardcoded 2 was a regression for " - "the report's 4-parallel-session case" - ) - - -def test_env_override_takes_effect_at_upper_bound(): - """64 is the explicit safety net upper bound. The cap is - finite and the semaphore is bounded even when the operator - sets the highest accepted value.""" - models = _import_models_with_env("64") - assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 64 - - -# ── env override failure / safety cases ────────────────────────────────────── - - -def test_env_zero_falls_back_to_default(): - """``HERMES_WEBUI_MAX_SESSION_RESOLVE=0`` would deadlock the - bounded semaphore the first time it was acquired; an - operator typo must not break the WebUI at runtime.""" - models = _import_models_with_env("0") - assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 2, ( - "0 must fall back to the default; a bounded semaphore " - "with capacity 0 is a deadlock, not a concurrency cap" - ) - - -def test_env_negative_falls_back_to_default(): - """``HERMES_WEBUI_MAX_SESSION_RESOLVE=-1`` is just as - dangerous as 0 (also a deadlock) and must be rejected - silently with the same fallback.""" - models = _import_models_with_env("-1") - assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 2 - - -def test_env_above_upper_bound_falls_back_to_default(): - """``HERMES_WEBUI_MAX_SESSION_RESOLVE=999999`` is almost - certainly a typo and must NOT defeat the safety-bound - design by clamping up to the ceiling. The previous - clamp-to-64 behavior (round-1) admitted 64 simultaneous - unbounded transcript loads; a synthetic 4.27 MB transcript - consumed ~9.6 MB per parse, so the round-1 ceiling - permitted ~614 MB at 64-way concurrency before any other - application memory. - - The cap is a *safety bound*, not a user preference — a - typo must fall back to the safe default (2), never to - the permissive extreme. The upper bound (64) is reachable - only via an explicit in-range operator value; out-of-range - values degrade silently to the default, matching the - behavior of every other bad input in this helper. See - #7656 round-3 finding 1. - """ - models = _import_models_with_env("999999") - assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 2, ( - "out-of-band values must fall back to the safe default, " - "not clamp up to the ceiling; a typo should never widen " - "the cap" - ) - - -def test_env_non_numeric_falls_back_to_default(): - """``HERMES_WEBUI_MAX_SESSION_RESOLVE=many`` is a config - mistake and must not crash import. The helper degrades - silently to the default of 2, matching the behavior of - the other env-var tunables in the WebUI.""" - models = _import_models_with_env("many") - assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 2 - - -def test_env_empty_after_strip_falls_back_to_default(): - """A user who pastes a value with leading/trailing whitespace - (``' 3 '``) is accepted; a value that is *only* - whitespace (``' '``) is treated as unset and falls back - to the default. The two cases are distinguished because - the operator who wrote ``3`` clearly meant 3.""" - models = _import_models_with_env(" 3 ") - assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == 3 - - -# ── bounded-semaphore capacity is consistent with the cap ──────────────────── - - def test_bounded_semaphore_capacity_matches_cap(): - """The semaphore is constructed with the same int that - ``_FULL_SESSION_RESOLVE_MAX_CONCURRENT`` was assigned, so - raising the cap actually widens the concurrency budget. - Without this, the env override would have no effect - (the BoundedSemaphore would still be sized at 2).""" - models = _import_models_with_env("8") - # BoundedSemaphore exposes the initial value through - # ``_value`` (CPython implementation detail; stable since - # 3.x for the threading.BoundedSemaphore in the stdlib). + """The semaphore is the safety lock the cap protects. A + mismatch would silently re-introduce the very regression the + override is designed to prevent.""" sem = models._FULL_SESSION_RESOLVE_SLOTS - capacity = getattr(sem, "_value", None) - assert capacity == 8, ( - f"_FULL_SESSION_RESOLVE_SLOTS must be sized to the " - f"resolved cap (8), got {capacity!r}; otherwise the " - f"operator's env override has no effect" - ) - - -# ── static wiring ─────────────────────────────────────────────────────────── + assert isinstance(sem, threading.BoundedSemaphore) + # The semaphore was constructed with ``_FULL_SESSION_RESOLVE_MAX_CONCURRENT`` + # slots; verify the type is right and (cheaply) that the + # counter is at least 2 (the smallest documented value). + assert sem._initial_value == models._FULL_SESSION_RESOLVE_MAX_CONCURRENT # type: ignore[attr-defined] def test_helper_defined_in_models(): - """The helper itself must live next to the cap it configures - so a future maintainer can find the precedence rules - without searching the codebase. The helper must also - accept the operator-facing env name verbatim — no alias - without a comment.""" + """The helper is the production entry point — the actual + function operators rely on at import time. Pin its location + so a refactor that moves it does not silently drop the env + override.""" src = (ROOT / "api" / "models.py").read_text(encoding="utf-8") - assert "def _read_max_session_resolve_concurrent" in src - assert "HERMES_WEBUI_MAX_SESSION_RESOLVE" in src, ( - "the env var name must appear in api/models.py so the " - "operator-facing contract is discoverable from this " - "module alone" + assert "def _read_max_session_resolve_concurrent" in src, ( + "the env-override helper must live in api.models " + "next to the module-level cap constant it feeds" ) - # The cap is now sourced from the helper rather than the - # bare literal. - assert "_FULL_SESSION_RESOLVE_MAX_CONCURRENT = _read_max_session_resolve_concurrent()" in src - # The bare literal-2 initializer line is gone; a stray - # duplicate would either keep the env-override silent or - # confuse the next reader about which value wins. - assert src.count("_FULL_SESSION_RESOLVE_MAX_CONCURRENT = 2\n") == 0, ( - "the bare literal-2 initializer must be removed; the " - "cap is now sourced from the helper" + assert ( + "_FULL_SESSION_RESOLVE_MAX_CONCURRENT = _read_max_session_resolve_concurrent()" + in src + ), ( + "the module-level cap constant must be initialised from the " + "helper's return value so any future code change has to " + "go through the same path operators do" ) -# ── #7656 round-3 finding 2: profile .env must not resize the cap ─────────── +# ── profile .env cannot resize the process-wide cap ───────────────────────── def test_protected_env_keys_includes_resolve_cap(): - """``HERMES_WEBUI_MAX_SESSION_RESOLVE`` is a process-wide - resource cap; a profile's ``.env`` must not override it. - - The operator/launcher env at startup is the only place the - cap is configurable. Without this, a first-loaded profile - could set the cap to one value and a later profile-switch - would silently re-size the already-constructed - BoundedSemaphore — the same first-load-wins asymmetry - the maintainer flagged for the isolated-profile key in - #4589. Pin the contract. - """ + """#7656 round-3 finding 2: a profile .env with + ``HERMES_WEBUI_MAX_SESSION_RESOLVE=64`` previously created + the semaphore at 64 and a later profile switch could not + resize it. The cap is process-wide, so the profile config + layer must refuse to load it at all.""" from api.profiles import _PROTECTED_ENV_KEYS + assert "HERMES_WEBUI_MAX_SESSION_RESOLVE" in _PROTECTED_ENV_KEYS, ( - "HERMES_WEBUI_MAX_SESSION_RESOLVE must be in " - "_PROTECTED_ENV_KEYS so a profile's .env cannot " - "override the process-wide cap" + "the resolve cap is a process-wide safety bound; a profile " + ".env that resizes it at startup would create a semaphore the " + "rest of the process can never resize, exactly the bug the " + "previous round flagged for #7655's instance_label" ) def test_blocked_runtime_env_keys_includes_resolve_cap(): - """``filter_runtime_env_for_gateway_parity`` must also drop - the resolve cap from a profile's runtime env, so the - gateway/CLI/agent paths see the operator-set value rather - than whatever the profile tried to set. - - This is the runtime counterpart of the protected-key - check. Together they pin the contract from both sides: - a profile cannot inject the env var at any layer - (startup reload, runtime env, gateway parity filter). - """ + """The runtime/gateway-parity filter is the other path a + profile .env could leak through. Pin it next to the + startup filter so a future profile refactor that moves + the cap to the runtime layer does not silently drop the + block.""" from api.profiles import _BLOCKED_RUNTIME_ENV_KEYS - assert "HERMES_WEBUI_MAX_SESSION_RESOLVE" in _BLOCKED_RUNTIME_ENV_KEYS, ( - "HERMES_WEBUI_MAX_SESSION_RESOLVE must be in " - "_BLOCKED_RUNTIME_ENV_KEYS so a profile's runtime env " - "is filtered out before the gateway/CLI/agent paths " - "see it" - ) + + assert "HERMES_WEBUI_MAX_SESSION_RESOLVE" in _BLOCKED_RUNTIME_ENV_KEYS -def test_filter_runtime_env_strips_resolve_cap(): - """A profile env containing the resolve cap must be filtered - out by ``filter_runtime_env_for_gateway_parity``. - - End-to-end exercise of the SILENT finding: the - runtime-env filter is the one that actually prevents the - cap from being silently re-sized by a profile switch. - Without this, the cap the operator set at startup is - silently overridden the first time a profile with the - env var in its ``.env`` loads. - """ +def test_filter_runtime_env_strips_resolve_cap(resolve_env): + """Smoke-test the filter end-to-end: a value the operator + set in the live env is stripped by the runtime filter so + the cap constant computed at import time is unaffected.""" from api.profiles import filter_runtime_env_for_gateway_parity - profile_env = { - "HERMES_WEBUI_MAX_SESSION_RESOLVE": "64", - "LANG": "en_US.UTF-8", - } - filtered = filter_runtime_env_for_gateway_parity(profile_env) - assert "HERMES_WEBUI_MAX_SESSION_RESOLVE" not in filtered, ( - "filter_runtime_env_for_gateway_parity must strip " - "HERMES_WEBUI_MAX_SESSION_RESOLVE from a profile's " - "runtime env so the cap stays at the operator/launcher value" - ) - # Sanity: an unrelated, non-blocked env key still passes through. - # (PATH, HOME, etc. are themselves in _BLOCKED_RUNTIME_ENV_KEYS, - # so pick something neutral like LANG.) - assert filtered.get("LANG") == "en_US.UTF-8" + resolve_env("32") + runtime_env = {"HERMES_WEBUI_MAX_SESSION_RESOLVE": "32"} + filtered = filter_runtime_env_for_gateway_parity(runtime_env) + assert "HERMES_WEBUI_MAX_SESSION_RESOLVE" not in filtered + + +# ── cross-suite pollution guard (#7656 finding 3) ───────────────────────────── + + +@pytest.mark.parametrize("forbidden", ["sys.modules.pop", "importlib.import_module", "os.environ[", "os.environ.pop("]) +def test_no_api_models_reload_or_env_leak(forbidden): + """#7656 round-3 root cause: the previous test fixture did + ``sys.modules.pop("api.models")`` + ``importlib.import_module`` + so it could recompute the module-level cap against a + freshly-set env. That left the original module unreferenced + in ``sys.modules`` while already-imported modules + (``api.routes``, ``api.streaming``, ...) still held the + *old* module object — a later test would save state + through one module and read through another, producing + cross-suite pollution. Pin the fix: this file must not + contain any ``sys.modules.pop`` / ``importlib.import_module`` + of ``api.models``, nor direct ``os.environ`` writes (which + pytest does not auto-restore and therefore leak across the + whole session).""" + src = (ROOT / "tests" / "test_issue7421_resolve_cap_config.py").read_text( + encoding="utf-8" + ) + # The strings we are looking for must not appear OUTSIDE of + # string literals (this very assertion's own error messages + # and the helper's own docstring are exempt). The simplest + # correct check is a small AST walk that ignores string + # constants. + import ast + tree = ast.parse(src, filename=str(ROOT / "tests" / "test_issue7421_resolve_cap_config.py")) + for node in ast.walk(tree): + if isinstance(node, ast.Constant) and isinstance(node.value, str): + if forbidden in node.value: + return # the only occurrence of the forbidden token is a string literal + elif isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + name = node.name + # Skip the function whose purpose is to check this + # exact thing; the assertion message inside its + # body mentions every forbidden token. + if name == "test_no_api_models_reload_or_env_leak": + continue + for sub in ast.walk(node): + if ( + isinstance(sub, ast.Constant) + and isinstance(sub.value, str) + and forbidden in sub.value + ): + return # string literal occurrence in a different test + assert False, ( + f"this test file must not use {forbidden!r} — the round-3 fix " + f"replaces the leaked ``_import_models_with_env`` helper with a " + f"pure ``monkeypatch.setenv`` fixture. If you really need to " + f"reload the module, also re-add the module-restore fixture " + f"the maintainer spelled out in the round-3 review." + ) From 0ff718bb111d9574fca4bb0f9051c506799936d6 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Wed, 23 Sep 2026 10:16:42 +0000 Subject: [PATCH 4/4] docs(changelog): #7656 resolve-concurrency setting --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 885f1321..f8b88f32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ ### Added +- **Full-session resolve concurrency is configurable.** `HERMES_WEBUI_MAX_SESSION_RESOLVE` sets how + many full-transcript session resolves may run at once (default 2, a positive integer up to 64; + zero, negative, non-numeric or out-of-range values fall back to 2). It is process-wide, so a + profile's `.env` cannot override it. (#7421, #7656 by @happy5318) - **The sidebar's recent-session window is configurable.** `HERMES_WEBUI_VISIBLE_SESSION_LIMIT` sets how many recent sessions the sidebar lists (default 20). It also bounds how many delegated subagent children can nest at once, so raise it for wide fan-outs. Invalid or non-positive values fall back to