Skip to content

codex: contain and reap workflow-run sandbox descendants (opt-in cgroup) - #449

Open
serxa wants to merge 5 commits into
mainfrom
serxa/codex-descendant-lifecycle
Open

serxa wants to merge 5 commits into
mainfrom
serxa/codex-descendant-lifecycle

Conversation

@serxa

@serxa serxa commented Sep 18, 2026 •

Copy link
Copy Markdown
Member

Problem

When a Codex workflow run ends (completed, cancelled, budget-stopped, or the daemon crashed), some descendant processes survive as orphans. Codex runs sandboxed commands under codex-linux-sandbox, which setsids into its own process group. CodexAppServerClient._signal_process_tree signals one process group (os.killpg), so these descendants are never reached. They stay parented to PID 1 after the run is done.

Fix

Opt-in codex.lifecycle.mode: strict launches a workflow run's codex app-server inside a transient systemd user scope (systemd-run --user --scope, KillMode=control-group). Every process the run forks stays in that cgroup, so systemctl --user stop kills the whole tree. Cleanup is confirmed by the scope's recursive cgroup.events showing populated=0.

Each contained run writes <runs_dir>/<run>/lifecycle.json with the unit name, InvocationID, boot_id and cgroup. The reap reads only that record, so it runs both when the run ends and at startup after a crash. It signals only a unit whose InvocationID matches the record. A failed systemctl query is retried at the next startup and is never treated as "gone". If the engine recreates a crashed client for the same run, the previous scope is reaped before the new launch.

Scope

  • Off by default. In disabled mode the launch is unchanged. Startup and terminal reap still clean up scopes recorded by earlier strict runs.
  • Applies to Codex workflow runs only. Interactive and cron sessions are unaffected.
  • Needs Linux, a reachable systemd --user manager and cgroup v2. Where any of these is missing, strict fails before exec; it never falls back to an uncontained launch.

Testing

tests/test_codex_lifecycle.py runs real systemd user scopes with a setsid + double-fork escaper fixture: contain and reap, nested child cgroups, refusing to stop a replaced unit, relaunch, startup reconcile, and the fail-before-exec and record-validation paths. No model or network. Existing Codex and workflow suites pass.

serxa and others added 5 commits September 17, 2026 14:38
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 <run>` 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@serxa
serxa marked this pull request as ready for review October 1, 2026 14:43
@alex-clickhouse

Copy link
Copy Markdown
Collaborator

P1: Prevent transport retries from creating a scope after terminal cleanup

At service.py:721–725, _finalize_terminal starts the scope reaper before the executor and stop sequence have finished. If the reaper kills the app-server before response content arrives, the engine's transport-recovery path treats the EOF as a crash and launches a replacement client. prepare_launch allows this once the original scope has a complete receipt. The stop sequence then disconnects the replacement client with killpg, but its setsid descendants survive. No further terminal reap is scheduled for the replacement scope.

I reproduced this at cd298ee5 using the real AgentEngine, WorkflowRunService, CodexBackend, and systemd user scopes, with an offline fake app-server that spawns a setsid/double-fork descendant and takes longer than the five-second interrupt grace to finish. Stopping before any response content arrives produced the same result through all three entry points:

  • kill_run: two scope launches, status killed, one surviving descendant.
  • _budget_kill: two scope launches, status budget_exhausted, one surviving descendant.
  • engine.stop_session: two scope launches, status killed, one surviving descendant.

In each case, the replacement scope's lifecycle record had no cleanup receipt. A normal-completion control created one scope and left no survivors. The reproduction makes no model API calls and does not mock lifecycle operations.

Please prevent retries once the run is terminal and perform the final scope reap after the executor and client teardown can no longer create another scope. Add an integration test for stop/budget enforcement racing with transport recovery; testing launch and reap separately does not cover this ordering.

@serxa

serxa commented Oct 6, 2026

Copy link
Copy Markdown
Member Author

TBH I'm not sure if the approach with cgroup is the right thing here. It might be an overkill for this issue. Anyway, this is what my nerve installation came up with when faced the problem on my Linux environment.

If you think direction is not correct feel free to close the PR, otherwise I can address the feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants