Skip to content

Commit 53428b8

Browse files
Elon Muskclaude
andauthored
fix(cli): resolve serve's cluster driver imports from the host app (#10645) (#10765)
`os serve` reached `@objectstack/service-cluster` and its driver through a bare dynamic `import()`. Node ESM resolves a bare specifier against the importer's own realpath, which for the CLI is inside the framework workspace, so the cluster packages -- shipped with a distribution and declared by the APP -- were invisible to it. Measured on a published EE image: the CLI's `node_modules/@objectstack/` held 48 packages and neither cluster one, so `OS_CLUSTER_DRIVER=redis` died at boot with `Cannot find package '@objectstack/service-cluster'` while app-side code loaded it fine. Both loads now go through `createHostImporter`, the host-anchored importer `serve` already uses for its organizations and capability loads. That fixes the class rather than these two packages: any app-declared optional package the CLI advertises it will load now resolves as the app declares it, which declaring the packages as CLI dependencies would not have done. The helper is hoisted to the top of the boot sequence. It sits in one long boot function, so a load placed above it silently falls back to bare resolution -- the same defect that previously cost the enterprise organizations load (cloud#1013). A new test pins both the boundary crossing and that ordering. No change to what `serve` accepts or refuses: an undeclared package is still refused by the unchanged #4719 declaration gate. Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r Co-authored-by: Claude <noreply@anthropic.com>
1 parent 63d603e commit 53428b8

3 files changed

Lines changed: 231 additions & 32 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
Fix `os serve` failing to boot with `OS_CLUSTER_DRIVER=redis` when the app
6+
declares `@objectstack/service-cluster` (#10645). The cluster gate and its
7+
driver were reached through a bare dynamic `import()`, which Node ESM resolves
8+
against the CLI's own realpath — inside the framework workspace — so packages
9+
installed under the host app were invisible to it and boot died with
10+
`Cannot find package '@objectstack/service-cluster'`. Both loads now go through
11+
the host-anchored importer `serve` already uses for its other optional and
12+
enterprise packages, so any package the app declares resolves the way the app
13+
declares it. The host importer is now defined at the top of the boot sequence
14+
rather than partway down, which is what made these two loads fall back to bare
15+
resolution in the first place. No change to what `serve` accepts or refuses:
16+
an undeclared package is still refused by the same declaration gate.
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `os serve` loads the cluster gate and its driver AS THE HOST APP DECLARES
5+
* THEM, not from the CLI's own `node_modules`.
6+
*
7+
* ── The defect ───────────────────────────────────────────────────────────
8+
*
9+
* Measured on a published EE image: with `OS_CLUSTER_DRIVER=redis` set, boot
10+
* died with
11+
*
12+
* Cannot find package '@objectstack/service-cluster' imported from
13+
* /repo/objectstack/packages/cli/dist/commands/serve.js
14+
*
15+
* The CLI's own `node_modules/@objectstack/` held 48 packages and NEITHER
16+
* cluster package; both were installed only under the app, which declares them.
17+
* `serve.ts` reached them through a bare dynamic `import()`, and Node ESM
18+
* resolves a bare specifier against the IMPORTER's realpath — the CLI's, inside
19+
* the framework workspace. So the one hop that could not work was CLI to app,
20+
* while app-side code loaded the very same packages fine.
21+
*
22+
* ── Why this is not fixed by declaring the packages ──────────────────────
23+
*
24+
* Adding `@objectstack/service-cluster*` to `packages/cli`'s dependencies would
25+
* silence this driver and leave the class open: the next app-declared optional
26+
* package the CLI advertises it will load breaks identically, a third-party
27+
* cluster driver can never work, and the open-core CLI would take a static
28+
* dependency on packages that ship with a distribution — the exact coupling the
29+
* non-literal specifier in `serve.ts` exists to avoid. The fix is to resolve
30+
* from the host app, which is what `createHostImporter` already does for the
31+
* organizations / capability loads further down `serve`.
32+
*
33+
* ── What is pinned here ──────────────────────────────────────────────────
34+
*
35+
* 1. The BOUNDARY, behaviourally and hermetically: a package that exists only
36+
* in a host app's `node_modules` is invisible to a bare import from this
37+
* file (which sits in `packages/cli`, the same resolution base as the
38+
* shipped `dist/commands/serve.js`) and IS loadable through the host
39+
* importer. The fixture package is synthetic on purpose — the contract is
40+
* "any app-declared optional package", not "these two cluster packages", and
41+
* a synthetic one needs nothing built.
42+
*
43+
* 2. The ORDERING, by source scan: `importFromHost` must be defined ABOVE the
44+
* cluster block. This is the half that actually regressed, twice — the
45+
* helper is a `const` in one long boot function, so a load placed above it
46+
* is not a compile error, it is a silent fall-back to bare resolution. The
47+
* first time it cost the enterprise organizations load (cloud#1013); the
48+
* second time it cost EE multi-node boot outright.
49+
*
50+
* The source scan reads `serve.ts` from THIS package, so no cross-package test
51+
* input is declared or needed.
52+
*/
53+
54+
import { describe, it, expect } from 'vitest';
55+
import { mkdtempSync, mkdirSync, writeFileSync, readFileSync } from 'node:fs';
56+
import { tmpdir } from 'node:os';
57+
import { dirname, join, resolve } from 'node:path';
58+
import { fileURLToPath } from 'node:url';
59+
import { createHostImporter } from '@objectstack/types/node';
60+
61+
const HERE = dirname(fileURLToPath(import.meta.url));
62+
63+
/** `packages/cli/src/commands/serve.ts` — same package, no escaping read. */
64+
const SERVE_SOURCE = readFileSync(resolve(HERE, 'serve.ts'), 'utf8');
65+
66+
/**
67+
* A host app that DECLARES an optional package and carries it in its own
68+
* `node_modules` — the shape of every EE app that declares
69+
* `@objectstack/service-cluster`. Nothing here is built or installed: the
70+
* package is three files written to a temp dir.
71+
*/
72+
function makeHostApp(pkgName: string, declare: boolean): string {
73+
const root = mkdtempSync(join(tmpdir(), 'os-host-app-'));
74+
writeFileSync(
75+
join(root, 'package.json'),
76+
JSON.stringify({
77+
name: 'fixture-host-app',
78+
version: '1.0.0',
79+
type: 'module',
80+
...(declare ? { dependencies: { [pkgName]: '1.0.0' } } : {}),
81+
}),
82+
);
83+
const pkgDir = join(root, 'node_modules', ...pkgName.split('/'));
84+
mkdirSync(pkgDir, { recursive: true });
85+
writeFileSync(
86+
join(pkgDir, 'package.json'),
87+
JSON.stringify({ name: pkgName, version: '1.0.0', type: 'module', main: 'index.js' }),
88+
);
89+
// The marker export stands in for `checkMultiNodeAllowed`: proof the module
90+
// that loaded is the app's copy, not something the CLI happened to resolve.
91+
writeFileSync(join(pkgDir, 'index.js'), 'export const loadedFrom = "host-app";\n');
92+
return root;
93+
}
94+
95+
describe('os serve → app-declared optional package resolution', () => {
96+
// A name no workspace package can satisfy, so a pass cannot come from the
97+
// CLI's own node_modules by accident.
98+
const PKG = '@os-fixture/cluster-driver-probe';
99+
100+
it('reproduces the asymmetry: an app-only package is invisible to a bare import', async () => {
101+
// This file resolves from `packages/cli`, exactly as `dist/commands/serve.js`
102+
// does — the failing hop the EE image measured.
103+
const bare: string = PKG;
104+
await expect(import(bare)).rejects.toMatchObject({
105+
code: expect.stringMatching(/MODULE_NOT_FOUND|ERR_MODULE_NOT_FOUND/),
106+
});
107+
});
108+
109+
it('crosses the boundary: the host importer loads what the app declares', async () => {
110+
const hostRoot = makeHostApp(PKG, true);
111+
const mod = await createHostImporter(hostRoot)(PKG);
112+
expect(mod.loadedFrom).toBe('host-app');
113+
});
114+
115+
it('still refuses a package the app does not declare (the gate is unchanged)', async () => {
116+
// Present in the app's node_modules but absent from its package.json.
117+
// Reachability must not substitute for declaration (#4719) — this fix moves
118+
// where a module is resolved FROM, it does not widen what serve accepts.
119+
const hostRoot = makeHostApp(PKG, false);
120+
await expect(createHostImporter(hostRoot)(PKG)).rejects.toMatchObject({
121+
code: 'MODULE_NOT_FOUND',
122+
});
123+
});
124+
});
125+
126+
describe('os serve → cluster block source shape', () => {
127+
it('loads the cluster gate and driver through the host importer', () => {
128+
expect(SERVE_SOURCE).toMatch(/await importFromHost\(__clusterPkg\)/);
129+
expect(SERVE_SOURCE).toMatch(
130+
/await importFromHost\(`@objectstack\/service-cluster-\$\{__clusterDriver\}`\)/,
131+
);
132+
});
133+
134+
it('never reaches the cluster packages through a bare dynamic import', () => {
135+
// The exact regression, in both spellings the block used.
136+
expect(SERVE_SOURCE).not.toMatch(/await import\(__clusterPkg\)/);
137+
expect(SERVE_SOURCE).not.toMatch(/await import\(`@objectstack\/service-cluster-/);
138+
});
139+
140+
it('defines importFromHost ABOVE the cluster block that consumes it', () => {
141+
const definition = SERVE_SOURCE.indexOf('const importFromHost = createHostImporter(');
142+
const clusterUse = SERVE_SOURCE.indexOf('await importFromHost(__clusterPkg)');
143+
144+
expect(definition, 'importFromHost definition not found — was it renamed?').toBeGreaterThan(-1);
145+
expect(clusterUse, 'cluster gate no longer loads via importFromHost').toBeGreaterThan(-1);
146+
147+
// `const` in one long boot function: a use above the definition is a
148+
// temporal-dead-zone throw at boot, and the load it guards is exactly the
149+
// one that must not fall back to bare resolution.
150+
expect(
151+
definition,
152+
'importFromHost is defined AFTER the cluster block. That is the defect this file '
153+
+ 'pins: every optional load placed above the helper silently resolves from the '
154+
+ "CLI's own node_modules instead of the host app's. Hoist the helper.",
155+
).toBeLessThan(clusterUse);
156+
});
157+
158+
it('keeps exactly one host-importer definition, so hoisting cannot fork it', () => {
159+
const definitions = [...SERVE_SOURCE.matchAll(/const importFromHost\s*=/g)];
160+
expect(definitions).toHaveLength(1);
161+
});
162+
});

‎packages/cli/src/commands/serve.ts‎

Lines changed: 53 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1409,6 +1409,43 @@ export default class Serve extends Command {
14091409
// keys off it too (#4012).
14101410
const loggerConfig = { level: bootLogLevel };
14111411

1412+
// Host-app package resolution — shared by every optional / enterprise
1413+
// package loaded from here down.
1414+
//
1415+
// Node ESM resolves a bare `import(pkg)` against the IMPORTER's own
1416+
// realpath. The CLI is reached through a workspace/`link:` dependency, so
1417+
// that realpath is inside the FRAMEWORK workspace: a bare import can only
1418+
// see what the framework itself installed. A package supplied by the app
1419+
// being served — a cloud-private one such as `@objectstack/organizations`,
1420+
// or anything a customer installs into their own project — is invisible
1421+
// to it no matter what the host app declares. Resolve from the host root
1422+
// instead; the CLI's own resolution stays as the fallback for the
1423+
// framework-owned packages the CLI depends on.
1424+
//
1425+
// #4719: "resolve from the host root" now means "resolve what the host
1426+
// root DECLARES". The host lookup was a CJS require, CJS honours
1427+
// NODE_PATH, and the pnpm bin shim exports NODE_PATH pointing at the
1428+
// hoisted workspace store — so anything transitively reachable from
1429+
// anywhere in the workspace resolved as if the app had declared it, and
1430+
// whether the D5 wall below fired came down to whether `serve` was reached
1431+
// through that shim. The declaration is the contract; reachability is not.
1432+
//
1433+
// Defined HERE, at the TOP of the boot sequence, because the very first
1434+
// optional package `serve` loads is the cluster gate a few lines below.
1435+
// This helper has now been hoisted twice for the same reason, which is the
1436+
// point worth keeping: every load placed ABOVE it silently falls back to a
1437+
// bare import and can only see the framework's own node_modules. It first
1438+
// sat below the auth block, so the enterprise organizations load resolved
1439+
// in the framework workspace, never found the cloud-private package, and
1440+
// every walled-posture deployment hit the ADR-0093 D5 fail-fast and exited
1441+
// 1 (cloud#1013). It then sat below the cluster block, so `serve` could not
1442+
// load an app-declared `@objectstack/service-cluster*` at all and EE
1443+
// multi-node boot died outright on `OS_CLUSTER_DRIVER=redis`. A new
1444+
// optional load added above this line reintroduces the same defect a third
1445+
// time — put it below, or hoist this further and say why here.
1446+
const hostRoot = process.cwd();
1447+
const importFromHost = createHostImporter(hostRoot);
1448+
14121449
// Cluster wiring: env-driven driver selection (mirrors OS_DATABASE_URL).
14131450
// The remote driver self-registers on import; import it dynamically so it
14141451
// works in BOTH config-boot and compiled-artifact mode. Open-core ships
@@ -1422,8 +1459,17 @@ export default class Serve extends Command {
14221459
// single-node rather than fail — multi-node is an add-on, never brick.
14231460
// Dynamic, non-literal specifier so the CLI does not statically depend
14241461
// on the cluster package (mirrors the remote-driver import below).
1462+
//
1463+
// Loaded through `importFromHost`, NOT a bare `import()`: the cluster
1464+
// packages ship with a distribution and are declared by the APP, so they
1465+
// live in the app's node_modules, while a bare import resolves against
1466+
// the CLI's own realpath and can only see the framework's. Measured on
1467+
// the EE image: the CLI's `node_modules/@objectstack/` held 48 packages
1468+
// and neither cluster one, so this line threw `Cannot find package
1469+
// '@objectstack/service-cluster'` and took the whole boot down — while
1470+
// app-side code loaded the very same package fine.
14251471
const __clusterPkg: string = '@objectstack/service-cluster';
1426-
const { checkMultiNodeAllowed } = (await import(__clusterPkg)) as {
1472+
const { checkMultiNodeAllowed } = (await importFromHost(__clusterPkg)) as {
14271473
checkMultiNodeAllowed: (requested?: number) => MultiNodeGateVerdict;
14281474
};
14291475
// Ask the gate about the topology the operator actually DECLARED.
@@ -1453,7 +1499,12 @@ export default class Serve extends Command {
14531499
// above, and deliberately not a downgrade.
14541500
const __capAdvisory = formatMultiNodeCapAdvisory(__gate);
14551501
if (__capAdvisory) console.warn(__capAdvisory);
1456-
try { await import(`@objectstack/service-cluster-${__clusterDriver}`); }
1502+
// Same host-anchored resolution as the gate above — the shipped
1503+
// drivers (`-redis`, `-postgres`, …) are app-declared too. The catch
1504+
// stays deliberately silent: the driver may already have been
1505+
// registered by the loaded config, and an absent driver is a
1506+
// documented fall-back to the in-memory cluster, not a boot failure.
1507+
try { await importFromHost(`@objectstack/service-cluster-${__clusterDriver}`); }
14571508
catch { /* may already be registered by the loaded config */ }
14581509
clusterConfig = { driver: __clusterDriver, url: process.env.OS_REDIS_URL };
14591510
}
@@ -2097,36 +2148,6 @@ export default class Serve extends Command {
20972148
}
20982149
}
20992150

2100-
// Host-app package resolution — shared by every optional / enterprise
2101-
// package loaded from here down.
2102-
//
2103-
// Node ESM resolves a bare `import(pkg)` against the IMPORTER's own
2104-
// realpath. The CLI is reached through a workspace/`link:` dependency, so
2105-
// that realpath is inside the FRAMEWORK workspace: a bare import can only
2106-
// see what the framework itself installed. A package supplied by the app
2107-
// being served — a cloud-private one such as `@objectstack/organizations`,
2108-
// or anything a customer installs into their own project — is invisible
2109-
// to it no matter what the host app declares. Resolve from the host root
2110-
// instead; the CLI's own resolution stays as the fallback for the
2111-
// framework-owned packages the CLI depends on.
2112-
//
2113-
// #4719: "resolve from the host root" now means "resolve what the host
2114-
// root DECLARES". The host lookup was a CJS require, CJS honours
2115-
// NODE_PATH, and the pnpm bin shim exports NODE_PATH pointing at the
2116-
// hoisted workspace store — so anything transitively reachable from
2117-
// anywhere in the workspace resolved as if the app had declared it, and
2118-
// whether the D5 wall below fired came down to whether `serve` was reached
2119-
// through that shim. The declaration is the contract; reachability is not.
2120-
//
2121-
// Defined HERE, above the auth block, because the enterprise organizations
2122-
// load inside it needs it: this helper used to be declared *after* that
2123-
// block, so the organizations load fell back to a bare import, resolved in
2124-
// the framework workspace, never found the cloud-private package, and every
2125-
// walled-posture deployment hit the ADR-0093 D5 fail-fast and exited 1
2126-
// (cloud#1013).
2127-
const hostRoot = process.cwd();
2128-
const importFromHost = createHostImporter(hostRoot);
2129-
21302151
// 5d. Auto-register AuthPlugin (and paired Security/Audit) when the
21312152
// 'auth' tier is enabled and no auth plugin is already configured.
21322153
// The Console expects /api/v1/auth/* to be served by better-auth via

0 commit comments

Comments
 (0)