[DEVEX-907] Make setup-node conditional in action-pre-commit to preserve baked pre-commit cache#198
Draft
HammerZhaoTuro wants to merge 2 commits into
Conversation
Skip `actions/setup-node` when `node` is already on PATH, matching the existing pattern for Python. This prevents setup-node from installing a different Node version that changes pre-commit's cache key, causing baked hook environments (e.g. mirrors-prettier) to miss and reinstall from scratch on every run. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Release notes previewBelow is a preview of the release notes if your PR gets merged. 3.6.2 (2026-07-15)Miscellaneous
Bug Fixes
Tests
|
Avoids double action download during testing. Will be updated to @v4 when the release is cut. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Summary
DEVEX-907: Make setup-node conditional in action-pre-commit to preserve baked pre-commit cache
Description
Problem
open-turo/action-pre-commit@v3unconditionally runsactions/setup-node@v6(defaulting to Node 24), even though the runner image (turo/actions-runner) already ships Node 22. This causes pre-commit'snodelanguage cache to miss for hooks likemirrors-prettier— despite the environments being successfully pre-built into the image (confirmed: 78/79 environments built, onlycspell-clifailed).Pre-commit resolves the cache directory name using
get_default_version(), which returns"system"or"default"depending on how Node is found on PATH. Whensetup-nodeinstalls Node 24 into/opt/hostedtoolcache(an emptyDir volume on k8s runners), it changes what pre-commit finds on PATH, flipping the resolved cache key and creating a directory name mismatch with the bakednode_env-systemenvironment.Observed:
mirrors-prettierreinstalls from scratch every run (~40-80s wasted), despite being cached in the image.Impact: ~449 repos use
mirrors-prettier@v3.1.0. Other affected node hooks include older commitlint versions withoutlanguage_version: systemandmirrors-eslint.Scope: lint job only
setup-noderuns insideaction-pre-commit, which runs only in the lint job of thek8s-toolingreusable workflow. Each job runs on a separate k8s pod — other jobs (test, kustomize-diff, kyverno-test) and separate workflows (release) are completely unaffected.Within the lint job, the only consumers of Node are the pre-commit hooks themselves (prettier, commitlint, eslint). Node version requirements for all hooks are satisfied by Node 22:
prettier@3.1.0— requires Node >= 14@commitlint/cli@^21— requires Node >= 22.12.0eslint@9.39.x— requires Node >= 18.18.0Hook output is deterministic by tool version, not Node runtime. Node 22 vs 24 produces identical formatting/linting results.
Change
Make
setup-nodeconditional — skip it whennodeis already on PATH, matching the existing pattern for Python. AddedNODE_BINdetection to the "Version files" step and gated both "Determine node version" and "Setup node" onenv.NODE_BIN == ''.When the runner image already has Node (e.g.
turo/actions-runnerwith Node 22), both steps are skipped and pre-commit sees the same system Node used during image build — cache keys match. When Node is absent,setup-noderuns as before (fallback).Why this is safe
@commitlint/cli@^21needing Node >= 22.12.0, which the baked runner image (Node 22.23.x) satisfies.setup-nodesaves ~50MB download per run./opt/hostedtoolcacheis an emptyDir volume destroyed when the pod exits, so everysetup-nodecall downloads Node fresh.Independent safety assessment
Full assessment (artifact) — covers setup-node side effects, blast radius, edge cases (GitHub-hosted runners,
.node-versionfiles, npm install in commitlint sub-action, external consumers), and expression semantics.Validation plan
dunlop-deployments(same approach as DEVEX-868)mirrors-prettiercache hits (no[INFO] Installing environmentin logs)References
Fixes DEVEX-907
Changes
🚀 PR created with fotingo