Skip to content

fix(mcp): treat empty capability objects as present; declare keyring - #960

Merged
agentforce314 merged 1 commit into
agentforce314:mainfrom
fxinfo24:fix/mcp-empty-capability-objects
Oct 4, 2026
Merged

agentforce314 merged 1 commit into
agentforce314:mainfrom
fxinfo24:fix/mcp-empty-capability-objects

Conversation

@fxinfo24

@fxinfo24 fxinfo24 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Problem

An MCP server that advertises {"tools": {}} is silently reported as having no tools, and list_tools() returns [] — every tool the server exposes is hidden.

The MCP spec models capability members as objects whose presence signals support and whose contents are optional. _parse_server_capabilities collapsed them with bool(), and bool({}) is False.

Observed against a live server advertising 89 tools, of which ClawCodex surfaced 0. A sibling server advertising {"tools": {"listChanged": false}} worked purely because that dict happens to be truthy.

Root cause

tools=bool(tools_cap)          # {"tools": {}}  -> False
prompts=bool(caps.get("prompts"))
resources=bool(caps.get("resources"))

Fix

Test presence, not truthiness, for all three capabilities. The nested listChanged sub-flag keeps its existing isinstance(tools_cap, dict) guard, so the ch15 refresh wiring is unchanged.

Second issue: undeclared runtime dependencies

Two packages were imported by src/ but missing from dependencies:

  • keyring — the OS keychain backing McpTokenStore in services/mcp/auth.py. The backend allowlist in auth.py is already written against keyring 25.x class names, so the dependency was clearly intended; without it, token storage failed closed and 27 MCP tests errored at collection.
  • exceptiongroup (Python < 3.11) — anyio raises this backport, not the 3.11 builtin. _unwrap_exception_group_message guarded only with except NameError, so on 3.10 it returned str(exc) — literally the opaque "unhandled errors in a TaskGroup (1 sub-exception)" text the function exists to strip.

Verification

  • New regression test fails on the old code (AssertionError: False is not true), passes on the new.
  • MCP suite: 598 passed, 7 skipped on Python 3.11 (CI's pinned version).
  • Full suite: 10894 passed, 15 skipped, 0 failed on Python 3.11.

Behavioural note for reviewers

This changes how every MCP server is interpreted: a server advertising {"tools": {}} now loads its tools instead of none. That is the spec-correct reading, but it is a deliberate behaviour change, hence the explicit call-out rather than a silent fix.

Note on Python 3.10

requires-python = ">=3.10" but CI only runs 3.11, so 3.10 is effectively unverified today. The exceptiongroup fix above matters specifically on 3.10. Worth considering a 3.10 CI leg or narrowing the floor — happy to follow up separately.

`initialize` results are parsed with `bool(caps.get("tools"))`, but the MCP
spec models capability members as *objects* whose presence signals support
and whose contents are optional. A server advertising `{"tools": {}}` is
therefore read as having no tools, and `list_tools()` returns `[]` — hiding
every tool the server actually exposes. Verified against a live server that
advertises 89 tools and exposed none.

Test the presence of the member rather than its truthiness, for `tools`,
`prompts` and `resources` alike. The nested `listChanged` sub-flag keeps its
existing `isinstance` guard, so the refresh wiring is unaffected.

Also declares two runtime dependencies that were being imported without
being listed:

- `keyring`, the OS keychain backing `McpTokenStore` in
  `services/mcp/auth.py`. `auth.py`'s backend allowlist is already written
  against keyring 25.x class names, but the package was absent from
  `dependencies`, so token storage failed closed and 27 MCP tests errored.
  Backends resolve at runtime and `PlaintextKeyring` is explicitly rejected,
  so the package alone degrades to "no secure storage", not to leaking.
- `exceptiongroup`, on Python < 3.11 only. anyio raises this backport rather
  than the 3.11 builtin, so `_unwrap_exception_group_message` guarded only
  with `except NameError` returned `str(exc)` — i.e. exactly the opaque
  "unhandled errors in a TaskGroup" text the function exists to strip.

Adds a regression test covering the empty-capability case, and updates the
adjacent comment that documented the old behaviour as a known quirk.
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

Test Results

     5 files   1 021 suites   21m 59s ⏱️
16 105 tests 16 082 ✅ 22 💤 1 ❌
32 180 runs  32 108 ✅ 71 💤 1 ❌

For more details on these failures, see this check.

Results for commit 796aa32.

@agentforce314

Copy link
Copy Markdown
Owner

LGTM. Thank you!

@agentforce314
agentforce314 merged commit e4436c0 into agentforce314:main Oct 4, 2026
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants