Repository navigation
ci: pin workflow actions to Node 24 release SHAs (#301) - #307
Merged
Merged
Conversation
Every `uses:` in CI and Package now names a full commit SHA with its release in a trailing comment, and each action moves to a release whose action.yml runs on node24: - actions/checkout v4 -> v7.0.1 - actions/upload-artifact v4 -> v7.0.2 - actions/setup-node v4 -> v7.1.0 - actions/setup-python v5 -> v7.1.0 - actions/setup-dotnet v4 -> v6.1.0 - github/codeql-action/upload-sarif v4 -> v4.39.3 - Swatinem/rust-cache v2 -> v2.9.3 Inputs, triggers, artifact names and paths are unchanged. A new workflow binding test rejects any action reference that is not a 40-character SHA with a version comment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
@sfl review |
Collaborator
Author
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
![hemsoft-sfl-reviewer[bot]](https://avatars.githubusercontent.com/in/5406096?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Verdict: PASS • Round 1
Confidence: 9/10
Review done by claude-opus-5-5 (same family: same-family: codex gated: codex Weekly 171% SUS 59 🔴 pause, defer)
The PR pins all 25 uses: references in ci.yml and package.yml to full commit SHAs on releases that run on node24. It also adds a static test that requires SHA pins with version comments. Every pin resolves to its upstream tag, and the inputs these workflows use still exist at those SHAs. The only finding is a Low gap in how strictly the new test matches uses: lines, so it does not block approval.
🔨 Full-Spectrum Review Summary
Effort: high
Review time: 1m 41s
Token count: Unavailable
| Pass | 🔴 Critical | 🟠 High | 🟡 Medium | ⚪ Low |
|---|---|---|---|---|
| 🔒 Security | 0 | 0 | 0 | 0 |
| ✅ Accuracy | 0 | 0 | 0 | 0 |
| 🧰 Quality & Maintainability | 0 | 0 | 0 | 1 |
| Total | 0 | 0 | 0 | 1 |
Key Findings
- ⚪ LOW — Quality & Maintainability: Pin test's line filter misses valid YAML step spellings.
scripts/tests/ci-workflow.test.cjs:72: The test selects lines with/^\s*(- )?uses:/. A valid YAML step written as- uses: actions/checkout@v4(more than one space after the dash), or in flow style- { uses: ... }, is never selected, so a floating tag could pass the test. The test also reads only the hardcodedci.ymlandpackage.yml, so a new workflow file is not checked. Neither spelling appears in this PR, so current CI is not affected. The impact is weaker protection against future regressions. Parsing the workflows with thejs-yamlalready loaded in this file and checking everysteps[].usesand job-leveluseswould close the gap. (non-blocking)
Coverage
- Files reviewed:
.github/workflows/ci.yml.github/workflows/package.ymlscripts/tests/ci-workflow.test.cjs
- Context read:
AGENTS.mdpackage.jsonscripts/ci-inputs.pyscripts/rust-metrics.pyscripts/rust-mutations.pyscripts/Invoke-SecurityScan.ps1.github/workflows (directory listing)upstream action.yml at each pinned SHAupstream release notes for checkout v5-v7, upload-artifact v5-v7, setup-node v5-v7, setup-python v6-v7, setup-dotnet v5-v6
- Verified:
- Compare
base_commit.sha=7ff35cbe248757d5eb877206f91ec34f982a2069, last commit =922b6a3945309356d46bc93930297c87c3da4e61, ahead_by 1, behind_by 0 - Each pinned SHA matches its tag upstream (annotated tags dereferenced for codeql-action v4.39.3 and rust-cache v2.9.3)
- Each pinned
action.yml(includingupload-sarif/action.yml) declaresruns.using: node24 - Counted 25
uses:lines at head (17 in ci.yml, 8 in package.yml), all SHA-pinned with matching version comments; only these two workflow files exist - Inputs still used (
fetch-depth,ref,name,path,retention-days,python-version,dotnet-version,node-version,cache,sarif_file,category) all exist in the pinnedaction.ymlfiles upload-artifactv7archivedefaults totrue, so zipped uploads are unchanged;include-hidden-filesdefault is unchanged- checkout
allow-unsafe-pr-checkoutonly affectspull_request_target/workflow_run; these workflows usepush,pull_requestandworkflow_dispatch - checkout v6 moves persisted credentials to a separate file and v6.0.1 adds worktree support;
rust-metrics.pyuses only localgit worktree add/remove,rev-parse,diffandls-files, with no authenticated fetch package.jsonhas nopackageManager/devEngines, so setup-node's automatic caching stays off;cache: 'npm'is set explicitly andnpm ci --ignore-scriptsskips thepostinstallscript- Job names and artifact names are unchanged, so existing tests that check job keys and step names are unaffected
- PR text says setup-dotnet is not exercised here, but
ci-inputs.pyneeds_retiredreturns true for.github/andscripts/paths, so this PR's CI should select the retired steps and run setup-dotnet v6.1.0. This affects only the PR description, not the code
- Compare
- Not verified:
- Runtime CI or Package results on this head, and whether Node 20 deprecation annotations disappear (execution is forbidden)
upload-sarifv4.39.3 behavior, which runs only onpushto main- GitHub-hosted runner versions against the 2.327.1 minimum (taken from GitHub-hosted runner policy, not checked)
- Release notes for codeql-action and rust-cache within the same major version were not read in full
- Tests run: None. Reviewer execution is disabled.
HemSoft full-spectrum review • 3 evidence-based passes • Reviewed head 922b6a3945309356d46bc93930297c87c3da4e61 • SFL v0
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.
Risk level: 4/10 (codexbar)
Worked on by: Claude Opus 5.5 on Home, for chief-dispatch-0cb32946
Closes #301.
Summary
CI and Package referenced every action by a movable tag, and four of them (
checkout@v4,upload-artifact@v4,setup-node@v4,setup-python@v5) target Node 20, which GitHub removed from Actions on 2026-09-23. Each run carried a "Node.js 20 is deprecated" annotation on every job.All 25
uses:lines (17 inci.yml, 8 inpackage.yml) now name a full commit SHA with the release in a trailing comment. Every pinned release'saction.ymldeclaresruns.using: node24:actions/checkoutv43d3c42e5…actions/upload-artifactv4cf430e03…actions/setup-nodev4949feb24…actions/setup-pythonv56d4fd22b…actions/setup-dotnetv40f4a354d…github/codeql-action/upload-sarifv433d7b85d…Swatinem/rust-cachev2d1527c8f…Each tag was resolved to its commit on the upstream repository, and the SHAs were checked a second time independently. Inputs, triggers, job names, artifact names and artifact paths are unchanged, so no docs needed updating.
scripts/tests/ci-workflow.test.cjsgains one test: everyuses:line in both workflows must beowner/repo[/path]@<40 hex chars> # vX.Y.Z. It fails onmain's workflows (first offenderactions/checkout@v4) and passes here.Release-note impact
I read the release notes for every major version between the old and new tags, and checked each change against how these workflows use the action:
allow-unsafe-pr-checkout(marked breaking): it applies only to fork code underpull_request_targetorworkflow_run. These workflows usepush,pull_requestandworkflow_dispatch, and v7.0.1 skips the check when the input keeps its default.persist-credentialsnow writes the token to a file under$RUNNER_TEMP, which the repo's git config reaches throughincludeIf, and v6.0.1 extended that to worktrees.rust-metricsrunsgit worktree addand checks for untracked inputs. Both stay local and the credential file is outside the workspace, so neither changes.package.jsonnames npm inpackageManagerordevEngines.packageManager. This repo has neither field and already setscache: 'npm'explicitly.NODE_AUTH_TOKENexport:builddoesn't publish or use a registry token.pip-installinput and EOL Pythons: neither is used;3.13is still supported.9.0.xis current. The input classifier treats.github/andscripts/changes as retired C# inputs, so this PR selects the .NET steps. Setup .NET v6.1.0 and all remaining gates passed in the current CI build job.archiveinput: it defaults totrue, the same zip upload as before. Artifact names staysecurity-scan-reports,rust-metrics,rust-mutations,npm-audit,stryker-report,ci-inputs,codexbar-macosandcodexbar-linux.Acceptance Criteria
uses:inci.ymlandpackage.ymlis a full 40-character SHA with its version in a trailing comment:grep -nE "uses: [^ ]+@[^ ]+" .github/workflows/*.ymllists 25 lines, all SHAs, and the new test enforces it.Definition of Done
workflow_dispatchrun ofPackagepass on the final head: CI, PR Package, and manual Package all completed successfully at922b6a3945309356d46bc93930297c87c3da4e61.scripts/tests/ci-workflow.test.cjspasses, with the new pin test:node --test scripts/tests/ci-workflow.test.cjsgives 5 passed, 0 failed.Validation
node --test scripts/tests/ci-workflow.test.cjson922b6a3(Node 24.22.1, js-yaml fromnpm ci --ignore-scripts): 5 passed, 0 failed.The same test file run against
main's two workflows: 4 passed, 1 failed (every action reference ... is a full commit SHA, actual- uses: actions/checkout@v4).Both workflows parse with js-yaml and keep their job lists (
security-scan, rust, rust-metrics, rust-mutations, build;msix, macos-app, linux-deb).The Rust gates were not run locally, since no Rust file changed. They run in this PR's CI on all three platforms.
Hosted validation: all 7 required CI checks passed; all 3 jobs in each Package run passed. There were no Node 20 annotations across all 13 jobs. Other annotations report hosted macOS runner capacity, the Ubuntu image migration, and a metrics fallback to completed base-run coverage after base tests failed. Candidate tests and the declared coverage/CRAP regression gates passed; the baseline-test failure cause was not diagnosed in this change.
Reviews: SFL Round 1 PASS and Codex clean receipt both cover the final head. Bot authors, editors and complete edit histories were authenticated; zero unresolved review threads. The SFL Low test-spelling note is nonblocking under the maintainer policy.
Limits
actions/setup-dotnetv6.1.0 was exercised successfully by the current CIbuildjob..github/andscripts/changes select retired C# qualification. This step result does not establish that the whole CI run passed.Upload Gitleaks SARIFstep runs only onpush, socodeql-action/upload-sarifv4.39.3 first runs on the merge commit's CI onmain.🤖 Generated with Claude Code