Skip to content

Commit 14935ab

Browse files
os-samclaude
andauthored
fix(metadata-protocol): refuse field reference-target queries instead of clearing them (#9603)
`field` metadata items are addressed by the composite key `<object>.<field>`, while every reference site that names a field holds the bare field name. The two vocabularies are disjoint, so `findReferencesToMeta` answered `{ references: [] }` for every field, always — rendered by the admin "Used by" panel as "Nothing in the metadata graph points at this item. Safe to delete." Adds `unanswerableTargetTypes` as the TARGET-side sibling of #9190's `unwalkableSourceTypes` (which records a source shape that could not be READ — a different fact) and refuses at the protocol seam with the 501 NOT_IMPLEMENTED the route already uses for its sibling refusal. No response field and no new error code. Claude-Session: https://claude.ai/code/session_017qYPmkKEsfbWY1yVg83p8F Co-authored-by: Claude <noreply@anthropic.com>
1 parent e8dba8a commit 14935ab

5 files changed

Lines changed: 501 additions & 1 deletion

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
Refuse `GET /api/v1/meta/field/<object>.<field>/references` instead of clearing it for deletion
6+
7+
A `field` metadata item is addressed by the composite key `<object>.<field>` (e.g. `account.owner`), but every metadata property that names a field holds the **bare** field name — `view.list.columns[].field`, `dataset.dimensions[].field`, `object.validations[].field`, `object.fields{}` and 150 further non-recursive paths across nine source types. The two sides are drawn from disjoint vocabularies, so the reference scan answered `{ references: [] }` for every field, on every deployment, regardless of real usage.
8+
9+
The admin "Used by" panel renders that empty answer verbatim as *"Nothing in the metadata graph points at this item. Safe to delete."* — an unanswerable question shown as a positive clearance, on the screen where someone decides to delete.
10+
11+
`findReferencesToMeta` now refuses a `field` target with `501 NOT_IMPLEMENTED` in the ADR-0112 envelope, carrying the answerable alternative (`GET /api/v1/meta/object/<object>/references`). Per ADR-0110 D3, a miss and a fault are different facts. Nothing is added to the success response, and no new error code is introduced — this is the same code the route already returns when the protocol cannot compute the graph at all.
12+
13+
Every other target type is unaffected: a genuine "nothing points at this item" still answers `{ references: [] }`.
Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#9327] The `field` metadata type can never match as a reference TARGET, and
5+
* is now refused instead of cleared.
6+
*
7+
* ## What was wrong, stated as the operator experienced it
8+
*
9+
* A `field` item is addressed by the COMPOSITE key `<object>.<field>` —
10+
* `GET /api/v1/meta/field/account.owner/references`. Every metadata property
11+
* that names a field holds the BARE name (`owner`): `view.list.columns[].field`,
12+
* `dataset.dimensions[].field`, `object.validations[].field`, `object.fields{}`
13+
* and 150 further non-recursive paths across nine source types. The two sides
14+
* are drawn from disjoint vocabularies, so the scan answered `{ references: [] }`
15+
* for every field, on every deployment, regardless of real usage.
16+
*
17+
* `objectui`'s metadata-admin renders that empty case verbatim as *"Nothing in
18+
* the metadata graph points at this item. Safe to delete."* — a question that
19+
* was never answerable, rendered as a positive clearance, on the screen where
20+
* someone decides to delete (ADR-0110 D3, the #8896 harm shape).
21+
*
22+
* ## What these pins assert, and what they deliberately do not
23+
*
24+
* ⭐ The load-bearing property of this file is that **a test asserting
25+
* `references: []` comes back would have passed against the defect itself**.
26+
* So every pin here asserts the REFUSAL — its `code` and `status` per the
27+
* ADR-0112 envelope — and the second `describe` seeds a field that is genuinely
28+
* referenced from four real sites, which is the case the old behaviour cleared
29+
* for deletion.
30+
*
31+
* ⛔ These do NOT pin fix shape (1) (qualifying bare names against the owning
32+
* object) — that is a capability upgrade with its own card, and it needs object
33+
* context this walker does not have. Nor shape (2) (matching a bare `owner`
34+
* against the key `account.owner`), which is rejected on-card: it swaps false
35+
* negatives for FALSE POSITIVES on delete confirmations, the worse direction on
36+
* this screen.
37+
*/
38+
39+
import { describe, expect, it } from 'vitest';
40+
import { ObjectStackProtocolImplementation } from './protocol.js';
41+
import { REFERENCE_SITES } from './reference-sites.js';
42+
43+
/** Same registry-backed stub the sibling derivation suite uses. */
44+
function protocolWith(items: Record<string, Array<Record<string, unknown>>>) {
45+
const engine: any = {
46+
async find() { return []; },
47+
async findOne() { return null; },
48+
async count() { return 0; },
49+
registry: {
50+
listItems: (type: string) => items[type] ?? [],
51+
getItem: () => undefined,
52+
getObject: () => undefined,
53+
isPackageDisabled: () => false,
54+
getPackage: () => undefined,
55+
registerItem: () => {},
56+
registerObject: () => {},
57+
applyNavContributions: (app: unknown) => app,
58+
},
59+
};
60+
return new ObjectStackProtocolImplementation(engine as never);
61+
}
62+
63+
/**
64+
* Assert the ADR-0112 refusal envelope, not merely that something threw.
65+
*
66+
* ⚠️ A bare `.rejects.toThrow()` is blind in both directions here: it passes on
67+
* any stray `Error` the walk might raise, and it says nothing about the status
68+
* the route will serve — which is the whole wire-visible point of the fix.
69+
*/
70+
async function expectUnanswerableRefusal(run: () => Promise<unknown>): Promise<Error> {
71+
let caught: unknown;
72+
try {
73+
await run();
74+
} catch (err) {
75+
caught = err;
76+
}
77+
expect(caught, 'expected a refusal, got a resolved answer').toBeInstanceOf(Error);
78+
const err = caught as Error & { code?: string; status?: number };
79+
expect(err.code).toBe('NOT_IMPLEMENTED');
80+
expect(err.status).toBe(501);
81+
return err;
82+
}
83+
84+
describe('[#9327] a `field` TARGET is refused, not cleared', () => {
85+
it('THE PIN: the composite key that always answered `[]` now refuses with 501 NOT_IMPLEMENTED', async () => {
86+
// Nothing is seeded on purpose: the pre-fix behaviour returned
87+
// `{ references: [] }` here too, so a test that accepted an empty list
88+
// would have been green against the defect. Only the refusal separates
89+
// the two.
90+
const protocol = protocolWith({});
91+
92+
const err = await expectUnanswerableRefusal(
93+
() => protocol.findReferencesToMeta({ type: 'field', name: 'account.owner' }),
94+
);
95+
96+
// The message is the operator's whole diagnosis at the moment they were
97+
// about to delete, so its first sentence is contract too.
98+
expect(err.message).toContain('cannot be computed');
99+
expect(err.message).toContain('account.owner');
100+
});
101+
102+
it('the refusal is PRESCRIPTIVE — it names the answerable question (ADR-0110 D3)', async () => {
103+
// A refusal that only says "no" moves the operator from a false
104+
// clearance to a dead end. A field's dependents ARE reachable, through
105+
// the object that owns it, and the owning object is recoverable from
106+
// the key the caller already typed.
107+
const protocol = protocolWith({});
108+
109+
const err = await expectUnanswerableRefusal(
110+
() => protocol.findReferencesToMeta({ type: 'field', name: 'account.owner' }),
111+
);
112+
113+
expect(err.message).toContain('GET /api/v1/meta/object/account/references');
114+
});
115+
116+
it('a bare field name is refused too — the key form is the fault, not the spelling', async () => {
117+
// `GET /meta/field/owner/references` is the same unanswerable question
118+
// wearing a shorter key: `owner` is not an addressable field item
119+
// either. Refusing only the DOTTED form would answer "nothing depends
120+
// on it" for the exact spelling an operator reaches for first.
121+
const protocol = protocolWith({});
122+
123+
const err = await expectUnanswerableRefusal(
124+
() => protocol.findReferencesToMeta({ type: 'field', name: 'owner' }),
125+
);
126+
127+
expect(err.message).toContain('<object>.<field>');
128+
});
129+
130+
it('the plural spelling folds to the same refusal, not to a 200', async () => {
131+
// #9157's canonical fold runs first, so `fields` reaches the refusal as
132+
// `field`. Worth pinning: a fold that ran AFTER the refusal check would
133+
// leave the plural URL answering `{ references: [] }` — the defect
134+
// surviving behind an alias.
135+
const protocol = protocolWith({});
136+
137+
await expectUnanswerableRefusal(
138+
() => protocol.findReferencesToMeta({ type: 'fields', name: 'account.owner' }),
139+
);
140+
});
141+
});
142+
143+
describe('[#9327] the refusal replaces a clearance that was measurably false', () => {
144+
it('a field with four real dependents was cleared as "safe to delete" — that answer is gone', async () => {
145+
// Every item below genuinely names `owner`. Pre-fix, this exact fixture
146+
// answered `{ references: [] }`, which the "Used by" panel renders as
147+
// "Nothing in the metadata graph points at this item. Safe to delete."
148+
const protocol = protocolWith({
149+
view: [{
150+
name: 'account_list',
151+
label: 'Accounts',
152+
object: 'account',
153+
list: { columns: [{ field: 'owner' }], sort: [{ field: 'owner' }] },
154+
}],
155+
dataset: [{ name: 'by_owner', dimensions: [{ field: 'owner' }] }],
156+
object: [{
157+
name: 'account',
158+
label: 'Account',
159+
fields: { owner: { name: 'owner', type: 'lookup', reference: 'user' } },
160+
validations: [{ field: 'owner', message: 'required' }],
161+
}],
162+
});
163+
164+
await expectUnanswerableRefusal(
165+
() => protocol.findReferencesToMeta({ type: 'field', name: 'account.owner' }),
166+
);
167+
});
168+
169+
it('sibling target types are untouched — the refusal is scoped to the key-form fault', async () => {
170+
// The failure mode of a refusal is over-refusing. `object` is addressed
171+
// by its own `name`, so its question stays answerable and its answer
172+
// stays exact.
173+
const protocol = protocolWith({
174+
object: [{
175+
name: 'task',
176+
label: 'Task',
177+
fields: { account_id: { name: 'account_id', type: 'lookup', reference: 'account' } },
178+
}],
179+
});
180+
181+
const result = await protocol.findReferencesToMeta({ type: 'object', name: 'account' });
182+
183+
expect(result.references).toEqual([
184+
{
185+
type: 'object',
186+
name: 'task',
187+
label: 'Task',
188+
path: 'fields.account_id.reference',
189+
kind: 'object reference',
190+
},
191+
]);
192+
});
193+
194+
it('an ordinary target with no dependents still answers `[]` — a MISS is still a miss', async () => {
195+
// ADR-0110 D3 cuts both ways: turning genuine "nothing points at this"
196+
// into a fault would be this card's harm inverted, and would make the
197+
// panel useless for the types it serves correctly.
198+
const protocol = protocolWith({ view: [{ name: 'lead_list', object: 'lead' }] });
199+
200+
const result = await protocol.findReferencesToMeta({ type: 'object', name: 'orphan' });
201+
202+
expect(result.references).toEqual([]);
203+
});
204+
});
205+
206+
describe('[#9327] the refused set is derived, and stays honest on its own', () => {
207+
it('THE PIN: exactly one declared type is unanswerable as a target, and it is named', () => {
208+
// ⚠️ If this set GROWS, some other type became unaddressable-by-name
209+
// and its "Used by" panel is now refusing where it used to answer. If
210+
// it SHRINKS to empty, the refusal silently stopped firing and every
211+
// field is being cleared for deletion again. Both directions are the
212+
// failure; do not "fix" a red here by widening the expectation.
213+
expect(REFERENCE_SITES.unanswerableTargetTypes).toEqual(['field']);
214+
});
215+
216+
it('`field` is refused as a TARGET while remaining walkable as a SOURCE', () => {
217+
// The distinction that made this a sibling rather than a widening of
218+
// `unwalkableSourceTypes`: that set is about a shape that could not be
219+
// READ. `field`'s shape reads fine — it contributes sites of its own —
220+
// so it never was and never will be a member there.
221+
expect(REFERENCE_SITES.unwalkableSourceTypes).not.toContain('field');
222+
expect(REFERENCE_SITES.unanswerableTargetTypes).not.toContain('external_catalog');
223+
});
224+
225+
it('the sites that can never match still EXIST — refusing is not the same as having no sites', () => {
226+
// This is why a "no sites → empty answer" shortcut would have been the
227+
// wrong fix: the index is full of properties naming `field`. They are
228+
// real declarations; what is impossible is matching them against the
229+
// key this endpoint is addressed by.
230+
const sites = REFERENCE_SITES.byTarget.get('field') ?? [];
231+
expect(sites.length).toBeGreaterThan(0);
232+
expect(sites.map((s) => `${s.fromType}.${s.property}`)).toContain('view.field');
233+
expect(sites.map((s) => `${s.fromType}.${s.property}`)).toContain('object.fields');
234+
});
235+
});

packages/metadata-protocol/src/protocol.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18208,6 +18208,17 @@ export class ObjectStackProtocolImplementation implements
1820818208
* `SEMANTIC_REFERENCE_SITES` carries the properties whose name does not
1820918209
* spell their target. Both are argued in `reference-sites.ts`.
1821018210
*
18211+
* [#9327] A TARGET type that could never MATCH is a third fact, and it is
18212+
* refused rather than answered. `field` items are addressed by the
18213+
* composite key `<object>.<field>` while every property that names a field
18214+
* holds the bare name, so the two sides are drawn from disjoint
18215+
* vocabularies and `{ references: [] }` was returned for every field, on
18216+
* every deployment, whatever was stored.
18217+
* {@link REFERENCE_SITES.unanswerableTargetTypes} carries that set and this
18218+
* method turns it into a `501 NOT_IMPLEMENTED` — the same code and envelope
18219+
* the route's sibling refusal (#9326) already uses, so no response field
18220+
* and no error code are added.
18221+
*
1821118222
* [#8896] A source type that could not be READ is a different fact and is
1821218223
* no longer answered the same way. This list is what an admin consults
1821318224
* before a rename / delete / type-narrowing, so a silently short answer
@@ -18259,6 +18270,54 @@ export class ObjectStackProtocolImplementation implements
1825918270
// Canonical by construction from the fold above — NOT a second fold.
1826018271
const singularTarget = request.type;
1826118272
const targetName = request.name;
18273+
18274+
// [#9327] REFUSE a target type whose addressing key no reference site
18275+
// can hold. This is the TARGET-side sibling of #9190's
18276+
// `unwalkableSourceTypes`, and it needed to be a sibling rather than a
18277+
// widening: that set records a source shape that could not be READ,
18278+
// while `field` reads perfectly, is walked as a source, and is named as
18279+
// a target by twelve derived sites — every one of which holds a BARE
18280+
// field name (`owner`) while this endpoint is addressed by the
18281+
// composite key (`account.owner`). Disjoint vocabularies, so the answer
18282+
// was `{ references: [] }` for every field, always, regardless of real
18283+
// usage.
18284+
//
18285+
// ⚠️ Why this refuses on the wire instead of being recorded at build
18286+
// time like its sibling. #9190 could move its discriminator OFF the
18287+
// response because the gap it records is BOUNDED — some answers get
18288+
// shorter. This gap is TOTAL: every answer for the type is empty, and
18289+
// the admin "Used by" panel renders that, verbatim, as "Nothing in the
18290+
// metadata graph points at this item. Safe to delete." A constant in a
18291+
// build does not reach the operator standing in front of that sentence,
18292+
// so recording it would leave the destructive clearance exactly where
18293+
// #8896 and ADR-0110 D3 say it must not be.
18294+
//
18295+
// ⛔ NOT the response-shape discriminator #9190 fenced to the spec seat.
18296+
// Nothing is added to the 200 body; this reuses the ADR-0112 nested
18297+
// envelope and the SAME `501 NOT_IMPLEMENTED` code the sibling refusal
18298+
// on this exact route already returns when the protocol cannot compute
18299+
// the graph at all (#9326). One route, one dialect for "the question
18300+
// was never asked".
18301+
//
18302+
// The message is prescriptive per ADR-0110 D3: it names the answerable
18303+
// question, because a field's dependents ARE reachable — through the
18304+
// object that owns it, which is where a field is authored and where the
18305+
// reference graph has real edges.
18306+
if (REFERENCE_SITES.unanswerableTargetTypes.includes(singularTarget)) {
18307+
const owner = targetName.includes('.') ? targetName.slice(0, targetName.indexOf('.')) : '<object>';
18308+
const err = new Error(
18309+
`[unanswerable_target] References to a '${singularTarget}' item cannot be computed. `
18310+
+ `A '${singularTarget}' is addressed by the composite key '<object>.<field>' `
18311+
+ `(here '${targetName}'), while every metadata property that names a field holds the `
18312+
+ `BARE field name — so no reference site can ever match this key and an empty answer `
18313+
+ `would mean "not computable", not "nothing depends on it". `
18314+
+ `Ask the owning object instead: GET /api/v1/meta/object/${owner}/references.`,
18315+
);
18316+
(err as any).code = 'NOT_IMPLEMENTED';
18317+
(err as any).status = 501;
18318+
throw err;
18319+
}
18320+
1826218321
const sites = REFERENCE_SITES.byTarget.get(singularTarget);
1826318322
if (!sites || sites.length === 0) {
1826418323
return { references: [] };

0 commit comments

Comments
 (0)