From 2824899321dcaa0ce971f989cfd557d34d8cb90f Mon Sep 17 00:00:00 2001 From: Yiipu Date: Wed, 5 Aug 2026 07:37:58 +0800 Subject: [PATCH] fix(terminal): correct off-by-one in _hermes_repo_root path resolution _hermes_repo_root used parents[1] which resolves to tools/ instead of the repository root. The file lives at tools/environments/local.py so it needs parents[2] to reach the actual repo root that Electron injects into PYTHONPATH. The test test_repo_root_stripped reused the module constant under test as its input. This made it pass regardless of what the constant pointed at. The test now computes the real repo root independently from the source file location. It fails with the old parents[1] code and passes with the fix. Reported by spfcraze in PR #78917 review. --- tests/tools/test_local_env_blocklist.py | 26 ++++++++++++++++++++----- tools/environments/local.py | 13 +++++++------ 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/tests/tools/test_local_env_blocklist.py b/tests/tools/test_local_env_blocklist.py index 88d4a51d67..58f50e3226 100644 --- a/tests/tools/test_local_env_blocklist.py +++ b/tests/tools/test_local_env_blocklist.py @@ -10,6 +10,7 @@ See: https://github.com/NousResearch/hermes-agent/issues/1264 import os import threading +from pathlib import Path from unittest.mock import MagicMock, patch import pytest @@ -532,16 +533,31 @@ class TestPythonpathSelectiveStrip: assert "/home/user/my-lib" in entries def test_repo_root_stripped(self): - """The Hermes repo root entry is stripped from PYTHONPATH.""" - from tools.environments.local import _strip_mismatched_site_packages, _hermes_repo_root - repo_root = str(_hermes_repo_root) + """The Hermes repo root entry is stripped from PYTHONPATH. + + Electron prepends the *actual* repository root (the directory + containing ``tools/``, ``hermes_cli/``, etc.) to PYTHONPATH so the + backend can ``import tools``. This test independently computes that + real repo root from the source-file location - three levels up from + ``tools/environments/local.py`` - rather than reusing the module + constant under test. That way an off-by-one in ``_hermes_repo_root`` + (e.g. ``parents[1]`` resolving to ``tools/``) would cause this test + to fail instead of silently passing. + """ + from tools.environments.local import _strip_mismatched_site_packages + + # Independently compute the real repo root: local.py lives at + # tools/environments/local.py, so the repo root is parents[2]. + local_file = Path(__import__("tools.environments.local", fromlist=["__file__"]).__file__).resolve() + real_repo_root = str(local_file.parents[2]) + env = { - "PYTHONPATH": os.pathsep.join([repo_root, "/home/user/my-lib"]), + "PYTHONPATH": os.pathsep.join([real_repo_root, "/home/user/my-lib"]), } _strip_mismatched_site_packages(env) pp = env.get("PYTHONPATH", "") entries = pp.split(os.pathsep) if pp else [] - assert repo_root not in entries + assert real_repo_root not in entries assert "/home/user/my-lib" in entries diff --git a/tools/environments/local.py b/tools/environments/local.py index d7378ea55a..3b00ba922e 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -1364,12 +1364,13 @@ def _is_path_under(child: Path, parent: Path) -> bool: #: the interpreter is actually inside a venv (``sys.prefix != sys.base_prefix``). _hermes_venv_root: Path = Path(sys.prefix) -#: The Hermes repository root - two levels up from this file -#: (``tools/environments/local.py`` -> ``tools/`` -> repo root). This is the -#: directory the Electron app prepends to PYTHONPATH so the backend can do -#: ``import tools``, ``import hermes_cli``, etc. Subprocesses that are NOT -#: the Hermes backend don't need it and it can shadow local packages. -_hermes_repo_root: Path = Path(__file__).resolve().parents[1] +#: The Hermes repository root - three levels up from this file +#: (``tools/environments/local.py`` -> ``tools/environments`` -> ``tools`` +#: -> repo root). This is the directory the Electron app prepends to +#: PYTHONPATH so the backend can do ``import tools``, ``import hermes_cli``, +#: etc. Subprocesses that are NOT the Hermes backend don't need it and it +#: can shadow local packages. +_hermes_repo_root: Path = Path(__file__).resolve().parents[2] #: Whether the current interpreter is running inside a venv. On Python 3.3+ #: ``sys.base_prefix != sys.prefix`` indicates a venv (or virtualenv).