Skip to content

Commit 780b4b5

Browse files
os-zhuangclaude
andauthored
feat(automation): schema-aware flow-condition validation at registration (#1928) (#3190)
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. Claude-Session: https://claude.ai/code/session_01Hnji7EEYR2mGt6pY53a8Bm Co-authored-by: Claude <noreply@anthropic.com>
1 parent f16b492 commit 780b4b5

4 files changed

Lines changed: 254 additions & 4 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
"@objectstack/service-automation": minor
3+
---
4+
5+
feat(automation): schema-aware flow-condition validation at registration (#1928)
6+
7+
`registerFlow` now runs the same schema-aware condition checks as
8+
`objectstack build` — so a flow registered dynamically (via the API / Studio,
9+
bypassing the build lint) still gets the guardrail. When the host wires an
10+
object-schema resolver, a flow condition that references an unknown field,
11+
likely-typos a field name, or does arithmetic/ordering on a text/boolean field
12+
against a number is surfaced as an **advisory warning** (logged), pointing at
13+
the object's real schema.
14+
15+
- New `AutomationEngine.setObjectSchemaResolver(resolver)` bridge (mirrors
16+
`setFunctionResolver`); `AutomationServicePlugin` wires it to
17+
`objectql.registry.getObject` in `start()`, before the flow pull, so
18+
registry-sourced flows are covered too.
19+
- **Strictly additive / zero regression**: the fatal set is unchanged (syntax,
20+
brace-in-CEL, unknown-function still throw); everything the schema pass adds is
21+
logged, never thrown, and the whole thing is a no-op when no resolver is wired.
22+
Flow conditions bind fields flat, so the check runs in `flattened` scope
23+
(flow variables stay `dyn` and are never flagged; equality is runtime-safe).
24+
25+
Builds on the tier-4 type-soundness check in `@objectstack/formula` /
26+
`@objectstack/lint` (#1928).

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

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import { describe, it, expect, beforeEach } from 'vitest';
44
import { LiteKernel } from '@objectstack/core';
55
import { AutomationEngine, DEFAULT_MAX_EXECUTION_LOG_SIZE } from './engine.js';
6-
import { AutomationServicePlugin } from './plugin.js';
6+
import { AutomationServicePlugin, parseObjectFieldSchema } from './plugin.js';
77
import { registerScreenNodes } from './builtin/screen-nodes.js';
88
import { InMemorySuspendedRunStore } from './suspended-run-store.js';
99
import type { NodeExecutor } from './engine.js';
@@ -821,6 +821,117 @@ describe('AutomationEngine', () => {
821821
});
822822
});
823823

824+
// ─── Schema-aware condition validation at registration (#1928) ───────
825+
826+
describe('Schema-aware condition validation at registration (#1928)', () => {
827+
// A logger that captures warnings, so we can assert the advisory channel.
828+
function loggerCapturing(warns: string[]) {
829+
const l: any = {
830+
info: () => {}, error: () => {}, debug: () => {},
831+
warn: (m: string) => warns.push(m),
832+
child: () => l,
833+
};
834+
return l;
835+
}
836+
837+
const OPP_SCHEMA = {
838+
crm_opportunity: {
839+
fields: ['stage', 'amount', 'is_active', 'title'],
840+
fieldTypes: { stage: 'select', amount: 'currency', is_active: 'boolean', title: 'text' } as Record<string, string>,
841+
} as { fields: string[]; fieldTypes: Record<string, string> },
842+
};
843+
844+
function makeFlow(condition: string) {
845+
return {
846+
name: 'opp_flow', label: 'Opp Flow', type: 'record_change',
847+
nodes: [
848+
{ id: 'start', type: 'start', label: 'Start', config: { objectName: 'crm_opportunity' } },
849+
{ id: 'check', type: 'decision', label: 'Check', config: { condition } },
850+
{ id: 'end', type: 'end', label: 'End' },
851+
],
852+
edges: [
853+
{ id: 'e1', source: 'start', target: 'check' },
854+
{ id: 'e2', source: 'check', target: 'end' },
855+
],
856+
};
857+
}
858+
859+
it('logs an advisory warning (does NOT throw) for a text field misused in arithmetic — tier 4', () => {
860+
const warns: string[] = [];
861+
const engine = new AutomationEngine(loggerCapturing(warns));
862+
engine.setObjectSchemaResolver((name) => (OPP_SCHEMA as Record<string, unknown>)[name] as any);
863+
expect(() => engine.registerFlow('opp_flow', makeFlow('title * 2 > 10'))).not.toThrow();
864+
const w = warns.filter((m) => /type mismatch/i.test(m));
865+
expect(w).toHaveLength(1);
866+
expect(w[0]).toMatch(/`title`/);
867+
});
868+
869+
it('logs an advisory warning for a likely field typo — tier 3', () => {
870+
const warns: string[] = [];
871+
const engine = new AutomationEngine(loggerCapturing(warns));
872+
engine.setObjectSchemaResolver((name) => (OPP_SCHEMA as Record<string, unknown>)[name] as any);
873+
expect(() => engine.registerFlow('opp_flow', makeFlow('stagee == "closed_won"'))).not.toThrow();
874+
expect(warns.some((m) => /did you mean `stage`/.test(m))).toBe(true);
875+
});
876+
877+
it('logs an advisory warning for an unknown record field — tier 2', () => {
878+
const warns: string[] = [];
879+
const engine = new AutomationEngine(loggerCapturing(warns));
880+
engine.setObjectSchemaResolver((name) => (OPP_SCHEMA as Record<string, unknown>)[name] as any);
881+
expect(() => engine.registerFlow('opp_flow', makeFlow('record.amont > 5'))).not.toThrow();
882+
expect(warns.some((m) => /unknown field `amont`/.test(m))).toBe(true);
883+
});
884+
885+
it('does not warn on sound conditions (number arithmetic, equality, flow variables)', () => {
886+
const warns: string[] = [];
887+
const engine = new AutomationEngine(loggerCapturing(warns));
888+
engine.setObjectSchemaResolver((name) => (OPP_SCHEMA as Record<string, unknown>)[name] as any);
889+
engine.registerFlow('opp_flow', makeFlow('amount / 100 > 5 && stage == "won" && expiring_count * 2 > 3'));
890+
expect(warns.filter((m) => /type mismatch|did you mean|unknown field/i.test(m))).toHaveLength(0);
891+
});
892+
893+
it('still HARD-FAILS a malformed condition (fatal set unchanged)', () => {
894+
const warns: string[] = [];
895+
const engine = new AutomationEngine(loggerCapturing(warns));
896+
engine.setObjectSchemaResolver((name) => (OPP_SCHEMA as Record<string, unknown>)[name] as any);
897+
expect(() => engine.registerFlow('opp_flow', makeFlow('{record.stage} == "won"'))).toThrow(/template braces|bare CEL/);
898+
});
899+
900+
it('is a no-op when no resolver is wired (registration behaviour unchanged)', () => {
901+
const warns: string[] = [];
902+
const engine = new AutomationEngine(loggerCapturing(warns));
903+
// No setObjectSchemaResolver — a tier-4 mistake registers with NO advisory.
904+
expect(() => engine.registerFlow('opp_flow', makeFlow('title * 2 > 10'))).not.toThrow();
905+
expect(warns.filter((m) => /type mismatch|did you mean|unknown field/i.test(m))).toHaveLength(0);
906+
});
907+
});
908+
909+
describe('parseObjectFieldSchema (#1928 — object-registry → schema hint)', () => {
910+
it('normalizes a name-keyed field map (the ObjectQL ServiceObject shape)', () => {
911+
const r = parseObjectFieldSchema({ title: { type: 'text' }, amount: { type: 'currency' }, done: { type: 'boolean' } });
912+
expect(r?.fields.sort()).toEqual(['amount', 'done', 'title']);
913+
expect(r?.fieldTypes).toEqual({ title: 'text', amount: 'currency', done: 'boolean' });
914+
});
915+
916+
it('normalizes an array of {name, type}', () => {
917+
const r = parseObjectFieldSchema([{ name: 'title', type: 'text' }, { name: 'amount', type: 'currency' }]);
918+
expect(r?.fields).toEqual(['title', 'amount']);
919+
expect(r?.fieldTypes).toEqual({ title: 'text', amount: 'currency' });
920+
});
921+
922+
it('lists a field with a non-string type but assigns it no type (→ dyn)', () => {
923+
const r = parseObjectFieldSchema({ meta: {}, name: { type: 'text' } });
924+
expect(r?.fields.sort()).toEqual(['meta', 'name']);
925+
expect(r?.fieldTypes).toEqual({ name: 'text' });
926+
});
927+
928+
it('returns undefined for a non-object fields value', () => {
929+
expect(parseObjectFieldSchema(undefined)).toBeUndefined();
930+
expect(parseObjectFieldSchema(null)).toBeUndefined();
931+
expect(parseObjectFieldSchema('nope')).toBeUndefined();
932+
});
933+
});
934+
824935
// ─── Plugin Integration Tests ────────────────────────────────────────
825936

826937
describe('AutomationServicePlugin (Kernel Integration)', () => {

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

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,22 @@ export type FlowFunctionHandler = (ctx: FlowFunctionContext) => unknown | Promis
234234
*/
235235
export type FlowFunctionResolver = (name: string) => FlowFunctionHandler | undefined;
236236

237+
/**
238+
* Resolves the schema of the object a flow's conditions bind against — its field
239+
* names and (spec) types — so `registerFlow` can run the same schema-aware
240+
* expression checks as `objectstack build` (ADR-0032 tiers 2–4): unknown
241+
* `record.<field>` refs, likely bare-field typos, and text/boolean fields
242+
* misused in arithmetic (#1928). Injected by the host (bridged to the object
243+
* registry), so the engine stays decoupled from any metadata store. Returns
244+
* `undefined` for an unknown object. When unwired, registration validation is
245+
* unchanged (syntax + bare-ref only). Everything it surfaces is advisory (logged
246+
* as a warning, never thrown) — a resolver can never break a flow that used to
247+
* register cleanly.
248+
*/
249+
export type FlowObjectSchemaResolver = (
250+
objectName: string,
251+
) => { fields?: readonly string[]; fieldTypes?: Record<string, string> } | undefined;
252+
237253
/**
238254
* A designer-facing view of one connector action — identity + its JSON-Schema
239255
* input/output. The runtime handler is intentionally omitted; this is metadata.
@@ -516,6 +532,9 @@ export class AutomationEngine implements IAutomationService {
516532
private connectorProviders = new Map<string, ConnectorProviderFactory>();
517533
/** Bridge to the host function registry for `script`-node calls (#1870), if wired. */
518534
private functionResolver: FlowFunctionResolver | null = null;
535+
/** Bridge to the host object registry for schema-aware condition validation at
536+
* registration (#1928), if wired. Advisory-only — see {@link FlowObjectSchemaResolver}. */
537+
private objectSchemaResolver: FlowObjectSchemaResolver | null = null;
519538
private executionLogs: ExecutionLogEntry[] = [];
520539
private readonly maxLogSize: number;
521540
private logger: Logger;
@@ -934,6 +953,18 @@ export class AutomationEngine implements IAutomationService {
934953
this.functionResolver = resolver;
935954
}
936955

956+
/**
957+
* Wire the engine to the host's object registry (#1928) so `registerFlow`
958+
* runs the same schema-aware condition checks as `objectstack build` —
959+
* unknown-field refs, likely bare-field typos, and text/boolean fields
960+
* misused in arithmetic. Everything it adds is advisory (logged, never
961+
* thrown); passing `null` detaches the bridge (registration reverts to
962+
* syntax + bare-ref validation only).
963+
*/
964+
setObjectSchemaResolver(resolver: FlowObjectSchemaResolver | null): void {
965+
this.objectSchemaResolver = resolver;
966+
}
967+
937968
/**
938969
* Resolve a named function for a `script` node. Returns `undefined` when no
939970
* resolver is wired or the name is unregistered — the node then fails the
@@ -2066,19 +2097,49 @@ export class AutomationEngine implements IAutomationService {
20662097
* Only the *predicate* surfaces are checked here (start/node `config.condition`
20672098
* and `edge.condition`) — node string fields are templates (a different
20682099
* dialect) and are validated by the template engine, not as CEL.
2100+
*
2101+
* #1928 — when an object-schema resolver is wired ({@link setObjectSchemaResolver}),
2102+
* a second, schema-aware pass surfaces the checks `objectstack build` runs
2103+
* (unknown-field refs, likely bare-field typos, text/boolean fields misused
2104+
* in arithmetic) as ADVISORY warnings (logged, never thrown). Flow conditions
2105+
* bind the record's fields flat, so the schema pass uses `flattened` scope.
2106+
* The fatal set is unchanged — a resolver can never break a flow that used to
2107+
* register cleanly.
20692108
*/
20702109
private validateFlowExpressions(flowName: string, flow: FlowParsed): void {
20712110
const failures: string[] = [];
20722111

2112+
// Resolve the flow's record-change target object (start node's
2113+
// `config.objectName`) so `record.*`/bare field refs can be checked
2114+
// against the real schema. Absent resolver / non-record flow → the hint
2115+
// is undefined and only the fatal syntax/bare-ref pass runs (unchanged).
2116+
const startNode = flow.nodes.find((n) => n.type === 'start');
2117+
const objectName = ((startNode?.config ?? {}) as Record<string, unknown>).objectName;
2118+
const schemaHint = typeof objectName === 'string'
2119+
? (() => {
2120+
const s = this.objectSchemaResolver?.(objectName);
2121+
return s ? { objectName, fields: s.fields, fieldTypes: s.fieldTypes, scope: 'flattened' as const } : undefined;
2122+
})()
2123+
: undefined;
2124+
20732125
const check = (where: string, raw: unknown): void => {
20742126
if (raw == null) return;
2075-
// Conditions are predicates (bare CEL). Delegate to the one shared
2076-
// validator (ADR-0032 §5) so the corrective message matches the CLI
2077-
// build and the agent `validate_expression` tool exactly.
2127+
// Fatal pass — syntax, brace-in-CEL, unknown-function (ADR-0032 §5).
2128+
// Unchanged: matches the CLI build's fatal set exactly.
20782129
const result = validateExpression('predicate', raw as string | { dialect?: string; source?: string });
20792130
for (const e of result.errors) {
20802131
failures.push(` • ${where}: ${e.message}\n source: \`${e.source}\``);
20812132
}
2133+
// Advisory schema-aware pass — only when the source is syntactically
2134+
// valid (else it would just re-report the fatal error). Everything it
2135+
// finds (field-existence, tier-3 typo, tier-4 type mismatch) is LOGGED,
2136+
// never thrown, so registration behaviour is strictly additive (#1928).
2137+
if (result.errors.length === 0 && schemaHint) {
2138+
const schemaPass = validateExpression('predicate', raw as string | { dialect?: string; source?: string }, schemaHint);
2139+
for (const issue of [...schemaPass.errors, ...schemaPass.warnings]) {
2140+
this.logger.warn(`[flow '${flowName}'] ${where}: ${issue.message}\n source: \`${issue.source}\``);
2141+
}
2142+
}
20822143
};
20832144

20842145
for (const node of flow.nodes) {

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,37 @@ import {
1818
type SuspendedRunStoreEngine,
1919
} from './suspended-run-store.js';
2020

21+
/**
22+
* #1928 — normalize an ObjectQL object's `fields` (a name-keyed map, or an
23+
* array of `{name, type}`) into the `{ fields, fieldTypes }` shape the flow
24+
* condition validator consumes. Fields whose `type` isn't a string are still
25+
* listed (so existence checks work) but contribute no type (→ treated as `dyn`).
26+
* Returns `undefined` when there is nothing usable. Exported for unit testing.
27+
*/
28+
export function parseObjectFieldSchema(
29+
fieldsDef: unknown,
30+
): { fields: string[]; fieldTypes: Record<string, string> } | undefined {
31+
if (!fieldsDef || typeof fieldsDef !== 'object') return undefined;
32+
const fields: string[] = [];
33+
const fieldTypes: Record<string, string> = {};
34+
if (Array.isArray(fieldsDef)) {
35+
for (const f of fieldsDef as Array<Record<string, unknown>>) {
36+
const n = (f as { name?: unknown })?.name;
37+
if (typeof n !== 'string') continue;
38+
fields.push(n);
39+
const t = (f as { type?: unknown })?.type;
40+
if (typeof t === 'string') fieldTypes[n] = t;
41+
}
42+
} else {
43+
for (const [n, def] of Object.entries(fieldsDef as Record<string, unknown>)) {
44+
fields.push(n);
45+
const t = (def as { type?: unknown })?.type;
46+
if (typeof t === 'string') fieldTypes[n] = t;
47+
}
48+
}
49+
return { fields, fieldTypes };
50+
}
51+
2152
/**
2253
* Configuration options for the AutomationServicePlugin.
2354
*/
@@ -439,6 +470,27 @@ export class AutomationServicePlugin implements Plugin {
439470
ctx.logger.debug('[Automation] objectql not present — script-node function calls will fail loudly when used');
440471
}
441472

473+
// #1928 — bridge the object registry so `registerFlow` runs the same
474+
// schema-aware condition checks as `objectstack build`: unknown-field
475+
// refs, likely bare-field typos, and text/boolean fields misused in
476+
// arithmetic. Everything it surfaces is advisory (logged, never thrown),
477+
// so a dynamically-registered flow (which bypasses the build lint) still
478+
// gets the guardrail. Wired BEFORE the flow pull below so pulled flows
479+
// are covered too. Best-effort: without ObjectQL, registration falls back
480+
// to syntax + bare-ref validation only.
481+
try {
482+
const ql = ctx.getService<{
483+
registry?: { getObject?: (name: string) => unknown };
484+
}>('objectql');
485+
if (ql?.registry && typeof ql.registry.getObject === 'function') {
486+
this.engine.setObjectSchemaResolver((objectName) =>
487+
parseObjectFieldSchema((ql.registry!.getObject!(objectName) as { fields?: unknown } | undefined)?.fields));
488+
ctx.logger.debug('[Automation] object-schema resolver bridged to objectql.registry (#1928 condition checks)');
489+
}
490+
} catch {
491+
ctx.logger.debug('[Automation] objectql registry not present — flow-condition checks limited to syntax');
492+
}
493+
442494
// Pull flow definitions from the ObjectQL schema registry. AppPlugin.init()
443495
// calls manifest.register(payload), which routes to ql.registerApp() and
444496
// stores each inline flow under type 'flow'. By the time start() runs,

0 commit comments

Comments
 (0)