Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions .changeset/21850-flow-approval-node-config-contract-refused.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
---
'@objectstack/spec': minor
---

A flow `approval` node's `config` is judged at parse against the contract the spec declares for it, `ApprovalNodeConfigSchema`, whole: an undeclared key, a refused value and a required key left out are each refused with a location, in the contract's own words.

Clause-②: yes (narrowing)

<!-- adr-0087: registered flow-approval-node-config-contract-refused -->

**BREAKING**: an accept-set narrowing on a published authoring surface, shipped as `minor` under the launch-window convention for accept-set narrowings.

**Why.** The approval node's executor parses `node.config` against `ApprovalNodeConfigSchema` before it does anything else and fails the node on any issue. Registration already refused an undeclared key, but a refused value such as `escalation.timeoutHours: 0.5` registered and then failed every run that reached the node, and no build door asked about either: `objectstack validate` and `objectstack compile` exited 0 on an `escalation.bogusKey` or a `timeoutHours: 0.5`, and compile copied it into `dist/objectstack.json`.

**What is refused.** An `approval` node, at any depth, whose `config` the approval contract refuses. The judge is `flowNodeConfigRefusals`, the one `FlowSchema.parse`, `AutomationEngine.registerFlow` (which parses first) and `objectstack validate` share; the approval contract joins it as a declared contract map beside the builtin executor contracts, with no plugin loaded. Every issue that contract raises is refused, because the executor refuses on every one:

- an undeclared key, at the key (`nodes.N.config.escalation.bogusKey`, one issue per key, the top level included), and a refused value, at its key (`nodes.N.config.escalation.timeoutHours` for `0.5` under its minimum of 1): the new closed-set code `node-config-refused-by-contract`, `params: { nodeType, key }`, whose message carries the contract's own sentence — for an alias, its did-you-mean (`timeout` → `timeoutHours`);
- a required key left out (`approvers`; `timeoutHours` inside an `escalation` block): `node-config-key-missing`, as for a builtin node, or `node-config-key-required-by-rule` where a rule of the contract requires it.

The issue's `code` is `custom`. That covers `FlowSchema`, `defineFlow()`, `defineStack` (`STACK_SCHEMA_INVALID`, 422, at `flows.N.nodes.M.config.<key>`), `os validate`, `os compile`, an artifact's parse, `registerFlow` and the metadata save door (`422 INVALID_METADATA`).

**What stays accepted, byte for byte.** Every approval node the contract accepts, an `approval_revise` node, and every builtin node: the builtin arm still judges only a key left out, so an undeclared key or a wrong-typed value on a builtin node is judged where it was before. A plugin node type whose contract the spec does not declare stays outside the build doors.

## FROM → TO

| you wrote | write instead |
|:--|:--|
| `escalation: { …, bogusKey: 1 }`, or any key the contract does not declare | delete the key, or rename it to the one the refusal's did-you-mean names (`timeout` → `timeoutHours`, `mode` → `behavior`, `quorum` → `minApprovals`) |
| `escalation: { timeoutHours: 0.5 }` | `escalation: { timeoutHours: 1 }` — whole wall-clock hours, at least 1 |
| `escalation: { enabled: false }` with no `timeoutHours` | delete the `escalation` block |
| `steps`, `entryCriteria`, `onApprove`, `onReject` or `rejectionBehavior` on the node | the flow graph, as the refusal's guidance says (successive nodes, the entering edge's `condition`, the `approve` / `reject` out-edges, a back-edge) |
| an approval node with no `approvers` | `approvers: [{ type: 'position', value: '<position>' }]` (or any approver the contract accepts) |

**The one-line fix: write the shape the approval contract declares at the key the refusal names.** The runtime never ran such a node, so the fix changes nothing a working flow does.

**Who is affected, measured.** At `5e0b489bca`, every approval node `config` authored in this repository parses under the contract: `examples/**` (15 nodes, all in the showcase), `content/docs/**` (6 snippets), `skills/**` (5 snippets) and the `packages/qa/dogfood` fixtures (6 nodes), and so does the Studio designer's approval seed at the pinned objectui commit. Deployed metadata, and repositories other than these two, were not measured. Where such a node already sits in a stored flow, the whole flow is refused at registration: at boot it is skipped with a warn naming it, its trigger not armed, while the flows beside it register.

### The kit

- **The refusal.** The declared contract map in `automation/flow-node-config-refusals.ts`, read by the same executor-contract arm of `flowNodeConfigRefusals`; the new code joins `FLOW_SLOT_REFUSAL_CODES`.
- **The ledger.** The D3 semantic entry `flow-approval-node-config-contract-refused` (protocol 18). No key is removed, so there is no tombstone, and there is no D2 conversion: the platform cannot know the approvers, the key or the value the author meant.
1 change: 0 additions & 1 deletion packages/lint/src/runtime-gate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ const cleanApprovalFlow = {
type: 'approval',
config: {
approvers: [{ type: 'expression', value: 'current.owner' }],
emptyApproverPolicy: 'reject',
},
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ const validApprovalFlow = () => {
const flow = brokenApprovalFlow();
flow.nodes[1]!.config = {
approvers: [{ type: 'expression', value: 'current.owner' }],
emptyApproverPolicy: 'reject',
} as any;
return flow;
};
Expand Down
1 change: 0 additions & 1 deletion packages/objectql/src/plugin.authoring-channel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,6 @@ describe('#6710 — the authoring channel is threaded from plugin option to prot
const flow = brokenApprovalFlow();
(flow.nodes[1] as any).config = {
approvers: [{ type: 'expression', value: 'current.owner' }],
emptyApproverPolicy: 'reject',
};
const result = await (kernel.getService('protocol') as any).saveMetaItem({
type: 'flow', name: 'leave_approval', item: flow,
Expand Down
2 changes: 1 addition & 1 deletion packages/services/service-automation/src/engine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2626,7 +2626,7 @@ describe('Action Descriptor Registry (ADR-0018)', () => {
type: 'autolaunched' as const,
nodes: [
{ id: 'start', type: 'start', label: 'Start' },
{ id: 'custom', type, label: 'Custom' },
{ id: 'custom', type, label: 'Custom', ...(type === 'approval' ? { config: { approvers: [{ type: 'user', value: 'u1' }] } } : {}) },
{ id: 'end', type: 'end', label: 'End' },
],
edges: [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,265 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#21850] The build doors judge an `approval` node's `config` against the
* contract the spec declares for it, `ApprovalNodeConfigSchema`, WHOLE — the
* declared contract map in `flow-node-config-refusals.ts`, beside the builtin
* executor contracts and read by the same judge, `flowNodeConfigRefusals`.
*
* The approval executor (`plugin-approvals`) parses `node.config ?? {}`
* against that contract before it does anything else and fails the node on
* ANY issue, so every contract finding is one the run would refuse: a key the
* contract requires left out, a key it does not declare, a value it refuses.
* `FlowSchema` used to accept all three, so `objectstack validate` and
* `objectstack compile` exited 0 on them and compile copied the shape into the
* artifact; the author learned otherwise at the first run.
*
* Every door that parses a flow meets the judge: `FlowSchema` itself,
* `defineStack`, the stack parse `objectstack validate` and `compile` run, the
* registered `flow` type schema the metadata save door validates against, and
* an artifact's parse — pinned here. `registerFlow` parses first, and
* `validateStackExpressions` calls the same judge.
*
* No plugin is loaded for any of it: the contract is the spec's own. The
* builtin arm is unchanged, presence-only — a control below holds it there.
*/

import { describe, expect, it } from 'vitest';

import { getMetadataTypeSchema } from '../kernel/metadata-type-schemas';
import { MIGRATIONS_BY_MAJOR, RETIRED_KEYS_BY_MAJOR } from '../migrations/registry';
import { ArtifactStagePackageBodySchema, ObjectStackDefinitionSchema, defineStack } from '../stack.zod';
import { ApprovalEscalationSchema, ApprovalNodeConfigSchema } from './approval.zod';
import { flowNodeConfigRefusals, getBuiltinNodeConfigContracts } from './flow-node-config-refusals';
import { FlowSchema } from './flow.zod';

const ENTRY_ID = 'flow-approval-node-config-contract-refused';

type Config = Record<string, unknown>;

const APPROVERS = [{ type: 'position', value: 'finance_reviewer' }];

/** A whole approval config the contract accepts — the accept control, and the base every probe edits. */
const VALID: Config = {
approvers: APPROVERS,
behavior: 'first_response',
lockRecord: true,
escalation: { enabled: true, timeoutHours: 4, action: 'notify', notifySubmitter: true },
};

const withEscalation = (escalation: Config): Config => ({ ...VALID, escalation });

/** start → the approval node → approve / reject ends. */
function flowWith(config: unknown, name = 'approval_probe') {
return {
name,
label: 'Approval probe',
type: 'autolaunched',
nodes: [
{ id: 'start', type: 'start', label: 'Start' },
{ id: 'gate', type: 'approval', label: 'Gate', ...(config === undefined ? {} : { config }) },
{ 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' },
],
};
}

interface IssueSig { code: string; path: string; message: string }

function issuesOf(flow: unknown): IssueSig[] {
const r = FlowSchema.safeParse(flow);
return r.success ? [] : r.error.issues.map((i) => ({ code: i.code, path: i.path.join('.'), message: i.message }));
}

/** The approval contract's own sentence at one issue path — read, never re-spelled. */
function contractSentence(schema: { safeParse(v: unknown): { success: boolean; error?: { issues: Array<{ path: PropertyKey[]; message: string }> } } }, value: unknown, path: string): string {
const own = schema.safeParse(value);
return own.success ? '' : own.error?.issues.find((i) => i.path.join('.') === path)?.message ?? '';
}

describe('FlowSchema judges an approval node config against its declared contract, whole', () => {
it('an undeclared escalation key is refused at nodes.1.config.escalation.bogusKey', () => {
const config = withEscalation({ timeoutHours: 2, action: 'notify', bogusKey: 1 });
const issues = issuesOf(flowWith(config));
expect(issues.map(({ code, path }) => ({ code, path }))).toEqual([
{ code: 'custom', path: 'nodes.1.config.escalation.bogusKey' },
]);
// The judge's own words, never re-spelled, carrying the contract's own sentence.
expect(issues[0]!.message).toBe(flowNodeConfigRefusals('approval', config)[0]!.message);
expect(issues[0]!.message).toContain(contractSentence(ApprovalEscalationSchema, config.escalation, ''));
});

it('a timeoutHours under the contract minimum is refused at nodes.1.config.escalation.timeoutHours', () => {
const issues = issuesOf(flowWith(withEscalation({ timeoutHours: 0.5, action: 'notify' })));
expect(issues.map(({ code, path }) => ({ code, path }))).toEqual([
{ code: 'custom', path: 'nodes.1.config.escalation.timeoutHours' },
]);
});

it('the judge answers both with its code, params and path', () => {
const bogus = flowNodeConfigRefusals('approval', withEscalation({ timeoutHours: 2, bogusKey: 1 }));
const halfHour = flowNodeConfigRefusals('approval', withEscalation({ timeoutHours: 0.5 }));
expect([...bogus, ...halfHour].map(({ code, params, path, source }) => ({ code, params, path, source }))).toEqual([
{ code: 'node-config-refused-by-contract', params: { nodeType: 'approval', key: 'escalation.bogusKey' }, path: 'escalation.bogusKey', source: '' },
{ code: 'node-config-refused-by-contract', params: { nodeType: 'approval', key: 'escalation.timeoutHours' }, path: 'escalation.timeoutHours', source: '' },
]);
});

it('an alias keeps the contract\'s own did-you-mean, beside the key it leaves out', () => {
const config = withEscalation({ timeout: 2 });
const issues = issuesOf(flowWith(config));
expect(issues.map(({ path }) => path).sort()).toEqual([
'nodes.1.config.escalation.timeout',
'nodes.1.config.escalation.timeoutHours',
]);
const aliasSentence = contractSentence(ApprovalEscalationSchema, config.escalation, '');
expect(aliasSentence).toContain('`timeoutHours`');
expect(issues.find((i) => i.path.endsWith('.timeout'))!.message).toContain(aliasSentence);
});

it('an undeclared top-level key is refused at the key, one refusal per key', () => {
const issues = issuesOf(flowWith({ ...VALID, steps: [], quorum: 2 }));
expect(issues.map(({ path }) => path)).toEqual(['nodes.1.config.steps', 'nodes.1.config.quorum']);
});

it('a key the contract requires, left out, is refused with the builtin arm\'s code', () => {
for (const config of [undefined, {}]) {
expect(issuesOf(flowWith(config)).map(({ path }) => path), JSON.stringify(config)).toEqual(['nodes.1.config.approvers']);
expect(flowNodeConfigRefusals('approval', config).map(({ code }) => code)).toEqual(['node-config-key-missing']);
}
});

it('a value a rule of the contract refuses is refused in the rule\'s own words', () => {
const config = { ...VALID, onEmptyApprovers: 'fail', fallbackApprovers: APPROVERS };
const [refusal] = flowNodeConfigRefusals('approval', config);
expect(refusal!.path).toBe('onEmptyApprovers');
expect(refusal!.message).toContain(contractSentence(ApprovalNodeConfigSchema, config, 'onEmptyApprovers'));
});

it('the judge refuses exactly what the contract refuses, over a sweep of configs', () => {
const sweep: unknown[] = [
undefined, {}, VALID,
{ approvers: [] },
{ approvers: 'u1' },
{ approvers: [{ type: 'user', value: 'u1' }] },
{ ...VALID, behavior: 'weighted' },
{ ...VALID, minApprovals: 0 },
{ ...VALID, maxRevisions: 1.5 },
{ ...VALID, decisionOutputs: ['note', { key: 'picked', type: 'user', multiple: true }] },
withEscalation({ timeoutHours: 1 }),
withEscalation({ enabled: false }),
withEscalation({ timeoutHours: 2, action: 'escalate' }),
withEscalation({ timeoutHours: '2' }),
];
for (const config of sweep) {
const refused = flowNodeConfigRefusals('approval', config).length > 0;
expect(refused, JSON.stringify(config)).toBe(!ApprovalNodeConfigSchema.safeParse(config ?? {}).success);
}
});
});

describe('what stays accepted (lit controls)', () => {
it('CONTROL: a valid approval node parses', () => {
expect(issuesOf(flowWith(VALID))).toEqual([]);
expect(flowNodeConfigRefusals('approval', VALID)).toEqual([]);
});

it('CONTROL: an escalation at the contract minimum, and a node with no escalation block, parse', () => {
expect(issuesOf(flowWith(withEscalation({ timeoutHours: 1 })))).toEqual([]);
expect(issuesOf(flowWith({ approvers: APPROVERS }))).toEqual([]);
});

it('CONTROL: the builtin arm stays presence-only — an undeclared key on a builtin node still parses', () => {
const flow = {
...flowWith(VALID),
nodes: [
{ id: 'start', type: 'start', label: 'Start' },
{ id: 'call', type: 'http', label: 'Call', config: { url: 'https://example.test', bogusKey: 1 } },
{ id: 'done', type: 'end', label: 'Done' },
],
edges: [{ id: 'e1', source: 'start', target: 'call' }, { id: 'e2', source: 'call', target: 'done' }],
};
expect(issuesOf(flow)).toEqual([]);
// …and the builtin map, the executor-reconciled one, did not gain the plugin type.
expect(getBuiltinNodeConfigContracts().has('approval')).toBe(false);
});

it('CONTROL: approval_revise carries no config contract and is not judged', () => {
expect(flowNodeConfigRefusals('approval_revise', { anything: 1 })).toEqual([]);
});
});

describe('every door that parses a flow refuses it', () => {
const stackWith = (flows: unknown[]) => ({
manifest: { id: 'com.example.approvals', name: 'approvals', version: '1.0.0', type: 'app', namespace: 'apv' },
objects: [{ name: 'apv_request', label: 'Request', fields: { title: { type: 'text', label: 'Title' } } }],
flows,
});
const refused = flowWith(withEscalation({ timeoutHours: 2, bogusKey: 1 }), 'apv_refused');
const accepted = flowWith(VALID, 'apv_ok');

it('defineStack wraps the refusal in its ADR-0112 envelope, at flows.N.nodes.1.config.escalation.bogusKey', () => {
let refusal: { code?: unknown; status?: unknown; issues?: Array<{ path: unknown[]; code: string }> } | undefined;
try {
defineStack(stackWith([accepted, refused]) as never);
} catch (e) {
refusal = e as typeof refusal;
}
expect(refusal, 'defineStack must refuse the approval flow').toBeDefined();
expect({ code: refusal!.code, status: refusal!.status }).toEqual({ code: 'STACK_SCHEMA_INVALID', status: 422 });
expect(refusal!.issues!.map((i) => ({ path: i.path.join('.'), code: i.code }))).toEqual([
{ path: 'flows.1.nodes.1.config.escalation.bogusKey', code: 'custom' },
]);
});

it('CONTROL: defineStack accepts the valid approval flow alone', () => {
expect(() => defineStack(stackWith([accepted]) as never)).not.toThrow();
});

it('ObjectStackDefinitionSchema — the stack parse validate and compile run — refuses it at the same path', () => {
const r = ObjectStackDefinitionSchema.safeParse(stackWith([flowWith(withEscalation({ timeoutHours: 0.5 }), 'apv_half')]));
expect(r.success).toBe(false);
expect(r.success ? [] : r.error.issues.map((i) => i.path.join('.'))).toEqual(['flows.0.nodes.1.config.escalation.timeoutHours']);
expect(ObjectStackDefinitionSchema.safeParse(stackWith([accepted])).success).toBe(true);
});

it('the registered `flow` type schema — what the metadata save door validates against — refuses it too', () => {
const schema = getMetadataTypeSchema('flow') as unknown as typeof FlowSchema;
expect(schema).toBeDefined();
const r = schema.safeParse(refused);
expect(r.success).toBe(false);
expect(r.success ? [] : r.error.issues.map((i) => i.path.join('.'))).toEqual(['nodes.1.config.escalation.bogusKey']);
expect(schema.safeParse(accepted).success).toBe(true);
});

it('an artifact\'s parse refuses it', () => {
const body = { id: 'com.example.approvals', name: 'approvals', version: '1.0.0', type: 'app' };
const r = ArtifactStagePackageBodySchema.safeParse({ ...body, flows: [refused] });
expect(r.success).toBe(false);
expect(r.success ? [] : r.error.issues.map((i) => i.path.join('.'))).toEqual(['flows.0.nodes.1.config.escalation.bogusKey']);
const ok = ArtifactStagePackageBodySchema.safeParse({ ...body, flows: [accepted] });
expect(ok.success, JSON.stringify(ok.error?.issues ?? [])).toBe(true);
});
});

describe('the ADR-0087 ledger', () => {
it('registers one D3 entry at protocol 18, with no D2 conversion', () => {
const entries = MIGRATIONS_BY_MAJOR[18]!.semantic.filter((e) => e.id === ENTRY_ID);
expect(entries, 'the narrowing needs its own D3 entry').toHaveLength(1);
const [entry] = entries;
expect(entry!.conversionIds ?? []).toEqual([]);
expect(entry!.acceptanceCriteria.length).toBeGreaterThan(0);
});

it('registers no tombstone: no approval config key is removed', () => {
const all = Object.values(RETIRED_KEYS_BY_MAJOR).flat();
expect(all.filter((k) => /Approval(NodeConfig|Escalation)[^:]*:/.test(k))).toEqual([]);
// CONTROL: the flattened table is the real one — it carries a known step-18 tombstone.
expect(all).toContain('api/RestApiEndpoint:timeout');
});
});
Loading
Loading