Repository navigation
fix(rate-limit): restore Keyv limiter compatibility contracts - #501
Conversation
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
the
size:L
|
Warning Review limit reached
Next review available in: 7 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Note
|
| Layer / File(s) | Summary |
|---|---|
Storage and result contracts src/shared/utils/rateLimiter.ts |
Adds RateLimitRule, optional result metadata, explicit Redis configuration checks, Keyv storage setup, test mode, stale-window eviction, reset, and cleanup behavior. |
Atomic in-memory rule evaluation src/shared/utils/rateLimiter.ts |
Validates rule values and checks all in-memory windows before incrementing any counter. |
Persistent and public multi-rule flow src/shared/utils/rateLimiter.ts |
Adds asynchronous Keyv multi-window checks, local fallback protection, overloaded checkRateLimit, checkRateLimitWithRules, and checkRateLimitArray. Removes the legacy single-rule wrapper. |
Estimated code review effort: 4 (Complex) | ~45 minutes
Sequence Diagram(s)
sequenceDiagram
participant Caller
participant checkRateLimit
participant Keyv
participant LocalProtection
Caller->>checkRateLimit: submit key and rules
checkRateLimit->>Keyv: validate and increment windows
Keyv-->>checkRateLimit: counters or persistence error
checkRateLimit->>LocalProtection: apply fallback protection
checkRateLimit-->>Caller: return rate-limit result
Possibly related PRs
- KooshaPari/OmniRoute#404: Modifies the same rate limiter APIs and Redis compatibility behavior.
Suggested reviewers: copilot, diegosouzapw
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check | The description explains the scope and validation, but it omits most required template sections, including issues, tests, coverage, reviewer notes, and pillar checks. | Complete the required template sections and document test files, coverage impact, reviewer risks, related issues, and 71-pillar checks. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly identifies the primary change: restoring Keyv rate-limiter compatibility contracts. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
- Create stacked PR
- Commit on current branch
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
fix/rate-limiter-keyv-contract-20260805
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| const counts = await Promise.all(keys.map((key) => store.get<number>(key))); | ||
| for (let index = 0; index < rules.length; index += 1) { | ||
| if ((counts[index] ?? 0) >= rules[index].limit) { | ||
| return { allowed: false, failedWindow: rules[index].window }; | ||
| } | ||
| } | ||
| await Promise.all( | ||
| rules.map((rule, index) => { | ||
| const remainingMs = (Math.floor(nowSeconds / rule.window) + 1) * rule.window * 1000 - nowMs; | ||
| return store.set(keys[index], (counts[index] ?? 0) + 1, remainingMs + 1000); | ||
| }) |
There was a problem hiding this comment.
Suggestion: The persistent path performs a non-atomic read/check followed by independent writes. Concurrent requests can read the same counts and overwrite each other's increments, allowing more requests than the configured limits and violating the all-windows increment contract. Use an atomic transaction or backend-supported increment-and-check operation. [race condition]
Severity Level: Critical 🚨
- ❌ Concurrent API-key requests can exceed configured limits.
- ⚠️ Multi-window enforcement is not atomic across instances.
- ⚠️ Rate-limit rejection at apiKeyPolicy.ts:657-668 can be bypassed.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/shared/utils/rateLimiter.ts
**Line:** 159:169
**Comment:**
*Race Condition: The persistent path performs a non-atomic read/check followed by independent writes. Concurrent requests can read the same counts and overwrite each other's increments, allowing more requests than the configured limits and violating the all-windows increment contract. Use an atomic transaction or backend-supported increment-and-check operation.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
Partially fixed in 44122b7: serialized Keyv check-and-write execution prevents the demonstrated concurrent interleaving within a running server process; the new regression admits exactly one of 12 simultaneous requests at limit 1. Cross-process atomicity requires a transactional backend API not exposed by @keyv/sqlite, so this thread remains open rather than claiming that broader guarantee.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b8c53c62e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/shared/utils/rateLimiter.ts`:
- Around line 220-231: Add automated tests for checkRateLimitWithRules covering
Keyv failure fallback, duplicate-rule handling, bounded local state, and
concurrent persistent checks. Reuse the existing rate-limiter test setup and
assert each scenario’s allowed/limited behavior, ensuring the production changes
in src/shared/utils/rateLimiter.ts are exercised.
- Around line 121-130: Update validateRules and the public rate-limiter entry
points to validate complete inputs with the shared Zod schemas from
src/shared/validation/schemas.ts. Parse suppliedRules and keyId before
generating keys or mutating limiter state, and use the parsed values throughout;
remove the manual rule-validation path where the schema provides equivalent
validation.
- Around line 121-130: Update validateRules to reject duplicate rule.window
values during validation, tracking windows seen while mapping or validating
rules and throwing a TypeError for repeats; preserve the existing
positive-integer validation and return behavior for unique rules.
- Around line 56-59: Bound both FALLBACK_MEMORY_STORE and positionalCounters
with expiry-aware storage and a hard capacity. Update their insertion and
cleanup paths so expired windows are removed and capacity pressure evicts only
inactive/expired entries, never active rate-limit state; retain
TEST_MEMORY_STORE behavior unless required by the shared storage implementation.
Apply the change consistently to the related paths referenced in the diff, using
EVICTION_THRESHOLD or the established capacity configuration.
- Around line 75-80: Update getRedisClient() to return the existing
Keyv/SQLite-backed Redis compatibility adapter when Redis is configured, instead
of unconditionally throwing “Redis is not available.” Preserve the “Redis is not
configured” error for unconfigured environments and ensure the returned adapter
supports the get, set, and del operations used by apiKeys.ts and apiKeyCache.ts.
- Around line 152-175: Move getKeyvStore() inside the try block of
checkKeyvRateLimit so initialization failures reach the fallback handler. Change
the catch to receive unknown errors, log the sanitized error via
sanitizeErrorMessage() with the existing pino context, then invoke
checkInMemoryRateLimit(FALLBACK_MEMORY_STORE, keyId, rules).
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 10f37c45-9bb8-4a25-8660-474914041bf1
📒 Files selected for processing (1)
src/shared/utils/rateLimiter.ts
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
L17 Latency Regression ReportThreshold: 10% p99 regression. |
L17 Latency Budget ReportChecked against: budgets/rest-endpoints.yaml. |
User description
Scope
Applies the isolated
f3b8ac89rate-limiter extraction only:src/shared/utils/rateLimiter.ts. It deliberately does not adopt the broad preservedfix/keyv-limiter-contracts-20260804branch.Provenance
f3b8ac89d0013fbc22fe3df3a120c40440e0898a34ff9073478b8c53c62e972118e7199ab9bd7e947dc2f4f5b3Validation
git diff --check origin/main...HEADvitest run --config vitest.config.ts tests/e2e/rate-limit.e2e.ts: 4 passed, 0 failedCodeAnt-AI Description
Restore compatible rate-limit behavior across API-key policies
What Changed
Impact
✅ Consistent enforcement across multiple rate-limit windows✅ Fewer unexpected Redis connection failures✅ Clearer invalid rate-limit configuration errors💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
New Features
Bug Fixes