test: isolate CLI config override tests - #105
Conversation
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (7)**/*.rs📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Files:
**/*.{rs,go,js,ts,tsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{rs,py,go,js,ts,tsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{js,ts,tsx,jsx,py,rs,go,java,c,cpp,h,cc,cxx,cs,rb,php,swift,kt}📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Files:
**/*.{py,js,ts,tsx,go,rs,md}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{rs,py,js,ts,tsx,go}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
{crates/**/tests/**,python/tests/**,go/nemo_flow/**/*_test.go}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
WalkthroughA new private test helper ChangesTest Configuration Isolation
🎯 2 (Simple) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
AjayThorve
left a comment
There was a problem hiding this comment.
LGTM, the changed tests pass locally.
|
/merge |
* fix: address outstanding sonar issues (#104) #### Overview Addresses the open Sonar maintainability findings reported on `release/0.2` by reducing Rust cognitive complexity and replacing duplicated Go test literals. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Split observability doctor and plugin editor control-flow into smaller helper functions. - Split OpenAI Responses request encoding overlays into focused helper functions. - Added constants for duplicated Go observability test literals. #### Where should the reviewer start? Start with `crates/core/src/codec/openai_responses.rs`, then review the smaller CLI refactors in `crates/cli/src/doctor.rs` and `crates/cli/src/plugins.rs`. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: none ## Summary by CodeRabbit * **Refactor** * Enhanced code organization in observability component checking, plugin configuration editing, and OpenAI response encoding for improved internal structure and maintainability. * **Tests** * Standardized error message formatting in observability plugin tests for improved consistency and clearer diagnostic output during test failures. [](https://app.coderabbit.ai/change-stack/NVIDIA/NeMo-Flow/pull/104) Authors: - Will Killian (https://github.com/willkill07) Approvers: - Ajay Thorve (https://github.com/AjayThorve) URL: #104 * test: isolate CLI config override tests (#105) #### Overview Isolates CLI config override unit tests from developer-level NeMo Flow configuration discovered through the normal user config path. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Adds a temp-scoped explicit config path helper for config tests that require no implicit user or project config discovery. - Updates the affected override tests to use that explicit path instead of relying on ambient process configuration. #### Where should the reviewer start? Start with `crates/cli/tests/coverage/config_tests.rs`, especially the tests around CLI plugin config overrides. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: none ## Summary by CodeRabbit * **Tests** * Enhanced test infrastructure for configuration resolution by improving test isolation and setup processes. [](https://app.coderabbit.ai/change-stack/NVIDIA/NeMo-Flow/pull/105) Authors: - Will Killian (https://github.com/willkill07) Approvers: - Ajay Thorve (https://github.com/AjayThorve) URL: #105 * chore: keep OpenClaw dependency in lockstep (#107) #### Overview Keep the OpenClaw plugin package dependency on `nemo-flow-node` aligned with the package version emitted by repository versioning and packaging workflows. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Change `nemo-flow-openclaw` to depend on the exact checked-in `nemo-flow-node` version. - Add a `justfile` helper that updates package dependency versions in both `package.json` and `package-lock.json`. - Wire the dependency update into `set-version`, `package-node`, and `package-openclaw` so prerelease package builds keep the Node and OpenClaw versions together. - Align OpenClaw non-tag CI package version suffixing with the Node package workflow. Validation run: - `just set-version 0.2.0-alpha.20260514`, verified OpenClaw's `nemo-flow-node` dependency updated without dependency success logs, then restored with `just set-version 0.2.0`. - `npm install --workspace=nemo-flow-node --ignore-scripts` - `npm install --workspace=nemo-flow-openclaw --ignore-scripts` - `npm run typecheck --workspace=nemo-flow-openclaw` - `npm run pack:check --workspace=nemo-flow-openclaw` - `just --list` - `git diff --check` - Commit-time pre-commit hooks passed for the staged files. #### Where should the reviewer start? Start with `justfile`, especially the new package dependency version helper and its calls from `set_node_package_versions`, `package-node`, and `package-openclaw`. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: none ## Summary by CodeRabbit ## Release Notes * **Chores** * Refined OpenClaw package versioning format for improved clarity * Pinned nemo-flow-node dependency to a stable version for enhanced reliability * Improved build and packaging tooling to ensure consistent dependency alignment [](https://app.coderabbit.ai/change-stack/NVIDIA/NeMo-Flow/pull/107) Authors: - Will Killian (https://github.com/willkill07) Approvers: - https://github.com/Salonijain27 URL: #107 * refactor: improve CLI coverage structure (#108) #### Overview Refactors the CLI coverage-heavy interactive setup/plugin editors so testable logic lives in focused helper modules, then adds coverage around the extracted behavior and related CLI paths. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Split plugin config file handling and Observability editor state helpers out of `plugins.rs` into testable modules. - Split setup config/model/file helper logic out of `setup.rs` into `setup/model.rs`. - Added focused coverage for CLI setup, plugin editing models, doctor formatting/check helpers, completion install helpers, server gateway forwarding paths, and CLI smoke behavior. - Updated Codecov component reporting from Gateway Runtime to CLI, set the CLI target to 88%, and excluded only the remaining TTY prompt shells. - Added module guidance so future testable logic does not go back into the interactive prompt files. #### Where should the reviewer start? Start with `crates/cli/src/plugins.rs` and `crates/cli/src/setup.rs` to see the orchestration-only boundary, then review `crates/cli/src/plugins/editor_model.rs`, `crates/cli/src/plugins/config_io.rs`, and `crates/cli/src/setup/model.rs` for the extracted testable logic. Validation run locally: - `cargo fmt --all` - `cargo test -p nemo-flow-cli` with isolated `XDG_CONFIG_HOME` - `cargo clippy --workspace --all-targets -- -D warnings` - `just test-rust` with isolated `XDG_CONFIG_HOME` - `cargo llvm-cov --package nemo-flow-cli --summary-only ...` - commit hook pre-commit checks, including cargo fmt, cargo clippy, and cargo check #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: none ## Summary by CodeRabbit * **New Features** * Plugin config preview, validation, and safer save/merge behavior; clearer scope selection and Hermes hook handling in setup. * **Chores / Refactor** * Reorganized CLI internals into smaller, testable modules to make interactive editors and the setup wizard more reliable. * **Tests** * Added extensive CLI unit and integration tests covering agents, doctor, completions, plugin editor TTY behavior, setup flows, and gateway/server forwarding. * **Chores** * Updated Codecov to include CLI coverage and exclude interactive prompt loops. Signed-off-by: Will Killian <wkillian@nvidia.com> * ci: fold OpenClaw checks into Node workflow (#106) #### Overview Fold OpenClaw validation into the existing Node.js CI workflow as a conditional step, and make `just test-openclaw` run the live smoke coverage path. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Add a `run_openclaw` input to the reusable Node.js workflow. - Run OpenClaw integration checks as a conditional step across the full Node test matrix. - Remove the separate OpenClaw workflow call and workflow file. - Keep OpenClaw package artifact creation inside the existing Node package job while leaving Node package steps gated by `run_package`. - Add the live smoke test path to `just test-openclaw` and document the target in contributor testing guidance. #### Where should the reviewer start? Start with `.github/workflows/ci_node.yml`, especially the conditional OpenClaw test step in the Node test job and the package-job gating. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: none <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added live-run OpenClaw smoke tests and a helper script to run them. * OpenClaw integration checks now run conditionally inside the Node.js test flow. * Test suites updated to use compiled test artifacts and adjusted import paths. * **Chores** * Removed standalone OpenClaw CI job; its checks and optional packaging were folded into the Node.js workflow and gated by inputs. * OpenClaw packaging and artifact upload are now conditional. * **Documentation** * Testing guide and integration README updated with live-test target and artifact locations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> * feat: update OpenClaw patch to use new config shape (#111) #### Overview This PR updates the OpenClaw patch to be aligned with `main` - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details <!-- Describe the changes made in this PR. --> #### Where should the reviewer start? <!-- Point to the most important file, test, or design decision. --> #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Closes # * feat: Add Deep Agents integration (#90) #### Overview * Primary entry point for end-users is the `add_nemo_flow_integration` method, which adds NeMo Flow middleware to the agent and subagents (if any). * Emit a mark when skills are loaded and HITL events * `langgraph` is promoted from a transitive dep to a direct-dep in the `langchain` extra since `python/nemo_flow/integrations/langchain/_serialization.py` now imports from `langgraph` * Adds work-around for installing OpenSSL on Windows-Arm CI runners - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details #### Where should the reviewer start? #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Closes # ## Summary by CodeRabbit * **New Features** * Deep Agents integration: observability for agent runs, tool calls, filesystem/sandbox operations, human-in-the-loop events, backend instrumentation, and middleware marks. * **Documentation** * Added Deep Agents integration README with setup, usage, and observed-event examples. * **Tests** * New integration and end-to-end tests covering middleware, backend instrumentation, sandbox behavior, and HITL flows; added shared test fixture. * **Chores** * Added optional Deep Agents dependency group and regenerated third‑party attributions/licenses. * **CI** * Updated CI and dev sync steps to include Deep Agents extras and platform-specific setup. [](https://app.coderabbit.ai/change-stack/NVIDIA/NeMo-Flow/pull/90) Authors: - David Gardner (https://github.com/dagardner-nv) Approvers: - Will Killian (https://github.com/willkill07) - https://github.com/Salonijain27 URL: #90 * chore: refresh dependency locks (#112) #### Overview Refresh dependency locks and runtime compatibility wiring for the `release/0.2` branch. - [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 - Updates Python optional integration constraints and lockfile entries for the release branch. - Updates OpenClaw peer compatibility to `2026.5.12` and syncs the local hook type mirror with the current session end reasons. - Adjusts Rust reqwest/rustls provider configuration and OTLP HTTP client selection so TLS and exporter initialization remain stable while unused optional transport packages stay out of the lockfile. - Makes Node attribution generation lockfile-driven so platform-gated optional packages are listed consistently across CI and local machines. - Pulls Node license text from integrity-checked locked npm artifacts when packages are not installed on the current platform. - Regenerates Python and Node attribution files from the refreshed lockfiles. Validation run: - `cargo test -p nemo-flow-cli` - `cargo check -p nemo-flow --features otel` - `cargo check -p nemo-flow --features otel,openinference` - `cargo test -p nemo-flow http_config_exports_scope_push_pop_and_marks_without_tokio_runtime -- --nocapture` - `npm test --workspace integrations/openclaw` - `uv run --extra langgraph --extra langchain-nvidia pytest python/tests/integrations/langchain python/tests/integrations/langgraph third_party/langgraph_tests` - `npm audit --ignore-scripts` - `cargo fmt --all --check` - `pre-commit run --files scripts/licensing/attributions_lockfile_md.py ATTRIBUTIONS-Node.md package-lock.json` - signed commit pre-commit suite #### Where should the reviewer start? Start with `scripts/licensing/attributions_lockfile_md.py`, `crates/cli/src/tls.rs`, `crates/core/Cargo.toml`, and `integrations/openclaw/package.json` to review the generator, runtime, and integration compatibility changes before the regenerated lockfile and attribution diffs. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: none ## Summary by CodeRabbit * **Dependencies** * Bumped several Python packages (langchain, langchain-core, langgraph, langgraph-*, urllib3) and raised OpenClaw compatibility to 2026.5.12. * **New Features** * Added "shutdown" and "restart" as session termination reasons. * **Chores** * Ensure TLS/crypto provider is initialized before network operations across CLI and services. * **Documentation** * Refreshed third‑party attributions and improved license extraction for Node/Python packages. [](https://app.coderabbit.ai/change-stack/NVIDIA/NeMo-Flow/pull/112) Authors: - Will Killian (https://github.com/willkill07) Approvers: - David Gardner (https://github.com/dagardner-nv) - https://github.com/Salonijain27 URL: #112 * fix: support Cursor CLI hook config (#109) #### Overview Fix Cursor CLI hook configuration so current `cursor-agent` builds can execute NeMo Flow hook commands, and document the remaining Cursor CLI hook coverage limits. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Generate Cursor `.cursor/hooks.json` entries as direct command entries with top-level `"version": 1`. - Preserve that Cursor-compatible shape when `nemo-flow run -- cursor-agent` temporarily patches project hooks. - Update `nemo-flow doctor cursor` to reject nested Cursor hook groups and missing or non-`1` top-level version fields. - Trim nullable fields from Cursor permission responses. - Update Cursor integration docs and packaged hooks to state the CLI limitation plainly: current Cursor CLI hook coverage is narrower than Cursor IDE hook coverage. #### Where should the reviewer start? Start with `crates/cli/src/installer.rs` for the hook-shape change, then `crates/cli/src/doctor.rs` for validation of user-managed Cursor hook files. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Closes https://linear.app/nvidia/issue/NMF-122/fix-cursor-cli-hook-config-and-document-stream-json-limitation ## Summary by CodeRabbit * **Refactor** * Standardized Cursor hook JSON to require a top-level version (1) and direct command entries; merged/generated hook files now enforce this shape. * **New Features** * Added stricter validation for Cursor hook files with clear, actionable status messages and test coverage. * **Bug Fixes** * Cursor hook responses/endpoints no longer include legacy message fields. * **Documentation** * Expanded Cursor integration and troubleshooting guidance, plus CLI verification steps. [](https://app.coderabbit.ai/change-stack/NVIDIA/NeMo-Flow/pull/109) Authors: - Ajay Thorve (https://github.com/AjayThorve) Approvers: - Will Killian (https://github.com/willkill07) URL: #109 --------- Signed-off-by: Will Killian <wkillian@nvidia.com> Co-authored-by: Will Killian <2007799+willkill07@users.noreply.github.com> Co-authored-by: David Gardner <96306125+dagardner-nv@users.noreply.github.com> Co-authored-by: Ajay Thorve <AjayThorve@users.noreply.github.com>
Overview
Isolates CLI config override unit tests from developer-level NeMo Flow configuration discovered through the normal user config path.
Details
Where should the reviewer start?
Start with
crates/cli/tests/coverage/config_tests.rs, especially the tests around CLI plugin config overrides.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit