From cba91346c244749974a9cab7fd2e239f9b97b87b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 10:22:34 +0000 Subject: [PATCH] fix(cli): resolve serve's cluster driver imports from the host app (#10645) `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. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r --- .../cli-serve-host-anchored-cluster-import.md | 16 ++ .../serve-cluster-host-resolution.test.ts | 162 ++++++++++++++++++ packages/cli/src/commands/serve.ts | 85 +++++---- 3 files changed, 231 insertions(+), 32 deletions(-) create mode 100644 .changeset/cli-serve-host-anchored-cluster-import.md create mode 100644 packages/cli/src/commands/serve-cluster-host-resolution.test.ts 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