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

.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