diff --git a/.changeset/5934-retire-onsuccess-callback-channel.md b/.changeset/5934-retire-onsuccess-callback-channel.md new file mode 100644 index 0000000000..d358562f7e --- /dev/null +++ b/.changeset/5934-retire-onsuccess-callback-channel.md @@ -0,0 +1,35 @@ +--- +'@object-ui/core': minor +'@object-ui/types': minor +'@object-ui/components': patch +--- + +BREAKING (`@object-ui/core`): `ActionRunner`'s legacy `ActionDef.onSuccess` +chained-callback channel is retired — `onSuccess` now has exactly the meaning the +contract declares (objectui#5934, maintainer ruling 2026-08-31). + +(The bump is `minor` by this repo's release model — objectui's major is pinned to +the `@objectstack` family major, and its own breaking changes ship as `minor` with +the break spelled out here, per `scripts/check-changeset-no-major.mjs`. This +paragraph is that spelling-out: the break below is real and consumer-visible.) + +- **What breaks, by specifier**: `import type { ActionDef } from '@object-ui/core'` — + `ActionDef['onSuccess']` was `ActionDef | ActionDef[]` (chained callbacks the runner + dispatched through `executeChain` after a success). It is now derived from the pinned + spec: `ActionSchema.onSuccess`'s closed strict `{ navigate: string, openIn?: 'self' | + 'newTab' }` block. Code that assigned a callback `ActionDef` (or an array of them) to + `onSuccess` no longer compiles, and at runtime a callback-shaped value gets NO reading — + no handler dispatch, no navigation, the action's own result untouched. `onFailure` is NOT + changed: the spec declares no such key, so it keeps its one runner-native meaning. +- **Why this is safe to take**: the channel was unreachable from validated metadata — + `@objectstack/spec` (17.2.0 pin) strict-refuses a callback shape inside `onSuccess` at + parse (`invalid_type` on `navigate` + `unrecognized_keys`), so no published/saved + metadata could ever carry one — and a producer census with a positive control found zero + producers outside the channel's own test pins. Migration for an out-of-repo consumer that + drove the channel programmatically: put the follow-up actions in `chain` (the runner's + declared chaining key, unchanged), or author the spec's `onSuccess` navigation block. +- `@object-ui/types` (minor): `UIActionSchema` now declares `onSuccess`, derived from the + spec's `ActionSchema.onSuccess` — the renderer view spells the key the four action + surfaces forward, so the forwards type-check. +- `@object-ui/components` (patch): the four action renderers forward `onSuccess` without + the `as any` casts (no behavior change — same key, same value, now typed). diff --git a/packages/app-shell/src/utils/__tests__/consoleServerAction.test.tsx b/packages/app-shell/src/utils/__tests__/consoleServerAction.test.tsx index cf4d4f184c..ac8e59f967 100644 --- a/packages/app-shell/src/utils/__tests__/consoleServerAction.test.tsx +++ b/packages/app-shell/src/utils/__tests__/consoleServerAction.test.tsx @@ -399,8 +399,9 @@ describe('a declared onSuccess block defers to the runner (objectui#5221)', () = }); it('a legacy chained-callback onSuccess is NOT mistaken for a declared hop', async () => { - // `{ type: 'notify' }` is the runner's older `ActionDef` callback channel, - // not the spec block. The redirectUrl convention must still run. + // `{ type: 'notify' }` was the runner's older `ActionDef` callback channel + // (retired by objectui#5934), not the spec block — an unparsed row can + // still carry the shape. The redirectUrl convention must still run. const openSpy = vi.spyOn(window, 'open').mockReturnValue(makeTab() as any); const navigate = vi.fn(); const { handler } = makeHandler({ diff --git a/packages/components/src/renderers/action/action-button.tsx b/packages/components/src/renderers/action/action-button.tsx index 454e277ff1..13b58810ff 100644 --- a/packages/components/src/renderers/action/action-button.tsx +++ b/packages/components/src/renderers/action/action-button.tsx @@ -198,9 +198,13 @@ const ActionButtonRenderer = forwardRef< // the app's own `navigationHandler`). Dropped here, the action // succeeded and the declared hop silently never happened — // objectui#5493, the same shape as `bodyShape` / `resultDialog` - // above. Cast because the key is spec-owned and not spelled on - // `@object-ui/types`' renderer view, exactly as `resultDialog` is. - onSuccess: (schema as any).onSuccess, + // above. Uncast since objectui#5934 retired the runner's legacy + // chained-callback meaning: both ends now derive the spec block + // (`UIActionSchema.onSuccess` on the read side, + // `ActionDef.onSuccess` on the write side), so the forward + // type-checks against the one declared meaning instead of hiding + // behind `as any`. + onSuccess: schema.onSuccess, }; await execute({ ...forwarded, ...localContext }); diff --git a/packages/components/src/renderers/action/action-group.tsx b/packages/components/src/renderers/action/action-group.tsx index e272a16655..01e6dfbc2a 100644 --- a/packages/components/src/renderers/action/action-group.tsx +++ b/packages/components/src/renderers/action/action-group.tsx @@ -276,7 +276,8 @@ const ActionGroupRenderer = forwardRef' }` as an action, and failed inside - // `executeNavigation` with "No URL provided for navigation action" — the - // author got a red toast and no hop. + // `readOnSuccessNavigation` stays as the shape guard, not as a + // discriminator: stored rows are rehydrated UNPARSED (#3903), so the value + // is still read as data, and a shape the spec refuses gets no reading — + // no navigation, no callback dispatch, no lenient fallback. if (result.success && action.onSuccess) { const navigation = readOnSuccessNavigation(action.onSuccess); if (navigation) { this.navigateOnSuccess(navigation, action, result); - } else { - const callbacks = Array.isArray(action.onSuccess) ? action.onSuccess : [action.onSuccess]; - await this.executeChain(callbacks, 'sequential'); } } if (!result.success && action.onFailure) { @@ -1999,15 +2012,14 @@ export interface OnSuccessNavigation { } /** - * Is this `onSuccess` the SPEC's navigation block, or this runner's older - * chained-callback channel (`ActionDef | ActionDef[]`)? + * Is this `onSuccess` the spec's navigation block? * * The test IS the spec's declaration: a non-array object carrying a STRING - * `navigate`. Nothing else can produce that shape — the spec object is strict - * with `navigate: z.string()` required, and on a callback `ActionDef`, - * `navigate` is the deprecated nested navigation ENVELOPE that - * `executeNavigation` reads `to`/`target`/`redirect` off, so a bare string - * there has never been runnable. + * `navigate`. Stored rows are rehydrated UNPARSED (#3903), so the runner reads + * the value as data and anything else gets NO reading — since objectui#5934 + * retired the legacy chained-callback channel (`ActionDef | ActionDef[]`), + * there is no other channel for an off-contract shape to fall into. This is a + * shape GUARD on unparsed data, not a discriminator between two meanings. */ export function readOnSuccessNavigation(value: unknown): OnSuccessNavigation | null { if (!value || typeof value !== 'object' || Array.isArray(value)) return null; diff --git a/packages/core/src/actions/__tests__/ActionRunner.onSuccessNavigation.test.ts b/packages/core/src/actions/__tests__/ActionRunner.onSuccessNavigation.test.ts index e62d47e15f..1e14f8031b 100644 --- a/packages/core/src/actions/__tests__/ActionRunner.onSuccessNavigation.test.ts +++ b/packages/core/src/actions/__tests__/ActionRunner.onSuccessNavigation.test.ts @@ -253,23 +253,30 @@ describe('ActionSchema.onSuccess — the two openIn spellings stay apart', () => }); }); -describe('ActionSchema.onSuccess — the legacy chained-callback channel is untouched', () => { - it('still runs an ActionDef callback, and does not treat it as navigation', async () => { - // `ActionDef.onSuccess?: ActionDef | ActionDef[]` predates the spec key and - // is a RUNTIME channel: `@objectstack/spec` strict-refuses `{ type: … }` - // inside `onSuccess`, so no validated metadata can reach it. Retiring it is - // its own card; this pins that implementing the spec key did not silently - // take it away. +describe('ActionSchema.onSuccess — the retired chained-callback channel gets no reading', () => { + it('neither dispatches a callback-shaped onSuccess nor treats it as navigation', async () => { + // `ActionDef.onSuccess?: ActionDef | ActionDef[]` predated the spec key as + // the runner's own chained-callback channel. objectui#5934 (maintainer + // ruling 2026-08-31) retired it: the spec strict-refuses `{ type: … }` + // inside `onSuccess` at parse, so no validated metadata could ever reach + // it, and the census found zero producers outside the channel's own pins. + // Stored rows rehydrate UNPARSED (#3903), so this pins the RUNTIME half of + // the retirement — the shape still reaches the runner as data, and gets NO + // reading: no handler dispatch, no navigation, and the action's own result + // is untouched. (`as never` is the test reaching around the compile-time + // half: the declared type now derives the spec block and refuses this + // shape at the authoring site.) const { runner, nav } = makeRunner({ id: 'rec_42' }); const cb = vi.fn(async () => ({ success: true })); runner.registerHandler('notify', cb as never); - await runner.execute({ + const result = await runner.execute({ type: 'api', name: 'clone_record', target: '/api/v1/records/clone', onSuccess: { type: 'notify', name: 'ping' }, } as never); - expect(cb).toHaveBeenCalledTimes(1); + expect(result.success).toBe(true); + expect(cb).not.toHaveBeenCalled(); expect(nav).not.toHaveBeenCalled(); }); }); diff --git a/packages/core/src/actions/__tests__/ActionRunner.test.ts b/packages/core/src/actions/__tests__/ActionRunner.test.ts index 387707bb85..417c8697e9 100644 --- a/packages/core/src/actions/__tests__/ActionRunner.test.ts +++ b/packages/core/src/actions/__tests__/ActionRunner.test.ts @@ -1019,17 +1019,29 @@ describe('ActionRunner', () => { // ========================================================================== describe('callbacks', () => { - it('should execute onSuccess callback after success', async () => { + // The `onSuccess` chained-callback channel (`ActionDef | ActionDef[]`) was + // retired by objectui#5934 (maintainer ruling 2026-08-31): the spec + // strict-refuses a callback shape inside `onSuccess` at parse, and the + // census found zero producers outside this file's own pins. The two tests + // that used to pin the channel now pin its ABSENCE — stored rows rehydrate + // UNPARSED (#3903), so the shapes still reach the runner as data, and must + // get no reading. `onFailure` is untouched: the spec declares no such key, + // so it keeps its one runner-native meaning. + it('a callback-shaped onSuccess is not dispatched — the channel is retired', async () => { const successHandler = vi.fn().mockResolvedValue({ success: true }); runner.registerHandler('notify', successHandler); - await runner.execute({ + const result = await runner.execute({ onClick: vi.fn(), + // `as never`: since #5934 the declared type derives the spec's + // `{ navigate, openIn }` block, so the compiler refuses this shape at + // the authoring site — the cast reaches around it to pin the runtime. onSuccess: { type: 'notify', params: { msg: 'ok' } }, toast: { showOnSuccess: false }, - }); + } as never); - expect(successHandler).toHaveBeenCalledOnce(); + expect(result.success).toBe(true); + expect(successHandler).not.toHaveBeenCalled(); }); it('should execute onFailure callback after failure', async () => { @@ -1045,20 +1057,21 @@ describe('ActionRunner', () => { expect(failureHandler).toHaveBeenCalledOnce(); }); - it('should support array of onSuccess callbacks', async () => { + it('an array of callback-shaped onSuccess entries is not dispatched either', async () => { const h1 = vi.fn().mockResolvedValue({ success: true }); const h2 = vi.fn().mockResolvedValue({ success: true }); runner.registerHandler('cb1', h1); runner.registerHandler('cb2', h2); - await runner.execute({ + const result = await runner.execute({ onClick: vi.fn(), onSuccess: [{ type: 'cb1' }, { type: 'cb2' }], toast: { showOnSuccess: false }, - }); + } as never); - expect(h1).toHaveBeenCalledOnce(); - expect(h2).toHaveBeenCalledOnce(); + expect(result.success).toBe(true); + expect(h1).not.toHaveBeenCalled(); + expect(h2).not.toHaveBeenCalled(); }); }); diff --git a/packages/core/src/actions/actionKeys.ts b/packages/core/src/actions/actionKeys.ts index dfb77c8938..c61c239ad8 100644 --- a/packages/core/src/actions/actionKeys.ts +++ b/packages/core/src/actions/actionKeys.ts @@ -75,10 +75,12 @@ * rejection" into a compile error at no cost. Hand-copying would have quietly * re-legitimized two dead keys — which is why the types are derived. * - * 17 keys `ActionDef` declares that the spec does not own — `actionType`, `api`, + * 16 keys `ActionDef` declares that the spec does not own — `actionType`, `api`, * `chain`, `chainMode`, `close`, `condition`, `confirm`, `endpoint`, `modal`, - * `navigate`, `onClick`, `onFailure`, `onSuccess`, `redirect`, `reload`, `toast`, - * `actionParams`. Step 2 marked `@deprecated`, with the spec spelling to use + * `navigate`, `onClick`, `onFailure`, `redirect`, `reload`, `toast`, + * `actionParams`. (`onSuccess` was the 17th until objectui#5934 retired the + * runner's chained-callback meaning; the key is now spec-owned and derived, + * like the 18 below.) Step 2 marked `@deprecated`, with the spec spelling to use * instead, ONLY the four the runner itself proves are aliases: `actionType` (→ * `type`), `api` and `endpoint` (→ `target`; `executeAPI` resolves * `api || endpoint || target`), and `navigate` (→ flat `target`/`openIn`; @@ -156,7 +158,6 @@ export const ACTION_DEF_KEYS = [ 'modal', 'chain', 'chainMode', - 'onSuccess', 'onFailure', 'opensInNewTab', 'newTabUrl', @@ -181,6 +182,10 @@ export const ACTION_DEF_KEYS = [ 'recordIdField', 'recordIdParam', 'requiresFeature', + // Moved from the runner-native cluster above by objectui#5934: the legacy + // chained-callback meaning is retired and the key's type now derives the + // spec's `{ navigate, openIn }` block. + 'onSuccess', 'shortcut', 'bulkEnabled', ] as const; @@ -240,12 +245,9 @@ export const SPEC_ACTION_KEYS = [ 'newTabUrl', 'objectName', // Declared by `ActionSchema` as of @objectstack/spec 17.1.0 (objectui#5328). - // Listing it here is a DIAGNOSTIC statement only — `KNOWN_ACTION_KEYS` feeds - // `warnOnUnknownActionKeys`, so without this row an author writing the key the - // spec now accepts would be warned it is unknown. It says nothing about the - // key being forwarded: the four declared action surfaces still drop it before - // the runner, tracked as KNOWN_GAPS in check-action-forward-parity.mjs and - // filed as objectui#5493. + // All four declared action surfaces forward it since objectui#5493/#6304, and + // `ActionDef` derives its type from the spec since objectui#5934 retired the + // runner's legacy chained-callback meaning for the same key. 'onSuccess', 'openIn', 'opensInNewTab', diff --git a/packages/types/src/ui-action.ts b/packages/types/src/ui-action.ts index edf950b30b..c3a7e4c4ed 100644 --- a/packages/types/src/ui-action.ts +++ b/packages/types/src/ui-action.ts @@ -451,6 +451,26 @@ export interface UIActionSchema { */ openIn?: 'self' | 'new-tab'; + /** + * Declared post-success navigation — the spec's closed strict + * `{ navigate, openIn }` block (`ActionSchema.onSuccess`, authorable since + * `@objectstack/spec` 17.1.0). All four declared action renderers forward it + * to the runner (objectui#5493/#6304), which performs the hop through the + * app's own `navigationHandler`. + * + * DERIVED from the spec, never hand-copied — a hand-written duplicate of a + * spec shape is a second contract that drifts silently. Declared on the + * renderer view since objectui#5934 retired `ActionRunner`'s legacy + * chained-callback meaning for the same key: with the spec block as the + * key's only meaning, the forward sites type-check without an `as any` cast. + * + * Note the inner `openIn` spelling is `'self' | 'newTab'` — NOT the + * top-level {@link openIn}'s `'self' | 'new-tab'`. The spec refuses each + * crossover spelling; the derivation keeps the two from ever being merged + * by hand. + */ + onSuccess?: SpecAction['onSuccess']; + /** API endpoint (for type: 'api') */ endpoint?: string;