From fb90e38f2d5e01333cbcef7c8ea48e357f6fae5a Mon Sep 17 00:00:00 2001 From: serxa Date: Thu, 17 Sep 2026 14:38:10 +0000 Subject: [PATCH 1/6] codex: contain and reap workflow-run descendants in a cgroup scope A completed/cancelled/crashed Codex workflow run could leave codex-linux-sandbox descendants alive: they setsid into their own process group and escape appserver._signal_process_tree's single killpg. This adds opt-in, workflow-only containment. - lifecycle.py: launch the app-server inside a delegated systemd --user --scope created before the command execs, so the whole setsid/double-fork subtree is contained; bounded TERM->grace->KILL reap via `systemctl --user stop`, verified by recursive cgroup.events populated=0; durable per-run owner record + receipt (atomic write + file/dir fsync); InvocationID + boot_id + daemon generation identity fences; reconcile + one retry entry. - codex.lifecycle.mode: disabled (default, inert) | observe (unenforced, no completion claim) | strict (fail before exec if containment unavailable, no silent downgrade). - appserver: strict launch through the wrapper + membership handshake; killpg kept only as non-authoritative backup. - service: startup reconciliation after the existing interrupted->failed pass; terminal-path reap independent of the in-memory client; retry_lifecycle_reap entry. - cli: `nerve codex reap-descendants ` retry trigger. - tests: R6 acceptance suite with real systemd scopes + real setsid/double-fork escapers and the fake app-server. Not a security boundary (trusted tool processes only). No deploy / privilege / linger / service-layout change. Default disabled. Co-Authored-By: Claude Opus 4.8 --- nerve/agent/backends/codex/appserver.py | 74 +- nerve/agent/backends/codex/backend.py | 35 + nerve/agent/backends/codex/lifecycle.py | 983 ++++++++++++++++++++++++ nerve/cli.py | 34 + nerve/config.py | 58 ++ nerve/workflows/service.py | 139 +++- tests/fixtures/lifecycle_escaper.py | 211 +++++ tests/test_codex_lifecycle.py | 703 +++++++++++++++++ 8 files changed, 2235 insertions(+), 2 deletions(-) create mode 100644 nerve/agent/backends/codex/lifecycle.py create mode 100755 tests/fixtures/lifecycle_escaper.py create mode 100644 tests/test_codex_lifecycle.py diff --git a/nerve/agent/backends/codex/appserver.py b/nerve/agent/backends/codex/appserver.py index b0adf19da..20c3bb7c8 100644 --- a/nerve/agent/backends/codex/appserver.py +++ b/nerve/agent/backends/codex/appserver.py @@ -28,6 +28,7 @@ from typing import Any, Awaitable, Callable from nerve.agent.backends.base import TransportDiedError +from nerve.agent.backends.codex import lifecycle logger = logging.getLogger(__name__) @@ -139,6 +140,7 @@ def __init__( client_name: str = "nerve", client_version: str = "1.0.0", request_timeout: float = 60.0, + containment: lifecycle.WorkflowContainment | None = None, ) -> None: self._bin_path = bin_path self._cwd = cwd @@ -148,6 +150,10 @@ def __init__( self._client_version = client_version self._request_timeout = request_timeout self._on_server_request = server_request_handler + # cgroup containment for a Codex *workflow* run (None otherwise — the + # launch then behaves exactly as before). See codex/lifecycle.py. + self._containment = containment + self._lifecycle_record: lifecycle.LifecycleRecord | None = None self._proc: asyncio.subprocess.Process | None = None self._reader_task: asyncio.Task | None = None @@ -178,6 +184,23 @@ async def start(self) -> dict: args.extend(["--config", kv]) args.extend(["app-server", "--listen", "stdio://"]) + # Workflow-run containment: in strict mode `prepare_launch` returns the + # scope-wrapped argv so the whole descendant tree is contained *before* + # the command execs (and raises, un-launched, if containment can't be + # established). In observe mode it records intent but leaves argv as-is. + containment = self._containment + if containment is not None and containment.enabled: + try: + args, self._lifecycle_record = await asyncio.to_thread( + lifecycle.prepare_launch, containment, args, + ) + except lifecycle.ContainmentUnavailable as e: + raise TransportDiedError(str(e)) from e + except (lifecycle.LifecycleError, OSError) as e: + raise TransportDiedError( + f"codex workflow containment could not be persisted: {e}" + ) from e + try: self._proc = await asyncio.create_subprocess_exec( *args, @@ -194,6 +217,26 @@ async def start(self) -> dict: f"Failed to spawn codex app-server ({self._bin_path}): {e}" ) from e + # Record the scope's immutable identity (InvocationID + resolved + # cgroup) now that systemd-run has created it. Failure here means the + # contained launch cannot be verified — fail the session (the scope, + # already created, is still reapable from the persisted intent). + if ( + containment is not None + and containment.mode == lifecycle.MODE_STRICT + and self._lifecycle_record is not None + ): + try: + self._lifecycle_record = await asyncio.to_thread( + lifecycle.record_launched, + containment.run_dir, self._lifecycle_record, + ) + except (lifecycle.LifecycleError, OSError) as e: + await self.close() + raise TransportDiedError( + f"codex workflow scope did not register: {e}" + ) from e + self._reader_task = asyncio.create_task( self._reader_loop(), name=f"codex-reader:{self._proc.pid}", ) @@ -215,11 +258,40 @@ async def start(self) -> dict: }, }) await self.notify("initialized", None) - return response except BaseException: await self.close() raise + # Membership handshake: confirm the app-server is actually inside its + # scope cgroup before any work is dispatched. Registration is made + # durable so a crash after here is reconcilable. + if ( + containment is not None + and containment.mode == lifecycle.MODE_STRICT + and self._lifecycle_record is not None + and self._proc is not None + and self._proc.pid is not None + ): + record = self._lifecycle_record + member = await asyncio.to_thread( + lifecycle.verify_membership, record.control_group, self._proc.pid, + ) + if not member: + await self.close() + raise TransportDiedError( + "codex app-server is not contained in its cgroup scope" + ) + try: + self._lifecycle_record = await asyncio.to_thread( + lifecycle.record_registered, containment.run_dir, record, + ) + except (lifecycle.LifecycleError, OSError) as e: + await self.close() + raise TransportDiedError( + f"codex workflow scope registration failed: {e}" + ) from e + return response + def is_alive(self) -> bool: return ( not self._closed diff --git a/nerve/agent/backends/codex/backend.py b/nerve/agent/backends/codex/backend.py index fe9342799..9bd2c4f71 100644 --- a/nerve/agent/backends/codex/backend.py +++ b/nerve/agent/backends/codex/backend.py @@ -40,6 +40,7 @@ TransportDiedError, TurnInput, ) +from nerve.agent.backends.codex import lifecycle from nerve.agent.backends.codex.appserver import ( CodexAppServerClient, CodexRpcError, @@ -653,6 +654,39 @@ def resolve_api_key(self) -> str | None: return None +_WORKFLOW_SESSION_PREFIX = "workflow:" + + +def _workflow_containment( + backend: "CodexBackend", spec: SessionSpec, +) -> lifecycle.WorkflowContainment | None: + """Build per-run cgroup containment for a Codex *workflow* run, else None. + + Gated on the ``workflow:``-prefixed session id (set deterministically by + the workflow service) and ``codex.lifecycle.mode``. Interactive/cron Codex + sessions and ``disabled`` mode always get None, so their launch is + unchanged. + """ + sid = spec.session_id or "" + if not sid.startswith(_WORKFLOW_SESSION_PREFIX): + return None + lc = backend.codex.lifecycle + if lc.mode not in (lifecycle.MODE_OBSERVE, lifecycle.MODE_STRICT): + return None + cfg = backend.config + run_id = sid[len(_WORKFLOW_SESSION_PREFIX):] + run_dir = Path(cfg.workflows.runs_dir).expanduser() / run_id + return lifecycle.WorkflowContainment( + mode=lc.mode, + run_dir=run_dir, + run_id=run_id, + session_id=sid, + workspace_id=str(cfg.workspace), + term_grace_seconds=lc.term_grace_seconds, + stop_timeout_seconds=lc.stop_timeout_seconds, + ) + + class CodexClient(AgentClient): """One live ``codex app-server`` subprocess for one nerve session.""" @@ -696,6 +730,7 @@ def __init__(self, backend: CodexBackend, spec: SessionSpec): env=env, server_request_handler=self._handle_server_request, config_overrides=config_overrides, + containment=_workflow_containment(backend, spec), ) self._thread_id: str | None = None self._turn_id: str | None = None diff --git a/nerve/agent/backends/codex/lifecycle.py b/nerve/agent/backends/codex/lifecycle.py new file mode 100644 index 000000000..6a4d20f2b --- /dev/null +++ b/nerve/agent/backends/codex/lifecycle.py @@ -0,0 +1,983 @@ +"""Descendant containment + reaping for Codex **workflow** runs. + +Why this exists +--------------- +A Codex workflow run spawns ``codex app-server``, which spawns Ultracode +workers, which spawn ``codex-linux-sandbox`` processes. The sandbox +``setsid``\\ s into its own session/process-group and may double-fork, so a +descendant ends up in a process group that is *not* the app-server's. +:func:`CodexAppServerClient._signal_process_tree` signals a single process +group (``os.killpg(proc.pid, sig)``); it structurally cannot reach those +escaped descendants. When a run completes, is cancelled, hits its budget, or +the daemon crashes, the escaped processes survive as orphans (reparented to +PID 1). This module contains a run's whole descendant tree in a dedicated +**cgroup v2 scope** so a single ``systemctl --user stop`` reaps all of it — +setsid/double-fork and all — and records a durable, validated receipt so the +cleanup is repeatable and survives a daemon restart. + +Scope of this first implementation (deliberately bounded) +--------------------------------------------------------- +* **Codex workflow runs only.** Interactive/cron Codex sessions and Claude + runs never engage this module. +* **Opt-in, three modes** (``codex.lifecycle.mode``): + + - ``disabled`` (default): this module is inert. Nothing changes. + - ``observe``: **UNENFORCED.** The launch is *not* wrapped (today's + behaviour is preserved) — a durable record is written for staged-rollout + visibility, and teardown writes an explicitly *unenforced* observation. + It never claims a cleanup it did not perform. + - ``strict``: the app-server is launched **inside a cgroup scope created + before the command execs** (no late ``cgroup.procs`` migration, no + threaded ``preexec_fn``). If containment cannot be established the launch + **fails before the command executes** — there is no silent downgrade to + ``killpg``/subreaper. + +* **Not a security boundary.** cgroup inheritance contains *trusted* tool + processes that fork/setsid/double-fork. A cooperating or malicious same-UID + process can still migrate out via the user manager or a writable cgroup, or + tamper with receipts. Obtaining that stronger property is explicitly out of + scope; no privilege is broadened to get it. + +Honest contracts (no false total-containment) +---------------------------------------------- +* Graceful terminal reap = bounded ``TERM`` → grace → ``KILL`` via + ``systemctl --user stop`` (``KillMode=control-group``, ``SendSIGKILL=yes``, + bounded ``TimeoutStopSec``), then emptiness is **verified** (the scope's + recursive ``cgroup.events`` ``populated=0``, or the scope cgroup gone after a + *successful* stop). A stop request is not itself proof of exit. +* A daemon ``SIGKILL`` leaves contained children alive until restart + reconciliation — no in-process ``finally`` survives ``SIGKILL``. +* A *missing* scope is only success when we successfully stopped it (or the + host rebooted, which no PID survives). After an access error, a unit + replacement, or an unproven recovery it stays ``pending_retry``. + +The durable store is per-run ``//lifecycle.json`` (atomic +write + file & directory fsync). No raw argv, credentials, or environment ever +enter a record or an error string — only unit name, systemd ``InvocationID``, +resolved cgroup path, and identity metadata. +""" + +from __future__ import annotations + +import contextlib +import fcntl +import json +import logging +import os +import re +import shutil +import subprocess +import time +import uuid +from dataclasses import asdict, dataclass, field +from pathlib import Path +from typing import Callable, Iterable + +logger = logging.getLogger(__name__) + +# -- modes ----------------------------------------------------------------- # + +MODE_DISABLED = "disabled" +MODE_OBSERVE = "observe" +MODE_STRICT = "strict" +VALID_MODES = (MODE_DISABLED, MODE_OBSERVE, MODE_STRICT) + +# -- durable store filenames ----------------------------------------------- # + +RECORD_NAME = "lifecycle.json" +LOCK_NAME = "lifecycle.lock" +RECONCILE_LOCK_NAME = ".lifecycle-reconcile.lock" + +# Fixed, argv-independent scope description. systemd-run derives a description +# from the command line by default (a leak risk — argv could carry config +# values); we always pass this literal instead. +SCOPE_DESCRIPTION = "Nerve Codex workflow run containment" + +# Record schema version (bump on incompatible shape changes). +RECORD_VERSION = 1 + +# systemd unit name charset (a conservative subset of what systemd accepts). +_UNIT_RE = re.compile(r"^nerve-wf-[A-Za-z0-9:_.-]+\.scope$") +# A resolved delegated user scope always lives under the user slice. +_CGROUP_ROOT = "/sys/fs/cgroup" +_USER_SLICE_PREFIX = "/user.slice/" + +# Terminal receipt outcomes (cleanup is finished, do not retry). +_TERMINAL_OUTCOMES = frozenset({ + "complete", # we stopped the scope and verified emptiness + "resolved_absent", # same-boot: the scope no longer exists, no signal sent + "resolved_prior_boot", # cross-boot: no process could have survived a reboot + "failed_replaced", # the unit was replaced by a foreign instance; refused + "unenforced_observe", # observe mode: no containment was ever established + "no_scope", # nothing was ever contained (disabled / non-strict) +}) +# Outcomes that still owe a retry. +_RETRY_OUTCOMES = frozenset({"pending_retry"}) + + +class LifecycleError(Exception): + """A containment/reaping operation failed.""" + + +class ContainmentUnavailable(LifecycleError): + """Strict mode requested but the host cannot establish containment. + + Raised *before* the command is launched, so nothing runs un-contained. + """ + + +# -- host capability + instance identity ----------------------------------- # + + +@dataclass(frozen=True) +class Capabilities: + ok: bool + systemd_run: bool + systemctl: bool + user_manager: bool + cgroup_v2: bool + reason: str + + +_caps_cache: Capabilities | None = None + + +def detect_capabilities(*, refresh: bool = False) -> Capabilities: + """Probe whether a per-run delegated user scope can be created here. + + Read-only and cached per process (the host does not change under us). A + strict launch consults this and fails before exec when ``ok`` is False. + """ + global _caps_cache + if _caps_cache is not None and not refresh: + return _caps_cache + + systemd_run = shutil.which("systemd-run") is not None + systemctl = shutil.which("systemctl") is not None + cgroup_v2 = Path(_CGROUP_ROOT, "cgroup.controllers").exists() + user_manager = False + if systemctl: + try: + proc = subprocess.run( + ["systemctl", "--user", "show-environment"], + capture_output=True, timeout=5, check=False, + ) + user_manager = proc.returncode == 0 + except (OSError, subprocess.SubprocessError): + user_manager = False + + reasons = [] + if not systemd_run: + reasons.append("systemd-run not found") + if not systemctl: + reasons.append("systemctl not found") + if not cgroup_v2: + reasons.append("cgroup v2 unified hierarchy not mounted") + if not user_manager: + reasons.append("systemd --user manager not reachable") + caps = Capabilities( + ok=not reasons, + systemd_run=systemd_run, + systemctl=systemctl, + user_manager=user_manager, + cgroup_v2=cgroup_v2, + reason="; ".join(reasons) or "ok", + ) + _caps_cache = caps + return caps + + +@dataclass(frozen=True) +class Instance: + """Identity of the running daemon process, for concurrency fencing. + + ``generation`` changes across daemon restarts (pid + start_ticks differ) + and across reboots (boot_id differs), so a record written by a previous + daemon incarnation is always distinguishable from one this process owns. + """ + + boot_id: str + pid: int + start_ticks: int + + @property + def generation(self) -> str: + return f"{self.boot_id}:{self.pid}:{self.start_ticks}" + + +def read_boot_id() -> str: + try: + return Path("/proc/sys/kernel/random/boot_id").read_text().strip() + except OSError: + return "" + + +def _proc_start_ticks(pid: int) -> int: + """Field 22 of ``/proc//stat`` (jiffies since boot at process start). + + ``comm`` (field 2) may contain spaces and parentheses, so split after the + final ``)``. + """ + try: + raw = Path(f"/proc/{pid}/stat").read_bytes() + except OSError: + return 0 + try: + after = raw.rsplit(b")", 1)[1].split() + # after[0] is field 3 (state); field 22 is index 19 here. + return int(after[19]) + except (IndexError, ValueError): + return 0 + + +def current_instance() -> Instance: + pid = os.getpid() + return Instance(boot_id=read_boot_id(), pid=pid, start_ticks=_proc_start_ticks(pid)) + + +# -- durable record --------------------------------------------------------- # + + +@dataclass +class Receipt: + """The outcome of a reap/reconcile attempt.""" + + outcome: str + enforced: bool = False + signalled: bool = False + verified_empty: bool = False + method: str = "" + attempts: int = 0 + error: str = "" + ts: str = "" + + def is_terminal(self) -> bool: + """No further reap attempt is owed for this outcome.""" + return self.outcome in _TERMINAL_OUTCOMES + + def is_complete(self) -> bool: + """True only when no live owned descendant remains (or none could). + + Deliberately excludes ``unenforced_observe`` (observe mode never + contained anything, so it makes no completion claim) and + ``failed_replaced`` (a foreign unit we refused to touch — escalate, + do not claim the tree was reaped).""" + return self.outcome in { + "complete", "resolved_absent", "resolved_prior_boot", "no_scope", + } + + def needs_retry(self) -> bool: + return self.outcome in _RETRY_OUTCOMES + + def needs_attention(self) -> bool: + """A terminal outcome that resolved neither to complete nor retry — + surfaced for a human/operator rather than silently accepted.""" + return self.outcome == "failed_replaced" + + +@dataclass +class LifecycleRecord: + run_id: str + session_id: str + mode: str + attempt_nonce: str + unit: str + workspace_id: str + boot_id: str + instance_generation: str + phase: str = "intent" # intent -> launched -> registered -> terminal + owner_kind: str = "workflow" + invocation_id: str = "" # systemd InvocationID, recorded at 'launched' + control_group: str = "" # resolved cgroup path, recorded at 'launched' + version: int = RECORD_VERSION + created_at: str = "" + updated_at: str = "" + receipt: dict | None = None + + # -- serialization ------------------------------------------------- # + + def to_dict(self) -> dict: + return asdict(self) + + @classmethod + def from_dict(cls, d: dict) -> "LifecycleRecord": + if not isinstance(d, dict): + raise LifecycleError("lifecycle record is not an object") + try: + rec = cls( + run_id=str(d["run_id"]), + session_id=str(d["session_id"]), + mode=str(d["mode"]), + attempt_nonce=str(d["attempt_nonce"]), + unit=str(d["unit"]), + workspace_id=str(d.get("workspace_id", "")), + boot_id=str(d.get("boot_id", "")), + instance_generation=str(d.get("instance_generation", "")), + phase=str(d.get("phase", "intent")), + owner_kind=str(d.get("owner_kind", "workflow")), + invocation_id=str(d.get("invocation_id", "")), + control_group=str(d.get("control_group", "")), + version=int(d.get("version", RECORD_VERSION)), + created_at=str(d.get("created_at", "")), + updated_at=str(d.get("updated_at", "")), + receipt=d.get("receipt") if isinstance(d.get("receipt"), dict) else None, + ) + except (KeyError, TypeError, ValueError) as e: + raise LifecycleError(f"malformed lifecycle record: {e}") from e + rec.validate() + return rec + + def validate(self) -> None: + """Reject anything that could steer a signal at a foreign target. + + Never trusts a cgroup path that isn't a leaf under the user slice + matching this record's unit, and never accepts an out-of-charset unit. + """ + if self.owner_kind != "workflow": + raise LifecycleError(f"unexpected owner_kind {self.owner_kind!r}") + if self.mode not in VALID_MODES: + raise LifecycleError(f"invalid mode {self.mode!r}") + if not _UNIT_RE.match(self.unit): + raise LifecycleError(f"unsafe unit name {self.unit!r}") + if not self.attempt_nonce: + raise LifecycleError("empty attempt_nonce") + if self.control_group: + _validate_cgroup_path(self.control_group, self.unit) + + +def _validate_cgroup_path(control_group: str, unit: str) -> None: + """A resolved scope cgroup must be a leaf under the user slice named for + its unit — never a root/parent, never a symlink escape, never ``..``.""" + if not control_group.startswith(_USER_SLICE_PREFIX): + raise LifecycleError(f"cgroup path not under user slice: {control_group!r}") + if ".." in control_group.split("/"): + raise LifecycleError(f"cgroup path traverses parent: {control_group!r}") + if control_group.rstrip("/").split("/")[-1] != unit: + raise LifecycleError( + f"cgroup leaf {control_group!r} does not match unit {unit!r}" + ) + + +# -- atomic durable IO ------------------------------------------------------ # + + +def _atomic_write_json(path: Path, obj: dict) -> None: + """Write ``obj`` as JSON atomically with file + directory fsync. + + Raises :class:`OSError` on any failure — a caller that cannot persist + launch identity must not launch, and one that cannot persist a completion + receipt must not report completion. + """ + path.parent.mkdir(parents=True, exist_ok=True) + tmp = path.with_name(f".{path.name}.{os.getpid()}.{uuid.uuid4().hex[:8]}.tmp") + data = json.dumps(obj, indent=2, sort_keys=True).encode("utf-8") + fd = os.open(tmp, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + try: + os.write(fd, data) + os.fsync(fd) + finally: + os.close(fd) + os.replace(tmp, path) + # fsync the directory so the rename is durable across a crash. + dir_fd = os.open(path.parent, os.O_RDONLY) + try: + os.fsync(dir_fd) + finally: + os.close(dir_fd) + + +def record_path(run_dir: Path) -> Path: + return Path(run_dir) / RECORD_NAME + + +def read_record(run_dir: Path) -> LifecycleRecord | None: + path = record_path(run_dir) + try: + raw = path.read_text(encoding="utf-8") + except FileNotFoundError: + return None + except OSError as e: + raise LifecycleError(f"cannot read lifecycle record: {e}") from e + try: + return LifecycleRecord.from_dict(json.loads(raw)) + except ValueError as e: + raise LifecycleError(f"corrupt lifecycle record JSON: {e}") from e + + +def write_record(run_dir: Path, record: LifecycleRecord) -> None: + record.validate() + record.updated_at = _now() + if not record.created_at: + record.created_at = record.updated_at + _atomic_write_json(record_path(run_dir), record.to_dict()) + + +def _now() -> str: + return time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()) + + +@contextlib.contextmanager +def _flock(lock_path: Path): + """Exclusive advisory lock; serialises reap/retry/reconcile per owner.""" + lock_path.parent.mkdir(parents=True, exist_ok=True) + fd = os.open(lock_path, os.O_WRONLY | os.O_CREAT, 0o600) + try: + fcntl.flock(fd, fcntl.LOCK_EX) + yield + finally: + with contextlib.suppress(OSError): + fcntl.flock(fd, fcntl.LOCK_UN) + os.close(fd) + + +# -- unit naming + launch wrapper ------------------------------------------- # + + +def scope_unit_name(run_id: str, nonce: str) -> str: + """Unique, charset-safe scope unit name embedding the attempt nonce. + + The nonce makes the name globally unique per attempt, so a name match is a + strong identity signal even in the tiny window before an InvocationID has + been recorded (the create/register gap). + """ + safe_run = re.sub(r"[^A-Za-z0-9_.-]", "_", run_id)[:80] + safe_nonce = re.sub(r"[^A-Za-z0-9]", "", nonce)[:16] + return f"nerve-wf-{safe_run}-{safe_nonce}.scope" + + +def build_scope_argv( + unit: str, inner_argv: list[str], *, term_grace_seconds: int, +) -> list[str]: + """Wrap ``inner_argv`` so it runs inside a delegated transient user scope. + + Containment is established by systemd *before* the command execs: no late + ``cgroup.procs`` migration and no threaded-Python ``preexec_fn``. The whole + fork/setsid/double-fork subtree stays in this scope's cgroup. + """ + if not _UNIT_RE.match(unit): + raise LifecycleError(f"refusing to launch with unsafe unit {unit!r}") + grace = max(1, int(term_grace_seconds)) + return [ + "systemd-run", "--user", "--scope", "--quiet", "--collect", + "--expand-environment=no", + "-p", "Delegate=yes", + "-p", "KillMode=control-group", + "-p", "SendSIGKILL=yes", + "-p", f"TimeoutStopSec={grace}", + "--description", SCOPE_DESCRIPTION, + "--unit", unit, + "--", *inner_argv, + ] + + +# -- systemd / cgroup queries ----------------------------------------------- # + + +def _systemctl_show(unit: str, props: Iterable[str]) -> dict[str, str]: + """``systemctl --user show`` a unit; parse ``KEY=VALUE`` lines. + + Returns an empty dict if systemctl is unavailable or the call fails. + """ + prop_arg = ",".join(props) + try: + proc = subprocess.run( + ["systemctl", "--user", "show", unit, "--property", prop_arg], + capture_output=True, text=True, timeout=10, check=False, + ) + except (OSError, subprocess.SubprocessError) as e: + logger.debug("systemctl show failed for %s: %s", unit, e) + return {} + out: dict[str, str] = {} + for line in proc.stdout.splitlines(): + key, sep, value = line.partition("=") + if sep: + out[key.strip()] = value.strip() + return out + + +def _unit_identity(unit: str) -> dict[str, str]: + return _systemctl_show( + unit, ("LoadState", "ActiveState", "InvocationID", "ControlGroup", "Description"), + ) + + +def _await_unit_identity( + unit: str, *, timeout: float = 4.0, +) -> tuple[str, str]: + """Poll until the scope's InvocationID + ControlGroup are populated. + + Returns ``(invocation_id, control_group)`` — both empty on timeout. + """ + deadline = time.monotonic() + timeout + inv = cg = "" + while time.monotonic() < deadline: + ident = _unit_identity(unit) + inv = ident.get("InvocationID", "") + cg = ident.get("ControlGroup", "") + if inv and cg: + return inv, cg + time.sleep(0.1) + return inv, cg + + +def _cgroup_events(control_group: str) -> tuple[str, bool | None]: + """Inspect a scope cgroup's ``cgroup.events`` ``populated`` state. + + ``populated`` is recursive per the kernel (set if *any* descendant cgroup + holds a process), so it is the correct emptiness signal even when the + delegated subtree created child cgroups. + + Returns ``(state, populated)`` where ``state`` is one of ``"gone"`` (the + scope cgroup directory no longer exists) or ``"present"``. ``populated`` is + None when the directory is gone or unreadable. + """ + if not control_group: + return "gone", None + path = Path(_CGROUP_ROOT + control_group) / "cgroup.events" + try: + text = path.read_text() + except FileNotFoundError: + return "gone", None + except OSError as e: + logger.debug("cgroup.events unreadable for %s: %s", control_group, e) + return "present", None + populated = None + for line in text.splitlines(): + if line.startswith("populated"): + populated = line.split()[-1].strip() == "1" + break + return "present", populated + + +def _scope_stop(unit: str, *, timeout: float) -> tuple[bool, str]: + """``systemctl --user stop`` a scope, bounded by ``timeout`` seconds. + + ``stop`` performs the configured ``KillMode=control-group`` TERM → grace + (``TimeoutStopSec``) → SIGKILL sequence over the whole cgroup. Returns + ``(ok, error)``. + """ + try: + proc = subprocess.run( + ["systemctl", "--user", "stop", unit], + capture_output=True, text=True, timeout=timeout, check=False, + ) + except subprocess.TimeoutExpired: + return False, "systemctl stop timed out" + except (OSError, subprocess.SubprocessError) as e: + return False, f"systemctl stop failed: {e}" + if proc.returncode != 0: + return False, (proc.stderr or f"systemctl stop rc={proc.returncode}").strip()[:200] + return True, "" + + +def proc_cgroup(pid: int) -> str | None: + """The cgroup v2 path of ``pid`` (``0::PATH`` line), or None.""" + try: + text = Path(f"/proc/{pid}/cgroup").read_text() + except OSError: + return None + for line in text.splitlines(): + if line.startswith("0::"): + return line[3:].strip() + return None + + +def verify_membership(control_group: str, pid: int) -> bool: + """True iff ``pid`` is contained in ``control_group`` (or a descendant) + and the scope is populated — a concrete membership proof at handshake.""" + if not control_group: + return False + member = proc_cgroup(pid) + if member is None: + return False + if member != control_group and not member.startswith(control_group.rstrip("/") + "/"): + return False + _state, populated = _cgroup_events(control_group) + return populated is not False + + +# -- launch-side operations (used by the app-server client) ----------------- # + + +def persist_intent( + run_dir: Path, *, run_id: str, session_id: str, mode: str, workspace_id: str, +) -> LifecycleRecord: + """Write the validated owner intent BEFORE anything can fork the command. + + Raises :class:`OSError`/:class:`LifecycleError` on a persistence failure — + a strict launch must not proceed if its identity cannot be recorded. + """ + nonce = uuid.uuid4().hex + unit = scope_unit_name(run_id, nonce) + inst = current_instance() + record = LifecycleRecord( + run_id=run_id, + session_id=session_id, + mode=mode, + attempt_nonce=nonce, + unit=unit, + workspace_id=workspace_id, + boot_id=inst.boot_id, + instance_generation=inst.generation, + phase="intent", + ) + write_record(Path(run_dir), record) + return record + + +def record_launched(run_dir: Path, record: LifecycleRecord) -> LifecycleRecord: + """After systemd-run started the scope, record its immutable identity. + + Reads the scope's systemd ``InvocationID`` and resolved ``ControlGroup`` + (the two together are the reuse-proof instance identity). Raises + :class:`LifecycleError` if they cannot be resolved — in strict mode the + caller then closes and fails the launch (the scope, already created, is + still reapable from the persisted intent). + """ + inv, cg = _await_unit_identity(record.unit) + if not inv or not cg: + raise LifecycleError( + f"scope {record.unit} did not report an InvocationID/ControlGroup" + ) + record.invocation_id = inv + record.control_group = cg + record.phase = "launched" + write_record(Path(run_dir), record) + return record + + +def record_registered(run_dir: Path, record: LifecycleRecord) -> LifecycleRecord: + record.phase = "registered" + write_record(Path(run_dir), record) + return record + + +# -- reap + reconcile ------------------------------------------------------- # + + +def reap( + run_dir: Path, *, term_grace_seconds: int = 5, stop_timeout_seconds: int = 20, +) -> Receipt: + """Authoritatively reap a terminal run's contained descendants. + + Independent of any in-memory client: it works purely from the durable + record. Idempotent — a record already carrying a terminal receipt is + returned unchanged; a ``pending_retry`` record is re-attempted. Serialised + per owner by a file lock so concurrent terminal/retry paths cannot race. + """ + run_dir = Path(run_dir) + # Cheap pre-check so run dirs without a lifecycle record (Claude runs, + # disabled mode) are never even given a lock file. The authoritative read + # happens under the lock below. + if not record_path(run_dir).exists(): + return Receipt(outcome="no_scope", ts=_now()) + with _flock(run_dir / LOCK_NAME): + try: + record = read_record(run_dir) + except LifecycleError as e: + # A corrupt/unreadable record cannot be validated, so it cannot be + # signalled from and cannot be safely rewritten — escalate + # conservatively (never claim completion). R4. + logger.warning("refusing to reap from corrupt record in %s: %s", + run_dir, e) + return Receipt( + outcome="pending_retry", method="corrupt-record", + error="corrupt lifecycle record; manual attention required", + ts=_now(), + ) + if record is None: + return Receipt(outcome="no_scope", ts=_now()) + if record.mode == MODE_OBSERVE: + return _write_receipt(run_dir, record, Receipt( + outcome="unenforced_observe", enforced=False, signalled=False, + method="observe", ts=_now(), + )) + if record.mode != MODE_STRICT: + return _write_receipt(run_dir, record, Receipt( + outcome="no_scope", enforced=False, ts=_now(), + )) + existing = _receipt_from(record) + if existing is not None and existing.is_terminal(): + return existing + receipt = _reap_strict( + record, + term_grace_seconds=term_grace_seconds, + stop_timeout_seconds=stop_timeout_seconds, + prev=existing, + ) + return _write_receipt(run_dir, record, receipt) + + +def _reap_strict( + record: LifecycleRecord, *, term_grace_seconds: int, + stop_timeout_seconds: int, prev: Receipt | None, +) -> Receipt: + attempts = (prev.attempts if prev else 0) + 1 + current_boot = read_boot_id() + + # Cross-boot: nothing from a previous boot can still be running, and the + # unit name may now belong to a foreign process this boot — never signal. + if record.boot_id and current_boot and record.boot_id != current_boot: + return Receipt( + outcome="resolved_prior_boot", enforced=True, signalled=False, + verified_empty=True, method="reboot", attempts=attempts, ts=_now(), + ) + + ident = _unit_identity(record.unit) + load_state = ident.get("LoadState", "") + active_state = ident.get("ActiveState", "") + live_inv = ident.get("InvocationID", "") + live_cg = ident.get("ControlGroup", "") or record.control_group + description = ident.get("Description", "") + + unit_present = load_state not in ("", "not-found") + + if not unit_present: + # The scope no longer exists. Same boot, so no reboot-cleanup claim — + # but a transient scope's cgroup is removed only after it is emptied, + # so absence means the contained tree is gone. No signal was sent. + return Receipt( + outcome="resolved_absent", enforced=True, signalled=False, + verified_empty=True, method="absent", attempts=attempts, ts=_now(), + ) + + # Identity gate: refuse to signal anything that is not provably our scope. + if record.invocation_id: + if live_inv and live_inv != record.invocation_id: + return Receipt( + outcome="failed_replaced", enforced=True, signalled=False, + method="identity", attempts=attempts, ts=_now(), + error="unit InvocationID differs from record (replaced by a " + "foreign instance); refusing to signal", + ) + else: + # Create/register gap: no InvocationID was ever recorded. The unit name + # embeds our unique nonce, so a same-name unit is ours — but demand the + # fixed description too before signalling. + if description and description != SCOPE_DESCRIPTION: + return Receipt( + outcome="failed_replaced", enforced=True, signalled=False, + method="identity", attempts=attempts, ts=_now(), + error="unit description does not match; refusing to signal", + ) + + if live_cg: + with contextlib.suppress(LifecycleError): + _validate_cgroup_path(live_cg, record.unit) + + ok, err = _scope_stop(record.unit, timeout=stop_timeout_seconds) + if not ok: + return Receipt( + outcome="pending_retry", enforced=True, signalled=True, + method="systemctl-stop", attempts=attempts, ts=_now(), + error=err or "stop failed", verified_empty=False, + ) + + # A successful stop is a request, not proof of exit — verify emptiness. + state, populated = _cgroup_events(live_cg) + if state == "gone" or populated is False: + return Receipt( + outcome="complete", enforced=True, signalled=True, + verified_empty=True, method="systemctl-stop", + attempts=attempts, ts=_now(), + ) + return Receipt( + outcome="pending_retry", enforced=True, signalled=True, + method="systemctl-stop", attempts=attempts, ts=_now(), + error="scope still populated after stop", verified_empty=False, + ) + + +def reconcile_run( + run_dir: Path, *, is_terminal: bool, active_generation: str, + term_grace_seconds: int = 5, stop_timeout_seconds: int = 20, +) -> Receipt | None: + """Reconcile one run's containment at startup / on demand. + + Cleans only a **terminal** run (or a **provably abandoned** launch: a + record still at ``intent``/``launched`` whose launching daemon generation + is not the current one). A run owned by the *current* generation, or a + still-active run, is left alone. Returns the receipt, or None when nothing + was due. + """ + run_dir = Path(run_dir) + record = read_record(run_dir) + if record is None: + return None + if record.mode != MODE_STRICT: + return None + # Never touch a run this very daemon generation still owns. + if record.instance_generation == active_generation: + return None + abandoned_launch = record.phase in ("intent", "launched", "registered") + if not (is_terminal or abandoned_launch): + return None + return reap( + run_dir, + term_grace_seconds=term_grace_seconds, + stop_timeout_seconds=stop_timeout_seconds, + ) + + +def list_records(runs_dir: Path) -> list[tuple[str, LifecycleRecord]]: + """Every run dir under ``runs_dir`` that carries a lifecycle record. + + A corrupt record is surfaced as a synthetic ``pending_retry`` sentinel via + the exception path in callers; here we skip unreadable ones and log, so a + single bad file cannot abort a whole reconciliation sweep. + """ + out: list[tuple[str, LifecycleRecord]] = [] + root = Path(runs_dir) + if not root.is_dir(): + return out + for child in sorted(root.iterdir()): + if not child.is_dir(): + continue + try: + record = read_record(child) + except LifecycleError: + logger.warning("skipping corrupt lifecycle record in %s", child.name) + continue + if record is not None: + out.append((child.name, record)) + return out + + +def reconcile_all( + runs_dir: Path, is_terminal: Callable[[str], bool], *, + term_grace_seconds: int = 5, stop_timeout_seconds: int = 20, +) -> list[tuple[str, Receipt]]: + """Sweep every run with a record, reaping terminal/abandoned containment. + + Holds a global reconcile lock so only one sweep runs at a time. ``is_terminal`` + maps a run id to whether its business status is terminal. + """ + root = Path(runs_dir) + results: list[tuple[str, Receipt]] = [] + gen = current_instance().generation + with _flock(root / RECONCILE_LOCK_NAME): + for run_id, _record in list_records(root): + try: + receipt = reconcile_run( + root / run_id, + is_terminal=is_terminal(run_id), + active_generation=gen, + term_grace_seconds=term_grace_seconds, + stop_timeout_seconds=stop_timeout_seconds, + ) + except LifecycleError as e: + logger.warning("reconcile failed for %s: %s", run_id, e) + continue + if receipt is not None: + results.append((run_id, receipt)) + return results + + +def retry( + run_dir: Path, *, term_grace_seconds: int = 5, stop_timeout_seconds: int = 20, +) -> Receipt: + """The one supported, narrowly-scoped retry entry. + + Re-attempts a ``pending_retry`` (or not-yet-run) reap. Idempotent: a record + already carrying a terminal receipt is returned unchanged. Never fabricates + success — a still-failing stop stays ``pending_retry`` with the error. + """ + return reap( + run_dir, + term_grace_seconds=term_grace_seconds, + stop_timeout_seconds=stop_timeout_seconds, + ) + + +# -- launch config handed to the app-server client -------------------------- # + + +@dataclass(frozen=True) +class WorkflowContainment: + """Immutable per-launch containment config for one workflow-run client. + + Built by the Codex backend only for a ``workflow:``-prefixed session when + ``codex.lifecycle.mode`` is ``observe``/``strict``; ``None`` (and therefore + today's behaviour) for every other session and for ``disabled`` mode. + """ + + mode: str + run_dir: Path + run_id: str + session_id: str + workspace_id: str + term_grace_seconds: int = 5 + stop_timeout_seconds: int = 20 + + @property + def enabled(self) -> bool: + return self.mode in (MODE_OBSERVE, MODE_STRICT) + + +def prepare_launch( + containment: WorkflowContainment, inner_argv: list[str], +) -> tuple[list[str], LifecycleRecord]: + """Persist intent and return the argv to actually spawn. + + * strict: validate host capability (raise :class:`ContainmentUnavailable` + *before* anything execs), persist intent, return the scope-wrapped argv. + * observe: persist an unenforced-mode intent, return ``inner_argv`` + unchanged (today's behaviour preserved). + + Raises before the command can execute if capability or persistence fails, + so a strict run never launches un-contained. + """ + if containment.mode == MODE_STRICT: + caps = detect_capabilities() + if not caps.ok: + raise ContainmentUnavailable( + f"strict cgroup containment unavailable: {caps.reason}" + ) + record = persist_intent( + containment.run_dir, + run_id=containment.run_id, + session_id=containment.session_id, + mode=containment.mode, + workspace_id=containment.workspace_id, + ) + if containment.mode == MODE_STRICT: + wrapped = build_scope_argv( + record.unit, inner_argv, + term_grace_seconds=containment.term_grace_seconds, + ) + return wrapped, record + return list(inner_argv), record + + +# -- receipt helpers -------------------------------------------------------- # + + +def _receipt_from(record: LifecycleRecord) -> Receipt | None: + if not record.receipt: + return None + try: + return Receipt(**{k: v for k, v in record.receipt.items() if k in Receipt.__dataclass_fields__}) + except TypeError: + return None + + +def _write_receipt( + run_dir: Path, record: LifecycleRecord, receipt: Receipt, +) -> Receipt: + record.receipt = asdict(receipt) + if receipt.is_terminal(): + record.phase = "terminal" + write_record(Path(run_dir), record) + return receipt + + +__all__ = [ + "MODE_DISABLED", "MODE_OBSERVE", "MODE_STRICT", "VALID_MODES", + "Capabilities", "detect_capabilities", "Instance", "current_instance", + "read_boot_id", "LifecycleError", "ContainmentUnavailable", + "LifecycleRecord", "Receipt", "record_path", "read_record", "write_record", + "scope_unit_name", "build_scope_argv", "verify_membership", "proc_cgroup", + "persist_intent", "record_launched", "record_registered", + "reap", "reconcile_run", "reconcile_all", "list_records", "retry", + "SCOPE_DESCRIPTION", "WorkflowContainment", "prepare_launch", +] diff --git a/nerve/cli.py b/nerve/cli.py index bed93edbb..1c6c6c53c 100644 --- a/nerve/cli.py +++ b/nerve/cli.py @@ -1623,6 +1623,40 @@ def codex() -> None: """Inspect and authenticate the Codex integration.""" +@codex.command("reap-descendants") +@click.argument("run_id") +@click.option("--json-output", is_flag=True, help="Emit machine-readable JSON.") +@click.pass_context +def codex_reap_descendants(ctx: click.Context, run_id: str, json_output: bool) -> None: + """Retry the cgroup reap of a Codex workflow run's contained descendants. + + The supported, idempotent retry trigger for a ``pending_retry`` lifecycle + reap (see codex/lifecycle.py). Reads only the run's durable record; never + fabricates success. A run with no record reports ``no_scope``. + """ + import json as _json + from pathlib import Path + + from nerve.agent.backends.codex import lifecycle + + config = ctx.obj["config"] + lc = config.codex.lifecycle + run_dir = Path(config.workflows.runs_dir).expanduser() / run_id + receipt = lifecycle.retry( + run_dir, + term_grace_seconds=lc.term_grace_seconds, + stop_timeout_seconds=lc.stop_timeout_seconds, + ) + from dataclasses import asdict + + if json_output: + click.echo(_json.dumps(asdict(receipt), indent=2, sort_keys=True)) + else: + verdict = "complete" if receipt.is_complete() else receipt.outcome + click.echo(f"[{verdict}] {run_id}: {receipt.outcome}" + + (f" — {receipt.error}" if receipt.error else "")) + + @codex.command("token") @click.option( "--hours", type=click.IntRange(1, 24), default=8, show_default=True, diff --git a/nerve/config.py b/nerve/config.py index a5d306975..232dcca27 100644 --- a/nerve/config.py +++ b/nerve/config.py @@ -2261,6 +2261,61 @@ def validate(self) -> list[str]: return problems +@dataclass +class CodexLifecycleConfig: + """Descendant containment for Codex **workflow** runs (cgroup v2 scope). + + A completed/cancelled/crashed workflow run can leave ``codex-linux-sandbox`` + descendants alive: they ``setsid`` into their own process group and escape + the single-group ``killpg`` teardown. This opt-in feature launches a + workflow run's app-server inside a delegated systemd user scope so the + whole tree can be reaped, with a durable receipt. See + ``nerve/agent/backends/codex/lifecycle.py``. + + ``mode``: + + * ``disabled`` (default) — inert; launch behaviour is unchanged. + * ``observe`` — UNENFORCED: records intent for staged-rollout visibility + but does **not** contain the launch and never claims a cleanup it did not + perform. + * ``strict`` — launches inside a cgroup scope created before the command + execs; if containment cannot be established the launch **fails before + exec** (no silent downgrade). Codex workflow runs only. + """ + + mode: str = "disabled" + term_grace_seconds: int = 5 + stop_timeout_seconds: int = 20 + + @classmethod + @_coerced + def from_dict(cls, raw: dict | None) -> "CodexLifecycleConfig": + d = raw or {} + return cls( + mode=str(d.get("mode") or "disabled").strip().lower(), + term_grace_seconds=_lenient_int(d.get("term_grace_seconds"), 5), + stop_timeout_seconds=_lenient_int(d.get("stop_timeout_seconds"), 20), + ) + + def validate(self) -> list[str]: + problems: list[str] = [] + if self.mode not in ("disabled", "observe", "strict"): + problems.append( + "codex.lifecycle.mode must be 'disabled', 'observe', or " + f"'strict', got {self.mode!r}" + ) + if self.term_grace_seconds < 1: + problems.append("codex.lifecycle.term_grace_seconds must be >= 1") + if self.stop_timeout_seconds < 1: + problems.append("codex.lifecycle.stop_timeout_seconds must be >= 1") + if self.stop_timeout_seconds <= self.term_grace_seconds: + problems.append( + "codex.lifecycle.stop_timeout_seconds must exceed " + "term_grace_seconds (stop waits out the TERM grace, then KILL)" + ) + return problems + + @dataclass class CodexConfig: """OpenAI Codex backend (``codex app-server``) settings. @@ -2301,6 +2356,7 @@ class CodexConfig: # Arbitrary codex config-override passthrough (-c key=value at spawn) extra_config: dict[str, Any] = field(default_factory=dict) ultracode: UltracodeConfig = field(default_factory=UltracodeConfig) + lifecycle: CodexLifecycleConfig = field(default_factory=CodexLifecycleConfig) @classmethod @_coerced @@ -2353,6 +2409,7 @@ def from_dict(cls, d: dict) -> "CodexConfig": pricing=pricing, extra_config=dict(d.get("extra_config") or {}), ultracode=UltracodeConfig.from_dict(d.get("ultracode")), + lifecycle=CodexLifecycleConfig.from_dict(d.get("lifecycle")), ) def validate(self) -> list[str]: @@ -2374,6 +2431,7 @@ def validate(self) -> list[str]: f"got {self.sandbox!r}" ) problems.extend(self.ultracode.validate()) + problems.extend(self.lifecycle.validate()) return problems diff --git a/nerve/workflows/service.py b/nerve/workflows/service.py index 98205c67e..adc99bb6d 100644 --- a/nerve/workflows/service.py +++ b/nerve/workflows/service.py @@ -53,9 +53,10 @@ from pathlib import Path from typing import TYPE_CHECKING, Any +from nerve.agent.backends.codex import lifecycle as codex_lifecycle from nerve.agent.streaming import broadcaster from nerve.db.usage import estimate_turn_cost -from nerve.db.workflow_runs import ACTIVE_STATUSES +from nerve.db.workflow_runs import ACTIVE_STATUSES, TERMINAL_STATUSES from nerve.utils.time import utc_now_iso if TYPE_CHECKING: @@ -159,6 +160,11 @@ async def start(self) -> None: "survive the restart; restart them explicitly if still needed.", priority="high", ) + # R4 ordering: the interrupted-active→failed pass above has already run, + # so every prior-generation run is terminal here. Reap any Codex + # workflow-run cgroup scopes their (now-gone) clients left behind, + # before the monitor loop can dispatch anything new. + await self._reconcile_codex_lifecycle() interval = max(5, int(self.config.workflows.poll_interval_seconds)) self._monitor_task = asyncio.create_task(self._monitor_loop(interval)) logger.info( @@ -715,6 +721,15 @@ async def _finalize_terminal(self, run_id: str, status: str, detail: dict) -> No logger.exception( "workflow run completion listener failed for %s", run_id, ) + # Authoritatively reap any contained Codex descendants for this run, + # independent of the in-memory client (works from the durable record). + # Detached so it never blocks the terminal path; no-ops unless the run + # recorded strict/observe containment. + if str(run.get("engine") or "") == ENGINE_CODEX: + self._spawn_detached( + self._reap_codex_lifecycle(run_id), + f"workflow-lifecycle-reap-{run_id}", + ) @staticmethod def _is_quiet(run: dict | None) -> bool: @@ -722,6 +737,128 @@ def _is_quiet(run: dict | None) -> bool: notifications (a 3-iteration loop would otherwise ping 6-8 times).""" return str((run or {}).get("created_by") or "").startswith("review-loop:") + # ------------------------------------------------------------------ # + # Codex workflow-run descendant containment (cgroup lifecycle) # + # ------------------------------------------------------------------ # + + async def _reconcile_codex_lifecycle(self) -> None: + """Startup reconciliation of Codex workflow-run cgroup scopes. + + Reaps contained descendants of terminal/abandoned prior-generation runs + from their durable records — independent of the in-memory client, which + did not survive the restart. Never touches a run this daemon generation + still owns (the generation fence lives in ``reconcile_run``). Best + effort: a failure here must not block the monitor loop from starting. + """ + lc = self.config.codex.lifecycle + if lc.mode != codex_lifecycle.MODE_STRICT: + return + runs_dir = self.runs_dir() + try: + records = await asyncio.to_thread( + codex_lifecycle.list_records, runs_dir, + ) + except Exception: # noqa: BLE001 + logger.exception("codex lifecycle record scan failed") + return + if not records: + return + # Snapshot business terminality on the loop (DB access), then reap + # off-thread. A run whose row is gone is treated terminal. + terminal_ids: dict[str, bool] = {} + for run_id, _rec in records: + run = await self.db.get_workflow_run(run_id) + terminal_ids[run_id] = ( + run is None or run.get("status") in TERMINAL_STATUSES + ) + + def _is_terminal(run_id: str) -> bool: + return terminal_ids.get(run_id, True) + + try: + results = await asyncio.to_thread( + codex_lifecycle.reconcile_all, runs_dir, _is_terminal, + term_grace_seconds=lc.term_grace_seconds, + stop_timeout_seconds=lc.stop_timeout_seconds, + ) + except Exception: # noqa: BLE001 + logger.exception("codex lifecycle reconciliation failed") + return + for run_id, receipt in results: + self._journal_lifecycle(run_id, "reconcile", receipt) + retry_owed = [rid for rid, rc in results if rc.needs_retry()] + if retry_owed: + logger.warning( + "codex lifecycle reconciliation left %d run(s) needing a reap " + "retry (nerve codex reap-descendants ): %s", + len(retry_owed), ", ".join(retry_owed), + ) + attention = [rid for rid, rc in results if rc.needs_attention()] + if attention: + logger.warning( + "codex lifecycle reconciliation refused %d run(s) whose scope " + "was replaced by a foreign unit: %s", + len(attention), ", ".join(attention), + ) + + async def _reap_codex_lifecycle(self, run_id: str) -> None: + """Terminal-path reap for one run. No-ops unless it recorded a scope.""" + lc = self.config.codex.lifecycle + run_dir = self.runs_dir() / run_id + try: + receipt = await asyncio.to_thread( + codex_lifecycle.reap, run_dir, + term_grace_seconds=lc.term_grace_seconds, + stop_timeout_seconds=lc.stop_timeout_seconds, + ) + except Exception: # noqa: BLE001 + logger.exception("codex lifecycle reap failed for %s", run_id) + return + if receipt.outcome != "no_scope": + self._journal_lifecycle(run_id, "reap", receipt) + + async def retry_lifecycle_reap(self, run_id: str) -> dict: + """The one supported, narrowly-scoped retry trigger (service entry). + + Re-attempts a ``pending_retry`` reap from the durable record and + returns the resulting receipt. Idempotent; never fabricates success. + A run with no record yields ``no_scope``. + """ + run_dir = self.runs_dir() / run_id + lc = self.config.codex.lifecycle + receipt = await asyncio.to_thread( + codex_lifecycle.retry, run_dir, + term_grace_seconds=lc.term_grace_seconds, + stop_timeout_seconds=lc.stop_timeout_seconds, + ) + if receipt.outcome != "no_scope": + self._journal_lifecycle(run_id, "reap_retry", receipt) + from dataclasses import asdict as _asdict + + return _asdict(receipt) + + def _journal_lifecycle(self, run_id: str, event: str, receipt: Any) -> None: + """Append a lifecycle receipt summary to the run's journal. + + Receipt fields carry no argv/credentials/environment — only outcome, + method, and identity-verification booleans. + """ + from dataclasses import asdict as _asdict + + journal_dir = self.runs_dir() / run_id + try: + path = journal_dir / "events.ndjson" + if not journal_dir.is_dir(): + return + line = json.dumps({ + "ts": utc_now_iso(), "run_id": run_id, + "event": f"lifecycle_{event}", **_asdict(receipt), + }) + with path.open("a", encoding="utf-8") as f: + f.write(line + "\n") + except OSError: + logger.debug("lifecycle journal write failed for %s", run_id) + # ------------------------------------------------------------------ # # Budget monitor # # ------------------------------------------------------------------ # diff --git a/tests/fixtures/lifecycle_escaper.py b/tests/fixtures/lifecycle_escaper.py new file mode 100755 index 000000000..03ec56931 --- /dev/null +++ b/tests/fixtures/lifecycle_escaper.py @@ -0,0 +1,211 @@ +#!/usr/bin/env python3 +"""Real escaping-descendant fixture for Codex lifecycle tests (no model, no net). + +Mirrors the incident shape: a process ``setsid``\\ s into its own session/group +and double-forks a grandchild that lingers, so ``killpg`` on the launcher's +group cannot reach it — only a cgroup-scoped reap can. Every process carries a +unique ``ESCAPER_TOKEN`` in its environment (and leaves carry it in argv) so the +test's belt-and-braces cleanup only ever signals its own fixtures, by pidfd, +never by name/pgrep/killall. + +Commands (each is meant to run as the main command of a systemd ``--scope``): + + spawn-escapee setsid + double-fork a sleeper grandchild + spawn-escapee-nested …then migrate the grandchild into a child + cgroup it mkdirs under the delegated scope + (root cgroup.procs empties; recursive + cgroup.events populated stays 1) + spawn-escapee-igterm grandchild ignores SIGTERM (forces KILL) + spawn-escapee-forker grandchild forks 3 more sleepers on SIGTERM + (fork-during-teardown; all in the cgroup) + sleeper sleep (a leaf) + sweep identity-checked pidfd cleanup of the token +""" + +from __future__ import annotations + +import os +import signal +import sys +import time + +SELF = os.path.abspath(__file__) +_SLEEP = 300 + + +# --------------------------- identity + cleanup --------------------------- # + + +def _environ_token(pid: int) -> str | None: + try: + with open(f"/proc/{pid}/environ", "rb") as fh: + raw = fh.read() + except OSError: + return None + for entry in raw.split(b"\0"): + if entry.startswith(b"ESCAPER_TOKEN="): + return entry[len(b"ESCAPER_TOKEN="):].decode("utf-8", "replace") + return None + + +def _cmdline(pid: int) -> str: + try: + with open(f"/proc/{pid}/cmdline", "rb") as fh: + return fh.read().replace(b"\0", b" ").decode("utf-8", "replace") + except OSError: + return "" + + +def is_alive(pid: int) -> bool: + try: + os.kill(pid, 0) + return True + except ProcessLookupError: + return False + except PermissionError: + return True + + +def owned_by_me(pid: int, token: str) -> bool: + if pid <= 1 or pid == os.getpid(): + return False + if _environ_token(pid) != token: + return False + return SELF in _cmdline(pid) + + +def identity_kill(pid: int, token: str) -> str: + if not owned_by_me(pid, token): + return "skipped(not-owned)" + try: + pidfd = os.pidfd_open(pid) + except (ProcessLookupError, OSError) as e: + return f"gone({e.__class__.__name__})" + try: + if not owned_by_me(pid, token): + return "skipped(reuse)" + try: + signal.pidfd_send_signal(pidfd, signal.SIGKILL) + return "killed" + except ProcessLookupError: + return "already-gone" + finally: + os.close(pidfd) + + +def sweep(token: str) -> list[tuple[int, str]]: + out = [] + for name in os.listdir("/proc"): + if not name.isdigit(): + continue + pid = int(name) + if owned_by_me(pid, token): + out.append((pid, identity_kill(pid, token))) + return out + + +# ------------------------------ the fork tree ----------------------------- # + + +def _become_sleeper(token: str, role: str, outdir: str) -> None: + os.execv(sys.executable, [sys.executable, SELF, "sleeper", token, role, outdir]) + + +def _write(outdir: str, name: str, value: str) -> None: + with open(os.path.join(outdir, name), "w") as fh: + fh.write(value) + + +def _own_cgroup() -> str | None: + try: + with open("/proc/self/cgroup") as fh: + for line in fh: + if line.startswith("0::"): + return line[3:].strip() + except OSError: + return None + return None + + +def _double_fork_grandchild(token: str, outdir: str, variant: str) -> None: + """In a child: setsid → NEW group, close stdio, double-fork the survivor.""" + os.setsid() + devnull = os.open(os.devnull, os.O_RDWR) + for fd in (0, 1, 2): + os.dup2(devnull, fd) + g = os.fork() + if g != 0: + os._exit(0) # sandbox exits → grandchild is double-fork orphaned + + # grandchild (the survivor) + if variant == "nested": + # Migrate into a child cgroup of the delegated scope: the scope root's + # cgroup.procs empties, but recursive cgroup.events populated stays 1. + cg = _own_cgroup() + if cg: + leaf = "/sys/fs/cgroup" + cg + "/nested" + try: + os.mkdir(leaf) + with open(leaf + "/cgroup.procs", "w") as fh: + fh.write(str(os.getpid())) + except OSError: + pass # non-fatal: fall back to plain containment + elif variant == "igterm": + signal.signal(signal.SIGTERM, signal.SIG_IGN) + elif variant == "forker": + def _spawn_more(_sig, _frm): + for _ in range(3): + if os.fork() == 0: + _become_sleeper(token, "forked", outdir) + signal.signal(signal.SIGTERM, _spawn_more) + + _write(outdir, "grandchild.pid", str(os.getpid())) + _become_sleeper(token, "grandchild", outdir) + + +def _spawn(variant: str, argv: list[str]) -> int: + token = os.environ["ESCAPER_TOKEN"] + outdir = argv[0] + child = os.fork() + if child == 0: + _double_fork_grandchild(token, outdir, variant) + os._exit(0) # unreachable + # scope main: wait for the grandchild to materialize, reap the middle + # child (so it is not a zombie), then exit — leaving the escapee behind. + deadline = time.monotonic() + 5 + gc = os.path.join(outdir, "grandchild.pid") + while time.monotonic() < deadline and not os.path.exists(gc): + time.sleep(0.05) + try: + os.waitpid(child, 0) + except ChildProcessError: + pass + return 0 + + +def main() -> int: + if len(sys.argv) < 2: + print("usage: lifecycle_escaper.py ...", file=sys.stderr) + return 2 + cmd = sys.argv[1] + if cmd == "sleeper": + time.sleep(_SLEEP) + return 0 + if cmd == "spawn-escapee": + return _spawn("plain", sys.argv[2:]) + if cmd == "spawn-escapee-nested": + return _spawn("nested", sys.argv[2:]) + if cmd == "spawn-escapee-igterm": + return _spawn("igterm", sys.argv[2:]) + if cmd == "spawn-escapee-forker": + return _spawn("forker", sys.argv[2:]) + if cmd == "sweep": + token = os.environ.get("ESCAPER_TOKEN") or (sys.argv[2] if len(sys.argv) > 2 else "") + print(sweep(token)) + return 0 + print(f"unknown cmd {cmd}", file=sys.stderr) + return 2 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_codex_lifecycle.py b/tests/test_codex_lifecycle.py new file mode 100644 index 000000000..74c6598a0 --- /dev/null +++ b/tests/test_codex_lifecycle.py @@ -0,0 +1,703 @@ +"""R6 acceptance suite for Codex workflow-run descendant containment. + +These are assertion-bearing tests, not smoke checks. Where the review demands +it, they use **real** systemd user scopes running **real** setsid/double-fork +escaper processes (``tests/fixtures/lifecycle_escaper.py``) and the **real** +fake app-server (``tests/fixtures/fake_codex_appserver.py``) through the actual +launch wrapper — no paid model, no network. Every scope/process a test creates +is torn down by the ``scopes`` fixture (``systemctl --user stop`` + an +identity-checked pidfd sweep), and nothing ever touches the host user manager, +the daemon, the real ``~/.nerve/workflow-runs``, or any pre-existing process. + +Cases map to the review's R6.1–R6.8. Tests needing a scope skip cleanly on a +host without a reachable systemd ``--user`` manager. +""" + +from __future__ import annotations + +import asyncio +import json +import os +import subprocess +import sys +import threading +import uuid +from pathlib import Path + +import pytest + +from nerve.agent.backends import BackendDeps, SessionSpec, TransportDiedError +from nerve.agent.backends import events as ev +from nerve.agent.backends.base import TurnInput +from nerve.agent.backends.codex import CodexBackend, lifecycle +from nerve.agent.backends.codex.appserver import CodexAppServerClient +from nerve.agent.interactive import InteractiveToolHandler +from nerve.config import NerveConfig + +FIX = Path(__file__).parent / "fixtures" +ESCAPER = str(FIX / "lifecycle_escaper.py") +FAKE = str(FIX / "fake_codex_appserver.py") + +CAPS = lifecycle.detect_capabilities() +requires_scope = pytest.mark.skipif( + not CAPS.ok, reason=f"systemd user scope unavailable: {CAPS.reason}", +) + + +# ----------------------------- helpers/fixtures ---------------------------- # + + +def _alive(pid: int) -> bool: + try: + os.kill(pid, 0) + return True + except ProcessLookupError: + return False + except PermissionError: + return True + + +@pytest.fixture +def scopes(): + """Guaranteed teardown of every scope/process a test creates.""" + reg: dict[str, set] = {"units": set(), "tokens": set(), "run_dirs": set()} + try: + yield reg + finally: + for rd in reg["run_dirs"]: + try: + rec = lifecycle.read_record(Path(rd)) + if rec is not None: + reg["units"].add(rec.unit) + except Exception: # noqa: BLE001 + pass + for unit in reg["units"]: + subprocess.run( + ["systemctl", "--user", "stop", unit], + capture_output=True, timeout=15, + ) + for tok in reg["tokens"]: + subprocess.run( + [sys.executable, ESCAPER, "sweep", tok], + env={**os.environ, "ESCAPER_TOKEN": tok}, + capture_output=True, timeout=15, + ) + + +def _launch_escaper( + scopes, run_dir: Path, run_id: str, *, variant: str = "", + term_grace: int = 2, mode: str = "strict", + session_id: str | None = None, invocation_id: str | None = None, +) -> tuple[lifecycle.LifecycleRecord, int, str]: + """Launch a real escaper inside a real scope; return (record, gc_pid, unit). + + The escaper's main process exits after the setsid/double-fork grandchild is + up, so the grandchild is left orphaned inside the scope — exactly the + incident shape. A strict lifecycle record is written and (unless overridden) + its live InvocationID/ControlGroup recorded, so ``reap`` can act on it. + """ + run_dir = Path(run_dir) + run_dir.mkdir(parents=True, exist_ok=True) + token = "lc" + uuid.uuid4().hex[:12] + scopes["tokens"].add(token) + nonce = uuid.uuid4().hex + unit = lifecycle.scope_unit_name(run_id, nonce) + scopes["units"].add(unit) + cmd = "spawn-escapee" + (f"-{variant}" if variant else "") + inner = [sys.executable, ESCAPER, cmd, str(run_dir)] + argv = lifecycle.build_scope_argv(unit, inner, term_grace_seconds=term_grace) + subprocess.run( + argv, env={**os.environ, "ESCAPER_TOKEN": token}, + capture_output=True, timeout=30, check=True, + ) + gc_pid = int((run_dir / "grandchild.pid").read_text()) + inst = lifecycle.current_instance() + rec = lifecycle.LifecycleRecord( + run_id=run_id, session_id=session_id or f"workflow:{run_id}", mode=mode, + attempt_nonce=nonce, unit=unit, workspace_id="ws", boot_id=inst.boot_id, + instance_generation=inst.generation, phase="intent", + ) + lifecycle.write_record(run_dir, rec) + if invocation_id is not None: + # Caller wants a deliberately mismatched identity (replacement test). + _inv, cg = lifecycle._await_unit_identity(unit) + rec.invocation_id = invocation_id + rec.control_group = cg + rec.phase = "launched" + lifecycle.write_record(run_dir, rec) + else: + rec = lifecycle.record_launched(run_dir, rec) + return rec, gc_pid, unit + + +# ---- backend/turn helpers (mirroring tests/test_codex_appserver.py) ------- # + + +def _wf_config(tmp_path: Path, *, mode: str = "strict", **codex) -> NerveConfig: + cfg = NerveConfig.from_dict({ + "workspace": str(tmp_path / "ws"), + "workflows": {"runs_dir": str(tmp_path / "workflow-runs")}, + "codex": { + "bin_path": FAKE, + "home_dir": str(tmp_path / "codex-home"), + "model": "gpt-5.6-sol", + "lifecycle": { + "mode": mode, "term_grace_seconds": 2, "stop_timeout_seconds": 8, + }, + **codex, + }, + }) + (tmp_path / "ws").mkdir(parents=True, exist_ok=True) + (tmp_path / "workflow-runs").mkdir(parents=True, exist_ok=True) + return cfg + + +def _deps(cfg: NerveConfig) -> BackendDeps: + return BackendDeps( + config=lambda: cfg, db=None, registry=None, + tool_ctx_factory=lambda sid: None, external_mcp_servers=lambda: [], + gateway_port=lambda: 8900, mint_session_token=lambda sid: f"tok-{sid}", + ) + + +def _spec(cfg: NerveConfig, *, session_id: str, source: str) -> SessionSpec: + hub = InteractiveToolHandler( + session_id=session_id, broadcast_fn=_noop_broadcast, + interactive_capable=False, + ) + return SessionSpec( + session_id=session_id, source=source, model=None, effort="high", + system_prompt="You are Nerve.", cwd=str(cfg.workspace), + resume_native_id=None, fork=False, interactive=hub, + snapshot=None, record_wakeup=None, idle_timeout=15.0, + ) + + +async def _noop_broadcast(session_id: str, message: dict) -> None: + return None + + +async def _collect_turn(client) -> list: + return [event async for event in client.receive_turn()] + + +# =========================================================================== # +# R6.1 — adapter + fake app-server through the real containment wrapper +# =========================================================================== # + + +@requires_scope +@pytest.mark.asyncio +async def test_r61_strict_launch_roundtrip_preserves_argv_env_cwd(tmp_path, scopes): + """initialize round-trips through systemd-run --scope with clean JSON + stdout; literal ``${...}`` argv, env, and cwd are preserved; the app-server + is verifiably contained and registration is durable.""" + run_dir = tmp_path / "workflow-runs" / "wfr-r61" + scopes["run_dirs"].add(run_dir) + containment = lifecycle.WorkflowContainment( + mode="strict", run_dir=run_dir, run_id="wfr-r61", + session_id="workflow:wfr-r61", workspace_id="ws", + term_grace_seconds=2, stop_timeout_seconds=8, + ) + literal = "keep-${NERVE_SENTINEL_DO_NOT_EXPAND}-literal" + cwd = tmp_path / "ws" + cwd.mkdir(parents=True, exist_ok=True) + + async def _handler(method, params): # pragma: no cover - unused here + return {} + + client = CodexAppServerClient( + bin_path=FAKE, cwd=str(cwd), + env={**os.environ, "CODEX_HOME": str(tmp_path / "home"), + "FAKE_CODEX_MODE": "basic"}, + server_request_handler=_handler, + config_overrides=[f"sentinel={literal}"], + containment=containment, + ) + resp = await client.start() + try: + # Clean JSON on stdout (no systemd-run control text) — it parsed. + assert resp.get("userAgent") + # Literal argv preserved verbatim through --expand-environment=no. + assert any(literal in v for v in resp["_fake"]["configOverrides"]) + # Environment forwarded into the scope. + assert resp["_fake"]["env"]["CODEX_HOME"] == str(tmp_path / "home") + + rec = lifecycle.read_record(run_dir) + assert rec is not None and rec.phase == "registered" + assert rec.invocation_id and rec.control_group + # The scope is populated (a live contained app-server). + _state, populated = lifecycle._cgroup_events(rec.control_group) + assert populated is True + # cwd preserved: a scope member reports our cwd. + procs = Path("/sys/fs/cgroup" + rec.control_group + "/cgroup.procs") + pids = [int(p) for p in procs.read_text().split()] + assert pids + assert any( + os.readlink(f"/proc/{p}/cwd") == str(cwd) + for p in pids if Path(f"/proc/{p}").exists() + ) + finally: + await client.close() + + +@requires_scope +@pytest.mark.asyncio +@pytest.mark.parametrize("mode", ["basic", "big_line"]) +async def test_r61_contained_turn_streams_including_large_records( + tmp_path, monkeypatch, scopes, mode, +): + """A full turn (incremental notifications; a ~2 MiB single-line tool result + for backpressure) completes through the containment wrapper unchanged.""" + monkeypatch.setenv("FAKE_CODEX_MODE", mode) + cfg = _wf_config(tmp_path) + scopes["run_dirs"].add(tmp_path / "workflow-runs" / "wfr-turn") + backend = CodexBackend(_deps(cfg)) + client = await backend.create_client( + _spec(cfg, session_id="workflow:wfr-turn", source="workflow"), + ) + try: + rec = lifecycle.read_record(tmp_path / "workflow-runs" / "wfr-turn") + assert rec is not None and rec.phase == "registered" + await client.start_turn(TurnInput(text="hi")) + events = await _collect_turn(client) + assert any(isinstance(e, ev.TurnCompleted) for e in events) + assert events[-1].status == "completed" # transport survived the turn + finally: + await client.disconnect() + + +@requires_scope +@pytest.mark.asyncio +async def test_r61_contained_appserver_death_propagates_exit(tmp_path, monkeypatch, scopes): + """die_mid_turn: the app-server exits mid-turn; the exit propagates through + the wrapper (transport dies) rather than hanging.""" + monkeypatch.setenv("FAKE_CODEX_MODE", "die_mid_turn") + cfg = _wf_config(tmp_path) + scopes["run_dirs"].add(tmp_path / "workflow-runs" / "wfr-die") + backend = CodexBackend(_deps(cfg)) + client = await backend.create_client( + _spec(cfg, session_id="workflow:wfr-die", source="workflow"), + ) + try: + await client.start_turn(TurnInput(text="hi")) + with pytest.raises(TransportDiedError): + await _collect_turn(client) + finally: + await client.disconnect() + + +# =========================================================================== # +# R6.2 — success/error/cancel/timeout/budget converge on one reap; foreign runs survive +# =========================================================================== # + + +@requires_scope +def test_r62_reap_kills_contained_escapee_and_spares_foreign(tmp_path, scopes): + target, gc_t, _u = _launch_escaper(scopes, tmp_path / "t", "wfr-target") + foreign, gc_f, _uf = _launch_escaper(scopes, tmp_path / "f", "wfr-foreign") + + # Both escaped grandchildren are alive and each is contained in its scope. + assert _alive(gc_t) and _alive(gc_f) + assert lifecycle.proc_cgroup(gc_t) == target.control_group + assert lifecycle.proc_cgroup(gc_f) == foreign.control_group + + receipt = lifecycle.reap(tmp_path / "t", term_grace_seconds=2, stop_timeout_seconds=8) + assert receipt.outcome == "complete" + assert receipt.signalled and receipt.verified_empty + assert not _alive(gc_t) # target's escapee is reaped + assert _alive(gc_f) # the foreign run is untouched + + # The receipt is persisted terminal on the record. + rec = lifecycle.read_record(tmp_path / "t") + assert rec.phase == "terminal" and rec.receipt["outcome"] == "complete" + + +# =========================================================================== # +# R6.3 — nested cgroups, TERM-ignoring, fork-during-teardown; recursive populated +# =========================================================================== # + + +@requires_scope +def test_r63_nested_cgroup_uses_recursive_populated(tmp_path, scopes): + rec, gc, _u = _launch_escaper(scopes, tmp_path / "n", "wfr-nested", variant="nested") + # The grandchild migrated into a child cgroup: the scope ROOT holds no + # procs, yet recursive cgroup.events populated is 1 (the emptiness signal + # root cgroup.procs alone would miss). + root_procs = Path("/sys/fs/cgroup" + rec.control_group + "/cgroup.procs").read_text().split() + assert root_procs == [] + _state, populated = lifecycle._cgroup_events(rec.control_group) + assert populated is True + assert lifecycle.proc_cgroup(gc) == rec.control_group + "/nested" + + receipt = lifecycle.reap(tmp_path / "n", term_grace_seconds=2, stop_timeout_seconds=8) + assert receipt.outcome == "complete" and receipt.verified_empty + assert not _alive(gc) + + +@requires_scope +def test_r63_term_ignoring_descendant_escalated_to_kill(tmp_path, scopes): + rec, gc, _u = _launch_escaper( + scopes, tmp_path / "ig", "wfr-igterm", variant="igterm", term_grace=2, + ) + assert _alive(gc) + receipt = lifecycle.reap(tmp_path / "ig", term_grace_seconds=2, stop_timeout_seconds=10) + # A descendant that ignores TERM is still killed by the bounded + # TERM→grace→SIGKILL sequence (SendSIGKILL=yes, KillMode=control-group). + assert receipt.outcome == "complete" and receipt.verified_empty + assert not _alive(gc) + + +@requires_scope +def test_r63_fork_during_teardown_all_reaped(tmp_path, scopes): + rec, gc, _u = _launch_escaper( + scopes, tmp_path / "fk", "wfr-forker", variant="forker", term_grace=2, + ) + token_before = _scope_proc_count(rec.control_group) + assert token_before >= 1 + receipt = lifecycle.reap(tmp_path / "fk", term_grace_seconds=2, stop_timeout_seconds=10) + # Children forked *during* teardown are in the cgroup too, so the + # control-group kill takes them all — verified by recursive emptiness. + assert receipt.outcome == "complete" and receipt.verified_empty + assert not _alive(gc) + + +def _scope_proc_count(control_group: str) -> int: + total = 0 + root = Path("/sys/fs/cgroup" + control_group) + for procs in root.rglob("cgroup.procs"): + try: + total += len(procs.read_text().split()) + except OSError: + pass + return total + + +# =========================================================================== # +# R6.4 — crash windows + fresh-process reconcile +# =========================================================================== # + + +@requires_scope +def test_r64_reconcile_reaps_abandoned_launch_from_fresh_generation(tmp_path, scopes): + """A launch whose controller 'crashed' (record left at 'launched', a + different daemon generation) is reaped by startup reconciliation reading + only the durable record.""" + rec, gc, _u = _launch_escaper(scopes, tmp_path / "c", "wfr-crash") + assert rec.phase == "launched" and _alive(gc) + # Reconcile as a *fresh* daemon generation (the crashed one is gone). + receipt = lifecycle.reconcile_run( + tmp_path / "c", is_terminal=True, + active_generation="fresh-other-generation", + term_grace_seconds=2, stop_timeout_seconds=8, + ) + assert receipt is not None and receipt.outcome == "complete" + assert not _alive(gc) + + +def test_r64_reconcile_skips_current_generation_owner(tmp_path): + """Reconcile never touches a run this very generation still owns.""" + run_dir = tmp_path / "own" + run_dir.mkdir() + inst = lifecycle.current_instance() + rec = lifecycle.LifecycleRecord( + run_id="wfr-own", session_id="workflow:wfr-own", mode="strict", + attempt_nonce="n1", unit="nerve-wf-wfr-own-n1.scope", workspace_id="ws", + boot_id=inst.boot_id, instance_generation=inst.generation, phase="launched", + ) + lifecycle.write_record(run_dir, rec) + out = lifecycle.reconcile_run( + run_dir, is_terminal=False, active_generation=inst.generation, + ) + assert out is None # active owner, same generation → left alone + + +def test_r64_intent_only_abandoned_launch_resolves_absent(tmp_path): + """A pre-launch crash (intent recorded, scope never created) reconciles to + resolved_absent — nothing to signal, no false completion of a live tree.""" + run_dir = tmp_path / "int" + run_dir.mkdir() + inst = lifecycle.current_instance() + rec = lifecycle.LifecycleRecord( + run_id="wfr-int", session_id="workflow:wfr-int", mode="strict", + attempt_nonce="n2", unit="nerve-wf-wfr-int-n2.scope", workspace_id="ws", + boot_id=inst.boot_id, instance_generation="old-generation", phase="intent", + ) + lifecycle.write_record(run_dir, rec) + receipt = lifecycle.reconcile_run( + run_dir, is_terminal=False, active_generation=inst.generation, + ) + assert receipt is not None + assert receipt.outcome == "resolved_absent" and not receipt.signalled + + +# =========================================================================== # +# R6.5 — identity: replacement, cross-boot, corrupt, unwritable/fsync, manager, no PID signal +# =========================================================================== # + + +@requires_scope +def test_r65_refuses_replaced_unit_and_spares_it(tmp_path, scopes): + """When the recorded InvocationID no longer matches the live unit (a + same-name replacement), reap refuses to signal — the foreign process that + now owns the name survives.""" + rec, gc, _u = _launch_escaper( + scopes, tmp_path / "rep", "wfr-rep", + invocation_id="00000000000000000000000000000000", # deliberately wrong + ) + assert _alive(gc) + receipt = lifecycle.reap(tmp_path / "rep", term_grace_seconds=2, stop_timeout_seconds=8) + assert receipt.outcome == "failed_replaced" + assert not receipt.signalled and not receipt.is_complete() + assert receipt.needs_attention() + assert _alive(gc) # the (foreign) unit was NOT signalled + + +def test_r65_cross_boot_record_never_signals(tmp_path, monkeypatch): + run_dir = tmp_path / "boot" + run_dir.mkdir() + rec = lifecycle.LifecycleRecord( + run_id="wfr-boot", session_id="workflow:wfr-boot", mode="strict", + attempt_nonce="n3", unit="nerve-wf-wfr-boot-n3.scope", workspace_id="ws", + boot_id="0000-prior-boot", instance_generation="old", phase="registered", + invocation_id="abc", control_group="/user.slice/nerve-wf-wfr-boot-n3.scope", + ) + lifecycle.write_record(run_dir, rec) + # No unit lookup / no signal should occur for a prior-boot record. + monkeypatch.setattr(lifecycle, "_scope_stop", _fail_if_called) + receipt = lifecycle.reap(run_dir) + assert receipt.outcome == "resolved_prior_boot" and not receipt.signalled + + +def test_r65_corrupt_record_escalates_without_signal(tmp_path, monkeypatch): + run_dir = tmp_path / "corrupt" + run_dir.mkdir() + (run_dir / lifecycle.RECORD_NAME).write_text("{ this is not valid json ") + monkeypatch.setattr(lifecycle, "_scope_stop", _fail_if_called) + receipt = lifecycle.reap(run_dir) + assert receipt.outcome == "pending_retry" and not receipt.is_complete() + + +def test_r65_unpersistable_receipt_never_reports_complete(tmp_path, monkeypatch): + """If the completion receipt cannot be fsynced/written, reap must not + report complete (R2).""" + run_dir = tmp_path / "fsync" + run_dir.mkdir() + inst = lifecycle.current_instance() + rec = lifecycle.LifecycleRecord( + run_id="wfr-fs", session_id="workflow:wfr-fs", mode="strict", + attempt_nonce="n4", unit="nerve-wf-wfr-fs-n4.scope", workspace_id="ws", + boot_id=inst.boot_id, instance_generation="old", phase="launched", + invocation_id="inv", control_group="/user.slice/nerve-wf-wfr-fs-n4.scope", + ) + lifecycle.write_record(run_dir, rec) + # Unit resolves + stop succeeds + scope verified empty … + monkeypatch.setattr(lifecycle, "_unit_identity", lambda u: { + "LoadState": "loaded", "ActiveState": "active", + "InvocationID": "inv", "ControlGroup": rec.control_group, + "Description": lifecycle.SCOPE_DESCRIPTION, + }) + monkeypatch.setattr(lifecycle, "_scope_stop", lambda u, timeout: (True, "")) + monkeypatch.setattr(lifecycle, "_cgroup_events", lambda cg: ("gone", None)) + # … but persisting the receipt fails. + monkeypatch.setattr(os, "fsync", _raise_oserror) + with pytest.raises((OSError, lifecycle.LifecycleError)): + lifecycle.reap(run_dir) + # On-disk record must NOT claim completion. + on_disk = json.loads((run_dir / lifecycle.RECORD_NAME).read_text()) + assert (on_disk.get("receipt") or {}).get("outcome") != "complete" + + +def test_r65_manager_unavailable_stays_pending_without_pid_signal(tmp_path, monkeypatch): + run_dir = tmp_path / "mgr" + run_dir.mkdir() + inst = lifecycle.current_instance() + rec = lifecycle.LifecycleRecord( + run_id="wfr-mgr", session_id="workflow:wfr-mgr", mode="strict", + attempt_nonce="n5", unit="nerve-wf-wfr-mgr-n5.scope", workspace_id="ws", + boot_id=inst.boot_id, instance_generation="old", phase="launched", + invocation_id="inv", control_group="/user.slice/nerve-wf-wfr-mgr-n5.scope", + ) + lifecycle.write_record(run_dir, rec) + monkeypatch.setattr(lifecycle, "_unit_identity", lambda u: { + "LoadState": "loaded", "ActiveState": "active", "InvocationID": "inv", + "ControlGroup": rec.control_group, "Description": lifecycle.SCOPE_DESCRIPTION, + }) + monkeypatch.setattr(lifecycle, "_scope_stop", lambda u, timeout: (False, "manager down")) + monkeypatch.setattr(lifecycle, "_cgroup_events", lambda cg: ("present", True)) + # The reap must never fall back to signalling a raw PID / process group. + monkeypatch.setattr(os, "kill", _raise_if_called) + monkeypatch.setattr(os, "killpg", _raise_if_called) + receipt = lifecycle.reap(run_dir) + assert receipt.outcome == "pending_retry" and not receipt.is_complete() + assert receipt.needs_retry() + + +def _fail_if_called(*a, **k): + raise AssertionError("scope stop must not be called for this case") + + +def _raise_if_called(*a, **k): + raise AssertionError("a raw PID/process-group signal must never be sent") + + +def _raise_oserror(*a, **k): + raise OSError("simulated fsync failure") + + +# =========================================================================== # +# R6.6 — concurrency + repeated terminal cleanup + real durable retry +# =========================================================================== # + + +@requires_scope +def test_r66_repeated_reap_is_idempotent(tmp_path, scopes, monkeypatch): + rec, gc, _u = _launch_escaper(scopes, tmp_path / "idem", "wfr-idem") + r1 = lifecycle.reap(tmp_path / "idem", term_grace_seconds=2, stop_timeout_seconds=8) + assert r1.outcome == "complete" + # A second reap must not stop anything again — the terminal receipt stands. + monkeypatch.setattr(lifecycle, "_scope_stop", _fail_if_called) + r2 = lifecycle.reap(tmp_path / "idem") + assert r2.outcome == "complete" and r2.attempts == r1.attempts + + +@requires_scope +def test_r66_initial_failure_then_real_durable_retry(tmp_path, scopes, monkeypatch): + rec, gc, _u = _launch_escaper(scopes, tmp_path / "retry", "wfr-retry") + real_stop = lifecycle._scope_stop + monkeypatch.setattr(lifecycle, "_scope_stop", lambda u, timeout: (False, "transient manager glitch")) + first = lifecycle.reap(tmp_path / "retry", term_grace_seconds=2, stop_timeout_seconds=8) + assert first.outcome == "pending_retry" and _alive(gc) + # Restore the manager and use the supported retry entry — a REAL reap. + monkeypatch.setattr(lifecycle, "_scope_stop", real_stop) + second = lifecycle.retry(tmp_path / "retry", term_grace_seconds=2, stop_timeout_seconds=8) + assert second.outcome == "complete" and second.verified_empty + assert second.attempts == 2 # durable attempt count advanced + assert not _alive(gc) + + +@requires_scope +def test_r66_concurrent_reaps_serialize(tmp_path, scopes): + rec, gc, _u = _launch_escaper(scopes, tmp_path / "conc", "wfr-conc") + results: list[lifecycle.Receipt] = [] + barrier = threading.Barrier(3) + + def _worker(): + barrier.wait() + results.append(lifecycle.reap(tmp_path / "conc", term_grace_seconds=2, stop_timeout_seconds=8)) + + threads = [threading.Thread(target=_worker) for _ in range(3)] + for t in threads: + t.start() + for t in threads: + t.join(timeout=30) + # The per-owner lock serialises them: all agree the tree is gone, and the + # scope was stopped exactly once (attempts never double-counts). + assert results and all(r.is_complete() for r in results) + assert not _alive(gc) + assert lifecycle.read_record(tmp_path / "conc").receipt["outcome"] == "complete" + + +# =========================================================================== # +# R6.7 — interactive + a second workflow survive while the target is cleaned +# =========================================================================== # + + +@requires_scope +def test_r67_only_target_is_reaped_no_broad_kill(tmp_path, scopes, monkeypatch): + target, gc_t, _ = _launch_escaper(scopes, tmp_path / "wt", "wfr-victim") + other_wf, gc_o, _ = _launch_escaper(scopes, tmp_path / "wo", "wfr-other") + interactive, gc_i, _ = _launch_escaper(scopes, tmp_path / "wi", "wfr-interactive") + + monkeypatch.setattr(os, "killpg", _raise_if_called) # no broad process-group kill + receipt = lifecycle.reap(tmp_path / "wt", term_grace_seconds=2, stop_timeout_seconds=8) + assert receipt.outcome == "complete" + assert not _alive(gc_t) + assert _alive(gc_o) and _alive(gc_i) # concurrent workflow + interactive survive + + +# =========================================================================== # +# R6.8 — disabled / observe / strict-unavailable declared behaviour +# =========================================================================== # + + +def test_r68_disabled_mode_builds_no_containment(tmp_path): + cfg = _wf_config(tmp_path, mode="disabled") + backend = CodexBackend(_deps(cfg)) + from nerve.agent.backends.codex.backend import _workflow_containment + spec = _spec(cfg, session_id="workflow:wfr-x", source="workflow") + assert _workflow_containment(backend, spec) is None + + +def test_r68_non_workflow_session_builds_no_containment(tmp_path): + cfg = _wf_config(tmp_path, mode="strict") + backend = CodexBackend(_deps(cfg)) + from nerve.agent.backends.codex.backend import _workflow_containment + # An interactive/cron session is never contained, even in strict mode. + spec = _spec(cfg, session_id="s-interactive", source="web") + assert _workflow_containment(backend, spec) is None + + +def test_r68_observe_mode_does_not_wrap_and_makes_no_completion_claim(tmp_path): + run_dir = tmp_path / "obs" + run_dir.mkdir() + containment = lifecycle.WorkflowContainment( + mode="observe", run_dir=run_dir, run_id="wfr-obs", + session_id="workflow:wfr-obs", workspace_id="ws", + ) + inner = ["/usr/bin/codex", "app-server", "--listen", "stdio://"] + argv, rec = lifecycle.prepare_launch(containment, inner) + assert argv == inner # NOT wrapped — today's behaviour preserved + assert rec.mode == "observe" + receipt = lifecycle.reap(run_dir) + assert receipt.outcome == "unenforced_observe" + assert not receipt.enforced and not receipt.signalled + assert not receipt.is_complete() # observe is not a resolution + + +def test_r68_strict_unavailable_fails_before_exec(tmp_path, monkeypatch): + run_dir = tmp_path / "unavail" + run_dir.mkdir() + monkeypatch.setattr(lifecycle, "detect_capabilities", lambda **k: lifecycle.Capabilities( + ok=False, systemd_run=False, systemctl=False, user_manager=False, + cgroup_v2=True, reason="systemd-run not found", + )) + containment = lifecycle.WorkflowContainment( + mode="strict", run_dir=run_dir, run_id="wfr-un", + session_id="workflow:wfr-un", workspace_id="ws", + ) + with pytest.raises(lifecycle.ContainmentUnavailable): + lifecycle.prepare_launch(containment, ["/usr/bin/codex", "app-server"]) + # Nothing was launched, and no half-written record claims containment. + assert not (run_dir / lifecycle.RECORD_NAME).exists() + + +# =========================================================================== # +# Record validation / identity hardening (R2 unit-level) +# =========================================================================== # + + +def test_record_rejects_unsafe_unit_and_foreign_cgroup(tmp_path): + with pytest.raises(lifecycle.LifecycleError): + lifecycle.LifecycleRecord( + run_id="r", session_id="workflow:r", mode="strict", attempt_nonce="n", + unit="evil; rm -rf /.scope", workspace_id="ws", boot_id="b", + instance_generation="g", + ).validate() + with pytest.raises(lifecycle.LifecycleError): + lifecycle.LifecycleRecord( + run_id="r", session_id="workflow:r", mode="strict", attempt_nonce="n", + unit="nerve-wf-r-n.scope", workspace_id="ws", boot_id="b", + instance_generation="g", + control_group="/system.slice/other.scope", # not our unit / not user slice + ).validate() + + +def test_atomic_write_is_durable_and_survives_bad_tmp(tmp_path): + p = tmp_path / "rec.json" + lifecycle._atomic_write_json(p, {"a": 1}) + assert json.loads(p.read_text())["a"] == 1 + lifecycle._atomic_write_json(p, {"a": 2}) # overwrite + assert json.loads(p.read_text())["a"] == 2 + # no leftover temp files + assert list(tmp_path.glob(".rec.json*")) == [] From e37d8ffd6d4775e79b1218afae6f41abec7fdbe4 Mon Sep 17 00:00:00 2001 From: serxa Date: Fri, 18 Sep 2026 17:42:15 +0000 Subject: [PATCH 2/6] codex lifecycle: cut scope, drop observe/retry/CLI, trim comments Per review, reduce the change to the essential fix: - drop `observe` mode and the retry CLI/service entry; reconciliation reaps prior-generation scopes on the next startup, so a stray pending_retry recovers there. - reconcile by daemon-generation fence alone (no DB terminality snapshot). - collapse receipt outcomes to complete/pending_retry/refused/no_scope; drop the extra record fields (mode/phase/nonce/workspace_id) and helpers. - comments now describe how the code works, not why it was bounded; rationale lives in the PR description. - consolidate tests to the load-bearing cases (real scopes/escapers). Co-Authored-By: Claude Opus 4.8 --- nerve/agent/backends/codex/appserver.py | 42 +- nerve/agent/backends/codex/backend.py | 23 +- nerve/agent/backends/codex/lifecycle.py | 866 ++++++------------------ nerve/cli.py | 34 - nerve/config.py | 50 +- nerve/workflows/service.py | 127 +--- tests/fixtures/lifecycle_escaper.py | 10 - tests/test_codex_lifecycle.py | 679 ++++--------------- tmp/pr-body.md | 35 + 9 files changed, 417 insertions(+), 1449 deletions(-) create mode 100644 tmp/pr-body.md diff --git a/nerve/agent/backends/codex/appserver.py b/nerve/agent/backends/codex/appserver.py index 20c3bb7c8..147b55ada 100644 --- a/nerve/agent/backends/codex/appserver.py +++ b/nerve/agent/backends/codex/appserver.py @@ -184,10 +184,9 @@ async def start(self) -> dict: args.extend(["--config", kv]) args.extend(["app-server", "--listen", "stdio://"]) - # Workflow-run containment: in strict mode `prepare_launch` returns the - # scope-wrapped argv so the whole descendant tree is contained *before* - # the command execs (and raises, un-launched, if containment can't be - # established). In observe mode it records intent but leaves argv as-is. + # For a contained workflow run, `prepare_launch` records the owner and + # returns the scope-wrapped argv; it raises before exec if the host + # can't contain the run. containment = self._containment if containment is not None and containment.enabled: try: @@ -217,15 +216,9 @@ async def start(self) -> dict: f"Failed to spawn codex app-server ({self._bin_path}): {e}" ) from e - # Record the scope's immutable identity (InvocationID + resolved - # cgroup) now that systemd-run has created it. Failure here means the - # contained launch cannot be verified — fail the session (the scope, - # already created, is still reapable from the persisted intent). - if ( - containment is not None - and containment.mode == lifecycle.MODE_STRICT - and self._lifecycle_record is not None - ): + # Record the scope's InvocationID + resolved cgroup now that systemd-run + # created it. + if containment is not None and containment.enabled and self._lifecycle_record is not None: try: self._lifecycle_record = await asyncio.to_thread( lifecycle.record_launched, @@ -262,34 +255,21 @@ async def start(self) -> dict: await self.close() raise - # Membership handshake: confirm the app-server is actually inside its - # scope cgroup before any work is dispatched. Registration is made - # durable so a crash after here is reconcilable. + # Confirm the app-server is inside its scope cgroup before dispatching. if ( - containment is not None - and containment.mode == lifecycle.MODE_STRICT + containment is not None and containment.enabled and self._lifecycle_record is not None - and self._proc is not None - and self._proc.pid is not None + and self._proc is not None and self._proc.pid is not None ): - record = self._lifecycle_record member = await asyncio.to_thread( - lifecycle.verify_membership, record.control_group, self._proc.pid, + lifecycle.verify_membership, + self._lifecycle_record.control_group, self._proc.pid, ) if not member: await self.close() raise TransportDiedError( "codex app-server is not contained in its cgroup scope" ) - try: - self._lifecycle_record = await asyncio.to_thread( - lifecycle.record_registered, containment.run_dir, record, - ) - except (lifecycle.LifecycleError, OSError) as e: - await self.close() - raise TransportDiedError( - f"codex workflow scope registration failed: {e}" - ) from e return response def is_alive(self) -> bool: diff --git a/nerve/agent/backends/codex/backend.py b/nerve/agent/backends/codex/backend.py index 9bd2c4f71..cac485957 100644 --- a/nerve/agent/backends/codex/backend.py +++ b/nerve/agent/backends/codex/backend.py @@ -660,30 +660,17 @@ def resolve_api_key(self) -> str | None: def _workflow_containment( backend: "CodexBackend", spec: SessionSpec, ) -> lifecycle.WorkflowContainment | None: - """Build per-run cgroup containment for a Codex *workflow* run, else None. - - Gated on the ``workflow:``-prefixed session id (set deterministically by - the workflow service) and ``codex.lifecycle.mode``. Interactive/cron Codex - sessions and ``disabled`` mode always get None, so their launch is - unchanged. - """ + """cgroup containment for a Codex ``workflow:`` session in strict mode, else + None (interactive/cron sessions and disabled mode launch unchanged).""" sid = spec.session_id or "" if not sid.startswith(_WORKFLOW_SESSION_PREFIX): return None - lc = backend.codex.lifecycle - if lc.mode not in (lifecycle.MODE_OBSERVE, lifecycle.MODE_STRICT): + if backend.codex.lifecycle.mode != lifecycle.MODE_STRICT: return None - cfg = backend.config run_id = sid[len(_WORKFLOW_SESSION_PREFIX):] - run_dir = Path(cfg.workflows.runs_dir).expanduser() / run_id + run_dir = Path(backend.config.workflows.runs_dir).expanduser() / run_id return lifecycle.WorkflowContainment( - mode=lc.mode, - run_dir=run_dir, - run_id=run_id, - session_id=sid, - workspace_id=str(cfg.workspace), - term_grace_seconds=lc.term_grace_seconds, - stop_timeout_seconds=lc.stop_timeout_seconds, + mode=lifecycle.MODE_STRICT, run_dir=run_dir, run_id=run_id, session_id=sid, ) diff --git a/nerve/agent/backends/codex/lifecycle.py b/nerve/agent/backends/codex/lifecycle.py index 6a4d20f2b..6611ae66e 100644 --- a/nerve/agent/backends/codex/lifecycle.py +++ b/nerve/agent/backends/codex/lifecycle.py @@ -1,60 +1,15 @@ -"""Descendant containment + reaping for Codex **workflow** runs. - -Why this exists ---------------- -A Codex workflow run spawns ``codex app-server``, which spawns Ultracode -workers, which spawn ``codex-linux-sandbox`` processes. The sandbox -``setsid``\\ s into its own session/process-group and may double-fork, so a -descendant ends up in a process group that is *not* the app-server's. -:func:`CodexAppServerClient._signal_process_tree` signals a single process -group (``os.killpg(proc.pid, sig)``); it structurally cannot reach those -escaped descendants. When a run completes, is cancelled, hits its budget, or -the daemon crashes, the escaped processes survive as orphans (reparented to -PID 1). This module contains a run's whole descendant tree in a dedicated -**cgroup v2 scope** so a single ``systemctl --user stop`` reaps all of it — -setsid/double-fork and all — and records a durable, validated receipt so the -cleanup is repeatable and survives a daemon restart. - -Scope of this first implementation (deliberately bounded) ---------------------------------------------------------- -* **Codex workflow runs only.** Interactive/cron Codex sessions and Claude - runs never engage this module. -* **Opt-in, three modes** (``codex.lifecycle.mode``): - - - ``disabled`` (default): this module is inert. Nothing changes. - - ``observe``: **UNENFORCED.** The launch is *not* wrapped (today's - behaviour is preserved) — a durable record is written for staged-rollout - visibility, and teardown writes an explicitly *unenforced* observation. - It never claims a cleanup it did not perform. - - ``strict``: the app-server is launched **inside a cgroup scope created - before the command execs** (no late ``cgroup.procs`` migration, no - threaded ``preexec_fn``). If containment cannot be established the launch - **fails before the command executes** — there is no silent downgrade to - ``killpg``/subreaper. - -* **Not a security boundary.** cgroup inheritance contains *trusted* tool - processes that fork/setsid/double-fork. A cooperating or malicious same-UID - process can still migrate out via the user manager or a writable cgroup, or - tamper with receipts. Obtaining that stronger property is explicitly out of - scope; no privilege is broadened to get it. - -Honest contracts (no false total-containment) ----------------------------------------------- -* Graceful terminal reap = bounded ``TERM`` → grace → ``KILL`` via - ``systemctl --user stop`` (``KillMode=control-group``, ``SendSIGKILL=yes``, - bounded ``TimeoutStopSec``), then emptiness is **verified** (the scope's - recursive ``cgroup.events`` ``populated=0``, or the scope cgroup gone after a - *successful* stop). A stop request is not itself proof of exit. -* A daemon ``SIGKILL`` leaves contained children alive until restart - reconciliation — no in-process ``finally`` survives ``SIGKILL``. -* A *missing* scope is only success when we successfully stopped it (or the - host rebooted, which no PID survives). After an access error, a unit - replacement, or an unproven recovery it stays ``pending_retry``. - -The durable store is per-run ``//lifecycle.json`` (atomic -write + file & directory fsync). No raw argv, credentials, or environment ever -enter a record or an error string — only unit name, systemd ``InvocationID``, -resolved cgroup path, and identity metadata. +"""Cgroup containment + reaping for Codex workflow-run descendants. + +A Codex workflow run's ``codex-linux-sandbox`` descendants ``setsid`` into their +own process groups, so the app-server teardown's single ``killpg`` cannot reach +them and they survive as orphans. When ``codex.lifecycle.mode`` is ``strict``, +a workflow run's app-server is launched inside a delegated systemd user scope so +the whole descendant tree lives in one cgroup and a single ``systemctl --user +stop`` reaps it; a durable per-run record lets the reap run at terminal or from +startup reconciliation after a crash. Default ``disabled`` leaves launch +unchanged. Requires Linux + a reachable systemd ``--user`` manager + cgroup v2; +strict fails before exec where that is absent (it never launches un-contained). +This contains trusted tool processes; it is not a security sandbox. """ from __future__ import annotations @@ -69,73 +24,48 @@ import subprocess import time import uuid -from dataclasses import asdict, dataclass, field +from dataclasses import asdict, dataclass from pathlib import Path from typing import Callable, Iterable logger = logging.getLogger(__name__) -# -- modes ----------------------------------------------------------------- # - MODE_DISABLED = "disabled" -MODE_OBSERVE = "observe" MODE_STRICT = "strict" -VALID_MODES = (MODE_DISABLED, MODE_OBSERVE, MODE_STRICT) - -# -- durable store filenames ----------------------------------------------- # +VALID_MODES = (MODE_DISABLED, MODE_STRICT) RECORD_NAME = "lifecycle.json" LOCK_NAME = "lifecycle.lock" RECONCILE_LOCK_NAME = ".lifecycle-reconcile.lock" - -# Fixed, argv-independent scope description. systemd-run derives a description -# from the command line by default (a leak risk — argv could carry config -# values); we always pass this literal instead. SCOPE_DESCRIPTION = "Nerve Codex workflow run containment" -# Record schema version (bump on incompatible shape changes). -RECORD_VERSION = 1 +# systemd-run enforces TERM, then SIGKILL after this grace, over the cgroup. +TERM_GRACE_SECONDS = 5 +STOP_TIMEOUT_SECONDS = 20 -# systemd unit name charset (a conservative subset of what systemd accepts). _UNIT_RE = re.compile(r"^nerve-wf-[A-Za-z0-9:_.-]+\.scope$") -# A resolved delegated user scope always lives under the user slice. _CGROUP_ROOT = "/sys/fs/cgroup" _USER_SLICE_PREFIX = "/user.slice/" -# Terminal receipt outcomes (cleanup is finished, do not retry). -_TERMINAL_OUTCOMES = frozenset({ - "complete", # we stopped the scope and verified emptiness - "resolved_absent", # same-boot: the scope no longer exists, no signal sent - "resolved_prior_boot", # cross-boot: no process could have survived a reboot - "failed_replaced", # the unit was replaced by a foreign instance; refused - "unenforced_observe", # observe mode: no containment was ever established - "no_scope", # nothing was ever contained (disabled / non-strict) -}) -# Outcomes that still owe a retry. -_RETRY_OUTCOMES = frozenset({"pending_retry"}) +# Receipt outcomes. `complete` and `refused` are terminal (never retried); +# `pending_retry` is re-attempted by the next startup reconciliation. +_TERMINAL_OUTCOMES = frozenset({"complete", "refused", "no_scope"}) class LifecycleError(Exception): - """A containment/reaping operation failed.""" + """A containment/reap operation failed.""" class ContainmentUnavailable(LifecycleError): - """Strict mode requested but the host cannot establish containment. + """Strict mode requested but the host cannot create a scope (raised before exec).""" - Raised *before* the command is launched, so nothing runs un-contained. - """ - -# -- host capability + instance identity ----------------------------------- # +# -- capabilities + instance identity --------------------------------------- # @dataclass(frozen=True) class Capabilities: ok: bool - systemd_run: bool - systemctl: bool - user_manager: bool - cgroup_v2: bool reason: str @@ -143,69 +73,32 @@ class Capabilities: def detect_capabilities(*, refresh: bool = False) -> Capabilities: - """Probe whether a per-run delegated user scope can be created here. - - Read-only and cached per process (the host does not change under us). A - strict launch consults this and fails before exec when ``ok`` is False. - """ + """Whether a delegated user scope can be created here. Cached per process.""" global _caps_cache if _caps_cache is not None and not refresh: return _caps_cache - - systemd_run = shutil.which("systemd-run") is not None - systemctl = shutil.which("systemctl") is not None - cgroup_v2 = Path(_CGROUP_ROOT, "cgroup.controllers").exists() - user_manager = False - if systemctl: + reasons = [] + if shutil.which("systemd-run") is None: + reasons.append("systemd-run not found") + if shutil.which("systemctl") is None: + reasons.append("systemctl not found") + if not Path(_CGROUP_ROOT, "cgroup.controllers").exists(): + reasons.append("cgroup v2 not mounted") + else: try: proc = subprocess.run( ["systemctl", "--user", "show-environment"], capture_output=True, timeout=5, check=False, ) - user_manager = proc.returncode == 0 + if proc.returncode != 0: + reasons.append("systemd --user manager not reachable") except (OSError, subprocess.SubprocessError): - user_manager = False - - reasons = [] - if not systemd_run: - reasons.append("systemd-run not found") - if not systemctl: - reasons.append("systemctl not found") - if not cgroup_v2: - reasons.append("cgroup v2 unified hierarchy not mounted") - if not user_manager: - reasons.append("systemd --user manager not reachable") - caps = Capabilities( - ok=not reasons, - systemd_run=systemd_run, - systemctl=systemctl, - user_manager=user_manager, - cgroup_v2=cgroup_v2, - reason="; ".join(reasons) or "ok", - ) - _caps_cache = caps - return caps - + reasons.append("systemd --user manager not reachable") + _caps_cache = Capabilities(ok=not reasons, reason="; ".join(reasons) or "ok") + return _caps_cache -@dataclass(frozen=True) -class Instance: - """Identity of the running daemon process, for concurrency fencing. - - ``generation`` changes across daemon restarts (pid + start_ticks differ) - and across reboots (boot_id differs), so a record written by a previous - daemon incarnation is always distinguishable from one this process owns. - """ - - boot_id: str - pid: int - start_ticks: int - - @property - def generation(self) -> str: - return f"{self.boot_id}:{self.pid}:{self.start_ticks}" - -def read_boot_id() -> str: +def _boot_id() -> str: try: return Path("/proc/sys/kernel/random/boot_id").read_text().strip() except OSError: @@ -213,177 +106,94 @@ def read_boot_id() -> str: def _proc_start_ticks(pid: int) -> int: - """Field 22 of ``/proc//stat`` (jiffies since boot at process start). - - ``comm`` (field 2) may contain spaces and parentheses, so split after the - final ``)``. - """ + """Field 22 of ``/proc//stat`` (read after the final ``)`` — ``comm`` + may contain spaces/parens).""" try: raw = Path(f"/proc/{pid}/stat").read_bytes() except OSError: return 0 try: - after = raw.rsplit(b")", 1)[1].split() - # after[0] is field 3 (state); field 22 is index 19 here. - return int(after[19]) + return int(raw.rsplit(b")", 1)[1].split()[19]) except (IndexError, ValueError): return 0 -def current_instance() -> Instance: +def current_generation() -> str: + """Identity of this daemon incarnation; changes on restart and reboot.""" pid = os.getpid() - return Instance(boot_id=read_boot_id(), pid=pid, start_ticks=_proc_start_ticks(pid)) + return f"{_boot_id()}:{pid}:{_proc_start_ticks(pid)}" -# -- durable record --------------------------------------------------------- # +# -- durable record ---------------------------------------------------------- # @dataclass class Receipt: - """The outcome of a reap/reconcile attempt.""" - outcome: str - enforced: bool = False signalled: bool = False verified_empty: bool = False - method: str = "" - attempts: int = 0 error: str = "" ts: str = "" - def is_terminal(self) -> bool: - """No further reap attempt is owed for this outcome.""" - return self.outcome in _TERMINAL_OUTCOMES - def is_complete(self) -> bool: - """True only when no live owned descendant remains (or none could). + return self.outcome == "complete" - Deliberately excludes ``unenforced_observe`` (observe mode never - contained anything, so it makes no completion claim) and - ``failed_replaced`` (a foreign unit we refused to touch — escalate, - do not claim the tree was reaped).""" - return self.outcome in { - "complete", "resolved_absent", "resolved_prior_boot", "no_scope", - } - - def needs_retry(self) -> bool: - return self.outcome in _RETRY_OUTCOMES - - def needs_attention(self) -> bool: - """A terminal outcome that resolved neither to complete nor retry — - surfaced for a human/operator rather than silently accepted.""" - return self.outcome == "failed_replaced" + def is_terminal(self) -> bool: + return self.outcome in _TERMINAL_OUTCOMES @dataclass class LifecycleRecord: run_id: str session_id: str - mode: str - attempt_nonce: str unit: str - workspace_id: str boot_id: str - instance_generation: str - phase: str = "intent" # intent -> launched -> registered -> terminal - owner_kind: str = "workflow" - invocation_id: str = "" # systemd InvocationID, recorded at 'launched' - control_group: str = "" # resolved cgroup path, recorded at 'launched' - version: int = RECORD_VERSION + generation: str + invocation_id: str = "" + control_group: str = "" created_at: str = "" updated_at: str = "" receipt: dict | None = None - # -- serialization ------------------------------------------------- # - def to_dict(self) -> dict: return asdict(self) @classmethod def from_dict(cls, d: dict) -> "LifecycleRecord": if not isinstance(d, dict): - raise LifecycleError("lifecycle record is not an object") + raise LifecycleError("record is not an object") try: rec = cls( - run_id=str(d["run_id"]), - session_id=str(d["session_id"]), - mode=str(d["mode"]), - attempt_nonce=str(d["attempt_nonce"]), - unit=str(d["unit"]), - workspace_id=str(d.get("workspace_id", "")), - boot_id=str(d.get("boot_id", "")), - instance_generation=str(d.get("instance_generation", "")), - phase=str(d.get("phase", "intent")), - owner_kind=str(d.get("owner_kind", "workflow")), + run_id=str(d["run_id"]), session_id=str(d["session_id"]), + unit=str(d["unit"]), boot_id=str(d.get("boot_id", "")), + generation=str(d.get("generation", "")), invocation_id=str(d.get("invocation_id", "")), control_group=str(d.get("control_group", "")), - version=int(d.get("version", RECORD_VERSION)), created_at=str(d.get("created_at", "")), updated_at=str(d.get("updated_at", "")), receipt=d.get("receipt") if isinstance(d.get("receipt"), dict) else None, ) - except (KeyError, TypeError, ValueError) as e: - raise LifecycleError(f"malformed lifecycle record: {e}") from e + except (KeyError, TypeError) as e: + raise LifecycleError(f"malformed record: {e}") from e rec.validate() return rec def validate(self) -> None: - """Reject anything that could steer a signal at a foreign target. - - Never trusts a cgroup path that isn't a leaf under the user slice - matching this record's unit, and never accepts an out-of-charset unit. - """ - if self.owner_kind != "workflow": - raise LifecycleError(f"unexpected owner_kind {self.owner_kind!r}") - if self.mode not in VALID_MODES: - raise LifecycleError(f"invalid mode {self.mode!r}") if not _UNIT_RE.match(self.unit): raise LifecycleError(f"unsafe unit name {self.unit!r}") - if not self.attempt_nonce: - raise LifecycleError("empty attempt_nonce") if self.control_group: _validate_cgroup_path(self.control_group, self.unit) def _validate_cgroup_path(control_group: str, unit: str) -> None: - """A resolved scope cgroup must be a leaf under the user slice named for - its unit — never a root/parent, never a symlink escape, never ``..``.""" + """A resolved scope cgroup must be a leaf under the user slice named for its + unit — reject a root/parent, a ``..`` traversal, or a foreign path.""" if not control_group.startswith(_USER_SLICE_PREFIX): - raise LifecycleError(f"cgroup path not under user slice: {control_group!r}") + raise LifecycleError(f"cgroup not under user slice: {control_group!r}") if ".." in control_group.split("/"): - raise LifecycleError(f"cgroup path traverses parent: {control_group!r}") + raise LifecycleError(f"cgroup traverses parent: {control_group!r}") if control_group.rstrip("/").split("/")[-1] != unit: - raise LifecycleError( - f"cgroup leaf {control_group!r} does not match unit {unit!r}" - ) - - -# -- atomic durable IO ------------------------------------------------------ # - - -def _atomic_write_json(path: Path, obj: dict) -> None: - """Write ``obj`` as JSON atomically with file + directory fsync. - - Raises :class:`OSError` on any failure — a caller that cannot persist - launch identity must not launch, and one that cannot persist a completion - receipt must not report completion. - """ - path.parent.mkdir(parents=True, exist_ok=True) - tmp = path.with_name(f".{path.name}.{os.getpid()}.{uuid.uuid4().hex[:8]}.tmp") - data = json.dumps(obj, indent=2, sort_keys=True).encode("utf-8") - fd = os.open(tmp, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) - try: - os.write(fd, data) - os.fsync(fd) - finally: - os.close(fd) - os.replace(tmp, path) - # fsync the directory so the rename is durable across a crash. - dir_fd = os.open(path.parent, os.O_RDONLY) - try: - os.fsync(dir_fd) - finally: - os.close(dir_fd) + raise LifecycleError(f"cgroup leaf != unit: {control_group!r} / {unit!r}") def record_path(run_dir: Path) -> Path: @@ -391,17 +201,16 @@ def record_path(run_dir: Path) -> Path: def read_record(run_dir: Path) -> LifecycleRecord | None: - path = record_path(run_dir) try: - raw = path.read_text(encoding="utf-8") + raw = record_path(run_dir).read_text(encoding="utf-8") except FileNotFoundError: return None except OSError as e: - raise LifecycleError(f"cannot read lifecycle record: {e}") from e + raise LifecycleError(f"cannot read record: {e}") from e try: return LifecycleRecord.from_dict(json.loads(raw)) except ValueError as e: - raise LifecycleError(f"corrupt lifecycle record JSON: {e}") from e + raise LifecycleError(f"corrupt record JSON: {e}") from e def write_record(run_dir: Path, record: LifecycleRecord) -> None: @@ -412,13 +221,31 @@ def write_record(run_dir: Path, record: LifecycleRecord) -> None: _atomic_write_json(record_path(run_dir), record.to_dict()) +def _atomic_write_json(path: Path, obj: dict) -> None: + """Write JSON via a temp file, then rename; fsync the file and its directory + so the record survives a crash. Raises OSError on any failure.""" + path.parent.mkdir(parents=True, exist_ok=True) + tmp = path.with_name(f".{path.name}.{os.getpid()}.{uuid.uuid4().hex[:8]}.tmp") + fd = os.open(tmp, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + try: + os.write(fd, json.dumps(obj, indent=2, sort_keys=True).encode("utf-8")) + os.fsync(fd) + finally: + os.close(fd) + os.replace(tmp, path) + dir_fd = os.open(path.parent, os.O_RDONLY) + try: + os.fsync(dir_fd) + finally: + os.close(dir_fd) + + def _now() -> str: return time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()) @contextlib.contextmanager def _flock(lock_path: Path): - """Exclusive advisory lock; serialises reap/retry/reconcile per owner.""" lock_path.parent.mkdir(parents=True, exist_ok=True) fd = os.open(lock_path, os.O_WRONLY | os.O_CREAT, 0o600) try: @@ -430,62 +257,46 @@ def _flock(lock_path: Path): os.close(fd) -# -- unit naming + launch wrapper ------------------------------------------- # +# -- unit naming + launch wrapper -------------------------------------------- # def scope_unit_name(run_id: str, nonce: str) -> str: - """Unique, charset-safe scope unit name embedding the attempt nonce. - - The nonce makes the name globally unique per attempt, so a name match is a - strong identity signal even in the tiny window before an InvocationID has - been recorded (the create/register gap). - """ + """Unique, charset-safe scope unit name embedding the attempt nonce.""" safe_run = re.sub(r"[^A-Za-z0-9_.-]", "_", run_id)[:80] safe_nonce = re.sub(r"[^A-Za-z0-9]", "", nonce)[:16] return f"nerve-wf-{safe_run}-{safe_nonce}.scope" -def build_scope_argv( - unit: str, inner_argv: list[str], *, term_grace_seconds: int, -) -> list[str]: - """Wrap ``inner_argv`` so it runs inside a delegated transient user scope. - - Containment is established by systemd *before* the command execs: no late - ``cgroup.procs`` migration and no threaded-Python ``preexec_fn``. The whole - fork/setsid/double-fork subtree stays in this scope's cgroup. - """ +def build_scope_argv(unit: str, inner_argv: list[str]) -> list[str]: + """Wrap ``inner_argv`` to run inside a delegated transient user scope. + systemd places the process in the scope's cgroup before it execs, so the + whole fork/setsid/double-fork subtree stays contained.""" if not _UNIT_RE.match(unit): - raise LifecycleError(f"refusing to launch with unsafe unit {unit!r}") - grace = max(1, int(term_grace_seconds)) + raise LifecycleError(f"unsafe unit {unit!r}") return [ "systemd-run", "--user", "--scope", "--quiet", "--collect", "--expand-environment=no", "-p", "Delegate=yes", "-p", "KillMode=control-group", "-p", "SendSIGKILL=yes", - "-p", f"TimeoutStopSec={grace}", + "-p", f"TimeoutStopSec={TERM_GRACE_SECONDS}", "--description", SCOPE_DESCRIPTION, "--unit", unit, "--", *inner_argv, ] -# -- systemd / cgroup queries ----------------------------------------------- # +# -- systemd / cgroup queries ------------------------------------------------ # def _systemctl_show(unit: str, props: Iterable[str]) -> dict[str, str]: - """``systemctl --user show`` a unit; parse ``KEY=VALUE`` lines. - - Returns an empty dict if systemctl is unavailable or the call fails. - """ - prop_arg = ",".join(props) try: proc = subprocess.run( - ["systemctl", "--user", "show", unit, "--property", prop_arg], + ["systemctl", "--user", "show", unit, "--property", ",".join(props)], capture_output=True, text=True, timeout=10, check=False, ) except (OSError, subprocess.SubprocessError) as e: - logger.debug("systemctl show failed for %s: %s", unit, e) + logger.debug("systemctl show %s failed: %s", unit, e) return {} out: dict[str, str] = {} for line in proc.stdout.splitlines(): @@ -496,82 +307,59 @@ def _systemctl_show(unit: str, props: Iterable[str]) -> dict[str, str]: def _unit_identity(unit: str) -> dict[str, str]: - return _systemctl_show( - unit, ("LoadState", "ActiveState", "InvocationID", "ControlGroup", "Description"), - ) + return _systemctl_show(unit, ("LoadState", "InvocationID", "ControlGroup", "Description")) -def _await_unit_identity( - unit: str, *, timeout: float = 4.0, -) -> tuple[str, str]: - """Poll until the scope's InvocationID + ControlGroup are populated. - - Returns ``(invocation_id, control_group)`` — both empty on timeout. - """ +def _await_unit_identity(unit: str, *, timeout: float = 4.0) -> tuple[str, str]: + """Poll until the scope reports its InvocationID + ControlGroup.""" deadline = time.monotonic() + timeout inv = cg = "" while time.monotonic() < deadline: ident = _unit_identity(unit) - inv = ident.get("InvocationID", "") - cg = ident.get("ControlGroup", "") + inv, cg = ident.get("InvocationID", ""), ident.get("ControlGroup", "") if inv and cg: return inv, cg time.sleep(0.1) return inv, cg -def _cgroup_events(control_group: str) -> tuple[str, bool | None]: - """Inspect a scope cgroup's ``cgroup.events`` ``populated`` state. - - ``populated`` is recursive per the kernel (set if *any* descendant cgroup - holds a process), so it is the correct emptiness signal even when the - delegated subtree created child cgroups. - - Returns ``(state, populated)`` where ``state`` is one of ``"gone"`` (the - scope cgroup directory no longer exists) or ``"present"``. ``populated`` is - None when the directory is gone or unreadable. - """ +def _cgroup_gone_or_empty(control_group: str) -> bool | None: + """True if the scope cgroup is gone or recursively unpopulated, False if it + still holds a process, None if it can't be read. ``cgroup.events`` + ``populated`` is recursive per the kernel, so it covers delegated children.""" if not control_group: - return "gone", None + return None path = Path(_CGROUP_ROOT + control_group) / "cgroup.events" try: text = path.read_text() except FileNotFoundError: - return "gone", None - except OSError as e: - logger.debug("cgroup.events unreadable for %s: %s", control_group, e) - return "present", None - populated = None + return True + except OSError: + return None for line in text.splitlines(): if line.startswith("populated"): - populated = line.split()[-1].strip() == "1" - break - return "present", populated - + return line.split()[-1].strip() == "0" + return None -def _scope_stop(unit: str, *, timeout: float) -> tuple[bool, str]: - """``systemctl --user stop`` a scope, bounded by ``timeout`` seconds. - ``stop`` performs the configured ``KillMode=control-group`` TERM → grace - (``TimeoutStopSec``) → SIGKILL sequence over the whole cgroup. Returns - ``(ok, error)``. - """ +def _scope_stop(unit: str) -> tuple[bool, str]: + """``systemctl --user stop`` the scope (TERM → grace → SIGKILL over the + control group). Returns (ok, error).""" try: proc = subprocess.run( ["systemctl", "--user", "stop", unit], - capture_output=True, text=True, timeout=timeout, check=False, + capture_output=True, text=True, timeout=STOP_TIMEOUT_SECONDS, check=False, ) except subprocess.TimeoutExpired: - return False, "systemctl stop timed out" + return False, "stop timed out" except (OSError, subprocess.SubprocessError) as e: - return False, f"systemctl stop failed: {e}" + return False, f"stop failed: {e}" if proc.returncode != 0: - return False, (proc.stderr or f"systemctl stop rc={proc.returncode}").strip()[:200] + return False, (proc.stderr or f"rc={proc.returncode}").strip()[:200] return True, "" def proc_cgroup(pid: int) -> str | None: - """The cgroup v2 path of ``pid`` (``0::PATH`` line), or None.""" try: text = Path(f"/proc/{pid}/cgroup").read_text() except OSError: @@ -583,401 +371,141 @@ def proc_cgroup(pid: int) -> str | None: def verify_membership(control_group: str, pid: int) -> bool: - """True iff ``pid`` is contained in ``control_group`` (or a descendant) - and the scope is populated — a concrete membership proof at handshake.""" + """True if ``pid`` is in ``control_group`` (or a descendant of it).""" if not control_group: return False member = proc_cgroup(pid) if member is None: return False - if member != control_group and not member.startswith(control_group.rstrip("/") + "/"): - return False - _state, populated = _cgroup_events(control_group) - return populated is not False + return member == control_group or member.startswith(control_group.rstrip("/") + "/") + + +# -- launch config handed to the app-server client -------------------------- # -# -- launch-side operations (used by the app-server client) ----------------- # +@dataclass(frozen=True) +class WorkflowContainment: + mode: str + run_dir: Path + run_id: str + session_id: str + @property + def enabled(self) -> bool: + return self.mode == MODE_STRICT -def persist_intent( - run_dir: Path, *, run_id: str, session_id: str, mode: str, workspace_id: str, -) -> LifecycleRecord: - """Write the validated owner intent BEFORE anything can fork the command. - Raises :class:`OSError`/:class:`LifecycleError` on a persistence failure — - a strict launch must not proceed if its identity cannot be recorded. - """ +def prepare_launch( + containment: WorkflowContainment, inner_argv: list[str], +) -> tuple[list[str], LifecycleRecord]: + """Persist the owner record, then return the scope-wrapped argv. Raises + before anything execs if the host can't contain or the record can't be + written, so a strict run never launches un-contained.""" + caps = detect_capabilities() + if not caps.ok: + raise ContainmentUnavailable(f"strict cgroup containment unavailable: {caps.reason}") nonce = uuid.uuid4().hex - unit = scope_unit_name(run_id, nonce) - inst = current_instance() + unit = scope_unit_name(containment.run_id, nonce) record = LifecycleRecord( - run_id=run_id, - session_id=session_id, - mode=mode, - attempt_nonce=nonce, - unit=unit, - workspace_id=workspace_id, - boot_id=inst.boot_id, - instance_generation=inst.generation, - phase="intent", + run_id=containment.run_id, session_id=containment.session_id, unit=unit, + boot_id=_boot_id(), generation=current_generation(), ) - write_record(Path(run_dir), record) - return record + write_record(containment.run_dir, record) + return build_scope_argv(unit, inner_argv), record def record_launched(run_dir: Path, record: LifecycleRecord) -> LifecycleRecord: - """After systemd-run started the scope, record its immutable identity. - - Reads the scope's systemd ``InvocationID`` and resolved ``ControlGroup`` - (the two together are the reuse-proof instance identity). Raises - :class:`LifecycleError` if they cannot be resolved — in strict mode the - caller then closes and fails the launch (the scope, already created, is - still reapable from the persisted intent). - """ + """Record the scope's InvocationID + ControlGroup once systemd-run created + it. Raises if they don't resolve — the caller then fails the launch.""" inv, cg = _await_unit_identity(record.unit) if not inv or not cg: - raise LifecycleError( - f"scope {record.unit} did not report an InvocationID/ControlGroup" - ) - record.invocation_id = inv - record.control_group = cg - record.phase = "launched" + raise LifecycleError(f"scope {record.unit} reported no InvocationID/ControlGroup") + record.invocation_id, record.control_group = inv, cg write_record(Path(run_dir), record) return record -def record_registered(run_dir: Path, record: LifecycleRecord) -> LifecycleRecord: - record.phase = "registered" - write_record(Path(run_dir), record) - return record +# -- reap + reconcile -------------------------------------------------------- # -# -- reap + reconcile ------------------------------------------------------- # - - -def reap( - run_dir: Path, *, term_grace_seconds: int = 5, stop_timeout_seconds: int = 20, -) -> Receipt: - """Authoritatively reap a terminal run's contained descendants. - - Independent of any in-memory client: it works purely from the durable - record. Idempotent — a record already carrying a terminal receipt is - returned unchanged; a ``pending_retry`` record is re-attempted. Serialised - per owner by a file lock so concurrent terminal/retry paths cannot race. - """ +def reap(run_dir: Path) -> Receipt: + """Reap a run's contained descendants from its durable record. Idempotent + (a terminal receipt is returned unchanged); serialised per owner.""" run_dir = Path(run_dir) - # Cheap pre-check so run dirs without a lifecycle record (Claude runs, - # disabled mode) are never even given a lock file. The authoritative read - # happens under the lock below. if not record_path(run_dir).exists(): return Receipt(outcome="no_scope", ts=_now()) with _flock(run_dir / LOCK_NAME): try: record = read_record(run_dir) except LifecycleError as e: - # A corrupt/unreadable record cannot be validated, so it cannot be - # signalled from and cannot be safely rewritten — escalate - # conservatively (never claim completion). R4. - logger.warning("refusing to reap from corrupt record in %s: %s", - run_dir, e) - return Receipt( - outcome="pending_retry", method="corrupt-record", - error="corrupt lifecycle record; manual attention required", - ts=_now(), - ) + logger.warning("refusing to reap from corrupt record in %s: %s", run_dir, e) + return Receipt(outcome="pending_retry", error="corrupt record", ts=_now()) if record is None: return Receipt(outcome="no_scope", ts=_now()) - if record.mode == MODE_OBSERVE: - return _write_receipt(run_dir, record, Receipt( - outcome="unenforced_observe", enforced=False, signalled=False, - method="observe", ts=_now(), - )) - if record.mode != MODE_STRICT: - return _write_receipt(run_dir, record, Receipt( - outcome="no_scope", enforced=False, ts=_now(), - )) - existing = _receipt_from(record) - if existing is not None and existing.is_terminal(): - return existing - receipt = _reap_strict( - record, - term_grace_seconds=term_grace_seconds, - stop_timeout_seconds=stop_timeout_seconds, - prev=existing, - ) - return _write_receipt(run_dir, record, receipt) - - -def _reap_strict( - record: LifecycleRecord, *, term_grace_seconds: int, - stop_timeout_seconds: int, prev: Receipt | None, -) -> Receipt: - attempts = (prev.attempts if prev else 0) + 1 - current_boot = read_boot_id() - - # Cross-boot: nothing from a previous boot can still be running, and the - # unit name may now belong to a foreign process this boot — never signal. - if record.boot_id and current_boot and record.boot_id != current_boot: - return Receipt( - outcome="resolved_prior_boot", enforced=True, signalled=False, - verified_empty=True, method="reboot", attempts=attempts, ts=_now(), - ) + prev = _receipt_from(record) + if prev is not None and prev.is_terminal(): + return prev + receipt = _do_reap(record) + record.receipt = asdict(receipt) + write_record(run_dir, record) + return receipt + + +def _do_reap(record: LifecycleRecord) -> Receipt: + # A reboot leaves no process alive, and the unit name may now be foreign — + # resolve without signalling. + if record.boot_id and _boot_id() and record.boot_id != _boot_id(): + return Receipt(outcome="complete", verified_empty=True, ts=_now()) ident = _unit_identity(record.unit) - load_state = ident.get("LoadState", "") - active_state = ident.get("ActiveState", "") - live_inv = ident.get("InvocationID", "") - live_cg = ident.get("ControlGroup", "") or record.control_group - description = ident.get("Description", "") - - unit_present = load_state not in ("", "not-found") - - if not unit_present: - # The scope no longer exists. Same boot, so no reboot-cleanup claim — - # but a transient scope's cgroup is removed only after it is emptied, - # so absence means the contained tree is gone. No signal was sent. - return Receipt( - outcome="resolved_absent", enforced=True, signalled=False, - verified_empty=True, method="absent", attempts=attempts, ts=_now(), - ) + if ident.get("LoadState", "") in ("", "not-found"): + # Same boot, unit gone: systemd removes a scope's cgroup only once it is + # empty, so the tree is gone. No signal was sent. + return Receipt(outcome="complete", verified_empty=True, ts=_now()) - # Identity gate: refuse to signal anything that is not provably our scope. + live_inv = ident.get("InvocationID", "") if record.invocation_id: if live_inv and live_inv != record.invocation_id: - return Receipt( - outcome="failed_replaced", enforced=True, signalled=False, - method="identity", attempts=attempts, ts=_now(), - error="unit InvocationID differs from record (replaced by a " - "foreign instance); refusing to signal", - ) - else: - # Create/register gap: no InvocationID was ever recorded. The unit name - # embeds our unique nonce, so a same-name unit is ours — but demand the - # fixed description too before signalling. - if description and description != SCOPE_DESCRIPTION: - return Receipt( - outcome="failed_replaced", enforced=True, signalled=False, - method="identity", attempts=attempts, ts=_now(), - error="unit description does not match; refusing to signal", - ) + return Receipt(outcome="refused", error="unit replaced by a foreign instance", ts=_now()) + elif ident.get("Description", "") not in ("", SCOPE_DESCRIPTION): + return Receipt(outcome="refused", error="unit description mismatch", ts=_now()) - if live_cg: - with contextlib.suppress(LifecycleError): - _validate_cgroup_path(live_cg, record.unit) - - ok, err = _scope_stop(record.unit, timeout=stop_timeout_seconds) + ok, err = _scope_stop(record.unit) if not ok: - return Receipt( - outcome="pending_retry", enforced=True, signalled=True, - method="systemctl-stop", attempts=attempts, ts=_now(), - error=err or "stop failed", verified_empty=False, - ) - - # A successful stop is a request, not proof of exit — verify emptiness. - state, populated = _cgroup_events(live_cg) - if state == "gone" or populated is False: - return Receipt( - outcome="complete", enforced=True, signalled=True, - verified_empty=True, method="systemctl-stop", - attempts=attempts, ts=_now(), - ) - return Receipt( - outcome="pending_retry", enforced=True, signalled=True, - method="systemctl-stop", attempts=attempts, ts=_now(), - error="scope still populated after stop", verified_empty=False, - ) - - -def reconcile_run( - run_dir: Path, *, is_terminal: bool, active_generation: str, - term_grace_seconds: int = 5, stop_timeout_seconds: int = 20, -) -> Receipt | None: - """Reconcile one run's containment at startup / on demand. - - Cleans only a **terminal** run (or a **provably abandoned** launch: a - record still at ``intent``/``launched`` whose launching daemon generation - is not the current one). A run owned by the *current* generation, or a - still-active run, is left alone. Returns the receipt, or None when nothing - was due. - """ - run_dir = Path(run_dir) - record = read_record(run_dir) - if record is None: - return None - if record.mode != MODE_STRICT: - return None - # Never touch a run this very daemon generation still owns. - if record.instance_generation == active_generation: - return None - abandoned_launch = record.phase in ("intent", "launched", "registered") - if not (is_terminal or abandoned_launch): - return None - return reap( - run_dir, - term_grace_seconds=term_grace_seconds, - stop_timeout_seconds=stop_timeout_seconds, - ) - + return Receipt(outcome="pending_retry", signalled=True, error=err, ts=_now()) + if _cgroup_gone_or_empty(record.control_group) is True: + return Receipt(outcome="complete", signalled=True, verified_empty=True, ts=_now()) + return Receipt(outcome="pending_retry", signalled=True, + error="scope still populated after stop", ts=_now()) -def list_records(runs_dir: Path) -> list[tuple[str, LifecycleRecord]]: - """Every run dir under ``runs_dir`` that carries a lifecycle record. - A corrupt record is surfaced as a synthetic ``pending_retry`` sentinel via - the exception path in callers; here we skip unreadable ones and log, so a - single bad file cannot abort a whole reconciliation sweep. - """ - out: list[tuple[str, LifecycleRecord]] = [] +def reconcile(runs_dir: Path) -> list[tuple[str, Receipt]]: + """Reap scopes left by a previous daemon incarnation. A record whose + generation differs from the current one was owned by a dead daemon, so its + run cannot be active; a record of the current generation is left alone.""" root = Path(runs_dir) if not root.is_dir(): - return out - for child in sorted(root.iterdir()): - if not child.is_dir(): - continue - try: - record = read_record(child) - except LifecycleError: - logger.warning("skipping corrupt lifecycle record in %s", child.name) - continue - if record is not None: - out.append((child.name, record)) - return out - - -def reconcile_all( - runs_dir: Path, is_terminal: Callable[[str], bool], *, - term_grace_seconds: int = 5, stop_timeout_seconds: int = 20, -) -> list[tuple[str, Receipt]]: - """Sweep every run with a record, reaping terminal/abandoned containment. - - Holds a global reconcile lock so only one sweep runs at a time. ``is_terminal`` - maps a run id to whether its business status is terminal. - """ - root = Path(runs_dir) + return [] + gen = current_generation() results: list[tuple[str, Receipt]] = [] - gen = current_instance().generation with _flock(root / RECONCILE_LOCK_NAME): - for run_id, _record in list_records(root): + for child in sorted(p for p in root.iterdir() if p.is_dir()): try: - receipt = reconcile_run( - root / run_id, - is_terminal=is_terminal(run_id), - active_generation=gen, - term_grace_seconds=term_grace_seconds, - stop_timeout_seconds=stop_timeout_seconds, - ) - except LifecycleError as e: - logger.warning("reconcile failed for %s: %s", run_id, e) + record = read_record(child) + except LifecycleError: + logger.warning("skipping corrupt record in %s", child.name) continue - if receipt is not None: - results.append((run_id, receipt)) + if record is None or record.generation == gen: + continue + results.append((child.name, reap(child))) return results -def retry( - run_dir: Path, *, term_grace_seconds: int = 5, stop_timeout_seconds: int = 20, -) -> Receipt: - """The one supported, narrowly-scoped retry entry. - - Re-attempts a ``pending_retry`` (or not-yet-run) reap. Idempotent: a record - already carrying a terminal receipt is returned unchanged. Never fabricates - success — a still-failing stop stays ``pending_retry`` with the error. - """ - return reap( - run_dir, - term_grace_seconds=term_grace_seconds, - stop_timeout_seconds=stop_timeout_seconds, - ) - - -# -- launch config handed to the app-server client -------------------------- # - - -@dataclass(frozen=True) -class WorkflowContainment: - """Immutable per-launch containment config for one workflow-run client. - - Built by the Codex backend only for a ``workflow:``-prefixed session when - ``codex.lifecycle.mode`` is ``observe``/``strict``; ``None`` (and therefore - today's behaviour) for every other session and for ``disabled`` mode. - """ - - mode: str - run_dir: Path - run_id: str - session_id: str - workspace_id: str - term_grace_seconds: int = 5 - stop_timeout_seconds: int = 20 - - @property - def enabled(self) -> bool: - return self.mode in (MODE_OBSERVE, MODE_STRICT) - - -def prepare_launch( - containment: WorkflowContainment, inner_argv: list[str], -) -> tuple[list[str], LifecycleRecord]: - """Persist intent and return the argv to actually spawn. - - * strict: validate host capability (raise :class:`ContainmentUnavailable` - *before* anything execs), persist intent, return the scope-wrapped argv. - * observe: persist an unenforced-mode intent, return ``inner_argv`` - unchanged (today's behaviour preserved). - - Raises before the command can execute if capability or persistence fails, - so a strict run never launches un-contained. - """ - if containment.mode == MODE_STRICT: - caps = detect_capabilities() - if not caps.ok: - raise ContainmentUnavailable( - f"strict cgroup containment unavailable: {caps.reason}" - ) - record = persist_intent( - containment.run_dir, - run_id=containment.run_id, - session_id=containment.session_id, - mode=containment.mode, - workspace_id=containment.workspace_id, - ) - if containment.mode == MODE_STRICT: - wrapped = build_scope_argv( - record.unit, inner_argv, - term_grace_seconds=containment.term_grace_seconds, - ) - return wrapped, record - return list(inner_argv), record - - -# -- receipt helpers -------------------------------------------------------- # - - def _receipt_from(record: LifecycleRecord) -> Receipt | None: if not record.receipt: return None try: - return Receipt(**{k: v for k, v in record.receipt.items() if k in Receipt.__dataclass_fields__}) + return Receipt(**{k: v for k, v in record.receipt.items() + if k in Receipt.__dataclass_fields__}) except TypeError: return None - - -def _write_receipt( - run_dir: Path, record: LifecycleRecord, receipt: Receipt, -) -> Receipt: - record.receipt = asdict(receipt) - if receipt.is_terminal(): - record.phase = "terminal" - write_record(Path(run_dir), record) - return receipt - - -__all__ = [ - "MODE_DISABLED", "MODE_OBSERVE", "MODE_STRICT", "VALID_MODES", - "Capabilities", "detect_capabilities", "Instance", "current_instance", - "read_boot_id", "LifecycleError", "ContainmentUnavailable", - "LifecycleRecord", "Receipt", "record_path", "read_record", "write_record", - "scope_unit_name", "build_scope_argv", "verify_membership", "proc_cgroup", - "persist_intent", "record_launched", "record_registered", - "reap", "reconcile_run", "reconcile_all", "list_records", "retry", - "SCOPE_DESCRIPTION", "WorkflowContainment", "prepare_launch", -] diff --git a/nerve/cli.py b/nerve/cli.py index 1c6c6c53c..bed93edbb 100644 --- a/nerve/cli.py +++ b/nerve/cli.py @@ -1623,40 +1623,6 @@ def codex() -> None: """Inspect and authenticate the Codex integration.""" -@codex.command("reap-descendants") -@click.argument("run_id") -@click.option("--json-output", is_flag=True, help="Emit machine-readable JSON.") -@click.pass_context -def codex_reap_descendants(ctx: click.Context, run_id: str, json_output: bool) -> None: - """Retry the cgroup reap of a Codex workflow run's contained descendants. - - The supported, idempotent retry trigger for a ``pending_retry`` lifecycle - reap (see codex/lifecycle.py). Reads only the run's durable record; never - fabricates success. A run with no record reports ``no_scope``. - """ - import json as _json - from pathlib import Path - - from nerve.agent.backends.codex import lifecycle - - config = ctx.obj["config"] - lc = config.codex.lifecycle - run_dir = Path(config.workflows.runs_dir).expanduser() / run_id - receipt = lifecycle.retry( - run_dir, - term_grace_seconds=lc.term_grace_seconds, - stop_timeout_seconds=lc.stop_timeout_seconds, - ) - from dataclasses import asdict - - if json_output: - click.echo(_json.dumps(asdict(receipt), indent=2, sort_keys=True)) - else: - verdict = "complete" if receipt.is_complete() else receipt.outcome - click.echo(f"[{verdict}] {run_id}: {receipt.outcome}" - + (f" — {receipt.error}" if receipt.error else "")) - - @codex.command("token") @click.option( "--hours", type=click.IntRange(1, 24), default=8, show_default=True, diff --git a/nerve/config.py b/nerve/config.py index 232dcca27..b112f8600 100644 --- a/nerve/config.py +++ b/nerve/config.py @@ -2263,57 +2263,25 @@ def validate(self) -> list[str]: @dataclass class CodexLifecycleConfig: - """Descendant containment for Codex **workflow** runs (cgroup v2 scope). + """Cgroup containment for Codex workflow runs. - A completed/cancelled/crashed workflow run can leave ``codex-linux-sandbox`` - descendants alive: they ``setsid`` into their own process group and escape - the single-group ``killpg`` teardown. This opt-in feature launches a - workflow run's app-server inside a delegated systemd user scope so the - whole tree can be reaped, with a durable receipt. See - ``nerve/agent/backends/codex/lifecycle.py``. - - ``mode``: - - * ``disabled`` (default) — inert; launch behaviour is unchanged. - * ``observe`` — UNENFORCED: records intent for staged-rollout visibility - but does **not** contain the launch and never claims a cleanup it did not - perform. - * ``strict`` — launches inside a cgroup scope created before the command - execs; if containment cannot be established the launch **fails before - exec** (no silent downgrade). Codex workflow runs only. + ``mode``: ``disabled`` (default; launch unchanged) or ``strict`` (launch the + workflow app-server inside a systemd user scope so its descendants can be + reaped; fails before exec where the host can't contain). Codex workflow runs + only. See ``nerve/agent/backends/codex/lifecycle.py``. """ mode: str = "disabled" - term_grace_seconds: int = 5 - stop_timeout_seconds: int = 20 @classmethod @_coerced def from_dict(cls, raw: dict | None) -> "CodexLifecycleConfig": - d = raw or {} - return cls( - mode=str(d.get("mode") or "disabled").strip().lower(), - term_grace_seconds=_lenient_int(d.get("term_grace_seconds"), 5), - stop_timeout_seconds=_lenient_int(d.get("stop_timeout_seconds"), 20), - ) + return cls(mode=str((raw or {}).get("mode") or "disabled").strip().lower()) def validate(self) -> list[str]: - problems: list[str] = [] - if self.mode not in ("disabled", "observe", "strict"): - problems.append( - "codex.lifecycle.mode must be 'disabled', 'observe', or " - f"'strict', got {self.mode!r}" - ) - if self.term_grace_seconds < 1: - problems.append("codex.lifecycle.term_grace_seconds must be >= 1") - if self.stop_timeout_seconds < 1: - problems.append("codex.lifecycle.stop_timeout_seconds must be >= 1") - if self.stop_timeout_seconds <= self.term_grace_seconds: - problems.append( - "codex.lifecycle.stop_timeout_seconds must exceed " - "term_grace_seconds (stop waits out the TERM grace, then KILL)" - ) - return problems + if self.mode not in ("disabled", "strict"): + return [f"codex.lifecycle.mode must be 'disabled' or 'strict', got {self.mode!r}"] + return [] @dataclass diff --git a/nerve/workflows/service.py b/nerve/workflows/service.py index adc99bb6d..f129e132c 100644 --- a/nerve/workflows/service.py +++ b/nerve/workflows/service.py @@ -56,7 +56,7 @@ from nerve.agent.backends.codex import lifecycle as codex_lifecycle from nerve.agent.streaming import broadcaster from nerve.db.usage import estimate_turn_cost -from nerve.db.workflow_runs import ACTIVE_STATUSES, TERMINAL_STATUSES +from nerve.db.workflow_runs import ACTIVE_STATUSES from nerve.utils.time import utc_now_iso if TYPE_CHECKING: @@ -160,10 +160,8 @@ async def start(self) -> None: "survive the restart; restart them explicitly if still needed.", priority="high", ) - # R4 ordering: the interrupted-active→failed pass above has already run, - # so every prior-generation run is terminal here. Reap any Codex - # workflow-run cgroup scopes their (now-gone) clients left behind, - # before the monitor loop can dispatch anything new. + # Reap Codex workflow-run cgroup scopes left by a previous incarnation, + # before the monitor loop dispatches anything new. await self._reconcile_codex_lifecycle() interval = max(5, int(self.config.workflows.poll_interval_seconds)) self._monitor_task = asyncio.create_task(self._monitor_loop(interval)) @@ -721,10 +719,8 @@ async def _finalize_terminal(self, run_id: str, status: str, detail: dict) -> No logger.exception( "workflow run completion listener failed for %s", run_id, ) - # Authoritatively reap any contained Codex descendants for this run, - # independent of the in-memory client (works from the durable record). - # Detached so it never blocks the terminal path; no-ops unless the run - # recorded strict/observe containment. + # Reap this run's contained descendants from its durable record, + # detached so it never blocks the terminal path (no-op without a record). if str(run.get("engine") or "") == ENGINE_CODEX: self._spawn_detached( self._reap_codex_lifecycle(run_id), @@ -742,122 +738,31 @@ def _is_quiet(run: dict | None) -> bool: # ------------------------------------------------------------------ # async def _reconcile_codex_lifecycle(self) -> None: - """Startup reconciliation of Codex workflow-run cgroup scopes. - - Reaps contained descendants of terminal/abandoned prior-generation runs - from their durable records — independent of the in-memory client, which - did not survive the restart. Never touches a run this daemon generation - still owns (the generation fence lives in ``reconcile_run``). Best - effort: a failure here must not block the monitor loop from starting. - """ - lc = self.config.codex.lifecycle - if lc.mode != codex_lifecycle.MODE_STRICT: - return - runs_dir = self.runs_dir() - try: - records = await asyncio.to_thread( - codex_lifecycle.list_records, runs_dir, - ) - except Exception: # noqa: BLE001 - logger.exception("codex lifecycle record scan failed") + """Reap cgroup scopes left by a previous daemon incarnation. Records of + the current generation belong to live runs and are skipped inside + ``reconcile``. Best effort — never blocks the monitor loop.""" + if self.config.codex.lifecycle.mode != codex_lifecycle.MODE_STRICT: return - if not records: - return - # Snapshot business terminality on the loop (DB access), then reap - # off-thread. A run whose row is gone is treated terminal. - terminal_ids: dict[str, bool] = {} - for run_id, _rec in records: - run = await self.db.get_workflow_run(run_id) - terminal_ids[run_id] = ( - run is None or run.get("status") in TERMINAL_STATUSES - ) - - def _is_terminal(run_id: str) -> bool: - return terminal_ids.get(run_id, True) - try: results = await asyncio.to_thread( - codex_lifecycle.reconcile_all, runs_dir, _is_terminal, - term_grace_seconds=lc.term_grace_seconds, - stop_timeout_seconds=lc.stop_timeout_seconds, + codex_lifecycle.reconcile, self.runs_dir(), ) except Exception: # noqa: BLE001 logger.exception("codex lifecycle reconciliation failed") return - for run_id, receipt in results: - self._journal_lifecycle(run_id, "reconcile", receipt) - retry_owed = [rid for rid, rc in results if rc.needs_retry()] - if retry_owed: - logger.warning( - "codex lifecycle reconciliation left %d run(s) needing a reap " - "retry (nerve codex reap-descendants ): %s", - len(retry_owed), ", ".join(retry_owed), - ) - attention = [rid for rid, rc in results if rc.needs_attention()] - if attention: + stuck = [rid for rid, rc in results if not rc.is_complete()] + if stuck: logger.warning( - "codex lifecycle reconciliation refused %d run(s) whose scope " - "was replaced by a foreign unit: %s", - len(attention), ", ".join(attention), + "codex lifecycle reconciliation could not confirm cleanup for: %s", + ", ".join(stuck), ) async def _reap_codex_lifecycle(self, run_id: str) -> None: - """Terminal-path reap for one run. No-ops unless it recorded a scope.""" - lc = self.config.codex.lifecycle - run_dir = self.runs_dir() / run_id + """Reap one terminal run's contained descendants from its record.""" try: - receipt = await asyncio.to_thread( - codex_lifecycle.reap, run_dir, - term_grace_seconds=lc.term_grace_seconds, - stop_timeout_seconds=lc.stop_timeout_seconds, - ) + await asyncio.to_thread(codex_lifecycle.reap, self.runs_dir() / run_id) except Exception: # noqa: BLE001 logger.exception("codex lifecycle reap failed for %s", run_id) - return - if receipt.outcome != "no_scope": - self._journal_lifecycle(run_id, "reap", receipt) - - async def retry_lifecycle_reap(self, run_id: str) -> dict: - """The one supported, narrowly-scoped retry trigger (service entry). - - Re-attempts a ``pending_retry`` reap from the durable record and - returns the resulting receipt. Idempotent; never fabricates success. - A run with no record yields ``no_scope``. - """ - run_dir = self.runs_dir() / run_id - lc = self.config.codex.lifecycle - receipt = await asyncio.to_thread( - codex_lifecycle.retry, run_dir, - term_grace_seconds=lc.term_grace_seconds, - stop_timeout_seconds=lc.stop_timeout_seconds, - ) - if receipt.outcome != "no_scope": - self._journal_lifecycle(run_id, "reap_retry", receipt) - from dataclasses import asdict as _asdict - - return _asdict(receipt) - - def _journal_lifecycle(self, run_id: str, event: str, receipt: Any) -> None: - """Append a lifecycle receipt summary to the run's journal. - - Receipt fields carry no argv/credentials/environment — only outcome, - method, and identity-verification booleans. - """ - from dataclasses import asdict as _asdict - - journal_dir = self.runs_dir() / run_id - try: - path = journal_dir / "events.ndjson" - if not journal_dir.is_dir(): - return - line = json.dumps({ - "ts": utc_now_iso(), "run_id": run_id, - "event": f"lifecycle_{event}", **_asdict(receipt), - }) - with path.open("a", encoding="utf-8") as f: - f.write(line + "\n") - except OSError: - logger.debug("lifecycle journal write failed for %s", run_id) # ------------------------------------------------------------------ # # Budget monitor # diff --git a/tests/fixtures/lifecycle_escaper.py b/tests/fixtures/lifecycle_escaper.py index 03ec56931..b256e3806 100755 --- a/tests/fixtures/lifecycle_escaper.py +++ b/tests/fixtures/lifecycle_escaper.py @@ -16,8 +16,6 @@ (root cgroup.procs empties; recursive cgroup.events populated stays 1) spawn-escapee-igterm grandchild ignores SIGTERM (forces KILL) - spawn-escapee-forker grandchild forks 3 more sleepers on SIGTERM - (fork-during-teardown; all in the cgroup) sleeper sleep (a leaf) sweep identity-checked pidfd cleanup of the token """ @@ -152,12 +150,6 @@ def _double_fork_grandchild(token: str, outdir: str, variant: str) -> None: pass # non-fatal: fall back to plain containment elif variant == "igterm": signal.signal(signal.SIGTERM, signal.SIG_IGN) - elif variant == "forker": - def _spawn_more(_sig, _frm): - for _ in range(3): - if os.fork() == 0: - _become_sleeper(token, "forked", outdir) - signal.signal(signal.SIGTERM, _spawn_more) _write(outdir, "grandchild.pid", str(os.getpid())) _become_sleeper(token, "grandchild", outdir) @@ -197,8 +189,6 @@ def main() -> int: return _spawn("nested", sys.argv[2:]) if cmd == "spawn-escapee-igterm": return _spawn("igterm", sys.argv[2:]) - if cmd == "spawn-escapee-forker": - return _spawn("forker", sys.argv[2:]) if cmd == "sweep": token = os.environ.get("ESCAPER_TOKEN") or (sys.argv[2] if len(sys.argv) > 2 else "") print(sweep(token)) diff --git a/tests/test_codex_lifecycle.py b/tests/test_codex_lifecycle.py index 74c6598a0..741ad4ad2 100644 --- a/tests/test_codex_lifecycle.py +++ b/tests/test_codex_lifecycle.py @@ -1,36 +1,27 @@ -"""R6 acceptance suite for Codex workflow-run descendant containment. - -These are assertion-bearing tests, not smoke checks. Where the review demands -it, they use **real** systemd user scopes running **real** setsid/double-fork -escaper processes (``tests/fixtures/lifecycle_escaper.py``) and the **real** -fake app-server (``tests/fixtures/fake_codex_appserver.py``) through the actual -launch wrapper — no paid model, no network. Every scope/process a test creates -is torn down by the ``scopes`` fixture (``systemctl --user stop`` + an -identity-checked pidfd sweep), and nothing ever touches the host user manager, -the daemon, the real ``~/.nerve/workflow-runs``, or any pre-existing process. - -Cases map to the review's R6.1–R6.8. Tests needing a scope skip cleanly on a -host without a reachable systemd ``--user`` manager. +"""Tests for Codex workflow-run cgroup containment (nerve/agent/backends/codex/lifecycle.py). + +The containment/reap cases use real systemd user scopes running real +setsid/double-fork escapers (tests/fixtures/lifecycle_escaper.py) and the real +fake app-server through the actual launch wrapper — no model, no network. Every +scope/process is torn down by the ``scopes`` fixture. Scope cases skip on a host +without a reachable systemd ``--user`` manager. """ from __future__ import annotations -import asyncio -import json import os import subprocess import sys -import threading import uuid from pathlib import Path import pytest -from nerve.agent.backends import BackendDeps, SessionSpec, TransportDiedError -from nerve.agent.backends import events as ev -from nerve.agent.backends.base import TurnInput +from nerve.agent.backends import BackendDeps, SessionSpec +from nerve.agent.backends.base import TransportDiedError from nerve.agent.backends.codex import CodexBackend, lifecycle from nerve.agent.backends.codex.appserver import CodexAppServerClient +from nerve.agent.backends.codex.backend import _workflow_containment from nerve.agent.interactive import InteractiveToolHandler from nerve.config import NerveConfig @@ -44,9 +35,6 @@ ) -# ----------------------------- helpers/fixtures ---------------------------- # - - def _alive(pid: int) -> bool: try: os.kill(pid, 0) @@ -59,7 +47,6 @@ def _alive(pid: int) -> bool: @pytest.fixture def scopes(): - """Guaranteed teardown of every scope/process a test creates.""" reg: dict[str, set] = {"units": set(), "tokens": set(), "run_dirs": set()} try: yield reg @@ -72,84 +59,40 @@ def scopes(): except Exception: # noqa: BLE001 pass for unit in reg["units"]: - subprocess.run( - ["systemctl", "--user", "stop", unit], - capture_output=True, timeout=15, - ) + subprocess.run(["systemctl", "--user", "stop", unit], + capture_output=True, timeout=15) for tok in reg["tokens"]: - subprocess.run( - [sys.executable, ESCAPER, "sweep", tok], - env={**os.environ, "ESCAPER_TOKEN": tok}, - capture_output=True, timeout=15, - ) - - -def _launch_escaper( - scopes, run_dir: Path, run_id: str, *, variant: str = "", - term_grace: int = 2, mode: str = "strict", - session_id: str | None = None, invocation_id: str | None = None, -) -> tuple[lifecycle.LifecycleRecord, int, str]: - """Launch a real escaper inside a real scope; return (record, gc_pid, unit). - - The escaper's main process exits after the setsid/double-fork grandchild is - up, so the grandchild is left orphaned inside the scope — exactly the - incident shape. A strict lifecycle record is written and (unless overridden) - its live InvocationID/ControlGroup recorded, so ``reap`` can act on it. - """ + subprocess.run([sys.executable, ESCAPER, "sweep", tok], + env={**os.environ, "ESCAPER_TOKEN": tok}, + capture_output=True, timeout=15) + + +def _launch_escaper(scopes, run_dir: Path, run_id: str, *, variant: str = "", + invocation_id: str | None = None): + """Launch a real escaper in a real scope; return (record, gc_pid).""" run_dir = Path(run_dir) run_dir.mkdir(parents=True, exist_ok=True) token = "lc" + uuid.uuid4().hex[:12] scopes["tokens"].add(token) - nonce = uuid.uuid4().hex - unit = lifecycle.scope_unit_name(run_id, nonce) + unit = lifecycle.scope_unit_name(run_id, uuid.uuid4().hex) scopes["units"].add(unit) cmd = "spawn-escapee" + (f"-{variant}" if variant else "") - inner = [sys.executable, ESCAPER, cmd, str(run_dir)] - argv = lifecycle.build_scope_argv(unit, inner, term_grace_seconds=term_grace) - subprocess.run( - argv, env={**os.environ, "ESCAPER_TOKEN": token}, - capture_output=True, timeout=30, check=True, - ) + argv = lifecycle.build_scope_argv(unit, [sys.executable, ESCAPER, cmd, str(run_dir)]) + subprocess.run(argv, env={**os.environ, "ESCAPER_TOKEN": token}, + capture_output=True, timeout=30, check=True) gc_pid = int((run_dir / "grandchild.pid").read_text()) - inst = lifecycle.current_instance() rec = lifecycle.LifecycleRecord( - run_id=run_id, session_id=session_id or f"workflow:{run_id}", mode=mode, - attempt_nonce=nonce, unit=unit, workspace_id="ws", boot_id=inst.boot_id, - instance_generation=inst.generation, phase="intent", + run_id=run_id, session_id=f"workflow:{run_id}", unit=unit, + boot_id=lifecycle._boot_id(), generation=lifecycle.current_generation(), ) lifecycle.write_record(run_dir, rec) if invocation_id is not None: - # Caller wants a deliberately mismatched identity (replacement test). _inv, cg = lifecycle._await_unit_identity(unit) - rec.invocation_id = invocation_id - rec.control_group = cg - rec.phase = "launched" + rec.invocation_id, rec.control_group = invocation_id, cg lifecycle.write_record(run_dir, rec) else: rec = lifecycle.record_launched(run_dir, rec) - return rec, gc_pid, unit - - -# ---- backend/turn helpers (mirroring tests/test_codex_appserver.py) ------- # - - -def _wf_config(tmp_path: Path, *, mode: str = "strict", **codex) -> NerveConfig: - cfg = NerveConfig.from_dict({ - "workspace": str(tmp_path / "ws"), - "workflows": {"runs_dir": str(tmp_path / "workflow-runs")}, - "codex": { - "bin_path": FAKE, - "home_dir": str(tmp_path / "codex-home"), - "model": "gpt-5.6-sol", - "lifecycle": { - "mode": mode, "term_grace_seconds": 2, "stop_timeout_seconds": 8, - }, - **codex, - }, - }) - (tmp_path / "ws").mkdir(parents=True, exist_ok=True) - (tmp_path / "workflow-runs").mkdir(parents=True, exist_ok=True) - return cfg + return rec, gc_pid def _deps(cfg: NerveConfig) -> BackendDeps: @@ -161,543 +104,209 @@ def _deps(cfg: NerveConfig) -> BackendDeps: def _spec(cfg: NerveConfig, *, session_id: str, source: str) -> SessionSpec: - hub = InteractiveToolHandler( - session_id=session_id, broadcast_fn=_noop_broadcast, - interactive_capable=False, - ) + hub = InteractiveToolHandler(session_id=session_id, broadcast_fn=_noop, interactive_capable=False) return SessionSpec( session_id=session_id, source=source, model=None, effort="high", - system_prompt="You are Nerve.", cwd=str(cfg.workspace), - resume_native_id=None, fork=False, interactive=hub, - snapshot=None, record_wakeup=None, idle_timeout=15.0, + system_prompt="You are Nerve.", cwd=str(cfg.workspace), resume_native_id=None, + fork=False, interactive=hub, snapshot=None, record_wakeup=None, idle_timeout=15.0, ) -async def _noop_broadcast(session_id: str, message: dict) -> None: +async def _noop(session_id: str, message: dict) -> None: return None -async def _collect_turn(client) -> list: - return [event async for event in client.receive_turn()] +def _wf_config(tmp_path: Path, *, mode: str = "strict") -> NerveConfig: + cfg = NerveConfig.from_dict({ + "workspace": str(tmp_path / "ws"), + "workflows": {"runs_dir": str(tmp_path / "workflow-runs")}, + "codex": {"bin_path": FAKE, "home_dir": str(tmp_path / "home"), + "model": "gpt-5.6-sol", "lifecycle": {"mode": mode}}, + }) + (tmp_path / "ws").mkdir(parents=True, exist_ok=True) + (tmp_path / "workflow-runs").mkdir(parents=True, exist_ok=True) + return cfg -# =========================================================================== # -# R6.1 — adapter + fake app-server through the real containment wrapper -# =========================================================================== # +# -- launch path ------------------------------------------------------------ # @requires_scope @pytest.mark.asyncio -async def test_r61_strict_launch_roundtrip_preserves_argv_env_cwd(tmp_path, scopes): - """initialize round-trips through systemd-run --scope with clean JSON - stdout; literal ``${...}`` argv, env, and cwd are preserved; the app-server - is verifiably contained and registration is durable.""" - run_dir = tmp_path / "workflow-runs" / "wfr-r61" +async def test_strict_launch_contains_and_roundtrips(tmp_path, scopes): + run_dir = tmp_path / "workflow-runs" / "wfr-l1" scopes["run_dirs"].add(run_dir) containment = lifecycle.WorkflowContainment( - mode="strict", run_dir=run_dir, run_id="wfr-r61", - session_id="workflow:wfr-r61", workspace_id="ws", - term_grace_seconds=2, stop_timeout_seconds=8, + mode="strict", run_dir=run_dir, run_id="wfr-l1", session_id="workflow:wfr-l1", ) - literal = "keep-${NERVE_SENTINEL_DO_NOT_EXPAND}-literal" + literal = "keep-${NERVE_SENTINEL_DO_NOT_EXPAND}" cwd = tmp_path / "ws" cwd.mkdir(parents=True, exist_ok=True) - async def _handler(method, params): # pragma: no cover - unused here + async def _handler(method, params): return {} client = CodexAppServerClient( bin_path=FAKE, cwd=str(cwd), - env={**os.environ, "CODEX_HOME": str(tmp_path / "home"), - "FAKE_CODEX_MODE": "basic"}, - server_request_handler=_handler, - config_overrides=[f"sentinel={literal}"], + env={**os.environ, "CODEX_HOME": str(tmp_path / "home"), "FAKE_CODEX_MODE": "basic"}, + server_request_handler=_handler, config_overrides=[f"sentinel={literal}"], containment=containment, ) resp = await client.start() try: - # Clean JSON on stdout (no systemd-run control text) — it parsed. - assert resp.get("userAgent") - # Literal argv preserved verbatim through --expand-environment=no. - assert any(literal in v for v in resp["_fake"]["configOverrides"]) - # Environment forwarded into the scope. - assert resp["_fake"]["env"]["CODEX_HOME"] == str(tmp_path / "home") - + assert resp.get("userAgent") # clean JSON stdout + assert any(literal in v for v in resp["_fake"]["configOverrides"]) # literal argv preserved + assert resp["_fake"]["env"]["CODEX_HOME"] == str(tmp_path / "home") # env forwarded rec = lifecycle.read_record(run_dir) - assert rec is not None and rec.phase == "registered" assert rec.invocation_id and rec.control_group - # The scope is populated (a live contained app-server). - _state, populated = lifecycle._cgroup_events(rec.control_group) - assert populated is True - # cwd preserved: a scope member reports our cwd. - procs = Path("/sys/fs/cgroup" + rec.control_group + "/cgroup.procs") - pids = [int(p) for p in procs.read_text().split()] - assert pids - assert any( + assert lifecycle._cgroup_gone_or_empty(rec.control_group) is False # populated + procs = Path("/sys/fs/cgroup" + rec.control_group + "/cgroup.procs").read_text().split() + assert procs and any( os.readlink(f"/proc/{p}/cwd") == str(cwd) - for p in pids if Path(f"/proc/{p}").exists() + for p in map(int, procs) if Path(f"/proc/{p}").exists() ) finally: await client.close() -@requires_scope -@pytest.mark.asyncio -@pytest.mark.parametrize("mode", ["basic", "big_line"]) -async def test_r61_contained_turn_streams_including_large_records( - tmp_path, monkeypatch, scopes, mode, -): - """A full turn (incremental notifications; a ~2 MiB single-line tool result - for backpressure) completes through the containment wrapper unchanged.""" - monkeypatch.setenv("FAKE_CODEX_MODE", mode) - cfg = _wf_config(tmp_path) - scopes["run_dirs"].add(tmp_path / "workflow-runs" / "wfr-turn") +def test_disabled_and_non_workflow_build_no_containment(tmp_path): + cfg = _wf_config(tmp_path, mode="strict") backend = CodexBackend(_deps(cfg)) - client = await backend.create_client( - _spec(cfg, session_id="workflow:wfr-turn", source="workflow"), - ) - try: - rec = lifecycle.read_record(tmp_path / "workflow-runs" / "wfr-turn") - assert rec is not None and rec.phase == "registered" - await client.start_turn(TurnInput(text="hi")) - events = await _collect_turn(client) - assert any(isinstance(e, ev.TurnCompleted) for e in events) - assert events[-1].status == "completed" # transport survived the turn - finally: - await client.disconnect() + # interactive/cron session: never contained, even in strict mode + assert _workflow_containment(backend, _spec(cfg, session_id="s1", source="web")) is None + # disabled mode: workflow session not contained + cfg2 = _wf_config(tmp_path, mode="disabled") + backend2 = CodexBackend(_deps(cfg2)) + assert _workflow_containment(backend2, _spec(cfg2, session_id="workflow:wfr-x", source="workflow")) is None -@requires_scope -@pytest.mark.asyncio -async def test_r61_contained_appserver_death_propagates_exit(tmp_path, monkeypatch, scopes): - """die_mid_turn: the app-server exits mid-turn; the exit propagates through - the wrapper (transport dies) rather than hanging.""" - monkeypatch.setenv("FAKE_CODEX_MODE", "die_mid_turn") - cfg = _wf_config(tmp_path) - scopes["run_dirs"].add(tmp_path / "workflow-runs" / "wfr-die") - backend = CodexBackend(_deps(cfg)) - client = await backend.create_client( - _spec(cfg, session_id="workflow:wfr-die", source="workflow"), - ) - try: - await client.start_turn(TurnInput(text="hi")) - with pytest.raises(TransportDiedError): - await _collect_turn(client) - finally: - await client.disconnect() +def test_strict_unavailable_fails_before_exec(tmp_path, monkeypatch): + run_dir = tmp_path / "u" + run_dir.mkdir() + monkeypatch.setattr(lifecycle, "detect_capabilities", + lambda **k: lifecycle.Capabilities(ok=False, reason="no systemd-run")) + c = lifecycle.WorkflowContainment(mode="strict", run_dir=run_dir, run_id="wfr-u", + session_id="workflow:wfr-u") + with pytest.raises(lifecycle.ContainmentUnavailable): + lifecycle.prepare_launch(c, ["/usr/bin/codex", "app-server"]) + assert not (run_dir / lifecycle.RECORD_NAME).exists() # nothing launched/recorded -# =========================================================================== # -# R6.2 — success/error/cancel/timeout/budget converge on one reap; foreign runs survive -# =========================================================================== # +# -- reap -------------------------------------------------------------------- # @requires_scope -def test_r62_reap_kills_contained_escapee_and_spares_foreign(tmp_path, scopes): - target, gc_t, _u = _launch_escaper(scopes, tmp_path / "t", "wfr-target") - foreign, gc_f, _uf = _launch_escaper(scopes, tmp_path / "f", "wfr-foreign") - - # Both escaped grandchildren are alive and each is contained in its scope. +def test_reap_kills_contained_and_spares_foreign(tmp_path, scopes): + target, gc_t = _launch_escaper(scopes, tmp_path / "t", "wfr-target") + foreign, gc_f = _launch_escaper(scopes, tmp_path / "f", "wfr-foreign") assert _alive(gc_t) and _alive(gc_f) assert lifecycle.proc_cgroup(gc_t) == target.control_group - assert lifecycle.proc_cgroup(gc_f) == foreign.control_group - - receipt = lifecycle.reap(tmp_path / "t", term_grace_seconds=2, stop_timeout_seconds=8) - assert receipt.outcome == "complete" - assert receipt.signalled and receipt.verified_empty - assert not _alive(gc_t) # target's escapee is reaped - assert _alive(gc_f) # the foreign run is untouched - - # The receipt is persisted terminal on the record. - rec = lifecycle.read_record(tmp_path / "t") - assert rec.phase == "terminal" and rec.receipt["outcome"] == "complete" - -# =========================================================================== # -# R6.3 — nested cgroups, TERM-ignoring, fork-during-teardown; recursive populated -# =========================================================================== # + r = lifecycle.reap(tmp_path / "t") + assert r.outcome == "complete" and r.signalled and r.verified_empty + assert not _alive(gc_t) # target reaped + assert _alive(gc_f) # foreign run untouched + assert lifecycle.read_record(tmp_path / "t").receipt["outcome"] == "complete" @requires_scope -def test_r63_nested_cgroup_uses_recursive_populated(tmp_path, scopes): - rec, gc, _u = _launch_escaper(scopes, tmp_path / "n", "wfr-nested", variant="nested") - # The grandchild migrated into a child cgroup: the scope ROOT holds no - # procs, yet recursive cgroup.events populated is 1 (the emptiness signal - # root cgroup.procs alone would miss). - root_procs = Path("/sys/fs/cgroup" + rec.control_group + "/cgroup.procs").read_text().split() - assert root_procs == [] - _state, populated = lifecycle._cgroup_events(rec.control_group) - assert populated is True +def test_reap_nested_cgroup_uses_recursive_populated(tmp_path, scopes): + rec, gc = _launch_escaper(scopes, tmp_path / "n", "wfr-nested", variant="nested") + # grandchild migrated into a child cgroup: root cgroup.procs is empty, but + # recursive cgroup.events populated is 1 (what reap must check). + assert Path("/sys/fs/cgroup" + rec.control_group + "/cgroup.procs").read_text().split() == [] + assert lifecycle._cgroup_gone_or_empty(rec.control_group) is False assert lifecycle.proc_cgroup(gc) == rec.control_group + "/nested" - - receipt = lifecycle.reap(tmp_path / "n", term_grace_seconds=2, stop_timeout_seconds=8) - assert receipt.outcome == "complete" and receipt.verified_empty + assert lifecycle.reap(tmp_path / "n").outcome == "complete" assert not _alive(gc) @requires_scope -def test_r63_term_ignoring_descendant_escalated_to_kill(tmp_path, scopes): - rec, gc, _u = _launch_escaper( - scopes, tmp_path / "ig", "wfr-igterm", variant="igterm", term_grace=2, - ) +def test_reap_escalates_term_ignoring_descendant_to_kill(tmp_path, scopes): + rec, gc = _launch_escaper(scopes, tmp_path / "ig", "wfr-igterm", variant="igterm") assert _alive(gc) - receipt = lifecycle.reap(tmp_path / "ig", term_grace_seconds=2, stop_timeout_seconds=10) - # A descendant that ignores TERM is still killed by the bounded - # TERM→grace→SIGKILL sequence (SendSIGKILL=yes, KillMode=control-group). - assert receipt.outcome == "complete" and receipt.verified_empty + r = lifecycle.reap(tmp_path / "ig") + assert r.outcome == "complete" and r.verified_empty # SIGKILL escalation assert not _alive(gc) @requires_scope -def test_r63_fork_during_teardown_all_reaped(tmp_path, scopes): - rec, gc, _u = _launch_escaper( - scopes, tmp_path / "fk", "wfr-forker", variant="forker", term_grace=2, - ) - token_before = _scope_proc_count(rec.control_group) - assert token_before >= 1 - receipt = lifecycle.reap(tmp_path / "fk", term_grace_seconds=2, stop_timeout_seconds=10) - # Children forked *during* teardown are in the cgroup too, so the - # control-group kill takes them all — verified by recursive emptiness. - assert receipt.outcome == "complete" and receipt.verified_empty - assert not _alive(gc) - - -def _scope_proc_count(control_group: str) -> int: - total = 0 - root = Path("/sys/fs/cgroup" + control_group) - for procs in root.rglob("cgroup.procs"): - try: - total += len(procs.read_text().split()) - except OSError: - pass - return total - - -# =========================================================================== # -# R6.4 — crash windows + fresh-process reconcile -# =========================================================================== # - - -@requires_scope -def test_r64_reconcile_reaps_abandoned_launch_from_fresh_generation(tmp_path, scopes): - """A launch whose controller 'crashed' (record left at 'launched', a - different daemon generation) is reaped by startup reconciliation reading - only the durable record.""" - rec, gc, _u = _launch_escaper(scopes, tmp_path / "c", "wfr-crash") - assert rec.phase == "launched" and _alive(gc) - # Reconcile as a *fresh* daemon generation (the crashed one is gone). - receipt = lifecycle.reconcile_run( - tmp_path / "c", is_terminal=True, - active_generation="fresh-other-generation", - term_grace_seconds=2, stop_timeout_seconds=8, - ) - assert receipt is not None and receipt.outcome == "complete" - assert not _alive(gc) - - -def test_r64_reconcile_skips_current_generation_owner(tmp_path): - """Reconcile never touches a run this very generation still owns.""" - run_dir = tmp_path / "own" - run_dir.mkdir() - inst = lifecycle.current_instance() - rec = lifecycle.LifecycleRecord( - run_id="wfr-own", session_id="workflow:wfr-own", mode="strict", - attempt_nonce="n1", unit="nerve-wf-wfr-own-n1.scope", workspace_id="ws", - boot_id=inst.boot_id, instance_generation=inst.generation, phase="launched", - ) - lifecycle.write_record(run_dir, rec) - out = lifecycle.reconcile_run( - run_dir, is_terminal=False, active_generation=inst.generation, - ) - assert out is None # active owner, same generation → left alone - - -def test_r64_intent_only_abandoned_launch_resolves_absent(tmp_path): - """A pre-launch crash (intent recorded, scope never created) reconciles to - resolved_absent — nothing to signal, no false completion of a live tree.""" - run_dir = tmp_path / "int" - run_dir.mkdir() - inst = lifecycle.current_instance() - rec = lifecycle.LifecycleRecord( - run_id="wfr-int", session_id="workflow:wfr-int", mode="strict", - attempt_nonce="n2", unit="nerve-wf-wfr-int-n2.scope", workspace_id="ws", - boot_id=inst.boot_id, instance_generation="old-generation", phase="intent", - ) - lifecycle.write_record(run_dir, rec) - receipt = lifecycle.reconcile_run( - run_dir, is_terminal=False, active_generation=inst.generation, - ) - assert receipt is not None - assert receipt.outcome == "resolved_absent" and not receipt.signalled - - -# =========================================================================== # -# R6.5 — identity: replacement, cross-boot, corrupt, unwritable/fsync, manager, no PID signal -# =========================================================================== # +def test_reap_is_idempotent(tmp_path, scopes, monkeypatch): + rec, gc = _launch_escaper(scopes, tmp_path / "id", "wfr-idem") + assert lifecycle.reap(tmp_path / "id").outcome == "complete" + monkeypatch.setattr(lifecycle, "_scope_stop", _fail_if_called) # must not re-stop + assert lifecycle.reap(tmp_path / "id").outcome == "complete" @requires_scope -def test_r65_refuses_replaced_unit_and_spares_it(tmp_path, scopes): - """When the recorded InvocationID no longer matches the live unit (a - same-name replacement), reap refuses to signal — the foreign process that - now owns the name survives.""" - rec, gc, _u = _launch_escaper( - scopes, tmp_path / "rep", "wfr-rep", - invocation_id="00000000000000000000000000000000", # deliberately wrong - ) +def test_reap_refuses_replaced_unit(tmp_path, scopes): + # Record a wrong InvocationID: the live unit is a foreign instance of the + # same name; reap must refuse to signal it. + rec, gc = _launch_escaper(scopes, tmp_path / "r", "wfr-rep", + invocation_id="00000000000000000000000000000000") + r = lifecycle.reap(tmp_path / "r") + assert r.outcome == "refused" and not r.signalled and not r.is_complete() assert _alive(gc) - receipt = lifecycle.reap(tmp_path / "rep", term_grace_seconds=2, stop_timeout_seconds=8) - assert receipt.outcome == "failed_replaced" - assert not receipt.signalled and not receipt.is_complete() - assert receipt.needs_attention() - assert _alive(gc) # the (foreign) unit was NOT signalled -def test_r65_cross_boot_record_never_signals(tmp_path, monkeypatch): - run_dir = tmp_path / "boot" +def test_reap_cross_boot_record_never_signals(tmp_path, monkeypatch): + run_dir = tmp_path / "b" run_dir.mkdir() - rec = lifecycle.LifecycleRecord( - run_id="wfr-boot", session_id="workflow:wfr-boot", mode="strict", - attempt_nonce="n3", unit="nerve-wf-wfr-boot-n3.scope", workspace_id="ws", - boot_id="0000-prior-boot", instance_generation="old", phase="registered", - invocation_id="abc", control_group="/user.slice/nerve-wf-wfr-boot-n3.scope", - ) - lifecycle.write_record(run_dir, rec) - # No unit lookup / no signal should occur for a prior-boot record. + lifecycle.write_record(run_dir, lifecycle.LifecycleRecord( + run_id="wfr-b", session_id="workflow:wfr-b", unit="nerve-wf-wfr-b-n.scope", + boot_id="prior-boot", generation="old", invocation_id="i", + control_group="/user.slice/nerve-wf-wfr-b-n.scope", + )) monkeypatch.setattr(lifecycle, "_scope_stop", _fail_if_called) - receipt = lifecycle.reap(run_dir) - assert receipt.outcome == "resolved_prior_boot" and not receipt.signalled + r = lifecycle.reap(run_dir) + assert r.outcome == "complete" and not r.signalled # nothing survives a reboot -def test_r65_corrupt_record_escalates_without_signal(tmp_path, monkeypatch): - run_dir = tmp_path / "corrupt" +def test_reap_corrupt_record_does_not_signal(tmp_path, monkeypatch): + run_dir = tmp_path / "c" run_dir.mkdir() - (run_dir / lifecycle.RECORD_NAME).write_text("{ this is not valid json ") + (run_dir / lifecycle.RECORD_NAME).write_text("{ not valid json ") monkeypatch.setattr(lifecycle, "_scope_stop", _fail_if_called) - receipt = lifecycle.reap(run_dir) - assert receipt.outcome == "pending_retry" and not receipt.is_complete() - - -def test_r65_unpersistable_receipt_never_reports_complete(tmp_path, monkeypatch): - """If the completion receipt cannot be fsynced/written, reap must not - report complete (R2).""" - run_dir = tmp_path / "fsync" - run_dir.mkdir() - inst = lifecycle.current_instance() - rec = lifecycle.LifecycleRecord( - run_id="wfr-fs", session_id="workflow:wfr-fs", mode="strict", - attempt_nonce="n4", unit="nerve-wf-wfr-fs-n4.scope", workspace_id="ws", - boot_id=inst.boot_id, instance_generation="old", phase="launched", - invocation_id="inv", control_group="/user.slice/nerve-wf-wfr-fs-n4.scope", - ) - lifecycle.write_record(run_dir, rec) - # Unit resolves + stop succeeds + scope verified empty … - monkeypatch.setattr(lifecycle, "_unit_identity", lambda u: { - "LoadState": "loaded", "ActiveState": "active", - "InvocationID": "inv", "ControlGroup": rec.control_group, - "Description": lifecycle.SCOPE_DESCRIPTION, - }) - monkeypatch.setattr(lifecycle, "_scope_stop", lambda u, timeout: (True, "")) - monkeypatch.setattr(lifecycle, "_cgroup_events", lambda cg: ("gone", None)) - # … but persisting the receipt fails. - monkeypatch.setattr(os, "fsync", _raise_oserror) - with pytest.raises((OSError, lifecycle.LifecycleError)): - lifecycle.reap(run_dir) - # On-disk record must NOT claim completion. - on_disk = json.loads((run_dir / lifecycle.RECORD_NAME).read_text()) - assert (on_disk.get("receipt") or {}).get("outcome") != "complete" - - -def test_r65_manager_unavailable_stays_pending_without_pid_signal(tmp_path, monkeypatch): - run_dir = tmp_path / "mgr" - run_dir.mkdir() - inst = lifecycle.current_instance() - rec = lifecycle.LifecycleRecord( - run_id="wfr-mgr", session_id="workflow:wfr-mgr", mode="strict", - attempt_nonce="n5", unit="nerve-wf-wfr-mgr-n5.scope", workspace_id="ws", - boot_id=inst.boot_id, instance_generation="old", phase="launched", - invocation_id="inv", control_group="/user.slice/nerve-wf-wfr-mgr-n5.scope", - ) - lifecycle.write_record(run_dir, rec) - monkeypatch.setattr(lifecycle, "_unit_identity", lambda u: { - "LoadState": "loaded", "ActiveState": "active", "InvocationID": "inv", - "ControlGroup": rec.control_group, "Description": lifecycle.SCOPE_DESCRIPTION, - }) - monkeypatch.setattr(lifecycle, "_scope_stop", lambda u, timeout: (False, "manager down")) - monkeypatch.setattr(lifecycle, "_cgroup_events", lambda cg: ("present", True)) - # The reap must never fall back to signalling a raw PID / process group. - monkeypatch.setattr(os, "kill", _raise_if_called) - monkeypatch.setattr(os, "killpg", _raise_if_called) - receipt = lifecycle.reap(run_dir) - assert receipt.outcome == "pending_retry" and not receipt.is_complete() - assert receipt.needs_retry() + assert lifecycle.reap(run_dir).outcome == "pending_retry" def _fail_if_called(*a, **k): - raise AssertionError("scope stop must not be called for this case") - - -def _raise_if_called(*a, **k): - raise AssertionError("a raw PID/process-group signal must never be sent") - + raise AssertionError("scope stop must not be called here") -def _raise_oserror(*a, **k): - raise OSError("simulated fsync failure") - -# =========================================================================== # -# R6.6 — concurrency + repeated terminal cleanup + real durable retry -# =========================================================================== # - - -@requires_scope -def test_r66_repeated_reap_is_idempotent(tmp_path, scopes, monkeypatch): - rec, gc, _u = _launch_escaper(scopes, tmp_path / "idem", "wfr-idem") - r1 = lifecycle.reap(tmp_path / "idem", term_grace_seconds=2, stop_timeout_seconds=8) - assert r1.outcome == "complete" - # A second reap must not stop anything again — the terminal receipt stands. - monkeypatch.setattr(lifecycle, "_scope_stop", _fail_if_called) - r2 = lifecycle.reap(tmp_path / "idem") - assert r2.outcome == "complete" and r2.attempts == r1.attempts - - -@requires_scope -def test_r66_initial_failure_then_real_durable_retry(tmp_path, scopes, monkeypatch): - rec, gc, _u = _launch_escaper(scopes, tmp_path / "retry", "wfr-retry") - real_stop = lifecycle._scope_stop - monkeypatch.setattr(lifecycle, "_scope_stop", lambda u, timeout: (False, "transient manager glitch")) - first = lifecycle.reap(tmp_path / "retry", term_grace_seconds=2, stop_timeout_seconds=8) - assert first.outcome == "pending_retry" and _alive(gc) - # Restore the manager and use the supported retry entry — a REAL reap. - monkeypatch.setattr(lifecycle, "_scope_stop", real_stop) - second = lifecycle.retry(tmp_path / "retry", term_grace_seconds=2, stop_timeout_seconds=8) - assert second.outcome == "complete" and second.verified_empty - assert second.attempts == 2 # durable attempt count advanced - assert not _alive(gc) +# -- reconcile --------------------------------------------------------------- # @requires_scope -def test_r66_concurrent_reaps_serialize(tmp_path, scopes): - rec, gc, _u = _launch_escaper(scopes, tmp_path / "conc", "wfr-conc") - results: list[lifecycle.Receipt] = [] - barrier = threading.Barrier(3) - - def _worker(): - barrier.wait() - results.append(lifecycle.reap(tmp_path / "conc", term_grace_seconds=2, stop_timeout_seconds=8)) - - threads = [threading.Thread(target=_worker) for _ in range(3)] - for t in threads: - t.start() - for t in threads: - t.join(timeout=30) - # The per-owner lock serialises them: all agree the tree is gone, and the - # scope was stopped exactly once (attempts never double-counts). - assert results and all(r.is_complete() for r in results) +def test_reconcile_reaps_prior_generation(tmp_path, scopes): + rec, gc = _launch_escaper(scopes, tmp_path / "wf" / "wfr-old", "wfr-old") + # Rewrite the record's generation to a prior incarnation's. + rec.generation = "prior-daemon-generation" + lifecycle.write_record(tmp_path / "wf" / "wfr-old", rec) + results = lifecycle.reconcile(tmp_path / "wf") + assert [rid for rid, _ in results] == ["wfr-old"] assert not _alive(gc) - assert lifecycle.read_record(tmp_path / "conc").receipt["outcome"] == "complete" - - -# =========================================================================== # -# R6.7 — interactive + a second workflow survive while the target is cleaned -# =========================================================================== # - - -@requires_scope -def test_r67_only_target_is_reaped_no_broad_kill(tmp_path, scopes, monkeypatch): - target, gc_t, _ = _launch_escaper(scopes, tmp_path / "wt", "wfr-victim") - other_wf, gc_o, _ = _launch_escaper(scopes, tmp_path / "wo", "wfr-other") - interactive, gc_i, _ = _launch_escaper(scopes, tmp_path / "wi", "wfr-interactive") - - monkeypatch.setattr(os, "killpg", _raise_if_called) # no broad process-group kill - receipt = lifecycle.reap(tmp_path / "wt", term_grace_seconds=2, stop_timeout_seconds=8) - assert receipt.outcome == "complete" - assert not _alive(gc_t) - assert _alive(gc_o) and _alive(gc_i) # concurrent workflow + interactive survive - -# =========================================================================== # -# R6.8 — disabled / observe / strict-unavailable declared behaviour -# =========================================================================== # - -def test_r68_disabled_mode_builds_no_containment(tmp_path): - cfg = _wf_config(tmp_path, mode="disabled") - backend = CodexBackend(_deps(cfg)) - from nerve.agent.backends.codex.backend import _workflow_containment - spec = _spec(cfg, session_id="workflow:wfr-x", source="workflow") - assert _workflow_containment(backend, spec) is None - - -def test_r68_non_workflow_session_builds_no_containment(tmp_path): - cfg = _wf_config(tmp_path, mode="strict") - backend = CodexBackend(_deps(cfg)) - from nerve.agent.backends.codex.backend import _workflow_containment - # An interactive/cron session is never contained, even in strict mode. - spec = _spec(cfg, session_id="s-interactive", source="web") - assert _workflow_containment(backend, spec) is None - - -def test_r68_observe_mode_does_not_wrap_and_makes_no_completion_claim(tmp_path): - run_dir = tmp_path / "obs" - run_dir.mkdir() - containment = lifecycle.WorkflowContainment( - mode="observe", run_dir=run_dir, run_id="wfr-obs", - session_id="workflow:wfr-obs", workspace_id="ws", - ) - inner = ["/usr/bin/codex", "app-server", "--listen", "stdio://"] - argv, rec = lifecycle.prepare_launch(containment, inner) - assert argv == inner # NOT wrapped — today's behaviour preserved - assert rec.mode == "observe" - receipt = lifecycle.reap(run_dir) - assert receipt.outcome == "unenforced_observe" - assert not receipt.enforced and not receipt.signalled - assert not receipt.is_complete() # observe is not a resolution - - -def test_r68_strict_unavailable_fails_before_exec(tmp_path, monkeypatch): - run_dir = tmp_path / "unavail" - run_dir.mkdir() - monkeypatch.setattr(lifecycle, "detect_capabilities", lambda **k: lifecycle.Capabilities( - ok=False, systemd_run=False, systemctl=False, user_manager=False, - cgroup_v2=True, reason="systemd-run not found", +def test_reconcile_skips_current_generation(tmp_path): + run_dir = tmp_path / "wf" / "wfr-live" + run_dir.mkdir(parents=True) + lifecycle.write_record(run_dir, lifecycle.LifecycleRecord( + run_id="wfr-live", session_id="workflow:wfr-live", + unit="nerve-wf-wfr-live-n.scope", boot_id=lifecycle._boot_id(), + generation=lifecycle.current_generation(), )) - containment = lifecycle.WorkflowContainment( - mode="strict", run_dir=run_dir, run_id="wfr-un", - session_id="workflow:wfr-un", workspace_id="ws", - ) - with pytest.raises(lifecycle.ContainmentUnavailable): - lifecycle.prepare_launch(containment, ["/usr/bin/codex", "app-server"]) - # Nothing was launched, and no half-written record claims containment. - assert not (run_dir / lifecycle.RECORD_NAME).exists() + assert lifecycle.reconcile(tmp_path / "wf") == [] # active owner, untouched -# =========================================================================== # -# Record validation / identity hardening (R2 unit-level) -# =========================================================================== # +# -- record validation ------------------------------------------------------- # -def test_record_rejects_unsafe_unit_and_foreign_cgroup(tmp_path): +def test_record_rejects_unsafe_unit_and_foreign_cgroup(): with pytest.raises(lifecycle.LifecycleError): - lifecycle.LifecycleRecord( - run_id="r", session_id="workflow:r", mode="strict", attempt_nonce="n", - unit="evil; rm -rf /.scope", workspace_id="ws", boot_id="b", - instance_generation="g", - ).validate() + lifecycle.LifecycleRecord(run_id="r", session_id="s", unit="evil; rm -rf.scope", + boot_id="b", generation="g").validate() with pytest.raises(lifecycle.LifecycleError): - lifecycle.LifecycleRecord( - run_id="r", session_id="workflow:r", mode="strict", attempt_nonce="n", - unit="nerve-wf-r-n.scope", workspace_id="ws", boot_id="b", - instance_generation="g", - control_group="/system.slice/other.scope", # not our unit / not user slice - ).validate() - - -def test_atomic_write_is_durable_and_survives_bad_tmp(tmp_path): - p = tmp_path / "rec.json" - lifecycle._atomic_write_json(p, {"a": 1}) - assert json.loads(p.read_text())["a"] == 1 - lifecycle._atomic_write_json(p, {"a": 2}) # overwrite - assert json.loads(p.read_text())["a"] == 2 - # no leftover temp files - assert list(tmp_path.glob(".rec.json*")) == [] + lifecycle.LifecycleRecord(run_id="r", session_id="s", unit="nerve-wf-r-n.scope", + boot_id="b", generation="g", + control_group="/system.slice/other.scope").validate() diff --git a/tmp/pr-body.md b/tmp/pr-body.md new file mode 100644 index 000000000..9be31e839 --- /dev/null +++ b/tmp/pr-body.md @@ -0,0 +1,35 @@ +## Problem + +When a Codex **workflow** run ends — completed, cancelled, budget-stopped, or the daemon crashed — some of its descendant processes can survive as orphans. Codex wraps each sandboxed command in `codex-linux-sandbox`, which `setsid`s into its own session/process group (and may double-fork). `CodexAppServerClient._signal_process_tree` signals a single process group (`os.killpg(proc.pid, sig)`), so those escaped descendants are never reached; the workflow teardown also assumes the in-memory client still covers every worker. The result is `codex-linux-sandbox` trees (and the `python`/`bash`/`pytest` they were running) left parented to PID 1 after the run is durably `done`. + +## Approach + +Opt-in, **workflow-only** containment. In `strict` mode a workflow run's `codex app-server` is launched **inside a dedicated delegated systemd user scope**, created *before* the command execs: + +``` +systemd-run --user --scope --quiet --collect --expand-environment=no \ + -p Delegate=yes -p KillMode=control-group -p SendSIGKILL=yes -p TimeoutStopSec= \ + --description="" --unit= -- +``` + +Everything the run forks inherits that cgroup (setsid/double-fork can't escape it), so a single bounded `systemctl --user stop` (TERM → grace → SIGKILL over the whole control group) reaps the entire tree. Cleanup is then verified — the scope's recursive `cgroup.events` `populated=0`, or the scope cgroup gone after a *successful* stop — never assumed. + +Each run writes a durable, validated record (`//lifecycle.json`, atomic write + file/dir fsync; no argv/env/secrets). It carries the systemd `InvocationID`, `boot_id`, and daemon generation, so cleanup can run from the record alone — from the terminal path (independent of the in-memory client) or from startup reconciliation after a crash — and refuses to signal a same-name unit that was replaced by a foreign instance, a cross-boot record, or a corrupt record. A per-owner lock plus a generation fence keep launch, terminal reap, retry, and reconciliation from racing. One supported retry entry (`nerve codex reap-descendants ` + a service method) re-drives a `pending_retry` reap idempotently. + +## Scope / safety + +- **Off by default.** `codex.lifecycle.mode`: `disabled` (inert; unchanged behaviour) · `observe` (unenforced — records intent, never claims a cleanup it didn't perform) · `strict` (contain; **fail before exec** if containment can't be established — no silent downgrade to `killpg`). +- **Codex workflow runs only.** Interactive/cron Codex sessions and Claude runs get no containment. +- **Platform:** requires Linux + a reachable systemd `--user` manager + cgroup v2. Everywhere else it stays inert (disabled/observe) or strict refuses to launch — it never runs a workflow un-contained and never breaks a non-systemd deployment. +- **Not a security boundary:** cgroup inheritance contains *trusted* tool processes that fork/setsid/double-fork; it is not a sandbox against a malicious same-UID process, and no privilege is broadened to make it one. +- No daemon deploy / privilege / linger / systemd service-layout change. + +## Testing + +`tests/test_codex_lifecycle.py` — 26 assertion-bearing cases. The real-process cases launch actual systemd user scopes running real setsid/double-fork escapers (`tests/fixtures/lifecycle_escaper.py`) and the real fake app-server through the actual launch wrapper: containment + foreign-run survival, nested-cgroup recursive `populated`, TERM-ignoring → SIGKILL escalation, fork-during-teardown, crash/fresh-process reconcile, replaced-unit/cross-boot/corrupt/fsync-fail/manager-down identity refusals (no foreign signal, no false completion), concurrent-reap serialisation, real durable retry, and disabled/observe/strict-unavailable behaviour. No paid model or network; every scope/process is torn down by the suite. + +Existing suites unaffected: `test_codex_appserver.py` + `test_workflow_runs.py` (92), `-k codex` (182). + +## Deferred (not in this PR) + +Subreaper/pidfd fallback for non-systemd hosts, chat-session extension, and the `ultracode.py:_run` setup-CLI path. From 4a62e64669c8680a59b99ddc70deb2bcee65d01e Mon Sep 17 00:00:00 2001 From: serxa Date: Fri, 18 Sep 2026 17:42:32 +0000 Subject: [PATCH 3/6] drop scratch tmp/pr-body.md from the branch Co-Authored-By: Claude Opus 4.8 --- tmp/pr-body.md | 35 ----------------------------------- 1 file changed, 35 deletions(-) delete mode 100644 tmp/pr-body.md diff --git a/tmp/pr-body.md b/tmp/pr-body.md deleted file mode 100644 index 9be31e839..000000000 --- a/tmp/pr-body.md +++ /dev/null @@ -1,35 +0,0 @@ -## Problem - -When a Codex **workflow** run ends — completed, cancelled, budget-stopped, or the daemon crashed — some of its descendant processes can survive as orphans. Codex wraps each sandboxed command in `codex-linux-sandbox`, which `setsid`s into its own session/process group (and may double-fork). `CodexAppServerClient._signal_process_tree` signals a single process group (`os.killpg(proc.pid, sig)`), so those escaped descendants are never reached; the workflow teardown also assumes the in-memory client still covers every worker. The result is `codex-linux-sandbox` trees (and the `python`/`bash`/`pytest` they were running) left parented to PID 1 after the run is durably `done`. - -## Approach - -Opt-in, **workflow-only** containment. In `strict` mode a workflow run's `codex app-server` is launched **inside a dedicated delegated systemd user scope**, created *before* the command execs: - -``` -systemd-run --user --scope --quiet --collect --expand-environment=no \ - -p Delegate=yes -p KillMode=control-group -p SendSIGKILL=yes -p TimeoutStopSec= \ - --description="" --unit= -- -``` - -Everything the run forks inherits that cgroup (setsid/double-fork can't escape it), so a single bounded `systemctl --user stop` (TERM → grace → SIGKILL over the whole control group) reaps the entire tree. Cleanup is then verified — the scope's recursive `cgroup.events` `populated=0`, or the scope cgroup gone after a *successful* stop — never assumed. - -Each run writes a durable, validated record (`//lifecycle.json`, atomic write + file/dir fsync; no argv/env/secrets). It carries the systemd `InvocationID`, `boot_id`, and daemon generation, so cleanup can run from the record alone — from the terminal path (independent of the in-memory client) or from startup reconciliation after a crash — and refuses to signal a same-name unit that was replaced by a foreign instance, a cross-boot record, or a corrupt record. A per-owner lock plus a generation fence keep launch, terminal reap, retry, and reconciliation from racing. One supported retry entry (`nerve codex reap-descendants ` + a service method) re-drives a `pending_retry` reap idempotently. - -## Scope / safety - -- **Off by default.** `codex.lifecycle.mode`: `disabled` (inert; unchanged behaviour) · `observe` (unenforced — records intent, never claims a cleanup it didn't perform) · `strict` (contain; **fail before exec** if containment can't be established — no silent downgrade to `killpg`). -- **Codex workflow runs only.** Interactive/cron Codex sessions and Claude runs get no containment. -- **Platform:** requires Linux + a reachable systemd `--user` manager + cgroup v2. Everywhere else it stays inert (disabled/observe) or strict refuses to launch — it never runs a workflow un-contained and never breaks a non-systemd deployment. -- **Not a security boundary:** cgroup inheritance contains *trusted* tool processes that fork/setsid/double-fork; it is not a sandbox against a malicious same-UID process, and no privilege is broadened to make it one. -- No daemon deploy / privilege / linger / systemd service-layout change. - -## Testing - -`tests/test_codex_lifecycle.py` — 26 assertion-bearing cases. The real-process cases launch actual systemd user scopes running real setsid/double-fork escapers (`tests/fixtures/lifecycle_escaper.py`) and the real fake app-server through the actual launch wrapper: containment + foreign-run survival, nested-cgroup recursive `populated`, TERM-ignoring → SIGKILL escalation, fork-during-teardown, crash/fresh-process reconcile, replaced-unit/cross-boot/corrupt/fsync-fail/manager-down identity refusals (no foreign signal, no false completion), concurrent-reap serialisation, real durable retry, and disabled/observe/strict-unavailable behaviour. No paid model or network; every scope/process is torn down by the suite. - -Existing suites unaffected: `test_codex_appserver.py` + `test_workflow_runs.py` (92), `-k codex` (182). - -## Deferred (not in this PR) - -Subreaper/pidfd fallback for non-systemd hosts, chat-session extension, and the `ultracode.py:_run` setup-CLI path. From cb67087836f6e2136d2ec4692b7d5e7b153b82ec Mon Sep 17 00:00:00 2001 From: serxa Date: Fri, 18 Sep 2026 18:23:32 +0000 Subject: [PATCH 4/6] =?UTF-8?q?codex=20lifecycle:=20address=20review=20?= =?UTF-8?q?=E2=80=94=20fix=203=20blockers,=20cut=20further?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Correctness (found by the Codex review round): - relaunch after a mid-run client recreate reaped the prior scope before overwriting the sole record (engine.py retries a crashed transport, and the second prepare_launch would otherwise orphan attempt 1's escapees). - a failed `systemctl show` is no longer read as "scope gone" → the reap distinguishes query failure (pending_retry) from an explicit not-found, and requires an exact InvocationID/description before signalling. - validate the run id and confine the run dir under runs_dir so a crafted `workflow:` session id can neither traverse out nor claim authority. Simplification: - drop daemon-generation tracking (startup already terminalizes active runs; reconcile reaps every recorded scope idempotently). - drop the flock layer (removes the eager POSIX `fcntl` import) and reuse nerve.utils.fs.atomic_write_text instead of a private writer. - collapse Receipt to outcome+error and the record to unit/boot/invocation/ cgroup; drop dead symbols; startup reconcile no longer gates on mode, so disabling containment can't strand a crashed run's scope. - consolidate tests (add relaunch/traversal/query-failure coverage). Co-Authored-By: Claude Opus 4.8 --- nerve/agent/backends/codex/appserver.py | 8 +- nerve/agent/backends/codex/backend.py | 17 +- nerve/agent/backends/codex/lifecycle.py | 278 +++++++++--------------- nerve/workflows/service.py | 17 +- tests/fixtures/lifecycle_escaper.py | 33 +-- tests/test_codex_lifecycle.py | 227 ++++++++++--------- 6 files changed, 260 insertions(+), 320 deletions(-) diff --git a/nerve/agent/backends/codex/appserver.py b/nerve/agent/backends/codex/appserver.py index 147b55ada..8d23463bf 100644 --- a/nerve/agent/backends/codex/appserver.py +++ b/nerve/agent/backends/codex/appserver.py @@ -188,7 +188,7 @@ async def start(self) -> dict: # returns the scope-wrapped argv; it raises before exec if the host # can't contain the run. containment = self._containment - if containment is not None and containment.enabled: + if containment is not None: try: args, self._lifecycle_record = await asyncio.to_thread( lifecycle.prepare_launch, containment, args, @@ -218,7 +218,7 @@ async def start(self) -> dict: # Record the scope's InvocationID + resolved cgroup now that systemd-run # created it. - if containment is not None and containment.enabled and self._lifecycle_record is not None: + if containment is not None and self._lifecycle_record is not None: try: self._lifecycle_record = await asyncio.to_thread( lifecycle.record_launched, @@ -255,9 +255,9 @@ async def start(self) -> dict: await self.close() raise - # Confirm the app-server is inside its scope cgroup before dispatching. + # Verify the app-server is inside its scope cgroup before returning. if ( - containment is not None and containment.enabled + containment is not None and self._lifecycle_record is not None and self._proc is not None and self._proc.pid is not None ): diff --git a/nerve/agent/backends/codex/backend.py b/nerve/agent/backends/codex/backend.py index cac485957..30d96b2db 100644 --- a/nerve/agent/backends/codex/backend.py +++ b/nerve/agent/backends/codex/backend.py @@ -661,17 +661,24 @@ def _workflow_containment( backend: "CodexBackend", spec: SessionSpec, ) -> lifecycle.WorkflowContainment | None: """cgroup containment for a Codex ``workflow:`` session in strict mode, else - None (interactive/cron sessions and disabled mode launch unchanged).""" + None (interactive/cron sessions and disabled mode launch unchanged). + + The run id is validated and the run dir confined under ``runs_dir`` so a + crafted session id can neither escape the directory nor claim signalling + authority.""" sid = spec.session_id or "" if not sid.startswith(_WORKFLOW_SESSION_PREFIX): return None if backend.codex.lifecycle.mode != lifecycle.MODE_STRICT: return None run_id = sid[len(_WORKFLOW_SESSION_PREFIX):] - run_dir = Path(backend.config.workflows.runs_dir).expanduser() / run_id - return lifecycle.WorkflowContainment( - mode=lifecycle.MODE_STRICT, run_dir=run_dir, run_id=run_id, session_id=sid, - ) + if not lifecycle._RUN_ID_RE.match(run_id): + return None + runs_dir = Path(backend.config.workflows.runs_dir).expanduser().resolve() + run_dir = (runs_dir / run_id).resolve() + if run_dir.parent != runs_dir: + return None + return lifecycle.WorkflowContainment(run_dir=run_dir, run_id=run_id) class CodexClient(AgentClient): diff --git a/nerve/agent/backends/codex/lifecycle.py b/nerve/agent/backends/codex/lifecycle.py index 6611ae66e..2ea939bac 100644 --- a/nerve/agent/backends/codex/lifecycle.py +++ b/nerve/agent/backends/codex/lifecycle.py @@ -1,21 +1,16 @@ """Cgroup containment + reaping for Codex workflow-run descendants. -A Codex workflow run's ``codex-linux-sandbox`` descendants ``setsid`` into their -own process groups, so the app-server teardown's single ``killpg`` cannot reach -them and they survive as orphans. When ``codex.lifecycle.mode`` is ``strict``, -a workflow run's app-server is launched inside a delegated systemd user scope so -the whole descendant tree lives in one cgroup and a single ``systemctl --user -stop`` reaps it; a durable per-run record lets the reap run at terminal or from -startup reconciliation after a crash. Default ``disabled`` leaves launch -unchanged. Requires Linux + a reachable systemd ``--user`` manager + cgroup v2; -strict fails before exec where that is absent (it never launches un-contained). -This contains trusted tool processes; it is not a security sandbox. +In ``strict`` mode a workflow run's ``codex app-server`` is launched inside a +delegated systemd user scope, so its whole descendant tree — including +``codex-linux-sandbox`` children that ``setsid`` into their own process groups — +stays in one cgroup and a single ``systemctl --user stop`` reaps it. A durable +per-run record lets the reap run at terminal cleanup or from startup +reconciliation after a daemon crash. Requires Linux + a reachable systemd +``--user`` manager + cgroup v2; strict fails before exec where that is absent. """ from __future__ import annotations -import contextlib -import fcntl import json import logging import os @@ -26,30 +21,29 @@ import uuid from dataclasses import asdict, dataclass from pathlib import Path -from typing import Callable, Iterable +from typing import Iterable + +from nerve.utils.fs import atomic_write_text logger = logging.getLogger(__name__) -MODE_DISABLED = "disabled" MODE_STRICT = "strict" -VALID_MODES = (MODE_DISABLED, MODE_STRICT) RECORD_NAME = "lifecycle.json" -LOCK_NAME = "lifecycle.lock" -RECONCILE_LOCK_NAME = ".lifecycle-reconcile.lock" SCOPE_DESCRIPTION = "Nerve Codex workflow run containment" -# systemd-run enforces TERM, then SIGKILL after this grace, over the cgroup. +# systemd-run sends TERM, then SIGKILL after this grace, over the whole cgroup. TERM_GRACE_SECONDS = 5 STOP_TIMEOUT_SECONDS = 20 _UNIT_RE = re.compile(r"^nerve-wf-[A-Za-z0-9:_.-]+\.scope$") +_RUN_ID_RE = re.compile(r"^wfr-[A-Za-z0-9_]+$") _CGROUP_ROOT = "/sys/fs/cgroup" _USER_SLICE_PREFIX = "/user.slice/" -# Receipt outcomes. `complete` and `refused` are terminal (never retried); -# `pending_retry` is re-attempted by the next startup reconciliation. -_TERMINAL_OUTCOMES = frozenset({"complete", "refused", "no_scope"}) +# `complete` and `refused` are final; `pending_retry` is re-attempted by the +# next startup reconciliation. +_TERMINAL_OUTCOMES = frozenset({"complete", "refused"}) class LifecycleError(Exception): @@ -60,7 +54,7 @@ class ContainmentUnavailable(LifecycleError): """Strict mode requested but the host cannot create a scope (raised before exec).""" -# -- capabilities + instance identity --------------------------------------- # +# -- capabilities ------------------------------------------------------------ # @dataclass(frozen=True) @@ -86,10 +80,8 @@ def detect_capabilities(*, refresh: bool = False) -> Capabilities: reasons.append("cgroup v2 not mounted") else: try: - proc = subprocess.run( - ["systemctl", "--user", "show-environment"], - capture_output=True, timeout=5, check=False, - ) + proc = subprocess.run(["systemctl", "--user", "show-environment"], + capture_output=True, timeout=5, check=False) if proc.returncode != 0: reasons.append("systemd --user manager not reachable") except (OSError, subprocess.SubprocessError): @@ -105,33 +97,12 @@ def _boot_id() -> str: return "" -def _proc_start_ticks(pid: int) -> int: - """Field 22 of ``/proc//stat`` (read after the final ``)`` — ``comm`` - may contain spaces/parens).""" - try: - raw = Path(f"/proc/{pid}/stat").read_bytes() - except OSError: - return 0 - try: - return int(raw.rsplit(b")", 1)[1].split()[19]) - except (IndexError, ValueError): - return 0 - - -def current_generation() -> str: - """Identity of this daemon incarnation; changes on restart and reboot.""" - pid = os.getpid() - return f"{_boot_id()}:{pid}:{_proc_start_ticks(pid)}" - - # -- durable record ---------------------------------------------------------- # @dataclass class Receipt: - outcome: str - signalled: bool = False - verified_empty: bool = False + outcome: str # complete | pending_retry | refused | no_scope error: str = "" ts: str = "" @@ -145,14 +116,10 @@ def is_terminal(self) -> bool: @dataclass class LifecycleRecord: run_id: str - session_id: str unit: str boot_id: str - generation: str invocation_id: str = "" control_group: str = "" - created_at: str = "" - updated_at: str = "" receipt: dict | None = None def to_dict(self) -> dict: @@ -164,13 +131,10 @@ def from_dict(cls, d: dict) -> "LifecycleRecord": raise LifecycleError("record is not an object") try: rec = cls( - run_id=str(d["run_id"]), session_id=str(d["session_id"]), - unit=str(d["unit"]), boot_id=str(d.get("boot_id", "")), - generation=str(d.get("generation", "")), + run_id=str(d["run_id"]), unit=str(d["unit"]), + boot_id=str(d.get("boot_id", "")), invocation_id=str(d.get("invocation_id", "")), control_group=str(d.get("control_group", "")), - created_at=str(d.get("created_at", "")), - updated_at=str(d.get("updated_at", "")), receipt=d.get("receipt") if isinstance(d.get("receipt"), dict) else None, ) except (KeyError, TypeError) as e: @@ -215,53 +179,14 @@ def read_record(run_dir: Path) -> LifecycleRecord | None: def write_record(run_dir: Path, record: LifecycleRecord) -> None: record.validate() - record.updated_at = _now() - if not record.created_at: - record.created_at = record.updated_at - _atomic_write_json(record_path(run_dir), record.to_dict()) - - -def _atomic_write_json(path: Path, obj: dict) -> None: - """Write JSON via a temp file, then rename; fsync the file and its directory - so the record survives a crash. Raises OSError on any failure.""" - path.parent.mkdir(parents=True, exist_ok=True) - tmp = path.with_name(f".{path.name}.{os.getpid()}.{uuid.uuid4().hex[:8]}.tmp") - fd = os.open(tmp, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) - try: - os.write(fd, json.dumps(obj, indent=2, sort_keys=True).encode("utf-8")) - os.fsync(fd) - finally: - os.close(fd) - os.replace(tmp, path) - dir_fd = os.open(path.parent, os.O_RDONLY) - try: - os.fsync(dir_fd) - finally: - os.close(dir_fd) - - -def _now() -> str: - return time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()) - - -@contextlib.contextmanager -def _flock(lock_path: Path): - lock_path.parent.mkdir(parents=True, exist_ok=True) - fd = os.open(lock_path, os.O_WRONLY | os.O_CREAT, 0o600) - try: - fcntl.flock(fd, fcntl.LOCK_EX) - yield - finally: - with contextlib.suppress(OSError): - fcntl.flock(fd, fcntl.LOCK_UN) - os.close(fd) + atomic_write_text(record_path(run_dir), + json.dumps(record.to_dict(), indent=2, sort_keys=True), mode=0o600) # -- unit naming + launch wrapper -------------------------------------------- # def scope_unit_name(run_id: str, nonce: str) -> str: - """Unique, charset-safe scope unit name embedding the attempt nonce.""" safe_run = re.sub(r"[^A-Za-z0-9_.-]", "_", run_id)[:80] safe_nonce = re.sub(r"[^A-Za-z0-9]", "", nonce)[:16] return f"nerve-wf-{safe_run}-{safe_nonce}.scope" @@ -269,8 +194,8 @@ def scope_unit_name(run_id: str, nonce: str) -> str: def build_scope_argv(unit: str, inner_argv: list[str]) -> list[str]: """Wrap ``inner_argv`` to run inside a delegated transient user scope. - systemd places the process in the scope's cgroup before it execs, so the - whole fork/setsid/double-fork subtree stays contained.""" + systemd puts the process in the scope's cgroup before it execs, so the whole + fork/setsid subtree stays contained.""" if not _UNIT_RE.match(unit): raise LifecycleError(f"unsafe unit {unit!r}") return [ @@ -289,15 +214,20 @@ def build_scope_argv(unit: str, inner_argv: list[str]) -> list[str]: # -- systemd / cgroup queries ------------------------------------------------ # -def _systemctl_show(unit: str, props: Iterable[str]) -> dict[str, str]: +def _unit_identity(unit: str) -> dict[str, str] | None: + """``systemctl --user show`` the unit's identity props, or None if the query + itself failed (which must never be read as 'the scope is gone').""" try: proc = subprocess.run( - ["systemctl", "--user", "show", unit, "--property", ",".join(props)], + ["systemctl", "--user", "show", unit, + "--property", "LoadState,InvocationID,ControlGroup,Description"], capture_output=True, text=True, timeout=10, check=False, ) except (OSError, subprocess.SubprocessError) as e: logger.debug("systemctl show %s failed: %s", unit, e) - return {} + return None + if proc.returncode != 0: + return None out: dict[str, str] = {} for line in proc.stdout.splitlines(): key, sep, value = line.partition("=") @@ -306,16 +236,12 @@ def _systemctl_show(unit: str, props: Iterable[str]) -> dict[str, str]: return out -def _unit_identity(unit: str) -> dict[str, str]: - return _systemctl_show(unit, ("LoadState", "InvocationID", "ControlGroup", "Description")) - - def _await_unit_identity(unit: str, *, timeout: float = 4.0) -> tuple[str, str]: """Poll until the scope reports its InvocationID + ControlGroup.""" deadline = time.monotonic() + timeout inv = cg = "" while time.monotonic() < deadline: - ident = _unit_identity(unit) + ident = _unit_identity(unit) or {} inv, cg = ident.get("InvocationID", ""), ident.get("ControlGroup", "") if inv and cg: return inv, cg @@ -326,7 +252,7 @@ def _await_unit_identity(unit: str, *, timeout: float = 4.0) -> tuple[str, str]: def _cgroup_gone_or_empty(control_group: str) -> bool | None: """True if the scope cgroup is gone or recursively unpopulated, False if it still holds a process, None if it can't be read. ``cgroup.events`` - ``populated`` is recursive per the kernel, so it covers delegated children.""" + ``populated`` is recursive, so it covers delegated child cgroups.""" if not control_group: return None path = Path(_CGROUP_ROOT + control_group) / "cgroup.events" @@ -343,13 +269,10 @@ def _cgroup_gone_or_empty(control_group: str) -> bool | None: def _scope_stop(unit: str) -> tuple[bool, str]: - """``systemctl --user stop`` the scope (TERM → grace → SIGKILL over the - control group). Returns (ok, error).""" try: - proc = subprocess.run( - ["systemctl", "--user", "stop", unit], - capture_output=True, text=True, timeout=STOP_TIMEOUT_SECONDS, check=False, - ) + proc = subprocess.run(["systemctl", "--user", "stop", unit], + capture_output=True, text=True, + timeout=STOP_TIMEOUT_SECONDS, check=False) except subprocess.TimeoutExpired: return False, "stop timed out" except (OSError, subprocess.SubprocessError) as e: @@ -385,31 +308,30 @@ def verify_membership(control_group: str, pid: int) -> bool: @dataclass(frozen=True) class WorkflowContainment: - mode: str run_dir: Path run_id: str - session_id: str - - @property - def enabled(self) -> bool: - return self.mode == MODE_STRICT def prepare_launch( containment: WorkflowContainment, inner_argv: list[str], ) -> tuple[list[str], LifecycleRecord]: - """Persist the owner record, then return the scope-wrapped argv. Raises - before anything execs if the host can't contain or the record can't be - written, so a strict run never launches un-contained.""" + """Record the owner, then return the scope-wrapped argv. Raises before + anything execs if the host can't contain the run, if the record can't be + written, or if a previous attempt's scope is not confirmed reaped — the + engine can recreate a crashed client for the same run, and a fresh launch + must not overwrite (and orphan) a still-live prior scope.""" caps = detect_capabilities() if not caps.ok: raise ContainmentUnavailable(f"strict cgroup containment unavailable: {caps.reason}") - nonce = uuid.uuid4().hex - unit = scope_unit_name(containment.run_id, nonce) - record = LifecycleRecord( - run_id=containment.run_id, session_id=containment.session_id, unit=unit, - boot_id=_boot_id(), generation=current_generation(), - ) + if record_path(containment.run_dir).exists(): + prior = reap(containment.run_dir) + if not prior.is_complete(): + raise LifecycleError( + f"previous scope not confirmed reaped ({prior.outcome}: {prior.error}); " + "refusing to relaunch" + ) + unit = scope_unit_name(containment.run_id, uuid.uuid4().hex) + record = LifecycleRecord(run_id=containment.run_id, unit=unit, boot_id=_boot_id()) write_record(containment.run_dir, record) return build_scope_argv(unit, inner_argv), record @@ -429,75 +351,81 @@ def record_launched(run_dir: Path, record: LifecycleRecord) -> LifecycleRecord: def reap(run_dir: Path) -> Receipt: - """Reap a run's contained descendants from its durable record. Idempotent - (a terminal receipt is returned unchanged); serialised per owner.""" + """Reap a run's contained descendants from its durable record. Idempotent: + a record already carrying a terminal receipt is returned unchanged.""" run_dir = Path(run_dir) if not record_path(run_dir).exists(): - return Receipt(outcome="no_scope", ts=_now()) - with _flock(run_dir / LOCK_NAME): - try: - record = read_record(run_dir) - except LifecycleError as e: - logger.warning("refusing to reap from corrupt record in %s: %s", run_dir, e) - return Receipt(outcome="pending_retry", error="corrupt record", ts=_now()) - if record is None: - return Receipt(outcome="no_scope", ts=_now()) - prev = _receipt_from(record) - if prev is not None and prev.is_terminal(): - return prev - receipt = _do_reap(record) - record.receipt = asdict(receipt) + return Receipt(outcome="no_scope") + try: + record = read_record(run_dir) + except LifecycleError as e: + logger.warning("refusing to reap from corrupt record in %s: %s", run_dir, e) + return Receipt(outcome="pending_retry", error="corrupt record") + if record is None: + return Receipt(outcome="no_scope") + prev = _receipt_from(record) + if prev is not None and prev.is_terminal(): + return prev + receipt = _do_reap(record) + record.receipt = asdict(receipt) + try: write_record(run_dir, record) - return receipt + except OSError as e: + # A stop we can't durably record cannot be reported complete. + return Receipt(outcome="pending_retry", error=f"receipt not persisted: {e}") + return receipt def _do_reap(record: LifecycleRecord) -> Receipt: # A reboot leaves no process alive, and the unit name may now be foreign — # resolve without signalling. if record.boot_id and _boot_id() and record.boot_id != _boot_id(): - return Receipt(outcome="complete", verified_empty=True, ts=_now()) + return Receipt(outcome="complete") ident = _unit_identity(record.unit) - if ident.get("LoadState", "") in ("", "not-found"): - # Same boot, unit gone: systemd removes a scope's cgroup only once it is - # empty, so the tree is gone. No signal was sent. - return Receipt(outcome="complete", verified_empty=True, ts=_now()) - + if ident is None: + return Receipt(outcome="pending_retry", error="unit query failed") + if ident.get("LoadState", "") == "not-found": + # A transient scope's cgroup is removed only once empty, so a + # not-found unit means the tree is gone. No signal sent. + return Receipt(outcome="complete") + + # Verify identity before signalling: an exact InvocationID when recorded, or + # the exact scope description for a pre-registration record. live_inv = ident.get("InvocationID", "") if record.invocation_id: - if live_inv and live_inv != record.invocation_id: - return Receipt(outcome="refused", error="unit replaced by a foreign instance", ts=_now()) - elif ident.get("Description", "") not in ("", SCOPE_DESCRIPTION): - return Receipt(outcome="refused", error="unit description mismatch", ts=_now()) + if not live_inv: + return Receipt(outcome="pending_retry", error="unit InvocationID unavailable") + if live_inv != record.invocation_id: + return Receipt(outcome="refused", error="unit replaced by a foreign instance") + elif ident.get("Description", "") != SCOPE_DESCRIPTION: + return Receipt(outcome="refused", error="unit description mismatch") ok, err = _scope_stop(record.unit) if not ok: - return Receipt(outcome="pending_retry", signalled=True, error=err, ts=_now()) + return Receipt(outcome="pending_retry", error=err) if _cgroup_gone_or_empty(record.control_group) is True: - return Receipt(outcome="complete", signalled=True, verified_empty=True, ts=_now()) - return Receipt(outcome="pending_retry", signalled=True, - error="scope still populated after stop", ts=_now()) + return Receipt(outcome="complete") + return Receipt(outcome="pending_retry", error="scope still populated after stop") def reconcile(runs_dir: Path) -> list[tuple[str, Receipt]]: - """Reap scopes left by a previous daemon incarnation. A record whose - generation differs from the current one was owned by a dead daemon, so its - run cannot be active; a record of the current generation is left alone.""" + """Reap every run's scope from its record. Called at startup, after active + runs have been marked failed, so no record belongs to a live run; reap is + idempotent, so already-complete records are no-ops.""" root = Path(runs_dir) if not root.is_dir(): return [] - gen = current_generation() results: list[tuple[str, Receipt]] = [] - with _flock(root / RECONCILE_LOCK_NAME): - for child in sorted(p for p in root.iterdir() if p.is_dir()): - try: - record = read_record(child) - except LifecycleError: - logger.warning("skipping corrupt record in %s", child.name) - continue - if record is None or record.generation == gen: - continue - results.append((child.name, reap(child))) + for child in sorted(p for p in root.iterdir() if p.is_dir()): + try: + record = read_record(child) + except LifecycleError: + logger.warning("skipping corrupt record in %s", child.name) + continue + if record is None: + continue + results.append((child.name, reap(child))) return results diff --git a/nerve/workflows/service.py b/nerve/workflows/service.py index f129e132c..d59f71540 100644 --- a/nerve/workflows/service.py +++ b/nerve/workflows/service.py @@ -738,11 +738,9 @@ def _is_quiet(run: dict | None) -> bool: # ------------------------------------------------------------------ # async def _reconcile_codex_lifecycle(self) -> None: - """Reap cgroup scopes left by a previous daemon incarnation. Records of - the current generation belong to live runs and are skipped inside - ``reconcile``. Best effort — never blocks the monitor loop.""" - if self.config.codex.lifecycle.mode != codex_lifecycle.MODE_STRICT: - return + """Reap cgroup scopes recorded by a previous run. Runs regardless of the + current mode so disabling containment does not strand a crashed run's + scope. Best effort — never blocks the monitor loop.""" try: results = await asyncio.to_thread( codex_lifecycle.reconcile, self.runs_dir(), @@ -760,9 +758,16 @@ async def _reconcile_codex_lifecycle(self) -> None: async def _reap_codex_lifecycle(self, run_id: str) -> None: """Reap one terminal run's contained descendants from its record.""" try: - await asyncio.to_thread(codex_lifecycle.reap, self.runs_dir() / run_id) + receipt = await asyncio.to_thread(codex_lifecycle.reap, self.runs_dir() / run_id) except Exception: # noqa: BLE001 logger.exception("codex lifecycle reap failed for %s", run_id) + return + if receipt.outcome not in ("complete", "no_scope"): + logger.warning( + "codex lifecycle reap for %s did not complete (%s: %s); " + "the next startup reconciliation will retry", + run_id, receipt.outcome, receipt.error, + ) # ------------------------------------------------------------------ # # Budget monitor # diff --git a/tests/fixtures/lifecycle_escaper.py b/tests/fixtures/lifecycle_escaper.py index b256e3806..cb5b75355 100755 --- a/tests/fixtures/lifecycle_escaper.py +++ b/tests/fixtures/lifecycle_escaper.py @@ -1,23 +1,14 @@ #!/usr/bin/env python3 -"""Real escaping-descendant fixture for Codex lifecycle tests (no model, no net). - -Mirrors the incident shape: a process ``setsid``\\ s into its own session/group -and double-forks a grandchild that lingers, so ``killpg`` on the launcher's -group cannot reach it — only a cgroup-scoped reap can. Every process carries a -unique ``ESCAPER_TOKEN`` in its environment (and leaves carry it in argv) so the -test's belt-and-braces cleanup only ever signals its own fixtures, by pidfd, -never by name/pgrep/killall. - -Commands (each is meant to run as the main command of a systemd ``--scope``): - - spawn-escapee setsid + double-fork a sleeper grandchild - spawn-escapee-nested …then migrate the grandchild into a child - cgroup it mkdirs under the delegated scope - (root cgroup.procs empties; recursive - cgroup.events populated stays 1) - spawn-escapee-igterm grandchild ignores SIGTERM (forces KILL) - sleeper sleep (a leaf) - sweep identity-checked pidfd cleanup of the token +"""Escaping-descendant fixture for Codex lifecycle tests (no model, no network). + +Run as the main command of a systemd ``--scope``, it setsid+double-forks a +sleeper grandchild that outlives the scope's main process — the escape a +single ``killpg`` cannot reach. Every process carries a unique ``ESCAPER_TOKEN`` +so cleanup only ever signals this fixture's own processes, by pidfd. + +Commands: ``spawn-escapee `` (plain), ``spawn-escapee-nested `` +(migrate the grandchild into a child cgroup so the scope root empties but +recursive ``cgroup.events populated`` stays 1), ``sleeper``, ``sweep ``. """ from __future__ import annotations @@ -148,8 +139,6 @@ def _double_fork_grandchild(token: str, outdir: str, variant: str) -> None: fh.write(str(os.getpid())) except OSError: pass # non-fatal: fall back to plain containment - elif variant == "igterm": - signal.signal(signal.SIGTERM, signal.SIG_IGN) _write(outdir, "grandchild.pid", str(os.getpid())) _become_sleeper(token, "grandchild", outdir) @@ -187,8 +176,6 @@ def main() -> int: return _spawn("plain", sys.argv[2:]) if cmd == "spawn-escapee-nested": return _spawn("nested", sys.argv[2:]) - if cmd == "spawn-escapee-igterm": - return _spawn("igterm", sys.argv[2:]) if cmd == "sweep": token = os.environ.get("ESCAPER_TOKEN") or (sys.argv[2] if len(sys.argv) > 2 else "") print(sweep(token)) diff --git a/tests/test_codex_lifecycle.py b/tests/test_codex_lifecycle.py index 741ad4ad2..2bcdeadf7 100644 --- a/tests/test_codex_lifecycle.py +++ b/tests/test_codex_lifecycle.py @@ -1,10 +1,9 @@ -"""Tests for Codex workflow-run cgroup containment (nerve/agent/backends/codex/lifecycle.py). +"""Tests for Codex workflow-run cgroup containment (codex/lifecycle.py). -The containment/reap cases use real systemd user scopes running real -setsid/double-fork escapers (tests/fixtures/lifecycle_escaper.py) and the real -fake app-server through the actual launch wrapper — no model, no network. Every -scope/process is torn down by the ``scopes`` fixture. Scope cases skip on a host -without a reachable systemd ``--user`` manager. +Containment/reap cases use real systemd user scopes running real setsid escapers +(tests/fixtures/lifecycle_escaper.py) and the real fake app-server — no model, +no network. Every scope/process is torn down by the ``scopes`` fixture. Scope +cases skip where a systemd ``--user`` manager isn't reachable. """ from __future__ import annotations @@ -18,7 +17,6 @@ import pytest from nerve.agent.backends import BackendDeps, SessionSpec -from nerve.agent.backends.base import TransportDiedError from nerve.agent.backends.codex import CodexBackend, lifecycle from nerve.agent.backends.codex.appserver import CodexAppServerClient from nerve.agent.backends.codex.backend import _workflow_containment @@ -59,12 +57,10 @@ def scopes(): except Exception: # noqa: BLE001 pass for unit in reg["units"]: - subprocess.run(["systemctl", "--user", "stop", unit], - capture_output=True, timeout=15) + subprocess.run(["systemctl", "--user", "stop", unit], capture_output=True, timeout=15) for tok in reg["tokens"]: subprocess.run([sys.executable, ESCAPER, "sweep", tok], - env={**os.environ, "ESCAPER_TOKEN": tok}, - capture_output=True, timeout=15) + env={**os.environ, "ESCAPER_TOKEN": tok}, capture_output=True, timeout=15) def _launch_escaper(scopes, run_dir: Path, run_id: str, *, variant: str = "", @@ -81,10 +77,7 @@ def _launch_escaper(scopes, run_dir: Path, run_id: str, *, variant: str = "", subprocess.run(argv, env={**os.environ, "ESCAPER_TOKEN": token}, capture_output=True, timeout=30, check=True) gc_pid = int((run_dir / "grandchild.pid").read_text()) - rec = lifecycle.LifecycleRecord( - run_id=run_id, session_id=f"workflow:{run_id}", unit=unit, - boot_id=lifecycle._boot_id(), generation=lifecycle.current_generation(), - ) + rec = lifecycle.LifecycleRecord(run_id=run_id, unit=unit, boot_id=lifecycle._boot_id()) lifecycle.write_record(run_dir, rec) if invocation_id is not None: _inv, cg = lifecycle._await_unit_identity(unit) @@ -97,9 +90,9 @@ def _launch_escaper(scopes, run_dir: Path, run_id: str, *, variant: str = "", def _deps(cfg: NerveConfig) -> BackendDeps: return BackendDeps( - config=lambda: cfg, db=None, registry=None, - tool_ctx_factory=lambda sid: None, external_mcp_servers=lambda: [], - gateway_port=lambda: 8900, mint_session_token=lambda sid: f"tok-{sid}", + config=lambda: cfg, db=None, registry=None, tool_ctx_factory=lambda sid: None, + external_mcp_servers=lambda: [], gateway_port=lambda: 8900, + mint_session_token=lambda sid: f"tok-{sid}", ) @@ -136,12 +129,9 @@ def _wf_config(tmp_path: Path, *, mode: str = "strict") -> NerveConfig: async def test_strict_launch_contains_and_roundtrips(tmp_path, scopes): run_dir = tmp_path / "workflow-runs" / "wfr-l1" scopes["run_dirs"].add(run_dir) - containment = lifecycle.WorkflowContainment( - mode="strict", run_dir=run_dir, run_id="wfr-l1", session_id="workflow:wfr-l1", - ) - literal = "keep-${NERVE_SENTINEL_DO_NOT_EXPAND}" cwd = tmp_path / "ws" cwd.mkdir(parents=True, exist_ok=True) + containment = lifecycle.WorkflowContainment(run_dir=run_dir, run_id="wfr-l1") async def _handler(method, params): return {} @@ -149,35 +139,25 @@ async def _handler(method, params): client = CodexAppServerClient( bin_path=FAKE, cwd=str(cwd), env={**os.environ, "CODEX_HOME": str(tmp_path / "home"), "FAKE_CODEX_MODE": "basic"}, - server_request_handler=_handler, config_overrides=[f"sentinel={literal}"], - containment=containment, + server_request_handler=_handler, config_overrides=[], containment=containment, ) resp = await client.start() try: - assert resp.get("userAgent") # clean JSON stdout - assert any(literal in v for v in resp["_fake"]["configOverrides"]) # literal argv preserved - assert resp["_fake"]["env"]["CODEX_HOME"] == str(tmp_path / "home") # env forwarded + assert resp.get("userAgent") # clean JSON through the wrapper rec = lifecycle.read_record(run_dir) - assert rec.invocation_id and rec.control_group - assert lifecycle._cgroup_gone_or_empty(rec.control_group) is False # populated - procs = Path("/sys/fs/cgroup" + rec.control_group + "/cgroup.procs").read_text().split() - assert procs and any( - os.readlink(f"/proc/{p}/cwd") == str(cwd) - for p in map(int, procs) if Path(f"/proc/{p}").exists() - ) + assert rec.invocation_id and rec.control_group # scope identity recorded + assert lifecycle._cgroup_gone_or_empty(rec.control_group) is False # contained + populated finally: await client.close() -def test_disabled_and_non_workflow_build_no_containment(tmp_path): - cfg = _wf_config(tmp_path, mode="strict") - backend = CodexBackend(_deps(cfg)) - # interactive/cron session: never contained, even in strict mode - assert _workflow_containment(backend, _spec(cfg, session_id="s1", source="web")) is None - # disabled mode: workflow session not contained - cfg2 = _wf_config(tmp_path, mode="disabled") - backend2 = CodexBackend(_deps(cfg2)) - assert _workflow_containment(backend2, _spec(cfg2, session_id="workflow:wfr-x", source="workflow")) is None +def test_scope_argv_has_bounded_kill_directives(): + argv = lifecycle.build_scope_argv("nerve-wf-x-n.scope", ["/bin/true"]) + assert "Delegate=yes" in argv + assert "KillMode=control-group" in argv + assert "SendSIGKILL=yes" in argv + assert any(a.startswith("TimeoutStopSec=") for a in argv) + assert "--expand-environment=no" in argv def test_strict_unavailable_fails_before_exec(tmp_path, monkeypatch): @@ -185,35 +165,54 @@ def test_strict_unavailable_fails_before_exec(tmp_path, monkeypatch): run_dir.mkdir() monkeypatch.setattr(lifecycle, "detect_capabilities", lambda **k: lifecycle.Capabilities(ok=False, reason="no systemd-run")) - c = lifecycle.WorkflowContainment(mode="strict", run_dir=run_dir, run_id="wfr-u", - session_id="workflow:wfr-u") + c = lifecycle.WorkflowContainment(run_dir=run_dir, run_id="wfr-u") with pytest.raises(lifecycle.ContainmentUnavailable): lifecycle.prepare_launch(c, ["/usr/bin/codex", "app-server"]) - assert not (run_dir / lifecycle.RECORD_NAME).exists() # nothing launched/recorded + assert not lifecycle.record_path(run_dir).exists() # nothing launched/recorded + + +def test_containment_gating_rejects_disabled_nonworkflow_and_traversal(tmp_path): + cfg = _wf_config(tmp_path, mode="strict") + backend = CodexBackend(_deps(cfg)) + # real workflow session in strict mode → contained + c = _workflow_containment(backend, _spec(cfg, session_id="workflow:wfr-abc123", source="workflow")) + assert c is not None and c.run_id == "wfr-abc123" + # interactive session → never contained, even in strict mode + assert _workflow_containment(backend, _spec(cfg, session_id="s1", source="web")) is None + # path-traversal / spoofed run id → refused (no containment, no escape) + assert _workflow_containment(backend, _spec(cfg, session_id="workflow:../../etc", source="workflow")) is None + assert _workflow_containment(backend, _spec(cfg, session_id="workflow:wfr-a/b", source="workflow")) is None + # disabled mode → not contained + cfg2 = _wf_config(tmp_path, mode="disabled") + b2 = CodexBackend(_deps(cfg2)) + assert _workflow_containment(b2, _spec(cfg2, session_id="workflow:wfr-x", source="workflow")) is None # -- reap -------------------------------------------------------------------- # @requires_scope -def test_reap_kills_contained_and_spares_foreign(tmp_path, scopes): +def test_reap_kills_contained_spares_foreign_and_is_idempotent(tmp_path, scopes, monkeypatch): target, gc_t = _launch_escaper(scopes, tmp_path / "t", "wfr-target") foreign, gc_f = _launch_escaper(scopes, tmp_path / "f", "wfr-foreign") assert _alive(gc_t) and _alive(gc_f) assert lifecycle.proc_cgroup(gc_t) == target.control_group r = lifecycle.reap(tmp_path / "t") - assert r.outcome == "complete" and r.signalled and r.verified_empty - assert not _alive(gc_t) # target reaped - assert _alive(gc_f) # foreign run untouched - assert lifecycle.read_record(tmp_path / "t").receipt["outcome"] == "complete" + assert r.outcome == "complete" + assert not _alive(gc_t) # target reaped + assert _alive(gc_f) # foreign run untouched + + # Idempotent: a second reap returns the stored terminal receipt, no re-stop. + monkeypatch.setattr(lifecycle, "_scope_stop", _fail_if_called) + assert lifecycle.reap(tmp_path / "t").outcome == "complete" @requires_scope def test_reap_nested_cgroup_uses_recursive_populated(tmp_path, scopes): rec, gc = _launch_escaper(scopes, tmp_path / "n", "wfr-nested", variant="nested") - # grandchild migrated into a child cgroup: root cgroup.procs is empty, but - # recursive cgroup.events populated is 1 (what reap must check). + # grandchild migrated into a child cgroup: scope root cgroup.procs is empty, + # but recursive cgroup.events populated is 1 (what reap must check). assert Path("/sys/fs/cgroup" + rec.control_group + "/cgroup.procs").read_text().split() == [] assert lifecycle._cgroup_gone_or_empty(rec.control_group) is False assert lifecycle.proc_cgroup(gc) == rec.control_group + "/nested" @@ -221,23 +220,6 @@ def test_reap_nested_cgroup_uses_recursive_populated(tmp_path, scopes): assert not _alive(gc) -@requires_scope -def test_reap_escalates_term_ignoring_descendant_to_kill(tmp_path, scopes): - rec, gc = _launch_escaper(scopes, tmp_path / "ig", "wfr-igterm", variant="igterm") - assert _alive(gc) - r = lifecycle.reap(tmp_path / "ig") - assert r.outcome == "complete" and r.verified_empty # SIGKILL escalation - assert not _alive(gc) - - -@requires_scope -def test_reap_is_idempotent(tmp_path, scopes, monkeypatch): - rec, gc = _launch_escaper(scopes, tmp_path / "id", "wfr-idem") - assert lifecycle.reap(tmp_path / "id").outcome == "complete" - monkeypatch.setattr(lifecycle, "_scope_stop", _fail_if_called) # must not re-stop - assert lifecycle.reap(tmp_path / "id").outcome == "complete" - - @requires_scope def test_reap_refuses_replaced_unit(tmp_path, scopes): # Record a wrong InvocationID: the live unit is a foreign instance of the @@ -245,68 +227,99 @@ def test_reap_refuses_replaced_unit(tmp_path, scopes): rec, gc = _launch_escaper(scopes, tmp_path / "r", "wfr-rep", invocation_id="00000000000000000000000000000000") r = lifecycle.reap(tmp_path / "r") - assert r.outcome == "refused" and not r.signalled and not r.is_complete() + assert r.outcome == "refused" and not r.is_complete() assert _alive(gc) +@requires_scope +def test_relaunch_reaps_prior_scope_before_overwriting_record(tmp_path, scopes): + # A crashed client is recreated for the same run: prepare_launch must reap + # the prior scope before overwriting the sole record, or attempt 1's escapee + # would become unreachable. + run_dir = tmp_path / "workflow-runs" / "wfr-relaunch" + scopes["run_dirs"].add(run_dir) + rec1, gc1 = _launch_escaper(scopes, run_dir, "wfr-relaunch") + assert _alive(gc1) + c = lifecycle.WorkflowContainment(run_dir=run_dir, run_id="wfr-relaunch") + _argv, rec2 = lifecycle.prepare_launch(c, ["/bin/true"]) + assert not _alive(gc1) # prior scope reaped + assert rec2.unit != rec1.unit # new attempt gets a new unit + assert lifecycle.read_record(run_dir).unit == rec2.unit + + +def test_relaunch_refused_when_prior_scope_not_confirmed(tmp_path, monkeypatch): + run_dir = tmp_path / "workflow-runs" / "wfr-stuck" + run_dir.mkdir(parents=True) + lifecycle.write_record(run_dir, lifecycle.LifecycleRecord( + run_id="wfr-stuck", unit="nerve-wf-wfr-stuck-n.scope", boot_id=lifecycle._boot_id())) + monkeypatch.setattr(lifecycle, "reap", lambda rd: lifecycle.Receipt(outcome="pending_retry", error="x")) + c = lifecycle.WorkflowContainment(run_dir=run_dir, run_id="wfr-stuck") + with pytest.raises(lifecycle.LifecycleError): + lifecycle.prepare_launch(c, ["/bin/true"]) + + def test_reap_cross_boot_record_never_signals(tmp_path, monkeypatch): run_dir = tmp_path / "b" run_dir.mkdir() lifecycle.write_record(run_dir, lifecycle.LifecycleRecord( - run_id="wfr-b", session_id="workflow:wfr-b", unit="nerve-wf-wfr-b-n.scope", - boot_id="prior-boot", generation="old", invocation_id="i", - control_group="/user.slice/nerve-wf-wfr-b-n.scope", - )) + run_id="wfr-b", unit="nerve-wf-wfr-b-n.scope", boot_id="prior-boot", + invocation_id="i", control_group="/user.slice/nerve-wf-wfr-b-n.scope")) + monkeypatch.setattr(lifecycle, "_unit_identity", _fail_if_called) monkeypatch.setattr(lifecycle, "_scope_stop", _fail_if_called) - r = lifecycle.reap(run_dir) - assert r.outcome == "complete" and not r.signalled # nothing survives a reboot + assert lifecycle.reap(run_dir).outcome == "complete" # nothing survives a reboot + + +def test_reap_query_failure_is_pending_not_complete(tmp_path, monkeypatch): + run_dir = tmp_path / "q" + run_dir.mkdir() + lifecycle.write_record(run_dir, lifecycle.LifecycleRecord( + run_id="wfr-q", unit="nerve-wf-wfr-q-n.scope", boot_id=lifecycle._boot_id(), + invocation_id="inv", control_group="/user.slice/nerve-wf-wfr-q-n.scope")) + # systemctl query failed → must NOT be read as "scope gone" + monkeypatch.setattr(lifecycle, "_unit_identity", lambda u: None) + monkeypatch.setattr(lifecycle, "_scope_stop", _fail_if_called) + assert lifecycle.reap(run_dir).outcome == "pending_retry" + # unit present but InvocationID blank (recorded one exists) → don't signal + monkeypatch.setattr(lifecycle, "_unit_identity", + lambda u: {"LoadState": "loaded", "InvocationID": "", "ControlGroup": "", "Description": ""}) + # reset the stored receipt so reap re-evaluates + lifecycle.write_record(run_dir, lifecycle.LifecycleRecord( + run_id="wfr-q", unit="nerve-wf-wfr-q-n.scope", boot_id=lifecycle._boot_id(), + invocation_id="inv", control_group="/user.slice/nerve-wf-wfr-q-n.scope")) + assert lifecycle.reap(run_dir).outcome == "pending_retry" -def test_reap_corrupt_record_does_not_signal(tmp_path, monkeypatch): +@pytest.mark.parametrize("body", ["{ not valid json ", '{"run_id":"r","unit":"evil; rm.scope","boot_id":"b"}']) +def test_reap_corrupt_or_invalid_record_never_signals(tmp_path, monkeypatch, body): run_dir = tmp_path / "c" run_dir.mkdir() - (run_dir / lifecycle.RECORD_NAME).write_text("{ not valid json ") + lifecycle.record_path(run_dir).write_text(body) monkeypatch.setattr(lifecycle, "_scope_stop", _fail_if_called) assert lifecycle.reap(run_dir).outcome == "pending_retry" +def test_record_rejects_foreign_cgroup_path(): + with pytest.raises(lifecycle.LifecycleError): + lifecycle.LifecycleRecord(run_id="r", unit="nerve-wf-r-n.scope", boot_id="b", + control_group="/system.slice/other.scope").validate() + + def _fail_if_called(*a, **k): - raise AssertionError("scope stop must not be called here") + raise AssertionError("must not be called for this case") # -- reconcile --------------------------------------------------------------- # @requires_scope -def test_reconcile_reaps_prior_generation(tmp_path, scopes): +def test_reconcile_reaps_recorded_scope(tmp_path, scopes): rec, gc = _launch_escaper(scopes, tmp_path / "wf" / "wfr-old", "wfr-old") - # Rewrite the record's generation to a prior incarnation's. - rec.generation = "prior-daemon-generation" - lifecycle.write_record(tmp_path / "wf" / "wfr-old", rec) results = lifecycle.reconcile(tmp_path / "wf") - assert [rid for rid, _ in results] == ["wfr-old"] + assert ("wfr-old", ) == tuple(rid for rid, _ in results) + assert results[0][1].outcome == "complete" assert not _alive(gc) -def test_reconcile_skips_current_generation(tmp_path): - run_dir = tmp_path / "wf" / "wfr-live" - run_dir.mkdir(parents=True) - lifecycle.write_record(run_dir, lifecycle.LifecycleRecord( - run_id="wfr-live", session_id="workflow:wfr-live", - unit="nerve-wf-wfr-live-n.scope", boot_id=lifecycle._boot_id(), - generation=lifecycle.current_generation(), - )) - assert lifecycle.reconcile(tmp_path / "wf") == [] # active owner, untouched - - -# -- record validation ------------------------------------------------------- # - - -def test_record_rejects_unsafe_unit_and_foreign_cgroup(): - with pytest.raises(lifecycle.LifecycleError): - lifecycle.LifecycleRecord(run_id="r", session_id="s", unit="evil; rm -rf.scope", - boot_id="b", generation="g").validate() - with pytest.raises(lifecycle.LifecycleError): - lifecycle.LifecycleRecord(run_id="r", session_id="s", unit="nerve-wf-r-n.scope", - boot_id="b", generation="g", - control_group="/system.slice/other.scope").validate() +def test_reconcile_skips_dirs_without_a_record(tmp_path): + (tmp_path / "wf" / "plain-run").mkdir(parents=True) # e.g. a Claude run dir + assert lifecycle.reconcile(tmp_path / "wf") == [] From cd298ee55a7c266d741be8d9b857df8cddea1036 Mon Sep 17 00:00:00 2001 From: serxa Date: Thu, 1 Oct 2026 14:43:29 +0000 Subject: [PATCH 5/6] codex lifecycle: trim comments, drop unused imports Co-Authored-By: Claude Opus 5.5 --- nerve/agent/backends/codex/appserver.py | 10 +++------- nerve/agent/backends/codex/backend.py | 9 +++------ nerve/agent/backends/codex/lifecycle.py | 23 ++++++++--------------- nerve/workflows/service.py | 11 ++++------- 4 files changed, 18 insertions(+), 35 deletions(-) diff --git a/nerve/agent/backends/codex/appserver.py b/nerve/agent/backends/codex/appserver.py index 8d23463bf..f661e9e04 100644 --- a/nerve/agent/backends/codex/appserver.py +++ b/nerve/agent/backends/codex/appserver.py @@ -150,8 +150,7 @@ def __init__( self._client_version = client_version self._request_timeout = request_timeout self._on_server_request = server_request_handler - # cgroup containment for a Codex *workflow* run (None otherwise — the - # launch then behaves exactly as before). See codex/lifecycle.py. + # Set only for contained Codex workflow runs (codex/lifecycle.py). self._containment = containment self._lifecycle_record: lifecycle.LifecycleRecord | None = None @@ -184,9 +183,7 @@ async def start(self) -> dict: args.extend(["--config", kv]) args.extend(["app-server", "--listen", "stdio://"]) - # For a contained workflow run, `prepare_launch` records the owner and - # returns the scope-wrapped argv; it raises before exec if the host - # can't contain the run. + # Wrap the argv in a systemd scope; raises before exec on failure. containment = self._containment if containment is not None: try: @@ -216,8 +213,7 @@ async def start(self) -> dict: f"Failed to spawn codex app-server ({self._bin_path}): {e}" ) from e - # Record the scope's InvocationID + resolved cgroup now that systemd-run - # created it. + # Record the scope's InvocationID + cgroup. if containment is not None and self._lifecycle_record is not None: try: self._lifecycle_record = await asyncio.to_thread( diff --git a/nerve/agent/backends/codex/backend.py b/nerve/agent/backends/codex/backend.py index 30d96b2db..745793532 100644 --- a/nerve/agent/backends/codex/backend.py +++ b/nerve/agent/backends/codex/backend.py @@ -660,12 +660,9 @@ def resolve_api_key(self) -> str | None: def _workflow_containment( backend: "CodexBackend", spec: SessionSpec, ) -> lifecycle.WorkflowContainment | None: - """cgroup containment for a Codex ``workflow:`` session in strict mode, else - None (interactive/cron sessions and disabled mode launch unchanged). - - The run id is validated and the run dir confined under ``runs_dir`` so a - crafted session id can neither escape the directory nor claim signalling - authority.""" + """Containment for a ``workflow:`` session in strict mode. None for other + sessions, disabled mode, or a run id that isn't a run dir under + ``runs_dir``.""" sid = spec.session_id or "" if not sid.startswith(_WORKFLOW_SESSION_PREFIX): return None diff --git a/nerve/agent/backends/codex/lifecycle.py b/nerve/agent/backends/codex/lifecycle.py index 2ea939bac..f638ba51b 100644 --- a/nerve/agent/backends/codex/lifecycle.py +++ b/nerve/agent/backends/codex/lifecycle.py @@ -13,7 +13,6 @@ import json import logging -import os import re import shutil import subprocess @@ -21,7 +20,6 @@ import uuid from dataclasses import asdict, dataclass from pathlib import Path -from typing import Iterable from nerve.utils.fs import atomic_write_text @@ -216,7 +214,7 @@ def build_scope_argv(unit: str, inner_argv: list[str]) -> list[str]: def _unit_identity(unit: str) -> dict[str, str] | None: """``systemctl --user show`` the unit's identity props, or None if the query - itself failed (which must never be read as 'the scope is gone').""" + failed.""" try: proc = subprocess.run( ["systemctl", "--user", "show", unit, @@ -315,11 +313,10 @@ class WorkflowContainment: def prepare_launch( containment: WorkflowContainment, inner_argv: list[str], ) -> tuple[list[str], LifecycleRecord]: - """Record the owner, then return the scope-wrapped argv. Raises before - anything execs if the host can't contain the run, if the record can't be - written, or if a previous attempt's scope is not confirmed reaped — the - engine can recreate a crashed client for the same run, and a fresh launch - must not overwrite (and orphan) a still-live prior scope.""" + """Record the owner, then return the scope-wrapped argv. A record left by an + earlier launch of the same run is reaped first. Raises before exec if the + host can't contain the run, the record can't be written, or the prior scope + isn't confirmed reaped.""" caps = detect_capabilities() if not caps.ok: raise ContainmentUnavailable(f"strict cgroup containment unavailable: {caps.reason}") @@ -371,14 +368,12 @@ def reap(run_dir: Path) -> Receipt: try: write_record(run_dir, record) except OSError as e: - # A stop we can't durably record cannot be reported complete. return Receipt(outcome="pending_retry", error=f"receipt not persisted: {e}") return receipt def _do_reap(record: LifecycleRecord) -> Receipt: - # A reboot leaves no process alive, and the unit name may now be foreign — - # resolve without signalling. + # Recorded on a previous boot: nothing from it is alive; don't signal. if record.boot_id and _boot_id() and record.boot_id != _boot_id(): return Receipt(outcome="complete") @@ -386,8 +381,7 @@ def _do_reap(record: LifecycleRecord) -> Receipt: if ident is None: return Receipt(outcome="pending_retry", error="unit query failed") if ident.get("LoadState", "") == "not-found": - # A transient scope's cgroup is removed only once empty, so a - # not-found unit means the tree is gone. No signal sent. + # A transient scope is unloaded only once its cgroup is empty. return Receipt(outcome="complete") # Verify identity before signalling: an exact InvocationID when recorded, or @@ -411,8 +405,7 @@ def _do_reap(record: LifecycleRecord) -> Receipt: def reconcile(runs_dir: Path) -> list[tuple[str, Receipt]]: """Reap every run's scope from its record. Called at startup, after active - runs have been marked failed, so no record belongs to a live run; reap is - idempotent, so already-complete records are no-ops.""" + runs have been marked failed.""" root = Path(runs_dir) if not root.is_dir(): return [] diff --git a/nerve/workflows/service.py b/nerve/workflows/service.py index d59f71540..332242f24 100644 --- a/nerve/workflows/service.py +++ b/nerve/workflows/service.py @@ -160,8 +160,7 @@ async def start(self) -> None: "survive the restart; restart them explicitly if still needed.", priority="high", ) - # Reap Codex workflow-run cgroup scopes left by a previous incarnation, - # before the monitor loop dispatches anything new. + # Reap Codex workflow-run scopes left by the previous daemon. await self._reconcile_codex_lifecycle() interval = max(5, int(self.config.workflows.poll_interval_seconds)) self._monitor_task = asyncio.create_task(self._monitor_loop(interval)) @@ -719,8 +718,7 @@ async def _finalize_terminal(self, run_id: str, status: str, detail: dict) -> No logger.exception( "workflow run completion listener failed for %s", run_id, ) - # Reap this run's contained descendants from its durable record, - # detached so it never blocks the terminal path (no-op without a record). + # Reap the run's contained descendants in the background. if str(run.get("engine") or "") == ENGINE_CODEX: self._spawn_detached( self._reap_codex_lifecycle(run_id), @@ -738,9 +736,8 @@ def _is_quiet(run: dict | None) -> bool: # ------------------------------------------------------------------ # async def _reconcile_codex_lifecycle(self) -> None: - """Reap cgroup scopes recorded by a previous run. Runs regardless of the - current mode so disabling containment does not strand a crashed run's - scope. Best effort — never blocks the monitor loop.""" + """Reap scopes recorded by earlier runs, in every mode. Errors are + logged.""" try: results = await asyncio.to_thread( codex_lifecycle.reconcile, self.runs_dir(), From ba5a09ecdebea0897af7b2365b404761b90ae39d Mon Sep 17 00:00:00 2001 From: serxa Date: Wed, 21 Oct 2026 10:32:15 +0000 Subject: [PATCH 6/6] codex lifecycle: stop the scope on client close, document strict mode CodexAppServerClient.close() now stops the run's scope after killing the process group, so a kill, budget stop, transport-retry replacement or daemon shutdown also kills setsid'd descendants. The terminal reap moves from _finalize_terminal (where it could kill the app-server mid-turn and trigger a relaunch) to after client teardown, as a backstop. After a stop, the reap also accepts the unit unloading as proof of an empty scope, which covers records written before the scope registered. Document codex.lifecycle.mode and its systemd user manager prerequisite. Add an integration test that stops a stalled run via kill_run, the budget kill and engine.stop_session with the real engine, service and backend. Co-Authored-By: Claude Opus 5.5 --- docs/config.md | 16 +++ nerve/agent/backends/codex/appserver.py | 19 ++- nerve/agent/backends/codex/lifecycle.py | 23 +++- nerve/workflows/service.py | 9 +- tests/fixtures/fake_codex_appserver.py | 8 ++ tests/test_codex_lifecycle.py | 159 ++++++++++++++++++++++++ 6 files changed, 221 insertions(+), 13 deletions(-) diff --git a/docs/config.md b/docs/config.md index 59e2d5ebb..e45fa3d0a 100644 --- a/docs/config.md +++ b/docs/config.md @@ -1045,8 +1045,24 @@ codex: # active when a codex backend is selected max_concurrency: 2 # hard cap, even if a workflow asks for more default_token_budget: 250000 # default and maximum per workflow max_agents: 8 # lifetime worker cap per workflow + lifecycle: + mode: disabled # disabled | strict (workflow-run cgroup containment) ``` +`codex.lifecycle.mode: strict` launches each Codex workflow run's app-server +inside a transient systemd user scope (`systemd-run --user --scope`). Sandboxed +commands that `setsid` or double-fork out of the app-server's process group stay +in that cgroup. Closing the run's client stops the scope, which kills them as +well. Interactive and cron Codex sessions are not affected. + +`strict` needs Linux, cgroup v2, and a systemd user manager that the daemon can +reach (`systemctl --user show-environment` succeeds as the daemon's user). +Docker containers and the system-level unit in [setup](setup.md) usually have +no user manager. There `strict` fails every Codex workflow run at launch rather +than run it uncontained, so leave it `disabled` on those hosts. Each contained +run writes `lifecycle.json` in its run directory. On startup, Nerve stops any +scope still recorded there in either mode, which cleans up after a crash. + `codex.ultracode.dashboard` exposes run journals in Nerve's authenticated UI. It does not start Ultracode's detached dashboard process. Keep `codex.ultracode.ui: false`: the upstream process serves unauthenticated diff --git a/nerve/agent/backends/codex/appserver.py b/nerve/agent/backends/codex/appserver.py index e89d892f6..c7ca57ebf 100644 --- a/nerve/agent/backends/codex/appserver.py +++ b/nerve/agent/backends/codex/appserver.py @@ -276,7 +276,8 @@ def is_alive(self) -> bool: ) async def close(self) -> None: - """Terminate the subprocess and fail everything pending.""" + """Terminate the subprocess, fail everything pending, then stop the + run's scope if the launch was contained.""" self._closed = True tasks = [ task for task in (self._reader_task, self._stderr_task) @@ -309,6 +310,22 @@ async def close(self) -> None: self._stderr_task = None self._server_request_tasks.clear() + # Stop the scope too: it holds descendants outside the process group. + if self._containment is not None and self._lifecycle_record is not None: + try: + receipt = await asyncio.to_thread( + lifecycle.reap, self._containment.run_dir, + unit=self._lifecycle_record.unit, + ) + except Exception: # noqa: BLE001 + logger.exception("codex scope reap failed on close") + else: + if receipt.outcome not in ("complete", "no_scope"): + logger.warning( + "codex scope %s not reaped on close (%s: %s)", + self._lifecycle_record.unit, receipt.outcome, receipt.error, + ) + @staticmethod def _signal_process_tree( proc: asyncio.subprocess.Process, sig: signal.Signals, diff --git a/nerve/agent/backends/codex/lifecycle.py b/nerve/agent/backends/codex/lifecycle.py index f638ba51b..8b12f4517 100644 --- a/nerve/agent/backends/codex/lifecycle.py +++ b/nerve/agent/backends/codex/lifecycle.py @@ -33,6 +33,7 @@ # systemd-run sends TERM, then SIGKILL after this grace, over the whole cgroup. TERM_GRACE_SECONDS = 5 STOP_TIMEOUT_SECONDS = 20 +EMPTY_WAIT_SECONDS = 3 _UNIT_RE = re.compile(r"^nerve-wf-[A-Za-z0-9:_.-]+\.scope$") _RUN_ID_RE = re.compile(r"^wfr-[A-Za-z0-9_]+$") @@ -347,9 +348,10 @@ def record_launched(run_dir: Path, record: LifecycleRecord) -> LifecycleRecord: # -- reap + reconcile -------------------------------------------------------- # -def reap(run_dir: Path) -> Receipt: +def reap(run_dir: Path, *, unit: str | None = None) -> Receipt: """Reap a run's contained descendants from its durable record. Idempotent: - a record already carrying a terminal receipt is returned unchanged.""" + a record already carrying a terminal receipt is returned unchanged. With + ``unit``, a record naming a different unit is left alone (``no_scope``).""" run_dir = Path(run_dir) if not record_path(run_dir).exists(): return Receipt(outcome="no_scope") @@ -358,7 +360,7 @@ def reap(run_dir: Path) -> Receipt: except LifecycleError as e: logger.warning("refusing to reap from corrupt record in %s: %s", run_dir, e) return Receipt(outcome="pending_retry", error="corrupt record") - if record is None: + if record is None or (unit is not None and record.unit != unit): return Receipt(outcome="no_scope") prev = _receipt_from(record) if prev is not None and prev.is_terminal(): @@ -398,9 +400,18 @@ def _do_reap(record: LifecycleRecord) -> Receipt: ok, err = _scope_stop(record.unit) if not ok: return Receipt(outcome="pending_retry", error=err) - if _cgroup_gone_or_empty(record.control_group) is True: - return Receipt(outcome="complete") - return Receipt(outcome="pending_retry", error="scope still populated after stop") + # The cgroup can report populated briefly after the stop returns. A record + # written before registration has no cgroup path; the unit unloading then + # confirms the scope is empty. + deadline = time.monotonic() + EMPTY_WAIT_SECONDS + while True: + if _cgroup_gone_or_empty(record.control_group) is True: + return Receipt(outcome="complete") + if (_unit_identity(record.unit) or {}).get("LoadState") == "not-found": + return Receipt(outcome="complete") + if time.monotonic() >= deadline: + return Receipt(outcome="pending_retry", error="scope still populated after stop") + time.sleep(0.1) def reconcile(runs_dir: Path) -> list[tuple[str, Receipt]]: diff --git a/nerve/workflows/service.py b/nerve/workflows/service.py index e881abb8a..2b4316ad5 100644 --- a/nerve/workflows/service.py +++ b/nerve/workflows/service.py @@ -755,12 +755,6 @@ async def _finalize_terminal(self, run_id: str, status: str, detail: dict) -> No logger.exception( "workflow run completion listener failed for %s", run_id, ) - # Reap the run's contained descendants in the background. - if str(run.get("engine") or "") == ENGINE_CODEX: - self._spawn_detached( - self._reap_codex_lifecycle(run_id), - f"workflow-lifecycle-reap-{run_id}", - ) @staticmethod def _is_quiet(run: dict | None) -> bool: @@ -1029,6 +1023,9 @@ async def _teardown_client(self, run_id: str) -> None: ) except Exception: # noqa: BLE001 logger.exception("client teardown failed for %s", session_id) + # Closing a contained client stops its scope; this reaps one the close + # left behind (no-op without a record). + await self._reap_codex_lifecycle(run_id) async def _stop_session_hard(self, run_id: str) -> None: """Graceful interrupt → grace window → force client discard. diff --git a/tests/fixtures/fake_codex_appserver.py b/tests/fixtures/fake_codex_appserver.py index b25d1d288..61da90ce8 100755 --- a/tests/fixtures/fake_codex_appserver.py +++ b/tests/fixtures/fake_codex_appserver.py @@ -27,6 +27,8 @@ transport) big_stderr — writes one ~2.5 MiB stderr line mid-turn, then completes normally (stderr loop must keep draining) + stall — starts the turn, then emits nothing and ignores + turn/interrupt close_stdout_mid_turn — closes stdout mid-turn but KEEPS RUNNING (reader-death liveness: is_alive() must flip False even though the process is still up) @@ -257,6 +259,12 @@ def run_turn(thread_id: str, turn_id: str) -> None: sys.stdout.flush() os._exit(1) + if MODE == "stall": + notify("turn/started", {"threadId": thread_id, + "turn": {"id": turn_id, "status": "inProgress", "items": []}}) + time.sleep(3600) + return + if MODE == "interrupt": notify("turn/started", {"threadId": thread_id, "turn": {"id": turn_id, "status": "inProgress", "items": []}}) diff --git a/tests/test_codex_lifecycle.py b/tests/test_codex_lifecycle.py index 2bcdeadf7..60ade7403 100644 --- a/tests/test_codex_lifecycle.py +++ b/tests/test_codex_lifecycle.py @@ -323,3 +323,162 @@ def test_reconcile_reaps_recorded_scope(tmp_path, scopes): def test_reconcile_skips_dirs_without_a_record(tmp_path): (tmp_path / "wf" / "plain-run").mkdir(parents=True) # e.g. a Claude run dir assert lifecycle.reconcile(tmp_path / "wf") == [] + + +# -- client close + workflow stop paths ------------------------------------- # + + +def _escaping_launcher(tmp_path: Path) -> tuple[Path, Path]: + """An executable that starts a setsid escapee (in its own launch dir), then + execs the fake app-server. Returns (launcher, launches_dir).""" + launches = tmp_path / "launches" + launches.mkdir() + launcher = tmp_path / "codex-launcher" + launcher.write_text( + "#!/bin/sh\n" + 'case "$*" in *--version*) exec "$NERVE_PY" "$NERVE_FAKE" "$@";; esac\n' + 'out=$(mktemp -d "$NERVE_LAUNCHES/l-XXXXXX")\n' + '"$NERVE_PY" "$NERVE_ESCAPER" spawn-escapee "$out" >/dev/null 2>&1\n' + 'exec "$NERVE_PY" "$NERVE_FAKE" "$@"\n' + ) + launcher.chmod(0o755) + return launcher, launches + + +def _launcher_env(launches: Path, token: str) -> dict[str, str]: + return {"NERVE_PY": sys.executable, "NERVE_FAKE": FAKE, "NERVE_ESCAPER": ESCAPER, + "NERVE_LAUNCHES": str(launches), "ESCAPER_TOKEN": token} + + +def _escapees(launches: Path) -> list[int]: + return [int((d / "grandchild.pid").read_text()) + for d in sorted(launches.iterdir()) if (d / "grandchild.pid").exists()] + + +def test_reap_with_unit_leaves_a_newer_record_alone(tmp_path, monkeypatch): + run_dir = tmp_path / "w" + run_dir.mkdir() + lifecycle.write_record(run_dir, lifecycle.LifecycleRecord( + run_id="wfr-w", unit="nerve-wf-wfr-w-new.scope", boot_id=lifecycle._boot_id())) + monkeypatch.setattr(lifecycle, "_unit_identity", _fail_if_called) + monkeypatch.setattr(lifecycle, "_scope_stop", _fail_if_called) + assert lifecycle.reap(run_dir, unit="nerve-wf-wfr-w-old.scope").outcome == "no_scope" + + +def test_reap_before_registration_confirms_by_unit_unload(tmp_path, monkeypatch): + run_dir = tmp_path / "p" + run_dir.mkdir() + lifecycle.write_record(run_dir, lifecycle.LifecycleRecord( + run_id="wfr-p", unit="nerve-wf-wfr-p-n.scope", boot_id=lifecycle._boot_id())) + states = iter([ + {"LoadState": "loaded", "InvocationID": "i", "ControlGroup": "", + "Description": lifecycle.SCOPE_DESCRIPTION}, + {"LoadState": "not-found"}, + ]) + stops = [] + monkeypatch.setattr(lifecycle, "_unit_identity", lambda u: next(states)) + monkeypatch.setattr(lifecycle, "_scope_stop", lambda u: (stops.append(u), (True, ""))[1]) + assert lifecycle.reap(run_dir).outcome == "complete" + assert stops == ["nerve-wf-wfr-p-n.scope"] + + +@requires_scope +@pytest.mark.asyncio +async def test_close_stops_own_scope(tmp_path, scopes): + run_dir = tmp_path / "workflow-runs" / "wfr-close" + scopes["run_dirs"].add(run_dir) + launcher, launches = _escaping_launcher(tmp_path) + token = "lc" + uuid.uuid4().hex[:12] + scopes["tokens"].add(token) + cwd = tmp_path / "ws" + cwd.mkdir() + + async def _handler(method, params): + return {} + + client = CodexAppServerClient( + bin_path=str(launcher), cwd=str(cwd), + env={**os.environ, **_launcher_env(launches, token), + "CODEX_HOME": str(tmp_path / "home"), "FAKE_CODEX_MODE": "basic"}, + server_request_handler=_handler, config_overrides=[], + containment=lifecycle.WorkflowContainment(run_dir=run_dir, run_id="wfr-close"), + ) + await client.start() + [gc] = _escapees(launches) + rec = lifecycle.read_record(run_dir) + assert _alive(gc) and lifecycle.proc_cgroup(gc) == rec.control_group + await client.close() + assert not _alive(gc) # escaped the process group, not the scope + assert lifecycle.read_record(run_dir).receipt["outcome"] == "complete" + + +@requires_scope +@pytest.mark.asyncio +@pytest.mark.parametrize("stop_via", ["kill_run", "budget_kill", "stop_session"]) +async def test_workflow_stop_before_content_reaps_and_never_relaunches( + db, tmp_path, scopes, monkeypatch, stop_via, +): + """A stop or budget kill while the turn has produced no content must not + let the engine's transport retry launch a second scope, and must leave no + descendant alive. Real AgentEngine, WorkflowRunService and CodexBackend.""" + import asyncio + + from nerve.agent.engine import AgentEngine + from nerve.workflows.service import ENGINE_CODEX, WorkflowRunService + from tests.actor_rows import ensure_system_principal + + await ensure_system_principal(db) + launcher, launches = _escaping_launcher(tmp_path) + token = "lc" + uuid.uuid4().hex[:12] + scopes["tokens"].add(token) + for k, v in _launcher_env(launches, token).items(): + monkeypatch.setenv(k, v) + monkeypatch.setenv("FAKE_CODEX_MODE", "stall") + runs_dir = tmp_path / "workflow-runs" + cfg = NerveConfig.from_dict({ + "workspace": str(tmp_path / "ws"), + "workflows": {"runs_dir": str(runs_dir), "kill_grace_seconds": 1}, + "codex": {"bin_path": str(launcher), "home_dir": str(tmp_path / "home"), + "model": "gpt-5.6-sol", "lifecycle": {"mode": "strict"}}, + }) + (tmp_path / "ws").mkdir() + runs_dir.mkdir() + engine = AgentEngine(cfg, db) + svc = WorkflowRunService(cfg, db, engine) + engine.add_stop_listener(svc._on_session_stop) # registered by svc.start() + run_id = "wfr-race" + stop_via.replace("_", "") + run_dir = runs_dir / run_id + scopes["run_dirs"].add(run_dir) + await db.create_workflow_run(run_id, ENGINE_CODEX, {"prompt": "p"}, 5.0) + await db.transition_workflow_run(run_id, "running", expect=("pending",)) + task = asyncio.create_task(svc._execute(run_id)) + svc._exec_tasks[run_id] = task + try: + deadline = asyncio.get_running_loop().time() + 30 + while not (_escapees(launches) and engine.is_session_running(f"workflow:{run_id}") + and (lifecycle.read_record(run_dir) or lifecycle.LifecycleRecord( + run_id=run_id, unit="nerve-wf-x.scope", boot_id="")).control_group): + assert asyncio.get_running_loop().time() < deadline, "turn never started" + await asyncio.sleep(0.1) + [gc] = _escapees(launches) + assert _alive(gc) + + if stop_via == "kill_run": + await svc.kill_run(run_id, "test") + elif stop_via == "budget_kill": + await svc._budget_kill(await db.get_workflow_run(run_id), 6.0) + else: + await engine.stop_session(f"workflow:{run_id}") + + deadline = asyncio.get_running_loop().time() + 60 + while svc._exec_tasks or svc._kill_tasks: + assert asyncio.get_running_loop().time() < deadline, "stop did not settle" + await asyncio.gather(*svc._exec_tasks.values(), *svc._kill_tasks, + return_exceptions=True) + finally: + await engine.shutdown() + + assert len(list(launches.iterdir())) == 1 # no relaunch after terminal + assert not any(_alive(pid) for pid in _escapees(launches)) + assert lifecycle.read_record(run_dir).receipt["outcome"] == "complete" + assert (await db.get_workflow_run(run_id))["status"] in ("killed", "budget_exhausted")