Skip to content

Commit cc60165

Browse files
os-zhuangclaude
andauthored
feat(lint): a flow update_record node writing an undeclared field gates the build (#4271) (#4369)
The write-set family #4305 (hooks) and #4344 (actions) opened had a third surface, and it was the one the docs had spent the longest recommending as the safe alternative to the other two. A flow `update_record` node whose `config.fields` names a field the target object never declares was caught by NOTHING: `validate-readonly-flow-writes.ts` walks that exact map and explicitly stepped over the unknown key (`if (!meta) continue; // a form/field-layout lint concern` — a referral to a rule that does not check writes), and `validate-flow-template-paths.ts` checks the `{record.<path>}` READ tokens interpolated into node config, never the write-side key. New rule `flow-node-write-unknown-field`, wired into REFERENCE_INTEGRITY_RULES so `os validate`, `os lint` and `os compile` report it at once — one more path than the hand-wired readonly rule next door reaches. It GATES where its two siblings advise. The hook and action rules are advisory because they PARSE JavaScript: the finding is only as good as the extractor. Nothing here is parsed — `config.fields` is a literal map next to a literal `objectName`, the same certainty `flow-update-readonly-field` already gates on one config key over. A rule that errors on a write the engine strips while only warning on a write that names no column at all would be incoherent in the same map. And the runtime consequence is not the benign "consumer skips the unknown name and renders the rest" that keeps page-field-unknown / form-field-unknown advisory. Both halves were measured: through the engine an undeclared key reaches `driver.update` verbatim, and on SQLite/knex it becomes `no such column: stagee` — the statement is rejected WHOLE, so the correctly named fields in the same payload never land either. On a schemaless datasource the stray key is persisted into a column no schema-driven read returns. One field index and one implicit-field set across all three surfaces: `indexObjectFields` and `IMPLICIT_FIELDS` are imported from the hook rule rather than copied. Every skip is silent and exists so the gate only fires on a certainty — templated `objectName`, non-literal `fields`, cross-package objects, objects declaring no fields at all (external / introspected schemas), dotted keys. `runAs` is deliberately not consulted, unlike the readonly rule: an elevated identity bypasses the readonly strip, but no run identity conjures a column. Scope is declared as data: FLOW_WRITE_NODE_TYPES (today `update_record`) and FLOW_WRITE_NODE_TYPES_DEFERRED (`create_record`, with its reason) are partition-tested against the CRUD node types that carry a `fields` write map, derived behaviourally from the spec's executor-written config schemas. Docs: automation/hook-bodies.mdx and automation/hooks.mdx carried the pre-#4271 framing (write side "not checked", an "accepted gap with no planned closure") long after both body rules landed. Both now state the real asymmetry, with an explicit callout that a clean run is not proof. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent fe0465c commit cc60165

11 files changed

Lines changed: 677 additions & 10 deletions
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
---
2+
"@objectstack/lint": minor
3+
"@objectstack/spec": patch
4+
---
5+
6+
feat(lint): a flow `update_record` node writing an undeclared field gates the build (#4271)
7+
8+
The write-set family #4305 (hooks) and #4344 (actions) opened had a third
9+
surface, and it was the one the docs had spent the longest recommending as the
10+
safe alternative to the other two. A flow `update_record` node whose
11+
`config.fields` names a field the target object never declares was caught by
12+
**nothing**: `validate-readonly-flow-writes.ts` walks that exact map and
13+
explicitly stepped over the unknown key (`if (!meta) continue; // a
14+
form/field-layout lint concern` — a referral to a rule that does not check
15+
writes), and `validate-flow-template-paths.ts` checks the `{record.<path>}`
16+
READ tokens interpolated into node config, never the write-side key. So the
17+
surface `hook-bodies.mdx` pointed authors at — "prefer a flow `update_record`
18+
node, whose structural `fields` config is checked" — was the least checked of
19+
the three.
20+
21+
**New rule — `flow-node-write-unknown-field`, and it is an `error`.** Wired into
22+
`REFERENCE_INTEGRITY_RULES`, so `os validate`, `os lint` and `os compile` report
23+
it at once (one more place than the hand-wired readonly rule next door reaches).
24+
25+
**Why it gates where its two siblings advise.** The hook and action rules are
26+
advisory because they PARSE JavaScript: the finding is only as good as the
27+
extractor, and a false positive kills an advisory lint. Nothing here is parsed —
28+
`config.fields` is a literal map next to a literal `objectName`, the same
29+
certainty `flow-update-readonly-field` already gates on one config key over. A
30+
rule that errors on a write the engine *strips* while only warning on a write
31+
that names no column at all would be incoherent in the same `fields` map.
32+
33+
And the runtime consequence is not the benign "consumer skips the unknown name
34+
and renders the rest" that keeps `page-field-unknown` / `form-field-unknown`
35+
advisory. Both halves were measured, not inferred:
36+
37+
- Through the engine, an undeclared key reaches `driver.update` verbatim — the
38+
flow executor calls the data engine directly, the UPDATE path strips only
39+
readonly/readonlyWhen, and the SQL driver's `formatInput` /
40+
`applyWriteColumnMap` pass an unrecognized key straight through (`m[k] ?? k`).
41+
- On SQLite/knex it becomes `update "deal" set "name" = 'n2', "stagee" = 'won' …
42+
→ no such column: stagee`. The statement is rejected **whole**: `name` —
43+
spelled correctly, in the same payload — does not land either, and the step
44+
fails with a driver error naming a column, far from the authoring mistake.
45+
- On a schemaless datasource nothing rejects it, so the stray key is persisted
46+
into a column the object never declares, where no schema-driven read returns
47+
it.
48+
49+
That is the call `validate-searchable-fields` makes for a stale entry and
50+
`validate-flow-template-paths` makes for a filter-position token: gate when the
51+
miss breaks or corrupts the operation, advise when it merely narrows the output.
52+
53+
**One field index and one implicit-field set across all three surfaces.**
54+
`indexObjectFields` and `IMPLICIT_FIELDS` are imported from the hook rule rather
55+
than copied, so the three rules cannot drift on what is writable without being
56+
authored — the shape #4330 collapsed one package over.
57+
58+
Every skip exists so the gate only ever fires on a certainty, and each is
59+
silent: a templated `objectName`, a non-literal `fields` map, an object this
60+
stack does not define, an object that declares no fields at all (external /
61+
datasource-introspected schemas, the same skip `validate-searchable-fields`
62+
takes), and dotted keys (a nested-path write, not a top-level column). `runAs`
63+
is deliberately NOT consulted, unlike the readonly rule that skips
64+
`runAs:'system'` — an elevated identity bypasses the readonly strip, but no run
65+
identity conjures a column.
66+
67+
**Scope is declared as data, not left as silence.** `FLOW_WRITE_NODE_TYPES`
68+
(today `update_record`) and `FLOW_WRITE_NODE_TYPES_DEFERRED` (`create_record`,
69+
with its reason) are partition-tested against the CRUD node types that carry a
70+
`fields` write map — derived behaviourally from the spec's executor-written
71+
config schemas, not restated — so a node type that grows one later fails that
72+
test until someone classifies it.
73+
74+
`@objectstack/spec`: `ScriptBodySchema`'s "prefer a flow `update_record` node,
75+
whose structural `fields` config is error-checked" note now names the rule that
76+
makes it true. Doc comment only — no schema or generated-artifact change.
77+
78+
Docs: #4355 had just rewritten `automation/hook-bodies.mdx` to record this gap
79+
honestly — "**Prefer a flow `update_record` node when the write set is fixed —
80+
but not for *this* check** … writing a field the object never declares is
81+
currently reported by nothing at all. On that one axis an L2 body is now the
82+
better-checked surface." That bullet, and the matching note in
83+
`automation/hooks.mdx`, are the two sentences this change makes false. Both now
84+
say the axis has flipped back — and why the flow side lands a level *stronger*
85+
than the body side rather than merely level with it.

content/docs/automation/hook-bodies.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ Because the checking is advisory and literal-only:
161161

162162
- **Treat `hook-body-write-unknown-field` as a build failure by convention.** It does not gate, but the rule is tuned for near-zero false positives — in practice a warning is a real typo.
163163
- **Check by hand what the parser cannot see.** Computed keys, spreads, aliased input and dynamic object names are invisible to the rule; for an array or `"*"` hook, every field must exist on every target.
164-
- **Prefer a flow `update_record` node when the write set is fixed — but not for *this* check.** A flow node's writes are structured config: they diff field-by-field, render in the Console designer, and a write to a `readonly:true` field is a **gating error** (`flow-update-readonly-field`) that hooks have no counterpart for. What an `update_record` node does *not* get today is a field-existence check — writing a field the object never declares is currently reported by nothing at all. On that one axis an L2 body is now the better-checked surface.
164+
- **Prefer a flow `update_record` node when the write set is fixed — and for *this* check most of all.** A flow node's writes are structured config: they diff field-by-field, render in the Console designer, and a write to a `readonly:true` field is a **gating error** (`flow-update-readonly-field`) that hooks have no counterpart for. Since [#4271](https://github.com/objectstack-ai/objectstack/issues/4271) the field-existence check gates there too — `flow-node-write-unknown-field` is an **error**, not the advisory warning a body gets, because a node's `fields` is a literal map next to a literal `objectName`: there is no parser in between that could have mis-extracted it, so a finding is a certainty rather than a best effort.
165165
- **Exercise the hook against a real object before shipping** — on SQL drivers the mistake surfaces on the first write; schemaless drivers won't tell you.
166166

167167
### Signature conventions

content/docs/automation/hooks.mdx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,10 @@ Two structural reasons to prefer the flow when either could work:
3939
hook body's write set is checked too, but only for the literal patterns a
4040
parser can recognise and only as an advisory warning (see
4141
[Hook & Action Bodies](/docs/automation/hook-bodies#write-set-checking)).
42-
Note the one axis where this reverses: writing a field the target object never
43-
declares warns in a hook body, and is reported nowhere in an `update_record`
44-
node.
42+
Field existence is checked on both surfaces, at different strengths: a hook
43+
body gets a warning, an `update_record` node a **gating error**
44+
(`flow-node-write-unknown-field`) — its `fields` is a literal map next to a
45+
literal `objectName`, so nothing could have mis-read it.
4546
- **A flow reviews as data.** The node graph diffs field-by-field and renders
4647
in the Console designer with per-node run history; a hook body reviews as
4748
code only.

packages/lint/src/index.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,23 @@ export type {
296296
ActionBodyWriteExclusion,
297297
} from './validate-action-body-writes.js';
298298

299+
// The same write-set question on the third surface — a flow `update_record`
300+
// node's structural `config.fields`. Shares the hook rule's field index and
301+
// implicit-field set so all three agree on what is writable without being
302+
// authored; gates (`error`) rather than advising, because a literal key against
303+
// a literal object name is a certainty the parsed-JS rules cannot claim.
304+
export {
305+
validateFlowNodeWrites,
306+
FLOW_NODE_WRITE_UNKNOWN_FIELD,
307+
FLOW_WRITE_NODE_TYPES,
308+
FLOW_WRITE_NODE_TYPES_DEFERRED,
309+
} from './validate-flow-node-writes.js';
310+
export type {
311+
FlowNodeWriteFinding,
312+
FlowNodeWriteSeverity,
313+
FlowWriteNodeDeferral,
314+
} from './validate-flow-node-writes.js';
315+
299316
// One entry point for the reference-resolution rules above (#3583 §5 D5).
300317
// Adding a rule to `REFERENCE_INTEGRITY_RULES` runs it on `validate`, `lint`
301318
// and `compile` at once — the CLI call sites do not change.

packages/lint/src/reference-integrity-suite.test.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ describe('reference-integrity suite — membership', () => {
2828
'validateAiAgentAuthoring',
2929
'validateHookBodyWrites',
3030
'validateActionBodyWrites',
31+
'validateFlowNodeWrites',
3132
]);
3233
});
3334

@@ -166,6 +167,15 @@ describe('reference-integrity suite — every member actually runs', () => {
166167
type: 'get_record',
167168
config: { objectName: 'crm_lead', filter: { name: '{record.budget}' } },
168169
},
170+
// validateFlowNodeWrites: the node's structural write map names a
171+
// field crm_lead does not declare — the third surface in the #4271
172+
// family, and the only one whose finding is a certainty rather than
173+
// an extraction (so it gates).
174+
{
175+
id: 'stamp',
176+
type: 'update_record',
177+
config: { objectName: 'crm_lead', fields: { lead_score: 100 } },
178+
},
169179
],
170180
},
171181
],
@@ -191,6 +201,7 @@ describe('reference-integrity suite — every member actually runs', () => {
191201
// The one member that emits a second rule id — see the suite's comment on
192202
// why it rides along instead of becoming its own entry.
193203
expect(rules).toContain('action-record-write-discarded');
204+
expect(rules).toContain('flow-node-write-unknown-field');
194205
});
195206

196207
it('carries a gating flow-template finding through the suite (#3810)', () => {
@@ -212,9 +223,9 @@ describe('reference-integrity suite — every member actually runs', () => {
212223
expect(typeof f.message).toBe('string');
213224
expect(typeof f.hint).toBe('string');
214225
}
215-
// Object references run first, action-body writes last.
226+
// Object references run first, flow-node writes last.
216227
expect(findings[0].rule).toBe('object-reference-unknown');
217-
expect(findings[findings.length - 1].rule).toBe('action-body-write-unknown-field');
228+
expect(findings[findings.length - 1].rule).toBe('flow-node-write-unknown-field');
218229
});
219230

220231
it('returns nothing for an empty stack', () => {

packages/lint/src/reference-integrity-suite.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ import { validateAiToolReferences } from './validate-ai-tool-references.js';
6767
import { validateAiAgentAuthoring } from './validate-ai-agent-authoring.js';
6868
import { validateHookBodyWrites } from './validate-hook-body-writes.js';
6969
import { validateActionBodyWrites } from './validate-action-body-writes.js';
70+
import { validateFlowNodeWrites } from './validate-flow-node-writes.js';
7071

7172
export type ReferenceIntegritySeverity = 'error' | 'warning';
7273

@@ -135,6 +136,13 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [
135136
// drift this suite exists to end (`validateReadonlyFlowWrites` is the
136137
// standing proof — wired into `validate` and `compile`, never into `lint`).
137138
{ name: 'validateActionBodyWrites', run: validateActionBodyWrites },
139+
// The third surface that writes a record field set: a flow `update_record`
140+
// node's `config.fields`. Same question as the two rules above, but the map
141+
// is structural metadata rather than parsed JS, so a finding is a certainty
142+
// and gates (`error`) — see that module for why, and why the docs' long-
143+
// standing "prefer a flow node, it's checked" advice was the least true of
144+
// the three until it landed.
145+
{ name: 'validateFlowNodeWrites', run: validateFlowNodeWrites },
138146
];
139147

140148
/**

0 commit comments

Comments
 (0)