fix(objectql): enforce array shape for multi-value fields in the write pipeline

.changeset/multivalue-field-shape.md

@@ -0,0 +1,5 @@
+---
+"@objectstack/objectql": patch
+---
+
+Enforce array shape for multi-value fields in the write pipeline (#2552). Lone scalars sent at a `multiselect` / `checkboxes` / `tags` field — or at a `select` / `radio` / `lookup` / `user` / `file` / `image` field flagged `multiple: true` — are now normalized into single-element arrays before validation instead of being stored verbatim (which silently corrupted the column shape), un-wrappable shapes are rejected with a new `invalid_type` validation code, and a legal array at a `select`+`multiple` field is no longer mis-rejected as `invalid_option`.

packages/objectql/src/engine-multivalue-normalize.test.ts

@@ -0,0 +1,171 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { ObjectQL } from './engine';
+import { SchemaRegistry } from './registry';
+
+/**
+ * #2552 — multi-value fields must reach the driver as ARRAYS.
+ *
+ * The write pipeline used to pass a lone scalar for a multiselect /
+ * tags / select+multiple / lookup+multiple field straight through to the
+ * driver (`PATCH { labels: "frontend" }` → stored verbatim as a string),
+ * silently corrupting the column shape for every array-consumer. The
+ * engine now normalizes unambiguous scalars into single-element arrays
+ * BEFORE validation, and validation rejects any remaining non-array
+ * shape with `invalid_type` instead of letting it hit storage.
+ *
+ * These tests assert on what the DRIVER receives — the actual corruption
+ * point — not just on validator return values.
+ */
+vi.mock('./registry', () => {
+  const instance: any = {
+    getObject: vi.fn(),
+    resolveObject: vi.fn((n: string) => instance.getObject(n)),
+    registerObject: vi.fn(),
+    getObjectOwner: vi.fn(),
+    registerNamespace: vi.fn(),
+    registerKind: vi.fn(),
+    registerItem: vi.fn(),
+    registerApp: vi.fn(),
+    installPackage: vi.fn(),
+    reset: vi.fn(),
+    metadata: { get: vi.fn(() => new Map()) },
+  };
+  function SchemaRegistry() {
+    return instance;
+  }
+  Object.assign(SchemaRegistry, instance);
+  return {
+    SchemaRegistry,
+    computeFQN: (_ns: string | undefined, name: string) => name,
+    parseFQN: (fqn: string) => ({ namespace: undefined, shortName: fqn }),
+    RESERVED_NAMESPACES: new Set(['base', 'system']),
+  };
+});
+
+const PROJECT_SCHEMA = {
+  name: 'project',
+  fields: {
+    name: { type: 'text' },
+    labels: { type: 'multiselect', options: ['frontend', 'backend', 'design'] },
+    tags: { type: 'tags' },
+    channels: { type: 'select', multiple: true, options: ['email', 'sms'] },
+    related_docs: { type: 'lookup', multiple: true, reference_to: 'document' },
+    // Field.user expands to type 'user' at runtime — the showcase
+    // team_members field ships exactly this shape (#2552 e2e regression).
+    team_members: { type: 'user', multiple: true, reference: 'sys_user' },
+    status: { type: 'select', options: ['active', 'done'] },
+  },
+};
+
+function makeDriver() {
+  const created: any[] = [];
+  const updated: any[] = [];
+  const driver: any = {
+    name: 'memory',
+    supports: {},
+    connect: vi.fn().mockResolvedValue(undefined),
+    disconnect: vi.fn().mockResolvedValue(undefined),
+    find: vi.fn().mockResolvedValue([]),
+    findOne: vi.fn().mockResolvedValue({ id: 'r1' }),
+    create: vi.fn(async (_obj: string, row: any) => {
+      created.push(row);
+      return { id: 'r1', ...row };
+    }),
+    update: vi.fn(async (_obj: string, id: string, row: any) => {
+      updated.push(row);
+      return { id, ...row };
+    }),
+    delete: vi.fn(),
+  };
+  return { driver, created, updated };
+}
+
+async function makeEngine(driver: any) {
+  vi.mocked((SchemaRegistry as any).getObject).mockImplementation((name: string) =>
+    name === 'project' ? PROJECT_SCHEMA : undefined,
+  );
+  const ql = new ObjectQL();
+  ql.registerDriver(driver, true);
+  await ql.init();
+  return ql;
+}
+
+describe('engine write pipeline — multi-value scalar normalization (#2552)', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it('insert: wraps scalars so the driver receives arrays', async () => {
+    const { driver, created } = makeDriver();
+    const ql = await makeEngine(driver);
+    await ql.insert('project', {
+      name: 'P1',
+      labels: 'frontend',
+      tags: 'urgent',
+      channels: 'email',
+      related_docs: 'doc-1',
+      team_members: 'user-1',
+    });
+    expect(created).toHaveLength(1);
+    expect(created[0]).toMatchObject({
+      labels: ['frontend'],
+      tags: ['urgent'],
+      channels: ['email'],
+      related_docs: ['doc-1'],
+      team_members: ['user-1'],
+    });
+  });
+
+  it('update: wraps scalars so the driver receives arrays', async () => {
+    const { driver, updated } = makeDriver();
+    const ql = await makeEngine(driver);
+    await ql.update('project', { id: 'r1', labels: 'design', team_members: 'user-2' });
+    expect(updated).toHaveLength(1);
+    expect(updated[0]).toMatchObject({ labels: ['design'], team_members: ['user-2'] });
+  });
+
+  it('update: legal arrays pass through untouched (select+multiple was mis-rejected before)', async () => {
+    const { driver, updated } = makeDriver();
+    const ql = await makeEngine(driver);
+    await ql.update('project', {
+      id: 'r1',
+      labels: ['frontend', 'design'],
+      channels: ['email', 'sms'],
+      related_docs: ['d1', 'd2'],
+      team_members: ['u1', 'u2'],
+    });
+    expect(updated[0]).toMatchObject({
+      labels: ['frontend', 'design'],
+      channels: ['email', 'sms'],
+      related_docs: ['d1', 'd2'],
+      team_members: ['u1', 'u2'],
+    });
+  });
+
+  it('update: un-wrappable junk is rejected BEFORE reaching the driver', async () => {
+    const { driver, updated } = makeDriver();
+    const ql = await makeEngine(driver);
+    await expect(
+      ql.update('project', { id: 'r1', labels: { nested: true } }),
+    ).rejects.toThrow(/invalid_type/i);
+    expect(updated).toHaveLength(0);
+  });
+
+  it('insert: invalid option inside a wrapped scalar still 400s', async () => {
+    const { driver, created } = makeDriver();
+    const ql = await makeEngine(driver);
+    await expect(ql.insert('project', { name: 'P2', labels: 'nope' })).rejects.toThrow(
+      /invalid_option/i,
+    );
+    expect(created).toHaveLength(0);
+  });
+
+  it('single-value fields are untouched', async () => {
+    const { driver, created } = makeDriver();
+    const ql = await makeEngine(driver);
+    await ql.insert('project', { name: 'P3', status: 'active' });
+    expect(created[0].status).toBe('active');
+  });
+});

packages/objectql/src/engine.ts

@@ -30,7 +30,7 @@ import { ExpressionEngine } from '@objectstack/formula';
 import type { Expression } from '@objectstack/spec';
 import { isAggregatedViewContainer, expandViewContainer } from '@objectstack/spec';
 import { bindHooksToEngine } from './hook-binder.js';
-import { validateRecord } from './validation/record-validator.js';
+import { validateRecord, normalizeMultiValueFields } from './validation/record-validator.js';
 import { evaluateValidationRules, needsPriorRecord, stripReadonlyWhenFields } from './validation/rule-validator.js';
 import { applyInMemoryAggregation } from './in-memory-aggregation.js';
 
@@ -2129,6 +2129,7 @@ export class ObjectQL implements IDataEngine {
             await this.encryptSecretFields(object, r, opCtx.context, hookContext.input.options);
           }
           for (const r of rows) {
+            normalizeMultiValueFields(schemaForValidation, r);
             validateRecord(schemaForValidation, r, 'insert');
             evaluateValidationRules(schemaForValidation as any, r, 'insert', { logger: this.logger });
           }
@@ -2147,6 +2148,7 @@ export class ObjectQL implements IDataEngine {
           );
           await this.applyAutonumbers(object, row, opCtx.context, driverOwnsAutonumber);
           await this.encryptSecretFields(object, row, opCtx.context, hookContext.input.options);
+          normalizeMultiValueFields(schemaForValidation, row);
           validateRecord(schemaForValidation, row, 'insert');
           evaluateValidationRules(schemaForValidation as any, row, 'insert', { logger: this.logger });
           result = await driver.create(object, row, hookContext.input.options as any);
@@ -2263,6 +2265,7 @@ export class ObjectQL implements IDataEngine {
            const updateSchema = this._registry.getObject(object);
            if (hookContext.input.id) {
                await this.encryptSecretFields(object, hookContext.input.data as Record<string, unknown>, opCtx.context, hookContext.input.options);
+               normalizeMultiValueFields(updateSchema, hookContext.input.data as Record<string, unknown>);
                validateRecord(updateSchema, hookContext.input.data as Record<string, unknown>, 'update');
                if (needsPriorRecord(updateSchema as any) || (this.hooks.get('afterUpdate')?.length ?? 0) > 0) {
                    const priorAst: QueryAST = { object, where: { id: hookContext.input.id }, limit: 1 };
@@ -2276,6 +2279,7 @@ export class ObjectQL implements IDataEngine {
                result = await driver.update(object, hookContext.input.id as string, hookContext.input.data as Record<string, unknown>, hookContext.input.options as any);
            } else if (options?.multi && driver.updateMany) {
                await this.encryptSecretFields(object, hookContext.input.data as Record<string, unknown>, opCtx.context, hookContext.input.options);
+               normalizeMultiValueFields(updateSchema, hookContext.input.data as Record<string, unknown>);
                validateRecord(updateSchema, hookContext.input.data as Record<string, unknown>, 'update');
                // Multi-row update: per-row prior state is not fetched (one query
                // per matched row would be unbounded). state_machine /

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

@@ -1,7 +1,7 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
 import { describe, it, expect } from 'vitest';
-import { validateRecord } from './record-validator.js';
+import { validateRecord, normalizeMultiValueFields } from './record-validator.js';
 
 /**
  * Required-field validation, with the autonumber exemption (#1603).
@@ -65,3 +65,127 @@ describe('validateRecord — time field accepts time-of-day', () => {
     expect(() => validateRecord(ds, { d: 'not-a-date' }, 'insert')).toThrow(/invalid_date/i);
   });
 });
+
+/**
+ * Multi-value field shape enforcement + scalar normalization (#2552).
+ *
+ * A multiselect (and every other array-shaped field) used to accept a lone
+ * scalar and store it VERBATIM — `PATCH { labels: "frontend" }` returned 200
+ * and read back as a string, corrupting the column for every consumer that
+ * expects an array (found via the console bulk-edit dialog, which pre-#2186
+ * sent scalars for multi params). `select`+`multiple` was worse: a legal
+ * ARRAY was stringified to "a,b" and rejected as invalid_option.
+ */
+describe('normalizeMultiValueFields — scalar → single-element array', () => {
+  const schema = {
+    fields: {
+      labels: { type: 'multiselect', options: ['frontend', 'backend', 'design'] },
+      tags: { type: 'tags' },
+      channels: { type: 'select', multiple: true, options: ['email', 'sms'] },
+      team_members: { type: 'lookup', multiple: true },
+      // Field.user expands to type 'user' at runtime (NOT 'lookup') — the
+      // showcase team_members regression that motivated widening the type set.
+      watchers: { type: 'user', multiple: true, reference: 'sys_user' },
+      attachments: { type: 'file', multiple: true },
+      status: { type: 'select', options: ['active', 'done'] },
+      owner: { type: 'lookup' },
+      assignee: { type: 'user' },
+    },
+  };
+
+  it('wraps a scalar for multiselect / tags / select+multiple / lookup+multiple / user+multiple / file+multiple', () => {
+    const data: Record<string, unknown> = {
+      labels: 'frontend',
+      tags: 'urgent',
+      channels: 'email',
+      team_members: 'user-1',
+      watchers: 'user-2',
+      attachments: 'file-key-1',
+    };
+    normalizeMultiValueFields(schema, data);
+    expect(data).toEqual({
+      labels: ['frontend'],
+      tags: ['urgent'],
+      channels: ['email'],
+      team_members: ['user-1'],
+      watchers: ['user-2'],
+      attachments: ['file-key-1'],
+    });
+  });
+
+  it('leaves arrays, null/undefined, and single-value fields untouched', () => {
+    const data: Record<string, unknown> = {
+      labels: ['frontend', 'design'],
+      tags: null,
+      status: 'active',
+      owner: 'user-1',
+      assignee: 'user-2',
+    };
+    normalizeMultiValueFields(schema, data);
+    expect(data).toEqual({
+      labels: ['frontend', 'design'],
+      tags: null,
+      status: 'active',
+      owner: 'user-1',
+      assignee: 'user-2',
+    });
+  });
+
+  it('does NOT wrap non-scalar junk (left for validateRecord to reject)', () => {
+    const data: Record<string, unknown> = { labels: { nested: true } };
+    normalizeMultiValueFields(schema, data);
+    expect(data.labels).toEqual({ nested: true });
+  });
+});
+
+describe('validateRecord — multi-value fields must be arrays', () => {
+  const schema = {
+    fields: {
+      labels: { type: 'multiselect', options: ['frontend', 'backend'] },
+      tags: { type: 'tags' },
+      channels: { type: 'select', multiple: true, options: ['email', 'sms'] },
+      team_members: { type: 'lookup', multiple: true },
+      watchers: { type: 'user', multiple: true, reference: 'sys_user' },
+      attachments: { type: 'file', multiple: true },
+      status: { type: 'select', options: ['active', 'done'] },
+    },
+  };
+
+  it('rejects a raw (un-normalized) scalar with invalid_type', () => {
+    for (const payload of [
+      { labels: 'frontend' },
+      { tags: 'urgent' },
+      { channels: 'email' },
+      { team_members: 'user-1' },
+      { watchers: 'user-1' },
+      { attachments: 'file-key-1' },
+    ]) {
+      expect(() => validateRecord(schema, payload, 'update')).toThrow(/invalid_type/i);
+    }
+  });
+
+  it('rejects a plain-object shape with invalid_type', () => {
+    expect(() => validateRecord(schema, { labels: { nested: true } }, 'update')).toThrow(/invalid_type/i);
+    expect(() => validateRecord(schema, { team_members: { id: 'u1' } }, 'update')).toThrow(/invalid_type/i);
+  });
+
+  it('accepts arrays (including for select+multiple, previously mis-rejected)', () => {
+    expect(() =>
+      validateRecord(
+        schema,
+        { labels: ['frontend'], tags: ['a', 'b'], channels: ['email', 'sms'], team_members: ['u1', 'u2'], watchers: ['u1'], attachments: ['k1', 'k2'] },
+        'update',
+      ),
+    ).not.toThrow();
+  });
+
+  it('still validates array ELEMENTS against options', () => {
+    expect(() => validateRecord(schema, { labels: ['nope'] }, 'update')).toThrow(/invalid_option/i);
+    expect(() => validateRecord(schema, { channels: ['fax'] }, 'update')).toThrow(/invalid_option/i);
+  });
+
+  it('does NOT regress single select / radio', () => {
+    expect(() => validateRecord(schema, { status: 'active' }, 'update')).not.toThrow();
+    expect(() => validateRecord(schema, { status: 'nope' }, 'update')).toThrow(/invalid_option/i);
+  });
+});