From e46c976db4598ae364733f815f2ffded2da1a7d2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 15:30:04 +0000 Subject: [PATCH] fix(lint): state the measured refusal in the three write-set rule messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WLJQhde67SeTccsmnBVarV --- .../write-set-messages-drop-driver-split.md | 53 ++++++++++++++++ .../src/validate-action-body-writes.test.ts | 29 +++++++++ .../lint/src/validate-action-body-writes.ts | 30 ++++++---- .../src/validate-flow-node-writes.test.ts | 35 +++++++++++ .../lint/src/validate-flow-node-writes.ts | 60 +++++++++++-------- .../src/validate-hook-body-writes.test.ts | 37 ++++++++++++ .../lint/src/validate-hook-body-writes.ts | 25 +++++++- 7 files changed, 231 insertions(+), 38 deletions(-) create mode 100644 .changeset/write-set-messages-drop-driver-split.md diff --git a/.changeset/write-set-messages-drop-driver-split.md b/.changeset/write-set-messages-drop-driver-split.md new file mode 100644 index 0000000000..d91e0d7d90 --- /dev/null +++ b/.changeset/write-set-messages-drop-driver-split.md @@ -0,0 +1,53 @@ +--- +"@objectstack/lint": patch +--- + +fix(lint): the three write-set rule messages now state the refusal authors actually get, not a retired driver split (#13858) + +Message text only. Rule ids, severities, match sets and hints are untouched, and +no finding changes shape — but a lint's own header states why the prose is +governed: *"a lint that misdescribes the failure it is warning about teaches the +wrong debugging instinct"*. These three sentences did. + +`validate-hook-body-writes` (the `ctx.api` branch), `validate-action-body-writes` +and `validate-flow-node-writes` all told the author that an undeclared write has +a **driver-dependent** outcome: + +> 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 + +For the paths those three rules judge, that has not been true since the +declared-field door landed (#8682 insert, #8738 update). All three describe a +write whose payload is **caller-supplied**, not a mutation of an in-flight +`ctx.input`: `ctx.api` is a `ScopedContext` over the running engine, and a flow +node hands its `fields` map to the data engine directly. The door refuses a +caller-named undeclared key from the object's field map **before any statement is +built**, so no driver is reached and there is no split to observe. + +Measured before the prose was rewritten — all three paths, both driver families, +through a real QuickJS sandbox, a real `ObjectQL` engine, the real +`AutomationEngine` with the real builtin CRUD node executors, real +`@objectstack/driver-sql` (better-sqlite3) and real `@objectstack/driver-memory`: + +| path | driver-sql | driver-memory | +|---|---|---| +| hook body `ctx.api.object(x).update({…})` | `INVALID_FIELD` / 400 | `INVALID_FIELD` / 400 | +| action body `ctx.api.object(x).update({…})` | `INVALID_FIELD` / 400 | `INVALID_FIELD` / 400 | +| flow `create_record` / `update_record` `fields` | `INVALID_FIELD` / 400 | `INVALID_FIELD` / 400 | + +Every run answered `Unknown field 'stagee' on object 'deal'`; nothing was stored +on either family, and the schemaless family kept **no** shadow column — the half +the old message promised and the runtime no longer delivers. + +The three messages now name that refusal in the vocabulary the `ctx.input` +sibling landed with (`REFUSED at run time — INVALID_FIELD / 400, identically on +every driver`), say why the door and not a driver answers, and keep each path's +own blast radius: the hook refusal fails the operation that triggered the hook, +the action refusal fails the action, and the flow node's refusal is whole — the +correctly named fields in the same payload never land either, `create_record` +never creates the row, and the step fails the run. That last clause is why the +flow rule still gates at `error`; the severity is unchanged. + +`unprovisionedAnchorWriteConsequence()` in the same files 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. That message was already correct. diff --git a/packages/lint/src/validate-action-body-writes.test.ts b/packages/lint/src/validate-action-body-writes.test.ts index b72dd906ed..9c3235307e 100644 --- a/packages/lint/src/validate-action-body-writes.test.ts +++ b/packages/lint/src/validate-action-body-writes.test.ts @@ -167,6 +167,35 @@ describe('validateActionBodyWrites — ctx.api writes', () => { expect(findings[0].hint).toContain("'discount_total'"); }); + // [#13858] The same rewrite as the hook sibling, from the same measurement: + // real QuickJS sandbox, a real L2 ACTION body run through + // `actionBodyRunnerFactory`, a real ObjectQL engine, real driver-sql + // (better-sqlite3) AND real driver-memory. Both families answered + // `INVALID_FIELD` / 400, "Unknown field 'stagee' on object 'deal'"; the + // target row was untouched and the memory family stored no shadow column. + // The old text promised a driver-level error on SQL and a persisted stray + // key on schemaless — neither happens on this path, and has not since + // #8682/#8738 put the declared-field door ahead of any statement. + it('states the measured refusal — INVALID_FIELD / 400 on every driver — and no driver split', () => { + const [finding] = validateActionBodyWrites( + stackWith("await ctx.api.object('crm_deal').update({ discont_total: 0 });"), + ); + + expect(finding.message).toContain('INVALID_FIELD / 400'); + expect(finding.message).toContain('identically on every driver'); + expect(finding.message).toContain('before any statement is built'); + // The reason the door — not a driver — is what answers. + expect(finding.message).toContain('ordinary CALLER write'); + // The action-side blast radius, the one word that differs from the hook + // sibling's sentence. Pinned so a future sweep cannot flatten the two. + expect(finding.message).toContain('fails the action'); + + expect(finding.message).not.toMatch(/driver-level error/); + expect(finding.message).not.toMatch(/schemaless/); + expect(finding.message).not.toMatch(/is persisted/); + expect(finding.message).not.toMatch(/write-path validator skips/); + }); + it('checks insert/create/update payloads (argument 0) and updateById at argument 1', () => { const findings = validateActionBodyWrites( stackWith( diff --git a/packages/lint/src/validate-action-body-writes.ts b/packages/lint/src/validate-action-body-writes.ts index bc372c3ae9..fad927f288 100644 --- a/packages/lint/src/validate-action-body-writes.ts +++ b/packages/lint/src/validate-action-body-writes.ts @@ -7,13 +7,18 @@ // `HookBodySchema` union, parsed by the same `HookBodySchema.safeParse` in // `actionBodyRunnerFactory` (packages/runtime/src/sandbox/body-runner.ts), run // in the same QuickJS sandbox. So it fails the same way — an action body that -// writes a field the target object never declares reaches the driver -// unfiltered, and the outcome is DRIVER-DEPENDENT: on SQL the stray column -// fails the whole call with a driver-level error far from the authoring -// mistake, on a schemaless driver the stray key is persisted. Same #4271 -// split as the hook side (see that file's header for the measured chain, and -// `undeclared-field-write-driver-split.integration.test.ts` for the pin); the -// hook rule alone left half the surface uncovered. +// writes a field the target object never declares is refused at run time, far +// from the authoring mistake. [#13858] That refusal is NOT driver-dependent, +// and the message says so: this rule judges exactly one shape, +// `ctx.api.object('').insert|create|update|updateById(…)`, and +// `ctx.api` is a ScopedContext over the running engine, so the payload is +// CALLER-supplied and the declared-field door (#8682 insert, #8738 update) +// refuses it — `INVALID_FIELD` / 400, identically on driver-sql and +// driver-memory, before any statement is built. Measured on both families +// through the real sandbox and the real engine; the caller-payload half of +// that door is pinned in +// `undeclared-field-write-driver-split.integration.test.ts`. The hook rule +// alone left half the surface uncovered, which is why this file exists. // // ─── What does NOT carry over ─────────────────────────────────────────────── // @@ -429,9 +434,14 @@ export function validateActionBodyWrites(stack: AnyRec): ActionBodyWriteFinding[ path: site.path, message: `body calls ctx.api.object('${w.object}').${w.method ?? 'update'}(…) writing '${w.field}', but ` + - `object '${w.object}' declares no such field. The write-path validator skips the unknown key — ` + - `on a SQL driver the whole action then fails with a driver-level error far from here; on a ` + - `schemaless driver (memory, MongoDB) the stray key is persisted (#4271).`, + // [#13858] Same door, same measurement as the hook sibling — ctx.api + // is a ScopedContext over the running engine, so this payload is + // CALLER-supplied and #8682/#8738 refuse it before any driver. + `object '${w.object}' declares no such field. ctx.api is a scoped handle on the running ` + + `engine, so the payload arrives as an ordinary CALLER write and the declared-field door ` + + `REFUSES it at run time — INVALID_FIELD / 400, identically on every driver (#4271), before ` + + `any statement is built. The write lands nothing, and the refusal escapes the body and ` + + `fails the action.`, hint: fixHint(w.field, [...known]), }); } diff --git a/packages/lint/src/validate-flow-node-writes.test.ts b/packages/lint/src/validate-flow-node-writes.test.ts index 81f3c1e094..049ed95eb8 100644 --- a/packages/lint/src/validate-flow-node-writes.test.ts +++ b/packages/lint/src/validate-flow-node-writes.test.ts @@ -181,6 +181,41 @@ describe('validateFlowNodeWrites', () => { expect(findings[0].hint).toMatch(/Did you mean (one of: )?'stage'/); }); + // [#13858] This rule GATES (severity `error`), so its message is what an + // author reads while their build is refused — the one place a wrong causal + // story costs the most. It used to say "on a SQL datasource the driver + // rejects the whole statement ('no such column') … on a schemaless one the + // stray key is persisted". Measured through the real AutomationEngine, the + // real builtin CRUD nodes, a real ObjectQL engine and BOTH families + // (driver-sql on better-sqlite3, driver-memory): neither happens. Both + // answered `INVALID_FIELD` / 400, "Unknown field 'stagee' on object 'deal'", + // the node folded that into `create_record(deal) failed: …`, the run failed, + // and nothing was stored on either family — no row on create, an untouched + // row and no shadow column on update. + it('states the measured refusal — INVALID_FIELD / 400 on every datasource — and no driver split', () => { + const [finding] = validateFlowNodeWrites({ + objects: [dealObject], + flows: [flowWith({ stagee: 'won' })], + }); + + expect(finding.message).toContain('INVALID_FIELD / 400'); + expect(finding.message).toContain('identically on every datasource'); + expect(finding.message).toContain('before any statement is built'); + // Why the door answers and not a datasource: the node hands `fields` + // straight to the data engine, so it is a caller payload. + expect(finding.message).toContain('ordinary caller payload'); + // The severity's own justification, unchanged by the rewrite and still + // stated: the refusal is WHOLE, so correctly named siblings are lost too. + expect(finding.message).toContain('never land either'); + expect(finding.message).toContain('the step fails the run'); + + // The retired driver split, both halves. + expect(finding.message).not.toMatch(/no such column/); + expect(finding.message).not.toMatch(/schemaless/); + expect(finding.message).not.toMatch(/is persisted/); + expect(finding.message).not.toMatch(/Nothing between the node and storage/); + }); + it('flags every unknown key in one node, and only those', () => { const findings = validateFlowNodeWrites({ objects: [dealObject], diff --git a/packages/lint/src/validate-flow-node-writes.ts b/packages/lint/src/validate-flow-node-writes.ts index 7a52736901..fae867443a 100644 --- a/packages/lint/src/validate-flow-node-writes.ts +++ b/packages/lint/src/validate-flow-node-writes.ts @@ -26,27 +26,31 @@ // // And the runtime consequence is not the benign "consumer skips the unknown // name and does the rest" that keeps `page-field-unknown` / `form-field-unknown` -// advisory. Nothing between the node and storage removes the key: the flow -// executor calls the data engine directly (bypassing the metadata-protocol -// ingress, which strips `readonly` — not unknown — keys anyway), the engine's -// write paths strip only readonly/readonlyWhen, and the SQL driver's -// `formatInput` / `applyWriteColumnMap` pass an unrecognized key straight -// through (`m[k] ?? k`). Every branch below was measured, not inferred: +// advisory. The flow executor calls the data engine directly (`data.insert` / +// `data.update` in service-automation's `builtin/crud-nodes.ts`, bypassing the +// metadata-protocol ingress), so the node's `fields` map arrives as an ordinary +// CALLER payload — and [#13858] the declared-field door (#8682 insert, #8738 +// update) refuses a caller-named undeclared key from the object's field map +// before any statement is built. Every branch below was measured through the +// real AutomationEngine, the real builtin CRUD nodes, the real engine and BOTH +// driver families (driver-sql on better-sqlite3, driver-memory), not inferred: // -// • Through the engine, an undeclared key reaches `driver.update` / -// `driver.create` verbatim, alongside the audit stamps. -// • On SQLite/knex an UPDATE becomes `update "deal" set "name" = 'n2', -// "stagee" = 'won' … → no such column: stagee`. The statement is rejected -// WHOLE: `name` — spelled correctly, in the same payload — does not land -// either, and the step fails with a driver error naming a column, far from -// the authoring mistake. -// • An INSERT fails the same way (`table deal has no column named stagee`), -// and one notch harder: the row is never created at all, so every later -// node that expected `{.id}` is working from a record that does not -// exist. -// • On a schemaless datasource (memory, MongoDB) nothing rejects it, so the -// stray key is persisted into a column the object never declares — where no -// schema-driven read surface will return it. +// • Both families answer identically — `INVALID_FIELD` / 400, "Unknown field +// 'stagee' on object 'deal'". No driver is reached, so there is no split to +// observe. +// • The write is refused WHOLE: `name` — spelled correctly, in the same +// payload — does not land either. +// • On `create_record` the row is never created at all, so every later node +// that expected `{.id}` is working from a record that does not exist. +// • The node catches the refusal and folds it into a step failure +// (`create_record(deal) failed: Unknown field 'stagee' on object 'deal'`), +// so the RUN fails — far from the authoring mistake, which is exactly why +// an author-time rule is still worth having. +// +// ⚠️ Until #13858 this block described the pre-#8682 driver split (SQL rejected +// the statement, a schemaless datasource persisted the stray key). That is +// retired, not merely restated: the severity below is unchanged because neither +// the old outcome nor the new one is ever "the rest still works". // // No outcome is "the rest still works". That is the same call // `validate-searchable-fields` makes for a stale entry and @@ -290,12 +294,18 @@ export function validateFlowNodeWrites(stack: AnyRec): FlowNodeWriteFinding[] { where: `flow "${flowName}" › ${nodeWhere}`, path: `${nodePath}.config.fields.${fieldName}`, message: - `${node.type} writes '${fieldName}', but object '${objectName}' declares no such field. Nothing ` + - `between the node and storage removes the key: on a SQL datasource the driver rejects the whole ` + - `statement ('no such column'), so the correctly named fields in this same payload never land ` + - `either${ + // [#13858] The node hands `fields` to the data engine directly + // (`data.insert` / `data.update` in service-automation's + // crud-nodes), so it is a CALLER payload and the #8682/#8738 + // declared-field door refuses it before any datasource is reached. + // Measured on driver-sql and driver-memory alike. + `${node.type} writes '${fieldName}', but object '${objectName}' declares no such field. The ` + + `node hands its fields map to the engine as an ordinary caller payload, so the ` + + `declared-field door REFUSES the whole write — INVALID_FIELD / 400, identically on every ` + + `datasource, before any statement is built. The correctly named fields in this same payload ` + + `never land either${ node.type === 'create_record' ? ' and the record is never created at all' : '' - }; on a schemaless one the stray key is persisted into a column no read surface returns.`, + }, and the step fails the run.`, hint: fixHint(fieldName, [...known]), }); } diff --git a/packages/lint/src/validate-hook-body-writes.test.ts b/packages/lint/src/validate-hook-body-writes.test.ts index 6cea10bd03..eabc442142 100644 --- a/packages/lint/src/validate-hook-body-writes.test.ts +++ b/packages/lint/src/validate-hook-body-writes.test.ts @@ -246,6 +246,43 @@ describe('validateHookBodyWrites — ctx.api writes', () => { expect(findings[0].hint).toContain("'email'"); }); + // [#13858] The message is the whole product of an advisory rule, so the + // sentence IS the deliverable. It used to promise a driver-dependent outcome + // ("on a SQL driver … a driver-level error; on a schemaless driver … the + // stray key is persisted"), which has not been true for this path since + // #8682/#8738: `ctx.api` is a ScopedContext over the running engine, so the + // payload is CALLER-supplied and the declared-field door refuses it first. + // + // Measured before this text was written — real QuickJS sandbox, real hook + // body, real ObjectQL, real driver-sql (better-sqlite3) AND real + // driver-memory: both families answered `INVALID_FIELD` / 400, "Unknown field + // 'stagee' on object 'deal'", the target row was untouched, and the memory + // family stored no shadow column. Same door the caller-payload half of + // `undeclared-field-write-driver-split.integration.test.ts` pins. + it('states the measured refusal — INVALID_FIELD / 400 on every driver — and no driver split', () => { + const [finding] = validateHookBodyWrites( + stackWith("await ctx.api.object('crm_deal').update({ id, stag: 'won' });"), + ); + + // What the author actually gets, in the vocabulary #13657 landed for the + // `ctx.input` sibling one branch over — one door, one phrasing. + expect(finding.message).toContain('INVALID_FIELD / 400'); + expect(finding.message).toContain('identically on every driver'); + expect(finding.message).toContain('before any statement is built'); + // Why it is refused there rather than by a driver: the payload is a + // CALLER's, which is the fact the whole rewrite turns on. + expect(finding.message).toContain('ordinary CALLER write'); + // ...and the blast radius that makes an author-time rule worth having. + expect(finding.message).toContain('fails the operation that triggered the hook'); + + // The retired claim, in both halves. Neither may come back without a + // measurement saying it should. + expect(finding.message).not.toMatch(/driver-level error/); + expect(finding.message).not.toMatch(/schemaless/); + expect(finding.message).not.toMatch(/is persisted/); + expect(finding.message).not.toMatch(/write-path validator skips/); + }); + it('checks updateById payloads at argument 1, not 0', () => { const findings = validateHookBodyWrites( stackWith("await ctx.api.object('crm_deal').updateById(ctx.input.id, { stag: 'won' });"), diff --git a/packages/lint/src/validate-hook-body-writes.ts b/packages/lint/src/validate-hook-body-writes.ts index 5f2fcd5762..e5cc5a839c 100644 --- a/packages/lint/src/validate-hook-body-writes.ts +++ b/packages/lint/src/validate-hook-body-writes.ts @@ -18,6 +18,17 @@ // is refused `INVALID_FIELD` / 400 identically on every driver, before any // statement is built. // +// [#13858] The `ctx.api` example in that first sentence answers for a +// DIFFERENT reason, and it always did since #8682/#8738: `ctx.api` is a +// ScopedContext over the running engine (`ObjectQL.buildHookApi`), so its +// payload is CALLER-supplied and the PRE-hook declared-field door refuses it — +// the same `INVALID_FIELD` / 400, before the hooks and before any statement. +// `applyMutationsToInput` above describes the `ctx.input` half only. Measured +// for all three write-set rules (this one, `validate-action-body-writes.ts`, +// `validate-flow-node-writes.ts`) through a real QuickJS sandbox, a real +// engine and both driver families: neither half reaches a driver any more, so +// no message in this family describes a driver split. +// // ⚠️ That does NOT retire this rule — it changes what it is worth. The runtime // refusal arrives at WRITE time, on whichever record first exercises the // branch; this rule arrives at AUTHOR time and names the field, the object and @@ -863,9 +874,17 @@ export function validateHookBodyWrites(stack: AnyRec): HookBodyWriteFinding[] { path, message: `body calls ctx.api.object('${w.object}').${w.method ?? 'update'}(…) writing '${w.field}', but ` + - `object '${w.object}' declares no such field. The write-path validator skips the unknown key — ` + - `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 (#4271).`, + // [#13858] ctx.api is a ScopedContext over the running engine + // (`ObjectQL.buildHookApi`), so this payload is CALLER-supplied and + // the declared-field door (#8682 insert, #8738 update) is what + // refuses it. Measured on both families; the ids stay in comments + // rather than in the string, which reaches authors and operators + // who cannot resolve a tracker number. + `object '${w.object}' declares no such field. ctx.api is a scoped handle on the running ` + + `engine, so the payload arrives as an ordinary CALLER write and the declared-field door ` + + `REFUSES it at run time — INVALID_FIELD / 400, identically on every driver (#4271), before ` + + `any statement is built. The nested write lands nothing, and the refusal escapes the body ` + + `and fails the operation that triggered the hook.`, hint: fixHint(w.field, [...known]), }); }