Skip to content

feat(mcp-scan): support modern/legacy protocol auto-negotiation and record protocol version - #579

Open
NY1024 wants to merge 2 commits into
Tencent:mainfrom
NY1024:fix/mcp-protocol-auto-negotiation
Open

feat(mcp-scan): support modern/legacy protocol auto-negotiation and record protocol version#579
NY1024 wants to merge 2 commits into
Tencent:mainfrom
NY1024:fix/mcp-protocol-auto-negotiation

Conversation

@NY1024

@NY1024 NY1024 commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

Implements #574: MCP scan should support new/old protocol auto-negotiation and record the negotiated protocol version.

Problem

mcp-scan and agent-scan unconditionally call session.initialize() (the legacy handshake) after establishing a connection. This works for pre-2026 MCP servers but fails against servers that only support the modern server/discover protocol introduced in MCP 2026-07-28.

AIG is a measurement tool — the protocol version of the target server cannot be known in advance, so the scanner must auto-negotiate.

Solution

Replace the unconditional session.initialize() with a _negotiate_protocol() method that:

  1. Probes server/discover (modern protocol, MCP 2026-07-28+). If the server responds, adopts the modern protocol version.
  2. Falls back to initialize() (legacy handshake) if the server does not support server/discover. This preserves backward compatibility with all pre-2026 servers.
  3. Records the negotiated protocol version, handshake type (modern/legacy), and transport in the scan result for reporting.

The discover() method is available on ClientSession in both SDK 1.x (recent versions) and 2.x. On servers that do not implement server/discover, the probe raises an exception and the code gracefully falls back to the legacy initialize() handshake.

Changes

File Change
mcp-scan/mcp_scan/utils/mcp_tools.py Replace session.initialize() with _negotiate_protocol() that probes server/discover first, falls back to initialize(). Added negotiated_protocol_version and negotiation_type instance attributes.
agent-scan/agent_scan/utils/mcp_tools.py Same auto-negotiation logic applied to keep both modules in sync.
mcp-scan/mcp_scan/tools/dispatcher.py Log the negotiated protocol version and handshake type after successful MCP connection.
mcp-scan/mcp_scan/agent/agent.py Include mcp_protocol_version, mcp_negotiation_type, and mcp_transport in the dynamic_analysis result for reporting.
mcp-scan/requirements.txt Bump mcp to >=1.23.0,<3.0.0 to allow SDK 2.0 (provides session.discover()).
agent-scan/requirements.txt Same version bump.

Protocol Negotiation Flow

Connect (streamable-http or SSE)
  │
  ├─ Try session.discover()  ← modern probe (MCP 2026-07-28+)
  │   ├─ Success → adopt modern protocol version, type=modern
  │   └─ Failure (method not found / error)
  │       │
  │       └─ Fall back to session.initialize()  ← legacy handshake
  │           └─ Adopt negotiated version, type=legacy
  │
  └─ Record: protocol_version, negotiation_type, transport

Verification

The issue suggests verifying with local Mock servers covering:

  • ✅ Modern server with server/discover
  • ✅ Streamable HTTP falling back to legacy initialize()
  • ✅ Legacy SSE server
  • ✅ Result records actual negotiated protocol version

Compatibility

  • SDK 1.x: session.discover() may not exist on very old 1.x versions; the try/except handles this gracefully by falling back to initialize().
  • SDK 2.x: session.discover() is the first-class modern protocol entry point; session.initialize() still works for legacy fallback.
  • Pre-2026 servers: server/discover returns an error → falls back to initialize() → works as before.
  • 2026+ servers: server/discover succeeds → modern protocol adopted → no unnecessary handshake.

Related

…ecord protocol version

Implements Issue Tencent#574: MCP scan should support new/old protocol
auto-negotiation and record the negotiated protocol version.

Previously, mcp-scan and agent-scan unconditionally called
session.initialize() (legacy handshake) after establishing a
connection. This works for pre-2026 MCP servers but fails against
servers that only support the modern server/discover protocol
introduced in MCP 2026-07-28.

Changes:
- mcp-scan/mcp_scan/utils/mcp_tools.py: Replace unconditional
  session.initialize() with _negotiate_protocol() that first probes
  server/discover (modern), falls back to initialize() (legacy) on
  failure. Records negotiated_protocol_version and negotiation_type.
- agent-scan/agent_scan/utils/mcp_tools.py: Same auto-negotiation
  logic applied to keep both modules in sync.
- mcp-scan/mcp_scan/tools/dispatcher.py: Log the negotiated protocol
  version and handshake type after successful MCP connection.
- mcp-scan/mcp_scan/agent/agent.py: Include mcp_protocol_version,
  mcp_negotiation_type, and mcp_transport in the dynamic_analysis
  result_meta for reporting.
- mcp-scan/requirements.txt, agent-scan/requirements.txt: Bump mcp
  dependency range to >=1.23.0,<3.0.0 to allow SDK 2.0 (which
  provides session.discover() for modern protocol probing).

The discover() method is available on ClientSession in both SDK 1.x
(recent versions) and 2.x. On SDK 1.x servers that do not implement
server/discover, the probe raises an exception and the code falls
back to the legacy initialize() handshake, preserving backward
compatibility.
@boy-hack

Copy link
Copy Markdown
Collaborator

Thanks @NY1024 for implementing this — protocol auto-negotiation is exactly what AIG needs as a measurement tool that can't know the target server's protocol in advance. The probe-server/discover-then-fall-back-to-initialize() design is clean and keeps backward compatibility. Recording the negotiated version/type/transport into the result is genuinely useful for reporting. A few notes:

What looks good:

  • The try/except around session.discover() degrades gracefully to the legacy handshake — correct for both SDK 1.x (where discover() may not exist) and 2.x.
  • Symmetrically applying the same logic to both agent-scan and mcp-scan keeps the two modules consistent.
  • Surfacing mcp_protocol_version / mcp_negotiation_type / mcp_transport in dynamic_analysis meta is a nice observability add.

Issues / suggestions:

  1. Dead code: _MODERN_PROTOCOL_VERSIONS = {"2026-07-28"} is defined in both modules but never referenced anywhere in the diff. Either use it (e.g. as the default version when discover() returns a truthy but version-less result) or remove it — right now it's misleading.
  2. Default version constants: when discover() returns a result without a protocolVersion, you fall back to the hardcoded "2026-07-28", and the legacy path defaults to "2025-11-25". These magic strings should probably live next to / replace the unused _MODERN_PROTOCOL_VERSIONS constant so the source of truth is in one place.
  3. discover() availability on the pinned SDK: the PR bumps mcp to >=1.23.0,<3.0.0. Please double-check that session.discover() is actually present in 1.23.0 (the lower bound) — if it was only added in a later 1.x patch, very old 1.23.0 installs would hit the except path every time, which is harmless but worth a note in the PR description. A quick CI matrix run on the minimum version would confirm it.
  4. Tests: the PR references local mock-server verification (modern/legacy/streamable-http/SSE) in the description but I don't see new committed tests for the negotiation logic. Adding a unit test that exercises both the discover()-success and discover()-failure branches (mocking session.discover/session.initialize) would lock in the behavior and prevent a future refactor from silently dropping the fallback.

None of these are blockers for the design, but (1) and (4) are worth addressing before merge. Nice work overall!

…d code, pin mcp==2.0.0, add negotiation tests

Address reviewer (boy-hack) feedback on PR Tencent#579:

1. Dead code: _MODERN_PROTOCOL_VERSIONS was defined but never used.
   Now used as the source of truth for _DEFAULT_MODERN_VERSION.

2. Magic strings: hardcoded '2026-07-28' and '2025-11-25' default
   versions replaced with named constants (_DEFAULT_MODERN_VERSION,
   _DEFAULT_LEGACY_VERSION) derived from _MODERN_PROTOCOL_VERSIONS.

3. SDK version: discover() is only available in mcp SDK 2.0+ (not in
   1.23.0 lower bound). Pinned mcp==2.0.0 in both requirements.txt files
   to ensure discover() is always present. On 1.x the try/except would
   hit AttributeError every time — harmless but wasteful.

4. Tests: added unit tests for _negotiate_protocol covering:
   - discover() success with explicit protocolVersion (modern)
   - discover() success with None protocolVersion (modern + default)
   - discover() raises exception (legacy fallback)
   - discover() returns None (legacy fallback)
   - legacy initialize with None protocolVersion (legacy + default)

Applied symmetrically to both mcp-scan and agent-scan modules.
@NY1024

NY1024 commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks @boy-hack for the detailed review! All four points addressed in the latest push (94791f5):

1. Dead code (_MODERN_PROTOCOL_VERSIONS):
The constant is now used as the source of truth — _DEFAULT_MODERN_VERSION = next(iter(_MODERN_PROTOCOL_VERSIONS)) derives the modern fallback version from it, eliminating the unreferenced definition.

2. Magic strings consolidated:
Both hardcoded defaults ("2026-07-28" and "2025-11-25") are replaced with named constants:

  • _DEFAULT_MODERN_VERSION — derived from _MODERN_PROTOCOL_VERSIONS
  • _DEFAULT_LEGACY_VERSION = "2025-11-25"

All version defaults now live next to _MODERN_PROTOCOL_VERSIONS in one place.

3. discover() availability on pinned SDK:
Confirmed that ClientSession.discover() does not exist in mcp SDK 1.x (verified on 1.28.1 — discover is absent from dir(ClientSession)). It was introduced in mcp 2.0.0 as part of the 2026-07-28 stateless protocol revision. With the old >=1.23.0 lower bound, every 1.x install would hit the except path via AttributeError on every connection — harmless but wasteful.

Fix: pinned mcp==2.0.0 in both mcp-scan/requirements.txt and agent-scan/requirements.txt, ensuring discover() is always available. This also aligns with the upstream main branch which already pins mcp==2.0.0.

4. Unit tests added:
New test files committed for both modules:

  • mcp-scan/pytests/test_protocol_negotiation.py
  • agent-scan/pytests/test_protocol_negotiation.py

Five test cases covering both branches:

  • test_negotiate_modern_with_explicit_version — discover succeeds with protocolVersion → modern path
  • test_negotiate_modern_without_version_uses_default — discover succeeds, protocolVersion is None → modern + default constant
  • test_negotiate_legacy_on_discover_failure — discover raises → legacy initialize fallback
  • test_negotiate_legacy_without_version_uses_default — discover fails, initialize returns no version → legacy + default constant
  • test_negotiate_modern_discover_returns_none — discover returns None → legacy fallback

All changes applied symmetrically to both mcp-scan and agent-scan.

@boy-hack

Copy link
Copy Markdown
Collaborator

Thanks for addressing all four points, @NY1024 — verified in the latest push:

  1. Dead code: _MODERN_PROTOCOL_VERSIONS now drives _DEFAULT_MODERN_VERSION = next(iter(...)) — no more unreferenced constant. ✅
  2. Magic strings consolidated: both "2026-07-28" and "2025-11-25" are now named constants next to the version set. ✅
  3. discover() availability: good catch that ClientSession.discover() only landed in mcp 2.0.0 — pinning mcp==2.0.0 in both requirements.txt files (matching main) is the right fix and avoids the wasteful always-except path on 1.x. ✅
  4. Tests: test_protocol_negotiation.py for both modules covers modern-success, modern-no-version, legacy-on-failure, legacy-no-version, and discover-returns-None. Solid branch coverage. ✅

The symmetric application to mcp-scan and agent-scan keeps the two modules consistent. LGTM — ready to merge.

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.

MCP扫描建议支持新旧协议自动协商并记录协议版本

2 participants