diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 99c4369a80..055b5ace5e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -138,7 +138,7 @@ jobs: # runner's own persistence, and npm ci always deletes+reinstalls node_modules by design -- so # neither the runner nor npm ci gives node_modules any real cross-run reuse on its own. This # explicit restore/save pair (via GitHub's own cache service, not the wiped local disk) fills that - # gap: an exact package-lock.json match skips npm ci entirely. Keyed separately per fork/trusted + # gap: an exact manifest+lockfile match skips npm ci entirely. Keyed separately per fork/trusted # (see the runs-on expression above) because self-hosted's Docker image and GitHub's ubuntu-latest # image are not guaranteed binary-compatible for native modules (sharp, workerd, fsevents, ...) -- # crossing them could load an incompatible native binary. Fork PRs get read-only cache tokens (a @@ -152,10 +152,12 @@ jobs: path: | node_modules apps/gittensory-ui/node_modules - # hashFiles('.nvmrc') matters as much as the lockfile: a Node bump with no lockfile change - # would otherwise still hit and silently reuse node_modules whose native addons (sharp, - # workerd, fsevents) were compiled against the OLD Node's ABI. - key: npm-${{ (github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == true) && 'fork' || 'trusted' }}-${{ hashFiles('.nvmrc') }}-${{ hashFiles('package-lock.json') }} + # hashFiles('.nvmrc') matters as much as the package manifests and lockfile: a Node bump + # with no lockfile change would otherwise still hit and silently reuse node_modules whose + # native addons (sharp, workerd, fsevents) were compiled against the OLD Node's ABI. The + # manifests matter too because npm ci validates package.json/package-lock.json consistency + # and runs lifecycle scripts from package.json; a package.json-only change must not skip it. + key: npm-${{ (github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == true) && 'fork' || 'trusted' }}-${{ hashFiles('.nvmrc') }}-${{ hashFiles('package.json', 'apps/*/package.json', 'packages/*/package.json', 'package-lock.json') }} - name: Install dependencies (retry on transient failures) if: ${{ steps.node-modules-cache.outputs.cache-hit != 'true' }} run: | @@ -313,8 +315,9 @@ jobs: uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 with: path: review-enrichment/node_modules - # Same Node-version guard as the root cache key above -- REES runs on the same pinned .nvmrc. - key: npm-rees-${{ (github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == true) && 'fork' || 'trusted' }}-${{ hashFiles('.nvmrc') }}-${{ hashFiles('review-enrichment/package-lock.json') }} + # Same Node-version and manifest guards as the root cache key above -- REES runs on the same + # pinned .nvmrc and has its own package.json lifecycle/install validation. + key: npm-rees-${{ (github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == true) && 'fork' || 'trusted' }}-${{ hashFiles('.nvmrc') }}-${{ hashFiles('review-enrichment/package.json', 'review-enrichment/package-lock.json') }} - name: REES install if: ${{ (github.event_name == 'push' || needs.changes.outputs.rees == 'true') && steps.rees-node-modules-cache.outputs.cache-hit != 'true' }} run: npm run rees:install diff --git a/test/unit/ci-dependency-cache.test.ts b/test/unit/ci-dependency-cache.test.ts index fab1385b66..0da94bcb82 100644 --- a/test/unit/ci-dependency-cache.test.ts +++ b/test/unit/ci-dependency-cache.test.ts @@ -31,7 +31,7 @@ function jobSteps(workflow: Record, jobName: string): Array { @@ -43,7 +43,11 @@ describe("CI dependency-install caching", () => { const restoreWith = record(restore.with, "restore.with"); expect(String(restoreWith.path)).toContain("node_modules"); expect(String(restoreWith.path)).toContain("apps/gittensory-ui/node_modules"); - expect(String(restoreWith.key)).toContain("hashFiles('package-lock.json')"); + expect(String(restoreWith.key)).toContain("hashFiles('package.json', 'apps/*/package.json', 'packages/*/package.json', 'package-lock.json')"); + expect(String(restoreWith.key)).toContain("package.json"); + expect(String(restoreWith.key)).toContain("apps/*/package.json"); + expect(String(restoreWith.key)).toContain("packages/*/package.json"); + expect(String(restoreWith.key)).toContain("package-lock.json"); // A Node bump (.nvmrc) with no lockfile change must still bust the cache -- otherwise a hit would // silently reuse node_modules whose native addons were compiled against the OLD Node's ABI. expect(String(restoreWith.key)).toContain("hashFiles('.nvmrc')"); @@ -70,7 +74,9 @@ describe("CI dependency-install caching", () => { const restore = step(steps, "Restore review-enrichment node_modules cache"); const restoreWith = record(restore.with, "restore.with"); expect(restoreWith.path).toBe("review-enrichment/node_modules"); - expect(String(restoreWith.key)).toContain("hashFiles('review-enrichment/package-lock.json')"); + expect(String(restoreWith.key)).toContain("hashFiles('review-enrichment/package.json', 'review-enrichment/package-lock.json')"); + expect(String(restoreWith.key)).toContain("review-enrichment/package.json"); + expect(String(restoreWith.key)).toContain("review-enrichment/package-lock.json"); expect(String(restoreWith.key)).toContain("hashFiles('.nvmrc')"); const install = step(steps, "REES install");