diff --git a/.changeset/action-strict-envelope-zero.md b/.changeset/action-strict-envelope-zero.md new file mode 100644 index 0000000000..63785cc338 --- /dev/null +++ b/.changeset/action-strict-envelope-zero.md @@ -0,0 +1,26 @@ +--- +'@objectstack/spec': minor +--- + +`action` rejects unknown keys, and the ADR-0010 protection-envelope debt list reaches zero. + +`ActionParamSchema` has been strict since #3746 — the template this whole campaign was generalized from, and the source of its sharpest lesson: `visibleWhen` → `visible` showed that the most valuable entry in an alias table is rarely a typo, it is a key that reads as a control and silently is not one. The action *around* the param stayed open for three more releases. + +**The AI exposure block is the reason this one mattered.** `ActionAiSchema` is the governance gate — its own doc says the platform's value is that "a human can govern exactly which capabilities the agent fleet is allowed to invoke", and that "a half-finished or unreviewed action must never be silently armed". Yet `requireConfirmation` (one letter off `requiresConfirmation`) was dropped in silence, so an author who asked for a human-in-the-loop gate on an AI-invoked action did not get one and was not told. Both that block and the action root now reject, with prescriptions for the two keys authors reach for at the wrong level (`exposed` and `requiresConfirmation` belong under `ai`). + +**An action's capability gate is real, and the near-misses now rename onto it.** `requiredPermissions` (ADR-0066 D4) is enforced with a 403 on the platform action route, so `permissions` / `capabilities` / `acl` are aliased to it rather than being told the gate lives elsewhere. What *is* tombstoned is the trap beside it: `visible` and `disabled` are UI predicates — **they hide or grey a button, they do not stop a request** — and an action with no UI surface is `locations: []`, still gated. + +That entry was wrong in the first draft of this change, in the direction that matters. It claimed an action carries no permission key and sent authors to the object's permission sets, which — had anyone followed it — invites deleting a working `requiredPermissions` gate. Caught by checking the docs the drift report flagged (`ui/actions.mdx` teaches exactly that key) against the schema. It is the ledger's finding 7 for the fourth time: **this campaign's own prescriptions are themselves a surface that can be confidently wrong**, and the only defence is verifying each one against the schema rather than against memory of it. + +`resultDialog` and its fields, the AI param hints, and the `bodyShape` wrapper close alongside. + +**The undeclared-envelope debt list is now empty.** The structural walk opened it with eight names (`action`, `book`, `field`, `job`, `mapping`, `page`, `translation`, `validation`) after replacing a probe that had been hiding seven of them; `action` was the last. The empty set is kept rather than deleted — with no exemptions, the `DECLARES the protection envelope` case now runs over every registered type, so a new type shipping without the spread fails immediately instead of being quietly added to a list. Adding a name back is filing a bug, not granting an exemption. + +Registered types closed at the top level: **24 of 25**. Only `view` remains. + +Two things the lint layer surfaced, recorded rather than papered over: + +- **The array-index test has run out of subject.** It was `pages[].regions[]`, then `objects[].actions[]`; with `action` closed there is no declared array-of-objects left anywhere in the registered surface that is still strip-mode. The walker's array handling is unchanged and still correct — what is gone is any metadata type that exercises it. The test now asserts the hand-off plus the per-node descent under a closed root (the #4522 fix), and says in place that an indexed assertion should be restored if a new strip surface ever appears. +- **`view` is the last open root**, so when it closes this layer has nothing left to warn about at a root. The test says to change the floor to 0 and assert the empty set *deliberately* — not to delete the test, because an empty result nobody chose is indistinguishable from a derivation that broke. + +Authoring impact: a key `ActionSchema` does not declare is now rejected instead of silently discarded — it was already being ignored, so no working action changes. diff --git a/content/docs/references/ui/action.mdx b/content/docs/references/ui/action.mdx index 8aee98c97b..a03cf3e524 100644 --- a/content/docs/references/ui/action.mdx +++ b/content/docs/references/ui/action.mdx @@ -113,6 +113,13 @@ const result = Action.parse(data); | **opensInNewTab** | `boolean` | optional | Open the action result in a new tab. The renderer pre-opens the tab synchronously on click (popup-blocker-safe) and navigates it to the handler's redirectUrl. | | **newTabUrl** | `string` | optional | Direct new-tab URL template (`{recordId}` placeholder). When set with opensInNewTab, the renderer navigates the pre-opened tab here immediately — no action POST. The endpoint must enforce auth itself. | | **aria** | `{ ariaLabel?: string; ariaDescribedBy?: string; role?: string }` | optional | ARIA accessibility attributes | +| **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). | +| **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. | +| **_lockSource** | `Enum<'artifact' \| 'package' \| 'env-forced'>` | optional | Layer that set _lock (artifact \| package \| env-forced). | +| **_provenance** | `Enum<'package' \| 'org' \| 'env-forced'>` | optional | Origin of the item (package \| org \| env-forced). | +| **_packageId** | `string` | optional | Owning package machine id. | +| **_packageVersion** | `string` | optional | Owning package version. | +| **_lockDocsUrl** | `string` | optional | Optional documentation link surfaced next to _lockReason. | --- diff --git a/packages/metadata-protocol/src/protocol.stored-conversions.test.ts b/packages/metadata-protocol/src/protocol.stored-conversions.test.ts index 7fc3f6d0f0..bc46b94a27 100644 --- a/packages/metadata-protocol/src/protocol.stored-conversions.test.ts +++ b/packages/metadata-protocol/src/protocol.stored-conversions.test.ts @@ -85,10 +85,15 @@ const legacyObjectRow = { }; // A pre-17 standalone action row still carrying the removed `execute` alias. +// NOTE: this once wrote `object: 'crm_invoice'`. `ActionSchema` has never +// declared `object` — the key is `objectName` — and `.strip` ate it, so the +// fixture read as a claim about real legacy data while being a typo. Closing +// the shape (#4001) made it a diagnostic. Sixth strip-era fiction this campaign +// has found in a test, and the first one dressed as a stored ROW. const legacyActionRow = { type: 'action', name: 'convert', - metadata: { name: 'convert', label: 'Convert', type: 'script', object: 'crm_invoice', execute: 'convertHandler' }, + metadata: { name: 'convert', label: 'Convert', type: 'script', objectName: 'crm_invoice', execute: 'convertHandler' }, }; describe('getMetaItems — stored rows are served canonical (#3903)', () => { diff --git a/packages/metadata-protocol/src/protocol.stored-migration.test.ts b/packages/metadata-protocol/src/protocol.stored-migration.test.ts index 5ee1c189df..3d67c4b8a8 100644 --- a/packages/metadata-protocol/src/protocol.stored-migration.test.ts +++ b/packages/metadata-protocol/src/protocol.stored-migration.test.ts @@ -139,7 +139,7 @@ const canonicalObjectRow = { const legacyActionRow = { type: 'action', name: 'convert', - metadata: { name: 'convert', label: 'Convert', type: 'script', object: 'crm_invoice', execute: 'convertHandler' }, + metadata: { name: 'convert', label: 'Convert', type: 'script', objectName: 'crm_invoice', execute: 'convertHandler' }, }; describe('migrateStoredMetadata — preview (#4327)', () => { diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index 653d10799f..a1bba850c4 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -7103,6 +7103,13 @@ "ui/AIChatWindowProps:aria", "ui/AIChatWindowProps:context", "ui/AIChatWindowProps:mode", + "ui/Action:_lock", + "ui/Action:_lockDocsUrl", + "ui/Action:_lockReason", + "ui/Action:_lockSource", + "ui/Action:_packageId", + "ui/Action:_packageVersion", + "ui/Action:_provenance", "ui/Action:ai", "ui/Action:aria", "ui/Action:body", diff --git a/packages/spec/src/kernel/metadata-authoring-lint.test.ts b/packages/spec/src/kernel/metadata-authoring-lint.test.ts index 4adf1e357c..b36745e52d 100644 --- a/packages/spec/src/kernel/metadata-authoring-lint.test.ts +++ b/packages/spec/src/kernel/metadata-authoring-lint.test.ts @@ -54,17 +54,20 @@ describe('coverage derivation (#3786 — no third hand-written list)', () => { // `page.regions[0].zzz` all `safeParse` to failure — and the check is worth // repeating on the next one, because a broken walk and a successful // graduation shrink this count identically. - // 3 → 2 when `dashboard` closed; `dashboard.zzz` was confirmed rejected by - // the parse first, same as the batch before it. - expect(lintables.length).toBeGreaterThanOrEqual(2); - // `view` matters doubly: it is a UNION (container | ViewItem | overlay), so - // its presence pins the union half of the posture logic — a regression that - // silently dropped unions would shrink coverage without failing the count. - // When `view` and `action` close, this whole layer has nothing left to warn - // about at a ROOT, which is the campaign finishing rather than the lint - // breaking — at that point assert the empty set deliberately, do not delete - // the test. - for (const expected of ['action', 'view']) { + // 3 → 2 when `dashboard` closed, 2 → 1 when `action` did; in each case the + // departed root was confirmed rejected by the parse before the number moved. + expect(lintables.length).toBeGreaterThanOrEqual(1); + // `view` is the LAST open root, and it matters doubly: it is a UNION + // (container | ViewItem | overlay), so its presence pins the union half of + // the posture logic — a regression that silently dropped unions would shrink + // coverage without failing the count. + // + // When `view` closes, this layer has nothing left to warn about at a ROOT. + // That is the campaign finishing, not the lint breaking. At that point change + // the floor to 0 and assert the empty set DELIBERATELY — do not delete this + // test, because an empty result that nobody chose is indistinguishable from + // a derivation that broke. + for (const expected of ['view']) { expect(lintableTypes, `expected '${expected}' to be lint-covered`).toContain(expected); } }); @@ -177,7 +180,7 @@ describe('the #4148 behaviours survive the generalization', () => { // strip site under a CLOSED root (the #4522 behaviour), and `view` — the // last open root, and the union case — reports at its own. const findings = lintUnknownAuthoringKeys({ - objects: [{ name: 'a', label: 'A', actions: [{ name: 'act', zzz: 1 }] }], + objects: [{ name: 'a', label: 'A', userActions: { zzz: 1 } }], views: [{ name: 'v', object: 'a', zzz: 1 }], }); // Deduped deliberately: `view` is a union (container | ViewItem | overlay) @@ -187,7 +190,7 @@ describe('the #4148 behaviours survive the generalization', () => { // and it becomes moot when `view` closes. Left recorded rather than papered // over by picking a non-union collection. expect([...new Set(findings.map((f) => `${f.surface}:${f.path}`))].sort()).toEqual([ - 'object:objects.a.actions.0.zzz', + 'object:objects.a.userActions.zzz', 'view:views.v.zzz', ]); }); @@ -241,17 +244,29 @@ describe('nested descent (#4001 evidence phase)', () => { }); it('reports inside an array element, indexed by position', () => { - // Was `pages[].regions[]` until `page` closed (#4001 batch 6a) — the parse - // rejects that key now. `object.actions[]` is the same structural case and - // carries a second property worth pinning: `object` itself is CLOSED, and - // its nested strip sites still report. That is the #4522 fix — the walk - // descends per node instead of gating a whole collection on its root's - // posture — so this test now covers the array index and that regression at - // once. - const [finding] = lintUnknownAuthoringKeys({ + // This test has now run out of subject, and that is worth saying plainly + // rather than deleting it or inventing a fixture. + // + // It was `pages[].regions[]`, then `objects[].actions[]` when `page` closed + // (6a), and with `action` closed (6d) there is no declared ARRAY OF OBJECTS + // left anywhere in the registered surface that is still strip-mode. The + // walker's array-index handling is unchanged and still correct; what is gone + // is any metadata type that exercises it. That is the ratchet finishing, not + // the walk regressing. + // + // So: assert the hand-off, and assert what still holds — the per-node + // descent under a CLOSED root, which is the #4522 fix and the reason the + // walk no longer gates a whole collection on its root's posture. If a new + // strip surface with a nested array ever appears, restore the indexed + // assertion here; do not let it go untested a second time. + expect(lintUnknownAuthoringKeys({ objects: [{ name: 'o1', actions: [{ name: 'a', zzz_nested: 1 }] }], + })).toEqual([]); + + const [nested] = lintUnknownAuthoringKeys({ + objects: [{ name: 'o1', userActions: { zzz_nested: 1 } }], }); - expect(finding).toMatchObject({ path: 'objects.o1.actions.0.zzz_nested', surface: 'object' }); + expect(nested).toMatchObject({ path: 'objects.o1.userActions.zzz_nested', surface: 'object' }); }); it('hands the field record and its nested array to the parse', () => { diff --git a/packages/spec/src/kernel/metadata-type-schemas.test.ts b/packages/spec/src/kernel/metadata-type-schemas.test.ts index e5ff7ef7e5..d53f05c58c 100644 --- a/packages/spec/src/kernel/metadata-type-schemas.test.ts +++ b/packages/spec/src/kernel/metadata-type-schemas.test.ts @@ -80,10 +80,16 @@ const PROBE: Record = { * actually checked (the other 24 took an early return), so it was the only * envelope gap anyone could see for as long as that probe was green — and it * outlasted every gap the probe was hiding. + * + * **This list is now EMPTY, and that is the end state — not a reason to delete + * it.** Every registered type declares the envelope its loader stamps. Keeping + * the empty set means the `DECLARES the protection envelope` case below runs + * over ALL types with no exemptions, so the day a new registered type ships + * without the spread it fails immediately rather than being quietly added here. + * If you find yourself adding a name back, that is a bug being filed, not an + * exemption being granted. */ -const UNDECLARED_ENVELOPE = new Set([ - 'action', -]); +const UNDECLARED_ENVELOPE = new Set([]); /** * Every object shape reachable from `schema`, unwrapping the wrappers the @@ -218,7 +224,7 @@ describe('registered metadata types', () => { * type fails this suite until the list shrinks, so the list cannot outlive the * debt and start exempting types that no longer need exempting. */ -const STILL_STRIP = new Set(['action', 'view']); +const STILL_STRIP = new Set(['view']); /** The registered schema's own top-level posture: `.strict()` sets a `never` catchall. */ function topLevelPosture(schema: unknown, depth = 0): 'strict' | 'strip' | null { @@ -285,7 +291,7 @@ describe('#4001 — registered-type closure is derived, not tallied', () => { it('reports the campaign number so a reader never has to count', () => { const closed = types.filter((t) => !STILL_STRIP.has(t)); expect(closed.length + STILL_STRIP.size).toBe(types.length); - expect(closed.length).toBe(23); + expect(closed.length).toBe(24); expect(types.length).toBe(25); }); }); diff --git a/packages/spec/src/ui/action.zod.ts b/packages/spec/src/ui/action.zod.ts index e8e5c370dd..56a05e6457 100644 --- a/packages/spec/src/ui/action.zod.ts +++ b/packages/spec/src/ui/action.zod.ts @@ -11,6 +11,8 @@ import { HookBodySchema } from '../data/hook-body.zod'; // deliberately import-free, so this cannot introduce a cycle. import { PUBLIC_AUTH_FEATURE_NAMES, lowerRequiresFeature } from '../kernel/public-auth-features'; import { strictUnknownKeyError } from '../shared/suggestions.zod'; +import { strictObject } from '../shared/strict-object'; +import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; /** * Action Parameter Schema @@ -325,7 +327,45 @@ const ActionAiCategorySchema = z.enum([ * UI `label`. The bridge in `@objectstack/service-ai` translates this block * into an `AIToolDefinition`. */ -export const ActionAiSchema = z.object({ +/** + * Shared history for this file (#4001). + * + * `ActionParamSchema` has been strict since #3746 — the campaign's own template, + * where `visibleWhen` → `visible` proved that the most valuable alias entry is + * rarely a typo but a key that reads as a control and silently is not one. The + * action AROUND the param stayed open for three more releases. + */ +const ACTION_HISTORY = + 'Until #4001 closed this shape these were dropped silently — the action still registered ' + + 'and still ran, without whatever the key was meant to configure or gate.'; + +export const ActionAiSchema = strictObject({ + surface: "this action's AI exposure block", + history: ACTION_HISTORY, + aliases: { + enabled: 'exposed', enable: 'exposed', aiEnabled: 'exposed', expose: 'exposed', visible: 'exposed', + prompt: 'description', toolDescription: 'description', summary: 'description', + type: 'category', kind: 'category', toolCategory: 'category', + hints: 'paramHints', parameterHints: 'paramHints', params: 'paramHints', + returns: 'outputSchema', responseSchema: 'outputSchema', output: 'outputSchema', + confirm: 'requiresConfirmation', requireConfirmation: 'requiresConfirmation', hitl: 'requiresConfirmation', humanInTheLoop: 'requiresConfirmation', + }, + guidance: { + // This block IS the governance gate — the doc above says a half-finished or + // unreviewed action must never be silently armed. A near-miss here is + // therefore the worst kind on this surface: the author believes they set a + // gate, and the gate does not exist. Name the two people reach for. + permissions: + 'AI invocation is not gated by a key here — an agent reaches this action only if a ' + + "surface-compatible SKILL declares it (ADR-0064), and who may talk to that agent is " + + "gated by the agent's `access` / `permissions` (enforced at the chat route since #1884). " + + 'For a human-approval step on the call itself, use `requiresConfirmation: true`.', + approval: + 'there is no approval workflow key here — `requiresConfirmation: true` forces a ' + + 'human-in-the-loop gate on the AI call. A multi-step business approval is an `approval` ' + + 'metadata item, not an action field.', + }, +}, { /** * Expose this action to AI agents as a callable tool. Default `false`. * Setting `true` REQUIRES `description`. @@ -351,7 +391,11 @@ export const ActionAiSchema = z.object({ * `description`, supply `examples`) WITHOUT changing the UI-facing field * metadata. Keys must match a declared `params[].name` (or `recordId`). */ - paramHints: z.record(z.string(), z.object({ + paramHints: z.record(z.string(), strictObject({ + surface: 'this AI parameter hint', + history: ACTION_HISTORY, + aliases: { desc: 'description', hint: 'description', values: 'enum', options: 'enum', choices: 'enum', allowed: 'enum', example: 'examples', sample: 'examples' }, + }, { description: z.string().optional(), enum: z.array(z.union([z.string(), z.number()])).optional(), examples: z.array(z.unknown()).optional(), @@ -390,7 +434,54 @@ export type ActionAi = z.infer; * restating a dozen field definitions and their `describe()` text, which is how * a second action vocabulary would start. */ -const actionObject = () => z.object({ +const actionObject = () => strictObject({ + surface: 'this action', + history: ACTION_HISTORY, + aliases: { + title: 'label', displayName: 'label', text: 'label', + object: 'objectName', entity: 'objectName', + actionType: 'type', + url: 'target', endpoint: 'target', path: 'target', href: 'target', + parameters: 'params', args: 'params', inputs: 'params', fields: 'params', + confirm: 'confirmText', confirmation: 'confirmText', confirmMessage: 'confirmText', + success: 'successMessage', successText: 'successMessage', toast: 'successMessage', + visibleWhen: 'visible', showWhen: 'visible', + disabledWhen: 'disabled', + style: 'variant', color: 'variant', appearance: 'variant', + placement: 'locations', location: 'locations', position: 'locations', + verb: 'method', httpMethod: 'method', + body: 'bodyExtra', payload: 'bodyExtra', + llm: 'ai', tool: 'ai', + dialog: 'resultDialog', result: 'resultDialog', + refresh: 'refreshAfter', reload: 'refreshAfter', + // The capability gate on an action IS a declared key — `requiredPermissions` + // (ADR-0066 D4), enforced with a 403 on the platform action route. So the + // near-misses of it must RENAME onto it, never be told the gate lives + // somewhere else. + permissions: 'requiredPermissions', capabilities: 'requiredPermissions', + requiresPermissions: 'requiredPermissions', requiredCapabilities: 'requiredPermissions', + acl: 'requiredPermissions', + }, + guidance: { + // `visible` / `disabled` are the trap worth naming on this surface: they + // look like access control and are not. The real gate is + // `requiredPermissions`, which is why the near-misses above rename onto it + // rather than pointing anywhere else. + hidden: + '`hidden` is not an action key, and hiding is not gating — `visible` and `disabled` are UI ' + + 'predicates that hide or grey a button, they do not stop a request. To actually gate ' + + 'invocation use `requiredPermissions` (ADR-0066 D4, enforced with a 403 on the platform ' + + 'action route). To declare an action with no UI surface at all, set `locations: []`.', + // Two AI-block keys authors reach for at the top level. Silently stripping + // either meant an action was armed for agents, or left ungated, in silence. + exposed: + 'AI exposure lives under `ai` — write `ai: { exposed: true, description: … }`. The ' + + 'description is the LLM-facing contract and is REQUIRED (≥40 chars) whenever exposed.', + requiresConfirmation: + 'the AI human-in-the-loop override lives under `ai` — write ' + + '`ai: { requiresConfirmation: true }`. `confirmText` is the separate UI confirm prompt.', + }, +}, { /** Machine name of the action */ name: SnakeCaseIdentifierSchema.describe('Machine name (lowercase snake_case)'), @@ -578,12 +669,20 @@ const actionObject = () => z.object({ * The dialog SHOULD set `refreshAfter` to true on close (separate from * the existing `refreshAfter` flag, which fires immediately on success). */ - resultDialog: z.object({ + resultDialog: strictObject({ + surface: 'this result dialog', + history: ACTION_HISTORY, + aliases: { label: 'title', heading: 'title', message: 'description', body: 'description', ok: 'acknowledge', confirm: 'acknowledge', button: 'acknowledge', display: 'format', render: 'format', show: 'fields', reveal: 'fields' }, + }, { title: I18nLabelSchema.optional(), description: I18nLabelSchema.optional(), acknowledge: I18nLabelSchema.optional().describe('Acknowledge button label, e.g. "I have saved this"'), format: z.enum(['qrcode', 'code-list', 'secret', 'text', 'json']).optional().describe('Default format for fields without their own format. Defaults to json when omitted.'), - fields: z.array(z.object({ + fields: z.array(strictObject({ + surface: 'this result dialog field', + history: ACTION_HISTORY, + aliases: { key: 'path', field: 'path', name: 'path', title: 'label', display: 'format', render: 'format' }, + }, { path: z.string().describe('Dot path into result.data (e.g. "totpURI", "client.client_secret").'), label: I18nLabelSchema.optional(), format: z.enum(['qrcode', 'code-list', 'secret', 'text', 'json']).optional().describe('Per-field format override.'), @@ -669,7 +768,11 @@ const actionObject = () => z.object({ */ bodyShape: z.union([ z.literal('flat'), - z.object({ wrap: z.string() }), + strictObject({ + surface: 'this body shape', + history: ACTION_HISTORY, + aliases: { key: 'wrap', under: 'wrap', nest: 'wrap', root: 'wrap' }, + }, { wrap: z.string() }), ]).optional().describe('Body wrapping: flat (default) or { wrap: key } to nest user-collected params under a key.'), /** * HTTP method to use when `type: 'api'`. Defaults to `POST`. Use `PATCH` to @@ -712,6 +815,13 @@ const actionObject = () => z.object({ /** ARIA accessibility attributes */ aria: AriaPropsSchema.optional().describe('ARIA accessibility attributes'), + + // ADR-0010 — runtime protection envelope (internal — set by the loader). + // `action` is a registered metadata type, so `MetadataPlugin`'s loader stamps + // `_packageId` / `_provenance` on it. Undeclared, they were dropped on every + // parse. This is the LAST name on the undeclared-envelope debt list the + // structural walk opened with eight of. + ...MetadataProtectionFields, }); export const ActionSchema = lazySchema(() => actionObject().refine((data) => {