Skip to content

Commit ba426b0

Browse files
claude[bot]claude
andauthored
fix(lint,metadata-protocol): a junk entry in stack.objects no longer crashes the reference-integrity seam, and a throwing probe rule is reported (#15494) (#15562)
* fix(lint,metadata-protocol): a junk entry in stack.objects no longer crashes the reference-integrity seam, and a throwing probe rule is reported `indexObjectGraph` is the first statement of every rule that resolves a field path, and its local `asArray` returned an array unchanged — so a `null` member of `stack.objects` reached `strName(obj.name)` and threw `TypeError: Cannot read properties of null (reading 'name')` before any member's own `if (!isRec(obj)) continue` could run. These rules are pure `(stack) => Finding[]` and run on the raw `lint` path as well as the parsed one, and at the runtime publish gate they are called inside the gate: a throw there is an exception on a write path, not a skipped finding. The entry is SKIPPED, not reported. Every sibling `asArray` in this package that spells the defensive read drops the member silently, each member of the family already answers the same question three lines below the call, and this module decides no severities by contract. Driving the whole `AUTHORING_RULES` table over `{ objects: [null, validObject] }` measured 28 rules judging it in silence and none reporting the junk entry. Second half, on the receipt: `runBuildProbes`' object plane wrapped the rule in `catch { findings = [] }`, so a crash produced the byte-identical receipt a clean object produces while `checked.objects` had already counted it. It now emits a `runtime`-layer `object_field_ref_rule_failed` error carrying the thrown message. Probes still never fail the publish they verify. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk * chore(changeset): patch @objectstack/lint and @objectstack/metadata-protocol for the object-graph null-entry guard Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk * test(metadata-protocol): mark the probe diagnostics code as ADR-0112 D6c on the literal's own line check:error-code-casing reads the closed SCREAMING_SNAKE catalog; a build-probe diagnostics code shipped inside a 200 receipt is D6c, which is why the gate exempts build-probes.ts and the objectql probe test whole. The per-literal mark is the narrower spelling of the same exemption. Written on the literal's own line deliberately: a multi-line comment above it was measured to move the literal out of the gate's recognition window, so the gate went green with the mark deleted — a suppression that was really a blind spot. Both marks are now load-bearing (removing either reds the gate). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk * docs(lint): name the filed follow-up (#15552) and state what this suite pin does NOT cover Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4444885 commit ba426b0

6 files changed

Lines changed: 347 additions & 6 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/lint': patch
3+
'@objectstack/metadata-protocol': patch
4+
---
5+
6+
A junk entry in `stack.objects` no longer crashes the reference-integrity rules, and a probe rule that throws is reported instead of read as "nothing wrong".
7+
8+
`indexObjectGraph` is the first statement of every rule that resolves a field path, and it read each `stack.objects` member without checking it was a record — so a `null` entry (an empty YAML list item, a partial editor write) threw `TypeError: Cannot read properties of null (reading 'name')` before any rule's own per-object guard could run. Because these rules also run inside the runtime publish gate, that was an exception on a write path rather than a missed finding. The seam now drops non-record entries — silently, matching every sibling collection reader in the package — and the valid objects beside them are judged exactly as before.
9+
10+
On the publish receipt, `runBuildProbes`' object plane wrapped its rule call in a catch that produced an empty finding list, so a crashed rule was indistinguishable from a clean object while `checked.objects` had already counted it. A rule that throws now surfaces as a `runtime`-layer `object_field_ref_rule_failed` error carrying the thrown message, so an unverified object never reads as a verified one. Probes still never fail the publish they verify.

packages/lint/src/object-graph.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,3 +194,43 @@ describe('filter-walk — walkFilterFieldKeys across the three authored shapes',
194194
expect(keys('a string')).toEqual([]);
195195
});
196196
});
197+
198+
describe('object-graph — a non-record entry in `stack.objects` (#15494)', () => {
199+
// The seam is the FIRST statement of every rule that resolves a field path,
200+
// so an unguarded read here threw before any member's own `if (!isRec(obj))
201+
// continue` could run — on the runtime publish door that is an exception on
202+
// a write path, not a skipped finding. Measured on `origin/main` at
203+
// 615fac3a0, `validateObjectFieldRefs({ objects: [null] })`:
204+
// TypeError: Cannot read properties of null (reading 'name')
205+
// at indexObjectGraph (src/object-graph.ts:159:30)
206+
// The entry is SKIPPED rather than reported: this module decides no
207+
// severities by contract, and a junk member is a shape defect the schema
208+
// owns — see `asArray`'s note for the three reasons and the measurement.
209+
210+
it('drops a null entry instead of throwing, and still indexes the rest', () => {
211+
const valid = { name: 'crm_lead', fields: { name: { type: 'text' } } };
212+
expect(() => indexObjectGraph({ objects: [null] })).not.toThrow();
213+
const g = indexObjectGraph({ objects: [null, valid, undefined, 'junk', 42, []] });
214+
expect([...g.keys()]).toEqual(['crm_lead']);
215+
expect(resolveFieldPath(g, 'crm_lead', 'name')).toMatchObject({ kind: 'ok' });
216+
});
217+
218+
it('drops a non-record FIELD entry too — the same read, one level down', () => {
219+
// `graphObjectOf` walks `obj.fields` through the identical helper, so
220+
// `fields: [null]` crashed at the same statement for the same reason.
221+
const g = indexObjectGraph({
222+
objects: [{ name: 'crm_lead', fields: [null, { name: 'amount', type: 'currency' }] }],
223+
});
224+
expect(resolveFieldPath(g, 'crm_lead', 'amount')).toMatchObject({ kind: 'ok' });
225+
});
226+
227+
it('reads a name-keyed map whose VALUE is not a record as a nameless object', () => {
228+
// `{ a: 'junk' }` used to spread the string's indices into the record; the
229+
// verdict was already `no-field-map`, and it still is — the entry keeps
230+
// its key so an object declaring nothing stays distinguishable from one
231+
// this stack never defined (skip 2 vs. skip 1).
232+
const g = indexObjectGraph({ objects: { a: 'junk', b: { fields: { n: { type: 'text' } } } } });
233+
expect(resolveFieldPath(g, 'a', 'n')).toMatchObject({ kind: 'unknowable', reason: 'no-field-map' });
234+
expect(resolveFieldPath(g, 'b', 'n')).toMatchObject({ kind: 'ok' });
235+
});
236+
});

packages/lint/src/object-graph.ts

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,51 @@ export interface GraphObject {
115115
/** object name → its resolvable surface, or `null` (skip 2). */
116116
export type ObjectGraph = ReadonlyMap<string, GraphObject | null>;
117117

118-
/** Coerce a collection (array or name-keyed map) to an array of records. */
118+
/** A plain record — not `null`, not an array. */
119+
function isRec(v: unknown): v is AnyRec {
120+
return !!v && typeof v === 'object' && !Array.isArray(v);
121+
}
122+
123+
/**
124+
* Coerce a collection (array or name-keyed map) to an array of records,
125+
* DROPPING every member that is not one.
126+
*
127+
* The drop is the whole point, and it is a SKIP rather than a finding.
128+
*
129+
* This seam is the first statement of every rule that resolves a field path,
130+
* so an entry it cannot read decides the fate of the entire family: an
131+
* unguarded read here threw `TypeError: Cannot read properties of null` out of
132+
* `indexObjectGraph` before any member's own per-object guard could run, which
133+
* on the runtime publish door is an exception on a WRITE path rather than the
134+
* silent miss this family exists to end. These rules are pure
135+
* `(stack) => Finding[]` (ADR-0019) and run on the RAW `lint` path as well as
136+
* the parsed one, so `objects` here is whatever the author's files deserialised
137+
* to — a YAML list item left empty is `null`, and nothing upstream of the raw
138+
* path has judged the shape.
139+
*
140+
* Skipping, not reporting, for three reasons that all point the same way:
141+
*
142+
* 1. It is what the rest of the family already does. Every sibling `asArray`
143+
* in this package that spells the defensive read at all drops the member
144+
* silently (`validate-nav-target-refs.ts`, `validate-flow-node-writes.ts`,
145+
* `validate-hook-body-writes.ts`, `validate-page-visualization-bindings.ts`
146+
* and the rest); not one of them emits a finding about it. Driving the
147+
* whole `AUTHORING_RULES` table over `{ objects: [null, validObject] }`
148+
* measured 28 rules judging it in silence and none reporting the junk
149+
* entry — the seam was the outlier, not the reporters.
150+
* 2. Each member of this family ALREADY answers the question three lines
151+
* below the call, with `if (!isRec(obj)) continue` in its own per-object
152+
* loop. A report from here would contradict the guard the same rule is
153+
* about to run.
154+
* 3. This module decides no severities and holds no rule ids by contract (see
155+
* the module note). A junk `objects` member is a SHAPE defect — the
156+
* schema's subject, not reference integrity's — and reporting it here
157+
* would emit the same finding once per member for one bad entry.
158+
*/
119159
function asArray(v: unknown): AnyRec[] {
120-
if (Array.isArray(v)) return v as AnyRec[];
121-
if (v && typeof v === 'object') {
122-
return Object.entries(v as AnyRec).map(([name, def]) => ({ name, ...(def as AnyRec) }));
160+
if (Array.isArray(v)) return v.filter(isRec);
161+
if (isRec(v)) {
162+
return Object.entries(v).map(([name, def]) => ({ name, ...(isRec(def) ? def : {}) }));
123163
}
124164
return [];
125165
}

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ import {
77
} from './reference-integrity-suite.js';
88
import { validateObjectReferences } from './validate-object-references.js';
99
import { validateTranslationReferences } from './validate-translation-references.js';
10+
import { validateObjectFieldRefs } from './validate-object-field-refs.js';
11+
import { validateListViewFieldRefs } from './validate-list-view-field-refs.js';
12+
import { validateDatasetReferences } from './validate-dataset-references.js';
1013

1114
describe('reference-integrity suite — membership', () => {
1215
// Deliberately a written-out list: adding a rule to the suite should be a
@@ -412,3 +415,73 @@ describe('reference-integrity suite — every member actually runs', () => {
412415
expect(validateReferenceIntegrity({})).toEqual([]);
413416
});
414417
});
418+
419+
describe('reference-integrity — a non-record entry in `stack.objects` (#15494)', () => {
420+
/**
421+
* One case per rule that resolves through the shared `indexObjectGraph`
422+
* seam. Enumerated from the source rather than written from memory —
423+
* `git grep -l indexObjectGraph packages/lint/src` names four rules:
424+
* `validateObjectFieldRefs`, `validateListViewFieldRefs`,
425+
* `validateDatasetReferences` and `validateWidgetBindings`. The first three
426+
* are the suite members and are the table below.
427+
*
428+
* ⛔ `validateWidgetBindings` is deliberately ABSENT, and not because it is
429+
* fixed. It is not a suite member (it runs on `os doctor` via
430+
* `AUTHORING_RULES`), and it carries a SECOND, independent null dereference
431+
* of its own — `validate-widget-bindings.ts:465`, in the aggregate-coherence
432+
* pass that runs BEFORE it ever reaches this seam — so the seam guard cannot
433+
* reach it. Measured after this change:
434+
* THROW validateWidgetBindings { objects: [null] }
435+
* TypeError: Cannot read properties of null (reading 'name')
436+
* at validateWidgetBindings (src/validate-widget-bindings.ts:465:18)
437+
* That file is held by another in-flight change, so the repair is filed as
438+
* #15552 rather than ridden here — together with the wider inventory the same
439+
* measurement turned up: 13 of 42 `AUTHORING_RULES` entries throw on this
440+
* input through five more unguarded readers of `stack.objects`, three of them
441+
* inside this very suite (`validate-object-references.ts`,
442+
* `indexObjectSearchTargets`, `indexObjectFields`). So the suite ENTRY POINT
443+
* still throws on `{ objects: [null] }` after this change; what this file
444+
* pins is the seam, per member, and no more than that.
445+
*
446+
* Each case asserts BOTH halves: the junk entry is not a crash, and the
447+
* valid object beside it is still judged — a guard that returned early would
448+
* satisfy the first half while silently deleting the rule.
449+
*/
450+
const validObject = {
451+
name: 'crm_lead',
452+
fields: { name: { type: 'text', label: 'Name' }, amount: { type: 'currency', label: 'Amount' } },
453+
// `nope` exists nowhere on the object — one dangling name per position, so
454+
// each member below has something of its own to report.
455+
highlightFields: ['name', 'nope'],
456+
listViews: { all: { type: 'grid', columns: ['name', 'nope'] } },
457+
};
458+
// `validateDatasetReferences` returns before the seam when a stack declares
459+
// no datasets, so the table's stack carries one — without it that member's
460+
// case would pass without ever reaching the code under test.
461+
const datasets = [
462+
{ name: 'lead_ds', object: 'crm_lead', dimensions: [{ field: 'nope' }], measures: [] },
463+
];
464+
465+
const members: ReadonlyArray<[string, (s: Record<string, unknown>) => Array<{ rule: string; path: string }>, string, string]> = [
466+
['validateObjectFieldRefs', validateObjectFieldRefs, 'object-field-ref-unknown', 'objects[1].highlightFields[1]'],
467+
['validateListViewFieldRefs', validateListViewFieldRefs, 'list-view-field-unknown', 'objects[1].listViews.all.columns[1]'],
468+
['validateDatasetReferences', validateDatasetReferences, 'dataset-field-unknown', 'datasets[0].dimensions[0].field'],
469+
];
470+
471+
for (const [name, run, rule, path] of members) {
472+
it(`${name}: a lone junk entry is skipped, not thrown`, () => {
473+
expect(() => run({ objects: [null], datasets })).not.toThrow();
474+
expect(() => run({ objects: [undefined, 'junk', 7], datasets })).not.toThrow();
475+
});
476+
477+
it(`${name}: the valid object beside a junk entry is still judged`, () => {
478+
const findings = run({ objects: [null, validObject], datasets });
479+
const hit = findings.find((f) => f.rule === rule);
480+
expect(hit, `${name} kept judging past the junk entry`).toBeDefined();
481+
// The path still counts the junk entry: the guard drops it from the
482+
// GRAPH, while each member's own loop keeps walking the raw array, so
483+
// reported positions stay stable against the author's file.
484+
expect(hit!.path).toBe(path);
485+
});
486+
}
487+
});
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #15494 — the object probe plane must never convert a rule CRASH into
5+
* "nothing wrong".
6+
*
7+
* ## What this pins, and the state it replaces
8+
*
9+
* `runBuildProbes`' object plane re-runs `validateObjectFieldRefs` over each
10+
* published object's ACTIVE body and counts it in `checked.objects`. The call
11+
* was wrapped in `catch { findings = [] }`, so a rule that threw produced the
12+
* byte-identical receipt a genuinely clean object produces: the count went up,
13+
* the issue list stayed empty. That is the one reading this plane exists to
14+
* make impossible — it was added (#15254) precisely because a count that
15+
* cannot go up is indistinguishable from a plane that found nothing wrong, and
16+
* the silent catch reinstated the same ambiguity one layer in.
17+
*
18+
* The crash that motivated the card is a null entry in `stack.objects`
19+
* dereferenced by the shared `indexObjectGraph` seam, repaired in
20+
* `@objectstack/lint` in the same change. This file pins the OTHER half, which
21+
* outlives that bug: whatever the next rule failure is, the receipt says the
22+
* object was not checked, and says why.
23+
*
24+
* ## Why the rule is mocked rather than provoked
25+
*
26+
* With the seam repaired there is no longer a published body that makes the
27+
* real rule throw — which is the point of the repair. Reaching the branch
28+
* therefore means substituting a throwing rule, and `build-probes.ts` imports
29+
* `@objectstack/lint` LAZILY (`await import`) at call time, so `vi.doMock`
30+
* plus a fresh module graph per test is exact: nothing else in the file, and
31+
* no other suite, sees a mocked lint package.
32+
*
33+
* ## The `adr0112-ok:` marks below
34+
*
35+
* `object_field_ref_rule_failed` is a build-probe diagnostics code shipped
36+
* inside a 200 receipt (ADR-0112 D6c), not an `error.code` from the closed
37+
* catalog — the same vocabulary as every other probe code, for which
38+
* `check:error-code-casing` exempts `build-probes.ts` and
39+
* `packages/objectql/src/build-probes.test.ts` whole. The marks here are the
40+
* narrower per-literal spelling of that same exemption, and they are written
41+
* on the literal's OWN line deliberately: a multi-line comment above the
42+
* literal was measured to move it out of the gate's recognition window
43+
* entirely, which reads as a suppression while actually being a blind spot.
44+
*/
45+
46+
import { describe, expect, it, vi, afterEach } from 'vitest';
47+
import type { ProbeEngine } from './build-probes.js';
48+
49+
const OBJECT_BODY = {
50+
name: 'crm_lead',
51+
fields: { name: { type: 'text', label: 'Name' } },
52+
highlightFields: ['name'],
53+
};
54+
55+
const getItem = async (type: string, name: string) =>
56+
type === 'object' && name === 'crm_lead' ? OBJECT_BODY : undefined;
57+
58+
/**
59+
* The probes' single engine read. The object plane never calls it, but the
60+
* double still honours the caller's `limit` by presence rather than ignoring
61+
* it — a `find` double that answers more rows than it was asked for is how a
62+
* limit regression rides through a green suite (`check:objectql-double-limit`).
63+
*/
64+
const engine: ProbeEngine = {
65+
find: async (_object: string, query: unknown) => {
66+
const rows = [{ id: 'r1' }, { id: 'r2' }];
67+
const limit = (query as { limit?: unknown } | undefined)?.limit;
68+
return typeof limit === 'number' ? rows.slice(0, limit) : rows;
69+
},
70+
};
71+
72+
afterEach(() => {
73+
vi.doUnmock('@objectstack/lint');
74+
vi.resetModules();
75+
});
76+
77+
async function probeWith(validateObjectFieldRefs: (stack: Record<string, unknown>) => unknown) {
78+
vi.resetModules();
79+
vi.doMock('@objectstack/lint', () => ({ validateObjectFieldRefs }));
80+
const { runBuildProbes } = await import('./build-probes.js');
81+
return runBuildProbes({
82+
engine,
83+
getItem,
84+
published: [{ type: 'object', name: 'crm_lead' }],
85+
});
86+
}
87+
88+
describe('runBuildProbes — a throwing object rule is reported, never swallowed', () => {
89+
it('surfaces the crash as a runtime-layer error naming the object and the thrown message', async () => {
90+
const report = await probeWith(() => {
91+
throw new TypeError("Cannot read properties of null (reading 'name')");
92+
});
93+
94+
// The count still goes up — the object WAS reached; what failed is the
95+
// judgement. Reporting one without the other is the ambiguity again.
96+
expect(report.checked.objects).toBe(1);
97+
expect(report.issues).toHaveLength(1);
98+
expect(report.issues[0]).toMatchObject({
99+
layer: 'runtime',
100+
severity: 'error',
101+
code: 'object_field_ref_rule_failed', // adr0112-ok: D6c build-probe diagnostics code
102+
artifact: { type: 'object', name: 'crm_lead' },
103+
});
104+
// The thrown message rides the receipt: without it the report says a
105+
// rule failed and gives nobody a way to find out which defect.
106+
expect(report.issues[0].message).toContain("Cannot read properties of null (reading 'name')");
107+
expect(report.issues[0].message).toContain('crm_lead');
108+
// ⛔ The one reading that must be impossible.
109+
expect(report.issues, 'a crash must not read as zero findings').not.toEqual([]);
110+
});
111+
112+
it('reports a non-Error throw too — the message is whatever was thrown', async () => {
113+
const report = await probeWith(() => {
114+
throw 'rule exploded';
115+
});
116+
expect(report.issues[0]).toMatchObject({ code: 'object_field_ref_rule_failed' }); // adr0112-ok: D6c build-probe diagnostics code
117+
expect(report.issues[0].message).toContain('rule exploded');
118+
});
119+
120+
it('a clean rule still produces the clean receipt — the contrast case', async () => {
121+
// Without this the test above would pass just as well against a probe
122+
// that reported a failure for every object.
123+
const report = await probeWith(() => []);
124+
expect(report.checked.objects).toBe(1);
125+
expect(report.issues).toEqual([]);
126+
});
127+
128+
it('a rule that finds a dangling reference still reports THAT, not a failure', async () => {
129+
const report = await probeWith(() => [
130+
{ path: 'objects.crm_lead.highlightFields[0]', message: 'no such field', hint: 'add it' },
131+
]);
132+
expect(report.issues).toHaveLength(1);
133+
expect(report.issues[0].code).toBe('object_field_ref_unknown');
134+
});
135+
});

0 commit comments

Comments
 (0)