fix(driver-sql): report an unbounded text-family field left on a pre-existing varchar column (#12121)

.changeset/drift-unbounded-text-column-over-stale-varchar.md

@@ -0,0 +1,68 @@
+---
+'@objectstack/driver-sql': minor
+---
+
+Report an **unbounded** text-family field left on a pre-existing `varchar`
+column, instead of leaving the operator with a refused write and no diagnostic
+
+After #11875/#12119 a **newly created** `signature` / `qrcode` column is TEXT and
+holds a data URI correctly. `initObjects` is additive-only, so on a database
+created by an earlier release nothing is missing, nothing is added, and the old
+`varchar(255)` column is kept forever — the boundary #12119's own changeset
+states in as many words. What was not stated is what the drift reporter did
+about it, and the answer was **nothing**.
+
+The varchar differ's entire branch required `declaredMaxLength !== undefined`, so
+on a pre-existing table it split the text family by whether its author had
+written a number:
+
+```
+Field.signature({ maxLength: 4096 }) over varchar(255) -> widen_varchar reported
+Field.signature() — no bound over varchar(255) -> (nothing) silent
+```
+
+The second row is the common case. Measured on the pre-fix tree, one
+`diffManagedTable` call per type on dialect `postgres` against a `varchar(255)`
+column: `text` / `textarea` / `html` / `markdown` / `richtext` / `code` /
+`signature` / `qrcode` with no `maxLength` each returned **zero** entries, while
+`{ type: 'signature', maxLength: 4096 }` over the same column returned exactly
+one `widen_varchar` in the same run — so the differ was working and this shape
+was simply invisible to it. An upgrading deployment therefore saw no change and
+no diagnostic, while the server kept refusing the same write; and the refusal is
+a poor substitute for a report, because the live probe behind objectql's
+`driver-fault-redaction.ts` measured Postgres's `22001` as identifier-only and
+naming the **type** rather than the column (`value too long for type character
+varying(255)`).
+
+The divergence is now **detected and reported** under a new report-only
+`manual_widen_varchar_to_text` op, naming the declared type, the physical width,
+the consequence, and both operator routes. Same `declared ≠ enforced` shape as
+the #11374 / #11431 / #11875 family, closed one door further along — at the
+migration seam rather than the authoring or write seam.
+
+**Nothing is migrated for you, and nothing new is refused.** There is no
+reconciler arm: `os migrate apply` reports the entry as skipped, exactly as it
+does for `manual_column_type_change`. The entry is `category: 'needs_confirm'`,
+so the artifact-pinned boot gate — which refuses a boot for `destructive` and
+nothing else — is unaffected: a deployment that merely refuses over-long values
+must not become a crash-loop on its next restart. Dev auto-reconcile takes
+`safe` only, so it never applies this unattended either. SQLite is excluded: it
+enforces no declared width, so there is no divergence to report.
+
+`manual_widen_varchar_to_text` is a **distinct** op rather than a second use of
+`manual_column_type_change`, for a measured reason: `os migrate
+multi-value-columns` selects its entire population by
+`op.type === 'manual_column_type_change'` and recovers the dialect by matching
+the message against `manualJsonConversionSql`, so sharing the op would hand this
+finding to a command whose remedy makes the column `json` — and, the message
+carrying no json statement, have it refused as `remedy_not_recognized` on every
+run.
+
+Graded `minor` rather than `patch` on two counts, matching the sibling drift-op
+addition that shipped for #11535: `detectManagedDrift` emits a finding on
+existing deployments where it previously emitted none (visible in `os migrate
+plan`, in `os migrate apply`'s skipped count and in the boot-time
+`[schema-drift]` warn), and the exported `DriftOp` union gains a member, which is
+additive for producers but widens a type any consumer switching exhaustively
+over it must account for. Nothing is removed, renamed or newly rejected, so it is
+not a breaking change.

content/docs/deployment/cli.mdx

@@ -543,7 +543,7 @@ diverges from the live schema, and the physical column wins at write time.
 |---------|-------------|
 | `os migrate plan` | Dry-run: show how the database has drifted from metadata, categorised safe / needs-confirm / destructive (no changes applied) |
 | `os migrate apply` | Reconcile the database to metadata. Applies loosening changes; destructive ones require `--allow-destructive` |
-| `os migrate multi-value-columns` | Migrate a stale `varchar`/`text` column to `json` where the field declares `multiple: true` — the one drift op `apply` never reconciles for you. Dry run by default; `--apply` runs the statement the finding prints |
+| `os migrate multi-value-columns` | Migrate a stale `varchar`/`text` column to `json` where the field declares `multiple: true` — one of two drift ops `apply` never reconciles for you. Dry run by default; `--apply` runs the statement the finding prints |
 
 ```bash
 os migrate plan # Preview drift (no changes)
@@ -632,7 +632,7 @@ occupancy on its own.
 | Category | Examples | Applied by |
 |----------|----------|------------|
 | `safe` | relax `NOT NULL` → nullable, widen a `varchar`, create a declared index, replace a legacy installation-wide unique with its per-organization composite | `os migrate apply` (and dev auto-reconcile) |
-| `needs_confirm` | non-narrowing type change, rebuild a non-unique index whose columns changed | `os migrate apply` |
+| `needs_confirm` | non-narrowing type change, rebuild a non-unique index whose columns changed | `os migrate apply` — except `manual_widen_varchar_to_text`, which nothing applies |
 | `destructive` | drop an orphaned column or index, tighten `NOT NULL`, narrow a type, rebuild an index as `UNIQUE` | `os migrate apply --allow-destructive` |
 
 #### Index drift
@@ -667,7 +667,7 @@ it reconciles via a table rebuild (copy → swap) that preserves your data.
 
 #### `os migrate multi-value-columns`
 
-The one drift op `os migrate apply` will **never** apply for you.
+`os migrate apply` will **never** apply this drift op — and it isn't the only one: `manual_widen_varchar_to_text` (an unbounded text-family field left on a pre-existing `varchar` column) is also never applied, but has no `os migrate` subcommand of its own. This section covers the op that does.
 
 A field that gains `multiple: true` over a database that already exists keeps
 its old `varchar` / `text` column: the additive sync adds columns, and never

packages/drivers/driver-sql/src/schema-drift.ts

@@ -171,6 +171,38 @@ export type DriftOp =
  * can show the divergence without re-deriving it from the message.
  */
  | { type: 'manual_column_type_change'; table: string; column: string; to: string; from: string }
+ /**
+ * REPORT ONLY (#12121). The column is a `varchar(n)` under a TEXT-family
+ * field that declared NO usable bound — the shape `createColumn` emits as
+ * TEXT — so the column caps writes the declaration allows, and **the platform
+ * deliberately does not change it**.
+ *
+ * ⛔ A DISTINCT op rather than a second use of `manual_column_type_change`,
+ * for a measured reason and not a stylistic one: `os migrate
+ * multi-value-columns` selects its ENTIRE population by
+ * `op.type === 'manual_column_type_change'` (`isStaleMultiValueColumn`, "the
+ * only op this command touches") and then recovers the dialect by matching the
+ * finding's message against {@link manualJsonConversionSql}. Reusing that op
+ * would hand this finding to a command whose remedy converts the column to
+ * **json** — the wrong column type for a signature — and, since this message
+ * embeds no json statement, the command would file it under
+ * `remedy_not_recognized`: one refusal line per finding, on every run, for a
+ * divergence that command has no business with.
+ *
+ * ⛔ There is NO reconciler arm, deliberately. `applyDriftOpInPlace` matches
+ * no case, so `applyMigrationEntries` reports the entry as skipped and logs
+ * it. Whether ObjectStack should run the `ALTER` itself is a separate decision
+ * with hazards this differ must not pre-empt: on MySQL a `MODIFY` restates the
+ * WHOLE column definition (silently dropping a NOT NULL or DEFAULT that is not
+ * repeated) and a TEXT column cannot carry a plain index without a prefix
+ * length, so the conversion can turn a working table into one whose declared
+ * index no longer exists.
+ *
+ * `from` is the physical type word and `to` is always `'text'`, spelled the way
+ * `manual_column_type_change` spells them so a renderer showing `from → to`
+ * needs no new arm.
+ */
+ | { type: 'manual_widen_varchar_to_text'; table: string; column: string; to: 'text'; from: string }
  /**
  * Retire the legacy platform-wide UNIQUE index on a now-tenant-scoped field
  * and put the composite `(tenantField, field)` in its place (#3696). The two
@@ -483,6 +515,46 @@ function acceptsStringifiedJson(type: string | undefined): boolean {
  return /char|text/i.test(String(type ?? ''));
 }
 
+/**
+ * The field types whose column is TEXT whenever the field declares no usable
+ * `maxLength` — `createColumn`'s text-family case (#11794 / #11875).
+ *
+ * ⚠️ Read the scope precisely: these are the types for which the emitter's
+ * answer is TEXT **regardless of whether an index keys the column**. That
+ * independence is what licenses the #12121 branch below to report on this shape
+ * without being told which columns are keyed, and it is not an assumption — it
+ * falls out of the emitter's own expression, `keyable = keyed ?
+ * keyableTextLength(field) : null`, whose keyed arm returns `null` for a field
+ * with no positive-integer bound. A text-family field that DID declare a keyable
+ * bound takes `varchar(maxLength)` when keyed, which is exactly why that branch
+ * is gated on the declaration being ABSENT and never fires for it.
+ *
+ * ⛔ NOT derived from the spec's `BOUNDED_STRING_FIELD_TYPES`, for the reason
+ * `sql-driver-12017-bounded-string-spec-parity.test.ts` argues in full: that set
+ * answers "may this type declare a bound?" and carries no varchar/TEXT
+ * partition, so deriving would have to INVENT an answer for every future member
+ * at the one seam where the maintainer has actually ruled per type. It is a
+ * hand-written list and is therefore PINNED rather than trusted —
+ * `schema-drift.unbounded-text-column.test.ts` probes the driver's OWN
+ * dispatch (`varcharColumnChars`) over every `FieldType` the spec declares and
+ * asserts set equality in both directions, plus the keyed-and-unkeyed `null`
+ * above for every member. A type entering or leaving `createColumn`'s text
+ * family reds there by name.
+ *
+ * ⚠️ It lives here rather than being imported from `sql-driver.ts` because the
+ * dependency runs the other way — `sql-driver.ts` imports this module — so an
+ * import would be a cycle. The pin is what stands in for a shared constant.
+ *
+ * ⛔ Module-exported so this package's own suites can pin it, and deliberately
+ * NOT added to `index.ts` — the same call {@link MULTI_VALUE_COLUMN_REMEDY_COMMAND}
+ * makes: nothing outside this package has a question this set answers.
+ */
+export const UNBOUNDED_TEXT_FIELD_TYPES: ReadonlySet<string> = new Set([
+ 'text', 'textarea', 'html', 'markdown', 'richtext', 'code',
+ // #11875 — joined the family once the write seam enforced their declared bound.
+ 'signature', 'qrcode',
+]);
+
 /**
  * Does a multi-value field's JSON column carry its type on THIS dialect — i.e.
  * does a stale textual column silently corrupt the value (#11535)?
@@ -863,6 +935,93 @@ export function diffManagedTable(args: {
  });
  }
  }
+
+ // ── an UNBOUNDED text-family field over a pre-existing varchar (#12121) ──
+ //
+ // The exact COMPLEMENT of the branch above: that one REQUIRES
+ // `declaredMaxLength !== undefined`, so on a pre-existing table the two
+ // partition the text family by whether its author wrote a number.
+ //
+ // Until this branch existed the undeclared half was reported by NOTHING, and
+ // that half is the common case. Measured on the pre-fix tree, one
+ // `diffManagedTable` call per type: a `text` / `textarea` / `html` /
+ // `markdown` / `richtext` / `code` / `signature` / `qrcode` field with no
+ // `maxLength` over a `character varying(255)` column returned **zero**
+ // entries on both enforcing dialects, while `{ type: 'signature', maxLength:
+ // 4096 }` over the same column returned `widen_varchar` in the same run — so
+ // the differ was working and this shape was simply invisible to it.
+ //
+ // What that silence costs: after #11875/#12119 a NEWLY created column for
+ // these types is TEXT and holds a data URI correctly, but the additive sync
+ // never revisits an existing column, so a deployment upgrading into that
+ // release gets no change AND no diagnostic. The server keeps refusing the
+ // same write, and the refusal is a poor substitute for a report: the live
+ // probe behind `objectql`'s `driver-fault-redaction.ts` measured Postgres's
+ // `22001` as identifier-only and naming the TYPE rather than the column
+ // (`value too long for type character varying(255)`), MySQL's `1406` as
+ // `Data too long for column 'label' at row 1`. Meanwhile every
+ // drift-reporting road in the platform — `os migrate plan`, `os migrate
+ // apply`, the artifact-pinned boot gate, the boot-time `[schema-drift]` warn
+ // — reads THIS function, so the one place that could have named the column
+ // and the cause named nothing at all.
+ //
+ // ## Why this needs no keyed-column input
+ //
+ // `createColumn` sizes a text-family column as `keyed ?
+ // keyableTextLength(field) : null`, and `keyableTextLength` returns `null`
+ // for a field with no positive-integer bound. So for the fields this branch
+ // SELECTS the emitter answers TEXT whether or not an index keys them: the
+ // differ does not have to know, and cannot be wrong about it. Pinned as such
+ // — see {@link UNBOUNDED_TEXT_FIELD_TYPES}.
+ //
+ // ## Severity `error`, category `needs_confirm` — and the category is the
+ // ## load-bearing half, exactly as it is for the base-type branch above
+ //
+ // ⛔ Do NOT "correct" `needs_confirm` to `destructive` to match how bad it
+ // sounds. `runArtifactBootMigrationGate` refuses a boot for `category ===
+ // 'destructive'` and for nothing else, and every database this finding
+ // describes is ALREADY SERVING — that is the premise of the report. A
+ // `destructive` spelling would convert a deployment that merely refuses
+ // over-long values into a crash-loop on its next restart. `safe` is wrong in
+ // the other direction: dev auto-reconcile applies `safe` entries unattended
+ // and there is no arm to apply.
+ //
+ // `severity` is read by NO gate — it is render weight — and `error` is the
+ // honest weight for the same reason the base-type branch takes it: there is
+ // no automatic repair, so the operator has to act.
+ if (
+ enforcesVarcharLength(dialect) &&
+ !declaresJsonColumn &&
+ declaredMaxLength === undefined &&
+ UNBOUNDED_TEXT_FIELD_TYPES.has(field.type || 'string') &&
+ isCharacterColumn(col.type) &&
+ typeof col.maxLength === 'number'
+ ) {
+ out.push({
+ kind: 'type_mismatch',
+ remoteName: table,
+ table,
+ column: fieldName,
+ expected: 'text',
+ actual: `varchar(${col.maxLength})`,
+ severity: 'error',
+ category: 'needs_confirm',
+ op: { type: 'manual_widen_varchar_to_text', table, column: fieldName, to: 'text', from: col.type },
+ message:
+ `${table}.${fieldName}: metadata declares \`${field.type || 'string'}\` with no ` +
+ `\`maxLength\`, so ObjectStack creates this column as TEXT — but the existing column is ` +
+ `\`varchar(${col.maxLength})\` and the additive sync never changes a column's type. The ` +
+ `column still caps at ${col.maxLength} characters, so the server refuses longer values the ` +
+ `declaration ALLOWS (Postgres 22001, MySQL ER_DATA_TOO_LONG) — a data URI in a ` +
+ `\`signature\`/\`qrcode\` field, or an ordinary rich-text body, is routinely past it ` +
+ `(#12121). ObjectStack does NOT migrate this column: "os migrate apply" reports this entry ` +
+ `as skipped. Two operator routes — declare a \`maxLength\` this dialect can express, which ` +
+ `turns this into the widen op "os migrate apply" performs; or convert the column to TEXT by ` +
+ `hand, with a backup taken first, restating the FULL column definition on MySQL (MODIFY ` +
+ `drops a NOT NULL or DEFAULT you do not repeat) and dropping any index that keys the column ` +
+ `first, since MySQL cannot key a TEXT column without a prefix length.`,
+ });
+ }
  }
 
  // ── orphaned columns (physical column, no metadata field) ──────────