fix(profiles): scope the detached models-rebuild worker to the active profile (#3957)

Codex + Opus both independently caught a CORE gap in the first cut: the
bounded /api/models rebuild runs on a detached 'models-catalog-rebuild' daemon
thread that inherits neither the request-profile thread-local (#798) nor
os.environ. So on a non-default profile the worker probed the DEFAULT profile's
credentials and, when the 4s budget was exceeded, published the rebuilt catalog
to the DEFAULT profile's disk cache (cross-contamination) — exactly the slow
path a non-default cold rebuild takes.

Fix:
- profile_scope_for_detached_worker(profile_name): sets the request-profile TLS
  AND applies the profile .env on the worker thread, restoring both on exit
  (no-op for default). Distinct from profile_env_for_active_request (which reads
  the current thread's TLS and must not clear it).
- get_available_models() captures the active profile on the request thread and
  wraps the rebuild worker body (probe + over-budget publish + disk save +
  fingerprint) in that scope; the legacy synchronous rebuild applies the profile
  env on the foreground. /api/models route no longer wraps (the work moved into
  get_available_models so ALL callers — chat/start, resolution — are fixed).
- 2 new regression tests incl. the worker-thread before/inside/after assertion.

Empirically verified: a fresh worker thread resolved models_cache.json/default
WITHOUT the scope (the bug) and models_cache.work.json + the work .env + the
work auth.json WITH it.

Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>

api/config.py

@@ -5250,10 +5250,40 @@ def get_available_models(*, prefer_cache: bool = False) -> dict:
         with _cache_build_cv:
             _cache_build_in_progress = True
 
+        # Capture the active per-request profile (#3957). The live provider
+        # probe inside the rebuild resolves credentials from os.environ /
+        # HERMES_HOME and the disk-cache path/fingerprint from the profile TLS;
+        # the detached worker thread below inherits NEITHER, so it must be
+        # captured here (on the request thread, where the TLS is valid) and
+        # re-bound on the worker. Empty / default for single-profile installs.
+        from contextlib import nullcontext as _nullcontext
+
+        _active_profile_name = ""
+        _prof_env_request = None
+        _prof_scope_worker = None
+        try:
+            from api.profiles import (
+                get_active_profile_name as _gapn,
+                profile_env_for_active_request as _prof_env_request,
+                profile_scope_for_detached_worker as _prof_scope_worker,
+            )
+            _active_profile_name = (_gapn() or "").strip()
+        except Exception:
+            _prof_env_request = None
+            _prof_scope_worker = None
+
         # Legacy synchronous (unbounded) rebuild — opt-in via budget<=0.
         if _LIVE_REBUILD_BUDGET_SECONDS <= 0:
             try:
-                result = _invoke_models_rebuild(_build_available_models_uncached)
+                # Foreground thread already carries the request-profile TLS;
+                # apply the profile env (no-op for default) for the live probe.
+                _sync_scope = (
+                    _prof_env_request("models rebuild (sync)")
+                    if _prof_env_request is not None
+                    else _nullcontext()
+                )
+                with _sync_scope:
+                    result = _invoke_models_rebuild(_build_available_models_uncached)
             except BaseException:
                 # Always reset the flag so waiting threads don't block for 60s
                 with _cache_build_cv:
@@ -5329,20 +5359,34 @@ def get_available_models(*, prefer_cache: bool = False) -> dict:
                 return True
 
         def _rebuild_worker():
-            try:
-                box["result"] = _invoke_models_rebuild(_build_available_models_uncached)
-            except Exception as exc:  # noqa: BLE001 — propagated to caller
-                box["error"] = exc
-            finally:
-                build_done.set()
-                # Only publish out-of-band if the foreground already gave up
-                # (over budget). Within budget the foreground publishes
-                # synchronously, so the worker must NOT touch the cache.
-                if budget_exceeded.is_set() and _claim_publish():
-                    if "result" in box:
-                        _publish_models_result(box["result"])
-                    else:
-                        _clear_build_in_progress()
+            # Re-bind the captured per-request profile on THIS worker thread
+            # (#3957): the daemon inherits neither the request-profile TLS nor
+            # os.environ, so without this it would probe the default profile's
+            # credentials and, over budget, publish the rebuilt catalog to the
+            # DEFAULT profile's disk cache. No-op for the default profile.
+            _worker_scope = (
+                _prof_scope_worker(_active_profile_name, "models rebuild (worker)")
+                if _prof_scope_worker is not None
+                else _nullcontext()
+            )
+            with _worker_scope:
+                try:
+                    box["result"] = _invoke_models_rebuild(_build_available_models_uncached)
+                except Exception as exc:  # noqa: BLE001 — propagated to caller
+                    box["error"] = exc
+                finally:
+                    build_done.set()
+                    # Only publish out-of-band if the foreground already gave up
+                    # (over budget). Within budget the foreground publishes
+                    # synchronously, so the worker must NOT touch the cache.
+                    # NOTE: the publish (and its disk write + fingerprint) runs
+                    # INSIDE this profile scope so the over-budget path writes
+                    # the correct profile's cache file.
+                    if budget_exceeded.is_set() and _claim_publish():
+                        if "result" in box:
+                            _publish_models_result(box["result"])
+                        else:
+                            _clear_build_in_progress()
 
         _worker = threading.Thread(
             target=_rebuild_worker,

api/profiles.py

@@ -821,6 +821,49 @@ def profile_env_for_active_request(
         yield
 
 
+@contextmanager
+def profile_scope_for_detached_worker(
+    profile_name,
+    purpose: str = "detached worker",
+    logger_override: Optional[logging.Logger] = None,
+):
+    """Bind BOTH the per-request profile TLS and the profile env on a NEW thread (#3957).
+
+    A detached worker thread (e.g. the ``models-catalog-rebuild`` daemon that
+    ``get_available_models`` spawns for a bounded rebuild) inherits neither the
+    spawning request's profile thread-local (issue #798) nor its ``os.environ``.
+    Without re-establishing both, the worker resolves the *default* profile:
+      - profile-keyed paths (``_get_models_cache_path`` / ``_get_config_path`` /
+        ``_get_auth_store_path`` / ``_models_cache_source_fingerprint``) read the
+        per-request profile via ``get_active_profile_name()`` — needs the TLS;
+      - credential lookups (``provider_model_ids`` / ``_lookup_custom_api_key_env``)
+        read ``os.environ`` — needs the profile ``.env`` applied.
+
+    Pass the profile name CAPTURED on the spawning thread (where the TLS is
+    valid) into the worker, then enter this scope at the top of the worker body.
+    It sets the request-profile TLS for this (worker) thread and applies the
+    profile env via ``profile_env_for_background_worker``, restoring both on exit.
+    No-op for the default/root profile.
+
+    Unlike ``profile_env_for_active_request`` (which reads the *current* thread's
+    TLS and must NOT clear it — the request thread keeps using it after the call),
+    this sets and then CLEARS the TLS, which is correct for a dedicated worker
+    thread that has no other use for it.
+    """
+    name = (profile_name or "").strip()
+    if not name or _is_root_profile(name):
+        yield
+        return
+    set_request_profile(name)
+    try:
+        with profile_env_for_background_worker(
+            name, purpose, logger_override=logger_override
+        ):
+            yield
+    finally:
+        clear_request_profile()
+
+
 def _set_hermes_home(home: Path):
     """Set HERMES_HOME env var and monkey-patch cached module-level paths."""
     os.environ['HERMES_HOME'] = str(home)

api/routes.py

@@ -5620,12 +5620,12 @@ def handle_get(handler, parsed) -> bool:
         return True
 
     if parsed.path == "/api/models":
-        # Apply the active per-request profile's env so the model catalog
-        # resolves against that profile's credentials, not the process-default
-        # profile's (#3957). No-op for the default profile.
-        from api.profiles import profile_env_for_active_request
-        with profile_env_for_active_request("/api/models", logger_override=logger):
-            return j(handler, get_available_models())
+        # Profile-scoping for non-default profiles (#3957) is handled INSIDE
+        # get_available_models() — it binds the active profile's env + TLS on
+        # the detached rebuild worker (and the legacy synchronous rebuild),
+        # which the request-thread wrapper could not reach. See
+        # api.config.get_available_models cold path + profile_scope_for_detached_worker.
+        return j(handler, get_available_models())
 
     if parsed.path == "/api/models/live":
         return _handle_live_models(handler, parsed)

tests/test_issue3957_profile_providers_models.py

@@ -168,15 +168,84 @@ def test_active_request_env_restores_on_exception(monkeypatch, tmp_path):
 
 
 def test_providers_and_models_routes_wrap_in_profile_env():
-    """The two read routes invoke the profile-env context manager (#3957).
+    """The two read routes are profile-scoped for non-default profiles (#3957).
 
-    Structural guard: a future refactor that drops the wrapper would silently
-    reintroduce the bug, so pin the wiring at the source level.
+    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/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).
     """
     routes_src = Path(profiles.__file__).resolve().parent.joinpath("routes.py").read_text(
         encoding="utf-8"
     )
-    # Both endpoint blocks must reference the active-request profile-env wrapper.
     assert "profile_env_for_active_request" in routes_src
-    # And it must be imported from the profiles module in routes.py.
-    assert routes_src.count("profile_env_for_active_request") >= 3  # 2 calls + >=1 import
+    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
+
+
+# ─────────────────────────────────────────────────────────────────────────────
+# Facet A (worker thread) — the detached models-rebuild worker is profile-scoped
+# (Codex CORE finding: the request-thread wrapper cannot reach the worker thread)
+# ─────────────────────────────────────────────────────────────────────────────
+
+
+def test_detached_worker_scope_noop_for_default_profile(monkeypatch):
+    """profile_scope_for_detached_worker is a no-op for the default profile."""
+    monkeypatch.setattr(profiles, "_is_root_profile", lambda n: n in ("", "default"))
+    monkeypatch.delenv("ISSUE_3957_WPROBE", raising=False)
+    # Default/empty name → no TLS set, no env applied.
+    with profiles.profile_scope_for_detached_worker("default", "test"):
+        assert profiles.get_active_profile_name() in ("", "default")
+        assert os.environ.get("ISSUE_3957_WPROBE") is None
+    with profiles.profile_scope_for_detached_worker("", "test"):
+        assert os.environ.get("ISSUE_3957_WPROBE") is None
+
+
+def test_detached_worker_scope_binds_profile_on_new_thread(monkeypatch, tmp_path):
+    """A worker thread re-binds the captured profile's TLS + env + cache path.
+
+    Reproduces the Codex CORE finding: WITHOUT the scope a new thread resolves
+    the default profile (cache path models_cache.json, no profile env); WITH it
+    the thread resolves the captured profile's cache file + .env.
+    """
+    import threading
+
+    base = tmp_path / ".hermes"
+    (base / "profiles" / "work").mkdir(parents=True)
+    (base / "profiles" / "work" / ".env").write_text(
+        "ISSUE_3957_WPROBE=worker-env\n", encoding="utf-8"
+    )
+    monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base)
+    # Point the default models cache at an isolated tmp file so the named path
+    # derives from it (models_cache.work.json under the same dir).
+    default_cache = tmp_path / "state" / "models_cache.json"
+    default_cache.parent.mkdir(parents=True, exist_ok=True)
+    monkeypatch.setattr(config, "_models_cache_path", default_cache)
+    monkeypatch.delenv("ISSUE_3957_WPROBE", raising=False)
+
+    out = {}
+
+    def worker():
+        # No TLS on this fresh thread yet → default profile resolution (the bug).
+        out["before_name"] = config._get_models_cache_path().name
+        out["before_env"] = os.environ.get("ISSUE_3957_WPROBE")
+        with profiles.profile_scope_for_detached_worker("work", "test-worker"):
+            out["inside_name"] = config._get_models_cache_path().name
+            out["inside_env"] = os.environ.get("ISSUE_3957_WPROBE")
+        out["after_name"] = config._get_models_cache_path().name
+        out["after_env"] = os.environ.get("ISSUE_3957_WPROBE")
+
+    t = threading.Thread(target=worker)
+    t.start()
+    t.join()
+
+    assert out["before_name"] == "models_cache.json"  # default (no scope)
+    assert out["before_env"] is None
+    assert out["inside_name"] == "models_cache.work.json"  # profile-scoped
+    assert out["inside_env"] == "worker-env"
+    assert out["after_name"] == "models_cache.json"  # restored
+    assert out["after_env"] is None
+