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

The same request body, with only `dryRun` flipped, produced two different
verdicts. A `number` field declaring `min: 0` received `-500`: the dry run
answered `ok:1, errors:0, created:1` — the Console wizard renders that as
「全部 1 行均有效」 — and the real write then answered `ok:0, errors:1,
penalty_amount must be ≥ 0`, dropping the row. A pre-check that cannot
predict the write is worse than no pre-check: it turns "your file has a
problem" into a false all-clear, and reviewers can't use it as evidence.

The dry-run branch in `runImport` returned before any field-constraint
check ran. Only two gates stood in front of it, and neither looks at a
declared bound:

  - `coerceRow` — pure value conversion (is this cell a number at all,
    does this select option exist, does this lookup resolve). `-500` is a
    perfectly good number, so it sailed through.
  - `firstMissingRequiredField` — required-presence only.

Everything the engine's `validateRecord` enforces (numeric range, string
length, formats) lives past that branch, on the write path only.

This closes the range/length half:

  - `ExportFieldMeta` now carries `min` / `max` / `minLength` /
    `maxLength`. The projection built by `buildFieldMetaMap` was dropping
    them, so the runner could not have checked a bound even if it wanted
    to.
  - `firstConstraintViolation` mirrors `validateRecord`'s numeric-range
    and string-length rules — same type applicability, same comparison,
    same `code` and `message` text — so both paths now report a violation
    identically.
  - The runner consults it on the DRY RUN ONLY. The write path already
    has the engine's own validation, which runs AFTER beforeInsert hooks;
    a pre-hook copy there could reject a row a hook would have made
    legal. The dry run has no such backstop.

Deliberately not a full mirror. 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 (a `validateOnly` write path, which `BatchOptions.validateOnly`
already declares but nothing implements) rather than growing this copy.
The bounded-type lists are the engine's own, not the spec's wider
`NUMERIC_VALUE_TYPES` / `STRING_VALUE_TYPES`: `progress` and `summary`
are numeric per the spec but unchecked by the engine, and using the wider
set would trade the false all-clear for a false alarm.

Covers all three consumers of the shared runner: the synchronous import
route, the async import-job worker, and plugin-auth's user import.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: 2 people

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;