Skip to content

Consolidate get_* and _list_* methods into single API#2719

Merged
jlowin merged 1 commit intomainfrom
consolidate-get-list-methods
Dec 25, 2025
Merged

Consolidate get_* and _list_* methods into single API#2719
jlowin merged 1 commit intomainfrom
consolidate-get-list-methods

Conversation

@jlowin
Copy link
Copy Markdown
Member

@jlowin jlowin commented Dec 25, 2025

The server had parallel implementations for listing components: get_tools() returned a dict indexed by name, while _list_tools() returned a list deduplicated by key. These were nearly identical with subtle differences in dedup logic and error handling.

This consolidates them into a single canonical method with an optional middleware parameter:

# Direct access (default) - no middleware
tools = await server.get_tools()

# With middleware chain - used by MCP handlers and mounted servers
tools = await server.get_tools(apply_middleware=True)

Breaking change: get_tools(), get_resources(), get_prompts(), and get_resource_templates() now return list instead of dict. The dict key was redundant since components already have .name or .uri attributes.

# Before
tools = await server.get_tools()
if "my_tool" in tools:
    tool = tools["my_tool"]

# After
tools = await server.get_tools()
tool = next((t for t in tools if t.name == "my_tool"), None)

The server had parallel implementations for listing components - get_tools()
returned dict by name, _list_tools() returned list by key. Consolidate into
get_tools(apply_middleware=False) -> list[Tool] as the single source of truth.

Breaking change: get_tools/resources/prompts/resource_templates now return
lists instead of dicts.
@jlowin jlowin added the v3 Targeted for FastMCP 3 label Dec 25, 2025
@marvin-context-protocol marvin-context-protocol Bot added breaking change Breaks backward compatibility. Requires minor version bump. Critical for maintainer attention. enhancement Improvement to existing functionality. For issues and smaller PR improvements. server Related to FastMCP server implementation or server-side functionality. labels Dec 25, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 25, 2025

Walkthrough

This pull request refactors component listing methods across the FastMCP server to return lists instead of dictionaries. The get_tools(), get_resources(), get_prompts(), and get_resource_templates() methods now accept an optional apply_middleware parameter and return list-based results keyed by component .key attributes rather than by .name or URIs. Internal provider calls and list operations are updated to use the new public API with middleware application. Documentation is added explaining the change with migration examples.

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: consolidating parallel get_* and list* methods into a single API.
Description check ✅ Passed The description thoroughly explains the consolidation rationale, breaking changes, and provides clear before/after code examples for migration.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch consolidate-get-list-methods

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3e67f9f and 08bd9ae.

⛔ Files ignored due to path filters (18)
  • tests/cli/test_run.py is excluded by none and included by none
  • tests/cli/test_server_args.py is excluded by none and included by none
  • tests/contrib/test_component_manager.py is excluded by none and included by none
  • tests/contrib/test_mcp_mixin.py is excluded by none and included by none
  • tests/deprecated/test_exclude_args.py is excluded by none and included by none
  • tests/deprecated/test_import_server.py is excluded by none and included by none
  • tests/server/providers/openapi/test_performance_comparison.py is excluded by none and included by none
  • tests/server/providers/test_local_provider.py is excluded by none and included by none
  • tests/server/providers/test_local_provider_prompts.py is excluded by none and included by none
  • tests/server/providers/test_local_provider_resources.py is excluded by none and included by none
  • tests/server/providers/test_local_provider_tools.py is excluded by none and included by none
  • tests/server/proxy/test_proxy_server.py is excluded by none and included by none
  • tests/server/test_mount.py is excluded by none and included by none
  • tests/server/test_server.py is excluded by none and included by none
  • tests/server/test_server_interactions.py is excluded by none and included by none
  • tests/server/test_tool_annotations.py is excluded by none and included by none
  • tests/server/test_tool_transformation.py is excluded by none and included by none
  • v3-notes/get-methods-consolidation.md is excluded by none and included by none
📒 Files selected for processing (4)
  • docs/development/upgrade-guide.mdx
  • src/fastmcp/server/providers/fastmcp_provider.py
  • src/fastmcp/server/server.py
  • src/fastmcp/utilities/inspect.py
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.mdx

📄 CodeRabbit inference engine (docs/.cursor/rules/mintlify.mdc)

docs/**/*.mdx: Use clear, direct language appropriate for technical audiences
Write in second person ('you') for instructions and procedures in MDX documentation
Use active voice over passive voice in MDX technical documentation
Employ present tense for current states and future tense for outcomes in MDX documentation
Maintain consistent terminology throughout all MDX documentation
Keep sentences concise while providing necessary context in MDX documentation
Use parallel structure in lists, headings, and procedures in MDX documentation
Lead with the most important information using inverted pyramid structure in MDX documentation
Use progressive disclosure in MDX documentation: present basic concepts before advanced ones
Break complex procedures into numbered steps in MDX documentation
Include prerequisites and context before instructions in MDX documentation
Provide expected outcomes for each major step in MDX documentation
End sections with next steps or related information in MDX documentation
Use descriptive, keyword-rich headings for navigation and SEO in MDX documentation
Focus on user goals and outcomes rather than system features in MDX documentation
Anticipate common questions and address them proactively in MDX documentation
Include troubleshooting for likely failure points in MDX documentation
Provide multiple pathways (beginner vs advanced) but offer an opinionated path to avoid overwhelming users in MDX documentation
Always include complete, runnable code examples that users can copy and execute in MDX documentation
Show proper error handling and edge case management in MDX code examples
Use realistic data instead of placeholder values in MDX code examples
Include expected outputs and results for verification in MDX code examples
Test all code examples thoroughly before publishing in MDX documentation
Specify language and include filename when relevant in MDX code examples
Add explanatory comments for complex logic in MDX code examples
Document all API...

Files:

  • docs/development/upgrade-guide.mdx
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Use Python ≥ 3.10 with full type annotations
Never use bare except - be specific with exception types

Files:

  • src/fastmcp/utilities/inspect.py
  • src/fastmcp/server/server.py
  • src/fastmcp/server/providers/fastmcp_provider.py
🧠 Learnings (2)
📚 Learning: 2025-11-26T21:51:44.174Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2025-11-26T21:51:44.174Z
Learning: Review and update related Manager classes (ToolManager, ResourceManager, PromptManager) when modifying MCP object definitions

Applied to files:

  • src/fastmcp/utilities/inspect.py
  • src/fastmcp/server/server.py
📚 Learning: 2025-12-21T21:37:55.031Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T21:37:55.031Z
Learning: Applies to src/fastmcp/**/__init__.py : Core types that define a module's purpose should be exported (e.g., `Middleware` from `fastmcp.server.middleware`), while specialized features can live in submodules

Applied to files:

  • src/fastmcp/utilities/inspect.py
  • src/fastmcp/server/server.py
  • src/fastmcp/server/providers/fastmcp_provider.py
🧬 Code graph analysis (2)
src/fastmcp/utilities/inspect.py (1)
src/fastmcp/server/server.py (4)
  • get_tools (793-837)
  • get_prompts (1042-1086)
  • get_resources (892-939)
  • get_resource_templates (964-1015)
src/fastmcp/server/providers/fastmcp_provider.py (1)
src/fastmcp/server/server.py (4)
  • get_tools (793-837)
  • get_resources (892-939)
  • get_resource_templates (964-1015)
  • get_prompts (1042-1086)
🪛 Ruff (0.14.10)
src/fastmcp/server/server.py

815-815: Unused lambda argument: context

(ARG005)


914-914: Unused lambda argument: context

(ARG005)


988-988: Unused lambda argument: context

(ARG005)


1064-1064: Unused lambda argument: context

(ARG005)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Run tests with lowest-direct dependencies
  • GitHub Check: Run tests: Python 3.10 on windows-latest
  • GitHub Check: Run tests: Python 3.13 on ubuntu-latest
  • GitHub Check: Run tests: Python 3.10 on ubuntu-latest
🔇 Additional comments (12)
docs/development/upgrade-guide.mdx (1)

75-93: LGTM! Clear and concise upgrade documentation.

The new section follows MDX documentation best practices: it uses the CodeGroup component appropriately, provides realistic before/after code examples, and explains the rationale for the change (dict key redundancy). The migration path using next() with a generator expression is idiomatic Python.

src/fastmcp/server/providers/fastmcp_provider.py (4)

477-485: LGTM! Consistent migration to public API.

The change from private _list_tools_middleware() to public get_tools(apply_middleware=True) aligns with the PR's consolidation objective. The wrapping logic correctly continues to wrap each tool in FastMCPProviderTool.


496-504: LGTM! Consistent with other list methods.


515-524: LGTM! Consistent with other list methods.


538-545: LGTM! Consistent with other list methods.

src/fastmcp/utilities/inspect.py (1)

109-113: LGTM! Clean migration to public API.

The four calls are updated consistently to use the new public get_*(apply_middleware=True) methods. The comment on line 109 accurately describes the purpose ("via middleware to respect filtering and preserve metadata"), and the downstream iteration over the returned lists works correctly.

src/fastmcp/server/server.py (6)

793-837: LGTM! Well-structured middleware integration with proper deduplication.

The implementation correctly:

  1. Uses apply_middleware=True to create context and apply middleware chain
  2. Falls back to direct provider querying when apply_middleware=False
  3. Deduplicates by .key (first provider wins)
  4. Uses logger.exception for error logging (includes traceback)
  5. Returns list[Tool] instead of dict

The lambda lambda context: self.get_tools(apply_middleware=False) on line 815 intentionally ignores the context parameter because this is the terminal handler in the middleware chain that doesn't need context—middleware has already been applied. The static analysis warning (ARG005) is a false positive in this pattern.


892-939: LGTM! Consistent with get_tools implementation.

The get_resources method follows the same pattern as get_tools, with appropriate changes for resource-specific attributes (.key for deduplication, resources/list method name). The unused context parameter in the lambda (line 914) follows the same justified pattern.


964-1015: LGTM! Consistent with other get_ implementations.*

The get_resource_templates method maintains consistency with the other listing methods. The unused context parameter (line 988) follows the same middleware terminal handler pattern.


1042-1086: LGTM! Consistent implementation for prompts.

The get_prompts method completes the quartet of consolidated listing methods. All four methods now follow the same pattern with apply_middleware parameter and list return type.


2458-2483: LGTM! Correct usage for deprecated import_server.

The import_server method correctly calls get_*() without apply_middleware since it's doing a one-time static copy of components (not routing through middleware chains). The iteration over the returned lists is straightforward and correct.


1206-1221: No changes needed. The nested context creation in _list_tools_mcp is intentional and properly supported by design. The Context class explicitly implements nesting via ContextVar and its __aenter__ method handles parent-child context relationships by inheriting state. This is documented in the Context class as a supported feature, not a redundancy concern.

Likely an incorrect or invalid review 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

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

@marvin-context-protocol
Copy link
Copy Markdown
Contributor

Test Failure Analysis

Summary: 9 tests failed on Windows due to pytest-xdist worker crashes with ConnectionResetError. The failures are not in the tests themselves, but in the test infrastructure.

Root Cause: Workers crashed during parallel test execution on Windows with error: ConnectionResetError: [WinError 10054] An existing connection was forcibly closed by the remote host. This occurred specifically during OAuth/auth-related tests and appears to be triggered by changes in how exceptions are logged.

Suspected Issue: The PR changes error logging from logger.warning() to logger.exception() in several places (tools/resources/prompts/templates listing). The logger.exception() method captures and includes full exception context. On Windows with pytest-xdist, there may be an issue with:

  1. Exception objects not being properly serializable for inter-process communication
  2. The rich logging handler having issues with complex tracebacks on Windows
  3. Thread exception handling interfering with worker communication

The log shows: PytestUnhandledThreadExceptionWarning: Exception in thread Thread-1 (run_server) followed by the connection reset.

Suggested Solution:

  1. Revert the logging changes for now - change logger.exception() back to logger.warning() in src/fastmcp/server/server.py at lines ~830, ~929, ~1003, and ~1076 (in the PR branch). The exception is already being stored in the result variable, so we can log it without using .exception():
# Instead of:
logger.exception(f"Error listing tools from provider {provider}")

# Use:
logger.warning(f"Error listing tools from provider {provider}: {result}")
  1. Verify the fix by running the tests locally on Windows if possible, or pushing the change and letting CI validate.

  2. Alternative approach if the logging change is important: Investigate using exc_info=True parameter with better error handling:

logger.warning(f"Error listing tools from provider {provider}", exc_info=result if isinstance(result, BaseException) else False)
Detailed Analysis

Failed Tests (all worker crashes, not test logic failures):

  • tests/server/auth/test_enhanced_error_responses.py::TestContentNegotiation::test_json_when_only_json_accepted
  • tests/server/auth/test_oauth_proxy.py::TestOAuthProxyClientRegistration::test_register_client
  • tests/server/auth/test_oauth_proxy.py::TestErrorPageRendering::test_callback_error_returns_html_page
  • tests/server/auth/test_enhanced_error_responses.py::TestContentNegotiation::test_json_when_no_accept_header
  • tests/server/auth/test_oauth_proxy.py::TestFallbackAccessTokenExpiry::test_fallback_parameter_defaults_to_none
  • tests/server/auth/test_oauth_proxy_redirect_validation.py::TestOAuthProxyRedirectValidation::test_proxy_default_allows_all
  • tests/server/auth/test_oauth_proxy.py::TestOAuthProxyClientRegistration::test_get_registered_client
  • tests/server/auth/test_oauth_proxy_redirect_validation.py::TestOAuthProxyRedirectValidation::test_proxy_custom_patterns
  • tests/server/auth/test_oidc_proxy.py::TestOIDCProxyInitialization::test_custom_token_verifier_with_audience_allowed

Error Excerpt:

maximum crashed workers reached: 8
ConnectionResetError: [WinError 10054] An existing connection was forcibly closed by the remote host

Key Code Change (src/fastmcp/server/server.py):

- logger.warning(f"Failed to get tools from {provider}: {result}")
+ logger.exception(f"Error listing tools from provider {provider}")

This pattern was repeated in 4 places for tools, resources, resource_templates, and prompts.

Related Files
  • src/fastmcp/server/server.py - Contains the logging changes that may be causing worker crashes
  • Tests in tests/server/auth/ - Where failures occurred, though the issue is infrastructure-related
  • .github/workflows/run-tests.yml - Test configuration with Windows parallel execution

@jlowin jlowin merged commit 125f79f into main Dec 25, 2025
17 of 18 checks passed
@jlowin jlowin deleted the consolidate-get-list-methods branch December 25, 2025 03:26
@marvin-context-protocol
Copy link
Copy Markdown
Contributor

Test Failure Analysis

Summary: Windows-only test failure in test_azure_specific_scopes caused by a pytest-xdist worker crash, not related to the PR changes.

Root Cause: The failure is a flaky infrastructure issue where a pytest-retry server thread encountered a ConnectionResetError: [WinError 10054] on Windows. The error message "worker 'gw0' crashed while running 'tests/server/auth/providers/test_azure.py::TestAzureProvider::test_azure_specific_scopes'" indicates the test worker died unexpectedly due to connection handling issues in the test infrastructure, not in the actual test code.

Evidence this is not related to the PR:

  • The test passed on Ubuntu (Python 3.10 and 3.13) and in lowest-direct dependencies mode
  • The test test_azure_specific_scopes (lines 213-237 in test_azure.py) is a simple synchronous test that only initializes an AzureProvider and checks its attributes - no network or async operations
  • The PR changes are about consolidating get_/list_ methods and returning lists instead of dicts, which doesn't affect Azure provider initialization
  • The ConnectionResetError is in pytest_retry/server.py (test infrastructure), not application code

Suggested Solution: Retry the failing workflow. This is a known class of flaky test failures on Windows with pytest-xdist where workers can crash due to connection handling issues. The test itself is sound and the changes in this PR don't touch the code path being tested.

If the failure persists on retry, consider:

  1. Marking this specific test with @pytest.mark.flaky(reruns=2) for Windows only
  2. Investigating pytest-retry configuration for Windows compatibility
Detailed Analysis

Failure Log

worker 'gw0' crashed while running 'tests/server/auth/providers/test_azure.py::TestAzureProvider::test_azure_specific_scopes'

Traceback (most recent call last):
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\threading.py", line 1016, in _bootstrap_inner
    self.run()
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\threading.py", line 953, in run
    self._target(*self._args, **self._kwargs)
  File "D:\a\fastmcp\fastmcp\.venv\lib\site-packages\pytest_retry\server.py", line 45, in run_server
    chunk = conn.recv(4096)
ConnectionResetError: [WinError 10054] An existing connection was forcibly closed by the remote host

Test Code (lines 213-237)

The failing test is straightforward - it just creates an AzureProvider instance and verifies attributes:

def test_azure_specific_scopes(self):
    provider = AzureProvider(
        client_id="test_client",
        client_secret="test_secret",
        tenant_id="test-tenant",
        base_url="https://myserver.com",
        required_scopes=["read", "write", "admin"],
        jwt_signing_key="test-secret",
    )
    assert provider is not None
    assert provider._token_validator.required_scopes == ["read", "write", "admin"]

No async operations, no network calls, no reason this should cause a worker crash.

Job Results

  • ✅ Ubuntu Python 3.10: Passed (3103 passed)
  • ✅ Ubuntu Python 3.13: Passed
  • ✅ Lowest-direct dependencies: Passed
  • ✅ Integration tests: Passed
  • Windows Python 3.10: Failed (1 failed, 3103 passed) ← Only this combination failed

The fact that 3103 other tests passed on Windows including many other Azure provider tests confirms this is an infrastructure flake, not a code issue.

Related Files
  • tests/server/auth/providers/test_azure.py:213-237 - The failing test (simple provider initialization)
  • CI logs show the error originates in pytest_retry/server.py not application code
  • No files in this PR touch Azure provider initialization logic

jlowin added a commit that referenced this pull request Dec 25, 2025
Add call_tool(), read_resource(), render_prompt() methods with apply_middleware parameter, following the pattern from #2719.
@jlowin jlowin removed the breaking change Breaks backward compatibility. Requires minor version bump. Critical for maintainer attention. label Jan 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvement to existing functionality. For issues and smaller PR improvements. server Related to FastMCP server implementation or server-side functionality. v3 Targeted for FastMCP 3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant