fix(client): require XADD TRIM strategy to match the command grammar (#3418)

* fix(client): require XADD TRIM strategy to match the command grammar

* fix(client): always emit XADD TRIM strategy and align remaining specs

Author: abhijeet117

packages/client/lib/commands/XADD.spec.ts

@@ -31,10 +31,11 @@ describe('XADD', () => {
           field: 'value'
         }, {
           TRIM: {
-            threshold: 1000
+            strategy: 'MINID',
+            threshold: 5
           }
         }),
-        ['XADD', 'key', '1000', '*', 'field', 'value']
+        ['XADD', 'key', 'MINID', '5', '*', 'field', 'value']
       );
     });
 
@@ -58,11 +59,12 @@ describe('XADD', () => {
           field: 'value'
         }, {
           TRIM: {
+            strategy: 'MAXLEN',
             strategyModifier: '=',
             threshold: 1000
           }
         }),
-        ['XADD', 'key', '=', '1000', '*', 'field', 'value']
+        ['XADD', 'key', 'MAXLEN', '=', '1000', '*', 'field', 'value']
       );
     });
 
@@ -72,11 +74,12 @@ describe('XADD', () => {
           field: 'value'
         }, {
           TRIM: {
+            strategy: 'MAXLEN',
             threshold: 1000,
             limit: 1
           }
         }),
-        ['XADD', 'key', '1000', 'LIMIT', '1', '*', 'field', 'value']
+        ['XADD', 'key', 'MAXLEN', '1000', 'LIMIT', '1', '*', 'field', 'value']
       );
     });
 
@@ -86,11 +89,12 @@ describe('XADD', () => {
           field: 'value'
         }, {
           TRIM: {
+            strategy: 'MAXLEN',
             threshold: 1000,
             limit: 0
           }
         }),
-        ['XADD', 'key', '1000', 'LIMIT', '0', '*', 'field', 'value']
+        ['XADD', 'key', 'MAXLEN', '1000', 'LIMIT', '0', '*', 'field', 'value']
       );
     });
 
@@ -100,11 +104,12 @@ describe('XADD', () => {
           field: 'value'
         }, {
           TRIM: {
+            strategy: 'MAXLEN',
             threshold: 1000,
             policy: STREAM_DELETION_POLICY.DELREF
           }
         }),
-        ['XADD', 'key', '1000', 'DELREF', '*', 'field', 'value']
+        ['XADD', 'key', 'MAXLEN', '1000', 'DELREF', '*', 'field', 'value']
       );
     });
 

packages/client/lib/commands/XADD.ts

@@ -32,7 +32,11 @@ export interface XAddOptions {
     iid: RedisArgument;
   };
   TRIM?: {
-    strategy?: 'MAXLEN' | 'MINID';
+    /**
+     * Required by the XADD grammar: without MAXLEN or MINID the server parses
+     * the threshold as the entry ID instead of trimming.
+     */
+    strategy: 'MAXLEN' | 'MINID';
     strategyModifier?: '=' | '~';
     threshold: number;
     limit?: number;
@@ -79,9 +83,7 @@ export function parseXAddArguments(
 
   // Trimming options
   if (options?.TRIM) {
-    if (options.TRIM.strategy) {
-      parser.push(options.TRIM.strategy);
-    }
+    parser.push(options.TRIM.strategy);
 
     if (options.TRIM.strategyModifier) {
       parser.push(options.TRIM.strategyModifier);

packages/client/lib/commands/XADD_NOMKSTREAM.spec.ts

@@ -33,10 +33,11 @@ describe('XADD NOMKSTREAM', () => {
           field: 'value'
         }, {
           TRIM: {
-            threshold: 1000
+            strategy: 'MINID',
+            threshold: 5
           }
         }),
-        ['XADD', 'key', 'NOMKSTREAM', '1000', '*', 'field', 'value']
+        ['XADD', 'key', 'NOMKSTREAM', 'MINID', '5', '*', 'field', 'value']
       );
     });
 
@@ -60,11 +61,12 @@ describe('XADD NOMKSTREAM', () => {
           field: 'value'
         }, {
           TRIM: {
+            strategy: 'MAXLEN',
             strategyModifier: '=',
             threshold: 1000
           }
         }),
-        ['XADD', 'key', 'NOMKSTREAM', '=', '1000', '*', 'field', 'value']
+        ['XADD', 'key', 'NOMKSTREAM', 'MAXLEN', '=', '1000', '*', 'field', 'value']
       );
     });
 
@@ -74,11 +76,12 @@ describe('XADD NOMKSTREAM', () => {
           field: 'value'
         }, {
           TRIM: {
+            strategy: 'MAXLEN',
             threshold: 1000,
             limit: 1
           }
         }),
-        ['XADD', 'key', 'NOMKSTREAM', '1000', 'LIMIT', '1', '*', 'field', 'value']
+        ['XADD', 'key', 'NOMKSTREAM', 'MAXLEN', '1000', 'LIMIT', '1', '*', 'field', 'value']
       );
     });
 
@@ -88,11 +91,12 @@ describe('XADD NOMKSTREAM', () => {
           field: 'value'
         }, {
           TRIM: {
+            strategy: 'MAXLEN',
             threshold: 1000,
             policy: STREAM_DELETION_POLICY.DELREF
           }
         }),
-        ['XADD', 'key', 'NOMKSTREAM', '1000', 'DELREF', '*', 'field', 'value']
+        ['XADD', 'key', 'NOMKSTREAM', 'MAXLEN', '1000', 'DELREF', '*', 'field', 'value']
       );
     });
 

packages/client/types-tests/xadd-trim.types-test.ts

@@ -0,0 +1,16 @@
+/**
+ * Compile-time regression: XADD's TRIM.strategy must be required. The official
+ * grammar makes MAXLEN|MINID mandatory inside the trim block; with the strategy
+ * omitted, parseXAddArguments emitted a bare threshold which the server parses
+ * as the entry ID instead of a trim instruction.
+ *
+ * Lives outside `lib/` so it is not picked up by the production build /
+ * typedoc. Checked with `npm run test:types -w @redis/client`.
+ */
+import { XAddOptions } from '../lib/commands/XADD';
+
+export function xaddTrimRequiresStrategy(): void {
+    // @ts-expect-error TRIM.strategy is required alongside threshold
+    const invalidTrim: NonNullable<XAddOptions['TRIM']> = { threshold: 1000 };
+    console.log(invalidTrim);
+}