fix(app-shell,plugin-dashboard,console): send query options with their $, and gate the shape

.changeset/5458-unprefixed-query-params.md

@@ -0,0 +1,39 @@
+---
+'@object-ui/app-shell': patch
+'@object-ui/plugin-dashboard': patch
+'@object-ui/console': patch
+---
+
+Fix four `find()` calls that passed a query option without its `$`, and gate the shape.
+
+`QueryParams` declares every query option `$`-prefixed and `convertQueryParams` copies
+exactly those keys, so an unprefixed spelling reaches no branch and is dropped — no throw,
+no warning, and it type-checks because the type carries `[key: string]: any` for
+adapter-specific params. For a dropped cap the result is an **unbounded** read rather than
+a truncated one: the platform's GET list route has no default page size, so the query
+returns the whole match set and stays invisible until the object is large.
+
+- `app-shell` `ObjectView` fetched the footer's record count with `{ limit: 0 }`. This one
+  **inverted** rather than widened — `$top: 0` is honoured end to end as "no records", so
+  the dropped key turned "count only, fetch nothing" into "fetch every row in the object",
+  on every mount and every refresh of every list view. It now sends `$top: 0` and reads
+  the count off `total` only; the row-counting fallbacks are gone rather than repointed,
+  because once zero rows are requested an empty `data` means "you asked for none", not
+  "the object is empty", and counting it would assert a confident `0`. With no total the
+  footer line is omitted instead.
+- `app-shell` `AssignedUsersSection` looked a permission set up with `{ …, limit: 1 }`,
+  one line from three correct `$top` calls.
+- `plugin-dashboard` `DashboardFilterBar` passed `fields` **and** `top` in one literal, so
+  a filter's option list read every row and every column of its source object while its
+  own comment described it as capped at 200. The same call read `records.items`, which is
+  not a `QueryResult` member, so against a real adapter the fallback produced no options
+  at all.
+- `console` `sdui-workbench-preview` passed `{ top: 200 }` and read `.records` off the
+  result in its page-source metadata.
+
+A new `object-ui/no-unprefixed-query-params` ESLint rule rejects the shape at write time:
+a known query-option name missing its `$` in the second argument of a `find`/`findOne`
+call. It is narrow on purpose — a closed list of spellings, anchored to the call — because
+the index signature exists so adapters can take adapter-specific params, and a rule that
+flagged any unprefixed key would report the shape the type was written to allow. Its
+sibling `no-query-params-under-options` (the `{ options: { $top } }` half) is unchanged.

apps/console/src/sdui-workbench-preview.tsx

@@ -83,8 +83,8 @@ function Page() {
   const [stats, setStats] = React.useState({ total: 0, active: 0 });
   const refreshStats = React.useCallback(async () => {
     if (!adapter) return;
-    const all = await adapter.find('showcase_project', { top: 200 });
-    const rows = Array.isArray(all) ? all : (all && all.records) || [];
+    const all = await adapter.find('showcase_project', { $top: 200 });
+    const rows = Array.isArray(all) ? all : (all && all.data) || [];
     setStats({ total: rows.length, active: rows.filter((r) => r.status === 'active').length });
   }, [adapter]);
   React.useEffect(() => { refreshStats(); }, [refreshStats, reloadKey]);

eslint-rules/index.js

@@ -6,6 +6,7 @@ import noInlineSpecConfig from './no-inline-spec-config.js';
 import noTryCatchAroundHook from './no-try-catch-around-hook.js';
 import noDynamicImportInTestHook from './no-dynamic-import-in-test-hook.js';
 import noQueryParamsUnderOptions from './no-query-params-under-options.js';
+import noUnprefixedQueryParams from './no-unprefixed-query-params.js';
 import buttonHasType from './button-has-type.js';
 import noUnpairedBadgeColorClasses from './no-unpaired-badge-color-classes.js';
 
@@ -16,6 +17,7 @@ export default {
     'no-try-catch-around-hook': noTryCatchAroundHook,
     'no-dynamic-import-in-test-hook': noDynamicImportInTestHook,
     'no-query-params-under-options': noQueryParamsUnderOptions,
+    'no-unprefixed-query-params': noUnprefixedQueryParams,
     'button-has-type': buttonHasType,
     'no-unpaired-badge-color-classes': noUnpairedBadgeColorClasses,
   },

eslint-rules/no-unprefixed-query-params.js

@@ -0,0 +1,221 @@
+/**
+ * ObjectUI ESLint rule: no-unprefixed-query-params
+ *
+ * Sibling of `no-query-params-under-options`, for the other half of the same
+ * defect class: a query option written WITHOUT its `$`, at the TOP level of a
+ * `find`/`findOne` params object — `find(obj, { top: 200 })` instead of
+ * `find(obj, { $top: 200 })`.
+ *
+ * `QueryParams` (`packages/types/src/data.ts`) declares every query option with
+ * a leading `$` — `$select`, `$filter`, `$orderby`, `$skip`, `$top`, `$expand`,
+ * `$search`, `$searchFields`, `$count` — and `convertQueryParams` in
+ * `@object-ui/data-objectstack` builds its outgoing options by copying exactly
+ * those. An unprefixed key reaches no branch and is dropped: no throw, no
+ * warning. `QueryParams` also carries `[key: string]: any`, so the type system
+ * accepts both spellings equally and nothing rejects the dead one.
+ *
+ * The consequence for a dropped cap is an UNBOUNDED read, not a truncated one.
+ * The platform's GET list route has no default page size — the pinned
+ * `@objectstack/client` serializes `top` only when the caller supplied it
+ * (`if (normalizedOptions.top != null)`) — so an absent `top` returns the whole
+ * match set. The symptom is therefore invisible until the object is large, and
+ * when it appears it reads as a data problem rather than a code problem.
+ *
+ * Three live sites existed when this rule landed (objectui#5458):
+ *
+ *  - `app-shell` `ObjectView.tsx` — `find(name, { limit: 0 })` for the footer's
+ *    record count. `$top: 0` is honoured end to end as "no records", so the
+ *    dropped key did not merely widen the read, it INVERTED the call: "count
+ *    only, fetch nothing" became "fetch every row in the object", on every
+ *    mount and every refresh of every list view.
+ *  - `app-shell` `metadata-admin/AssignedUsersSection.tsx` — `{ $filter: {…},
+ *    limit: 1 }`, one line away from three correct calls (`$top: 500`, `$top:
+ *    200`, `$top: 1000`). Half-correct spelling in a single object literal is
+ *    the shape a reviewer's eye slides over, which is the argument for
+ *    mechanising this rather than reviewing it.
+ *  - `apps/console` `sdui-workbench-preview.tsx` — `{ top: 200 }`, inside a
+ *    template literal holding runtime page metadata. See the boundary note
+ *    below: this rule does NOT catch that one, and cannot.
+ *
+ * A FOURTH site, which the card that commissioned this rule never named, was
+ * found by the rule itself on its first repo-wide run: `plugin-dashboard`
+ * `DashboardFilterBar.tsx` passed `fields` AND `top` in the same literal, so a
+ * dashboard filter's option list read every row and every column of its source
+ * object while its own comment described it as capped at 200. That is the
+ * argument for the rule in one site: two dropped keys, in one call, under a
+ * comment asserting the opposite.
+ *
+ * ## Scope, deliberately narrow so it discriminates
+ *
+ * Two independent narrowings, both load-bearing:
+ *
+ *  1. **Only a KNOWN query-option name.** Not "any unprefixed key". The index
+ *     signature exists because adapters legitimately take adapter-specific
+ *     params, so flagging every unprefixed key would report the shape the type
+ *     was written to allow — and a rule that cries wolf gets switched off. The
+ *     list below is closed and every entry maps to a real `QueryParams` key.
+ *  2. **Only the second argument of a `find`/`findOne` CALL.** Unlike its
+ *     sibling — whose `options`-holding-a-`$`-key signature is unmistakable
+ *     anywhere — every name on the list (`top`, `limit`, `filter`, `sort`,
+ *     `select`, `count`, …) is a perfectly ordinary object key elsewhere in
+ *     this repo. Outside a finder call the name carries no signal at all, so
+ *     the call is what makes the report meaningful.
+ *
+ * `Array.prototype.find` is a `.find(` member call too. It is excluded by its
+ * own signature rather than by naming: its first argument is a predicate, so a
+ * call whose first argument is a function literal is skipped. The key list is a
+ * second, independent filter — an array `find`'s optional second argument is a
+ * `thisArg`, which is not an object literal carrying `top`/`limit`.
+ *
+ * Only STATIC key names report — an identifier or a string literal. A computed
+ * key (`{ [k]: v }`) cannot be judged without running the code and a spread
+ * (`{ ...params }`) hides its keys, so neither says anything. Same rule as the
+ * sibling, same reason: null always means "say nothing".
+ *
+ * ## Known boundaries, recorded rather than silently omitted
+ *
+ *  - **Source inside a string is invisible to it.** `sdui-workbench-preview.tsx`
+ *    holds its page source in a template literal, which the parser sees as one
+ *    `TemplateLiteral` token and never as a `CallExpression`. No AST rule can
+ *    reach it. That site was fixed by hand for objectui#5458; a text scan is
+ *    the only mechanism that would gate it, and a text scan over this key list
+ *    would match the prose in this very file.
+ *  - **Only these spellings.** `orderBy` (camelCase) and `pageSize` are the
+ *    same mistake in spirit and are deliberately NOT listed: no site has ever
+ *    used them here, and the argument for this rule is that it discriminates.
+ *    Add one when a real instance appears, with the instance cited — which is
+ *    exactly how `fields` got on the list. It is not a `$`-less `$fields` (no
+ *    such key exists); it is an alias for `$select`, the same kind of entry as
+ *    `limit` -> `$top` and `offset` -> `$skip`, and it earned its place on a
+ *    live site: `plugin-dashboard` `DashboardFilterBar.tsx` passed `fields` and
+ *    `top` in ONE literal, so the filter-option query fetched every row and
+ *    every column of the source object.
+ *
+ * No autofixer. `limit` and `offset` do not rename to `$limit`/`$offset` — they
+ * rename to `$top`/`$skip` — and `filters` collapses to `$filter`, so a fix
+ * would have to merge into a key that may already be present in the same
+ * literal. More importantly `ObjectView`'s `limit: 0` needed a judgement about
+ * what the call MEANT before it could be rewritten; a fixer would have made
+ * that silently. objectui#5458.
+ *
+ * @type {import('eslint').Rule.RuleModule}
+ */
+
+/**
+ * Unprefixed spellings that are really query options, mapped to the
+ * `QueryParams` key each one means. Closed on purpose — see the scope note.
+ */
+const QUERY_OPTION_SPELLINGS = {
+  top: '$top',
+  limit: '$top',
+  skip: '$skip',
+  offset: '$skip',
+  filter: '$filter',
+  filters: '$filter',
+  select: '$select',
+  fields: '$select',
+  orderby: '$orderby',
+  sort: '$orderby',
+  expand: '$expand',
+  search: '$search',
+  count: '$count',
+};
+
+/** Methods whose second argument is a `QueryParams`. */
+const FINDER_METHODS = new Set(['find', 'findOne']);
+
+/**
+ * Static name of a property key, or null when it cannot be read off the AST
+ * (computed keys, spreads). Null always means "say nothing".
+ */
+function staticKeyName(node) {
+  if (node.type === 'SpreadElement' || node.type === 'ExperimentalSpreadProperty') return null;
+  if (node.computed) return null;
+  const key = node.key;
+  if (key.type === 'Identifier') return key.name;
+  if (key.type === 'Literal' && typeof key.value === 'string') return key.value;
+  return null;
+}
+
+/**
+ * Static method name of a member call (`a.find`, `a?.find`, `a['find']`), or
+ * null for anything whose callee cannot be read off the AST.
+ */
+function calleeMethodName(callee) {
+  if (!callee || callee.type !== 'MemberExpression') return null;
+  const property = callee.property;
+  if (callee.computed) {
+    return property.type === 'Literal' && typeof property.value === 'string'
+      ? property.value
+      : null;
+  }
+  return property.type === 'Identifier' ? property.name : null;
+}
+
+/**
+ * Look through the TypeScript wrappers that do not change the value, so a
+ * params literal written `{ limit: 1 } as QueryParams` is still read as the
+ * object literal it is. The index signature makes that cast compile, which is
+ * exactly the population this rule exists for — an evasion by `as` would be
+ * silent and would look deliberate.
+ */
+function unwrapExpression(node) {
+  let current = node;
+  while (
+    current
+    && (current.type === 'TSAsExpression'
+      || current.type === 'TSSatisfiesExpression'
+      || current.type === 'TSNonNullExpression'
+      || current.type === 'TSTypeAssertion')
+  ) {
+    current = current.expression;
+  }
+  return current;
+}
+
+/** `arr.find(predicate)` — an array search, not a data read. */
+function firstArgumentIsPredicate(node) {
+  const first = unwrapExpression(node.arguments[0]);
+  if (!first) return false;
+  return first.type === 'ArrowFunctionExpression' || first.type === 'FunctionExpression';
+}
+
+export default {
+  meta: {
+    type: 'problem',
+    docs: {
+      description:
+        'Disallow a query option spelled without its `$` in the params of a `find`/`findOne` call — `QueryParams` declares them all `$`-prefixed and `convertQueryParams` copies exactly those, so the unprefixed key is dropped and the query silently runs unbounded (objectui#5458).',
+      recommended: true,
+    },
+    schema: [],
+    messages: {
+      unprefixedQueryOption:
+        '`{{key}}` is not a `QueryParams` key — write `{{canonical}}`. `QueryParams` (@object-ui/types) declares every query option with a leading `$`, and `convertQueryParams` (@object-ui/data-objectstack) copies exactly those keys, so `{{key}}` reaches no branch and is dropped: no throw, no warning. Its `[key: string]: any` index signature exists for adapter-specific params, which is why the dead spelling type-checks. When the dropped key is a cap the read becomes UNBOUNDED rather than truncated — the platform GET list route has no default page size, so the query returns the whole match set and stays invisible until the object is large. See objectui#5458.',
+    },
+  },
+  create(context) {
+    return {
+      CallExpression(node) {
+        if (!FINDER_METHODS.has(calleeMethodName(node.callee))) return;
+        if (firstArgumentIsPredicate(node)) return;
+
+        const params = unwrapExpression(node.arguments[1]);
+        if (!params || params.type !== 'ObjectExpression') return;
+
+        for (const property of params.properties) {
+          const key = staticKeyName(property);
+          if (key === null) continue;
+          const canonical = QUERY_OPTION_SPELLINGS[key];
+          if (!canonical) continue;
+
+          context.report({
+            node: property.key,
+            messageId: 'unprefixedQueryOption',
+            data: { key, canonical },
+          });
+        }
+      },
+    };
+  },
+};