diff --git a/tests/tui_gateway/test_entry_import_off_main_thread.py b/tests/tui_gateway/test_entry_import_off_main_thread.py new file mode 100644 index 0000000000000..478053a8e5ad7 --- /dev/null +++ b/tests/tui_gateway/test_entry_import_off_main_thread.py @@ -0,0 +1,85 @@ +"""Regression test: importing tui_gateway.entry off the main thread must not crash. + +Background +---------- +``entry.py`` installs signal handlers (SIGPIPE, SIGTERM, …) at *import time*. +``signal.signal`` is only legal in the main thread, so a first import of +``entry`` from a worker thread raises +``ValueError: signal only works in main thread of the main interpreter``. + +The Desktop/WebSocket agent-build path (``server._build``) runs in a daemon +thread and does ``from tui_gateway.entry import ensure_mcp_discovery_started`` +(server.py:1866). On that path ``entry.main()`` is never run, so the worker +thread performs the *first* import of ``entry`` — which crashed and aborted +MCP discovery startup (#72667, regression from the 2026-07-26 websocket-MCP +discovery fix). + +Fix: guard the import-time signal installs with a main-thread check. Handlers +are process-global, so installing them only when reached in the main thread is +sufficient; importing from a worker thread becomes a safe no-op. + +This test runs the import in a *fresh* subprocess worker thread to guarantee a +clean first-import (the test process already imported entry in its main +thread, so an in-process import would not reproduce the bug). +""" + +import subprocess +import sys + +REPO_ROOT = "." + + +def _spawn_worker_import_entry(): + """Run `import tui_gateway.entry` for the first time inside a worker thread. + + Returns (returncode, stdout, stderr). + """ + code = ( + "import threading, sys, signal, os\n" + "sys.stdout.reconfigure(line_buffering=True)\n" + "sys.stderr.reconfigure(line_buffering=True)\n" + "errs = []\n" + "def _worker():\n" + " try:\n" + " import tui_gateway.entry\n" + " except Exception as e:\n" + " errs.append(repr(e))\n" + "t = threading.Thread(target=_worker, daemon=True)\n" + "t.start(); t.join(timeout=15)\n" + "if errs:\n" + " sys.stdout.write('IMPORT_FAILED: ' + errs[0] + '\\n')\n" + " sys.exit(2)\n" + "# main thread of this process still installs SIGPIPE handler\n" + "h = signal.getsignal(signal.SIGPIPE)\n" + "sys.stdout.write('OK handler_installed=' + str(h is signal.SIG_IGN or callable(h)) + '\\n')\n" + "sys.exit(0)\n" + ) + proc = subprocess.run( + [sys.executable, "-c", code], + cwd=REPO_ROOT, + capture_output=True, + text=True, + timeout=60, + env={**__import__("os").environ}, + ) + return proc.returncode, proc.stdout, proc.stderr + + +def test_entry_imports_cleanly_from_worker_thread(): + """First import of tui_gateway.entry from a worker thread must succeed.""" + rc, out, err = _spawn_worker_import_entry() + # entry import may emit to stdout or stderr depending on the runner; check both. + combined = out + err + assert rc == 0, f"entry import off main thread failed (rc={rc}): {err!r}" + assert "OK" in combined, f"unexpected output: {out!r} / {err!r}" + + +def test_entry_installs_sigpipe_handler_in_main_thread(): + """Even though it can be imported off-thread, the main-thread path still + installs the SIGPIPE handler (process-global, so it applies everywhere).""" + rc, out, err = _spawn_worker_import_entry() + combined = out + err + assert rc == 0, f"entry import off main thread failed (rc={rc}): {err!r}" + assert "handler_installed=True" in combined, ( + f"SIGPIPE handler not installed: {out!r} / {err!r}" + ) diff --git a/tui_gateway/entry.py b/tui_gateway/entry.py index bf41c12018164..2b8ccee8838a0 100644 --- a/tui_gateway/entry.py +++ b/tui_gateway/entry.py @@ -13,6 +13,7 @@ hermes_bootstrap.harden_import_path() import json import logging import signal +import threading import time import traceback @@ -184,18 +185,49 @@ def _log_signal(signum: int, frame) -> None: # with hasattr so ``python -m tui_gateway.entry`` (spawned by # ``hermes --tui``) imports cleanly there. SIGBREAK (Windows' Ctrl+Break) # is installed when available as a weaker equivalent of SIGHUP. -if hasattr(signal, "SIGPIPE"): - signal.signal(signal.SIGPIPE, signal.SIG_IGN) -if hasattr(signal, "SIGTERM"): - signal.signal(signal.SIGTERM, _log_signal) +# +# signal.signal() is only legal in the MAIN thread. On the Desktop/WebSocket +# agent-build path, server._build() runs in a daemon thread and does +# ``from tui_gateway.entry import ensure_mcp_discovery_started`` as the first +# import of entry (entry.main() is never run there), which used to raise +# "ValueError: signal only works in main thread of the main interpreter" and +# abort MCP discovery startup. Install each handler only when we're in the +# main thread: handlers are process-global, so a main-thread import anywhere +# in the process still installs them for everyone, and an off-thread import +# (Desktop build path) simply no-ops instead of crashing the import. This +# preserves the original SIG_IGN/SIG_DFL behavior on the classic TUI/serve +# path while fixing the off-thread import crash. + + +def _install_signal(signame, handler): + """Install a signal handler if legal in this thread. + + signal.signal() raises ValueError outside the main thread; skip silently + there so a worker-thread import of this module (Desktop build path) does + not abort. On any main-thread import the handler is installed as before. + """ + if threading.current_thread() is not threading.main_thread(): + return + sig = getattr(signal, signame, None) + if sig is None: + return # Windows: SIGPIPE/SIGHUP absent + try: + signal.signal(sig, handler) + except (ValueError, OSError, RuntimeError): + # Not in the main thread despite the check, or handler rejected. + # Skip rather than crash the import (see above). + pass + + +_install_signal("SIGPIPE", signal.SIG_IGN) +_install_signal("SIGTERM", _log_signal) if hasattr(signal, "SIGHUP"): - signal.signal(signal.SIGHUP, _log_signal) + _install_signal("SIGHUP", _log_signal) elif hasattr(signal, "SIGBREAK"): # Windows-only: Ctrl+Break in a console window delivers SIGBREAK. # Route it through the same handler so kills are diagnosable. - signal.signal(signal.SIGBREAK, _log_signal) -if hasattr(signal, "SIGINT"): - signal.signal(signal.SIGINT, signal.SIG_IGN) + _install_signal("SIGBREAK", _log_signal) +_install_signal("SIGINT", signal.SIG_IGN) def _log_exit(reason: str) -> None: