diff --git a/.changeset/cli-serve-host-anchored-cluster-import.md b/.changeset/cli-serve-host-anchored-cluster-import.md new file mode 100644 index 0000000000..9778ee09b5 --- /dev/null +++ b/.changeset/cli-serve-host-anchored-cluster-import.md @@ -0,0 +1,16 @@ +--- +"@objectstack/cli": patch +--- + +Fix `os serve` failing to boot with `OS_CLUSTER_DRIVER=redis` when the app +declares `@objectstack/service-cluster` (#10645). The cluster gate and its +driver were reached through a bare dynamic `import()`, which Node ESM resolves +against the CLI's own realpath — inside the framework workspace — so packages +installed under the host app were invisible to it and boot died with +`Cannot find package '@objectstack/service-cluster'`. Both loads now go through +the host-anchored importer `serve` already uses for its other optional and +enterprise packages, so any package the app declares resolves the way the app +declares it. The host importer is now defined at the top of the boot sequence +rather than partway down, which is what made these two loads fall back to bare +resolution in the first place. No change to what `serve` accepts or refuses: +an undeclared package is still refused by the same declaration gate. diff --git a/packages/cli/src/commands/serve-cluster-host-resolution.test.ts b/packages/cli/src/commands/serve-cluster-host-resolution.test.ts new file mode 100644 index 0000000000..de6d58a883 --- /dev/null +++ b/packages/cli/src/commands/serve-cluster-host-resolution.test.ts @@ -0,0 +1,162 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os serve` loads the cluster gate and its driver AS THE HOST APP DECLARES + * THEM, not from the CLI's own `node_modules`. + * + * ── The defect ─────────────────────────────────────────────────────────── + * + * Measured on a published EE image: with `OS_CLUSTER_DRIVER=redis` set, boot + * died with + * + * Cannot find package '@objectstack/service-cluster' imported from + * /repo/objectstack/packages/cli/dist/commands/serve.js + * + * The CLI's own `node_modules/@objectstack/` held 48 packages and NEITHER + * cluster package; both were installed only under the app, which declares them. + * `serve.ts` reached them through a bare dynamic `import()`, and Node ESM + * resolves a bare specifier against the IMPORTER's realpath — the CLI's, inside + * the framework workspace. So the one hop that could not work was CLI to app, + * while app-side code loaded the very same packages fine. + * + * ── Why this is not fixed by declaring the packages ────────────────────── + * + * Adding `@objectstack/service-cluster*` to `packages/cli`'s dependencies would + * silence this driver and leave the class open: the next app-declared optional + * package the CLI advertises it will load breaks identically, a third-party + * cluster driver can never work, and the open-core CLI would take a static + * dependency on packages that ship with a distribution — the exact coupling the + * non-literal specifier in `serve.ts` exists to avoid. The fix is to resolve + * from the host app, which is what `createHostImporter` already does for the + * organizations / capability loads further down `serve`. + * + * ── What is pinned here ────────────────────────────────────────────────── + * + * 1. The BOUNDARY, behaviourally and hermetically: a package that exists only + * in a host app's `node_modules` is invisible to a bare import from this + * file (which sits in `packages/cli`, the same resolution base as the + * shipped `dist/commands/serve.js`) and IS loadable through the host + * importer. The fixture package is synthetic on purpose — the contract is + * "any app-declared optional package", not "these two cluster packages", and + * a synthetic one needs nothing built. + * + * 2. The ORDERING, by source scan: `importFromHost` must be defined ABOVE the + * cluster block. This is the half that actually regressed, twice — the + * helper is a `const` in one long boot function, so a load placed above it + * is not a compile error, it is a silent fall-back to bare resolution. The + * first time it cost the enterprise organizations load (cloud#1013); the + * second time it cost EE multi-node boot outright. + * + * The source scan reads `serve.ts` from THIS package, so no cross-package test + * input is declared or needed. + */ + +import { describe, it, expect } from 'vitest'; +import { mkdtempSync, mkdirSync, writeFileSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createHostImporter } from '@objectstack/types/node'; + +const HERE = dirname(fileURLToPath(import.meta.url)); + +/** `packages/cli/src/commands/serve.ts` — same package, no escaping read. */ +const SERVE_SOURCE = readFileSync(resolve(HERE, 'serve.ts'), 'utf8'); + +/** + * A host app that DECLARES an optional package and carries it in its own + * `node_modules` — the shape of every EE app that declares + * `@objectstack/service-cluster`. Nothing here is built or installed: the + * package is three files written to a temp dir. + */ +function makeHostApp(pkgName: string, declare: boolean): string { + const root = mkdtempSync(join(tmpdir(), 'os-host-app-')); + writeFileSync( + join(root, 'package.json'), + JSON.stringify({ + name: 'fixture-host-app', + version: '1.0.0', + type: 'module', + ...(declare ? { dependencies: { [pkgName]: '1.0.0' } } : {}), + }), + ); + const pkgDir = join(root, 'node_modules', ...pkgName.split('/')); + mkdirSync(pkgDir, { recursive: true }); + writeFileSync( + join(pkgDir, 'package.json'), + JSON.stringify({ name: pkgName, version: '1.0.0', type: 'module', main: 'index.js' }), + ); + // The marker export stands in for `checkMultiNodeAllowed`: proof the module + // that loaded is the app's copy, not something the CLI happened to resolve. + writeFileSync(join(pkgDir, 'index.js'), 'export const loadedFrom = "host-app";\n'); + return root; +} + +describe('os serve → app-declared optional package resolution', () => { + // A name no workspace package can satisfy, so a pass cannot come from the + // CLI's own node_modules by accident. + const PKG = '@os-fixture/cluster-driver-probe'; + + it('reproduces the asymmetry: an app-only package is invisible to a bare import', async () => { + // This file resolves from `packages/cli`, exactly as `dist/commands/serve.js` + // does — the failing hop the EE image measured. + const bare: string = PKG; + await expect(import(bare)).rejects.toMatchObject({ + code: expect.stringMatching(/MODULE_NOT_FOUND|ERR_MODULE_NOT_FOUND/), + }); + }); + + it('crosses the boundary: the host importer loads what the app declares', async () => { + const hostRoot = makeHostApp(PKG, true); + const mod = await createHostImporter(hostRoot)(PKG); + expect(mod.loadedFrom).toBe('host-app'); + }); + + it('still refuses a package the app does not declare (the gate is unchanged)', async () => { + // Present in the app's node_modules but absent from its package.json. + // Reachability must not substitute for declaration (#4719) — this fix moves + // where a module is resolved FROM, it does not widen what serve accepts. + const hostRoot = makeHostApp(PKG, false); + await expect(createHostImporter(hostRoot)(PKG)).rejects.toMatchObject({ + code: 'MODULE_NOT_FOUND', + }); + }); +}); + +describe('os serve → cluster block source shape', () => { + it('loads the cluster gate and driver through the host importer', () => { + expect(SERVE_SOURCE).toMatch(/await importFromHost\(__clusterPkg\)/); + expect(SERVE_SOURCE).toMatch( + /await importFromHost\(`@objectstack\/service-cluster-\$\{__clusterDriver\}`\)/, + ); + }); + + it('never reaches the cluster packages through a bare dynamic import', () => { + // The exact regression, in both spellings the block used. + expect(SERVE_SOURCE).not.toMatch(/await import\(__clusterPkg\)/); + expect(SERVE_SOURCE).not.toMatch(/await import\(`@objectstack\/service-cluster-/); + }); + + it('defines importFromHost ABOVE the cluster block that consumes it', () => { + const definition = SERVE_SOURCE.indexOf('const importFromHost = createHostImporter('); + const clusterUse = SERVE_SOURCE.indexOf('await importFromHost(__clusterPkg)'); + + expect(definition, 'importFromHost definition not found — was it renamed?').toBeGreaterThan(-1); + expect(clusterUse, 'cluster gate no longer loads via importFromHost').toBeGreaterThan(-1); + + // `const` in one long boot function: a use above the definition is a + // temporal-dead-zone throw at boot, and the load it guards is exactly the + // one that must not fall back to bare resolution. + expect( + definition, + 'importFromHost is defined AFTER the cluster block. That is the defect this file ' + + 'pins: every optional load placed above the helper silently resolves from the ' + + "CLI's own node_modules instead of the host app's. Hoist the helper.", + ).toBeLessThan(clusterUse); + }); + + it('keeps exactly one host-importer definition, so hoisting cannot fork it', () => { + const definitions = [...SERVE_SOURCE.matchAll(/const importFromHost\s*=/g)]; + expect(definitions).toHaveLength(1); + }); +}); diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 9b8c0863c1..b02db0a705 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -1409,6 +1409,43 @@ export default class Serve extends Command { // keys off it too (#4012). const loggerConfig = { level: bootLogLevel }; + // Host-app package resolution — shared by every optional / enterprise + // package loaded from here down. + // + // Node ESM resolves a bare `import(pkg)` against the IMPORTER's own + // realpath. The CLI is reached through a workspace/`link:` dependency, so + // that realpath is inside the FRAMEWORK workspace: a bare import can only + // see what the framework itself installed. A package supplied by the app + // being served — a cloud-private one such as `@objectstack/organizations`, + // or anything a customer installs into their own project — is invisible + // to it no matter what the host app declares. Resolve from the host root + // instead; the CLI's own resolution stays as the fallback for the + // framework-owned packages the CLI depends on. + // + // #4719: "resolve from the host root" now means "resolve what the host + // root DECLARES". The host lookup was a CJS require, CJS honours + // NODE_PATH, and the pnpm bin shim exports NODE_PATH pointing at the + // hoisted workspace store — so anything transitively reachable from + // anywhere in the workspace resolved as if the app had declared it, and + // whether the D5 wall below fired came down to whether `serve` was reached + // through that shim. The declaration is the contract; reachability is not. + // + // Defined HERE, at the TOP of the boot sequence, because the very first + // optional package `serve` loads is the cluster gate a few lines below. + // This helper has now been hoisted twice for the same reason, which is the + // point worth keeping: every load placed ABOVE it silently falls back to a + // bare import and can only see the framework's own node_modules. It first + // sat below the auth block, so the enterprise organizations load resolved + // in the framework workspace, never found the cloud-private package, and + // every walled-posture deployment hit the ADR-0093 D5 fail-fast and exited + // 1 (cloud#1013). It then sat below the cluster block, so `serve` could not + // load an app-declared `@objectstack/service-cluster*` at all and EE + // multi-node boot died outright on `OS_CLUSTER_DRIVER=redis`. A new + // optional load added above this line reintroduces the same defect a third + // time — put it below, or hoist this further and say why here. + const hostRoot = process.cwd(); + const importFromHost = createHostImporter(hostRoot); + // Cluster wiring: env-driven driver selection (mirrors OS_DATABASE_URL). // The remote driver self-registers on import; import it dynamically so it // works in BOTH config-boot and compiled-artifact mode. Open-core ships @@ -1422,8 +1459,17 @@ export default class Serve extends Command { // single-node rather than fail — multi-node is an add-on, never brick. // Dynamic, non-literal specifier so the CLI does not statically depend // on the cluster package (mirrors the remote-driver import below). + // + // Loaded through `importFromHost`, NOT a bare `import()`: the cluster + // packages ship with a distribution and are declared by the APP, so they + // live in the app's node_modules, while a bare import resolves against + // the CLI's own realpath and can only see the framework's. Measured on + // the EE image: the CLI's `node_modules/@objectstack/` held 48 packages + // and neither cluster one, so this line threw `Cannot find package + // '@objectstack/service-cluster'` and took the whole boot down — while + // app-side code loaded the very same package fine. const __clusterPkg: string = '@objectstack/service-cluster'; - const { checkMultiNodeAllowed } = (await import(__clusterPkg)) as { + const { checkMultiNodeAllowed } = (await importFromHost(__clusterPkg)) as { checkMultiNodeAllowed: (requested?: number) => MultiNodeGateVerdict; }; // Ask the gate about the topology the operator actually DECLARED. @@ -1453,7 +1499,12 @@ export default class Serve extends Command { // above, and deliberately not a downgrade. const __capAdvisory = formatMultiNodeCapAdvisory(__gate); if (__capAdvisory) console.warn(__capAdvisory); - try { await import(`@objectstack/service-cluster-${__clusterDriver}`); } + // Same host-anchored resolution as the gate above — the shipped + // drivers (`-redis`, `-postgres`, …) are app-declared too. The catch + // stays deliberately silent: the driver may already have been + // registered by the loaded config, and an absent driver is a + // documented fall-back to the in-memory cluster, not a boot failure. + try { await importFromHost(`@objectstack/service-cluster-${__clusterDriver}`); } catch { /* may already be registered by the loaded config */ } clusterConfig = { driver: __clusterDriver, url: process.env.OS_REDIS_URL }; } @@ -2097,36 +2148,6 @@ export default class Serve extends Command { } } - // Host-app package resolution — shared by every optional / enterprise - // package loaded from here down. - // - // Node ESM resolves a bare `import(pkg)` against the IMPORTER's own - // realpath. The CLI is reached through a workspace/`link:` dependency, so - // that realpath is inside the FRAMEWORK workspace: a bare import can only - // see what the framework itself installed. A package supplied by the app - // being served — a cloud-private one such as `@objectstack/organizations`, - // or anything a customer installs into their own project — is invisible - // to it no matter what the host app declares. Resolve from the host root - // instead; the CLI's own resolution stays as the fallback for the - // framework-owned packages the CLI depends on. - // - // #4719: "resolve from the host root" now means "resolve what the host - // root DECLARES". The host lookup was a CJS require, CJS honours - // NODE_PATH, and the pnpm bin shim exports NODE_PATH pointing at the - // hoisted workspace store — so anything transitively reachable from - // anywhere in the workspace resolved as if the app had declared it, and - // whether the D5 wall below fired came down to whether `serve` was reached - // through that shim. The declaration is the contract; reachability is not. - // - // Defined HERE, above the auth block, because the enterprise organizations - // load inside it needs it: this helper used to be declared *after* that - // block, so the organizations load fell back to a bare import, resolved in - // the framework workspace, never found the cloud-private package, and every - // walled-posture deployment hit the ADR-0093 D5 fail-fast and exited 1 - // (cloud#1013). - const hostRoot = process.cwd(); - const importFromHost = createHostImporter(hostRoot); - // 5d. Auto-register AuthPlugin (and paired Security/Audit) when the // 'auth' tier is enabled and no auth plugin is already configured. // The Console expects /api/v1/auth/* to be served by better-auth via