stacklok / stacklok/toolhive

Port the Redis session specs to RawMCPClient so they pin the Legacy era explicitly

Open
#6,090 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

needs-triage
Dominant language
Go
Stars
2.2k
Forks
300
Avg merge
1d 15h
Merged PRs (30d)
184

Description

The structural fact this rests on

The 2026-07-28 revision removed Mcp-Session-Id and the initialize handshake: there are no sessions on the Modern client edge, by design. So everything vMCP builds on sessions applies to Legacy clients only — Redis-backed cross-pod reconstruction, pod-restart recovery, lazy eviction of sessions terminated on another pod, and HMAC session token binding.

None of that is a bug, and none of it is broken by serving Modern. A Modern client has no session to share, bind or reconstruct, and each of its requests is authenticated independently — so token binding isn't bypassed, there is simply nothing to bind.

That distinction matters because it determines the fix: this is coexistence, not a capability gap. Redis session storage is deliberately not an entry in #6033's Modern capability gate (modernDispatchBlockers), and the contract comment there records the rule. A Redis-configured vMCP serves Modern happily: Legacy clients get shared reconstructible sessions, Modern clients are served statelessly and store nothing. That coexistence is asserted end-to-end by virtualmcp_dual_era_redis_test.go (#6056), including an explicit "Modern edge must not mint a session".

The actual problem: three specs can't pin their era

Three specs in test/e2e/thv-operator/virtualmcp/virtualmcp_redis_session_test.go build their Legacy session through CreateInitializedMCPClient:

  • :282 — cross-pod session reconstruction (pod A → pod B)
  • :423 — session recovery after pod restart
  • :655 — lazy eviction after termination on another pod

go-sdk v1.7's Connect is Modern-first — it probes server/discover before initialize — and the mcpcompat shim cannot pin a protocol version (#5911: ClientSessionOptions.protocolVersion is unexported and the shim passes nil options). So against a Modern-serving vMCP these clients negotiate Modern, get no session, and the sessionID must be assigned after Initialize assertions fail. Observed on #6033's earlier heads, e.g. virtualmcp_redis_session_test.go:426 on kind v1.35.1.

They fail for their client's reason, not the feature's.

#5911 does not block the fix

The raw client exists precisely for this: e2e.RawMCPClient with e2e.NewLegacyInitializeRequest / e2e.NewLegacyRequest pins the era per request, and the session ID comes back in the Mcp-Session-Id response header. virtualmcp_dual_era_redis_test.go's own header comment prescribes exactly this pattern, and #6051 is the precedent for making a spec's Legacy dependency explicit rather than incidental.

The port is mechanical: a legacyInitialize-style helper (see test/e2e/vmcp_dual_era_test.go:217) plus raw tools/list/tools/call with WithSessionID and MCP-Protocol-Version: 2025-11-25.

Since these specs cover Legacy-session semantics — the only kind that exist — pinning them to Legacy narrows nothing. It makes the spec's actual subject explicit.

Follow-up shape

  1. Port the three specs to RawMCPClient. Consider promoting a legacySessionClient helper into the operator e2e package so future session specs don't reach for the auto-negotiating client by default — that default is what turned a client limitation into a red shard.
  2. Longer term, ask upstream go-sdk to export the protocol-version pin (#5911's proper fix), after which mcpcompat could offer a Legacy-pinned constructor and this raw-request scaffolding becomes optional.

Refs #6033, #6056, #6051, #5911.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in test/e2e/thv-operator/virtualmcp/virtualmcp_redis_session_test.go at the three specs around lines 282, 423, and 655, then compare the raw-request pattern in virtualmcp_dual_era_test.go:217 and virtualmcp_dual_era_redis_test.go. Port those specs to e2e.RawMCPClient with Legacy requests, session IDs, and the stated protocol version; the Redis session assertions should pass while Modern remains stateless.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
testing-qa
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.