diff --git a/.changeset/embedded-action-crossref-validation.md b/.changeset/embedded-action-crossref-validation.md new file mode 100644 index 0000000000..ff7a4a7890 --- /dev/null +++ b/.changeset/embedded-action-crossref-validation.md @@ -0,0 +1,59 @@ +--- +"@objectstack/spec": minor +--- + +fix(spec): `defineStack`'s action cross-reference walk now reaches OBJECT-EMBEDDED actions (#7397) + +The same defect as #6889, one authoring position over. `validateCrossReferences` iterated +the registered list (`config.actions`) and — since #6889 — inline page-element actions. +Neither walk visited actions authored directly on an object +(`config.objects[].actions[]`), even though that position carries the **full +`ActionSchema`**: the identical symbol the registered collection uses, not a narrower +embedded shape. So a `type: 'modal'` or `type: 'flow'` target written there was never +resolved. The card's ten-row probe, measured on `main` before this change — every stack +declares both a page and a flow, so the plugin size-gate cannot explain the acceptances, +and each embedded row's registered twin is built from the same helper with the same +arguments: + +``` +a embedded modal -> page : ACCEPTED (h registered: ACCEPTED) +b embedded modal -> nothing : ACCEPTED (f registered: REJECTED) <- split +c embedded flow -> nothing : ACCEPTED (g registered: REJECTED) <- split +d embedded modal -> object : ACCEPTED (i registered: REJECTED) <- split +e embedded flow -> flow : ACCEPTED (j registered: ACCEPTED) +``` + +Rows b/f, c/g and d/i are the **same action object in two authoring positions with +opposite verdicts**. Row b is the defect on its own terms — a target naming nothing at all +built clean, shipped, and failed only when a user clicked it. + +Why the registered walk could not already cover it: `validateCrossReferences` runs +**before** `mergeActionsIntoObjects`, and that merge only ever copies top-level → object +(by `objectName`), never the reverse. An action authored only on the object therefore +never appeared in the validated `config.actions` at all. + +**Now**: `config.objects[].actions[]` is walked and every action found is subjected to the +**same** two target checks as a registered one — same rule, same message tail, same size +gates. Messages keep the registered wording from `references` onward and change only the +subject, because an embedded action is located by its owning object (`name` is required at +this position, so it is always available): + +``` +Action 'new_task' on object 'task' references page 'nowhere' (via modal target) which is +not defined in pages. +``` + +Scope of the modal arm is the maintainer's ruling on #6739 (2026-08-09): **a +`type: 'modal'` target names a PAGE, only** — so this arm mirrors the registered one +rather than also accepting an object name, closing the d/i split exactly as #6889 closed +the inline one. `objectName` is deliberately left unchecked at this position: the key does +exist on this shape, but what it should mean on an action already embedded on an object — +a mere existence check, or a consistency check against the owning object's name — is an +open contract question filed separately rather than settled as a side effect of this walk. + +**Acceptance-face narrowing.** A stack carrying a dangling embedded `modal`/`flow` target +now fails to build where it previously built clean. Census of the shipped corpus found +**zero** stacks affected: no object in the reference corpus hand-authors a `modal`- or +`flow`-typed action at this position, and the ordinary way `objects[].actions[]` gets +populated — the top-level → object merge — runs after validation and is unaffected. A +vacuity guard pins that merged shape so the census cannot go stale silently. diff --git a/packages/spec/src/stack-inline-action-crossref.test.ts b/packages/spec/src/stack-inline-action-crossref.test.ts index f5d86a021e..1f386c26e3 100644 --- a/packages/spec/src/stack-inline-action-crossref.test.ts +++ b/packages/spec/src/stack-inline-action-crossref.test.ts @@ -1,6 +1,7 @@ /** - * `defineStack` cross-reference validation reaches INLINE (page-element) - * actions — #6889. + * `defineStack` cross-reference validation reaches every authoring position an + * action's modal/flow `target` can be written in — INLINE page-element actions + * (#6889) and OBJECT-EMBEDDED actions (#7397). * * An action authored inline on a page element (`element:button` → * `properties.action`, an `InlineActionSchema`) never enters `config.actions`, @@ -22,6 +23,28 @@ * mirrors registered exactly — same rule, same message tail, one more * traversal. * + * The SECOND half of this file is the same defect one authoring position over + * (#7397). `config.objects[].actions[]` carries the FULL `ActionSchema` — the + * identical symbol the registered collection uses — yet it too was never + * target-validated, because `validateCrossReferences` runs BEFORE + * `mergeActionsIntoObjects` and that merge only ever copies top-level → + * object. An action authored only on the object therefore never appeared in + * the validated `config.actions`. Measured on `main` @ `d13ce33`, with the + * registered twin of each row built from the same helper and the same + * arguments as the control: + * + * ``` + * a embedded modal -> page : ACCEPTED (h registered: ACCEPTED) + * b embedded modal -> nothing : ACCEPTED (f registered: REJECTED) ← split + * c embedded flow -> nothing : ACCEPTED (g registered: REJECTED) ← split + * d embedded modal -> object : ACCEPTED (i registered: REJECTED) ← split + * e embedded flow -> flow : ACCEPTED (j registered: ACCEPTED) + * ``` + * + * Rows b/f, c/g and d/i are the same action object in two authoring positions + * with opposite verdicts; a/h and e/j confirm the legitimate shapes survive in + * both. Row d's verdict is fixed by the same #6739 ruling, not re-decided here. + * * Message shape is contract here (one condition ⇒ one wording), so these pin * full message text rather than `toThrow()` alone: a bare throw assertion * cannot tell "refused for the right reason" from "refused because the fixture @@ -252,3 +275,204 @@ describe('defineStack — inline action cross-references: what the walk must NOT }))).toEqual([]); }); }); + +// ─── #7397 — the OBJECT-EMBEDDED position ──────────────────────────── + +const pages = [pageWith([])]; + +/** + * A stack whose ONLY action is embedded on the object — no `config.actions` at + * all, which is exactly the shape the top-level → object merge can never + * reach. + */ +const embeddedStack = (action: unknown, extra: Record = {}) => ({ + manifest: baseManifest, + objects: [{ ...objects[0], actions: [action] }], + pages, + ...extra, +}); + +/** The same action in the REGISTERED position — the control for each row. */ +const registeredStack = (action: unknown, extra: Record = {}) => ({ + manifest: baseManifest, + objects, + pages, + actions: [action], + ...extra, +}); + +const modalAction = (target: string) => ({ name: 'probe_new_task', label: 'New', type: 'modal' as const, target }); +const flowAction = (target: string) => ({ name: 'probe_run', label: 'Run', type: 'flow' as const, target }); + +describe('defineStack — object-embedded action cross-references: modal targets (#7397)', () => { + it('rejects a dangling embedded modal target (probe row b) with the registered rule\'s wording', () => { + expect(refusals(embeddedStack(modalAction('probe_nowhere')))).toEqual([ + "Action 'probe_new_task' on object 'probe_task' " + + "references page 'probe_nowhere' (via modal target) which is not defined in pages.", + ]); + }); + + it('rejects an embedded modal target naming an OBJECT — #6739 ruling A, a modal target names a page (probe row d)', () => { + expect(refusals(embeddedStack(modalAction('probe_task')))).toEqual([ + "Action 'probe_new_task' on object 'probe_task' " + + "references page 'probe_task' (via modal target) which is not defined in pages.", + ]); + }); + + it('accepts an embedded modal target naming a declared page — the legitimate shape survives (probe row a)', () => { + expect(refusals(embeddedStack(modalAction('probe_home')))).toEqual([]); + }); + + it('skips embedded modal targets when the stack declares NO pages — same size gate as the registered rule', () => { + // The referenced page may be provided by a plugin; embedded must not be + // stricter than registered. + expect(refusals({ + manifest: baseManifest, + objects: [{ ...objects[0], actions: [modalAction('probe_nowhere')] }], + })).toEqual([]); + }); +}); + +describe('defineStack — object-embedded action cross-references: flow targets (#7397)', () => { + it('rejects an embedded flow target that names no declared flow (probe row c)', () => { + expect(refusals(embeddedStack(flowAction('probe_nowhere'), { flows }))).toEqual([ + "Action 'probe_run' on object 'probe_task' " + + "references flow 'probe_nowhere' which is not defined in flows.", + ]); + }); + + it('accepts an embedded flow target that names a declared flow (probe row e)', () => { + expect(refusals(embeddedStack(flowAction('probe_flow'), { flows }))).toEqual([]); + }); + + it('skips embedded flow targets when the stack declares NO flows — same size gate as the registered rule', () => { + expect(refusals(embeddedStack(flowAction('probe_nowhere')))).toEqual([]); + }); +}); + +describe('defineStack — object-embedded action cross-references: the b/f, c/g, d/i splits (#7397)', () => { + it.each([ + ['modal → nothing (rows b/f)', modalAction('probe_nowhere')], + ['modal → object (rows d/i)', modalAction('probe_task')], + ['flow → nothing (rows c/g)', flowAction('probe_nowhere')], + ])('gives the same verdict embedded or registered: %s', (_label, action) => { + expect(refusals(embeddedStack(action, { flows })).length).toBe(1); + expect(refusals(registeredStack(action, { flows })).length).toBe(1); + }); + + it.each([ + ['modal → declared page (rows a/h)', modalAction('probe_home')], + ['flow → declared flow (rows e/j)', flowAction('probe_flow')], + ])('accepts in BOTH positions: %s', (_label, action) => { + expect(refusals(embeddedStack(action, { flows }))).toEqual([]); + expect(refusals(registeredStack(action, { flows }))).toEqual([]); + }); +}); + +describe('defineStack — object-embedded action cross-references: the traversal itself (#7397)', () => { + it('labels each offender with ITS OWN object, not a fixed subject', () => { + const config = { + manifest: baseManifest, + objects: [ + { ...objects[0], actions: [modalAction('probe_nowhere')] }, + { + name: 'probe_note', + label: 'Probe Note', + fields: { body: { type: 'text' as const } }, + actions: [flowAction('probe_elsewhere')], + }, + ], + pages, + flows, + }; + + expect(refusals(config)).toEqual([ + "Action 'probe_new_task' on object 'probe_task' " + + "references page 'probe_nowhere' (via modal target) which is not defined in pages.", + "Action 'probe_run' on object 'probe_note' " + + "references flow 'probe_elsewhere' which is not defined in flows.", + ]); + }); + + it('reports every offending action on one object, not just the first', () => { + const config = { + manifest: baseManifest, + objects: [{ + ...objects[0], + actions: [ + { name: 'probe_one', label: 'One', type: 'modal' as const, target: 'probe_nowhere' }, + { name: 'probe_two', label: 'Two', type: 'flow' as const, target: 'probe_elsewhere' }, + ], + }], + pages, + flows, + }; + + expect(refusals(config)).toEqual([ + "Action 'probe_one' on object 'probe_task' " + + "references page 'probe_nowhere' (via modal target) which is not defined in pages.", + "Action 'probe_two' on object 'probe_task' " + + "references flow 'probe_elsewhere' which is not defined in flows.", + ]); + }); + + it('reports a registered action ONCE, not twice — the merge runs AFTER validation', () => { + // `mergeActionsIntoObjects` copies a top-level action carrying `objectName` + // into that object's `actions`. It runs at the END of `defineStack`, so the + // embedded walk must not see the copy. If validation is ever reordered + // ahead of the merge, this case starts reporting the same action twice — + // once as registered, once as embedded — and goes red. + const config = { + manifest: baseManifest, + objects, + pages, + actions: [{ ...modalAction('probe_nowhere'), objectName: 'probe_task' }], + }; + + expect(refusals(config)).toEqual([ + "Action 'probe_new_task' references page 'probe_nowhere' (via modal target) which is not defined in pages.", + ]); + }); + + it('still checks the target of an embedded action that also names its object', () => { + expect(refusals(embeddedStack({ ...modalAction('probe_nowhere'), objectName: 'probe_task' }))).toEqual([ + "Action 'probe_new_task' on object 'probe_task' " + + "references page 'probe_nowhere' (via modal target) which is not defined in pages.", + ]); + }); + + it('leaves an object with no actions array alone', () => { + expect(refusals({ manifest: baseManifest, objects, pages, flows })).toEqual([]); + }); +}); + +describe('defineStack — object-embedded action cross-references: what the walk must NOT refuse (#7397)', () => { + it.each([ + ['form', { name: 'probe_form', label: 'Form', type: 'form', target: 'probe_task.edit' }], + ['url', { name: 'probe_url', label: 'Url', type: 'url', target: '/environments' }], + ['api', { name: 'probe_api', label: 'Api', type: 'api', target: '/api/v1/x', method: 'POST' }], + ['script', { name: 'probe_script', label: 'Script', type: 'script', target: 'doThing' }], + ])('leaves an embedded `%s` action alone — only modal and flow targets are cross-referenced', (_type, action) => { + expect(refusals(embeddedStack(action, { flows }))).toEqual([]); + }); + + it('is vacuity-guarded: the ordinary merged shape a shipped stack produces still builds', () => { + // `objects[].actions[]` is overwhelmingly WRITTEN by the merge rather than + // by hand — a top-level action with `objectName` lands there on the way + // out of `defineStack`. Feeding that output back in must stay clean, or the + // corpus census in PR #7397 has gone stale. + const built = build({ + manifest: baseManifest, + objects, + pages, + flows, + actions: [ + { ...modalAction('probe_home'), objectName: 'probe_task' }, + { ...flowAction('probe_flow'), objectName: 'probe_task' }, + ], + }); + + expect(built.objects?.[0]?.actions?.map((a) => a.name)).toEqual(['probe_new_task', 'probe_run']); + expect(refusals(built)).toEqual([]); + }); +}); diff --git a/packages/spec/src/stack.zod.ts b/packages/spec/src/stack.zod.ts index 8c5ef29816..ae8ee8761b 100644 --- a/packages/spec/src/stack.zod.ts +++ b/packages/spec/src/stack.zod.ts @@ -1153,6 +1153,51 @@ function validateCrossReferences(config: ObjectStackDefinition): string[] { } } + // The SAME two target checks, one more traversal: actions authored directly + // on an object (`config.objects[].actions[]`) — #7397, the #6889 defect one + // authoring position over. + // + // Why the registered walk above cannot cover it: `validateCrossReferences` + // runs BEFORE `mergeActionsIntoObjects`, and that merge only ever copies + // top-level → object (by `objectName`), never the reverse. An action authored + // ONLY on the object therefore never appears in `config.actions` and reached + // no cross-reference check at all — measured on #7397, where an embedded + // `type: 'modal', target: 'probe_nowhere'` built clean while the identical + // action in the registered position was refused. + // + // This position carries the FULL `ActionSchema` (`object.zod.ts`'s + // `actions: z.array(ActionSchema)`) — the identical symbol the registered + // collection uses at `:256`, not a narrower embedded shape — so `name` is + // required here and `target` has already been canonicalized by the parse + // this function runs after. Only the SUBJECT of the message differs from the + // registered rule: an embedded action is located by its owning object. + // + // Scope of the modal branch is the same #6739 ruling the other two arms + // read: a `type: 'modal'` target names a PAGE, only. + // + // `objectName` is deliberately NOT checked here. The key exists on this + // shape (unlike the inline one), but what it should MEAN on an action + // already embedded on an object — a mere existence check, or a consistency + // check against the owning object's name — is an open contract question, + // filed separately rather than settled as a side effect of this walk. + if (config.objects) { + for (const obj of config.objects) { + for (const action of obj.actions ?? []) { + if (action.type === 'flow' && action.target && flowNames.size > 0 && !flowNames.has(action.target)) { + errors.push( + `Action '${action.name}' on object '${obj.name}' references flow '${action.target}' which is not defined in flows.`, + ); + } + + if (action.type === 'modal' && action.target && pageNames.size > 0 && !pageNames.has(action.target)) { + errors.push( + `Action '${action.name}' on object '${obj.name}' references page '${action.target}' (via modal target) which is not defined in pages.`, + ); + } + } + } + } + // The SAME two target checks, one more traversal: inline (page-element) // actions (#6889). Same rule, same message tail — only the subject differs, // because an inline action is located by page + path rather than by a