diff --git a/.changeset/18199-multi-valued-invariant-cli.md b/.changeset/18199-multi-valued-invariant-cli.md new file mode 100644 index 00000000000..a09c1d6c2de --- /dev/null +++ b/.changeset/18199-multi-valued-invariant-cli.md @@ -0,0 +1,21 @@ +--- +"@objectstack/cli": patch +--- + +`os generate migration` and `os generate types` ask the ONE definition of "is this field multi-valued" — `isMultiValueField` in `@objectstack/spec` — instead of reading `field.multiple` raw, so the DDL they scaffold is the DDL `driver-sql` creates for the same object again (#18199). + +Clause-②: no — no schema key moves, no accept set widens or narrows, no export changes. The generators' inputs and outputs keep their shapes; what changes is which predicate decides one branch inside them. + +The maintainer ruling of 2026-09-13 (decision batch #128 item 5, option 1′) gave "multi-valued" one definition and made storage follow it. #17469 landed the `driver-sql` half — `createColumn` short-circuits on the spec predicate above its own type switch, `isJsonField` and `fieldHasColumn` derive from it — and left `packages/cli` reading the flag. For one release the two answered differently, which is #14829 ("the platform and the GENERATED DDL as two lists") in reverse: + +| declaration | `os generate migration` before | `driver-sql` | now | +|---|---|---|---| +| `{ type: 'text', multiple: true }` | `JSONB` / `table.jsonb` | `TEXT` | `TEXT` / `table.text` | +| `{ type: 'lookup', multiple: true }` | `JSONB` / `table.jsonb` | JSON column | unchanged | + +Two further shapes moved with it, both the same raw read: + +- **`os generate types` stops emitting a nested array for a redundantly-flagged option type.** `multiple: true` is accepted (redundantly) on `multiselect` / `checkboxes` / `tags`, and the generated property type was `string[][]`; it is `string[]` now, which is what the value contract says and what the platform stores. +- **A column DEFAULT is no longer withheld from a single-value field that carries the flag.** `{ type: 'text', multiple: true, defaultValue: 'x' }` emitted a column with no DEFAULT while the driver emits `DEFAULT 'x'`. + +⚠️ These declarations are refused at the authoring entrance by the same ruling's `FieldSchema` change, so they reach the generators only through the doors that never run it (`registerExternalObject` / `initObjects`, and a hand-written config the generators read unvalidated). Reachable, not authorable — which is why this is a `patch` and not a break. diff --git a/content/docs/protocol/objectql/types.mdx b/content/docs/protocol/objectql/types.mdx index 1ce5c6e1c64..d5973f26078 100644 --- a/content/docs/protocol/objectql/types.mdx +++ b/content/docs/protocol/objectql/types.mdx @@ -1049,7 +1049,11 @@ tags: There is no generic `array` field type. To store multiple values, use `tags` (free-form strings), `multiselect` (multiple choices from `options`), or set -`multiple: true` on a scalar/`lookup` field to store an array of that type. +`multiple: true` on one of the **multi-capable** types — `select`, `lookup`, +`user`, `file`, `image` — to store an array of that type. `multiple: true` on +any other type (`text`, `number`, a date type, `radio`) is **refused when the +field is parsed**, naming the alternative to use instead: a cell holding several +values at once is declared only on the types whose storage has an array form. **Storage:** @@ -1258,10 +1262,17 @@ affinity there and SQLite still accepts a fractional value as a REAL. The refusal `rating` gains is a PostgreSQL/MySQL effect: PostgreSQL refuses a fractional star count outright, and MySQL **rounds** it (4.5 arrives as 5). -Any field flagged `multiple: true` becomes a `JSON` column regardless of its -type. Relationship columns are plain id strings with no database `FOREIGN KEY` -constraint (see `lookup` above). The MongoDB driver is schemaless — it issues no -DDL and stores the value it is given. +A **multi-valued** field becomes a `JSON` column whatever its element type would +have been — the flag is answered before the type is looked up at all. "Multi-valued" +has one definition, `isMultiValueField` in `packages/spec`, and every producer of a +column reads it: `SqlDriver.createColumn`, the read-side deserializer, the drift +detector and both `os generate migration` formats. It is true for an inherently-multi +option type (`multiselect` / `checkboxes` / `tags`) with or without the flag, and for +one of the multi-capable types (`select`, `lookup`, `user`, `file`, `image`) carrying +`multiple: true`; `multiple: true` on any other type is refused when the field is +parsed, so no other declaration reaches this rule. Relationship columns are plain id +strings with no database `FOREIGN KEY` constraint (see `lookup` above). The MongoDB +driver is schemaless — it issues no DDL and stores the value it is given. There is no Redis persistence backend. ObjectQL's data drivers are SQL diff --git a/packages/cli/src/commands/generate-field-type-vocabulary.pin.test.ts b/packages/cli/src/commands/generate-field-type-vocabulary.pin.test.ts index ef9f5a8bf2e..983bd41b28b 100644 --- a/packages/cli/src/commands/generate-field-type-vocabulary.pin.test.ts +++ b/packages/cli/src/commands/generate-field-type-vocabulary.pin.test.ts @@ -86,9 +86,15 @@ * listed here), asserted against what the generators actually EMIT; * - the DRIVER'S OWN SOURCE, read where it lives, for the questions the spec * does not answer — which types are virtual, and what a reference column's - * physical shape is. ⛔ The driver is the authority for which column - * exists; the spec's `isMultiValueField` is the ADR-0104 D1 VALUE contract - * and answers a different question (#14829's pin argues this in full); + * physical shape is. The driver is the authority for which column exists. + * ⭐ [#18199] That clause used to continue "; the spec's `isMultiValueField` + * is the ADR-0104 D1 VALUE contract and answers a different question + * (#14829's pin argues this in full)", and the second half stopped being + * true: the maintainer ruling of 2026-09-13 (decision batch #128 item 5, + * option 1′) gave "multi-valued" ONE definition, #17469 derived all three + * driver sites from it, and #18199 derived `generate.ts` from it too. The + * authority is unchanged — asking `isMultiValueField` about multi-value IS + * asking the driver's own rule now; * - or the file's INTERNAL agreement — an array-typed answer in * `FIELD_TYPE_MAP` and a scalar column in `FIELD_TYPE_SQL_MAP` is a * contradiction whoever is right, and `autonumber` was exactly that. diff --git a/packages/cli/src/commands/generate-multiple-json-column.pin.test.ts b/packages/cli/src/commands/generate-multiple-json-column.pin.test.ts index 8ffce0b57ee..669d0c31cef 100644 --- a/packages/cli/src/commands/generate-multiple-json-column.pin.test.ts +++ b/packages/cli/src/commands/generate-multiple-json-column.pin.test.ts @@ -49,22 +49,29 @@ * `MULTI_CAPABLE_TYPES` ∪ `MULTI_OPTION_TYPES`, and all three driver sites now * derive from `isMultiValueField`. * - * ⚠️ So the two halves this file exists to hold together have SPLIT on the - * retired shapes, and the split is real and is recorded rather than papered - * over: `packages/cli/src/commands/generate.ts` still reads `field.multiple` - * raw, so `os generate migration` emits JSONB for a `text` field flagged - * `multiple: true` while the driver now emits a varchar for it — #14829 in - * reverse, one notch narrower. It is BOUNDED, because that declaration is - * refused at the authoring entrance and can only reach the generators through - * the unvalidated door they explicitly serve. Aligning `generate.ts` is - * another lane's card and is deliberately NOT done here; the assertions below - * therefore still state what the CLI emits, and the two source-read pins state - * what the driver decides, with this paragraph between them. + * ⭐ [#18199] THE SPLIT THAT PARAGRAPH OPENED IS CLOSED, and this one is the + * record of it. #17469 moved the driver and left `generate.ts` reading + * `field.multiple` raw, so for one release this file carried a paragraph saying + * the two halves it exists to hold together had SPLIT: `os generate migration` + * emitted JSONB for a `text` field flagged `multiple: true` while the driver + * emitted a varchar for it — #14829 in reverse, one notch narrower. All five + * reads in `generate.ts` now go through its own `declaredMultiValued` seam onto + * the same `isMultiValueField`, so the assertions below state ONE answer again + * and the arms that used to record the divergence are inverted rather than + * deleted — a `text` field flagged `multiple: true` is a SCALAR in all three + * CLI surfaces, which is what the driver stores. + * + * ⛔ The bound has NOT moved and is not what was fixed: `FieldSchema` refuses + * that declaration at the authoring entrance, so these readers reach it only + * through the unvalidated door they explicitly serve (`registerExternalObject` + * / `initObjects`, and a hand-written config). What changed is that the two + * answers behind that door are now one. * * `MULTI_CAPABLE_TYPES` is still imported here rather than transcribed — it is * the roster this pin SWEEPS, so a type added to that spec class is measured on - * the day it lands. It is not the implementation's gate, and the type-blindness - * control below is what states the difference as an assertion. + * the day it lands. It is not the implementation's gate; `isMultiValueField` + * is, and the type-blindness control below states the difference between the + * roster and the predicate as an assertion. * * ## Anti-vacuity * @@ -80,7 +87,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; -import { MULTI_CAPABLE_TYPES } from '@objectstack/spec/data'; +import { isMultiValueField, MULTI_CAPABLE_TYPES, MULTI_OPTION_TYPES } from '@objectstack/spec/data'; import { describe, expect, it } from 'vitest'; import { @@ -97,9 +104,31 @@ const DRIVER_SQL_SRC = path.resolve(HERE, '../../../drivers/driver-sql/src'); * restated) plus `text` — a type that is NOT in that roster and whose scalar * answer is a varchar, which is what makes the type-blindness of the rule * assertable rather than merely described. + * + * [#18199] `text` is still swept and is still the probe; what it probes has + * inverted. It used to demonstrate that the generators read the FLAG and not + * the type, by taking a JSON column the spec predicate would have refused; it + * now demonstrates that they read the PREDICATE, by taking the scalar column + * the driver gives it. The membership assertion in the first control is what + * keeps the probe meaningful either way. */ const FLAGGED_TYPES: readonly string[] = [...MULTI_CAPABLE_TYPES, 'text']; +/** + * [#18199] The swept types the ONE definition calls multi-valued when flagged — + * derived from the predicate itself, ⛔ never a second list. `text` drops out + * here and only here, which is the whole delta this card landed. + */ +const MULTI_VALUED_WHEN_FLAGGED: readonly string[] = + FLAGGED_TYPES.filter((type) => isMultiValueField({ type, multiple: true })); + +/** + * [#18199] The swept types it calls SINGLE-valued even flagged — the same + * derivation, complemented, so the two arms below cannot both go empty and pass. + */ +const SINGLE_VALUED_WHEN_FLAGGED: readonly string[] = + FLAGGED_TYPES.filter((type) => !isMultiValueField({ type, multiple: true })); + /** One object carrying, for each swept type, a flagged field and its scalar twin. */ function probeConfig(): Record { const fields: Record> = {}; @@ -144,6 +173,11 @@ describe('#14829 — `multiple: true` is one answer across all three surfaces', // `text` is the type-blindness probe: it must NOT be in the roster, or the // control below stops distinguishing the flag rule from the value rule. expect(MULTI_CAPABLE_TYPES.has('text')).toBe(false); + // [#18199] …and the two derived arms must BOTH be populated, or one of the + // sweeps below is an empty loop that passes while measuring nothing. This + // is the non-vacuity the derivation buys instead of a second hand-list. + expect(MULTI_VALUED_WHEN_FLAGGED.length).toBeGreaterThanOrEqual(6); + expect(SINGLE_VALUED_WHEN_FLAGGED).toEqual(['text']); }); it('control — all three generators really emitted a table for the probe', () => { @@ -178,12 +212,19 @@ describe('#14829 — `multiple: true` is one answer across all three surfaces', expect(sqlColumn('single_text')).toBe('TEXT'); expect(tsColumn('single_text')).toBe("table.text('single_text')"); // …and it still discriminates: the scalar answer is not the JSON one. - expect(sqlColumn('single_text')).not.toBe(sqlColumn('multi_text')); + // + // [#18199] On `lookup`, not `text`. The pair has to straddle the ONE + // definition to discriminate anything, and `text` no longer does — flagged + // or not it is the same scalar column now, which is the fix. `lookup` is + // multi-capable, so `single_` vs `multi_` there is still exactly the + // scalar-vs-JSON contrast this control exists to prove is being measured. + expect(sqlColumn('single_lookup')).not.toBe(sqlColumn('multi_lookup')); + expect(tsColumn('single_lookup')).not.toBe(tsColumn('multi_lookup')); expect(sqlColumn('single_file')).toBe('VARCHAR(2048)'); expect(tsInterfaceType('single_lookup')).toBe('string'); }); - for (const type of FLAGGED_TYPES) { + for (const type of MULTI_VALUED_WHEN_FLAGGED) { it(`${type} + multiple:true — array TS type AND a JSON column in both migrations`, () => { const declared = tsInterfaceType(`multi_${type}`); expect(declared, `os generate types must give a flagged ${type} an array type`) @@ -204,20 +245,102 @@ describe('#14829 — `multiple: true` is one answer across all three surfaces', }); } - it('the flag decides before the type in the GENERATORS — a type outside MULTI_CAPABLE_TYPES too', () => { - // Stated as its own assertion because it is the one place this pin departs - // from the spec's value predicate. + it('[#18199] the PREDICATE decides in the generators — a flagged type outside the roster stays scalar', () => { + // ⭐ THE INVERTED ARM, and the reason it is inverted rather than deleted: + // this is the one row where the flag and the ONE definition disagree, so it + // is the only row that can tell which of the two `generate.ts` is reading. + // + // It used to read "the flag decides before the type in the GENERATORS — a + // type outside MULTI_CAPABLE_TYPES too" and assert JSONB here, because that + // is what the flag rule produced. #17469 made that a DIVERGENCE from the + // driver (which emits a varchar for this declaration) and this file + // recorded it as such; #18199 closed it from the CLI side. // - // ⚠️ [#17469] It is now a DIVERGENCE, not an agreement, and the assertion - // is unchanged for that reason: it states what `generate.ts` emits, which - // this card does not touch. The driver no longer gives this field a JSON - // column — `text` + `multiple: true` is refused at the authoring entrance - // and is a varchar there. Bounded by that refusal; aligning `generate.ts` - // is another lane's card. ⛔ Do not read this row as "the platform stores - // it as JSON" any more. + // ⛔ A future edit that turns these back into JSONB is reinstating the + // second definition of "multi-valued", not fixing a stale expectation. expect(MULTI_CAPABLE_TYPES.has('text')).toBe(false); - expect(sqlColumn('multi_text')).toBe('JSONB'); - expect(tsColumn('multi_text')).toBe("table.jsonb('multi_text')"); + expect(isMultiValueField({ type: 'text', multiple: true })).toBe(false); + expect( + sqlColumn('multi_text'), + 'os generate migration --format sql gave a flagged `text` field a JSON column. The ONE ' + + 'definition of multi-valued (`isMultiValueField`, maintainer ruling 2026-09-13) answers ' + + 'false for it and driver-sql emits a varchar — a JSONB here is #14829 in reverse.', + ).toBe('TEXT'); + expect(tsColumn('multi_text')).toBe("table.text('multi_text')"); + expect( + tsInterfaceType('multi_text'), + 'os generate types called a flagged `text` field an array while both migration formats ' + + 'give it a scalar column — the three CLI surfaces have split again.', + ).toBe('string'); + }); + + it('[#18199] the flag is REDUNDANT on an inherently-multi option type — one level of array, not two', () => { + // The other half of routing the one predicate through `fieldTypeToTs`. + // `isMultiValueField` answers true for `multiselect` / `checkboxes` / `tags` + // with or WITHOUT the flag, while `FIELD_TYPE_MAP`'s entry for those is + // already the array (`string[]`). Wrapping the predicate's verdict around + // that table a second time emits `string[][]`, and `FieldSchema` accepts + // the flag on these types (it is redundant there, never refused), so the + // declaration is fully authorable — measured `multiselect?: string[][]` + // before this card. + // + // Swept off `MULTI_OPTION_TYPES` rather than listed, for the same reason + // every other roster here is imported. + expect(MULTI_OPTION_TYPES.size).toBeGreaterThanOrEqual(3); + const fields: Record> = {}; + for (const type of MULTI_OPTION_TYPES) { + fields[`flagged_${type}`] = { type, multiple: true }; + fields[`bare_${type}`] = { type }; + } + const out = generateTypesFromConfig({ objects: { probe: { name: 'probe', fields } } }); + // Non-vacuity: the interface really emitted, and the bare twin is the + // control — if BOTH read `string[][]` this assertion would be blind to the + // double wrap it exists to catch. + expect(out).toContain('export interface ProbeRecord {'); + for (const type of MULTI_OPTION_TYPES) { + const flagged = out.match(new RegExp(`^ {2}flagged_${type}\\??: (.+);$`, 'm')); + const bare = out.match(new RegExp(`^ {2}bare_${type}\\??: (.+);$`, 'm')); + expect(flagged, `no interface member emitted for flagged_${type}`).not.toBeNull(); + expect(bare, `no interface member emitted for bare_${type}`).not.toBeNull(); + expect(flagged?.[1], `a redundantly-flagged ${type} was emitted as a nested array`) + .toBe(bare?.[1]); + expect(flagged?.[1]).not.toMatch(/\[\]\[\]$/); + expect(flagged?.[1]).toMatch(/\[\]$/); + } + }); + + it('[#18199] every swept declaration agrees with `isMultiValueField` on all three CLI surfaces', () => { + // The invariant this card exists to restore, stated once as an assertion + // instead of being spread across the arms above: for each swept type, the + // TS property type is an array exactly when the ONE definition says the + // value is one, and both migration formats give it a JSON column on exactly + // the same rows. + // + // Non-vacuity: the sweep is the union of the two derived arms, and the + // first control has already pinned that neither is empty; the counts are + // re-asserted here so this loop cannot silently become one. + const swept = [...MULTI_VALUED_WHEN_FLAGGED, ...SINGLE_VALUED_WHEN_FLAGGED]; + expect(swept.length).toBe(FLAGGED_TYPES.length); + let arrays = 0; + let scalars = 0; + for (const type of swept) { + const multiValued = isMultiValueField({ type, multiple: true }); + const isJsonSql = sqlColumn(`multi_${type}`) === 'JSONB'; + const isJsonTs = tsColumn(`multi_${type}`) === `table.jsonb('multi_${type}')`; + const isArrayTs = /\[\]$/.test(tsInterfaceType(`multi_${type}`)); + expect(isJsonSql, `--format sql disagreed with isMultiValueField for a flagged ${type}`) + .toBe(multiValued); + expect(isJsonTs, `--format typescript disagreed with isMultiValueField for a flagged ${type}`) + .toBe(multiValued); + expect(isArrayTs, `os generate types disagreed with isMultiValueField for a flagged ${type}`) + .toBe(multiValued); + if (multiValued) arrays += 1; + else scalars += 1; + } + // Both outcomes really occurred, so "they all agree" is not "they are all + // the same answer". + expect(arrays).toBeGreaterThan(0); + expect(scalars).toBeGreaterThan(0); }); /** diff --git a/packages/cli/src/commands/generate.ts b/packages/cli/src/commands/generate.ts index 8d8c34a0f75..be774af8162 100644 --- a/packages/cli/src/commands/generate.ts +++ b/packages/cli/src/commands/generate.ts @@ -26,10 +26,20 @@ import { // four helpers beside it — never transcribed — so a type added to the spec is // admitted by these generators on the same commit. FieldType, + // [#18199] THE one definition of "is this field multi-valued" (maintainer + // ruling 2026-09-13, decision batch #128 item 5, option 1′). Imported for + // exactly the reason the block above gives — never transcribed — and read + // through {@link declaredMultiValued}, the single local seam every site in + // this file asks. + isMultiValueField, isNowDefaultToken, isRuntimeDefaultToken, isTenancyDisabled, isUniqueDeclared, + // [#18199] Not a second answer to "is this multi-valued": the roster states + // which types {@link FIELD_TYPE_MAP} has ALREADY spelled as an array, so the + // one predicate's verdict is not applied to that table twice. + MULTI_OPTION_TYPES, numericColumnFor, // #16726 — the name gate below. IMPORTED for the same reason as the five // above: it asks the schema whether a name is legal instead of restating @@ -636,9 +646,55 @@ const FIELD_TYPE_MAP: Record = { address: 'Data.AddressValue', } satisfies Record; -function fieldTypeToTs(fieldType: string, multiple?: boolean): string { +/** + * [#18199] Is this field MULTI-VALUED — the one question, asked of the one + * predicate `@objectstack/spec` publishes. + * + * Maintainer ruling 2026-09-13 (decision batch #128 item 5, option 1′): there + * is ONE definition of "multi-valued", `isMultiValueField`, and storage follows + * it. #17469 moved `driver-sql` onto it — `createColumn` short-circuits on + * `isMultiValuedColumn(...)` above its own `switch (type)`, `isJsonField` is + * `JSON_COLUMN_TYPES.has(type) || isMultiValuedColumn(type, field)`, and + * `schema-drift`'s `fieldHasColumn` opens with `isMultiValueField(...)` — and + * left this file behind on a raw `field.multiple` read. That gap was measurable: + * a `text` field flagged `multiple: true` got JSONB from `os generate migration` + * and a varchar from the driver that actually creates the table, which is #14829 + * ("the platform and the GENERATED DDL as two lists") in reverse. + * + * Takes the RESOLVED type rather than reading `field.type`, for the same reason + * `isMultiValuedColumn` does: every caller here has already resolved it through + * {@link declaredFieldType}, and two spellings of that resolution is how the + * drift this closes started. + * + * ⛔ The predicate is called, never re-spelled. `MULTI_CAPABLE_TYPES` / + * `MULTI_OPTION_TYPES` membership tests written out here would be a second + * answer to a question the ruling gave exactly one. + */ +function declaredMultiValued(fieldType: string, field: unknown): boolean { + const declaring = field as { multiple?: unknown } | null | undefined; + return isMultiValueField({ type: fieldType, multiple: declaring?.multiple === true }); +} + +/** + * The generated TypeScript property type for one field. + * + * [#18199] The second argument is {@link declaredMultiValued}'s verdict, not a + * raw `field.multiple`. It is spelled `multiValued` rather than `multiple` so a + * future caller cannot hand it the flag again without noticing. + * + * ⚠️ {@link FIELD_TYPE_MAP}'s entry is the WHOLE value type, not an element + * type — the inherently-multi option types already read `string[]` there, and + * `isMultiValueField` answers true for them with or without the flag. Wrapping + * again would emit `string[][]` for a `multiselect`, which `FieldSchema` + * accepts flagged (the flag is REDUNDANT on those types, never refused). So the + * roster is consulted for what the table has already said, ⛔ not as a second + * multi-value predicate. `vector` / `repeater` are array entries too and need + * no such guard: the one predicate answers false for both. + */ +function fieldTypeToTs(fieldType: string, multiValued: boolean): string { const base = FIELD_TYPE_MAP[fieldType] || 'unknown'; - return multiple ? `${base}[]` : base; + if (!multiValued) return base; + return MULTI_OPTION_TYPES.has(fieldType) ? base : `${base}[]`; } /** [#16319] The closed `FieldType` vocabulary as a Set — built once, off the spec enum. */ @@ -754,7 +810,8 @@ export function generateTypesFromConfig(config: Record): string for (const [fieldName, fieldDef] of Object.entries(fields)) { // [#16319] Was `String(fieldDef.type || 'text')`. See {@link declaredFieldType}. const fType = declaredFieldType(name, fieldName, fieldDef); - const tsType = fieldTypeToTs(fType, !!fieldDef.multiple); + // [#18199] Was `!!fieldDef.multiple`. See {@link declaredMultiValued}. + const tsType = fieldTypeToTs(fType, declaredMultiValued(fType, fieldDef)); const required = fieldDef.required ? '' : '?'; if (fieldDef.label) { lines.push(` /** ${fieldDef.label} */`); @@ -1200,7 +1257,8 @@ function generateClientFromConfig(config: Record): string { for (const [fieldName, fieldDef] of Object.entries(fields)) { // [#16319] Was `String(fieldDef.type || 'text')`. See {@link declaredFieldType}. const fType = declaredFieldType(name, fieldName, fieldDef); - const tsType = fieldTypeToTs(fType, !!fieldDef.multiple); + // [#18199] Was `!!fieldDef.multiple`. See {@link declaredMultiValued}. + const tsType = fieldTypeToTs(fType, declaredMultiValued(fType, fieldDef)); const required = fieldDef.required ? '' : '?'; lines.push(` ${fieldName}${required}: ${tsType};`); } @@ -1328,9 +1386,10 @@ async function runClientGeneration(configPath: string | undefined, flags: { outp * PRE-EXISTING entry byte-for-byte alone. This is the third direction: entries * that existed, keyed on a real member, and described something the platform * does not do. Each is now the platform's own answer, read from - * `packages/drivers/driver-sql/src/sql-driver.ts` — ⛔ the DRIVER is the - * authority for which column exists, never the spec's `isMultiValueField` - * VALUE predicate (see {@link fieldTypeToSql}): + * `packages/drivers/driver-sql/src/sql-driver.ts` — the DRIVER is the authority + * for which column exists, and since #17469 the driver derives its multi-value + * half from the spec's `isMultiValueField`, so asking that predicate here IS + * asking the driver's own rule (see {@link declaredMultiValued}): * * `autonumber` SERIAL → VARCHAR(255). The runtime issues a RENDERED * string (prefix + counter + suffix); `createColumn`'s @@ -1682,9 +1741,14 @@ function declaredNotNull(field: unknown): boolean { * * ## What this deliberately does NOT emit, each because the driver does not * - * - **A `multiple: true` field.** `createColumn` short-circuits on the flag and - * returns before both the nullability line and this one, so a multi-value - * column carries no DEFAULT on the platform either. + * - **A MULTI-VALUE field.** `createColumn` short-circuits on the multi-value + * question and returns before both the nullability line and this one, so a + * multi-value column carries no DEFAULT on the platform either. [#18199] The + * question is {@link declaredMultiValued}, not a raw `field.multiple`: the + * driver's own short-circuit became `isMultiValuedColumn(...)` in #17469, so + * a `text` field flagged `multiple: true` is an ordinary column there and + * reaches the default question — reading the flag raw here withheld a DEFAULT + * the platform emits. * - **An option-level `default: true`** on a `select`. `applyDeclaredColumnDefault` * states at length why that stays out of DDL (one resolver owns the precedence; * the `multiple` shape has no scalar DDL form; a retrofit would divide @@ -1703,7 +1767,8 @@ type DeclaredColumnDefault = function declaredColumnDefault(field: unknown, type: string): DeclaredColumnDefault { const declaring = field as { defaultValue?: unknown; multiple?: unknown } | undefined; - if (declaring?.multiple) return { kind: 'none' }; + // [#18199] Was `if (declaring?.multiple)`. See {@link declaredMultiValued}. + if (declaredMultiValued(type, declaring)) return { kind: 'none' }; const dv = declaring?.defaultValue; if (dv === undefined || dv === null) return { kind: 'none' }; if (isNowDefaultToken(dv)) { @@ -2271,32 +2336,39 @@ function partitionUniqueIndexes( /** * The column one field takes. * - * `multiple` is answered FIRST, before the type is looked up at all, because + * MULTI-VALUE is answered FIRST, before the type is looked up at all, because * that is what the platform does. `SqlDriver.createColumn` short-circuits on - * `field.multiple` ABOVE its own `switch (type)`; `isJsonField` is - * `JSON_COLUMN_TYPES.has(type) || !!field.multiple`; and `fieldHasColumn` - * opens with `if (field?.multiple) return true` under the comment "Mirrors - * `SqlDriver.createColumn` exactly ... including `multiple` (a JSON column)". - * Three statements of one rule: a flagged field is a JSON column whatever its - * element type would have been, so the element type gets no vote here either - * (#14829). Before this, one authored `Field.lookup({ multiple: true })` - * produced `account?: string[]` from `os generate types` and a scalar - * `VARCHAR(36)` column from this generator, in the same run. - * - * WARNING: this is deliberately NOT the spec's `isMultiValueField`. That is the - * ADR-0104 D1 VALUE contract ("is the persisted value an array"), gated on - * `MULTI_CAPABLE_TYPES`; asking it here would answer VARCHAR for a `text` - * field the driver gives a JSON column - the same drift one notch narrower. - * The column question belongs to the driver, and the driver's answer is the - * flag alone. `generate-multiple-json-column.pin.test.ts` pins both halves. + * `isMultiValuedColumn(...)` ABOVE its own `switch (type)`; `isJsonField` is + * `JSON_COLUMN_TYPES.has(type) || isMultiValuedColumn(type, field)`; and + * `fieldHasColumn` opens with `if (isMultiValueField(...)) return true` under + * the comment "Mirrors `SqlDriver.createColumn` exactly ... including + * `multiple` (a JSON column)". Three statements of one rule: a multi-value + * field is a JSON column whatever its element type would have been, so the + * element type gets no vote here either (#14829). Before this, one authored + * `Field.lookup({ multiple: true })` produced `account?: string[]` from + * `os generate types` and a scalar `VARCHAR(36)` column from this generator, in + * the same run. + * + * ⭐ [#18199] THE QUESTION IS THE SPEC'S `isMultiValueField`, and this paragraph + * is the record of the reversal. It used to open "WARNING: this is deliberately + * NOT the spec's `isMultiValueField`", on the ground that the column question + * belongs to the driver and the driver's answer was the flag alone. The second + * half of that stopped being true: the maintainer ruling of 2026-09-13 + * (decision batch #128 item 5, option 1′) gives "multi-valued" ONE definition + * and #17469 derived all three driver sites above from it. So the premise held + * and the conclusion inverted — the column question still belongs to the + * driver, and the driver now answers it with the spec predicate. Asking the + * flag raw here is what made `os generate migration` emit JSONB for a `text` + * field the driver gives a varchar. Asked through {@link declaredMultiValued}. + * `generate-multiple-json-column.pin.test.ts` pins both halves. * * The JSON spelling is READ from this table's own `json` entry rather than * restated, so the two cannot drift about what a JSON column is spelled here. * * `null` means NO COLUMN — the answer for a virtual field type (#14828). It is * the table's own entry, not a second decision here, and it composes in the - * driver's order: `multiple` still wins first, so a flagged field of any type - * is a JSON column and never reaches the lookup at all. + * driver's order: multi-value still wins first, so a multi-value field of any + * type is a JSON column and never reaches the lookup at all. * * ⚠️ The lookup is by OWN-PROPERTY PRESENCE, not by the value being falsy or * nullish, because `null` is a meaningful ANSWER and every other spelling @@ -2327,12 +2399,15 @@ function partitionUniqueIndexes( * declaration produce two different columns, and refusing is the ruled answer. */ function fieldTypeToSql( + // [#18199] `multiValued` is {@link declaredMultiValued}'s verdict, not a raw + // `field.multiple`; the rename is so a future caller cannot hand it the flag + // again without noticing. fieldType: string, - multiple?: boolean, + multiValued: boolean, maxLength?: unknown, keyed?: boolean, ): string | null { - if (multiple) return FIELD_TYPE_SQL_MAP.json; + if (multiValued) return FIELD_TYPE_SQL_MAP.json; const base = Object.prototype.hasOwnProperty.call(FIELD_TYPE_SQL_MAP, fieldType) ? FIELD_TYPE_SQL_MAP[fieldType] : 'TEXT'; @@ -2440,7 +2515,8 @@ export function generateMigrationSql(config: Record): string { const fType = declaredFieldType(tableName, fieldName, fieldDef); const sqlType = fieldTypeToSql( fType, - !!fieldDef.multiple, + // [#18199] Was `!!fieldDef.multiple`. See {@link declaredMultiValued}. + declaredMultiValued(fType, fieldDef), fieldDef.maxLength, keyColumns.has(fieldName), ); @@ -2606,15 +2682,16 @@ export function generateMigrationTs(config: Record): string { // name is kept so the emitter below reads unchanged. const required = declaredNotNull(fieldDef) ? '.notNullable()' : '.nullable()'; - // #14829 - `multiple` before the type, exactly as `SqlDriver.createColumn` - // does it: the driver short-circuits on the flag above its own per-type - // switch, so a flagged field is a JSON column whatever its element type - // would have been. Emitted here rather than as a switch arm because the - // switch cases on the TYPE and the type has no vote in this decision; - // the spelling is this generator's own JSON arm, stated once more. - // See `fieldTypeToSql` for why the authority is the driver's flag rule - // and not the spec's `isMultiValueField` value predicate. - if (fieldDef.multiple) { + // #14829 - MULTI-VALUE before the type, exactly as `SqlDriver.createColumn` + // does it: the driver short-circuits above its own per-type switch, so a + // multi-value field is a JSON column whatever its element type would have + // been. Emitted here rather than as a switch arm because the switch cases + // on the TYPE and the type has no vote in this decision; the spelling is + // this generator's own JSON arm, stated once more. + // [#18199] The question is {@link declaredMultiValued} — the spec's + // `isMultiValueField`, which is what the driver's own short-circuit + // became in #17469. See `fieldTypeToSql` for the reversal in full. + if (declaredMultiValued(fType, fieldDef)) { lines.push(` table.jsonb('${fieldName}')${required};`); emittedColumns.add(fieldName); continue; diff --git a/packages/runtime/src/action-execution.ts b/packages/runtime/src/action-execution.ts index 1bd873a79ab..c9b6c637e77 100644 --- a/packages/runtime/src/action-execution.ts +++ b/packages/runtime/src/action-execution.ts @@ -1264,6 +1264,22 @@ function collectScreenFieldSpecs(flow: any): Map { * carries them directly (inline overrides win). `obj` is the action's * parent object schema (holds `.fields`); pass `undefined` for a global * action with only inline params. + * + * ⛔ [#18199] THE `multiple` READ BELOW IS NOT THE MULTI-VALUE QUESTION — do not + * "align" it on `isMultiValueField` because it matched a grep for + * `field.multiple`. #18199 is the ruling card for the maintainer decision of + * 2026-09-13 (decision batch #128 item 5, option 1′) that "multi-valued" has ONE + * definition, and this site was measured against it and EXCLUDED, for the reason + * the ADR-0104 D2 sentence above already gives: this function INHERITS an + * author's declaration onto a descriptor, it does not decide anything from it. + * The one definition is asked of the descriptor one frame down, where the + * decision actually happens — `validateActionParams` builds the param's schema + * with `valueSchemaFor({ type, multiple, options })`, whose last line is + * `isMultiValueField(def) ? z.array(element) : element`. So the runtime already + * follows the one definition here, through the spec's own consumer. Pre-computing + * the verdict into this field would ALSO break the inheritance contract: the key + * would stop carrying what the author wrote, and the predicate would then be + * applied to its own output. */ export function resolveDeclaredActionParams(_deps: ActionExecutionDeps, action: any, obj: any): ResolvedActionParam[] { const fields: Record = obj?.fields ?? {};