From d4b772cbda8a3e9147dbd59ce21b76d5152f585e Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 20 Sep 2026 14:34:13 -0700 Subject: [PATCH] fix(tools): search_files keeps its matches when killpg is refused On macOS a search that reaches its `limit` takes the early-stop branch that TERMs rg's process group; rg has often already exited, and macOS answers killpg on a zombie-only group with EPERM instead of ESRCH. The PermissionError escaped _kill_process_group_posix, unwound to search_tool and the drained matches were replaced by `{"error": "[Errno 1] Operation not permitted"}` (#116855, same symptom as #104696). The helper now treats EPERM like "nothing left to signal" and falls back to killing the known PIDs so a live child cannot escape. It also never killpg's the caller's own group (#107029): a child that shares our pgid is torn down by PID. Diagnosis credit: #116949 (@liuhao1024), redone slim. Co-authored-by: liuhao1024 --- tests/tools/test_search_native_rg.py | 13 +++++++++++++ tools/environments/local.py | 14 ++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/tests/tools/test_search_native_rg.py b/tests/tools/test_search_native_rg.py index de84ee985d..2ba2b36cea 100644 --- a/tests/tools/test_search_native_rg.py +++ b/tests/tools/test_search_native_rg.py @@ -112,3 +112,16 @@ def test_kill_switch_routes_search_back_to_the_shell(tree, ops_factory, monkeypa assert result.total_count == 4 assert any(c.startswith("test -e") for c in calls) assert any("pipefail" in c and "rg" in c for c in calls) + + +def test_limit_hit_keeps_drained_matches_when_group_kill_is_refused(tree, ops_factory, monkeypatch): + """Reaching ``limit`` takes the early-stop branch that TERMs rg's group. macOS answers + ``killpg`` with EPERM (not ESRCH) for a zombie-only group; that must not surface as a tool + error nor discard the matches already drained (#116855).""" + import os + + monkeypatch.setenv("HERMES_NATIVE_FILE_READ", "1") + ops = ops_factory(tree, []) + monkeypatch.setattr(os, "killpg", lambda pgid, sig: (_ for _ in ()).throw(PermissionError(1, "Operation not permitted"))) + result = ops.search(pattern="needle", path=str(tree), limit=2) + assert not result.error and len(result.matches) == 2, result.to_dict() diff --git a/tools/environments/local.py b/tools/environments/local.py index 894af95a1f..d944f66b75 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -1,6 +1,7 @@ """Local execution environment — spawn-per-call with session snapshot.""" import contextlib +import errno import logging import ntpath import os @@ -792,6 +793,10 @@ def _kill_process_group_posix(proc) -> None: except Exception: descendants = [] try: + if pgid == os.getpgrp(): + # The child shares OUR group (a spawner that skipped setsid): killpg would + # signal the caller itself — the gateway on Darwin (#107029). Tear down by PID. + raise PermissionError(errno.EPERM, "child shares the caller's process group") os.killpg(pgid, signal.SIGTERM) # windows-footgun: ok — POSIX only (see _IS_WINDOWS gate in caller) if not _wait_for_group_exit(proc, pgid, 1.0): os.killpg(pgid, signal.SIGKILL) # windows-footgun: ok — POSIX only (see _IS_WINDOWS gate in caller) @@ -800,6 +805,15 @@ def _kill_process_group_posix(proc) -> None: proc.wait(timeout=0.2) except ProcessLookupError: pass + except PermissionError: + # macOS answers killpg with EPERM (not ESRCH) once the group's only members are + # unreaped zombies — rg exiting between the caller's poll() and the TERM after the + # drain hit its limit (#116855). Nothing group-wide is signalable, and the error + # must not escape: the caller still owns the output it drained. Signal the known + # PIDs instead so a live child (a group we may not signal) cannot outlive us. + for target in (proc, *descendants): + with contextlib.suppress(Exception): + target.kill() _sweep_escaped_descendants(descendants, pgid)