feat: add Harbor multi-harness SDK demo - #29
Conversation
|
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:
WalkthroughFabricAgent now writes a Fabric spec and runs a Python SDK runner inside the task environment. The Harbor demo task adds scripted, Hermes, Codex, and telemetry profiles plus task assets, docs, and tests for the new flow. ChangesHarbor SDK Runner Refactor and Demo Task
Sequence Diagram(s)sequenceDiagram
participant Harbor as Harbor CLI
participant FabricAgent
participant runner as runner.py (in-task)
participant FabricClient
participant Harness as Harness Adapter
Harbor->>FabricAgent: run(request, context)
FabricAgent->>FabricAgent: write spec → /tmp/fabric-run.json
FabricAgent->>runner: python3 -m nemo_fabric.integrations.harbor.runner --spec --result
runner->>runner: load_sources() → FabricConfig + profiles
runner->>FabricClient: FabricClient().run(config, profiles, RunRequest)
FabricClient->>Harness: dispatch to selected harness adapter
Harness-->>FabricClient: adapter result
FabricClient-->>runner: result mapping
runner->>runner: write /logs/agent/fabric-result.json
runner-->>FabricAgent: exit 0
FabricAgent->>Harbor: context.metadata["fabric"] populated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python/src/nemo_fabric/integrations/harbor/__init__.py (1)
58-60: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCreate the configured spec directory instead of hard-coding
/tmp.
fabric_spec_pathis now a constructor parameter, but setup still only creates/tmp. Any caller that overrides the spec path to a different directory will fail at Line 82 before the runner starts. The mkdir step should derive the parent fromself.fabric_spec_pathrather than assuming the default path.Also applies to: 81-86
🤖 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 `@python/src/nemo_fabric/integrations/harbor/__init__.py` around lines 58 - 60, The setup flow in Harbor is still hard-coding /tmp when preparing directories, so callers that pass a custom fabric_spec_path can fail before the runner starts. Update the async setup method in the Harbor integration to derive the spec directory from self.fabric_spec_path and create its parent directory instead of assuming /tmp, while keeping the existing /logs/agent creation and ensure_success handling unchanged.
🤖 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 `@integrations/harbor/demo/README.md`:
- Around line 104-113: The Harbor demo README step uses the `open` command in
the Phoenix startup sequence, which is macOS-only and breaks the reviewer-run
flow on Linux. Update the instructions around the Phoenix launch/check step to
either use a platform-aware opener in the same block or replace the `open
http://localhost:6006` line with a manual instruction to visit the URL so the
demo works cross-platform.
In `@integrations/harbor/demo/task/environment/Dockerfile`:
- Around line 8-10: The Dockerfile’s rustup installation step pipes curl
directly into sh, which can mask download failures under the default shell
behavior. Update the RUN step in the Rust toolchain install block to avoid a
plain curl | sh pipeline by separating the download from execution, checking the
download result before invoking sh, or otherwise making the step fail on any
curl error. Use the existing rustup install stanza and the PATH setup nearby to
keep the change localized.
In `@integrations/harbor/demo/task/environment/fabric/profiles/telemetry.yaml`:
- Around line 33-36: The telemetry profile hardcodes the Phoenix endpoint host
in the openinference configuration, which makes the demo fragile on Linux
reviewer setups. Update the openinference endpoint in the telemetry profile to
read the Phoenix host/URL from config or an environment variable, and use that
value wherever the endpoint is assembled so it no longer depends on
host.docker.internal. If you keep a default, ensure it can be overridden cleanly
by the demo/runtime configuration.
In `@integrations/harbor/demo/task/solution/solve.sh`:
- Around line 3-8: The patch application in solve.sh is too permissive because
Path.read_text().replace(...) can silently do nothing if calculator.py no longer
contains the exact target text. Update the script to verify the expected pattern
exists before writing, and make it fail immediately if the replacement target is
missing; use the Path handling logic in the inline Python snippet and the
calculator.py rewrite step as the place to enforce this check.
In `@tests/test_harbor_runner.py`:
- Around line 43-49: Avoid keeping load_codex_adapter() as a module-level helper
in the test file; move this setup into the single test that uses it or convert
it into a pytest fixture so the file matches the repo’s fixture-based
conventions. Update the caller to use the fixture or inline the import-loading
logic, and keep the behavior in tests/test_harbor_runner.py centered around the
existing load_codex_adapter symbol.
- Around line 52-96: The current load_sources coverage only verifies the happy
path, so add failure-path assertions in
test_runner_loads_typed_sources_and_applies_harbor_model or a nearby test for
missing config/profile files and malformed YAML inputs. Exercise the runner’s
file-loading boundary through
nemo_fabric.integrations.harbor.runner.load_sources, and assert it raises the
expected error when config_path or profile_paths point to absent/invalid
documents. Keep the new cases aligned with the existing typed-source setup so
the tests validate the same API surface under failure conditions.
---
Outside diff comments:
In `@python/src/nemo_fabric/integrations/harbor/__init__.py`:
- Around line 58-60: The setup flow in Harbor is still hard-coding /tmp when
preparing directories, so callers that pass a custom fabric_spec_path can fail
before the runner starts. Update the async setup method in the Harbor
integration to derive the spec directory from self.fabric_spec_path and create
its parent directory instead of assuming /tmp, while keeping the existing
/logs/agent creation and ensure_success handling unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 60bdebe9-bf44-4bbf-ba12-7fbfb4cef8ae
📒 Files selected for processing (24)
.gitignoreREADME.mdintegrations/harbor/README.mdintegrations/harbor/demo/README.mdintegrations/harbor/demo/task/environment/Dockerfileintegrations/harbor/demo/task/environment/calculator.pyintegrations/harbor/demo/task/environment/fabric/adapters/scripted/fabric-adapter.jsonintegrations/harbor/demo/task/environment/fabric/adapters/scripted/run.pyintegrations/harbor/demo/task/environment/fabric/agent.yamlintegrations/harbor/demo/task/environment/fabric/profiles/codex.yamlintegrations/harbor/demo/task/environment/fabric/profiles/hermes.yamlintegrations/harbor/demo/task/environment/fabric/profiles/smoke.yamlintegrations/harbor/demo/task/environment/fabric/profiles/telemetry.yamlintegrations/harbor/demo/task/instruction.mdintegrations/harbor/demo/task/solution/solve.shintegrations/harbor/demo/task/task.tomlintegrations/harbor/demo/task/tests/test.shintegrations/harbor/demo/task/tests/verify.pypyproject.tomlpython/src/nemo_fabric/integrations/harbor/README.mdpython/src/nemo_fabric/integrations/harbor/__init__.pypython/src/nemo_fabric/integrations/harbor/runner.pypython/tests/smoke_harbor_integration.pytests/test_harbor_runner.py
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: Rust / Test: Add Harbor multi-harness SDK demo
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 8.74s
�[1m�[92m Running�[0m unittests src/main.rs (target/debug/deps/fabric-d6b0cd643d758f74)
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-039dd9c04deba413)
running 41 tests
test config::tests::errors_for_unknown_manifest_profile ... ok
GitHub Actions: Rust / 0_Test.txt: Add Harbor multi-harness SDK demo
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 8.74s
�[1m�[92m Running�[0m unittests src/main.rs (target/debug/deps/fabric-d6b0cd643d758f74)
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-039dd9c04deba413)
running 41 tests
test config::tests::errors_for_unknown_manifest_profile ... ok
🧰 Additional context used
📓 Path-based instructions (4)
python/src/nemo_fabric/**/*
⚙️ CodeRabbit configuration file
python/src/nemo_fabric/**/*: Review Python SDK changes for typed API consistency, import-time dependency neutrality, async/session behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/src/nemo_fabric/integrations/harbor/README.mdpython/src/nemo_fabric/integrations/harbor/runner.pypython/src/nemo_fabric/integrations/harbor/__init__.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_harbor_runner.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_harbor_runner.pypython/tests/smoke_harbor_integration.py
{docs/**,README.md,AGENTS.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,AGENTS.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency with generated schemas.
Files:
README.md
🧠 Learnings (2)
📚 Learning: 2026-06-29T22:34:52.407Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 27
File: adapters/codex-cli/fabric-adapter.json:13-15
Timestamp: 2026-06-29T22:34:52.407Z
Learning: In NeMo-Fabric adapter manifest files (e.g., `*/fabric-adapter.json`), keep `config.accepts` limited to the top-level Fabric capability sections that `resolve_capability_plan` consumes (such as `models`, `tools`, `mcp`, `skills`, `telemetry`). Do not add adapter-owned `harness.settings` keys to `config.accepts`; `harness.settings` should remain adapter-owned and be passed through unchanged.
Applied to files:
integrations/harbor/demo/task/environment/fabric/adapters/scripted/fabric-adapter.json
📚 Learning: 2026-06-28T04:03:32.877Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 26
File: python/tests/smoke_typed_config.py:163-177
Timestamp: 2026-06-28T04:03:32.877Z
Learning: In NVIDIA NeMo Fabric Python SDK serialization of `RuntimeCapabilities` (to satisfy the “parity contract” with Rust core and the CLI), do not emit metadata keys when the corresponding metadata is absent. Instead, omit those fields entirely so the produced JSON matches the Rust/CLI output (e.g., avoid `null`, empty objects, or placeholder metadata). During review, verify the serializer/builders follow this omission rule and that Python outputs/parity tests reflect the same shape.
Applied to files:
python/src/nemo_fabric/integrations/harbor/runner.pypython/tests/smoke_harbor_integration.pypython/src/nemo_fabric/integrations/harbor/__init__.py
🧬 Code graph analysis (2)
python/src/nemo_fabric/integrations/harbor/runner.py (1)
python/src/nemo_fabric/integrations/harbor/__init__.py (1)
name(53-54)
tests/test_harbor_runner.py (1)
python/src/nemo_fabric/integrations/harbor/__init__.py (1)
name(53-54)
🪛 ast-grep (0.44.0)
integrations/harbor/demo/task/environment/fabric/adapters/scripted/run.py
[info] 18-24: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"harness": "scripted",
"response": "Fixed multiply(a, b) in /app/calculator.py",
"request_id": payload["request"]["request_id"],
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
python/src/nemo_fabric/integrations/harbor/runner.py
[info] 74-74: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result, indent=2)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
tests/test_harbor_runner.py
[info] 95-95: use jsonify instead of json.dumps for JSON output
Context: json.dumps(config.to_mapping())
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[warning] 187-187: Do not make http calls without encryption
Context: "http://host.docker.internal:6006/v1/traces"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
python/tests/smoke_harbor_integration.py
[info] 146-146: Do not hardcode temporary file or directory names
Context: "/tmp/fabric-run.json"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
python/src/nemo_fabric/integrations/harbor/__init__.py
[info] 32-32: Do not hardcode temporary file or directory names
Context: "/tmp/fabric-run.json"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
[info] 125-125: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload, indent=2)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Checkov (3.3.2)
integrations/harbor/demo/task/environment/Dockerfile
[low] 1-19: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-19: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🪛 GitHub Actions: Python / 0_Test.txt
python/tests/smoke_harbor_integration.py
[error] 145-145: smoke_harbor_integration.py crashed while executing: asyncio.run(main()) -> await agent.run("fix the bug", environment, context).
python/src/nemo_fabric/integrations/harbor/__init__.py
[error] 95-95: Test failed with AttributeError: 'FabricAgent' object has no attribute 'extra_env' when calling FabricAgent.run (env=self.extra_env).
🪛 GitHub Actions: Python / Test
python/tests/smoke_harbor_integration.py
[error] 145-145: Test failed because agent.run(...) raised AttributeError in harbor integration code. Traceback points to FabricAgent.run calling env=self.extra_env.
python/src/nemo_fabric/integrations/harbor/__init__.py
[error] 95-95: Runtime failure: AttributeError: 'FabricAgent' object has no attribute 'extra_env' (step failed during Harbor integration agent.run).
🪛 Hadolint (2.14.0)
integrations/harbor/demo/task/environment/Dockerfile
[warning] 3-3: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>
(DL3008)
[warning] 8-8: Set the SHELL option -o pipefail before RUN with a pipe in it. If you are using /bin/sh in an alpine image or if your shell is symlinked to busybox then consider explicitly setting your SHELL to /bin/ash, or disable this check
(DL4006)
🪛 Ruff (0.15.20)
integrations/harbor/demo/task/tests/verify.py
[warning] 9-9: Assertion should be broken down into multiple parts
Break down assertion into multiple parts
(PT018)
python/src/nemo_fabric/integrations/harbor/runner.py
[warning] 49-49: Prefer TypeError exception for invalid type
(TRY004)
[warning] 49-49: Avoid specifying long messages outside the exception class
(TRY003)
tests/test_harbor_runner.py
[warning] 46-46: Assertion should be broken down into multiple parts
Break down assertion into multiple parts
(PT018)
python/tests/smoke_harbor_integration.py
[warning] 79-79: Unused method argument: cwd
(ARG002)
[warning] 80-80: Unused method argument: timeout_sec
(ARG002)
[warning] 82-82: Dynamically typed expressions (typing.Any) are disallowed in **_
(ANN401)
[error] 147-147: Probable insecure usage of temporary file or directory: "/tmp/fabric-run.json"
(S108)
python/src/nemo_fabric/integrations/harbor/__init__.py
[error] 33-33: Probable insecure usage of temporary file or directory: "/tmp/fabric-run.json"
(S108)
🪛 Trivy (0.69.3)
integrations/harbor/demo/task/environment/Dockerfile
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
[info] 1-1: No HEALTHCHECK defined
Add HEALTHCHECK instruction in your Dockerfile
Rule: DS-0026
(IaC/Dockerfile)
🔇 Additional comments (1)
pyproject.toml (1)
18-21: 🩺 Stability & AvailabilityClarify the Harbor extra's Python support boundary.
nemo-fabric[harbor]only installsharboron Python 3.12+, while the project itself still advertises>=3.10and the Harbor docs in this PR use the extra directly. On 3.10/3.11 that install resolves toPyYAMLonly, so the integration looks available until the Harbor entrypoint is imported or invoked. Please align the packaging/docs one way or the other instead of silently degrading here.
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)
python/tests/smoke_harbor_integration.py (1)
77-123: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winThe fake runner now hides invocation and failure-path regressions.
Any command containing
nemo_fabric.integrations.harbor.runnerproduces a successful result file. That means this smoke will not fail if the code invokes the wrong CLI shape, omits required runner arguments/output handling, or mishandles a non-zero runner exit. The changed SDK runner path needs at least one negative case here, and the success stub should be gated on the exact expected invocation. As per path instructions, tests underpython/tests/**must cover error paths for the changed API surface.🤖 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 `@python/tests/smoke_harbor_integration.py` around lines 77 - 123, The `exec` stub in `smoke_harbor_integration.py` is too permissive because any command containing `nemo_fabric.integrations.harbor.runner` is treated as success, masking CLI and failure-path regressions. Tighten the mock in `exec` so the success fixture only triggers for the exact expected runner invocation used by the smoke, and add at least one negative test case that simulates a bad runner command or non-zero exit. Use the existing `exec`, `ExecResult`, and `/logs/agent/fabric-result.json` handling to verify the code still validates runner arguments and error behavior.Source: Path instructions
🤖 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 `@python/tests/smoke_harbor_integration.py`:
- Around line 77-123: The `exec` stub in `smoke_harbor_integration.py` is too
permissive because any command containing
`nemo_fabric.integrations.harbor.runner` is treated as success, masking CLI and
failure-path regressions. Tighten the mock in `exec` so the success fixture only
triggers for the exact expected runner invocation used by the smoke, and add at
least one negative test case that simulates a bad runner command or non-zero
exit. Use the existing `exec`, `ExecResult`, and
`/logs/agent/fabric-result.json` handling to verify the code still validates
runner arguments and error behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 4bbe3021-65e8-4b9d-aa8b-c6c3c72690a4
📒 Files selected for processing (1)
python/tests/smoke_harbor_integration.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Test
⚠️ CI failures not shown inline (2)
GitHub Actions: Rust / Test: Add Harbor multi-harness SDK demo
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 7.52s
�[1m�[92m Running�[0m unittests src/main.rs (target/debug/deps/fabric-d6b0cd643d758f74)
running 0 tests
�[1m�[92m Running�[0m unittests src/lib.rs (target/debug/deps/fabric_core-039dd9c04deba413)
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
running 41 tests
test config::tests::errors_for_unknown_manifest_profile ... ok
GitHub Actions: Rust / 0_Test.txt: Add Harbor multi-harness SDK demo
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 7.52s
�[1m�[92m Running�[0m unittests src/main.rs (target/debug/deps/fabric-d6b0cd643d758f74)
running 0 tests
�[1m�[92m Running�[0m unittests src/lib.rs (target/debug/deps/fabric_core-039dd9c04deba413)
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
running 41 tests
test config::tests::errors_for_unknown_manifest_profile ... ok
🧰 Additional context used
📓 Path-based instructions (1)
{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:
python/tests/smoke_harbor_integration.py
🧠 Learnings (1)
📚 Learning: 2026-06-28T04:03:32.877Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 26
File: python/tests/smoke_typed_config.py:163-177
Timestamp: 2026-06-28T04:03:32.877Z
Learning: In NVIDIA NeMo Fabric Python SDK serialization of `RuntimeCapabilities` (to satisfy the “parity contract” with Rust core and the CLI), do not emit metadata keys when the corresponding metadata is absent. Instead, omit those fields entirely so the produced JSON matches the Rust/CLI output (e.g., avoid `null`, empty objects, or placeholder metadata). During review, verify the serializer/builders follow this omission rule and that Python outputs/parity tests reflect the same shape.
Applied to files:
python/tests/smoke_harbor_integration.py
🔇 Additional comments (1)
python/tests/smoke_harbor_integration.py (1)
25-30: 🎯 Functional Correctness
extra_envis fine here Harbor'sBaseAgentaccepts passthrough kwargs, so keepingextra_envin the stub does not hide a constructor mismatch.> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/test_harbor_runner.py (1)
101-132: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd the missing-profile-path failure case.
These tests now cover absent config and malformed YAML, but
load_sources()still has no assertion forprofile_pathspointing to a nonexistent file. That leaves one of the new file-loading error paths untested.As per path instructions, "Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant."
🤖 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_harbor_runner.py` around lines 101 - 132, Add a new failure-case test in test_runner_rejects_missing_config/test_runner_rejects_malformed_profile coverage for load_sources() when profile_paths contains a nonexistent file. Reuse the existing test pattern in test_harbor_runner.py, call nemo_fabric.integrations.harbor.runner.load_sources with a valid config_path and a missing profile path, and assert the expected FileNotFoundError so the new file-loading error path is covered alongside the current config and YAML parsing checks.Source: Path instructions
🤖 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.
Duplicate comments:
In `@tests/test_harbor_runner.py`:
- Around line 101-132: Add a new failure-case test in
test_runner_rejects_missing_config/test_runner_rejects_malformed_profile
coverage for load_sources() when profile_paths contains a nonexistent file.
Reuse the existing test pattern in test_harbor_runner.py, call
nemo_fabric.integrations.harbor.runner.load_sources with a valid config_path and
a missing profile path, and assert the expected FileNotFoundError so the new
file-loading error path is covered alongside the current config and YAML parsing
checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 7070a2dc-4a24-4c20-96bd-988c0535985b
📒 Files selected for processing (4)
integrations/harbor/demo/README.mdintegrations/harbor/demo/task/environment/Dockerfileintegrations/harbor/demo/task/solution/solve.shtests/test_harbor_runner.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Test
⚠️ CI failures not shown inline (2)
GitHub Actions: Rust / Test: Add Harbor multi-harness SDK demo
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.65s
�[1m�[92m Running�[0m unittests src/main.rs (target/debug/deps/fabric-d6b0cd643d758f74)
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-039dd9c04deba413)
running 41 tests
test config::tests::errors_for_unknown_manifest_profile ... ok
GitHub Actions: Rust / 0_Test.txt: Add Harbor multi-harness SDK demo
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.65s
�[1m�[92m Running�[0m unittests src/main.rs (target/debug/deps/fabric-d6b0cd643d758f74)
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-039dd9c04deba413)
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: 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_harbor_runner.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_harbor_runner.py
🔇 Additional comments (4)
integrations/harbor/demo/task/environment/Dockerfile (1)
8-10: LGTM!integrations/harbor/demo/task/solution/solve.sh (1)
7-11: LGTM!integrations/harbor/demo/README.md (1)
114-115: LGTM!tests/test_harbor_runner.py (1)
213-223: LGTM!
AjayThorve
changed the title
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
74f3d10 to
2e6199b
Compare
|
📖 Fern docs preview: https://nvidia-preview-pull-request-29.docs.buildwithfern.com/nemo/fabric |
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
2 participants
Summary
FabricClientWhy
Harbor should own tasks, sandboxes, verification, rewards, and run layout. Fabric should own normalized harness selection and execution. This integration keeps that boundary explicit while showing that Harbor can use one adapter across multiple harnesses and telemetry configurations.
Validation
uv run --group test pytest -q(146 passed, 7 skipped)cargo test --workspace --locked(41 passed)cargo check -p fabric-pythoncargo fmt --all -- --checkuvx ruff check tests/test_harbor_runner.py python/src/nemo_fabric/integrations/harbor integrations/harbor/demouv lock --checkpython python/tests/smoke_harbor_integration.pyharbor run: 1 trial, 0 exceptions, mean reward 1.000host.docker.internal:6006Notes
The demo uses current Harbor 0.16.1 APIs and pins Codex CLI 0.142.4. The real Hermes, Relay, and Codex variants require their corresponding local credentials; the smoke variant does not.
Summary by CodeRabbit
multiplyreturns the proper product and verified it in tests.--akconfiguration examples.