feat: move Codex adapter to v1alpha2 - #198
Conversation
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe Codex adapter now uses typed ChangesCodex typed runtime integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Lifecycle
participant AgentConfig
participant CodexRuntime
participant Relay
participant CodexSDK
Lifecycle->>AgentConfig: Parse agent_config
AgentConfig->>CodexRuntime: Provide validated configuration
CodexRuntime->>Relay: Prepare typed runtime inputs
CodexRuntime->>CodexSDK: Build thread options and invoke
CodexSDK-->>CodexRuntime: Return execution result
CodexRuntime->>Relay: Normalize relay output
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Fern docs preview: https://nvidia-preview-pull-request-198.docs.buildwithfern.com/nemo/fabric |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/adapters/test_codex_adapter.py (1)
352-390: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove the dead
capability_plantest setup.The adapter now reads MCP servers from
inputs.config.mcpand skills frominputs.config.skills. It never readscapability_plan. These tests still setcodex_payload["capability_plan"]next to the typedconfigblocks, so each fixture declares the same servers and skill paths twice.The duplicated setup gives no coverage and will drift from the typed contract. Delete the
capability_planassignments at lines 354-376, 428-430, 460, 474-480, and 496, and keep only theconfigblocks.♻️ Example for `test_sdk_registers_native_skill_roots`
- codex_payload["capability_plan"] = { - "native": {"skill_paths": ["skills/review", "skills/test"]} - } codex_payload["config"]["skills"] = { "paths": ["skills/review", "skills/test"] }Also applies to: 419-433, 451-461, 473-485, 494-497, 575-603
🤖 Prompt for 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. In `@tests/adapters/test_codex_adapter.py` around lines 352 - 390, Remove the unused codex_payload["capability_plan"] assignments from the MCP and skill setup in test_sdk_maps_native_mcp_servers_into_thread_config and the other affected tests, including test_sdk_registers_native_skill_roots. Retain only the corresponding codex_payload["config"]["mcp"] and codex_payload["config"]["skills"] blocks so fixtures exercise the typed configuration consumed by the adapter.
🤖 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 `@adapters/codex/src/nemo_fabric_adapters/codex/adapter.py`:
- Around line 1011-1017: Wrap RuntimeContext.from_mapping in both the start and
invoke flows with validation-error handling, using the surrounding start/invoke
methods as the integration points. Convert malformed or missing runtime_context
ValidationError failures into lifecycle.LifecycleError with code
"codex_invalid_runtime_context" and a clear message, while preserving the
existing runtime-ID mismatch validation and normal execution paths.
In `@tests/adapters/test_codex_adapter.py`:
- Around line 1276-1290: Update the two tests around runtime_input to expect
pydantic.ValidationError instead of Exception, and provide match patterns
targeting the telemetry env field in the first test and the telemetry section
field in test_environment_rejects_non_mapping_runtime_telemetry. Keep the
existing invalid payload parametrization and runtime_input calls unchanged.
---
Outside diff comments:
In `@tests/adapters/test_codex_adapter.py`:
- Around line 352-390: Remove the unused codex_payload["capability_plan"]
assignments from the MCP and skill setup in
test_sdk_maps_native_mcp_servers_into_thread_config and the other affected
tests, including test_sdk_registers_native_skill_roots. Retain only the
corresponding codex_payload["config"]["mcp"] and
codex_payload["config"]["skills"] blocks so fixtures exercise the typed
configuration consumed by the adapter.
🪄 Autofix
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: 3bdfa044-64ba-4bf8-9ccf-6a567d8935d7
⛔ Files ignored due to path filters (2)
adapters/codex/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
adapters/codex/fabric-adapter.jsonadapters/codex/pyproject.tomladapters/codex/src/nemo_fabric_adapters/codex/adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.pycrates/fabric-cli/assets/adapters/codex/fabric-adapter.jsoncrates/fabric-core/src/runtime.rstests/adapters/test_adapter_package_metadata.pytests/adapters/test_codex_adapter.py
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (33)
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
adapters/codex/fabric-adapter.jsontests/adapters/test_adapter_package_metadata.pycrates/fabric-cli/assets/adapters/codex/fabric-adapter.jsonadapters/common/src/nemo_fabric_adapters/common/utils.pycrates/fabric-core/src/runtime.rstests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
adapters/codex/fabric-adapter.jsontests/adapters/test_adapter_package_metadata.pycrates/fabric-cli/assets/adapters/codex/fabric-adapter.jsonadapters/codex/pyproject.tomladapters/common/src/nemo_fabric_adapters/common/utils.pycrates/fabric-core/src/runtime.rstests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
**/*.{json,jsonschema}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Public contract changes must keep checked-in JSON Schema snapshots synchronized.
Files:
adapters/codex/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/codex/fabric-adapter.json
adapters/*/fabric-adapter.json
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Define a narrow, truthful adapter descriptor; keep
config.accepts,config.generates, requirements, telemetry declarations, lifecycle capabilities, and advertised capabilities synchronized with implementation and tests.
Files:
adapters/codex/fabric-adapter.json
adapters/*/{README.md,fabric-adapter.json}
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Document installation, supported configuration, harness-only settings, credentials, lifecycle, telemetry, artifacts, limitations, and focused test commands; keep documentation consistent with descriptor claims.
Files:
adapters/codex/fabric-adapter.json
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public NeMo Fabric contracts.
Files:
adapters/codex/fabric-adapter.jsonadapters/codex/pyproject.tomladapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.Use
snake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Files:
tests/adapters/test_adapter_package_metadata.pyadapters/common/src/nemo_fabric_adapters/common/utils.pycrates/fabric-core/src/runtime.rstests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.In Python SDK, adapters, examples, and tests, follow the existing style, use type annotations for public APIs, and keep native binding declarations synchronized with their Rust implementations.
Files:
tests/adapters/test_adapter_package_metadata.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{rs,py,pyi}: If public configuration types change, confirm schema snapshot tests injust test-rustpass and review generated schema diffs.
For schema or public contract changes, run both language suites and review changes underschemas/and generated API references.
Files:
tests/adapters/test_adapter_package_metadata.pyadapters/common/src/nemo_fabric_adapters/common/utils.pycrates/fabric-core/src/runtime.rstests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
tests/adapters/**/*.py
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
tests/adapters/**/*.py: If an adapter or integration changes, run its focused tests.
For adapter behavior changes, run focused adapter tests undertests/adapters, then runjust test-python.
Files:
tests/adapters/test_adapter_package_metadata.pytests/adapters/test_codex_adapter.py
**/*.{py,pyi,rs}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For Python SDK or PyO3 binding changes, use
python-tests, run focused pytest tests first, thenjust test-python; rebuild withjust build-pythonwhen native code or packaging changes.Public contract changes must keep native Python binding declarations synchronized with their Rust implementations.
Files:
tests/adapters/test_adapter_package_metadata.pyadapters/common/src/nemo_fabric_adapters/common/utils.pycrates/fabric-core/src/runtime.rstests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
**/*.{rs,py,toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
When editing version helpers, verify every
nemo-fabric-*workspace package through Cargo metadata and reject a static version inpython/pyproject.toml.
Files:
tests/adapters/test_adapter_package_metadata.pyadapters/codex/pyproject.tomladapters/common/src/nemo_fabric_adapters/common/utils.pycrates/fabric-core/src/runtime.rstests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
**/*.{toml,rs,py}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
Avoid blind repository-wide replacement of version-like strings; distinguish package-version references from examples and unrelated dependency versions.
Files:
tests/adapters/test_adapter_package_metadata.pyadapters/codex/pyproject.tomladapters/common/src/nemo_fabric_adapters/common/utils.pycrates/fabric-core/src/runtime.rstests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
**/*.{md,mdx,yml,py,rs,sh}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Keep documentation aligned with current NeMo Fabric behavior, repository layout, entry points, commands, package names, APIs, bindings, and support claims.
Files:
tests/adapters/test_adapter_package_metadata.pyadapters/common/src/nemo_fabric_adapters/common/utils.pycrates/fabric-core/src/runtime.rstests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/adapters/test_adapter_package_metadata.pytests/adapters/test_codex_adapter.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorAsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
If a fixture is needed in multiple test files, define it once inconftest.pyrather than repeating it.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a<fixture_name>_fixturefunction; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its returned value is unused or it returns no value.
Avoid defensive programming in tests; access expected values directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
When adapter installation metadata changes, packaging metadata tests must directly assert that the root project depends unconditionally on the exact-versionnemo-fabric-runtimedistribution.
Packaging metadata tests must verify that each root harness extra delegates to the matching version of the leaf adapter'sharnessextra.
Packaging metadata tests must verify that bare leaf dependencies remain adapter-owned and that the rootadapter-testsdependency group installs each leaf through itsharnessextra.
Packaging metadata tests must verify that every leaf providesfull; only adapters importing NeMo Relay Python APIs providerelay, while adapters using an external Relay executable havefullequal toharness.
Files:
tests/adapters/test_adapter_package_metadata.pytests/adapters/test_codex_adapter.py
{tests/**,python/tests/**}
⚙️ CodeRabbit configuration file
{tests/**,python/tests/**}: Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.
Files:
tests/adapters/test_adapter_package_metadata.pytests/adapters/test_codex_adapter.py
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For any Rust change, run
just test-rustandcargo fmt --all -- --check.For Rust core, CLI, or shared runtime semantic changes, run Rust formatting and tests, and add Python tests when behavior is exposed through the SDK.
Use Rust stable tooling; format Rust code with
cargo fmt --all, verify formatting withcargo fmt --all -- --check, and compile withcargo check --workspace --locked.
Files:
adapters/codex/pyproject.tomlcrates/fabric-core/src/runtime.rs
**/{Cargo.toml,Cargo.lock,pyproject.toml,package.json}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For new or updated dependencies, document the functional need, alternatives considered, and why the selected dependency is the narrowest fit.
Files:
adapters/codex/pyproject.toml
**/*.{toml,lock}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a manifest or lockfile changes, run the license-diff script against
origin/main, review transitive license changes, and run theattributions-rustandattributions-pythonpre-commit hooks.
Files:
adapters/codex/pyproject.toml
**/*.{yml,yaml,toml,lock}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For CI or packaging changes, use
maintain-ciormaintain-packaging, then run recipes and checks whose behavior changed.
Files:
adapters/codex/pyproject.toml
{pyproject.toml,adapters/**/pyproject.toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
{pyproject.toml,adapters/**/pyproject.toml}: Update the literalproject.versionin the root setuptools project and every adapterpyproject.toml.
Keep internal exact-version requirements aligned: rootnemo-fabric-* == <version>optional dependencies and each adapter'snemo-fabric-adapters-common == <version>dependency.
Files:
adapters/codex/pyproject.toml
**/*.{toml,yaml,yml,sh,bash}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
TOML, YAML, and shell files must use the specified
#SPDX copyright and Apache-2.0 license headers.
Files:
adapters/codex/pyproject.toml
**/{Cargo.toml,pyproject.toml}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
**/{Cargo.toml,pyproject.toml}: Keep package names, dependency declarations, import paths, module names, and workspace or Python package metadata internally consistent.
Prefer the standard library, an existing dependency, or a small local implementation before adding a new direct dependency.
When multiple dependencies satisfy the requirement, prefer a maintained OSS option with clear SPDX metadata, a smaller transitive graph, and permissive licensing such as Apache-2.0, MIT, BSD, or ISC.
Files:
adapters/codex/pyproject.toml
**/{Cargo.toml,pyproject.toml,Cargo.lock,uv.lock}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
**/{Cargo.toml,pyproject.toml,Cargo.lock,uv.lock}: For new dependencies, record the functional need, viable alternatives considered, why the selected dependency is the narrowest fit, and any unresolved licensing question.
After updating manifests or lockfiles, runuv run --no-project python scripts/licensing/license_diff.py --base-ref origin/mainand review added packages and license changes.
Keep workspace, Python package, and lockfile versions aligned wherever the packaging contract requires alignment.
Files:
adapters/codex/pyproject.toml
adapters/*/pyproject.toml
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
adapters/*/pyproject.toml: Keep leaf adapter runtime dependencies razor-thin and adapter-owned. Do not declare the wrapped harness/SDK or dependencies already declared by its supported package; provide dependency-free fallbacks for optional libraries.
Provide every installable Python leaf adapter with aharnessextra and afullextra for package-installable integrations; provide arelayextra when importing NeMo Relay Python APIs, but not when Relay is an external executable.
For packaged harnesses, declare and document aharnessversion constraint supported by upstream contracts and test evidence; do not claim a broader range than the evidence supports.
Files:
adapters/codex/pyproject.toml
adapters/*/{pyproject.toml,uv.lock}
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Keep each adapter package independent and small, using Python package metadata and lock files as applicable.
Files:
adapters/codex/pyproject.toml
adapters/*/src/**/*.py
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
adapters/*/src/**/*.py: Implement adapters using the existing Fabricpythonorprocessrunner and normalized request/result contracts; do not add a runner or one-off abstraction for a single adapter.
Treat normalizedconfig, Fabric-resolved plans, andruntime_contextas authoritative; reserveharness.settingsfor adapter-wide behavior and apply precedence in the order: normalized config, plans/context, harness settings, descriptor/default values.
Reject conflicting duplicate declarations and unsupported behavior with actionable errors naming the field and supported alternatives; never silently drop configuration.
Validate dependency versions, hooks, and credentials before harness invocation, and never expose credential values in outputs, errors, events, logs, or fixtures.
Forward only required system, selected credential, telemetry, and documented harness-specific environment variables; never forward or log unrelated environment values.
Maintain one local adapter host per Fabric runtime across orderedstart→invoke*→stopoperations. Emit one JSON lifecycle response per request on stdout and diagnostics on stderr.
Return harness-level invocation failures as successful lifecycle responses containingresponse: null,failed: true, and structurederrorfields (code,message,retryable, and optionalmetadata).
Do not emit NeMo Relay stream records on adapter stdout; return exactly one terminal lifecycle response while streaming occurs through the SDK-owned out-of-band endpoint.
Scope workspace, generated configuration, state, sessions, and artifacts to the resolved runtime context, and isolate stateful adapter instances by Fabric runtime ID.
Usestartto initialize adapter-owned harness state, retain it for continuation across repeatedinvokecalls on the same runtime, and release it instop.
Files:
adapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Implement new runtime or binding behavior in the shared Rust core first.
Files:
crates/fabric-core/src/runtime.rs
crates/fabric-core/**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes under
crates/fabric-coremust run both the Rust and Python test suites.
Files:
crates/fabric-core/src/runtime.rs
**/*.{rs,rmeta}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Rust code changes, run
cargo fmt --all -- --checkandjust test-rust.
Files:
crates/fabric-core/src/runtime.rs
crates/fabric-core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/fabric-corechanges in a way exposed through Python, run both the Rust and Python suites.For Rust API reference changes, update Rust documentation comments under
crates/fabric-core/instead of generated API reference files.
Files:
crates/fabric-core/src/runtime.rs
crates/fabric-core/src/**/*.rs
⚙️ CodeRabbit configuration file
crates/fabric-core/src/**/*.rs: Review the Rust core for runtime lifecycle correctness, handle validation, capability routing accuracy, schema stability, and error semantics.
Public API changes should match committed schemas, tests, and documentation.
Files:
crates/fabric-core/src/runtime.rs
🧠 Learnings (2)
📚 Learning: 2026-06-29T22:34:52.407Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 27
File: adapters/codex-cli/fabric-adapter.json:13-15
Timestamp: 2026-06-29T22:34:52.407Z
Learning: In NeMo-Fabric adapter manifest files (e.g., `*/fabric-adapter.json`), keep `config.accepts` limited to the top-level Fabric capability sections that `resolve_capability_plan` consumes (such as `models`, `tools`, `mcp`, `skills`, `telemetry`). Do not add adapter-owned `harness.settings` keys to `config.accepts`; `harness.settings` should remain adapter-owned and be passed through unchanged.
Applied to files:
adapters/codex/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/codex/fabric-adapter.json
📚 Learning: 2026-07-09T22:28:51.689Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 43
File: adapters/claude-sdk/src/nemo_fabric_adapters/claude_sdk/adapter.py:164-168
Timestamp: 2026-07-09T22:28:51.689Z
Learning: In the NeMo-Fabric adapters, treat path values used in Fabric adapter configuration (including logic like `_resolve_path` in adapter.py) as config-root-relative. Do not apply `Path.expanduser()` (or otherwise apply `~`/home or shell-style expansion), because it will make the resolved paths normalize inconsistently across adapters. Also, do not rely on or add any resolution behavior that uses `harness.settings.cwd` as an override point for these adapter paths—`harness.settings.cwd` is explicitly unsupported in this adapter context.
Applied to files:
adapters/codex/src/nemo_fabric_adapters/codex/adapter.py
🪛 Ruff (0.16.1)
tests/adapters/test_codex_adapter.py
[warning] 1159-1159: Missing return type annotation for private function block
Add return type annotation: None
(ANN202)
[error] 1241-1241: Possible hardcoded password assigned to: "FABRIC_UNRELATED_SECRET"
(S105)
[warning] 1281-1281: Do not assert blind exception: Exception
(B017)
[warning] 1281-1281: pytest.raises(Exception) is too broad, set the match parameter or use a more specific exception
(PT011)
[warning] 1289-1289: Do not assert blind exception: Exception
(B017)
[warning] 1289-1289: pytest.raises(Exception) is too broad, set the match parameter or use a more specific exception
(PT011)
adapters/codex/src/nemo_fabric_adapters/codex/adapter.py
[warning] 754-754: Dynamically typed expressions (typing.Any) are disallowed in result
(ANN401)
[warning] 834-834: Dynamically typed expressions (typing.Any) are disallowed in _open_thread
(ANN401)
[warning] 840-840: Dynamically typed expressions (typing.Any) are disallowed in thread
(ANN401)
[warning] 953-955: Abstract raise to an inner function
(TRY301)
🔇 Additional comments (9)
adapters/codex/fabric-adapter.json (1)
63-63: LGTM!adapters/codex/src/nemo_fabric_adapters/codex/adapter.py (2)
32-34: LGTM!Also applies to: 95-103, 142-147, 157-183, 186-212, 238-302, 305-321, 361-392, 395-403, 450-495, 553-602, 605-645, 648-676, 753-792, 807-860, 885-899, 935-998, 1075-1075, 1129-1129
498-519: 🗄️ Data Integrity & IntegrationKeep
runtime_contextas a mapping. The lifecycle host replaces onlypayload["config"]; the Relay path does not receive a typedruntime_context.> Likely an incorrect or invalid review comment.crates/fabric-cli/assets/adapters/codex/fabric-adapter.json (1)
63-63: LGTM!tests/adapters/test_adapter_package_metadata.py (1)
76-76: LGTM!tests/adapters/test_codex_adapter.py (1)
15-52: LGTM!Also applies to: 89-89, 112-118, 507-512, 535-535, 547-547, 559-559, 655-655, 882-882, 994-995, 1040-1040, 1073-1073, 1154-1164, 1238-1258, 1299-1299
adapters/common/src/nemo_fabric_adapters/common/utils.py (1)
234-236: LGTM!Also applies to: 259-265, 306-306
crates/fabric-core/src/runtime.rs (1)
1819-1821: LGTM!Also applies to: 2460-2460, 2650-2668
adapters/codex/pyproject.toml (1)
28-28: 🗄️ Data Integrity & IntegrationNo change needed. The editable source path exists, its
project.versionis0.2.0, andPACKAGE_VERSIONresolves to0.2.0.> Likely an incorrect or invalid review comment.
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
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 `@tests/adapters/test_codex_adapter.py`:
- Around line 1282-1283: Update the pytest.raises match pattern in the
runtime_input validation test to use a raw regex with the dot in “telemetry.env”
escaped, ensuring the assertion matches the exact field path and satisfies Ruff
RUF043.
🪄 Autofix
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: 54a0ba75-1a68-42b8-9aa3-e9bac38f4940
📒 Files selected for processing (2)
adapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_codex_adapter.py
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Preview docs
- GitHub Check: Test (arm64)
- GitHub Check: Test (x86_64)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
tests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
tests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.Use
snake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Files:
tests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.In Python SDK, adapters, examples, and tests, follow the existing style, use type annotations for public APIs, and keep native binding declarations synchronized with their Rust implementations.
Files:
tests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{rs,py,pyi}: If public configuration types change, confirm schema snapshot tests injust test-rustpass and review generated schema diffs.
For schema or public contract changes, run both language suites and review changes underschemas/and generated API references.
Files:
tests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
tests/adapters/**/*.py
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
tests/adapters/**/*.py: If an adapter or integration changes, run its focused tests.
For adapter behavior changes, run focused adapter tests undertests/adapters, then runjust test-python.
Files:
tests/adapters/test_codex_adapter.py
**/*.{py,pyi,rs}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For Python SDK or PyO3 binding changes, use
python-tests, run focused pytest tests first, thenjust test-python; rebuild withjust build-pythonwhen native code or packaging changes.Public contract changes must keep native Python binding declarations synchronized with their Rust implementations.
Files:
tests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
**/*.{rs,py,toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
When editing version helpers, verify every
nemo-fabric-*workspace package through Cargo metadata and reject a static version inpython/pyproject.toml.
Files:
tests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
**/*.{toml,rs,py}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
Avoid blind repository-wide replacement of version-like strings; distinguish package-version references from examples and unrelated dependency versions.
Files:
tests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
**/*.{md,mdx,yml,py,rs,sh}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Keep documentation aligned with current NeMo Fabric behavior, repository layout, entry points, commands, package names, APIs, bindings, and support claims.
Files:
tests/adapters/test_codex_adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/adapters/test_codex_adapter.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorAsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
If a fixture is needed in multiple test files, define it once inconftest.pyrather than repeating it.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a<fixture_name>_fixturefunction; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its returned value is unused or it returns no value.
Avoid defensive programming in tests; access expected values directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
When adapter installation metadata changes, packaging metadata tests must directly assert that the root project depends unconditionally on the exact-versionnemo-fabric-runtimedistribution.
Packaging metadata tests must verify that each root harness extra delegates to the matching version of the leaf adapter'sharnessextra.
Packaging metadata tests must verify that bare leaf dependencies remain adapter-owned and that the rootadapter-testsdependency group installs each leaf through itsharnessextra.
Packaging metadata tests must verify that every leaf providesfull; only adapters importing NeMo Relay Python APIs providerelay, while adapters using an external Relay executable havefullequal toharness.
Files:
tests/adapters/test_codex_adapter.py
{tests/**,python/tests/**}
⚙️ CodeRabbit configuration file
{tests/**,python/tests/**}: Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.
Files:
tests/adapters/test_codex_adapter.py
adapters/*/src/**/*.py
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
adapters/*/src/**/*.py: Implement adapters using the existing Fabricpythonorprocessrunner and normalized request/result contracts; do not add a runner or one-off abstraction for a single adapter.
Treat normalizedconfig, Fabric-resolved plans, andruntime_contextas authoritative; reserveharness.settingsfor adapter-wide behavior and apply precedence in the order: normalized config, plans/context, harness settings, descriptor/default values.
Reject conflicting duplicate declarations and unsupported behavior with actionable errors naming the field and supported alternatives; never silently drop configuration.
Validate dependency versions, hooks, and credentials before harness invocation, and never expose credential values in outputs, errors, events, logs, or fixtures.
Forward only required system, selected credential, telemetry, and documented harness-specific environment variables; never forward or log unrelated environment values.
Maintain one local adapter host per Fabric runtime across orderedstart→invoke*→stopoperations. Emit one JSON lifecycle response per request on stdout and diagnostics on stderr.
Return harness-level invocation failures as successful lifecycle responses containingresponse: null,failed: true, and structurederrorfields (code,message,retryable, and optionalmetadata).
Do not emit NeMo Relay stream records on adapter stdout; return exactly one terminal lifecycle response while streaming occurs through the SDK-owned out-of-band endpoint.
Scope workspace, generated configuration, state, sessions, and artifacts to the resolved runtime context, and isolate stateful adapter instances by Fabric runtime ID.
Usestartto initialize adapter-owned harness state, retain it for continuation across repeatedinvokecalls on the same runtime, and release it instop.
Files:
adapters/codex/src/nemo_fabric_adapters/codex/adapter.py
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public NeMo Fabric contracts.
Files:
adapters/codex/src/nemo_fabric_adapters/codex/adapter.py
🧠 Learnings (1)
📚 Learning: 2026-07-09T22:28:51.689Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 43
File: adapters/claude-sdk/src/nemo_fabric_adapters/claude_sdk/adapter.py:164-168
Timestamp: 2026-07-09T22:28:51.689Z
Learning: In the NeMo-Fabric adapters, treat path values used in Fabric adapter configuration (including logic like `_resolve_path` in adapter.py) as config-root-relative. Do not apply `Path.expanduser()` (or otherwise apply `~`/home or shell-style expansion), because it will make the resolved paths normalize inconsistently across adapters. Also, do not rely on or add any resolution behavior that uses `harness.settings.cwd` as an override point for these adapter paths—`harness.settings.cwd` is explicitly unsupported in this adapter context.
Applied to files:
adapters/codex/src/nemo_fabric_adapters/codex/adapter.py
🪛 Ruff (0.16.1)
tests/adapters/test_codex_adapter.py
[warning] 1282-1282: Pattern passed to match= contains metacharacters but is neither escaped nor raw
(RUF043)
🔇 Additional comments (2)
adapters/codex/src/nemo_fabric_adapters/codex/adapter.py (1)
32-32: LGTM!Also applies to: 932-940, 969-969, 1023-1023
tests/adapters/test_codex_adapter.py (1)
15-15: LGTM!Also applies to: 1286-1298
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
1 participant
Overview
Move the Codex adapter to the
fabric.adapter/v1alpha2typed contract. Lifecycle startup validatesAgentConfig, and an intentionally small dataclass keeps normalized configuration,RuntimeContext, and base directory together while model, MCP, skills, instructions, and telemetry are read directly from the contract.The runtime telemetry context now carries the already-resolved native telemetry config required by Codex. Relay keeps its prior precedence rules through the shared typed model-name override. No shared helper is removed: caller audit still finds Claude, Codex, Hermes, and Deep Agents users.
Validated with
pytest -q tests/adapters/test_codex_adapter.py tests/adapters/test_adapter_package_metadata.py(65 passed),pytest -q tests/integrations/test_harbor_runner.py::test_codex_adapter_maps_fabric_request_to_sdk(1 passed), andcargo test -p nemo-fabric-core runtime_context_preserves_native_telemetry_config --lib(1 passed).Where should the reviewer start?
Start with
adapters/codex/src/nemo_fabric_adapters/codex/adapter.py, thencrates/fabric-core/src/runtime.rsfor the native telemetry handoff.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Relates to FABRIC-185
I confirm this contribution is my own work, or I have the right to submit it under this project's license.
I searched existing issues and open pull requests, and this does not duplicate existing work.