Skip to content

fix(gateway): remove -k flag from kickstart in launchd_restart to prevent double-spawn - #42450

Open
liuhao1024 wants to merge 2 commits into
NousResearch:mainfrom
liuhao1024:fix/gateway-kickstart-double-start
Open

fix(gateway): remove -k flag from kickstart in launchd_restart to prevent double-spawn#42450
liuhao1024 wants to merge 2 commits into
NousResearch:mainfrom
liuhao1024:fix/gateway-kickstart-double-start

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes launchd_restart() to use plain kickstart (without -k flag), preventing a double-spawn race condition on macOS 26+ when KeepAlive=true is set in the LaunchAgent plist.

Related Issue

Fixes #42446

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Root Cause

In launchd_restart() (hermes_cli/gateway.py), the code already performs graceful termination:

  1. terminate_pid() sends SIGTERM to the gateway
  2. _wait_for_gateway_exit() waits for the process to exit

Then kickstart -k is called, but -k means "terminate if running, then start." This races with KeepAlive=true:

  1. SIGTERM sent → gateway begins shutdown
  2. kickstart -k detects running process → terminates again
  3. KeepAlive=true detects termination → respawns
  4. kickstart also starts a new instance
  5. Result: two concurrent gateway processes

Changes Made

  • hermes_cli/gateway.py: Removed -k flag from kickstart in launchd_restart() (line 3540), since the preceding code already handles termination. This matches the pattern used by all other kickstart calls in the codebase (launchd_start paths and the fallback path in the same function).
  • tests/cli/test_launchd_restart_kickstart.py: Added 4 regression tests verifying the kickstart command does not include -k, that termination happens before kickstart, and that self-restart (SIGUSR1) skips kickstart entirely.

Testing

$ python -m pytest tests/cli/test_launchd_restart_kickstart.py -v
tests/cli/test_launchd_restart_kickstart.py::test_kickstart_uses_no_kill_flag PASSED
tests/cli/test_launchd_restart_kickstart.py::test_kickstart_no_kill_flag_when_pid_absent PASSED
tests/cli/test_launchd_restart_kickstart.py::test_terminate_called_before_kickstart PASSED
tests/cli/test_launchd_restart_kickstart.py::test_self_restart_skips_kickstart PASSED

4 passed in 0.13s

Checklist

  • I have read the contribution guidelines
  • My code follows the project's coding standards
  • I have added tests that prove my fix is effective
  • All new and existing tests passed

…vent double-spawn

The -k flag in launchctl kickstart re-terminates the service and then
starts it. Since launchd_restart() already sends SIGTERM and waits for
the gateway to exit, the -k flag races with KeepAlive=true:

1. terminate_pid() sends SIGTERM → gateway begins shutdown
2. kickstart -k detects running process → terminates again
3. KeepAlive=true detects termination → respawns
4. kickstart also starts a new instance
5. Result: two concurrent gateway processes

All other kickstart calls in the codebase (launchd_start paths at
L3390, L3405, L3421, and the fallback at L3560) already use plain
kickstart without -k.

Fixes NousResearch#42446
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery labels Jun 9, 2026
The -k flag was removed from kickstart in the production code to prevent
double-spawn, but two tests still asserted on the old command with -k.
Update both tests to match the new kickstart behavior.

@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 the focused macOS report and regression coverage.

Problems

  • launchd_restart() deliberately retains kickstart -k for the failed-drain branch: current main logs a forced restart when _wait_for_gateway_exit() returns false at hermes_cli/gateway.py:4347-4351, then runs kickstart -k at hermes_cli/gateway.py:4352. Making the command plain unconditionally removes that fallback.
  • The added tests do not exercise _wait_for_gateway_exit(...)=False; test_terminate_called_before_kickstart verifies calls occurred, not their ordering or the timeout behavior.

Suggested changes

  • Split the command choice by the drain result: preserve a hard restart after timeout, and cover both branches in tests/hermes_cli/test_gateway_service.py's existing ordered-call tests.

Automated hermes-sweeper review.

Comment thread hermes_cli/gateway.py
f"⚠ Gateway drain timed out after {drain_timeout:.0f}s — forcing launchd restart"
)
subprocess.run(["launchctl", "kickstart", "-k", target], check=True, timeout=90)
# Use plain kickstart (no -k) — the SIGTERM + wait above already

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.

Please keep a forced-restart branch when _wait_for_gateway_exit() returns False. Current main explicitly logs that the drain timed out and then uses kickstart -k to force the restart; making this plain unconditionally removes that fallback.

@teknium1 teknium1 added 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 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hermes gateway restart triggers double gateway start on macOS 26+ due to launchctl kickstart -k racing with KeepAlive

3 participants