|
| 1 | +--- |
| 2 | +"@objectstack/platform-objects": patch |
| 3 | +--- |
| 4 | + |
| 5 | +fix(platform-objects): `sys_setting`'s declared unique index becomes per-organization (#8555) |
| 6 | + |
| 7 | +`sys_setting` declared its row identity as a table-level index with bare |
| 8 | +`unique: true`. At the DECLARED-index level that is the positional spelling of |
| 9 | +`'global'` — the listed columns verbatim — so `(namespace, key, scope, user_id)` |
| 10 | +materialized as an **installation-wide** unique index on a tenant-scoped object. |
| 11 | +(Field-level `unique: true` means the opposite, per-organization, and has since |
| 12 | +#3696; `packages/lint` names that divergence "the #4986 trap".) This is the sixth |
| 13 | +instance of the class ruled on 2026-08-13, after #8461, #8556 and #8554's five. |
| 14 | + |
| 15 | +| object | package | was | now | |
| 16 | +|---|---|---|---| |
| 17 | +| `sys_setting` | `platform-objects` | `[namespace, key, scope, user_id]` global | same columns, per organization | |
| 18 | + |
| 19 | +## Why per-organization, when this object had an argument for staying global |
| 20 | + |
| 21 | +`sys_setting` carries a `scope` column, so the card that filed this asked a real |
| 22 | +question first: if `scope` itself encoded tenancy, the installation-wide key was |
| 23 | +correct and the fix was to spell `'global'` explicitly. It does not. `scope` is |
| 24 | +the cascade LAYER — `global | tenant | user`, a priority ladder walked |
| 25 | +env > global > tenant > user > default — and the organization is carried by |
| 26 | +`organization_id` and nothing else. `SettingsService.loadRows` says so outright |
| 27 | +("per-tenant isolation for `tenant`-scope rows is still enforced by the engine"), |
| 28 | +`upsertRow` bypasses the tenant audit only for `scope='global'` rows "because |
| 29 | +global rows are platform-wide", and the `lifecycle` manifest depends on the |
| 30 | +per-organization reading: `retention_overrides` is `scope: 'tenant'` precisely so |
| 31 | +that "regulated tenants set years; dev sets days ... one deployment can carry |
| 32 | +both" (ADR-0057 §3.2). |
| 33 | + |
| 34 | +The `scope='global'` layer is **not** lost by scoping the index. The organization |
| 35 | +key part is NULL-safe (`COALESCE(organization_id, '__global__')`, ADR-0120 D3), |
| 36 | +and platform rows carry no organization — so they share one bucket and stay |
| 37 | +unique among themselves, which is exactly the installation-wide platform default |
| 38 | +the resolver reads at rung 2. |
| 39 | + |
| 40 | +## Measured live on a real engine before the fix |
| 41 | + |
| 42 | +Two organizations, the same `(namespace, key)`, `OS_TENANCY_POSTURE=isolated`, |
| 43 | +driving the real shipped declaration: |
| 44 | + |
| 45 | +``` |
| 46 | +scope='user' org_jia POST (mail, smtp_host, user, usr_1) → 201 |
| 47 | + org_yi POST the SAME → 409 UNIQUE_VIOLATION |
| 48 | + org_yi POST an unused key → 201 ← the control |
| 49 | + org_yi GET the colliding key → total 0 |
| 50 | +scope='tenant' org_jia 201 / org_yi the SAME → 201 |
| 51 | +scope='global' platform 201 / platform the SAME → 201 |
| 52 | +``` |
| 53 | + |
| 54 | +The 409 is the class defect: a per-value refusal on a row the caller cannot read |
| 55 | +is a cross-tenant existence oracle, and two organizations could not hold |
| 56 | +independent per-user settings for the same key. |
| 57 | + |
| 58 | +⚠️ **The two 201s are a second, independent defect that this release does NOT |
| 59 | +fix.** `user_id` is NULL on every `tenant` and `global` row, and SQL UNIQUE is |
| 60 | +NULL-distinct, so the declared row identity is unenforced on those limbs — even |
| 61 | +within one organization, two rows for the same `(namespace, key, scope)` are |
| 62 | +accepted. The organization key part is NULL-safe; the author-declared `user_id` |
| 63 | +column is not. Closing that needs a contract decision about null-safety on |
| 64 | +author-declared columns plus a duplicate pre-flight for databases that have |
| 65 | +already accumulated duplicates, so it is filed as #8629 rather than smuggled in |
| 66 | +here. It is pinned as a live fact in the driver suite so this change cannot be |
| 67 | +read as having fixed it. |
| 68 | + |
| 69 | +## ⚠️ Operators: a migration is REQUIRED, and deploying this release is not it |
| 70 | + |
| 71 | +Respelling a declared index changes its generated **name**. On an existing |
| 72 | +database `initObjects` is additive: it creates the new per-organization composite |
| 73 | +at boot and **never drops the old global index**, which goes on enforcing. Until |
| 74 | +the retirement is applied, a deployed installation that has taken this release |
| 75 | +still refuses a second organization's per-user setting — that is asserted as a |
| 76 | +test, not assumed. |
| 77 | + |
| 78 | +``` |
| 79 | +os migrate plan # one `replace_unique_index` on sys_setting, categorised safe |
| 80 | +os migrate apply # no --allow-destructive needed |
| 81 | +``` |
| 82 | + |
| 83 | +It plans as **one pure relaxation**, not as two findings. That matters: if it |
| 84 | +read as "composite missing" (safe) plus "old global index orphaned" |
| 85 | +(destructive, opt-in), an operator applying only the safe half would keep the |
| 86 | +global index — keep the defect — while the plan read as applied. The `#8461` |
| 87 | +`replace_unique_index` arm covers it unchanged (no driver change in this |
| 88 | +release), applies CREATE-before-DROP so uniqueness is never unenforced in |
| 89 | +between, drops the legacy index only once the replacement is confirmed present, |
| 90 | +and converges to no drift. |
| 91 | + |
| 92 | +Two notes worth an operator's attention: |
| 93 | + |
| 94 | +- The replacement index name, |
| 95 | + `uniq_sys_setting_organization_id_namespace_key_scope_user_id`, is exactly 60 |
| 96 | + characters — the limit — so it is emitted untruncated rather than |
| 97 | + hash-suffixed. |
| 98 | +- Because the replacement does **not** tighten the `user_id` column, the |
| 99 | + migration still applies cleanly to a database that already carries duplicate |
| 100 | + `scope='tenant'` rows (which the old index permitted). Row counts are |
| 101 | + preserved; nothing is deduplicated. |
| 102 | + |
| 103 | +## Not breaking |
| 104 | + |
| 105 | +A relaxation admits key pairs that were previously refused and refuses nothing |
| 106 | +that previously succeeded, so no caller that worked before fails now. Every write |
| 107 | +to `sys_setting` goes through `SettingsService.set()`, whose upsert keys on |
| 108 | +`(namespace, key, scope, user_id)` under the engine's tenant scoping — the shape |
| 109 | +this index now matches. |
0 commit comments