Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/cli-serve-host-importer-caller-base.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
---
"@objectstack/cli": patch
---

**Fix:** `os serve`'s host importer now states its own resolution base, so a package the served app does not declare resolves from `packages/cli` instead of from `@objectstack/types` (#11157).

`createHostImporter` has two legs. The **declared** leg resolves out of the served app's `node_modules` (#4719; #11185 fixed *which* app that is). The **undeclared** leg falls back to "the importing package's own resolution" — and which package that is depends entirely on where the `import()` is physically written, because Node ESM resolves a bare specifier against the module containing the call. #10943 turned that into an explicit parameter, `options.fallbackImport`, so a caller can hand in its own `import()`. `@objectstack/verify` (`bootStack`) and the `packages/qa/dogfood` enterprise probe both pass theirs; `serve`'s `importFromHost` did not, so it advertised the CLI's resolution and actually used `@objectstack/types`', which under a pnpm-isolated layout sees only `@objectstack/spec`.

**Measured accept-set delta**: the undeclared fallback now reaches exactly what `packages/cli` itself declares, and nothing else. Re-measured with `import.meta.resolve` from a probe in each package — `chalk`, `@objectstack/plugin-auth` and `@objectstack/plugin-audit` resolve from `packages/cli` and not from `@objectstack/types`; every specifier `serve` itself routes through the helper (`@objectstack/service-cluster` and its drivers, `@objectstack/service-i18n`, `@objectstack/organizations`, `@objectstack/service-ai`, `@objectstack/service-ai-studio`) resolves from **neither**, which is why this was harmless in every shape that ships today. The #4719 declaration gate on the declared leg is untouched: a package that is merely reachable is still refused, and no app gains a way to load something it has not declared.

**Two user-visible consequences.** A `plugins: [...]` entry naming a package the app does not declare but the CLI ships now resolves through the host importer rather than a separate local `import()` — same module, one attempt instead of two. And the undeclared-package diagnostic drops its "the caller did not pass `fallbackImport`" note, which `@objectstack/types` emits only for callers that withhold their base; the note existed so this gap would report itself, and it has now been closed rather than silenced.

`Serve.importConfigPlugin`'s three-branch shape collapses to two in the same change. The undeclared branch kept a local `import()` *because* the helper's fallback resolved from the wrong package; with the base threaded, that branch and the re-entry branch are the same call, so the declaration is read once — by `readHostDeclaration` inside the helper — instead of being asked there and again here. Behaviour was measured case by case first: the app's declared copy still wins, a declared-but-uninstalled package still reports the install remedy, a package present-but-throwing still propagates as a crash (both paths gate on the one shared `isModuleNotFoundError`), and a package resolvable nowhere still produces the #4719 "declare it in that app's package.json" text.
37 changes: 25 additions & 12 deletions packages/cli/src/commands/serve-cluster-host-resolution.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -278,18 +278,31 @@ 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, 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)',
// The app's own `plugins: [...]` config entries, routed through
// `Serve.importConfigPlugin` (#10908). ONE bare `import()` site remains there,
// and it is the reason this list exists rather than a hole in it: the
// specifier is not a package name at all (an absolute path, a `file://` URL, a
// `node:` builtin), so nothing a package.json can declare, and every one of
// those spellings means the same module from every base.
//
// It used to be TWO. The second was the UNDECLARED branch, which kept a local
// `import()` because the host importer's fallback resolved from
// `@objectstack/types` rather than from this CLI. #11157 threaded the base
// (`fallbackImport`), which made that branch identical to the helper's own
// fallback, and it was collapsed into `importFromHost`. Pinned behaviourally,
// not by this comment, in `serve-config-plugin-host-resolution.test.ts` and
// `serve-host-fallback-base.test.ts`.
pluginSpecifier: 'the non-package branch: an absolute path, a file:// URL or a node: builtin (#10908)',
// `importFromHost`'s own `fallbackImport` (#11157) — the caller base
// `createHostImporter` resolves everything the served app does NOT declare
// from. It is a bare `import()` on purpose and it MUST be written in this
// file: ESM resolves a bare specifier against the module containing the call,
// so moving it anywhere else moves the base, which is the whole defect. Its
// parameter is the helper's argument, so no scan can know the specifier —
// and no scan needs to: this site is not a load of any particular package,
// it is the resolution base every other undeclared load is handed.
fallbackSpecifier:
"importFromHost's caller base — the CLI's own resolver, handed to createHostImporter (#11157)",
};

/**
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,10 +15,18 @@ import Serve from './serve.js';
* 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
* The repair moves ONLY the declared case. These tests pin every branch the
* method has, including the ones that exist to keep behaviour a naive
* `await importFromHost(specifier)` would have taken away — see
* `Serve.importConfigPlugin` for the measurements.
*
* ⚠️ #11157 collapsed the shape from three branches to two: once `importFromHost`
* hands `createHostImporter` this file's own resolver (`fallbackImport`), the
* helper's undeclared leg IS the local `import()` the undeclared branch used to
* make, so that branch and the re-entry branch became one call. Every assertion
* below is unchanged and still describes real behaviour — that is what made the
* collapse safe to take. The one that had to move is the structural one at the
* bottom: the declaration read now has a single owner inside the helper.
*/

const roots: string[] = [];
Expand DownExpand Up@@ -143,11 +151,16 @@ describe('os serve → the missing-plugin diagnostic is a chosen text (#10908 /
*/
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.
// `chalk` is declared by packages/cli and by no fixture app. An app that
// writes `plugins: ['@objectstack/plugin-auth']` without declaring it boots
// today, and this is the assertion that says it still does.
//
// ⚠️ This assertion is why #11157 had to land BEFORE the branch collapse and
// not after. It used to be kept true by a local `import()` here; it is now
// kept true by `importFromHost` carrying this file's base. Take the base
// away and this line goes red — measured, and pinned again from the other
// side (with the no-base control beside it) in
// `serve-host-fallback-base.test.ts`.
const root = makeApp(APP_ONLY, { declare: false, install: false });

const mod = await Serve.importConfigPlugin('chalk', root);
Expand DownExpand Up@@ -204,10 +217,24 @@ describe('os serve → the config-plugin load stays wired to the helper', () =>
});

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.
// A helper that reached the app's copy by some route OTHER than the host
// importer would still pass the behavioural tests above, so pin the wiring.
//
// ⚠️ This used to also require `isDeclaredByHost(pluginSpecifier, root)` in
// this method. #11157 removed that call — not the check. `importFromHost`
// now carries this file's resolution base, which made the local undeclared
// branch identical to the helper's own fallback, so the declaration is read
// exactly once, by `readHostDeclaration` inside `createHostImporter`. Asking
// the same question twice in two places is the fork Prime Directive #12
// exists to prevent; requiring the second copy HERE would have pinned it.
// The single owner is pinned in `packages/types/src/node.test.ts`.
const helper = SERVE_SOURCE.slice(SERVE_SOURCE.indexOf('static async importConfigPlugin'));
expect(helper).toContain('isDeclaredByHost(pluginSpecifier, root)');
expect(helper).toContain('importFromHost(pluginSpecifier, root)');
const body = helper.slice(0, helper.indexOf('\n }\n'));
expect(body).toContain('importFromHost(pluginSpecifier, root)');
// The resolver is never chosen by a second, local reading of the manifest.
expect(body).not.toContain('isDeclaredByHost');
// …and the entry is never handed to a bare `import()` once it names a
// package: that is the #10908 defect itself.
expect(body).not.toMatch(/if \(isDeclaredByHost/);
});
});
136 changes: 136 additions & 0 deletions packages/cli/src/commands/serve-host-fallback-base.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* `serve` hands `createHostImporter` its OWN resolution base (#11157) — the
* half of that card an in-process test can honestly measure.
*
* ── The defect ───────────────────────────────────────────────────────────
*
* `createHostImporter`'s UNDECLARED leg falls back to "the importing package's
* own resolution", and which package that is depends on where the `import()` is
* physically WRITTEN: Node ESM resolves a bare specifier against the module
* containing the call. #10943 made it an explicit parameter,
* `options.fallbackImport`. `@objectstack/verify` and the `packages/qa/dogfood`
* probe pass theirs; `serve`'s `importFromHost` did not, so its fallback
* resolved from `@objectstack/types` — which under a pnpm-isolated layout sees
* only `@objectstack/spec`.
*
* ── ⛔ DO NOT ASSERT RESOLUTION IN THIS FILE — it cannot fail here ──────────
*
* `@objectstack/types` is a LINKED workspace package, so Vite processes it as
* source instead of externalising it and rewrites the `import()` inside
* `packages/types/dist/node.mjs` to its own resolver — which resolves from the
* vitest root, `packages/cli`. MEASURED in this checkout: an in-process
* `createHostImporter(appRoot)('chalk')`, with NO caller base at all, RESOLVES
* under vitest and THROWS `Cannot find package 'chalk'` under Node.
*
* Under vitest the two bases ARE the same base. A "before/after" written here
* is green both ways, and — worse — so is the anti-vacuity control beside it,
* so nothing reports that the pin stopped measuring anything. The resolution
* pins therefore live in `test/serve-host-fallback-base.e2e.test.ts`, which
* spawns a real Node process. This is also why the `chalk` assertion in
* `serve-config-plugin-host-resolution.test.ts` is a behaviour statement and
* not the measurement of this card.
*
* ── What DOES fail here, and why it is not a proxy ─────────────────────────
*
* `undeclaredMessage` (`@objectstack/types/node`) composes two different texts
* depending on `fallbackImport !== undefined`. That branch is pure logic: no
* resolver touches it, so vitest cannot flatten it. Before this card `serve` got
* the text that tells the reader the caller withheld its base — a sentence this
* card makes false. Moving the branch is part of the fix, not evidence about it.
*
* This file reads only `serve.ts` and `package.json` from its own package.
*/

import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { dirname, join, resolve } from 'node:path';
import { fileURLToPath } from 'node:url';
import { afterAll, describe, expect, it } from 'vitest';
import Serve from './serve.js';

const HERE = dirname(fileURLToPath(import.meta.url));

/** `packages/cli/package.json` — this package's OWN declared surface. */
const CLI_MANIFEST = JSON.parse(
readFileSync(resolve(HERE, '..', '..', 'package.json'), 'utf8'),
) as { dependencies?: Record<string, string> };

/** Declared by `packages/cli`, resolvable from it, NOT from `@objectstack/types`. */
const CLI_DECLARED = 'chalk';

/** A name no package anywhere can satisfy, so no result can be an accident. */
const NOWHERE = '@os-fixture/host-fallback-base-probe';

const roots: string[] = [];
afterAll(() => {
for (const r of roots) rmSync(r, { recursive: true, force: true });
});

/** A served app that declares nothing. */
function makeApp(): string {
const root = mkdtempSync(join(tmpdir(), 'os-fallback-base-'));
roots.push(root);
writeFileSync(
join(root, 'package.json'),
JSON.stringify({ name: 'fixture-app', version: '1.0.0', type: 'module' }),
);
return root;
}

describe('os serve → the undeclared diagnostic takes the caller-supplied-base branch', () => {
it('names the APP (#11185) and no longer says the caller withheld its base (#11157)', async () => {
const root = makeApp();

const err = (await Serve.importConfigPlugin(NOWHERE, root).catch((e: unknown) => e)) as Error;

expect(err).toBeInstanceOf(Error);
expect(err.message).toContain(`Failed to import plugin '${NOWHERE}':`);
expect(err.message).toContain(`Cannot find package '${NOWHERE}'`);
// #11185's text: the app being served, never the process CWD.
expect(err.message).toContain(`host app: ${root}`);
expect(err.message).not.toContain(`host app: ${process.cwd()}`);
// #11157: the other branch of the same message. `serve` supplies its base
// now, so the note that exists to report the gap must not be printed.
expect(err.message).not.toContain('the caller did not pass `fallbackImport`');
// The #4719 remedy the helper owns is unchanged — this card moved a base,
// not the declaration contract.
expect(err.message).toMatch(/Declare it in that app's package\.json/);
expect(err.message).toMatch(/merely REACHABLE is not enough/);
});
});

describe('os serve → the base is wired at the single importer construction', () => {
const SERVE_SOURCE = readFileSync(resolve(HERE, 'serve.ts'), 'utf8');

it('the specifier the e2e pin uses is one packages/cli DECLARES', () => {
// Guards the e2e against the manifest changing under it: if `chalk` stopped
// being a declared dependency, that pin could still pass by workspace
// hoisting and would no longer measure the accept-set this card widens.
expect(Object.keys(CLI_MANIFEST.dependencies ?? {})).toContain(CLI_DECLARED);
});

it('passes fallbackImport where the importer is built', () => {
// `serve-cluster-host-resolution.test.ts` pins that there is exactly ONE
// `createHostImporter(` in this file. This pins that the one carries a base.
expect(SERVE_SOURCE).toMatch(/createHostImporter\(hostRoot,\s*\{/);
expect(SERVE_SOURCE).toMatch(
/fallbackImport: \(fallbackSpecifier\) => import\(\/\* webpackIgnore: true \*\/ fallbackSpecifier\)/,
);
// A URL/string base would compile and silently ignore the parent argument —
// measured on Node v22 and recorded in `@objectstack/types/node`. It is not
// a spelling variant of the line above; it is the phantom fix of this card.
expect(SERVE_SOURCE).not.toMatch(/fallbackImport:\s*(?:import\.meta\.url|['"`])/);
});

it('the config-plugin path no longer re-implements the declaration read', () => {
// Collapsed in #11157: the undeclared branch's local `import()` and the
// re-entry branch became the same call once the base was threaded, so the
// declaration is read once, by `readHostDeclaration` inside the helper.
const helper = SERVE_SOURCE.slice(SERVE_SOURCE.indexOf('static async importConfigPlugin'));
const body = helper.slice(0, helper.indexOf('\n }\n'));
expect(body).toContain('importFromHost(pluginSpecifier, root)');
expect(body).not.toContain('isDeclaredByHost');
});
});
Loading
Loading