From bc42742bb9b1536290caf49a993d50173f237678 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 20:55:19 +0000 Subject: [PATCH 1/2] fix(deps): declare the Node 22 floor the repo already builds on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `engines.node` said `>=20.6.0`, but nothing in the repo has built or tested on Node 20 for some time: - `ci.yml`, `e2e-tests.yml` and `security.yml` all pin `node-version: 22`. - `apps/web/Dockerfile` builds `FROM node:22-slim`. - Core runtime dependencies declare `>=22.0.0` in the lockfile — `openai@7.3.0`, `ai@7.0.47`, `@ai-sdk/gateway@4.0.36` and the whole `@supabase/*` client set. The gap was silent by construction. `npm install` only emits an `EBADENGINE` warning when a package wants a newer Node than the root declares, so no check ever went red. The consequence is on Vercel, which reads `engines.node` to select the runtime for `apps/web`: a floor of `>=20.6.0` permits it to pick a Node 20 runtime on which those dependencies are unsupported. Raise the floor to `>=22.0.0` so the advertised support matches what is actually built, tested and shipped. `npm` moves to `>=10.0.0` to match — Node 22 ships npm 10, so `>=8.0.0` described a combination that cannot occur. `tests/unit/test_node_engines_floor.py` locks the invariant to the toolchain: the advertised major must equal the major every CI workflow pins and the major the production image ships, and must satisfy the core runtime dependencies' locked floors. Against the previous `>=20.6.0` those five assertions fail; with this change all ten pass. Deliberately not asserted is that the floor dominates *every* dependency's declared floor. Some dev tooling (`vitest@4.1.10`, `eslint-visitor-keys@5.0.1`) declares `>=24` and some optional platform binaries declare `>=22.12`, yet CI is green on Node 22 because those floors are advisory. Encoding dominance would assert a rule the repo does not follow and would force the floor past the version it actually runs. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NBdBTcFjhyjyKP9bDuj4gZ --- package.json | 4 +- tests/unit/test_node_engines_floor.py | 142 ++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 2 deletions(-) create mode 100644 tests/unit/test_node_engines_floor.py diff --git a/package.json b/package.json index ba5c0dd4c..8ea683991 100644 --- a/package.json +++ b/package.json @@ -16,8 +16,8 @@ "apps/*" ], "engines": { - "node": ">=20.6.0", - "npm": ">=8.0.0" + "node": ">=22.0.0", + "npm": ">=10.0.0" }, "devDependencies": { "@modelcontextprotocol/sdk": "^1.30.0", diff --git a/tests/unit/test_node_engines_floor.py b/tests/unit/test_node_engines_floor.py new file mode 100644 index 000000000..659bde212 --- /dev/null +++ b/tests/unit/test_node_engines_floor.py @@ -0,0 +1,142 @@ +"""Regression tests for the declared Node engine floor. + +The root `package.json` advertises the Node versions this repo supports. That +claim is load-bearing in a place that never fails loudly: Vercel reads +`engines.node` to pick the runtime for `apps/web`. `npm install` only *warns* +(`EBADENGINE`) when a package wants a newer Node, so a stale floor never turns a +check red. + +Before this test the floor said `>=20.6.0` while every gate in the repo ran on +Node 22 (`ci.yml`, `e2e-tests.yml`, `security.yml`, and `apps/web/Dockerfile`), +and core runtime dependencies — `openai@7`, `ai@7`, the Supabase client — all +declare `>=22`. Node 20 was therefore advertised but never built or tested. + +The invariant locked in here is *toolchain consistency*: the version the repo +promises must be the version it actually builds and tests on. Deliberately not +asserted is "the floor dominates every dependency's declared floor" — some dev +tooling (`vitest`, `eslint-visitor-keys`) declares `>=24` and some optional +platform binaries declare `>=22.12`, yet CI passes on Node 22 because those +floors are advisory. Asserting dominance would encode a rule the repo does not +actually follow. +""" + +from __future__ import annotations + +import json +import re +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +PACKAGE_JSON = REPO_ROOT / "package.json" +PACKAGE_LOCK = REPO_ROOT / "package-lock.json" +WEB_DOCKERFILE = REPO_ROOT / "apps/web/Dockerfile" + +# Workflows whose Node pin defines "the version we actually test on". +CI_WORKFLOWS = ( + REPO_ROOT / ".github/workflows/ci.yml", + REPO_ROOT / ".github/workflows/e2e-tests.yml", + REPO_ROOT / ".github/workflows/security.yml", +) + +# Direct runtime dependencies whose floor the deployed app must satisfy. +RUNTIME_DEPS_UNDER_TEST = ("openai", "ai", "@ai-sdk/gateway") + +_MIN_VERSION = re.compile(r">=\s*v?(\d+)(?:\.(\d+))?(?:\.(\d+))?") +_NODE_VERSION_KEY = re.compile(r"""node-version:\s*['"]?(\d+)""") +_DOCKER_NODE = re.compile(r"^FROM\s+node:(\d+)", re.MULTILINE) + + +def _parse_floor(spec: str | None) -> tuple[int, int, int] | None: + """Return the `>=` floor of a semver range as a comparable tuple.""" + if not spec: + return None + match = _MIN_VERSION.search(spec) + if match is None: + return None + major, minor, patch = match.groups() + return (int(major), int(minor or 0), int(patch or 0)) + + +def _load(path: Path) -> dict: + assert path.exists(), f"{path} should exist" + return json.loads(path.read_text()) + + +def _declared_floor() -> tuple[int, int, int]: + engines = _load(PACKAGE_JSON).get("engines") or {} + floor = _parse_floor(engines.get("node")) + assert ( + floor is not None + ), "root package.json must declare engines.node as a >= range" + return floor + + +def _locked_floor(name: str) -> tuple[int, int, int] | None: + """Strictest declared Node floor for any copy of `name` in the lockfile.""" + strictest: tuple[int, int, int] | None = None + for path, meta in (_load(PACKAGE_LOCK).get("packages") or {}).items(): + if not path or not isinstance(meta, dict): + continue + if path.split("node_modules/")[-1] != name: + continue + floor = _parse_floor((meta.get("engines") or {}).get("node")) + if floor is not None and (strictest is None or floor > strictest): + strictest = floor + return strictest + + +def test_declared_floor_matches_the_version_ci_tests_on() -> None: + """The advertised major must be the major every CI workflow pins.""" + declared_major = _declared_floor()[0] + for workflow in CI_WORKFLOWS: + assert workflow.exists(), f"{workflow} should exist" + pinned = {int(m) for m in _NODE_VERSION_KEY.findall(workflow.read_text())} + assert pinned, f"{workflow.name} should pin a node-version" + assert pinned == {declared_major}, ( + f"{workflow.name} tests on Node {sorted(pinned)} but package.json " + f"advertises >={declared_major}. The repo must promise what it tests." + ) + + +def test_declared_floor_matches_the_version_the_image_ships() -> None: + """The advertised major must be the major the production image is built on.""" + assert WEB_DOCKERFILE.exists(), f"{WEB_DOCKERFILE} should exist" + tags = {int(m) for m in _DOCKER_NODE.findall(WEB_DOCKERFILE.read_text())} + assert tags, "apps/web/Dockerfile should build FROM a pinned node: tag" + declared_major = _declared_floor()[0] + assert tags == {declared_major}, ( + f"apps/web/Dockerfile ships Node {sorted(tags)} but package.json " + f"advertises >={declared_major}." + ) + + +@pytest.mark.parametrize("dependency", RUNTIME_DEPS_UNDER_TEST) +def test_declared_floor_satisfies_core_runtime_dependencies(dependency: str) -> None: + """A runtime dep must never demand a newer Node than Vercel is told to use.""" + required = _locked_floor(dependency) + assert required is not None, ( + f"expected {dependency} to declare engines.node in package-lock.json; " + "the fixture has drifted and this test would otherwise be vacuous" + ) + declared = _declared_floor() + assert declared >= required, ( + f"{dependency} requires Node >={'.'.join(map(str, required))} but " + f"package.json advertises >={'.'.join(map(str, declared))}, so Vercel " + "may select a runtime the dependency does not support" + ) + + +@pytest.mark.parametrize( + ("spec", "expected"), + [ + (">=22.0.0", (22, 0, 0)), + (">=22", (22, 0, 0)), + (">= 20.6.0", (20, 6, 0)), + ("^20.0.0", None), + (None, None), + ], +) +def test_parse_floor(spec: str | None, expected: tuple[int, int, int] | None) -> None: + assert _parse_floor(spec) == expected From 339ac79dfe94f947a4f509a1e34d53237f97c61d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 20:59:31 +0000 Subject: [PATCH 2/2] fix(tests): stop reading union engine ranges as a minimum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses both findings from CodeRabbit's review on #1479. `_parse_floor` used `re.search`, so it pulled the trailing `>=` clause out of a union range and reported it as a floor. That is wrong, and it misled the original analysis of this very change: `vitest@4.1.10` is `^20.0.0 || ^22.0.0 || >=24.0.0` and `eslint-visitor-keys@5.0.1` is `^20.19.0 || ^22.13.0 || >=24` — both explicitly admit Node 22. They were described as "requiring >=24" only because the parser said so. The same applies to `chrome-devtools-mcp@1.6.0` and `vite@8.2.0`. The regex is now anchored, so a range is reduced to a floor only when it is entirely one `>=X[.Y[.Z]]` clause. Unions and compound ranges return `None`, and a selected runtime dependency that adopts such a form now fails with a message telling the reader to re-read the range by hand rather than letting the assertion pass vacuously. Parser cases cover the four real union ranges in the lockfile plus `>=22 <24`. Workflow Node pins are now read with `yaml.safe_load` and extracted from `with["node-version"]`, matching the precedent in `tests/unit/test_auto_label_workflow.py`. The previous text match would have counted a commented-out `# node-version: 20` as a pin and failed the test while the workflow still ran Node 22. Covered by a test using a fixture that contains a commented pin and a prose mention. The Dockerfile check stays a text assertion: it anchors on `FROM` at line start and so cannot match a comment. PyYAML is already a declared dependency (`pyproject.toml`, `requirements.txt`) and several existing workflow tests import it. No change to the invariant or its scope: floor vs CI pins, production image, and the three selected runtime dependencies. `pytest tests/unit/test_node_engines_floor.py` -> 19 passed; reverting `engines` to `>=20.6.0` still fails exactly the 5 assertions that encode the fix. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NBdBTcFjhyjyKP9bDuj4gZ --- tests/unit/test_node_engines_floor.py | 107 ++++++++++++++++++++++---- 1 file changed, 94 insertions(+), 13 deletions(-) diff --git a/tests/unit/test_node_engines_floor.py b/tests/unit/test_node_engines_floor.py index 659bde212..ac6658fe5 100644 --- a/tests/unit/test_node_engines_floor.py +++ b/tests/unit/test_node_engines_floor.py @@ -12,21 +12,30 @@ declare `>=22`. Node 20 was therefore advertised but never built or tested. The invariant locked in here is *toolchain consistency*: the version the repo -promises must be the version it actually builds and tests on. Deliberately not -asserted is "the floor dominates every dependency's declared floor" — some dev -tooling (`vitest`, `eslint-visitor-keys`) declares `>=24` and some optional -platform binaries declare `>=22.12`, yet CI passes on Node 22 because those -floors are advisory. Asserting dominance would encode a rule the repo does not -actually follow. +promises must be the version it actually builds and tests on. + +Deliberately not asserted is "the floor dominates every dependency's declared +floor". Much of the tree uses *union* ranges — `vitest@4.1.10` is +`^20.0.0 || ^22.0.0 || >=24.0.0`, `eslint-visitor-keys@5.0.1` is +`^20.19.0 || ^22.13.0 || >=24` — whose trailing `>=` branch is one alternative +among several, not an unconditional minimum. Both explicitly admit Node 22. +Reading such a range as "requires >=24" is simply wrong, so `_parse_floor` +below refuses to guess: it accepts only a range that is a single, complete +`>=` clause and returns `None` for every other form. A selected runtime +dependency that adopts an unsupported form fails loudly rather than silently +contributing a fabricated floor. """ from __future__ import annotations import json import re +from collections.abc import Iterator from pathlib import Path +from typing import Any import pytest +import yaml REPO_ROOT = Path(__file__).resolve().parents[2] PACKAGE_JSON = REPO_ROOT / "package.json" @@ -43,16 +52,52 @@ # Direct runtime dependencies whose floor the deployed app must satisfy. RUNTIME_DEPS_UNDER_TEST = ("openai", "ai", "@ai-sdk/gateway") -_MIN_VERSION = re.compile(r">=\s*v?(\d+)(?:\.(\d+))?(?:\.(\d+))?") -_NODE_VERSION_KEY = re.compile(r"""node-version:\s*['"]?(\d+)""") +# Anchored: the whole range must be one `>=` clause. An unanchored search would +# happily pull `>=24.0.0` out of `^20.0.0 || ^22.0.0 || >=24.0.0` and report a +# floor of 24 for a range that accepts Node 20. +_MIN_VERSION = re.compile(r"^>=\s*v?(\d+)(?:\.(\d+))?(?:\.(\d+))?$") _DOCKER_NODE = re.compile(r"^FROM\s+node:(\d+)", re.MULTILINE) +def _walk(node: Any) -> Iterator[Any]: + """Yield every mapping nested anywhere inside a parsed YAML document.""" + if isinstance(node, dict): + yield node + for value in node.values(): + yield from _walk(value) + elif isinstance(node, list): + for item in node: + yield from _walk(item) + + +def _pinned_node_majors(workflow: Path) -> set[int]: + """Every Node major a workflow pins via `actions/setup-node`. + + Parsed rather than text-matched so a commented-out or documentation-only + `node-version:` line cannot fail this test spuriously. + """ + document = yaml.safe_load(workflow.read_text()) + majors: set[int] = set() + for mapping in _walk(document): + pin = (mapping.get("with") or {}).get("node-version") if mapping else None + if pin is None: + continue + match = re.match(r"v?(\d+)", str(pin).strip()) + if match: + majors.add(int(match.group(1))) + return majors + + def _parse_floor(spec: str | None) -> tuple[int, int, int] | None: - """Return the `>=` floor of a semver range as a comparable tuple.""" + """Return the floor of a semver range, or `None` if it is not a bare `>=`. + + Only a range that is *entirely* one `>=X[.Y[.Z]]` clause has an unambiguous + minimum. Unions (`a || b`) and compound ranges (`>=22 <24`) return `None` + rather than a guess — see the module docstring. + """ if not spec: return None - match = _MIN_VERSION.search(spec) + match = _MIN_VERSION.match(spec.strip()) if match is None: return None major, minor, patch = match.groups() @@ -92,7 +137,7 @@ def test_declared_floor_matches_the_version_ci_tests_on() -> None: declared_major = _declared_floor()[0] for workflow in CI_WORKFLOWS: assert workflow.exists(), f"{workflow} should exist" - pinned = {int(m) for m in _NODE_VERSION_KEY.findall(workflow.read_text())} + pinned = _pinned_node_majors(workflow) assert pinned, f"{workflow.name} should pin a node-version" assert pinned == {declared_major}, ( f"{workflow.name} tests on Node {sorted(pinned)} but package.json " @@ -117,8 +162,12 @@ def test_declared_floor_satisfies_core_runtime_dependencies(dependency: str) -> """A runtime dep must never demand a newer Node than Vercel is told to use.""" required = _locked_floor(dependency) assert required is not None, ( - f"expected {dependency} to declare engines.node in package-lock.json; " - "the fixture has drifted and this test would otherwise be vacuous" + f"expected {dependency} to declare engines.node in package-lock.json as " + "a bare '>=' range. Either it is now absent, or it moved to a union or " + "compound range that _parse_floor deliberately refuses to reduce to a " + "single floor. Re-read the range by hand and either widen the parser or " + "drop this dependency from RUNTIME_DEPS_UNDER_TEST — do not let the " + "check pass vacuously." ) declared = _declared_floor() assert declared >= required, ( @@ -134,9 +183,41 @@ def test_declared_floor_satisfies_core_runtime_dependencies(dependency: str) -> (">=22.0.0", (22, 0, 0)), (">=22", (22, 0, 0)), (">= 20.6.0", (20, 6, 0)), + (" >=22.0.0 ", (22, 0, 0)), + ("v22.0.0", None), ("^20.0.0", None), + ("", None), (None, None), + # Union ranges have no single minimum: the trailing `>=` branch is one + # alternative, not a floor. These are the real ranges carried by + # vitest@4.1.10, eslint-visitor-keys@5.0.1, chrome-devtools-mcp@1.6.0 + # and vite@8.2.0 — every one of them admits Node 22, so reading the + # last branch as a requirement would be actively wrong. + ("^20.0.0 || ^22.0.0 || >=24.0.0", None), + ("^20.19.0 || ^22.13.0 || >=24", None), + ("^20.19.0 || >=22.12.0", None), + (">=22.0.0 || >=24.0.0", None), + # Compound ranges are bounded above, so `>=` alone does not describe them. + (">=22.0.0 <24.0.0", None), ], ) def test_parse_floor(spec: str | None, expected: tuple[int, int, int] | None) -> None: assert _parse_floor(spec) == expected + + +def test_pinned_node_majors_ignores_commented_and_unrelated_keys( + tmp_path: Path, +) -> None: + """A `node-version` outside a `with:` block must not be read as a pin.""" + workflow = tmp_path / "sample.yml" + workflow.write_text( + "jobs:\n" + " build:\n" + " steps:\n" + " - uses: actions/setup-node@v4\n" + ' with:\n node-version: "22"\n' + " # node-version: 18 <- a comment must be ignored\n" + " - name: not a setup step\n" + " env:\n NOTE: node-version 16 mentioned in prose\n" + ) + assert _pinned_node_majors(workflow) == {22}