fix(metadata-protocol): sort audit history and global search by order, not direction (#4674)

.changeset/orderby-direction-vocabulary.md

@@ -0,0 +1,14 @@
+---
+"@objectstack/metadata-protocol": patch
+---
+
+fix(metadata-protocol): 元数据审计历史与全局搜索按 `order` 排序,不再按 `direction` (#4674)
+
+`protocol.ts` 里两处内部 `engine.find` 调用把排序写成 `{ field, direction: 'desc' }`。QueryAST 的排序形状是 `SortNodeSchema` = `{ field, order }`,两个真实驱动都只认 `.order` 且没有 `direction` 回退——`undefined === 'desc'` 为假,于是两个查询实际都在**升序**运行。`direction` 是 `IReportService` 的词汇,是另一份契约,这正是错误拼写看起来合理的原因。
+
+由于两个查询都带 `limit`,方向错误不只是把一页重排,而是**改变了哪些行会被返回**:
+
+- **元数据审计历史**取到的是最旧的 `limit` 条事件——一个对象生命的开头,而永远不是它最近的变更。在长期存在的对象上,编辑者要找的东西一条也看不到。
+- **全局搜索**取到的是最陈旧的 `perObject` 条匹配,最近编辑过的记录恰好被 `limit` 截断掉——而那正是搜索者最可能想要的。
+
+两处的 `as any` / `: any` 一并去掉:`EngineQueryOptions.orderBy` 是 `SortNodeSchema[]`,本来就会拒绝 `direction`,而类型擦除正是让它溜过去的原因。恢复类型是这次改动价值的大头,因为对内部调用方来说 `tsc` 就是那条被执行的渠道。

packages/metadata-protocol/src/protocol.orderby-vocabulary.test.ts

@@ -0,0 +1,132 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// Two internal `engine.find` calls sorted by `direction`, a key nothing on that
+// path reads (#4674).
+//
+// The QueryAST sort shape is `SortNodeSchema` = `{ field, order }`
+// (`packages/spec/src/data/query.zod.ts`), and both real drivers normalize off
+// `.order` with no fallback — `sql-driver` maps `item.order === 'desc'`,
+// `mongodb-driver` the same. With `order` absent, `undefined === 'desc'` is
+// false and both land on ASCENDING. `direction` is `IReportService`'s
+// vocabulary; it is a genuinely different contract, which is how the wrong
+// spelling looked plausible.
+//
+// Because both queries carry a `limit`, the wrong direction did not merely
+// reorder a page — it changed WHICH ROWS CAME BACK. So these tests assert on
+// identity, not sequence: with a limit smaller than the fixture, sorting the
+// wrong way returns a disjoint set. An order-only assertion would have passed
+// against a fake that ignored `orderBy` entirely.
+//
+// Nothing caught this because both sites erased their types (`} as any)` and
+// `const opts: any`), the protocol's `INVALID_SORT` normalizer does not run on
+// calls the protocol makes to `this.engine.find` directly, and that normalizer
+// rejects bad VALUES rather than unknown KEYS — the schema is not `.strict()`,
+// so `direction` was dropped rather than flagged.
+
+import { describe, it, expect, vi } from 'vitest';
+import { ObjectStackProtocolImplementation } from './protocol.js';
+
+/**
+ * A `find` that honours the QueryAST contract for sort and limit, and nothing
+ * else. Filtering is deliberately not implemented: what is under test is which
+ * rows survive `orderBy` + `limit`, and a double that also filtered would let a
+ * sort bug hide behind a `where` that happened to select the right rows.
+ *
+ * It reads `order` — the shape the drivers read. A double that read `direction`
+ * would agree with the bug instead of catching it, which is exactly what the
+ * publish-rollback double did until this change.
+ */
+function makeFind(rowsByObject: Record<string, any[]>) {
+ return vi.fn(async (object: string, opts: any = {}) => {
+ const rows = [...(rowsByObject[object] ?? [])];
+ for (const { field, order } of [...(opts.orderBy ?? [])].reverse()) {
+ rows.sort((a, b) => {
+ const av = a[field], bv = b[field];
+ if (av === bv) return 0;
+ return (av < bv ? -1 : 1) * (order === 'desc' ? -1 : 1);
+ });
+ }
+ return typeof opts.limit === 'number' ? rows.slice(0, opts.limit) : rows;
+ });
+}
+
+/** The options the protocol handed to `engine.find` on its first call. */
+const optionsFrom = (find: any) => find.mock.calls[0][1];
+
+const AUDIT_ROWS = ['2024-01-01', '2024-02-01', '2024-03-01', '2024-04-01', '2024-05-01'].map(
+ (d, i) => ({
+ id: `a${i + 1}`,
+ occurred_at: `${d}T00:00:00.000Z`,
+ actor: 'someone',
+ operation: 'save',
+ outcome: 'allowed',
+ code: 'OK',
+ }),
+);
+
+describe('auditMetaItem sorts newest-first (#4674)', () => {
+ function makeProtocol() {
+ const find = makeFind({ sys_metadata_audit: AUDIT_ROWS });
+ const engine = { registry: { getObject: () => undefined }, find };
+ return { p: new ObjectStackProtocolImplementation(engine as any), find };
+ }
+
+ it('returns the NEWEST `limit` events, not the oldest', async () => {
+ const { p } = makeProtocol();
+ const { events } = await p.auditMetaItem({ type: 'objects', name: 'invoice', limit: 2 });
+
+ // The whole defect in one assertion: ascending returns a1/a2 here.
+ expect(events.map(e => e.id)).toEqual(['a5', 'a4']);
+ });
+
+ it('asks for `order`, never `direction`', async () => {
+ const { p, find } = makeProtocol();
+ await p.auditMetaItem({ type: 'objects', name: 'invoice', limit: 2 });
+
+ const sort = optionsFrom(find).orderBy;
+ expect(sort).toEqual([{ field: 'occurred_at', order: 'desc' }]);
+ // Named explicitly: `direction` reads as a well-formed "sort by
+ // occurred_at, direction unspecified" and passes every existing check.
+ expect(sort[0]).not.toHaveProperty('direction');
+ });
+});
+
+const SEARCH_ROWS = ['2024-01-01', '2024-02-01', '2024-03-01', '2024-04-01'].map((d, i) => ({
+ id: `c${i + 1}`,
+ name: `Acme ${i + 1}`,
+ updated_at: `${d}T00:00:00.000Z`,
+}));
+
+const CONTACT = {
+ name: 'contact',
+ fields: { name: { name: 'name', type: 'text', searchable: true } },
+};
+
+describe('searchAll sorts newest-first (#4674)', () => {
+ function makeProtocol() {
+ const find = makeFind({ contact: SEARCH_ROWS });
+ const engine = {
+ registry: { getObject: (n: string) => (n === 'contact' ? CONTACT : undefined), getAllObjects: () => [CONTACT] },
+ find,
+ };
+ return { p: new ObjectStackProtocolImplementation(engine as any), find };
+ }
+
+ it('returns the most recently updated matches, not the stalest', async () => {
+ const { p } = makeProtocol();
+ const { hits } = await p.searchAll({ q: 'Acme', perObject: 2 });
+
+ // Ascending returned c1/c2 — the stalest rows, with the recently-edited
+ // ones truncated away by `perObject`.
+ expect(hits.map(h => h.id)).toEqual(['c4', 'c3']);
+ });
+
+ it('asks for `order`, never `direction`', async () => {
+ const { p, find } = makeProtocol();
+ await p.searchAll({ q: 'Acme', perObject: 2 });
+
+ const sort = optionsFrom(find).orderBy;
+ expect(sort).toEqual([{ field: 'updated_at', order: 'desc' }]);
+ expect(sort[0]).not.toHaveProperty('direction');
+ });
+});

packages/metadata-protocol/src/protocol.ts

@@ -24,7 +24,7 @@ import {
  SEARCHABLE_TEXTUAL_TYPES, SEARCHABLE_ENUM_TYPES, SEARCH_AUTO_EXCLUDED_FIELDS,
  RPC_QUERY_ALIAS_SLOTS, foldQueryAliasSlots,
  type QueryAliasConflict, type QueryAliasSlot,
- type DroppedFieldsEvent, type QueryAST,
+ type DroppedFieldsEvent, type QueryAST, type EngineQueryOptions,
 } from '@objectstack/spec/data';
 import { PLURAL_TO_SINGULAR, SINGULAR_TO_PLURAL } from '@objectstack/spec/shared';
 import { applyConversionsToStoredItem, type ConversionNotice } from '@objectstack/spec';
@@ -3311,11 +3311,20 @@ export class ObjectStackProtocolImplementation implements
  type: singular,
  name: request.name,
  };
+ // `order`, NOT `direction`: the QueryAST sort shape is
+ // `SortNodeSchema` = `{ field, order }`, and both drivers normalize
+ // off `.order` with no fallback. `direction` is `IReportService`'s
+ // vocabulary and is silently DROPPED here (the schema is not
+ // `.strict()`), which left this query running ascending — the
+ // OLDEST `limit` audit events, i.e. the beginning of an object's
+ // life and never its recent changes (#4674). The `as any` is gone
+ // for the same reason: `EngineQueryOptions` rejects the wrong key,
+ // and erasing the type is what let it through.
  const rows = await this.engine.find('sys_metadata_audit', {
  where,
- orderBy: [{ field: 'occurred_at', direction: 'desc' }],
+ orderBy: [{ field: 'occurred_at', order: 'desc' }],
  limit,
- } as any);
+ });
  const events = (Array.isArray(rows) ? rows : []).map((r: any) => ({
  id: r.id,
  occurredAt:
@@ -5098,10 +5107,15 @@ export class ObjectStackProtocolImplementation implements
  const where = andClauses.length === 1 ? andClauses[0] : { $and: andClauses };
 
  try {
- const opts: any = {
+ // `order`, NOT `direction` — see the audit-history query above.
+ // Ascending here returned the STALEST `perObject` matches and
+ // truncated away the recently-edited records a searcher is most
+ // likely to want (#4674). Typed rather than `any` so the
+ // contract rejects the wrong key at the call site.
+ const opts: EngineQueryOptions = {
  where,
  limit: perObject,
- orderBy: [{ field: 'updated_at', direction: 'desc' }],
+ orderBy: [{ field: 'updated_at', order: 'desc' }],
  };
  if (request.context !== undefined) opts.context = request.context;
 

packages/objectql/src/protocol-publish-rollback.test.ts

@@ -82,12 +82,21 @@ function makeStubEngine() {
  async find(table: string, opts: { where: Record<string, unknown> }) {
  if (table === 'sys_metadata_history') {
  const out = historyRows.filter((h) => matchesHistory(h, opts.where));
- if (opts && (opts as any).orderBy) {
- const { field, direction } = (opts as any).orderBy;
+ // QueryAST shape: `orderBy` is an ARRAY of `{ field, order }`
+ // (SortNodeSchema). This double used to destructure
+ // `{ field, direction }` off the array itself, so both names
+ // read `undefined` — it spoke the `direction` vocabulary the
+ // engine does not read (#4674) AND, because an array has no
+ // `.field`, sorted nothing at all. Either way a test built on
+ // it would have ratified the broken behaviour.
+ const orderBy = (opts as any).orderBy;
+ if (Array.isArray(orderBy) && orderBy.length > 0) {
  out.sort((a: any, b: any) => {
- const av = a[field]; const bv = b[field];
- if (av < bv) return direction === 'desc' ? 1 : -1;
- if (av > bv) return direction === 'desc' ? -1 : 1;
+ for (const { field, order } of orderBy) {
+ const av = a[field]; const bv = b[field];
+ if (av < bv) return order === 'desc' ? 1 : -1;
+ if (av > bv) return order === 'desc' ? -1 : 1;
+ }
  return 0;
  });
  }