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
26 changes: 26 additions & 0 deletions .changeset/action-strict-envelope-zero.md
Original file line number Diff line number Diff line change
@@ -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.
7 changes: 7 additions & 0 deletions content/docs/references/ui/action.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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. |


---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)', () => {
Expand Down
7 changes: 7 additions & 0 deletions packages/spec/authorable-surface.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
59 changes: 37 additions & 22 deletions packages/spec/src/kernel/metadata-authoring-lint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
Expand Down Expand Up @@ -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)
Expand All @@ -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',
]);
});
Expand Down Expand Up @@ -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', () => {
Expand Down
16 changes: 11 additions & 5 deletions packages/spec/src/kernel/metadata-type-schemas.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,16 @@ const PROBE: Record<string, unknown> = {
* 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<string>([
'action',
]);
const UNDECLARED_ENVELOPE = new Set<string>([]);

/**
* Every object shape reachable from `schema`, unwrapping the wrappers the
Expand Down Expand Up @@ -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<string>(['action', 'view']);
const STILL_STRIP = new Set<string>(['view']);

/** The registered schema's own top-level posture: `.strict()` sets a `never` catchall. */
function topLevelPosture(schema: unknown, depth = 0): 'strict' | 'strip' | null {
Expand Down Expand Up @@ -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);
});
});
Loading
Loading