fix(cli): restrict uv-tool-install detection to running interpreter
Copilot review on PR #29703 flagged two issues with the `uv tool list` fallback in `is_uv_tool_install`: 1. False positive: `uv tool list` returns the *machine*'s installed tools, not the active install. A regular pip/venv Hermes on a host that also has `uv tool install hermes-agent` available would be misclassified as a uv-tool install, and `hermes update` would upgrade the wrong copy. 2. Overhead: the subprocess call (up to a 15s timeout) was triggered even from `recommended_update_command_for_method`, which just computes a display string. Restrict detection to properties of the running interpreter (`sys.prefix` and `sys.executable` — both can carry the uv-tool layout marker depending on entry point). Drop the `uv tool list` fallback and the `uv_path` parameter entirely. `_cmd_update_pip` now also surfaces a clear hint when the runtime looks like a uv-tool install but `uv` is missing from PATH, instead of silently falling back to `python -m pip`.
This commit is contained in:
@ -329,39 +329,31 @@ def stamp_install_method(method: str) -> None:
|
||||
pass
|
||||
|
||||
|
||||
def is_uv_tool_install(uv_path: Optional[str] = None) -> bool:
|
||||
"""Return True when Hermes is installed via ``uv tool install hermes-agent``.
|
||||
def is_uv_tool_install() -> bool:
|
||||
"""Return True when the *running* Hermes lives in a ``uv tool`` layout.
|
||||
|
||||
``uv tool`` installs live outside any virtualenv, so ``uv pip install``
|
||||
(the previous update path) fails with ``No virtual environment found``.
|
||||
The fast path inspects ``sys.prefix`` for the standard uv tool layout
|
||||
(``.../uv/tools/hermes-agent/...``); the authoritative fallback shells
|
||||
out to ``uv tool list``. Returns False on any error so callers fall
|
||||
back to the legacy pip path.
|
||||
``uv tool install hermes-agent`` places the install at
|
||||
``.../uv/tools/hermes-agent/...`` (default ``~/.local/share/uv/tools``,
|
||||
or ``$UV_TOOL_DIR/...``). Such installs live outside any virtualenv, so
|
||||
``uv pip install`` fails with ``No virtual environment found`` and the
|
||||
update path must use ``uv tool upgrade`` instead.
|
||||
|
||||
Detection is intentionally restricted to properties of the running
|
||||
interpreter (``sys.prefix`` / ``sys.executable``). We deliberately do
|
||||
NOT consult ``uv tool list``: it would also return True when
|
||||
``hermes-agent`` happens to be uv-tool-installed on the machine while
|
||||
the *active* Hermes is a regular pip/venv install, causing
|
||||
``hermes update`` to upgrade the wrong copy. It would also block on a
|
||||
subprocess call (~seconds) just to compute a recommendation string.
|
||||
"""
|
||||
prefix = os.path.normpath(sys.prefix).replace(os.sep, "/").lower()
|
||||
if "/uv/tools/hermes-agent/" in prefix + "/":
|
||||
def _has_uv_tool_marker(path: str) -> bool:
|
||||
norm = os.path.normpath(path).replace(os.sep, "/").lower()
|
||||
return "/uv/tools/hermes-agent/" in norm + "/"
|
||||
|
||||
if _has_uv_tool_marker(sys.prefix):
|
||||
return True
|
||||
if _has_uv_tool_marker(sys.executable or ""):
|
||||
return True
|
||||
if uv_path is None:
|
||||
import shutil
|
||||
uv_path = shutil.which("uv")
|
||||
if not uv_path:
|
||||
return False
|
||||
try:
|
||||
result = subprocess.run(
|
||||
[uv_path, "tool", "list"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=15,
|
||||
)
|
||||
except (OSError, subprocess.SubprocessError):
|
||||
return False
|
||||
if result.returncode != 0:
|
||||
return False
|
||||
for line in result.stdout.splitlines():
|
||||
tokens = line.strip().split()
|
||||
if tokens and tokens[0] == "hermes-agent":
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
@ -374,11 +366,10 @@ def recommended_update_command_for_method(method: str) -> str:
|
||||
if method == "docker":
|
||||
return "docker pull nousresearch/hermes-agent:latest"
|
||||
if method == "pip":
|
||||
if is_uv_tool_install():
|
||||
return "uv tool upgrade hermes-agent"
|
||||
import shutil
|
||||
uv = shutil.which("uv")
|
||||
if uv:
|
||||
if is_uv_tool_install(uv):
|
||||
return "uv tool upgrade hermes-agent"
|
||||
if shutil.which("uv"):
|
||||
return "uv pip install --upgrade hermes-agent"
|
||||
return "pip install --upgrade hermes-agent"
|
||||
return "hermes update"
|
||||
|
||||
@ -8977,11 +8977,13 @@ def _cmd_update_pip(args):
|
||||
print("→ Checking PyPI for updates...")
|
||||
|
||||
uv = shutil.which("uv")
|
||||
if uv:
|
||||
if is_uv_tool_install(uv):
|
||||
cmd = [uv, "tool", "upgrade", "hermes-agent"]
|
||||
else:
|
||||
cmd = [uv, "pip", "install", "--upgrade", "hermes-agent"]
|
||||
if is_uv_tool_install():
|
||||
if not uv:
|
||||
print("✗ Detected a uv-tool install but `uv` is not on PATH; install uv and retry.")
|
||||
sys.exit(1)
|
||||
cmd = [uv, "tool", "upgrade", "hermes-agent"]
|
||||
elif uv:
|
||||
cmd = [uv, "pip", "install", "--upgrade", "hermes-agent"]
|
||||
else:
|
||||
cmd = [sys.executable, "-m", "pip", "install", "--upgrade", "hermes-agent"]
|
||||
|
||||
|
||||
@ -6,6 +6,10 @@ environment found``. ``is_uv_tool_install`` should detect this layout and
|
||||
both the user-facing recommended command and the actual
|
||||
``_cmd_update_pip`` subprocess invocation should switch to
|
||||
``uv tool upgrade hermes-agent``.
|
||||
|
||||
Detection is restricted to properties of the running interpreter
|
||||
(``sys.prefix`` / ``sys.executable``) so a pip/venv install on a machine
|
||||
that also has ``uv tool install hermes-agent`` does not get misclassified.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
@ -26,68 +30,59 @@ class TestIsUvToolInstall:
|
||||
from hermes_cli import config
|
||||
|
||||
with patch.object(config.sys, "prefix", "/home/user/.local/share/uv/tools/hermes-agent"):
|
||||
assert config.is_uv_tool_install("uv") is True
|
||||
assert config.is_uv_tool_install() is True
|
||||
|
||||
def test_returns_true_when_uv_tool_list_includes_hermes_agent(self):
|
||||
from hermes_cli import config
|
||||
|
||||
completed = subprocess.CompletedProcess(
|
||||
["uv", "tool", "list"],
|
||||
0,
|
||||
stdout="hermes-agent v0.14.0\n- hermes\n- hermes-bot\nblack v23.0.0\n- black\n",
|
||||
stderr="",
|
||||
)
|
||||
with patch.object(config.sys, "prefix", "/some/unrelated/venv"), \
|
||||
patch("subprocess.run", return_value=completed) as mock_run:
|
||||
assert config.is_uv_tool_install("/usr/local/bin/uv") is True
|
||||
mock_run.assert_called_once()
|
||||
assert mock_run.call_args[0][0] == ["/usr/local/bin/uv", "tool", "list"]
|
||||
|
||||
def test_returns_false_when_uv_tool_list_lacks_hermes_agent(self):
|
||||
from hermes_cli import config
|
||||
|
||||
completed = subprocess.CompletedProcess(
|
||||
["uv", "tool", "list"], 0, stdout="black v23.0.0\n- black\nruff v0.5.0\n- ruff\n", stderr=""
|
||||
)
|
||||
with patch.object(config.sys, "prefix", "/some/unrelated/venv"), \
|
||||
patch("subprocess.run", return_value=completed):
|
||||
assert config.is_uv_tool_install("uv") is False
|
||||
|
||||
def test_returns_false_when_uv_tool_list_fails(self):
|
||||
from hermes_cli import config
|
||||
|
||||
completed = subprocess.CompletedProcess(["uv", "tool", "list"], 2, stdout="", stderr="oops")
|
||||
with patch.object(config.sys, "prefix", "/some/unrelated/venv"), \
|
||||
patch("subprocess.run", return_value=completed):
|
||||
assert config.is_uv_tool_install("uv") is False
|
||||
|
||||
def test_returns_false_when_subprocess_raises(self):
|
||||
def test_returns_true_when_sys_executable_matches_uv_tool_layout(self):
|
||||
"""Some uv-tool layouts surface the marker on ``sys.executable`` (bin/python)."""
|
||||
from hermes_cli import config
|
||||
|
||||
with patch.object(config.sys, "prefix", "/some/unrelated/venv"), \
|
||||
patch("subprocess.run", side_effect=subprocess.TimeoutExpired(["uv"], 15)):
|
||||
assert config.is_uv_tool_install("uv") is False
|
||||
patch.object(
|
||||
config.sys,
|
||||
"executable",
|
||||
"/home/user/.local/share/uv/tools/hermes-agent/bin/python",
|
||||
):
|
||||
assert config.is_uv_tool_install() is True
|
||||
|
||||
def test_returns_false_when_no_uv_available(self):
|
||||
def test_returns_false_when_neither_prefix_nor_executable_matches(self):
|
||||
from hermes_cli import config
|
||||
|
||||
with patch.object(config.sys, "prefix", "/some/unrelated/venv"), \
|
||||
patch("shutil.which", return_value=None):
|
||||
patch.object(config.sys, "executable", "/usr/bin/python3"):
|
||||
assert config.is_uv_tool_install() is False
|
||||
|
||||
def test_indented_alias_line_does_not_false_positive(self):
|
||||
"""A tool whose alias line is ``- hermes-agent`` shouldn't match."""
|
||||
def test_does_not_consult_uv_tool_list(self):
|
||||
"""Detection must NOT shell out: ``uv tool list`` would false-positive
|
||||
when the active install is pip/venv but the machine also has
|
||||
``uv tool install hermes-agent`` somewhere on disk. Copilot review on
|
||||
PR #29703 flagged this; the fix is to never call ``uv tool list``
|
||||
from the detection path."""
|
||||
from hermes_cli import config
|
||||
|
||||
completed = subprocess.CompletedProcess(
|
||||
["uv", "tool", "list"],
|
||||
0,
|
||||
stdout="some-other-tool v1.0.0\n- hermes-agent\n",
|
||||
stderr="",
|
||||
)
|
||||
with patch.object(config.sys, "prefix", "/some/unrelated/venv"), \
|
||||
patch("subprocess.run", return_value=completed):
|
||||
assert config.is_uv_tool_install("uv") is False
|
||||
patch.object(config.sys, "executable", "/usr/bin/python3"), \
|
||||
patch("subprocess.run") as mock_run:
|
||||
assert config.is_uv_tool_install() is False
|
||||
mock_run.assert_not_called()
|
||||
|
||||
def test_case_insensitive_match(self):
|
||||
"""Match must be case-insensitive — Windows paths preserve case
|
||||
(e.g. ``...AppData\\Local\\UV\\Tools\\hermes-agent``) and a case-sensitive
|
||||
check would miss them. We exercise the lower-cased compare path here
|
||||
without monkey-patching ``os.sep``, which would break the whole suite."""
|
||||
from hermes_cli import config
|
||||
|
||||
with patch.object(
|
||||
config.sys, "prefix", "/HOME/USER/.local/share/UV/Tools/hermes-agent"
|
||||
):
|
||||
assert config.is_uv_tool_install() is True
|
||||
|
||||
def test_handles_empty_executable(self):
|
||||
from hermes_cli import config
|
||||
|
||||
with patch.object(config.sys, "prefix", "/some/unrelated/venv"), \
|
||||
patch.object(config.sys, "executable", ""):
|
||||
assert config.is_uv_tool_install() is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@ -104,6 +99,16 @@ class TestRecommendedUpdateCommandForUvTool:
|
||||
cmd = config.recommended_update_command_for_method("pip")
|
||||
assert cmd == "uv tool upgrade hermes-agent"
|
||||
|
||||
def test_uv_tool_install_recommends_uv_tool_upgrade_even_without_uv_on_path(self):
|
||||
"""Recommendation reflects the *install method*, not whether ``uv`` is
|
||||
currently on PATH — the user needs to know the right command to run."""
|
||||
from hermes_cli import config
|
||||
|
||||
with patch("shutil.which", return_value=None), \
|
||||
patch.object(config, "is_uv_tool_install", return_value=True):
|
||||
cmd = config.recommended_update_command_for_method("pip")
|
||||
assert cmd == "uv tool upgrade hermes-agent"
|
||||
|
||||
def test_uv_pip_install_keeps_legacy_recommendation(self):
|
||||
"""Existing behavior: uv is on PATH but Hermes is a regular pip install."""
|
||||
from hermes_cli import config
|
||||
@ -114,12 +119,28 @@ class TestRecommendedUpdateCommandForUvTool:
|
||||
assert cmd == "uv pip install --upgrade hermes-agent"
|
||||
|
||||
def test_no_uv_falls_back_to_plain_pip(self):
|
||||
from hermes_cli.config import recommended_update_command_for_method
|
||||
from hermes_cli import config
|
||||
|
||||
with patch("shutil.which", return_value=None):
|
||||
cmd = recommended_update_command_for_method("pip")
|
||||
with patch("shutil.which", return_value=None), \
|
||||
patch.object(config, "is_uv_tool_install", return_value=False):
|
||||
cmd = config.recommended_update_command_for_method("pip")
|
||||
assert cmd == "pip install --upgrade hermes-agent"
|
||||
|
||||
def test_recommendation_does_not_spawn_subprocess(self):
|
||||
"""Computing the recommendation string must be cheap — no ``uv tool list``
|
||||
spawn. Copilot review on PR #29703 flagged the prior subprocess hop
|
||||
as adding overhead and a multi-second timeout window for what is
|
||||
purely a display string."""
|
||||
from hermes_cli import config
|
||||
|
||||
with patch.object(config.sys, "prefix", "/some/unrelated/venv"), \
|
||||
patch.object(config.sys, "executable", "/usr/bin/python3"), \
|
||||
patch("shutil.which", return_value="/usr/local/bin/uv"), \
|
||||
patch("subprocess.run") as mock_run:
|
||||
cmd = config.recommended_update_command_for_method("pip")
|
||||
mock_run.assert_not_called()
|
||||
assert cmd == "uv pip install --upgrade hermes-agent"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _cmd_update_pip subprocess command
|
||||
@ -162,7 +183,8 @@ class TestCmdUpdatePipUsesUvTool:
|
||||
from hermes_cli.main import _cmd_update_pip
|
||||
|
||||
mock_run.return_value = subprocess.CompletedProcess(["pip"], 0, stdout="", stderr="")
|
||||
with patch("shutil.which", return_value=None):
|
||||
with patch("shutil.which", return_value=None), \
|
||||
patch("hermes_cli.config.is_uv_tool_install", return_value=False):
|
||||
_cmd_update_pip(SimpleNamespace())
|
||||
|
||||
cmd = mock_run.call_args[0][0]
|
||||
@ -178,3 +200,18 @@ class TestCmdUpdatePipUsesUvTool:
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
_cmd_update_pip(SimpleNamespace())
|
||||
assert exc_info.value.code == 1
|
||||
|
||||
@patch("subprocess.run")
|
||||
def test_uv_tool_install_without_uv_on_path_exits_with_hint(self, mock_run):
|
||||
"""If the running interpreter looks like a uv-tool install but ``uv`` is
|
||||
somehow missing from PATH, surface a clear hint instead of silently
|
||||
falling back to ``python -m pip``, which would either fail (no venv)
|
||||
or upgrade the wrong copy."""
|
||||
from hermes_cli.main import _cmd_update_pip
|
||||
|
||||
with patch("shutil.which", return_value=None), \
|
||||
patch("hermes_cli.config.is_uv_tool_install", return_value=True):
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
_cmd_update_pip(SimpleNamespace())
|
||||
assert exc_info.value.code == 1
|
||||
mock_run.assert_not_called()
|
||||
|
||||
Reference in New Issue
Block a user