fix: FastMCP v3 tag filtering and run_args UnboundLocalError in server setup#249
Conversation
…n server setup Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates CodeWeaver’s MCP server integration to accommodate FastMCP v3 API changes (tag filtering + stdio proxy setup) and prevents a stdio server setup crash caused by an uninitialized run_args.
Changes:
- Update
_setup_serverto removeinclude_tags/exclude_tagsfrom FastMCP constructor args and apply tag visibility viaapp.disable()/app.enable(). - Fix
create_stdio_serversorun_argsis always derived fromhttp_settingsregardless of which settings branch is taken. - Bump FastMCP and related dependency lock entries; minor unit-test import reformatting.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/codeweaver/server/mcp/server.py |
FastMCP v3 tag filtering changes + stdio proxy run_args initialization fix |
uv.lock |
Dependency lock updates (FastMCP >=3) and transitive dependency changes |
tests/unit/test_main.py |
Import formatting only |
tests/unit/core/types/test_embedding_batch_info_intent.py |
Import formatting only |
tests/unit/core/types/test_chunk_embeddings_properties.py |
Import formatting only |
Comments suppressed due to low confidence (2)
src/codeweaver/server/mcp/server.py:316
run_argscan beNoneif it comes from an unvalidatedFastMcpServerSettingsDict(e.g., user config setsrun_args: null). In that casesetup_runargs(run_args, ...)will raise because it expects a mapping. Consider normalizing right after popping (e.g.,run_args = mutable_args.pop("run_args") or {}) and/or validating intoFastMcpHttpRunArgsbefore callingsetup_runargs.
run_args = mutable_args.pop("run_args", {})
# Remove transport from args - it's not a FastMCP constructor parameter
mutable_args.pop("transport", None)
# FastMCP v3: include_tags/exclude_tags are no longer constructor parameters.
# Extract them here and apply via app.enable()/app.disable() after construction.
include_tags: set[str] | None = mutable_args.pop("include_tags", None)
exclude_tags: set[str] | None = mutable_args.pop("exclude_tags", None)
# Always use default middleware classes for this transport
from codeweaver.server.mcp.middleware import default_middleware_for_transport
middleware = default_middleware_for_transport(transport)
if is_http:
run_args = setup_runargs(run_args, host, port, verbose=verbose, debug=debug)
app = FastMCP(
src/codeweaver/server/mcp/server.py:333
include_tags/exclude_tagsare removed frommutable_argsand never passed intoCwMcpHttpState.from_app(...), soCwMcpHttpState.mcp_settingswill no longer reflect these settings (it is built from the kwargs infrom_appwheninstructionsis present). Consider adding these keys back into the kwargs passed tofrom_appso the state object still contains the fullFastMcpServerSettingsDict(even if FastMCP v3 no longer accepts them in the constructor).
# FastMCP v3: Apply tag-based visibility filtering using the new enable()/disable() API
if exclude_tags:
app.disable(tags=exclude_tags)
if include_tags:
app.enable(tags=include_tags, only=True)
if is_http:
from codeweaver.server.mcp.state import CwMcpHttpState
# Pass the instantiated middleware from app.middleware, not the classes
return CwMcpHttpState.from_app(
app, **(mutable_args | {"run_args": run_args, "middleware": app.middleware})
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else: | ||
| http_settings = _get_fastmcp_settings_map(http=True) | ||
| run_args = http_settings.get("run_args", {}) | ||
| run_args = http_settings.get("run_args", {}) |
There was a problem hiding this comment.
Code Review: FastMCP v3 Migration
This PR successfully addresses the FastMCP v3 migration issues. The changes are well-implemented and follow the correct migration pattern.
✅ Positive Aspects
-
Correct API Migration: The tag filtering migration from constructor parameters to post-construction methods is correct:
- Properly extracts
include_tagsandexclude_tagsbefore FastMCP construction - Applies them via the new
app.disable()andapp.enable()API after instantiation - Uses
only=Truefor allowlist behavior withinclude_tags, which matches the v3 pattern
- Properly extracts
-
Fixed UnboundLocalError: The
run_argsfix increate_stdio_serveris correct:- Moved from inside the
elseblock to after the if/else, ensuring it's always defined - This prevents the
UnboundLocalErrorwhensettingscontainsmcp_server
- Moved from inside the
-
Good Documentation: Clear inline comments explain the v3 API changes
-
Dependency Update:
uv.lockshows FastMCP upgraded from 2.14.5 to 3.1.1, and removed unused dependencies (cloudpickle, croniter, diskcache, fakeredis, lupa)
🔍 Minor Observations
-
Import Order Changes: The test file import reordering (moving local imports after third-party) and blank line additions are style improvements, though unrelated to the core fix. These follow PEP 8 conventions, which is good.
-
Lock File Changes: The uv.lock changes include:
- Removal of several unused packages (cloudpickle, croniter, diskcache, fakeredis, lupa)
- Addition of aiofile, caio (new FastMCP v3 dependencies)
- Addition of opentelemetry-api, uncalled-for, watchfiles, pyyaml (new FastMCP v3 features)
-
Type Annotations: The code correctly types
include_tagsandexclude_tagsasset[str] | None
🤔 Questions/Suggestions
-
Testing: Are there existing tests that cover tag filtering behavior? It would be good to ensure that:
- Tools/resources with
exclude_tagsare properly hidden - Tools/resources with
include_tagsare properly exposed - The allowlist behavior with
only=Trueworks as expected
- Tools/resources with
-
Computed Fields: I noticed that
include_tagsandexclude_tagsare defined as computed fields insrc/codeweaver/server/config/settings.py. These computed fields return the tags as sets, which matches the expected type for the v3 API.
✅ Verdict
The implementation is correct and addresses the stated issues. The code properly migrates from FastMCP v2 to v3 patterns. CI checks are pending, but the code changes look solid.
Recommendation: Approve once CI passes, assuming there are no test failures.
FastMCP v3 removed
include_tags/exclude_tagsasFastMCP()constructor parameters, causing errors when settings (which include these as computed fields) were passed directly. Additionally,run_argswas only assigned in theelsebranch ofcreate_stdio_server, producing anUnboundLocalErrorwhensettingsprovided amcp_serverkey.Changes
_setup_server: Popinclude_tagsandexclude_tagsfrommutable_argsbeforeFastMCP()construction; apply via the v3 visibility API post-construction:create_stdio_server: Moverun_argsextraction outside the if/else so it always reads fromhttp_settingsregardless of which branch populates it.🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.