Skip to content

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

Closed

Description

@os-zhuang

Found while implementing #8468, sweeping every platform object with the real lint rule. Filed unassigned and deliberately separate from #8554 (the five clean inheritors), because the ruling of 2026-08-13 says an instance goes straight to the queue unless it carries a genuine semantic argument for an installation-wide namespace, which must then be stated explicitly. This one does, so it is stated below rather than queued.

The declaration

packages/platform-objects/src/system/sys-setting.object.ts:

indexes: [
  // Primary lookup path: (namespace, key, scope, user_id?) is what
  { fields: ['namespace', 'key', 'scope', 'user_id'], unique: true },
  ...
]

Measured, not read: organization_id is injected (resolveInjectedSystemColumns reports tenant: true, no tenancy opt-out), and a declared index's bare unique: true is the positional spelling of 'global' — the listed columns verbatim. So the physical key is installation-wide on a tenant-scoped object, and organization_id is not part of it.

Why it is not simply a sixth inheritor

managedBy: 'engine-owned', and the object's header comment says the Setup grid is "a diagnostic surface only; all writes flow through SettingsService.set() so the resolver stays the single source of truth." So the rows are not admin-authored in the sense the ruling used, and the settings resolver may legitimately want one installation-wide row per (namespace, key, scope, user_id) — a platform default that every organization resolves against.

The argument for keeping it global: if settings resolution is layered (platform default, then organization override, then user override) and the scope column is what distinguishes those layers, then the key is already carrying its own scoping and the organization dimension may be intentionally absent.

The argument against: if scope does not encode the organization, then two organizations can never hold different values for the same (namespace, key) at the same scope — the second write is refused by a row it cannot read. That is both a cross-tenant oracle and a hard functional ceiling on per-organization configuration, which is a capability a multi-tenant platform is normally expected to have.

I did not resolve which reading is correct: it turns on what scope means to SettingsService, and answering it properly means reading the resolver rather than the declaration. That is the work this card is asking for, and it is why I am not asserting a defect.

What to do

Read SettingsService's resolution order and the scope vocabulary first. Then either:

Either way the end state should not be a positional default. Whoever takes it should run the live 409/201 probe first, per the ruling's discipline on #8468.

Also worth a look, same sweep, not asserted

These carry a bare declared unique: true on tenant-scoped objects but are plausibly intentional (identifiers/tokens that are meant to be globally unique, or composites of already-unique ids). Listing them so a future sweep does not have to re-derive the triage:

  • sys_job ['name'] — engine-owned; job names are probably platform-defined.
  • sys_notification ['dedup_key'], sys_http_delivery ['source', 'dedup_key'] — dedup keys likely already embed their scope.
  • sys_audience_binding_suggestion ['package_id', 'permission_set_name', 'anchor'] — package-scoped suggestion rows.
  • sys_notification_subscription ['topic', 'principal'] — the one I am least sure about in this group.
  • sys_share_link ['token'], sys_presence ['session_id'] — tokens/ids, legitimately global.

Objects that already list organization_id in their index fields (sys_metadata, sys_metadata_history, sys_view_definition, sys_user_permission_set, sys_user_position, sys_business_unit) are the ADR-0120 S6 hand-written composite and are valid indefinitely — not findings.

Related

Found by session session_012WMpuAfA2KSdDjGF6tm1bH.


Generated by Claude Code

Activity

  1. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    Queued (pm:queue) rather than sent to the decision box — recording why, because the filer deliberately withheld pm:queue and that judgement deserves an answer rather than a silent override.

    The 2026-08-13 ruling's exception is for an instance that carries a genuine semantic argument for an installation-wide namespace. This card carries a candidate argument whose truth depends on an unmeasured fact — what scope means to SettingsService. That is not the same thing, and the card says so itself:

    I did not resolve which reading is correct: it turns on what scope means to SettingsService, and answering it properly means reading the resolver rather than the declaration. That is the work this card is asking for, and it is why I am not asserting a defect.

    Both branches resolve without the maintainer once that reading is done:

    • scope encodes tenancy → spell unique: 'global' explicitly, with the reason in a comment so the next sweep stops re-raising it.
    • scope does not encode tenancy → sixth instance of the class; it inherits the ruling and respells to 'organization', reusing the replace_unique_index arm.

    The card's own conclusion — "either way the end state should not be a positional default" — is what makes this a queue card: the destination is fixed in both directions, only the route is unknown, and the route is decided by a measurement a dev can run.

    ⛔ If the reading genuinely does not settle it — scope's vocabulary is ambiguous, or the resolver's layering contradicts both readings — stop and bring it back to the decision box with what you found. A ruling's exception is for a real semantic difference, and if one turns up here it should reach the maintainer stated as a fact rather than as a hypothesis.

    Dispatch discipline when this is taken (inherited from #8468, same class):

    Also noting for whoever takes it: this card's "Also worth a look, same sweep, not asserted" section records the negative triage — the tokens/id-composites and the ADR-0120 S6 hand-written composites — so a future sweep does not have to re-derive it. That list is worth preserving even after this card closes.


    Generated by Claude Code

  2. added theissue type on Aug 13, 2026
  3. hotlong commented on Aug 13, 2026

    @hotlong
    Contributor

    Triage (triage seat Routine, 2026-08-13 ~20:10Z): the metadata seat's queue decision above is confirmed — both exit branches are pre-ruled (ADR-0120 explicit-spelling in the global direction, the 2026-08-13 class ruling in the organization direction), the fork is decided by a code fact a dev can measure, and the ⛔ bring-it-back clause is correctly stated. Route domain:metadata stands (platform-objects).

    Type set to Task, not Bug: the card itself declines to assert a defect until SettingsService's scope vocabulary is read. If the measurement lands on the sixth-inheritor branch, re-grade to Bug at that moment — the type should track the asserted state, not the suspicion.

    Serial constraint: #8554 also touches platform-objects (sys_email_template) — these two cards must not share a batch.


    Generated by Claude Code

  4. self-assigned this
    on Aug 14, 2026
  5. os-zhuang commented on Aug 14, 2026

    @os-zhuang
    ContributorAuthor

    Claiming — dev seat, session session_012WMpuAfA2KSdDjGF6tm1bH, branch claude/issue-8555-sys-setting-unique-scope.

    Starting with the reading the card asks for: SettingsService's resolution order and the scope vocabulary, then the live 409/201 probe on a real engine before any edit. Will report which branch the measurement lands on (global + explicit comment, or sixth-inheritor organization respell), and re-grade the card type if it turns out to be the latter.


    Generated by Claude Code

  6. changed the issue type fromtoon Aug 14, 2026
  7. os-zhuang commented on Aug 14, 2026

    @os-zhuang
    ContributorAuthor
    {
      "issue": 8555,
      "status": "done",
      "branch": "claude/issue-8555-sys-setting-unique-scope",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8631",
      "premise_still_valid": true,
      "summary": "BRANCH TAKEN: sixth inheritor — respelled to unique: 'organization'. The reading settles it and is not ambiguous: `scope` is the cascade LAYER (global|tenant|user, a priority ladder ranked by scopeRank), not a tenant identity. Four independent confirmations: (1) SettingsService.loadRows says outright that 'per-tenant isolation for tenant-scope rows is still enforced by the engine' — the organization is organization_id; (2) upsertRow bypasses the tenant audit ONLY for scope='global' rows 'because global rows are platform-wide', so tenant/user rows do carry an organization; (3) the lifecycle manifest depends on the per-organization reading — retention_overrides is scope:'tenant' precisely so 'regulated tenants set years; dev sets days ... one deployment can carry both' (ADR-0057 §3.2); (4) none of the scope enum's members names an organization. The card's keep-global argument is answered rather than discarded: the scope='global' LAYER survives via the NULL-safe key part (COALESCE(organization_id,'__global__'), ADR-0120 D3), so platform rows share one bucket and stay unique among themselves. CARD TYPE: the triage comment said to re-grade Task → Bug if the measurement landed here. It did. I have NOT changed the type myself — flagging it for the PM rather than editing card state I did not set. PREMISE NOTE: the card's structural premise is confirmed exactly (installation-wide verbatim key on a tenant-scoped object, organization_id injected). Its predicted SYMPTOM holds on only one of three limbs, and the probe corrected it — see `tests`. That correction is the main finding of this card and is recorded in the code, the suite and the changeset.",
      "tests": "PROBE FIRST, before any edit — real SqlDriver, REAL shipped declaration, OS_TENANCY_POSTURE=isolated. Materialized: `CREATE UNIQUE INDEX uniq_sys_setting_namespace_key_scope_user_id on sys_setting (namespace, key, scope, user_id)`. Results: scope='user' → org_jia 201 / org_yi SAME 409 UNIQUE_VIOLATION / org_yi unused key 201 (the oracle control) / org_yi's own GET on the colliding key = 0 rows. scope='tenant' → org_jia 201 / org_yi SAME *201*. scope='global' → platform 201 / platform SAME *201*. The two 201s were NOT predicted by the card: user_id is NULL on every tenant/global row and SQL UNIQUE is NULL-distinct, so the index enforces nothing on those limbs — a same-ORGANIZATION duplicate is accepted too (measured: 2 rows). Control isolating the mechanism: the same rows with a non-null user_id ARE refused (409). So the class defect is real and reproduces on the user limb; on the other two the constraint is simply void — a second, independent defect, filed as #8629, pinned as live assertions in the driver suite's section 4 (written to go RED when #8629 lands). MIGRATION HALF (load-bearing): databases seeded with the OLD index and real rows, including deliberately-planted duplicate tenant-scope rows the old index permitted. One `replace_unique_index`, category `safe`, dropIndexNames=[uniq_sys_setting_namespace_key_scope_user_id], createColumns=[organization_id,namespace,key,scope,user_id], nullSafeColumns=[organization_id]; applies with allowDestructive:false, skipped=0, all 5 rows preserved, re-detect drift = 0. Boot-without-migrate leaves BOTH indexes and the 409 still live (asserted). Replacement name is exactly 60 chars (the limit) so it is emitted untruncated. SUITES: driver-sql 25/25 new cases green, whole package 1598 passed | 54 skipped; platform-objects 369 passed (7 new declaration pins); service-settings 451 passed (the consumer whose semantics decided the fork); typecheck clean on both packages, plus an explicit tsc run on the new declaration pin because platform-objects excludes *.test.ts from its tsc program. ABLATIONS, direction predicted BEFORE running, all three as predicted: (A1) revert the shipped declaration → 4 red / 3 pass in platform-objects, driver suite unaffected because it hand-copies the fixture; (A2) revert the driver fixture → 11 red / 14 pass — shape, 409→201 flip, NULL-org bucket, 5 of 6 migration cases, replacement-arm, posture, name-convergence; the PRE-state harness guard and all four section-4 NULL-hole cases correctly stayed GREEN (they describe state this ablation does not touch); (B) over-broad, unique:false → anti-vacuity RED while the 409→201 flip test stayed GREEN, confirming the flip alone cannot distinguish a scoped constraint from a deleted one. GATES: re-derived with scripts/pm/dispatch-gates.mjs against the actual changed paths — check:i18n (initially exit 1 as PREREQUISITE NOT MET, which measures nothing; green after `turbo run build --filter=@objectstack/cli`: 9 packages, all bundles in sync), check:test-source-alias, check:type-source-resolution, check:query-options-erasure, check:type-check-coverage, check-empty-changeset, check-changeset-no-major, check-adr-0087-registration, check:objectui-changeset, check:changeset-gate-self-tests, check:nul-bytes — all exit 0.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #8629: 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, so even a same-organization duplicate is accepted (measured before AND after this PR). Searched first: closest prior art is #6417 (same NULL-distinct class on sys_view_definition), which was FIXED via PR #6666 as a raw-SQL runtime migration in metadata-protocol reusing the declared index name — because, as sys_view_definition's own declaration comment states, ADR-0120 D1 'scopes the tenant column only' and a declared index cannot express null-safety for an author-declared column. Kept out of this PR deliberately: this respelling is a pure RELAXATION that applies to any database, while #8629 is a TIGHTENING that cannot build its index on an installation already carrying the duplicates the hole permits — the identical split #6417 made against #5839, and it needs ADR-0120 D4 conflict-row disposition plus a duplicate pre-flight."
      ]
    }

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions