mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-18 04:41:56 +00:00
fix(terminal): bridge docker_env config to TERMINAL_DOCKER_ENV
Problem: terminal.docker_env set in config.yaml was silently ignored.
Docker containers never received the user-specified env vars.
Root cause: docker_env was missing from all three config→env bridging
maps (cli.py env_mappings, gateway/run.py _terminal_env_map,
hermes_cli/config.py _config_to_env_sync) and from the terminal_tool
_get_env_config() reader. _create_environment() consumed the key from
container_config correctly, but it was always {} because TERMINAL_DOCKER_ENV
was never set.
Also extend the list-serialisation branches in cli.py and gateway/run.py
to handle dict values via json.dumps (lists already used json.dumps;
plain str() on a dict produces undecodable output).
Fix:
- cli.py: add "docker_env": "TERMINAL_DOCKER_ENV" to env_mappings;
serialise dict values with json.dumps alongside existing list path
- gateway/run.py: same additions to _terminal_env_map and serialisation
- hermes_cli/config.py: add "terminal.docker_env": "TERMINAL_DOCKER_ENV"
to _config_to_env_sync so `hermes config set terminal.docker_env …`
persists to .env correctly
- tools/terminal_tool.py: add docker_env key to _get_env_config() reading
TERMINAL_DOCKER_ENV via _parse_env_var with default "{}"
Tests: add test_docker_env_is_bridged_everywhere to
tests/tools/test_terminal_config_env_sync.py — stash-verified: fails on
origin/main, passes with fix.
Fixes #20537
This commit is contained in:
parent
53ec32819c
commit
116a1446a4
5 changed files with 22 additions and 2 deletions
3
cli.py
3
cli.py
|
|
@ -517,6 +517,7 @@ def load_cli_config() -> Dict[str, Any]:
|
|||
"container_disk": "TERMINAL_CONTAINER_DISK",
|
||||
"container_persistent": "TERMINAL_CONTAINER_PERSISTENT",
|
||||
"docker_volumes": "TERMINAL_DOCKER_VOLUMES",
|
||||
"docker_env": "TERMINAL_DOCKER_ENV",
|
||||
"docker_mount_cwd_to_workspace": "TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE",
|
||||
"docker_run_as_host_user": "TERMINAL_DOCKER_RUN_AS_HOST_USER",
|
||||
"sandbox_dir": "TERMINAL_SANDBOX_DIR",
|
||||
|
|
@ -540,7 +541,7 @@ def load_cli_config() -> Dict[str, Any]:
|
|||
continue
|
||||
if _file_has_terminal_config or env_var not in os.environ:
|
||||
val = terminal_config[config_key]
|
||||
if isinstance(val, list):
|
||||
if isinstance(val, (list, dict)):
|
||||
os.environ[env_var] = json.dumps(val)
|
||||
else:
|
||||
os.environ[env_var] = str(val)
|
||||
|
|
|
|||
|
|
@ -460,6 +460,7 @@ if _config_path.exists():
|
|||
"container_disk": "TERMINAL_CONTAINER_DISK",
|
||||
"container_persistent": "TERMINAL_CONTAINER_PERSISTENT",
|
||||
"docker_volumes": "TERMINAL_DOCKER_VOLUMES",
|
||||
"docker_env": "TERMINAL_DOCKER_ENV",
|
||||
"docker_mount_cwd_to_workspace": "TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE",
|
||||
"docker_run_as_host_user": "TERMINAL_DOCKER_RUN_AS_HOST_USER",
|
||||
"sandbox_dir": "TERMINAL_SANDBOX_DIR",
|
||||
|
|
@ -478,7 +479,7 @@ if _config_path.exists():
|
|||
# receives a literal "~/" which the kernel rejects.
|
||||
if _cfg_key == "cwd" and isinstance(_val, str):
|
||||
_val = os.path.expanduser(_val)
|
||||
if isinstance(_val, list):
|
||||
if isinstance(_val, (list, dict)):
|
||||
os.environ[_env_var] = json.dumps(_val)
|
||||
else:
|
||||
os.environ[_env_var] = str(_val)
|
||||
|
|
|
|||
|
|
@ -4843,6 +4843,7 @@ def set_config_value(key: str, value: str):
|
|||
"terminal.vercel_runtime": "TERMINAL_VERCEL_RUNTIME",
|
||||
"terminal.docker_mount_cwd_to_workspace": "TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE",
|
||||
"terminal.docker_run_as_host_user": "TERMINAL_DOCKER_RUN_AS_HOST_USER",
|
||||
"terminal.docker_env": "TERMINAL_DOCKER_ENV",
|
||||
# terminal.cwd intentionally excluded — CLI resolves at runtime,
|
||||
# gateway bridges it in gateway/run.py. Persisting to .env causes
|
||||
# stale values to poison child processes.
|
||||
|
|
|
|||
|
|
@ -208,3 +208,19 @@ def test_docker_mount_cwd_to_workspace_is_bridged_everywhere():
|
|||
assert "docker_mount_cwd_to_workspace" in _gateway_env_map_keys()
|
||||
assert "docker_mount_cwd_to_workspace" in _save_config_env_sync_keys()
|
||||
assert "TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE" in _terminal_tool_env_var_names()
|
||||
|
||||
|
||||
def test_docker_env_is_bridged_everywhere():
|
||||
"""Regression pin for docker_env config key being silently ignored.
|
||||
|
||||
``terminal.docker_env`` in config.yaml specifies extra env vars to inject
|
||||
into the Docker container at runtime. The key was present in
|
||||
_create_environment's container_config consumer (line ~1130) but never
|
||||
bridged from config.yaml to TERMINAL_DOCKER_ENV, so the dict was always
|
||||
empty regardless of what the user set. Guard all four bridging points so
|
||||
this cannot regress.
|
||||
"""
|
||||
assert "docker_env" in _cli_env_map_keys()
|
||||
assert "docker_env" in _gateway_env_map_keys()
|
||||
assert "docker_env" in _save_config_env_sync_keys()
|
||||
assert "TERMINAL_DOCKER_ENV" in _terminal_tool_env_var_names()
|
||||
|
|
|
|||
|
|
@ -1085,6 +1085,7 @@ def _get_env_config() -> Dict[str, Any]:
|
|||
"container_disk": _parse_env_var("TERMINAL_CONTAINER_DISK", "51200"), # MB (default 50GB)
|
||||
"container_persistent": os.getenv("TERMINAL_CONTAINER_PERSISTENT", "true").lower() in ("true", "1", "yes"),
|
||||
"docker_volumes": _parse_env_var("TERMINAL_DOCKER_VOLUMES", "[]", json.loads, "valid JSON"),
|
||||
"docker_env": _parse_env_var("TERMINAL_DOCKER_ENV", "{}", json.loads, "valid JSON"),
|
||||
"docker_run_as_host_user": os.getenv("TERMINAL_DOCKER_RUN_AS_HOST_USER", "false").lower() in ("true", "1", "yes"),
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue