Skip to content

Commit 859ded3

Browse files
claude[bot]claude
andauthored
fix(spec): refuse a whitespace-only reference on lookup/master_detail (#16920)
The #13632 refinement spelled its emptiness test as `reference === ''`, so a whitespace-only target passed a door whose whole purpose is to name an object. Measured on the built artifact: absent and `''` were refused while `' '` and `'\t\n'` were accepted, at both the field and the document level. Apply the test to the trimmed value, so a blank target joins absent and `''` under the same `custom` issue, the same `reference` path and the same message. The notion of blank is `.trim()` — the same one `EvaluatedExpressionSchema` applies to `source`, not a third one. Trimming is for the TEST only: a target with surrounding whitespace is authored and is still stored as written, and a non-string still answers `invalid_type` from the base schema. Both boundaries are pinned. Claude-Session: https://claude.ai/code/session_016N6xmWt5hYm94ffVEwGH8x Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0bb2318 commit 859ded3

3 files changed

Lines changed: 140 additions & 2 deletions

File tree

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
fix(spec): `FieldSchema` refuses a WHITESPACE-ONLY `reference` on `lookup` / `master_detail`
6+
7+
**BREAKING** accept-set narrowing on `FieldSchema`, shipped as `minor` under the
8+
repo's launch-window convention for breaking changes — the same grade the nearest
9+
tightening precedents shipped with, including #13632, the narrowing this one
10+
finishes.
11+
12+
#13632 closed the declared-but-unenforced gap on `FieldSchema.reference` in 17.3.0,
13+
but spelled its emptiness test as an equality against `''`, so a whitespace-only
14+
target (`reference: ' '`) passed a door whose whole purpose is to name an object.
15+
Measured on the built artifact before this change: absent and `''` were refused,
16+
`' '` and `'\t\n'` were **accepted**, at both the field level (`FieldSchema`) and
17+
the document level (`ObjectSchema`), on `lookup` and `master_detail` alike.
18+
19+
A blank target names no object either. The declared grammar for an object name is
20+
`/^[a-z_][a-z0-9_]*$/` (`ObjectSchema`'s own `fields` key schema), so no
21+
whitespace-bearing string can ever resolve to one, and all three consequences the
22+
existing refusal message lists hold verbatim for `' '`: the record picker has no
23+
object to query, `$expand` has nothing to resolve, and no relationship index can be
24+
built. It is also the state a cleared target picker emits — `''` and `' '` are one
25+
authoring gesture that was getting opposite verdicts.
26+
27+
What newly gets rejected: `type: 'lookup'` or `type: 'master_detail'` whose
28+
`reference` is present but consists only of whitespace. It joins absent and `''`
29+
under the same `custom` issue, on the same `reference` path, with the same
30+
prescriptive message — no new message and no new error shape. The notion of blank
31+
is `.trim()`, the same one `EvaluatedExpressionSchema` applies to `source`, not a
32+
third one.
33+
34+
Everything else is untouched. Trimming is applied to the TEST only, never to the
35+
stored value: a target with surrounding whitespace (`' company '`) is still accepted
36+
and still round-trips byte-identically. A non-string `reference` still answers
37+
`invalid_type` from the base schema, not the custom message — that distinction is
38+
deliberate and pinned. Non-relationship types never carried the requirement, and the
39+
`Field.lookup()` / `Field.masterDetail()` helpers take the target as their first
40+
positional argument, so helper-authored fields cannot produce this shape.
41+
42+
The measured population of affected authored sources is zero: one repo-wide census
43+
over all tracked files found a single whitespace-only `reference` in the tree, an
44+
objectql test fixture cast past Zod on the documented `registerObject` path that
45+
skips schema validation by design — it does not reach this door, and it is green
46+
after the change. The census and its positive controls are recorded on the PR.
47+
Downstream, objectui's two metadata writers already refuse this shape with
48+
`reference.trim() !== ''`; upstream trimming turns their declared divergence into
49+
contract-following, and that note can now be retired.
50+
51+
<!-- adr-0087: not-required (no-migration-prescription) A validity narrowing over an existing key: `reference` is not removed, renamed or re-shaped, so there is no tombstone and nothing mechanical for `objectstack migrate meta` to rewrite. The parse refusal is the channel that reaches an affected author, at the parse site, carrying the remedy; which target object a blank `lookup` / `master_detail` was meant to point at is authoring intent no migration entry can decide on an upgrader's behalf — and the measured population of affected sources is zero across all tracked files (census on the PR). Mirrors the disposition of #13632, whose emptiness test this completes. -->

packages/spec/src/data/field.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2278,6 +2278,78 @@ describe('Relationship target — `reference` required on lookup/master_detail (
22782278
},
22792279
);
22802280

2281+
// [#16126] A whitespace-only target is the same hole a third way: it names
2282+
// no object either (no whitespace-bearing string can match the declared
2283+
// object-name grammar), and it is what a cleared target picker emits when
2284+
// the value round-trips through an input. The notion of blank is `.trim()`,
2285+
// the same one `EvaluatedExpressionSchema` applies to `source`.
2286+
2287+
it.each(['lookup', 'master_detail'] as const)(
2288+
'refuses a %s whose reference is whitespace-only — same issue, same path, same message as `\'\'`',
2289+
(type) => {
2290+
const result = FieldSchema.safeParse({
2291+
name: 'company_id',
2292+
label: 'Company',
2293+
type,
2294+
reference: ' ',
2295+
});
2296+
expect(result.success).toBe(false);
2297+
const issue = result.error!.issues.find((i) => i.path.join('.') === 'reference');
2298+
expect(issue).toBeDefined();
2299+
expect(issue!.code).toBe('custom');
2300+
expect(issue!.message).toMatch(/non-empty `reference`/);
2301+
expect(issue!.message).toMatch(/target object/);
2302+
},
2303+
);
2304+
2305+
it.each(['\t', '\n', ' \t\n '] as const)(
2306+
'refuses a lookup whose reference is only whitespace (%j) — not just the space character',
2307+
(reference) => {
2308+
const result = FieldSchema.safeParse({
2309+
name: 'company_id', label: 'Company', type: 'lookup', reference,
2310+
});
2311+
expect(result.success).toBe(false);
2312+
expect(
2313+
result.error!.issues.find((i) => i.path.join('.') === 'reference')?.code,
2314+
).toBe('custom');
2315+
},
2316+
);
2317+
2318+
it('refuses a whitespace-only reference at the DOCUMENT level too, located at the field', () => {
2319+
const result = ObjectSchema.safeParse({
2320+
name: 'acct_note',
2321+
label: 'Note',
2322+
fields: { rel: { name: 'rel', label: 'Rel', type: 'lookup', reference: ' ' } },
2323+
});
2324+
expect(result.success).toBe(false);
2325+
const issue = result.error!.issues.find(
2326+
(i) => i.path.join('.') === 'fields.rel.reference',
2327+
);
2328+
expect(issue).toBeDefined();
2329+
expect(issue!.code).toBe('custom');
2330+
});
2331+
2332+
it.each([42, null, {}] as const)(
2333+
'keeps a non-string reference (%j) answering `invalid_type`, not the custom message',
2334+
(reference) => {
2335+
const result = FieldSchema.safeParse({
2336+
name: 'company_id', label: 'Company', type: 'lookup', reference,
2337+
});
2338+
expect(result.success).toBe(false);
2339+
const issue = result.error!.issues.find((i) => i.path.join('.') === 'reference');
2340+
expect(issue!.code).toBe('invalid_type');
2341+
expect(issue!.message).not.toMatch(/non-empty `reference`/);
2342+
},
2343+
);
2344+
2345+
it('trims only to TEST — a name with surrounding whitespace is authored and is stored as written', () => {
2346+
const result = FieldSchema.safeParse({
2347+
name: 'company_id', label: 'Company', type: 'lookup', reference: ' company ',
2348+
});
2349+
expect(result.success).toBe(true);
2350+
if (result.success) expect(result.data.reference).toBe(' company ');
2351+
});
2352+
22812353
it.each(['lookup', 'master_detail'] as const)(
22822354
'accepts a %s with a non-empty reference (positive control: the check refuses only the hole)',
22832355
(type) => {

packages/spec/src/data/field.zod.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,7 +1129,8 @@ export const FieldSchema = lazySchema(() => {
11291129
* Used by `lookup` and `master_detail` field types to define cross-object references.
11301130
* The `reference` property is **required** for these types — it identifies the target
11311131
* object whose records this field links to, and the superRefine below enforces it:
1132-
* a `lookup` / `master_detail` whose `reference` is missing or empty is refused at
1132+
* a `lookup` / `master_detail` whose `reference` is missing, empty, or
1133+
* whitespace-only is refused at
11331134
* parse time. The engine uses `reference` during $expand
11341135
* post-processing to resolve foreign key IDs into full related objects via batch queries.
11351136
*
@@ -1822,9 +1823,23 @@ export const FieldSchema = lazySchema(() => {
18221823
// measured as accepted before this check). `Field.lookup()` /
18231824
// `Field.masterDetail()` take the target as their first positional
18241825
// argument, so helper-authored fields cannot miss it.
1826+
//
1827+
// [#16126] The emptiness test is applied to the TRIMMED value, so a
1828+
// whitespace-only `reference` joins `undefined` and `''` under this one
1829+
// issue and this one message. It names no object either: the declared
1830+
// grammar for an object name is `/^[a-z_][a-z0-9_]*$/` (`ObjectSchema`'s
1831+
// own `fields` key schema), so no whitespace-bearing string can ever
1832+
// resolve to one, and all three consequences the message lists hold
1833+
// verbatim for `' '`. It is also the state a cleared target picker
1834+
// emits: `''` and `' '` are one authoring gesture that was getting
1835+
// opposite verdicts. The notion of blank is `.trim()` — the same one
1836+
// `EvaluatedExpressionSchema` applies to `source`, not a third one.
1837+
// Trimming is for the TEST only: a value with surrounding whitespace is
1838+
// authored and is still stored as written, and a non-string still
1839+
// answers `invalid_type` from the base schema before this runs.
18251840
if (
18261841
(field.type === 'lookup' || field.type === 'master_detail') &&
1827-
(field.reference === undefined || field.reference === '')
1842+
(field.reference === undefined || field.reference.trim() === '')
18281843
) {
18291844
ctx.addIssue({
18301845
code: 'custom',

0 commit comments

Comments
 (0)