Skip to content

Commit 5ea2e38

Browse files
os-litantclaude
andauthored
test(runtime,cli): pin the ctx.api and flow-node undeclared-field-write refusals (#15370)
* wip(test): ctx.api undeclared-write pins, runtime half Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * wip(test): flow-node undeclared-write pin, cli half Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * test(runtime,cli): pin the ctx.api and flow-node undeclared-write refusals Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * test: type the query-options bags so the erasure ratchet holds Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 29b4aaa commit 5ea2e38

2 files changed

Lines changed: 510 additions & 2 deletions

File tree

Lines changed: 343 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,343 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#14241] What a flow CRUD node's `fields` write map ACTUALLY does when it
5+
* names a field the target object never declares — pinned end to end, through
6+
* the real AutomationEngine, the real builtin CRUD node executors, a real
7+
* ObjectQL and a real driver.
8+
*
9+
* ## The sentence this file holds up
10+
*
11+
* `validate-flow-node-writes` (@objectstack/lint) is `severity: 'error'` — it
12+
* GATES, where its two `ctx.api` siblings only advise — and its header states
13+
* the runtime consequence as measured fact:
14+
*
15+
* • the declared-field door refuses the write — `INVALID_FIELD` / 400,
16+
* "Unknown field 'stagee' on object 'deal'", identically on every
17+
* datasource, before any statement is built;
18+
* • the write is refused WHOLE: a correctly named field in the SAME payload
19+
* does not land either;
20+
* • on `create_record` the row is never created at all, so every later node
21+
* expecting `{<node>.id}` is working from a record that does not exist;
22+
* • the node catches the refusal and folds it into a step failure
23+
* (`create_record(deal) failed: …`), so the RUN fails — far from the
24+
* authoring mistake, which is why an author-time rule is worth having.
25+
*
26+
* #13858 rewrote that prose after measuring it. The harness it measured with
27+
* was a scratch and was deleted, so from that day the three corrected messages
28+
* asserted a runtime behaviour that nothing pinned — the exact drift
29+
* `packages/runtime/src/sandbox/undeclared-field-write-driver-split.integration.test.ts`
30+
* exists to prevent for the two call shapes IT covers. This file is the flow
31+
* node's half; the `ctx.api` half lives in that runtime file, beside the two
32+
* shapes it already pinned.
33+
*
34+
* ## Why this shape is not already covered
35+
*
36+
* The flow executor calls the data engine directly (`data.insert` /
37+
* `data.update` in service-automation's `builtin/crud-nodes.ts`), bypassing the
38+
* metadata-protocol ingress, so the node's `fields` map arrives as an ORDINARY
39+
* CALLER PAYLOAD. The refusal itself is therefore the pre-hook declared-field
40+
* door (#8682 insert, #8738 update) — which the runtime file already pins on
41+
* both driver families, including the schemaless family's "no shadow column".
42+
* What is unpinned, and what this file adds, is everything the flow layer wraps
43+
* around that refusal: whether the run fails or reports a clean success, what
44+
* the step says, whether the correctly named siblings survive, and whether the
45+
* row exists afterwards.
46+
*
47+
* ## Why there is no second driver arm here
48+
*
49+
* The lint prose says "identically on every datasource" — and the reason it can
50+
* is structural, not statistical: NO DRIVER IS REACHED. So this file proves the
51+
* structural fact directly (`writes` below counts every write verb the driver
52+
* is asked to perform, and the refusal cases assert zero) rather than sampling
53+
* two families and inferring it. A second family run could only ever agree with
54+
* the first about a code path neither of them executes.
55+
*
56+
* ⚠️ That is also the only shape available here. The schemaless witness in this
57+
* repo is `@objectstack/driver-memory`, whose every declaration is disposed of
58+
* in `scripts/driver-memory-census.ledger.json` and gated by
59+
* `pnpm check:driver-memory-census` (#6664, from #5704 / #5499). Admitting a
60+
* new test consumer of a frozen driver is a maintainer ruling, not a test
61+
* author's call — and the CLI's own ledger entry records that "the CLI imports
62+
* the driver nowhere". The zero-write assertion is what makes that a
63+
* non-sacrifice: the family-split question cannot arise below a door nothing
64+
* gets past.
65+
*/
66+
67+
import { describe, it, expect, afterEach } from 'vitest';
68+
import { mkdtempSync, rmSync } from 'node:fs';
69+
import { tmpdir } from 'node:os';
70+
import { join } from 'node:path';
71+
import { ObjectQL } from '@objectstack/objectql';
72+
import { SqlDriver } from '@objectstack/driver-sql';
73+
import { AutomationEngine, registerCrudNodes } from '@objectstack/service-automation';
74+
import type { EngineQueryOptions } from '@objectstack/spec/data';
75+
76+
/**
77+
* The read-backs, TYPED rather than cast — `check:query-options-erasure`
78+
* counts an `as any` options bag in test code too, and these are ordinary
79+
* `where` bags with no reason to be erased.
80+
*/
81+
const allRows: EngineQueryOptions = { where: {} };
82+
const rowById = (id: unknown): EngineQueryOptions => ({ where: { id } });
83+
84+
/** `stagee` is the typo under test; `stage` is the field that exists. */
85+
const DEAL = {
86+
name: 'deal',
87+
fields: {
88+
name: { type: 'text', name: 'name' },
89+
stage: { type: 'text', name: 'stage' },
90+
amount: { type: 'number', name: 'amount' },
91+
},
92+
};
93+
94+
/** Silent logger — the engine and the node pack both take one. */
95+
function makeLogger(): any {
96+
const l: any = { info() {}, warn() {}, error() {}, debug() {} };
97+
l.child = () => l;
98+
return l;
99+
}
100+
101+
/**
102+
* Every write verb the driver contract exposes, counted.
103+
*
104+
* This is the file's load-bearing instrument, not a convenience: "identically
105+
* on every datasource" is a claim about a code path that is never entered, and
106+
* the only honest way to pin a never-entered path is to watch the entrance.
107+
*/
108+
const WRITE_VERBS = ['create', 'update', 'upsert', 'delete', 'bulkCreate', 'bulkUpdate', 'updateMany', 'deleteMany'] as const;
109+
110+
function countWrites(driver: any): { total: () => number; byVerb: Record<string, number> } {
111+
const byVerb: Record<string, number> = {};
112+
for (const verb of WRITE_VERBS) {
113+
const original = driver[verb];
114+
if (typeof original !== 'function') continue;
115+
byVerb[verb] = 0;
116+
driver[verb] = function patched(this: unknown, ...args: unknown[]) {
117+
byVerb[verb] += 1;
118+
return original.apply(driver, args);
119+
};
120+
}
121+
return { total: () => Object.values(byVerb).reduce((a, b) => a + b, 0), byVerb };
122+
}
123+
124+
/** A `create_record` flow whose node writes `fields` into `deal`. */
125+
function createFlow(name: string, fields: Record<string, unknown>) {
126+
return {
127+
name, label: name, type: 'autolaunched',
128+
nodes: [
129+
{ id: 'start', type: 'start', label: 'Start' },
130+
{ id: 'c', type: 'create_record', label: 'Create', config: { objectName: 'deal', fields } },
131+
{ id: 'end', type: 'end', label: 'End' },
132+
],
133+
edges: [
134+
{ id: 'e1', source: 'start', target: 'c' },
135+
{ id: 'e2', source: 'c', target: 'end' },
136+
],
137+
} as any;
138+
}
139+
140+
/** An `update_record` flow naming one row by scalar id (no bulk intent). */
141+
function updateFlow(name: string, id: unknown, fields: Record<string, unknown>) {
142+
return {
143+
name, label: name, type: 'autolaunched',
144+
nodes: [
145+
{ id: 'start', type: 'start', label: 'Start' },
146+
{ id: 'u', type: 'update_record', label: 'Update', config: { objectName: 'deal', filter: { id }, fields } },
147+
{ id: 'end', type: 'end', label: 'End' },
148+
],
149+
edges: [
150+
{ id: 'e1', source: 'start', target: 'u' },
151+
{ id: 'e2', source: 'u', target: 'end' },
152+
],
153+
} as any;
154+
}
155+
156+
describe('#14241 a flow CRUD node writing an undeclared field', () => {
157+
let engine: ObjectQL | null = null;
158+
let dir: string | null = null;
159+
160+
afterEach(async () => {
161+
try { await engine?.destroy(); } catch { /* noop */ }
162+
engine = null;
163+
if (dir) { rmSync(dir, { recursive: true, force: true }); dir = null; }
164+
});
165+
166+
/** A real ObjectQL on a real sqlite table with only the declared columns. */
167+
async function boot() {
168+
dir = mkdtempSync(join(tmpdir(), 'os-14241-'));
169+
const driver = new SqlDriver({
170+
client: 'better-sqlite3',
171+
connection: { filename: join(dir, 'data.sqlite') },
172+
useNullAsDefault: true,
173+
});
174+
await driver.initObjects([DEAL]);
175+
engine = new ObjectQL();
176+
engine.registerDriver(driver as any, true);
177+
await engine.init();
178+
engine.registry.registerObject(DEAL as any);
179+
return { ql: engine, driver };
180+
}
181+
182+
/** The real builtin CRUD nodes over that engine — no stub in the chain. */
183+
function automationOver(ql: ObjectQL) {
184+
const logger = makeLogger();
185+
const automation = new AutomationEngine(logger);
186+
registerCrudNodes(automation, {
187+
logger,
188+
getService: (n: string) => (n === 'data' ? ql : undefined),
189+
} as any);
190+
return automation;
191+
}
192+
193+
const stepOf = async (automation: AutomationEngine, flow: string, nodeId: string) => {
194+
const runs = await automation.listRuns(flow);
195+
return runs[0].steps.find((s: any) => s.nodeId === nodeId)!;
196+
};
197+
198+
// ─── The envelope, at the seam the node hands the payload to ──────────────
199+
200+
/**
201+
* The node's `fields` map IS a caller payload, so the refusal it meets is
202+
* the ADR-0112 envelope the three lint messages quote. The node folds that
203+
* envelope into a string, and the engine then stamps the step it failed
204+
* `NODE_FAILURE` (`create_record` re-surfaces a node-level `code` only for
205+
* `DUPLICATE_RECORD`, `update_record` for nothing at all, and neither
206+
* reaches `step.error.code` anyway). So this is the one place in the flow
207+
* chain where the DOOR's `code` and `status` are still observable, and the
208+
* step assertions below pin what is left of them. Asserted here rather than
209+
* left to the step's prose: a message can be reworded, and a `toThrow()`
210+
* would pass on any error at all — including the driver-level failure this
211+
* door exists to make unreachable.
212+
*/
213+
it('the door answers INVALID_FIELD / 400 for the exact payload the node builds', async () => {
214+
const { ql } = await boot();
215+
216+
const onInsert: any = await ql.insert('deal', { name: 'first', stagee: 'won' } as any)
217+
.catch((x: unknown) => x);
218+
const seed: any = await ql.insert('deal', { name: 'seed', stage: 'open', amount: 10 });
219+
const onUpdate: any = await ql.update('deal', { id: seed.id, stagee: 'won' } as any)
220+
.catch((x: unknown) => x);
221+
222+
for (const err of [onInsert, onUpdate]) {
223+
expect(err?.code).toBe('INVALID_FIELD');
224+
expect(err?.status).toBe(400);
225+
expect(err?.field).toBe('stagee');
226+
expect(err?.message).toBe("Unknown field 'stagee' on object 'deal'");
227+
}
228+
}, 30000);
229+
230+
// ─── create_record ────────────────────────────────────────────────────────
231+
232+
describe('create_record', () => {
233+
it('fails the RUN, and the step names the refusal', async () => {
234+
const { ql } = await boot();
235+
const automation = automationOver(ql);
236+
automation.registerFlow('f_create_bad', createFlow('f_create_bad', { name: 'first', stagee: 'won' }));
237+
238+
const res = await automation.execute('f_create_bad', { userId: 'u1' });
239+
240+
expect(res.success).toBe(false);
241+
const step = await stepOf(automation, 'f_create_bad', 'c');
242+
expect(step.status).toBe('failure');
243+
// The step's error is an envelope of its own, and the WHOLE of it is
244+
// pinned: the flow layer reclassifies every failing node to
245+
// `NODE_FAILURE` (engine.ts, its single step-push site) and carries
246+
// the door's message verbatim inside it. So `INVALID_FIELD` is NOT
247+
// what a run reports — the message is the only channel that
248+
// survives the fold, which is why it is asserted whole rather than
249+
// by `toContain`.
250+
expect(step.error).toEqual({
251+
code: 'NODE_FAILURE',
252+
message: "create_record(deal) failed: Unknown field 'stagee' on object 'deal'",
253+
});
254+
}, 30000);
255+
256+
it('creates NO row — so a later {<node>.id} has nothing to read', async () => {
257+
const { ql, driver } = await boot();
258+
const writes = countWrites(driver);
259+
const automation = automationOver(ql);
260+
automation.registerFlow('f_create_none', createFlow('f_create_none', { name: 'first', stagee: 'won' }));
261+
262+
await automation.execute('f_create_none', { userId: 'u1' });
263+
264+
expect(await ql.find('deal', allRows)).toHaveLength(0);
265+
// "before any statement is built", measured rather than asserted in
266+
// prose: the driver was never asked to write anything, which is why
267+
// no datasource can answer this differently.
268+
expect(writes.total()).toBe(0);
269+
}, 30000);
270+
271+
it('CONTROL — the same node spelled right creates the row', async () => {
272+
const { ql } = await boot();
273+
const automation = automationOver(ql);
274+
automation.registerFlow('f_create_ok', createFlow('f_create_ok', { name: 'first', stage: 'won' }));
275+
276+
const res = await automation.execute('f_create_ok', { userId: 'u1' });
277+
278+
expect(res.success).toBe(true);
279+
const rows: any[] = await ql.find('deal', allRows);
280+
expect(rows).toHaveLength(1);
281+
expect(rows[0].stage).toBe('won');
282+
}, 30000);
283+
});
284+
285+
// ─── update_record ────────────────────────────────────────────────────────
286+
287+
describe('update_record', () => {
288+
it('fails the RUN, and the step names the refusal', async () => {
289+
const { ql } = await boot();
290+
const seed: any = await ql.insert('deal', { name: 'seed', stage: 'open', amount: 10 });
291+
const automation = automationOver(ql);
292+
automation.registerFlow('f_update_bad', updateFlow('f_update_bad', seed.id, { stagee: 'won' }));
293+
294+
const res = await automation.execute('f_update_bad', { userId: 'u1' });
295+
296+
expect(res.success).toBe(false);
297+
const step = await stepOf(automation, 'f_update_bad', 'u');
298+
expect(step.status).toBe('failure');
299+
expect(step.error).toEqual({
300+
code: 'NODE_FAILURE',
301+
message: "update_record(deal) failed: Unknown field 'stagee' on object 'deal'",
302+
});
303+
}, 30000);
304+
305+
it('refuses the write WHOLE — the correctly named field in the same map does not land either', async () => {
306+
const { ql, driver } = await boot();
307+
const seed: any = await ql.insert('deal', { name: 'seed', stage: 'open', amount: 10 });
308+
const writes = countWrites(driver);
309+
const automation = automationOver(ql);
310+
automation.registerFlow(
311+
'f_update_whole',
312+
updateFlow('f_update_whole', seed.id, { name: 'renamed', stagee: 'won' }),
313+
);
314+
315+
await automation.execute('f_update_whole', { userId: 'u1' });
316+
317+
const after: any = (await ql.find('deal', rowById(seed.id)))[0];
318+
// `name` was spelled correctly and rode in the same payload. An
319+
// author reading "the unknown key is skipped" would expect it to
320+
// land; it does not.
321+
expect(after.name).toBe('seed');
322+
expect(after.stage).toBe('open');
323+
// The assertion that separates a refusal from a silent write: a
324+
// datasource with no schema to check against would keep the key.
325+
expect(after).not.toHaveProperty('stagee');
326+
expect(writes.total()).toBe(0);
327+
}, 30000);
328+
329+
it('CONTROL — the same node spelled right updates the row', async () => {
330+
const { ql } = await boot();
331+
const seed: any = await ql.insert('deal', { name: 'seed', stage: 'open', amount: 10 });
332+
const automation = automationOver(ql);
333+
automation.registerFlow('f_update_ok', updateFlow('f_update_ok', seed.id, { name: 'renamed', stage: 'won' }));
334+
335+
const res = await automation.execute('f_update_ok', { userId: 'u1' });
336+
337+
expect(res.success).toBe(true);
338+
const after: any = (await ql.find('deal', rowById(seed.id)))[0];
339+
expect(after.name).toBe('renamed');
340+
expect(after.stage).toBe('won');
341+
}, 30000);
342+
});
343+
});

0 commit comments

Comments
 (0)