Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions .changeset/action-confirm-params-pair-guard.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
---
"@objectstack/spec": minor
---

feat(spec): an action may no longer pair `confirmText` with a non-empty `params` (#7428)

**Acceptance narrowing — this refuses metadata that parsed before.** An action
declaring `confirmText` beside a non-empty `params` array shows the user **two
sequential dialogs for one decision**: the console action runner awaits the
confirm, *then* the param prompt, so the first dialog already reads as "the
action ran" while nothing has been sent yet.

The maintainer's 2026-08-10 ruling on #7278 settled the shape: carry the confirm
question in the action's top-level `description` — which the param dialog renders
under its title — and drop `confirmText`. One condition, one wording, one dialog.

Two PRs repaired the sites that shipped this (#7592 for `plugin-approvals`,
#7827 for the fourteen in `platform-objects`). Repairing instances does not stop
the next one being written, which is what this refusal is for. It ships as a
**refusal rather than a warning** because the in-repo `ActionSchema` census is
now **0** — nothing legal breaks — and a warning that fires on every build of an
untouched project is a check nobody reads.

**Migrating.** Move the sentence, do not delete it:

```diff
defineAction({
name: 'ban_user',
label: 'Ban User',
- confirmText: 'Ban this user? They will be signed out until unbanned.',
+ description: 'Ban this user? They will be signed out until unbanned.',
params: [{ name: 'reason', label: 'Reason', type: 'textarea' }],
})
```

Not `ai.description` — that is the LLM-facing tool contract (≥40 chars, required
when `ai.exposed`), and putting the question there arms a tool description while
the dialog falls back to its generic line.

**What is deliberately NOT refused:**

- **`confirmText` on a param-LESS action** stays correct and untouched — there is
no second dialog to fold the question into, and stripping it would delete the
only warning the user ever sees.
- **`confirmText` beside an empty `params: []`** — nothing is collected, so no
second dialog opens.
- **A view's `bulkActionDefs`.** `BulkActionDefSchema` is a separate schema on
which the pair is *intended*: its params are inputs collected once before the
run, `confirmText` sits above the affected-record summary, and a `required`
param blocks that same dialog's Confirm button — one dialog, so there is
nothing to collapse. The guard lives on `ActionSchema`'s refinement chain and
is structurally incapable of reaching it; a pinning test asserts the bulk
pairing still parses, so a future widening of the guard goes red rather than
landing on correct declarations.
- **Requiring `description` whenever `params` is present.** Forbidding the pair
is the narrowest guard with measured pull behind it; the wider demand has no
measured failure behind it and would be its own decision.

`InlineActionSchema` is likewise unaffected — it picks fields from the shared
factory rather than deriving from this refinement chain, and it does not pick
`description`, so the remedy has no slot on that surface yet.
13 changes: 10 additions & 3 deletions content/docs/protocol/objectui/actions.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -376,7 +376,7 @@ successMessage: Customer deleted.
refreshAfter: true
```

- `confirmText` — message shown in a confirm dialog before the action runs.
- `confirmText` — message shown in a confirm dialog before the action runs. **Param-LESS actions only.** On a registered action, pairing it with a non-empty `params` is *refused* at authoring time (#7428): the runner chains confirmation *then* param collection, so the pair opens two dialogs for one decision and the first already reads as "the action ran" while nothing has been sent. When the action collects params, put the question on `description` instead — the param dialog renders it under its title, so the user gets one dialog with the question intact. Not `ai.description`, which is the LLM-facing tool contract. (A view's `bulkActionDefs` are a different surface, where the pair renders a single dialog and stays correct.)
- `successMessage` — toast shown after a successful run. When omitted the UI shows a generic "Action completed" toast, so set this for any action whose outcome isn't self-evident.
- `errorMessage` — toast shown when the action fails; overrides the raw server error with author-controlled copy.
- `undoable` — marks a single-record update action as offering an **Undo** affordance in the success toast, which restores the record's prior field values. The action runtime snapshots the record before the update and only builds the undo operation when this flag is set, so an action that omits it gets no Undo. Single-record updates only: there is nothing to snapshot when the action isn't scoped to one record.
Expand DownExpand Up@@ -480,7 +480,10 @@ ai:
type: api
method: POST
target: /api/v1/auth/admin/oauth2/toggle-disabled
confirmText: Disable this OAuth application? Existing integrations will stop working immediately.
# The confirm question rides `description`, not `confirmText`: this action
# collects params, and pairing the two keys is refused (#7428) because it
# would open two dialogs for one decision.
description: Disable this OAuth application? Existing integrations will stop working immediately.
params:
- { name: client_id, field: client_id, defaultFromRow: true, required: true }
```
Expand All@@ -497,7 +500,11 @@ ai:
type: api
method: POST
target: /api/v1/auth/sys-oauth-application/rotate-secret
confirmText: Rotate this application's client secret? The current secret stops working immediately.
# Three dialogs collapse to two, and the survivors are the two the user needs:
# ONE param dialog (question + `client_id`), then the post-run `resultDialog`
# that reveals the new secret. A post-run reveal is not a second pre-run
# decision, so it is not part of the pair `description` replaces (#7428).
description: Rotate this application's client secret? The current secret stops working immediately, and the new one is shown only once.
params:
- { name: client_id, field: client_id, defaultFromRow: true, required: true }
resultDialog:
Expand Down
4 changes: 2 additions & 2 deletions content/docs/references/ui/action.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -75,7 +75,7 @@ const result = ActionSchema.parse(data);
| **params** | `{ name?: string; field?: string; objectOverride?: string; label?: string \| Record<string, string>; … }[]` | 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) |
| **order** | `number` | optional | Sort order within a location group (lower = higher). Promotes/demotes an action toward the record_header primary button; stable, so actions without `order` keep their registration order. |
| **confirmText** | `string \| Record<string, string>` | optional | Confirmation message before execution |
| **confirmText** | `string \| Record<string, string>` | optional | Confirmation message before execution. On a registered action, pairing this with a non-empty `params` is refused (#7428) — that opens a second dialog for one decision; put the question on `description` instead. Correct on a param-LESS action, where the confirm is the only dialog there is. |
| **successMessage** | `string \| Record<string, string>` | optional | Success message to show after execution |
| **errorMessage** | `string \| Record<string, string>` | optional | Error message to show when the action fails (overrides the raw error). |
| **refreshAfter** | `boolean` | optional | Refresh view after execution |
Expand DownExpand Up@@ -245,7 +245,7 @@ const result = ActionSchema.parse(data);
| **method** | `Enum<'POST' \| 'PATCH' \| 'PUT' \| 'DELETE'>` | optional | HTTP method for type:"api" actions. Defaults to POST. |
| **params** | `{ name?: string; field?: string; objectOverride?: string; label?: string \| Record<string, string>; … }[]` | optional | Input parameters required from user — an ActionParam[] DEFINITION array, never a payload map (a static request body goes in `bodyExtra`). |
| **bodyExtra** | `Record<string, any>` | optional | Static request-body fields for a type:"api" action, merged last (overrides user params). `{{page.<var>}}` tokens are resolved by the runtime. This — not `params` — is where a payload goes. |
| **confirmText** | `string \| Record<string, string>` | optional | Confirmation message before execution |
| **confirmText** | `string \| Record<string, string>` | optional | Confirmation message before execution. On a registered action, pairing this with a non-empty `params` is refused (#7428) — that opens a second dialog for one decision; put the question on `description` instead. Correct on a param-LESS action, where the confirm is the only dialog there is. |
| **successMessage** | `string \| Record<string, string>` | optional | Success message to show after execution |
| **errorMessage** | `string \| Record<string, string>` | optional | Error message to show when the action fails (overrides the raw error). |
| **refreshAfter** | `boolean` | optional | Refresh view after execution |
Expand Down
238 changes: 238 additions & 0 deletions packages/spec/src/ui/action-confirm-params-guard.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* #7428 — the authoring-time guard on the `confirmText` + `params` PAIR.
*
* #7278 and #7309 repaired the 16 shipped sites that opened two dialogs for one
* decision (PRs #7592 and #7827). Repairing instances does not stop the next one
* being written; this refusal is the structural half, and it is the reason the
* card exists as a separate issue from either migration.
*
* **What these tests pin is the BOUNDARY, not just the refusal.** The pair is
* wrong on `ActionSchema` and CORRECT on `BulkActionDefSchema` — measured on
* #7428 (2026-08-11), and the distinction is the schema the def is validated by,
* not a heuristic about whether the params happen to be optional. A guard
* written against the raw `confirmText` + `params: [` key pair would land red on
* four correct `examples/app-showcase` bulk defs on day one, which is the
* permanently-noisy-check shape the card was filed to avoid. So the acceptance
* direction is pinned as hard as the rejection direction: a future "helpful"
* widening of this guard onto the bulk surface goes RED here.
*
* The rejection tests assert the issue PATH and the MESSAGE SUBSTANCE rather
* than a bare `success === false`. One condition, one wording: a refusal whose
* message does not name both keys and point at the remedy sends the author
* looking for a different bug.
*/

import { describe, expect, it } from 'vitest';
import { ActionSchema, InlineActionSchema, defineAction } from './action.zod';
import { BulkActionDefSchema } from './bulk-action.zod';

/**
* Minimum legal registered action. `type` defaults to `script`, whose own
* refinement requires an inline `body` or a `target` naming a bundle function —
* leaving both off would fail for a reason that has nothing to do with this
* guard and would make every assertion below unreadable.
*/
const base = { name: 'approval_reject', label: 'Reject', target: 'rejectApproval' } as const;

// `type` is narrowed rather than widened to `string` so this literal is also
// assignable to `defineAction`'s typed input, which the last test below calls.
const oneParam = [{ name: 'reason', label: 'Reason', type: 'textarea' as const, required: true }];

/** The single issue this guard raises, or `undefined` if it did not fire. */
const guardIssue = (result: ReturnType<typeof ActionSchema.safeParse>) =>
result.success
? undefined
: result.error.issues.find((i) => i.path.join('.') === 'confirmText');

describe('#7428 — `confirmText` + non-empty `params` is refused on ActionSchema', () => {
it('refuses the pair, at the `confirmText` path', () => {
const result = ActionSchema.safeParse({
...base,
confirmText: 'Reject this request?',
params: oneParam,
});

expect(result.success).toBe(false);
// The path is asserted because it is what an editor/CLI underlines. Pointing
// at `params` would tell the author to delete the inputs they need; the key
// that has to go is `confirmText`.
expect(guardIssue(result)?.path).toEqual(['confirmText']);
});

it('says WHY, naming both keys and the remedy — not a bare rejection', () => {
const result = ActionSchema.safeParse({
...base,
confirmText: 'Reject this request?',
params: oneParam,
});
const message = guardIssue(result)?.message ?? '';

// Both halves of the offending pair, so the author can see what collided.
expect(message).toContain('`confirmText`');
expect(message).toContain('`params`');
// The consequence, in user-visible terms rather than schema terms.
expect(message).toContain('TWO dialogs');
// The remedy, which is the whole point of the #7278 ruling.
expect(message).toContain('`description`');
// …and the remedy's own trap: the LLM-facing key one level down is NOT it.
expect(message).toContain('ai.description');
// The exception that keeps `confirmText` a live key rather than a retired
// one — an author who reads only this message must not conclude otherwise.
expect(message).toContain('param-LESS');
});

it('fires on the localized-map form of `confirmText` too, not just a string', () => {
// `confirmText` is `I18nLabelSchema`, so a bare truthiness check written
// against a string would miss the map form and let the defect back in
// through the localized door.
const result = ActionSchema.safeParse({
...base,
confirmText: { en: 'Reject this request?', 'zh-CN': '拒绝该请求?' },
params: oneParam,
});

expect(result.success).toBe(false);
expect(guardIssue(result)?.path).toEqual(['confirmText']);
});

it('cannot be smuggled in through an ALIAS spelling — that door is shut upstream', () => {
// `confirm` → `confirmText` and `inputs` → `params` are declared aliases on
// this surface, and this repo REJECTS a near-miss with a rename arrow rather
// than folding it silently (Prime Directive #12 — one contract, no dialects).
// So the aliased pair never reaches this refinement at all: it is refused one
// layer earlier, by key recognition. Pinned because the guard's coverage claim
// depends on it — if aliases ever became a silent fold, the pair would arrive
// post-fold and this test is where that change gets noticed.
const result = ActionSchema.safeParse({
...base,
confirm: 'Reject this request?',
inputs: oneParam,
});

expect(result.success).toBe(false);
const issue = result.success ? undefined : result.error.issues[0];
expect(issue?.code).toBe('unrecognized_keys');
expect(issue?.message).toContain('Did you mean `confirm` → `confirmText`');
expect(issue?.message).toContain('`inputs` → `params`');
});

it('throws from `defineAction`, which is where an author meets it', () => {
// A refusal is only worth having if it reaches the authoring call site —
// `defineAction` is what the platform objects and every app actually call.
expect(() =>
defineAction({ ...base, confirmText: 'Reject this request?', params: oneParam }),
).toThrow(/TWO dialogs/);
});
});

describe('#7428 — what the guard must NOT touch', () => {
it('accepts `confirmText` on a param-LESS action — the confirm is the only dialog', () => {
const result = ActionSchema.safeParse({
...base,
confirmText: 'Reject this request?',
});

expect(result.success).toBe(true);
});

it('accepts `confirmText` beside an EMPTY `params` array', () => {
// An empty array collects nothing, so no second dialog opens. Refusing it
// would be a refusal with no user-visible defect behind it.
const result = ActionSchema.safeParse({ ...base, confirmText: 'Sure?', params: [] });

expect(result.success).toBe(true);
});

it('accepts `params` + `description` — the shape #7278 migrated TO', () => {
// If this ever goes red the guard has swallowed its own remedy and the two
// migrations have nowhere to land.
const result = ActionSchema.safeParse({
...base,
description: 'Reject this request? Say why — the requester sees it.',
params: oneParam,
});

expect(result.success).toBe(true);
});

it('does NOT additionally require `description` when `params` is present', () => {
// Deliberately not widened (#7428 ruling 3): forbidding the pair is the
// narrowest guard with measured pull behind it. Requiring dialog copy on
// every param-collecting action is a strictly bigger authoring demand with
// no measured failure behind it — it would need its own card.
const result = ActionSchema.safeParse({ ...base, params: oneParam });

expect(result.success).toBe(true);
});
});

describe('#7428 — the guard is scoped to ActionSchema by SCHEMA BOUNDARY', () => {
it('BulkActionDefSchema still ACCEPTS `confirmText` + non-empty `params`', () => {
// The pinning test the boundary ruling asks for. This pairing is INTENDED
// on the bulk surface: per that schema's own describe() text the params are
// "inputs collected once before the run", `confirmText` is shown "above the
// affected-record summary", and a `required` param "blocks the Confirm
// button until a value is present" — one dialog, so there is no second one
// to collapse. `examples/app-showcase`'s four defs are this shape and are
// correct as written. A widening of the guard onto this schema lands here.
const result = BulkActionDefSchema.safeParse({
name: 'set_labels',
label: 'Set Labels',
operation: 'update',
confirmText: 'Set these labels on every selected project?',
params: [
{
name: 'labels',
label: 'Labels',
type: 'select',
multiple: true,
required: true,
options: [{ label: 'Frontend', value: 'frontend' }],
},
],
});

expect(result.success).toBe(true);
expect(result.success && result.data.confirmText)
.toBe('Set these labels on every selected project?');
expect(result.success && result.data.params?.length).toBe(1);
});

it('the two schemas are independent — the bulk def is not validated by ActionSchema', () => {
// The structural claim behind the pin above, asserted rather than assumed:
// `BulkActionDefSchema` is its own `strictObject`, so the same literal is
// not even a legal ACTION (no `operation` key on that surface). If the two
// were ever unified, this goes red before the guard silently widens.
const asAction = ActionSchema.safeParse({
name: 'set_labels',
label: 'Set Labels',
operation: 'update',
confirmText: 'Set these labels on every selected project?',
params: [{ name: 'labels', label: 'Labels', type: 'select' }],
});

expect(asAction.success).toBe(false);
});

it('InlineActionSchema is out of reach too — it picks fields, not this refine chain', () => {
// Recording the guard's real blast radius rather than assuming it. Inline
// actions derive from the shared field factory via `.pick()`, so no
// refinement on `ActionSchema` reaches them — and the pick deliberately
// omits `description`, so the #7278 remedy has no slot on that surface to
// move a question into. Same reason the bulk defs were struck from the
// target set: a refusal whose remedy is unreachable is a dead end, not a
// guard. Whether the inline surface should gain `description` FIRST and the
// guard SECOND is left open on #7428 rather than presumed here.
const result = InlineActionSchema.safeParse({
type: 'url',
target: '/approvals?reject=1',
label: 'Reject',
confirmText: 'Reject this request?',
params: [{ name: 'reason', label: 'Reason', type: 'textarea' }],
});

expect(result.success).toBe(true);
});
});
Loading
Loading