fix(caching): make TTL test deterministic, fix intermittent CI failure

[RealLumenHere]

Fixes #15.

Root cause

cache.test.ts › Basic Redis Operations › should handle TTL correctly races the wall clock. MockRedisClient.ttl() computes:

const remaining = Math.max(0, ttl - Date.now());
return Math.floor(remaining / 1000);

With a 1000ms TTL, remaining starts at 1000 and only ever counts down — any real time elapsing between cache.set() and the following cache.get() (routine under this repo's maxWorkers: 1 jest config, worse under CI contention) pushes remaining below 1000ms, floor rounds down to 0, and the falsy check in RedisCache.get() (resultArr[1] && Number(resultArr[1]) > 0) then leaves result.ttl undefined. That's the exact "received value must be a number or bigint" failure from #15 — hit is still true because the value itself hasn't expired, only the metadata rounds away.

Fix

Freeze the clock for the duration of the test:

jest.useFakeTimers({ now: Date.now() });
try {
  await cache.set("key1", "value1", CacheStrategy.TTL_REDIS, ttl);
  const result = await cache.get("key1", CacheStrategy.TTL_REDIS);
  expect(result.hit).toBe(true);
  expect(result.ttl).toBeGreaterThan(0);
} finally {
  jest.useRealTimers();
}

set() and get() now observe the identical timestamp, so remaining is always the full 1000ms and floors to a deterministic 1. This fixes the root cause (an uncontrolled clock) rather than loosening the assertion or padding the TTL.

Verification

• Reproduced the flake locally against the unmodified test: 15/60 failures (25%) in a tight loop, --maxWorkers=1, single machine — confirms it's the described race, not CI-environment noise.
• With the fix: 60/60 clean in the same loop/config.
• npm run ci (lint + tsc --noEmit + build + full test suite) green on Node 22 — 944 tests passed, 7 pre-existing skips, no regressions.
• Isolated re-run of src/api/caching/src/cache.test.ts green on Node 18 and Node 20 (Docker node:18/node:20 images), matching the CI matrix.
• Node 18 install emits pre-existing EBADENGINE warnings from fastify's own transitive deps (want Node ≥20) — unrelated to this change, tests still pass; flagging since package.json declares engines.node >= 18.0.0 but the workflow matrix only actually tests 20/22.

Single-file change, no production code touched.

[SkinnnyJay]

Reviewed the diff, the mock, and the surrounding suite. The diagnosis in the description is correct and I verified it against the code on main:

MockRedisClient.ttl() (cache.test.ts:79-84) does Math.floor(Math.max(0, expiry - Date.now()) / 1000). With ttl = 1000, that returns 1 only while zero wall-clock milliseconds have elapsed — one tick later it floors to 0, RedisCache.get()'s resultArr[1] && Number(resultArr[1]) > 0 check goes falsy, and result.ttl is undefined. That is exactly the received value must be a number or bigint in #15, and it explains why hit was still true. Confirmed.

The fix is sound: jest.useFakeTimers({ now: Date.now() }) in a try/finally freezes Date.now() across set()/get(), so remaining is a full 1000 and floors to 1. Modern fake timers don't intercept microtasks, so the awaits still resolve normally, and the finally restores real timers even if an assertion throws — restoreMocks: true in jest.config.js doesn't cover timers, so that finally is doing real work. No production code touched, one test, reversible.

Three notes, none of them blocking.

1. The mock is what's actually unfaithful, and this doesn't fix it.
Real Redis TTL rounds to nearest ((ttl+500)/1000), so a live key with a 1000 ms expiry returns 1 for the first half-second. The mock floors, so it returns 0 after 1 ms. The mock is strictly more fragile than the thing it's standing in for. A one-line Math.round(remaining / 1000) at cache.test.ts:84 would fix the whole class at the source and needs no frozen clock. This is the only Redis-mock TTL assertion in the file today (line 426 — 336 and 526 are the memory cache), so the blast radius is genuinely one test, but the mock also feeds ttl into the LRU and LFU eval branches (lines 139, 153); nothing asserts on it there yet. Worth a follow-up either way.

2. #3 supersedes this.
Open PR #3 rewrites these same Lua scripts from SETEX math.floor(ttl/1000) / TTL to SET ... PX / PTTL, and rewrites the mock to a pttlMs() helper returning milliseconds. Once that lands, a 1000 ms TTL reads back as ~1000 with a full second of headroom and this race stops existing — the useFakeTimers wrapper here becomes vestigial. The two don't conflict textually (#3's hunks are at lines 1, 43, 93-197, 226, 842; this is at 419-430), so merge order doesn't matter mechanically. I'd still take this now: #3 has been open since July 4, is nine files, and is unreviewed, while the flake is degrading CI signal on every open PR today. Cheap to land, cheap to delete later.

3. jest is used as an ambient global here.
The file imports its test globals explicitly (import { describe, it, expect, beforeEach, afterEach } from "@jest/globals") but this change reaches for a bare jest. It works — jest.config.js doesn't set injectGlobals: false — and it's the choice that avoids a conflict with #3, which adds jest to that same import line. Flagging it as a deliberate inconsistency rather than asking you to change it.

The actual blocker isn't the code. The CI run on this branch is action_required and has been since Aug 30 — zero check runs on 218d344, mergeStateStatus: UNSTABLE. It's sitting under the first-time-contributor approval gate, which is the same thing #15 says swallowed the runs on the other 13 PRs. Approve the workflow run, let the Node 18/20/22 matrix report, and merge.

Recommendation: approve the run, merge, and open a follow-up for the Math.floor → Math.round mock fidelity fix (or close that follow-up out when #3 lands). Closing Fixes #15 on merge is correct.

[SkinnnyJay]

Merged — thanks, and sorry this sat six days over a button rather than anything in the code.

For the record on what was actually blocking it: the run had been sitting at action_required since Aug 30 under the first-time-contributor policy, so zero checks had ever executed on 218d344. I approved it this morning; test (20) and test (22) both passed (5m20s / 4m55s) and it went in as bf80952. #15 auto-closed.

Your diagnosis held up on inspection — Math.floor(remaining / 1000) returning 1 only at zero elapsed milliseconds is exactly right, and the try/finally matters more than it looks since restoreMocks: true in jest.config.js doesn't restore timers.

Two follow-ups so nothing here gets lost:

• Filed MockRedisClient.ttl() floors where real Redis rounds, making sub-2s TTL assertions racy #17 for the root cause you worked around. The mock floors where real Redis rounds to nearest, which is why that test could race at all — a one-line Math.round fixes the class rather than the instance.
• MockRedisClient.ttl() floors where real Redis rounds, making sub-2s TTL assertions racy #17 also tracks removing the useFakeTimers wrapper you added here, because fix(caching): O(n^2) eviction, sub-second TTL crash, LFU frequency reset #3 supersedes both. Its SETEX/TTL → SET PX/PTTL change makes the race impossible and the frozen clock unnecessary. Heads up that fix(caching): O(n^2) eviction, sub-second TTL crash, LFU frequency reset #3 is now blocked on something unrelated I found while reviewing it — a concurrency hang in the LFU rewrite — so it may be a while.

Good, well-evidenced fix. The 15/60 → 60/60 reproduction in the description is what made this easy to accept.