fix(objectql): reject wire-only alias spellings (sort/select/skip/populate) on direct engine calls

.changeset/engine-rejects-wire-only-aliases.md

@@ -0,0 +1,32 @@
+---
+"@objectstack/objectql": patch
+---
+
+fix(objectql): a direct engine call carrying `sort`/`select`/`skip`/`populate` now throws instead of silently dropping the parameter (#4371)
+
+The engine folds `filter`→`where` and `top`→`limit` itself (#4346); the other
+four pairs in `RPC_QUERY_ALIAS_SLOTS` fold at the RPC/protocol layer only,
+because their value shapes need lowering (`sort`'s `{field: 'asc'}` record
+form, `populate`'s name list) that belongs there. A **direct** `engine.find()`
+/ `findOne()` never crosses that layer, so one of those keys used to ride the
+AST verbatim, drivers read only the canonical name, and the request succeeded
+with the parameter discarded — `sort` + `limit` ("the latest N") silently
+returning an arbitrary N. Three shipped instances were fixed in #4370, and a
+fourth sat in the engine's own autonumber seeding (`select` — now `fields`).
+
+`find`/`findOne` now reject a non-null wire-only spelling with an error naming
+the canonical key and shape, e.g.:
+
+> `find('task') does not accept 'sort': 'sort' is a wire spelling of
+> 'orderBy', folded by the RPC/protocol layer — a direct engine call bypasses
+> that fold, so the value would be silently dropped, not applied. Pass
+> 'orderBy' (SortNode[]: [{ field, order: 'asc' | 'desc' }]) instead.`
+
+Migration for direct engine callers (HTTP/RPC callers are unaffected — the
+wire fold is unchanged): `select: [...]` → `fields: [...]`;
+`sort: {f: 'asc'}` or `sort: [{field, order}]` → `orderBy: [{field, order}]`;
+`skip: n` → `offset: n`; `populate: ['rel']` → `expand: {rel: {object: 'rel'}}`.
+An explicit `null` under a wire spelling remains a withdrawal (ignored), and
+the where-only methods (`update`/`delete`/`count`/`aggregate`) are unchanged —
+their contracts honour no sort/projection/pagination in either spelling
+(unknown-key enforcement there is #4371's follow-up scope).

packages/objectql/src/engine-wire-alias-reject.test.ts

@@ -0,0 +1,187 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #4371 — a direct engine call carrying a wire-only alias spelling
+ * (`sort`/`select`/`skip`/`populate`) is REJECTED, not silently dropped.
+ *
+ * Regression: #4346 folds `filter`→`where` and `top`→`limit` on every engine
+ * entry point, but the other four pairs in `RPC_QUERY_ALIAS_SLOTS` fold at the
+ * RPC/protocol layer only (their value shapes need lowering that belongs
+ * there). A direct `engine.find()` never crosses that layer, so the alias key
+ * rode the AST verbatim, drivers read only the canonical name, and the request
+ * SUCCEEDED with the parameter discarded — `sort` + `limit` ("the latest N")
+ * silently returning an arbitrary N. Three shipped instances were fixed in
+ * #4370; a fourth sat in the engine's own `seedAutonumber`. The engine now
+ * throws at the boundary, naming the canonical key and shape, while the
+ * RPC/wire path keeps folding exactly as before — the fold must NOT move.
+ */
+
+import { describe, it, expect, beforeEach } from 'vitest';
+import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
+import { ObjectQL } from './engine.js';
+
+const task = {
+    name: 'task',
+    label: 'Task',
+    fields: {
+        id: { name: 'id', type: 'text' as const, primaryKey: true },
+        title: { name: 'title', type: 'text' as const },
+        status: { name: 'status', type: 'text' as const },
+    },
+};
+
+/** Memory driver that records every AST handed to find/findOne. */
+function makeRecordingDriver() {
+    const stores = new Map<string, Map<string, Record<string, unknown>>>();
+    const storeFor = (o: string) => { let s = stores.get(o); if (!s) { s = new Map(); stores.set(o, s); } return s; };
+    const finds: any[] = [];
+    let nextId = 0;
+    const matches = (row: Record<string, unknown>, where: any): boolean => {
+        if (!where || typeof where !== 'object') return true;
+        for (const [k, v] of Object.entries(where)) {
+            if (k.startsWith('$')) continue;
+            const exp = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v;
+            if ((row[k] ?? null) !== (exp ?? null)) return false;
+        }
+        return true;
+    };
+    const run = (o: string, ast: any) => {
+        let rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where));
+        const ord = Array.isArray(ast?.orderBy) ? ast.orderBy : [];
+        if (ord.length > 0) {
+            rows = [...rows].sort((a: any, b: any) => {
+                for (const { field, order } of ord) {
+                    const cmp = String(a?.[field] ?? '').localeCompare(String(b?.[field] ?? ''));
+                    if (cmp !== 0) return order === 'desc' ? -cmp : cmp;
+                }
+                return 0;
+            });
+        }
+        if (typeof ast?.offset === 'number' && ast.offset > 0) rows = rows.slice(ast.offset);
+        return typeof ast?.limit === 'number' && ast.limit > 0 ? rows.slice(0, ast.limit) : rows;
+    };
+    const driver: any = {
+        name: 'memory', version: '0.0.0', supports: {},
+        async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
+        async find(o: string, ast: any) { finds.push(ast); return run(o, ast); },
+        findStream() { throw new Error('ns'); },
+        async findOne(o: string, ast: any) { finds.push(ast); return run(o, ast)[0] ?? null; },
+        async create(o: string, data: Record<string, unknown>) {
+            nextId += 1; const id = (data.id as string) ?? `r_${nextId}`; const row = { ...data, id }; storeFor(o).set(id, row); return row;
+        },
+        async update(o: string, id: string, data: Record<string, unknown>) {
+            const s = storeFor(o); const cur = s.get(id); if (!cur) throw new Error(`nf ${o}/${id}`);
+            const up = { ...cur, ...data, id }; s.set(id, up); return up;
+        },
+        async delete(o: string, id: string) { return storeFor(o).delete(id); },
+        async count(o: string, ast: any) { return run(o, ast).length; },
+        async bulkCreate(o: string, rows: Record<string, unknown>[]) { return Promise.all(rows.map((r) => this.create(o, r))); },
+        async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, async commit() {}, async rollback() {},
+    };
+    return { driver, stores, finds };
+}
+
+describe('wire-only alias spellings are rejected on direct engine calls (#4371)', () => {
+    let engine: ObjectQL;
+    let finds: any[];
+
+    beforeEach(async () => {
+        engine = new ObjectQL();
+        const mem = makeRecordingDriver();
+        finds = mem.finds;
+        engine.registerDriver(mem.driver, true);
+        await engine.init();
+        engine.registry.registerObject(task as any);
+        await engine.insert('task', { title: 'B', status: 'done' });
+        await engine.insert('task', { title: 'A', status: 'open' });
+        await engine.insert('task', { title: 'C', status: 'done' });
+        finds.length = 0; // drop insert-path reads; the pins below own this log
+    });
+
+    // ── each wire spelling throws before the driver sees anything ──────
+
+    it.each([
+        ['sort', { created_at: 'asc' }, 'orderBy'],
+        ['select', ['id', 'title'], 'fields'],
+        ['skip', 10, 'offset'],
+        ['populate', ['owner'], 'expand'],
+    ])('find({%s}) is refused, naming the canonical key', async (alias, value, canonical) => {
+        await expect(engine.find('task', { [alias]: value } as any)).rejects.toThrow(
+            new RegExp(`wire spelling of '${canonical}'.*Pass '${canonical}'`, 's'),
+        );
+        expect(finds).toHaveLength(0);
+    });
+
+    it('findOne({sort}) is refused too — "first row of THIS order" degrades the same way', async () => {
+        await expect(engine.findOne('task', { sort: { title: 'asc' } } as any))
+            .rejects.toThrow(/findOne\('task'\) does not accept 'sort'/);
+        expect(finds).toHaveLength(0);
+    });
+
+    it('a sort already in SortNode[] shape is still refused — the KEY is the contract', async () => {
+        await expect(engine.find('task', { sort: [{ field: 'title', order: 'asc' }] } as any))
+            .rejects.toThrow(/wire spelling of 'orderBy'/);
+    });
+
+    it('several wire spellings at once are reported in ONE rejection', async () => {
+        await expect(
+            engine.find('task', { sort: { title: 'asc' }, populate: ['owner'] } as any),
+        ).rejects.toThrow(/'sort'.*'populate'|'populate'.*'sort'/s);
+    });
+
+    // ── the canonical spellings all work — the migration target is real ─
+
+    it('find({where, fields, orderBy, offset, limit}) reaches the driver canonically', async () => {
+        const rows = await engine.find('task', {
+            where: { status: 'done' },
+            fields: ['id', 'title'],
+            orderBy: [{ field: 'title', order: 'desc' }],
+            offset: 0,
+            limit: 10,
+        });
+        expect(rows.map((r: any) => r.title)).toEqual(['C', 'B']);
+        expect(finds).toHaveLength(1);
+        expect(finds[0].orderBy).toEqual([{ field: 'title', order: 'desc' }]);
+        expect(finds[0].sort).toBeUndefined();
+    });
+
+    // ── null stays a withdrawal, exactly like the folded slots ──────────
+
+    it('an explicit null wire spelling is a withdrawal, not a rejection', async () => {
+        const rows = await engine.find('task', { sort: null } as any);
+        expect(rows).toHaveLength(3);
+    });
+
+    // ── the folded pairs did not regress behind the new parameter ──────
+
+    it('find({filter, top}) still folds (#4346) with the rejection guard in place', async () => {
+        const rows = await engine.find('task', { filter: { status: 'done' }, top: 1 } as any);
+        expect(rows).toHaveLength(1);
+        expect(finds[0].where).toEqual({ status: 'done' });
+        expect(finds[0].limit).toBe(1);
+    });
+
+    // ── the RPC/wire path still folds — the fold must NOT move (#4371 pin 3) ─
+
+    it('wire `sort` through the protocol layer folds to orderBy and passes the guard', async () => {
+        const protocol = new ObjectStackProtocolImplementation(engine as any);
+        const out: any = await protocol.findData({ object: 'task', query: { sort: '-title' } });
+        expect(out.records.map((r: any) => r.title)).toEqual(['C', 'B', 'A']);
+        expect(finds).toHaveLength(1);
+        expect(finds[0].orderBy).toEqual([{ field: 'title', order: 'desc' }]);
+        expect(finds[0].sort).toBeUndefined();
+    });
+
+    it('wire `select`/`skip`/`populate` through the protocol layer pass the guard as canonical keys', async () => {
+        const protocol = new ObjectStackProtocolImplementation(engine as any);
+        const out: any = await protocol.findData({
+            object: 'task',
+            query: { select: 'id,title', skip: 1, sort: 'title' },
+        });
+        expect(out.records.map((r: any) => r.title)).toEqual(['B', 'C']);
+        expect(finds[0].fields).toEqual(expect.arrayContaining(['id', 'title']));
+        expect(finds[0].offset).toBe(1);
+        expect(finds[0].select).toBeUndefined();
+        expect(finds[0].skip).toBeUndefined();
+    });
+});

packages/objectql/src/engine.ts

@@ -116,13 +116,57 @@ const ENGINE_WHERE_SLOTS: readonly QueryAliasSlot[] =
 const ENGINE_QUERY_SLOTS: readonly QueryAliasSlot[] =
   RPC_QUERY_ALIAS_SLOTS.filter((slot) => slot.canonical === 'where' || slot.canonical === 'limit');
 
+/**
+ * [#4371] The slots the engine does NOT fold — the wire-only pairs
+ * (`select`→`fields`, `sort`→`orderBy`, `skip`→`offset`, `populate`→`expand`),
+ * derived as the complement of {@link ENGINE_QUERY_SLOTS} so a seventh pair
+ * added to the spec table lands on exactly one side of the split.
+ *
+ * Their values need shape lowering (`sort`'s `{field: 'asc'}` record form,
+ * `populate`'s name list) that belongs to the RPC/protocol layers, so folding
+ * here would re-implement the lowering per reader — the #3795 condition. But a
+ * DIRECT engine call never crosses those layers: the alias key used to ride
+ * the AST verbatim, drivers read only the canonical name, and the parameter
+ * was silently dropped — three shipped "latest N in arbitrary order" bugs
+ * (#4370) plus the engine's own `seedAutonumber`. Declared ≠ enforced
+ * (AGENTS.md PD #10): the find-shaped entry points now REJECT these spellings,
+ * naming the canonical key and shape, so the mistake throws at the call site
+ * instead of degrading the result.
+ *
+ * Deliberately NOT applied to the where-only methods
+ * (`update`/`delete`/`count`/`aggregate`): their contracts honour no
+ * sort/projection/pagination at all, so "pass `orderBy` instead" would
+ * redirect the caller to a key those methods silently ignore too. Unknown-key
+ * enforcement for those bags is #4371's option (2), scoped separately.
+ */
+const ENGINE_WIRE_ONLY_SLOTS: readonly QueryAliasSlot[] =
+  RPC_QUERY_ALIAS_SLOTS.filter((slot) => !ENGINE_QUERY_SLOTS.includes(slot));
+
+/**
+ * Canonical shape each wire-only slot's value must be rewritten into — quoted
+ * by the rejection so the error carries the full migration, not just the key
+ * rename (the value shapes differ; that is WHY the engine cannot fold them).
+ */
+const WIRE_ONLY_CANONICAL_SHAPES: Record<string, string> = {
+  fields: "a string[] of field names",
+  orderBy: "SortNode[]: [{ field, order: 'asc' | 'desc' }]",
+  offset: 'a number of rows to skip',
+  expand: 'a record of { relationName: QueryAST }',
+};
+
 /**
  * Fold the deprecated alias spellings of an engine option bag into their
  * canonical QueryAST keys, under the #3795/#4181 rule: an alias alone moves to
  * the canonical key, redundant identical spellings collapse, DIFFERENT values
  * for one slot are irreconcilable and throw (picking a winner IS the silent
  * drop), and an explicit `null` alias is a withdrawal.
  *
+ * `rejectSlots` ({@link ENGINE_WIRE_ONLY_SLOTS}) names the slots whose alias
+ * spellings the engine can neither fold nor honour: a non-null value under one
+ * throws, quoting the canonical key and shape (#4371). `null` stays a
+ * withdrawal here too — it carries no intent a drop could lose — and rides
+ * through for drivers to ignore, exactly as before.
+ *
  * Returns the SAME reference when no alias spelling is present (the common
  * path allocates nothing — `withResolvedWhere` discipline); otherwise folds a
  * shallow copy, because the bag belongs to the caller and may be reused (view
@@ -133,8 +177,31 @@ function foldEngineOptionAliases<T extends object | undefined>(
   operation: string,
   bag: T,
   slots: readonly QueryAliasSlot[],
+  rejectSlots?: readonly QueryAliasSlot[],
 ): T {
   if (!bag) return bag;
+  if (rejectSlots) {
+    const refused = rejectSlots.flatMap((slot) =>
+      slot.aliases
+        .filter((alias) => (bag as Record<string, unknown>)[alias] != null)
+        .map((alias) => ({ alias, canonical: slot.canonical })),
+    );
+    if (refused.length > 0) {
+      throw new Error(
+        `${operation}('${object}') does not accept ` +
+        `${refused.map((r) => `'${r.alias}'`).join(', ')}: ` +
+        refused
+          .map(
+            (r) =>
+              `'${r.alias}' is a wire spelling of '${r.canonical}', folded by the RPC/protocol ` +
+              `layer — a direct engine call bypasses that fold, so the value would be silently ` +
+              `dropped, not applied. Pass '${r.canonical}' ` +
+              `(${WIRE_ONLY_CANONICAL_SHAPES[r.canonical] ?? 'the canonical QueryAST shape'}) instead.`,
+          )
+          .join(' '),
+      );
+    }
+  }
   if (!slots.some((slot) => slot.aliases.some((alias) => alias in bag))) return bag;
   const folded: Record<string, unknown> = { ...bag };
   foldQueryAliasSlots(folded, slots, (conflict) => {
@@ -1169,8 +1236,13 @@ export class ObjectQL implements IDataEngine {
     execCtx?: ExecutionContextInput,
   ): Promise<number> {
     try {
+      // Canonical `fields`, not the wire spelling `select` — this call sat on
+      // the exact #4371 silent drop (the projection never applied; the scan
+      // worked only because an unprojected row still carries `field`), and the
+      // catch below would have swallowed the guard's rejection into "seed
+      // from 0", i.e. duplicate autonumbers.
       const rows = await this.find(object, {
-        select: ['id', field],
+        fields: ['id', field],
         limit: 5000,
         context: execCtx,
       } as any);
@@ -2919,8 +2991,11 @@ export class ObjectQL implements IDataEngine {
     // spec's slot table — the driver AST only understands the canonical keys,
     // so an unfolded `{ filter }` would match ALL rows (silent over-grant —
     // surfaced by ADR-0057's sharing/graph read path). Same fold, same
-    // conflict rule on every engine entry point (#4346).
-    query = foldEngineOptionAliases(object, 'find', query, ENGINE_QUERY_SLOTS);
+    // conflict rule on every engine entry point (#4346). The wire-only
+    // spellings (`sort`/`select`/`skip`/`populate`) are REJECTED instead of
+    // folded — a direct call carrying one used to have it silently dropped
+    // (#4371, three shipped instances in #4370).
+    query = foldEngineOptionAliases(object, 'find', query, ENGINE_QUERY_SLOTS, ENGINE_WIRE_ONLY_SLOTS);
     this.logger.debug('Find operation starting', { object, query });
     const driver = this.getDriver(object);
     const ast: QueryAST = { object, ...query };
@@ -3056,7 +3131,9 @@ export class ObjectQL implements IDataEngine {
     // matched the first row of the WHOLE table rather than the predicate.
     // `top` folds into `limit` here too, but findOne is single-row by
     // contract, so the literal `limit: 1` below wins over both spellings.
-    query = foldEngineOptionAliases(objectName, 'findOne', query, ENGINE_QUERY_SLOTS);
+    // Wire-only spellings are rejected, same as find() (#4371) — `sort`
+    // matters here too: findOne({ sort }) means "first row of THIS order".
+    query = foldEngineOptionAliases(objectName, 'findOne', query, ENGINE_QUERY_SLOTS, ENGINE_WIRE_ONLY_SLOTS);
     this.logger.debug('FindOne operation', { objectName });
     const driver = this.getDriver(objectName);
     const ast: QueryAST = { object: objectName, ...query, limit: 1 };