Skip to content

Commit 7967133

Browse files
os-zhuangclaude
andauthored
feat(lint): catch a stale searchableFields entry at authoring time (#4254 follow-up) (#4328)
`searchableFields` is `z.array(z.string())` in both `object.zod.ts` and the list-view schema, so nothing ever checked that an entry resolves to anything. Rename a field and the old name stays behind — Zod-valid, shipped, pointing at a column that no longer exists. The engine tolerates it, which is what kept the drift invisible: `resolveSearchFields` filters the declaration down to fields that exist and says nothing. The tolerance fails in the direction nobody expects — some entries stale searches a NARROWER set than the object declares, and every entry stale empties the filtered set, falling through to the auto-default. A declaration whose whole purpose is to CHOOSE the searchable set ends up selecting one the author never wrote. It also stops being quiet downstream: clients echo the declaration verbatim as `$searchFields`, so once the REST read path validates that override (#4254) a stale entry becomes a 400 INVALID_FIELD on every list search for that object. New rule `searchable-field-unknown`, appended to REFERENCE_INTEGRITY_RULES so it runs on `os validate` / `os lint` / `os compile` with no CLI edit. Covers the object's own ADR-0061 declaration and the list views that narrow it. Gating, unlike the advisory field-existence rules: those describe a consumer that skips an unknown name and renders the rest; this one either selects the wrong set or refuses the request. Existence only — a field that exists but is an odd search target is a choice, not drift. Skips an object this stack does not define, one with no authored field map, and registry-injected system columns (derived from the spec's FIELD_GROUP_SYSTEM_FIELDS + SystemFieldName rather than a sixth hand copy). Dotted paths ARE flagged, unlike in sibling rules: search matches the field map by exact string, so `owner_id.name` is dropped exactly like a typo. Verified against the shipped examples: showcase / crm / todo all validate clean, and injecting `email` into the showcase account object fails `os validate` with `objects[0].searchableFields[3]` and exit 1. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 07d812f commit 7967133

6 files changed

Lines changed: 711 additions & 0 deletions
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
feat(lint): a `searchableFields` entry naming no field is caught at authoring
6+
time, not at request time
7+
8+
`searchableFields` is `z.array(z.string())` in both `object.zod.ts` and the
9+
list-view schema, so nothing ever checked that an entry resolves to anything.
10+
Rename a field and the old name stays behind — Zod-valid, shipped, pointing at
11+
a column that no longer exists.
12+
13+
The engine tolerates it, which is exactly what kept the drift invisible:
14+
`resolveSearchFields` filters the declaration down to fields that exist
15+
(`searchableFields?.filter((f) => all[f])`) and says nothing. The tolerance
16+
fails in the direction nobody expects:
17+
18+
- **some entries stale**`$search` scans a NARROWER set than the object
19+
declares. Records that should match do not, and the response is
20+
indistinguishable from "no such record";
21+
- **every entry stale** → the filtered set is empty, so resolution falls
22+
through to the AUTO-DEFAULT (name/title + short-text fields). A declaration
23+
whose whole purpose is to CHOOSE the searchable set ends up selecting one the
24+
author never wrote — the "asked narrower, answered wider" inversion #4226
25+
closed on the projection axis.
26+
27+
It also stops being quiet downstream. Clients echo the declaration verbatim as
28+
the `$searchFields` override (objectui's list search sends
29+
`schema.searchableFields`), so once the REST read path validates that override
30+
against the object (#4254), a stale entry the engine had been silently skipping
31+
becomes a `400 INVALID_FIELD` on every list search for that object — a
32+
request-time break whose cause is an authoring typo made long before.
33+
34+
**New rule — `searchable-field-unknown` (gating).** Wired into
35+
`REFERENCE_INTEGRITY_RULES`, so it runs on `os validate`, `os lint` and
36+
`os compile` with no CLI edit. It covers the object's own ADR-0061 declaration
37+
and the list views that narrow it (`objects[].listViews`, a `defineView`
38+
default `list`, and named `listViews`), resolving each entry against the bound
39+
object's declared fields.
40+
41+
`error`, not the advisory level the other field-existence rules use
42+
(`page-field-unknown`, `form-field-unknown`, `semantic-role-field-unknown` are
43+
all warnings). Those describe a consumer that SKIPS an unknown name and renders
44+
the rest; this describes a declaration that either selects the wrong set or
45+
refuses the request outright — the same call `validate-flow-template-paths`
46+
makes for a filter-position token, where the miss widens the query instead of
47+
shrinking the page.
48+
49+
Existence only: a field that exists but is an odd search target (a `json`
50+
column) is NOT flagged — an explicit `searchableFields` is authoritative, so
51+
declaring one is a choice, not drift. Three skips keep false positives near zero
52+
(ADR-0072 D1): an object this stack does not define, an object with no authored
53+
field map (external / datasource-introspected), and registry-injected system
54+
columns — the last derived from the spec's own `FIELD_GROUP_SYSTEM_FIELDS` and
55+
`SystemFieldName` rather than hand-copied, since this package already carries
56+
five slightly-different copies of that list.
57+
58+
Dotted paths are the one place this rule is stricter than its siblings. They
59+
skip `owner_id.name` because the query engine resolves the traversal; search
60+
does not — `resolveSearchFields` matches the field map by exact string, so a
61+
dotted entry is dropped exactly like a typo, and it is the spelling most likely
62+
borrowed from `select`/`sort`. It is flagged, with its own fix hint.

packages/lint/src/index.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,15 @@ export {
194194
} from './validate-object-references.js';
195195
export type { ObjectRefFinding, ObjectRefSeverity } from './validate-object-references.js';
196196

197+
export {
198+
validateSearchableFields,
199+
SEARCHABLE_FIELD_UNKNOWN,
200+
} from './validate-searchable-fields.js';
201+
export type {
202+
SearchableFieldFinding,
203+
SearchableFieldSeverity,
204+
} from './validate-searchable-fields.js';
205+
197206
export { validateActionNameRefs, ACTION_NAME_UNDEFINED } from './validate-action-name-refs.js';
198207
export type { ActionNameRefFinding, ActionNameRefSeverity } from './validate-action-name-refs.js';
199208

packages/lint/src/reference-integrity-suite.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ describe('reference-integrity suite — membership', () => {
1616
it('holds exactly the reference-resolution rules, in report order', () => {
1717
expect(REFERENCE_INTEGRITY_RULES.map((r) => r.name)).toEqual([
1818
'validateObjectReferences',
19+
'validateSearchableFields',
1920
'validateActionNameRefs',
2021
'validatePageFieldBindings',
2122
'validateChartBindings',
@@ -48,6 +49,10 @@ describe('reference-integrity suite — every member actually runs', () => {
4849
{
4950
name: 'crm_lead',
5051
fields: { name: { type: 'text', label: 'Name' } },
52+
// validateSearchableFields: `budget` is not a field on crm_lead, so the
53+
// ADR-0061 declaration is stale — the engine drops it and searches a
54+
// narrower set than the object declares.
55+
searchableFields: ['name', 'budget'],
5156
permissions: {},
5257
},
5358
],
@@ -144,6 +149,7 @@ describe('reference-integrity suite — every member actually runs', () => {
144149
const rules = new Set(findings.map((f) => f.rule));
145150

146151
expect(rules).toContain('object-reference-unknown');
152+
expect(rules).toContain('searchable-field-unknown');
147153
expect(rules).toContain('action-name-undefined');
148154
expect(rules).toContain('page-field-unknown');
149155
expect(rules).toContain('chart-measure-unknown');

packages/lint/src/reference-integrity-suite.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,15 @@
3232
* severities (see that module: a filter-position miss gates, every other
3333
* position advises), which is why the suite's contract is severity-agnostic.
3434
*
35+
* `validateSearchableFields` is a member on the same reading, one layer in: an
36+
* ADR-0061 `searchableFields` entry is a field name written in metadata,
37+
* resolved against the object's own declared fields. It gates (`error`) because
38+
* the engine's tolerance for a stale entry — silently filtering it out — either
39+
* narrows the searched set below what the object declares or, once every entry
40+
* is stale, falls through to the auto-default and searches a set the author
41+
* never wrote. See that module for why the other field-existence rules stay
42+
* advisory and this one does not.
43+
*
3544
* Rules that check SHAPE rather than reference (view containers, responsive
3645
* styles, seed replay safety, seed state machines, seed/security posture) stay
3746
* out — they answer a different question and have their own call sites.
@@ -46,6 +55,7 @@
4655
*/
4756

4857
import { validateObjectReferences } from './validate-object-references.js';
58+
import { validateSearchableFields } from './validate-searchable-fields.js';
4959
import { validateActionNameRefs } from './validate-action-name-refs.js';
5060
import { validatePageFieldBindings } from './validate-page-field-bindings.js';
5161
import { validateChartBindings } from './validate-chart-bindings.js';
@@ -91,6 +101,7 @@ export interface ReferenceIntegrityRule {
91101
*/
92102
export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [
93103
{ name: 'validateObjectReferences', run: validateObjectReferences },
104+
{ name: 'validateSearchableFields', run: validateSearchableFields },
94105
{ name: 'validateActionNameRefs', run: validateActionNameRefs },
95106
{ name: 'validatePageFieldBindings', run: validatePageFieldBindings },
96107
{ name: 'validateChartBindings', run: validateChartBindings },

0 commit comments

Comments
 (0)