Repository navigation
sys_setting's declared row identity is unenforced on every tenant and global row — user_id is NULL there and SQL UNIQUE is NULL-distinct #8629
Description
Activity
- addedbugSomething isn't workingSomething isn't working
on Aug 14, 2026 Triage: routed
needs-user-decision+domain:metadata(declaration lives inpackages/platform-objects, route-1 fix would land inmetadata-protocol), type Bug — declared row identity ≠ enforced on thetenant/globallimbs.Premise check on
origin/main(2026-08-14):sys-setting.object.ts:191currently declares{ fields: ['namespace','key','scope','user_id'], unique: true }— the card quotes the'organization'spelling, which is on the in-flight #8555 branch, not yet onmain. The NULL-distinctuser_idhole is spelling-independent (the card's own control run shows the mechanism is the NULL, not the scope value), so the premise holds either way. The pinned section-4 assertions likewise live on the #8555 branch. Serial: implementation cannot start before #8555 lands (same declaration line, same index name).Why the human floor, not the #8468 inheritance: the settled installation-wide-unique family covers the relaxation shape (bare
unique: true→ per-organization). This card is the opposite move — a tightening that can fail to build the index on duplicate-carrying installations, i.e. ADR-0120 D4 ceremony plus a stored-data disposition; and route 2 is authorable-surface design (ADR-class). Both are human-floor categories, and the sibling ruling (#6417) itself recorded this half as an independent tradeoff needing its own ruling:「这是独立的取舍,需要单独拍板。」Four-prism block:
- Platform long-term coherence — third instance of the same NULL-distinct class (driver-sql: 单租户栈上 organization_id 恒为 NULL,#3696 的 (tenant, col) 复合 UNIQUE 因 NULL-distinct 而完全不生效 —— 字段级 unique: true 静默零约束 #5030 tenant column → ADR-0120 D3; [metadata-core] sys_view_definition:owner/organization_id 为 NULL 的视图完全不受唯一索引约束(两条同名活跃共享视图可并存) #6417
owner→ raw-SQL runtime patch; nowuser_id). Route 2 (declared NULL-safe column vocabulary) closes the class and lets a declaration state its own row identity; route 1 accretes a third out-of-bandCREATE UNIQUE INDEXand widens the declared≠enforced gap. - Measured business pull — live and measured on a real engine: duplicate
tenant/globalrows make the effective value of a real key (lifecycle.retention_overrides) unspecified, andset()'s upsert updates an arbitrary duplicate. Admin-visible misconfiguration with no diagnosis path; reaches real retention behaviour. - AI-agent error-resistance — a declaration that silently does not mean what it says is the exact trap contract-first exists to prevent; on this axis route 2 (expressible, enforced constraint) beats route 1 (hidden runtime patch). For the data disposition: refuse-and-list beats keep-newest — silently discarding admin-authored configuration is silent tolerance.
- Startup scope discipline — route 1 is small and shipped twice (PR fix(metadata): sys_view_definition active-row index gets a NULL-safe key — same-name active shared views can no longer coexist (#6417) #6666 pattern,
ensureOverlayIndex); route 2 is spec-surface design + ADR (or ADR-0120 amendment) + MySQL functional-key fallback work. Scope discipline says route 1 now, route 2 parked to the ADR-0120/v18 train (natural neighbour: ADR-0120 协议 18 项:D2 conversion(声明索引 unique: true → 'global')+ 裸 true 硬拒 + synonym pin 退役 —— 挂 18 列车,现在勿动工 #5082).
Recommendation: route 1 now —
kernel:readyNULL-safe unique index viaCOALESCE(user_id, ''), reusing the declared index name sosyncDeclaredIndexesnever re-imposes the NULL-distinct form; duplicate disposition = refuse-to-migrate with an operator-facing list in theos migrate planpre-flight (settings are admin-authored; keep-one is a data decision an operator should make, not a default we pick); file the route-2 vocabulary card on the v18/ADR-0120 train so the class closes there.Two questions for the maintainer:
- Route 1 now with route 2 parked to v18 — yes, or go straight to the vocabulary/ADR route?
- Duplicate-row disposition on migration: refuse + operator-facing list (recommended), or deterministic keep-newest?
Not auto-adjudicable (human floor: stored-data migration shape + ADR-class route choice). Prisms 1/3 lean route 2 long-term while prism 4 leans route 1 now — genuinely split on timing, which is itself above the confidence gate.
Generated by Claude Code
- Platform long-term coherence — third instance of the same NULL-distinct class (driver-sql: 单租户栈上 organization_id 恒为 NULL,#3696 的 (tenant, col) 复合 UNIQUE 因 NULL-distinct 而完全不生效 —— 字段级 unique: true 静默零约束 #5030 tenant column → ADR-0120 D3; [metadata-core] sys_view_definition:owner/organization_id 为 NULL 的视图完全不受唯一索引约束(两条同名活跃共享视图可并存) #6417
Maintainer ruling (2026-08-14, verbatim: 「同意」, approving item 3 of the six-item ruling list: 「#8629 走 Route 1+拒绝迁移列清单?」): Route 1 now — runtime NULL-safe unique index issued at
kernel:ready(precedent PR #6666); Route 2 (declaring NULL-safe uniqueness in the spec vocabulary) is deferred to v18 as the ADR-class long-term form. Migration semantics: refuse-to-migrate and list the duplicate rows for the operator — never keep-newest; silently discarding admin-authored config rows is a data-loss trade that is not worth saving one manual confirmation. Serial constraint noted at triage stands: implementation is blocked behind #8555's landing.needs-user-decision→pm:queue,target:v17applied; Seat A (#8667).
Generated by Claude Code
Claim: PM loop round 1
Session:session_01XeQRiAa7vYRVX5Fog7Zby8
Branch:claude/issue-8629-sys-setting-null-safe-unique
Worktree:objectstack-issue-8629
Domain:domain:metadata
File surface:packages/metadata-protocol/src/migrations/**+ its wiring inpackages/metadata-protocol/src/index.ts;packages/drivers/driver-sql/src/sql-driver-sys-setting-organization-unique.test.tssection 4 only (stop on breach; explain in the report)
Container & model:M,mode:subagent,model: opus
Serial constraints cleared: #8555 has landed —6cb81c7onorigin/main,unique: 'organization'verified atsys-setting.object.ts:235, and the pin file exists at the declared path. The triage serial («implementation cannot start before #8555 lands») is discharged. Of the 6 open PRs in this repo, none touchesmetadata-protocol/src/migrationsor the sys-setting pin file.⚠️ driver-sqlis shared with #8622, which this seat is deliberately holding to the next batch for that reason — do not extend intosql-driver.tsor the upsert dialect tests.Dispatching under the maintainer's direct-dispatch instruction to this PM session (2026-08-14, verbatim, untranslated): 「metadata + engine-core + drivers 优先处理bug」. Lane seat #6367 is vacant — Seat A de-registered from all eight lane seats at 15:04Z per 「你只需要处理在手的几个任务,不需要再接管车道。」, so this card is unowned rather than contested.
Implementing the maintainer's 2026-08-14 ruling recorded in
5293093894: Route 1 (runtime NULL-safe unique index atkernel:ready, precedent PR #6666), Route 2 deferred to v18; migration semantics refuse-to-migrate and list the duplicate rows for the operator, never keep-newest.⚠️ Declared cross-lane risk, carried into the dispatch rather than discovered mid-run: the duplicate pre-flight belongs toos migrate plan, which lives inpackages/cli(domain:cli). The dev is instructed to land the index inmetadata-protocoland to stop and report if the operator-facing list forces an edit insidepackages/cli— this seat will route that half rather than have it patched across a lane boundary.
Generated by Claude Code
{ "issue": 8629, "status": "done", "branch": "claude/issue-8629-sys-setting-null-safe-unique", "pr": "https://github.com/objectstack-ai/objectstack/pull/8719", "premise_still_valid": true, "summary": "Route 1 as ruled: a runtime NULL-safe UNIQUE index for sys_setting, issued at kernel:ready from packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts, folding COALESCE(organization_id,'__global__') (ADR-0120 D3, unchanged from what the driver emits) and COALESCE(user_id,'') (the ensureOverlayIndex spelling for a non-tenant nullable discriminator), reusing the DECLARED index name so syncDeclaredIndexes never re-imposes the NULL-distinct form. Duplicate disposition is refuse-to-migrate: the existing probe-first order proves the form buildable under a throwaway name before the declared name is dropped, so a conflict keeps the previous index, deletes nothing, and reports at error with the exact row-listing query — asserted as behaviour, not comment. packages/spec untouched (route 2 is v18); packages/cli untouched (see open_questions). All three PM mechanism assumptions re-verified true on origin/main, plus four design facts measured rather than assumed (see tests).", "tests": "All at HEAD 9643bbc (git rev-parse --short HEAD, taken at the verification run; no commit after it). `pnpm --filter @objectstack/metadata-protocol test` -> 'Test Files 90 passed (90) / Tests 1346 passed (1346)' (29 new in sys-setting-identity-index.test.ts). `pnpm --filter @objectstack/driver-sql test` -> 'Test Files 96 passed | 4 skipped (100) / Tests 1643 passed | 54 skipped (1697)'; the flipped pin file alone: 31 passed. `pnpm --filter @objectstack/driver-sql typecheck` -> Done (metadata-protocol declares no typecheck script; its tsup build emits .d.ts cleanly). Dependency closures built first (`pnpm --filter '<pkg>^...' build`). REVERSE VERIFICATION, direction predicted before running (RED): removing user_id from SYS_SETTING_NULL_SENTINELS fails 10 of 29 cases — the real-SQLite duplicate-refusal cases AND the whole conflict-path group, so nothing is vacuous; restored with `git checkout <branch> -- <path>` from the commit and proved byte-identical via git hash-object 7b941be8c51feb00a01810c95e391f6933099577. MEASURED, not assumed: (1) the driver's real DDL for the shipped declaration, used verbatim as the test fixture; (2) detectManagedDrift() == [] after the migration and again after a later boot's additive sync (isRuntimeManagedIndex claims it — a second, non-tenant COALESCE key part makes it non-sync-reproducible), so the reconciler never proposes rebuilding it away; (3) a second initObjects leaves the tightened DDL byte for byte; (4) the conflict path on a duplicate-carrying database throws SQLITE_CONSTRAINT_UNIQUE with the previous index intact and both rows alive. GATES (union re-derived from the actual diff with scripts/pm/dispatch-gates.mjs, all green): check:cross-package-test-inputs + node scripts/check-cross-package-test-inputs.mjs, check:durability-log-level, check:test-source-alias, check:type-source-resolution, check:query-options-erasure, check:type-check-coverage, check:type-check-debt (after `turbo run build --filter=./packages/* --filter=./packages/*/*` — 70/70 successful; re-measure OK, 33 ledger entries, none above ceiling), check:nul-bytes, plus five the dispatch list did not name and the re-derivation did: check:changeset-gate-self-tests, check:objectui-changeset, check:empty-changeset, check:adr-0087-registration, node scripts/check-changeset-no-major.mjs. Self-scanned the diff for raw control bytes beyond the gate (grep -naP): none.", "open_questions": [ { "question": "The cross-lane half did NOT force a packages/cli edit — but it is worth a decision whether the `os migrate plan` pre-flight should also learn this index. The operator-facing duplicate list is delivered inside the boot's conflict report (the exact GROUP BY query, plus a pointer to `os migrate plan`), which is what both precedent migrations do. Because the index is runtime-managed, the drift differ deliberately skips it, so `os migrate plan` will NOT list these duplicates on its own — an operator who reaches for the CLI first, rather than reading the boot log, gets nothing.", "options": [ "A — leave as shipped: the boot's error line carries the query, identical to ensureOverlayIndex and ensureViewDefinitionActiveIndex. No cli edit, no lane crossing, consistent with two precedents.", "B — route a follow-up card to the domain:cli seat to have `os migrate plan` run the three runtime migrations' duplicate probes and report them, closing the class for all three at once." ], "recommendation": "A now, B as a separate card if the maintainer wants the CLI to be a complete answer — the gap is identical for all three runtime migrations, so fixing it here alone would leave the class half-closed and would cross a lane for one instance of it. I did not file B unprompted: it is a capability decision about the CLI's contract, not a defect." } ], "out_of_scope_findings": [] }Notes the PM asked for, outside the JSON:
- PM mechanism assumptions, all three re-verified true.
sys_setting's unique key is installation-wide on a tenant-scoped object — but unlike the #8323 class it has a real argument for staying that way #8555 has landed (6cb81c7is inorigin/main's history; the declaration readsunique: 'organization'); the pin file exists and its section 4 did assert the hole as live (measured red-on-fix by construction — the flipped cases fail against the pre-fix index); the route-1 precedents are where stated. Theoverlay-index.test.ts:26defect shape was read before writing: it is the drop-then-create ordering, already inverted repo-wide bypartial-index-probe.ts's probe-first order ([metadata-protocol] ensureOverlayIndex 先 DROP 后 CREATE:partial 索引建失败时 sys_metadata 会静默地失去覆盖层唯一约束 #6418), which this migration reuses rather than re-deriving — so the "window with no constraint" cannot recur here, and the test asserts the previous index is intact byte for byte after a refusal. - One file outside the named "expected landing", stated rather than buried:
packages/metadata-protocol/src/plugin.ts. It is the only seam that arms akernel:readymigration (the sibling is armed there too), so without it the module would be dead code. Same package, same directory tree, no hard stop crossed. packages/spec,packages/cli,sql-driver.tsandsql-driver-upsert-conflict-target-dialects.test.ts: untouched. Thedriver-sqlsurface is the one pin file only.- Deliberate scope limit inherited from the precedent: the migration is gated on
environmentId === undefined, so per-project (cloud) kernels do not get it — the same gate, for the same stated reason, asensureViewDefinitionActiveIndex. If a cloud project kernel composesservice-settings, the hole stays open there. Worth a ruling if that shape exists; I did not widen the gate unilaterally.
Generated by Claude Code
- PM mechanism assumptions, all three re-verified true.
ACCEPT — PR #8719. Implementation reviewed against GitHub, not against the report.
PM session
session_01XeQRiAa7vYRVX5Fog7Zby8. Landing is not done: gates are still converging, and the flip to ready is gated on their conclusions (below).Scope — checked from
get_files, not from the PR body8 files, every one inside the declared surface, and all three hard stops held:
stop result packages/cli(the cross-lane stop)untouched — the operator-facing duplicate list is delivered inside the boot's conflict report, the same way both precedent migrations deliver theirs, so no lane was crossed sql-driver.ts+sql-driver-upsert-conflict-target-dialects.test.ts(#8622's territory)untouched — the only driver-sqlfile in the diff is the sys-setting pinpackages/spec(Route 2 is v18)untouched One file beyond the surface I named:
packages/metadata-protocol/src/plugin.ts(+29), the seam that arms akernel:readymigration — without it the new module is dead code. In-package, necessary, and declared by the dev rather than discovered by me, which is the behaviour the file-surface clause is for. Accepted.The pin was flipped, not defanged — the check that mattered most
Section 4 is +158/−34, and the deletions are the ones that should go: the "out of scope, filed as #8629, written to go RED when that card lands" framing. What survives and what arrives:
BEFORE: a SAME-organization tenant-scope duplicate is accepted— the hole is still measured, not merely described in a comment.the hole was the NULL, not the layer— the mechanism control is retained. Without it, "tenant rows are unconstrained" could still be misread as something about thescopevalue.- New
AFTERcases pin the 201→409 flip on both the tenant and thegloballayer, that the user layer is unchanged (same user refused, different user allowed — a two-sided control), that the index survives a later boot's additive sync, that the reconciler does not report it as drift, and that on a duplicate-carrying database the tightening is refused with nothing deleted. - Anti-vacuity is explicit:
AFTER the migration (anti-vacuity): the key is still PER-ORGANIZATION. Corroborated by the dev's reverse verification — removinguser_idfrom the sentinel map turns 10 of 29 cases red, including the real-SQLite refusal cases and the whole conflict-path group, so these assertions are not constant-true.
The maintainer's ruling is asserted as behaviour, not prose: refuse-to-migrate with the previous index left in force and both rows alive, never keep-newest.
The sibling-migration refactor is behaviour-preserving
resolveIndexExecmoved intopartial-index-probe.tsasresolveIndexExecForTable(engine, table). I diffed the extracted body against the original: identical four-step ladder (getDriverForObject(table)→engine.driver→getDriver(table)→ iterate thedriversMap), identical per-probeattemptguards, identicalraw-before-executeseam.view-definition-active-index.tspasses its ownVIEW_DEFINITION_TABLE, so that migration's behaviour is unchanged.Gates
Check Changeset,Check PR Size,ADR maintainer approval,Console Pin Freshness,Check Documentation Links,No other open PR may claim the same issueandPart-of PR must not also close its cardare green.Test Core (1-3),Build Core,ESLint,TypeScript Type Check,Temporal Conformance (live PG + MySQL)and the threeDogfood Regression Gateshards are stillin_progress— an honest draft-time reading, not a pass. ⛔ The PR stays draft until those job conclusions are read directly; a local green does not substitute.The docs-drift bot flagged three hand-written pages as advisory. Judged not to need edits: this change adds a runtime index and alters no documented behaviour of the metadata lifecycle, the services checklist or the HTTP protocol. The release-owned
v9.mdxis read-only and was correctly left alone.The open question — settled as shipped, with the class recorded separately
The dev asked whether
os migrate planshould also learn this index, noting the drift differ deliberately skips runtime-managed indexes, so an operator who reaches for the CLI before reading the boot log sees nothing.Option A as shipped, per its recommendation. This is a sequencing judgement, not a product decision: the gap is identical for all three runtime migrations (
ensureOverlayIndex,ensureViewDefinitionActiveIndex, and this one), so closing it here alone would leave the class half-closed and cross a lane for one instance. Recorded as its own card so it is not lost.
Generated by Claude Code
- added 3 commits that reference this issue
on Aug 17, 2026 - added a commit that references this issue
on Oct 9, 2026
Found while implementing #8555 (which scopes this same index per organization). Filed unassigned and deliberately separate: #8555 is a pure relaxation, this one is a tightening, and the two need different migration ceremony. Same split #6417 made against #5839, and for the same stated reason.
The fact
packages/platform-objects/src/system/sys-setting.object.tsdeclares the object's row identity asand the object's own header calls that the row identity.
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 is void on thetenantandgloballimbs, which are exactly the ones carrying organization-level and platform-level configuration.The organization key part is NULL-safe (
COALESCE(organization_id, '__global__'), ADR-0120 D3).user_idis not, and D1's vocabulary scopes the tenant column only —sys_view_definition's declaration says so in its own comment, about its ownownercolumn:Measured, live, on a real engine
Real
SqlDriver, the real shipped declaration,OS_TENANCY_POSTURE=isolated. Identical before and after #8555 — this is not something that card introduced or fixed:The control identifies the mechanism: it is the NULL, not the
scopevalue.Pinned as live assertions in
packages/drivers/driver-sql/src/sql-driver-sys-setting-organization-unique.test.ts, section 4 ("the NULL-distinct user_id hole"), written to go red when this card lands so a fix must come and flip them rather than leaving a stale comment behind.User-reachable consequence
SettingsServiceresolves a layer withrows.find((r) => r.key === key && r.scope === 'tenant')— a positional pick over whatever the table returns. With duplicates permitted, which value an organization gets for a tenant-scoped setting is unspecified, and a write throughset()upserts againstwhere { namespace, key, scope, user_id }, which matches an arbitrary one of them. Two rows can therefore disagree indefinitely with no way for an admin to see why the effective value is not the one they set.lifecycle.retention_overridesis a live tenant-scoped key, so this reaches real retention behaviour.Sibling objects likely share the shape —
sys_setting_auditmirrors the same three-valuescope— but that is unverified and deliberately not asserted here.Why it is not a rider on #8555
#8555 respells
unique: trueto'organization'. That is a relaxation: it admits key pairs previously refused and refuses nothing that previously succeeded, so it cannot fail to apply on any existing database. Verified there against a database deliberately seeded with pre-existing duplicatetenant-scope rows — the replacement index builds and every row survives, precisely because it does not tightenuser_id.Making
user_idNULL-safe is the opposite: it is a tightening that will fail to create the index on any installation that has already accumulated duplicates, which the void constraint has been permitting all along. That needs ADR-0120 D4's conflict-row disposition and a duplicate pre-flight inos migrate plan. Exactly the tradeoff #6417 recorded, verbatim and untranslated:Two routes, both with precedent
metadata-protocol'sensureViewDefinitionActiveIndexissues a raw-SQLCREATE UNIQUE INDEXatkernel:ready, reusing the declared index's name sosyncDeclaredIndexes(which skips by name) never re-imposes the NULL-distinct form on a later boot.ensureOverlayIndexdoes the same withCOALESCE(package_id, '')forsys_metadata. Proven twice; leaves the declaration unable to say what it means.SqlDriver.createNullSafeUniqueIndex's existing MySQL functional-key fallback extended to it.Route 2 is the ADR-0078 direction (a declaration that cannot express its own constraint is a declared-but-unenforced surface), but route 1 is what has shipped twice. Recording both rather than choosing.
Decision needed before implementation
For a duplicate-carrying installation, what happens to the extra rows — refuse to migrate and hand the operator a list, or apply a deterministic keep-one rule (newest
updated_at)? Settings values are admin-authored configuration, so silently discarding one is a real data decision, not a cleanup.Related
sys_setting's unique key is installation-wide on a tenant-scoped object — but unlike the #8323 class it has a real argument for staying that way #8555 — the per-organization respelling of this same index (the relaxation half).sys_view_definition, fixed via route 1.