From 27b93089f1c0783b2114db2f837a0cd6bd168d41 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 17:11:13 +0000 Subject: [PATCH 1/2] fix(app-shell): editing an action in Studio no longer stops the object from saving The action inspector writes the spec's nested `ai` block instead of the flat `aiExposed` / `aiDescription` it refuses, clears the keys a Type switch makes refused, and edits Body Shape as Flat or Wrap(key). The Data pillar holds an action that is only unfinished, and places a refused one on the action, with "Show me" opening it on the Actions view. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QsR2jSHgVcT66bTgBM8Yq3 --- .../26728-26736-action-edits-keep-saving.md | 19 ++ ...ionDefaultInspector.aiBlock-26728.test.tsx | 160 ++++++++++++ .../inspectors/ActionDefaultInspector.tsx | 154 +++++++++++- ...DefaultInspector.typeSwitch-26736.test.tsx | 204 +++++++++++++++ ...ctorStrings.hookAction.i18n-10586.test.tsx | 3 +- .../metadata-admin/previews/ActionPreview.tsx | 5 +- .../automationPreviews.i18n-10862.test.tsx | 4 +- .../DataPillar.actionRefusal-26736.test.tsx | 236 ++++++++++++++++++ .../studio-design/ObjectActionsPanel.tsx | 16 +- .../studio-design/StudioDesignSurface.tsx | 20 +- .../metadataError.actionHeld-26736.test.ts | 130 ++++++++++ .../src/views/studio-design/metadataError.ts | 128 +++++++++- ui/packages/i18n/src/namespaces/studio/en.ts | 10 + ui/packages/i18n/src/namespaces/studio/zh.ts | 10 + 14 files changed, 1074 insertions(+), 25 deletions(-) create mode 100644 .changeset/26728-26736-action-edits-keep-saving.md create mode 100644 ui/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.aiBlock-26728.test.tsx create mode 100644 ui/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.typeSwitch-26736.test.tsx create mode 100644 ui/packages/app-shell/src/views/studio-design/DataPillar.actionRefusal-26736.test.tsx create mode 100644 ui/packages/app-shell/src/views/studio-design/metadataError.actionHeld-26736.test.ts diff --git a/.changeset/26728-26736-action-edits-keep-saving.md b/.changeset/26728-26736-action-edits-keep-saving.md new file mode 100644 index 00000000000..f3291baa947 --- /dev/null +++ b/.changeset/26728-26736-action-edits-keep-saving.md @@ -0,0 +1,19 @@ +--- +'@object-ui/app-shell': patch +'@object-ui/i18n': patch +--- + +fix(app-shell): editing an action in Studio no longer stops the object from saving + +Clause-②: no +Metadata-protocol: none + +Three edits in Studio's action editor (Data › an object › Actions) wrote an action the spec refuses, and from then on every save of the object was refused until a reload, with nothing on screen naming the action: + +- **Expose to AI agents** wrote a flat `aiExposed` (and its description a flat `aiDescription`). It now writes the spec's nested block, `ai: { exposed, description }`, and reads it back; unticking leaves a draft the spec accepts. A draft that still carries the flat keys is cleaned by the control's next write. The action preview's AI pill reads the same block. +- **Switching the Type** kept keys the new type refuses (a script `body` on an API action, `onSuccess`, `outcomeMessages`, `undoable: true`). The switch now clears exactly those, and keeps everything the new type accepts. +- **Body Shape** was a free-text box, so the documented `{ "wrap": "payload" }` was written as a string the spec refuses. It is now a Flat / Wrap control with a key input; a stored `{ wrap }` reads back as Wrap with its key, and Wrap is written only once it has a key. + +The Data pillar now holds an action that is only unfinished, as it already held a field with no options: after "Run a script" before a source is typed, a Type switch before its endpoint, address or target is filled, or "Expose to AI agents" before a long enough description, no save goes out, and a neutral line names the input on the action until it is filled. A save the server still refuses is shown at the action: the strip names it (and the input, when the editor has one for that key), and "Show me" opens it on the Actions view. The next accepted save clears the strip, with no reload. + +New `studio` namespace strings (en, zh): the Body Shape control, and the refusal and held lines for an action. diff --git a/ui/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.aiBlock-26728.test.tsx b/ui/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.aiBlock-26728.test.tsx new file mode 100644 index 00000000000..1dcecbaf2e5 --- /dev/null +++ b/ui/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.aiBlock-26728.test.tsx @@ -0,0 +1,160 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack#26728 — "Expose to AI agents" writes the spec's nested `ai` + * block, and unticking leaves a draft the spec accepts. + * + * The control used to write a flat `aiExposed` (and its description a flat + * `aiDescription`). The spec's `ActionSchema` refuses both by name, so one tick + * refused every later save of the object, and unticking wrote `aiExposed: + * false`, still refused. The pins drive the real inspector inside a host that + * merges each patch shallowly (as `ObjectActionsPanel` applies `onPatch`), send + * the held draft through a JSON round trip (the body the save sends), and parse + * it with the spec's own `ActionSchema`: + * + * - a tick writes `ai.exposed`, and the spec's only refusal is the + * description it requires beside it — the input the inspector opens; + * - a description of the spec's minimum length parses, and reads back; + * - unticking parses, with or without a description typed; + * - an older draft carrying the flat keys parses after the control's next + * write, which clears them; + * - the other `ai` keys (edited under "More fields") are kept. + * + * Parity: `AI_DESCRIPTION_MIN` is the spec's own minimum, measured both sides. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, fireEvent } from '@testing-library/react'; +import { ActionSchema } from '@objectstack/spec/ui'; + +// objectui#4697: the inspector reads the shared client on mount; stub it so +// nothing reaches the network, as the sibling inspector suites do. +const state = vi.hoisted(() => ({ + metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]), withPreviewDrafts() { return this; } }, +})); +vi.mock('../useMetadata', () => ({ useMetadataClient: () => state.metadataClient })); + +import { ActionDefaultInspector, AI_DESCRIPTION_MIN } from './ActionDefaultInspector'; + +afterEach(cleanup); + +type Draft = Record; + +const SCRIPT_ACTION: Draft = { + name: 'invoice_send', + label: 'Send', + objectName: 'invoice', + locations: ['record_more'], + type: 'script', + body: { language: 'expression', source: 'true' }, +}; + +const DESCRIPTION = 'Send the invoice to its customer once it has been approved by finance.'; + +function mountHost(initial: Draft) { + const seen: { draft: Draft; writes: Draft[] } = { draft: initial, writes: [] }; + function Host() { + const [draft, setDraft] = React.useState(initial); + seen.draft = draft; + return ( + { + seen.writes.push(patch); + setDraft((d) => ({ ...d, ...patch })); + }} + /> + ); + } + render(); + return seen; +} + +const sent = (draft: Draft): Draft => JSON.parse(JSON.stringify(draft)); + +function refusals(draft: Draft): string[] { + const parsed = ActionSchema.safeParse(sent(draft)); + return parsed.success ? [] : parsed.error.issues.map((i) => `${i.path.join('.')}:${i.code}`); +} + +const exposeBox = () => screen.getByRole('checkbox', { name: 'Expose to AI agents' }); +const typeDescription = (text: string) => + fireEvent.change(screen.getByPlaceholderText('When and why an agent should call this action…'), { target: { value: text } }); + +describe('aiBlockPin — "Expose to AI agents" writes the nested ai block (objectstack#26728)', () => { + it('a tick writes ai.exposed, never a flat key; the only refusal is the description it requires', () => { + const seen = mountHost(SCRIPT_ACTION); + expect(refusals(seen.draft)).toEqual([]); + + fireEvent.click(exposeBox()); + expect(seen.writes.at(-1)).toStrictEqual({ ai: { exposed: true } }); + expect(sent(seen.draft)).not.toHaveProperty('aiExposed'); + // Before the fix: `:unrecognized_keys` at the action, on every later save. + expect(refusals(seen.draft)).toEqual(['ai.description:custom']); + expect(exposeBox()).toBeChecked(); + }); + + it('a description of the spec minimum parses, and reads back into the box', () => { + const seen = mountHost(SCRIPT_ACTION); + fireEvent.click(exposeBox()); + typeDescription('short'); + expect(refusals(seen.draft)).toEqual(['ai.description:too_small']); + + typeDescription(DESCRIPTION); + expect(sent(seen.draft).ai).toStrictEqual({ exposed: true, description: DESCRIPTION }); + expect(refusals(seen.draft)).toEqual([]); + expect(screen.getByPlaceholderText('When and why an agent should call this action…')).toHaveValue(DESCRIPTION); + }); + + it('unticking parses: a finished description is kept, one the spec would refuse goes with the box', () => { + const seen = mountHost(SCRIPT_ACTION); + fireEvent.click(exposeBox()); + typeDescription(DESCRIPTION); + fireEvent.click(exposeBox()); + expect(sent(seen.draft).ai).toStrictEqual({ exposed: false, description: DESCRIPTION }); + expect(refusals(seen.draft)).toEqual([]); + + fireEvent.click(exposeBox()); + typeDescription('short'); + fireEvent.click(exposeBox()); + expect(sent(seen.draft).ai).toStrictEqual({ exposed: false }); + expect(refusals(seen.draft)).toEqual([]); + }); + + it('an older draft carrying the flat keys parses after the control\'s next write', () => { + // The shape the control wrote before this fix: refused at the action. + const seen = mountHost({ ...SCRIPT_ACTION, aiExposed: true, aiDescription: 'x' }); + expect(refusals(seen.draft)).toEqual([':unrecognized_keys']); + // The flat keys are never read: the box is unticked. + expect(exposeBox()).not.toBeChecked(); + + fireEvent.click(exposeBox()); + fireEvent.click(exposeBox()); + expect(sent(seen.draft)).not.toHaveProperty('aiExposed'); + expect(sent(seen.draft)).not.toHaveProperty('aiDescription'); + expect(refusals(seen.draft)).toEqual([]); + }); + + it('control: the other ai keys are kept through a tick and an untick', () => { + const seen = mountHost({ ...SCRIPT_ACTION, ai: { category: 'data', requiresConfirmation: true } }); + fireEvent.click(exposeBox()); + typeDescription(DESCRIPTION); + expect(sent(seen.draft).ai).toStrictEqual({ category: 'data', requiresConfirmation: true, exposed: true, description: DESCRIPTION }); + expect(refusals(seen.draft)).toEqual([]); + }); +}); + +describe('aiBlockPin — AI_DESCRIPTION_MIN is the spec minimum (objectstack#26728)', () => { + const withDescription = (length: number): Draft => ({ ...SCRIPT_ACTION, ai: { exposed: true, description: 'x'.repeat(length) } }); + + it('the spec accepts a description of that length and refuses one shorter', () => { + expect(refusals(withDescription(AI_DESCRIPTION_MIN))).toEqual([]); + expect(refusals(withDescription(AI_DESCRIPTION_MIN - 1))).toEqual(['ai.description:too_small']); + }); +}); diff --git a/ui/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx b/ui/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx index 2aa41055670..df2e14cf8e1 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx +++ b/ui/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx @@ -23,9 +23,11 @@ * collapsed "More fields" SchemaForm fed the live server schema, with the * curated keys hidden so nothing is double-edited and nothing is lost. * - * AI exposure uses the flattened `aiExposed` / `aiDescription` keys (the - * objectui/server convention that ActionPreview reads), not the nested `ai` - * block, so the curated control and the runtime/preview agree. + * AI exposure writes the spec's nested `ai` block (`ai.exposed`, + * `ai.description`, ADR-0011). It used to write flat `aiExposed` / + * `aiDescription` keys, which the spec refuses by name, so one tick refused + * every later save of the object (objectstack#26728). The two flat keys are + * cleared by the AI control's next write, and never read. */ import * as React from 'react'; @@ -359,6 +361,47 @@ const UPDATE_OPERATION_REFUSES = [ */ const UPDATE_OPERATION_REFUSED_LOCATION = 'list_toolbar'; +/** + * objectstack#26736 — the keys `@objectstack/spec` refuses on an action whose + * `type` is not one of the listed ones, so a Type switch clears them. Without + * this, typing a script body and then switching to API kept the `body`, the + * spec refused it ("`body` only runs for `type: 'script'`"), and every later + * save of the object was refused with it. + * + * `undoable` is refused only as `true`, so only `true` is cleared. A key the + * new type still admits is kept, `target` included: it is the input every + * non-script type shows, and the spec judges its value on the new type. + * + * Written out rather than read off `ActionSchema` at runtime, for the reason + * {@link BODY_KEYS_BY_LANGUAGE} gives. `ActionDefaultInspector.typeSwitch-26736.test.tsx` + * parses each key on every `ActionType` with the spec, both ways. + */ +// eslint-disable-next-line react-refresh/only-export-components -- see above +export const TYPE_SCOPED_KEYS: ReadonlyArray<{ key: string; types: ReadonlySet; refused: (value: unknown) => boolean }> = [ + { key: 'body', types: new Set(['script']), refused: (v) => v !== undefined }, + { key: 'onSuccess', types: new Set(['api', 'script', 'flow']), refused: (v) => v !== undefined }, + { key: 'outcomeMessages', types: new Set(['api', 'script']), refused: (v) => v !== undefined }, + { key: 'undoable', types: new Set(['api']), refused: (v) => v === true }, +]; + +/** + * The shortest `ai.description` the spec accepts (`ActionAiSchema`, ADR-0011). + * The inspector's "too short" line and the untick below read it; the AI test + * pins it against the spec. + */ +// eslint-disable-next-line react-refresh/only-export-components -- see above +export const AI_DESCRIPTION_MIN = 40; + +/** + * objectstack#26736 — the two shapes the spec's `bodyShape` union takes: + * `'flat'`, or `{ wrap: KEY }`. The control offers exactly these, so a stored + * `{ wrap }` reads back as Wrap with its key, and nothing else can be written. + */ +const BODY_SHAPE_OPTS: KeyedOption[] = [ + { value: 'flat', labelKey: 'engine.inspector.action.bodyShape.flat' }, + { value: 'wrap', labelKey: 'engine.inspector.action.bodyShape.wrap' }, +]; + /** What a script action DOES — `operation` is authored beside `type`, not in it. */ const SCRIPT_OPERATIONS: KeyedOption[] = [ { value: '', labelKey: 'engine.inspector.action.scriptOp.run' }, @@ -368,7 +411,7 @@ const SCRIPT_OPERATIONS: KeyedOption[] = [ /** Keys this inspector edits with its own controls — hidden from the fallback. */ const CURATED_FIELDS = [ 'name', 'label', 'objectName', 'icon', 'variant', 'component', - 'type', 'target', 'execute', 'body', 'method', + 'type', 'target', 'execute', 'body', 'method', 'bodyShape', // The declarative single-record write, edited by the Behavior section's // "What it does" control and its field-value editor. Curated — and therefore // hidden from the fallback form — because the spec's refusal set around @@ -378,7 +421,7 @@ const CURATED_FIELDS = [ 'operation', 'patch', 'params', 'locations', 'confirmText', 'successMessage', 'errorMessage', 'refreshAfter', 'undoable', 'mode', - 'visible', 'disabled', 'aiExposed', 'aiDescription', + 'visible', 'disabled', ]; /** @@ -672,9 +715,72 @@ export function ActionDefaultInspector({ ? requiredPermissions.filter((c) => !heldCapabilities.includes(c)) : []; - // AI exposure — flattened keys (objectui/server convention, read by ActionPreview). - const aiExposed = draft.aiExposed === true; - const aiDescription = typeof draft.aiDescription === 'string' ? (draft.aiDescription as string) : ''; + /* ─── objectstack#26728 — AI exposure is the spec's nested `ai` block ───── + * + * Every write goes through `patchAi`: it merges into the block the draft + * holds (the other `ai` keys, edited under "More fields", are kept) and + * clears the flat `aiExposed` / `aiDescription` an older draft may carry, + * which the spec refuses by name. Unticking drops a description the spec + * would refuse as too short: the box that edits it is hidden while + * unexposed, so the author could not fix it there. */ + const ai = (draft.ai && typeof draft.ai === 'object' && !Array.isArray(draft.ai) ? draft.ai : {}) as Record; + const aiExposed = ai.exposed === true; + const aiDescription = typeof ai.description === 'string' ? ai.description : ''; + const patchAi = (change: Record) => { + const next: Record = { ...ai, ...change }; + for (const key of Object.keys(next)) if (next[key] === undefined) delete next[key]; + const out: Record = { ai: next }; + for (const flat of ['aiExposed', 'aiDescription']) if (flat in draft) out[flat] = undefined; + onPatch(out); + }; + const commitAiExposed = (exposed: boolean) => + patchAi(exposed || aiDescription.length >= AI_DESCRIPTION_MIN ? { exposed } : { exposed, description: undefined }); + + /** + * objectstack#26736 — a Type switch clears what the new type refuses + * ({@link TYPE_SCOPED_KEYS}). Only keys the draft holds are written, so a + * switch with nothing to clear writes the type alone. + */ + const commitType = (next: string) => { + const change: Record = { type: next }; + for (const { key, types, refused } of TYPE_SCOPED_KEYS) { + if (!types.has(next) && refused(draft[key])) change[key] = undefined; + } + onPatch(change); + }; + + /** + * objectstack#26736 — Body Shape as the spec's union: Flat, or Wrap under a + * key. Wrap is written only once it has a key: the spec accepts an empty + * one, which would nest the params under `""`. Until then the choice is + * held here, and the draft keeps what it had. + */ + const bodyShape = draft.bodyShape; + const storedWrap = + bodyShape && typeof bodyShape === 'object' && typeof (bodyShape as Record).wrap === 'string' + ? ((bodyShape as Record).wrap as string) + : undefined; + // Stamped with the action it was picked on: one inspector serves the + // actions a host lists in turn. + const [wrapPickedFor, setWrapPickedFor] = React.useState(null); + const bodyShapeMode = storedWrap !== undefined || wrapPickedFor === str('name') ? 'wrap' : 'flat'; + const commitBodyShapeMode = (mode: string) => { + if (mode === 'wrap') { + setWrapPickedFor(str('name')); + return; + } + setWrapPickedFor(null); + onPatch({ bodyShape: 'flat' }); + }; + const commitWrapKey = (key: string) => { + const trimmed = key.trim(); + if (trimmed === '') { + setWrapPickedFor(str('name')); + if (storedWrap !== undefined) onPatch({ bodyShape: undefined }); + return; + } + onPatch({ bodyShape: { wrap: trimmed } }); + }; /* ─── Blocking CEL verdicts → the host's Save gate (objectui#4527) ───── * @@ -764,7 +870,7 @@ export function ActionDefaultInspector({ {/* 2 ─ Behavior (type-first) */}
- onPatch({ type: v })} disabled={readOnly || isUpdateOperation} /> + {isUpdateOperation && (
{withSlots(tr('engine.inspector.action.pinnedScript'), { script: script })} @@ -881,6 +987,30 @@ export function ActionDefaultInspector({ )} )} + {/* Shown on an api action, and on any other that holds one, so a + stored shape is never edited out of sight. */} + {(type === 'api' || bodyShape !== undefined) && !isUpdateOperation && ( +
+ + {bodyShapeMode === 'wrap' && ( + + )} +
{tr('engine.inspector.action.bodyShapeHint')}
+
+ )}
{/* 3 ─ Inputs (params) */} @@ -1043,19 +1173,19 @@ export function ActionDefaultInspector({ {/* 7 ─ AI exposure */}
- onPatch({ aiExposed: v })} disabled={readOnly} /> + {aiExposed ? (