fix(mcp-client): ensure tools are only invoked when available

[willkill07]

Description

During MCP client reauth, we do not ensure there is a new, valid connection. Guard this in the following way:

• Add a new is_connected property
• Extend the lifetime of a tool while in use (get_tool + acall)

Adds exhaustive testing for scenarios.

Closes

By Submitting this PR I confirm:

• I am familiar with the Contributing Guidelines.
• We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
  • Any contribution which contains commits that are not Signed-Off will not be accepted.
• When the PR is ready for review, new or existing tests cover these changes.
• When the PR is ready for review, the documentation is up to date with these changes.

Summary by CodeRabbit

Release Notes

• New Features

  • Added is_connected property to check if an active connection is established

• Improvements

  • Enhanced tool invocation with improved session-aware routing and stricter connectivity validation before execution
  • Improved session context handling for per-session tool calls

[coderabbitai]

Walkthrough

The pull request adds a new is_connected property to report MCP client connection state and refactors tool invocation logic to centralize handling through a new _invoke_tool helper, improving session context management for per-session tool calls. Comprehensive test coverage is added for both the connection state tracking and session-aware invocation flows.

Changes

Cohort / File(s)	Summary
Connection State Tracking packages/nvidia_nat_mcp/src/nat/plugins/mcp/client/client_base.py	Added public is_connected property to check for active, initialized connection by verifying exit stack existence and connection establishment state.
Tool Invocation Refactoring packages/nvidia_nat_mcp/src/nat/plugins/mcp/client/client_impl.py	Introduced _invoke_tool helper to centralize tool invocation logic, handling direct input and kwargs-based approaches with None-field stripping; reworked mcp_session_tool_function to route through the helper and ensure session context acquisition for per-session calls with connectivity validation.
Base Client Tests packages/nvidia_nat_mcp/tests/client/test_mcp_client_base.py	Added async test functions validating is_connected lifecycle behavior across context entry/exit and reconnection failure scenarios using MockMCPClient simulation.
Tool Invocation Tests packages/nvidia_nat_mcp/tests/client/test_mcp_client_impl.py	Added extensive test classes and helper utilities (_make_group, _FakeTool, _FakeMCPClient, _TrackingTool) covering default-user and session-based invocation paths, disconnection states, and context activation verification.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely describes the main change: adding safeguards to ensure tools are only invoked when available, which aligns with all modified files.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

packages/nvidia_nat_mcp/tests/client/test_mcp_client_impl.py (1)

228-236: Fixture naming doesn't follow the project convention.

Per coding guidelines, pytest fixtures should specify the name argument and use the fixture_ prefix on the decorated function.

♻️ Proposed fix

-    `@pytest.fixture`
-    def session_group(self):
+    `@pytest.fixture`(name="session_group")
+    def fixture_session_group(self):

As per coding guidelines: "Pytest fixtures should define the name argument when applying the pytest.fixture decorator. The fixture function being decorated should be named using the fixture_ prefix, using snake_case."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/nvidia_nat_mcp/tests/client/test_mcp_client_impl.py` around lines
228 - 236, Rename the pytest fixture function and explicitly set its decorator
name to follow project conventions: change the function from session_group to
fixture_session_group and update the decorator to
`@pytest.fixture`(name="session_group"); keep the body (group creation,
_shared_auth_provider, _default_user_id, _client_config.session_aware_tools)
unchanged so tests still reference the fixture by "session_group" while the
local function follows the fixture_ prefix convention.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/nvidia_nat_mcp/tests/client/test_mcp_client_impl.py`:
- Around line 228-236: Rename the pytest fixture function and explicitly set its
decorator name to follow project conventions: change the function from
session_group to fixture_session_group and update the decorator to
`@pytest.fixture`(name="session_group"); keep the body (group creation,
_shared_auth_provider, _default_user_id, _client_config.session_aware_tools)
unchanged so tests still reference the fixture by "session_group" while the
local function follows the fixture_ prefix convention.

[dnandakumar-nv]

Elegant. Nice.

[dnandakumar-nv]

/merge

[AnuradhaKaruppiah]

Thanks for taking care of this @willkill07