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

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

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH

* test(platform-objects,plugin-security): pin the organization-scoped declarations; changeset

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.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".`,