From ac44a1aea7243129978b11a22209f54f5a3bf654 Mon Sep 17 00:00:00 2001 From: ms-lead-reviewer Date: Wed, 22 Jul 2026 13:57:17 -0500 Subject: [PATCH] fix(mosaic): resolve lease-activation CLI via exported "." entry, not "./package.json" (#869 C1 review fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Independent review of PR #870 found defaultCapabilityProbe() resolved the installed CLI via req.resolve('@mosaicstack/mosaic/package.json') — a subpath NOT present in package.json's `exports` map. Node throws ERR_PACKAGE_PATH_NOT_EXPORTED for that on every real install, which the catch-all silently turned into an always-null probe: leaseEnforcementActivatable() could never return true anywhere, defeating the card's purpose. Fix: resolve via the already-exported "." entry instead (require.resolve('@mosaicstack/mosaic') -> dist/index.js), then take cli.js as its sibling in the same built dist/ directory — matching package.json's bin.mosaic mapping. No change to the exports map itself (that would mask a separate, out-of-scope pre-existing issue in resolveTool()/launch.ts per review guidance). Adds a positive-path test that stages a realistic fake dist/index.js + dist/cli.js at the package's real resolved location and asserts defaultCapabilityProbe() returns the actual {name, version} capability object with zero mocking of the resolver. Verified red against the prior (reverted-and-restored) buggy resolution before landing the fix, and green after — the previous only-unmocked test asserted null for the wrong reason (masking this bug) and is left in place alongside the new one. Re-verified: launch.spec.ts (30), fail-closed-regression.spec.ts (2), and runtime_tools_unittest.py (25, including both C-REGRESS-locked fail-closed cases) all still green. typecheck/lint/format:check pass via `turbo run ... --filter=@mosaicstack/mosaic`. Co-Authored-By: Claude Opus 4.8 --- .../commands/lease-activation-probe.spec.ts | 53 ++++++++++++++++++- .../src/commands/lease-activation-probe.ts | 11 +++- 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/packages/mosaic/src/commands/lease-activation-probe.spec.ts b/packages/mosaic/src/commands/lease-activation-probe.spec.ts index 338c8a3b..4a7e1f72 100644 --- a/packages/mosaic/src/commands/lease-activation-probe.spec.ts +++ b/packages/mosaic/src/commands/lease-activation-probe.spec.ts @@ -1,5 +1,8 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, afterEach } from 'vitest'; import { Command } from 'commander'; +import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; import { LEASE_ACTIVATION_CAPABILITY, LEASE_CAPABILITY_PROBE_COMMAND, @@ -115,6 +118,54 @@ describe('defaultCapabilityProbe', () => { // artifact advertising the capability. expect(defaultCapabilityProbe()).toBeNull(); }); + + describe('positive path — a real built dist/cli.js advertises the capability', () => { + // Resolve the ACTUAL @mosaicstack/mosaic package root on disk (two levels + // up from src/commands/), so this test exercises the exact same + // require.resolve('@mosaicstack/mosaic') self-reference codepath + // defaultCapabilityProbe() itself uses — no mocking of the resolver. + // This is the regression proof for the reviewer-found bug: the previous + // implementation resolved via the NOT-exported './package.json' subpath + // (ERR_PACKAGE_PATH_NOT_EXPORTED on every real install), which the + // catch-all silently turned into an always-null probe. That bug returns + // null here regardless of a built dist/cli.js being present — so this + // test fails red against it and only passes once resolution goes through + // the package's already-exported "." entry. + const mosaicRoot = join(dirname(fileURLToPath(import.meta.url)), '..', '..'); + const distDir = join(mosaicRoot, 'dist'); + const distIndexPath = join(distDir, 'index.js'); + const distCliPath = join(distDir, 'cli.js'); + + // dist/ is gitignored and unbuilt in a fresh checkout; only remove what + // THIS test created, never a real build that predates it. + const distDirPreexisted = existsSync(distDir); + const indexPreexisted = existsSync(distIndexPath); + const cliPreexisted = existsSync(distCliPath); + + afterEach(() => { + if (!cliPreexisted) rmSync(distCliPath, { force: true }); + if (!indexPreexisted) rmSync(distIndexPath, { force: true }); + if (!distDirPreexisted) rmSync(distDir, { recursive: true, force: true }); + }); + + it('returns the real {name, version} capability object', () => { + mkdirSync(distDir, { recursive: true }); + // Minimal stand-in for the built "." export target — only needs to + // exist for require.resolve('@mosaicstack/mosaic') to succeed; its + // content is never loaded by defaultCapabilityProbe(). + writeFileSync(distIndexPath, 'export {};\n'); + // Minimal stand-in for the built CLI's hidden __lease-capability + // subcommand — prints exactly what registerLeaseCapabilityProbe() + // wires the real `mosaic __lease-capability` command to print. + writeFileSync( + distCliPath, + `process.stdout.write(JSON.stringify(${JSON.stringify(LEASE_ACTIVATION_CAPABILITY)}));\n`, + ); + + const result = defaultCapabilityProbe(); + expect(result).toEqual(LEASE_ACTIVATION_CAPABILITY); + }); + }); }); describe('defaultSupervisorProbe', () => { diff --git a/packages/mosaic/src/commands/lease-activation-probe.ts b/packages/mosaic/src/commands/lease-activation-probe.ts index 9507561a..02385221 100644 --- a/packages/mosaic/src/commands/lease-activation-probe.ts +++ b/packages/mosaic/src/commands/lease-activation-probe.ts @@ -94,8 +94,15 @@ export function registerLeaseCapabilityProbe(program: Command): void { export function defaultCapabilityProbe(): LeaseActivationCapability | null { try { const req = createRequire(import.meta.url); - const pkgJsonPath = req.resolve('@mosaicstack/mosaic/package.json'); - const cliEntry = join(dirname(pkgJsonPath), 'dist', 'cli.js'); + // Resolve via the package's "." export (already present in package.json's + // `exports` map) rather than a "./package.json" subpath — the latter is + // NOT exported, so `require.resolve('@mosaicstack/mosaic/package.json')` + // throws ERR_PACKAGE_PATH_NOT_EXPORTED on every real install, which the + // catch below would silently turn into an always-null probe. The "." + // export resolves to `dist/index.js`; `cli.js` is its sibling in the same + // built `dist/` directory (see package.json's `bin.mosaic`). + const mainEntry = req.resolve('@mosaicstack/mosaic'); + const cliEntry = join(dirname(mainEntry), 'cli.js'); if (!existsSync(cliEntry)) return null; const output = execFileSync(process.execPath, [cliEntry, LEASE_CAPABILITY_PROBE_COMMAND], {