Skip to content

Replace vendored DI with uncalled-for#3301

Merged
chrisguidry merged 17 commits intomainfrom
uncalled-for-di
Mar 2, 2026
Merged

Replace vendored DI with uncalled-for#3301
chrisguidry merged 17 commits intomainfrom
uncalled-for-di

Conversation

@chrisguidry
Copy link
Copy Markdown
Collaborator

@chrisguidry chrisguidry commented Feb 25, 2026

FastMCP vendored a minimal DI engine extracted from Docket (~164 lines) with try/except fallback patterns everywhere. The uncalled-for package is a clean, typed extraction of this same system, and since Docket will also depend on it (chrisguidry/docket#353), uncalled_for.Dependency becomes the single canonical base class.

This deletes the _vendor/docket_di/ directory, replaces all the try/except import patterns with direct uncalled_for imports, and updates the Dependency.executioncurrent_execution ContextVar references to match the Docket branch. The Progress class now delegates to an internal impl and returns self from __aenter__ (matching Docket's pattern) so that ty's generic resolution works without type: ignore suppressions.

# before (repeated everywhere)
try:
    from docket.dependencies import Dependency
except ImportError:
    from fastmcp._vendor.docket_di import Dependency

# after
from uncalled_for import Dependency

Temporarily points pydocket at the use-uncalled-for branch so both sides can be validated together in CI. Once chrisguidry/docket#353 is merged and released, the pydocket dep goes back to a normal version pin.

Closes #3251
Makes #3292 better

(Maybe) closes #3320

🤖 Generated with Claude Code

@marvin-context-protocol marvin-context-protocol Bot added enhancement Improvement to existing functionality. For issues and smaller PR improvements. breaking change Breaks backward compatibility. Requires minor version bump. Critical for maintainer attention. labels Feb 25, 2026
@chrisguidry chrisguidry requested a review from jlowin February 25, 2026 22:49
@chrisguidry
Copy link
Copy Markdown
Collaborator Author

@jlowin this draft is still pointing to a branch of docket where I'm validating that we won't have any compatibility problems with 2.x when I release it. Will bump the version here once I'm sure.

@marvin-context-protocol
Copy link
Copy Markdown
Contributor

marvin-context-protocol Bot commented Feb 26, 2026

Test Failure Analysis

(Updated 2026-02-26 — edited to reflect latest workflow run 22445358624)

Summary: Two integration tests in TestGithubMCPRemote timed out (>30s) while waiting for a response from the GitHub Copilot remote MCP server (https://api.githubcopilot.com/mcp/). This is unrelated to the code changes in this PR.

Root Cause: The failures are transient network timeouts against an external third-party service. Both test_list_tools and test_call_tool_list_commits made real HTTP connections to https://api.githubcopilot.com/mcp/ and received no response within 30 seconds. The asyncio event loop was blocking on epoll_wait awaiting data that never arrived from the remote GitHub MCP server.

Suggested Solution: Re-run the failed integration test job. These failures are caused by external service unavailability, not by code changes in this PR. No code modifications are needed.

Detailed Analysis

Failing tests (both in tests/integration_tests/test_github_mcp_remote.py):

  • TestGithubMCPRemote::test_list_tools
  • TestGithubMCPRemote::test_call_tool_list_commits

Error pattern (identical for both):

Failed: Timeout (>30.0s) from pytest-timeout

The test logs show the client dispatched the call but the remote server never responded:

DEBUG    [Client-c605] called call_tool: list_commits    tools.py:133

The thread dump at timeout time shows the asyncio event loop stuck in I/O polling:

selectors.EpollSelector.select(timeout=29.745)
  fd_event_list = self._selector.poll(timeout, max_ev)
E  Failed: Timeout (>30.0s) from pytest-timeout.

These tests require FASTMCP_GITHUB_TOKEN and make live calls to the GitHub Copilot MCP API. The timeout indicates the remote endpoint was unreachable or slow at the time of the run.

Related Files
  • tests/integration_tests/test_github_mcp_remote.py — Contains both failing tests. These tests connect to https://api.githubcopilot.com/mcp/ using a bearer token and call real GitHub MCP tools.

@marvin-context-protocol
Copy link
Copy Markdown
Contributor

marvin-context-protocol Bot commented Feb 27, 2026

Test Failure Analysis

(Updated 2026-03-02 — edited to reflect latest workflow run 22596255403)

Summary: The integration test test_github_api_schema_performance timed out (>10s) during list_tools() due to slow schema dereferencing. This failure appears unrelated to the PR's DI library changes.

Root Cause: The timeout occurred inside _merge_ref_siblings in src/fastmcp/utilities/json_schema.py, which is called recursively during dereference_refs for each of the 500+ tools in the GitHub API schema. This function runs synchronously for every tool in list_tools() via DereferenceRefsMiddleware. The test fetches a live schema from github/rest-api-description main branch — if that schema has grown or if the CI runner was under load, the 10s timeout can be breached. The PR's changes (swapping _vendor/docket_di for uncalled-for) do not touch the schema dereferencing path.

Suggested Solution: Re-run the failed integration test job — this looks like a flaky failure due to CI environment performance variability and/or the live-fetched GitHub schema having grown. If it consistently fails after re-runs, the schema should be pinned to a specific commit/tag rather than fetching from main.

Detailed Analysis

Failing test: tests/server/providers/openapi/test_openapi_performance.py::TestOpenAPIPerformance::test_github_api_schema_performance

Error:

FAILED tests/server/providers/openapi/test_openapi_performance.py::TestOpenAPIPerformance::test_github_api_schema_performance
- Failed: Timeout (>10.0s) from pytest-timeout.

Stack trace at timeout:

tests/server/providers/openapi/test_openapi_performance.py:65: in test_github_api_schema_performance
    tools = await mcp_server.list_tools()
src/fastmcp/server/middleware/dereference.py:30: in on_list_tools
    return [_dereference_tool(tool) for tool in tools]
src/fastmcp/server/middleware/dereference.py:52: in _dereference_tool
    updates["output_schema"] = dereference_refs(tool.output_schema)
src/fastmcp/utilities/json_schema.py:101: in dereference_refs
    merged = _merge_ref_siblings(schema, dereferenced, defs)
src/fastmcp/utilities/json_schema.py:169: in _merge_ref_siblings  [repeated many times]
    result[key] = _merge_ref_siblings(original[key], value, defs, visited)

Why this is likely not caused by the PR:

  • The PR only modifies dependency injection internals (_vendor/docket_diuncalled-for), not schema processing code
  • _merge_ref_siblings and DereferenceRefsMiddleware are unchanged
  • The test docstring explicitly notes: "On a local machine, this test passes in ~2 seconds, but in GHA CI we see times as high as 6-7 seconds" — the 10s limit has always been tight
  • The schema is fetched live from refs/heads/main of github/rest-api-description, so schema growth over time can push the test over the limit
Related Files
  • src/fastmcp/utilities/json_schema.py_merge_ref_siblings / dereference_refs (CPU-intensive dereferencing logic)
  • src/fastmcp/server/middleware/dereference.pyDereferenceRefsMiddleware.on_list_tools (calls dereference_refs for each tool)
  • tests/server/providers/openapi/test_openapi_performance.py — failing performance test

@omikader
Copy link
Copy Markdown

I believe this also closes #3251

@chrisguidry
Copy link
Copy Markdown
Collaborator Author

I think it could! We can do a little more here to support Shared in the case where you don't have tasks enabled, let me take a look at that, it should be a fairly quick add.

@chrisguidry
Copy link
Copy Markdown
Collaborator Author

@omikader thanks for the nudge, this was definitely straightforward to ensure: 80526ba

@chrisguidry chrisguidry removed the breaking change Breaks backward compatibility. Requires minor version bump. Critical for maintainer attention. label Feb 27, 2026
@chrisguidry
Copy link
Copy Markdown
Collaborator Author

I took off the breaking change label because I disagree with that assessment, I've gone to a lot of trouble in Docket to make sure it's still presenting the same import surface and behavior as it did in 0.17.x

chrisguidry and others added 11 commits March 2, 2026 09:52
FastMCP vendored a minimal DI engine extracted from Docket (~164 lines)
with try/except fallback patterns everywhere. The `uncalled-for` package
is a clean, typed extraction of this same system, and since Docket will
also depend on it (chrisguidry/docket#353), `uncalled_for.Dependency`
becomes the single canonical base class.

This deletes the `_vendor/docket_di/` directory, replaces all the
try/except import patterns with direct `uncalled_for` imports, and
updates the `Dependency.execution` → `current_execution` ContextVar
references to match the Docket branch. The `Progress` class now
delegates to an internal impl and returns `self` from `__aenter__`
(matching Docket's pattern) so that ty's generic resolution works
without `type: ignore` suppressions.

Temporarily points pydocket at the `use-uncalled-for` branch so both
sides can be validated together in CI.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Internal code like azure.py should import from the fastmcp namespace
rather than reaching into uncalled_for directly.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
The DI engine now comes from uncalled-for, so the docs should credit
it alongside Docket. Also updates the Docket docs link to docket.lol.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Enters a SharedContext at server lifetime so that Shared() dependencies
from uncalled-for resolve once and are cached across tool/resource/prompt
calls. When running with docket, the Worker already handles this; this
covers the non-docket path and direct call_tool() usage.

Also re-exports Shared from fastmcp.dependencies.

Closes #3251

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@chrisguidry chrisguidry marked this pull request as ready for review March 2, 2026 16:29
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 108bd684c3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/fastmcp/server/server.py Outdated
chrisguidry and others added 2 commits March 2, 2026 15:03
The old `_ensure_shared_context` on the server called `__aenter__()` on a
lazy `SharedContext` but never `__aexit__()`, leaking the exit stack and
its resources. Moved the SharedContext management into Context's own
enter/exit so it's properly paired: when docket is available the lifespan
handles it, otherwise Context creates and cleans up a per-request one.

Updated Shared() tests to use Client (which runs the lifespan) rather
than calling server methods directly, since cross-request sharing
requires a lifespan.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Copy link
Copy Markdown
Member

@jlowin jlowin left a comment

Choose a reason for hiding this comment

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

@chrisguidry chrisguidry merged commit 547daf7 into main Mar 2, 2026
7 checks passed
@chrisguidry chrisguidry deleted the uncalled-for-di branch March 2, 2026 21:35
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Linter does not reckognize Depends anymore Support docket's Shared() dependencies in FastMCP tools

3 participants