Skip to content

Commit 596b462

Browse files
os-zhuangclaude
andauthored
fix(spec): EngineQueryOptionsSchema.search accepts the canonical bare query string (#7178) (#7249)
* fix(spec): EngineQueryOptionsSchema.search accepts the canonical bare query string (#7178) `BaseQuerySchema.search` (query.zod.ts, hence QueryAST, hence DriverQuery) has been `z.union([z.string(), FullTextSearchSchema])` since its own drift repair, with a doc comment saying why: the bare string IS the canonical Tier-1 contract (ADR-0061 D1), it is what every surface sends, and it is what the dogfood HTTP proof pins. `EngineQueryOptionsSchema.search` — the options type of IDataEngine.find/findOne — declared the structured form only. The runtime never agreed with that narrowing: `normalizeSearch` in objectql/src/search-filter.ts opens with `if (typeof raw === 'string')`, and protocol-data.test.ts asserts the protocol layer hands the engine a bare string. So the type forbade what the engine serves, and callers paid the standard price: `as any` on the query argument, which does not suppress `search` alone — it switches off checking for where/orderBy/fields in the same literal. This schema is not `.strict()`, so an unknown key there is silently dropped; the cast the divergence forced was precisely the cast check:query-options-erasure exists to stop. Same-family drift REPAIR, not a new dialect — the identical fix BaseQuerySchema.search already carries. On the query side it surfaced as a validation failure when #3899 started validating request bodies; here it surfaced as TS2345 when #6231 retyped DatabaseLoader's read helpers to DriverQuery and the engine branch alone refused to compile. Consumer census before landing (the card's own guard): every site reading object-form members off an engine-options `search` already narrows with `typeof` — engine.ts's `$search` expansion, search-filter.ts's normalizeSearch, and metadata-protocol/protocol.ts's searchFields ingress gate. No consumer needed a guard added and none changes behavior. `count` is untouched: EngineCountOptionsSchema declares no `search` key at all. With the schemas agreed, the casts the divergence forced are deleted — DatabaseLoader's three engine-branch `as any` (real where/orderBy/fields checking recovered on the metadata main read path) and the seven in engine-findone-contract.test.ts that were passing the canonical spelling. query-options-erasure-baseline.json ratcheted down accordingly (the loader file leaves the grandfather list entirely; test surface 256 -> 249). Closes #7178 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ymfZNzYCQpoptQ2sHbwU3 * docs(kernel): the hand-written IDataEngine contract page shows search's canonical spelling (#7178) `content/docs/kernel/contracts/data-engine.mdx` hand-mirrors the `EngineQueryOptions` interface and declared `search?: FullTextSearch`. That was true of the schema until this branch widened it; leaving it makes the hand-written face say the opposite of the generated face (`references/data/data-engine.mdx`, regenerated in the previous commit) about the same key — the shape that costs AI authors the most. Surfaced by the PR's own docs-drift advisory. It is the ONLY hand-written page in the repo carrying this declaration (`grep 'search?: FullTextSearch'` over content/docs: one hit); the other pages the advisory lists reach @objectstack/spec by package-level fan-out and make no claim about this key. The query-side pages (`objectql/query-syntax`, `data-modeling/queries`) already document the union, since `BaseQuerySchema.search` has carried it since its own repair. Deliberately NOT fixed here: the same block omits `searchFields` entirely. That is #7170's defect (triage deduped it as distinct) and belongs to that card, not this one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ymfZNzYCQpoptQ2sHbwU3 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 162036d commit 596b462

8 files changed

Lines changed: 153 additions & 26 deletions

File tree

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/metadata": patch
4+
---
5+
6+
fix(spec): `EngineQueryOptionsSchema.search` accepts the bare query string ADR-0061 D1 calls canonical (#7178)
7+
8+
Two sibling schemas in `packages/spec` described the same key and disagreed.
9+
`BaseQuerySchema.search` (`query.zod.ts`, hence `QueryAST`, hence `DriverQuery`)
10+
has been `z.union([z.string(), FullTextSearchSchema])` since its own drift
11+
repair, with a doc comment saying why: the bare string **is** the canonical
12+
Tier-1 contract (ADR-0061 D1 — "the client sends only the query text; the server
13+
resolves which fields to search from object metadata"), it is what every surface
14+
sends, and it is what the dogfood HTTP proof pins.
15+
`EngineQueryOptionsSchema.search` — the options type of `IDataEngine.find` /
16+
`findOne` — declared the structured `FullTextSearchSchema` **only**.
17+
18+
The runtime never agreed with that narrowing. `expandSearchOnAst`
19+
(`objectql/src/engine.ts`) reads `search` through `normalizeSearch`, whose first
20+
line is `if (typeof raw === 'string') return { query: raw }`, and
21+
`protocol-data.test.ts` asserts the protocol layer hands the engine a bare
22+
string. So the type forbade what the engine serves, and callers paid the
23+
standard price: `as any` on the query argument — which does not suppress
24+
`search` alone, it switches off checking for `where` / `orderBy` / `fields` in
25+
the same literal. Since this schema is not `.strict()`, an unknown key there is
26+
**silently dropped**, so the cast this divergence forced was precisely the cast
27+
`check:query-options-erasure` exists to stop.
28+
29+
This is the same-family drift REPAIR, not a new dialect — the identical fix
30+
`BaseQuerySchema.search` already carries, for the identical reason. On the query
31+
side the divergence surfaced as a validation failure the moment #3899 started
32+
validating request bodies; here it surfaced as a type error, when #6231 retyped
33+
`DatabaseLoader`'s read helpers to `DriverQuery` and the **engine** branch alone
34+
refused to compile (TS2345 — `DriverQuery` not assignable to
35+
`EngineQueryOptionsParsed`, purely because of `search`; nothing else differs).
36+
37+
Consumer census before landing, per the card's own guard: every site that reads
38+
object-form members off an engine-options `search` already narrows with `typeof`
39+
`engine.ts` (`typeof raw === 'object' ? raw?.fields : undefined`),
40+
`search-filter.ts` `normalizeSearch`, and `metadata-protocol/protocol.ts`'s
41+
`searchFields` ingress gate. No consumer needed a guard added, and none changes
42+
behavior: they were all written for the union already. `count` is untouched —
43+
`EngineCountOptionsSchema` declares no `search` key at all.
44+
45+
With the schemas agreed, the casts the divergence forced are deleted:
46+
`DatabaseLoader`'s three engine-branch `as any` (`_find` / `_findOne` /
47+
`_count`), which restores real `where` / `orderBy` / `fields` checking on the
48+
metadata main read path, and the seven `as any` in
49+
`engine-findone-contract.test.ts` that were passing the canonical spelling.
50+
`scripts/query-options-erasure-baseline.json` is ratcheted down accordingly.

content/docs/kernel/contracts/data-engine.mdx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,9 @@ interface EngineQueryOptions {
110110
limit?: number; // LIMIT
111111
offset?: number; // OFFSET
112112
top?: number; // Alias for limit (OData compat)
113-
search?: FullTextSearch; // Full-text search
113+
search?: string | FullTextSearch; // Full-text search — the bare query text
114+
// is canonical (ADR-0061 D1); the object
115+
// form carries the Tier-2 knobs (#7178)
114116
expand?: Record<string, QueryAST>; // Recursive relation loading
115117
context?: ExecutionContext; // Identity, tenant, transaction — any subset
116118
}

content/docs/references/data/data-engine.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@ QueryAST-aligned query options for IDataEngine.find() operations
519519
| **offset** | `number` | optional | |
520520
| **top** | `number` | optional | |
521521
| **cursor** | `never` | optional | [REMOVED] `query.cursor` was removed in @objectstack/spec 17 (#4286, ADR-0049) — no driver ever implemented keyset pagination, so the cursor was accepted and ignored and every page came back identical (a caller looping "until hasMore is false" never terminates). Delete the key; `QueryBuilder.cursor()` was removed with it. Express the keyset as an ordinary `where` predicate on your sort key — `where: { created_at: { $gt: last.created_at } }` with the matching `orderBy` — which every driver executes with canonicalised comparands. A first-class cursor, if ever built, will be a response-minted opaque token, not this caller-built record. |
522-
| **search** | `{ query: string; fields?: string[]; fuzzy: boolean; operator: Enum<'and' \| 'or'>; … }` | optional | |
522+
| **search** | `string \| { query: string; fields?: string[]; fuzzy: boolean; operator: Enum<'and' \| 'or'>; … }` | optional | |
523523
| **searchFields** | `string[]` | optional | |
524524
| **expand** | `Record<string, { object: string; fields?: string[]; where?: any; search?: string \| object; … }>` | optional | |
525525
| **distinct** | `never` | optional | [REMOVED] `query.distinct` was removed in @objectstack/spec 17 (#4286, ADR-0049 / ADR-0078) — no driver ever rendered SELECT DISTINCT; the flag's only observable effect was MIS-WIRED: the REST list path treated a distinct query as not countable and silently degraded `total`/`hasMore` to a page-local estimate while still returning duplicate rows. Delete the key; `QueryBuilder.distinct()` was removed with it, and the count suppression is gone (`total` is truthful again). For unique values of one column use the SQL/memory drivers' `distinct(object, field)` door; for unique combinations, `groupBy`; for a deduplicated count, the `count_distinct` aggregation. |

packages/metadata/src/loaders/database-loader.ts

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -225,34 +225,41 @@ export class DatabaseLoader implements MetadataLoader {
225225
// Internal CRUD helpers (driver vs engine)
226226
// ==========================================
227227

228-
// NOTE (#6231): the DRIVER branch below takes `query` unchanged and uncast —
229-
// `DriverQuery` is `Omit<QueryAST, 'object'>`, so the object name travels as
230-
// argument one only. The ENGINE branch still carries `as any`, and that cast
231-
// is NOT vestigial: `EngineQueryOptionsSchema.search` admits only the
232-
// structured `FullTextSearchSchema`, while `QueryAST.search` (hence
233-
// `DriverQuery`) also admits the bare query string that ADR-0061 D1 calls the
234-
// canonical Tier-1 spelling and that the engine actually serves. Until those
235-
// two schemas agree, `DriverQuery` is not assignable to
236-
// `EngineQueryOptionsParsed`. Tracked as #7178; do not "fix" it here by
237-
// narrowing the cast.
228+
// NOTE (#6231, closed out by #7178): BOTH branches below now take `query`
229+
// unchanged and uncast. `DriverQuery` is `Omit<QueryAST, 'object'>`, so the
230+
// object name travels as argument one only — that was always enough for the
231+
// driver branch. The ENGINE branch used to carry `as any`, for one reason:
232+
// `EngineQueryOptionsSchema.search` admitted only the structured
233+
// `FullTextSearchSchema`, while `QueryAST.search` (hence `DriverQuery`) also
234+
// admits the bare query string that ADR-0061 D1 calls the canonical Tier-1
235+
// spelling and that the engine actually serves, so `DriverQuery` was not
236+
// assignable to `EngineQueryOptionsParsed`. #7178 aligned the two schemas;
237+
// the casts are now genuinely vestigial and are gone, which restores real
238+
// `where`/`orderBy`/`fields` checking on the metadata main read path — this
239+
// schema is not `.strict()`, so an unknown key here is SILENTLY DROPPED
240+
// (`check:query-options-erasure`'s own rationale) and the erased type was
241+
// the only thing standing between a typo and that silence.
242+
//
243+
// If a future edit makes one of these stop compiling, the honest fix is to
244+
// reconcile the two schemas again — not to reinstate the cast.
238245

239246
private async _find(table: string, query: DriverQuery): Promise<Record<string, unknown>[]> {
240247
if (this.engine) {
241-
return this.engine.find(table, query as any);
248+
return this.engine.find(table, query);
242249
}
243250
return this.driver!.find(table, query);
244251
}
245252

246253
private async _findOne(table: string, query: DriverQuery): Promise<Record<string, unknown> | null> {
247254
if (this.engine) {
248-
return this.engine.findOne(table, query as any);
255+
return this.engine.findOne(table, query);
249256
}
250257
return this.driver!.findOne(table, query);
251258
}
252259

253260
private async _count(table: string, query: DriverQuery): Promise<number> {
254261
if (this.engine) {
255-
return this.engine.count(table, query as any);
262+
return this.engine.count(table, query);
256263
}
257264
return this.driver!.count(table, query);
258265
}

packages/objectql/src/engine-findone-contract.test.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -141,13 +141,13 @@ describe('findOne executes what it declares and refuses an empty predicate (#441
141141
// ── (1) `search` is a predicate on findOne, not a dropped key ────────
142142

143143
it('findOne({search}) matches the searched record, not the first row', async () => {
144-
const row = await engine.findOne('crm_account', { search: 'Two' } as any);
144+
const row = await engine.findOne('crm_account', { search: 'Two' });
145145
expect(row?.name).toBe('Two');
146146
expect(row?.id).not.toBe(one.id);
147147
});
148148

149149
it('the search term reaches the driver as a $contains predicate — `search` never does', async () => {
150-
await engine.findOne('crm_account', { search: 'Two' } as any);
150+
await engine.findOne('crm_account', { search: 'Two' });
151151
const { ast } = lastRead();
152152
expect(ast.where).toBeTruthy();
153153
expect(JSON.stringify(ast.where)).toContain('$contains');
@@ -160,13 +160,13 @@ describe('findOne executes what it declares and refuses an empty predicate (#441
160160
// `industry`, only the latter can hit — and a narrowed miss must be a
161161
// miss, not a fall-back to an unpredicated read.
162162
const narrowed = { searchFields: ['industry'] };
163-
expect(await engine.findOne('crm_account', { search: 'Two', ...narrowed } as any)).toBeNull();
164-
expect((await engine.findOne('crm_account', { search: 'Metals', ...narrowed } as any))?.id)
163+
expect(await engine.findOne('crm_account', { search: 'Two', ...narrowed })).toBeNull();
164+
expect((await engine.findOne('crm_account', { search: 'Metals', ...narrowed }))?.id)
165165
.toBe(two.id);
166166
});
167167

168168
it('find({search}) is unchanged — the expansion moved, it did not fork', async () => {
169-
const rows = await engine.find('crm_account', { search: 'Two' } as any);
169+
const rows = await engine.find('crm_account', { search: 'Two' });
170170
expect(rows.map((r: any) => r.name)).toEqual(['Two']);
171171
});
172172

@@ -176,7 +176,7 @@ describe('findOne executes what it declares and refuses an empty predicate (#441
176176
// "predicate resolved to empty" shape the issue names. Before #4419 the
177177
// forced `limit: 1` turned it into the object's first row.
178178
for (const term of ['', ' ']) {
179-
await expect(engine.findOne('crm_account', { search: term } as any))
179+
await expect(engine.findOne('crm_account', { search: term }))
180180
.rejects.toThrow(/selects no particular record/);
181181
}
182182
expect(reads).toHaveLength(0);
@@ -293,7 +293,7 @@ describe('findOne executes what it declares and refuses an empty predicate (#441
293293

294294
it('a miss is still null — the guard did not turn "not found" into an error', async () => {
295295
expect(await engine.findOne('crm_account', { where: { id: 'nope' } } as any)).toBeNull();
296-
expect(await engine.findOne('crm_account', { search: 'nope' } as any)).toBeNull();
296+
expect(await engine.findOne('crm_account', { search: 'nope' })).toBeNull();
297297
});
298298

299299
// ── (3) drift pin: every declared findOne option is executed ────────

packages/spec/src/data/data-engine.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
DataEngineRequestSchema,
2626
DroppedFieldsEventSchema,
2727
} from './data-engine.zod';
28+
import { QuerySchema } from './query.zod';
2829

2930
describe('DataEngineFilterSchema', () => {
3031
it('should accept simple key-value filter', () => {
@@ -376,6 +377,53 @@ describe('EngineQueryOptionsSchema', () => {
376377
expect(options.expand!.owner.object).toBe('user');
377378
});
378379

380+
// ── `search`: both spellings, canonical one first (#7178) ────────────
381+
382+
it('accepts the BARE query string — the canonical ADR-0061 D1 spelling (#7178)', () => {
383+
// This is the pin that was RED before #7178: the schema declared only the
384+
// structured form, so the spelling the executor actually serves, every
385+
// surface sends, and `BaseQuerySchema.search` already accepts was rejected
386+
// here — and every engine caller wanting it had to `as any` the whole query.
387+
const options = EngineQueryOptionsSchema.parse({ search: 'acme corp' });
388+
expect(options.search).toBe('acme corp');
389+
});
390+
391+
it('still accepts the structured FullTextSearch form — the Tier-2 knobs (#7178)', () => {
392+
const options = EngineQueryOptionsSchema.parse({
393+
search: { query: 'acme corp', fields: ['name', 'industry'] },
394+
});
395+
expect(typeof options.search).toBe('object');
396+
expect((options.search as { query: string }).query).toBe('acme corp');
397+
expect((options.search as { fields?: string[] }).fields).toEqual(['name', 'industry']);
398+
});
399+
400+
it('accepts search alongside searchFields, in both spellings (#7178)', () => {
401+
expect(EngineQueryOptionsSchema.parse({
402+
search: 'acme', searchFields: ['name'],
403+
}).searchFields).toEqual(['name']);
404+
expect(EngineQueryOptionsSchema.parse({
405+
search: { query: 'acme' }, searchFields: ['name'],
406+
}).searchFields).toEqual(['name']);
407+
});
408+
409+
it('rejects a search that is neither a string nor a FullTextSearch (#7178)', () => {
410+
// The union widens the accept face by exactly one spelling — it does not
411+
// open the key to anything.
412+
expect(() => EngineQueryOptionsSchema.parse({ search: 42 })).toThrow();
413+
expect(() => EngineQueryOptionsSchema.parse({ search: { fields: ['name'] } })).toThrow();
414+
});
415+
416+
it('matches BaseQuerySchema.search — the two sibling schemas agree (#7178)', () => {
417+
// The whole point of the repair: what QuerySchema accepts for `search`,
418+
// the engine options schema accepts too. `DriverQuery` (= Omit<QueryAST,
419+
// 'object'>) is assignable to `EngineQueryOptionsParsed` again because of
420+
// this, which is what lets `database-loader`'s engine branch drop its casts.
421+
for (const search of ['acme corp', { query: 'acme corp', fields: ['name'] }]) {
422+
expect(QuerySchema.parse({ object: 'crm_account', search })).toBeDefined();
423+
expect(EngineQueryOptionsSchema.parse({ search })).toBeDefined();
424+
}
425+
});
426+
379427
it('rejects the removed cursor/distinct keys with the query.* prescriptions (#4286)', () => {
380428
expect(() => EngineQueryOptionsSchema.parse({ cursor: { id: 'x' } }))
381429
.toThrow(/query\.cursor.*removed/s);

packages/spec/src/data/data-engine.zod.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,29 @@ export const EngineQueryOptionsSchema = lazySchema(() => BaseEngineOptionsSchema
115115
/** Keyset cursor — REMOVED (#4286); same tombstone as `QuerySchema.cursor`. */
116116
cursor: retiredKey(QUERY_CURSOR_REMOVED),
117117

118-
/** Full-text search configuration */
119-
search: FullTextSearchSchema.optional(),
118+
/**
119+
* Full-Text Search.
120+
*
121+
* The bare string IS the canonical Tier-1 contract (ADR-0061 D1: "the
122+
* client sends only the query text; the server resolves which fields to
123+
* search from object metadata") — it is what every surface sends, what the
124+
* engine's `$search` expansion actually serves, and what the dogfood HTTP
125+
* proof (`showcase-search.dogfood.test.ts`) pins. The structured
126+
* `FullTextSearchSchema` form remains for the declared Tier-2 knobs.
127+
*
128+
* The union is schema-side drift REPAIR, not a new dialect — the same
129+
* repair `BaseQuerySchema.search` (`query.zod.ts`) already carries, and for
130+
* the same reason: this schema declared only the object form while the
131+
* executor and the ADR's own conformance ledger served the string. Here the
132+
* divergence surfaced as a type error rather than a validation failure
133+
* (#7178): `DriverQuery` (= `Omit<QueryAST, 'object'>`, which inherits the
134+
* union) was not assignable to `EngineQueryOptionsParsed` purely because of
135+
* this key, so every engine caller wanting the canonical spelling had to
136+
* `as any` the whole query — switching off `where`/`orderBy`/`fields`
137+
* checking too, and, since this schema is not `.strict()`, arming exactly
138+
* the silent-key-drop that `check:query-options-erasure` exists to stop.
139+
*/
140+
search: z.union([z.string(), FullTextSearchSchema]).optional(),
120141

121142
/**
122143
* Fields the `search` expansion may match against — intersected with the

scripts/query-options-erasure-baseline.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
"packages/core/src/security/resolve-authz-context.ts": 1,
3636
"packages/metadata-protocol/src/protocol.ts": 6,
3737
"packages/metadata-protocol/src/seed-loader.ts": 3,
38-
"packages/metadata/src/loaders/database-loader.ts": 3,
3938
"packages/objectql/src/engine.ts": 9,
4039
"packages/plugins/plugin-approvals/src/approval-service.ts": 10,
4140
"packages/plugins/plugin-approvals/src/approver-org-scope.ts": 2,
@@ -51,6 +50,6 @@
5150
"packages/services/service-settings/src/settings-service.ts": 2
5251
},
5352
"testSurface": {
54-
"sites": 256
53+
"sites": 249
5554
}
5655
}

0 commit comments

Comments
 (0)