Skip to content

Commit ab56ea3

Browse files
os-justinclaude
andauthored
refactor(rest)!: ImportProtocolLike declares the request each of its three required members receives (#17420)
* wip: type ImportProtocolLike's three required members Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt * wip: converge doubles and add pins Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt * wip: test layer green * changeset + pins * changeset: adr-0087 disposition * changeset: symbol path * probe no-migration-prescription * probe runtime-interface-only packages/rest/src/import-runner.ts#ImportProtocolLike * changeset: no-migration-prescription disposition --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 31064ca commit ab56ea3

11 files changed

Lines changed: 320 additions & 41 deletions
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
---
2+
"@objectstack/rest": minor
3+
---
4+
5+
refactor(rest)!: `ImportProtocolLike` declares the request each of its three required members receives, instead of `args: any` (#16952)
6+
7+
The exported extension point `runImport` accepts a protocol through now states its own contract.
8+
9+
**FROM** — every required member erased its parameter, so the interface declared nothing about the request it would hand an implementor:
10+
11+
```ts
12+
export interface ImportProtocolLike {
13+
findData(args: any): Promise<any>;
14+
createData(args: any): Promise<any>;
15+
updateData(args: any): Promise<any>;
16+
}
17+
```
18+
19+
**TO** — each member names the declared spec request, wrapped in the server-scoped envelope the runner adds (`ImportProtocolRequest`, exported alongside):
20+
21+
```ts
22+
export type ImportProtocolRequest<R> = R & { context?: any; environmentId?: string };
23+
24+
export interface ImportProtocolLike {
25+
findData(args: ImportProtocolRequest<FindDataRequest>): Promise<any>;
26+
createData(args: ImportProtocolRequest<CreateDataRequest>): Promise<any>;
27+
updateData(args: ImportProtocolRequest<UpdateDataRequest>): Promise<any>;
28+
}
29+
```
30+
31+
**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.
32+
33+
**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:
34+
35+
```ts
36+
// before — compiles, and silently degrades to match-everything when `$filter` is absent
37+
async findData(args: any) {
38+
const where = args?.query?.$filter ?? {};
39+
const limit = args?.query?.$top ?? 2;
40+
}
41+
42+
// after — drop your own annotation and let the declaration type the parameter
43+
async findData(args) {
44+
const where = args.query!.where;
45+
const limit = args.query!.limit;
46+
}
47+
```
48+
49+
⛔ 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.
50+
51+
⚠️ 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.
52+
53+
<!-- 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

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,18 @@
99

1010
import { describe, it, expect, vi } from 'vitest';
1111
import { runImport, type ImportProtocolLike } from './import-runner';
12+
13+
/**
14+
* [#16952] The doubles below are annotated FROM the exported declaration
15+
* (`ImportProtocolLike`), never from a hand-written restatement of the shape
16+
* the runner happens to send. A local parameter annotation was one of the
17+
* three non-authoritative places this card converged: it froze a dialect no
18+
* compiler held anyone to, so it kept compiling — and kept passing — after the
19+
* runner moved to another one. ⛔ Never widen these back to an inline object
20+
* type; that re-opens the seam.
21+
*/
22+
type FindArgs = Parameters<ImportProtocolLike['findData']>[0];
23+
type CreateArgs = Parameters<ImportProtocolLike['createData']>[0];
1224
import type { ExportFieldMeta } from './export-format.js';
1325

1426
const metaMap = new Map<string, ExportFieldMeta>([
@@ -85,9 +97,9 @@ describe('runImport — bulk create batching (framework#2678)', () => {
8597
const createManyData = vi.fn(async () => {
8698
throw new Error('CHECK constraint failed');
8799
});
88-
const createData = vi.fn(async (args: { data: { name: string } }) => {
100+
const createData = vi.fn(async (args: CreateArgs) => {
89101
if (args.data.name === 'r1') throw new Error('CHECK constraint failed: name');
90-
return { id: `id_${args.data.name}`, record: { id: `id_${args.data.name}` } };
102+
return { id: `id_${String(args.data.name)}`, record: { id: `id_${String(args.data.name)}` } };
91103
});
92104
const p: ImportProtocolLike = {
93105
findData: vi.fn(async () => []),
@@ -107,7 +119,7 @@ describe('runImport — bulk create batching (framework#2678)', () => {
107119
});
108120

109121
it('falls back to one createData call per row when the protocol has no createManyData', async () => {
110-
const createData = vi.fn(async (args: { data: { name: string } }) => ({ id: `id_${args.data.name}` }));
122+
const createData = vi.fn(async (args: CreateArgs) => ({ id: `id_${String(args.data.name)}` }));
111123
const p: ImportProtocolLike = {
112124
findData: vi.fn(async () => []),
113125
createData,
@@ -123,10 +135,10 @@ describe('runImport — bulk create batching (framework#2678)', () => {
123135

124136
it('retries a transient createData failure on the no-createManyData fallback path (#3150)', async () => {
125137
let attempts = 0;
126-
const createData = vi.fn(async (args: { data: { name: string } }) => {
138+
const createData = vi.fn(async (args: CreateArgs) => {
127139
attempts++;
128140
if (attempts === 1) throw new Error('fetch failed'); // one transient blip, then succeeds
129-
return { id: `id_${args.data.name}` };
141+
return { id: `id_${String(args.data.name)}` };
130142
});
131143
const p: ImportProtocolLike = {
132144
findData: vi.fn(async () => []),
@@ -149,8 +161,8 @@ describe('runImport — bulk create batching (framework#2678)', () => {
149161
const updateData = vi.fn(async (args: { id: string }) => ({ id: args.id }));
150162
// Row 1 ('existing') matches an existing record → update; the rest are creates.
151163
// [#16638] Reads the CANONICAL `where` the runner sends, not `$filter`.
152-
const findData = vi.fn(async (args: { query: { where: { name?: string } } }) =>
153-
(args.query.where.name === 'existing' ? [{ id: 'existing_id', name: 'existing' }] : []));
164+
const findData = vi.fn(async (args: FindArgs) =>
165+
(args.query!.where!.name === 'existing' ? [{ id: 'existing_id', name: 'existing' }] : []));
154166
const p: ImportProtocolLike = { findData, createData: vi.fn(), updateData, createManyData };
155167

156168
const summary = await runImport({

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,17 @@
1414

1515
import { describe, it, expect, vi } from 'vitest';
1616
import { runImport, type ImportProtocolLike } from './import-runner';
17+
18+
/**
19+
* [#16952] The doubles below are annotated FROM the exported declaration
20+
* (`ImportProtocolLike`), never from a hand-written restatement of the shape
21+
* the runner happens to send. A local parameter annotation was one of the
22+
* three non-authoritative places this card converged: it froze a dialect no
23+
* compiler held anyone to, so it kept compiling — and kept passing — after the
24+
* runner moved to another one. ⛔ Never widen these back to an inline object
25+
* type; that re-opens the seam.
26+
*/
27+
type CreateArgs = Parameters<ImportProtocolLike['createData']>[0];
1728
import type { ExportFieldMeta } from './export-format.js';
1829

1930
const metaMap = new Map<string, ExportFieldMeta>([
@@ -40,7 +51,7 @@ function rowsOf(n: number): Array<Record<string, any>> {
4051
function syncProtocol(): ImportProtocolLike {
4152
return {
4253
findData: vi.fn(async () => []),
43-
createData: vi.fn(async (args: { data: { name: string } }) => ({ id: `id_${args.data.name}` })),
54+
createData: vi.fn(async (args: CreateArgs) => ({ id: `id_${String(args.data.name)}` })),
4455
updateData: vi.fn(async () => ({})),
4556
createManyData: vi.fn(async (args: { records: any[] }) => ({
4657
records: args.records.map((r) => ({ id: `id_${r.name}`, ...r })),

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,14 @@ async function makeRealProtocol() {
9999
const impl = new ObjectStackProtocolImplementation(engine as any);
100100
// `runImport` needs find/create only for an insert-mode run; delegate both to
101101
// the real implementation so the ingress is genuinely on the path.
102+
// [#16952] The bridge is annotated FROM the exported declaration, so the
103+
// request this test hands the REAL implementation is the one the contract
104+
// declares — `args: any` here would have made the bridge itself another
105+
// place the dialect was only observed.
102106
const p: ImportProtocolLike = {
103-
findData: (args: any) => impl.findData(args as any) as any,
104-
createData: (args: any) => impl.createData(args as any) as any,
105-
updateData: (args: any) => impl.updateData(args as any) as any,
107+
findData: (args) => impl.findData(args) as any,
108+
createData: (args) => impl.createData(args) as any,
109+
updateData: (args) => impl.updateData(args) as any,
106110
};
107111
return { p, inserted, logger };
108112
}

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,17 @@
1515

1616
import { describe, it, expect, vi } from 'vitest';
1717
import { runImport, type ImportProtocolLike } from './import-runner';
18+
19+
/**
20+
* [#16952] Annotated FROM the exported declaration (`ImportProtocolLike`).
21+
* These doubles used to say `args: any`, which is the erasure this card
22+
* retired at the declaration — an implementor that re-states `any` on its own
23+
* parameter opts back out of the contract, because the annotation wins over
24+
* the contextual type.
25+
*/
26+
type CreateArgs = Parameters<ImportProtocolLike['createData']>[0];
27+
type UpdateArgs = Parameters<ImportProtocolLike['updateData']>[0];
28+
type CreateManyArgs = Parameters<NonNullable<ImportProtocolLike['createManyData']>>[0];
1829
import type { ExportFieldMeta } from './export-format.js';
1930

2031
const metaMap = new Map<string, ExportFieldMeta>([['name', { name: 'name', type: 'text' }]]);
@@ -37,15 +48,15 @@ function makeProvider() {
3748
let idc = 0;
3849
const p: ImportProtocolLike = {
3950
findData: vi.fn(async () => []),
40-
createData: vi.fn(async (args: any) => {
51+
createData: vi.fn(async (args: CreateArgs) => {
4152
contexts.push(args.context);
4253
return { id: `d${++idc}`, ...args.data };
4354
}),
44-
updateData: vi.fn(async (args: any) => {
55+
updateData: vi.fn(async (args: UpdateArgs) => {
4556
contexts.push(args.context);
4657
return { id: args.id, ...args.data };
4758
}),
48-
createManyData: vi.fn(async (args: any) => {
59+
createManyData: vi.fn(async (args: CreateManyArgs) => {
4960
contexts.push(args.context);
5061
return { records: args.records.map((r: any) => ({ id: `d${++idc}`, ...r })) };
5162
}),

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

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,18 @@ import { describe, it, expect, vi } from 'vitest';
1212
import { runImport, type ImportProtocolLike } from './import-runner';
1313
import type { ExportFieldMeta } from './export-format.js';
1414

15+
/**
16+
* [#16952] The doubles below are annotated FROM the exported declaration
17+
* (`ImportProtocolLike`), never from a hand-written restatement of the shape
18+
* the runner happens to send. A local parameter annotation was one of the
19+
* three non-authoritative places this card converged: it froze a dialect no
20+
* compiler held anyone to, so it kept compiling — and kept passing — after the
21+
* runner moved to another one. ⛔ Never widen these back to an inline object
22+
* type; that re-opens the seam.
23+
*/
24+
type FindArgs = Parameters<ImportProtocolLike['findData']>[0];
25+
type CreateArgs = Parameters<ImportProtocolLike['createData']>[0];
26+
1527
const metaMap = new Map<string, ExportFieldMeta>([['name', { name: 'name', type: 'text' }]]);
1628

1729
const baseOpts = {
@@ -44,7 +56,7 @@ function makeProtocol(opts: { firstCall?: 'throw' | 'shortReturn' } = {}) {
4456
if (calls === 1 && opts.firstCall === 'shortReturn') return { records: [] }; // committed, bad count
4557
return { records: recs };
4658
});
47-
const createData = vi.fn(async (args: { data: { name: string } }) => {
59+
const createData = vi.fn(async (args: CreateArgs) => {
4860
const rec = { id: `id-${++idc}`, ...args.data };
4961
store.push(rec);
5062
return rec;
@@ -61,8 +73,11 @@ function makeProtocol(opts: { firstCall?: 'throw' | 'shortReturn' } = {}) {
6173
* to `{}` — which is the vacuity being closed here, so both are recorded.
6274
*/
6375
const appliedFilters: Array<Record<string, any>> = [];
64-
const findData = vi.fn(async (args: { query: { where: Record<string, any>; limit?: number } }) => {
65-
const filter = args.query.where;
76+
const findData = vi.fn(async (args: FindArgs) => {
77+
// Both slots are OPTIONAL on the declared contract, and the `!`s say so
78+
// while keeping the refusal: an absent one throws here exactly as it did
79+
// before, rather than degrading into a match-everything probe.
80+
const filter = args.query!.where!;
6681
appliedFilters.push(filter);
6782
// Supports equality and { $in: [...] } — the id recheck (framework#3173)
6883
// queries by pre-assigned id $in, like the real SQL driver does.
@@ -75,8 +90,11 @@ function makeProtocol(opts: { firstCall?: 'throw' | 'shortReturn' } = {}) {
7590
return { p, store, createManyData, createData, findData, appliedFilters };
7691
}
7792

78-
/** One recorded `findData` probe, as the double above receives it. */
79-
type FindProbe = { query: { where: Record<string, any>; limit?: number } };
93+
/**
94+
* One recorded `findData` probe — the DECLARED parameter type, not a
95+
* restatement of it. [#16952]
96+
*/
97+
type FindProbe = FindArgs;
8098

8199
/**
82100
* ⭐ [#16638] Every probe the runner sends must NARROW — the assertion this
@@ -99,12 +117,12 @@ function expectEveryProbeNarrowed(
99117
): void {
100118
expect(calls.length).toBeGreaterThan(0);
101119
for (const [args] of calls) {
102-
expect(Object.keys(args.query.where)).not.toHaveLength(0);
120+
expect(Object.keys(args.query!.where!)).not.toHaveLength(0);
103121
}
104122
// The payload half is the drift alarm; this is the vacuity half. The filter
105123
// the double APPLIED must be the one it was handed — an equality a `?? {}`
106124
// default breaks even while the runner's payload stays perfectly canonical.
107-
expect(appliedFilters).toEqual(calls.map(([args]) => args.query.where));
125+
expect(appliedFilters).toEqual(calls.map(([args]) => args.query!.where!));
108126
for (const filter of appliedFilters) expect(Object.keys(filter)).not.toHaveLength(0);
109127
}
110128

@@ -128,7 +146,7 @@ describe('runImport — idempotent retry with natural keys (framework#3149)', ()
128146
// ⭐ [#16638] …and every probe that produced those numbers actually
129147
// constrained something. The natural-key probes carry the match field.
130148
expectEveryProbeNarrowed(findData.mock.calls, appliedFilters);
131-
expect(findData.mock.calls.map(([a]) => Object.keys(a.query.where))).toContainEqual(['name']);
149+
expect(findData.mock.calls.map(([a]) => Object.keys(a.query!.where!))).toContainEqual(['name']);
132150
});
133151

134152
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)', ()
167185
// Read `where` / `limit`, the keys `FindDataRequest` declares — a drift
168186
// back to `$filter` / `$top` reddens here before it reaches an implementor.
169187
expectEveryProbeNarrowed(findData.mock.calls, appliedFilters);
170-
const probes = findData.mock.calls.map(([a]) => a.query);
188+
const probes = findData.mock.calls.map(([a]) => a.query!);
171189
expect(probes).toHaveLength(1);
172-
expect(Object.keys(probes[0].where)).toEqual(['id']);
173-
expect([...probes[0].where.id.$in].sort()).toEqual(store.map((r) => r.id).sort());
190+
expect(Object.keys(probes[0].where!)).toEqual(['id']);
191+
expect([...(probes[0].where!.id as { $in: string[] }).$in].sort()).toEqual(store.map((r) => r.id).sort());
174192
expect(probes[0].limit).toBe(store.length);
175193
});
176194

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,17 @@
1111

1212
import { describe, it, expect, vi } from 'vitest';
1313
import { runImport, type ImportProtocolLike } from './import-runner';
14+
15+
/**
16+
* [#16952] The doubles below are annotated FROM the exported declaration
17+
* (`ImportProtocolLike`), never from a hand-written restatement of the shape
18+
* the runner happens to send. A local parameter annotation was one of the
19+
* three non-authoritative places this card converged: it froze a dialect no
20+
* compiler held anyone to, so it kept compiling — and kept passing — after the
21+
* runner moved to another one. ⛔ Never widen these back to an inline object
22+
* type; that re-opens the seam.
23+
*/
24+
type FindArgs = Parameters<ImportProtocolLike['findData']>[0];
1425
import type { ExportFieldMeta } from './export-format.js';
1526

1627
// `parent` is a lookup back to this same object (a category tree).
@@ -44,8 +55,8 @@ function makeProtocol(seed: Array<Record<string, any>> = []) {
4455
}));
4556
// [#16638] Reads the CANONICAL `where` the runner sends. ⛔ No `?? {}`: an
4657
// absent filter must throw here, never degrade into a match-everything probe.
47-
const findData = vi.fn(async (args: { query: { where: Record<string, any> } }) => {
48-
const filter = args.query.where;
58+
const findData = vi.fn(async (args: FindArgs) => {
59+
const filter = args.query!.where!;
4960
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; }));
5061
});
5162
const p: ImportProtocolLike = { findData, createData: vi.fn(), updateData: vi.fn(), createManyData };

0 commit comments

Comments
 (0)