Fix ZMQ env timeouts#1355
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
d584e01 to
99f0ba8
Compare
99f0ba8 to
33caa2f
Compare
|
Addressed the review comments in f37b555:
Verification:
|
Amp-Thread-ID: https://ampcode.com/threads/T-019e1cff-d733-71ec-9f18-1189805dcba4 Co-authored-by: Amp <[email protected]>
f37b555 to
64f2b92
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 64f2b92. Configure here.
| """ZMQ-based environment client.""" | ||
|
|
||
| DEFAULT_REQUEST_TIMEOUT = 36_000 # 10h | ||
| DEFAULT_REQUEST_TIMEOUT: float | None = None |
There was a problem hiding this comment.
Default timeout removed, requests can hang forever
High Severity
DEFAULT_REQUEST_TIMEOUT was changed from 36_000 (10 hours) to None, removing the safety-net timeout entirely. The PR description states "with a 24h default," but None means asyncio.wait_for waits indefinitely. Since all callers (EnvClient.run_rollout and run_group) pass timeout=None, and nothing in the codebase overrides DEFAULT_REQUEST_TIMEOUT on instances, every ZMQ request now blocks forever if the server fails to respond.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 64f2b92. Configure here.


Summary
timeout_minuteswhen present, or from hosted-runnerPRIME_EVAL_TIMEOUT_MINUTESwhen running from CLI flagsstop_condition="env_request_timeout"instead of aborting the whole eval, while leaving unrelated TimeoutErrors untouchedA companion platform change is prepared locally to set
PRIME_EVAL_TIMEOUT_MINUTESin hosted eval runner sandboxes from the existingtimeout_minutesfield.Verification
uv run pytest tests/test_eval_cli.py tests/test_env_server.py tests/test_environment_extra.py— 104 passeduv run ruff check verifiers/types.py verifiers/utils/eval_utils.py verifiers/scripts/eval.py tests/test_eval_cli.py tests/test_env_server.py tests/test_environment_extra.py docs/evaluation.md docs/reference.mduv run pytest tests/had 1418 passed, 29 skipped, 3 unrelated failures: missing OpenAI key for toxicity_explanation/wiki_search and unsatisfiable bfcl_v3 dependency on mistralaiNote
Medium Risk
Changes ZMQ env request behavior by removing the 10h hardcoded default timeout, which can alter how long evals wait for env responses (potentially hanging if callers don’t supply a timeout). Scope is small and isolated to the ZMQ client.
Overview
Removes the hardcoded
10hDEFAULT_REQUEST_TIMEOUTinZMQEnvClientand replaces it withNone, making env request timeouts unlimited by default unless an explicittimeoutis passed tosend_request.Reviewed by Cursor Bugbot for commit 64f2b92. Bugbot is set up for automated code reviews on this repo. Configure here.