diff --git a/.changeset/27828-flow-node-config-finding-path.md b/.changeset/27828-flow-node-config-finding-path.md new file mode 100644 index 00000000000..fdba204303e --- /dev/null +++ b/.changeset/27828-flow-node-config-finding-path.md @@ -0,0 +1,12 @@ +--- +'@objectstack/lint': patch +--- + +fix(lint): a flow node config-value finding carries the slot's key path, so Studio's Problems row selects the node + +Clause-②: no +Metadata-protocol: none + +`validateStackExpressions` located a finding on a flow node's config value (an `expression-invalid` on an Update Records `fields` value, a node `condition`, a text slot, an undeclared config key, a `script` node with no `function`) by its `where` prose alone. The runtime authoring gate then used that prose as the finding's `path`, so a draft's `_diagnostics` entry for it could not be resolved to a node: Studio's Problems list showed the refusal but could not select or badge the node. + +Each such finding now also carries the slot's key path, in the spelling a flow edge's `condition` finding uses: `flows[0].nodes[1].config.fields.status` (a draft's `_diagnostics` reads `nodes.1.config.fields.status`). A `connector_action` input value is located in its own block (`flows[0].nodes[2].connectorConfig.input.customerId`). The `where` prose and the message are unchanged. As for edges, a node inside a nested region (a `loop` body) and a map-form `flows` collection have no index to spell, so their findings keep `where` alone. The CLI's `rule: … at …` line prints the key path for these findings, as it does for an edge's. diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index 050837c2cdb..eeeff688735 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -793,7 +793,8 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ rule: EXPRESSION_INVALID, where: i.where, // [#26858] The slot's key path where the pass spells one (a flow edge's - // `condition`), so a draft's `_diagnostics` can address the field. + // `condition`, #27828 a flow node's config value), so a draft's + // `_diagnostics` can address the field. path: i.path ?? i.where, // [#22161] The authored source is a QUOTE of what the author wrote, not // a fix, so it rides the verdict — `— source: \`…\`` is the spelling diff --git a/packages/lint/src/validate-expressions.edge-condition-path.test.ts b/packages/lint/src/validate-expressions.edge-condition-path.test.ts index c298bb51546..187de25d343 100644 --- a/packages/lint/src/validate-expressions.edge-condition-path.test.ts +++ b/packages/lint/src/validate-expressions.edge-condition-path.test.ts @@ -75,7 +75,7 @@ describe('expression-invalid on a flow edge condition carries a key path (#26858 for (const i of onEdge) expect(i.path).toBeUndefined(); }); - it('a finding off the edge condition gains no path', () => { + it('a finding off the edge condition gains no edge path (#27828: a node condition carries its node\'s)', () => { const issues = validateStackExpressions({ flows: [ flow([{ id: 'e1', source: 'start', target: 'end' }], { @@ -87,7 +87,7 @@ describe('expression-invalid on a flow edge condition carries a key path (#26858 ], }); expect(issues.length).toBeGreaterThan(0); - for (const i of issues) expect(i.path).toBeUndefined(); + for (const i of issues) expect(i.path).toBe('flows[0].nodes[0].config.condition'); }); it('reaches the publish gate: the refusal is located at the edge condition, message unchanged', () => { diff --git a/packages/lint/src/validate-expressions.node-config-path.test.ts b/packages/lint/src/validate-expressions.node-config-path.test.ts new file mode 100644 index 00000000000..aca37971fff --- /dev/null +++ b/packages/lint/src/validate-expressions.node-config-path.test.ts @@ -0,0 +1,142 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #27828 — a flow node's config-value finding carries the slot's key path. + * + * `expression-invalid` is what publish refuses a node's config value with + * (measured on a blank expression in an Update Records value), and its finding + * was located by its `where` prose alone (`flow 'f' · node 'set' (update_record) + * update_record field value at config.fields.status`). A draft's + * `_diagnostics` path was therefore that prose, so Studio's Problems row listed + * the refusal but could not select the node. The finding now carries the key + * path in the spelling the edge findings use since #26858 + * (`flows[0].nodes[1].config.fields.status`) beside the unchanged `where` and + * message. + */ + +import { describe, expect, it } from 'vitest'; + +import { runRuntimeAuthoringRules } from './runtime-gate.js'; +import { validateStackExpressions } from './validate-expressions.js'; +import type { ExprIssue } from './validate-expressions.js'; + +type AnyRec = Record; + +const BLANK = { dialect: 'cel', source: ' ' }; + +const START = { id: 'start', type: 'start', label: 'Start' }; +const END = { id: 'end', type: 'end', label: 'End' }; + +/** An Update Records node whose `status` value is a blank CEL envelope. */ +const SET = { + id: 'set', + type: 'update_record', + label: 'Set', + config: { objectName: 'task', filter: { id: 'x' }, fields: { title: 'kept', status: BLANK } }, +}; + +function flow(nodes: unknown[], extra: AnyRec = {}): AnyRec { + return { + name: 'node_cfg', + label: 'Node config', + type: 'autolaunched', + nodes, + edges: [ + { id: 'e1', source: 'start', target: 'set' }, + { id: 'e2', source: 'set', target: 'end' }, + ], + ...extra, + }; +} + +const onNode = (issues: ExprIssue[], id: string) => issues.filter((i) => i.where.includes(`node '${id}'`)); + +describe('a flow node config-value finding carries a key path (#27828)', () => { + it('names the node by its index in the authored list, where and message unchanged', () => { + const issues = onNode(validateStackExpressions({ flows: [flow([START, SET, END])] }), 'set'); + expect(issues.length).toBeGreaterThan(0); + for (const i of issues) { + expect(i.path).toBe('flows[0].nodes[1].config.fields.status'); + expect(i.where).toBe("flow 'node_cfg' · node 'set' (update_record) update_record field value at config.fields.status"); + } + }); + + it('indexes the raw list: a null member before the node does not renumber it', () => { + const issues = onNode(validateStackExpressions({ flows: [flow([START, null, SET, END])] }), 'set'); + expect(issues.length).toBeGreaterThan(0); + for (const i of issues) expect(i.path).toBe('flows[0].nodes[2].config.fields.status'); + }); + + it('indexes the flow in the stack', () => { + const clean = { ...flow([START, END]), name: 'clean', edges: [{ id: 'e1', source: 'start', target: 'end' }] }; + const issues = onNode(validateStackExpressions({ flows: [clean, flow([START, SET, END])] }), 'set'); + expect(issues.length).toBeGreaterThan(0); + for (const i of issues) expect(i.path).toBe('flows[1].nodes[1].config.fields.status'); + }); + + it('every node-config arm locates its finding at the slot it judged', () => { + const nodes = [ + { ...START, config: { condition: 'record.name ==' } }, + // A value slot's retired single-brace token (the template-refusal arm). + { id: 'tok', type: 'update_record', label: 'Tok', config: { objectName: 'task', filter: { id: 'x' }, fields: { title: '{record.name}' } } }, + // A text slot's single-brace token (the text-slot arm). + { id: 'say', type: 'notify', label: 'Say', config: { recipients: ['a@b.c'], title: 'Hi {record.name}', message: 'ok' } }, + // A key the node's contract does not declare (the config-refusal arm). + { id: 'odd', type: 'notify', label: 'Odd', config: { bogusKey: 1, recipients: ['a@b.c'], title: 'ok', message: 'ok' } }, + // A script with no callable (the script arm). + { id: 'run', type: 'script', label: 'Run', config: {} }, + // A sibling-block value slot (#27968): the path names its block. + { + id: 'call', + type: 'connector_action', + label: 'Call', + config: {}, + connectorConfig: { connectorId: 'crm', actionId: 'push', input: { customerId: '{record.id}' } }, + }, + ]; + const issues = validateStackExpressions({ flows: [flow(nodes, { edges: [] })] }); + const pathsOf = (id: string) => [...new Set(onNode(issues, id).map((i) => i.path))]; + expect(pathsOf('start')).toEqual(['flows[0].nodes[0].config.condition']); + expect(pathsOf('tok')).toEqual(['flows[0].nodes[1].config.fields.title']); + expect(pathsOf('say')).toEqual(['flows[0].nodes[2].config.title']); + expect(pathsOf('odd')).toEqual(['flows[0].nodes[3].config.bogusKey']); + expect(pathsOf('run')).toEqual(['flows[0].nodes[4].config.function']); + expect(pathsOf('call')).toEqual(['flows[0].nodes[5].connectorConfig.input.customerId']); + }); + + it('a map-form flows collection has no index to spell, so the finding keeps where alone', () => { + const { name: _name, ...body } = flow([START, SET, END]); + const issues = onNode(validateStackExpressions({ flows: { node_cfg: body } }), 'set'); + expect(issues.length).toBeGreaterThan(0); + for (const i of issues) expect(i.path).toBeUndefined(); + }); + + it('a node inside a nested region keeps where alone, as a nested edge does', () => { + const loop = { + id: 'each', + type: 'loop', + label: 'Each', + config: { + collection: { dialect: 'cel', source: '[1, 2]' }, + itemVariable: 'item', + body: { nodes: [{ ...SET, id: 'inner' }], edges: [] }, + }, + }; + const issues = onNode( + validateStackExpressions({ flows: [flow([START, loop, END], { edges: [{ id: 'e1', source: 'start', target: 'each' }, { id: 'e2', source: 'each', target: 'end' }] })] }), + 'inner', + ); + expect(issues.length).toBeGreaterThan(0); + for (const i of issues) expect(i.path).toBeUndefined(); + }); + + it('reaches the publish gate: the refusal is located at the node config value, message unchanged', () => { + const result = runRuntimeAuthoringRules({ type: 'flow', item: flow([START, SET, END]) }); + const refusals = result.errors.filter((f) => f.rule === 'expression-invalid'); + expect(refusals.length).toBeGreaterThan(0); + for (const f of refusals) { + expect(f.path).toMatch(/^flows(\[0\]|\.node_cfg)\.nodes\[1\]\.config\.fields\.status$/); + expect(f.where).toBe("flow 'node_cfg' · node 'set' (update_record) update_record field value at config.fields.status"); + } + }); +}); diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index d22debb11d6..0b89a6a369e 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -3624,6 +3624,10 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t // its `traversals` and `multiHopFields` — the formula package's own // result keys, never metadata keys. 'analysis', + // [#27828] A node's `where` → judged key path index, filled by the + // node walk's own sites. Its keys are `Map.prototype.set` / `get`, never + // metadata keys; the config it describes is read off the tabled `cfg`. + 'nodeSlotKeys', ]); expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]); }); diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index fbb6e807cc1..92208b4529b 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -165,7 +165,10 @@ export interface ExprIssue { * [#26858] The judged slot's key path in the stack, in the spelling the other * rules' findings carry (`flows[0].edges[1].condition`), beside the human * `where`. Set only where the slot has one: a top-level flow edge's - * `condition`, and (#27643) an object validation rule's `condition` / `when` + * `condition`, (#27828) a top-level flow node's judged config value + * (`flows[0].nodes[1].config.fields.status`, a sibling block's + * `flows[0].nodes[2].connectorConfig.input.customerId`), and (#27643) an + * object validation rule's `condition` / `when` * (`objects[0].validations[1].condition`), a nested branch's included * (`objects[0].validations[1].then.condition`). Absent, the finding is * located by `where` alone. @@ -2722,7 +2725,13 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression // that guarantee stops, through the file's one home for it. for (const node of recordsOf(graph.nodes)) { const cfg = (node.config ?? {}) as AnyRec; + // [#27828] Each site below names the key it judged beside its `where`; + // after the walk, this node's findings carry that key path (see the + // stamp at the end of this body). + const nodeFirst = issues.length; + const nodeSlotKeys = new Map(); const nodeCondWhere = `${at} · node '${node.id}' (${node.type}) condition`; + nodeSlotKeys.set(nodeCondWhere, 'config.condition'); if (!checkStructuralCondition(nodeCondWhere, cfg.condition).refused) { const from = issues.length; check(nodeCondWhere, cfg.condition, objectName); @@ -2762,8 +2771,10 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression || (configRefusal.code === 'node-config-refused-by-contract' && configRefusal.path === 'functionName') ))); for (const configRefusal of configRefusals) { + const configWhere = `${at} · node '${node.id}' (${nodeType}) config.${configRefusal.path}`; + nodeSlotKeys.set(configWhere, `config.${configRefusal.path}`); issues.push({ - where: `${at} · node '${node.id}' (${nodeType}) config.${configRefusal.path}`, + where: configWhere, message: configRefusal.message, source: configRefusal.source, severity: 'error', @@ -2774,6 +2785,7 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression // `connectorConfig.input` — and names the block it is in. for (const found of resolveFlowNodeExpressions(nodeType, cfg, node)) { const slotWhere = `${at} · node '${node.id}' (${nodeType}) ${found.entry.label} at ${found.block}.${found.path}`; + nodeSlotKeys.set(slotWhere, `${found.block}.${found.path}`); // [#15137] `value` slots are checkable too, by their own rule — see // `checkDeclaredValue`. Not shadowing-checked: the shadow diagnostic is // about a bare identifier resolving to a variable instead of a field, @@ -2832,8 +2844,10 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression // and the two legacy `assignment` shapes, so build and registration // give one verdict, and each refusal names the token's CEL spelling. for (const templateRefusal of flowNodeValueTemplateRefusals(nodeType, cfg, node)) { + const templateWhere = `${at} · node '${node.id}' (${nodeType}) ${templateRefusal.label} at ${templateRefusal.block}.${templateRefusal.path}`; + nodeSlotKeys.set(templateWhere, `${templateRefusal.block}.${templateRefusal.path}`); issues.push({ - where: `${at} · node '${node.id}' (${nodeType}) ${templateRefusal.label} at ${templateRefusal.block}.${templateRefusal.path}`, + where: templateWhere, message: templateRefusal.message, source: templateRefusal.source, severity: 'error', @@ -2850,6 +2864,7 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression // throw at the node mid-run. for (const slot of flowNodeTextSlotSources(nodeType, cfg)) { const slotWhere = `${at} · node '${node.id}' (${nodeType}) ${slot.label} at config.${slot.path}`; + nodeSlotKeys.set(slotWhere, `config.${slot.path}`); const tokenRefusal = textSlotTemplateRefusal(slot.source); if (tokenRefusal !== undefined) { issues.push({ where: slotWhere, message: tokenRefusal, source: slot.source, severity: 'error' }); @@ -2879,6 +2894,8 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression const action = typeof cfg.actionType === 'string' ? cfg.actionType.trim() : ''; const retired = ['actionType', 'template', 'recipients', 'variables', 'script'] .filter((k) => cfg[k] != null); + // The first retired key the author wrote, or the `function` to write. + nodeSlotKeys.set(`${at} · node '${node.id}' (script) callable`, `config.${retired[0] ?? 'function'}`); if (retired.length > 0) { issues.push({ where: `${at} · node '${node.id}' (script) callable`, @@ -2914,6 +2931,20 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression }); } } + // [#27828] A top-level node's findings also carry the judged slot's key + // path, the #26858 edge shape (`flows[0].nodes[1].config.fields.status`), + // indexed over the list the author wrote (the coerced one drops `null` + // members and would renumber). A nested region's node keeps `where`, as + // its edges do. + const nodeIndex = flowIndex >= 0 && graph.path.length === 0 && Array.isArray(flow.nodes) + ? flow.nodes.indexOf(node) + : -1; + if (nodeIndex >= 0) { + for (let k = nodeFirst; k < issues.length; k++) { + const key = nodeSlotKeys.get(issues[k]!.where); + if (key !== undefined) issues[k] = { ...issues[k]!, path: `flows[${flowIndex}].nodes[${nodeIndex}].${key}` }; + } + } } // `recordsOf`, not `as unknown as AnyRec[]` (#16910) — the same reason as // the `graph.nodes` reader above: a NESTED region's edge list is only diff --git a/packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts b/packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts index 7aaedb81a2f..bd2aa15eab0 100644 --- a/packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts +++ b/packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts @@ -462,6 +462,39 @@ describe('a draft reads the verdict its publish gives (#26707)', () => { expect.objectContaining({ path: 'nodes.0.config.secret', code: 'flow-api-trigger-secret-missing' }), ); }); + + it('[#27828] a node config value refused by expression-invalid is located at the node, not in where prose', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + // Studio's Update Records value with its expression left blank. + const flow = { + name: 'blank_value', + label: 'Blank Value', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'set', + type: 'update_record', + label: 'Set', + config: { objectName: 'deal', filter: { id: 'x' }, fields: { name: { dialect: 'cel', source: ' ' } } }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'set' }, + { id: 'e2', source: 'set', target: 'end' }, + ], + }; + await stageDraft(protocol, 'flow', flow); + + const diagnostics = await draftDiagnostics(protocol, 'flow', flow.name); + const refused = (diagnostics.errors ?? []).filter((e) => e.code === 'expression-invalid'); + expect(diagnostics.valid).toBe(false); + expect(refused.length).toBeGreaterThan(0); + // `nodes..…` is the spelling Studio's Problems list maps onto a node. + for (const e of refused) expect(e.path).toBe('nodes.1.config.fields.name'); + }); }); describe('[#27444] an object draft without its package namespace prefix', () => {