fix(agent-environment): advertise MCP client roots during list_tools#58
Open
Abhishek21g wants to merge 2 commits into
Open
fix(agent-environment): advertise MCP client roots during list_tools#58Abhishek21g wants to merge 2 commits into
Abhishek21g wants to merge 2 commits into
Conversation
…ols stalls Filesystem MCP servers request roots during startup; without a roots handler the FastMCP client times out or returns "List roots not supported".
Derive os.getenv default from DEFAULT_CLIENT_ROOTS and monkeypatch MCP_CLIENT_ROOTS in test_default_roots.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/data) so filesystem-related MCP servers get a timelyroots/listresponse instead of timing out or loggingList roots not supported.parse_client_roots()with unit tests and documentMCP_CLIENT_ROOTSinenv.template.Fixes #17, #18
Problem
During eval startup, MCP servers (notably filesystem) request initial roots from the client. The agent-environment
Clientwas created without arootshandler, which produces intermittent:This can slow or destabilize
list_toolswhen many servers start concurrently.Solution
Pass
roots=parse_client_roots()tofastmcp.Client. Default is/data, matchingdesktop-commanderallowedDirectoriesand filesystem paths inmcp_server_template.json. Override via comma-separatedMCP_CLIENT_ROOTS.Test plan
cd services/agent-environment && PYTHONPATH=src python3 -m pytest tests/test_client_roots.py tests/test_mcp_config_sync.py -vmake build && make run-dockerwithENABLED_SERVERS=filesystem,git— confirm no roots timeout spam in logs during startupcurl -X POST http://localhost:1984/list-toolssucceedsGreptile Summary
This PR fixes intermittent
roots/listtimeout errors duringlist_toolsstartup by registering filesystem roots with thefastmcp.Client. It adds a small, focusedparse_client_roots()helper, wires it intomcp_client.py, and documents theMCP_CLIENT_ROOTSenv var. Previously flagged issues (hardcoded default inos.getenv, env isolation in tests) have both been resolved.client_roots.py: New module withDEFAULT_CLIENT_ROOTS = ["/data"]andparse_client_roots()that correctly references the constant as theos.getenvfallback, with a safe empty-list guard.mcp_client.py: Callsparse_client_roots()at module level and passes the result toClient(roots=...), which FastMCP accepts as alist[str]and converts tomcp.types.Rootobjects internally.test_client_roots.py: Covers default, single, multi-root, and whitespace-stripping cases; a test for the env-var-is-set code path is missing but the rest of the logic is well validated.Confidence Score: 5/5
Safe to merge — the change is additive, touches only startup initialization, and both previously reported issues have been corrected.
The new code is small and isolated: a pure-function parser, a module-level call that reads an env var at startup, and a single extra kwarg on the existing Client constructor. FastMCP documents list[str] as a valid roots input and handles the conversion to mcp.types.Root internally. There are no data-path or auth changes, and the fix directly addresses a known startup race condition.
No files require special attention. The only gap is a missing test for the env-var-is-set code path in test_client_roots.py, but the runtime behavior is straightforward.
Important Files Changed
parse_client_roots()with correct DEFAULT_CLIENT_ROOTS reference in theos.getenvfallback and a safe fallback when the parsed list is empty.parse_client_roots()call and passes the result asroots=tofastmcp.Client;loggeris already initialised at line 11, so the newlogger.infocall is safe.monkeypatch), single root, multiple roots, and whitespace stripping; the env-var-is-set path forparse_client_roots(None)is not tested but the core logic is validated.MCP_CLIENT_ROOTSin the correct section with a commented-out example and a clear description of the default.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant E as Environment (MCP_CLIENT_ROOTS) participant CR as client_roots.py participant MC as mcp_client.py (module init) participant FC as fastmcp.Client participant MS as MCP Server (e.g. filesystem) MC->>CR: parse_client_roots() CR->>E: os.getenv("MCP_CLIENT_ROOTS", "/data") E-->>CR: "/data" (or custom paths) CR-->>MC: ["/data"] MC->>FC: "Client(config, roots=["/data"])" Note over FC: converts str to mcp.types.Root(uri=FileUrl) MS->>FC: roots/list request FC-->>MS: "[Root(uri="file:///data")]"%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant E as Environment (MCP_CLIENT_ROOTS) participant CR as client_roots.py participant MC as mcp_client.py (module init) participant FC as fastmcp.Client participant MS as MCP Server (e.g. filesystem) MC->>CR: parse_client_roots() CR->>E: os.getenv("MCP_CLIENT_ROOTS", "/data") E-->>CR: "/data" (or custom paths) CR-->>MC: ["/data"] MC->>FC: "Client(config, roots=["/data"])" Note over FC: converts str to mcp.types.Root(uri=FileUrl) MS->>FC: roots/list request FC-->>MS: "[Root(uri="file:///data")]"Reviews (2): Last reviewed commit: "test(agent-environment): isolate default..." | Re-trigger Greptile