diff --git a/.changeset/seed-skip-flows-boolean-coercion-loop-guard.md b/.changeset/seed-skip-flows-boolean-coercion-loop-guard.md new file mode 100644 index 0000000000..881b0524b4 --- /dev/null +++ b/.changeset/seed-skip-flows-boolean-coercion-loop-guard.md @@ -0,0 +1,28 @@ +--- +'@objectstack/spec': patch +'@objectstack/objectql': patch +'@objectstack/metadata-protocol': patch +'@objectstack/trigger-record-change': patch +'@objectstack/service-automation': patch +--- + +Package metadata seed can no longer wedge the platform via record-change automation. + +A seeded record whose lifecycle flow self-triggered (a `record-after-update` flow +writing back to its own trigger record) looped forever when its boolean re-entry +guard never tripped — booleans persist as integer `1` on SQLite/libsql and CEL +`1 != true` is `true`. During first-boot seed (which awaits automation) this hung +the whole kernel build. + +Three layers: +- `ExecutionContext.skipTriggers` (set by the seed-loader, threaded onto + `HookContext.session` via `buildSession`) makes the record-change trigger skip + flow dispatch for seed/bulk writes — seed data is end-state reference data, not + user events. Lifecycle hooks still run. +- `coerceBooleanFields()` converts SQLite 0/1 (and `'0'/'1'/'true'/'false'`) to + real booleans on the after-hook view of a record (`hookContext.result` / + `.previous`), so flow conditions see JS booleans. The value returned to the + caller is unchanged. +- The automation engine breaks a flow re-entering for the same record while an + execution is still on the stack (`activeRecordFlows`), a backstop for any + self-trigger loop. diff --git a/packages/metadata-protocol/src/seed-loader.ts b/packages/metadata-protocol/src/seed-loader.ts index 16ded64dd2..20a5f22cd6 100644 --- a/packages/metadata-protocol/src/seed-loader.ts +++ b/packages/metadata-protocol/src/seed-loader.ts @@ -606,8 +606,16 @@ export class SeedLoaderService implements ISeedLoaderService { * disables the SecurityPlugin's auto-injection of `organization_id` / * `owner_id` — seeds either declare those fields explicitly per * record, or are intentionally cross-tenant / global. + * + * `skipTriggers` suppresses record-change AUTOMATION (autolaunched flow + * triggers) for seed writes: a package's seed is pre-existing END-STATE + * reference/sample data, not a stream of user events, so firing + * on-create/on-update flows (notifications, escalations, assignments, + * approvals) for it is semantically wrong and dangerous — a self-triggering + * flow can loop and wedge the whole first-boot (2026-07-06 incident). + * Lifecycle HOOKS (derived/default fields, validation) still run. */ - private static readonly SEED_OPTIONS = { context: { isSystem: true } } as const; + private static readonly SEED_OPTIONS = { context: { isSystem: true, skipTriggers: true } } as const; private async writeRecord( objectName: string, diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 79e65ccaf6..1c721e4c02 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -30,7 +30,7 @@ import { ExpressionEngine } from '@objectstack/formula'; import type { Expression } from '@objectstack/spec'; import { isAggregatedViewContainer, expandViewContainer } from '@objectstack/spec'; import { bindHooksToEngine } from './hook-binder.js'; -import { validateRecord, normalizeMultiValueFields } from './validation/record-validator.js'; +import { validateRecord, normalizeMultiValueFields, coerceBooleanFields } from './validation/record-validator.js'; import { evaluateValidationRules, needsPriorRecord, stripReadonlyWhenFields } from './validation/rule-validator.js'; import { applyInMemoryAggregation } from './in-memory-aggregation.js'; @@ -690,6 +690,10 @@ export class ObjectQL implements IDataEngine { // Propagate system-elevated flag so hooks can distinguish engine // self-writes (e.g. approval status mirror) from genuine user writes. ...((execCtx as any).isSystem ? { isSystem: true } : {}), + // Propagate the automation-suppression flag so the record-change trigger + // can skip flow dispatch for seed/bulk writes (ADR: seed loads end-state + // data, not user events). + ...((execCtx as any).skipTriggers ? { skipTriggers: true } : {}), } as HookContext['session']; } @@ -2189,7 +2193,13 @@ export class ObjectQL implements IDataEngine { } hookContext.event = 'afterInsert'; - hookContext.result = result; + // Coerce `boolean` fields (SQLite/libsql return 0/1) to real booleans on + // the after-hook view so flow trigger conditions (`record.is_escalated + // != true`) and `{record.}` interpolation see JS booleans, not + // ints. A shallow copy — the value returned to the caller is untouched. + hookContext.result = Array.isArray(result) + ? result.map((r) => coerceBooleanFields(schemaForValidation as any, r as any)) + : coerceBooleanFields(schemaForValidation as any, result as any); await this.triggerHooks('afterInsert', hookContext); // Roll-up: recompute parent summary fields that aggregate this object. @@ -2328,8 +2338,14 @@ export class ObjectQL implements IDataEngine { } hookContext.event = 'afterUpdate'; - hookContext.result = result; - if (priorRecord) hookContext.previous = priorRecord; + // Coerce boolean fields (SQLite 0/1 → JS bool) on the after-hook view + // of both the new row and the prior row, so flow conditions comparing + // `record.is_escalated`/`previous.status` against booleans behave. + // Shallow copies — the value returned to the caller is untouched. + hookContext.result = Array.isArray(result) + ? result.map((r) => coerceBooleanFields(updateSchema as any, r as any)) + : coerceBooleanFields(updateSchema as any, result as any); + if (priorRecord) hookContext.previous = coerceBooleanFields(updateSchema as any, priorRecord as any); await this.triggerHooks('afterUpdate', hookContext); // Roll-up: recompute parent summaries; pass priorRecord too so a child diff --git a/packages/objectql/src/validation/record-validator.test.ts b/packages/objectql/src/validation/record-validator.test.ts index 7e4f4b0e60..4abfa6764a 100644 --- a/packages/objectql/src/validation/record-validator.test.ts +++ b/packages/objectql/src/validation/record-validator.test.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; -import { validateRecord, normalizeMultiValueFields, ValidationError } from './record-validator.js'; +import { validateRecord, normalizeMultiValueFields, coerceBooleanFields, ValidationError } from './record-validator.js'; /** * Required-field validation, with the autonumber exemption (#1603). @@ -231,3 +231,60 @@ describe('ValidationError — top-level message is human-readable', () => { ]); }); }); + +describe('coerceBooleanFields — SQLite 0/1 → real booleans', () => { + const schema = { + fields: { + is_escalated: { type: 'boolean' }, + is_closed: { type: 'boolean' }, + active: { type: 'boolean' }, + name: { type: 'text' }, + priority: { type: 'select', options: ['low', 'critical'] }, + count: { type: 'number' }, + }, + }; + + it('coerces integer 0/1 on boolean fields, leaves others untouched', () => { + const row = { is_escalated: 1, is_closed: 0, name: 'Case', priority: 'critical', count: 5 }; + const out = coerceBooleanFields(schema, row); + expect(out.is_escalated).toBe(true); + expect(out.is_closed).toBe(false); + expect(out.name).toBe('Case'); + expect(out.priority).toBe('critical'); + expect(out.count).toBe(5); + }); + + it('fixes the incident predicate: `is_escalated != true` after coercion', () => { + const raw = { is_escalated: 1 }; + // Pre-coercion: an int 1 is NOT === true (the bug). + expect((raw.is_escalated as unknown) !== true).toBe(true); + const out = coerceBooleanFields(schema, raw); + // Post-coercion: the guard correctly suppresses re-fire. + expect(out.is_escalated !== true).toBe(false); + }); + + it('coerces string forms too', () => { + expect(coerceBooleanFields(schema, { active: '1' }).active).toBe(true); + expect(coerceBooleanFields(schema, { active: 'true' }).active).toBe(true); + expect(coerceBooleanFields(schema, { active: '0' }).active).toBe(false); + expect(coerceBooleanFields(schema, { active: 'false' }).active).toBe(false); + }); + + it('preserves null/undefined (nullable boolean stays null, not false)', () => { + const out = coerceBooleanFields(schema, { is_escalated: null, is_closed: undefined }); + expect(out.is_escalated).toBe(null); + expect(out.is_closed).toBe(undefined); + }); + + it('leaves real booleans and unrecognised strings as-is; no copy when nothing changes', () => { + expect(coerceBooleanFields(schema, { active: true }).active).toBe(true); + expect(coerceBooleanFields(schema, { active: 'maybe' }).active).toBe('maybe'); + const noBool = { name: 'x', count: 2 }; + expect(coerceBooleanFields(schema, noBool)).toBe(noBool); // same ref — untouched + }); + + it('is null/empty-safe', () => { + expect(coerceBooleanFields(undefined, { a: 1 } as any)).toEqual({ a: 1 }); + expect(coerceBooleanFields(schema, null as any)).toBe(null); + }); +}); diff --git a/packages/objectql/src/validation/record-validator.ts b/packages/objectql/src/validation/record-validator.ts index 56b5fa4795..b2e5f86493 100644 --- a/packages/objectql/src/validation/record-validator.ts +++ b/packages/objectql/src/validation/record-validator.ts @@ -170,6 +170,43 @@ export function normalizeMultiValueFields( } } +/** + * Coerce `boolean`-typed fields from their SQL storage form (integer `0`/`1`, + * or the strings `'0'`/`'1'`/`'true'`/`'false'`) into real JS booleans, on a + * SHALLOW COPY of `row`. SQLite/libsql have no native boolean, so a driver + * returns `1` for a `true` column — which then leaks into CEL/flow conditions + * where `record.is_escalated != true` becomes `1 != true` (always true, no + * int↔bool coercion) and a re-entry guard never trips (2026-07-06 infinite + * escalation loop). Returns the input unchanged when there is nothing to coerce. + * + * Only touches declared `boolean` fields; every other value is passed through. + * Null/undefined are preserved (a nullable boolean stays null, not `false`). + */ +export function coerceBooleanFields>( + objectSchema: { fields?: Record } | undefined | null, + row: T | undefined | null, +): T { + if (!objectSchema?.fields || !row || typeof row !== 'object') return row as T; + let copy: Record | undefined; + for (const [name, def] of Object.entries(objectSchema.fields)) { + if (!def || def.type !== 'boolean') continue; + if (!(name in row)) continue; + const v = (row as Record)[name]; + if (v === null || v === undefined || typeof v === 'boolean') continue; + let coerced: boolean; + if (typeof v === 'number') coerced = v !== 0; + else if (typeof v === 'string') { + const s = v.trim().toLowerCase(); + if (s === '1' || s === 'true') coerced = true; + else if (s === '0' || s === 'false' || s === '') coerced = false; + else continue; // unrecognised — leave as-is + } else continue; + if (!copy) copy = { ...(row as Record) }; + copy[name] = coerced; + } + return (copy ?? row) as T; +} + function validateOne(name: string, def: FieldDef, value: unknown): FieldValidationError | null { // ── required ──────────────────────────────────────────────────── // `autonumber` is runtime-owned: the value is generated by the engine / diff --git a/packages/services/service-automation/src/engine-reentrancy-guard.test.ts b/packages/services/service-automation/src/engine-reentrancy-guard.test.ts new file mode 100644 index 0000000000..08776344e3 --- /dev/null +++ b/packages/services/service-automation/src/engine-reentrancy-guard.test.ts @@ -0,0 +1,135 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Regression guard for the 2026-07-06 incident: a `record-after-update` flow + * whose action writes back to its OWN trigger record re-fires itself. Normally + * the start `condition` suppresses the second fire, but a broken guard makes it + * INFINITE — HotCRM's `case_escalation` guards on `record.is_escalated != true`, + * yet a `boolean` field persists as integer `1` on SQLite/libsql and CEL + * `1 != true` is `true`, so it never trips. During first-boot seed (which awaits + * automation to settle) that infinite cascade wedged the whole per-env kernel + * build, leaving the environment unopenable. + * + * The engine now breaks the SAME flow re-entering for the SAME record while an + * execution is still on the stack (see `activeRecordFlows`). This test drives + * the exact shape: a node executor that re-invokes `execute()` for the same + * flow+record, simulating the update→afterUpdate→dispatch→execute cascade. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { AutomationEngine } from './engine.js'; + +function createTestLogger() { + return { debug() {}, info() {}, warn() {}, error() {} } as any; +} + +describe('AutomationEngine — record-flow re-entrancy loop guard', () => { + let engine: AutomationEngine; + beforeEach(() => { + engine = new AutomationEngine(createTestLogger()); + }); + + it('breaks a self-triggering flow that re-fires for the same record', async () => { + let executeCalls = 0; + let skippedInner = false; + + // A node that mimics `case_escalation`'s action: it writes back to the + // trigger record, which (in the real runtime) re-dispatches the same flow + // for the same record. Here we invoke execute() directly to model that + // synchronous cascade. + engine.registerNodeExecutor({ + type: 'self_retrigger', + async execute(_node, _vars, _ctx?: any) { + executeCalls += 1; + if (executeCalls > 50) throw new Error('INFINITE LOOP — guard failed to break re-entry'); + // Re-fire the SAME flow for the SAME record (the loop shape). + const r = await engine.execute('looping_flow', { + record: { id: 'case-1', is_escalated: 1 }, // int 1, like SQLite + object: 'crm_case', + event: 'record-after-update', + } as any); + if ((r.output as any)?.reason === 'reentrancy_loop_guard') skippedInner = true; + return { success: true }; + }, + }); + + engine.registerFlow('looping_flow', { + name: 'looping_flow', + label: 'Looping', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'act', type: 'self_retrigger', label: 'Re-fire' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'act' }, + { id: 'e2', source: 'act', target: 'end' }, + ], + }); + + const result = await engine.execute('looping_flow', { + record: { id: 'case-1', is_escalated: 1 }, + object: 'crm_case', + event: 'record-after-update', + } as any); + + // The OUTER run completes; the INNER re-entry is broken by the guard + // (not an infinite loop). executeCalls stays at 1 (the guard short-circuits + // before the inner run reaches the node again). + expect(result.success).toBe(true); + expect(skippedInner).toBe(true); + expect(executeCalls).toBe(1); + }); + + it('does NOT block a different record (legitimate cross-record fan-out)', async () => { + const seen: string[] = []; + engine.registerNodeExecutor({ + type: 'touch', + async execute() { return { success: true }; }, + }); + engine.registerFlow('per_record', { + name: 'per_record', label: 'Per record', type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 't', type: 'touch', label: 'Touch' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 't' }, + { id: 'e2', source: 't', target: 'end' }, + ], + }); + for (const id of ['a', 'b', 'c']) { + const r = await engine.execute('per_record', { record: { id }, object: 'o', event: 'record-after-insert' } as any); + if (r.success && !(r.output as any)?.reason) seen.push(id); + } + // All three distinct records run fully — the guard only trips on re-entry + // for the SAME record while active. + expect(seen).toEqual(['a', 'b', 'c']); + }); + + it('does NOT block a different flow on the same record (distinct-flow chain)', async () => { + engine.registerNodeExecutor({ type: 'noop', async execute() { return { success: true }; } }); + for (const name of ['flow_x', 'flow_y']) { + engine.registerFlow(name, { + name, label: name, type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'n', type: 'noop', label: 'noop' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'n' }, + { id: 'e2', source: 'n', target: 'end' }, + ], + }); + } + const rx = await engine.execute('flow_x', { record: { id: 'rec-1' }, object: 'o', event: 'record-after-update' } as any); + const ry = await engine.execute('flow_y', { record: { id: 'rec-1' }, object: 'o', event: 'record-after-update' } as any); + expect(rx.success).toBe(true); + expect((rx.output as any)?.reason).toBeUndefined(); + expect(ry.success).toBe(true); + expect((ry.output as any)?.reason).toBeUndefined(); + }); +}); diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index a51ac72f83..a4082d7e8a 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -426,6 +426,26 @@ export class AutomationEngine implements IAutomationService { private flows = new Map(); private flowEnabled = new Map(); + /** + * Re-entrancy guard for record-triggered flows (complements the intra-run + * {@link MAX_NODE_REENTRIES} back-edge guard, which cannot see a self-trigger + * loop because each re-fire is a NEW run with a new id). + * + * A `record-after-update` flow whose action writes back to its OWN trigger + * record re-fires itself (update → afterUpdate → dispatch → execute → update + * → …). Normally the flow's start `condition` suppresses the second fire, but + * a broken guard makes it INFINITE — e.g. HotCRM's `case_escalation` guards on + * `record.is_escalated != true`, but a `boolean` field persists as integer `1` + * on SQLite/libsql and CEL `1 != true` is `true`, so it never trips. During + * first-boot seed (which awaits automation to settle) that infinite cascade + * wedges the whole per-env kernel build → the env is unopenable (2026-07-06). + * + * Keyed by `flowName::recordId`: the SAME flow re-entering for the SAME record + * while an execution is still on the stack is broken. Different flows or + * different records are unaffected, so legitimate cross-record fan-out and + * distinct-flow chains still run. + */ + private readonly activeRecordFlows = new Set(); /** Flows the persisted deployment `status` currently marks disabled * (`obsolete`/`invalid`), tracked so a status flip back to active/draft * re-enables on the next (re)register even if the flow had been turned off. */ @@ -1098,6 +1118,21 @@ export class AutomationEngine implements IAutomationService { return { success: false, error: `Flow '${flowName}' is disabled` }; } + // Re-entrancy loop guard (see `activeRecordFlows`). Break the SAME flow + // re-firing for the SAME record while a prior execution is still active — + // a self-trigger cascade whose start condition fails to suppress it would + // otherwise loop forever and wedge the caller (fatally, mid seed). + const guardRecordId = (context?.record as { id?: unknown } | undefined)?.id; + const reentryKey = guardRecordId != null ? `${flowName}::${String(guardRecordId)}` : undefined; + if (reentryKey && this.activeRecordFlows.has(reentryKey)) { + this.logger.warn( + `[automation] flow '${flowName}' re-entered for the same record '${String(guardRecordId)}' while still running — breaking self-trigger loop. ` + + `Its start condition did not suppress the re-fire; if it guards on a boolean field (e.g. \`is_escalated != true\`), note booleans persist as 0/1 on SQLite/libsql and CEL \`1 != true\` is true.`, + ); + return { success: true, output: { skipped: true, reason: 'reentrancy_loop_guard' } }; + } + if (reentryKey) this.activeRecordFlows.add(reentryKey); + // Initialize variable context const variables = new Map(); if (flow.variables) { @@ -1281,6 +1316,11 @@ export class AutomationEngine implements IAutomationService { error: errorMessage, durationMs, }; + } finally { + // Release the re-entrancy guard for this (flow, record). Runs before + // the returned promise settles, so an error-retry re-run (whose inner + // execute happens after its own await) is not falsely blocked. + if (reentryKey) this.activeRecordFlows.delete(reentryKey); } } diff --git a/packages/spec/src/data/hook.zod.ts b/packages/spec/src/data/hook.zod.ts index c38922b0a5..0f848ef36c 100644 --- a/packages/spec/src/data/hook.zod.ts +++ b/packages/spec/src/data/hook.zod.ts @@ -206,6 +206,7 @@ export const HookContextSchema = lazySchema(() => z.object({ roles: z.array(z.string()).optional(), accessToken: z.string().optional(), isSystem: z.boolean().optional().describe('True when the call was made with an elevated system context (engine self-writes)'), + skipTriggers: z.boolean().optional().describe('True when record-change automation (flow triggers) must be suppressed for this write — e.g. package seed replay. Lifecycle hooks still run.'), }).optional().describe('Current session context'), /** diff --git a/packages/spec/src/kernel/execution-context.zod.ts b/packages/spec/src/kernel/execution-context.zod.ts index cb2054ffed..bf25032cc4 100644 --- a/packages/spec/src/kernel/execution-context.zod.ts +++ b/packages/spec/src/kernel/execution-context.zod.ts @@ -120,7 +120,21 @@ export const ExecutionContextSchema = lazySchema(() => z.object({ /** Whether this is a system-level operation (bypasses permission checks) */ isSystem: z.boolean().default(false), - + + /** + * Suppress record-change AUTOMATION (autolaunched flow triggers: + * record-after-insert / -update / -delete) for writes made under this + * context. Lifecycle HOOKS still run (derived/default fields, validation). + * + * Set by bulk/reference loads — notably package metadata SEED replay — where + * the records are pre-existing END-STATE data, not user events: firing + * "on create/update" automation (notifications, escalations, assignments, + * approvals) for seed rows is semantically wrong AND dangerous. The + * 2026-07-06 incident was a seeded critical case whose escalation flow + * self-triggered into an infinite loop that wedged the whole kernel build. + */ + skipTriggers: z.boolean().optional(), + /** Raw access token (for external API call pass-through) */ accessToken: z.string().optional(), diff --git a/packages/triggers/trigger-record-change/src/record-change-trigger.test.ts b/packages/triggers/trigger-record-change/src/record-change-trigger.test.ts index 2322691384..3c9907436e 100644 --- a/packages/triggers/trigger-record-change/src/record-change-trigger.test.ts +++ b/packages/triggers/trigger-record-change/src/record-change-trigger.test.ts @@ -291,3 +291,29 @@ describe('RecordChangeTriggerPlugin', () => { expect(registerTrigger).toHaveBeenCalledTimes(1); }); }); + +// ─── seed / bulk suppression (context.skipTriggers) ───────────────── + +describe('RecordChangeTrigger — skipTriggers suppression', () => { + it('does NOT dispatch the flow when the write session sets skipTriggers (seed replay)', async () => { + const { engine, hooks } = fakeEngine(); + const trigger = new RecordChangeTrigger(engine, silentLogger()); + let fired = 0; + trigger.start(binding(), async () => { fired += 1; }); + + // A seed/bulk write carries session.skipTriggers=true (from + // ExecutionContext.skipTriggers threaded via buildSession). + await hooks[0].handler(hookCtx({ session: { userId: 'u9', skipTriggers: true } as any })); + expect(fired).toBe(0); + }); + + it('DOES dispatch the flow for a normal user write (skipTriggers absent)', async () => { + const { engine, hooks } = fakeEngine(); + const trigger = new RecordChangeTrigger(engine, silentLogger()); + let fired = 0; + trigger.start(binding(), async () => { fired += 1; }); + + await hooks[0].handler(hookCtx()); + expect(fired).toBe(1); + }); +}); diff --git a/packages/triggers/trigger-record-change/src/record-change-trigger.ts b/packages/triggers/trigger-record-change/src/record-change-trigger.ts index 7b337abead..0645399b65 100644 --- a/packages/triggers/trigger-record-change/src/record-change-trigger.ts +++ b/packages/triggers/trigger-record-change/src/record-change-trigger.ts @@ -113,6 +113,16 @@ export class RecordChangeTrigger implements FlowTrigger { const handler = async (ctx: HookContext): Promise => { try { + // Seed/bulk suppression: writes made with `context.skipTriggers` + // (notably package metadata SEED replay) must NOT fire + // record-change automation — seed rows are pre-existing end-state + // data, not user events. Firing "on create/update" flows for them + // is semantically wrong and was the vector for the 2026-07-06 + // self-trigger loop that wedged first-boot. Lifecycle hooks still + // ran (they are separate); only the flow dispatch is skipped here. + if ((ctx.session as { skipTriggers?: boolean } | undefined)?.skipTriggers) { + return; + } const automationCtx = this.buildContext(binding, ctx); await callback(automationCtx); } catch (err) {