fix: rate limit MCP calls by API key (ORB-8) - #462
Conversation
|
Approach for ORB-8:
The expected files are |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7cfd4d1bd3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
DEGRADED: same-vendor review Same-vendor bias direction is known: this review can favor code from the same vendor family. Its magnitude in a real PR loop is unmeasured. Verdict: BLOCKING Head: Blocking findings[HIGH] The one-minute MCP policies reset their full allowances every 15 seconds.
[HIGH] General MCP rate-limit errors lose the JSON-RPC request id.
Non-blocking follow-ups
N/A dimensions
Dimensions 2, 3, 4, 6, and 11 were applicable and produced no additional findings. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a18530d641
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |



DEGRADED: same-vendor review
Implements ORB-8.
Summary
get_daily_summary,get_goal_review, andget_retrospectivecalls.execute_agent_operation_v2wrapsget_daily_summary,get_retrospective, orreview_goals.Retry-After, and a JSON-RPCrate_limit_exceedederror.Design choices
The ticket did not prescribe numeric budgets. The general policy uses 60 per minute because the existing
tagspolicy uses that budget. The AI policy uses 15 per minute because the existingai-operationsandhabit-suggestpolicies use that budget.AI classification lives in the MCP middleware so rate limiting happens before tool execution and can return an explicit transport-level 429. Both policies use the existing database-backed
DistributedRateLimitService, so counts are shared across instances.The middleware now deserializes the envelope through the installed SDK's
JsonRpcMessage,JsonRpcRequest, andCallToolRequestParamstypes. It does not hand-parse an assumed MCP shape. Wrapped operation IDs come from the verifiedparams.arguments.operationIdlocation, and their fingerprints use the innerparams.arguments.argumentsvalue, matchingAgentOperationExecutor.Validation
dotnet build Orbit.slnx --no-restore: 0 errorsdotnet test --no-build --no-restore: 5,605 passed, 0 failedWebApplicationExtensionsMcpTests: 31 passed, 0 failedExternal interface evidence
The installed package is
ModelContextProtocol.Core1.4.1. Its local nuspec pins source commit2b7fd35fbe58dfb9f00eae8b3393e1a7361b5e01.Complete serializable
tools/callrequest key set and types from that installed package:The SDK source-generation options use
WhenWritingNull, so optional null fields above are absent rather than serialized as null.CallToolRequestParams.Argumentsis the tool argument dictionary. For this repository'sexecute_agent_operation_v2method, that dictionary hasoperationId: string, optional innerarguments: JSON object, and optionalconfirmationToken: string, as defined byAgentTools.ExecuteAgentOperation.Re-derive from the installed package without credentials:
Inspect the generated files
ModelContextProtocol.Protocol/JsonRpcMessage.cs,JsonRpcMessageWithId.cs,JsonRpcRequest.cs,RequestId.cs,RequestParams.cs,CallToolRequestParams.cs, andMcpTaskMetadata.cs.ModelContextProtocol.Client/McpClient.csshowsCallToolAsyncsending methodtools/callwithCallToolRequestParams;ModelContextProtocol/McpSession.csconstructs theJsonRpcRequest. The positive middleware tests serialize these same installed SDK types and assert the complete emitted key set instead of hand-authoring a fixture.The authentication partition claims remain in-repository contracts:
JwtTokenServiceemitsClaimTypes.NameIdentifier, andApiKeyAuthenticationHandleremits both that claim andapi_key_id.Evidence
Backend-only change. No screenshot or visible-effect evidence applies.