diff --git a/.changeset/9336-end-node-refusal-message-control.md b/.changeset/9336-end-node-refusal-message-control.md new file mode 100644 index 0000000000..7cd75a35c3 --- /dev/null +++ b/.changeset/9336-end-node-refusal-message-control.md @@ -0,0 +1,38 @@ +--- +'@object-ui/app-shell': minor +--- + +Give the flow `end` node's inspector a typed control for `config.message`, the key a +refused outcome requires (objectui#9336). + +`EndConfigSchema` cross-validates `outcome` and `message` in **both** directions: +`outcome: 'refused'` **requires** a `message` (a refusal nobody can explain is the shape +that contract exists to replace), and `outcome: 'completed'` — including an omitted +`outcome`, since the declared default resolves to `completed` — **refuses** one (a +completion renders nothing, so the key would be a silent no-op). + +The form offered a typed control for `outcome` and none for `message`. Since +objectui#9278 made Outcome a closed two-option dropdown, `refused` is one click away — +and picking it, with nothing else done, produced a flow that **fails to load**, because +the one key that outcome requires was authorable only through the Advanced (JSON) block. +That was a blocked authoring path, not a missing nicety. + +The `end` group now offers **"Why the run was refused"** — a `textarea`, gated on +`showWhen: { field: 'outcome', equals: ['refused'] }`. The kind, label, placeholder and +help are derived from the installed spec rather than chosen: `EndConfigSchema` describes +`message` as a `{token}` template "interpolated at run time exactly like a screen +`description`", and that field is a `textarea` in this same table (as is the sibling +`message` key on `notify`). + +The gate is what makes the pair authorable in **both** directions. An unset `outcome` +resolves through the declared `completed` default, so the field stays off screen until +the author actually picks `refused`; a **stored** `message` re-shows it regardless +(objectui#6499's stored-value rule), which is the only way an author can clear a stale +message after switching back to `completed`. Clearing deletes the key rather than +storing `''` — the empty string is refused under *both* outcomes. + +The Outcome field's help (English and the zh-CN overlay) no longer says the message is +"set in Advanced", since it no longer is. + +Not addressed here: objectui#9335, the same group's `outputVariable` row — a key +`EndConfigSchema` refuses by name. That row is deliberately untouched and remains open. diff --git a/packages/app-shell/src/views/metadata-admin/i18n.ts b/packages/app-shell/src/views/metadata-admin/i18n.ts index a8b8b205dd..ae9be18432 100644 --- a/packages/app-shell/src/views/metadata-admin/i18n.ts +++ b/packages/app-shell/src/views/metadata-admin/i18n.ts @@ -4105,9 +4105,15 @@ const FLOW_FIELD_ZH: Record> = { end: { outcome: { label: '结果', - help: '运行在此处如何结束。“已完成”是普通终态,也是省略该键时的取值。“已拒绝”把拒绝记为一等结果 —— 它是一次成功的评估,只是结论为否 —— 并要求给出拒绝理由 message({token} 模板),在“高级”中填写。', + help: '运行在此处如何结束。“已完成”是普通终态,也是省略该键时的取值。“已拒绝”把拒绝记为一等结果 —— 它是一次成功的评估,只是结论为否 —— 并要求在下方字段中给出拒绝理由 message({token} 模板)。', opts: { completed: '已完成', refused: '已拒绝' }, }, + // objectui#9336 —— 仅在“结果”选为“已拒绝”时出现(见 flow-node-config 的 + // `showWhen`);契约两个方向都校验:已拒绝必须有 message,已完成则拒收它。 + message: { + label: '拒绝原因', + help: '当结果为“已拒绝”时必填;结果为“已完成”时则被拒收 —— 已完成的结束节点不渲染任何文本,该键会成为静默的空操作。支持 {token} 模板(如 {record.name}),与屏幕节点的“描述”一致。', + }, outputVariable: { label: '输出变量' }, }, decision: { diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.endMessage-9336.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.endMessage-9336.test.tsx new file mode 100644 index 0000000000..8f5d77928a --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.endMessage-9336.test.tsx @@ -0,0 +1,345 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#9336 — the flow `end` node's `config.message`, the key + * `outcome: 'refused'` REQUIRES and `outcome: 'completed'` REFUSES. + * + * `EndConfigSchema` cross-validates the pair in BOTH directions through a + * `superRefine`. The form offered a typed control for `outcome` (objectui#9278 + * made it a closed two-option dropdown) and NONE for `message`, so the most + * direct route through the repaired control — pick `refused`, save — produced a + * flow that fails to load. That is a BLOCKED AUTHORING PATH, not a missing + * nicety, and it is the shape these rows pin: not "a field exists" but "the + * outcome an author can pick is one they can then satisfy". + * + * ## What makes each reading a reading rather than a dead probe + * + * Every negative here is paired with a lit control differing in exactly one + * way, because a zero whose control is also dead proves nothing: + * + * - the spec rows print BOTH signs from one parse: `refused` alone is refused, + * `refused` + `message` is accepted, and `completed` + `message` is refused. + * An accept-only loop would pass against a schema that accepts everything. + * - the "hidden on a completed end" row is paired with the SAME query finding + * the SAME control on a refused end — so an absence is a gate, never a + * renderer that drew nothing (objectui#8350's lesson). + * - the queries proving `message` absent are paired with `Outcome`, which is + * present in every one of those renders. + * + * ## The gate, and why it is `showWhen` rather than an always-on field + * + * An always-on `message` box would trade this defect for the contract's OTHER + * direction: a completion carrying a message is refused just as loudly. The + * descriptor is therefore gated on the controller, and the two halves that make + * the pair fully authorable are both pinned below — the field is OFF while the + * outcome resolves to `completed` (through the declared default, on an unset + * key), and a STORED message re-shows it anyway, which is the only way an + * author can clear a stale message after switching back. + * + * ⛔ These rows pin the CONTRACT's direction, not this file's taste. Every + * expectation about what the schema accepts is derived from the installed + * `EndConfigSchema` at run time, never from a literal restated here. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, fireEvent } from '@testing-library/react'; + +// Same doubles as the sibling flow-node suites: the engine config-schema hook +// publishes nothing (so the STATIC table is what renders), and object fields +// resolve empty. +vi.mock('../previews/useFlowNodePalette', () => ({ + useActionConfigSchemas: () => ({}), + useFlowNodePalette: () => [], +})); +vi.mock('../previews/useObjectFields', () => ({ + useObjectFields: () => ({ fields: [], loading: false, error: null }), +})); + +import { FlowNodeInspector } from './FlowNodeInspector'; +import { fieldsForNodeType, isFieldVisible } from './flow-node-config'; +import type { MetadataSelection } from '../preview-registry'; +// ⛔ The `/automation` subpath is load bearing — these read `undefined` off the +// package root, and any `.parse` on them then throws. +import { EndConfigSchema, FlowNodeSchema } from '@objectstack/spec/automation'; + +/* ── The `meta/*` double ─────────────────────────────────────────────────── + * `FlowNodeInspector` renders `FlowReferenceField` for reference-kind keys, and + * that field resolves options through a real `fetch` under happy-dom. Served + * from a recorder whose `afterEach` fails on any URL outside the metadata + * routes, so an escape to a data endpoint reds here instead of vanishing into a + * `.catch`. + * ────────────────────────────────────────────────────────────────────────── */ +const META_PREFIX = '/api/v1/meta/'; +let metaCalls: string[] = []; +const routeOf = (url: string) => url.split('?')[0]; + +beforeEach(() => { + metaCalls = []; + vi.stubGlobal( + 'fetch', + vi.fn(async (input: unknown) => { + const url = String( + input && typeof input === 'object' && 'url' in input ? (input as { url: unknown }).url : input, + ); + metaCalls.push(url); + const route = routeOf(url); + if (!route.startsWith(META_PREFIX)) { + return { ok: false, status: 404, headers: new Headers(), json: async () => ({}) }; + } + return { + ok: true, + status: 200, + headers: new Headers(), + json: async () => ({ type: route.slice(META_PREFIX.length), items: [] }), + }; + }), + ); +}); + +afterEach(() => { + expect(metaCalls.filter((url) => !routeOf(url).startsWith(META_PREFIX))).toEqual([]); + // Unmount BEFORE restoring the real `fetch` (objectui#7439). + cleanup(); + vi.unstubAllGlobals(); +}); + +function draftWith(config: Record) { + return { nodes: [{ id: 'n1', type: 'end', label: 'Done', config }], edges: [] }; +} + +function renderInspector(draft: Record) { + const onPatch = vi.fn(); + const utils = render( + , + ); + return { onPatch, ...utils }; +} + +const endFields = () => fieldsForNodeType('end'); +const messageField = () => endFields().find((f) => f.id === 'message'); +const outcomeField = () => endFields().find((f) => f.id === 'outcome'); + +/** + * The `message` control as the author sees it, found by the placeholder the + * descriptor declares — `Label` here is not `htmlFor`-bound, so a label query + * cannot reach the input itself. + */ +const messageControl = () => + screen.queryByPlaceholderText(messageField()!.placeholder!) as HTMLTextAreaElement | null; + +/* ── 1. The contract, read off the installed spec ─────────────────────────── */ + +describe('the contract this form has to make satisfiable (objectui#9336)', () => { + const parse = (config: Record) => + FlowNodeSchema.safeParse({ id: 'e', type: 'end', label: 'E', config }); + + it('still declares `message`, and still judges the pair in BOTH directions', () => { + // THE VACUITY GUARD. Every row below is about a key; a spec that stopped + // declaring it would make the form's new field wrong rather than right, and + // these rows would otherwise pass over a schema that no longer has an + // opinion. + expect( + Object.keys((EndConfigSchema as unknown as { shape: Record }).shape), + 'EndConfigSchema still declares exactly the pair this card is about', + ).toEqual(['outcome', 'message']); + + // Both signs, one reading. The ACCEPTED rows are what make the REFUSED ones + // a measurement: a schema that refused everything would pass the negatives + // alone. + expect(parse({ outcome: 'completed' }).success, '`completed` alone is accepted').toBe(true); + expect( + parse({ outcome: 'refused', message: 'Refused: {record.name} is a confirmed duplicate' }).success, + '`refused` WITH a message is accepted — the shape the form must be able to produce', + ).toBe(true); + expect( + parse({ outcome: 'refused' }).success, + '`refused` with no message is REFUSED — which is what an author could only produce before', + ).toBe(false); + expect( + parse({ outcome: 'completed', message: 'x' }).success, + 'and the other direction: a completion carrying a message is REFUSED', + ).toBe(false); + expect( + parse({ message: 'x' }).success, + 'including when `outcome` is OMITTED — the default resolves to `completed`, so the rule still bites', + ).toBe(false); + }); + + it('and refuses the empty string under BOTH outcomes — so clearing must DELETE the key', () => { + // This is why the control must not commit `''`. `setAtPath` deletes a leaf + // on an empty commit; if it ever started storing `''` instead, an author + // who cleared this box would ship an unloadable flow under either outcome. + expect(parse({ outcome: 'refused', message: '' }).success, "refused + '' is refused (min(1))").toBe(false); + expect(parse({ outcome: 'completed', message: '' }).success, "completed + '' is refused too").toBe(false); + }); +}); + +/* ── 2. The descriptor ────────────────────────────────────────────────────── */ + +describe('the end group offers a typed control for `message` (objectui#9336)', () => { + it('declares a `message` field, gated on the outcome that requires it', () => { + const field = messageField(); + expect(field, 'the end node offers a typed control for `message` at all').toBeDefined(); + expect( + field!.path, + 'and it writes the key the contract names, under config', + ).toEqual(['config', 'message']); + // Derived, not chosen: the spec describes `message` as interpolated + // "exactly like a screen `description`", and that field is a textarea in + // this same table — as is the sibling `message` key on `notify`. + expect(field!.kind, 'a {token} template body is not authored in a one-line box').toBe('textarea'); + expect(field!.showWhen, 'it is gated on the outcome that requires it').toEqual({ + field: 'outcome', + equals: ['refused'], + }); + // The controller the gate names must EXIST in this group, or `showWhen` + // silently resolves to "never admit" and the field becomes unreachable. + expect( + endFields().some((f) => f.id === field!.showWhen!.field), + 'the gate names a controller that exists in this group', + ).toBe(true); + // ⛔ No declared default: `message` is `.optional()` with no `.default()`, + // and a `defaultValue` here would be an unchecked claim about the spec. + expect(field!.defaultValue, 'the spec declares no default for `message`').toBeUndefined(); + }); + + it('leaves the `outputVariable` row of the same group untouched (objectui#9335 is NOT ridden)', () => { + // The separability pin. objectui#9335 is a live, unclaimed defect in this + // same group: `outputVariable` is a key `EndConfigSchema` refuses BY NAME. + // This card does not fix it, and this row exists so a later reader can see + // that it was left deliberately rather than missed — and so that a future + // repair of #9335 has to come past this assertion on purpose. + const outputVariable = endFields().find((f) => f.id === 'outputVariable'); + expect(outputVariable, 'the `outputVariable` row is still exactly as it was').toBeDefined(); + expect(outputVariable!.kind).toBe('text'); + expect(outputVariable!.showWhen, 'ungated, as before — untouched by this card').toBeUndefined(); + expect( + FlowNodeSchema.safeParse({ + id: 'e', + type: 'end', + label: 'E', + config: { outcome: 'refused', message: 'why', outputVariable: 'result' }, + }).success, + 'objectui#9335 is still open and still unsavable — this card claims no repair of it', + ).toBe(false); + }); + + it('the visibility resolver admits it only for a refused outcome', () => { + const field = messageField()!; + const fields = endFields(); + expect( + isFieldVisible(field, { id: 'n', type: 'end', config: {} }, fields), + 'an UNSET outcome resolves through the declared `completed` default — hidden', + ).toBe(false); + expect( + isFieldVisible(field, { id: 'n', type: 'end', config: { outcome: 'completed' } }, fields), + 'and an explicit `completed` keeps it hidden', + ).toBe(false); + expect( + isFieldVisible(field, { id: 'n', type: 'end', config: { outcome: 'refused' } }, fields), + 'picking `refused` puts it on screen', + ).toBe(true); + // The stored-value re-show rule (objectui#6499 Option C) — the ONLY way an + // author can clear a stale message after switching back to `completed`. + expect( + isFieldVisible(field, { id: 'n', type: 'end', config: { outcome: 'completed', message: 'stale' } }, fields), + 'a STORED message stays visible under `completed`, so it can be cleared rather than stranded', + ).toBe(true); + // The lit control for the two negatives above: the SAME resolver says yes to + // the ungated sibling on the very same node, so `false` is a gate and not a + // resolver that refuses everything. + expect( + isFieldVisible(outcomeField()!, { id: 'n', type: 'end', config: {} }, fields), + 'the ungated Outcome field is visible on the same node — the negatives above are a gate, not a dead resolver', + ).toBe(true); + }); +}); + +/* ── 3. The rendered inspector ────────────────────────────────────────────── */ + +describe('and the author actually sees it on a refused end (objectui#9336)', () => { + it('renders the control on a refused end — and the label states the contract', () => { + renderInspector(draftWith({ outcome: 'refused' })); + expect(messageControl(), 'the message control is rendered at all').not.toBeNull(); + expect( + screen.queryByText(messageField()!.label), + 'and is labelled in the spec’s own words', + ).not.toBeNull(); + }); + + it('hides it on a completed end — with the SAME query finding it one render away', () => { + // The paired control. This negative differs from the positive above in + // exactly one way: the stored outcome. + renderInspector(draftWith({ outcome: 'completed' })); + expect(messageControl(), 'a completed end does not offer the key its outcome refuses').toBeNull(); + expect( + screen.queryByRole('combobox', { name: 'Outcome' }), + 'the lit control: this render DID draw the end inspector', + ).not.toBeNull(); + cleanup(); + + renderInspector(draftWith({ outcome: 'refused' })); + expect( + messageControl(), + 'and the identical query finds it when the outcome is refused — so the null above is a gate', + ).not.toBeNull(); + }); + + it('hides it on an end with NO stored outcome — the default the runtime applies', () => { + renderInspector(draftWith({})); + expect(messageControl(), 'an unset outcome resolves to `completed`, so the field is off screen').toBeNull(); + expect( + screen.queryByRole('combobox', { name: 'Outcome' }), + 'the lit control, again: the inspector rendered', + ).not.toBeNull(); + }); + + it('re-shows a STORED message under `completed`, so a stale value can be cleared', () => { + renderInspector(draftWith({ outcome: 'completed', message: 'stale refusal text' })); + const control = messageControl(); + expect(control, 'a stored message is never hidden from the author who has to clear it').not.toBeNull(); + expect(control!.value, 'and it shows the stored text').toBe('stale refusal text'); + }); + + it('writes `config.message`, and a cleared box DELETES the key rather than storing ""', () => { + // The end-to-end row: what the author types has to reach the key the + // contract requires, in a shape the contract accepts. + const { onPatch } = renderInspector(draftWith({ outcome: 'refused' })); + const control = messageControl()!; + fireEvent.change(control, { target: { value: 'Refused: {record.name} is a confirmed duplicate' } }); + fireEvent.blur(control); + + expect(onPatch, 'the edit reached the draft').toHaveBeenCalled(); + const written = onPatch.mock.calls.at(-1)![0] as { nodes: Array> }; + const config = written.nodes[0].config as Record; + expect(config.message, 'the typed text lands on the key the contract names').toBe( + 'Refused: {record.name} is a confirmed duplicate', + ); + // ⭐ The whole point of the card, stated as a parse: what the form now + // produces from a `refused` outcome LOADS. + expect( + FlowNodeSchema.safeParse({ id: 'n1', type: 'end', label: 'Done', config }).success, + 'and the node the form produced is one the contract accepts — the blocked path is open', + ).toBe(true); + + // Now clear it. `setAtPath` deletes an empty leaf; a commit of `''` would be + // refused under BOTH outcomes (pinned above), so this row is load bearing. + cleanup(); + const second = renderInspector(draftWith({ outcome: 'refused', message: 'stale' })); + const stored = messageControl()!; + fireEvent.change(stored, { target: { value: '' } }); + fireEvent.blur(stored); + const cleared = (second.onPatch.mock.calls.at(-1)![0] as { nodes: Array> }).nodes[0] + .config as Record; + expect('message' in cleared, 'clearing DELETES the key — it never stores the empty string').toBe(false); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.inactiveRetained.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.inactiveRetained.test.ts index e13ad33833..c17cc83504 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.inactiveRetained.test.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.inactiveRetained.test.ts @@ -156,13 +156,19 @@ describe('inactiveRetainedKind — mechanical coverage of every showWhen group', it('finds the groups the census measured — and does not tag everything', () => { // If this drifts, the census in the PR body is stale; re-measure before // trusting the coverage claim below. - expect(gatedByType.size).toBe(8); // 8 (name, ...) buckets — 7 canonical + the `script_task` alias + // ⚑ RE-MEASURED at landing, not carried over: objectui#9336 gave the `end` + // node its first `showWhen` field (`config.message`, gated on + // `outcome === 'refused'`), so `end` became a NEW bucket. 8 -> 9 buckets, + // 33 -> 34 gated fields, 24 -> 23 ungated types. Triage's standing ruling + // for the counts objectui#9277/#9278 share applies here too: re-measure at + // landing time rather than copying a number out of the card. + expect(gatedByType.size).toBe(9); // 9 (name, ...) buckets — 8 canonical + the `script_task` alias const totalGated = [...gatedByType.values()].reduce((n, fs) => n + fs.length, 0); - expect(totalGated).toBe(33); + expect(totalGated).toBe(34); // The discriminating control: most node types have NO showWhen at all, so a // predicate that simply said "yes" everywhere would fail here. const ungatedTypes = TYPES.filter((t) => !gatedByType.has(t)); - expect(ungatedTypes.length).toBe(24); + expect(ungatedTypes.length).toBe(23); expect(ungatedTypes).toContain('create_record'); expect(ungatedTypes).toContain('connector_action'); }); @@ -203,7 +209,7 @@ describe('inactiveRetainedKind — mechanical coverage of every showWhen group', checked += 1; } } - expect(checked).toBe(33); + expect(checked).toBe(34); // objectui#9336 added `end.message` — see the note above. }); }); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts index 7fa56dc242..4ea15bf4f6 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts @@ -491,7 +491,42 @@ const FLOW_NODE_CONFIG: Record = { // no typed control for yet: it REQUIRES a `message` saying why, as a // {token} template. Named here so picking it is not a one-click route to // a flow that will not load; the key itself stays authorable in Advanced. - help: 'How the run ends here. "Completed" is the ordinary terminal and is what an omitted key applies. "Refused" records a first-class refusal — a successful evaluation that says no — and requires a message saying why (a {token} template), which is set in Advanced.', + help: 'How the run ends here. "Completed" is the ordinary terminal and is what an omitted key applies. "Refused" records a first-class refusal — a successful evaluation that says no — and requires a message saying why (a {token} template), authored in the field below.', + }), + // objectui#9336 — the other half of that cross-field rule, which + // `EndConfigSchema` enforces in BOTH directions via a `superRefine`: + // `refused` REQUIRES `message`, `completed` (including an omitted key, + // which the declared default resolves to `completed`) REFUSES it. With no + // typed control, picking `refused` in this form was a blocked authoring + // path — the one key the chosen outcome requires was reachable only through + // Advanced JSON, so the most direct route through the repaired dropdown + // produced a flow that fails to load. + // + // `textarea` is derived, not chosen: the spec's own `message` description + // says it is "interpolated at run time exactly like a screen `description`", + // and that field IS a `textarea` in this table — as is the sibling + // `message` key on `notify`. The label and placeholder are the spec's own + // words (its describe opens "Why the run was refused", and both the + // describe and the refusal message give this exact example), so the form + // states the contract rather than a second vocabulary. + // + // ⛔ No `defaultValue`: `message` is `z.string().min(1).optional()` with no + // `.default()`, and a declaration here is read as a claim about the + // installed spec (see the `defaultValue` doc comment above). + // + // The gate is what makes the pair authorable in both directions. An unset + // `outcome` resolves through the declared `completed` default, so the field + // stays off screen until the author actually picks `refused`; a STORED + // `message` re-shows it regardless (`isFieldVisible`'s stored-value rule), + // which is what lets an author clear a stale message after switching back + // to `completed` — the case the contract's other direction refuses. The + // clearing itself lands: `setAtPath` deletes the leaf on an empty commit, + // so the key goes away rather than becoming `''`, which this schema refuses + // under BOTH outcomes (`min(1)`, and the completed-forbids-message rule). + cfg('message', 'Why the run was refused', 'textarea', { + placeholder: 'Refused: {record.name} is a confirmed duplicate', + help: 'Required when the outcome is "Refused", and refused on a completed end — a completion renders nothing, so the key would be a silent no-op. A {token} template interpolated at run time (e.g. {record.name}), exactly like a screen Description.', + showWhen: { field: 'outcome', equals: ['refused'] }, }), cfg('outputVariable', 'Output variable', 'text', { placeholder: 'result' }), ],