chore(deps): bump actions/checkout from 4 to 7 - #22
Conversation
LabelsThe following labels could not be found: Please fix the above issues or remove invalid values from |
dependabot
Bot
added
dependencies
srpatcha
left a comment
srpatcha
left a comment
There was a problem hiding this comment.
Approving, with a caution proportional to the size of the jump.
Verified
actions/checkout v7 exists and is current (latest
v7.0.1). The diff is a pure version-string
change across the workflows.
This is v4 → v7, three majors
Most of this batch moves one major. This one does not, and the intermediate
majors are where behaviour changed — actions/download-artifact in particular
altered artifact naming and matching semantics across v4/v5, so a workflow that
uploads under one name and downloads by pattern can silently retrieve nothing
rather than fail.
The check most likely to catch that is an end-to-end run that uploads and then
downloads an artifact in the same workflow. Worth confirming that path exists
here before merging, because a download that quietly returns zero files produces
a downstream error that looks like anything but a version bump.
The failing checks are not this PR
Lint & Format and Quick Checks fail identically on all five open Dependabot
PRs and on master. Reproduced locally against master:
$ ruff check eosim/
Found 324 errors.
$ ruff format --check eosim/
178 files would be reformatted, 15 files already formatted
Pre-existing, inherited by every PR. Worth fixing separately — a permanently red
check trains people to ignore it — but not a reason to hold this.
Approving because the version is real and the change is mechanical. If anything
in this repository round-trips artifacts between jobs, I would want one green run
of that path before merging rather than after.
srpatcha
left a comment
srpatcha
left a comment
There was a problem hiding this comment.
Review — EoSim#22 "chore(deps): bump actions/checkout from 4 to 7"
head: 2d6b29d author: app/dependabot ci: fail (Lint & Format, Quick Checks — pre-existing on master, not caused by this PR)
Verdict: Safe by inspection, and of the five open EoSim dependabot PRs this is the one with a concrete reason to land: it clears the Node 20 deprecation warning that every EoSim job currently emits. The one breaking change in the v4→v7 range does not apply here, verified below. Same CI-integrity caveats as EoSim#21 — the pipeline this merges under cannot fail.
Findings
| # | Severity | File:line | Finding | Recommended fix |
|---|---|---|---|---|
| 1 | High | .github/workflows/ci.yml:263-278, repo branch protection, .github/workflows/ci.yml:226 | Three pre-existing CI-integrity defects, detailed with evidence in the review of EoSim#21 and identical here: ci-summary is an aggregate gate that cannot fail (if: always(), needs: everything, and no step evaluates needs.*.result — it reported pass on this PR while lint failed and six jobs were skipped); master has required_status_checks: null so nothing gates merge; and the Security Scan job's pip-audit --strict 2>/dev/null || true reports success unconditionally. Not introduced by this diff. |
See EoSim#21 review, "Proposed changes", for the ordered fix. Do not require CI Summary in branch protection until it is capable of failing. |
| 2 | Medium | CI state — Lint & Format, Quick Checks | Both red on pre-existing ruff violations in eosim/ (I001, F401, UP031, N806 across eosim/analysis, eosim/api, eosim/artifacts, eosim/cli, eosim/plugins, eosim/tests), unrelated to this diff. Because test, coverage, validate-platforms, simulator-smoke, docs and build all declare needs: lint, every job that would exercise the 22 changed uses: lines was skipped. A PR that rewrites actions/checkout in nine workflow files has been proven by exactly one thing: that Security Scan and Simulator Smoke — the two jobs with no needs: lint — still check out. |
Land a ruff check --fix / ruff format pass on master first, then re-run this PR so the checkout bump is actually exercised across the matrix, including the macos/windows legs where a checkout regression is most likely to show. |
| 3 | Low | .github/workflows/scorecard.yml:25, .github/workflows/sync-release-branch.yml, .github/workflows/deploy-pages.yml, .github/workflows/codeql.yml, .github/workflows/video-build.yml | Five of the nine touched workflows are schedule/push/tag/workflow_dispatch-triggered only. Their checkout@v7 lines will first execute on a cron tick, a merge to master, or a tag — never on this PR. The blast radius of this bump is wider than what any PR run can cover, even a green one. |
Merge decision only. If you want early signal, workflow_dispatch the scheduled ones once after merge rather than waiting for the Monday 06:00 cron. |
Not a finding, verified as inapplicable. The one genuine breaking change in the v4→v7 range is v6.1.0's safer pull_request_target defaults (github.blog changelog 2026-06-18): checkout of a fork PR head is now blocked under pull_request_target and workflow_run unless allow-unsafe-pr-checkout is set. EoSim uses neither trigger. Verified by grep across every file in git ls-tree origin/master .github/workflows/ — zero hits for pull_request_target, zero for workflow_run. The remaining v5/v6/v7 changes are Node 24 runtime, ESM conversion and dependency bumps, none of which touch the inputs used here; the only with: blocks in the diff are fetch-depth: 0 and ref:, both unchanged across the range.
Positive reason to land this, recorded with evidence. The Lint & Format job log (run 33365422715) ends with:
##[warning]Node.js 20 is deprecated. The following actions target Node.js 20 but are being forced to run on Node.js 24: actions/checkout@v4, actions/setup-python@v5.
actions/checkout@v7 removes half of that. The other half needs the matching actions/setup-python bump, which dependabot has not opened for EoSim — worth opening by hand, since the forcing behaviour is a deprecation grace period, not a permanent state.
Architecture conformance
Conforms. Infrastructure-tier change (master design §21) to a Tier-1 Foundation repository. No source, #include, link line or manifest entry — §5.1 dependency direction untouched. Findings 1-2 bear on §17 and §28 in the same way as EoSim#21, and are already filed as the §28.2 proposal of 2026-09-02 and the §28.3 proposal of 2026-09-03. No new proposal warranted.
Proposed changes
None to the diff. Repository order of operations is in the EoSim#21 review; step 1 (green lint on master) is the prerequisite for this PR having any test evidence at all.
Verification I ran
- Grep over every workflow in
git ls-tree --name-only origin/master .github/workflows/forpull_request_targetandworkflow_run→ zero matches, which is what makes the v6.1.0 breaking change inapplicable. gh run view --job 99404978635 --log-failed(Lint & Format) and--job 99404978392(Quick Checks) → the ruff violations in finding 2,exit code 1, and the Node 20 deprecation warning quoted above.gh api repos/embeddedos-org/EoSim/branches/master/protection→required_status_checks: null.gh pr view 22 --json mergeStateStatus,reviewDecision,mergeable→UNSTABLE APPROVED MERGEABLE.- Read the changed workflows from
origin/masterviagit show, not from the working tree.
Not checked
- Nothing was executed. No workflow ran with
checkout@v7. Every job that would have — the 3×N test matrix, coverage, docs, build — was skipped behindneeds: lint. - I did not verify behaviour on the
macosorwindowsmatrix legs, which is where an ESM/Node 24 checkout change is most likely to surface and where nothing has run. - The local EoSim checkout is dirty (179 modified tracked
.pycfiles) and two commits behindorigin/master; the sync step correctly left it alone and I read everything fromorigin/master. I did not stash, reset or check out anything. - I did not check whether any self-hosted runner is in use; the Node 24 requirement (Actions Runner ≥ 2.327.1) is satisfied by GitHub-hosted runners and unverified for anything else.
Automated architecture review of 2d6b29dff44b — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.
Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 7. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v7) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
2d6b29d to
a98a751
Compare
srpatcha
left a comment
srpatcha
left a comment
There was a problem hiding this comment.
Review — EoSim#22 "chore(deps): bump actions/checkout from 4 to 7"
head: a98a751 author: app/dependabot ci: fail (3 red — CI Summary, Lint & Format, Quick Checks; 5 green; 5 skipping)
Verdict: Follow-up on the review at 2d6b29df. One prior finding is genuinely fixed and the evidence is visible in this PR's own check list. But the ground has moved underneath this PR in a way that changes the merge decision: Dependabot was removed from this repository yesterday, so this branch is an orphan that will never be rebased or completed — and as it stands, merging it leaves EoSim running three different actions/checkout majors.
Prior findings — status
| Prior # | Status | Evidence |
|---|---|---|
1a — ci-summary is an aggregate gate that cannot fail (High) |
Resolved in 44b2038 (#26) |
ci.yml:269-294 adds a Gate step that reads every needs.*.result out of env and exit 1s on any non-success. Best available evidence: CI Summary is red on this very PR, where the prior review recorded it reporting pass with lint red and six jobs skipped. |
1b — master has required_status_checks: null (High) |
Open | Re-checked today: gh api repos/embeddedos-org/EoSim/branches/master/protection → {"enforce_admins": false, "required_status_checks": null}. Fixing 1a made CI Summary capable of failing; nothing yet makes it capable of blocking. The two halves only work together. |
1c — pip-audit --strict 2>/dev/null || true (High) |
Open, untouched | ci.yml:205 is unchanged: pip install -e "." && pip-audit --strict 2>/dev/null || true. Security Scan is green on this PR and structurally cannot be anything else. Note the || true covers the whole line, so a failure of pip install -e "." is swallowed too — the audit can be skipped entirely and still report success. .ai/reviewer.md: a security step that reports success without having run. |
| 2 — lint red, every dependent job skipped (Medium) | Partially addressed | Still red (Lint & Format, Quick Checks) and Test, Code Coverage, Build Documentation, Build Package and Simulator Smoke Test are still skipping. But coverage is better than last time: Validate Platforms, Coverage Gate and Simulator Smoke now run and pass, where the prior review could only point at Security Scan and Simulator Smoke. 1c5e4f8 (#27) repointed Validate Platforms at the relocated descriptor directory. The 3×N OS/Python matrix — the only thing that would actually exercise 23 rewritten checkout lines on macOS and Windows — still has not run. |
| 3 — five of nine touched workflows are schedule/push/tag-only (Low) | Unchanged by nature | Still true, still a merge-decision note rather than a defect. |
Findings
| # | Severity | File:line | Finding | Recommended fix |
|---|---|---|---|---|
| A | High | repository configuration (ef6c3df, on master, not on this branch) |
Dependabot has been removed from this repository, and this PR outlived it. ef6c3df chore(ci): remove Dependabot, add Claude + Copilot code review (#15) (2026-09-13) deletes .github/dependabot.yml — 75 lines, gone; git show origin/master:.github/dependabot.yml now fails — and its message states that alerts and automated security fixes were disabled via the API and that 90 open Dependabot PRs were closed. This one is still open. The consequence for review: there will be no rebase, no follow-up bump, and no actions/setup-python PR — the prior review recommended opening that one by hand, and that is now the only way it can happen. Treat this as a one-shot manual decision, not a routine bot PR. The same commit notes it "also stops CVE alerts for dependencies", which is already recorded in the standing proposal of 2026-09-03, "Dependabot was disabled org-wide, and no section of the design owns dependency-vulnerability monitoring" — so no new proposal is appended. |
Decide deliberately. Either merge it and immediately follow with a hand-written PR covering finding B, or close it and do the whole actions/checkout bump once, correctly, in a single reviewed change. Leaving it open indefinitely is the worst of the three. |
| B | Medium | .github/workflows/nightly.yml, .github/workflows/weekly.yml (at this head); .github/workflows/claude-code-review.yml (on master) |
The bump does not do what its title says, and merging it leaves three checkout majors in one repository. At this head, nine workflows carry actions/checkout@v7 (23 call sites) but nightly.yml and weekly.yml are still on @v4 (9 call sites) — Dependabot never touched them. Separately, master gained claude-code-review.yml at actions/checkout@v6, which this branch does not contain. Post-merge state: v4 in nightly and weekly, v6 in claude-code-review, v7 in the other nine. Per finding A nothing will ever converge them automatically. |
In the follow-up PR, move nightly.yml, weekly.yml and claude-code-review.yml to @v7 as well, so the repo has one checkout version and the Node 20 deprecation is actually cleared rather than halved. |
| C | Medium | .github/workflows/linked-issue.yml:4 |
The prior review's "verified as inapplicable" rests on a premise that is no longer true. It concluded the v6.1.0 breaking change — fork-PR-head checkout blocked under pull_request_target/workflow_run without allow-unsafe-pr-checkout — could not apply, on the evidence of zero pull_request_target hits across the workflows. 0450aca (#33) has since added linked-issue.yml, which declares pull_request_target: at :4. The conclusion still holds, for a narrower reason, and I re-derived it rather than carrying it forward: linked-issue.yml contains no actions/checkout of its own, so this PR does not touch it; it delegates to embeddedos-org/.github's reusable policy workflow, whose checkout is SHA-pinned (11bd71901bbe5b1630ceea73d27597364c9af683, a v4) and targets repository: embeddedos-org/.github at an explicit ref: — never the pull-request head. Nothing in this diff can reach the v6.1.0 change. |
No change. Recorded so the next reviewer does not re-run the old grep, find a hit, and assume the worst — and so that whoever bumps the reusable workflow's own checkout knows to check this. |
| D | Low | branch dependabot/github_actions/actions/checkout-7 |
The branch is 2 commits behind master (ef6c3df, f47eb03 chore(deps): bump actions/github-script from 7 to 9 (#25)). Ordinarily a bot would rebase; per finding A, none will. |
Fold into the decision in A. If merging, confirm GitHub's merge produces the union rather than reverting claude-code-review.yml. |
Architecture conformance
Conforms, unchanged from the prior review. This is an Infrastructure-tier change (master design §21) to a Tier 1 — Foundation repository. No source file, #include, import, link line, CMake entry or manifest dependency is touched, so §5.1's dependency law is not engaged and nothing becomes a runtime dependency. No §21.1 split question arises.
The sections still at stake are §17 and §28, and the picture has improved in exactly one place. §28's evidence model requires that a check which passes has actually run; ci-summary now satisfies that (prior 1a), which is real progress on a Tier-1 repo's gate. It is undercut by 1b and 1c: a gate that can fail but does not block merge, sitting alongside a Security Scan that reports success unconditionally. Under §28 none of these three green ticks — Security Scan, and by extension CI Summary when it does go green — is evidence for anything. Both gaps are already filed as the §28.2 proposal of 2026-09-02 and the §28.3 proposal of 2026-09-03; no new proposal is warranted.
Worth stating plainly for the merge decision: the positive case from the prior review survives. actions/checkout@v7 does remove half the Node 20 deprecation warning that every EoSim job currently emits, and the only with: blocks in the diff are fetch-depth: 0 and ref:, both unchanged across the v4→v7 range. The change is still safe by inspection. It is the process around it that has changed, not the diff.
Proposed changes
No change to the diff. Order of operations for the repository:
- Decide A explicitly — merge-then-follow-up, or close-and-redo. Do not let it sit.
- Whichever route, one hand-written PR to bring
nightly.yml,weekly.ymlandclaude-code-review.ymlto@v7, plus theactions/setup-pythonbump that clears the other half of the Node 20 warning. Nothing else will produce these now. (A, B) ruff check --fix/ruff formatonmastersoLint & Formatgoes green and the test matrix stops being skipped. Until then no PR in this repo has behavioural evidence. (prior 2)- Remove the
2>/dev/null || truefromci.yml:205soSecurity Scancan fail. (prior 1c) - Only after 3 and 4: set
required_status_checksonmasterto includeCI Summary, which is now finally capable of failing. Requiring it today would pin the branch behind a red lint backlog. (prior 1b)
Not checked
- Nothing was executed. No workflow ran with
checkout@v7on this head. The jobs that would exercise the bump across the OS/Python matrix —Test,Code Coverage,Build Documentation,Build Package,Simulator Smoke Test— are allskippingbehindneeds: lint. macOS and Windows legs, where an ESM/Node 24 checkout change is most likely to surface, remain completely unexercised, as they were at the last review. - Findings B, C and D are Observed from
git ls-tree/git showagainstorigin/masterand against the PR heada98a7514, and finding A fromgit show ef6c3dfplus the absence of.github/dependabot.ymlonorigin/master. Nothing was run to demonstrate them. - I did not verify that the 90 closed Dependabot PRs are closed, nor why this one survived the sweep. Quoted from the
ef6c3dfcommit message, not independently confirmed. - The reusable policy workflow was read, not run. Finding C's claim that its checkout targets
embeddedos-org/.githubrather than the PR head comes from readinglinked-issue-policy.yml; I did not execute it against a fork PR. - The local
EoSimcheckout is dirty — 179 modified tracked.pycfiles — and it was left untouched, as the rules of engagement require. Everything above was read fromorigin/masteror from the PR head object, never from the working tree. Notef4b221a chore: untrack compiled bytecode (#20)has landed onmaster, so this dirt is now stale local state rather than tracked content; it still needs a human to clear it, and this review did not. - Prior finding 1a is confirmed by proxy, not by construction. I read the new
Gatestep and observedCI Summaryred on this PR; I did not construct a case where all jobs succeed to confirm the gate then passes. mergeable/mergeStateStatuswere not usable in the bundle. Given finding D, a human should confirm the merge result before clicking.
Automated architecture review of a98a75140144 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.
1 participant
Bumps actions/checkout from 4 to 7.
Release notes
Sourced from actions/checkout's releases.
... (truncated)
Changelog
Sourced from actions/checkout's changelog.
... (truncated)
Commits
3d3c42eprep v7.0.1 release (#2531)2880268escape values passed to --unset (#2530)12cd223trim only ascii whitespace for branch (#2521)62661c4skip running unsafe pr check if input is default (#2518)e8d4307Bump the minor-actions-dependencies group with 2 updates (#2499)631c942eslint 9 (#2474)4f1f4aeBump actions/upload-artifact from 4 to 7 (#2476)ba09753Bump actions/checkout from 6 to 7 (#2488)b9e0990Bump docker/login-action from 3.3.0 to 4.2.0 (#2479)e8cb398Bump docker/build-push-action from 6.5.0 to 7.2.0 (#2478)