chore(ci): standardize the repository on Node 22 (PNI-186) - #2325
Open
mme wants to merge 4 commits into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+12
to
+27
| on: | ||
| push: | ||
| branches: [main] | ||
| paths: | ||
| - ".node-version" | ||
| - ".github/workflows/**" | ||
| # The whole scripts directory, fixtures included: a change that weakens a | ||
| # fixture has to re-run the suite that fixture belongs to. | ||
| - ".github/scripts/**" | ||
| pull_request: | ||
| paths: | ||
| - ".node-version" | ||
| - ".github/workflows/**" | ||
| # The whole scripts directory, fixtures included: a change that weakens a | ||
| # fixture has to re-run the suite that fixture belongs to. | ||
| - ".github/scripts/**" |
| contents: read | ||
|
|
||
| jobs: | ||
| toolchain-consistency: |
Contributor
Python Preview PackagesVersion
Install with uvAdd the TestPyPI index to your [[tool.uv.index]]
name = "testpypi"
url = "https://test.pypi.org/simple/"
explicit = trueThen install the packages you need: # Core SDK
uv add 'ag-ui-protocol==0.0.0.dev1785947872' --index testpypi
# Integrations (each already depends on the matching ag-ui-protocol preview)
uv add 'ag-ui-langgraph==0.0.0.dev1785947872' --index testpypi
uv add 'ag-ui-crewai==0.0.0.dev1785947872' --index testpypi
# NOTE: ag-ui-agent-spec depends on pyagentspec (git-only, not on PyPI).
# You will need to install pyagentspec separately from its git repo.
uv add 'ag-ui-agent-spec==0.0.0.dev1785947872' --index testpypi
uv add 'ag_ui_adk==0.0.0.dev1785947872' --index testpypi
uv add 'ag_ui_strands==0.0.0.dev1785947872' --index testpypiInstall with pippip install \
--index-url https://test.pypi.org/simple/ \
--extra-index-url https://pypi.org/simple/ \
ag-ui-protocol==0.0.0.dev1785947872
Commit: 3e10374 |
@ag-ui/a2a-middleware
@ag-ui/a2ui-middleware
@ag-ui/event-throttle-middleware
@ag-ui/mcp-apps-middleware
@ag-ui/mcp-middleware
@ag-ui/a2a
@ag-ui/adk
@ag-ui/ag2
@ag-ui/agno
@ag-ui/aws-strands
@ag-ui/claude-agent-sdk
@ag-ui/claude-managed-agents
@ag-ui/crewai
@ag-ui/langchain
@ag-ui/langgraph
@ag-ui/llamaindex
@ag-ui/mastra
@ag-ui/pydantic-ai
@ag-ui/vercel-ai-sdk
@ag-ui/watsonx
@ag-ui/a2ui-toolkit
create-ag-ui-app
@ag-ui/client
@ag-ui/core
@ag-ui/encoder
@ag-ui/proto
commit: |
Node versions were declared in several places and didn't agree. Ten
`actions/setup-node` invocations had "22" typed into them and two read
`package.json`, whose `engines.node` range (">=18") resolves to whatever
the newest satisfying Node happens to be — so those two jobs silently ran
a different major from the other ten. The ten agreed with each other by
luck, and no file said which Node this repository is developed and tested
with.
Add `.node-version` at the root containing 22, and point all 12
invocations across 8 workflows at it via `node-version-file`. None names
a version inline anymore.
Every one of the 12 already checks the repository out earlier in the same
job, and no checkout uses `sparse-checkout`, a `path:` subdirectory or a
container, so the file is at the workspace root when `setup-node` runs —
which is where it resolves a relative `node-version-file` from,
regardless of any step's `working-directory`. A bare major is a valid
`.node-version`: setup-node reads it as a semver range and resolves the
latest 22.x.
One consequence worth knowing: a prerelease dispatch of
publish-release.yml against a ref that predates this commit will fail at
`setup-node`, because the file it now reads is not in that tree.
publish-commit.yml already had that property via `package.json`.
`package.json#engines.node` is untouched. That manifest is private and
never published, so its range is a floor on installs in this repository
rather than anything consumers see.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both jobs ran `corepack enable` alongside `pnpm/action-setup`, which already resolves `packageManager` from package.json. Two mechanisms were installing the same pnpm 10.33.4, and which one won depended on step order. It used to run before `setup-node`, so its shims landed in the runner image's Node bin directory and were shadowed the moment `setup-node` prepended the toolcache Node — making the corepack step dead code and leaving `pnpm/action-setup` in charge. Moving it after `setup-node` would have inverted that: the shims would land in the Node 22 bin directory and win, making `pnpm/action-setup` inert instead, and adding a registry fetch to a job whose pnpm binary was already on disk. Neither arrangement is worth keeping, so remove the step. `pnpm/action-setup` self-installs pnpm and puts it on PATH, and resolves its version from `packageManager` when given no `version:` input, which is the case here. Node 22's bin directory ships `corepack` and `npm` but no `pnpm` shim unless `corepack enable` creates one, so `pnpm` in the later steps resolves to the action's binary. The repository's other ten setup-node jobs already work exactly this way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pointing the 12 call sites at one file fixes today. Without a guard, the next site someone adds re-creates the problem, because copying an existing step and typing a version into it is the natural way to add one. No existing tool has an opinion here — actionlint and zizmor do not care which Node you select. Add `.github/scripts/check-toolchain-consistency.sh`, which fails on a `setup-node` that names a version inline, reads the wrong version file, or has no version input, and on a `.node-version` that is missing, empty or not 22. Run it in CI from a new Toolchain Consistency workflow, fixtures first — a check that no longer discriminates is worse than no check, because it reports green either way. Version inputs are attributed to the `setup-node` step they are written in, by indentation. Counting matching lines file-wide was the obvious shortcut and it fails open: any stray `node-version-file:` line — in a `run:` heredoc, in another action's `with:` — pays for a step that has no version input, and the check goes green on real drift. An inline `node-version` is reported even when a correct `node-version-file` sits beside it, because setup-node gives the inline value precedence, so that is the version the step actually runs. Reading nothing and finding nothing are kept distinct, which is where a guard like this usually rots. Each of these is reported rather than passed over: an unreadable file; an absent `.github/workflows`; a directory whose contents cannot be listed; a directory holding no workflows; an entry that is not a regular file, including a dangling symlink; a file with no YAML keys the scan can read, such as one in the wrong encoding; and a scan that did not run to completion, detected by the absence of its own end-of-file record. The success message states how many invocations across how many files it examined, so a green result cannot mislead by omission. Cover it with 24 cases over 22 fixture roots — miniature repositories the check is pointed at via its optional root argument. Each failing case asserts the message AND the exact problem count; each passing case asserts the coverage numbers, not just the success sentence. Without the first, a check that fails for the wrong reason passes; without the second, one that also reports spurious problems passes; without the third, one that quietly stops seeing steps or whole files keeps printing success with smaller numbers. Both helpers require their fixture to exist, since `missing-version-file` is defined by an absent file. Fixtures were chosen against mutants that survived earlier drafts: drift in the second workflow file (a scan that stops after the first), drift in the last step (only an end-of-file flush evaluates it), drift written in the repository's own SHA-pinned `# v7.0.0` notation (a scan that skips any line with a comment), a correct step followed by a bare one (per-step state that is not reset), a lone-dash sequence entry, a quoted `"node-version"` key, the action's name appearing as data inside a `run:` block, and an action whose name merely starts the same. Each of those made a real green-while-broken hole before it had a fixture. It is a line scan, not a YAML parse, and the header says so along with what that costs. Verified: 24/24 on this tree; 13 findings against the parent tree (the 12 drifted call sites plus the absent root file); and every rule's report, when removed, makes the suite go red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add a Toolchain section to CONTRIBUTING.md covering which version managers read `.node-version` and how: fnm resolves a bare major, nvm reads only `.nvmrc` so the version has to be passed through, and nodenv and asdf resolve the file to an exactly-installed version — asdf only with `legacy_version_file = yes`. Point the two repo-development docs that said "Node 20+" at the file rather than at a number. Naming the version in prose would have added two more places to update on every bump, and nothing checks prose. State the check's scope honestly rather than letting its name imply more than it does: Node in `.github/workflows/` only. pnpm is not guarded, and 9 of 11 `pnpm/action-setup` steps pin `version:` inline while 2 rely on `package.json#packageManager` — the same drift shape, still unguarded, recorded here rather than fixed under a Node ticket. Document the bump, which touches more than the obvious file: the constant in the check moves with `.node-version`, and so do the fixture pins and the expected-message string in the test suite, while `wrong-pinned-version` has to stay on a different major. Then publish-release.yml's npm-OIDC workaround needs review, because it exists only to work around the npm that Node 22 bundles. Correct what `engines.node` is while describing it. The root manifest is `private: true` and never published, so its `>=18` is a floor on installs in this repository — and it is what `node-version-file: "package.json"` resolved before this change, making it a toolchain input rather than a consumer promise. Of the published packages only `@ag-ui/watsonx` and `@ag-ui/claude-managed-agents` declare a range at all, both `>=20.3.0` and set independently; the six core packages declare none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mme
force-pushed
the
markus/pni-186-standardize-the-ag-ui-repository-on-node-22
branch
from
August 5, 2026 16:37
f6823df to
7f086b8
Compare
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
2 participants
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.
Fixes PNI-186.
The problem
Node versions were declared in several places and didn't agree. Ten
actions/setup-nodeinvocations named"22"inline; two readpackage.json, whoseengines.noderange (>=18) resolves to whatever the newest satisfying Node happens to be — so those two jobs were silently running Node 24/25. Nothing said which Node this repository is developed and tested with, and nothing was watching for drift.What this does
.node-versionat the root containing22. One declaration, read by CI and by local version managers alike.actions/setup-nodeinvocations across 8 workflows at it vianode-version-file. None names a version inline anymore. Each of the 12 already checks the repository out first, so the file resolves.corepack enableto aftersetup-nodein bothpublish-commit.ymljobs. It ran before, so that one command used whatever Node the runner image shipped..github/scripts/check-toolchain-consistency.sh, which fails on asetup-nodethat names a version inline, reads the wrong version file, or names no version at all — and on a.node-versionthat isn't 22.Toolchain Consistencyworkflow, fixtures first — a check that no longer discriminates is worse than no check..node-version, and documents the toolchain inCONTRIBUTING.md.package.json#engines.nodestays at>=18. That range states what consumers of the published packages may use; narrowing it is a separate compatibility decision and explicitly out of scope here.On the check's design
It is a line scan, not a YAML parse, and its header says so. It catches a version written into a workflow in every style anyone in this repository writes. Unusual-but-legal YAML — anchors, folded scalars, quoted keys — can slip past it. That's an accepted limit rather than an oversight: the drift being guarded against is someone copying a step and typing a version into it, not deliberate evasion.
An earlier draft of this PR also flagged jobs that run Node tooling with no
setup-nodeat all. That rule carried nearly all of the implementation's complexity, found zero findings against the 24 real workflows, and was the only source of false positives — so it was cut rather than hardened, taking the script from 219 lines to 114.Verification
Under Node 22.20.0 with pnpm 10.33.4, via the repository's Nx targets:
pnpm run buildpnpm run check-typespnpm run testpnpm run lintpnpm run test:exportsverify-nx-release-allowlist/verify-release-scope-dropdowns/verify-config-manifest-namesRun against the parent tree, the check reports 13 findings — the 12 drifted call sites plus the absent root file — and passes on this branch.
Pre-existing failures, red both before and after
None relate to Node resolution. Recorded here for separate repair, per the ticket:
@ag-ui/core:lint— itslintscript callseslint, which neithercorenor the root declares, so it isn't on PATH for that project. Reproduced on a pristine tree.@ag-ui/cross-language-tests:test— needs a local .NET SDK. CI runs this suite (dotnet / cross-language (TS <-> C#)) and it passes.@ag-ui/mastra:test:exports—typesVersionsmapscopilotkitbut nota2ui, so node10 resolution of@ag-ui/mastra/a2uifails.Worth flagging separately: neither
lintnortest:exportsruns in CI at all. That's why the PNI-188 baseline recorded CI as fully green without seeing #1 or #3. Probably its own ticket.🤖 Generated with Claude Code