perf(ci): remove redundant Next.js cache, exclude docs build, add .next/dev exclusion#2153
perf(ci): remove redundant Next.js cache, exclude docs build, add .next/dev exclusion#2153nick-inkeep merged 11 commits intomainfrom
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
nick-inkeep
force-pushed
the
ci/split-docs-build
branch
from
2dd5173 to
814bd1f
Compare
nick-inkeep
changed the title
The main CI workflow runs all turbo tasks in parallel (concurrency=15) on a 16GB runner. When turbo cache misses cluster, 9+ Node.js processes launch simultaneously — each with a 4GB heap limit — and the Linux OOM killer sends SIGKILL (exit code 137). The agents-docs Next.js build (335 static pages, 3.7min compile) is the heaviest contributor. Since agents-docs is a pure leaf node (nothing depends on it, no tests, deployed separately via Vercel), it can safely build in isolation: - Exclude @inkeep/agents-docs from main CI turbo check - Remove agents-docs build cache step from main CI - Add new ci-docs.yml workflow with path-based triggers - ci-docs builds on ubuntu-latest with 8GB heap (generous for isolation) - Turbo automatically builds transitive deps (agents-core, agents-cli) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
agents-cli lives at the repo root (agents-cli/), not under packages/. The path 'packages/agents-cli/src/**' would never match, meaning CLI source changes wouldn't trigger the docs CI. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The old command was `pnpm check` which resolves turbo via the package scripts. The bare `turbo` command isn't on PATH in CI runners — need `pnpm exec turbo` to resolve the local devDependency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addresses review feedback: changes to ci-docs.yml itself now trigger a validation run, making the workflow self-validating rather than relying solely on the weekly cron for workflow file changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ci-docs.yml now runs only lint+typecheck for agents-docs. Vercel preview already runs `next build` on every PR, so running it again in GHA was duplicate work. This cuts the ci-docs job from ~4min to ~1min and removes the Next.js build cache step (no longer needed). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The separate ci-docs.yml was created to isolate the heavy Next.js SSG build. With build now delegated to Vercel preview, only lint and typecheck remain — lightweight enough to run in the main CI job. The agents-docs build filter stays to prevent OOM. A new step runs lint+typecheck for agents-docs after the main check completes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…bo outputs - Remove `actions/cache` for `.next/cache` in CI — redundant when Turborepo manages builds. Turbo cache hits replay all outputs (skipping `next build` entirely); on cache misses the stale compiler cache provides minimal benefit. The official Next.js docs recommendation is stale/generic and hasn't been updated for Turborepo users (confirmed via Turborepo CI docs). - Add `!.next/dev/**` to turbo.json build outputs — Next.js 16+ stores Turbopack's dev-time filesystem cache in `.next/dev/` (can grow to 2-3+ GB). Without excluding it, this massive dev cache gets captured into Turborepo's cache artifacts, bloating remote cache and slowing CI restores. (ref: vercel/turborepo#11212) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
nick-inkeep
force-pushed
the
ci/split-docs-build
branch
from
7918e0c to
4a115ad
Compare
![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
💭 Consider (2) 💭
💭 1) scope: workflow consistency Remove redundant Next.js cache step from cypress.yml for consistency
Issue: The PR removes the "Setup Next.js build cache" step from ci.yml but leaves an identical step in cypress.yml (lines 90-97). Both workflows use Turborepo for caching.
Why: The PR description's rationale (Turborepo makes .next/cache GitHub Actions caching redundant) applies equally to both workflows. Leaving it in one but not the other creates maintenance confusion.
Fix: Consider removing lines 90-97 from cypress.yml in a follow-up PR for consistency. This is harmless as-is (redundant caching doesn't break anything), so it's fine to address separately.
Refs:
- cypress.yml:90-97 — the redundant cache step
- ci.yml diff — where the same step was removed
💭 2) create-agents-template/turbo.json:21 Add .next/dev/ exclusion to template turbo.json**
Issue: The PR adds !.next/dev/** to the monorepo's turbo.json but not to create-agents-template/turbo.json (line 21), which uses the same outputs pattern.
Why: Projects scaffolded from the template could experience the same Turbopack dev cache bloat if they enable Turborepo remote caching.
Fix: Consider updating create-agents-template/turbo.json line 21 to: "outputs": ["dist/**", "build/**", ".next/**", "!.next/cache/**", "!.next/dev/**"]. This is lower priority since template users may not use Turborepo remote caching.
Refs:
✅ APPROVE
Summary: Clean, well-researched CI optimization. The changes correctly remove redundant caching infrastructure and prevent Turbopack dev cache bloat. The PR description provides excellent justification with profiling data. The two "Consider" items are minor consistency suggestions for follow-up — neither blocks this PR. Ship it! 🚀
Discarded (4)
| Location | Issue | Reason Discarded |
|---|---|---|
ci.yml |
GitHub Actions using mutable version tags (@v4, @v5) instead of SHA pins | Pre-existing pattern not introduced by this PR |
turbo.json:40 |
Build outputs exclusion is correctly formatted | Positive observation, not an issue |
ci.yml:12-14 |
Permissions are correctly scoped | Positive observation, not an issue |
ci.yml:77 |
Uses frozen lockfile for deterministic installs | Positive observation, not an issue |
Reviewers (2)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
6 | 0 | 2 | 0 | 0 | 0 | 4 |
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 6 | 0 | 2 | 0 | 0 | 0 | 4 |
- ci job: ubuntu-16gb (4 cores/16GB) → ubuntu-32gb (8 cores/32GB) - e2e job: ubuntu-latest (2 cores/7GB) → ubuntu-32gb (8 cores/32GB) - NODE_OPTIONS: 4GB → 8GB heap (agents-api:typecheck was 4m39s bottleneck) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
Re-review scope: This review covers only the delta since the last automated review (commit 53f920cc4).
Delta Analysis
The delta consists of a single commit upgrading CI runner resources:
| Location | Change | Assessment |
|---|---|---|
ci.yml:22 |
ubuntu-16gb → ubuntu-32gb |
✅ Appropriate upgrade given OOM concerns with 9+ parallel Node processes |
ci.yml:100 |
Node heap 4096 → 8192 |
✅ Proportional to runner memory increase (maintains ~25% ratio) |
ci.yml:151 |
ubuntu-latest → ubuntu-32gb |
✅ Consistent with main CI job; E2E job builds multiple packages |
All changes are straightforward resource scaling with sound rationale.
✅ APPROVE
Summary: Clean delta — the runner and heap upgrades are proportional and well-justified given the PR's OOM prevention goals. The Node heap increase from 4GB to 8GB matches the runner memory doubling (16GB → 32GB), maintaining adequate headroom. The prior review's "Consider" items (cypress.yml consistency, template turbo.json) remain valid suggestions for follow-up but don't block this PR. Ship it! 🚀
Discarded (3)
| Location | Issue | Reason Discarded |
|---|---|---|
cypress.yml:18 |
Runner still on ubuntu-16gb while ci.yml upgraded |
Already raised in prior review as "Consider" item #1 — not re-raising |
ci.yml:100 |
Node heap increase is proportional | Positive observation, not an issue |
ci.yml:151 |
E2E job upgrade might be overkill | Speculative cost concern; PR author likely profiled and determined 32GB was needed |
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
3 | 0 | 0 | 0 | 0 | 0 | 3 |
| Total | 3 | 0 | 0 | 0 | 0 | 0 | 3 |
Dead code — no package test script references it. Turbo dispatches tests per-package, each using its own vitest.config.ts. The agents-cli CI config is a separate local file at agents-cli/vitest.config.ci.ts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
PR Review Summary
(1) Total Issues | Risk: Low
Re-review scope: This review covers only the delta since the last automated review (commit 53f920cc4). The delta is a single commit (077b2baaf) deleting the root vitest.config.ci.ts file (66 lines removed).
Delta Analysis
The deletion of vitest.config.ci.ts is safe and correct:
| Verification | Result |
|---|---|
| File was unused | ✅ No references in CI workflows, root package.json, or turbo.json |
| Tests still work | ✅ turbo test invokes per-package test scripts, each with their own vitest configs |
| Active CI config preserved | ✅ agents-cli/vitest.config.ci.ts (which IS referenced) remains untouched |
🟡 Minor (1) 🟡
🟡 1) internal-surface-areas Stale reference to deleted file in skill documentation
Issue: The internal-surface-areas skill at line 85 still references vitest.config.ci.ts in the "Vitest workspace configs" row under Test Infrastructure. This reference is now stale.
Why: The skill serves as internal documentation for the monorepo's surface areas. Stale references create confusion when developers use this skill to understand the codebase's test infrastructure.
Fix: Update .agents/skills/internal-surface-areas/SKILL.md line 85:
- Remove
vitest.config.ci.tsfrom the Source Code column - Update description to remove "with CI-specific overrides" since the root CI override no longer exists
Refs:
🕐 Pending Recommendations (2)
- 💭
scope: workflow consistencyRemove redundant.next/cachestep from cypress.yml for consistency - 💭
create-agents-template/turbo.json:21Add.next/dev/**exclusion to template turbo.json
💡 APPROVE WITH SUGGESTIONS
Summary: The delta is a clean removal of genuinely unused dead code. The root vitest.config.ci.ts was never referenced by any workflow or script — tests run via Turbo which dispatches to per-package configs. One minor suggestion: update the internal-surface-areas skill to remove the now-stale reference. Prior "Consider" items from earlier reviews remain valid suggestions for follow-up but don't block this PR. Ship it! 🚀
Discarded (1)
| Location | Issue | Reason Discarded |
|---|---|---|
coverage.config.ts |
File imports from deleted vitest.config.ci.ts |
Verified false — coverage.config.ts does not import from the deleted file; it's a standalone coverage config |
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
1 | 1 | 0 | 0 | 0 | 0 | 0 |
| Total | 1 | 1 | 0 | 0 | 0 | 0 | 0 |
- Upgrade Cypress workflow runner from ubuntu-16gb to ubuntu-32gb - Remove redundant .next/cache GHA cache step (Turborepo handles this) - Fix npx turbo → pnpm exec turbo in composite action for consistency Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
Re-review scope: This review covers only the delta since the last automated review (commit 077b2baaf). The delta is a single commit (d5fbc44d5) with 3 changes:
| Location | Change | Assessment |
|---|---|---|
cypress.yml:18 |
ubuntu-16gb → ubuntu-32gb |
✅ Consistent with ci.yml runner upgrades |
cypress.yml:90-97 |
Removed .next/cache cache step |
✅ Addresses prior review's "Consider" item #1 |
cypress-e2e/action.yml:47,86 |
pnpm turbo / npx turbo → pnpm exec turbo |
✅ Standardizes turbo invocations |
All delta changes are clean and correct.
🧹 While You're Here (1) 🧹
🧹 1) ci.yml:244 Turbo invocation inconsistency remains in ci.yml
Issue: The delta standardizes turbo invocations in cypress-e2e/action.yml to use pnpm exec turbo, but ci.yml line 244 still uses pnpm turbo run build. Both forms work (pnpm resolves the bin from node_modules), but pnpm exec turbo is explicit.
Why: Consistency across all workflow files makes maintenance easier and avoids confusion about whether the invocation style matters.
Fix: Update line 244 to pnpm exec turbo run build --filter=.... This is cosmetic and can be addressed in a follow-up.
Refs:
🕐 Pending Recommendations (2)
- 💭
create-agents-template/turbo.json:21Add.next/dev/**exclusion to template turbo.json for consistency - 🟡
internal-surface-areasStale reference to deletedvitest.config.ci.tsin skill documentation
✅ APPROVE
Summary: Clean delta that directly addresses the prior review's suggestion to remove the redundant .next/cache step from cypress.yml. The turbo invocation standardization and runner upgrade are well-executed. The two pending recommendations from prior reviews remain valid suggestions for follow-up but don't block this PR. Ship it! 🚀
Discarded (0)
No findings were discarded in this review.
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
1 | 0 | 0 | 1 | 0 | 0 | 0 |
| Total | 1 | 0 | 0 | 1 | 0 | 0 | 0 |
1 participant
Summary
Optimize CI performance across all workflows by upgrading runners, removing redundant caching, cleaning up dead code, and fixing inconsistent turbo invocations.
Motivation
CI wall-clock time was the primary bottleneck for PR iteration speed. The main
cijob took 17-19 minutes onubuntu-16gbrunners, withagents-api:typecheckalone consuming ~4m39s due to V8 heap pressure. The Cypress workflow had similar inefficiencies. Additionally, stale configuration files and inconsistent tool invocations added confusion without benefit.Approach
Systematic audit and optimization of all CI workflows:
ubuntu-16gb/ubuntu-latesttoubuntu-32gb(8 cores, 32GB RAM, 300GB SSD)NODE_OPTIONS --max-old-space-sizefrom 4096 to 8192 to eliminate GC thrashing during TypeScript type-checking.next/cacheGHA caching — Turborepo makes this redundant (on cache hit,next buildis skipped entirely; on cache miss, stale compiler cache provides minimal benefit).next/dev/**from turbo build outputs — Next.js 16+ stores Turbopack dev-time cache here (2-3+ GB); prevents local dev cache from bloating remote cachevitest.config.ci.ts— Never referenced by any package test script; Turbo dispatches tests per-package using each package's own vitest confignpx turboandpnpm turbowithpnpm exec turboin Cypress composite actionArchitectural decisions
.next/cacheGHA cache removal: Verified with Turborepo and Next.js primary sources that this is redundant when using Turborepo with Vercel Remote Cache. Turborepo caching is all-or-nothing — on hit,next buildis skipped entirely and outputs are replayed. The official Turborepo GitHub Actions guide does not recommend caching.next/cacheseparately. (Turborepo CI docs).next/dev/**exclusion: This directory is only created bynext dev, never bynext build, making this a no-op in CI. However, it's a defensive best practice for local development where Turbopack's dev cache can grow to 2-3+ GB and would otherwise be captured into Turborepo's cache artifacts. (Reddit PSA, Next.js 16 upgrade guide)Root
vitest.config.ci.tsremoval: Confirmed dead code — Turbo dispatchestestper-package, each using its ownvitest.config.ts. The root CI config was stale (defined 5 projects vs 7 in rootvitest.config.ts), and Vitest docs confirm "None of the configuration options are inherited from the root-level config file" in workspace mode.Changes
.github/workflows/ci.ymlcijob runner fromubuntu-16gbtoubuntu-32gbcreate-agents-e2ejob runner fromubuntu-latesttoubuntu-32gbNODE_OPTIONS --max-old-space-sizefrom 4096 to 8192actions/cache@v4step.github/workflows/cypress.ymlubuntu-16gbtoubuntu-32gbactions/cache@v4step.github/composite-actions/cypress-e2e/action.ymlnpx turbo build→pnpm exec turbo build(line 86)pnpm turbo run build→pnpm exec turbo run build(line 47)turbo.json!.next/dev/**to build outputs exclusion listvitest.config.ci.ts(deleted)How to verify
agents-api:typecheck: Expected ~1m30s (was 4m39s)ubuntu-latest)vitest.config.ci.tsTest plan
ubuntu-32gbwith increased heapagents-api:typecheckdropped from 4m39s → ~1m30s (-68%)ubuntu-32gbvitest.config.ci.tsis dead code (no references in any package.json test scripts)Generated with Claude Code