fix(gateway): make shutdown diagnostics cross-platform - #64215
Conversation
…s-platform Two test suites failed on macOS despite passing on Linux: * tests/gateway/test_shutdown_forensics.py::test_spawns_subprocess_and_writes_output spawn_async_diagnostic invoked GNU coreutils 'timeout', which is not present on macOS (and not on stock Git Bash). The Popen immediately raised FileNotFoundError and the function returned None. Replace the external 'timeout' wrapper with an in-script watchdog: a backgrounded subshell that sleeps for timeout_seconds then SIGKILLs the parent shell. A foreground 'trap ... EXIT' cancels the watchdog on clean exit so fast runs don't pay the sleep. Net contract matches the prior GNU-timeout behaviour. Also drop the GNU-only 'ps --sort=-pcpu' and 'cat /proc/loadavg' for BSD-equivalent variants (sysctl -n vm.loadavg) so the script runs on macOS too. * tests/hermes_cli/test_signal_handler_kanban_worker.py::test_sigterm_with_kanban_task_env_terminates_quickly _is_alive_like_dispatcher only inspected /proc/<pid>/status on Linux. On macOS a SIGTERM'd + os._exit'd worker leaves a zombie whose PID still answers os.kill(pid, 0) successfully, so the polling loop never observed a transition to dead and the test timed out after 2s. Mirror the production _pid_alive helper in hermes_cli/kanban_db.py: keep the /proc-based check on Linux and add a BSD-style 'ps -o stat=' probe on darwin that treats any 'Z' in the stat field as dead. Both branches use a tight timeout so a wedged ps can't stall the polling loop. Verification: pytest -q tests/gateway/test_shutdown_forensics.py tests/hermes_cli/test_signal_handler_kanban_worker.py — 33 passed, 10 consecutive runs, no flakes. ruff check clean for both files.
There was a problem hiding this comment.
Pull request overview
This PR improves gateway shutdown diagnostics and related tests to work on macOS by removing GNU/Linux-specific dependencies (timeout, /proc/loadavg, and GNU ps --sort) and adding portable fallbacks. This helps ensure shutdown forensics actually run and that the kanban worker “alive” detection test treats zombie processes as dead on macOS.
Changes:
- Replace GNU
timeoutusage in shutdown diagnostics with an inline bash watchdog that self-terminates aftertimeout_seconds. - Remove GNU
ps --sortusage and add a macOS-compatible load average fallback (sysctl -n vm.loadavg). - Update the kanban worker signal-handler test helper to detect zombie state on macOS via
ps -o stat=.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
gateway/shutdown_forensics.py |
Makes the async shutdown diagnostic script self-timeout and adds macOS-compatible loadavg collection. |
tests/hermes_cli/test_signal_handler_kanban_worker.py |
Extends the test’s “pid alive” helper to treat macOS zombie processes as dead. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "echo '--- ps auxf (top 60 by cpu) ---'; " | ||
| "ps auxf --sort=-pcpu 2>/dev/null | head -60; " | ||
| "ps auxf 2>/dev/null | head -60; " |
alt-glitch
added
type/bug
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused portability fix. The current-head premise is still present: gateway/shutdown_forensics.py:258 invokes GNU timeout, and the PR target files have not changed since base 226e8de827a669e8ffa7035b27d70c19e44b1208.
Problems
gateway/shutdown_forensics.py:248now runs unsortedps auxf, but the preceding diagnostic heading still saystop 60 by cpu. This makes the captured shutdown log inaccurate. This matches the existing Copilot inline observation.
Suggested changes
- Rename the heading to reflect unsorted output (for example,
first 60), or add a genuinely portable CPU-ordering path with a fallback.
The Darwin zombie probe in the test helper aligns with current production handling at hermes_cli/kanban_db.py:6119-6133.
Automated hermes-sweeper review.
| "echo '--- date ---'; date -u +%Y-%m-%dT%H:%M:%SZ; " | ||
| "echo '--- ps auxf (top 60 by cpu) ---'; " | ||
| "ps auxf --sort=-pcpu 2>/dev/null | head -60; " | ||
| "ps auxf 2>/dev/null | head -60; " |
There was a problem hiding this comment.
The preceding heading still says top 60 by cpu, but this fallback is no longer CPU-sorted. Please rename it to describe the actual unsorted output (for example, first 60) or restore a portable ordering path.
teknium1
added
the
sweeper:blast-contained
Summary
timeoutdependency in shutdown diagnostics with a self-contained shell watchdogps --sortand add a macOS load-average fallbackpsWhy
Stock macOS does not provide GNU
timeout,/proc/loadavg, or GNUps --sort. The diagnostic subprocess therefore failed before collecting anything, while zombie workers could be misclassified as alive by the test helper.Verification
scripts/run_tests.sh tests/gateway/test_shutdown_forensics.py tests/hermes_cli/test_signal_handler_kanban_worker.pyruff checkpassedgit diff --checkpassed