feat(cli): lint the contradictory uniqueness double-declaration (#3991)

.changeset/lint-unique-double-declaration.md

@@ -0,0 +1,37 @@
+---
+"@objectstack/cli": patch
+---
+
+feat(cli): lint the contradictory uniqueness double-declaration (#3991)
+
+New advisory rule `unique/double-declaration`, reported by `os lint` and
+`os build`. It fires when one column carries BOTH a field-level `unique: true`
+and an object-level single-column unique index:
+
+```ts
+email: Field.email({ unique: true }), // per-tenant since #3696
+indexes: [{ fields: ['email'], unique: true }], // platform-wide, verbatim
+```
+
+The two spellings deliberately mean different things (see `IndexSchema`), and
+each is legitimate alone. Together on one column they never are:
+
+- On a **tenant-scoped** object they contradict. The stricter one wins
+ physically, so the global index enforces uniqueness and the per-tenant
+ composite becomes a constraint nothing can trip — one of the two authored
+ intents is silently discarded. Worse, it hides the #3696 semantic change:
+ the switch from global to per-tenant has *no observable effect* while the
+ declared index still enforces the old behaviour, so the author never learns
+ their tenancy model and their real constraint disagree — until a second
+ tenant reuses the value and is rejected.
+- On a **tenancy-less** object they are the same index declared twice.
+
+Tenancy is deliberately not inferred at authoring time (`organization_id` is
+injected by the kernel at registration, not authored), so the message names
+both readings and the fix spells out the choice: `unique: 'global'` plus
+dropping the index for platform-wide, or dropping the index for per-tenant
+(or writing it out as `fields: ['organization_id', 'email']`).
+
+A field already declared `unique: 'global'` is exempt — the index restates
+that intent rather than losing it. Advisory only: the artifact is well-defined,
+so this never fails a build.

content/docs/data-modeling/indexing.mdx

@@ -24,6 +24,51 @@ indexes: [
 ]
 ```
 
+## Two ways to say "unique" — and they mean different things
+
+Uniqueness can be declared in two places, and the choice is not cosmetic:
+
+| Declaration | Materializes as | Scope |
+|:---|:---|:---|
+| 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 |
+
+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`.
+
+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 }`.
+
+<Callout type="warn">
+**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:
+
+```typescript
+// ⚠️ contradictory — the global index wins, the per-tenant scope is dead
+email: Field.email({ unique: true }),
+indexes: [{ fields: ['email'], unique: true }],
+```
+
+`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).
+</Callout>
+
+<Callout type="warn">
+**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.
+</Callout>
+
 ### When to Add Indexes
 
 ✅ **Add indexes for:**

packages/cli/src/commands/compile.ts

@@ -19,6 +19,7 @@ import { validateSecurityPosture, validateOrgAxisRedLines, buildAccessMatrix, di
 import { validateReadonlyFlowWrites } from '@objectstack/lint';
 import { lintFlowPatterns } from '../utils/lint-flow-patterns.js';
 import { lintAutonumberFormats } from '../utils/lint-autonumber-formats.js';
+import { lintUniqueDeclarations } from '../lint/data-model-rules.js';
 import { lintLivenessProperties } from '../utils/lint-liveness-properties.js';
 import { lintViewRefs } from '../utils/lint-view-refs.js';
 import { preflightRequiredCapabilities, renderCapabilityMessage } from '../utils/capability-preflight.js';
@@ -499,6 +500,28 @@ export default class Compile extends Command {
  }
  }
 
+ // 3d-quinquies. Contradictory uniqueness declarations (#3991). A column
+ // carrying BOTH a field-level `unique: true` and a single-column
+ // declared unique index has two intents, of which exactly one takes
+ // effect: since #3696 the field-level form is per-tenant while a
+ // declared index is platform-wide, so the global index wins and the
+ // tenant composite becomes unreachable. Advisory — the artifact is
+ // well-defined; the cost is a declaration that does nothing. Shares
+ // `lintUniqueDeclarations` with `os lint` so both agree.
+ const uniqueLint = lintUniqueDeclarations(
+ Array.isArray((result.data as Record<string, unknown>).objects)
+ ? ((result.data as Record<string, unknown>).objects as any[])
+ : [],
+ );
+ if (uniqueLint.length > 0 && !flags.json) {
+ console.log('');
+ for (const f of uniqueLint) {
+ printWarning(`${f.path}: ${f.message}`);
+ if (f.fix) console.log(chalk.dim(` ${f.fix}`));
+ console.log(chalk.dim(` rule: ${f.rule}`));
+ }
+ }
+
  // 3d-quater. View-reference lint (#2554) — resolves form action targets
  // and view-key collisions at build time. A `type:'form'` target that
  // names a missing view or a LIST view opens a broken/blank form at

packages/cli/src/commands/validate.ts

@@ -24,6 +24,7 @@ import { validateFlowTriggerReadiness } from '@objectstack/lint';
 import { validateReadonlyFlowWrites } from '@objectstack/lint';
 import { lintFlowPatterns } from '../utils/lint-flow-patterns.js';
 import { lintAutonumberFormats } from '../utils/lint-autonumber-formats.js';
+import { lintUniqueDeclarations } from '../lint/data-model-rules.js';
 import { lintLivenessProperties } from '../utils/lint-liveness-properties.js';
 import { lintViewRefs } from '../utils/lint-view-refs.js';
 import { preflightRequiredCapabilities, renderCapabilityMessage } from '../utils/capability-preflight.js';
@@ -592,12 +593,24 @@ export default class Validate extends Command {
  const viewRefErrors = viewRefLint.filter((f) => f.severity === 'error');
  const viewRefWarnings = viewRefLint.filter((f) => f.severity !== 'error');
 
+ // Contradictory uniqueness declarations (#3991) — a column carrying both a
+ // field-level `unique: true` and a single-column declared unique index has
+ // two intents, of which exactly one takes effect. Advisory. Mapped into the
+ // `{ where, hint }` shape the shared renderer below expects; the rule lives
+ // in `lint/data-model-rules.ts` so `os lint` reports the same finding.
+ const uniqueLintWarnings = lintUniqueDeclarations(
+ Array.isArray((result.data as Record<string, unknown>).objects)
+ ? ((result.data as Record<string, unknown>).objects as any[])
+ : [],
+ ).map((f) => ({ where: f.path, message: f.message, hint: f.fix ?? '', rule: f.rule, severity: 'warning' as const }));
+
  const authoringLintErrors = [...flowLintErrors, ...autonumberErrors, ...viewRefErrors];
  const authoringLintWarnings = [
  ...flowLintWarnings,
  ...livenessLint,
  ...autonumberWarnings,
  ...viewRefWarnings,
+ ...uniqueLintWarnings,
  ];
  if (authoringLintErrors.length > 0) {
  if (flags.json) {

packages/cli/src/lint/data-model-rules.ts

@@ -69,6 +69,92 @@ function refOf(def: any): string | undefined {
  return def?.reference || def?.reference_to;
 }
 
+// ─── Uniqueness declarations ────────────────────────────────────────
+
+export const UNIQUE_DOUBLE_DECLARATION = 'unique/double-declaration';
+
+/** Is `unique` declared at all? Mirrors `isUniqueDeclared` in @objectstack/spec/data. */
+function uniqueDeclared(u: unknown): boolean {
+ return u === true || u === 'global';
+}
+
+/**
+ * R10 — the same column carries BOTH a field-level `unique: true` and an
+ * object-level single-column unique index (#3991).
+ *
+ * The two spellings are deliberately different (see `IndexSchema`): field-level
+ * `unique: true` is tenant-scoped since #3696 — it materializes as
+ * `(organization_id, col)`, unique *within* the tenant — while a declared index
+ * is materialized over exactly the columns listed, i.e. platform-wide. Both are
+ * legitimate on their own; together on one column they are never right:
+ *
+ * - On a tenant-scoped object they CONTRADICT. The stricter one wins
+ * physically, so the global index enforces uniqueness and the tenant
+ * composite becomes a constraint nothing can ever trip. One of the two
+ * intents the author wrote is silently discarded.
+ * - On a tenancy-less object they are exactly REDUNDANT — both describe the
+ * same single-column unique index, under the same generated name.
+ *
+ * Tenancy is deliberately NOT inferred here: `organization_id` is injected by
+ * the kernel at registration rather than authored, so an authoring-time guess
+ * would be wrong half the time. The combination is worth flagging either way,
+ * and the message names both readings so the author picks the one they meant.
+ *
+ * A field declared `unique: 'global'` is exempt: it already says
+ * platform-wide, so the declared index restates the same intent rather than
+ * contradicting it (still redundant, but not a silent loss of meaning).
+ *
+ * Advisory. The resulting stack is well-defined — the cost is an intent that
+ * never takes effect, not a broken artifact — so this never fails a build.
+ */
+export function lintUniqueDeclarations(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 declaredIndexes = Array.isArray(obj.indexes) ? obj.indexes : [];
+ if (declaredIndexes.length === 0) continue;
+
+ // Columns covered by a declared SINGLE-column unique index. A composite
+ // (`['organization_id', 'email']`) is the explicit tenant-scoped spelling —
+ // it agrees with the field-level default rather than fighting it.
+ const singleColumnUniqueIndexes = new Map<string, any>();
+ for (const idx of declaredIndexes) {
+ if (!uniqueDeclared(idx?.unique)) continue;
+ const cols = Array.isArray(idx?.fields) ? idx.fields.filter((f: unknown) => typeof f === 'string') : [];
+ if (cols.length !== 1) continue;
+ if (!singleColumnUniqueIndexes.has(cols[0])) singleColumnUniqueIndexes.set(cols[0], idx);
+ }
+ if (singleColumnUniqueIndexes.size === 0) continue;
+
+ for (const { name, def } of fieldEntries(obj.fields)) {
+ if (!uniqueDeclared(def?.unique)) continue;
+ if (def.unique === 'global') continue; // already says platform-wide — no lost intent
+ const idx = singleColumnUniqueIndexes.get(name);
+ if (!idx) continue;
+ const indexLabel = typeof idx?.name === 'string' && idx.name.trim() ? ` '${idx.name.trim()}'` : '';
+ issues.push({
+ severity: 'warning',
+ rule: UNIQUE_DOUBLE_DECLARATION,
+ message:
+ `"${obj.name}.${name}" declares field-level \`unique: true\` AND a single-column unique index${indexLabel} on the same column. ` +
+ `Since #3696 the field-level form is scoped per tenant — \`(tenant, ${name})\` — while a declared index is materialized ` +
+ `over exactly its \`fields\`, i.e. platform-wide. On a tenant-scoped object the global index wins and the per-tenant ` +
+ `constraint can never be reached; on a tenancy-less object the two are the same index declared twice. Either way one of ` +
+ `the two declarations has no effect.`,
+ path: `objects[${i}]`,
+ fix:
+ `Pick the intent: for platform-wide uniqueness set \`unique: 'global'\` on '${name}' and drop the duplicate index; ` +
+ `for per-tenant uniqueness drop the index (the field-level declaration already builds the tenant composite), ` +
+ `or spell the index out as \`fields: ['organization_id', '${name}']\` if you want it explicit.`,
+ });
+ }
+ }
+ return issues;
+}
+
 // ─── Rule engine ────────────────────────────────────────────────────
 
 /**
@@ -77,7 +163,9 @@ function refOf(def: any): string | undefined {
  * metadata-generation scorer.
  */
 export function lintDataModel(objects: any[]): LintIssue[] {
- const issues: LintIssue[] = [];
+ // R10 lives in its own exported function so `os build` can run that ONE rule
+ // without pulling in the whole best-practice sweep (#3991).
+ const issues: LintIssue[] = lintUniqueDeclarations(objects);
  if (!Array.isArray(objects) || objects.length === 0) return issues;
 
  // Index: parent object name → child relationships pointing at it.

packages/cli/test/data-model-rules.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from 'vitest';
-import { lintDataModel } from '../src/lint/data-model-rules';
+import { lintDataModel, lintUniqueDeclarations } from '../src/lint/data-model-rules';
 import { lintConfig } from '../src/commands/lint';
 
 const rulesOf = (issues: { rule: string }[]) => issues.map((i) => i.rule);
@@ -173,3 +173,119 @@ describe('lintConfig integration', () => {
  expect(dataModel.filter((i) => i.severity !== 'suggestion')).toEqual([]);
  });
 });
+
+// #3991 — the same column declared unique twice, in two spellings that mean
+// different things. One of the two intents is always discarded.
+describe('lintUniqueDeclarations — contradictory uniqueness (#3991)', () => {
+ const RULE = 'unique/double-declaration';
+
+ const withBoth = [
+ {
+ name: 'crm_contact',
+ fields: { email: { type: 'email', unique: true } },
+ indexes: [{ fields: ['email'], unique: true }],
+ },
+ ];
+
+ it('returns [] for empty input', () => {
+ expect(lintUniqueDeclarations([])).toEqual([]);
+ expect(lintUniqueDeclarations(undefined as any)).toEqual([]);
+ });
+
+ it('flags field-level unique + a single-column unique index on the same column', () => {
+ const issues = lintUniqueDeclarations(withBoth);
+ expect(issues).toHaveLength(1);
+ expect(issues[0].rule).toBe(RULE);
+ expect(issues[0].severity).toBe('warning'); // advisory — never fails a build
+ expect(issues[0].message).toContain('crm_contact.email');
+ // The message must name BOTH readings, since tenancy is not inferred here.
+ expect(issues[0].message).toMatch(/per tenant|tenant/i);
+ expect(issues[0].message).toMatch(/platform-wide/i);
+ // And the fix must spell out both ways to resolve it.
+ expect(issues[0].fix).toContain("unique: 'global'");
+ expect(issues[0].fix).toContain('organization_id');
+ });
+
+ it('surfaces through lintDataModel too, so `os lint` reports it', () => {
+ expect(has(lintDataModel(withBoth), RULE)).toBe(true);
+ });
+
+ // ── Shapes that must stay quiet ──────────────────────────────────────
+
+ it("exempts unique: 'global' — the index restates the intent, it does not lose it", () => {
+ const issues = lintUniqueDeclarations([
+ {
+ name: 'runtime',
+ fields: { hostname: { type: 'text', unique: 'global' } },
+ indexes: [{ fields: ['hostname'], unique: true }],
+ },
+ ]);
+ expect(issues).toEqual([]);
+ });
+
+ it('exempts an explicit tenant COMPOSITE index — that agrees with the field-level default', () => {
+ const issues = lintUniqueDeclarations([
+ {
+ name: 'crm_contact',
+ fields: { email: { type: 'email', unique: true } },
+ indexes: [{ fields: ['organization_id', 'email'], unique: true }],
+ },
+ ]);
+ expect(issues).toEqual([]);
+ });
+
+ it('ignores a NON-unique index on the same column', () => {
+ const issues = lintUniqueDeclarations([
+ {
+ name: 'crm_contact',
+ fields: { email: { type: 'email', unique: true } },
+ indexes: [{ fields: ['email'] }],
+ },
+ ]);
+ expect(issues).toEqual([]);
+ });
+
+ it('ignores a unique index on a DIFFERENT column', () => {
+ const issues = lintUniqueDeclarations([
+ {
+ name: 'crm_contact',
+ fields: { email: { type: 'email', unique: true }, code: { type: 'text' } },
+ indexes: [{ fields: ['code'], unique: true }],
+ },
+ ]);
+ expect(issues).toEqual([]);
+ });
+
+ it('is quiet when only one of the two spellings is used', () => {
+ expect(lintUniqueDeclarations([
+ { name: 'a', fields: { email: { type: 'email', unique: true } } },
+ ])).toEqual([]);
+ expect(lintUniqueDeclarations([
+ { name: 'b', fields: { email: { type: 'email' } }, indexes: [{ fields: ['email'], unique: true }] },
+ ])).toEqual([]);
+ });
+
+ it('names the declared index when it carries an explicit name', () => {
+ const issues = lintUniqueDeclarations([
+ {
+ name: 'crm_product',
+ fields: { sku: { type: 'text', unique: true } },
+ indexes: [{ name: 'uniq_product_sku', fields: ['sku'], unique: true }],
+ },
+ ]);
+ expect(issues[0].message).toContain("'uniq_product_sku'");
+ });
+
+ it('reports each offending column once, across several objects', () => {
+ const issues = lintUniqueDeclarations([
+ ...withBoth,
+ {
+ name: 'crm_lead',
+ fields: { email: { type: 'email', unique: true }, sku: { type: 'text', unique: true } },
+ indexes: [{ fields: ['email'], unique: true }, { fields: ['sku'], unique: true }],
+ },
+ ]);
+ expect(issues.map((i) => i.message.match(/"([^"]+)"/)?.[1]).sort())
+ .toEqual(['crm_contact.email', 'crm_lead.email', 'crm_lead.sku']);
+ });
+});