fix(gateway): report launchd supervision accurately - #67744
Conversation
35e3afd to
9a3b580
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused gateway diagnostics fix. The two reported premises remain on current main: hermes_cli/gateway.py:7263-7266 reports a discovered PID as manual when no generated plist exists, and gateway/shutdown_forensics.py:134-143 treats any PPID-1 process as systemd.
Problems
hermes_cli/gateway.py:1314addssubprocess.run(..., text=True)without an explicit encoding.scripts/check-windows-footguns.py:328-348flags that pattern, and commitc89481db5estandardizedencoding='utf-8', errors='replace'for production text-mode subprocess calls.
Suggested changes
- Add
encoding="utf-8", errors="replace"to the new launchctl probe, matching the adjacent launchctl call athermes_cli/gateway.py:1335-1340on the PR head.
Automated hermes-sweeper review.
| result = subprocess.run( | ||
| ["launchctl", "list"], | ||
| capture_output=True, | ||
| text=True, |
There was a problem hiding this comment.
Please add encoding="utf-8", errors="replace" here. scripts/check-windows-footguns.py:328-348 flags subprocess calls using text=True without encoding=, and this would regress the production-wide UTF-8 policy from c89481db5e.
Match the text-mode subprocess convention standardized in c89481d (encoding='utf-8', errors='replace') flagged by check-windows-footguns. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed in 9e0a5ca — the new |
SummaryTwenty-three PRs address or reference four shutdown-forensics clusters: macOS supervisor detection, systemd manager-scope timeout false positives, Windows planned-stop labels, and portable shutdown diagnostics; several also bundle unrelated voice, installer, auxiliary-client, or WhatsApp changes. The focused diffs either constrain or replace the PPID-1 heuristic, select or validate the owning systemd manager, relabel marker-driven stops, or remove the GNU Related pull requests
Duplicates#25511, #25566, and #25687 duplicate #25525; the shutdown-forensics portion of #25570 also overlaps #25525. #36766, #37324, #57901, and #72943 share the LoadState-gate mechanism, while #61017 and #61532 duplicate #54396's cgroup-scope mechanism; #64029 combines both mechanisms. #71617 duplicates #67744. #64515 and #74011 overlap on the Python timeout supervisor, while #74011 additionally adds Darwin-native diagnostics; #67708 is only a test-side shim for that same portability problem. Suggested consolidationAuthor action on #67744: rebase onto main or split out the uniquely salvageable external-label launchd status detection and supervisor taxonomy, retain the now-addressed UTF-8 subprocess fix, and avoid duplicating #25525, the recorded best existing fix for the exact macOS PPID-1 issue. Keep the distinct recorded best-fix paths visible—#37324 for the strict LoadState fallback, #54396 for cgroup-scope selection, #64029 for their combined hardening, and #61615 for Windows planned-stop labels—while closing or leaving closed their explicit duplicates; coordinate #64515/#74011 around one portable supervisor, preserving #74011's Darwin-native diagnostics. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I25508(["issue #25508 (closed)"])
I25510(["issue #25510 (closed)"])
subgraph Dup67744 ["PRs duplicating each other"]
P67744["PR #67744 (open)"]
P71617["PR #71617 (closed)"]
end
P67744 -.->|partial| I25508
P67744 -.->|partial| I25510
class I25508 closed
class I25510 closed
class P67744 open
class P71617 closed
class P67744 target
click I25508 "https://github.com/NousResearch/hermes-agent/issues/25508"
click I25510 "https://github.com/NousResearch/hermes-agent/issues/25510"
click P67744 "https://github.com/NousResearch/hermes-agent/pull/67744"
click P71617 "https://github.com/NousResearch/hermes-agent/pull/71617"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 23 pull requests and 6 issues in this complex. Diffs were read for 20 of 23 PRs (rest unavailable); Assessment working set: 132 kB of PR diffs, 62 kB of issue/PR text, 29 kB of discussion (45 comments), 65 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
|
Rebase-on-behalf delivery — this PR's branch was rebased onto current main ( Rebased branch: Conflict resolution (3 files):
To adopt (author): py_compile verified; test-coverage superset verified (no main regressions). This unblocks the gateway R1 extraction window (hunk gate). |
What does this PR do?
Fixes two misleading macOS gateway diagnostics:
hermes gateway statusreported a live gateway as “Running manually” when it was actually owned by a system LaunchDaemon using an operator-defined label such ascom.example.hermes-gateway.under_systemd=yes, even though PID 1 is launchd on macOS and may be another init/supervisor on Linux.The status path now correlates discovered gateway PIDs with
launchctl list, so externally managed launchd jobs are reported truthfully without suggesting a duplicate service install. Shutdown logs now emitsupervisor=systemd|launchd|pid1|none; the legacy structuredunder_systemdfield remains but is true only when explicit systemd markers are present.Related Issue
No issue filed. I searched open issues and PRs for
launchd status manual system service,under_systemd launchd, and the exact"Running manually" launchdsymptom; no duplicate was found.Related but non-duplicative: #65177 changes restart lifecycle ownership. It does not address external launchd labels or shutdown supervisor labelling.
Type of Change
Changes Made
hermes_cli/gateway.pygateway/shutdown_forensics.pysupervisor=...instead of the misleadingunder_systemd=yes|nolog tokenHow to Test
pytest tests/hermes_cli/test_gateway_service.py::TestLaunchdServiceRecovery \ tests/hermes_cli/test_gateway_service.py::TestGatewaySystemServiceRouting::test_gateway_status_reports_external_launchd_supervisor \ tests/gateway/test_shutdown_forensics.py -o 'addopts=' -qhermes gateway statusfrom a shell.hermes gateway install.supervisor=launchd, notunder_systemd=yes.Verification performed after rebasing onto current upstream
main:git diff --check: passedChecklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass — focused and affected-file suites were run; full repository suite was not run locallyDocumentation & Housekeeping
docs/, docstrings) — docstrings updated; no user-facing docs change requiredcli-config.yaml.exampleif I added/changed config keys — N/A, no config changesCONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/A, no architecture/workflow changeScreenshots / Logs
Observed before:
Observed after:
Shutdown formatter after: