fix: disable Codex multi-agent v2 for Relay runs#427
Conversation
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughCodex configuration installation and launch now disable ChangesCodex multi-agent v2 handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant CodexLaunch
participant CodexConfig
participant Backup
CLI->>CodexLaunch: prepare launch arguments
CodexLaunch->>CodexConfig: set features.multi_agent_v2.enabled=false
CLI->>CodexConfig: install or uninstall configuration
CodexConfig->>Backup: sanitize or read saved configuration
Backup-->>CodexConfig: return saved multi_agent_v2 state
CodexConfig-->>CLI: updated configuration
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/cli/src/agents/codex/host.rs`:
- Around line 1545-1553: Add coverage for the None branch of
restore_multi_agent_v2_enabled in the plugin host tests: start with an original
configuration lacking the multi_agent_v2 section, install, then uninstall
through the backup-restore path, and assert the multi_agent_v2 section is fully
removed. Keep the existing enabled-value restoration test unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: a265b6b5-63df-4fc6-bc5a-93d503b776e8
📒 Files selected for processing (4)
crates/cli/src/agents/codex/host.rscrates/cli/src/agents/codex/launch.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/agents/plugin_host_tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Check / Run
🧰 Additional context used
📓 Path-based instructions (9)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/cli/src/agents/codex/launch.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/agents/plugin_host_tests.rscrates/cli/src/agents/codex/host.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/cli/src/agents/codex/launch.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/agents/plugin_host_tests.rscrates/cli/src/agents/codex/host.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/cli/src/agents/codex/launch.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/agents/plugin_host_tests.rscrates/cli/src/agents/codex/host.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/cli/src/agents/codex/launch.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/agents/plugin_host_tests.rscrates/cli/src/agents/codex/host.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/cli/src/agents/codex/launch.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/agents/plugin_host_tests.rscrates/cli/src/agents/codex/host.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/cli/src/agents/codex/launch.rscrates/cli/src/agents/codex/host.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/cli/src/agents/codex/launch.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/agents/plugin_host_tests.rscrates/cli/src/agents/codex/host.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/cli/src/agents/codex/launch.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/agents/plugin_host_tests.rscrates/cli/src/agents/codex/host.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/agents/plugin_host_tests.rs
🔇 Additional comments (13)
crates/cli/src/agents/codex/host.rs (8)
750-753: LGTM!
931-938: LGTM!
1184-1198: LGTM!
1200-1218: LGTM!
1512-1526: LGTM!
1528-1543: LGTM!
1555-1578: LGTM!
1379-1387: 🎯 Functional CorrectnessExisting backups are preserved.
refresh_codex_config_backupreuses the prior backup when one exists and only snapshots the live config if no backup file is present.crates/cli/tests/coverage/agents/plugin_host_tests.rs (3)
1164-1167: LGTM!
1176-1221: LGTM!
2046-2065: LGTM!crates/cli/src/agents/codex/launch.rs (1)
36-45: LGTM!crates/cli/tests/coverage/agents/launcher_tests.rs (1)
269-273: LGTM!
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
Live-tested the Codex install, repair, uninstall, and transparent-run paths against regular and inline TOML configs. I also ran a real delegated Codex task through Relay and confirmed the child prompt and response were recorded as readable ATOF events. The focused Codex tests pass, and I didn’t find any blocking issues in the diff. LGTM. |
|
/merge |
#### Overview Updates the local Codex integration guide and documents the multi-agent delegation encryption behavior and its practical observability boundaries. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Refreshes Codex prerequisites, gateway lifecycle and configuration scope, capture behavior, and safe installation guidance. - Explains what Relay can record for encrypted multi-agent delegation: the parent tool metadata and ciphertext, child routing fields, and child response; it cannot decrypt the delegated task. - Documents the available mitigation and the configuration, restore, and settings caveats. - Contains no implementation changes. - Breaking changes: none. Validation: - `just docs` - `just docs-linkcheck` - targeted `uv run pre-commit run --files docs/nemo-relay-cli/codex.mdx` #### Where should the reviewer start? Start with `docs/nemo-relay-cli/codex.mdx`. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Closes: RELAY-478 - Relates to: #425 - Relates to: #427 Authors: - Will Killian (https://github.com/willkill07) Approvers: - Maryam Najafian (https://github.com/mnajafian-nv) - https://github.com/lvojtku URL: #426
Overview
Disable Codex multi-agent v2 when Codex runs through NeMo Relay so delegated task content remains readable to Relay observability instead of arriving as opaque encrypted content.
Details
nemo-relay install codexwritefeatures.multi_agent_v2.enabled = falsealongside the managed provider and hooks configuration.features.multi_agent_v2table.features.multi_agent_v2.enabled=falseas a temporary Codex config override for transparent wrapped execution.just test-rust,cargo fmt --all -- --check,cargo clippy --workspace --all-targets -- -D warnings, anduv run pre-commit run --all-filespassed.Where should the reviewer start?
Start with
crates/cli/src/agents/codex/host.rsfor persistent install and rollback ownership, thencrates/cli/src/agents/codex/launch.rsfor the wrapped-execution override.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
New Features
Bug Fixes