fix(table): close the review findings on the dual-grammar boundaries

From the PR #6067 review (greptile + bugbot), all verified before fixing:

1. Hybrid nodes on the async destructive routes (P1/High). The dual union's
   legacy branch accepts any non-empty object WITHOUT stripping, so a node
   carrying both a group key and leaf keys reached toLegacyFilter Zod-approved
   — and predicateToFilter converted it group-first, silently DROPPING the leaf
   and widening a select-all delete. predicateToFilter now throws on hybrids
   (lossless-or-throw, like its other rules), toLegacyFilter shape-validates
   first, and the GET native-predicate path shape-validates too.

2. A column literally named all/any (P1). NAME_PATTERN allows it, and
   isTablePredicate routed any object with those keys to the predicate
   compiler. It now requires the group value to be an ARRAY: the legacy
   equality shorthand and operator objects on such a column keep compiling as
   legacy, and an array-valued legacy condition was always a dropped no-op, so
   predicate precedence on arrays regresses nothing.

3. Bulk keying (P2). resolveBulkFilter validated predicates as NAME-keyed and
   translated unconditionally — wrong for the ID-keyed grid (session wire is
   identity). Validation now runs AFTER wire translation against STORAGE keys
   (new validateStoragePredicate), which is keying-correct for every caller and
   keeps the property that a typo'd column on a destructive path is a 400, not
   a silent match-nothing no-op.

4. 500s on downgrade rejection (Medium). delete-async called toLegacyFilter
   outside its try, and cancel-runs/columns-run mapped the throw to the generic
   500. All three now return the validation message as a 400.

5. SortSpec broke the wire (High). requestJson threw on arrays of objects, so
   any active grid sort died client-side before the request — and the server
   contract rejected string-encoded values anyway, on both grammars. Arrays
   containing objects now travel as one JSON-string param (exactly what the
   serializer's own guard comment prescribed), and the rows/find query
   contracts decode JSON-string filter/sort/after before the union runs.
   Proven end-to-end with a real NextRequest for both grammars.

Session bulk predicates are now id-keyed pass-through (matching the grid);
name-keyed translation remains for INTERNAL_JWT workflow tools — tests updated
to the corrected contract and extended for every finding.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R

Author: TheodoreSpeaks

apps/sim/app/api/table/[tableId]/cancel-runs/route.ts

@@ -5,6 +5,7 @@ import { parseRequest } from '@/lib/api/server'
 import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
 import { generateRequestId } from '@/lib/core/utils/request'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
+import { TableQueryValidationError } from '@/lib/table/errors'
 import { toLegacyFilter } from '@/lib/table/query-builder/converters'
 import { cancelWorkflowGroupRuns } from '@/lib/table/workflow-columns'
 import { accessError, checkAccess, tableFilterError } from '@/app/api/table/utils'
@@ -61,6 +62,11 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro
 
     return NextResponse.json({ success: true, data: { cancelled } })
   } catch (error) {
+    // A predicate that Zod accepts but the downgrade rejects (hybrid node,
+    // eq-with-array, valueless op) is caller error, not a server fault.
+    if (error instanceof TableQueryValidationError) {
+      return NextResponse.json({ error: error.message }, { status: 400 })
+    }
     logger.error(`[${requestId}] cancel-runs failed:`, error)
     return NextResponse.json({ error: 'Failed to cancel runs' }, { status: 500 })
   }

apps/sim/app/api/table/[tableId]/columns/run/route.ts

@@ -5,6 +5,7 @@ import { parseRequest } from '@/lib/api/server'
 import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
 import { generateRequestId } from '@/lib/core/utils/request'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
+import { TableQueryValidationError } from '@/lib/table/errors'
 import { toLegacyFilter } from '@/lib/table/query-builder/converters'
 import { runWorkflowColumn } from '@/lib/table/workflow-columns'
 import { accessError, checkAccess, tableFilterError } from '@/app/api/table/utils'
@@ -60,6 +61,11 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro
 
     return NextResponse.json({ success: true, data: { dispatchId } })
   } catch (error) {
+    // A predicate that Zod accepts but the downgrade rejects (hybrid node,
+    // eq-with-array, valueless op) is caller error, not a server fault.
+    if (error instanceof TableQueryValidationError) {
+      return NextResponse.json({ error: error.message }, { status: 400 })
+    }
     if (error instanceof Error && error.message === 'Invalid workspace ID') {
       return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 })
     }

apps/sim/app/api/table/[tableId]/delete-async/route.test.ts

@@ -208,4 +208,25 @@ describe('POST /api/table/[tableId]/delete-async', () => {
     expect(mockReleaseJobClaim).toHaveBeenCalledWith('tbl_1', 'job-id-xyz')
     expect(mockRunTableDelete).not.toHaveBeenCalled()
   })
+
+  /**
+   * PR #6067 review finding (greptile P1 / bugbot High): a hybrid filter — group
+   * key AND leaf keys on one node — passes the dual-grammar union via the
+   * non-stripping legacy branch, and the downgrade used to convert group-first,
+   * silently dropping the leaf and WIDENING an async select-all delete.
+   */
+  it('rejects a hybrid group+leaf filter with 400 instead of widening the delete', async () => {
+    const response = await makeRequest({
+      workspaceId: 'workspace-1',
+      filter: {
+        all: [{ field: 'tenant_id', op: 'eq', value: 'acme' }],
+        field: 'status',
+        op: 'eq',
+        value: 'archived',
+      },
+    })
+    expect(response.status).toBe(400)
+    const body = await response.json()
+    expect(body.error).toMatch(/not both/)
+  })
 })

apps/sim/app/api/table/[tableId]/delete-async/route.ts

@@ -8,7 +8,9 @@ import { isTriggerDevEnabled } from '@/lib/core/config/env-flags'
 import { runDetached } from '@/lib/core/utils/background'
 import { generateRequestId } from '@/lib/core/utils/request'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
+import type { Filter } from '@/lib/table'
 import { markTableDeleteFailed, runTableDelete } from '@/lib/table/delete-runner'
+import { TableQueryValidationError } from '@/lib/table/errors'
 import { markTableJobRunning, releaseJobClaim } from '@/lib/table/jobs/service'
 import { assertRowDelete } from '@/lib/table/mutation-locks'
 import { toLegacyFilter } from '@/lib/table/query-builder/converters'
@@ -46,8 +48,18 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro
   const { tableId } = parsed.data.params
   const { workspaceId, filter: wireFilter, excludeRowIds, estimatedCount } = parsed.data.body
   // Dual-grammar wire: a predicate downgrades losslessly-or-throws to the
-  // legacy Filter the runners/persisted payloads still compile.
-  const filter = toLegacyFilter(wireFilter)
+  // legacy Filter the runners/persisted payloads still compile. A shape the
+  // union accepted but the downgrade rejects (hybrid node, eq-with-array) is
+  // caller error — 400, never the generic 500.
+  let filter: Filter | undefined
+  try {
+    filter = toLegacyFilter(wireFilter)
+  } catch (error) {
+    if (error instanceof TableQueryValidationError) {
+      return NextResponse.json({ error: error.message }, { status: 400 })
+    }
+    throw error
+  }
 
   const access = await checkAccess(tableId, userId, 'write')
   if (!access.ok) return accessError(access, requestId, tableId)

apps/sim/app/api/table/[tableId]/rows/route.test.ts

@@ -290,13 +290,43 @@ describe('PUT/DELETE /api/table/[tableId]/rows — predicate filters', () => {
     return DELETE(req, { params: Promise.resolve({ tableId: 'tbl_1' }) })
   }
 
-  it('PUT translates a name-keyed predicate to an id-keyed filter under SESSION auth', async () => {
+  /**
+   * Keying follows the caller (PR #6067 review): the grid authors ID-keyed
+   * predicates and the session wire is identity, so ids pass through — and a
+   * NAME under session auth is just an unknown storage key, rejected like any
+   * other typo rather than half-translated.
+   */
+  it('PUT passes an id-keyed predicate through untouched under SESSION auth', async () => {
+    authAs('session')
+    const res = await callPut({
+      workspaceId: 'workspace-1',
+      filter: { all: [{ field: 'col_aaa', op: 'eq', value: 'Ada' }] },
+      data: { col_aaa: 'Grace' },
+    })
+
+    expect(res.status).toBe(200)
+    const args = mockUpdateRowsByFilter.mock.calls[0][1]
+    expect(args.filter).toEqual({ $and: [{ col_aaa: 'Ada' }] })
+  })
+
+  it('PUT rejects an unknown storage key under SESSION auth with 400', async () => {
     authAs('session')
     const res = await callPut({
       workspaceId: 'workspace-1',
       filter: { all: [{ field: 'Name', op: 'eq', value: 'Ada' }] },
       data: { col_aaa: 'Grace' },
     })
+    expect(res.status).toBe(400)
+    expect(mockUpdateRowsByFilter).not.toHaveBeenCalled()
+  })
+
+  it('PUT translates a name-keyed predicate for INTERNAL_JWT callers', async () => {
+    authAs('internal_jwt')
+    const res = await callPut({
+      workspaceId: 'workspace-1',
+      filter: { all: [{ field: 'Name', op: 'eq', value: 'Ada' }] },
+      data: { Name: 'Grace' },
+    })
 
     expect(res.status).toBe(200)
     const args = mockUpdateRowsByFilter.mock.calls[0][1]

apps/sim/app/api/table/[tableId]/rows/route.ts

@@ -27,9 +27,11 @@ import {
 } from '@/lib/table'
 import { TableQueryValidationError } from '@/lib/table/errors'
 import { isTablePredicate, predicateToFilter } from '@/lib/table/query-builder/converters'
-import { validatePredicate } from '@/lib/table/query-builder/validate'
+import {
+  validatePredicateShape,
+  validateStoragePredicate,
+} from '@/lib/table/query-builder/validate'
 import { queryRows } from '@/lib/table/rows/service'
-import { predicateToStorage } from '@/lib/table/select-values'
 import type { TablePredicate } from '@/lib/table/types'
 import { type RowWireTranslators, rowWireTranslators } from '@/app/api/table/row-wire'
 import { accessError, checkAccess, rowWriteErrorResponse } from '@/app/api/table/utils'
@@ -60,8 +62,15 @@ function resolveBulkFilter(
   wire: RowWireTranslators
 ): Filter {
   if (isTablePredicate(raw)) {
-    validatePredicate(raw, schema.columns)
-    return predicateToFilter(predicateToStorage(raw, schema))
+    // Shape first (keying-agnostic: hybrid nodes, leaf value rules), then let
+    // the wire translate — identity for the ID-keyed grid, names→ids for
+    // workflow tools — and validate the RESULT against storage keys. Post-
+    // translation, any unresolved field is a typo in the caller's own keying,
+    // and on a destructive path a typo must 400, not silently match nothing.
+    validatePredicateShape(raw)
+    const translated = wire.predicateIn(raw)
+    validateStoragePredicate(translated, schema.columns)
+    return predicateToFilter(translated)
   }
   return wire.filterIn(raw)
 }
@@ -294,7 +303,14 @@ export const GET = withRouteHandler(
         table,
         {
           ...(validated.filter && isTablePredicate(validated.filter as Filter | TablePredicate)
-            ? { predicate: wire.predicateIn(validated.filter as TablePredicate) }
+            ? {
+                predicate: (() => {
+                  // Shape-check first: nothing upstream validates this branch, and
+                  // an unchecked hybrid node would silently widen the result.
+                  validatePredicateShape(validated.filter as TablePredicate)
+                  return wire.predicateIn(validated.filter as TablePredicate)
+                })(),
+              }
             : { filter: validated.filter ? wire.filterIn(validated.filter as Filter) : undefined }),
           sort: resolveWireSort(validated.sort as Sort | SortSpec | undefined, wire),
           limit: validated.limit,

apps/sim/lib/api/client/request.test.ts

@@ -54,18 +54,36 @@ describe('requestJson query serialization', () => {
     expect(calledUrl).not.toContain('[object Object]')
   })
 
-  it('throws instead of silently corrupting an array-of-objects query param', async () => {
-    mockFetchReturning({ ok: true })
+  it('JSON-encodes an array-of-objects query param instead of corrupting or throwing', async () => {
+    // A SortSpec ([{field, direction}]) is the everyday case: repeat-append
+    // would send "[object Object]", so the whole array travels as ONE JSON
+    // string param, mirroring plain objects; the server contract decodes it.
+    const fetchMock = mockFetchReturning({ ok: true })
 
-    const badContract = defineRouteContract({
+    const contract = defineRouteContract({
       method: 'GET',
       path: '/api/test',
       query: z.object({ items: z.array(z.object({ a: z.string() })) }),
       response: { mode: 'json', schema: z.object({ ok: z.boolean() }) },
     })
 
-    await expect(requestJson(badContract, { query: { items: [{ a: 'x' }] } })).rejects.toThrow(
-      /arrays of objects are not URL-safe/
-    )
+    await requestJson(contract, { query: { items: [{ a: 'x' }] } })
+    const url = String(fetchMock.mock.calls[0][0])
+    expect(decodeURIComponent(url)).toContain('items=[{"a":"x"}]')
+  })
+
+  it('keeps repeat-append for scalar arrays', async () => {
+    const fetchMock = mockFetchReturning({ ok: true })
+
+    const contract = defineRouteContract({
+      method: 'GET',
+      path: '/api/test',
+      query: z.object({ tags: z.array(z.string()) }),
+      response: { mode: 'json', schema: z.object({ ok: z.boolean() }) },
+    })
+
+    await requestJson(contract, { query: { tags: ['a', 'b'] } })
+    const url = String(fetchMock.mock.calls[0][0])
+    expect(url).toContain('tags=a&tags=b')
   })
 })

apps/sim/lib/api/client/request.ts

@@ -67,19 +67,17 @@ function appendQuery(path: string, query: unknown): string {
     if (value === undefined || value === null || value === '') continue
 
     if (Array.isArray(value)) {
+      // An array of objects (e.g. a SortSpec) is not repeat-append-able — each
+      // item would stringify to "[object Object]" and silently corrupt the
+      // request (the knowledge tagFilters bug). Encode the WHOLE array as one
+      // JSON string param, mirroring how plain objects are sent below; the
+      // server-side contract decodes it. Scalar arrays keep repeat-append.
+      if (value.some((item) => item !== null && typeof item === 'object')) {
+        searchParams.set(key, JSON.stringify(value))
+        continue
+      }
       for (const item of value) {
         if (item === undefined || item === null || item === '') continue
-        // A non-scalar in a query array would stringify to "[object Object]" and
-        // silently corrupt the request. Encode such values as a single JSON
-        // string param and decode them server-side instead. Failing loudly here
-        // keeps the boundary honest (this is how the knowledge tagFilters bug
-        // shipped undetected).
-        if (typeof item === 'object') {
-          throw new Error(
-            `Cannot serialize query param "${key}": arrays of objects are not URL-safe — ` +
-              'encode the value as a JSON string param and decode it server-side.'
-          )
-        }
         searchParams.append(key, String(item))
       }
       continue

apps/sim/lib/api/contracts/tables-predicate.test.ts

@@ -10,6 +10,7 @@ import {
   deleteTableRowsBodySchema,
   predicateSchema,
   rowQueryBodySchema,
+  tableRowsQuerySchema,
   updateRowsByFilterBodySchema,
 } from '@/lib/api/contracts/tables'
 import { validatePredicate } from '@/lib/table/query-builder/validate'
@@ -223,3 +224,38 @@ describe('hybrid group+leaf nodes are rejected, not silently narrowed', () => {
     ).toBe(true)
   })
 })
+
+/**
+ * Wire transport: requestJson serializes structured query params as JSON
+ * strings; jsonQueryValue decodes them before the union runs. Without it, a
+ * sorted or filtered grid request 400s at the boundary.
+ */
+describe('query-string JSON transport (jsonQueryValue)', () => {
+  it('decodes string-encoded predicate, legacy filter, and sort spec', () => {
+    const parsed = rowQueryStringSchemaProbe({
+      workspaceId: 'ws-1',
+      filter: JSON.stringify({ all: [{ field: 'a', op: 'eq', value: 1 }] }),
+      sort: JSON.stringify([{ field: 'a', direction: 'asc' }]),
+    })
+    expect(parsed.filter).toEqual({ all: [{ field: 'a', op: 'eq', value: 1 }] })
+    expect(parsed.sort).toEqual([{ field: 'a', direction: 'asc' }])
+
+    const legacy = rowQueryStringSchemaProbe({
+      workspaceId: 'ws-1',
+      filter: JSON.stringify({ status: { $eq: 'x' } }),
+      sort: JSON.stringify({ status: 'desc' }),
+    })
+    expect(legacy.filter).toEqual({ status: { $eq: 'x' } })
+    expect(legacy.sort).toEqual({ status: 'desc' })
+  })
+
+  it('still rejects a non-JSON garbage string with the real schema error', () => {
+    expect(() => rowQueryStringSchemaProbe({ workspaceId: 'ws-1', filter: 'not json' })).toThrow()
+  })
+})
+
+function rowQueryStringSchemaProbe(input: Record<string, unknown>) {
+  const result = tableRowsQuerySchema.safeParse(input)
+  if (!result.success) throw new Error(JSON.stringify(result.error.issues[0]))
+  return result.data
+}

apps/sim/lib/api/contracts/tables.ts

@@ -545,20 +545,37 @@ export const deleteTableRowsBodySchema = z
     message: 'Provide either filter or rowIds, but not both',
   })
 
+/**
+ * Query-param transport for a structured value: `requestJson` serializes
+ * objects/arrays into a single JSON-string param, and this decodes it before
+ * the real schema runs. Non-JSON strings pass through untouched so the inner
+ * schema produces the real error; already-parsed values (POST bodies reusing a
+ * schema) are untouched too.
+ */
+const jsonQueryValue = <S extends z.ZodType>(schema: S) =>
+  z.preprocess((value) => {
+    if (typeof value !== 'string' || value === '') return value
+    try {
+      return JSON.parse(value)
+    } catch {
+      return value
+    }
+  }, schema)
+
 /** Unrefined base so v1 contracts can `.extend()` — consumers use {@link tableRowsQuerySchema}. */
 export const tableRowsQueryBaseSchema = z.object({
   workspaceId: workspaceIdSchema,
   // Dual-grammar during the v2 transition: the strict predicate tree wins the
   // union; anything else falls through to the legacy `$`-object. Same for sort:
   // an ordered spec array vs the legacy `{col: dir}` record.
-  filter: z.union([predicateSchema, domainObjectSchema<Filter>()]).optional(),
-  sort: z.union([sortSpecSchema, domainObjectSchema<Sort>()]).optional(),
+  filter: jsonQueryValue(z.union([predicateSchema, domainObjectSchema<Filter>()])).optional(),
+  sort: jsonQueryValue(z.union([sortSpecSchema, domainObjectSchema<Sort>()])).optional(),
   /**
    * Keyset cursor `(orderKey, id)` for the default row order — each page is an index seek
    * instead of OFFSET's scan-and-discard. Mutually exclusive with `sort` (cursors only make
    * sense on the default order); takes precedence over `offset`.
    */
-  after: domainObjectSchema<TableRowsCursor>().optional(),
+  after: jsonQueryValue(domainObjectSchema<TableRowsCursor>()).optional(),
   limit: z
     .preprocess(
       (value) =>
@@ -865,8 +882,8 @@ export type RowQueryResponse = ContractJsonResponse<typeof rowQueryContract>
 export const findTableRowsQuerySchema = z.object({
   workspaceId: workspaceIdSchema,
   q: requiredFieldSchema('Search query is required'),
-  filter: z.union([predicateSchema, domainObjectSchema<Filter>()]).optional(),
-  sort: z.union([sortSpecSchema, domainObjectSchema<Sort>()]).optional(),
+  filter: jsonQueryValue(z.union([predicateSchema, domainObjectSchema<Filter>()])).optional(),
+  sort: jsonQueryValue(z.union([sortSpecSchema, domainObjectSchema<Sort>()])).optional(),
 })
 
 /** One matching cell: its 0-based ordinal in the filtered+sorted view, its row id, and the column name. */