Skip to content

Commit 20890fe

Browse files
committed
fix(app-shell): canonicalize reference keys on the by-name object-schema serve path (objectui#7650)
`MetadataProvider` has two paths that hand an object schema to a reader and only one of them normalized. `ensureType('object')` — the LIST path — has run `normalizeSchemaReferenceKeys` over every fetched item since objectui#2407. `getItem('object', name)` — the BY-NAME path, behind the published `useMetadataItem` hook — ran `extractItem`, which unwraps the `{ item }` envelope and normalizes nothing. Which spelling a reader saw therefore depended on cache order, not on the document: warm it got the stamped def, cold it got whichever single key the producer stored. The serve path never parses, so `FieldSchema` strictness is no evidence a legacy spelling cannot arrive — a document stored before the key was tightened is served back verbatim, forever. `getItem` now applies the same idempotent, in-place stamp for `type === 'object'`. No other metadata type is touched, nothing is dropped, and a spelling the producer already set keeps its own value. The coverage comment in `packages/core/src/utils/reference-keys.ts` said this file runs at "exactly two production call sites". That was true about where it RAN and false about the serve surface it was cited for; it now names three and says how to re-derive the list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01611D6ZaRaMmwTNQmSbk8MH
1 parent 8a388ee commit 20890fe

4 files changed

Lines changed: 243 additions & 5 deletions

File tree

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
Canonicalize `reference` / `reference_to` on the BY-NAME object-schema serve path
6+
too (objectui#7650).
7+
8+
`MetadataProvider` has two paths that hand an object schema to a reader, and only
9+
one of them normalized. `ensureType('object')` — the LIST path — has run
10+
`normalizeSchemaReferenceKeys` over every fetched item since objectui#2407 / PR
11+
#2587. `getItem('object', name)` — the BY-NAME path, behind the published
12+
`useMetadataItem` hook — ran `extractItem`, which unwraps the `{ item }` envelope
13+
and normalizes nothing.
14+
15+
So which spelling a reader saw depended on **cache order** rather than on the
16+
document: warm (the list had already been fetched) it got the def with both keys
17+
stamped, cold it got whichever single key the producer had stored. A consumer that
18+
reads one key rendered a raw id in the cold case and the relation in the warm one —
19+
the objectui#2407 bug, still reachable through the one door left open.
20+
21+
This matters because the serve path never parses. objectui#7650 measured that
22+
`ObjectStackAdapter.getObjectSchema` applies exactly two mutations and runs no
23+
`ObjectSchema.parse` at all, so `FieldSchema` strictness gates the WRITE door only:
24+
a document stored before a key was tightened is served back verbatim, forever, and
25+
a host with its own `getObjectSchema` is served straight through.
26+
27+
`getItem` now applies the same idempotent, in-place stamp for `type === 'object'`,
28+
so a def the list pass already normalized is untouched, and no other metadata type
29+
is affected. Nothing is dropped or overwritten: a spelling the producer set keeps
30+
its own value.

‎packages/app-shell/src/providers/MetadataProvider.tsx‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -786,6 +786,23 @@ export function MetadataProvider({ children, adapter, ttlMs = DEFAULT_TTL_MS }:
786786
const promise = fetchItem
787787
.then((res: unknown) => {
788788
const item = extractItem(res);
789+
// objectui#7650 — the BY-NAME serve path needs the same
790+
// canonicalization the LIST path applies in `ensureType` above.
791+
//
792+
// `extractItem` only unwraps the `{ item }` envelope; it normalizes
793+
// nothing. So an object def fetched by name reached readers carrying
794+
// whichever single spelling its producer stored, while the very same
795+
// def arriving through `ensureType` carried both. Which one a reader
796+
// got depended on cache order, not on the document — a cold
797+
// `useMetadataItem('object', name)` took this path, a warm one hit
798+
// the list-populated `byName` entry above and saw the stamped def.
799+
//
800+
// This is a serve path, not an ingestion path in miniature: the
801+
// published `useMetadataItem` hook is exported from
802+
// `@object-ui/app-shell`, so the raw def reaches out-of-repo
803+
// consumers too. Idempotent and in place, so a def the list pass
804+
// already stamped is untouched.
805+
if (type === 'object' && item) normalizeSchemaReferenceKeys(item);
789806
if (item) entry.byName.set(name, item);
790807
debug(`fetched item type=${type} name=${name} in ${Date.now() - started}ms`);
791808
pending!.delete(name);
Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* objectui#7650 — the BY-NAME object-schema serve path canonicalizes too.
5+
*
6+
* ## What was broken
7+
*
8+
* `MetadataProvider` has two paths that hand an object schema to a reader, and
9+
* only one of them normalized:
10+
*
11+
* - `ensureType('object')` — the LIST path — ran `normalizeSchemaReferenceKeys`
12+
* over every fetched item (objectui#2407 / PR #2587).
13+
* - `getItem('object', name)` — the BY-NAME path, behind the PUBLISHED
14+
* `useMetadataItem` hook — ran `extractItem`, which only unwraps the
15+
* `{ item }` envelope and normalizes nothing.
16+
*
17+
* So which spelling a reader saw depended on cache order rather than on the
18+
* document: warm (list already fetched) it got the stamped def, cold it got
19+
* whatever single key the producer stored. objectui#7650 measured that the
20+
* serve path never parses, so `FieldSchema` strictness is no evidence a legacy
21+
* spelling cannot reach a consumer — a document stored before the key was
22+
* tightened is served verbatim, forever.
23+
*
24+
* ## What these pins hold
25+
*
26+
* The assertions read the value a CONSUMER gets back from `getItem`, never the
27+
* provider's internals — a re-plumbing that still served an unstamped def by
28+
* some other route would satisfy an internals assertion and violate the card.
29+
*
30+
* The negative pins are the load-bearing half: a normalizer that fired for
31+
* every metadata type, or that overwrote a key the producer had already set,
32+
* would pass every positive assertion here.
33+
*/
34+
35+
import { describe, it, expect, vi } from 'vitest';
36+
import { render, waitFor } from '@testing-library/react';
37+
import { MetadataProvider, useMetadata } from '../MetadataProvider';
38+
39+
type Ctx = ReturnType<typeof useMetadata>;
40+
41+
/**
42+
* Serves ONE named document per type from `docs`, and records every by-name
43+
* fetch so the cache-order pin can prove which path answered.
44+
*/
45+
function makeAdapter(docs: Record<string, Record<string, unknown>>, itemCalls: string[]) {
46+
return {
47+
clearCache: vi.fn(),
48+
getClient: () => ({
49+
meta: {
50+
// The list path serves nothing: every test here starts from a COLD
51+
// by-name cache, which is exactly the state the bug needed.
52+
getItems: (type: string) => Promise.resolve({ type, items: [] }),
53+
getItem: (type: string, name: string) => {
54+
itemCalls.push(`${type}/${name}`);
55+
const doc = docs[`${type}/${name}`];
56+
// Fresh clone per call — the provider normalizes IN PLACE, and a
57+
// shared fixture object would let one test's stamp leak into the next
58+
// assertion and turn a red into a green.
59+
return Promise.resolve({ item: doc ? structuredClone(doc) : null });
60+
},
61+
},
62+
}),
63+
} as unknown as Parameters<typeof MetadataProvider>[0]['adapter'];
64+
}
65+
66+
/** Mounts the provider and hands the live context back to the test body. */
67+
async function withProvider(
68+
docs: Record<string, Record<string, unknown>>,
69+
itemCalls: string[] = [],
70+
): Promise<Ctx> {
71+
let ctx: Ctx | null = null;
72+
function Probe() {
73+
ctx = useMetadata();
74+
return null;
75+
}
76+
render(
77+
<MetadataProvider adapter={makeAdapter(docs, itemCalls)}>
78+
<Probe />
79+
</MetadataProvider>,
80+
);
81+
await waitFor(() => expect(ctx).not.toBeNull());
82+
return ctx as unknown as Ctx;
83+
}
84+
85+
describe('MetadataProvider.getItem canonicalizes object schemas (objectui#7650)', () => {
86+
it('stamps `reference_to` on a by-name object def that spells only `reference`', async () => {
87+
const ctx = await withProvider({
88+
'object/account': {
89+
name: 'account',
90+
fields: { owner: { type: 'lookup', reference: 'user' } },
91+
},
92+
});
93+
94+
const item = await ctx.getItem('object', 'account');
95+
96+
expect(item.fields.owner.reference).toBe('user');
97+
expect(item.fields.owner.reference_to).toBe('user');
98+
});
99+
100+
it('stamps `reference` on a by-name object def that spells only the legacy `reference_to`', async () => {
101+
const ctx = await withProvider({
102+
'object/contact': {
103+
name: 'contact',
104+
fields: { account_id: { type: 'lookup', reference_to: 'account' } },
105+
},
106+
});
107+
108+
const item = await ctx.getItem('object', 'contact');
109+
110+
expect(item.fields.account_id.reference).toBe('account');
111+
expect(item.fields.account_id.reference_to).toBe('account');
112+
});
113+
114+
it('normalizes the ARRAY field-container shape the metadata API also serves', async () => {
115+
const ctx = await withProvider({
116+
'object/lead': {
117+
name: 'lead',
118+
fields: [{ name: 'owner', type: 'lookup', reference: 'user' }],
119+
},
120+
});
121+
122+
const item = await ctx.getItem('object', 'lead');
123+
124+
expect(item.fields[0].reference_to).toBe('user');
125+
});
126+
127+
it('NEGATIVE — leaves a non-`object` metadata type untouched', async () => {
128+
const ctx = await withProvider({
129+
// A `view` document that happens to carry a `fields` map. Nothing but the
130+
// `object` type is a field-def carrier, and normalizing one would be this
131+
// provider inventing a convention the contract does not declare.
132+
'view/account_list': {
133+
name: 'account_list',
134+
fields: { owner: { type: 'lookup', reference: 'user' } },
135+
},
136+
});
137+
138+
const item = await ctx.getItem('view', 'account_list');
139+
140+
expect(item.fields.owner.reference).toBe('user');
141+
expect(item.fields.owner.reference_to).toBeUndefined();
142+
});
143+
144+
it('NEGATIVE — never overwrites a spelling the producer already set', async () => {
145+
const ctx = await withProvider({
146+
'object/opportunity': {
147+
name: 'opportunity',
148+
fields: {
149+
// Deliberately inconsistent: if the stamp overwrote rather than
150+
// filled, one of these two values would change.
151+
owner: { type: 'lookup', reference: 'user', reference_to: 'legacy_user' },
152+
},
153+
},
154+
});
155+
156+
const item = await ctx.getItem('object', 'opportunity');
157+
158+
expect(item.fields.owner.reference).toBe('user');
159+
expect(item.fields.owner.reference_to).toBe('legacy_user');
160+
});
161+
162+
it('serves the SAME canonical shape on a repeat read, without a second fetch', async () => {
163+
const itemCalls: string[] = [];
164+
const ctx = await withProvider(
165+
{
166+
'object/account': {
167+
name: 'account',
168+
fields: { owner: { type: 'lookup', reference: 'user' } },
169+
},
170+
},
171+
itemCalls,
172+
);
173+
174+
await ctx.getItem('object', 'account');
175+
const second = await ctx.getItem('object', 'account');
176+
177+
// The by-name cache answered the second read (so the normalization must
178+
// have stuck to the cached object, not to a throwaway copy).
179+
expect(itemCalls.filter((c) => c === 'object/account')).toHaveLength(1);
180+
expect(second.fields.owner.reference_to).toBe('user');
181+
});
182+
});

‎packages/core/src/utils/reference-keys.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,23 @@
5050
* ## ⛔ WHAT THIS WARNING DOES NOT COVER — state it, do not overclaim it
5151
*
5252
* This warning fires ONLY where this file runs, and this file runs at exactly
53-
* two production call sites, both of them ingestion choke points:
53+
* three production call sites, all of them ingestion choke points:
5454
*
55-
* packages/app-shell/src/providers/MetadataProvider.tsx (metadata type `object`)
55+
* packages/app-shell/src/providers/MetadataProvider.tsx (`ensureType`, metadata type `object`)
56+
* packages/app-shell/src/providers/MetadataProvider.tsx (`getItem`, metadata type `object`)
5657
* packages/data-objectstack/src/index.ts (ObjectStackAdapter.getObjectSchema)
5758
*
58-
* Both of those STAMP the def, so a def that triggers this warning is also a
59-
* def that still resolves. ⇒ The warning fires precisely where nothing is
60-
* broken.
59+
* All three STAMP the def, so a def that triggers this warning is also a def
60+
* that still resolves. ⇒ The warning fires precisely where nothing is broken.
61+
*
62+
* ⚠️ The third of those is new in objectui#7650 and the count above used to
63+
* read TWO. The old count was true about where this file RAN and false about
64+
* the serve surface it was cited for: `MetadataProvider` normalized on its
65+
* LIST path only, so an object def fetched BY NAME — the published
66+
* `useMetadataItem` hook, and any cold-cache read behind it — was served with
67+
* whichever single spelling the producer stored. ⛔ Do not re-derive this list
68+
* by grepping for the call: derive it from the serve paths that hand an object
69+
* schema to a reader, and check each one calls in.
6170
*
6271
* ⚠️ The BREAK surface is the complement of that: `getObjectSchema` is a
6372
* required member of the published `DataSource` interface and the readers call

0 commit comments

Comments
 (0)