Uh oh!
There was an error while loading. Please reload this page.
fix(platform-objects): scope sys_setting's declared unique index per organization (#8555) - #8631
Merged
Merged
Conversation
…organization (#8555) A DECLARED index's bare `unique: true` is the positional spelling of 'global' (the listed columns verbatim), so (namespace, key, scope, user_id) materialized as an installation-wide unique index on a tenant-scoped object. The card left the direction open: if `scope` itself encoded tenancy, the right end state was an explicit 'global'. Reading SettingsService settles it — `scope` is the cascade LAYER (global | tenant | user, ranked by scopeRank) and the organization is carried by organization_id alone. So this is the sixth instance of the #8323 class and respells to 'organization'. Measured live before the fix, real driver, OS_TENANCY_POSTURE=isolated: scope='user' org_jia 201 / org_yi SAME 409 UNIQUE_VIOLATION / org_yi unused 201 / org_yi's own GET 0 rows scope='tenant' org_jia 201 / org_yi SAME 201 scope='global' platform 201 / platform SAME 201 The 409 is the cross-tenant existence oracle. The two 201s are a SECOND defect this change does not fix — user_id is NULL on tenant/global rows and SQL UNIQUE is NULL-distinct, so the declared row identity is void there. Pinned as a live fact in the driver suite and filed separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH
…#8555) The second defect the probe surfaced now has a card, so the declaration, the driver suite's section 4 and the changeset name it instead of saying "filed separately". States why it is not a rider: this respelling is a pure relaxation and applies to any database, while #8629 is a tightening that cannot build its index on an installation carrying the duplicates the hole permits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 2 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
August 14, 2026 04:43
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#8555
The fork, and the measurement that decided it
The card deliberately asserted no defect. A DECLARED index's bare
unique: trueis the positional spelling of'global'— the listed columns verbatim — so(namespace, key, scope, user_id)materialized as an installation-wide unique index on a tenant-scoped object (organization_idinjected, notenancyopt-out). Butsys_settingcarries ascopecolumn, and IFscopeencoded tenancy the installation-wide key would be correct and the right end state was an explicit'global'.It does not.
scopeis the cascade LAYER, not the tenant:global | tenant | user— a priority ladder walked env > global > tenant > user > default, ranked byscopeRank.SettingsService.loadRowssays the organization dimension lives elsewhere outright: "per-tenant isolation fortenant-scope rows is still enforced by the engine". The column isorganization_id.upsertRowbypasses the tenant audit only forscope='global'rows, "because global rows are platform-wide" — i.e.tenant/userrows do carry an organization.lifecyclemanifest is built on the per-organization reading:retention_overridesisscope: 'tenant'precisely so "regulated tenants set years; dev sets days ... one deployment can carry both" (ADR-0057 §3.2).So
scope='tenant'means "the organization layer" — one row per organization — and this is the sixth instance of the #8323 class, inheriting the 2026-08-13 ruling. Branch taken: respell to'organization'. Per the triage comment, the card's type should re-grade Task → Bug.The card's "argument for keeping it global" is answered rather than discarded: the
scope='global'LAYER survives because the organization key part is NULL-safe (COALESCE(organization_id, '__global__'), ADR-0120 D3). Platform rows carry no organization, so they share one bucket and stay unique among themselves — the installation-wide platform default the resolver reads at rung 2 is preserved without the whole index being global.The live probe, run first
Real
SqlDriver, the real shipped declaration,OS_TENANCY_POSTURE=isolated:The 409 is the class defect: a per-value refusal on a row the caller cannot read is a cross-tenant existence oracle, and two organizations could not hold independent per-user settings for one key.
tenantlimb; there is none, becauseuser_idis NULL there and SQL UNIQUE is NULL-distinct — so the index enforces nothing on thetenantandgloballimbs, not even against a same-organization duplicate. That is a second, independent defect. This PR does not fix it and does not claim to: it is pinned as a live fact in the driver suite's section 4 and filed as #8629, which is a tightening (it cannot build its index on an installation already carrying the duplicates this hole permits) whereas this PR is a pure relaxation.#8629 is not addressed here.
Migration — the load-bearing half
Respelling changes the index's generated name, and
initObjectsis additive: it creates the new composite at boot and never drops the old one, so a deployed installation that takes this release without running the plan keeps the defect. Asserted, not assumed.Verified against databases seeded with the OLD index and real rows — including deliberately-planted duplicate
tenant-scope rows, which the old index permitted. One pure relaxation (not a safe-half + destructive-orphan pair), applies without--allow-destructive, preserves all 5 rows, converges to zero drift. The replacement nameuniq_sys_setting_organization_id_namespace_key_scope_user_idis exactly 60 chars — the limit — so it is emitted untruncated rather than hash-suffixed, and differs from the legacy name, so thelegacyName === replacement.nameguard does not swallow the retirement.Tests
25 cases driver-side (
sql-driver-sys-setting-organization-unique.test.ts), 7 declaration-side. No length or delta assertions onindexesanywhere — membership and spelled scope only.Ablations, direction predicted before running:
unique: trueunique: trueunique: false(constraint removed)Ablation B is the one worth reading: "409 flips to 201" passes when the constraint is removed entirely, which is why the anti-vacuity arm (a same-organization duplicate must still be refused) is what actually distinguishes scoped from deleted.
Verification
pnpm --filter @objectstack/driver-sql test— 1598 passed / 54 skippedpnpm --filter @objectstack/platform-objects test— 369 passedpnpm --filter @objectstack/service-settings test— 451 passed (the consumer whose semantics decided the fork)typecheckboth packages clean; the new declaration pin typechecked explicitly, sinceplatform-objectsexcludes*.test.tsfrom its tsc programscripts/pm/dispatch-gates.mjsagainst the actual changed paths:check:i18n(green after building the CLI — it had refused as "prerequisite not met", which measures nothing),check:test-source-alias,check:type-source-resolution,check:query-options-erasure,check:type-check-coverage, all five changeset gates,check:nul-bytesScope
sys_settingonly. The card's negative-triage list was not re-derived and not acted on; nothing in the measurement changes its verdicts.sys_notification_subscription/sys_audience_binding_suggestion(#8577) andsys_job(#8578) remain untouched.Generated by Claude Code