Skip to content

fix(gateway): classify planned service restarts - #57419

Open
dorukardahan wants to merge 3 commits into
NousResearch:mainfrom
dorukardahan:doruk/planned-service-restart-marker-20260703
Open

dorukardahan wants to merge 3 commits into
NousResearch:mainfrom
dorukardahan:doruk/planned-service-restart-marker-20260703

Conversation

@dorukardahan

@dorukardahan dorukardahan commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Classifies service-managed gateway restarts as planned service restarts instead of unexpected external kills or --replace takeovers:

  • adds a dedicated .gateway-planned-service-restart.json marker
  • validates marker PID + process start time to avoid PID-reuse false positives
  • consumes/clears stale or matched markers so they cannot affect later unrelated signals
  • surfaces the marker in shutdown forensics
  • preserves takeover marker semantics for actual --replace flows

Root cause

v0.18.0 has restart/takeover markers, but a normal service-manager restart sends the same SIGTERM shape as an unexpected external kill. Reusing the takeover marker makes operator logs misleading; having no marker misclassifies planned service restarts.

Related open issues

Related merged precedent

Scope / risk

  • Best-effort marker only; restart proceeds even if the marker cannot be written.
  • Marker TTL and process start-time check bound stale/PID-reuse risk.
  • Does not change adapter startup/shutdown ordering.

Verification

  • python -m pytest tests/gateway/test_status.py
  • python -m py_compile gateway/status.py gateway/run.py gateway/shutdown_forensics.py tests/gateway/test_status.py
  • git diff --check
  • GitHub: all required checks pass on current head 878298a27f59

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 3, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the careful PID/start-time and stale-marker handling. The current patch is incomplete, however.

Problems

  • The new marker is never written by production code. gateway/status.py adds write_planned_service_restart_marker(), and gateway/run.py consumes it, but gh pr diff 57419 contains no call to the writer. The planned-service-restart branch therefore cannot run.
  • The added tests cover helper serialization/consumption only; they do not exercise a restart initiator and the signal-handler classification together.
  • Current main has changed restart mechanics: hermes_cli/gateway.py:3299-3356 first attempts SIGUSR1/exit-75 restart handling and falls back to systemctl restart. The salvage needs to integrate with that live flow rather than only adding a signal-handler marker consumer.

Suggested changes

  • Write the marker immediately before the applicable current SIGTERM/service-restart operation, using its target PID.
  • Add an end-to-end unit-level test for writer → signal handler classification, including the current fallback path.

Automated hermes-sweeper review.

Comment thread gateway/run.py
@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state 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
# Conflicts:
#	gateway/status.py
#	tests/gateway/test_status.py
@dorukardahan

Copy link
Copy Markdown
Contributor Author

Refreshed onto current main at 3be565fbdee3115ab5b9338551768b8e5e655c56 and closed the missing producer path from the earlier review.

The planned-service-restart marker is now written immediately before the existing forced service-restart operations:

  • systemd fallback after the graceful SIGUSR1 restart does not complete
  • launchd fallback before terminating the running gateway

The focused integration test exercises the real systemd initiator order and verifies that the marker consumer classifies the target as planned before systemctl restart; the launchd test asserts marker-before-SIGTERM ordering.

Validation on candidate 618dd7c185126423abe7b0ca9c49d217f6ad4801:

  • 195 passed across test_status.py, test_shutdown_forensics.py, test_gateway_shutdown.py, and test_gateway_service.py
  • new focused integration tests: 2 passed
  • Ruff, py_compile, and PR-delta git diff --check: clean

# Conflicts:
#	tests/hermes_cli/test_gateway_service.py

This branch has not been deployed

No deployments
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 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.

3 participants