Skip to content

Pin forwarding tools/call against a stale-Legacy stateless backend - #6010

Merged
JAORMX merged 1 commit into
mainfrom
test/5992-forwarding-cached-legacy
Jul 27, 2026
Merged

Pin forwarding tools/call against a stale-Legacy stateless backend#6010
JAORMX merged 1 commit into
mainfrom
test/5992-forwarding-cached-legacy

Conversation

@JAORMX

@JAORMX JAORMX commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Part of #5992. That issue's headline problem — a Legacy-cached backend redeployed stateless/Modern staying pinned Legacy forever — was fixed in #5997 (legacyInit reads the genuinely-negotiated protocol version off every InitializeResult and flips the cache in-band, pinned by TestListCapabilities_MisCachedLegacy_SelfHealsViaSDKNegotiation). #6002 re-scoped the TTL re-probe as an open question, not requested work.

What remained from the issue body is the "also worth a test" half:

The "stale Legacy cache is harmless" property is currently pinned only for ListCapabilities (forwarding=false). The forwarding CallTool path (WithContinuousListening standalone GET stream) against a real stateless Modern backend is untested — low risk, but the harmlessness claim is asserted more broadly than it's covered.

This adds that test.

Why the forwarding arm is the one worth pinning. Binding forwarders makes newStreamableHTTPClient enable transport.WithContinuousListening(), which opens the standalone server→client GET stream. Opened against a session-less backend, that stream is exactly what made the pooled session-factory connect fail in #6006 (subscriptions/listen → "session not found"). The per-call legacy path survives because go-sdk's streamableClientConn.sessionUpdated opens that stream only when the negotiated protocol is < 2026-07-28 (mcp/streamable.go:2096) — and the SDK's Modern-first client negotiates Modern on the mis-cached-Legacy hop, so the gate suppresses the stream entirely. legacyInit then flips the cache to Modern in-band.

A regression in that version gate (or a shim change opening the stream unconditionally) would break only this forwarding path — invisible to the existing ListCapabilities pin. This test makes it fail loudly.

Type of change

  • Other (describe): test-only

Test plan

  • Unit tests (task test) — full pkg/vmcp/client package green with -race
  • Linting (task lint-fix)

TestCallTool_MisCachedLegacy_ForwardingAgainstStatelessBackend: binds stub forwarders, mis-caches Legacy against a real stateless (Modern) go-sdk backend, drives the forwarding CallTool, and asserts both the successful round-trip and the in-band cache flip to Modern. The harness dependency (factory Connect + legacyInit) is the exact path #6006 proved fatal without the gate, so the pin is non-vacuous.

API Compatibility

  • This PR does not break the v1beta1 API.

No production code changed.

Does this introduce a user-facing change?

No.

@github-actions github-actions Bot added the size/XS Extra small PR: < 100 lines changed label Jul 27, 2026
Part of #5992. The 'stale Legacy cache is harmless' property was pinned
only for ListCapabilities (forwarding=false). The forwarding tools/call
path is the risky arm: binding forwarders makes newStreamableHTTPClient
enable WithContinuousListening, whose standalone server->client GET
stream is what made the pooled session-factory connect fail against a
session-less backend in #6006.

The per-call path survives because go-sdk's streamableClientConn opens
that stream only when the negotiated protocol is < 2026-07-28, and the
SDK's Modern-first client negotiates Modern on the mis-cached-Legacy
hop — suppressing the stream entirely. legacyInit then flips the cache
to Modern in-band. Pin it against a real stateless go-sdk backend so a
regression in that version gate surfaces here, not in production.
@JAORMX
JAORMX force-pushed the test/5992-forwarding-cached-legacy branch from aded70e to 079ddc0 Compare July 27, 2026 11:52
@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Jul 27, 2026
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.08%. Comparing base (a6a0d4c) to head (079ddc0).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6010      +/-   ##
==========================================
- Coverage   72.15%   72.08%   -0.07%     
==========================================
  Files         720      720              
  Lines       74744    74744              
==========================================
- Hits        53929    53879      -50     
- Misses      16942    17010      +68     
+ Partials     3873     3855      -18     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JAORMX
JAORMX merged commit d390730 into main Jul 27, 2026
48 checks passed
@JAORMX
JAORMX deleted the test/5992-forwarding-cached-legacy branch July 27, 2026 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants