Skip to content

Commit a314318

Browse files
fix(app-shell): the condition builder offers user.isPlatformAdmin instead of user.isAdmin and user.role (#28848)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9UbbLtK4TajQZzgz4LPGqpe79W)_ Fixes #28833 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:元数据管理里的条件行构建器(Studio / Setup 的「可见条件」等)默认主题列表,把 `user.isAdmin` 换成 `user.isPlatformAdmin`,去掉 `user.role`;原始 CEL 编辑器的示例占位文字同样改教 `user.isPlatformAdmin`。新增一个测试,把列表里每个 `user.*` 主题拿到真实的表达式作用域里求值。 - **为什么改**:浏览器端绑定的用户对象(`buildExpressionUser`)根本没有 `isAdmin`,选它建出的行一求值就报错;`user.role` 是 better-auth 的旧字段,平台管理员读出来也是 `user`。两者都建不出「仅管理员可见」的门。 - **风险与回滚**:只改 `ui/packages/app-shell` 一个源文件里的两个常量和测试,不动 spec、服务端和已存的元数据。已存的 `user.isAdmin` / `user.role` 条件照常以行模式打开,主题原样保留,不会被改写。实测 `examples/**` 与平台应用里没有使用这两个主题的条件。回滚 = revert 本 PR。 - **先看**:`ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx` 的 `CONTEXT_SUBJECTS` 与 `CEL_EXAMPLE_CLAUSES`;再看新测试 `ConditionBuilder.contextSubjectsBag.test.ts`。 - **两分钟验证**:`cd ui && pnpm exec vitest run packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.contextSubjectsBag.test.ts`,应 1 个文件 8 个用例全部通过。 - **需要你做的**:无,席位落地。 ## What changes `CONTEXT_SUBJECTS`, the row builder's default context subjects, now offers `record.id`, `user.id`, `user.email` and `user.isPlatformAdmin`: - **`user.isAdmin` → `user.isPlatformAdmin`.** The `user` object `buildExpressionScope` binds (under `user`, `current_user`, `ctx.user` and `os.user`) is `buildExpressionUser`'s output. On both branches its members are `id`, `name`, `email`, `role`, `isPlatformAdmin` and `positions`, and there is no `isAdmin`. `isPlatformAdmin` is the platform-admin signal. It is `false` on the anonymous branch, so a row on it answers instead of faulting. - **`user.role` dropped.** It is in the bag, but it is better-auth's legacy `sys_user.role` scalar. ObjectStack never writes it (ADR-0068 D2), and it reads `user` for a signed-in platform admin too. - **`CEL_EXAMPLE_CLAUSES`**, the source of the raw editor's placeholder, teaches `user.isPlatformAdmin` instead of `user.isAdmin`. Both constants are now exported so the new test can read them. Neither is re-exported by the package entry, so the published surface does not move. `conditionScope.ts` needs no edit. It names no `user` member: it hands the server tier `RECORD_CONDITION_SUBJECTS` (`record.id` only), the client tier inherits `CONTEXT_SUBJECTS`, and its root lists (`CLIENT_CONDITION_ROOTS`) are roots, not members. So it follows the list by construction. The card says a row on `user.isAdmin` "never opens". The row faults, and what a fault does depends on which gate evaluates the row. An action's `visible` / `disabled` fail closed (`useCondition` with `throwOnError`, `ActionEngine`), and navigation withholds (`evaluateNavigationVisibility`), so on those the gate never opens. An object field's `visible` goes through `evaluateVisibility`, which fails open by documented design, so there it opens for everyone. Either way it never means "admins only". ## Tests - `ConditionBuilder.contextSubjectsBag.test.ts` (new, 8 cases, `unit` project). It reads the member list off `buildExpressionUser` for a platform admin, a member and an anonymous visitor. It evaluates every offered `user.*` subject against `createExpressionEvaluator` (the real bag) on the evaluator's fail-closed seam. And it checks that a "`user.isPlatformAdmin` is true" row opens for the admin only. Controls: `user.isAdmin` faults on every branch, and `user.role` resolves to `user` for an admin and is not offered. The example placeholder's `user` clause evaluates on every branch. - `ConditionBuilder.subjectVocabulary.test.tsx`: the default-subject shape now lists `user.isPlatformAdmin` and neither removed subject. - `ConditionBuilder.contextSubjects.test.tsx`: the "still offers `user.*`" case holds `user.isPlatformAdmin`. Its lint leg passes for `user.isAdmin` too, because the engine lint accepts any member of an accepted root. That is why the new pin evaluates against the bag instead. - `ConditionWidget.conditionSubjects.test.tsx`: one doc comment's example spelling. <details><summary>Ablation (scripts/ablation-replace.mjs, at 8f66ea8)</summary> - `user.isAdmin` re-added to `CONTEXT_SUBJECTS`: 3 failed, 5 passed (each branch: `expected [ 'id', 'name', 'email', 'role', …(2) ] to include 'isAdmin'`). Restored, blob equal to HEAD. - `CEL_EXAMPLE_CLAUSES` put back to `user.isAdmin`: 1 failed (the placeholder case), 7 passed. Restored, blob `90d12c3f` equal to HEAD, `git diff HEAD` empty. </details> ## Acceptance notes - Stored conditions, measured with `git grep` at `2a4c0034` over `examples`, `ui/examples`, `ui/apps` and `packages` (non-test, non-markdown) for `user` / `current_user` / `ctx.user` / `os.user` with `.isAdmin` or `.role`. No authored condition uses either subject. The one hit is a test fixture, `packages/metadata/src/__fixtures__/hotcrm-17.1-built-bare-root-predicates.artifact.json` (`current_user.role == "admin"`). The other hits are doc comments in `plugin-auth`, `spec` and `runtime` that describe the legacy scalar. - A stored `user.isAdmin` or `user.role` row still opens in row mode. The builder appends an out-of-vocabulary subject as its own option, as for `org.id` (`ConditionBuilder.contextSubjects.test.tsx`). Nothing is rewritten on open. - The bag also carries `user.name` and `user.positions`, which are not offered. Adding subjects is outside this card. ## Verification All on the final head `457b72bb` (`origin/main` `892eb668` merged, which carries #28832), after `pnpm install --frozen-lockfile` and `turbo run build --filter='@object-ui/app-shell^...'` plus formula, sdui-parser, lint and client (64 tasks, exit 0), under `scripts/pm/os-verify-lock.sh`. - Every app-shell test file naming `ConditionBuilder`, `conditionScope`, `CONTEXT_SUBJECTS`, `celExampleForRoots` or `buildExpressionUser` (`git grep -l`, `--maxWorkers=2`): 60 files, 667 tests passed. The same set without `buildExpressionUser` at `8f66ea84`, before the merge: 53 files, 611 passed. CI's Test UI runs the rest of app-shell. - `pnpm --filter @object-ui/app-shell run type-check` (both `tsc` programs, the test one included): exit 0 at `457b72bb` and at `8f66ea84`. - ESLint on the 5 changed ts/tsx files: 0 errors. The two `react-refresh/only-export-components` warnings on the newly exported constants are the same class as the file's existing ones (`RECORD_CONDITION_ROOTS`, `celExampleForRoots`). - Derived gates (`node scripts/pm/dispatch-gates.mjs --commands`): 34 of 34 run at `457b72bb`, each exit 0 (`check:type-check-debt` under the lock), including `node scripts/check-sdui-manifest.mjs`, `pnpm --dir ui exec vitest run scripts/` and the PR-body gates with this body as `GITHUB_EVENT_PATH`. `--ran` reconciles 34 derived, 34 run, 0 NOT-MEASURED, 0 UNRUN. Also 34 of 34 at `8f66ea84`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01QsR2jSnzNBkkhbAQrfcS1S Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4a49ad7 commit a314318

6 files changed

Lines changed: 149 additions & 9 deletions
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
fix(app-shell): the condition builder offers `user.isPlatformAdmin` instead of `user.isAdmin` and `user.role`
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
The no-code condition builder's subject list offered `user.isAdmin` and `user.role`. Neither builds an admin gate. `user.isAdmin` is not a member of the identity the browser evaluates against, so a row on it faults: a field or action `visible` gate then shows the item to everyone, and a navigation gate hides it from everyone. `user.role` is better-auth's legacy scalar, which ObjectStack never writes and which reads `user` for a signed-in platform admin too.
11+
12+
The list now offers `user.isPlatformAdmin`, the platform-admin flag the identity carries on every branch (`false` when signed out), and no longer offers `user.role`. The raw editor's example placeholder teaches `user.isPlatformAdmin` as well. A stored condition on either old subject still opens in row mode with its subject intact and is never rewritten. A new test evaluates every offered `user.*` subject against the real expression scope, so the list cannot drift from it again.

‎ui/packages/app-shell/src/views/metadata-admin/ConditionWidget.conditionSubjects.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ function Harness({ context }: { context?: WidgetContext }) {
154154
);
155155
}
156156

157-
/** The root of a subject spelling — `user` for `user.isAdmin`. */
157+
/** The root of a subject spelling — `user` for `user.isPlatformAdmin`. */
158158
const rootOf = (subject: string) => subject.split('.')[0];
159159

160160
/**

‎ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.contextSubjects.test.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,9 @@ describe('the default subject vocabulary ↔ the engine (objectui#9855)', () =>
145145
// narrows it by DECLARING a `context` instead. Both legs are asserted so
146146
// this cannot be satisfied by an empty dropdown.
147147
const subjects = contextSubjectsOf(await defaultSubjects());
148-
expect(subjects).toContain('user.isAdmin');
148+
expect(subjects).toContain('user.isPlatformAdmin');
149149
expect(subjects).toContain('record.id');
150-
expect(await errorsFor('user.isAdmin')).toEqual([]);
150+
expect(await errorsFor('user.isPlatformAdmin')).toEqual([]);
151151
});
152152
});
153153

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The row builder's `user.*` subjects are members of the bag the browser
5+
* really evaluates against (objectstack#28833).
6+
*
7+
* `CONTEXT_SUBJECTS` offered `user.isAdmin`, which `buildExpressionUser` never
8+
* put on the bound `user` object, and `user.role`, the legacy better-auth
9+
* scalar. The engine's LINT cannot see either problem: it accepts any member of
10+
* an accepted root, so `ConditionBuilder.contextSubjects.test.tsx` stayed green
11+
* on `user.isAdmin`. What can see it is the bag, so these cases read the member
12+
* list off `buildExpressionUser` on both branches and evaluate every offered
13+
* subject against `buildExpressionScope`, the one scope the app-shell binds.
14+
*
15+
* Derived, not retyped (ui AGENTS.md #9): a member renamed or dropped in
16+
* `expressionUser.ts` reddens here, and so does a subject added to the list
17+
* that the bag does not carry.
18+
*/
19+
20+
import { describe, it, expect } from 'vitest';
21+
22+
import { CEL_EXAMPLE_CLAUSES, CONTEXT_SUBJECTS, celRootsMentioned } from './ConditionBuilder';
23+
import { buildExpressionUser } from '../../../providers/expressionUser';
24+
import { createExpressionEvaluator } from '../../../providers/ExpressionProvider';
25+
26+
const ADMIN = { id: 'u-admin', name: 'Ada', email: 'ada@example.com', isPlatformAdmin: true };
27+
const MEMBER = { id: 'u-member', name: 'Bo', email: 'bo@example.com' };
28+
29+
/** The three identities a client gate meets: admin, ordinary member, signed out. */
30+
const BRANCHES = [
31+
['platform admin', buildExpressionUser(ADMIN)],
32+
['member', buildExpressionUser(MEMBER)],
33+
['anonymous', buildExpressionUser(null)],
34+
] as const;
35+
36+
const USER_SUBJECTS = CONTEXT_SUBJECTS.map((s) => s.value).filter((v) => v.startsWith('user.'));
37+
38+
/** Evaluate on the evaluator's fail-closed seam: a fault throws instead of answering `true`. */
39+
function evaluate(expr: string, user: Record<string, unknown>): boolean {
40+
return createExpressionEvaluator({ user }).evaluateCondition(expr, { throwOnError: true });
41+
}
42+
43+
/* ── control — the instrument can FAIL ─────────────────────────────────── */
44+
45+
describe('the bag refuses the members this card removed (objectstack#28833)', () => {
46+
it('`user.isAdmin` faults on every branch: it is not a member of the bag', () => {
47+
// Run first: every case below is worthless if a missing member does not
48+
// fault. If `buildExpressionUser` ever grows an `isAdmin`, this reddens and
49+
// the removal gets re-read instead of outliving its reason.
50+
for (const [branch, user] of BRANCHES) {
51+
expect(Object.keys(user), branch).not.toContain('isAdmin');
52+
expect(() => evaluate('user.isAdmin', user), branch).toThrow();
53+
}
54+
});
55+
56+
it('`user.role` is in the bag but answers no admin question: a signed-in admin reads `user`', () => {
57+
// Why it was dropped rather than kept: the member resolves, so the bag pin
58+
// alone would admit it. It is the legacy scalar, defaulted to `user` for
59+
// every session, platform admins included.
60+
expect(evaluate("user.role == 'user'", buildExpressionUser(ADMIN))).toBe(true);
61+
expect(USER_SUBJECTS).not.toContain('user.role');
62+
});
63+
});
64+
65+
/* ── subject — every offered `user.*` member is in the bag ─────────────── */
66+
67+
describe('every `user.*` subject the dropdown offers is a member of the bag (objectstack#28833)', () => {
68+
it('offers some `user.*` subject at all — an empty list would satisfy the cases below', () => {
69+
expect(USER_SUBJECTS).toContain('user.isPlatformAdmin');
70+
});
71+
72+
it.each(BRANCHES)('each subject is an own key of the %s identity and evaluates without a fault', (_branch, user) => {
73+
for (const subject of USER_SUBJECTS) {
74+
const member = subject.slice('user.'.length);
75+
expect(Object.keys(user), subject).toContain(member);
76+
expect(() => evaluate(`${subject} != null`, user), subject).not.toThrow();
77+
}
78+
});
79+
80+
it('a row "user.isPlatformAdmin is true" opens for the platform admin and no one else', () => {
81+
// The row builder compiles a value-less truthy row on a context subject to
82+
// the bare subject; this is the gate the card's author was trying to build.
83+
expect(evaluate('user.isPlatformAdmin', buildExpressionUser(ADMIN))).toBe(true);
84+
expect(evaluate('user.isPlatformAdmin', buildExpressionUser(MEMBER))).toBe(false);
85+
expect(evaluate('user.isPlatformAdmin', buildExpressionUser(null))).toBe(false);
86+
});
87+
});
88+
89+
/* ── the raw editor's worked example teaches the same members ──────────── */
90+
91+
describe('the placeholder example names only members the bag carries (objectstack#28833)', () => {
92+
it('teaches a `user` clause, and that clause evaluates on every branch', () => {
93+
const userClauses = CEL_EXAMPLE_CLAUSES.filter((c) => celRootsMentioned(c).includes('user'));
94+
expect(userClauses.length, 'the example teaches no `user` clause: dead instrument').toBeGreaterThan(0);
95+
for (const clause of userClauses) {
96+
for (const [branch, user] of BRANCHES) {
97+
expect(() => evaluate(clause, user), `${clause} (${branch})`).not.toThrow();
98+
}
99+
}
100+
});
101+
});

‎ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.subjectVocabulary.test.tsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -255,8 +255,10 @@ describe('#6296 DEFAULT-PATH INVARIANCE — the record-scoped consumers are unto
255255
'record.id',
256256
'user.id',
257257
'user.email',
258-
'user.role',
259-
'user.isAdmin',
258+
'user.isPlatformAdmin',
259+
// ⛔ no `user.role` (the legacy better-auth scalar) and no `user.isAdmin`
260+
// (a member the bag never carried); the bag half is pinned in
261+
// `ConditionBuilder.contextSubjectsBag.test.ts`.
260262
// ⛔ no `org.id`: objectui#9855 withdrew it — no host binds an `org` root
261263
// and the engine refuses the spelling. The verdict behind that removal is
262264
// re-derived from the engine in `ConditionBuilder.contextSubjects.test.tsx`;

‎ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -220,13 +220,35 @@ function declaredSubjectValues(
220220
* raw editor's SUGGESTIONS, so a mount that loses one loses an offer and keeps
221221
* the spelling; this list is the row builder's ONLY subject control, so
222222
* dropping an entry drops the author's ability to build that row at all.
223+
*
224+
* ## Every `user.*` member is a key the bag really carries (objectstack#28833)
225+
*
226+
* The `user` object `buildExpressionScope` binds is `buildExpressionUser`'s
227+
* output (`providers/expressionUser.ts`), and a member it does not put there
228+
* is not `false`: the predicate FAULTS, which a fail-open gate (an action's or
229+
* a field's `visible`) answers by showing the item to everyone and a
230+
* withholding gate (navigation) answers by showing it to no one. Either way an
231+
* admin gate built on such a member never means "admins only".
232+
*
233+
* - `user.isPlatformAdmin` is the platform-admin signal, carried on both the
234+
* signed-in and the anonymous branch (anonymous: `false`). It replaces an
235+
* `isAdmin` member no producer ever set.
236+
* - ⛔ No `user.role`. The bag does carry it, but it is better-auth's legacy
237+
* `sys_user.role` scalar: ObjectStack never writes it (ADR-0068 D2), it
238+
* reads `user` for a signed-in platform admin too, and offering it beside
239+
* the other identity members reads as "the platform role". A stored
240+
* `user.role` row still opens in row mode, because an out-of-vocabulary
241+
* subject is appended as its own option.
242+
*
243+
* `ConditionBuilder.contextSubjectsBag.test.ts` re-derives the member list
244+
* from `buildExpressionUser` on both branches and evaluates each subject
245+
* against the real bag, so a member renamed or dropped there reddens here.
223246
*/
224-
const CONTEXT_SUBJECTS = [
247+
export const CONTEXT_SUBJECTS: ReadonlyArray<{ value: string; label?: string }> = [
225248
{ value: 'record.id', label: 'record.id' },
226249
{ value: 'user.id', label: 'user.id' },
227250
{ value: 'user.email', label: 'user.email' },
228-
{ value: 'user.role', label: 'user.role' },
229-
{ value: 'user.isAdmin', label: 'user.isAdmin' },
251+
{ value: 'user.isPlatformAdmin', label: 'user.isPlatformAdmin' },
230252
];
231253

232254
const norm = (s: string) => s.replace(/\s+/g, ' ').trim();
@@ -471,8 +493,11 @@ export function celRootsMentioned(expr: string): string[] {
471493
* Each clause carries its own roots in its own text, and {@link
472494
* celExampleForRoots} reads them back out with {@link celRootsMentioned}, so a
473495
* clause cannot claim a root it does not spell.
496+
*
497+
* A `user.*` clause names a member the bag carries, the rule
498+
* {@link CONTEXT_SUBJECTS} states, pinned by the same test.
474499
*/
475-
const CEL_EXAMPLE_CLAUSES = ["record.status != 'done'", 'user.isAdmin'] as const;
500+
export const CEL_EXAMPLE_CLAUSES = ["record.status != 'done'", 'user.isPlatformAdmin'] as const;
476501

477502
/**
478503
* The placeholder example for a mount that offers `offered` — every clause

0 commit comments

Comments
 (0)