Problem
Two related problems in .github/workflows/lint.yml.
1. Cache fallback restores a stale cache, hiding violations
The restore step falls back to a catch-all key:
restore-keys: |
${{ runner.os }}-eslint-${{ hashFiles(...) }}-
${{ runner.os }}-eslint- # any latest eslint cache, regardless of configWhen a lint-config/rule change lands (e.g. bumping eslint-config-expensify), the exact key misses but the catch-all restores a cache built under the old rules. ESLint only caches files that lint clean and only re-lints files whose content or config hash changed — plugin source inside node_modules is not part of that hash. So files that were clean under the old rule are never re-evaluated under the new rule, and new violations silently pass CI. Only a cold full lint catches them — the exact run that OOMs.
2. Cold full-repo lint runs out of memory
Whenever the first npm run lint fails for any reason, the retry logic clears the cache and re-runs cold:
if ! npm run lint; then
echo "Lint failed, clearing cache and retrying..."
rm -rf node_modules/.cache/eslint
npm run lint
fi
That cold, full-repo lint runs with --concurrency=auto, each allowed up to --max_old_space_size=8192. Combined demand exceeds runner RAM (~16GB) and a worker gets killed:
Error [ERR_WORKER_OUT_OF_MEMORY]: Worker terminated due to reaching memory limit: JS heap out of memory
##[error]Process completed with exit code 2.
This is pre-existing and independent of any single PR — it reproduces on unrelated commits whose only commonality is that the first lint pass failed. Examples:
All three: first cached lint fails → clearing cache and retrying... → cold lint → OOM.
Proposed fix
- Bust the cache only on lint config / lint dependency changes. Hash a lockfile slice containing only lint-affecting deps (anything matching
eslint, which covers eslint, all eslint-*/@typescript-eslint plugins, and the eslint-config-expensify github dep) plus eslint.config.mjs and config/eslint/**. Drop the catch-all ${{ runner.os }}-eslint- fallback so a config/dep change gets a clean cold lint instead of a stale cache.
- Lower cold-lint concurrency (
--concurrency=2 in scripts/lint.sh) so the full re-lint fits in runner memory (2 × 8GB ≈ 16GB). But we should verify how long the lint checks will take then.
Issue Owner
Current Issue Owner: @VickyStash
Problem
Two related problems in
.github/workflows/lint.yml.1. Cache fallback restores a stale cache, hiding violations
The restore step falls back to a catch-all key:
When a lint-config/rule change lands (e.g. bumping
eslint-config-expensify), the exact key misses but the catch-all restores a cache built under the old rules. ESLint only caches files that lint clean and only re-lints files whose content or config hash changed — plugin source insidenode_modulesis not part of that hash. So files that were clean under the old rule are never re-evaluated under the new rule, and new violations silently pass CI. Only a cold full lint catches them — the exact run that OOMs.2. Cold full-repo lint runs out of memory
Whenever the first
npm run lintfails for any reason, the retry logic clears the cache and re-runs cold:That cold, full-repo lint runs with
--concurrency=auto, each allowed up to--max_old_space_size=8192. Combined demand exceeds runner RAM (~16GB) and a worker gets killed:This is pre-existing and independent of any single PR — it reproduces on unrelated commits whose only commonality is that the first lint pass failed. Examples:
no-unsafe-type-assertionseatbelt)no-unused-vars/no-duplicates)consistent-type-imports)All three: first cached lint fails →
clearing cache and retrying...→ cold lint → OOM.Proposed fix
eslint, which coverseslint, alleslint-*/@typescript-eslintplugins, and theeslint-config-expensifygithub dep) pluseslint.config.mjsandconfig/eslint/**. Drop the catch-all${{ runner.os }}-eslint-fallback so a config/dep change gets a clean cold lint instead of a stale cache.--concurrency=2inscripts/lint.sh) so the full re-lint fits in runner memory (2 × 8GB ≈ 16GB). But we should verify how long the lint checks will take then.Issue Owner
Current Issue Owner: @VickyStash