Skip to content

Commit b849e69

Browse files
qq9340100claude
andauthored
fix(lint): warn when searchableFields declares an unprovisioned injected anchor (#8404) (#8665)
* fix(lint): warn when searchableFields declares an unprovisioned injected anchor (#8404) `validate-searchable-fields.ts` is the fifth blanket-SYSTEM_FIELDS read site. Both its gates were correct about existence and blind to provenance, so a declared anchor on an ADR-0015 external object lints clean and the view's `$searchFields` narrowing scans a column with no storage behind it. Adds `searchable-field-unprovisioned` (WARN, never gating -- #4330's cost asymmetry), reusing the #8340 shared message builders rather than re-wording them. The `:239` stub is kept: measured, it is what keeps the linter's resolution equal to the runtime's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jqe56GnYFddggeAyfkZFVz * chore(changeset): minor, not patch -- this adds a public export (#8404) `SEARCHABLE_FIELD_UNPROVISIONED` is a new public export from `packages/lint/src/index.ts`, and `checkSearchableFieldList` gains an additive optional parameter. `patch` under-reports public-surface expansion to everyone reading the changelog downstream. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jqe56GnYFddggeAyfkZFVz * test(lint): pin the react surface's anchor-index threading (#8404) Measured gap: dropping `unprovisionedAnchors` from the `<ListView>` `checkSearchableFieldList` call left typecheck AND the whole 72-file lint suite green. The index stays read by `checkBlockFieldProps` in the same function, so `noUnusedLocals` does not fire there (it does in validate-searchable-fields.ts, where the index would become unused), and the metadata-surface tests cannot see this call site. This test is the only thing that goes red when the argument is dropped -- verified by re-running the ablation with it in place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jqe56GnYFddggeAyfkZFVz --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b803635 commit b849e69

6 files changed

Lines changed: 287 additions & 2 deletions
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
fix(lint): ask the provenance question at the fifth blanket-`SYSTEM_FIELDS` read site — `searchableFields` (#8404)
6+
7+
`validate-searchable-fields.ts` judged a declared `searchableFields` entry
8+
against the object-independent `SYSTEM_FIELDS` union, exactly as the four
9+
filter/page-binding rules did before #8340 wired them to the per-object index.
10+
Both of its gates were correct about EXISTENCE and structurally blind to
11+
PROVENANCE: `:345` keeps `searchable-field-unknown` silent for any name in the
12+
union, and `resolveAllowedSet` goes further — it manufactures a stub meta for
13+
such an entry so it survives the resolution's existence filter exactly as it
14+
does at runtime.
15+
16+
On an ADR-0015 `external` object the platform registers its injected anchors
17+
(`owner_id`, `organization_id`, the audit family, …) and provisions no storage
18+
behind them (#7865 / #8116), so:
19+
20+
```
21+
searchableFields: ['name', 'owner_id'] // external object
22+
```
23+
24+
linted clean, the stub kept the entry in the resolved allow-list, and the
25+
view's `$searchFields` narrowing then scanned a column empty on every record —
26+
#4830's own failure mode (a narrower search than declared, silently) reached by
27+
a different route.
28+
29+
A new `searchable-field-unprovisioned` rule now warns on such an entry, on the
30+
object's own canonical set and on a list view's narrowing alike, reusing
31+
`unprovisionedAnchorCause` / `unprovisionedAnchorHint` so the sentence matches
32+
the four #8340 rules verbatim rather than becoming a second copy (#4830). WARN,
33+
never gating, per #4330's cost asymmetry: the remote schema is not visible to
34+
this pass, so the finding describes a degradation rather than a refusal.
35+
36+
**The `:239` stub is KEPT.** It is not incidental — it is what makes the linter's
37+
resolution agree with the runtime's, which resolves the declared branch against
38+
the registry field map. Measured by disabling it: the existing "keeps runtime
39+
parity when the object declares system columns searchable" test goes red
40+
(`expected [] to have a length of 1 but got +0`), because the declaration
41+
existence-filters to empty and resolution falls through to the auto-default.
42+
Dropping it would have been a behaviour change dressed as a warning.
43+
44+
The warning is emitted per declared entry in the checker's entry loop, never
45+
inside `resolveAllowedSet` — that helper reads the OBJECT's declaration and runs
46+
once per narrowing, so warning there would repeat one object-level fact for
47+
every view and attribute it to the view's path.
48+
49+
`checkSearchableFieldList` takes the index as an OPTIONAL trailing parameter,
50+
the same shape #8340 gave `checkFieldRefs`: its absence means the caller did not
51+
build the index and the provenance question goes unasked — the previous
52+
behaviour, preserved for out-of-repo callers (cloud graph-lint, the AI authoring
53+
path). Both in-repo callers pass it.

packages/lint/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,7 @@ export {
349349
validateSearchableFields,
350350
SEARCHABLE_FIELD_UNKNOWN,
351351
SEARCHABLE_FIELD_UNSEARCHABLE,
352+
SEARCHABLE_FIELD_UNPROVISIONED,
352353
} from './validate-searchable-fields.js';
353354
export type {
354355
SearchableFieldFinding,

packages/lint/src/validate-react-page-props.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
import {
1414
SEARCHABLE_FIELD_UNKNOWN,
1515
SEARCHABLE_FIELD_UNSEARCHABLE,
16+
SEARCHABLE_FIELD_UNPROVISIONED,
1617
} from './validate-searchable-fields.js';
1718
import { PAGE_FIELD_UNKNOWN, PAGE_FIELD_UNPROVISIONED } from './validate-page-field-bindings.js';
1819
// The gate PARSES `ChartAggregateSchema` since #5020, so the function
@@ -339,6 +340,32 @@ describe('validateReactPageProps — <ListView> searchableFields (#4329)', () =>
339340
expect(f[0].message).toContain('400 INVALID_FIELD');
340341
});
341342

343+
it('[#8404] warns on an unprovisioned anchor, and PINS that this surface threads the index', () => {
344+
// Load-bearing beyond the warning itself. `validateReactPageProps` builds
345+
// `unprovisionedAnchors` for its other field-prop checks, so if this
346+
// `checkSearchableFieldList` call ever stops passing it, the index stays
347+
// READ elsewhere in the same function — no TS6133, no type error, and the
348+
// metadata-surface tests in validate-searchable-fields.test.ts cannot see
349+
// this call site at all. Measured: with the argument dropped here, typecheck
350+
// and the whole 72-file lint suite stay green. This test is the only thing
351+
// that goes red, which is precisely why it exists.
352+
const external = {
353+
name: 'ext_account',
354+
external: { remoteName: 'accounts' },
355+
fields: { name: { type: 'text' } },
356+
};
357+
const f = validateReactPageProps(
358+
listPage(list(`objectName="ext_account" searchableFields={['name', 'owner_id']}`), [external]),
359+
);
360+
361+
expect(f).toHaveLength(1);
362+
expect(f[0].rule).toBe(SEARCHABLE_FIELD_UNPROVISIONED);
363+
expect(f[0].severity).toBe('warning');
364+
expect(f[0].path).toBe('pages[0].source › searchableFields[1]');
365+
expect(f[0].message).toContain('external object (ADR-0015)');
366+
expect(f[0].message).toContain('$searchFields');
367+
});
368+
342369
it('flags a dotted path — search cannot resolve the traversal', () => {
343370
const f = validateReactPageProps(
344371
listPage(list(`objectName="crm_account" searchableFields={['owner_id.name']}`)),

packages/lint/src/validate-react-page-props.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1084,6 +1084,11 @@ export function validateReactPageProps(stack: AnyRec): ReactPropFinding[] {
10841084
where,
10851085
`${path} › searchableFields`,
10861086
'searchableFields',
1087+
// A `<ListView>` prop is a view-level narrowing — the checker's
1088+
// default, spelled out here because the #8404 provenance index
1089+
// follows it positionally.
1090+
'narrowing',
1091+
unprovisionedAnchors,
10871092
),
10881093
);
10891094
}

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

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,13 @@ import { describe, it, expect } from 'vitest';
44
import { resolveSearchFields } from '@objectstack/spec/data';
55
import {
66
validateSearchableFields,
7+
checkSearchableFieldList,
8+
indexObjectSearchTargets,
79
SEARCHABLE_FIELD_UNKNOWN,
810
SEARCHABLE_FIELD_UNSEARCHABLE,
11+
SEARCHABLE_FIELD_UNPROVISIONED,
912
} from './validate-searchable-fields.js';
13+
import { indexUnprovisionedAnchors } from './system-fields.js';
1014

1115
/**
1216
* The drift this rule exists for: `email` was renamed to `billing_email` and
@@ -757,3 +761,125 @@ describe('[#6674] validateSearchableFields — a virtual formula entry', () => {
757761
expect(findings[0].message).toContain("is a virtual 'formula' field");
758762
});
759763
});
764+
765+
/**
766+
* [#8404] The FIFTH blanket-`SYSTEM_FIELDS` read site. Existence and provenance
767+
* are different questions: on an ADR-0015 `external` object the platform
768+
* registers `owner_id` and provisions no storage behind it, so the entry
769+
* resolves (skip 3 keeps `searchable-field-unknown` silent, correctly) and
770+
* scans a column empty on every record.
771+
*
772+
* The external object DECLARES a field map on purpose — an external object with
773+
* none takes skip 2 and never reaches any of this, so a fixture without
774+
* `fields` would assert nothing.
775+
*/
776+
describe('[#8404] validateSearchableFields — a declared unprovisioned anchor', () => {
777+
const externalStack = (objectExtra: Record<string, unknown> = {}) => ({
778+
objects: [
779+
{
780+
name: 'ext_customer',
781+
external: { remoteName: 'customers' },
782+
fields: { name: { type: 'text' }, tier: { type: 'select' } },
783+
...objectExtra,
784+
},
785+
],
786+
});
787+
const only = (findings: ReturnType<typeof validateSearchableFields>) =>
788+
findings.filter((f) => f.rule === SEARCHABLE_FIELD_UNPROVISIONED);
789+
790+
it('warns on the object\'s own canonical set, and the existence rule stays silent', () => {
791+
const findings = validateSearchableFields(
792+
externalStack({ searchableFields: ['name', 'owner_id'] }),
793+
);
794+
795+
expect(findings.filter((f) => f.rule === SEARCHABLE_FIELD_UNKNOWN)).toHaveLength(0);
796+
const warned = only(findings);
797+
expect(warned).toHaveLength(1);
798+
expect(warned[0].severity).toBe('warning');
799+
expect(warned[0].path).toBe('objects[0].searchableFields[1]');
800+
expect(warned[0].message).toContain('owner_id');
801+
expect(warned[0].message).toContain('external object (ADR-0015)');
802+
// The canonical consequence, not the narrowing one.
803+
expect(warned[0].message).toContain('narrower than it declares');
804+
expect(warned[0].hint).toContain('columnMap');
805+
});
806+
807+
it('is silent on the local twin — platform storage is real (mutation: drop `external`)', () => {
808+
// The negative that proves the rule discriminates on PROVENANCE rather than
809+
// on the NAME: same declaration, same `owner_id`, non-external object.
810+
const findings = validateSearchableFields(
811+
externalStack({ external: undefined, searchableFields: ['name', 'owner_id'] }),
812+
);
813+
814+
expect(findings).toEqual([]);
815+
});
816+
817+
it('is silent when the author DECLARES the column (#7859 — a remote column they vouch for)', () => {
818+
const findings = validateSearchableFields(
819+
externalStack({
820+
fields: { name: { type: 'text' }, owner_id: { type: 'text' } },
821+
searchableFields: ['name', 'owner_id'],
822+
}),
823+
);
824+
825+
expect(only(findings)).toHaveLength(0);
826+
});
827+
828+
it('names the NARROWING consequence on a list view, and warns once per authored entry', () => {
829+
// Two authoring locations declare the same anchor — the object's own set
830+
// and the view that narrows it. Each is a separate edit the author must
831+
// make, so each warns exactly once; the emission site is the entry loop,
832+
// never `resolveAllowedSet` (which would repeat the object-level fact for
833+
// every view).
834+
const findings = validateSearchableFields(
835+
externalStack({
836+
searchableFields: ['name', 'owner_id'],
837+
listViews: { all: { type: 'grid', searchableFields: ['owner_id'] } },
838+
}),
839+
);
840+
841+
const warned = only(findings);
842+
expect(warned.map((f) => f.path)).toEqual([
843+
'objects[0].searchableFields[1]',
844+
'objects[0].listViews.all.searchableFields[0]',
845+
]);
846+
expect(warned[1].message).toContain('$searchFields');
847+
expect(warned[1].message).toContain('empty on every');
848+
// The stub keeps the anchor inside the resolved allow-list, so the #4830
849+
// admissibility rule stays silent and this is the ONLY finding on it.
850+
expect(findings.filter((f) => f.rule === SEARCHABLE_FIELD_UNSEARCHABLE)).toHaveLength(0);
851+
});
852+
853+
it('asks the provenance question only when the caller builds the index', () => {
854+
// The optional trailing parameter's contract: its absence is the pre-#8404
855+
// behaviour, preserved for out-of-repo callers (cloud graph-lint, the AI
856+
// authoring path). Same stack, same core, index withheld -> silence.
857+
const stack = externalStack({ searchableFields: ['name', 'owner_id'] });
858+
const targets = indexObjectSearchTargets(stack);
859+
860+
const withoutIndex = checkSearchableFieldList(
861+
['name', 'owner_id'],
862+
'ext_customer',
863+
targets,
864+
'where',
865+
'p',
866+
'searchableFields',
867+
'canonical',
868+
);
869+
expect(withoutIndex).toEqual([]);
870+
871+
const withIndex = checkSearchableFieldList(
872+
['name', 'owner_id'],
873+
'ext_customer',
874+
targets,
875+
'where',
876+
'p',
877+
'searchableFields',
878+
'canonical',
879+
indexUnprovisionedAnchors(stack),
880+
);
881+
expect(withIndex).toHaveLength(1);
882+
expect(withIndex[0].rule).toBe(SEARCHABLE_FIELD_UNPROVISIONED);
883+
expect(withIndex[0].path).toBe('p[1]');
884+
});
885+
});

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

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,21 @@
9797
* the runtime would refuse — `created_by` in a view's narrowing — is a
9898
* missed finding, not a wrong one.)
9999
*
100+
* Skip 3 answers EXISTENCE, and since #8404 it no longer ends the matter. On an
101+
* ADR-0015 `external` object the platform registers its injected anchors and
102+
* provisions no storage behind them (#7865 / #8116), so `owner_id` there is
103+
* addressable and empty on every record. Existence rightly stays silent —
104+
* PROVENANCE is a second question, asked only of the names skip 3 already
105+
* decided not to flag, and answered by the per-object index
106+
* ({@link indexUnprovisionedAnchors}) rather than the object-independent union.
107+
* A declared anchor survives into the resolved allow-list and the view's
108+
* `$searchFields` narrowing, so it reads as search coverage and scans a column
109+
* that can never match — #4830's own failure mode reached by a different route.
110+
* WARNING, never gating, for #4330's cost asymmetry: the remote schema is not
111+
* visible to this pass, so the finding describes a degradation, not a refusal
112+
* (the same call `warnUnprovisionedAnchors` makes in `validate-expressions.ts`
113+
* and the four filter/binding rules #8340 wired).
114+
*
100115
* Dotted paths are NOT skipped here, unlike every sibling rule. Elsewhere
101116
* `owner_id.name` is left alone because the query engine resolves the traversal;
102117
* search does not — `resolveSearchFields` matches the field map by exact string,
@@ -112,10 +127,16 @@ import {
112127
SEARCH_AUTO_EXCLUDED_FIELDS,
113128
type SearchFieldMeta,
114129
} from '@objectstack/spec/data';
115-
import { SYSTEM_FIELDS } from './system-fields.js';
130+
import {
131+
SYSTEM_FIELDS,
132+
indexUnprovisionedAnchors,
133+
unprovisionedAnchorCause,
134+
unprovisionedAnchorHint,
135+
} from './system-fields.js';
116136

117137
export const SEARCHABLE_FIELD_UNKNOWN = 'searchable-field-unknown';
118138
export const SEARCHABLE_FIELD_UNSEARCHABLE = 'searchable-field-unsearchable';
139+
export const SEARCHABLE_FIELD_UNPROVISIONED = 'searchable-field-unprovisioned';
119140

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

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

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

338368
for (let i = 0; i < declared.length; i++) {
@@ -369,6 +399,39 @@ export function checkSearchableFieldList(
369399
continue;
370400
}
371401

402+
// ── [#8404] Provenance — the second question about a name skip 3 kept ──
403+
//
404+
// Existence answered "yes" (authored, or a registry-injected system
405+
// column). On a federated object the injected anchor is addressable and
406+
// has no storage, so the entry survives `resolveAllowedSet`'s stub into the
407+
// resolved allow-list and scans a column empty on every record. Emitted
408+
// HERE, per declared entry, rather than at the stub: `resolveAllowedSet`
409+
// reads the OBJECT's declaration and runs once per narrowing, so warning
410+
// there would repeat one object-level fact for every view and attribute it
411+
// to the view's path. Deliberately NOT `continue` — the later checks are
412+
// no-ops for a name absent from authored `fields` (no meta to be virtual,
413+
// and the admissibility pass skips it at ③), so falling through keeps this
414+
// warning additive instead of masking a finding about an authored column.
415+
if (anchors?.has(name)) {
416+
findings.push({
417+
severity: 'warning',
418+
rule: SEARCHABLE_FIELD_UNPROVISIONED,
419+
where,
420+
path: `${path}[${i}]`,
421+
message:
422+
`${subject} entry "${name}" resolves on object "${objectName}", but ` +
423+
`${unprovisionedAnchorCause(objectName, name)}` +
424+
(role === 'narrowing'
425+
? ` — clients echo this declaration verbatim as the '$searchFields' override, so ` +
426+
`every toolbar search on this list scans a column that is empty on every ` +
427+
`record: it reads as search coverage and matches nothing.`
428+
: ` — 'search' scans it on every record and it can never match, so the object's ` +
429+
`searchable set is narrower than it declares. Should it be the ONLY entry that ` +
430+
`resolves, the set scans nothing at all.`),
431+
hint: unprovisionedAnchorHint(objectName, name),
432+
});
433+
}
434+
372435
// ── [#6674] Virtual entries — EVERY surface, canonical included ──
373436
//
374437
// The one check that is not view-level, because the runtime's declared
@@ -481,6 +544,7 @@ export function validateSearchableFields(stack: AnyRec): SearchableFieldFinding[
481544

482545
const objects = asArray(stack.objects);
483546
const fieldsByObject = indexObjectSearchTargets(stack);
547+
const unprovisionedAnchors = indexUnprovisionedAnchors(stack);
484548

485549
const check = (
486550
declared: unknown,
@@ -491,7 +555,16 @@ export function validateSearchableFields(stack: AnyRec): SearchableFieldFinding[
491555
role: SearchableFieldRole,
492556
) => {
493557
findings.push(
494-
...checkSearchableFieldList(declared, objectName, fieldsByObject, where, path, subject, role),
558+
...checkSearchableFieldList(
559+
declared,
560+
objectName,
561+
fieldsByObject,
562+
where,
563+
path,
564+
subject,
565+
role,
566+
unprovisionedAnchors,
567+
),
495568
);
496569
};
497570

0 commit comments

Comments
 (0)