Fix Modal backend not working from CLI
Two config systems used different key names for the terminal backend: - hermes_cli/config.py, README, and all docs use "terminal.backend" - cli.py's env var mapping only recognized "terminal.env_type" Users following the docs who set `backend: modal` in ~/.hermes/config.yaml had it silently ignored -- TERMINAL_ENV always defaulted to "local". Additionally, when no config file existed, cli.py's hardcoded defaults overwrote any TERMINAL_ENV=modal set in .env, despite the comment saying "env vars take precedence." Fixes: - cli.py now normalizes "backend" -> "env_type" (backend takes precedence) - Defaults no longer overwrite .env when no config file terminal section exists - hermes status reads from config as fallback when env var isn't set Also fixes four related bugs found in the Modal/sandbox lifecycle: - file_tools cache not cleared on sandbox cleanup (stale ops on dead sandbox) - Global lock held during slow Modal teardown (blocked all tool calls 10-15s) - Race condition in file_tools between existence check and access (KeyError) - Per-task creation locks never cleaned up (memory leak)
This commit is contained in:
parent
8117d0adab
commit
2c7deb41f6
2 changed files with 31 additions and 4 deletions
24
cli.py
24
cli.py
|
|
@ -125,12 +125,20 @@ def load_cli_config() -> Dict[str, Any]:
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# Track whether the config file explicitly set terminal config.
|
||||||
|
# When using defaults (no config file / no terminal section), we should NOT
|
||||||
|
# overwrite env vars that were already set by .env -- only a user's config
|
||||||
|
# file should be authoritative.
|
||||||
|
_file_has_terminal_config = False
|
||||||
|
|
||||||
# Load from file if exists
|
# Load from file if exists
|
||||||
if config_path.exists():
|
if config_path.exists():
|
||||||
try:
|
try:
|
||||||
with open(config_path, "r") as f:
|
with open(config_path, "r") as f:
|
||||||
file_config = yaml.safe_load(f) or {}
|
file_config = yaml.safe_load(f) or {}
|
||||||
|
|
||||||
|
_file_has_terminal_config = "terminal" in file_config
|
||||||
|
|
||||||
# Handle model config - can be string (new format) or dict (old format)
|
# Handle model config - can be string (new format) or dict (old format)
|
||||||
if "model" in file_config:
|
if "model" in file_config:
|
||||||
if isinstance(file_config["model"], str):
|
if isinstance(file_config["model"], str):
|
||||||
|
|
@ -157,9 +165,14 @@ def load_cli_config() -> Dict[str, Any]:
|
||||||
print(f"[Warning] Failed to load cli-config.yaml: {e}")
|
print(f"[Warning] Failed to load cli-config.yaml: {e}")
|
||||||
|
|
||||||
# Apply terminal config to environment variables (so terminal_tool picks them up)
|
# Apply terminal config to environment variables (so terminal_tool picks them up)
|
||||||
# Only set if not already set in environment (env vars take precedence)
|
|
||||||
terminal_config = defaults.get("terminal", {})
|
terminal_config = defaults.get("terminal", {})
|
||||||
|
|
||||||
|
# Normalize config key: the new config system (hermes_cli/config.py) and all
|
||||||
|
# documentation use "backend", the legacy cli-config.yaml uses "env_type".
|
||||||
|
# Accept both, with "backend" taking precedence (it's the documented key).
|
||||||
|
if "backend" in terminal_config:
|
||||||
|
terminal_config["env_type"] = terminal_config["backend"]
|
||||||
|
|
||||||
# Handle special cwd values: "." or "auto" means use current working directory
|
# Handle special cwd values: "." or "auto" means use current working directory
|
||||||
if terminal_config.get("cwd") in (".", "auto", "cwd"):
|
if terminal_config.get("cwd") in (".", "auto", "cwd"):
|
||||||
terminal_config["cwd"] = os.getcwd()
|
terminal_config["cwd"] = os.getcwd()
|
||||||
|
|
@ -182,10 +195,15 @@ def load_cli_config() -> Dict[str, Any]:
|
||||||
"sudo_password": "SUDO_PASSWORD",
|
"sudo_password": "SUDO_PASSWORD",
|
||||||
}
|
}
|
||||||
|
|
||||||
# CLI config overrides .env for terminal settings
|
# Apply config values to env vars so terminal_tool picks them up.
|
||||||
|
# If the config file explicitly has a [terminal] section, those values are
|
||||||
|
# authoritative and override any .env settings. When using defaults only
|
||||||
|
# (no config file or no terminal section), don't overwrite env vars that
|
||||||
|
# were already set by .env -- the user's .env is the fallback source.
|
||||||
for config_key, env_var in env_mappings.items():
|
for config_key, env_var in env_mappings.items():
|
||||||
if config_key in terminal_config:
|
if config_key in terminal_config:
|
||||||
os.environ[env_var] = str(terminal_config[config_key])
|
if _file_has_terminal_config or env_var not in os.environ:
|
||||||
|
os.environ[env_var] = str(terminal_config[config_key])
|
||||||
|
|
||||||
# Apply browser config to environment variables
|
# Apply browser config to environment variables
|
||||||
browser_config = defaults.get("browser", {})
|
browser_config = defaults.get("browser", {})
|
||||||
|
|
|
||||||
|
|
@ -91,7 +91,16 @@ def show_status(args):
|
||||||
print()
|
print()
|
||||||
print(color("◆ Terminal Backend", Colors.CYAN, Colors.BOLD))
|
print(color("◆ Terminal Backend", Colors.CYAN, Colors.BOLD))
|
||||||
|
|
||||||
terminal_env = os.getenv("TERMINAL_ENV", "local")
|
terminal_env = os.getenv("TERMINAL_ENV", "")
|
||||||
|
if not terminal_env:
|
||||||
|
# Fall back to config file value when env var isn't set
|
||||||
|
# (hermes status doesn't go through cli.py's config loading)
|
||||||
|
try:
|
||||||
|
from hermes_cli.config import load_config
|
||||||
|
_cfg = load_config()
|
||||||
|
terminal_env = _cfg.get("terminal", {}).get("backend", "local")
|
||||||
|
except Exception:
|
||||||
|
terminal_env = "local"
|
||||||
print(f" Backend: {terminal_env}")
|
print(f" Backend: {terminal_env}")
|
||||||
|
|
||||||
if terminal_env == "ssh":
|
if terminal_env == "ssh":
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue