fix(rest): import dry run must bound-check values, not just coerce them (#3956)

packages/rest/src/export-format.ts

@@ -37,6 +37,18 @@ export interface ExportFieldMeta {
  readonly?: boolean;
  /** Field declares a `defaultValue` the engine applies on insert (satisfies required). */
  hasDefault?: boolean;
+ // The bounds below drive the import path's field-constraint pre-check
+ // (import-coerce.ts `firstConstraintViolation`), mirroring the engine's
+ // `validateRecord` so a dry run predicts a range/length rejection instead of
+ // green-lighting a row the real write then fails (framework#3956).
+ /** Lower bound for numeric fields. */
+ min?: number;
+ /** Upper bound for numeric fields. */
+ max?: number;
+ /** Minimum character count for string fields. */
+ minLength?: number;
+ /** Maximum character count for string fields. */
+ maxLength?: number;
 }
 
 /**
@@ -137,6 +149,10 @@ export function buildFieldMetaMap(schema: unknown): Map<string, ExportFieldMeta>
  // ⇒ no default): any non-null default — literal, expression object, or the
  // `current_user` token — counts as satisfying a required field.
  hasDefault: f.defaultValue != null,
+ min: typeof f.min === 'number' ? f.min : undefined,
+ max: typeof f.max === 'number' ? f.max : undefined,
+ minLength: typeof f.minLength === 'number' ? f.minLength : undefined,
+ maxLength: typeof f.maxLength === 'number' ? f.maxLength : undefined,
  });
  }
  return map;

packages/rest/src/import-coerce.test.ts

@@ -14,6 +14,7 @@ import {
  matchOption,
  splitMulti,
  coerceRow,
+ firstConstraintViolation,
 } from './import-coerce';
 import type { ExportFieldMeta } from './export-format';
 
@@ -272,3 +273,74 @@ describe('coerceRow', () => {
  expect(data).toEqual({ mystery: 'raw' });
  });
 });
+
+describe('firstConstraintViolation (framework#3956)', () => {
+ const meta = (defs: Record<string, Partial<ExportFieldMeta>>): Map<string, ExportFieldMeta> => {
+ const m = new Map<string, ExportFieldMeta>();
+ for (const [name, d] of Object.entries(defs)) m.set(name, { name, ...d });
+ return m;
+ };
+
+ it('reports a numeric value below `min` with the engine\'s own message', () => {
+ // The issue's repro: penalty_amount { type: 'number', min: 0, max: 9999999.99 }
+ const metaMap = meta({ penalty_amount: { type: 'number', min: 0, max: 9999999.99 } });
+ expect(firstConstraintViolation({ penalty_amount: -500 }, metaMap)).toEqual({
+ field: 'penalty_amount', code: 'min_value', message: 'penalty_amount must be ≥ 0',
+ });
+ });
+
+ it('reports a numeric value above `max`', () => {
+ const metaMap = meta({ pct: { type: 'percent', max: 100 } });
+ expect(firstConstraintViolation({ pct: 101 }, metaMap)).toEqual({
+ field: 'pct', code: 'max_value', message: 'pct must be ≤ 100',
+ });
+ });
+
+ it('reports string length violations both ways', () => {
+ const metaMap = meta({ code: { type: 'text', minLength: 3, maxLength: 5 } });
+ expect(firstConstraintViolation({ code: 'abcdef' }, metaMap)).toEqual({
+ field: 'code', code: 'max_length', message: 'code must be ≤ 5 characters (got 6)',
+ });
+ expect(firstConstraintViolation({ code: 'ab' }, metaMap)).toEqual({
+ field: 'code', code: 'min_length', message: 'code must be ≥ 3 characters (got 2)',
+ });
+ });
+
+ it('accepts values inside the declared bounds', () => {
+ const metaMap = meta({
+ amount: { type: 'currency', min: 0, max: 100 },
+ title: { type: 'text', maxLength: 10 },
+ });
+ expect(firstConstraintViolation({ amount: 0 }, metaMap)).toBeNull();
+ expect(firstConstraintViolation({ amount: 100 }, metaMap)).toBeNull();
+ expect(firstConstraintViolation({ title: 'ten chars!' }, metaMap)).toBeNull();
+ });
+
+ it('skips absent values — a bound never fires on a field the row omits', () => {
+ const metaMap = meta({ amount: { type: 'number', min: 10 } });
+ expect(firstConstraintViolation({}, metaMap)).toBeNull();
+ expect(firstConstraintViolation({ amount: null }, metaMap)).toBeNull();
+ expect(firstConstraintViolation({ amount: '' }, metaMap)).toBeNull();
+ });
+
+ it('skips system / readonly columns the importer never supplies', () => {
+ const metaMap = meta({
+ seq: { type: 'number', min: 100, system: true },
+ score: { type: 'number', min: 100, readonly: true },
+ });
+ expect(firstConstraintViolation({ seq: 1, score: 1 }, metaMap)).toBeNull();
+ });
+
+ it('leaves an unparseable number to coerceRow rather than double-reporting', () => {
+ const metaMap = meta({ amount: { type: 'number', min: 0 } });
+ expect(firstConstraintViolation({ amount: 'abc' }, metaMap)).toBeNull();
+ });
+
+ it('bound-checks only the types the engine bound-checks', () => {
+ // `progress` is numeric per the spec but the engine's validateOne leaves it
+ // unchecked — mirroring the wider spec set here would reject rows the real
+ // write accepts.
+ const metaMap = meta({ p: { type: 'progress', min: 0, max: 1 } });
+ expect(firstConstraintViolation({ p: 42 }, metaMap)).toBeNull();
+ });
+});

packages/rest/src/import-coerce.ts

@@ -425,6 +425,84 @@ export function firstMissingRequiredField(
  return null;
 }
 
+// ── field-constraint pre-check ─────────────────────────────────────
+
+/**
+ * Number field types the engine bound-checks with `min` / `max`, and string
+ * field types it bound-checks with `minLength` / `maxLength`.
+ *
+ * These are the engine's OWN lists (objectql `record-validator.ts`
+ * `validateOne`), deliberately NOT the spec's `NUMERIC_VALUE_TYPES` /
+ * `STRING_VALUE_TYPES` — those are wider (`progress`, `summary`, `color`,
+ * `signature`, …) and the engine leaves their values unchecked. Using the
+ * wider sets here would make the dry run reject rows the real write accepts,
+ * trading a false all-clear for a false alarm.
+ */
+const BOUNDED_NUMBER_TYPES = new Set(['number', 'currency', 'percent', 'rating', 'slider']);
+const BOUNDED_STRING_TYPES = new Set([
+ 'text', 'textarea', 'email', 'url', 'phone', 'password', 'markdown', 'html', 'richtext', 'code',
+]);
+
+/**
+ * The first declared bound a coerced row violates, or `null` when every
+ * supplied value is in range.
+ *
+ * Mirrors the numeric-range and string-length rules of the engine's
+ * `validateRecord` — same type applicability, same comparison, same `code` and
+ * `message` text — so the import's dry run predicts the verdict the real write
+ * produces (framework#3956). Before this, a dry run only reported *coercion*
+ * failures (a cell that isn't a number at all), so `-500` in a `min: 0` column
+ * was reported valid and then rejected by the write with
+ * `penalty_amount must be ≥ 0`.
+ *
+ * Applies to CREATE and UPDATE alike: the engine validates every supplied
+ * value on both, and a value the row doesn't carry is skipped here exactly as
+ * `validateOne` skips a missing one.
+ *
+ * NOT a complete mirror of `validateRecord`, and not meant to be — format
+ * checks (email/url/phone), object-level `validations` rules, uniqueness and
+ * the state machine still surface only on the real write. Closing those means
+ * validating through the engine itself rather than growing this copy.
+ */
+export function firstConstraintViolation(
+ data: Record<string, unknown>,
+ metaMap: Map<string, ExportFieldMeta>,
+): FieldCoerceError | null {
+ for (const meta of metaMap.values()) {
+ if (meta.system || meta.readonly) continue;
+ if (REQUIRED_CHECK_SKIP.has(meta.name)) continue;
+ const value = data[meta.name];
+ if (isBlankValue(value)) continue; // absent → nothing to bound-check
+ const t = meta.type ?? '';
+
+ if (BOUNDED_NUMBER_TYPES.has(t)) {
+ const n = typeof value === 'number' ? value : Number(value);
+ if (!Number.isFinite(n)) continue; // a non-number is coerceRow's verdict, not ours
+ if (meta.min !== undefined && n < meta.min) {
+ return { field: meta.name, code: 'min_value', message: `${meta.name} must be ≥ ${meta.min}` };
+ }
+ if (meta.max !== undefined && n > meta.max) {
+ return { field: meta.name, code: 'max_value', message: `${meta.name} must be ≤ ${meta.max}` };
+ }
+ continue;
+ }
+
+ if (BOUNDED_STRING_TYPES.has(t)) {
+ // `String(value)` matches the engine, which stringifies a non-string
+ // (e.g. a `multiple` text cell joined by the export separator) before
+ // measuring it.
+ const s = typeof value === 'string' ? value : String(value);
+ if (meta.maxLength !== undefined && s.length > meta.maxLength) {
+ return { field: meta.name, code: 'max_length', message: `${meta.name} must be ≤ ${meta.maxLength} characters (got ${s.length})` };
+ }
+ if (meta.minLength !== undefined && s.length < meta.minLength) {
+ return { field: meta.name, code: 'min_length', message: `${meta.name} must be ≥ ${meta.minLength} characters (got ${s.length})` };
+ }
+ }
+ }
+ return null;
+}
+
 /**
  * Coerce a whole raw row into a storage-ready record. Unknown columns (no
  * matching field metadata) pass through untouched so ad-hoc / schemaless

packages/rest/src/import-integration.test.ts

@@ -125,6 +125,9 @@ const MEMBER = {
  name: 'tier', type: 'select' as const, label: 'Tier', required: true, defaultValue: 'standard',
  options: [{ label: 'Standard', value: 'standard' }, { label: 'Gold', value: 'gold' }],
  },
+ // framework#3956 — bounded fields. The dry run used to ignore both.
+ penalty_amount: { name: 'penalty_amount', type: 'number' as const, label: '处罚金额', min: 0, max: 9999999.99 },
+ nickname: { name: 'nickname', type: 'text' as const, label: 'Nickname', maxLength: 5 },
  },
 };
 
@@ -435,6 +438,52 @@ describe('import route — required-field dry-run fidelity', () => {
  for (const r of res._json.results) expect(r).toMatchObject({ field: 'member_name', code: 'required' });
  });
 
+ // framework#3956 — the dry run reported `ok:1, created:1` for a row the very
+ // same endpoint then rejected with `penalty_amount must be ≥ 0`, because the
+ // dry-run branch returned before any field-constraint check ran. Unlike the
+ // required pre-check above, this one is NOT gated on `runAutomations`: it runs
+ // on the dry run only, where the engine's own validation is never reached.
+ it('dry run fails a row that violates min/max — same verdict, same message as the real write', async () => {
+ const rows = [{ id: 'p1', member_name: 'Eve', status: 'active', penalty_amount: -500 }];
+
+ const dry = await imp({ format: 'json', dryRun: true, rows });
+ expect(dry._json).toMatchObject({ dryRun: true, total: 1, ok: 0, errors: 1, created: 0 });
+ expect(dry._json.results[0]).toMatchObject({
+ row: 1, ok: false, action: 'failed', field: 'penalty_amount',
+ code: 'min_value', error: 'penalty_amount must be ≥ 0',
+ });
+
+ // The real write reaches the engine's validateRecord and says the same.
+ const real = await imp({ format: 'json', rows });
+ expect(real._json).toMatchObject({ total: 1, ok: 0, errors: 1, created: 0 });
+ expect(real._json.results[0].error).toContain('penalty_amount must be ≥ 0');
+ expect(await engine.findOne('member', { where: { id: 'p1' } })).toBeNull();
+ });
+
+ it('dry run fails an over-long string too (maxLength), and passes in-range rows', async () => {
+ const over = await imp({ format: 'json', dryRun: true, rows: [
+ { id: 'p2', member_name: 'Fay', status: 'active', nickname: 'toolongname' },
+ ] });
+ expect(over._json).toMatchObject({ ok: 0, errors: 1 });
+ expect(over._json.results[0]).toMatchObject({
+ field: 'nickname', code: 'max_length', error: 'nickname must be ≤ 5 characters (got 11)',
+ });
+
+ // Boundary values are legal — the pre-check must not over-reject.
+ const ok = await imp({ format: 'json', dryRun: true, rows: [
+ { id: 'p3', member_name: 'Gus', status: 'active', penalty_amount: 0, nickname: 'exact' },
+ ] });
+ expect(ok._json).toMatchObject({ ok: 1, errors: 0, created: 1 });
+ });
+
+ it('bound-checks update rows as well as creates', async () => {
+ await engine.insert('member', { id: 'p4', member_name: 'Hana', status: 'active', penalty_amount: 10 });
+ const res = await imp({ format: 'json', dryRun: true, writeMode: 'update', matchFields: ['id'],
+ rows: [{ id: 'p4', penalty_amount: -1 }] });
+ expect(res._json).toMatchObject({ ok: 0, errors: 1, updated: 0 });
+ expect(res._json.results[0]).toMatchObject({ field: 'penalty_amount', code: 'min_value' });
+ });
+
  it('required check does not apply to update-mode rows (only the touched fields matter)', async () => {
  await engine.insert('member', { id: 'm6', member_name: 'Dan', status: 'active', tier: 'gold' });
  // writeMode:update on an existing match, touching only member_name — status

packages/rest/src/import-runner.ts

@@ -1,7 +1,7 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
 import { randomUUID } from 'node:crypto';
-import { coerceRow, firstMissingRequiredField, type RefResolver, type RefMatch } from './import-coerce.js';
+import { coerceRow, firstMissingRequiredField, firstConstraintViolation, type RefResolver, type RefMatch } from './import-coerce.js';
 import type { ExportFieldMeta } from './export-format.js';
 import { bulkWrite, withTransientRetry, defaultIsTransientError, type BulkWriteRowResult } from '@objectstack/core';
 
@@ -601,9 +601,23 @@ export function runImport(opts: RunImportOptions): Promise<ImportRunSummary> {
  skipped++;
  results[i] = { row: rowNo, ok: true, action: 'skipped', code: 'NO_MATCH' };
  } else if (dryRun) {
- okCount++;
- if (willUpdate) { updated++; results[i] = { row: rowNo, ok: true, action: 'updated', id: String((existing as any).id ?? '') || undefined }; }
- else { created++; results[i] = { row: rowNo, ok: true, action: 'created' }; }
+ // Field-constraint pre-check — DRY RUN ONLY (framework#3956).
+ // The write path is already covered: the engine's own
+ // `validateRecord` runs there (after beforeInsert hooks) and
+ // produces this exact message, so re-checking here would only add
+ // a pre-hook copy that could reject a row a hook would have made
+ // legal. The dry run has no such backstop — without this it
+ // reported `ok: true` for a row the very same endpoint then
+ // failed with `VALIDATION_FAILED`.
+ const violation = firstConstraintViolation(data, metaMap);
+ if (violation) {
+ errCount++;
+ results[i] = { row: rowNo, ok: false, action: 'failed', field: violation.field, code: violation.code, error: violation.message };
+ } else {
+ okCount++;
+ if (willUpdate) { updated++; results[i] = { row: rowNo, ok: true, action: 'updated', id: String((existing as any).id ?? '') || undefined }; }
+ else { created++; results[i] = { row: rowNo, ok: true, action: 'created' }; }
+ }
  } else if (willUpdate) {
  const target = existing as Record<string, any>;
  let res2: unknown;