Rectify: Orphaned Detached Child Processes — Spawner-Death Immunity - #4695
Merged
Trecek merged 22 commits intoAug 19, 2026
Conversation
…ring Implements the spawner-death immunity mechanism for issue #4678 (Incident A): every detached child spawned through spawn_owned_process now durably records a process tether (spawner identity, child identity, absolute not_after ceiling) as a mandatory, fail-closed side effect. sweep_orphaned_tethers is the single generic reaper wired into every boot/open chokepoint, reaping only identity-verified targets of a dead-or-expired guardian. New module: execution/process/_process_tether.py - TetherRecord/TetherSpec/TetherSweepReport/TetherSweepOutcome/OrphanedTetherRecord - write_tether/remove_tether/update_tether_workload (via write_versioned_json/ read_versioned_json) - sweep_orphaned_tethers[_async] and find_orphaned_tethers (read-only listing) - DEFAULT_TETHER_CEILING_SECONDS (24h) / INTERACTIVE_TETHER_CEILING_SECONDS (48h) Core wiring: - spawn_owned_process gains a required `tether: TetherSpec` kwarg; writes the record after group-leadership validation, fails closed on unwritable dirs or unreadable child identity (Linux only). cleanup() removes the tether on a complete settlement. - run_managed_async/run_managed_sync/DefaultSubprocessRunner thread a new `ceiling_seconds` parameter through to the tether; the SubprocessRunner Protocol and RecordingSubprocessRunner/ReplayingSubprocessRunner gained the matching parameter for protocol conformance. - PTY-wrapper hazard: run_managed_async resolves and attaches the real workload's identity to the tether for every PTY-wrapped spawn on Linux, decoupled from whether linux_tracing is enabled — a wrapper-only tether would recreate the orphan class one level down when the wrapper dies but the workload survives reparented. - run_cook_attempt gains a required `not_after` ceiling enforced in both the non-PTY poll loop and the PTY observer's cancelled callback — the live spawner's own in-process backstop, independent of the tether sweep. - New IL-008 exception: exploration/collectors/_bounded.py migrates its one raw subprocess.Popen(start_new_session=True) onto the funnel (the tether plan's mandated closure of the last unfunneled detached spawn). Chokepoint wiring: sweep_orphaned_tethers_async in all three lifespan boot gates (fleet/food-truck/skill) and the periodic _cleanup_stale_loop; the codex/daemon manual-only reapers promoted to automatic in fleet/food-truck via _reap_self_excluded_codex_and_daemon_orphans; open_kitchen's new tether_sweep transition step covers interactive sessions. Config: ProcessTetherConfig (orphan_ceiling_seconds/cook_ceiling_seconds, literal defaults parity-tested against the module constants since config cannot import execution) + a warn-only coherence gate; threaded through cook launch (_session_cook.py/_session_launch.py/_fleet_session.py) and the three headless invocation sites, mirroring max_extension_seconds's existing threading pattern. Ops surface: `autoskillit process-orphans [--reap]` CLI command mirroring codex-orphans/daemon-orphans, plus a warn-only doctor check (46). Registry-sync: core/__init__.pyi (read_pid_namespace_inode), execution process/__init__.py and execution/__init__.py re-export facades, DURABLE_ARTIFACT_WRITERS, test_process_submodules.py's strict-equality export set, the kill-funnel and StrEnum-compare AST allowlists, doctor check count, and various line/file-count arch-guard exemptions. task test-check: 38057 passed, 616 skipped, 27 xfailed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e-mark - _record_spawn captures spawner_pid/spawner_starttime_ticks/boot_id/ child_starttime_ticks/pidns_inode/not_after on the manifest (optional additions, schema_version stays 1) so the codex store is self-sufficient for its own recovery decisions independent of the Phase-1 tether. - recover() replaces the unconditional mark-without-kill with verify-before-mark: identity-verified live child is killed via the shared kill funnel before being marked reaped; dead/identity-mismatched children are marked reaped without a kill attempt; legacy manifests with no identity fields fail closed (never kill, never lie) and leave the view retained with state="failed"; kill failures also leave the view retained. These leave-unreaped outcomes warn-and-continue rather than raising, so an unresolvable view can never block the two unguarded recover_cook_history() call sites (_session_cook.py, _session_order.py). - ceiling_seconds threaded through prepare_attempt/record_spawn from both interactive cook launch paths (cook, fleet/nonpersistent launch), with a literal-default fallback on the CodingAgentBackend protocol itself since core (IL-0) cannot import execution (IL-1). - Registry-sync: _codex_session_storage.py added to the kill-funnel AST allowlist alongside its new kill_process_tree call. - Death-assertion upgrades: idle-stall, PTY stale/timeout, heartbeat, and Channel B lifecycle tests now assert the child process actually died, not just the termination reason enum. - New test_extension_cap_kills_real_process proves max_extension_seconds kills a real continuously-active process, not just a fake pid=1 scope decision. Phase 2 of the orphan-process-immunity rectify plan (.autoskillit/temp/rectify/rectify_orphan_process_immunity_2026-08-17_151729.md). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…emd ceiling - test_no_detached_spawn_outside_owned_funnel: codebase-wide AST guard that flags any subprocess.Popen/asyncio.create_subprocess_exec/_shell call setting start_new_session=True or process_group=0 outside the funnel (spawn_owned_process), plus a violations-by-default fallback for calls whose kwargs cannot be statically proven safe (**kwargs unpack, non-literal keyword values) — closing the exact indirection the funnel itself uses internally. Two-row allowlist ties rationale to entry so it cannot drift the way the kill-funnel guard's docstring already had. - test_spawn_owned_process_requires_tether: locks the required-param invariant spawn_owned_process already enforces (Phase 1) against future soft-defaulting. - resource_exhaustion_guard.py: new PreToolUse guard denying Bash/run_cmd commands matching a backgrounded-infinite-loop or `kill %N` job-control pattern — the exact shape of issue #4678 Incident B. Documents its own threat model (GuardFall bypass classes) rather than claiming completeness; the tether ceiling and host limits are the structural backstop this pattern guard cannot be. - Optional, default-off systemd-run --user --scope ceiling: probe_systemd_ scope_available + wrap_systemd_scope in _process_tether.py, threaded through run_managed_async/DefaultSubprocessRunner/the SubprocessRunner protocol, the three headless invocation sites, and both interactive cook spawn branches (non-PTY and PTY-wrapper). Inert unless ProcessTetherConfig.systemd_scope_enabled is set; the config field's docstring records the three conditions that make it real (WSL2 systemd, loginctl enable-linger, probe success) and the one that makes it unreliable (RuntimeMaxSec is not reliably enforced) — the tether's wall-clock not_after stays the ceiling of record. - Fixture/budget maintenance: refreshed the codex hook-event snapshot and HOOK_REGISTRY_HASH for the new guard registration; bumped the three architectural size budgets (headless/_headless_execute.py, hooks/guards/ file count, _codex_session_storage.py line limit) that the legitimate parameter/file additions in Phases 2-3 pushed past their prior ceilings, with rationale attached at each site. - Test-double repair: several hand-written CodingAgentBackend stubs across tests/cli/ implemented cook_session_context with an explicit parameter list (not **kwargs), so Phase 2's ceiling_seconds addition and this phase's systemd_scope_enabled addition to the protocol surfaced as TypeErrors when real callers passed them — fixed each stub in place rather than loosening them to **kwargs, matching the existing style at each call site. Phase 3 (final phase) of the orphan-process-immunity rectify plan (.autoskillit/temp/rectify/rectify_orphan_process_immunity_2026-08-17_151729.md). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three registry-style contract tests only run under the full suite (they were absent from every targeted subset I'd run through Phase 3): - test_moved_scripts_must_be_in_retired: new subdir script resource_exhaustion_guard.py needs a NEW_SUBDIR_BASENAMES entry (it was never flat, so RETIRED_SCRIPT_BASENAMES doesn't apply). - test_safety_hook_counts_match_registry: docs/safety/hooks.md's counts (51 total / 37 PreToolUse) and the missing per-hook section for the new guard, now 52 / 38 with an entry matching the module's threat-model docstring. - test_coding_agent_backend_new_lifecycle_signatures_are_exact: this one predates Phase 3 — Phase 2 added ceiling_seconds to CodingAgentBackend.cook_session_context but never updated this exact- signature registry test. Fixed in the same commit as the other two since all three surfaced together from the same full-suite run. Verified via the test_check MCP gate (unfiltered — "large_changeset" full run): 38156 passed, 619 skipped, 27 xfailed, 0 failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…velop Rebasing onto upstream/develop brought in the #4664 decompose refactor's new execution/process/_termination.py (9 -> 10 files), and this branch's own _process_tether.py (Phase 1) pushes the same subdir to 11 -- one over the default 10-file cap with no prior exemption entry for this subdir. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… note Two MISSING findings from the 2026-08-18 audit-impl run, both real gaps against Phase 1 Step 2 item 6 / Step 1 test 6 of the rectify plan: - cook/order startup did not sweep orphaned tethers before recover_cook_history() at the unguarded call sites (_session_cook.py:325, _session_order.py:189), leaving the Phase 1 "every chokepoint" invariant broken for cook/order startup. Both now run sweep_orphaned_tethers(default_tether_dir()) fail-open before the call, matching every other chokepoint's try/except-warn pattern. - test_cook_attempt_enforces_ceiling_in_both_wait_branches was absent, so the live-spawner ceiling enforcement claim (both the non-PTY _wait_for_owned_exit poll and the PTY observer.relay cancelled-callback terminate at not_after) had no regression test. Added to tests/cli/test_cook_process_lifecycle.py, mirroring this file's existing _wait_until_gone/_spec idioms and the elapsed-time assertion style from test_extension_cap_kills_real_process. Also addresses the Phase 2 ODD note: test_record_spawn_captures_spawn_identity used os.getpid() as both spawner and child pid, so child_starttime_ticks trivially matched spawner_starttime_ticks and never proved independent derivation from the child's own /proc entry. Now spawns a real child. Not addressed (documented disagreement / non-blocking per the audit's own labels, see conversation record): - ODD R6 (write_tether machine_local=False): the plan's machine_local=True + detection=None combination is self-contradictory — _validate_durable_artifact_writer_defs asserts every machine_local=True entry must have a detection callable, so that combination would crash at import time. machine_local in this registry means "bakes an AutoSkillit install path that a relocation could break" (see the four existing machine_local=True entries, all paired with a find_broken_* self-heal detector); tether records carry only process identity (PIDs, boot_id) with zero AutoSkillit-install-path content and a wholly different staleness mechanism (the sweep, not repair-on-detect). False is the semantically correct value. - ODD R13/R16 (Phase 3/Phase 2 work landed in the Phase 1 commit) and R17/R18 (Phase 3 tests alongside Phase 1 tests): all four are commit-boundary-only observations the audit itself labels non-blocking / "not a defect" / "no behavioral issue". Every symbol each note describes is correctly wired in the tree regardless of which commit it landed in; moving them would require rewriting this branch's commit history (interactive rebase), which this environment disallows and which AGENTS.md's commit discipline does not require for informational findings. - Phase 2 "out-of-scope work visible in diff": the audit labels this informational/not blocking; no separate action beyond what's above. task test-check: 38171 passed, 621 skipped, 27 xfailed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Review finding (critical/defense): TetherSpec accepted any ceiling_seconds value (negative, zero, NaN, inf) without validation. A non-positive value makes not_after pre-past, causing immediate reap of a live spawner's child; NaN silently disables the ceiling check forever. Add a __post_init__ mirroring ProcessTetherConfig.validate()'s existing check and the codebase's math.isfinite() convention used elsewhere.
…ction Review finding (warning/defense): TetherRecord (frozen dataclass, durably written to disk and re-read on every sweep) had no range validation on child_pid/child_pgid/spawner_pid/starttime_ticks/not_after/ workload_pid. Add __post_init__ raising ValueError on invalid values; callers already wrap deserialization in except (ValueError, KeyError, TypeError), so no call-site changes are needed.
Review finding (warning/cohesion): run_managed_sync lacked the systemd_scope_enabled defense-in-depth knob that its async sibling run_managed_async already has, leaving sync callers structurally unable to opt into the kernel-enforced ceiling. Mirror run_managed_async's wrap_systemd_scope call.
Review finding (warning/tests): test_pty_wrapped_spawn_updates_tether_with_workload_identity only asserted workload_pid != result.pid and workload_starttime_ticks is not None, which would pass for negative/zero garbage values. Add bound checks so a regression in the real /proc-based identity resolution fails the test.
Follow-up to the prior commit: Pyright flagged `workload_pid > 0` as unsafe since the tuple-unpacked value is typed object. Add an isinstance narrow, matching the workload_starttime_ticks assertion right below it.
…pers Review finding (info/tests): _run_guard and _run_bash_guard were near-identical, differing only in the tool_input key and tool_name. Merge into one _run(cmd, shape=...) helper and add a shape parametrize dimension to the 4 pairs of duplicated test functions, collapsing 8 near-duplicate functions to 4 while preserving every (command, shape) test case. 51/51 tests still pass.
…tests Review finding (warning/tests): test_malformed_json_fail_open and test_missing_cmd_field_fail_open only asserted output == "", not that main() actually exits 0 as documented — the hook exits explicitly on every fail-open path. Add a _run_capture_exit helper (used only by these two tests) and assert exit_code == 0 alongside the existing output assertion. Other suggested coverage (missing tool_name/ tool_input, non-string cmd, cross-shape leak, metacharacter bypass) intentionally not added — already covered elsewhere or explicitly out of this guard's documented scope. 51/51 tests still pass.
Review finding (warning/slop): a 20-line inline comment above ProcessTetherConfig.systemd_scope_enabled (WSL2/linger/probe preconditions, RuntimeMaxSec unreliability) was the sole canonical location for load-bearing operational rationale, disguised as a field comment. Extract it to docs/decisions/0010-systemd-scope-defense-in-depth.md following the existing ADR-0001..0009 convention, replace the inline block with a short cross-referencing comment, and update wrap_systemd_scope's docstring cross-reference to point at the same ADR.
Review finding (info/cohesion): RecordingSubprocessRunner.run, RecordingSubprocessRunner._record_session, and ReplayingSubprocessRunner.run each carried a literal ceiling_seconds: float = 86400.0 default with no parity test guarding drift, unlike the core/execution pair. Verified no import-layer restriction applies (execution/recording.py is IL-1, same layer as execution/process) and no import cycle exists. Import DEFAULT_TETHER_CEILING_SECONDS instead. 52/52 tests still pass.
…probe tests Review finding (info/tests): the four TestSystemdScopeWrap probe tests each re-imported `subprocess as _subprocess` inside the function body despite the module already importing subprocess at top level. Use the module-level import instead. 27/27 tests still pass.
Review finding (info/cohesion): _skill_auto_gate_boot deliberately omits the codex/daemon orphan reap that _fleet_auto_gate_boot and _food_truck_auto_gate_boot both call (per test_boot_step_symmetry.py's explicit carve-out), but the asymmetry was only documented in the test file, not at the call site. Extend the existing docstring omissions sentence.
Review finding (info/cohesion): the CLI (_process_orphans.py) and doctor (_doctor_runtime.py) rendered the same OrphanedTetherRecord+reason shape via independently-maintained f-strings with different prefixes and no shared source of truth. Add format_orphaned_tether_fields() next to OrphanedTetherRecord, export it through execution/process and execution facades, and use it at both call sites. 321/321 relevant tests still pass.
test_check surfaced 3 pinned-fixture failures caused by the review-fix commits in this branch, not by the original PR: - test_rule_4_max_three_segments: the new ADR-0010 filename exceeds the 3-kebab-segment naming rule, same as every other numbered ADR in docs/decisions/ — add it to the existing segment_count allowlist. - test_req_api_003_params: run_managed_sync gained systemd_scope_enabled (REQ-API-003 pin), mirroring run_managed_async's already-pinned REQ-API-001 param set which includes the same field. - test_process_facade_reexports_all_public_symbols: process.__all__ gained format_orphaned_tether_fields. All three are deliberate, reviewed additions from this branch's own fix commits, not regressions — update the pins to match.
…n-process-immunity-20260818-145810
The develop-merge (2edda61, #4694) brought in the FORWARDING_SITES / DYNAMIC_READ_EXEMPTIONS ground-truth registry in tests/_ambient_env_surface.py. This branch's own diff shifted line numbers in six of the registered files (cli/session/_session_launch.py, execution/backends/_codex_probes.py, execution/backends/claude.py, execution/backends/codex.py, execution/evidence_reader.py, server/_lifespan/_session_boots.py), so the registry's line-number keys no longer matched the AST scanner's findings on the merged tree. Updated the 12 stale keys to their current line numbers; justification text and site set are unchanged.
…4699) Empty commit only. The prior CI runs on this branch were pinned to a pre-fix merge-ref snapshot (rerunning a workflow replays the original frozen github.sha rather than recomputing the PR merge ref against the current base branch), so they kept exercising the flaky apt/ripgrep path even after the fix merged to develop. This forces a fresh pull_request synchronize event so the checks run against develop's current tip. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Trecek
deleted the
impl-rectify-orphan-process-immunity-20260818-145810
branch
August 19, 2026 17:51
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Every child-session lifecycle mechanism (timeout, idle/stale watchdogs, extension cap, reap bookkeeping) runs inside the spawning Python process, while children are deliberately detached into fresh process groups. When the spawner dies, enforcement dies with it (issue #4678, Incident A: a codex cook session survived its dead spawner 4 days at ~87% CPU). Four prior fixes (#4246, #4536, #4569, #4573) each closed one orphan signature; each new topology escapes because nothing makes registration-for-out-of-process-enforcement structurally inseparable from spawning.
Closes #4678
Implementation Plan
Plan file:
.autoskillit/temp/rectify/rectify_orphan_process_immunity_2026-08-17_151729.md🤖 Generated with Claude Code via AutoSkillit