Add session-specific visibility control via Context#2917
Conversation
Tools can now enable/disable components for their session only using ctx.enable_components() and ctx.disable_components(). Session rules override global transforms and persist across requests within the session.
Test Failure AnalysisSummary: The static analysis workflow failed because ruff auto-fixed an import ordering issue in Root Cause: The imports in Suggested Solution: Reorder the imports in import asyncio
from rich import print
from rich.panel import Panel
from fastmcp import Client
from server import serverThe proper order is:
After making this change, run Detailed AnalysisThe CI log shows: Running -from fastmcp import Client
from server import server
+
+from fastmcp import ClientThis is a standard import organization enforcement by ruff's I (isort) rules defined in Related Files
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f14b0dc594
ℹ️ 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".
| tools = await apply_session_transforms(tools) | ||
|
|
||
| # Then apply enabled filtering at the server level | ||
| tools = [t for t in await self.list_tools() if is_enabled(t)] | ||
| tools = [t for t in tools if is_enabled(t)] |
There was a problem hiding this comment.
Enforce enable/disable on direct component calls
Enabled filtering is now only applied in the list path (get_tools filters after apply_session_transforms), but call_tool/read_resource/render_prompt still call get_tool/get_resource/get_prompt, which (per Provider semantics) return disabled components without filtering and no longer have the FastMCP overrides. That means a client who already knows a disabled tool/resource/prompt name or URI can still execute/read/render it even after server.disable(...) or ctx.disable_components(...), defeating the intended access/visibility control. This is a behavior regression for disable-based gating and will allow hidden components to be invoked directly.
Useful? React with 👍 / 👎.
| # Send notifications based on components hint | ||
| if components is None or "tool" in components: | ||
| await self.send_notification(mcp.types.ToolListChangedNotification()) | ||
| if components is None or "resource" in components: | ||
| await self.send_notification(mcp.types.ResourceListChangedNotification()) |
There was a problem hiding this comment.
Notify resource list changes for template-only updates
Session visibility rules support components={'template'}, but _save_visibility_rules only emits tool/resource/prompt notifications. If a session enables/disables templates with components={'template'}, no resources/list_changed notification is sent, so clients that cache template lists will not refresh and will keep stale visibility. Since templates are listed via list_resource_templates but share the resources/list_changed channel, template-only updates should still trigger a resource list change notification.
Useful? React with 👍 / 👎.
Test Failure AnalysisSummary: Multiple tests failing because disabled components are not being filtered when fetching individual components by name/URI. Root Cause: The PR accidentally deleted four critical override methods from the
These methods call
Suggested Solution: Restore the four deleted override methods in Detailed AnalysisFailed tests:
Error pattern: The test expects Git diff evidence: git diff main...origin/session-visibility -- src/fastmcp/server/server.py | grep -A 5 "^-.*async def get_"Shows all four methods were deleted (lines starting with Related FilesPrimary file to fix:
For reference (shows correct implementation):
Test files showing expected behavior:
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds per-session visibility control to Context: async methods enable_components, disable_components, and reset_components persist visibility rules in session state and translate them into Enabled transforms. Introduces apply_session_transforms to convert session rules into transforms and apply them when assembling component lists. Server listing/getter methods now apply session transforms before filtering by is_enabled. Adjusts Enabled and provider enable/disable APIs to use set-based typing. Adds an examples namespace-activation server and client demonstrating session-scoped activation of tagged tools. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/fastmcp/server/transforms/enabled.py (1)
61-90: Defensively copy mutable criteria sets.
Storing caller-owned sets allows later mutation to change transform behavior unexpectedly.♻️ Suggested fix
- self.names = names - self.keys = keys + self.names = set(names) if names else None + self.keys = set(keys) if keys else None self.version = version - self.tags = tags # e.g., {"internal", "deprecated"} - self.components = components # e.g., {"tool", "prompt"} + self.tags = set(tags) if tags else None # e.g., {"internal", "deprecated"} + self.components = set(components) if components else None # e.g., {"tool", "prompt"}
| ``` | ||
| Namespace Activation Demo | ||
|
|
||
| ╭─────────────────── Initial Tools ───────────────────╮ | ||
| │ activate_finance, activate_admin, deactivate_all │ | ||
| ╰─────────────────────────────────────────────────────╯ | ||
|
|
||
| → Calling activate_finance() | ||
| Finance tools activated | ||
| ╭─────────────── After Activating Finance ────────────╮ | ||
| │ analyze_portfolio, get_market_data, execute_trade, │ | ||
| │ activate_finance, activate_admin, deactivate_all │ | ||
| ╰─────────────────────────────────────────────────────╯ | ||
|
|
||
| → Calling get_market_data(symbol='AAPL') | ||
| {'symbol': 'AAPL', 'price': 150.25, 'change': '+2.5%'} | ||
|
|
||
| → Calling activate_admin() | ||
| Admin tools activated | ||
| ╭────────────── After Activating Admin ───────────────╮ | ||
| │ analyze_portfolio, get_market_data, execute_trade, │ | ||
| │ list_users, reset_user_password, activate_finance, │ | ||
| │ activate_admin, deactivate_all │ | ||
| ╰─────────────────────────────────────────────────────╯ | ||
|
|
||
| → Calling deactivate_all() | ||
| All namespaces deactivated | ||
| ╭────────────── After Deactivating All ───────────────╮ | ||
| │ activate_finance, activate_admin, deactivate_all │ | ||
| ╰─────────────────────────────────────────────────────╯ | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the example output fence.
markdownlint MD040 expects a language identifier on fenced blocks.
🔧 Suggested fix
-```
+```text
Namespace Activation Demo
@@
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
25-25: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
Test Failure AnalysisSummary: The static analysis check failed because ruff auto-fixed an import ordering violation in the new example file, but the pre-commit hook rejects changes made during CI runs. Root Cause: In The pre-commit hook ran ruff, which automatically fixed this ordering issue, but because files were modified during the check, the workflow failed with exit code 1. This is the expected behavior - the code should be properly formatted before pushing. Suggested Solution: Run from rich import print
from rich.panel import Panel
from server import server
from fastmcp import ClientThis follows the standard import grouping: standard library → third-party (rich) → local modules (server) → blank line → project imports (fastmcp). Detailed AnalysisRelevant Log ExcerptThe Fix Applied by Ruff--- a/examples/namespace_activation/client.py
+++ b/examples/namespace_activation/client.py
@@ -8,9 +8,9 @@ import asyncio
from rich import print
from rich.panel import Panel
+from server import server
from fastmcp import Client
-from server import serverRelated Files
|
Test Failure AnalysisSummary: Test Root Cause: The test at The stack trace shows the test is stuck in: Suggested Solution: Pass a Change line 811-819 in proxy = OAuthProxy(
upstream_authorization_endpoint=mock_oauth_provider.authorize_endpoint,
upstream_token_endpoint=mock_oauth_provider.token_endpoint,
upstream_client_id="mock-client",
upstream_client_secret="mock-secret",
token_verifier=MockTokenVerifier(),
base_url="http://localhost:8000",
jwt_signing_key="test-secret",
)To: from key_value.aio.stores.memory import MemoryStore
proxy = OAuthProxy(
upstream_authorization_endpoint=mock_oauth_provider.authorize_endpoint,
upstream_token_endpoint=mock_oauth_provider.token_endpoint,
upstream_client_id="mock-client",
upstream_client_secret="mock-secret",
token_verifier=MockTokenVerifier(),
base_url="http://localhost:8000",
jwt_signing_key="test-secret",
client_storage=MemoryStore(), # Use in-memory storage to avoid Windows DiskStore timeout
)Detailed AnalysisFailed Test Log ExcerptThe test hangs when trying to initialize the SQLite database for Examples of Correct PatternOther tests in the same directory already use this pattern:
All of these pass Related Files
|
Tools can now dynamically enable/disable components for their session only, with rules that override global transforms and persist across requests.
Session A calls
activate_finance()→ sees finance tools.Session B doesn't → still sees only basic tools.
API:
ctx.enable_components(names=..., tags=..., version=..., components=...)ctx.disable_components(...)ctx.reset_components()- clear session rulesCloses #2911