fix(objectql)!: a number field refuses an array, a boolean or an object with invalid_number (#20309)

.changeset/20309-number-arm-non-string-refused.md

@@ -0,0 +1,57 @@
+---
+"@objectstack/objectql": minor
+---
+
+fix(objectql)!: a number, currency, percent, rating, slider or progress field refuses an array, a boolean or an object with `invalid_number` (#20309)
+
+Clause-②: no (narrowing)
+
+**BREAKING**: shipped as `minor` under the launch-window convention
+(`check-changeset-no-major` refuses `major` until GA; breaking-ness is carried by
+this banner and the ADR-0087 disposition below, never by the level). The
+narrowing: an array, a boolean or an object whose `Number()` is finite, such as
+`[500]`, `[]`, `true` or `false`, written to one of those fields is now refused
+with `400 VALIDATION_FAILED` / `invalid_number`. It used to be accepted and
+stored as sent.
+
+## What was wrong
+
+The record validator judged `Number(value)` on a number-typed field, but the
+write carried `value` itself. Every value that JavaScript coerces to a finite
+number therefore passed the check and reached the driver unchanged:
+
+- **SQLite** stored `[500]` as the TEXT `'[500]'`, which a read returned as the
+  string `"[500]"`; `[]` as the TEXT `'[]'`; and `true` / `false` as `1` / `0`.
+- **memory** stored the array or the boolean itself.
+
+`[5, 7]` and `{}` were already refused, because `Number()` of each is `NaN`.
+
+## What changes
+
+- On `number`, `currency`, `percent`, `rating`, `slider` and `progress`, a value
+  that is neither a number nor a string is refused with `invalid_number`: an
+  array, a boolean, a plain object, a `Date`. This holds on every engine, REST,
+  batch and import write door, because they all write through the same
+  validator.
+- A number is judged and stored exactly as before, and so are the `min`, `max`
+  and `scale` checks and their messages.
+- A string is also unchanged. It is still judged by `Number()` and stored as
+  sent. Which strings a number field accepts is a separate change.
+- `summary` is still not judged by this check (it is in the spec's
+  `COMPUTED_VALUE_TYPES`). A blank still becomes `null` before the check runs.
+
+## Rows already stored
+
+This refuses new writes only; a stored value is never re-read by the check.
+Rows written earlier on SQLite may hold such a value as TEXT in a numeric
+column. To find them, run this once per number-typed column:
+
+```sql
+SELECT id, "FIELD" FROM "OBJECT" WHERE typeof("FIELD") = 'text';
+```
+
+OBJECT is the object name and FIELD is the field name. A match is a cell that
+SQLite could not store as a number: an array written as TEXT, or a string such
+as `'0x10'`. Decide its number by hand; nothing here rewrites it.
+
+<!-- adr-0087: not-required (no-migration-prescription) Nothing authored moves: `packages/spec` is untouched and no metadata key is added, removed or reshaped, so `objectstack migrate meta` has nothing to rewrite and the ledger has no row to gain. What is refused is a caller-written VALUE (an array, a boolean or an object on a number-typed field) at the write door; stored rows are never re-read by the check, and a caller that sends a number, a string or a blank is unaffected. The other categories are closed on facts: the package publishes (not `unpublished`); no ADR-0087 id covers a write-door value check (not `registered` / `already-registered`); and the change is runtime behaviour, not a TypeScript declaration (not `runtime-interface-only` / `type-surface-only`). -->

packages/objectql/src/engine-number-value-door.test.ts

@@ -0,0 +1,157 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #20309 — on every engine write door, an array, boolean or object on a number
+ * field is refused before the driver sees it.
+ *
+ * The arm used to judge `Number(value)` while the write carried `value`, so
+ * `[500]`, `[]` and `true` passed and reached the driver as sent. Measured on
+ * `origin/main` c74de10a94: memory stored each verbatim (the array, the
+ * boolean), and SQLite stored `'[500]'` / `'[]'` as TEXT and `true` as `1`.
+ *
+ * This file pins the DRIVER-FACING half on each door: a refused value never
+ * reaches the driver (no `create`, `bulkCreate`, `update` or `updateMany`
+ * call carries it), and an accepted number arrives as the same number
+ * (`Object.is`), so for a number what the arm judged is what the driver
+ * receives. Memory and MongoDB store exactly this payload. The SQL physical
+ * column is pinned in `packages/rest/src/rest-data-number-value.test.ts`.
+ * A string is not judged differently here: that half waits on #20336.
+ */
+
+import { describe, it, expect, beforeEach } from 'vitest';
+import { COMPUTED_VALUE_TYPES, NUMERIC_VALUE_TYPES } from '@objectstack/spec/data';
+import { ObjectQL } from './engine.js';
+import { ValidationError } from './validation/record-validator.js';
+
+const JUDGED = [...NUMERIC_VALUE_TYPES].filter((t) => !COMPUTED_VALUE_TYPES.has(t));
+const f = (t: string) => `f_${t}`;
+
+/** The card's table and the non-string coercions it named: all refused. */
+const REFUSED: ReadonlyArray<readonly [string, unknown]> = [
+  ['[500]', [500]],
+  ['[5, 7]', [5, 7]],
+  ['[]', []],
+  ['true', true],
+  ['false', false],
+  ['{}', {}],
+];
+
+interface Call { fn: string; rows: Record<string, unknown>[] }
+
+function makeStubDriver() {
+  const calls: Call[] = [];
+  const rows = new Map<string, Record<string, unknown>>();
+  let n = 0;
+  const put = (data: Record<string, unknown>) => {
+    const row = { ...data, id: (data.id as string) ?? `r${++n}` };
+    rows.set(row.id as string, row);
+    return row;
+  };
+  const driver: any = {
+    name: 'stub', version: '0.0.0', supports: {},
+    async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
+    async find() { return [...rows.values()]; },
+    async findOne(_o: string, q: any) {
+      const id = (q?.where ?? q?.filter ?? q)?.id;
+      return (typeof id === 'string' ? rows.get(id) : rows.values().next().value) ?? null;
+    },
+    async count() { return rows.size; },
+    async create(_o: string, data: Record<string, unknown>) {
+      calls.push({ fn: 'create', rows: [{ ...data }] });
+      return put(data);
+    },
+    async bulkCreate(_o: string, list: Record<string, unknown>[]) {
+      calls.push({ fn: 'bulkCreate', rows: list.map((r) => ({ ...r })) });
+      return list.map(put);
+    },
+    async update(_o: string, id: string, data: Record<string, unknown>) {
+      calls.push({ fn: 'update', rows: [{ ...data }] });
+      return put({ ...(rows.get(id) ?? {}), ...data, id });
+    },
+    async updateMany(_o: string, _ast: unknown, data: Record<string, unknown>) {
+      calls.push({ fn: 'updateMany', rows: [{ ...data }] });
+      return rows.size;
+    },
+    async upsert(o: string, data: Record<string, unknown>) { return this.create(o, data); },
+    async delete() { return true; },
+    async bulkUpdate() { return []; }, async bulkDelete() {},
+    async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; },
+    async commit() {}, async rollback() {},
+  };
+  return { driver, calls };
+}
+
+const OBJ = {
+  name: 'num_door',
+  label: 'Number door',
+  fields: {
+    id: { name: 'id', type: 'text' as const, primaryKey: true },
+    ...Object.fromEntries(JUDGED.map((t) => [f(t), { name: f(t), type: t }])),
+  },
+};
+
+async function refusal(fn: () => Promise<unknown>) {
+  try {
+    await fn();
+  } catch (e) {
+    expect(e).toBeInstanceOf(ValidationError);
+    return { code: (e as ValidationError).code, fields: (e as ValidationError).fields.map((x) => [x.field, x.code]) };
+  }
+  return null;
+}
+
+describe('engine write doors: the number arm judges what the driver receives (#20309)', () => {
+  let engine: ObjectQL;
+  let stub: ReturnType<typeof makeStubDriver>;
+
+  beforeEach(async () => {
+    stub = makeStubDriver();
+    engine = new ObjectQL();
+    engine.registerDriver(stub.driver, true);
+    await engine.init();
+    engine.registry.registerObject(OBJ as any);
+  });
+
+  /** Every value any driver write call carried for `field`. */
+  const written = (field: string) =>
+    stub.calls.flatMap((c) => c.rows).filter((r) => field in r).map((r) => r[field]);
+
+  describe.each(JUDGED)('%s', (type) => {
+    it.each(REFUSED)('%s is refused on insert, insert([...]), update by id and update by predicate, and never reaches the driver', async (_l, value) => {
+      await engine.insert('num_door', { id: 'seed', [f(type)]: 1 });
+      stub.calls.length = 0;
+      const expected = { code: 'VALIDATION_FAILED', fields: [[f(type), 'invalid_number']] };
+
+      expect(await refusal(() => engine.insert('num_door', { id: 'a', [f(type)]: structuredClone(value) }))).toEqual(expected);
+      expect(await refusal(() => engine.insert('num_door', [{ id: 'b', [f(type)]: structuredClone(value) }]))).toEqual(expected);
+      expect(await refusal(() => engine.update('num_door', { id: 'seed', [f(type)]: structuredClone(value) }))).toEqual(expected);
+      expect(await refusal(() => engine.update('num_door', { [f(type)]: structuredClone(value) }, { where: { id: { $in: ['seed'] } }, multi: true } as any))).toEqual(expected);
+
+      const outcomes = await engine.insertMany('num_door', [{ id: 'm', [f(type)]: structuredClone(value) }]);
+      expect(outcomes.map((o) => o.ok)).toEqual([false]);
+
+      expect(written(f(type))).toEqual([]);
+    });
+  });
+
+  it('the dry run agrees with the write', async () => {
+    const refused = await engine.validate('num_door', { id: 'p1', f_number: [500] });
+    expect(refused.valid).toBe(false);
+    expect(refused.results?.[0]?.errors.map((e: any) => [e.field, e.code])).toEqual([['f_number', 'invalid_number']]);
+    expect((await engine.validate('num_door', { id: 'p2', f_number: 500 })).valid).toBe(true);
+  });
+
+  it('CONTROL: a finite number reaches the driver as the same number, on insert and update', async () => {
+    const VALID = [500, 12.5, 0, -3];
+    for (const type of JUDGED) {
+      for (const v of VALID) {
+        stub.calls.length = 0;
+        await engine.insert('num_door', { id: `v_${type}_${v}`, [f(type)]: v });
+        await engine.update('num_door', { id: `v_${type}_${v}`, [f(type)]: v });
+        const got = written(f(type));
+        expect(got, `${type} ${v}`).toHaveLength(2);
+        for (const g of got) expect(Object.is(g, v), `${type} ${v}`).toBe(true);
+      }
+    }
+  });
+});

packages/objectql/src/validation/record-validator.number-value.test.ts

@@ -0,0 +1,147 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #20309 — the number arm refuses a value that is neither a number nor a
+ * string: an array, a boolean, an object.
+ *
+ * ## The defect
+ *
+ * The arm judged `Number(value)` and the write carried `value`. So every value
+ * JS coerces to a finite number passed the check and reached the driver as it
+ * was sent. Measured on `origin/main` c74de10a94 through the real engine and
+ * REST doors:
+ *
+ * - `[500]`: SQLite stored the TEXT `'[500]'` and memory stored the array;
+ * - `[]`: SQLite stored `'[]'` and memory stored the array;
+ * - `true` / `false`: SQLite stored `1` / `0` and memory stored the boolean.
+ *
+ * `[5, 7]` and `{}` were already refused, because `Number()` of each is `NaN`.
+ *
+ * ## What this file pins
+ *
+ * - The refusal set on every judged type, on insert and update, as the
+ *   `VALIDATION_FAILED` envelope with `invalid_number`.
+ * - Parity with the spec over every non-string input: the arm refuses exactly
+ *   what the spec's stored value schema for the type (`valueSchemaFor`,
+ *   `z.number().finite()`) refuses.
+ * - The controls: a finite number passes, including `0` and a negative; a
+ *   blank is still `null` before the arm (#20308); `summary` is still not
+ *   judged (the seat ruling on #20308).
+ * - The STRING half is unchanged: a string is still judged by `Number()`, as
+ *   at base. Which strings a number field accepts waits on the producer census
+ *   and the spec's numeric grammar (#20336). The characterization below turns
+ *   red when that half lands, on purpose.
+ *
+ * The driver-facing half (what reaches the driver on each engine door) is
+ * `../engine-number-value-door.test.ts`. The physical column, through REST on
+ * SQLite, is `packages/rest/src/rest-data-number-value.test.ts`.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { COMPUTED_VALUE_TYPES, NUMERIC_VALUE_TYPES, valueSchemaFor } from '@objectstack/spec/data';
+import { normalizeBlankTypedValues, validateRecord, ValidationError } from './record-validator.js';
+
+const JUDGED = [...NUMERIC_VALUE_TYPES].filter((t) => !COMPUTED_VALUE_TYPES.has(t));
+
+/** Non-strings JS coerces to a finite number, which the old arm let through. */
+const NEWLY_REFUSED: ReadonlyArray<readonly [string, unknown]> = [
+  ['[500]', [500]],
+  ['[]', []],
+  ['[\'12\']', ['12']],
+  ['true', true],
+  ['false', false],
+  ['a Date', new Date(0)],
+  ['a Number object', Object(7)],
+];
+/** Values the old arm already refused; their answer is unchanged. */
+const ALREADY_REFUSED: ReadonlyArray<readonly [string, unknown]> = [
+  ['[5, 7]', [5, 7]],
+  ['{}', {}],
+  ['NaN', Number.NaN],
+  ['Infinity', Number.POSITIVE_INFINITY],
+  ["'Infinity'", 'Infinity'],
+  ["'abc'", 'abc'],
+];
+const ACCEPTED: ReadonlyArray<readonly [string, unknown]> = [
+  ['500', 500],
+  ['12.5', 12.5],
+  ['0', 0],
+  ['-3', -3],
+  ['1e21', 1e21],
+];
+
+function schemaOf(types: readonly string[]) {
+  return { fields: Object.fromEntries(types.map((t) => [`f_${t}`, { name: `f_${t}`, type: t }])) } as any;
+}
+
+/** The field-level answer, or `null` when the write is accepted. */
+function answer(type: string, value: unknown, mode: 'insert' | 'update') {
+  try {
+    validateRecord(schemaOf([type]), { [`f_${type}`]: value }, mode);
+    return null;
+  } catch (e) {
+    expect(e).toBeInstanceOf(ValidationError);
+    expect((e as ValidationError).code).toBe('VALIDATION_FAILED');
+    return (e as ValidationError).fields.map((x) => [x.field, x.code]);
+  }
+}
+
+describe('the number arm: an array, boolean or object is invalid_number (#20309)', () => {
+  it('the judged population is the spec numeric class minus the computed class: the six types', () => {
+    // A control on the set itself: if it emptied, every case below would pass
+    // over nothing.
+    expect([...JUDGED].sort()).toEqual(['currency', 'number', 'percent', 'progress', 'rating', 'slider']);
+  });
+
+  describe.each(JUDGED)('%s', (type) => {
+    it.each([...NEWLY_REFUSED, ...ALREADY_REFUSED])('refuses %s with invalid_number, on insert and update', (_label, value) => {
+      for (const mode of ['insert', 'update'] as const) {
+        expect(answer(type, value, mode), mode).toEqual([[`f_${type}`, 'invalid_number']]);
+      }
+    });
+
+    it.each(ACCEPTED)('accepts the number %s, on insert and update', (_label, value) => {
+      for (const mode of ['insert', 'update'] as const) {
+        expect(answer(type, value, mode), mode).toBeNull();
+      }
+    });
+  });
+
+  it("refuses exactly what the spec's stored value schema refuses, over every non-string input", () => {
+    // `valueSchemaFor` is the spec's declaration of the stored value for the
+    // type. Off the string half, the arm and the declaration must not disagree.
+    for (const type of JUDGED) {
+      const declared = valueSchemaFor({ type }, 'stored');
+      for (const [label, value] of [...NEWLY_REFUSED, ...ALREADY_REFUSED, ...ACCEPTED]) {
+        if (typeof value === 'string') continue;
+        const specAccepts = declared.safeParse(value).success;
+        expect(answer(type, value, 'insert') === null, `${type} ${label}`).toBe(specAccepts);
+      }
+    }
+  });
+
+  it('UNCHANGED here: a string is still judged by Number(), as at base (the string half waits on #20336)', () => {
+    // A characterization, not an endorsement: the spec's stored value schema
+    // refuses every one of these. It turns red when the string half lands.
+    for (const type of JUDGED) {
+      for (const s of ['12', '12.5', '0x10', ' 12 ', '1e3']) {
+        expect(answer(type, s, 'insert'), `${type} ${JSON.stringify(s)}`).toBeNull();
+      }
+    }
+  });
+
+  it('CONTROL: a blank is still null before the arm, so it is never judged (#20308)', () => {
+    for (const blank of ['', '  ']) {
+      const row = normalizeBlankTypedValues(schemaOf(JUDGED), Object.fromEntries(JUDGED.map((t) => [`f_${t}`, blank])));
+      for (const t of JUDGED) expect((row as Record<string, unknown>)[`f_${t}`], t).toBeNull();
+      expect(() => validateRecord(schemaOf(JUDGED), row as Record<string, unknown>, 'insert')).not.toThrow();
+    }
+  });
+
+  it('CONTROL: summary is not judged by the arm (the seat ruling on #20308)', () => {
+    expect(COMPUTED_VALUE_TYPES.has('summary')).toBe(true);
+    for (const [, value] of [...NEWLY_REFUSED, ...ACCEPTED]) {
+      expect(answer('summary', value, 'insert')).toBeNull();
+    }
+  });
+});