fix(core): an import row with several bad cells reports every one in errors (#25582)

.changeset/25582-import-row-every-cell-error.md

@@ -0,0 +1,15 @@
+---
+'@objectstack/core': patch
+---
+
+fix(core): an import row with several bad cells reports every one of them in `errors` (#25582)
+
+Clause-②: yes (widening: a failed import row whose cells did not coerce gains the optional `errors` list, `ImportRowResultSchema.errors`; every existing key keeps its value)
+Metadata-protocol: none
+Ruling-ref: #25582 ruling `6377844879` letter A, proposal 4; the spec seat's fill rule `6434667735`
+
+A row with several bad cells used to report only the first one, so an operator fixed one cell, imported again, and only then saw the next. The row now carries `errors`, one `{ field, code, message }` element per failed cell, in column order. The first-error keys `field`, `code` and `error` are unchanged and name element 0, so a reader of those keys sees the same answer as before.
+
+- It reaches every path that reports rows: the dry run and the commit, on the synchronous `POST /api/v1/data/:object/import` and on the async import job's results.
+- It is filled only from the import's cell-coercion errors (`invalid_date`, `invalid_number`, `invalid_option` and the rest). A row that failed for another reason omits it: the dry run's validate verdict, a refused write, a duplicate or ambiguous match key.
+- `ImportRowResult` (`@objectstack/core`) gains the optional `errors?: ValidateDataIssue[]`.

packages/core/src/utils/import-runner-cell-errors.test.ts

@@ -0,0 +1,131 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#25582, maintainer ruling 6377844879 letter A, proposal 4] A row with
+ * several bad cells reports every one of them in `errors`, one element per
+ * failed cell in column order, beside the first-error keys `field` / `code` /
+ * `error`, which keep naming element 0 (`ImportRowResultSchema.errors`).
+ *
+ * The list is filled from `coerceRow`'s cell errors only. A row the validate
+ * verdict refuses (the dry run) and a row whose write is refused (the commit,
+ * through `toFailedResult`) carry no `errors`: on those paths `error` is not
+ * element 0's message, and the spec's describe names cell-coercion errors as
+ * the list's only source.
+ *
+ * Driven against an `ImportProtocolLike` double; the route, the async job and
+ * a real engine are pinned in `packages/rest/src/import-row-cell-errors.test.ts`.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import { runImport, type ImportProtocolLike } from './import-runner';
+import type { ExportFieldMeta } from './import-field-meta.js';
+
+type ValidateArgs = Parameters<NonNullable<ImportProtocolLike['validateData']>>[0];
+
+const OBJECT = 'supplier';
+
+const metaMap = new Map<string, ExportFieldMeta>([
+  ['name', { name: 'name', type: 'text', label: 'Name' }],
+  ['opened', { name: 'opened', type: 'date', label: 'Opened' }],
+  ['qty', { name: 'qty', type: 'number', label: 'Qty' }],
+  ['country', { name: 'country', type: 'select', label: 'Country', options: [{ label: 'China', value: 'cn' }] }],
+]);
+
+const baseOpts = {
+  objectName: OBJECT,
+  metaMap,
+  writeMode: 'insert' as const,
+  matchFields: [] as string[],
+  runAutomations: false,
+  trimWhitespace: true,
+  createMissingOptions: false,
+  skipBlankMatchKey: false,
+};
+
+/** Three bad cells, in this column order, around a good one. */
+const BAD_ROW = { opened: 'not a day', name: 'Acme', qty: 'many', country: 'DE' };
+
+function protocol(over: Partial<ImportProtocolLike> = {}): ImportProtocolLike {
+  return {
+    findData: vi.fn(async () => []),
+    createData: vi.fn(async (args: any) => ({ record: { id: 'r1', ...args.data } })),
+    updateData: vi.fn(),
+    validateData: vi.fn(async (args: ValidateArgs) => ({
+      object: OBJECT, mode: args.mode ?? 'insert', valid: true,
+      results: [{ valid: true, errors: [], warnings: [] }],
+      posture: { valueShapeStrict: false, mediaValueShapeStrict: false },
+    })),
+    ...over,
+  };
+}
+
+function expectEveryCellError(row: any) {
+  expect(row, JSON.stringify(row)).toMatchObject({ row: 1, ok: false, action: 'failed' });
+  expect(row.errors.map((e: any) => [e.field, e.code])).toEqual([
+    ['opened', 'invalid_date'],
+    ['qty', 'invalid_number'],
+    ['country', 'invalid_option'],
+  ]);
+  // Element 0 is the first-error keys, and every element is `{ field, code, message }`.
+  expect({ field: row.field, code: row.code, error: row.error })
+    .toEqual({ field: row.errors[0].field, code: row.errors[0].code, error: row.errors[0].message });
+  for (const e of row.errors) expect(Object.keys(e).sort()).toEqual(['code', 'field', 'message']);
+}
+
+describe('[#25582] a row with several bad cells reports every cell error', () => {
+  it('the dry run: every cell error, in column order, element 0 = the first-error keys', async () => {
+    const p = protocol();
+    const s = await runImport({ ...baseOpts, p, dryRun: true, rows: [BAD_ROW, { name: 'Good' }] });
+    expect(s).toMatchObject({ ok: 1, errors: 1 });
+    expectEveryCellError(s.results[0]);
+    // The good row has no list.
+    expect(s.results[1]).not.toHaveProperty('errors');
+  });
+
+  it('the commit: the same report, and the row is never written', async () => {
+    const p = protocol();
+    const s = await runImport({ ...baseOpts, p, dryRun: false, rows: [BAD_ROW] });
+    expect(s).toMatchObject({ ok: 0, errors: 1, created: 0 });
+    expectEveryCellError(s.results[0]);
+    expect(p.createData).not.toHaveBeenCalled();
+  });
+
+  it('one bad cell: a one-element list naming it', async () => {
+    const s = await runImport({ ...baseOpts, p: protocol(), dryRun: true, rows: [{ name: 'Acme', qty: 'many' }] });
+    expect(s.results[0]).toMatchObject({ field: 'qty', code: 'invalid_number' });
+    expect(s.results[0].errors).toEqual([{ field: 'qty', code: 'invalid_number', message: s.results[0].error }]);
+  });
+});
+
+describe('[#25582] a row that failed for another reason carries no `errors`', () => {
+  it('a validate-verdict refusal (dry run)', async () => {
+    const p = protocol({
+      validateData: vi.fn(async (args: ValidateArgs) => ({
+        object: OBJECT, mode: args.mode ?? 'insert', valid: false,
+        results: [{
+          valid: false,
+          errors: [
+            { field: 'name', code: 'required', message: 'Name is required' },
+            { field: 'qty', code: 'min', message: 'Qty must be at least 1' },
+          ],
+          warnings: [],
+        }],
+        posture: { valueShapeStrict: false, mediaValueShapeStrict: false },
+      })),
+    });
+    const s = await runImport({ ...baseOpts, p, dryRun: true, rows: [{ qty: '0' }] });
+    expect(s.results[0]).toMatchObject({ row: 1, ok: false, action: 'failed', field: 'name', code: 'required' });
+    expect(s.results[0]).not.toHaveProperty('errors');
+  });
+
+  it('a refused write (commit, through the create door\'s verdict)', async () => {
+    const refusal = Object.assign(new Error('Name is required'), {
+      statusCode: 400, code: 'VALIDATION_FAILED',
+      fields: [{ field: 'name', code: 'required', message: 'Name is required' }],
+    });
+    const p = protocol({ createData: vi.fn(async () => { throw refusal; }) });
+    const s = await runImport({ ...baseOpts, p, dryRun: false, rows: [{ qty: '3' }] });
+    expect(s.results[0]).toMatchObject({ row: 1, ok: false, action: 'failed' });
+    expect(s.results[0]).not.toHaveProperty('errors');
+  });
+});

packages/core/src/utils/import-runner.ts

@@ -84,6 +84,16 @@ export interface ImportRowResult {
   field?: string;
   error?: string;
   code?: string;
+  /**
+   * [#25582] Every cell error of a failed row, one element per failed cell in
+   * column order, each `{ field, code, message }` (`ImportRowResultSchema.errors`),
+   * so one import names every bad cell instead of one per re-import. Filled
+   * only from `coerceRow`'s cell errors, where `field` / `code` / `error` above
+   * are element 0. ⛔ Never on a validate-verdict row or a refused write: their
+   * `error` is not element 0's message, and the spec names cell errors as the
+   * list's only source.
+   */
+  errors?: ValidateDataIssue[];
   /**
    * Findings this deployment ADMITS rather than rejects — today, ADR-0104
    * value shapes under a warn-first posture (#4633). The row is `ok`: the
@@ -1220,7 +1230,10 @@ export function runImport(opts: RunImportOptions): Promise<ImportRunSummary> {
         if (errors.length > 0) {
           const first = errors[0];
           errCount++;
-          results[i] = { row: rowNo, ok: false, action: 'failed', field: first.field, code: first.code, error: first.message };
+          results[i] = {
+            row: rowNo, ok: false, action: 'failed', field: first.field, code: first.code, error: first.message,
+            errors: errors.map(({ field, code, message }) => ({ field, code, message })),
+          };
         } else if (shared) {
           // [#26133] Another row of this import carries the same key.
           errCount++;

packages/rest/src/import-row-cell-errors.test.ts

@@ -0,0 +1,191 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#25582, maintainer ruling 6377844879 letter A, proposal 4] An import row
+ * with several bad cells reports every one of them, so one import tells the
+ * operator about every bad cell instead of one per re-import. Through the real
+ * routes over a real {@link ObjectQL} + {@link ObjectStackProtocolImplementation}
+ * + SQLite `:memory:`, with the platform's own `sys_import_job`: the
+ * synchronous `POST /data/:object/import` and the async job's
+ * `GET /data/import/jobs/:jobId/results`, each on the dry run and the commit.
+ *
+ * The row carries `errors` (`ImportRowResultSchema.errors`): one element per
+ * failed cell, in column order, each `{ field, code, message }`. The
+ * first-error keys `field` / `code` / `error` stay and name element 0. Measured
+ * at the base (`45de8c6cba`): the same row reported its first cell only, with
+ * no `errors` key, on all four paths.
+ *
+ * A row that failed for another reason omits the key: the dry run's validate
+ * verdict (`required` here) and the commit's refused write (the create door's
+ * `required` verdict) both name a field, but not as a cell error.
+ *
+ * The runner's own cases are `packages/core/src/utils/import-runner-cell-errors.test.ts`.
+ */
+
+import { describe, it, expect, afterEach } from 'vitest';
+import { ObjectQL } from '@objectstack/objectql';
+import { SqlDriver } from '@objectstack/driver-sql';
+import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
+import { SysImportJob } from '@objectstack/platform-objects/audit';
+import { SysMetadataAuditObject, SysMetadataCommitObject, SysMetadataHistoryObject, SysMetadataObject } from '@objectstack/metadata-core';
+import { RestServer } from './rest-server';
+import { installDefaultOrganization } from './default-organization.testkit.js';
+
+const OBJECT = 'supplier_cells_25582';
+
+const SUPPLIER = {
+  name: OBJECT, label: 'Supplier cells 25582', systemFields: false,
+  fields: {
+    id: { name: 'id', type: 'text' as const, primaryKey: true, label: 'ID' },
+    name: { name: 'name', type: 'text' as const, label: 'Name', required: true },
+    opened: { name: 'opened', type: 'date' as const, label: 'Opened' },
+    qty: { name: 'qty', type: 'number' as const, label: 'Qty' },
+    country: {
+      name: 'country', type: 'select' as const, label: 'Country',
+      options: [{ label: '中国', value: 'cn' }, { label: '美国', value: 'us' }],
+    },
+  },
+};
+
+function createMockServer() {
+  const noop = () => {};
+  return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} };
+}
+
+function makeRes() {
+  const res: any = {
+    write: () => true, end: () => {},
+    header: () => res,
+    status: (code: number) => { res._status = code; return res; },
+    json: (body: any) => { res._json = body; return res; },
+  };
+  return res;
+}
+
+const liveEngines: ObjectQL[] = [];
+afterEach(async () => {
+  while (liveEngines.length) {
+    try { await liveEngines.pop()?.destroy(); } catch { /* noop */ }
+  }
+});
+
+async function boot() {
+  const engine = new ObjectQL();
+  liveEngines.push(engine);
+  engine.registerDriver(new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), true);
+  await engine.init();
+  engine.registry.registerObject(SUPPLIER as any);
+  engine.registry.registerObject(SysImportJob as any);
+  await engine.syncSchemas();
+  // A real `single` install's Default Organization: the job row's system
+  // writes derive it (ADR-0131 D9).
+  await installDefaultOrganization(engine);
+  // The route reads the object's field metadata through the protocol, which
+  // reads the stored-metadata family: without it every cell passes through
+  // uncoerced and the engine's validator answers instead.
+  for (const o of [SysMetadataObject, SysMetadataHistoryObject, SysMetadataAuditObject, SysMetadataCommitObject]) {
+    if (!engine.registry.getObject(o.name)) engine.registry.registerObject(o as any);
+  }
+
+  const protocol = new ObjectStackProtocolImplementation(engine as any);
+  const rest = new RestServer(createMockServer() as any, protocol as any, { api: { requireAuth: false } } as any);
+  (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' });
+  rest.registerRoutes();
+  const call = async (method: string, path: string, body: unknown, params: Record<string, string> = { object: OBJECT }): Promise<{ status: number; body: any }> => {
+    const route = rest.getRoutes().find((r: any) => r.method === method && r.path === path);
+    expect(route, `${method} ${path}`).toBeDefined();
+    const res = makeRes();
+    await route!.handler({ params, body, query: {}, headers: {} } as any, res);
+    return { status: res._status ?? 200, body: res._json };
+  };
+
+  /** The synchronous route's per-row report. */
+  const syncRows = async (rows: Array<Record<string, unknown>>, dryRun: boolean) => {
+    const r = await call('POST', '/api/v1/data/:object/import', { format: 'json', rows, dryRun });
+    expect(r.status, JSON.stringify(r.body)).toBe(200);
+    return r.body.results as any[];
+  };
+
+  /** The async job's per-row report, read off the results route once the job is terminal. */
+  const jobRows = async (rows: Array<Record<string, unknown>>, dryRun: boolean) => {
+    const created = await call('POST', '/api/v1/data/:object/import/jobs', { format: 'json', rows, dryRun });
+    expect(created.status, JSON.stringify(created.body)).toBe(201);
+    const jobId = String(created.body.jobId);
+    let progress: { status: number; body: any } | undefined;
+    for (let i = 0; i < 200; i++) {
+      progress = await call('GET', '/api/v1/data/import/jobs/:jobId', undefined, { jobId });
+      if (['succeeded', 'failed', 'cancelled'].includes(progress.body?.status)) break;
+      await new Promise((r) => setTimeout(r, 5));
+    }
+    expect(progress?.body?.status, JSON.stringify(progress?.body)).toBe('succeeded');
+    const results = await call('GET', '/api/v1/data/import/jobs/:jobId/results', undefined, { jobId });
+    return [...(results.body.results as any[])].sort((a, b) => a.row - b.row);
+  };
+
+  return { engine, syncRows, jobRows };
+}
+
+/** Three bad cells around good ones, in this column order. */
+const BAD_ROW = { id: 'b1', name: 'Acme', opened: '2011-13-45', qty: 'many', country: 'DE' };
+const GOOD_ROW = { id: 'g1', name: 'Good', opened: '2011-07-01', qty: '3', country: 'cn' };
+
+function expectEveryCellError(row: any) {
+  expect(row, JSON.stringify(row)).toMatchObject({ row: 1, ok: false, action: 'failed' });
+  expect(row.errors?.map((e: any) => [e.field, e.code]), JSON.stringify(row)).toEqual([
+    ['opened', 'invalid_date'],
+    ['qty', 'invalid_number'],
+    ['country', 'invalid_option'],
+  ]);
+  for (const e of row.errors) {
+    expect(Object.keys(e).sort()).toEqual(['code', 'field', 'message']);
+    expect(typeof e.message).toBe('string');
+  }
+  // The first-error keys are element 0, as they were before the list.
+  expect({ field: row.field, code: row.code, error: row.error })
+    .toEqual({ field: row.errors[0].field, code: row.errors[0].code, error: row.errors[0].message });
+}
+
+describe('[#25582] every cell error of a row reaches the report', () => {
+  for (const dryRun of [true, false]) {
+    const leg = dryRun ? 'dry run' : 'commit';
+
+    it(`sync route, ${leg}`, async () => {
+      const b = await boot();
+      const rows = await b.syncRows([BAD_ROW, GOOD_ROW], dryRun);
+      expectEveryCellError(rows[0]);
+      expect(rows[1]).toMatchObject({ row: 2, ok: true, action: 'created' });
+      expect(rows[1]).not.toHaveProperty('errors');
+      expect(await b.engine.findOne(OBJECT, { where: { id: 'b1' } })).toBeNull();
+    });
+
+    it(`async job, ${leg}`, async () => {
+      const b = await boot();
+      const rows = await b.jobRows([BAD_ROW, GOOD_ROW], dryRun);
+      expectEveryCellError(rows[0]);
+      expect(rows[1]).toMatchObject({ row: 2, ok: true, action: 'created' });
+      expect(rows[1]).not.toHaveProperty('errors');
+    });
+  }
+});
+
+describe('[#25582] a row that failed for another reason carries no `errors`', () => {
+  /** Every cell coerces; `name` is required and missing. */
+  const NO_NAME = { id: 'n1', qty: '3' };
+
+  it('the dry run\'s validate verdict (sync and async)', async () => {
+    const b = await boot();
+    for (const rows of [await b.syncRows([NO_NAME], true), await b.jobRows([NO_NAME], true)]) {
+      expect(rows[0], JSON.stringify(rows[0])).toMatchObject({ row: 1, ok: false, action: 'failed', field: 'name', code: 'required' });
+      expect(rows[0]).not.toHaveProperty('errors');
+    }
+  });
+
+  it('the commit\'s refused write (sync and async)', async () => {
+    const b = await boot();
+    for (const rows of [await b.syncRows([NO_NAME], false), await b.jobRows([NO_NAME], false)]) {
+      expect(rows[0], JSON.stringify(rows[0])).toMatchObject({ row: 1, ok: false, action: 'failed', field: 'name', code: 'required' });
+      expect(rows[0]).not.toHaveProperty('errors');
+    }
+    expect(await b.engine.findOne(OBJECT, { where: { id: 'n1' } })).toBeNull();
+  });
+});