From e210fd8c1ffd937996aecdba2a3675a387e65109 Mon Sep 17 00:00:00 2001 From: Shanthan Subramaniam Date: Sun, 23 Aug 2026 23:16:54 -0400 Subject: [PATCH] fix(gateway): resolve PairingStore's default pairing dir lazily, not at import time PairingStore(profile=None) resolved its storage directory from the module-level PAIRING_DIR constant, which was computed exactly once, at module import time. A long-lived process (the gateway, started once at container/process boot) can import this module before HERMES_HOME or a profile's context is fully established, freezing PAIRING_DIR to a wrong value for the rest of that process's lifetime -- even though a freshly started, short-lived process (e.g. the `hermes pairing` CLI) re-imports the module later with the environment already correct. That asymmetry is exactly what made pending pairing codes issued by the gateway process unrecoverable (the pending-code write landed under the stale, wrong directory) while CLI-invoked writes to the same nominal directory kept working -- see #93449 for the full writeup and a live reproduction. tests/hermes_cli/test_dashboard_admin_endpoints.py already carried a comment acknowledging this exact staleness in passing ("the module-level PAIRING_DIR is bound at import"), and TestProfileScopedStorage::test_default_store_uses_global_dir's own comment describes working around it rather than it being intentional behavior -- this fixes the underlying cause both were compensating for. The profile-scoped branch already resolved its directory lazily inside __init__ (matching this docstring's claim that resolution is lazy); this brings the non-profile branch in line with it. Fix keeps PAIRING_DIR as the same test seam already used throughout the test suite (`patch("gateway.pairing.PAIRING_DIR", tmp_path)`, ~30 call sites) unchanged: it's now a None sentinel instead of an eagerly computed path, and a new _default_pairing_dir() helper resolves it fresh on every call, honoring a patched (non-None) value when one is set. No existing test needed to change. Added a regression test that does not patch PAIRING_DIR directly and instead exercises the real lazy-resolution path across two different HERMES_HOME values in the same process -- confirmed it fails on the pre-fix code (gets stuck with whatever the first PairingStore() call in the test session happened to see) and passes with the fix. Verified: tests/gateway/test_pairing.py (39, incl. the new one), tests/hermes_cli/test_pairing.py, and tests/tools/test_pr_6656_regressions.py all pass unmodified. --- gateway/pairing.py | 36 ++++++++++++++++++++++++++++++++--- tests/gateway/test_pairing.py | 25 ++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/gateway/pairing.py b/gateway/pairing.py index 42ce7a89e5..23d213d689 100644 --- a/gateway/pairing.py +++ b/gateway/pairing.py @@ -56,7 +56,37 @@ LOCKOUT_SECONDS = 3600 # Lockout duration after too many failures MAX_PENDING_PER_PLATFORM = 3 # Max pending codes per platform MAX_FAILED_ATTEMPTS = 5 # Failed approvals before lockout -PAIRING_DIR = get_hermes_dir("platforms/pairing", "pairing") +# Default (non-profile-scoped) pairing directory. Left unresolved (``None``) +# here rather than computed eagerly: this module is imported once by the +# long-lived gateway process at container/process boot, and computing the +# path eagerly freezes it to whatever HERMES_HOME/profile context existed +# at that exact import moment for the rest of the process's lifetime -- +# even if a context-local override (see hermes_constants.set_hermes_home_override) +# is established afterward. A freshly-started, short-lived process (e.g. the +# ``hermes pairing`` CLI) re-imports this module later with the final +# environment already in place, so it never observes the stale value -- the +# resulting asymmetry is what made pending pairing codes issued by the +# gateway unrecoverable while CLI-side writes to the same directory kept +# working (NousResearch/hermes-agent#93449). +# +# ``_default_pairing_dir()`` below resolves this fresh on every call in +# production. Tests patch this attribute directly to a concrete path for +# isolation (e.g. ``patch("gateway.pairing.PAIRING_DIR", tmp_path)``); that +# continues to work unchanged, since a patched (non-``None``) value takes +# precedence over recomputing. +PAIRING_DIR = None + + +def _default_pairing_dir() -> Path: + """Resolve the default (non-profile-scoped) pairing directory. + + Recomputed on every call rather than cached at import time -- see the + ``PAIRING_DIR`` comment above for why. Honors ``PAIRING_DIR`` when a + caller (typically a test) has explicitly set it to a concrete path. + """ + if PAIRING_DIR is not None: + return PAIRING_DIR + return get_hermes_dir("platforms/pairing", "pairing") # Platform value -> its per-platform allowlist env var. When an operator has @@ -371,7 +401,7 @@ def _migrate_split_pairing_dirs( home = home or get_hermes_home() old_dir = home / "pairing" new_dir = home / "platforms" / "pairing" - active = active or PAIRING_DIR + active = active if active is not None else _default_pairing_dir() alternate = new_dir if active.resolve() == old_dir.resolve() else old_dir _merge_pairing_dir(active, alternate) @@ -434,7 +464,7 @@ class PairingStore: home=profile_home, ) else: - self._dir = PAIRING_DIR + self._dir = _default_pairing_dir() self._dir.mkdir(parents=True, exist_ok=True) if profile: # Explicit stores must resolve exactly as a standalone diff --git a/tests/gateway/test_pairing.py b/tests/gateway/test_pairing.py index b377eb175e..fbd1de4c80 100644 --- a/tests/gateway/test_pairing.py +++ b/tests/gateway/test_pairing.py @@ -574,6 +574,31 @@ class TestProfileScopedStorage: assert store._dir == tmp_path assert store._approved_path("weixin") == tmp_path / "weixin-approved.json" + def test_default_store_is_not_frozen_at_first_use(self, tmp_path, monkeypatch): + """Regression test for #93449. + + PairingStore() (no profile) must not freeze its directory to + whatever HERMES_HOME resolved to the first time this module's + default path was computed. A long-lived process (the gateway, + started once at container/process boot) can construct a + PairingStore before HERMES_HOME/profile context is fully + established; a later store in the same process must still pick up + the real, current value instead of being stuck with a stale one. + Deliberately does not patch PAIRING_DIR directly, unlike the sibling + test above -- this exercises the real (unpatched) lazy-resolution + path itself. + """ + first_home = tmp_path / "first" + second_home = tmp_path / "second" + + monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: first_home) + first_store = PairingStore() + assert first_store._dir == first_home / "platforms" / "pairing" + + monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: second_home) + second_store = PairingStore() + assert second_store._dir == second_home / "platforms" / "pairing" + def test_profile_store_uses_profiles_subdir(self, tmp_path, monkeypatch): """Explicit profile stores use that profile's normal Hermes layout.""" monkeypatch.setenv("HERMES_HOME", str(tmp_path))