Skip to content

fix(mcpproxy): default caching hints on resources/read re-encode - #2782

Merged
nacx merged 5 commits into
theagentrouter:mainfrom
abdulaziz-dev-lab:fix/mcpproxy-resources-read-cachescope
Oct 6, 2026
Merged

nacx merged 5 commits into
theagentrouter:mainfrom
abdulaziz-dev-lab:fix/mcpproxy-resources-read-cachescope

Conversation

@abdulaziz-dev-lab

@abdulaziz-dev-lab abdulaziz-dev-lab commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Description

The legacy resources/read path decodes the backend result into mcp.ReadResourceResult to rewrite resource URIs and then re-encodes it. mcp.Cacheable has no omitempty (still true in go-sdk v1.8.0), so when a backend omits ttlMs/cacheScope, as pre-SEP-2549 servers do, the client receives "cacheScope":"". That is not a valid value, and strict clients reject the result (#2665). The list merges have set these fields since #2545, so this is the last place in internal/mcpproxy that re-encodes a cacheable result without them.

This sets a missing cacheScope to "private" before re-encoding. Read results can depend on the authenticated user and the gateway cannot tell when they don't, so it does not mark them shareable; the spec recommends "private" for such results [1]. A missing ttlMs decodes to 0, which the spec treats as immediately stale. Hints sent by the backend are passed through unchanged.

Tests: the existing resources/read case in Test_maybeResponseModify now uses a raw backend payload (the old fixture was built with json.Marshal(&mcp.ReadResourceResult{}) and already contained the empty scope). New cases check the encoded result when the backend omits the hints, sends an empty cacheScope, or sends its own (ttlMs: 5000, "public").

I also checked it end to end with aigw run in front of a minimal MCP server:

Build Backend result Client result
main {"contents":[...]} {"ttlMs":0,"cacheScope":"",...}
main {"ttlMs":0,"cacheScope":"",...} {"ttlMs":0,"cacheScope":"",...}
this PR {"contents":[...]} {"ttlMs":0,"cacheScope":"private",...}
this PR {"ttlMs":0,"cacheScope":"",...} {"ttlMs":0,"cacheScope":"private",...}
this PR {"ttlMs":5000,"cacheScope":"public",...} {"ttlMs":5000,"cacheScope":"public",...}

Related Issues/PRs (if applicable)

Fixes #2665 (the list responses were fixed in #2545; this covers resources/read)

Special notes for reviewers (if applicable)

1: https://modelcontextprotocol.io/specification/2026-07-28/server/utilities/caching#cache-scope-field

The legacy resources/read path decodes the backend result into
mcp.ReadResourceResult to rewrite URIs and re-encodes it. Cacheable has
no omitempty, so backends that omit ttlMs/cacheScope were proxied as
"cacheScope":"", which is not a valid value.

Normalize the hints with mergeCachingHintsFromBackends, matching the
list merge behavior from theagentrouter#2545, and add wire-level tests.

Related: theagentrouter#2665
Signed-off-by: Abdulaziz Alharbi <246710009+abdulaziz-dev-lab@users.noreply.github.com>
@abdulaziz-dev-lab
abdulaziz-dev-lab requested a review from a team as a code owner October 4, 2026 13:38
@netlify

netlify Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for theagentrouter canceled.

Name Link
🔨 Latest commit a499c81
🔍 Latest deploy log https://app.netlify.com/projects/theagentrouter/deploys/6ac5074857d99f00079b5759

Read results can depend on the authenticated user, and the gateway
cannot tell when they don't, so a missing cacheScope now becomes
"private" instead of "public". The caching spec recommends "private"
for user-dependent resources/read results. A missing ttlMs still
decodes to 0 (immediately stale), and hints sent by the backend are
passed through unchanged.

The tests also cover a backend that sends an empty cacheScope, and
check that an explicit "public" from the backend is kept.

Signed-off-by: Abdulaziz Alharbi <246710009+abdulaziz-dev-lab@users.noreply.github.com>
@nacx
nacx enabled auto-merge (squash) October 5, 2026 14:06
@codecov

codecov Bot commented Oct 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@abdulaziz-dev-lab

Copy link
Copy Markdown
Contributor Author

/retest

@abdulaziz-dev-lab

Copy link
Copy Markdown
Contributor Author

The red checks here are unrelated to this change. The Unit Test failures are the TestRunExtprocStartFailure flake on main (#2793, fix in #2794). The Envoy Gateway v1.8.5 e2e job timed out after its tests had passed, when a kubectl delete in a test cleanup hung. I'll merge main once #2794 lands.

@abdulaziz-dev-lab

Copy link
Copy Markdown
Contributor Author

/retest

1 similar comment
@abdulaziz-dev-lab

Copy link
Copy Markdown
Contributor Author

/retest

@abdulaziz-dev-lab

Copy link
Copy Markdown
Contributor Author

/retest

@nacx
nacx merged commit daa9f89 into theagentrouter:main Oct 6, 2026
63 of 65 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.

MCP proxy merge functions drop upstream cache metadata and emit invalid cacheScope: "" on list responses

2 participants