feat(automation,formula): warn on undeclared flow-node config keys (#4045)

.changeset/unknown-node-config-key-warning.md

@@ -0,0 +1,47 @@
+---
+'@objectstack/service-automation': minor
+'@objectstack/formula': minor
+---
+
+Warn on flow-node `config` keys the node type does not declare (#4045).
+
+`FlowNodeSchema.config` is `z.record(z.unknown())`, so a misspelled or invented
+config key was accepted in total silence: `visibleIf` instead of `visibleWhen`
+registered cleanly, was never read, and the only symptom was a feature that quietly
+did not happen. That diagnostic vacuum is what made #3528 take three passes and two
+wrong diagnoses to resolve.
+
+`registerFlow` now compares each node's `config` against its descriptor's
+`configSchema` and warns on anything undeclared, located and with the declared set
+listed:
+
+```
+[flow 'lead_conversion'] node 'screen_1' (screen): unknown config key `visibleIf`
+  at config.fields[0].visibleIf — It is not declared by this node type's
+  configSchema, so nothing reads it. Declared here: name, label, type, required,
+  visibleWhen.
+```
+
+The walk descends where the schema declares structure and **stops at free-form
+keyValue maps**, whose keys are author data (`filter: { status: 'stale' }`).
+Descending matters: the #3528 typo class lives *inside* the `screen` field
+repeater, so a top-level-only comparison would miss the exact mistake this exists
+to catch.
+
+**Warn, never reject.** An undeclared key is an author typo, a key the executor
+genuinely reads that its hand-written `configSchema` never declared (`notify.source`
+was exactly this), or dead config. Only 4 of the 13 schema-carrying builtins have
+been audited for the second population, so hard-failing would gamble on the other
+nine. Tightening to an error is a later, per-key decision once this warning has
+measured the real distribution. Nothing about the published `configSchema` changes,
+so no consumer sees a different shape.
+
+`@objectstack/formula` now exports `nearestName`, the edit-distance helper already
+used for unknown-field and unknown-role suggestions, so "did you mean?"
+diagnostics share one threshold. It is deliberately a bonus rather than the
+mechanism — `visibleIf` → `visibleWhen` is distance 4 against a threshold of 3, so
+the declared set is always listed instead of only as a fallback.
+
+Also fixes the first real finding from the new check: `showcase_inquiry_purge`'s
+`get_record` node carried `mode: 'records'`, which no executor reads, with a comment
+crediting it for behaviour that `limit > 1` actually produces.

examples/app-showcase/src/automation/flows/index.ts

@@ -1357,13 +1357,16 @@ export const InquiryPurgeFlow = defineFlow({
       id: 'purge_check',
       type: 'get_record',
       label: 'Find closed inquiries',
-      // records mode + outputVariable: the result lands in the variable
-      // context, where the out-edge CEL below reads it (the engine routes on
-      // EDGE conditions — same contract as BudgetApprovalFlow's gate).
+      // `limit > 1` is what makes this a LIST read (`find`, not `findOne`) — the
+      // executor has no `mode` key, so a `mode: 'records'` used to sit here doing
+      // nothing while the comment credited it for the behaviour `limit` actually
+      // produced (found by the #4045 unknown-config-key check). `outputVariable`
+      // then lands the array in the variable context, where the out-edge CEL below
+      // reads it — the engine routes on EDGE conditions, same contract as
+      // BudgetApprovalFlow's gate.
       config: {
         objectName: 'showcase_inquiry',
         filter: { status: 'closed' },
-        mode: 'records',
         limit: 200,
         outputVariable: 'closedInquiries',
       },

packages/formula/src/index.ts

@@ -28,7 +28,7 @@ export type { CelFilterCompileResult, CelFilterCompileOptions, CelFilterFailReas
 export { matchesFilterCondition } from './matches-filter';
 // ADR-0032 — shared validator + introspection (one validator for build,
 // registration, and the agent-callable validate_expression tool).
-export { validateExpression, introspectScope, expectedDialect, inferExpressionType, CEL_STDLIB_FUNCTIONS } from './validate';
+export { validateExpression, introspectScope, expectedDialect, inferExpressionType, nearestName, CEL_STDLIB_FUNCTIONS } from './validate';
 export type { FieldRole, ExprInput, ExprSchemaHint, ExprValidationError, ExprValidationResult, InferredValueType } from './validate';
 export type { SeedValue, SeedPrimitive } from './seed-eval';
 export type { DialectEngine, EvalContext, EvalResult, EvalError } from './types';

packages/formula/src/validate.ts

@@ -216,6 +216,20 @@ function checkFieldExistence(source: string, schema: ExprSchemaHint | undefined,
 }
 
 /** Cheap edit-distance suggestion for typo'd field names. */
+/**
+ * The closest candidate to `name`, or `undefined` when nothing is close enough
+ * to be worth suggesting.
+ *
+ * The public face of the same edit-distance heuristic this module already uses
+ * for unknown field refs and unknown roles, exported so other "did you mean?"
+ * diagnostics reuse one threshold instead of each inventing their own — an
+ * author (increasingly an agent) should not get a suggestion here and silence
+ * there for the same class of typo.
+ */
+export function nearestName(name: string, candidates: readonly string[]): string | undefined {
+  return nearest(name, candidates);
+}
+
 function nearest(name: string, candidates: readonly string[]): string | undefined {
   let best: string | undefined;
   let bestD = Infinity;

packages/services/service-automation/src/builtin/config-unknown-keys.test.ts

@@ -0,0 +1,159 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * Unknown flow-node `config` keys are reported at registration (#4045).
+ *
+ * `FlowNodeSchema.config` is `z.record(z.unknown())`, so before this a misspelled
+ * or invented config key was accepted in **total silence**: `visibleIf` instead of
+ * `visibleWhen` registered cleanly, was never read, and the only symptom was a
+ * feature that quietly did not happen. That is the diagnostic vacuum that made
+ * #3528 take three passes and two wrong diagnoses.
+ *
+ * Warn, never reject — see `validateNodeConfigKeys` for why. These tests pin both
+ * halves of that: the warning fires with an actionable suggestion, AND
+ * registration still succeeds.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { AutomationEngine } from '../engine.js';
+import { installBuiltinNodes } from './index.js';
+
+function recordingLogger() {
+  const warnings: string[] = [];
+  const logger: any = {
+    info() {}, error() {}, debug() {},
+    warn(msg: string) { warnings.push(msg); },
+    child() { return logger; },
+  };
+  return { logger, warnings };
+}
+
+function engineWith(logger: any) {
+  const engine = new AutomationEngine(logger);
+  installBuiltinNodes(engine, { logger, getService() { throw new Error('none'); } } as any);
+  return engine;
+}
+
+/** A one-node flow carrying `config` verbatim on a node of `type`. */
+function flowWith(type: string, config: Record<string, unknown>) {
+  return {
+    name: 'f', label: 'F', type: 'screen', status: 'active', version: 1,
+    nodes: [
+      { id: 'start', type: 'start', label: 'S', config: {} },
+      { id: 'n1', type, label: 'N', config },
+      { id: 'end', type: 'end', label: 'E' },
+    ],
+    edges: [
+      { id: 'e1', source: 'start', target: 'n1', type: 'default' },
+      { id: 'e2', source: 'n1', target: 'end', type: 'default' },
+    ],
+  };
+}
+
+describe('unknown node config keys (#4045)', () => {
+  it('flags a misspelled key with a did-you-mean suggestion', () => {
+    const { logger, warnings } = recordingLogger();
+    const engine = engineWith(logger);
+
+    // The typo lives INSIDE the field repeater — where the real #3528 one did.
+    engine.registerFlow('f', flowWith('screen', {
+      fields: [{ name: 'opportunityName', required: true, visibleIf: 'createOpportunity == true' }],
+    }));
+
+    const hit = warnings.find((w) => w.includes('visibleIf'));
+    expect(hit, 'the typo should be reported').toBeDefined();
+    expect(hit).toContain("node 'n1'");
+    expect(hit).toContain('(screen)');
+    // Located to the exact element, so the author knows WHICH field.
+    expect(hit).toContain('config.fields[0].visibleIf');
+    // The load-bearing half for an agent author: the correct key is named. Note
+    // it comes from the DECLARED SET, not the suggestion — `visibleIf` →
+    // `visibleWhen` is edit-distance 4 against `nearestName`'s threshold of 3, so
+    // this exact typo gets no did-you-mean. Printing the declared set is what
+    // makes the diagnostic actionable regardless.
+    expect(hit).toContain('Declared here: name, label, type, required, visibleWhen.');
+  });
+
+  it('adds a did-you-mean when the typo IS within edit distance', () => {
+    const { logger, warnings } = recordingLogger();
+    const engine = engineWith(logger);
+
+    engine.registerFlow('f', flowWith('screen', { titl: 'Details' }));
+
+    const hit = warnings.find((w) => w.includes('titl'));
+    expect(hit).toContain('did you mean `title`?');
+    // …and still lists the declared set alongside it.
+    expect(hit).toContain('Declared here:');
+  });
+
+  it('still registers the flow — warn, never reject', () => {
+    const { logger } = recordingLogger();
+    const engine = engineWith(logger);
+    expect(() => engine.registerFlow('f', flowWith('screen', { visibleIf: 'a == true' }))).not.toThrow();
+    // And it is really registered, not swallowed.
+    expect(engine.getFlow('f')).toBeDefined();
+  });
+
+  it('lists the declared keys when nothing is close enough to suggest', () => {
+    const { logger, warnings } = recordingLogger();
+    const engine = engineWith(logger);
+
+    engine.registerFlow('f', flowWith('screen', { totallyMadeUp: 42 }));
+
+    const hit = warnings.find((w) => w.includes('totallyMadeUp'));
+    expect(hit).toBeDefined();
+    expect(hit).not.toContain('did you mean');
+    // The declared set is always present, so there is always somewhere to go.
+    expect(hit).toContain('Declared here:');
+    expect(hit).toContain('fields');
+  });
+
+  it('reports every undeclared key, not just the first', () => {
+    const { logger, warnings } = recordingLogger();
+    const engine = engineWith(logger);
+
+    engine.registerFlow('f', flowWith('screen', { hideWhen: 'x', submitLabel: 'Go' }));
+
+    expect(warnings.some((w) => w.includes('hideWhen'))).toBe(true);
+    expect(warnings.some((w) => w.includes('submitLabel'))).toBe(true);
+  });
+
+  it('stays quiet on a fully declared config', () => {
+    const { logger, warnings } = recordingLogger();
+    const engine = engineWith(logger);
+
+    engine.registerFlow('f', flowWith('screen', {
+      title: 'Details',
+      fields: [{ name: 'a', type: 'boolean', required: true, visibleWhen: 'b == true' }],
+      waitForInput: true,
+      defaults: { x: 1 },
+    }));
+
+    expect(warnings.filter((w) => w.includes('unknown config key'))).toEqual([]);
+  });
+
+  it('stays quiet for a node type that publishes no configSchema', () => {
+    // `decision` / `script` are deliberately schemaless (config-schemas.test.ts):
+    // nothing is declared, so nothing can be undeclared.
+    const { logger, warnings } = recordingLogger();
+    const engine = engineWith(logger);
+
+    engine.registerFlow('f', flowWith('decision', { condition: 'a == b', whateverElse: 1 }));
+
+    expect(warnings.filter((w) => w.includes('unknown config key'))).toEqual([]);
+  });
+
+  it('does not flag the keys of a keyValue map — those are author data', () => {
+    // The walk descends where the schema declares structure and STOPS at a
+    // free-form map: `filter: { status: 'stale' }` keys are data, not config keys.
+    const { logger, warnings } = recordingLogger();
+    const engine = engineWith(logger);
+
+    engine.registerFlow('f', flowWith('get_record', {
+      objectName: 'crm_lead',
+      filter: { status: 'stale', anythingAtAll: true },
+    }));
+
+    expect(warnings.filter((w) => w.includes('unknown config key'))).toEqual([]);
+  });
+});

packages/services/service-automation/src/engine.ts

@@ -17,7 +17,18 @@ import { ConnectorSchema } from '@objectstack/spec/integration';
 // silently returned `false`, so EVERY start-node / edge condition (record-change
 // `previous.*`, `budget > 100000`, …) skipped its flow. A static import binds the
 // engine at module load in both ESM and CJS builds.
-import { ExpressionEngine, validateExpression } from '@objectstack/formula';
+import { ExpressionEngine, validateExpression, nearestName } from '@objectstack/formula';
+
+/**
+ * The slice of a descriptor's JSON-Schema `configSchema` that the undeclared-key
+ * walk reads (#4045). Structural only — no validation semantics.
+ */
+interface ConfigSchemaNode {
+    type?: string;
+    properties?: Record<string, ConfigSchemaNode>;
+    items?: ConfigSchemaNode;
+    additionalProperties?: unknown;
+}
 import { runIsUnscopedUserMode, flowTouchesData } from './runtime-identity.js';
 import { isGuardRefusal } from './guard-refusal.js';
 
@@ -1473,6 +1484,12 @@ export class AutomationEngine implements IAutomationService {
         // executeNode() already throws NO_EXECUTOR at run time for unknown types.
         this.validateNodeTypes(name, parsed);
 
+        // #4045 — warn on config keys the node's descriptor does not declare
+        // (a `visibleIf` typo is silently accepted today). Warn-only: see
+        // validateNodeConfigKeys for why hard-failing would gamble on the nine
+        // builtins whose read-vs-declared drift has not been audited.
+        this.validateNodeConfigKeys(name, parsed);
+
         // ADR-0032 §Decision 1a — parse-validate every predicate at registration,
         // so a malformed condition (e.g. the #1491 `{record.x}` template-brace-in-
         // CEL mistake) is a LOUD registration error with the offending source,
@@ -2717,6 +2734,107 @@ export class AutomationEngine implements IAutomationService {
         }
     }
 
+    /**
+     * Warn about node `config` keys the node type's descriptor does not declare
+     * (#4045 — the warn half of the unknown-key ladder).
+     *
+     * `FlowNodeSchema.config` is `z.record(z.unknown())`, so a misspelled or
+     * invented config key is accepted in total silence today: `visibleIf` instead
+     * of `visibleWhen` registers cleanly and then does nothing, which is exactly
+     * the failure shape that made #3528 take three passes to diagnose. The key is
+     * never read, so there is no runtime error to trace back — the only symptom
+     * is a feature that quietly does not happen.
+     *
+     * **Warn, never reject.** An undeclared key falls into three populations and
+     * this seam cannot yet tell them apart: an author typo (which we want to
+     * reject), a key the executor genuinely reads that its hand-written
+     * `configSchema` never declared (`notify.source` was exactly this until
+     * #4045 — rejecting those breaks working apps), and dead config nobody reads
+     * (harmless). Only 4 of the 13 schema-carrying builtins have been audited for
+     * the second population, so hard-failing here would gamble on the 9 that have
+     * not. The warning is what measures that distribution; tightening to an error
+     * is a later, per-key decision once the data exists, and belongs with a
+     * tombstone that carries the prescription (the `UNKNOWN_KEY_GUIDANCE` pattern
+     * in `object.zod.ts`).
+     *
+     * Soft-fail matches {@link validateNodeTypes} directly above — same function,
+     * same `logger.warn` channel, same reasoning about not breaking flows over a
+     * diagnostic. Nothing about the published `configSchema` changes, so no
+     * consumer (designer form generation included) sees a different shape.
+     */
+    private validateNodeConfigKeys(flowName: string, flow: FlowParsed): void {
+        for (const node of flow.nodes) {
+            const schema = this.actionDescriptors.get(node.type)?.configSchema as ConfigSchemaNode | undefined;
+            // No descriptor, or a deliberately schemaless type (`decision`,
+            // `script`, `wait`, `subflow` publish none — see config-schemas.test)
+            // ⇒ nothing is declared, so nothing can be undeclared.
+            if (!schema) continue;
+            this.warnUndeclaredConfigKeys(flowName, node, schema, node.config, 'config');
+        }
+    }
+
+    /**
+     * Walk `value` against `schema` in lockstep, warning on keys the schema does
+     * not declare.
+     *
+     * Descends **only where the schema declares structure** — an object with fixed
+     * `properties`, or an array whose `items` do. It deliberately stops at a
+     * keyValue map (`additionalProperties: true` with no `properties`): those keys
+     * are author *data* (`filter: { status: 'stale' }`), not config keys, and
+     * flagging them would make the check useless noise.
+     *
+     * Descending matters rather than being thoroughness for its own sake: the
+     * #3528 typo class lives *inside* the `screen` field repeater, not at the top
+     * level. `visibleWhen` is a property of `fields[].items`, so a top-level-only
+     * comparison would miss `visibleIf` — the exact mistake this check exists to
+     * catch — while still reporting the rarer top-level ones. A test pins it.
+     */
+    private warnUndeclaredConfigKeys(
+        flowName: string,
+        node: FlowNodeParsed,
+        schema: ConfigSchemaNode,
+        value: unknown,
+        path: string,
+    ): void {
+        if (schema.type === 'array' && schema.items) {
+            if (!Array.isArray(value)) return;
+            value.forEach((element, index) => {
+                this.warnUndeclaredConfigKeys(flowName, node, schema.items!, element, `${path}[${index}]`);
+            });
+            return;
+        }
+
+        // A free-form map declares no properties — its keys are author data.
+        if (!schema.properties || schema.additionalProperties === true) return;
+        if (value == null || typeof value !== 'object' || Array.isArray(value)) return;
+
+        const declared = Object.keys(schema.properties);
+        for (const key of Object.keys(value as Record<string, unknown>)) {
+            const child = schema.properties[key];
+            if (child) {
+                this.warnUndeclaredConfigKeys(flowName, node, child, (value as Record<string, unknown>)[key], `${path}.${key}`);
+                continue;
+            }
+            // The declared set is printed ALWAYS, not only as a fallback when the
+            // edit-distance heuristic misses. It misses more than you would
+            // expect: `visibleIf` → `visibleWhen` is distance 4 against a
+            // threshold of 3, so the very typo this check exists to catch gets no
+            // suggestion. Loosening `nearestName` is the wrong fix — it is shared
+            // with the unknown-field and unknown-role diagnostics, where a looser
+            // threshold means confidently wrong suggestions over hundreds of
+            // candidates. A config object declares at most a dozen keys, so simply
+            // listing them is both cheap and complete, and the suggestion becomes
+            // a bonus for the cases it does catch.
+            const suggestion = nearestName(key, declared);
+            this.logger.warn(
+                `[flow '${flowName}'] node '${node.id}' (${node.type}): unknown config key \`${key}\` at ${path}.${key}` +
+                (suggestion ? ` — did you mean \`${suggestion}\`?` : '') +
+                ` It is not declared by this node type's configSchema, so nothing reads it.` +
+                ` Declared here: ${declared.join(', ')}.`,
+            );
+        }
+    }
+
     /**
      * ADR-0032 §Decision 1a — parse-validate every predicate in the flow at
      * registration. Predicates are bare CEL; this catches the #1491 class