fix(ci): limit Turbo concurrency to prevent browser test CPU starvation#2095
fix(ci): limit Turbo concurrency to prevent browser test CPU starvation#2095nick-inkeep merged 1 commit intomainfrom
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The ubuntu-16gb runner has 4 vCPUs but turbo.json sets concurrency to 15. During cache-miss runs, 15 concurrent tasks starve Playwright browser tests of CPU, causing Monaco editor initialization to time out. Limit CI concurrency to 8 via CLI flag (2x vCPUs), which prevents starvation while preserving parallelism. Uses --concurrency flag in CI only so local dev keeps the higher turbo.json setting. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
d744e20 to
627b4f6
Compare
There was a problem hiding this comment.
PR Review Summary
(1) Total Issues | Risk: Low
🔴❗ Critical (0) ❗🔴
None.
🟠⚠️ Major (0) 🟠⚠️
None.
🟡 Minor (0) 🟡
None.
💭 Consider (0) 💭
None.
🧹 While You're Here (1) 🧹
🧹 1) .github/workflows/ci.yml Actions pinned to mutable tags instead of SHAs
Issue: GitHub Actions in this workflow (and all other workflows in the repo) use mutable version tags (e.g., actions/checkout@v5, actions/cache@v4) rather than full SHA pins.
Why: While first-party actions/* are lower risk than third-party actions, SHA pinning provides stronger supply chain security guarantees. This is a pre-existing pattern across the repository, not introduced by this PR.
Fix: Consider a follow-up PR to adopt SHA pinning with Dependabot for automated updates, or document the decision to accept mutable tags for first-party actions.
Refs:
✅ APPROVE
Summary: This is a clean, well-justified CI fix. The change correctly limits Turbo concurrency to 8 (2x the 4 vCPU runner spec) to prevent CPU starvation during cache-miss runs that caused Playwright browser tests to time out. The inline comment clearly documents the rationale, and using the CLI --concurrency flag is the right approach — it applies the constraint only in CI while preserving the higher concurrency setting in turbo.json for local development on beefier machines. Ship it! 🚀
Discarded (2)
| Location | Issue | Reason Discarded |
|---|---|---|
.github/workflows/ci.yml:113-115 |
CI concurrency change is correct and well-justified | Positive feedback, not an issue |
.github/workflows/ci.yml:112-114 |
Comment accurately reflects runner spec and rationale | Positive feedback, not an issue |
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
3 | 0 | 0 | 1 | 0 | 0 | 2 |
| Total | 3 | 0 | 0 | 1 | 0 | 0 | 2 |
The ubuntu-16gb runner (4 vCPUs, 16GB RAM) gets OOM-killed when turbo concurrency=15 triggers multiple Next.js builds concurrently during cache-miss runs (e.g., any PR touching agents-core). This limits CI concurrency to 8 via CLI flag (turbo.json stays at 15 for local dev). Re-applies the fix from #2095 which was reverted in #2116. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
--concurrencyCLI flagubuntu-16gbrunner has 4 vCPUs; 15 concurrent tasks causes CPU starvation during cache-miss runs, which makes Playwright browser tests (Monaco editor initialization) time outturbo.jsonstays at 15 for local dev on beefier machinesWhy
--concurrency=8?What was ruled out
dependsOnoverrides — artificial coupling, maintenance burdenfileParallelism: false— unnecessary if Turbo concurrency is properly constrainedTest plan
🤖 Generated with Claude Code