Repository navigation
repl_env: add managed child queries with bounded cleanup - #1527
jucamohedano wants to merge 6 commits into
Conversation
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.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ 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
left a comment
There was a problem hiding this comment.
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:140anddocs/source/environments/repl.md: retain the removed vLLM and separate outer/inner-model recipes in a linked example.recursive_backends.pydoes 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:83and: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())
Summary
Adds opt-in background child queries to
repl_envthroughrlm_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, andrlm_waitsupport inspection and result collection. A wait timeout does not cancel the child.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
repl_env, with no new runtime dependencies or environment transport changes.Type of Change
Alignment Checklist
.claude/docs/PRINCIPLES.mdand this PR aligns with our principles.claude/docs/INVARIANTS.mdand no invariants are violated/pre-submit-pr(orbash .claude/hooks/lint.shand tests) and addressed all issuesThe prescribed checks were run. The final checkbox remains unchecked because the repository-wide lint hook reports existing upstream findings, detailed below.
RFC Status
repl_env#1478Test 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:
FINAL(...)and answer-dictionary finalization, with execution, child creation, and reset still blocked.With the CI test dependencies installed:
Validation results
These local results cover the current fix and regression tests. GitHub CI will also validate the published head.
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
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_envso parents can keep executing REPL code while recursive subtasks run in supervised worker processes. Blockingrlm_query(prompt)behavior is unchanged;rlm_query(..., blocking=False)returns an episode handle and is wired throughManagedChildConfig,ManagedChildren, andManagedChildBackend, withrlm_status/rlm_result/rlm_waitinjected into the REPL namespace.LocalRLMRunnerandcreate_server_recursive_controllercan enable managed mode (mutually exclusive with a custombackend_factory).LocalREPLEnv/REPLEnvironmentaccept an optionalrecursive_controllerso 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 surfacesChildCleanupErrorand blocks reuse.Docs for the REPL environment are tightened and expanded for managed usage; system prompts note the keyword-only
blockingflag. 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.