docs(ci): spec for GitHub Actions Node 24 runtime upgrade (#36850) - #37193
docs(ci): spec for GitHub Actions Node 24 runtime upgrade (#36850)#37193nicobytes wants to merge 6 commits into
Conversation
Issue-resolution spec (Spec-Kit PR 1) for upgrading the 124 in-scope GitHub Actions references under .github/ to node24 majors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…de-24-runtime-majors
There was a problem hiding this comment.
Pull request overview
This PR is Spec-Kit PR 1/2 for issue #36850, documenting the problem, scope, risks, and acceptance criteria for upgrading GitHub Actions used under .github/ to Node 24–compatible majors to eliminate Node 20 runtime deprecation annotations and prevent future CI breakage.
Changes:
- Adds a complete
spec.mddescribing the CI drift problem, in-scope actions, and upgrade targets. - Defines acceptance criteria and a proposed guard approach (script + lint job) to prevent future action-version drift.
- Documents key risk areas (notably
download-artifactv8 behavior) and explicitly deferred follow-ups.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Claude finished @nicobytes's task in 1m 2s —— View job Review — spec-only PR
This PR adds a single specification document ( Consistency checks (all pass)
Resolved (prior review threads)
Observations (non-blocking, author's call)
No issues found in the reviewable content — the spec is internally consistent and the three prior findings are genuinely resolved (not just acknowledged). This is a scoping/measurability review, which is exactly what a Spec-Kit PR 1 should get; the risk analysis (download-artifact |
- github-script v9: correct the deferral rationale. v9 does NOT break
require() — src/main.ts still injects wrapRequire (a Proxy over
__non_webpack_require__ handling bare and ./-relative IDs), verified
against tag v9.0.0. The real exposure is Octokit v5->v7 under 14
inline scripts; require('@actions/github') and const getOctokit have
0 occurrences here.
- AC-005: make the criterion auditable against both of setup-node's
caching paths — no cache:/cache-dependency-path at any call site
(explicit), and no root package.json (automatic, added in v5).
- AC-007: drop the hard-coded 124/51 baseline from the criterion; the
count drifts with main, so the AC is now non-zero -> zero.
- Define the headline count (executable uses: lines in yml/yaml) and
correct the README item: 8 stale refs across 7 files, including three
prose mentions, not 5 examples.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merging main brought in #37150, which deleted core-web/libs/dotcms and stripped `npm run build:docs:dotcms` from publish_docs.yml itself. Only build:docs:dotcms-models survives, so the cleanup follow-up should lean towards deletion rather than revival. Re-verified post-merge: setup-node@v2-beta + node-version 16.13.2 are still present there, so the in-scope pin bump is unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Thanks — all three review points are addressed in 1d216ff and c3a5c23. On the Medium, you caught a real error in my rationale, but the proposed replacement isn't right either, so I went to the source.
|
AC-001 requires residual Node deprecation annotations to be attributable to a *named* follow-up issue; the spec described one without naming it. Points the four deferral sites at #37194. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
❌ Issue Linking RequiredThis PR could not be linked to an issue. All PRs must be linked to an issue for tracking purposes. How to fix this:Option 1: Add keyword to PR body (Recommended - auto-removes this comment)
Option 2: Link via GitHub UI (Note: won't clear the failed check)
Why is this required?Issue linking ensures proper tracking, documentation, and helps maintain project history. It connects your code changes to the problem they solve. |
…de-24-runtime-majors
What this is
Spec-Kit PR 1 of 2 — this PR contains
spec.mdand nothing else.Per SPEC_KIT_QUICK_START.md §3, the gate is approval, not merge: once a reviewer approves this spec, planning starts. Don't wait on the merge queue. PR 2 will carry the implementation and link back here.
Issue linking — intentionally a plain reference. Parent issue: #36850. This PR carries no
auto-linking keyword, by design. The parent is answered by PR 2 — the actual sweep of 124 action
references — not by this spec, so a keyword here would retire the parent the moment PR 1 merges with
every stale pin still in place. The Issue Linking check therefore stays red on this PR: expected,
not an oversight.
Follow-up scope is tracked in #37194.
Specification phase for #36850 (see the note above on why no closing keyword is used). Sequencing gate is clear — #36838 landed on 2026-08-04.
What to review
Not the code (there is none). Review whether this is the right problem, scoped right, with measurable criteria.
The problem
GitHub retired the Node 20 action runtime. 124 references to 8 actions across 51 files under
.github/are still onnode20-era majors (some as old ascheckout@v2,setup-node@v2-beta). Today that means deprecation annotations on every run; when the runner drops the compatibility shim, it means the build, test and release pipeline stops working.ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSIONis explicitly excluded.Three decisions in the spec worth your attention
1. AC-005 deviates from an AC written in the issue. The issue asks for
package-manager-cache: falsewhere pnpm caching is manual. That precondition doesn't exist here:setup-nodereads only$GITHUB_WORKSPACE/package.json, and this repo has no rootpackage.json(only a straypackage-lock.json).core-web/package.json'spackageManager: pnpm@10.17.1is invisible to it, and v6+ limits auto-caching to npm anyway. Auto-caching cannot fire at any of the 8 sites. The spec documents the deviation rather than adding 8 lines of verifiably dead config that would get cargo-culted. This is the deviation that needs your sign-off.2. Scope grew by 4 actions beyond the issue's list. The runner emits the annotation per job, listing that job's stale actions. Bumping only the issue's 6 clears Initial Artifact Build — satisfying the issue's literal AC — but leaves annotations on
initialize,label-prandtest. Addinggithub-script→v8,dorny/paths-filter→v4.0.3,dawidd6/action-download-artifact→v24 anddocker/login-action→v4.6.0 closes the PR pipeline completely.github-scriptdeliberately stops at v8, a pure node24 bump with no behavior change. (An earlier draft justified this by claiming v9 breaksrequire()— that was wrong, and is corrected in the spec: v9 still injectsrequireviawrapRequire. v9's real cost is Octokit v5 → v7 under 14 inline scripts, which is a different review from a runtime bump.)3. The guard script + lint job ship in PR 2 as a droppable batch-0 commit.
.github/filters.yaml'sbackendfilter only matchescicd_comp_*.yml,cicd_1-pr.ymlandcore-cicd/**/action.yml— so roughly 38 of the 51 files PR 2 touches get no build, no test and no lint today. That gap is why this issue exists. Constitution Principle V also needs a committed, confirmed-failing test, and there is no unit/integration/Postman layer for workflow YAML. If you disagree, it's one self-contained commit to drop.Risk assessment already done
Two items looked dangerous and were run to ground against upstream source, so PR 2 doesn't have to re-litigate them:
checkout@v7's fork-PR block cannot fire in this repo. Readingsrc/unsafe-pr-checkout-helper.tsandsrc/input-helper.tsat tagv7.0.1: the guard isn't even reached for a default self-checkout.cicd_post-workflow-reporting.yml:58is a bare- uses: actions/checkout@v4with nowith:block;cicd_publish-pr-test-image.ymlhas no checkout step at all and is gated to non-forks. Conclusion: no v6 pins, and noallow-unsafe-pr-checkout: true— adding it would permanently disarm a real protection against a risk we don't have.download-artifact@v8'sdigest-mismatch: errordefault is the one genuine green-to-red risk, concentrated on the multi-GBmaven-repoartifact and onfinalize'sbuild-reports-*pattern (N artifacts = N chances to fail, on every PR and merge-queue run). PR 2 lands v8 with explicitdigest-mismatch: warnfirst — making the bump provably behavior-neutral — then flips toerrorin a separate revertable commit.No product surface is touched: no Java, no Angular, no DB, no ES mapping, no API contract. Nothing falls under ROLLBACK_UNSAFE_CATEGORIES.md.
Deliberately deferred to follow-ups
runs.using: 'node16'(issue-fetcher,issue-labeler,changelog-report,rc-changelog) — these needdist/rebuilds, and a regeneratednccbundle can't be diff-reviewed the way YAML can.node16is more deprecated thannode20, so this should be filed immediately.aws-actions/configure-aws-credentialsv1→v6 needs OIDC and org-level trust-policy work, andslackapi/slack-github-actionv1→v2+ changes the payload format..github/dependabot.ymlfor thegithub-actionsecosystem, and extending.github/filters.yamlso.github/**gets real validation. These two are the structural fix for this class of drift — they're what makes the next runtime bump routine.🤖 Generated with Claude Code