diff --git a/.changeset/adr0120-isolated-install-gate-and-truth-sweep.md b/.changeset/adr0120-isolated-install-gate-and-truth-sweep.md new file mode 100644 index 0000000000..77c74805ef --- /dev/null +++ b/.changeset/adr0120-isolated-install-gate-and-truth-sweep.md @@ -0,0 +1,81 @@ +--- +"@objectstack/types": minor +"@objectstack/cloud-connection": minor +"@objectstack/lint": minor +"@objectstack/cli": minor +"@objectstack/spec": patch +"@objectstack/driver-sql": patch +--- + +feat(types,cloud-connection,lint,cli): ADR-0120 17.x 收尾 —— `isolated` 安装期姿态硬门(D5e)、D5c 重拼写 advisory、成文契约扫荡与三姿态 conformance (#5081) + +ADR-0120 17.x 波的第三块,也是最后一块。前两块已在 main 上:#5212(driver 侧 +D3+D4 —— `COALESCE(organization_id, '__global__')` 物化、drift 两侧同步、重复预检) +与 #5208(spec 词汇 `'organization'` + D5a/D5b lint)。本次补齐三件事:安装期的 +姿态决策点、剩余的成文契约、以及把「一个 app 包跑遍三种姿态」从假设变成测试。 + +**D5e —— 装进 `isolated` 环境时的硬门。** 词汇本身是姿态无关的:作者说的是业务 +边界(`'organization'` 一个组织一份 / `'global'` 整个安装一份),没有任何索引形状 +读姿态。唯一的残留在一个方向上:`isolated` 下组织就是**不同客户**,此时 app 业务 +对象上的 `'global'` 唯一既跨客户过度约束,又变成跨客户的存在性预言机(S10/S14)。 +维护者裁定这是**硬门而非 advisory**:把带 `'global'` 唯一(非 `sys` 对象)的 app +装进 `isolated` 环境会**停下来并逐索引列出**,安装者(通常是 AI agent)要么确认它 +确实是平台级的,要么改写为 `'organization'`;确认按 ADR-0104 attestation 风格 +留痕在安装清单里(`InstalledManifestEntry.globalUniqueAttestation` —— 确认了什么、 +谁确认的、何时、在哪个姿态下问的),**之后不复问**。 + +- 停下的安装**什么都不留**:先于 hot-register 和任何 ledger 写入,所以作者改完 + 元数据可以直接重试,不需要先卸载。 +- 逐索引确认是有牙齿的:`confirmGlobalUniques` 收 `true` 或明确的 id 数组,只确认 + 其中一条仍会在剩下的那条上停住。 +- 升级引入的**新**约束会被问,老的答案继续算数。 +- 另一个姿态下给出的确认**不算同意** —— `isolated` 那个问题在 `single` 下从未被 + 问过,所以按「未确认」处理(唯一不会静默放行跨客户约束的方向)。 +- ⛔ **永不做成启动期告警**(#4884 纪律)。boot 时的 rehydrate 不评估此门;门够不到 + 的两类存量 —— 门禁上线前的安装、装后姿态变更的环境 —— 由 `os doctor` 与 + `os migrate plan` 的 advisory 形态覆盖。 + +判定里有三条是承重的,别「简化」掉:声明索引上的裸 `unique: true` **算**(D1 说它 +就是 `'global'` 的位置式拼写,排除它等于让整个 17.x 可以靠拼写绕过);字段级 +`true` **不算**(它是 `'organization'`,永久合法);`sys_`/`base_` 对象**不算** +(S5 那批引擎幂等键天然就是平台级的,每次安装都问一遍就是 #4884 的误报类)。 + +CLI: `os package install` 新增 `--confirm-global-uniques`,并把 409 渲染成可读的 +逐条清单而不是一句 "Install failed (409)"。 + +**D5c —— 遗留手写组织复合索引的 advisory。** 新规则 +`unique/legacy-organization-composite`:声明的唯一索引自己列出了组织列 +(`{ fields: ['name','organization_id'], unique: true }`)—— 这是词汇出现之前手写 +per-organization 的写法。它读起来像「每组织唯一」,物化出来却是普通复合索引,而 +SQL UNIQUE 是 NULL-distinct 的:组织列为 NULL 的行上它**什么都不约束**(#5030), +在单组织部署上那就是每一行。改写成 `unique: 'organization'`(`fields` 原样保留, +driver 会把已列出的组织列**就地**变成 NULL-safe 形式)正是补上这个洞的动作。 +**永远只是 advisory,永远不自动修**:老拼写永久合法、零强制 drift,而 opt-in 是 +真实的物理收紧,要走 D4 的 `recreate_index` + 重复预检。 + +**D6 —— 成文契约扫荡。** `content/docs/data-modeling/indexing.mdx` 的 +§Two ways to say "unique" 全节按新词汇重写(含 `os:check` 代码块); +`content/docs/protocol/objectql/schema.mdx` 的 §Uniqueness and tenancy 重写为 +§Uniqueness and scope —— 其中那句「单租户部署不受影响,租户列是常量,复合索引 +退化为单列索引」是 #5030 **证伪过的原话**,现已替换为 D3 的 NULL-safe 事实; +`content/docs/deployment/cli.mdx` 的 `replace_unique_index` / `recreate_index` +条目补上 NULL-safe 形状与重复预检;`content/docs/references/**` 经 +`gen:schema && gen:docs` 再生成,未手改。 + +按 ADR-0120 Resolved #2 的非规范性引导(官方示例/脚手架/生成器在新代码中输出 +显式拼写),`skills/objectstack-data/**` 的索引与校验规则整体扫过:声明索引一律 +说清 scope,并新增一节完整讲 `'organization'` 的 NULL-safe 语义与「永远不写姿态」。 +顺带修掉那里长期使用的 `tenant_id` —— 平台的列叫 `organization_id`。 +`examples/**`、`create-objectstack` 模板与 `os generate` 经核查**根本没有声明任何 +唯一约束**,故无可扫;这是核查结论,不是遗漏。 + +**三姿态 conformance(ADR §Acceptance tests)。** 同一个 fixture app 在 +`single | group | isolated` 三姿态下启动,逐 S 行用**真实的违规插入**断言 enforcement +(S1/S2/S3/S4/S5/S6/S7/S8/S9/S11/S12),并逐姿态捕获物化出的索引键,断言三者 +**逐字节相同** —— 「没有任何索引形状读姿态」这句话一旦有两者不同就是假的。相同性 +断言配了一条正向断言(对着期望的键形状),这样「三次都什么都没建」不会读成「一致」。 +外加 ADR 只要的那一条 transition smoke:在 `single` 下建库、`isolated` 下重新打开, +drift op 为零。 + +对既有部署的影响:除新增的安装期确认外,本次不改变任何已有物化行为。字段级 +`unique: true` 一如既往合法。 diff --git a/content/docs/data-modeling/field-types.mdx b/content/docs/data-modeling/field-types.mdx index 1afbbd9c25..8b8acb1d29 100644 --- a/content/docs/data-modeling/field-types.mdx +++ b/content/docs/data-modeling/field-types.mdx @@ -49,7 +49,7 @@ Email address with built-in format validation. | `maxLength` | `number` | — | Maximum character length | ```typescript -{ name: 'email', label: 'Email', type: 'email', required: true, unique: true } +{ name: 'email', label: 'Email', type: 'email', required: true, unique: 'organization' } ``` ### `url` diff --git a/content/docs/data-modeling/fields.mdx b/content/docs/data-modeling/fields.mdx index a7a6112c7f..23fc6f0249 100644 --- a/content/docs/data-modeling/fields.mdx +++ b/content/docs/data-modeling/fields.mdx @@ -21,7 +21,7 @@ export const Contact = ObjectSchema.create({ fields: { first_name: Field.text({ label: 'First Name', required: true }), last_name: Field.text({ label: 'Last Name', required: true }), - email: Field.email({ label: 'Email', unique: true }), + email: Field.email({ label: 'Email', unique: 'organization' }), phone: Field.phone({ label: 'Phone' }), birth_date: Field.date({ label: 'Date of Birth' }), is_active: Field.boolean({ label: 'Active', defaultValue: true }), diff --git a/content/docs/data-modeling/indexing.mdx b/content/docs/data-modeling/indexing.mdx index 4aa559f4e7..24b2ded18d 100644 --- a/content/docs/data-modeling/indexing.mdx +++ b/content/docs/data-modeling/indexing.mdx @@ -12,63 +12,130 @@ Optimize query performance with indexes: indexes: [ // Single field index { fields: ['name'], unique: false }, - - // Unique index - { fields: ['email'], unique: true }, - + + // Unique index — one holder per organization + { fields: ['email'], unique: 'organization' }, + + // Unique index — one holder across the whole installation + { fields: ['hostname'], unique: 'global' }, + // Compound index { fields: ['type', 'is_active'], unique: false }, - + // Lookup field index { fields: ['owner'], unique: false }, ] ``` -## Two ways to say "unique" — and they mean different things +## Two ways to say "unique" — one vocabulary for both -Uniqueness can be declared in two places, and the choice is not cosmetic: +Uniqueness can be declared in two places: on a field, or as a declared index. +Both use the same word to say **which boundary the value must be unique +within** ([ADR-0120](https://github.com/objectstack-ai/objectstack/blob/main/docs/adr/0120-unique-scope-vocabulary-and-null-safe-tenant-uniqueness.md)): -| Declaration | Materializes as | Scope | +| Scope | Meaning | Materializes as | |:---|:---|:---| -| Field-level `unique: true` | `(organization_id, field)` | Unique **within** an organization | -| Field-level `unique: 'global'` | `(field)` | Platform-wide | -| Declared index `{ fields: ['email'], unique: true }` | `(email)` — exactly the listed columns | Platform-wide | +| `unique: 'organization'` | One holder **per organization** | `(COALESCE(organization_id, '__global__'), …fields)` | +| `unique: 'global'` | One holder across the **whole installation** | exactly the listed column(s) | + +```typescript +// Field level — the scope is the same word in both places +email: Field.email({ unique: 'organization' }), // one contact per org +hostname: Field.text({ unique: 'global' }), // one holder, platform-wide -A field-level `unique: true` is **tenant-scoped**. It has no syntax for a -composite, so the platform supplies the tenant column for you — which is what -a multi-tenant application almost always wants: two organizations may each -have a contact `john@acme.com`. +indexes: [ + // per organization — you do NOT list organization_id yourself + { fields: ['department', 'code'], unique: 'organization' }, + // platform-wide — exactly the columns you list + { fields: ['source', 'dedup_key'], unique: 'global' }, +] +``` -A **declared index is taken verbatim**. No tenant column is injected, because -many declared indexes are legitimately platform-wide (a DNS hostname, a -reserved slug, an external provider id). To scope one per tenant, list the -column yourself: `{ fields: ['organization_id', 'email'], unique: true }`. +Two rules make the choice safe to write and safe to deploy: + +1. **You state a business boundary, never a deployment shape.** The same app + package runs unmodified under every tenancy posture — no index shape reads + the posture, so moving a deployment between postures has no schema + consequence. +2. **`'organization'` is NULL-safe.** Rows with no organization (platform rows, + and *every* row on a single-organization deployment) collapse into one + platform bucket and are unique among themselves. Without this a plain + `(organization_id, …)` composite enforces nothing on those rows, because SQL + `UNIQUE` treats every `NULL` as distinct. + + +**Bare `unique: true`.** On a **field** it means `'organization'` and stays +valid indefinitely — `email: Field.email({ unique: true })` is correct, and the +explicit spelling is simply preferred in new code. + +On a **declared index** it is the deprecated spelling of `'global'`: it +materializes over exactly the listed columns. Because that reads like the +field-level meaning but does the opposite, `os lint` / `os build` / `os validate` +report it as `unique/unscoped-declared-index`, and it is rejected outright at +protocol 18. State the scope. + -**Do not declare both on the same column.** The stricter one wins physically, -so the platform-wide index enforces uniqueness and the per-tenant constraint -can never be reached — one of the two intents you wrote is silently discarded: +**Do not declare both on the same column.** The installation-wide one wins +physically, so the per-organization constraint can never be reached — one of the +two intents you wrote is silently discarded: ```typescript -// ⚠️ contradictory — the global index wins, the per-tenant scope is dead -email: Field.email({ unique: true }), -indexes: [{ fields: ['email'], unique: true }], +// ⚠️ contradictory — the global index wins, the per-organization scope is dead +email: Field.email({ unique: 'organization' }), +indexes: [{ fields: ['email'], unique: 'global' }], ``` `os lint` / `os build` report this as `unique/double-declaration`. Pick one: -set `unique: 'global'` on the field and drop the index for platform-wide -uniqueness, or drop the index for per-tenant uniqueness (the field-level -declaration already builds the composite). +keep `unique: 'global'` on the field and drop the index for platform-wide +uniqueness, or drop the index and keep `unique: 'organization'` on the field. -**Never put a platform-wide unique index on an `autonumber` field.** The -autonumber sequence is per tenant — every organization counts from `1` — so a -global unique index rejects the second organization's `CASE-00001` on insert. -Use `{ fields: ['organization_id', 'case_number'], unique: true }` so the -constraint matches the sequence that feeds it. +**Never put a `'global'` unique index on an `autonumber` field.** The autonumber +sequence is per organization — every organization counts from `1` — so an +installation-wide unique index rejects the second organization's `CASE-00001` +on insert. Use `unique: 'organization'` so the constraint matches the sequence +that feeds it. +### Already have a hand-written organization composite? + +Metadata written before the vocabulary existed spells the per-organization +constraint by listing the column itself: + +```typescript +// the legacy spelling — still valid, still materialized exactly as written +indexes: [{ fields: ['name', 'organization_id'], unique: true }] +``` + +This keeps working forever and forces no migration, so `os lint` only *suggests* +the change (`unique/legacy-organization-composite`). Opting in is worth it +because it closes the NULL hole above — keep `fields` exactly as they are and +change the scope word: + +```typescript +indexes: [{ fields: ['name', 'organization_id'], unique: 'organization' }] +``` + +The listed organization column becomes NULL-safe in place; no second +organization key part is added. Because this genuinely tightens the constraint, +it surfaces as a `recreate_index` migration guarded by a duplicate pre-flight +probe — if rows the old index wrongly admitted are still there, `os migrate plan` +reports them instead of failing a boot. See +[CLI · migrations](/docs/deployment/cli). + +### Installing an app into an `isolated` deployment + +Under the `isolated` posture, organizations are separate **customers**. A +`'global'` unique on an app's own object therefore constrains *across +customers* — right for a DNS hostname or an external provider id, almost never +right for a business rule. Installing such an app stops and lists each index so +the installer can confirm it or rewrite it to `'organization'`; the answer is +recorded in the install manifest and never asked again. `os doctor` and +`os migrate plan` report the same finding for apps installed before that check +existed, or for environments whose posture changed afterwards. + ### When to Add Indexes ✅ **Add indexes for:** diff --git a/content/docs/data-modeling/objects.mdx b/content/docs/data-modeling/objects.mdx index bc342b6c3e..d94b794c33 100644 --- a/content/docs/data-modeling/objects.mdx +++ b/content/docs/data-modeling/objects.mdx @@ -197,7 +197,7 @@ Optimize query performance: ```typescript indexes: [ { fields: ['name'], type: 'btree', unique: false }, - { fields: ['email'], type: 'btree', unique: true }, + { fields: ['email'], type: 'btree', unique: 'organization' }, { fields: ['type', 'status'], type: 'btree', unique: false }, ] ``` diff --git a/content/docs/data-modeling/validation.mdx b/content/docs/data-modeling/validation.mdx index 9a229a2aff..52c7e26a59 100644 --- a/content/docs/data-modeling/validation.mdx +++ b/content/docs/data-modeling/validation.mdx @@ -10,7 +10,7 @@ description: Define data integrity rules — formula conditions, uniqueness, for Three patterns that look like validation rules are deliberately **not** rule types, because each needs I/O or is a client-side concern. Use the layer that already does each one correctly: -- **Uniqueness** → a unique index (`ObjectSchema.indexes`, `{ fields, unique: true }`, with `partial` for a scoped constraint) or field-level `unique: true`. A SELECT-then-INSERT rule is inherently racy (TOCTOU); a DB unique constraint is not. +- **Uniqueness** → a unique index (`ObjectSchema.indexes`, `{ fields, unique: 'organization' | 'global' }` — state the scope, [ADR-0120](/docs/data-modeling/indexing); `partial` for a scoped constraint) or field-level `unique`. A SELECT-then-INSERT rule is inherently racy (TOCTOU); a DB unique constraint is not. - **Async / remote validation** → a client-form concern, and an SSRF/latency hazard on the server write path. Keep it in the form layer, or enforce the invariant with a `unique` index / lifecycle hook. - **Custom handler** → a `beforeInsert` / `beforeUpdate` lifecycle hook, the supported extension point for arbitrary validation code. - **Delete-time guards** → a `beforeDelete` lifecycle hook. Validation rules run only on insert/update (a delete carries no record payload to validate), so there is no `'delete'` validation event — block or gate deletions from a `beforeDelete` hook. @@ -127,11 +127,11 @@ There is **no** `uniqueness` validation type. A SELECT-then-INSERT rule is inher ```typescript // Field-level uniqueness -email: Field.email({ label: 'Contact Email', unique: true }), +email: Field.email({ label: 'Contact Email', unique: 'organization' }), // Composite / scoped uniqueness via ObjectSchema.indexes indexes: [ - { fields: ['code', 'organization'], unique: true }, + { fields: ['code'], unique: 'organization' }, // `partial` expresses a scoped/conditional constraint ] ``` @@ -332,7 +332,7 @@ validations: [ ] // Order-number uniqueness is enforced with an index, not a validation rule: -// indexes: [{ fields: ['order_number'], unique: true }] +// indexes: [{ fields: ['order_number'], unique: 'global' }] ``` ## CEL Functions and Operators diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index eaf0a8f956..05f3266a90 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -598,7 +598,7 @@ occupancy on its own. | Category | Examples | Applied by | |----------|----------|------------| -| `safe` | relax `NOT NULL` → nullable, widen a `varchar`, create a declared index, replace a legacy global unique with its tenant-scoped composite | `os migrate apply` (and dev auto-reconcile) | +| `safe` | relax `NOT NULL` → nullable, widen a `varchar`, create a declared index, replace a legacy installation-wide unique with its per-organization composite | `os migrate apply` (and dev auto-reconcile) | | `needs_confirm` | non-narrowing type change, rebuild a non-unique index whose columns changed | `os migrate apply` | | `destructive` | drop an orphaned column or index, tighten `NOT NULL`, narrow a type, rebuild an index as `UNIQUE` | `os migrate apply --allow-destructive` | @@ -609,8 +609,8 @@ occupancy on its own. | Op | What it means | |----|---------------| | `create_index` | Metadata declares an index the database does not have | -| `replace_unique_index` | A field's `unique` used to be enforced platform-wide, but metadata now scopes it per tenant — the legacy single-column index is swapped for the `(tenantField, field)` composite. A pure relaxation: it creates before it drops, and cannot fail | -| `recreate_index` | An index exists under the declared name but with different columns/uniqueness. The additive sync skips it by name, so it must be dropped and rebuilt | +| `replace_unique_index` | A field's `unique` used to be enforced installation-wide, but metadata now scopes it per organization — the legacy single-column index is swapped for the NULL-safe `(COALESCE(organization_id, '__global__'), field)` composite. A pure relaxation: it creates before it drops, and cannot fail | +| `recreate_index` | An index exists under the declared name but with different columns/uniqueness. The additive sync skips it by name, so it must be dropped and rebuilt. This is also how a per-organization unique becomes NULL-safe: a **tightening**, so it runs a duplicate pre-flight probe first — rows the old NULL-distinct index wrongly admitted **block** the op with a report instead of failing a boot, and the old index stays in place until they are resolved | | `drop_index` | An index carrying ObjectStack's generated naming (`uniq_…` / `idx_…`) that metadata no longer declares | Orphan detection is deliberately limited to indexes ObjectStack itself @@ -620,7 +620,7 @@ drift, and `--allow-destructive` will not delete it. **Dev self-heal.** `os dev` runs the SQL driver with `autoMigrate: 'safe'`, so safe changes (you just made a field optional; a `unique` field became -tenant-scoped) are applied to your existing dev database automatically on +organization-scoped) are applied to your existing dev database automatically on restart — no `os migrate` needed, no data loss. Auto-reconcile is **dev-only and never destructive**; it is force-disabled under `NODE_ENV=production`, where every change is shown by `os migrate plan` before you apply it deliberately. diff --git a/content/docs/protocol/kernel/lifecycle.mdx b/content/docs/protocol/kernel/lifecycle.mdx index 356bf33322..29247fca56 100644 --- a/content/docs/protocol/kernel/lifecycle.mdx +++ b/content/docs/protocol/kernel/lifecycle.mdx @@ -483,7 +483,7 @@ export default ObjectSchema.create({ name: 'salesforce_account', label: 'Salesforce Account', fields: { - salesforce_id: Field.text({ required: true, unique: true }), + salesforce_id: Field.text({ required: true, unique: 'global' }), account_name: Field.text(), last_sync: Field.datetime(), }, diff --git a/content/docs/protocol/objectql/schema.mdx b/content/docs/protocol/objectql/schema.mdx index 42519167ac..d5e0d711c0 100644 --- a/content/docs/protocol/objectql/schema.mdx +++ b/content/docs/protocol/objectql/schema.mdx @@ -280,7 +280,7 @@ fields: | `type` | `string` | All | **Required.** Field type (see [Types](/docs/protocol/objectql/types)). | | `label` | `string` | All | **Required.** Display label in UI. | | `required` | `boolean` | All | Validation: Field must have a value. | -| `unique` | `boolean \| 'global'` | All | Enforce uniqueness at database level. `true` = unique **within the tenant**; `'global'` = unique platform-wide. See [Uniqueness and tenancy](#uniqueness-and-tenancy). | +| `unique` | `boolean \| 'global' \| 'organization'` | All | Enforce uniqueness at database level, at a stated scope. `'organization'` = one holder **per organization** (`true` is its positional synonym); `'global'` = one holder across the whole installation. See [Uniqueness and scope](#uniqueness-and-scope). | | `searchable` | `boolean` | All | Is searchable. | | `defaultValue` | `any` | All | Default value when creating new records. | | `description` | `string` | All | Tooltip/Help text. | @@ -538,9 +538,9 @@ Optimize query performance with indexes: ```yaml indexes: - # Single-field unique index + # Single-field unique index — state the scope (ADR-0120) - fields: [email] - unique: true + unique: organization # Composite index - fields: [account_id, status] @@ -568,54 +568,70 @@ Supported index `type` values: `btree` (default), `hash`, `gin`, `gist`, `fullte - Fields that change frequently - Large text fields (use full-text search instead) -### Uniqueness and tenancy +### Uniqueness and scope -On a tenant-scoped object, **field-level `unique: true` means unique *within* the -tenant**, not platform-wide. The driver materializes it as a composite index -`(tenantField, field)`: +`unique` states **which boundary the value must be unique within**. There are +exactly two boundaries, and the same two words work on a field and on a declared +index (ADR-0120): ```yaml fields: code: type: autonumber autonumberFormat: 'PROD-{00000}' - unique: true # each tenant may hold its own PROD-00001 + unique: organization # each organization may hold its own PROD-00001 + hostname: + type: text + unique: global # no two organizations may claim the same hostname ``` -This matches every other tenant-aware part of the platform: reads are filtered by -the tenant predicate, writes stamp the tenant column, and the auto-number sequence -gives each tenant a counter starting at 1. A platform-wide index would contradict -the sequence outright — the second tenant's `PROD-00001` would be rejected by an -index it cannot see, and the rejection itself would reveal that *some other tenant* -holds the value. +`'organization'` matches every other organization-aware part of the platform: +reads are filtered by the organization predicate, writes stamp the organization +column, and the auto-number sequence gives each organization a counter starting +at 1. An installation-wide index would contradict the sequence outright — the +second organization's `PROD-00001` would be rejected by an index it cannot see, +and the rejection itself would reveal that *some other organization* holds the +value. -For the identifiers that genuinely are platform-wide — a DNS hostname, a reserved -slug, an external provider id, a device identity — say so explicitly: +Use `'global'` for the identifiers that genuinely are installation-wide: a DNS +hostname, a reserved slug, an external provider id, a device identity, an engine +dedup key. -```yaml -fields: - hostname: - type: text - unique: global # no two tenants may claim the same hostname -``` +Field-level `unique: true` is the positional synonym of `'organization'` and +stays valid indefinitely; `'organization'` is simply the preferred spelling in +new code. -On an object with no tenant column (`tenancy: { enabled: false }`, or simply no -tenant field) both spellings behave identically. **Single-tenant deployments are -unaffected** — the tenant column is constant, so the composite index degenerates -to the single-column one. +**The organization key part is NULL-safe.** It materializes as +`COALESCE(organization_id, '__global__')`, so rows carrying no organization — +platform rows, and *every* row on a single-organization deployment — form one +platform bucket that is unique among itself. A plain `(organization_id, field)` +composite would enforce nothing on those rows, because SQL `UNIQUE` treats every +`NULL` as distinct. The value stored in the column is still `NULL`; the sentinel +exists only inside the index key, so `WHERE organization_id = '__global__'` +matches nothing by design. -**Declared `indexes` are different:** they are materialized over exactly the -columns you list, with no tenant column injected. Write the tenant column yourself -when you want a per-tenant index: +On an object with no organization column (`tenancy: { enabled: false }`, or +simply no such field) `'organization'` degrades to the listed columns alone — +identical to `'global'` there. + +**Declared `indexes` use the same vocabulary:** ```yaml indexes: - - fields: [organization_id, code] # unique per tenant - unique: true - - fields: [hostname] # unique platform-wide - unique: true + - fields: [department, code] # unique per organization — + unique: organization # you do NOT list the organization column + - fields: [hostname] # unique across the whole installation + unique: global ``` +Bare `unique: true` on a *declared index* is the deprecated spelling of +`'global'` (materialized over exactly the listed columns). It is warned by +`os lint` / `os build` / `os validate` as `unique/unscoped-declared-index` and +rejected at protocol 18 — state the scope instead. A legacy index that lists the +organization column itself (`fields: [organization_id, code]`) keeps working +unchanged; respelling its scope to `'organization'` makes the listed column +NULL-safe in place. + ## Lifecycle Hooks Record-triggered logic is **not** an object-schema field — `triggers` (and `hooks`, @@ -800,7 +816,7 @@ fields: # query indexes in the object-level `indexes[]` array email: type: text - unique: true + unique: organization # Use appropriate field types status: @@ -913,7 +929,7 @@ fields: type: text label: SKU required: true - unique: true + unique: organization searchable: true name: diff --git a/content/docs/protocol/objectql/types.mdx b/content/docs/protocol/objectql/types.mdx index c0f11ad5c4..55a8002732 100644 --- a/content/docs/protocol/objectql/types.mdx +++ b/content/docs/protocol/objectql/types.mdx @@ -153,7 +153,7 @@ email: type: email label: Email Address required: true - unique: true + unique: organization ``` **Validation:** a deliberately permissive, ReDoS-safe shape check — a local part, diff --git a/content/docs/protocol/objectui/concept.mdx b/content/docs/protocol/objectui/concept.mdx index c91125966a..43f1f72ac2 100644 --- a/content/docs/protocol/objectui/concept.mdx +++ b/content/docs/protocol/objectui/concept.mdx @@ -219,7 +219,7 @@ fields: email: type: email required: true - unique: true + unique: organization phone: type: tel format: E.164 # +12125551234 diff --git a/content/docs/protocol/objectui/index.mdx b/content/docs/protocol/objectui/index.mdx index c0e379c39c..ea80e8e3ab 100644 --- a/content/docs/protocol/objectui/index.mdx +++ b/content/docs/protocol/objectui/index.mdx @@ -415,7 +415,7 @@ fields: maxLength: 100 email: type: email - unique: true + unique: organization status: type: select options: diff --git a/content/docs/references/data/validation.mdx b/content/docs/references/data/validation.mdx index 65ba7fe6b8..feae326b0e 100644 --- a/content/docs/references/data/validation.mdx +++ b/content/docs/references/data/validation.mdx @@ -51,11 +51,15 @@ These were once declared here but never enforced. Because the contract above rul no-ops. Use the layer that already does each one correctly: -- **Uniqueness** → a unique **index** (`ObjectSchema.indexes`, `\{ fields, unique: true \}`, +- **Uniqueness** → a unique **index** whose scope is stated (`ObjectSchema.indexes`, with -with `partial` for a scoped/conditional constraint), or field-level `unique: true`. A +`unique: 'organization'` for one holder per organization or `unique: 'global'` for one -SELECT-then-INSERT "rule" is inherently racy (TOCTOU); a DB unique constraint is not. +across the whole installation — ADR-0120; `partial` for a scoped/conditional constraint), + +or field-level `unique`. A SELECT-then-INSERT "rule" is inherently racy (TOCTOU); a DB + +unique constraint is not. - **Async / remote validation** → a client-form concern (`debounce`/`validatorUrl` only mean diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index 74d8de7016..c60ee03eb1 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -10,6 +10,15 @@ import { printHeader, printSuccess, printWarning, printError, printStep, printIn import { loadConfig, configExists } from '../utils/config.js'; import { checkSpecVersionGap } from '../utils/spec-version.js'; import { validateWidgetBindings } from '@objectstack/lint'; +import { + resolveTenancyPosture, + collectGlobalUniques, + unconfirmedGlobalUniques, + describeGlobalUniqueFinding, + postureGatesGlobalUniques, + GLOBAL_UNIQUE_ISOLATED_PRESCRIPTION, + type GlobalUniqueFinding, +} from '@objectstack/types'; interface HealthCheckResult { name: string; @@ -231,6 +240,76 @@ export function findUnusedObjects(config: any): string[] { return unused; } +// ─── ADR-0120 D5e — `isolated`-posture unique-scope advisory ──────── + +/** + * The ADVISORY half of ADR-0120 D5e. + * + * The hard gate lives at the install seam, where the two things it needs are + * both present: the app being installed, and an installer who can answer. It + * structurally cannot reach two populations, and this is where those are + * reported instead: + * + * 1. **Installs that predate the gate** — a ledger entry with no attestation. + * 2. **Environments whose posture changed after install** — nothing was + * installed under `isolated`, so nothing was ever asked. + * + * Plus the case with no install seam at all: an app declared in this project's + * own `objectstack.config.ts`, which is code, not a marketplace install. + * + * ⛔ This is `os doctor` / `os migrate plan`, deliberately — NOT a boot-time + * warning. A startup diagnostic here would fire on every boot of every affected + * deployment forever, which is the #4884 false-alarm class the ADR names by + * number. A command someone runs on purpose is the right frequency for a + * finding whose resolution is a human/agent decision. + */ +interface UniqueScopeAdvisory { + /** Where the finding came from — a config-declared app, or a ledger entry. */ + source: string; + finding: GlobalUniqueFinding; +} + +/** Read the installed-package ledger without going through HTTP. Best-effort: + * a runtime that never installed anything simply has no directory. */ +async function readInstalledPackageEntries(cwd: string): Promise { + try { + // Dynamic, like serve.ts's cloud-connection load: `os doctor` must still + // run in a checkout where the optional package is not resolvable. + const mod: any = await import('@objectstack/cloud-connection'); + const dir = path.join(cwd, mod.DEFAULT_INSTALLED_PACKAGES_DIR ?? '.objectstack/installed-packages'); + if (!fs.existsSync(dir)) return []; + return new mod.LocalManifestSource(dir).list(); + } catch { + return []; + } +} + +/** + * Collect every unanswered installation-wide unique this environment would run + * under `isolated`. Returns an empty list under every other posture: there + * `'global'` is the correct, unambiguous meaning (`single` = one customer; + * `group` = the installation IS the customer company). + */ +async function findUnscopedGlobalUniques(cwd: string, config: any): Promise { + const posture = resolveTenancyPosture(); + if (!postureGatesGlobalUniques(posture)) return []; + + const out: UniqueScopeAdvisory[] = []; + for (const finding of collectGlobalUniques(config?.objects)) { + out.push({ source: 'this project’s metadata', finding }); + } + for (const entry of await readInstalledPackageEntries(cwd)) { + const findings = collectGlobalUniques(entry?.manifest?.objects); + // Subtract what the install ceremony already answered for — an attested + // install must not be re-reported, or the advisory becomes the recurring + // nag the gate exists to avoid. + for (const finding of unconfirmedGlobalUniques(findings, entry?.globalUniqueAttestation, posture)) { + out.push({ source: `installed package '${entry?.manifestId ?? entry?.packageId}'`, finding }); + } + } + return out; +} + // ─── Filesystem Checks ────────────────────────────────────────────── function walkDir(dir: string, ext: string): string[] { @@ -602,6 +681,24 @@ export default class Doctor extends Command { } } + // ADR-0120 D5e advisory — installation-wide uniques under `isolated`. + // Runs whenever a config loaded, whether or not it declares objects: + // the ledger half reports installed packages this project never + // declared. + if (postureGatesGlobalUniques(resolveTenancyPosture())) { + printStep("Checking unique scopes against the 'isolated' tenancy posture..."); + const scopeFindings = await findUnscopedGlobalUniques(cwd, config); + if (scopeFindings.length > 0) { + hasWarnings = true; + for (const { source, finding } of scopeFindings) { + printWarning(`Unique scope ${describeGlobalUniqueFinding(finding)} (${source})`); + } + console.log(chalk.dim(` → ${GLOBAL_UNIQUE_ISOLATED_PRESCRIPTION}`)); + } else { + printSuccess("Unique scope No unconfirmed installation-wide uniques for this 'isolated' environment"); + } + } + // Orphan views if (Array.isArray(config.views) && config.views.length > 0) { printStep('Checking for orphan views...'); diff --git a/packages/cli/src/commands/migrate/plan.ts b/packages/cli/src/commands/migrate/plan.ts index 5bfcfa1a74..a92597d0a6 100644 --- a/packages/cli/src/commands/migrate/plan.ts +++ b/packages/cli/src/commands/migrate/plan.ts @@ -21,6 +21,13 @@ import { } from '../../utils/schema-migrate.js'; import { probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js'; import { describeOccupancy } from '../../utils/sqlite-occupancy.js'; +import { + resolveTenancyPosture, + collectGlobalUniques, + describeGlobalUniqueFinding, + postureGatesGlobalUniques, + GLOBAL_UNIQUE_ISOLATED_PRESCRIPTION, +} from '@objectstack/types'; /** * `os migrate plan` — dry-run diff of metadata vs the physical database, @@ -88,6 +95,16 @@ export default class MigratePlan extends Command { const drift = await stack.driver.detectManagedDrift(); const pending = stack.pendingSchemaWork; + // [ADR-0120 D5e, advisory form] Installation-wide uniques on app objects + // are a decision point under the `isolated` posture — organizations there + // are separate CUSTOMERS. The HARD gate runs at app install; this covers + // the two populations it cannot reach (installs predating the gate, and + // environments whose posture changed after install). Never fatal: a plan + // writes nothing, and this reports a metadata decision, not drift. + const uniqueScopeAdvisory = postureGatesGlobalUniques(resolveTenancyPosture()) + ? collectGlobalUniques(stack.allObjects()) + : []; + if (flags.json) { await emitJson({ database: stack.dbLabel, @@ -95,6 +112,22 @@ export default class MigratePlan extends Command { total: drift.length, changes: drift, pending, + ...(uniqueScopeAdvisory.length > 0 + ? { + uniqueScopeAdvisory: { + posture: resolveTenancyPosture(), + adr: 'ADR-0120 D5e', + findings: uniqueScopeAdvisory.map((f) => ({ + id: f.id, + object: f.object, + kind: f.kind, + ...(f.name ? { name: f.name } : {}), + columns: f.columns, + spelling: f.spelling, + })), + }, + } + : {}), ...(occupancy.status === 'busy' ? { occupancy: { status: 'busy', signal: occupancy.signal, detail: occupancy.detail } } : {}), @@ -103,6 +136,18 @@ export default class MigratePlan extends Command { return; } + if (uniqueScopeAdvisory.length > 0) { + printWarning( + `${uniqueScopeAdvisory.length} installation-wide unique constraint(s) on app objects, in an ` + + "'isolated'-posture environment (ADR-0120 D5e):", + ); + for (const finding of uniqueScopeAdvisory) { + console.log(chalk.dim(` • ${describeGlobalUniqueFinding(finding)}`)); + } + console.log(chalk.dim(` → ${GLOBAL_UNIQUE_ISOLATED_PRESCRIPTION}`)); + console.log(''); + } + printInfo(`Database: ${chalk.white(stack.dbLabel)}`); printInfo(`Examined ${chalk.white(String(stack.managedTableCount))} managed table(s).`); console.log(''); diff --git a/packages/cli/src/commands/package/install.ts b/packages/cli/src/commands/package/install.ts index b150547aea..c5f9e4e17c 100644 --- a/packages/cli/src/commands/package/install.ts +++ b/packages/cli/src/commands/package/install.ts @@ -71,6 +71,17 @@ export default class PackageInstall extends Command { env: 'OS_CLOUD_TIMEOUT_MS', default: 120_000, }), + // [ADR-0120 D5e] The installer's answer to the `isolated`-posture question. + // Deliberately NOT default-on and deliberately not named `--force`: it + // records an affirmative fact ("these constraints are genuinely + // platform-wide") into the install manifest, where `os doctor` can later + // show who affirmed it and when. + 'confirm-global-uniques': Flags.boolean({ + description: + "Confirm this app's installation-wide (`unique: 'global'`) constraints are genuinely platform-wide when " + + "installing into an 'isolated'-posture runtime (ADR-0120 D5e). Recorded in the install manifest; asked once.", + default: false, + }), }; async run(): Promise { @@ -151,6 +162,7 @@ export default class PackageInstall extends Command { // ---- Install --------------------------------------------------------- printStep(`Installing ${label} into ${runtime}...`); + if (flags['confirm-global-uniques']) body.confirmGlobalUniques = true; const res = await this.request(`${runtime}/api/v1/marketplace/install-local`, { method: 'POST', headers: { @@ -161,6 +173,23 @@ export default class PackageInstall extends Command { }, flags.timeout); if (!res.ok) { + // [ADR-0120 D5e] The `isolated`-posture stop. Rendered as its own case + // rather than a bare "Install failed (409)": the whole point of the gate + // is that the installer READS the list and decides per index, so the + // list has to survive the trip through the CLI intact. + if (res.body?.error?.code === 'UNIQUE_SCOPE_CONFIRMATION_REQUIRED') { + printError('Install stopped — installation-wide unique constraints need a decision (ADR-0120 D5e)'); + console.log(''); + for (const line of String(res.body.error.message).split('\n')) { + console.log(` ${line}`); + } + console.log(''); + console.log(' Confirm them as genuinely platform-wide:'); + console.log(` os package install ${args.package} --confirm-global-uniques`); + console.log(" …or edit the app's metadata to `unique: 'organization'` and rebuild."); + this.exit(1); + return; + } if (res.status === 401) { printError( 'The runtime rejected the call as unauthenticated. Pass --email/--password ' + diff --git a/packages/cli/src/utils/schema-migrate.ts b/packages/cli/src/utils/schema-migrate.ts index 3e6e4f40c7..a9fef33cd5 100644 --- a/packages/cli/src/utils/schema-migrate.ts +++ b/packages/cli/src/utils/schema-migrate.ts @@ -15,6 +15,7 @@ import chalk from 'chalk'; import type { ManagedDriftEntry, DriftCategory, PendingSchemaWork } from '@objectstack/driver-sql'; import { isInPlaceSchemaWork } from '@objectstack/driver-sql'; +import type { IObjectQLEngine } from '@objectstack/spec/contracts'; import { describeDriverConnection } from './connection-display.js'; export type { PendingSchemaWork }; @@ -45,6 +46,13 @@ export interface SchemaStack { * (#3917). Always `[]` unless the stack was booted with `deferSchemaDdl`. */ pendingSchemaWork: PendingSchemaWork[]; + /** + * Every object the booted stack knows about — the set the plan is computed + * against, including objects that arrived from installed packages rather than + * this project's `objectstack.config.ts`. Read by the ADR-0120 D5e + * unique-scope advisory; best-effort (`[]` when no ObjectQL service composed). + */ + allObjects: () => unknown[]; /** * Perform the deferred sync — call only once the operator has confirmed the * plan. Returns the work it actually ran (`[]` when nothing was deferred). @@ -206,6 +214,29 @@ export async function bootSchemaStack( managedTableCount, kernel, pendingSchemaWork, + /** + * Every object this booted stack knows about — the same set the plan is + * computed against. + * + * Exposed for the ADR-0120 D5e advisory in `os migrate plan`: the advisory + * must describe the objects the migration is actually planning for, not a + * re-read of `objectstack.config.ts`, which on a runtime serving installed + * marketplace packages is a strict subset. Best-effort — a stack with no + * ObjectQL service reports none, and the advisory then simply says nothing. + */ + allObjects: (): unknown[] => { + try { + // The `objectql` slot's contract is `IObjectQLEngine` (#4251) — read it + // through that rather than erasing the lookup to `any`, so a rename of + // `registry` / `getAllObjects` breaks this at compile time instead of + // silently reporting zero objects and turning the D5e advisory mute. + const getService = (kernel as { getService?: (name: string) => unknown })?.getService; + const ql = getService?.call(kernel, 'objectql') as IObjectQLEngine | undefined; + return ql?.registry?.getAllObjects?.() ?? []; + } catch { + return []; + } + }, flushSchemaDdl: async () => (defer && driver?.flushDeferredSchemaDdl ? await driver.flushDeferredSchemaDdl() : []), diff --git a/packages/cli/test/data-model-rules.test.ts b/packages/cli/test/data-model-rules.test.ts index 9a9abab525..b70a6704f4 100644 --- a/packages/cli/test/data-model-rules.test.ts +++ b/packages/cli/test/data-model-rules.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { lintDataModel, lintUniqueDeclarations, lintUnscopedDeclaredIndexes } from '@objectstack/lint'; +import { lintDataModel, lintUniqueDeclarations, lintUnscopedDeclaredIndexes, lintLegacyOrganizationComposites } from '@objectstack/lint'; import { lintConfig } from '../src/commands/lint'; const rulesOf = (issues: { rule: string }[]) => issues.map((i) => i.rule); @@ -447,3 +447,114 @@ describe('lintUniqueDeclarations — double declaration, four scope quadrants (# .toEqual(['crm_contact.email', 'crm_lead.email', 'crm_lead.sku']); }); }); + +// ADR-0120 D5c — the S6 legacy hand-written organization composite. The +// vocabulary now has a word for what these indexes were always trying to say, +// and the respelling is also what closes their NULL hole (#5030). +describe('lintLegacyOrganizationComposites — S6 respelling nudge (ADR-0120 D5c)', () => { + const RULE = 'unique/legacy-organization-composite'; + const legacy = (issues: { rule: string }[]) => issues.filter((i) => i.rule === RULE); + + it('returns [] for empty input', () => { + expect(lintLegacyOrganizationComposites([])).toEqual([]); + expect(lintLegacyOrganizationComposites(undefined as any)).toEqual([]); + }); + + it('warns on a declared unique that lists organization_id, and explains the NULL hole', () => { + const issues = lintLegacyOrganizationComposites([ + { + name: 'sys_team', + fields: { name: { type: 'text' }, organization_id: { type: 'text' } }, + indexes: [{ fields: ['name', 'organization_id'], unique: true }], + }, + ]); + expect(issues).toHaveLength(1); + expect(issues[0].rule).toBe(RULE); + expect(issues[0].severity).toBe('warning'); // advisory forever — zero forced drift + expect(issues[0].path).toBe('objects[0].indexes[0]'); + expect(issues[0].message).toContain('#5030'); + expect(issues[0].message).toContain('NULL-distinct'); + expect(issues[0].fix).toContain("unique: 'organization'"); + // The respelling keeps `fields` — the driver makes the LISTED column + // NULL-safe in place rather than prepending a second key part. + expect(issues[0].fix).toContain('keep `fields` exactly as they are'); + // …and it is honest that opting in is a real migration, not a free rename. + expect(issues[0].fix).toContain('recreate_index'); + expect(issues[0].fix).toContain('pre-flight'); + }); + + it('fires on the explicit `global` spelling too — the scope is still wrong for the shape', () => { + const issues = legacy(lintLegacyOrganizationComposites([ + { + name: 'sys_business_unit', + fields: {}, + indexes: [{ name: 'uk_bu', fields: ['code', 'organization_id'], unique: 'global' }], + }, + ])); + expect(issues).toHaveLength(1); + expect(issues[0].message).toContain("'uk_bu'"); + }); + + it("stays quiet once respelled to 'organization' — that IS the fix", () => { + expect(lintLegacyOrganizationComposites([ + { + name: 'sys_team', + fields: {}, + indexes: [{ fields: ['name', 'organization_id'], unique: 'organization' }], + }, + ])).toEqual([]); + }); + + it('stays quiet for composites that do not list the organization column, and for non-uniques', () => { + expect(lintLegacyOrganizationComposites([ + { + name: 'crm_case', + fields: {}, + indexes: [ + { fields: ['department', 'code'], unique: true }, // R11's business, not R12's + { fields: ['organization_id', 'created_at'] }, // not unique at all + { fields: ['organization_id', 'status'], unique: false }, + ], + }, + ])).toEqual([]); + }); + + it('stays quiet for a unique on the organization column ALONE — not a composite', () => { + // There is no per-organization reading to recover: `'organization'` on an + // index whose only column IS the organization column would say nothing. + expect(lintLegacyOrganizationComposites([ + { name: 'org_settings', fields: {}, indexes: [{ fields: ['organization_id'], unique: true }] }, + ])).toEqual([]); + }); + + it("honors a declared tenancy.tenantField spelling", () => { + const issues = legacy(lintLegacyOrganizationComposites([ + { + name: 'legacy_thing', + tenancy: { tenantField: 'tenant_ref' }, + fields: {}, + indexes: [ + { fields: ['code', 'tenant_ref'], unique: true }, + // organization_id is NOT this object's tenant column — no finding + { fields: ['code2', 'organization_id'], unique: true }, + ], + }, + ])); + expect(issues).toHaveLength(1); + expect(issues[0].message).toContain("'tenant_ref'"); + }); + + it('surfaces through lintDataModel (os lint), exactly once', () => { + const objs = [{ + name: 'sys_member', + fields: {}, + indexes: [{ fields: ['organization_id', 'user_id'], unique: true }], + }]; + expect(has(lintDataModel(objs), RULE)).toBe(true); + expect(lintDataModel(objs).filter((i) => i.rule === RULE)).toHaveLength(1); + // R11 fires on the same index for the OTHER reason (unstated scope) — the + // two rules are complementary, not duplicates: R11 says "say which scope", + // R12 says "the shape tells me which one you meant". + expect(has(lintDataModel(objs), 'unique/unscoped-declared-index')).toBe(true); + }); +}); diff --git a/packages/cloud-connection/src/local-manifest-source.ts b/packages/cloud-connection/src/local-manifest-source.ts index f59b96ea7f..f509a42443 100644 --- a/packages/cloud-connection/src/local-manifest-source.ts +++ b/packages/cloud-connection/src/local-manifest-source.ts @@ -31,6 +31,8 @@ import { existsSync, mkdirSync, readFileSync, readdirSync, unlinkSync, writeFileSync } from 'node:fs'; import { join, resolve } from 'node:path'; +import type { GlobalUniqueAttestation } from '@objectstack/types'; + /** One installed-package entry — desired state + provenance. */ export interface InstalledManifestEntry { packageId: string; @@ -50,6 +52,21 @@ export interface InstalledManifestEntry { * table after a purge is desired state, not data loss. Cleared again by * install/reseed runs that land rows. */ sampleDataPurged?: boolean; + /** + * [ADR-0120 D5e] The installer's answer to the `isolated`-posture question + * about this package's installation-wide (`'global'`) uniques — recorded + * ADR-0104 attestation style: the fact affirmed, by whom, when, and under + * which posture it was asked. + * + * This is what makes the hard stop a ONE-TIME ceremony rather than a + * recurring prompt: `unconfirmedGlobalUniques` subtracts these ids from the + * findings, so a reinstall or upgrade only ever asks about constraints + * nobody has answered for yet. The ledger is the right home because it is + * the desired-state record that survives restarts — a memory-only + * confirmation would re-ask on every process boot, which is precisely the + * boot-time nagging #4884 forbids. + */ + globalUniqueAttestation?: GlobalUniqueAttestation; } /** Default ledger location, relative to the runtime's working directory. */ diff --git a/packages/cloud-connection/src/marketplace-install-local-plugin.ts b/packages/cloud-connection/src/marketplace-install-local-plugin.ts index 1740917ed3..f88b1bcf72 100644 --- a/packages/cloud-connection/src/marketplace-install-local-plugin.ts +++ b/packages/cloud-connection/src/marketplace-install-local-plugin.ts @@ -42,7 +42,17 @@ */ import type { Plugin, PluginContext } from '@objectstack/core'; -import { resolveTenancyPosture } from '@objectstack/types'; +import { + resolveTenancyPosture, + collectGlobalUniques, + unconfirmedGlobalUniques, + recordGlobalUniqueAttestation, + buildGlobalUniqueStopMessage, + describeGlobalUniqueFinding, + postureGatesGlobalUniques, + GLOBAL_UNIQUE_CONFIRMATION_REQUIRED, + type GlobalUniqueFinding, +} from '@objectstack/types'; import { postureEnforcesWall, type TenancyPosture } from '@objectstack/spec/security'; import { resolveCloudUrl } from './cloud-url.js'; import { resolveMarketplacePublicBaseUrl } from './marketplace-public-url.js'; @@ -553,6 +563,45 @@ export class MarketplaceInstallLocalPlugin implements Plugin { }, 409); } + // 2b. [ADR-0120 D5e] `isolated`-posture gate on installation-wide + // uniques. Runs BEFORE hot-register and before anything is written + // to the ledger: a stopped install must leave the runtime exactly as + // it found it, so the installer can rewrite the metadata and retry + // without an uninstall in between. + const previousEntry = this.ledger.read(manifestId); + const gate = this.evaluateGlobalUniqueGate(manifest, previousEntry, body, userId); + if (gate.blocked) { + ctx.logger?.warn?.( + `[MarketplaceInstallLocal] install of ${manifestId} stopped by the ADR-0120 D5e posture gate ` + + `(${gate.pending.length} unconfirmed installation-wide unique(s))`, + ); + return c.json({ + success: false, + error: { + code: GLOBAL_UNIQUE_CONFIRMATION_REQUIRED, + message: buildGlobalUniqueStopMessage(manifestId, gate.pending), + // Machine-readable so an AI installer can decide per index + // instead of re-parsing the prose it was handed. + details: { + posture: gate.posture, + findings: gate.pending.map((f) => ({ + id: f.id, + object: f.object, + kind: f.kind, + ...(f.name ? { name: f.name } : {}), + columns: f.columns, + spelling: f.spelling, + describe: describeGlobalUniqueFinding(f), + })), + confirmWith: { + body: { confirmGlobalUniques: gate.pending.map((f) => f.id) }, + cli: 'os package install … --confirm-global-uniques', + }, + }, + }, + }, 409); + } + // 3. Hot-register FIRST so a malformed inline manifest fails the // install loudly rather than persisting a broken record that // would also fail on every subsequent rehydrate. @@ -586,6 +635,10 @@ export class MarketplaceInstallLocalPlugin implements Plugin { installedAt: new Date().toISOString(), installedBy: userId, withSampleData: false, + // [ADR-0120 D5e] Carry the attestation across the reinstall so the + // ceremony is not re-run for constraints already answered for, and + // fold in whatever this install confirmed. + ...(gate.attestation ? { globalUniqueAttestation: gate.attestation } : {}), }; try { this.ledger.write(entry); @@ -692,6 +745,79 @@ export class MarketplaceInstallLocalPlugin implements Plugin { }, 200); }; + /** + * [ADR-0120 D5e] Decide whether this install must stop for the + * `isolated`-posture confirmation, and what attestation the ledger entry + * should carry afterwards. + * + * The three outcomes, in the order they are checked: + * + * 1. **Not `isolated`** — no findings are decision points at all. Under + * `single` there is one customer; under `group` the installation IS the + * customer company, which is exactly what `'global'` means there. The + * previous attestation (if any) is carried through untouched rather than + * dropped: a posture may flip back, and a confirmation already given is + * still a fact about the posture it was given under. + * 2. **Every finding already answered for** — proceed silently. This is the + * "之后不复问" half of the decision, and it is why the record lives in the + * durable ledger rather than in memory. + * 3. **Something unanswered** — stop, unless THIS request confirms it. + * + * Whatever this returns as `attestation` is written into the install + * manifest in ADR-0104 attestation style — the fact affirmed, by whom, when, + * and under which posture — so the ceremony is evidence, not a dismissed + * prompt. + * + * `confirmGlobalUniques` accepts either `true` (confirm everything the gate + * lists, the shape a CLI `--confirm-global-uniques` produces) or an explicit + * array of finding ids. The array form is the per-index ceremony the ADR + * asks for: confirming two of three constraints still stops on the third, + * so an installer cannot blanket-approve a list it did not read by echoing + * back one id. + */ + private evaluateGlobalUniqueGate = ( + manifest: any, + previous: InstalledEntry | null, + body: any, + installerId: string | null, + ): { + blocked: boolean; + posture: string; + pending: GlobalUniqueFinding[]; + attestation?: InstalledEntry['globalUniqueAttestation']; + } => { + const posture = resolveTenancyPosture(); + const carried = previous?.globalUniqueAttestation; + if (!postureGatesGlobalUniques(posture)) { + return { blocked: false, posture, pending: [], ...(carried ? { attestation: carried } : {}) }; + } + + const findings = collectGlobalUniques(manifest?.objects); + const pending = unconfirmedGlobalUniques(findings, carried, posture); + if (pending.length === 0) { + return { blocked: false, posture, pending: [], ...(carried ? { attestation: carried } : {}) }; + } + + const raw = body?.confirmGlobalUniques; + const confirmedIds = + raw === true + ? pending.map((f) => f.id) + : Array.isArray(raw) + ? pending.filter((f) => raw.includes(f.id)).map((f) => f.id) + : []; + const stillPending = pending.filter((f) => !confirmedIds.includes(f.id)); + if (stillPending.length > 0) { + return { blocked: true, posture, pending: stillPending }; + } + + return { + blocked: false, + posture, + pending: [], + attestation: recordGlobalUniqueAttestation(carried, confirmedIds, posture, installerId), + }; + }; + /** * Detect whether `manifestId` is already known to the kernel and classify * the source so we can refuse vs upgrade gracefully. diff --git a/packages/cloud-connection/src/marketplace-install-local-posture-gate.test.ts b/packages/cloud-connection/src/marketplace-install-local-posture-gate.test.ts new file mode 100644 index 0000000000..2a78f46314 --- /dev/null +++ b/packages/cloud-connection/src/marketplace-install-local-posture-gate.test.ts @@ -0,0 +1,339 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0120 D5e] The `isolated`-posture install gate for installation-wide + * (`'global'`) uniques, exercised at the real install seam. + * + * What is being pinned, and why each half matters: + * + * - **It stops, per index.** Under `isolated` an app's `'global'` unique + * constrains ACROSS CUSTOMERS and can reveal that another customer holds a + * value (ADR-0120 S10/S14). The install must not land while that is + * unanswered — an advisory nobody reads is the ADR-0049/0078 class this ADR + * exists to close. + * - **It leaves nothing behind.** A stopped install writes no ledger entry and + * registers nothing, so the installer can rewrite the metadata to + * `'organization'` and retry without an uninstall. + * - **It never re-asks.** The confirmation lands in the install manifest + * (ADR-0104 attestation style), and a reinstall/upgrade reads it back. This is + * what keeps the ceremony one-time — and what keeps it OUT of the boot path + * (#4884: never a startup warning). + * - **It is posture-scoped.** Under `single` / `group` the same manifest + * installs silently: there `'global'` means the installation, which under + * `group` IS the customer company. No finding is a decision point. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { MarketplaceInstallLocalPlugin } from './marketplace-install-local-plugin.js'; +import { LocalManifestSource } from './local-manifest-source.js'; + +type Handler = (c: any) => Promise; + +function makeRawApp() { + const routes = new Map(); + return { + routes, + get: (p: string, h: Handler) => routes.set(`GET ${p}`, h), + post: (p: string, h: Handler) => routes.set(`POST ${p}`, h), + delete: (p: string, h: Handler) => routes.set(`DELETE ${p}`, h), + }; +} + +function makeCtx(rawApp: any, services: Record) { + const hooks = new Map(); + return { + ctx: { + hook: (e: string, h: any) => hooks.set(e, h), + getService: (name: string) => { + if (name === 'http-server') return { getRawApp: () => rawApp }; + const svc = services[name]; + if (svc === undefined) throw new Error(`no ${name}`); + return svc; + }, + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + }, + fire: async () => { await hooks.get('kernel:ready')?.(); }, + }; +} + +function makeC(body: any) { + const json = vi.fn((payload: any, status?: number) => ({ payload, status: status ?? 200 })); + return { + req: { + url: 'http://localhost:3000/api/v1/marketplace/install-local', + raw: new Request('http://localhost:3000/x'), + json: async () => body, + param: () => undefined, + }, + json, + }; +} + +/** An app carrying BOTH `'global'` spellings on its own objects, plus the + * per-organization and platform-owned shapes the gate must ignore. */ +const APP_WITH_GLOBAL_UNIQUES = { + id: 'com.acme.mrp', + namespace: 'mrp', + version: '1.0.0', + objects: [ + { + name: 'material', + fields: { + // S14: "unique across the whole company" — right under `group`, + // crosses customers under `isolated`. THE decision point. + code: { type: 'text', unique: 'global' }, + // per-organization — correct under every posture, never asked. + name: { type: 'text', unique: true }, + }, + indexes: [ + // the deprecated bare spelling: same physical shape, same hazard + { name: 'uk_material_ext', fields: ['external_id'], unique: true }, + // per-organization declared index — never asked + { fields: ['plant', 'batch'], unique: 'organization' }, + ], + }, + { + // platform-owned: the S5 engine idempotency inventory, platform-wide + // by construction — never a decision point. + name: 'sys_job', + fields: {}, + indexes: [{ fields: ['name'], unique: true }], + }, + ], +}; + +const EXPECTED_IDS = ['material:field:code', 'material:index:external_id']; + +let dir: string; +let posture: string | undefined; + +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'mil-d5e-')); + posture = process.env.OS_TENANCY_POSTURE; +}); +afterEach(() => { + rmSync(dir, { recursive: true, force: true }); + if (posture === undefined) delete process.env.OS_TENANCY_POSTURE; + else process.env.OS_TENANCY_POSTURE = posture; + vi.restoreAllMocks(); +}); + +async function mountInstall(storageDir: string) { + const register = vi.fn(); + const syncSchemas = vi.fn(async () => undefined); + const rawApp = makeRawApp(); + const { ctx, fire } = makeCtx(rawApp, { + manifest: { register }, + auth: { api: { getSession: async () => ({ user: { id: 'usr_installer' } }) } }, + objectql: { syncSchemas }, + }); + const plugin = new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir }); + await plugin.start(ctx as any); + await fire(); + const install = rawApp.routes.get('POST /api/v1/marketplace/install-local')!; + return { install, register, syncSchemas, ctx }; +} + +describe("ADR-0120 D5e — install into an 'isolated' environment", () => { + it('HARD-STOPS and lists each installation-wide unique', async () => { + process.env.OS_TENANCY_POSTURE = 'isolated'; + const { install } = await mountInstall(dir); + + const res = await install(makeC({ manifest: APP_WITH_GLOBAL_UNIQUES })); + + expect(res.status).toBe(409); + expect(res.payload.success).toBe(false); + expect(res.payload.error.code).toBe('UNIQUE_SCOPE_CONFIRMATION_REQUIRED'); + // Per-index listing — the ceremony is per constraint, not per app. + expect(res.payload.error.details.findings.map((f: any) => f.id)).toEqual(EXPECTED_IDS); + expect(res.payload.error.message).toContain('material.code — field-level'); + expect(res.payload.error.message).toContain("material — declared index 'uk_material_ext' [external_id]"); + expect(res.payload.error.message).toContain("rewrite it to `unique: 'organization'`"); + // The per-organization declarations and the sys_ object are NOT listed: + // asking about those on every install is the #4884 false-alarm class. + expect(res.payload.error.message).not.toContain('sys_job'); + expect(res.payload.error.message).not.toContain('plant'); + }); + + it('a stopped install registers nothing and writes no ledger entry', async () => { + process.env.OS_TENANCY_POSTURE = 'isolated'; + const { install, register, syncSchemas } = await mountInstall(dir); + register.mockClear(); + + await install(makeC({ manifest: APP_WITH_GLOBAL_UNIQUES })); + + expect(register).not.toHaveBeenCalled(); + expect(syncSchemas).not.toHaveBeenCalled(); + expect(new LocalManifestSource(dir).read('com.acme.mrp')).toBeNull(); + }); + + it('confirming records the attestation in the install manifest and installs', async () => { + process.env.OS_TENANCY_POSTURE = 'isolated'; + const { install, register } = await mountInstall(dir); + + const res = await install(makeC({ manifest: APP_WITH_GLOBAL_UNIQUES, confirmGlobalUniques: true })); + + expect(res.payload.success).toBe(true); + expect(register).toHaveBeenCalled(); + const entry = new LocalManifestSource(dir).read('com.acme.mrp')!; + expect(entry.globalUniqueAttestation).toMatchObject({ + posture: 'isolated', + confirmed: EXPECTED_IDS.slice().sort(), + attestedBy: 'usr_installer', + }); + expect(typeof entry.globalUniqueAttestation!.attestedAt).toBe('string'); + }); + + it('never re-asks: the reinstall of an attested package installs silently', async () => { + process.env.OS_TENANCY_POSTURE = 'isolated'; + const first = await mountInstall(dir); + await first.install(makeC({ manifest: APP_WITH_GLOBAL_UNIQUES, confirmGlobalUniques: true })); + + // Fresh plugin instance = a process restart. The record is on DISK, so + // the answer survives — a memory-only confirmation would re-ask here, + // which is the boot-time nagging #4884 forbids. + const second = await mountInstall(dir); + const res = await second.install(makeC({ manifest: APP_WITH_GLOBAL_UNIQUES })); + + expect(res.payload.success).toBe(true); + expect(new LocalManifestSource(dir).read('com.acme.mrp')!.globalUniqueAttestation!.confirmed) + .toEqual(EXPECTED_IDS.slice().sort()); + }); + + it('asks about a NEW constraint an upgrade introduces, and only that one', async () => { + process.env.OS_TENANCY_POSTURE = 'isolated'; + const first = await mountInstall(dir); + await first.install(makeC({ manifest: APP_WITH_GLOBAL_UNIQUES, confirmGlobalUniques: true })); + + const upgraded = { + ...APP_WITH_GLOBAL_UNIQUES, + version: '2.0.0', + objects: [ + ...APP_WITH_GLOBAL_UNIQUES.objects, + { name: 'plant', fields: { site_code: { type: 'text', unique: 'global' } } }, + ], + }; + const second = await mountInstall(dir); + const res = await second.install(makeC({ manifest: upgraded })); + + expect(res.status).toBe(409); + expect(res.payload.error.details.findings.map((f: any) => f.id)).toEqual(['plant:field:site_code']); + }); + + it('a PARTIAL confirmation still stops on the unconfirmed remainder', async () => { + // The per-index ceremony has teeth: echoing back one id does not + // blanket-approve a list the installer did not read. + process.env.OS_TENANCY_POSTURE = 'isolated'; + const { install } = await mountInstall(dir); + + const res = await install(makeC({ + manifest: APP_WITH_GLOBAL_UNIQUES, + confirmGlobalUniques: ['material:field:code'], + })); + + expect(res.status).toBe(409); + expect(res.payload.error.details.findings.map((f: any) => f.id)).toEqual(['material:index:external_id']); + expect(new LocalManifestSource(dir).read('com.acme.mrp')).toBeNull(); + }); + + it('an app whose uniques are all per-organization installs without ceremony', async () => { + process.env.OS_TENANCY_POSTURE = 'isolated'; + const { install } = await mountInstall(dir); + + const res = await install(makeC({ + manifest: { + id: 'com.acme.clean', + version: '1.0.0', + objects: [{ + name: 'contact', + fields: { email: { type: 'email', unique: true } }, + indexes: [{ fields: ['department', 'code'], unique: 'organization' }], + }], + }, + })); + + expect(res.payload.success).toBe(true); + expect(new LocalManifestSource(dir).read('com.acme.clean')!.globalUniqueAttestation).toBeUndefined(); + }); +}); + +describe('ADR-0120 D5e — the gate is posture-scoped', () => { + for (const p of ['single', 'group'] as const) { + it(`installs the SAME manifest with no ceremony under '${p}'`, async () => { + // Posture portability: one app package, every posture. Under + // `single` there is one customer; under `group` the installation IS + // the customer company — which is what `'global'` means there. + process.env.OS_TENANCY_POSTURE = p; + const { install } = await mountInstall(dir); + + const res = await install(makeC({ manifest: APP_WITH_GLOBAL_UNIQUES })); + + expect(res.payload.success).toBe(true); + expect(new LocalManifestSource(dir).read('com.acme.mrp')!.globalUniqueAttestation).toBeUndefined(); + }); + } + + it("an attestation given under 'single' is not consent under 'isolated'", async () => { + // The question "knowing organizations here are separate CUSTOMERS, is + // this genuinely platform-wide?" was never asked under `single`. A + // record from there answers a different question, so it must not carry. + process.env.OS_TENANCY_POSTURE = 'single'; + const first = await mountInstall(dir); + await first.install(makeC({ manifest: APP_WITH_GLOBAL_UNIQUES, confirmGlobalUniques: true })); + + process.env.OS_TENANCY_POSTURE = 'isolated'; + const second = await mountInstall(dir); + const res = await second.install(makeC({ manifest: APP_WITH_GLOBAL_UNIQUES })); + + expect(res.status).toBe(409); + expect(res.payload.error.details.findings.map((f: any) => f.id)).toEqual(EXPECTED_IDS); + }); + + it('carries an existing attestation forward when the posture is no longer gated', async () => { + process.env.OS_TENANCY_POSTURE = 'isolated'; + const first = await mountInstall(dir); + await first.install(makeC({ manifest: APP_WITH_GLOBAL_UNIQUES, confirmGlobalUniques: true })); + + // A posture flip must not silently erase the record: the confirmation + // remains a true fact about the posture it was given under, and the + // deployment may flip back. + process.env.OS_TENANCY_POSTURE = 'group'; + const second = await mountInstall(dir); + await second.install(makeC({ manifest: APP_WITH_GLOBAL_UNIQUES })); + + expect(new LocalManifestSource(dir).read('com.acme.mrp')!.globalUniqueAttestation) + .toMatchObject({ posture: 'isolated', confirmed: EXPECTED_IDS.slice().sort() }); + }); +}); + +describe('ADR-0120 D5e — the gate is NOT a boot-time check (#4884)', () => { + it('rehydrating an unattested install at kernel:ready neither stops nor warns about scope', async () => { + // A ledger entry written before the gate existed (or whose environment + // flipped to `isolated` afterwards) is EXACTLY the case the ADR routes + // to `os doctor` / `os migrate plan`. Firing here would nag on every + // boot of every such deployment forever — the #4884 false-alarm class. + const ledger = new LocalManifestSource(dir); + ledger.write({ + packageId: 'com.acme.mrp', + versionId: '1.0.0', + manifestId: 'com.acme.mrp', + version: '1.0.0', + manifest: APP_WITH_GLOBAL_UNIQUES, + installedAt: new Date().toISOString(), + installedBy: 'usr_legacy', + }); + + process.env.OS_TENANCY_POSTURE = 'isolated'; + const { ctx, register } = await mountInstall(dir); + + // Rehydrate ran during mount (kernel:ready) — the app is registered… + expect(register.mock.calls.some((call) => call[0]?.id === 'com.acme.mrp')).toBe(true); + // …and nothing complained about unique scope at boot. + const warned = (ctx.logger.warn as any).mock.calls.map((c: any[]) => String(c[0])).join('\n'); + expect(warned).not.toContain('UNIQUE_SCOPE'); + expect(warned).not.toContain('installation-wide unique'); + }); +}); diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index 8de21edadc..9c69a30f9d 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -128,7 +128,11 @@ import { lintFlowPatterns } from './lint-flow-patterns.js'; import { lintLivenessProperties } from './lint-liveness-properties.js'; import { lintAutonumberFormats } from './lint-autonumber-formats.js'; import { lintViewRefs } from './lint-view-refs.js'; -import { lintUniqueDeclarations, lintUnscopedDeclaredIndexes } from './data-model-rules.js'; +import { + lintUniqueDeclarations, + lintUnscopedDeclaredIndexes, + lintLegacyOrganizationComposites, +} from './data-model-rules.js'; type AnyRec = Record; @@ -793,6 +797,32 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ hint: f.fix ?? '', })), }, + // ADR-0120 D5c — a declared unique listing the organization column IS the + // hand-written per-organization composite (S6). Advisory nudge toward the + // `'organization'` respelling, which is also what closes its NULL hole + // (#5030). Never auto-fixed: opting in is a real D4 tightening. + { + name: 'lintLegacyOrganizationComposites', + tier: 'advisory', + input: 'parsed', + commands: ['validate', 'build'], + source: 'packages/lint/src/data-model-rules.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_OBJECT_WRITES_P2, + scopeReason: + '`os lint` already reports this rule through `lintDataModel`, which calls it directly alongside ' + + 'R10/R11 in its best-practice sweep — registering it for `lint` as well would report every finding ' + + 'twice. This is coverage recorded, not coverage missing: all three commands report the rule.', + run: (stack) => + lintLegacyOrganizationComposites(Array.isArray(stack.objects) ? (stack.objects as unknown[]) : []).map((f) => ({ + severity: f.severity === 'suggestion' ? ('info' as const) : f.severity, + rule: f.rule, + where: f.path, + path: f.path, + message: f.message, + hint: f.fix ?? '', + })), + }, // ADR-0090 D7 — the security-domain publish linter. Every `error` rule mirrors // a runtime enforcement point (fail-closed OWD default, canonical enum, anchor // binding gate, vocabulary freeze), moving the failure from a runtime deny to diff --git a/packages/lint/src/data-model-rules.ts b/packages/lint/src/data-model-rules.ts index 3dfd7bd3c4..2e76ee1a33 100644 --- a/packages/lint/src/data-model-rules.ts +++ b/packages/lint/src/data-model-rules.ts @@ -73,6 +73,21 @@ function refOf(def: any): string | undefined { export const UNIQUE_DOUBLE_DECLARATION = 'unique/double-declaration'; export const UNIQUE_UNSCOPED_DECLARED_INDEX = 'unique/unscoped-declared-index'; +export const UNIQUE_LEGACY_ORGANIZATION_COMPOSITE = 'unique/legacy-organization-composite'; + +/** + * The organization column, as an AUTHOR would have spelled it. + * + * `organization_id` is kernel-injected at registration, not authored — which is + * exactly why an author who typed it into an index's `fields` was hand-writing + * the per-organization composite the vocabulary now has a word for (ADR-0120 + * S6). An object may declare a different column via `tenancy.tenantField`; that + * spelling is honored here for the same reason. + */ +function authoredTenantColumn(obj: any): string { + const declared = obj?.tenancy?.tenantField; + return typeof declared === 'string' && declared.trim() ? declared.trim() : 'organization_id'; +} /** Is `unique` declared at all? Mirrors `isUniqueDeclared` in @objectstack/spec/data. */ function uniqueDeclared(u: unknown): boolean { @@ -256,6 +271,78 @@ export function lintUniqueDeclarations(objects: any[]): LintIssue[] { return issues; } +/** + * R12 (ADR-0120 D5c) — a declared unique index whose column list CONTAINS the + * organization column: the hand-written per-organization composite (S6), + * predating the vocabulary that can now say so. + * + * Why this is worth a nudge rather than left alone. The legacy spelling + * `{ fields: ['organization_id', 'name'], unique: true }` says "per + * organization" to a reader and materializes as a plain composite — and SQL + * UNIQUE is NULL-distinct, so on every row where the organization column is + * NULL it enforces **nothing** (#5030, measured). On a single-organization + * deployment that is *every* row. The `'organization'` respelling is what closes + * that hole: the driver makes the LISTED organization column NULL-safe in place + * (`COALESCE(organization_id, '__global__')`), so the NULL rows become one + * platform bucket that is unique among themselves. + * + * **Advisory, and deliberately no auto-fix.** ADR-0120 D5c is explicit that the + * legacy spelling stays valid and unmigrated forever if untouched — zero forced + * drift. Opting in is a real physical tightening that goes through the D4 + * ceremony (a `recreate_index` gated by the duplicate pre-flight probe), because + * the rows the void constraint admitted may still be there. Fixing this on the + * author's behalf would schedule that migration without asking. + * + * Not fired for `unique: 'organization'` — that IS the respelling — nor for a + * unique declared on the organization column ALONE, which is not a composite and + * has no per-organization reading to recover. + */ +export function lintLegacyOrganizationComposites(objects: any[]): LintIssue[] { + const issues: LintIssue[] = []; + if (!Array.isArray(objects) || objects.length === 0) return issues; + + for (let i = 0; i < objects.length; i++) { + const obj = objects[i]; + if (!obj?.name) continue; + const tenantColumn = authoredTenantColumn(obj); + const declaredIndexes = Array.isArray(obj.indexes) ? obj.indexes : []; + + for (let j = 0; j < declaredIndexes.length; j++) { + const idx = declaredIndexes[j]; + // Already the target spelling, or not a unique at all. + if (!uniqueDeclared(idx?.unique) || idx.unique === 'organization') continue; + const cols = Array.isArray(idx?.fields) + ? idx.fields.filter((f: unknown) => typeof f === 'string') + : []; + if (cols.length < 2) continue; // a lone organization column is not a composite + if (!cols.includes(tenantColumn)) continue; + + const indexLabel = typeof idx?.name === 'string' && idx.name.trim() ? ` '${idx.name.trim()}'` : ''; + const spelling = `\`unique: ${typeof idx.unique === 'string' ? `'${idx.unique}'` : idx.unique}\``; + const rest = cols.filter((c: string) => c !== tenantColumn); + issues.push({ + severity: 'warning', + rule: UNIQUE_LEGACY_ORGANIZATION_COMPOSITE, + message: + `"${obj.name}" declares index${indexLabel} [${cols.join(', ')}] with ${spelling} and lists the organization ` + + `column '${tenantColumn}' itself — the hand-written per-organization composite that predates the scope ` + + `vocabulary (ADR-0120 S6). It reads as "unique per organization" but materializes as a plain composite, and ` + + `SQL UNIQUE is NULL-distinct: on every row whose '${tenantColumn}' is NULL it enforces nothing (#5030) — which ` + + `on a single-organization deployment is every row.`, + path: `objects[${i}].indexes[${j}]`, + fix: + `State the scope instead: \`unique: 'organization'\` on this index (keep \`fields\` exactly as they are — the ` + + `driver makes the listed '${tenantColumn}' NULL-safe in place rather than prepending a second organization key ` + + `part). ${rest.length > 0 ? `The constraint then really is "one ${rest.join(' + ')} per organization". ` : ''}` + + `Opting in is a physical tightening: it surfaces as a \`recreate_index\` drift op gated by the duplicate ` + + `pre-flight probe (ADR-0120 D4), so pre-existing duplicate NULL-organization rows block it with a report ` + + `rather than failing a boot. Leaving it as-is stays valid indefinitely and forces no drift.`, + }); + } + } + return issues; +} + // ─── Rule engine ──────────────────────────────────────────────────── /** @@ -264,12 +351,13 @@ export function lintUniqueDeclarations(objects: any[]): LintIssue[] { * metadata-generation scorer. */ export function lintDataModel(objects: any[]): LintIssue[] { - // R10/R11 live in their own exported functions so `os validate`/`os build` + // R10/R11/R12 live in their own exported functions so `os validate`/`os build` // can run those rules without pulling in the whole best-practice sweep - // (#3991, ADR-0120 D5a) — here `os lint` picks both up. + // (#3991, ADR-0120 D5a/D5b/D5c) — here `os lint` picks all three up. const issues: LintIssue[] = [ ...lintUnscopedDeclaredIndexes(objects), ...lintUniqueDeclarations(objects), + ...lintLegacyOrganizationComposites(objects), ]; if (!Array.isArray(objects) || objects.length === 0) return issues; diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index fa8bfb98c8..2dc7a20039 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -462,9 +462,11 @@ export { export { lintUniqueDeclarations, lintUnscopedDeclaredIndexes, + lintLegacyOrganizationComposites, lintDataModel, UNIQUE_DOUBLE_DECLARATION, UNIQUE_UNSCOPED_DECLARED_INDEX, + UNIQUE_LEGACY_ORGANIZATION_COMPOSITE, } from './data-model-rules.js'; export type { LintIssue, Severity } from './data-model-rules.js'; diff --git a/packages/plugins/driver-sql/src/adr0120-three-posture-conformance.test.ts b/packages/plugins/driver-sql/src/adr0120-three-posture-conformance.test.ts new file mode 100644 index 0000000000..0fa6f81ace --- /dev/null +++ b/packages/plugins/driver-sql/src/adr0120-three-posture-conformance.test.ts @@ -0,0 +1,418 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { SqlDriver, classifyIndexKeyPart, parseIndexDdl } from '../src/index.js'; + +/** + * ADR-0120 §Acceptance tests — posture portability (S13/S14). + * + * ## The claim under test + * + * A metadata app is authored **once** and must run unmodified under every + * tenancy posture — `single | group | isolated` (ADR-0105 D1) — and under + * database-per-customer deployment. The vocabulary makes that possible by + * naming **business boundaries**, never postures: the author says + * `'organization'` (one holder per organization) or `'global'` (one holder + * across the whole installation), and the driver materializes the same physical + * shape everywhere. The ADR states this as an invariant with teeth: + * + * > **no index shape reads the posture, so a posture change has zero automatic + * > schema consequences.** + * + * That is what this suite pins, and it is why the suite is written at the + * DRIVER level rather than as three kernel boots: the driver is the only layer + * that materializes a unique constraint, so if the posture leaks into a shape + * at all, it leaks here. Three full kernel boots would exercise the same + * `initObjects` call through 100× the machinery and prove strictly less about + * the constraint, because a kernel boot cannot insert the violating pair. + * + * Each posture therefore boots the SAME fixture app against a fresh in-memory + * database with the environment a deployment of that posture really carries + * (`OS_TENANCY_POSTURE` **and** the `OS_MULTI_ORG_ENABLED` it reconciles with — + * the driver reads the latter for its tenant-audit warning, so setting only the + * former would leave half the posture unrepresented). Then, per posture: + * + * 1. every S-row's enforcement is asserted with a REAL violating insert, and + * 2. the materialized key parts are captured, so the three postures can be + * compared byte-for-byte against each other. + * + * Plus one smoke assertion for transitions (the ADR asks for exactly one, and + * explicitly no transition matrix): a database built under `single`, re-read + * under `isolated`, reports **zero drift ops**. + * + * ## Reading a green run honestly + * + * Assertion (2) is the load-bearing one and it is a SAMENESS assertion, which + * is the kind that can pass by producing nothing. So the shapes are also + * asserted against their expected key parts positively — a run where + * `initObjects` silently created no unique index at all would satisfy "all + * three postures agree" and fail these. + */ + +/** The postures, with the environment a real deployment of each carries. */ +const POSTURES = [ + { posture: 'single', multiOrg: undefined }, + { posture: 'group', multiOrg: 'true' }, + { posture: 'isolated', multiOrg: 'true' }, +] as const; + +/** + * ONE fixture app, covering the ADR's business-requirement matrix rows that a + * unique constraint can be observed at: authored once, deployed under all three + * postures, never edited between them. + */ +const FIXTURE_APP = [ + { + // S1 — per-organization unique field. Two organizations may each hold + // `a@b.com`; a duplicate WITHIN one may not. S8's mixed population rides + // along: NULL-organization rows form one platform bucket (D3). + name: 'crm_contact', + fields: { + organization_id: { type: 'string' }, + email: { type: 'string', unique: true }, + }, + }, + { + // S2 — platform-wide unique field. A device identity is one holder across + // the installation, in every posture. + name: 'crm_device', + fields: { + organization_id: { type: 'string' }, + serial: { type: 'string', unique: 'global' }, + }, + }, + { + // S3 — per-organization COMPOSITE, in the new spelling. The author states + // the boundary; the driver supplies the organization key part. + name: 'crm_case', + fields: { + organization_id: { type: 'string' }, + department: { type: 'string' }, + code: { type: 'string' }, + }, + indexes: [{ fields: ['department', 'code'], unique: 'organization' }], + }, + { + // S4/S5 — platform-wide composite: an engine-style dedup key, verbatim. + // These are the constraints the ADR's rejected alternative would have + // silently converted into NULL-void composites. + name: 'crm_delivery', + fields: { + organization_id: { type: 'string' }, + source: { type: 'string' }, + dedup_key: { type: 'string' }, + }, + indexes: [{ fields: ['source', 'dedup_key'], unique: 'global' }], + }, + { + // S6 — the legacy hand-written organization composite, left in the old + // spelling on purpose: it must keep its exact physical shape in every + // posture (zero forced drift). Lint D5c nudges toward the respelling; the + // driver forces nothing. + name: 'crm_team', + fields: { + organization_id: { type: 'string' }, + name: { type: 'string' }, + }, + indexes: [{ fields: ['name', 'organization_id'], unique: true }], + }, + { + // S9 — autonumber paired with a per-organization unique. Every organization + // counts from 1, so the constraint must be per-organization by construction + // or the second organization's PROD-00001 is rejected by an index it cannot + // see (the #3696 existence oracle). + name: 'crm_product', + fields: { + organization_id: { type: 'string' }, + code: { type: 'autonumber', format: 'PROD-{00000}', unique: true }, + }, + }, + { + // S11 — a tenancy-less object: `'organization'` degrades to the listed + // columns alone, exactly as field-level `true` already does. + name: 'crm_registry', + tenancy: { enabled: false }, + fields: { slug: { type: 'string' } }, + indexes: [{ fields: ['slug'], unique: 'organization' }], + }, + { + // S12 — non-unique indexes are untouched by this ADR. + name: 'crm_event', + fields: { + organization_id: { type: 'string' }, + status: { type: 'string' }, + }, + indexes: [{ fields: ['status'], unique: false }], + }, +] as const; + +/** + * The expected unique key parts, per table — the SAME map for every posture. + * `COALESCE(col)` is the literal-agnostic reading of the NULL-safe organization + * key part, matching the identity the drift differ compares on. + */ +const EXPECTED_UNIQUE_KEY_PARTS: Record> = { + crm_contact: { uniq_crm_contact_organization_id_email: ['COALESCE(organization_id)', 'email'] }, + crm_device: { uniq_crm_device_serial: ['serial'] }, + // Note the index NAME: the deterministic name is derived from the RESOLVED + // columns, so a declared `'organization'` index carries the organization + // column in its name even though the author never listed it. That is the + // differ's identity too, which is why declaring and detecting cannot drift. + crm_case: { + uniq_crm_case_organization_id_department_code: ['COALESCE(organization_id)', 'department', 'code'], + }, + crm_delivery: { uniq_crm_delivery_source_dedup_key: ['source', 'dedup_key'] }, + crm_team: { uniq_crm_team_name_organization_id: ['name', 'organization_id'] }, + crm_product: { uniq_crm_product_organization_id_code: ['COALESCE(organization_id)', 'code'] }, + crm_registry: { uniq_crm_registry_slug: ['slug'] }, +}; + +const REJECTED = /UNIQUE constraint failed|duplicate key value/; + +describe('ADR-0120 — one app package, three tenancy postures (S13/S14 acceptance)', () => { + let driver: SqlDriver; + let savedPosture: string | undefined; + let savedMultiOrg: string | undefined; + let tmpDbDir: string | undefined; + + beforeEach(() => { + savedPosture = process.env.OS_TENANCY_POSTURE; + savedMultiOrg = process.env.OS_MULTI_ORG_ENABLED; + }); + + afterEach(async () => { + await driver?.disconnect(); + if (tmpDbDir) { + rmSync(tmpDbDir, { recursive: true, force: true }); + tmpDbDir = undefined; + } + if (savedPosture === undefined) delete process.env.OS_TENANCY_POSTURE; + else process.env.OS_TENANCY_POSTURE = savedPosture; + if (savedMultiOrg === undefined) delete process.env.OS_MULTI_ORG_ENABLED; + else process.env.OS_MULTI_ORG_ENABLED = savedMultiOrg; + }); + + const makeDriver = (opts: any = {}) => + new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + ...opts, + }); + + const applyPosture = (posture: string, multiOrg: string | undefined) => { + process.env.OS_TENANCY_POSTURE = posture; + if (multiOrg === undefined) delete process.env.OS_MULTI_ORG_ENABLED; + else process.env.OS_MULTI_ORG_ENABLED = multiOrg; + }; + + /** Unique index name → canonical key parts, literal elided. */ + async function uniqueKeyParts(table: string): Promise> { + const k = (driver as any).knex; + const list: any = await k.raw(`PRAGMA index_list(${table})`); + const master: any = await k.raw( + `SELECT name, sql FROM sqlite_master WHERE type = 'index' AND tbl_name = ?`, + [table], + ); + const ddlByName = new Map(); + for (const r of Array.isArray(master) ? master : (master?.rows ?? [])) { + if (typeof r?.sql === 'string' && r.sql) ddlByName.set(r.name, r.sql); + } + const out: Record = {}; + for (const idx of list) { + if (idx.origin === 'pk' || idx.unique !== 1) continue; + const parsed = parseIndexDdl(ddlByName.get(idx.name) ?? ''); + if (parsed) { + out[idx.name] = parsed.keyParts.map((p) => { + const part = classifyIndexKeyPart(p); + if (part.kind === 'column') return part.column; + return part.column === null ? p : `COALESCE(${part.column})`; + }); + } else { + const info: any = await k.raw(`PRAGMA index_info(${idx.name})`); + out[idx.name] = info.map((c: any) => c.name); + } + } + return out; + } + + /** Every table's unique shape, for cross-posture comparison. */ + async function shapeSnapshot(): Promise>> { + const snap: Record> = {}; + for (const table of Object.keys(EXPECTED_UNIQUE_KEY_PARTS)) { + snap[table] = await uniqueKeyParts(table); + } + return snap; + } + + const shapesByPosture: Record>> = {}; + + for (const { posture, multiOrg } of POSTURES) { + describe(`posture: ${posture}`, () => { + beforeEach(async () => { + applyPosture(posture, multiOrg); + driver = makeDriver(); + await driver.initObjects(FIXTURE_APP as any); + }); + + it('materializes exactly the declared boundaries — no posture in any shape', async () => { + const snap = await shapeSnapshot(); + shapesByPosture[posture] = snap; + // Positive assertion first: a run that created NO unique index would + // otherwise satisfy the cross-posture sameness check below vacuously. + for (const [table, expected] of Object.entries(EXPECTED_UNIQUE_KEY_PARTS)) { + expect({ [table]: snap[table] }).toEqual({ [table]: expected }); + } + }); + + it('S1 — a per-organization unique field: shared across organizations, enforced within one', async () => { + await driver.create('crm_contact', { organization_id: 'org_a', email: 'a@b.com' }); + // Two organizations may each hold the value… + await expect(driver.create('crm_contact', { organization_id: 'org_b', email: 'a@b.com' })) + .resolves.toBeTruthy(); + // …and a duplicate within one may not. + await expect(driver.create('crm_contact', { organization_id: 'org_a', email: 'a@b.com' })) + .rejects.toThrow(REJECTED); + }); + + it('S7/S8 — the NULL-organization bucket is a real bucket, in every posture', async () => { + // THE #5030 headline, and the reason `'organization'` could ship at all: + // before D3 this pair inserted twice on a single-organization stack + // (SQL UNIQUE is NULL-distinct), so every field-level unique was a + // silent no-op there. It must now be enforced under `single` exactly as + // under `isolated` — the posture is not what makes a constraint real. + await driver.create('crm_contact', { email: 'platform@b.com' }); + await expect(driver.create('crm_contact', { email: 'platform@b.com' })) + .rejects.toThrow(REJECTED); + // …while an organization-owned row with the same value still fits (S8). + await expect(driver.create('crm_contact', { organization_id: 'org_a', email: 'platform@b.com' })) + .resolves.toBeTruthy(); + }); + + it('S2 — a platform-wide unique field crosses organizations, in every posture', async () => { + await driver.create('crm_device', { organization_id: 'org_a', serial: 'SN-1' }); + await expect(driver.create('crm_device', { organization_id: 'org_b', serial: 'SN-1' })) + .rejects.toThrow(REJECTED); + }); + + it("S3 — a declared `unique: 'organization'` composite is per organization", async () => { + await driver.create('crm_case', { organization_id: 'org_a', department: 'ops', code: 'C1' }); + await expect(driver.create('crm_case', { organization_id: 'org_b', department: 'ops', code: 'C1' })) + .resolves.toBeTruthy(); + await expect(driver.create('crm_case', { organization_id: 'org_a', department: 'ops', code: 'C1' })) + .rejects.toThrow(REJECTED); + // …and the NULL bucket is constrained too (D3 reaches declared indexes). + await driver.create('crm_case', { department: 'ops', code: 'C9' }); + await expect(driver.create('crm_case', { department: 'ops', code: 'C9' })) + .rejects.toThrow(REJECTED); + }); + + it('S4/S5 — a platform-wide dedup key stays platform-wide, NULL organization included', async () => { + // The nine engine idempotency keys live or die on this row: they are + // written by sudo with a NULL organization, so a composite would have + // enforced nothing on exactly the rows that matter. + await driver.create('crm_delivery', { source: 'smtp', dedup_key: 'K1' }); + await expect(driver.create('crm_delivery', { source: 'smtp', dedup_key: 'K1' })) + .rejects.toThrow(REJECTED); + await expect(driver.create('crm_delivery', { organization_id: 'org_a', source: 'smtp', dedup_key: 'K1' })) + .rejects.toThrow(REJECTED); + }); + + it('S6 — the legacy hand-written composite keeps its exact shape, unmigrated', async () => { + // Zero forced drift: the old spelling is untouched until the author + // opts in. Its NULL hole therefore SURVIVES here, and that is the + // correct, documented outcome — not a bug this suite should paper over. + await driver.create('crm_team', { organization_id: 'org_a', name: 'Core' }); + await expect(driver.create('crm_team', { organization_id: 'org_a', name: 'Core' })) + .rejects.toThrow(REJECTED); + await expect(driver.create('crm_team', { organization_id: 'org_b', name: 'Core' })) + .resolves.toBeTruthy(); + // The un-closed NULL hole, pinned as the honest status quo (D5c nudges + // toward the respelling that closes it; nothing forces it). + await driver.create('crm_team', { name: 'Platform' }); + await expect(driver.create('crm_team', { name: 'Platform' })).resolves.toBeTruthy(); + }); + + it('S9 — autonumber + per-organization unique: every organization counts from 1', async () => { + const a = await driver.create('crm_product', { organization_id: 'org_a' }); + const b = await driver.create('crm_product', { organization_id: 'org_b' }); + expect(a.code).toBe('PROD-00001'); + expect(b.code).toBe('PROD-00001'); + }); + + it("S11 — `'organization'` on a tenancy-less object degrades to the listed columns", async () => { + await driver.create('crm_registry', { slug: 'alpha' }); + await expect(driver.create('crm_registry', { slug: 'alpha' })).rejects.toThrow(REJECTED); + }); + + it('S12 — a non-unique index constrains nothing', async () => { + await driver.create('crm_event', { organization_id: 'org_a', status: 'open' }); + await expect(driver.create('crm_event', { organization_id: 'org_a', status: 'open' })) + .resolves.toBeTruthy(); + }); + + it('the app is converged on arrival — zero drift right after registration', async () => { + expect(await driver.detectManagedDrift()).toEqual([]); + }); + }); + } + + describe('posture portability', () => { + it('all three postures materialize BYTE-IDENTICAL unique shapes', async () => { + // Captured independently per posture above; compared here. This is the + // ADR's invariant stated as an equality — "no index shape reads the + // posture" is false the moment any two of these differ. + const captured: Record>> = {}; + for (const { posture, multiOrg } of POSTURES) { + applyPosture(posture, multiOrg); + await driver?.disconnect(); + driver = makeDriver(); + await driver.initObjects(FIXTURE_APP as any); + captured[posture] = await shapeSnapshot(); + } + expect(captured.group).toEqual(captured.single); + expect(captured.isolated).toEqual(captured.single); + // …and they are the DECLARED shapes, not three identical empties. + expect(captured.single).toEqual(EXPECTED_UNIQUE_KEY_PARTS); + }); + + it('a posture flip by itself emits ZERO drift ops (the transition smoke)', async () => { + // The ADR asks for exactly one transition assertion and explicitly no + // transition matrix: because no shape reads the posture, a re-platforming + // event has no automatic schema consequence. Build the database under + // `single`, then read it back under `isolated` on a driver that never saw + // the first — a real restart after a posture change. + applyPosture('single', undefined); + // A real file, not `:memory:`: the point is that a SECOND driver — a + // restart after the posture change — opens the database the first one + // built. (An on-disk temp file, not a `file:…?cache=shared` URI: + // better-sqlite3 takes the filename literally and would create a file by + // that name in the package directory.) + tmpDbDir = mkdtempSync(join(tmpdir(), 'adr0120-flip-')); + const shared = join(tmpDbDir, 'posture-flip.sqlite'); + driver = makeDriver({ connection: { filename: shared } }); + await driver.initObjects(FIXTURE_APP as any); + expect(await driver.detectManagedDrift()).toEqual([]); + const before = await shapeSnapshot(); + + applyPosture('isolated', 'true'); + const flipped = makeDriver({ connection: { filename: shared } }); + try { + await flipped.initObjects(FIXTURE_APP as any); + const drift = await flipped.detectManagedDrift(); + expect(drift).toEqual([]); + // The physical shapes the flipped runtime sees are the ones the + // `single` boot created — nothing was rebuilt behind anyone's back. + const swap = driver; + driver = flipped; + expect(await shapeSnapshot()).toEqual(before); + driver = swap; + } finally { + await flipped.disconnect(); + } + }); + }); +}); diff --git a/packages/spec/src/data/validation.zod.ts b/packages/spec/src/data/validation.zod.ts index bc6504d5f4..93e8d04f28 100644 --- a/packages/spec/src/data/validation.zod.ts +++ b/packages/spec/src/data/validation.zod.ts @@ -37,9 +37,11 @@ import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; * (they need I/O or are client-side concerns), they were removed rather than left as silent * no-ops. Use the layer that already does each one correctly: * - * - **Uniqueness** → a unique **index** (`ObjectSchema.indexes`, `{ fields, unique: true }`, - * with `partial` for a scoped/conditional constraint), or field-level `unique: true`. A - * SELECT-then-INSERT "rule" is inherently racy (TOCTOU); a DB unique constraint is not. + * - **Uniqueness** → a unique **index** whose scope is stated (`ObjectSchema.indexes`, with + * `unique: 'organization'` for one holder per organization or `unique: 'global'` for one + * across the whole installation — ADR-0120; `partial` for a scoped/conditional constraint), + * or field-level `unique`. A SELECT-then-INSERT "rule" is inherently racy (TOCTOU); a DB + * unique constraint is not. * - **Async / remote validation** → a client-form concern (`debounce`/`validatorUrl` only mean * anything against keystrokes) and an SSRF/latency hazard on the server write path. Keep it in * the form layer, or enforce the underlying invariant with a `unique` index / lifecycle hook. diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 2e0f1b996b..6329f56da3 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -8,6 +8,10 @@ export * from './error-leak.js'; export * from './keyset-walk.js'; export * from './module-not-found.js'; export * from './response-envelope.js'; +// [ADR-0120 D5e] The `isolated`-posture install gate for `'global'` uniques — +// the pure enumerator both the hard stop (install seam) and the advisories +// (`os doctor` / `os migrate plan`) read, so the three cannot drift apart. +export * from './unique-scope-install-gate.js'; // Placeholder for Kernel interface to avoid circular dependency // The actual Kernel implementation will satisfy this interface. diff --git a/packages/types/src/unique-scope-install-gate.test.ts b/packages/types/src/unique-scope-install-gate.test.ts new file mode 100644 index 0000000000..c1937ddc30 --- /dev/null +++ b/packages/types/src/unique-scope-install-gate.test.ts @@ -0,0 +1,209 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { + collectGlobalUniques, + globalUniqueFindingId, + isPlatformOwnedObject, + fieldUniqueIsGlobal, + declaredIndexUniqueIsGlobal, + unconfirmedGlobalUniques, + recordGlobalUniqueAttestation, + buildGlobalUniqueStopMessage, + postureGatesGlobalUniques, + describeGlobalUniqueFinding, + type GlobalUniqueFinding, +} from './unique-scope-install-gate.js'; + +/** + * ADR-0120 D5e — the enumeration behind the `isolated` install gate. + * + * These pin the CLASSIFICATION, which is where the gate can silently go wrong: + * a spelling wrongly excluded is a bypass (the constraint lands cross-customer + * and nobody is asked), a spelling wrongly included is the #4884 false-alarm + * class (every install of every app stops over a per-organization constraint). + */ +describe('ADR-0120 D5e — collectGlobalUniques', () => { + it('finds a field-level `unique: \'global\'` on an app object', () => { + const findings = collectGlobalUniques([ + { name: 'material', fields: { code: { type: 'text', unique: 'global' } } }, + ]); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + object: 'material', + kind: 'field', + name: 'code', + columns: ['code'], + spelling: 'global', + }); + }); + + it("finds a declared index spelled `unique: 'global'`", () => { + const findings = collectGlobalUniques([ + { name: 'account', fields: {}, indexes: [{ name: 'uk_ext', fields: ['external_id'], unique: 'global' }] }, + ]); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ object: 'account', kind: 'index', name: 'uk_ext', spelling: 'global' }); + }); + + it('ALSO finds a declared index spelled with the deprecated bare `true` — same physical shape (D1)', () => { + // The gate must not be bypassable by spelling. Bare `true` on a declared + // index IS `'global'` until protocol 18 rejects it (#5082); a gate that read + // only the explicit word would leave the identical hazard uncaught for the + // whole of 17.x. + const findings = collectGlobalUniques([ + { name: 'account', fields: {}, indexes: [{ fields: ['code'], unique: true }] }, + ]); + expect(findings).toHaveLength(1); + expect(findings[0].spelling).toBe(true); + expect(describeGlobalUniqueFinding(findings[0])).toContain("deprecated bare spelling of 'global'"); + }); + + it('does NOT flag per-organization declarations in any spelling', () => { + const findings = collectGlobalUniques([ + { + name: 'contact', + // field-level `true` is the documented synonym of `'organization'` + // (Resolved #2 — valid indefinitely, never a finding). + fields: { email: { type: 'email', unique: true }, code: { type: 'text', unique: 'organization' } }, + indexes: [{ fields: ['department', 'code'], unique: 'organization' }], + }, + ]); + expect(findings).toEqual([]); + }); + + it('does not flag non-unique indexes or undeclared uniques', () => { + const findings = collectGlobalUniques([ + { + name: 'invoice', + fields: { note: { type: 'text' }, total: { type: 'number', unique: false } }, + indexes: [{ fields: ['status'], unique: false }, { fields: ['issued_at'] }], + }, + ]); + expect(findings).toEqual([]); + }); + + it('skips platform-owned objects — the S5 engine idempotency inventory', () => { + // `sys_job.name`, `sys_notification.dedup_key`, … are platform-wide BY + // CONSTRUCTION and identical under every posture. Asking about them on each + // app install is the #4884 false-alarm class. + const findings = collectGlobalUniques([ + { name: 'sys_job', fields: {}, indexes: [{ fields: ['name'], unique: true }] }, + { name: 'sys_notification', fields: {}, indexes: [{ fields: ['dedup_key'], unique: 'global' }] }, + { name: 'base_thing', fields: { slug: { type: 'text', unique: 'global' } } }, + { name: 'app_thing', fields: { slug: { type: 'text', unique: 'global' } } }, + ]); + expect(findings.map((f) => f.object)).toEqual(['app_thing']); + }); + + it('accepts the array field shape as well as the map shape', () => { + const findings = collectGlobalUniques([ + { name: 'device', fields: [{ name: 'serial', type: 'text', unique: 'global' }] }, + ]); + expect(findings.map((f) => f.id)).toEqual([globalUniqueFindingId('device', 'field', ['serial'])]); + }); + + it('is deterministic: objects in supplied order, fields before indexes', () => { + const findings = collectGlobalUniques([ + { name: 'b_obj', fields: { x: { unique: 'global' } }, indexes: [{ fields: ['y'], unique: 'global' }] }, + { name: 'a_obj', fields: { z: { unique: 'global' } } }, + ]); + expect(findings.map((f) => f.id)).toEqual([ + 'b_obj:field:x', + 'b_obj:index:y', + 'a_obj:field:z', + ]); + }); + + it('tolerates junk input without throwing', () => { + expect(collectGlobalUniques(undefined)).toEqual([]); + expect(collectGlobalUniques(null)).toEqual([]); + expect(collectGlobalUniques([null, {}, { name: '' }, { name: 'x', fields: 'nope', indexes: 'nope' }])).toEqual([]); + expect(collectGlobalUniques([{ name: 'x', indexes: [{ unique: 'global' }] }])).toEqual([]); + }); +}); + +describe('ADR-0120 D5e — spelling predicates', () => { + it('field level: only the explicit `\'global\'` is installation-wide', () => { + expect(fieldUniqueIsGlobal('global')).toBe(true); + expect(fieldUniqueIsGlobal(true)).toBe(false); + expect(fieldUniqueIsGlobal('organization')).toBe(false); + expect(fieldUniqueIsGlobal(false)).toBe(false); + expect(fieldUniqueIsGlobal(undefined)).toBe(false); + }); + + it('declared index: `\'global\'` AND bare `true` are installation-wide', () => { + expect(declaredIndexUniqueIsGlobal('global')).toBe(true); + expect(declaredIndexUniqueIsGlobal(true)).toBe(true); + expect(declaredIndexUniqueIsGlobal('organization')).toBe(false); + expect(declaredIndexUniqueIsGlobal(false)).toBe(false); + }); + + it('platform-owned prefixes', () => { + expect(isPlatformOwnedObject('sys_job')).toBe(true); + expect(isPlatformOwnedObject('SYS_JOB')).toBe(true); + expect(isPlatformOwnedObject('base_thing')).toBe(true); + expect(isPlatformOwnedObject('system_of_record')).toBe(false); + expect(isPlatformOwnedObject('contact')).toBe(false); + expect(isPlatformOwnedObject(undefined)).toBe(false); + }); +}); + +describe('ADR-0120 D5e — posture gating', () => { + it("gates on 'isolated' only", () => { + expect(postureGatesGlobalUniques('isolated')).toBe(true); + expect(postureGatesGlobalUniques('multi')).toBe(true); // legacy alias of isolated + expect(postureGatesGlobalUniques('group')).toBe(false); + expect(postureGatesGlobalUniques('single')).toBe(false); + expect(postureGatesGlobalUniques('nonsense')).toBe(false); + }); +}); + +describe('ADR-0120 D5e — attestation (ADR-0104 style)', () => { + const findings: GlobalUniqueFinding[] = collectGlobalUniques([ + { name: 'material', fields: { code: { unique: 'global' } }, indexes: [{ fields: ['ext_id'], unique: 'global' }] }, + ]); + + it('with no attestation every finding needs an answer', () => { + expect(unconfirmedGlobalUniques(findings, null, 'isolated')).toHaveLength(2); + }); + + it('a recorded confirmation is never re-asked', () => { + const att = recordGlobalUniqueAttestation(null, findings.map((f) => f.id), 'isolated', 'usr_1', '2026-08-04T00:00:00.000Z'); + expect(unconfirmedGlobalUniques(findings, att, 'isolated')).toEqual([]); + expect(att).toMatchObject({ posture: 'isolated', attestedBy: 'usr_1', attestedAt: '2026-08-04T00:00:00.000Z' }); + }); + + it('a NEW constraint in a later version is asked about; the old answers stand', () => { + const first = recordGlobalUniqueAttestation(null, ['material:field:code'], 'isolated', 'usr_1'); + const still = unconfirmedGlobalUniques(findings, first, 'isolated'); + expect(still.map((f) => f.id)).toEqual(['material:index:ext_id']); + + const second = recordGlobalUniqueAttestation(first, ['material:index:ext_id'], 'isolated', 'usr_2'); + expect(second.confirmed).toEqual(['material:field:code', 'material:index:ext_id']); + expect(unconfirmedGlobalUniques(findings, second, 'isolated')).toEqual([]); + }); + + it('an attestation from another posture is NOT consent — the question was never asked there', () => { + const underSingle = recordGlobalUniqueAttestation(null, findings.map((f) => f.id), 'single'); + expect(unconfirmedGlobalUniques(findings, underSingle, 'isolated')).toHaveLength(2); + // …and re-recording under `isolated` replaces rather than merges. + const underIsolated = recordGlobalUniqueAttestation(underSingle, ['material:field:code'], 'isolated'); + expect(underIsolated.confirmed).toEqual(['material:field:code']); + }); +}); + +describe('ADR-0120 D5e — stop message', () => { + it('lists each index and carries the confirm/rewrite prescription', () => { + const findings = collectGlobalUniques([ + { name: 'material', fields: { code: { unique: 'global' } }, indexes: [{ name: 'uk_ext', fields: ['a', 'b'], unique: true }] }, + ]); + const msg = buildGlobalUniqueStopMessage('com.acme.mrp', findings); + expect(msg).toContain('com.acme.mrp'); + expect(msg).toContain("declares 2 installation-wide unique constraint(s)"); + expect(msg).toContain('material.code — field-level'); + expect(msg).toContain("material — declared index 'uk_ext' [a, b]"); + expect(msg).toContain("rewrite it to `unique: 'organization'`"); + expect(msg).toContain('never again for the same constraints'); + }); +}); diff --git a/packages/types/src/unique-scope-install-gate.ts b/packages/types/src/unique-scope-install-gate.ts new file mode 100644 index 0000000000..10b0e0bfa6 --- /dev/null +++ b/packages/types/src/unique-scope-install-gate.ts @@ -0,0 +1,347 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0120 D5e] The `isolated`-posture install gate for `'global'` uniqueness. + * + * ## Why a gate exists at all + * + * ADR-0120's scope vocabulary is deliberately **posture-invariant**: the author + * states a business boundary (`'organization'` = one holder per organization, + * `'global'` = one holder across the whole installation) and the same app + * package runs unmodified under every tenancy posture (ADR-0105 D1 + * `single | group | isolated`). No index shape reads the posture — a posture + * flip has zero automatic schema consequences, which is exactly what makes one + * app package serve all three. + * + * One residual survives that invariance, and only in one direction + * (ADR-0120 §Posture portability, Resolved question #4): + * + * - Under `single` / `group`, `'global'` means "the installation" — which for a + * `group` deployment IS the customer company (集团). An app business rule + * spelled `'global'` is correct there. + * - Under `isolated`, organizations are **separate customers**. The identical + * declaration now crosses customers: it over-constrains (customer B cannot + * reuse customer A's material code) and it becomes a cross-tenant existence + * oracle — the very leak #3696 closed for field-level uniques (S10). + * + * `'global'` is therefore physically posture-invariant but not *safety*-invariant, + * and the ADR's S14 row records the honest cost: "unique across the whole + * company" is not expressible in metadata alone, because it means the + * installation under `group` and one organization under `isolated`. A third, + * posture-resolved word (`'company'`) was designed and **rejected** — it is the + * one token that cannot be used without first understanding the posture + * spectrum, exactly the cognitive load an AI-authored vocabulary must not carry. + * The scenario is handled **here**, at the deployment seam, instead. + * + * ## Why a HARD stop and not an advisory + * + * Maintainer decision, 2026-08-04 (ADR-0120 Resolved #4). An advisory that + * nobody reads leaves a cross-customer constraint enforced in production — the + * ADR-0049/0078 class this whole ADR exists to close. So installing an app that + * carries `'global'` uniques on non-`sys` objects into an `isolated` environment + * **stops**, lists each index, and asks the installer (typically an AI agent) to + * either confirm it as genuinely platform-wide or rewrite it to + * `'organization'`. The confirmation is recorded in the install manifest + * (ADR-0104 attestation style) so it is **never re-asked**. + * + * ⛔ **Never a boot-time warning** (#4884 discipline). A deployment whose apps + * were installed before this gate existed, or whose posture changed after + * install, is reached by the ADVISORY form in `os doctor` / `os migrate plan` — + * the two cases a gate at the install seam structurally cannot see. Turning + * this into a startup diagnostic would fire on every boot of every deployment + * forever, which is the false-alarm class #4884 retired. + * + * ## What counts as a finding + * + * | Declaration | Finding? | Why | + * |:---|:---|:---| + * | field `unique: 'global'` | ✅ | one holder across the installation — crosses customers under `isolated` | + * | declared index `unique: 'global'` | ✅ | same boundary, spelled on the index | + * | declared index `unique: true` | ✅ | ADR-0120 D1: bare `true` **is** the deprecated positional spelling of `'global'`; identical physical shape, identical hazard. Excluding it would leave the gate bypassable by spelling for the whole of 17.x | + * | field `unique: true` / `'organization'` | ❌ | per-organization — correct under every posture | + * | declared index `unique: 'organization'` | ❌ | per-organization (D3 NULL-safe key part) | + * | anything on a `sys_*` object | ❌ | engine idempotency / dedup keys (the ADR's S5 inventory) are platform-wide **by construction**; asking about them on every install is the false-alarm class again | + * + * The enumeration is a pure projection of declared metadata — no tenancy + * inference, no database access — which is what lets the identical function + * serve the hard gate, `os doctor` and `os migrate plan`. + */ + +import { normalizeTenancyPosture, type TenancyPosture } from '@objectstack/spec/security'; + +/** Objects owned by the platform itself never raise a finding. */ +const SYS_OBJECT_PREFIXES = ['sys_', 'base_'] as const; + +/** + * Is this object platform-owned (the ADR's "`sys` objects")? + * + * The ADR scopes the gate to **non-`sys`** objects because the platform's own + * `'global'` uniques are the S5 inventory — `sys_job.name`, + * `sys_notification.dedup_key`, `http_delivery (source, dedup_key)` and the rest + * — engine idempotency keys that are platform-wide on purpose and identical + * under every posture. Re-confirming them on every app install would be the + * #4884 false-alarm class with extra steps. + * + * `base_` is included alongside `sys_`: it is the platform's other reserved + * object prefix, carrying the same "owned by the framework, not the app" + * meaning. An app object can never legitimately claim either. + */ +export function isPlatformOwnedObject(objectName: unknown): boolean { + const name = typeof objectName === 'string' ? objectName.trim().toLowerCase() : ''; + if (!name) return false; + return SYS_OBJECT_PREFIXES.some((prefix) => name.startsWith(prefix)); +} + +/** + * Does a FIELD-level `unique` value ask for the installation-wide boundary? + * + * Only the explicit `'global'` does. Bare `true` at field level is the + * documented, unambiguous synonym of `'organization'` (ADR-0120 D1 — + * "field-level bare `true` stays valid indefinitely", Resolved #2), so it is + * never a finding. + */ +export function fieldUniqueIsGlobal(unique: unknown): boolean { + return unique === 'global'; +} + +/** + * Does a DECLARED-INDEX `unique` value ask for the installation-wide boundary? + * + * `'global'` and bare `true` both do. Per ADR-0120 D1 the bare spelling **is** + * `'global'` — "today's verbatim semantics, materialized over exactly the listed + * columns" — deprecated (lint `unique/unscoped-declared-index` warns in 17.x, + * protocol 18 rejects it, #5082) but physically identical while it lasts. A gate + * that judged only the explicit word would be bypassable by writing the + * deprecated one, which is the #4986 trap wearing the gate's own uniform. + */ +export function declaredIndexUniqueIsGlobal(unique: unknown): boolean { + return unique === 'global' || unique === true; +} + +/** One installation-wide unique declaration found on an app (non-`sys`) object. */ +export interface GlobalUniqueFinding { + /** Stable identity for the attestation record — see {@link globalUniqueFindingId}. */ + readonly id: string; + /** Object (and therefore table) the declaration sits on. */ + readonly object: string; + /** Which spelling carried it. */ + readonly kind: 'field' | 'index'; + /** Field name for `kind: 'field'`; the index's declared name (when it has one) otherwise. */ + readonly name?: string; + /** The columns the constraint spans, in declaration order. */ + readonly columns: readonly string[]; + /** The exact authored value (`true` | `'global'`) — quoted back in the stop message. */ + readonly spelling: true | 'global'; +} + +/** + * Stable id for one finding, used as the attestation key. + * + * Keyed by object + kind + **columns**, deliberately NOT by the index's optional + * `name`: a declared index may be anonymous, and renaming an index does not + * change which constraint the installer confirmed. Two indexes on the same + * object spanning the same columns are the same constraint by any physical + * reading, so collapsing them is correct rather than lossy. + */ +export function globalUniqueFindingId( + objectName: string, + kind: 'field' | 'index', + columns: readonly string[], +): string { + return `${objectName}:${kind}:${columns.join('+')}`; +} + +/** Field map or field array — both authoring shapes are accepted. */ +function fieldEntriesOf(fields: unknown): Array<{ name: string; def: any }> { + if (!fields) return []; + if (Array.isArray(fields)) { + return fields + .filter((f: any) => f && f.name != null) + .map((f: any) => ({ name: String(f.name), def: f })); + } + if (typeof fields !== 'object') return []; + return Object.entries(fields as Record).map(([name, def]) => ({ name, def })); +} + +/** + * Enumerate every installation-wide unique declared on an app's non-`sys` + * objects (ADR-0120 D5e). + * + * Pure and posture-agnostic on purpose: the CALLER decides whether the posture + * makes these findings a hard stop (`isolated`, at install) or an advisory + * (`os doctor` / `os migrate plan`). Deterministic order — objects as supplied, + * fields before indexes within an object — so the stop message and the + * attestation record are reproducible across runs. + */ +export function collectGlobalUniques(objects: unknown): GlobalUniqueFinding[] { + if (!Array.isArray(objects)) return []; + const findings: GlobalUniqueFinding[] = []; + + for (const obj of objects as any[]) { + const objectName = typeof obj?.name === 'string' ? obj.name.trim() : ''; + if (!objectName) continue; + if (isPlatformOwnedObject(objectName)) continue; + + for (const { name, def } of fieldEntriesOf(obj?.fields)) { + if (!fieldUniqueIsGlobal(def?.unique)) continue; + findings.push({ + id: globalUniqueFindingId(objectName, 'field', [name]), + object: objectName, + kind: 'field', + name, + columns: [name], + spelling: 'global', + }); + } + + const declaredIndexes = Array.isArray(obj?.indexes) ? obj.indexes : []; + for (const idx of declaredIndexes as any[]) { + if (!declaredIndexUniqueIsGlobal(idx?.unique)) continue; + const columns = Array.isArray(idx?.fields) + ? idx.fields.filter((f: unknown) => typeof f === 'string').map((f: string) => f) + : []; + if (columns.length === 0) continue; + const indexName = typeof idx?.name === 'string' && idx.name.trim() ? idx.name.trim() : undefined; + findings.push({ + id: globalUniqueFindingId(objectName, 'index', columns), + object: objectName, + kind: 'index', + ...(indexName ? { name: indexName } : {}), + columns, + spelling: idx.unique === true ? true : 'global', + }); + } + } + + return findings; +} + +/** + * The attestation recorded in the install manifest once an installer has + * confirmed a set of findings as genuinely platform-wide (ADR-0104 style). + * + * Shape follows the ADR-0104 precedent rather than inventing one: the FACT + * observed (which constraint ids a human/agent affirmed), WHO affirmed it, WHEN, + * and under WHICH posture the question was asked. That last field is what keeps + * the record honest — an attestation given under `isolated` is evidence about + * `isolated`, and nothing else. + * + * Never rewritten in place: confirmations ACCUMULATE. A later install of a newer + * version that adds a new `'global'` index asks about the new one only — the + * earlier answers stand, which is the "之后不复问" half of the decision. + */ +export interface GlobalUniqueAttestation { + /** Posture the confirmation was given under. */ + readonly posture: TenancyPosture; + /** Finding ids affirmed as genuinely platform-wide. */ + readonly confirmed: readonly string[]; + /** ISO timestamp of the most recent confirmation. */ + readonly attestedAt: string; + /** Identity of the confirming installer, when the seam knows one. */ + readonly attestedBy?: string | null; +} + +/** + * Which findings still need an answer, given an existing attestation. + * + * Returns the findings NOT covered by `attestation.confirmed`. An empty result + * means the install proceeds silently — this is the mechanism behind "never + * re-asked". + * + * An attestation recorded under a DIFFERENT posture does not carry over: the + * question "is this genuinely platform-wide, knowing organizations here are + * separate customers?" was never asked. Confirmations made under `isolated` are + * the only ones that answer it, so a `single`-posture record is treated as + * absent rather than as consent — the conservative direction, and the only one + * that cannot silently admit a cross-customer constraint. + */ +export function unconfirmedGlobalUniques( + findings: readonly GlobalUniqueFinding[], + attestation: GlobalUniqueAttestation | undefined | null, + posture: TenancyPosture, +): GlobalUniqueFinding[] { + if (!attestation || attestation.posture !== posture) return [...findings]; + const confirmed = new Set(attestation.confirmed ?? []); + return findings.filter((f) => !confirmed.has(f.id)); +} + +/** + * Merge a new set of confirmations into an existing attestation. + * + * Additive by construction — see {@link GlobalUniqueAttestation}. A record from + * another posture is replaced rather than merged: its `confirmed` ids answered a + * different question. + */ +export function recordGlobalUniqueAttestation( + previous: GlobalUniqueAttestation | undefined | null, + confirmedIds: readonly string[], + posture: TenancyPosture, + attestedBy?: string | null, + now: string = new Date().toISOString(), +): GlobalUniqueAttestation { + const carried = previous && previous.posture === posture ? previous.confirmed ?? [] : []; + const merged = Array.from(new Set([...carried, ...confirmedIds])).sort(); + return { + posture, + confirmed: merged, + attestedAt: now, + ...(attestedBy !== undefined ? { attestedBy } : {}), + }; +} + +/** Render one finding the way both the hard stop and the advisory quote it. */ +export function describeGlobalUniqueFinding(finding: GlobalUniqueFinding): string { + const spelling = finding.spelling === true ? '`unique: true`' : "`unique: 'global'`"; + const deprecated = finding.spelling === true ? ' [deprecated bare spelling of \'global\']' : ''; + if (finding.kind === 'field') { + return `${finding.object}.${finding.name} — field-level ${spelling}`; + } + const label = finding.name ? ` '${finding.name}'` : ''; + return `${finding.object} — declared index${label} [${finding.columns.join(', ')}] ${spelling}${deprecated}`; +} + +/** + * The prescription every surface repeats verbatim, so the hard stop and the two + * advisories cannot drift into three different pieces of advice. + */ +export const GLOBAL_UNIQUE_ISOLATED_PRESCRIPTION = + "Under the 'isolated' posture organizations are separate CUSTOMERS, so an installation-wide unique " + + 'constrains across customers and can reveal that another customer already holds a value (ADR-0120 S10/S14). ' + + 'For each index above, either (a) confirm it is genuinely platform-wide — an infrastructure/dedup key, a DNS ' + + 'hostname, an external provider id — or (b) rewrite it to `unique: \'organization\'` so it is one holder per ' + + 'organization. See ADR-0120 §Posture portability.'; + +/** + * The full hard-stop message for an install into an `isolated` environment. + * + * Built here rather than at the install seam so the CLI, the HTTP surface and + * the tests all quote one text. + */ +export function buildGlobalUniqueStopMessage( + appLabel: string, + findings: readonly GlobalUniqueFinding[], +): string { + const lines = findings.map((f) => ` • ${describeGlobalUniqueFinding(f)}`); + return ( + `'${appLabel}' declares ${findings.length} installation-wide unique constraint(s) on its own objects, and this ` + + "environment runs the 'isolated' tenancy posture (ADR-0120 D5e):\n" + + `${lines.join('\n')}\n` + + `${GLOBAL_UNIQUE_ISOLATED_PRESCRIPTION}\n` + + 'Re-run the install with the confirmation to record it in the install manifest — it is asked once, ' + + 'never again for the same constraints.' + ); +} + +/** Error code the install seam returns when the gate stops an install. */ +export const GLOBAL_UNIQUE_CONFIRMATION_REQUIRED = 'UNIQUE_SCOPE_CONFIRMATION_REQUIRED'; + +/** + * Does this posture make `'global'` uniques a decision point at all? + * + * `isolated` only. Under `single` there is one customer; under `group` the + * installation IS the customer company, which is what `'global'` means there — + * both are the benign direction the ADR leaves to the app's install notes. + */ +export function postureGatesGlobalUniques(posture: unknown): boolean { + return normalizeTenancyPosture(posture) === 'isolated'; +} diff --git a/scripts/adr-anchors.json b/scripts/adr-anchors.json index 0bb0d2a3a0..eb0c33f305 100644 --- a/scripts/adr-anchors.json +++ b/scripts/adr-anchors.json @@ -3,133 +3,216 @@ "anchors": [ { "file": "packages/objectql/src/lifecycle/lifecycle-service.ts", - "adrs": ["ADR-0057"], + "adrs": [ + "ADR-0057" + ], "invariant": "ADR-0057 P4 settings overrides are bounded from below by consumer-registered retention floors (§3.3 amendment, #5195). An override under a floor — global or tenant-scoped — is REJECTED, never clamped to the floor: the declared window is the one fallback, the same resolution an unparseable override already gets, and a clamp would enforce a third number written in neither the declaration nor the settings. Floors are runtime wiring, not spec surface, for the reap-guard reason plus one of their own — the first floor IS `DbQueueAdapterOptions.idempotencyWindowMs`, a per-kernel construction option a static `lifecycle` key could only copy and drift from. The rejection is `error`-level with consequence and fix because what it prevents (duplicate delivery on a reaped dedup row) leaves the system looking healthy." }, { "file": "packages/objectql/src/validation/rule-validator.ts", - "adrs": ["ADR-0057", "ADR-0058"], + "adrs": [ + "ADR-0057", + "ADR-0058" + ], "invariant": "A declared field lock is the SERVER's to enforce (ADR-0057 D10 — the client grid is courtesy). A `readonlyWhen` whose predicate names a scope root the write path could not bind resolves to LOCKED, not to \"not locked\": \"could not check\" must never read as \"allowed\" on a field the author declared frozen. This narrows ADR-0058 D5's fail-soft tier deliberately and only for that case — a merely BROKEN predicate (undeclared key, null overload, parse fault, throw) still fails open, and requiredWhen / option visibleWhen are untouched." }, { "file": "packages/objectql/src/hook-wrappers.ts", - "adrs": ["ADR-0058", "ADR-0112"], + "adrs": [ + "ADR-0058", + "ADR-0112" + ], "invariant": "An unevaluable hook `condition` ABORTS the operation (ADR-0058's write-path addendum, #4775) — `onError` never sees it and cannot soften it back into a silent skip. On a predicate (`multi: true`) bulk write the rejection must name the LIMITATION, not the author. Since #5038 that rejection is scoped to the BATCH dispatch — the `before*` phase, which fires once for N rows because it may still rewrite the shared payload; after-hooks are dispatched per row with `previous` bound (ADR-0058's bulk-write addendum), so they never reach it. The message must therefore name the phase as the reason and point at the matching `after*` event, and must NOT promise an expiry that already happened. The discriminator is `limitation`, deliberately NOT `code` — ADR-0112 makes `error.code` a closed wire vocabulary and the REST layer promotes a thrown error's `.code` onto the envelope, so naming it `code` would mint an unregistered wire code by side effect." }, { "file": "packages/spec/src/identity/membership-role.ts", - "adrs": ["ADR-0090", "ADR-0108"], + "adrs": [ + "ADR-0090", + "ADR-0108" + ], "invariant": "The membership-role vocabulary is CLOSED and framework-owned (owner/admin/delegated_admin/member). Capability travels through positions, never through `sys_member.role`." }, { "file": "packages/plugins/plugin-auth/src/auth-manager.ts", - "adrs": ["ADR-0108"], + "adrs": [ + "ADR-0108" + ], "invariant": "better-auth's organization roles map registers the closed framework vocabulary ONLY. App-declared `position` / `permission` names are not organization roles — registering one makes it storable in `sys_member.role`, which `resolve-authz-context` projects into `current_user.positions`." }, { "file": "packages/plugins/plugin-auth/src/auth-plugin.ts", - "adrs": ["ADR-0108"], + "adrs": [ + "ADR-0108" + ], "invariant": "Nothing widens the `sys_member.role` / `sys_invitation.role` selects at boot, and there is no organization-role derivation hook. The objects are registered as authored." }, { "file": "packages/platform-objects/src/identity/sys-member.object.ts", - "adrs": ["ADR-0108"], + "adrs": [ + "ADR-0108" + ], "invariant": "`role` is the closed four-name select. It is enforced on write, and that is the guardrail making an ungoverned capability grant unrepresentable — not a limitation to work around." }, { "file": "packages/platform-objects/src/identity/sys-invitation.object.ts", - "adrs": ["ADR-0105", "ADR-0108"], + "adrs": [ + "ADR-0105", + "ADR-0108" + ], "invariant": "`role` mirrors `sys_member.role` (the value that lands there on acceptance). Capability rides in the `positions` placement field, authorized against the issuer's adminScope." }, { "file": "packages/core/src/security/resolve-authz-context.ts", - "adrs": ["ADR-0068", "ADR-0095"], + "adrs": [ + "ADR-0068", + "ADR-0095" + ], "invariant": "No enforcement-time code path may consult the better-auth role directly — `mapMembershipRole` is a grant-PROVISIONING concern here; posture and adjudication run off the resulting capability grants (ADR-0095 D3). `platform_admin` is DERIVED from an unscoped `admin_full_access` user grant — there is no trusted stored boolean (ADR-0068 D2)." }, { "file": "packages/plugins/plugin-security/src/delegated-admin-gate.ts", - "adrs": ["ADR-0090"], + "adrs": [ + "ADR-0090" + ], "invariant": "Administration is a scoped capability (D12). Writes to the RBAC link tables and `sys_member` are governed: subtree anchoring, assignable-set allowlist, strict containment." }, { "file": "packages/plugins/plugin-auth/src/invitation-role-cap.ts", - "adrs": ["ADR-0105"], + "adrs": [ + "ADR-0105" + ], "invariant": "An invitation may add a person, never authority above the issuer's own grade. An issuer below admin grade may invite as plain `member` only." }, { "file": "packages/core/src/security/posture-ladder.ts", - "adrs": ["ADR-0095"], + "adrs": [ + "ADR-0095" + ], "invariant": "Posture DERIVES from held capability grants, never from a better-auth role (D3) — re-reading the role here reopens the #2836 dual-track class. The ladder is monotonic: each rung's visible set contains the rung below's, one rung maps to exactly one injection rule, and EXTERNAL is deny-by-default (explicit shares only; OWD never widens it)." }, { "file": "packages/core/src/security/grant-validity.ts", - "adrs": ["ADR-0091"], + "adrs": [ + "ADR-0091" + ], "invariant": "A grant row outside its validity window must not resolve, and correctness lives at RESOLUTION time — never in a cleanup job (ADR-0049: no unenforced security properties). Half-open `[from, until)` in UTC; absent bounds are unbounded; a present-but-unparseable bound disables the grant, fail closed." }, { "file": "packages/plugins/plugin-security/src/tenant-layer.ts", - "adrs": ["ADR-0095"], + "adrs": [ + "ADR-0095" + ], "invariant": "Layer 0 (the tenant wall) is an independent, always-first, AND-composed filter sharing no compiler, no merge step and no bypass bit with business RLS (D1). Merging it into the RLS compiler would let a Layer-1 change weaken tenant isolation and the superuser business-RLS bypass cross the wall." }, { "file": "packages/plugins/plugin-security/src/auto-org-admin-grant.ts", - "adrs": ["ADR-0105"], + "adrs": [ + "ADR-0105" + ], "invariant": "The one provisioning bridge from an admin-grade membership to the `organization_admin` capability grant — idempotent, and revoked on demotion. Under a wall-less posture it hands out `organization_admin_no_bypass` instead: blanket record visibility must be granted deliberately, never as a side effect of a membership role (the ADR-0105 F2 finding)." }, { "file": "packages/plugins/plugin-security/src/invitation-placement.ts", - "adrs": ["ADR-0105"], + "adrs": [ + "ADR-0105" + ], "invariant": "Placement intent is authorized at ISSUANCE by dry-running `DelegatedAdminGate` against the very `sys_user_position` rows acceptance would write — an invitation can never place what its issuer could not have assigned directly. Acceptance runs under system context, so skipping the issuance check ('acceptance re-checks anyway') would be an escalation hole, not a simplification." }, { "file": "packages/spec/src/identity/position.zod.ts", - "adrs": ["ADR-0090"], + "adrs": [ + "ADR-0090" + ], "invariant": "Positions are deliberately FLAT — no `parent`, no hierarchy (D3). Visibility rollup lives on the business-unit tree; adding a second tree here is the mistake ADR-0057 D5 retired and D3 bans, and the word 'role' is reserved-forbidden in this vocabulary." }, { "file": "packages/plugins/plugin-security/src/permission-evaluator.ts", - "adrs": ["ADR-0057", "ADR-0066"], + "adrs": [ + "ADR-0057", + "ADR-0066" + ], "invariant": "The superuser bypass derives solely from a resolved set carrying the `objects['*']` wildcard with `viewAllRecords`/`modifyAllRecords` (ADR-0066 D2) — no stored boolean, no role fast-path. Scope depth (own/unit/unit_and_below/org) resolves to an effective access depth per operation class (ADR-0057 D1); an unenforceable depth is a compile error, never silent fail-open (ADR-0049)." }, { "file": "packages/spec/src/contracts/objectql-engine.ts", - "adrs": ["ADR-0119"], + "adrs": [ + "ADR-0119" + ], "invariant": "`transaction` is DECLARED on the `objectql` slot contract — plugin space reaches ADR-0034's ambient transaction by name, not through `as unknown as` casts. Required, not optional, per this file's own rule. Its two caveats (default-driver only; the callback runs with NO transaction when the driver lacks `beginTransaction`) are part of the declared meaning, so a caller that cannot lose atomicity silently must fail closed rather than assume it held." }, { "file": "packages/metadata-protocol/src/protocol.ts", - "adrs": ["ADR-0119"], + "adrs": [ + "ADR-0119" + ], "invariant": "`batchData`'s `atomic` is REAL or REFUSED, never silent best-effort. An explicitly atomic batch runs inside ONE `engine.transaction()`, so a failure rolls back every prior write and the response reports zero successes (rows marked ROLLED_BACK / NOT_ATTEMPTED — reporting a rolled-back row as `success: true` was the original bug, not merely the missing transaction). A runtime that cannot roll back gets 501 NOT_IMPLEMENTED; degrading to best-effort is how the flag came to lie." }, { "file": "packages/spec/src/kernel/functional-completeness.ts", - "adrs": ["ADR-0078"], + "adrs": [ + "ADR-0078" + ], "invariant": "Every rule here cites the runtime line that silently skips the instance, and every deliberate NON-rule cites the evidence that exempts it (ADR-0078 §6). `multiselect` without `options` is NOT flagged — `record-validator.ts` blesses it verbatim as free-form tags, which is §1 case (3) genuinely-optional; `user` relationships and `timeline`/`tree` views are exempt for their own stated reasons. A rule added without its skip-site citation, or an exemption 'fixed', is a false prescription: it tells an AI author to change working metadata, which is the failure this gate exists to prevent." }, { "file": "packages/spec/src/data/field.zod.ts", - "adrs": ["ADR-0120"], + "adrs": [ + "ADR-0120" + ], "invariant": "`UniqueScopeSchema` is the closed scope vocabulary `boolean | 'global' | 'organization'` (ADR-0120 D1). `'organization'` is the explicit synonym of field-level `true` — identical materialization, and `isUniqueDeclared` MUST count it (a word the vocabulary accepts but no driver reads is declarable-but-inert, ADR-0078). `'tenant'` and `'org'` are rejected, never aliased — the parse error names `'organization'` (§Terminology; PD #12: one contract, no dialects). The single-tenant story is D3's NULL-safe COALESCE bucket, NOT the retired 'composite degenerates to single-column' claim #5030 falsified." }, { "file": "packages/spec/src/data/object.zod.ts", - "adrs": ["ADR-0120"], + "adrs": [ + "ADR-0120" + ], "invariant": "`IndexSchema.unique` scope contract (ADR-0120 D1, amending #3696): `'global'` = verbatim columns (no organization column injected); `'organization'` = the driver prepends the NULL-safe organization key part (COALESCE(organization_id, '__global__')) at registration; bare `true` = deprecated positional spelling of `'global'` — warned in 17.x (lint unique/unscoped-declared-index), rejected at protocol 18 (#5082). Do NOT re-broaden the describe() back to 'true and global are synonyms, list the tenant column yourself' — that is the #4986 trap, and the hand-written composite is not NULL-safe (#5030)." }, { "file": "packages/lint/src/data-model-rules.ts", - "adrs": ["ADR-0120"], - "invariant": "The uniqueness rules judge SPELLINGS only, never inferred tenancy or posture (authoring-time tenancy inference is impossible — `organization_id` is kernel-injected; the dead end is documented on #4698). `unique/unscoped-declared-index` (D5a) fires on bare declared `unique: true` — warning in 17.x, the protocol-18 gate rejects the spelling (#5082). `unique/double-declaration` (D5b) is the four-quadrant scope matrix: cross-scope = contradiction (the installation-wide side wins physically), same-scope = redundancy. Fix texts speak the `'organization'`/`'global'` vocabulary — never resurrect the hand-written `['organization_id', …]` advice (not NULL-safe, #5030)." + "adrs": [ + "ADR-0120" + ], + "invariant": "The uniqueness rules judge SPELLINGS only, never inferred tenancy or posture (authoring-time tenancy inference is impossible — `organization_id` is kernel-injected; the dead end is documented on #4698). `unique/unscoped-declared-index` (D5a) fires on bare declared `unique: true` — warning in 17.x, the protocol-18 gate rejects the spelling (#5082). `unique/double-declaration` (D5b) is the four-quadrant scope matrix: cross-scope = contradiction (the installation-wide side wins physically), same-scope = redundancy. Fix texts speak the `'organization'`/`'global'` vocabulary — never resurrect the hand-written `['organization_id', …]` advice (not NULL-safe, #5030). `unique/legacy-organization-composite` (D5c) is the S6 respelling NUDGE and stays ADVISORY forever: the legacy hand-written composite is valid indefinitely and forces ZERO drift, so this rule must never gain an auto-fix or an `error` severity — opting in is a physical tightening that goes through the D4 duplicate pre-flight." }, { "file": "packages/plugins/driver-sql/src/schema-drift.ts", - "adrs": ["ADR-0120"], + "adrs": [ + "ADR-0120" + ], "invariant": "The organization key part of every organization-scoped unique index is the NULL-safe COALESCE form — `COALESCE(, '__global__')` — never the bare column: SQL UNIQUE is NULL-distinct, so the bare composite enforces NOTHING on NULL-organization rows, which on a single-tenant stack is every row (#5030). Declared-index `unique: 'global'`/bare `true` stays VERBATIM (the #3696 contract, now the 'global' arm of the vocabulary); `'organization'` prepends the key part at registration. Expected and physical sides compare through the SAME normalization, literal-agnostic on the COALESCE literal — two spellings of the literal are one constraint, never drift (#4884)." }, { "file": "packages/plugins/driver-sql/src/sql-driver.ts", - "adrs": ["ADR-0120"], + "adrs": [ + "ADR-0120" + ], "invariant": "The bare-composite → NULL-safe tightening migrates through the ceremony (ADR-0120 D4): a `recreate_index` gated by the duplicate pre-flight probe — clean data grades it `safe` (dev autoMigrate may apply), duplicates BLOCK it with a row report and the old index stays in place; apply re-probes, so even --allow-destructive cannot drop a constraint whose replacement is not creatable. Storage stays NULL — GLOBAL_TENANT is an index-key fold, never written to the organization column." + }, + { + "file": "packages/types/src/unique-scope-install-gate.ts", + "adrs": [ + "ADR-0120", + "ADR-0105", + "ADR-0104" + ], + "invariant": "The ADR-0120 D5e enumeration is PURE and posture-agnostic — the caller decides hard stop (install into `isolated`) vs advisory (`os doctor` / `os migrate plan`), which is what keeps those three surfaces from drifting into three different verdicts. Three classifications are load-bearing and must not be 'simplified': a declared index's bare `unique: true` COUNTS (D1 makes it the positional spelling of `'global'` — excluding it would make the gate bypassable by spelling for all of 17.x); field-level `true` does NOT (it means `'organization'`, valid indefinitely, Resolved #2); `sys_`/`base_` objects do NOT (the S5 engine idempotency inventory is platform-wide by construction — re-asking per install is the #4884 false-alarm class). An attestation recorded under another posture is NOT consent: `unconfirmedGlobalUniques` treats it as absent, because the `isolated` question was never asked there. ⛔ This must never be wired into a boot-time diagnostic (#4884)." + }, + { + "file": "packages/cloud-connection/src/marketplace-install-local-plugin.ts", + "adrs": [ + "ADR-0120", + "ADR-0104" + ], + "invariant": "The ADR-0120 D5e gate runs at the INSTALL seam only, BEFORE hot-register and before any ledger write — a stopped install must leave the runtime exactly as it found it so the installer can respell to `'organization'` and retry without an uninstall. The confirmation is recorded in the install manifest (`InstalledManifestEntry.globalUniqueAttestation`, ADR-0104 attestation style: fact + who + when + under which posture) and CARRIED ACROSS reinstalls, which is the whole '之后不复问' half of the decision — moving it to memory would re-ask on every process boot, i.e. exactly the #4884 boot-time nagging the ADR forbids. Confirmations accumulate; a partial confirmation still stops on the remainder. Rehydrate at `kernel:ready` never evaluates the gate." + }, + { + "file": "packages/plugins/driver-sql/src/adr0120-three-posture-conformance.test.ts", + "adrs": [ + "ADR-0120", + "ADR-0105" + ], + "invariant": "ADR-0120's posture-portability acceptance test: ONE fixture app, booted under `single | group | isolated`, must materialize BYTE-IDENTICAL unique shapes — 'no index shape reads the posture' is false the moment any two differ, and a posture flip must emit zero drift ops. The cross-posture comparison is a sameness assertion, so it is paired with a positive assertion against EXPECTED_UNIQUE_KEY_PARTS: three identically-empty runs must not read as agreement. S6's un-closed NULL hole is pinned as the honest status quo (zero forced drift), not papered over." } ] } diff --git a/skills/objectstack-data/SKILL.md b/skills/objectstack-data/SKILL.md index c077881bff..f9d3d708f8 100644 --- a/skills/objectstack-data/SKILL.md +++ b/skills/objectstack-data/SKILL.md @@ -371,8 +371,9 @@ The **complete** set of validation types (`ValidationRuleSchema` discriminators) - `conditional` — Apply a nested rule only `when` a predicate holds > **There is NO `unique` validation type** (removed from the spec in #1475). -> Enforce uniqueness — including composite — with a **unique index**: -> `indexes: [{ fields: ['tenant_id', 'email'], unique: true }]`. +> Enforce uniqueness — including composite — with a **unique index**, and state +> its scope (ADR-0120): +> `indexes: [{ fields: ['department', 'email'], unique: 'organization' }]`. See [rules/validation.md](./rules/validation.md) for all types and examples. @@ -382,12 +383,20 @@ See [rules/validation.md](./rules/validation.md) for all types and examples. ```typescript indexes: [ - { fields: ['status', 'created_at'] }, // btree (default) - { fields: ['email'], unique: true }, // btree + unique - { fields: ['description'], type: 'fulltext' }, // non-default type + { fields: ['status', 'created_at'] }, // btree (default) + { fields: ['email'], unique: 'organization' }, // unique per organization + { fields: ['hostname'], unique: 'global' }, // unique platform-wide + { fields: ['description'], type: 'fulltext' }, // non-default type ] ``` +> **A unique index must state its scope** — `'organization'` (one holder per +> organization, NULL-safe) or `'global'` (one holder across the installation). +> On a declared index bare `unique: true` is the deprecated spelling of +> `'global'`: it reads like "per organization" and does the opposite, so `os lint` +> warns and protocol 18 rejects it. On a FIELD, `unique: true` means +> `'organization'` and stays valid. + See [rules/indexing.md](./rules/indexing.md) for composite/partial/gin/gist indexes. ### Lifecycle Hooks diff --git a/skills/objectstack-data/rules/field-types.md b/skills/objectstack-data/rules/field-types.md index f57bf133f3..b6a503b0b3 100644 --- a/skills/objectstack-data/rules/field-types.md +++ b/skills/objectstack-data/rules/field-types.md @@ -215,7 +215,10 @@ What kind of data? { type: 'email', required: true, - unique: true, + // State the scope (ADR-0120): 'organization' = one holder per organization + // (NULL-safe), 'global' = one across the whole installation. Bare `true` at + // field level still means 'organization' and stays valid. + unique: 'organization', } ``` diff --git a/skills/objectstack-data/rules/indexing.md b/skills/objectstack-data/rules/indexing.md index a439c06c9e..6e77dd9aed 100644 --- a/skills/objectstack-data/rules/indexing.md +++ b/skills/objectstack-data/rules/indexing.md @@ -25,14 +25,51 @@ ObjectStack automatically creates indexes for: ```typescript indexes: [ - { fields: ['status', 'created_at'] }, // btree (default) - { fields: ['email'], unique: true }, // btree + unique - { fields: ['description'], type: 'fulltext' }, // non-default type - { fields: ['tags'], type: 'gin' }, // non-default type - { fields: ['location'], type: 'gist' }, // non-default type + { fields: ['status', 'created_at'] }, // btree (default) + { fields: ['email'], unique: 'organization' }, // btree + unique per org + { fields: ['hostname'], unique: 'global' }, // btree + unique platform-wide + { fields: ['description'], type: 'fulltext' }, // non-default type + { fields: ['tags'], type: 'gin' }, // non-default type + { fields: ['location'], type: 'gist' }, // non-default type ] ``` +## Unique scope — always state it (ADR-0120) + +A unique index must say **which boundary** the value is unique within. There are +exactly two, and the same words work on a field and on a declared index: + +| Scope | Meaning | Materializes as | +|-------|---------|-----------------| +| `unique: 'organization'` | One holder **per organization** | `(COALESCE(organization_id, '__global__'), …fields)` | +| `unique: 'global'` | One holder across the **whole installation** | exactly the listed columns | + +```typescript +// ✅ per organization — do NOT list organization_id yourself +{ fields: ['department', 'code'], unique: 'organization' } + +// ✅ platform-wide — a hostname, an external id, an engine dedup key +{ fields: ['source', 'dedup_key'], unique: 'global' } + +// ❌ scope unstated — this is the DEPRECATED spelling of 'global'. +// It reads like "per organization" and does the opposite. +// `os lint` reports unique/unscoped-declared-index; protocol 18 rejects it. +{ fields: ['code'], unique: true } +``` + +Notes an author has to know: + +- **`'organization'` is NULL-safe.** Rows with no organization — and *every* row + on a single-organization deployment — form one platform bucket that is unique + among itself. A plain `(organization_id, x)` composite enforces nothing there, + because SQL `UNIQUE` treats every `NULL` as distinct. +- **On a FIELD, `unique: true` means `'organization'`** and stays valid forever; + `'organization'` is just the preferred spelling in new code. Only on a + *declared index* is bare `true` deprecated. +- **You never write the posture.** The same declaration is correct under every + tenancy posture — state the business boundary, not the deployment shape. +- **`'tenant'` and `'org'` are rejected.** The word is `'organization'`. + ## When to Add Indexes ### ✅ Always Index @@ -78,11 +115,15 @@ indexes: [ ```typescript indexes: [ - // Single column uniqueness - { fields: ['email'], unique: true }, + // Single column, one holder per organization + { fields: ['email'], unique: 'organization' }, + + // Composite, one holder per organization — the organization key part is + // supplied by the driver; do not list organization_id yourself + { fields: ['department', 'username'], unique: 'organization' }, - // Composite uniqueness - { fields: ['tenant_id', 'username'], unique: true }, + // Single column, one holder across the whole installation + { fields: ['hostname'], unique: 'global' }, ] ``` @@ -99,7 +140,7 @@ indexes: [ // Only index non-deleted records { fields: ['email'], - unique: true, + unique: 'organization', partial: "deleted_at IS NULL", }, ] @@ -152,7 +193,7 @@ indexes: [ ```typescript indexes: [ { fields: ['status'], type: 'btree', unique: false }, // ❌ Redundant defaults - { fields: ['email'], type: 'btree', unique: true }, // ❌ Redundant type + { fields: ['email'], type: 'btree', unique: 'organization' }, // ❌ Redundant type ] ``` @@ -161,7 +202,7 @@ indexes: [ ```typescript indexes: [ { fields: ['status'] }, // ✅ btree and unique: false are defaults - { fields: ['email'], unique: true }, // ✅ btree is default, only specify unique + { fields: ['email'], unique: 'organization' }, // ✅ btree is default; the scope is required ] ``` @@ -260,7 +301,7 @@ Use partial indexes to index only a subset of rows: // Only index non-deleted records { fields: ['email'], - unique: true, + unique: 'organization', partial: "deleted_at IS NULL", } ``` diff --git a/skills/objectstack-data/rules/relationships.md b/skills/objectstack-data/rules/relationships.md index a6bb227277..10ad74cae4 100644 --- a/skills/objectstack-data/rules/relationships.md +++ b/skills/objectstack-data/rules/relationships.md @@ -127,7 +127,7 @@ export default ObjectSchema.create({ indexes: [ // One assignment per (project, employee) pair — uniqueness is an index // concern; there is no 'unique' validation type. - { fields: ['project_id', 'employee_id'], unique: true }, + { fields: ['project_id', 'employee_id'], unique: 'organization' }, ], }); ``` diff --git a/skills/objectstack-data/rules/validation.md b/skills/objectstack-data/rules/validation.md index 0050fabf17..21bbd61e38 100644 --- a/skills/objectstack-data/rules/validation.md +++ b/skills/objectstack-data/rules/validation.md @@ -79,8 +79,9 @@ uniqueness — is declared as a unique **index** on the object: ```typescript indexes: [ - { fields: ['email'], unique: true }, // single-field uniqueness - { fields: ['tenant_id', 'email'], unique: true }, // composite uniqueness + { fields: ['email'], unique: 'organization' }, // one per organization + { fields: ['department', 'email'], unique: 'organization' }, // composite, per organization + { fields: ['hostname'], unique: 'global' }, // one across the installation ] ``` @@ -406,7 +407,7 @@ Not a validation — declare a unique index on the object: ```typescript indexes: [ - { fields: ['tenant_id', 'email'], unique: true }, + { fields: ['department', 'email'], unique: 'organization' }, ] ``` @@ -418,7 +419,7 @@ indexes: [ 4. **Priority order** — System validations first (0-99), app validations second (100-999), user validations last (1000+) 5. **Clear error messages** — Tell users exactly what's wrong and how to fix it 6. **State machine for workflows** — Use state_machine instead of complex script logic -7. **Uniqueness is an index concern** — Declare `indexes: [{ fields, unique: true }]`, never a script-based existence check +7. **Uniqueness is an index concern** — Declare `indexes: [{ fields, unique: 'organization' | 'global' }]` with the scope stated, never a script-based existence check 8. **External checks are hooks** — Call APIs from `beforeInsert`/`beforeUpdate` hooks, not validations 9. **Cross-field for comparisons** — More efficient than script validation 10. **Test thoroughly** — Validate edge cases, nulls, empty strings