Skip to content

Commit 4b2cbf7

Browse files
claude[bot]claude
andauthored
fix(lint): state the measured refusal in the three write-set rule messages (#14243)
The ctx.api branch of validate-hook-body-writes, validate-action-body-writes and validate-flow-node-writes all described a driver-dependent outcome for an undeclared write — a driver-level error on SQL, a silently persisted stray key on a schemaless driver. For the paths these three rules judge, that stopped being true when the declared-field door landed: all three carry a CALLER-supplied payload (ctx.api is a ScopedContext over the running engine; a flow node hands its fields map to the data engine directly), and the door refuses a caller-named undeclared key before any statement is built. Measured on all three paths and both driver families before the prose was rewritten — real QuickJS sandbox, real ObjectQL engine, real AutomationEngine with the builtin CRUD node executors, driver-sql (better-sqlite3) and driver-memory: every run answered INVALID_FIELD / 400, "Unknown field 'stagee' on object 'deal'", nothing was stored, and the schemaless family kept no shadow column. Message text only — rule ids, severities, match sets and hints unchanged. unprovisionedAnchorWriteConsequence() is untouched: an ADR-0015 external object's injected anchor IS declared in the registered schema, so it passes the door by construction and the remote database really is what refuses it. Each corrected message is pinned, positively on the new wording and negatively on the retired driver-split phrasing. The stale file headers that asserted the same split are corrected alongside the sentences they explain. Claude-Session: https://claude.ai/code/session_01WLJQhde67SeTccsmnBVarV Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7d3b1b7 commit 4b2cbf7

7 files changed

Lines changed: 231 additions & 38 deletions
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
---
2+
"@objectstack/lint": patch
3+
---
4+
5+
fix(lint): the three write-set rule messages now state the refusal authors actually get, not a retired driver split (#13858)
6+
7+
Message text only. Rule ids, severities, match sets and hints are untouched, and
8+
no finding changes shape — but a lint's own header states why the prose is
9+
governed: *"a lint that misdescribes the failure it is warning about teaches the
10+
wrong debugging instinct"*. These three sentences did.
11+
12+
`validate-hook-body-writes` (the `ctx.api` branch), `validate-action-body-writes`
13+
and `validate-flow-node-writes` all told the author that an undeclared write has
14+
a **driver-dependent** outcome:
15+
16+
> on a SQL driver the whole call then fails with a driver-level error far from here; on a schemaless driver (memory, MongoDB) the stray key is persisted
17+
18+
For the paths those three rules judge, that has not been true since the
19+
declared-field door landed (#8682 insert, #8738 update). All three describe a
20+
write whose payload is **caller-supplied**, not a mutation of an in-flight
21+
`ctx.input`: `ctx.api` is a `ScopedContext` over the running engine, and a flow
22+
node hands its `fields` map to the data engine directly. The door refuses a
23+
caller-named undeclared key from the object's field map **before any statement is
24+
built**, so no driver is reached and there is no split to observe.
25+
26+
Measured before the prose was rewritten — all three paths, both driver families,
27+
through a real QuickJS sandbox, a real `ObjectQL` engine, the real
28+
`AutomationEngine` with the real builtin CRUD node executors, real
29+
`@objectstack/driver-sql` (better-sqlite3) and real `@objectstack/driver-memory`:
30+
31+
| path | driver-sql | driver-memory |
32+
|---|---|---|
33+
| hook body `ctx.api.object(x).update({…})` | `INVALID_FIELD` / 400 | `INVALID_FIELD` / 400 |
34+
| action body `ctx.api.object(x).update({…})` | `INVALID_FIELD` / 400 | `INVALID_FIELD` / 400 |
35+
| flow `create_record` / `update_record` `fields` | `INVALID_FIELD` / 400 | `INVALID_FIELD` / 400 |
36+
37+
Every run answered `Unknown field 'stagee' on object 'deal'`; nothing was stored
38+
on either family, and the schemaless family kept **no** shadow column — the half
39+
the old message promised and the runtime no longer delivers.
40+
41+
The three messages now name that refusal in the vocabulary the `ctx.input`
42+
sibling landed with (`REFUSED at run time — INVALID_FIELD / 400, identically on
43+
every driver`), say why the door and not a driver answers, and keep each path's
44+
own blast radius: the hook refusal fails the operation that triggered the hook,
45+
the action refusal fails the action, and the flow node's refusal is whole — the
46+
correctly named fields in the same payload never land either, `create_record`
47+
never creates the row, and the step fails the run. That last clause is why the
48+
flow rule still gates at `error`; the severity is unchanged.
49+
50+
`unprovisionedAnchorWriteConsequence()` in the same files is **untouched**: an
51+
ADR-0015 external object's injected anchor *is* declared in the registered
52+
schema, so it passes the door by construction and the remote database really is
53+
what refuses it. That message was already correct.

packages/lint/src/validate-action-body-writes.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,35 @@ describe('validateActionBodyWrites — ctx.api writes', () => {
167167
expect(findings[0].hint).toContain("'discount_total'");
168168
});
169169

170+
// [#13858] The same rewrite as the hook sibling, from the same measurement:
171+
// real QuickJS sandbox, a real L2 ACTION body run through
172+
// `actionBodyRunnerFactory`, a real ObjectQL engine, real driver-sql
173+
// (better-sqlite3) AND real driver-memory. Both families answered
174+
// `INVALID_FIELD` / 400, "Unknown field 'stagee' on object 'deal'"; the
175+
// target row was untouched and the memory family stored no shadow column.
176+
// The old text promised a driver-level error on SQL and a persisted stray
177+
// key on schemaless — neither happens on this path, and has not since
178+
// #8682/#8738 put the declared-field door ahead of any statement.
179+
it('states the measured refusal — INVALID_FIELD / 400 on every driver — and no driver split', () => {
180+
const [finding] = validateActionBodyWrites(
181+
stackWith("await ctx.api.object('crm_deal').update({ discont_total: 0 });"),
182+
);
183+
184+
expect(finding.message).toContain('INVALID_FIELD / 400');
185+
expect(finding.message).toContain('identically on every driver');
186+
expect(finding.message).toContain('before any statement is built');
187+
// The reason the door — not a driver — is what answers.
188+
expect(finding.message).toContain('ordinary CALLER write');
189+
// The action-side blast radius, the one word that differs from the hook
190+
// sibling's sentence. Pinned so a future sweep cannot flatten the two.
191+
expect(finding.message).toContain('fails the action');
192+
193+
expect(finding.message).not.toMatch(/driver-level error/);
194+
expect(finding.message).not.toMatch(/schemaless/);
195+
expect(finding.message).not.toMatch(/is persisted/);
196+
expect(finding.message).not.toMatch(/write-path validator skips/);
197+
});
198+
170199
it('checks insert/create/update payloads (argument 0) and updateById at argument 1', () => {
171200
const findings = validateActionBodyWrites(
172201
stackWith(

packages/lint/src/validate-action-body-writes.ts

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,18 @@
77
// `HookBodySchema` union, parsed by the same `HookBodySchema.safeParse` in
88
// `actionBodyRunnerFactory` (packages/runtime/src/sandbox/body-runner.ts), run
99
// in the same QuickJS sandbox. So it fails the same way — an action body that
10-
// writes a field the target object never declares reaches the driver
11-
// unfiltered, and the outcome is DRIVER-DEPENDENT: on SQL the stray column
12-
// fails the whole call with a driver-level error far from the authoring
13-
// mistake, on a schemaless driver the stray key is persisted. Same #4271
14-
// split as the hook side (see that file's header for the measured chain, and
15-
// `undeclared-field-write-driver-split.integration.test.ts` for the pin); the
16-
// hook rule alone left half the surface uncovered.
10+
// writes a field the target object never declares is refused at run time, far
11+
// from the authoring mistake. [#13858] That refusal is NOT driver-dependent,
12+
// and the message says so: this rule judges exactly one shape,
13+
// `ctx.api.object('<literal>').insert|create|update|updateById(…)`, and
14+
// `ctx.api` is a ScopedContext over the running engine, so the payload is
15+
// CALLER-supplied and the declared-field door (#8682 insert, #8738 update)
16+
// refuses it — `INVALID_FIELD` / 400, identically on driver-sql and
17+
// driver-memory, before any statement is built. Measured on both families
18+
// through the real sandbox and the real engine; the caller-payload half of
19+
// that door is pinned in
20+
// `undeclared-field-write-driver-split.integration.test.ts`. The hook rule
21+
// alone left half the surface uncovered, which is why this file exists.
1722
//
1823
// ─── What does NOT carry over ───────────────────────────────────────────────
1924
//
@@ -429,9 +434,14 @@ export function validateActionBodyWrites(stack: AnyRec): ActionBodyWriteFinding[
429434
path: site.path,
430435
message:
431436
`body calls ctx.api.object('${w.object}').${w.method ?? 'update'}(…) writing '${w.field}', but ` +
432-
`object '${w.object}' declares no such field. The write-path validator skips the unknown key — ` +
433-
`on a SQL driver the whole action then fails with a driver-level error far from here; on a ` +
434-
`schemaless driver (memory, MongoDB) the stray key is persisted (#4271).`,
437+
// [#13858] Same door, same measurement as the hook sibling — ctx.api
438+
// is a ScopedContext over the running engine, so this payload is
439+
// CALLER-supplied and #8682/#8738 refuse it before any driver.
440+
`object '${w.object}' declares no such field. ctx.api is a scoped handle on the running ` +
441+
`engine, so the payload arrives as an ordinary CALLER write and the declared-field door ` +
442+
`REFUSES it at run time — INVALID_FIELD / 400, identically on every driver (#4271), before ` +
443+
`any statement is built. The write lands nothing, and the refusal escapes the body and ` +
444+
`fails the action.`,
435445
hint: fixHint(w.field, [...known]),
436446
});
437447
}

packages/lint/src/validate-flow-node-writes.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,41 @@ describe('validateFlowNodeWrites', () => {
181181
expect(findings[0].hint).toMatch(/Did you mean (one of: )?'stage'/);
182182
});
183183

184+
// [#13858] This rule GATES (severity `error`), so its message is what an
185+
// author reads while their build is refused — the one place a wrong causal
186+
// story costs the most. It used to say "on a SQL datasource the driver
187+
// rejects the whole statement ('no such column') … on a schemaless one the
188+
// stray key is persisted". Measured through the real AutomationEngine, the
189+
// real builtin CRUD nodes, a real ObjectQL engine and BOTH families
190+
// (driver-sql on better-sqlite3, driver-memory): neither happens. Both
191+
// answered `INVALID_FIELD` / 400, "Unknown field 'stagee' on object 'deal'",
192+
// the node folded that into `create_record(deal) failed: …`, the run failed,
193+
// and nothing was stored on either family — no row on create, an untouched
194+
// row and no shadow column on update.
195+
it('states the measured refusal — INVALID_FIELD / 400 on every datasource — and no driver split', () => {
196+
const [finding] = validateFlowNodeWrites({
197+
objects: [dealObject],
198+
flows: [flowWith({ stagee: 'won' })],
199+
});
200+
201+
expect(finding.message).toContain('INVALID_FIELD / 400');
202+
expect(finding.message).toContain('identically on every datasource');
203+
expect(finding.message).toContain('before any statement is built');
204+
// Why the door answers and not a datasource: the node hands `fields`
205+
// straight to the data engine, so it is a caller payload.
206+
expect(finding.message).toContain('ordinary caller payload');
207+
// The severity's own justification, unchanged by the rewrite and still
208+
// stated: the refusal is WHOLE, so correctly named siblings are lost too.
209+
expect(finding.message).toContain('never land either');
210+
expect(finding.message).toContain('the step fails the run');
211+
212+
// The retired driver split, both halves.
213+
expect(finding.message).not.toMatch(/no such column/);
214+
expect(finding.message).not.toMatch(/schemaless/);
215+
expect(finding.message).not.toMatch(/is persisted/);
216+
expect(finding.message).not.toMatch(/Nothing between the node and storage/);
217+
});
218+
184219
it('flags every unknown key in one node, and only those', () => {
185220
const findings = validateFlowNodeWrites({
186221
objects: [dealObject],

packages/lint/src/validate-flow-node-writes.ts

Lines changed: 35 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -26,27 +26,31 @@
2626
//
2727
// And the runtime consequence is not the benign "consumer skips the unknown
2828
// name and does the rest" that keeps `page-field-unknown` / `form-field-unknown`
29-
// advisory. Nothing between the node and storage removes the key: the flow
30-
// executor calls the data engine directly (bypassing the metadata-protocol
31-
// ingress, which strips `readonly` — not unknown — keys anyway), the engine's
32-
// write paths strip only readonly/readonlyWhen, and the SQL driver's
33-
// `formatInput` / `applyWriteColumnMap` pass an unrecognized key straight
34-
// through (`m[k] ?? k`). Every branch below was measured, not inferred:
29+
// advisory. The flow executor calls the data engine directly (`data.insert` /
30+
// `data.update` in service-automation's `builtin/crud-nodes.ts`, bypassing the
31+
// metadata-protocol ingress), so the node's `fields` map arrives as an ordinary
32+
// CALLER payload — and [#13858] the declared-field door (#8682 insert, #8738
33+
// update) refuses a caller-named undeclared key from the object's field map
34+
// before any statement is built. Every branch below was measured through the
35+
// real AutomationEngine, the real builtin CRUD nodes, the real engine and BOTH
36+
// driver families (driver-sql on better-sqlite3, driver-memory), not inferred:
3537
//
36-
// • Through the engine, an undeclared key reaches `driver.update` /
37-
// `driver.create` verbatim, alongside the audit stamps.
38-
// • On SQLite/knex an UPDATE becomes `update "deal" set "name" = 'n2',
39-
// "stagee" = 'won' … → no such column: stagee`. The statement is rejected
40-
// WHOLE: `name` — spelled correctly, in the same payload — does not land
41-
// either, and the step fails with a driver error naming a column, far from
42-
// the authoring mistake.
43-
// • An INSERT fails the same way (`table deal has no column named stagee`),
44-
// and one notch harder: the row is never created at all, so every later
45-
// node that expected `{<node>.id}` is working from a record that does not
46-
// exist.
47-
// • On a schemaless datasource (memory, MongoDB) nothing rejects it, so the
48-
// stray key is persisted into a column the object never declares — where no
49-
// schema-driven read surface will return it.
38+
// • Both families answer identically — `INVALID_FIELD` / 400, "Unknown field
39+
// 'stagee' on object 'deal'". No driver is reached, so there is no split to
40+
// observe.
41+
// • The write is refused WHOLE: `name` — spelled correctly, in the same
42+
// payload — does not land either.
43+
// • On `create_record` the row is never created at all, so every later node
44+
// that expected `{<node>.id}` is working from a record that does not exist.
45+
// • The node catches the refusal and folds it into a step failure
46+
// (`create_record(deal) failed: Unknown field 'stagee' on object 'deal'`),
47+
// so the RUN fails — far from the authoring mistake, which is exactly why
48+
// an author-time rule is still worth having.
49+
//
50+
// ⚠️ Until #13858 this block described the pre-#8682 driver split (SQL rejected
51+
// the statement, a schemaless datasource persisted the stray key). That is
52+
// retired, not merely restated: the severity below is unchanged because neither
53+
// the old outcome nor the new one is ever "the rest still works".
5054
//
5155
// No outcome is "the rest still works". That is the same call
5256
// `validate-searchable-fields` makes for a stale entry and
@@ -290,12 +294,18 @@ export function validateFlowNodeWrites(stack: AnyRec): FlowNodeWriteFinding[] {
290294
where: `flow "${flowName}" › ${nodeWhere}`,
291295
path: `${nodePath}.config.fields.${fieldName}`,
292296
message:
293-
`${node.type} writes '${fieldName}', but object '${objectName}' declares no such field. Nothing ` +
294-
`between the node and storage removes the key: on a SQL datasource the driver rejects the whole ` +
295-
`statement ('no such column'), so the correctly named fields in this same payload never land ` +
296-
`either${
297+
// [#13858] The node hands `fields` to the data engine directly
298+
// (`data.insert` / `data.update` in service-automation's
299+
// crud-nodes), so it is a CALLER payload and the #8682/#8738
300+
// declared-field door refuses it before any datasource is reached.
301+
// Measured on driver-sql and driver-memory alike.
302+
`${node.type} writes '${fieldName}', but object '${objectName}' declares no such field. The ` +
303+
`node hands its fields map to the engine as an ordinary caller payload, so the ` +
304+
`declared-field door REFUSES the whole write — INVALID_FIELD / 400, identically on every ` +
305+
`datasource, before any statement is built. The correctly named fields in this same payload ` +
306+
`never land either${
297307
node.type === 'create_record' ? ' and the record is never created at all' : ''
298-
}; on a schemaless one the stray key is persisted into a column no read surface returns.`,
308+
}, and the step fails the run.`,
299309
hint: fixHint(fieldName, [...known]),
300310
});
301311
}

packages/lint/src/validate-hook-body-writes.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,43 @@ describe('validateHookBodyWrites — ctx.api writes', () => {
246246
expect(findings[0].hint).toContain("'email'");
247247
});
248248

249+
// [#13858] The message is the whole product of an advisory rule, so the
250+
// sentence IS the deliverable. It used to promise a driver-dependent outcome
251+
// ("on a SQL driver … a driver-level error; on a schemaless driver … the
252+
// stray key is persisted"), which has not been true for this path since
253+
// #8682/#8738: `ctx.api` is a ScopedContext over the running engine, so the
254+
// payload is CALLER-supplied and the declared-field door refuses it first.
255+
//
256+
// Measured before this text was written — real QuickJS sandbox, real hook
257+
// body, real ObjectQL, real driver-sql (better-sqlite3) AND real
258+
// driver-memory: both families answered `INVALID_FIELD` / 400, "Unknown field
259+
// 'stagee' on object 'deal'", the target row was untouched, and the memory
260+
// family stored no shadow column. Same door the caller-payload half of
261+
// `undeclared-field-write-driver-split.integration.test.ts` pins.
262+
it('states the measured refusal — INVALID_FIELD / 400 on every driver — and no driver split', () => {
263+
const [finding] = validateHookBodyWrites(
264+
stackWith("await ctx.api.object('crm_deal').update({ id, stag: 'won' });"),
265+
);
266+
267+
// What the author actually gets, in the vocabulary #13657 landed for the
268+
// `ctx.input` sibling one branch over — one door, one phrasing.
269+
expect(finding.message).toContain('INVALID_FIELD / 400');
270+
expect(finding.message).toContain('identically on every driver');
271+
expect(finding.message).toContain('before any statement is built');
272+
// Why it is refused there rather than by a driver: the payload is a
273+
// CALLER's, which is the fact the whole rewrite turns on.
274+
expect(finding.message).toContain('ordinary CALLER write');
275+
// ...and the blast radius that makes an author-time rule worth having.
276+
expect(finding.message).toContain('fails the operation that triggered the hook');
277+
278+
// The retired claim, in both halves. Neither may come back without a
279+
// measurement saying it should.
280+
expect(finding.message).not.toMatch(/driver-level error/);
281+
expect(finding.message).not.toMatch(/schemaless/);
282+
expect(finding.message).not.toMatch(/is persisted/);
283+
expect(finding.message).not.toMatch(/write-path validator skips/);
284+
});
285+
249286
it('checks updateById payloads at argument 1, not 0', () => {
250287
const findings = validateHookBodyWrites(
251288
stackWith("await ctx.api.object('crm_deal').updateById(ctx.input.id, { stag: 'won' });"),

0 commit comments

Comments
 (0)