Skip to content

docs: state an explicit minimum Node.js version in the README - #443

Open
AmaadMartin wants to merge 1 commit into
mainfrom
fix/readme-explicit-node-version-prerequisite
Open

docs: state an explicit minimum Node.js version in the README#443
AmaadMartin wants to merge 1 commit into
mainfrom
fix/readme-explicit-node-version-prerequisite

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):
    Closes: #issue_number
    Related: #issue_number

No open issue tracks this. Context: the wording being replaced was introduced by review feedback on google#526.

  1. Or, if no issue exists, describe the change:

Problem: README.md:55 — the only Node.js requirement statement anywhere in
the repository — reads:

> **Prerequisite:** ADK for TypeScript requires a current Node.js LTS release.

Three concrete defects:

  1. Not mechanically checkable. A reader cannot compare "a current Node.js
    LTS release" against node -v, and a reviewer cannot compare it against
    anything in the repo.
  2. Meaning drifts with no commit. The set of current LTS lines changes every
    October and again at each end-of-life, so the identical sentence asserts a
    strictly rising floor over time. A user who complied in one cycle is silently
    non-compliant later, with no diff to point at.
  3. Over-claims. It implies a non-LTS or older-LTS runtime is unsupported. The
    measured floor across the resolved dependency graph is Node 20, so the line
    both fails to warn a Node 18 user that they are genuinely below the floor and
    wrongly discourages a Node 20 user who is fine.

google#526 originally landed "Node.js 18 or newer"; the reviewer
correctly objected that 18 was end-of-life and that a hardcoded number goes
stale, and the fix removed the number entirely. That traded a wrong number for
no number. This PR restores an explicit floor, but derives it from the
repository's own dependency constraints rather than from a guess — which answers
the original objection instead of dodging it.

Solution: one line of Markdown.

-> **Prerequisite:** ADK for TypeScript requires a current Node.js LTS release.
+> **Prerequisite:** ADK for TypeScript requires Node.js 20.19 or newer.

git diff --stat is README.md | 2 +- — 1 file, 1 insertion, 1 deletion.

Where 20.19 comes from (measured, not asserted). There is no engines field
in any manifest, no .nvmrc/.node-version/.tool-versions, and both
workflows call actions/setup-node@v6 with no with: block — so CI pins
nothing and there is no tested-version set that could honestly be cited. The one
source of truth in the repo is the 797 engines.node constraints in the
committed package-lock.json. Evaluating every one of them with semver:

Node candidate Unsatisfied constraints
18.20.8 37
20.0.0 70
20.17.0 6
20.18.9 6
20.19.0 0
22.12.0 5 (eslint-visitor-keys wants ^22.13.0)
24.0.0 0

20.19.0 is therefore the exact minimum on the 20 line — the smallest version
that contradicts nothing in the lockfile. The six constraints that 20.18.9
violates are vite@7.3.5 (^20.19.0 || >=22.12.0) and five nested copies of
eslint-visitor-keys (^20.19.0 || ^22.13.0 || >=24). The tightest runtime
constraints are @google/genai@2.9.0 (>=20.0.0, direct dep of core) and
commander@14.0.3 (>=20, direct dep of dev). Nothing anywhere in the tree
requires a major above 20, and both the root and dev manifests pin
@types/node@^20.12.7, so the codebase is type-checked against the Node 20 API
surface. A claim of 22 or 24 would be unsupported by the repository.

Note this is a dependency floor, not a statement about Node's own support
calendar — deliberately, since any such statement would reintroduce the
calendar-drift defect being fixed here. The new text contains no time-relative
wording and makes no claim about which versions CI tests, because CI pins none.

Deliberately out of scope (called out so the omission is not read as an
oversight): declaring the floor in machine-readable form — engines.node in the
four manifests plus a pinned node-version/.nvmrc in the two workflows. That
is what would make this claim enforceable by npm and provable by CI, and it is a
user-visible change (npm emits EBADENGINE) that deserves its own review. It is
already queued as a separate task.

Collision check. Before writing anything I listed all 344 open PRs on the
fork (gh pr list --state open --limit 1000) and, separately, diffed the root
README.md blob of every remote branch against main: no branch changes the
Prerequisite: line
, so there is no competing implementation. Four PRs are
topically adjacent but file-disjoint — #133 (.nvmrc + engines.node + CI
pins), #416, #428 and #406 (CI Node pinning) — none of them touches
README.md. This PR is branched from main rather than stacked, because the
diffs do not overlap and stacking would drag eight unrelated files in.

PR title. Titled docs: on purpose. release-please-config.json links
main, adk, devtools and integrations under one linked-versions group,
and merges here are squashed on the PR title — so a feat:/fix: title would
cut a release of all four packages for a one-line docs edit. docs: bumps
nothing.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.

This is a small documentation fix: zero executable lines are added, so there is
nothing to unit test and the new-line-coverage requirement is vacuously
satisfied (0 new lines, 0 uncovered).

No test was added, on purpose. A test that reads README.md and matches
/Node\.js 20\.19/ would be a change-detector anti-test: it pins marketing copy,
breaks on any future rewording, and verifies nothing about ADK's behaviour. The
durable enforcement mechanism for a Node floor is engines.node plus a pinned CI
node-version, which is the separately queued follow-up described above — not a
Vitest assertion.

In place of the usual "prove the test can fail" mutation, the number itself was
falsified against the lockfile: the bisect in the table above shows 20.18.9
leaves 6 unsatisfied constraints and 20.19.0 leaves 0, so the claim would have
been detectably wrong at any lower value. Reproduce with:

node -e '
const semver=require("semver"), l=require("./package-lock.json");
const bad=v=>Object.entries(l.packages).filter(([,p])=>p.engines?.node
  && !semver.satisfies(v,p.engines.node)).length;
for (const v of ["18.20.8","20.18.9","20.19.0","24.0.0"]) console.log(v, bad(v));
'

Unit Tests:
[ ] I have added or updated unit tests for my change. — not applicable, see above.
[x] All unit tests pass locally. — the repo suite was not run in full (a
Markdown-only diff cannot perturb it, and full-suite runs are avoided);
a targeted run confirms the tree is healthy:
npx vitest run --project unit:core core/test/utils/case_utils_test.ts
→ 1 file, 6 tests passed.

Local validation on the exact pushed commit (e949811e) — every gate the
validation workflow runs, all exit 0:

Command Result
npm install ok (package-lock.json unchanged afterwards)
npm run build ok
npm run format:check ok (globs **/*.ts; does not read README.md)
npx prettier --check README.md ok — the check that actually covers this file
npm run lint ok
npm run docs:check ok (typedoc.json sets "readme": "none", so this is a no-regression smoke check here)
npx secretlint "**/*" ok
npx vitest run --project unit:core core/test/utils/case_utils_test.ts 6 passed

Postconditions verified: grep -n LTS README.md → no hits; grep -n 20.19 README.md → exactly one hit on line 55; line length 71 (under the 80-column
convention .prettierrc.js sets for this file); LF endings, no trailing
whitespace, single trailing newline; git status --short shows exactly one
modified path.

Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.

  1. Open the Files changed tab (or any Markdown renderer) and confirm the
    Installation section shows one blockquote reading Prerequisite: ADK for
    TypeScript requires Node.js 20.19 or newer., correctly styled, immediately
    above the npm install @google/adk fence, with one blank line either side.
  2. Run node -v and confirm the sentence is now decidable in a single
    comparison — the exact property the previous wording lacked.
  3. Cross-check the number against the lockfile with the snippet above and
    confirm nothing in the resolved tree excludes Node 20.19.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas. — n/a, Markdown prose only.
[x] I have added tests that prove my fix is effective or that my feature works. — no test added, deliberately; see the Testing Plan for the reasoning and for the lockfile bisect used instead.
[x] New and existing unit tests pass locally with my changes.

CI

All checks pass on e949811e: run-tests (ubuntu-latest), run-tests (macos-latest), run-tests (windows-latest), the cross-language run-tests
job, check-license and auto-assign.

Disclosing one detail rather than hiding it: on the first attempt
run-tests (windows-latest) failed with
core/test/code_executors/unsafe_local_code_executor_test.ts > should execute shell code and return stdout — Test timed out in 5000ms (1 failed / 2679
passed). That is a pre-existing Windows flake, not a regression, and the
evidence is:

  • This diff is one line of Markdown; it adds no executable code and cannot
    reach a shell-execution test.
  • The base commit itself (b390217e, main, zero changes) also fails
    run-tests (windows-latest) — run 30669370416 — there on a different
    test, tests/integration/adk_web/webui_test.ts. Different test each run is
    the signature of a Windows timing flake, and Windows is already red on main.
  • npx vitest run --project unit:core core/test/code_executors/unsafe_local_code_executor_test.ts
    passes locally: 18/18.
  • Re-running only the failed job on the identical commit passed (9m33s), with
    no code change in between.

The underlying Windows-flake work is already tracked separately; it is out of
scope here.

The Installation prerequisite read "a current Node.js LTS release", which is
not mechanically checkable and silently asserts a higher floor every time a
new LTS line opens. Replace it with the concrete floor the repository can
actually justify.

20.19 is the smallest version that satisfies every engines.node constraint in
the committed package-lock.json: 20.18.9 violates 6 of them (vite's
^20.19.0 || >=22.12.0 and five copies of eslint-visitor-keys'
^20.19.0 || ^22.13.0 || >=24), while 20.19.0 violates none. Nothing in the
resolved tree requires a major above 20.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant