Merge pull request #1024 from objectstack-ai/copilot/fix-double-pass-hack-merge

Author: hotlong

ROADMAP.md

@@ -1188,7 +1188,11 @@ Plugin architecture refactoring to support true modular development, plugin isol
 - [x] Remove duplicate `mergeActionsIntoObjects()` from root config and console shared config
 - [x] Remove duplicate `mergeViewsIntoObjects()` from root config and console shared config (moved into `composeStacks`)
 - [x] Refactor root `objectstack.config.ts` and `apps/console/objectstack.shared.ts` to use `composeStacks()`
-- [x] Unit tests for `composeStacks()` (13 tests covering merging, dedup, views, actions, cross-stack)
+- [x] Unit tests for `composeStacks()` (15 tests covering merging, dedup, views, actions, cross-stack, conflict detection)
+- [x] Eliminate `defineStack()` double-pass hack — single `composeStacks()` call produces final config with runtime properties (listViews, actions) preserved. `defineStack()` Zod validation stripped these fields, requiring a second `composeStacks` pass to restore them.
+- [x] Use `composed.apps` unified flow in console shared config — replaced manual `[...crmApps, ...(todoConfig.apps || []), ...]` spreading with CRM navigation patch applied to composed output
+- [x] Use `composed.reports` in console shared config — replaced `...(crmConfig.reports || [])` with `...(composed.reports || [])` to include reports from all stacks
+- [x] **composeStacks responsibility split:** `@object-ui/core` composeStacks handles runtime mapping (merging views→objects listViews, actions→objects) while `@objectstack/spec` composeStacks handles protocol-level composition (broader field concatenation, manifest selection, i18n). ObjectUI apps use the core version for single-pass config with runtime properties preserved.
 
 **Phase 2 — Dynamic Plugin Loading (Planned)**
 - [ ] Hot-reload / lazy loading of plugins for development

apps/console/objectstack.shared.ts

@@ -1,4 +1,3 @@
-import { defineStack } from '@objectstack/spec';
 import type { ObjectStackDefinition } from '@objectstack/spec';
 import { composeStacks } from '@object-ui/core';
 import crmConfigImport from '@object-ui/example-crm/objectstack.config';
@@ -18,33 +17,31 @@ const crmConfig = resolveDefault<ObjectStackDefinition>(crmConfigImport);
 const todoConfig = resolveDefault<ObjectStackDefinition>(todoConfigImport);
 const kitchenSinkConfig = resolveDefault<ObjectStackDefinition>(kitchenSinkConfigImport);
 
-// Patch CRM App Navigation to include Report using a supported navigation type
-// (type: 'url' passes schema validation while still routing correctly via React Router)
-const crmApps = crmConfig.apps ? JSON.parse(JSON.stringify(crmConfig.apps)) : [];
-if (crmApps.length > 0) {
-    const crmApp = crmApps[0];
-    if (crmApp && crmApp.navigation) {
-        // Insert report after dashboard
-        const dashboardIdx = crmApp.navigation.findIndex((n: any) => n.id === 'nav_dashboard');
-        const insertIdx = dashboardIdx !== -1 ? dashboardIdx + 1 : 0;
-        crmApp.navigation.splice(insertIdx, 0, {
-            id: 'nav_sales_report',
-            type: 'url',
-            url: '/apps/crm_app/report/sales_performance_q1',
-            label: 'Sales Report',
-            icon: 'file-bar-chart'
-        });
-    }
-}
-
-// Compose all example stacks into a single merged definition.
-// composeStacks handles object deduplication (override), views→objects mapping,
-// and actions→objects assignment via objectName.
+// Single-pass composition: composeStacks handles object deduplication (override),
+// views→objects mapping, and actions→objects assignment via objectName.
+// No defineStack() validation pass — it would strip runtime properties (listViews,
+// actions) from objects, requiring a double-pass hack to restore them.
 const composed = composeStacks(
   [crmConfig, todoConfig, kitchenSinkConfig] as Record<string, any>[],
   { objectConflict: 'override' },
 );
 
+// Patch CRM App Navigation to include Report using a supported navigation type
+// (type: 'url' passes schema validation while still routing correctly via React Router)
+const apps = JSON.parse(JSON.stringify(composed.apps || []));
+const crmApp = apps.find((a: any) => a.name === 'crm_app');
+if (crmApp?.navigation) {
+    const dashboardIdx = crmApp.navigation.findIndex((n: any) => n.id === 'nav_dashboard');
+    const insertIdx = dashboardIdx !== -1 ? dashboardIdx + 1 : 0;
+    crmApp.navigation.splice(insertIdx, 0, {
+        id: 'nav_sales_report',
+        type: 'url',
+        url: '/apps/crm_app/report/sales_performance_q1',
+        label: 'Sales Report',
+        icon: 'file-bar-chart'
+    });
+}
+
 export const sharedConfig = {
   // ============================================================================
   // Project Metadata
@@ -58,15 +55,11 @@ export const sharedConfig = {
   // Merged Stack Configuration (CRM + Todo + Kitchen Sink)
   // ============================================================================
   objects: composed.objects,
-  apps: [
-    ...crmApps,
-    ...(todoConfig.apps || []),
-    ...(kitchenSinkConfig.apps || []),
-  ],
+  apps,
   dashboards: composed.dashboards,
   reports: [
-    ...(crmConfig.reports || []),
-    // Manually added report since CRM config validation prevents it
+    ...(composed.reports || []),
+    // Console-specific report not in any example stack
     {
       name: 'sales_performance_q1',
       label: 'Q1 Sales Performance',
@@ -99,17 +92,4 @@ export const sharedConfig = {
   ]
 };
 
-const allConfigs = [crmConfig, todoConfig, kitchenSinkConfig];
-
-// defineStack() validates the config but strips non-standard properties like
-// listViews and actions from objects. A second composeStacks pass restores
-// these runtime properties onto the validated objects. This double-pass is
-// necessary because defineStack's Zod schema doesn't preserve custom fields.
-const validated = defineStack(sharedConfig as Parameters<typeof defineStack>[0]);
-export default {
-  ...validated,
-  objects: composeStacks([
-    { objects: validated.objects || [] },
-    ...allConfigs.map((cfg: any) => ({ views: cfg.views || [], actions: cfg.actions || [] })),
-  ]).objects,
-};
+export default sharedConfig;

objectstack.config.ts

@@ -14,7 +14,6 @@
  * `kernel.use(new CRMPlugin())`. In the dev workspace, we collect their
  * configs via `getConfig()` and merge them with `composeStacks()`.
  */
-import { defineStack } from '@objectstack/spec';
 import { AppPlugin, DriverPlugin } from '@objectstack/runtime';
 import { ObjectQLPlugin } from '@objectstack/objectql';
 import { InMemoryDriver } from '@objectstack/driver-memory';
@@ -34,14 +33,14 @@ const allConfigs = plugins.map((p) => {
   return (raw as any).default || raw;
 });
 
-// Compose all plugin configs into a single stack definition.
-// composeStacks handles object deduplication, views→objects mapping,
-// and actions→objects assignment via objectName.
+// Single-pass composition: composeStacks handles object deduplication,
+// views→objects mapping, and actions→objects assignment via objectName.
+// No defineStack() validation pass needed — it would strip runtime properties
+// (listViews, actions) from objects, requiring a second merge pass to restore them.
 const composed = composeStacks(allConfigs, { objectConflict: 'override' });
 
-// Validate via defineStack, then re-apply runtime properties (listViews, actions)
-// that defineStack strips during validation.
-const mergedApp = defineStack({
+const mergedApp = {
+  ...composed,
   manifest: {
     id: 'dev-workspace',
     name: 'dev_workspace',
@@ -50,24 +49,6 @@ const mergedApp = defineStack({
     type: 'app',
     data: composed.manifest.data,
   },
-  objects: composed.objects,
-  views: composed.views,
-  apps: composed.apps,
-  dashboards: composed.dashboards,
-  reports: composed.reports,
-  pages: composed.pages,
-} as any);
-
-// defineStack() validates the config but strips non-standard properties like
-// listViews and actions from objects. A second composeStacks pass restores
-// these runtime properties onto the validated objects. This double-pass is
-// necessary because defineStack's Zod schema doesn't preserve custom fields.
-const mergedAppWithViews = {
-  ...mergedApp,
-  objects: composeStacks([
-    { objects: mergedApp.objects || [] },
-    ...allConfigs.map((cfg: any) => ({ views: cfg.views || [], actions: cfg.actions || [] })),
-  ]).objects,
 };
 
 // Export only plugins — no top-level objects/manifest/apps.
@@ -77,7 +58,7 @@ export default {
   plugins: [
     new ObjectQLPlugin(),
     new DriverPlugin(new InMemoryDriver()),
-    new AppPlugin(mergedAppWithViews),
+    new AppPlugin(mergedApp),
     new HonoServerPlugin({ port: 3000 }),
     new ConsolePlugin(),
   ],

packages/core/src/utils/__tests__/compose-stacks.test.ts

@@ -281,4 +281,32 @@ describe('composeStacks', () => {
     // Manifest data merged
     expect(result.manifest.data).toHaveLength(2);
   });
+
+  it('should detect conflicting object names with objectConflict: "error"', () => {
+    // Simulates a scenario where two plugins define 'account' without prefixing
+    const crm = {
+      objects: [{ name: 'account', label: 'CRM Account', fields: {} }],
+    };
+    const ks = {
+      objects: [{ name: 'account', label: 'KS Account', fields: {} }],
+    };
+
+    expect(() => composeStacks([crm, ks], { objectConflict: 'error' })).toThrow(
+      'duplicate object name "account"'
+    );
+  });
+
+  it('should allow prefixed object names to coexist without conflict', () => {
+    // After renaming: CRM keeps 'account', Kitchen Sink uses 'ks_account'
+    const crm = {
+      objects: [{ name: 'account', label: 'CRM Account', fields: {} }],
+    };
+    const ks = {
+      objects: [{ name: 'ks_account', label: 'KS Account', fields: {} }],
+    };
+
+    const result = composeStacks([crm, ks], { objectConflict: 'error' });
+    expect(result.objects).toHaveLength(2);
+    expect(result.objects.map((o: any) => o.name).sort()).toEqual(['account', 'ks_account']);
+  });
 });