core: evaluate node semver ranges instead of guessing the leading major
resolve_node_bin() could not parse a range, so it fell back to "pin by the first number in the string" and took the newest installed version of that major. For openclaw's engines pin `>=22.19.0 <23 || >=23.11.0` that picked v22.17.0 — below the range's own floor — and baked it into the service's unit PATH. The daemon refused to start on it and crash-looped 260k+ times (~7.4s CPU per attempt, every ~5.5s, since before the journal's retention window). StartLimitBurst never tripped because five attempts span ~27s, outside the 10s StartLimitIntervalUSec. Parse the pin properly: split on `||`, expand each comparator (>=, >, <=, <, =, ^, ~, hyphen ranges, x/* wildcards, partial versions) into bounds with node-semver's partial semantics, and pick the newest installed version satisfying any alternative. openclaw now resolves to v24.18.0 via its second alternative. A pin this parser cannot read at all still falls back to newest-installed rather than guessing a version. When nothing satisfies the pin it now fails with the installed versions listed, so the next occurrence is diagnosable at apply time rather than six days later in a restart loop.
This commit is contained in:
@@ -27,12 +27,24 @@ import os
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
# A pin that is a bare, exact-ish version: "24", "24.14", "24.14.1" (optional "v").
|
||||
_EXACT_RE = re.compile(r"v?(\d+)(?:\.(\d+))?(?:\.(\d+))?$")
|
||||
# An installed nvm dir: v24.14.1
|
||||
_INSTALLED_RE = re.compile(r"v(\d+)\.(\d+)\.(\d+)$")
|
||||
# Wildcards/aliases that mean "newest installed".
|
||||
_NEWEST_ALIASES = {"", "*", "x", "node", "latest", "current", "lts", "lts/*"}
|
||||
# One comparator: an optional operator then a (possibly partial/wildcard) version,
|
||||
# e.g. ">=22.19.0", "^24.1", "~24", "24.x", "23".
|
||||
_PART = r"\d+|[xX*]"
|
||||
_COMPARATOR_RE = re.compile(
|
||||
rf"^(?P<op>>=|<=|>|<|=|\^|~)?\s*v?(?P<maj>{_PART})"
|
||||
rf"(?:\.(?P<min>{_PART}))?(?:\.(?P<pat>{_PART}))?"
|
||||
r"(?:[-+][0-9A-Za-z.\-]+)?$"
|
||||
)
|
||||
# A hyphen range: "22.19.0 - 23.11.0".
|
||||
_HYPHEN_RE = re.compile(r"^(.+?)\s+-\s+(.+)$")
|
||||
|
||||
Version = tuple[int, int, int]
|
||||
# A parsed comparator: (op, version), ANDed within one range alternative.
|
||||
Bound = tuple[str, Version]
|
||||
|
||||
|
||||
class ToolchainError(Exception):
|
||||
@@ -83,17 +95,136 @@ def _installed() -> list[tuple[tuple[int, int, int], Path]]:
|
||||
return out
|
||||
|
||||
|
||||
def _num(tok: str | None) -> int | None:
|
||||
"""A version part as an int, or None when absent or a wildcard (``x``/``*``)."""
|
||||
if tok is None or tok in ("x", "X", "*"):
|
||||
return None
|
||||
return int(tok)
|
||||
|
||||
|
||||
def _bounds(token: str) -> list[Bound] | None:
|
||||
"""One comparator as a list of ANDed (op, version) bounds.
|
||||
|
||||
Returns ``[]`` for an unconstrained token (``*``/``x``) and ``None`` when the
|
||||
token isn't a comparator this parser understands. Partial versions follow
|
||||
node-semver: ``>22`` means ``>=23.0.0``, ``<23`` means ``<23.0.0``, a bare
|
||||
``24`` means ``>=24.0.0 <25.0.0``."""
|
||||
t = token.strip()
|
||||
if not t or t in ("*", "x", "X"):
|
||||
return []
|
||||
m = _COMPARATOR_RE.match(t)
|
||||
if not m:
|
||||
return None
|
||||
op = m["op"] or "="
|
||||
maj, minor, patch = _num(m["maj"]), _num(m["min"]), _num(m["pat"])
|
||||
if maj is None:
|
||||
return [] # "x" / "x.y" — unconstrained
|
||||
# How many parts were given concretely: 24 → 1, 24.14 → 2, 24.14.1 → 3.
|
||||
precision = 1 if minor is None else (2 if patch is None else 3)
|
||||
base: Version = (maj, minor or 0, patch or 0)
|
||||
|
||||
if op == ">=":
|
||||
return [(">=", base)]
|
||||
if op == ">":
|
||||
if precision == 1:
|
||||
return [(">=", (maj + 1, 0, 0))]
|
||||
if precision == 2:
|
||||
return [(">=", (maj, (minor or 0) + 1, 0))]
|
||||
return [(">", base)]
|
||||
if op == "<":
|
||||
return [("<", base)]
|
||||
if op == "<=":
|
||||
if precision == 1:
|
||||
return [("<", (maj + 1, 0, 0))]
|
||||
if precision == 2:
|
||||
return [("<", (maj, (minor or 0) + 1, 0))]
|
||||
return [("<=", base)]
|
||||
if op == "^":
|
||||
# Caret: up to the next non-zero leftmost part.
|
||||
if maj > 0:
|
||||
hi: Version = (maj + 1, 0, 0)
|
||||
elif (minor or 0) > 0:
|
||||
hi = (0, (minor or 0) + 1, 0)
|
||||
else:
|
||||
hi = (0, 0, (patch or 0) + 1)
|
||||
return [(">=", base), ("<", hi)]
|
||||
if op == "~":
|
||||
hi = (maj + 1, 0, 0) if precision == 1 else (maj, (minor or 0) + 1, 0)
|
||||
return [(">=", base), ("<", hi)]
|
||||
# "=" or bare: a partial pin is the range it prefixes.
|
||||
if precision == 1:
|
||||
return [(">=", base), ("<", (maj + 1, 0, 0))]
|
||||
if precision == 2:
|
||||
return [(">=", base), ("<", (maj, (minor or 0) + 1, 0))]
|
||||
return [("==", base)]
|
||||
|
||||
|
||||
def _parse_range(pin: str) -> list[list[Bound]] | None:
|
||||
"""A semver range as alternatives (``||``) of ANDed bounds.
|
||||
|
||||
Returns ``None`` when no alternative could be parsed — the caller then falls
|
||||
back to "newest installed" rather than guessing a version."""
|
||||
alternatives: list[list[Bound]] = []
|
||||
for alt in pin.split("||"):
|
||||
alt = alt.strip()
|
||||
hyphen = _HYPHEN_RE.match(alt)
|
||||
tokens = (
|
||||
[f">={hyphen[1]}", f"<={hyphen[2]}"]
|
||||
if hyphen
|
||||
else [t for t in re.split(r"[\s,]+", alt) if t]
|
||||
)
|
||||
bounds: list[Bound] = []
|
||||
ok = True
|
||||
for tok in tokens:
|
||||
parsed = _bounds(tok)
|
||||
if parsed is None:
|
||||
ok = False
|
||||
break
|
||||
bounds.extend(parsed)
|
||||
if ok:
|
||||
alternatives.append(bounds)
|
||||
return alternatives or None
|
||||
|
||||
|
||||
def _satisfies(ver: Version, bounds: list[Bound]) -> bool:
|
||||
"""Does a concrete version meet every bound in one alternative?"""
|
||||
for op, target in bounds:
|
||||
if op == ">=" and not ver >= target:
|
||||
return False
|
||||
if op == ">" and not ver > target:
|
||||
return False
|
||||
if op == "<=" and not ver <= target:
|
||||
return False
|
||||
if op == "<" and not ver < target:
|
||||
return False
|
||||
if op == "==" and ver != target:
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
def _install_hint(pin: str) -> str:
|
||||
"""What to tell the user to `nvm install`.
|
||||
|
||||
An exact-ish pin installs verbatim; for a range, name the highest major it
|
||||
mentions — the newest line the program declares it supports."""
|
||||
majors = [int(m) for m in re.findall(r"(\d+)", pin)]
|
||||
if _COMPARATOR_RE.match(pin.strip()) and not re.search(r"[<>=^~|]", pin):
|
||||
return pin
|
||||
return str(max(majors)) if majors else "--lts"
|
||||
|
||||
|
||||
def resolve_node_bin(source: Path | None) -> Path | None:
|
||||
"""Resolve a program's pinned node version to its ``.../bin`` dir on this box.
|
||||
|
||||
Returns ``None`` when the program pins nothing — wildpc injects no node, it does
|
||||
not guess. Raises :class:`ToolchainError` when a version *is* pinned but no
|
||||
matching version is installed.
|
||||
installed version satisfies it.
|
||||
|
||||
Matching: an exact-ish pin (``24`` / ``24.14`` / ``24.14.1``) matches installed
|
||||
versions by that precision, newest wins. A range or alias (``>=24``, ``^24.1``,
|
||||
``lts/*``) is pinned by its leading major (or newest installed if it names none)
|
||||
— precise pins belong in ``.node-version``."""
|
||||
Matching is real semver-range evaluation: every ``||`` alternative is parsed
|
||||
into comparators and the **newest installed version satisfying any alternative**
|
||||
wins. So ``>=22.19.0 <23 || >=23.11.0`` selects an installed 24.x, and never an
|
||||
installed 22.17.0 — which sits below the range's own floor. A pin this parser
|
||||
can't read at all falls back to newest-installed rather than guessing."""
|
||||
if source is None:
|
||||
return None
|
||||
pin = read_node_pin(source)
|
||||
@@ -106,28 +237,23 @@ def resolve_node_bin(source: Path | None) -> Path | None:
|
||||
return installed[0][1]
|
||||
raise ToolchainError(
|
||||
f"node {pin!r} pinned (in {source}) but no node is installed under "
|
||||
f"{node_versions_dir()} — run `nvm install {pin}`"
|
||||
f"{node_versions_dir()} — run `nvm install {_install_hint(pin)}`"
|
||||
)
|
||||
|
||||
low = pin.strip().lower()
|
||||
if low in _NEWEST_ALIASES:
|
||||
if pin.strip().lower() in _NEWEST_ALIASES:
|
||||
return newest_or_raise()
|
||||
|
||||
exact = _EXACT_RE.match(low)
|
||||
if exact:
|
||||
parts = [int(g) for g in exact.groups() if g is not None]
|
||||
else:
|
||||
# A range/alias (>=24, ^24.1, ~24, 24.x): pin by leading major only.
|
||||
major = re.search(r"\d+", low)
|
||||
if not major:
|
||||
alternatives = _parse_range(pin)
|
||||
if alternatives is None:
|
||||
return newest_or_raise()
|
||||
parts = [int(major.group())]
|
||||
precision = len(parts)
|
||||
|
||||
for ver, bindir in installed: # newest first
|
||||
if list(ver[:precision]) == parts:
|
||||
if any(_satisfies(ver, bounds) for bounds in alternatives):
|
||||
return bindir
|
||||
|
||||
have = ", ".join(".".join(str(p) for p in v) for v, _ in installed) or "none"
|
||||
raise ToolchainError(
|
||||
f"node {pin!r} pinned (in {source}) but not installed under "
|
||||
f"{node_versions_dir()} — run `nvm install {pin}`"
|
||||
f"node {pin!r} pinned (in {source}) but no installed version satisfies it "
|
||||
f"(installed: {have}) under {node_versions_dir()} — "
|
||||
f"run `nvm install {_install_hint(pin)}`"
|
||||
)
|
||||
|
||||
@@ -118,4 +118,84 @@ class TestResolveNodeBin:
|
||||
|
||||
def test_default_dir_is_nvm(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.delenv("WILDPC_NODE_VERSIONS_DIR", raising=False)
|
||||
assert toolchains.node_versions_dir() == Path.home() / ".nvm" / "versions" / "node"
|
||||
assert (
|
||||
toolchains.node_versions_dir() == Path.home() / ".nvm" / "versions" / "node"
|
||||
)
|
||||
|
||||
|
||||
class TestSemverRanges:
|
||||
"""Range pins are evaluated, not reduced to their leading major.
|
||||
|
||||
The regression these guard: openclaw pins `>=22.19.0 <23 || >=23.11.0`, and a
|
||||
leading-major heuristic picked the newest installed 22.x — v22.17.0, *below*
|
||||
the range's own floor — baking a node into the systemd unit that the program
|
||||
refuses to run on."""
|
||||
|
||||
def test_compound_range_uses_later_alternative(
|
||||
self, tmp_path: Path, nvm: Path
|
||||
) -> None:
|
||||
_install(nvm, "v24.18.0", "v22.17.0", "v20.19.3")
|
||||
(tmp_path / "package.json").write_text(
|
||||
'{"engines": {"node": ">=22.19.0 <23 || >=23.11.0"}}'
|
||||
)
|
||||
assert resolve_node_bin(tmp_path) == nvm / "v24.18.0" / "bin"
|
||||
|
||||
def test_never_picks_below_lower_bound(self, tmp_path: Path, nvm: Path) -> None:
|
||||
# Only a 22.x that fails the pin's own >=22.19.0 floor is installed.
|
||||
_install(nvm, "v22.17.0", "v20.19.3")
|
||||
(tmp_path / "package.json").write_text(
|
||||
'{"engines": {"node": ">=22.19.0 <23 || >=23.11.0"}}'
|
||||
)
|
||||
with pytest.raises(ToolchainError, match="no installed version satisfies"):
|
||||
resolve_node_bin(tmp_path)
|
||||
|
||||
def test_upper_bound_excludes_newer(self, tmp_path: Path, nvm: Path) -> None:
|
||||
_install(nvm, "v24.18.0", "v22.19.1")
|
||||
(tmp_path / "package.json").write_text('{"engines": {"node": ">=22.19.0 <23"}}')
|
||||
assert resolve_node_bin(tmp_path) == nvm / "v22.19.1" / "bin"
|
||||
|
||||
def test_error_names_installed_versions(self, tmp_path: Path, nvm: Path) -> None:
|
||||
_install(nvm, "v22.17.0")
|
||||
(tmp_path / "package.json").write_text('{"engines": {"node": ">=24"}}')
|
||||
with pytest.raises(ToolchainError, match=r"installed: 22\.17\.0"):
|
||||
resolve_node_bin(tmp_path)
|
||||
|
||||
def test_tilde_pins_minor(self, tmp_path: Path, nvm: Path) -> None:
|
||||
_install(nvm, "v24.15.0", "v24.14.9", "v24.14.1")
|
||||
(tmp_path / ".nvmrc").write_text("~24.14.1")
|
||||
assert resolve_node_bin(tmp_path) == nvm / "v24.14.9" / "bin"
|
||||
|
||||
def test_caret_excludes_next_major(self, tmp_path: Path, nvm: Path) -> None:
|
||||
_install(nvm, "v25.0.0", "v24.14.1")
|
||||
(tmp_path / "package.json").write_text('{"engines": {"node": "^24.1.0"}}')
|
||||
assert resolve_node_bin(tmp_path) == nvm / "v24.14.1" / "bin"
|
||||
|
||||
def test_gt_partial_means_next_major(self, tmp_path: Path, nvm: Path) -> None:
|
||||
# node-semver: ">22" is >=23.0.0, so an installed 22.19.0 does not match.
|
||||
_install(nvm, "v23.1.0", "v22.19.0")
|
||||
(tmp_path / "package.json").write_text('{"engines": {"node": ">22"}}')
|
||||
assert resolve_node_bin(tmp_path) == nvm / "v23.1.0" / "bin"
|
||||
|
||||
def test_exact_pin_excludes_neighbours(self, tmp_path: Path, nvm: Path) -> None:
|
||||
_install(nvm, "v24.14.9", "v24.14.1")
|
||||
(tmp_path / ".node-version").write_text("=24.14.1")
|
||||
assert resolve_node_bin(tmp_path) == nvm / "v24.14.1" / "bin"
|
||||
|
||||
def test_hyphen_range(self, tmp_path: Path, nvm: Path) -> None:
|
||||
_install(nvm, "v24.0.0", "v23.5.0", "v22.1.0")
|
||||
(tmp_path / "package.json").write_text(
|
||||
'{"engines": {"node": "22.19.0 - 23.11.0"}}'
|
||||
)
|
||||
assert resolve_node_bin(tmp_path) == nvm / "v23.5.0" / "bin"
|
||||
|
||||
def test_x_wildcard_minor(self, tmp_path: Path, nvm: Path) -> None:
|
||||
_install(nvm, "v25.0.0", "v24.14.1")
|
||||
(tmp_path / ".node-version").write_text("24.x")
|
||||
assert resolve_node_bin(tmp_path) == nvm / "v24.14.1" / "bin"
|
||||
|
||||
def test_unparseable_pin_falls_back_to_newest(
|
||||
self, tmp_path: Path, nvm: Path
|
||||
) -> None:
|
||||
_install(nvm, "v24.14.1", "v20.10.0")
|
||||
(tmp_path / ".nvmrc").write_text("lts/hydrogen")
|
||||
assert resolve_node_bin(tmp_path) == nvm / "v24.14.1" / "bin"
|
||||
|
||||
Reference in New Issue
Block a user