diff --git a/.changeset/connector-input-schema-typed-fields-4305.md b/.changeset/connector-input-schema-typed-fields-4305.md new file mode 100644 index 000000000..7bd5feddb --- /dev/null +++ b/.changeset/connector-input-schema-typed-fields-4305.md @@ -0,0 +1,13 @@ +--- +'@object-ui/app-shell': patch +--- + +the connector node's Input section derives typed fields from the action descriptor's `inputSchema` + +A `connector_action` node published its input contract and the designer ignored it. `GET /api/v1/automation/connectors` serves each action's `inputSchema` — the connector's own JSON Schema, projected verbatim by the engine from `ConnectorActionSchema.inputSchema` — and nothing in `app-shell` read it: `git grep inputSchema` over the package found a field declaration and a comment. So after committing an action, the Input section stayed a single untyped key/value repeater, and the author typed raw key names against a contract the picker beside it already knew. The connector and action pickers were correct throughout; this was the last untyped step of that flow. + +The mapping is not a new one. `json-schema-to-fields` — the resolver the inspector already uses for a node type's engine-published `configSchema` — speaks exactly this language, so the descriptor's schema goes through it unchanged and a small adapter only re-roots what comes back: that resolver hard-roots every field at `config.`, while a connector's inputs live in the spec-structured sibling block `connectorConfig.input`, which is what the executor reads. Nothing here interprets JSON Schema a second time. A property the resolver declines — an `array` with no `items` (Slack's `blocks`), a bare `{type:'object'}` (REST's `headers`), a union — is not claimed either, so no descriptor can make a stored key unreachable. + +The stored map is the constraint that shapes the rest. Typed fields read and write the SAME `connectorConfig.input` map an existing flow already committed, key by key, so editing one input leaves every other key — declared or not — at its stored value and in its stored position. `additionalProperties` decides what happens beside them, measured rather than assumed: no shipped connector emits the key at all, JSON Schema's default for an absent one is open, and the executor passes the whole map to the handler unvalidated, so undeclared keys really are accepted. An open schema therefore keeps the repeater alongside the typed fields, trimmed to exactly the keys they do not own and merging its commit back over them instead of replacing the map. A closed one (`additionalProperties: false`) drops the repeater — unless the stored map still holds undeclared keys, because hiding config an older flow committed is worse than offering an editor the new schema no longer invites. + +Descriptors that publish no `inputSchema`, an unreachable registry, a node with no action chosen yet, and an array-shaped input are all left exactly as they were: the generic repeater, unchanged. Field labels and help come from the descriptor's own `title` and `description` — the connector's i18n channel — so no UI copy was added. JSON Schema `required` is read by the engine at dispatch and is still not represented in the form; the field model has no requiredness concept to carry it. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.connectorInput.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.connectorInput.test.tsx new file mode 100644 index 000000000..835fd49ad --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.connectorInput.test.tsx @@ -0,0 +1,302 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * FlowNodeInspector — a committed connector action's Input section derives typed + * fields from the descriptor's `inputSchema` (#4305). + * + * The card's repro, driven through the REAL render path: a `connector_action` + * node with a committed connector + action, against a runtime registry + * (`GET /api/v1/automation/connectors`) whose action declares an `inputSchema`. + * Before the fix the Input section is a single untyped key/value repeater and no + * schema-derived field exists. + * + * The pins that must NOT move are here too: a descriptor with no `inputSchema` + * keeps the byte-identical repeater, an unresolved connector keeps it, and every + * edit round-trips through the SAME `connectorConfig.input` map (declared keys, + * undeclared extras and their order all survive). + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, fireEvent, waitFor } from '@testing-library/react'; + +vi.mock('../previews/useFlowNodePalette', async (orig) => ({ + ...(await orig>()), + useActionConfigSchemas: () => ({}), + useFlowNodePalette: () => [], +})); +vi.mock('../previews/useObjectFields', () => ({ + useObjectFields: () => ({ fields: [], loading: false, error: null }), +})); + +import { FlowNodeInspector } from './FlowNodeInspector'; +import type { MetadataSelection } from '../preview-registry'; + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); +}); + +/** slack-connector.ts `chat.postMessage`, verbatim (no additionalProperties). */ +const SLACK_POST_MESSAGE = { + type: 'object', + required: ['channel'], + properties: { + channel: { type: 'string', description: 'Channel id, user id, or #name' }, + text: { type: 'string', description: 'Message text' }, + thread_ts: { type: 'string', description: 'Thread root ts to reply into' }, + blocks: { type: 'array', description: 'Block Kit blocks' }, + }, +}; + +function mockRegistry(actions: unknown[]) { + vi.stubGlobal( + 'fetch', + vi.fn(async () => + new Response( + JSON.stringify({ + success: true, + data: { + connectors: [ + { name: 'slack', label: 'Slack', type: 'saas', origin: 'plugin', state: 'ready', actions }, + ], + }, + }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ), + ), + ); +} + +/** A committed connector action: connector + action chosen, inputs stored. */ +function makeDraft(input: unknown = { channel: 'C123', legacy_note: 'keep me' }, actionId = 'chat.postMessage') { + return { + nodes: [ + { id: 'start', type: 'start' }, + { + id: 'post', + type: 'connector_action', + label: 'Post to Slack', + connectorConfig: { connectorId: 'slack', actionId, ...(input === undefined ? {} : { input }) }, + }, + ], + edges: [{ source: 'start', target: 'post' }], + }; +} + +const SELECTION: MetadataSelection = { kind: 'node', id: 'post' }; + +function renderInspector(draft: Record) { + const onPatch = vi.fn(); + const utils = render( + , + ); + return { onPatch, ...utils }; +} + +/** + * The input bound to a TYPED field, found via its label. + * + * Deliberately not `getByDisplayValue(...)`: the untyped repeater also renders + * the stored value in a cell, so a bare display-value probe passes on the + * UNFIXED code and can never go red (measured — it did, on the red-first run). + * Going through the label is what separates "a typed Channel field holds C123" + * from "some repeater row happens to show C123". + */ +function typedFieldInput(labelText: string): HTMLInputElement { + const label = screen.getByText(labelText); + const input = label.parentElement?.querySelector('input'); + if (!input) throw new Error(`no input under the "${labelText}" field label`); + return input as HTMLInputElement; +} + +/** The node as the inspector would write it back, read off the last onPatch. */ +function patchedInput(onPatch: ReturnType): unknown { + expect(onPatch).toHaveBeenCalled(); + const patch = onPatch.mock.calls.at(-1)![0] as { nodes?: Array> }; + const node = (patch.nodes ?? []).find((n) => n.id === 'post')!; + return (node.connectorConfig as Record | undefined)?.input; +} + +describe('#4305 — the Input section derives typed fields from the descriptor inputSchema', () => { + it('renders a typed field per declared input key, labelled from the descriptor', async () => { + mockRegistry([{ key: 'chat.postMessage', label: 'Post Message', inputSchema: SLACK_POST_MESSAGE }]); + renderInspector(makeDraft()); + + expect(await screen.findByText('Channel')).toBeTruthy(); + expect(screen.getByText('Text')).toBeTruthy(); + expect(screen.getByText('Thread Ts')).toBeTruthy(); + // The descriptor's own `description` is the label/help channel — no new UI copy. + expect(screen.getByText('Channel id, user id, or #name')).toBeTruthy(); + }); + + it('renders a STORED input value into its typed field (not just an empty form)', async () => { + mockRegistry([{ key: 'chat.postMessage', label: 'Post Message', inputSchema: SLACK_POST_MESSAGE }]); + renderInspector(makeDraft()); + + await screen.findByText('Channel'); + expect(typedFieldInput('Channel').value).toBe('C123'); + // An unset declared key renders as an empty typed field, not as nothing. + expect(typedFieldInput('Text').value).toBe(''); + }); + + it('editing a typed field writes back into the SAME input map, preserving every other key', async () => { + mockRegistry([{ key: 'chat.postMessage', label: 'Post Message', inputSchema: SLACK_POST_MESSAGE }]); + const { onPatch } = renderInspector(makeDraft()); + + await screen.findByText('Channel'); + fireEvent.change(typedFieldInput('Channel'), { target: { value: 'C999' } }); + + expect(patchedInput(onPatch)).toEqual({ channel: 'C999', legacy_note: 'keep me' }); + }); + + it('filling a previously-unset declared key ADDS it without disturbing the stored keys', async () => { + mockRegistry([{ key: 'chat.postMessage', label: 'Post Message', inputSchema: SLACK_POST_MESSAGE }]); + const { onPatch } = renderInspector(makeDraft()); + + await screen.findByText('Text'); + fireEvent.change(typedFieldInput('Text'), { target: { value: 'hello' } }); + + expect(patchedInput(onPatch)).toEqual({ channel: 'C123', legacy_note: 'keep me', text: 'hello' }); + }); + + it('an undeclared stored key stays editable in the extras repeater (schema is OPEN)', async () => { + mockRegistry([{ key: 'chat.postMessage', label: 'Post Message', inputSchema: SLACK_POST_MESSAGE }]); + renderInspector(makeDraft()); + + // Two settlings to wait out, not one: the registry fetch (which adds the + // typed fields), and THEN the repeater's own resync — it keeps its rows in + // local draft state and rebuilds them from the trimmed value one render + // later. Probing between the two reads the pre-trim rows and would fail + // against correct behaviour (measured: it did). + await screen.findByText('Channel'); + // …never re-offering a key that now has its own typed field. + await waitFor(() => expect(screen.queryByDisplayValue('channel')).toBeNull()); + // The repeater survives, holding ONLY the undeclared key. + expect(screen.getByDisplayValue('legacy_note')).toBeTruthy(); + }); + + it('editing an extra through the repeater preserves the typed keys (the commit MERGES, never replaces)', async () => { + mockRegistry([{ key: 'chat.postMessage', label: 'Post Message', inputSchema: SLACK_POST_MESSAGE }]); + const { onPatch } = renderInspector(makeDraft()); + + await screen.findByText('Channel'); + // Grab the cell only once the repeater has resynced to the extras-only rows + // — the pre-resync element is replaced, so editing it would commit nothing. + await waitFor(() => expect(screen.queryByDisplayValue('channel')).toBeNull()); + const extraValue = screen.getByDisplayValue('keep me'); + fireEvent.change(extraValue, { target: { value: 'edited' } }); + fireEvent.blur(extraValue); + + expect(patchedInput(onPatch)).toEqual({ channel: 'C123', legacy_note: 'edited' }); + }); + + it('a CLOSED schema (additionalProperties:false) drops the repeater', async () => { + mockRegistry([ + { + key: 'chat.postMessage', + label: 'Post Message', + inputSchema: { ...SLACK_POST_MESSAGE, additionalProperties: false }, + }, + ]); + renderInspector(makeDraft({ channel: 'C123' })); + + expect(await screen.findByText('Channel')).toBeTruthy(); + // No key cell at all — the repeater is gone, not merely empty. + expect(screen.queryByPlaceholderText('Key')).toBeNull(); + }); + + it('a CLOSED schema still shows stored extras, so existing config is never hidden', async () => { + mockRegistry([ + { + key: 'chat.postMessage', + label: 'Post Message', + inputSchema: { ...SLACK_POST_MESSAGE, additionalProperties: false }, + }, + ]); + renderInspector(makeDraft()); + + await screen.findByText('Channel'); + // …while the declared key it DOES cover has moved to its typed field. + await waitFor(() => expect(screen.queryByDisplayValue('channel')).toBeNull()); + // The legacy key the closed schema does not declare is still reachable… + expect(screen.getByDisplayValue('legacy_note')).toBeTruthy(); + expect(typedFieldInput('Channel').value).toBe('C123'); + }); +}); + +describe('#4305 — pins that must NOT move', () => { + it('a descriptor with NO inputSchema keeps the generic repeater, showing every key', async () => { + mockRegistry([{ key: 'plain', label: 'Plain' }]); + renderInspector(makeDraft({ channel: 'C123', legacy_note: 'keep me' }, 'plain')); + + expect(await screen.findByDisplayValue('channel')).toBeTruthy(); + expect(screen.getByDisplayValue('legacy_note')).toBeTruthy(); + // …and no schema-derived field appeared. + expect(screen.queryByText('Thread Ts')).toBeNull(); + }); + + it('a no-schema descriptor still commits the whole map through the repeater', async () => { + mockRegistry([{ key: 'plain', label: 'Plain' }]); + const { onPatch } = renderInspector(makeDraft({ channel: 'C123' }, 'plain')); + + const valueCell = await screen.findByDisplayValue('C123'); + fireEvent.change(valueCell, { target: { value: 'C999' } }); + fireEvent.blur(valueCell); + + expect(patchedInput(onPatch)).toEqual({ channel: 'C999' }); + }); + + it('an unreachable registry leaves the repeater exactly as it was', async () => { + vi.stubGlobal('fetch', vi.fn(async () => new Response('nope', { status: 500 }))); + renderInspector(makeDraft()); + + expect(await screen.findByDisplayValue('channel')).toBeTruthy(); + expect(screen.queryByText('Thread Ts')).toBeNull(); + }); + + it('a node with no action committed yet keeps the repeater', async () => { + mockRegistry([{ key: 'chat.postMessage', label: 'Post Message', inputSchema: SLACK_POST_MESSAGE }]); + render( + , + ); + + expect(await screen.findByDisplayValue('channel')).toBeTruthy(); + expect(screen.queryByText('Thread Ts')).toBeNull(); + }); + + it('an ARRAY-shaped input is left wholly to the repeater (never coerced to an object)', async () => { + mockRegistry([{ key: 'chat.postMessage', label: 'Post Message', inputSchema: SLACK_POST_MESSAGE }]); + renderInspector(makeDraft([{ variable: 'channel', value: 'C123' }])); + + expect(await screen.findByDisplayValue('channel')).toBeTruthy(); + expect(screen.queryByText('Thread Ts')).toBeNull(); + }); + + it('the connector + action pickers still render (the picker behaviours are untouched)', async () => { + mockRegistry([{ key: 'chat.postMessage', label: 'Post Message', inputSchema: SLACK_POST_MESSAGE }]); + renderInspector(makeDraft()); + + await waitFor(() => expect(screen.getByText('Connector')).toBeTruthy()); + expect(screen.getByText('Action')).toBeTruthy(); + expect(screen.getByDisplayValue('slack')).toBeTruthy(); + expect(screen.getByDisplayValue('chat.postMessage')).toBeTruthy(); + }); +}); 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 a41062477..b8159e4bb 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.tsx @@ -41,6 +41,14 @@ import { } from './flow-node-config'; import { translateNodeLabel } from '../i18n'; import { jsonSchemaToFlowFields } from './json-schema-to-fields'; +import { + applyConnectorInputForm, + connectorActionInputSchema, + connectorInputExtras, + connectorInputFields, + mergeConnectorInputExtras, + useConnectorRegistry, +} from './connector-input-fields'; import { applyDecisionBranches, syncDecisionEdgesByOrder, withBranchTargets } from './flow-decision-edges'; import { useActionConfigSchemas } from '../previews/useFlowNodePalette'; import { FlowNodeConfigField } from './FlowNodeConfigField'; @@ -137,18 +145,52 @@ export function FlowNodeInspector({ selection, draft, onPatch, onClearSelection, ); // In-scope variable references for this node, for the data-picker (#1934). const { groups: scopeGroups, approvalExpressionGroups } = useFlowScope(draft as Record, loc?.scopeAnchorId, nestedLoopRefs); + // #4305 — a COMMITTED connector action (connector + action both chosen) types + // its Input section from that action's descriptor `inputSchema`. Read the + // committed pair and the stored input map off the node's spec-structured + // `connectorConfig` block: both are needed before any registry fetch, and the + // stored map decides whether a closed schema still needs the repeater. + const { connectorId, actionId, storedInput } = React.useMemo<{ + connectorId?: string; + actionId?: string; + storedInput?: unknown; + }>(() => { + const cc = node?.connectorConfig; + if (!cc || typeof cc !== 'object' || Array.isArray(cc)) return {}; + const block = cc as Record; + return { + connectorId: typeof block.connectorId === 'string' && block.connectorId ? block.connectorId : undefined, + actionId: typeof block.actionId === 'string' && block.actionId ? block.actionId : undefined, + storedInput: block.input, + }; + }, [node]); + const connectors = useConnectorRegistry(!!connectorId && !!actionId); + const connectorInput = React.useMemo( + () => connectorInputFields(connectorActionInputSchema(connectors, connectorId, actionId)), + [connectors, connectorId, actionId], + ); + // Hoisted so this memo reads only the node's TYPE, never the node object: with + // `node?.type` inline the compiler infers a dependency on all of `node` while + // the declared dep is the narrower `node?.type`, and it then declines to + // memoize the form at all ("existing memoization could not be preserved"). + const nodeType = node?.type; const fields = React.useMemo(() => { - const schema = node?.type ? configSchemas[node.type] : undefined; + const schema = nodeType ? configSchemas[nodeType] : undefined; const serverFields = schema !== undefined ? jsonSchemaToFlowFields(schema) : null; // A published configSchema describes `node.config` ONLY, so it replaces just // the config-rooted fields — the spec-structured sibling blocks // (connectorConfig / waitEventConfig / boundaryConfig) and top-level // `timeoutMs` are always kept from the hand-written group (framework#4045). - const resolved = mergeServerFlowFields(serverFields, node?.type); + const resolved = mergeServerFlowFields(serverFields, nodeType); // Localize both the hardcoded table and the engine-published configSchema // fields (they share field ids for built-in nodes); no-op for English. - return localizeFlowFields(node?.type, resolved, locale); - }, [configSchemas, node?.type, locale]); + const localized = localizeFlowFields(nodeType, resolved, locale); + // Applied AFTER localization on purpose: the typed input fields are labelled + // by the DESCRIPTOR (its `title` / `description`) — the connector's own i18n + // channel — so they must not be overlaid from the client's zh table, while + // the extras repeater keeps the localized "Input" label it always had. + return applyConnectorInputForm(localized, connectorInput, storedInput); + }, [configSchemas, nodeType, locale, connectorInput, storedInput]); const config = asConfig(node); const visibleFields = fields.filter((f) => isFieldVisible(f, node, fields)); @@ -221,6 +263,17 @@ export function FlowNodeInspector({ selection, draft, onPatch, onClearSelection, const path = field.path; let stored = value; let nextEdges: FlowEdge[] | undefined; + // #4305 — the Input repeater standing beside typed fields edits only the + // undeclared extras, but its commit REPLACES whatever map it was handed. Fold + // the edit back over the stored map so the typed keys (and the stored key + // order) survive: without this, one extras edit would wipe every typed input. + if (field.omitKeys) { + stored = mergeConnectorInputExtras( + getFieldValue(node, field), + (value && typeof value === 'object' && !Array.isArray(value) ? (value as Record) : {}), + field.omitKeys, + ); + } // Decision→edge mirroring is TOP-LEVEL only. A top-level decision drives // routing via its out-edges (the engine/simulator read edge.condition, not // node.config.conditions), and the Branches editor's Target column (#1942) @@ -357,7 +410,11 @@ export function FlowNodeInspector({ selection, draft, onPatch, onClearSelection, getFieldValue(node, field), Array.isArray((draft as { edges?: unknown }).edges) ? ((draft as { edges: FlowEdge[] }).edges) : [], ) - : getFieldValue(node, effField); + : effField.omitKeys + // #4305 — show this repeater only the keys the typed sibling + // fields do not own; `setField` merges its commit back. + ? connectorInputExtras(getFieldValue(node, effField), effField.omitKeys) + : getFieldValue(node, effField); return ( { + it('reads the chosen action’s inputSchema out of the registry payload', () => { + expect(connectorActionInputSchema(CONNECTORS, 'slack', 'chat.postMessage')).toEqual(SLACK_POST_MESSAGE); + }); + + it('returns undefined for an action that declares none, an unknown action, an unknown connector, and a non-list payload', () => { + expect(connectorActionInputSchema(CONNECTORS, 'slack', 'noSchema')).toBeUndefined(); + expect(connectorActionInputSchema(CONNECTORS, 'slack', 'nope')).toBeUndefined(); + expect(connectorActionInputSchema(CONNECTORS, 'jira', 'chat.postMessage')).toBeUndefined(); + expect(connectorActionInputSchema(CONNECTORS, undefined, 'chat.postMessage')).toBeUndefined(); + expect(connectorActionInputSchema(CONNECTORS, 'slack', undefined)).toBeUndefined(); + expect(connectorActionInputSchema(null, 'slack', 'chat.postMessage')).toBeUndefined(); + }); +}); + +describe('connectorInputFields — re-rooting onto connectorConfig.input', () => { + it('maps the slack schema to typed fields rooted at connectorConfig.input.', () => { + const form = connectorInputFields(SLACK_POST_MESSAGE); + expect(form).not.toBeNull(); + // `blocks` is an `array` with no `items` — the resolver declines it (it has + // no representable list kind), so it is absent here and stays editable in + // the extras repeater. That is the fallback working, not a gap. + expect(form!.fields.map((f) => [f.id, f.path.join('.'), f.kind, f.label])).toEqual([ + ['connectorConfig.input.channel', 'connectorConfig.input.channel', 'text', 'Channel'], + ['connectorConfig.input.text', 'connectorConfig.input.text', 'text', 'Text'], + ['connectorConfig.input.thread_ts', 'connectorConfig.input.thread_ts', 'text', 'Thread Ts'], + ]); + }); + + it('carries the descriptor’s own description through as the field help (the i18n channel)', () => { + const form = connectorInputFields(SLACK_POST_MESSAGE); + expect(form!.fields[0].help).toBe('Channel id, user id, or #name'); + }); + + it('prefers the schema `title` over the humanized key', () => { + const form = connectorInputFields({ + type: 'object', + properties: { api_key: { type: 'string', title: 'API key' } }, + }); + expect(form!.fields[0].label).toBe('API key'); + }); + + it('reports declaredKeys as the keys that actually got a typed field', () => { + const form = connectorInputFields(SLACK_POST_MESSAGE); + expect(form!.declaredKeys).toEqual(['channel', 'text', 'thread_ts']); + }); + + it('leaves an unmappable property OFF the form and OUT of declaredKeys (it stays repeater-editable)', () => { + // `oneOf` with no `type` is exactly the case the resolver declines to map; + // it must not be silently claimed, or the key becomes uneditable. + const form = connectorInputFields({ + type: 'object', + properties: { + ok: { type: 'string' }, + weird: { oneOf: [{ type: 'string' }, { type: 'number' }] }, + }, + }); + expect(form!.declaredKeys).toEqual(['ok']); + expect(form!.fields).toHaveLength(1); + }); + + it('flattens one level of nested object (the openapi-connector shape)', () => { + const form = connectorInputFields({ + type: 'object', + properties: { + path: { type: 'object', properties: { id: { type: 'string' } }, required: ['id'] }, + query: { type: 'object', properties: { limit: { type: 'number' } } }, + }, + required: ['path'], + }); + expect(form!.fields.map((f) => [f.id, f.path.join('.'), f.kind])).toEqual([ + ['connectorConfig.input.path.id', 'connectorConfig.input.path.id', 'text'], + ['connectorConfig.input.query.limit', 'connectorConfig.input.query.limit', 'number'], + ]); + // The flattened group is claimed by its TOP-level key, so the repeater does + // not also offer `path` / `query` as raw JSON cells. + expect(form!.declaredKeys).toEqual(['path', 'query']); + }); + + it('maps an `additionalProperties` map property to the keyValue widget', () => { + const form = connectorInputFields({ + type: 'object', + properties: { + headers: { type: 'object', additionalProperties: { type: 'string' }, description: 'Per-request headers' }, + }, + }); + expect(form!.fields[0].kind).toBe('keyValue'); + expect(form!.fields[0].path).toEqual(['connectorConfig', 'input', 'headers']); + }); + + it('declines a bare `{type:"object"}` property (rest-connector `headers`/`query`) — it stays repeater-editable', () => { + // No fixed `properties` AND no `additionalProperties`: the resolver has + // nothing to render, so the key must NOT be claimed as declared. + const form = connectorInputFields({ + type: 'object', + properties: { + method: { type: 'string' }, + headers: { type: 'object', description: 'Per-request headers' }, + body: {}, + }, + }); + expect(form!.declaredKeys).toEqual(['method']); + }); + + it('rewrites a showWhen controller reference onto the re-rooted id', () => { + const form = connectorInputFields({ + type: 'object', + properties: { + retry: { + type: 'object', + properties: { enabled: { type: 'boolean' }, attempts: { type: 'number' } }, + }, + }, + }); + const attempts = form!.fields.find((f) => f.id.endsWith('attempts'))!; + expect(attempts.showWhen).toEqual({ field: 'connectorConfig.input.retry.enabled', equals: ['true'] }); + // …and the controller it names really is in the same list. + expect(form!.fields.some((f) => f.id === attempts.showWhen!.field)).toBe(true); + }); + + it('returns null for a schema that is not a usable object schema', () => { + expect(connectorInputFields(undefined)).toBeNull(); + expect(connectorInputFields({})).toBeNull(); + expect(connectorInputFields({ type: 'string' })).toBeNull(); + expect(connectorInputFields({ type: 'object' })).toBeNull(); // no properties + }); + + it('CONNECTOR_INPUT_PATH is the spec block the executor reads', () => { + expect(CONNECTOR_INPUT_PATH).toEqual(['connectorConfig', 'input']); + }); +}); + +describe('connectorInputFields — measured additionalProperties semantics', () => { + it('treats an ABSENT additionalProperties as OPEN (every shipped connector omits it)', () => { + expect(connectorInputFields(SLACK_POST_MESSAGE)!.open).toBe(true); + }); + + it('treats `additionalProperties: false` as CLOSED', () => { + expect(connectorInputFields({ ...SLACK_POST_MESSAGE, additionalProperties: false })!.open).toBe(false); + }); + + it('treats `additionalProperties: true` / a value schema as OPEN', () => { + expect(connectorInputFields({ ...SLACK_POST_MESSAGE, additionalProperties: true })!.open).toBe(true); + expect( + connectorInputFields({ ...SLACK_POST_MESSAGE, additionalProperties: { type: 'string' } })!.open, + ).toBe(true); + }); +}); + +describe('connectorInputExtras / mergeConnectorInputExtras — the round-trip contract', () => { + const declared = ['channel', 'text']; + + it('extras are the stored keys the typed fields do NOT own', () => { + expect(connectorInputExtras({ channel: 'C1', text: 'hi', legacy_note: 'keep' }, declared)).toEqual({ + legacy_note: 'keep', + }); + }); + + it('extras of a non-object (or absent) stored value are empty', () => { + expect(connectorInputExtras(undefined, declared)).toEqual({}); + expect(connectorInputExtras([{ variable: 'a', value: 1 }], declared)).toEqual({}); + }); + + it('merging preserves the declared keys, their values, AND the stored key order', () => { + const stored = { channel: 'C1', legacy_note: 'keep', text: 'hi' }; + expect(mergeConnectorInputExtras(stored, { legacy_note: 'edited' }, declared)).toEqual({ + channel: 'C1', + legacy_note: 'edited', + text: 'hi', + }); + expect(Object.keys(mergeConnectorInputExtras(stored, { legacy_note: 'edited' }, declared)!)).toEqual([ + 'channel', + 'legacy_note', + 'text', + ]); + }); + + it('a removed extra is dropped while every declared key survives', () => { + expect(mergeConnectorInputExtras({ channel: 'C1', legacy_note: 'keep' }, {}, declared)).toEqual({ + channel: 'C1', + }); + }); + + it('a newly added extra is appended', () => { + expect(mergeConnectorInputExtras({ channel: 'C1' }, { fresh: 2 }, declared)).toEqual({ + channel: 'C1', + fresh: 2, + }); + }); + + it('collapses to undefined only when nothing at all is left (so the block is pruned)', () => { + expect(mergeConnectorInputExtras({}, {}, declared)).toBeUndefined(); + expect(mergeConnectorInputExtras({ channel: 'C1' }, {}, declared)).toEqual({ channel: 'C1' }); + }); +}); + +describe('applyConnectorInputForm — splicing into the connector_action group', () => { + const base = (): FlowConfigField[] => fieldsForNodeType('connector_action'); + + it('is a no-op when there is no form (no descriptor / no inputSchema)', () => { + const fields = base(); + expect(applyConnectorInputForm(fields, null)).toEqual(fields); + expect(applyConnectorInputForm(fields, null)).toBe(fields); + }); + + it('replaces the Input repeater IN PLACE, keeping the surrounding fields and their order', () => { + const form = connectorInputFields(SLACK_POST_MESSAGE)!; + const out = applyConnectorInputForm(base(), form); + expect(out.map((f) => f.id)).toEqual([ + 'connectorConfig.connectorId', + 'connectorConfig.actionId', + 'connectorConfig.input.channel', + 'connectorConfig.input.text', + 'connectorConfig.input.thread_ts', + 'connectorConfig.input', // the extras repeater, still open-schema + 'timeoutMs', + ]); + }); + + it('an OPEN schema keeps the repeater, carrying the declared keys as omitKeys', () => { + const form = connectorInputFields(SLACK_POST_MESSAGE)!; + const repeater = applyConnectorInputForm(base(), form).find((f) => f.id === 'connectorConfig.input')!; + expect(repeater.kind).toBe('keyValue'); + expect(repeater.omitKeys).toEqual(['channel', 'text', 'thread_ts']); + }); + + it('a CLOSED schema drops the repeater entirely', () => { + const form = connectorInputFields({ ...SLACK_POST_MESSAGE, additionalProperties: false })!; + const out = applyConnectorInputForm(base(), form); + expect(out.some((f) => f.id === 'connectorConfig.input')).toBe(false); + expect(out.map((f) => f.id)).toEqual([ + 'connectorConfig.connectorId', + 'connectorConfig.actionId', + 'connectorConfig.input.channel', + 'connectorConfig.input.text', + 'connectorConfig.input.thread_ts', + 'timeoutMs', + ]); + }); + + it('leaves a group that has no Input field untouched', () => { + const form = connectorInputFields(SLACK_POST_MESSAGE)!; + const httpFields = fieldsForNodeType('http_request'); + expect(applyConnectorInputForm(httpFields, form)).toBe(httpFields); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/connector-input-fields.ts b/packages/app-shell/src/views/metadata-admin/inspectors/connector-input-fields.ts new file mode 100644 index 000000000..9f7d84e99 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/connector-input-fields.ts @@ -0,0 +1,281 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * connector-input-fields — turn a connector action descriptor's `inputSchema` + * into the typed Input fields of a committed `connector_action` node (#4305). + * + * Before this, the Input section was a single generic key/value repeater: the + * descriptor published a JSON Schema for the action's inputs and nothing in + * `app-shell` read it, so an author typed raw key names against a contract the + * designer already knew. + * + * ## One resolver, not a second field generator + * + * The mapping itself is {@link jsonSchemaToFlowFields} — the same engine-schema + * resolver the inspector already uses for a node type's published `configSchema`. + * The descriptor's input language is the same language (measured below), so this + * module adds no interpreter: it only RE-ROOTS the resolver's output, because + * that resolver hard-roots every field it emits at `['config', ]` while a + * connector's inputs live in the spec-structured sibling block + * `node.connectorConfig.input` ({@link CONNECTOR_INPUT_PATH}) — which is what the + * executor reads (`service-automation` `connector-nodes.ts`: + * `handler((cfg.input ?? {}) as Record, handlerCtx)`). + * + * Anything the resolver declines to map (an `array` with no `items`, a bare + * `{type:'object'}`, a union) is simply not claimed here either — it stays + * editable in the repeater, so no descriptor can make a stored key unreachable. + * + * ## The measured schema language + * + * `ConnectorActionDescriptor.inputSchema` is `Record` typed and + * documented as JSON Schema (`@objectstack/spec` + * `integration/connector-descriptor.ts`), and the engine projects it VERBATIM + * from the connector's authored `ConnectorActionSchema.inputSchema`. Every + * shipped connector emits a plain object schema: + * • slack `{type:'object', required:['channel'], properties:{…string/array}}` + * • rest `{type:'object', properties:{ method, path, headers:{type:'object'}, … }}` + * • openapi `{type:'object', properties:{ path|query|header:{type:'object',properties}, body }, required}` + * • mcp the MCP tool's own `inputSchema`, passed straight through + * + * ⚠️ This is NOT the flow-node dialect of the same word: `flow.nodes[].inputSchema` + * (spec `automation/flow.zod.ts`) is `Record` + * — a per-key `required: true` map, not JSON Schema. They are different contracts + * that share a name; only the descriptor one is read here. + * + * ## additionalProperties — measured, then followed + * + * No shipped connector emits `additionalProperties` at all, and JSON Schema's + * default for an absent one is OPEN. The runtime agrees: the executor passes the + * whole input map to the handler unvalidated, so undeclared keys really are + * accepted. So the rule is the JSON-Schema rule — CLOSED iff + * `additionalProperties === false`, OPEN otherwise (absent, `true`, or a value + * schema) — and an open schema keeps the repeater alongside the typed fields for + * exactly the keys the typed fields don't own. + * + * A closed schema drops the repeater, EXCEPT when the stored map already holds + * undeclared keys: this package's standing rule is that a field is shown when it + * holds a stored value "so existing config is never hidden" + * ({@link isFieldVisible}), and silently hiding a key an older flow committed + * would be worse than offering an editor the new schema no longer invites. + * + * NOT represented: JSON Schema `required`. `FlowConfigField` has no requiredness + * concept, so the typed fields cannot mark it — a descriptor's `required: [...]` + * is read by the engine at dispatch, not enforced in this form. + */ + +import * as React from 'react'; +import { jsonSchemaToFlowFields } from './json-schema-to-fields'; +import type { FlowConfigField } from './flow-node-config'; +import { apiBase } from '../previews/useFlowNodePalette'; + +/** Where a connector node's mapped inputs live on the node (spec-structured). */ +export const CONNECTOR_INPUT_PATH: readonly string[] = ['connectorConfig', 'input']; + +/** Field-id prefix for the re-rooted inputs — matches the `block.key` convention. */ +const ID_PREFIX = CONNECTOR_INPUT_PATH.join('.'); + +/** The id of the generic Input repeater this form replaces / trims. */ +export const CONNECTOR_INPUT_FIELD_ID = ID_PREFIX; + +/** A descriptor's input contract, resolved into inspector fields. */ +export interface ConnectorInputForm { + /** Typed fields, rooted at `connectorConfig.input.*`, in schema order. */ + fields: FlowConfigField[]; + /** + * The top-level input keys the typed fields own — i.e. the keys the extras + * repeater must NOT re-offer. Derived from the fields actually emitted, so a + * property the resolver declined is absent here and stays repeater-editable. + */ + declaredKeys: string[]; + /** Whether undeclared keys are accepted (`additionalProperties !== false`). */ + open: boolean; +} + +function isPlainObject(v: unknown): v is Record { + return !!v && typeof v === 'object' && !Array.isArray(v); +} + +/** + * Find one action's `inputSchema` in a `GET /automation/connectors` payload + * (already unwrapped to the connector array). Returns undefined for any miss — + * unknown connector, unknown action, or an action that declares none — so the + * caller simply keeps the generic repeater. + */ +export function connectorActionInputSchema( + connectors: unknown, + connectorName: string | undefined, + actionKey: string | undefined, +): unknown { + if (!Array.isArray(connectors) || !connectorName || !actionKey) return undefined; + const connector = connectors.find( + (c) => isPlainObject(c) && c.name === connectorName, + ) as Record | undefined; + if (!connector || !Array.isArray(connector.actions)) return undefined; + const action = connector.actions.find( + (a) => isPlainObject(a) && a.key === actionKey, + ) as Record | undefined; + const schema = action?.inputSchema; + return isPlainObject(schema) ? schema : undefined; +} + +/** Re-root one resolver field from `config.*` onto `connectorConfig.input.*`. */ +function reroot(field: FlowConfigField): FlowConfigField { + const out: FlowConfigField = { + ...field, + id: `${ID_PREFIX}.${field.id}`, + // Drop the resolver's `config` root, keep the key (and any flattened + // sub-key) it appended. + path: [...CONNECTOR_INPUT_PATH, ...field.path.slice(1)], + }; + // A gated group's `showWhen` names its controller BY ID, so it has to follow + // the same re-rooting or it would point at a field that no longer exists. + if (field.showWhen) { + out.showWhen = { ...field.showWhen, field: `${ID_PREFIX}.${field.showWhen.field}` }; + } + return out; +} + +/** + * Resolve a descriptor `inputSchema` into the Input section's typed fields, or + * `null` when it is not a usable object schema (so the caller keeps the generic + * repeater unchanged). + */ +export function connectorInputFields(schema: unknown): ConnectorInputForm | null { + const raw = jsonSchemaToFlowFields(schema); + if (!raw || raw.length === 0) return null; + const fields = raw.map(reroot); + // The owned key is the first segment after `connectorConfig.input` — so a + // flattened group (`…input.retry.attempts`) is claimed by `retry`, once. + const declaredKeys: string[] = []; + for (const f of fields) { + const key = f.path[CONNECTOR_INPUT_PATH.length]; + if (key && !declaredKeys.includes(key)) declaredKeys.push(key); + } + const additional = isPlainObject(schema) ? (schema as { additionalProperties?: unknown }).additionalProperties : undefined; + return { fields, declaredKeys, open: additional !== false }; +} + +/** + * The stored input keys the typed fields do NOT own — what the repeater edits + * once a schema is in play. A non-object (or absent) stored value has none: the + * array form some key/value fields tolerate is not a legal connector input + * (spec: `input: z.record(z.string(), z.unknown())`), and is left wholly alone. + */ +export function connectorInputExtras( + stored: unknown, + declaredKeys: readonly string[], +): Record { + if (!isPlainObject(stored)) return {}; + const out: Record = {}; + for (const [k, v] of Object.entries(stored)) { + if (!declaredKeys.includes(k)) out[k] = v; + } + return out; +} + +/** + * Fold an edited extras map back into the full stored input map — the round-trip + * contract. The repeater's own commit REPLACES whatever it was given, so handing + * it the extras subset without this merge would drop every typed key on the + * first extras edit. + * + * Stored key ORDER is preserved (declared keys keep their values and positions, + * edited extras keep theirs, removed extras drop out, new ones append) so an + * untouched key is written back exactly as it was read. + */ +export function mergeConnectorInputExtras( + stored: unknown, + extras: Record, + declaredKeys: readonly string[], +): Record | undefined { + const base = isPlainObject(stored) ? stored : {}; + const out: Record = {}; + for (const [k, v] of Object.entries(base)) { + if (declaredKeys.includes(k)) out[k] = v; + else if (Object.prototype.hasOwnProperty.call(extras, k)) out[k] = extras[k]; + } + for (const [k, v] of Object.entries(extras)) { + if (!Object.prototype.hasOwnProperty.call(out, k)) out[k] = v; + } + return Object.keys(out).length ? out : undefined; +} + +/** + * Splice a resolved form into a node's field list, replacing the generic Input + * repeater IN PLACE (so the connector / action pickers above it and `timeoutMs` + * below it keep their positions). + * + * The repeater is kept after the typed fields when the schema is open, or when + * the stored map still holds undeclared keys (never hide existing config); it + * then carries `omitKeys` so it edits only what the typed fields don't own. + * Returns the input array unchanged (same reference) when there is nothing to do. + */ +export function applyConnectorInputForm( + fields: FlowConfigField[], + form: ConnectorInputForm | null, + storedInput?: unknown, +): FlowConfigField[] { + if (!form) return fields; + // The array form is not a legal connector input; typed fields would have to + // objectify it to write, so the whole map is left to the repeater instead. + if (Array.isArray(storedInput)) return fields; + const at = fields.findIndex((f) => f.id === CONNECTOR_INPUT_FIELD_ID); + if (at === -1) return fields; + const repeater = fields[at]; + const hasExtras = Object.keys(connectorInputExtras(storedInput, form.declaredKeys)).length > 0; + const keepRepeater = form.open || hasExtras; + return [ + ...fields.slice(0, at), + ...form.fields, + ...(keepRepeater ? [{ ...repeater, omitKeys: form.declaredKeys }] : []), + ...fields.slice(at + 1), + ]; +} + +/** + * The runtime connector registry (`GET /api/v1/automation/connectors`) — the + * same list the connector / action pickers follow, so the Input section can only + * ever type itself from a descriptor those pickers would also offer. + * + * `enabled === false` skips the fetch entirely (the hook stays unconditional). + * Any failure degrades to `[]`, which resolves to no schema and therefore to the + * unchanged generic repeater — offline, older backend, or plugin absent. + */ +export function useConnectorRegistry(enabled: boolean): unknown[] { + const [connectors, setConnectors] = React.useState([]); + + React.useEffect(() => { + // No synchronous reset on the disabled edge: setting state straight from an + // effect body cascades a render, and there is nothing to reset for — with no + // committed connector/action, `connectorActionInputSchema` resolves to + // undefined regardless of what this list still holds, so the Input section + // falls back to the generic repeater either way. + if (!enabled) return; + let cancelled = false; + const controller = new AbortController(); + (async () => { + try { + const res = await fetch(`${apiBase()}/automation/connectors`, { + credentials: 'include', + headers: { Accept: 'application/json' }, + signal: controller.signal, + }); + if (!res.ok) return; + const payload = (await res.json()) as { + data?: { connectors?: unknown[] }; + connectors?: unknown[]; + }; + const list = payload?.data?.connectors ?? payload?.connectors ?? []; + if (!cancelled && Array.isArray(list)) setConnectors(list); + } catch { + /* offline / aborted — the generic repeater stays */ + } + })(); + return () => { + cancelled = true; + controller.abort(); + }; + }, [enabled]); + + return connectors; +} 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 8b526b3cf..97dd88ac4 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 @@ -233,6 +233,16 @@ export interface FlowConfigField { showWhen?: { field: string; equals: string[] }; /** Column schema for `objectList` fields (array-of-objects repeater). */ columns?: FlowConfigColumn[]; + /** + * For a `keyValue` field that shares its map with typed sibling fields: the + * keys those siblings own, which this editor must neither show nor overwrite + * (#4305). A connector action whose descriptor publishes an `inputSchema` gets + * a typed field per declared key, and the repeater stays behind — bound to the + * SAME `connectorConfig.input` map — for the undeclared extras only. The host + * inspector filters the value it passes down and merges the commit back, so + * the stored map round-trips whole. + */ + omitKeys?: string[]; /** Reference target for `reference` fields — drives the combobox data source. */ ref?: FlowReferenceSpec; /**