Skip to content

Commit a4cb781

Browse files
claude[bot]zhuangjianguoclaude
authored
fix(cli): serve's host importer states its own resolution base (#11405)
* fix(cli): serve's host importer states its own resolution base `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. #10943 made it an explicit parameter; `@objectstack/verify` and the 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`. Thread the base, and collapse `Serve.importConfigPlugin`'s three-branch shape to two: the local `import()` its undeclared branch kept, and the re-entry it made for the failure text, are now the same call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR * test(cli): name @objectstack/types by specifier, not by a filesystem climb `check:cross-package-test-inputs` flagged `resolve(HERE, '../../types/dist/ node.mjs')` in the new e2e: a filesystem climb names a repo SOURCE input no turbo glob covers. The child now resolves the helper from serve.ts's own location by specifier, which is what serve.ts itself does and what the gate asks for — a bare specifier is an installed dependency, and @objectstack/cli already declares @objectstack/types, so turbo's task graph carries that edge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR --------- Co-authored-by: os-zhuang <zhuangjianguo@steedos.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent b36a6d7 commit a4cb781

6 files changed

Lines changed: 569 additions & 74 deletions
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
**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).
6+
7+
`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`.
8+
9+
**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.
10+
11+
**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.
12+
13+
`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.

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

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -278,18 +278,31 @@ const UNRESOLVABLE_BARE_IMPORTS: Record<string, string> = {
278278
// Serve.CAPABILITY_PROVIDERS — every `pkg` in that table is CLI-declared.
279279
'spec.pkg': 'Serve.CAPABILITY_PROVIDERS entries are all CLI-declared',
280280
'ex.pkg': 'CAPABILITY_PROVIDERS `extras` entries are all CLI-declared',
281-
// The app's own `plugins: [...]` config entries, now routed through
282-
// `Serve.importConfigPlugin` (#10908). Two bare `import()` sites remain there,
283-
// both reached only AFTER the declaration has been consulted, and both are the
284-
// reason this list exists rather than a hole in it:
285-
// • the specifier is not a package name at all (path, `file://`, `node:`) —
286-
// nothing a package.json can declare;
287-
// • the served app does NOT declare it, so it must resolve from this CLI,
288-
// which is exactly the pre-existing behaviour #10908 promised to keep.
289-
// The DECLARED case — the only one this card moves — goes to `importFromHost`.
290-
// Pinned behaviourally, not by this comment, in
291-
// `serve-config-plugin-host-resolution.test.ts`.
292-
pluginSpecifier: 'post-declaration branches: a path/URL, or a package the app does not declare (#10908)',
281+
// The app's own `plugins: [...]` config entries, routed through
282+
// `Serve.importConfigPlugin` (#10908). ONE bare `import()` site remains there,
283+
// and it is the reason this list exists rather than a hole in it: the
284+
// specifier is not a package name at all (an absolute path, a `file://` URL, a
285+
// `node:` builtin), so nothing a package.json can declare, and every one of
286+
// those spellings means the same module from every base.
287+
//
288+
// It used to be TWO. The second was the UNDECLARED branch, which kept a local
289+
// `import()` because the host importer's fallback resolved from
290+
// `@objectstack/types` rather than from this CLI. #11157 threaded the base
291+
// (`fallbackImport`), which made that branch identical to the helper's own
292+
// fallback, and it was collapsed into `importFromHost`. Pinned behaviourally,
293+
// not by this comment, in `serve-config-plugin-host-resolution.test.ts` and
294+
// `serve-host-fallback-base.test.ts`.
295+
pluginSpecifier: 'the non-package branch: an absolute path, a file:// URL or a node: builtin (#10908)',
296+
// `importFromHost`'s own `fallbackImport` (#11157) — the caller base
297+
// `createHostImporter` resolves everything the served app does NOT declare
298+
// from. It is a bare `import()` on purpose and it MUST be written in this
299+
// file: ESM resolves a bare specifier against the module containing the call,
300+
// so moving it anywhere else moves the base, which is the whole defect. Its
301+
// parameter is the helper's argument, so no scan can know the specifier —
302+
// and no scan needs to: this site is not a load of any particular package,
303+
// it is the resolution base every other undeclared load is handed.
304+
fallbackSpecifier:
305+
"importFromHost's caller base — the CLI's own resolver, handed to createHostImporter (#11157)",
293306
};
294307

295308
/**

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

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,18 @@ import Serve from './serve.js';
1515
* CLI could see. Green in a dev checkout, absent on a real distribution layout
1616
* (#10908; the same mechanism as cloud#1013 and #10645).
1717
*
18-
* The repair moves ONLY the declared case. These tests pin all three branches,
19-
* because two of them exist to keep behaviour that a naive
18+
* The repair moves ONLY the declared case. These tests pin every branch the
19+
* method has, including the ones that exist to keep behaviour a naive
2020
* `await importFromHost(specifier)` would have taken away — see
2121
* `Serve.importConfigPlugin` for the measurements.
22+
*
23+
* ⚠️ #11157 collapsed the shape from three branches to two: once `importFromHost`
24+
* hands `createHostImporter` this file's own resolver (`fallbackImport`), the
25+
* helper's undeclared leg IS the local `import()` the undeclared branch used to
26+
* make, so that branch and the re-entry branch became one call. Every assertion
27+
* below is unchanged and still describes real behaviour — that is what made the
28+
* collapse safe to take. The one that had to move is the structural one at the
29+
* bottom: the declaration read now has a single owner inside the helper.
2230
*/
2331

2432
const roots: string[] = [];
@@ -143,11 +151,16 @@ describe('os serve → the missing-plugin diagnostic is a chosen text (#10908 /
143151
*/
144152
describe('os serve → the branches that must NOT move (#10908 supersedes nothing)', () => {
145153
it('keeps this CLI as the resolver for a package the app does not declare', async () => {
146-
// `chalk` is declared by packages/cli and by no fixture app. Today's bare
147-
// `import()` finds it; through the host importer's fallback — which resolves
148-
// from `@objectstack/types` — it does not. An app that writes
149-
// `plugins: ['@objectstack/plugin-auth']` without declaring it boots today,
150-
// and this is the assertion that says it still does.
154+
// `chalk` is declared by packages/cli and by no fixture app. An app that
155+
// writes `plugins: ['@objectstack/plugin-auth']` without declaring it boots
156+
// today, and this is the assertion that says it still does.
157+
//
158+
// ⚠️ This assertion is why #11157 had to land BEFORE the branch collapse and
159+
// not after. It used to be kept true by a local `import()` here; it is now
160+
// kept true by `importFromHost` carrying this file's base. Take the base
161+
// away and this line goes red — measured, and pinned again from the other
162+
// side (with the no-base control beside it) in
163+
// `serve-host-fallback-base.test.ts`.
151164
const root = makeApp(APP_ONLY, { declare: false, install: false });
152165

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

206219
it('the declaration decides the resolver, so the gate keeps its say (#4719)', () => {
207-
// A helper that stopped consulting the declaration would still pass every
208-
// behavioural test above that uses a DECLARED fixture, so pin the wiring.
220+
// A helper that reached the app's copy by some route OTHER than the host
221+
// importer would still pass the behavioural tests above, so pin the wiring.
222+
//
223+
// ⚠️ This used to also require `isDeclaredByHost(pluginSpecifier, root)` in
224+
// this method. #11157 removed that call — not the check. `importFromHost`
225+
// now carries this file's resolution base, which made the local undeclared
226+
// branch identical to the helper's own fallback, so the declaration is read
227+
// exactly once, by `readHostDeclaration` inside `createHostImporter`. Asking
228+
// the same question twice in two places is the fork Prime Directive #12
229+
// exists to prevent; requiring the second copy HERE would have pinned it.
230+
// The single owner is pinned in `packages/types/src/node.test.ts`.
209231
const helper = SERVE_SOURCE.slice(SERVE_SOURCE.indexOf('static async importConfigPlugin'));
210-
expect(helper).toContain('isDeclaredByHost(pluginSpecifier, root)');
211-
expect(helper).toContain('importFromHost(pluginSpecifier, root)');
232+
const body = helper.slice(0, helper.indexOf('\n }\n'));
233+
expect(body).toContain('importFromHost(pluginSpecifier, root)');
234+
// The resolver is never chosen by a second, local reading of the manifest.
235+
expect(body).not.toContain('isDeclaredByHost');
236+
// …and the entry is never handed to a bare `import()` once it names a
237+
// package: that is the #10908 defect itself.
238+
expect(body).not.toMatch(/if \(isDeclaredByHost/);
212239
});
213240
});
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `serve` hands `createHostImporter` its OWN resolution base (#11157) — the
5+
* half of that card an in-process test can honestly measure.
6+
*
7+
* ── The defect ───────────────────────────────────────────────────────────
8+
*
9+
* `createHostImporter`'s UNDECLARED leg falls back to "the importing package's
10+
* own resolution", and which package that is depends on where the `import()` is
11+
* physically WRITTEN: Node ESM resolves a bare specifier against the module
12+
* containing the call. #10943 made it an explicit parameter,
13+
* `options.fallbackImport`. `@objectstack/verify` and the `packages/qa/dogfood`
14+
* probe pass theirs; `serve`'s `importFromHost` did not, so its fallback
15+
* resolved from `@objectstack/types` — which under a pnpm-isolated layout sees
16+
* only `@objectstack/spec`.
17+
*
18+
* ── ⛔ DO NOT ASSERT RESOLUTION IN THIS FILE — it cannot fail here ──────────
19+
*
20+
* `@objectstack/types` is a LINKED workspace package, so Vite processes it as
21+
* source instead of externalising it and rewrites the `import()` inside
22+
* `packages/types/dist/node.mjs` to its own resolver — which resolves from the
23+
* vitest root, `packages/cli`. MEASURED in this checkout: an in-process
24+
* `createHostImporter(appRoot)('chalk')`, with NO caller base at all, RESOLVES
25+
* under vitest and THROWS `Cannot find package 'chalk'` under Node.
26+
*
27+
* Under vitest the two bases ARE the same base. A "before/after" written here
28+
* is green both ways, and — worse — so is the anti-vacuity control beside it,
29+
* so nothing reports that the pin stopped measuring anything. The resolution
30+
* pins therefore live in `test/serve-host-fallback-base.e2e.test.ts`, which
31+
* spawns a real Node process. This is also why the `chalk` assertion in
32+
* `serve-config-plugin-host-resolution.test.ts` is a behaviour statement and
33+
* not the measurement of this card.
34+
*
35+
* ── What DOES fail here, and why it is not a proxy ─────────────────────────
36+
*
37+
* `undeclaredMessage` (`@objectstack/types/node`) composes two different texts
38+
* depending on `fallbackImport !== undefined`. That branch is pure logic: no
39+
* resolver touches it, so vitest cannot flatten it. Before this card `serve` got
40+
* the text that tells the reader the caller withheld its base — a sentence this
41+
* card makes false. Moving the branch is part of the fix, not evidence about it.
42+
*
43+
* This file reads only `serve.ts` and `package.json` from its own package.
44+
*/
45+
46+
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
47+
import { tmpdir } from 'node:os';
48+
import { dirname, join, resolve } from 'node:path';
49+
import { fileURLToPath } from 'node:url';
50+
import { afterAll, describe, expect, it } from 'vitest';
51+
import Serve from './serve.js';
52+
53+
const HERE = dirname(fileURLToPath(import.meta.url));
54+
55+
/** `packages/cli/package.json` — this package's OWN declared surface. */
56+
const CLI_MANIFEST = JSON.parse(
57+
readFileSync(resolve(HERE, '..', '..', 'package.json'), 'utf8'),
58+
) as { dependencies?: Record<string, string> };
59+
60+
/** Declared by `packages/cli`, resolvable from it, NOT from `@objectstack/types`. */
61+
const CLI_DECLARED = 'chalk';
62+
63+
/** A name no package anywhere can satisfy, so no result can be an accident. */
64+
const NOWHERE = '@os-fixture/host-fallback-base-probe';
65+
66+
const roots: string[] = [];
67+
afterAll(() => {
68+
for (const r of roots) rmSync(r, { recursive: true, force: true });
69+
});
70+
71+
/** A served app that declares nothing. */
72+
function makeApp(): string {
73+
const root = mkdtempSync(join(tmpdir(), 'os-fallback-base-'));
74+
roots.push(root);
75+
writeFileSync(
76+
join(root, 'package.json'),
77+
JSON.stringify({ name: 'fixture-app', version: '1.0.0', type: 'module' }),
78+
);
79+
return root;
80+
}
81+
82+
describe('os serve → the undeclared diagnostic takes the caller-supplied-base branch', () => {
83+
it('names the APP (#11185) and no longer says the caller withheld its base (#11157)', async () => {
84+
const root = makeApp();
85+
86+
const err = (await Serve.importConfigPlugin(NOWHERE, root).catch((e: unknown) => e)) as Error;
87+
88+
expect(err).toBeInstanceOf(Error);
89+
expect(err.message).toContain(`Failed to import plugin '${NOWHERE}':`);
90+
expect(err.message).toContain(`Cannot find package '${NOWHERE}'`);
91+
// #11185's text: the app being served, never the process CWD.
92+
expect(err.message).toContain(`host app: ${root}`);
93+
expect(err.message).not.toContain(`host app: ${process.cwd()}`);
94+
// #11157: the other branch of the same message. `serve` supplies its base
95+
// now, so the note that exists to report the gap must not be printed.
96+
expect(err.message).not.toContain('the caller did not pass `fallbackImport`');
97+
// The #4719 remedy the helper owns is unchanged — this card moved a base,
98+
// not the declaration contract.
99+
expect(err.message).toMatch(/Declare it in that app's package\.json/);
100+
expect(err.message).toMatch(/merely REACHABLE is not enough/);
101+
});
102+
});
103+
104+
describe('os serve → the base is wired at the single importer construction', () => {
105+
const SERVE_SOURCE = readFileSync(resolve(HERE, 'serve.ts'), 'utf8');
106+
107+
it('the specifier the e2e pin uses is one packages/cli DECLARES', () => {
108+
// Guards the e2e against the manifest changing under it: if `chalk` stopped
109+
// being a declared dependency, that pin could still pass by workspace
110+
// hoisting and would no longer measure the accept-set this card widens.
111+
expect(Object.keys(CLI_MANIFEST.dependencies ?? {})).toContain(CLI_DECLARED);
112+
});
113+
114+
it('passes fallbackImport where the importer is built', () => {
115+
// `serve-cluster-host-resolution.test.ts` pins that there is exactly ONE
116+
// `createHostImporter(` in this file. This pins that the one carries a base.
117+
expect(SERVE_SOURCE).toMatch(/createHostImporter\(hostRoot,\s*\{/);
118+
expect(SERVE_SOURCE).toMatch(
119+
/fallbackImport: \(fallbackSpecifier\) => import\(\/\* webpackIgnore: true \*\/ fallbackSpecifier\)/,
120+
);
121+
// A URL/string base would compile and silently ignore the parent argument —
122+
// measured on Node v22 and recorded in `@objectstack/types/node`. It is not
123+
// a spelling variant of the line above; it is the phantom fix of this card.
124+
expect(SERVE_SOURCE).not.toMatch(/fallbackImport:\s*(?:import\.meta\.url|['"`])/);
125+
});
126+
127+
it('the config-plugin path no longer re-implements the declaration read', () => {
128+
// Collapsed in #11157: the undeclared branch's local `import()` and the
129+
// re-entry branch became the same call once the base was threaded, so the
130+
// declaration is read once, by `readHostDeclaration` inside the helper.
131+
const helper = SERVE_SOURCE.slice(SERVE_SOURCE.indexOf('static async importConfigPlugin'));
132+
const body = helper.slice(0, helper.indexOf('\n }\n'));
133+
expect(body).toContain('importFromHost(pluginSpecifier, root)');
134+
expect(body).not.toContain('isDeclaredByHost');
135+
});
136+
});

0 commit comments

Comments
 (0)