feat(ratelimit): add admission limiter primitives - #44
Conversation
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
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 |
|
CodeRabbit (@coderabbitai) review Please review the pure admission-control core on the current draft head, especially opaque principal construction, HMAC domain separation, hard-cap overflow behavior, retry/reset arithmetic, stale eviction, clock rollback, and idempotent WebSocket release. Endpoint integration is intentionally not present yet. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Changed files ---'
git diff --name-status HEAD^ HEAD
printf '%s\n' '--- Diff summary ---'
git diff --stat HEAD^ HEAD
printf '%s\n' '--- Rate-limit related paths ---'
fd -i 'ratelimit|rate-limit|rate_limit|limiter|admission' . -t f | sort
printf '%s\n' '--- TypeScript outlines in changed files ---'
while IFS= read -r file; do
case "$file" in
*.ts|*.tsx)
printf '\n### %s\n' "$file"
ast-grep outline "$file" --items all --view expanded || true
;;
esac
done < <(git diff --name-only HEAD^ HEAD)Length of output: 1356 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- src/ratelimit/token-bucket.ts outline ---'
ast-grep outline src/ratelimit/token-bucket.ts --items all --view expanded || true
printf '%s\n' '--- src/ratelimit/token-bucket.ts patch ---'
git diff --no-ext-diff --unified=80 HEAD^ HEAD -- src/ratelimit/token-bucket.ts
printf '%s\n' '--- Current implementation ---'
cat -n src/ratelimit/token-bucket.ts
for file in \
tests/ratelimit.test.ts \
tests/forward-admission-separation.test.ts \
tests/rate-limit-reset-credits.test.ts
do
if [ -f "$file" ]; then
printf '\n--- %s ---\n' "$file"
cat -n "$file"
fi
doneLength of output: 56796 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- src/ratelimit files ---'
fd . src/ratelimit -t f | sort
printf '%s\n' '--- src/ratelimit source outlines ---'
for file in $(fd -e ts . src/ratelimit -t f | sort); do
printf '\n### %s\n' "$file"
ast-grep outline "$file" --items all --view expanded || true
done
printf '%s\n' '--- PR-base diff for src/ratelimit ---'
git diff --no-ext-diff --unified=40 HEAD^ HEAD -- src/ratelimit
printf '%s\n' '--- src/ratelimit source ---'
for file in $(fd -e ts . src/ratelimit -t f | sort); do
printf '\n### %s\n' "$file"
cat -n "$file"
done
printf '%s\n' '--- Typed limiter call sites ---'
rg -n -P --glob '*.ts' --glob '*.tsx' -C 4 '\.(consume|reserve)\s*\(' src tests || trueLength of output: 42525
High — stale release handles can remove post-reset reservations. For example:
The old handle decrements Capture a generation value in each accepted reservation. Increment the generation in private generation = 0;
reset(): void {
this.generation += 1;
this.counts.clear();
this.globalCount = 0;
// reset stats
}
reserve(/* ... */): ConcurrencyReservation {
// validate and admit
const generation = this.generation;
let released = false;
return Object.freeze({
accepted: true,
principalCount: nextPrincipal,
globalCount: this.globalCount,
release: () => {
if (released) return;
released = true;
if (generation !== this.generation) return;
// decrement the owned reservation
},
});
}Add a test that reserves, resets, reserves again, and then calls the first release handle. Verify that the second reservation remains counted. I found no other issue in the requested areas. The principal HMAC inputs are domain-separated. The token-bucket overflow state is bounded by surface count. The monotonic clock handling prevents token minting after rollback. The retry and reset values use integer ceiling arithmetic. ✅ Action performedReview finished.
|
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
|
[code]smith (@codesmith-bot) Continue implementation on the existing Required design:
Keep runtime changes scoped to rate limiting. Do not add dynamic plugins, another logger, another config validator, raw fingerprint diagnostics, or provider/model labels. |
|
Got it, I'm reviewing the PR comments first. |
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
greptile-apps Please review the current ready head |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
One important state change first: PR #44 was merged into The PR implements the full admission integration from
|
![View with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg)
Architecture triage 2026-08-02 (donor, not future SSOT)State: MERGED → Classification: Vault-auth: none in this diff. Explicitly deferred principal-from-auth and route wiring. Cross-repo Vault e2e is unrelated to this merge-ready status (already merged). Donor extraction candidates: |
1 participant
Summary
Implements the pure admission-control primitives from lane 3 of
structure/plugin-metrics-ratelimit-benchmarks.mdwithout activating rate limiting in any runtime route.Explicit boundaries
Originbypass exists in the API.Tests
tests/ratelimit.test.tscovers:Validation
This PR is intentionally inert until the follow-up integration lane imports it.
Greptile Summary
This change introduces process-local principal fingerprinting, token-bucket admission control, and WebSocket concurrency reservation primitives with focused tests. Runtime testing found that changing a token-bucket policy for an existing bucket reinterprets elapsed time using the new rate and burst: a low-to-high change admits requests that should remain limited, while a high-to-low change rejects a request that should be available. Update the token-bucket state transition before merging.
Confidence Score: 4/5
Not safe to merge until token-bucket policy changes preserve the governing policy for elapsed bucket state.
A controlled-clock execution reproduced both directions of the policy-transition failure against the real limiter: raising a policy after depletion over-admits requests, while lowering it can deny a request that should be admitted.
Files Needing Attention: src/ratelimit/token-bucket.ts needs an explicit policy-change transition and regression coverage in tests/ratelimit.test.ts.
What T-Rex did
Comments Outside Diff (1)
General comment
{ requestsPerMinute: 1, burst: 1 }at t=0 admits 10 requests at t=10s when the later call supplies{ requestsPerMinute: 600, burst: 10 }; unchanged low policy instead denies. Conversely, a bucket exhausted under the high policy at t=0 is denied at t=1s when the later call supplies the low policy, while unchanged high policy allows.BucketStatedoes not retain the policy that governed the elapsed interval, andrefill(state, policy, now)receives the latest policy input. Its calculation at lines 208–213 therefore applies the latest rate and burst cap retroactively.Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(ratelimit): invalidate release handl..." | Re-trigger Greptile
Context used: