fix: wait for Relay ATIF finalization - #181
Conversation
Signed-off-by: Ajay Thorve <athorve@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:
WalkthroughClaude and Codex adapters now track local Relay ATIF artifacts. They wait for finalized JSON output after successful turns. Finalization timeouts return adapter-specific non-retryable errors and make the runtime unusable. Tests cover delayed output, stale files, timeouts, and runtime isolation. ChangesRelay ATIF synchronization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant InvocationAdapter
participant Runtime
participant RuntimeDirectory
InvocationAdapter->>RuntimeDirectory: Snapshot local ATIF paths
InvocationAdapter->>Runtime: Execute Claude or Codex turn
Runtime-->>InvocationAdapter: Return successful completion
InvocationAdapter->>RuntimeDirectory: Poll for finalized JSON artifact
RuntimeDirectory-->>InvocationAdapter: Return artifact path or timeout
InvocationAdapter->>Runtime: Mark unusable on timeout
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/common/src/nemo_fabric_adapters/common/relay_artifacts.py`:
- Around line 43-73: Update
adapters/common/src/nemo_fabric_adapters/common/relay_artifacts.py lines 43-73
so snapshot_atif_paths records a per-path fingerprint including inode, size, and
nanosecond modification time, and _finalized_atif_path accepts paths that are
new or whose fingerprint differs from before while preserving the complete-JSON
validation. Update tests/adapters/test_adapters_common_relay_artifacts.py lines
54-94 to replace the assertion rejecting modified existing files with coverage
confirming a post-snapshot modification is recognized as finalized.
In `@tests/adapters/test_claude_adapter.py`:
- Around line 912-921: Update the timeout test assertions around the ATIF
response to explicitly verify that output["failed"] is True, alongside the
existing output["error"] and unavailable-state assertions. Use the Claude
timeout test’s output assertion block and preserve all existing checks.
🪄 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: 9fcff026-e3ae-484f-a284-c6d3c4ae194e
📒 Files selected for processing (6)
adapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pytests/adapters/test_adapters_common_relay_artifacts.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.py
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (17)
**/*.{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_adapters_common_relay_artifacts.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_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_adapters_common_relay_artifacts.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_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_adapters_common_relay_artifacts.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_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_adapters_common_relay_artifacts.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_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_adapters_common_relay_artifacts.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_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_adapters_common_relay_artifacts.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_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_adapters_common_relay_artifacts.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_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_adapters_common_relay_artifacts.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_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_adapters_common_relay_artifacts.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_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_adapters_common_relay_artifacts.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_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_adapters_common_relay_artifacts.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_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_adapters_common_relay_artifacts.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_adapter.py
tests/adapters/test_*.py
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
tests/adapters/test_*.py: Add focused tests for descriptor shape and exact capabilities, positive normalized-surface mappings, rejection of unsupported values and unenforceable policies, result normalization without secret leakage, one-shot execution, continuation, runtime isolation, and the packaged subprocess entry point.
Provide a credential-free fixture coveringplan,doctor, andrun; keep credential-dependent live-harness tests opt-in while retaining a deterministic CI end-to-end path.
Files:
tests/adapters/test_adapters_common_relay_artifacts.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_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_adapters_common_relay_artifacts.pytests/adapters/test_codex_adapter.pytests/adapters/test_claude_adapter.py
adapters/**/*
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Place repository adapters under
adapters/<name>, define their install extra and packaged descriptor, select the matchingharness.resolutionstrategy, and document repository orbase_dirdescriptor discovery.
Files:
adapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
adapters/**/*.{py,js,ts,java,go,rs}
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
adapters/**/*.{py,js,ts,java,go,rs}: Use the existing Fabricpythonorprocessrunner and normalized request/result contracts. Reuseadapters/common/only when its contract fits; do not add a runner or abstraction for one adapter.
For adapters consuming normalized configuration or runtime context, use the complete Fabric invocation and treatconfig,capability_plan,telemetry_plan, andruntime_contextas authoritative. Reserveharness.settingsfor harness-specific behavior.
Apply configuration precedence in this order: normalized config; Fabric-resolved plans and runtime context; harness-specific settings; descriptor and adapter defaults. Reject conflicting duplicate declarations or unsupported behavior with actionable field-specific errors; never silently drop configuration.
Run dependency and authentication preflight before invoking the harness. Declare fixed dependencies in descriptor requirements, validate versions, hooks, and credentials, and never expose credential values in output, errors, events, logs, or fixtures.
Forward only required system variables, selected credential variables, telemetry variables, and documented harness-specific environment; never forward or log unrelated environment values.
Run one local adapter host per Fabric runtime for orderedstart→invoke*→stop. Emit one JSON lifecycle response per request on stdout and diagnostics on stderr; treat early exit as a host crash.
Return harness-level invoke failures in a successful lifecycle response withresponse: null,failed: true, and structurederrorcontainingcode,message,retryable, and optionalmetadata.
Do not emit NeMo Relay stream records on adapter stdout. Relay records are sent out of band through the SDK-owned NDJSON ATOF endpoint; the adapter must return exactly one terminal lifecycle response.
Scope workspace, generated configuration, state, sessions, and artifacts to the resolved runtime context. Write artifacts only within the resolved r...
Files:
adapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pyadapters/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/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/relay_artifacts.pyadapters/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/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/codex/src/nemo_fabric_adapters/codex/adapter.py
🪛 ast-grep (0.45.0)
tests/adapters/test_adapters_common_relay_artifacts.py
[info] 77-77: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"schema_version": "ATIF-v1.7", "steps": []})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 122-122: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"schema_version": "ATIF-v1.7", "steps": []})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
tests/adapters/test_claude_adapter.py
[info] 825-825: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"schema_version": "ATIF-v1.7", "steps": []})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.16.1)
tests/adapters/test_adapters_common_relay_artifacts.py
[warning] 73-73: Missing return type annotation for private function finish_candidate
Add return type annotation: None
(ANN202)
[warning] 119-119: Missing return type annotation for private function write_atif
(ANN202)
adapters/codex/src/nemo_fabric_adapters/codex/adapter.py
[warning] 35-35: Use from nemo_fabric_adapters.common import relay_artifacts in lieu of alias
Replace with from nemo_fabric_adapters.common import relay_artifacts
(PLR0402)
[warning] 1096-1102: Abstract raise to an inner function
(TRY301)
tests/adapters/test_claude_adapter.py
[warning] 823-823: Missing return type annotation for private function write_atif
Add return type annotation: None
(ANN202)
🔇 Additional comments (6)
adapters/common/src/nemo_fabric_adapters/common/relay_artifacts.py (1)
1-40: LGTM!tests/adapters/test_adapters_common_relay_artifacts.py (1)
1-51: LGTM!Also applies to: 96-149
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py (1)
35-35: LGTM!Also applies to: 886-886
adapters/codex/src/nemo_fabric_adapters/codex/adapter.py (1)
35-35: LGTM!Also applies to: 1070-1070
tests/adapters/test_claude_adapter.py (1)
755-854: LGTM!tests/adapters/test_codex_adapter.py (1)
648-671: LGTM!
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
AnuradhaKaruppiah
left a comment
There was a problem hiding this comment.
LGTM. left one minor agent suggested comment.
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
adapters/codex/src/nemo_fabric_adapters/codex/adapter.py (1)
1078-1104: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not collect Relay artifacts after an ATIF timeout.
When
wait_for_finalized_atifreturnsNone, this branch creates a timeout failure. The method then continues to_relay_output, which rescans the Relay artifact directory. A stale artifact, or an artifact written just after the deadline, can therefore be attached to the timeout response even though it was not finalized for this turn.Skip artifact collection for this failure, or restrict collection to the finalized artifact returned by the wait logic. Add a regression test with a pre-existing artifact and assert that the timeout response does not expose it.
🤖 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 `@adapters/codex/src/nemo_fabric_adapters/codex/adapter.py` around lines 1078 - 1104, Prevent the timeout failure raised in the ATIF finalization block around wait_for_finalized_atif from reaching _relay_output artifact rescanning. Track this specific timeout condition and skip Relay artifact collection, or pass only a finalized artifact when available, so pre-existing or late-written files are not attached. Add a regression test covering a pre-existing artifact and assert the timeout response excludes it.
🤖 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.
Outside diff comments:
In `@adapters/codex/src/nemo_fabric_adapters/codex/adapter.py`:
- Around line 1078-1104: Prevent the timeout failure raised in the ATIF
finalization block around wait_for_finalized_atif from reaching _relay_output
artifact rescanning. Track this specific timeout condition and skip Relay
artifact collection, or pass only a finalized artifact when available, so
pre-existing or late-written files are not attached. Add a regression test
covering a pre-existing artifact and assert the timeout response excludes it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 4d086bfa-1c5c-4de7-9027-012418ef7b1d
📒 Files selected for processing (2)
adapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_codex_adapter.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (17)
**/*.{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/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_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/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_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:
adapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_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:
adapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_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:
adapters/codex/src/nemo_fabric_adapters/codex/adapter.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:
adapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_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:
adapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_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:
adapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_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:
adapters/codex/src/nemo_fabric_adapters/codex/adapter.pytests/adapters/test_codex_adapter.py
adapters/**/*
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Place repository adapters under
adapters/<name>, define their install extra and packaged descriptor, select the matchingharness.resolutionstrategy, and document repository orbase_dirdescriptor discovery.
Files:
adapters/codex/src/nemo_fabric_adapters/codex/adapter.py
adapters/**/*.{py,js,ts,java,go,rs}
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
adapters/**/*.{py,js,ts,java,go,rs}: Use the existing Fabricpythonorprocessrunner and normalized request/result contracts. Reuseadapters/common/only when its contract fits; do not add a runner or abstraction for one adapter.
For adapters consuming normalized configuration or runtime context, use the complete Fabric invocation and treatconfig,capability_plan,telemetry_plan, andruntime_contextas authoritative. Reserveharness.settingsfor harness-specific behavior.
Apply configuration precedence in this order: normalized config; Fabric-resolved plans and runtime context; harness-specific settings; descriptor and adapter defaults. Reject conflicting duplicate declarations or unsupported behavior with actionable field-specific errors; never silently drop configuration.
Run dependency and authentication preflight before invoking the harness. Declare fixed dependencies in descriptor requirements, validate versions, hooks, and credentials, and never expose credential values in output, errors, events, logs, or fixtures.
Forward only required system variables, selected credential variables, telemetry variables, and documented harness-specific environment; never forward or log unrelated environment values.
Run one local adapter host per Fabric runtime for orderedstart→invoke*→stop. Emit one JSON lifecycle response per request on stdout and diagnostics on stderr; treat early exit as a host crash.
Return harness-level invoke failures in a successful lifecycle response withresponse: null,failed: true, and structurederrorcontainingcode,message,retryable, and optionalmetadata.
Do not emit NeMo Relay stream records on adapter stdout. Relay records are sent out of band through the SDK-owned NDJSON ATOF endpoint; the adapter must return exactly one terminal lifecycle response.
Scope workspace, generated configuration, state, sessions, and artifacts to the resolved runtime context. Write artifacts only within the resolved r...
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
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
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/adapters/test_*.py
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
tests/adapters/test_*.py: Add focused tests for descriptor shape and exact capabilities, positive normalized-surface mappings, rejection of unsupported values and unenforceable policies, result normalization without secret leakage, one-shot execution, continuation, runtime isolation, and the packaged subprocess entry point.
Provide a credential-free fixture coveringplan,doctor, andrun; keep credential-dependent live-harness tests opt-in while retaining a deterministic CI end-to-end path.
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
🧠 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
🔇 Additional comments (3)
adapters/codex/src/nemo_fabric_adapters/codex/adapter.py (2)
35-35: LGTM!
1068-1071: LGTM!tests/adapters/test_codex_adapter.py (1)
139-179: LGTM!Also applies to: 591-632, 635-677
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
dagardner-nv
left a comment
There was a problem hiding this comment.
Approving for deps
|
Fern docs preview: https://nvidia-preview-pull-request-181.docs.buildwithfern.com/nemo/fabric |
|
/merge |
#### Overview Forward-port the Relay ATIF finalization fix from #181 to main. Relay can report an agent turn complete before its asynchronous ATIF subscriber has finished writing the trajectory. Codex and Claude previously collected Relay artifacts immediately after that terminal result, so a successful invocation could include ATOF while missing its ATIF. This PR adds a shared finalization barrier for Codex and Claude. It does not change package versions, dependency constraints, manifests, lockfiles, or the observability schema. #### Details - Snapshot cheap filesystem fingerprints (device, inode, size, and nanosecond modification time) for runtime-scoped ATIF files before each invocation. - After a successful Codex or Claude turn, inspect only new or changed candidates and accept one when it contains a complete JSON object. - Preserve support for new files, atomic replacement, overwrite, and append without reading unchanged trajectories from earlier turns. - Use a five-second condition-based deadline rather than a fixed sleep. - On timeout, return a non-retryable adapter error with no Relay artifacts, mark the persistent runtime unavailable, and bypass later directory rescanning so a late ATIF cannot be attributed to another response. - Skip the local-file wait for remote-only ATIF storage. - Preserve streaming, ATOF collection, persistent SDK sessions, and the runtime-owned Relay gateway. - Leave Deep Agents and Hermes behavior unchanged; their existing lifecycle boundaries already finalize Relay output before collection. No public API or configuration contract changes, and there are no breaking changes. #### Validation - just test-python — 662 passed, 17 skipped. - Ruff formatting check on all changed Python files — passed. - Pre-commit on all files — passed, including copyright, Ruff, GitHub Actions lint, Cargo/uv lock freshness, both attribution checks, and dependency license diff. - git diff --check upstream/main...HEAD — passed. - Platform evaluator on the installed main packages at parallelism 10: Codex 10/10 and Claude 10/10, with 20 isolated runtime-scoped ATIF directories and zero contract failures. - Platform non-target regression at parallelism 5: Deep Agents 5/5 and Hermes 5/5, with ten isolated directories, valid ATIF-v1.7 trajectories, and zero contract failures. Rust tests were not rerun because this PR changes only Python adapter behavior and tests. Documentation is unchanged because public configuration and API contracts are unchanged. #### Where should the reviewer start? Start with adapters/common/src/nemo_fabric_adapters/common/relay_artifacts.py, then the calls from CodexRuntime.invoke() and ClaudeRuntime.invoke(). The focused tests cover delayed and partial writes, new and changed paths, remote-only storage, timeout behavior, late-file exclusion, unusable runtimes, and parallel runtime-directory isolation. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to #181 and the NeMo Platform ATIF release blocker. - [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. ## Summary by CodeRabbit * **New Features** * Added reliable collection of Relay-generated ATIF artifacts for Claude and Codex interactions. * Delayed artifact creation is now detected and finalized before results are returned. * **Bug Fixes** * Incomplete or unchanged artifacts are ignored. * Artifact finalization timeouts now produce clear, non-retryable failures and prevent further use of the affected runtime. * **Tests** * Added coverage for delayed artifacts, timeout handling, modified files, and runtime isolation. Authors: - Ajay Thorve (https://github.com/AjayThorve) Approvers: - Zhongxuan (Daniel) Wang (https://github.com/zhongxuanwang-nv) URL: #187
#### Overview Relay hook responses can return before the terminal scope event reaches observability subscribers. Consumers that read ATIF immediately after the hook response can therefore observe ATOF but miss the final ATIF trajectory. This change attaches a completion receipt to the exact terminal scope event. Hook handling waits for that receipt only after releasing session-manager locks. It does not drain work queued later by another session. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Add an internal `SubscriberDelivery` receipt for one queued event. - Return receipts from terminal turn, session, and subagent scope closures. - Await receipts after releasing shared session and alignment locks. - Preserve the shared FIFO dispatcher: work already ahead can delay a receipt, but later unrelated session work is not captured. - Preserve full server teardown ordering: close all sessions, globally flush subscribers, then clear plugins. - Keep streaming and public schema behavior unchanged. - Document the hook-response and full-shutdown delivery guarantees. No Fabric artifact polling or retry hotfix is required. Fabric still needs its separate Relay 0.7 and schema-v3 compatibility work. Validation: - `just test-rust` - `CARGO_INCREMENTAL=0 just test-python` — 639 passed - `XDG_CONFIG_HOME=<isolated> just test-go` - `PATH=<Node 24>:$PATH just test-node` — 353 passed - `CARGO_INCREMENTAL=0 cargo clippy --workspace --all-targets -- -D warnings` - `just docs` - `uv run pre-commit run --all-files` - Fabric Codex Relay E2E: one-shot ATOF and ATIF visibility plus two streamed turns - Platform live evaluator E2E: `AgentEvaluator -> Fabric -> Codex -> Relay`, with ATIF available immediately for metric evaluation - Platform parallel evaluator E2E: two concurrent trials produced two isolated ATIF files #### Where should the reviewer start? Start with `SubscriberDelivery` in `crates/core/src/api/runtime/subscriber_dispatcher.rs` and `pop_scope_with_subscriber_delivery` in `crates/core/src/api/scope.rs`. Then review `SessionManager::apply_events` in `crates/cli/src/sessions/mod.rs`, followed by the terminal-response and shutdown tests in `crates/cli/tests/coverage/shared/server_tests.rs`. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to NVIDIA/NeMo-Fabric#181 ## Summary by CodeRabbit - **New Features** - Hook responses now wait for related turn, subagent, or session-end events to finish processing. - Session and turn closures provide more reliable completion handling. - **Bug Fixes** - Improved event delivery ordering and error reporting. - Events blocked in one session no longer prevent other sessions from processing. - Terminal snapshots and events are flushed during server shutdown. - **Documentation** - Updated lifecycle documentation to explain hook completion and shutdown behavior. Authors: - Ajay Thorve (https://github.com/AjayThorve) Approvers: - Will Killian (https://github.com/willkill07) URL: #727
Overview
Fix the Relay artifact lifecycle race for both Codex and Claude without
upgrading Relay or changing the observability schema. Fabric now waits for the
configured local ATIF exporter to finish before it snapshots relay_artifacts,
so a successful invocation cannot be returned while its trajectory is still in
Relay's asynchronous subscriber queue.
The readiness logic is shared in adapters/common. The existing
nemo-relay>=0.6.0,<0.7 contract remains unchanged, so this fix is suitable for
the release/0.1 line and supersedes the Relay 0.7 dependency-upgrade mitigation
in #180.
This branch also sets the release version to 0.1.1 across the Rust workspace,
all seven Python distributions, exact internal dependency pins, and lockfiles.
Details
mtime) for runtime-scoped ATIF files before an invocation.
metadata-changed paths and return as soon as one contains a complete JSON
object. This supports a new file, atomic replacement, overwrite, and append.
the final path; file existence or timestamps do not prove the write is
complete. Metadata selects candidates, while successful JSON parsing proves
readiness.
codex_relay_atif_timeout or claude_relay_atif_timeout and makes the persistent
runtime unavailable, preventing a late artifact from being attributed to a
later turn.
on the later runtime-unavailable response.
Relay does not produce a local ATIF in that mode.
runtime-owned Relay gateway.
Codex and Claude need this guard because both collect artifacts immediately
after an SDK terminal result while Relay hook events are dispatched
asynchronously. Hermes already flushes subscribers before collection, and Deep
Agents collects after leaving its Relay plugin context, so those adapters are
unchanged.
Runtime-scoped output directories keep parallel evaluator sessions isolated.
The wait is condition-based and normally adds only the time Relay needs to
finish the file.
Release packaging
versions are unchanged; the locks add newly published Relay 0.6 source and
musllinux artifact records.
aiohttp license-file path.
Validation
0.1.1 pins were verified.
Cargo/uv lock freshness, generated attributions, and dependency-license
checks.
persistent two-turn streaming runtimes.
with isolated runtime-scoped ATIF directories, valid ATIF-v1.7 trajectories,
non-empty steps, and zero contract failures.
with 20 distinct runtime-scoped ATIF directories and zero failures.
Hermes 5/5 against Relay 0.6, with ten distinct runtime-scoped directories,
valid trajectories and promoted trace evidence, and zero failures.
Documentation is unchanged because public configuration and dependency
contracts are unchanged. There are no breaking changes.
Where should the reviewer start?
Start with
adapters/common/src/nemo_fabric_adapters/common/relay_artifacts.py, then the
calls from CodexRuntime.invoke() and ClaudeRuntime.invoke(). The focused tests
cover delayed and partial writes, unchanged-path exclusion, changed-path
detection, remote-only configuration, the hard timeout, no-rescan timeout
behavior, unusable-runtime behavior, and parallel runtime-directory isolation.
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Relates to fix: require Relay 0.7 for coding-agent gateways #180 and the NeMo Platform 0.4 ATIF release blocker.
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.
Summary by CodeRabbit
New Features
Bug Fixes