Skip to content

Commit ad217b1

Browse files
os-zhuangos-samclaude
authored
fix(metadata-protocol): compile the seed-tenancy migration statements for the connected dialect, so they run on MySQL (#9381) (#9440)
* fix(metadata-protocol): compile the seed-tenancy migration statements for the connected dialect (#9381) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTKPDRoynY8i3HmdSFUxFj * test(ci): run the metadata-protocol migration statements against the live MySQL (#9381) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTKPDRoynY8i3HmdSFUxFj --------- Co-authored-by: os-steve <sam@objectstack.ai> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent f63afb2 commit ad217b1

12 files changed

Lines changed: 601 additions & 64 deletions

File tree

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
---
2+
"@objectstack/metadata-protocol": minor
3+
"@objectstack/runtime": patch
4+
---
5+
6+
fix(metadata-protocol): compile the seed-tenancy backfill's statements for the connected dialect, so they run on MySQL (#9381)
7+
8+
`seed-tenancy-backfill.ts` quoted every identifier the ANSI way (`"x"`) on every
9+
dialect. MySQL does not run with `ANSI_QUOTES` — measured on a live MySQL 8.0.46,
10+
whose `sql_mode` is
11+
`ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION`,
12+
and nothing in `driver-sql` sets one — so `"x"` is a string literal there and all
13+
seven statements failed with `ER_PARSE_ERROR`. The repair for #8686 therefore
14+
never ran on MySQL, silently: a migration must not fail a boot, so every call site
15+
turns the failure into a warning and the symptom was a skipped repair in the log
16+
rather than an error.
17+
18+
The statements are now compiled for the driver actually connected, and the seam
19+
carries the dialect with it (`resolveSeedTenancySeam` returns `{ exec, client }`;
20+
`backfillSeedTenancy` takes that pair) so a caller cannot lose it. Two further
21+
MySQL-only defects in the same statements, both measured on the same server, are
22+
fixed with it: `last_value` is a reserved word on MySQL 8.0 and is now quoted
23+
wherever it is unqualified, and the stamp's exclusion sub-SELECTs go through a
24+
derived table because MySQL refuses `UPDATE t … (SELECT … FROM t)` with
25+
`ER_UPDATE_TABLE_USED`. SQLite and PostgreSQL keep the exact ANSI spelling they
26+
had (both re-verified live).
27+
28+
`resolveSeedTenancyExec` stays exported and unchanged for callers that resolve the
29+
dialect themselves; `backfillSeedTenancy` now takes the seam object instead of a
30+
bare exec.

.github/workflows/ci.yml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,33 @@ jobs:
734734
--filter @objectstack/service-analytics \
735735
test
736736
737+
# ── The migration statements, on the live MySQL (#9381) ───────────────
738+
#
739+
# `metadata-protocol` builds raw SQL by hand for the three dialects the
740+
# platform supports, and MySQL is the only one that can fail: it does not
741+
# run with `ANSI_QUOTES`, so the ANSI `"identifier"` the seed-tenancy
742+
# backfill used to emit unconditionally was a STRING LITERAL there and
743+
# every statement was an `ER_PARSE_ERROR`. Nothing surfaced it, because a
744+
# migration must never fail a boot — every call site turns the failure
745+
# into a warning, so the symptom was a skipped repair in the log.
746+
#
747+
# This rides this job rather than getting its own because this is where a
748+
# live MySQL already exists. Only the live file runs: the rest of the
749+
# package's suite has no server axis and runs in Test Core.
750+
- name: Build metadata-protocol and its dependencies
751+
run: pnpm exec turbo run build --filter=@objectstack/metadata-protocol... --concurrency=4
752+
753+
- name: Run the metadata-protocol migration statements against live MySQL
754+
env:
755+
OS_TEST_MYSQL_URL: mysql://root:root@127.0.0.1:3306/conformance
756+
# Same vacuous-pass guard as the driver-sql leg: this runner
757+
# provisioned the server, so a missing URL is a defect in the runner
758+
# and must be a red rather than a skip.
759+
OS_EXPECT_LIVE_DIALECT_MATRIX: '1'
760+
run: |
761+
pnpm --filter @objectstack/metadata-protocol exec vitest run \
762+
src/migrations/seed-tenancy-backfill.live-mysql.test.ts
763+
737764
dogfood:
738765
# Sharded 3-way: the suite is ~60 independent test files, each booting its
739766
# own in-process app; a single 4-vCPU runner needed ~7½ minutes for the

packages/cli/src/commands/migrate/duplicates.pre-repair.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ describe('#8928 — why it must run BEFORE the #8686 backfill', () => {
131131
]);
132132

133133
// The real repair, run exactly as a boot would run it.
134-
const repair = await backfillSeedTenancy(exec);
134+
const repair = await backfillSeedTenancy({ exec, client: 'better-sqlite3' });
135135
expect(repair.status).toBe('applied');
136136

137137
const afterRepair = await report();

packages/metadata-protocol/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
},
4444
"devDependencies": {
4545
"@types/node": "^26.1.2",
46+
"mysql2": "^3.23.1",
4647
"tsup": "^8.5.1",
4748
"typescript": "^6.0.3",
4849
"vitest": "^4.1.10"

packages/metadata-protocol/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ export type {
7979
// twice (maintainer ruling 2026-08-15: contract option 1, stored data shape 2).
8080
export {
8181
backfillSeedTenancy,
82+
resolveSeedTenancySeam,
8283
resolveSeedTenancyExec,
8384
normalizeRows,
8485
buildSequencesPresenceSql,
@@ -93,6 +94,7 @@ export {
9394
ORGANIZATION_FIELD,
9495
ORGANIZATION_TABLE,
9596
} from './migrations/seed-tenancy-backfill.js';
97+
export type { SeedTenancySeam } from './migrations/seed-tenancy-backfill.js';
9698
export type {
9799
SeedTenancyExec,
98100
SeedTenancyLogger,
Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,217 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #9381 — the seed-tenancy backfill's statements, RUN on a live MySQL.
5+
*
6+
* ## Why this file exists rather than one more text pin
7+
*
8+
* The defect it guards is invisible on the two dialects the rest of the suite
9+
* runs on. `"identifier"` is correct ANSI on SQLite and PostgreSQL, so a
10+
* SQLite-only or Postgres-only test passes with the bug fully present. MySQL is
11+
* the only dialect that can fail — it does not run with `ANSI_QUOTES`, so `"x"`
12+
* is a STRING LITERAL there — and the module's own header names MySQL as a
13+
* supported backend. A migration whose statements cannot parse on a backend the
14+
* module claims is declared ≠ enforced, and the reason it never surfaced is that
15+
* every call site swallows a migration failure into a warning by design: on
16+
* MySQL the symptom was a skipped repair in the boot log, not an error.
17+
*
18+
* ## Non-vacuity
19+
*
20+
* The suite ASSERTS the server is not running with `ANSI_QUOTES` before it
21+
* asserts anything else. On a server that had it, these statements would parse
22+
* with the bug present and a green run would mean nothing — the same
23+
* vacuous-pass hole `live-dialect-matrix.testkit.ts` closes for the timezone
24+
* axis, and the exact condition #9381's premise step had to rule out.
25+
*
26+
* ## Provisioning
27+
*
28+
* Needs `OS_TEST_MYSQL_URL` (same variable the driver-sql live matrix uses) and
29+
* reports a named SKIP without one — never a silent pass. A runner that knows it
30+
* provisioned the server sets `OS_EXPECT_LIVE_DIALECT_MATRIX=1`, which turns the
31+
* missing URL into a failure so a dropped `env:` line cannot quietly return this
32+
* seam to no coverage at all.
33+
*
34+
* Everything runs in its OWN database (`os_metadata_protocol_9381`), created on
35+
* the spot, because two of the three tables this migration touches have fixed
36+
* platform names (`_objectstack_sequences`, `sys_organization`) that other live
37+
* suites also use.
38+
*/
39+
40+
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
41+
import mysql from 'mysql2/promise';
42+
import {
43+
backfillSeedTenancy,
44+
buildCollisionProbeSql,
45+
buildCounterMergeSql,
46+
buildGlobalCounterDeleteSql,
47+
buildOrganizationProbeSql,
48+
buildSequencesPresenceSql,
49+
buildSplitProbeSql,
50+
buildStampSql,
51+
GLOBAL_TENANT,
52+
SEQUENCES_TABLE,
53+
type SeedTenancySeam,
54+
} from './seed-tenancy-backfill.js';
55+
56+
const MYSQL_URL = process.env.OS_TEST_MYSQL_URL;
57+
const EXPECT_LIVE = process.env.OS_EXPECT_LIVE_DIALECT_MATRIX === '1';
58+
const DB = 'os_metadata_protocol_9381';
59+
const OBJECT = 'os9381_case';
60+
const FIELD = 'case_number';
61+
62+
if (!MYSQL_URL && EXPECT_LIVE) {
63+
describe('#9381 live MySQL', () => {
64+
it('OS_TEST_MYSQL_URL must be set — this runner declared it provisioned a server', () => {
65+
throw new Error(
66+
'OS_EXPECT_LIVE_DIALECT_MATRIX=1 without OS_TEST_MYSQL_URL: the live MySQL cell for ' +
67+
'the metadata-protocol migrations would have been skipped, returning #9381 to zero ' +
68+
'coverage on the only dialect that can exhibit it.',
69+
);
70+
});
71+
});
72+
}
73+
74+
describe.skipIf(!MYSQL_URL)('#9381 seed-tenancy backfill on a LIVE MySQL', () => {
75+
let conn: mysql.Connection;
76+
let seam: SeedTenancySeam;
77+
78+
beforeAll(async () => {
79+
const bootstrap = await mysql.createConnection(MYSQL_URL!);
80+
await bootstrap.query(`CREATE DATABASE IF NOT EXISTS \`${DB}\``);
81+
await bootstrap.end();
82+
83+
conn = await mysql.createConnection(`${MYSQL_URL}`);
84+
await conn.query(`USE \`${DB}\``);
85+
// The one session SET `SqlDriver` itself performs on a mysql connection.
86+
await conn.query(`SET time_zone = '+00:00'`);
87+
88+
seam = {
89+
exec: (sql: string, params?: unknown[]) => conn.query(sql, params ?? []),
90+
client: 'mysql2',
91+
};
92+
});
93+
94+
afterAll(async () => {
95+
if (!conn) return;
96+
await conn.query(`DROP DATABASE IF EXISTS \`${DB}\``);
97+
await conn.end();
98+
});
99+
100+
const seedFixture = async (): Promise<void> => {
101+
await conn.query(`DROP TABLE IF EXISTS \`${SEQUENCES_TABLE}\``);
102+
await conn.query(`DROP TABLE IF EXISTS \`${OBJECT}\``);
103+
await conn.query(`DROP TABLE IF EXISTS \`sys_organization\``);
104+
// Column names spelled the way the driver's own `createSequencesTable`
105+
// spells them; `last_value` is quoted here for the same reason the migration
106+
// has to quote it (see the reserved-word assertion below).
107+
await conn.query(
108+
`CREATE TABLE \`${SEQUENCES_TABLE}\` (` +
109+
'`key_hash` VARCHAR(64), `object` VARCHAR(64), `tenant_id` VARCHAR(64), ' +
110+
'`field` VARCHAR(64), `scope` VARCHAR(255) NOT NULL DEFAULT \'\', ' +
111+
'`last_value` INT, `updated_at` DATETIME(3))',
112+
);
113+
await conn.query(
114+
`CREATE TABLE \`${OBJECT}\` (` +
115+
'`id` VARCHAR(64), `case_number` VARCHAR(64), `organization_id` VARCHAR(64))',
116+
);
117+
await conn.query('CREATE TABLE `sys_organization` (`id` VARCHAR(64))');
118+
await conn.query("INSERT INTO `sys_organization` (`id`) VALUES ('org_live')");
119+
await conn.query(
120+
`INSERT INTO \`${SEQUENCES_TABLE}\` (\`key_hash\`, \`object\`, \`tenant_id\`, \`field\`, \`last_value\`) ` +
121+
`VALUES ('h_global', '${OBJECT}', '${GLOBAL_TENANT}', '${FIELD}', 38), ` +
122+
`('h_org', '${OBJECT}', 'org_live', '${FIELD}', 4)`,
123+
);
124+
// The card's own repro: seeded rows carry NULL, API rows carry the org, and
125+
// CASE-00001/2 were minted on BOTH sides.
126+
await conn.query(
127+
`INSERT INTO \`${OBJECT}\` (\`id\`, \`case_number\`, \`organization_id\`) VALUES ` +
128+
"('s1','CASE-00001',NULL),('s2','CASE-00002',NULL),('s3','CASE-00003',NULL)," +
129+
"('a1','CASE-00001','org_live'),('a2','CASE-00002','org_live')",
130+
);
131+
};
132+
133+
it('the server is NOT running with ANSI_QUOTES — without this the run proves nothing', async () => {
134+
const [rows] = await conn.query('SELECT @@session.sql_mode AS sql_mode, VERSION() AS version');
135+
const mode = String((rows as Array<{ sql_mode: string }>)[0]!.sql_mode);
136+
// Printed so the CI log carries the measurement, not just the verdict.
137+
// eslint-disable-next-line no-console
138+
console.log(
139+
`[#9381] live MySQL ${(rows as Array<{ version: string }>)[0]!.version} sql_mode=${mode}`,
140+
);
141+
expect(mode).not.toContain('ANSI_QUOTES');
142+
});
143+
144+
it('every statement the migration builds PARSES and runs on MySQL', async () => {
145+
await seedFixture();
146+
const client = 'mysql2';
147+
const statements: Array<[string, string, unknown[]]> = [
148+
['presence probe', buildSequencesPresenceSql(client), []],
149+
['split probe', buildSplitProbeSql(client), [GLOBAL_TENANT, GLOBAL_TENANT]],
150+
['organization probe', buildOrganizationProbeSql(client), []],
151+
['collision probe', buildCollisionProbeSql(OBJECT, FIELD, client), []],
152+
['stamp', buildStampSql(OBJECT, [FIELD], client), ['org_live']],
153+
['counter merge', buildCounterMergeSql(client), [38, OBJECT, FIELD, 'org_live']],
154+
['global counter delete', buildGlobalCounterDeleteSql(client), [OBJECT, FIELD, GLOBAL_TENANT]],
155+
];
156+
for (const [label, sql, params] of statements) {
157+
// A failure here names the statement AND its text — the parse error alone
158+
// does not say which builder produced it.
159+
await expect(
160+
conn.query(sql, params),
161+
`${label} must run on MySQL — statement: ${sql}`,
162+
).resolves.toBeDefined();
163+
}
164+
});
165+
166+
it('a multi-autonumber object stamps with one derived table per guard', async () => {
167+
await seedFixture();
168+
await conn.query(`ALTER TABLE \`${OBJECT}\` ADD COLUMN \`ticket_no\` VARCHAR(64)`);
169+
// Two guards in ONE statement: a repeated derived-table alias would be
170+
// ER_NONUNIQ_TABLE, and the un-wrapped form ER_UPDATE_TABLE_USED.
171+
await expect(
172+
conn.query(buildStampSql(OBJECT, [FIELD, 'ticket_no'], 'mysql2'), ['org_live']),
173+
).resolves.toBeDefined();
174+
});
175+
176+
it('repairs the split end to end, and reports the already-minted duplicates', async () => {
177+
await seedFixture();
178+
const warnings: string[] = [];
179+
const result = await backfillSeedTenancy(seam, {
180+
warn: (m: string) => warnings.push(m),
181+
info: () => {},
182+
} as never);
183+
184+
expect(result.status).toBe('applied');
185+
expect(result.organizationId).toBe('org_live');
186+
expect(result.splits).toEqual([
187+
{ object: OBJECT, field: FIELD, globalLastValue: 38, organizationLastValue: 4 },
188+
]);
189+
// Reported, never renumbered — the two values minted on both sides.
190+
expect(result.collisions.map((c) => c.value).sort()).toEqual(['CASE-00001', 'CASE-00002']);
191+
192+
// The movable row moved; the two colliding rows kept their NULL.
193+
const [rows] = await conn.query(
194+
`SELECT \`id\`, \`organization_id\` FROM \`${OBJECT}\` ORDER BY \`id\``,
195+
);
196+
const byId = Object.fromEntries(
197+
(rows as Array<{ id: string; organization_id: string | null }>).map((r) => [
198+
r.id,
199+
r.organization_id,
200+
]),
201+
);
202+
expect(byId.s3).toBe('org_live');
203+
expect(byId.s1).toBeNull();
204+
expect(byId.s2).toBeNull();
205+
206+
// The counters were merged at max(last_value) and the `__global__` row retired.
207+
const [counters] = await conn.query(
208+
`SELECT \`tenant_id\`, \`last_value\` FROM \`${SEQUENCES_TABLE}\` ORDER BY \`tenant_id\``,
209+
);
210+
expect(counters).toEqual([{ tenant_id: 'org_live', last_value: 38 }]);
211+
});
212+
213+
it('is idempotent — a second run finds no split', async () => {
214+
const second = await backfillSeedTenancy(seam);
215+
expect(second.status).toBe('no-split');
216+
});
217+
});

0 commit comments

Comments
 (0)