mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-18 04:41:56 +00:00
fix(plugins): resolve Git binary for installs under minimal PATH
Resolve git via shutil.which with POSIX and Git-for-Windows fallbacks before clone and pull so Dashboard/API installs do not misreport Git as missing. Add regression tests for the resolver and pull subprocess invocation.
This commit is contained in:
parent
124fbb0af0
commit
c8ede8aa1b
2 changed files with 110 additions and 2 deletions
|
|
@ -9,6 +9,7 @@ rendered with Rich Markdown. Otherwise a default confirmation is shown.
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import functools
|
||||||
import logging
|
import logging
|
||||||
import os
|
import os
|
||||||
import shutil
|
import shutil
|
||||||
|
|
@ -23,6 +24,41 @@ from hermes_cli.config import cfg_get
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
|
@functools.lru_cache(maxsize=1)
|
||||||
|
def _resolve_git_executable() -> Optional[str]:
|
||||||
|
"""Resolve a git binary for subprocess use when ``PATH`` may be minimal.
|
||||||
|
|
||||||
|
Matches other Hermes subprocess resolution: :func:`shutil.which` first,
|
||||||
|
then common Git for Windows install paths and POSIX defaults.
|
||||||
|
"""
|
||||||
|
found = shutil.which("git")
|
||||||
|
if found:
|
||||||
|
return found
|
||||||
|
if os.name == "nt":
|
||||||
|
prog = os.environ.get("ProgramFiles", r"C:\Program Files")
|
||||||
|
prog_x86 = os.environ.get("ProgramFiles(x86)", r"C:\Program Files (x86)")
|
||||||
|
local = os.environ.get("LOCALAPPDATA", "")
|
||||||
|
candidates = [
|
||||||
|
os.path.join(prog, "Git", "cmd", "git.exe"),
|
||||||
|
os.path.join(prog, "Git", "bin", "git.exe"),
|
||||||
|
os.path.join(prog_x86, "Git", "cmd", "git.exe"),
|
||||||
|
os.path.join(prog_x86, "Git", "bin", "git.exe"),
|
||||||
|
]
|
||||||
|
if local:
|
||||||
|
candidates.extend(
|
||||||
|
(
|
||||||
|
os.path.join(local, "Programs", "Git", "cmd", "git.exe"),
|
||||||
|
os.path.join(local, "Programs", "Git", "bin", "git.exe"),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
candidates = ["/usr/bin/git", "/usr/local/bin/git", "/bin/git"]
|
||||||
|
for c in candidates:
|
||||||
|
if c and os.path.isfile(c):
|
||||||
|
return c
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
class PluginOperationError(Exception):
|
class PluginOperationError(Exception):
|
||||||
"""Recoverable plugin install/update failure (CLI exits; HTTP maps to 4xx)."""
|
"""Recoverable plugin install/update failure (CLI exits; HTTP maps to 4xx)."""
|
||||||
|
|
||||||
|
|
@ -324,9 +360,13 @@ def _install_plugin_core(identifier: str, *, force: bool) -> tuple[Path, dict, s
|
||||||
with tempfile.TemporaryDirectory() as tmp:
|
with tempfile.TemporaryDirectory() as tmp:
|
||||||
tmp_target = Path(tmp) / "plugin"
|
tmp_target = Path(tmp) / "plugin"
|
||||||
|
|
||||||
|
git_exe = _resolve_git_executable()
|
||||||
|
if not git_exe:
|
||||||
|
raise PluginOperationError("git is not installed or not in PATH.")
|
||||||
|
|
||||||
try:
|
try:
|
||||||
result = subprocess.run(
|
result = subprocess.run(
|
||||||
["git", "clone", "--depth", "1", git_url, str(tmp_target)],
|
[git_exe, "clone", "--depth", "1", git_url, str(tmp_target)],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
timeout=60,
|
timeout=60,
|
||||||
|
|
@ -1472,9 +1512,12 @@ def dashboard_update_user_plugin(name: str) -> dict[str, Any]:
|
||||||
|
|
||||||
|
|
||||||
def _git_pull_plugin_dir(target: Path) -> tuple[bool, str]:
|
def _git_pull_plugin_dir(target: Path) -> tuple[bool, str]:
|
||||||
|
git_exe = _resolve_git_executable()
|
||||||
|
if not git_exe:
|
||||||
|
return False, "git is not installed or not in PATH."
|
||||||
try:
|
try:
|
||||||
result = subprocess.run(
|
result = subprocess.run(
|
||||||
["git", "pull", "--ff-only"],
|
[git_exe, "pull", "--ff-only"],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
timeout=60,
|
timeout=60,
|
||||||
|
|
|
||||||
|
|
@ -12,9 +12,11 @@ import pytest
|
||||||
import yaml
|
import yaml
|
||||||
|
|
||||||
from hermes_cli.plugins_cmd import (
|
from hermes_cli.plugins_cmd import (
|
||||||
|
PluginOperationError,
|
||||||
_copy_example_files,
|
_copy_example_files,
|
||||||
_read_manifest,
|
_read_manifest,
|
||||||
_repo_name_from_url,
|
_repo_name_from_url,
|
||||||
|
_resolve_git_executable,
|
||||||
_resolve_git_url,
|
_resolve_git_url,
|
||||||
_sanitize_plugin_name,
|
_sanitize_plugin_name,
|
||||||
plugins_command,
|
plugins_command,
|
||||||
|
|
@ -99,6 +101,69 @@ class TestResolveGitUrl:
|
||||||
_resolve_git_url("a/b/c")
|
_resolve_git_url("a/b/c")
|
||||||
|
|
||||||
|
|
||||||
|
# ── _resolve_git_executable ─────────────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
class TestResolveGitExecutable:
|
||||||
|
"""Fallback resolution when bare ``git`` is not discoverable via ``PATH``."""
|
||||||
|
|
||||||
|
def teardown_method(self):
|
||||||
|
_resolve_git_executable.cache_clear()
|
||||||
|
|
||||||
|
def test_prefers_shutil_which(self):
|
||||||
|
import hermes_cli.plugins_cmd as pc
|
||||||
|
|
||||||
|
_resolve_git_executable.cache_clear()
|
||||||
|
with patch.object(pc.shutil, "which", return_value="/usr/local/bin/git"):
|
||||||
|
assert pc._resolve_git_executable() == "/usr/local/bin/git"
|
||||||
|
|
||||||
|
def test_fallback_posix_first_matching_path(self):
|
||||||
|
import hermes_cli.plugins_cmd as pc
|
||||||
|
|
||||||
|
_resolve_git_executable.cache_clear()
|
||||||
|
|
||||||
|
def _isfile(p: str) -> bool:
|
||||||
|
return p == "/usr/local/bin/git"
|
||||||
|
|
||||||
|
with patch.object(pc.shutil, "which", return_value=None):
|
||||||
|
with patch.object(pc.os, "name", "posix"):
|
||||||
|
with patch.object(pc.os.path, "isfile", side_effect=_isfile):
|
||||||
|
assert pc._resolve_git_executable() == "/usr/local/bin/git"
|
||||||
|
|
||||||
|
def test_returns_none_when_unavailable(self):
|
||||||
|
import hermes_cli.plugins_cmd as pc
|
||||||
|
|
||||||
|
_resolve_git_executable.cache_clear()
|
||||||
|
with patch.object(pc.shutil, "which", return_value=None):
|
||||||
|
with patch.object(pc.os, "name", "posix"):
|
||||||
|
with patch.object(pc.os.path, "isfile", return_value=False):
|
||||||
|
assert pc._resolve_git_executable() is None
|
||||||
|
|
||||||
|
def test_git_pull_uses_resolved_executable(self, tmp_path):
|
||||||
|
import hermes_cli.plugins_cmd as pc
|
||||||
|
|
||||||
|
_resolve_git_executable.cache_clear()
|
||||||
|
with patch.object(
|
||||||
|
pc,
|
||||||
|
"_resolve_git_executable",
|
||||||
|
return_value="/resolved/git",
|
||||||
|
):
|
||||||
|
with patch.object(pc.subprocess, "run") as run:
|
||||||
|
run.return_value = MagicMock(returncode=0, stdout="Already up to date\n", stderr="")
|
||||||
|
ok, msg = pc._git_pull_plugin_dir(tmp_path)
|
||||||
|
assert ok is True
|
||||||
|
run.assert_called_once()
|
||||||
|
assert run.call_args[0][0][0] == "/resolved/git"
|
||||||
|
|
||||||
|
def test_install_core_raises_when_git_unresolved(self):
|
||||||
|
import hermes_cli.plugins_cmd as pc
|
||||||
|
|
||||||
|
_resolve_git_executable.cache_clear()
|
||||||
|
with patch.object(pc, "_resolve_git_executable", return_value=None):
|
||||||
|
with pytest.raises(PluginOperationError, match="git is not installed"):
|
||||||
|
pc._install_plugin_core("owner/repo", force=True)
|
||||||
|
|
||||||
|
|
||||||
# ── _repo_name_from_url ──────────────────────────────────────────────────
|
# ── _repo_name_from_url ──────────────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue