fix(#3961): preserve mirrored live-model profile reads
This commit is contained in:
parent
846951304d
commit
321090efcc
4 changed files with 59 additions and 14 deletions
|
|
@ -6415,7 +6415,7 @@ def get_available_models(*, prefer_cache: bool = False) -> dict:
|
|||
try:
|
||||
from api.profiles import (
|
||||
get_active_profile_name as _gapn,
|
||||
profile_env_for_active_request as _prof_env_request,
|
||||
profile_env_for_active_request_readonly as _prof_env_request,
|
||||
profile_scope_for_detached_worker as _prof_scope_worker,
|
||||
)
|
||||
_active_profile_name = (_gapn() or "").strip()
|
||||
|
|
|
|||
|
|
@ -901,7 +901,7 @@ def profile_env_for_background_worker(
|
|||
|
||||
|
||||
@contextmanager
|
||||
def profile_env_for_active_request(
|
||||
def profile_env_for_active_request_readonly(
|
||||
purpose: str = "provider/model read",
|
||||
logger_override: Optional[logging.Logger] = None,
|
||||
):
|
||||
|
|
@ -964,6 +964,27 @@ def profile_env_for_active_request(
|
|||
_clear_thread_env()
|
||||
|
||||
|
||||
@contextmanager
|
||||
def profile_env_for_active_request(
|
||||
purpose: str = "active request",
|
||||
logger_override: Optional[logging.Logger] = None,
|
||||
):
|
||||
"""Apply the active per-request profile through the legacy mirrored path.
|
||||
|
||||
Some request-scoped readers still delegate into Hermes helpers that resolve
|
||||
credentials directly from process env or ``get_hermes_home()``. Those paths
|
||||
stay on the mirrored scope until they are fully audited.
|
||||
"""
|
||||
profile = (get_active_profile_name() or "").strip()
|
||||
if not profile or _is_root_profile(profile):
|
||||
yield
|
||||
return
|
||||
with profile_env_for_background_worker(
|
||||
profile, purpose, logger_override=logger_override
|
||||
):
|
||||
yield
|
||||
|
||||
|
||||
@contextmanager
|
||||
def profile_scope_for_detached_worker(
|
||||
profile_name,
|
||||
|
|
|
|||
|
|
@ -7773,8 +7773,8 @@ def handle_get(handler, parsed) -> bool:
|
|||
# profile's (#3957). Without this, get_auth_status() probes on a
|
||||
# non-default profile resolve the wrong/empty creds and can stall past
|
||||
# the 30s frontend timeout. No-op for the default profile.
|
||||
from api.profiles import profile_env_for_active_request
|
||||
with profile_env_for_active_request("/api/providers", logger_override=logger):
|
||||
from api.profiles import profile_env_for_active_request_readonly
|
||||
with profile_env_for_active_request_readonly("/api/providers", logger_override=logger):
|
||||
return j(handler, get_providers())
|
||||
|
||||
# ── Plugins/hooks visibility (read-only, no callback/source internals) ──
|
||||
|
|
@ -7790,8 +7790,8 @@ def handle_get(handler, parsed) -> bool:
|
|||
# read/write runs under the process-default profile, so a multi-profile
|
||||
# client would see (and seed) the default profile's pool instead of its
|
||||
# own (#4247/#4067 profile-isolation class).
|
||||
from api.profiles import profile_env_for_active_request
|
||||
with profile_env_for_active_request("/api/provider/quota", logger_override=logger):
|
||||
from api.profiles import profile_env_for_active_request_readonly
|
||||
with profile_env_for_active_request_readonly("/api/provider/quota", logger_override=logger):
|
||||
return j(handler, get_provider_quota(provider_id, refresh=refresh))
|
||||
|
||||
if parsed.path == "/api/provider/cost-history":
|
||||
|
|
|
|||
|
|
@ -19,7 +19,7 @@ The fix:
|
|||
- ``api.config._get_models_cache_path()`` returns a profile-keyed path
|
||||
(``models_cache.<profile>.json`` for named profiles; unchanged
|
||||
``models_cache.json`` for the default/root profile).
|
||||
- ``api.profiles.profile_env_for_active_request()`` applies the active
|
||||
- ``api.profiles.profile_env_for_active_request_readonly()`` applies the active
|
||||
per-request profile's env around the read; no-op for the default profile.
|
||||
"""
|
||||
|
||||
|
|
@ -113,7 +113,7 @@ def test_active_request_env_noop_for_default_profile(monkeypatch):
|
|||
monkeypatch.setattr(profiles, "get_active_profile_name", lambda: "default")
|
||||
monkeypatch.setattr(profiles, "_is_root_profile", lambda n: n in ("", "default"))
|
||||
monkeypatch.delenv("ISSUE_3957_PROBE", raising=False)
|
||||
with profiles.profile_env_for_active_request("test"):
|
||||
with profiles.profile_env_for_active_request_readonly("test"):
|
||||
# No env mutation, no HERMES_HOME change for the default profile.
|
||||
assert os.environ.get("ISSUE_3957_PROBE") is None
|
||||
assert os.environ.get("ISSUE_3957_PROBE") is None
|
||||
|
|
@ -135,7 +135,7 @@ def test_active_request_env_applies_named_profile_env(monkeypatch, tmp_path):
|
|||
try:
|
||||
assert profiles.get_active_profile_name() == "work"
|
||||
assert config._thread_local_env_value("ISSUE_3957_PROBE") == "from-process-env"
|
||||
with profiles.profile_env_for_active_request("test"):
|
||||
with profiles.profile_env_for_active_request_readonly("test"):
|
||||
assert config._thread_local_env_value("ISSUE_3957_PROBE") == "from-work-profile"
|
||||
assert os.environ.get("ISSUE_3957_PROBE") == "from-process-env"
|
||||
# Restored after the block exits.
|
||||
|
|
@ -160,7 +160,7 @@ def test_active_request_env_restores_on_exception(monkeypatch, tmp_path):
|
|||
try:
|
||||
with_raised = False
|
||||
try:
|
||||
with profiles.profile_env_for_active_request("test"):
|
||||
with profiles.profile_env_for_active_request_readonly("test"):
|
||||
assert config._thread_local_env_value("ISSUE_3957_PROBE") == "from-work-profile"
|
||||
assert os.environ.get("ISSUE_3957_PROBE") == "from-process-env"
|
||||
raise ValueError("boom")
|
||||
|
|
@ -201,7 +201,7 @@ def test_active_request_scope_prefers_profile_key_over_process_env_for_custom_pr
|
|||
"from-process-env",
|
||||
None,
|
||||
)
|
||||
with profiles.profile_env_for_active_request("test"):
|
||||
with profiles.profile_env_for_active_request_readonly("test"):
|
||||
assert config.resolve_custom_provider_connection("custom:team") == (
|
||||
"from-work-profile",
|
||||
None,
|
||||
|
|
@ -225,18 +225,41 @@ def test_active_request_scope_sets_context_local_hermes_home(monkeypatch, tmp_pa
|
|||
|
||||
profiles.set_request_profile("work")
|
||||
try:
|
||||
with profiles.profile_env_for_active_request("test"):
|
||||
with profiles.profile_env_for_active_request_readonly("test"):
|
||||
assert get_hermes_home() == work_home
|
||||
finally:
|
||||
profiles.clear_request_profile()
|
||||
|
||||
|
||||
def test_active_request_legacy_scope_still_mirrors_process_env(monkeypatch, tmp_path):
|
||||
"""Live-model request scope still mirrors env for agent-side readers."""
|
||||
base = tmp_path / ".hermes"
|
||||
(base / "profiles" / "work").mkdir(parents=True)
|
||||
(base / "profiles" / "work" / ".env").write_text(
|
||||
"ISSUE_3957_PROBE=from-work-profile\n", encoding="utf-8"
|
||||
)
|
||||
monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base)
|
||||
monkeypatch.setenv("ISSUE_3957_PROBE", "from-process-env")
|
||||
|
||||
profiles.set_request_profile("work")
|
||||
try:
|
||||
with profiles.profile_env_for_active_request("test"):
|
||||
assert os.environ.get("ISSUE_3957_PROBE") == "from-work-profile"
|
||||
assert os.environ.get("ISSUE_3957_PROBE") == "from-process-env"
|
||||
finally:
|
||||
profiles.clear_request_profile()
|
||||
|
||||
|
||||
def test_providers_and_models_routes_wrap_in_profile_env():
|
||||
"""The two read routes are profile-scoped for non-default profiles (#3957).
|
||||
|
||||
Structural guard: a future refactor that drops the wiring would silently
|
||||
reintroduce the bug, so pin it at the source level.
|
||||
- /api/providers wraps the synchronous read in profile_env_for_active_request.
|
||||
- /api/providers and /api/provider/quota wrap the synchronous read in
|
||||
profile_env_for_active_request_readonly.
|
||||
- /api/models/live stays on the mirrored profile_env_for_active_request
|
||||
path because provider_model_ids() still delegates into agent helpers
|
||||
that read process env / HERMES_HOME directly.
|
||||
- /api/models relies on get_available_models() scoping its detached
|
||||
rebuild worker via profile_scope_for_detached_worker (the request-thread
|
||||
wrapper cannot reach the worker thread — Codex CORE finding).
|
||||
|
|
@ -244,7 +267,8 @@ def test_providers_and_models_routes_wrap_in_profile_env():
|
|||
routes_src = Path(profiles.__file__).resolve().parent.joinpath("routes.py").read_text(
|
||||
encoding="utf-8"
|
||||
)
|
||||
assert "profile_env_for_active_request" in routes_src
|
||||
assert 'with profile_env_for_active_request("/api/models/live"' in routes_src
|
||||
assert "profile_env_for_active_request_readonly" in routes_src
|
||||
config_src = Path(config.__file__).resolve().read_text(encoding="utf-8")
|
||||
assert "profile_scope_for_detached_worker" in config_src
|
||||
assert "_get_models_cache_path" in config_src
|
||||
|
|
|
|||
Loading…
Reference in a new issue