From a3b952f2378897a5628f54d947845392fe5d9ed3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 13:56:17 +0000 Subject: [PATCH] fix(metadata-protocol): zero organizations is a third state, not the ambiguous one (#12395) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #8686 split diagnostic guarded on `organizationIds.length !== 1`, folding "no organizations yet" together with "several organizations". They are opposite conditions: with several the owner is underdetermined, but with none there is no second partition, so each object runs exactly one `__global__` counter and the line's claim of two live counters and an active duplicate-minting hazard was false at the one moment a fresh install actually read it. Zero now returns `no-organization-yet` at `info` — reported, not silenced, and named after the 0/1/several line objectql's `resolveSystemWriteOrganization` already draws. A FAILED organization probe keeps the loud path (#9261): unknown is not zero. The repair threshold is unchanged — data still moves on exactly `length === 1`. The affected-object list is now described as a probe-time snapshot: `kernel:ready` can be reached while an over-budget inline seed is still writing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o --- ...fill-zero-organization-is-not-ambiguous.md | 32 ++++ .../migrations/seed-tenancy-backfill.test.ts | 145 ++++++++++++++++++ .../src/migrations/seed-tenancy-backfill.ts | 100 +++++++++++- 3 files changed, 271 insertions(+), 6 deletions(-) create mode 100644 .changeset/backfill-zero-organization-is-not-ambiguous.md diff --git a/.changeset/backfill-zero-organization-is-not-ambiguous.md b/.changeset/backfill-zero-organization-is-not-ambiguous.md new file mode 100644 index 0000000000..fd1634260f --- /dev/null +++ b/.changeset/backfill-zero-organization-is-not-ambiguous.md @@ -0,0 +1,32 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata-protocol): the seed-tenancy backfill stops reporting a duplicate-minting hazard on a zero-organization first boot (#12395) + +The `#8686` split diagnostic guarded on `organizationIds.length !== 1`, which folded +two opposite conditions into one loud warning. With **several** organizations the +owner of an untenanted row is genuinely underdetermined and the warning is right. +With **none** there is no second partition at all: every object runs exactly one +`__global__` counter, so the line's claim that the named objects "run two autonumber +counters and can mint the same `unique` identifier twice" was false precisely when a +fresh install read it. (The `organizationLastValue: 0` it reported alongside is the +split probe's `LEFT JOIN` finding no second row, not a second counter at zero.) + +Zero organizations is now its own state — `no-organization-yet`, named after and +matching the 0 / 1 / several line `objectql`'s `resolveSystemWriteOrganization` +already draws — logged at `info` rather than `warn`. It is not silenced: the split +is still reported, because the observation is real even though the hazard is not. +It self-heals at the first sign-up, when the `sys_organization`-insert handoff runs +the same repair against a settled database. + +Two things this deliberately does not change. An organization probe that **failed** +still takes the loud path and now says so — an unreadable probe returns the same +empty array as a genuine zero, and reading it as "no organizations yet" is the +confusion `objectql` fixed in `#9261`. And the repair threshold is untouched: data +is still modified on exactly `organizationIds.length === 1` and nothing else. + +The affected-object list is also now described as what it is — a snapshot taken when +the probe ran. The probe runs at `kernel:ready`, which a boot can reach while an +over-budget inline seed is still writing in the background, so a first boot can name +fewer objects than the settled database holds. 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 6440c02def..b96fd65d62 100644 --- a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts @@ -808,3 +808,148 @@ describe('#9451 the seed-tenancy repair leaves a durable receipt', () => { expect(resolveSeedTenancyLedger({ getObject: () => ({}), find: async () => [] })).toBeUndefined(); }); }); + +describe('#12395 zero organizations is a third state, not the ambiguous one', () => { + /** + * The contract these cases pin is a DISCRIMINATION, not a wording: + * `organizationCount: 0` must not warn, and `organizationCount: 2` must still + * warn. Both arms are asserted in every case that can carry both, because a + * diagnostic silenced in both directions would pass a one-armed test while + * being strictly worse than the line it replaced. + * + * NOTE ON EXISTING ASSERTIONS: none were changed. The two pins that already + * existed on `skipped-ambiguous-organization` — `null-seam.test.ts` and + * runtime's `seed-tenancy-autonumber-split.integration.test.ts` — both drive + * the TWO-organization arm (`org_a`/`org_b`, `org_second`), which keeps its + * status and its warning here. No test covered the zero arm before this one. + */ + function spy() { + const warn: Array<[string, unknown]> = []; + const info: Array<[string, unknown]> = []; + return { + warn, + info, + logger: { + info: (m: string, p?: unknown) => void info.push([m, p]), + warn: (m: string, p?: unknown) => void warn.push([m, p]), + error: () => {}, + }, + }; + } + + /** A seam holding one `__global__` counter and `orgs` organizations. */ + function seam(orgs: string[], opts: { organizationProbeThrows?: boolean } = {}) { + const sql: string[] = []; + const exec = async (statement: string) => { + sql.push(statement); + if (statement.includes('WHERE 1 = 0')) return []; + if (statement.includes('LEFT JOIN')) { + return [ + { + object: 'crm_case', + field: 'case_number', + global_last_value: 38, + // No organization-scoped row exists: the LEFT JOIN yields NULL here. + organization_last_value: null, + }, + ]; + } + if (statement.includes(ORGANIZATION_TABLE)) { + if (opts.organizationProbeThrows) throw new Error('connection reset by peer'); + return orgs.map((id) => ({ id })); + } + return []; + }; + return { seam: { exec, client: 'better-sqlite3' as const }, sql }; + } + + const HAZARD = 'can mint the same "unique" identifier twice'; + + it('[zero] does NOT warn, and reports its own state instead of the ambiguous one', async () => { + const log = spy(); + const { seam: s } = seam([]); + const result = await backfillSeedTenancy(s, log.logger as any); + + // The discrimination, arm 1. + expect(log.warn).toHaveLength(0); + expect(log.info).toHaveLength(1); + expect(result.status).toBe('no-organization-yet'); + expect(log.info[0][1]).toMatchObject({ organizationCount: 0 }); + + // Still VISIBLE — silenced about harm, not about the observation. + expect(result.splits).toEqual([ + { object: 'crm_case', field: 'case_number', globalLastValue: 38, organizationLastValue: 0 }, + ]); + }); + + it('[several] still warns, and still names the hazard', async () => { + const log = spy(); + const { seam: s } = seam(['org_a', 'org_b']); + const result = await backfillSeedTenancy(s, log.logger as any); + + // The discrimination, arm 2 — unchanged from before this card. + expect(log.warn).toHaveLength(1); + expect(log.info).toHaveLength(0); + expect(result.status).toBe('skipped-ambiguous-organization'); + expect(log.warn[0][1]).toMatchObject({ organizationCount: 2 }); + expect(log.warn[0][0]).toContain(HAZARD); + }); + + it('[the claim moved with the state] only the ambiguous arm asserts the minting hazard', async () => { + // The card's actual complaint: the line claimed two live counters and an + // active duplicate-minting risk at a moment when exactly one counter + // existed. Asserting BOTH arms is what keeps this from going green on a + // rewrite that simply deletes the sentence everywhere. + const zero = spy(); + await backfillSeedTenancy(seam([]).seam, zero.logger as any); + const several = spy(); + await backfillSeedTenancy(seam(['org_a', 'org_b']).seam, several.logger as any); + + expect(zero.info[0][0]).not.toContain(HAZARD); + expect(several.warn[0][0]).toContain(HAZARD); + // And the benign line says why it is benign, in the counter's own terms. + expect(zero.info[0][0]).toContain('exactly ONE counter'); + }); + + it('[no writes] the zero state touches no data — the repair threshold is unchanged', async () => { + // Clause-② evidence in executable form: the set of inputs on which this + // migration MODIFIES data is exactly what it was — `length === 1` — so the + // zero arm must still issue reads only. + const { seam: s, sql } = seam([]); + const result = await backfillSeedTenancy(s, spy().logger as any); + + expect(result.objectsStamped).toBe(0); + const writes = sql.filter((q) => /^\s*(UPDATE|DELETE|INSERT)/i.test(q)); + expect(writes).toEqual([]); + }); + + it('[#9261] an organization probe that FAILED is not read as "no organizations yet"', async () => { + // The probe returns the same empty array for "none" and for "could not + // ask". Folding the second into the benign path would convert an outage + // into a reassuring info line — the confusion objectql already fixed in + // `resolveSystemWriteOrganization`. Unknown is not zero. + const log = spy(); + const { seam: s } = seam([], { organizationProbeThrows: true }); + const result = await backfillSeedTenancy(s, log.logger as any); + + expect(result.status).toBe('skipped-ambiguous-organization'); + expect(log.info).toHaveLength(0); + expect(log.warn).toHaveLength(1); + expect(log.warn[0][1]).toMatchObject({ organizationProbeError: 'connection reset by peer' }); + expect(log.warn[0][0]).toContain('probe FAILED'); + }); + + it('[snapshot] every list-bearing branch says the list is a probe-time snapshot', async () => { + // Problem 2. The affected list is read at `kernel:ready`, which a boot can + // reach while an over-budget inline seed is still writing — measured at 3 + // objects named where the settled database held 9. Stated rather than + // reordered away; see SNAPSHOT_CAVEAT's comment for why boot does not wait. + const zero = spy(); + await backfillSeedTenancy(seam([]).seam, zero.logger as any); + const several = spy(); + await backfillSeedTenancy(seam(['org_a', 'org_b']).seam, several.logger as any); + + expect(zero.info[0][0]).toContain('snapshot taken when the probe ran'); + expect(several.warn[0][0]).toContain('snapshot taken when the probe ran'); + }); +}); diff --git a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts index a2cc97cf51..f0b4e3ae3f 100644 --- a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts @@ -193,7 +193,20 @@ export type SeedTenancyBackfillStatus = | 'no-split' /** A split exists but the install is multi-tenant — ruled: skip, loudly. */ | 'skipped-multi-tenant' - /** A split exists but the organization count is not exactly 1. */ + /** + * A split exists and the install holds NO organization yet (#12395). + * + * Benign, and deliberately NOT folded into `skipped-ambiguous-organization`: + * with zero organizations there is no second partition, so each object runs + * exactly one counter and nothing can be minted twice. The state self-heals at + * the first sign-up through the `sys_organization`-insert handoff. + * + * The same 0 / 1 / several line objectql already draws in + * `resolveSystemWriteOrganization`, whose `no-organization-yet` decision this + * is named after — "⛔ Refusing here would refuse first boot itself." + */ + | 'no-organization-yet' + /** A split exists but the install holds SEVERAL organizations — no derivable owner. */ | 'skipped-ambiguous-organization' /** The backfill ran. */ | 'applied'; @@ -573,6 +586,27 @@ export function buildSplitProbeSql(client?: string): string { */ const PLATFORM_NAMESPACE = /^(sys_|cloud_|ai_)/; +/** + * What the affected-object list is, and is not (#12395). + * + * It is read from `_objectstack_sequences` at the instant this probe runs, and + * this probe runs on `kernel:ready`. A seed that overruns its budget keeps + * writing in the BACKGROUND past that point (`[Seeder] Inline seed exceeded + * ms budget … continuing in background to avoid blocking kernel start`), so a + * first boot can reach here with only part of the seed's counters allocated — + * measured at 194 ms apart, naming 3 objects where the settled database holds 9. + * + * Stated rather than removed by ordering: the boot pass exists for the + * EXISTING-install half, whose rows are already written and need no wait, and + * the fresh-install half is delivered by the `sys_organization`-insert handoff, + * which by construction runs after sign-up. Making boot block on seed settlement + * would delay a repair that has nothing to wait for. + */ +const SNAPSHOT_CAVEAT = + `This list is a snapshot taken when the probe ran, not a census: a boot that reaches ` + + `'kernel:ready' while an over-budget inline seed is still writing in the background names only ` + + `the counters allocated so far, so a later run on the same database may name more.`; + /** The organizations the install has, capped — the single-tenant guard reads this. */ export function buildOrganizationProbeSql(client?: string): string { return `SELECT ${quoteIdent('id', client)} FROM ${quoteIdent(ORGANIZATION_TABLE, client)}`; @@ -1245,21 +1279,69 @@ export async function backfillSeedTenancy( `no derivable answer to which organization owns the untenanted rows. Remedy: decide the owner per ` + `object, then UPDATE SET ${ORGANIZATION_FIELD} = '' WHERE ${ORGANIZATION_FIELD} ` + `IS NULL, and merge that object's '${GLOBAL_TENANT}' row in ${SEQUENCES_TABLE} into the ` + - `organization-scoped row at the greater last_value.`, + `organization-scoped row at the greater last_value. ` + + SNAPSHOT_CAVEAT, { splits, posture: resolveTenancyPosture() }, ); return { status: 'skipped-multi-tenant', splits, collisions: [], objectsStamped: 0 }; } - // 4. Exactly one organization, or there is nothing derivable to adopt. + // 4. How many organizations does the install hold? Three answers, not two: + // none yet (benign, 4a), exactly one (derivable — the repair runs), or + // several (ambiguous, 4b). + // + // A probe that THREW is tracked separately and must never reach 4a. It + // yields the same empty array as a genuine zero, and reading a failure as + // "no organizations yet" is a known way to turn an outage into a benign- + // looking log line — objectql fixed that exact confusion in + // `resolveSystemWriteOrganization`'s probe (#9261). Unknown is not zero. let organizationIds: string[] = []; + let organizationProbeError = ''; try { organizationIds = (await selectRows(exec, buildOrganizationProbeSql(client))) .map((r) => (r.id == null ? '' : String(r.id))) .filter((id) => id.length > 0); - } catch { + } catch (e) { + organizationProbeError = (e as Error).message || 'unknown error'; organizationIds = []; } + // 4a. NO organization yet — benign, and NOT the ambiguous case (#12395). + // + // `!== 1` used to fold this together with "several organizations", and the + // two are opposite conditions. With several, the owner is genuinely + // underdetermined and an operator has to choose. With NONE, there is no + // second partition to be split ACROSS: every counter is the one + // `__global__` row, so "two autonumber counters" and "can mint the same + // identifier twice" — what the loud branch below says — are both false + // here, at a moment when they read as an active data-integrity emergency. + // (The `organizationLastValue: 0` this state reports is `buildSplitProbeSql`'s + // LEFT JOIN finding no second row, not a second counter sitting at zero.) + // + // Nor is it a state anyone can act on: seeds load inline during `start()`, + // while the first organization is created by plugin-auth's + // `ensureDefaultOrganization` behind an admin permission-set grant, so it + // cannot exist until a sign-up POST reaches a running server. The repair is + // already scheduled for that exact moment by the `sys_organization`-insert + // handoff in runtime's app-plugin. + // + // `info`, not silence. The split is real even though the hazard is not, and + // a diagnostic silenced in BOTH directions would be worse than the one it + // replaces — this still says what was seen, it just stops claiming harm. + if (organizationIds.length === 0 && organizationProbeError === '') { + logger?.info?.( + `[metadata-protocol] seed/API tenancy split detected on an install with no organization yet — ` + + `nothing to adopt, and nothing at risk (#8686). Affected: ${affected}. ` + + `${ORGANIZATION_TABLE} is empty, so each of these objects runs exactly ONE counter (its ` + + `'${GLOBAL_TENANT}' row) and no "unique" identifier can be minted twice while there is only ` + + `one partition. No operator action: this self-heals at the first sign-up, when the ` + + `${ORGANIZATION_TABLE}-insert handoff runs this same repair against a settled database. ` + + SNAPSHOT_CAVEAT, + { splits, organizationCount: 0 }, + ); + return { status: 'no-organization-yet', splits, collisions: [], objectsStamped: 0 }; + } + + // 4b. SEVERAL organizations — the genuinely ambiguous case, still loud. if (organizationIds.length !== 1) { logger?.warn?.( `[metadata-protocol] seed/API tenancy split detected but the target organization is not ` + @@ -1268,8 +1350,14 @@ export async function backfillSeedTenancy( `${ORGANIZATION_TABLE} (exactly 1 is required to adopt one without guessing). Until this is ` + `resolved these objects run two autonumber counters and can mint the same "unique" identifier ` + `twice. Remedy: as above — stamp the untenanted rows with the owning organization and merge the ` + - `'${GLOBAL_TENANT}' counter row into the organization-scoped one.`, - { splits, organizationCount: organizationIds.length }, + `'${GLOBAL_TENANT}' counter row into the organization-scoped one. ` + + (organizationProbeError === '' + ? '' + : `NOTE: the ${ORGANIZATION_TABLE} probe FAILED (${organizationProbeError}), so the count ` + + `above is "unknown", not a measured zero — an unreadable probe is reported here rather ` + + `than through the benign no-organization-yet path (#9261). `) + + SNAPSHOT_CAVEAT, + { splits, organizationCount: organizationIds.length, organizationProbeError }, ); return { status: 'skipped-ambiguous-organization', splits, collisions: [], objectsStamped: 0 }; }