Skip to content

fix(gateway): wait for launchd restart readiness in-band (#56524) - #56908

Open
izumi0uu wants to merge 1 commit into
NousResearch:mainfrom
izumi0uu:fix/launchd-inband-restart-56524
Open

fix(gateway): wait for launchd restart readiness in-band (#56524)#56908
izumi0uu wants to merge 1 commit into
NousResearch:mainfrom
izumi0uu:fix/launchd-inband-restart-56524

Conversation

@izumi0uu

@izumi0uu izumi0uu commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the macOS launchd in-band restart/update path so Hermes does not report success before a fresh gateway process is actually runtime-ready.

The old self-restart path returned immediately after sending SIGUSR1, which let hermes update / hermes gateway restart complete while the old gateway was still draining and before launchd had confirmed a healthy replacement. In the gateway-hosted terminal path, that also left fallback launchctl kickstart -k vulnerable to being killed with the old gateway process group.

This PR waits for the old PID to exit, waits for a fresh launchd-managed PID to reach gateway_state=running, and uses a detached watcher plus a shared launchd recovery helper so forced fallback survives in-band teardown.

Related Issue

Fixes #56524

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/gateway.py
    • add _wait_for_pid_exit() and _wait_for_launchd_service_restart() so launchd restart success means a fresh runtime-ready gateway PID, not just a signal request
    • add a detached launchd restart watcher for in-band self-restart / update flows so fallback restart survives gateway teardown
    • unify launchd kickstart / unloaded-job recovery / detached fallback behind _kickstart_launchd_service_and_wait()
    • route launchd_restart() through the shared recovery path and only clear the unsupported marker after runtime-ready recovery
  • tests/hermes_cli/test_gateway_service.py
    • cover graceful self-restart watcher flow
    • cover drain-timeout fallback to launchctl kickstart -k
    • cover fast relaunch with no None PID gap
    • cover readiness timeout log guidance
    • cover unloaded-job recovery waiting for runtime-ready after bootout -> bootstrap -> kickstart

How to Test

  1. Reproduce the old behavior on the clean baseline for #56524:
    • self-restart requests return ✓ Service restart requested immediately after SIGUSR1, before launchd runtime-ready confirmation
  2. Verify the targeted launchd restart coverage:
    • ./scripts/run_tests.sh tests/hermes_cli/test_gateway_service.py -k 'launchd_restart_drains_running_gateway_before_kickstart or launchd_restart_self_request or wait_for_launchd_service_restart_timeout_reports_logs or launchd_restart_boots_out_stale_registration_before_bootstrap or launchd_restart_falls_back_to_detached_on_error_5'
    • .venv/bin/python -m pytest -q tests/hermes_cli/test_gateway_service.py -k 'launchd_restart_self_request or wait_for_launchd_service_restart_timeout_reports_logs'
  3. Verify static checks:
    • ruff check hermes_cli/gateway.py tests/hermes_cli/test_gateway_service.py
    • git diff --check
  4. Optional broader check:
    • .venv/bin/python -m pytest -q tests/hermes_cli/test_gateway_service.py
    • On this macOS host it still ends at 182 passed, 6 failed; the 6 failures are the pre-existing user-systemd baseline failures, unrelated to this patch

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.3 / launchd

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

N/A

…h#56524)

Constraint: launchd in-band restarts can kill the foreground tool before a fallback kickstart runs\nRejected: return success immediately after SIGUSR1 | launchd may not have spawned a runtime-ready replacement yet\nConfidence: high\nScope-risk: moderate\nDirective: keep launchd self-restart, watcher fallback, and readiness waits on one shared recovery path\nTested: ./scripts/run_tests.sh tests/hermes_cli/test_gateway_service.py -k 'launchd_restart_drains_running_gateway_before_kickstart or launchd_restart_self_request or wait_for_launchd_service_restart_timeout_reports_logs or launchd_restart_boots_out_stale_registration_before_bootstrap or launchd_restart_falls_back_to_detached_on_error_5'\nTested: .venv/bin/python -m pytest -q tests/hermes_cli/test_gateway_service.py -k 'launchd_restart_self_request or wait_for_launchd_service_restart_timeout_reports_logs'\nTested: ruff check hermes_cli/gateway.py tests/hermes_cli/test_gateway_service.py\nTested: git diff --check\nNot-tested: full repo pytest remains blocked by six pre-existing user-systemd failures in tests/hermes_cli/test_gateway_service.py on this macOS host
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists labels Jul 2, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Fix PR for #56524, in the macOS launchd gateway-respawn family. Distinct mechanism from the sibling launchd fixes -- #42450 (drops -k from kickstart), #43181 (recovers an unloaded launchd job), #10317 (skips the redundant kickstart -k race). This one waits for the old PID to exit and for a fresh launchd-managed PID to reach running before reporting success. Cross-linking the family for maintainer review.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused launchd recovery work. The premise is confirmed on current main: the in-band branch reports success immediately after SIGUSR1 at hermes_cli/gateway.py:4327-4330, and the normal branch reports success immediately after launchctl kickstart -k at hermes_cli/gateway.py:4352-4354. Neither path currently verifies a fresh runtime-ready PID.

The proposed readiness predicate matches the established systemd contract in hermes_cli/gateway.py:1095-1124, and the PR adds coverage for the self-restart, forced fallback, fast relaunch, timeout, and unloaded-job paths. No blocking issue was identified in static review. GitHub reports the PR as mergeable clean with successful required CI checks.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
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 comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Telegram in-band hermes update can leave launchd gateway offline after drain timeout

3 participants