Skip to content

Commit f566a1d

Browse files
committed
fix(spec): the $search auto field set's lead ORDERS the set, it must not admit one (#4483)
`autoDefaultFields` filtered every field through three exclusions (`SEARCH_AUTO_EXCLUDED_FIELDS`, `hidden`, unsearchable type) and then prepended the display/name/title field on an EXISTENCE check alone — so the exclusions did not hold for whichever field happened to lead, and the module's own "system / audit / heavy fields never auto-included" invariant was false. Not a contrived shape: ADR-0079's `provisionPrimary(schema, { synthesize: false })` designates `nameField` at registration, and on a table whose only textual column IS the primary key (system tables, junction tables, append-only logs) it designates `id`. `$search` then expanded to `{ id: { $contains: term } }` — a substring scan over the primary key, returning a narrow and semantically wrong row set. It loosened a second layer too: `resolveSearchFieldResolution` is also the #4254 REST ingress gate's arbiter for "would the engine actually scan this field", so with `id` in `allowed` a `$searchFields=id` override was ACCEPTED rather than refused. The lead's job is to put the primary title FIRST, never to admit it, so it is now chosen from the already-filtered set. An excluded / hidden / unsearchable display field simply does not lead and the set is unchanged; an eligible one still leads, so the ordering intent is intact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD
1 parent 0f9faa2 commit f566a1d

2 files changed

Lines changed: 146 additions & 4 deletions

File tree

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import {
5+
resolveSearchFieldResolution,
6+
resolveSearchFields,
7+
SEARCH_AUTO_EXCLUDED_FIELDS,
8+
} from './search-fields';
9+
10+
// ---------------------------------------------------------------------------
11+
// [#4483] The auto-default's "lead" field ORDERS the set; it must not ADMIT one.
12+
//
13+
// `autoDefaultFields` filters every field through three exclusions, then used to
14+
// prepend the display/name/title field on an EXISTENCE check alone — so the
15+
// exclusions did not hold for whichever field happened to lead. The regression
16+
// this pins is not hypothetical: ADR-0079 designates `nameField` at
17+
// registration, and on a table whose only textual column is the primary key it
18+
// designates `id`, turning `$search` into a substring scan over the PK.
19+
// ---------------------------------------------------------------------------
20+
describe('[#4483] $search auto field set — lead orders, never admits', () => {
21+
const pkOnly = {
22+
id: { type: 'text' },
23+
amount: { type: 'number' },
24+
};
25+
26+
it('excludes `id` with no display field (the already-correct baseline)', () => {
27+
expect(resolveSearchFieldResolution({ fields: pkOnly })).toEqual({
28+
allowed: [],
29+
source: 'auto',
30+
});
31+
});
32+
33+
it('a displayField on the exclusion list does NOT re-enter the set', () => {
34+
// Pre-#4483 this returned `{ allowed: ['id'] }`.
35+
expect(resolveSearchFieldResolution({ fields: pkOnly, displayField: 'id' })).toEqual({
36+
allowed: [],
37+
source: 'auto',
38+
});
39+
});
40+
41+
it('every SEARCH_AUTO_EXCLUDED_FIELDS member stays out even as displayField', () => {
42+
for (const excluded of SEARCH_AUTO_EXCLUDED_FIELDS) {
43+
const { allowed } = resolveSearchFieldResolution({
44+
fields: { [excluded]: { type: 'text' }, title: { type: 'text' } },
45+
displayField: excluded,
46+
});
47+
expect(allowed, `'${excluded}' leaked into the auto set as displayField`).toEqual(['title']);
48+
}
49+
});
50+
51+
it('a hidden display field does not lead and does not enter', () => {
52+
const { allowed } = resolveSearchFieldResolution({
53+
fields: { name: { type: 'text', hidden: true }, subject: { type: 'text' } },
54+
displayField: 'name',
55+
});
56+
expect(allowed).toEqual(['subject']);
57+
});
58+
59+
it('a display field of an unsearchable TYPE does not enter', () => {
60+
const { allowed } = resolveSearchFieldResolution({
61+
fields: { avatar: { type: 'image' }, subject: { type: 'text' } },
62+
displayField: 'avatar',
63+
});
64+
expect(allowed).toEqual(['subject']);
65+
});
66+
67+
it('the `name` / `title` bypasses are gated by the same predicate', () => {
68+
// Both exist but are unsearchable — neither may lead nor enter.
69+
const { allowed } = resolveSearchFieldResolution({
70+
fields: {
71+
name: { type: 'json' },
72+
title: { type: 'vector' },
73+
subject: { type: 'text' },
74+
},
75+
});
76+
expect(allowed).toEqual(['subject']);
77+
});
78+
79+
it('an ELIGIBLE display field still leads — the ordering intent is intact', () => {
80+
const { allowed } = resolveSearchFieldResolution({
81+
fields: {
82+
code: { type: 'text' },
83+
subject: { type: 'text' },
84+
stage: { type: 'select' },
85+
},
86+
displayField: 'subject',
87+
});
88+
expect(allowed[0]).toBe('subject');
89+
expect(new Set(allowed)).toEqual(new Set(['subject', 'code', 'stage']));
90+
});
91+
92+
it('falls back to `name`, then `title`, for the lead position', () => {
93+
expect(
94+
resolveSearchFieldResolution({
95+
fields: { code: { type: 'text' }, name: { type: 'text' } },
96+
}).allowed[0],
97+
).toBe('name');
98+
expect(
99+
resolveSearchFieldResolution({
100+
fields: { code: { type: 'text' }, title: { type: 'text' } },
101+
}).allowed[0],
102+
).toBe('title');
103+
});
104+
105+
it('a declared `searchableFields` list is unaffected by the lead rule', () => {
106+
// `declared` is the author's explicit choice and bypasses the auto-default
107+
// entirely — including its exclusions. Pinned so the fix is not read as
108+
// narrowing the declared path too.
109+
expect(
110+
resolveSearchFieldResolution({ fields: pkOnly, searchableFields: ['id'], displayField: 'id' }),
111+
).toEqual({ allowed: ['id'], source: 'declared' });
112+
});
113+
114+
it('the #4254 ingress gate no longer admits `$searchFields=id`', () => {
115+
// `resolveSearchFields` intersects the override with `allowed`; with `id`
116+
// out of `allowed` the override matches nothing and cannot widen the scan.
117+
expect(resolveSearchFields({ fields: pkOnly, displayField: 'id', requestedFields: 'id' }))
118+
.toEqual([]);
119+
});
120+
});

packages/spec/src/data/search-fields.ts

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,32 @@ function autoDefaultFields(fields: Record<string, SearchFieldMeta>, displayField
7272
if (SEARCH_AUTO_EXCLUDED_TYPES.has(t)) return false;
7373
return SEARCHABLE_TEXTUAL_TYPES.has(t) || SEARCHABLE_ENUM_TYPES.has(t);
7474
});
75-
// Lead with the display/name field when present.
76-
const lead = displayField && fields[displayField] ? displayField
77-
: fields.name ? 'name'
78-
: fields.title ? 'title'
75+
// Lead with the display/name field — ORDERING ONLY (#4483).
76+
//
77+
// `lead` used to be picked by EXISTENCE (`fields[displayField]`), then
78+
// prepended unconditionally, so it re-entered the set after the three
79+
// exclusions above had already rejected it. That made
80+
// `SEARCH_AUTO_EXCLUDED_FIELDS` — whose contract is "never auto-included" —
81+
// untrue for whichever field happened to be the display field, and the case
82+
// is not contrived: ADR-0079's `provisionPrimary(schema, { synthesize: false })`
83+
// designates `nameField` at registration, and on a table whose only textual
84+
// column IS the primary key (system tables, junction tables, append-only
85+
// logs) it designates `id`. `$search` then expanded to
86+
// `{ id: { $contains: <term> } }` — a substring scan over the primary key.
87+
//
88+
// It also loosened the #4254 REST ingress gate one layer up, which asks this
89+
// same resolution whether a `$searchFields` override names a field the engine
90+
// would actually scan: with `id` in `allowed`, `$searchFields=id` was
91+
// ACCEPTED instead of refused.
92+
//
93+
// The lead's job is to put the primary title FIRST, never to admit it, so it
94+
// is now chosen from `names` — the already-filtered set. A display field that
95+
// is excluded, hidden or of an unsearchable type simply does not lead, and
96+
// the set is unchanged.
97+
const eligible = (f: string | undefined): f is string => !!f && names.includes(f);
98+
const lead = eligible(displayField) ? displayField
99+
: eligible('name') ? 'name'
100+
: eligible('title') ? 'title'
79101
: undefined;
80102
if (!lead) return names;
81103
return [lead, ...names.filter((f) => f !== lead)];

0 commit comments

Comments
 (0)