feat(lint): ban erasing engine query options to any, with a counted shrink-only baseline (#4918) (#5600)

`IDataEngine.find/findOne/count/aggregate` declare their options as
`EngineQueryOptions` / `EngineCountOptions` / `EngineAggregateOptions`, and
`IDataDriver` declares the same slots as `QueryAST` + `DriverOptions`. For an
INTERNAL caller `tsc` is the only channel enforcing those keys: the protocol's
ingress normalizer never runs on a direct engine call, and the options schemas
are not `.strict()`, so an unknown key is silently DROPPED rather than
rejected. One `as any` on the options argument switches that off for the call
site while looking identical to code that has it.

That is #4674: two internal queries spelled their sort
`{ field, direction: 'desc' }` where the QueryAST shape is `SortNodeSchema` =
`{ field, order }`. Both drivers normalize off `.order` with no fallback, so
both ran ASCENDING, and because both carried a `limit` the wrong direction
changed WHICH ROWS came back — audit history returned the oldest events and
global search the stalest matches. #4720 restored the two sites, #4721 closed
the external (REST/RPC) callers with a strict schema plus an ingress
normalizer; this is the third leg, and it stops the shape regrowing internally.

New rule `query-options/no-any-erasure` (eslint.config.mjs), three shapes:

- an `any` assertion at argument 1 or 2 of a query method — argument 0 is the
  object NAME on every one of these signatures, which is also what keeps
  `Array.prototype.find(cb)` out of the rule entirely;
- `orderBy: … as any`, which sits one level below the argument and so is
  invisible to the argument-position check;
- the split form (`const opts: any = { … }` … `find(o, opts)`) — the shape
  #4674's global-search site actually used. Scope analysis, not a name
  heuristic; needs no type information, so it stays in the untyped lint pass.

The assertion chain is walked, so `{ … } as any as EngineQueryOptions` — which
checks the literal against nothing and then re-labels it with the contract —
is caught too. `as unknown as EngineQueryOptions` is deliberately NOT matched:
it names the contract being bypassed, keeps the rest of the call checked, and
greps as an intentional act, so it is the sanctioned spelling for input that is
deliberately off-contract (a test asserting the engine REJECTS an option).

It is a dedicated plugin rule rather than three more `no-restricted-syntax`
selectors because flat config does not MERGE rule options: a second block
setting `no-restricted-syntax` over `packages/**` would REPLACE the
slot-lookup block's selector list for every file both match, silently deleting
that rule. The two guards also need independent `ignores`.

Residual re-measured on the branch point (the issue's numbers were taken at
89d2a4e, two days and ~40 merges earlier): 84 non-test sites in 19 files and
267 in test code, none of them a false positive. Both go into
`scripts/query-options-erasure-baseline.json` and neither is swept here — part
of the residual is a real type boundary (objectql's `hookContext.input.options`,
the metadata loader's query bag) that needs the boundary type written, not the
assertion deleted.

`scripts/check-query-options-erasure-ratchet.mjs` (`pnpm
check:query-options-erasure`, wired into lint.yml next to the slot-lookup
ratchet) is what makes the baseline mean something. Non-test files are
grandfathered by path, and an `ignores` entry silences the WHOLE file, so the
baseline carries per-file counts measured with the grandfathering lifted — a
new erasure in a listed file cannot ride the old entry. Test code is held by
one aggregate decrease-only number instead of per-file counts, because an
unknown share of those sites are legitimate and a per-file ratchet would go red
on a new rejection test with no honest remedy.


Claude-Session: https://claude.ai/code/session_01GX3sL71LFq8m2usg6VqTSE

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.github/workflows/lint.yml

@@ -76,6 +76,24 @@ jobs:
       - name: Slot-lookup ratchet
         run: pnpm check:slot-lookup
 
+      # Engine query-options erasure ratchet (#4918). The slot-lookup rule above
+      # protects the service LOOKUP; this one protects what you pass to the
+      # service you looked up. `IDataEngine.find/findOne/count/aggregate` declare
+      # their options as `EngineQueryOptions` & co., and for an internal caller
+      # `tsc` is the ONLY channel enforcing them — the protocol's ingress
+      # normalizer never sees a direct engine call, and the options schemas are
+      # not `.strict()`, so an unknown key is silently DROPPED. #4674 is the
+      # bill: two queries sorted by `direction` instead of `order`, both with a
+      # `limit`, so both returned the OLDEST rows — audit history that never
+      # showed recent changes, and a search that truncated away fresh records.
+      # #4720 restored those two sites and #4721 closed the external callers;
+      # this stops the shape regrowing internally. Same ratchet mechanism as
+      # slot-lookup for the non-test residual, plus one aggregate decrease-only
+      # number for test code (a test whose subject IS off-contract engine input
+      # must be able to build it). Runs its own --self-test first.
+      - name: Engine query-options erasure ratchet
+        run: pnpm check:query-options-erasure
+
       # Raw control-byte guard (#3127 / #4890 / #5157 / #5460). Scans every
       # tracked TEXT file for a raw ASCII control byte — 0x00-0x08, 0x0b, 0x0c,
       # 0x0e-0x1f and 0x7f, i.e. everything except tab/LF/CR. Two distinct

eslint.config.mjs

@@ -212,6 +212,198 @@ const slotLookupPlugin = {
   },
 };
 
+// ---------------------------------------------------------------------------
+// [#4918] Engine query-options `any`-erasure guard.
+//
+// The same failure as the slot-lookup rule above, one layer further in: the
+// contract exists, `tsc` is willing to enforce it, and one annotation switches
+// that off for the call site while looking identical to code that has it.
+//
+// `IDataEngine.find/findOne/count/aggregate` declare their options as
+// `EngineQueryOptions` / `EngineCountOptions` / `EngineAggregateOptions`
+// (`packages/spec/src/contracts/data-engine.ts`), and `IDataDriver` declares the
+// same slots as `QueryAST` + `DriverOptions`. For an INTERNAL caller `tsc` is
+// the ONLY enforced channel on that path: the protocol's ingress normalizer does
+// not run on calls the protocol itself makes to `this.engine.find`, and the
+// options schemas are not `.strict()`, so an unknown key is silently DROPPED
+// rather than rejected. Erase the type and a wrong key becomes a no-op that
+// nothing anywhere reports.
+//
+// #4674 is the bill: two internal queries spelled their sort
+// `{ field, direction: 'desc' }` — `IReportService`'s vocabulary — where the
+// QueryAST shape is `SortNodeSchema` = `{ field, order }`. Both drivers
+// normalize off `.order` with no fallback, so both queries ran ASCENDING, and
+// because both carried a `limit` the wrong direction changed WHICH ROWS came
+// back: metadata audit history returned the oldest events (never an object's
+// recent changes) and global search returned the stalest matches. `#4720`
+// restored those two sites, `#4721` closed the external (REST/RPC) callers with
+// a strict schema plus an ingress normalizer, and this rule is the third leg —
+// it stops the erasure regrowing on the internal side.
+const ENGINE_QUERY_READ_METHODS = ['find', 'findOne', 'count', 'aggregate'];
+
+// Exported so `scripts/check-query-options-erasure-ratchet.mjs` measures the
+// SAME surface this rule blocks. The ratchet lifts these to count the test-side
+// residual; the rule itself never runs on them.
+//
+// The first cut is deliberately non-test only (the 08-03 triage on #4918). Test
+// code holds the large majority of the erasures, and an unknown share of those
+// are legitimate: a test whose SUBJECT is off-contract engine input (see
+// `engine-unknown-option.test.ts`, `engine-wire-alias-reject.test.ts`) has to
+// erase the type to construct input `tsc` would otherwise refuse. A blocking
+// rule there would fight the tests that prove the contract is enforced, so the
+// test surface is held by a COUNT instead — see the ratchet.
+export const QUERY_OPTIONS_TEST_GLOBS = [
+  '**/*.test.{ts,tsx,mts,cts}',
+  '**/*.spec.{ts,tsx,mts,cts}',
+];
+
+// The rule's own id, exported so the ratchet identifies this rule's reports
+// exactly rather than by message text. (The slot-lookup ratchet matches on
+// message because that rule shares `no-restricted-syntax` with three others;
+// this one is a dedicated rule, so the id is available and is stricter.)
+export const QUERY_OPTIONS_RULE_ID = 'query-options/no-any-erasure';
+
+export const QUERY_OPTIONS_ANY_MESSAGE =
+  'Do not erase an engine query-options value to `any` — not as `find(obj, { … } ' +
+  'as any)`, not as an `orderBy: … as any`, and not as a `const opts: any` that is ' +
+  'then passed as the options argument. `EngineQueryOptions` (and `QueryAST` on the ' +
+  'driver side) already declare every key these methods read, and for an internal ' +
+  'caller `tsc` is the ONLY channel that enforces them: the protocol\'s ingress ' +
+  'normalizer does not run on a direct engine call, and the options schemas are not ' +
+  '`.strict()`, so an unknown key is silently DROPPED, never rejected. That is #4674 ' +
+  '— two queries sorted by `direction` (IReportService\'s vocabulary) instead of ' +
+  '`order` (SortNodeSchema\'s), both with a `limit`, so both quietly returned the ' +
+  'OLDEST rows: audit history that never showed an object\'s recent changes, and a ' +
+  'global search that truncated away the freshly-edited records. The declared type ' +
+  'would have rejected `direction` at the call site; the erasure is the only reason ' +
+  'it compiled. Type the value instead (`const opts: EngineQueryOptions = { … }`, or ' +
+  'just drop the assertion — these signatures already infer). If the value is ' +
+  'DELIBERATELY off-contract — a test asserting the engine REJECTS an unknown option ' +
+  '— write `as unknown as EngineQueryOptions`: that names the contract being ' +
+  'bypassed, keeps the rest of the call type-checked, and greps as an intentional ' +
+  'act, none of which a bare `as any` does. See issues #4674, #4720, #4721, #4918.';
+
+// [#4918] The unswept residual, grandfathered BY FILE from
+// `scripts/query-options-erasure-baseline.json` — same mechanism, and same
+// reasoning, as SLOT_LOOKUP_UNSWEPT above: `pnpm lint` runs with
+// `--no-inline-config`, so the escape has to live in config, and one shrinking
+// counted list is the ratchet made visible. An `ignores` entry silences the
+// WHOLE file, which is exactly why the baseline carries per-file COUNTS and
+// `pnpm check:query-options-erasure` enforces them.
+//
+// ⛔ Do NOT sweep these sites in the same PR that touches this rule. Part of the
+// residual is a real type boundary (`hookContext.input.options`, the metadata
+// loader's `Record<string, unknown>` query bag) and needs the boundary type
+// written, not the assertion deleted — a separate batch.
+const QUERY_OPTIONS_UNSWEPT = Object.keys(JSON.parse(
+  readFileSync(new URL('./scripts/query-options-erasure-baseline.json', import.meta.url), 'utf8'),
+).nonTest);
+
+const queryOptionsPlugin = {
+  rules: {
+    'no-any-erasure': {
+      meta: {
+        type: 'problem',
+        docs: { description: 'Ban erasing an engine query-options value to `any`.' },
+        schema: [],
+        messages: { erased: QUERY_OPTIONS_ANY_MESSAGE },
+      },
+      create(context) {
+        const methods = new Set(ENGINE_QUERY_READ_METHODS);
+
+        /**
+         * True when `node` is, or wraps, an `any` assertion.
+         *
+         * Walks the whole assertion chain rather than testing the outermost
+         * node, so `{ … } as any as EngineQueryOptions` is caught too: that
+         * spelling checks the literal against nothing and then re-labels the
+         * result with the contract, which erases the keys exactly as `as any`
+         * does while reading as if it were typed. `as unknown as X` is NOT
+         * matched, on purpose — see the message.
+         */
+        const erasesToAny = (node) => {
+          for (let cur = node; cur; cur = cur.expression) {
+            if (cur.type === 'TSAsExpression' || cur.type === 'TSTypeAssertion') {
+              if (cur.typeAnnotation?.type === 'TSAnyKeyword') return true;
+              continue;
+            }
+            if (cur.type === 'TSNonNullExpression') continue;
+            return false;
+          }
+          return false;
+        };
+
+        /**
+         * True when `name` resolves, in scope, to a local VARIABLE declared
+         * `: any` — the split form (`const opts: any = { … }` … `find(o, opts)`)
+         * that #4674's global-search site actually used.
+         *
+         * Scope analysis, not a name heuristic: a rule keyed on the identifier's
+         * spelling would flag every `const options: any` in the repo whether or
+         * not it ever reaches a query, and miss the ones spelled anything else.
+         * Deliberately restricted to variable declarations — an `: any`
+         * PARAMETER forwarded into a query is a different (and much larger,
+         * mostly test-double) population, out of this cut's scope.
+         */
+        const declaredAnyVariable = (name, node) => {
+          for (let scope = context.sourceCode.getScope(node); scope; scope = scope.upper) {
+            const variable = scope.variables.find((v) => v.name === name);
+            if (!variable) continue;
+            return variable.defs.some(
+              (d) =>
+                d.node?.type === 'VariableDeclarator' &&
+                d.node.id?.typeAnnotation?.typeAnnotation?.type === 'TSAnyKeyword',
+            );
+          }
+          return false;
+        };
+
+        return {
+          CallExpression(node) {
+            if (node.callee?.type !== 'MemberExpression') return;
+            const property = node.callee.property;
+            if (property?.type !== 'Identifier' || !methods.has(property.name)) return;
+
+            node.arguments.forEach((argument, index) => {
+              // Argument 0 is the object/table NAME on every one of these
+              // signatures; the options bags are 1 (the query) and 2
+              // (`BaseEngineOptions` / `DriverOptions`). Starting at 1 is also
+              // what keeps `Array.prototype.find(cb)` — same method name,
+              // callback at index 0 — out of the rule entirely.
+              if (index < 1 || !argument) return;
+              if (erasesToAny(argument)) {
+                context.report({ node: argument, messageId: 'erased' });
+                return;
+              }
+              if (argument.type === 'Identifier' && declaredAnyVariable(argument.name, node)) {
+                context.report({ node: argument, messageId: 'erased' });
+              }
+            });
+          },
+
+          // `orderBy` is scoped in by name because it is the key #4674 was
+          // actually wrong about, and it is erased one level below the argument
+          // — `...(ast.orderBy ? { orderBy: ast.orderBy as any } : {})` sits
+          // inside an otherwise-typed options literal, so the argument-position
+          // check above cannot see it. `SortNodeSchema` is the shape everywhere
+          // this key appears.
+          Property(node) {
+            if (node.computed) return;
+            const key = node.key;
+            const isOrderBy =
+              (key?.type === 'Identifier' && key.name === 'orderBy') ||
+              (key?.type === 'Literal' && key.value === 'orderBy');
+            if (!isOrderBy) return;
+            if (erasesToAny(node.value)) {
+              context.report({ node: node.value, messageId: 'erased' });
+            }
+          },
+        };
+      },
+    },
+  },
+};
+
 export default [
   {
     files: ['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}'],
@@ -393,4 +585,48 @@ export default [
       ],
     },
   },
+  // issue #4918 — engine query-options `any`-erasure guard. Rationale and the
+  // #4674 cost are on `QUERY_OPTIONS_ANY_MESSAGE` above.
+  //
+  // This is a dedicated PLUGIN rule and not three more `no-restricted-syntax`
+  // selectors, for two reasons that both matter:
+  //
+  //   1. Flat config does not MERGE rule options. A second block setting
+  //      `no-restricted-syntax` over `packages/**` would REPLACE the
+  //      slot-lookup block's selector list for every file both blocks match —
+  //      silently deleting that rule. The two guards also need independent
+  //      `ignores` (their unswept sets are different files), which one shared
+  //      block cannot give them.
+  //   2. The split form needs SCOPE analysis to resolve an identifier to its
+  //      declaration, which esquery cannot express — the same reason
+  //      `slot-lookup/no-any-assignment` exists. Scope analysis needs no type
+  //      information, so this still runs in the plain (untyped) lint pass.
+  //
+  // KNOWN RESIDUAL, stated rather than implied: an erasure that happens through
+  // a typed indirection — a helper declared `(o, q?: any) => engine.find(o, q)`,
+  // or a wrapper whose own return type is `Promise<any>` — erases the contract
+  // just as effectively and this rule cannot see it (an `: any` PARAMETER
+  // forwarded into a query is a real shape, ~50 sites, almost all of them test
+  // doubles; judging it needs the call graph, not one file's scopes). Same
+  // boundary as the slot-lookup rule's own KNOWN RESIDUAL, and the same answer:
+  // it belongs to a typed-lint pass, not here.
+  {
+    files: ['packages/**/*.{ts,tsx,mts,cts}'],
+    ignores: [
+      '**/node_modules/**',
+      '**/dist/**',
+      // First cut is non-test code (08-03 triage). The ratchet lifts this and
+      // holds the test residual to a count instead.
+      ...QUERY_OPTIONS_TEST_GLOBS,
+      // Pre-existing sites, grandfathered by file and counted — see
+      // QUERY_OPTIONS_UNSWEPT and `pnpm check:query-options-erasure`.
+      ...QUERY_OPTIONS_UNSWEPT,
+    ],
+    languageOptions: {
+      parser: tsParser,
+      parserOptions: { ecmaVersion: 'latest', sourceType: 'module' },
+    },
+    plugins: { 'query-options': queryOptionsPlugin },
+    rules: { 'query-options/no-any-erasure': 'error' },
+  },
 ];

package.json

@@ -40,6 +40,7 @@
     "check:org-identifier": "node scripts/check-org-identifier.mjs",
     "check:authz-resolver": "node scripts/check-single-authz-resolver.mjs --self-test && node scripts/check-single-authz-resolver.mjs",
     "check:slot-lookup": "node scripts/check-slot-lookup-ratchet.mjs",
+    "check:query-options-erasure": "node scripts/check-query-options-erasure-ratchet.mjs --self-test && node scripts/check-query-options-erasure-ratchet.mjs",
     "check:service-providers": "node scripts/check-service-providers.mjs",
     "check:route-envelope": "node scripts/check-route-envelope.mjs --self-test && node scripts/check-route-envelope.mjs",
     "check:error-code-casing": "node scripts/check-error-code-casing.mjs --self-test && node scripts/check-error-code-casing.mjs",