fix(ai_mail): escape systemd cgroup for daemonized wakes (td-48)
This commit is contained in:
@@ -62,6 +62,21 @@ PyPI version — not the changelog header.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Daemonized wakes killed by systemd cgroup teardown (td-48)** — timer-fired
|
||||
`wake_branch()` calls spawned the dispatch monitor + claude child, then died
|
||||
within seconds with no email and a stale lock, while the *same* wake from an
|
||||
interactive terminal worked. Root cause: a systemd oneshot service defaults to
|
||||
`KillMode=control-group`, so when the ~1.7s tick process exits, systemd SIGTERMs
|
||||
**every member of its cgroup** — `start_new_session=True` is irrelevant because
|
||||
systemd tracks by cgroup, not process group. Fix in `ai_mail` dispatch: detect
|
||||
the systemd context (`INVOCATION_ID`) and re-spawn the monitor via
|
||||
`systemd-run --user` in its **own transient unit**, escaping the parent cgroup
|
||||
(falls back to direct `Popen` when not under systemd); plus `stdin=DEVNULL` on
|
||||
both the monitor and claude `Popen` calls and monitor PID self-registration in
|
||||
the lock. Now genuinely live-proven through the timer: 3 branches
|
||||
(commons/cli/backup) woken purely by `daemon-tick.timer` each emailed @devpulse
|
||||
and exited clean (~20s, code=0). 737 ai_mail tests green, seedgo 100%.
|
||||
|
||||
- **seedgo-audit — telegram ported-but-unwired functions** — the DPLAN-0218
|
||||
relocation pulled the telegram lib into the seedgo gate's scope, surfacing 16
|
||||
`unused_function` flags across 8 handler files. These are *not* dead code —
|
||||
|
||||
@@ -245,6 +245,7 @@ def _run_with_startup_check(
|
||||
|
||||
try:
|
||||
popen_kwargs = {
|
||||
"stdin": subprocess.DEVNULL,
|
||||
"stdout": stdout_fh if stdout_fh is not None else subprocess.DEVNULL,
|
||||
"stderr": stderr_fh,
|
||||
"cwd": cwd,
|
||||
@@ -327,6 +328,18 @@ def main():
|
||||
|
||||
json_handler.log_operation("dispatch_monitor_start", {"branch": branch_email, "sender": sender})
|
||||
|
||||
# Self-register PID in lock file — the parent may have written its own
|
||||
# PID during pre-spawn lock acquisition (DPLAN-0155), and under
|
||||
# systemd-run the parent PID belongs to the caller, not the monitor.
|
||||
try:
|
||||
lock_path_obj = Path(lock_file)
|
||||
if lock_path_obj.exists():
|
||||
ld = json.loads(lock_path_obj.read_text(encoding="utf-8"))
|
||||
ld["pid"] = os.getpid()
|
||||
lock_path_obj.write_text(json.dumps(ld, indent=2), encoding="utf-8")
|
||||
except (json.JSONDecodeError, OSError):
|
||||
logger.info("[monitor] Could not self-register PID in lock file %s", lock_file)
|
||||
|
||||
# Open stderr log for claude output (rotate if > 500KB)
|
||||
stderr_fh = None
|
||||
try:
|
||||
|
||||
@@ -290,6 +290,78 @@ def _check_pid_alive(pid: int) -> bool:
|
||||
return True # Exists but can't check — assume alive
|
||||
|
||||
|
||||
def _spawn_in_systemd_scope(monitor_cmd, branch_path, spawn_env, branch_email, lock_file_path, custom_message, status):
|
||||
"""Spawn monitor in its own systemd unit to survive cgroup cleanup (td-48).
|
||||
|
||||
When wake_branch() runs inside a systemd oneshot service (e.g.
|
||||
daemon-tick.timer), the default KillMode=control-group sends SIGTERM to
|
||||
every process in the cgroup once the main process exits — killing the
|
||||
detached monitor and its claude child. systemd-run --user creates a
|
||||
transient service unit with its own cgroup so the monitor survives.
|
||||
|
||||
Returns True on success, False to fall back to direct Popen.
|
||||
"""
|
||||
unit_name = f"dispatch-{branch_email.lstrip('@')}"
|
||||
env_file = branch_path / "logs" / ".dispatch_env"
|
||||
|
||||
try:
|
||||
with open(env_file, "w", encoding="utf-8") as ef:
|
||||
for key, val in spawn_env.items():
|
||||
if "\n" not in str(val):
|
||||
ef.write(f"{key}={val}\n")
|
||||
env_file.chmod(0o600)
|
||||
except OSError as e:
|
||||
logger.warning("[wake] Failed to write env file for systemd-run: %s", e)
|
||||
return False
|
||||
|
||||
systemd_cmd = [
|
||||
"systemd-run",
|
||||
"--user",
|
||||
"--unit",
|
||||
unit_name,
|
||||
"--collect",
|
||||
"--property",
|
||||
f"WorkingDirectory={branch_path}",
|
||||
"--property",
|
||||
f"EnvironmentFile={env_file}",
|
||||
"--property",
|
||||
"StandardInput=null",
|
||||
"--",
|
||||
] + monitor_cmd
|
||||
|
||||
try:
|
||||
result = subprocess.run(systemd_cmd, capture_output=True, text=True, timeout=15)
|
||||
if result.returncode != 0:
|
||||
logger.warning("[wake] systemd-run failed (rc=%d): %s", result.returncode, result.stderr.strip())
|
||||
return False
|
||||
except (subprocess.SubprocessError, OSError) as e:
|
||||
logger.warning("[wake] systemd-run failed: %s", e)
|
||||
return False
|
||||
|
||||
try:
|
||||
pid_result = subprocess.run(
|
||||
["systemctl", "--user", "show", f"{unit_name}.service", "-p", "MainPID", "--value"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=5,
|
||||
)
|
||||
monitor_pid = int(pid_result.stdout.strip())
|
||||
if monitor_pid > 0:
|
||||
lock_data = {
|
||||
"pid": monitor_pid,
|
||||
"timestamp": time.strftime("%Y-%m-%dT%H:%M:%S"),
|
||||
"branch": str(branch_path),
|
||||
"subject": custom_message or "daemon wake",
|
||||
}
|
||||
with open(lock_file_path, "w", encoding="utf-8") as f:
|
||||
json.dump(lock_data, f, indent=2)
|
||||
except (subprocess.SubprocessError, ValueError, OSError) as e:
|
||||
logger.info("[wake] Could not query systemd unit PID: %s", e)
|
||||
|
||||
status.ok("spawn", f"Monitor started via systemd scope ({unit_name})")
|
||||
return True
|
||||
|
||||
|
||||
# ─── Branch Resolution ──────────────────────────────────
|
||||
|
||||
|
||||
@@ -487,54 +559,92 @@ def wake_branch(
|
||||
return status, False
|
||||
status.ok("lock-acquire", "Dispatch lock acquired")
|
||||
|
||||
try:
|
||||
process = subprocess.Popen(
|
||||
# When inside a systemd oneshot service (e.g. daemon-tick.timer), the
|
||||
# default KillMode=control-group sends SIGTERM to all cgroup members
|
||||
# when the service exits — killing the detached monitor. Escape by
|
||||
# launching the monitor in its own transient systemd unit (td-48).
|
||||
spawned_via_scope = False
|
||||
monitor_pid = 0
|
||||
if os.environ.get("INVOCATION_ID") and shutil.which("systemd-run"):
|
||||
spawned_via_scope = _spawn_in_systemd_scope(
|
||||
monitor_cmd,
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
start_new_session=True,
|
||||
cwd=str(branch_path),
|
||||
env=spawn_env,
|
||||
branch_path,
|
||||
spawn_env,
|
||||
email,
|
||||
lock_file_path,
|
||||
custom_message,
|
||||
status,
|
||||
)
|
||||
|
||||
monitor_pid = process.pid
|
||||
if not spawned_via_scope:
|
||||
try:
|
||||
process = subprocess.Popen(
|
||||
monitor_cmd,
|
||||
stdin=subprocess.DEVNULL,
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
start_new_session=True,
|
||||
cwd=str(branch_path),
|
||||
env=spawn_env,
|
||||
)
|
||||
|
||||
# Update lock with real monitor PID
|
||||
lock_file = branch_path / ".ai_mail.local" / ".dispatch.lock"
|
||||
lock_data = {
|
||||
"pid": monitor_pid,
|
||||
"timestamp": time.strftime("%Y-%m-%dT%H:%M:%S"),
|
||||
"branch": str(branch_path),
|
||||
"subject": custom_message or "manual wake",
|
||||
}
|
||||
with open(lock_file, "w", encoding="utf-8") as f:
|
||||
json.dump(lock_data, f, indent=2)
|
||||
monitor_pid = process.pid
|
||||
|
||||
status.ok("spawn", f"Monitor started (PID {monitor_pid})")
|
||||
# Update lock with real monitor PID
|
||||
lock_file = branch_path / ".ai_mail.local" / ".dispatch.lock"
|
||||
lock_data = {
|
||||
"pid": monitor_pid,
|
||||
"timestamp": time.strftime("%Y-%m-%dT%H:%M:%S"),
|
||||
"branch": str(branch_path),
|
||||
"subject": custom_message or "manual wake",
|
||||
}
|
||||
with open(lock_file, "w", encoding="utf-8") as f:
|
||||
json.dump(lock_data, f, indent=2)
|
||||
|
||||
except FileNotFoundError as e:
|
||||
lock_file = branch_path / ".ai_mail.local" / ".dispatch.lock"
|
||||
lock_file.unlink(missing_ok=True)
|
||||
logger.warning("[wake] Spawn failed — script not found: %s", e)
|
||||
status.fail("spawn", "Python or monitor script not found")
|
||||
return status, False
|
||||
except Exception as e:
|
||||
lock_file = branch_path / ".ai_mail.local" / ".dispatch.lock"
|
||||
lock_file.unlink(missing_ok=True)
|
||||
logger.warning("[wake] Spawn failed for %s: %s", branch_email, e)
|
||||
status.fail("spawn", f"{type(e).__name__}: {e}")
|
||||
return status, False
|
||||
status.ok("spawn", f"Monitor started (PID {monitor_pid})")
|
||||
|
||||
except FileNotFoundError as e:
|
||||
lock_file = branch_path / ".ai_mail.local" / ".dispatch.lock"
|
||||
lock_file.unlink(missing_ok=True)
|
||||
logger.warning("[wake] Spawn failed — script not found: %s", e)
|
||||
status.fail("spawn", "Python or monitor script not found")
|
||||
return status, False
|
||||
except Exception as e:
|
||||
lock_file = branch_path / ".ai_mail.local" / ".dispatch.lock"
|
||||
lock_file.unlink(missing_ok=True)
|
||||
logger.warning("[wake] Spawn failed for %s: %s", branch_email, e)
|
||||
status.fail("spawn", f"{type(e).__name__}: {e}")
|
||||
return status, False
|
||||
|
||||
# Step 9: Liveness check (brief wait then verify)
|
||||
time.sleep(2)
|
||||
if _check_pid_alive(monitor_pid):
|
||||
status.ok("alive", f"Agent responding (PID {monitor_pid} alive)")
|
||||
if spawned_via_scope:
|
||||
_unit = f"dispatch-{email.lstrip('@')}"
|
||||
try:
|
||||
check = subprocess.run(
|
||||
["systemctl", "--user", "is-active", f"{_unit}.service"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=5,
|
||||
)
|
||||
if check.stdout.strip() == "active":
|
||||
status.ok("alive", f"Agent responding (unit {_unit} active)")
|
||||
else:
|
||||
status.fail("alive", f"Agent died immediately ({check.stdout.strip()})")
|
||||
lock_file = branch_path / ".ai_mail.local" / ".dispatch.lock"
|
||||
lock_file.unlink(missing_ok=True)
|
||||
return status, False
|
||||
except Exception as e:
|
||||
logger.info("[wake] Cannot verify systemd unit %s: %s", _unit, e)
|
||||
status.warn("alive", "Cannot verify systemd unit status — assuming running")
|
||||
else:
|
||||
status.fail("alive", f"Agent died immediately (PID {monitor_pid})")
|
||||
# Clean up lock
|
||||
lock_file = branch_path / ".ai_mail.local" / ".dispatch.lock"
|
||||
lock_file.unlink(missing_ok=True)
|
||||
return status, False
|
||||
if _check_pid_alive(monitor_pid):
|
||||
status.ok("alive", f"Agent responding (PID {monitor_pid} alive)")
|
||||
else:
|
||||
status.fail("alive", f"Agent died immediately (PID {monitor_pid})")
|
||||
lock_file = branch_path / ".ai_mail.local" / ".dispatch.lock"
|
||||
lock_file.unlink(missing_ok=True)
|
||||
return status, False
|
||||
|
||||
# Desktop notification
|
||||
notif_body = custom_message[:80] if custom_message else "Manual wake: check inbox"
|
||||
|
||||
@@ -601,6 +601,7 @@ class TestWakeBranchSpawnEnv:
|
||||
|
||||
local_bin = str(_Path.home() / ".local" / "bin")
|
||||
monkeypatch.setenv("PATH", "/usr/bin:/bin")
|
||||
monkeypatch.delenv("INVOCATION_ID", raising=False)
|
||||
|
||||
captured_envs: list = []
|
||||
|
||||
@@ -831,6 +832,7 @@ def _patch_wake_deps(monkeypatch, **overrides):
|
||||
monkeypatch.setattr(wake_mod, attr, val)
|
||||
|
||||
monkeypatch.setattr("aipass.ai_mail.apps.handlers.dispatch.wake.time.sleep", lambda _: None)
|
||||
monkeypatch.delenv("INVOCATION_ID", raising=False)
|
||||
|
||||
|
||||
class _FakeProc:
|
||||
|
||||
Reference in New Issue
Block a user