fix(platform-objects,plugin-security,driver-sql): scope sys_user_preference and sys_capability uniqueness per organization (#8323)

.changeset/tenant-scoped-platform-object-uniques.md

@@ -0,0 +1,54 @@
+---
+"@objectstack/platform-objects": patch
+"@objectstack/plugin-security": patch
+"@objectstack/driver-sql": patch
+---
+
+fix(platform-objects,plugin-security,driver-sql): `sys_user_preference` and `sys_capability` uniqueness is per organization (#8323)
+
+Both objects declared their uniqueness as a table-level index with bare
+`unique: true`. At the DECLARED-index level that is the positional spelling of
+`'global'` — the listed columns verbatim — so on a tenant-scoped object it
+materialized an **installation-wide** unique index. (Field-level `unique: true`
+means the opposite, per-organization, and has since #3696; `packages/lint` names
+that divergence "the #4986 trap" and warns on it via
+`unique/unscoped-declared-index`.) Measured on a deployment running
+`OS_TENANCY_POSTURE=isolated`:
+
+- **A user in two organizations could never persist a preference key they had
+ already used in the first one.** `sys_user_preference`'s `(user_id, key)` was
+ installation-wide, so the second organization's write was refused by a row the
+ caller cannot read — and `data-objectstack`'s `userState.save()` swallows the
+ failure by design, so "recent items" and similar preferences silently stopped
+ persisting in a user's second workspace, with no error anywhere.
+- **`sys_capability.name` refusals were an existence oracle across tenants.** An
+ organization could POST a name and read `409` vs `201` to learn whether some
+ other organization — or the platform seed — already held it, while its own
+ `GET` on that name returned zero rows.
+
+Both declarations now say `unique: 'organization'` (ADR-0120 D1), materializing
+`(COALESCE(organization_id,'__global__'), …)`. Platform-seeded rows carry no
+organization and the key part is NULL-safe (ADR-0120 D3), so they stay unique
+among themselves and `bootstrapSystemCapabilities`' upsert-by-name is unaffected.
+Same-organization duplicates are still refused — the constraint is scoped, not
+removed.
+
+The bare `unique: true` spelling itself is **unchanged**; whether it should be
+reinterpreted is #5082 (v18), and the publish-time authoring advisory is #8379.
+
+**Migration (`@objectstack/driver-sql`).** Respelling a declared index changes
+its generated name, which on a deployed database read as two unrelated findings:
+the composite missing (`create_index`, safe) and the old global index orphaned
+(`drop_index`, **destructive**). An operator applying only the safe half would
+have kept the global index — i.e. kept the defect — while the plan read as
+applied. The declared-index respelling now routes through the same
+`replace_unique_index` retirement the field-level `unique` migration has used
+since #3728: one finding, categorised `safe`, CREATE before DROP, and the legacy
+index dropped only once the replacement is confirmed present. Any two rows
+colliding on `(organization, …fields)` already collided on `(…fields)`, so the
+replacement can neither fail on existing data nor lose any.
+
+Operators upgrading a deployed database should run `os migrate plan` / `os
+migrate apply` — no `--allow-destructive` is required. Until the retirement is
+applied the old index keeps enforcing, so the constraint is never unenforced at
+any point in the migration.

packages/drivers/driver-sql/src/schema-drift.ts

@@ -1042,6 +1042,20 @@ export interface LegacyUniqueReplacement {
  column: string;
  legacyNames: string[];
  replacement: ExpectedIndex;
+ /**
+ * The EXACT physical key columns the superseded index must have, in key
+ * order — the shape the replacement relaxes away from.
+ *
+ * For a field-level unique this is `[column]`: the pre-#3696 single-column
+ * global index. For a DECLARED index respelled from the global spelling to
+ * `'organization'` (#8323) it is the index's listed columns, which may be
+ * several — `sys_user_preference`'s `(user_id, key)` is the case that put
+ * this here. Matching on the name alone is not enough (an unrelated index
+ * may collide with the generated spelling), and matching on a single leading
+ * column is not enough either: `(user_id, key)` and `(user_id, tenant)` share
+ * one, and only one of them is the index being replaced.
+ */
+ legacyColumns: string[];
 }
 
 /**
@@ -1092,6 +1106,7 @@ export function legacyUniqueReplacements(args: {
  const columns = [tenantField, name];
  out.push({
  column: name,
+ legacyColumns: [name],
  legacyNames,
  // NULL-safe organization key part (ADR-0120 D3). Still a pure
  // relaxation to create from under the legacy GLOBAL single-column
@@ -1105,6 +1120,59 @@ export function legacyUniqueReplacements(args: {
  },
  });
  }
+
+ // ── Declared indexes respelled from the global spelling to 'organization' ──
+ //
+ // #8323: the same retirement, one level up. A declared index's bare
+ // `unique: true` is the positional spelling of `'global'` — the listed
+ // columns VERBATIM — so respelling it `'organization'` changes the
+ // materialized shape from `(…listed)` to `(COALESCE(tenant,'__global__'),
+ // …listed)`, and with it the generated NAME. On a deployed database that
+ // reads as two unrelated findings: the composite is missing (create, safe)
+ // and the old global index is an orphan (drop, DESTRUCTIVE, opt-in). An
+ // operator who applies only the safe half keeps the global index — and the
+ // global index is the defect, so the migration would look applied while the
+ // cross-organization refusal it exists to remove is still enforced.
+ //
+ // Routing it through the SAME `replace_unique_index` op the field-level
+ // retirement uses states it as what it is: one pure relaxation, categorised
+ // `safe`, applied CREATE-before-DROP so uniqueness is never unenforced in
+ // between, and dropping the old index only once the replacement is confirmed
+ // present. Any two rows colliding on `(tenant, …listed)` already collided on
+ // `(…listed)`, so the create cannot fail on existing data and no data is lost.
+ for (const idx of Array.isArray(declaredIndexes) ? declaredIndexes : []) {
+ if (idx?.unique !== 'organization') continue;
+ // An EXPLICITLY NAMED index keeps its name across the respelling, so there
+ // is no second name to retire — same name, new definition, which is
+ // `recreate_index`'s job (drop-then-create under one name). Emitting a
+ // replacement here as well would propose dropping the very index the
+ // recreate is rebuilding.
+ if (typeof idx?.name === 'string' && idx.name.trim()) continue;
+ const listed = Array.isArray(idx?.fields)
+ ? idx.fields.filter((f): f is string => typeof f === 'string' && f.length > 0)
+ : [];
+ if (listed.length === 0) continue;
+ // Every listed column must exist physically, or there is no index to match
+ // and nothing the replacement could be created from.
+ if (!listed.every((c) => physicalColumns.has(c))) continue;
+ const replacement = normalizeDeclaredIndex(table, idx, tenantField);
+ if (!replacement) continue;
+ const legacyName = buildIndexName(table, listed, true);
+ // The S6 hand-written composite already lists the tenant column, so
+ // `normalizeDeclaredIndex` prepends nothing and the "legacy" name IS the
+ // current name. Nothing was superseded; the D4 NULL-safe tightening path
+ // owns that transition.
+ if (legacyName === replacement.name) continue;
+ // An index the CURRENT metadata declares is by definition not legacy
+ // (#3955) — the same guard the field-level arm applies.
+ if (declaredNames.has(legacyName)) continue;
+ out.push({
+ column: listed[0],
+ legacyColumns: listed,
+ legacyNames: [legacyName],
+ replacement,
+ });
+ }
  return out;
 }
 
@@ -1197,13 +1265,25 @@ export function diffManagedIndexes(args: {
 
  // ── 1. Legacy platform-wide unique superseded by a tenant composite ──
  for (const l of legacy) {
- // Only a *single-column unique on that very column* is the legacy shape.
- // Matching on the name alone would let an unrelated index that happens to
- // collide with the legacy spelling be dropped.
+ // Only a *plain unique on exactly those columns, in key order* is the
+ // legacy shape. Matching on the name alone would let an unrelated index
+ // that happens to collide with the legacy spelling be dropped.
+ //
+ // `legacyColumns` is `[column]` for the field-level retirement and the
+ // declared index's listed columns for the #8323 respelling — the same
+ // question either way, asked once. The plainness guards matter for the
+ // multi-column arm: an index carrying an expression key part, a NULL-safe
+ // organization part or a WHERE predicate is NOT the verbatim global shape
+ // being relaxed, whatever its column identities read as.
  const present = l.legacyNames.filter((n) => {
  const p = byName.get(n);
  if (!p || p.primary || isRuntimeManagedIndex(p, runtimeCreated, tenantField)) return false;
- return p.unique && p.columns.length === 1 && p.columns[0] === l.column;
+ if (!p.unique || p.partial === true) return false;
+ if ((p.expressions?.length ?? 0) > 0 || (p.nullSafeColumns?.length ?? 0) > 0) return false;
+ return (
+ p.columns.length === l.legacyColumns.length &&
+ p.columns.every((c, i) => c === l.legacyColumns[i])
+ );
  });
  if (present.length === 0) continue;
  for (const n of present) explained.add(n);
@@ -1213,7 +1293,7 @@ export function diffManagedIndexes(args: {
  table,
  column: l.column,
  expected: indexSignature(l.replacement.columns, true, l.replacement.nullSafeColumns),
- actual: indexSignature([l.column], true),
+ actual: indexSignature(l.legacyColumns, true),
  severity: 'warning',
  category: 'safe',
  op: {
@@ -1226,7 +1306,7 @@ export function diffManagedIndexes(args: {
  ...(l.replacement.nullSafeColumns ? { nullSafeColumns: l.replacement.nullSafeColumns } : {}),
  },
  message:
- `${table}.${l.column}: a legacy platform-wide UNIQUE index (${present.join(', ')}) still enforces ` +
+ `${table}.${l.legacyColumns.join('+')}: a legacy platform-wide UNIQUE index (${present.join(', ')}) still enforces ` +
  `uniqueness across ALL tenants, but metadata scopes it per '${l.replacement.columns[0]}' — a second ` +
  `tenant reusing the value is rejected on insert (#3696). Replacing it with ${indexSignature(l.replacement.columns, true, l.replacement.nullSafeColumns)} ` +
  `is a pure relaxation: run "os migrate apply".`,