diff --git a/.changeset/17319-action-bulk-dispatch-contract.md b/.changeset/17319-action-bulk-dispatch-contract.md new file mode 100644 index 0000000000..a2a9ddf6df --- /dev/null +++ b/.changeset/17319-action-bulk-dispatch-contract.md @@ -0,0 +1,14 @@ +--- +"@objectstack/spec": minor +"@objectstack/lint": minor +--- + +An action can now **declare which bulk dispatch contract its body is written for**, and a list view that wires it the other way is refused at authoring time instead of handing the body the opposite input in silence. + +A list view has always been able to wire the same declared action two ways, and the two deliver opposite shapes to the same body: `bulkActions: ['']` promotes the action to a def and dispatches it **once per selected row** (that row's `recordId`, no `_selectedIds`), while a `bulkActionDefs` entry with `execution: 'aggregate'` makes **one** dispatch for the whole selection (every id in `params._selectedIds`, no `recordId`). The action declared neither, so both mismatches failed quietly and in opposite directions — an aggregate body wired bare-string read `_selectedIds` as `undefined`, fell into its single-record branch and reported success for one row out of ten; a per-record body wired aggregate found no `recordId` and threw its own "nothing selected", which reads like a selection bug. Nothing caught either: `recordId` and `_selectedIds` are both built-in action params (ADR-0104), so the strict params gate admits either bag without a word, and the wiring lives on the view while the declaration would live on the action, so no single parse has both halves. + +- **`ActionSchema` gains `execution`**, and it is `bulkActionDefs`' own vocabulary — the same key, the same two values (`'perRecord' | 'aggregate'`), the def's `BulkActionExecutionSchema` **imported rather than re-declared**, so there is no second spelling to drift. The near-miss keys (`dispatch`, `dispatchContract`, `bulkExecution`, `bulkDispatch`) rename onto it; ⛔ `mode` deliberately does **not**, because on an action `mode` is a declared key of its own. +- **`@objectstack/lint` gains `action-dispatch-contract-mismatch`** (severity `error`), a member of the reference-integrity suite, so it runs on `os validate`, `os lint` and `os compile` at once. It names the action, the view and **both** contracts — the declared one and the wired one — and offers both ends of the fix, because which end is wrong is the author's call. It judges every list tier: a view's `list`, each `listViews.`, and an object's own `listViews`. +- **⛔ No silent default.** `execution` is optional and an action that omits it is *undeclared*, never defaulted to a contract — which is also the honest state of a body written to serve both (it reads `recordId` *and* `_selectedIds`), and why no third enum member was added. Existing sources are migrated by the new ADR-0087 semantic entry `action-bulk-dispatch-contract-undeclared`, which derives the declaration from the view wirings where they are unambiguous and hands back a structured TODO where one action is wired both ways. + +Nothing about dispatch changes: this release adds a declaration and a build-time refusal measured against it. Existing apps are unaffected until they declare the key — the new rule has nothing to judge on an undeclared action, by construction. diff --git a/content/docs/references/data/object.mdx b/content/docs/references/data/object.mdx index f76e1a4a7f..4572821e29 100644 --- a/content/docs/references/data/object.mdx +++ b/content/docs/references/data/object.mdx @@ -452,6 +452,7 @@ const result = ApiMethod.parse(data); | **body** | `{ language: 'expression'; source: string } \| { language: 'js'; source: string; capabilities?: Enum<'api.read' \| 'api.write' \| 'api.transaction' \| 'crypto.uuid' \| 'log'>[]; timeoutMs?: integer; … }` | optional | Action body — expression (L1) or sandboxed JS (L2). Only used when type is `script`. | | **operation** | `Enum<'update'>` | optional | The declarative single-record field write, mirroring a list view's `bulkActionDefs`: `'update'` applies `patch` (merged under the collected `params`) to the current record on the data plane AS THE CALLER — never system-elevated — so the caller's permissions, the object's hooks and its validations fire as for a user edit. `type` stays at its default `'script'` (the platform action route the write is performed on); `target`/`body`/`method`/`bodyExtra` are refused beside it. `'delete'` and `'custom'` have no row-level form. | | **patch** | `Record` | optional | For `operation: 'update'` — static field values written to the current record, merged UNDER the user-supplied `params` so a fixed value can be declared without exposing it in the dialog. Written on the data plane as the caller: object permissions, hooks and validations fire as for a user edit. Refused on an action without `operation: 'update'` (it would be silently dropped). | +| **execution** | `Enum<'perRecord' \| 'aggregate'>` | optional | The bulk dispatch contract this action's BODY is written for, in `bulkActionDefs`' own vocabulary: 'perRecord' = one dispatch per selected row carrying that row's `recordId` (the view's `bulkActions: ['']` bare-string form); 'aggregate' = ONE dispatch for the whole selection carrying every id in `params._selectedIds` (a `bulkActionDefs` entry with `execution: 'aggregate'`). Optional with NO default — omit it only when the body genuinely serves both. A list view wiring a declared action under the other contract is refused by `@objectstack/lint` (`action-dispatch-contract-mismatch`). | | **execute** | `never` | optional | [REMOVED] `execute` was removed in @objectstack/spec 17 — use `target`. Rename the key; the value (a handler / flow / URL ref) is unchanged. Run `os migrate meta --from 16` to list the mechanical edits for existing sources; apply them by hand. | | **params** | `{ name?: string; field?: string; objectOverride?: string; label?: string \| Record; … }[]` | optional | Input parameters required from user — an ActionParam[] DEFINITION array, never a payload map (a static request body goes in `bodyExtra`). | | **variant** | `Enum<'primary' \| 'secondary' \| 'danger' \| 'ghost' \| 'link'>` | optional | Button visual variant for styling (primary = highlighted, danger = destructive, ghost = transparent) | diff --git a/content/docs/references/kernel/metadata-plugin.mdx b/content/docs/references/kernel/metadata-plugin.mdx index be3b619479..771c1d8cd0 100644 --- a/content/docs/references/kernel/metadata-plugin.mdx +++ b/content/docs/references/kernel/metadata-plugin.mdx @@ -321,6 +321,7 @@ const result = MetadataBulkResultSchema.parse(data); | **body** | `{ language: 'expression'; source: string } \| { language: 'js'; source: string; capabilities?: Enum<'api.read' \| 'api.write' \| 'api.transaction' \| 'crypto.uuid' \| 'log'>[]; timeoutMs?: integer; … }` | optional | Action body — expression (L1) or sandboxed JS (L2). Only used when type is `script`. | | **operation** | `Enum<'update'>` | optional | The declarative single-record field write, mirroring a list view's `bulkActionDefs`: `'update'` applies `patch` (merged under the collected `params`) to the current record on the data plane AS THE CALLER — never system-elevated — so the caller's permissions, the object's hooks and its validations fire as for a user edit. `type` stays at its default `'script'` (the platform action route the write is performed on); `target`/`body`/`method`/`bodyExtra` are refused beside it. `'delete'` and `'custom'` have no row-level form. | | **patch** | `Record` | optional | For `operation: 'update'` — static field values written to the current record, merged UNDER the user-supplied `params` so a fixed value can be declared without exposing it in the dialog. Written on the data plane as the caller: object permissions, hooks and validations fire as for a user edit. Refused on an action without `operation: 'update'` (it would be silently dropped). | +| **execution** | `Enum<'perRecord' \| 'aggregate'>` | optional | The bulk dispatch contract this action's BODY is written for, in `bulkActionDefs`' own vocabulary: 'perRecord' = one dispatch per selected row carrying that row's `recordId` (the view's `bulkActions: ['']` bare-string form); 'aggregate' = ONE dispatch for the whole selection carrying every id in `params._selectedIds` (a `bulkActionDefs` entry with `execution: 'aggregate'`). Optional with NO default — omit it only when the body genuinely serves both. A list view wiring a declared action under the other contract is refused by `@objectstack/lint` (`action-dispatch-contract-mismatch`). | | **execute** | `never` | optional | [REMOVED] `execute` was removed in @objectstack/spec 17 — use `target`. Rename the key; the value (a handler / flow / URL ref) is unchanged. Run `os migrate meta --from 16` to list the mechanical edits for existing sources; apply them by hand. | | **params** | `{ name?: string; field?: string; objectOverride?: string; label?: string \| Record; … }[]` | optional | Input parameters required from user — an ActionParam[] DEFINITION array, never a payload map (a static request body goes in `bodyExtra`). | | **variant** | `Enum<'primary' \| 'secondary' \| 'danger' \| 'ghost' \| 'link'>` | optional | Button visual variant for styling (primary = highlighted, danger = destructive, ghost = transparent) | diff --git a/content/docs/references/ui/action.mdx b/content/docs/references/ui/action.mdx index 694608886e..ebeee31f50 100644 --- a/content/docs/references/ui/action.mdx +++ b/content/docs/references/ui/action.mdx @@ -40,6 +40,7 @@ const result = ActionSchema.parse(data); | **body** | `{ language: 'expression'; source: string } \| { language: 'js'; source: string; capabilities?: Enum<'api.read' \| 'api.write' \| 'api.transaction' \| 'crypto.uuid' \| 'log'>[]; timeoutMs?: integer; … }` | optional | Action body — expression (L1) or sandboxed JS (L2). Only used when type is `script`. | | **operation** | `Enum<'update'>` | optional | The declarative single-record field write, mirroring a list view's `bulkActionDefs`: `'update'` applies `patch` (merged under the collected `params`) to the current record on the data plane AS THE CALLER — never system-elevated — so the caller's permissions, the object's hooks and its validations fire as for a user edit. `type` stays at its default `'script'` (the platform action route the write is performed on); `target`/`body`/`method`/`bodyExtra` are refused beside it. `'delete'` and `'custom'` have no row-level form. | | **patch** | `Record` | optional | For `operation: 'update'` — static field values written to the current record, merged UNDER the user-supplied `params` so a fixed value can be declared without exposing it in the dialog. Written on the data plane as the caller: object permissions, hooks and validations fire as for a user edit. Refused on an action without `operation: 'update'` (it would be silently dropped). | +| **execution** | `Enum<'perRecord' \| 'aggregate'>` | optional | The bulk dispatch contract this action's BODY is written for, in `bulkActionDefs`' own vocabulary: 'perRecord' = one dispatch per selected row carrying that row's `recordId` (the view's `bulkActions: ['']` bare-string form); 'aggregate' = ONE dispatch for the whole selection carrying every id in `params._selectedIds` (a `bulkActionDefs` entry with `execution: 'aggregate'`). Optional with NO default — omit it only when the body genuinely serves both. A list view wiring a declared action under the other contract is refused by `@objectstack/lint` (`action-dispatch-contract-mismatch`). | | **execute** | `never` | optional | [REMOVED] `execute` was removed in @objectstack/spec 17 — use `target`. Rename the key; the value (a handler / flow / URL ref) is unchanged. Run `os migrate meta --from 16` to list the mechanical edits for existing sources; apply them by hand. | | **params** | `{ name?: string; field?: string; objectOverride?: string; label?: string \| Record; … }[]` | optional | Input parameters required from user — an ActionParam[] DEFINITION array, never a payload map (a static request body goes in `bodyExtra`). | | **variant** | `Enum<'primary' \| 'secondary' \| 'danger' \| 'ghost' \| 'link'>` | optional | Button visual variant for styling (primary = highlighted, danger = destructive, ghost = transparent) | diff --git a/content/docs/ui/actions.mdx b/content/docs/ui/actions.mdx index 4bac025a2d..3f2021af15 100644 --- a/content/docs/ui/actions.mdx +++ b/content/docs/ui/actions.mdx @@ -289,6 +289,49 @@ action — one that acts on a whole selection and has no single-record home by construction — exist at all. +## Declare the bulk dispatch contract + +A list view can wire the same action two ways, and the two hand your body +**opposite** input: + +| wiring | dispatches | the body receives | +| --- | --- | --- | +| `bulkActions: ['']` | once **per selected row** | that row's `recordId`, no `_selectedIds` | +| `bulkActionDefs: [{ name: '', operation: 'custom', execution: 'aggregate' }]` | **once** for the whole selection | `params._selectedIds: string[]`, no `recordId` | + +Say which one you wrote the body for, with the same key and the same two values +the def uses: + +```typescript +defineAction({ + name: 'export_zip', + type: 'api', + target: '/api/v1/export/zip', + execution: 'aggregate', // this body reads params._selectedIds +}); +``` + +`os validate` / `os lint` / `os build` then refuse a list view that wires it the +other way (`action-dispatch-contract-mismatch`), naming the action, the view and +both contracts. Without the declaration both mismatches fail **silently, in +opposite directions**: an aggregate body wired bare-string reads `_selectedIds` +as `undefined`, takes its single-record branch and reports success for one row +out of ten; a per-record body wired aggregate finds no `recordId` and throws its +own "nothing selected", which reads like a selection bug. Nothing else can catch +it — `recordId` and `_selectedIds` are both built-in action params, so the +strict params gate admits either bag without a word. + + + There is **no default**. An action that omits `execution` is checked against + neither wiring — which is also the honest declaration for a body written to + serve both contracts (it reads `recordId` *and* `_selectedIds`). There is no + third value for "both": one call and N calls have different side effects, so + if the two wirings want different behaviour they are two actions. + Upgrading an existing app? `os migrate meta` emits the + `action-bulk-dispatch-contract-undeclared` TODO, which derives the + declaration from the wirings you already have. + + ## Collect input and shape the UX - **`params`** — prompt the user for input before execution. Prefer diff --git a/content/docs/ui/views.mdx b/content/docs/ui/views.mdx index c5d4635cbc..aea74eb38c 100644 --- a/content/docs/ui/views.mdx +++ b/content/docs/ui/views.mdx @@ -244,6 +244,28 @@ Aggregate is the def form, which is where `execution` lives. A def that says time: the renderer has no action attached to such a def, so it used to render a button that reported success for every selected record and did nothing. +**The action gets a say, and the two ends are checked against each other.** An +action declares the contract its body was written for with the same key and the +same two values the def uses — `execution: 'perRecord' | 'aggregate'` (see +[Actions](/docs/ui/actions#declare-the-bulk-dispatch-contract)). Wiring a +declared action the other way is refused by `os validate` / `os lint` / +`os build` with `action-dispatch-contract-mismatch`, naming the action, the +view and both contracts: + +```typescript +// actions: { name: 'export_zip', type: 'api', execution: 'aggregate', … } + +bulkActions: ['export_zip'], // ⛔ refused — the bare string is the per-record + // contract; this body reads `_selectedIds` +bulkActionDefs: [ + { name: 'export_zip', operation: 'custom', execution: 'aggregate' }, // ✅ +] +``` + +There is **no default**: an action that omits `execution` is checked against +neither wiring, which is also the right declaration for a body deliberately +written to serve both. + **Gating a def by capability.** An inline def takes `requiredPermissions: string[]` with `action.requiredPermissions` semantics — absent/empty always passes, several entries AND, unknown caller capabilities diff --git a/examples/app-showcase/src/ui/actions/index.ts b/examples/app-showcase/src/ui/actions/index.ts index 4141ade19f..84f3731ab8 100644 --- a/examples/app-showcase/src/ui/actions/index.ts +++ b/examples/app-showcase/src/ui/actions/index.ts @@ -52,6 +52,12 @@ export const MarkDoneAction = defineAction({ "return { ok: true, id: id };", capabilities: ['api.write'], }, + // #17319 — the dispatch contract this BODY is written for. It reads + // `ctx.recordId` and throws 'No record to mark done' without one, so an + // aggregate wiring would fail on every click with a message that reads like + // a selection bug. Declaring it makes that wiring a build-time refusal + // (`action-dispatch-contract-mismatch`) instead. + execution: 'perRecord', successMessage: 'Task marked done.', // Hide once the task is complete. Gate on `record.done` (the boolean this // action sets) so the button vanishes after a successful click and stays @@ -127,6 +133,11 @@ export const RecalcEstimateAction = defineAction({ objectName: task, type: 'api', target: '/api/v1/showcase/recalc', + // #17319 — one POST per record; the endpoint's per-record branch reads the + // single id. Its aggregate twin below is a SEPARATE action against the same + // endpoint, which is what the platform used to require: one body, one + // contract, and until this key no way to say which. + execution: 'perRecord', successMessage: 'Estimate recalculated.', locations: ['record_more', 'record_section'], // The endpoint is record-scoped and rejects a body without an id. On a @@ -139,9 +150,11 @@ export const RecalcEstimateAction = defineAction({ /** * api, AGGREGATE-dispatched — the `execution: 'aggregate'` specimen - * (objectui#3139). The action itself is an ordinary api action; what makes it - * aggregate is the VIEW's `bulkActionDefs` entry naming it with - * `execution: 'aggregate'` (see `task.view.ts` → `bulk_actions`). The + * (objectui#3139). Since #17319 the ACTION declares the contract its body is + * written for (`execution: 'aggregate'`, below) and the VIEW performs the + * dispatch through a `bulkActionDefs` entry naming it with the same key and + * value (see `task.view.ts` → `bulk_actions`); a bare-string wiring of it is + * now refused at authoring time instead of quietly recalculating one row. The * renderer then dispatches it ONCE for the whole selection, with every * selected id in `params._selectedIds` — the recalc endpoint's batch branch * recomputes all of them in that single call (the "one zip for N devices" @@ -175,6 +188,12 @@ export const RecalcSelectionAction = defineAction({ objectName: task, type: 'api', target: '/api/v1/showcase/recalc', + // #17319 — ONE dispatch for the whole selection, every id in + // `params._selectedIds`. The view's `bulkActionDefs` entry still performs + // the dispatch; this declares what the body was written to receive, so a + // bare-string `bulkActions` wiring of it is refused rather than silently + // recalculating one row out of ten. + execution: 'aggregate', successMessage: 'Estimates recalculated for the whole selection.', locations: ['record_more'], recordIdParam: 'recordId', diff --git a/examples/app-showcase/src/ui/views/field-zoo.view.ts b/examples/app-showcase/src/ui/views/field-zoo.view.ts index 5aeef7e84c..645955d179 100644 --- a/examples/app-showcase/src/ui/views/field-zoo.view.ts +++ b/examples/app-showcase/src/ui/views/field-zoo.view.ts @@ -152,6 +152,15 @@ export const FieldZooViews = defineView({ * specimens it acts on Full and reports Minimal as skipped, rather than * quietly including it. * + * #17319 — `showcase_zoo_visible_string` is wired BOTH ways in this file: as a + * bare string in the two `bulkActions` lists above (per-record) and here as an + * aggregate def. That is deliberate and it stays UNDECLARED: its body + * (`predicate-matrix.action.ts`) reads `ctx.recordId` AND `input._selectedIds` + * and copes with either, which is the one honest reason to omit `action.execution`. + * ⛔ Declaring either contract on it would make the OTHER wiring a lint error + * (`action-dispatch-contract-mismatch`) — there is no third enum member for + * "both", and no silent default for the omission. + * * `execution: 'aggregate'` is not decoration — a `custom` def without it is * a no-op the parser refuses outright ("the button runs, reports success * for every selected record, and does nothing"). Aggregate means ONE diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index db55d05cba..fec463dc00 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -501,6 +501,8 @@ export type { } from './validate-object-field-refs.js'; export { validateActionNameRefs, ACTION_NAME_UNDEFINED } from './validate-action-name-refs.js'; +export { validateActionDispatchContract, ACTION_DISPATCH_CONTRACT_MISMATCH } from './validate-action-dispatch-contract.js'; +export type { ActionDispatchContract, ActionDispatchContractFinding } from './validate-action-dispatch-contract.js'; export type { ActionNameRefFinding, ActionNameRefSeverity } from './validate-action-name-refs.js'; export { validateActionLocations, ACTION_NO_PLACEMENT } from './validate-action-locations.js'; diff --git a/packages/lint/src/reference-integrity-suite.test.ts b/packages/lint/src/reference-integrity-suite.test.ts index 8cd01fb3b4..988cfef505 100644 --- a/packages/lint/src/reference-integrity-suite.test.ts +++ b/packages/lint/src/reference-integrity-suite.test.ts @@ -32,6 +32,12 @@ describe('reference-integrity suite — membership', () => { // members with nothing to inspect on the click path. 'validateObjectFieldRefs', 'validateActionNameRefs', + // [#17319] The same action name, one question on: the name-ref member + // above asks whether the selection bar's name resolves to an action; this + // one asks whether the wiring it resolves through matches the dispatch + // contract that action declares. Placed beside it so a dead name and a + // live-but-mis-wired one report together. + 'validateActionDispatchContract', 'validatePageFieldBindings', // [#14073] The same page, one question out: the BINDING behind each // visualization `appearance.allowedVisualizations` whitelists, resolved diff --git a/packages/lint/src/reference-integrity-suite.ts b/packages/lint/src/reference-integrity-suite.ts index 458c433694..68a234cc68 100644 --- a/packages/lint/src/reference-integrity-suite.ts +++ b/packages/lint/src/reference-integrity-suite.ts @@ -105,6 +105,7 @@ import { validateSortableFields } from './validate-sortable-fields.js'; import { validateListViewFieldRefs } from './validate-list-view-field-refs.js'; import { validateObjectFieldRefs } from './validate-object-field-refs.js'; import { validateActionNameRefs } from './validate-action-name-refs.js'; +import { validateActionDispatchContract } from './validate-action-dispatch-contract.js'; import { validatePageFieldBindings } from './validate-page-field-bindings.js'; import { validatePageVisualizationBindings } from './validate-page-visualization-bindings.js'; import { validateChartBindings } from './validate-chart-bindings.js'; @@ -325,6 +326,21 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [ // an object's own field-name lists. { name: 'validateObjectFieldRefs', runtimeTypes: ['flow', 'object'], run: validateObjectFieldRefs }, { name: 'validateActionNameRefs', run: validateActionNameRefs }, + // [#17319] The same action name, one question on: `validateActionNameRefs` + // asks whether the name a list view's selection bar writes resolves to an + // action at all; this member asks whether the WIRING it resolves through + // matches the dispatch contract that action declares its body was written + // for. Placed directly after it so the two report together — a dead name + // first, then a live name delivered the wrong input shape. + // + // NO `runtimeTypes`, i.e. the frozen `flow` default, and for the same reason + // the member above it takes the default: it resolves against `stack.actions`, + // which no per-write snapshot carries. The failure it would take on a `view` + // crossing is the gentler one (this member returns early on an empty + // declaration map, so it would go silent rather than refuse), but a member + // that is structurally unable to judge the snapshot has no business being + // dispatched on it. + { name: 'validateActionDispatchContract', run: validateActionDispatchContract }, { name: 'validatePageFieldBindings', run: validatePageFieldBindings }, // [#14073] The same page, one question out. `validatePageFieldBindings` // above resolves the field NAMES an interface page writes; this member diff --git a/packages/lint/src/validate-action-dispatch-contract.test.ts b/packages/lint/src/validate-action-dispatch-contract.test.ts new file mode 100644 index 0000000000..82ac3c51e8 --- /dev/null +++ b/packages/lint/src/validate-action-dispatch-contract.test.ts @@ -0,0 +1,209 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #17319 — both directions of the refusal, and both directions of the +// ACCEPTANCE. A rule that only ever fires is indistinguishable from a rule +// that always fires, so every refusal pin below has a correctly-wired twin +// that must stay clean; those twins are the cost-direction half of this file +// and they are the ones that hold the blast radius down when someone +// "strengthens" the rule later. + +import { describe, it, expect } from 'vitest'; +import { + validateActionDispatchContract, + ACTION_DISPATCH_CONTRACT_MISMATCH, +} from './validate-action-dispatch-contract.js'; + +/** + * The showcase's own pair, reduced: two actions against ONE endpoint, written + * for the two contracts. They exist as two actions precisely because, until + * #17319, the platform had no way for one action to say which one it was. + */ +const stack = (actions: Record[], list: Record) => ({ + objects: [{ name: 'task', fields: { name: { type: 'text' } } }], + actions, + views: [{ name: 'task', object: 'task', list }], +}); + +const PER_RECORD = { name: 'recalc_estimate', label: 'Recalc', type: 'api', execution: 'perRecord' }; +const AGGREGATE = { name: 'recalc_selection', label: 'Recalc all', type: 'api', execution: 'aggregate' }; +const UNDECLARED = { name: 'mark_done', label: 'Mark done', type: 'script' }; + +describe('validateActionDispatchContract — the refusal (both directions)', () => { + it('refuses an `aggregate`-declared action wired as a bare string', () => { + const findings = validateActionDispatchContract( + stack([AGGREGATE], { bulkActions: ['recalc_selection'] }), + ); + expect(findings).toHaveLength(1); + expect(findings[0]!.severity).toBe('error'); + expect(findings[0]!.rule).toBe(ACTION_DISPATCH_CONTRACT_MISMATCH); + expect(findings[0]!.path).toBe('views[0].list.bulkActions[0]'); + }); + + it('refuses a `perRecord`-declared action wired through an aggregate def', () => { + const findings = validateActionDispatchContract( + stack([PER_RECORD], { + bulkActionDefs: [{ name: 'recalc_estimate', operation: 'custom', execution: 'aggregate' }], + }), + ); + expect(findings).toHaveLength(1); + expect(findings[0]!.severity).toBe('error'); + expect(findings[0]!.path).toBe('views[0].list.bulkActionDefs[0]'); + }); + + // The ruling's own words for item 2: the refusal names the action, the view + // and BOTH contracts. Asserted as four independent substrings rather than one + // golden string, so re-wording the prose does not silently drop a name. + it('names the action, the view and BOTH contracts, in both directions', () => { + for (const [findings, declared, wired] of [ + [validateActionDispatchContract(stack([AGGREGATE], { bulkActions: ['recalc_selection'] })), 'aggregate', 'perRecord'], + [ + validateActionDispatchContract( + stack([PER_RECORD], { + bulkActionDefs: [{ name: 'recalc_estimate', operation: 'custom', execution: 'aggregate' }], + }), + ), + 'perRecord', + 'aggregate', + ], + ] as const) { + const f = findings[0]!; + const action = declared === 'aggregate' ? 'recalc_selection' : 'recalc_estimate'; + expect(f.message).toContain(`"${action}"`); // the action + expect(f.where).toContain('view "task"'); // the view + expect(f.message).toContain(`execution: '${declared}'`); // the declared contract + expect(f.message).toContain(`execution: '${wired}'`); // the wired contract + // …and what each one actually delivers, so the reader does not have to + // already know which key belongs to which. + expect(f.message).toContain('_selectedIds'); + expect(f.message).toContain('recordId'); + } + }); + + it('offers both ends of the fix, never only one', () => { + const f = validateActionDispatchContract(stack([AGGREGATE], { bulkActions: ['recalc_selection'] }))[0]!; + expect(f.hint).toContain("execution: 'perRecord'"); // change the declaration + expect(f.hint).toContain('bulkActionDefs'); // or change the wiring + }); +}); + +describe('validateActionDispatchContract — the cost direction (must stay clean)', () => { + it('accepts a `perRecord`-declared action wired as a bare string', () => { + expect( + validateActionDispatchContract(stack([PER_RECORD], { bulkActions: ['recalc_estimate'] })), + ).toEqual([]); + }); + + it('accepts an `aggregate`-declared action wired through an aggregate def', () => { + expect( + validateActionDispatchContract( + stack([AGGREGATE], { + bulkActionDefs: [{ name: 'recalc_selection', operation: 'custom', execution: 'aggregate' }], + }), + ), + ).toEqual([]); + }); + + it('accepts the showcase shape: both actions, both wirings, in ONE list view', () => { + expect( + validateActionDispatchContract( + stack([PER_RECORD, AGGREGATE, UNDECLARED], { + bulkActions: ['mark_done', 'recalc_estimate'], + bulkActionDefs: [{ name: 'recalc_selection', operation: 'custom', execution: 'aggregate' }], + }), + ), + ).toEqual([]); + }); + + // ⛔ No silent default (the ruling's 「创业阶段不渐进」). An undeclared action + // is undeclared, not per-record-until-proven-otherwise — including when it is + // wired BOTH ways, which is the case the ADR-0087 semantic migration entry + // hands back as a structured TODO rather than deciding. + it('says nothing about an UNDECLARED action, even wired both ways', () => { + expect( + validateActionDispatchContract( + stack([UNDECLARED], { + bulkActions: ['mark_done'], + bulkActionDefs: [{ name: 'mark_done', operation: 'custom', execution: 'aggregate' }], + }), + ), + ).toEqual([]); + }); + + it('leaves a data-plane def alone even when its button id matches a declared action', () => { + // `operation: 'update'` dispatches no action at all — its `name` is a + // button id. Judging it would refuse a def that never reaches the body. + expect( + validateActionDispatchContract( + stack([AGGREGATE], { + bulkActionDefs: [{ name: 'recalc_selection', operation: 'update', patch: { done: true } }], + }), + ), + ).toEqual([]); + }); + + it('skips a def carrying an inlined `actionDef` — it brings its own dispatcher', () => { + expect( + validateActionDispatchContract( + stack([PER_RECORD], { + bulkActionDefs: [ + { name: 'recalc_estimate', operation: 'custom', execution: 'aggregate', actionDef: { type: 'api' } }, + ], + }), + ), + ).toEqual([]); + }); + + it("stays silent when two declarations of one name disagree — that is not the view's defect", () => { + const findings = validateActionDispatchContract({ + objects: [{ name: 'task', actions: [{ name: 'recalc', type: 'api', execution: 'aggregate' }] }], + actions: [{ name: 'recalc', type: 'api', execution: 'perRecord' }], + views: [{ name: 'task', object: 'task', list: { bulkActions: ['recalc'] } }], + }); + expect(findings).toEqual([]); + }); + + it('returns nothing at all for a stack that declares no contract anywhere', () => { + expect( + validateActionDispatchContract(stack([UNDECLARED], { bulkActions: ['mark_done'] })), + ).toEqual([]); + expect(validateActionDispatchContract({})).toEqual([]); + }); +}); + +describe('validateActionDispatchContract — every list tier', () => { + it('judges `listViews.` on a view', () => { + const findings = validateActionDispatchContract({ + objects: [{ name: 'task' }], + actions: [AGGREGATE], + views: [ + { + name: 'task', + object: 'task', + listViews: { bulk: { bulkActions: ['recalc_selection'] } }, + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0]!.path).toBe('views[0].listViews.bulk.bulkActions[0]'); + expect(findings[0]!.where).toContain('listViews.bulk'); + }); + + it("judges an OBJECT's own `listViews` — the tier an object-embedded action is wired from", () => { + const findings = validateActionDispatchContract({ + objects: [ + { + name: 'task', + actions: [PER_RECORD], + listViews: { + all: { + bulkActionDefs: [{ name: 'recalc_estimate', operation: 'custom', execution: 'aggregate' }], + }, + }, + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0]!.path).toBe('objects[0].listViews.all.bulkActionDefs[0]'); + expect(findings[0]!.where).toContain('object "task"'); + }); +}); diff --git a/packages/lint/src/validate-action-dispatch-contract.ts b/packages/lint/src/validate-action-dispatch-contract.ts new file mode 100644 index 0000000000..04113f029f --- /dev/null +++ b/packages/lint/src/validate-action-dispatch-contract.ts @@ -0,0 +1,289 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0049 — references] The bulk **dispatch contract** a list view wires an + * action under, checked against the contract the action's body declares it was + * written for (issue #17319; maintainer ruling, decision batch #121 item 3, + * 2026-09-12). + * + * ## The gap this closes + * + * A list view can wire the same declared action two ways, and the two deliver + * OPPOSITE input to the same body: + * + * - `bulkActions: ['']` — the bare-string form. The renderer promotes + * the action to a def and dispatches it **once per selected row**: each call + * carries that row's `recordId` and **no** `_selectedIds`. + * - a `bulkActionDefs` entry with `execution: 'aggregate'` — **one** dispatch + * for the whole selection: every id arrives in `params._selectedIds` and + * there is **no** `recordId`. + * + * Both mismatches used to fail quietly, in opposite directions: an aggregate + * body wired bare-string reads `_selectedIds` as `undefined`, takes its + * single-record branch, and reports success for one row out of ten; a + * per-record body wired aggregate finds no `recordId` and throws its own + * "nothing selected", which reads like a selection bug rather than a wiring + * one. + * + * ## Why nothing else can catch it + * + * The ADR-0104 strict params gate is structurally blind here, and that is worth + * stating precisely rather than assuming: `validateActionParams` admits every + * member of `ACTION_PARAM_BUILTIN_KEYS` — `recordId`, `objectName`, + * `_selectedIds` — without a declaration, and declaring one is refused by + * construction. So the two bags the two wirings produce differ in exactly the + * keys that gate is required to wave through, and it returns zero issues for + * both (pinned from the other side in `packages/spec/src/ui/action-params.test.ts`). + * The schema cannot see it either: the wiring lives on the VIEW and the + * declaration on the ACTION, so no single parse has both. + * + * ## What this rule refuses — and what it deliberately does not + * + * It refuses a **mismatch**: an action that DECLARES `execution` wired by a + * list view under the other contract. It names the action, the view and BOTH + * contracts, because the fix is a choice between them and a message naming one + * is a message that has already chosen. + * + * It says nothing about an action that declares NO `execution`. That is not a + * gap left open, it is the ruling's 「创业阶段不渐进」 in the one place it + * lands: there is **no silent default**, so undeclared means undeclared — not + * "per-record until proven otherwise" — and refusing it would refuse every + * app that predates the key. Undeclared is also the honest state of a body + * written to serve both contracts (it reads `recordId` AND `_selectedIds` and + * copes with either); the showcase ships one. Existing sources get their + * declarations from the ADR-0087 semantic migration entry + * `action-bulk-dispatch-contract-undeclared`, which derives them from these + * same wirings where they are unambiguous — so the population this rule judges + * grows by migration, never by guess. That is the ADR-0072 D1 zero-false- + * positive posture the sibling members hold. + * + * Nor does it re-check that the name resolves at all: `validateActionNameRefs` + * owns `action-name-undefined`, and a dead name gets one finding, not two. + */ + +import { recordsOf } from './object-graph.js'; + +export const ACTION_DISPATCH_CONTRACT_MISMATCH = 'action-dispatch-contract-mismatch'; + +export type ActionDispatchContractSeverity = 'error' | 'warning'; + +/** The two dispatch contracts, spelled as `bulkActionDefs.execution` spells them. */ +export type ActionDispatchContract = 'perRecord' | 'aggregate'; + +export interface ActionDispatchContractFinding { + /** Always `error` — the body is handed input it was not written for, silently. */ + severity: ActionDispatchContractSeverity; + /** Diagnostic rule id. */ + rule: string; + /** Human-readable location, e.g. `view "crm_lead" · list "all" · bulkActions`. */ + where: string; + /** Config path, e.g. `views[0].list.bulkActions[1]`. */ + path: string; + /** What is wrong. */ + message: string; + /** How to fix it. */ + hint: string; +} + +type AnyRec = Record; + +const CONTRACTS: readonly ActionDispatchContract[] = ['perRecord', 'aggregate']; + +function strName(v: unknown): string | undefined { + return typeof v === 'string' && v.length > 0 ? v : undefined; +} + +function contractOf(v: unknown): ActionDispatchContract | undefined { + return CONTRACTS.find((c) => c === v); +} + +/** + * One sentence per contract, written so the pair reads as a choice: each says + * how many dispatches happen and which of the two builtin keys arrives. Both + * sentences appear in every finding — the declared one and the wired one — + * because naming only the one that is "wrong" presumes which end the author + * meant to change. + */ +const CONTRACT_PROSE: Readonly> = { + perRecord: + "`execution: 'perRecord'` (the view's `bulkActions: ['']` bare-string form) — the " + + "renderer promotes the action to a def and dispatches it ONCE PER selected row, each call " + + "carrying that row's `recordId` and NO `_selectedIds`", + aggregate: + "`execution: 'aggregate'` (a `bulkActionDefs` entry naming the action) — ONE dispatch for the " + + 'whole selection, carrying every selected id in `params._selectedIds` and NO `recordId`', +}; + +/** What the body actually sees when it is written for one contract and wired the other. */ +const MISFIRE: Readonly> = { + perRecord: + 'a body written per-record finds no `recordId` on the single aggregate call and typically ' + + 'throws its own "nothing selected" — which reads in the console like a selection bug rather ' + + 'than a wiring one', + aggregate: + 'a body written for the aggregate call reads `_selectedIds` as `undefined` on every per-row ' + + 'dispatch, falls through to its single-record branch, and reports success for one row out of ' + + 'however many were selected', +}; + +/** + * Every action name in the stack that DECLARES a dispatch contract, mapped to + * the contract it declares. + * + * A name declared more than once (a global action and an object-embedded one, + * or two objects) contributes only when every declaration agrees: two + * declarations that disagree are a defect in the DECLARATIONS, not in any + * wiring, and charging a view for it would name the wrong file. Undeclared + * names are absent from the map, which is what makes "undeclared is not + * defaulted" structural here rather than a branch someone can drop. + */ +function collectDeclaredContracts(stack: AnyRec): Map { + const seen = new Map>(); + + const note = (action: unknown) => { + if (!action || typeof action !== 'object') return; + const a = action as AnyRec; + const name = strName(a.name); + if (!name) return; + const declared = contractOf(a.execution) ?? 'none'; + if (!seen.has(name)) seen.set(name, new Set()); + seen.get(name)!.add(declared); + }; + + for (const action of recordsOf(stack.actions)) note(action); + for (const obj of recordsOf(stack.objects)) { + if (!obj || typeof obj !== 'object') continue; + for (const action of recordsOf(obj.actions)) note(action); + } + + const declared = new Map(); + for (const [name, contracts] of seen) { + if (contracts.size !== 1) continue; + const only = [...contracts][0]!; + if (only !== 'none') declared.set(name, only); + } + return declared; +} + +/** + * Validate every list-view bulk wiring in a stack against the wired action's + * own dispatch declaration. Returns findings (empty = clean). + */ +export function validateActionDispatchContract(stack: AnyRec): ActionDispatchContractFinding[] { + const findings: ActionDispatchContractFinding[] = []; + if (!stack || typeof stack !== 'object') return findings; + + const declaredBy = collectDeclaredContracts(stack); + if (declaredBy.size === 0) return findings; + + const check = ( + name: string, + wired: ActionDispatchContract, + where: string, + path: string, + viewLabel: string, + ) => { + const declared = declaredBy.get(name); + if (declared === undefined || declared === wired) return; + + const rewire = wired === 'perRecord' + ? `move the wiring to \`bulkActionDefs: [{ name: '${name}', operation: 'custom', execution: 'aggregate' }]\`` + : `drop the def and name the action in the view's \`bulkActions: ['${name}']\` instead`; + + findings.push({ + severity: 'error', + rule: ACTION_DISPATCH_CONTRACT_MISMATCH, + where, + path, + message: + `Action "${name}" declares ${CONTRACT_PROSE[declared]}, but ${viewLabel} wires it as ` + + `${CONTRACT_PROSE[wired]}. The two contracts deliver opposite input to the same body: ` + + `${MISFIRE[declared]}. Nothing refuses this at runtime — \`recordId\` and ` + + '`_selectedIds` are both builtin action params (ADR-0104), so the strict params gate ' + + 'admits either bag without a word.', + hint: + `Pick the contract the body is actually written for and make both ends say it: either ` + + `change the action's declaration to \`execution: '${wired}'\` (if the body was written ` + + `for the wiring), or ${rewire} (if the body was written for the declaration). If the ` + + `two wirings are both wanted, they are two actions — one call and N calls have ` + + `different side effects, which is why the platform will not silently unify them.`, + }); + }; + + /** + * One list container: the default `list`, a `listViews.` entry, or an + * object-embedded one. Shared for the reason `validate-action-name-refs` + * shares its own — an object has no top-level `list`, and its `listViews` + * are a tier that has been missed before. + */ + const checkListContainer = ( + container: unknown, + owner: string, + label: string, + path: string, + ) => { + if (!container || typeof container !== 'object') return; + const list = container as AnyRec; + const viewLabel = `${owner} · ${label}`; + + const bare = Array.isArray(list.bulkActions) ? list.bulkActions : []; + for (let ai = 0; ai < bare.length; ai++) { + const name = strName(bare[ai]); + if (!name) continue; + check(name, 'perRecord', `${viewLabel} · bulkActions`, `${path}.bulkActions[${ai}]`, viewLabel); + } + + // Only an `execution: 'aggregate'` def NAMES an action (#4457): an + // `update`/`delete` def is a data-plane mass mutation whose `name` is a + // button id, and a hand-inlined `actionDef` carries its own dispatcher and + // resolves against nothing — the same two skips the name-ref sibling makes, + // for the same reasons. + const defs = Array.isArray(list.bulkActionDefs) ? (list.bulkActionDefs as AnyRec[]) : []; + for (let di = 0; di < defs.length; di++) { + const def = defs[di]; + if (!def || typeof def !== 'object') continue; + if (def.execution !== 'aggregate') continue; + if (def.actionDef !== undefined) continue; + const name = strName(def.name); + if (!name) continue; + check( + name, + 'aggregate', + `${viewLabel} · bulkActionDefs[${di}]`, + `${path}.bulkActionDefs[${di}]`, + viewLabel, + ); + } + }; + + // ── List views: `list` + each `listViews.`, on views AND on objects ── + const views = recordsOf(stack.views); + for (let vi = 0; vi < views.length; vi++) { + const view = views[vi]; + if (!view || typeof view !== 'object') continue; + const viewName = strName(view.name) ?? strName(view.object) ?? `#${vi}`; + const owner = `view "${viewName}"`; + + checkListContainer(view.list, owner, 'list', `views[${vi}].list`); + const listViews = view.listViews; + if (listViews && typeof listViews === 'object' && !Array.isArray(listViews)) { + for (const [key, lv] of Object.entries(listViews as AnyRec)) { + checkListContainer(lv, owner, `listViews.${key}`, `views[${vi}].listViews.${key}`); + } + } + } + + const objects = recordsOf(stack.objects); + for (let oi = 0; oi < objects.length; oi++) { + const obj = objects[oi]; + if (!obj || typeof obj !== 'object') continue; + const objListViews = obj.listViews; + if (!objListViews || typeof objListViews !== 'object' || Array.isArray(objListViews)) continue; + const owner = `object "${strName(obj.name) ?? `#${oi}`}"`; + for (const [key, lv] of Object.entries(objListViews as AnyRec)) { + checkListContainer(lv, owner, `listViews.${key}`, `objects[${oi}].listViews.${key}`); + } + } + + return findings; +} diff --git a/packages/metadata-protocol/src/protocol.meta-types-degenerate-derivation.test.ts b/packages/metadata-protocol/src/protocol.meta-types-degenerate-derivation.test.ts index 19eab51294..a7a30cb4a3 100644 --- a/packages/metadata-protocol/src/protocol.meta-types-degenerate-derivation.test.ts +++ b/packages/metadata-protocol/src/protocol.meta-types-degenerate-derivation.test.ts @@ -2,7 +2,7 @@ /** * [#17501] `GET /meta/types` must not serve an empty JSON Schema for a type - * that accepts 47 keys. + * that accepts 48 keys. * * ## What was wrong * @@ -23,7 +23,7 @@ * `additionalProperties: false` 663 to 637). So the fix gates the authoring * derivation behind a degeneracy check, and the load-bearing assertion is the * BLAST RADIUS: exactly one served type may differ from the pre-fix - * derivation. A suite that only pinned `action`'s 47 keys would stay green + * derivation. A suite that only pinned `action`'s 48 keys would stay green * through a later widening to `io: 'input'` for everything — which is the * change this card exists to refuse. This one goes red on it. * @@ -147,7 +147,7 @@ describe('#17501 — /meta/types serves a real schema for `action`, and moves no const properties = served!.properties as Record; expect(properties, '`action` must name its properties').toBeDefined(); - expect(Object.keys(properties).length).toBe(47); + expect(Object.keys(properties).length).toBe(48); // A sample an author would actually address, and the one #17500's // repeater titles need a node to sit on. for (const key of ['name', 'label', 'objectName', 'type', 'params', 'locations']) { diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 2c62711d60..a689f14e7f 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -420,7 +420,7 @@ const _warnedDegenerateDerivation = new Set(); * ## [#17501] Why this tries TWICE, and why only sometimes * * `/meta/types` served `action` as `{"$schema": "..."}` — an empty schema for a - * type that accepts 47 keys — because `ActionSchema` is + * type that accepts 48 keys — because `ActionSchema` is * `lazySchema(() => actionObject().refine(...))`, a `ZodPipe`, and the OUTPUT * derivation of a pipe carries no properties. The hand-crafted fallback * declared for exactly this case never fired: conversion did not throw, it diff --git a/packages/spec/authorable-surface/ui.json b/packages/spec/authorable-surface/ui.json index 625ae5cfcc..9d2131e842 100644 --- a/packages/spec/authorable-surface/ui.json +++ b/packages/spec/authorable-surface/ui.json @@ -25,6 +25,7 @@ "ui/Action:disabled", "ui/Action:errorMessage", "ui/Action:execute [RETIRED]", + "ui/Action:execution", "ui/Action:icon", "ui/Action:label", "ui/Action:locations", diff --git a/packages/spec/liveness/action.json b/packages/spec/liveness/action.json index d4f70f9b3a..41930a31d4 100644 --- a/packages/spec/liveness/action.json +++ b/packages/spec/liveness/action.json @@ -55,6 +55,13 @@ "evidence": "packages/runtime/src/action-execution.ts#declarativeUpdateWrite (`const patch = action?.patch` — the ONE producer of the write bag `{ ...patch, ...params }`; contract point 4, the static patch sits UNDER the values the dialog collected so a param of the same name wins, and nothing else from the action is merged); packages/runtime/src/action-execution.ts#executeDeclarativeUpdateAction (calls it and hands the bag to a single data-plane `update` of the routed row under the CALLER's own execution context; an empty bag is refused with a located 400 rather than answered 200 for a no-op, and `undoable` reads back the prior value of exactly the keys the bag names) — reached from both server doors, `handleActionsRequest` (REST) and `invokeBusinessAction` (MCP `run_action`); packages/spec/src/ui/action.zod.ts#refuseDeclarativeUpdateContradictions (authoring-time: `patch` is refused on an action without `operation: 'update'` — it would be silently dropped; `operation: 'update'` with neither `patch` nor `params` is refused as nothing-to-write)", "note": "The static field values of the declarative write, merged UNDER the collected `params` (a param of the same name wins). Passed through verbatim — no transform, no field-existence check here (a lint reference diagnostic). FLIPPED `planned` -> `live` 2026-09-08 (#15080) on the runtime half #15079 (PR #15448, merged 2026-09-04): the declared object is now the base of the bag one data-plane `update` writes, so authoring a key here changes what lands on the row. Declared by the #14092 maintainer ruling (2026-09-01): the row-level counterpart of a list view's `bulkActionDefs` `operation: 'update'`, spelled with the same words. The `patch`-UNDER-`params` precedence is the bulk def's own rule mirrored word for word, and it is pinned both ways — through the door and as the pure function — in packages/runtime/src/action-declarative-update.test.ts, beside the pin that the wire cannot widen the bag (an undeclared param is refused by ADR-0104 D2) and the pin that `undoable` restores exactly the keys this bag names. `evidenceScope: in-repo` is exact and deliberate: the verdict rests on the framework-side executor alone; the console half (objectui#7551) is a SECOND reader the flip did not wait for." }, + "execution": { + "status": "live", + "verifiedAt": "2026-09-13", + "evidenceScope": "in-repo", + "evidence": "packages/lint/src/validate-action-dispatch-contract.ts#validateActionDispatchContract (the one consumer: it reads `action.execution` into the declared-contract map and refuses a list view that wires the action under the other contract, `action-dispatch-contract-mismatch` at severity `error`, naming the action, the view and both contracts)", + "note": "#17319, maintainer ruling decision batch #121 item 3 (2026-09-12). Wired as a member of the reference-integrity suite, so the refusal runs on `os validate`, `os lint` and `os compile` at once rather than on one of them (that wiring file names the RULE, never this key, so it is not cited as evidence). The bulk dispatch contract an action's BODY is written for, in `bulkActionDefs`' own vocabulary — the def's `BulkActionExecutionSchema` imported rather than re-declared, so the ruling's ⛔ no-third-spelling is structural. `live` on an AUTHORING consumer, the `dashboard.widgets.suppressWarnings` precedent: the key changes no dispatch by itself, it is the declaration a build-time refusal is measured against, and deleting it would delete a real outcome. ⚠ Deliberately NOT a runtime refusal: the ruling ruled the lint half (item 2); a runtime refusal of a mismatched dispatch is a later card and would be a SECOND reader of this row, not a precondition of its verdict. ⛔ No silent default (「创业阶段不渐进」): the key is optional and an action that omits it is undeclared, not defaulted — which is also the honest state of a body written to serve both contracts. Existing sources are migrated by the ADR-0087 semantic entry `action-bulk-dispatch-contract-undeclared`, whose input was the census of `bulkActions` / `execution` over this repo and hotcrm@c716a2ccb3d31574a1a238a590f3e331ddae0200." + }, "target": { "status": "live", "verifiedAt": "2026-08-28", diff --git a/packages/spec/liveness/state-counts.md b/packages/spec/liveness/state-counts.md index 4fcea1ad80..0372e16c7c 100644 --- a/packages/spec/liveness/state-counts.md +++ b/packages/spec/liveness/state-counts.md @@ -30,7 +30,7 @@ for both corollaries. | `object` | 51 | 0 | 0 | 0 | 1 | 52 | | `field` | 90 | 0 | 0 | 1 | 2 | 93 | | `flow` | 34 | 0 | 0 | 6 | 0 | 40 | -| `action` | 43 | 0 | 0 | 3 | 2 | 48 | +| `action` | 44 | 0 | 0 | 3 | 2 | 49 | | `hook` | 19 | 0 | 0 | 3 | 0 | 22 | | `permission` | 36 | 0 | 0 | 6 | 0 | 42 | | `position` | 12 | 0 | 0 | 0 | 0 | 12 | @@ -63,4 +63,4 @@ for both corollaries. | `batch_endpoints` | 5 | 0 | 0 | 2 | 0 | 7 | | `route_generation` | 0 | 0 | 0 | 4 | 0 | 4 | | `realtime_subscription` | 0 | 0 | 0 | 6 | 0 | 6 | -| **total** | **861** | **5** | **1** | **95** | **10** | **972** | +| **total** | **862** | **5** | **1** | **95** | **10** | **973** | diff --git a/packages/spec/src/migrations/entries/semantic/18.action-bulk-dispatch-contract-undeclared.ts b/packages/spec/src/migrations/entries/semantic/18.action-bulk-dispatch-contract-undeclared.ts new file mode 100644 index 0000000000..778f5a1cef --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.action-bulk-dispatch-contract-undeclared.ts @@ -0,0 +1,47 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +export const entry: SemanticMigration = { + id: 'action-bulk-dispatch-contract-undeclared', + surface: '`action.execution` — the bulk dispatch contract an action’s body is written for', + replacement: + "Declare `execution: 'perRecord' | 'aggregate'` on every action a list view wires into the " + + 'selection bar, DERIVED from the wiring that action already has: a view naming it in ' + + "`bulkActions: ['']` (the bare-string form) dispatches it once per selected row with " + + "that row's `recordId` ⇒ `execution: 'perRecord'`; a `bulkActionDefs` entry naming it with " + + "`execution: 'aggregate'` dispatches it once for the whole selection with every id in " + + "`params._selectedIds` ⇒ `execution: 'aggregate'`. The derivation is exact wherever an " + + 'action is wired ONE way, because the wiring is what the body has been receiving all along ' + + '— declaring it changes no behaviour, it writes down the behaviour. ⛔ There is no default: ' + + 'an action no view bulk-wires, and an action whose body genuinely serves both contracts ' + + '(it reads `recordId` AND `_selectedIds` and copes with either), stays UNDECLARED rather ' + + 'than being given a value.', + reason: + 'Not losslessly convertible, because the fact being written down does not live on the item ' + + 'being rewritten. The declaration belongs to the ACTION and the evidence for it belongs to ' + + 'the VIEWS — potentially several, in other files or other packages — so no per-item ' + + 'transform has both halves in hand, and `objectstack migrate meta` rewrites stored metadata ' + + 'by key. The residue is genuinely a judgement: an action wired BOTH ways has no correct ' + + 'value, because one call and N calls have different side effects and the platform will not ' + + 'silently unify them (the #17319 ruling refused exactly that option). Such an action is TWO ' + + 'actions — split the body along the line the two wirings already draw and declare each half ' + + '— or, if the body was deliberately written to serve both, it stays undeclared and the two ' + + 'wirings stand. The census that is this migration’s input was taken 2026-09-13 over ' + + 'objectstack@a9c64779046 (shipped app metadata, test fixtures excluded: 13 distinct ' + + 'bulk-wired actions — 11 unambiguously per-record, 1 unambiguously aggregate, 1 wired both ' + + 'ways) and hotcrm@c716a2ccb3d31574a1a238a590f3e331ddae0200 (3 distinct bulk-wired actions — ' + + '2 per-record, 1 aggregate, 0 wired both ways). So the both-ways residue is real but rare, ' + + 'which is why it is a structured TODO and not a blocking rewrite.', + acceptanceCriteria: + '`objectstack validate` (and `os lint` / `os build`) reports no ' + + '`action-dispatch-contract-mismatch` finding on the stack; every action a list view wires ' + + 'into the selection bar either declares the `execution` its wiring implies, or is ' + + 'deliberately left undeclared with the reason recorded beside it; no action is wired both ' + + 'ways while declaring either contract. Prove the derivation rather than assuming it: for ' + + "each action you declared `'aggregate'`, its body reads `params._selectedIds` and does NOT " + + "depend on `ctx.recordId`; for each you declared `'perRecord'`, the reverse. Run the bulk " + + 'button once per declared action against a multi-row selection and confirm the number of ' + + 'dispatches matches the declaration (N for per-record, one for aggregate) — a mismatch that ' + + 'used to be silent is what this key exists to surface.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 29109ec367..1a9ef31deb 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -5490,6 +5490,49 @@ const step18: MigrationStep = { // entry id by `gen:migration-registry` (#7297). Add an entry by adding a // FILE — never by editing between the markers, which is generated. // + { + id: 'action-bulk-dispatch-contract-undeclared', + surface: '`action.execution` — the bulk dispatch contract an action’s body is written for', + replacement: + "Declare `execution: 'perRecord' | 'aggregate'` on every action a list view wires into the " + + 'selection bar, DERIVED from the wiring that action already has: a view naming it in ' + + "`bulkActions: ['']` (the bare-string form) dispatches it once per selected row with " + + "that row's `recordId` ⇒ `execution: 'perRecord'`; a `bulkActionDefs` entry naming it with " + + "`execution: 'aggregate'` dispatches it once for the whole selection with every id in " + + "`params._selectedIds` ⇒ `execution: 'aggregate'`. The derivation is exact wherever an " + + 'action is wired ONE way, because the wiring is what the body has been receiving all along ' + + '— declaring it changes no behaviour, it writes down the behaviour. ⛔ There is no default: ' + + 'an action no view bulk-wires, and an action whose body genuinely serves both contracts ' + + '(it reads `recordId` AND `_selectedIds` and copes with either), stays UNDECLARED rather ' + + 'than being given a value.', + reason: + 'Not losslessly convertible, because the fact being written down does not live on the item ' + + 'being rewritten. The declaration belongs to the ACTION and the evidence for it belongs to ' + + 'the VIEWS — potentially several, in other files or other packages — so no per-item ' + + 'transform has both halves in hand, and `objectstack migrate meta` rewrites stored metadata ' + + 'by key. The residue is genuinely a judgement: an action wired BOTH ways has no correct ' + + 'value, because one call and N calls have different side effects and the platform will not ' + + 'silently unify them (the #17319 ruling refused exactly that option). Such an action is TWO ' + + 'actions — split the body along the line the two wirings already draw and declare each half ' + + '— or, if the body was deliberately written to serve both, it stays undeclared and the two ' + + 'wirings stand. The census that is this migration’s input was taken 2026-09-13 over ' + + 'objectstack@a9c64779046 (shipped app metadata, test fixtures excluded: 13 distinct ' + + 'bulk-wired actions — 11 unambiguously per-record, 1 unambiguously aggregate, 1 wired both ' + + 'ways) and hotcrm@c716a2ccb3d31574a1a238a590f3e331ddae0200 (3 distinct bulk-wired actions — ' + + '2 per-record, 1 aggregate, 0 wired both ways). So the both-ways residue is real but rare, ' + + 'which is why it is a structured TODO and not a blocking rewrite.', + acceptanceCriteria: + '`objectstack validate` (and `os lint` / `os build`) reports no ' + + '`action-dispatch-contract-mismatch` finding on the stack; every action a list view wires ' + + 'into the selection bar either declares the `execution` its wiring implies, or is ' + + 'deliberately left undeclared with the reason recorded beside it; no action is wired both ' + + 'ways while declaring either contract. Prove the derivation rather than assuming it: for ' + + "each action you declared `'aggregate'`, its body reads `params._selectedIds` and does NOT " + + "depend on `ctx.recordId`; for each you declared `'perRecord'`, the reverse. Run the bulk " + + 'button once per declared action against a multi-row selection and confirm the number of ' + + 'dispatches matches the declaration (N for per-record, one for aggregate) — a mismatch that ' + + 'used to be silent is what this key exists to surface.', + }, { id: 'address-location-value-unknown-keys-refused', surface: 'stored `address` and `location` field VALUES (`AddressSchema` / `AddressValueSchema`, ' diff --git a/packages/spec/src/ui/action-dispatch-contract.test.ts b/packages/spec/src/ui/action-dispatch-contract.test.ts new file mode 100644 index 0000000000..1431546c4c --- /dev/null +++ b/packages/spec/src/ui/action-dispatch-contract.test.ts @@ -0,0 +1,144 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #17319 — an action declares the bulk dispatch contract its body is written + * for (maintainer ruling, decision batch #121 item 3, 2026-09-12: 「同意」). + * + * What these tests pin, in the order the defect is argued: + * + * - THE DEFECT, reproduced first and kept: the SAME declared action, under the + * two wirings, produces two params bags that differ in exactly the keys the + * ADR-0104 strict gate is required to wave through — so `validateActionParams` + * returns ZERO issues for both. That is the "nothing catches it" half of the + * card, measured here rather than recalled, and it is deliberately still + * true after this change: the key added by #17319 is an AUTHORING + * declaration, so the reproduction stands and the refusal lands in + * `@objectstack/lint` (`action-dispatch-contract-mismatch`). + * - THE VOCABULARY: the set of values the ACTION accepts is exactly + * `BulkActionExecutionSchema.options` — the def's own two — and nothing + * else. The ruling admits no third spelling, so the pin is the accepted SET, + * measured against a battery of third spellings that must all be refused. + * - ACCEPT / REFUSE: both values parse; the near-miss KEY spellings rename onto + * `execution`; `mode` does NOT (it is a declared action key with its own + * meaning, unlike on the def, where `mode` aliases onto `execution`). + * - NO SILENT DEFAULT: an action that omits the key parses, and the parsed + * shape carries NO `execution` — not `'perRecord'`, not `'aggregate'`. + */ + +import { describe, expect, it } from 'vitest'; +import { ActionSchema } from './action.zod'; +import { BulkActionDefSchema, BulkActionExecutionSchema } from './bulk-action.zod'; +import { ACTION_PARAM_BUILTIN_KEYS, validateActionParams } from './action-params.zod'; +import type { ResolvedActionParam } from './action-params.zod'; + +/** The showcase's aggregate-side action, reduced to what the contract needs. */ +const undeclaredAction = { + name: 'recalc_selection', + label: 'Recalculate selection', + type: 'api' as const, + target: '/api/recalc', +}; +const recalcSelection = { ...undeclaredAction, execution: 'aggregate' as const }; + +describe('#17319 — the defect, reproduced (and still true: this is an authoring key)', () => { + it('hands the SAME action opposite input under the two wirings, with zero diagnostics', () => { + // One declared param — everything else in each bag is a builtin the author + // cannot declare and the gate must admit. + const resolved: ResolvedActionParam[] = [{ name: 'format', type: 'text' }]; + + // Wiring A — `bulkActions: ['recalc_selection']`: N dispatches, each + // carrying ONE row id and no selection. + const perRecordBag = { format: 'png', recordId: 'task_1', objectName: 'task' }; + // Wiring B — a `bulkActionDefs` entry with `execution: 'aggregate'`: ONE + // dispatch carrying the whole selection and no record id. + const aggregateBag = { format: 'png', _selectedIds: ['task_1', 'task_2', 'task_3'], objectName: 'task' }; + + // Opposite input… + expect('recordId' in perRecordBag).toBe(true); + expect('_selectedIds' in perRecordBag).toBe(false); + expect('recordId' in aggregateBag).toBe(false); + expect('_selectedIds' in aggregateBag).toBe(true); + + // …and the strict params gate is silent on both, because the two keys that + // DECIDE the contract are the two it is required to admit undeclared. + expect(validateActionParams(resolved, perRecordBag)).toEqual([]); + expect(validateActionParams(resolved, aggregateBag)).toEqual([]); + expect(ACTION_PARAM_BUILTIN_KEYS).toContain('recordId'); + expect(ACTION_PARAM_BUILTIN_KEYS).toContain('_selectedIds'); + + // The gate is not broken — it refuses a bag key that is NOT a builtin. The + // control that makes the two silences above a reading rather than a dead + // probe: it could have come back the other way, and for this key it does. + expect(validateActionParams(resolved, { format: 'png', selectedIds: ['a'] }).map((i) => i.code)) + .toEqual(['unknown_field']); + }); +}); + +describe("#17319 — the vocabulary is `bulkActionDefs`' own", () => { + it('accepts exactly the def`s two options on the action, and no third spelling', () => { + expect(BulkActionExecutionSchema.options).toEqual(['perRecord', 'aggregate']); + + for (const value of BulkActionExecutionSchema.options) { + expect(ActionSchema.safeParse({ ...undeclaredAction, execution: value }).success).toBe(true); + } + + // Every plausible third spelling — including `per_record`, the one the + // filing card proposed and therefore the one most likely to be typed. + const thirdSpellings = [ + 'per_record', 'perrecord', 'PerRecord', 'record', 'single', 'each', 'fanout', 'fan_out', + 'batch', 'bulk', 'all', 'once', 'set', 'aggregated', 'Aggregate', '', + ]; + for (const value of thirdSpellings) { + expect( + { value, accepted: ActionSchema.safeParse({ ...undeclaredAction, execution: value }).success }, + ).toEqual({ value, accepted: false }); + } + }); + + it('leaves the def`s own key untouched — mirrored, not moved', () => { + expect(BulkActionDefSchema.safeParse({ + name: 'recalc_selection', operation: 'custom', execution: 'aggregate', + }).success).toBe(true); + }); +}); + +describe('#17319 — accept, refuse, and the key spellings', () => { + it('accepts both declared contracts and keeps the value verbatim', () => { + expect(ActionSchema.parse(recalcSelection).execution).toBe('aggregate'); + expect(ActionSchema.parse({ ...undeclaredAction, execution: 'perRecord' }).execution).toBe('perRecord'); + }); + + it('refuses a third value at the `execution` path, naming the two that exist', () => { + const res = ActionSchema.safeParse({ ...undeclaredAction, execution: 'per_record' }); + expect(res.success).toBe(false); + const issue = res.error!.issues.find((i) => i.path.join('.') === 'execution'); + expect(issue).toBeDefined(); + expect(JSON.stringify(issue)).toContain('perRecord'); + expect(JSON.stringify(issue)).toContain('aggregate'); + }); + + it('renames the near-miss KEY spellings onto `execution`', () => { + for (const alias of ['dispatch', 'dispatchContract', 'bulkExecution', 'bulkDispatch']) { + const res = ActionSchema.safeParse({ ...undeclaredAction, [alias]: 'aggregate' }); + expect({ alias, ok: res.success }).toEqual({ alias, ok: false }); + expect(JSON.stringify(res.error!.issues)).toContain('execution'); + } + }); + + it('⛔ does NOT rename `mode` — on an ACTION that is a declared key of its own', () => { + // The bulk def aliases `mode` onto `execution`; an action must not, or a + // real `mode: 'create'` declaration would be renamed out from under its + // author. This is the one place the two surfaces' alias tables differ. + const res = ActionSchema.safeParse({ ...undeclaredAction, mode: 'create' }); + expect(res.success).toBe(true); + expect(res.data!.mode).toBe('create'); + }); +}); + +describe('#17319 — ⛔ no silent default for an undeclared action', () => { + it('parses an action that omits the key, and leaves it ABSENT', () => { + const parsed = ActionSchema.parse(undeclaredAction); + expect(parsed.execution).toBeUndefined(); + expect(Object.prototype.hasOwnProperty.call(parsed, 'execution')).toBe(false); + }); +}); diff --git a/packages/spec/src/ui/action.zod.ts b/packages/spec/src/ui/action.zod.ts index 90a6e6c486..2527b90910 100644 --- a/packages/spec/src/ui/action.zod.ts +++ b/packages/spec/src/ui/action.zod.ts @@ -13,6 +13,13 @@ import { FieldType } from '../data/field.zod'; import { MULTI_CAPABLE_TYPES, isMultiValueField } from '../data/field-value.zod'; import { checkLiteralDefaultValue } from '../data/default-value-shape'; import { isActionParamValuePresent } from './action-params.zod'; +// #17319 — the action's `execution` declaration is the bulk def's OWN enum, +// imported rather than re-declared: the maintainer ruling (decision batch #121 +// item 3) admits no third spelling of the two dispatch contracts, and sharing +// the schema object is the only form of that which cannot drift. Imported +// file-directly for the reason the neighbours above are: `bulk-action.zod` +// reaches only `shared/` + `data/`, so it cannot close a cycle back to `ui/`. +import { BulkActionExecutionSchema } from './bulk-action.zod'; import { SnakeCaseIdentifierSchema } from '../shared/identifiers.zod'; import { ExpressionInputSchema } from '../shared/expression.zod'; import { I18nLabelSchema, AriaPropsSchema } from './i18n.zod'; @@ -857,6 +864,15 @@ const actionObject = () => strictObject({ // author borrows for "the field values to write" (`values`, `set`, or the // verb itself) all rename onto the two declarative-update keys. op: 'operation', values: 'patch', set: 'patch', update: 'patch', + // #17319 — the words an author reaches for when declaring which bulk + // dispatch contract the body was written for. The card that filed the gap + // proposed `dispatch`, so that spelling is the one most likely to be + // typed; the canonical key is `execution`, the bulk def's own. + // ⛔ NOT `mode`: the def aliases `mode` onto `execution`, but on an ACTION + // `mode` is a DECLARED key (create/edit/delete/custom), so renaming it here + // would eat a real declaration. + dispatch: 'execution', dispatchContract: 'execution', + bulkExecution: 'execution', bulkDispatch: 'execution', // #5013 — `body` is DECLARED on this schema (the `script` action's L1/L2 // hook body), so an alias filed under it could never run; `payload` is the // live spelling that still needs pointing at `bodyExtra`. @@ -1117,6 +1133,62 @@ const actionObject = () => strictObject({ */ patch: z.record(z.string(), z.unknown()).optional().describe("For `operation: 'update'` — static field values written to the current record, merged UNDER the user-supplied `params` so a fixed value can be declared without exposing it in the dialog. Written on the data plane as the caller: object permissions, hooks and validations fire as for a user edit. Refused on an action without `operation: 'update'` (it would be silently dropped)."), + /** + * The **bulk dispatch contract this action's body is written for** (#17319, + * maintainer ruling, decision batch #121 item 3, 2026-09-12). + * + * A list view can wire the same declared action two ways, and the two hand + * the SAME body opposite input: + * + * - `bulkActions: ['']` — the bare-string form. The renderer promotes + * the action to a def and dispatches it **once per selected row**; each + * call carries that row's `recordId` and **no** `_selectedIds`. + * - a `bulkActionDefs` entry with `execution: 'aggregate'` — **one** + * dispatch for the whole selection; every id arrives in the builtin + * `params._selectedIds` and there is **no** `recordId`. + * + * Until this key existed the action declared neither, so both mismatches + * failed quietly and in opposite directions: an aggregate body wired + * bare-string reads `_selectedIds` as `undefined`, falls into its + * single-record branch and reports success for one row out of ten; a + * per-record body wired aggregate finds no `recordId` and throws its own + * "nothing selected", which reads like a selection bug. **Nothing caught + * either**: the ADR-0104 strict params gate cannot, because `_selectedIds` + * and `recordId` are both `ACTION_PARAM_BUILTIN_KEYS` — admitted + * without a declaration, and never declarable — so the one key that decides + * the contract is exactly the key that gate is structurally blind to + * (pinned in `action-params.test.ts`). The cost was paid in prose: the + * reference CRM carried the distinction in hand-copied comment blocks, the + * largest surviving constraint block on its action surface. + * + * **The vocabulary is `bulkActionDefs`' own, deliberately — `execution`, + * `'perRecord' | 'aggregate'`, the very {@link BulkActionExecutionSchema} + * the def parses with.** Not a second spelling of one idea: an action and a + * def name the same two dispatches with the same word and the same two + * values, the way `operation` / `patch` already mirror the def's + * declarative update. Importing the def's enum rather than re-declaring it + * is what makes "no third spelling" structural instead of remembered. + * + * **Optional, and there is NO silent default** (the ruling's + * 「创业阶段不渐进」). An action that omits it is *undeclared*, not + * defaulted to either contract, and `@objectstack/lint` refuses nothing — + * undeclared is also the honest state of a body written to serve BOTH + * contracts (it reads `recordId` and `_selectedIds` and copes with either), + * which is why no third enum member was added for it. Existing actions get + * their declaration from the ADR-0087 semantic migration entry + * `action-bulk-dispatch-contract-undeclared`, which derives it from the + * view wirings where they are unambiguous and hands back a structured TODO + * where one action is wired both ways. + * + * ENFORCEMENT: authoring-time, by `@objectstack/lint`'s + * `validateActionDispatchContract` (`action-dispatch-contract-mismatch`, + * severity `error`) — a list view that wires a declared action under the + * OTHER contract is refused, naming the action, the view and both + * contracts. The key changes no dispatch by itself; it is the declaration + * the refusal is measured against. + */ + execution: BulkActionExecutionSchema.optional().describe("The bulk dispatch contract this action's BODY is written for, in `bulkActionDefs`' own vocabulary: 'perRecord' = one dispatch per selected row carrying that row's `recordId` (the view's `bulkActions: ['']` bare-string form); 'aggregate' = ONE dispatch for the whole selection carrying every id in `params._selectedIds` (a `bulkActionDefs` entry with `execution: 'aggregate'`). Optional with NO default — omit it only when the body genuinely serves both. A list view wiring a declared action under the other contract is refused by `@objectstack/lint` (`action-dispatch-contract-mismatch`)."), + /** * [REMOVED in protocol 17 — #3855] The deprecated alias of `target`. * Tombstoned rather than deleted: `ActionSchema` is `strictObject`, so a diff --git a/packages/spec/src/ui/bulk-action.test.ts b/packages/spec/src/ui/bulk-action.test.ts index 4574cf4ca1..6ad9cccd8c 100644 --- a/packages/spec/src/ui/bulk-action.test.ts +++ b/packages/spec/src/ui/bulk-action.test.ts @@ -126,6 +126,17 @@ describe('BulkActionDefSchema (#4457)', () => { expect(issues.join('\n')).toContain('`excution` → `execution`'); }); + it('renames `mode` onto `execution` — the alias an ACTION deliberately lacks', () => { + // The def aliases `mode`; `ActionSchema` must NOT, because there `mode` + // is a declared key (create/edit/delete/custom) and renaming it would eat + // a real declaration — pinned in `action-dispatch-contract.test.ts`. The + // two surfaces' alias tables differ by exactly this entry, so without + // this assertion deleting the def-side `mode` alias reds nothing while + // silently falsifying that comparison. + const issues = reject({ name: 'recalc_selection', operation: 'custom', mode: 'aggregate' }); + expect(issues.join('\n')).toContain('`mode` → `execution`'); + }); + it('refuses a hand-written `actionDef` with the reason, not a spelling hint', () => { const issues = reject({ name: 'recalc_selection',