Skip to content

feat: Add MCP-backed dynamic tool fetching to Python SDK - #39

Merged
ryoppippi merged 3 commits into
mainfrom
ENG-11393-add-fetch-tools-in-python-sdk
Nov 12, 2025
Merged

feat: Add MCP-backed dynamic tool fetching to Python SDK#39
ryoppippi merged 3 commits into
mainfrom
ENG-11393-add-fetch-tools-in-python-sdk

Conversation

@ryoppippi

@ryoppippiryoppippi commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

Implements fetch_tools() method with MCP-backed dynamic tool discovery, matching the Node SDK implementation.

Related: StackOneHQ/stackone-ai-node#114

CopilotAI review requested due to automatic review settings November 11, 2025 15:52

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements MCP (Model Context Protocol) integration for dynamic tool discovery in the Python SDK, achieving feature parity with the Node SDK. The changes enable runtime tool fetching from the StackOne API without requiring SDK updates.

Key changes:

  • Added RPC-backed tool execution through the /actions/rpc endpoint via new _StackOneRpcTool class
  • Implemented MCP client integration with pagination support using streamablehttp_client transport
  • Added comprehensive test suite with 12 tests covering account, provider, and action filtering scenarios

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
tests/test_toolset_mcp.pyNew comprehensive test suite with 12 tests organized into TestAccountFiltering and TestProviderAndActionFiltering classes
tests/test_toolset.pyRemoved obsolete non-MCP fetch_tools tests, cleaned up imports
stackone_ai/toolset.pyCore MCP implementation including _StackOneRpcTool, _fetch_mcp_tools, thread-safe async wrapper, and enhanced fetch_tools() method
stackone_ai/feedback/tool.pyAdded type ignore comments for Pydantic field validators and improved code formatting
README.mdUpdated documentation with MCP feature information and installation instructions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

try:
from mcp import types as mcp_types
from mcp.client.session import ClientSession
from mcp.client.streamable_http import streamablehttp_client

CopilotAINov 11, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import path mcp.client.streamable_http appears to be incorrect. Based on the MCP Python SDK package structure, this should be mcp.client.streamablehttp (without the underscore). The correct import should be from mcp.client.streamablehttp import streamablehttp_client.

Suggested change
frommcp.client.streamable_httpimportstreamablehttp_client
frommcp.client.streamablehttpimportstreamablehttp_client

Copilot uses AI. Check for mistakes.
def runner() -> None:
try:
result["value"] = asyncio.run(awaitable)
except BaseException as exc: # pragma: no cover - surfaced in caller context

CopilotAINov 11, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except block directly handles BaseException.

Suggested change
exceptBaseExceptionasexc: # pragma: no cover - surfaced in caller context
exceptExceptionasexc: # pragma: no cover - surfaced in caller context

Copilot uses AI. Check for mistakes.
@ryoppippi
ryoppippiforce-pushed the ENG-11393-add-fetch-tools-in-python-sdk branch from f195fb5 to 764c149CompareNovember 11, 2025 15:54
Add MCP (Model Context Protocol) integration to enable dynamic tool fetching
at runtime, matching the Node SDK functionality. This allows users to pull
the latest tool definitions from the StackOne API without SDK updates.
Key Features:
- Dynamic tool discovery via MCP server endpoint
- _StackOneRpcTool class for RPC-backed execution at /actions/rpc
- Account filtering with set_accounts() and account_ids parameter
- Provider filtering (case-insensitive, prefix-based)
- Action filtering with glob pattern support
- Thread-safe async execution wrapper (_run_async)
- Proper authentication with Basic Auth headers
Implementation Details:
- MCP client integration with streamablehttp transport
- Tool catalog fetching with pagination support (cursor-based)
- Schema normalization for proper nullable field handling
- RPC payload construction with body/headers/path/query support
- Account scoping via x-account-id header
- User-Agent tracking for SDK version telemetry
Dependencies:
- Requires optional 'mcp' extra: pip install 'stackone-ai[mcp]'
- Raises ToolsetConfigError if MCP dependencies not available
The implementation maintains API parity with the Node SDK's
stackone.mcp-fetch functionality while following Python idioms.
Add 12 new tests covering MCP-backed tool fetching to match Node SDK test
coverage in stackone.mcp-fetch.spec.ts. Tests are organized into logical
groups for account and provider/action filtering scenarios.
Test Coverage:
Account Filtering (TestAccountFiltering):
- fetch_tools_with_single_account_id: Single account filtering
- fetch_tools_with_multiple_account_ids: Multiple account filtering
- fetch_tools_with_set_accounts: Using set_accounts() method
- fetch_tools_account_ids_override_set_accounts: Parameter precedence
- fetch_tools_with_constructor_account_id: Constructor-based account
- fetch_tools_with_empty_account_ids: Empty list handling
Provider/Action Filtering (TestProviderAndActionFiltering):
- fetch_tools_with_provider_filter: Single provider filtering
- fetch_tools_with_multiple_providers: Multiple provider filtering
- fetch_tools_with_action_glob_pattern: Glob pattern matching
- fetch_tools_with_exact_action_match: Exact action name matching
- fetch_tools_with_provider_and_action_filters: Combined filtering
- fetch_tools_with_exclusion_pattern: Negative glob patterns
Removed:
- Deleted old fetch_tools tests that used non-MCP approach
- These tests were testing the old implementation that loaded from
OpenAPI specs instead of the MCP server
The test suite uses comprehensive mocking of MCP client components:
- ClientSession for MCP protocol communication
- streamablehttp_client for HTTP transport
- Tool list responses with pagination support
- Proper async context manager handling
All tests verify:
- Correct tool count after filtering
- Proper tool presence/absence
- Account ID preservation in tool context
- Filter precedence and interaction
@ryoppippi
ryoppippiforce-pushed the ENG-11393-add-fetch-tools-in-python-sdk branch from 764c149 to 1cecc8aCompareNovember 11, 2025 15:55
@ryoppippi
ryoppippiforce-pushed the ENG-11393-add-fetch-tools-in-python-sdk branch from 1cecc8a to a127c6cCompareNovember 11, 2025 15:57

@cubic-dev-aicubic-dev-aiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files

@NicolasBelissentNicolasBelissent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

import threading
import warnings
from typing import Any
from collections.abc import Coroutine

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

never heard of these they good?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryoppippi
ryoppippi merged commit d72ca80 into mainNov 12, 2025
6 checks passed
@ryoppippi
ryoppippi deleted the ENG-11393-add-fetch-tools-in-python-sdk branch November 12, 2025 10:01
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ryoppippi@NicolasBelissent