feat(lint): flow-node-write-unknown-field 覆盖 create_record(#4271)

.changeset/flow-create-record-write-lint.md

@@ -0,0 +1,49 @@
+---
+"@objectstack/lint": patch
+---
+
+feat(lint): `flow-node-write-unknown-field` covers `create_record` too (#4271)
+
+#4369 shipped the flow write-set gate on `update_record` alone and parked
+`create_record` in `FLOW_WRITE_NODE_TYPES_DEFERRED` with its reason — a gating
+rule earning its severity one measured surface at a time, recorded as data
+rather than left as silence. This measures the other half and moves it across.
+
+**The INSERT path fails the same way, one notch harder.** Same literal
+`config.fields` map, same `objectName` binding, same journey to the driver — the
+engine hands an undeclared key to `driver.create` verbatim, alongside the audit
+stamps. On SQLite/knex it becomes `table deal has no column named stagee` and
+the statement is rejected whole, so the correctly named fields in the same
+payload never land either. The extra harm is what does *not* exist afterwards:
+the row is never created, so every later node reading `{<node>.id}` from that
+node's `outputVariable` is working from a record that was never written. An
+`update_record` failure at least leaves the record intact.
+
+So the message now names that consequence on `create_record` and only there —
+"…and the record is never created at all" — instead of one sentence blurred to
+fit both.
+
+Nothing else moves: same rule id, same `error` severity, the same silent bails
+(templated `objectName`, non-literal `fields`, cross-package objects, objects
+declaring no fields, dotted keys), and `runAs` is still not consulted. Each skip
+is now pinned on the create surface as well as the update one, so the two node
+types cannot drift into different behaviour.
+
+**`FLOW_WRITE_NODE_TYPES_DEFERRED` is now empty and deliberately kept.** The
+partition test derives the full `fields`-write-map set behaviourally from the
+spec's executor-written config schemas, so a node type that grows one later
+belongs to neither list and fails that test until someone classifies it.
+Deleting the empty array would turn that forced decision back into a default.
+
+Two non-members are now excluded on the shape of their failure rather than by
+omission, both stated in the module header and one pinned by a test:
+`get_record.fields` is a projection (`z.array(z.string())`) — a READ, where an
+unknown entry narrows the selection instead of breaking the statement — and
+`screen.defaults` is forwarded into the `ScreenSpec` the client renders, so an
+unknown key is a prefill the renderer ignores. That inert "skips it and renders
+the rest" case is exactly what this rule's `error` severity is defined against.
+
+Verified against the repo's own apps: app-crm, app-todo and app-showcase all
+still validate clean with `create_record` covered — including crm's
+convert-lead flow, which creates an account and an opportunity before updating
+the lead.

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

@@ -97,13 +97,36 @@ describe('FLOW_WRITE_NODE_TYPES — the covered-node ledger', () => {
     expect(classified).toEqual(withWriteMap);
   });
 
+  // Every write-map node type is covered as of #4371, so the deferred list is
+  // empty and the two tests below are vacuous TODAY. Both are kept live because
+  // they are what makes a FUTURE deferral honest: the partition test above
+  // forces a new node type into one list or the other, and these decide what
+  // the uncovered list is allowed to mean.
+  it('every covered type actually reports — the ledger describes behaviour, not intent', () => {
+    for (const type of FLOW_WRITE_NODE_TYPES) {
+      const findings = validateFlowNodeWrites({
+        objects: [dealObject],
+        flows: [
+          {
+            name: 'f',
+            nodes: [{ id: 'n', type, config: { objectName: 'deal', fields: { stagee: 'won' } } }],
+          },
+        ],
+      });
+      expect(findings.map((f) => f.rule), `covered type '${type}' reports nothing`).toEqual([
+        FLOW_NODE_WRITE_UNKNOWN_FIELD,
+      ]);
+      expect(findings[0].severity).toBe('error');
+    }
+  });
+
   it('gives every deferral a non-empty reason', () => {
     for (const deferral of FLOW_WRITE_NODE_TYPES_DEFERRED) {
       expect(deferral.reason.length, `deferral '${deferral.type}' carries no reason`).toBeGreaterThan(0);
     }
   });
 
-  it('leaves every deferred type unchecked — the ledger describes behaviour, not intent', () => {
+  it('leaves every deferred type unchecked', () => {
     for (const deferral of FLOW_WRITE_NODE_TYPES_DEFERRED) {
       const findings = validateFlowNodeWrites({
         objects: [dealObject],
@@ -117,6 +140,26 @@ describe('FLOW_WRITE_NODE_TYPES — the covered-node ledger', () => {
       expect(findings, `deferred type '${deferral.type}' is being checked`).toEqual([]);
     }
   });
+
+  // A node type NOT in the ledger at all must stay silent — the guard that the
+  // covered set is an allowlist rather than "anything with a fields key".
+  it('ignores a fields-bearing node type outside the ledger', () => {
+    const findings = validateFlowNodeWrites({
+      objects: [dealObject],
+      flows: [
+        {
+          name: 'f',
+          nodes: [
+            // `screen` carries `defaults`/`fields`, but an object-form screen
+            // forwards them to the client renderer — an unknown key there is an
+            // ignored prefill, not a write that reaches storage.
+            { id: 'n', type: 'screen', config: { objectName: 'deal', fields: { stagee: 'won' } } },
+          ],
+        },
+      ],
+    });
+    expect(findings).toEqual([]);
+  });
 });
 
 describe('validateFlowNodeWrites', () => {
@@ -268,6 +311,80 @@ describe('validateFlowNodeWrites', () => {
     );
   });
 
+  // ── create_record — the same map on the INSERT surface (#4371) ───────
+  it('errors when a create_record node writes a field the object never declares', () => {
+    const flow = {
+      name: 'seed_deal',
+      nodes: [
+        { id: 'start', type: 'start', config: {} },
+        {
+          id: 'seed',
+          type: 'create_record',
+          label: 'Seed deal',
+          config: { objectName: 'deal', fields: { name: 'ACME', stagee: 'won' }, outputVariable: 'created' },
+        },
+      ],
+    };
+    const findings = validateFlowNodeWrites({ objects: [dealObject], flows: [flow] });
+    expect(findings).toHaveLength(1);
+    expect(findings[0].severity).toBe('error');
+    expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.stagee');
+    expect(findings[0].where).toBe('flow "seed_deal" › node "Seed deal"');
+    // The INSERT consequence is strictly worse than the UPDATE one and the
+    // message says so: the row never exists, so `{created.id}` downstream is
+    // reading from a record that was never written.
+    expect(findings[0].message).toContain('the record is never created at all');
+  });
+
+  it('names only the UPDATE consequence for an update_record node', () => {
+    const findings = validateFlowNodeWrites({
+      objects: [dealObject],
+      flows: [flowWith({ stagee: 'won' })],
+    });
+    expect(findings[0].message).not.toContain('never created at all');
+  });
+
+  it('takes every skip on create_record too', () => {
+    const createFlow = (config: Record<string, unknown>) => ({
+      name: 'f',
+      nodes: [{ id: 'c', type: 'create_record', config }],
+    });
+    // templated object, non-literal fields, unknown object, fieldless object
+    expect(
+      validateFlowNodeWrites({
+        objects: [dealObject],
+        flows: [createFlow({ objectName: '{target}', fields: { stagee: 1 } })],
+      }),
+    ).toEqual([]);
+    expect(
+      validateFlowNodeWrites({ objects: [dealObject], flows: [createFlow({ objectName: 'deal', fields: '{all}' })] }),
+    ).toEqual([]);
+    expect(
+      validateFlowNodeWrites({ objects: [], flows: [createFlow({ objectName: 'deal', fields: { stagee: 1 } })] }),
+    ).toEqual([]);
+    expect(
+      validateFlowNodeWrites({
+        objects: [{ name: 'deal', external: true }],
+        flows: [createFlow({ objectName: 'deal', fields: { stagee: 1 } })],
+      }),
+    ).toEqual([]);
+  });
+
+  it('does NOT flag a readonly field on create_record — INSERT is engine-exempt from that strip', () => {
+    // The readonly rule skips create_record entirely (a create may legitimately
+    // seed readonly columns). This rule asks a different question, so a DECLARED
+    // readonly field is clean here for its own reason: it resolves to a column.
+    const withReadonly = {
+      name: 'deal',
+      fields: { stage: { type: 'text' }, approval_status: { type: 'text', readonly: true } },
+    };
+    const findings = validateFlowNodeWrites({
+      objects: [withReadonly],
+      flows: [{ name: 'f', nodes: [{ id: 'c', type: 'create_record', config: { objectName: 'deal', fields: { approval_status: 'approved' } } }] }],
+    });
+    expect(findings).toEqual([]);
+  });
+
   // ── the family boundary ──────────────────────────────────────────────
   it('does not duplicate the readonly rule: a declared readonly field is that rule’s business, not this one', () => {
     const withReadonly = {

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

@@ -1,6 +1,6 @@
 // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
 //
-// Author-time write-set check for a flow `update_record` node's `fields` — the
+// Author-time write-set check for a flow CRUD node's `fields` write map — the
 // THIRD surface in the family #4271 opened, and the one the docs spent the
 // longest recommending as the safe alternative to the other two.
 //
@@ -29,35 +29,47 @@
 // 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
-// UPDATE path strips only readonly/readonlyWhen, and the SQL driver's
+// write paths strip only readonly/readonlyWhen, and the SQL driver's
 // `formatInput` / `applyWriteColumnMap` pass an unrecognized key straight
-// through (`m[k] ?? k`). Both halves were measured, not inferred:
+// through (`m[k] ?? k`). Every branch below was measured, not inferred:
 //
-//   • Through the engine, an undeclared key reaches `driver.update` verbatim,
-//     alongside the audit stamps.
-//   • On SQLite/knex it then becomes `update "deal" set "name" = 'n2',
+//   • 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 `{<node>.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.
 //
-// Neither outcome is "the rest still works". That is the same call
+// No outcome is "the rest still works". That is the same call
 // `validate-searchable-fields` makes for a stale entry and
 // `validate-flow-template-paths` makes for a filter-position token: gate when
 // the miss breaks or corrupts the operation, advise when it merely narrows the
 // output. Every skip below exists so that gate only ever fires on a certainty.
 //
 // ─── Scope ──────────────────────────────────────────────────────────────────
 //
-// {@link FLOW_WRITE_NODE_TYPES} — today `update_record` alone — with the
-// deliberate non-member (`create_record`) declared as data in
-// {@link FLOW_WRITE_NODE_TYPES_DEFERRED} rather than left as silence, and the
-// two halves partition-tested against the CRUD node types that carry a `fields`
-// write map. A node type that grows one later fails that test until someone
-// classifies it.
+// {@link FLOW_WRITE_NODE_TYPES} — every CRUD node type that carries a `fields`
+// WRITE map: `update_record` (#4369) and `create_record` (#4371). The deferred
+// half, {@link FLOW_WRITE_NODE_TYPES_DEFERRED}, is now empty, and the partition
+// test still derives the full set behaviourally from the spec's
+// executor-written config schemas — so a node type that grows a write map later
+// lands on neither side and fails that test until someone classifies it.
+//
+// `get_record.fields` is NOT a member and never will be: it is a projection
+// (`z.array(z.string())`), a READ, and an unknown entry there narrows the
+// selection rather than breaking the statement. `screen.defaults` is not one
+// either — an object-form screen forwards it into the `ScreenSpec` the client
+// renders, so an unknown key is a form prefill the renderer ignores: inert, the
+// "skips it and renders the rest" case this rule's severity is defined against.
+// Both are excluded on the shape of their failure, not by omission.
 //
 // `runAs` is deliberately NOT consulted, unlike its readonly sibling. A
 // `runAs:'system'` flow is elevated past the readonly strip, which is why that
@@ -101,7 +113,7 @@ export const FLOW_NODE_WRITE_UNKNOWN_FIELD = 'flow-node-write-unknown-field';
 // a write map later cannot land on the uncovered side by nobody noticing.
 
 /** Flow node types whose `config.fields` keys this rule resolves. */
-export const FLOW_WRITE_NODE_TYPES: readonly string[] = ['update_record'];
+export const FLOW_WRITE_NODE_TYPES: readonly string[] = ['update_record', 'create_record'];
 
 /** A `fields`-bearing node type this rule does NOT cover yet, and why. */
 export interface FlowWriteNodeDeferral {
@@ -112,24 +124,21 @@ export interface FlowWriteNodeDeferral {
 }
 
 /**
- * `fields`-bearing CRUD node types deliberately left out of v1.
+ * `fields`-bearing CRUD node types deliberately left uncovered.
+ *
+ * **Empty, and that is the point.** #4369 shipped `update_record` alone and
+ * parked `create_record` here with its reason — a gating rule earning its
+ * severity one measured surface at a time — rather than leaving the other half
+ * as silence. #4371 measured the INSERT path (`table deal has no column named
+ * stagee`, and the row never created at all), found it strictly worse than the
+ * UPDATE one, and moved it across.
  *
- * `create_record` fails identically — same literal map, same `objectName`
- * binding, same driver fate — and covering it is one entry in
- * {@link FLOW_WRITE_NODE_TYPES} plus its fixtures. It is out of scope here only
- * because the reported gap (#4001 family, the `update_record` surface the docs
- * recommended) is the UPDATE one, and a gating rule earns its severity one
- * measured surface at a time. Recorded rather than omitted so the remaining
- * half is a decision on the record, not a discovery someone makes twice.
+ * The slot stays because the partition test derives the full `fields`-write-map
+ * set from the spec's own config schemas: a node type that grows one later
+ * belongs to neither list and fails that test until someone puts it in one.
+ * Deleting this array would turn that forced decision back into a default.
  */
-export const FLOW_WRITE_NODE_TYPES_DEFERRED: readonly FlowWriteNodeDeferral[] = [
-  {
-    type: 'create_record',
-    reason:
-      "same literal `config.fields` map and same `objectName` binding as update_record, so the check carries " +
-      'over verbatim; deferred only to land the gating severity on one surface first',
-  },
-];
+export const FLOW_WRITE_NODE_TYPES_DEFERRED: readonly FlowWriteNodeDeferral[] = [];
 
 type AnyRec = Record<string, unknown>;
 
@@ -231,7 +240,9 @@ export function validateFlowNodeWrites(stack: AnyRec): FlowNodeWriteFinding[] {
             `${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; on a schemaless one the stray key is persisted into a column no read surface returns.`,
+            `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.`,
           hint: fixHint(fieldName, [...known]),
         });
       }