diff --git a/.changeset/action-no-placement-lint.md b/.changeset/action-no-placement-lint.md new file mode 100644 index 0000000000..bac8df01c6 --- /dev/null +++ b/.changeset/action-no-placement-lint.md @@ -0,0 +1,45 @@ +--- +"@objectstack/lint": minor +"@objectstack/cli": minor +"@objectstack/metadata-protocol": minor +--- + +Lint an action nobody placed (ADR-0078 Phase 3, Tier-A `action-locations`). + +New advisory rule `action-no-placement`: an action that declares no +`locations` and that no list view places by name renders on **no** surface — +it parses, publishes, and appears in Setup, while no user can ever click it. +ADR-0078 names this shape in its opening paragraph and Phase 3 asks for +exactly this rule; the shared completeness predicate it envisioned was never +built, so this lands standalone, one verified shape at a time. + +What made it verifiable now: objectui#3142 collapsed four disagreeing +renderers onto one placement predicate. Before that, `action:bar` and the +record header rendered an *undeclared* action anyway, so the shape only looked +inert on paper. As of objectui 17.1 it is measurably inert. + +Two things are deliberately **not** flagged: + +- **`locations: []`** — the documented headless action (callable over REST / + MCP / AI, no UI surface). ADR-0110 D3 refuses an undeclared handler, so a + headless declaration is the only legal way to expose one. The rule therefore + distinguishes "nowhere, deliberately" (`[]`) from an unstated placement (key + absent) and only reports the latter. +- **Actions a view places by name** — `bulkActions`, `bulkActionDefs` + (including `execution: 'aggregate'` defs, whose whole point is an action with + no single-record home) and `rowActions`, across all three list-view tiers: + `views[i].list`, `views[i].listViews.` and the object-embedded + `objects[i].listViews.`. + +Advisory, never fatal — a view in another installed package may be the one +placing the action, the same reason `validateSemanticRoles` and +`lintLivenessProperties` warn rather than gate. + +Also: the action form schema in `@objectstack/metadata-protocol` no longer +declares `shortcut` / `bulkEnabled`. Both were retired as `retiredKey()` +tombstones in spec 17, and this schema is what the Studio designer renders its +fallback form from — so advertising them handed authors two inputs that could +only ever produce an unsaveable draft (objectui#3145 removed the matching +dedicated controls). And `content/docs/ui/actions.mdx` now says which surface +is the exception to location filtering, instead of a blanket claim its own +showcase contradicted. diff --git a/content/docs/ui/actions.mdx b/content/docs/ui/actions.mdx index d81ca55ac8..9060705ef2 100644 --- a/content/docs/ui/actions.mdx +++ b/content/docs/ui/actions.mdx @@ -210,9 +210,17 @@ defineView({ ``` -Naming an action in a widget does **not** bypass location filtering — the +Naming an action in a **widget** does not bypass location filtering — the engine still requires the action to declare the matching location (that's why `MarkDoneAction` above includes `record_section`). + +The **selection bar is the exception**, and the only one: an action named in a +list view's `bulkActions` or `bulkActionDefs` is placed by that declaration, +not by `locations`. That is what the retired `action.bulkEnabled` tombstone +prescribes ("the multi-select toolbar is driven by the LIST VIEW's +`bulkActions` / `bulkActionDefs`"), and it is what lets an aggregate bulk +action — one that acts on a whole selection and has no single-record home by +construction — exist at all. ## Collect input and shape the UX diff --git a/packages/cli/src/lint/authoring-rules.ts b/packages/cli/src/lint/authoring-rules.ts index 38df529a05..2b0d594b9a 100644 --- a/packages/cli/src/lint/authoring-rules.ts +++ b/packages/cli/src/lint/authoring-rules.ts @@ -94,6 +94,7 @@ import { validateVisibilityPredicates, validateSecurityPosture, validateOrgAxisRedLines, + validateActionLocations, } from '@objectstack/lint'; import { lintFlowPatterns } from '../utils/lint-flow-patterns.js'; import { lintLivenessProperties } from '../utils/lint-liveness-properties.js'; @@ -385,6 +386,20 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ source: 'packages/lint/src/validate-semantic-roles.ts', run: (stack) => validateSemanticRoles(stack), }, + // ADR-0078 Phase 3 (Tier-A `action-locations`) — an action that declares no + // `locations` and that no view places by name renders on no surface at all. + // objectui#3142 made that measurable: four renderers used to show an + // undeclared action anyway, and now none does. Advisory: a view in another + // installed package may be the one placing it, and `locations: []` (the + // documented headless shape) is deliberately never flagged. + { + name: 'validateActionLocations', + tier: 'advisory', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-action-locations.ts', + run: (stack) => validateActionLocations(stack), + }, // framework#3434 — seeds replay on every boot, so a `mode: 'insert'` dataset // duplicates its table on every restart. { diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index d0b64fc48c..d3b666748e 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -207,6 +207,9 @@ export type { export { validateActionNameRefs, ACTION_NAME_UNDEFINED } from './validate-action-name-refs.js'; export type { ActionNameRefFinding, ActionNameRefSeverity } from './validate-action-name-refs.js'; +export { validateActionLocations, ACTION_NO_PLACEMENT } from './validate-action-locations.js'; +export type { ActionLocationsFinding, ActionLocationsSeverity } from './validate-action-locations.js'; + export { validatePageFieldBindings, PAGE_FIELD_UNKNOWN } from './validate-page-field-bindings.js'; export type { PageFieldFinding, PageFieldSeverity } from './validate-page-field-bindings.js'; diff --git a/packages/lint/src/validate-action-locations.test.ts b/packages/lint/src/validate-action-locations.test.ts new file mode 100644 index 0000000000..e140082b8e --- /dev/null +++ b/packages/lint/src/validate-action-locations.test.ts @@ -0,0 +1,165 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { validateActionLocations, ACTION_NO_PLACEMENT } from './validate-action-locations.js'; + +/** A stack whose single action declares a real placement. */ +const placed = () => ({ + objects: [{ name: 'crm_lead', fields: { name: { type: 'text' } } }], + actions: [ + { + name: 'crm_convert_lead', + label: 'Convert', + type: 'script', + locations: ['record_header'], + }, + ], +}); + +/** The same action with the placement key absent. */ +const unplaced = () => ({ + objects: [{ name: 'crm_lead', fields: { name: { type: 'text' } } }], + actions: [{ name: 'crm_convert_lead', label: 'Convert', type: 'script' }], +}); + +describe('validateActionLocations', () => { + it('flags an action that declares no locations and that no view places', () => { + const findings = validateActionLocations(unplaced()); + + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('warning'); + expect(findings[0].rule).toBe(ACTION_NO_PLACEMENT); + expect(findings[0].path).toBe('actions[0]'); + expect(findings[0].where).toBe('action "crm_convert_lead"'); + expect(findings[0].message).toContain('renders on no surface'); + expect(findings[0].hint).toContain('locations: []'); + }); + + it('accepts a declared placement', () => { + expect(validateActionLocations(placed())).toEqual([]); + }); + + it('walks object-embedded actions too', () => { + const findings = validateActionLocations({ + objects: [ + { + name: 'crm_lead', + actions: [{ name: 'crm_score', label: 'Score', type: 'script' }], + }, + ], + }); + + expect(findings).toHaveLength(1); + expect(findings[0].path).toBe('objects[0].actions[0]'); + }); + + it('ignores a nameless action — that is action-name-*’s problem, not this rule’s', () => { + expect(validateActionLocations({ actions: [{ label: 'Nameless', type: 'script' }] })).toEqual([]); + }); + + describe('— headless actions (`locations: []`) are never flagged', () => { + it('accepts an explicitly empty placement', () => { + // `content/docs/ui/actions.mdx` documents the empty array as the way to + // declare a REST/MCP/AI-callable action with no UI surface. ADR-0110 D3 + // refuses an UNdeclared handler, so this is the only legal shape for one + // — flagging it would fight that ADR. + const findings = validateActionLocations({ + actions: [{ name: 'crm_sync_remote', label: 'Sync', type: 'script', locations: [] }], + }); + expect(findings).toEqual([]); + }); + + it('distinguishes "nowhere, deliberately" from an unstated placement', () => { + const findings = validateActionLocations({ + actions: [ + { name: 'said_nowhere', type: 'script', locations: [] }, + { name: 'said_nothing', type: 'script' }, + ], + }); + expect(findings.map((f) => f.where)).toEqual(['action "said_nothing"']); + }); + }); + + describe('— a view that places the action by NAME exempts it', () => { + it('exempts an action named in a list view’s bulkActions', () => { + const findings = validateActionLocations({ + ...unplaced(), + views: [{ name: 'crm_lead', list: { bulkActions: ['crm_convert_lead'] } }], + }); + expect(findings).toEqual([]); + }); + + it('exempts an action named in a bulkActionDefs entry (incl. aggregate defs)', () => { + // objectui#3139: an aggregate bulk action has no single-record location + // by construction — the view naming it IS the placement. + const findings = validateActionLocations({ + ...unplaced(), + views: [ + { + name: 'crm_lead', + list: { + bulkActionDefs: [ + { name: 'crm_convert_lead', operation: 'custom', execution: 'aggregate' }, + ], + }, + }, + ], + }); + expect(findings).toEqual([]); + }); + + it('exempts an action named in rowActions', () => { + const findings = validateActionLocations({ + ...unplaced(), + views: [{ name: 'crm_lead', list: { rowActions: ['crm_convert_lead'] } }], + }); + expect(findings).toEqual([]); + }); + + it('exempts via a named listViews entry, not just the default list', () => { + const findings = validateActionLocations({ + ...unplaced(), + views: [{ name: 'crm_lead', listViews: { hot: { bulkActions: ['crm_convert_lead'] } } }], + }); + expect(findings).toEqual([]); + }); + + it('exempts via an OBJECT-embedded list view — an object has no top-level `list`', () => { + const findings = validateActionLocations({ + objects: [ + { + name: 'crm_lead', + listViews: { all: { bulkActions: ['crm_convert_lead'] } }, + }, + ], + actions: [{ name: 'crm_convert_lead', label: 'Convert', type: 'script' }], + }); + expect(findings).toEqual([]); + }); + + it('still flags an action no view names, alongside one that is named', () => { + const findings = validateActionLocations({ + actions: [ + { name: 'named_one', type: 'script' }, + { name: 'orphan_one', type: 'script' }, + ], + views: [{ name: 'crm_lead', list: { bulkActions: ['named_one'] } }], + }); + expect(findings.map((f) => f.where)).toEqual(['action "orphan_one"']); + }); + }); + + describe('— floor', () => { + it('returns nothing for a clean stack', () => { + expect(validateActionLocations(placed())).toEqual([]); + }); + + it('returns nothing for an empty stack', () => { + expect(validateActionLocations({})).toEqual([]); + }); + + it('returns nothing for a null stack', () => { + expect(validateActionLocations(null as unknown as Record)).toEqual([]); + }); + }); +}); diff --git a/packages/lint/src/validate-action-locations.ts b/packages/lint/src/validate-action-locations.ts new file mode 100644 index 0000000000..2e6ed2974a --- /dev/null +++ b/packages/lint/src/validate-action-locations.ts @@ -0,0 +1,182 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0078 Phase 3 — Tier-A `action-locations`] An action nobody placed. + * + * `locations` is an action's placement declaration. An action that omits it — + * and that no view names in `bulkActions` / `bulkActionDefs` / `rowActions` — + * has no surface at all: it parses, it publishes, Setup lists it, and no user + * can ever click it. ADR-0078 names this shape in its opening paragraph ("a + * `summary` with no `summaryOperations`; **an `action` with no `locations`**; + * … Each parses, 'renders', reports success — and does nothing") and Phase 3 + * asks for exactly this rule, one verified shape at a time. + * + * The renderer half is now unambiguous: objectui#3142 collapsed four + * disagreeing renderers onto one predicate — an action renders at a location + * only if it DECLARES that location. Before that, `action:bar` and the record + * header showed an undeclared action *everywhere*, which is what made this + * shape look alive; it is measurably inert as of objectui 17.1. + * + * ## What is NOT flagged, and why + * + * **`locations: []` — a deliberate headless action.** `content/docs/ui/ + * actions.mdx` ("Headless actions: declare it, then hide it") documents the + * empty array as a first-class shape: the action stays callable over REST / + * MCP / AI and keeps its capability gate, param contract and audit trail, + * while claiming no UI surface. ADR-0110 D3 refuses an *undeclared* handler, + * so a headless declaration is the only legal way to expose such an action — + * flagging it would fight that ADR. The distinction this rule draws is + * therefore between an author who said "nowhere, deliberately" (`[]`) and one + * who never said anything at all (key absent). + * + * **Actions a view places by NAME.** Naming an action in a list view's + * `bulkActions` or `bulkActionDefs` IS its placement — the selection bar is + * driven by the view, never by `locations` (that is what the retired + * `action.bulkEnabled` tombstone prescribes, and what objectui#3139's + * aggregate bulk mode relies on: an action that only makes sense over a + * selection has no single-record location by construction). `rowActions` is + * exempted on the same zero-false-positive posture (ADR-0072 D1): it is the + * same field pair on the same container, and an author who named an action + * there has stated an intent — a name that resolves to nothing is already + * `action-name-undefined`'s job, not this rule's. + * + * Scope note: this rule asks only "did anyone place this action?". It + * deliberately does NOT check that a declared location is one a renderer + * actually serves, nor that a view's named action belongs to that view's + * object — distinct classes with their own rules. Cross-package placement (a + * view in another installed package naming this action) is the one legitimate + * miss, which is why this is a **warning**: like every other "declared but + * does nothing" finding in this package (`validateSemanticRoles`, + * `lintLivenessProperties`), it is high-signal and never fatal. + */ + +export const ACTION_NO_PLACEMENT = 'action-no-placement'; + +export type ActionLocationsSeverity = 'error' | 'warning'; + +export interface ActionLocationsFinding { + /** Always `warning` — cross-package placement is a legitimate miss. */ + severity: ActionLocationsSeverity; + /** Diagnostic rule id. */ + rule: string; + /** Human-readable location, e.g. `action "crm_convert_lead"`. */ + where: string; + /** Config path, e.g. `actions[2]` or `objects[0].actions[1]`. */ + path: string; + /** What is wrong. */ + message: string; + /** How to fix it. */ + hint: string; +} + +type AnyRec = Record; + +function asArray(v: unknown): AnyRec[] { + if (Array.isArray(v)) return v as AnyRec[]; + if (v && typeof v === 'object') { + return Object.entries(v as AnyRec).map(([name, def]) => ({ name, ...(def as AnyRec) })); + } + return []; +} + +function strName(v: unknown): string | undefined { + return typeof v === 'string' && v.length > 0 ? v : undefined; +} + +function strList(v: unknown): string[] { + return Array.isArray(v) ? v.filter((x): x is string => typeof x === 'string' && x.length > 0) : []; +} + +/** + * Every action name a view places by NAME, across all three list-view tiers: + * `views[i].list`, `views[i].listViews.`, and the object-embedded + * `objects[i].listViews.` (an object has no top-level `list`). Missing + * the object-embedded tier would flag actions that an object's own view + * places — the trap `validate-list-view-mode.ts` already walks around. + */ +function collectNamePlacedActions(stack: AnyRec): Set { + const placed = new Set(); + + const harvest = (container: unknown): void => { + if (!container || typeof container !== 'object') return; + const list = container as AnyRec; + for (const key of ['rowActions', 'bulkActions'] as const) { + for (const n of strList(list[key])) placed.add(n); + } + // A `bulkActionDefs` entry is a loose record; its `name` is the action it + // dispatches. Inline field-patch defs (`operation: 'update'`) carry a name + // that matches no action — harmless here, since an unmatched name simply + // never exempts anything. + for (const def of asArray(list.bulkActionDefs)) { + const n = strName(def?.name); + if (n) placed.add(n); + } + }; + + const harvestListViews = (listViews: unknown): void => { + if (!listViews || typeof listViews !== 'object' || Array.isArray(listViews)) return; + for (const lv of Object.values(listViews as AnyRec)) harvest(lv); + }; + + for (const view of asArray(stack.views)) { + if (!view || typeof view !== 'object') continue; + harvest(view.list); + harvestListViews(view.listViews); + } + for (const obj of asArray(stack.objects)) { + if (!obj || typeof obj !== 'object') continue; + harvestListViews(obj.listViews); + } + + return placed; +} + +/** + * Flag every action that declares no placement and that no view places by + * name. Returns findings (empty = clean). + */ +export function validateActionLocations(stack: AnyRec): ActionLocationsFinding[] { + const findings: ActionLocationsFinding[] = []; + if (!stack || typeof stack !== 'object') return findings; + + const namePlaced = collectNamePlacedActions(stack); + + const check = (action: AnyRec | undefined, path: string): void => { + if (!action || typeof action !== 'object') return; + // `[]` is the documented headless shape — the author said "nowhere" on + // purpose. Only a MISSING key is unstated placement. + if ('locations' in action) return; + const name = strName(action.name); + if (!name) return; // nameless actions are `action-name-*`'s problem + if (namePlaced.has(name)) return; + + findings.push({ + severity: 'warning', + rule: ACTION_NO_PLACEMENT, + where: `action "${name}"`, + path, + message: + `Action "${name}" declares no \`locations\` and no view places it by name, ` + + 'so it renders on no surface — the button exists in metadata and nowhere in the UI.', + hint: + 'Add the surface it belongs on, e.g. `locations: [\'record_header\']` (or `list_item`, ' + + '`list_toolbar`, `record_more`, `record_section`, `record_related`, `global_nav`); or ' + + "place it from a list view's `bulkActions` / `bulkActionDefs` if it acts on a selection. " + + 'If it is meant to be callable over REST / MCP / AI with no UI surface, say so explicitly ' + + 'with `locations: []` — an empty array is the documented headless shape and is never flagged.', + }); + }; + + const actions = asArray(stack.actions); + for (let i = 0; i < actions.length; i++) check(actions[i], `actions[${i}]`); + + const objects = asArray(stack.objects); + for (let oi = 0; oi < objects.length; oi++) { + const obj = objects[oi]; + if (!obj || typeof obj !== 'object') continue; + const own = asArray(obj.actions); + for (let ai = 0; ai < own.length; ai++) check(own[ai], `objects[${oi}].actions[${ai}]`); + } + + return findings; +} diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 8ec5dede16..f9a28d49f4 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -207,8 +207,14 @@ const HAND_CRAFTED_SCHEMAS: Record> = { component: { type: 'string' }, visible: { type: 'string' }, disabled: { type: 'string' }, - shortcut: { type: 'string' }, - bulkEnabled: { type: 'boolean', default: false }, + // No `shortcut` / `bulkEnabled`: spec 17 retired both as + // `retiredKey()` tombstones, so authoring either is a hard parse + // rejection. This schema is what the Studio designer renders its + // fallback form from, so leaving them here handed authors two + // inputs that could only ever produce an unsaveable draft + // (objectui#3145 removed the matching dedicated controls). + // `bulkEnabled`'s replacement is the list view's `bulkActions` / + // `bulkActionDefs`; `shortcut` has none. aiExposed: { type: 'boolean', default: false }, recordIdParam: { type: 'string' }, recordIdField: { type: 'string' },