Skip to content

repl_env: add managed child queries with bounded cleanup - #1527

Open
jucamohedano wants to merge 6 commits into
openenv:mainfrom
jucamohedano:managed-repl-child-lifecycle
Open

jucamohedano wants to merge 6 commits into
openenv:mainfrom
jucamohedano:managed-repl-child-lifecycle

Conversation

@jucamohedano

@jucamohedano jucamohedano commented Oct 28, 2026 •

Copy link
Copy Markdown

Summary

Adds opt-in background child queries to repl_env through rlm_query(..., blocking=False). Parents can continue working while children run, then inspect or collect their results.

Managed cleanup confirms worker process exit—or reports failure—instead of treating a cancellation request as proof that execution stopped.

Refs #1478.

Behavior

  • rlm_query(prompt) continues to block and return an answer by default.
  • rlm_query(prompt, blocking=False) returns an episode-scoped handle and requires managed mode.
  • rlm_status, rlm_result, and rlm_wait support inspection and result collection. A wait timeout does not cancel the child.
  • Each worker constructs its model client inside its own process; the inference server can remain shared.
  • The root supervisor tracks all registered workers, including nested children, and enforces depth, child-count, concurrency, and execution-time limits.

Parent finalization, reset, and close stop admission and request cooperative shutdown. Remaining workers receive SIGTERM and then SIGKILL after configurable grace periods. Cleanup waits for confirmed process exit; failure blocks environment reuse.

Lifecycle transitions are serialized. Observer callbacks run separately so blocked or failing callbacks cannot prevent worker cleanup.

Scope and possible follow-ups

  • Scoped to repl_env, with no new runtime dependencies or environment transport changes.
  • POSIX-only process supervision, tested on Linux.
  • Does not contain arbitrary subprocesses created by custom code or guarantee inference-server generation cancellation.
  • Not included: REPL tools allowing the parent model to cancel one chosen child while continuing its own work, or send further instructions to an existing child. These could be follow-up contributions.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • New environment
  • Refactoring

Alignment Checklist

  • I have read .claude/docs/PRINCIPLES.md and this PR aligns with our principles
  • I have checked .claude/docs/INVARIANTS.md and no invariants are violated
  • I have run /pre-submit-pr (or bash .claude/hooks/lint.sh and tests) and addressed all issues

The prescribed checks were run. The final checkbox remains unchecked because the repository-wide lint hook reports existing upstream findings, detailed below.

RFC Status

Test Plan

The suite combines unit tests for API routing and bookkeeping with real-process tests for shutdown, nesting, and failure recovery. Model responses are scripted; no GPU or model API credentials are required.

Coverage includes:

  • Blocking defaults, nonblocking handles, and existing model arguments.
  • Rejection of nonblocking queries without managed mode.
  • Shared limits, handle invalidation, and nested-worker ownership.
  • Blocked model calls, including workers that ignore SIGTERM.
  • Reset/close serialization and cleanup-failure recovery.
  • Final answer preservation when cleanup fails for both FINAL(...) and answer-dictionary finalization, with execution, child creation, and reset still blocked.
  • Broken worker communication pipes and watcher startup failure.
  • Callback isolation and protection against signalling already-reaped workers.

With the CI test dependencies installed:

PYTHONPATH=src:envs uv run pytest \
  tests/envs/test_repl_env.py \
  tests/envs/test_repl_env_reset_cache.py \
  tests/envs/test_repl_managed_children.py -v

Validation results

These local results cover the current fix and regression tests. GitHub CI will also validate the published head.

  • Latest run of the unchanged pre-submit test hook: 4,250 passed in 268 seconds, within its existing 300-second deadline.
  • Supplemental Python 3.11 CI cases: 65 passed, giving 4,315 passes across the two runs.
  • Full Python 3.12 CI suite: 4,362 passed.
  • All 60 managed-child regression cases run on both versions without skips.
  • Managed-child testcase time on Python 3.11 reduced from approximately 157 to 96 seconds by removing redundant process setup.
  • Isolated mutation checks confirmed that the lighter tests still detect broken budget accounting and reset/close serialization.
  • Changed-file formatting, import sorting, Ruff lint, and documentation synchronization passed.

Existing upstream lint findings

The repository-wide lint hook still reports 44 formatting files and two import-sort findings. All remaining findings also occur on upstream main; none concern files changed by this PR.

Claude Code Review

  • Reviewed with Claude Code

A separate Codex-assisted manual review covered the implementation, repository principles, and invariants. No Claude Code review is claimed.


Note

Medium Risk
Opt-in, but the change adds POSIX multiprocess supervision, signal-based teardown, and new REPL APIs that affect recursive execution and episode lifecycle; mistakes could leak workers or block environment reuse.

Overview
Adds opt-in managed child queries to repl_env so parents can keep executing REPL code while recursive subtasks run in supervised worker processes. Blocking rlm_query(prompt) behavior is unchanged; rlm_query(..., blocking=False) returns an episode handle and is wired through ManagedChildConfig, ManagedChildren, and ManagedChildBackend, with rlm_status / rlm_result / rlm_wait injected into the REPL namespace.

LocalRLMRunner and create_server_recursive_controller can enable managed mode (mutually exclusive with a custom backend_factory). LocalREPLEnv / REPLEnvironment accept an optional recursive_controller so server and in-process paths share the same recursion wiring. Lifecycle rules stop or reap workers on finalize, reset, and close, with cooperative shutdown escalating to SIGTERM/SIGKILL; failed cleanup surfaces ChildCleanupError and blocks reuse.

Docs for the REPL environment are tightened and expanded for managed usage; system prompts note the keyword-only blocking flag. A large Linux-focused regression suite covers limits, nesting, cleanup, and callbacks.

Reviewed by Cursor Bugbot for commit e2b89d8. Bugbot is set up for automated code reviews on this repo. Configure here.

Add opt-in process supervision and rlm_query(blocking=False), with bounded shutdown, nested worker ownership, callback isolation, and regression coverage.

Refs openenv#1478
Keep all 58 regression cases while using unit checks for routing, budgets, observer errors, and lifecycle locking. Preserve real-process cleanup and nesting coverage and the unchanged pre-submit timeout.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit e2b89d8. Configure here.

Record the computed answer before stopping managed children. Cover FINAL() and answer-dictionary finalization while keeping cleanup failures blocking execution, child creation, and reset.

@burtenshaw burtenshaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes.

P1: Stop managed workers before draining a waiting server session. envs/repl_env/server/repl_environment.py:692 reaches controller cleanup only when env.close() runs. rlm_wait blocks the session executor at envs/repl_env/managed_children.py:297, while server cleanup queues env.close() on that same executor at src/openenv/core/env_server/http_server.py:516. An active wait therefore prevents shutdown from reaching the supervisor.

Reproduction: serve a managed REPLEnvironment through create_app, with shutdown/terminate/kill settings of 0.05/0.05/2 seconds and a 60-second child timeout. Use a scripted model call that blocks and ignores SIGTERM. Execute child = rlm_query('blocked', blocking=False), then rlm_wait(child, timeout_s=30). Send SIGTERM to Uvicorn with --timeout-graceful-shutdown 4. The four-second server grace exceeds the controller's 3.1-second cleanup budget. On Python 3.12, the server exited after 4.12 seconds; four seconds later, ps still showed worker PID 22533 with PPID 1 and state S. The same leak reproduced on Python 3.13. Both runs used macOS. The reproduction fails its process-exit assertion and explicitly cleans up the leaked worker afterward.

The smallest fix is to invoke the managed supervisor's worker cleanup before waiting for the session execution thread to drain. Add a server-level regression that sends SIGTERM during an active result wait and verifies every registered PID has exited. The documented server path needs this integration to satisfy #1478's session-end cleanup contract.

Nonblocking suggestions:

  • envs/repl_env/README.md:140 and docs/source/environments/repl.md: retain the removed vLLM and separate outer/inner-model recipes in a linked example. recursive_backends.py does not provide an equivalent runnable recipe. The remaining quick starts, configuration, reward, and finalization instructions are preserved, and documentation synchronization passes.
  • envs/repl_env/recursive_controller.py:83 and :104: consolidate the repeated controller construction. The supervisor's process ownership, IPC, escalation, and cleanup-failure handling follow the design agreed in #1478; keep that machinery local to this environment.

Validation: 124 existing and managed-child cases passed on Python 3.13. On Python 3.12, 123 passed together and the async case passed after installing pytest-asyncio. The 60 managed cases ran from a temporary copy with only the Linux platform skip removed. Independent probes passed for reset, close, all finalization paths, SIGTERM-resistant workers, a nested parent killed mid-wait, stale handles, concurrent-session isolation, and configured limits. Those successful cleanup cases left no PID entries in ps. Changed-file import sorting, formatting, lint, and docs synchronization pass. The repository lint hook reports two unchanged import-sort findings and 44 unchanged formatting findings.

Bugbot's final-answer-loss finding is resolved at envs/repl_env/server/repl_environment.py:562; both answer-preservation regressions pass. There are no unresolved Bugbot threads. No additional simulation controls are exposed through the new helpers.

Reproduction: review_server_app.py and shutdown_wait_probe.py
# review_server_app.py
import os
from pathlib import Path
from uuid import uuid4

from openenv.core.env_server import create_app
from repl_env.models import REPLAction, REPLObservation, REPLState
from repl_env.server.repl_environment import REPLEnvironment

from lifecycle_probe import controller


def make_environment():
    directory = Path(os.environ['PROBE_DIR']) / uuid4().hex
    return REPLEnvironment(recursive_controller=controller(directory, per_child_timeout_s=60))


app = create_app(make_environment, REPLAction, REPLObservation, state_cls=REPLState, max_concurrent_envs=4)
# shutdown_wait_probe.py
"""Reproduce shutdown while a live session is blocked collecting a child."""

import asyncio
import json
import os
import signal
import socket
import subprocess
import sys
import time
from pathlib import Path

import httpx
import websockets

from lifecycle_probe import ROOT, ps_assert_gone, wait_ready


async def main():
    directory = ROOT / 'shutdown-active-wait' / str(time.monotonic_ns())
    directory.mkdir(parents=True)
    with socket.socket() as sock:
        sock.bind(('127.0.0.1', 0))
        port = sock.getsockname()[1]
    log = (directory / 'server.log').open('w')
    process = subprocess.Popen(
        [sys.executable, '-m', 'uvicorn', 'review_server_app:app', '--host', '127.0.0.1', '--port', str(port), '--log-level', 'warning', '--timeout-graceful-shutdown', '4'],
        env=dict(os.environ, PROBE_DIR=str(directory), ENABLE_WEB_INTERFACE='false'),
        stdout=log, stderr=log,
    )
    pids = []
    try:
        async with httpx.AsyncClient() as client:
            for _ in range(200):
                assert process.poll() is None
                try:
                    if (await client.get(f'http://127.0.0.1:{port}/health')).status_code == 200:
                        break
                except httpx.RequestError:
                    pass
                await asyncio.sleep(0.1)
            else:
                raise AssertionError('Server failed to start')

        ws = await websockets.connect(f'ws://127.0.0.1:{port}/ws')
        await ws.send(json.dumps({'type': 'reset', 'data': {}}))
        assert json.loads(await ws.recv())['type'] == 'observation'
        await ws.send(json.dumps({'type': 'step', 'data': {'code': "child = rlm_query('PROBE_IGNORE_TERM', blocking=False)"}}))
        assert json.loads(await ws.recv())['data']['observation']['result']['success']
        pids = await asyncio.to_thread(wait_ready, directory)
        await ws.send(json.dumps({'type': 'step', 'data': {'code': 'rlm_wait(child, timeout_s=30)'}}))
        await asyncio.sleep(0.25)
        started = time.monotonic()
        process.terminate()
        try:
            code = await asyncio.to_thread(process.wait, 8)
            outcome = {'server_exitcode': code}
        except subprocess.TimeoutExpired:
            snapshot = subprocess.run(['ps', '-o', 'pid=,ppid=,stat=,command=', '-p', ','.join(map(str, [process.pid] + pids))], text=True, capture_output=True)
            outcome = {'server_still_running': True, 'ps_after_5s': snapshot.stdout.strip()}
        outcome.update({'elapsed_s': round(time.monotonic() - started, 2), 'worker_pids': pids, 'directory': str(directory)})
        print(json.dumps(outcome), flush=True)
        if process.poll() is not None:
            # Allow more than the entire configured cleanup budget to elapse.
            await asyncio.sleep(4)
        if process.poll() is None:
            for path in directory.rglob('*.ready'):
                (path.parent / 'release').touch()
            await asyncio.to_thread(process.wait, 10)
        await ws.close()
        ps_assert_gone('server SIGTERM during active wait, four seconds after exit', pids)
    finally:
        for path in directory.rglob('*.ready'):
            (path.parent / 'release').touch()
        if process.poll() is None:
            try:
                await asyncio.to_thread(process.wait, 5)
            except subprocess.TimeoutExpired:
                process.kill()
                await asyncio.to_thread(process.wait, 5)
        for pid in pids:
            try:
                os.kill(pid, signal.SIGKILL)
            except ProcessLookupError:
                pass
        log.close()


if __name__ == '__main__':
    asyncio.run(main())

This branch has not been deployed

No deployments
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