Skip to content

Commit 4ac12ef

Browse files
os-zhuangclaude
andauthored
fix(spec,lint): a formula field in searchableFields is refused loudly (#6674) (#7103)
* fix(spec,lint): a formula field in searchableFields is refused loudly (#6674) #4254 closed the fail-open on the unknown-name axis. The same shape survived one axis over, on names that are perfectly real: the declared branch of `resolveSearchFieldResolution` filtered by EXISTENCE only, so a `formula` field declared in `searchableFields` entered the allowed set — and the #4254 ingress gate, which reads that same set, accepted it for exactly that reason. A formula value is computed on read and no driver materializes a column for it, so the `$contains` the engine expands `$search` into has nothing to scan. Measured: 0 rows on driver-memory, 0 rows WITH NO ERROR on driver-sql. The declaration read as search coverage and delivered none. - spec (the deciding face): the declared branch filters on existence AND scannability; new `SEARCH_VIRTUAL_TYPES` / `isVirtualSearchField` are the one judgment resolution, gate and linter share. The resolution stays non-throwing — internal callers never pass an ingress, which is why #4254 put the loudness at the ingress. - metadata-protocol: 400 INVALID_FIELD under its own reason, split out before the declared/auto branch because both of those messages are wrong for it. - lint: a build error on the object's own set as well as a view's narrowing, under the existing `searchable-field-unsearchable` rule. The storage-not-taste carve-out is kept and pinned by controls in all three packages: a declared `json` / `lookup` column is still executed, because it has a column and CAN match. Corpus sweep of objectstack + objectui + cloud: zero authored declarations affected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2 * chore(spec): regenerate the api-surface snapshot for the two #6674 exports `check:api-surface` (inside the TypeScript Type Check job) judged the public surface "0 breaking, 2 added" and asked for the snapshot. Both additions are intentional and are the design's centre: `SEARCH_VIRTUAL_TYPES` and `isVirtualSearchField` are the ONE judgment the spec resolution, the #4254 ingress gate and the linter all read, so that they cannot drift about which field types have a stored column. Snapshot delta is exactly those two names in api-surface/data.json. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2 * chore(spec): record the two #6674 exports in the #7090 export-origins baseline PR #7090 landed `check:export-origins` after this branch was cut, so the merge of `origin/main` brought a required gate the branch had never satisfied: the `data` shard was stale for the two exports this PR adds. `pnpm --filter @objectstack/spec gen:export-origins` — one shard rewritten, two added lines, both resolving to the single declaration site `src/data/search-fields.ts`. No re-homed origin and no second origin for an existing name, so this is not the #4411 dual-source trap the gate warns about; `check:dual-source-exports` agrees (0 new). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e5fd28c commit 4ac12ef

15 files changed

Lines changed: 694 additions & 29 deletions

File tree

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
"@objectstack/spec": patch
4+
"@objectstack/lint": patch
5+
---
6+
7+
fix(spec,lint): a virtual `formula` field in `searchableFields` is refused loudly, not admitted verbatim (#6674)
8+
9+
#4254 closed the fail-open on the unknown-name axis: a `$searchFields` entry the
10+
engine would not scan is `400 INVALID_FIELD`, never a silently widened search.
11+
The same shape survived one axis over, on names that are perfectly real.
12+
13+
The declared branch of `resolveSearchFieldResolution` filtered entries by
14+
EXISTENCE only, so a `formula` field declared in `searchableFields` entered the
15+
allowed set — and the ingress gate, which reads that same set, accepted it for
16+
exactly that reason. Measured on `origin/main`:
17+
18+
```
19+
AUTO: {"allowed":["name","project_name"],"source":"auto"} formula excluded
20+
DECL-FORMULA: {"allowed":["name","project_name_formula"],"source":"declared"} admitted verbatim
21+
?search=Apollo&searchFields=project_name_formula -> 200, 0 rows silent
22+
```
23+
24+
Zero rows is the defect. A formula value is computed on read and no driver
25+
materializes a column for it (`driver-sql` `fieldHasColumn`, driver-turso's
26+
"Virtual — no column"), so the `$contains` the engine expands `$search` into has
27+
nothing to scan: 0 rows on driver-memory (the property is absent from the stored
28+
row) and 0 rows WITH NO ERROR on driver-sql/better-sqlite3. The declaration read
29+
as search coverage and delivered none.
30+
31+
- **`@objectstack/spec` — the deciding face.** The declared branch now filters on
32+
existence AND scannability: an entry naming a virtual field is not admitted.
33+
New exports `SEARCH_VIRTUAL_TYPES` (exactly `formula`, pinned) and
34+
`isVirtualSearchField` — one judgment, so the resolution, the gate and the
35+
linter cannot drift about which types have a column. The resolution itself
36+
stays non-throwing: it is consulted on every search by internal callers that
37+
never pass an ingress, which is why #4254 put the loudness at the ingress.
38+
- **`@objectstack/metadata-protocol``400 INVALID_FIELD` with its own reason.**
39+
Split out before the declared/auto branch, because both of those messages are
40+
wrong for it: "outside the declared set" is false when the entry IS in the
41+
list, and the auto-default's "declare `searchableFields` to choose the
42+
searchable set" would instruct the author to write the declaration being
43+
refused. The new message names the field, its type, that the value is computed
44+
on read and never stored, and the fix (mirror onto a stored text field).
45+
- **`@objectstack/lint` — a build error at authoring time**, on the object's own
46+
`searchableFields` as well as a view's narrowing, under the existing
47+
`searchable-field-unsearchable` rule (no new rule id). This narrows the
48+
canonical surface, which #4830 had deliberately left existence-only.
49+
50+
The carve-out that made canonical existence-only is deliberately KEPT and pinned
51+
by controls in all three packages: the dividing line is STORAGE, not search
52+
quality. A `json` or `lookup` column declared in `searchableFields` is still the
53+
author's choice and still executed — a `$contains` over the stored JSON text or
54+
the stored foreign key. Narrow and rarely useful, but a scan that CAN match, so
55+
it is neither a 400 nor a finding. Only "there is no column at all" is refused.
56+
57+
**Compatibility.** A corpus sweep of this repo plus `objectui` and `cloud` found
58+
ZERO authored `searchableFields` naming a formula-typed field, so nothing in the
59+
tree changes verdict. For an already-published object that does carry one:
60+
loading is unaffected (no schema-parse change — `searchableFields` is still
61+
`z.array(z.string())`, this is a resolution and enforcement rule); a plain
62+
`?search=` keeps returning the SAME rows, because the dropped entry matched none
63+
of them; only a request that NAMES the formula field flips from `200` with no
64+
rows to `400 INVALID_FIELD` — including objectui's list search, which echoes the
65+
declaration verbatim. An object whose `searchableFields` is ENTIRELY formula
66+
entries filters to empty and falls through to the auto-default, exactly as an
67+
all-stale declaration has since #4254; the linter reports the declaration rather
68+
than leaving that swap silent.

content/docs/data-modeling/schema-design.mdx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,13 @@ by the auto-default anyway; declare the set explicitly when you want to pin it.
140140
`$contains` predicate against one has nothing to scan (the SQL driver would emit
141141
a `WHERE` over a column that does not exist). A CEL formula also only reads this
142142
record's own fields (`record.<field>`), so it cannot fetch the related title in
143-
the first place. Nothing catches the mistake for you — `searchableFields` admits
144-
any field the object declares, so a formula entry passes both lint and the
145-
ingress gate and then just never matches.
143+
the first place.
144+
145+
Since #6674 the mistake is **refused rather than silent**: a `formula` entry in
146+
any `searchableFields` — the object's own set included — is an `os validate`
147+
error (`searchable-field-unsearchable`), and a request naming one is `400
148+
INVALID_FIELD`. It used to clear both and then match nothing, which read as
149+
search coverage and delivered none.
146150
</Callout>
147151

148152
**Keeping the mirror fresh.** A mirror is denormalized data, only as current as

content/docs/references/data/object.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ const result = ApiMethod.parse(data);
137137
| **highlightFields** | `string[]` | optional | [ADR-0085] Ordered most-important fields; first entry wins where only one fits. Drives default columns, cards, previews, detail highlight strip. Renamed from compactLayout. |
138138
| **stageField** | `string \| false` | optional | [ADR-0085] Lifecycle stage field (linear/ordered), or false to declare the status field non-linear and suppress stage heuristics. Absent = heuristic detection allowed. |
139139
| **listViews** | `Record<string, { name?: string; label?: string \| Record<string, string>; type?: Enum<'grid' \| 'kanban' \| 'gallery' \| 'calendar' \| 'timeline' \| 'gantt' \| 'map' \| 'chart' \| 'tree'>; data?: object \| … +3 more; … }>` | optional | Built-in named list views (segmented tabs) shipped with the object schema — "views" mode, dropdown userFilters allowed, no page-only tabs (ADR-0047) |
140-
| **searchableFields** | `string[]` | optional | Fields the `$search` query matches against (ADR-0061). Canonical default for the record picker, list quick-search and global search; views may narrow it. When unset, search auto-defaults to the name/title field plus short-text fields. |
140+
| **searchableFields** | `string[]` | optional | Fields the `$search` query matches against (ADR-0061). Canonical default for the record picker, list quick-search and global search; views may narrow it. When unset, search auto-defaults to the name/title field plus short-text fields. Entries must name a STORED column: a virtual `formula` field is computed on read and materializes no column, so searching it can never match and it is refused (#6674) — mirror the value onto a stored text field and declare that. |
141141
| **enable** | `{ trackHistory?: boolean; searchable?: boolean; apiEnabled?: boolean; apiMethods?: Enum<'get' \| 'list' \| 'create' \| 'update' \| 'delete' \| 'bulk'>[]; … }` | optional | Enabled system features modules |
142142
| **sharingModel** | `Enum<'private' \| 'public_read' \| 'public_read_write' \| 'controlled_by_parent'>` | optional | Org-Wide Default record visibility (OWD) for INTERNAL users. Canonical four only (legacy aliases removed, ADR-0090 D4): private (owner-only) \| public_read (everyone reads, owner writes) \| public_read_write (everyone reads+writes) \| controlled_by_parent (derived from the master record). A CUSTOM object that omits this resolves to private at runtime (ADR-0090 D1). |
143143
| **externalSharingModel** | `Enum<'private' \| 'public_read' \| 'public_read_write' \| 'controlled_by_parent'>` | optional | [ADR-0090 D11] OWD for external (portal/partner) principals. Defaults to private; must be &lt;= sharingModel in openness. |

content/docs/ui/views.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ all.
160160
| key omitted, or `searchableFields: []` | clean | scans the object's full allowed set |
161161
| a renamed / mistyped column, or a dotted path | `searchable-field-unknown` | `400 INVALID_FIELD` |
162162
| a real column outside the allowed set | `searchable-field-unsearchable` | `400 INVALID_FIELD` |
163+
| a virtual `formula` columnno stored column to scan (#6674) | `searchable-field-unsearchable` | `400 INVALID_FIELD` |
163164

164165
Both diagnostics are **errors**, not warnings`os validate` fails the build.
165166
The object's own set, and the stored-mirror prescription, are covered under

packages/lint/src/validate-searchable-fields.test.ts

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,3 +647,113 @@ describe('validateSearchableFields — objectstack-ui SKILL.md parity (#6675)',
647647
expect(refused[0].message).toContain("of type 'lookup', which 'search' cannot scan");
648648
});
649649
});
650+
651+
/**
652+
* [#6674] A virtual `formula` entry — the one check that runs on the object's
653+
* OWN set as well as on a view's narrowing.
654+
*
655+
* The card's shape: the entry names a real field, so the existence check passes
656+
* it; the runtime's declared branch admitted it verbatim; and the search then
657+
* matched nothing, because a formula value is computed on read and no driver
658+
* materializes a column for it (0 rows on driver-memory, 0 rows WITH NO ERROR on
659+
* driver-sql). Declared coverage, zero delivery — the fail-open #4254 closed on
660+
* the unknown-name axis, surviving on the known-but-virtual one.
661+
*/
662+
describe('[#6674] validateSearchableFields — a virtual formula entry', () => {
663+
const accountFields = {
664+
name: { type: 'text' },
665+
billing_email: { type: 'email' },
666+
payload: { type: 'json' },
667+
account_id: { type: 'lookup', reference: 'crm_account' },
668+
display_label: { type: 'formula', expression: "record.name + ' · x'" },
669+
};
670+
671+
it("flags it on the OBJECT's own searchableFields — previously clean", () => {
672+
const findings = validateSearchableFields({
673+
objects: [
674+
{
675+
name: 'crm_account',
676+
fields: accountFields,
677+
searchableFields: ['name', 'display_label'],
678+
},
679+
],
680+
});
681+
682+
expect(findings).toHaveLength(1);
683+
expect(findings[0].rule).toBe(SEARCHABLE_FIELD_UNSEARCHABLE);
684+
expect(findings[0].severity).toBe('error');
685+
expect(findings[0].path).toBe('objects[0].searchableFields[1]');
686+
expect(findings[0].where).toBe('object "crm_account"');
687+
expect(findings[0].message).toContain("is a virtual 'formula' field");
688+
expect(findings[0].message).toContain('computed on read and never stored');
689+
// The fix is a STORED mirror — the same prescription #6673 put on the
690+
// neighbouring hints, and the only one that can work here.
691+
expect(findings[0].hint).toContain('stored text field');
692+
expect(findings[0].hint).toContain('400 INVALID_FIELD');
693+
});
694+
695+
it('flags it on a list view narrowing too', () => {
696+
const findings = validateSearchableFields({
697+
objects: [
698+
{
699+
name: 'crm_account',
700+
fields: accountFields,
701+
listViews: { all: { type: 'grid', searchableFields: ['name', 'display_label'] } },
702+
},
703+
],
704+
});
705+
706+
expect(findings).toHaveLength(1);
707+
expect(findings[0].rule).toBe(SEARCHABLE_FIELD_UNSEARCHABLE);
708+
expect(findings[0].path).toBe('objects[0].listViews.all.searchableFields[1]');
709+
expect(findings[0].message).toContain("is a virtual 'formula' field");
710+
});
711+
712+
it('CONTROL — a json or lookup entry on the OBJECT\'s own set stays clean', () => {
713+
// The carve-out #4830 wrote down, deliberately preserved: the runtime's
714+
// declared branch executes those (a `$contains` over the stored JSON text /
715+
// the stored foreign key). Narrow and rarely useful, but a scan that CAN
716+
// match — flagging it would reject metadata the runtime accepts (ADR-0072
717+
// D1). If this control ever goes red, #6674 has quietly become "the declared
718+
// branch is type-filtered", which it is not.
719+
expect(
720+
validateSearchableFields({
721+
objects: [
722+
{
723+
name: 'crm_account',
724+
fields: accountFields,
725+
searchableFields: ['name', 'payload', 'account_id'],
726+
},
727+
],
728+
}),
729+
).toEqual([]);
730+
});
731+
732+
it('CONTROL — a stale entry on the object\'s own set keeps the #4254 message', () => {
733+
const findings = validateSearchableFields({
734+
objects: [
735+
{ name: 'crm_account', fields: accountFields, searchableFields: ['name', 'gone'] },
736+
],
737+
});
738+
739+
expect(findings).toHaveLength(1);
740+
expect(findings[0].rule).toBe(SEARCHABLE_FIELD_UNKNOWN);
741+
expect(findings[0].message).toContain('is not a field on object');
742+
});
743+
744+
it('an ALL-virtual declaration is reported, not silently swapped for the auto-default', () => {
745+
// The degenerate case: at runtime the declaration filters to empty and
746+
// resolution falls through to the auto-default, so the object silently
747+
// searches a set the author never wrote. The build error is what stops that
748+
// being invisible.
749+
const findings = validateSearchableFields({
750+
objects: [
751+
{ name: 'crm_account', fields: accountFields, searchableFields: ['display_label'] },
752+
],
753+
});
754+
755+
expect(findings).toHaveLength(1);
756+
expect(findings[0].path).toBe('objects[0].searchableFields[0]');
757+
expect(findings[0].message).toContain("is a virtual 'formula' field");
758+
});
759+
});

packages/lint/src/validate-searchable-fields.ts

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,25 @@
6060
* linter, gate and engine cannot drift apart (the same one-source move
6161
* #4254 made between gate and engine).
6262
*
63-
* The OBJECT's own `searchableFields` stays existence-only: the runtime's
64-
* declared branch filters by existence, never by type, so a json or lookup
65-
* column declared THERE is a choice the engine executes (a `$contains` over
66-
* the raw column), not a 400. Flagging it would reject metadata the runtime
67-
* accepts — the false finding that makes authors stop trusting the linter
68-
* (ADR-0072 D1).
63+
* 3. VIRTUALITY, on EVERY surface — the object's own set included
64+
* (`searchable-field-unsearchable`, #6674): a `formula` entry names a real
65+
* field, so check 1 passes it, and the runtime's declared branch admitted
66+
* it verbatim — but the value is computed on read with no stored column, so
67+
* the scan looks at nothing. Measured 0 rows on driver-memory and 0 rows
68+
* WITH NO ERROR on driver-sql. Since #6674 the spec resolution drops such
69+
* an entry and both enforcement faces refuse it by name.
70+
*
71+
* The OBJECT's own `searchableFields` stays existence-only OTHERWISE, and the
72+
* dividing line is STORAGE, not search quality: the runtime's declared branch
73+
* filters by existence and scannability, never by type taste, so a json or
74+
* lookup column declared THERE is a choice the engine executes (a `$contains`
75+
* over the stored JSON text / the stored foreign key) — narrow, rarely useful,
76+
* but a scan that CAN match, so it is neither a 400 nor a finding. Flagging
77+
* those would reject metadata the runtime accepts — the false finding that
78+
* makes authors stop trusting the linter (ADR-0072 D1). A virtual entry is the
79+
* opposite case: no column exists on any driver, so admitting it is the
80+
* fail-open #4254 closed one axis over, surviving on the known-but-virtual
81+
* axis.
6982
*
7083
* Three skips keep false positives near zero (ADR-0072 D1):
7184
*
@@ -93,6 +106,7 @@
93106

94107
import {
95108
resolveSearchFieldResolution,
109+
isVirtualSearchField,
96110
SEARCHABLE_TEXTUAL_TYPES,
97111
SEARCHABLE_ENUM_TYPES,
98112
SEARCH_AUTO_EXCLUDED_FIELDS,
@@ -124,8 +138,9 @@ export interface SearchableFieldFinding {
124138
* Which runtime judgment applies to the declaration being checked:
125139
*
126140
* - `'canonical'` — the object's own `searchableFields`. The runtime honors
127-
* any entry that exists (existence-filtered, never type-filtered), so only
128-
* existence is checked.
141+
* any entry that exists and has a stored column to scan (existence- and
142+
* scannability-filtered, never type-filtered), so existence and virtuality
143+
* are checked and nothing else (#6674).
129144
* - `'narrowing'` — a list view's `searchableFields` (metadata or react
130145
* surface). Clients echo it as the `$searchFields` override, which the
131146
* #4254 ingress gate intersects with the object's allowed set — entries the
@@ -354,6 +369,35 @@ export function checkSearchableFieldList(
354369
continue;
355370
}
356371

372+
// ── [#6674] Virtual entries — EVERY surface, canonical included ──
373+
//
374+
// The one check that is not view-level, because the runtime's declared
375+
// branch no longer executes it: a `formula` value is computed on read and
376+
// has no stored column, so the entry can never match wherever it is
377+
// declared. Judged by the spec's own predicate, so linter and runtime
378+
// cannot disagree about which types have a column.
379+
if (isVirtualSearchField(target.fields[name])) {
380+
const vtype = target.fields[name]?.type;
381+
findings.push({
382+
severity: 'error',
383+
rule: SEARCHABLE_FIELD_UNSEARCHABLE,
384+
where,
385+
path: `${path}[${i}]`,
386+
message:
387+
`${subject} entry "${name}" on object "${objectName}" is a virtual ` +
388+
`'${vtype}' field: its value is computed on read and never stored, so no ` +
389+
`driver materializes a column for 'search' to scan and the entry can never ` +
390+
`match. It reads as search coverage and delivers none — the runtime used to ` +
391+
`admit it verbatim because the declaration named it (#6674).`,
392+
hint:
393+
`Mirror the computed value onto a stored text field on "${objectName}" and ` +
394+
`declare that instead, or drop "${name}". At runtime the ingress gate now ` +
395+
`refuses this entry with 400 INVALID_FIELD, the same answer a stale entry ` +
396+
`gets (#4254).`,
397+
});
398+
continue;
399+
}
400+
357401
// ── Runtime admissibility (#4830) — view-level narrowings only ──
358402
if (!resolution || resolution.allowed.has(name)) continue;
359403
// ③ System column outside the allowed set: its runtime metadata is

0 commit comments

Comments
 (0)