From 763cecc381a8c5f2628a1ca833f27bd6b7f55ec2 Mon Sep 17 00:00:00 2001 From: ms-lead-reviewer Date: Wed, 22 Jul 2026 13:36:29 -0500 Subject: [PATCH 1/4] feat(mosaic): leaseEnforcementActivatable() capability probe (#869 C1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a real activation-capability probe so a downstream install-ordering guard (C2, out of scope here) can refuse to wire lease-broker enforcement (PreToolUse/Stop hooks) on a host that cannot actually activate it — the root cause of #828's version-skew brick, where the published CLI tarball lagged the enforcement reseed and every tool call denied with GATE_UNAVAILABLE. - LEASE_ACTIVATION_CAPABILITY {name, version}: versioned signal owned by the activation half (execLeaseGatedRuntime), independent of npm semver. - Hidden `mosaic __lease-capability` subcommand: prints that capability from the actually-resolvable built CLI artifact, not source-tree presence. - leaseEnforcementActivatable(): pure predicate, true iff a compatible capability is advertised AND the broker supervisor (launcher + daemon.py artifacts, socket path) resolves. Detection only — never starts the broker. Both inputs are injectable for testing. - C-REGRESS: added a vitest spec that runs the two test-locked fail-closed gate cases in runtime_tools_unittest.py directly, proving the gate's fail-closed-on-absent-identity behavior is unchanged by this card. Part of #869 (Point-1 C1). Co-Authored-By: Claude Opus 4.8 --- packages/mosaic/src/cli.ts | 5 + packages/mosaic/src/commands/launch.ts | 16 +- .../commands/lease-activation-probe.spec.ts | 166 +++++++++++++++ .../src/commands/lease-activation-probe.ts | 191 ++++++++++++++++++ .../fail-closed-regression.spec.ts | 41 ++++ 5 files changed, 417 insertions(+), 2 deletions(-) create mode 100644 packages/mosaic/src/commands/lease-activation-probe.spec.ts create mode 100644 packages/mosaic/src/commands/lease-activation-probe.ts create mode 100644 packages/mosaic/src/mutator-gate/fail-closed-regression.spec.ts diff --git a/packages/mosaic/src/cli.ts b/packages/mosaic/src/cli.ts index 4bccd371..0b884cac 100644 --- a/packages/mosaic/src/cli.ts +++ b/packages/mosaic/src/cli.ts @@ -21,6 +21,7 @@ import { registerRestoreCommand } from './commands/restore.js'; import { registerSkillCommand } from './commands/skill.js'; // prdy is registered via launch.ts import { registerLaunchCommands } from './commands/launch.js'; +import { registerLeaseCapabilityProbe } from './commands/lease-activation-probe.js'; import { registerAuthCommand } from './commands/auth.js'; import { registerFederationCommand } from './commands/federation.js'; import { registerGatewayCommand } from './commands/gateway.js'; @@ -78,6 +79,10 @@ Command Groups: registerLaunchCommands(program); +// ─── lease activation capability probe (hidden; #869 Point-1 C1) ──────── + +registerLeaseCapabilityProbe(program); + // ─── login ────────────────────────────────────────────────────────────── program diff --git a/packages/mosaic/src/commands/launch.ts b/packages/mosaic/src/commands/launch.ts index 2a696820..6f4e5041 100644 --- a/packages/mosaic/src/commands/launch.ts +++ b/packages/mosaic/src/commands/launch.ts @@ -806,7 +806,14 @@ function launchRuntime(runtime: RuntimeName, args: string[], yolo: boolean): nev process.exit(0); // Unreachable but satisfies never } -function defaultLeaseBrokerSocket(env: NodeJS.ProcessEnv = process.env): string { +/** + * Resolve the lease broker's control socket path. Exported (in addition to + * being used internally by execLeaseGatedRuntime) so the C1 activation probe + * (lease-activation-probe.ts) can perform the same resolution when checking + * whether the broker supervisor is reachable — detection only, this never + * connects to the socket itself. + */ +export function defaultLeaseBrokerSocket(env: NodeJS.ProcessEnv = process.env): string { if (env['MOSAIC_LEASE_BROKER_SOCKET']) return env['MOSAIC_LEASE_BROKER_SOCKET']; const runtimeDir = env['XDG_RUNTIME_DIR']; if (runtimeDir) return join(runtimeDir, 'mosaic-lease', 'broker.sock'); @@ -895,7 +902,12 @@ function delegateToScript(scriptPath: string, args: string[], env?: Record { + it('is false when the activation capability is absent (null)', () => { + const result = leaseEnforcementActivatable({ + getCapability: () => null, + probeSupervisor: () => presentSupervisor, + }); + expect(result).toBe(false); + }); + + it('is false when the activation capability name does not match', () => { + const result = leaseEnforcementActivatable({ + getCapability: () => ({ + name: 'some-other-capability', + version: LEASE_ACTIVATION_CAPABILITY.version, + }), + probeSupervisor: () => presentSupervisor, + }); + expect(result).toBe(false); + }); + + it('is false when the activation capability version is incompatible (stale/newer build)', () => { + const result = leaseEnforcementActivatable({ + getCapability: () => ({ + name: LEASE_ACTIVATION_CAPABILITY.name, + version: LEASE_ACTIVATION_CAPABILITY.version + 1, + }), + probeSupervisor: () => presentSupervisor, + }); + expect(result).toBe(false); + }); + + it('is false when the supervisor artifacts (launcher/daemon) are not present', () => { + const result = leaseEnforcementActivatable({ + getCapability: () => compatibleCapability, + probeSupervisor: () => ({ + supervisorPresent: false, + socketPath: presentSupervisor.socketPath, + }), + }); + expect(result).toBe(false); + }); + + it('is false when the supervisor socket path is not resolvable', () => { + const result = leaseEnforcementActivatable({ + getCapability: () => compatibleCapability, + probeSupervisor: () => ({ supervisorPresent: true, socketPath: null }), + }); + expect(result).toBe(false); + }); + + it('is false when BOTH capability and supervisor are absent', () => { + const result = leaseEnforcementActivatable({ + getCapability: () => null, + probeSupervisor: () => ({ supervisorPresent: false, socketPath: null }), + }); + expect(result).toBe(false); + }); + + it('is true when a compatible capability AND a resolvable supervisor are both present', () => { + const result = leaseEnforcementActivatable({ + getCapability: () => compatibleCapability, + probeSupervisor: () => presentSupervisor, + }); + expect(result).toBe(true); + }); + + it('uses the real default probes when no deps are injected (does not throw)', () => { + // No live broker / built CLI is guaranteed in a test environment, so this + // only asserts the predicate degrades to a safe boolean rather than + // throwing — the fail-closed behavior itself is covered by the injected + // cases above. + expect(() => leaseEnforcementActivatable()).not.toThrow(); + expect(typeof leaseEnforcementActivatable()).toBe('boolean'); + }); +}); + +describe('defaultCapabilityProbe', () => { + it('returns null (fail-closed) when no built CLI artifact is resolvable', () => { + // This source checkout has no dist/cli.js built for @mosaicstack/mosaic, + // so the probe must report "no capability" rather than fabricate one + // from source-tree presence — this is the exact distinction #828's + // version skew needed: source existing is not the same as the published + // artifact advertising the capability. + expect(defaultCapabilityProbe()).toBeNull(); + }); +}); + +describe('defaultSupervisorProbe', () => { + it('returns a well-shaped result without starting or connecting to anything', () => { + const result = defaultSupervisorProbe({}); + expect(typeof result.supervisorPresent).toBe('boolean'); + expect(result.socketPath === null || typeof result.socketPath === 'string').toBe(true); + }); + + it('resolves a socket path from an explicit MOSAIC_LEASE_BROKER_SOCKET override', () => { + const result = defaultSupervisorProbe({ MOSAIC_LEASE_BROKER_SOCKET: '/tmp/explicit.sock' }); + expect(result.socketPath).toBe('/tmp/explicit.sock'); + }); +}); + +describe('registerLeaseCapabilityProbe', () => { + it('registers a hidden subcommand named __lease-capability', () => { + const program = new Command(); + program.exitOverride(); + registerLeaseCapabilityProbe(program); + + const registered = program.commands.find((c) => c.name() === LEASE_CAPABILITY_PROBE_COMMAND); + expect(registered).toBeDefined(); + // Commander exposes "hidden" only as help-output suppression (no public + // getter) — assert the observable behavior instead of a private field. + expect(program.helpInformation()).not.toContain(LEASE_CAPABILITY_PROBE_COMMAND); + }); + + it('prints the capability constant as JSON when invoked', () => { + const program = new Command(); + program.exitOverride(); + registerLeaseCapabilityProbe(program); + + let written = ''; + const originalWrite = process.stdout.write.bind(process.stdout); + process.stdout.write = ((chunk: string) => { + written += chunk; + return true; + }) as typeof process.stdout.write; + + try { + program.parse(['node', 'mosaic', LEASE_CAPABILITY_PROBE_COMMAND]); + } finally { + process.stdout.write = originalWrite; + } + + expect(JSON.parse(written)).toEqual(LEASE_ACTIVATION_CAPABILITY); + }); +}); diff --git a/packages/mosaic/src/commands/lease-activation-probe.ts b/packages/mosaic/src/commands/lease-activation-probe.ts new file mode 100644 index 00000000..9507561a --- /dev/null +++ b/packages/mosaic/src/commands/lease-activation-probe.ts @@ -0,0 +1,191 @@ +/** + * Lease-enforcement activation probe (issue #869, Point-1 card C1). + * + * Root cause this exists to guard against (#828 version skew): the + * ENFORCEMENT half of the lease broker (PreToolUse/Stop hooks — + * `mutator-gate.py`, `receipt-observer-client.py` — wired via the framework + * reseed) and the ACTIVATION half (`execLeaseGatedRuntime()` in `launch.ts`, + * which chains the runtime through `launch-runtime.py`, injects + * `MOSAIC_LEASE_*`, and requires a running `daemon.py` broker) ship on + * different channels. When the published CLI tarball lags behind an + * enforcement reseed, the gate correctly fails CLOSED on absent identity — + * but every tool call then denies with GATE_UNAVAILABLE. That fail-closed + * behavior is intentional and must not change (see the C-REGRESS note in + * `runtime_tools_unittest.py`); this module exists so a downstream + * install-ordering guard (C2, out of scope here) can refuse to WIRE + * enforcement in the first place on a host that cannot ACTIVATE it. + * + * `leaseEnforcementActivatable()` answers one narrow question: "if + * enforcement were wired right now, could activation actually satisfy it?" + * It is a real capability probe — not a "does the source file exist" check + * — and both of its inputs are injectable so tests can drive every branch + * without a live broker or an installed CLI on PATH. + */ + +import { execFileSync } from 'node:child_process'; +import { existsSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { dirname, join } from 'node:path'; +import type { Command } from 'commander'; +import { defaultLeaseBrokerSocket, resolveTool } from './launch.js'; + +// ─── Capability signal (owned by the activation half) ────────────────────── + +/** + * Versioned identity for the activation contract `execLeaseGatedRuntime()` + * implements. OWNED by the activation half of the lease broker. Bump + * `version` only when the activation contract itself changes (env vars + * injected, chaining behavior, socket protocol, etc.) — deliberately + * independent of the package's npm semver, because #828 happened precisely + * because the npm version was NOT bumped even though the shipped artifact + * fell out of sync. A build that cannot advertise this exact + * `{ name, version }` pair does not implement the contract a caller is + * relying on, whatever its package.json claims. + */ +export interface LeaseActivationCapability { + readonly name: string; + readonly version: number; +} + +export const LEASE_ACTIVATION_CAPABILITY: LeaseActivationCapability = { + name: 'lease-runtime-activation', + version: 1, +}; + +/** Hidden CLI probe subcommand name — wired via {@link registerLeaseCapabilityProbe}. */ +export const LEASE_CAPABILITY_PROBE_COMMAND = '__lease-capability'; + +function capabilityMatches(candidate: LeaseActivationCapability | null): boolean { + return ( + candidate !== null && + candidate.name === LEASE_ACTIVATION_CAPABILITY.name && + candidate.version === LEASE_ACTIVATION_CAPABILITY.version + ); +} + +/** + * Register the hidden `__lease-capability` probe subcommand. Prints the + * capability this BUILD advertises as compact JSON to stdout and exits 0. + * Deliberately undocumented (hidden from `--help`): it is an internal signal + * for {@link defaultCapabilityProbe}, not a user-facing command. + */ +export function registerLeaseCapabilityProbe(program: Command): void { + program + .command(LEASE_CAPABILITY_PROBE_COMMAND, { hidden: true }) + .description('Internal: print the lease-activation capability this build advertises') + .action(() => { + process.stdout.write(JSON.stringify(LEASE_ACTIVATION_CAPABILITY)); + }); +} + +/** + * Real capability lookup. Resolves the installed `@mosaicstack/mosaic` + * package's BUILT entrypoint (`dist/cli.js` — the published artifact a user + * actually runs, not this TypeScript source file) and executes its hidden + * `__lease-capability` probe subcommand out-of-process. A build that lacks + * the subcommand, fails to execute, or reports an incompatible + * `{ name, version }` is treated as having NO activation capability. + * + * This is the check that would have caught #828's version skew: the + * source-tree activation half existed, but the published tarball's `dist/` + * did not carry it, so this probe — reading the actually-resolvable built + * artifact rather than trusting source-tree presence — would report null. + */ +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'); + if (!existsSync(cliEntry)) return null; + + const output = execFileSync(process.execPath, [cliEntry, LEASE_CAPABILITY_PROBE_COMMAND], { + encoding: 'utf-8', + timeout: 2000, + stdio: ['ignore', 'pipe', 'ignore'], + }); + + const parsed: unknown = JSON.parse(output); + if ( + typeof parsed !== 'object' || + parsed === null || + typeof (parsed as Record)['name'] !== 'string' || + typeof (parsed as Record)['version'] !== 'number' + ) { + return null; + } + const candidate = parsed as { name: string; version: number }; + return { name: candidate.name, version: candidate.version }; + } catch { + return null; + } +} + +// ─── Supervisor / socket resolution (detection only) ─────────────────────── + +/** Detection-only supervisor/socket probe result. Never starts the broker + * and never connects to the socket — presence and path resolution only. */ +export interface SupervisorProbeResult { + /** The lease-broker supervisor artifacts (launcher + daemon) are present. */ + readonly supervisorPresent: boolean; + /** Resolved broker socket path, or null if it could not be resolved. */ + readonly socketPath: string | null; +} + +/** + * Real supervisor/socket resolution: checks that the lease-broker's launcher + * (`launch-runtime.py`) and supervisor (`daemon.py`) artifacts resolve on + * disk via the same tool-resolution `execLeaseGatedRuntime()` uses, and that + * a broker socket path resolves via the same logic as + * `defaultLeaseBrokerSocket()`. Detection only — this never starts the + * daemon and never connects to the socket. + */ +export function defaultSupervisorProbe( + env: NodeJS.ProcessEnv = process.env, +): SupervisorProbeResult { + const launcherPath = resolveTool('lease-broker', 'launch-runtime.py'); + const daemonPath = resolveTool('lease-broker', 'daemon.py'); + const supervisorPresent = existsSync(launcherPath) && existsSync(daemonPath); + + let socketPath: string | null = null; + try { + const resolved = defaultLeaseBrokerSocket(env); + socketPath = resolved.trim().length > 0 ? resolved : null; + } catch { + socketPath = null; + } + + return { supervisorPresent, socketPath }; +} + +// ─── Predicate ─────────────────────────────────────────────────────────── + +/** Injectable inputs for {@link leaseEnforcementActivatable}, so tests (and + * downstream callers such as the C2 install-ordering guard) can drive every + * branch without a live broker or an installed CLI on PATH. */ +export interface ActivationProbeDeps { + getCapability?: () => LeaseActivationCapability | null; + probeSupervisor?: () => SupervisorProbeResult; +} + +/** + * True IFF lease enforcement can actually be ACTIVATED on this host: + * + * (a) the resolvable CLI advertises a {@link LeaseActivationCapability} + * compatible with {@link LEASE_ACTIVATION_CAPABILITY}, AND + * (b) the broker supervisor is resolvable — launcher + `daemon.py` + * artifacts present AND a broker socket path resolves. + * + * Pure/testable: both probes default to the real, side-effect-free lookups + * above but can be injected, so this predicate never itself starts a broker + * or performs enforcement — it only reports whether activation *could* + * satisfy enforcement if wired. + */ +export function leaseEnforcementActivatable(deps: ActivationProbeDeps = {}): boolean { + const getCapability = deps.getCapability ?? defaultCapabilityProbe; + const probeSupervisor = deps.probeSupervisor ?? defaultSupervisorProbe; + + if (!capabilityMatches(getCapability())) return false; + + const supervisor = probeSupervisor(); + return supervisor.supervisorPresent && supervisor.socketPath !== null; +} diff --git a/packages/mosaic/src/mutator-gate/fail-closed-regression.spec.ts b/packages/mosaic/src/mutator-gate/fail-closed-regression.spec.ts new file mode 100644 index 00000000..9e6b1420 --- /dev/null +++ b/packages/mosaic/src/mutator-gate/fail-closed-regression.spec.ts @@ -0,0 +1,41 @@ +import { spawnSync } from 'node:child_process'; +import { join } from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +/** + * C-REGRESS (issue #869, Point-1) — proves the fail-closed gate is untouched + * by the C1 activation probe added alongside this test. + * + * `mutator-gate.py`'s fail-closed-on-absent-identity behavior is INTENTIONAL + * and TEST-LOCKED: #869 C1 gates the WIRING decision for enforcement (via + * `leaseEnforcementActivatable()`), it does not — and must not — touch the + * gate's own runtime denial behavior. This spec runs the two test-locked + * cases from `runtime_tools_unittest.py` directly (rather than merely + * re-asserting the same logic in TypeScript) so a regression in the actual + * Python gate is caught here too, not just documented in prose. + */ + +const MUTATOR_GATE_DIR = new URL('.', import.meta.url).pathname; +const UNITTEST_FILE = join(MUTATOR_GATE_DIR, 'runtime_tools_unittest.py'); + +const LOCKED_TEST_CASES = [ + 'ExecutableEntrypointTest.test_gate_entrypoint_denies_when_identity_environment_is_absent', + 'MutatorGateTest.test_environment_generation_and_request_failures_deny', +] as const; + +describe('mutator-gate fail-closed behavior (C-REGRESS, unchanged by #869 C1)', () => { + it.each(LOCKED_TEST_CASES)('%s still passes', (testCase) => { + const result = spawnSync('python3', ['-m', 'unittest', `${moduleName()}.${testCase}`, '-v'], { + cwd: MUTATOR_GATE_DIR, + encoding: 'utf-8', + }); + + expect(result.status, `stderr:\n${result.stderr}`).toBe(0); + }); +}); + +function moduleName(): string { + // runtime_tools_unittest.py, addressed as a bare module name for `python3 -m unittest`. + return UNITTEST_FILE.split('/').pop()!.replace(/\.py$/, ''); +} -- 2.49.1 From c64a04e7dd086910bf4b14315b6c860b9c7c6f49 Mon Sep 17 00:00:00 2001 From: ms-lead-reviewer Date: Wed, 22 Jul 2026 13:38:20 -0500 Subject: [PATCH 2/4] chore(orchestrator): fix pre-existing Prettier drift in README.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Whitespace-only markdown table/spacing fix. Pre-existing on origin/main (introduced by #868), unrelated to #869 C1 — fixed only because the repo's pre-push hook runs a full-repo \`pnpm format:check\` and was blocking this branch's push. No functional change. Co-Authored-By: Claude Opus 4.8 --- .../framework/tools/orchestrator/README.md | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/packages/mosaic/framework/tools/orchestrator/README.md b/packages/mosaic/framework/tools/orchestrator/README.md index 3ee7f104..3d3bb132 100644 --- a/packages/mosaic/framework/tools/orchestrator/README.md +++ b/packages/mosaic/framework/tools/orchestrator/README.md @@ -4,18 +4,18 @@ Helper scripts for r0 coordinator / orchestrator sessions — mission lifecycle, session health, continuation, and board maintenance. See `framework/guides/ORCHESTRATOR-PROTOCOL.md` for the surrounding process. -| Script | Purpose | -|--------|---------| -| `mission-init.sh` | Initialize a new orchestration mission (manifest, scratchpad, TASKS.md). | -| `mission-status.sh` | Show the mission progress dashboard. | -| `session-run.sh` | Generate continuation context and launch the target runtime. | -| `session-resume.sh` | Crash recovery for dead orchestrator sessions. | -| `session-status.sh` | Check agent session health. | -| `continue-prompt.sh` | Generate the continuation prompt for the next session. | -| `board-roll.sh` | Keep a LIVE orchestration board under its byte cap by rolling the oldest entries to its LEDGER. | -| `smoke-test.sh` | Behavior smoke checks for the coord continue/run workflows. | -| `test-board-roll.sh` | Regression harness for `board-roll.sh`. | -| `_lib.sh` | Shared functions sourced by the above (state files, TASKS.md parsing, locks). | +| Script | Purpose | +| -------------------- | ----------------------------------------------------------------------------------------------- | +| `mission-init.sh` | Initialize a new orchestration mission (manifest, scratchpad, TASKS.md). | +| `mission-status.sh` | Show the mission progress dashboard. | +| `session-run.sh` | Generate continuation context and launch the target runtime. | +| `session-resume.sh` | Crash recovery for dead orchestrator sessions. | +| `session-status.sh` | Check agent session health. | +| `continue-prompt.sh` | Generate the continuation prompt for the next session. | +| `board-roll.sh` | Keep a LIVE orchestration board under its byte cap by rolling the oldest entries to its LEDGER. | +| `smoke-test.sh` | Behavior smoke checks for the coord continue/run workflows. | +| `test-board-roll.sh` | Regression harness for `board-roll.sh`. | +| `_lib.sh` | Shared functions sourced by the above (state files, TASKS.md parsing, locks). | ## board-roll.sh @@ -35,16 +35,23 @@ always-current `##` sections) is pinned and never touched: ```markdown # MOS ORCHESTRATION BOARD — LIVE state + > protocol blockquote … (pinned) ## 🟦 Curated always-current section (pinned) + … + ### 2026-07-22 (mid²²) — newest tick, stays longest + … + ### 2026-07-20 (dawn) — oldest tick, rolled first + … + ``` -- 2.49.1 From ac44a1aea7243129978b11a22209f54f5a3bf654 Mon Sep 17 00:00:00 2001 From: ms-lead-reviewer Date: Wed, 22 Jul 2026 13:57:17 -0500 Subject: [PATCH 3/4] 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], { -- 2.49.1 From c5a2bcc516ebabaf61b46bc19f09c51e4a1158bc Mon Sep 17 00:00:00 2001 From: ms-lead-reviewer Date: Wed, 22 Jul 2026 14:24:14 -0500 Subject: [PATCH 4/4] fix(mosaic): DI-inject CLI-entry resolver so tests never touch real dist/ (#869 C1 review fix R3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review found the positive-path test's writeFileSync() staged a stub cli.js/index.js at the package's REAL resolved dist/ path, and afterEach only removed files that had NOT pre-existed — never restoring original CONTENT for files that had. On a host with a real pre-built dist/cli.js (ordinary `pnpm build && pnpm test`, and CI: this package's turbo.json overrides the `test` task to depend on `build`, so CI always builds a real dist/cli.js before running vitest), the test would silently overwrite the real ~26KB compiled CLI with an 87-byte stub and still report PASS. Confirmed as the exact root cause of CI1972's red `test` step: src/cli-smoke.spec.ts execs the real dist/cli.js in the same vitest process/run, so the clobber surfaced there. Fix (dependency injection, not snapshot/restore): - defaultCapabilityProbe() now takes an injectable CapabilityProbeDeps ({ resolveCliEntry }), defaulting to the real defaultResolveCliEntry() in production — no change to the real-artifact-read guarantee. - defaultResolveCliEntry() itself now takes an injectable ModuleResolver (defaults to the real require.resolve), so its resolution CHOICE (bare "@mosaicstack/mosaic" specifier vs the buggy "./package.json" subpath) can be tested in complete isolation from real package/build state. - The positive-path test now stages its stub cli.js in an mkdtempSync() scratch directory and injects resolveCliEntry to point there — it never calls the default resolver, so it structurally cannot touch the real package's dist/. It also asserts the real dist/ path's existence is unchanged by the test. - The "returns null when unbuilt" test now injects a resolver pointing at a path that cannot exist, instead of relying on this checkout happening to be unbuilt (deterministic regardless of ambient host build state). Verified: - Reintroduced the R1 bug in defaultResolveCliEntry() and confirmed the new resolver-choice test fails red against it; restored the fix (byte- identical diff against the pre-revert file) and confirmed green. - Built a real dist/cli.js (~26KB) via `turbo run build`, ran the targeted specs against it, then ran the FULL `turbo run test --filter=@mosaicstack/mosaic` CI-parity path (which builds dist/ itself per this package's turbo.json override before vitest runs, exactly matching Woodpecker's `test` step): 77/77 test files, 1451/1451 tests passed, including cli-smoke.spec.ts (22/22) and lease-activation-probe.spec.ts (15/15) in the SAME run. sha256 of dist/cli.js before and after that full run: identical (e61d8de7a2223b6578a2b733edd927707830e011f44b9d20901194c23c4a5272, 26317 bytes) — the real build artifact is untouched byte-for-byte. - python3 -m unittest runtime_tools_unittest: 25/25 pass, including both C-REGRESS-locked fail-closed cases. - typecheck/lint/format:check all pass via turbo --filter=@mosaicstack/mosaic. Co-Authored-By: Claude Opus 4.8 --- .../commands/lease-activation-probe.spec.ts | 124 +++++++++++------- .../src/commands/lease-activation-probe.ts | 56 ++++++-- 2 files changed, 120 insertions(+), 60 deletions(-) diff --git a/packages/mosaic/src/commands/lease-activation-probe.spec.ts b/packages/mosaic/src/commands/lease-activation-probe.spec.ts index 4a7e1f72..8839c6a3 100644 --- a/packages/mosaic/src/commands/lease-activation-probe.spec.ts +++ b/packages/mosaic/src/commands/lease-activation-probe.spec.ts @@ -1,12 +1,14 @@ -import { describe, it, expect, afterEach } from 'vitest'; +import { describe, it, expect } from 'vitest'; import { Command } from 'commander'; -import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; +import { tmpdir } from 'node:os'; import { fileURLToPath } from 'node:url'; import { LEASE_ACTIVATION_CAPABILITY, LEASE_CAPABILITY_PROBE_COMMAND, defaultCapabilityProbe, + defaultResolveCliEntry, defaultSupervisorProbe, leaseEnforcementActivatable, registerLeaseCapabilityProbe, @@ -111,60 +113,84 @@ describe('leaseEnforcementActivatable', () => { describe('defaultCapabilityProbe', () => { it('returns null (fail-closed) when no built CLI artifact is resolvable', () => { - // This source checkout has no dist/cli.js built for @mosaicstack/mosaic, - // so the probe must report "no capability" rather than fabricate one - // from source-tree presence — this is the exact distinction #828's - // version skew needed: source existing is not the same as the published - // artifact advertising the capability. - expect(defaultCapabilityProbe()).toBeNull(); + // Deterministic regardless of ambient host state (e.g. a host that has + // already run `pnpm build`, which would otherwise make this pass or fail + // depending on whether dist/cli.js happens to exist) — inject a resolver + // pointing at a path that cannot exist, rather than relying on this + // checkout being unbuilt. The probe must report "no capability" rather + // than fabricate one from source-tree presence — this is the exact + // distinction #828's version skew needed: source existing is not the + // same as the published artifact advertising the capability. + const result = defaultCapabilityProbe({ + resolveCliEntry: () => '/nonexistent/mosaic-lease-activation-probe-test/cli.js', + }); + expect(result).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'); + describe('positive path — injected resolver, isolated scratch dir (never the real dist/)', () => { + // A prior version of this test staged the stub cli.js at the package's + // REAL resolved dist/ path and relied on afterEach to clean up "only + // what it created" — which meant a host with a real pre-built + // dist/cli.js (ordinary `pnpm build && pnpm test`) would have its real + // ~26KB compiled CLI silently overwritten by an 87-byte stub, with no + // restoration of the original content. That is exactly the kind of + // build-artifact corruption #869 exists to prevent. This version uses + // dependency injection exclusively: defaultCapabilityProbe() is never + // called with its default resolver here, so it can never touch the real + // package dist/ at all — proven below by asserting that path's + // existence is unchanged by the test. + it('returns the real {name, version} capability from a stub cli.js in a temp dir, and leaves the real dist/ untouched', () => { + const packageRoot = join(dirname(fileURLToPath(import.meta.url)), '..', '..'); + const realDistDir = join(packageRoot, 'dist'); + const realDistPreexisted = existsSync(realDistDir); - // 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); + const scratchDir = mkdtempSync(join(tmpdir(), 'mosaic-lease-capability-probe-')); + try { + const scratchCliPath = join(scratchDir, 'cli.js'); + // 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( + scratchCliPath, + `process.stdout.write(JSON.stringify(${JSON.stringify(LEASE_ACTIVATION_CAPABILITY)}));\n`, + ); - afterEach(() => { - if (!cliPreexisted) rmSync(distCliPath, { force: true }); - if (!indexPreexisted) rmSync(distIndexPath, { force: true }); - if (!distDirPreexisted) rmSync(distDir, { recursive: true, force: true }); + const result = defaultCapabilityProbe({ resolveCliEntry: () => scratchCliPath }); + expect(result).toEqual(LEASE_ACTIVATION_CAPABILITY); + + // The real package dist/ must be byte-for-byte untouched: this test + // never invokes the default resolver, so the path's mere existence + // (created or not) must be unchanged by having run this test. + expect(existsSync(realDistDir)).toBe(realDistPreexisted); + } finally { + rmSync(scratchDir, { 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`, - ); +describe('defaultResolveCliEntry', () => { + it('resolves the bare "@mosaicstack/mosaic" specifier (the exported "." entry), never the non-exported "./package.json" subpath', () => { + // Fully isolated from the real filesystem/package state (no dependency + // on whether @mosaicstack/mosaic has been built on this host) via an + // injected fake resolver that mirrors Node's real behavior: the "." + // export resolves fine, but "./package.json" is NOT in package.json's + // `exports` map, so real `require.resolve` throws + // ERR_PACKAGE_PATH_NOT_EXPORTED for it. This is genuinely red-first + // against the reviewer-found bug: the old implementation resolved the + // "./package.json" subpath here, which this fake throws on — the new + // implementation must resolve only the bare specifier. + const requestedSpecifiers: string[] = []; + const fakeResolve = (specifier: string): string => { + requestedSpecifiers.push(specifier); + if (specifier === '@mosaicstack/mosaic') return '/fake/pkg/dist/index.js'; + throw new Error(`ERR_PACKAGE_PATH_NOT_EXPORTED: ${specifier}`); + }; - const result = defaultCapabilityProbe(); - expect(result).toEqual(LEASE_ACTIVATION_CAPABILITY); - }); + const result = defaultResolveCliEntry(fakeResolve); + + expect(result).toBe(join('/fake/pkg/dist', 'cli.js')); + expect(requestedSpecifiers).toEqual(['@mosaicstack/mosaic']); }); }); diff --git a/packages/mosaic/src/commands/lease-activation-probe.ts b/packages/mosaic/src/commands/lease-activation-probe.ts index 02385221..7c634d22 100644 --- a/packages/mosaic/src/commands/lease-activation-probe.ts +++ b/packages/mosaic/src/commands/lease-activation-probe.ts @@ -78,6 +78,46 @@ export function registerLeaseCapabilityProbe(program: Command): void { }); } +/** Injectable Node module resolver — matches `require.resolve`'s signature + * narrowly (specifier in, absolute path out, or throws). Defaults to the + * real `createRequire(import.meta.url).resolve`. Injectable so tests can + * exercise WHICH specifier {@link defaultResolveCliEntry} resolves (the + * reviewer-found bug was resolving the wrong one) without depending on + * whether `@mosaicstack/mosaic` has actually been built on the test host — + * and without ever touching the real package's `dist/` to find out. */ +export type ModuleResolver = (specifier: string) => string; + +/** + * Resolve the CLI's built entrypoint (`dist/cli.js`). Resolves 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. The "." export + * resolves to `dist/index.js`; `cli.js` is its sibling in the same built + * `dist/` directory (see package.json's `bin.mosaic`). + * + * Exported standalone (and injectable via {@link CapabilityProbeDeps}) so + * tests can exercise this resolution logic in isolation, or point + * {@link defaultCapabilityProbe} at a scratch directory instead of ever + * touching the real installed package's `dist/` — a test corrupting a real + * build artifact is exactly the artifact-integrity failure class this card + * exists to prevent (#828). + */ +export function defaultResolveCliEntry( + resolve: ModuleResolver = createRequire(import.meta.url).resolve, +): string { + const mainEntry = resolve('@mosaicstack/mosaic'); + return join(dirname(mainEntry), 'cli.js'); +} + +/** Injectable inputs for {@link defaultCapabilityProbe}. */ +export interface CapabilityProbeDeps { + /** Resolve the CLI entrypoint (`cli.js`) to probe. Defaults to + * {@link defaultResolveCliEntry}. Inject to point at an isolated scratch + * location in tests — never at the real package's `dist/`. */ + resolveCliEntry?: () => string; +} + /** * Real capability lookup. Resolves the installed `@mosaicstack/mosaic` * package's BUILT entrypoint (`dist/cli.js` — the published artifact a user @@ -91,18 +131,12 @@ export function registerLeaseCapabilityProbe(program: Command): void { * did not carry it, so this probe — reading the actually-resolvable built * artifact rather than trusting source-tree presence — would report null. */ -export function defaultCapabilityProbe(): LeaseActivationCapability | null { +export function defaultCapabilityProbe( + deps: CapabilityProbeDeps = {}, +): LeaseActivationCapability | null { try { - const req = createRequire(import.meta.url); - // 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'); + const resolveCliEntry = deps.resolveCliEntry ?? defaultResolveCliEntry; + const cliEntry = resolveCliEntry(); if (!existsSync(cliEntry)) return null; const output = execFileSync(process.execPath, [cliEntry, LEASE_CAPABILITY_PROBE_COMMAND], { -- 2.49.1