feat: Add Relay integration to codex-cli adapter - #30
Conversation
Signed-off-by: David Gardner <dagardner@nvidia.com>
…based upon it being named 'plugins.toml' and being in the same directory as the config.toml. There is no way to provide an explicit path to the plugin toml Signed-off-by: David Gardner <dagardner@nvidia.com>
…trings not a path to a toml file, fix placement of the codex --config flag to not conflict with relay's flags Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
…rmes specific. Update codex adapter to use it Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@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:
WalkthroughCodex CLI now supports optional local Relay gateway startup and generated profiles. Shared helpers centralize payload loading, session-id derivation, and Relay config writing. Relay profiles and smoke/E2E tests were updated for multi-profile invocation and relay-specific artifact paths. ChangesCodex CLI Relay integration
Shared payload loading and relay config writing
Relay profiles and multi-profile tests
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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-cli/src/nemo_fabric_adapters/codex_cli/adapter.py`:
- Around line 283-287: The generated Codex profile name is derived directly from
runtime_id, which can allow unsafe path-like values and collisions between
invocations. Update the profile creation and cleanup flow around
profile_path.write_text and the later unlink/delete logic to normalize
runtime_id into a safe slug and append a request/invocation-specific suffix
before using it as the profile filename. Make the same naming change wherever
the profile name/path is reused so each invocation gets a unique, sanitized
profile and cannot overwrite or delete another run’s profile.
- Around line 529-542: Relay startup failures currently bypass the JSON result
contract because `load_relay_plugin_config`, `write_relay_configs`, or
`start_relay_gateway` can raise before the Codex launch error handling runs.
Update `main()` in `adapter.py` to catch Relay startup exceptions around the
`use_relay` block and route them into the same structured failure path used for
Codex launch errors, returning a failed `CompletedProcess` with `launch_error`
set. Keep the public Fabric contract unchanged by preserving the existing
successful flow and only converting Relay startup exceptions into the structured
adapter output.
- Around line 476-477: The adapter is forwarding the entire parent environment
to nemo-relay via the env merge in the Codex CLI adapter, which can leak
secrets; update the environment construction in the relevant adapter method to
use the same sanitized/allowlisted env used for Codex and then explicitly add
only the Relay-specific variables needed. Locate the change in the adapter.py
logic around the cwd/env launch setup and adjust the env passed to the Relay
subprocess so it no longer includes os.environ wholesale.
In `@tests/test_adapaters_common_utils.py`:
- Around line 210-246: Add tests for the error handling in write_relay_configs,
not just the successful TOML write path. Add one case around
common_utils.write_relay_configs that unsets FABRIC_RELAY_CONFIG_PATH and
asserts it raises RuntimeError, and another that simulates tomli_w being
unavailable and asserts the function returns (None, None) while emitting the
stderr fallback message. Use the existing test_write_relay_configs pattern and
the write_relay_configs symbol to keep the new coverage adjacent to the current
happy-path test.
In `@tests/test_codex_cli.py`:
- Around line 392-399: The test test_wait_for_relay_gateway_times_out takes the
monkeypatch fixture but never uses it, so remove that unused parameter from the
function signature to keep Ruff ARG001 clean. Keep the rest of the test logic
unchanged, including the calls to load_codex_adapter, MagicMock, and
wait_for_relay_gateway.
- Around line 330-350: The run_codex test is missing coverage for the new
adapter output contract, since it ignores the returned value after calling
adapter.run_codex. Capture the output from run_codex and assert that it includes
the new relay_runtime and relay_artifacts fields alongside the existing
command/gateway assertions, using the run_codex method and its returned object
to keep the changed API surface covered.
- Around line 353-389: The current test only covers the successful Relay startup
path in start_relay_gateway; add an error-path test that forces
wait_for_relay_gateway to raise after subprocess.Popen returns a process. In
that test, use the same adapter fixture and mocks, then assert
start_relay_gateway propagates the failure and invokes stop_relay_gateway with
the spawned process so cleanup is verified.
In `@tests/test_hermes_e2e.py`:
- Around line 55-65: The async fixture run_hermes_with_relay is triggering Ruff
ASYNC240 because Path(...).resolve() is being called in the event loop; to keep
the test clean, avoid the pathlib I/O call there by computing the resolved paths
outside the async context or replacing the resolve usage with a non-blocking
path normalization approach. Update the artifact path setup for artifact_root
and relay_artifact_root in run_hermes_with_relay so the async fixture no longer
performs blocking filesystem work.
- Around line 62-64: The test setup in self.relay_artifact_root hardcodes the
"relay" profile name, which ties the artifact path to one specific relay
variant. Update the path construction to derive the relay artifact directory
from the selected relay-family profile in profile_names (or a shared relay
profile identifier) so subclasses like relay-otel or relay-openinference
continue to resolve the correct artifacts path without changing this test.
🪄 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: bf9ba382-4b8a-42f6-9fd4-2098ddd0e918
📒 Files selected for processing (20)
adapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.pyadapters/common/src/nemo_fabric_adapters/common/hermes.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/hermes-cli/src/nemo_fabric_adapters/hermes_cli/adapter.pyadapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.pyexamples/code-review-agent/profiles/hermes-cli-relay-otel.yamlexamples/code-review-agent/profiles/hermes-cli-relay.yamlexamples/code-review-agent/profiles/hermes-relay-cli-openinference.yamlexamples/code-review-agent/profiles/hermes-relay-openinference.yamlexamples/code-review-agent/profiles/hermes-relay-otel.yamlexamples/code-review-agent/profiles/hermes-relay.yamlexamples/code-review-agent/profiles/relay-openinference.yamlexamples/code-review-agent/profiles/relay-otel.yamlexamples/code-review-agent/profiles/relay.yamltests/smoke_cli.pytests/smoke_relay_integration.pytests/test_adapaters_common_hermes.pytests/test_adapaters_common_utils.pytests/test_codex_cli.pytests/test_hermes_e2e.py
💤 Files with no reviewable changes (7)
- examples/code-review-agent/profiles/hermes-relay.yaml
- examples/code-review-agent/profiles/hermes-relay-cli-openinference.yaml
- examples/code-review-agent/profiles/hermes-cli-relay.yaml
- examples/code-review-agent/profiles/hermes-relay-openinference.yaml
- examples/code-review-agent/profiles/hermes-cli-relay-otel.yaml
- examples/code-review-agent/profiles/hermes-relay-otel.yaml
- tests/test_adapaters_common_hermes.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
{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 Fabric contracts.
Files:
examples/code-review-agent/profiles/relay-openinference.yamlexamples/code-review-agent/profiles/relay.yamlexamples/code-review-agent/profiles/relay-otel.yamladapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.pyadapters/hermes-cli/src/nemo_fabric_adapters/hermes_cli/adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/common/src/nemo_fabric_adapters/common/hermes.pyadapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Usepytestto run tests.
Do not add@pytest.mark.asyncioto test functions; async tests are detected and run automatically by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, useunittest.mock.MagicMockorunittest.mock.AsyncMock(withspecwhen needed) instead of defining a new class.
Name mocked classes with amockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures across test files; if a fixture is needed in multiple test files, place it inconftest.py.
Define fixtures with@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])anddef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notPreferpytest.mark.parametrizeover creating individual tests for different input types. If a fixture is needed for a test but does not return a value or the value is unused, use@pytest.mark.usefixtures`.
Files:
tests/smoke_cli.pytests/smoke_relay_integration.pytests/test_adapaters_common_utils.pytests/test_codex_cli.pytests/test_hermes_e2e.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/smoke_cli.pytests/smoke_relay_integration.pytests/test_adapaters_common_utils.pytests/test_codex_cli.pytests/test_hermes_e2e.py
🧠 Learnings (1)
📚 Learning: 2026-06-29T22:34:57.500Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 27
File: tests/smoke_codex_cli.py:19-31
Timestamp: 2026-06-29T22:34:57.500Z
Learning: In NVIDIA/NeMo-Fabric, treat files in `tests/` named `smoke_*.py` as directly executed (gated) smoke scripts rather than pytest-collected tests. When reviewing or adding code to these files, keep them runnable as standalone scripts (e.g., via a script `main`/entrypoint) and avoid patterns that would make pytest collect them as normal test cases (e.g., defining pytest-style `test_*` functions/classes meant for collection). For Codex CLI adapter behavioral/pytest coverage, use the separate `tests/test_codex_cli.py` rather than adding pytest tests into `tests/smoke_*.py`.
Applied to files:
tests/smoke_cli.pytests/smoke_relay_integration.py
🧬 Code graph analysis (4)
adapters/common/src/nemo_fabric_adapters/common/utils.py (2)
adapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.py (1)
runtime_context(105-105)tests/test_adapaters_common_utils.py (1)
runtime_context(53-53)
adapters/common/src/nemo_fabric_adapters/common/hermes.py (1)
adapters/hermes-cli/src/nemo_fabric_adapters/hermes_cli/adapter.py (1)
configure_hermes_relay(67-67)
tests/test_codex_cli.py (1)
adapters/common/src/nemo_fabric_adapters/common/utils.py (1)
config_root(34-35)
adapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.py (1)
adapters/common/src/nemo_fabric_adapters/common/utils.py (3)
load_relay_plugin_config(103-127)settings_payload(72-74)collect_relay_artifacts(317-335)
🪛 ast-grep (0.44.0)
tests/test_adapaters_common_utils.py
[info] 65-65: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"request": {"input": "from file"}})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
adapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.py
[warning] 423-423: Request-controlled URL passed to urlopen; validate against an allowlist to prevent SSRF.
Context: urllib.request.urlopen(health_url, timeout=1)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(urlopen-unsanitized-data)
[error] 467-480: Command coming from incoming request
Context: subprocess.Popen(
[
relay_command,
"--config",
str(relay_config_path),
"--bind",
f"127.0.0.1:{port}",
],
cwd=cwd,
env={**os.environ, **build_env(payload)},
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
start_new_session=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[error] 555-567: Use of unsanitized data to create processes
Context: subprocess.run(
command,
cwd=cwd,
env=build_env(
payload,
relay_gateway_url=relay_gateway_url,
),
input=request_to_prompt(payload),
text=True,
capture_output=True,
check=False,
timeout=timeout,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(os-system-unsanitized-data)
[error] 555-567: Command coming from incoming request
Context: subprocess.run(
command,
cwd=cwd,
env=build_env(
payload,
relay_gateway_url=relay_gateway_url,
),
input=request_to_prompt(payload),
text=True,
capture_output=True,
check=False,
timeout=timeout,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[info] 647-647: use jsonify instead of json.dumps for JSON output
Context: json.dumps(output, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.15.20)
adapters/common/src/nemo_fabric_adapters/common/utils.py
[warning] 347-347: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 363-363: Consider moving this statement to an else block
(TRY300)
tests/test_codex_cli.py
[warning] 180-180: Yoda condition detected
(SIM300)
[warning] 183-183: Yoda condition detected
Rewrite as command[-3:-1] == ["--model", "gpt-5.4"]
(SIM300)
[warning] 392-392: Unused function argument: monkeypatch
(ARG001)
tests/test_hermes_e2e.py
[warning] 61-61: Async functions should not use pathlib.Path methods, use trio.Path or anyio.path
(ASYNC240)
adapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.py
[warning] 172-174: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 215-215: Prefer TypeError exception for invalid type
(TRY004)
[warning] 215-215: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 227-227: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 232-234: Prefer TypeError exception for invalid type
(TRY004)
[warning] 232-234: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 298-300: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 397-397: Prefer TypeError exception for invalid type
(TRY004)
[warning] 397-397: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 420-422: Avoid specifying long messages outside the exception class
(TRY003)
[error] 424-424: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[warning] 430-430: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 437-440: Use contextlib.suppress(ProcessLookupError) instead of try-except-pass
Replace try-except-pass with with contextlib.suppress(ProcessLookupError): ...
(SIM105)
[error] 468-468: subprocess call: check for execution of untrusted input
(S603)
[warning] 514-514: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 537-537: Avoid specifying long messages outside the exception class
(TRY003)
[error] 556-556: subprocess call: check for execution of untrusted input
(S603)
🔇 Additional comments (14)
examples/code-review-agent/profiles/relay-openinference.yaml (1)
1-34: LGTM!examples/code-review-agent/profiles/relay-otel.yaml (1)
1-30: LGTM!examples/code-review-agent/profiles/relay.yaml (1)
1-30: LGTM!tests/smoke_cli.py (1)
63-87: LGTM!tests/smoke_relay_integration.py (1)
37-47: LGTM!tests/test_hermes_e2e.py (2)
24-24: LGTM!Also applies to: 124-124, 145-147, 192-194, 211-219, 233-241
229-230: 🎯 Functional Correctness
plugins.tomlpath matcheswrite_relay_configs. The helper writes Relay plugin config toconfig_path.parent / "relay-config" / "plugins.toml", so the new assertion is correct.> Likely an incorrect or invalid review comment.adapters/common/src/nemo_fabric_adapters/common/utils.py (1)
41-66: LGTM!Also applies to: 337-366
tests/test_adapaters_common_utils.py (1)
7-16: LGTM!Also applies to: 59-99
adapters/common/src/nemo_fabric_adapters/common/hermes.py (1)
152-152: LGTM!adapters/hermes-cli/src/nemo_fabric_adapters/hermes_cli/adapter.py (1)
31-37: LGTM!Also applies to: 64-64
adapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.py (1)
187-187: LGTM!adapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.py (1)
13-19: LGTM!Also applies to: 36-57, 151-207, 210-237, 239-282, 387-430, 433-453, 605-633, 647-647
tests/test_codex_cli.py (1)
40-43: LGTM!Also applies to: 162-258, 259-283, 402-414
Signed-off-by: David Gardner <dagardner@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 `@adapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.py`:
- Around line 435-439: The Relay cleanup in the shutdown path only kills the
direct process, so descendants can survive and keep resources open. Update the
Relay termination logic in the adapter’s process cleanup flow to restore
process-group signaling when `nemo-relay` is started in its own session, and
keep a Windows-safe fallback for terminating the tree. Make the fix in the Relay
shutdown/cleanup code that currently calls `process.terminate()`,
`process.wait()`, and `process.kill()`, ensuring behavior stays compatible with
existing Fabric/public contract handling.
🪄 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: 1f4c816b-e6a3-42a7-9f27-4e4b10a966a7
📒 Files selected for processing (2)
adapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.pytests/test_codex_cli.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Test
⚠️ CI failures not shown inline (2)
GitHub Actions: Rust / Test: feat: Add Relay integration to codex-cli adapter
Conclusion: failure
##[group]Run cargo test --workspace --locked
�[36;1mcargo test --workspace --locked�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
CARGO_INCREMENTAL: 0
CARGO_PROFILE_DEV_DEBUG: 0
CARGO_TERM_COLOR: always
RUST_BACKTRACE: short
RUSTFLAGS: -D warnings
CARGO_UNSTABLE_SPARSE_REGISTRY: true
CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse
CACHE_ON_FAILURE: false
##[endgroup]
�[1m�[92m Compiling�[0m fabric-core v0.1.0 (/home/runner/work/NeMo-Fabric/NeMo-Fabric/crates/fabric-core)
�[1m�[92m Compiling�[0m fabric-cli v0.1.0 (/home/runner/work/NeMo-Fabric/NeMo-Fabric/crates/fabric-cli)
�[1m�[92m Finished�[0m `test` profile [unoptimized] target(s) in 5.59s
�[1m�[92m Running�[0m unittests src/main.rs (target/debug/deps/fabric-c3572a4d481471cf)
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
�[1m�[92m Running�[0m unittests src/lib.rs (target/debug/deps/fabric_core-9b081566c1d472c1)
running 41 tests
test config::tests::errors_for_unknown_manifest_profile ... ok
GitHub Actions: Rust / 0_Test.txt: feat: Add Relay integration to codex-cli adapter
Conclusion: failure
##[group]Run cargo test --workspace --locked
�[36;1mcargo test --workspace --locked�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
CARGO_INCREMENTAL: 0
CARGO_PROFILE_DEV_DEBUG: 0
CARGO_TERM_COLOR: always
RUST_BACKTRACE: short
RUSTFLAGS: -D warnings
CARGO_UNSTABLE_SPARSE_REGISTRY: true
CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse
CACHE_ON_FAILURE: false
##[endgroup]
�[1m�[92m Compiling�[0m fabric-core v0.1.0 (/home/runner/work/NeMo-Fabric/NeMo-Fabric/crates/fabric-core)
�[1m�[92m Compiling�[0m fabric-cli v0.1.0 (/home/runner/work/NeMo-Fabric/NeMo-Fabric/crates/fabric-cli)
�[1m�[92m Finished�[0m `test` profile [unoptimized] target(s) in 5.59s
�[1m�[92m Running�[0m unittests src/main.rs (target/debug/deps/fabric-c3572a4d481471cf)
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
�[1m�[92m Running�[0m unittests src/lib.rs (target/debug/deps/fabric_core-9b081566c1d472c1)
running 41 tests
test config::tests::errors_for_unknown_manifest_profile ... ok
🧰 Additional context used
📓 Path-based instructions (3)
{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 Fabric contracts.
Files:
adapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Usepytestto run tests.
Do not add@pytest.mark.asyncioto test functions; async tests are detected and run automatically by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, useunittest.mock.MagicMockorunittest.mock.AsyncMock(withspecwhen needed) instead of defining a new class.
Name mocked classes with amockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures across test files; if a fixture is needed in multiple test files, place it inconftest.py.
Define fixtures with@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])anddef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notPreferpytest.mark.parametrizeover creating individual tests for different input types. If a fixture is needed for a test but does not return a value or the value is unused, use@pytest.mark.usefixtures`.
Files:
tests/test_codex_cli.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/test_codex_cli.py
🔇 Additional comments (1)
tests/test_codex_cli.py (1)
8-8: LGTM!Also applies to: 402-423
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_codex_cli.py (1)
260-286: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winMisplaced relay-output assertions break this test's own premise.
test_relay_requires_exact_truesetsFABRIC_RELAY_ENABLED="TRUE"specifically to prove relay is not activated (confirmed bymock_load_config.assert_not_called()at line 286). Butrun_codexonly addsrelay_runtime/relay_artifactsto the output whenrelay_plugin_config is not None, which only happens inside theuse_relaybranch. Since that branch is never entered here,assert "relay_runtime" in resultandassert "relay_artifacts" in result(lines 280-281) will fail against the real adapter — these assertions contradict the test's intent and appear to have been added to the wrong test case.🐛 Proposed fix: remove the relay-output assertions from this negative test
result = adapter.run_codex(codex_payload) command = mock_run.call_args.args[0] - assert "relay_runtime" in result - assert "relay_artifacts" in result assert command[0] == "codex"🤖 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/test_codex_cli.py` around lines 260 - 286, The relay-output checks in test_relay_requires_exact_true contradict the test’s purpose because FABRIC_RELAY_ENABLED="TRUE" is meant to keep relay disabled and mock_load_config uncalled. Remove the assertions that expect relay_runtime and relay_artifacts from this negative case, and keep the test focused on verifying that run_codex does not enter the relay path, does not add --config, and uses the fabric-runtime-1 profile.
♻️ Duplicate comments (1)
tests/test_codex_cli.py (1)
289-350: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRelay-enabled test still discards
run_codexreturn value — past review comment not addressed.
test_run_codex_configures_relayis the test that actually enables relay end-to-end, yet line 330 callsadapter.run_codex(codex_payload)without capturing the result, sorelay_runtime/relay_artifactsin the output remain unasserted here — the coverage gap the prior review flagged is still open (the assertions appear to have been misapplied to a different, non-relay test instead; see the comment on lines 260-286).✅ Proposed fix: capture and assert the relay output contract
- adapter.run_codex(codex_payload) + result = adapter.run_codex(codex_payload) command = mock_run.call_args.args[0] assert command[0] == "codex" assert "nemo-relay" not in command assert command[command.index("--profile") + 1] == profile_name assert mock_run.call_args.kwargs["env"]["NEMO_RELAY_GATEWAY_URL"] == gateway_url assert "CODEX_HOME" not in mock_run.call_args.kwargs["env"] assert not profile_path.exists() + assert result["relay_runtime"]["enabled"] is True + assert "relay_artifacts" in result🤖 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/test_codex_cli.py` around lines 289 - 350, In test_run_codex_configures_relay, the relay-enabled path still ignores the return value from run_codex, so the relay output contract is not verified. Capture the result of adapter.run_codex(codex_payload) and assert the relay-specific fields on it, especially relay_runtime and relay_artifacts, using the existing test setup around run_codex, start_relay_gateway, and stop_relay_gateway. Keep the existing command/env assertions, but add checks that the returned structure reflects relay being enabled end-to-end.
🤖 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/test_codex_cli.py`:
- Around line 474-476: The test setup in the CODEX CLI test uses a hardcoded
temporary directory path, which should be replaced with a path derived from
tmp_path for portability and isolation. Update the environment assignment near
the OPENAI_API_KEY and FABRIC_UNRELATED_SECRET setup to build CODEX_HOME from
tmp_path instead of "/tmp/codex-home", keeping the unrelated secret fixture
unchanged since it is intentional test data. Use the existing test context in
test_codex_cli to ensure the temporary home directory is unique per test run.
---
Outside diff comments:
In `@tests/test_codex_cli.py`:
- Around line 260-286: The relay-output checks in test_relay_requires_exact_true
contradict the test’s purpose because FABRIC_RELAY_ENABLED="TRUE" is meant to
keep relay disabled and mock_load_config uncalled. Remove the assertions that
expect relay_runtime and relay_artifacts from this negative case, and keep the
test focused on verifying that run_codex does not enter the relay path, does not
add --config, and uses the fabric-runtime-1 profile.
---
Duplicate comments:
In `@tests/test_codex_cli.py`:
- Around line 289-350: In test_run_codex_configures_relay, the relay-enabled
path still ignores the return value from run_codex, so the relay output contract
is not verified. Capture the result of adapter.run_codex(codex_payload) and
assert the relay-specific fields on it, especially relay_runtime and
relay_artifacts, using the existing test setup around run_codex,
start_relay_gateway, and stop_relay_gateway. Keep the existing command/env
assertions, but add checks that the returned structure reflects relay being
enabled end-to-end.
🪄 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: 64334793-c149-4617-8213-d72c32739cf4
📒 Files selected for processing (2)
.agents/skills/python-tests/SKILL.mdtests/test_codex_cli.py
📜 Review details
⚠️ CI failures not shown inline (4)
GitHub Actions: Rust / Test: feat: Add Relay integration to codex-cli adapter
Conclusion: failure
##[group]Run cargo test --workspace --locked
�[36;1mcargo test --workspace --locked�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
CARGO_INCREMENTAL: 0
CARGO_PROFILE_DEV_DEBUG: 0
CARGO_TERM_COLOR: always
RUST_BACKTRACE: short
RUSTFLAGS: -D warnings
CARGO_UNSTABLE_SPARSE_REGISTRY: true
CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse
CACHE_ON_FAILURE: false
##[endgroup]
�[1m�[92m Compiling�[0m fabric-core v0.1.0 (/home/runner/work/NeMo-Fabric/NeMo-Fabric/crates/fabric-core)
�[1m�[92m Compiling�[0m fabric-cli v0.1.0 (/home/runner/work/NeMo-Fabric/NeMo-Fabric/crates/fabric-cli)
�[1m�[92m Finished�[0m `test` profile [unoptimized] target(s) in 6.17s
�[1m�[92m Running�[0m unittests src/main.rs (target/debug/deps/fabric-c3572a4d481471cf)
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
�[1m�[92m Running�[0m unittests src/lib.rs (target/debug/deps/fabric_core-9b081566c1d472c1)
running 41 tests
test config::tests::errors_for_unknown_manifest_profile ... ok
GitHub Actions: Rust / 0_Test.txt: feat: Add Relay integration to codex-cli adapter
Conclusion: failure
##[group]Run cargo test --workspace --locked
�[36;1mcargo test --workspace --locked�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
CARGO_INCREMENTAL: 0
CARGO_PROFILE_DEV_DEBUG: 0
CARGO_TERM_COLOR: always
RUST_BACKTRACE: short
RUSTFLAGS: -D warnings
CARGO_UNSTABLE_SPARSE_REGISTRY: true
CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse
CACHE_ON_FAILURE: false
##[endgroup]
�[1m�[92m Compiling�[0m fabric-core v0.1.0 (/home/runner/work/NeMo-Fabric/NeMo-Fabric/crates/fabric-core)
�[1m�[92m Compiling�[0m fabric-cli v0.1.0 (/home/runner/work/NeMo-Fabric/NeMo-Fabric/crates/fabric-cli)
�[1m�[92m Finished�[0m `test` profile [unoptimized] target(s) in 6.17s
�[1m�[92m Running�[0m unittests src/main.rs (target/debug/deps/fabric-c3572a4d481471cf)
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
�[1m�[92m Running�[0m unittests src/lib.rs (target/debug/deps/fabric_core-9b081566c1d472c1)
running 41 tests
test config::tests::errors_for_unknown_manifest_profile ... ok
GitHub Actions: Python / 0_Test.txt: feat: Add Relay integration to codex-cli adapter
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1muv sync --group test --no-group dev --extra hermes --extra relay�[0m
�[36;1muv run pytest�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
CARGO_INCREMENTAL: 0
CARGO_PROFILE_DEV_DEBUG: 0
CARGO_TERM_COLOR: always
RUST_BACKTRACE: short
RUSTFLAGS: -D warnings
CARGO_UNSTABLE_SPARSE_REGISTRY: true
CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse
CACHE_ON_FAILURE: false
UV_PYTHON_INSTALL_DIR: /home/runner/work/_temp/uv-python-dir
UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
##[endgroup]
Resolved 138 packages in 672ms
Downloading pydantic-core (2.0MiB)
Downloading cryptography (4.3MiB)
Downloading openai (1.1MiB)
Downloading pillow (6.8MiB)
Downloading pygments (1.2MiB)
Downloading uvloop (4.2MiB)
Downloading nemo-relay (6.0MiB)
Downloading hermes-agent (8.8MiB)
Downloaded pydantic-core
Downloaded uvloop
Downloaded pillow
Downloaded nemo-relay
Downloaded pygments
Downloaded cryptography
Downloaded openai
Downloaded hermes-agent
Prepared 67 packages in 893ms
Installed 67 packages in 82ms
+ annotated-doc==0.0.4
+ annotated-types==0.7.0
+ anyio==4.14.1
+ certifi==2026.5.20
+ cffi==2.0.0
+ charset-normalizer==3.4.7
+ click==8.4.2
+ coverage==7.14.3
+ croniter==6.0.0
+ cryptography==46.0.7
+ distro==1.9.0
+ fastapi==0.139.0
+ fire==0.7.1
+ h11==0.16.0
+ hermes-agent==0.18.0
+ httpcore==1.0.9
+ httptools==0.8.0
+ httpx==0.28.1
+ idna==3.18
+ iniconfig==2.3.0
+ jinja2==3.1.6
+ jiter==0.16.0
+ markdown==3.10.2
+ markdown-it-py==4.2.0
+ markupsafe==3.0.3
+ mdurl==0.1.2
+ nemo-relay==0.4.0
+ openai==2.24.0
+ packaging==26.0
+ pathspec==1.1.1
+ pillow==12.2.0
+ pluggy==1.6.0
+ prompt-toolkit==3.0.52
+ psutil==7.2.2
+ ptyprocess==0.7.0
+ pycparser==3.0
+ pydantic==2.13.4
+ pydantic-core==2.46.4
+ pygments==2.20.0
+ pyjwt==2.13.0
+ pytest==9.1.1
+ pyt...
GitHub Actions: Python / Test: feat: Add Relay integration to codex-cli adapter
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1muv sync --group test --no-group dev --extra hermes --extra relay�[0m
�[36;1muv run pytest�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
CARGO_INCREMENTAL: 0
CARGO_PROFILE_DEV_DEBUG: 0
CARGO_TERM_COLOR: always
RUST_BACKTRACE: short
RUSTFLAGS: -D warnings
CARGO_UNSTABLE_SPARSE_REGISTRY: true
CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse
CACHE_ON_FAILURE: false
UV_PYTHON_INSTALL_DIR: /home/runner/work/_temp/uv-python-dir
UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
##[endgroup]
Resolved 138 packages in 672ms
Downloading pydantic-core (2.0MiB)
Downloading cryptography (4.3MiB)
Downloading openai (1.1MiB)
Downloading pillow (6.8MiB)
Downloading pygments (1.2MiB)
Downloading uvloop (4.2MiB)
Downloading nemo-relay (6.0MiB)
Downloading hermes-agent (8.8MiB)
Downloaded pydantic-core
Downloaded uvloop
Downloaded pillow
Downloaded nemo-relay
Downloaded pygments
Downloaded cryptography
Downloaded openai
Downloaded hermes-agent
Prepared 67 packages in 893ms
Installed 67 packages in 82ms
+ annotated-doc==0.0.4
+ annotated-types==0.7.0
+ anyio==4.14.1
+ certifi==2026.5.20
+ cffi==2.0.0
+ charset-normalizer==3.4.7
+ click==8.4.2
+ coverage==7.14.3
+ croniter==6.0.0
+ cryptography==46.0.7
+ distro==1.9.0
+ fastapi==0.139.0
+ fire==0.7.1
+ h11==0.16.0
+ hermes-agent==0.18.0
+ httpcore==1.0.9
+ httptools==0.8.0
+ httpx==0.28.1
+ idna==3.18
+ iniconfig==2.3.0
+ jinja2==3.1.6
+ jiter==0.16.0
+ markdown==3.10.2
+ markdown-it-py==4.2.0
+ markupsafe==3.0.3
+ mdurl==0.1.2
+ nemo-relay==0.4.0
+ openai==2.24.0
+ packaging==26.0
+ pathspec==1.1.1
+ pillow==12.2.0
+ pluggy==1.6.0
+ prompt-toolkit==3.0.52
+ psutil==7.2.2
+ ptyprocess==0.7.0
+ pycparser==3.0
+ pydantic==2.13.4
+ pydantic-core==2.46.4
+ pygments==2.20.0
+ pyjwt==2.13.0
+ pytest==9.1.1
+ pyt...
🧰 Additional context used
📓 Path-based instructions (3)
.agents/skills/python-tests/**
⚙️ CodeRabbit configuration file
.agents/skills/python-tests/**: ---
name: python-tests
description: Python tests for Fabric; use this when writing tests
author: NVIDIA Corporation and Affiliates
license: Apache-2.0Python Test Style
- Pytest is used to run tests.
- Do not add
@pytest.mark.asyncioto any test. Async tests are automatically detected and run by the async runner; the decorator is unnecessary clutter.- Do not add a
-> Nonereturn type annotation to test functions. This is not a common convention in pytest and adds unnecessary verbosity.- When mocking a class, do not define a new class. Use
unittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.- The name of the mocked class should be prefixed with
mock, notfake.- Prefer pytest fixtures over helper methods.
- Do not repeat fixtures, if a fixture is needed in multiple test files, place it in a
conftest.pyfile.- When creating a fixture follow this pattern:
Only specify the scope argument when the value is something other than "function".`@pytest.fixture`(name="<fixture_name>"[, scope="<scope>"]) def <fixture_name>_fixture() -> <return_type>: ...- Prefer
pytest.mark.parametrizeover creating individual tests for
different input types.- If a fixture is needed for a test, but either does not return a value or the value is not used in the test, use the
@pytest.mark.usefixturesdecorator.Common Commands
# Focused test loop uv run pytest -k "<pattern>" # Run all tests uv run pytestReferences
pyproject.tomltests/conftest.py
Files:
.agents/skills/python-tests/SKILL.md
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 and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, useunittest.mock.MagicMockorunittest.mock.AsyncMock(withspecwhen needed) instead of defining a new class.
Name mocked classes with amockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it intests/conftest.py.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])with a matching<fixture_name>_fixture()function, and only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.
If a fixture is needed but does not return a value or its value is unused, use@pytest.mark.usefixtures.
When modifying environment variables in tests, useos.environ;tests/conftest.pyprovides an autouserestore_environ_fixturethat restores the environment after each test, somonkeypatch.setenvis unnecessary.
Files:
tests/test_codex_cli.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/test_codex_cli.py
🧬 Code graph analysis (1)
tests/test_codex_cli.py (1)
adapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.py (2)
run_codex(497-619)codex_home(290-292)
🪛 ast-grep (0.44.0)
tests/test_codex_cli.py
[info] 474-474: Do not hardcode temporary file or directory names
Context: "/tmp/codex-home"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
🪛 GitHub Actions: Python / 0_Test.txt
tests/test_codex_cli.py
[error] 280-280: Pytest failure in test_relay_requires_exact_true: AssertionError: expected 'relay_runtime' in result, but it was not found. FAILED tests/test_codex_cli.py::test_relay_requires_exact_true
🪛 GitHub Actions: Python / Test
tests/test_codex_cli.py
[error] 280-280: Test failed: test_relay_requires_exact_true. AssertionError: expected 'relay_runtime' to be present in result, but it was missing.
🪛 Ruff (0.15.20)
tests/test_codex_cli.py
[error] 475-475: Probable insecure usage of temporary file or directory: "/tmp/codex-home"
(S108)
[error] 476-476: Possible hardcoded password assigned to: "FABRIC_UNRELATED_SECRET"
(S105)
🔇 Additional comments (5)
.agents/skills/python-tests/SKILL.md (1)
28-28: LGTM!tests/test_codex_cli.py (4)
353-425: 🎯 Functional CorrectnessConfirm the Relay startup cleanup error-path test was added.
The prior review requested a test where
wait_for_relay_gatewayraises aftersubprocess.Popenreturns a process, assertingstop_relay_gatewayis invoked with that process for cleanup. The line-range summary for this block only mentions healthz polling, timeout raising, and stop-then-kill behavior — no mention of thestart_relay_gatewayfailure path calling cleanup. As per path instructions,{tests/**,python/tests/**}tests should cover error paths and lifecycle cleanup for the changed API surface.#!/bin/bash # Check whether start_relay_gateway's failure path (wait_for_relay_gateway raising) is tested with stop_relay_gateway assertions rg -n -B3 -A20 'def test_start_relay_gateway' tests/test_codex_cli.pySource: Path instructions
453-501: LGTM! Directos.environmanipulation here correctly relies on the autouserestore_environ_fixturefor cleanup, per coding guidelines.
667-671: LGTM!
6-9: LGTM!os.environusage forCODEX_HOMEandFABRIC_RELAY_ENABLEDcorrectly leverages the autouse env-restore fixture instead ofmonkeypatch.setenv.Also applies to: 41-44, 204-215
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@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/test_codex_cli.py`:
- Around line 301-305: The test in test_codex_cli should verify the actual relay
contract, not just that relay_runtime and relay_artifacts exist. Strengthen the
assertions around adapter.run_codex(codex_payload) by checking the expected
relay_runtime fields (enabled, emitter, mode, config_path) and the
relay_artifacts payload content, so malformed metadata from run_codex would
fail. Use the existing result object and command capture in this test to assert
the specific relay shape and values.
🪄 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: 4da3fe9b-dc63-4d43-815b-bdf6ec1a5ef4
📒 Files selected for processing (1)
tests/test_codex_cli.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Test
⚠️ CI failures not shown inline (2)
GitHub Actions: Rust / 0_Test.txt: feat: Add Relay integration to codex-cli adapter
Conclusion: failure
##[group]Run cargo test --workspace --locked
�[36;1mcargo test --workspace --locked�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
CARGO_INCREMENTAL: 0
CARGO_PROFILE_DEV_DEBUG: 0
CARGO_TERM_COLOR: always
RUST_BACKTRACE: short
RUSTFLAGS: -D warnings
CARGO_UNSTABLE_SPARSE_REGISTRY: true
CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse
CACHE_ON_FAILURE: false
##[endgroup]
�[1m�[92m Compiling�[0m fabric-core v0.1.0 (/home/runner/work/NeMo-Fabric/NeMo-Fabric/crates/fabric-core)
�[1m�[92m Compiling�[0m fabric-cli v0.1.0 (/home/runner/work/NeMo-Fabric/NeMo-Fabric/crates/fabric-cli)
�[1m�[92m Finished�[0m `test` profile [unoptimized] target(s) in 5.98s
�[1m�[92m Running�[0m unittests src/main.rs (target/debug/deps/fabric-c3572a4d481471cf)
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
�[1m�[92m Running�[0m unittests src/lib.rs (target/debug/deps/fabric_core-9b081566c1d472c1)
running 41 tests
test config::tests::errors_for_unknown_manifest_profile ... ok
GitHub Actions: Rust / Test: feat: Add Relay integration to codex-cli adapter
Conclusion: failure
##[group]Run cargo test --workspace --locked
�[36;1mcargo test --workspace --locked�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
CARGO_INCREMENTAL: 0
CARGO_PROFILE_DEV_DEBUG: 0
CARGO_TERM_COLOR: always
RUST_BACKTRACE: short
RUSTFLAGS: -D warnings
CARGO_UNSTABLE_SPARSE_REGISTRY: true
CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse
CACHE_ON_FAILURE: false
##[endgroup]
�[1m�[92m Compiling�[0m fabric-core v0.1.0 (/home/runner/work/NeMo-Fabric/NeMo-Fabric/crates/fabric-core)
�[1m�[92m Compiling�[0m fabric-cli v0.1.0 (/home/runner/work/NeMo-Fabric/NeMo-Fabric/crates/fabric-cli)
�[1m�[92m Finished�[0m `test` profile [unoptimized] target(s) in 5.98s
�[1m�[92m Running�[0m unittests src/main.rs (target/debug/deps/fabric-c3572a4d481471cf)
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
�[1m�[92m Running�[0m unittests src/lib.rs (target/debug/deps/fabric_core-9b081566c1d472c1)
running 41 tests
test config::tests::errors_for_unknown_manifest_profile ... ok
🧰 Additional context used
📓 Path-based instructions (2)
**/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
**/tests/**/*.py: Use Pytest for Python tests; do not add@pytest.mark.asyncioto tests because async tests are automatically detected by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecargument when necessary.
Name mocked classes with amockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the function as<fixture_name>_fixture() -> <return_type>; only specifyscopewhen it is not"function".
Preferpytest.mark.parametrizeover creating individual tests for different input types.
If a fixture is needed for a test but does not return a value or its value is unused, use@pytest.mark.usefixtures.
If a test needs to modify environment variables, useos.environ;tests/conftest.pyprovides an autouserestore_environ_fixturethat restores the environment after each test, somonkeypatch.setenvis unnecessary.
Files:
tests/test_codex_cli.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/test_codex_cli.py
🧬 Code graph analysis (1)
tests/test_codex_cli.py (1)
adapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.py (1)
run_codex(497-619)
AjayThorve
left a comment
AjayThorve
left a comment
There was a problem hiding this comment.
The intended Codex + Relay flow works end to end, including gateway lifecycle and ATOF/ATIF generation. Requesting changes only for the cross-run artifact contamination above.
…d-codex Signed-off-by: David Gardner <dagardner@nvidia.com>
…file Signed-off-by: David Gardner <dagardner@nvidia.com>
|
📖 Fern docs preview: https://nvidia-preview-pull-request-30.docs.buildwithfern.com/nemo/fabric |
Signed-off-by: David Gardner <dagardner@nvidia.com>
2 participants
nemo-relayin gateway mode independently of Codex~/.codex)fabric run examples/code-review-agent \ --profile hermes_sdk \ --profile relay_otel \ --input "who are you?"load_payloadandruntime_session_idtocommon/utils.pywrite_relay_plugins_tomlnow writes both a config.toml and a plugins.toml, the method was renamed towrite_relay_configsSummary by CodeRabbit