chore(deps): bump pnpm/action-setup from 4 to 6 - #40
dependabot[bot] wants to merge 1 commit into
Conversation
Bumps [pnpm/action-setup](https://github.com/pnpm/action-setup) from 4 to 6. - [Release notes](https://github.com/pnpm/action-setup/releases) - [Commits](pnpm/action-setup@v4...v6) --- updated-dependencies: - dependency-name: pnpm/action-setup dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
LabelsThe following labels could not be found: Please fix the above issues or remove invalid values from |
srpatcha
left a comment
There was a problem hiding this comment.
Approving. Target version verified to exist and be current, and the failing CI
here is not caused by this bump.
I checked every target in this batch against the upstream release list rather
than assuming Dependabot picked a real tag:
mymindstorm/setup-emsdk v16 latest v16 exists
actions/setup-java v6 latest v6.0.0 exists
pnpm/action-setup v6 latest v6.0.10 exists
actions/checkout v7 latest v7.0.1 exists
All current majors.
The red CI is not yours
Every job in this run fails with:
apps/epdf/epdf.h:5:10: fatal error: eapps_core.h: No such file or directory
master does not build, and has not for a while — 48 fatal errors from a clean
clone. The cause is .gitignore:61, a bare core rule intended for Unix core
dumps, which matches any file or directory named core at any depth and
therefore excluded core/, the source tree. core/common/include/eapps_core.h
is included by all 44 app targets and was never committed:
$ git ls-files | grep -c eapps_core.h
0
#42 fixes that. Once it lands, re-run these and the result will mean something.
Merging this before #42 is harmless — it cannot make a build that already fails
fail differently — but the check will stay red until #42 is in.
Worth noting for whoever triages these: a major-version bump of a GitHub Action
can carry breaking changes, and the usual way to find out is CI. That signal is
currently unavailable in this repository for reasons unrelated to any of these
four PRs, so approval here rests on the version existing and the diff being a
pure version string change — not on a green run.
srpatcha
left a comment
There was a problem hiding this comment.
Review — eApps#40 "chore(deps): bump pnpm/action-setup from 4 to 6"
head: 120f22c author: app/dependabot ci: fail (11 of 11 jobs) — pre-existing on master, see below
Verdict: The bump is safe and, as far as I can establish, has no effect at all: pnpm is provisioned into a lane that never successfully uses it. There is no pnpm-lock.yaml anywhere in the repository, every pnpm invocation in the job is written with an || npm fallback, and two of the lane's three steps iterate over directories containing zero package.json files. Merge it, but do not read the green-or-red result as saying anything about pnpm.
Findings
| # | Severity | File:line | Finding | Recommended fix |
|---|---|---|---|---|
| 1 | Medium | .github/workflows/ci.yml:110, :122, :134, :146 |
Every pnpm invocation in the job is written so that pnpm failing is indistinguishable from pnpm working: (cd "$d" && (pnpm install --frozen-lockfile || npm ci) && (pnpm run build || npm run build)). .ai/reviewer.md names this shape — a verification whose result is discarded — and the brief says not to round it down. Applied to this PR specifically: pnpm/action-setup is the step under change, and the lane is constructed so that a regression in it silently routes to npm. Bumping two majors of a tool the workflow is engineered not to depend on cannot be validated by that workflow. |
Decide which package manager this lane uses and let it fail. If it is pnpm, drop the || npm ci / || npm run build fallbacks and commit the lockfile (finding 2). If it is npm, delete the pnpm/action-setup step and this PR with it. The current arrangement pays for pnpm and gets npm's guarantees. |
| 2 | Medium | .github/workflows/ci.yml:110 |
pnpm install --frozen-lockfile cannot succeed: there is no pnpm-lock.yaml anywhere in the repository. Verified: find . -name pnpm-lock.yaml -not -path "*/node_modules/*" returns nothing. --frozen-lockfile requires one, so this command fails on every run and the || npm ci branch is always taken — and npm ci requires a package-lock.json, which is also absent from web-apps/ and browser-extensions/. This is the mechanism behind the one failing step that persists on current master (JS/TS lane :: desktop-apps lint+build): desktop-apps/eoffice/ has a package.json and a pnpm-workspace.yaml but no lockfile, so with set -e in effect the step fails. §9.2 requires "reproducible lockfiles/manifests for production builds"; a --frozen-lockfile flag with no lockfile is the inverse of that. |
Commit pnpm-lock.yaml for desktop-apps/eoffice (it already has pnpm-workspace.yaml, so pnpm is the intended manager there). That fixes the persistent CI failure and makes pnpm/action-setup — and therefore this PR — actually load-bearing. |
| 3 | Low | .github/workflows/ci.yml:95, :107-136 |
The job is named "JS/TS lane (web-apps + browser-extensions + desktop-apps)" and two of those three lanes contain nothing to build. Verified: find ./web-apps -maxdepth 2 -name package.json → 0; find ./browser-extensions -maxdepth 2 -name package.json → 0; find ./desktop-apps -maxdepth 2 -name package.json → 1 (desktop-apps/eoffice/package.json). Both directories exist and are full of app subdirectories, so the if [ -d ... ] guard passes, the for loop runs, the inner if [ -f "$d/package.json" ] never matches, and the step exits 0 having done nothing — without even the ::notice:: the else branch would have printed. A green step here means "no package.json found", which reads identically to "everything built". |
Fail, or at least warn, when a lane matches zero packages: count the matches in the loop and emit ::warning:: (or exit non-zero) if the count is 0. Otherwise the lane's coverage can drop to nothing without the job's colour changing — the same silent-drop failure mode eos' own tests/unit/test_cmake_test_registration.py exists to catch, and that eos#134 ("catch tests that are compiled but never called") is currently generalising. |
The bump itself is mechanically complete and correct: one call site, grep -rn "pnpm/action-setup@" finds no remaining @v4.
CI state
All 11 required checks are red, and none of it is attributable to this PR. Verified:
- Master run
32813232464(2026-08-25, six days before this PR was opened) failed on the identical 11 jobs at the identical 11 steps. That is the state of the branch point, not a regression. - Master has since partly recovered: run
33501131456(2026-09-01) fails on only 2 jobs —Native (windows-latest) :: BuildandJS/TS lane :: desktop-apps lint+build. - Ten of the eleven failures are outside this diff's reach entirely (
Native (…),Category (…),Sanitizers— none touches pnpm). The eleventh,JS/TS lane, is the job this PR edits, and it fails atdesktop-apps lint+buildfor the lockfile reason in finding 2 — which predates the PR and is unchanged by it.
Rebasing onto current master should take this from 11 red checks to 2. Fixing finding 2 would take it to 1.
Architecture conformance
Conforms. §21 places CI configuration in the Infrastructure tier; a package-manager provisioning action is build-time only and never a runtime dependency, so §5.1's dependency law is untouched and no eApps source changes. The lane builds Tier-5 reference applications (§20.1: eBrowser and eOffice are capability proofs, not pillars), so the stakes are low. Where the design does bear on findings 1–3 is §9.2 and §3's Yocto lesson — "reproducible lockfiles/manifests for production builds", "deterministic builds" — and §11.1, which makes .eapp a first-class artifact type. An artifact whose dependency resolution differs run to run, from whichever of two package managers happened to succeed, has no reproducible provenance to publish under §14 of the component contract (Integrity: "hashes, signatures and provenance").
Proposed changes
In order:
- Merge the bump. v5's Node 24 requirement is satisfied and v6 only adds pnpm 11 support; there is no reason to stay on v4.
- Rebase onto current
masterso the check summary is interpretable. - Separately: commit
pnpm-lock.yamlfordesktop-apps/eoffice(finding 2). This is the smallest change that both fixes the one real CI failure in this lane and makespnpm/action-setupdo something. - Separately: drop the
|| npm ci/|| npm run buildfallbacks once (3) lands (finding 1), so the lane reports pnpm failures instead of absorbing them. - Separately: make a zero-match lane warn or fail (finding 3).
No fix PR opened. Findings 1–3 are Medium, and each of them changes what CI enforces rather than repairing a mechanical defect — and generating a lockfile for a project I cannot install dependencies for is not something I can verify by running anything, which is the brief's bar.
Verification I ran
- Upstream release notes for both majors crossed. v5.0.0 (2026-03-17): "Updated the action to use Node.js 24" — nothing else. v6.0.0 (2026-04-10): "Added support for pnpm v11" — nothing else. No input removed, renamed or redefined across either.
- The one call site (
ci.yml:104-106) passesversion: 9explicitly. That matters becausepnpm/action-setuperrors when it can resolve a version from neither its input nor apackageManagerfield, and errors on some ambiguous combinations of the two; here the input is explicit. Verified there is no conflictingpackageManagerfield:grep -rn packageManager eApps/package.json eApps/*/package.jsonreturns nothing. pnpm 9 is still installable under v6, whose change is only to add pnpm 11. - Runner floor satisfied. The job is
runs-on: ubuntu-latest(ci.yml:96); everyruns-on:in eApps is a GitHub-hosted label, so v5's Node 24 move needs no self-hosted runner upgrade. - Findings 2 and 3, the load-bearing evidence:
find . -name pnpm-lock.yaml -not -path "*/node_modules/*"→ none; nopackage-lock.jsonunderweb-apps/orbrowser-extensions/;package.jsoncounts of 0 / 0 / 1 for the three lanes;desktop-apps/eoffice/holds bothpackage.jsonandpnpm-workspace.yaml. Readci.yml:94-152in full — all four steps use the same(pnpm … || npm …)shape underset -e. - CI attribution: job-to-workflow mapping from
ci.yml:20-95, and the failing job/step sets compared against master runs32813232464and33501131456viagh run view.
Not checked
- The bump in action. The
JS/TS lanejob fails before pnpm's version could matter, and I did not run the lane locally, so I have not observedpnpm/action-setup@v6install pnpm in this repository. The assessment rests on the changelogs plus the call-site audit. - Whether
desktop-apps/eofficebuilds. I did not read itspackage.json, runpnpm install, or check whether its dependencies resolve — so I do not know whether adding the lockfile in finding 2 turns the step green or merely moves the failure topnpm run build. Recommending the lockfile is diagnosis of the current failure, not a prediction that it is the last one. Unknown. - Whether
web-apps/andbrowser-extensions/are supposed to contain buildable packages. I confirmed they contain many subdirectories and zeropackage.jsonfiles; whether that is by design (source-only apps built elsewhere) or a gap is not something the workflow or the directory layout told me. Finding 3 is about the workflow's silence either way, not about which answer is right. - The nine unrelated pre-existing failures. Established as pre-dating this PR and unreachable from it; not diagnosed.
actions/setup-node@v4atci.yml:101, the step immediately above — not in this diff, and eApps#41 (actions/checkout4 → 7) is the only other action bump currently open.
Automated architecture review of 120f22cd2624 — 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 pnpm/action-setup from 4 to 6.
Release notes
Sourced from pnpm/action-setup's releases.
Commits
0977fd9docs: Update README to include devEngines.packageManager (#273)48261acfix: update pnpm to v11.19.0 (#283)75677f7ci: use pnpm 11 forpr-check(#284)769ae71refactor: introduce restore keys for cache (#280)6fed91fdocs(README): point users to the successor pnpm/setup action (#282)0ebf471fix: update pnpm to v11.7.0 (#267)0e279bbfix: update pnpm to 11.1.1 (#248)3e83581fix: drop patchPnpmEnv so standalone+self-update works on Windows (#258)551b42edocs(README): fixcache_dependency_pathtype (#257)739bfe4fix: self-update bootstrap to packageManager-pinned version (#233) (#256)Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)