fix: align turbo cache keys between CI and Cypress workflows#2043
fix: align turbo cache keys between CI and Cypress workflows#2043nick-inkeep merged 4 commits intomainfrom
Conversation
The Cypress workflow gets 0% turbo cache hits (~101s rebuild) despite CI achieving 100% cache hits (~17s) for the same code. Three root causes of cache key divergence: 1. globalEnv includes ANTHROPIC_API_KEY and OPENAI_API_KEY, which are set in CI but not Cypress. These are runtime-only (env.ts loaded at service start, not during build) and don't affect build outputs. 2. globalDependencies includes ".env", a gitignored file that setup-dev creates in Cypress but never exists in CI. Different file existence produces different global hashes. 3. Build task inputs use ".env*" glob, which matches 2 files in CI but 3 in Cypress (because setup-dev created .env). Confirmed via dry-run: different task hashes (cbcb775e vs e03ac1ed). Fix: - Remove ANTHROPIC_API_KEY and OPENAI_API_KEY from globalEnv - Move them to test task env (tests may depend on mock provider behavior) - Remove .env from globalDependencies (all vars tracked via globalEnv) - Change build inputs from ".env*" to ".env.example" (committed file only) - Add ENVIRONMENT: test to Cypress workflow (matches CI) Expected: Cypress turbo build drops from ~101s to ~17s via cache hits. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
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 correctly addresses turbo cache key divergence between CI and Cypress workflows through three targeted fixes:
-
Scoping runtime-only env vars —
ANTHROPIC_API_KEYandOPENAI_API_KEYare moved fromglobalEnvto thetesttask'senv. These keys are loaded at runtime viaenv.ts(imported inagents-api/src/index.ts), not at build time, so they should not affect build cache keys. -
Removing gitignored file from inputs — Changing
.env*glob to.env.exampleeliminates the hash mismatch caused by.envexisting in Cypress (created bysetup-dev) but not in CI. -
Aligning environment variable — Adding
ENVIRONMENT: testto Cypress workflow ensuresglobalEnvhashes match CI.
🔴❗ Critical (0) ❗🔴
None.
🟠⚠️ Major (0) 🟠⚠️
None.
🟡 Minor (0) 🟡
None.
💭 Consider (0) 💭
None.
✅ APPROVE
Summary: Clean, well-researched fix with solid evidence (dry-run verification, runtime vs build-time analysis). The three changes are logically correct and should achieve the expected ~6x speedup in Cypress builds. Ship it! 🚀
Discarded (2)
| Location | Issue | Reason Discarded |
|---|---|---|
create-agents-template/turbo.json |
Template uses different env strategy (API keys in globalEnv, .env* glob) |
Pre-existing pattern difference, not introduced by this PR. Template is for customer projects with real .env files locally — divergence is intentional. |
.github/workflows/cypress.yml:53 |
GitHub Actions use mutable tags (@v4, @v5) instead of SHA pins | Pre-existing repo-wide pattern across all 35+ action references. Not a regression and out of scope for this cache alignment fix. |
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
5 | 0 | 0 | 0 | 0 | 0 | 2 |
| Total | 5 | 0 | 0 | 0 | 0 | 0 | 2 |
Note: 3 findings were positive validations (INFO severity confirming correctness), not issues.
The DoltGres health check uses `SELECT 1` which only validates basic connectivity. The `dolt_status` system table may not be initialized yet, causing intermittent failures in `setup-dev` migrations: DrizzleQueryError: relation "dolt_status" does not exist Fix: Change health check to `SELECT count(*) FROM dolt_status` which verifies the DoltGres repository is fully initialized before marking the container as healthy. Also align Cypress health check params with CI (retries: 10, start-period: 30s). 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 is a delta review covering 1 new commit (f2517a55) since the prior automated review. The turbo.json cache alignment changes were already reviewed and approved.
Delta Changes Reviewed
-
DoltGres health check improvement — Changed from
SELECT 1toSELECT count(*) FROM dolt_statusin both CI and Cypress workflows. This is a more robust readiness check that verifies DoltGres system tables are initialized, not just that postgres accepts connections. -
Cypress retry/timeout alignment — Aligned Cypress workflow settings with CI:
health-retries: 5 → 10health-start-period: 20s → 30s
Both changes are correct and improve reliability:
- The
dolt_statuscheck ensures Dolt's versioning layer is ready, preventing race conditions where postgres is up but Dolt isn't fully initialized - The retry/timeout alignment matches the existing CI settings (lines 153-157 in ci.yml), ensuring consistent behavior across workflows
🔴❗ Critical (0) ❗🔴
None.
🟠⚠️ Major (0) 🟠⚠️
None.
🟡 Minor (0) 🟡
None.
💭 Consider (0) 💭
None.
✅ APPROVE
Summary: The delta changes improve DoltGres health check reliability and align Cypress workflow settings with CI. Combined with the already-approved turbo cache alignment changes, this PR should achieve the expected ~6x speedup in Cypress builds while also improving container readiness detection. Ship it! 🚀
Discarded (1)
| Location | Issue | Reason Discarded |
|---|---|---|
docker-compose*.yml files |
Docker Compose files still use SELECT 1 for DoltGres health checks instead of dolt_status |
Pre-existing pattern for local development, not introduced by this PR. The PR scope is CI/Cypress workflow alignment; local dev docker-compose files are a separate concern and could be addressed in a follow-up if desired. |
Reviewers (2)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 1 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 1 |
Note: Both reviewers confirmed the delta changes are correct with no issues.
The GHA service container health check is insufficient — GitHub Actions may proceed even with unhealthy containers. Add an explicit readiness poll that blocks until DoltGres system tables (dolt_status) are available before running setup-dev migrations. 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 is a delta review covering 1 new commit (36143d94) since the prior automated review. The turbo.json cache alignment changes and container health check improvements were already reviewed and approved.
Delta Changes Reviewed
Commit 36143d94: Add explicit DoltGres readiness wait before setup-dev
The new step (lines 28-43 in cypress-e2e/action.yml) adds a shell-based readiness wait loop that:
- Queries
SELECT count(*) FROM dolt_statusup to 30 times with 2-second intervals (60s max) - Exits early on first successful connection
- Provides clear timeout messaging if DoltGres isn't ready
This is a belt-and-suspenders approach alongside the container health check. While the container health check uses the same query, there are known edge cases where GitHub Actions reports a service container as healthy before the network path from the runner is fully established. The explicit wait step provides an additional layer of reliability for migration operations that follow.
🔴❗ Critical (0) ❗🔴
None.
🟠⚠️ Major (0) 🟠⚠️
None.
🟡 Minor (0) 🟡
None.
💭 Consider (0) 💭
None.
✅ APPROVE
Summary: The delta commit adds a defensive readiness check that follows established CI patterns (similar to the SpiceDB wait loop at ci.yml:239-246). Combined with the already-approved turbo cache alignment and container health check changes, this PR should achieve the expected ~6x speedup in Cypress builds while improving DoltGres initialization reliability. Ship it! 🚀
Discarded (3)
| Location | Issue | Reason Discarded |
|---|---|---|
cypress-e2e/action.yml:28-43 |
Redundant readiness check duplicates container health check query | Intentional belt-and-suspenders approach; addresses known GitHub Actions edge cases where container health check passes but network path isn't fully established. Minimal latency impact (exits early on success). |
cypress-e2e/action.yml:33 |
Missing psql dependency check / 2>/dev/null hides errors |
The dependency is implicitly satisfied by ubuntu-latest runners. The 2>/dev/null suppresses expected "connection refused" errors during retries; timeout provides clear failure path if psql is truly missing. |
cypress-e2e/action.yml:38-39 |
Timeout message hardcodes "60 seconds" | INFO-level maintainability nitpick; current arithmetic is correct. Adding dynamic computation would add bash complexity without meaningful benefit. |
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 |
Note: All findings were valid observations but assessed as acceptable tradeoffs for CI reliability.
When ENVIRONMENT=test (needed for turbo cache alignment), the createAgentsManageDatabaseClient function returns an in-memory PGLite client instead of connecting to real DoltGres. This causes migrate-dolt.ts to fail with 'relation dolt_status does not exist' because PGLite doesn't have DoltGres system tables. Override ENVIRONMENT=development for setup-dev (which runs migrations) and the API server (which needs real DoltGres at runtime). The ENVIRONMENT=test value is still used by turbo for cache key computation during the build step. 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 is a delta review covering 1 new commit (1edd21ae) since the prior automated review. The turbo.json cache alignment changes, DoltGres health check improvements, and explicit readiness wait loop were already reviewed and approved.
Delta Changes Reviewed
Commit 1edd21ae: Add step-level ENVIRONMENT=development overrides
The delta adds 3 lines to .github/composite-actions/cypress-e2e/action.yml:
- Line 47-48:
env: ENVIRONMENT: developmentfor the "Setup development environment" step - Line 54:
ENVIRONMENT: developmentfor the "Start backend API server" step
Why this is correct:
-
Workflow-level
ENVIRONMENT: test(cypress.yml:128) is required for turbo cache key alignment — turbo includesENVIRONMENTin itsglobalEnvhash, so Cypress must match CI's value during the build phase. -
Step-level
ENVIRONMENT: developmentoverrides are required because:packages/agents-core/src/db/manage/manage-client.tsselects the database client based onENVIRONMENT- When
ENVIRONMENT=test: returns in-memory PGLite (JavaScript SQLite) — which does NOT have DoltGres system tables - When
ENVIRONMENT=development: connects to real PostgreSQL (DoltGres) via node-postgres - The setup-dev step runs migrations that query
dolt_status— this requires the real DoltGres connection - The API server step needs real database connections at runtime
-
Scope isolation: Step-level environment variables in GitHub Actions override workflow-level values only for that specific process. The turbo cache key is computed during the build step (before these steps), so the override doesn't affect cache alignment.
This is the correct resolution for the interaction between turbo cache requirements and database client selection.
🔴❗ Critical (0) ❗🔴
None.
🟠⚠️ Major (0) 🟠⚠️
None.
🟡 Minor (0) 🟡
None.
💭 Consider (0) 💭
None.
✅ APPROVE
Summary: The delta commit correctly resolves the interaction between workflow-level ENVIRONMENT: test (needed for turbo cache alignment) and step-level database connection requirements (needs real DoltGres, not PGLite). Combined with the already-approved turbo cache alignment, DoltGres health check, and readiness wait changes, this PR should achieve the expected ~6x speedup in Cypress builds while ensuring reliable database operations. Well-reasoned fix — ship it! 🚀
Discarded (0)
No discarded findings.
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 changes confirmed correct with no issues. The reviewer validated that step-level env overrides are the appropriate pattern for this use case.
1 participant
Summary
Enable turbo remote cache reuse between the CI and Cypress workflows by aligning their cache key inputs. Also fixes a DoltGres readiness race condition that caused intermittent Cypress failures.
Impact: Cypress turbo build goes from 0% to 75% cache hit rate (3 of 4 packages: agents-core, agents-sdk, agents-cli).
agents-manage-uiremains uncacheable across workflows because Next.js embeds environment variables at build time.Problem
Turbo computes different task hashes in CI vs Cypress due to three mismatches:
globalEnvdivergence —ANTHROPIC_API_KEY,OPENAI_API_KEY, andENVIRONMENTare set in CI but not in Cypress. SinceglobalEnventries are hashed into every task, any difference invalidates the entire cache.globalDependenciesdivergence —.envis listed as a global dependency. Cypress creates it viasetup-dev; CI does not. The file is gitignored, so its presence/absence varies by workflow..env*glob in build task inputs matches the gitignored.envin Cypress but finds nothing in CI, producing different input hashes.Separately, DoltGres health checks used
SELECT 1(connectivity only), which could pass before Dolt system tables were available — causingsetup-devmigrations to fail ondolt_status.Changes
turbo.json:
globalDependencies: [".env"]— gitignored file causes cross-workflow hash mismatchANTHROPIC_API_KEYandOPENAI_API_KEYfromglobalEnv— these are runtime-only values that do not affect build outputs; moved totesttaskenvwhere they belong.env*to.env.examplein build task inputs — tracks the committed schema file, not the gitignored instancecypress.yml:
ENVIRONMENT: testto match CI (turbo hash alignment)SELECT count(*) FROM dolt_statusvalidates system table readiness, not just TCP connectivityci.yml:
dolt_statusquery)cypress-e2e/action.yml:
setup-dev(defense-in-depth against container health check races)ENVIRONMENT=developmentonsetup-devand API server steps — the workflow-levelENVIRONMENT=testis for turbo hash computation only; application code must seedevelopmentsocreateAgentsManageDatabaseClient()connects to real DoltGres instead of returning an in-memory PGLite clientNotes
turbo.jsoninvalidates all existing remote cache entries. The first CI and Cypress runs after merge will rebuild the cache (~16 min). Subsequent runs return to normal.env.tsZod validation). Removing.envfrom tracking has no effect — CI never creates it, andnext buildsetsNODE_ENV=productionwhich skips.envloading. TheENVIRONMENT=developmentoverrides preserve the same database connectivity behavior Cypress had before this PR.Test plan
ANTHROPIC_API_KEYandOPENAI_API_KEYare not used during any build task (runtime-only viaenv.ts).envis not read at build time (next.config.tsskips env loading whenNODE_ENV=production)turbo build --dry-run)turbo test --dry-run=json)🤖 Generated with Claude Code