From be7450c78e80798c9b8b0a7b11644604368adceb Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 15:24:42 +0000 Subject: [PATCH 1/6] test(dogfood): pin node config values refused at registration through both doors (red on main) Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude --- ...fig-values-at-registration.dogfood.test.ts | 209 ++++++++++++++++++ 1 file changed, 209 insertions(+) create mode 100644 packages/qa/dogfood/test/flow-node-config-values-at-registration.dogfood.test.ts diff --git a/packages/qa/dogfood/test/flow-node-config-values-at-registration.dogfood.test.ts b/packages/qa/dogfood/test/flow-node-config-values-at-registration.dogfood.test.ts new file mode 100644 index 00000000000..0968da245fb --- /dev/null +++ b/packages/qa/dogfood/test/flow-node-config-values-at-registration.dogfood.test.ts @@ -0,0 +1,209 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// A flow node's config VALUES are judged where the flow registers, by the +// schema its executor parses at run time — through both registration doors +// an operator has: the package a stack ships (package load) and the admin +// write door (`POST /automation`). +// +// ## What was broken +// +// Registration checked only the config's key NAMES (against the node type's +// descriptor `configSchema`); the values were parsed for the first time when a +// run reached the node. An approval node with `escalation.timeoutHours: 0.5` +// (the contract says `>= 1`) therefore registered and loaded `active`, every +// record the trigger matched was created, and every run then failed at the +// approval node: no approval request opened, so the record existed without +// the gate it was meant to pass, and the user who saved it saw nothing. +// +// ## What each case pins +// +// - package load: the sub-hour flow is refused (not registered), and the boot +// says why with a located error (the flow, the node, the config path); +// - package load, the control: a valid escalation registers and RUNS — a +// record of the trigger's object opens a pending approval request; +// - the admin door: the sub-hour flow is refused `400 VALIDATION_FAILED` with +// the located error, and nothing is registered under its name; +// - the admin door, the control: a valid escalation registers. +// +// The fixture is built with `strict: false`, on purpose: the build door judges +// the same flow on its own, and this file pins the two runtime doors behind it, +// so the invalid body has to reach them. Everything else about the stack is an +// ordinary authored package. + +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { ApprovalsServicePlugin } from '@objectstack/plugin-approvals'; +import { RecordChangeTriggerPlugin } from '@objectstack/trigger-record-change'; + +const OBJECT = 'esc_value_request'; +/** A position nothing in this fixture staffs: the request opens and waits. */ +const UNSTAFFED = 'esc_value_unstaffed'; + +const PACKAGED_SUB_HOUR = 'esc_value_packaged_sub_hour'; +const PACKAGED_VALID = 'esc_value_packaged_valid'; +const DOOR_SUB_HOUR = 'esc_value_door_sub_hour'; +const DOOR_VALID = 'esc_value_door_valid'; + +/** An active, record-triggered flow whose one approval node carries `escalation`. */ +function gatedFlow(name: string, escalation: Record) { + return { + name, + label: `Escalation gate ${name}`, + type: 'autolaunched', + status: 'active', + nodes: [ + { + id: 'start', + type: 'start', + label: 'On Create', + config: { objectName: OBJECT, triggerType: 'record-after-create' }, + }, + { + id: 'gate', + type: 'approval', + label: 'Gate', + config: { + approvers: [{ type: 'position', value: UNSTAFFED }], + behavior: 'first_response', + escalation, + }, + }, + { id: 'approved', type: 'end', label: 'Approved' }, + { id: 'rejected', type: 'end', label: 'Rejected' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'gate' }, + { id: 'e2', source: 'gate', target: 'approved', label: 'approve' }, + { id: 'e3', source: 'gate', target: 'rejected', label: 'reject' }, + ], + }; +} + +const SUB_HOUR = { enabled: true, timeoutHours: 0.5, action: 'notify' }; +const VALID = { enabled: true, timeoutHours: 4, action: 'notify' }; + +const fixtureStack = defineStack( + { + manifest: { + id: 'com.dogfood.escalation-values', + namespace: 'esc_value', + version: '0.0.0', + type: 'app', + name: 'Escalation Values Fixture', + description: 'One object and two approval flows, one with an escalation value its contract refuses.', + }, + // ADR-0097: a record-change trigger registers only when the app declares it. + requires: ['automation', 'triggers'], + objects: [ + ObjectSchema.create({ + name: OBJECT, + label: 'Escalation Value Request', + pluralLabel: 'Escalation Value Requests', + sharingModel: 'public_read_write', + fields: { name: Field.text({ label: 'Name', required: true }) }, + }), + ], + flows: [gatedFlow(PACKAGED_SUB_HOUR, SUB_HOUR), gatedFlow(PACKAGED_VALID, VALID)] as never, + }, + { strict: false }, +); + +interface DispatcherEnvelope { + success?: boolean; + data?: Record; + error?: { code?: string; message?: string }; +} + +describe('a flow node config value its executor refuses is refused at registration', () => { + let stack: VerifyStack; + let token: string; + /** Everything the platform logger wrote while the stack booted. */ + let bootOutput = ''; + + const call = async (method: string, path: string, body?: unknown) => { + const res = await stack.apiAs(token, method, path, body); + const json = (await res.json().catch(() => ({}))) as DispatcherEnvelope; + return { status: res.status, json }; + }; + + beforeAll(async () => { + // The core logger writes through the process streams, not `console.*`. + const lines: string[] = []; + const sink = (chunk: unknown): boolean => { + lines.push(String(chunk)); + return true; + }; + const outSpy = vi.spyOn(process.stdout, 'write').mockImplementation(sink as never); + const errSpy = vi.spyOn(process.stderr, 'write').mockImplementation(sink as never); + try { + stack = await bootStack(fixtureStack as unknown as Parameters[0], { + automation: true, + extraPlugins: [new RecordChangeTriggerPlugin(), new ApprovalsServicePlugin()], + }); + } finally { + outSpy.mockRestore(); + errSpy.mockRestore(); + bootOutput = lines.join(''); + } + token = await stack.signIn(); + }, 120_000); + + afterAll(async () => { + await stack?.stop(); + }); + + it('package load: the sub-hour flow is not registered', async () => { + const read = await call('GET', `/automation/${PACKAGED_SUB_HOUR}`); + expect(read.status, JSON.stringify(read.json)).toBe(404); + }); + + it('package load: the boot names the flow, the node and the config path it refused', () => { + const refusal = bootOutput + .split('\n') + .filter((line) => line.includes(PACKAGED_SUB_HOUR) && line.includes('escalation.timeoutHours')); + expect(refusal.length, 'no boot line locates the refused value').toBeGreaterThan(0); + expect(refusal.some((line) => line.includes("node 'gate'"))).toBe(true); + }); + + it('package load, the control: a valid escalation registers active and runs', async () => { + const read = await call('GET', `/automation/${PACKAGED_VALID}`); + expect(read.status, JSON.stringify(read.json)).toBe(200); + expect(read.json.data?.status).toBe('active'); + + const created = await stack.apiAs(token, 'POST', `/data/${OBJECT}`, { name: 'gated' }); + expect(created.status, await created.clone().text()).toBe(201); + const createdJson = (await created.json()) as { id?: string; record?: { id?: string } }; + const recordId = String(createdJson.id ?? createdJson.record?.id); + + // The run reached the approval node and the node opened its request: + // only an escalation its executor accepts gets that far. + const pending = await stack.apiAs(token, 'GET', '/approvals/requests?status=pending'); + expect(pending.status).toBe(200); + const rows = ((await pending.json()) as { data: Array> }).data; + const opened = rows.filter((row) => String(row.record_id) === recordId); + expect(opened.length, JSON.stringify(rows)).toBe(1); + expect(opened[0].pending_approvers).toEqual([`position:${UNSTAFFED}`]); + }); + + it('the admin door: the sub-hour flow is refused with a located error, and nothing registers', async () => { + const refused = await call('POST', '/automation', gatedFlow(DOOR_SUB_HOUR, SUB_HOUR)); + expect(refused.status, JSON.stringify(refused.json)).toBe(400); + expect(refused.json.error?.code).toBe('VALIDATION_FAILED'); + const message = String(refused.json.error?.message ?? ''); + expect(message).toContain(DOOR_SUB_HOUR); + expect(message).toContain("node 'gate'"); + expect(message).toContain('escalation.timeoutHours'); + + const read = await call('GET', `/automation/${DOOR_SUB_HOUR}`); + expect(read.status, JSON.stringify(read.json)).toBe(404); + }); + + it('the admin door, the control: a valid escalation registers', async () => { + const created = await call('POST', '/automation', gatedFlow(DOOR_VALID, VALID)); + expect(created.status, JSON.stringify(created.json)).toBe(200); + const read = await call('GET', `/automation/${DOOR_VALID}`); + expect(read.status, JSON.stringify(read.json)).toBe(200); + }); +}); From ecf04889ee52f7866eae710e96454d815aec0e59 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 15:28:21 +0000 Subject: [PATCH 2/6] fix(service-automation): registration parses node config values with the executor-declared contract; the approval node declares its own Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude --- .../src/approval-node-config-contract.test.ts | 96 +++++ .../plugin-approvals/src/approval-node.ts | 14 + .../src/builtin/parse-config.ts | 6 +- .../services/service-automation/src/engine.ts | 103 ++++++ ...node-config-values-at-registration.test.ts | 331 ++++++++++++++++++ .../services/service-automation/src/plugin.ts | 18 + 6 files changed, 566 insertions(+), 2 deletions(-) create mode 100644 packages/plugins/plugin-approvals/src/approval-node-config-contract.test.ts create mode 100644 packages/services/service-automation/src/node-config-values-at-registration.test.ts diff --git a/packages/plugins/plugin-approvals/src/approval-node-config-contract.test.ts b/packages/plugins/plugin-approvals/src/approval-node-config-contract.test.ts new file mode 100644 index 00000000000..e4d84b8ffce --- /dev/null +++ b/packages/plugins/plugin-approvals/src/approval-node-config-contract.test.ts @@ -0,0 +1,96 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The approval node declares the contract its executor parses (#21848), so the + * engine judges an approval node's config VALUES when a flow registers rather + * than at the first run that reaches the node. + * + * Before: the node published only the JSON Schema form of its contract, which + * registration reads for key NAMES; `escalation.timeoutHours: 0.5` (the + * contract says `>= 1`) registered, loaded `active`, and every run then failed + * at the node with `Approval node 'gate' has invalid config: …` and no + * approval request opened. + */ + +import { describe, it, expect } from 'vitest'; +import { AutomationEngine } from '@objectstack/service-automation'; +import { ApprovalNodeConfigSchema } from '@objectstack/spec/automation'; +import { ApprovalService } from './approval-service.js'; +import { registerApprovalNode, type ApprovalAutomationSurface } from './approval-node.js'; + +const noopLogger = { info() {}, warn() {}, error() {}, debug() {} }; + +type RegisteredExecutor = Parameters[0]; + +/** Capture what `registerApprovalNode` hands the engine, by node type. */ +function captureRegistrations(): { surface: ApprovalAutomationSurface; byType: Map } { + const byType = new Map(); + return { + byType, + surface: { registerNodeExecutor: (executor) => { byType.set(executor.type, executor); } }, + }; +} + +function gatedFlow(name: string, escalation: Record) { + return { + name, + label: name, + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'gate', + type: 'approval', + label: 'Gate', + config: { approvers: [{ type: 'user', value: 'u1' }], escalation }, + }, + { id: 'approved', type: 'end', label: 'Approved' }, + { id: 'rejected', type: 'end', label: 'Rejected' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'gate' }, + { id: 'e2', source: 'gate', target: 'approved', label: 'approve' }, + { id: 'e3', source: 'gate', target: 'rejected', label: 'reject' }, + ], + }; +} + +describe('the approval node declares its config contract', () => { + it('is the very schema its executor parses — not a copy, not the JSON Schema form', async () => { + const { surface, byType } = captureRegistrations(); + registerApprovalNode(surface, new ApprovalService({ engine: {} as any, logger: noopLogger }), noopLogger); + + const approval = byType.get('approval'); + expect(approval?.configContract).toBe(ApprovalNodeConfigSchema); + + // …and `execute` refuses exactly what the contract refuses, before it + // reads anything else (no run id, no record: the parse comes first). + const config = { approvers: [{ type: 'user', value: 'u1' }], escalation: { timeoutHours: 0.5 } }; + expect(ApprovalNodeConfigSchema.safeParse(config).success).toBe(false); + const result = await approval!.execute({ id: 'gate', config }, new Map(), {}); + expect(result.success).toBe(false); + expect(result.error).toContain('escalation.timeoutHours'); + + // The revise window parses no config, so it declares none. + expect(byType.get('approval_revise')?.configContract).toBeUndefined(); + }); + + it('on the engine: `timeoutHours: 0.5` is refused at registration, located; a valid escalation registers', async () => { + const engine = new AutomationEngine(noopLogger as any); + registerApprovalNode(engine, new ApprovalService({ engine: {} as any, logger: noopLogger }), noopLogger); + + let refusal: Error | undefined; + try { + engine.registerFlow('sub_hour', gatedFlow('sub_hour', { enabled: true, timeoutHours: 0.5, action: 'notify' }) as never); + } catch (err) { + refusal = err as Error; + } + expect(refusal, 'registered a flow whose approval escalation the contract refuses').toBeDefined(); + expect(refusal!.message).toContain("Flow 'sub_hour' rejected"); + expect(refusal!.message).toContain("node 'gate' (approval): config.escalation.timeoutHours: "); + expect(await engine.getFlow('sub_hour')).toBeNull(); + + engine.registerFlow('valid', gatedFlow('valid', { enabled: true, timeoutHours: 1, action: 'notify' }) as never); + expect(await engine.getFlow('valid')).not.toBeNull(); + }); +}); diff --git a/packages/plugins/plugin-approvals/src/approval-node.ts b/packages/plugins/plugin-approvals/src/approval-node.ts index e2027bb0c67..89c6ef4c4c7 100644 --- a/packages/plugins/plugin-approvals/src/approval-node.ts +++ b/packages/plugins/plugin-approvals/src/approval-node.ts @@ -36,6 +36,13 @@ export interface ApprovalAutomationSurface { registerNodeExecutor(executor: { type: string; descriptor?: unknown; + /** + * #21848: the contract `execute` parses `node.config` against — the engine + * parses every node of this type with it when a flow registers, so a value + * the run would refuse refuses the flow instead. Mirrors + * `NodeExecutor.configContract`. + */ + configContract?: { safeParse(value: unknown): unknown }; execute(node: any, variables: Map, context: any): Promise<{ success: boolean; output?: Record; @@ -129,6 +136,13 @@ export function registerApprovalNode( // rather than a hardcoded client form — the engine owns the shape. configSchema: getApprovalNodeConfigJsonSchema(), }), + // #21848: the SAME schema `execute` parses below, declared so the engine + // parses every approval node's config with it when a flow registers. The + // JSON Schema above is the form's projection and settles key names only; + // a value this refuses (`escalation.timeoutHours: 0.5` under its `>= 1`) + // used to register and load `active`, then fail every run at this node + // with no approval request opened. + configContract: ApprovalNodeConfigSchema, async execute(node, variables, context) { const parsed = ApprovalNodeConfigSchema.safeParse(node.config ?? {}); if (!parsed.success) { diff --git a/packages/services/service-automation/src/builtin/parse-config.ts b/packages/services/service-automation/src/builtin/parse-config.ts index c2472a2abaa..b6e63d3b20d 100644 --- a/packages/services/service-automation/src/builtin/parse-config.ts +++ b/packages/services/service-automation/src/builtin/parse-config.ts @@ -74,8 +74,10 @@ export type ParsedNodeConfig = | { ok: false; refusal: { success: false; error: string; errorClass: 'guard' } }; /** `config.fields[0].name` — the same path spelling the registration-time - * undeclared-key diagnostic uses, so both layers report locations alike. */ -function formatIssuePath(path: ReadonlyArray): string { + * undeclared-key diagnostic uses, so both layers report locations alike. + * [#21848] Also the spelling of registration's config-VALUE refusal + * (`AutomationEngine.validateNodeConfigValues`), which reads it from here. */ +export function formatIssuePath(path: ReadonlyArray): string { let out = 'config'; for (const seg of path) { out += typeof seg === 'number' ? `[${seg}]` : `.${String(seg)}`; diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 597e506a771..b5e94fb23b6 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -229,6 +229,12 @@ import { describeThrownForLog } from './thrown-cause-diagnostics.js'; // `../guard-refusal.js` and package-external contracts, so nothing it pulls in // reaches back here. import { interpolateText } from './builtin/template.js'; +// [#21848] A node's config contract (`NodeExecutor.configContract`) is the +// structural `safeParse` view the builtin executors already parse through, and +// the registration-time refusal spells its paths the way that execute-time +// refusal does. Safe in this direction for the same reason as the line above: +// `parse-config.ts` imports only `../guard-refusal.js`. +import { formatIssuePath, type NodeConfigContract } from './builtin/parse-config.js'; /** * Does this `decision` take EVERY out-edge whose condition holds (#15429)? @@ -261,6 +267,27 @@ export interface NodeExecutor { */ readonly descriptor?: ActionDescriptor; + /** + * The contract this executor parses `node.config` against before it does + * anything else, when it parses one — declared here so registration judges + * the config by the SAME schema the run would (#21848). Pass the very + * object `execute()` parses, never a copy or a second, hand-written check. + * + * {@link AutomationEngine.registerFlow} parses every node of this type + * with it and refuses the flow on any finding, naming the flow, the node + * and the config path: an executor that fails the node on a finding would + * fail every run that reached the node, and the config is metadata, so no + * run can get past it until the flow changes. Absent ⇒ the node's config is + * judged at registration on its key NAMES alone (the descriptor's + * `configSchema`), exactly as before this member existed. + * + * Declare it only where the executor parses its stored config as written. + * One that reads the config only after transforming it (interpolating + * `{token}`s into typed slots first, say) would see a different value than + * registration does, and must not declare it. + */ + readonly configContract?: NodeConfigContract; + /** * Execute a node * @param node - Current node definition @@ -4385,6 +4412,14 @@ export class AutomationEngine implements IAutomationService { // types, keyValue maps). this.validateNodeConfigKeys(name, parsed); + // #21848 — and the VALUES: every node whose executor declares the + // contract it parses its config against is parsed with that contract + // here, so a value the run would refuse (an approval escalation's + // `timeoutHours: 0.5` under its `>= 1`) refuses the flow instead of + // failing every run that reaches the node. After the key check, so an + // undeclared key keeps its own refusal and prescriptions. + this.validateNodeConfigValues(name, parsed); + // #15429 — parse every `decision` node's config against the spec's // `DecisionConfigSchema` and refuse the flow on an invalid `mode`, with // the schema's own sentence — the same answer `os validate` gives. @@ -10320,6 +10355,74 @@ export class AutomationEngine implements IAutomationService { } } + /** + * REJECT a node `config` VALUE its own executor would refuse (#21848) — + * the value half of {@link validateNodeConfigKeys}, which judges names. + * + * Before this pass registration read a node's config against the + * descriptor's JSON-Schema `configSchema` for its key NAMES only, and the + * values were first parsed when a run reached the node. So a value the + * contract refuses registered and loaded `active`: an approval node whose + * `escalation.timeoutHours` was `0.5` (the contract says `>= 1`) let every + * matching record be created, then failed every run at the node — no + * approval request opened, the record stood without the gate it was meant + * to pass, and the user who saved it saw nothing. + * + * **The executor's own contract is the one judge.** A node type whose + * executor declares {@link NodeExecutor.configContract} — the very object + * its `execute()` parses — has every node's `config ?? {}` parsed with it + * here, as the run would parse it, and every finding refuses the flow. + * ⛔ No second, hand-written value check, and no judging against the + * descriptor's JSON Schema, which is the form's projection of the contract + * and drops what JSON Schema cannot say (a `superRefine` rule). + * + * **Absent ⇒ unchanged.** A node type whose executor declares no contract + * — the builtins, which parse theirs inside `execute()` (#20316 judges + * only their required keys' presence at the flow parse), the schemaless + * ones, and any type whose executor is not registered yet — is judged on + * key names alone, exactly as before. "Not registered yet" is a boot fact: + * a plugin registers its executor from its own `start()`, after the boot + * flow pull, so the boot pull cannot judge its nodes; the `kernel:ready` + * cold-boot bind re-registers every flow once those executors exist, and + * a flow it refuses is withdrawn there (`AutomationServicePlugin`), the + * same end as a flow the boot pull refuses. + * + * The refusal mirrors the key-name one — the flow, then one line per + * finding naming the node, its type and the config path in the spelling + * the execute-time refusal uses (`config.escalation.timeoutHours`), with the + * contract's own sentence — and, like it, every `registerFlow` caller + * already handles the throw: the boot pull and the syncs skip the flow + * loudly, the `/automation` write doors answer `400 VALIDATION_FAILED`. + */ + private validateNodeConfigValues(flowName: string, flow: FlowParsed): void { + const violations: string[] = []; + for (const graph of collectFlowGraphs(flow)) { + for (const node of graph.nodes) { + const contract = this.nodeExecutors.get(node.type)?.configContract; + if (!contract) continue; + const verdict = contract.safeParse(node.config ?? {}); + if (verdict.success) continue; + const issues = verdict.error?.issues ?? []; + const found = issues.length > 0 + ? issues.map((issue) => `${formatIssuePath(issue.path)}: ${issue.message}`) + : ['config: refused by the contract, which named no issue']; + for (const finding of found) { + const line = `node '${node.id}' (${node.type}): ${finding}`; + violations.push(graph.scope ? `${graph.scope} · ${line}` : line); + } + } + } + if (violations.length > 0) { + throw new Error( + `Flow '${flowName}' rejected: ${violations.length} config value(s) the node's own contract refuses.\n` + + violations.map((v) => ` - ${v}`).join('\n') + + `\nThe node's executor parses its config against this same contract before it does anything ` + + `else and fails the node on any finding, so every run that reached the node would fail there. ` + + `The config is metadata, so re-registering changes nothing — correct the value at the path above.`, + ); + } + } + /** * [#15429] The registration-time reader of a `decision` node's `mode`. * diff --git a/packages/services/service-automation/src/node-config-values-at-registration.test.ts b/packages/services/service-automation/src/node-config-values-at-registration.test.ts new file mode 100644 index 00000000000..887af550ee6 --- /dev/null +++ b/packages/services/service-automation/src/node-config-values-at-registration.test.ts @@ -0,0 +1,331 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * A node's config VALUES are judged at registration by the contract its + * executor parses at run time (#21848) — the value half of the key-name check + * (`config-unknown-keys.test.ts`). + * + * Before: registration read a node's config against the descriptor's JSON + * Schema for key NAMES only, so an approval node with + * `escalation.timeoutHours: 0.5` (the contract says `>= 1`) registered, loaded + * `active`, and failed every run at the node with no approval request opened. + * + * Pinned here, on the engine and on the plugin's boot path: + * - a value the declared contract refuses refuses the flow, naming the flow, + * the node and the config path, and nothing is registered; + * - the refusal is the same CLASS as the key-name refusal (a plain error with + * no `code` / `status` of its own), so every door answers it the same way; + * - the judge is the executor's own contract, rules JSON Schema cannot carry + * included, and nodes inside an ADR-0031 region are judged too; + * - a node type whose executor declares no contract keeps today's behaviour; + * - on boot, a flow the boot pull registered before its node's executor + * existed, and the `kernel:ready` bind then refused, is withdrawn — it does + * not stay registered and bound behind the refusal's warning. + */ + +import { describe, it, expect } from 'vitest'; +import { LiteKernel } from '@objectstack/core'; +import type { Plugin, PluginContext } from '@objectstack/core'; +import { + APPROVAL_NODE_TYPE, + ApprovalNodeConfigSchema, + defineActionDescriptor, + getApprovalNodeConfigJsonSchema, +} from '@objectstack/spec/automation'; +import type { AutomationContext } from '@objectstack/spec/contracts'; +import { AutomationEngine } from './engine.js'; +import type { FlowTrigger, FlowTriggerBinding, NodeExecutor } from './engine.js'; +import { AutomationServicePlugin } from './plugin.js'; + +const flush = () => new Promise((r) => setTimeout(r, 0)); + +const SUB_HOUR = { enabled: true, timeoutHours: 0.5, action: 'notify' }; +const VALID = { enabled: true, timeoutHours: 4, action: 'notify' }; + +function silentLogger() { + const warnings: string[] = []; + const logger: any = { + info() {}, error() {}, debug() {}, + warn(msg: string, meta?: unknown) { warnings.push(`${msg} ${meta ? JSON.stringify(meta) : ''}`); }, + child() { return logger; }, + }; + return { logger, warnings }; +} + +/** + * An `approval` executor registered the way `plugin-approvals` registers its + * own: the descriptor publishes the JSON Schema (key names), and + * `configContract` is the schema `execute` parses. `withContract: false` is the + * shape every executor had before the member existed. + */ +function approvalExecutor(withContract = true): NodeExecutor { + return { + type: APPROVAL_NODE_TYPE, + descriptor: defineActionDescriptor({ + type: APPROVAL_NODE_TYPE, + version: '1.0.0', + name: 'Approval', + category: 'human', + paradigms: ['flow'], + source: 'plugin', + supportsPause: true, + resumeAuthority: 'service', + configSchema: getApprovalNodeConfigJsonSchema() as Record, + }), + ...(withContract ? { configContract: ApprovalNodeConfigSchema } : {}), + async execute() { + return { success: true, suspend: true }; + }, + }; +} + +/** An active, record-triggered flow whose approval node carries `escalation`. */ +function gatedFlow(name: string, escalation: Record, extraConfig: Record = {}) { + return { + name, + label: name, + type: 'autolaunched', + status: 'active', + nodes: [ + { id: 'start', type: 'start', label: 'Start', config: { objectName: 'expense', triggerType: 'record-after-create' } }, + { + id: 'gate', + type: APPROVAL_NODE_TYPE, + label: 'Gate', + config: { approvers: [{ type: 'position', value: 'manager' }], escalation, ...extraConfig }, + }, + { id: 'approved', type: 'end', label: 'Approved' }, + { id: 'rejected', type: 'end', label: 'Rejected' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'gate' }, + { id: 'e2', source: 'gate', target: 'approved', label: 'approve' }, + { id: 'e3', source: 'gate', target: 'rejected', label: 'reject' }, + ], + }; +} + +/** Register and return the thrown error (fails the test if it registers). */ +function refusalOf(engine: AutomationEngine, name: string, definition: unknown): Error & { code?: unknown; status?: unknown } { + try { + engine.registerFlow(name, definition as never); + } catch (err) { + return err as Error & { code?: unknown; status?: unknown }; + } + throw new Error(`expected '${name}' to be refused at registration, and it registered`); +} + +describe('registration judges node config VALUES with the executor-declared contract', () => { + it('refuses `escalation.timeoutHours: 0.5`, locating the flow, the node and the config path', async () => { + const engine = new AutomationEngine(silentLogger().logger); + engine.registerNodeExecutor(approvalExecutor()); + + const err = refusalOf(engine, 'sub_hour', gatedFlow('sub_hour', SUB_HOUR)); + expect(err.message).toContain("Flow 'sub_hour' rejected: 1 config value(s) the node's own contract refuses."); + expect(err.message).toContain("node 'gate' (approval): config.escalation.timeoutHours: "); + // The contract's own sentence, not a second one. + const contractSentence = ApprovalNodeConfigSchema.safeParse(gatedFlow('x', SUB_HOUR).nodes[1].config) + .error?.issues[0]?.message; + expect(contractSentence).toBeTruthy(); + expect(err.message).toContain(`config.escalation.timeoutHours: ${contractSentence}`); + // Nothing registered under the name. + expect(await engine.getFlow('sub_hour')).toBeNull(); + }); + + it('is the key-name refusal\'s class: a plain error with no code or status of its own', () => { + const engine = new AutomationEngine(silentLogger().logger); + engine.registerNodeExecutor(approvalExecutor()); + + const value = refusalOf(engine, 'sub_hour', gatedFlow('sub_hour', SUB_HOUR)); + const key = refusalOf(engine, 'bogus_key', gatedFlow('bogus_key', { ...VALID, bogusKey: 1 })); + // The key-name refusal still owns an undeclared key (it runs first). + expect(key.message).toContain('undeclared config key(s)'); + expect(key.message).toContain('at config.escalation.bogusKey'); + // Both reach the doors as the same class, so the `/automation` write + // doors answer both `400 VALIDATION_FAILED` and the boot skips both. + for (const err of [value, key]) { + expect(err).toBeInstanceOf(Error); + expect(err.code).toBeUndefined(); + expect(err.status).toBeUndefined(); + } + expect(Object.getPrototypeOf(value)).toBe(Object.getPrototypeOf(key)); + }); + + it('registers a valid escalation, and stores it unchanged', async () => { + const engine = new AutomationEngine(silentLogger().logger); + engine.registerNodeExecutor(approvalExecutor()); + + engine.registerFlow('valid', gatedFlow('valid', VALID) as never); + const stored = await engine.getFlow('valid'); + expect(stored?.nodes.find((n) => n.id === 'gate')?.config).toMatchObject({ escalation: VALID }); + }); + + it('judges with the whole contract: a rule JSON Schema cannot carry refuses too', () => { + const engine = new AutomationEngine(silentLogger().logger); + engine.registerNodeExecutor(approvalExecutor()); + + // `fallbackApprovers` is read only under `onEmptyApprovers: 'fallback'` + // — a `superRefine` rule of the contract, absent from its JSON Schema. + const definition = gatedFlow('ignored_fallback', VALID, { + onEmptyApprovers: 'fail', + fallbackApprovers: [{ type: 'user', value: 'u1' }], + }); + expect(ApprovalNodeConfigSchema.safeParse(definition.nodes[1].config).success).toBe(false); + const err = refusalOf(engine, 'ignored_fallback', definition); + expect(err.message).toContain("node 'gate' (approval): config.fallbackApprovers: "); + }); + + it('a node type whose executor declares no contract keeps key-name-only judgement', async () => { + const engine = new AutomationEngine(silentLogger().logger); + engine.registerNodeExecutor(approvalExecutor(false)); + + engine.registerFlow('unjudged', gatedFlow('unjudged', SUB_HOUR) as never); + expect(await engine.getFlow('unjudged')).not.toBeNull(); + }); + + it('judges a node inside an ADR-0031 region, naming the region', async () => { + const engine = new AutomationEngine(silentLogger().logger); + // A test node type whose contract refuses `count` below 1 — structural, + // like every contract the engine reads. + engine.registerNodeExecutor({ + type: 'stamp', + configContract: { + safeParse(value: unknown) { + const count = (value as { count?: unknown }).count; + return typeof count === 'number' && count >= 1 + ? { success: true, data: value } + : { success: false, error: { issues: [{ path: ['count'], message: 'must be at least 1' }] } }; + }, + }, + async execute() { + return { success: true }; + }, + }); + const flow = (count: number) => ({ + name: 'looped', + label: 'Looped', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'each', + type: 'loop', + label: 'Each', + config: { + collection: '{items}', + body: { + nodes: [{ id: 'inner', type: 'stamp', label: 'Inner', config: { count } }], + edges: [], + }, + }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'each' }, + { id: 'e2', source: 'each', target: 'end' }, + ], + }); + + const err = refusalOf(engine, 'looped', flow(0)); + expect(err.message).toMatch(/· node 'inner' \(stamp\): config\.count: must be at least 1/); + expect(err.message).toContain('each'); + + engine.registerFlow('looped', flow(2) as never); + expect(await engine.getFlow('looped')).not.toBeNull(); + }); +}); + +// ── The boot path: the executor registers AFTER the boot pull ────────────── + +/** A recording `record_change` trigger (stands in for the real one). */ +function recordingRecordChangeTrigger() { + const bound = new Set(); + const trigger: FlowTrigger = { + type: 'record_change', + start(binding: FlowTriggerBinding, _cb: (ctx: AutomationContext) => Promise) { + bound.add(binding.flowName); + }, + stop(flowName: string) { + bound.delete(flowName); + }, + }; + return { trigger, has: (n: string) => bound.has(n) }; +} + +/** + * The two reads a boot binds flows from: the `objectql` registry (the boot + * pull in `AutomationServicePlugin.start()`) and the protocol's flow view (the + * `kernel:ready` bind) — both serving the same packaged flows, as a real boot + * does. + */ +function flowSourcesPlugin(flows: unknown[], rec: ReturnType): Plugin { + return { + name: 'test.flow-sources', + version: '1.0.0', + async init(ctx: PluginContext) { + const c = ctx as unknown as { + registerService(n: string, s: unknown): void; + getService(n: string): T; + }; + c.registerService('objectql', { + registry: { + listItems: (type: string) => (type === 'flow' ? flows : []), + getObject: () => undefined, + }, + }); + c.registerService('protocol', { + async getMetaItemsForExecution(q: { type: string }) { + return { items: q.type === 'flow' ? flows : [] }; + }, + }); + c.getService('automation').registerTrigger(rec.trigger); + }, + }; +} + +/** + * A plugin that contributes the `approval` executor from its own `start()` — + * after `AutomationServicePlugin.start()` pulled and registered the flows, + * which is where `ApprovalsServicePlugin` registers it. + */ +function lateApprovalPlugin(): Plugin { + return { + name: 'test.late-approval', + version: '1.0.0', + async init() {}, + async start(ctx: PluginContext) { + (ctx as unknown as { getService(n: string): T }) + .getService('automation') + .registerNodeExecutor(approvalExecutor()); + }, + }; +} + +describe('boot: a flow the kernel:ready bind refuses does not stay registered from the boot pull', () => { + it('withdraws the refused flow and keeps the valid one bound', async () => { + const rec = recordingRecordChangeTrigger(); + const kernel = new LiteKernel({ logger: { level: 'silent' } } as never); + kernel.use(new AutomationServicePlugin()); + kernel.use( + flowSourcesPlugin( + [{ ...gatedFlow('sub_hour', SUB_HOUR), _packageId: 'app.fixture' }, { ...gatedFlow('valid', VALID), _packageId: 'app.fixture' }], + rec, + ), + ); + kernel.use(lateApprovalPlugin()); + await kernel.bootstrap(); + await flush(); + + const engine = kernel.getService('automation'); + expect(await engine.getFlow('sub_hour'), 'refused at load ⇒ not registered').toBeNull(); + expect(rec.has('sub_hour'), 'refused at load ⇒ not bound').toBe(false); + expect(engine.getActiveTriggerBindings().map((b) => b.flowName)).not.toContain('sub_hour'); + + // Only the refused flow: the package's valid flow is registered and bound. + expect(await engine.getFlow('valid')).not.toBeNull(); + expect(rec.has('valid')).toBe(true); + + await kernel.shutdown(); + }); +}); diff --git a/packages/services/service-automation/src/plugin.ts b/packages/services/service-automation/src/plugin.ts index d69c5a8bcc7..fb06fd8ad14 100644 --- a/packages/services/service-automation/src/plugin.ts +++ b/packages/services/service-automation/src/plugin.ts @@ -2482,6 +2482,18 @@ export class AutomationServicePlugin implements Plugin { * flows down, so a transient empty/failed read at boot can't unbind the flows * the boot pull already registered. registerFlow is idempotent, so re-binding * a flow the boot pull already registered is harmless. + * + * [#21848] …except where this bind REFUSES the flow: then the registration + * the boot pull made is withdrawn, so a flow refused at load is not left + * registered, whichever of the two boot steps refused it. The two can + * disagree because the node-type vocabulary grows between them — a plugin + * registers its node executor (and the config contract registration + * judges that node by) from its own `start()`, after the boot pull, so the + * pull registered such a flow unjudged and armed it. Leaving that + * registration in place behind this refusal's warning kept the flow + * `active` and bound: every run then failed at the node the refusal + * located. Only the refused name is withdrawn; a failed or empty READ + * still tears nothing down (the early returns above). */ private async syncFlowsFromProtocol(ctx: PluginContext): Promise { if (!this.engine) return; @@ -2502,6 +2514,12 @@ export class AutomationServicePlugin implements Plugin { flow: entry.name, ...describeThrownForLog(err), }); + // [#21848] The boot pull's registration of this flow does not + // outlive this refusal — see the docblock. + if ((await this.engine.getFlow(entry.name)) !== null) { + this.engine.withdrawFlow(entry.name); + this.syncedFlowNames.delete(entry.name); + } } } if (bound > 0) { From 2ef0c612b2e0561cee7690eff8647223be4372a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 15:35:52 +0000 Subject: [PATCH 3/6] test(service-automation): the superRefine case locates at the rule's own path Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude --- .../src/node-config-values-at-registration.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/services/service-automation/src/node-config-values-at-registration.test.ts b/packages/services/service-automation/src/node-config-values-at-registration.test.ts index 887af550ee6..083eff6102a 100644 --- a/packages/services/service-automation/src/node-config-values-at-registration.test.ts +++ b/packages/services/service-automation/src/node-config-values-at-registration.test.ts @@ -172,7 +172,7 @@ describe('registration judges node config VALUES with the executor-declared cont }); expect(ApprovalNodeConfigSchema.safeParse(definition.nodes[1].config).success).toBe(false); const err = refusalOf(engine, 'ignored_fallback', definition); - expect(err.message).toContain("node 'gate' (approval): config.fallbackApprovers: "); + expect(err.message).toContain("node 'gate' (approval): config.onEmptyApprovers: fallbackApprovers is only read"); }); it('a node type whose executor declares no contract keeps key-name-only judgement', async () => { From 31e5a85c481c1664f4f8cff6201f44993f8b4e21 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 15:48:45 +0000 Subject: [PATCH 4/6] docs(automation): registration also judges the values of an executor-declared config contract; changeset Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude --- ...1848-node-config-values-at-registration.md | 22 +++++++++++++++++++ content/docs/automation/flows.mdx | 10 +++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 .changeset/21848-node-config-values-at-registration.md diff --git a/.changeset/21848-node-config-values-at-registration.md b/.changeset/21848-node-config-values-at-registration.md new file mode 100644 index 00000000000..9f06cf29d4e --- /dev/null +++ b/.changeset/21848-node-config-values-at-registration.md @@ -0,0 +1,22 @@ +--- +'@objectstack/service-automation': minor +'@objectstack/plugin-approvals': minor +--- + +fix(service-automation)!: a flow registers only if every node's config value passes the contract its executor parses, so an approval node's `escalation.timeoutHours: 0.5` is refused at registration and at package load instead of failing every run + +Clause-②: no (narrowing) + + + +**BREAKING**: a flow that registered and loaded before can now be refused. It ships as `minor` under the launch-window convention for accept-set narrowings. + +**What was accepted before.** Registration read a node's `config` against the node type's descriptor `configSchema` for its key NAMES only; the values were parsed for the first time when a run reached the node. An approval node whose `escalation.timeoutHours` was `0.5` (the contract says at least `1`) therefore registered through `POST /automation` and loaded `active` from a package, every record its trigger matched was created, and every run then failed at the approval node with `Approval node 'gate' has invalid config`: no approval request opened, so the record stood without the gate it was meant to pass, and the user who saved it saw nothing. + +**What is refused now.** A node executor may declare the contract it parses its config against (`NodeExecutor.configContract`, an optional member). Registration parses every node of that type with it, inside region bodies too, and refuses the flow on any finding. The approval node declares `ApprovalNodeConfigSchema`, the schema its executor already parses, so every value that schema refuses is refused at registration, rules its JSON Schema cannot carry included (a `fallbackApprovers` list beside an `onEmptyApprovers` policy that never reads it, for one). Through the `/automation` write doors the refusal is `400 VALIDATION_FAILED`, the class the undeclared-key refusal already has. At package load the flow is skipped with a warning and not registered; the rest of the package loads. + +**Package load, both halves.** A plugin registers its node executor from its own `start()`, after the boot pull registers the package's flows, so the boot pull cannot judge those nodes; the `kernel:ready` bind re-registers every flow once the executors exist. A flow that bind refuses is now withdrawn. Before, the boot pull's registration stayed behind the refusal's warning, so the flow stayed `active` and bound to its trigger. That covered an approval node's undeclared config key as well: it was refused with a warning and kept running. It is now withdrawn like any refused flow. + +**What an author sees now.** The refusal names the flow, then one line per finding with the node, its type and the config path, followed by the contract's own sentence: `node 'gate' (approval): config.escalation.timeoutHours: Too small: expected number to be >=1`. The handling is to correct the value at the path it names; the flow then registers as before. + +**Unchanged.** What `execute` accepts. A node type whose executor declares no contract, which today covers every built-in node type, `approval_revise` and any third-party node type, is judged on key names alone as before; the built-in executors still parse their contracts at execute. A flow refused on a re-registration through the write doors keeps the definition the engine already held. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index 892c2aef296..f7eddf33e2d 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -138,7 +138,7 @@ Each node performs a specific action in the flow. | `id` | `string` | ✅ | Unique node identifier — unique across the **whole flow**: the top-level `nodes[]` and every region body (`loop.body`, `parallel.branches[]`, `try_catch.try` / `.catch`, at any depth) share one id space, and `FlowSchema` refuses a reused id at parse (`Duplicate node id …`, naming both locations) | | `type` | `string` | ✅ | Node type — a built-in id from the table above **or** a plugin-registered one. Per ADR-0018 the spec does not gate this with a closed enum; it is checked against the live action registry once that registry is complete — plugins contribute node types while they start, so flows registered during boot are checked in one pass when the vocabulary closes (all plugins started), and anything registered after that (Studio publish, dev reload) is checked immediately. Unknown types warn, never reject; executing one fails with `NO_EXECUTOR` | | `label` | `string` | ✅ | Display label | -| `config` | `object` | optional | Type-specific configuration — the registered executor's `configSchema` owns its shape. Keys that schema does not declare are rejected at `registerFlow()`, and the built-in executors `parse()` the value against their Zod contract before running (#4277) | +| `config` | `object` | optional | Type-specific configuration — the registered executor's `configSchema` owns its shape. Keys that schema does not declare are rejected at `registerFlow()`, values too where the executor declares the contract it parses (the `approval` node does), and the built-in executors `parse()` the value against their Zod contract before running (#4277) | | `connectorConfig` | `object` | ✅ on `connector_action` | `{ connectorId, actionId, input }` — the only input a `connector_action` node's executor reads. `FlowSchema` refuses a `connector_action` node without it, and one whose `connectorId` or `actionId` is blank (empty or whitespace only), at any depth including a region body. `connectorId` is the registered connector's `name`, `actionId` one of the action keys it declares; `input` is optional | | `position` | `{ x, y }` | optional | Visual position on canvas | | `timeoutMs` | `number` | optional | Per-node execution timeout | @@ -155,7 +155,13 @@ node type's published `configSchema` does not declare — naming the path, the declared key set, and a did-you-mean — and the contract-carrying builtins additionally `parse()` their config at execute time, refusing the node on a type or missing-`required` violation (#4277). A node type that publishes no -`configSchema` declares nothing, so nothing can be undeclared. +`configSchema` declares nothing, so nothing can be undeclared. Where a node +type's executor declares the contract it parses (`configContract` — the +`approval` node does), `registerFlow()` also parses every node of that type with +it, and a value the run would refuse (`escalation.timeoutHours: 0.5` under its +`>= 1`) refuses the flow, naming the flow, the node and the config path — at the +`/automation` write doors (`400 VALIDATION_FAILED`) and at boot, where a refused +flow is skipped with a warning and not left registered. ### Node Examples From 1bf4fe00bb97d6f384485e06fdc147fc8938ffeb Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 18:24:02 +0000 Subject: [PATCH 5/6] =?UTF-8?q?refactor(service-automation):=20drop=20the?= =?UTF-8?q?=20registration=20value=20judge=20and=20NodeExecutor.configCont?= =?UTF-8?q?ract=20=E2=80=94=20FlowSchema=20now=20judges=20the=20approval?= =?UTF-8?q?=20contract;=20keep=20the=20cold-boot=20withdraw?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude --- .../src/approval-node-config-contract.test.ts | 96 ----- .../plugin-approvals/src/approval-node.ts | 14 - .../src/builtin/parse-config.ts | 6 +- .../services/service-automation/src/engine.ts | 103 ------ .../flow-cold-boot-refusal-withdraw.test.ts | 172 +++++++++ ...node-config-values-at-registration.test.ts | 331 ------------------ .../services/service-automation/src/plugin.ts | 14 +- 7 files changed, 181 insertions(+), 555 deletions(-) delete mode 100644 packages/plugins/plugin-approvals/src/approval-node-config-contract.test.ts create mode 100644 packages/services/service-automation/src/flow-cold-boot-refusal-withdraw.test.ts delete mode 100644 packages/services/service-automation/src/node-config-values-at-registration.test.ts diff --git a/packages/plugins/plugin-approvals/src/approval-node-config-contract.test.ts b/packages/plugins/plugin-approvals/src/approval-node-config-contract.test.ts deleted file mode 100644 index e4d84b8ffce..00000000000 --- a/packages/plugins/plugin-approvals/src/approval-node-config-contract.test.ts +++ /dev/null @@ -1,96 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * The approval node declares the contract its executor parses (#21848), so the - * engine judges an approval node's config VALUES when a flow registers rather - * than at the first run that reaches the node. - * - * Before: the node published only the JSON Schema form of its contract, which - * registration reads for key NAMES; `escalation.timeoutHours: 0.5` (the - * contract says `>= 1`) registered, loaded `active`, and every run then failed - * at the node with `Approval node 'gate' has invalid config: …` and no - * approval request opened. - */ - -import { describe, it, expect } from 'vitest'; -import { AutomationEngine } from '@objectstack/service-automation'; -import { ApprovalNodeConfigSchema } from '@objectstack/spec/automation'; -import { ApprovalService } from './approval-service.js'; -import { registerApprovalNode, type ApprovalAutomationSurface } from './approval-node.js'; - -const noopLogger = { info() {}, warn() {}, error() {}, debug() {} }; - -type RegisteredExecutor = Parameters[0]; - -/** Capture what `registerApprovalNode` hands the engine, by node type. */ -function captureRegistrations(): { surface: ApprovalAutomationSurface; byType: Map } { - const byType = new Map(); - return { - byType, - surface: { registerNodeExecutor: (executor) => { byType.set(executor.type, executor); } }, - }; -} - -function gatedFlow(name: string, escalation: Record) { - return { - name, - label: name, - type: 'autolaunched', - nodes: [ - { id: 'start', type: 'start', label: 'Start' }, - { - id: 'gate', - type: 'approval', - label: 'Gate', - config: { approvers: [{ type: 'user', value: 'u1' }], escalation }, - }, - { id: 'approved', type: 'end', label: 'Approved' }, - { id: 'rejected', type: 'end', label: 'Rejected' }, - ], - edges: [ - { id: 'e1', source: 'start', target: 'gate' }, - { id: 'e2', source: 'gate', target: 'approved', label: 'approve' }, - { id: 'e3', source: 'gate', target: 'rejected', label: 'reject' }, - ], - }; -} - -describe('the approval node declares its config contract', () => { - it('is the very schema its executor parses — not a copy, not the JSON Schema form', async () => { - const { surface, byType } = captureRegistrations(); - registerApprovalNode(surface, new ApprovalService({ engine: {} as any, logger: noopLogger }), noopLogger); - - const approval = byType.get('approval'); - expect(approval?.configContract).toBe(ApprovalNodeConfigSchema); - - // …and `execute` refuses exactly what the contract refuses, before it - // reads anything else (no run id, no record: the parse comes first). - const config = { approvers: [{ type: 'user', value: 'u1' }], escalation: { timeoutHours: 0.5 } }; - expect(ApprovalNodeConfigSchema.safeParse(config).success).toBe(false); - const result = await approval!.execute({ id: 'gate', config }, new Map(), {}); - expect(result.success).toBe(false); - expect(result.error).toContain('escalation.timeoutHours'); - - // The revise window parses no config, so it declares none. - expect(byType.get('approval_revise')?.configContract).toBeUndefined(); - }); - - it('on the engine: `timeoutHours: 0.5` is refused at registration, located; a valid escalation registers', async () => { - const engine = new AutomationEngine(noopLogger as any); - registerApprovalNode(engine, new ApprovalService({ engine: {} as any, logger: noopLogger }), noopLogger); - - let refusal: Error | undefined; - try { - engine.registerFlow('sub_hour', gatedFlow('sub_hour', { enabled: true, timeoutHours: 0.5, action: 'notify' }) as never); - } catch (err) { - refusal = err as Error; - } - expect(refusal, 'registered a flow whose approval escalation the contract refuses').toBeDefined(); - expect(refusal!.message).toContain("Flow 'sub_hour' rejected"); - expect(refusal!.message).toContain("node 'gate' (approval): config.escalation.timeoutHours: "); - expect(await engine.getFlow('sub_hour')).toBeNull(); - - engine.registerFlow('valid', gatedFlow('valid', { enabled: true, timeoutHours: 1, action: 'notify' }) as never); - expect(await engine.getFlow('valid')).not.toBeNull(); - }); -}); diff --git a/packages/plugins/plugin-approvals/src/approval-node.ts b/packages/plugins/plugin-approvals/src/approval-node.ts index 89c6ef4c4c7..e2027bb0c67 100644 --- a/packages/plugins/plugin-approvals/src/approval-node.ts +++ b/packages/plugins/plugin-approvals/src/approval-node.ts @@ -36,13 +36,6 @@ export interface ApprovalAutomationSurface { registerNodeExecutor(executor: { type: string; descriptor?: unknown; - /** - * #21848: the contract `execute` parses `node.config` against — the engine - * parses every node of this type with it when a flow registers, so a value - * the run would refuse refuses the flow instead. Mirrors - * `NodeExecutor.configContract`. - */ - configContract?: { safeParse(value: unknown): unknown }; execute(node: any, variables: Map, context: any): Promise<{ success: boolean; output?: Record; @@ -136,13 +129,6 @@ export function registerApprovalNode( // rather than a hardcoded client form — the engine owns the shape. configSchema: getApprovalNodeConfigJsonSchema(), }), - // #21848: the SAME schema `execute` parses below, declared so the engine - // parses every approval node's config with it when a flow registers. The - // JSON Schema above is the form's projection and settles key names only; - // a value this refuses (`escalation.timeoutHours: 0.5` under its `>= 1`) - // used to register and load `active`, then fail every run at this node - // with no approval request opened. - configContract: ApprovalNodeConfigSchema, async execute(node, variables, context) { const parsed = ApprovalNodeConfigSchema.safeParse(node.config ?? {}); if (!parsed.success) { diff --git a/packages/services/service-automation/src/builtin/parse-config.ts b/packages/services/service-automation/src/builtin/parse-config.ts index b6e63d3b20d..c2472a2abaa 100644 --- a/packages/services/service-automation/src/builtin/parse-config.ts +++ b/packages/services/service-automation/src/builtin/parse-config.ts @@ -74,10 +74,8 @@ export type ParsedNodeConfig = | { ok: false; refusal: { success: false; error: string; errorClass: 'guard' } }; /** `config.fields[0].name` — the same path spelling the registration-time - * undeclared-key diagnostic uses, so both layers report locations alike. - * [#21848] Also the spelling of registration's config-VALUE refusal - * (`AutomationEngine.validateNodeConfigValues`), which reads it from here. */ -export function formatIssuePath(path: ReadonlyArray): string { + * undeclared-key diagnostic uses, so both layers report locations alike. */ +function formatIssuePath(path: ReadonlyArray): string { let out = 'config'; for (const seg of path) { out += typeof seg === 'number' ? `[${seg}]` : `.${String(seg)}`; diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index b5e94fb23b6..597e506a771 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -229,12 +229,6 @@ import { describeThrownForLog } from './thrown-cause-diagnostics.js'; // `../guard-refusal.js` and package-external contracts, so nothing it pulls in // reaches back here. import { interpolateText } from './builtin/template.js'; -// [#21848] A node's config contract (`NodeExecutor.configContract`) is the -// structural `safeParse` view the builtin executors already parse through, and -// the registration-time refusal spells its paths the way that execute-time -// refusal does. Safe in this direction for the same reason as the line above: -// `parse-config.ts` imports only `../guard-refusal.js`. -import { formatIssuePath, type NodeConfigContract } from './builtin/parse-config.js'; /** * Does this `decision` take EVERY out-edge whose condition holds (#15429)? @@ -267,27 +261,6 @@ export interface NodeExecutor { */ readonly descriptor?: ActionDescriptor; - /** - * The contract this executor parses `node.config` against before it does - * anything else, when it parses one — declared here so registration judges - * the config by the SAME schema the run would (#21848). Pass the very - * object `execute()` parses, never a copy or a second, hand-written check. - * - * {@link AutomationEngine.registerFlow} parses every node of this type - * with it and refuses the flow on any finding, naming the flow, the node - * and the config path: an executor that fails the node on a finding would - * fail every run that reached the node, and the config is metadata, so no - * run can get past it until the flow changes. Absent ⇒ the node's config is - * judged at registration on its key NAMES alone (the descriptor's - * `configSchema`), exactly as before this member existed. - * - * Declare it only where the executor parses its stored config as written. - * One that reads the config only after transforming it (interpolating - * `{token}`s into typed slots first, say) would see a different value than - * registration does, and must not declare it. - */ - readonly configContract?: NodeConfigContract; - /** * Execute a node * @param node - Current node definition @@ -4412,14 +4385,6 @@ export class AutomationEngine implements IAutomationService { // types, keyValue maps). this.validateNodeConfigKeys(name, parsed); - // #21848 — and the VALUES: every node whose executor declares the - // contract it parses its config against is parsed with that contract - // here, so a value the run would refuse (an approval escalation's - // `timeoutHours: 0.5` under its `>= 1`) refuses the flow instead of - // failing every run that reaches the node. After the key check, so an - // undeclared key keeps its own refusal and prescriptions. - this.validateNodeConfigValues(name, parsed); - // #15429 — parse every `decision` node's config against the spec's // `DecisionConfigSchema` and refuse the flow on an invalid `mode`, with // the schema's own sentence — the same answer `os validate` gives. @@ -10355,74 +10320,6 @@ export class AutomationEngine implements IAutomationService { } } - /** - * REJECT a node `config` VALUE its own executor would refuse (#21848) — - * the value half of {@link validateNodeConfigKeys}, which judges names. - * - * Before this pass registration read a node's config against the - * descriptor's JSON-Schema `configSchema` for its key NAMES only, and the - * values were first parsed when a run reached the node. So a value the - * contract refuses registered and loaded `active`: an approval node whose - * `escalation.timeoutHours` was `0.5` (the contract says `>= 1`) let every - * matching record be created, then failed every run at the node — no - * approval request opened, the record stood without the gate it was meant - * to pass, and the user who saved it saw nothing. - * - * **The executor's own contract is the one judge.** A node type whose - * executor declares {@link NodeExecutor.configContract} — the very object - * its `execute()` parses — has every node's `config ?? {}` parsed with it - * here, as the run would parse it, and every finding refuses the flow. - * ⛔ No second, hand-written value check, and no judging against the - * descriptor's JSON Schema, which is the form's projection of the contract - * and drops what JSON Schema cannot say (a `superRefine` rule). - * - * **Absent ⇒ unchanged.** A node type whose executor declares no contract - * — the builtins, which parse theirs inside `execute()` (#20316 judges - * only their required keys' presence at the flow parse), the schemaless - * ones, and any type whose executor is not registered yet — is judged on - * key names alone, exactly as before. "Not registered yet" is a boot fact: - * a plugin registers its executor from its own `start()`, after the boot - * flow pull, so the boot pull cannot judge its nodes; the `kernel:ready` - * cold-boot bind re-registers every flow once those executors exist, and - * a flow it refuses is withdrawn there (`AutomationServicePlugin`), the - * same end as a flow the boot pull refuses. - * - * The refusal mirrors the key-name one — the flow, then one line per - * finding naming the node, its type and the config path in the spelling - * the execute-time refusal uses (`config.escalation.timeoutHours`), with the - * contract's own sentence — and, like it, every `registerFlow` caller - * already handles the throw: the boot pull and the syncs skip the flow - * loudly, the `/automation` write doors answer `400 VALIDATION_FAILED`. - */ - private validateNodeConfigValues(flowName: string, flow: FlowParsed): void { - const violations: string[] = []; - for (const graph of collectFlowGraphs(flow)) { - for (const node of graph.nodes) { - const contract = this.nodeExecutors.get(node.type)?.configContract; - if (!contract) continue; - const verdict = contract.safeParse(node.config ?? {}); - if (verdict.success) continue; - const issues = verdict.error?.issues ?? []; - const found = issues.length > 0 - ? issues.map((issue) => `${formatIssuePath(issue.path)}: ${issue.message}`) - : ['config: refused by the contract, which named no issue']; - for (const finding of found) { - const line = `node '${node.id}' (${node.type}): ${finding}`; - violations.push(graph.scope ? `${graph.scope} · ${line}` : line); - } - } - } - if (violations.length > 0) { - throw new Error( - `Flow '${flowName}' rejected: ${violations.length} config value(s) the node's own contract refuses.\n` + - violations.map((v) => ` - ${v}`).join('\n') + - `\nThe node's executor parses its config against this same contract before it does anything ` + - `else and fails the node on any finding, so every run that reached the node would fail there. ` + - `The config is metadata, so re-registering changes nothing — correct the value at the path above.`, - ); - } - } - /** * [#15429] The registration-time reader of a `decision` node's `mode`. * diff --git a/packages/services/service-automation/src/flow-cold-boot-refusal-withdraw.test.ts b/packages/services/service-automation/src/flow-cold-boot-refusal-withdraw.test.ts new file mode 100644 index 00000000000..dc3afc98276 --- /dev/null +++ b/packages/services/service-automation/src/flow-cold-boot-refusal-withdraw.test.ts @@ -0,0 +1,172 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * A flow the `kernel:ready` cold-boot bind refuses is withdrawn, not left + * registered from the boot pull (#21848). + * + * A boot registers flows twice. The boot pull (`AutomationServicePlugin.start()`) + * runs before a plugin that contributes a node type has registered its executor + * from its own `start()`, so it cannot check that node's config keys against the + * descriptor's `configSchema`. The `kernel:ready` bind re-registers every flow + * once the executor exists. When that bind refused the flow, it only warned: the + * boot pull's registration stayed, `active` and bound to its trigger. + * + * Pinned on the real plugin path (a `LiteKernel`, the boot pull and the + * protocol view serving the same packaged flows, the node type's executor + * registered from a later plugin's `start()`): + * - the refused flow is withdrawn — not registered, not bound; + * - only that flow: the package's valid flow stays registered and bound. + */ + +import { describe, it, expect } from 'vitest'; +import { LiteKernel } from '@objectstack/core'; +import type { Plugin, PluginContext } from '@objectstack/core'; +import { defineActionDescriptor } from '@objectstack/spec/automation'; +import type { AutomationContext } from '@objectstack/spec/contracts'; +import { AutomationEngine } from './engine.js'; +import type { FlowTrigger, FlowTriggerBinding, NodeExecutor } from './engine.js'; +import { AutomationServicePlugin } from './plugin.js'; + +const flush = () => new Promise((r) => setTimeout(r, 0)); + +/** A plugin node type the spec knows nothing about: its descriptor declares `count`. */ +const STAMP = 'test_stamp'; + +function stampExecutor(): NodeExecutor { + return { + type: STAMP, + descriptor: defineActionDescriptor({ + type: STAMP, + version: '1.0.0', + name: 'Stamp', + category: 'custom', + paradigms: ['flow'], + source: 'plugin', + configSchema: { type: 'object', properties: { count: { type: 'number' } } }, + }), + async execute() { + return { success: true }; + }, + }; +} + +/** An active, record-triggered, packaged flow whose one plugin node carries `config`. */ +function stampFlow(name: string, config: Record) { + return { + name, + label: name, + type: 'autolaunched', + status: 'active', + _packageId: 'app.fixture', + nodes: [ + { id: 'start', type: 'start', label: 'Start', config: { objectName: 'expense', triggerType: 'record-after-create' } }, + { id: 'stamp', type: STAMP, label: 'Stamp', config }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'stamp' }, + { id: 'e2', source: 'stamp', target: 'end' }, + ], + }; +} + +/** A recording `record_change` trigger (stands in for the real one). */ +function recordingRecordChangeTrigger() { + const bound = new Set(); + const trigger: FlowTrigger = { + type: 'record_change', + start(binding: FlowTriggerBinding, _cb: (ctx: AutomationContext) => Promise) { + bound.add(binding.flowName); + }, + stop(flowName: string) { + bound.delete(flowName); + }, + }; + return { trigger, has: (n: string) => bound.has(n) }; +} + +/** + * The two reads a boot binds flows from: the `objectql` registry (the boot + * pull) and the protocol's flow view (the `kernel:ready` bind), both serving + * the same packaged flows, as a real boot does. + */ +function flowSourcesPlugin(flows: unknown[], rec: ReturnType): Plugin { + return { + name: 'test.flow-sources', + version: '1.0.0', + async init(ctx: PluginContext) { + const c = ctx as unknown as { + registerService(n: string, s: unknown): void; + getService(n: string): T; + }; + c.registerService('objectql', { + registry: { + listItems: (type: string) => (type === 'flow' ? flows : []), + getObject: () => undefined, + }, + }); + c.registerService('protocol', { + async getMetaItemsForExecution(q: { type: string }) { + return { items: q.type === 'flow' ? flows : [] }; + }, + }); + c.getService('automation').registerTrigger(rec.trigger); + }, + }; +} + +/** Contributes the plugin node type from its own `start()`, after the boot pull. */ +function lateStampPlugin(): Plugin { + return { + name: 'test.late-stamp', + version: '1.0.0', + async init() {}, + async start(ctx: PluginContext) { + (ctx as unknown as { getService(n: string): T }) + .getService('automation') + .registerNodeExecutor(stampExecutor()); + }, + }; +} + +describe('boot: a flow the kernel:ready bind refuses does not stay registered from the boot pull', () => { + it('withdraws the refused flow and keeps the valid one bound', async () => { + const rec = recordingRecordChangeTrigger(); + const kernel = new LiteKernel({ logger: { level: 'silent' } } as never); + kernel.use(new AutomationServicePlugin()); + // `cuont` is a key the node type's descriptor does not declare. + kernel.use(flowSourcesPlugin([stampFlow('typo', { cuont: 2 }), stampFlow('valid', { count: 2 })], rec)); + kernel.use(lateStampPlugin()); + await kernel.bootstrap(); + await flush(); + + const engine = kernel.getService('automation'); + expect(await engine.getFlow('typo'), 'refused at load ⇒ not registered').toBeNull(); + expect(rec.has('typo'), 'refused at load ⇒ not bound').toBe(false); + expect(engine.getActiveTriggerBindings().map((b) => b.flowName)).not.toContain('typo'); + + // Only the refused flow: the package's valid flow is registered and bound. + expect(await engine.getFlow('valid')).not.toBeNull(); + expect(rec.has('valid')).toBe(true); + + await kernel.shutdown(); + }); + + it('the refusal is the kernel:ready bind\'s: the same body registers when the executor is absent', async () => { + // The control for the pin above: with no plugin contributing the node + // type, neither boot step can check its keys, so the flow registers. + // A green pin above therefore reads the bind's refusal, not one the + // boot pull or the flow parse made on its own. + const rec = recordingRecordChangeTrigger(); + const kernel = new LiteKernel({ logger: { level: 'silent' } } as never); + kernel.use(new AutomationServicePlugin()); + kernel.use(flowSourcesPlugin([stampFlow('typo', { cuont: 2 })], rec)); + await kernel.bootstrap(); + await flush(); + + const engine = kernel.getService('automation'); + expect(await engine.getFlow('typo')).not.toBeNull(); + + await kernel.shutdown(); + }); +}); diff --git a/packages/services/service-automation/src/node-config-values-at-registration.test.ts b/packages/services/service-automation/src/node-config-values-at-registration.test.ts deleted file mode 100644 index 083eff6102a..00000000000 --- a/packages/services/service-automation/src/node-config-values-at-registration.test.ts +++ /dev/null @@ -1,331 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * A node's config VALUES are judged at registration by the contract its - * executor parses at run time (#21848) — the value half of the key-name check - * (`config-unknown-keys.test.ts`). - * - * Before: registration read a node's config against the descriptor's JSON - * Schema for key NAMES only, so an approval node with - * `escalation.timeoutHours: 0.5` (the contract says `>= 1`) registered, loaded - * `active`, and failed every run at the node with no approval request opened. - * - * Pinned here, on the engine and on the plugin's boot path: - * - a value the declared contract refuses refuses the flow, naming the flow, - * the node and the config path, and nothing is registered; - * - the refusal is the same CLASS as the key-name refusal (a plain error with - * no `code` / `status` of its own), so every door answers it the same way; - * - the judge is the executor's own contract, rules JSON Schema cannot carry - * included, and nodes inside an ADR-0031 region are judged too; - * - a node type whose executor declares no contract keeps today's behaviour; - * - on boot, a flow the boot pull registered before its node's executor - * existed, and the `kernel:ready` bind then refused, is withdrawn — it does - * not stay registered and bound behind the refusal's warning. - */ - -import { describe, it, expect } from 'vitest'; -import { LiteKernel } from '@objectstack/core'; -import type { Plugin, PluginContext } from '@objectstack/core'; -import { - APPROVAL_NODE_TYPE, - ApprovalNodeConfigSchema, - defineActionDescriptor, - getApprovalNodeConfigJsonSchema, -} from '@objectstack/spec/automation'; -import type { AutomationContext } from '@objectstack/spec/contracts'; -import { AutomationEngine } from './engine.js'; -import type { FlowTrigger, FlowTriggerBinding, NodeExecutor } from './engine.js'; -import { AutomationServicePlugin } from './plugin.js'; - -const flush = () => new Promise((r) => setTimeout(r, 0)); - -const SUB_HOUR = { enabled: true, timeoutHours: 0.5, action: 'notify' }; -const VALID = { enabled: true, timeoutHours: 4, action: 'notify' }; - -function silentLogger() { - const warnings: string[] = []; - const logger: any = { - info() {}, error() {}, debug() {}, - warn(msg: string, meta?: unknown) { warnings.push(`${msg} ${meta ? JSON.stringify(meta) : ''}`); }, - child() { return logger; }, - }; - return { logger, warnings }; -} - -/** - * An `approval` executor registered the way `plugin-approvals` registers its - * own: the descriptor publishes the JSON Schema (key names), and - * `configContract` is the schema `execute` parses. `withContract: false` is the - * shape every executor had before the member existed. - */ -function approvalExecutor(withContract = true): NodeExecutor { - return { - type: APPROVAL_NODE_TYPE, - descriptor: defineActionDescriptor({ - type: APPROVAL_NODE_TYPE, - version: '1.0.0', - name: 'Approval', - category: 'human', - paradigms: ['flow'], - source: 'plugin', - supportsPause: true, - resumeAuthority: 'service', - configSchema: getApprovalNodeConfigJsonSchema() as Record, - }), - ...(withContract ? { configContract: ApprovalNodeConfigSchema } : {}), - async execute() { - return { success: true, suspend: true }; - }, - }; -} - -/** An active, record-triggered flow whose approval node carries `escalation`. */ -function gatedFlow(name: string, escalation: Record, extraConfig: Record = {}) { - return { - name, - label: name, - type: 'autolaunched', - status: 'active', - nodes: [ - { id: 'start', type: 'start', label: 'Start', config: { objectName: 'expense', triggerType: 'record-after-create' } }, - { - id: 'gate', - type: APPROVAL_NODE_TYPE, - label: 'Gate', - config: { approvers: [{ type: 'position', value: 'manager' }], escalation, ...extraConfig }, - }, - { id: 'approved', type: 'end', label: 'Approved' }, - { id: 'rejected', type: 'end', label: 'Rejected' }, - ], - edges: [ - { id: 'e1', source: 'start', target: 'gate' }, - { id: 'e2', source: 'gate', target: 'approved', label: 'approve' }, - { id: 'e3', source: 'gate', target: 'rejected', label: 'reject' }, - ], - }; -} - -/** Register and return the thrown error (fails the test if it registers). */ -function refusalOf(engine: AutomationEngine, name: string, definition: unknown): Error & { code?: unknown; status?: unknown } { - try { - engine.registerFlow(name, definition as never); - } catch (err) { - return err as Error & { code?: unknown; status?: unknown }; - } - throw new Error(`expected '${name}' to be refused at registration, and it registered`); -} - -describe('registration judges node config VALUES with the executor-declared contract', () => { - it('refuses `escalation.timeoutHours: 0.5`, locating the flow, the node and the config path', async () => { - const engine = new AutomationEngine(silentLogger().logger); - engine.registerNodeExecutor(approvalExecutor()); - - const err = refusalOf(engine, 'sub_hour', gatedFlow('sub_hour', SUB_HOUR)); - expect(err.message).toContain("Flow 'sub_hour' rejected: 1 config value(s) the node's own contract refuses."); - expect(err.message).toContain("node 'gate' (approval): config.escalation.timeoutHours: "); - // The contract's own sentence, not a second one. - const contractSentence = ApprovalNodeConfigSchema.safeParse(gatedFlow('x', SUB_HOUR).nodes[1].config) - .error?.issues[0]?.message; - expect(contractSentence).toBeTruthy(); - expect(err.message).toContain(`config.escalation.timeoutHours: ${contractSentence}`); - // Nothing registered under the name. - expect(await engine.getFlow('sub_hour')).toBeNull(); - }); - - it('is the key-name refusal\'s class: a plain error with no code or status of its own', () => { - const engine = new AutomationEngine(silentLogger().logger); - engine.registerNodeExecutor(approvalExecutor()); - - const value = refusalOf(engine, 'sub_hour', gatedFlow('sub_hour', SUB_HOUR)); - const key = refusalOf(engine, 'bogus_key', gatedFlow('bogus_key', { ...VALID, bogusKey: 1 })); - // The key-name refusal still owns an undeclared key (it runs first). - expect(key.message).toContain('undeclared config key(s)'); - expect(key.message).toContain('at config.escalation.bogusKey'); - // Both reach the doors as the same class, so the `/automation` write - // doors answer both `400 VALIDATION_FAILED` and the boot skips both. - for (const err of [value, key]) { - expect(err).toBeInstanceOf(Error); - expect(err.code).toBeUndefined(); - expect(err.status).toBeUndefined(); - } - expect(Object.getPrototypeOf(value)).toBe(Object.getPrototypeOf(key)); - }); - - it('registers a valid escalation, and stores it unchanged', async () => { - const engine = new AutomationEngine(silentLogger().logger); - engine.registerNodeExecutor(approvalExecutor()); - - engine.registerFlow('valid', gatedFlow('valid', VALID) as never); - const stored = await engine.getFlow('valid'); - expect(stored?.nodes.find((n) => n.id === 'gate')?.config).toMatchObject({ escalation: VALID }); - }); - - it('judges with the whole contract: a rule JSON Schema cannot carry refuses too', () => { - const engine = new AutomationEngine(silentLogger().logger); - engine.registerNodeExecutor(approvalExecutor()); - - // `fallbackApprovers` is read only under `onEmptyApprovers: 'fallback'` - // — a `superRefine` rule of the contract, absent from its JSON Schema. - const definition = gatedFlow('ignored_fallback', VALID, { - onEmptyApprovers: 'fail', - fallbackApprovers: [{ type: 'user', value: 'u1' }], - }); - expect(ApprovalNodeConfigSchema.safeParse(definition.nodes[1].config).success).toBe(false); - const err = refusalOf(engine, 'ignored_fallback', definition); - expect(err.message).toContain("node 'gate' (approval): config.onEmptyApprovers: fallbackApprovers is only read"); - }); - - it('a node type whose executor declares no contract keeps key-name-only judgement', async () => { - const engine = new AutomationEngine(silentLogger().logger); - engine.registerNodeExecutor(approvalExecutor(false)); - - engine.registerFlow('unjudged', gatedFlow('unjudged', SUB_HOUR) as never); - expect(await engine.getFlow('unjudged')).not.toBeNull(); - }); - - it('judges a node inside an ADR-0031 region, naming the region', async () => { - const engine = new AutomationEngine(silentLogger().logger); - // A test node type whose contract refuses `count` below 1 — structural, - // like every contract the engine reads. - engine.registerNodeExecutor({ - type: 'stamp', - configContract: { - safeParse(value: unknown) { - const count = (value as { count?: unknown }).count; - return typeof count === 'number' && count >= 1 - ? { success: true, data: value } - : { success: false, error: { issues: [{ path: ['count'], message: 'must be at least 1' }] } }; - }, - }, - async execute() { - return { success: true }; - }, - }); - const flow = (count: number) => ({ - name: 'looped', - label: 'Looped', - type: 'autolaunched', - nodes: [ - { id: 'start', type: 'start', label: 'Start' }, - { - id: 'each', - type: 'loop', - label: 'Each', - config: { - collection: '{items}', - body: { - nodes: [{ id: 'inner', type: 'stamp', label: 'Inner', config: { count } }], - edges: [], - }, - }, - }, - { id: 'end', type: 'end', label: 'End' }, - ], - edges: [ - { id: 'e1', source: 'start', target: 'each' }, - { id: 'e2', source: 'each', target: 'end' }, - ], - }); - - const err = refusalOf(engine, 'looped', flow(0)); - expect(err.message).toMatch(/· node 'inner' \(stamp\): config\.count: must be at least 1/); - expect(err.message).toContain('each'); - - engine.registerFlow('looped', flow(2) as never); - expect(await engine.getFlow('looped')).not.toBeNull(); - }); -}); - -// ── The boot path: the executor registers AFTER the boot pull ────────────── - -/** A recording `record_change` trigger (stands in for the real one). */ -function recordingRecordChangeTrigger() { - const bound = new Set(); - const trigger: FlowTrigger = { - type: 'record_change', - start(binding: FlowTriggerBinding, _cb: (ctx: AutomationContext) => Promise) { - bound.add(binding.flowName); - }, - stop(flowName: string) { - bound.delete(flowName); - }, - }; - return { trigger, has: (n: string) => bound.has(n) }; -} - -/** - * The two reads a boot binds flows from: the `objectql` registry (the boot - * pull in `AutomationServicePlugin.start()`) and the protocol's flow view (the - * `kernel:ready` bind) — both serving the same packaged flows, as a real boot - * does. - */ -function flowSourcesPlugin(flows: unknown[], rec: ReturnType): Plugin { - return { - name: 'test.flow-sources', - version: '1.0.0', - async init(ctx: PluginContext) { - const c = ctx as unknown as { - registerService(n: string, s: unknown): void; - getService(n: string): T; - }; - c.registerService('objectql', { - registry: { - listItems: (type: string) => (type === 'flow' ? flows : []), - getObject: () => undefined, - }, - }); - c.registerService('protocol', { - async getMetaItemsForExecution(q: { type: string }) { - return { items: q.type === 'flow' ? flows : [] }; - }, - }); - c.getService('automation').registerTrigger(rec.trigger); - }, - }; -} - -/** - * A plugin that contributes the `approval` executor from its own `start()` — - * after `AutomationServicePlugin.start()` pulled and registered the flows, - * which is where `ApprovalsServicePlugin` registers it. - */ -function lateApprovalPlugin(): Plugin { - return { - name: 'test.late-approval', - version: '1.0.0', - async init() {}, - async start(ctx: PluginContext) { - (ctx as unknown as { getService(n: string): T }) - .getService('automation') - .registerNodeExecutor(approvalExecutor()); - }, - }; -} - -describe('boot: a flow the kernel:ready bind refuses does not stay registered from the boot pull', () => { - it('withdraws the refused flow and keeps the valid one bound', async () => { - const rec = recordingRecordChangeTrigger(); - const kernel = new LiteKernel({ logger: { level: 'silent' } } as never); - kernel.use(new AutomationServicePlugin()); - kernel.use( - flowSourcesPlugin( - [{ ...gatedFlow('sub_hour', SUB_HOUR), _packageId: 'app.fixture' }, { ...gatedFlow('valid', VALID), _packageId: 'app.fixture' }], - rec, - ), - ); - kernel.use(lateApprovalPlugin()); - await kernel.bootstrap(); - await flush(); - - const engine = kernel.getService('automation'); - expect(await engine.getFlow('sub_hour'), 'refused at load ⇒ not registered').toBeNull(); - expect(rec.has('sub_hour'), 'refused at load ⇒ not bound').toBe(false); - expect(engine.getActiveTriggerBindings().map((b) => b.flowName)).not.toContain('sub_hour'); - - // Only the refused flow: the package's valid flow is registered and bound. - expect(await engine.getFlow('valid')).not.toBeNull(); - expect(rec.has('valid')).toBe(true); - - await kernel.shutdown(); - }); -}); diff --git a/packages/services/service-automation/src/plugin.ts b/packages/services/service-automation/src/plugin.ts index fb06fd8ad14..e36e7077781 100644 --- a/packages/services/service-automation/src/plugin.ts +++ b/packages/services/service-automation/src/plugin.ts @@ -2487,13 +2487,13 @@ export class AutomationServicePlugin implements Plugin { * the boot pull made is withdrawn, so a flow refused at load is not left * registered, whichever of the two boot steps refused it. The two can * disagree because the node-type vocabulary grows between them — a plugin - * registers its node executor (and the config contract registration - * judges that node by) from its own `start()`, after the boot pull, so the - * pull registered such a flow unjudged and armed it. Leaving that - * registration in place behind this refusal's warning kept the flow - * `active` and bound: every run then failed at the node the refusal - * located. Only the refused name is withdrawn; a failed or empty READ - * still tears nothing down (the early returns above). + * registers its node executor, and with it the descriptor `configSchema` + * registration checks that node's config keys against, from its own + * `start()`, after the boot pull, so the pull registered such a flow + * unjudged and armed it. Leaving that registration in place behind this + * refusal's warning kept the flow `active` and bound to its trigger. + * Only the refused name is withdrawn; a failed or empty READ still tears + * nothing down (the early returns above). */ private async syncFlowsFromProtocol(ctx: PluginContext): Promise { if (!this.engine) return; From 195134d737c291f69789a0bc4f9c2031abacb1b4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 18:44:11 +0000 Subject: [PATCH 6/6] test(dogfood), docs, changeset: door pins through the flow parse's approval judge plus a plugin-node withdraw pin; changeset and docs say what the PR still does Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude --- ...1848-node-config-values-at-registration.md | 17 +- content/docs/automation/flows.mdx | 17 +- ...fig-values-at-registration.dogfood.test.ts | 175 ++++++++++++++---- 3 files changed, 155 insertions(+), 54 deletions(-) diff --git a/.changeset/21848-node-config-values-at-registration.md b/.changeset/21848-node-config-values-at-registration.md index 9f06cf29d4e..34abbdd3c23 100644 --- a/.changeset/21848-node-config-values-at-registration.md +++ b/.changeset/21848-node-config-values-at-registration.md @@ -1,22 +1,19 @@ --- '@objectstack/service-automation': minor -'@objectstack/plugin-approvals': minor --- -fix(service-automation)!: a flow registers only if every node's config value passes the contract its executor parses, so an approval node's `escalation.timeoutHours: 0.5` is refused at registration and at package load instead of failing every run +fix(service-automation)!: a flow the `kernel:ready` cold-boot bind refuses is no longer left registered and `active` from the boot pull Clause-②: no (narrowing) - + -**BREAKING**: a flow that registered and loaded before can now be refused. It ships as `minor` under the launch-window convention for accept-set narrowings. +**BREAKING**: a flow that loaded `active` before can now be absent after boot. It ships as `minor` under the launch-window convention for accept-set narrowings. -**What was accepted before.** Registration read a node's `config` against the node type's descriptor `configSchema` for its key NAMES only; the values were parsed for the first time when a run reached the node. An approval node whose `escalation.timeoutHours` was `0.5` (the contract says at least `1`) therefore registered through `POST /automation` and loaded `active` from a package, every record its trigger matched was created, and every run then failed at the approval node with `Approval node 'gate' has invalid config`: no approval request opened, so the record stood without the gate it was meant to pass, and the user who saved it saw nothing. +**What was kept before.** A boot registers a package's flows twice. The boot pull runs before a plugin that contributes a node type has registered its executor from its own `start()`, so it cannot check that node's config keys against the descriptor's `configSchema`, and it registers the flow and arms its trigger. The `kernel:ready` bind then re-registers every flow once the executor exists. When it refused one, for an undeclared config key for instance, it logged `[Automation] cold-boot flow bind: failed to register flow` and nothing else: the boot pull's registration stayed, `active` and bound to its trigger, so every run reached the node the refusal located. -**What is refused now.** A node executor may declare the contract it parses its config against (`NodeExecutor.configContract`, an optional member). Registration parses every node of that type with it, inside region bodies too, and refuses the flow on any finding. The approval node declares `ApprovalNodeConfigSchema`, the schema its executor already parses, so every value that schema refuses is refused at registration, rules its JSON Schema cannot carry included (a `fallbackApprovers` list beside an `onEmptyApprovers` policy that never reads it, for one). Through the `/automation` write doors the refusal is `400 VALIDATION_FAILED`, the class the undeclared-key refusal already has. At package load the flow is skipped with a warning and not registered; the rest of the package loads. +**What happens now.** A flow the `kernel:ready` bind refuses is withdrawn: it is not registered, its trigger is unbound, and the same warning names the flow and the refusal. Only that flow is withdrawn; the rest of the package loads, as it already did for a flow the boot pull refuses. A failed or empty read of the flow list still tears nothing down. -**Package load, both halves.** A plugin registers its node executor from its own `start()`, after the boot pull registers the package's flows, so the boot pull cannot judge those nodes; the `kernel:ready` bind re-registers every flow once the executors exist. A flow that bind refuses is now withdrawn. Before, the boot pull's registration stayed behind the refusal's warning, so the flow stayed `active` and bound to its trigger. That covered an approval node's undeclared config key as well: it was refused with a warning and kept running. It is now withdrawn like any refused flow. +**What an author sees now.** The flow is absent (`GET /automation/:name` answers `404`), and the boot warning carries the located refusal. The handling is to correct the config the warning locates; the flow then registers as before. -**What an author sees now.** The refusal names the flow, then one line per finding with the node, its type and the config path, followed by the contract's own sentence: `node 'gate' (approval): config.escalation.timeoutHours: Too small: expected number to be >=1`. The handling is to correct the value at the path it names; the flow then registers as before. - -**Unchanged.** What `execute` accepts. A node type whose executor declares no contract, which today covers every built-in node type, `approval_revise` and any third-party node type, is judged on key names alone as before; the built-in executors still parse their contracts at execute. A flow refused on a re-registration through the write doors keeps the definition the engine already held. +**Unchanged.** What `registerFlow` refuses, at any door. A flow refused through the `/automation` write doors keeps the definition the engine already held, and a runtime reload that brings a refused body keeps the registered one. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index f7eddf33e2d..00f676ba82c 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -138,7 +138,7 @@ Each node performs a specific action in the flow. | `id` | `string` | ✅ | Unique node identifier — unique across the **whole flow**: the top-level `nodes[]` and every region body (`loop.body`, `parallel.branches[]`, `try_catch.try` / `.catch`, at any depth) share one id space, and `FlowSchema` refuses a reused id at parse (`Duplicate node id …`, naming both locations) | | `type` | `string` | ✅ | Node type — a built-in id from the table above **or** a plugin-registered one. Per ADR-0018 the spec does not gate this with a closed enum; it is checked against the live action registry once that registry is complete — plugins contribute node types while they start, so flows registered during boot are checked in one pass when the vocabulary closes (all plugins started), and anything registered after that (Studio publish, dev reload) is checked immediately. Unknown types warn, never reject; executing one fails with `NO_EXECUTOR` | | `label` | `string` | ✅ | Display label | -| `config` | `object` | optional | Type-specific configuration — the registered executor's `configSchema` owns its shape. Keys that schema does not declare are rejected at `registerFlow()`, values too where the executor declares the contract it parses (the `approval` node does), and the built-in executors `parse()` the value against their Zod contract before running (#4277) | +| `config` | `object` | optional | Type-specific configuration — the registered executor's `configSchema` owns its shape. Keys that schema does not declare are rejected at `registerFlow()`, an `approval` node's config is judged whole against its declared contract at the flow parse, and the built-in executors `parse()` the value against their Zod contract before running (#4277) | | `connectorConfig` | `object` | ✅ on `connector_action` | `{ connectorId, actionId, input }` — the only input a `connector_action` node's executor reads. `FlowSchema` refuses a `connector_action` node without it, and one whose `connectorId` or `actionId` is blank (empty or whitespace only), at any depth including a region body. `connectorId` is the registered connector's `name`, `actionId` one of the action keys it declares; `input` is optional | | `position` | `{ x, y }` | optional | Visual position on canvas | | `timeoutMs` | `number` | optional | Per-node execution timeout | @@ -155,13 +155,14 @@ node type's published `configSchema` does not declare — naming the path, the declared key set, and a did-you-mean — and the contract-carrying builtins additionally `parse()` their config at execute time, refusing the node on a type or missing-`required` violation (#4277). A node type that publishes no -`configSchema` declares nothing, so nothing can be undeclared. Where a node -type's executor declares the contract it parses (`configContract` — the -`approval` node does), `registerFlow()` also parses every node of that type with -it, and a value the run would refuse (`escalation.timeoutHours: 0.5` under its -`>= 1`) refuses the flow, naming the flow, the node and the config path — at the -`/automation` write doors (`400 VALIDATION_FAILED`) and at boot, where a refused -flow is skipped with a warning and not left registered. +`configSchema` declares nothing, so nothing can be undeclared. An `approval` +node's config is judged whole by the flow parse, against the contract the spec +declares for it: an undeclared key or a value its executor would refuse +(`escalation.timeoutHours: 0.5` under its `>= 1`) refuses the flow at the +config path, so `registerFlow()` answers it at the `/automation` write doors +(`400 VALIDATION_FAILED`) and at boot. At boot a refused flow is skipped with a +warning and not left registered — including one refused only once a plugin's +node type has registered, after the boot pull had registered it. ### Node Examples diff --git a/packages/qa/dogfood/test/flow-node-config-values-at-registration.dogfood.test.ts b/packages/qa/dogfood/test/flow-node-config-values-at-registration.dogfood.test.ts index 0968da245fb..0df763c15e2 100644 --- a/packages/qa/dogfood/test/flow-node-config-values-at-registration.dogfood.test.ts +++ b/packages/qa/dogfood/test/flow-node-config-values-at-registration.dogfood.test.ts @@ -1,9 +1,9 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. // -// A flow node's config VALUES are judged where the flow registers, by the -// schema its executor parses at run time — through both registration doors -// an operator has: the package a stack ships (package load) and the admin -// write door (`POST /automation`). +// A flow whose node config breaks the node's contract is refused where it +// registers — through both registration doors an operator has: the package a +// stack ships (package load) and the admin write door (`POST /automation`) — +// and a flow refused at load is not left registered. // // ## What was broken // @@ -13,26 +13,39 @@ // (the contract says `>= 1`) therefore registered and loaded `active`, every // record the trigger matched was created, and every run then failed at the // approval node: no approval request opened, so the record existed without -// the gate it was meant to pass, and the user who saved it saw nothing. +// the gate it was meant to pass, and the user who saved it saw nothing. The +// flow parse (`FlowSchema`, which `registerFlow` runs first) now judges an +// approval node's config against its declared contract, whole. +// +// And at package load, a refusal could fail to hold. The boot pull registers a +// package's flows before a plugin that contributes a node type has registered +// its executor, so it cannot check that node's config keys; the `kernel:ready` +// bind re-registers every flow once the executor exists, and when it refused +// one it only warned — the boot pull's registration stayed `active`. // // ## What each case pins // -// - package load: the sub-hour flow is refused (not registered), and the boot -// says why with a located error (the flow, the node, the config path); +// - package load: an approval node's out-of-range escalation and its +// undeclared escalation key are refused (not registered), and the boot names +// the flow and the located config path; // - package load, the control: a valid escalation registers and RUNS — a // record of the trigger's object opens a pending approval request; -// - the admin door: the sub-hour flow is refused `400 VALIDATION_FAILED` with -// the located error, and nothing is registered under its name; +// - package load, a plugin node type whose executor registers after the boot +// pull: an undeclared config key is refused by the `kernel:ready` bind, and +// the flow is not left registered; its valid sibling is; +// - the admin door: both approval refusals answer `400 VALIDATION_FAILED` +// located at the config path, and nothing is registered under either name; // - the admin door, the control: a valid escalation registers. // // The fixture is built with `strict: false`, on purpose: the build door judges -// the same flow on its own, and this file pins the two runtime doors behind it, -// so the invalid body has to reach them. Everything else about the stack is an -// ordinary authored package. +// the same flows on its own, and this file pins the two runtime doors behind +// it, so the invalid bodies have to reach them. Everything else about the stack +// is an ordinary authored package. import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; import { bootStack, type VerifyStack } from '@objectstack/verify'; import { defineStack } from '@objectstack/spec'; +import { defineActionDescriptor } from '@objectstack/spec/automation'; import { ObjectSchema, Field } from '@objectstack/spec/data'; import { ApprovalsServicePlugin } from '@objectstack/plugin-approvals'; import { RecordChangeTriggerPlugin } from '@objectstack/trigger-record-change'; @@ -42,10 +55,18 @@ const OBJECT = 'esc_value_request'; const UNSTAFFED = 'esc_value_unstaffed'; const PACKAGED_SUB_HOUR = 'esc_value_packaged_sub_hour'; +const PACKAGED_UNKNOWN_KEY = 'esc_value_packaged_unknown_key'; const PACKAGED_VALID = 'esc_value_packaged_valid'; +const PACKAGED_PLUGIN_TYPO = 'esc_value_packaged_plugin_typo'; +const PACKAGED_PLUGIN_VALID = 'esc_value_packaged_plugin_valid'; const DOOR_SUB_HOUR = 'esc_value_door_sub_hour'; +const DOOR_UNKNOWN_KEY = 'esc_value_door_unknown_key'; const DOOR_VALID = 'esc_value_door_valid'; +/** The config path the flow parse locates, on the approval node (`nodes[1]`). */ +const SUB_HOUR_PATH = 'nodes.1.config.escalation.timeoutHours'; +const UNKNOWN_KEY_PATH = 'nodes.1.config.escalation.bogusKey'; + /** An active, record-triggered flow whose one approval node carries `escalation`. */ function gatedFlow(name: string, escalation: Record) { return { @@ -82,8 +103,61 @@ function gatedFlow(name: string, escalation: Record) { } const SUB_HOUR = { enabled: true, timeoutHours: 0.5, action: 'notify' }; +const UNKNOWN_KEY = { enabled: true, timeoutHours: 4, action: 'notify', bogusKey: 1 }; const VALID = { enabled: true, timeoutHours: 4, action: 'notify' }; +/** + * A plugin node type the spec knows nothing about, contributed the way a + * plugin contributes one: its executor (and the descriptor whose + * `configSchema` declares `count`) registers from the plugin's own `start()`, + * after the automation plugin's boot pull. + */ +const STAMP = 'esc_value_stamp'; + +function stampPlugin() { + return { + name: 'com.dogfood.esc-value-stamp', + version: '0.0.0', + async init() {}, + async start(ctx: { getService(name: string): T }) { + ctx.getService<{ registerNodeExecutor(executor: unknown): void }>('automation').registerNodeExecutor({ + type: STAMP, + descriptor: defineActionDescriptor({ + type: STAMP, + version: '0.0.0', + name: 'Stamp', + category: 'custom', + paradigms: ['flow'], + source: 'plugin', + configSchema: { type: 'object', properties: { count: { type: 'number' } } }, + }), + async execute() { + return { success: true }; + }, + }); + }, + }; +} + +/** A flow run by hand whose one node is the plugin node type, carrying `config`. */ +function stampFlow(name: string, config: Record) { + return { + name, + label: `Stamp ${name}`, + type: 'autolaunched', + status: 'active', + nodes: [ + { id: 'start', type: 'start', label: 'Start', config: {} }, + { id: 'stamp', type: STAMP, label: 'Stamp', config }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'stamp' }, + { id: 'e2', source: 'stamp', target: 'end' }, + ], + }; +} + const fixtureStack = defineStack( { manifest: { @@ -92,7 +166,7 @@ const fixtureStack = defineStack( version: '0.0.0', type: 'app', name: 'Escalation Values Fixture', - description: 'One object and two approval flows, one with an escalation value its contract refuses.', + description: 'One object, approval flows with escalation values their contract refuses, and plugin-node flows.', }, // ADR-0097: a record-change trigger registers only when the app declares it. requires: ['automation', 'triggers'], @@ -105,7 +179,14 @@ const fixtureStack = defineStack( fields: { name: Field.text({ label: 'Name', required: true }) }, }), ], - flows: [gatedFlow(PACKAGED_SUB_HOUR, SUB_HOUR), gatedFlow(PACKAGED_VALID, VALID)] as never, + flows: [ + gatedFlow(PACKAGED_SUB_HOUR, SUB_HOUR), + gatedFlow(PACKAGED_UNKNOWN_KEY, UNKNOWN_KEY), + gatedFlow(PACKAGED_VALID, VALID), + // `cuont` is a key the plugin node type's descriptor does not declare. + stampFlow(PACKAGED_PLUGIN_TYPO, { cuont: 2 }), + stampFlow(PACKAGED_PLUGIN_VALID, { count: 2 }), + ] as never, }, { strict: false }, ); @@ -113,10 +194,14 @@ const fixtureStack = defineStack( interface DispatcherEnvelope { success?: boolean; data?: Record; - error?: { code?: string; message?: string }; + error?: { + code?: string; + message?: string; + details?: { fields?: Array<{ field?: string; code?: string; message?: string }> }; + }; } -describe('a flow node config value its executor refuses is refused at registration', () => { +describe('a flow node config its contract refuses is refused at registration, and not left registered at load', () => { let stack: VerifyStack; let token: string; /** Everything the platform logger wrote while the stack booted. */ @@ -128,6 +213,10 @@ describe('a flow node config value its executor refuses is refused at registrati return { status: res.status, json }; }; + /** The boot lines that name `flow` and carry `fragment`. */ + const bootLines = (flow: string, fragment: string) => + bootOutput.split('\n').filter((line) => line.includes(flow) && line.includes(fragment)); + beforeAll(async () => { // The core logger writes through the process streams, not `console.*`. const lines: string[] = []; @@ -140,7 +229,7 @@ describe('a flow node config value its executor refuses is refused at registrati try { stack = await bootStack(fixtureStack as unknown as Parameters[0], { automation: true, - extraPlugins: [new RecordChangeTriggerPlugin(), new ApprovalsServicePlugin()], + extraPlugins: [new RecordChangeTriggerPlugin(), new ApprovalsServicePlugin(), stampPlugin()], }); } finally { outSpy.mockRestore(); @@ -154,17 +243,18 @@ describe('a flow node config value its executor refuses is refused at registrati await stack?.stop(); }); - it('package load: the sub-hour flow is not registered', async () => { - const read = await call('GET', `/automation/${PACKAGED_SUB_HOUR}`); - expect(read.status, JSON.stringify(read.json)).toBe(404); + it('package load: the sub-hour and the undeclared-key approval flows are not registered', async () => { + for (const name of [PACKAGED_SUB_HOUR, PACKAGED_UNKNOWN_KEY]) { + const read = await call('GET', `/automation/${name}`); + expect(read.status, `${name}: ${JSON.stringify(read.json)}`).toBe(404); + } }); - it('package load: the boot names the flow, the node and the config path it refused', () => { - const refusal = bootOutput - .split('\n') - .filter((line) => line.includes(PACKAGED_SUB_HOUR) && line.includes('escalation.timeoutHours')); - expect(refusal.length, 'no boot line locates the refused value').toBeGreaterThan(0); - expect(refusal.some((line) => line.includes("node 'gate'"))).toBe(true); + it('package load: the boot names each refused flow and the config path it refused', () => { + expect(bootLines(PACKAGED_SUB_HOUR, 'nodes[1].config.escalation.timeoutHours').length, bootOutput.slice(-4000)) + .toBeGreaterThan(0); + expect(bootLines(PACKAGED_UNKNOWN_KEY, 'nodes[1].config.escalation.bogusKey').length, bootOutput.slice(-4000)) + .toBeGreaterThan(0); }); it('package load, the control: a valid escalation registers active and runs', async () => { @@ -187,17 +277,30 @@ describe('a flow node config value its executor refuses is refused at registrati expect(opened[0].pending_approvers).toEqual([`position:${UNSTAFFED}`]); }); - it('the admin door: the sub-hour flow is refused with a located error, and nothing registers', async () => { - const refused = await call('POST', '/automation', gatedFlow(DOOR_SUB_HOUR, SUB_HOUR)); - expect(refused.status, JSON.stringify(refused.json)).toBe(400); - expect(refused.json.error?.code).toBe('VALIDATION_FAILED'); - const message = String(refused.json.error?.message ?? ''); - expect(message).toContain(DOOR_SUB_HOUR); - expect(message).toContain("node 'gate'"); - expect(message).toContain('escalation.timeoutHours'); - - const read = await call('GET', `/automation/${DOOR_SUB_HOUR}`); + it('package load: a flow the kernel:ready bind refuses is not left registered from the boot pull', async () => { + const read = await call('GET', `/automation/${PACKAGED_PLUGIN_TYPO}`); expect(read.status, JSON.stringify(read.json)).toBe(404); + expect(bootLines(PACKAGED_PLUGIN_TYPO, 'config.cuont').length, bootOutput.slice(-4000)).toBeGreaterThan(0); + + // Only that flow: its valid sibling of the same node type is registered. + const sibling = await call('GET', `/automation/${PACKAGED_PLUGIN_VALID}`); + expect(sibling.status, JSON.stringify(sibling.json)).toBe(200); + }); + + it('the admin door: both approval refusals answer 400 VALIDATION_FAILED at the config path, and nothing registers', async () => { + for (const [name, escalation, path] of [ + [DOOR_SUB_HOUR, SUB_HOUR, SUB_HOUR_PATH], + [DOOR_UNKNOWN_KEY, UNKNOWN_KEY, UNKNOWN_KEY_PATH], + ] as const) { + const refused = await call('POST', '/automation', gatedFlow(name, escalation)); + expect(refused.status, JSON.stringify(refused.json)).toBe(400); + expect(refused.json.error?.code).toBe('VALIDATION_FAILED'); + const fields = refused.json.error?.details?.fields ?? []; + expect(fields.map((f) => f.field), JSON.stringify(refused.json)).toContain(path); + + const read = await call('GET', `/automation/${name}`); + expect(read.status, JSON.stringify(read.json)).toBe(404); + } }); it('the admin door, the control: a valid escalation registers', async () => {