fix(usage_limit): atomic usage limit enforcement across concurrent requests

[raushan728]

The usage limiter was doing a read-then-increment in two separate steps, so concurrent requests could both pass the limit check before either one incremented.

Added check_and_increment to the store trait single mutex lock for in-memory, Lua script for Redis. Pre-checks all rules before incrementing; denied requests may still consume quota from earlier rules under concurrent load. TTL only set on first increment within a window.

[greptile-apps]

Greptile Summary

This PR replaces the old read-then-increment two-phase approach in check_and_record with a check_and_increment primitive that is atomic per-rule — backed by a single Mutex acquisition for the in-memory store and a Lua script for Redis. The pre-check get() phase is retained as an early-exit optimization to avoid touching any counters when a rule is clearly already over limit based on the last-seen count.

• UsageStore trait gains check_and_increment(key, delta, max, expiry) with a Redis Lua implementation (GET → compare → INCRBY → conditional EXPIREAT on first use) and an in-memory Mutex implementation that resets expired entries in the same lock.
• check_and_record now runs a non-atomic pre-check loop (using get()) to build pending_increments, then calls check_and_increment sequentially per rule; the multi-rule quota-leak limitation (earlier rules may be permanently incremented before a later rule denies) is explicitly documented in the doc-comment and test.
• Two new concurrency tests are added: a single-rule test that verifies the limit is respected under high concurrency, and a multi-rule test that asserts only the bottleneck rule's maximum is honoured (with a comment acknowledging the quota-leak side-effect on the lifetime counter).

Confidence Score: 4/5

Safe to merge for single-rule deployments; multi-rule configurations will experience a documented quota leak under concurrent load that is tracked as a follow-up.

The atomic check_and_increment correctly enforces limits within a single rule under concurrent load, which is the primary goal of this fix. The multi-rule quota-leak (earlier rules permanently increment before a later rule denies) is a real behaviour change from the old two-phase approach, but the team has acknowledged it and explicitly documented it in the doc-comment and the test. The u32::MAX overflow guard is present. No new correctness issues were found beyond what is already tracked.

crates/lib/src/usage_limit/usage_tracker.rs — the sequential check_and_increment loop for multi-rule configs warrants a second look once rollback support is available as a follow-up.

Important Files Changed

Filename	Overview
crates/lib/src/usage_limit/usage_store.rs	Adds check_and_increment to the trait and all three impls; Redis Lua script correctly guards TTL with TTL < 0; in-memory path has the u32::MAX overflow guard; ErrorUsageStore ignores get-error flag when deciding allow/deny.
crates/lib/src/usage_limit/usage_tracker.rs	Replaces two-phase commit with pre-check + sequential atomic increments; multi-rule quota-leak is documented but present; Phase 2 denial message omits count/max detail; new concurrent tests validate single-rule atomicity but multi-rule leak assertion is intentionally weak.

Sequence Diagram

sequenceDiagram
    participant R as Request
    participant T as UsageTracker
    participant S as UsageStore

    R->>T: check_and_record(ctx)

    note over T: Phase 1 — pre-check (non-atomic)
    loop For each rule
        T->>S: get(key)
        S-->>T: current count
        alt "current + delta > max"
            T-->>R: Denied (no counters touched)
        else passes
            T->>T: push to pending_increments
        end
    end

    note over T: Phase 2 — atomic increment per rule
    loop For each pending rule
        T->>S: check_and_increment(key, delta, max, expiry)
        note over S: Redis: Lua GET→compare→INCRBY→EXPIREAT
        S-->>T: allowed (bool)
        alt denied (concurrent race)
            T-->>R: Denied — earlier rules already incremented
        else allowed
            T->>T: continue
        end
    end

    T-->>R: Allowed

Loading

_{Reviews (10): Last reviewed commit: "chore(usage_limit): refine comments and ..." | Re-trigger Greptile}

[devin-ai-integration]

Devin Review found 2 potential issues.

View 4 additional findings in Devin Review.

[github-actions]

This PR has been inactive for 7 days and has been marked as stale. It will be closed in 5 days if there is no further activity.

[github-actions]

This PR has been inactive for 7 days and has been marked as stale. It will be closed in 5 days if there is no further activity.

[github-actions]

This PR has been inactive for 7 days and has been marked as stale. It will be closed in 5 days if there is no further activity.

[raushan728]

Just keeping this PR active

[dev-jodee]

small nits