fix: reduce Cypress CI memory pressure with larger runner#2036
fix: reduce Cypress CI memory pressure with larger runner#2036nick-inkeep merged 1 commit intomainfrom
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This PR makes well-reasoned changes to address Cypress CI memory pressure. The approach is sound: upgrading to a larger runner, reducing V8 heap limits to appropriate levels, and disabling video recording to reduce memory overhead. The code cleanup (removing dead video upload step and after:spec handler) is consistent with the configuration changes.
💭 Consider (2) 💭
💭 1) .github/workflows/cypress.yml Verify ubuntu-16gb runner is configured in org settings
Issue: The ubuntu-16gb runner label is used for the first time in this repository. While this is a valid GitHub-hosted larger runner label format, larger runners require GitHub Team or Enterprise plan and explicit configuration in org/repo settings.
Why: If the label is misconfigured, the workflow will queue indefinitely waiting for a runner that doesn't exist. The PR test plan already notes this verification step — just ensuring it's on the radar.
Refs:
💭 2) .github/workflows/cypress.yml Consider adding timeout-minutes for the job
Issue: The Cypress workflow job lacks a timeout-minutes setting. E2E tests can hang indefinitely due to browser crashes, network issues, or test flakiness.
Why: Without a timeout, the job could consume runner minutes until GitHub's 6-hour default timeout. Given this PR specifically addresses OOM crash issues, adding an explicit timeout (e.g., timeout-minutes: 30) would provide a safety net if crashes still occur.
Fix: Add timeout-minutes: 30 to the job definition:
jobs:
ci:
runs-on: ubuntu-16gb
timeout-minutes: 30Refs:
✅ APPROVE
Summary: Clean, well-reasoned fix for CI memory pressure. The changes are internally consistent (video disabled → video upload removed → cleanup handler removed) and the memory optimizations are appropriate for the new runner size. The two "Consider" items are optional hardening suggestions rather than blocking concerns. Ship it! 🚀
Discarded (5)
| Location | Issue | Reason Discarded |
|---|---|---|
.github/workflows/cypress.yml |
Missing explicit permissions: block |
Pre-existing pattern — ci.yml and other workflows also lack this. Consistent with repo conventions. |
.github/workflows/cypress.yml:52 |
Actions pinned to mutable tags instead of SHAs | Pre-existing pattern across all workflows in repo. Not introduced by this PR. |
cypress.config.ts:35 |
V8 heap limit reduced to 2048MB | Positive observation — this is the intended fix, not an issue. |
cypress.config.ts:23 |
Video disabled removes debugging aid | Informational — PR accounts for this (screenshots still captured). Reasonable tradeoff. |
action.yml:90-96 |
Video upload step removal | Positive observation — correct cleanup of dead code. |
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
7 | 0 | 2 | 0 | 0 | 0 | 5 |
| Total | 7 | 0 | 2 | 0 | 0 | 0 | 5 |
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This is a delta re-review scoped to changes since the last automated review. The delta (commit c0594749) refines the initial Cypress memory optimization approach by:
- Re-enabled video recording (
video: true) — preserves debugging capability for failed tests - Restored video upload step — ensures failure videos are captured as artifacts
- Restored
after:speccleanup handler — follows Cypress best practice to delete videos for passing tests, reducing artifact size - Removed aggressive Chrome flags (
--disable-dev-shm-usage,--disable-gpu,--js-flags=--max-old-space-size=2048) — these are unnecessary with the larger runner and were potentially causing issues rather than solving them
The delta changes are well-reasoned: with the larger runner (ubuntu-16gb), the aggressive memory-saving flags become unnecessary, and reverting to a cleaner configuration with video recording enabled provides better debugging capability without sacrificing stability.
🕐 Pending Recommendations (2)
The following items from the prior review remain open as optional suggestions:
- 💭
.github/workflows/cypress.yml:18Verifyubuntu-16gbrunner is configured in org settings - 💭
.github/workflows/cypress.ymlConsider addingtimeout-minutesfor the job
✅ APPROVE
Summary: Clean refinement to the Cypress CI configuration. The delta changes correctly restore video recording and cleanup handlers while removing unnecessary Chrome flags that were overcomplicating the solution. The core fix (larger runner + CYPRESS_NO_COMMAND_LOG=1) remains intact. Ship it! 🚀
Discarded (1)
| Location | Issue | Reason Discarded |
|---|---|---|
.github/composite-actions/cypress-e2e/action.yml:99 |
Actions pinned to mutable tags (v4) instead of SHAs |
Pre-existing pattern across all workflows in repo. Consistent with repo conventions. |
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
1 | 0 | 0 | 0 | 0 | 0 | 1 |
| Total | 1 | 0 | 0 | 0 | 0 | 0 | 1 |
Note: Delta re-review with minimal scope. Single reviewer dispatched for CI/infrastructure changes.
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This is a delta re-review scoped to changes since commit c0594749 (1 commit: cd5b54e4 "perf: speed up Cypress CI pipeline").
The delta introduces well-reasoned performance optimizations to the Cypress E2E infrastructure:
timeout-minutes: 30added — Addresses the prior review's "Consider" suggestion for explicit job timeout protection- Simplified dependency install — Removed redundant
cd agents-manage-ui && pnpm installstep; singlepnpm install --frozen-lockfileat root is sufficient - Explicit setup steps — Split monolithic
pnpm setup-devintopnpm db:migrate+pnpm db:auth:init, improving visibility into CI step failures pnpm exec cypress install— Changed fromnpxtopnpm execfor consistency with the monorepo's package manager- Chrome performance flags — Added standard headless optimization flags (
--disable-extensions,--disable-background-networking,--metrics-recording-only,--no-first-run,--mute-audio, etc.) - Cypress performance settings — Added
waitForAnimations: falseandscrollBehavior: falseto speed up test execution - Removed
cy.wait(500)anti-pattern — Eliminated arbitrary wait in login command, following Cypress best practices --no-runner-uiflag — Disables the runner UI in headless mode for reduced overhead
All changes are internally consistent and follow CI/Cypress best practices. The prior review's two "Consider" suggestions have been addressed.
✅ APPROVE
Summary: Clean set of performance optimizations for the Cypress CI pipeline. The delta changes address prior feedback and introduce no new concerns. The split of setup-dev into explicit steps improves debuggability, the Chrome flags are standard Puppeteer recommendations, and removing cy.wait(500) follows Cypress best practices. Ship it! 🚀
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: Delta re-review with minimal scope. Single reviewer dispatched for CI/infrastructure changes. Prior review suggestions have been addressed in this commit.
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This is a delta re-review scoped to changes since commit cd5b54e4 (1 commit: caf3d96b "fix: remove --disable-background-networking and redundant Chrome flags").
The delta further simplifies the Chrome launch configuration by removing 5 flags that are redundant in headless CI:
| Removed Flag | Reason Redundant |
|---|---|
--disable-background-networking |
Chrome already minimizes networking in headless mode |
--disable-default-apps |
No default apps load in headless CI |
--disable-sync |
Google account sync is not active in headless mode |
--metrics-recording-only |
Telemetry flag with no meaningful impact in headless CI |
--no-first-run |
First-run dialogs don't appear in headless mode |
The remaining flags (--no-sandbox, --disable-features=IsolateOrigins,site-per-process, --disable-extensions, --disable-translate, --mute-audio) represent an appropriate minimal set for stable headless Chrome in CI:
--no-sandbox— Required for running Chrome as root in CI containers--disable-features=IsolateOrigins,site-per-process— Memory optimization that reduces process count--disable-extensions/--disable-translate/--mute-audio— Standard cleanup flags
✅ APPROVE
Summary: Clean delta that removes genuinely redundant Chrome flags. The PR has evolved through 4 commits to a well-reasoned final state: larger runner (ubuntu-16gb) with a minimal Chrome flag set. Prior review feedback (timeout-minutes, runner verification) has been addressed. Ready to ship! 🚀
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: Minimal delta re-review. Single reviewer dispatched for CI/infrastructure changes. The removed flags are validated as redundant for headless Chrome in CI.
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This is a delta re-review scoped to changes since commit caf3d96b. The delta consists of a single commit:
e01c4307 ci: re-trigger Cypress workflow after Actions budget increase
This is an empty commit with zero code changes — it exists solely to re-trigger the Cypress CI workflow after the GitHub Actions budget was increased.
No new code to review.
The PR's substantive changes were reviewed and approved across 4 prior review cycles. All "Consider" suggestions from earlier reviews have been addressed:
- ✅
timeout-minutes: 30added (commitcd5b54e4) - ✅ Runner label verification noted in test plan
✅ APPROVE
Summary: Empty commit to re-trigger CI — no code changes to review. The PR remains well-structured and ready to ship. 🚀
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| — | — | — | — | — | — | — | — |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: No reviewers dispatched — delta contains zero code changes (empty CI trigger commit).
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This is a delta re-review scoped to changes since commit e01c4307. The delta consists of a single commit:
26644379 ci: re-trigger after budget propagation
This is an empty commit with zero code changes — it exists solely to re-trigger the Cypress CI workflow after the GitHub Actions budget was propagated.
No new code to review.
The PR's substantive changes were reviewed and approved across 5 prior review cycles. All "Consider" suggestions from earlier reviews have been addressed:
- ✅
timeout-minutes: 30added (commitcd5b54e4) - ✅ Runner label verification noted in test plan
✅ APPROVE
Summary: Empty commit to re-trigger CI — no code changes to review. The PR remains well-structured and ready to ship. 🚀
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| — | — | — | — | — | — | — | — |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: No reviewers dispatched — delta contains zero code changes (empty CI trigger commit).
nick-inkeep
force-pushed
the
fix/cypress-ci-memory
branch
from
48f1185 to
07d1192
Compare
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This PR improves Cypress E2E test reliability in CI through a well-reasoned set of optimizations:
timeout-minutes: 30— Provides a safety net for hung tests (addresses prior "Consider" feedback)- Simplified dependency install — Single
pnpm install --frozen-lockfileinstead of redundant cd step pnpm execfor Cypress — Consistent with monorepo package manager conventions- Minimal Chrome flags — Clean set (
--no-sandbox,--disable-features=IsolateOrigins,site-per-process,--disable-extensions,--disable-translate,--mute-audio) replacing aggressive memory-saving flags CYPRESS_NO_COMMAND_LOG=1— Reduces DOM snapshot accumulation in headless CIwaitForAnimations: false+--no-runner-ui— Speeds up test execution- Removed
cy.wait(500)anti-pattern — Follows Cypress best practices
Notable Observations
- PR description is outdated — mentions disabling video (
video: false), but video is enabled (video: true) with anafter:spechandler to delete videos for passing tests (which is the Cypress-recommended approach) - Runner temporarily rolled back —
ubuntu-16gbmentioned in description, but code usesubuntu-latestwith TODO comment pending enterprise billing propagation
The changes have been iteratively refined through 12 prior review cycles, all approving. All prior "Consider" suggestions have been addressed in the current code.
✅ APPROVE
Summary: Clean, well-reasoned CI/DevOps improvements for Cypress E2E test reliability. The changes are internally consistent, follow best practices, and have been thoroughly reviewed. The only outstanding item is the runner upgrade (blocked on enterprise billing), which is appropriately documented with a TODO comment. Ship it and monitor for OOM crashes — if they persist on ubuntu-latest, prioritize getting the billing sorted for ubuntu-16gb. 🚀
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: Single reviewer dispatched for CI/infrastructure changes. Zero findings returned — the PR is clean and has been well-refined through prior iterations.
nick-inkeep
force-pushed
the
fix/cypress-ci-memory
branch
from
07d1192 to
ef60b76
Compare
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This PR optimizes Cypress E2E test reliability and performance in CI through a well-reasoned set of changes:
- Runner upgrade —
ubuntu-16gbprovides 16GB RAM (vs ~7GB onubuntu-latest), giving Chrome processes sufficient memory headroom timeout-minutes: 30— Explicit safety net for hung tests (previously defaulted to GitHub's 6-hour limit)- Simplified dependency install — Single
pnpm install --frozen-lockfileinstead of redundant cd/install step pnpm execfor Cypress — Consistent with monorepo package manager conventions- Chrome flags rationalized — Removed cargo-cult flags (
--disable-dev-shm-usage,--disable-gpu,--js-flags) in favor of a minimal effective set (--no-sandbox,--disable-features=IsolateOrigins,site-per-process,--disable-extensions,--disable-translate,--mute-audio) CYPRESS_NO_COMMAND_LOG=1— Prevents DOM snapshot accumulation in headless CI (significant memory savings)waitForAnimations: false— Speeds up test execution by skipping CSS animation waits--no-runner-uiflag — Reduces overhead in headless mode- Removed
cy.wait(500)anti-pattern — Follows Cypress best practices (arbitrary waits are flaky)
Notable Observations
ubuntu-16gbis unique to this workflow — all other CI workflows useubuntu-latest. This is appropriate given the memory-intensive nature of browser-based E2E testing.- PR description is slightly outdated — mentions
video: false, but the actual code usesvideo: truewith anafter:spechandler that deletes videos for passing tests (the Cypress-recommended approach). - 13 prior automated reviews — All approved. Prior "Consider" suggestions (
timeout-minutes, runner verification) have been addressed.
✅ APPROVE
Summary: Clean, well-reasoned CI/DevOps improvements for Cypress E2E test reliability. The changes are internally consistent, follow best practices, and have been thoroughly reviewed. The ubuntu-16gb runner is justified for memory-intensive browser testing, and the Chrome flag rationalization removes unnecessary cargo-cult settings. Ship it! 🚀
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: Single reviewer dispatched for pure CI/infrastructure changes. Zero findings returned — the PR is clean and has been well-refined through 13 prior review iterations.
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This is a delta re-review scoped to changes since commit ef60b76b. The delta consists of a single commit:
04fbe57a ci: trigger with ubuntu-16gb runner
This is an empty commit with zero code changes — it exists solely to re-trigger the Cypress CI workflow now that the ubuntu-16gb runner label has been configured in the GitHub org settings.
No new code to review.
The PR's substantive changes were reviewed and approved across 14 prior review cycles. All prior "Consider" suggestions have been addressed:
- ✅
timeout-minutes: 30added - ✅ Runner label (
ubuntu-16gb) now configured and active
✅ APPROVE
Summary: Empty commit to re-trigger CI with the larger runner — no code changes to review. The PR is well-refined through extensive prior iterations and ready to ship. The ubuntu-16gb runner should provide the memory headroom needed to eliminate Chrome OOM crashes. 🚀
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| — | — | — | — | — | — | — | — |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: No reviewers dispatched — delta contains zero code changes (empty CI trigger commit).
- Add experimentalMemoryManagement, numTestsKeptInMemory: 0, waitForAnimations: false - Replace ineffective Chrome flags (--disable-dev-shm-usage, --disable-gpu, --js-flags=--max-old-space-size=4096) with targeted ones (--disable-extensions, --disable-translate, --mute-audio) - Add --no-runner-ui to headless Cypress runs to reduce memory overhead - Remove unnecessary cy.wait(500) from login command - Add timeout-minutes: 30 to workflow, CYPRESS_NO_COMMAND_LOG: 1 - Fix composite action: use pnpm exec instead of npx, add step name - Simplify duplicate pnpm install to single frozen-lockfile call Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
nick-inkeep
force-pushed
the
fix/cypress-ci-memory
branch
from
04fbe57 to
45d9cd9
Compare
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This PR optimizes the Cypress E2E CI workflow to address Chrome OOM crashes and improve test reliability. The changes are well-reasoned and follow CI/Cypress best practices:
- Runner upgrade —
ubuntu-16gbprovides 16GB RAM (vs ~7GB onubuntu-latest), giving Chrome processes sufficient memory headroom for browser-based E2E testing timeout-minutes: 30— Explicit safety net for hung tests (addresses prior "Consider" feedback)- Simplified dependency install — Single
pnpm install --frozen-lockfileinstead of redundant cd/install step pnpm execfor Cypress — Consistent with monorepo package manager conventions- Chrome flags rationalized — Removed cargo-cult flags (
--disable-dev-shm-usage,--disable-gpu,--js-flags) in favor of a minimal effective set (--no-sandbox,--disable-features=IsolateOrigins,site-per-process,--disable-extensions,--disable-translate,--mute-audio) CYPRESS_NO_COMMAND_LOG=1— Prevents DOM snapshot accumulation in headless CI (significant memory savings)waitForAnimations: false— Speeds up test execution by skipping CSS animation waits--no-runner-uiflag — Reduces overhead in headless mode- Removed
cy.wait(500)anti-pattern — Follows Cypress best practices (arbitrary waits are flaky)
Notable Observations
ubuntu-16gbis unique to this workflow — all other CI workflows useubuntu-latest. This is appropriate given the memory-intensive nature of browser-based E2E testing.- PR description is slightly outdated — mentions
video: false, but the actual code usesvideo: truewith anafter:spechandler that deletes videos for passing tests (the Cypress-recommended approach). - 15 prior automated reviews — All approved. Prior "Consider" suggestions (
timeout-minutes, runner verification) have been addressed in the current code.
✅ APPROVE
Summary: Clean, well-reasoned CI/DevOps improvements for Cypress E2E test reliability. The changes are internally consistent, follow best practices, and have been thoroughly refined through multiple iterations. The ubuntu-16gb runner is justified for memory-intensive browser testing, and the Chrome flag rationalization removes unnecessary cargo-cult settings. Ship it! 🚀
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: Single reviewer dispatched for pure CI/infrastructure changes. Zero findings returned — the PR is clean and has been well-refined through 15 prior review iterations.
- Add experimentalMemoryManagement, numTestsKeptInMemory: 0, waitForAnimations: false - Replace ineffective Chrome flags (--disable-dev-shm-usage, --disable-gpu, --js-flags=--max-old-space-size=4096) with targeted ones (--disable-extensions, --disable-translate, --mute-audio) - Add --no-runner-ui to headless Cypress runs to reduce memory overhead - Remove unnecessary cy.wait(500) from login command - Add timeout-minutes: 30 to workflow, CYPRESS_NO_COMMAND_LOG: 1 - Fix composite action: use pnpm exec instead of npx, add step name - Simplify duplicate pnpm install to single frozen-lockfile call Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
ubuntu-latest(2 vCPU / 7GB) toubuntu-16gb(4 vCPU / 16GB) to eliminate Chrome Renderer OOM crashes (30% failure rate)video: false) to save ~200MB memory overheadCYPRESS_NO_COMMAND_LOG=1to prevent DOM snapshot accumulation in headless CITest plan
ubuntu-16gbrunner label is configured in GitHub org settings🤖 Generated with Claude Code