fix: address code review - recursive isFilterAST validation, type safety, guard against array entries

Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>

Author: Copilot

ROADMAP.md

@@ -110,6 +110,10 @@ This strategy ensures rapid iteration while maintaining a clear path to producti
 | `defineStack()` strict by default | ✅ | `strict: true` default since v3.0.2, validates schemas + cross-references |
 | `z.any()` elimination in UI protocol | ✅ | All `filter` fields → `FilterConditionSchema` or `ViewFilterRuleSchema`, all `value` fields → typed unions |
 | Filter format unification | ✅ | MongoDB-style filters use `FilterConditionSchema`, declarative view/tab filters use `ViewFilterRuleSchema` — `z.array(z.unknown())` eliminated |
+| Filter parameter naming (`filter` vs `filters`) | ✅ | Canonical HTTP param: `filter` (singular). `filters` accepted for backward compat. `HttpFindQueryParamsSchema` added. Client SDK + protocol.ts unified. |
+| `isFilterAST()` structural validation | ✅ | Exported from `data/filter.zod.ts` — validates AST shape (comparison/logical/legacy) instead of naïve `Array.isArray`. `VALID_AST_OPERATORS` constant. |
+| GET by ID parameter pollution prevention | ✅ | `GetDataRequestSchema` allowlists `select`/`expand`. Dispatcher whitelists only safe params. |
+| Dispatcher response field mapping | ✅ | `handleApiEndpoint` uses spec-correct `result.records`/`result.total` instead of `result.data`/`result.count` |
 | Seed data → object cross-reference | ✅ | `validateCrossReferences` detects seed data referencing undefined objects |
 | Navigation → object/dashboard/page/report cross-reference | ✅ | App navigation items validated against defined metadata (recursive group support) |
 | Negative validation tests (dashboard, page, report, view) | ✅ | Missing required fields, invalid enums, type violations, cross-reference errors all covered |

packages/client/src/index.ts

@@ -1455,10 +1455,12 @@ export class ObjectStackClient {
              // Detect AST filter format vs simple key-value map. AST filters use an array structure
              // with [field, operator, value] or [logicOp, ...nodes] shape (see isFilterAST from spec).
              // For complex filter expressions, use .query() which builds a proper QueryAST.
-             if (this.isFilterAST(filterValue)) {
+             if (this.isFilterAST(filterValue) || Array.isArray(filterValue)) {
+                 // AST or any array → serialize as JSON in `filter` param
                  queryParams.set('filter', JSON.stringify(filterValue));
-             } else {
-                 Object.entries(filterValue).forEach(([k, v]) => {
+             } else if (typeof filterValue === 'object' && filterValue !== null) {
+                 // Plain key-value map → append each as individual query params
+                 Object.entries(filterValue as Record<string, unknown>).forEach(([k, v]) => {
                      if (v !== undefined && v !== null) {
                         queryParams.append(k, String(v));
                      }

packages/runtime/src/http-dispatcher.ts

@@ -380,7 +380,7 @@ export class HttpDispatcher {
                 // Spec: Only select/expand are allowlisted query params for GET by ID.
                 // All other query parameters are discarded to prevent parameter pollution.
                 const { select, expand } = query || {};
-                const allowedParams: Record<string, any> = {};
+                const allowedParams: Record<string, unknown> = {};
                 if (select != null) allowedParams.select = select;
                 if (expand != null) allowedParams.expand = expand;
                 // Spec: broker returns GetDataResponse = { object, id, record }

packages/spec/src/data/filter.test.ts

@@ -984,6 +984,22 @@ describe('isFilterAST', () => {
     expect(isFilterAST(['and', 'not-an-array'])).toBe(false);
     expect(isFilterAST(['or', 123])).toBe(false);
   });
+
+  it('should recursively validate children of logical nodes', () => {
+    // Valid: children are valid AST nodes
+    expect(isFilterAST(['and', ['status', '=', 'active'], ['age', '>', 18]])).toBe(true);
+    // Invalid: children are arrays but not valid AST nodes
+    expect(isFilterAST(['and', [1, 2, 3]])).toBe(false);
+    // Nested valid AST
+    expect(isFilterAST(['and', ['or', ['a', '=', 1], ['b', '=', 2]], ['c', '>', 3]])).toBe(true);
+  });
+
+  it('should recursively validate legacy flat array children', () => {
+    // Valid: all children are valid comparison nodes
+    expect(isFilterAST([['status', '=', 'active'], ['age', '>', 18]])).toBe(true);
+    // Invalid: children are arbitrary arrays
+    expect(isFilterAST([[1, 2], [3, 4]])).toBe(false);
+  });
 });
 
 // ============================================================================

packages/spec/src/data/filter.zod.ts

@@ -381,7 +381,7 @@ export function isFilterAST(filter: unknown): boolean {
   if (typeof first === 'string') {
     const lower = first.toLowerCase();
     if (lower === 'and' || lower === 'or') {
-      return filter.length >= 2 && filter.slice(1).every((child: unknown) => Array.isArray(child));
+      return filter.length >= 2 && filter.slice(1).every((child: unknown) => isFilterAST(child));
     }
 
     // Comparison node: [field, operator, value]
@@ -391,7 +391,7 @@ export function isFilterAST(filter: unknown): boolean {
   }
 
   // Legacy flat array: [[cond], [cond], ...]
-  if (filter.every((item: unknown) => Array.isArray(item))) {
+  if (filter.every((item: unknown) => isFilterAST(item))) {
     return filter.length > 0;
   }