Skip to content

fix(gateway): make shutdown diagnostics and 2 tests work on macOS - #41958

Closed
3tty0n wants to merge 1 commit into
NousResearch:mainfrom
3tty0n:fix/macos-test-portability
Closed

fix(gateway): make shutdown diagnostics and 2 tests work on macOS#41958
3tty0n wants to merge 1 commit into
NousResearch:mainfrom
3tty0n:fix/macos-test-portability

Conversation

@3tty0n

@3tty0n 3tty0n commented Jun 8, 2026

Copy link
Copy Markdown

What does this PR do?

Makes the gateway's shutdown diagnostics work on macOS and fixes two tests that only passed on Linux. All three issues surface when running the suite on a darwin dev box; they're invisible on Linux/CI, so they had silently accumulated.

The one behavior change is real and user-facing: spawn_async_diagnostic() (the detached ps-style snapshot written when the gateway receives SIGTERM/SIGINT) hard-coded the GNU timeout binary. Stock macOS ships neither timeout nor gtimeout (the latter only arrives with Homebrew coreutils), so subprocess.Popen(["timeout", ...]) raised FileNotFoundError, was swallowed, and the function returned None — meaning macOS gateway operators got no shutdown diagnostics at all. The fix resolves timeout/gtimeout via shutil.which() and falls back to running bash directly when neither exists. The fallback loses the hard self-clean cap, but the snapshot script's commands are already individually bounded (head/tail) and the process is detached via start_new_session, so a best-effort snapshot beats no diagnostics.

The other two are test-robustness fixes (no production change):

  • test_media_files_routed_by_type compared send_voice/send_video/etc. paths against an un-resolved mkdtemp() path, but the media-delivery validator (validate_media_delivery_path) resolves symlinks before its denylist check. On macOS the tempdir lives under /var → /private/var, so the assertions mismatched. realpath()-ing the tempdir makes expected == actual on every platform.
  • test_gateway_stop_systemd_service_restart_exits_cleanly asserted exit code 0, but stop() deliberately keeps a non-zero (EX_TEMPFAIL) exit on darwin for launchd's KeepAlive. The test simulated systemd via INVOCATION_ID but never pinned sys.platform, so it failed on a macOS host. Pinning sys.platform="linux" mirrors the existing companion test test_gateway_stop_launchd_service_restart_keeps_nonzero_exit.

Related Issue

Related: #32681

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

  • gateway/shutdown_forensics.pyspawn_async_diagnostic() now resolves timeout/gtimeout via shutil.which() and falls back to bare bash when neither is on PATH (stock macOS). Added import shutil.
  • tests/gateway/test_background_command.pyrealpath() the media tempdir in test_media_files_routed_by_type so expected paths match the validator's symlink-resolved output (/var → /private/var on macOS).
  • tests/gateway/test_gateway_shutdown.py — pin sys.platform="linux" in test_gateway_stop_systemd_service_restart_exits_cleanly so the systemd clean-exit path is exercised regardless of host OS.

How to Test

  1. On macOS (or any host without GNU timeout on PATH):
    scripts/run_tests.sh tests/gateway/test_shutdown_forensics.py \
                         tests/gateway/test_background_command.py \
                         tests/gateway/test_gateway_shutdown.py
    All pass (66 tests). On main, three fail on macOS:
    • test_shutdown_forensics.py::TestSpawnAsyncDiagnostic::test_spawns_subprocess_and_writes_output (pid None)
    • test_background_command.py::TestRunBackgroundTask::test_media_files_routed_by_type (/var vs /private/var)
    • test_gateway_shutdown.py::test_gateway_stop_systemd_service_restart_exits_cleanly (exit 75 vs 0)
  2. Behavior check for the code fix: with no timeout/gtimeout on PATH, spawn_async_diagnostic(log, "SIGTERM") now returns a real pid and writes the === shutdown diagnostic @ SIGTERM === header to the log (previously returned None and wrote nothing).

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(gateway): ...)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix (no unrelated commits)
  • I've run the affected suites and they pass (scripts/run_tests.sh over the 3 files: 66 tests, all green)
  • I've added tests for my changes (the code fix is covered by the existing test_spawns_subprocess_and_writes_output, which fails on main/macOS and passes with this change)
  • I've tested on my platform: macOS (Darwin 25.5.0)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A (behavior fix; existing docstrings still accurate)
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A (no config keys)
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) — yes; this PR is the cross-platform fix (macOS), and the Windows skip path in spawn_async_diagnostic is unchanged
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

$ scripts/run_tests.sh tests/gateway/test_shutdown_forensics.py \
                       tests/gateway/test_background_command.py \
                       tests/gateway/test_gateway_shutdown.py
=== Summary: 3 files, 66 tests passed, 0 failed (100% complete) ===

Three macOS-portability fixes surfaced when running the suite on a
darwin dev box (all pass on Linux/CI, so they were previously invisible):

* spawn_async_diagnostic() hard-coded GNU `timeout`, which stock macOS
  doesn't ship (neither `timeout` nor `gtimeout` without Homebrew
  coreutils). Popen raised FileNotFoundError and the function returned
  None, so macOS gateways silently produced no shutdown snapshot.  Now
  resolve `timeout`/`gtimeout` via shutil.which() and fall back to bare
  `bash` when neither exists — the script's commands are already bounded
  (head/tail) and detached via start_new_session, so a best-effort
  snapshot without the hard cap beats no diagnostics at all.

* test_media_files_routed_by_type compared send_voice/video/etc. paths
  against an un-resolved mkdtemp() path, but the media-delivery validator
  (validate_media_delivery_path) resolves symlinks before its denylist
  check.  On macOS the tempdir is /var/... → /private/var/..., so the
  assertions mismatched.  realpath() the tempdir so expected == actual.

* test_gateway_stop_systemd_service_restart_exits_cleanly asserted a
  clean exit code 0, but stop() deliberately keeps a non-zero
  (EX_TEMPFAIL) exit on darwin for launchd KeepAlive.  The test simulated
  systemd via INVOCATION_ID but never pinned sys.platform, so it failed
  on a macOS host.  Pin sys.platform="linux", mirroring the companion
  test_gateway_stop_launchd_service_restart_keeps_nonzero_exit.

Verified via scripts/run_tests.sh: 3 files, 66 tests, all pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@3tty0n

3tty0n commented Jun 8, 2026

Copy link
Copy Markdown
Author

I close this PR because there is duplicated PRs: #13733, #31021, #32344

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant