fix(data): implicit field filters AND-merge with an explicit filter instead of being silently dropped (#4164)

.changeset/rest-list-implicit-filter-and-merge.md

@@ -0,0 +1,26 @@
+---
+"@objectstack/metadata-protocol": patch
+---
+
+fix(data): implicit field filters compose with an explicit `filter` by AND instead of being silently dropped (#4164)
+
+`GET /api/v1/data/:object?filter={"status":"open"}&owner_id=usr_1` used to
+apply only the explicit filter: the bare `owner_id` predicate was neither
+merged nor reported — it rode to the engine as a stray AST key no driver
+reads, and the response over-returned. The mirror of #4134's silent zero,
+same disease, opposite direction.
+
+The two now compose the way the request reads: `{ $and: [explicit, implicit] }`
+— the same combinator the engine already uses to fold the `search` predicate
+into an existing `where`, and one the cross-backend filter-logic conformance
+suite pins. Contradictory sides (`?filter={"status":"open"}&status=closed`)
+apply both predicates and intersect to an honest empty set. Pagination totals
+(`total` / `hasMore`) are computed over the merged predicate, so they cannot
+disagree with `records`.
+
+**What changes for callers:** requests that sent both an explicit `filter` and
+bare field parameters now get the narrower, as-written result set instead of
+the explicit filter alone. Requests sending only one of the two mechanisms are
+unaffected. Thanks to #4134 (shipped previously), every bare parameter that
+reaches the merge is a verified field name, so the merge can never introduce a
+zero-matching predicate.

content/docs/api/data-api.mdx

@@ -32,9 +32,12 @@ Query records with filtering, sorting, selection, and pagination.
 
 A query parameter the endpoint does not reserve is read as a field-level
 equality filter, so `?status=done` is shorthand for
-`?filter={"status":"done"}`. Because such a parameter *is* a predicate, one
-naming a field the object does not have could only ever match zero records —
-so the endpoint rejects it instead of returning an empty page:
+`?filter={"status":"done"}`. When an explicit `filter` is also present, the
+two compose by AND — `?filter={"amount":{"$gte":100}}&status=done` applies
+both predicates, the same way the `search` parameter composes with `filter`.
+Because such a parameter *is* a predicate, one naming a field the object does
+not have could only ever match zero records — so the endpoint rejects it
+instead of returning an empty page:
 
 ```http
 GET /api/v1/data/showcase_task?pageSize=5

packages/metadata-protocol/src/protocol.ts

@@ -3182,27 +3182,44 @@ export class ObjectStackProtocolImplementation implements
         }
 
         // Flat field filters: REST-style query params like ?id=abc&status=open
-        // After extracting all known query parameters, any remaining keys are
-        // treated as implicit field-level equality filters merged into `where`.
-        // Every one of them is a verified field name by this point.
+        // are implicit field-level equality predicates. Every leftover key is a
+        // verified field name by this point — the #4134 gate above runs FIRST,
+        // which is exactly what makes the merge below safe: an unknown name
+        // already 400'd, so nothing merged here can be a predicate that matches
+        // nothing.
         //
-        // The `!options.where` guard is #4134's deliberate scope edge: when an
-        // explicit filter won, a leftover REAL field name is still dropped
-        // silently (it rides on to `engine.find` as a stray AST key no driver
-        // reads). Same disease, opposite direction — over-returning rather than
-        // zeroing — and fixing it means choosing a semantics (AND-merge vs.
-        // reject the conflict), so it is filed as #4164 rather than smuggled in
-        // here. The UNKNOWN half is already loud: the gate above runs before
-        // this branch, so a bad name 400s either way.
-        if (!options.where) {
+        // [#4164] Implicit predicates now COMPOSE with an explicit `where`
+        // instead of being silently dropped. `?filter={...}&status=open` means
+        // what it says — both apply, `{ $and: [explicit, implicit] }` — the
+        // same composition the engine itself uses to fold the `$search`
+        // predicate or an expand's declared filter into an existing `where`,
+        // and a combinator the #3774 conformance suite pins across every
+        // FilterCondition backend. Contradictory sides need no special case:
+        // both predicates apply and the intersection is an HONEST zero (the
+        // filters ran), unlike the pre-#4134 zero (a filter that never ran).
+        // Consuming the keys here also stops them riding to `engine.find` as
+        // stray top-level AST junk, and count() below reads the same
+        // `options.where`, so pagination totals see the merged predicate too.
+        //
+        // Deliberately NOT merged: a non-object truthy `where` — the parse
+        // tolerance above keeps an unparseable `filter` JSON string as-is
+        // (#4181), and folding real predicates into that garbage would neither
+        // apply them nor surface the actual bug. That path keeps its pre-#4164
+        // shape until the tolerance itself is fixed at the source.
+        const explicitWhere = options.where;
+        const whereIsMergeable = !explicitWhere || typeof explicitWhere === 'object';
+        if (leftoverParams.length > 0 && whereIsMergeable) {
             const implicitFilters: Record<string, unknown> = {};
             for (const key of leftoverParams) {
                 implicitFilters[key] = options[key];
                 delete options[key];
             }
-            if (Object.keys(implicitFilters).length > 0) {
-                options.where = implicitFilters;
-            }
+            // An absent or empty explicit filter (`?filter={}`, `?filter=`) is
+            // vacuous — the implicit predicates stand alone rather than being
+            // wrapped in a one-armed `$and`.
+            options.where = !explicitWhere || Object.keys(explicitWhere).length === 0
+                ? implicitFilters
+                : { $and: [explicitWhere, implicitFilters] };
         }
 
         // Route to engine.aggregate() when the query has GROUP BY / aggregations.

packages/objectql/src/protocol-data.test.ts

@@ -760,16 +760,99 @@ describe('ObjectStackProtocolImplementation - Data Operations', () => {
             ).rejects.toMatchObject({ field: 'zzzz', fields: ['zzzz', 'yyyy'] });
         });
 
-        it('leaves the known-field-plus-explicit-filter case alone (#4164)', async () => {
-            // The scope edge, pinned so a later change to it is deliberate: a
-            // REAL field name alongside an explicit filter is still dropped
-            // silently. #4134 made only the UNKNOWN half loud.
+        it('AND-merges a known field with an explicit filter instead of dropping it (#4164)', async () => {
+            // The #4134 pin test used to freeze the drop behavior here; #4164
+            // is the deliberate change it was waiting for. Both predicates now
+            // apply, and the consumed key no longer rides to the engine as a
+            // stray top-level AST key.
             const { protocol, engine } = makeProtocol();
             await protocol.findData({
                 object: 'showcase_task',
                 query: { filter: { status: 'open' }, owner_id: 'usr_1' },
             });
-            expect(engine.find.mock.calls[0][1].where).toEqual({ status: 'open' });
+            const opts = engine.find.mock.calls[0][1];
+            expect(opts.where).toEqual({ $and: [{ status: 'open' }, { owner_id: 'usr_1' }] });
+            expect(opts.owner_id).toBeUndefined();
+        });
+
+        it('merges every implicit field as ONE $and arm', async () => {
+            const { protocol, engine } = makeProtocol();
+            await protocol.findData({
+                object: 'showcase_task',
+                query: { filter: { status: 'open' }, owner_id: 'usr_1', due_date: '2026-08-01' },
+            });
+            expect(engine.find.mock.calls[0][1].where).toEqual({
+                $and: [{ status: 'open' }, { owner_id: 'usr_1', due_date: '2026-08-01' }],
+            });
+        });
+
+        it('a contradictory pair applies both sides — an honest empty set, not a silent wide one', async () => {
+            // `?filter={"status":"open"}&status=closed` — both predicates run;
+            // the intersection being empty is the caller's contradiction, not a
+            // dropped filter. No special-casing of same-field collisions.
+            const { protocol, engine } = makeProtocol();
+            await protocol.findData({
+                object: 'showcase_task',
+                query: { filter: { status: 'open' }, status: 'closed' },
+            });
+            expect(engine.find.mock.calls[0][1].where).toEqual({
+                $and: [{ status: 'open' }, { status: 'closed' }],
+            });
+        });
+
+        it('a vacuous explicit filter ({} or empty string) lets implicit predicates stand alone', async () => {
+            const { protocol, engine } = makeProtocol();
+            await protocol.findData({
+                object: 'showcase_task',
+                query: { filter: {}, owner_id: 'usr_1' },
+            });
+            expect(engine.find.mock.calls[0][1].where).toEqual({ owner_id: 'usr_1' });
+            // `?filter=` — the parse tolerance keeps '' as-is; the old guard
+            // treated it as no-filter, and the merge must keep doing so.
+            await protocol.findData({
+                object: 'showcase_task',
+                query: { filter: '', owner_id: 'usr_1' },
+            });
+            expect(engine.find.mock.calls[1][1].where).toEqual({ owner_id: 'usr_1' });
+        });
+
+        it('count() sees the merged where — pagination totals cannot disagree with records (#4164)', async () => {
+            const { protocol, engine } = makeProtocol();
+            await protocol.findData({
+                object: 'showcase_task',
+                query: { filter: { status: 'open' }, owner_id: 'usr_1', top: 5 },
+            });
+            expect(engine.count).toHaveBeenCalledTimes(1);
+            expect(engine.count.mock.calls[0][1].where).toEqual({
+                $and: [{ status: 'open' }, { owner_id: 'usr_1' }],
+            });
+        });
+
+        it('merges identically through the $filter JSON-string alias', async () => {
+            const { protocol, engine } = makeProtocol();
+            await protocol.findData({
+                object: 'showcase_task',
+                query: { $filter: JSON.stringify({ status: 'open' }), owner_id: 'usr_1' },
+            });
+            expect(engine.find.mock.calls[0][1].where).toEqual({
+                $and: [{ status: 'open' }, { owner_id: 'usr_1' }],
+            });
+        });
+
+        it('leaves a non-object where (unparseable filter JSON) untouched — pinned until #4181', async () => {
+            // `?filter={oops` — the parse tolerance keeps the raw string and it
+            // becomes `where`. Folding real predicates into that garbage would
+            // neither apply them nor surface the tolerance bug itself (#4181),
+            // so this path keeps its pre-#4164 shape: string where forwarded,
+            // implicit key left as a stray option.
+            const { protocol, engine } = makeProtocol();
+            await protocol.findData({
+                object: 'showcase_task',
+                query: { filter: '{oops', owner_id: 'usr_1' },
+            });
+            const opts = engine.find.mock.calls[0][1];
+            expect(opts.where).toBe('{oops');
+            expect(opts.owner_id).toBe('usr_1');
         });
 
         it('rejects even when an explicit filter rode along — the 400 must not depend on it', async () => {

packages/objectql/src/protocol-unknown-query-param.test.ts

@@ -46,6 +46,13 @@ function makeMemoryDriver() {
     const matchesWhere = (row: Record<string, unknown>, where: any): boolean => {
         if (!where || typeof where !== 'object') return true;
         for (const [k, v] of Object.entries(where)) {
+            // `$and` must be honored, not skipped — #4164 composes the explicit
+            // filter and the implicit field predicates through it, so a matcher
+            // that ignores `$and` would green-light a merge that does nothing.
+            if (k === '$and' && Array.isArray(v)) {
+                if (!v.every((arm) => matchesWhere(row, arm))) return false;
+                continue;
+            }
             if (k.startsWith('$')) continue;
             const expected = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v;
             const a = row[k] === undefined ? null : row[k];
@@ -206,4 +213,45 @@ describe('#4134 — unknown list query params (real ObjectQL engine)', () => {
             protocol.findData({ object: 'no_such_object', query: { pageSize: '5' } }),
         ).rejects.toMatchObject({ status: 404, code: 'OBJECT_NOT_FOUND' });
     });
+
+    // ─────────────────────────────────────────────────────────────
+    // #4164 — implicit predicates compose with an explicit filter
+    // ─────────────────────────────────────────────────────────────
+
+    it('a bare field param NARROWS an explicit filter instead of vanishing (#4164)', async () => {
+        // Pre-#4164 the `title` predicate was dropped and this returned all 8
+        // open tasks — over-returning, the mirror of #4134's zeroing.
+        const r: any = await protocol.findData({
+            object: 'showcase_task',
+            query: { filter: JSON.stringify({ status: 'open' }), title: 'Task 3' },
+        });
+        expect(r.total).toBe(1);
+        expect(r.records.map((x: any) => x.id)).toEqual(['t3']);
+    });
+
+    it('contradictory sides intersect to an honest zero — both filters ran', async () => {
+        // 'Task 3' is open, so requiring status=done alongside it matches
+        // nothing. Unlike the #4134 zero, every written predicate was applied.
+        const r: any = await protocol.findData({
+            object: 'showcase_task',
+            query: { filter: JSON.stringify({ status: 'done' }), title: 'Task 3' },
+        });
+        expect(r.total).toBe(0);
+    });
+
+    it('pagination totals are computed over the MERGED predicate', async () => {
+        // 8 open rows share created_at; page of 3 → total must be 8 (the
+        // merged match count), not 10 (unfiltered) nor 3 (page-local).
+        const r: any = await protocol.findData({
+            object: 'showcase_task',
+            query: {
+                filter: JSON.stringify({ status: 'open' }),
+                created_at: '2026-07-30T00:00:00.000Z',
+                top: 3,
+            },
+        });
+        expect(r.records).toHaveLength(3);
+        expect(r.total).toBe(8);
+        expect(r.hasMore).toBe(true);
+    });
 });