feat: redo tool filtering for better tests by owtaylor · Pull Request #457 · rhel-lightspeed/linux-mcp-server

owtaylor · 2026-05-12T18:37:00Z

Filtering tools at server startup made it hard to test the effects of CONFIG.toolset. Switch to a system where tools are completely filtered in the middleware. (Doing it at the session level would be possible, but the direction for future protocol versions of MCP is to remove sessions entirely)

This allows making the tests for server.py more comprehensive and greatly reduces the amount of mocking necessary.

All tests for server.py are moved into a new test_server.py.
Instead of making "fixed" the default toolset for tools without a tag, fixed tools are tagged just like run_script tools are.
toolset.Toolset is modified to only have tags for inclusion rather than exclusion tags as well.
A new setup_client fixture is added that is more flexible than the old mcp_client fixture - it's callable and the desired toolset and mcp-app support are passed in.
Our mcp-app HTML resource is properly filtered along with the tools.

The end result is test coverage of server.py is increased from 70% => 95%.

github-actions · 2026-05-12T18:37:11Z

For team members: test commit 125099c in internal GitLab

codecov · 2026-05-12T18:38:31Z

Codecov Report

❌ Patch coverage is 97.10145% with 8 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
tests/conftest.py	90.00%	1 Missing and 3 partials ⚠️
src/linux_mcp_server/server.py	96.10%	2 Missing and 1 partial ⚠️
tests/test_server.py	99.21%	0 Missing and 1 partial ⚠️

Flag	Coverage Δ
unittests	`97.25% <97.10%> (+0.59%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/linux_mcp_server/tools/logs.py	`100.00% <ø> (ø)`
src/linux_mcp_server/tools/network.py	`100.00% <ø> (ø)`
src/linux_mcp_server/tools/processes.py	`100.00% <ø> (ø)`
src/linux_mcp_server/tools/run_script.py	`97.26% <ø> (ø)`
src/linux_mcp_server/tools/services.py	`80.00% <ø> (ø)`
src/linux_mcp_server/tools/storage.py	`97.82% <ø> (ø)`
src/linux_mcp_server/tools/system_info.py	`100.00% <ø> (ø)`
src/linux_mcp_server/toolset.py	`100.00% <100.00%> (ø)`
tests/test_toolset.py	`100.00% <100.00%> (ø)`
tests/tools/test_run_script.py	`100.00% <100.00%> (ø)`
... and 3 more

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

panyamkeerthana · 2026-05-12T19:22:01Z

-        # Tool must not have any excluded tags
-        if self.exclude_tags and self.exclude_tags.intersection(tool_tags):
+        # Tool must have one of the required tags
+        if self.tags and self.tags.isdisjoint(tool_tags):


I like the simplicity, much easier to understand than the old include/exclude kwargs stuff! Just want to make sure I understand the logic correctly,I want to confirm

self.tags = set() - Does this mean all tools get included?

tool_tags = set() - Does this mean the tool gets excluded from everything?

This part actually makes me confused a bit. self.tags is acting like an allow list when it contains a least one value, but when it's an empty set, it means no filtering just include any tools. Maybe it's just me. Anyway, since self.tags being an empty set is just never the case for now, we don't have to consider this edge case.

Jazzcort

Just left some of my thoughts. Everything looks good to me! Slick as always! 😁

Jazzcort · 2026-05-19T17:23:04Z



+def _hide_app_tools_for_client(client_params: InitializeRequestParams):
+    # Versions of goose before 1.29.0 don't understand _meta.ui.visiblity, so would


Just a quick question here. So in FastMcp 3.x, if the tools is not listed in tools/list, it not callable with tools/call. 🤔

Jazzcort · 2026-05-19T17:38:25Z

+            return False
+
+        if self.mcp_apps:
+            if "mcp_apps_exclude" in component.tags:


Found a probably wrong documentation for "mcp_apps_exclude" in scripts/generate_tool_docs.py when I was confused about why we have both mcp_apps_exclude and mcp_apps_only, but after I rubbed my head around it now it make whole lots of sense. 😁

Probably wrong documentation
It should be (e.g. Goose Desktop, Claude Desktop ...etc.) right?
If yes, I can do a separate patch for this.

if "mcp_apps_only" in tool.tags: notes.append("Only available with clients that support MCP apps (e.g. RHEL Lightspeed).")

Jazzcort · 2026-05-19T17:51:22Z

+
+    The dynamic behavior is done *per call*, not per session, since that's more future-proof.
+    (Future MCP protocol versions will remove sessions entirely:
+    https://modelcontextprotocol.io/seps/2575-stateless-mcp). This means doing it ourselves


FastMcp already has the sessionless option for the http transport. I hope the change will be just making it the default setting in the future. Anyway, we can always do it ourselves. 😁

Jazzcort · 2026-05-19T18:27:32Z

-        # Tool must not have any excluded tags
-        if self.exclude_tags and self.exclude_tags.intersection(tool_tags):
+        # Tool must have one of the required tags
+        if self.tags and self.tags.isdisjoint(tool_tags):


This part actually makes me confused a bit. self.tags is acting like an allow list when it contains a least one value, but when it's an empty set, it means no filtering just include any tools. Maybe it's just me. Anyway, since self.tags being an empty set is just never the case for now, we don't have to consider this edge case.

github-actions · 2026-05-19T19:21:16Z

For team members: test commit 402cbac in internal GitLab

owtaylor · 2026-05-19T19:24:41Z

This part actually makes me confused a bit. self.tags is acting like an allow list when it contains a least one value, but when it's an empty set, it means no filtering just include any tools. Maybe it's just me. Anyway, since self.tags being an empty set is just never the case for now, we don't have to consider this edge case.

Just pushed a new version differing only in that includes_tool is now:

   def includes_tool(self, tool_tags: set[str]) -> bool:
        # Tool must have one of the required tags
        return not self.tags.isdisjoint(tool_tags)

Hopefully everyone will be less confused now!

Filtering tools at server startup made it hard to test the effects of CONFIG.toolset. Switch to a system where tools are completely filtered in the middleware. (Doing it at the session level would be possible, but the direction for future protocol versions of MCP is to remove sessions entirely) This allows making the tests for server.py more comprehensive and greatly reduces the amount of mocking necessary. * All tests for server.py are moved into a new test_server.py. * Instead of making "fixed" the default toolset for tools without a tag, fixed tools are tagged just like run_script tools are. * toolset.Toolset is modified to only have tags for inclusion rather than exclusion tags as well. * A new setup_client fixture is added that is more flexible than the old mcp_client fixture - it's callable and the desired toolset and mcp-app support are passed in. * Our mcp-app HTML resource is properly filtered along with the tools.

github-actions · 2026-05-19T19:25:13Z

For team members: test commit 9d705a3 in internal GitLab

Jazzcort

LGTM! 😁

owtaylor requested review from Jazzcort and panyamkeerthana May 12, 2026 18:37

owtaylor requested a review from a team as a code owner May 12, 2026 18:37

panyamkeerthana reviewed May 12, 2026

View reviewed changes

owtaylor mentioned this pull request May 18, 2026

refactor: improve execution tool guidance for script execution #431

Merged

Jazzcort previously approved these changes May 19, 2026

View reviewed changes

owtaylor dismissed Jazzcort’s stale review via 402cbac May 19, 2026 19:21

owtaylor force-pushed the tool-filtering-for-better-tests branch from 125099c to 402cbac Compare May 19, 2026 19:21

owtaylor force-pushed the tool-filtering-for-better-tests branch from 402cbac to 9d705a3 Compare May 19, 2026 19:25

owtaylor requested a review from Jazzcort May 19, 2026 19:29

Jazzcort approved these changes May 19, 2026

View reviewed changes

owtaylor merged commit c371c4a into rhel-lightspeed:main May 19, 2026
33 of 34 checks passed



		def _hide_app_tools_for_client(client_params: InitializeRequestParams):
		# Versions of goose before 1.29.0 don't understand _meta.ui.visiblity, so would

Conversation

owtaylor commented May 12, 2026

Uh oh!

github-actions Bot commented May 12, 2026

Uh oh!

codecov Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

panyamkeerthana May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jazzcort May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Jazzcort left a comment

Choose a reason for hiding this comment

Uh oh!

Jazzcort May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Jazzcort May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Jazzcort May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Jazzcort May 19, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 19, 2026

Uh oh!

owtaylor commented May 19, 2026

Uh oh!

github-actions Bot commented May 19, 2026

Uh oh!

Jazzcort left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented May 12, 2026 •

edited

Loading

panyamkeerthana May 12, 2026 •

edited

Loading