Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions .changeset/searchable-fields-anchor-provenance.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
---
"@objectstack/lint": minor
---

fix(lint): ask the provenance question at the fifth blanket-`SYSTEM_FIELDS` read site — `searchableFields` (#8404)

`validate-searchable-fields.ts` judged a declared `searchableFields` entry
against the object-independent `SYSTEM_FIELDS` union, exactly as the four
filter/page-binding rules did before #8340 wired them to the per-object index.
Both of its gates were correct about EXISTENCE and structurally blind to
PROVENANCE: `:345` keeps `searchable-field-unknown` silent for any name in the
union, and `resolveAllowedSet` goes further — it manufactures a stub meta for
such an entry so it survives the resolution's existence filter exactly as it
does at runtime.

On an ADR-0015 `external` object the platform registers its injected anchors
(`owner_id`, `organization_id`, the audit family, …) and provisions no storage
behind them (#7865 / #8116), so:

```
searchableFields: ['name', 'owner_id'] // external object
```

linted clean, the stub kept the entry in the resolved allow-list, and the
view's `$searchFields` narrowing then scanned a column empty on every record —
#4830's own failure mode (a narrower search than declared, silently) reached by
a different route.

A new `searchable-field-unprovisioned` rule now warns on such an entry, on the
object's own canonical set and on a list view's narrowing alike, reusing
`unprovisionedAnchorCause` / `unprovisionedAnchorHint` so the sentence matches
the four #8340 rules verbatim rather than becoming a second copy (#4830). WARN,
never gating, per #4330's cost asymmetry: the remote schema is not visible to
this pass, so the finding describes a degradation rather than a refusal.

**The `:239` stub is KEPT.** It is not incidental — it is what makes the linter's
resolution agree with the runtime's, which resolves the declared branch against
the registry field map. Measured by disabling it: the existing "keeps runtime
parity when the object declares system columns searchable" test goes red
(`expected [] to have a length of 1 but got +0`), because the declaration
existence-filters to empty and resolution falls through to the auto-default.
Dropping it would have been a behaviour change dressed as a warning.

The warning is emitted per declared entry in the checker's entry loop, never
inside `resolveAllowedSet` — that helper reads the OBJECT's declaration and runs
once per narrowing, so warning there would repeat one object-level fact for
every view and attribute it to the view's path.

`checkSearchableFieldList` takes the index as an OPTIONAL trailing parameter,
the same shape #8340 gave `checkFieldRefs`: its absence means the caller did not
build the index and the provenance question goes unasked — the previous
behaviour, preserved for out-of-repo callers (cloud graph-lint, the AI authoring
path). Both in-repo callers pass it.
1 change: 1 addition & 0 deletions packages/lint/src/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -349,6 +349,7 @@ export {
validateSearchableFields,
SEARCHABLE_FIELD_UNKNOWN,
SEARCHABLE_FIELD_UNSEARCHABLE,
SEARCHABLE_FIELD_UNPROVISIONED,
} from './validate-searchable-fields.js';
export type {
SearchableFieldFinding,
Expand Down
27 changes: 27 additions & 0 deletions packages/lint/src/validate-react-page-props.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -13,6 +13,7 @@ import {
import {
SEARCHABLE_FIELD_UNKNOWN,
SEARCHABLE_FIELD_UNSEARCHABLE,
SEARCHABLE_FIELD_UNPROVISIONED,
} from './validate-searchable-fields.js';
import { PAGE_FIELD_UNKNOWN, PAGE_FIELD_UNPROVISIONED } from './validate-page-field-bindings.js';
// The gate PARSES `ChartAggregateSchema` since #5020, so the function
Expand DownExpand Up@@ -339,6 +340,32 @@ describe('validateReactPageProps — <ListView> searchableFields (#4329)', () =>
expect(f[0].message).toContain('400 INVALID_FIELD');
});

it('[#8404] warns on an unprovisioned anchor, and PINS that this surface threads the index', () => {
// Load-bearing beyond the warning itself. `validateReactPageProps` builds
// `unprovisionedAnchors` for its other field-prop checks, so if this
// `checkSearchableFieldList` call ever stops passing it, the index stays
// READ elsewhere in the same function — no TS6133, no type error, and the
// metadata-surface tests in validate-searchable-fields.test.ts cannot see
// this call site at all. Measured: with the argument dropped here, typecheck
// and the whole 72-file lint suite stay green. This test is the only thing
// that goes red, which is precisely why it exists.
const external = {
name: 'ext_account',
external: { remoteName: 'accounts' },
fields: { name: { type: 'text' } },
};
const f = validateReactPageProps(
listPage(list(`objectName="ext_account" searchableFields={['name', 'owner_id']}`), [external]),
);

expect(f).toHaveLength(1);
expect(f[0].rule).toBe(SEARCHABLE_FIELD_UNPROVISIONED);
expect(f[0].severity).toBe('warning');
expect(f[0].path).toBe('pages[0].source › searchableFields[1]');
expect(f[0].message).toContain('external object (ADR-0015)');
expect(f[0].message).toContain('$searchFields');
});

it('flags a dotted path — search cannot resolve the traversal', () => {
const f = validateReactPageProps(
listPage(list(`objectName="crm_account" searchableFields={['owner_id.name']}`)),
Expand Down
5 changes: 5 additions & 0 deletions packages/lint/src/validate-react-page-props.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -1084,6 +1084,11 @@ export function validateReactPageProps(stack: AnyRec): ReactPropFinding[] {
where,
`${path} › searchableFields`,
'searchableFields',
// A `<ListView>` prop is a view-level narrowing — the checker's
// default, spelled out here because the #8404 provenance index
// follows it positionally.
'narrowing',
unprovisionedAnchors,
),
);
}
Expand Down
126 changes: 126 additions & 0 deletions packages/lint/src/validate-searchable-fields.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,9 +4,13 @@ import { describe, it, expect } from 'vitest';
import { resolveSearchFields } from '@objectstack/spec/data';
import {
validateSearchableFields,
checkSearchableFieldList,
indexObjectSearchTargets,
SEARCHABLE_FIELD_UNKNOWN,
SEARCHABLE_FIELD_UNSEARCHABLE,
SEARCHABLE_FIELD_UNPROVISIONED,
} from './validate-searchable-fields.js';
import { indexUnprovisionedAnchors } from './system-fields.js';

/**
* The drift this rule exists for: `email` was renamed to `billing_email` and
Expand DownExpand Up@@ -757,3 +761,125 @@ describe('[#6674] validateSearchableFields — a virtual formula entry', () => {
expect(findings[0].message).toContain("is a virtual 'formula' field");
});
});

/**
* [#8404] The FIFTH blanket-`SYSTEM_FIELDS` read site. Existence and provenance
* are different questions: on an ADR-0015 `external` object the platform
* registers `owner_id` and provisions no storage behind it, so the entry
* resolves (skip 3 keeps `searchable-field-unknown` silent, correctly) and
* scans a column empty on every record.
*
* The external object DECLARES a field map on purpose — an external object with
* none takes skip 2 and never reaches any of this, so a fixture without
* `fields` would assert nothing.
*/
describe('[#8404] validateSearchableFields — a declared unprovisioned anchor', () => {
const externalStack = (objectExtra: Record<string, unknown> = {}) => ({
objects: [
{
name: 'ext_customer',
external: { remoteName: 'customers' },
fields: { name: { type: 'text' }, tier: { type: 'select' } },
...objectExtra,
},
],
});
const only = (findings: ReturnType<typeof validateSearchableFields>) =>
findings.filter((f) => f.rule === SEARCHABLE_FIELD_UNPROVISIONED);

it('warns on the object\'s own canonical set, and the existence rule stays silent', () => {
const findings = validateSearchableFields(
externalStack({ searchableFields: ['name', 'owner_id'] }),
);

expect(findings.filter((f) => f.rule === SEARCHABLE_FIELD_UNKNOWN)).toHaveLength(0);
const warned = only(findings);
expect(warned).toHaveLength(1);
expect(warned[0].severity).toBe('warning');
expect(warned[0].path).toBe('objects[0].searchableFields[1]');
expect(warned[0].message).toContain('owner_id');
expect(warned[0].message).toContain('external object (ADR-0015)');
// The canonical consequence, not the narrowing one.
expect(warned[0].message).toContain('narrower than it declares');
expect(warned[0].hint).toContain('columnMap');
});

it('is silent on the local twin — platform storage is real (mutation: drop `external`)', () => {
// The negative that proves the rule discriminates on PROVENANCE rather than
// on the NAME: same declaration, same `owner_id`, non-external object.
const findings = validateSearchableFields(
externalStack({ external: undefined, searchableFields: ['name', 'owner_id'] }),
);

expect(findings).toEqual([]);
});

it('is silent when the author DECLARES the column (#7859 — a remote column they vouch for)', () => {
const findings = validateSearchableFields(
externalStack({
fields: { name: { type: 'text' }, owner_id: { type: 'text' } },
searchableFields: ['name', 'owner_id'],
}),
);

expect(only(findings)).toHaveLength(0);
});

it('names the NARROWING consequence on a list view, and warns once per authored entry', () => {
// Two authoring locations declare the same anchor — the object's own set
// and the view that narrows it. Each is a separate edit the author must
// make, so each warns exactly once; the emission site is the entry loop,
// never `resolveAllowedSet` (which would repeat the object-level fact for
// every view).
const findings = validateSearchableFields(
externalStack({
searchableFields: ['name', 'owner_id'],
listViews: { all: { type: 'grid', searchableFields: ['owner_id'] } },
}),
);

const warned = only(findings);
expect(warned.map((f) => f.path)).toEqual([
'objects[0].searchableFields[1]',
'objects[0].listViews.all.searchableFields[0]',
]);
expect(warned[1].message).toContain('$searchFields');
expect(warned[1].message).toContain('empty on every');
// The stub keeps the anchor inside the resolved allow-list, so the #4830
// admissibility rule stays silent and this is the ONLY finding on it.
expect(findings.filter((f) => f.rule === SEARCHABLE_FIELD_UNSEARCHABLE)).toHaveLength(0);
});

it('asks the provenance question only when the caller builds the index', () => {
// The optional trailing parameter's contract: its absence is the pre-#8404
// behaviour, preserved for out-of-repo callers (cloud graph-lint, the AI
// authoring path). Same stack, same core, index withheld -> silence.
const stack = externalStack({ searchableFields: ['name', 'owner_id'] });
const targets = indexObjectSearchTargets(stack);

const withoutIndex = checkSearchableFieldList(
['name', 'owner_id'],
'ext_customer',
targets,
'where',
'p',
'searchableFields',
'canonical',
);
expect(withoutIndex).toEqual([]);

const withIndex = checkSearchableFieldList(
['name', 'owner_id'],
'ext_customer',
targets,
'where',
'p',
'searchableFields',
'canonical',
indexUnprovisionedAnchors(stack),
);
expect(withIndex).toHaveLength(1);
expect(withIndex[0].rule).toBe(SEARCHABLE_FIELD_UNPROVISIONED);
expect(withIndex[0].path).toBe('p[1]');
});
});
77 changes: 75 additions & 2 deletions packages/lint/src/validate-searchable-fields.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -97,6 +97,21 @@
* the runtime would refuse — `created_by` in a view's narrowing — is a
* missed finding, not a wrong one.)
*
* Skip 3 answers EXISTENCE, and since #8404 it no longer ends the matter. On an
* ADR-0015 `external` object the platform registers its injected anchors and
* provisions no storage behind them (#7865 / #8116), so `owner_id` there is
* addressable and empty on every record. Existence rightly stays silent —
* PROVENANCE is a second question, asked only of the names skip 3 already
* decided not to flag, and answered by the per-object index
* ({@link indexUnprovisionedAnchors}) rather than the object-independent union.
* A declared anchor survives into the resolved allow-list and the view's
* `$searchFields` narrowing, so it reads as search coverage and scans a column
* that can never match — #4830's own failure mode reached by a different route.
* WARNING, never gating, for #4330's cost asymmetry: the remote schema is not
* visible to this pass, so the finding describes a degradation, not a refusal
* (the same call `warnUnprovisionedAnchors` makes in `validate-expressions.ts`
* and the four filter/binding rules #8340 wired).
*
* Dotted paths are NOT skipped here, unlike every sibling rule. Elsewhere
* `owner_id.name` is left alone because the query engine resolves the traversal;
* search does not — `resolveSearchFields` matches the field map by exact string,
Expand All@@ -112,10 +127,16 @@ import {
SEARCH_AUTO_EXCLUDED_FIELDS,
type SearchFieldMeta,
} from '@objectstack/spec/data';
import { SYSTEM_FIELDS } from './system-fields.js';
import {
SYSTEM_FIELDS,
indexUnprovisionedAnchors,
unprovisionedAnchorCause,
unprovisionedAnchorHint,
} from './system-fields.js';

export const SEARCHABLE_FIELD_UNKNOWN = 'searchable-field-unknown';
export const SEARCHABLE_FIELD_UNSEARCHABLE = 'searchable-field-unsearchable';
export const SEARCHABLE_FIELD_UNPROVISIONED = 'searchable-field-unprovisioned';

export type SearchableFieldSeverity = 'error' | 'warning';

Expand DownExpand Up@@ -324,6 +345,14 @@ export function checkSearchableFieldList(
path: string,
subject: string,
role: SearchableFieldRole = 'narrowing',
// [#8404] `objectName -> its unprovisioned injected anchors`
// ({@link indexUnprovisionedAnchors}). OPTIONAL, and its absence means
// exactly one thing: this caller did not build the index, so the provenance
// question goes unasked and only existence/admissibility are answered — the
// pre-#8404 behaviour, preserved for out-of-repo callers of this exported
// core (cloud graph-lint, the AI authoring path). Every in-repo caller passes
// it: `validateSearchableFields` below and `validate-react-page-props`.
unprovisionedAnchors?: ReadonlyMap<string, ReadonlySet<string>>,
): SearchableFieldFinding[] {
const findings: SearchableFieldFinding[] = [];
if (!Array.isArray(declared) || declared.length === 0) return findings;
Expand All@@ -333,6 +362,7 @@ export function checkSearchableFieldList(
if (!target) return findings; // ② external / introspected — no authored field map

const known = target.names;
const anchors = unprovisionedAnchors?.get(objectName);
const resolution = role === 'narrowing' ? resolveAllowedSet(target) : undefined;

for (let i = 0; i < declared.length; i++) {
Expand DownExpand Up@@ -369,6 +399,39 @@ export function checkSearchableFieldList(
continue;
}

// ── [#8404] Provenance — the second question about a name skip 3 kept ──
//
// Existence answered "yes" (authored, or a registry-injected system
// column). On a federated object the injected anchor is addressable and
// has no storage, so the entry survives `resolveAllowedSet`'s stub into the
// resolved allow-list and scans a column empty on every record. Emitted
// HERE, per declared entry, rather than at the stub: `resolveAllowedSet`
// reads the OBJECT's declaration and runs once per narrowing, so warning
// there would repeat one object-level fact for every view and attribute it
// to the view's path. Deliberately NOT `continue` — the later checks are
// no-ops for a name absent from authored `fields` (no meta to be virtual,
// and the admissibility pass skips it at ③), so falling through keeps this
// warning additive instead of masking a finding about an authored column.
if (anchors?.has(name)) {
findings.push({
severity: 'warning',
rule: SEARCHABLE_FIELD_UNPROVISIONED,
where,
path: `${path}[${i}]`,
message:
`${subject} entry "${name}" resolves on object "${objectName}", but ` +
`${unprovisionedAnchorCause(objectName, name)}` +
(role === 'narrowing'
? ` — clients echo this declaration verbatim as the '$searchFields' override, so ` +
`every toolbar search on this list scans a column that is empty on every ` +
`record: it reads as search coverage and matches nothing.`
: ` — 'search' scans it on every record and it can never match, so the object's ` +
`searchable set is narrower than it declares. Should it be the ONLY entry that ` +
`resolves, the set scans nothing at all.`),
hint: unprovisionedAnchorHint(objectName, name),
});
}

// ── [#6674] Virtual entries — EVERY surface, canonical included ──
//
// The one check that is not view-level, because the runtime's declared
Expand DownExpand Up@@ -481,6 +544,7 @@ export function validateSearchableFields(stack: AnyRec): SearchableFieldFinding[

const objects = asArray(stack.objects);
const fieldsByObject = indexObjectSearchTargets(stack);
const unprovisionedAnchors = indexUnprovisionedAnchors(stack);

const check = (
declared: unknown,
Expand All@@ -491,7 +555,16 @@ export function validateSearchableFields(stack: AnyRec): SearchableFieldFinding[
role: SearchableFieldRole,
) => {
findings.push(
...checkSearchableFieldList(declared, objectName, fieldsByObject, where, path, subject, role),
...checkSearchableFieldList(
declared,
objectName,
fieldsByObject,
where,
path,
subject,
role,
unprovisionedAnchors,
),
);
};

Expand Down
Loading