refactor: address all open Sonar findings

[willkill07]

Overview

Address all four open Sonar findings on upstream main by reducing cognitive complexity without changing behavior. The Sonar MCP inventory and project measures report four code smells, zero bugs, zero vulnerabilities, and zero security hotspots.

• 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

• Extract disconnect-grace expiration from disconnected in crates/cli/src/daemon/broker/server/socket.rs (rust:S3776, previously 26).
• Extract worker launch supervision from supervise_route in crates/cli/src/daemon/mcp/mod.rs (rust:S3776, previously 32), preserving failure reporting, registration refresh, and pending-child cleanup.
• Group the native fixture's tool execution intercept registrations in a helper in crates/core/tests/fixtures/native_plugin/src/lib.rs (rust:S3776, previously 30), preserving registration order and scoped, isolated, and concurrent delegation.
• Extract directional codec validation and resolution from TestLlmExecutionInterceptResolvesDirectionalCodecs in go/nemo_relay/llm_test.go (go:S3776, previously 26), preserving codec lifetime assertions.

No public API changes or Sonar suppressions.

Validation:

• Passed: 36 focused daemon MCP/socket tests, the native fixture's sdk_cdylib_registers_tool_request_intercept test, and the focused Go directional codec test.
• Passed: just build-test-plugin-fixtures, just ci=true test-go, and staged uv run pre-commit run (including Clippy, Rust checks, Go vet, formatting, and link checks). Commit hooks also passed.
• Sonar MCP local analysis reports zero issues in the updated Go test file. The MCP does not support local Rust analysis; server analysis must confirm the Rust findings are cleared.
• just test-rust: 5,439 passed and 38 failed in configuration/plugin-policy tests on this host, which has /etc/nemo-relay/plugins.toml. Interrupted after ffi_activation_explicit_config_replaces_discovered_user_config stalled for over three minutes; 88 tests were not completed, and the recipe did not reach its example suites. The daemon tests and native fixture behavior tests passed.
• Python and Node suites were not run because their observable contracts are unchanged. Sonar hotspot listing was permission-denied, but project measures report zero hotspots; dependency-risk search is unavailable because Sonar Advanced Security is disabled.

Where should the reviewer start?

Start with supervise_worker_launch in crates/cli/src/daemon/mcp/mod.rs and verify that each worker failure still refreshes registration before the route loop continues. Then review expire_disconnected_peer for unchanged transaction ownership and generation checks.

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

• Relates to: none

Summary by CodeRabbit

• Refactor

  • Improved handling of disconnected peers and worker startup outcomes, including launch failures, early exits, and readiness timeouts.
  • Streamlined plugin tool-interception setup while preserving existing behavior.

• Tests

  • Consolidated checks for request and response codec identities and capabilities in execution-intercept tests.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: NVIDIA/NeMo-Relay/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: c1ea80ea-f1d9-479b-883b-a0ee8f927d29

📥 Commits

Reviewing files that changed from the base of the PR and between 88b1edf and 29cdc8c.

📒 Files selected for processing (4)

• crates/cli/src/daemon/broker/server/socket.rs
• crates/cli/src/daemon/mcp/mod.rs
• crates/core/tests/fixtures/native_plugin/src/lib.rs
• go/nemo_relay/llm_test.go

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

📜 Recent review details

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

• GitHub Check: Rust / Package (linux-amd64)
• GitHub Check: Rust / Package (linux-musl-amd64)
• GitHub Check: Rust / Test (windows-amd64)
• GitHub Check: Rust / Package (macos-arm64)
• GitHub Check: Rust / Package (linux-arm64)
• GitHub Check: Rust / Test (linux-arm64)
• GitHub Check: Rust / Package (windows-arm64)
• GitHub Check: Rust / Package (windows-amd64)
• GitHub Check: Rust / Package (linux-musl-arm64)
• GitHub Check: Go / Test (windows-arm64)
• GitHub Check: Rust / Test (linux-amd64)
• GitHub Check: Go / Test (linux-arm64)
• GitHub Check: Rust / Test (macos-arm64)
• GitHub Check: Rust / Test (windows-arm64)
• GitHub Check: Go / Test (windows-amd64)
• GitHub Check: Go / Test (linux-amd64)
• GitHub Check: Go / Test (macos-arm64)
• GitHub Check: Check / Run

🧰 Additional context used

📓 Path-based instructions (3)

Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.

⚙️ CodeRabbit configuration file

Files:

• go/nemo_relay/llm_test.go
• crates/core/tests/fixtures/native_plugin/src/lib.rs

Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.

⚙️ CodeRabbit configuration file

Files:

• crates/core/tests/fixtures/native_plugin/src/lib.rs

Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.

⚙️ CodeRabbit configuration file

Files:

• go/nemo_relay/llm_test.go

🔇 Additional comments (5)

crates/cli/src/daemon/broker/server/socket.rs (1)

789-838: LGTM!

crates/core/tests/fixtures/native_plugin/src/lib.rs (2)

223-223: LGTM!

---

361-478: LGTM!

crates/cli/src/daemon/mcp/mod.rs (1)

437-440: LGTM!

Also applies to: 463-526

go/nemo_relay/llm_test.go (1)

777-777: LGTM!

Also applies to: 809-809

---

Walkthrough

The changes extract existing disconnect cleanup, worker launch supervision, plugin interceptor registration, and test codec validation logic into private helpers. The summaries report that the existing interceptor paths and disconnect cleanup behavior are retained.

Changes

Peer disconnect expiry

Layer / File(s)	Summary
Disconnect expiry helper crates/cli/src/daemon/broker/server/socket.rs	disconnected starts expire_disconnected_peer, which retains the deadline and peer-state checks before session cleanup, peer removal, and waiter notification.

Worker launch supervision

Layer / File(s)	Summary
Worker launch supervision crates/cli/src/daemon/mcp/mod.rs	supervise_route delegates launch handling to supervise_worker_launch. The helper handles launch failures, early child exits, and readiness timeouts, and returns refreshed directives when needed.

Native plugin fixture registration

Layer / File(s)	Summary
Tool-execution interceptor registration crates/core/tests/fixtures/native_plugin/src/lib.rs	register delegates interceptor registration to register_fixture_tool_execution. The helper retains the existing request and result marking and next call paths.

Go execution test codec checks

Layer / File(s)	Summary
Directional codec validation go/nemo_relay/llm_test.go	The execution-intercept test uses resolveDirectionalExecutionTestCodecs to validate and resolve the request and response codecs before continuing.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 29cdc

No actionable merge-blocking issue is established; the change is mergeable after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title uses the allowed Conventional Commits format, uses the imperative summary "address all open Sonar findings," is 41 characters long, and has no trailing period.
Description check	✅ Passed	The description includes all required sections and provides detailed scope, validation results, reviewer guidance, and related-issue information. The related-issues entry states "Relates to: none" rat…

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

[mnajafian-nv]

Lgtm, no p0s or p1s found.

[mnajafian-nv]

/merge