Skip to content

fix(v1): detect env-server worker death instead of hanging - #2131

Open
eligotts wants to merge 7 commits into
mainfrom
fix/env-server-death-detection
Open

eligotts wants to merge 7 commits into
mainfrom
fix/env-server-death-detection

Conversation

@eligotts

@eligotts eligotts commented Jul 24, 2026 •

Copy link
Copy Markdown
Contributor

Problem

An env-server process can fail during startup or a pool worker can die while serving. Previously, the parent could keep waiting for an address or a reply until a long timeout, leaving the orchestrator stuck at Loading training environments.

Changes

  • Report single-server and pooled-server readiness only after serving resources are ready and a worker answers a health probe.
  • Poll the child exit code while waiting for its address and include the server log tail in startup errors.
  • Reap dead workers, preserve already-queued replies, and fail only their orphaned in-flight requests.
  • Keep loading workers out of request dispatch until their health probe succeeds.
  • Replace dead workers through a non-fatal spawn path with an independent per-slot recovery budget that resets after readiness.
  • Retry missing slots once per reap interval and back off failed elastic scale-ups independently.

Review follow-ups

  • Addressed Macroscope's replacement-spawn, queued-reply, and elastic-capacity findings.
  • Addressed Bugbot's readiness, failed-spawn, dead-before-reap, recovery-budget, and static-pool findings.
  • Kept the request/reply protocol unchanged.

Verification

  • uv run pytest tests/test_env_server.py tests/v1/test_configs.py tests/test_imports.py — 29 passed
  • uv run ruff check .
  • uv run ruff format --check .
  • uv run ty check verifiers
  • uv run pre-commit run --all-files
  • Spawned-process smoke coverage for early child death, single-server readiness, pooled readiness, log-tail propagation, live/ready-only dispatch, and failed scale-up accounting

Companion orchestrator integration: PrimeIntellect-ai/prime-rl#3139.


Note

Medium Risk
Changes broker startup, worker lifecycle, and eval server spawn semantics on a critical training/eval path; wire protocol is unchanged but readiness timing and failure modes differ from before.

Overview
Fixes cases where a crashed or still-loading env server left clients and orchestrators waiting indefinitely instead of surfacing an error.

Readiness waits for a real worker. Pooled brokers no longer publish their address at socket bind; they wait until a worker answers a startup health probe (and single-process EnvServer reports after serving() is up). Eval’s server path uses shared wait_for_address, which polls the child exit code and includes log tail on failure, replacing blind address_queue.get plus redundant wait_for_server_startup.

Dead workers are handled in the broker. The serve loop periodically reaps exited workers, drains queued replies before retiring sockets, fails orphaned in-flight requests with BaseResponse(success=False, ...), and replaces workers up to MAX_WORKER_RESTARTS. Mid-serve spawns go through _try_spawn_worker so a failed replacement does not take down the broker; requests with no live workers fail immediately.

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

Note

Fix env-server worker death detection to fail fast instead of hanging

  • EnvServerPool now publishes its address only after at least one worker has responded to a health probe, ensuring clients never connect to an unready pool.
  • Dead workers are detected by polling exit codes; their in-flight requests receive explicit error replies instead of hanging indefinitely, and replacement workers are spawned up to MAX_WORKER_RESTARTS times with exponential backoff.
  • wait_for_address in pool.py polls the process exit code concurrently with the address queue, so startup failures surface immediately with a log tail rather than blocking until timeout.
  • server.py delays putting the address on address_queue until the server is fully ready to serve, aligning with the new readiness semantics.
  • Behavioral Change: run_eval_server in runner.py replaces a plain address_queue.get(timeout=600) + wait_for_server_startup with a single await wait_for_address(proc, address_queue) that fails fast on worker death.

Macroscope summarized 40154d6.

An env server that died was invisible to its client. Three gaps, all of which
turned a crash into an unbounded wait:

- The broker reported its address as soon as the frontend socket was bound,
  before any worker had loaded the env. A spawner was told "ready" about a pool
  whose env was still loading — or already dead — so an env that fails to import
  surfaced much later as a rollout that never returns.
- A dead worker was never reaped. Its `active` count never drained, so
  least-busy dispatch elected the corpse for every subsequent request, and its
  in-flight requests went unanswered forever (`EnvClient` awaits rollout replies
  untimed, by design).
- `run_eval_server` read the address queue with a bare 600s timeout, so a server
  that died on startup was reported as a timeout rather than a crash.

The broker now reports its address only once a worker answers `health`, polls
worker exit codes on its serve loop, answers a dead worker's in-flight requests
with an error the client raises on, and replaces the worker up to
MAX_WORKER_RESTARTS before failing requests outright. `wait_for_address` is the
shared queue reader that watches the process's exit code alongside the queue and
quotes the tail of its log file — used here by `run_eval_server`, and by
prime-rl's orchestrator for the servers it spawns.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread verifiers/v1/serve/pool.py Resolved
Comment thread verifiers/v1/serve/pool.py Resolved
… reaping

Two problems in the death handling itself, from review:

- `_replace_worker` called `_spawn_worker` unguarded. A worker death caused by
  memory pressure is exactly when spawning its replacement can fail, and that
  exception escaped the serve loop — killing the broker and stranding every
  other client's in-flight requests. Mid-serve spawns now go through
  `_try_spawn_worker`, which logs and leaves the pool one worker short. Scale-up
  had the same latent bug and takes the same path. Startup spawns still
  propagate: a pool with no workers has nothing to serve.
- Reaping closed a dead worker's socket after relaying only one reply, so a
  worker that answered several requests and then exited had the rest discarded
  and reported to clients as `worker died`. The reaper now drains the socket
  before retiring it. The serve loop still takes one reply per pass, so a chatty
  worker can't starve the frontend.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@eligotts
eligotts marked this pull request as ready for review July 24, 2026 21:00
Comment thread verifiers/v1/serve/pool.py Resolved
@macroscopeapp

macroscopeapp Bot commented Jul 24, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

This PR adds substantial worker death detection and replacement infrastructure with new state management and lifecycle logic. An unresolved review comment identifies a medium-severity bug where exhausted restart budgets permanently reduce pool capacity. Both the scope of changes and the open finding warrant human review.

You can customize Macroscope's approvability policy. Learn more.

A replacement that failed to spawn left the pool with no workers, and an empty
pool has no dead worker left to drive another attempt — so `_reap_dead_workers`
never tried again and every later request was refused with `no live workers`,
with the restart budget still unspent. One transient failure to fork
permanently disabled the pool.

The reaper now retries while budget remains, bounded by its own interval and by
MAX_WORKER_RESTARTS. Once the budget is spent the pool stays empty and fails
requests fast, which is the honest end state: a host that cannot fork is not
something the broker should paper over.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread verifiers/v1/serve/pool.py Resolved
Comment thread verifiers/v1/cli/eval/runner.py Resolved
eligotts pushed a commit to PrimeIntellect-ai/prime-rl that referenced this pull request Jul 24, 2026
An env server that died on startup left the orchestrator waiting: `_spawn` read
the address queue with a bare 600s timeout, so a server that crashed (bad
import, missing credentials, OOM) was reported as
"did not report its address within 600s" — a timeout message for a crash whose
real traceback sat unread in logs/envs/. Nothing watched the server after
startup either, so a mid-run death silently stalled the pipeline: its in-flight
requests never answer and the dispatcher awaits rollout replies untimed.

- `_wait_for_address` polls the child's exit code alongside the queue and quotes
  the tail of its log file, where the child's redirected output landed. The 600s
  timeout now bounds only a genuinely slow start.
- `Envs.check_alive`, called on each main-loop pass, raises if a spawned server
  has exited, so the run fails loudly instead of quietly draining.
- Env servers get a death pipe, so one can no longer outlive the orchestrator
  (SIGKILL included) with its worker pool, sandboxes, and tunnels.
- `Envs.start` registers its shutdown before the first spawn. A failure part-way
  through startup previously leaked the servers already up, and since they are
  non-daemonic, multiprocessing's implicit join at exit wedged the process.
- Drop the unused `Env.shutdown` (`Envs.shutdown` is the only caller path, and
  it terminates with a join rather than a bare terminate).

Deliberately self-contained: no deps/verifiers bump, so this needs no config
migration and no relock. PrimeIntellect-ai/verifiers#2131 fixes the other half
(a pooled server reporting its address before any worker has loaded the env, and
a dead worker never being reaped), and prime-rl picks that up with the next
routine verifiers bump.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eligotts pushed a commit to PrimeIntellect-ai/prime-rl that referenced this pull request Jul 24, 2026
An env server that died on startup left the orchestrator waiting: `_spawn` read
the address queue with a bare 600s timeout, so a server that crashed (bad
import, missing credentials, OOM) was reported as
"did not report its address within 600s" — a timeout message for a crash whose
real traceback sat unread in logs/envs/. Nothing watched the server after
startup either, so a mid-run death silently stalled the pipeline: its in-flight
requests never answer and the dispatcher awaits rollout replies untimed.

- `_spawn` reads the queue through verifiers' `wait_for_address`, which polls the
  child's exit code alongside the queue and quotes the tail of its log file. The
  600s timeout now bounds only a genuinely slow start.
- `Envs.check_alive`, called on each main-loop pass, raises if a spawned server
  has exited, so the run fails loudly instead of quietly draining.
- Env servers get a death pipe, so one can no longer outlive the orchestrator
  (SIGKILL included) with its worker pool, sandboxes, and tunnels.
- `Envs.start` registers its shutdown before the first spawn. A failure part-way
  through startup previously leaked the servers already up, and since they are
  non-daemonic, multiprocessing's implicit join at exit wedged the process.
- Drop the unused `Env.shutdown` (`Envs.shutdown` is the only caller path, and
  it terminates with a join rather than a bare terminate).

Bumps deps/verifiers for `wait_for_address` and the broker-side worker-death
handling it pairs with (PrimeIntellect-ai/verifiers#2131), and relocks for the
env package that bump adds. CI is red until this repo's configs are migrated for
verifiers#2106, which moved `runtime` off the harness onto the agent — a
separate chore this PR deliberately does not carry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread verifiers/v1/serve/pool.py Resolved
Comment thread verifiers/v1/serve/pool.py Resolved
Comment thread verifiers/v1/serve/pool.py Resolved
Comment thread verifiers/v1/serve/pool.py Resolved
Comment thread verifiers/v1/serve/pool.py Resolved
Comment thread verifiers/v1/serve/pool.py Resolved
Comment thread verifiers/v1/serve/pool.py Resolved

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 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.

Reviewed by Cursor Bugbot for commit 40154d6. Configure here.

slots = len(self.workers) + len(self._replacement_queue)
if self.max_workers is not None and slots >= self.max_workers:
return
if self.in_flight >= 0.9 * slots * self.multiplex:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Exhausted slots block elastic recovery

Medium Severity

Exhausted restart entries stay in _replacement_queue forever and are counted in _maybe_scale_up via slots. That permanently lowers effective capacity after transient spawn failures, and with an empty pool _on_request returns before scale-up, so even max_workers=None cannot add a fresh worker once budgets are spent.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 40154d6. Configure here.

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.

1 participant