From adf20150606146f2262d9f270d015168c251a6e0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 07:02:35 +0000 Subject: [PATCH] feat(automation): schema-aware flow-condition validation at registration (#1928) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit registerFlow now runs the same schema-aware condition checks as objectstack build, so a flow registered dynamically (API/Studio, bypassing the build lint) still gets the guardrail. When an object-schema resolver is wired, a condition that references an unknown field, likely-typos a field, or does arithmetic/ ordering on a text/boolean field against a number is surfaced as an advisory warning (logged), against the object's real schema. - New AutomationEngine.setObjectSchemaResolver bridge (mirrors setFunctionResolver); AutomationServicePlugin wires it to objectql.registry.getObject in start(), before the flow pull. - Strictly additive: the fatal set is unchanged (syntax, brace-in-CEL, unknown-function still throw); schema findings are logged, never thrown; no-op when unwired. Flattened scope — flow variables stay dyn, equality safe. - parseObjectFieldSchema helper normalizes the registry's field map/array. Tests: service-automation 316 (+10: 6 engine resolver, 4 parser); no regression. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Hnji7EEYR2mGt6pY53a8Bm --- ...ow-registration-schema-aware-validation.md | 26 ++++ .../service-automation/src/engine.test.ts | 113 +++++++++++++++++- .../services/service-automation/src/engine.ts | 67 ++++++++++- .../services/service-automation/src/plugin.ts | 52 ++++++++ 4 files changed, 254 insertions(+), 4 deletions(-) create mode 100644 .changeset/flow-registration-schema-aware-validation.md diff --git a/.changeset/flow-registration-schema-aware-validation.md b/.changeset/flow-registration-schema-aware-validation.md new file mode 100644 index 0000000000..935bbb5579 --- /dev/null +++ b/.changeset/flow-registration-schema-aware-validation.md @@ -0,0 +1,26 @@ +--- +"@objectstack/service-automation": minor +--- + +feat(automation): schema-aware flow-condition validation at registration (#1928) + +`registerFlow` now runs the same schema-aware condition checks as +`objectstack build` — so a flow registered dynamically (via the API / Studio, +bypassing the build lint) still gets the guardrail. When the host wires an +object-schema resolver, a flow condition that references an unknown field, +likely-typos a field name, or does arithmetic/ordering on a text/boolean field +against a number is surfaced as an **advisory warning** (logged), pointing at +the object's real schema. + +- New `AutomationEngine.setObjectSchemaResolver(resolver)` bridge (mirrors + `setFunctionResolver`); `AutomationServicePlugin` wires it to + `objectql.registry.getObject` in `start()`, before the flow pull, so + registry-sourced flows are covered too. +- **Strictly additive / zero regression**: the fatal set is unchanged (syntax, + brace-in-CEL, unknown-function still throw); everything the schema pass adds is + logged, never thrown, and the whole thing is a no-op when no resolver is wired. + Flow conditions bind fields flat, so the check runs in `flattened` scope + (flow variables stay `dyn` and are never flagged; equality is runtime-safe). + +Builds on the tier-4 type-soundness check in `@objectstack/formula` / +`@objectstack/lint` (#1928). diff --git a/packages/services/service-automation/src/engine.test.ts b/packages/services/service-automation/src/engine.test.ts index ee6bd62449..46a742646c 100644 --- a/packages/services/service-automation/src/engine.test.ts +++ b/packages/services/service-automation/src/engine.test.ts @@ -3,7 +3,7 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { LiteKernel } from '@objectstack/core'; import { AutomationEngine, DEFAULT_MAX_EXECUTION_LOG_SIZE } from './engine.js'; -import { AutomationServicePlugin } from './plugin.js'; +import { AutomationServicePlugin, parseObjectFieldSchema } from './plugin.js'; import { registerScreenNodes } from './builtin/screen-nodes.js'; import { InMemorySuspendedRunStore } from './suspended-run-store.js'; import type { NodeExecutor } from './engine.js'; @@ -821,6 +821,117 @@ describe('AutomationEngine', () => { }); }); +// ─── Schema-aware condition validation at registration (#1928) ─────── + +describe('Schema-aware condition validation at registration (#1928)', () => { + // A logger that captures warnings, so we can assert the advisory channel. + function loggerCapturing(warns: string[]) { + const l: any = { + info: () => {}, error: () => {}, debug: () => {}, + warn: (m: string) => warns.push(m), + child: () => l, + }; + return l; + } + + const OPP_SCHEMA = { + crm_opportunity: { + fields: ['stage', 'amount', 'is_active', 'title'], + fieldTypes: { stage: 'select', amount: 'currency', is_active: 'boolean', title: 'text' } as Record, + } as { fields: string[]; fieldTypes: Record }, + }; + + function makeFlow(condition: string) { + return { + name: 'opp_flow', label: 'Opp Flow', type: 'record_change', + nodes: [ + { id: 'start', type: 'start', label: 'Start', config: { objectName: 'crm_opportunity' } }, + { id: 'check', type: 'decision', label: 'Check', config: { condition } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'check' }, + { id: 'e2', source: 'check', target: 'end' }, + ], + }; + } + + it('logs an advisory warning (does NOT throw) for a text field misused in arithmetic — tier 4', () => { + const warns: string[] = []; + const engine = new AutomationEngine(loggerCapturing(warns)); + engine.setObjectSchemaResolver((name) => (OPP_SCHEMA as Record)[name] as any); + expect(() => engine.registerFlow('opp_flow', makeFlow('title * 2 > 10'))).not.toThrow(); + const w = warns.filter((m) => /type mismatch/i.test(m)); + expect(w).toHaveLength(1); + expect(w[0]).toMatch(/`title`/); + }); + + it('logs an advisory warning for a likely field typo — tier 3', () => { + const warns: string[] = []; + const engine = new AutomationEngine(loggerCapturing(warns)); + engine.setObjectSchemaResolver((name) => (OPP_SCHEMA as Record)[name] as any); + expect(() => engine.registerFlow('opp_flow', makeFlow('stagee == "closed_won"'))).not.toThrow(); + expect(warns.some((m) => /did you mean `stage`/.test(m))).toBe(true); + }); + + it('logs an advisory warning for an unknown record field — tier 2', () => { + const warns: string[] = []; + const engine = new AutomationEngine(loggerCapturing(warns)); + engine.setObjectSchemaResolver((name) => (OPP_SCHEMA as Record)[name] as any); + expect(() => engine.registerFlow('opp_flow', makeFlow('record.amont > 5'))).not.toThrow(); + expect(warns.some((m) => /unknown field `amont`/.test(m))).toBe(true); + }); + + it('does not warn on sound conditions (number arithmetic, equality, flow variables)', () => { + const warns: string[] = []; + const engine = new AutomationEngine(loggerCapturing(warns)); + engine.setObjectSchemaResolver((name) => (OPP_SCHEMA as Record)[name] as any); + engine.registerFlow('opp_flow', makeFlow('amount / 100 > 5 && stage == "won" && expiring_count * 2 > 3')); + expect(warns.filter((m) => /type mismatch|did you mean|unknown field/i.test(m))).toHaveLength(0); + }); + + it('still HARD-FAILS a malformed condition (fatal set unchanged)', () => { + const warns: string[] = []; + const engine = new AutomationEngine(loggerCapturing(warns)); + engine.setObjectSchemaResolver((name) => (OPP_SCHEMA as Record)[name] as any); + expect(() => engine.registerFlow('opp_flow', makeFlow('{record.stage} == "won"'))).toThrow(/template braces|bare CEL/); + }); + + it('is a no-op when no resolver is wired (registration behaviour unchanged)', () => { + const warns: string[] = []; + const engine = new AutomationEngine(loggerCapturing(warns)); + // No setObjectSchemaResolver — a tier-4 mistake registers with NO advisory. + expect(() => engine.registerFlow('opp_flow', makeFlow('title * 2 > 10'))).not.toThrow(); + expect(warns.filter((m) => /type mismatch|did you mean|unknown field/i.test(m))).toHaveLength(0); + }); +}); + +describe('parseObjectFieldSchema (#1928 — object-registry → schema hint)', () => { + it('normalizes a name-keyed field map (the ObjectQL ServiceObject shape)', () => { + const r = parseObjectFieldSchema({ title: { type: 'text' }, amount: { type: 'currency' }, done: { type: 'boolean' } }); + expect(r?.fields.sort()).toEqual(['amount', 'done', 'title']); + expect(r?.fieldTypes).toEqual({ title: 'text', amount: 'currency', done: 'boolean' }); + }); + + it('normalizes an array of {name, type}', () => { + const r = parseObjectFieldSchema([{ name: 'title', type: 'text' }, { name: 'amount', type: 'currency' }]); + expect(r?.fields).toEqual(['title', 'amount']); + expect(r?.fieldTypes).toEqual({ title: 'text', amount: 'currency' }); + }); + + it('lists a field with a non-string type but assigns it no type (→ dyn)', () => { + const r = parseObjectFieldSchema({ meta: {}, name: { type: 'text' } }); + expect(r?.fields.sort()).toEqual(['meta', 'name']); + expect(r?.fieldTypes).toEqual({ name: 'text' }); + }); + + it('returns undefined for a non-object fields value', () => { + expect(parseObjectFieldSchema(undefined)).toBeUndefined(); + expect(parseObjectFieldSchema(null)).toBeUndefined(); + expect(parseObjectFieldSchema('nope')).toBeUndefined(); + }); +}); + // ─── Plugin Integration Tests ──────────────────────────────────────── describe('AutomationServicePlugin (Kernel Integration)', () => { diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index fa9407ea9e..d321d4896b 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -234,6 +234,22 @@ export type FlowFunctionHandler = (ctx: FlowFunctionContext) => unknown | Promis */ export type FlowFunctionResolver = (name: string) => FlowFunctionHandler | undefined; +/** + * Resolves the schema of the object a flow's conditions bind against — its field + * names and (spec) types — so `registerFlow` can run the same schema-aware + * expression checks as `objectstack build` (ADR-0032 tiers 2–4): unknown + * `record.` refs, likely bare-field typos, and text/boolean fields + * misused in arithmetic (#1928). Injected by the host (bridged to the object + * registry), so the engine stays decoupled from any metadata store. Returns + * `undefined` for an unknown object. When unwired, registration validation is + * unchanged (syntax + bare-ref only). Everything it surfaces is advisory (logged + * as a warning, never thrown) — a resolver can never break a flow that used to + * register cleanly. + */ +export type FlowObjectSchemaResolver = ( + objectName: string, +) => { fields?: readonly string[]; fieldTypes?: Record } | undefined; + /** * A designer-facing view of one connector action — identity + its JSON-Schema * input/output. The runtime handler is intentionally omitted; this is metadata. @@ -516,6 +532,9 @@ export class AutomationEngine implements IAutomationService { private connectorProviders = new Map(); /** Bridge to the host function registry for `script`-node calls (#1870), if wired. */ private functionResolver: FlowFunctionResolver | null = null; + /** Bridge to the host object registry for schema-aware condition validation at + * registration (#1928), if wired. Advisory-only — see {@link FlowObjectSchemaResolver}. */ + private objectSchemaResolver: FlowObjectSchemaResolver | null = null; private executionLogs: ExecutionLogEntry[] = []; private readonly maxLogSize: number; private logger: Logger; @@ -934,6 +953,18 @@ export class AutomationEngine implements IAutomationService { this.functionResolver = resolver; } + /** + * Wire the engine to the host's object registry (#1928) so `registerFlow` + * runs the same schema-aware condition checks as `objectstack build` — + * unknown-field refs, likely bare-field typos, and text/boolean fields + * misused in arithmetic. Everything it adds is advisory (logged, never + * thrown); passing `null` detaches the bridge (registration reverts to + * syntax + bare-ref validation only). + */ + setObjectSchemaResolver(resolver: FlowObjectSchemaResolver | null): void { + this.objectSchemaResolver = resolver; + } + /** * Resolve a named function for a `script` node. Returns `undefined` when no * resolver is wired or the name is unregistered — the node then fails the @@ -2066,19 +2097,49 @@ export class AutomationEngine implements IAutomationService { * Only the *predicate* surfaces are checked here (start/node `config.condition` * and `edge.condition`) — node string fields are templates (a different * dialect) and are validated by the template engine, not as CEL. + * + * #1928 — when an object-schema resolver is wired ({@link setObjectSchemaResolver}), + * a second, schema-aware pass surfaces the checks `objectstack build` runs + * (unknown-field refs, likely bare-field typos, text/boolean fields misused + * in arithmetic) as ADVISORY warnings (logged, never thrown). Flow conditions + * bind the record's fields flat, so the schema pass uses `flattened` scope. + * The fatal set is unchanged — a resolver can never break a flow that used to + * register cleanly. */ private validateFlowExpressions(flowName: string, flow: FlowParsed): void { const failures: string[] = []; + // Resolve the flow's record-change target object (start node's + // `config.objectName`) so `record.*`/bare field refs can be checked + // against the real schema. Absent resolver / non-record flow → the hint + // is undefined and only the fatal syntax/bare-ref pass runs (unchanged). + const startNode = flow.nodes.find((n) => n.type === 'start'); + const objectName = ((startNode?.config ?? {}) as Record).objectName; + const schemaHint = typeof objectName === 'string' + ? (() => { + const s = this.objectSchemaResolver?.(objectName); + return s ? { objectName, fields: s.fields, fieldTypes: s.fieldTypes, scope: 'flattened' as const } : undefined; + })() + : undefined; + const check = (where: string, raw: unknown): void => { if (raw == null) return; - // Conditions are predicates (bare CEL). Delegate to the one shared - // validator (ADR-0032 §5) so the corrective message matches the CLI - // build and the agent `validate_expression` tool exactly. + // Fatal pass — syntax, brace-in-CEL, unknown-function (ADR-0032 §5). + // Unchanged: matches the CLI build's fatal set exactly. const result = validateExpression('predicate', raw as string | { dialect?: string; source?: string }); for (const e of result.errors) { failures.push(` • ${where}: ${e.message}\n source: \`${e.source}\``); } + // Advisory schema-aware pass — only when the source is syntactically + // valid (else it would just re-report the fatal error). Everything it + // finds (field-existence, tier-3 typo, tier-4 type mismatch) is LOGGED, + // never thrown, so registration behaviour is strictly additive (#1928). + if (result.errors.length === 0 && schemaHint) { + const schemaPass = validateExpression('predicate', raw as string | { dialect?: string; source?: string }, schemaHint); + for (const issue of [...schemaPass.errors, ...schemaPass.warnings]) { + this.logger.warn(`[flow '${flowName}'] ${where}: ${issue.message}\n source: \`${issue.source}\``); + } + } }; for (const node of flow.nodes) { diff --git a/packages/services/service-automation/src/plugin.ts b/packages/services/service-automation/src/plugin.ts index ff819c60b5..1215b3d62b 100644 --- a/packages/services/service-automation/src/plugin.ts +++ b/packages/services/service-automation/src/plugin.ts @@ -18,6 +18,37 @@ import { type SuspendedRunStoreEngine, } from './suspended-run-store.js'; +/** + * #1928 — normalize an ObjectQL object's `fields` (a name-keyed map, or an + * array of `{name, type}`) into the `{ fields, fieldTypes }` shape the flow + * condition validator consumes. Fields whose `type` isn't a string are still + * listed (so existence checks work) but contribute no type (→ treated as `dyn`). + * Returns `undefined` when there is nothing usable. Exported for unit testing. + */ +export function parseObjectFieldSchema( + fieldsDef: unknown, +): { fields: string[]; fieldTypes: Record } | undefined { + if (!fieldsDef || typeof fieldsDef !== 'object') return undefined; + const fields: string[] = []; + const fieldTypes: Record = {}; + if (Array.isArray(fieldsDef)) { + for (const f of fieldsDef as Array>) { + const n = (f as { name?: unknown })?.name; + if (typeof n !== 'string') continue; + fields.push(n); + const t = (f as { type?: unknown })?.type; + if (typeof t === 'string') fieldTypes[n] = t; + } + } else { + for (const [n, def] of Object.entries(fieldsDef as Record)) { + fields.push(n); + const t = (def as { type?: unknown })?.type; + if (typeof t === 'string') fieldTypes[n] = t; + } + } + return { fields, fieldTypes }; +} + /** * Configuration options for the AutomationServicePlugin. */ @@ -439,6 +470,27 @@ export class AutomationServicePlugin implements Plugin { ctx.logger.debug('[Automation] objectql not present — script-node function calls will fail loudly when used'); } + // #1928 — bridge the object registry so `registerFlow` runs the same + // schema-aware condition checks as `objectstack build`: unknown-field + // refs, likely bare-field typos, and text/boolean fields misused in + // arithmetic. Everything it surfaces is advisory (logged, never thrown), + // so a dynamically-registered flow (which bypasses the build lint) still + // gets the guardrail. Wired BEFORE the flow pull below so pulled flows + // are covered too. Best-effort: without ObjectQL, registration falls back + // to syntax + bare-ref validation only. + try { + const ql = ctx.getService<{ + registry?: { getObject?: (name: string) => unknown }; + }>('objectql'); + if (ql?.registry && typeof ql.registry.getObject === 'function') { + this.engine.setObjectSchemaResolver((objectName) => + parseObjectFieldSchema((ql.registry!.getObject!(objectName) as { fields?: unknown } | undefined)?.fields)); + ctx.logger.debug('[Automation] object-schema resolver bridged to objectql.registry (#1928 condition checks)'); + } + } catch { + ctx.logger.debug('[Automation] objectql registry not present — flow-condition checks limited to syntax'); + } + // Pull flow definitions from the ObjectQL schema registry. AppPlugin.init() // calls manifest.register(payload), which routes to ql.registerApp() and // stores each inline flow under type 'flow'. By the time start() runs,