ci: Update cache key for packages-build in CI workflow#40101
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughThe CI workflow's cache key strategy for the packages-build artifact was updated to use the source archive hash instead of the previously computed packages build hash for cache invalidation. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
190-194:⚠️ Potential issue | 🟡 MinorAlign cache observability with the new cache key.
After Line 188 switched to
source-hash, Line 194 still printspackages-build-cache-key, which makes cache diagnostics misleading.Proposed patch
- name: Cache observability (packages-build) run: | echo "### 📦 packages-build cache" >> $GITHUB_STEP_SUMMARY echo "- **hit**: \`${{ steps.packages-cache-build.outputs.cache-hit }}\`" >> $GITHUB_STEP_SUMMARY - echo "- **key**: \`${{ needs.release-versions.outputs.packages-build-cache-key }}\`" >> $GITHUB_STEP_SUMMARY + echo "- **key**: \`${{ needs.release-versions.outputs.source-hash }}\`" >> $GITHUB_STEP_SUMMARY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 190 - 194, The cache observability block prints the old cache key variable (needs.release-versions.outputs.packages-build-cache-key) which is now incorrect after switching to the new `source-hash`; update the echo that prints the key in the "Cache observability (packages-build)" step to use the new output name (needs.release-versions.outputs.source-hash) so the summary shows the actual key being used, and leave the cache-hit line (steps.packages-cache-build.outputs.cache-hit) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 190-194: The cache observability block prints the old cache key
variable (needs.release-versions.outputs.packages-build-cache-key) which is now
incorrect after switching to the new `source-hash`; update the echo that prints
the key in the "Cache observability (packages-build)" step to use the new output
name (needs.release-versions.outputs.source-hash) so the summary shows the
actual key being used, and leave the cache-hit line
(steps.packages-cache-build.outputs.cache-hit) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4eaa13c3-dc38-48e1-a9f6-fe49f0c86724
📒 Files selected for processing (1)
.github/workflows/ci.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
188-188: Cache key migration tosource-hashis a solid fix.Line 188 now invalidates
packages-buildcache on source changes, which avoids stale build artifact reuse.
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/ci.yml">
<violation number="1" location=".github/workflows/ci.yml:188">
P2: The cache summary now prints a different key than the one used for the `packages-build` cache, which makes cache-hit debugging misleading.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| path: | | ||
| /tmp/RocketChat-packages-build.tar.gz | ||
| key: ${{ runner.arch }}-${{ runner.os }}-packages-build-${{ needs.release-versions.outputs.packages-build-cache-key }} | ||
| key: ${{ runner.arch }}-${{ runner.os }}-packages-build-${{ needs.release-versions.outputs.source-hash }} |
There was a problem hiding this comment.
P2: The cache summary now prints a different key than the one used for the packages-build cache, which makes cache-hit debugging misleading.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yml, line 188:
<comment>The cache summary now prints a different key than the one used for the `packages-build` cache, which makes cache-hit debugging misleading.</comment>
<file context>
@@ -185,7 +185,7 @@ jobs:
path: |
/tmp/RocketChat-packages-build.tar.gz
- key: ${{ runner.arch }}-${{ runner.os }}-packages-build-${{ needs.release-versions.outputs.packages-build-cache-key }}
+ key: ${{ runner.arch }}-${{ runner.os }}-packages-build-${{ needs.release-versions.outputs.source-hash }}
- name: Cache observability (packages-build)
</file context>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #40101 +/- ##
===========================================
+ Coverage 70.11% 70.12% +0.01%
===========================================
Files 3273 3273
Lines 116237 116237
Branches 20580 20610 +30
===========================================
+ Hits 81497 81514 +17
+ Misses 31437 31420 -17
Partials 3303 3303
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
ARCH-2085
Summary by CodeRabbit