diff --git a/packages/lint/src/validate-searchable-fields.test.ts b/packages/lint/src/validate-searchable-fields.test.ts index 3ad01ad155..3b7849286a 100644 --- a/packages/lint/src/validate-searchable-fields.test.ts +++ b/packages/lint/src/validate-searchable-fields.test.ts @@ -1,6 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; +import { resolveSearchFields } from '@objectstack/spec/data'; import { validateSearchableFields, SEARCHABLE_FIELD_UNKNOWN, @@ -475,3 +476,167 @@ describe('validateSearchableFields — list views that narrow the set', () => { expect(findings).toEqual([]); }); }); + +/** + * [#6675] Skill-parity — `skills/objectstack-ui/SKILL.md` › "Toolbar Search + * (`searchableFields`, ADR-0061)" quotes this rule's two diagnostics verbatim + * and states three boundaries as fact. The skill ships to third parties via + * `npx skills add`, so a reader who follows it is following THIS code; if the + * wording or a verdict moves and nobody re-reads the skill, the published text + * teaches a rule the platform no longer has. + * + * The same reason `validate-rls-predicate-enforceability.test.ts` pins the RLS + * predicates the data skill prints. Change any assertion here and the skill + * section is what needs editing, not the assertion. + */ +describe('validateSearchableFields — objectstack-ui SKILL.md parity (#6675)', () => { + /** The object the skill's examples and quoted error texts are written against. */ + const supportCase = { + name: 'support_case', + nameField: 'subject', + searchableFields: ['subject', 'case_number', 'description'], + fields: { + subject: { type: 'text' }, + case_number: { type: 'autonumber' }, + description: { type: 'textarea' }, + status: { type: 'select' }, + account_id: { type: 'lookup', reference: 'crm_account' }, + account_name: { type: 'text' }, + }, + }; + + /** A `defineView` container whose `triage` list narrows the object's set. */ + const viewStack = (searchableFields: unknown, objectOverrides: Record = {}) => ({ + objects: [{ ...supportCase, ...objectOverrides }], + views: [ + { + name: 'support_case', + objectName: 'support_case', + list: { + label: 'All Cases', + type: 'grid', + data: { provider: 'object', object: 'support_case' }, + columns: ['subject', 'status'], + }, + listViews: { + triage: { + label: 'Triage', + type: 'grid', + data: { provider: 'object', object: 'support_case' }, + columns: ['case_number', 'subject', 'status'], + ...(searchableFields === undefined ? {} : { searchableFields }), + }, + }, + }, + ], + }); + + it('the skill\'s `os:check` example lints clean — a subset of the allowed set', () => { + // SKILL.md: `listViews.triage.searchableFields: ['case_number', 'subject']`. + expect(validateSearchableFields(viewStack(['case_number', 'subject']))).toEqual([]); + }); + + it('omitting the key lints clean (row 2 of the skill\'s boundary table)', () => { + expect(validateSearchableFields(viewStack(undefined))).toEqual([]); + }); + + /** + * The skill states an empty array is identical to omitting the key — the + * claim an author most needs, because the spelling suggests the opposite. + * + * The lint half of it is deliberately NOT the assertion that carries this + * test. `checkSearchableFieldList` returns early on a zero-length array, and + * even without that early return the entry loop has nothing to iterate — so + * "lints clean" is green because nothing was produced, not because the + * verdict is right, and it cannot go red on a regression. It is asserted + * below only to pin that no finding appears; the load-bearing assertion is + * the next one. + * + * `resolveSearchFields` is where `[]` acquires meaning: it is the ONE + * resolution the ingress gate (`assertSearchFieldsAreSearchable`) and the + * engine (`expandSearchToFilter`) share, so an empty request resolving to + * the full allowed set IS the runtime behaviour the skill describes. Narrow + * the fall-through and this goes red. + */ + it('`searchableFields: []` is ABSENT, not "search off" — it resolves to the FULL allowed set', () => { + expect(validateSearchableFields(viewStack([]))).toEqual([]); + + const resolutionArgs = { + fields: supportCase.fields, + searchableFields: supportCase.searchableFields, + displayField: supportCase.nameField, + }; + // An empty narrowing scans every column the object allows … + expect(resolveSearchFields({ ...resolutionArgs, requestedFields: [] })) + .toEqual(['subject', 'case_number', 'description']); + // … which is exactly what omitting the key does … + expect(resolveSearchFields(resolutionArgs)) + .toEqual(['subject', 'case_number', 'description']); + // … and strictly MORE than a one-entry narrowing, the inversion the skill + // calls out: `[]` searches wider than `['subject']`. + expect(resolveSearchFields({ ...resolutionArgs, requestedFields: ['subject'] })) + .toEqual(['subject']); + }); + + it('quotes the dotted-path diagnostic exactly as the skill prints it', () => { + const findings = validateSearchableFields(viewStack(['subject', 'account_id.name'])); + + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(SEARCHABLE_FIELD_UNKNOWN); + expect(findings[0].severity).toBe('error'); + expect(findings[0].message).toBe( + 'list-view searchableFields entry "account_id.name" is not a field on object ' + + '"support_case". The declaration is stale: searching it can never match, and the ' + + 'engine silently drops it — leaving a narrower search than declared, or the ' + + 'auto-default set once every entry is dropped.', + ); + }); + + it('quotes the outside-the-declared-set diagnostic exactly as the skill prints it', () => { + const findings = validateSearchableFields(viewStack(['subject', 'status'])); + + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(SEARCHABLE_FIELD_UNSEARCHABLE); + expect(findings[0].severity).toBe('error'); + expect(findings[0].message).toBe( + 'list-view searchableFields entry "status" is outside object "support_case"\'s ' + + 'declared searchableFields (subject, case_number, description) — the set \'search\' ' + + 'scans. Clients echo this declaration verbatim as the \'$searchFields\' override, ' + + 'and the runtime refuses an entry outside the allowed set: every toolbar search on ' + + 'this list returns 400 INVALID_FIELD (#4254).', + ); + }); + + /** + * The correction the skill makes to a type-first reading: on an object that + * DECLARES its set, the declaration is the boundary and the field's type is + * not consulted — a lookup inside it is scanned, a text column outside it is + * refused. Both directions, because either alone reads as a coincidence. + */ + it('a lookup INSIDE the object\'s declared set is accepted; a text column OUTSIDE it is not', () => { + const declaresLookup = { searchableFields: ['subject', 'account_id'] }; + + expect(validateSearchableFields(viewStack(['account_id'], declaresLookup))).toEqual([]); + + const refused = validateSearchableFields(viewStack(['account_name'], declaresLookup)); + expect(refused).toHaveLength(1); + expect(refused[0].rule).toBe(SEARCHABLE_FIELD_UNSEARCHABLE); + expect(refused[0].message).toContain('"account_name"'); + }); + + /** + * …and the mirror image: with NO declaration on the object, the auto-default + * is the boundary, so type is exactly what decides. `select` is in the + * text-like set the skill lists; `lookup` is not. + */ + it('with no object declaration, the auto-default type list decides', () => { + const noDeclaration = { searchableFields: undefined }; + + expect(validateSearchableFields(viewStack(['subject', 'status'], noDeclaration))).toEqual([]); + + const refused = validateSearchableFields(viewStack(['account_id'], noDeclaration)); + expect(refused).toHaveLength(1); + expect(refused[0].rule).toBe(SEARCHABLE_FIELD_UNSEARCHABLE); + expect(refused[0].message).toContain("of type 'lookup', which 'search' cannot scan"); + }); +}); diff --git a/skills/objectstack-ui/SKILL.md b/skills/objectstack-ui/SKILL.md index a8ff9de44b..f3d430aa2b 100644 --- a/skills/objectstack-ui/SKILL.md +++ b/skills/objectstack-ui/SKILL.md @@ -342,6 +342,122 @@ Rules: right cluster. Authors only control the `allowedVisualizations` whitelist; a single-entry whitelist locks the visualization (no switcher). +### Toolbar Search (`searchableFields`, ADR-0061) + +The toolbar's search box scans a set the **object** owns. A list view's +`searchableFields` **narrows** that set for this one list — it can never widen +it, and the runtime enforces that by **refusing the request**, not by quietly +dropping the extra name. + + +```typescript +import { defineView } from '@objectstack/spec'; + +const data = { provider: 'object' as const, object: 'support_case' }; + +export const CaseViews = defineView({ + // No `searchableFields` → the toolbar searches everything the object allows. + list: { label: 'All Cases', type: 'grid', data, columns: ['subject', 'status'] }, + listViews: { + // This list only: search the reference number and the subject line. + triage: { + label: 'Triage', type: 'grid', data, + columns: ['case_number', 'subject', 'status'], + searchableFields: ['case_number', 'subject'], + }, + }, +}); +``` + +**What the object allows** is resolved server-side, and it is the whole rule: + +| The object … | The allowed set is | +|:-------------|:-------------------| +| declares `searchableFields` | **that list, verbatim** — whatever the field types are | +| declares nothing | the auto-default: the name field + the text-like columns (`text` / `email` / `phone` / `url` / `autonumber` / `textarea` / `markdown` / `select` / `status`) | + +So field **type** decides only in the second row. On an object that declares +`searchableFields: ['subject', 'account_id']`, a view narrowing to +`['account_id']` — a lookup — is **accepted** and scanned; on the same object, +narrowing to a `text` column the object left out is **refused**. Judge every +entry against the object's allowed set, never against the type list. + +Modelling side — the object's own set, and the stored-mirror prescription for +searching by a related record's title: **objectstack-data → Search Fields +(`searchableFields`)**. Query side (`search.fields` over the API): +**objectstack-query → Full-Text Search**. + +#### ⛔ One bad entry 400s EVERY search on that list + +The client echoes this declaration verbatim as the `$searchFields` override — +the active view's list wins over the object's — and the ingress gate refuses any +entry outside the allowed set before the engine ever runs. The blast radius is +the list's whole search box, for every user and every term: not a narrower +result, no result at all. + +| What you write on the view | `os validate` | Toolbar search at runtime | +|:---------------------------|:--------------|:--------------------------| +| a subset of the allowed set | clean | scans exactly those columns | +| key omitted | clean | scans the object's full allowed set | +| `searchableFields: []` | clean | **identical to omitting it** — see below | +| a renamed / mistyped column | `searchable-field-unknown` | `400 INVALID_FIELD` | +| a dotted path (`account_id.name`) | `searchable-field-unknown` | `400 INVALID_FIELD` | +| a real column outside the allowed set | `searchable-field-unsearchable` | `400 INVALID_FIELD` | + +Both diagnostics are **errors**, not warnings — `os validate` fails the build. +The two you will actually hit, verbatim: + +```text +list-view searchableFields entry "account_id.name" is not a field on object +"support_case". The declaration is stale: searching it can never match, and the +engine silently drops it — leaving a narrower search than declared, or the +auto-default set once every entry is dropped. + +list-view searchableFields entry "status" is outside object "support_case"'s +declared searchableFields (subject, case_number, description) — the set 'search' +scans. Clients echo this declaration verbatim as the '$searchFields' override, +and the runtime refuses an entry outside the allowed set: every toolbar search +on this list returns 400 INVALID_FIELD (#4254). +``` + +#### `searchableFields: []` does NOT turn search off + +An empty array is **absent**, at all three layers: the client omits the +`$searchFields` key entirely, the ingress gate treats a zero-length override as +no override, and the engine falls through to the object's allowed set. A view +written `searchableFields: []` searches **more** columns than one written +`searchableFields: ['subject']`, which is the opposite of what the spelling +suggests. + +To actually remove the search box from the toolbar, toggle the affordance — +a different key, on the same view: + + +```typescript +import { defineView } from '@objectstack/spec'; + +const data = { provider: 'object' as const, object: 'support_case' }; + +export const AuditViews = defineView({ + list: { + label: 'Audit Log', type: 'grid', data, + columns: ['case_number', 'status'], + userActions: { search: false }, // ← no search box; `searchableFields: []` would NOT do this + }, +}); +``` + +#### Searching by a related record's title + +Never reach for a dotted path. `search` scans the queried object's **own** +columns — unlike `columns` / `sort` / `filter`, the search axis resolves no +traversal, so `account_id.name` is refused rather than silently dropped. Copy +the parent's title into a **stored** field on this object and put that field in +the object's `searchableFields`; the view then narrows to it like any other +column. The full prescription — the mirror field, the two hooks that maintain +it, and why a `formula` field cannot be the mirror — lives in +**objectstack-data → Search Fields (`searchableFields`)**. + ### Sorting ```typescript