Skip to content

Commit 8d37efb

Browse files
os-salesclaude
andauthored
Lock banner titles every state that opens it; lock vocabulary declared once (#5024) (#5672)
Co-authored-by: Claude <noreply@anthropic.com>
1 parent d7573b3 commit 8d37efb

7 files changed

Lines changed: 303 additions & 17 deletions

File tree

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
'@object-ui/data-objectstack': patch
3+
'@object-ui/app-shell': patch
4+
---
5+
6+
The metadata lock banner can no longer render an amber, padlocked box with no
7+
title, and the ADR-0010 §3.6 lock vocabulary is declared once instead of three
8+
times (objectui#5024).
9+
10+
`MetadataLayered.lock` and `MetadataAuditEntry.lockState` each spelled the four
11+
states out by hand, 42 lines apart in one file, compared by no gate. They are now
12+
one exported `MetadataLockState` — derived from `GetMetaItemLayeredResponseSchema`'s
13+
`z.enum` in `@objectstack/spec`, which already owns this vocabulary, so the copies
14+
were restating a schema rather than filling a gap.
15+
16+
The user-visible half is the banner. Its title was three independent `&&` branches
17+
with no fallback, while the switch that opens the banner is true for any non-`none`
18+
value — so a lock state outside the four opened the box and left the headline
19+
empty. That is reachable without a fifth state ever being added here:
20+
`MetadataClient.layered()` casts the wire value through unchecked, so a newer
21+
server reaches this banner as-is. Measured, not assumed — feeding `no-publish`
22+
through the page rendered the padlock, the border and an empty title. The title is
23+
now a keyed lookup with a loud fallback that names the unrecognised token, so a
24+
fifth state fails `type-check` here and, if one arrives from a server anyway, the
25+
operator reads a sentence instead of a blank box.

‎packages/app-shell/src/views/metadata-admin/AuditPanel.lockState.test.tsx‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,13 @@
1919
*
2020
* The state list below is bound to `MetadataAuditEntry['lockState']` itself
2121
* rather than spelled freely, so a fifth lock state is covered by these cases
22-
* the moment the union gains it. Unlike objectui#4982's overlay scopes there is
23-
* no `@objectstack/spec` enum to read at runtime — this union is hand-written in
24-
* `packages/data-objectstack` — hence the `satisfies`-checked key trick instead
25-
* of a `.options` array. (The compile-time half of the coverage is
22+
* the moment the vocabulary gains it. That field is `MetadataLockState` since
23+
* objectui#5024, derived from `GetMetaItemLayeredResponseSchema`'s `z.enum` —
24+
* so unlike what this comment first claimed, objectui#4982's overlay scopes are
25+
* NOT the odd one out: the spec declares this enum too (17.1.0). The
26+
* `satisfies`-checked key trick is kept over a runtime `.options` array because
27+
* it fails at `type-check` rather than at run time, which is the earlier of the
28+
* two; nothing about the derivation forces the choice either way. (The compile-time half of the coverage is
2629
* `LOCK_STATE_ZH` in `./i18n`, whose key type is that union: a new state with no
2730
* zh-CN label fails `type-check` before it can reach the column.)
2831
*/
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The lock banner must never render a headline-less amber box — objectui#5024.
5+
*
6+
* ## The defect
7+
*
8+
* The banner's title was three independent `&&` branches with no `else` and no
9+
* fallback:
10+
*
11+
* {layered?.lock === 'full' && t('engine.edit.lockFull', locale)}
12+
* {layered?.lock === 'no-overlay' && t('engine.edit.lockNoOverlay', locale)}
13+
* {layered?.lock === 'no-delete' && t('engine.edit.lockNoDelete', locale)}
14+
*
15+
* while the switch that OPENS the banner is `layered?.lock && lock !== 'none'`
16+
* — true for *any* non-`none` value. So a lock state outside the ADR-0010 §3.6
17+
* four opens the amber box, draws the padlock and the border, and leaves the
18+
* title `<div>` empty: a locked-looking banner that never says what is locked
19+
* or why.
20+
*
21+
* ## Why this is reachable today, not only "the day a fifth state lands"
22+
*
23+
* The card framed it as dormant, on the reading that both hand-written unions
24+
* list the same four values. The unions are not the gate. `MetadataClient.
25+
* layered()` passes the wire value through with an unchecked cast —
26+
*
27+
* ...(body.lock !== undefined ? { lock: body.lock as MetadataLayered['lock'] } : {}),
28+
*
29+
* — over a `res.json()` body. There is no Zod parse, no allowlist, no default
30+
* on this path. The union constrains what this repo may *write*; it constrains
31+
* nothing about what a server may *send*. A backend that grows a fifth state
32+
* reaches this banner with zero code change here, which is why the fix has to
33+
* be a runtime fallback and not only a compile-time exhaustiveness check: a
34+
* `satisfies` assertion is satisfied by the current four either way and would
35+
* have left the blank banner exactly as it was.
36+
*
37+
* ## What this suite pins
38+
*
39+
* The four known states keep their existing sentences (this half passes before
40+
* the fix — it is the control that says a red fifth-value case is the defect
41+
* and not a broken harness), and an out-of-vocabulary state renders a loud,
42+
* non-empty fallback that also surfaces the raw token so the state a server
43+
* actually sent is diagnosable from the screen.
44+
*/
45+
46+
import '@testing-library/jest-dom/vitest';
47+
import { describe, it, expect, vi, afterEach } from 'vitest';
48+
import { render, screen, cleanup, waitFor } from '@testing-library/react';
49+
import { MemoryRouter } from 'react-router-dom';
50+
51+
const objectDef = {
52+
name: 'showcase_account',
53+
label: 'Account',
54+
fields: [{ name: 'title', label: 'Title', type: 'text' }],
55+
};
56+
57+
const layeredImpl = { current: vi.fn() };
58+
59+
const mockClient = {
60+
list: vi.fn(async () => []),
61+
listDrafts: vi.fn(async () => []),
62+
get: vi.fn(async () => null),
63+
getDraft: vi.fn(async () => null),
64+
references: vi.fn(async () => []),
65+
layered: vi.fn(async (...args: unknown[]) => layeredImpl.current(...args)),
66+
};
67+
68+
vi.mock('./useMetadata', async (importOriginal) => {
69+
const mod = await importOriginal<typeof import('./useMetadata')>();
70+
return {
71+
...mod,
72+
useMetadataClient: () => mockClient,
73+
useMetadataTypes: () => ({
74+
loading: false,
75+
error: null,
76+
entries: [
77+
{
78+
type: 'object',
79+
name: 'object',
80+
label: 'Object',
81+
allowOrgOverride: false,
82+
allowRuntimeCreate: true,
83+
schema: {
84+
type: 'object',
85+
properties: { name: { type: 'string' }, label: { type: 'string' } },
86+
},
87+
},
88+
],
89+
}),
90+
};
91+
});
92+
93+
import { MetadataResourceEditPage } from './ResourceEditPage';
94+
95+
afterEach(() => {
96+
cleanup();
97+
vi.clearAllMocks();
98+
});
99+
100+
/**
101+
* Mount the editor over a layered envelope carrying `lock`. `provenance: 'org'`
102+
* keeps the separate installed-package notice out of the way so the assertions
103+
* below read the LOCK banner and nothing else.
104+
*/
105+
async function renderWithLock(lock: unknown) {
106+
layeredImpl.current = vi.fn(async () => ({
107+
code: { ...objectDef, _packageId: 'com.example.base', _provenance: 'org' },
108+
overlay: null,
109+
overlayScope: null,
110+
effective: objectDef,
111+
provenance: 'org',
112+
packageId: 'com.example.base',
113+
editable: true,
114+
lock,
115+
}));
116+
render(
117+
<MemoryRouter initialEntries={['/metadata/object/showcase_account']}>
118+
<MetadataResourceEditPage type="object" name="showcase_account" />
119+
</MemoryRouter>,
120+
);
121+
await waitFor(() => expect(mockClient.layered).toHaveBeenCalled());
122+
return waitFor(() => {
123+
const el = screen.queryByTestId('lock-banner-title');
124+
expect(el).not.toBeNull();
125+
return el as HTMLElement;
126+
});
127+
}
128+
129+
describe('ResourceEditPage lock banner — every state that opens the banner also titles it (#5024)', () => {
130+
// The control half: these pass before the fix as well as after. If the
131+
// out-of-vocabulary case below goes red while these stay green, the red is
132+
// the defect rather than a harness that cannot mount the page.
133+
it.each([
134+
['full', 'This item is locked and cannot be edited or deleted.'],
135+
['no-overlay', 'This item is locked and cannot be edited.'],
136+
['no-delete', 'This item is locked and cannot be deleted.'],
137+
])('known state %s keeps its existing sentence', async (lock, sentence) => {
138+
const el = await renderWithLock(lock);
139+
expect(el.textContent?.trim()).toBe(sentence);
140+
});
141+
142+
it('an out-of-vocabulary state from the wire still gets a title', async () => {
143+
// The whole defect in one assertion: pre-fix this element exists (the
144+
// banner opened) and is EMPTY.
145+
const el = await renderWithLock('no-publish');
146+
expect(el.textContent?.trim()).not.toBe('');
147+
});
148+
149+
it('an out-of-vocabulary state names the raw token it could not read', async () => {
150+
// "Loudly", per the triage ruling: a generic "this is locked" that hides
151+
// WHICH state arrived would leave an operator with nothing to report.
152+
const el = await renderWithLock('no-publish');
153+
expect(el.textContent).toContain('no-publish');
154+
});
155+
156+
it('a non-string lock value cannot crash or blank the banner', async () => {
157+
// `layered()` casts whatever JSON held; a malformed body is the same class
158+
// of unchecked input as an unknown state name.
159+
const el = await renderWithLock(42);
160+
expect(el.textContent?.trim()).not.toBe('');
161+
});
162+
});

‎packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx‎

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ import {
8282
import { Empty, EmptyTitle, EmptyDescription } from '@object-ui/components';
8383
import type {
8484
MetadataLayered,
85+
MetadataLockState,
8586
MetadataReference,
8687
} from '@object-ui/data-objectstack';
8788
import { PageShell } from './PageShell.js';
@@ -127,6 +128,62 @@ import { describeIssuePath } from './issuePath.js';
127128
import { buildCreateModeBody } from './createBody.js';
128129
import { errorCodeIs, errorCodeIsAnyOf } from '@object-ui/types';
129130

131+
/**
132+
* ADR-0010 §3.6 lock state -> the lock banner's headline sentence.
133+
*
134+
* Keyed on `MetadataLockState` — which derives from `packages/spec`'s own
135+
* `z.enum` — so a fifth state added upstream fails `type-check` HERE, naming
136+
* the label that is missing. That is the strictness the audit panel's
137+
* `LOCK_STATE_ZH` has had since objectui#5004 and this banner did not: its
138+
* title used to be three independent `&&` branches with no `else`.
139+
*
140+
* `Exclude<…, 'none'>` because `none` never banners — `isLocked` gates the
141+
* whole box on `lock && lock !== 'none'`. Typing the record over exactly the
142+
* states that CAN reach the screen keeps the two rules from drifting apart,
143+
* and still fails on a state added to the union.
144+
*/
145+
const LOCK_BANNER_TITLE_KEY: Record<Exclude<MetadataLockState, 'none'>, string> = {
146+
'no-overlay': 'engine.edit.lockNoOverlay',
147+
'no-delete': 'engine.edit.lockNoDelete',
148+
full: 'engine.edit.lockFull',
149+
};
150+
151+
/**
152+
* The banner's headline for whatever `lock` ACTUALLY arrived — including a
153+
* value the lookup above has never heard of (objectui#5024).
154+
*
155+
* The compile-time half cannot be the whole fix. `MetadataLockState` types what
156+
* this repo may WRITE; it constrains nothing about what a server may SEND,
157+
* because `MetadataClient.layered()` casts the wire value in unchecked:
158+
*
159+
* ...(body.lock !== undefined ? { lock: body.lock as MetadataLayered['lock'] } : {}),
160+
*
161+
* over a raw `res.json()` body — no parse, no allowlist, no default. So a
162+
* back end that grows a fifth state reaches this banner with no code change
163+
* here at all. Measured rather than assumed: feeding `no-publish` through this
164+
* page opened the amber box, drew the padlock and the border, and left the
165+
* title `<div>` empty. An exhaustive `satisfies` alone would have type-checked
166+
* green over that exact render.
167+
*
168+
* Hence a sentence for the unrecognised value, carrying the raw token: the
169+
* operator who meets this is the only person able to report which state their
170+
* server actually sent, and a generic "this is locked" would take that away.
171+
* `String(lock)` rather than a cast — the same unchecked path can hand us a
172+
* number or an object, and this must not throw on the way to explaining itself.
173+
*/
174+
function lockBannerTitle(
175+
lock: MetadataLayered['lock'],
176+
locale: string | undefined,
177+
): string {
178+
if (
179+
typeof lock === 'string' &&
180+
Object.prototype.hasOwnProperty.call(LOCK_BANNER_TITLE_KEY, lock)
181+
) {
182+
return t(LOCK_BANNER_TITLE_KEY[lock as Exclude<MetadataLockState, 'none'>], locale);
183+
}
184+
return tFormat('engine.edit.lockUnknown', locale, { state: String(lock) });
185+
}
186+
130187
/**
131188
* Metadata types whose canvas IS the primary create-time authoring
132189
* surface, so we render the preview/inspector split during create
@@ -1990,10 +2047,8 @@ function MetadataResourceEditPageImpl({
19902047
<div className="text-xs text-amber-900 border border-amber-300/70 bg-amber-50/70 rounded-md px-3 py-2.5 dark:text-amber-200 dark:border-amber-700/40 dark:bg-amber-950/20 flex items-start gap-2.5">
19912048
<Lock className="h-3.5 w-3.5 mt-0.5 shrink-0 opacity-80" />
19922049
<div className="flex-1 min-w-0">
1993-
<div className="font-medium">
1994-
{layered?.lock === 'full' && t('engine.edit.lockFull', locale)}
1995-
{layered?.lock === 'no-overlay' && t('engine.edit.lockNoOverlay', locale)}
1996-
{layered?.lock === 'no-delete' && t('engine.edit.lockNoDelete', locale)}
2050+
<div className="font-medium" data-testid="lock-banner-title">
2051+
{lockBannerTitle(layered?.lock, locale)}
19972052
</div>
19982053
{lockReason && <div className="mt-0.5 opacity-90">{lockReason}</div>}
19992054
{layered?.lockDocsUrl && (

‎packages/app-shell/src/views/metadata-admin/i18n.ts‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,13 @@ const ENGINE_STRINGS_EN: Record<string, string> = {
282282
'engine.edit.lockFull': 'This item is locked and cannot be edited or deleted.',
283283
'engine.edit.lockNoOverlay': 'This item is locked and cannot be edited.',
284284
'engine.edit.lockNoDelete': 'This item is locked and cannot be deleted.',
285+
// objectui#5024 — the headline for a lock state this console has no
286+
// sentence for. Reachable without any change here: `layered()` casts the
287+
// wire value through unchecked, so a newer server can send a fifth state
288+
// today. Names the raw token because the operator who sees it is the only
289+
// one who can report which state their server sent.
290+
'engine.edit.lockUnknown':
291+
'This item is locked, but this console does not recognise the lock state \u2018{state}\u2019 — it may come from a newer server. Some operations will be blocked.',
285292
'engine.edit.history': 'History',
286293
'engine.edit.auditTab': 'Audit log',
287294
'engine.edit.auditCount': 'events',
@@ -2101,6 +2108,8 @@ const ENGINE_STRINGS_ZH: Record<string, string> = {
21012108
'engine.edit.lockFull': '该元数据已锁定,不可编辑或删除。',
21022109
'engine.edit.lockNoOverlay': '该元数据已锁定,不可编辑。',
21032110
'engine.edit.lockNoDelete': '该元数据已锁定,不可删除。',
2111+
'engine.edit.lockUnknown':
2112+
'该元数据已锁定,但当前控制台无法识别锁状态“{state}”——它可能来自更新版本的服务端。部分操作将被阻止。',
21042113
'engine.edit.history': '历史',
21052114
'engine.edit.auditTab': '审计日志',
21062115
'engine.edit.auditCount': '条记录',
@@ -4244,13 +4253,15 @@ const LAYER_SCOPE_ZH: Record<NonNullable<MetadataOverlayScope>, string> = {
42444253
* zh-CN labels for the ADR-0010 §3.6 four-state metadata lock, keyed by the
42454254
* producer field the only consumer actually renders.
42464255
*
4247-
* The key type is `MetadataAuditEntry['lockState']` — the hand-written union in
4248-
* `packages/data-objectstack/src/metadata-client.ts`, not a `@objectstack/spec`
4249-
* enum, because this repo owns that union today. (Whether the spec should own
4250-
* the lock vocabulary is a separate question; it is deliberately not answered
4251-
* here.) Binding the keys means a fifth lock state added to the union stops this
4252-
* record from compiling and names the label that is missing, instead of the
4253-
* column silently shipping a raw English token.
4256+
* The key type is `MetadataAuditEntry['lockState']`, which since objectui#5024
4257+
* is `MetadataLockState` — derived from `GetMetaItemLayeredResponseSchema`'s
4258+
* `z.enum` in `@objectstack/spec`. It was a hand-written union in
4259+
* `packages/data-objectstack/src/metadata-client.ts` when this comment was
4260+
* first written, on the belief that the spec had no enum to derive from; the
4261+
* spec does declare one (17.1.0), so the question of ownership this comment
4262+
* used to leave open is answered upstream, not here. Binding the keys means a
4263+
* fifth lock state stops this record from compiling and names the label that is
4264+
* missing, instead of the column silently shipping a raw English token.
42544265
*
42554266
* That silent path is objectui#5004, and it was total rather than partial: the
42564267
* table used to hold `draft` / `locked` / `published` / `none` — a draft-status

‎packages/data-objectstack/src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4687,6 +4687,7 @@ export type {
46874687
MetadataError,
46884688
MetadataValidationIssue,
46894689
MetadataLayered,
4690+
MetadataLockState,
46904691
MetadataOverlayScope,
46914692
MetadataReference,
46924693
MetadataDiagnostics,

‎packages/data-objectstack/src/metadata-client.ts‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,35 @@ export interface MetadataDeleteOptions extends MetadataClientSaveOptions {
265265
*/
266266
export type MetadataOverlayScope = GetMetaItemLayeredResponse['overlayScope'];
267267

268+
/**
269+
* ADR-0010 §3.6 — the four-state metadata protection lock
270+
* (`none` / `no-overlay` / `no-delete` / `full`), read from
271+
* `GetMetaItemLayeredResponseSchema`'s own `z.enum` rather than spelled out
272+
* again here (objectui#5024).
273+
*
274+
* It used to be written out twice in this file, 42 lines apart: once as
275+
* {@link MetadataLayered.lock} (optional) and once as
276+
* {@link MetadataAuditEntry.lockState} (nullable), identical in every other
277+
* respect and compared by no gate. A fifth state added to one would have left
278+
* the other compiling — the same "declared N times, diffed by nothing" failure
279+
* objectui#4972 and objectui#4984 record for other vocabularies.
280+
*
281+
* Deriving beats a local alias the two merely share. The producer of these
282+
* values is the framework, and `packages/spec` already declares the vocabulary,
283+
* so the copies were restating a schema that existed rather than filling a gap.
284+
* The card that reported this recorded the opposite — "`@objectstack/spec` 也没
285+
* 有对应的 `z.enum` 可派生" — and so did the audit panel's neighbouring comment;
286+
* both predate the enum, which ships in `@objectstack/spec` 17.1.0. This is the
287+
* same treatment {@link MetadataOverlayScope} above already gets, and it closes
288+
* the cross-repo half of the drift, not just the in-repo half.
289+
*
290+
* ⚠️ This types what this repo may WRITE. It does NOT constrain what a server
291+
* may SEND: {@link MetadataClient.layered} casts the wire value through
292+
* unchecked, so every reader must still handle a value outside these four. The
293+
* lock banner in `ResourceEditPage` is the worked example.
294+
*/
295+
export type MetadataLockState = GetMetaItemLayeredResponse['lock'];
296+
268297
/**
269298
* Layered view of a metadata item — the body of
270299
* `GET /meta/:type/:name/layers` (`GetMetaItemLayeredResponseSchema`).
@@ -295,7 +324,7 @@ export interface MetadataLayered<T = unknown> {
295324
_diagnostics?: MetadataDiagnostics;
296325
// ── ADR-0010 Phase 1 — protection envelope ──
297326
/** 4-state lock: `none` / `no-overlay` / `no-delete` / `full`. */
298-
lock?: 'none' | 'no-overlay' | 'no-delete' | 'full';
327+
lock?: MetadataLockState;
299328
/** Human-readable reason for the lock (tooltip text). */
300329
lockReason?: string;
301330
/** Which layer set the lock: artifact / package / overlay / env-forced. */
@@ -337,7 +366,7 @@ export interface MetadataAuditEntry {
337366
/** Machine-readable reason code (`item_locked`, `ok`, …). */
338367
code: string;
339368
/** Effective lock at the moment of the attempt. */
340-
lockState: 'none' | 'no-overlay' | 'no-delete' | 'full' | null;
369+
lockState: MetadataLockState | null;
341370
/** True when admin forced the write through despite the lock. */
342371
lockOverridden: boolean;
343372
/** Request-id for trace correlation (if propagated). */

0 commit comments

Comments
 (0)