fix(cli): resolve an app-declared config plugin from the app, not the CLI (#10948)

* fix(cli): resolve an app-declared config plugin from the app, not the CLI (#10908)

`plugins: [...]` in the served app's own `objectstack.config.ts` is the
documented extension path, and its string entries are the most app-owned
specifiers in `serve.ts` — yet they were loaded with a bare `import()`, which
Node ESM resolves against the CLI's realpath. A plugin the APP declares was
therefore only loadable where it happened to be hoisted somewhere the CLI could
see it: green in a dev checkout, missing on a real distribution layout. Same
class as cloud#1013 and #10645, on the surface users are told to use.

`Serve.importConfigPlugin` now lets the DECLARATION pick the resolver, which is
what #4719 says should decide. Handing every specifier to `importFromHost`
would not have been a superset — measured, twice over:

  * a relative specifier would re-base from this file's directory to
    `@objectstack/types/dist/`, because the host importer's pass-through
    re-enters `import()` from inside that package;
  * an UNDECLARED bare name would too, and that one takes working deployments
    away: from an app that declares nothing, `@objectstack/plugin-auth` and
    `@objectstack/plugin-audit` resolve from the CLI and fail through the host
    importer under a pnpm-isolated layout.

So only the declared case moves; the other two branches keep the exact
resolution they had. Nothing changes about which plugins are ACCEPTED — the
#4719 declaration gate is untouched and no undeclared package gains a way in.

The missing-plugin diagnostic now nests the #4719 remedy, and is pinned as a
chosen text rather than left to drift.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r

* test(cli): pin that the app's copy wins over the CLI's own for a declared plugin (#10908)

The first draft of this file could not tell the two apart: its fixture package
was invisible to the CLI, so the declared branch and the undeclared fallback
both converged on the app's copy and an ablation that disabled the declaration
check stayed green. Declaring a package the CLI ALSO resolves (`chalk`) is what
makes "which copy wins" observable, which is the resolution-policy question the
card is actually about.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r

* docs(cli): point the config-plugin branches at the cards carrying their questions (#10908)

Both non-declared branches exist because of a measurement, and each measurement
turned into a card: #10943 (the host importer's fallback resolves from
`@objectstack/types`, contradicting its own docblock) and #10944 (a relative
`plugins:` entry resolves against the CLI's directory, so it cannot work).
Naming them here is what stops the next reader re-deriving the branch or
deleting it as redundant.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: Elon Musk

.changeset/serve-config-plugin-host-resolution.md

@@ -0,0 +1,25 @@
+---
+"@objectstack/cli": patch
+---
+
+`os serve` now resolves a `plugins: [...]` entry the served app **declares** from
+that app, instead of from the CLI (#10908).
+
+`plugins: [...]` in the app's own `objectstack.config.ts` is the documented way
+to extend a deployment, but its string entries were loaded with a bare
+`import()`, which Node ESM resolves against the CLI's realpath. An app that
+wrote `plugins: ['@acme/my-plugin']` and declared `@acme/my-plugin` in its own
+`package.json` could therefore only be served where that package happened to be
+hoisted somewhere the CLI could see it — true in a dev checkout, absent on a
+real distribution layout. Same mechanism as the cluster and organizations loads
+fixed earlier.
+
+Only the **declared** case moves. A specifier the app does not declare still
+resolves from the CLI exactly as before, and a path or `file://` URL keeps the
+base it always had, so no deployment loses a plugin it is loading today. Which
+plugins are *accepted* is unchanged — the declaration gate is untouched.
+
+One user-facing message changes: when a declared plugin cannot be loaded, the
+`Failed to import plugin '<name>'` error now carries the declaration remedy
+("declare it in that app's `package.json`", or the install-problem text when the
+app declares it but it is not installed) instead of a bare `Cannot find package`.

packages/cli/src/commands/serve-cluster-host-resolution.test.ts

@@ -278,11 +278,18 @@ const UNRESOLVABLE_BARE_IMPORTS: Record<string, string> = {
   // Serve.CAPABILITY_PROVIDERS — every `pkg` in that table is CLI-declared.
   'spec.pkg': 'Serve.CAPABILITY_PROVIDERS entries are all CLI-declared',
   'ex.pkg': 'CAPABILITY_PROVIDERS `extras` entries are all CLI-declared',
-  // The app's own `plugins: [...]` config entries — an app-supplied specifier, so
-  // this IS the class, but no source scan can classify it and host-anchoring it
-  // changes a user-facing error message plus which copy of a CLI-declared plugin
-  // wins. Filed as #10908 rather than widened here.
-  plugin: 'app-supplied plugin name from objectstack.config.ts — see #10908',
+  // The app's own `plugins: [...]` config entries, now routed through
+  // `Serve.importConfigPlugin` (#10908). Two bare `import()` sites remain there,
+  // both reached only AFTER the declaration has been consulted, and both are the
+  // reason this list exists rather than a hole in it:
+  //   • the specifier is not a package name at all (path, `file://`, `node:`) —
+  //     nothing a package.json can declare;
+  //   • the served app does NOT declare it, so it must resolve from this CLI,
+  //     which is exactly the pre-existing behaviour #10908 promised to keep.
+  // The DECLARED case — the only one this card moves — goes to `importFromHost`.
+  // Pinned behaviourally, not by this comment, in
+  // `serve-config-plugin-host-resolution.test.ts`.
+  pluginSpecifier: 'post-declaration branches: a path/URL, or a package the app does not declare (#10908)',
 };
 
 /**

packages/cli/src/commands/serve-config-plugin-host-resolution.test.ts

@@ -0,0 +1,202 @@
+import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { pathToFileURL } from 'node:url';
+import { readFileSync } from 'node:fs';
+import { afterAll, describe, expect, it } from 'vitest';
+import Serve from './serve.js';
+
+/**
+ * `plugins: [...]` in the served app's own `objectstack.config.ts` is THE
+ * documented way to extend a deployment, and its string entries are the most
+ * app-owned specifiers in `serve.ts`. They were loaded with a bare `import()`,
+ * which Node ESM resolves against the CLI's realpath — so a plugin the APP
+ * declares could only be served where it happened to be hoisted somewhere the
+ * CLI could see. Green in a dev checkout, absent on a real distribution layout
+ * (#10908; the same mechanism as cloud#1013 and #10645).
+ *
+ * The repair moves ONLY the declared case. These tests pin all three branches,
+ * because two of them exist to keep behaviour that a naive
+ * `await importFromHost(specifier)` would have taken away — see
+ * `Serve.importConfigPlugin` for the measurements.
+ */
+
+const roots: string[] = [];
+afterAll(() => {
+  for (const r of roots) rmSync(r, { recursive: true, force: true });
+});
+
+/**
+ * An app root with its own `package.json`, optionally DECLARING `pkgName` and
+ * optionally carrying it in its own `node_modules`. Nothing is installed or
+ * built: the package is two files in a temp dir, and its marker export is how a
+ * test proves WHICH copy loaded.
+ */
+function makeApp(
+  pkgName: string,
+  opts: { declare: boolean; install: boolean; marker?: string },
+): string {
+  const root = mkdtempSync(join(tmpdir(), 'os-cfg-plugin-'));
+  roots.push(root);
+  writeFileSync(
+    join(root, 'package.json'),
+    JSON.stringify({
+      name: 'fixture-app',
+      version: '1.0.0',
+      type: 'module',
+      ...(opts.declare ? { dependencies: { [pkgName]: '1.0.0' } } : {}),
+    }),
+  );
+  if (opts.install) {
+    const dir = join(root, 'node_modules', ...pkgName.split('/'));
+    mkdirSync(dir, { recursive: true });
+    writeFileSync(
+      join(dir, 'package.json'),
+      JSON.stringify({ name: pkgName, version: '1.0.0', type: 'module', main: 'index.js' }),
+    );
+    writeFileSync(
+      join(dir, 'index.js'),
+      `export default { name: ${JSON.stringify(opts.marker ?? 'app-copy')} };\n`,
+    );
+  }
+  return root;
+}
+
+// A name no workspace package can satisfy, so a pass can never come from the
+// CLI's own node_modules by accident.
+const APP_ONLY = '@os-fixture/config-plugin-probe';
+
+describe('os serve → an app-declared `plugins: [...]` package resolves from the APP (#10908)', () => {
+  it('loads the copy the served app declares and carries — the defect, repaired', async () => {
+    const root = makeApp(APP_ONLY, { declare: true, install: true, marker: 'app-copy' });
+
+    // The failing hop, reproduced first: this file resolves from `packages/cli`
+    // exactly as `dist/commands/serve.js` does, and cannot see the app's package.
+    const bare: string = APP_ONLY;
+    await expect(import(bare)).rejects.toMatchObject({
+      code: expect.stringMatching(/MODULE_NOT_FOUND|ERR_MODULE_NOT_FOUND/),
+    });
+
+    const mod = await Serve.importConfigPlugin(APP_ONLY, root);
+    expect(mod.default).toEqual({ name: 'app-copy' });
+  });
+
+  it("prefers the APP's copy over the CLI's own when BOTH can resolve the name", async () => {
+    // The resolution-policy question the card names: which copy wins when the
+    // CLI also ships the package. `chalk` is declared by packages/cli and
+    // resolves from this file, so a fixture app that declares its OWN `chalk`
+    // is the only way to tell the two apart — and the app's declaration is the
+    // contract (#4719), so the app's copy must win.
+    const root = makeApp('chalk', { declare: true, install: true, marker: 'app-owned-chalk' });
+
+    const mod = await Serve.importConfigPlugin('chalk', root);
+
+    expect(mod.default).toEqual({ name: 'app-owned-chalk' });
+    // Not a tautology: the CLI's own resolution of the same name finds the real
+    // package, which is what this line would have loaded before the fix.
+    const cliCopy: any = await import('chalk');
+    expect(cliCopy.default).not.toEqual({ name: 'app-owned-chalk' });
+  });
+
+  it('a package the app DECLARES but never installed reports the INSTALL remedy, not an absence', async () => {
+    const root = makeApp(APP_ONLY, { declare: true, install: false });
+
+    const err = await Serve.importConfigPlugin(APP_ONLY, root).catch((e: unknown) => e as Error);
+
+    expect(err).toBeInstanceOf(Error);
+    expect(err.message).toContain(`Failed to import plugin '${APP_ONLY}':`);
+    // The app asked for it, so re-reading the manifest is not the remedy.
+    expect(err.message).toContain('DECLARES it');
+    expect(err.message).toMatch(/INSTALL problem, not a declaration problem/);
+  });
+});
+
+/**
+ * Triage ② on #10908: host-anchoring changes the user-facing text a missing
+ * plugin produces — the wrapper now nests `createHostImporter`'s #4719 remedy.
+ * That is a better diagnostic, but it is VISIBLE, so it is pinned here as a
+ * chosen behaviour rather than left to drift.
+ */
+describe('os serve → the missing-plugin diagnostic is a chosen text (#10908 / #4719)', () => {
+  it('names the plugin, then tells the author to DECLARE it in that app', async () => {
+    const root = makeApp(APP_ONLY, { declare: false, install: false });
+
+    const err = await Serve.importConfigPlugin(APP_ONLY, root).catch((e: unknown) => e as Error);
+
+    expect(err).toBeInstanceOf(Error);
+    // The wrapper `serve` has always put around a failed plugin load.
+    expect(err.message).toContain(`Failed to import plugin '${APP_ONLY}':`);
+    // …now carrying the #4719 remedy instead of a bare "Cannot find package".
+    expect(err.message).toMatch(/Declare it in that app's package\.json/);
+    expect(err.message).toContain(root);
+    // The gate's own reasoning survives into what the user reads: being merely
+    // reachable is refused ON PURPOSE, so nobody "fixes" this with NODE_PATH.
+    expect(err.message).toMatch(/merely REACHABLE is not enough/);
+  });
+});
+
+/**
+ * The two branches that exist so this card could not take working deployments
+ * away. Both were MEASURED against `createHostImporter` before being written:
+ * its pass-through and its undeclared fallback both re-enter `import()` from
+ * inside `@objectstack/types`, which moves the resolution base.
+ */
+describe('os serve → the branches that must NOT move (#10908 supersedes nothing)', () => {
+  it('keeps this CLI as the resolver for a package the app does not declare', async () => {
+    // `chalk` is declared by packages/cli and by no fixture app. Today's bare
+    // `import()` finds it; through the host importer's fallback — which resolves
+    // from `@objectstack/types` — it does not. An app that writes
+    // `plugins: ['@objectstack/plugin-auth']` without declaring it boots today,
+    // and this is the assertion that says it still does.
+    const root = makeApp(APP_ONLY, { declare: false, install: false });
+
+    const mod = await Serve.importConfigPlugin('chalk', root);
+    expect(mod.default ?? mod).toBeTruthy();
+  });
+
+  it('keeps a RELATIVE specifier anchored to serve.ts, not to @objectstack/types', async () => {
+    const root = makeApp(APP_ONLY, { declare: false, install: false });
+    const missing = './__no_such_config_plugin_10908__.js';
+
+    const err = await Serve.importConfigPlugin(missing, root).catch((e: unknown) => e as Error);
+
+    expect(err).toBeInstanceOf(Error);
+    expect(err.message).toContain(`Failed to import plugin '${missing}':`);
+    // The base is what this pins: the directory holding serve.ts. Routing this
+    // spelling through the host importer would silently re-base it under
+    // `@objectstack/types/dist/`, which is the regression this branch prevents.
+    // Neither base is the served app's root — whether a relative entry SHOULD
+    // resolve there is #10944, deliberately left open by this card.
+    expect(err.message).toContain('commands');
+    expect(err.message).not.toContain('types/dist');
+  });
+
+  it('loads an absolute path and a file:// URL unchanged (base-independent spellings)', async () => {
+    const root = makeApp(APP_ONLY, { declare: false, install: false });
+    const file = join(root, 'local-plugin.js');
+    writeFileSync(file, 'export default { name: "app-local-plugin" };\n');
+
+    for (const spelling of [file, pathToFileURL(file).href]) {
+      const mod = await Serve.importConfigPlugin(spelling, root);
+      expect(mod.default).toEqual({ name: 'app-local-plugin' });
+    }
+  });
+});
+
+describe('os serve → the config-plugin load stays wired to the helper', () => {
+  const SERVE_SOURCE = readFileSync(new URL('./serve.ts', import.meta.url), 'utf8');
+
+  it('the boot loop calls the helper and no longer bare-imports the entry', () => {
+    expect(SERVE_SOURCE).toContain('await Serve.importConfigPlugin(plugin, hostRoot)');
+    // The exact shape the card was filed against — it must not come back.
+    expect(SERVE_SOURCE).not.toMatch(/const imported = await import\(plugin\)/);
+  });
+
+  it('the declaration decides the resolver, so the gate keeps its say (#4719)', () => {
+    // A helper that stopped consulting the declaration would still pass every
+    // behavioural test above that uses a DECLARED fixture, so pin the wiring.
+    const helper = SERVE_SOURCE.slice(SERVE_SOURCE.indexOf('static async importConfigPlugin'));
+    expect(helper).toContain('isDeclaredByHost(pluginSpecifier, root)');
+    expect(helper).toContain('importFromHost(pluginSpecifier, root)');
+  });
+});

packages/cli/src/commands/serve.ts

@@ -54,6 +54,7 @@ import {
   createHostImporter,
   hostImportFailureKind,
   isDeclaredByHost,
+  packageNameFromSpecifier,
   readHostDeclaration,
   type HostImporter,
 } from '@objectstack/types/node';
@@ -393,6 +394,98 @@ export default class Serve extends Command {
     return 'off';
   }
 
+  /**
+   * Load one `plugins: [...]` entry of the served app's own config that is
+   * written as a STRING (#10908).
+   *
+   * This is the most app-owned specifier in the whole file — it is supplied by
+   * the app being served, and `plugins: [...]` is THE documented way to extend a
+   * deployment. It used to be loaded with a bare `import()`, which Node ESM
+   * resolves against the IMPORTER's realpath: this CLI's. So an app that writes
+   * `plugins: ['@acme/my-plugin']` and DECLARES `@acme/my-plugin` in its own
+   * package.json could only be served where that package happened to be hoisted
+   * somewhere the CLI could see it — true in a dev checkout, false in a real
+   * distribution layout. Same mechanism as cloud#1013 and #10645, but on the
+   * surface users are explicitly told to use.
+   *
+   * ── Why this is three branches and not `await importFromHost(specifier)` ────
+   *
+   * The obvious repair is to hand every specifier to `importFromHost`. MEASURED,
+   * that is NOT a superset of what this line does today — in two ways, both of
+   * which would take working deployments away:
+   *
+   *   1. A RELATIVE specifier changes base. `createHostImporter` passes a
+   *      non-package specifier through to an `import()` that physically lives in
+   *      `@objectstack/types`, and ESM resolves a relative specifier against the
+   *      module CONTAINING the call — so `'./local-plugin.js'` would resolve
+   *      against `@objectstack/types/dist/` instead of this file's directory.
+   *      Neither base is the served app's root, so no relative spelling works
+   *      the way an author would expect either way; #10944 carries that
+   *      question, and this branch is why the answer stays open rather than
+   *      being decided by a silent re-base here.
+   *   2. An UNDECLARED bare name changes base the same way, and this one bites.
+   *      `createHostImporter`'s fallback is documented as "the importing
+   *      package's own resolution", but the import it falls back to also lives
+   *      in `@objectstack/types`, which under a pnpm-isolated layout can see
+   *      only `@objectstack/types`'s own dependencies. Measured from an app that
+   *      declares nothing: `@objectstack/plugin-auth` and `@objectstack/plugin-
+   *      audit` resolve from THIS package and fail through the host importer.
+   *      An app that writes `plugins: ['@objectstack/plugin-auth']` without
+   *      declaring it — a spelling this repo's own fixtures use — boots today
+   *      and would stop booting. The helper's own docblock claims the opposite
+   *      ("falls back to the importing package's own resolution"); that text is
+   *      wrong, and #10943 carries the fix. Until it lands, a caller that needs
+   *      its own resolution has to ask the declaration itself, as below.
+   *
+   * So the declaration is what selects the resolver, exactly as #4719 says it
+   * should, and each branch keeps the resolution it already had:
+   *
+   *   • not a package name (path, `file://` URL, `node:` builtin) → unchanged;
+   *     nothing a package.json can declare, so the gate has no opinion.
+   *   • DECLARED by the served app → `importFromHost`: the app's own copy wins.
+   *     This is the repair — the whole card is this branch.
+   *   • UNDECLARED → this CLI's own resolution, byte-identical to the bare
+   *     `import()` that has always been here. No app loses a plugin it does not
+   *     declare but the CLI ships.
+   *
+   * Nothing about WHICH plugins are accepted changes: this only moves where a
+   * declared one resolves FROM. The #4719 declaration gate is untouched, and no
+   * undeclared package gains a way in that it did not already have.
+   *
+   * @param pluginSpecifier The string as the app wrote it in `plugins: [...]`.
+   * @param hostRoot Root of the served app; defaults to the process CWD, the
+   * same value `serve`'s boot path computes.
+   */
+  static async importConfigPlugin(pluginSpecifier: string, hostRoot?: string): Promise<any> {
+    const root = hostRoot ?? process.cwd();
+    try {
+      // `await` inside the `try` rather than a bare `return`: a returned promise
+      // would settle OUTSIDE it and skip the diagnostic wrapper below.
+      if (packageNameFromSpecifier(pluginSpecifier) === undefined) {
+        return await import(/* webpackIgnore: true */ pluginSpecifier);
+      }
+      if (isDeclaredByHost(pluginSpecifier, root)) {
+        return await importFromHost(pluginSpecifier, root);
+      }
+      try {
+        return await import(/* webpackIgnore: true */ pluginSpecifier);
+      } catch (cliError: unknown) {
+        // Present but broken is a crash, not an absence — never reinterpret it.
+        if (!Serve.isModuleNotFoundError(cliError)) throw cliError;
+        // Undeclared AND unresolvable anywhere. Re-enter the host importer for
+        // the failure alone: it owns the #4719 "declare it in that app's
+        // package.json" remedy, and having one owner of that wording is why
+        // this does not compose the message itself.
+        return await importFromHost(pluginSpecifier, root);
+      }
+    } catch (importError: any) {
+      // The wrapper lives with the load it describes, so the composed
+      // user-facing string is testable rather than assembled at the call site
+      // (triage on #10908 requires this text be CHOSEN, not drift).
+      throw new Error(`Failed to import plugin '${pluginSpecifier}': ${importError.message}`);
+    }
+  }
+
   /**
    * Tier-gated capability tokens → the tier each one opens when listed in
    * `requires`. These have no CAPABILITY_PROVIDERS entry — their loading is
@@ -2673,12 +2766,11 @@ export default class Serve extends Command {
 
             // Resolve string references (package names)
             if (typeof plugin === 'string') {
-              try {
-                 const imported = await import(plugin);
-                 pluginToLoad = imported.default || imported;
-              } catch (importError: any) {
-                 throw new Error(`Failed to import plugin '${plugin}': ${importError.message}`);
-              }
+              // Host-anchored, NOT a bare `import()`: this specifier comes from
+              // the served app's own config, so what the app DECLARES about it is
+              // the contract (#10908). The helper carries the failure wrapper too.
+              const imported = await Serve.importConfigPlugin(plugin, hostRoot);
+              pluginToLoad = imported.default || imported;
             }
 
             // Wrap raw config objects (no init/start) into AppPlugin