fix(objectql)!: enforce a progress field's declared min / max at the write seam (#20386)

.changeset/20386-progress-min-max-enforced.md

@@ -0,0 +1,21 @@
+---
+"@objectstack/objectql": minor
+---
+
+fix(objectql)!: a `progress` field's declared `min` / `max` are enforced on writes — a value outside them is refused with `min_value` / `max_value`, exactly as on `number` (#20386)
+
+Clause-②: no (narrowing)
+
+**BREAKING** — a narrowing of the write accept set on `@objectstack/objectql`, shipped as `minor` under the repo's launch-window convention (`check-changeset-no-major` refuses `major` until GA); the breaking-ness is carried by this banner and the ADR-0087 disposition, never by the level. Nothing an author writes changes spelling: `min` and `max` keep their keys, their type and their legality on every field type.
+
+`FieldSchema.min` / `max` declare a check ("Checked on the WRITTEN value only") with no type exclusion, but the record validator returned for a `progress` field right after its finite-number check, above the bounds. So a `progress` field declaring `max: 100` stored `150`, and one declaring `min: 0` stored `-5`, with `201` on memory and SQLite, while a `number` field with the same bounds refused both. The bounds now bind on `progress` at the one place a write is judged.
+
+**What a caller sees, before → after.** A `progress` write outside a declared bound: `201`, stored as sent → `400 VALIDATION_FAILED` with field code `max_value` (`constraint: { max }`) or `min_value` (`constraint: { min }`), nothing stored. That is the `number` field's answer, envelope for envelope, in all four locales. The REST create, batch, update and updateMany routes all answer it, and `validate` (the dry run) predicts it. A value inside the bounds, or on either bound (both are inclusive), writes exactly as before. Only a write that CARRIES the field is judged: a stored value outside a bound is never re-read and survives an update that does not send it.
+
+The fix, when a write is refused: send a value inside the bounds, or widen or delete the field's `min` / `max` to match what it really holds.
+
+⛔ Only the bounds. `scale` and `precision` stay unread on `progress`: each key's own contract names the types it binds on, and `progress` is in neither set, so `33.5` still writes into a `progress` field that declares `scale: 0`.
+
+**Who is affected, measured** on `origin/main` `dc0ab6a2e`: the two example-app `progress` fields (`examples/app-showcase` `showcase_task.progress` and the field zoo's `f_progress`) both declare `min: 0, max: 100`, and every value their seeds and actions write (12 seed rows, one `progress: 100` action) is inside. The console's `progress` editor, objectui's `SliderField`, drives a Radix slider bounded by the field's declared `min` / `max`, so it cannot emit a value outside them.
+
+<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable moves: `min` and `max` keep their keys, their type (`z.number().optional()`) and their legality on every field type, `packages/spec` is untouched, and no stored metadata representation changes, so `objectstack migrate meta` has nothing to rewrite and the ledger has no row to gain. What narrows is the record validator's write accept set for values under bounds the field already declares, which is runtime behaviour, not an authored shape. The other categories are closed on facts: the package publishes (not `unpublished`); no ADR-0087 id is minted here and none covers this (not `registered` / `already-registered`); and runtime behaviour changes, not only a TypeScript declaration (not `runtime-interface-only` / `type-surface-only`). -->

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

@@ -152,12 +152,14 @@ describe('the numeric type door is NUMERIC_VALUE_TYPES minus COMPUTED_VALUE_TYPE
     expect(refusal(type, 'abc')).toBeNull();
   });
 
-  it('progress takes the type check only, and summary none — no bound or scale is newly enforced', () => {
-    // The boundary the branch states: a declared `max` / `scale` on these two
-    // was never enforced, and this change does not start (a separate decision).
-    for (const type of ['progress', 'summary']) {
-      const fields = { [`f_${type}`]: { name: `f_${type}`, type, max: 100, scale: 0 } } as any;
-      expect(() => validateRecord({ fields }, { [`f_${type}`]: 150.5 }, 'insert'), type).not.toThrow();
-    }
+  it('summary takes no check at all, and progress no `scale` — a declared `max` / `scale` on summary is not read', () => {
+    // The boundary the branch states for `summary`: its shape is the producer's,
+    // so a declared `max` / `scale` is never enforced on it.
+    const summary = { f_summary: { name: 'f_summary', type: 'summary', max: 100, scale: 0 } } as any;
+    expect(() => validateRecord({ fields: summary }, { f_summary: 150.5 }, 'insert')).not.toThrow();
+    // `progress` took the type check here, and its declared `min` / `max` since
+    // #20386 (record-validator.progress-bounds.test.ts) — but still no `scale`.
+    const progress = { f_progress: { name: 'f_progress', type: 'progress', max: 100, scale: 0 } } as any;
+    expect(() => validateRecord({ fields: progress }, { f_progress: 50.5 }, 'insert')).not.toThrow();
   });
 });

packages/objectql/src/validation/record-validator.precision.test.ts

@@ -187,7 +187,7 @@ describe('validateRecord — `precision` on `currency` and `percent` (#19992)',
 });
 
 describe('validateRecord — where `precision` binds, and where it does not (#19992)', () => {
-  it('binds on number, currency, percent, rating and slider; ⛔ not on progress, whose bounds the numeric branch never reads', () => {
+  it('binds on number, currency, percent, rating and slider; ⛔ not on progress, which takes only `min` / `max` (#20386)', () => {
     for (const type of ['number', 'currency', 'percent', 'rating', 'slider']) {
       const s = { fields: { v: { type, label: 'V', precision: 1, ...(type === 'percent' ? { max: 100 } : {}) } } };
       expect(fieldsOf(s, { v: 12 })?.[0], type).toMatchObject({ field: 'v', code: 'max_precision' });

packages/objectql/src/validation/record-validator.progress-bounds.test.ts

@@ -0,0 +1,252 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+import { describe, it, expect, beforeEach } from 'vitest';
+import { validateRecord, ValidationError } from './record-validator.js';
+import { ObjectQL } from '../engine.js';
+
+/**
+ * #20386 — a `progress` field's declared `min` / `max` are ENFORCED at the
+ * write seam, with the `number` field's codes (`min_value` / `max_value`).
+ *
+ * Before this, the number arm returned for `progress` right after the finite
+ * check, above the bounds. Measured through the REST create route on
+ * `origin/main` dc0ab6a2e: `150` into `max: 100` and `-5` into `min: 0` both
+ * answered 201 and were stored, on memory and on SQLite, while a `number` field
+ * with the same bounds refused both. `FieldSchema.min` / `max` declare the
+ * check 「Checked on the WRITTEN value only」 with no type exclusion, and
+ * triage 5865053231 ruled ENFORCE.
+ *
+ * ⛔ Only the bounds: `scale` and `precision` name their own type sets, and
+ * `progress` is in neither, so both stay unread on it. The pins below hold
+ * that line from both sides — the same declaration on `slider` refuses.
+ *
+ * The REST physical-column half (SQLite) is
+ * `packages/rest/src/rest-data-progress-bounds.test.ts`. Memory and MongoDB
+ * store exactly the payload the engine hands them, which the stub driver in the
+ * last block records.
+ */
+
+const fieldsOf = (
+  schema: Parameters<typeof validateRecord>[0],
+  data: Record<string, unknown>,
+  mode: 'insert' | 'update' = 'insert',
+) => {
+  try {
+    validateRecord(schema, data, mode);
+  } catch (e) {
+    expect(e).toBeInstanceOf(ValidationError);
+    expect((e as ValidationError).code).toBe('VALIDATION_FAILED');
+    return (e as ValidationError).fields;
+  }
+  return null;
+};
+
+const BOUNDED = (type: string, extra: Record<string, unknown> = {}) => ({
+  fields: { v: { type, label: 'Done', min: 0, max: 100, ...extra } },
+});
+
+describe('validateRecord — a `progress` field\'s `min` / `max` are enforced (#20386): the triage pins', () => {
+  it('`progress` `max: 100` refuses 150 with `max_value`, the `number` field\'s code', () => {
+    const errs = fieldsOf(BOUNDED('progress'), { v: 150 });
+    expect(errs).toHaveLength(1);
+    expect(errs![0]).toMatchObject({ field: 'v', code: 'max_value', constraint: { max: 100 } });
+    // The template is interpolated: the bound reaches the sentence.
+    expect(errs![0].message).toContain('100');
+    expect(errs![0].message).not.toContain('{{');
+  });
+
+  it('`progress` `min: 0` refuses -5 with `min_value`, the `number` field\'s code', () => {
+    const errs = fieldsOf(BOUNDED('progress'), { v: -5 });
+    expect(errs).toHaveLength(1);
+    expect(errs![0]).toMatchObject({ field: 'v', code: 'min_value', constraint: { min: 0 } });
+  });
+
+  it('a value inside the bounds is accepted, and both bounds are inclusive', () => {
+    for (const v of [50, 0, 100, 33.5]) {
+      expect(fieldsOf(BOUNDED('progress'), { v }), String(v)).toBeNull();
+    }
+  });
+
+  it('the refusal is the `number` field\'s refusal, envelope for envelope', () => {
+    for (const v of [150, -5]) {
+      const strip = (errs: ReturnType<typeof fieldsOf>) => errs?.map(({ field, code, constraint }) => ({ field, code, constraint }));
+      expect(strip(fieldsOf(BOUNDED('progress'), { v })), String(v)).toEqual(strip(fieldsOf(BOUNDED('number'), { v })));
+    }
+  });
+
+  it('one bound declared alone binds alone', () => {
+    const maxOnly = { fields: { v: { type: 'progress', label: 'Done', max: 100 } } };
+    expect(fieldsOf(maxOnly, { v: 101 })?.[0]).toMatchObject({ field: 'v', code: 'max_value' });
+    expect(fieldsOf(maxOnly, { v: -1000 })).toBeNull();
+    const minOnly = { fields: { v: { type: 'progress', label: 'Done', min: 0 } } };
+    expect(fieldsOf(minOnly, { v: -0.5 })?.[0]).toMatchObject({ field: 'v', code: 'min_value' });
+    expect(fieldsOf(minOnly, { v: 1e6 })).toBeNull();
+    // No bound declared: nothing is invented for the type (no implicit 0..100).
+    const unbounded = { fields: { v: { type: 'progress', label: 'Done' } } };
+    expect(fieldsOf(unbounded, { v: 150 })).toBeNull();
+    expect(fieldsOf(unbounded, { v: -5 })).toBeNull();
+  });
+
+  it('refuses on update too, judges a string-carried number after coercion, and never re-reads an omitted field', () => {
+    expect(fieldsOf(BOUNDED('progress'), { v: 150 }, 'update')?.[0]).toMatchObject({ field: 'v', code: 'max_value' });
+    expect(fieldsOf(BOUNDED('progress'), { v: '150' })?.[0]).toMatchObject({ field: 'v', code: 'max_value' });
+    expect(fieldsOf(BOUNDED('progress'), { v: '-5' })?.[0]).toMatchObject({ field: 'v', code: 'min_value' });
+    expect(fieldsOf(BOUNDED('progress'), { v: '50' })).toBeNull();
+    // The WRITTEN value only: an update that does not carry the field is not judged.
+    expect(fieldsOf(BOUNDED('progress'), { other: 1 }, 'update')).toBeNull();
+  });
+});
+
+describe('validateRecord — ⛔ `progress` takes the bounds only, never `scale` or `precision` (#20386)', () => {
+  it('`scale: 0` is not read on `progress` — the same declaration on `slider` refuses', () => {
+    expect(fieldsOf(BOUNDED('progress', { scale: 0 }), { v: 33.5 })).toBeNull();
+    expect(fieldsOf(BOUNDED('slider', { scale: 0 }), { v: 33.5 })?.[0]).toMatchObject({ field: 'v', code: 'max_scale' });
+  });
+
+  it('`precision: 2` is not read on `progress` — the same declaration on `slider` refuses', () => {
+    expect(fieldsOf(BOUNDED('progress', { precision: 2 }), { v: 99.5 })).toBeNull();
+    expect(fieldsOf(BOUNDED('slider', { precision: 2 }), { v: 99.5 })?.[0]).toMatchObject({ field: 'v', code: 'max_precision' });
+  });
+
+  it('a bound still answers first when `scale` / `precision` are declared beside it', () => {
+    expect(fieldsOf(BOUNDED('progress', { scale: 0, precision: 2 }), { v: 150.5 })?.[0]).toMatchObject({ field: 'v', code: 'max_value' });
+  });
+});
+
+// ---------------------------------------------------------------------------
+// Every engine write door reaches the refusal, the bulk doors included (AGENTS.md
+// Prime Directive #10: "check every call site, bulk paths included"). The stub
+// driver records what it is handed, so a refused write is shown to reach
+// nothing, and an accepted one to arrive as the same number.
+// ---------------------------------------------------------------------------
+
+function makeStubDriver() {
+  const calls: Array<{ fn: string; data: unknown }> = [];
+  const rows = new Map<string, Record<string, unknown>>();
+  let n = 0;
+  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', data: { ...data } });
+      const row = { ...data, id: (data.id as string) ?? `r${++n}` };
+      rows.set(row.id as string, row);
+      return row;
+    },
+    async bulkCreate(_o: string, list: Record<string, unknown>[]) {
+      calls.push({ fn: 'bulkCreate', data: list.map((r) => ({ ...r })) });
+      return list.map((r) => {
+        const row = { ...r, id: (r.id as string) ?? `r${++n}` };
+        rows.set(row.id as string, row);
+        return row;
+      });
+    },
+    async update(_o: string, id: string, data: Record<string, unknown>) {
+      calls.push({ fn: 'update', data: { ...data } });
+      const row = { ...(rows.get(id) ?? {}), ...data, id };
+      rows.set(id, row);
+      return row;
+    },
+    async updateMany(_o: string, _ast: unknown, data: Record<string, unknown>) {
+      calls.push({ fn: 'updateMany', data: { ...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 TASK = {
+  name: 'progress_task',
+  label: 'Task',
+  fields: {
+    id: { name: 'id', type: 'text' as const, primaryKey: true },
+    done: { name: 'done', type: 'progress' as const, label: 'Done', min: 0, max: 100 },
+  },
+};
+
+describe('engine write doors — a `progress` bound is refused on every door, bulk included (#20386)', () => {
+  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(TASK as any);
+  });
+
+  const refusal = async (fn: () => Promise<unknown>) => {
+    try {
+      await fn();
+    } catch (e) {
+      expect(e).toBeInstanceOf(ValidationError);
+      return { code: (e as ValidationError).code, fields: (e as ValidationError).fields.map((f) => [f.field, f.code]) };
+    }
+    return null;
+  };
+  const OVER = { code: 'VALIDATION_FAILED', fields: [['done', 'max_value']] };
+  const UNDER = { code: 'VALIDATION_FAILED', fields: [['done', 'min_value']] };
+  const writes = () => stub.calls.filter((c) => c.fn !== 'find');
+  /** Every value any driver write call carried for `done`. */
+  const written = () =>
+    writes()
+      .flatMap((c) => (Array.isArray(c.data) ? c.data : [c.data]) as Record<string, unknown>[])
+      .filter((r) => 'done' in r)
+      .map((r) => r.done);
+
+  it('insert of one row, and of an array of rows — the whole batch is refused and nothing reaches the driver', async () => {
+    expect(await refusal(() => engine.insert('progress_task', { id: 'a', done: 150 }))).toEqual(OVER);
+    expect(await refusal(() => engine.insert('progress_task', { id: 'b', done: -5 }))).toEqual(UNDER);
+    expect(
+      await refusal(() => engine.insert('progress_task', [{ id: 'c1', done: 50 }, { id: 'c2', done: 150 }])),
+    ).toEqual(OVER);
+    expect(writes()).toEqual([]);
+  });
+
+  it('insertMany (partial success): the out-of-bound row fails alone, the fitting row is written', async () => {
+    const outcomes = await engine.insertMany('progress_task', [{ id: 'm1', done: 50 }, { id: 'm2', done: 150 }]);
+    expect(outcomes.map((o) => o.ok)).toEqual([true, false]);
+    const failed = outcomes[1] as { ok: false; error: unknown };
+    expect(failed.error).toBeInstanceOf(ValidationError);
+    expect((failed.error as ValidationError).fields.map((f) => [f.field, f.code])).toEqual([['done', 'max_value']]);
+    expect(written()).toEqual([50]);
+  });
+
+  it('update by id and update by predicate (multi) — refused before the driver', async () => {
+    await engine.insert('progress_task', { id: 'u1', done: 10 });
+    stub.calls.length = 0;
+    expect(await refusal(() => engine.update('progress_task', { id: 'u1', done: 150 }))).toEqual(OVER);
+    expect(
+      await refusal(() => engine.update('progress_task', { done: -5 }, { where: { id: { $in: ['u1'] } }, multi: true } as any)),
+    ).toEqual(UNDER);
+    expect(writes().filter((c) => c.fn === 'update' || c.fn === 'updateMany')).toEqual([]);
+  });
+
+  it('the dry run (`validate`) predicts the same refusal', async () => {
+    const refused = await engine.validate('progress_task', { id: 'p1', done: 150 });
+    expect(refused.valid).toBe(false);
+    expect(refused.results?.[0]?.errors.map((e: any) => [e.field, e.code])).toEqual([['done', 'max_value']]);
+    expect((await engine.validate('progress_task', { id: 'p2', done: 100 })).valid).toBe(true);
+  });
+
+  it('CONTROL — a value inside the bounds reaches the driver as the same number on every door', async () => {
+    await engine.insert('progress_task', { id: 'k1', done: 100 });
+    await engine.insert('progress_task', [{ id: 'k2', done: 0 }]);
+    await engine.update('progress_task', { id: 'k1', done: 45.5 });
+    await engine.update('progress_task', { done: 60 }, { where: { id: { $in: ['k1'] } }, multi: true } as any);
+    const got = written();
+    expect(got).toEqual([100, 0, 45.5, 60]);
+  });
+});