Skip to content

fix(client): XADD/XTRIM with LIMIT 0 must emit the argument#3342

Merged
nkaradzhov merged 1 commit into
redis:masterfrom
Develop-KIM:fix/xadd-xtrim-limit-0
Jul 16, 2026
Merged

fix(client): XADD/XTRIM with LIMIT 0 must emit the argument#3342
nkaradzhov merged 1 commit into
redis:masterfrom
Develop-KIM:fix/xadd-xtrim-limit-0

Conversation

@Develop-KIM

@Develop-KIM Develop-KIM commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Description

XTRIM ... LIMIT and XADD ... TRIM.limit are guarded by a truthy check, so passing an explicit 0 drops the argument entirely:

await client.xTrim('key', 'MAXLEN', 1, { strategyModifier: '~', LIMIT: 0 });
// sends: XTRIM key MAXLEN ~ 1        <- LIMIT 0 is gone

That matters because LIMIT 0 and omitting LIMIT are not the same thing. LIMIT 0 means unlimited trimming, while omitting it caps eviction at 100 * stream-node-max-entries (10000 with the defaults). So today a caller who explicitly asks for unlimited trimming silently gets the capped default instead.

I checked this against Redis 8.8 with a 20000-entry stream, trimming with MAXLEN ~ 1:

call command sent entries evicted XLEN after
{ LIMIT: 0 } (before) XTRIM key MAXLEN ~ 1 10000 10000
{ LIMIT: 0 } (after) XTRIM key MAXLEN ~ 1 LIMIT 0 19900 100

The fix is to guard on !== undefined so 0 is forwarded, same as the recent XGROUP ... ENTRIESREAD 0 fix in #3333. Both XTRIM and XADD's TRIM.limit are affected, so I fixed both and added a LIMIT 0 argument test for each.


Checklist

  • Does npm test pass with this change (including linting)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?

One caveat on the first box: I don't have Docker on this machine, so I couldn't run the full suite locally — the integration tests spin up containers. I ran the transformArguments assertions directly and verified the end-to-end behaviour against a local Redis 8.8 server (the table above). Happy to adjust if CI turns up anything.

No docs change — this restores the documented behaviour rather than changing the API.


Note

Low Risk
Small, localized fix to command serialization with matching unit tests; behavior change only when callers pass 0, aligning with documented Redis semantics.

Overview
XADD (TRIM.limit) and XTRIM (LIMIT) now include the LIMIT clause when the value is 0, by checking !== undefined instead of a truthy guard.

Previously, LIMIT: 0 was dropped from the wire protocol, so Redis treated the call as “no LIMIT” (default eviction cap) rather than unlimited trimming per LIMIT 0. Unit tests assert the serialized argv for limit/LIMIT of 0.

Reviewed by Cursor Bugbot for commit 63fe547. Bugbot is set up for automated code reviews on this repo. Configure here.

`XADD ... TRIM.limit` and `XTRIM ... LIMIT` were guarded by a truthy check
(`if (options.TRIM.limit)` / `if (options?.LIMIT)`), so an explicit `0` was
silently dropped and the server fell back to its implicit default limit.

Per the Redis docs, `LIMIT 0` means unlimited trimming, whereas omitting
LIMIT caps eviction at `100 * stream-node-max-entries` (10000 by default),
so the two are observably different. Against Redis 8.8, trimming a
20000-entry stream with `MAXLEN ~ 1` evicts 10000 entries without LIMIT but
19900 with `LIMIT 0` — meaning a caller asking for unlimited trimming
currently gets the capped default instead.

Guard on `!== undefined` so `0` is forwarded, matching the recent XGROUP
ENTRIESREAD 0 fix (redis#3333). Adds a `LIMIT 0` argument test for both commands.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nkaradzhov
nkaradzhov merged commit 1487657 into redis:master Jul 16, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants