CenterForDigitalHumanities / CenterForDigitalHumanities/TinyNode
CD workflows use `npm install` instead of `npm ci`, so deploys can drift from the lockfile
- Dominant language
- JavaScript
- Stars
- 1
- Forks
- 3
- Avg merge
- 29m
- Merged PRs (30d)
- 1
Description
## 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:
```yaml
# .github/workflows/cd_dev.yaml:53-56
- name: Install dependencies and run the test
run: |
npm ci
npm run allTests
```
```yaml
# .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).
Contributor guide
Assessment
This issue has not been assessed yet.