Skip to content

Commit a2509d7

Browse files
Trumpclaude
andauthored
fix(service-automation): refuse a nullish value envelope attributed, in the shared refusal (#17262)
* fix(service-automation): refuse a nullish value envelope attributed, in the shared refusal (#16439) `AutomationEngine.evaluateValueEnvelope` derives its verdict from `valueEnvelopeRefusals` — the same call `registerFlow` makes — so registration's reject set and evaluation's reject set are one set by construction. That covered every malformed envelope and exactly two shapes fell outside it: `null` and `undefined`. Neither published primitive judges them (the shape rule is a no-op off `isExpressionEnvelopeShaped`; `validateExpression` reads an absent `source` as "not authored"), so both returned no findings and the method read `envelope.source` off nothing — a bare `TypeError` with no `where`, no source and no rule. The presence rule is stated in the SHARED refusal, not as a guard in the evaluator: a reject reason living only on the evaluation side would end the property this design exists for, and the docblock that says so would stop being true. Refused rather than admitted. The asymmetry with the predicate path is deliberate: `structuralConditionRefusal` admits nullish because the condition field is optional, so absence there means "the author wrote no predicate". A value slot's envelope IS the value, so an absent one is a caller handing nothing where a value was required. Measured, not assumed: the value-role feeder (`resolveFlowNodeExpressions`) emits only envelope-shaped objects, so `registerFlow` never presents a nullish value to the shared refusal — this adds nothing to the registration reject set. An authored `null` in an `assignments` slot still parses (`FlowSchema.parse` accepts it) and still registers. The regression is the whole table, not the two repaired cells: all ten shapes the card enumerates are pinned to their messages byte for byte, so the eight already attributed are shown to be untouched. The condition side is pinned too, unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * test(service-automation): type the run output read in the nullish-envelope pin (#16439) `ExecutionResult.output` is typed `{}`, so reading `.digest` off it was a TS2339 the package's own `typecheck` catches (its tsconfig reaches every `*.test.ts`, which is how this surfaced). Narrowed at the read; the assertion is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 980dc78 commit a2509d7

3 files changed

Lines changed: 335 additions & 4 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/service-automation": patch
3+
---
4+
5+
fix(service-automation): a `null` / `undefined` envelope is refused attributed, not as a raw `TypeError` (#16439)
6+
7+
`AutomationEngine.evaluateValueEnvelope` derives its verdict from `valueEnvelopeRefusals` — the same call `registerFlow` makes — so registration's reject set and evaluation's reject set are one set by construction. That covered every malformed **envelope**, and exactly two shapes fell outside it: `null` and `undefined`. Neither published primitive judges them (the shape rule is a no-op on anything not `isExpressionEnvelopeShaped`, and `validateExpression` reads an absent `source` as "not authored"), so both returned no findings and the method went on to read `envelope.source` off nothing — `TypeError: Cannot read properties of null (reading 'source')`, with no `where`, no source and no rule. Driven across the ten shapes the card enumerates, eight failed attributed and only these two did not.
8+
9+
Both now fail attributed like the other eight, led by the published `ASSIGNMENT_VALUE_ENVELOPE_REFUSAL` sentence and carrying the `where` and the source. The rule is stated in the **shared** refusal, never as a guard in the evaluator: a reject reason living only on the evaluation side would end the very property this design has.
10+
11+
Refused rather than admitted, and the asymmetry with the predicate path is deliberate: `structuralConditionRefusal` admits `null` / `undefined` because the condition *field* is optional, so absence there means "the author wrote no predicate". A value slot's envelope **is** the value, so an absent one is a caller handing nothing where a value was required.
12+
13+
**Why `patch`, not `minor` and not nothing.** Nothing changes for authored metadata: the only production call site guards with `isExpressionEnvelopeShaped`, which neither shape satisfies, and the value-role feeder emits only envelope-shaped objects, so `registerFlow` never presents a nullish value to the shared refusal — measured, and pinned. An authored `null` in an `assignments` slot is still a literal, still parses and still registers. What does move is the runtime behaviour of a **public method on an exported class**: a direct caller that passed a nullish envelope used to get a language-level `TypeError` and now gets an attributed `Error`. That is a published surface, so it is not silent — but it adds no API, no option and no capability, and no correct caller has to adapt, which is what makes it a patch rather than a minor.
Lines changed: 271 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,271 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* **Every shape `evaluateValueEnvelope` can be handed fails ATTRIBUTED** (#16439)
5+
* — the exhaustive sweep the card is built on, pinned as one table.
6+
*
7+
* `AutomationEngine.evaluateValueEnvelope` is a public method on an exported
8+
* class, so its argument is not only what the `assignment` executor produces:
9+
* the executor guards the call with `isExpressionEnvelopeShaped`, but a DIRECT
10+
* caller has no such door. Driven across the ten shapes the card enumerates,
11+
* eight folded into one attributed refusal — a `where`, the source, and the
12+
* published rule sentence — and exactly two, `null` and `undefined`, fell
13+
* through as a bare `TypeError: Cannot read properties of null (reading
14+
* 'source')`. One cell left over after an exhaustive sweep, which is what makes
15+
* it the exception rather than the rule.
16+
*
17+
* ⚠️ The regression is the WHOLE table, not the two repaired cells. A test
18+
* covering only `null` / `undefined` could not show the other eight were left
19+
* alone, and "left the other eight alone" is most of what makes the fix safe —
20+
* so every row is pinned to its message BYTE-FOR-BYTE. The eight literals below
21+
* were captured on the unfixed tree at `origin/main` and must not drift; the
22+
* two new ones join them in the same shape.
23+
*
24+
* Composed from the published constants wherever one exists
25+
* (`ASSIGNMENT_VALUE_ENVELOPE_REFUSAL`, `EVALUATED_EXPRESSION_SOURCE_REQUIRED`)
26+
* so this file re-spells no sentence that has an owner. The two fragments below
27+
* that ARE literal belong to `@objectstack/formula`, which deliberately does not
28+
* export them (`validate.ts`: "the published surface … does not move for this
29+
* fix"); a legitimate rewording there re-captures this table rather than
30+
* loosening it.
31+
*
32+
* The refusal is stated in the SHARED `valueEnvelopeRefusals` — the same call
33+
* `registerFlow` makes — never as a guard in the evaluator, so registration's
34+
* reject set and evaluation's reject set stay one set by construction. The two
35+
* readings that show this adds nothing to what `registerFlow` rejects are
36+
* pinned at the bottom of this file, together with the deliberate asymmetry on
37+
* the condition side, which this card must NOT harmonise away.
38+
*/
39+
40+
import { describe, it, expect, beforeEach } from 'vitest';
41+
import {
42+
ASSIGNMENT_VALUE_ENVELOPE_REFUSAL,
43+
FlowSchema,
44+
isExpressionEnvelopeShaped,
45+
resolveFlowNodeExpressions,
46+
structuralConditionRefusal,
47+
} from '@objectstack/spec/automation';
48+
import { EVALUATED_EXPRESSION_SOURCE_REQUIRED } from '@objectstack/spec';
49+
import { AutomationEngine } from '../engine.js';
50+
import { registerLogicNodes } from './logic-nodes.js';
51+
52+
function createTestLogger(): any {
53+
return {
54+
info: () => {}, warn: () => {}, error: () => {}, debug: () => {},
55+
child: () => createTestLogger(),
56+
};
57+
}
58+
function createCtx(): any {
59+
return { logger: createTestLogger(), getService: () => undefined };
60+
}
61+
62+
/** The `where` every attributed refusal must carry — the slot's own ledger path. */
63+
const WHERE = 'assignments.digest';
64+
65+
/** `@objectstack/formula`'s non-string-`source` refusal, unexported by design. */
66+
const NON_STRING_SOURCE =
67+
'invalid value envelope: an expression envelope carries its expression as a string `source` — found a number. '
68+
+ "Write the expression as bare text (e.g. `record.rating >= 4`), or as an envelope whose `source` is that text "
69+
+ "(e.g. `{ dialect: 'cel', source: '…' }`).";
70+
71+
/** The CEL engine's own fault for an envelope it cannot run, unexported by design. */
72+
const AST_ONLY = 'AST-only evaluation not yet supported; persist `source`';
73+
74+
/** A refusal raised by `valueEnvelopeRefusals`: rule sentence, detail, source. */
75+
const refusal = (detail: string) => `${WHERE}: ${ASSIGNMENT_VALUE_ENVELOPE_REFUSAL} ${detail} — source: \`\``;
76+
/** A located CEL fault raised after the refusals pass found nothing. */
77+
const celFault = (detail: string) => `${WHERE}: value expression failed to evaluate as CEL: ${detail} — source: \`\`.`;
78+
79+
/**
80+
* The card's table, every row. `attributedBefore` records which rows already
81+
* failed attributed on the unfixed tree — the eight that must not move, versus
82+
* the two this card repairs.
83+
*/
84+
const TABLE: ReadonlyArray<{
85+
label: string;
86+
value: unknown;
87+
message: string;
88+
attributedBefore: boolean;
89+
}> = [
90+
{ label: '{ source: 1 }', value: { source: 1 }, attributedBefore: true, message: refusal(NON_STRING_SOURCE) },
91+
{
92+
label: "{ dialect: 'cel', source: 1 }", value: { dialect: 'cel', source: 1 }, attributedBefore: true,
93+
message: refusal('`source`: ' + EVALUATED_EXPRESSION_SOURCE_REQUIRED),
94+
},
95+
{
96+
label: "{ dialect: 'cel', source: {} }", value: { dialect: 'cel', source: {} }, attributedBefore: true,
97+
message: refusal('`source`: ' + EVALUATED_EXPRESSION_SOURCE_REQUIRED),
98+
},
99+
{
100+
label: '{ ast, source: 1 }', value: { ast: { op: 'value' }, source: 1 }, attributedBefore: true,
101+
message: refusal(NON_STRING_SOURCE),
102+
},
103+
{
104+
label: "{ dialect: 'cel' }", value: { dialect: 'cel' }, attributedBefore: true,
105+
message: refusal('`source`: ' + EVALUATED_EXPRESSION_SOURCE_REQUIRED),
106+
},
107+
// Not envelope-shaped and not nullish: both validators pass, `.source` reads
108+
// `undefined` off a boxed primitive / an array / a plain object, and the CEL
109+
// engine faults on the empty source — located, and carrying it.
110+
{ label: '42', value: 42, attributedBefore: true, message: celFault(AST_ONLY) },
111+
{ label: "['a']", value: ['a'], attributedBefore: true, message: celFault(AST_ONLY) },
112+
{ label: '{}', value: {}, attributedBefore: true, message: celFault(AST_ONLY) },
113+
// The two cells this card repairs. Before: `TypeError: Cannot read properties
114+
// of null (reading 'source')`, with no `where`, no source and no rule.
115+
{
116+
label: 'null', value: null, attributedBefore: false,
117+
message: refusal(
118+
'no envelope was passed: the argument is `null`, so there is nothing to evaluate. An absent envelope is '
119+
+ "not \"not authored\" — the predicate side admits absence because the condition field is optional, but a "
120+
+ "value slot's envelope IS the value. Write `{ dialect: 'cel', source: '…' }`.",
121+
),
122+
},
123+
{
124+
label: 'undefined', value: undefined, attributedBefore: false,
125+
message: refusal(
126+
'no envelope was passed: the argument is `undefined`, so there is nothing to evaluate. An absent envelope '
127+
+ "is not \"not authored\" — the predicate side admits absence because the condition field is optional, but "
128+
+ "a value slot's envelope IS the value. Write `{ dialect: 'cel', source: '…' }`.",
129+
),
130+
},
131+
];
132+
133+
/** The thrown error, so a pin can assert its class and its message. */
134+
function catchError(fn: () => unknown): Error {
135+
try {
136+
fn();
137+
} catch (error) {
138+
return error as Error;
139+
}
140+
throw new Error('expected the call to throw');
141+
}
142+
143+
describe('evaluateValueEnvelope — every shape fails attributed (#16439)', () => {
144+
let engine: AutomationEngine;
145+
beforeEach(() => {
146+
engine = new AutomationEngine(createTestLogger());
147+
registerLogicNodes(engine, createCtx());
148+
});
149+
150+
it.each(TABLE)('$label — attributed, byte for byte', ({ value, message }) => {
151+
// The cast is the surface itself: the declared parameter is an envelope, so
152+
// only an untyped direct caller reaches the nullish rows. That is precisely
153+
// the caller this card exists for.
154+
const error = catchError(() => engine.evaluateValueEnvelope(value as any, new Map(), WHERE));
155+
156+
// ⛔ Not a `TypeError` — the failure mode this card removes. Asserted as the
157+
// class, so a future refactor cannot satisfy the message pin with a throw
158+
// that is still a language-level fault.
159+
expect(error).toBeInstanceOf(Error);
160+
expect(error).not.toBeInstanceOf(TypeError);
161+
162+
// Attributed: the `where` leads, the source is carried, and the message is
163+
// byte-identical to what this shape has always produced.
164+
expect(error.message.startsWith(`${WHERE}: `), 'the refusal must carry its `where`').toBe(true);
165+
expect(error.message).toContain('source: `');
166+
expect(error.message).toBe(message);
167+
});
168+
169+
it('the table is the card\'s table: ten shapes, eight already attributed, two repaired', () => {
170+
expect(TABLE).toHaveLength(10);
171+
expect(TABLE.filter((row) => row.attributedBefore)).toHaveLength(8);
172+
expect(TABLE.filter((row) => !row.attributedBefore).map((row) => row.label)).toEqual(['null', 'undefined']);
173+
});
174+
175+
it('every row leads with the published rule or a located CEL fault — no third vocabulary', () => {
176+
for (const row of TABLE) {
177+
const rest = row.message.slice(`${WHERE}: `.length);
178+
const known =
179+
rest.startsWith(ASSIGNMENT_VALUE_ENVELOPE_REFUSAL) || rest.startsWith('value expression failed to evaluate as CEL:');
180+
expect(known, `${row.label} must lead with a published sentence`).toBe(true);
181+
}
182+
});
183+
});
184+
185+
describe('the refusal is the SHARED one — registration and evaluation stay one set (#16439)', () => {
186+
let engine: AutomationEngine;
187+
beforeEach(() => {
188+
engine = new AutomationEngine(createTestLogger());
189+
registerLogicNodes(engine, createCtx());
190+
});
191+
192+
/**
193+
* Why teaching the shared refusal costs the registration side nothing —
194+
* measured, not assumed. The value-role feeder emits ONLY envelope-shaped
195+
* objects, and neither nullish shape is one, so `registerFlow` never presents
196+
* a nullish value to `valueEnvelopeRefusals` at all.
197+
*/
198+
it('the value-role feeder never emits a nullish value, so registration cannot reach the new rule', () => {
199+
expect(isExpressionEnvelopeShaped(null)).toBe(false);
200+
expect(isExpressionEnvelopeShaped(undefined)).toBe(false);
201+
202+
const emitted = resolveFlowNodeExpressions('assignment', {
203+
assignments: { nothing: null, absent: undefined, ok: { dialect: 'cel', source: '1' } },
204+
});
205+
expect(emitted.map((e) => e.path)).toEqual(['assignments.ok']);
206+
});
207+
208+
/**
209+
* The other half of the same reading, and the one triage asked for by name:
210+
* an authored `null` in a declared `value` slot parses today. It still does —
211+
* `null` in an assignment map is a literal, and this card does not touch that.
212+
*/
213+
it('`FlowSchema.parse` still accepts an authored `null` in a declared `value` slot, and the flow still registers', () => {
214+
const flow = {
215+
name: 'assign_flow', label: 'Assign Flow', type: 'autolaunched' as const,
216+
variables: [{ name: 'digest', type: 'text', isOutput: true }],
217+
nodes: [
218+
{ id: 'start', type: 'start' as const, label: 'Start' },
219+
{ id: 'assign', type: 'assignment' as const, label: 'Set', config: { assignments: { nothing: null } } },
220+
{ id: 'end', type: 'end' as const, label: 'End' },
221+
],
222+
edges: [
223+
{ id: 'e1', source: 'start', target: 'assign' },
224+
{ id: 'e2', source: 'assign', target: 'end' },
225+
],
226+
};
227+
const parsed = FlowSchema.safeParse(flow);
228+
expect(parsed.success).toBe(true);
229+
expect((parsed as any).data.nodes[1].config).toEqual({ assignments: { nothing: null } });
230+
expect(() => engine.registerFlow('assign_flow', flow as any)).not.toThrow();
231+
});
232+
233+
/** An authored `null` still assigns `null` — the executor path is untouched. */
234+
it('an authored `null` still assigns the literal `null` at run time', async () => {
235+
const flow = {
236+
name: 'assign_null', label: 'Assign Null', type: 'autolaunched' as const,
237+
variables: [{ name: 'digest', type: 'text', isOutput: true }],
238+
nodes: [
239+
{ id: 'start', type: 'start' as const, label: 'Start' },
240+
{ id: 'assign', type: 'assignment' as const, label: 'Set', config: { assignments: { digest: null } } },
241+
{ id: 'end', type: 'end' as const, label: 'End' },
242+
],
243+
edges: [
244+
{ id: 'e1', source: 'start', target: 'assign' },
245+
{ id: 'e2', source: 'assign', target: 'end' },
246+
],
247+
};
248+
engine.registerFlow('assign_null', flow as any);
249+
const result = await engine.execute('assign_null', {} as any);
250+
expect(result.success).toBe(true);
251+
expect((result.output as Record<string, unknown> | undefined)?.digest ?? null).toBeNull();
252+
});
253+
});
254+
255+
describe('the condition side does not move — the asymmetry is deliberate (#16439)', () => {
256+
/**
257+
* ⛔ This card must not "harmonise" the predicate path. `null` / `undefined`
258+
* are admitted there ON PURPOSE: the condition FIELD is optional, so absence
259+
* means "the author wrote no predicate". A value slot's envelope IS the value,
260+
* which is why the same absence is refused on this side and only this side.
261+
*/
262+
it('`structuralConditionRefusal` still returns nothing for `null` / `undefined`', () => {
263+
expect(structuralConditionRefusal(null as any)).toBeUndefined();
264+
expect(structuralConditionRefusal(undefined as any)).toBeUndefined();
265+
});
266+
267+
it('and still refuses the malformed condition shapes it always refused', () => {
268+
expect(structuralConditionRefusal({ source: 1 } as any)).toBeDefined();
269+
expect(structuralConditionRefusal({} as any)).toBeDefined();
270+
});
271+
});

0 commit comments

Comments
 (0)