Merge pull request #905 from objectstack-ai/copilot/fix-pnpm-dev-start-issue

Author: hotlong

CHANGELOG.md

@@ -8,6 +8,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Fixed
+- **AppPlugin getService crash on missing services** — `AppPlugin.start()` and
+  `loadTranslations()` now wrap `ctx.getService()` in try/catch, since the kernel's
+  `getService` throws when a service is not registered (rather than returning `undefined`).
+  This was the direct cause of `plugin.app.com.example.crm failed to start` — the i18n
+  service was not registered, so `getService('i18n')` threw an unhandled exception.
+- **CLI serve: host config AppPlugin mis-wrap** — `serve.ts` no longer wraps a host/aggregator config
+  (one that already contains instantiated plugins in its `plugins` array) with an extra `AppPlugin`.
+  This prevents the `plugin.app.dev-workspace failed to start` error and eliminates duplicate plugin
+  registration when running `pnpm dev`.
+- **plugin-auth CJS→ESM interop** — Added `module` and `exports` fields to
+  `@objectstack/plugin-auth` package.json so Node.js resolves the ESM build (`.mjs`) when using
+  dynamic `import()`, eliminating the `ExperimentalWarning: CommonJS module … is loading ES Module`
+  warning caused by `better-auth` being ESM-only.
 - **i18n service registration & state inconsistency** — Discovery API (`getDiscoveryInfo`) now uses
   the same async `resolveService()` fallback chain that request handlers (`handleI18n`) use, ensuring
   the reported service status is always consistent with actual runtime availability.

packages/cli/src/commands/serve.ts

@@ -7,6 +7,7 @@ import net from 'net';
 import chalk from 'chalk';
 import { bundleRequire } from 'bundle-require';
 import { loadConfig } from '../utils/config.js';
+import { isHostConfig } from '../utils/plugin-detection.js';
 import {
   printHeader,
   printKV,
@@ -190,7 +191,10 @@ export default class Serve extends Command {
       }
 
       // 3. Auto-register AppPlugin if config contains app definitions
-      if (config.objects || config.manifest || config.apps) {
+      // Skip if config is a host/aggregator config that already contains
+      // instantiated plugins — wrapping it would cause duplicate registration
+      // and startup failures (e.g. plugin.app.dev-workspace).
+      if (!isHostConfig(config) && (config.objects || config.manifest || config.apps)) {
         try {
            const { AppPlugin } = await import('@objectstack/runtime');
            await kernel.use(new AppPlugin(config));

packages/cli/src/utils/plugin-detection.ts

@@ -0,0 +1,16 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * Detect whether a loaded config is a host/aggregator configuration.
+ *
+ * A host config already contains a `plugins` array with instantiated Plugin
+ * objects (i.e. objects that have an `init` method). Such configs must NOT
+ * be wrapped again by AppPlugin in the CLI, as that would cause duplicate
+ * plugin registration and startup failures.
+ */
+export function isHostConfig(config: any): boolean {
+  return (
+    Array.isArray(config.plugins) &&
+    config.plugins.some((p: any) => typeof p?.init === 'function')
+  );
+}

packages/cli/test/serve-host-config.test.ts

@@ -0,0 +1,77 @@
+import { describe, it, expect } from 'vitest';
+import { isHostConfig } from '../src/utils/plugin-detection';
+
+/**
+ * Tests for the host config detection logic used in serve.ts
+ *
+ * When the root config is a host/aggregator (i.e. its `plugins` array
+ * contains already-instantiated Plugin objects), the CLI must NOT wrap
+ * it again with AppPlugin, as that would cause duplicate registration
+ * and a `plugin.app.<id> failed to start` error.
+ */
+describe('Host config detection', () => {
+
+  it('should detect host config with instantiated plugins', () => {
+    const config = {
+      manifest: { id: 'dev-workspace', name: 'dev_workspace' },
+      plugins: [
+        { name: 'objectql', init: async () => {}, start: async () => {} },
+        { name: 'driver', init: async () => {}, start: async () => {} },
+      ],
+    };
+    expect(isHostConfig(config)).toBe(true);
+  });
+
+  it('should NOT detect pure app bundle config (no plugins)', () => {
+    const config = {
+      manifest: { id: 'my-app', name: 'my_app' },
+      objects: [{ name: 'task', fields: [] }],
+    };
+    expect(isHostConfig(config)).toBe(false);
+  });
+
+  it('should NOT detect config with empty plugins array', () => {
+    const config = {
+      manifest: { id: 'my-app', name: 'my_app' },
+      objects: [{ name: 'task', fields: [] }],
+      plugins: [],
+    };
+    expect(isHostConfig(config)).toBe(false);
+  });
+
+  it('should NOT detect config with string plugin references', () => {
+    const config = {
+      manifest: { id: 'my-app', name: 'my_app' },
+      plugins: ['@objectstack/plugin-auth', '@objectstack/objectql'],
+    };
+    expect(isHostConfig(config)).toBe(false);
+  });
+
+  it('should NOT detect config with plain object plugins (no init method)', () => {
+    const config = {
+      manifest: { id: 'my-app', name: 'my_app' },
+      plugins: [
+        { name: 'some-plugin', version: '1.0.0' },
+      ],
+    };
+    expect(isHostConfig(config)).toBe(false);
+  });
+
+  it('should detect if at least one plugin has init method', () => {
+    const config = {
+      manifest: { id: 'dev-workspace' },
+      plugins: [
+        { name: 'plain-bundle', version: '1.0.0' },
+        { name: 'real-plugin', init: async () => {}, start: async () => {} },
+      ],
+    };
+    expect(isHostConfig(config)).toBe(true);
+  });
+
+  it('should handle config without plugins property', () => {
+    const config = {
+      manifest: { id: 'my-app', name: 'my_app' },
+    };
+    expect(isHostConfig(config)).toBe(false);
+  });
+});

packages/plugins/plugin-auth/package.json

@@ -4,7 +4,15 @@
   "license": "Apache-2.0",
   "description": "Authentication & Identity Plugin for ObjectStack",
   "main": "dist/index.js",
+  "module": "dist/index.mjs",
   "types": "dist/index.d.ts",
+  "exports": {
+    ".": {
+      "import": "./dist/index.mjs",
+      "require": "./dist/index.js",
+      "types": "./dist/index.d.ts"
+    }
+  },
   "scripts": {
     "build": "tsup --config ../../../tsup.config.ts",
     "test": "vitest run",

packages/runtime/src/app-plugin.test.ts

@@ -100,6 +100,22 @@ describe('AppPlugin', () => {
         );
     });
 
+    it('start should handle getService throwing for objectql', async () => {
+        const bundle = { id: 'com.test.throw' };
+        const plugin = new AppPlugin(bundle);
+        
+        vi.mocked(mockContext.getService).mockImplementation(() => {
+            throw new Error("[Kernel] Service 'objectql' not found");
+        });
+        
+        await plugin.start!(mockContext);
+        
+        expect(mockContext.logger.warn).toHaveBeenCalledWith(
+            expect.stringContaining('ObjectQL engine service not found'), 
+            expect.any(Object)
+        );
+    });
+
     // ═══════════════════════════════════════════════════════════════
     // i18n translation auto-loading
     // ═══════════════════════════════════════════════════════════════
@@ -173,6 +189,26 @@ describe('AppPlugin', () => {
             );
         });
 
+        it('should skip translation loading when getService throws for i18n', async () => {
+            vi.mocked(mockContext.getService).mockImplementation((name: string) => {
+                if (name === 'objectql') return mockQL;
+                throw new Error("[Kernel] Service 'i18n' not found");
+            });
+
+            const bundle = {
+                id: 'com.test.i18nthrow',
+                translations: [{ en: { messages: { hello: 'Hello' } } }],
+            };
+            const plugin = new AppPlugin(bundle);
+            await plugin.start!(mockContext);
+
+            // Should log debug but not throw
+            expect(mockContext.logger.debug).toHaveBeenCalledWith(
+                expect.stringContaining('No i18n service registered'),
+                expect.any(Object)
+            );
+        });
+
         it('should handle bundle with no translations gracefully', async () => {
             const bundle = { id: 'com.test.notrans' };
             const plugin = new AppPlugin(bundle);

packages/runtime/src/app-plugin.ts

@@ -60,9 +60,15 @@ export class AppPlugin implements Plugin {
 
         // Execute Runtime Step
         // Retrieve ObjectQL engine from services
-        // We cast to any/ObjectQL because ctx.getService returns unknown
-        const ql = ctx.getService('objectql') as any;
-        
+        // ctx.getService throws when a service is not registered, so we
+        // must use try/catch instead of a null-check.
+        let ql: any;
+        try {
+            ql = ctx.getService('objectql');
+        } catch {
+            // Service not registered — handled below
+        }
+
         if (!ql) {
             ctx.logger.warn('ObjectQL engine service not found', { 
                 appName: this.name,
@@ -201,7 +207,15 @@ export class AppPlugin implements Plugin {
      * this keeps AppPlugin resilient across server/dev/mock environments.
      */
     private loadTranslations(ctx: PluginContext, appId: string): void {
-        const i18nService = ctx.getService('i18n') as II18nService | undefined;
+        // ctx.getService throws when a service is not registered, so we
+        // must use try/catch to gracefully skip when no i18n plugin is loaded.
+        let i18nService: II18nService | undefined;
+        try {
+            i18nService = ctx.getService('i18n') as II18nService;
+        } catch {
+            // Service not registered — handled below
+        }
+
         if (!i18nService) {
             ctx.logger.debug('[i18n] No i18n service registered; skipping translation loading', { appId });
             return;