ci: unify Node.js version to 22

[lml2468]

Summary

Unify Node.js version to 22 (current LTS) by replacing the [18, 20] matrix with a single 22 target.

Closes #76

Why

• Node 18 reached EOL in April 2025
• Node 20 enters maintenance mode in October 2026
• octo-adapters already uses Node 22
• The matrix was testing against two outdated versions; a single current-LTS target reduces CI time and aligns with org standard

Changes

• .github/workflows/ci.yml: Remove strategy.matrix.node-version: [18, 20], use node-version: 22 directly

Validation

• YAML syntax verified
• actionlint passed

[Jerry-Xin]

This PR is relevant to octo-admin, but it does not actually unify the repository’s Node.js version.

🔴 Blocking

• 🔴 Critical: .github/workflows/ci.yml:83 now validates the app on Node.js 22, but Dockerfile still builds production assets with node:18-alpine. That leaves the deployable/container build on the EOL runtime the PR says it is removing, and it creates a CI/runtime mismatch where Node 22-compatible dependency changes can pass CI but fail Docker image builds. Update the Docker build stage to Node 22 as part of this PR, or add CI coverage that explicitly builds the Docker image if the Dockerfile must remain on Node 18 for a documented reason.

💬 Non-blocking

• 🔵 Suggestion: Consider adding a single source of truth such as .node-version, .nvmrc, or package.json engines.node once the Dockerfile and CI are aligned, so future Node version updates do not drift across files.

✅ Highlights

• The workflow simplification itself is clear and the actions/setup-node usage at .github/workflows/ci.yml:81-84 is valid YAML/action configuration.

[yujiawei]

Code Review — PR #73 (octo-admin)

Verdict: APPROVED ✅

A clean, single-line CI maintenance change. The build job in .github/workflows/ci.yml drops the strategy.matrix.node-version: [18, 20] block and pins actions/setup-node to node-version: '22' directly.

Verification

• Rationale is correct. Node 18 reached EOL (April 2025) and Node 20 is heading into maintenance; Node 22 is the current active LTS. Consolidating onto a single current-LTS target is the right call and matches the stated org standard (octo-adapters already on 22).
• No stale matrix references. The build job had only one setup-node step using ${{ matrix.node-version }}, and the matrix is no longer referenced anywhere else in the job after removal. No dangling ${{ matrix.* }} expressions remain.
• No cross-file inconsistency. Scanned the other 17 workflow files (docker-publish.yml, release-publish.yml, codeql.yml, etc.) — none pin a Node version, so there is no second source of truth left out of sync. There is no .nvmrc and no engines field in package.json, so nothing else needs to be updated to stay consistent.
• Surrounding job logic untouched. The changes preflight gating, draft-PR conditions, and cache: npm are all preserved.
• YAML/quoting. Quoting '22' is good practice (avoids any ambiguity in how the value is parsed). PR notes actionlint passed.

Notes (non-blocking)

• Reduced version coverage is an accepted tradeoff. Dropping the matrix means CI no longer catches code that works on 22 but breaks on an older runtime. Since the project is standardizing on 22 as the deployment target, this is fine — flagging only so it's a conscious choice.
• Floating minor/patch. node-version: '22' resolves to the latest 22.x available on the runner, so the exact minor can drift over time. Acceptable for CI; pinning a more specific version is unnecessary here.

No P0/P1 issues. Approving.

[yujiawei]

Code Review — PR #73 (octo-admin)

Verdict: CHANGES_REQUESTED 🔴

This supersedes my earlier approval on this PR — I missed a cross-file dependency on first pass.

The CI change itself is clean, but the PR does not deliver on its stated goal of unifying the Node.js version to 22. The production build is left on EOL Node 18.

🔴 Blocking (P1)

Dockerfile:1 still builds on node:18-alpine.

FROM node:18-alpine AS build
...
COPY --from=build /app/dist /usr/share/nginx/html/admin

.github/workflows/ci.yml now validates the app on Node 22, but the deployable container build stage still uses node:18-alpine. Consequences:

1. Goal not met — the PR title/description say "unify Node.js version to 22", but the actual production/deploy runtime stays on the EOL version the PR claims to be removing. CI is unified; the real build artifact is not.
2. CI/runtime mismatch — a Node 22-compatible dependency or syntax change can pass npx tsc --noEmit / npm test / npm run build in CI yet fail (or silently behave differently) in the Docker build stage on Node 18. CI green no longer implies a buildable image.

Required: bump the Dockerfile build stage to node:22-alpine as part of this PR so CI and the deployable image agree. If the Dockerfile must stay on Node 18 for a documented reason, then add CI coverage that explicitly builds the Docker image, and update the PR description so it no longer claims a full unification.

💬 Non-blocking suggestion

• Once the Dockerfile and CI are aligned on 22, add a single source of truth — .nvmrc / .node-version or package.json engines.node — so future Node bumps don't drift across ci.yml and Dockerfile again. (Currently there is no .nvmrc and no engines field, which is what allowed this drift.)

✅ Highlights

• The workflow simplification is correct: removing the [18, 20] matrix in favor of a single current-LTS target is the right direction, and the actions/setup-node config at ci.yml:81-84 is valid (quoting '22' is good practice).
• No other workflow files pin a Node version, so ci.yml is the only CI source of truth — the only remaining inconsistency is the Dockerfile.

Approving the CI direction, but blocking on the Dockerfile so the merge actually achieves what the PR says.

[yujiawei]

Code Review — PR #73 (octo-admin)

Verdict: APPROVED ✅

This PR cleanly unifies the project's Node.js version to 22 (current active LTS), and the current revision now delivers on its stated goal across all three runtime sources.

Verification (at head d78b42d0)

• ✅ CI runner — .github/workflows/ci.yml: the strategy.matrix.node-version: [18, 20] block is removed and actions/setup-node is pinned to node-version: '22'. The build job had a single setup-node step, and no dangling ${{ matrix.* }} references remain. The surrounding changes preflight gating, draft-PR conditions, and cache: npm are all preserved.
• ✅ Production build image — Dockerfile:1: the build stage is now FROM node:22-alpine (previously node:18-alpine). This is the important fix: CI and the deployable container build now agree, eliminating the CI/runtime mismatch where a Node 22-compatible change could pass CI but fail (or behave differently) in the Docker build on the EOL runtime. The second stage (nginx:alpine) is unaffected.
• ✅ Single source of truth — .nvmrc is added with 22, giving local dev environments a consistent target and reducing the risk of future drift between ci.yml and Dockerfile.
• ✅ No remaining Node 18/20 references — scanned the other 17 workflow files and both READMEs; none pin a Node version, and package.json has no engines field, so there is no second source left out of sync.
• ✅ Rationale is sound — Node 18 reached EOL (April 2025), Node 20 is entering maintenance, and Node 22 is the current active LTS, matching the stated org standard.

Notes (non-blocking)

• Reduced version coverage — dropping the [18, 20] matrix means CI no longer cross-tests against older runtimes. Since the project standardizes on 22 as both CI and deploy target, this is the correct tradeoff; flagging only so it's a conscious choice.
• Floating minor/patch — node:22-alpine and node-version: '22' resolve to the latest 22.x available, so the exact minor can drift over time. Acceptable for both CI and image builds; the .nvmrc keeps them aligned on the major.

No P0/P1 issues. The change is correct, complete, and internally consistent. Approving.

[Jerry-Xin]

One-line summary: The PR is in scope and the Node 22 unification is consistent across CI, local nvm usage, and the Docker build stage, with no blocking issues found.

💬 Non-blocking

• 🟡 Warning: The PR rationale says Node 22 is the “current LTS,” but as of June 6, 2026 the official Node.js release schedule lists 22.x as Maintenance LTS, 24.x as Active LTS, and 26.x as Current. The code can still target Node 22 because it remains supported until April 30, 2027, but the PR description should be corrected if accuracy matters. README/PR description, .github/workflows/ci.yml:83, Dockerfile:1. (github.com)
• 🔵 Suggestion: Consider adding "engines": { "node": ">=22 <23" } or similar to package.json if the project wants npm and deployment tooling to enforce the same runtime expectation as .nvmrc and CI. package.json:1.

✅ Highlights

• CI now uses a single Node target cleanly via actions/setup-node@v4. .github/workflows/ci.yml:81
• Local developer runtime guidance was added with .nvmrc. .nvmrc:1
• Docker build runtime matches CI. Dockerfile:1

Validation performed locally under Node v22.22.1 / npm 10.9.4: npm ci, npm test, and npm run build all passed.

[Jerry-Xin]

One-line summary: The PR is in scope and the Node 22 unification is consistent across CI, local nvm usage, and the Docker build stage, with no blocking issues found.

💬 Non-blocking

• 🟡 Warning: The PR rationale says Node 22 is the “current LTS,” but as of June 6, 2026 the official Node.js release schedule lists 22.x as Maintenance LTS, 24.x as Active LTS, and 26.x as Current. The code can still target Node 22 because it remains supported until April 30, 2027, but the PR description should be corrected if accuracy matters. README/PR description, .github/workflows/ci.yml:83, Dockerfile:1. (github.com)
• 🔵 Suggestion: Consider adding "engines": { "node": ">=22 <23" } or similar to package.json if the project wants npm and deployment tooling to enforce the same runtime expectation as .nvmrc and CI. package.json:1.

✅ Highlights

• CI now uses a single Node target cleanly via actions/setup-node@v4. .github/workflows/ci.yml:81
• Local developer runtime guidance was added with .nvmrc. .nvmrc:1
• Docker build runtime matches CI. Dockerfile:1

Validation performed locally under Node v22.22.1 / npm 10.9.4: npm ci, npm test, and npm run build all passed.

[Jerry-Xin]

One-line summary: The PR is in scope and the Node 22 unification is consistent across CI, local nvm usage, and the Docker build stage, with no blocking issues found.

💬 Non-blocking

• 🟡 Warning: The PR rationale says Node 22 is the “current LTS,” but as of June 6, 2026 the official Node.js release schedule lists 22.x as Maintenance LTS, 24.x as Active LTS, and 26.x as Current. The code can still target Node 22 because it remains supported until April 30, 2027, but the PR description should be corrected if accuracy matters. README/PR description, .github/workflows/ci.yml:83, Dockerfile:1. (github.com)
• 🔵 Suggestion: Consider adding "engines": { "node": ">=22 <23" } or similar to package.json if the project wants npm and deployment tooling to enforce the same runtime expectation as .nvmrc and CI. package.json:1.

✅ Highlights

• CI now uses a single Node target cleanly via actions/setup-node@v4. .github/workflows/ci.yml:81
• Local developer runtime guidance was added with .nvmrc. .nvmrc:1
• Docker build runtime matches CI. Dockerfile:1

Validation performed locally under Node v22.22.1 / npm 10.9.4: npm ci, npm test, and npm run build all passed.