Skip to content

Commit 47557d1

Browse files
committed
fix(objectql)!: enforce a progress field's declared min / max at the write seam (#20386)
The number arm returned for `progress` right after the finite check, above the bounds, so a `progress` field declaring `max: 100` stored 150 and one declaring `min: 0` stored -5 (201 on memory and SQLite) while a `number` field refused both. `FieldSchema.min` / `max` declare the check with no type exclusion; triage ruled ENFORCE. The early return moves below the bounds and stays above `scale` and `precision`, whose own contracts name type sets `progress` is not in, so only the bounds start binding. Refusals carry the `number` field's codes, `min_value` / `max_value`. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN
1 parent dc0ab6a commit 47557d1

4 files changed

Lines changed: 280 additions & 16 deletions

File tree

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -152,12 +152,14 @@ describe('the numeric type door is NUMERIC_VALUE_TYPES minus COMPUTED_VALUE_TYPE
152152
expect(refusal(type, 'abc')).toBeNull();
153153
});
154154

155-
it('progress takes the type check only, and summary none — no bound or scale is newly enforced', () => {
156-
// The boundary the branch states: a declared `max` / `scale` on these two
157-
// was never enforced, and this change does not start (a separate decision).
158-
for (const type of ['progress', 'summary']) {
159-
const fields = { [`f_${type}`]: { name: `f_${type}`, type, max: 100, scale: 0 } } as any;
160-
expect(() => validateRecord({ fields }, { [`f_${type}`]: 150.5 }, 'insert'), type).not.toThrow();
161-
}
155+
it('summary takes no check at all, and progress no `scale` — a declared `max` / `scale` on summary is not read', () => {
156+
// The boundary the branch states for `summary`: its shape is the producer's,
157+
// so a declared `max` / `scale` is never enforced on it.
158+
const summary = { f_summary: { name: 'f_summary', type: 'summary', max: 100, scale: 0 } } as any;
159+
expect(() => validateRecord({ fields: summary }, { f_summary: 150.5 }, 'insert')).not.toThrow();
160+
// `progress` took the type check here, and its declared `min` / `max` since
161+
// #20386 (record-validator.progress-bounds.test.ts) — but still no `scale`.
162+
const progress = { f_progress: { name: 'f_progress', type: 'progress', max: 100, scale: 0 } } as any;
163+
expect(() => validateRecord({ fields: progress }, { f_progress: 50.5 }, 'insert')).not.toThrow();
162164
});
163165
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ describe('validateRecord — `precision` on `currency` and `percent` (#19992)',
187187
});
188188

189189
describe('validateRecord — where `precision` binds, and where it does not (#19992)', () => {
190-
it('binds on number, currency, percent, rating and slider; ⛔ not on progress, whose bounds the numeric branch never reads', () => {
190+
it('binds on number, currency, percent, rating and slider; ⛔ not on progress, which takes only `min` / `max` (#20386)', () => {
191191
for (const type of ['number', 'currency', 'percent', 'rating', 'slider']) {
192192
const s = { fields: { v: { type, label: 'V', precision: 1, ...(type === 'percent' ? { max: 100 } : {}) } } };
193193
expect(fieldsOf(s, { v: 12 })?.[0], type).toMatchObject({ field: 'v', code: 'max_precision' });
Lines changed: 252 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect, beforeEach } from 'vitest';
4+
import { validateRecord, ValidationError } from './record-validator.js';
5+
import { ObjectQL } from '../engine.js';
6+
7+
/**
8+
* #20386 — a `progress` field's declared `min` / `max` are ENFORCED at the
9+
* write seam, with the `number` field's codes (`min_value` / `max_value`).
10+
*
11+
* Before this, the number arm returned for `progress` right after the finite
12+
* check, above the bounds. Measured through the REST create route on
13+
* `origin/main` dc0ab6a2e: `150` into `max: 100` and `-5` into `min: 0` both
14+
* answered 201 and were stored, on memory and on SQLite, while a `number` field
15+
* with the same bounds refused both. `FieldSchema.min` / `max` declare the
16+
* check 「Checked on the WRITTEN value only」 with no type exclusion, and
17+
* triage 5865053231 ruled ENFORCE.
18+
*
19+
* ⛔ Only the bounds: `scale` and `precision` name their own type sets, and
20+
* `progress` is in neither, so both stay unread on it. The pins below hold
21+
* that line from both sides — the same declaration on `slider` refuses.
22+
*
23+
* The REST physical-column half (SQLite) is
24+
* `packages/rest/src/rest-data-progress-bounds.test.ts`. Memory and MongoDB
25+
* store exactly the payload the engine hands them, which the stub driver in the
26+
* last block records.
27+
*/
28+
29+
const fieldsOf = (
30+
schema: Parameters<typeof validateRecord>[0],
31+
data: Record<string, unknown>,
32+
mode: 'insert' | 'update' = 'insert',
33+
) => {
34+
try {
35+
validateRecord(schema, data, mode);
36+
} catch (e) {
37+
expect(e).toBeInstanceOf(ValidationError);
38+
expect((e as ValidationError).code).toBe('VALIDATION_FAILED');
39+
return (e as ValidationError).fields;
40+
}
41+
return null;
42+
};
43+
44+
const BOUNDED = (type: string, extra: Record<string, unknown> = {}) => ({
45+
fields: { v: { type, label: 'Done', min: 0, max: 100, ...extra } },
46+
});
47+
48+
describe('validateRecord — a `progress` field\'s `min` / `max` are enforced (#20386): the triage pins', () => {
49+
it('`progress` `max: 100` refuses 150 with `max_value`, the `number` field\'s code', () => {
50+
const errs = fieldsOf(BOUNDED('progress'), { v: 150 });
51+
expect(errs).toHaveLength(1);
52+
expect(errs![0]).toMatchObject({ field: 'v', code: 'max_value', constraint: { max: 100 } });
53+
// The template is interpolated: the bound reaches the sentence.
54+
expect(errs![0].message).toContain('100');
55+
expect(errs![0].message).not.toContain('{{');
56+
});
57+
58+
it('`progress` `min: 0` refuses -5 with `min_value`, the `number` field\'s code', () => {
59+
const errs = fieldsOf(BOUNDED('progress'), { v: -5 });
60+
expect(errs).toHaveLength(1);
61+
expect(errs![0]).toMatchObject({ field: 'v', code: 'min_value', constraint: { min: 0 } });
62+
});
63+
64+
it('a value inside the bounds is accepted, and both bounds are inclusive', () => {
65+
for (const v of [50, 0, 100, 33.5]) {
66+
expect(fieldsOf(BOUNDED('progress'), { v }), String(v)).toBeNull();
67+
}
68+
});
69+
70+
it('the refusal is the `number` field\'s refusal, envelope for envelope', () => {
71+
for (const v of [150, -5]) {
72+
const strip = (errs: ReturnType<typeof fieldsOf>) => errs?.map(({ field, code, constraint }) => ({ field, code, constraint }));
73+
expect(strip(fieldsOf(BOUNDED('progress'), { v })), String(v)).toEqual(strip(fieldsOf(BOUNDED('number'), { v })));
74+
}
75+
});
76+
77+
it('one bound declared alone binds alone', () => {
78+
const maxOnly = { fields: { v: { type: 'progress', label: 'Done', max: 100 } } };
79+
expect(fieldsOf(maxOnly, { v: 101 })?.[0]).toMatchObject({ code: 'max_value' });
80+
expect(fieldsOf(maxOnly, { v: -1000 })).toBeNull();
81+
const minOnly = { fields: { v: { type: 'progress', label: 'Done', min: 0 } } };
82+
expect(fieldsOf(minOnly, { v: -0.5 })?.[0]).toMatchObject({ code: 'min_value' });
83+
expect(fieldsOf(minOnly, { v: 1e6 })).toBeNull();
84+
// No bound declared: nothing is invented for the type (no implicit 0..100).
85+
const unbounded = { fields: { v: { type: 'progress', label: 'Done' } } };
86+
expect(fieldsOf(unbounded, { v: 150 })).toBeNull();
87+
expect(fieldsOf(unbounded, { v: -5 })).toBeNull();
88+
});
89+
90+
it('refuses on update too, judges a string-carried number after coercion, and never re-reads an omitted field', () => {
91+
expect(fieldsOf(BOUNDED('progress'), { v: 150 }, 'update')?.[0]).toMatchObject({ code: 'max_value' });
92+
expect(fieldsOf(BOUNDED('progress'), { v: '150' })?.[0]).toMatchObject({ code: 'max_value' });
93+
expect(fieldsOf(BOUNDED('progress'), { v: '-5' })?.[0]).toMatchObject({ code: 'min_value' });
94+
expect(fieldsOf(BOUNDED('progress'), { v: '50' })).toBeNull();
95+
// The WRITTEN value only: an update that does not carry the field is not judged.
96+
expect(fieldsOf(BOUNDED('progress'), { other: 1 }, 'update')).toBeNull();
97+
});
98+
});
99+
100+
describe('validateRecord — ⛔ `progress` takes the bounds only, never `scale` or `precision` (#20386)', () => {
101+
it('`scale: 0` is not read on `progress` — the same declaration on `slider` refuses', () => {
102+
expect(fieldsOf(BOUNDED('progress', { scale: 0 }), { v: 33.5 })).toBeNull();
103+
expect(fieldsOf(BOUNDED('slider', { scale: 0 }), { v: 33.5 })?.[0]).toMatchObject({ code: 'max_scale' });
104+
});
105+
106+
it('`precision: 2` is not read on `progress` — the same declaration on `slider` refuses', () => {
107+
expect(fieldsOf(BOUNDED('progress', { precision: 2 }), { v: 99.5 })).toBeNull();
108+
expect(fieldsOf(BOUNDED('slider', { precision: 2 }), { v: 99.5 })?.[0]).toMatchObject({ code: 'max_precision' });
109+
});
110+
111+
it('a bound still answers first when `scale` / `precision` are declared beside it', () => {
112+
expect(fieldsOf(BOUNDED('progress', { scale: 0, precision: 2 }), { v: 150.5 })?.[0]).toMatchObject({ code: 'max_value' });
113+
});
114+
});
115+
116+
// ---------------------------------------------------------------------------
117+
// Every engine write door reaches the refusal, the bulk doors included (AGENTS.md
118+
// Prime Directive #10: "check every call site, bulk paths included"). The stub
119+
// driver records what it is handed, so a refused write is shown to reach
120+
// nothing, and an accepted one to arrive as the same number.
121+
// ---------------------------------------------------------------------------
122+
123+
function makeStubDriver() {
124+
const calls: Array<{ fn: string; data: unknown }> = [];
125+
const rows = new Map<string, Record<string, unknown>>();
126+
let n = 0;
127+
const driver: any = {
128+
name: 'stub', version: '0.0.0', supports: {},
129+
async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
130+
async find() { return [...rows.values()]; },
131+
async findOne(_o: string, q: any) {
132+
const id = (q?.where ?? q?.filter ?? q)?.id;
133+
return (typeof id === 'string' ? rows.get(id) : rows.values().next().value) ?? null;
134+
},
135+
async count() { return rows.size; },
136+
async create(_o: string, data: Record<string, unknown>) {
137+
calls.push({ fn: 'create', data: { ...data } });
138+
const row = { ...data, id: (data.id as string) ?? `r${++n}` };
139+
rows.set(row.id as string, row);
140+
return row;
141+
},
142+
async bulkCreate(_o: string, list: Record<string, unknown>[]) {
143+
calls.push({ fn: 'bulkCreate', data: list.map((r) => ({ ...r })) });
144+
return list.map((r) => {
145+
const row = { ...r, id: (r.id as string) ?? `r${++n}` };
146+
rows.set(row.id as string, row);
147+
return row;
148+
});
149+
},
150+
async update(_o: string, id: string, data: Record<string, unknown>) {
151+
calls.push({ fn: 'update', data: { ...data } });
152+
const row = { ...(rows.get(id) ?? {}), ...data, id };
153+
rows.set(id, row);
154+
return row;
155+
},
156+
async updateMany(_o: string, _ast: unknown, data: Record<string, unknown>) {
157+
calls.push({ fn: 'updateMany', data: { ...data } });
158+
return rows.size;
159+
},
160+
async upsert(o: string, data: Record<string, unknown>) { return this.create(o, data); },
161+
async delete() { return true; },
162+
async bulkUpdate() { return []; }, async bulkDelete() {},
163+
async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; },
164+
async commit() {}, async rollback() {},
165+
};
166+
return { driver, calls };
167+
}
168+
169+
const TASK = {
170+
name: 'progress_task',
171+
label: 'Task',
172+
fields: {
173+
id: { name: 'id', type: 'text' as const, primaryKey: true },
174+
done: { name: 'done', type: 'progress' as const, label: 'Done', min: 0, max: 100 },
175+
},
176+
};
177+
178+
describe('engine write doors — a `progress` bound is refused on every door, bulk included (#20386)', () => {
179+
let engine: ObjectQL;
180+
let stub: ReturnType<typeof makeStubDriver>;
181+
182+
beforeEach(async () => {
183+
stub = makeStubDriver();
184+
engine = new ObjectQL();
185+
engine.registerDriver(stub.driver, true);
186+
await engine.init();
187+
engine.registry.registerObject(TASK as any);
188+
});
189+
190+
const refusal = async (fn: () => Promise<unknown>) => {
191+
try {
192+
await fn();
193+
} catch (e) {
194+
expect(e).toBeInstanceOf(ValidationError);
195+
return { code: (e as ValidationError).code, fields: (e as ValidationError).fields.map((f) => [f.field, f.code]) };
196+
}
197+
return null;
198+
};
199+
const OVER = { code: 'VALIDATION_FAILED', fields: [['done', 'max_value']] };
200+
const UNDER = { code: 'VALIDATION_FAILED', fields: [['done', 'min_value']] };
201+
const writes = () => stub.calls.filter((c) => c.fn !== 'find');
202+
/** Every value any driver write call carried for `done`. */
203+
const written = () =>
204+
writes()
205+
.flatMap((c) => (Array.isArray(c.data) ? c.data : [c.data]) as Record<string, unknown>[])
206+
.filter((r) => 'done' in r)
207+
.map((r) => r.done);
208+
209+
it('insert of one row, and of an array of rows — the whole batch is refused and nothing reaches the driver', async () => {
210+
expect(await refusal(() => engine.insert('progress_task', { id: 'a', done: 150 }))).toEqual(OVER);
211+
expect(await refusal(() => engine.insert('progress_task', { id: 'b', done: -5 }))).toEqual(UNDER);
212+
expect(
213+
await refusal(() => engine.insert('progress_task', [{ id: 'c1', done: 50 }, { id: 'c2', done: 150 }])),
214+
).toEqual(OVER);
215+
expect(writes()).toEqual([]);
216+
});
217+
218+
it('insertMany (partial success): the out-of-bound row fails alone, the fitting row is written', async () => {
219+
const outcomes = await engine.insertMany('progress_task', [{ id: 'm1', done: 50 }, { id: 'm2', done: 150 }]);
220+
expect(outcomes.map((o) => o.ok)).toEqual([true, false]);
221+
const failed = outcomes[1] as { ok: false; error: unknown };
222+
expect(failed.error).toBeInstanceOf(ValidationError);
223+
expect((failed.error as ValidationError).fields.map((f) => [f.field, f.code])).toEqual([['done', 'max_value']]);
224+
expect(written()).toEqual([50]);
225+
});
226+
227+
it('update by id and update by predicate (multi) — refused before the driver', async () => {
228+
await engine.insert('progress_task', { id: 'u1', done: 10 });
229+
stub.calls.length = 0;
230+
expect(await refusal(() => engine.update('progress_task', { id: 'u1', done: 150 }))).toEqual(OVER);
231+
expect(
232+
await refusal(() => engine.update('progress_task', { done: -5 }, { where: { id: { $in: ['u1'] } }, multi: true } as any)),
233+
).toEqual(UNDER);
234+
expect(writes().filter((c) => c.fn === 'update' || c.fn === 'updateMany')).toEqual([]);
235+
});
236+
237+
it('the dry run (`validate`) predicts the same refusal', async () => {
238+
const refused = await engine.validate('progress_task', { id: 'p1', done: 150 });
239+
expect(refused.valid).toBe(false);
240+
expect(refused.results?.[0]?.errors.map((e: any) => [e.field, e.code])).toEqual([['done', 'max_value']]);
241+
expect((await engine.validate('progress_task', { id: 'p2', done: 100 })).valid).toBe(true);
242+
});
243+
244+
it('CONTROL — a value inside the bounds reaches the driver as the same number on every door', async () => {
245+
await engine.insert('progress_task', { id: 'k1', done: 100 });
246+
await engine.insert('progress_task', [{ id: 'k2', done: 0 }]);
247+
await engine.update('progress_task', { id: 'k1', done: 45.5 });
248+
await engine.update('progress_task', { done: 60 }, { where: { id: { $in: ['k1'] } }, multi: true } as any);
249+
const got = written();
250+
expect(got).toEqual([100, 0, 45.5, 60]);
251+
});
252+
});

‎packages/objectql/src/validation/record-validator.ts‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@
2828
* - number types an array, boolean or object is `invalid_number`, never
2929
* coerced (#20309); a number, or a string by `Number()`,
3030
* must be finite
31-
* - `min` / `max` (number/currency/percent/rating/slider)
31+
* - `min` / `max` (number/currency/percent/rating/slider/progress — `progress`
32+
* since #20386; it takes neither `scale` nor `precision`)
3233
* - `scale` more decimal places than the field's STORED allowance →
3334
* `max_scale` (#7501; rejection, NEVER rounding —
3435
* maintainer ruling 2026-08-11), on `number` / `percent` /
@@ -942,19 +943,28 @@ function validateOne(
942943
if (!Number.isFinite(n)) {
943944
return fail('invalid_number');
944945
}
945-
// [#20308] `progress` joined the TYPE check above, and only that. The
946-
// bounds and `scale` below keep the five types they always read: `scale`'s
947-
// own contract names the types it is enforced on (`number`, `percent`,
948-
// `rating`, `slider`), and `min` / `max` on `progress` were never enforced;
949-
// starting to enforce either would narrow what a caller may write, which is
950-
// a separate decision from "a numeric column holds a number".
951-
if (t === 'progress') return null;
946+
// `min` / `max` bind on every type through this door, `progress` included.
947+
// [#20386] `progress` joined the TYPE check above in #20308 and, with this
948+
// change, the bounds: `FieldSchema.min` / `max` declare 「Checked on the
949+
// WRITTEN value only」 with no type exclusion, and triage 5865053231 ruled
950+
// ENFORCE — a `progress` field that declares `max: 100` stored `150` (and
951+
// `min: 0` stored `-5`) with 201 on memory and SQLite while `number`
952+
// refused both. It answers the `number` field's codes, `min_value` /
953+
// `max_value`. A narrowing of what a caller may write, shipped BREAKING.
952954
if (def.min !== undefined && n < def.min) {
953955
return fail('min_value', { min: def.min });
954956
}
955957
if (def.max !== undefined && n > def.max) {
956958
return fail('max_value', { max: def.max });
957959
}
960+
// ⛔ …and only the bounds. `scale` and `precision` below keep the five
961+
// types they always read, because each key's own contract names them:
962+
// `scale` 「Applies to `number`, `percent`, `rating` and `slider` fields」,
963+
// `precision` 「Enforced on writes of `number`, `currency`, `percent`,
964+
// `rating` and `slider` fields … Not read on any other field type」.
965+
// Starting either on `progress` would narrow a write that no contract
966+
// names, so this return stays below the bounds and above both.
967+
if (t === 'progress') return null;
958968
// ── `scale` — enforced by REJECTION, never rounding (#7501) ──
959969
// Maintainer ruling 2026-08-11: an over-scale value is refused the way an
960970
// out-of-range one is; silent rounding is silently altering data. Applies

0 commit comments

Comments
 (0)