fix(objectql)!: a write's returned row and its prior read serve the declared fields, never an orphaned column

.changeset/21613-write-result-declared-fields.md

@@ -0,0 +1,39 @@
+---
+'@objectstack/objectql': minor
+---
+
+fix(objectql)!: the record a write returns, and the prior read it binds as `previous`, serve the object's declared fields and the platform's system columns, never a column no metadata declares (#21613)
+
+**BREAKING (narrowing)** — what a released write door returns shrinks. A column that
+no metadata declares, typically a field retired in an upgrade whose column additive
+schema sync leaves in the table until `os migrate apply --allow-destructive`, is no
+longer returned by any write through the engine. This completes the read-side rule of
+the previous release (#21571) for writes.
+
+| write | before | now |
+| --- | --- | --- |
+| `PATCH /api/v1/data/:object/:id`, batch `update`, `updateMany` | the post-write row read back with `select *`: retired columns with their stored values | the declared fields, the registry's system columns, `id`, `created_at`, `updated_at` |
+| `POST /api/v1/data/:object`, `POST /api/v1/data/:object/:id/clone`, `createMany`, batch `create` / `upsert`, `insertMany` outcomes | the inserted row from `returning('*')`: retired columns as `null` | the same declared set |
+| `data.record.created` / `data.record.updated` events, and the webhook deliveries that carry them as `after` | the same whole row | the declared set |
+| `engine.insert` / `engine.update` in process (actions, flows, plugins), and a hook's `ctx.result` | the whole row | the declared set |
+| a hook's `ctx.previous` on update and delete (by id and per row), and the audit ledger's delete `old_value` and create `new_value` | the whole stored row, retired columns and their values included | the declared set |
+
+**Unchanged:** declared fields keep their treatment. An `internal: true` field is still
+returned whole on the engine-level write result to the privileged writer that just
+wrote it, and still stripped from every data-door response; formulas are still
+hydrated onto the result; the registry-injected tenant, owner and audit columns are
+still returned. No driver changed: the engine shapes the rows any driver returns, so
+the answer is the same on every driver and every door. A retired column's values stay
+in the table.
+
+**If you still read a retired column's values off a write** (for example a hook, a
+flow or a webhook receiver that copied an old column into its replacement): run that
+conversion BEFORE upgrading to this release, while the old field is still declared, or,
+once it lands, read the unmapped columns through the operator-only `os migrate` read
+(objectstack#21573). There is no flag that re-opens undeclared columns on a write. A
+reader that needs a column must declare it as a field; a validation rule, a
+`readonlyWhen` or a hook condition that reads a column must name a declared field too.
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) No authorable key, spelling, export, config field or stored shape is removed or renamed, and no stored row is read or rewritten: the change narrows which physical columns the engine's write verbs and their prior reads return, to the field map the metadata already declares. A retired column's values stay in the table; what an app does with them is an operator action stated above (convert before upgrading, or the operator-only os migrate read), not a FROM to TO mapping that objectstack migrate meta could apply. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers a write result's projection (not registered / already-registered); and the change is runtime behaviour, not a declaration (not runtime-interface-only / type-surface-only). -->

packages/objectql/src/engine.test.ts

@@ -1406,6 +1406,10 @@ describe('ObjectQL Engine', () => {
                 name: 'invoice',
                 fields: {
                     amount: { type: 'number', readonlyWhen: 'record.locked == true' },
+                    // [#21613] Declared: the rules below read them, and a
+                    // write's prior read now carries the declared fields only.
+                    locked: { type: 'boolean' },
+                    limit: { type: 'number' },
                 },
                 validations: [{ type: 'cross_field', name: 'amount_cap', message: 'amount exceeds limit', condition: 'record.amount > record.limit', fields: ['amount'] }],
             } as any);
@@ -1466,7 +1470,9 @@ describe('ObjectQL Engine', () => {
         it('reports a readonlyWhen-locked field on a single-id update (reason readonly_when)', async () => {
             vi.mocked(SchemaRegistry.getObject).mockReturnValue({
                 name: 'invoice',
-                fields: { amount: { type: 'number', readonlyWhen: 'record.locked == true' } },
+                // [#21613] `locked` is declared: the prior read the rule
+                // reads carries the declared fields only.
+                fields: { amount: { type: 'number', readonlyWhen: 'record.locked == true' }, locked: { type: 'boolean' } },
             } as any);
             vi.mocked(mockDriver.findOne).mockResolvedValue({ id: '1', locked: true, amount: 100 } as any);
 
@@ -1483,7 +1489,9 @@ describe('ObjectQL Engine', () => {
         it('reports bulk-path strips too (multi update — locked in ≥1 matched row)', async () => {
             vi.mocked(SchemaRegistry.getObject).mockReturnValue({
                 name: 'invoice',
-                fields: { amount: { type: 'number', readonlyWhen: 'record.locked == true' } },
+                // [#21613] `locked` is declared: the prior read the rule
+                // reads carries the declared fields only.
+                fields: { amount: { type: 'number', readonlyWhen: 'record.locked == true' }, locked: { type: 'boolean' } },
             } as any);
             vi.mocked(mockDriver.find).mockResolvedValue([
                 { id: 'a', locked: false, amount: 10 },

packages/objectql/src/engine.ts

@@ -13628,6 +13628,25 @@ export class ObjectQL implements IObjectQLEngine {
           );
         }
 
+        // [#21613] A write's returned row is a served row, so it carries the
+        // read verbs' default projection: the DECLARED field set
+        // (`declared-read-columns.ts`). A table column no metadata declares —
+        // a field retired in an upgrade, whose column additive sync leaves
+        // behind — rides back in `returning('*')` on driver-sql, and on a new
+        // row it reads null. Shaped HERE, on the rows as the driver returned
+        // them and after the one-row-per-input guard, so the formulas, the
+        // `afterInsert` hooks, the roll-up, the `data.record.created` events
+        // (and the webhooks that deliver them), the partial-mode outcomes and
+        // the door's own ingress strip all see the declared record — one
+        // decision for every door, never a per-door strip. Declared fields
+        // keep their treatment: `internal: true` fields stay whole on this
+        // engine-level result (the A-prime ruling, below), because an
+        // undeclared column is no field at all.
+        const declaredInsertColumns = declaredColumnSet(schemaForValidation);
+        result = isBatch
+          ? rowsWithDeclaredColumnsOnly(result as unknown[], declaredInsertColumns)
+          : withDeclaredColumnsOnly(result, declaredInsertColumns);
+
         // Coerce `boolean` fields (SQLite/libsql return 0/1) to real booleans on
         // the after-hook view so flow trigger conditions (`record.is_escalated
         // != true`) and `{record.<bool>}` interpolation see JS booleans, not
@@ -14433,7 +14452,16 @@ export class ObjectQL implements IObjectQLEngine {
            // the same for the same reason.
            const priorAst: QueryAST = { object, where: { id }, limit: 1 };
            const preOpts = this.buildDriverOptions(object, opCtx.context, hookContext.input.options as any);
-           priorRecord = await driver.findOne(object, priorAst, preOpts);
+           // [#21613] The pre-image is shaped like every other row this engine
+           // reads off a driver: the declared field set, so `previous` (bound
+           // for the hooks below, and the audit ledger's side of every update
+           // diff) and the write's returned row are ONE view. Shaping only the
+           // returned row would make every update of a row carrying a retired
+           // column record that column as changed, its stored value included.
+           priorRecord = withDeclaredColumnsOnly(
+             await driver.findOne(object, priorAst, preOpts) as Record<string, unknown> | null,
+             declaredColumnSet(updateSchema),
+           );
            // ── [#7867] The not-found gate ──────────────────────────────────
            //
            // A by-id update whose id names no row was a SILENT NO-OP that
@@ -14556,7 +14584,13 @@ export class ObjectQL implements IObjectQLEngine {
            readPriorRows = async () => {
                if (!priorRowsRead) {
                    priorRowsRead = true;
-                   priorRows = (await driver.find(object, ast, preOpts) as Record<string, unknown>[]) ?? [];
+                   // [#21613] The matched rows' pre-images, shaped like the
+                   // by-id prior read: each per-row `afterUpdate` composes its
+                   // `result` from one of them, so both sides stay declared.
+                   priorRows = rowsWithDeclaredColumnsOnly(
+                     (await driver.find(object, ast, preOpts) as Record<string, unknown>[]) ?? [],
+                     declaredColumnSet(updateSchema),
+                   );
                }
                return priorRows;
            };
@@ -15082,6 +15116,15 @@ export class ObjectQL implements IObjectQLEngine {
                } catch (driverError) {
                    throw envelopeUniqueViolation(driverError, object);
                }
+               // [#21613] The by-id update returns the driver's post-write
+               // readback (`select *` on driver-sql), so a column no metadata
+               // declares rode back in the PATCH 200 `record` with its stored
+               // value. Same default projection as the read verbs and the
+               // insert result, applied on the row as the driver returned it:
+               // before the formulas, the `afterUpdate` hooks, the roll-up, the
+               // `data.record.updated` event and the door's ingress strip. A
+               // `null` readback (the row left the caller's scope) stays `null`.
+               result = withDeclaredColumnsOnly(result, declaredColumnSet(updateSchema));
            } else {
                // [#6262] A bulk SET clause must not carry `id`. Reaching this
                // branch AT ALL means `resolveEngineUpdateDispatch` returned
@@ -16735,10 +16778,18 @@ export class ObjectQL implements IObjectQLEngine {
       // tenant scope onto a raw driver read. Skipping it here would read
       // outside this write's transaction and across the tenant boundary —
       // `update()`'s prior read passes the same bag for the same reason.
+      // [#21613] Both pre-image reads below are shaped to the declared field
+      // set, as `update()`'s are: the pre-image is the delete's `previous`, and
+      // the audit ledger records it whole as the delete's `old_value`, so a
+      // column no metadata declares reached that served row with its value.
+      const declaredDeleteColumns = declaredColumnSet(deleteSchema);
       const readPreImage = async (targetId: unknown): Promise<Record<string, unknown> | null> => {
         const preAst: QueryAST = { object, where: { id: targetId }, limit: 1 };
         const preOpts = this.buildDriverOptions(object, opCtx.context, hookContext.input.options as any);
-        return (await driver.findOne(object, preAst, preOpts)) as Record<string, unknown> | null;
+        return withDeclaredColumnsOnly(
+          (await driver.findOne(object, preAst, preOpts)) as Record<string, unknown> | null,
+          declaredDeleteColumns,
+        );
       };
       const bindPreImage = (row: Record<string, unknown> | null): void => {
         // Never fabricate: a row that is not there leaves `previous` UNBOUND
@@ -16878,7 +16929,11 @@ export class ObjectQL implements IObjectQLEngine {
         const perRowAfterHooks = this.hasHooksFor('afterDelete', object);
         if (perRowBeforeHooks || perRowAfterHooks) {
           const preOpts = this.buildDriverOptions(object, opCtx.context, hookContext.input.options as any);
-          const doomed = (await driver.find(object, ast, preOpts) as Record<string, unknown>[]) ?? [];
+          // [#21613] Each doomed row is its per-row `previous`: declared set.
+          const doomed = rowsWithDeclaredColumnsOnly(
+            (await driver.find(object, ast, preOpts) as Record<string, unknown>[]) ?? [],
+            declaredDeleteColumns,
+          );
           // [D6] One ceiling, both phases, BEFORE the first per-row dispatch
           // and before the driver call.
           this.assertBulkPerRowHookBudget(

packages/objectql/src/plugin.integration.test.ts

@@ -1725,6 +1725,9 @@ describe('ObjectQLPlugin - Metadata Service Integration', () => {
           fields: {
             status: { name: 'status', label: 'Status', type: 'text' },
             amount: { name: 'amount', label: 'Amount', type: 'number', readonlyWhen: "record.status == 'paid'" } as any,
+            // [#21613] Declared: the rule reads it off the prior read, which
+            // carries the declared fields only.
+            limit: { name: 'limit', label: 'Limit', type: 'number' },
           },
           validations: [{ type: 'cross_field', name: 'amount_cap', message: 'amount exceeds limit', condition: 'record.amount > record.limit', fields: ['amount'] }] as any,
         },