Summary
tests.yaml installs with npm ci, but all four install steps in the CD workflows use npm install. npm ci installs exactly what package-lock.json pins; npm install is free to resolve newer in-range versions and rewrite the lockfile. The result is that the dependency tree CI validates is not necessarily the tree that runs on vlcdhp02 or vlcdhprdp02.
Affected steps
| File |
Line |
Job |
Runs on |
Needs devDeps? |
.github/workflows/cd_dev.yaml |
55 |
test |
ubuntu-latest |
yes (npm run allTests) |
.github/workflows/cd_dev.yaml |
84 |
deploy |
vlcdhp02 |
no |
.github/workflows/cd_prod.yaml |
20 |
test |
vlcdhprdp02 |
yes (npm run allTests) |
.github/workflows/cd_prod.yaml |
46 |
deploy |
vlcdhprdp02 |
no |
For contrast, tests.yaml:27 and tests.yaml:47 already do this correctly.
Why it matters
1. Deploys are not reproducible. A caret range like ^17.4.2 lets the server resolve a different version at deploy time than CI tested. This directly undercuts lockfile work — #128 pins a tree, verifies it passes npm audit, npm-check, and the full suite, and then the deploy step is free to install something else.
2. Silent drift stays silent. npm ci fails loudly when package.json and package-lock.json disagree. npm install quietly reconciles them. A stale lockfile can therefore ride along unnoticed — which is exactly what happened to the lockfile root version field, stuck at 0.1.1 on main while package.json said 1.0.0 until #128 regenerated it.
3. cd_prod.yaml runs both jobs against the same directory on the same box. The test job does npm install in /srv/node/tiny-node/, then the deploy job does git stash && git checkout main && git pull && npm install in that same directory. If the first npm install rewrites package-lock.json, the working tree is now dirty and the git stash masks it rather than surfacing it. With npm ci the lockfile is never written, so there is nothing to stash.
4. The dev cache is already configured for npm ci. cd_dev.yaml:41-52 caches ~/.npm keyed on hashFiles('**/package-lock.json'). That is the npm download cache, which is precisely what npm ci consumes — so this swap costs nothing on the hosted runner and arguably speeds it up.
Proposed change
Replace npm install with npm ci in all four places. Minimal and behavior-preserving:
# .github/workflows/cd_dev.yaml:53-56
- name: Install dependencies and run the test
run: |
npm ci
npm run allTests
# .github/workflows/cd_prod.yaml:14-21
- name: Test the app on the server
run: |
cd /srv/node/tiny-node/
git stash
git checkout main
git pull
npm ci
npm run allTests
The two deploy steps (cd_dev.yaml:84, cd_prod.yaml:46) take the same npm ci swap.
Optional refinement, with a tradeoff
The deploy steps only need runtime dependencies, so npm ci --omit=dev would work there and would keep playwright, c8, and supertest off the production server. The cost: after a deploy, running npm run allTests by hand on that box fails until someone reinstalls devDeps. Since cd_prod.yaml runs its test job in the same directory, the next pipeline run would reinstall them anyway — but manual on-server testing between deploys would break. Worth deciding deliberately rather than folding into this change by default.
Caveats
npm ci deletes node_modules before installing, so it is slower than a warm npm install on the self-hosted boxes. The reproducibility is the point, but the added minute per deploy is real.
npm ci hard-requires package-lock.json to be present and in sync. That is the desired behavior, but it means a future PR that edits package.json without regenerating the lock will now fail CD instead of silently self-correcting.
Context
Found during static review of #128. Related to #124, which covers the other reproducibility gap in these same CD workflows (actions pinned to @master rather than a release tag).
Summary
tests.yamlinstalls withnpm ci, but all four install steps in the CD workflows usenpm install.npm ciinstalls exactly whatpackage-lock.jsonpins;npm installis free to resolve newer in-range versions and rewrite the lockfile. The result is that the dependency tree CI validates is not necessarily the tree that runs on vlcdhp02 or vlcdhprdp02.Affected steps
.github/workflows/cd_dev.yamltestubuntu-latestnpm run allTests).github/workflows/cd_dev.yamldeployvlcdhp02.github/workflows/cd_prod.yamltestvlcdhprdp02npm run allTests).github/workflows/cd_prod.yamldeployvlcdhprdp02For contrast,
tests.yaml:27andtests.yaml:47already do this correctly.Why it matters
1. Deploys are not reproducible. A caret range like
^17.4.2lets the server resolve a different version at deploy time than CI tested. This directly undercuts lockfile work — #128 pins a tree, verifies it passesnpm audit,npm-check, and the full suite, and then the deploy step is free to install something else.2. Silent drift stays silent.
npm cifails loudly whenpackage.jsonandpackage-lock.jsondisagree.npm installquietly reconciles them. A stale lockfile can therefore ride along unnoticed — which is exactly what happened to the lockfile rootversionfield, stuck at0.1.1onmainwhilepackage.jsonsaid1.0.0until #128 regenerated it.3.
cd_prod.yamlruns both jobs against the same directory on the same box. Thetestjob doesnpm installin/srv/node/tiny-node/, then thedeployjob doesgit stash && git checkout main && git pull && npm installin that same directory. If the firstnpm installrewritespackage-lock.json, the working tree is now dirty and thegit stashmasks it rather than surfacing it. Withnpm cithe lockfile is never written, so there is nothing to stash.4. The dev cache is already configured for
npm ci.cd_dev.yaml:41-52caches~/.npmkeyed onhashFiles('**/package-lock.json'). That is the npm download cache, which is precisely whatnpm ciconsumes — so this swap costs nothing on the hosted runner and arguably speeds it up.Proposed change
Replace
npm installwithnpm ciin all four places. Minimal and behavior-preserving:The two deploy steps (
cd_dev.yaml:84,cd_prod.yaml:46) take the samenpm ciswap.Optional refinement, with a tradeoff
The deploy steps only need runtime dependencies, so
npm ci --omit=devwould work there and would keepplaywright,c8, andsupertestoff the production server. The cost: after a deploy, runningnpm run allTestsby hand on that box fails until someone reinstalls devDeps. Sincecd_prod.yamlruns its test job in the same directory, the next pipeline run would reinstall them anyway — but manual on-server testing between deploys would break. Worth deciding deliberately rather than folding into this change by default.Caveats
npm cideletesnode_modulesbefore installing, so it is slower than a warmnpm installon the self-hosted boxes. The reproducibility is the point, but the added minute per deploy is real.npm cihard-requirespackage-lock.jsonto be present and in sync. That is the desired behavior, but it means a future PR that editspackage.jsonwithout regenerating the lock will now fail CD instead of silently self-correcting.Context
Found during static review of #128. Related to #124, which covers the other reproducibility gap in these same CD workflows (actions pinned to
@masterrather than a release tag).