Skip to content

fix: make shutdown diagnostics portable without coreutils timeout - #64515

Open
faisfamilytravel wants to merge 1 commit into
NousResearch:mainfrom
faisfamilytravel:xo/fix-portable-shutdown-diagnostics
Open

fix: make shutdown diagnostics portable without coreutils timeout#64515
faisfamilytravel wants to merge 1 commit into
NousResearch:mainfrom
faisfamilytravel:xo/fix-portable-shutdown-diagnostics

Conversation

@faisfamilytravel

Copy link
Copy Markdown
Contributor

Problem

gateway.shutdown_forensics.spawn_async_diagnostic() launches diagnostics through the GNU timeout executable. macOS does not ship timeout or gtimeout, so the supported macOS path catches FileNotFoundError, returns None, and silently produces no shutdown diagnostic.

Fix

Run the detached diagnostic through the current Python interpreter. The wrapper:

  • starts the Bash diagnostic in its own process group;
  • enforces the configured timeout with Popen.wait(timeout=...);
  • kills the diagnostic process group on timeout;
  • preserves the existing non-blocking caller behavior and output file contract;
  • removes the GNU coreutils dependency.

Verification

  • Reproduced on macOS with neither timeout nor gtimeout installed.
  • 31 passed in tests/gateway/test_shutdown_forensics.py.
  • Ruff passed for the changed source and tests.
  • Added a regression test asserting the launcher uses sys.executable and carries an internal TimeoutExpired path instead of invoking coreutils.

@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 Jul 14, 2026
@faisfamilytravel
faisfamilytravel force-pushed the xo/fix-portable-shutdown-diagnostics branch from c304a98 to 0b1cf9a Compare July 14, 2026 16:43
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused portability fix. The premise is confirmed on current main: gateway/shutdown_forensics.py:257 invokes GNU timeout, and gateway/shutdown_forensics.py:265-270 converts its absence into a silent None result. The proposed sys.executable wrapper in PR commit 0b1cf9ac267e retains detached, non-blocking launch behavior while moving timeout enforcement into the stdlib.

The PR base is an ancestor of the inspected HEAD and GitHub reports it mergeable; the targeted current-main code remains unchanged. The existing real subprocess/output test plus the new launcher regression cover the relevant contract.

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 16, 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-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.

3 participants