Feature request: native Rate Limiting (MCP spec compliance) #388

Activity

1. 4t145

   added the

   Enhancement

   issue type on Aug 29, 2025
2. alexhancock

   added

   enhancementNew feature or requestNew feature or request

   P2Medium: important but non-blocking improvementMedium: important but non-blocking improvement

   T-coreCore library changesCore library changes

   help wantedExtra attention is neededExtra attention is needed

   on Feb 26, 2026
3. alexhancock commented on Feb 27, 2026

   

   alexhancock

   on Feb 27, 2026

   Contributor

   More actions

   This is true - we need to look into this

   👍React with 👍1Reacted by Dale Seo
4. github-actions commented on Jul 22, 2026

   

   github-actionsBot

   on Jul 22, 2026 – with GitHub Actions

   Contributor

   More actions

   This issue has had no activity for 60 days and is now marked as stale. It will be closed in 14 days if there is no further activity. Add the keep label to keep it open.
5. github-actions

   added

   Stale

   on Jul 22, 2026
6. darth-raijin commented on Jul 30, 2026

   

   darth-raijin

   on Jul 30, 2026

   More actions

   Hey, adding my two cents:

   Limiting on Transport::send() indeed simplifies the implementation of rate limiting and helps implementations conform to the specification.

   With that being said, I would lean towards making it a configurable building block, rather than having the SDK, which cannot know an application's specific requirements or capacity constraints, enforce a single policy.

   For example, different applications may want to rate limit based on the client's IP or the authenticated principal.

   Similarly, not all operations necessarily have the same computational cost. Some tool invocations may be computationally expensive or return large amounts of data, while others may be lightweight or heavily cached.

   It could therefore be useful if applications can configure different policies for different message types, while the SDK simply provides the hooks to integrate a rate limiting layer. I believe this would keep the SDK flexible while making it easier for implementations to comply with the specification.
7. github-actions

   removed

   Stale

   on Jul 31, 2026
8. IgorKasianenko commented on Aug 10, 2026

   

   IgorKasianenko

   on Aug 10, 2026

   More actions

   I would like to work on this. One design can cover what @bug-ops and @darth-raijin each ask for. Before writing anything real, though, I want to flag that the spec rules quoted in the issue body have changed since it was filed in 2025-08 against spec 2025-06-18. I read them again against 2026-07-28, which is the version rmcp targets (ProtocolVersion::V_2026_07_28):

   Rule as filed	State in 2026-07-28
   Completion: "MUST implement appropriate rate limiting"	Same. Still in Completion §Security: "Implementations MUST: … Implement appropriate rate limiting"
   Progress: "Both parties SHOULD implement rate limiting to prevent flooding"	Same. Progress §Implementation Notes
   Logging: "Servers SHOULD rate limit log messages"	Still there, but Logging is deprecated by SEP-2577. rmcp already marks those types #[deprecated]
   Sampling: "Clients SHOULD implement rate limiting"	Still there, but Sampling is deprecated by SEP-2577, and it now travels inside InputRequiredResult (MRTR), not as a sampling/createMessage JSON-RPC request
   Elicitation: "Clients SHOULD implement rate limiting"	Gone. It was 2025-06-18 §Security bullet 6. There is no such bullet in 2026-07-28. Elicitation also travels inside InputRequiredResult now

   Two things follow from that, and I think they should shape the design:

   1. The only MUST is inbound, on the server side. completion/complete is a ClientRequest, so a server receives it. A limiter that only runs in Transport::send() cannot meet that MUST for a server. It cannot meet it for a client either, because the client is the sender.
   2. Of the five rules, only Progress is served well by an outbound wrapper. Two are deprecated, one is gone, and the MUST is inbound. So a token bucket in send() on its own buys less than the issue body suggests. That is not a reason to drop it. It is a reason to scope it honestly and to say what we do about inbound traffic.

   Proposal

   Two constraints from the thread: the SDK should not force one policy on everyone (@darth-raijin), and a transport wrapper is the natural place for the outbound half (@bug-ops). Both hold if the SDK ships the mechanism plus an opt-in default, and changes nothing by default.

   Two small traits. They are split so that key extraction can borrow the message (sync), while the limiter itself can be async, which a Redis or other shared limiter has to be:

   /// Step 1 (sync, borrows the message): work out an owned limiter key.
   /// This is the hook for keying on client IP, authenticated principal, or per-tool
   /// cost. Those live in `Extensions`, and the SDK cannot know about them.
   pub trait RateLimitKeyExtractor<R: ServiceRole>: Send + Sync + 'static {
       type Key: Send + 'static;
       fn extract(&self, direction: Direction, message: &TxJsonRpcMessage<R>) -> Option<Self::Key>;
   }
   
   /// Step 2 (async, owns the key): ask for capacity.
   pub trait RateLimiter<K>: Send + Sync + 'static {
       fn acquire(&self, key: K) -> impl Future<Output = RateLimitDecision> + Send;
   }
   
   #[non_exhaustive]
   pub enum RateLimitDecision {
       Allow,
       Delay(Duration),                            // wait for capacity, then carry on
       Drop,                                       // notifications only, see below
       Reject { retry_after: Option<Duration> },   // typed error
   }

   The wrapper follows SinkStreamTransport: hold an Arc<L> and clone it into the async block, and build the inner send future up front so concurrent sends still work. The send future is Send + 'static, so nothing can borrow from &mut self. The Transport trait itself does not change, so no existing transport breaks.

   Drop is honoured for notifications and turned into Reject for requests. Dropping a notifications/progress is exactly what the spec wants. Quietly dropping a completion/complete would be a bug.

   The default extractor keys on the spec message class, so the common case needs no user code:

   let policies = HashMap::from([
       // SHOULD: shed progress spam rather than fail the send.
       (MessageClass::Progress,   Policy::new(20, 5.0, Exhausted::Drop)),
       // MUST: surfaced as a typed error, never dropped quietly.
       (MessageClass::Completion, Policy::new(10, 2.0, Exhausted::Reject)),
   ]);
   
   let transport = RateLimitedTransport::new(
       inner_transport,
       ByMessageClass::default(),         // or your own extractor
       TokenBucket::new(policies, None),  // or your own limiter
   );

   (Policy is built through Policy::new rather than a struct literal because the crate lints exhaustive_structs, so every public type here carries #[non_exhaustive].)

   The working code is on a branch in my fork, so it can be read as a diff rather than as code blocks here: main...IgorKasianenko:rate-limit-388 (2 files, +489). It is one commit on top of b5cf34e, built in the repo's dev container image, and it compiles against the current Transport trait with no change to it. Three tests cover the under-limit case, the over-limit case, and the split between message types: a completion over its limit returns LimitExceeded, and a progress notification over its limit is dropped and reported as success.

   $ rustc --version
   rustc 1.96.1 (31fca3adb 2026-06-26)
   
   $ cargo test -p rmcp --lib --features client,server transport::rate_limit
   running 3 tests
   test transport::rate_limit::tests::unclassified_messages_are_never_limited ... ok
   test transport::rate_limit::tests::under_the_limit_passes_through ... ok
   test transport::rate_limit::tests::over_the_limit_rejects_a_request_and_drops_a_notification ... ok
   
   test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 204 filtered out; finished in 0.00s

   just check, just test and the fmt half of just fix are all clean with the module in place:

   $ cargo clippy --all-targets --all-features -- -D warnings
       Compiling rmcp v3.1.2 (/workspace/crates/rmcp)
       Finished `dev` profile [unoptimized + debuginfo] target(s) in 4.77s
   
   $ cargo +nightly fmt --all -- --check
   $ echo $?
   0
   
   $ cargo test --all-features
   ... 86 test binaries: 774 passed; 0 failed

   I am not opening a PR until the shape is agreed here.

   Two notes on how this sits with the rest of the crate:

   • No new dependency. The token bucket needs only std::time::Instant, tokio::sync::Mutex and tokio::time::sleep, and rmcp already depends on all of those. It should still sit behind a feature (rate-limit?), off by default, to match how the crate gates everything else.
   • One small addition is needed for classification. ClientRequest::method() exists today. ServerRequest, ClientNotification and ServerNotification have nothing like it. A GetMethod trait implemented for all four would let the wrapper work for either role. It is purely additive: the bound goes on the wrapper's impl, not on ServiceRole.

   Questions before I write anything real

   1. Inbound. Should rate limiting also apply to receive()? That is where the real DoS risk sits for a server, and it is the only place the completion MUST can be met. But receive() -> Option<RxJsonRpcMessage> can only slow down (backpressure) or drop. It cannot reply with a JSON-RPC error, which is what a refused completion/complete should get. Is a service-layer hook in scope, or should this PR cover outbound only and leave inbound for a follow-up?
   2. Default behaviour. I assume opt-in only: no limiter unless the user wraps their transport, so nothing changes for current users. Would you rather have a default policy for completion/complete, given it is a MUST?
   3. Deprecated features. Should Sampling and Logging classes be included at all, given SEP-2577? I would include them, because peers on older protocol versions still exist, but not document them as recommended.
   4. Shape. Two traits plus a wrapper, or would you prefer one trait with a richer context object?

   Is this the right way to go with it? I would rather settle the shape here than in a PR.

   @DaleSeo @alexhancock — one small housekeeping point: .github/workflows/stale.yml exempts issues labelled keep, but that label does not exist in the repo yet, so nobody can apply it. This issue was marked stale on 2026-07-22 and only escaped because of the 07-30 comment. Could one of you create keep and put it on this issue, so the discussion does not get closed underneath it?

   🚀React with 🚀1Reacted by Mohamed Macow
9. ECD5A commented on Sep 22, 2026

   

   ECD5A

   on Sep 22, 2026

   More actions

   @IgorKasianenko, I read through your prototype and the inbound question you raised. I could help with a focused follow-up on the security test matrix rather than duplicate the transport design: verify that an exhausted completion/complete policy is surfaced as an error, while progress notifications are only dropped when the policy allows it, across the relevant inbound and outbound paths.

   Would that be useful as a separate test-focused contribution, or do you prefer to keep inbound handling for a later service-layer follow-up?