Skip to content

fix(gateway): honor systemd unit scope in shutdown check - #61017

Open
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix/61003-systemd-timeout-scope
Open

fix(gateway): honor systemd unit scope in shutdown check#61017
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix/61003-systemd-timeout-scope

Conversation

@LeonSGP43

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a false-positive stale-systemd-unit warning in shutdown forensics. For system-scope gateway services, Hermes was querying systemctl --user show first; systemd returns the default TimeoutStopUSec=1min 30s for unknown user units, so the code could warn on every startup even when the real system unit had sufficient stop time configured.

Related Issue

Fixes #61003

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

  • Record the cgroup path while discovering the current .service unit.
  • Use the cgroup owner slice to choose the correct systemctl manager: system scope for /system.slice/..., user scope for /user.slice/....
  • Keep a user-then-system fallback only when the cgroup path does not clearly identify the manager.
  • Add regression tests for both system-scope and user-scope units in tests/gateway/test_shutdown_forensics.py.

How to Test

  1. uv run pytest tests/gateway/test_shutdown_forensics.py -k CheckSystemdTimingAlignment -q
  2. uv run python -m py_compile gateway/shutdown_forensics.py tests/gateway/test_shutdown_forensics.py
  3. git diff --check

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 local toolchain via uv / Python 3.11

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

  • Local proof: the new system-scope and user-scope alignment tests pass, syntax check passes, and git diff --check is clean.
  • Note: the broader tests/gateway/test_shutdown_forensics.py -q file still has one unrelated preexisting failure in TestSpawnAsyncDiagnostic::test_spawns_subprocess_and_writes_output on this machine; the alignment-specific slice passes.

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Jul 8, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #54396 (earliest open PR using the same fix). Both parse /proc/self/cgroup and select the systemctl manager scope from the cgroup owner slice (/system.slice -> system, /user.slice -> --user) to fix the false-positive check_systemd_timing_alignment warning. Related cluster: #34734/#36766/#37324/#58735 use the alternate LoadState-gate mechanism; #61003 is the issue this fixes. Maintainer to pick the canonical fix from the cgroup-scope pair (#54396 / #61017).

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression fix. Current main still queries systemctl --user before system scope and stops at the first parseable timeout in gateway/shutdown_forensics.py:367-389; that is the faulty path documented in #61003. The cgroup-slice routing in this PR addresses that path while preserving the existing fallback for unrecognized cgroups, and the added tests cover both /system.slice and /user.slice command selection.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 10, 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 duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

shutdown_forensics: false-positive 'Stale systemd unit' warning when unit exists in system scope only

3 participants