refactor(rest)!: ImportProtocolLike declares the request each of its three required members receives

.changeset/import-protocol-typed-args.md

@@ -0,0 +1,53 @@
+---
+"@objectstack/rest": minor
+---
+
+refactor(rest)!: `ImportProtocolLike` declares the request each of its three required members receives, instead of `args: any` (#16952)
+
+The exported extension point `runImport` accepts a protocol through now states its own contract.
+
+**FROM** — every required member erased its parameter, so the interface declared nothing about the request it would hand an implementor:
+
+```ts
+export interface ImportProtocolLike {
+  findData(args: any): Promise<any>;
+  createData(args: any): Promise<any>;
+  updateData(args: any): Promise<any>;
+}
+```
+
+**TO** — each member names the declared spec request, wrapped in the server-scoped envelope the runner adds (`ImportProtocolRequest`, exported alongside):
+
+```ts
+export type ImportProtocolRequest<R> = R & { context?: any; environmentId?: string };
+
+export interface ImportProtocolLike {
+  findData(args: ImportProtocolRequest<FindDataRequest>): Promise<any>;
+  createData(args: ImportProtocolRequest<CreateDataRequest>): Promise<any>;
+  updateData(args: ImportProtocolRequest<UpdateDataRequest>): Promise<any>;
+}
+```
+
+**Why this is breaking-ish, and released as `minor`.** This is a narrowing of a published surface: an implementor that compiles today may stop compiling. Nothing about the values the runner sends changes — the request objects are byte-for-byte the ones #16638 already made canonical — so no runtime behaviour moves. What changes is that the compiler now holds an implementor to the same `QuerySchema` the runner is held to: `where` / `limit` / `offset` / `fields` / `orderBy` / `expand` are declared, and the wire spellings `$filter` / `$top` are not.
+
+**Migration for implementors.** If your `findData` / `createData` / `updateData` reads a wire alias, it will now fail to compile — that diagnostic is the point of this change, and the fix is to read the canonical key:
+
+```ts
+// before — compiles, and silently degrades to match-everything when `$filter` is absent
+async findData(args: any) {
+  const where = args?.query?.$filter ?? {};
+  const limit = args?.query?.$top ?? 2;
+}
+
+// after — drop your own annotation and let the declaration type the parameter
+async findData(args) {
+  const where = args.query!.where;
+  const limit = args.query!.limit;
+}
+```
+
+⛔ An implementor that keeps an explicit `args: any` annotation of its own opts back out: the annotation wins over the contextual type, and the contract reaches nothing. Leave the parameter unannotated, or name `ImportProtocolRequest<FindDataRequest>` explicitly.
+
+⚠️ The `?? {}` shape in the "before" is the mechanism that made a dialect mismatch silent rather than loud: an unrecognised query does not throw, it degrades into a filter that constrains nothing, so a duplicate probe stops discriminating and an upsert updates the wrong record. Prefer a read that throws.
+
+<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable is renamed, retired or re-typed: `packages/spec` is untouched, no metadata key changes its name, type or optionality, and no stored `sys_metadata` shape moves — every request body and every authored file parses byte-identically to before, so `objectstack migrate meta`, `spec-changes.json` and the generated upgrade guide have nothing to rewrite. What narrows is a TypeScript parameter annotation on one exported interface, so the affected party is a source-code implementor and the delivery channel is the compiler at their own call site. The FROM/TO block in this body is a prescription for THAT reader, not for a metadata upgrader. -->

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

@@ -9,6 +9,18 @@
 
 import { describe, it, expect, vi } from 'vitest';
 import { runImport, type ImportProtocolLike } from './import-runner';
+
+/**
+ * [#16952] The doubles below are annotated FROM the exported declaration
+ * (`ImportProtocolLike`), never from a hand-written restatement of the shape
+ * the runner happens to send. A local parameter annotation was one of the
+ * three non-authoritative places this card converged: it froze a dialect no
+ * compiler held anyone to, so it kept compiling — and kept passing — after the
+ * runner moved to another one. ⛔ Never widen these back to an inline object
+ * type; that re-opens the seam.
+ */
+type FindArgs = Parameters<ImportProtocolLike['findData']>[0];
+type CreateArgs = Parameters<ImportProtocolLike['createData']>[0];
 import type { ExportFieldMeta } from './export-format.js';
 
 const metaMap = new Map<string, ExportFieldMeta>([
@@ -85,9 +97,9 @@ describe('runImport — bulk create batching (framework#2678)', () => {
     const createManyData = vi.fn(async () => {
       throw new Error('CHECK constraint failed');
     });
-    const createData = vi.fn(async (args: { data: { name: string } }) => {
+    const createData = vi.fn(async (args: CreateArgs) => {
       if (args.data.name === 'r1') throw new Error('CHECK constraint failed: name');
-      return { id: `id_${args.data.name}`, record: { id: `id_${args.data.name}` } };
+      return { id: `id_${String(args.data.name)}`, record: { id: `id_${String(args.data.name)}` } };
     });
     const p: ImportProtocolLike = {
       findData: vi.fn(async () => []),
@@ -107,7 +119,7 @@ describe('runImport — bulk create batching (framework#2678)', () => {
   });
 
   it('falls back to one createData call per row when the protocol has no createManyData', async () => {
-    const createData = vi.fn(async (args: { data: { name: string } }) => ({ id: `id_${args.data.name}` }));
+    const createData = vi.fn(async (args: CreateArgs) => ({ id: `id_${String(args.data.name)}` }));
     const p: ImportProtocolLike = {
       findData: vi.fn(async () => []),
       createData,
@@ -123,10 +135,10 @@ describe('runImport — bulk create batching (framework#2678)', () => {
 
   it('retries a transient createData failure on the no-createManyData fallback path (#3150)', async () => {
     let attempts = 0;
-    const createData = vi.fn(async (args: { data: { name: string } }) => {
+    const createData = vi.fn(async (args: CreateArgs) => {
       attempts++;
       if (attempts === 1) throw new Error('fetch failed'); // one transient blip, then succeeds
-      return { id: `id_${args.data.name}` };
+      return { id: `id_${String(args.data.name)}` };
     });
     const p: ImportProtocolLike = {
       findData: vi.fn(async () => []),
@@ -149,8 +161,8 @@ 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.
     // [#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 findData = vi.fn(async (args: FindArgs) =>
+      (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-cancel.test.ts

@@ -14,6 +14,17 @@
 
 import { describe, it, expect, vi } from 'vitest';
 import { runImport, type ImportProtocolLike } from './import-runner';
+
+/**
+ * [#16952] The doubles below are annotated FROM the exported declaration
+ * (`ImportProtocolLike`), never from a hand-written restatement of the shape
+ * the runner happens to send. A local parameter annotation was one of the
+ * three non-authoritative places this card converged: it froze a dialect no
+ * compiler held anyone to, so it kept compiling — and kept passing — after the
+ * runner moved to another one. ⛔ Never widen these back to an inline object
+ * type; that re-opens the seam.
+ */
+type CreateArgs = Parameters<ImportProtocolLike['createData']>[0];
 import type { ExportFieldMeta } from './export-format.js';
 
 const metaMap = new Map<string, ExportFieldMeta>([
@@ -40,7 +51,7 @@ function rowsOf(n: number): Array<Record<string, any>> {
 function syncProtocol(): ImportProtocolLike {
   return {
     findData: vi.fn(async () => []),
-    createData: vi.fn(async (args: { data: { name: string } }) => ({ id: `id_${args.data.name}` })),
+    createData: vi.fn(async (args: CreateArgs) => ({ id: `id_${String(args.data.name)}` })),
     updateData: vi.fn(async () => ({})),
     createManyData: vi.fn(async (args: { records: any[] }) => ({
       records: args.records.map((r) => ({ id: `id_${r.name}`, ...r })),

packages/rest/src/import-runner-historical-readonly-insert.test.ts

@@ -99,10 +99,14 @@ async function makeRealProtocol() {
   const impl = new ObjectStackProtocolImplementation(engine as any);
   // `runImport` needs find/create only for an insert-mode run; delegate both to
   // the real implementation so the ingress is genuinely on the path.
+  // [#16952] The bridge is annotated FROM the exported declaration, so the
+  // request this test hands the REAL implementation is the one the contract
+  // declares — `args: any` here would have made the bridge itself another
+  // place the dialect was only observed.
   const p: ImportProtocolLike = {
-    findData: (args: any) => impl.findData(args as any) as any,
-    createData: (args: any) => impl.createData(args as any) as any,
-    updateData: (args: any) => impl.updateData(args as any) as any,
+    findData: (args) => impl.findData(args) as any,
+    createData: (args) => impl.createData(args) as any,
+    updateData: (args) => impl.updateData(args) as any,
   };
   return { p, inserted, logger };
 }

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

@@ -15,6 +15,17 @@
 
 import { describe, it, expect, vi } from 'vitest';
 import { runImport, type ImportProtocolLike } from './import-runner';
+
+/**
+ * [#16952] Annotated FROM the exported declaration (`ImportProtocolLike`).
+ * These doubles used to say `args: any`, which is the erasure this card
+ * retired at the declaration — an implementor that re-states `any` on its own
+ * parameter opts back out of the contract, because the annotation wins over
+ * the contextual type.
+ */
+type CreateArgs = Parameters<ImportProtocolLike['createData']>[0];
+type UpdateArgs = Parameters<ImportProtocolLike['updateData']>[0];
+type CreateManyArgs = Parameters<NonNullable<ImportProtocolLike['createManyData']>>[0];
 import type { ExportFieldMeta } from './export-format.js';
 
 const metaMap = new Map<string, ExportFieldMeta>([['name', { name: 'name', type: 'text' }]]);
@@ -37,15 +48,15 @@ function makeProvider() {
   let idc = 0;
   const p: ImportProtocolLike = {
     findData: vi.fn(async () => []),
-    createData: vi.fn(async (args: any) => {
+    createData: vi.fn(async (args: CreateArgs) => {
       contexts.push(args.context);
       return { id: `d${++idc}`, ...args.data };
     }),
-    updateData: vi.fn(async (args: any) => {
+    updateData: vi.fn(async (args: UpdateArgs) => {
       contexts.push(args.context);
       return { id: args.id, ...args.data };
     }),
-    createManyData: vi.fn(async (args: any) => {
+    createManyData: vi.fn(async (args: CreateManyArgs) => {
       contexts.push(args.context);
       return { records: args.records.map((r: any) => ({ id: `d${++idc}`, ...r })) };
     }),

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

@@ -12,6 +12,18 @@ import { describe, it, expect, vi } from 'vitest';
 import { runImport, type ImportProtocolLike } from './import-runner';
 import type { ExportFieldMeta } from './export-format.js';
 
+/**
+ * [#16952] The doubles below are annotated FROM the exported declaration
+ * (`ImportProtocolLike`), never from a hand-written restatement of the shape
+ * the runner happens to send. A local parameter annotation was one of the
+ * three non-authoritative places this card converged: it froze a dialect no
+ * compiler held anyone to, so it kept compiling — and kept passing — after the
+ * runner moved to another one. ⛔ Never widen these back to an inline object
+ * type; that re-opens the seam.
+ */
+type FindArgs = Parameters<ImportProtocolLike['findData']>[0];
+type CreateArgs = Parameters<ImportProtocolLike['createData']>[0];
+
 const metaMap = new Map<string, ExportFieldMeta>([['name', { name: 'name', type: 'text' }]]);
 
 const baseOpts = {
@@ -44,7 +56,7 @@ function makeProtocol(opts: { firstCall?: 'throw' | 'shortReturn' } = {}) {
     if (calls === 1 && opts.firstCall === 'shortReturn') return { records: [] };     // committed, bad count
     return { records: recs };
   });
-  const createData = vi.fn(async (args: { data: { name: string } }) => {
+  const createData = vi.fn(async (args: CreateArgs) => {
     const rec = { id: `id-${++idc}`, ...args.data };
     store.push(rec);
     return rec;
@@ -61,8 +73,11 @@ function makeProtocol(opts: { firstCall?: 'throw' | 'shortReturn' } = {}) {
    * 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;
+  const findData = vi.fn(async (args: FindArgs) => {
+    // Both slots are OPTIONAL on the declared contract, and the `!`s say so
+    // while keeping the refusal: an absent one throws here exactly as it did
+    // before, rather than degrading into a match-everything probe.
+    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.
@@ -75,8 +90,11 @@ function makeProtocol(opts: { firstCall?: 'throw' | 'shortReturn' } = {}) {
   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 } };
+/**
+ * One recorded `findData` probe — the DECLARED parameter type, not a
+ * restatement of it. [#16952]
+ */
+type FindProbe = FindArgs;
 
 /**
  * ⭐ [#16638] Every probe the runner sends must NARROW — the assertion this
@@ -99,12 +117,12 @@ function expectEveryProbeNarrowed(
 ): void {
   expect(calls.length).toBeGreaterThan(0);
   for (const [args] of calls) {
-    expect(Object.keys(args.query.where)).not.toHaveLength(0);
+    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));
+  expect(appliedFilters).toEqual(calls.map(([args]) => args.query!.where!));
   for (const filter of appliedFilters) expect(Object.keys(filter)).not.toHaveLength(0);
 }
 
@@ -128,7 +146,7 @@ describe('runImport — idempotent retry with natural keys (framework#3149)', ()
     // ⭐ [#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']);
+    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 () => {
@@ -167,10 +185,10 @@ describe('runImport — idempotent retry with natural keys (framework#3149)', ()
     // 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);
+    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(Object.keys(probes[0].where!)).toEqual(['id']);
+    expect([...(probes[0].where!.id as { $in: string[] }).$in].sort()).toEqual(store.map((r) => r.id).sort());
     expect(probes[0].limit).toBe(store.length);
   });
 

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

@@ -11,6 +11,17 @@
 
 import { describe, it, expect, vi } from 'vitest';
 import { runImport, type ImportProtocolLike } from './import-runner';
+
+/**
+ * [#16952] The doubles below are annotated FROM the exported declaration
+ * (`ImportProtocolLike`), never from a hand-written restatement of the shape
+ * the runner happens to send. A local parameter annotation was one of the
+ * three non-authoritative places this card converged: it froze a dialect no
+ * compiler held anyone to, so it kept compiling — and kept passing — after the
+ * runner moved to another one. ⛔ Never widen these back to an inline object
+ * type; that re-opens the seam.
+ */
+type FindArgs = Parameters<ImportProtocolLike['findData']>[0];
 import type { ExportFieldMeta } from './export-format.js';
 
 // `parent` is a lookup back to this same object (a category tree).
@@ -44,8 +55,8 @@ function makeProtocol(seed: Array<Record<string, any>> = []) {
   }));
   // [#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;
+  const findData = vi.fn(async (args: FindArgs) => {
+    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 };