Skip to content

Commit 0e5bea6

Browse files
claude[bot]claude
andauthored
feat(cli): os migrate multi-value-columns — the operator-run stale-column migration (#11894)
* feat(cli): os migrate multi-value-columns — operator-run stale column migration The operator-run half of #11535, ruled C on #11700: the platform warns and ships an explicit migration an operator invokes, and never runs it for them. The statement is the one driver-sql's `manual_column_type_change` finding prints (#11720, measured against live Postgres 16.13 / MySQL 8.0.46); the command refuses to execute anything the finding does not contain verbatim. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR * refactor(cli,driver-sql): import the remedy statement instead of copying it Re-exports `manualJsonConversionSql` from driver-sql's index (one line) and has `os migrate multi-value-columns` import it, dropping the CLI's copy. The import is lazy and injected: no CLI production module may statically value-import a driver package (#5726), so the command `await import()`s it at the point of use and hands it to the planner, which stays synchronous. The runtime containment check survives, with a different claim: it is now only the DIALECT probe (a ManagedDriftEntry carries no dialect), matching the engine's finding against the engine's own function. Its refusal branch is still reachable. `remedy-fidelity` becomes `dialect-probe`, minus three cases that could no longer fail for any reason belonging to this package. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 30d96ab commit 0e5bea6

8 files changed

Lines changed: 1236 additions & 0 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@objectstack/cli": minor
3+
"@objectstack/driver-sql": minor
4+
---
5+
6+
New operator-run command `os migrate multi-value-columns`: migrates a stale `varchar`/`text` column to `json` where the field declares `multiple: true` — the `manual_column_type_change` drift `os migrate apply` reports and deliberately never reconciles for you (#11535, ruled C on #11700). Flags: `--apply` (default off), `--yes`/`-y`, `--force`, `--table <name>` (repeatable), `--database-url`, `--json`. **Dry-run contract: without `--apply` the command executes nothing at all** — it prints the exact statements and the database they would run against, opens no seam and issues no probe, and a run is verified to have left the column type and every row unchanged. `--apply` runs `@objectstack/driver-sql`'s own `manualJsonConversionSql` — newly re-exported from that package's index for this consumer, its only other change — i.e. the statement the drift finding itself prints (Postgres: one `ALTER … USING (CASE …)` with `json_build_array`; MySQL: the two row-shaping `UPDATE`s then `ALTER … MODIFY … json`), refuses to execute anything the finding does not contain verbatim, re-runs detection afterwards and exits non-zero if the finding has not cleared. SQLite is excluded — the stale column round-trips a real array there, so the finding is never raised. Rows corrupted before the column is migrated are out of scope, and the command is never invoked automatically: nothing on the boot path reaches it.

‎content/docs/deployment/cli.mdx‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,7 @@ diverges from the live schema, and the physical column wins at write time.
543543
|---------|-------------|
544544
| `os migrate plan` | Dry-run: show how the database has drifted from metadata, categorised safe / needs-confirm / destructive (no changes applied) |
545545
| `os migrate apply` | Reconcile the database to metadata. Applies loosening changes; destructive ones require `--allow-destructive` |
546+
| `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 |
546547

547548
```bash
548549
os migrate plan # Preview drift (no changes)
@@ -664,6 +665,79 @@ first. It never drops a table that is absent from your metadata, and on SQLite
664665
it reconciles via a table rebuild (copy → swap) that preserves your data.
665666
</Callout>
666667

668+
#### `os migrate multi-value-columns`
669+
670+
The one drift op `os migrate apply` will **never** apply for you.
671+
672+
A field that gains `multiple: true` over a database that already exists keeps
673+
its old `varchar` / `text` column: the additive sync adds columns, and never
674+
changes the type of one that is already there. The write path then stores an
675+
array as the **stringified literal** `'["a","b"]'` and reads it back as a
676+
string, so whatever consumes the value receives one opaque id instead of a
677+
list — a hook copying it into a child record's single-value lookup writes the
678+
whole string as one id. `os migrate plan` reports it as
679+
`manual_column_type_change`, at severity `error` and category `needs-confirm`,
680+
which is why it neither refuses your boot nor is ever reconciled automatically:
681+
changing a column's type on a serving production database, unattended, is not
682+
something the platform will do while you are not watching.
683+
684+
```bash
685+
os migrate multi-value-columns # Dry run: the exact statements, executed NOT AT ALL
686+
os migrate multi-value-columns --json # The same, machine-readable
687+
os migrate multi-value-columns --apply # Run them (prompts)
688+
os migrate multi-value-columns --apply --yes --json # CI / scripts
689+
os migrate multi-value-columns --table crm_case # Restrict to one physical table (repeatable)
690+
os migrate multi-value-columns --database-url postgres://…
691+
```
692+
693+
**Take a backup first.** The dry run is the default and writes nothing at all —
694+
not a probe, not a temporary table — so run it, read the statements it prints,
695+
and only then re-run with `--apply`.
696+
697+
The statement is the one the drift finding itself prints, per dialect, and the
698+
command refuses to run anything else: if the finding no longer contains a
699+
statement the command recognises, it says so and tells you to apply the
700+
finding's statement by hand rather than falling back to SQL of its own.
701+
702+
| Dialect | What runs |
703+
|---------|-----------|
704+
| PostgreSQL | One `ALTER TABLE … ALTER COLUMN … TYPE json USING (CASE …)`. Legacy single values become one-element arrays (`json_build_array`, **not** `to_json`, which would produce a JSON *scalar* that is still not an array); an already-stringified array is cast through; `NULL` and `''` both become `NULL` |
705+
| MySQL | Three statements in order: `UPDATE … JSON_ARRAY(…)` over the legacy single values, `UPDATE … SET … = NULL` over the empty strings, then `ALTER TABLE … MODIFY … json`. MySQL will not cast text to json implicitly, so the rows have to move first or the `ALTER` dies on the first legacy value |
706+
| SQLite | Nothing — and nothing is needed. SQLite's read path parses the value regardless of what the column calls itself, so the same stale column round-trips a real array. The finding is never raised there |
707+
708+
After a successful run the command re-runs detection and requires the finding to
709+
be **gone**; a run whose statements succeeded while the column is still reported
710+
exits non-zero rather than telling you it migrated something it did not.
711+
712+
<Callout type="warn">
713+
**Rollback.** The conversion is not information-preserving: both `NULL` and the
714+
empty string become `NULL`, so once it succeeds those two states cannot be told
715+
apart again — **restoring your backup is the only faithful rollback**, which is
716+
why there is no `--undo`.
717+
718+
Reverting only the column *type* (Postgres: `ALTER TABLE … ALTER COLUMN … TYPE
719+
text USING …::text`) leaves JSON text in a text column; metadata still declares
720+
the field multi-value, so the finding returns on the next boot and the
721+
corruption resumes on the next write. Treat it as an incident stopgap, not a
722+
rollback.
723+
724+
On **PostgreSQL** the whole remedy is one statement: if it fails, the column is
725+
untouched and there is nothing to roll back. On **MySQL** it is three, and DDL
726+
there commits implicitly — a failure midway leaves the table partly converted.
727+
Re-run the command: each statement skips the rows a previous run already moved,
728+
so finishing an interrupted run is safe.
729+
730+
If the `ALTER` fails naming an **index**, drop the index on that column first (a
731+
json column cannot carry a plain btree) and re-run, then recreate it in a shape
732+
your dialect supports for json.
733+
</Callout>
734+
735+
**Rows corrupted before you migrate the column are yours to repair.** This
736+
command converts the column and the values *in* it. A stringified array that a
737+
hook or an integration already copied into some *other* single-value column is
738+
not something it looks for, and it is deliberately not something it will grow
739+
into: that repair is specific to what your automations did with the value.
740+
667741
#### Data migrations
668742

669743
The commands above reconcile **schema**. A *data* migration rewrites rows, and
Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#11733] The command runs the ENGINE's statement, and works out WHICH dialect
5+
* it is running by reading the engine's own finding.
6+
*
7+
* ## What this file used to be, and why it is smaller
8+
*
9+
* It began as a fidelity suite: `manualJsonConversionSql` was not on
10+
* `@objectstack/driver-sql`'s public surface, so the command carried a copy of
11+
* the statement and this suite held the copy to the engine's, byte for byte.
12+
* The one-line re-export landed with this card, the copy is gone, and with it
13+
* every assertion whose only job was to compare two spellings of one statement.
14+
* Three cases were deleted rather than left behind:
15+
*
16+
* - "identifiers are the finding's own table and column" — it tested the
17+
* CLI's builder handling its arguments. There is no CLI builder now; the
18+
* call site's argument passing is covered by the plan cases below, which
19+
* compare a real plan against `manualJsonConversionSql(dialect, …)`.
20+
* - "postgres keeps both corrections measurement forced" and the content half
21+
* of the MySQL case (`JSON_ARRAY` present, `json_build_array` absent). Both
22+
* now assert `driver-sql`'s CONTENT from a consumer's suite. They are not
23+
* vacuous — the engine could change that text — but that is precisely the
24+
* problem: they could only ever fail for a reason that has nothing to do
25+
* with this package, turning a deliberate engine correction into a red CLI
26+
* suite. `driver-sql` owns those, and pins them in
27+
* `schema-drift.base-type-mismatch.test.ts`, where they are also EXECUTED
28+
* against live Postgres 16.13 and MySQL 8.0.46.
29+
*
30+
* ## What is left is not fidelity, and can still fail
31+
*
32+
* Two claims, both about this package:
33+
*
34+
* 1. **the coupling the dialect probe reads** — the finding's message still
35+
* EMBEDS the remedy. Nothing in the CLI can keep that true, and everything
36+
* in the CLI depends on it: the probe decides Postgres from MySQL by which
37+
* dialect's statement the message contains, because a `ManagedDriftEntry`
38+
* carries no dialect and a client-spelling table copied out of the driver
39+
* could only disagree with it. If the engine ever stops interpolating the
40+
* statement, these go red here — where the consumer that would silently
41+
* lose its dialect lives.
42+
* 2. **the split, and the refusal** — how this command turns one engine
43+
* statement into the statements a seam takes, and what it does with a
44+
* finding it cannot read a dialect from.
45+
*/
46+
47+
import { describe, it, expect } from 'vitest';
48+
import {
49+
diffManagedTable,
50+
manualJsonConversionSql,
51+
type ManagedDriftEntry,
52+
type PhysicalColumn,
53+
} from '@objectstack/driver-sql';
54+
import { splitRemedyStatements, planStaleColumnTargets, CORRUPTING_DIALECTS } from './multi-value-columns.js';
55+
56+
const TABLE = 'proj_task';
57+
const COLUMN = 'tags';
58+
59+
/** Exactly what the command hands the planner in `run()`. */
60+
const SQL = { sql: manualJsonConversionSql };
61+
62+
/** The stale column, in each dialect's own type spelling (#11720's fixtures). */
63+
const STALE: Record<'postgres' | 'mysql', PhysicalColumn[]> = {
64+
postgres: [{ name: COLUMN, type: 'character varying', nullable: true, maxLength: 255 }],
65+
mysql: [{ name: COLUMN, type: 'varchar', nullable: true, maxLength: 255 }],
66+
};
67+
68+
const engineFinding = (dialect: 'postgres' | 'mysql'): ManagedDriftEntry => {
69+
const out = diffManagedTable({
70+
table: TABLE,
71+
fields: { [COLUMN]: { type: 'lookup', multiple: true } as any },
72+
columns: STALE[dialect],
73+
dialect,
74+
});
75+
// Non-vacuity: if the engine stopped reporting this shape, every assertion
76+
// below would pass against nothing.
77+
expect(out).toHaveLength(1);
78+
expect(out[0].op.type).toBe('manual_column_type_change');
79+
return out[0];
80+
};
81+
82+
describe('the coupling the dialect probe depends on (#11733)', () => {
83+
for (const dialect of CORRUPTING_DIALECTS) {
84+
it(`${dialect}: the finding's message still EMBEDS the remedy, which is what the probe matches on`, () => {
85+
// Not "the CLI agrees with the engine" — there is one function now, so
86+
// that could not fail. This is the engine's MESSAGE against the engine's
87+
// FUNCTION: it fails the day the message stops carrying the statement,
88+
// which is the day this command can no longer tell Postgres from MySQL.
89+
expect(engineFinding(dialect).message).toContain(manualJsonConversionSql(dialect, TABLE, COLUMN));
90+
});
91+
}
92+
93+
it('the two dialect forms are distinguishable — a probe reading one cannot match the other', () => {
94+
// The premise of reading the dialect off the message. If the forms were
95+
// substrings of one another the probe would resolve the wrong dialect and
96+
// run the wrong DDL, so this is asserted rather than assumed.
97+
const pg = manualJsonConversionSql('postgres', TABLE, COLUMN);
98+
const my = manualJsonConversionSql('mysql', TABLE, COLUMN);
99+
expect(pg).not.toBe(my);
100+
expect(engineFinding('postgres').message).not.toContain(my);
101+
expect(engineFinding('mysql').message).not.toContain(pg);
102+
});
103+
});
104+
105+
describe('splitting the engine statement into what a seam can run (#11733)', () => {
106+
it('mysql is three statements, in the order that makes the ALTER survivable', () => {
107+
const statements = splitRemedyStatements(manualJsonConversionSql('mysql', TABLE, COLUMN));
108+
expect(statements).toHaveLength(3);
109+
expect(statements[0]).toMatch(/^UPDATE .* JSON_ARRAY/);
110+
expect(statements[1]).toMatch(/^UPDATE .*= NULL WHERE/);
111+
expect(statements[2]).toMatch(/^ALTER TABLE .*MODIFY .*json$/);
112+
});
113+
114+
it('postgres is ONE statement — which is why a failed conversion leaves the column untouched', () => {
115+
// The rollback notes state this as a fact about Postgres; it is a fact
116+
// about the STATEMENT, so it is read off the statement.
117+
expect(splitRemedyStatements(manualJsonConversionSql('postgres', TABLE, COLUMN))).toHaveLength(1);
118+
});
119+
120+
it('the semicolon split loses nothing — neither form carries a semicolon inside a literal', () => {
121+
// The split is the one #11720's live suite executes the remedy with. It is
122+
// safe because of a property of THESE statements, so the property is pinned
123+
// rather than assumed: re-joining the parts reproduces the original.
124+
for (const dialect of CORRUPTING_DIALECTS) {
125+
const sql = manualJsonConversionSql(dialect, TABLE, COLUMN);
126+
expect(`${splitRemedyStatements(sql).join('; ')};`).toBe(sql.trim());
127+
}
128+
});
129+
});
130+
131+
describe('planning refuses anything it cannot read a dialect from (#11733)', () => {
132+
for (const dialect of CORRUPTING_DIALECTS) {
133+
it(`${dialect}: a real finding plans the engine's statements and names the dialect`, () => {
134+
const plan = planStaleColumnTargets([engineFinding(dialect)], SQL);
135+
expect(plan.refusals).toEqual([]);
136+
expect(plan.targets).toHaveLength(1);
137+
expect(plan.targets[0]).toMatchObject({ table: TABLE, column: COLUMN, to: 'json', dialect });
138+
// Also the call site's argument passing: these are the statements for
139+
// THIS table and column, not for a pair fixed anywhere in the command.
140+
expect(plan.targets[0].statements).toEqual(
141+
splitRemedyStatements(manualJsonConversionSql(dialect, TABLE, COLUMN)),
142+
);
143+
expect(plan.targets[0].from).toBe(dialect === 'postgres' ? 'character varying' : 'varchar');
144+
});
145+
}
146+
147+
it('a finding whose message carries no statement we can read a dialect from is REFUSED', () => {
148+
// The failure this closes: the engine rewords the message, the probe can no
149+
// longer tell Postgres from MySQL, and the command picks one anyway and
150+
// runs the wrong dialect's DDL against a customer's table. It refuses
151+
// instead — and says what to do by hand.
152+
const entry = engineFinding('postgres');
153+
const mutated = { ...entry, message: entry.message.replace('json_build_array', 'to_json') };
154+
155+
const plan = planStaleColumnTargets([mutated], SQL);
156+
expect(plan.targets).toEqual([]);
157+
expect(plan.refusals).toHaveLength(1);
158+
expect(plan.refusals[0]).toMatchObject({ table: TABLE, column: COLUMN, reason: 'remedy_not_recognized' });
159+
expect(plan.refusals[0].detail).toContain('os migrate plan');
160+
});
161+
162+
it('ignores every drift op that is not this one', () => {
163+
const others = diffManagedTable({
164+
table: TABLE,
165+
fields: { [COLUMN]: { type: 'string', maxLength: 50 } as any },
166+
columns: STALE.postgres,
167+
dialect: 'postgres',
168+
});
169+
expect(others.map((d) => d.op.type)).toEqual(['narrow_varchar']); // the instrument found something
170+
expect(planStaleColumnTargets(others, SQL)).toEqual({ targets: [], refusals: [] });
171+
});
172+
173+
it('--table narrows to the tables named, and drops the rest silently', () => {
174+
const a = engineFinding('postgres');
175+
const b = { ...a, table: 'crm_case', op: { ...(a.op as any), table: 'crm_case' } } as ManagedDriftEntry;
176+
// `b`'s message still carries `proj_task`'s statement, so it can only be
177+
// planned if the filter lets it through — which it must not.
178+
expect(planStaleColumnTargets([a, b], { ...SQL, tables: [TABLE] }).targets.map((t) => t.table)).toEqual([TABLE]);
179+
expect(planStaleColumnTargets([a, b], { ...SQL, tables: ['nothing_here'] }).targets).toEqual([]);
180+
});
181+
});

0 commit comments

Comments
 (0)