fix: clearer terminal backend requirement errors
Salvaged from PR #979 onto current main. Preserve the current terminal backend checks while surfacing actionable preflight errors for unknown TERMINAL_ENV values, missing SSH host/user configuration, and missing Modal credentials/config. Tighten the modal regression test so it deterministically exercises the config-missing path.
This commit is contained in:
@@ -1,15 +1,77 @@
|
||||
import importlib
|
||||
import logging
|
||||
|
||||
from tools.terminal_tool import check_terminal_requirements
|
||||
terminal_tool_module = importlib.import_module("tools.terminal_tool")
|
||||
|
||||
|
||||
def _clear_terminal_env(monkeypatch):
|
||||
"""Remove terminal env vars that could affect requirements checks."""
|
||||
keys = [
|
||||
"TERMINAL_ENV",
|
||||
"TERMINAL_SSH_HOST",
|
||||
"TERMINAL_SSH_USER",
|
||||
"MODAL_TOKEN_ID",
|
||||
"HOME",
|
||||
"USERPROFILE",
|
||||
]
|
||||
for key in keys:
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
|
||||
|
||||
def test_local_terminal_requirements_do_not_depend_on_minisweagent(monkeypatch, caplog):
|
||||
"""Local backend uses Hermes' own LocalEnvironment wrapper and should not
|
||||
be marked unavailable just because `minisweagent` isn't importable."""
|
||||
_clear_terminal_env(monkeypatch)
|
||||
monkeypatch.setenv("TERMINAL_ENV", "local")
|
||||
|
||||
with caplog.at_level(logging.ERROR):
|
||||
ok = check_terminal_requirements()
|
||||
ok = terminal_tool_module.check_terminal_requirements()
|
||||
|
||||
assert ok is True
|
||||
assert "Terminal requirements check failed" not in caplog.text
|
||||
|
||||
|
||||
def test_unknown_terminal_env_logs_error_and_returns_false(monkeypatch, caplog):
|
||||
_clear_terminal_env(monkeypatch)
|
||||
monkeypatch.setenv("TERMINAL_ENV", "unknown-backend")
|
||||
|
||||
with caplog.at_level(logging.ERROR):
|
||||
ok = terminal_tool_module.check_terminal_requirements()
|
||||
|
||||
assert ok is False
|
||||
assert any(
|
||||
"Unknown TERMINAL_ENV 'unknown-backend'" in record.getMessage()
|
||||
for record in caplog.records
|
||||
)
|
||||
|
||||
|
||||
def test_ssh_backend_without_host_or_user_logs_and_returns_false(monkeypatch, caplog):
|
||||
_clear_terminal_env(monkeypatch)
|
||||
monkeypatch.setenv("TERMINAL_ENV", "ssh")
|
||||
|
||||
with caplog.at_level(logging.ERROR):
|
||||
ok = terminal_tool_module.check_terminal_requirements()
|
||||
|
||||
assert ok is False
|
||||
assert any(
|
||||
"SSH backend selected but TERMINAL_SSH_HOST and TERMINAL_SSH_USER" in record.getMessage()
|
||||
for record in caplog.records
|
||||
)
|
||||
|
||||
|
||||
def test_modal_backend_without_token_or_config_logs_specific_error(monkeypatch, caplog, tmp_path):
|
||||
_clear_terminal_env(monkeypatch)
|
||||
monkeypatch.setenv("TERMINAL_ENV", "modal")
|
||||
monkeypatch.setenv("HOME", str(tmp_path))
|
||||
monkeypatch.setenv("USERPROFILE", str(tmp_path))
|
||||
monkeypatch.setattr(terminal_tool_module, "ensure_minisweagent_on_path", lambda *_args, **_kwargs: None)
|
||||
monkeypatch.setattr(terminal_tool_module.importlib.util, "find_spec", lambda _name: object())
|
||||
|
||||
with caplog.at_level(logging.ERROR):
|
||||
ok = terminal_tool_module.check_terminal_requirements()
|
||||
|
||||
assert ok is False
|
||||
assert any(
|
||||
"Modal backend selected but no MODAL_TOKEN_ID environment variable" in record.getMessage()
|
||||
for record in caplog.records
|
||||
)
|
||||
|
||||
@@ -1171,7 +1171,13 @@ def check_terminal_requirements() -> bool:
|
||||
|
||||
elif env_type == "ssh":
|
||||
# Check that host and user are configured
|
||||
return bool(config.get("ssh_host")) and bool(config.get("ssh_user"))
|
||||
if not config.get("ssh_host") or not config.get("ssh_user"):
|
||||
logger.error(
|
||||
"SSH backend selected but TERMINAL_SSH_HOST and TERMINAL_SSH_USER "
|
||||
"are not both set. Configure both or switch TERMINAL_ENV to 'local'."
|
||||
)
|
||||
return False
|
||||
return True
|
||||
|
||||
elif env_type == "modal":
|
||||
ensure_minisweagent_on_path(Path(__file__).resolve().parent.parent)
|
||||
@@ -1179,16 +1185,30 @@ def check_terminal_requirements() -> bool:
|
||||
logger.error("mini-swe-agent is required for modal terminal backend but is not importable")
|
||||
return False
|
||||
# Check for modal token
|
||||
return os.getenv("MODAL_TOKEN_ID") is not None or Path.home().joinpath(".modal.toml").exists()
|
||||
has_token = os.getenv("MODAL_TOKEN_ID") is not None
|
||||
has_config = Path.home().joinpath(".modal.toml").exists()
|
||||
if not (has_token or has_config):
|
||||
logger.error(
|
||||
"Modal backend selected but no MODAL_TOKEN_ID environment variable "
|
||||
"or ~/.modal.toml config file was found. Configure Modal or choose "
|
||||
"a different TERMINAL_ENV."
|
||||
)
|
||||
return False
|
||||
return True
|
||||
|
||||
elif env_type == "daytona":
|
||||
from daytona import Daytona
|
||||
return os.getenv("DAYTONA_API_KEY") is not None
|
||||
|
||||
else:
|
||||
logger.error(
|
||||
"Unknown TERMINAL_ENV '%s'. Use one of: local, docker, singularity, "
|
||||
"modal, daytona, ssh.",
|
||||
env_type,
|
||||
)
|
||||
return False
|
||||
except Exception as e:
|
||||
logger.error("Terminal requirements check failed: %s", e)
|
||||
logger.error("Terminal requirements check failed: %s", e, exc_info=True)
|
||||
return False
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user