From 2a32fe8914bc2c4a77e2ee5f762dd59582b5e3ea Mon Sep 17 00:00:00 2001
From: webtecnica <webtecnica@users.noreply.github.com>
Date: Wed, 22 Jul 2026 13:19:21 -0300
Subject: [PATCH] fix(macos): use launchctl submit instead of start_new_session
 for plist reload helper (#69098)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The deferred launchd reload helper used start_new_session=True to detach
from the gateway's process group. However, setsid(2) alone does NOT move
the child outside the launchd job's process coalition — when launchctl
bootout fires on the gateway label, launchd terminates ALL processes in
that coalition, including the setsid-detached helper, leaving the service
permanently unloaded.

Fix by spawning the helper via launchctl submit, which creates a
transient launchd one-shot job that is wholly independent of the
gateway's coalition. This ensures the helper survives bootout and can
complete the bootstrap+verify cycle.

Also writes a durable pre-bootout marker to the reload log so the
distinction between 'helper never started' and 'helper ran but
bootout/bootstrap failed' can be diagnosed.

Fixes #69098
---
 hermes_cli/gateway.py                    | 32 +++++++++++++++++++++---
 tests/hermes_cli/test_gateway_service.py | 17 ++++++++++---
 2 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/hermes_cli/gateway.py b/hermes_cli/gateway.py
index 2a7c2f3de35a..abac5fd0758a 100644
--- a/hermes_cli/gateway.py
+++ b/hermes_cli/gateway.py
@@ -4124,6 +4124,11 @@ def refresh_launchd_plist_if_needed() -> bool:
             reload_log_path.parent.mkdir(parents=True, exist_ok=True)
         except OSError:
             pass
+
+        # Write a durable pre-bootout marker so we can distinguish "helper
+        # never started" from "helper ran but bootout/bootstrap failed".
+        _append_launchd_reload_log(f"Launchd reload helper started for {target}")
+
         # Retry until launchctl LISTS the label (not merely a zero bootstrap
         # exit) or the drain window elapses. The failure happens while the old
         # gateway is still draining (default agent.restart_drain_timeout=180s),
@@ -4146,18 +4151,39 @@ def refresh_launchd_plist_if_needed() -> bool:
             f"fi"
         )
         try:
+            # Spawn the reload helper via `launchctl submit` (a transient
+            # launchd one-shot job) instead of `start_new_session=True`.
+            # `start_new_session=True` only calls setsid(2), which creates a
+            # new POSIX session but does NOT move the child outside the
+            # launchd job's process coalition.  When `launchctl bootout` fires
+            # on the gateway label, launchd terminates ALL processes in that
+            # coalition — including a setsid-detached child (#69098).
+            #
+            # `launchctl submit` creates a wholly independent transient launchd
+            # job that launchd manages separately from the gateway, so bootout
+            # of the gateway job cannot reach the helper.
+            submit_label = f"{label}.reload.{os.getpid()}.{int(time.time())}"
             subprocess.Popen(
-                ["/bin/bash", "-c", reload_script],
-                start_new_session=True,
+                [
+                    "launchctl", "submit",
+                    "-l", submit_label,
+                    "-o", str(reload_log_path),
+                    "-e", str(reload_log_path),
+                    "--",
+                    "/bin/bash", "-c", reload_script,
+                ],
                 stdout=subprocess.DEVNULL,
                 stderr=subprocess.DEVNULL,
             )
         except Exception as e:
             logger.warning("Deferred launchd reload could not be spawned: %s", e)
+            _append_launchd_reload_log(
+                f"FAILED to spawn launchd reload helper for {target}: {e}"
+            )
             return False
         print(
             "↻ Updated gateway launchd service definition; reload deferred to a "
-            "detached helper (refresh ran inside the gateway process tree)"
+            "transient launchd job (refresh ran inside the gateway process tree)"
         )
         return True
 
diff --git a/tests/hermes_cli/test_gateway_service.py b/tests/hermes_cli/test_gateway_service.py
index 7041ab9aee4a..803c6a8793f1 100644
--- a/tests/hermes_cli/test_gateway_service.py
+++ b/tests/hermes_cli/test_gateway_service.py
@@ -735,12 +735,21 @@ class TestLaunchdServiceRecovery:
         assert "--replace" in plist_path.read_text(encoding="utf-8")
         # No DIRECT bootout/bootstrap ran (those would kill us mid-sequence).
         assert not [c for c in run_calls if "bootout" in c or "bootstrap" in c]
-        # Exactly one detached helper was spawned, in a new session, and it
-        # performs both bootout and bootstrap.
+        # Exactly one Popen call was made for the transient launchd job.
         assert len(popen_calls) == 1
         cmd, kwargs = popen_calls[0]
-        assert kwargs.get("start_new_session") is True
-        script = cmd[-1]
+        # Must use `launchctl submit` (not `start_new_session=True`) so the
+        # helper runs as a transient launchd job outside the gateway's process
+        # coalition, surviving bootout (#69098).
+        assert cmd[:3] == ["launchctl", "submit", "-l"]
+        assert kwargs.get("start_new_session") is not True
+        assert "-o" in cmd
+        assert "-e" in cmd
+        # The script is passed via -- /bin/bash -c ...
+        bash_idx = cmd.index("--") + 1
+        assert cmd[bash_idx] == "/bin/bash"
+        assert cmd[bash_idx + 1] == "-c"
+        script = cmd[bash_idx + 2]
         assert "bootout" in script and "bootstrap" in script
         assert str(plist_path) in script