ci(lint): fix the intermittent ESLint worker OOM - #312
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 502bb460ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Heads-up on the merge just pushed: master picked up
So Worth a look from whoever added it in case there was a constraint I am not seeing — full numbers are in CODE-468. |
The runbook still pinned --concurrency=2 and cited 2-vCPU runners; CI has since moved to lint:ci with concurrency off and a bigger heap (#312).
The runbook still pinned --concurrency=2 and cited 2-vCPU runners; CI has since moved to lint:ci with concurrency off and a bigger heap (#312).
Closes CODE-463's blocker. Fixes CODE-468.
Why CI kept dying
pnpm lintrunseslint --concurrency=auto.autoresolves toos.availableParallelism() >> 1, and ESLint creates those workers with noresourceLimits, so each inherits the main thread's heap ceiling. On a public-repoubuntu-latest(4 vCPU / 16 GB) that is exactly 2 workers, each capped at the workflow's--max-old-space-size=4096.Every worker builds its own typescript-eslint project-service programs, so the type graph is duplicated per worker rather than split across them. Two workers each need more than 4 GB, and the job sat just under that cliff — which is why it was intermittent (12 failures / 2 successes since 09:00, including a cargo-only bump that passed while its neighbours failed) rather than a clean break. It stayed green on dev machines only because a high-core box resolves
autoto many more workers, each with a smaller share.Measured on this repo
--concurrency--max-old-space-sizeauto)† died mid-run. The CI failure reproduces locally at
--concurrency=2.Lowering concurrency is not the fix and raising it does not fit: total RSS scales with worker count because each worker re-materializes the programs, so 16 GB cannot host more than 2 workers, while 2 workers each need more ceiling than they have. Single-threaded also uses less total CPU here (78 s user vs 107–158 s for 2 workers) — the duplicated program building outweighs the parallelism. This matches typescript-eslint's own guidance to disable concurrency for typed linting and lean on
--cacheinstead.What changed
lint:ci— a new script,eslint --concurrency=off --cache --cache-strategy content.pnpm lint/lint:fixkeep--concurrency=auto, so local behavior is untouched; only the workflow's Lint step switches.--cache-strategy contentis load-bearing. The defaultmetadatastrategy keys on mtime+size, and a fresh checkout rewrites every mtime, so it would score a 0 % hit rate in CI. Verified: aftertouch-ing every.ts/.tsxin the repo, a warm run still finished in 2.25 s.actions/cacheon.eslintcache, keyed per run with arestore-keysprefix so each run restores the newest prior entry and writes a fresh one.NODE_OPTIONS4096 → 6144. Still required: a cold cache walks the whole repo, and single-threaded at 4096 OOMs. It is a ceiling, not an allocation, so other jobs are unaffected in practice.Cold vs warm, both at the CI ceiling: 53.7 s / 6.2 GB → 2.4 s / 0.44 GB, same 344 findings.
Cache correctness
ESLint invalidates an entry when the file's content hash changes or when its resolved config hash changes — and that config hash folds in the ESLint and Node versions — so the cache key deliberately doesn't try to encode any of that. Verified by adding a
debugger;to a cached file: the cached run reported 345 problems / 1 error, matching an uncached control run of the same file.One real limitation to be aware of: the cache tracks each file's own content, not cross-file type dependencies. With type-aware rules, editing file A can change what a rule should report in an unchanged file B, and B's entry stays valid.
pnpm typecheckruns uncached in the same job, so genuine type breakage is still caught; what could be missed is a type-derived lint finding in a file nobody touched. Worth revisiting if that ever bites.Second, unrelated breakage in the same job
With Lint fixed, the job runs far enough to reach
Test, which then failed onpackages/host/assets— the committed pi closure manifest no longer matched the lockfile afterchore(deps): bump ws from 7.5.11 to 8.21.1 (#289). Lint was failing first and short-circuiting the job, so this never surfaced on CI. Regenerated viapnpm -F @linkcode/assets run generate:pi-closure(second commit); the real diff is three nestedws@8.21.0entries.Verification
pnpm check:ciexit 0 andpnpm test2056 passed / 5 skipped, both after the regeneration. The cache behavior above was driven directly rather than inferred.