Skip to content

Commit 7ec7d8d

Browse files
committed
test(metadata-protocol,cli): seed the sequences fixtures with the key the platform actually stores
CI caught the #12394 handoff writing a SECOND counter row for one logical sequence, on live MySQL and on SQLite alike. Root cause is the fixtures, not the repair: both hand-seeded `key_hash` as an invented string (`'h1'`/`'h2'` and `'h_global'`/`'h_org'`), which was inert for as long as the repair addressed counter rows by `(object, field, tenant_id)`. #12394 addresses the destination row by `key_hash` — the table's only key — so an invented hash describes a table no install can hold: the org row reads ABSENT and a second row is inserted beside it. Measured: the driver stores `key_hash = sha256(object US tenant US field US scope)` for every row it writes, and `ensureSequencesKeyHashShape` recomputes the same hash for every legacy row it migrates. - cli: take the hash from the driver's own `sequenceKeyHash`, so the fixture is the same bytes the only production writer would have written and cannot drift. - metadata-protocol live-MySQL: spell the derivation independently (this package does not depend on driver-sql), making it a third spelling and therefore a pin on it; give `key_hash` its real PRIMARY KEY. - metadata-protocol unit: new #12394 suite over a KEYED store that answers the probe by its parameter and enforces the primary key. The INSERT-vs-UPDATE decision had no unit coverage keyed by a real hash — every existing fake matched on statement shape and handed back its one row for any key. `sequenceKeyHash` is exported from the module for that suite; it is NOT re-exported from the package index, so the published surface is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o
1 parent 47fee77 commit 7ec7d8d

4 files changed

Lines changed: 218 additions & 6 deletions

File tree

packages/cli/src/utils/platform-migrations-arming.integration.test.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,24 @@ async function writeDamagedInstall(): Promise<void> {
149149
t.bigInteger('last_value').notNullable().defaultTo(0);
150150
t.timestamp('updated_at');
151151
});
152+
// [#12394] `key_hash` is DERIVED, never invented. It used to be seeded here as
153+
// the placeholders `'h1'`/`'h2'`, which was harmless only for as long as the
154+
// repair addressed counter rows by `(object, field, tenant_id)`: nothing read
155+
// the column, so any string did. #12394's handoff addresses the destination
156+
// row by `key_hash` — the key the table is actually keyed on — so a fixture
157+
// carrying an invented hash describes a table the platform cannot produce, and
158+
// the repair correctly reads the org row as ABSENT and inserts a SECOND one.
159+
//
160+
// Taking the hash from the driver's own `sequenceKeyHash` rather than
161+
// re-spelling it here is the point: this fixture is now the same bytes the
162+
// only production writer of this table would have written, and it cannot drift
163+
// from it. `ensureSequencesKeyHashShape` recomputes the same hash for every
164+
// legacy row it migrates, so this is the shape of every real install.
165+
const keyHash = (object: string, tenantId: string, field: string, scope = ''): string =>
166+
(seed as any).sequenceKeyHash(object, tenantId, field, scope);
152167
await k(SEQUENCES_TABLE).insert([
153-
{ key_hash: 'h1', object: 'crm_case', tenant_id: GLOBAL_TENANT, field: 'case_number', scope: '', last_value: SEEDED_LAST_VALUE },
154-
{ key_hash: 'h2', object: 'crm_case', tenant_id: ORG_ID, field: 'case_number', scope: '', last_value: 1 },
168+
{ key_hash: keyHash('crm_case', GLOBAL_TENANT, 'case_number'), object: 'crm_case', tenant_id: GLOBAL_TENANT, field: 'case_number', scope: '', last_value: SEEDED_LAST_VALUE },
169+
{ key_hash: keyHash('crm_case', ORG_ID, 'case_number'), object: 'crm_case', tenant_id: ORG_ID, field: 'case_number', scope: '', last_value: 1 },
155170
]);
156171
await seed.disconnect();
157172
}

packages/metadata-protocol/src/migrations/seed-tenancy-backfill.live-mysql.test.ts

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
*/
4747

4848
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
49+
import { createHash } from 'node:crypto';
4950
import mysql from 'mysql2/promise';
5051
import {
5152
backfillSeedTenancy,
@@ -68,6 +69,32 @@ const DB = currentLiveMysqlDatabase();
6869
const OBJECT = 'os9381_case';
6970
const FIELD = 'case_number';
7071

72+
/**
73+
* The row key of `_objectstack_sequences`, spelled the way its only production
74+
* writer spells it (#12394).
75+
*
76+
* This fixture used to seed `key_hash` as the placeholders `'h_global'` and
77+
* `'h_org'`. That was invisible for as long as the repair addressed counter rows
78+
* by `(object, field, tenant_id)` — nothing read the column, so any string did.
79+
* #12394's handoff addresses the destination row by `key_hash`, which is the key
80+
* the table is actually keyed on, so an invented hash describes a table no
81+
* install can hold: the repair reads the organization row as ABSENT and inserts
82+
* a SECOND counter for one logical sequence.
83+
*
84+
* Spelled here rather than imported because `metadata-protocol` does not depend
85+
* on `driver-sql` — which makes this a THIRD independent spelling of the same
86+
* derivation, and therefore a pin on it: a separator or field-order change in
87+
* `seed-tenancy-backfill.ts` stops matching these rows and this suite goes red.
88+
* The separator is the ASCII unit separator, written as the escape \u001f and
89+
* never as a raw control byte — the same discipline the module and the driver
90+
* both keep.
91+
*/
92+
function sequenceKeyHash(object: string, tenantId: string, field: string, scope: string): string {
93+
return createHash('sha256')
94+
.update(`${object}\u001f${tenantId}\u001f${field}\u001f${scope}`)
95+
.digest('hex');
96+
}
97+
7198
if (!MYSQL_URL && EXPECT_LIVE) {
7299
describe('#9381 live MySQL', () => {
73100
it('OS_TEST_MYSQL_URL must be set — this runner declared it provisioned a server', () => {
@@ -113,9 +140,18 @@ describe.skipIf(!MYSQL_URL)('#9381 seed-tenancy backfill on a LIVE MySQL', () =>
113140
// Column names spelled the way the driver's own `createSequencesTable`
114141
// spells them; `last_value` is quoted here for the same reason the migration
115142
// has to quote it (see the reserved-word assertion below).
143+
//
144+
// [#12394] `key_hash` carries its real PRIMARY KEY. The driver declares it
145+
// `.notNullable().primary()`, and it is the ONLY key this table has — no
146+
// unique index stands behind `(object, tenant_id, field, scope)`. Seeding it
147+
// as a plain column let a repair that wrote a SECOND row for one logical
148+
// counter land quietly as an extra row instead of an `ER_DUP_ENTRY`; with
149+
// the real key here, that defect can only ever be an error on the two
150+
// dialects that enforce it.
116151
await conn.query(
117152
`CREATE TABLE \`${SEQUENCES_TABLE}\` (` +
118-
'`key_hash` VARCHAR(64), `object` VARCHAR(64), `tenant_id` VARCHAR(64), ' +
153+
'`key_hash` VARCHAR(64) NOT NULL PRIMARY KEY, `object` VARCHAR(64), ' +
154+
'`tenant_id` VARCHAR(64), ' +
119155
'`field` VARCHAR(64), `scope` VARCHAR(255) NOT NULL DEFAULT \'\', ' +
120156
'`last_value` INT, `updated_at` DATETIME(3))',
121157
);
@@ -127,8 +163,12 @@ describe.skipIf(!MYSQL_URL)('#9381 seed-tenancy backfill on a LIVE MySQL', () =>
127163
await conn.query("INSERT INTO `sys_organization` (`id`) VALUES ('org_live')");
128164
await conn.query(
129165
`INSERT INTO \`${SEQUENCES_TABLE}\` (\`key_hash\`, \`object\`, \`tenant_id\`, \`field\`, \`last_value\`) ` +
130-
`VALUES ('h_global', '${OBJECT}', '${GLOBAL_TENANT}', '${FIELD}', 38), ` +
131-
`('h_org', '${OBJECT}', 'org_live', '${FIELD}', 4)`,
166+
`VALUES (?, '${OBJECT}', '${GLOBAL_TENANT}', '${FIELD}', 38), ` +
167+
`(?, '${OBJECT}', 'org_live', '${FIELD}', 4)`,
168+
[
169+
sequenceKeyHash(OBJECT, GLOBAL_TENANT, FIELD, ''),
170+
sequenceKeyHash(OBJECT, 'org_live', FIELD, ''),
171+
],
132172
);
133173
// The card's own repro: seeded rows carry NULL, API rows carry the org, and
134174
// CASE-00001/2 were minted on BOTH sides.

packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import {
4343
buildCounterInsertSql,
4444
buildGlobalCounterDeleteByKeyHashSql,
4545
buildSequencesPresenceSql,
46+
sequenceKeyHash,
4647
SEQUENCES_TABLE,
4748
GLOBAL_TENANT,
4849
ORGANIZATION_FIELD,
@@ -361,6 +362,157 @@ describe('#9381 dialect-aware statement text', () => {
361362
});
362363
});
363364

365+
/**
366+
* #12394 — the counter handoff, against a store that is KEYED.
367+
*
368+
* ## The hole this closes
369+
*
370+
* Every other fake seam in this file answers by STATEMENT SHAPE: it matches on
371+
* `sql.includes('"key_hash" = ?')` and hands back the one row it is holding,
372+
* without ever reading the parameter. A fake like that models a database in
373+
* which every key addresses the same row — so the one decision this handoff
374+
* makes, INSERT-vs-UPDATE keyed by `key_hash`, is answered correctly no matter
375+
* what key the migration asks with. The branch cannot fail here, which is the
376+
* same thing as saying it is not covered.
377+
*
378+
* It was not covered anywhere else at unit speed either: the UPDATE branch (an
379+
* organization-scoped row that ALREADY exists) reached CI only through two
380+
* integration fixtures, and both of them seeded `key_hash` as an invented string
381+
* — `'h1'`/`'h2'` and `'h_global'`/`'h_org'`. Those were inert for as long as the
382+
* repair addressed counter rows by `(object, field, tenant_id)`. The moment it
383+
* began addressing them by `key_hash`, both fixtures described a table no
384+
* install can hold, the destination row read as ABSENT, and the repair inserted
385+
* a SECOND counter for one logical sequence — on SQLite and on MySQL alike, so
386+
* this was never a dialect gap.
387+
*
388+
* ## What makes this fixture able to fail
389+
*
390+
* It is a real keyed store: rows live in a `Map` under their own `key_hash`, the
391+
* probe answers by the parameter it was given, and the INSERT REFUSES a key that
392+
* is already present — because `key_hash` is the table's PRIMARY KEY and its
393+
* only key (no unique index stands behind `(object, tenant_id, field, scope)`).
394+
* A fake looser than the producer is how a dead write path ships green; this one
395+
* is exactly as strict. A repair that wrote its mark under a key other than the
396+
* one it probed now ends with two rows or a refused write, and both are red.
397+
*/
398+
describe('#12394 the counter handoff writes the row the driver will read', () => {
399+
const OBJECT = 'crm_case';
400+
const FIELD = 'case_number';
401+
const ORG = 'org_a';
402+
403+
/** A `_objectstack_sequences` that is keyed the way the real table is keyed. */
404+
function keyedSequences(seed: Array<Record<string, unknown>>) {
405+
const rows = new Map<string, Record<string, unknown>>();
406+
for (const row of seed) rows.set(String(row.key_hash), { ...row });
407+
const exec = async (sql: string, params: unknown[] = []): Promise<unknown> => {
408+
if (sql.includes('WHERE 1 = 0')) return []; // presence + key-shape probes
409+
if (sql.includes('LEFT JOIN')) {
410+
return [
411+
{ object: OBJECT, field: FIELD, global_last_value: 38, organization_last_value: 1 },
412+
];
413+
}
414+
if (sql.includes(ORGANIZATION_TABLE)) return [{ id: ORG }];
415+
if (sql.includes('rows_holding')) return [];
416+
if (sql.startsWith('INSERT') && sql.includes(SEQUENCES_TABLE)) {
417+
const key = String(params[0]);
418+
// The PRIMARY KEY, enforced. Without this the fixture would absorb the
419+
// very defect it exists to catch.
420+
if (rows.has(key)) throw new Error(`duplicate key value violates the primary key: ${key}`);
421+
rows.set(key, {
422+
key_hash: key,
423+
object: String(params[1]),
424+
tenant_id: String(params[2]),
425+
field: String(params[3]),
426+
scope: String(params[4]),
427+
last_value: Number(params[5]),
428+
});
429+
return [];
430+
}
431+
if (sql.startsWith('UPDATE') && sql.includes(SEQUENCES_TABLE)) {
432+
const row = rows.get(String(params[1]));
433+
if (row) row.last_value = Number(params[0]);
434+
return [];
435+
}
436+
if (sql.startsWith('DELETE') && sql.includes(SEQUENCES_TABLE)) {
437+
rows.delete(String(params[0]));
438+
return [];
439+
}
440+
if (sql.startsWith('UPDATE') || sql.startsWith('DELETE')) return []; // the stamp
441+
// The org-scoped row, addressed by the key the caller actually asked with.
442+
if (sql.includes('"key_hash" = ?')) {
443+
const row = rows.get(String(params[0]));
444+
return row ? [{ last_value: row.last_value }] : [];
445+
}
446+
// The `__global__` rows for one object/field — one per scope.
447+
if (sql.includes('tenant_id')) {
448+
return [...rows.values()].filter((r) => r.tenant_id === GLOBAL_TENANT);
449+
}
450+
return [];
451+
};
452+
return { rows, seam: { exec } as never };
453+
}
454+
455+
const globalRow = {
456+
key_hash: sequenceKeyHash(OBJECT, GLOBAL_TENANT, FIELD, ''),
457+
object: OBJECT,
458+
tenant_id: GLOBAL_TENANT,
459+
field: FIELD,
460+
scope: '',
461+
last_value: 38,
462+
};
463+
464+
it('[first boot] creates the organization row at the merged mark, then retires __global__', async () => {
465+
const { rows, seam } = keyedSequences([globalRow]);
466+
const result = await backfillSeedTenancy(seam);
467+
468+
expect(result.status).toBe('applied');
469+
// One row, under the key the DRIVER will compute — not under any key.
470+
expect([...rows.keys()]).toEqual([sequenceKeyHash(OBJECT, ORG, FIELD, '')]);
471+
expect([...rows.values()][0]!.last_value).toBe(38);
472+
});
473+
474+
it('[the CI regression] an existing organization row is RAISED, never duplicated', async () => {
475+
// The shape both integration fixtures were really in, spelled with the key
476+
// the platform actually stores. A repair that probes with one key and writes
477+
// under another leaves two rows here, which is what CI caught.
478+
const { rows, seam } = keyedSequences([
479+
globalRow,
480+
{
481+
key_hash: sequenceKeyHash(OBJECT, ORG, FIELD, ''),
482+
object: OBJECT,
483+
tenant_id: ORG,
484+
field: FIELD,
485+
scope: '',
486+
last_value: 1,
487+
},
488+
]);
489+
const result = await backfillSeedTenancy(seam);
490+
491+
expect(result.status).toBe('applied');
492+
expect(rows.size).toBe(1);
493+
expect([...rows.values()][0]).toMatchObject({ tenant_id: ORG, last_value: 38 });
494+
});
495+
496+
it('[never lowered] an organization row already ahead of __global__ keeps its own mark', async () => {
497+
const { rows, seam } = keyedSequences([
498+
globalRow,
499+
{
500+
key_hash: sequenceKeyHash(OBJECT, ORG, FIELD, ''),
501+
object: OBJECT,
502+
tenant_id: ORG,
503+
field: FIELD,
504+
scope: '',
505+
last_value: 91,
506+
},
507+
]);
508+
await backfillSeedTenancy(seam);
509+
510+
expect(rows.size).toBe(1);
511+
// The merge rule is the greater of the two COUNTERS, never the data max.
512+
expect([...rows.values()][0]!.last_value).toBe(91);
513+
});
514+
});
515+
364516
/**
365517
* #9451 — the durable receipt.
366518
*

packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -709,7 +709,12 @@ export function buildGlobalCounterDeleteSql(client?: string): string {
709709
*
710710
* The separator is the ASCII unit separator, spelled as the escape \u001f and never as a raw control byte — the driver spells it the same way.
711711
*/
712-
function sequenceKeyHash(object: string, tenantId: string, field: string, scope: string): string {
712+
export function sequenceKeyHash(
713+
object: string,
714+
tenantId: string,
715+
field: string,
716+
scope: string,
717+
): string {
713718
return createHash('sha256')
714719
.update(`${object}\u001f${tenantId}\u001f${field}\u001f${scope}`)
715720
.digest('hex');

0 commit comments

Comments
 (0)