refactor: resolve remaining Sonar findings

[willkill07]

Overview

• 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.

Details

• Refactor the three remaining marketplace-install transaction paths below Sonar's cognitive-complexity threshold while preserving staging, rollback, and registration recovery behavior.
• Remove four redundant Go test variable declarations reported by Sonar.
• Clears the seven live open Sonar findings on main: three rust:S3776 findings and four godre:S8193 findings.

Where should the reviewer start?

Review crates/cli/src/installation/marketplace/mod.rs, especially the extracted transaction helpers and the existing marketplace installer tests that exercise their error paths.

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

• Relates to: none

Validation

• cargo test -p nemo-relay-cli installation::marketplace
• just test-go
• cargo clippy --workspace --all-targets -- -D warnings
• uv run pre-commit run --files crates/cli/src/installation/marketplace/mod.rs go/nemo_relay/event_sanitizers_test.go go/nemo_relay/plugin_activation_test.go
• just test-rust was started but stalled in the existing native-plugin integration executable; it was stopped after more than four minutes with no CPU activity.

Summary by CodeRabbit

• Bug Fixes

  • Improved reliability of forced marketplace installations, including staged content promotion and recovery after installation or registration failures.
  • Enhanced cleanup and rollback handling to help preserve the previous working state when installation steps fail.

• Tests

  • Clarified error handling in marketplace-related test scenarios without changing expected behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 22b1ad81-c1b7-46bb-8454-2952968c6733

📥 Commits

Reviewing files that changed from the base of the PR and between 1bb1d04 and 3e5bdef.

📒 Files selected for processing (3)

• crates/cli/src/installation/marketplace/mod.rs
• go/nemo_relay/event_sanitizers_test.go
• go/nemo_relay/plugin_activation_test.go

📜 Recent review details

⏰ Context from checks skipped due to timeout. (1)

• GitHub Check: Check / Run

🧰 Additional context used

📓 Path-based instructions (16)

go/nemo_relay/**/*.go

📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)

go/nemo_relay/**/*.go: Format changed Go packages with cd go/nemo_relay && go fmt ./...
Run Go tests with just test-go to build and test the NeMo Relay Go binding
Use just build-go when you want an explicit build-only pass or need the artifact for other work
Use just ci=true test-go when you need the CI-style coverage and JUnit path
On macOS, set DYLD_LIBRARY_PATH to the ../../target/release directory before running the raw go test command directly

Use PascalCase for public Go APIs.

Files:

• go/nemo_relay/plugin_activation_test.go
• go/nemo_relay/event_sanitizers_test.go

**/*.go

📄 CodeRabbit inference engine (CONTRIBUTING.md)

When changing the experimental Go binding, format Go code with gofmt and keep go vet ./... passing.

Files:

• go/nemo_relay/plugin_activation_test.go
• go/nemo_relay/event_sanitizers_test.go

**/*.{rs,py,go,js,ts,c,h}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use language-appropriate naming conventions: Rust snake_case, C FFI exports prefixed nemo_relay_, Go PascalCase, Node.js camelCase, and Python snake_case.

Files:

• go/nemo_relay/plugin_activation_test.go
• go/nemo_relay/event_sanitizers_test.go
• crates/cli/src/installation/marketplace/mod.rs

**/*.{rs,go,js,ts}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding // comment form.

Files:

• go/nemo_relay/plugin_activation_test.go
• go/nemo_relay/event_sanitizers_test.go
• crates/cli/src/installation/marketplace/mod.rs

{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}

📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)

Update the language-native bindings for every exposed surface in Python, Go, and Node.js.

Files:

• go/nemo_relay/plugin_activation_test.go
• go/nemo_relay/event_sanitizers_test.go

{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}

📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)

Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.

Files:

• go/nemo_relay/plugin_activation_test.go
• go/nemo_relay/event_sanitizers_test.go

**/*.{py,go,js,ts}

📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)

Keep Python, Go, and Node.js config objects and subscriber/exporter methods aligned so all bindings expose the same logical knobs and semantics.

Files:

• go/nemo_relay/plugin_activation_test.go
• go/nemo_relay/event_sanitizers_test.go

go/nemo_relay/**

📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)

Keep shared plugin helpers in go/nemo_relay aligned with plugin registration, composition, and lifecycle behavior.

Files:

• go/nemo_relay/plugin_activation_test.go
• go/nemo_relay/event_sanitizers_test.go

**/*

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, use maintain-dynamic-plugins and include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, prefer uv run pre-commit run --files <changed files...>.
Before review or handoff, run uv run pre-commit run --all-files.

Files:

• go/nemo_relay/plugin_activation_test.go
• go/nemo_relay/event_sanitizers_test.go
• crates/cli/src/installation/marketplace/mod.rs

**/*.{rs,py,go,js,ts}

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

If a language surface changed, always run that language's test target even when Rust core did not change.

Files:

• go/nemo_relay/plugin_activation_test.go
• go/nemo_relay/event_sanitizers_test.go
• crates/cli/src/installation/marketplace/mod.rs

go/nemo_relay/**/*

⚙️ CodeRabbit configuration file

go/nemo_relay/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.

Files:

• go/nemo_relay/plugin_activation_test.go
• go/nemo_relay/event_sanitizers_test.go

{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}

⚙️ CodeRabbit configuration file

{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.

Files:

• go/nemo_relay/plugin_activation_test.go
• go/nemo_relay/event_sanitizers_test.go

**/*.rs

📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)

**/*.rs: Any Rust change must run just test-rust
Any Rust change must run cargo fmt --all
Any Rust change must run cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all for all FFI work since it is Rust work
Run just test-rust to validate FFI changes
Run cargo clippy --workspace --all-targets -- -D warnings to enforce strict linting on FFI work

When Rust files changed as part of Go work, also run cargo fmt --all, just test-rust, and cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all when Rust files are changed as part of Node work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files are changed as part of Node work
Run just test-rust when Rust files are changed as part of Node work

When changing the core Rust runtime or Rust-facing API surface, format Rust code with cargo fmt (rustfmt defaults), keep cargo clippy -- -D warnings clean, and satisfy cargo deny check per deny.toml.

**/*.rs: If any Rust code changed, always run just test-rust.
If any Rust code changed, also run cargo fmt --all.
If any Rust code changed, also run cargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, run cargo fmt --all and cargo clippy --workspace --all-targets -- -D warnings even if relying on pre-commit.

Files:

• crates/cli/src/installation/marketplace/mod.rs

**/*.{rs,py}

📄 CodeRabbit inference engine (AGENTS.md)

Follow binding naming conventions in Rust and Python: use snake_case.

Files:

• crates/cli/src/installation/marketplace/mod.rs

**/*.{rs,py,js,mjs,cjs,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{rs,py,js,mjs,cjs,ts,tsx}: Use Json = serde_json::Value in Rust-facing runtime APIs where the existing code expects JSON payloads.
Use Result<T> with FlowError in core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.

Files:

• crates/cli/src/installation/marketplace/mod.rs

{crates/**/src/**/*.rs,python/**/*.py}

📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)

Do not add tests under src; Rust tests belong in crate tests/ trees, and Python SDK tests belong under python/tests.

Files:

• crates/cli/src/installation/marketplace/mod.rs

🔇 Additional comments (4)

go/nemo_relay/event_sanitizers_test.go (1)

192-193: LGTM!

Also applies to: 205-206, 227-228

go/nemo_relay/plugin_activation_test.go (1)

672-673: LGTM!

crates/cli/src/installation/marketplace/mod.rs (2)

345-361: 📐 Maintainability & Code Quality

Complete the required Rust test run before merge.

The PR summary says just test-rust was stopped after a stalled native-plugin executable, so mandatory validation has not completed. Resolve or isolate the stall, then rerun the target successfully.

As per coding guidelines, “Any Rust change must run just test-rust.”

Source: Coding guidelines

---

363-413: LGTM!

Also applies to: 428-551, 618-780

---

Walkthrough

The marketplace installer extracts forced-install staging, content promotion, registration, and rollback recovery into helpers. Two Go tests refactor inline error checks into separate assignments without changing behavior.

Changes

Marketplace installation flow

Layer / File(s)	Summary
Transaction staging and marketplace installation crates/cli/src/installation/marketplace/mod.rs	Forced installs stage marketplace content and snapshots through a helper; installation now separates staged promotion from unstaged writes, lock acquisition, and incomplete-install cleanup.
Registration and rollback recovery crates/cli/src/installation/marketplace/mod.rs	Registration, plugin setup, doctor execution, uncertain-state reconciliation, replacement retirement, rollback, and snapshot restoration are delegated to dedicated helpers.

Go test error-handling cleanup

Layer / File(s)	Summary
Explicit test error handling go/nemo_relay/event_sanitizers_test.go, go/nemo_relay/plugin_activation_test.go	Registration and JSON decoding errors are assigned before separate conditional checks; existing assertions and return behavior remain unchanged.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Installer
  participant Marketplace
  participant Snapshot
  participant Gateway
  participant HostRegistration

  Installer->>Marketplace: Stage marketplace content
  Installer->>Snapshot: Acquire replacement lock and start snapshot
  Installer->>Gateway: Refresh gateway
  Installer->>Marketplace: Promote staged content
  Installer->>HostRegistration: Register marketplace and plugin
  HostRegistration-->>Installer: Report registration result
  Installer->>Snapshot: Retire or restore replacement during recovery

Loading

Possibly related PRs

• NVIDIA/NeMo-Relay#395 — Related marketplace installer transaction and registration/rollback changes.
• NVIDIA/NeMo-Relay#428 — Related forced-install snapshot, lock, and transaction wiring changes.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title follows Conventional Commits and accurately summarizes the refactor and Sonar-finding cleanup.
Description check	✅ Passed	The description matches the template with overview, details, reviewer guidance, and validation; only the related issue placeholder is unpopulated.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

_{Comment @coderabbitai help to get the list of available commands.}