diff --git a/.changeset/6499-inactive-values-retained.md b/.changeset/6499-inactive-values-retained.md new file mode 100644 index 000000000..09a4fc09d --- /dev/null +++ b/.changeset/6499-inactive-values-retained.md @@ -0,0 +1,35 @@ +--- +'@object-ui/app-shell': minor +--- + +metadata-admin inspectors: name the "inactive values retained" state instead of +rendering it as live configuration (objectui#6499). + +`showWhen` gates rendering only, and `isFieldVisible` additionally re-shows any +field that already holds a stored value — deliberately, "so existing config is +never hidden". The consequence on screen: an author who enabled a controller, +filled its dependent fields, then switched the controller back off keeps seeing +those fields as ordinary, live-looking controls. The stored config and the +switch beside it disagree, and nothing said which one was in effect. + +Per the maintainer ruling of 2026-08-27 (Option C), the values are KEPT and the +state is made explicit. Pruning on save was rejected: it silently discards +config an author entered, and inverts the very rule that stops config from +vanishing unseen. + +- New `inactiveRetainedKind(field, node, fields)` in `flow-node-config.ts` — a + pure read that reports a field rendered ONLY because the stored-value re-show + rule fired. It distinguishes `'controller-off'` (a real toggle the author can + switch back on) from `'no-controller'` (the `__legacy__` render-only keys, + where no such toggle exists and saying otherwise would be a fresh lie). +- `FlowNodeConfigField` renders the notice beside the affected control, with a + "Clear value" action so the author can discard the residue **deliberately**. + Read-only inspectors show the notice without the action. +- Coverage is every `showWhen` group in the inspectors, pinned mechanically: + all 33 gated fields across the descriptor tables, plus the two runtime + producers (an engine-published `configSchema` and a connector input schema) + that mint groups no source file contains. + +Render-layer only: no save-path change, no data deletion, and `isFieldVisible`'s +stored-value re-show rule is unchanged — clearing is an ordinary author-initiated +field commit, the same write as emptying the control by hand. diff --git a/packages/app-shell/src/views/metadata-admin/i18n.ts b/packages/app-shell/src/views/metadata-admin/i18n.ts index b80f109b6..604b7487e 100644 --- a/packages/app-shell/src/views/metadata-admin/i18n.ts +++ b/packages/app-shell/src/views/metadata-admin/i18n.ts @@ -395,6 +395,12 @@ const ENGINE_STRINGS_EN: Record = { 'engine.inspector.flowNode.config': 'Config (JSON)', 'engine.inspector.flowNode.advanced': 'Advanced (JSON)', 'engine.inspector.flowNode.advancedHint': 'Optional custom keys not covered by the form above — most flows don\u2019t need this.', + // objectui#6499 — a dependent field whose controller is off still renders, + // because a stored value is never hidden. These name that state instead of + // letting it read as live configuration. + 'engine.inspector.flowNode.inactiveRetained': 'Kept, not in effect — its controlling field is off. The value stays saved until you clear it.', + 'engine.inspector.flowNode.inactiveRetainedOrphan': 'Kept, not in effect — nothing activates this field. The value stays saved so it is not lost silently.', + 'engine.inspector.flowNode.inactiveRetainedClear': 'Clear value', 'engine.inspector.flowNode.noConfig': 'No configuration needed for this node type.', 'engine.inspector.flowNode.nestedIdHint': 'A node inside a container region keeps its id here — rename it in the container’s Advanced JSON.', 'engine.inspector.flowNode.kv.add': 'Add entry', @@ -2211,6 +2217,9 @@ const ENGINE_STRINGS_ZH: Record = { 'engine.inspector.flowNode.config': '配置（JSON）', 'engine.inspector.flowNode.advanced': '高级（JSON）', 'engine.inspector.flowNode.advancedHint': '上方表单未覆盖的可选自定义键 —— 大多数流程无需填写。', + 'engine.inspector.flowNode.inactiveRetained': '已保留，未生效 —— 控制字段处于关闭。该值仍然存储，直到你清除它。', + 'engine.inspector.flowNode.inactiveRetainedOrphan': '已保留，未生效 —— 没有任何开关能启用此字段。该值仍然存储，以免静默丢失。', + 'engine.inspector.flowNode.inactiveRetainedClear': '清除值', 'engine.inspector.flowNode.noConfig': '此节点类型无需配置。', 'engine.inspector.flowNode.nestedIdHint': '容器区域内的节点 ID 在此只读 —— 请在容器的高级 JSON 中重命名。', 'engine.inspector.flowNode.kv.add': '添加条目', diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.tsx index 5b63e86e5..b8114aaf2 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.tsx @@ -7,14 +7,14 @@ */ import * as React from 'react'; -import type { FlowConfigField } from './flow-node-config.js'; +import type { FlowConfigField, InactiveRetainedKind } from './flow-node-config.js'; import { t } from '../i18n.js'; import { InspectorNumberField, InspectorSelectField, InspectorCheckboxField, } from './_shared.js'; -import { Label } from '@object-ui/components'; +import { Button, Label } from '@object-ui/components'; import { FlowKeyValueField } from './FlowKeyValueField.js'; import { FlowStringListField } from './FlowStringListField.js'; import { FlowObjectListField } from './FlowObjectListField.js'; @@ -65,9 +65,26 @@ export interface FlowNodeConfigFieldProps { * the value. */ triggerScope?: TriggerScope; + /** + * objectui#6499 — set when this field is on screen ONLY because it holds a + * stored value its `showWhen` controller does not currently admit. Supplied + * by the host inspector, which owns the node and the sibling field set (this + * component sees neither), and computed by `inactiveRetainedKind`. + * + * `undefined`/`null` is the normal case and renders exactly as before, so a + * caller that does not pass it is unaffected. + */ + inactiveRetained?: InactiveRetainedKind | null; + /** + * Clears the retained value — an ordinary field commit of `undefined`, the + * same write the author would make by emptying the control by hand. The + * ruling keeps hidden values by default; this is how the author discards one + * DELIBERATELY. Omit to render the notice without the button. + */ + onClearInactive?: () => void; } -export function FlowNodeConfigField({ field, value, onCommit, disabled, locale, context, scopeGroups, approvalScopeGroups, triggerScope }: FlowNodeConfigFieldProps) { +export function FlowNodeConfigField({ field, value, onCommit, disabled, locale, context, scopeGroups, approvalScopeGroups, triggerScope, inactiveRetained, onClearInactive }: FlowNodeConfigFieldProps) { const refMode: 'expression' | 'template' = field.refMode ?? (field.kind === 'expression' ? 'expression' : 'template'); // objectui#6226 — the row-based condition builder, on the fields that opted in @@ -281,6 +298,44 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale, return (
{control} + {/* + objectui#6499 — the "inactive values retained" affordance. Rendered + ABOVE the expression/scope notes and independently of them: those judge + the value's CONTENT, this one says the value is not in effect at all, + and an author who cannot see the second will misread the first. + Deliberately not a `disabled` control — the value is still editable, it + just is not live, and greying it out would hide the very text the + ruling asks the author to be able to read and act on. + */} + {inactiveRetained && ( +
+

+ {t( + inactiveRetained === 'no-controller' + ? 'engine.inspector.flowNode.inactiveRetainedOrphan' + : 'engine.inspector.flowNode.inactiveRetained', + locale, + )} +

+ {onClearInactive && ( + + )} +
+ )} {exprIssue && (

{exprIssue.message} diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.inactiveRetained.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.inactiveRetained.test.tsx new file mode 100644 index 000000000..187e1957b --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.inactiveRetained.test.tsx @@ -0,0 +1,146 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The rendered "inactive values retained" affordance (objectui#6499, Option C). + * + * The predicate is pinned in `flow-node-config.inactiveRetained.test.ts`; this + * file pins what the AUTHOR sees, because the whole ruling is about what is + * visible on screen. A hidden-but-stored dependent value keeps rendering — the + * ruling forbids deleting it — so the only thing that can tell the author it is + * inert is this notice beside it. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, fireEvent, within } from '@testing-library/react'; + +vi.mock('../previews/useFlowNodePalette', () => ({ + useActionConfigSchemas: () => ({}), + useFlowNodePalette: () => [], +})); +vi.mock('../previews/useObjectFields', () => ({ + useObjectFields: () => ({ fields: [], loading: false, error: null }), +})); + +import { FlowNodeInspector } from './FlowNodeInspector'; +import type { MetadataSelection } from '../preview-registry'; + +afterEach(cleanup); + +function draftWith(config: Record, type = 'approval') { + return { nodes: [{ id: 'gate', type, label: 'Gate', config }], edges: [] }; +} + +function renderInspector(draft: Record, readOnly = false) { + const onPatch = vi.fn(); + const utils = render( + , + ); + return { onPatch, ...utils }; +} + +const notices = () => screen.queryAllByTestId('inactive-retained'); + +describe('the affordance appears exactly when a value is hidden-but-stored', () => { + it('appears for the escalation residue the card measured', () => { + // enable → fill → toggle back off → save. The stored payload the UI has + // been manufacturing: `{ enabled: false, timeoutHours: 24 }`. + renderInspector(draftWith({ escalation: { enabled: false, timeoutHours: 24 } })); + const found = notices(); + expect(found).toHaveLength(1); + expect(found[0].textContent).toMatch(/kept, not in effect/i); + expect(found[0].textContent).toMatch(/controlling field is off/i); + expect(found[0].getAttribute('data-inactive-retained')).toBe('controller-off'); + // The value itself is still on screen and still stored — nothing pruned. + expect(screen.getByDisplayValue('24')).toBeInTheDocument(); + }); + + it('does NOT appear when the controller is on — same stored value', () => { + renderInspector(draftWith({ escalation: { enabled: true, timeoutHours: 24 } })); + expect(notices()).toHaveLength(0); + expect(screen.getByDisplayValue('24')).toBeInTheDocument(); // control: the field IS rendered + }); + + it('does NOT appear when the controller is off and nothing is stored', () => { + renderInspector(draftWith({ escalation: { enabled: false } })); + expect(notices()).toHaveLength(0); + expect(screen.queryByDisplayValue('24')).not.toBeInTheDocument(); + }); + + it('does NOT appear on a node with no gated fields at all', () => { + renderInspector(draftWith({ objectName: 'contract', outputVariable: 'r' }, 'create_record')); + expect(notices()).toHaveLength(0); + }); + + it('appears once per retained dependent, not once for the group', () => { + renderInspector(draftWith({ escalation: { enabled: false, timeoutHours: 24, escalateTo: 'sre-lead' } })); + expect(notices()).toHaveLength(2); + }); + + it('uses the no-controller wording for a `__legacy__` render-only key', () => { + renderInspector({ nodes: [{ id: 'gate', type: 'decision', label: 'Gate', config: { condition: 'amount > 10000' } }], edges: [] }); + const found = notices(); + expect(found).toHaveLength(1); + expect(found[0].getAttribute('data-inactive-retained')).toBe('no-controller'); + expect(found[0].textContent).toMatch(/nothing activates this field/i); + // and NOT the controller wording — there is no toggle to point at + expect(found[0].textContent).not.toMatch(/controlling field is off/i); + }); +}); + +describe('clearing is deliberate, author-initiated, and goes through the ordinary field commit', () => { + it('offers a clear button that removes the retained key', () => { + const { onPatch } = renderInspector(draftWith({ escalation: { enabled: false, timeoutHours: 24 } })); + const clear = within(notices()[0]).getByRole('button', { name: /clear value/i }); + fireEvent.click(clear); + const patched = onPatch.mock.calls.at(-1)![0] as any; + // The retained key is gone; the controller the author actually set stays. + expect(patched.nodes[0].config.escalation).toEqual({ enabled: false }); + }); + + it('does not clear anything until the author clicks — rendering is a read', () => { + const { onPatch } = renderInspector(draftWith({ escalation: { enabled: false, timeoutHours: 24 } })); + expect(onPatch).not.toHaveBeenCalled(); + }); + + it('offers no clear button in a read-only inspector', () => { + renderInspector(draftWith({ escalation: { enabled: false, timeoutHours: 24 } }), true); + expect(notices()).toHaveLength(1); // the author can still SEE the state + expect(within(notices()[0]).queryByRole('button', { name: /clear value/i })).toBeNull(); + }); +}); + +describe('assertions that must NOT move', () => { + it('an ordinary live field renders with no notice and no clear button', () => { + renderInspector(draftWith({ escalation: { enabled: true, timeoutHours: 24 } })); + expect(screen.queryByRole('button', { name: /clear value/i })).toBeNull(); + expect(notices()).toHaveLength(0); + }); + + it('the retained value survives a re-render — the affordance never prunes on its own', () => { + const draft = draftWith({ escalation: { enabled: false, timeoutHours: 24 } }); + const { onPatch, rerender } = renderInspector(draft); + rerender( + , + ); + expect(onPatch).not.toHaveBeenCalled(); + expect((draft as any).nodes[0].config.escalation).toEqual({ enabled: false, timeoutHours: 24 }); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.tsx index c201f10e4..975b4673a 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.tsx @@ -34,6 +34,7 @@ import { mergeServerFlowFields, localizeFlowFields, isFieldVisible, + inactiveRetainedKind, getFieldValue, configKeyOf, FLOW_NODE_TYPE_OPTIONS, @@ -454,6 +455,13 @@ export function FlowNodeInspector({ selection, draft, onPatch, onClearSelection, scopeGroups={scopeGroups} approvalScopeGroups={approvalExpressionGroups} triggerScope={triggerScope} + // objectui#6499 — a gated field that survived the filter above ONLY + // because it holds a stored value is inert config wearing a live + // control's clothes. Name it, and offer the deliberate clear. + // Computed from `field` (not `effField`): the read is by `path`, + // which the nested-branch rewrite above does not touch. + inactiveRetained={inactiveRetainedKind(field, node, fields)} + onClearInactive={readOnly ? undefined : () => setField(field, undefined)} /> ); })} 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 new file mode 100644 index 000000000..7d90d8260 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.inactiveRetained.test.ts @@ -0,0 +1,255 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `inactiveRetainedKind` — the "inactive values retained" predicate + * (objectui#6499, maintainer ruling 2026-08-27, Option C). + * + * The defect: `showWhen` gates RENDERING only, and `isFieldVisible` additionally + * re-shows any field that already holds a stored value ("so existing config is + * never hidden"). So after an author fills a dependent field and switches its + * controller back off, the field keeps rendering — as an ordinary, live-looking + * control. The stored config says one thing, the switch beside it says another, + * and nothing on screen tells the author which. + * + * The ruling KEEPS the value (pruning on save was rejected outright: it deletes + * config an author entered, and inverts the very rule that stops config from + * vanishing unseen). What changes is that the state is now NAMED. This file + * pins the predicate that names it, plus the half that must not move: the + * re-show rule itself. + */ + +import { describe, it, expect } from 'vitest'; +import { + fieldsForNodeType, + isFieldVisible, + inactiveRetainedKind, + FLOW_NODE_TYPE_OPTIONS, + type FlowConfigField, +} from './flow-node-config.js'; +import { jsonSchemaToFlowFields } from './json-schema-to-fields.js'; +import { connectorInputFields } from './connector-input-fields.js'; + +const approval = () => fieldsForNodeType('approval'); +const field = (fields: FlowConfigField[], id: string) => fields.find((f) => f.id === id)!; + +describe('inactiveRetainedKind — the escalation instance the card measured', () => { + const fields = approval(); + const timeout = () => field(fields, 'escalation.timeoutHours'); + + it('flags a dependent value whose controller was switched back off', () => { + // Exactly the chain on the card: enable, fill, toggle off, save. + const node = { id: 'a', type: 'approval', config: { escalation: { enabled: false, timeoutHours: 24 } } }; + expect(isFieldVisible(timeout(), node, fields)).toBe(true); // still rendered — the re-show rule + expect(inactiveRetainedKind(timeout(), node, fields)).toBe('controller-off'); + }); + + it('does NOT flag the same value while the controller is on', () => { + const node = { id: 'a', type: 'approval', config: { escalation: { enabled: true, timeoutHours: 24 } } }; + expect(isFieldVisible(timeout(), node, fields)).toBe(true); + expect(inactiveRetainedKind(timeout(), node, fields)).toBeNull(); + }); + + it('does NOT flag a gated field that holds nothing — it is hidden, not retained', () => { + const node = { id: 'a', type: 'approval', config: { escalation: { enabled: false } } }; + expect(isFieldVisible(timeout(), node, fields)).toBe(false); + expect(inactiveRetainedKind(timeout(), node, fields)).toBeNull(); + }); + + it('does NOT flag an UNGATED field, however it is filled', () => { + const ungated = fields.filter((f) => !f.showWhen); + expect(ungated.length).toBeGreaterThan(0); // control: the node type really has some + const node = { id: 'a', type: 'approval', config: { escalation: { enabled: false, timeoutHours: 24 } } }; + for (const f of ungated) expect(inactiveRetainedKind(f, node, fields)).toBeNull(); + }); + + it('an absent controller value resolves through the spec defaultValue, not to "off"', () => { + // `escalation.enabled` declares defaultValue 'false', so an omitted key is + // off — and a stored dependent under it is therefore retained-but-inactive. + // + // ⚠️ Coupled to objectui#6620 ON PURPOSE. That card is the mirror defect: + // `@objectstack/spec` flipped `ApprovalEscalation.enabled` to `default(true)` + // upstream, so once this repo consumes a spec release carrying the flip, this + // descriptor's `defaultValue: 'false'` becomes wrong and must follow. When it + // does, THIS assertion flips to `toBeNull()` — an omitted key will mean ON, + // and a stored dependent under it is live, not retained. Measured here on + // 2026-08-29: installed spec is 17.2.0, still `.default(false)`, so the two + // agree and #6620 is latent. The failure is the intended signal, not a break. + const node = { id: 'a', type: 'approval', config: { escalation: { timeoutHours: 24 } } }; + expect(inactiveRetainedKind(timeout(), node, fields)).toBe('controller-off'); + }); + + it('flags every dependent in the group, not just the first', () => { + const node = { + id: 'a', + type: 'approval', + config: { escalation: { enabled: false, timeoutHours: 24, action: 'reassign', escalateTo: 'sre-lead', notifySubmitter: true } }, + }; + for (const id of ['escalation.timeoutHours', 'escalation.action', 'escalation.escalateTo', 'escalation.notifySubmitter']) { + expect(inactiveRetainedKind(field(fields, id), node, fields), id).toBe('controller-off'); + } + }); +}); + +describe('inactiveRetainedKind — the `__legacy__` sentinel is a group too', () => { + it('reports no-controller for a render-only legacy key holding a value', () => { + const fields = fieldsForNodeType('decision'); + const condition = field(fields, 'condition'); + expect(condition.showWhen).toEqual({ field: '__legacy__', equals: [] }); + const node = { id: 'd', type: 'decision', config: { condition: 'amount > 10000' } }; + expect(isFieldVisible(condition, node, fields)).toBe(true); + // NOT 'controller-off': there is no toggle to switch back on, and telling + // the author to go find one would be a fresh lie on this very screen. + expect(inactiveRetainedKind(condition, node, fields)).toBe('no-controller'); + }); + + it('reports nothing for the same legacy key when it is empty', () => { + const fields = fieldsForNodeType('decision'); + const condition = field(fields, 'condition'); + const node = { id: 'd', type: 'decision', config: {} }; + expect(isFieldVisible(condition, node, fields)).toBe(false); + expect(inactiveRetainedKind(condition, node, fields)).toBeNull(); + }); +}); + +/** + * Coverage is "all `showWhen` groups in the metadata-admin inspectors" (the + * ruling). This walks the descriptor tables mechanically rather than trusting a + * hand-written list, so a group added later is covered — or turns this red. + */ +describe('inactiveRetainedKind — mechanical coverage of every showWhen group', () => { + const TYPE_ALIASES = ['task', 'user_task', 'service_task', 'script_task', 'notification', 'signal', 'webhook', 'for_each']; + const CANONICAL = [ + 'start', 'end', 'decision', 'assignment', 'loop', 'map', 'create_record', 'update_record', + 'delete_record', 'get_record', 'http_request', 'script', 'screen', 'approval', 'wait', + 'subflow', 'notify', 'connector_action', 'parallel', 'try_catch', 'parallel_gateway', + 'join_gateway', 'boundary_event', 'legacy_action', + ]; + const TYPES = [...new Set([...CANONICAL, ...FLOW_NODE_TYPE_OPTIONS, ...TYPE_ALIASES])]; + + /** Write `value` at `field.path` on a fresh node object. */ + function nodeWith(type: string, entries: Array<[string[], unknown]>) { + const node: Record = { id: 'n', type }; + for (const [path, value] of entries) { + let cur = node; + for (const seg of path.slice(0, -1)) { + if (typeof cur[seg] !== 'object' || cur[seg] === null) cur[seg] = {}; + cur = cur[seg] as Record; + } + cur[path[path.length - 1]] = value; + } + return node; + } + + const gatedByType = new Map(); + for (const type of TYPES) { + const gated = fieldsForNodeType(type).filter((f) => f.showWhen); + if (gated.length) gatedByType.set(type, gated); + } + + 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 + const totalGated = [...gatedByType.values()].reduce((n, fs) => n + fs.length, 0); + expect(totalGated).toBe(33); + // 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).toContain('create_record'); + expect(ungatedTypes).toContain('connector_action'); + }); + + it('every gated field in every group flags when stored + controller unmet, and not when empty', () => { + let checked = 0; + for (const [type, gated] of gatedByType) { + const fields = fieldsForNodeType(type); + for (const f of gated) { + const controller = fields.find((c) => c.id === f.showWhen!.field); + // Drive the controller to a value its `equals` does NOT contain. For the + // `__legacy__` sentinel there is no controller and `equals` is empty, so + // nothing needs driving — it never admits. + const entries: Array<[string[], unknown]> = []; + if (controller) { + const off = controller.kind === 'boolean' ? false : '__not_a_declared_option__'; + entries.push([controller.path, off]); + } + // 1. empty dependent → not retained (and, for a real controller, hidden) + const emptyNode = nodeWith(type, entries); + expect(inactiveRetainedKind(f, emptyNode, fields), `${type}/${f.id} empty`).toBeNull(); + + // 2. stored dependent → visible AND flagged + const storedNode = nodeWith(type, [...entries, [f.path, f.kind === 'boolean' ? true : 'stored-value']]); + expect(isFieldVisible(f, storedNode, fields), `${type}/${f.id} visible`).toBe(true); + expect(inactiveRetainedKind(f, storedNode, fields), `${type}/${f.id} flagged`).toBe( + controller ? 'controller-off' : 'no-controller', + ); + + // 3. stored dependent + controller ADMITS → visible and NOT flagged + if (controller && f.showWhen!.equals.length > 0) { + const on = f.showWhen!.equals[0]; + const onValue = controller.kind === 'boolean' ? on === 'true' : on; + const liveNode = nodeWith(type, [[controller.path, onValue], [f.path, f.kind === 'boolean' ? true : 'stored-value']]); + expect(isFieldVisible(f, liveNode, fields), `${type}/${f.id} live visible`).toBe(true); + expect(inactiveRetainedKind(f, liveNode, fields), `${type}/${f.id} live not flagged`).toBeNull(); + } + checked += 1; + } + } + expect(checked).toBe(33); + }); +}); + +describe('inactiveRetainedKind — the two DERIVED producers, not just the static table', () => { + // An engine-published configSchema and a connector descriptor both mint + // `showWhen` groups at runtime, and both funnel through the same filter in + // FlowNodeInspector — so the affordance covers groups that do not exist in + // any source file. + it('covers a group derived from an engine configSchema', () => { + const fields = jsonSchemaToFlowFields({ + type: 'object', + properties: { + retry: { type: 'object', properties: { enabled: { type: 'boolean' }, attempts: { type: 'integer' } } }, + }, + })!; + const attempts = field(fields, 'retry.attempts'); + expect(attempts.showWhen).toEqual({ field: 'retry.enabled', equals: ['true'] }); + expect(inactiveRetainedKind(attempts, { id: 'n', type: 'x', config: { retry: { enabled: false, attempts: 3 } } }, fields)).toBe('controller-off'); + expect(inactiveRetainedKind(attempts, { id: 'n', type: 'x', config: { retry: { enabled: true, attempts: 3 } } }, fields)).toBeNull(); + }); + + it('covers a group derived from a connector input schema', () => { + const form = connectorInputFields({ + type: 'object', + properties: { tls: { type: 'object', properties: { enabled: { type: 'boolean' }, caCert: { type: 'string' } } } }, + })!; + const caCert = field(form.fields, 'connectorConfig.input.tls.caCert'); + const node = { id: 'n', type: 'connector_action', connectorConfig: { input: { tls: { enabled: false, caCert: 'PEM' } } } }; + expect(inactiveRetainedKind(caCert, node, form.fields)).toBe('controller-off'); + const on = { id: 'n', type: 'connector_action', connectorConfig: { input: { tls: { enabled: true, caCert: 'PEM' } } } }; + expect(inactiveRetainedKind(caCert, on, form.fields)).toBeNull(); + }); +}); + +/** + * The half that must NOT move. `isFieldVisible` keeps re-showing stored values; + * the ruling forbids touching that rule, and forbids any prune. These assert + * the non-change directly, so a later "cleanup" that quietly turns the + * affordance into a deletion fails here rather than in production. + */ +describe('the stored-value re-show rule is unchanged', () => { + const fields = approval(); + + it('still shows a hidden-but-stored field (the rule the prune option would have inverted)', () => { + const node = { id: 'a', type: 'approval', config: { escalation: { enabled: false, timeoutHours: 24 } } }; + expect(isFieldVisible(field(fields, 'escalation.timeoutHours'), node, fields)).toBe(true); + }); + + it('reading the affordance does not mutate the node', () => { + const node = { id: 'a', type: 'approval', config: { escalation: { enabled: false, timeoutHours: 24 } } }; + const before = JSON.stringify(node); + inactiveRetainedKind(field(fields, 'escalation.timeoutHours'), node, fields); + isFieldVisible(field(fields, 'escalation.timeoutHours'), node, fields); + expect(JSON.stringify(node)).toBe(before); + }); +}); 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 f1388507c..92c95b6ae 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 @@ -1084,8 +1084,44 @@ export function isFieldVisible( fields: FlowConfigField[], ): boolean { if (!field.showWhen) return true; + // ⛔ THE STORED-VALUE RE-SHOW RULE — do not weaken it, and do not invert it + // into a prune. objectui#6499 ruled (maintainer, 2026-08-27, Option C) that a + // hidden-but-stored dependent value is KEPT: the author sees it and clears it + // deliberately. Deleting it on save is the rejected option precisely because + // this line is what stops config an author entered from vanishing unseen. + if (hasStoredValue(field, node)) return true; + return controllerAdmits(field, node, fields); +} + +/** + * Whether this field currently holds a stored value — the input to the + * re-show rule above, named so {@link inactiveRetainedKind} asks the same + * question in the same words rather than re-deriving "empty". + */ +function hasStoredValue( + field: FlowConfigField, + node: Record | null | undefined, +): boolean { const own = getFieldValue(node, field); - if (own !== undefined && own !== null && own !== '') return true; + return own !== undefined && own !== null && own !== ''; +} + +/** + * Whether a gated field's CONTROLLER admits it — the `showWhen` predicate on + * its own, deliberately blind to the field's own stored value. + * + * Split out of {@link isFieldVisible} (behaviour unchanged — it is that + * function's former tail) so the affordance below and the visibility filter + * read ONE definition of "the controller says yes". Duplicating the resolution + * would let the two drift, and the drift would be silent: the affordance would + * quietly stop matching the fields it is supposed to annotate. + */ +function controllerAdmits( + field: FlowConfigField, + node: Record | null | undefined, + fields: FlowConfigField[], +): boolean { + if (!field.showWhen) return true; const controller = fields.find((f) => f.id === field.showWhen!.field); if (!controller) return false; const raw = getFieldValue(node, controller); @@ -1095,6 +1131,48 @@ export function isFieldVisible( return typeof value === 'string' && field.showWhen.equals.includes(value); } +/** + * Why a gated field is on screen when its controller does not admit it. + * + * `'controller-off'` — the descriptor names a controller that EXISTS in this + * node's field set and currently resolves to something outside `equals`. The + * author can turn it back on, so the affordance says so. + * + * `'no-controller'` — the descriptor's `showWhen.field` resolves to no field + * at all. The `__legacy__` sentinel (`equals: []`, no such field) is the + * deliberate instance: a render-only key that is never offered for fresh + * authoring and appears solely because a value is stored. There is nothing to + * switch back on, so the affordance must NOT tell the author to go find a + * toggle — that would be a new small lie on a screen whose whole defect was + * showing inert config as if it were live. + */ +export type InactiveRetainedKind = 'controller-off' | 'no-controller'; + +/** + * The "inactive values retained" predicate (objectui#6499, Option C). + * + * True exactly when a field is rendered ONLY because {@link isFieldVisible}'s + * stored-value re-show rule fired — i.e. the value is stored, the controller + * does not admit it, and so what the author sees is retained-but-inert config. + * Returns `null` for every other field, including a gated field the controller + * genuinely admits and a gated field holding nothing. + * + * This is a READ. It changes no stored value, and nothing on the save path + * consults it; clearing is an ordinary author-initiated field commit through + * the inspector's existing `setField`, exactly as if the author had emptied the + * control by hand. + */ +export function inactiveRetainedKind( + field: FlowConfigField, + node: Record | null | undefined, + fields: FlowConfigField[], +): InactiveRetainedKind | null { + if (!field.showWhen) return null; + if (!hasStoredValue(field, node)) return null; + if (controllerAdmits(field, node, fields)) return null; + return fields.some((f) => f.id === field.showWhen!.field) ? 'controller-off' : 'no-controller'; +} + /** Node types offered in the inspector's type picker (spec `FlowNodeAction`). */ export const FLOW_NODE_TYPE_OPTIONS = [ 'start',