Skip to content

fix(gateway): restrict ppid==1 systemd detection to Linux only - #25525

Open
zccyman wants to merge 1 commit into
NousResearch:mainfrom
atyou2happy:fix/shutdown-forensics-macos-ppid
Open

fix(gateway): restrict ppid==1 systemd detection to Linux only#25525
zccyman wants to merge 1 commit into
NousResearch:mainfrom
atyou2happy:fix/shutdown-forensics-macos-ppid

Conversation

@zccyman

@zccyman zccyman commented May 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #25508

When shutdown_forensics.py detects ppid == 1, it assumes the process runs under systemd. On macOS, PID 1 is launchd (Apple's service manager), not systemd. This causes the gateway to incorrectly use systemd-specific shutdown logic on macOS.

Root Cause

In gateway/shutdown_forensics.py line 143:

ctx["under_systemd"] = bool(invocation_id) or ppid == 1

The ppid == 1 check is platform-agnostic, but PID 1 only implies systemd on Linux.

Fix

Restrict the ppid == 1 heuristic to Linux only:

ctx["under_systemd"] = bool(invocation_id) or (sys.platform.startswith("linux") and ppid == 1)

The INVOCATION_ID environment variable check remains platform-independent (it's only set by systemd on Linux, so no false positive on macOS).

Test Plan

  • 3 new tests in tests/gateway/test_shutdown_forensics.py:
    • test_under_systemd_false_on_macos_when_ppid_is_one — macOS + ppid==1 → under_systemd=False
    • test_under_systemd_true_on_linux_when_ppid_is_one — Linux + ppid==1 → under_systemd=True
    • test_under_systemd_invocation_id_overrides_platformINVOCATION_ID always wins
  • All 33 tests in test_shutdown_forensics.py pass (30 existing + 3 new)

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have labels May 14, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related: #25508 (root issue, closed), #25510 (duplicate, closed), #25511 (competing PR, closed). This PR is the surviving fix for the ppid==1 false positive on macOS.

@wesleysimplicio

Copy link
Copy Markdown
Contributor

I just closed my own salvage of #25511 (#25687) in favor of this one. While reviewing the same area I rewrote the tests to use monkeypatch.setattr(sf.sys, "platform", "darwin") / monkeypatch.setattr(sf.os, "getppid", lambda: 1) so they actually exercise the runtime expression instead of asserting against literal-string expressions. If you'd like, I can push those two test cases as a follow-up branch off your PR, or you're welcome to copy them — happy to help land this.

@zccyman

zccyman commented May 16, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @wesleysimplicio for closing #25687 in favor of this one — that's a strong endorsement, much appreciated.

Your test rewrite suggestion is spot-on — monkeypatch.setattr(sf.sys, "platform", "darwin") / monkeypatch.setattr(sf.os, "getppid", lambda: 1) is far more robust than string-based assertions because it exercises the actual runtime expression. I'll update the tests accordingly. Pushing soon.

@zccyman

zccyman commented May 16, 2026

Copy link
Copy Markdown
Contributor Author

@wesleysimplicio Good news — the current patch already includes three monkeypatch.setattr-based tests that exercise the runtime expression directly:

  1. test_under_systemd_false_on_macos_when_ppid_is_onemonkeypatch.setattr(sf.sys, "platform", "darwin") + getppid=lambda: 1
  2. test_under_systemd_true_on_linux_when_ppid_is_onemonkeypatch.setattr(sf.sys, "platform", "linux") + getppid=lambda: 1
  3. test_under_systemd_invocation_id_overrides_platform — darwin + INVOCATION_ID=abc123

Exactly the pattern you described. Let me know if you see anything else.

@zccyman
zccyman force-pushed the fix/shutdown-forensics-macos-ppid branch from 0e929da to a04e6b7 Compare May 18, 2026 00:29
jasonjcwu pushed a commit to jasonjcwu/hermes-agent that referenced this pull request May 19, 2026
When the advisor tool is removed from the tools array (e.g. disabled for
cost control on follow-up turns), Anthropic rejects requests whose history
contains residual server_tool_use(name=advisor) or advisor_tool_result blocks.

Added _strip_advisor_blocks() which replaces these blocks with
<advisor_feedback> text blocks so the executor retains the semantic
context of past advice. Called in build_anthropic_kwargs() when advisor
is not active.

Inspired by LiteLLM's strip_advisor_blocks_from_messages() (PR NousResearch#25525).
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused platform correction. Current main still has the unguarded PPID-1 heuristic at gateway/shutdown_forensics.py:143, and this diff directly scopes that heuristic to Linux while retaining INVOCATION_ID behavior. The added darwin/Linux/INVOCATION_ID tests exercise the runtime expression rather than a string representation. The two target files have not changed since the PR base, so this is mechanically salvageable.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 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 P3 Low — cosmetic, nice to have 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-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: shutdown_forensics incorrectly sets under_systemd=True on macOS (ppid==1 is launchd, not systemd)

4 participants