From 8272dfb7debdb6178b0d271168343ac34a805353 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Thu, 13 Aug 2026 11:53:39 -0500 Subject: [PATCH] test(lease): bind probe path and timeout evidence (#869) --- REPORT-A1207.md | 71 +++++++++ .../commands/lease-activation-probe.spec.ts | 77 ++++++++-- .../src/commands/lease-activation-probe.ts | 19 ++- .../mutator-gate/version_coupling_unittest.py | 140 +++++++++++++++++- 4 files changed, 293 insertions(+), 14 deletions(-) create mode 100644 REPORT-A1207.md diff --git a/REPORT-A1207.md b/REPORT-A1207.md new file mode 100644 index 00000000..752e7c90 --- /dev/null +++ b/REPORT-A1207.md @@ -0,0 +1,71 @@ +# REPORT A1207 + +Date: 2026-08-13 +Branch: `fix/869-lease-probe-timeout` +Starting head: `2373a5ad345fb316ad2460f6390baab1f45ba08f` +Base: `216cd72226cd9ee17eea461cfe7cd0e010a22f02` + +## What changed + +- Added Python behavior tests using isolated temporary directories and marker-writing fake `mosaic` executables. They prove that the supplied `PATH` wins over ambient `os.environ["PATH"]`, and that absent or empty supplied `PATH` values do not search ambient paths, platform defaults, or the current directory. +- Bound Python override behavior with executable fakes: a valid `MOSAIC_LEASE_VERSION_PROBE_COMMAND` wins over supplied and ambient `PATH`; an invalid override returns `None` without PATH fallback. +- Added a Python runner binding test that captures kwargs and requires `timeout=10.0`. Existing timeout, transport-error, and nonzero-exit checks remain fail-closed with `None`. +- Added the optional TypeScript dependency-injection seam `CapabilityProbeExecFile`, defaulting to the existing real `execFileSync` implementation. Production callers have no behavior change. +- Added TypeScript tests that capture child-process options and require exactly `timeout: 10_000`. Injected timeout, spawn-error, nonzero-exit, unparseable JSON, and malformed-object cases all return `null`. +- Removed the ambient no-dependency TypeScript smoke case that could execute a built checkout's real CLI. Default resolver and supervisor behavior retain their isolated tests, while capability transport tests now use an isolated artifact or the injected transport. + +No Python production code changed relative to `2373a5ad`. The only production delta is the optional TypeScript child-process injection seam. + +## Hermeticity incident and correction + +An initial ambient-lookup mutation run exposed that the pre-existing Python "not resolvable" test left ambient process PATH uncontrolled. On this host, that mutation resolved and executed the host `mosaic` capability probe. A post-build intermediate TypeScript run also let the pre-existing no-dependency smoke case execute the checkout's built `dist/cli.js` capability probe. No `claude` process was run. I then isolated the Python test's ambient PATH, removed the TypeScript ambient smoke case, repeated the PATH mutation using only marker-writing temporary fakes, and repeated the final suites without either real probe path. + +## Mutation evidence + +Each mutation was applied independently, its focused suite was run, and the production source was restored before the final run. + +| Mutation | Result | Reddened test name(s) | +| ------------------------------------------------------------------------------------------ | ------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `shutil.which("mosaic", path=environ.get("PATH", ""))` to ambient `shutil.which("mosaic")` | RED, three failures | `ProbeActivationCapabilityTest.test_supplied_path_wins_over_ambient_process_path`; `ProbeActivationCapabilityTest.test_absent_or_empty_supplied_path_never_falls_back_or_executes` for both absent and empty PATH subtests | +| Python `PROBE_TIMEOUT_SECONDS: 10.0` to `2.0` | RED, one failure | `ProbeActivationCapabilityTest.test_probe_passes_ten_second_timeout_to_runner` | +| TypeScript `LEASE_CAPABILITY_PROBE_TIMEOUT_MS: 10_000` to `2_000` | RED, one failure | `defaultCapabilityProbe > passes the exact ten-second timeout to the injected child-process transport` | + +## Final test run + +Dependencies were installed first with `pnpm install --frozen-lockfile`. Workspace dependencies were then built with `pnpm --filter '@mosaicstack/mosaic...' run build` so package type declarations were available. + +```text +$ cd packages/mosaic && python3 src/mutator-gate/version_coupling_unittest.py +................... +---------------------------------------------------------------------- +Ran 19 tests in 0.007s + +OK + +$ pnpm exec vitest run src/commands/lease-activation-probe.spec.ts +✓ src/commands/lease-activation-probe.spec.ts (20 tests) 80ms +Test Files 1 passed (1) +Tests 20 passed (20) +``` + +```text +$ pnpm exec prettier --check packages/mosaic/src/commands/lease-activation-probe.ts packages/mosaic/src/commands/lease-activation-probe.spec.ts +Checking formatting... +All matched files use Prettier code style! + +$ pnpm --filter @mosaicstack/mosaic lint +> eslint src + +$ pnpm --filter @mosaicstack/mosaic typecheck +> tsc --noEmit + +$ python3 -m py_compile packages/mosaic/src/mutator-gate/version_coupling_unittest.py packages/mosaic/framework/tools/lease-broker/activation_version_gate.py + +$ git diff --check +``` + +All commands above exited zero. + +## Ambiguities skipped + +None. diff --git a/packages/mosaic/src/commands/lease-activation-probe.spec.ts b/packages/mosaic/src/commands/lease-activation-probe.spec.ts index 8839c6a3..a886f77e 100644 --- a/packages/mosaic/src/commands/lease-activation-probe.spec.ts +++ b/packages/mosaic/src/commands/lease-activation-probe.spec.ts @@ -7,11 +7,13 @@ import { fileURLToPath } from 'node:url'; import { LEASE_ACTIVATION_CAPABILITY, LEASE_CAPABILITY_PROBE_COMMAND, + LEASE_CAPABILITY_PROBE_TIMEOUT_MS, defaultCapabilityProbe, defaultResolveCliEntry, defaultSupervisorProbe, leaseEnforcementActivatable, registerLeaseCapabilityProbe, + type CapabilityProbeExecFile, type LeaseActivationCapability, type SupervisorProbeResult, } from './lease-activation-probe.js'; @@ -35,6 +37,17 @@ const presentSupervisor: SupervisorProbeResult = { socketPath: '/run/user/1000/mosaic-lease/broker.sock', }; +function withScratchCli(run: (cliPath: string) => T): T { + const scratchDir = mkdtempSync(join(tmpdir(), 'mosaic-lease-capability-probe-')); + try { + const cliPath = join(scratchDir, 'cli.js'); + writeFileSync(cliPath, '// isolated fake; injected execFile means this is never executed\n'); + return run(cliPath); + } finally { + rmSync(scratchDir, { recursive: true, force: true }); + } +} + describe('leaseEnforcementActivatable', () => { it('is false when the activation capability is absent (null)', () => { const result = leaseEnforcementActivatable({ @@ -100,15 +113,6 @@ describe('leaseEnforcementActivatable', () => { }); 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', () => { @@ -127,6 +131,61 @@ describe('defaultCapabilityProbe', () => { expect(result).toBeNull(); }); + it('passes the exact ten-second timeout to the injected child-process transport', () => { + withScratchCli((cliPath) => { + let captured: + | { + file: string; + args: string[]; + options: Parameters[2]; + } + | undefined; + const execFile: CapabilityProbeExecFile = (file, args, options) => { + captured = { file, args, options }; + return JSON.stringify(LEASE_ACTIVATION_CAPABILITY); + }; + + const result = defaultCapabilityProbe({ resolveCliEntry: () => cliPath, execFile }); + + expect(result).toEqual(LEASE_ACTIVATION_CAPABILITY); + expect(captured).toEqual({ + file: process.execPath, + args: [cliPath, LEASE_CAPABILITY_PROBE_COMMAND], + options: { + encoding: 'utf-8', + timeout: 10_000, + stdio: ['ignore', 'pipe', 'ignore'], + }, + }); + expect(captured?.options.timeout).toBe(LEASE_CAPABILITY_PROBE_TIMEOUT_MS); + }); + }); + + it.each([ + ['timeout', Object.assign(new Error('timed out'), { code: 'ETIMEDOUT' })], + ['spawn error', Object.assign(new Error('spawn failed'), { code: 'ENOENT' })], + ['nonzero exit', Object.assign(new Error('child exited 1'), { status: 1 })], + ])('returns null (fail-closed) on child-process %s', (_failure, error) => { + withScratchCli((cliPath) => { + const execFile: CapabilityProbeExecFile = () => { + throw error; + }; + + expect(defaultCapabilityProbe({ resolveCliEntry: () => cliPath, execFile })).toBeNull(); + }); + }); + + it.each([ + ['unparseable JSON', 'not-json'], + ['malformed object', JSON.stringify({ name: LEASE_ACTIVATION_CAPABILITY.name })], + ])('returns null (fail-closed) on %s output', (_failure, output) => { + withScratchCli((cliPath) => { + const execFile: CapabilityProbeExecFile = () => output; + + expect(defaultCapabilityProbe({ resolveCliEntry: () => cliPath, execFile })).toBeNull(); + }); + }); + 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 diff --git a/packages/mosaic/src/commands/lease-activation-probe.ts b/packages/mosaic/src/commands/lease-activation-probe.ts index c3987cb1..3e90e2a5 100644 --- a/packages/mosaic/src/commands/lease-activation-probe.ts +++ b/packages/mosaic/src/commands/lease-activation-probe.ts @@ -123,12 +123,28 @@ export function defaultResolveCliEntry( return join(dirname(mainEntry), 'cli.js'); } +/** Narrow injectable seam for the synchronous child process used by the + * capability probe. */ +export type CapabilityProbeExecFile = ( + file: string, + args: string[], + options: { + encoding: BufferEncoding; + timeout: number; + stdio: ['ignore', 'pipe', 'ignore']; + }, +) => string; + /** 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; + /** Execute the resolved CLI entrypoint. Defaults to the real + * `execFileSync`. Inject so transport behavior and options can be tested + * without spawning a process. */ + execFile?: CapabilityProbeExecFile; } /** @@ -152,7 +168,8 @@ export function defaultCapabilityProbe( const cliEntry = resolveCliEntry(); if (!existsSync(cliEntry)) return null; - const output = execFileSync(process.execPath, [cliEntry, LEASE_CAPABILITY_PROBE_COMMAND], { + const execFile: CapabilityProbeExecFile = deps.execFile ?? execFileSync; + const output = execFile(process.execPath, [cliEntry, LEASE_CAPABILITY_PROBE_COMMAND], { encoding: 'utf-8', timeout: LEASE_CAPABILITY_PROBE_TIMEOUT_MS, stdio: ['ignore', 'pipe', 'ignore'], diff --git a/packages/mosaic/src/mutator-gate/version_coupling_unittest.py b/packages/mosaic/src/mutator-gate/version_coupling_unittest.py index 6151170e..c915b06e 100644 --- a/packages/mosaic/src/mutator-gate/version_coupling_unittest.py +++ b/packages/mosaic/src/mutator-gate/version_coupling_unittest.py @@ -24,11 +24,15 @@ from __future__ import annotations import importlib.util import io +import os +import shlex import subprocess import sys +import tempfile import unittest from contextlib import redirect_stderr from pathlib import Path +from unittest import mock TOOLS_DIR = Path(__file__).parents[2] / "framework/tools/lease-broker" @@ -57,6 +61,20 @@ def matching_capability() -> dict[str, object]: return dict(VERSION_GATE.EXPECTED_ACTIVATION_CAPABILITY) +def write_fake_mosaic(directory: Path, marker: Path) -> Path: + directory.mkdir(parents=True, exist_ok=True) + executable = directory / "mosaic" + executable.write_text( + "#!/bin/sh\n" + f"printf '%s\\n' executed >> {shlex.quote(str(marker))}\n" + "printf '%s\\n' " + "'{\"name\":\"lease-runtime-activation\",\"version\":1}'\n", + encoding="utf-8", + ) + executable.chmod(0o755) + return executable + + class AssertActivationCapabilityMatchesTest(unittest.TestCase): """Unit-level coverage of `activation_version_gate.py`'s own assertion, isolated from the launch-runtime.py seam it is wired into below.""" @@ -110,11 +128,102 @@ class ProbeActivationCapabilityTest(unittest.TestCase): handling — never spawns a real `mosaic` process.""" def test_returns_none_when_mosaic_is_not_resolvable_on_path(self) -> None: - result = VERSION_GATE.default_probe_activation_capability( - {"PATH": "/nonexistent-bin-dir-for-869-c4-test"} - ) + # Keep even a deliberate ambient-lookup mutation away from any host + # installation. The dedicated hermeticity tests below provide fake + # ambient executables and markers. + with mock.patch.dict( + os.environ, {"PATH": "/nonexistent-ambient-bin-dir-for-869-c4-test"} + ): + result = VERSION_GATE.default_probe_activation_capability( + {"PATH": "/nonexistent-bin-dir-for-869-c4-test"} + ) self.assertIsNone(result) + def test_supplied_path_wins_over_ambient_process_path(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + supplied_marker = root / "supplied.marker" + ambient_marker = root / "ambient.marker" + supplied_bin = root / "supplied-bin" + ambient_bin = root / "ambient-bin" + write_fake_mosaic(supplied_bin, supplied_marker) + write_fake_mosaic(ambient_bin, ambient_marker) + + with mock.patch.dict(os.environ, {"PATH": str(ambient_bin)}): + result = VERSION_GATE.default_probe_activation_capability( + {"PATH": str(supplied_bin)} + ) + + self.assertEqual(result, matching_capability()) + self.assertTrue(supplied_marker.exists()) + self.assertFalse(ambient_marker.exists()) + + def test_absent_or_empty_supplied_path_never_falls_back_or_executes(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + ambient_marker = root / "ambient.marker" + current_directory_marker = root / "current-directory.marker" + ambient_bin = root / "ambient-bin" + current_directory = root / "current-directory" + write_fake_mosaic(ambient_bin, ambient_marker) + write_fake_mosaic(current_directory, current_directory_marker) + original_directory = Path.cwd() + + try: + os.chdir(current_directory) + with mock.patch.dict(os.environ, {"PATH": str(ambient_bin)}): + for supplied_environment in ({}, {"PATH": ""}): + with self.subTest(environ=supplied_environment): + result = VERSION_GATE.default_probe_activation_capability( + supplied_environment + ) + self.assertIsNone(result) + self.assertFalse(ambient_marker.exists()) + self.assertFalse(current_directory_marker.exists()) + finally: + os.chdir(original_directory) + + def test_valid_override_wins_and_invalid_override_does_not_fall_back_to_path( + self, + ) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + supplied_marker = root / "supplied.marker" + ambient_marker = root / "ambient.marker" + override_marker = root / "override.marker" + supplied_bin = root / "supplied-bin" + ambient_bin = root / "ambient-bin" + override_bin = root / "override-bin" + write_fake_mosaic(supplied_bin, supplied_marker) + write_fake_mosaic(ambient_bin, ambient_marker) + override_executable = write_fake_mosaic(override_bin, override_marker) + + with mock.patch.dict(os.environ, {"PATH": str(ambient_bin)}): + result = VERSION_GATE.default_probe_activation_capability( + { + "PATH": str(supplied_bin), + VERSION_GATE.MOSAIC_COMMAND_OVERRIDE_VAR: str(override_executable), + } + ) + self.assertEqual(result, matching_capability()) + self.assertTrue(override_marker.exists()) + self.assertFalse(supplied_marker.exists()) + self.assertFalse(ambient_marker.exists()) + + override_marker.unlink() + result = VERSION_GATE.default_probe_activation_capability( + { + "PATH": str(supplied_bin), + VERSION_GATE.MOSAIC_COMMAND_OVERRIDE_VAR: str( + root / "invalid-override" / "mosaic" + ), + } + ) + self.assertIsNone(result) + self.assertFalse(override_marker.exists()) + self.assertFalse(supplied_marker.exists()) + self.assertFalse(ambient_marker.exists()) + def test_override_command_is_parsed_and_the_probe_subcommand_is_not_double_appended( self, ) -> None: @@ -135,6 +244,29 @@ class ProbeActivationCapabilityTest(unittest.TestCase): self.assertEqual(result, {"name": "lease-runtime-activation", "version": 1}) self.assertEqual(captured, [["/fake/mosaic", "__lease-capability"]]) + def test_probe_passes_ten_second_timeout_to_runner(self) -> None: + captured_argv: list[str] = [] + captured_kwargs: dict[str, object] = {} + + class FakeCompleted: + returncode = 0 + stdout = '{"name": "lease-runtime-activation", "version": 1}' + + def fake_run(argv: list[str], **kwargs: object) -> FakeCompleted: + captured_argv.extend(argv) + captured_kwargs.update(kwargs) + return FakeCompleted() + + result = VERSION_GATE.default_probe_activation_capability( + {VERSION_GATE.MOSAIC_COMMAND_OVERRIDE_VAR: "/fake/mosaic"}, + run=fake_run, + ) + + self.assertEqual(result, matching_capability()) + self.assertEqual(captured_argv, ["/fake/mosaic"]) + self.assertEqual(captured_kwargs["timeout"], 10.0) + self.assertEqual(captured_kwargs["check"], False) + def test_fails_closed_on_nonzero_exit_malformed_json_and_missing_fields(self) -> None: class NonZeroExit: returncode = 1 @@ -174,7 +306,7 @@ class ProbeActivationCapabilityTest(unittest.TestCase): def test_fails_closed_on_timeout_and_transport_error(self) -> None: def timeout_run(*_args: object, **_kwargs: object) -> None: - raise subprocess.TimeoutExpired(cmd="mosaic", timeout=2.0) + raise subprocess.TimeoutExpired(cmd="mosaic", timeout=10.0) def oserror_run(*_args: object, **_kwargs: object) -> None: raise OSError("no such file or directory")