Skip to content

fix(engine): rate-limit.ts retryAfterMs unbounded under backward clock skew #5829

Description

@JSONbored

Context

packages/loopover-engine/src/governor/rate-limit.ts's evaluateLocalRateLimit is the pure, deterministic bucket-math primitive backing the Governor's local rate limiting. It computes retryAfterMs — how long a blocked caller should wait — from a resetAtMs derived from the bucket's recorded window start and the current clock reading:

const windowElapsed = now - windowStartMs >= windowMs;
const effectiveWindowStart = windowElapsed ? now : windowStartMs;
const resetAtMs = effectiveWindowStart + windowMs;
...
const retryAfterMs = allowed ? 0 : Math.max(0, resetAtMs - now);

windowElapsed only detects a forward-moving clock relative to the bucket's windowStartMs. If nowMs passed to a later call is ever less than windowStartMs from a previous call — a real possibility for a Date.now()-derived clock reading across an NTP correction or a container/VM clock reset — windowElapsed evaluates false, effectiveWindowStart stays at the old windowStartMs, and resetAtMs - now grows by however far the clock stepped backward, on top of the normal windowMs. A caller blocked at the rate limit right after a backward clock jump gets a retryAfterMs that can be many times the configured window — e.g. a 60-second window reporting several minutes of backoff after a half-second backward step, worse the larger the jump.

The module's own doc comment already states its normalization goal precisely: "so a non-finite, fractional, or negative count/limit/window can never produce a NaN or negative decision" — but it stops at NaN/negative; it does not claim (or enforce) that retryAfterMs is bounded by windowMs, which a reasonable caller would assume given the "rolling window" framing.

There is no dedicated test file for rate-limit.ts itself — it's only exercised indirectly via write-rate-limit-enforcement.test.ts's shallow composition tests, none of which vary nowMs backward relative to a bucket's windowStartMs.

Requirements

  • Clamp retryAfterMs to at most windowMs (the configured window length) regardless of how far nowMs has moved backward relative to windowStartMs. A backward clock step should never produce a wait longer than one full configured window.
  • Preserve all existing forward-clock behavior exactly — this is a narrowing fix (bound the output), not a change to the allowed/blocked decision logic itself.
  • Add a dedicated test file for evaluateLocalRateLimit if one doesn't already exist at packages/loopover-engine/test/rate-limit.test.ts, covering: a normal forward-elapsing window, a blocked call within the window, and a backward clock step (nowMs < bucket.windowStartMs) verifying retryAfterMs <= windowMs.

Deliverables

  • evaluateLocalRateLimit in rate-limit.ts clamps retryAfterMs to [0, windowMs].
  • A new or extended test file covering the backward-clock-skew case with an explicit assertion that retryAfterMs never exceeds windowMs.
  • Updated doc comment on evaluateLocalRateLimit stating the bounded-output guarantee explicitly (matching the existing "never NaN or negative" comment style).

Test Coverage Requirements

packages/loopover-engine/src/** is measured by Codecov; target 99%+ patch coverage on every changed line and branch, including the new clamp's active and inactive arms (skew present vs. absent) and the regression test described above.

Expected Outcome

evaluateLocalRateLimit's retryAfterMs is always bounded by the configured windowMs, even when the injected clock reading moves backward relative to a bucket's recorded window start — matching the "rolling window" contract callers already rely on and extending the module's existing "output can never be NaN/negative" normalization discipline to cover this bound too.

Links & Resources

  • packages/loopover-engine/src/governor/rate-limit.ts (evaluateLocalRateLimit, finiteNonNegativeInt)
  • packages/loopover-engine/test/write-rate-limit-enforcement.test.ts (existing indirect coverage)

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions