fix(plugin-auth): read the canonical QueryAST in the admin-import protocol

`runImport` now sends `where` / `limit`, and it takes an INJECTED
`ImportProtocolLike`. The wire-alias folding the runner's rewrite relied on
lives in `ObjectStackProtocolImplementation`; a hand-written protocol never
passes through it, so `admin-import-users.ts` read `args.query.$filter` and
got `undefined`.

The failure mode is not a missing filter but an unbounded one: `?? {}` turns
the unread key into an empty filter, the upsert duplicate probe stops
discriminating, and an admin import updates the wrong user. Both halves were
red on this branch (`admin-import-users.test.ts:560` and `:592`).

- The adapter reads `args.query.where` / `args.query.limit`, with no `??`
  behind either. One dialect, and an absent `query` is a loud TypeError
  rather than a silent match-everything.
- The three `import-runner` test doubles read `where` too. Two were red
  (`import-runner-selfref.test.ts`, `import-runner-bulk.test.ts`); the third
  was GREEN FOR THE WRONG REASON — its degraded `{}` matched the whole store,
  so every no-duplicate assertion held without the probe discriminating.
- `import-runner-idempotency.test.ts` gains the assertion that closes that:
  every probe must narrow, and the id recheck is pinned to the
  `id: { $in: [...] }` over exactly the pre-assigned ids, bounded by `limit`.

`ImportProtocolLike.findData(args: any)` is deliberately untouched — narrowing
a published extension point is a contract decision and has its own card.

Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
Co-authored-by: Claude <noreply@anthropic.com>

Author: claude

.changeset/plugin-auth-admin-import-canonical-query.md

@@ -0,0 +1,12 @@
+---
+"@objectstack/plugin-auth": patch
+---
+
+`runAdminImportUsers`'s hand-written `ImportProtocolLike` reads the CANONICAL QueryAST (`where` / `limit`) — the payload `@objectstack/rest`'s import runner sends as of this same release — instead of the wire-only `$filter` / `$top`.
+
+`POST /api/v1/auth/admin/import-users` reuses the shared import runner but swaps in an identity-specific protocol, because an identity write is `auth.api.createUser` and not an engine insert. That protocol is hand-written, so it never passes through `ObjectStackProtocolImplementation` — the normalizer that folds `$filter` onto `where` and `$top` onto `limit` for a caller arriving off the HTTP door. It has to read the canonical keys itself.
+
+- **A mismatch here does not produce a missing filter, it produces an unbounded one.** `const where = args?.query?.$filter ?? {}` turns an unread key into an empty filter, and an empty filter constrains nothing: the upsert duplicate probe stops discriminating, `findExisting` matches rows it was given no key for, and an admin import updates the WRONG user. Both halves are measured in `admin-import-users.test.ts` — the email-match case reported `updated: 2` where one of the two rows was new, and the phone-match case sent a probe carrying no `where` at all.
+- **One dialect, and no default behind it.** The two reads are now `args.query.where` and `args.query.limit`, with no `??`. A default here would not be tolerance for an older caller — this handle is fed by the runner, never off the wire — it is precisely the lenient fallback that converts a spelling mismatch into a silent match-everything. A request that arrives without a `query` now costs a loud `TypeError` instead.
+
+⚠️ No published version shipped the mismatch. The runner's rewrite and this adapter land in the same release, and `@objectstack/plugin-auth` depends on `@objectstack/rest` at an exact workspace version, so the two cannot be installed apart. What this entry records is why they move together — and what the same mismatch costs any OTHER hand-written `ImportProtocolLike`, which the `@objectstack/rest` entry calls out for implementors.

packages/plugins/plugin-auth/src/admin-import-users.ts

@@ -349,10 +349,23 @@ export async function runAdminImportUsers(
   }
 
   const protocol: ImportProtocolLike = {
-    // findExisting path: `{ $filter, $top }` against sys_user.
+    // [#16638] findExisting path: the CANONICAL QueryAST against sys_user.
+    // `runImport` hands this handle a `FindDataRequest`, and the `QuerySchema`
+    // that request declares carries `where` / `limit` — it declares neither
+    // `$filter` nor `$top`. Those two are wire-only spellings the protocol
+    // normalizer folds for a caller off the HTTP door; this protocol is
+    // hand-written and never passes through that normalizer, so it must read
+    // the canonical keys itself.
+    //
+    // Two dialects would be the lenient `??` alias Prime Directive #12 forbids,
+    // and the default is not a harmless belt either: a `where` that falls back
+    // to `{}` stops constraining anything, so the duplicate probe matches rows
+    // it was given no key for and the upsert updates the WRONG user. One
+    // dialect, read straight — a request that arrives without a `query` is a
+    // caller defect and costs a loud TypeError, not a silent match-everything.
     async findData(args: any) {
-      const where = args?.query?.$filter ?? {};
-      const limit = args?.query?.$top ?? 2;
+      const where = args.query.where;
+      const limit = args.query.limit;
       return engine.find(args.object, { where, limit, context: SYSTEM_CTX } as any);
     },
 

packages/rest/src/import-runner-bulk.test.ts

@@ -148,8 +148,9 @@ describe('runImport — bulk create batching (framework#2678)', () => {
     }));
     const updateData = vi.fn(async (args: { id: string }) => ({ id: args.id }));
     // Row 1 ('existing') matches an existing record → update; the rest are creates.
-    const findData = vi.fn(async (args: { query?: { $filter?: { name?: string } } }) =>
-      (args.query?.$filter?.name === 'existing' ? [{ id: 'existing_id', name: 'existing' }] : []));
+    // [#16638] Reads the CANONICAL `where` the runner sends, not `$filter`.
+    const findData = vi.fn(async (args: { query: { where: { name?: string } } }) =>
+      (args.query.where.name === 'existing' ? [{ id: 'existing_id', name: 'existing' }] : []));
     const p: ImportProtocolLike = { findData, createData: vi.fn(), updateData, createManyData };
 
     const summary = await runImport({

packages/rest/src/import-runner-idempotency.test.ts

@@ -49,8 +49,21 @@ function makeProtocol(opts: { firstCall?: 'throw' | 'shortReturn' } = {}) {
     store.push(rec);
     return rec;
   });
-  const findData = vi.fn(async (args: { query?: { $filter?: Record<string, any> } }) => {
-    const filter = args.query?.$filter ?? {};
+  // [#16638] Reads the CANONICAL `where` the runner sends. ⛔ The `?? {}` this
+  // replaces is what made this whole file pass VACUOUSLY once the runner moved
+  // to `where`: with `$filter` undefined every recheck degraded to `{}`, which
+  // constrains nothing, so the recheck matched the entire store and the
+  // no-duplicate assertions below held without the probe discriminating at all.
+  // Reading `where` straight means an absent filter throws instead.
+  /**
+   * [#16638] Every filter this double actually APPLIED. Pinning the payload
+   * alone would still pass over a double that read the wrong key and defaulted
+   * to `{}` — which is the vacuity being closed here, so both are recorded.
+   */
+  const appliedFilters: Array<Record<string, any>> = [];
+  const findData = vi.fn(async (args: { query: { where: Record<string, any>; limit?: number } }) => {
+    const filter = args.query.where;
+    appliedFilters.push(filter);
     // Supports equality and { $in: [...] } — the id recheck (framework#3173)
     // queries by pre-assigned id $in, like the real SQL driver does.
     return store.filter((row) => Object.entries(filter).every(([k, v]) => { if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); 
@@ -59,12 +72,45 @@ function makeProtocol(opts: { firstCall?: 'throw' | 'shortReturn' } = {}) {
     }));
   });
   const p: ImportProtocolLike = { findData, createData, updateData: vi.fn(), createManyData };
-  return { p, store, createManyData, createData };
+  return { p, store, createManyData, createData, findData, appliedFilters };
+}
+
+/** One recorded `findData` probe, as the double above receives it. */
+type FindProbe = { query: { where: Record<string, any>; limit?: number } };
+
+/**
+ * ⭐ [#16638] Every probe the runner sends must NARROW — the assertion this
+ * file was missing, and the reason it stayed GREEN through a payload rewrite
+ * that broke it. While the double read `args.query.$filter`, a runner sending
+ * `where` left that read `undefined`, the `?? {}` default turned it into an
+ * empty filter, and an empty filter constrains NOTHING: every recheck matched
+ * the entire store, so each `store` / `created` expectation below held without
+ * the probe discriminating between one row and any other. Passing was not
+ * evidence. `{}` is the shape that has to be refused, so it is asserted
+ * against directly.
+ *
+ * `Object.keys` on an ABSENT `where` throws rather than reporting zero keys,
+ * and that is deliberate: a spelling drift must be loud here, not degrade into
+ * a probe that matches everything.
+ */
+function expectEveryProbeNarrowed(
+  calls: ReadonlyArray<readonly [FindProbe]>,
+  appliedFilters: ReadonlyArray<Record<string, any>>,
+): void {
+  expect(calls.length).toBeGreaterThan(0);
+  for (const [args] of calls) {
+    expect(Object.keys(args.query.where)).not.toHaveLength(0);
+  }
+  // The payload half is the drift alarm; this is the vacuity half. The filter
+  // the double APPLIED must be the one it was handed — an equality a `?? {}`
+  // default breaks even while the runner's payload stays perfectly canonical.
+  expect(appliedFilters).toEqual(calls.map(([args]) => args.query.where));
+  for (const filter of appliedFilters) expect(Object.keys(filter)).not.toHaveLength(0);
 }
 
 describe('runImport — idempotent retry with natural keys (framework#3149)', () => {
   it('upsert+matchFields: a transient retry after commit does not duplicate rows', async () => {
-    const { p, store, createManyData } = makeProtocol({ firstCall: 'throw' });
+    const { p, store, createManyData, findData, appliedFilters } = makeProtocol({ firstCall: 'throw' });
 
     const summary = await runImport({
       ...baseOpts, p, writeMode: 'upsert', matchFields: ['name'],
@@ -79,6 +125,10 @@ describe('runImport — idempotent retry with natural keys (framework#3149)', ()
     expect(store).toHaveLength(2); // no duplicates
     expect(summary.created).toBe(2);
     expect(summary.errors).toBe(0);
+    // ⭐ [#16638] …and every probe that produced those numbers actually
+    // constrained something. The natural-key probes carry the match field.
+    expectEveryProbeNarrowed(findData.mock.calls, appliedFilters);
+    expect(findData.mock.calls.map(([a]) => Object.keys(a.query.where))).toContainEqual(['name']);
   });
 
   it('upsert+matchFields: a short createManyData return degrades and still does not duplicate', async () => {
@@ -97,7 +147,7 @@ describe('runImport — idempotent retry with natural keys (framework#3149)', ()
   });
 
   it('pure insert (no matchFields): pre-assigned ids make the retry exactly-once too (#3173)', async () => {
-    const { p, store, createManyData } = makeProtocol({ firstCall: 'throw' });
+    const { p, store, createManyData, findData, appliedFilters } = makeProtocol({ firstCall: 'throw' });
 
     const summary = await runImport({
       ...baseOpts, p, writeMode: 'insert', matchFields: [],
@@ -110,6 +160,18 @@ describe('runImport — idempotent retry with natural keys (framework#3149)', ()
     expect(store).toHaveLength(2);
     expect(summary.created).toBe(2);
     expect(summary.errors).toBe(0);
+
+    // ⭐ [#16638] The recheck is the whole mechanism of #3173, so pin the
+    // payload it was handed rather than only the outcome: `id: { $in: [...] }`
+    // over exactly the ids the runner pre-assigned, bounded to that many rows.
+    // Read `where` / `limit`, the keys `FindDataRequest` declares — a drift
+    // back to `$filter` / `$top` reddens here before it reaches an implementor.
+    expectEveryProbeNarrowed(findData.mock.calls, appliedFilters);
+    const probes = findData.mock.calls.map(([a]) => a.query);
+    expect(probes).toHaveLength(1);
+    expect(Object.keys(probes[0].where)).toEqual(['id']);
+    expect([...probes[0].where.id.$in].sort()).toEqual(store.map((r) => r.id).sort());
+    expect(probes[0].limit).toBe(store.length);
   });
 
   it('pure insert: legitimate duplicate rows survive the retry intact (each copy has its own id) (#3173)', async () => {

packages/rest/src/import-runner-selfref.test.ts

@@ -42,8 +42,10 @@ function makeProtocol(seed: Array<Record<string, any>> = []) {
       return rec;
     }),
   }));
-  const findData = vi.fn(async (args: { query?: { $filter?: Record<string, any> } }) => {
-    const filter = args.query?.$filter ?? {};
+  // [#16638] Reads the CANONICAL `where` the runner sends. ⛔ No `?? {}`: an
+  // absent filter must throw here, never degrade into a match-everything probe.
+  const findData = vi.fn(async (args: { query: { where: Record<string, any> } }) => {
+    const filter = args.query.where;
     return store.filter((row) => Object.entries(filter).every(([k, v]) => { if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); return row[k] === v; }));
   });
   const p: ImportProtocolLike = { findData, createData: vi.fn(), updateData: vi.fn(), createManyData };