Uh oh!
There was an error while loading. Please reload this page.
fix(metadata-protocol): enforce sys_setting's declared row identity on the tenant and global layers (#8629) - #8719
Conversation
…n the tenant and global layers (#8629) `sys-setting.object.ts` declares `{ fields: ['namespace','key','scope','user_id'], unique: 'organization' }` as the object's row identity, but `user_id` is NULL on every row that is not `scope='user'` and SQL UNIQUE is NULL-distinct — so the constraint was void on exactly the two limbs carrying organization-level and platform-level configuration. Route 1 per the maintainer's 2026-08-14 ruling: a runtime NULL-safe UNIQUE index issued at `kernel:ready`, folding both nullable key parts (`COALESCE(organization_id, '__global__')`, ADR-0120 D3, unchanged from what the driver emits; `COALESCE(user_id, '')`, the `ensureOverlayIndex` spelling for a non-tenant nullable discriminator) and reusing the DECLARED index name so the additive sync never re-imposes the NULL-distinct form. Duplicate-carrying installations: refuse-to-migrate, never keep-newest. The tightening is proved buildable under a throwaway probe name before the declared name is dropped, so a conflict leaves the previous index in place, deletes nothing, and reports at `error` with the exact query that lists the offending rows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XeQRiAa7vYRVX5Fog7Zby8
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#8629
sys-setting.object.tsdeclares the object's row identity as{ fields: ['namespace', 'key', 'scope', 'user_id'], unique: 'organization' }. It was not one:user_idis NULL on every row that is notscope='user'(SettingsService.setcomputes it asscope === 'user' ? ctx.userId ?? null : null), and SQL UNIQUE treats NULLs as mutually distinct — so the constraint was void on thetenantandgloballimbs, exactly the two carrying organization-level and platform-level configuration.Implements the maintainer's ruling of 2026-08-14 (comment
5293093894): Route 1 now — a runtime NULL-safe unique index issued atkernel:ready, the PR #6666 paradigm. Route 2 (declaring NULL-safe uniqueness in the spec vocabulary) is deferred to v18 andpackages/specis untouched. Migration semantics: refuse-to-migrate and list the duplicate rows for the operator, never keep-newest.The premise, re-verified on
origin/mainMeasured before writing anything, with the real
SqlDriverover the real shipped declaration on SQLite. The declared index materializes asand over it: two identical
scope='tenant'rows in ONE organization both land; two identicalscope='global'platform defaults both land; the same rows with a non-NULLuser_idare refused. The premise holds, and the control identifies the mechanism as the NULL rather than thescopevalue.What lands
packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts— the third instance of this package's runtime-index paradigm, deliberately borrowingoverlay-index.tsandview-definition-active-index.tsrather than inventing a variant:COALESCE(organization_id, '__global__')(ADR-0120 D3's tenant form, byte-identical to what the driver already emits) andCOALESCE(user_id, '')(theensureOverlayIndexspelling for a non-tenant nullable discriminator). Neither sentinel is invented here; storage is untouched, only the index folds.syncDeclaredIndexes— which skips by name — never re-imposes the NULL-distinct form. Measured: a later boot's additive sync leaves the tightened definition byte for byte, anddetectManagedDrift()returns[]both immediately after the migration and after that later boot (an index carrying a non-tenant expression key part is notisSyncReproducibleIndex, soisRuntimeManagedIndexclaims it — the same protectionidx_sys_metadata_overlay_activehas).idx_sys_setting_identity_probe), rather than the declared name plus a_probesuffix: the declared name is already on the 60-characterINDEX_NAME_MAXboundary, so the suffixed spelling would be 66 — over PostgreSQL's 63-byte truncation and MySQL's 64-character error.errorwith the exact query that lists the offending rows plus a pointer toos migrate plan. Asserted as behaviour: on a duplicate-carrying database both rows survive with their ids, the previous index DDL is unchanged and still enforcing, and the migration converges only once an operator removes a row themselves.sys_settingis registered by the optionalservice-settings, so a kernel can legitimately reachkernel:readywithout it. Absence is a silent no-op, not a degradation line — one unactionableerrorper boot is how the actionable ones stop being read.resolveIndexExec's body moved intopartial-index-probe.tsasresolveIndexExecForTable(engine, table)—sys_settingwas about to be its third hard-coded copy, and a resolver that asks about the wrong table is the buggetDriverForObjectexists to prevent.view-definition-active-index.tskeeps its exported name and delegates.Armed from the same seam and behind the same
environmentId === undefinedgate as its sibling, in its owntry/catchso one migration that could not be armed does not skip the other.The pin file
sql-driver-sys-setting-organization-unique.test.tssection 4 was written to go RED when this card lands. It is flipped, not relaxed: the BEFORE cases still measure the hole on both spellings of the declaration (the #8555 respelling genuinely does not fix it, and the suite must keep saying so), and new AFTER cases apply the migration's DDL through the driver's own raw seam and pin that the same-organization tenant duplicate and the second platform default both flip from 201 to 409, that the key stays per-organization (anti-vacuity), that the user layer is unchanged, that the index survives a later boot's sync, that the reconciler reports no drift, and that a duplicate-carrying database refuses the build without losing a row. The DDL literal is hand-copied becausedriver-sqlmust not depend onmetadata-protocol— the same one-directional guard the declaration mirror in that file already uses, and it is stated in the file.Verification
Run at
9643bbc, the head of this branch.pnpm --filter @objectstack/metadata-protocol test— 90 files, 1346 tests, all passing (29 new).pnpm --filter @objectstack/driver-sql test— 96 files, 1643 passing / 54 skipped (the flipped pin file: 31 passing).pnpm --filter @objectstack/driver-sql typecheck— clean. (metadata-protocoldeclares notypecheckscript;pnpm --filter @objectstack/metadata-protocol buildemits its.d.tscleanly.)user_idfrom the sentinel map and the suite must go RED. It does — 10 of 29 cases fail, including the real-SQLite duplicate-refusal cases and the whole conflict-path group, so the assertions are not vacuous. Restored from the commit and proved byte-identical withgit hash-object(7b941be8…).check:cross-package-test-inputs(+ theci.ymlscript form),check:durability-log-level,check:test-source-alias,check:type-source-resolution,check:query-options-erasure,check:type-check-coverage,check:type-check-debt(afterturbo run buildover the full package closure — 70/70 successful),check:nul-bytes, and the changeset family the union added on top of the dispatch list:check:changeset-gate-self-tests,check:objectui-changeset,check:empty-changeset,check:adr-0087-registration,check-changeset-no-major.mjs.Scope
The
os migrate planpre-flight lives inpackages/cli, another lane. It is not touched: the operator-facing duplicate list is delivered inside the boot's conflict report, exactly as both precedent migrations deliver theirs, so no cross-lane edit was needed.sql-driver.tsandsql-driver-upsert-conflict-target-dialects.test.tsare untouched (#8622 owns those).packages/specis untouched (route 2 is v18).Generated by Claude Code