Skip to content

feat(simple_agent): a resources server can end the episode with X-NeMo-Gym-Episode-Done - #5624

Closed
d-molinari wants to merge 1 commit into
mainfrom
dmolinari/simple-agent-episode-done
Closed

d-molinari wants to merge 1 commit into
mainfrom
dmolinari/simple-agent-episode-done

Conversation

@d-molinari

@d-molinari d-molinari commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Lets a resources server end the simple agent's episode once its task is over.

The simple agent ends an episode only on a turn without tool calls, or at max_steps. A resources server whose task has an explicit end, such as a completion call, cannot stop it there: a policy that keeps calling tools after completing the task runs until the step cap, and each of those calls still reaches the server.

NEMO_GYM_EPISODE_DONE_HEADER (X-NeMo-Gym-Episode-Done, exported by nemo_gym.server_runtime): a resources server sets it to true on a tool response once the episode is over. The simple agent then runs nothing more. Each later call of that turn gets a "Not run" output, so every call in the transcript keeps its output, and no model call follows. The simple agent's README describes the header.

OfficeEnv's resources server (merged in #5009) will set it when a task is completed, in a follow-up once this PR merges.

Validation

  • responses_api_agents/simple_agent/tests/test_app.py: with the header, the call after the one that ended the episode does not run, gets the "Not run" output, and no model call follows; without it, both calls run and the model is called again. With main's agent the header case fails. It passes 5 of 5 runs (Linux, Python 3.13).
  • The simple agent's tests pass, and so do the public API, facade and server_runtime tests, apart from one module that needs cloudpickle, which the venv lacks, as on main. scripts/migrate_imports.py --check . reports the same as on main, and pre-commit's import-linter passes.
  • Gym's core suite (pytest -m "not sandbox") fails only where main does, the same ids in the same venv.
  • pre-commit and detect-secrets pass on the changed files, and the commit is signed off.

Rollouts

N/A: servers that do not set the header behave as before.

Compatibility and benchmark impact

None: the agent stops early only on a response that carries the header, and no server sets it today.

Checklist

  • I have read the contributing guidelines.
  • The change is focused; unrelated "drive-by" edits are tracked as separate issues/PRs.
  • Tests added or updated and pass locally.
  • Documentation added or updated (responses_api_agents/simple_agent/README.md).
  • Pre-commit checks pass locally.
  • New source files include the required Apache-2.0 SPDX header (no new files).
  • All commits have DCO sign-off (git commit -s).

🤖 Generated with Claude Code

@copy-pr-bot

copy-pr-bot Bot commented Nov 4, 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.

@d-molinari
d-molinari marked this pull request as ready for review November 5, 2026 00:46
…o-Gym-Episode-Done

The simple agent ends an episode only on a turn without tool calls or at max_steps. A resources server whose task has
an explicit end (a completion call) could not stop it there: a policy that keeps calling tools after completing runs
until the step cap, and every call it makes still reaches the server.

NEMO_GYM_EPISODE_DONE_HEADER (X-NeMo-Gym-Episode-Done), exported by nemo_gym.server_runtime: a resources server sets
it to "true" on a tool response once the episode is over. After a response carrying it, the simple agent runs nothing
more. The turn's later calls each get a "Not run" output, so every call in the transcript keeps its output, and the
episode ends. Servers that never set the header are unchanged. Simple Agent's README describes the header.

A test that fails on the parent: with the header, the call after the one that ended the episode does not run, gets
the "Not run" output, and no model call follows; without it, both calls run and the model is called again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Daniele Molinari <dmolinari@nvidia.com>

@svcnvidia-nemo-ci svcnvidia-nemo-ci 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.

SHIP WITH CARE

RISK — Validate the new termination path with a real model rollout

The PR marks rollouts N/A because existing servers don't set the header. However, this adds a behavior-changing agent exit in the tool loop, while the new regression test mocks both model and resources responses. The trusted quality bar requires representative real-model rollouts and inspection of agent and verifier behavior for behavior-changing agent work. This is a validation gap, not a demonstrated runtime failure.

Correction: Run a small real-model pairing through /run or the single-agent environment, using a resources tool that updates terminal task state and returns the header. Record the configuration, command, and inspected result in the PR. No broad implementation change is needed to address this evidence gap.

Acceptance and preservation: Confirm that the terminating tool's actual output appears in the transcript, no later policy-model or tool request runs, and verification still checks the completed state and returns the expected reward. Retain the regression test's assertion that later calls in the same turn receive matching Not run outputs. Run an unmarked control and confirm normal tool/model continuation and verification remain unchanged. These are proposed checks; I haven't executed them.

I inspected all five changed files and the directly affected response, session, checkpoint, trajectory, and verification paths. I found no additional qualifying issues. This was a static review; I didn't execute tests or rollouts.

Review generated by codex. This was a full review. Request a full review with /review reset=true.

@d-molinari
d-molinari force-pushed the dmolinari/simple-agent-episode-done branch from 80cc314 to 79ff1a1 Compare November 5, 2026 01:06

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

@d-molinari let's discuss this more. a resources server inverting the control like this works against the framework's design around the environment server. IMO the episode orchestration (including episode termination) should live there. if the resources server is currently needing to control this, then I think it warrants being an environment server instead. fyi @ffrujeri @yaoyu-33

@d-molinari

Copy link
Copy Markdown
Contributor Author

@ananthsub thanks for the comment, let me iterate on the design to avoid changing the semantics of the resource server.

@d-molinari

Copy link
Copy Markdown
Contributor Author

Closing this PR in favor of introducing an environment server for OfficeEnv

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.

3 participants