From 9efd903c16d8ce37102351d31fe91254182b99a8 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Fri, 14 Aug 2026 23:41:15 -0500 Subject: [PATCH] lease gate: stop denying every seat launch on a 2s probe budget The activation-capability probe spawns the whole Node CLI rather than exec'ing a binary. Measured 3.0-3.7s on an idle 4-core VM and 3.55-3.61s on web1, against `node -e 0` at 0.05s. The budget was 2.0s, so the probe timed out on every call on both hosts. The gate is fail-closed, and an expiry is indistinguishable from "no capability", so every fleet seat launch was denied with a version-skew message telling the operator to "upgrade both as one unit" -- advice that cannot fix a timeout. This is why web1 shows roster seats with no live sessions. Raise the budget to 20s, well clear of the measured range, and add MOSAIC_LEASE_VERSION_PROBE_TIMEOUT_SECONDS for slower hosts. Unusable override values fall back to the default rather than removing the bound. Also fix _resolve_probe_command ignoring the environ it is handed: shutil.which was called without path=, so it read the ambient PATH. That made test_returns_none_when_mosaic_is_not_resolvable_on_path pass only because the 2.0s budget expired first -- right answer, wrong reason, and it masked the timeout defect. The suite's runtime drops from 2.0s to 0.002s, which is that accidental timeout leaving. Verified: 18/18 version_coupling_unittest (new tests red against the old gate: 2 failures + 1 error), tsc build clean, test-start-agent-session.sh and test-fleet-units.sh rc=0. invariant_r_unittest fails identically with and without this change (pinned pi 0.84.1 vs installed 0.84.2). --- .../lease-broker/activation_version_gate.py | 35 ++++++++++++++++-- .../mutator-gate/version_coupling_unittest.py | 37 +++++++++++++++++++ 2 files changed, 69 insertions(+), 3 deletions(-) diff --git a/packages/mosaic/framework/tools/lease-broker/activation_version_gate.py b/packages/mosaic/framework/tools/lease-broker/activation_version_gate.py index b1d151b2..e49f633e 100644 --- a/packages/mosaic/framework/tools/lease-broker/activation_version_gate.py +++ b/packages/mosaic/framework/tools/lease-broker/activation_version_gate.py @@ -62,7 +62,20 @@ EXPECTED_ACTIVATION_CAPABILITY: Final[ActivationCapability] = { # capability as compact JSON. LEASE_CAPABILITY_PROBE_COMMAND: Final = "__lease-capability" -PROBE_TIMEOUT_SECONDS: Final = 2.0 +# Running the probe boots the whole Node CLI; it does not merely exec a binary. +# Measured: 3.0-3.7 s on an idle 4-core VM and 3.55-3.61 s on web1, against +# `node -e 0` at 0.05 s. The former 2.0 s budget therefore expired on every +# call on both hosts. Because the probe is fail-closed, an expiry is +# indistinguishable from "no capability", so every seat launch was denied with +# a version-skew message that no upgrade could fix. Sized well above the +# measured range: the gate still fails closed, it just no longer fails closed +# on a stopwatch. +PROBE_TIMEOUT_SECONDS: Final = 20.0 + +# Override hook: seconds to wait for the probe, for hosts slow or loaded enough +# that even the default is tight. Non-numeric or non-positive values are +# ignored in favour of the default rather than disabling the bound. +PROBE_TIMEOUT_OVERRIDE_VAR: Final = "MOSAIC_LEASE_VERSION_PROBE_TIMEOUT_SECONDS" # Override hook: a full shell-style command line (parsed with `shlex.split`) # to run INSTEAD of resolving `mosaic` on PATH and appending the probe @@ -83,12 +96,28 @@ class VersionCouplingError(Exception): silent pass, and never let its absence be treated as compatible.""" +def _resolve_probe_timeout(environ: Mapping[str, str]) -> float: + raw = environ.get(PROBE_TIMEOUT_OVERRIDE_VAR) + if not raw: + return PROBE_TIMEOUT_SECONDS + try: + seconds = float(raw) + except ValueError: + return PROBE_TIMEOUT_SECONDS + if seconds <= 0 or seconds != seconds or seconds == float("inf"): + return PROBE_TIMEOUT_SECONDS + return seconds + + def _resolve_probe_command(environ: Mapping[str, str]) -> list[str] | None: override = environ.get(MOSAIC_COMMAND_OVERRIDE_VAR) if override: parsed = shlex.split(override) return parsed or None - resolved = shutil.which("mosaic") + # Resolve against the caller's PATH, not the ambient process one. The + # function is handed an `environ` and honoured it only for the override + # var, so a caller passing an explicit PATH was silently ignored here. + resolved = shutil.which("mosaic", path=environ.get("PATH")) if resolved is None: return None return [resolved, LEASE_CAPABILITY_PROBE_COMMAND] @@ -116,7 +145,7 @@ def default_probe_activation_capability( command, capture_output=True, text=True, - timeout=PROBE_TIMEOUT_SECONDS, + timeout=_resolve_probe_timeout(source_environment), check=False, ) except (OSError, subprocess.TimeoutExpired, ValueError): diff --git a/packages/mosaic/src/mutator-gate/version_coupling_unittest.py b/packages/mosaic/src/mutator-gate/version_coupling_unittest.py index 6151170e..a3e88190 100644 --- a/packages/mosaic/src/mutator-gate/version_coupling_unittest.py +++ b/packages/mosaic/src/mutator-gate/version_coupling_unittest.py @@ -187,6 +187,43 @@ class ProbeActivationCapabilityTest(unittest.TestCase): ) self.assertIsNone(result) + def test_probe_budget_exceeds_real_cli_boot_time(self) -> None: + # Regression: the budget was 2.0 s while the probe boots the whole Node + # CLI, measured at 3.0-3.7 s on an idle VM and 3.55-3.61 s on web1. The + # gate is fail-closed, so an expiry is indistinguishable from "no + # capability" and every seat launch was denied with a version-skew + # message no upgrade could fix. Guards the ordering, not a stopwatch. + self.assertGreater(VERSION_GATE.PROBE_TIMEOUT_SECONDS, 10.0) + + def test_probe_timeout_override_is_honoured_and_rejects_unusable_values(self) -> None: + self.assertEqual(VERSION_GATE._resolve_probe_timeout({}), VERSION_GATE.PROBE_TIMEOUT_SECONDS) + self.assertEqual( + VERSION_GATE._resolve_probe_timeout( + {VERSION_GATE.PROBE_TIMEOUT_OVERRIDE_VAR: "45"} + ), + 45.0, + ) + # An unusable override must fall back to the default rather than + # removing the bound: no override may make the gate wait forever. + for unusable in ("", "abc", "0", "-5", "nan", "inf"): + with self.subTest(value=unusable): + self.assertEqual( + VERSION_GATE._resolve_probe_timeout( + {VERSION_GATE.PROBE_TIMEOUT_OVERRIDE_VAR: unusable} + ), + VERSION_GATE.PROBE_TIMEOUT_SECONDS, + ) + + def test_command_resolution_honours_the_supplied_path(self) -> None: + # Regression: `shutil.which` was called without `path=`, so it read the + # ambient process PATH and silently ignored the environ handed in. On a + # host with `mosaic` installed that made the not-resolvable case pass + # only because the 2.0 s budget expired first — right answer, wrong + # reason, and it masked the timeout defect above. + self.assertIsNone( + VERSION_GATE._resolve_probe_command({"PATH": "/nonexistent-bin-dir-for-869-c4-test"}) + ) + class LaunchRuntimeVersionCouplingSeamTest(unittest.TestCase): """End-to-end (still fully faked) coverage of the seam as wired into