Repository navigation
Migrate review action dependencies to Node 24 (WEB-5090) - #44
miguelcalderon merged 5 commits into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 PR Summary:
This PR migrates every GitHub Action dependency of the composite action and this repository's own CI workflows from Node 20 action releases to Node 24 releases (checkout v5, cache/cache-save v5, setup-python v6, setup-node v5, upload-artifact v6), keeping all pins as immutable commit SHAs. Application Node.js versions (18 in the composite, 20 in test CI) and the public action contract are deliberately unchanged. To guard the migration, it adds an offline runtime checker (scripts/action_runtime_dependencies.py plus a CLI wrapper) that walks local composite actions and validates each external uses reference against a recorded JSON metadata fixture, with focused pytest coverage. PyYAML is introduced as a test-only dependency via a new requirements-dev.txt, installed in test CI and documented in the README.
11 files reviewed
| File | Changes |
|---|---|
action.yml |
Repin cache, checkout, setup-python, setup-node, upload-artifact to Node 24 |
.github/workflows/*.yml |
Repin CI actions to Node 24; install dev requirements |
scripts/action_runtime_dependencies.py |
New offline action-runtime checker library and CLI entry point |
scripts/fixtures/action-runtime-metadata.json |
Recorded runtime metadata keyed by exact action SHA |
claudecode/test_action_runtime_dependencies.py |
Tests for checker traversal, CLI exit codes, and pin assertions |
claudecode/requirements-dev.txt |
Test-only PyYAML dependency kept out of runtime requirements |
docs & config |
Runner requirement docs, fixture un-ignore rule, execution plan |
Found 5 testing, reliability, and correctness issues. Consider addressing the suggestions in the comments.
- Correct setup-python v4.9.1 fixture record (node16, not node20) - Add refresh-action-runtime-metadata.py: --check compares every record with the upstream manifest at that SHA, --write refreshes it - Reject external composite records without recorded dependencies and validate recorded dependencies recursively, with cycle detection - CLI: exit 2 with a one-line error on missing or non-action input, accept docker:// steps, put the script directory on sys.path - Workflow test: subset and by-name assertions with a clear message for unrecorded actions instead of exact list and index matching - README: name the actions/runner 2.327.1 floor, document the guard and refresh check - Remove the exec plan; docs/ holds customization docs only Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Re-reviewing: state changed from APPROVED to APPROVED
There was a problem hiding this comment.
📋 PR Summary:
This PR bumps the action's own GitHub Action dependencies (checkout, cache, cache/save, setup-python, setup-node, upload-artifact) to their first Node 24 majors, pinned by commit SHA, in action.yml and both repository workflows, and explicitly disables setup-node v5's new default package-manager caching to preserve prior behavior. It also adds an offline guard (scripts/check-action-runtime-dependencies.py plus a supporting library) that walks the local composite action graph and rejects any reachable external action whose recorded runtime is not Node 24 or Docker, backed by a SHA-keyed fixture. A companion refresh-action-runtime-metadata.py --check/--write derives the fixture's runtime fields from upstream manifests, PyYAML is added as a dev-only dependency, and README documents the runner requirement and how to run the new tooling.
12 files reviewed
| File | Changes |
|---|---|
action.yml |
Bump cache, checkout, setup-python, setup-node, upload-artifact to Node 24 SHAs |
.github/workflows/*.yml |
Bump CI actions to Node 24 pins; install dev requirements |
scripts/action_runtime_dependencies.py |
Library that walks composite action graph and validates runtimes |
scripts/action_runtime_metadata.py |
Fetches upstream runs.using per SHA and diffs the fixture |
scripts/*.py (CLIs) |
CLI entry points for the offline guard and fixture refresh |
scripts/fixtures/action-runtime-metadata.json |
Recorded per-SHA action runtimes used by the offline guard |
claudecode/test_action_runtime_dependencies.py |
Tests for the guard, refresh tooling, and pinned CI actions |
deps & docs |
PyYAML dev dependency, runner-requirement docs, fixture un-ignore |
Found 3 testing, maintainability, and security issues. Consider addressing the suggestions in the comments.
…equire SHA pins - Test every job in every .github/workflows file against the fixture - Refresh script derives composite dependencies from upstream runs.steps, mapping the composite's local ./path steps to owner/repo/path@sha - Reject action references not pinned to a full 40-hex commit SHA Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Re-reviewing: state changed from APPROVED to APPROVED
…harden step parsing - Validate recorded composite dependencies with the same full-SHA rule as manifest references; the refresh tool rejects unpinned fixture keys - Keep an upstream composite's ./path steps verbatim and reject such records: the runner resolves them against the caller workspace, not upstream - Skip null or non-mapping steps and non-string uses in both modules - Workflow test helper ignores docker:// steps - Cover every workflow job, tag pins, malformed steps and local-step rejection with tests Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
📋 PR Summary:
Bumps every GitHub Action this repository depends on (checkout, cache, cache/save, setup-python, setup-node, upload-artifact) to their first Node 24 major, pinned by full commit SHA, across action.yml and both workflows, and explicitly disables setup-node v5's new default package-manager caching to preserve prior behavior. To keep the migration from silently regressing, it adds an offline guard (scripts/check-action-runtime-dependencies.py plus action_runtime_dependencies.py) that walks action.yml and its local composites and rejects any external action whose recorded runtime is not Node 24 or Docker, validating external composites recursively through a recorded dependencies list and requiring full-SHA pins throughout. A companion scripts/refresh-action-runtime-metadata.py derives the fixture from upstream manifests at each pinned SHA (--check / --write), so records are derived rather than asserted. PyYAML is added as a dev-only dependency, with 20+ new pytest cases and README notes on the runner requirement (actions/runner 2.327.1+).
12 files reviewed
| File | Changes |
|---|---|
action.yml |
Node 24 action pins; disable setup-node package-manager cache |
.github/workflows/*.yml |
Node 24 pins; install dev requirements in test CI |
scripts/action_runtime_dependencies.py |
Offline runtime guard with recursive composite and SHA-pin validation |
scripts/action_runtime_metadata.py |
Derives runtime and composite dependencies from upstream manifests |
scripts/*.py CLIs |
CLI entry points for checking and refreshing runtime metadata |
scripts/fixtures/action-runtime-metadata.json |
Per-SHA runtime fixture for legacy and current action pins |
claudecode/test_action_runtime_dependencies.py |
Tests for the guard, refresh tooling, and workflow pins |
deps and docs |
PyYAML dev dependency, runner requirement notes, fixture un-ignore |
Found 3 testing and maintainability issues. Consider addressing the suggestions in the comments.
…e un-ignore, update docs - Add external_reference_errors() so the workflow-level test applies the checker's own policy (docker and recorded composites accepted, tag pins and legacy runtimes rejected) instead of reimplementing it - Un-ignore scripts/fixtures/*.json rather than one filename - AGENTS.md testing section installs requirements-dev, documents the guard and refresh check, and drops the stale test count Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 participant
What
Bumps the action's own GitHub Action dependencies to Node 24 releases, pinned by commit SHA, in
action.ymland both workflows:Nothing changes for users of the action: the reviewer still runs on Node 18, test CI on Node 20, and inputs, outputs, triggers, permissions and cache keys are untouched.
setup-nodev5 turns on package-manager caching by default, so it is explicitly disabled to keep the previous behavior.Also included:
scripts/check-action-runtime-dependencies.py, that fails when any action reachable fromaction.ymlis recorded as anything other than Node 24 or Docker. External composite actions must have their dependencies recorded and are checked recursively; ones with local./pathsteps are rejected, since the runner resolves those against the caller's workspace. It reads a fixture of per-SHA runtimes and has its own tests. PyYAML is added as a dev-only dependency for it.scripts/refresh-action-runtime-metadata.py --checkcompares every fixture record, runtime and composite dependencies, with the upstream manifest at that SHA, so the fixture is derived, not asserted.--writerefreshes it. Only full-SHA pins are accepted.Why these versions
These are the first Node 24 major of each action, not the newest (v7 exists for most of them). This PR is about the runtime change only. Later majors carry unrelated behavior changes, for example checkout v6 moves persisted credentials to a separate file. The monorepo follow-up can move further as its own decision.
Context
First step of WEB-5090. Next comes a PSPDFKit monorepo PR that adopts this repo's merged SHA and updates its other actions.
Validation
refresh-action-runtime-metadata.py --check: all 15 records match upstream.test_eval_engine.pyfailures are pre-existing and reproduce onmain.git diff --checkclean. Both CI workflows green.Not validated yet