chore(e2e): expose Zustand stores in all builds for E2E test access#302
Conversation
__ZUSTAND_STORES was behind import.meta.env.DEV, so it was never set in CI production builds. This caused the vault readiness check in E2E test 3.7 (and other tests using __ZUSTAND_STORES) to hang forever waiting for stores that don't exist. The stores are read-only Zustand hooks — no secrets beyond what the browser already holds in memory. Multiple E2E tests depend on them for auth state extraction, vault readiness checks, and public key reads. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: ada05ad0948b
WalkthroughNarrowed E2E exposure and replaced broad window store injection with a minimal Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/main.tsx`:
- Around line 57-70: Currently you unconditionally attach the raw Zustand hooks
to window via __ZUSTAND_STORES (auth: useAuthStore, vault: useVaultStore,
folder: useFolderStore, sync: useSyncStore), which exposes getState/setState and
secrets; gate this assignment behind an E2E-only build flag (e.g.
process.env.REACT_APP_E2E === 'true' or a compile-time FLAG_E2E) so it only runs
in test builds, and replace exposing raw hooks with a minimal test API that only
returns the specific read-only values Playwright needs (e.g. helper functions
like getAuthPublicState(), isVaultReady(), getPublicKeys()) instead of exposing
useAuthStore/useVaultStore/useFolderStore directly; ensure the code checks the
flag before creating window.__ZUSTAND_STORES and that the exposed helpers do not
provide access to private keys or setState.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cbfeb854-bbb9-414e-a315-a2d749873ef0
📒 Files selected for processing (1)
apps/web/src/main.tsx
Replace unconditional store exposure with a VITE_E2E env var check. Production deploys never set this flag, so the code is tree-shaken out. CI E2E workflow sets VITE_E2E=true at build time. - main.tsx: expose __ZUSTAND_STORES only when DEV or VITE_E2E is set - e2e.yml: add VITE_E2E=true to web build env vars Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 5d735eafde14
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/web/src/main.tsx (1)
59-59:⚠️ Potential issue | 🟠 MajorUse explicit
'true'check forVITE_E2Eguard.At Line 59,
import.meta.env.VITE_E2Eis evaluated by truthiness;'false'is still truthy and would expose__ZUSTAND_STORESunintentionally.🔧 Proposed fix
-if (import.meta.env.DEV || import.meta.env.VITE_E2E) { +if (import.meta.env.DEV || import.meta.env.VITE_E2E === 'true') {#!/bin/bash set -euo pipefail # Verify current guard usage and whether strict equality is used. rg -n "import\.meta\.env\.VITE_E2E|VITE_E2E\s*===" apps/web/src/main.tsx # Verify where VITE_E2E is defined in workflows and with what values. rg -n "VITE_E2E" .github/workflows🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/main.tsx` at line 59, The guard currently uses import.meta.env.VITE_E2E by truthiness which treats the string "false" as true; update the condition so VITE_E2E is compared explicitly to the string 'true' (e.g., change the if in main.tsx from if (import.meta.env.DEV || import.meta.env.VITE_E2E) to if (import.meta.env.DEV || import.meta.env.VITE_E2E === 'true')) so __ZUSTAND_STORES is only exposed when VITE_E2E is actually set to 'true'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/web/src/main.tsx`:
- Line 59: The guard currently uses import.meta.env.VITE_E2E by truthiness which
treats the string "false" as true; update the condition so VITE_E2E is compared
explicitly to the string 'true' (e.g., change the if in main.tsx from if
(import.meta.env.DEV || import.meta.env.VITE_E2E) to if (import.meta.env.DEV ||
import.meta.env.VITE_E2E === 'true')) so __ZUSTAND_STORES is only exposed when
VITE_E2E is actually set to 'true'.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c40d3d73-409e-4d9b-a0c3-07e3915d6420
📒 Files selected for processing (2)
.github/workflows/e2e.ymlapps/web/src/main.tsx
- wallet-login-helpers: replace vault readiness poll via __ZUSTAND_STORES with waiting for file list / empty state - full-workflow test 3.7: remove __ZUSTAND_STORES vault poll, rely on file list appearing as proof of vault + SDK readiness - multi-account-wallet: extract public key from Settings page UI instead of reading Zustand auth store. Remove accessToken and rootIpnsName from WalletTestAccount (only used by conflict detection which reads stores directly) Remaining __ZUSTAND_STORES usages (gated by VITE_E2E): - conflict-detection: needs accessToken + rootIpnsName for direct API calls that simulate another device (no UI equivalent) - web3auth-helpers: old test-login state injection infrastructure - recycle-bin: best-effort quota check with null fallback Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 17fcef0f2c15
Instead of exposing all Zustand store hooks (which include setState, private keys, and folder keys), expose only two getter functions via window.__E2E: - getAccessToken() — for conflict-detection API calls - getRootIpnsName() — for conflict-detection sequence bumps This is the only active test that needs in-browser state. All other tests now use UI elements (file list, Settings page public key). Removed DEV guard — __E2E is exclusively behind VITE_E2E which is only set in the CI E2E workflow build. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 8b7cf8245b9e
There was a problem hiding this comment.
Pull request overview
This PR addresses E2E flakiness/hangs caused by relying on window.__ZUSTAND_STORES in CI contexts, by (a) gating store exposure behind an explicit E2E build flag and (b) shifting some E2E readiness checks / key extraction to UI signals instead of Zustand internals.
Changes:
- Expose
window.__ZUSTAND_STORESin web builds whenVITE_E2Eis enabled (or in dev), and setVITE_E2Ein the E2E GitHub Actions workflow build step. - Update wallet-based E2E helpers/tests to wait for file list / empty state UI rather than polling vault keys from Zustand.
- Change multi-account wallet helper to extract the user’s public key from the Settings UI instead of reading the auth store.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
apps/web/src/main.tsx |
Adds `DEV |
.github/workflows/e2e.yml |
Sets VITE_E2E=true during the CI build step to enable E2E-only hooks in production builds. |
tests/e2e/utils/wallet-login-helpers.ts |
Replaces Zustand-based “vault ready” polling with UI-based readiness signals. |
tests/e2e/utils/multi-account-wallet.ts |
Switches public key extraction to Settings UI and removes access-token/root-ipns extraction from Zustand. |
tests/e2e/tests/full-workflow.spec.ts |
Removes Zustand vault readiness polling in the page reload test and relies on UI sync completion. |
… wait - Compare VITE_E2E === 'true' explicitly (not truthy) to prevent accidental exposure when set to 'false' or other strings - Validate extracted public key: trim whitespace, assert non-empty and starts with 0x, throw descriptive error on failure - Remove duplicate file-list wait in createWalletTestAccount since loginViaWallet already performs the same readiness check Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: c03f99b91c5f
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/e2e/utils/multi-account-wallet.ts (1)
35-35: Consider handling emptytextContentmore explicitly.If the element exists but has no text (e.g., timing issue), the function returns an empty string which might cause confusing test failures downstream.
🛡️ Optional: Add validation for empty publicKey
const publicKey = (await pubKeyElement.textContent()) ?? ''; + if (!publicKey) { + throw new Error('Public key element found but textContent was empty'); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/utils/multi-account-wallet.ts` at line 35, The current assignment const publicKey = (await pubKeyElement.textContent()) ?? '' can silently produce an empty string; change it to explicitly validate the result of await pubKeyElement.textContent() (check for null or empty string) and either retry/wait for text to appear or throw a clear error; reference the pubKeyElement/TextContent call and the publicKey variable so the fix is applied where the element value is read (e.g., replace the null-coalescing with an explicit check and surface a descriptive error or implement a short retry/wait loop before failing).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/e2e/utils/multi-account-wallet.ts`:
- Line 35: The current assignment const publicKey = (await
pubKeyElement.textContent()) ?? '' can silently produce an empty string; change
it to explicitly validate the result of await pubKeyElement.textContent() (check
for null or empty string) and either retry/wait for text to appear or throw a
clear error; reference the pubKeyElement/TextContent call and the publicKey
variable so the fix is applied where the element value is read (e.g., replace
the null-coalescing with an explicit check and surface a descriptive error or
implement a short retry/wait loop before failing).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0fdba5b4-127d-4c7b-9622-a5d3275177ba
📒 Files selected for processing (5)
apps/web/src/main.tsxtests/e2e/tests/conflict-detection.spec.tstests/e2e/tests/full-workflow.spec.tstests/e2e/utils/multi-account-wallet.tstests/e2e/utils/wallet-login-helpers.ts
Summary
Root cause of the persistent test 3.7 failure:
window.__ZUSTAND_STORESwas only set whenimport.meta.env.DEVis true, but CI runs a production build (pnpm --filter @cipherbox/web build). ThewaitForFunctionchecking vault readiness via__ZUSTAND_STORESwould hang forever because the stores were never exposed.This worked before PR #296 because the old test flow used
reinjectTestAuthAfterReload()(which manually injected auth state) instead of checking Zustand stores for vault readiness.Changes
apps/web/src/main.tsx: Removeimport.meta.env.DEVguard around__ZUSTAND_STORESassignment. The stores are read-only Zustand hooks — no secrets beyond what the browser already holds in memory. Multiple E2E tests depend on them (conflict-detection, multi-account-wallet, sharing-workflow, full-workflow).Test plan
__ZUSTAND_STORES__ZUSTAND_STORES(conflict-detection, sharing-workflow) pass🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
Tests