fix(ADR-0048): rescope os lint naming/namespace-prefix to intra-package duplicates

.changeset/fix-lint-namespace-prefix-adr0048.md

@@ -0,0 +1,26 @@
+---
+"@objectstack/cli": patch
+---
+
+fix(ADR-0048): rescope the `os lint` `naming/namespace-prefix` rule to intra-package duplicates
+
+ADR-0048 §3.4 retired the per-item cross-package collision throw — two
+installed packages may legitimately ship the same bare name (e.g. `page/home`),
+stored under distinct composite keys and disambiguated by package-scoped
+resolution. The `naming/namespace-prefix` lint rule was never updated to match,
+so it still:
+
+- **fired on every bare-named UI/automation item** (apps/pages/dashboards/flows/
+  actions/reports/datasets) regardless of whether a duplicate existed — a normal
+  single-package app got dozens of false positives (hotcrm: 63), and
+- **claimed the package would "collide on the registry key and fail at install"**,
+  which is no longer true.
+
+The rule now warns **only on a genuine intra-package duplicate `(type, name)`
+pair** within the linted config — the narrow authoring-time hygiene case ADR-0048
+§3.4 explicitly leaves to `os lint` ("an author shipping two `page/home` in one
+package"). A unique bare name produces zero warnings. The message no longer
+claims an install failure; it explains the items shadow each other on the
+registry key and that distinct packages may reuse the same name freely (the
+namespace prefix is an optional convention). Runtime/registry behavior is
+unchanged.

packages/cli/src/commands/lint.ts

@@ -216,48 +216,68 @@ export function lintConfig(config: any): LintIssue[] {
     }
   }
 
-  // ── Namespace-prefix advisory for bare-named metadata (ADR-0048 §3.3) ──
-  // Cross-package collision detection (ADR-0048) makes a clash between two
-  // packages' bare-named items (`page`/`flow`/`action`/…) a loud error at
-  // registration time. This shifts that left: a non-fatal nudge to prefix
-  // the name with the package namespace at authoring time, so the clash is
-  // unlikely to ever happen. Objects are already prefix-*enforced* (error)
-  // in defineStack; views are object-derived; `doc` has its own build lint
-  // — so they are excluded here. Warning only — prefix stays a recommended
-  // convention for legacy types, not a retroactive requirement.
+  // ── Intra-package duplicate-name advisory (ADR-0048 §3.4) ──
+  // ADR-0048 §3.4 retired the per-item CROSS-package throw: package ids are
+  // globally unique, so two installed packages shipping the same bare name
+  // (e.g. `page/home`) legitimately COEXIST under distinct composite keys and
+  // each caller resolves to its own via package-scoped resolution. A bare name
+  // is therefore NOT a collision risk and must not warn on its own.
+  //
+  // What the lint still earns its keep on is the narrow authoring-time hygiene
+  // case the ADR explicitly leaves to `os lint`: "an author shipping two
+  // `page/home` in one package". Two items of the same (type, name) declared
+  // within ONE package's config share a single composite registry key and
+  // shadow each other (last-write-wins). We only see one package's config here,
+  // so the legitimate signal is a genuine duplicate `(type, name)` pair within
+  // it — never a unique bare name.
+  //
+  // Objects are already prefix-*enforced* (error) in defineStack; views are
+  // object-derived; `doc` has its own build lint — so they are excluded here.
   const ns: string | undefined = config.manifest?.namespace;
-  if (ns) {
-    // A name is namespace-safe if it IS the namespace (ADR-0019: a package's
-    // single app is conventionally named after the namespace, e.g. `crm`),
-    // is prefixed with it (`crm_lead`), or is a platform-reserved `sys_` name.
-    const isNamespaceSafe = (name: string) =>
-      name === ns || name.startsWith(`${ns}_`) || name.startsWith('sys_');
-
-    // Bare-named UI/automation types subject to the cross-package collision
-    // (ADR-0048 §1.1). Data-driven so a new bare-named type is one line.
-    const PREFIXED_TYPES: Array<{ key: string; label: string }> = [
-      { key: 'apps', label: 'App' },
-      { key: 'pages', label: 'Page' },
-      { key: 'dashboards', label: 'Dashboard' },
-      { key: 'flows', label: 'Flow' },
-      { key: 'actions', label: 'Action' },
-      { key: 'reports', label: 'Report' },
-      { key: 'datasets', label: 'Dataset' },
-    ];
-
-    for (const { key, label } of PREFIXED_TYPES) {
-      const items: any[] = Array.isArray(config[key]) ? config[key] : [];
-      for (let i = 0; i < items.length; i++) {
-        const name = items[i]?.name;
-        if (typeof name !== 'string' || !name || isNamespaceSafe(name)) continue;
-        issues.push({
-          severity: 'warning',
-          rule: 'naming/namespace-prefix',
-          message: `${label} "${name}" is not namespace-prefixed. Two packages defining "${name}" collide on the registry key and fail at install (ADR-0048). Rename to "${ns}_${name}".`,
-          path: `${key}[${i}].name`,
-          fix: `${ns}_${name}`,
-        });
+
+  // Bare-named UI/automation types that share the generic registry namespace.
+  // Data-driven so a new bare-named type is one line.
+  const PREFIXED_TYPES: Array<{ key: string; label: string }> = [
+    { key: 'apps', label: 'App' },
+    { key: 'pages', label: 'Page' },
+    { key: 'dashboards', label: 'Dashboard' },
+    { key: 'flows', label: 'Flow' },
+    { key: 'actions', label: 'Action' },
+    { key: 'reports', label: 'Report' },
+    { key: 'datasets', label: 'Dataset' },
+  ];
+
+  for (const { key, label } of PREFIXED_TYPES) {
+    const items: any[] = Array.isArray(config[key]) ? config[key] : [];
+    // First occurrence of each name → its index, so a later duplicate can point
+    // back at the original declaration.
+    const firstSeen = new Map<string, number>();
+    for (let i = 0; i < items.length; i++) {
+      const name = items[i]?.name;
+      if (typeof name !== 'string' || !name) continue;
+      const original = firstSeen.get(name);
+      if (original === undefined) {
+        firstSeen.set(name, i);
+        continue;
       }
+      // Genuine intra-package duplicate: two items of the same (type, name)
+      // declared in this package's config. They collapse onto one registry key
+      // and shadow each other. Renaming one with the package namespace prefix
+      // (`crm_home`) is the simplest fix; any distinct name works.
+      const suggestion = ns && !name.startsWith(`${ns}_`) ? `${ns}_${name}` : undefined;
+      issues.push({
+        severity: 'warning',
+        rule: 'naming/namespace-prefix',
+        message:
+          `${label} "${name}" is declared more than once in this package ` +
+          `(also at ${key}[${original}].name). Two items of the same type sharing ` +
+          `a bare name within one package shadow each other on the registry key ` +
+          `(ADR-0048 §3.4) — rename one${suggestion ? `, e.g. "${suggestion}"` : ''}. ` +
+          `Distinct packages may reuse the same name freely; the namespace prefix ` +
+          `is an optional convention, not a collision-avoidance requirement.`,
+        path: `${key}[${i}].name`,
+        ...(suggestion ? { fix: suggestion } : {}),
+      });
     }
   }
 

packages/cli/test/lint-namespace-prefix.test.ts

@@ -6,63 +6,99 @@ import { lintConfig } from '../src/commands/lint';
 const RULE = 'naming/namespace-prefix';
 const prefixIssues = (config: any) => lintConfig(config).filter((i) => i.rule === RULE);
 
-describe('lint — namespace-prefix advisory (ADR-0048 §3.3)', () => {
-  it('warns on a bare-named page without the namespace prefix', () => {
+describe('lint — intra-package duplicate-name advisory (ADR-0048 §3.4)', () => {
+  it('stays silent on unique bare names — they are NOT a collision (ADR-0048 §3.4)', () => {
+    // The cross-package throw was retired: distinct packages coexist on the
+    // same bare name via package-scoped resolution. A single package with
+    // unique bare names must produce zero warnings (was 63 false positives
+    // for hotcrm under the old over-broad rule).
     const issues = prefixIssues({
       manifest: { namespace: 'crm' },
-      pages: [{ name: 'home', label: 'Home' }],
+      apps: [{ name: 'crm' }],
+      pages: [{ name: 'home' }, { name: 'settings' }],
+      dashboards: [{ name: 'overview' }],
+      flows: [{ name: 'onboard' }],
+      actions: [{ name: 'send' }],
+      reports: [{ name: 'pipeline' }],
+      datasets: [{ name: 'sales' }],
+    });
+    expect(issues).toHaveLength(0);
+  });
+
+  it('warns on an actual intra-package duplicate (type, name) pair', () => {
+    const issues = prefixIssues({
+      manifest: { namespace: 'crm' },
+      pages: [{ name: 'home', label: 'Home' }, { name: 'home', label: 'Home 2' }],
     });
     expect(issues).toHaveLength(1);
     expect(issues[0].severity).toBe('warning');
-    expect(issues[0].path).toBe('pages[0].name');
-    expect(issues[0].fix).toBe('crm_home');
+    // The warning points at the duplicate occurrence, not the first.
+    expect(issues[0].path).toBe('pages[1].name');
+    expect(issues[0].message).toContain('declared more than once');
+    expect(issues[0].message).toContain('pages[0].name');
     expect(issues[0].message).toContain('ADR-0048');
+    // Suggests a namespace-prefixed rename when a namespace is available.
+    expect(issues[0].fix).toBe('crm_home');
   });
 
-  it('accepts a namespace-prefixed name', () => {
-    expect(
-      prefixIssues({ manifest: { namespace: 'crm' }, flows: [{ name: 'crm_onboarding' }] }),
-    ).toHaveLength(0);
+  it('does not claim the package will fail at install', () => {
+    const issues = prefixIssues({
+      manifest: { namespace: 'crm' },
+      flows: [{ name: 'onboard' }, { name: 'onboard' }],
+    });
+    expect(issues).toHaveLength(1);
+    // ADR-0048 §3.4 retired the per-item cross-package throw — the old
+    // "collide on the registry key and fail at install" claim is false.
+    expect(issues[0].message).not.toContain('fail at install');
+    expect(issues[0].message).not.toContain('Two packages');
   });
 
-  it('exempts an app named after the namespace (ADR-0019 single-app convention)', () => {
-    // `defineApp({ name: 'crm' })` in namespace `crm` must NOT warn.
-    expect(
-      prefixIssues({ manifest: { namespace: 'crm' }, apps: [{ name: 'crm' }] }),
-    ).toHaveLength(0);
+  it('detects duplicates per type independently across every bare-named type', () => {
+    const issues = prefixIssues({
+      manifest: { namespace: 'crm' },
+      apps: [{ name: 'a' }, { name: 'a' }],
+      pages: [{ name: 'p' }, { name: 'p' }],
+      dashboards: [{ name: 'd' }, { name: 'd' }],
+      flows: [{ name: 'f' }, { name: 'f' }],
+      actions: [{ name: 'ac' }, { name: 'ac' }],
+      reports: [{ name: 'r' }, { name: 'r' }],
+      datasets: [{ name: 'ds' }, { name: 'ds' }],
+    });
+    expect(issues).toHaveLength(7);
+    expect(new Set(issues.map((i) => i.severity))).toEqual(new Set(['warning']));
   });
 
-  it('exempts platform-reserved sys_ names', () => {
+  it('treats the same name under different types as distinct (no false positive)', () => {
+    // `page/home` and `flow/home` live under different registry collections —
+    // they do not collide, so a shared name across types must not warn.
     expect(
-      prefixIssues({ manifest: { namespace: 'crm' }, pages: [{ name: 'sys_admin' }] }),
+      prefixIssues({
+        manifest: { namespace: 'crm' },
+        pages: [{ name: 'home' }],
+        flows: [{ name: 'home' }],
+      }),
     ).toHaveLength(0);
   });
 
-  it('covers every bare-named UI/automation type', () => {
+  it('warns on duplicates even when no namespace is declared (omits the fix)', () => {
+    // Duplicate names shadow each other regardless of namespace; without a
+    // namespace there is no prefix to suggest, so no `fix` is offered.
     const issues = prefixIssues({
-      manifest: { namespace: 'crm' },
-      apps: [{ name: 'other' }],
-      pages: [{ name: 'home' }],
-      dashboards: [{ name: 'overview' }],
-      flows: [{ name: 'onboard' }],
-      actions: [{ name: 'send' }],
-      reports: [{ name: 'pipeline' }],
-      datasets: [{ name: 'sales' }],
+      pages: [{ name: 'home' }, { name: 'home' }],
     });
-    expect(issues).toHaveLength(7);
-    expect(new Set(issues.map((i) => i.severity))).toEqual(new Set(['warning']));
-  });
-
-  it('is silent when the package declares no namespace', () => {
-    // No manifest.namespace → nothing to prefix against; stays quiet.
-    expect(prefixIssues({ pages: [{ name: 'home' }] })).toHaveLength(0);
+    expect(issues).toHaveLength(1);
+    expect(issues[0].fix).toBeUndefined();
+    expect(issues[0].message).toContain('declared more than once');
   });
 
   it('does not touch objects (already prefix-enforced as an error) or views', () => {
     const issues = prefixIssues({
       manifest: { namespace: 'crm' },
-      objects: [{ name: 'lead', fields: { id: { type: 'text' } } }],
-      views: [{ name: 'lead.all' }],
+      objects: [
+        { name: 'lead', fields: { id: { type: 'text' } } },
+        { name: 'lead', fields: { id: { type: 'text' } } },
+      ],
+      views: [{ name: 'lead.all' }, { name: 'lead.all' }],
     });
     expect(issues).toHaveLength(0);
   });