👷 github: cache nx database - #1315
nfmelendez wants to merge 2 commits into
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughNx now tracks additional environment variables and GitHub Actions caches Nx workspace database files for test and e2e jobs. ChangesNx cache tracking
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to This change only adjusts build-cache tracking so cached bundles are not reused across different chains or environments, and extends the CI cache to include Nx workspace database files. Configuration files parse correctly and no product behavior is affected, so it is ready to merge. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37beb585f1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| { | ||
| "env": "EXPO_PUBLIC_ENV" | ||
| }, |
There was a problem hiding this comment.
Include the e2e mnemonic in the mobile cache inputs
When EXPO_PUBLIC_E2E_MNEMONIC changes without another hashed input changing, Nx can restore a mobile bundle containing the previous value: Expo embeds this variable into the web bundle at src/utils/e2e.ts:27, while the e2e workflow supplies the current secret and server/test/anvil.ts:56-59 funds the account derived from that current value. The restored app therefore signs with an unfunded account and the web flow fails, so add EXPO_PUBLIC_E2E_MNEMONIC to these mobile inputs alongside EXPO_PUBLIC_ENV.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a56dcbeb25
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| .nx/workspace-data/*.db* | ||
| key: nx-${{ runner.os }}-${{ github.sha }} | ||
| restore-keys: nx-${{ runner.os }}- | ||
| - run: ls -la .nx/workspace-data; cat /etc/machine-id; cat /var/lib/dbus/machine-id; hostname |
There was a problem hiding this comment.
Remove the cache inspection that fails on a cold cache
In the test job, when neither the exact key nor any restore key exists, actions/cache leaves .nx/workspace-data absent, so this first ls exits nonzero and GitHub's bash -e shell terminates the job before dependencies or tests run. GNU ls --help specifically documents status 2 for failure to access a command-line argument. Remove this diagnostic step or guard the directory access so a fresh or evicted cache can be populated.
Useful? React with 👍 / 👎.
a56dcbe to
37beb58
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37beb585f1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| path: .nx/cache | ||
| path: | | ||
| .nx/cache | ||
| .nx/workspace-data/*.db* |
There was a problem hiding this comment.
Invalidate restored task records when CI toolchains change
When a commit changes only a CI-provided toolchain—such as the Foundry version configured at .github/workflows/test.yaml:21-22—the exact SHA key misses, but restore-keys: nx-${{ runner.os }}- restores the preceding database because restore keys are prefix-matched (GitHub cache documentation). The restored Nx task hashes do not include this workflow or the externally installed Foundry version (sharedGlobals contains only nx.json, package.json, and pnpm-lock.yaml), so cached contract test and coverage records can cause the suite to be skipped under the newly selected toolchain. Include toolchain/workflow inputs in the cache namespace or Nx hashes before persisting the database.
Useful? React with 👍 / 👎.
| { | ||
| "env": "CHAIN_ID" | ||
| }, |
There was a problem hiding this comment.
Include the alternate chain selector in the e2e hash
When CHAIN_ID is unset, common/wagmi.config.ts:18 selects Optimism on EAS and Optimism Sepolia otherwise based on EAS_BUILD_RUNNER. The e2e bundle imports exaAccountFactoryAddress from the generated chain module, but ^production excludes generated/**; hashing only CHAIN_ID therefore gives the two environments the same e2e:build hash even though their generated factory addresses differ. A build after changing only EAS_BUILD_RUNNER can restore getAccount.js for the wrong chain, so add EAS_BUILD_RUNNER to this bundle input as the mobile project already does.
Useful? React with 👍 / 👎.
what
cache
.nx/workspace-data/*.db*next to.nx/cache, so the restored nx cache is actually usable, and addCHAIN_IDandEXPO_PUBLIC_ENVto the nx inputs of the app and e2e bundles, so a working cache never hands one job a bundle built for another chain.stacked on #1314: with cache hits working, the stale
lcov.infofiles restored into.nx/cachewould reach codecov even more often unless uploads are already restricted to explicit files.nx cache
since nx 19 the cache index lives in a sqlite database under
.nx/workspace-data, and.nx/cacheonly holds outputs. the workflow persisted.nx/cachealone, so nx ignored every restored entry: thetestjob read 7 of 59 tasks from cache whether the restored cache was present or empty, and those 7 were thegenerate:*tasks written bypnpm installin the same run. the database file is named after the machine id, read from/etc/machine-id, and github runners share it: a probe on a fresh runner (runnervmlun5p) found58b34b8c91a94400a52c175421986a53-v3.dbrestored from a cache saved by another runner, matching its own/etc/machine-id, so a cache saved on one runner is recognized on another.nx inputs
a working cache exposed a gap in the task hashes.
common/generated/chain.tsis produced bygenerate:wagmifromCHAIN_ID, but thebuildtargets of the app and of.maestrohash^production, which excludesgenerated/**, and did not listCHAIN_ID. thetestjob (noCHAIN_ID) and thee2ejob (CHAIN_ID=31337) therefore computed the same hash fore2e:buildwith different contents. on the first re-run thee2ejob restored thetestjob'sgetAccount.js, built against the optimism sepolia factory, and the web flow failed asserting0x0Df0…4bA5while the app showed0x699F…7C6e.CHAIN_IDis now an input of both bundles, andEXPO_PUBLIC_ENVof the app, which inlines it. the expo fingerprint is unchanged:fingerprint.config.cjsignores the rootpackage.jsonand.maestro/package.jsonis not a source.verification
two attempts of the same run on different runners, the second restoring the exact cache key saved by the first:
teste2ee2e:buildCHAIN_ID=31337hash is absent from thetestjob cache🤖 Generated with Claude Code
Summary by CodeRabbit