Skip to content

feat(egress): add 'hermes egress health' subcommand - #71135

Open
erhnysr wants to merge 3 commits into
NousResearch:mainfrom
erhnysr:feat/egress-health-subcommand
Open

feat(egress): add 'hermes egress health' subcommand#71135
erhnysr wants to merge 3 commits into
NousResearch:mainfrom
erhnysr:feat/egress-health-subcommand

Conversation

@erhnysr

@erhnysr erhnysr commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a hermes egress health subcommand — a lightweight liveness check for the iron-proxy daemon, requested by @teknium1 as a standalone follow-up to the iron-proxy egress feature (#30179) now that it has re-landed (#70848).

hermes egress health              # one-shot check
hermes egress health --watch      # poll every second until healthy (or Ctrl-C)
hermes egress health --timeout 30 # exit non-zero if not healthy within N seconds

Exit codes are script/cron-friendly:

code meaning
0 proxy is up and listening
1 pid exists but the port is not accepting connections
2 proxy not running or not configured

The bind-host detail (@teknium1's review note)

The liveness report reads the configured bind host via _read_http_listen_from_config() instead of hardcoding 127.0.0.1. This matters:

  • Linux: the daemon binds the docker bridge gateway (e.g. 172.17.0.1) so sandboxes can reach it via host.docker.internal:host-gateway. A loopback-only probe would report a perfectly healthy daemon as dead.
  • macOS / Windows Docker Desktop: loopback is reachable from containers and is the least-exposed bind, so that's what's reported.

get_status() already probes this host for status.listening; the command surfaces the same host:port in its output so the reported address matches what was actually tested. It falls back to loopback only when no proxy.yaml exists (matching get_status()'s own fallback).

Scope

Deliberately minimal — only the CLI command, its tests, and docs. No changes to the re-landed iron_proxy.py, so this sits cleanly on top of the reland and does not conflict with @Bartok9's stack (#35149 / #35187 / #35188), which is queued to build on top of this once it lands.

  • hermes_cli/proxy_cli.pyhealth subparser + cmd_health
  • tests/hermes_cli/test_proxy_cli_health.py — 9 tests (exit codes, --watch, --timeout, KeyboardInterrupt, and the bind-host reporting: docker-bridge host reported not loopback, and loopback fallback when unconfigured)
  • website/docs/user-guide/egress/iron-proxy.md — CLI reference tree + Failure-modes entry

Testing

$ .venv/bin/python -m pytest tests/hermes_cli/test_proxy_cli_health.py \
    tests/test_iron_proxy.py tests/test_iron_proxy_cli.py tests/hermes_cli/test_proxy.py -q
190 passed

ruff check clean on the changed files.

Closes the health-endpoint follow-up raised in the #30179 review.

erhnysr added 2 commits July 25, 2026 04:52
Adds a lightweight health check for the iron-proxy daemon.

Usage:
  hermes egress health            # one-shot check
  hermes egress health --watch    # poll until healthy
  hermes egress health --timeout 30  # fail after 30s

Exit codes:
  0  proxy is up and listening
  1  pid exists but port not accepting connections
  2  proxy not running or not configured

The liveness report uses the configured bind host from
_read_http_listen_from_config() rather than a hardcoded 127.0.0.1.
On Linux the daemon binds the docker bridge gateway (e.g. 172.17.0.1)
so sandboxes can reach it; a loopback-only probe would report a
perfectly healthy daemon as dead. get_status() already probes that
host for status.listening, and we surface the same host:port in the
output so the reported address matches what was tested. Falls back to
loopback only when no proxy.yaml exists (matching get_status()).

Closes the health-endpoint follow-up raised in PR NousResearch#30179 review.

9 tests added in test_proxy_cli_health.py.
Add the health subcommand to the CLI reference tree and Failure modes
section so operators can diagnose proxy state quickly.

Companion to the feat(egress) health subcommand implementation.
@Bartok9

Bartok9 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Nice — thanks @erhnysr, this gives me the clean base I wanted. Now that the reland (#70848) is in and hermes_cli/proxy_cli.py + agent/proxy_sources/iron_proxy.py are back at their original paths, I'll rebase my stack (#35149 doctor/audit, #35187 harden, #35188 rotate-ca) onto current main and sequence them behind this so #71135 lands first as the operator-facing liveness surface. Good call reading the configured bind host via _read_http_listen_from_config() rather than hardcoding 127.0.0.1 — that's the exact Linux docker-bridge gotcha from the earlier review rounds. My #35149 _check_token_swap CONNECT-level probe composes on top of your liveness check rather than duplicating it (port-up vs. swap-actually-firing are different signals). Will keep everything scoped to the management-API full-reload contract. 👍

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels Jul 25, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding a focused operator-facing liveness command. The configured-bind-host approach matches current get_status() behavior (agent/proxy_sources/iron_proxy.py:2335-2375) and avoids the documented Docker bridge false-negative.

Problems

  • hermes_cli/proxy_cli.py:829 returns immediately whenever --watch is absent, so hermes egress health --timeout 30 never waits. That conflicts with the PR's argument help (hermes_cli/proxy_cli.py:145) and documented standalone example (website/docs/user-guide/egress/iron-proxy.md:243).
  • The command inventory in website/docs/reference/cli-commands.md:651-708 also needs the new health subcommands; it currently proceeds from status directly to disable.

Suggested changes

  • Make a positive timeout enter the polling path, or reject timeout without watch, and add a regression test for that contract.
  • Update the CLI reference alongside the user-guide page.

Automated hermes-sweeper review.

Comment thread hermes_cli/proxy_cli.py Outdated
)
return 0

if not watch:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--timeout is advertised as usable on its own, but this early return bypasses the deadline loop, so hermes egress health --timeout 30 exits after one probe. Please either make a positive timeout imply polling or reject it without --watch, with a regression test for the chosen contract.

@Bartok9

Bartok9 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Confirmed @teknium1's read — cmd_health hits if not watch: return _check() (hermes_cli/proxy_cli.py:828) and returns after a single probe, so --timeout 30 on its own never enters the deadline loop even though the arg help and the iron-proxy.md:243 example advertise it as standalone.

@erhnysr the cleanest fix is to let a positive --timeout imply the polling path (keeps the documented standalone contract intact rather than rejecting it):

    # A positive --timeout implies polling even without --watch, so the
    # documented `hermes egress health --timeout 30` waits for the
    # deadline instead of returning after a single probe.
    if not watch and timeout <= 0:
        return _check()

That reuses the existing deadline (already computed from timeout) and the --watch loop below unchanged. Worth a small regression test asserting --timeout N (no --watch) polls until the deadline rather than exiting immediately, plus the CLI reference update in website/docs/reference/cli-commands.md:651-708 so health shows up between status and disable. Happy to send a follow-up commit for the test + docs if that's useful.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 30, 2026
cmd_health returned after a single probe whenever --watch was absent, so
'hermes egress health --timeout 30' never entered the deadline loop even
though the arg help and iron-proxy.md advertise it as standalone. Let a
positive --timeout imply polling (Bartok9's suggested fix), reusing the
existing deadline and --watch loop unchanged.

- regression tests: --timeout N without --watch polls to healthy, and
  polls to the deadline then exits non-zero when never healthy
- docs: add 'hermes egress health' + --watch/--timeout to the CLI
  command inventory in cli-commands.md (was missing between status and
  disable)
@erhnysr

erhnysr commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @teknium1 — both points fixed in 9cd7635.

1. --timeout without --watch now polls. Took @Bartok9's suggested approach (thanks!) rather than rejecting the flag combo, so the documented standalone hermes egress health --timeout 30 contract stays intact:

# A positive --timeout implies polling even without --watch, so the
# documented `hermes egress health --timeout 30` waits for the deadline
# instead of returning after a single probe.
if not watch and timeout <= 0:
    return _check()

This reuses the existing deadline (already computed from timeout) and the poll loop below unchanged — a positive --timeout alone now enters the same deadline-bounded loop as --watch.

2. Regression tests (tests/hermes_cli/test_proxy_cli_health.py):

  • test_timeout_without_watch_polls_until_healthy — an initially-down proxy that comes up before the deadline is reported healthy (asserts it probed repeatedly, not once).
  • test_timeout_without_watch_returns_nonzero_at_deadline — never-healthy case polls to the deadline then exits non-zero.

3. CLI reference. Added hermes egress health + --watch/--timeout to the command inventory in website/docs/reference/cli-commands.md between status and disable, consistent with the user-guide page.

Full suite green: 192 passed (was 190), ruff clean on the changed files.

@Bartok9

Bartok9 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Thanks @erhnysr9cd7635 matches the contract I was hoping for (--timeout alone enters the deadline loop, plus the two regression tests and the CLI inventory entry). CI looks green so far on the new head. Once this merges I'll rebase #35149 / #35187 / #35188 behind it.

@erhnysr

erhnysr commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Gentle ping on this one — CI is green and both review points are addressed as of 9cd7635. @Bartok9 has #35149/#35187/#35188 sequenced behind it, so landing this unblocks that stack.

@Bartok9

Bartok9 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

+1 on the ping — still green on my side and the 9cd7635 contract (--timeout alone polls, tests + CLI inventory) looks good to merge as-is. My stack stays sequenced behind this; happy to rebase #35149/#35187/#35188 as soon as it lands. No further changes needed from me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants