|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * Unknown flow-node `config` keys are reported at registration (#4045). |
| 5 | + * |
| 6 | + * `FlowNodeSchema.config` is `z.record(z.unknown())`, so before this a misspelled |
| 7 | + * or invented config key was accepted in **total silence**: `visibleIf` instead of |
| 8 | + * `visibleWhen` registered cleanly, was never read, and the only symptom was a |
| 9 | + * feature that quietly did not happen. That is the diagnostic vacuum that made |
| 10 | + * #3528 take three passes and two wrong diagnoses. |
| 11 | + * |
| 12 | + * Warn, never reject — see `validateNodeConfigKeys` for why. These tests pin both |
| 13 | + * halves of that: the warning fires with an actionable suggestion, AND |
| 14 | + * registration still succeeds. |
| 15 | + */ |
| 16 | + |
| 17 | +import { describe, it, expect } from 'vitest'; |
| 18 | +import { AutomationEngine } from '../engine.js'; |
| 19 | +import { installBuiltinNodes } from './index.js'; |
| 20 | + |
| 21 | +function recordingLogger() { |
| 22 | + const warnings: string[] = []; |
| 23 | + const logger: any = { |
| 24 | + info() {}, error() {}, debug() {}, |
| 25 | + warn(msg: string) { warnings.push(msg); }, |
| 26 | + child() { return logger; }, |
| 27 | + }; |
| 28 | + return { logger, warnings }; |
| 29 | +} |
| 30 | + |
| 31 | +function engineWith(logger: any) { |
| 32 | + const engine = new AutomationEngine(logger); |
| 33 | + installBuiltinNodes(engine, { logger, getService() { throw new Error('none'); } } as any); |
| 34 | + return engine; |
| 35 | +} |
| 36 | + |
| 37 | +/** A one-node flow carrying `config` verbatim on a node of `type`. */ |
| 38 | +function flowWith(type: string, config: Record<string, unknown>) { |
| 39 | + return { |
| 40 | + name: 'f', label: 'F', type: 'screen', status: 'active', version: 1, |
| 41 | + nodes: [ |
| 42 | + { id: 'start', type: 'start', label: 'S', config: {} }, |
| 43 | + { id: 'n1', type, label: 'N', config }, |
| 44 | + { id: 'end', type: 'end', label: 'E' }, |
| 45 | + ], |
| 46 | + edges: [ |
| 47 | + { id: 'e1', source: 'start', target: 'n1', type: 'default' }, |
| 48 | + { id: 'e2', source: 'n1', target: 'end', type: 'default' }, |
| 49 | + ], |
| 50 | + }; |
| 51 | +} |
| 52 | + |
| 53 | +describe('unknown node config keys (#4045)', () => { |
| 54 | + it('flags a misspelled key with a did-you-mean suggestion', () => { |
| 55 | + const { logger, warnings } = recordingLogger(); |
| 56 | + const engine = engineWith(logger); |
| 57 | + |
| 58 | + // The typo lives INSIDE the field repeater — where the real #3528 one did. |
| 59 | + engine.registerFlow('f', flowWith('screen', { |
| 60 | + fields: [{ name: 'opportunityName', required: true, visibleIf: 'createOpportunity == true' }], |
| 61 | + })); |
| 62 | + |
| 63 | + const hit = warnings.find((w) => w.includes('visibleIf')); |
| 64 | + expect(hit, 'the typo should be reported').toBeDefined(); |
| 65 | + expect(hit).toContain("node 'n1'"); |
| 66 | + expect(hit).toContain('(screen)'); |
| 67 | + // Located to the exact element, so the author knows WHICH field. |
| 68 | + expect(hit).toContain('config.fields[0].visibleIf'); |
| 69 | + // The load-bearing half for an agent author: the correct key is named. Note |
| 70 | + // it comes from the DECLARED SET, not the suggestion — `visibleIf` → |
| 71 | + // `visibleWhen` is edit-distance 4 against `nearestName`'s threshold of 3, so |
| 72 | + // this exact typo gets no did-you-mean. Printing the declared set is what |
| 73 | + // makes the diagnostic actionable regardless. |
| 74 | + expect(hit).toContain('Declared here: name, label, type, required, visibleWhen.'); |
| 75 | + }); |
| 76 | + |
| 77 | + it('adds a did-you-mean when the typo IS within edit distance', () => { |
| 78 | + const { logger, warnings } = recordingLogger(); |
| 79 | + const engine = engineWith(logger); |
| 80 | + |
| 81 | + engine.registerFlow('f', flowWith('screen', { titl: 'Details' })); |
| 82 | + |
| 83 | + const hit = warnings.find((w) => w.includes('titl')); |
| 84 | + expect(hit).toContain('did you mean `title`?'); |
| 85 | + // …and still lists the declared set alongside it. |
| 86 | + expect(hit).toContain('Declared here:'); |
| 87 | + }); |
| 88 | + |
| 89 | + it('still registers the flow — warn, never reject', () => { |
| 90 | + const { logger } = recordingLogger(); |
| 91 | + const engine = engineWith(logger); |
| 92 | + expect(() => engine.registerFlow('f', flowWith('screen', { visibleIf: 'a == true' }))).not.toThrow(); |
| 93 | + // And it is really registered, not swallowed. |
| 94 | + expect(engine.getFlow('f')).toBeDefined(); |
| 95 | + }); |
| 96 | + |
| 97 | + it('lists the declared keys when nothing is close enough to suggest', () => { |
| 98 | + const { logger, warnings } = recordingLogger(); |
| 99 | + const engine = engineWith(logger); |
| 100 | + |
| 101 | + engine.registerFlow('f', flowWith('screen', { totallyMadeUp: 42 })); |
| 102 | + |
| 103 | + const hit = warnings.find((w) => w.includes('totallyMadeUp')); |
| 104 | + expect(hit).toBeDefined(); |
| 105 | + expect(hit).not.toContain('did you mean'); |
| 106 | + // The declared set is always present, so there is always somewhere to go. |
| 107 | + expect(hit).toContain('Declared here:'); |
| 108 | + expect(hit).toContain('fields'); |
| 109 | + }); |
| 110 | + |
| 111 | + it('reports every undeclared key, not just the first', () => { |
| 112 | + const { logger, warnings } = recordingLogger(); |
| 113 | + const engine = engineWith(logger); |
| 114 | + |
| 115 | + engine.registerFlow('f', flowWith('screen', { hideWhen: 'x', submitLabel: 'Go' })); |
| 116 | + |
| 117 | + expect(warnings.some((w) => w.includes('hideWhen'))).toBe(true); |
| 118 | + expect(warnings.some((w) => w.includes('submitLabel'))).toBe(true); |
| 119 | + }); |
| 120 | + |
| 121 | + it('stays quiet on a fully declared config', () => { |
| 122 | + const { logger, warnings } = recordingLogger(); |
| 123 | + const engine = engineWith(logger); |
| 124 | + |
| 125 | + engine.registerFlow('f', flowWith('screen', { |
| 126 | + title: 'Details', |
| 127 | + fields: [{ name: 'a', type: 'boolean', required: true, visibleWhen: 'b == true' }], |
| 128 | + waitForInput: true, |
| 129 | + defaults: { x: 1 }, |
| 130 | + })); |
| 131 | + |
| 132 | + expect(warnings.filter((w) => w.includes('unknown config key'))).toEqual([]); |
| 133 | + }); |
| 134 | + |
| 135 | + it('stays quiet for a node type that publishes no configSchema', () => { |
| 136 | + // `decision` / `script` are deliberately schemaless (config-schemas.test.ts): |
| 137 | + // nothing is declared, so nothing can be undeclared. |
| 138 | + const { logger, warnings } = recordingLogger(); |
| 139 | + const engine = engineWith(logger); |
| 140 | + |
| 141 | + engine.registerFlow('f', flowWith('decision', { condition: 'a == b', whateverElse: 1 })); |
| 142 | + |
| 143 | + expect(warnings.filter((w) => w.includes('unknown config key'))).toEqual([]); |
| 144 | + }); |
| 145 | + |
| 146 | + it('does not flag the keys of a keyValue map — those are author data', () => { |
| 147 | + // The walk descends where the schema declares structure and STOPS at a |
| 148 | + // free-form map: `filter: { status: 'stale' }` keys are data, not config keys. |
| 149 | + const { logger, warnings } = recordingLogger(); |
| 150 | + const engine = engineWith(logger); |
| 151 | + |
| 152 | + engine.registerFlow('f', flowWith('get_record', { |
| 153 | + objectName: 'crm_lead', |
| 154 | + filter: { status: 'stale', anythingAtAll: true }, |
| 155 | + })); |
| 156 | + |
| 157 | + expect(warnings.filter((w) => w.includes('unknown config key'))).toEqual([]); |
| 158 | + }); |
| 159 | +}); |
0 commit comments