Skip to content

fix(core): isolate component runtimes and preserve rollout identity - #3591

Merged
JeffPengCoder merged 15 commits into
NVIDIA-NeMo:mainfrom
JeffPengCoder:feature/gym-core-runtime-contracts
Oct 21, 2026
Merged

JeffPengCoder merged 15 commits into
NVIDIA-NeMo:mainfrom
JeffPengCoder:feature/gym-core-runtime-contracts

Conversation

@JeffPengCoder

@JeffPengCoder JeffPengCoder commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

Separate the shared infrastructure changes inherited by #2554 and #3140 from OSWorld-specific behavior. The OSWorld/AgentENV companion #3592 and its dependency-metadata fix #3649 have merged. This branch now includes upstream main d96b33770 through normal merges; the original feature history is preserved.

Split (October 21). As suggested in review, the sandbox-provider changes moved to #4822 and the shared aiohttp lifecycle to #4823. This PR keeps only the component runtime policy, rollout identity and the vLLM transport-log context; none of it uses the moved code. Running OSWorld on AgentEnv needs #4822 as well, because main's E2B provider has no endpoint().

Synchronization with current main

The October 8 merge preserves upstream behavior rather than restoring older implementations:

Scope and compatibility

There is no main-relative diff under benchmarks/osworld, resources_servers/osworld, or responses_api_agents/osworld_agent. Their current code is inherited from main. No task definitions, prompts, history-policy configuration, rewards, sampling parameters or training recipes are changed by this synchronization.

#3649 already fixed OSWorld's Python 3.13/NumPy dependency conflict at the package-metadata source. This PR does not restore NumPy overrides. Generic role-local uv.toml handling remains relevant for CPU Torch selection and isolation from enclosing training workspaces; parent-sensitive packages keep the parent runtime's authority.

Validation

October 21: review fixes

Commits 513300d77, dc9943eda, 2dcb8d9a2 and 39dfe1c57 address the review, and 2d8d8e521 moves the provider and HTTP-lifecycle files to #4822 and #4823. They were tested on macOS ARM64 with Python 3.13.14 and the same dependencies as below (aiohttp 3.14.4, yarl 1.25.1, FastAPI 0.135.2, real E2B SDK 2.46.0):

  • The core unit suite plus the vLLM, simple-agent, compaction-agent and OSWorld tests on head 2d8d8e521: 10,537 passed. No test that passes on main d96b33770 fails here; the remaining failures are identical on main and specific to this macOS machine. uv lock --check --offline passed.
  • New or changed tests cover:
    • both component-root layouts; the wheel-install case fails without the guard;
    • index-derived capture keys for legacy rows, including a resumed attempt;
    • dispatch logging only with request debugging enabled;
    • the transport log recording the post-override sampling values.
  • pre-commit run on the changed files: passed.

Together with #4822 and #4823, which carry the measured head's provider and HTTP-lifecycle changes unchanged, these fixes do not change the OSWorld path measured below:

  • In a source checkout, the component-root guard keeps the previous behavior.
  • Rollout collection no longer adds a _ng_rollout_id field and logs dispatches only on request.
  • The vLLM change removes a print.
  • OSWorld does not use the legacy compaction session.

October 20 synchronization: OSWorld score reproduction

The synchronized head 6055f9ea3 (main d96b33770 merged; #3139 is in main as 415ec2989) reproduces the OSWorld score: no degradation.

Setup. One run on the OSWorld 361-task set (the 369-task suite without the 8 Google Drive tasks), with the same settings as the #3139 score reproduction. Model: Nemotron-3-Nano-Omni-30B-A3B-Reasoning-BF16 (HF revision 24e67ea0), served by vLLM 0.23.0 with TP8 on 8×H100. Settings: fixed-3 image history (the benchmark default), 100 steps, temperature 0.6, top_p 0.95, 16,384 max output tokens. Desktops ran on an AgentEnv (E2B-compatible) deployment with task concurrency 128. Of the score-defining inputs, only the Gym source changed. That head also contained the provider and HTTP-lifecycle changes now in #4822 and #4823.

Run Gym source Raw SR Binary SR
this PR, synchronized 6055f9ea3 48.13% 166/361 (45.98%)
#3139 reproduction, PR arm main d4c0f86c7 + #3591 + #3139 46.62% 159/361 (44.04%)
#3139 reproduction, control arm main d4c0f86c7 + #3591 44.61% 154/361 (42.66%)
  • Difference. Against the PR arm, Δ raw is +1.51 pp, with a paired 95% bootstrap interval of [−2.56, +5.58] pp. For binary SR, 31 tasks succeeded only in this run and 24 only in the PR arm (McNemar exact p = 0.42). Against the control arm, Δ raw is +3.52 pp, interval [−0.78, +7.89] pp; binary 38 vs. 26 (p = 0.17). Both intervals contain 0. Repeated identical runs have differed by 1.2–2.8 pp.
  • Coverage. All 361 tasks were scored: none masked, none missing. One task failed in environment setup, before any model call, on a transient download of a task setup file. It was re-run alone against the same model server and scored 1.0. The other 360 rows were not re-run.
  • Same policy and contract. All 361 tasks have the same history_policy_id (osworld-history-policy-d171b7730c73e7c384ab2b25) and agent_contract_id (osworld-agent-contract-cb1698f34a75d3dbf5fd5063) as both feat(osworld): add sink-window history policy #3139 arms.
  • Terminations. 260 agent_done, 89 at the 100-step limit, 5 agent_fail and 7 model_response_unparseable. Infeasible tasks score only on an explicit model FAIL: 3 of the 29 scored, all through agent_fail; the 13 that reached the step limit scored 0.
  • Not exercised here. No context overflow occurred, so the shrink path was not exercised. The opt-in sink-window policy was not run.

Deployment notes, with no score effect:

October 9 synchronization

Merge commit 4547ad10ae5f1c0c78d8122902fb33042babcea7 includes main 8908cb24d. The only textual merge conflict was uv.lock: the resolution preserves both e2b>=2.46.0,<3.0.0 and upstream's fastapi!=0.142.*. No package versions differ from upstream main's lockfile, and no changes from #4128 or #3368 were merged.

  • 2,804 passed, zero skipped, one platform-specific test deselected on macOS ARM64 / Python 3.13.14.
  • Final regression dependencies: aiohttp 3.14.4, yarl 1.25.1, FastAPI 0.135.2, and real E2B SDK 2.46.0. HTTP dependencies were resolved into a temporary test-only directory; existing training environments were not modified.
  • The selection below was extended with test_cli_endpoint_readiness.py, test_ray_opt_out.py, test_responses_converter.py, test_responses_api_model_streaming.py, and tests/unit_tests/h2_ping_sidecar.
  • uv lock --offline, uv lock --check --offline, source/lock constraint and version-boundary checks, pre-commit run --all-files, and the main-relative git diff --check: passed.
  • The deselected procfs test is the same previously reproduced macOS/main failure described below. Full repository coverage, the eight-shard server suite, and live GPU/VM rollouts were not run for this synchronization.

October 8 validation

Against the final synchronized code, on macOS ARM64 / Python 3.13.14:

  • 2,235 passed, zero skipped; one platform-specific test deselected in the final run. The full selection was also run before the final non-overlapping upstream update: its sole failure, test_memory_profiler_uses_spawned_server_metadata, assumes Linux procfs. It fails identically on a separate, unmodified checkout of main 64a734ed7; no platform workaround was added to this PR.
  • The focused suites cover Core/CLI, generated installer subprocesses, Docker/OpenSandbox/E2B, rollout collection/correlation, episode/session APIs, vLLM, compaction, and OSWorld agent/resources/benchmark interfaces.
  • Optional E2B 2.46.0 was supplied in an isolated temporary dependency directory, exercising the real SDK API/call-shape and shared local HTTP tests without live sandbox credentials. Existing training environments were not modified.
  • pre-commit run --all-files, uv lock --check --offline, and the main-relative git diff --check: passed.

Test selection, with the development and optional E2B dependencies available:

NEMO_GYM_EXTRA_ROOTS="$PWD" PYTHONPATH="$PWD" RAY_TMPDIR=/tmp python -m pytest \
  tests/unit_tests/test_{base_resources_server,cli,cli_setup_command,cli_server_launch,global_config,docker_provider,e2b_endpoint,e2b_provider,e2b_sdk_transport_fallback,opensandbox_provider,opensandbox_http_transport,rollout_collection,rollout_correlation,server_utils,episode_types,environment_server,agent_sessions,resources_sessions,sandbox_access,tool_access}.py \
  responses_api_models/vllm_model/tests \
  responses_api_agents/simple_agent_with_compaction/tests/test_compaction.py \
  responses_api_agents/osworld_agent/tests resources_servers/osworld/tests benchmarks/osworld/tests \
  --import-mode=importlib -q -o addopts='' -o log_cli=false \
  -k 'not test_memory_profiler_uses_spawned_server_metadata'

This is full-scope under the repository CI classifier. The complete repository coverage gate and eight-shard server suite were not run locally and still require repository CI.

Rollout validation scope

The October 8 and 9 synchronizations were validated without GPU rollouts, live sandboxes/VMs or benchmarks. The October 20 synchronization was validated by the 361-task OSWorld score reproduction above, and the October 21 review fixes by unit tests; those fixes do not change the path that reproduction measured. No GRPO or training run was launched, so none of these checks claims full training completion. No internal credentials, deployment details or private run links are included.

Extract shared support from JeffPengCoder/Gym 360b0df. Include E2B endpoint/transport and vLLM model changes; retain upstream GymAiohttpTransport while preserving the no-config Ray worker fallback. Keep OSWorld tasks, agents and resources out of this PR.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>
@JeffPengCoder
JeffPengCoder requested a review from a team as a code owner September 22, 2026 04:08
@JeffPengCoder JeffPengCoder added feature New capabilities, enhancements, or enablement work area:core Shared APIs, servers, telemetry, health, and registries needs-review PR is ready for code review and waiting on a reviewer labels Sep 22, 2026
@copy-pr-bot

copy-pr-bot Bot commented Sep 22, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@yaoyu-33 yaoyu-33 added the complexity:high Cross-component or shared-contract change with a large review surface label Sep 22, 2026
bxyu-nvidia pushed a commit that referenced this pull request Sep 22, 2026
…3592)

## What does this PR do?

Extract the OSWorld-owned changes from #2554 and #3140 into one
environment-focused PR. Both original PRs and their branches remain
unchanged. This branch is based on upstream `6bbc05ed9`; the
implementation comes from the AgentENV benchmark source `360b0df13`.

- Exact multi-turn evidence, logical rollout identity,
fixed-window/hysteresis history policies, and explicit train/eval
contracts.
- Runner-owned episode limits and termination facts, evaluator-owned
rewards, and separate admission decisions. Canonical OSWorld tasks and
evaluator semantics are preserved.
- Stateful OSWorld resources server, remote environment support, runtime
declarations, benchmark preparation, and regression tests.
- AgentENV desktop adaptation through Gym's E2B provider: prebuilt
templates, service-port forwarding, configurable noVNC port, and bounded
cleanup after failed startup.

Changes are limited to `benchmarks/osworld`, the relevant `examples`
config, `resources_servers/osworld`, and
`responses_api_agents/osworld_agent`. OSWorld-specific dependency-policy
tests move into the agent suite.

### Companion and merge dependency

The companion **Gym Core runtime-contracts PR #3591** owns the generic
installer, rollout collection, HTTP lifecycle, sandbox providers, and
shared `vllm_model` changes. E2B gateway/hostname endpoint resolution
and SDK transport integration belong there because they serve any E2B
consumer, not just OSWorld.

The two diffs have no overlapping files and both target `main`. This is
review/scope separation, not a claim of runtime independence: **the
complete AgentENV path requires the companion Core PR**, particularly
`E2BProvider.endpoint()`. The Core PR should land first for complete
AgentENV enablement.

## Validation

- OSWorld agent/resources/benchmark suites: **353 passed**.
- Combined OSWorld + Core tree: **1,425 passed, 2 skipped** across both
component suites and focused core, sandbox, transport, model, and
compaction regressions. This includes local HTTP connection-sharing
tests.
- `pre-commit run --all-files`, `uv lock --check --offline`, and `git
diff --check`: passed.
- Both skips require the optional E2B SDK, absent locally: real-SDK
call-shape introspection and E2B/OpenSandbox cross-provider connection
sharing. Other local HTTP-sharing fixtures passed. GitHub lint,
copyright, DCO and secret detection passed.
- Commands for the environment suite:

```bash
NEMO_GYM_EXTRA_ROOTS="$PWD" PYTHONPATH=. python -m pytest \
  responses_api_agents/osworld_agent/tests resources_servers/osworld/tests \
  benchmarks/osworld/tests --import-mode=importlib -q -o addopts='' -o log_cli=false
```

An exhaustive Git-object reconstruction audit accounts for all **92**
predecessor change paths: **80 byte-identical**, six clean
upstream/source merges, two E2B transport/test conflict adaptations,
three dependency/test-organization updates, and one comment-only
false-positive annotation on the golden-prompt SHA256. The hash and
assertion are unchanged. No unaccounted paths; unrelated upstream files
and original executable modes are preserved. Runtime-policy assertions
for `vllm_model` remain in the companion PR rather than making the
OSWorld test suite own them.

## Rollouts

The recorded predecessor completed **361/361** canonical OSWorld tasks
on AgentENV: **46.9182% raw reward**, **159/361 binary successes**, no
missing/duplicate/masked rows. It used Omni v3, fixed-three-image
history, 100 steps, temperature 0.60, and top_p 0.95. Four model-format
terminations were evaluator-scored; this is not a claim of zero format
failures.

The actual CW-DFW run checkout was rechecked:
`360b0df131de7671e2a6e243f21c8150a061c5ce`, clean. [Recipe and run
evidence](https://gitlab-master.nvidia.com/jepeng/rl/-/blob/36462b79e257cf054c67228d0942c069af29deee/osworld/recipes/benchmarking/nano-omni-osworld-bench/models/nano-omni-v3/cw-dfw-h100-x86_64/agentenv-canonical-s100/README.md).

**No fresh GPU rollout or full 361-task evaluation was run for this
split.** These scores belong to the predecessor; they are not a new
benchmark claim for the split on newer main. No new environment behavior
was intentionally introduced by the split.

## Compatibility and benchmark impact

The task definitions, evaluator, prompts, parser, history-policy
implementation, and exact-trace implementation are unchanged from the
tested source. Training's hysteresis policy remains configurable; the
recorded benchmark's fixed-3 setting is not imposed on training. The
obsolete `httpx-aiohttp` requirement is removed because newer upstream
owns `GymAiohttpTransport`; Core preserves the no-config worker
guard/fallback. Other agent implementations are not modified here.

The automatic Claude review currently fails while obtaining its OIDC
token (`ACTIONS_ID_TOKEN_REQUEST_URL` is unavailable). This is an
upstream workflow permission issue, not a code-review finding; no CI
configuration is changed in this PR.

---------

Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
@github-actions github-actions Bot added the sla:triage-overdue Review assignment is over the one-business-day SLA label Sep 23, 2026
cmunley1 pushed a commit that referenced this pull request Sep 23, 2026
## What does this PR do?

Fix the OSWorld agent's Python 3.13 dependency-resolution failure by
correcting the OSWorld package metadata at its source, rather than
adding another NumPy override or changing Gym's shared installer.

- Update both OSWorld consumers to
[JeffPengCoder/OSWorld@f32ab2b7](JeffPengCoder/OSWorld@f32ab2b),
now published on `nv-gym`.
- Remove the redundant NumPy overrides from the agent and resources
server. Their normal `numpy>=2.1,<2.5` requirements remain unchanged.
- Update revision documentation and dependency-policy tests. No Gym
Core, CI workflow, root dependency, or other-agent changes.

The fork commit synchronizes `pyproject.toml`, `requirements.txt`, and
`setup.py`: Python <3.13 retains NumPy 1.26/OpenCV 4.8; Python >=3.13
declares NumPy >=2.1,<3 with headless OpenCV >=4.10.0.84,<5. It adds 18
marker/ABI-pair regression cases. These are compatibility ranges, not
experimental version locks.

## Why is this needed?

The [scheduled server-suite
failure](https://github.com/NVIDIA-NeMo/Gym/actions/runs/35816620013/job/107039403224)
occurs before OSWorld tests run:

```text
osworld==0.1.0 depends on numpy<2
the agent requires numpy>=2.1,<2.5
No solution found
```

#3592 introduced the agent's NumPy 2 requirement while its pinned
OSWorld archive still declared `numpy<2`. `osworld==0.1.0` is the fork
package's metadata version; CI already downloads our explicit GitHub
archive, not an unrelated PyPI package. The later README-only #3631 did
not introduce this regression.

The role's `uv.toml` NumPy override depended on explicit config loading
in the still-open #3591. Current main's generated requirements install
does not load that role config in this checkout layout. Fixing the fork
metadata removes this particular dependency on #3591. Its other
runtime/embedded-Ray changes remain separate and are not claimed fixed
here.

## Validation

- Reproduced the original NumPy resolver conflict on the old archive.
- Real Linux x86_64 / Python 3.13.14 dependency resolution passed for
both agent and resources server using the published new archive, without
NumPy overrides.
- Gym's actual generated agent install command also passed with only
`--dry-run` and a Linux target added, in a fresh temporary venv and
including the parent's Ray 2.56.1/OpenAI 2.44.0 requirements: **350
packages resolved**.
- OSWorld agent, resources-server, and benchmark regression suites:
**353 passed**, run in the existing development venv.
- Fork regression suite: **18 passed**; it failed against the old
metadata. Built the fork wheel and inspected the emitted conditional
`Requires-Dist` entries.
- Isolated CPython 3.13.14 NumPy/OpenCV imports and PNG encode/decode
passed for NumPy 2.1.0 and 2.4.6, both with OpenCV 4.10.0.84, on macOS
ARM64.
- `pre-commit run --all-files` and `git diff --check`: passed.

The full eight-shard server suite and a fresh Linux managed-venv
installation are left to CI; resolver checks are not a claim that those
runs completed locally.

## Compatibility and benchmark impact

The fork's `desktop_env`, `mm_agents`, and `evaluation_examples` Git
tree IDs are byte-identical to the previous pinned revision. Tasks,
evaluators, prompts, parser, history policy, and exact-trace behavior
are unchanged. Existing Gym role runtime ranges are also unchanged; only
their upstream metadata conflict is removed.

Model rollout: **N/A for this metadata-only fix**. No new model score or
training result is claimed. Other stale dependency overrides are
intentionally retained; this is not a general OSWorld packaging
migration.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
@Glorf

Glorf commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

/ok to test 47f3fcf

@nemo-automation-bot
nemo-automation-bot Bot requested review from a team September 23, 2026 13:12
Glorf added a commit that referenced this pull request Sep 28, 2026
## Summary

Add an opt-in `sink_window` history policy to the OSWorld Nemotron
adapter: retain the earliest screenshots alongside a recent-image
window, with intervening turns represented as text in chronological
order.

This branch includes upstream `main` at `d4c0f86c7` through a normal
merge, including the NumPy metadata fix #3649, CI preflight update
#3658, and sandbox lifecycle telemetry #3633. The OSWorld foundation
from #2554/#3140 was consolidated and merged through #3592, so this PR
no longer carries the old exact-trace/Core stack or depends on #2554. It
does not require #3591. No separate issue is needed for this follow-up
policy extension.

## Changes

- Add configurable sink, low-water and high-water counts. Both
watermarks include the sink; equal watermarks slide, while unequal
watermarks provide hysteresis.
- Represent non-contiguous image selections with intervals and render
folded text in chronological order.
- Distinguish context overflow, transport failure, truncated output,
empty output and unparseable output; retain
`agent_outcome_family=model_response_invalid` for compatibility.
- On context overflow, reduce the recent-image window within the
existing retry budget, preserve the sink/current observation, record
actual shrink events, and stop when further reduction is impossible.
- Preserve main's immediate model-timeout propagation. Report the
terminal attempt's completion fact so an earlier malformed sample cannot
hide a subsequent transport failure.

The latest synchronization also annotates six deterministic SHA-256
prompt goldens that the previous secrets-detector run falsely identified
as credentials. Only those literal lines are annotated; the digest
values, assertions, repository scanner configuration, and baseline are
unchanged.

## Design and compatibility

History policy selects image/text turns; the adapter owns model protocol
rendering and parsing. The runner owns horizon and runtime failure
handling, the evaluator owns reward, and runtime admission owns masking.
This PR does not change those ownership boundaries or add a
training/evaluation switch.

Fixed-window and hysteresis policy IDs and normal-path rendered prompts
remain covered by deterministic golden tests, including fixed-3 and
3-10-3. Sink-window is opt-in, not a new default or a claim of improved
benchmark score. Context-overflow recovery and more precise failure
outcomes are intentional behavioral changes and may affect failed-run
outcomes. The legacy scalar `image_window_start` accessor rejects
non-contiguous plans; consumers should use `image_intervals` or per-turn
decisions.

Scope relative to current main remains limited to
`responses_api_agents/osworld_agent/`: two implementation files,
documentation, and tests. No Gym Core, dependency, deployment,
sandbox-provider or model-server changes are included in the PR diff.

## Validation

Re-run on the final main-synchronized tree:

- **440 passed, zero skipped** across OSWorld agent, resource-server and
benchmark tests; one non-failing Starlette deprecation warning.
- `pre-commit run --all-files`: passed.
- `uv lock --check --offline`: passed.
- `git diff --check`: passed.
- The same `detect-secrets==1.5.0` hook and checked-in baseline used by
CI passed on all nine PR-scoped files after the inline false-positive
annotations.
- Existing model-timeout propagation, runner admission, prompt-parity,
sink selection, overflow recovery and policy-identity regressions
passed. The unchanged-overflow retry and mixed parse/transport
completion bugs had been reproduced before their earlier fixes.

Test invocation, using an existing compatible development environment
from this checkout:

```bash
export PYTHONPATH="$PWD"
export NEMO_GYM_EXTRA_ROOTS="$PWD"
python -m pytest \
  responses_api_agents/osworld_agent/tests \
  resources_servers/osworld/tests \
  benchmarks/osworld/tests \
  --import-mode=importlib -o addopts='' -o log_cli=false -q
pre-commit run --all-files
```

The explicit source root prevents an older editable checkout from
shadowing the branch under test; importlib mode avoids the shared
`osworld` test-package name collision. These checks are local evidence,
not a claim that the upstream CI run has completed.

## Score reproduction

OSWorld 361 tasks, Nemotron-3-Nano-Omni v3 (`24e67ea0`), fixed-3
history, 100 steps, one A/B run per arm. Control = main `d4c0f86c7` +
#3591 (needed for the E2B `endpoint()` on AgentEnv); treatment = control
+ this PR. Raw SR 44.61% → 46.62% (Δ +2.01 pp, paired 95% CI [−2.12,
+6.07] pp); binary 154 → 159 of 361 (McNemar p = 0.59); 0 masked and 0
missing in both arms; identical history-policy and agent-contract IDs.
No degradation. The context-overflow shrink path was not triggered in
either run.

---------

Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Co-authored-by: Michał Bień <mbien@nvidia.com>
Merge upstream main through 64a734e. Preserve environment-based config transport, bounded rollout admission, episode identity and HTTP retry/telemetry controls. Copy legacy dispatch payloads only after admission; keep role-local runtime policy and E2B embedded-worker compatibility. Add regressions for the merged contracts.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
@aroshanghias-nvd

Copy link
Copy Markdown
Contributor

This overlaps with our context compaction refactor in #3368. We remove ContextCompactionSession, so the final_policy_decision fix won’t carry over directly; the replacement checks guards before generation and acknowledges history after accepting the response. The rest of your infrastructure changes look largely independent. Rather than stack the whole PR, let’s coordinate merge order and preserve both sets of behavior in rollout collection and the model proxy.

@oyilmaz-nvidia
oyilmaz-nvidia added this pull request to the merge queue Oct 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/39656066230

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 21, 2026
…pin conflicts

- A server's .python-version now selects its interpreter only when it excludes the
  parent's Python. A request that admits it (such as >=3.13.14,<3.14, 3.13 or
  cpython@3.13) builds the venv with the parent's exact version, because Ray rejects
  processes on different patch releases. Variant and pre-release requests
  (3.13+freethreaded, 3.13.14+debug, 3.13.14rc1) and requests for other implementations
  are passed to uv unchanged. Like uv, the first non-comment line is the request.
- Only the parent's constraints (the Ray pin) are passed as uv overrides, through process
  substitution like the existing -c flag. head_server_deps such as openai stay ordinary
  requirements, so a server pin that conflicts with them fails setup instead of being
  replaced silently.
- Drop the environment-identity file: nothing read it, so it could not detect a stale venv,
  and its tests passed whatever it contained.
- Replace the vacuous vllm_model marker-file test with one that checks the real policy, and
  document per-server .python-version/uv.toml and NEMO_GYM_ALLOWED_COMPONENT_ROOTS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
@yaoyu-33

Copy link
Copy Markdown
Contributor

@JeffPengCoder I pushed c004893 to this branch. It addresses the remaining runtime-policy points from review rather than leaving them as follow-ups. It is a fast-forward on top of your e3ea83a92, so nothing of yours was rewritten. Feel free to adjust or revert any part of it.

What changed (nemo_gym/cli/setup_command.py, its tests, the vLLM runtime test, and reference/configuration.mdx):

  1. The server Python stays on the parent's patch release.
    • Problem: a component .python-version range such as >=3.13.14,<3.14 replaced the parent-exact python_version, which global_config.py pins because Ray checks Python at patch level. That let uv pick a different 3.13.x for osworld_agent, which is ray_enabled.
    • New rule: when the request admits the parent's Python, the venv uses the parent's exact version. Only a request that excludes it is passed to uv as written.
    • The request may be a range, 3.13, or cpython@3.13. Variant, pre-release and other-implementation requests (3.13+freethreaded, 3.13.14+debug, pypy@3.10) still go to uv unchanged.
    • As in uv, the first line that is not a comment is used.
  2. Only the Ray pin is an override.
    • Before: head_server_deps (the parent's openai, telemetry and plugin packages) were written to an overrides file, which silently replaced a server's own conflicting pin. A conflict used to fail the install.
    • Now: only head_server_constraints (the Ray pin) are passed as --overrides, through process substitution like the existing -c. Your fix for stale transitive Ray pins is unchanged. Parent dependencies are plain requirements again, so conflicts fail loudly.
  3. The environment-identity file is removed.
    • Nothing read .nemo-gym-environment-identity, so it could not detect a stale venv.
    • The tests that covered it used skip_venv_if_present=False, so they passed whatever it contained.
    • If you want stale-venv detection, it would need to be wired into the --skip-if-ready path. That seemed better as its own change.
  4. Docs. Per-server .python-version, uv.toml, the parent-pin rules and NEMO_GYM_ALLOWED_COMPONENT_ROOTS are documented under "Server Virtual Environments".
  5. The vLLM runtime test is replaced. The marker-file test asserted that files that never existed were absent. The new test checks through setup_env_command that the model venv is built with the parent's Python, and keeps the <3.14 check.

Validation on macOS ARM64, Python 3.13.14, uv 0.8.18:

  • Unit tests:
    • test_cli_setup_command.py, test_cli.py, test_global_config.py, test_rollout_collection.py and vllm_model/tests: 1,520 passed.
    • The only failure is test_memory_profiler_uses_spawned_server_metadata, the macOS procfs failure that also happens on main.
    • With the old setup_command.py, 21 of the setup-command tests fail.
  • pre-commit run on the changed files: passed.
  • Real uv runs of the generated commands:
    • Parent 3.13.14 with >=3.13.14,<3.14 builds the venv on 3.13.14.
    • Ray is absent when the server does not need it.
    • A server that requires Ray gets 2.58.0.
    • A server pinning ray==2.54.0 is overridden to 2.58.0.
    • A server pinning packaging==25.0 against a parent packaging==24.2 fails the install.

@yaoyu-33 yaoyu-33 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.

Re-approving at c004893. My earlier approval was dismissed when I pushed the runtime-policy follow-up; that commit and its validation are described in the comment above. The OSWorld-only x-nemo-gym-log-* follow-up in vllm_model still stands as a separate change.

@yaoyu-33

Copy link
Copy Markdown
Contributor

/ok to test c004893

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/39691150761

Merged via the queue into NVIDIA-NeMo:main with commit 8af4df7 Oct 21, 2026
49 of 56 checks passed
@JeffPengCoder
JeffPengCoder deleted the feature/gym-core-runtime-contracts branch October 21, 2026 23:29
JeffPengCoder added a commit to JeffPengCoder/Gym that referenced this pull request Oct 22, 2026
Main's OSWorld configs already set create.transport_retries, create.transport_retry_delay_s (Docker) and connection.port_routing (E2B), but the provider configs do not define them, so both providers fail to build from those configs. Split from NVIDIA-NeMo#3591 so the provider contracts can land first.

- Docker: a remote Docker transport can lose the docker run response after the daemon created the container. Retry with the same fixed name, and after a timeout, transport error or same-name conflict, inspect that exact name and reuse it only when the sandbox ownership label and the requested image both match. An unrelated container is never removed.
- E2B: endpoint() resolves a declared service port. connection.port_routing selects hostname routing (one DNS name per port) or gateway routing (one origin; the SDK's per-sandbox headers plus E2b-Sandbox-Port); auto picks gateway when a sandbox URL is configured. Gateway routing needs ConnectionConfig.sandbox_headers, so the SDK range becomes e2b>=2.46.0,<3.0.0 in the dependency, installation hint, README and example.
- E2B transport: borrow Gym's aiohttp pool only when the process already has Gym's global config; otherwise keep the SDK's own transport, so a Ray worker never reaches Hydra's argument parser before creating a sandbox.
- OpenSandbox: endpoint() reports an actionable error for SDKs without Sandbox.get_endpoint and tolerates responses without endpoint headers.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>
yaoyu-33 added a commit to yaoyu-33/Gym that referenced this pull request Oct 22, 2026
Resolve the test import conflict: main's NVIDIA-NeMo#3591 imports BaseRunRequest next to
this branch's BaseVerifyRequest.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
yaoyu-33 pushed a commit to yaoyu-33/Gym that referenced this pull request Oct 26, 2026
…nts (NVIDIA-NeMo#4822)

Main's OSWorld configs already set create.transport_retries, create.transport_retry_delay_s (Docker) and connection.port_routing (E2B), but the provider configs do not define them, so both providers fail to build from those configs. Split from NVIDIA-NeMo#3591 so the provider contracts can land first.

- Docker: a remote Docker transport can lose the docker run response after the daemon created the container. Retry with the same fixed name, and after a timeout, transport error or same-name conflict, inspect that exact name and reuse it only when the sandbox ownership label and the requested image both match. An unrelated container is never removed.
- E2B: endpoint() resolves a declared service port. connection.port_routing selects hostname routing (one DNS name per port) or gateway routing (one origin; the SDK's per-sandbox headers plus E2b-Sandbox-Port); auto picks gateway when a sandbox URL is configured. Gateway routing needs ConnectionConfig.sandbox_headers, so the SDK range becomes e2b>=2.46.0,<3.0.0 in the dependency, installation hint, README and example.
- E2B transport: borrow Gym's aiohttp pool only when the process already has Gym's global config; otherwise keep the SDK's own transport, so a Ray worker never reaches Hydra's argument parser before creating a sandbox.
- OpenSandbox: endpoint() reports an actionable error for SDKs without Sandbox.get_endpoint and tolerates responses without endpoint headers.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Co-authored-by: Marc Romeyn <marcromeyn@gmail.com>
khazic pushed a commit to khazic/Gym-lao that referenced this pull request Oct 28, 2026
…eMo#4823)

The process-wide data client and the reserved control client were closed at exit with asyncio.run(), i.e. on a new event loop rather than the loop that created them. Record the owner loop and close both clients there: schedule the close when called from the owner loop, hand it to the owner loop when that loop runs in another thread, and run it to completion otherwise. close_global_aiohttp_client() offers the same close to async callers.

Clients also stop dialing wildcard bind addresses: a server bound to 0.0.0.0 or :: is reached at 127.0.0.1 or [::1]. A proxy-aware HTTP client can otherwise send a request for 0.0.0.0 to an external proxy instead of the local server.

Split from NVIDIA-NeMo#3591.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>

This branch was previously deployed

1 inactive deployment
public — c004893a Deployed Oct 21, 2026 by copy-pr-bot[bot] via release / finalize / notify #6458
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core Shared APIs, servers, telemetry, health, and registries complexity:high Cross-component or shared-contract change with a large review surface feature New capabilities, enhancements, or enablement work needs-review PR is ready for code review and waiting on a reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants