fix(core): validate grep result limits (#5389)

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

docs/developers/tools/file-system.md

@@ -155,7 +155,7 @@ notebook_edit(
   - `pattern` (string, required): The regular expression pattern to search for in file contents (e.g., `"function\\s+myFunction"`, `"log.*Error"`).
   - `path` (string, optional): File or directory to search in. Defaults to current working directory.
   - `glob` (string, optional): Glob pattern to filter files (e.g. `"*.js"`, `"src/**/*.{ts,tsx}"`).
-  - `limit` (number, optional): Limit output to first N matching lines. Optional - shows all matches if not specified.
+  - `limit` (integer, optional): Limit output to first N matching lines. Must be a positive integer. Optional - shows all matches if not specified.
 - **Behavior:**
   - Uses ripgrep for fast search when available; otherwise falls back to a JavaScript-based search implementation.
   - Returns matching lines with file paths and line numbers.

packages/core/src/tools/grep.test.ts

@@ -154,6 +154,20 @@ describe('GrepTool', () => {
       expect(grepTool.validateToolParams(params)).toBeNull();
     });
 
+    it('should return null for a positive integer limit', () => {
+      const params: GrepToolParams = { pattern: 'hello', limit: 2 };
+      expect(grepTool.validateToolParams(params)).toBeNull();
+    });
+
+    it.each([
+      [0, 'params/limit must be >= 1'],
+      [-1, 'params/limit must be >= 1'],
+      [1.5, 'params/limit must be integer'],
+    ])('should return error for invalid limit %s', (limit, expectedError) => {
+      const params: GrepToolParams = { pattern: 'hello', limit };
+      expect(grepTool.validateToolParams(params)).toBe(expectedError);
+    });
+
     it('should return error if pattern is missing', () => {
       const params = { path: '.' } as unknown as GrepToolParams;
       expect(grepTool.validateToolParams(params)).toBe(
@@ -658,8 +672,7 @@ describe('GrepTool', () => {
       expect(result.returnDisplay).toBe('Found 30 matches');
     });
 
-    it('should not validate limit parameter', () => {
-      // limit parameter has no validation constraints in the new implementation
+    it('should validate a positive limit parameter', () => {
       const params = { pattern: 'test', limit: 5 };
       const error = grepTool.validateToolParams(params as GrepToolParams);
       expect(error).toBeNull();

packages/core/src/tools/grep.ts

@@ -629,9 +629,10 @@ export class GrepTool extends BaseDeclarativeTool<GrepToolParams, ToolResult> {
               'File or directory to search in. Defaults to current working directory.',
           },
           limit: {
-            type: 'number',
+            type: 'integer',
+            minimum: 1,
             description:
-              'Limit output to first N matching lines. Optional - shows all matches if not specified.',
+              'Limit output to first N matching lines. Must be a positive integer. Optional - shows all matches if not specified.',
           },
         },
         required: ['pattern'],
@@ -648,6 +649,13 @@ export class GrepTool extends BaseDeclarativeTool<GrepToolParams, ToolResult> {
   protected override validateToolParamValues(
     params: GrepToolParams,
   ): string | null {
+    if (
+      params.limit !== undefined &&
+      (!Number.isInteger(params.limit) || params.limit <= 0)
+    ) {
+      return 'limit must be a positive integer';
+    }
+
     // Validate pattern is a valid regex
     try {
       new RegExp(params.pattern);

packages/core/src/tools/ripGrep.test.ts

@@ -118,6 +118,20 @@ describe('RipGrepTool', () => {
       expect(grepTool.validateToolParams(params)).toBeNull();
     });
 
+    it('should return null for a positive integer limit', () => {
+      const params: RipGrepToolParams = { pattern: 'hello', limit: 2 };
+      expect(grepTool.validateToolParams(params)).toBeNull();
+    });
+
+    it.each([
+      [0, 'params/limit must be >= 1'],
+      [-1, 'params/limit must be >= 1'],
+      [1.5, 'params/limit must be integer'],
+    ])('should return error for invalid limit %s', (limit, expectedError) => {
+      const params: RipGrepToolParams = { pattern: 'hello', limit };
+      expect(grepTool.validateToolParams(params)).toBe(expectedError);
+    });
+
     it('should return error if pattern is missing', () => {
       const params = { path: '.' } as unknown as RipGrepToolParams;
       expect(grepTool.validateToolParams(params)).toBe(

packages/core/src/tools/ripGrep.ts

@@ -528,9 +528,10 @@ export class RipGrepTool extends BaseDeclarativeTool<
               'File or directory to search in (rg PATH). Defaults to current working directory.',
           },
           limit: {
-            type: 'number',
+            type: 'integer',
+            minimum: 1,
             description:
-              'Limit output to first N lines/entries. Optional - shows all matches if not specified.',
+              'Limit output to first N lines/entries. Must be a positive integer. Optional - shows all matches if not specified.',
           },
         },
         required: ['pattern'],
@@ -555,6 +556,13 @@ export class RipGrepTool extends BaseDeclarativeTool<
       return errors;
     }
 
+    if (
+      params.limit !== undefined &&
+      (!Number.isInteger(params.limit) || params.limit <= 0)
+    ) {
+      return 'limit must be a positive integer';
+    }
+
     // Validate pattern is a valid regex
     try {
       new RegExp(params.pattern);