Skip to content

fix(codex): retain native rollout evidence and retry rate limits - #4047

Closed
lafrancef wants to merge 2 commits into
mainfrom
flafrance/codex-harness-conformance
Closed

lafrancef wants to merge 2 commits into
mainfrom
flafrance/codex-harness-conformance

Conversation

@lafrancef

Copy link
Copy Markdown
Contributor

Codex's CLI display events omit the native tool-call IDs and model-attempt ownership required by the conformance suite, and its provider disables HTTP 429 retries. Correlated rollouts now read temporary native session evidence, carry invocation headers, and use a local forwarding endpoint that retries 429 at most twice while retaining every captured attempt.

Tool arguments, results, shell exit status, and available timeout evidence are retained before the temporary session is removed. Other HTTP errors remain unchanged, and the existing rollout timeout bounds execution.

Depends on #4045 for the shared collector correction; this PR is based on that branch to keep its diff harness-specific. No separate issue is needed: the failures reproduce with the conformance suite from #3766.

All seven conformance scenarios pass with Codex 0.144.4 (previously 0/7). Reproduce with python scripts/run_harness_conformance.py --harness codex --output /tmp/codex-conformance using a fresh output directory. Error/retry ownership satisfies TE-8, with TE-9 as the alternative response-ID contract. pre-commit run --all-files is clean. A real gpt-4.1-mini rollout computed 17 × 23, wrote and read 391 through a shell tool, and returned CONFORMANCE_DONE; the independent verifier checked both the file and final answer and awarded 1.0 (two provider calls).

@lafrancef lafrancef added bug Something isn't working area:agent Agent harnesses and Responses API agent behavior needs-review PR is ready for code review and waiting on a reviewer labels Oct 5, 2026
@copy-pr-bot

copy-pr-bot Bot commented Oct 5, 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:medium Single-domain change with interacting parts or a moderate review surface label Oct 5, 2026
@github-actions github-actions Bot added the sla:triage-overdue Review assignment is over the one-business-day SLA label Oct 6, 2026
@lafrancef
lafrancef force-pushed the flafrance/pi-harness-conformance branch from e5c1ae2 to 5dc3d2e Compare October 6, 2026 20:44
Base automatically changed from flafrance/pi-harness-conformance to main October 6, 2026 21:15
@giuliolovisotto
giuliolovisotto self-requested a review October 8, 2026 06:28

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

This will make codex fully observable.

@github-actions github-actions Bot removed the sla:triage-overdue Review assignment is over the one-business-day SLA label Oct 8, 2026
@lafrancef

Copy link
Copy Markdown
Contributor Author

/ok to test a7ef5b1

lafrancef and others added 2 commits October 8, 2026 11:57
Signed-off-by: Frederic Lafrance <flafrance@nvidia.com>
When `codex exec` exceeds `timeout`, the run can still end in an assistant
message, so `finished_naturally` reported True. Carry the timeout to `run()`
through an internal `_ng_agent_timed_out` key (removed before `/verify`),
set `finished_naturally=False` and add `agent_timed_out` to the verify
response. Reward and the verify payload are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
@lafrancef
lafrancef force-pushed the flafrance/codex-harness-conformance branch from a7ef5b1 to 384445c Compare October 8, 2026 10:09
@lafrancef

Copy link
Copy Markdown
Contributor Author

/ok to test 384445c

@lafrancef

Copy link
Copy Markdown
Contributor Author

Superseded by #4450 - Thanks @yaoyu-33

@lafrancef lafrancef closed this Oct 12, 2026
ananthsub pushed a commit to ananthsub/Gym that referenced this pull request Oct 12, 2026
The main/merge-queue [run
38069955548](https://github.com/NVIDIA-NeMo/Gym/actions/runs/38069955548/job/114266522363)
completed all probe jobs but failed the P0 measurement: Codex 0/7,
OpenCode 5/7, Pi 6/7, Hermes 7/7. The reporter is correctly exposing
retained-evidence failures.

This integrates the applicable existing fixes with the current
native-session adapters:

- Port NVIDIA-NeMo#4047's temporary Codex session reader and bounded 429 forwarding
retries, including its timeout result preservation. Keep invocation
ownership, original tool IDs/arguments/results and shell exit status
before removing the temporary session. Native sandbox-session execution
remains as implemented on main.
- Port the shell-outcome portion of NVIDIA-NeMo#4046 into the shared OpenCode
artifact parser. Nonzero exits fail; absent/null/invalid exits remain
unknown. Preserve native turn semantics, per that PR's review
discussion.
- Require one terminal-400 token reconciliation outcome: `healthy`, with
a known zero aggregate for zero finished responses. Preserve Hermes'
actual runtime counters when early error returns omit them. Failed model
termination still must be `unhealthy`; missing aggregate usage and
incorrect nonzero totals fail. The health engine is unchanged.

The token-total rule follows merged NVIDIA-NeMo#4287. This deliberately does not
revive closed NVIDIA-NeMo#4286's proposal to accept either healthy or unobserved.
No separate issue is necessary: the linked run is the reproduction, and
NVIDIA-NeMo#4046/NVIDIA-NeMo#4047 plus FEA-461 track the underlying gaps. Existing PRs are
left intact for author/maintainer coordination. Codex work is attributed
to Frederic Lafrance and Giulio Lovisotto; OpenCode's nonzero-exit
correction derives from Frederic's patch.

Validation on macOS arm64 / Python 3.13.14, OpenAI SDK 2.44.0; base
`728ac3395d1bc26b995b03ffb9aa6f0362b768ea`:

- **2,050 passed, 113 skipped** in the final selected suite; skips are
platform/optional-runtime cases.
- Full `pre-commit run --all-files` and staged/working-tree `git diff
--check` pass. Commit has DCO sign-off.
- **28/28 conformance scenarios pass** across all four pinned harnesses;
final Codex-only rerun is also 7/7.
- Real `nvidia/nemotron-3-super-120b-a12b` model smoke: Codex, OpenCode
and Hermes each have four captured model calls, one failed tool followed
by two successful tools, and reward **1.0**. A local verifier
independently checked the file contains `391` and the final answer
contains `CONFORMANCE_DONE`. OpenCode's initial attempt was denied by
its native external-directory permission; the rerun explicitly granted
only the smoke's temporary artifact directory. Both attempts are
retained.

Exact test selection:

```bash
python -m pytest --import-mode=importlib -q -o addopts='' -o log_cli=false \
  scripts/harness_conformance/tests tests/unit_tests/harness_capabilities \
  tests/unit_tests/test_harness_conformance_ci.py tests/unit_tests/test_harness_conformance_checks.py \
  tests/unit_tests/test_rollout_collection.py tests/unit_tests/test_rollout_health.py \
  responses_api_agents/codex_agent/tests responses_api_agents/opencode_agent/tests \
  responses_api_agents/opencode_sandboxed_agent/tests responses_api_agents/pi_agent/tests \
  responses_api_agents/hermes_agent/tests
python scripts/run_harness_conformance.py --harness codex --harness opencode --harness pi --harness hermes --output <NEW_DIRECTORY>
```

Runtime pins: Codex 0.144.4, OpenCode 1.17.11, Pi 0.80.2, Hermes
26bb847a. Source imports were verified against the repair worktree; an
early mixed-source/SDK setup attempt was discarded. The final Codex
suite was rerun after adding malformed-record protection.

Compatibility: correlated local Codex runs temporarily persist a session
and can retry 429 twice; OpenCode shell-status evidence becomes
accurate; terminal-rejection aggregate usage is strict. No dataset,
reward policy, model settings, or benchmark-score claim changes. These
probes qualify local adapter evidence; remote sandbox lifecycle
qualification, Linux-only skipped tests and full repository CI remain
separate. CI classification is **full**, because runner/shared test
paths changed. No CI trust-gate comment was posted.


Follow-up validation (`daab232a8`): Codex returns observations and
timeout state directly instead of encoding synthetic stdout events. All
227 Codex tests passed (45 skipped), all-file pre-commit passed, and all
seven fresh Codex conformance scenarios passed. Retry behavior is
unchanged.

---------

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
Co-authored-by: Frederic Lafrance <flafrance@nvidia.com>
Co-authored-by: Giulio Lovisotto <glovisotto@nvidia.com>

This branch was previously deployed

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

Labels

area:agent Agent harnesses and Responses API agent behavior bug Something isn't working complexity:medium Single-domain change with interacting parts or a moderate review surface 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.

3 participants