Share one node_modules cache key so CI can actually restore it

.github/workflows/deploy-base.yml

@@ -81,12 +81,50 @@ jobs:
         with:
           node-version: '24'
 
+      # KEEP THIS KEY IDENTICAL to the `Cache node_modules` steps in
+      # test.yml and nightly-tests.yml. GHA scopes cache entries to the
+      # writing ref, and a `pull_request` run can only read its own
+      # `refs/pull/N/merge` scope plus the default branch's -- so the
+      # default branch is the only scope a cache can be shared through.
+      #
+      # It used to key this `deploy-node-modules-*`. Same directory, same
+      # lockfile hash, one prefix apart, so CI could never see what this
+      # workflow wrote: every PR missed, ran `npm ci`, and saved another
+      # ~145 MB copy into a scope only it could read. That pushed the store
+      # to 194 entries / 10.59 GB against GitHub's 10 GB cap, permanently
+      # evicting.
+      #
+      # nightly-tests.yml also writes this key on `main` (a `schedule`
+      # trigger runs on the default branch), so it is not the sole writer
+      # -- but it fires once a day, and its entry goes stale the moment the
+      # lockfile moves. This workflow runs on every merge, which is what
+      # keeps a current entry in the shared scope. Before the fix, main
+      # held exactly one `node-modules-*` entry, at a stale hash, from a
+      # nightly run.
+      #
+      # The path list and the `-v2` are the second half of the same bug.
+      # This is an npm *workspaces* repo and deps do not all hoist, so
+      # caching root `node_modules` alone restored a tree `npm ls` rejects
+      # -- the `Validate cached node_modules` gate then wiped it and
+      # reinstalled, on every hit. The cache had never saved a second even
+      # when it hit. `-v2` is there because the path set changed while the
+      # lockfile hash did not, and actions/cache will not overwrite an
+      # existing key; without the bump every job would keep restoring the
+      # old, incomplete entry. Keep the list in sync with package.json's
+      # `workspaces`.
+      #
+      # See BS#2256; pinned by
+      # tests/unit/scripts/ci-node-modules-cache.test.ts.
       - name: Cache node_modules
         if: inputs.target == ''
         uses: actions/cache@v6
         with:
-          path: node_modules
-          key: deploy-node-modules-${{ runner.os }}-node24-${{ hashFiles('package-lock.json') }}
+          path: |
+            node_modules
+            apps/*/node_modules
+            jobs/*/node_modules
+            shared/*/node_modules
+          key: node-modules-v2-${{ runner.os }}-node24-${{ hashFiles('package-lock.json') }}
 
       - name: Install dependencies
         if: inputs.target == ''

.github/workflows/nightly-tests.yml

@@ -31,8 +31,12 @@ jobs:
         id: cache-node-modules
         uses: actions/cache@v6
         with:
-          path: node_modules
-          key: node-modules-${{ runner.os }}-node24-${{ hashFiles('package-lock.json') }}
+          path: |
+            node_modules
+            apps/*/node_modules
+            jobs/*/node_modules
+            shared/*/node_modules
+          key: node-modules-v2-${{ runner.os }}-node24-${{ hashFiles('package-lock.json') }}
 
       - name: Validate cached node_modules
         id: validate-cache

.github/workflows/test.yml

@@ -123,7 +123,19 @@ jobs:
               # each other. Without them in `src`, a PR that edits only one
               # of them skips `unit-tests` entirely — the guard for these
               # files never runs on a change to the exact files it checks.
-              - '.github/workflows/test.yml'
+              #
+              # BS#2256 widened this from `.github/workflows/test.yml` to the
+              # whole directory. `ci-node-modules-cache.test.ts` globs every
+              # workflow file, so the same trap had reopened one file over:
+              # a PR reverting `deploy-base.yml`'s cache key — the literal
+              # defect that issue fixed — touched no `src` path, so
+              # `unit-tests` never ran and the required check reported
+              # `skipped`, which branch protection accepts as passing.
+              #
+              # Keep this a directory glob. Any guard that reads a workflow
+              # file needs that file to trigger the job the guard runs in,
+              # and enumerating them one at a time has now failed twice.
+              - '.github/workflows/**'
               - 'dev_env/docker-compose.yml'
 
       - name: Determine if integration tests should run
@@ -157,8 +169,12 @@ jobs:
         id: cache-node-modules
         uses: actions/cache@v6
         with:
-          path: node_modules
-          key: node-modules-${{ runner.os }}-node24-${{ hashFiles('package-lock.json') }}
+          path: |
+            node_modules
+            apps/*/node_modules
+            jobs/*/node_modules
+            shared/*/node_modules
+          key: node-modules-v2-${{ runner.os }}-node24-${{ hashFiles('package-lock.json') }}
 
       - name: Validate cached node_modules
         id: validate-cache
@@ -291,8 +307,12 @@ jobs:
         id: cache-node-modules
         uses: actions/cache@v6
         with:
-          path: node_modules
-          key: node-modules-${{ runner.os }}-node24-${{ hashFiles('package-lock.json') }}
+          path: |
+            node_modules
+            apps/*/node_modules
+            jobs/*/node_modules
+            shared/*/node_modules
+          key: node-modules-v2-${{ runner.os }}-node24-${{ hashFiles('package-lock.json') }}
 
       - name: Validate cached node_modules
         id: validate-cache
@@ -396,8 +416,12 @@ jobs:
         id: cache-node-modules
         uses: actions/cache@v6
         with:
-          path: node_modules
-          key: node-modules-${{ runner.os }}-node24-${{ hashFiles('package-lock.json') }}
+          path: |
+            node_modules
+            apps/*/node_modules
+            jobs/*/node_modules
+            shared/*/node_modules
+          key: node-modules-v2-${{ runner.os }}-node24-${{ hashFiles('package-lock.json') }}
 
       - name: Validate cached node_modules
         id: validate-cache

docs/deploy.md

@@ -40,6 +40,12 @@ The `npm ci` layer still busts on any `package-lock.json` change — exactly the
 
 **ECR storage**: the `:buildcache` tag holds the live cache manifest per repo; `.github/ecr-lifecycle-policy.json` (applied via `Apply ECR Lifecycle Policy`, run every `build` invocation so a newly-created repository picks it up on its first build) expires untagged images (the dangling layers left behind each time `:buildcache` is retagged) after 1 day and caps `sha-*` tags at the 20 most recent per repo. The `buildcache` tag itself is never matched by either rule, since it's always tagged and never has the `sha-` prefix.
 
+## Dependency caching (`node_modules`)
+
+<!-- @rule id=shared-node-modules-cache-key enforced-by=tests/unit/scripts/ci-node-modules-cache.test.ts added=2026-08-23 incidents=#2256 -->
+
+**Every `node_modules` cache step in every workflow must use the identical key and the identical path list** — the list covering the workspace `node_modules` directories (`apps/*`, `jobs/*`, `shared/*`), not just the root one. GHA scopes an entry to the ref that wrote it, so only a default-branch run can fill the scope PRs restore from; and deps don't all hoist here, so a root-only tree fails the `Validate cached node_modules` gate and gets reinstalled anyway. Bump the key's `-vN` whenever the path list changes — the lockfile hash won't move, and `actions/cache` never overwrites an existing key. `tests/unit/scripts/ci-node-modules-cache.test.ts` enforces all of this and carries the full history (#2256).
+
 ## Host disk reclamation (pre-pull GC)
 
 <!-- @rule id=reclaim-before-pull enforced-by=none added=2026-07-27 incidents=#run-30313671442 -->

tests/unit/scripts/ci-node-modules-cache.test.ts

@@ -0,0 +1,230 @@
+/**
+ * Pin the GitHub Actions `node_modules` cache to a single, restorable key
+ * (BS#2256).
+ *
+ * # The failure this guards against
+ *
+ * GHA scopes every cache entry to the ref that wrote it. A `pull_request`
+ * run writes into `refs/pull/<N>/merge`, which only that one PR can read.
+ * The one scope every PR *can* read is the default branch's -- so the
+ * default branch is the only scope a cache can be shared through.
+ *
+ * `test.yml` triggers only on `pull_request` and keyed its cache
+ * `node-modules-*`. `deploy-base.yml` runs on push to `main` but keyed the
+ * same directory, built from the same lockfile hash, as
+ * `deploy-node-modules-*`. One prefix apart, so CI never restored what the
+ * merge had just written: each PR missed, ran `npm ci`, and saved another
+ * ~145 MB copy into a scope only it could read.
+ *
+ * Measured 2026-08-23 before the fix: 194 entries / 10.59 GB against
+ * GitHub's 10 GB per-repo cap, so the store was permanently evicting.
+ * 49 of the 50 `node-modules-*` entries were PR-scoped (6.93 GB); one
+ * lockfile hash had TEN identical copies, one per PR. `npm ci` ran on
+ * 13 of 13 sampled branches, 25-34s each.
+ *
+ * `nightly-tests.yml` also writes this key on `main` -- a `schedule`
+ * trigger runs on the default branch -- so it was never true that nothing
+ * could populate the shared scope. But nightly fires once a day and its
+ * entry goes stale as soon as the lockfile moves, which is exactly what
+ * had happened: main held a single `node-modules-*` entry, at a stale
+ * hash. The per-merge writer is what keeps a current entry there.
+ *
+ * # Why this file is a `readFileSync` test, and what that costs
+ *
+ * It reads workflow YAML as text, so Jest's dependency graph has no edge
+ * from these workflows to this spec. That is fine now -- BS#2255 moved CI
+ * to the full unit suite -- but it does mean the job running this spec has
+ * to be *triggered* by a change to the files it reads. `detect-changes`'s
+ * `src` filter therefore globs `.github/workflows/**`; narrowing it back
+ * to individual files re-opens the hole (BS#1807, then BS#2256).
+ */
+
+import * as fs from 'fs';
+import * as path from 'path';
+
+const repoRoot = path.resolve(__dirname, '../../..');
+const workflowDir = path.join(repoRoot, '.github/workflows');
+
+/**
+ * The single key every `node_modules` cache step must use. Keep the
+ * `runner.os` and lockfile-hash components -- they are what make the entry
+ * safe to share across workflows.
+ */
+const CANONICAL_KEY = "node-modules-v2-${{ runner.os }}-node24-${{ hashFiles('package-lock.json') }}";
+
+/**
+ * Every path a `node_modules` cache step must cover. This is an npm
+ * *workspaces* monorepo (`apps/*`, `jobs/*`, `shared/*`) and dependencies
+ * do not all hoist -- `apps/backend`, `apps/auth` and
+ * `shared/authentication` each carry their own `node_modules`.
+ *
+ * Caching root `node_modules` alone therefore restored a tree that
+ * `npm ls --depth=0` rejects (the workspaces' own deps are missing, so
+ * root's hoisted versions read as `invalid`), which trips the
+ * `Validate cached node_modules` gate, wipes the directory, and reinstalls.
+ * Every cache hit did that, so the cache never saved a second even when it
+ * hit. Keep this list in sync with `package.json`'s `workspaces`.
+ */
+const CANONICAL_PATHS = ['node_modules', 'apps/*/node_modules', 'jobs/*/node_modules', 'shared/*/node_modules'];
+
+/**
+ * Exact number of steps expected to cache `node_modules`, as of BS#2256:
+ * three jobs in test.yml, plus deploy-base.yml and nightly-tests.yml.
+ *
+ * Pinned exactly, not as a floor. A floor cannot catch a step dropping OUT
+ * of the checked set -- rewriting one `path: node_modules` as a block list
+ * to add a second directory, say -- which would silently exempt it from
+ * every assertion below while they all kept passing. Changing this number
+ * should be a deliberate edit with a look at the key.
+ */
+const EXPECTED_NODE_MODULES_CACHE_STEPS = 5;
+
+interface Step {
+  file: string;
+  text: string;
+}
+
+function workflowFiles(): string[] {
+  return fs
+    .readdirSync(workflowDir)
+    .filter((f) => f.endsWith('.yml') || f.endsWith('.yaml'))
+    .sort();
+}
+
+/**
+ * Drop comment-only lines. Every check here must read YAML, not prose --
+ * the first cut of this guard tripped on its own explanatory comments,
+ * which quote the retired `deploy-node-modules-*` prefix verbatim.
+ */
+function withoutComments(text: string): string {
+  return text
+    .split('\n')
+    .filter((line) => !/^\s*#/.test(line))
+    .join('\n');
+}
+
+/** Split a workflow into step-sized chunks. A step starts at `- <key>:`. */
+function stepsOf(file: string): Step[] {
+  const lines = fs.readFileSync(path.join(workflowDir, file), 'utf-8').split('\n');
+  const starts: number[] = [];
+  lines.forEach((line, i) => {
+    if (/^\s+- (name|uses|id|if):/.test(line)) starts.push(i);
+  });
+  return starts.map((start, i) => ({
+    file,
+    text: lines.slice(start, starts[i + 1] ?? lines.length).join('\n'),
+  }));
+}
+
+const allSteps: Step[] = workflowFiles().flatMap(stepsOf);
+
+/** The value of a scalar `with:` input, or undefined when absent/a block. */
+function scalar(step: Step, key: string): string | undefined {
+  const m = withoutComments(step.text).match(new RegExp(`^\\s*${key}:[ \\t]*(\\S.*?)\\s*$`, 'm'));
+  return m?.[1];
+}
+
+/**
+ * Every path a cache step covers, handling both `path: node_modules` and
+ * the block form:
+ *
+ *   path: |
+ *     node_modules
+ *     .cache
+ */
+function cachedPaths(step: Step): string[] {
+  const lines = withoutComments(step.text).split('\n');
+  const i = lines.findIndex((l) => /^\s*path:/.test(l));
+  if (i === -1) return [];
+  const head = lines[i].replace(/^\s*path:[ \t]*/, '').trim();
+  if (head && head !== '|' && head !== '|-' && head !== '>') return [head];
+
+  const indent = (lines[i].match(/^\s*/) ?? [''])[0].length;
+  const out: string[] = [];
+  for (const line of lines.slice(i + 1)) {
+    if (!line.trim()) continue;
+    if ((line.match(/^\s*/) ?? [''])[0].length <= indent) break;
+    out.push(line.trim().replace(/^-\s*/, ''));
+  }
+  return out;
+}
+
+const cacheSteps = allSteps.filter((s) => /uses: actions\/cache@/.test(s.text));
+const nodeModulesCacheSteps = cacheSteps.filter((s) => cachedPaths(s).includes('node_modules'));
+
+describe('the node_modules cache is one shared, restorable key (BS#2256)', () => {
+  it('finds every node_modules cache step', () => {
+    // If the parser silently matches nothing (or stops matching a step
+    // that was reworded), every assertion below passes vacuously -- the
+    // exact shape of failure BS#2249 was about.
+    expect(cacheSteps.length).toBeGreaterThanOrEqual(EXPECTED_NODE_MODULES_CACHE_STEPS);
+    expect(nodeModulesCacheSteps.map((s) => s.file).sort()).toEqual([
+      'deploy-base.yml',
+      'nightly-tests.yml',
+      'test.yml',
+      'test.yml',
+      'test.yml',
+    ]);
+    expect(nodeModulesCacheSteps).toHaveLength(EXPECTED_NODE_MODULES_CACHE_STEPS);
+  });
+
+  it('caches node_modules from a workflow that runs on the default branch', () => {
+    // The whole point is cross-PR sharing, and only a default-branch run
+    // can write the scope PRs read from. `deploy-base.yml` (push to main)
+    // is the per-merge writer; `nightly-tests.yml` (schedule) is the daily
+    // one. If neither still caches node_modules, the shared key is
+    // unreachable again and this guard is proving nothing.
+    const files = new Set(nodeModulesCacheSteps.map((s) => s.file));
+    expect([...files].some((f) => f === 'deploy-base.yml' || f === 'nightly-tests.yml')).toBe(true);
+    expect(files.has('deploy-base.yml')).toBe(true);
+  });
+
+  it('caches the workspace node_modules directories too', () => {
+    // A partial tree under this key is worse than no cache: it restores,
+    // fails `npm ls`, and reinstalls anyway -- paying the download for
+    // nothing. See CANONICAL_PATHS.
+    const offenders = nodeModulesCacheSteps
+      .filter((s) => JSON.stringify(cachedPaths(s)) !== JSON.stringify(CANONICAL_PATHS))
+      .map((s) => `${s.file}: ${JSON.stringify(cachedPaths(s))}`);
+    expect(offenders).toEqual([]);
+  });
+
+  it('uses one identical key everywhere', () => {
+    // Report file + key so a failure names the offender directly.
+    const offenders = nodeModulesCacheSteps
+      .filter((s) => scalar(s, 'key') !== CANONICAL_KEY)
+      .map((s) => `${s.file}: ${scalar(s, 'key')}`);
+    expect(offenders).toEqual([]);
+  });
+
+  it('never reintroduces a workflow-specific prefix', () => {
+    // `deploy-node-modules-*` was the original defect.
+    const offenders = workflowFiles().filter((f) =>
+      /deploy-node-modules-|test-node-modules-|nightly-node-modules-/.test(
+        withoutComments(fs.readFileSync(path.join(workflowDir, f), 'utf-8'))
+      )
+    );
+    expect(offenders).toEqual([]);
+  });
+
+  it('does not fall back to a stale tree via restore-keys', () => {
+    // A partial-match restore would hand a job a node_modules built from a
+    // different lockfile; exact-match plus `npm ci` on miss is the right
+    // trade.
+    for (const step of nodeModulesCacheSteps) {
+      expect(withoutComments(step.text)).not.toContain('restore-keys');
+    }
+  });
+});
+
+describe('the guard is reachable from the workflows it reads (BS#2256)', () => {
+  const testWorkflow = withoutComments(fs.readFileSync(path.join(workflowDir, 'test.yml'), 'utf-8'));
+
+  it("detect-changes' src filter globs the whole workflow directory", () => {
+    // Without this, a PR that edits only a workflow skips `unit-tests`,
+    // this spec never runs, and the required check reports `skipped` --
+    // which branch protection treats as passing. Enumerating individual
+    // workflow files here has already failed twice (BS#1807, BS#2256).
+    expect(testWorkflow).toContain("- '.github/workflows/**'");
+  });
+});