Skip to content

Commit 57e00f5

Browse files
fix(metadata-protocol): the save door stores an action as its ActionSchema parse, so requiresFeature / requiresMembershipReach are lowered into visible (#26880)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X7aTDKRB5qtXEtLGphyrQr2)_ Fixes #26723 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么:** 元数据保存入口(`/meta` 保存对象、单独保存动作,草稿与发布两种模式)现在把动作按 `ActionSchema` 解析后的结果存盘,`requiresFeature` / `requiresMembershipReach` 会被并入 `visible`。 - **为什么改:** 以前这两个门槛原样存着、没有 `visible`,控制台只读 `visible`,所以普通成员也能看到只给管理员或特定功能开放的按钮。 - **风险与回滚:** 只影响新保存的动作;已存的旧行不改写,重新保存一次即修好。回滚就是 revert 本 PR。 - **先看:** `packages/metadata-protocol/src/protocol-stored-action-parse.ts`(新 helper);`protocol.ts` 保存入口里新增的那一行。 - **两分钟验证:** `pnpm --filter @objectstack/metadata-protocol exec vitest run src/protocol.stored-action-gate-lowering.test.ts`,应见 7 passed。 - **需要你做的:** 无,席位落地。 ## What changes `saveMetaItem` (`packages/metadata-protocol`) judges an `object` or `action` body by its spec schema. It then stored the request body as written, apart from the operator and form-section grafts. `ActionSchema`'s parse is what lowers `requiresMembershipReach` and `requiresFeature` into `visible`, so a stored action kept both gate keys raw and had no `visible`. The console reads `visible` alone, so the button was offered to every user. This was measured on `origin/main` `4c29f38447` by the red leg below. The door now stores the parse it already ran. `withParsedStoredActions` (new, `protocol-stored-action-parse.ts`) is called once, right after the existing grafts: - **`object`:** the object's own `actions` array becomes the parsed array (`ObjectSchema`'s `actions` is `ActionSchema[]`). Every other key of the body stays as written. - **`action`:** the body is the parsed action. - **Any other type**, or an object with no `actions` array, is unchanged. This is the same parse the code-package registration door has stored since #25981 (`parsePackageAction` in `@objectstack/objectql`). The lowering is not re-implemented here: the values are read off the parse tree the door already computed. The parse is idempotent over its own output, so a GET then PUT round trip stores the same bytes (pinned). The schema block runs before the draft/active split, so both modes are covered by one hunk. ### The action endpoint (measured, ruled) Asked by the triage line: does `POST /api/v1/actions/:object/:action` enforce the gate? **No.** - The door checks `requiredPermissions` (`actionPermissionError`, `packages/runtime/src/action-execution.ts:832`), the activation switch and the action type. - `visible` is deliberately unread there (contract point 6, `action-execution.ts:2484`). It is pinned by `action-declarative-update.test.ts` "point 6", run locally: 2 passed. - The parsed action no longer carries the raw keys: both lowerings return the input without the key. So dispatch cannot see what was declared without a spec change. - The spec already rules both keys UI courtesy, not authorization (`ui/action.zod.ts` docblocks on `visible` and `requiresMembershipReach`). - Every in-repo carrier is a `type: 'api'` platform-objects identity action calling a better-auth endpoint, whose own door is the authority. `POST /actions` answers 400 for `api` and `url` actions. The seat ruled option A on the card: no dispatch enforcement and no spec change. This PR fixes the save door only. ## Tests New: `packages/metadata-protocol/src/protocol.stored-action-gate-lowering.test.ts`, through the real `saveMetaItem`, reading the `sys_metadata` row the save wrote: - An object saved in draft and in publish mode, with one action carrying `requiresFeature: 'ssoEnforced'` and one carrying `requiresMembershipReach: 'delete_organization'`, is stored with neither key. Each action has its lowered `visible`, deep-equal to what `ObjectSchema.create` gives the same declaration. - **Control:** an action without the keys is stored as its parse with no `visible`, and the object's other keys are stored as written. - Re-saving a stored body stores the same actions (idempotent). - An object with no `actions` key is stored without one. - A standalone `action` saved in draft and in publish mode is stored lowered, deep-equal to `ActionSchema.parse`. <details><summary>Red first, ablation</summary> - **Red.** With `protocol.ts` written back to the base blob `4ccf8acedb` (no `withParsedStoredActions` reference): `Tests 5 failed | 2 passed (7)`. The two greens are the idempotency and no-`actions` controls. Restored: blob `92f1deab22` equals HEAD, and `git diff HEAD` is empty. - **Ablation** (`scripts/ablation-replace.mjs --delete`, anchor `request.item = withParsedStoredActions(singularType, request.item, parsed.data);`, hit once): blob `92f1deab22` to `2ecf252721`, anchor 1 to 0. Result: `Tests 5 failed | 2 passed (7)`. The tool restored it: `blob == HEAD (92f1dea) and git diff HEAD is empty`. The test imports `./protocol.js` from source, so there is no dist leg. </details> ## Acceptance notes - **Stored rows are not migrated** (ruling). The showcase, the other examples and HotCRM declare neither key on any action (0 carriers in source), so none of their rows carry it from code. A row an author saved through Studio before this fix keeps the raw keys until it is saved again; saving it again stores it lowered. - **What else the parse adds** to a stored action: schema defaults, and a CEL string shorthand wrapped in `{ dialect, source }`. That is the shape a packaged action is already served in. - **The UI needs no change.** Nothing under `ui/**` reads `requiresFeature` or `requiresMembershipReach` at runtime. `ui/packages/core/src/actions/ActionRunner.ts:469` is a type declaration only, and the renderers read `visible`. - **Studio round trip.** After a save, the action shows the composed `visible` and no gate key, as a packaged action already does. - **Overlap.** #26856 is the only open PR touching `protocol.ts`, and its hunks are far from this one: they sit at lines 216 to 11001, and this hunk is at about line 20950. ## Verification Head `91803f6f`. <details><summary>Package suites and typecheck</summary> - `pnpm --filter @objectstack/metadata-protocol test`: `Test Files 271 passed | 4 skipped (275)`, `Tests 21669 passed | 26 skipped (21695)`, exit 0. `typecheck` exit 0, `build` exit 0. Run at `a55b8f4b`; the only later commit is the engine-double ledger JSON. - Consumer files that save actions through the door, after building the `@objectstack/runtime^...` and `@objectstack/rest^...` closures with this fix in `metadata-protocol/dist`: - runtime: `http-dispatcher`, `action-activation-unpackaged`, `field-error-code-catalog`, `mcp-package-object-discovery`, `actions-disabled-package`, `meta-overlay-read-your-writes`, `stored-metadata-body-boundary.pin`, `stored-metadata-reader-contexts.pin`. Result: `Test Files 8 passed (8)`, `Tests 544 passed (544)`. - rest: `rest.test.ts`. Result: `Tests 215 passed (215)`. - `eslint --no-inline-config` over the three touched `metadata-protocol` files: exit 0. - `test:repo` and the full runtime and rest suites are left to CI (Test Core). </details> <details><summary>Gates</summary> - `dispatch-gates.mjs --commands` on this diff derives 69 families, and all 69 were run at `91803f6f`. `--ran` reconciles them as 68 run and 1 NOT-MEASURED. - The NOT-MEASURED family is `check:dual-build-cjs-loads`, which exited 3 (PREREQUISITE NOT MET: it needs every package's `dist/`, and this worktree has only the closures built). CI runs it. - `check:engine-double-contract` first named the new test's double. The ledger was updated with `--write` in the second commit, and the family is green. - `check-plugin-teardown-shape --self-test` exited 3 on the shallow clone. After fetching its control commit it passed (48 cases). </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FEkQ5kBygwShyUWpFansVP --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5kBygwShyUWpFansVP)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2720874 commit 57e00f5

5 files changed

Lines changed: 225 additions & 0 deletions

File tree

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
fix(metadata-protocol): the metadata save door stores an action as its `ActionSchema` parse, so a declared `requiresFeature` / `requiresMembershipReach` is lowered into `visible`
6+
7+
Clause-②: no — the save door refuses nothing it accepted before. It already judged the body by the same parse; what changes is only what it stores for an action that passed.
8+
9+
Metadata-protocol: none
10+
11+
`PUT /api/v1/meta/object/NAME` (draft and active) and a standalone `action` save judged an action by `ActionSchema`, then stored the request body as written. `ActionSchema`'s parse lowers `requiresMembershipReach` and `requiresFeature` into `visible`, and the console reads `visible` alone, so the stored action kept both gate keys raw, had no `visible`, and its button was offered to every user.
12+
13+
Now the door stores the parse it already ran:
14+
15+
- an object's own `actions` array is stored as the parsed array; every other key of the object body stays as written;
16+
- a standalone `action` item is stored as the parsed action.
17+
18+
A stored action therefore carries the lowered `visible` (and its schema defaults, and a CEL shorthand wrapped in `{ dialect, source }`), the same shape a code package's action has been stored in since #25981. The parse is idempotent over its own output, so a GET then PUT round trip stores the same bytes.
19+
20+
Rows already saved before this fix are not rewritten. Saving the object (or action) again stores it lowered.
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#26723] The actions a metadata save stores, as their `ActionSchema` parse.
5+
*
6+
* The save door judges a body by its type's spec schema and then keeps the
7+
* request body as written, with a few normalizations grafted back
8+
* (`graftFoldedFormSections`, `graftNormalizedOperators`). For an action that
9+
* dropped the one transform a reader depends on: `ActionSchema` lowers
10+
* `requiresMembershipReach` and `requiresFeature` into `visible`, the console
11+
* reads `visible` alone, and the stored body carried the two gate keys raw with
12+
* no `visible`. The declared gate did nothing: the button was offered to every
13+
* caller.
14+
*
15+
* So an action is stored as the parse the door already ran, the same parse the
16+
* code-package registration door stores since #25981 (`parsePackageAction` in
17+
* `@objectstack/objectql`'s registry). ⛔ No second copy of the lowering: the
18+
* values here are the parse's own output, read off the `parsed` tree, never
19+
* re-derived. The parse is idempotent over its own output, so a GET → PUT
20+
* round trip of a stored body stores the same bytes.
21+
*
22+
* - `object`: its own `actions` array is replaced by the parsed array
23+
* (`ObjectSchema`'s `actions` is `ActionSchema[]`); every other key of the
24+
* body stays as written.
25+
* - `action`: the body is the parsed action.
26+
* - Any other type, or an object with no `actions` array, is returned as is.
27+
*/
28+
export function withParsedStoredActions(singularType: string, authored: unknown, parsed: unknown): unknown {
29+
if (singularType === 'action') return parsed;
30+
if (singularType !== 'object') return authored;
31+
if (!authored || typeof authored !== 'object' || Array.isArray(authored)) return authored;
32+
const actions = (parsed as { actions?: unknown } | null | undefined)?.actions;
33+
if (!Array.isArray((authored as { actions?: unknown }).actions) || !Array.isArray(actions)) return authored;
34+
return { ...(authored as Record<string, unknown>), actions };
35+
}
Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#26723] An action the metadata save door stores is stored as its
5+
* `ActionSchema` parse, so a declared `requiresFeature` /
6+
* `requiresMembershipReach` is lowered into `visible` — on an object's own
7+
* `actions` and on a standalone `action` item, in draft and in active saves.
8+
*
9+
* The door already judged the body by that parse (`ObjectSchema`'s `actions`
10+
* is `ActionSchema[]`), then stored the request body as written: the gate keys
11+
* rode along raw and no `visible` was stored, so the console — which reads
12+
* `visible` alone — offered the button to every caller. The code-package
13+
* registration door has stored the parse since #25981; this is the save-door
14+
* sibling of that fix.
15+
*/
16+
import { describe, expect, it } from 'vitest';
17+
import { ObjectSchema } from '@objectstack/spec/data';
18+
import { ActionSchema } from '@objectstack/spec/ui';
19+
import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core';
20+
import { ObjectStackProtocolImplementation } from './protocol.js';
21+
22+
/** The engine surface the repository write path touches (as the external-binding harness). */
23+
function makeProtocol() {
24+
const rows: Record<string, unknown>[] = [];
25+
let nextId = 0;
26+
const engine: any = {
27+
async findOne(object: string, query?: EngineFindOneQueryInput) {
28+
assertEngineFindOnePredicate(object, query); return null; },
29+
async find() { return []; },
30+
async insert(table: string, data: Record<string, unknown>) {
31+
nextId += 1;
32+
if (table === 'sys_metadata') rows.push(data);
33+
return { id: `r_${nextId}` };
34+
},
35+
async update(_t: string, data: Record<string, unknown>, opts?: Record<string, unknown>) {
36+
assertEngineUpdateDispatch(data, opts);
37+
return { id: null };
38+
},
39+
async delete(_t: string, opts?: Record<string, unknown>) {
40+
assertEngineDeleteDispatch(opts);
41+
return { deleted: 0 };
42+
},
43+
registry: { registerItem: () => {}, registerObject: () => {}, getItem: () => undefined },
44+
};
45+
const protocol: any = new ObjectStackProtocolImplementation(engine, () => new Map());
46+
return { protocol, rows };
47+
}
48+
49+
/** The body of the one `sys_metadata` row a save wrote, as stored. */
50+
function storedBody(rows: Record<string, unknown>[], type: string, name: string): any {
51+
const hits = rows.filter((r) => r.type === type && r.name === name);
52+
expect(hits).toHaveLength(1);
53+
const metadata = hits[0].metadata;
54+
return typeof metadata === 'string' ? JSON.parse(metadata) : metadata;
55+
}
56+
57+
const SSO_ONLY = {
58+
name: 'sso_only', label: 'SSO only', type: 'url' as const, target: 'https://example.com/sso',
59+
locations: ['record_header' as const], requiresFeature: 'ssoEnforced' as const,
60+
};
61+
const OWNER_ONLY = {
62+
name: 'owner_only', label: 'Owner only', type: 'url' as const, target: 'https://example.com/owner',
63+
locations: ['record_header' as const], visible: "record.name != ''",
64+
requiresMembershipReach: 'delete_organization' as const,
65+
};
66+
const PLAIN = {
67+
name: 'plain', label: 'Plain', type: 'url' as const, target: 'https://example.com/plain',
68+
locations: ['record_header' as const],
69+
};
70+
71+
const order = (actions: unknown[]) => ({
72+
name: 'gap_order',
73+
label: 'Order',
74+
sharingModel: 'private',
75+
fields: { name: { name: 'name', label: 'Name', type: 'text' } },
76+
actions,
77+
});
78+
79+
/** What the spec's own parse makes of the same declaration. */
80+
const parsedAction = (action: unknown) => ActionSchema.parse(action) as any;
81+
82+
describe('#26723 the save door stores an action as its ActionSchema parse', () => {
83+
it.each(['draft', 'publish'] as const)(
84+
'an object saved in %s mode stores its gated actions lowered into `visible`',
85+
async (mode) => {
86+
const { protocol, rows } = makeProtocol();
87+
const result = await protocol.saveMetaItem({
88+
type: 'object', name: 'gap_order', mode,
89+
item: order([structuredClone(SSO_ONLY), structuredClone(OWNER_ONLY)]),
90+
});
91+
expect(result.success).toBe(true);
92+
93+
const stored = storedBody(rows, 'object', 'gap_order');
94+
const [sso, owner] = stored.actions;
95+
expect(sso).not.toHaveProperty('requiresFeature');
96+
// `ssoEnforced` is an opt-in flag, so its gate reads `== true`.
97+
expect(sso.visible).toEqual({ dialect: 'cel', source: 'features.ssoEnforced == true' });
98+
expect(owner).not.toHaveProperty('requiresMembershipReach');
99+
expect(owner.visible).toEqual(parsedAction(OWNER_ONLY).visible);
100+
// The same actions `ObjectSchema.create` gives the same declaration.
101+
const authored: any = ObjectSchema.create(order([SSO_ONLY, OWNER_ONLY]) as any);
102+
expect(stored.actions).toEqual(authored.actions);
103+
},
104+
);
105+
106+
it('control: an action without the gate keys is stored as its parse, and the rest of the body as written', async () => {
107+
const { protocol, rows } = makeProtocol();
108+
const item = order([structuredClone(PLAIN)]);
109+
await protocol.saveMetaItem({ type: 'object', name: 'gap_order', item });
110+
111+
const stored = storedBody(rows, 'object', 'gap_order');
112+
expect(stored.actions).toEqual([parsedAction(PLAIN)]);
113+
expect(stored.actions[0]).not.toHaveProperty('visible');
114+
expect(stored.fields).toEqual(item.fields);
115+
});
116+
117+
it('re-saving a stored body is a no-op: the parse is idempotent over its own output', async () => {
118+
const first = makeProtocol();
119+
await first.protocol.saveMetaItem({ type: 'object', name: 'gap_order', item: order([structuredClone(SSO_ONLY)]) });
120+
const once = storedBody(first.rows, 'object', 'gap_order');
121+
122+
const second = makeProtocol();
123+
await second.protocol.saveMetaItem({ type: 'object', name: 'gap_order', item: structuredClone(once) });
124+
expect(storedBody(second.rows, 'object', 'gap_order').actions).toEqual(once.actions);
125+
});
126+
127+
it('an object with no `actions` key is stored without one', async () => {
128+
const { protocol, rows } = makeProtocol();
129+
const { actions: _none, ...bare } = order([]);
130+
await protocol.saveMetaItem({ type: 'object', name: 'gap_order', item: bare });
131+
expect(storedBody(rows, 'object', 'gap_order')).not.toHaveProperty('actions');
132+
});
133+
134+
it.each(['draft', 'publish'] as const)(
135+
'a standalone action saved in %s mode is stored lowered',
136+
async (mode) => {
137+
const { protocol, rows } = makeProtocol();
138+
const result = await protocol.saveMetaItem({
139+
type: 'action', name: 'sso_only', mode,
140+
item: { ...structuredClone(SSO_ONLY), objectName: 'gap_order' },
141+
});
142+
expect(result.success).toBe(true);
143+
144+
const stored = storedBody(rows, 'action', 'sso_only');
145+
expect(stored).not.toHaveProperty('requiresFeature');
146+
expect(stored.visible).toEqual({ dialect: 'cel', source: 'features.ssoEnforced == true' });
147+
expect(stored).toEqual(parsedAction({ ...SSO_ONLY, objectName: 'gap_order' }));
148+
},
149+
);
150+
});

‎packages/metadata-protocol/src/protocol.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,7 @@ import { reportSealedOverlayRows } from './protocol-sealed-overlay-report.js';
405405
import { expandUnderOwnName, resolveRowlessExpandedView } from './protocol-view-expansion-names.js';
406406
// [#25467 cut 23] The read-side conversion of a stored item: the ADR-0087 chain with its notices, and the warned-once read with the page `type` default.
407407
import { convertStoredItem, convertStoredItemDetailed } from './protocol-stored-item-conversion.js';
408+
import { withParsedStoredActions } from './protocol-stored-action-parse.js';
408409
// [#25467 cut 24] The live metadata-type listing: the SchemaRegistry's types and the `metadata` service's, de-duplicated.
409410
import { listLiveMetadataTypes } from './protocol-live-metadata-types.js';
410411
// [#25467 cut 24] The save door's 409 for a `ConflictError`, with its decision-audit row.
@@ -20929,6 +20930,10 @@ export class ObjectStackProtocolImplementation implements
2092920930
graftFoldedFormSections(request.item, parsed.data),
2093020931
parsed.data,
2093120932
);
20933+
// [#26723] …and an action is stored as its parse, so its
20934+
// `requiresFeature` / `requiresMembershipReach` is lowered
20935+
// into `visible`. See {@link withParsedStoredActions}.
20936+
request.item = withParsedStoredActions(singularType, request.item, parsed.data);
2093220937
}
2093320938
// [#20101] …and a page's declared `type` default, the one
2093420939
// default that a reader of the served body selects on. It is

‎scripts/engine-double-contract.pinned.json‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1706,6 +1706,21 @@
17061706
"verb": "update",
17071707
"pinned": 1
17081708
},
1709+
{
1710+
"file": "packages/metadata-protocol/src/protocol.stored-action-gate-lowering.test.ts",
1711+
"verb": "delete",
1712+
"pinned": 1
1713+
},
1714+
{
1715+
"file": "packages/metadata-protocol/src/protocol.stored-action-gate-lowering.test.ts",
1716+
"verb": "findOne",
1717+
"pinned": 1
1718+
},
1719+
{
1720+
"file": "packages/metadata-protocol/src/protocol.stored-action-gate-lowering.test.ts",
1721+
"verb": "update",
1722+
"pinned": 1
1723+
},
17091724
{
17101725
"file": "packages/metadata-protocol/src/protocol.stored-conversions.test.ts",
17111726
"verb": "delete",

0 commit comments

Comments
 (0)