Skip to content

feat(docker): docker_network config field for sandbox network attachment - #16359

Closed
sasha-id wants to merge 1 commit into
NousResearch:mainfrom
sasha-id:feature/docker-network-config-field-upstream
Closed

feat(docker): docker_network config field for sandbox network attachment#16359
sasha-id wants to merge 1 commit into
NousResearch:mainfrom
sasha-id:feature/docker-network-config-field-upstream

Conversation

@sasha-id

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds a docker_network field to the terminal: config schema, exposed by widening DockerEnvironment.network from bool to bool | str. Lets the sandbox container attach to a user-defined docker network so it can reach compose-deployed sibling services (databases, MCP servers, monitoring) by container DNS — a common deployment pattern that's currently unreachable without heavyweight workarounds (HERMES_DOCKER_BINARY wrapper, iptables, or tailscale routing).

Specifically narrower than a generic docker_extra_args knob — single-purpose, no privilege-escalation surface (no way to bypass _SECURITY_ARGS).

Related Issue

No prior issue — opening speculatively. Happy to file one and convert if maintainers prefer that flow.

Type of Change

  • ✨ New feature (non-breaking change that adds functionality)

Changes Made

  • tools/environments/docker.py: signature network: boolnetwork: bool | str; body conditional rewritten to handle three cases (is False--network=none, isinstance(_, str)--network=<value>, else default).
  • tools/terminal_tool.py: extract docker_network from container_config (default True) in _create_environment; pass network=docker_network to _DockerEnvironment; add "docker_network": config.get("docker_network", True) to the container_config builder so the YAML field actually reaches the call site.
  • cli-config.yaml.example: document the new field under the Docker backend section, alongside the existing docker_forward_env / docker_mount_cwd_to_workspace examples.
  • tests/tools/test_docker_environment.py: 3 new constructor tests (network=True omits flag, network=False adds --network=none, network="name" adds --network=name).
  • tests/tools/test_parse_env_var.py: 2 new threading tests (docker_network extracted from container_config reaches _DockerEnvironment; absence defaults to True).

How to Test

  1. pytest tests/tools/test_docker_environment.py tests/tools/test_parse_env_var.py -v — all green (5 new + pre-existing 31 = 36 passed).
  2. End-to-end: in ~/.hermes/config.yaml, set:
    terminal:
      backend: "docker"
      docker_network: "my-named-net"
    and start a sibling container on my-named-net (e.g. docker run --rm --network=my-named-net --name sibling alpine sleep 3600). Then in a hermes session: getent hosts sibling resolves; curl http://sibling:<port> succeeds.
  3. Backward compatibility: same config without docker_network runs identically (no --network flag — Docker's default bridge).

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this feature (single commit)
  • I've run pytest tests/tools/test_docker_environment.py tests/tools/test_parse_env_var.py -q and all tests pass (36 passed, including the 5 new)
  • I've added tests for my changes
  • I've tested on my platform: macOS 15 / Docker via OrbStack, deployed to Debian 12 LXC

Documentation & Housekeeping

  • I've updated relevant documentation (cli-config.yaml.example + behavior described in commit body)
  • I've updated cli-config.yaml.example for the new config key
  • N/A — no architecture or workflow change
  • I've considered cross-platform impact: the --network=<name> flag is identical across Docker on Linux/macOS/Windows; no platform-specific code paths touched
  • N/A — not a tool, this is a config field

Notes for reviewer

One subtle behavior change worth disclosing: the body conditional changed from if not network: to if network is False: ... elif isinstance(network, str): .... Previously any falsy non-bool (None, "", 0) would trigger --network=none. Now only literal False does. In practice the only caller is cc.get("docker_network", True) which returns True | False | str, so users with existing configs see no change. Mentioning explicitly so it doesn't surprise future spelunkers.

Widen DockerEnvironment's `network` parameter from bool to bool | str.
Allows attaching the sandbox to a named user-defined docker network
(e.g. for accessing compose-deployed sibling services like databases,
MCP servers, or monitoring stacks via container DNS).

- True (default): no --network flag — docker default bridge (current behavior)
- False: --network=none (current behavior preserved)
- str: --network=<name> (new — joins the specified network)

Schema addition in terminal: config:

  terminal:
    backend: docker
    docker_network: my-net    # new — string

Existing configs without `docker_network` retain identical behavior.
@sasha-id
sasha-id force-pushed the feature/docker-network-config-field-upstream branch from 2f310bd to 254ed3f Compare May 4, 2026 16:16
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have backend/docker Docker container execution tool/terminal Terminal execution and process management area/docker Docker image, Compose, packaging labels May 4, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Likely duplicate of #4922 — same feature: wiring docker_network config through docker backend to docker run --network.

1 similar comment
@alt-glitch

Copy link
Copy Markdown
Collaborator

Likely duplicate of #4922 — same feature: wiring docker_network config through docker backend to docker run --network.

@sasha-id

Copy link
Copy Markdown
Contributor Author

Closing — upstream's docker_extra_args (commit ebf2ea5, shipped in v2026.5.7) subsumes this PR's use case for my deployment. I can wire the docker network via docker_extra_args: ["--network", "hermes-mcp-net"] instead of a dedicated config field.

@sasha-id sasha-id closed this May 21, 2026
@sasha-id
sasha-id deleted the feature/docker-network-config-field-upstream branch May 21, 2026 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docker Docker image, Compose, packaging backend/docker Docker container execution P3 Low — cosmetic, nice to have tool/terminal Terminal execution and process management type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants