Round the Redis key–value store TTL up to whole seconds - #1034

Summary

RedisKvStore.set() passed options.ttl.total("second") straight to Redis
SETEX, which takes only whole seconds. Any Temporal.Duration that was not
an exact number of seconds was rejected by the server, so the write failed
instead of storing the value with an approximated expiry. The TTL is now
rounded up to the next whole second, with a floor of one second.

The one-second granularity is SETEX's, not Redis's — SET with PX expresses
millisecond expiries — so the rounding is a property of the command this
adapter uses rather than of the server.

Targets 2.0-maintenance rather than main, per @dahlia's direction on the
issue: "Confirmed that this reproduces in 2.0.x. Since this is a bug fix,
please target 2.0-maintenance rather than main." (comment) The issue
was filed against main; the reproduction below was re-run on
2.0-maintenance rather than assumed.

This is a fork PR, so the workflows land as action_required — they need a
maintainer to approve the run before CI can report.

Related issue

• fixes RedisKvStore.set() rejects any TTL that is not a whole number of seconds #1028

Reproduction

Against Redis 7.4.11 (redis:7-alpine in Docker) on 2.0-maintenance at
9c8ec49, writing the same value with each TTL and reading back PTTL:

A whole number of seconds is unchanged, so nothing that worked before behaves
differently.

Why round up rather than to the nearest second

Rounding to the nearest second would shorten an expiry by up to half a second.
Rounding up never shortens one, and three things point the same way:

Every other KvStore implementation keeps a value for at least as long as it
was asked to.

Redis is the only backend here that has to approximate, and the Workers adapter
already establishes that when a backend cannot express the requested lifetime,
this codebase extends it rather than cutting it short. Rounding down would
make Redis the only adapter that can drop a value before its TTL.

The costs are asymmetric. Expiring late costs at most one extra second of
staleness on a cache. Expiring early can change behaviour: a TTL used to
suppress duplicate work starts letting duplicates through. Fedify does exactly
that — the inbox listener and the queue handler both mark an activity as
processed with kv.set(cacheKey, true, { ttl: 1 day }).

It matches the issue's expected behaviour. "A sub-second TTL is expected to
be normalized to the smallest expiry Redis can express" is what rounding up
produces for the sub-second cases.

In practice nothing in the library on this branch is affected either way, since
every TTL Fedify itself passes is already a whole number of seconds. The
reachable path is application code, which KvStoreSetOptions.ttl opens to any
Temporal.Duration, and which #1027 made easier to hit on newer branches by
letting an application choose publicKeyTtl and
httpMessageSignaturesSpecTtl directly.

Changes

• packages/redis/src/kv.ts: add expirySeconds(), which returns
  Math.max(1, Math.ceil(ttl.total("second"))), and call it from set().
  The Math.max matters only when Math.ceil yields zero — a zero or
  negative duration — and it documents why the rounding goes up.
• packages/redis/src/kv.test.ts: two regression tests, described below.
• changes.d/redis/ttl-whole-seconds.md and CHANGES.md.

RedisKvStore has no cas() method, so set() is the only call site.

Verification

The regression tests fail without the patch. Reverting set() to pass the
raw total fails both of them; the end-to-end one reports the error from the
issue verbatim:

ReplyError: ERR value is not an integer or out of range

Each half of the expression is pinned by its own assertion.

The Math.round mutation is the one that matters for review: it still produces
a whole positive number for every case in the issue's table, and only the 1.4-
second case separates it from Math.ceil. The last row is why the negative
cases are not redundant with the zero one — an implementation that clamped zero
to one second and let negatives through would have passed the table as it
originally stood, and reached SETEX with an expiry it rejects.

One test needs no Redis server. RedisKvStore.set() rounds a TTL up to whole seconds passes the store a stand-in client that records what set()
hands to SETEX, and checks the exact integer for 1s, 5min, 1.5s, 1.4s, 500ms,
1ms, 0, −1ms, −30s and −1h, asserting each result is a whole positive number.
RedisKvStore.set() stores a sub-second TTL is REDIS_URL-gated and covers the
end-to-end behaviour, including a −30s TTL and that a 30-second TTL still reads
back as 30.

Both new tests use node:test directly rather than @fedify/fixture, because
a test registered through the fixture does not run under Node.js in this
package. The fixture's Node.js branch calls require("node:test") inside a
try with an empty catch; require is not defined in an ES module, so
registration fails silently wherever the Node.js run loads ES modules — which
is the case here, since @fedify/redis runs node --experimental-transform- types --test over the ES-module sources. A test that must fail reports
success:

node: ok 1 - src/probe.test.ts     # pass 1, # fail 0    <- never executed
bun : (fail) PROBE: this must fail   0 pass
deno: PROBE: this must fail ... FAILED

Written with the fixture, the server-free test above — the one whose whole
point is to run where there is no Redis — would have reported a pass on
Node.js without executing. packages/postgres/src/kv.test.ts already imports
test from node:test directly, so this follows existing practice in the
repository.

The nine pre-existing @fedify/redis fixture tests are affected by the same
gap, and so is most of the monorepo: of the 229 fixture-registered tests across
40 files, only the one in @fedify/webfinger executes on Node.js, because that
package ships a .cjs test build alongside the ESM one and require works
there. @fedify/fedify's own Node.js run reports 28 passing tests that are
all bare .mjs file entries. None of that is this fix's to repair — it is
noted here only to explain the import choice, and is worth a separate issue.

Branch and changelog. Rebased onto the current 2.0-maintenance
(d863a12). 2.0.27 was released in the meantime, so the entry now sits under
2.0.28 and the fragment was regenerated with sacho sync; sacho check passes.

Tests. @fedify/redis: Deno 14 passed / 0 failed, Bun 14 passed / 0
failed, Node.js 7 passed / 0 failed. The Node.js number is lower for the
reason just described, not because anything was skipped there. mise run check passes, sacho check included.

AI use

Claude Opus 5 assisted with the patch, the tests, and running the checks above;
the commit carries an Assisted-by trailer. Every number in this description
was measured against a real Redis instance rather than inferred from reading
the code.

Checklist

• Did you add a changelog entry to the CHANGES.md?
• Did you write some relevant docs about this change (if it's a new
  feature)? — not a new feature; the rounding is documented on
  expirySeconds().
• Did you write a regression test to reproduce the bug (if it's a bug
  fix)?
• Did you write some tests for this change (if it's a new feature)?
• Did you run mise test on your machine? — yes, for @fedify/redis on
  all three runtimes; see Verification.

Additional notes

• A zero or negative TTL now stores the value for one second. Both used
  to fail with ERR invalid expire time in 'setex' command, so this is
  strictly better than the current behaviour, but it is not what the other
  adapters do: they read a non-positive TTL as already expired. This is a
  policy choice rather than a consequence of the rounding, and it is now
  documented as one on set() and on the helper. An alternative would be to
  delete the key instead of storing it for one second, which matches how the
  other adapters read a non-positive TTL. I kept the one-second floor because
  it is the smallest expiry SETEX can express and because deleting on
  set() is a larger behavioural change than this fix needs; happy to switch
  if you prefer the other reading.
• Reframed: the one-second granularity is SETEX's, not Redis's. If you
  would prefer exact sub-second expiry, switching set() to SET … PX would
  remove the rounding — and with it the zero and negative edge cases —
  entirely. I kept SETEX here to stay minimal on a maintenance branch and
  because RedisKvStore.set() rejects any TTL that is not a whole number of seconds #1028 asked for normalisation rather than a new expiry path, and
  you have said keeping it is fine; noting the option so the choice is on the
  record rather than implied.
• A TTL can now be up to one second longer than requested. For the cache
  lifetimes a KvStore holds, that is an extra second of staleness at worst.
• This does not change any existing write. Every TTL that worked before —
  every whole number of seconds, which is all Fedify itself uses — produces
  the same SETEX argument as before.