From f7b251cbae1a077f1236fdea3458666d90c9a6ea Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 09:44:56 +0000 Subject: [PATCH 1/2] fix(metadata-protocol): write the merged autonumber high-water mark before retiring the __global__ counter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #8686 seed/API tenancy handoff ran an UPDATE of the organization-scoped _objectstack_sequences row followed by an unconditional DELETE of the '__global__' one. On a fresh install there is no organization-scoped row yet, so the UPDATE matched nothing (a success on every dialect), the DELETE ran anyway, and the counter table was left empty — sending SqlDriver.getNextSequenceValue back into its one-time MAX(data) bootstrap and re-issuing an already-allocated business identifier. The handoff is now one ordered decision per scope: write the merged mark (INSERT when the destination row is absent, UPDATE when it is not), read it back, and only then retire the '__global__' row by its own stored key_hash. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o --- ...ed-tenancy-counter-handoff-writes-first.md | 50 +++ .../seed-tenancy-backfill.null-seam.test.ts | 18 +- .../migrations/seed-tenancy-backfill.test.ts | 130 +++++++- .../src/migrations/seed-tenancy-backfill.ts | 297 ++++++++++++++++-- ...nancy-autonumber-split.integration.test.ts | 148 ++++++++- 5 files changed, 598 insertions(+), 45 deletions(-) create mode 100644 .changeset/seed-tenancy-counter-handoff-writes-first.md diff --git a/.changeset/seed-tenancy-counter-handoff-writes-first.md b/.changeset/seed-tenancy-counter-handoff-writes-first.md new file mode 100644 index 0000000000..18f4043ba9 --- /dev/null +++ b/.changeset/seed-tenancy-counter-handoff-writes-first.md @@ -0,0 +1,50 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +fix(metadata-protocol): the #8686 tenancy backfill writes the merged autonumber high-water mark before it deletes anything (#12394) + +The seed/API tenancy handoff destroyed the counter it was supposed to move. It ran two +independent statements — an `UPDATE` of the organization-scoped `_objectstack_sequences` +row, then an unconditional `DELETE` of the `'__global__'` one — and on a **fresh install** +there is no organization-scoped row yet, because no API create has happened. The `UPDATE` +matched nothing, which is a success on every dialect; the `DELETE` ran regardless; the +counter table was left empty. `SqlDriver.getNextSequenceValue` then re-entered the +`if (!existing)` bootstrap its own docstring reserves for first allocation, re-derived the +counter from `MAX(data)`, and **re-issued a business identifier that had already been +handed out** — measured on 17.1.0: `ACC-000009` on two different records. + +The zero-row case is the *normal* first-boot shape, not an edge case: it is precisely the +shape `buildSplitProbeSql`'s `LEFT JOIN` was widened to catch, so the repair fired on +exactly the installs where its merge loop body never executed. + +The handoff is now one ordered decision per scope: + +1. **write** the merged mark — `INSERT` when the organization-scoped row is absent, + `UPDATE` when it exists; +2. **read it back** — "the statement did not throw" was never evidence a row was written, + and an `UPDATE` matching zero rows is exactly the defect above; +3. **then** retire the `'__global__'` row, addressed by its own stored `key_hash`, so a + retirement can only ever hit the row whose mark was just merged. + +A throw at any step leaves the `'__global__'` row in place — which is the state the next +boot's split probe detects and retries — so a failed repair now loses nothing. + +Per **scope**, because a `{YYYYMMDD}` / `{field}` / per-parent format runs one counter row +per rendered prefix. The old merge was scope-blind in both directions: it could raise every +scope's counter to one merged value, and it deleted every scope's `'__global__'` row. + +The merge rule itself is unchanged and is the 2026-08-15 ruling's: the greater of the two +**counters**, never the data max. That rule is the whole point — a counter is allowed to +sit ahead of its rows (a rolled-back insert burns a number, by design), and that gap is +exactly what the old handoff threw away. + +Graded **patch**: a defect repair inside an existing migration. It adds no export to +`@objectstack/metadata-protocol`'s public index — the new SQL builders are module-scoped +for their own unit tests, matching the index's own recorded rule that an export added so a +test can import a value is the shape to catch before it ships. + +No change to the allocator. Reaching `if (!existing)` is not evidence of lost state — a new +tenant, a new day and a new `{field}` group each reach it legitimately, and a destroyed +counter leaves no row behind to tell the two apart — so a guard there would fire on the hot +path and still not detect this. The repair belongs where the state was destroyed. diff --git a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.null-seam.test.ts b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.null-seam.test.ts index 58566d9495..150fb776d0 100644 --- a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.null-seam.test.ts +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.null-seam.test.ts @@ -250,9 +250,17 @@ describe('#10789 a seam that answers with no rows still reports no-split', () => // installs it exists for. This seam answers every SELECT and hands back a // NON-result-set for every write — and the repair must still complete. const writes: string[] = []; - const exec: SeedTenancyExec = async (sql: string) => { - if (sql.startsWith('UPDATE') || sql.startsWith('DELETE')) { + // #12394: the handoff reads its own write back before retiring anything, so + // the counter row is modelled rather than assumed — the WRITES still answer + // with a non-result-set, which is what this case exists to pin. + let orgCounter: Record | undefined; + const exec: SeedTenancyExec = async (sql: string, params?: unknown[]) => { + if (sql.startsWith('UPDATE') || sql.startsWith('DELETE') || sql.startsWith('INSERT')) { writes.push(sql.slice(0, 6)); + if (sql.startsWith('INSERT')) orgCounter = { last_value: Number(params?.[5]) }; + if (sql.startsWith('UPDATE') && sql.includes('_objectstack_sequences')) { + orgCounter = { last_value: Number(params?.[0]) }; + } return { affectedRows: 3 }; // not a result set, by design } if (sql.includes('WHERE 1 = 0')) return []; @@ -263,7 +271,8 @@ describe('#10789 a seam that answers with no rows still reports no-split', () => } if (sql.includes(ORGANIZATION_TABLE)) return [{ id: 'org_a' }]; if (sql.includes('rows_holding')) return []; - if (sql.includes('tenant_id')) return [{ tenant_id: 'org_a', last_value: 1 }]; + if (sql.includes('"key_hash" = ?')) return orgCounter ? [orgCounter] : []; + if (sql.includes('tenant_id')) return [{ key_hash: 'hash-global', scope: '', last_value: 38 }]; return []; }; @@ -274,6 +283,9 @@ describe('#10789 a seam that answers with no rows still reports no-split', () => expect(result.organizationId).toBe('org_a'); expect(writes).toContain('UPDATE'); expect(writes).toContain('DELETE'); + // The write that used to be missing: on a fresh install there is no + // organization-scoped counter row to raise, so the mark is INSERTed. + expect(writes).toContain('INSERT'); }); }); diff --git a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts index 19cf040200..9f6be67559 100644 --- a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts @@ -36,6 +36,12 @@ import { buildStampSql, buildCounterMergeSql, buildGlobalCounterDeleteSql, + buildSequencesKeyShapeProbeSql, + buildGlobalCounterProbeSql, + buildCounterRowProbeSql, + buildCounterMergeByKeyHashSql, + buildCounterInsertSql, + buildGlobalCounterDeleteByKeyHashSql, buildSequencesPresenceSql, SEQUENCES_TABLE, GLOBAL_TENANT, @@ -132,6 +138,72 @@ describe('#8686 SQL builders', () => { it('the presence probe reads no rows', () => { expect(buildSequencesPresenceSql()).toContain('WHERE 1 = 0'); }); + + // ── #12394 — the handoff writes before it deletes ───────────────────────── + // + // The old merge was an UPDATE against a row that does not exist on a fresh + // install, followed by an unconditional DELETE. These pin the two statements + // that make the write REACH somewhere, and the key they are addressed by. + + it('the key-shape probe reads no rows and names key_hash', () => { + expect(buildSequencesKeyShapeProbeSql()).toContain('WHERE 1 = 0'); + expect(buildSequencesKeyShapeProbeSql()).toContain('"key_hash"'); + }); + + it('the counter INSERT names every column the current table shape requires', () => { + const modern = buildCounterInsertSql(true); + // `key_hash` is NOT NULL with no default — an INSERT that omitted it could + // not land at all — and `scope` is what keeps a per-day/per-group counter + // from collapsing into another scope's row. + for (const column of ['key_hash', 'object', 'tenant_id', 'field', 'scope', 'last_value']) { + expect(modern).toContain(`"${column}"`); + } + expect(modern).toContain('VALUES (?, ?, ?, ?, ?, ?)'); + // `updated_at` is left to the column default: one less column to be wrong + // about on a table shape this module did not create. + expect(modern).not.toContain('"updated_at"'); + }); + + it('the legacy shape is addressed by the three columns the driver falls back to', () => { + // A table whose `key_hash` rebuild has not run or did not succeed. The + // driver keys those by (object, tenant_id, field); so does this. + const legacy = buildCounterInsertSql(false); + expect(legacy).toContain('VALUES (?, ?, ?, ?)'); + expect(legacy).not.toContain('"key_hash"'); + expect(legacy).not.toContain('"scope"'); + expect(buildCounterRowProbeSql(false)).toContain( + 'WHERE "object" = ? AND "field" = ? AND "tenant_id" = ?', + ); + expect(buildGlobalCounterProbeSql(false)).not.toContain('"scope"'); + }); + + it('the modern shape is addressed by key_hash alone, and so is the retirement', () => { + expect(buildCounterRowProbeSql(true)).toContain('WHERE "key_hash" = ?'); + expect(buildCounterMergeByKeyHashSql()).toContain('WHERE "key_hash" = ?'); + // The `__global__` row is retired by its OWN stored hash — read back, never + // recomputed — so a scope spelled wrong cannot delete a row it did not merge. + expect(buildGlobalCounterDeleteByKeyHashSql()).toContain('DELETE FROM "_objectstack_sequences"'); + expect(buildGlobalCounterDeleteByKeyHashSql()).toContain('WHERE "key_hash" = ?'); + expect(buildGlobalCounterProbeSql(true)).toContain('"key_hash"'); + expect(buildGlobalCounterProbeSql(true)).toContain('"scope"'); + }); + + it('every new counter statement binds its values and names no literal tenant', () => { + for (const sql of [ + buildSequencesKeyShapeProbeSql(), + buildGlobalCounterProbeSql(true), + buildGlobalCounterProbeSql(false), + buildCounterRowProbeSql(true), + buildCounterRowProbeSql(false), + buildCounterMergeByKeyHashSql(), + buildCounterInsertSql(true), + buildCounterInsertSql(false), + buildGlobalCounterDeleteByKeyHashSql(), + ]) { + expect(sql).not.toContain(GLOBAL_TENANT); + expect(sql).toContain(SEQUENCES_TABLE); + } + }); }); describe('#8686 identifier gate', () => { @@ -192,6 +264,15 @@ describe('#9381 dialect-aware statement text', () => { buildStampSql('crm_case', ['case_number'], client), buildCounterMergeSql(client), buildGlobalCounterDeleteSql(client), + buildSequencesKeyShapeProbeSql(client), + buildGlobalCounterProbeSql(true, client), + buildGlobalCounterProbeSql(false, client), + buildCounterRowProbeSql(true, client), + buildCounterRowProbeSql(false, client), + buildCounterMergeByKeyHashSql(client), + buildCounterInsertSql(true, client), + buildCounterInsertSql(false, client), + buildGlobalCounterDeleteByKeyHashSql(client), ]; for (const sql of statements) { expect(sql).not.toContain('"'); @@ -206,9 +287,19 @@ describe('#9381 dialect-aware statement text', () => { it('quotes `last_value` — a RESERVED word on MySQL 8.0 — wherever it is unqualified', () => { // `LAST_VALUE()` is a window function there, so a bare `last_value` is a // parse error even when the table name is spelled correctly. Measured. - const merge = buildCounterMergeSql(client); - expect(merge).toContain(`SET ${bt('last_value')} = ?`); - expect(merge).not.toMatch(/(? { @@ -234,6 +325,15 @@ describe('#9381 dialect-aware statement text', () => { buildStampSql('crm_case', ['case_number'], client), buildCounterMergeSql(client), buildGlobalCounterDeleteSql(client), + buildSequencesKeyShapeProbeSql(client), + buildGlobalCounterProbeSql(true, client), + buildGlobalCounterProbeSql(false, client), + buildCounterRowProbeSql(true, client), + buildCounterRowProbeSql(false, client), + buildCounterMergeByKeyHashSql(client), + buildCounterInsertSql(true, client), + buildCounterInsertSql(false, client), + buildGlobalCounterDeleteByKeyHashSql(client), ]; for (const sql of statements) { expect(sql).not.toContain(BACKTICK); @@ -275,11 +375,17 @@ describe('#9381 dialect-aware statement text', () => { describe('#9451 the seed-tenancy repair leaves a durable receipt', () => { /** One split object, one organization, no collisions — drives the repair to `applied`. */ function fakeSeamExec(overrides: { collisions?: Record[] } = {}) { - return async (sql: string, _params?: unknown[]): Promise => { + // The organization-scoped counter row, held as a one-row store. #12394's + // handoff WRITES the merged high-water mark and READS IT BACK before it + // retires the `__global__` row, so a fixture that answered the read-back + // with nothing would be modelling a database that silently dropped the + // write — and the repair would correctly refuse to delete anything. + let orgCounter: Record | undefined; + return async (sql: string, params?: unknown[]): Promise => { // Dispatched on the statements the module actually compiles, so a builder // that changed shape breaks this fixture rather than silently turning it // into a healthy install. - if (sql.includes('WHERE 1 = 0')) return []; // presence probe + if (sql.includes('WHERE 1 = 0')) return []; // presence + key-shape probes if (sql.includes('LEFT JOIN')) { return [ { @@ -292,8 +398,20 @@ describe('#9451 the seed-tenancy repair leaves a durable receipt', () => { } if (sql.includes(ORGANIZATION_TABLE)) return [{ id: 'org_a' }]; if (sql.includes('rows_holding')) return overrides.collisions ?? []; + if (sql.startsWith('INSERT')) { + orgCounter = { last_value: Number(params?.[5]) }; + return []; + } + if (sql.startsWith('UPDATE') && sql.includes(SEQUENCES_TABLE)) { + orgCounter = { last_value: Number(params?.[0]) }; + return []; + } if (sql.startsWith('UPDATE') || sql.startsWith('DELETE')) return []; - if (sql.includes('tenant_id')) return [{ tenant_id: 'org_a', last_value: 1 }]; + // The organization-scoped row, addressed the way the driver addresses it. + if (sql.includes('"key_hash" = ?')) return orgCounter ? [orgCounter] : []; + // The `__global__` rows for one object/field — one per scope, carrying the + // stored hash the retirement is addressed by. + if (sql.includes('tenant_id')) return [{ key_hash: 'hash-global', scope: '', last_value: 38 }]; return []; }; } diff --git a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts index d7daae0241..194c5eed60 100644 --- a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts @@ -105,8 +105,37 @@ * rolled-back transaction, rows deleted since). Taking the counter high-water * mark can only ever skip numbers; taking the data max could re-issue one that * a burned allocation already handed out. + * + * ## The handoff writes BEFORE it deletes (#12394) + * + * The first implementation of the merge lost the very mark the paragraph above + * insists on. It ran an `UPDATE` of the organization-scoped counter row and then + * a `DELETE` of the `__global__` one, as two independent statements — and on a + * FRESH install, which is the normal shape rather than an edge case, there is no + * organization-scoped row yet: no API create has happened. The `UPDATE` matched + * nothing, which is a SUCCESS on every dialect; the `DELETE` ran regardless; and + * `_objectstack_sequences` was left empty. `SqlDriver.getNextSequenceValue` then + * re-entered the one-time `MAX(data)` bootstrap its own docstring reserves for + * first allocation, re-derived the counter from the surviving rows, and handed a + * business identifier out A SECOND TIME — measured on 17.1.0: `ACC-000009` on + * two different records, because one number had been burned. + * + * So the handoff is now one ordered decision instead of two hopeful statements: + * write the merged mark (INSERT when the destination row is absent, UPDATE when + * it is not), READ IT BACK, and only then retire the `__global__` row — per + * scope, since a scoped format runs one counter row per rendered prefix. See + * {@link mergeSplitCounter}. + * + * ⛔ What this deliberately does NOT do is teach the allocator to notice that it + * is bootstrapping a second time. Reaching `if (!existing)` is not evidence of + * lost state: a brand-new tenant, a new day, a new `{field}` group each reach it + * legitimately, on a mature install, constantly — and a destroyed counter leaves + * no row behind to tell the two apart. A guard there would fire on the hot path + * and still not detect this defect. The repair belongs where the state is + * destroyed, which is here. */ +import { createHash } from 'node:crypto'; import { resolveTenancyPosture } from '@objectstack/types'; import { postureEnforcesWall } from '@objectstack/spec/security'; import { DATA_MIGRATION_FLAG_OBJECT, type DataMigrationFlag } from '@objectstack/spec/system'; @@ -657,15 +686,228 @@ export function buildGlobalCounterDeleteSql(client?: string): string { ); } -/** The organization-scoped counter rows for one split object/field. */ -function buildOrgCounterProbeSql(client?: string): string { +/** + * SHA-256 of the composite counter key — the `_objectstack_sequences` primary + * key, exactly as `SqlDriver.sequenceKeyHash` computes it (#12394). + * + * Re-spelled here rather than imported, for the same reason {@link GLOBAL_TENANT} + * is: `metadata-protocol` must not depend on a driver. The duplication is real, + * and it is the reason this module used to DELETE the `__global__` row rather + * than move it — "a hash spelled two ways is a counter the driver cannot find". + * That objection was right about the hazard and wrong about the remedy: deleting + * the row hands the merged high-water mark to nobody, and the driver's one-time + * `MAX(data)` bootstrap then re-derives a number it has ALREADY handed out. + * + * So the duplication is kept and CONTROLLED instead. `packages/runtime`'s + * `seed-tenancy-autonumber-split.integration.test.ts` drives the real + * `SeedLoaderService`, a real `ObjectQL` and a real `SqlDriver` across a BURNED + * number, on a fixed-prefix and on a `{field}`-scoped format: a hash disagreeing + * with the driver's by one byte leaves a row the driver never reads, the driver + * re-enters its bootstrap, and the burned number is re-issued — which is exactly + * what those two cases assert does not happen. Divergence is a red test, not a + * silent counter. + * + * 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. + */ +function sequenceKeyHash(object: string, tenantId: string, field: string, scope: string): string { + return createHash('sha256') + .update(`${object}\u001f${tenantId}\u001f${field}\u001f${scope}`) + .digest('hex'); +} + +/** + * Does the counter table carry the current `key_hash` primary key? + * + * The one question that decides how a counter row is KEYED — and therefore how a + * missing organization-scoped row has to be created. The driver answers it for + * itself with an in-process flag (`SqlDriver.sequencesHasKeyHash`) this module + * cannot see, so it is asked of the DATABASE, in the same `WHERE 1 = 0` idiom + * {@link buildSequencesPresenceSql} already uses: a table without the column + * refuses the statement, and the refusal IS the answer. + * + * `key_hash` implies `scope` — `createSequencesTable` has only ever created the + * two together — so one probe settles both. A table without it is one whose + * `ensureSequencesKeyHashShape` rebuild has not run or did not succeed; the + * driver keys those by `(object, tenant_id, field)` and refuses per-scope + * formats outright. Matching that here is not a fallback dialect — it is the + * same key the driver itself uses against the same table. + */ +export function buildSequencesKeyShapeProbeSql(client?: string): string { + return `SELECT ${quoteIdent('key_hash', client)} FROM ${quoteIdent(SEQUENCES_TABLE, client)} WHERE 1 = 0`; +} + +/** + * The `__global__` counter rows for one split object/field — one per scope. + * + * The sibling of {@link buildCounterRowProbeSql}, and the row this migration + * actually has to MOVE. It reads the STORED `key_hash` rather than recomputing + * it, so each row is retired by its own identity: a scope this module spelled + * wrong can then never delete a row it did not just merge. + */ +export function buildGlobalCounterProbeSql(hasKeyHash: boolean, client?: string): string { + const q = (name: string) => quoteIdent(name, client); + const columns = hasKeyHash + ? `${q('key_hash')}, ${q('scope')}, ${q('last_value')}` + : q('last_value'); + return ( + `SELECT ${columns} FROM ${q(SEQUENCES_TABLE)} ` + + `WHERE ${q('object')} = ? AND ${q('field')} = ? AND ${q('tenant_id')} = ?` + ); +} + +/** + * Read ONE counter row by the key the driver uses on this table shape. + * + * Called twice per scope, for two different jobs: to decide INSERT vs UPDATE, + * and — after the write — to VERIFY that the merged mark actually landed, before + * anything is deleted. The second read is the guard the original handoff never + * had: an `UPDATE` matching no row is a SUCCESS on every dialect, so "the + * statement did not throw" was never evidence that the mark had been written. + */ +export function buildCounterRowProbeSql(hasKeyHash: boolean, client?: string): string { const q = (name: string) => quoteIdent(name, client); + const where = hasKeyHash + ? `${q('key_hash')} = ?` + : `${q('object')} = ? AND ${q('field')} = ? AND ${q('tenant_id')} = ?`; + return `SELECT ${q('last_value')} FROM ${q(SEQUENCES_TABLE)} WHERE ${where}`; +} + +/** Raise an EXISTING organization-scoped counter row, addressed by `key_hash`. */ +export function buildCounterMergeByKeyHashSql(client?: string): string { + const q = (name: string) => quoteIdent(name, client); + // `last_value` unqualified is an `ER_PARSE_ERROR` on MySQL 8.0 — the reserved + // `LAST_VALUE()` window function there. See {@link buildCounterMergeSql}. return ( - `SELECT ${q('tenant_id')}, ${q('last_value')} FROM ${q(SEQUENCES_TABLE)} ` + - `WHERE ${q('object')} = ? AND ${q('field')} = ? AND ${q('tenant_id')} <> ?` + `UPDATE ${q(SEQUENCES_TABLE)} SET ${q('last_value')} = ?, ` + + `${q('updated_at')} = CURRENT_TIMESTAMP WHERE ${q('key_hash')} = ?` ); } +/** + * Create the organization-scoped counter row AT the merged high-water mark. + * + * The statement this handoff was missing. On a FRESH install — the normal first + * boot, not an edge case — there is no organization-scoped row to raise, because + * no API create has happened yet; the `UPDATE` matched nothing, reported success, + * and the mark was then deleted with the `__global__` row. + * + * `updated_at` is left to the column default rather than named here: it is + * defaulted in every shape of this table, and naming it would be one more column + * to be wrong about on a legacy install. + */ +export function buildCounterInsertSql(hasKeyHash: boolean, client?: string): string { + const q = (name: string) => quoteIdent(name, client); + const columns = hasKeyHash + ? [q('key_hash'), q('object'), q('tenant_id'), q('field'), q('scope'), q('last_value')] + : [q('object'), q('tenant_id'), q('field'), q('last_value')]; + return ( + `INSERT INTO ${q(SEQUENCES_TABLE)} (${columns.join(', ')}) ` + + `VALUES (${columns.map(() => '?').join(', ')})` + ); +} + +/** + * Move one object/field's `__global__` high-water mark INTO the organization, + * and only then retire the row it came from (#12394). + * + * ## The order, and why every step of it is load-bearing + * + * The handoff this replaces did two independent things and hoped they added up: + * it `UPDATE`d the organization-scoped counter row, then `DELETE`d the + * `__global__` one unconditionally. On a fresh install — the normal shape, right + * after the first sign-up — there IS no organization-scoped row yet, so the + * UPDATE matched nothing (a success, on every dialect), the DELETE ran anyway, + * and the counter table was left EMPTY. The driver then re-entered its one-time + * `MAX(data)` bootstrap and re-issued an identifier that had already been handed + * out: measured on 17.1.0, `ACC-000009` minted twice, to two different records. + * + * So the three steps are ordered and each is verified: + * + * 1. **Write, per scope.** A scoped format (`{YYYYMMDD}`, `{field}`, per-parent) + * runs one counter row per rendered prefix, so the mark is moved scope by + * scope, keyed the way the driver keys it. `INSERT` when the destination row + * is absent, `UPDATE` when it exists — the absent case is the FIRST-BOOT + * case, not an edge one. + * 2. **Read it back.** "The statement did not throw" is not evidence that a row + * was written: an `UPDATE` matching zero rows throws nowhere, and that is + * precisely the defect above. The destination row is re-read and must hold at + * least the merged value. + * 3. **Then delete** — the `__global__` row, by its own stored `key_hash`, so + * the retirement can only ever hit the row whose mark was just merged. + * + * A throw at any point leaves this object's `__global__` row in place, which is + * the state the next boot's split probe detects and retries. Partial progress is + * safe for the same reason: a scope already merged and retired is simply no + * longer split. + * + * The merge rule itself is unchanged and is the ruling's: the greater of the two + * COUNTERS, never the data max — a counter is allowed to sit ahead of its rows, + * and that gap is exactly what must survive the handoff. + */ +async function mergeSplitCounter( + exec: SeedTenancyExec, + client: string | undefined, + hasKeyHash: boolean, + object: string, + field: string, + organizationId: string, +): Promise { + const globalRows = await selectRows(exec, buildGlobalCounterProbeSql(hasKeyHash, client), [ + object, + field, + GLOBAL_TENANT, + ]); + for (const globalRow of globalRows) { + const scope = hasKeyHash && globalRow.scope != null ? String(globalRow.scope) : ''; + const globalValue = toNumber(globalRow.last_value); + // How the DRIVER addresses the destination row on this table shape. + const orgKey = hasKeyHash + ? [sequenceKeyHash(object, organizationId, field, scope)] + : [object, field, organizationId]; + + const probe = buildCounterRowProbeSql(hasKeyHash, client); + const existing = await selectRows(exec, probe, orgKey); + const merged = Math.max(globalValue, existing.length > 0 ? toNumber(existing[0].last_value) : 0); + + if (existing.length > 0) { + await exec( + hasKeyHash ? buildCounterMergeByKeyHashSql(client) : buildCounterMergeSql(client), + hasKeyHash ? [merged, ...orgKey] : [merged, object, field, organizationId], + ); + } else { + await exec( + buildCounterInsertSql(hasKeyHash, client), + hasKeyHash + ? [orgKey[0], object, organizationId, field, scope, merged] + : [object, organizationId, field, merged], + ); + } + + const landed = await selectRows(exec, probe, orgKey); + const landedValue = landed.length > 0 ? toNumber(landed[0].last_value) : -1; + if (landedValue < merged) { + throw new Error( + `the organization-scoped counter for ${object}.${field}` + + (scope === '' ? '' : ` (scope ${JSON.stringify(scope)})`) + + ` reads ${landedValue < 0 ? 'ABSENT' : String(landedValue)} after the merge, expected at ` + + `least ${merged} — the '${GLOBAL_TENANT}' row is NOT retired`, + ); + } + + if (hasKeyHash) { + await exec(buildGlobalCounterDeleteByKeyHashSql(client), [String(globalRow.key_hash)]); + } else { + await exec(buildGlobalCounterDeleteSql(client), [object, field, GLOBAL_TENANT]); + } + } +} + +/** Retire ONE `__global__` counter row, by its own stored `key_hash`. */ +export function buildGlobalCounterDeleteByKeyHashSql(client?: string): string { + const q = (name: string) => quoteIdent(name, client); + return `DELETE FROM ${q(SEQUENCES_TABLE)} WHERE ${q('key_hash')} = ?`; +} + function toNumber(value: unknown): number { const n = Number(value); return Number.isFinite(n) ? n : 0; @@ -1081,6 +1323,17 @@ export async function backfillSeedTenancy( } } + // The counter table's key shape, asked ONCE — it is a property of the + // database, not of a split, and it decides how every write below is addressed. + // Unreadable for any reason reads as the legacy shape, which is the + // conservative answer: it is the key the driver falls back to as well. + let hasKeyHash = false; + try { + hasKeyHash = isResultSet(await exec(buildSequencesKeyShapeProbeSql(client))); + } catch { + hasKeyHash = false; + } + for (const split of splits) { // A counter that describes a partition the rows never reached would be a // false receipt — the exact shape the stamp-then-merge ordering exists to @@ -1088,38 +1341,16 @@ export async function backfillSeedTenancy( // same split and retries the whole repair. if (stampFailures.includes(split.object)) continue; try { - const orgRows = await selectRows(exec, buildOrgCounterProbeSql(client), [ - split.object, - split.field, - GLOBAL_TENANT, - ]); - for (const row of orgRows) { - const tenantId = row.tenant_id == null ? '' : String(row.tenant_id); - if (!tenantId) continue; - // The ruling's merge rule: the greater of the two COUNTERS, never the - // data max — a counter is allowed to sit ahead of its rows. - const merged = Math.max(split.globalLastValue, toNumber(row.last_value)); - await exec(buildCounterMergeSql(client), [merged, split.object, split.field, tenantId]); - } - // Retire the `__global__` counter last. - // - // When there was NO organization-scoped counter (the fresh-install case, - // `orgRows` empty) this delete is the whole reconciliation, and it is - // deliberately a delete rather than an insert of a replacement row. The - // driver keys counters by a `key_hash` it computes in app code, so writing - // a new row from here would mean re-spelling that hash in a second place — - // and a hash spelled two ways is a counter the driver cannot find. - // - // Deleting instead hands the job to the driver's own first-allocation - // bootstrap, which is already exactly right: `getNextSequenceValue` sees no - // row, scans `scanMaxNumericTail` SCOPED TO THE RESOLVED TENANT — which, the - // stamp above having just run, now includes the adopted seed rows — and - // starts at that max + 1. One tested code path, no duplicated hashing. - await exec(buildGlobalCounterDeleteSql(client), [split.object, split.field, GLOBAL_TENANT]); + // Re-entrant by construction: the `__global__` rows this reads are the + // ones it retires, so a second pass over a duplicated (object, field) — + // which the split probe produces whenever an object holds several scopes — + // finds nothing left to move. + await mergeSplitCounter(exec, client, hasKeyHash, split.object, split.field, organizationId); } catch (e) { logger?.warn?.( `[metadata-protocol] seed tenancy backfill could not merge the counter for ` + - `${split.object}.${split.field} (#8686)`, + `${split.object}.${split.field} (#8686) — the '${GLOBAL_TENANT}' counter is left in ` + + `place, so the high-water mark is intact and the next boot retries the repair`, { error: (e as Error).message }, ); } diff --git a/packages/runtime/src/seed-tenancy-autonumber-split.integration.test.ts b/packages/runtime/src/seed-tenancy-autonumber-split.integration.test.ts index f8f74cf77f..48e4bf946a 100644 --- a/packages/runtime/src/seed-tenancy-autonumber-split.integration.test.ts +++ b/packages/runtime/src/seed-tenancy-autonumber-split.integration.test.ts @@ -143,6 +143,15 @@ const readSequences = async (driver: any) => lastValue: Number(r.last_value), })); +/** The same rows WITH their scope — a `{field}`/date format runs one per scope. */ +const readScopedSequences = async (driver: any) => + (await driver.knex('_objectstack_sequences').select('tenant_id', 'scope', 'last_value').orderBy('scope')) + .map((r: any) => ({ + tenant: String(r.tenant_id), + scope: String(r.scope), + lastValue: Number(r.last_value), + })); + const readDuplicates = async (driver: any) => driver .knex('crm_case') @@ -205,8 +214,12 @@ describe('#8686 seed/API tenancy split — autonumber scope', () => { // Every seeded row now carries the organization — one tenancy contract for // both write paths, which is what option 1 says. expect(await countUntenanted(driver)).toBe(0); - // The `__global__` pseudo-tenant is gone as a peer of the real organization. - expect(await readSequences(driver)).toEqual([]); + // The `__global__` pseudo-tenant is gone as a peer of the real organization + // — and its HIGH-WATER MARK did not go with it (#12394). This assertion used + // to read `toEqual([])`, which pinned the defect as if it were the contract: + // an empty counter table is exactly what sends the driver back into its + // one-time `MAX(data)` bootstrap. + expect(await readSequences(driver)).toEqual([{ tenant: ORG_ID, lastValue: SEEDED_ROWS }]); // The API path now continues the SAME sequence instead of restarting it. const numbers: string[] = []; @@ -336,12 +349,141 @@ describe('#8686 seed/API tenancy split — autonumber scope', () => { await createOrganization(engine); expect(await countUntenanted(driver)).toBe(0); - expect(await readSequences(driver)).toEqual([]); + // Merged into the organization's own row, not deleted along with the + // `__global__` one (#12394). + expect(await readSequences(driver)).toEqual([{ tenant: ORG_ID, lastValue: SEEDED_ROWS }]); // And the very first API create on this install already continues the seed's // sequence, which is the outcome the card measured going wrong. expect((await apiCreate(engine, 'api 1')).case_number).toBe('CASE-00039'); }); + /** + * #12394 — the handoff must carry the COUNTER's high-water mark, not the data + * max. Both cases below BURN a number first, and that is the whole point. + * + * ⚠️ A pin that asserts "no duplicates after seed + sign-up + create" is GREEN + * with and without the defect: with no number burned, the `MAX(data)` rescan + * the driver falls back to lands on exactly the value the counter held, so the + * two candidate mechanisms are indistinguishable. Only a burned number — a + * value the platform has already handed out that no surviving row holds — + * separates them, and `SqlDriver.getNextSequenceValue`'s own docstring names + * that state as BY DESIGN ("a rolled-back insert burns a number", and after + * the one-time bootstrap "the data table is never consulted again"). + * + * Deleting the highest-numbered row is the stand-in the card measured with: + * it leaves the counter at 38 and the data max at 37, which is precisely the + * shape a rolled-back allocation leaves behind. + */ + it('[#12394] a burned number survives the handoff — the merged counter is written, not destroyed', async () => { + const { driver, engine } = await bootInstall(); + await seedFreshInstall(engine); + + // Burn the top number. The counter stays at 38; the data max is now 37. + await (driver as any).knex('crm_case').where({ case_number: 'CASE-00038' }).delete(); + expect(await readSequences(driver)).toEqual([{ tenant: GLOBAL_TENANT, lastValue: SEEDED_ROWS }]); + const dataMax = String( + (await (driver as any).knex('crm_case').max({ m: 'case_number' }))[0].m, + ); + expect(dataMax).toBe('CASE-00037'); + + await createOrganization(engine); + const result = await backfillSeedTenancy(resolveSeedTenancySeam(engine), createLogger() as any); + expect(result.status).toBe('applied'); + + // The mechanism: the merged high-water mark is WRITTEN into the + // organization's own counter row. Before this fix the merge targeted a row + // that did not exist yet, the `__global__` row was deleted unconditionally, + // and this table came back empty. + expect(await readSequences(driver)).toEqual([{ tenant: ORG_ID, lastValue: SEEDED_ROWS }]); + + // The consequence: CASE-00038 is NOT handed out a second time. + expect((await apiCreate(engine, 'api 1')).case_number).toBe('CASE-00039'); + expect((await apiCreate(engine, 'api 2')).case_number).toBe('CASE-00040'); + }); + + /** + * #12394, per-scope half — the same handoff on a `{field}`-scoped format. + * + * A scoped autonumber runs ONE counter row per rendered prefix, keyed by a + * `key_hash` over `(object, tenant, field, scope)`. So the merge cannot be + * scope-blind in either direction: writing one row for the object/field pair + * would leave every other scope's mark destroyed, and writing a row under the + * wrong `scope` produces a counter the driver can never find — which reads + * exactly like the destroyed mark it was supposed to repair. + * + * This case is also the cross-package agreement pin for the row key: if the + * hash this migration computes disagreed with the driver's by one byte, the + * driver would miss the row, re-enter its `MAX(data)` bootstrap, and re-issue + * the burned number below. + */ + it('[#12394] each {field} scope keeps its own high-water mark across the handoff', async () => { + const ticketObject = { + name: 'crm_ticket', + fields: { + subject: { type: 'text' }, + region: { type: 'text' }, + organization_id: { type: 'text' }, + ticket_no: { type: 'autonumber', format: '{region}-{0000}', unique: 'organization' }, + }, + } as any; + + const driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + openDrivers.push(driver); + const engine = new ObjectQL(); + engine.registerDriver(driver as any, true); + await engine.init(); + engine.registry.registerObject(ticketObject, '#12394'); + engine.registry.registerObject(ORG_OBJECT, '#12394'); + await driver.initObjects([ticketObject, ORG_OBJECT]); + + // Seed-shaped writes: untenanted, because no organization exists yet. + const seed = (region: string, n: number) => + engine.insert( + 'crm_ticket', + { subject: `seeded ${region} ${n}`, region }, + { context: { isSystem: true } } as any, + ); + for (let i = 1; i <= 3; i++) await seed('EU', i); + for (let i = 1; i <= 2; i++) await seed('US', i); + + // Two independent counters, one per rendered scope. + expect(await readScopedSequences(driver)).toEqual([ + { tenant: GLOBAL_TENANT, scope: 'EU-', lastValue: 3 }, + { tenant: GLOBAL_TENANT, scope: 'US-', lastValue: 2 }, + ]); + + // Burn the top EU number only. EU data max drops to EU-0002; US is untouched. + await (driver as any).knex('crm_ticket').where({ ticket_no: 'EU-0003' }).delete(); + + await createOrganization(engine); + const result = await backfillSeedTenancy(resolveSeedTenancySeam(engine), createLogger() as any); + expect(result.status).toBe('applied'); + + // Both marks move into the organization, each under its OWN scope. + expect(await readScopedSequences(driver)).toEqual([ + { tenant: ORG_ID, scope: 'EU-', lastValue: 3 }, + { tenant: ORG_ID, scope: 'US-', lastValue: 2 }, + ]); + + // EU does not re-issue the burned EU-0003, and US continues its own run. + const eu = await engine.insert( + 'crm_ticket', + { subject: 'api eu', region: 'EU', organization_id: ORG_ID }, + { context: { isSystem: true } } as any, + ); + const us = await engine.insert( + 'crm_ticket', + { subject: 'api us', region: 'US', organization_id: ORG_ID }, + { context: { isSystem: true } } as any, + ); + expect(eu.ticket_no).toBe('EU-0004'); + expect(us.ticket_no).toBe('US-0003'); + }); + it('[GUARD] platform seeds stay global — sys_/cloud_/ai_ are never adopted', async () => { // The seed loader deliberately leaves platform-namespace seeds untenanted. // A backfill that adopted them would manufacture a NEW disagreement between From 7ec7d8d4b43e9d6e34b7fa8331876317f994ea46 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 12:05:32 +0000 Subject: [PATCH 2/2] test(metadata-protocol,cli): seed the sequences fixtures with the key the platform actually stores MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o --- ...form-migrations-arming.integration.test.ts | 19 ++- .../seed-tenancy-backfill.live-mysql.test.ts | 46 +++++- .../migrations/seed-tenancy-backfill.test.ts | 152 ++++++++++++++++++ .../src/migrations/seed-tenancy-backfill.ts | 7 +- 4 files changed, 218 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/utils/platform-migrations-arming.integration.test.ts b/packages/cli/src/utils/platform-migrations-arming.integration.test.ts index c20f6a5e74..6c2a23ac0e 100644 --- a/packages/cli/src/utils/platform-migrations-arming.integration.test.ts +++ b/packages/cli/src/utils/platform-migrations-arming.integration.test.ts @@ -149,9 +149,24 @@ async function writeDamagedInstall(): Promise { t.bigInteger('last_value').notNullable().defaultTo(0); t.timestamp('updated_at'); }); + // [#12394] `key_hash` is DERIVED, never invented. It used to be seeded here as + // the placeholders `'h1'`/`'h2'`, which was harmless only for as long as the + // repair addressed counter rows by `(object, field, tenant_id)`: nothing read + // the column, so any string did. #12394's handoff addresses the destination + // row by `key_hash` — the key the table is actually keyed on — so a fixture + // carrying an invented hash describes a table the platform cannot produce, and + // the repair correctly reads the org row as ABSENT and inserts a SECOND one. + // + // Taking the hash from the driver's own `sequenceKeyHash` rather than + // re-spelling it here is the point: this fixture is now the same bytes the + // only production writer of this table would have written, and it cannot drift + // from it. `ensureSequencesKeyHashShape` recomputes the same hash for every + // legacy row it migrates, so this is the shape of every real install. + const keyHash = (object: string, tenantId: string, field: string, scope = ''): string => + (seed as any).sequenceKeyHash(object, tenantId, field, scope); await k(SEQUENCES_TABLE).insert([ - { key_hash: 'h1', object: 'crm_case', tenant_id: GLOBAL_TENANT, field: 'case_number', scope: '', last_value: SEEDED_LAST_VALUE }, - { key_hash: 'h2', object: 'crm_case', tenant_id: ORG_ID, field: 'case_number', scope: '', last_value: 1 }, + { 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 }, + { key_hash: keyHash('crm_case', ORG_ID, 'case_number'), object: 'crm_case', tenant_id: ORG_ID, field: 'case_number', scope: '', last_value: 1 }, ]); await seed.disconnect(); } diff --git a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.live-mysql.test.ts b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.live-mysql.test.ts index 6f7c0c3b95..9c7b46b4ee 100644 --- a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.live-mysql.test.ts +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.live-mysql.test.ts @@ -46,6 +46,7 @@ */ import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { createHash } from 'node:crypto'; import mysql from 'mysql2/promise'; import { backfillSeedTenancy, @@ -68,6 +69,32 @@ const DB = currentLiveMysqlDatabase(); const OBJECT = 'os9381_case'; const FIELD = 'case_number'; +/** + * The row key of `_objectstack_sequences`, spelled the way its only production + * writer spells it (#12394). + * + * This fixture used to seed `key_hash` as the placeholders `'h_global'` and + * `'h_org'`. That was invisible for as long as the repair addressed counter rows + * by `(object, field, tenant_id)` — nothing read the column, so any string did. + * #12394's handoff addresses the destination row by `key_hash`, which is the key + * the table is actually keyed on, so an invented hash describes a table no + * install can hold: the repair reads the organization row as ABSENT and inserts + * a SECOND counter for one logical sequence. + * + * Spelled here rather than imported because `metadata-protocol` does not depend + * on `driver-sql` — which makes this a THIRD independent spelling of the same + * derivation, and therefore a pin on it: a separator or field-order change in + * `seed-tenancy-backfill.ts` stops matching these rows and this suite goes red. + * The separator is the ASCII unit separator, written as the escape \u001f and + * never as a raw control byte — the same discipline the module and the driver + * both keep. + */ +function sequenceKeyHash(object: string, tenantId: string, field: string, scope: string): string { + return createHash('sha256') + .update(`${object}\u001f${tenantId}\u001f${field}\u001f${scope}`) + .digest('hex'); +} + if (!MYSQL_URL && EXPECT_LIVE) { describe('#9381 live MySQL', () => { 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', () => // Column names spelled the way the driver's own `createSequencesTable` // spells them; `last_value` is quoted here for the same reason the migration // has to quote it (see the reserved-word assertion below). + // + // [#12394] `key_hash` carries its real PRIMARY KEY. The driver declares it + // `.notNullable().primary()`, and it is the ONLY key this table has — no + // unique index stands behind `(object, tenant_id, field, scope)`. Seeding it + // as a plain column let a repair that wrote a SECOND row for one logical + // counter land quietly as an extra row instead of an `ER_DUP_ENTRY`; with + // the real key here, that defect can only ever be an error on the two + // dialects that enforce it. await conn.query( `CREATE TABLE \`${SEQUENCES_TABLE}\` (` + - '`key_hash` VARCHAR(64), `object` VARCHAR(64), `tenant_id` VARCHAR(64), ' + + '`key_hash` VARCHAR(64) NOT NULL PRIMARY KEY, `object` VARCHAR(64), ' + + '`tenant_id` VARCHAR(64), ' + '`field` VARCHAR(64), `scope` VARCHAR(255) NOT NULL DEFAULT \'\', ' + '`last_value` INT, `updated_at` DATETIME(3))', ); @@ -127,8 +163,12 @@ describe.skipIf(!MYSQL_URL)('#9381 seed-tenancy backfill on a LIVE MySQL', () => await conn.query("INSERT INTO `sys_organization` (`id`) VALUES ('org_live')"); await conn.query( `INSERT INTO \`${SEQUENCES_TABLE}\` (\`key_hash\`, \`object\`, \`tenant_id\`, \`field\`, \`last_value\`) ` + - `VALUES ('h_global', '${OBJECT}', '${GLOBAL_TENANT}', '${FIELD}', 38), ` + - `('h_org', '${OBJECT}', 'org_live', '${FIELD}', 4)`, + `VALUES (?, '${OBJECT}', '${GLOBAL_TENANT}', '${FIELD}', 38), ` + + `(?, '${OBJECT}', 'org_live', '${FIELD}', 4)`, + [ + sequenceKeyHash(OBJECT, GLOBAL_TENANT, FIELD, ''), + sequenceKeyHash(OBJECT, 'org_live', FIELD, ''), + ], ); // The card's own repro: seeded rows carry NULL, API rows carry the org, and // CASE-00001/2 were minted on BOTH sides. diff --git a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts index 9f6be67559..6440c02def 100644 --- a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts @@ -43,6 +43,7 @@ import { buildCounterInsertSql, buildGlobalCounterDeleteByKeyHashSql, buildSequencesPresenceSql, + sequenceKeyHash, SEQUENCES_TABLE, GLOBAL_TENANT, ORGANIZATION_FIELD, @@ -361,6 +362,157 @@ describe('#9381 dialect-aware statement text', () => { }); }); +/** + * #12394 — the counter handoff, against a store that is KEYED. + * + * ## The hole this closes + * + * Every other fake seam in this file answers by STATEMENT SHAPE: it matches on + * `sql.includes('"key_hash" = ?')` and hands back the one row it is holding, + * without ever reading the parameter. A fake like that models a database in + * which every key addresses the same row — so the one decision this handoff + * makes, INSERT-vs-UPDATE keyed by `key_hash`, is answered correctly no matter + * what key the migration asks with. The branch cannot fail here, which is the + * same thing as saying it is not covered. + * + * It was not covered anywhere else at unit speed either: the UPDATE branch (an + * organization-scoped row that ALREADY exists) reached CI only through two + * integration fixtures, and both of them seeded `key_hash` as an invented string + * — `'h1'`/`'h2'` and `'h_global'`/`'h_org'`. Those were inert for as long as the + * repair addressed counter rows by `(object, field, tenant_id)`. The moment it + * began addressing them by `key_hash`, both fixtures described a table no + * install can hold, the destination row read as ABSENT, and the repair inserted + * a SECOND counter for one logical sequence — on SQLite and on MySQL alike, so + * this was never a dialect gap. + * + * ## What makes this fixture able to fail + * + * It is a real keyed store: rows live in a `Map` under their own `key_hash`, the + * probe answers by the parameter it was given, and the INSERT REFUSES a key that + * is already present — because `key_hash` is the table's PRIMARY KEY and its + * only key (no unique index stands behind `(object, tenant_id, field, scope)`). + * A fake looser than the producer is how a dead write path ships green; this one + * is exactly as strict. A repair that wrote its mark under a key other than the + * one it probed now ends with two rows or a refused write, and both are red. + */ +describe('#12394 the counter handoff writes the row the driver will read', () => { + const OBJECT = 'crm_case'; + const FIELD = 'case_number'; + const ORG = 'org_a'; + + /** A `_objectstack_sequences` that is keyed the way the real table is keyed. */ + function keyedSequences(seed: Array>) { + const rows = new Map>(); + for (const row of seed) rows.set(String(row.key_hash), { ...row }); + const exec = async (sql: string, params: unknown[] = []): Promise => { + if (sql.includes('WHERE 1 = 0')) return []; // presence + key-shape probes + if (sql.includes('LEFT JOIN')) { + return [ + { object: OBJECT, field: FIELD, global_last_value: 38, organization_last_value: 1 }, + ]; + } + if (sql.includes(ORGANIZATION_TABLE)) return [{ id: ORG }]; + if (sql.includes('rows_holding')) return []; + if (sql.startsWith('INSERT') && sql.includes(SEQUENCES_TABLE)) { + const key = String(params[0]); + // The PRIMARY KEY, enforced. Without this the fixture would absorb the + // very defect it exists to catch. + if (rows.has(key)) throw new Error(`duplicate key value violates the primary key: ${key}`); + rows.set(key, { + key_hash: key, + object: String(params[1]), + tenant_id: String(params[2]), + field: String(params[3]), + scope: String(params[4]), + last_value: Number(params[5]), + }); + return []; + } + if (sql.startsWith('UPDATE') && sql.includes(SEQUENCES_TABLE)) { + const row = rows.get(String(params[1])); + if (row) row.last_value = Number(params[0]); + return []; + } + if (sql.startsWith('DELETE') && sql.includes(SEQUENCES_TABLE)) { + rows.delete(String(params[0])); + return []; + } + if (sql.startsWith('UPDATE') || sql.startsWith('DELETE')) return []; // the stamp + // The org-scoped row, addressed by the key the caller actually asked with. + if (sql.includes('"key_hash" = ?')) { + const row = rows.get(String(params[0])); + return row ? [{ last_value: row.last_value }] : []; + } + // The `__global__` rows for one object/field — one per scope. + if (sql.includes('tenant_id')) { + return [...rows.values()].filter((r) => r.tenant_id === GLOBAL_TENANT); + } + return []; + }; + return { rows, seam: { exec } as never }; + } + + const globalRow = { + key_hash: sequenceKeyHash(OBJECT, GLOBAL_TENANT, FIELD, ''), + object: OBJECT, + tenant_id: GLOBAL_TENANT, + field: FIELD, + scope: '', + last_value: 38, + }; + + it('[first boot] creates the organization row at the merged mark, then retires __global__', async () => { + const { rows, seam } = keyedSequences([globalRow]); + const result = await backfillSeedTenancy(seam); + + expect(result.status).toBe('applied'); + // One row, under the key the DRIVER will compute — not under any key. + expect([...rows.keys()]).toEqual([sequenceKeyHash(OBJECT, ORG, FIELD, '')]); + expect([...rows.values()][0]!.last_value).toBe(38); + }); + + it('[the CI regression] an existing organization row is RAISED, never duplicated', async () => { + // The shape both integration fixtures were really in, spelled with the key + // the platform actually stores. A repair that probes with one key and writes + // under another leaves two rows here, which is what CI caught. + const { rows, seam } = keyedSequences([ + globalRow, + { + key_hash: sequenceKeyHash(OBJECT, ORG, FIELD, ''), + object: OBJECT, + tenant_id: ORG, + field: FIELD, + scope: '', + last_value: 1, + }, + ]); + const result = await backfillSeedTenancy(seam); + + expect(result.status).toBe('applied'); + expect(rows.size).toBe(1); + expect([...rows.values()][0]).toMatchObject({ tenant_id: ORG, last_value: 38 }); + }); + + it('[never lowered] an organization row already ahead of __global__ keeps its own mark', async () => { + const { rows, seam } = keyedSequences([ + globalRow, + { + key_hash: sequenceKeyHash(OBJECT, ORG, FIELD, ''), + object: OBJECT, + tenant_id: ORG, + field: FIELD, + scope: '', + last_value: 91, + }, + ]); + await backfillSeedTenancy(seam); + + expect(rows.size).toBe(1); + // The merge rule is the greater of the two COUNTERS, never the data max. + expect([...rows.values()][0]!.last_value).toBe(91); + }); +}); + /** * #9451 — the durable receipt. * diff --git a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts index 194c5eed60..a2cc97cf51 100644 --- a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts @@ -709,7 +709,12 @@ export function buildGlobalCounterDeleteSql(client?: string): string { * * 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. */ -function sequenceKeyHash(object: string, tenantId: string, field: string, scope: string): string { +export function sequenceKeyHash( + object: string, + tenantId: string, + field: string, + scope: string, +): string { return createHash('sha256') .update(`${object}\u001f${tenantId}\u001f${field}\u001f${scope}`) .digest('hex');