Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .changeset/retired-action-keys-unauthorable.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
"@object-ui/app-shell": minor
"@object-ui/core": minor
---

Stop offering the retired `action.shortcut` / `action.bulkEnabled` keys.

`@objectstack/spec` 17 retired both as `retiredKey()` tombstones: authoring
either one is a hard PARSE REJECTION, so a draft carrying it cannot be saved
at all. The designer still offered controls for both — a "Bulk — apply to
multiple selected rows" checkbox and a "Shortcut" text field — which meant the
Studio action inspector let an author build a draft the platform would then
refuse, with the rejection arriving later and nowhere near the checkbox.

- **Action inspector**: both controls removed. The keys stay hidden from the
fallback form (the server's live schema still advertises them, so dropping
them from the hidden list would put the inputs straight back) — now under a
`RETIRED_FIELDS` list that says why, so nobody "restores the missing
control". `bulkEnabled`'s replacement is the list view's `bulkActions` /
`bulkActionDefs`; `shortcut` has none.
- **Action preview**: the `shortcut` and `bulk` pills are gone — they could
only ever render for metadata the platform now refuses.
- **`ActionEngine.registerActions`**: no longer harvests the two retired keys
from authored metadata, which made two dead registration options look
load-bearing. Both are still accepted on the single-action
`registerAction(action, options)` overload, where a HOST passes them
explicitly.
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,29 @@ function ActionTargetField({ type, value, onCommit, cfg, readOnly }: {
const CURATED_FIELDS = [
'name', 'label', 'objectName', 'icon', 'variant', 'component',
'type', 'target', 'execute', 'body', 'method',
'params', 'locations', 'bulkEnabled',
'confirmText', 'successMessage', 'errorMessage', 'refreshAfter', 'undoable', 'mode', 'shortcut',
'params', 'locations',
'confirmText', 'successMessage', 'errorMessage', 'refreshAfter', 'undoable', 'mode',
'visible', 'disabled', 'aiExposed', 'aiDescription',
];

/**
* Keys hidden from the fallback form for the OPPOSITE reason: not because this
* inspector edits them, but because `@objectstack/spec` 17 retired them as
* `retiredKey()` tombstones — authoring either one is a hard PARSE REJECTION,
* so a draft carrying it cannot be saved at all.
*
* They stay listed because the fallback renders from the server's live schema,
* which still advertises both; dropping them here would put the inputs back.
* This inspector used to offer its own controls for them (a "Bulk — apply to
* multiple selected rows" checkbox and a "Shortcut" text field), which is how
* the designer let an author build a draft the platform would then refuse.
*
* Do not add controls back. `bulkEnabled`'s replacement is the LIST VIEW's
* `bulkActions` / `bulkActionDefs`; `shortcut` has none — register the key in
* the Console keyboard stack and have its handler invoke the action by name.
*/
const RETIRED_FIELDS = ['bulkEnabled', 'shortcut'];

/* ─────────────── small helpers ─────────────── */

function SectionHeader({ title, hint }: { title: string; hint?: string }) {
Expand Down Expand Up @@ -420,7 +438,9 @@ export function ActionDefaultInspector({
<div className="grid grid-cols-2 gap-2 pt-1">
<InspectorSelectField label="Component" value={str('component') || undefined} options={COMPONENT_OPTS} onCommit={(v) => onPatch({ component: v })} disabled={readOnly} />
</div>
<InspectorCheckboxField label="Bulk — apply to multiple selected rows" value={!!draft.bulkEnabled} onCommit={(v) => onPatch({ bulkEnabled: v })} disabled={readOnly} />
{/* No "Bulk" checkbox here: `action.bulkEnabled` is a spec-17
tombstone (see RETIRED_FIELDS). Selection placement is declared on
the LIST VIEW, in `bulkActions` / `bulkActionDefs`. */}
</div>

{/* 5 ─ Feedback */}
Expand All @@ -429,9 +449,11 @@ export function ActionDefaultInspector({
<InspectorTextField label="Confirm prompt" value={localize(draft.confirmText)} onCommit={(v) => onPatch({ confirmText: v })} placeholder="Ask before running (leave blank to skip)" disabled={readOnly} />
<InspectorTextField label="Success message" value={localize(draft.successMessage)} onCommit={(v) => onPatch({ successMessage: v })} disabled={readOnly} />
<InspectorTextField label="Error message" value={localize(draft.errorMessage)} onCommit={(v) => onPatch({ errorMessage: v })} disabled={readOnly} />
{/* No "Shortcut" field beside Mode: `action.shortcut` is a spec-17
tombstone (see RETIRED_FIELDS) — nothing ever read it, and
authoring it now fails the parse. */}
<div className="grid grid-cols-2 gap-2">
<InspectorSelectField label="Mode" value={str('mode') || undefined} options={MODE_OPTS} onCommit={(v) => onPatch({ mode: v })} disabled={readOnly} />
<InspectorTextField label="Shortcut" value={str('shortcut')} onCommit={(v) => onPatch({ shortcut: v })} placeholder="e.g. Ctrl+S" disabled={readOnly} mono />
</div>
<div className="flex flex-wrap gap-x-4 gap-y-1">
<InspectorCheckboxField label="Refresh view after" value={!!draft.refreshAfter} onCommit={(v) => onPatch({ refreshAfter: v })} disabled={readOnly} />
Expand Down Expand Up @@ -475,7 +497,7 @@ export function ActionDefaultInspector({
<SchemaForm
schema={fallbackSchema}
value={draft}
hiddenFields={CURATED_FIELDS}
hiddenFields={[...CURATED_FIELDS, ...RETIRED_FIELDS]}
readOnly={readOnly}
onChange={(next) => onPatch(next)}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* and `label` so authors can see the visual weight before they
* ship it (primary buttons are highlighted, danger turns red,
* icon-only actions render a compact icon button).
* 2. A metadata strip: type, target, locations, shortcut, bulk
* 2. A metadata strip: type, target, locations
* flag, AI exposure, refreshAfter, confirmText.
* 3. A params table when the action prompts the user — this is the
* modal/drawer it would open on click. We render it as a static
Expand All @@ -30,14 +30,12 @@ import {
Code2,
Eye,
Globe,
Keyboard,
LayoutGrid,
Link2,
Lock,
MoreHorizontal,
Pencil,
RefreshCw,
ScanLine,
Sparkles,
Square,
Workflow,
Expand Down Expand Up @@ -158,8 +156,9 @@ export function ActionPreview({ name, draft }: MetadataPreviewProps) {
const variant = (d.variant as string | undefined) || undefined;
const component = String(d.component ?? '');
const locations = Array.isArray(d.locations) ? (d.locations as string[]) : [];
const shortcut = (d.shortcut as string | undefined) || undefined;
const bulkEnabled = !!d.bulkEnabled;
// No `shortcut` / `bulkEnabled` here: both are spec-17 `retiredKey()`
// tombstones, so a preview of them could only ever render for metadata the
// platform now refuses to parse. See ActionDefaultInspector's RETIRED_FIELDS.
const refreshAfter = !!d.refreshAfter;
const aiExposed = d.aiExposed;
const confirmText = localize(d.confirmText);
Expand Down Expand Up @@ -204,8 +203,6 @@ export function ActionPreview({ name, draft }: MetadataPreviewProps) {
{objectName && <Pill icon={Square} label={`object: ${objectName}`} mono />}
{variant && <Pill label={`variant: ${variant}`} />}
{component && <Pill icon={MoreHorizontal} label={component} />}
{shortcut && <Pill icon={Keyboard} label={shortcut} mono />}
{bulkEnabled && <Pill icon={ScanLine} label="bulk" tone="green" />}
{refreshAfter && <Pill icon={RefreshCw} label="refresh after" />}
{aiExposed === false && <Pill icon={Bot} label="AI: opted out" tone="amber" />}
{aiExposed === true && <Pill icon={Sparkles} label="AI: exposed" />}
Expand Down
14 changes: 11 additions & 3 deletions packages/core/src/actions/ActionEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,21 @@ export class ActionEngine {
}
}

/** Register multiple actions from an ActionSchema array */
/**
* Register multiple actions from an ActionSchema array.
*
* Only `locations` is harvested from the metadata. `shortcut` and
* `bulkEnabled` used to be read here too, but spec 17 retired both as
* `retiredKey()` tombstones — a declaration carrying either one no longer
* parses, so harvesting them read a key that cannot exist and made two dead
* options look load-bearing. Both remain accepted on the single-action
* `registerAction(action, options)` overload, where a HOST passes them
* explicitly; they are simply no longer sourced from authored metadata.
*/
registerActions(actions: ActionDef[]): void {
for (const action of actions) {
this.registerAction(action, {
locations: (action as any).locations,
shortcut: (action as any).shortcut,
bulkEnabled: (action as any).bulkEnabled,
});
}
}
Expand Down
13 changes: 13 additions & 0 deletions packages/core/src/actions/__tests__/ActionEngine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,19 @@ describe('ActionEngine', () => {
expect(engine.getAction('save')).toBeDefined();
expect(engine.getAction('delete')).toBeDefined();
});

it('does not harvest the retired `shortcut` / `bulkEnabled` keys from metadata', () => {
// spec 17 retired both as `retiredKey()` tombstones — a declaration
// carrying either no longer parses, so reading them here made two dead
// options look load-bearing. A host may still pass them explicitly to
// `registerAction`; they are just not sourced from authored metadata.
engine.registerActions([
{ name: 'stale', type: 'api', shortcut: 'ctrl+k', bulkEnabled: true } as never,
]);
expect(engine.getAction('stale')).toBeDefined();
expect(engine.getShortcuts()).toHaveLength(0);
expect(engine.getBulkActions().map((a) => a.name)).not.toContain('stale');
});
});

describe('unregisterAction', () => {
Expand Down
26 changes: 19 additions & 7 deletions packages/react/src/hooks/__tests__/useActionEngine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,20 @@ describe('useActionEngine', () => {
});

describe('getBulkActions', () => {
it('returns only bulk-enabled actions', () => {
// Deliberately INVERTED. `sampleActions` still carries the stale
// `bulkEnabled: true` — spec 17 retired the key as a `retiredKey()`
// tombstone, so metadata like this no longer parses at all, and harvesting
// it here made a dead registration option look load-bearing. Same posture
// as plugin-grid's "ignores a stale bulkEnabled flag on an object action".
// The engine's bulk mechanics keep their coverage in ActionEngine.test.ts,
// where a HOST passes `{ bulkEnabled: true }` to `registerAction`
// explicitly — which is still supported.
it('does not harvest the retired bulkEnabled key from metadata', () => {
const { result } = renderHook(() =>
useActionEngine({ actions: sampleActions }),
);

const bulkActions = result.current.getBulkActions();
expect(bulkActions.length).toBe(1);
expect(bulkActions[0].name).toBe('mark_complete');
expect(result.current.getBulkActions()).toEqual([]);
});
});

Expand Down Expand Up @@ -119,7 +125,14 @@ describe('useActionEngine', () => {
});

describe('handleShortcut', () => {
it('handles registered keyboard shortcut', async () => {
// Deliberately INVERTED, for the same reason as getBulkActions above:
// `sampleActions` still declares the stale `shortcut: 'ctrl+k'`, which
// spec 17 retired. Its tombstone is explicit that nothing ever consumed it
// ("no keydown listener feeds ActionEngine.getShortcuts(), and objectui's
// keyboard stack is hand-registered and never consults action metadata"),
// so harvesting it only kept a dead path looking alive. A host that wants
// a shortcut still passes one to `registerAction` explicitly.
it('does not harvest the retired shortcut key from metadata', async () => {
const { result } = renderHook(() =>
useActionEngine({ actions: sampleActions }),
);
Expand All @@ -129,8 +142,7 @@ describe('useActionEngine', () => {
shortcutResult = await result.current.handleShortcut('ctrl+k');
});

expect(shortcutResult).not.toBeNull();
expect(shortcutResult.success).toBe(true);
expect(shortcutResult).toBeNull();
});

it('returns null for unregistered shortcut', async () => {
Expand Down
Loading