Chore: bump @types/node off the end-of-life Node 20 line - #444
Open
AmaadMartin wants to merge 1 commit into
Open
Chore: bump @types/node off the end-of-life Node 20 line#444AmaadMartin wants to merge 1 commit into
AmaadMartin wants to merge 1 commit into
Conversation
AmaadMartin
force-pushed
the
fix/types-node-22-lts
branch
from
August 1, 2026 09:26
61848ff to
c841d0d
Compare
The root and dev manifests both pinned "@types/node": "^20.12.7", which resolved to 20.19.43. The Node 20 "Iron" line ended on 2026-04-30 per the nodejs/Release schedule, so the compiler was checking every TypeScript file in the repo against the API surface of a runtime that is no longer supported. That hides two things: APIs the runtime has but the typings omit, and - the dangerous direction - APIs removed after 20 that the typings still describe as present. Move both manifests to ^22.20.1, the highest published 22.x. Node 22 "Jod" is the oldest line still supported (maintenance until 2027-04-30), and @types/node should describe the oldest runtime we support rather than the newest: typings ahead of the runtime re-create this same defect pointing the other way, blessing calls that are absent at execution time. 22 is also the oldest runtime CI actually executes. Neither workflow pins a Node version - both call actions/setup-node@v6 with no node-version - so every leg takes whatever its runner image ships, and those images have diverged: ubuntu-latest and windows-latest run v22.23.1 while macos-latest runs v24.18.0. Typing against the oldest of those is the safe direction. Note that 24 is not merely undesirable here, it does not currently build: with @types/node@24 installed, core/src/telemetry/tracing.ts:359 fails with TS2741 because a hand-rolled async iterator object literal does not implement [Symbol.asyncDispose]. That is tracked separately and is out of scope for a dependency bump. Staying inside 22.x also keeps undici-types at ~6.21.0, so the fetch/Headers/FormData/Response globals are untouched. @types/node is a devDependency in both manifests and is not part of either package's published dependency or type closure, so nothing changes for consumers. No source file changes and no emitted JavaScript changes. Verified: build, lint, format:check, docs:check and secretlint all exit 0, and `tsc --noEmit` produces a byte-identical diagnostic set before and after the bump (281 pre-existing errors, unchanged).
AmaadMartin
force-pushed
the
fix/types-node-22-lts
branch
from
August 1, 2026 10:09
c841d0d to
97b030c
Compare
This was referenced Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
Closes: #issue_number
Related: #issue_number
Problem: The root
package.jsonanddev/package.jsonboth pinned"@types/node": "^20.12.7", which resolved to20.19.43. Per the publicnodejs/Releaseschedule, the Node 20 "Iron" line ended on 2026-04-30 — it is already past end-of-life, not approaching it.So the compiler was checking all 531 TypeScript files in the repo against an API surface older than the runtime that executes them. Two silent defects follow, and the second is the dangerous one:
There is no runtime symptom today. The defect is that the type checker is checking against the wrong contract, and the gap widens every release.
Solution: Move both manifests to
^22.20.1(highest published 22.x) and regeneratepackage-lock.json. Three files, no source changes, no behaviour change.Why 22 and not 24 or 26.
@types/nodemust describe the oldest supported runtime, never the newest — typings ahead of the runtime re-create this exact defect pointing the other way. From the same public schedule: v20 ended2026-04-30, so v22 "Jod" is now the oldest line still supported (maintenance until2027-04-30). v24 is the newer active LTS; v26 is not an LTS line yet.22 is also the oldest Node that CI actually executes — I measured this rather than assuming it, and the runner images have quietly diverged. Neither workflow pins a Node version (
actions/setup-node@v6with nonode-versionand nonode-version-file), so every leg takes whatever its runner image ships. Read out of the CI logs for this very PR:run-tests (ubuntu-latest)v22.23.1run-tests (windows-latest)v22.23.1run-tests (macos-latest)v24.18.0Cross-Language Tests(macos-latest)v24.18.0So CI is simultaneously running two different Node majors depending on the OS image, and the oldest of them is
22.23.1. Typing against 22 covers every leg; typing against 24 would not. This drift is a direct consequence of not pinningnode-version— out of scope here, and already the subject of separate open PRs. Local verification ran on Nodev22.22.2.Choosing 22 over 24 is not just policy — it is empirically forced. I measured it (see the falsification section below):
@types/node@24.13.3breaksnpm run buildwith a real, pre-existing latent bug in this repo:That is a hand-rolled iterator object literal that does not implement the full async-generator protocol;
@types/node@24ships theAsyncDisposableglobals that make TypeScript demand it. Fixing that is out of scope for a dependency bump and would need a real code change plus a test, so it has been queued as its own task. Staying inside 22.x also keepsundici-typesat~6.21.0(unchanged), so thefetch/Headers/FormData/Responseglobals under the eightfetch()call sites are untouched.Scope notes.
@types/nodeis adevDependencyin both manifests, is absent fromcore's anddev'sdependencies/peerDependencies, and is not in either package'sfiles. Nothing changes for consumers of the published packages, and no emitted JavaScript changes because no source file changes.core/package.jsonandintegrations/package.jsondo not declare@types/node; they consume the hoisted root copy, so the ambient type surface moves for all four workspaces even though only two manifests change.The
chore:subject is deliberate and does not match thefix/...branch name. release-please keys off the commit subject, and afix:/feat:subject would open a release PR bumping@google/adk,@google/adk-devtoolsand@google/adk-integrationsin lockstep for a devDependency change that ships nothing.node_modules/mariadbdeclares its own@types/node: ^24.xand keeps a nested copy. That is expected and was left alone — nooverridesblock was added.Lockfile regeneration — note on how it was produced. The lockfile delta is exactly 3 hunks / 10 lines: the two
devDependenciesrange lines and thenode_modules/@types/nodeentry. Everything else is untouched (1174 entries before and after, none added or removed,lockfileVersion3 unchanged,undici-typesstill~6.21.0), and every addedresolvedURL points atregistry.npmjs.org.It was regenerated by
npm installfrom the base lockfile with a modern npm (11.18.0) — deliberately, and worth recording because an earlier attempt got this wrong. Regenerating with an older npm (9.2.x) drops the"license": "MIT"field from the rewritten entry: on a clean install that npm writes nolicensemetadata at all, and on an incremental install it preserves existing entries while writing the newly-resolved one without it. The result looks like a legitimate npm rewrite but silently deletes metadata that 1173 of the lockfile's entries carry. Regenerating under npm 11 — the npm that ships with the Node 24 on the macOS runner — reproduces the entry in full,licenseincluded.No
enginesfield, no.nvmrc, and no workflownode-versionpin were added — those are separate queued tasks, deliberately out of scope here.Collision check (required before implementation):
gh pr list --repo AmaadMartin/adk-js --state open --limit 100returned 100 open PRs; I diffed every plausibly adjacent one (#443,#428,#416,#406,#383,#382,#425,#360,#345). No open PR bumps@types/node. Findings:#382/#360/#425touch the same manifests/lockfile but on unrelated lines (@types/express, root test deps, removinggts) — textual overlap only, so I branched frommainrather than stacking arbitrarily on one of three.#383adds"@types/node": "^20.12.7"to two CommonJS integration fixture manifests (tests/integration/build_setup/ts_commonjs*/package.json) — different files, outside this task's three-file scope, but if both land those fixtures will still pin the EOL line and should be followed up. Re-checked before the final push: #445 ("declare a supported Node.js range on the published packages") has since opened and touchesdev/package.jsonandpackage-lock.jsontoo, but on theenginesfield rather than@types/node— again textual overlap only, no competing implementation. It is the complementary half of this work: it establishes the declared floor, while this PR moves the typings onto a supported line.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.
Unit Tests:
[ ] I have added or updated unit tests for my change. — deliberately not done; see below. Zero new/modified production lines, so there is nothing to unit test.
[x] All unit tests pass locally. — with one caveat:
2573 passed, 1 failed, and that single failure is pre-existing onmain, environmental, and identical before and after this change (detailed below).No new unit tests were added, deliberately. There are zero new or modified lines of production code, so new-line coverage is vacuous. A test asserting a version string in
package.jsonwould test npm, not ADK, and is review noise. The proof for a typings-only bump is that the entire existing diagnostic surface and test suite are unchanged — so the evidence below is a before/after equivalence proof rather than a new assertion.Baseline captured first, on unmodified
main, then re-measured after the bump:@types/node20.19.43 (before)@types/node22.20.1 (after)npm installnpm run buildnpx tsc --noEmit --pretty false(post-build)npm run lintnpm run format:checknpm run docs:checknpx secretlint "**/*"vitest run --project unit:core --project unit:devThe acceptance criterion is
diff /tmp/tsc-before.log /tmp/tsc-after.logbeing empty, and it is:About the 281 errors and the 1 failing test — both are pre-existing on
mainand unrelated to this change. I verified both against unmodifiedmainbefore touching anything:npm run ts:checkis already red onmain(it is not a CI step, which is how it stayed red). The 281 errors are the src-vs-dist duality: test files import both@google/adk(→core/dist/types/*.d.ts) and../../src/*.js, so TypeScript sees two nominally distinct copies of every class with aprivatemember. The criterion here is "the diagnostic set is unchanged", not "zero errors" — I did not try to fix these.dev/test/cli/cli_create_test.ts > createAgent > Interactive Mode > should handle Vertex AI selection with gcloud defaults. It fails identically before and after: it expects the stubbed project idgcloud-projectbut reads the developer machine's real gcloud default project. It is a local-environment leak, not a regression, and it does not reproduce on CI runners which have no gcloud config. I did not touch it.Proving the check can actually fail (falsification). A green equivalence check is worthless if the harness cannot go red, so I mutated the one input that matters — the
@types/nodemajor — and confirmed both gates fire:So the harness has real signal: a wrong bump is caught by both
npm run buildand the tsc equivalence diff, while22.20.1leaves both untouched. The tree was then restored and re-verified (buildexit 0,tsc281 errors, byte-identical to baseline,git statusclean).Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.
All five were run. Results:
22.20.1/6.21.0/^22.20.1twice; exactly three files; both greps empty; CLI--helpexits 0 and prints the full command list. The dev server started and served the sample agents normally — this exercisesexpress,node:http,node:fsand the agent loader, i.e. the parts of the Node surface the typings describe:Cross-platform coverage. Everything measured locally was on Linux, and Windows is the leg most likely to expose a genuine difference (path- and
fs-typing edges), so a green Linux run alone would not be sufficient. CI covers all three OS legs, and eachrun-testsleg runs the full gate:npm install,npx secretlint "**/*",npm run build,npm run test:coverage(statements 86 / branches 87 / functions 88 / lines 86),npm run lint,npm run format:check,npm run docs:check. Coverage did not move, as expected for a change with zero production-code lines.The Windows leg is flaky on this branch, disclosed rather than buried. Across four CI runs of what is runtime-identical code,
windows-latestwent pass → fail → pass → fail, whileubuntu-latest,macos-latestand the cross-language leg passed every time. Two distinct failures appeared, and neither is a type error:The first is a 5s budget on a child-process spawn — the classic assertion a loaded Windows runner misses. The second is the OS refusing to bind an ephemeral port: Windows reserves blocks of ephemeral ports, and a pre-selected port landing in a reserved range fails
EACCES(notEADDRINUSE).Neither can be caused by this change, on two independent grounds:
git rev-parse <sha>^{tree}=54dd3c84…for both commits), so nothing about the code differed.@types/nodeis erased at compile time and vitest transpiles with esbuild without type checking, so the executed JavaScript is bit-identical. There is no mechanism by which it could affect a child-process timeout or a TCP bind.I did not touch, skip, re-time, or
.skipeither test to get green — both are outside this PR's scope, and adjacent open PRs (#355, #373) already work in the first file. Both have been queued as their own tasks (the port one should bind port0and read the assigned port back, rather than pre-selecting).I did not touch, skip, re-time, or
.skipthat test to get green — it is outside this PR's scope, and adjacent open PRs (#355, #373) are already working in that file. It has been queued as its own task instead.Checklist
[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[ ] I have commented my code, particularly in hard-to-understand areas. — n/a, no code changed; the rationale lives in this description and the commit message.
[ ] I have added tests that prove my fix is effective or that my feature works. — no tests added (see above). The equivalent evidence is the byte-identical
tscbefore/after proof, plus the falsification run showing that proof can go red.[x] New and existing unit tests pass locally with my changes, with the one pre-existing environmental failure noted above.