Skip to content

Stop using ping as the non-cacheable result example - #513

Merged
koic merged 1 commit into
modelcontextprotocol:mainfrom
koic:fix_non_cacheable_cache_hints_test
Aug 13, 2026
Merged

Stop using ping as the non-cacheable result example#513
koic merged 1 commit into
modelcontextprotocol:mainfrom
koic:fix_non_cacheable_cache_hints_test

Conversation

@koic

@koic koic commented Aug 13, 2026

Copy link
Copy Markdown
Member

Motivation and Context

main is red on every Ruby version: "modern non-cacheable results carry no cache hints" errors with undefined method 'key?' for nil. The test reaches for response[:result] on a request that no longer produces a result.

Two changes met on main without conflicting textually. #499 added the test and picked ping as its example of a result that is not cacheable. #511 then made a modern-enveloped request naming a method SEP-2575 removed answer with -32601 even before the connection era locks, and ping is one of those methods, so the response now carries an error rather than a result. Each branch was green against the main it was written on; the pairing only failed once both had landed.

ping was the wrong example regardless. It is absent from the 2026-07-28 schema, so no modern request can name it and receive a result of any shape. tools/call is the accurate stand-in: the spec types CallToolResult as a plain Result, while the ones it types as CacheableResult are the discover, list, and read families. The test now calls the tool it already registers, and a comment records why ping cannot come back here - the same note #511 left where it replaced ping with prompts/list in the neighboring resultType test.

Only the test changes. The behavior both #499 and #511 implement is correct; the example the test chose was not.

How Has This Been Tested?

The test errors on main and passes with this change. bundle exec rake (tests, RuboCop, and conformance baseline) passes. The rest of the suite was searched for the same pattern, a modern-enveloped request naming a method the modern lifecycle removed: the only other occurrence is the assertion #511 added deliberately, which checks that such a request does not lock the era, and which reads no result.

Breaking Changes

None. No library code changes.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

## Motivation and Context

`main` is red on every Ruby version: "modern non-cacheable results carry no cache hints" errors with
`undefined method 'key?' for nil`. The test reaches for `response[:result]` on a request that no longer
produces a result.

Two changes met on `main` without conflicting textually. modelcontextprotocol#499 added the test and picked `ping` as its example of
a result that is not cacheable. modelcontextprotocol#511 then made a modern-enveloped request naming a method SEP-2575 removed answer
with `-32601` even before the connection era locks, and `ping` is one of those methods, so the response now carries
an error rather than a result. Each branch was green against the `main` it was written on; the pairing only failed
once both had landed.

`ping` was the wrong example regardless. It is absent from the 2026-07-28 schema, so no modern request can name it
and receive a result of any shape. `tools/call` is the accurate stand-in: the spec types `CallToolResult` as
a plain `Result`, while the ones it types as `CacheableResult` are the discover, list, and read families.
The test now calls the tool it already registers, and a comment records why `ping` cannot come back here -
the same note modelcontextprotocol#511 left where it replaced `ping` with `prompts/list` in the neighboring `resultType` test.

Only the test changes. The behavior both modelcontextprotocol#499 and modelcontextprotocol#511 implement is correct; the example the test chose was not.

## How Has This Been Tested?

The test errors on `main` and passes with this change. `bundle exec rake` (tests, RuboCop, and conformance baseline) passes.
The rest of the suite was searched for the same pattern, a modern-enveloped request naming a method the modern lifecycle removed:
the only other occurrence is the assertion modelcontextprotocol#511 added deliberately, which checks that such a request does not lock the era,
and which reads no result.

## Breaking Changes

None. No library code changes.
@koic
koic merged commit c3ca40f into modelcontextprotocol:main Aug 13, 2026
11 checks passed
@koic
koic deleted the fix_non_cacheable_cache_hints_test branch August 13, 2026 16:03
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.

1 participant