Skip to content

chore: add caching for MCP registry client#1460

Open
qstearns wants to merge 1 commit intomainfrom
ex-mcp/registry-caching
Open

chore: add caching for MCP registry client#1460
qstearns wants to merge 1 commit intomainfrom
ex-mcp/registry-caching

Conversation

@qstearns
Copy link
Contributor

@qstearns qstearns commented Feb 2, 2026

Summary

Adds Redis-backed caching to the MCP registry client to reduce redundant HTTP requests to the registry backend.

  • Implements 24-hour TTL cache for both list servers and server details responses
  • Cache keys are derived from request URL + headers (hashed) to handle multi-tenant authentication
  • Optional cache dependency injection (passes nil to disable caching)
  • All existing tests updated to pass nil cache parameter

Implementation Details

  • Created CachedListServersResponse and CachedServerDetailsResponse types that implement cache.CacheableObject
  • Cache lookups occur after authorization headers are set
  • Cache stores happen on successful responses only
  • SHA-256 hash of sorted headers ensures proper cache isolation per tenant

Test plan

  • Existing unit tests pass with nil cache parameter
  • Cache integration tested in development environment
  • Verified cache key uniqueness across different tenants/auth contexts

🤖 Generated with Claude Code


Open with Devin

@qstearns qstearns requested a review from a team as a code owner February 2, 2026 22:10
@changeset-bot
Copy link

changeset-bot bot commented Feb 2, 2026

⚠️ No Changeset found

Latest commit: 96c4e7b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Feb 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
gram Ready Ready Preview, Comment Feb 6, 2026 5:59pm
gram-docs-redirect Ready Ready Preview, Comment Feb 6, 2026 5:59pm

Request Review

@qstearns qstearns changed the title Add caching for MCP registry client chore: add caching for MCP registry client Feb 2, 2026
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View issue and 4 additional flags in Devin Review.

Open in Devin Review

h := sha256.New()
for _, k := range keys {
vals := req.Header[k]
sort.Strings(vals)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Cache key generation mutates HTTP request headers in place

The registryCacheKey function mutates the original HTTP request's headers by sorting header values in place.

Click to expand

Issue

At server/internal/externalmcp/registry_cache.go:53-54, the code gets a reference to the header values slice and sorts it in place:

vals := req.Header[k]
sort.Strings(vals)

In Go, req.Header[k] returns the actual slice stored in the map, not a copy. When sort.Strings(vals) is called, it modifies the original slice, thereby mutating the HTTP request's headers.

Impact

The cache key is generated before the HTTP request is sent (see registryclient.go:202 and registryclient.go:316). This means the request headers are mutated before c.httpClient.Do(req) is called. While header order typically doesn't affect HTTP semantics, this:

  1. Violates the principle of least surprise - a cache key function shouldn't have side effects
  2. Could cause issues if the HTTP client, transport, or any middleware depends on header order
  3. Could cause issues if the request object is inspected or reused after this call

Expected Behavior

The cache key generation should not modify the input request. A copy of the values should be made before sorting.

Recommendation: Make a copy of the slice before sorting:

vals := req.Header[k]
valsCopy := make([]string, len(vals))
copy(valsCopy, vals)
sort.Strings(valsCopy)
_, _ = fmt.Fprintf(h, "%s=%s\n", k, strings.Join(valsCopy, ","))
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@github-actions github-actions bot added the preview Spawn a preview environment label Feb 2, 2026
@speakeasybot
Copy link
Collaborator

speakeasybot commented Feb 2, 2026

🚀 Preview Environment (PR #1460)

Preview URL: https://pr-1460.dev.getgram.ai

Component Status Details Updated (UTC)
✅ Database Ready Created and validated 2026-02-06 18:07:03.
✅ Images Available Container images ready 2026-02-06 18:06:24.

Gram Preview Bot

@speakeasybot speakeasybot removed the preview Spawn a preview environment label Feb 3, 2026
Copy link
Member

@walker-tx walker-tx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preview Spawn a preview environment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants