From e6bf5f7122bc8f28e1df9ef64c17d9e90eac361b Mon Sep 17 00:00:00 2001 From: garnet Date: Sun, 24 May 2026 11:46:32 -0500 Subject: [PATCH] fix(cli): stop main() leaking SIGPIPE=SIG_DFL into the process (sy-6zq, sy-1n1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit main() installed SIGPIPE=SIG_DFL (so piped CLI output ends silently like a normal Unix tool) but never restored it. The disposition is process-global, and main() is imported and called directly by ~10 tests plus library callers — so SIG_DFL leaked into the pytest process. A later unrelated broken-pipe write during output capture or coverage teardown was then delivered as SIGPIPE, silently killing the runner with exit 141. Which CI matrix entry died was non-deterministic (depends on test ordering), which is why the flake appeared to wander across Python 3.11/3.12/3.14 and blocked the merge queue. Fix has two layers: - main() snapshots the prior SIGPIPE handler (signal.getsignal) and restores it in the finally block. CLI piping behavior during the run is unchanged; the global disposition simply no longer outlives the invocation. - conftest.py adds an autouse fixture that restores SIGPIPE around every test, making this class of leak structurally impossible regardless of which code paths a test exercises (mirrors the existing network-block fixture). isatty() gating was rejected: stdout is not a tty exactly when piped to head, which is the case the SIG_DFL restore exists to handle. Verified: full suite 3039 passed (no exit 141, 87% cov); broken-pipe + shim tests 14 passed; CLI piping stays silent and exits 0; ruff clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/synth_panel/main.py | 25 ++++++++++++++++++++++++- tests/conftest.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/synth_panel/main.py b/src/synth_panel/main.py index 01c3173..7fc15a8 100644 --- a/src/synth_panel/main.py +++ b/src/synth_panel/main.py @@ -90,7 +90,22 @@ def _quiet_broken_pipe() -> None: def main(argv: list[str] | None = None) -> int: """CLI entry point. Returns exit code.""" - if hasattr(signal, "SIGPIPE"): + # SIGPIPE disposition is process-global. ``main`` is the console-script + # entry point, but it is *also* imported and called directly by the test + # suite and library callers — so installing SIG_DFL unconditionally and + # never undoing it leaks the disposition into the calling process. A later, + # unrelated broken-pipe write (pytest output capture, coverage teardown, an + # xdist worker pipe) then takes a SIGPIPE and the whole process dies + # silently with exit 141. That is the non-deterministic CI killer tracked + # under sy-6zq / sy-1n1. Capture the prior handler so we can hand the + # disposition back when ``main`` returns. (Gating on ``isatty()`` would be + # wrong: stdout is *not* a tty exactly when piped to ``head``, which is the + # case the SIG_DFL restore exists to handle.) + prev_sigpipe = None + have_sigpipe = hasattr(signal, "SIGPIPE") + if have_sigpipe: + # Windows has no SIGPIPE — that's why the hasattr guard. + prev_sigpipe = signal.getsignal(signal.SIGPIPE) signal.signal(signal.SIGPIPE, signal.SIG_DFL) try: return _main(argv) @@ -109,6 +124,14 @@ def main(argv: list[str] | None = None) -> int: # before the interpreter's own shutdown flush triggers the # "Exception ignored" warning on stderr. _quiet_broken_pipe() + # Restore the caller's SIGPIPE handler. For the real CLI process this + # is harmless cleanup right before exit (stdout is already pointed at + # /dev/null by _quiet_broken_pipe); for in-process callers it stops + # SIG_DFL from outliving this invocation. ``getsignal`` returns None + # when the prior handler was installed from non-Python code, which + # ``signal.signal`` cannot reinstall — skip that rare case. + if have_sigpipe and prev_sigpipe is not None: + signal.signal(signal.SIGPIPE, prev_sigpipe) def _main(argv: list[str] | None) -> int: diff --git a/tests/conftest.py b/tests/conftest.py index a9d14bb..ca5b117 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -10,6 +10,7 @@ class of bugs structurally impossible in CI. from __future__ import annotations +import signal import socket from pathlib import Path @@ -55,3 +56,35 @@ def _isolate_credentials_store(tmp_path_factory: pytest.TempPathFactory, monkeyp """ sandbox: Path = tmp_path_factory.mktemp("synthpanel-creds") monkeypatch.setenv("SYNTHPANEL_CREDENTIALS_PATH", str(sandbox / "credentials.json")) + + +@pytest.fixture(autouse=True) +def _restore_sigpipe_disposition(): + """Snapshot and restore the process-global SIGPIPE handler around each test. + + ``synth_panel.main.main`` and its ``_quiet_broken_pipe`` helper deliberately + install ``SIGPIPE=SIG_DFL`` so piped CLI output (``synthpanel … | head``) + ends silently like a normal Unix tool. The disposition is *process-global*, + so a test that calls ``main()`` or ``_quiet_broken_pipe()`` can leave + SIG_DFL installed for the remainder of the pytest session. After that, any + broken-pipe write during pytest's output capture or coverage teardown is + delivered as SIGPIPE and silently kills the runner with exit 141 — the + non-deterministic CI failure tracked under sy-1n1 / sy-6zq (the matrix + entry that dies varies run to run because it depends on test ordering). + + Restoring the prior handler after every test makes that leak structurally + impossible regardless of which code paths a test exercises, in the same + spirit as the network-block fixture above. + """ + if not hasattr(signal, "SIGPIPE"): + # Windows has no SIGPIPE; nothing to guard. + yield + return + prev = signal.getsignal(signal.SIGPIPE) + try: + yield + finally: + # ``getsignal`` returns None when the handler was installed from + # non-Python code; ``signal.signal`` cannot reinstall that, so skip it. + if prev is not None: + signal.signal(signal.SIGPIPE, prev)