Skip to content

Commit 33e939f

Browse files
fix(driver-sql): schema drift reports a single-value JSON-class field on a stale text column (#16073)
* fix(driver-sql): schema drift reports a single-value JSON-class field on a stale text column `createColumn` gives a json column to every JSON-class TYPE and `isJsonField` is `JSON_COLUMN_TYPES.has(type) || !!field.multiple`, but `diffManagedTable`'s base-type branch asked only `field.multiple === true`. A single-value `file` / `location` / `record` / `vector` / `json` field on a `varchar`/`text` column was therefore written as JSON by the writer and invisible to the differ — and the additive sync never revisits a column, so the divergence was permanent and silent. Measured on the previous tree: all fifteen JSON-class types the spec declares produced zero findings on that column under `postgres` and `mysql`. The detector now reads the writer's predicate. The remedy splits by VALUE SHAPE: `os migrate multi-value-columns` wraps each value in a one-element JSON array, so it stays offered to array-valued fields — whose message is unchanged character for character, which is what lets `planStaleColumnTargets` keep recovering the dialect from it — and is withheld from single-value ones, whose message carries neither the command nor its statement and is therefore refused with `remedy_not_recognized` rather than running array SQL over scalar rows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ * fix(driver-sql): keep the tracker id out of the drift message's runtime prose `check:doc-authoring` refuses a `#NNNN` in a string an operator reads — it resolves to nothing without the tracker (maintainer ruling 2026-08-12). The anchor stays in the `//` comment beside the emission and in git history; the message now carries the CAUSE in words instead, which is what the reader actually needs. The pin asserts that wording rather than the id. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a7da4de commit 33e939f

5 files changed

Lines changed: 470 additions & 9 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/driver-sql": minor
3+
---
4+
5+
Schema drift now reports a SINGLE-VALUE JSON-class column that a stale `varchar`/`text` column is holding — the population the detector could never see.
6+
7+
The driver decides a field's column type with `JSON_COLUMN_TYPES.has(type) || !!field.multiple`: `createColumn` gives a json column to every JSON-class TYPE, and `isJsonField` — the read-side deserializer — asks the same question. The drift detector asked only `field.multiple === true`. So a single-value `file` / `image` / `location` / `address` / `record` / `vector` / `json` field (and the option families) sitting on a `varchar` or `text` column was written as JSON by the writer and did not exist to the differ. Because the additive sync never migrates a column's type, that column stayed wrong permanently and nothing reported it. Measured on the previous tree, one call per type: all fifteen JSON-class types the spec declares returned zero findings over a `character varying(2048)` column on `postgres` and `mysql`, while the same column under a `multiple: true` field returned one in the same run.
8+
9+
The detector now reads the writer's own predicate, so the two halves can no longer disagree about which declarations get a json column. `SQLite is unchanged and still reports nothing`: its read path parses a textual column regardless of what the column calls itself, re-measured on an in-memory cell as a byte-identical round-trip between the stale column and the driver's own.
10+
11+
**The remedy is offered to the array-valued half only.** `os migrate multi-value-columns` repairs a stale column by wrapping each stored value in a one-element JSON array, which is the right repair for a field whose value is a list and the wrong one for a field whose value is a scalar or an object. Findings for array-valued fields (`multiple: true`, and the inherently-multi option types) keep their message character for character, so that command keeps recovering the dialect from it and keeps working exactly as before. Findings for single-value JSON-class fields carry a message of their own that names neither the command nor its statement, explains why the automated route is withheld, and describes the by-hand conversion; the command refuses such an entry (`remedy_not_recognized`) instead of running array SQL over scalar rows.
12+
13+
Also fixed by the same predicate: a single-value JSON-class field declaring a `maxLength` over a wider `varchar` column used to be reported as `narrow_varchar` at category `destructive` — inviting `os migrate apply --allow-destructive` to rewrite the column to a narrower varchar, the opposite of the repair it needs. It is now reported once, as the base-type divergence.

packages/cli/src/commands/migrate/multi-value-columns.dialect-probe.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,59 @@ describe('planning refuses anything it cannot read a dialect from (#11733)', ()
170170
expect(planStaleColumnTargets(others, SQL)).toEqual({ targets: [], refusals: [] });
171171
});
172172

173+
it('a SINGLE-VALUE JSON-class column is refused, never planned — this command wraps values in an array (#15771)', () => {
174+
// ⚠️ The cross-package half of #15771, pinned where the damage would land.
175+
//
176+
// The engine's base-type detector used to be keyed to `field.multiple`
177+
// alone, so a single-value JSON-class field (`file`, `location`, `record`,
178+
// …) on a stale `varchar` column produced no finding at all. Widening it
179+
// put a NEW population in front of this command, and this command's repair
180+
// is `json_build_array(col)` / `JSON_ARRAY(col)` — it WRAPS each stored
181+
// value in a one-element array. Measured with this planner before the
182+
// engine's message split: a single-value finding carrying the multi-value
183+
// message was accepted as a target and planned with `json_build_array`,
184+
// i.e. this command would have converted a scalar `"file_01HXYZ"` into
185+
// `["\"file_01HXYZ\""]` on a customer's table.
186+
//
187+
// The engine therefore emits that population with a message carrying
188+
// NEITHER the command's name NOR the statement, which lands it in the
189+
// refusal branch above. That coupling is invisible from either side alone,
190+
// so it is asserted from BOTH: the engine's side pins that the message
191+
// omits the statement, and this pins what this command then does with it.
192+
const single = diffManagedTable({
193+
table: TABLE,
194+
fields: { [COLUMN]: { type: 'file' } as any },
195+
columns: STALE.postgres,
196+
dialect: 'postgres',
197+
});
198+
// Non-vacuity: the engine really does report this shape now. If it stops,
199+
// the refusal below would pass over an empty array.
200+
expect(single.map((d) => d.op.type)).toEqual(['manual_column_type_change']);
201+
202+
const plan = planStaleColumnTargets(single, SQL);
203+
expect(plan.targets).toEqual([]);
204+
expect(plan.refusals).toHaveLength(1);
205+
expect(plan.refusals[0]).toMatchObject({ table: TABLE, column: COLUMN, reason: 'remedy_not_recognized' });
206+
expect(plan.refusals[0].detail).toContain('os migrate plan');
207+
208+
// ⭐ And the CONTRAST in the same run, which is what makes the refusal a
209+
// discrimination rather than a command that refuses everything: an
210+
// inherently-multi option type on the identical column holds an ARRAY, so
211+
// it keeps the remedy and is planned.
212+
const arrayValued = diffManagedTable({
213+
table: TABLE,
214+
fields: { [COLUMN]: { type: 'tags' } as any },
215+
columns: STALE.postgres,
216+
dialect: 'postgres',
217+
});
218+
const arrayPlan = planStaleColumnTargets(arrayValued, SQL);
219+
expect(arrayPlan.refusals).toEqual([]);
220+
expect(arrayPlan.targets).toHaveLength(1);
221+
expect(arrayPlan.targets[0].statements).toEqual(
222+
splitRemedyStatements(manualJsonConversionSql('postgres', TABLE, COLUMN)),
223+
);
224+
});
225+
173226
it('--table narrows to the tables named, and drops the rest silently', () => {
174227
const a = engineFinding('postgres');
175228
const b = { ...a, table: 'crm_case', op: { ...(a.op as any), table: 'crm_case' } } as ManagedDriftEntry;

packages/drivers/driver-sql/src/schema-drift.base-type-mismatch.test.ts

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import { SqlDriver } from './sql-driver.js';
5151
import {
5252
diffManagedTable,
5353
manualJsonConversionSql,
54+
JSON_COLUMN_FIELD_TYPES,
5455
MULTI_VALUE_COLUMN_REMEDY_COMMAND,
5556
type PhysicalColumn,
5657
type SqlDialectName,
@@ -297,6 +298,172 @@ describe('diffManagedTable — multi-value field over a stale textual column (#1
297298
});
298299
});
299300

301+
// ── #15771: the SINGLE-VALUE half of the same fork ─────────────────────────
302+
//
303+
// The writer asks `JSON_COLUMN_TYPES.has(type) || !!field.multiple`; the
304+
// detector above asked only `multiple`. So a single-value JSON-class field on a
305+
// `varchar`/`text` column was written as JSON and reported by NOTHING — and, as
306+
// with #11535, the defect's shape is SILENCE, so "a finding was produced" is
307+
// again the weak assertion. Every case below pins the finding on the right
308+
// column with the right diagnosis, and the shapes that must stay silent are
309+
// pinned as silent in the same breath.
310+
311+
describe('diffManagedTable — a SINGLE-VALUE JSON-class field over a stale textual column (#15771)', () => {
312+
/** The card's two, named rather than swept, because these are the ones a deployment meets. */
313+
const SINGLE_VALUE_JSON = ['file', 'location'] as const;
314+
315+
it('reports single-value `file` and `location` on a varchar column, on BOTH enforcing dialects', () => {
316+
// Measured on the pre-fix tree: every one of these four returned `[]`.
317+
for (const type of SINGLE_VALUE_JSON) {
318+
for (const dialect of ['postgres', 'mysql'] as const) {
319+
const out = diffTags({ type }, staleColumn('character varying', 2048), dialect);
320+
expect(out, `${type}/${dialect}`).toHaveLength(1);
321+
expect(out[0]).toMatchObject({
322+
kind: 'type_mismatch',
323+
table: 'proj_task',
324+
column: 'tags',
325+
expected: 'json',
326+
actual: 'character varying',
327+
severity: 'error',
328+
// ⛔ NOT `destructive`, for the reason the multi-value case's category
329+
// pin argues in full: the boot gate reads CATEGORY, every database
330+
// this describes is already serving, and reporting the corruption
331+
// must not be the thing that takes the app down.
332+
category: 'needs_confirm',
333+
op: { type: 'manual_column_type_change', table: 'proj_task', column: 'tags', to: 'json', from: 'character varying' },
334+
});
335+
}
336+
}
337+
});
338+
339+
it('SQLITE stays silent — the reverse control that proves the pin discriminates', () => {
340+
// ⛔ Do not drop this leg. Without it every assertion above is equally
341+
// satisfied by a detector that reports unconditionally. And the silence is
342+
// MEASURED rather than scoped away: on an in-memory cell a single-value
343+
// `file` id round-trips through the stale `varchar(2048)` column and
344+
// through the driver's own `text` column with byte-identical stored bytes
345+
// (`"file_01HXYZ"` in both), because SQLite's read path `JSON.parse`s a
346+
// textual column regardless of what it calls itself.
347+
for (const type of SINGLE_VALUE_JSON) {
348+
expect(diffTags({ type }, staleColumn('varchar', 2048), 'sqlite'), type).toEqual([]);
349+
}
350+
});
351+
352+
it('says nothing once the column IS json — the repaired database', () => {
353+
for (const type of SINGLE_VALUE_JSON) {
354+
expect(diffTags({ type }, staleColumn('json'), 'postgres'), type).toEqual([]);
355+
expect(diffTags({ type }, staleColumn('json'), 'mysql'), type).toEqual([]);
356+
}
357+
});
358+
359+
it('closes the blind spot for EVERY JSON-class type the spec declares, not just the file family', () => {
360+
// The card's scope, asserted rather than described: the fork applies to
361+
// every single-value member of the writer's set, whichever way the
362+
// `VARCHAR(2048)`-vs-json generator divergence (#15041) is ruled.
363+
const jsonClassTypes = [...JSON_COLUMN_FIELD_TYPES].filter((t) => t !== 'object' && t !== 'array');
364+
expect(jsonClassTypes.length).toBeGreaterThan(10);
365+
366+
const blind = jsonClassTypes.filter(
367+
(type) => diffTags({ type }, staleColumn('character varying', 2048), 'postgres').length === 0,
368+
);
369+
expect(blind).toEqual([]);
370+
371+
// Non-vacuity: a type OUTSIDE the set is still silent in the same run, so
372+
// the zero above is a discriminating measurement, not an always-report.
373+
expect(diffTags({ type: 'string' }, staleColumn('character varying', 2048), 'postgres')).toEqual([]);
374+
expect(diffTags({ type: 'integer' }, staleColumn('character varying', 2048), 'postgres')).toEqual([]);
375+
});
376+
377+
// ── the remedy split: the message a broken remedy must NOT carry ──────────
378+
//
379+
// ⚠️ MEASURED before this branch shipped, with the command's own planner:
380+
// `planStaleColumnTargets` accepted a single-value finding carrying the
381+
// MULTI-VALUE message as a target and planned
382+
// `... ELSE json_build_array("doc") END`. That wraps the stored value in a
383+
// ONE-ELEMENT ARRAY — right for a field declaring `multiple: true`, wrong for
384+
// a field whose value is a scalar or an object. So the finding is emitted for
385+
// both populations and the remedy is offered to ONE.
386+
387+
it('the single-value message names NEITHER the command NOR its statement, so the command refuses it', () => {
388+
for (const type of SINGLE_VALUE_JSON) {
389+
for (const dialect of ['postgres', 'mysql'] as const) {
390+
const [entry] = diffTags({ type }, staleColumn('character varying', 2048), dialect);
391+
expect(entry.message, `${type}/${dialect}`).not.toContain(MULTI_VALUE_COLUMN_REMEDY_COMMAND);
392+
// This is the CLI's dialect probe, reproduced: no dialect's statement
393+
// is present, so `planStaleColumnTargets` recovers no dialect and
394+
// refuses the entry (`remedy_not_recognized`) instead of running array
395+
// SQL over scalar rows. Pinned from the consumer side too, in
396+
// `packages/cli/.../multi-value-columns.dialect-probe.test.ts`.
397+
for (const d of ['postgres', 'mysql'] as const) {
398+
expect(entry.message).not.toContain(manualJsonConversionSql(d, 'proj_task', 'tags'));
399+
}
400+
}
401+
}
402+
});
403+
404+
it('the single-value message still carries the whole diagnosis an operator acts on', () => {
405+
// A finding with no remedy is still a finding: the operator has to be able
406+
// to act from the one line a restart prints.
407+
const [entry] = diffTags({ type: 'file' }, staleColumn('character varying', 2048), 'postgres');
408+
expect(entry.message).toContain('proj_task.tags');
409+
expect(entry.message).toContain('`file`');
410+
expect(entry.message).toContain('character varying');
411+
expect(entry.message).toContain('json');
412+
// ⛔ NOT the issue id — `check:doc-authoring` refuses a tracker id in
413+
// runtime prose (maintainer ruling 2026-08-12: an operator can resolve
414+
// neither the number nor the tracker). The anchor lives in the `//`
415+
// comment beside the emission and in git history. What the operator DOES
416+
// need is the cause, in words:
417+
expect(entry.message).toMatch(/JSON-ENCODED/);
418+
expect(entry.message).toMatch(/backup/i);
419+
expect(entry.message).toMatch(/btree/i);
420+
// And it says WHY the automated route is withheld, rather than leaving the
421+
// operator to discover the refusal by running it.
422+
expect(entry.message).toMatch(/ONE-ELEMENT JSON ARRAY/);
423+
});
424+
425+
it('an INHERENTLY-MULTI option type keeps the array remedy — the split is by value shape, not by `multiple`', () => {
426+
// `multiselect` / `checkboxes` / `tags` hold a list with or without the
427+
// flag, so the wrapping remedy is the right repair for them and the
428+
// message that names it is the right message. Getting this leg wrong in
429+
// either direction is a real cost: withheld, an operator loses a working
430+
// command; offered to a scalar field, it corrupts.
431+
for (const type of ['multiselect', 'checkboxes', 'tags'] as const) {
432+
const [entry] = diffTags({ type }, staleColumn('character varying', 255), 'postgres');
433+
expect(entry.message, type).toContain(MULTI_VALUE_COLUMN_REMEDY_COMMAND);
434+
expect(entry.message, type).toContain(manualJsonConversionSql('postgres', 'proj_task', 'tags'));
435+
}
436+
});
437+
438+
it('leaves the MULTI-VALUE message byte-identical — the CLI recovers the dialect from it', () => {
439+
// ⚠️ The contract this card was most able to break. Widening the finding's
440+
// population must not move one character of the message the array half
441+
// emits, because `planStaleColumnTargets` reads the dialect by containment.
442+
// (Proven against `origin/main` itself while the change was written: the
443+
// whole entry — message included — compared equal for `lookup`, `string`
444+
// and `file` with `multiple: true`, on both dialects.)
445+
for (const dialect of ['postgres', 'mysql'] as const) {
446+
const [entry] = diffTags({ type: 'lookup', multiple: true }, staleColumn('character varying', 255), dialect);
447+
expect(entry.message).toContain(manualJsonConversionSql(dialect, 'proj_task', 'tags'));
448+
expect(entry.message).toContain(MULTI_VALUE_COLUMN_REMEDY_COMMAND);
449+
expect(entry.message).toContain('metadata declares a multi-value field (stored as `json`)');
450+
}
451+
});
452+
453+
it('a single-value JSON-class field with a maxLength reports the base type ONCE, never `narrow_varchar`', () => {
454+
// The same trap the multi-value case fell into, one population to the left:
455+
// before this change `{ type: 'file', maxLength: 50 }` over a
456+
// `varchar(255)` column reported `narrow_varchar` at category
457+
// **destructive** — inviting `os migrate apply --allow-destructive` to
458+
// rewrite the column to `varchar(50)`, the exact opposite of the repair it
459+
// needs. `createColumn` never sizes a JSON-class column from `maxLength`.
460+
for (const dialect of ['postgres', 'mysql'] as const) {
461+
const out = diffTags({ type: 'file', maxLength: 50 }, staleColumn('character varying', 255), dialect);
462+
expect(out.map((d) => d.op.type)).toEqual(['manual_column_type_change']);
463+
}
464+
});
465+
});
466+
300467
// ── Half 2: end to end, on every provisioned dialect ────────────────────────
301468

302469
const TABLE = 'os11535_task';

0 commit comments

Comments
 (0)